mmc: sdhci-brcmstb: host controller clock enhancements

From: Al Cooper <[email protected]>

Re-organize the flags by basing the bit names on the flag that they apply to. Also change the "flags" member in the "brcmstb_match_priv" struct to const.

Signed-off-by Al Cooper <[email protected]>

Signed-off-by Kamal Dasu <[email protected]>

Acked-by Florian Fainelli <[email protected]>

Acked-by Adrian Hunter <[email protected]>

---
 drivers/mmc/host/sdhci-brcmstb.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

drivers/mmc/host/sdhci-brcmstb.c +16 -17

index f24623aac2db..244780481193 100644
@@ -18,20 +18,22 @@
 #define SDHCI_VENDOR 0x78
 #define  SDHCI_VENDOR_ENHANCED_STRB 0x1
 
-#define BRCMSTB_PRIV_FLAGS_NO_64BIT            BIT(0)
-#define BRCMSTB_PRIV_FLAGS_BROKEN_TIMEOUT      BIT(1)
+#define BRCMSTB_MATCH_FLAGS_NO_64BIT           BIT(0)
+#define BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT     BIT(1)
+
+#define BRCMSTB_PRIV_FLAGS_HAS_CQE             BIT(0)
 
 #define SDHCI_ARASAN_CQE_BASE_ADDR             0x200
 
 struct sdhci_brcmstb_priv {
        void __iomem *cfg_regs;
-       bool has_cqe;
+       unsigned int flags;
 };
 
 struct brcmstb_match_priv {
        void (*hs400es)(struct mmc_host *mmc, struct mmc_ios *ios);
        struct sdhci_ops *ops;
-       unsigned int flags;
+       const unsigned int flags;
 };
 
 static void sdhci_brcmstb_hs400es(struct mmc_host *mmc, struct mmc_ios *ios)

@@ -134,13 +136,13 @@ static struct sdhci_ops sdhci_brcmstb_ops_7216 = {
 };
 
 static struct brcmstb_match_priv match_priv_7425 = {
-       .flags = BRCMSTB_PRIV_FLAGS_NO_64BIT |
-       BRCMSTB_PRIV_FLAGS_BROKEN_TIMEOUT,
+       .flags = BRCMSTB_MATCH_FLAGS_NO_64BIT |
+       BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT,
        .ops = &sdhci_brcmstb_ops,
 };
 
 static struct brcmstb_match_priv match_priv_7445 = {
-       .flags = BRCMSTB_PRIV_FLAGS_BROKEN_TIMEOUT,
+       .flags = BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT,
        .ops = &sdhci_brcmstb_ops,
 };
 

@@ -176,7 +178,7 @@ static int sdhci_brcmstb_add_host(struct sdhci_host *host,
        bool dma64;
        int ret;
 
-       if (!priv->has_cqe)
+       if ((priv->flags & BRCMSTB_PRIV_FLAGS_HAS_CQE) == 0)
                return sdhci_add_host(host);
 
        dev_dbg(mmc_dev(host->mmc), "CQE is enabled\n");

@@ -225,7 +227,6 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
        struct sdhci_brcmstb_priv *priv;
        struct sdhci_host *host;
        struct resource *iomem;
-       bool has_cqe = false;
        struct clk *clk;
        int res;
 

@@ -244,10 +245,6 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
                return res;
 
        memset(&brcmstb_pdata, 0, sizeof(brcmstb_pdata));
-       if (device_property_read_bool(&pdev->dev, "supports-cqe")) {
-               has_cqe = true;
-               match_priv->ops->irq = sdhci_brcmstb_cqhci_irq;
-       }
        brcmstb_pdata.ops = match_priv->ops;
        host = sdhci_pltfm_init(pdev, &brcmstb_pdata,
                                sizeof(struct sdhci_brcmstb_priv));

@@ -258,7 +255,10 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
 
        pltfm_host = sdhci_priv(host);
        priv = sdhci_pltfm_priv(pltfm_host);
-       priv->has_cqe = has_cqe;
+       if (device_property_read_bool(&pdev->dev, "supports-cqe")) {
+               priv->flags |= BRCMSTB_PRIV_FLAGS_HAS_CQE;
+               match_priv->ops->irq = sdhci_brcmstb_cqhci_irq;
+       }
 
        /* Map in the non-standard CFG registers */
        iomem = platform_get_resource(pdev, IORESOURCE_MEM, 1);

@@ -287,14 +287,14 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
         * properties through mmc_of_parse().
         */
        host->caps = sdhci_readl(host, SDHCI_CAPABILITIES);
-       if (match_priv->flags & BRCMSTB_PRIV_FLAGS_NO_64BIT)
+       if (match_priv->flags & BRCMSTB_MATCH_FLAGS_NO_64BIT)
                host->caps &= ~SDHCI_CAN_64BIT;
        host->caps1 = sdhci_readl(host, SDHCI_CAPABILITIES_1);
        host->caps1 &= ~(SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_SDR104 |
                         SDHCI_SUPPORT_DDR50);
        host->quirks |= SDHCI_QUIRK_MISSING_CAPS;
 
-       if (match_priv->flags & BRCMSTB_PRIV_FLAGS_BROKEN_TIMEOUT)
+       if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
                host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
        res = sdhci_brcmstb_add_host(host, priv);
--

2.17.1

From: Al Cooper <[email protected]>

Enabling this feature will allow the controller to stop the bus clock when the bus is idle. The feature is not part of the standard and is unique to newer Arasan cores and is enabled with a bit in a vendor specific register. This feature will only be enabled for non-removable devices because they don't switch the voltage and clock gating breaks SD Card volatge switching.

Signed-off-by Al Cooper <[email protected]>

Signed-off-by Kamal Dasu <[email protected]>

Acked-by Florian Fainelli <[email protected]>

Acked-by Adrian Hunter <[email protected]>

---
 drivers/mmc/host/sdhci-brcmstb.c | 35 +++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

drivers/mmc/host/sdhci-brcmstb.c +34 -2

index 244780481193..683d0c685748 100644
@@ -17,11 +17,14 @@
 
 #define SDHCI_VENDOR 0x78
 #define  SDHCI_VENDOR_ENHANCED_STRB 0x1
+#define  SDHCI_VENDOR_GATE_SDCLK_EN 0x2
 
 #define BRCMSTB_MATCH_FLAGS_NO_64BIT           BIT(0)
 #define BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT     BIT(1)
+#define BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE     BIT(2)
 
 #define BRCMSTB_PRIV_FLAGS_HAS_CQE             BIT(0)
+#define BRCMSTB_PRIV_FLAGS_GATE_CLOCK          BIT(1)
 
 #define SDHCI_ARASAN_CQE_BASE_ADDR             0x200
 

@@ -36,6 +39,27 @@ struct brcmstb_match_priv {
        const unsigned int flags;
 };
 
+static inline void enable_clock_gating(struct sdhci_host *host)
+{
+       u32 reg;
+
+       reg = sdhci_readl(host, SDHCI_VENDOR);
+       reg |= SDHCI_VENDOR_GATE_SDCLK_EN;
+       sdhci_writel(host, reg, SDHCI_VENDOR);
+}
+
+void brcmstb_reset(struct sdhci_host *host, u8 mask)
+{
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_brcmstb_priv *priv = sdhci_pltfm_priv(pltfm_host);
+
+       sdhci_reset(host, mask);
+
+       /* Reset will clear this, so re-enable it */
+       if (priv->flags & BRCMSTB_PRIV_FLAGS_GATE_CLOCK)
+               enable_clock_gating(host);
+}
+
 static void sdhci_brcmstb_hs400es(struct mmc_host *mmc, struct mmc_ios *ios)
 {
        struct sdhci_host *host = mmc_priv(mmc);

@@ -131,7 +155,7 @@ static struct sdhci_ops sdhci_brcmstb_ops = {
 static struct sdhci_ops sdhci_brcmstb_ops_7216 = {
        .set_clock = sdhci_brcmstb_set_clock,
        .set_bus_width = sdhci_set_bus_width,
-       .reset = sdhci_reset,
+       .reset = brcmstb_reset,
        .set_uhs_signaling = sdhci_brcmstb_set_uhs_signaling,
 };
 

@@ -147,6 +171,7 @@ static struct brcmstb_match_priv match_priv_7445 = {
 };
 
 static const struct brcmstb_match_priv match_priv_7216 = {
+       .flags = BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE,
        .hs400es = sdhci_brcmstb_hs400es,
        .ops = &sdhci_brcmstb_ops_7216,
 };

@@ -273,6 +298,14 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
        if (res)
                goto err;
 
+       /*
+        * Automatic clock gating does not work for SD cards that may
+        * voltage switch so only enable it for non-removable devices.
+        */
+       if ((match_priv->flags & BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE) &&
+           (host->mmc->caps & MMC_CAP_NONREMOVABLE))
+               priv->flags |= BRCMSTB_PRIV_FLAGS_GATE_CLOCK;
+
        /*
         * If the chip has enhanced strobe and it's enabled, add
         * callback
--

2.17.1

Quoted 9 lines · Kamal Dasu

From: Al Cooper <[email protected]>

Enabling this feature will allow the controller to stop the bus clock when the bus is idle. The feature is not part of the standard and is unique to newer Arasan cores and is enabled with a bit in a vendor specific register. This feature will only be enabled for non-removable devices because they don't switch the voltage and clock gating breaks SD Card volatge switching.

Rather than using a HW specific thing for this, it may be better to use runtime PM. There are plenty of examples to get inspired from, so it should be rather easy to implement, I think. More importantly, it should work for both (e)MMC and SD cards, unless there are some specific things to manage for this controller.

When it comes to SDIO, some driver simply bumps the runtime PM usage count (pm_runtime_get_noresume()) to prevent the device from being runtime suspended. There are ways to work around this, let me know if you need some guidance around how to fix that too.

That said, I am not entirely opposed to $subject patch, but I wanted to point out that there are better alternatives.

From: Al Cooper <[email protected]>

Enabling this feature will allow the controller to stop the bus clock when the bus is idle. The feature is not part of the standard and is unique to newer Arasan cores and is enabled with a bit in a vendor specific register. This feature will only be enabled for non-removable devices because they don't switch the voltage and clock gating breaks SD Card volatge switching.

Signed-off-by Al Cooper <[email protected]>

Signed-off-by Kamal Dasu <[email protected]>

Acked-by Florian Fainelli <[email protected]>

Acked-by Adrian Hunter <[email protected]>

---
 drivers/mmc/host/sdhci-brcmstb.c | 35 +++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

drivers/mmc/host/sdhci-brcmstb.c +34 -2

index 244780481193..683d0c685748 100644
@@ -17,11 +17,14 @@
 
 #define SDHCI_VENDOR 0x78
 #define  SDHCI_VENDOR_ENHANCED_STRB 0x1
+#define  SDHCI_VENDOR_GATE_SDCLK_EN 0x2
 
 #define BRCMSTB_MATCH_FLAGS_NO_64BIT           BIT(0)
 #define BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT     BIT(1)
+#define BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE     BIT(2)
 
 #define BRCMSTB_PRIV_FLAGS_HAS_CQE             BIT(0)
+#define BRCMSTB_PRIV_FLAGS_GATE_CLOCK          BIT(1)
 
 #define SDHCI_ARASAN_CQE_BASE_ADDR             0x200
 

@@ -36,6 +39,27 @@ struct brcmstb_match_priv {
        const unsigned int flags;
 };
 
+static inline void enable_clock_gating(struct sdhci_host *host)
+{
+       u32 reg;
+
+       reg = sdhci_readl(host, SDHCI_VENDOR);
+       reg |= SDHCI_VENDOR_GATE_SDCLK_EN;
+       sdhci_writel(host, reg, SDHCI_VENDOR);
+}
+
+void brcmstb_reset(struct sdhci_host *host, u8 mask)
+{
+       struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+       struct sdhci_brcmstb_priv *priv = sdhci_pltfm_priv(pltfm_host);
+
+       sdhci_reset(host, mask);
+
+       /* Reset will clear this, so re-enable it */
+       if (priv->flags & BRCMSTB_PRIV_FLAGS_GATE_CLOCK)
+               enable_clock_gating(host);
+}
+
 static void sdhci_brcmstb_hs400es(struct mmc_host *mmc, struct mmc_ios *ios)
 {
        struct sdhci_host *host = mmc_priv(mmc);

@@ -131,7 +155,7 @@ static struct sdhci_ops sdhci_brcmstb_ops = {
 static struct sdhci_ops sdhci_brcmstb_ops_7216 = {
        .set_clock = sdhci_brcmstb_set_clock,
        .set_bus_width = sdhci_set_bus_width,
-       .reset = sdhci_reset,
+       .reset = brcmstb_reset,
        .set_uhs_signaling = sdhci_brcmstb_set_uhs_signaling,
 };
 

@@ -147,6 +171,7 @@ static struct brcmstb_match_priv match_priv_7445 = {
 };
 
 static const struct brcmstb_match_priv match_priv_7216 = {
+       .flags = BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE,
        .hs400es = sdhci_brcmstb_hs400es,
        .ops = &sdhci_brcmstb_ops_7216,
 };

@@ -273,6 +298,14 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
        if (res)
                goto err;
 
+       /*
+        * Automatic clock gating does not work for SD cards that may
+        * voltage switch so only enable it for non-removable devices.
+        */
+       if ((match_priv->flags & BRCMSTB_MATCH_FLAGS_HAS_CLOCK_GATE) &&
+           (host->mmc->caps & MMC_CAP_NONREMOVABLE))
+               priv->flags |= BRCMSTB_PRIV_FLAGS_GATE_CLOCK;
+
        /*
         * If the chip has enhanced strobe and it's enabled, add
         * callback
--

2.17.1

clocks:

Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml +6 -3

-    maxItems: 1
-    description:
-      handle to core clock for the sdhci controller.
+    minItems: 1
+    items:
+      - description: handle to core clock for the sdhci controller
+      - description: improved 150Mhz clock for sdhci controller (Optional clock)
 
   clock-names:
+    minItems: 1
     items:
       - const: sw_sdio
+      - const: sdio_freq # Optional clock
 
   sdhci,auto-cmd12:
     type: boolean
     description: Specifies that controller should use auto CMD12

The 72116B0 has improved SDIO controllers that allow the max clock rate to be increased from a max of 100MHz to a max of 150MHz. Optional "sdio_freq" clock is used to drive the bus clock if present optional property "max-frequency" specifies a base clock frequency in Hz that overrides the base clock frequency in the CAPS registers.

Signed-off-by Kamal Dasu <[email protected]>

---
 .../bindings/mmc/brcm,sdhci-brcmstb.yaml      | 24 +++++++++++++++----
 1 file changed, 19 insertions(+), 5 deletions(-)

Documentation/devicetree/bindings/mmc/brcm,sdhci-brcmstb.yaml +0 -0

index dccd5ad96981..bf716c0cf096 100644

From: Al Cooper <[email protected]>

The 72116B0 has improved SDIO controllers that allow the max clock rate to be increased from a max of 100MHz to a max of 150MHz. The driver will need to get the clock and increase it's default rate and override the caps register, that still indicates a max of 100MHz. The new clock will be named "sdio_freq" in the DT node's "clock-names" list. The driver will use a DT property, "clock-frequency", to enable this functionality and will get the actual rate in MHz from the property to allow various speeds to be requested.

Signed-off-by Al Cooper <[email protected]>

Signed-off-by Kamal Dasu <[email protected]>

---
  drivers/mmc/host/sdhci-brcmstb.c | 25 +++++++++++++++++++++++++
  1 file changed, 25 insertions(+)

drivers/mmc/host/sdhci-brcmstb.c +3 -0

index 683d0c685748..51a23e9f4535 100644
@@ -250,6 +250,7 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
  	struct sdhci_pltfm_host *pltfm_host;
  	const struct of_device_id *match;
  	struct sdhci_brcmstb_priv *priv;
+	u32 base_clock_hz = 0;
  	struct sdhci_host *host;
  	struct resource *iomem;
  	struct clk *clk;

@@ -330,6 +331,30 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)
  	if (match_priv->flags & BRCMSTB_MATCH_FLAGS_BROKEN_TIMEOUT)
  		host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
+	/* Change the base clock frequency if the DT property exists */
+	if (device_property_read_u32(&pdev->dev, "max-frequency",