Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1472972 > unrolled thread

[PATCH v2 0/4] Control clock for accessing syscon provided by arasan IP

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/4] Control clock for accessing syscon provided by arasan IP Shawn Lin <shawn.lin@rock-chips.com> - 2016-08-31 03:50 +0200
    [PATCH v2 4/4] clk: rockchip: remove CLK_IGNORE_UNUSED flag for aclk_emmc_grf on rk3399 Shawn Lin <shawn.lin@rock-chips.com> - 2016-08-31 03:50 +0200
      Re: [PATCH v2 4/4] clk: rockchip: remove CLK_IGNORE_UNUSED flag for  aclk_emmc_grf on rk3399 Ziyuan Xu <xzy.xu@rock-chips.com> - 2016-08-31 05:20 +0200
    [PATCH v2 1/4] Documentation: mmc: sdhci-of-arasan: Add clk_syscon as an optional one Shawn Lin <shawn.lin@rock-chips.com> - 2016-08-31 03:50 +0200
    [PATCH v2 3/4] arm64: dts: rockchip: add clk_syscon for sdhci on rk3399 Shawn Lin <shawn.lin@rock-chips.com> - 2016-08-31 03:50 +0200
      Re: [PATCH v2 3/4] arm64: dts: rockchip: add clk_syscon for sdhci on  rk3399 Ziyuan Xu <xzy.xu@rock-chips.com> - 2016-08-31 05:30 +0200
    [PATCH v2 2/4] mmc: sdhci-of-arasan: Control clock for accessing syscon Shawn Lin <shawn.lin@rock-chips.com> - 2016-08-31 03:50 +0200
      Re: [PATCH v2 2/4] mmc: sdhci-of-arasan: Control clock for accessing  syscon Ziyuan Xu <xzy.xu@rock-chips.com> - 2016-08-31 05:30 +0200
      Re: [PATCH v2 2/4] mmc: sdhci-of-arasan: Control clock for accessing  syscon Adrian Hunter <adrian.hunter@intel.com> - 2016-09-01 15:10 +0200

#1472972 — [PATCH v2 0/4] Control clock for accessing syscon provided by arasan IP

This patchset is gonna take over the ownership of aclk_emmc_grf
by sdhci-of-arasan which is used to access corecfg_* stuff provided
by arasan. This clock is optional for sdhci-of-arasan which means we
would not break the driver  without adding this. But we strongly
recommend the upcoming users whose platforms have the similar
cases to add this rather than to add any flags for their specific
clock drivers to make this clock always on.

This patchset is based on linux-next. And patch 4 should *not* be
landed before others, so it would be nice to pack these four patches
via Ulf's tree with Heiko's ack for rockchip part, or vice versa. :)


Changes in v2:
- assign NULL to clk_syscon if it's not deferral error.

Shawn Lin (4):
  Documentation: mmc: sdhci-of-arasan: Add clk_syscon as an optional one
  mmc: sdhci-of-arasan: Control clock for accessing syscon
  arm64: dts: rockchip: add clk_syscon for sdhci on rk3399
  clk: rockchip: remove CLK_IGNORE_UNUSED flag for aclk_emmc_grf on
    rk3399

 .../devicetree/bindings/mmc/arasan,sdhci.txt       |  7 +++--
 arch/arm64/boot/dts/rockchip/rk3399.dtsi           |  5 ++--
 drivers/clk/rockchip/clk-rk3399.c                  |  2 +-
 drivers/mmc/host/sdhci-of-arasan.c                 | 33 ++++++++++++++++++++--
 4 files changed, 39 insertions(+), 8 deletions(-)

-- 
2.3.7

[toc] | [next] | [standalone]

#1472973 — [PATCH v2 4/4] clk: rockchip: remove CLK_IGNORE_UNUSED flag for aclk_emmc_grf on rk3399

aclk_emmc_grf is used for accessing corecfg_* of emmc stuff within
GRF block. We don't need to add CLK_IGNORE_UNUSED for it now as the
emmc driver will enable/disable it explicitly when needed.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v2: None

 drivers/clk/rockchip/clk-rk3399.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
index ede6c47..908f684 100644
--- a/drivers/clk/rockchip/clk-rk3399.c
+++ b/drivers/clk/rockchip/clk-rk3399.c
@@ -934,7 +934,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
 			RK3399_CLKGATE_CON(32), 8, GFLAGS),
 	GATE(ACLK_EMMC_NOC, "aclk_emmc_noc", "aclk_emmc", CLK_IGNORE_UNUSED,
 			RK3399_CLKGATE_CON(32), 9, GFLAGS),
-	GATE(ACLK_EMMC_GRF, "aclk_emmcgrf", "aclk_emmc", CLK_IGNORE_UNUSED,
+	GATE(ACLK_EMMC_GRF, "aclk_emmcgrf", "aclk_emmc", 0,
 			RK3399_CLKGATE_CON(32), 10, GFLAGS),
 
 	/* perilp0 */
-- 
2.3.7

[toc] | [prev] | [next] | [standalone]

#1472999 — Re: [PATCH v2 4/4] clk: rockchip: remove CLK_IGNORE_UNUSED flag for aclk_emmc_grf on rk3399

On 2016年08月31日 09:37, Shawn Lin wrote:
> aclk_emmc_grf is used for accessing corecfg_* of emmc stuff within
> GRF block. We don't need to add CLK_IGNORE_UNUSED for it now as the
> emmc driver will enable/disable it explicitly when needed.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

This patchset test result:

localhost driver # cat /sys/kernel/debug/clk/clk_summary | grep emmc
           gpll_aclk_emmc_src              1            1 
594000000          0 0
              aclk_emmc                    3            5 
198000000          0 0
                 aclk_emmcgrf              1            2 
198000000          0 0
                 aclk_emmc_noc             1            1 
198000000          0 0
                 aclk_emmccore             0            0 
198000000          0 0
           clk_emmc                        1            2 
148500000          0 0
              emmc_cardclock               0            0 
148500000          0 0
           cpll_aclk_emmc_src              0            0 
800000000          0 0

After unbind driver.
localhost driver # cat /sys/kernel/debug/clk/clk_summary | grep emmc
           gpll_aclk_emmc_src              1            1 
594000000          0 0
              aclk_emmc                    1            2 
198000000          0 0
                 aclk_emmcgrf              0            0 
198000000          0 0
                 aclk_emmc_noc             1            1 
198000000          0 0
                 aclk_emmccore             0            0 
198000000          0 0
           clk_emmc                        0            0 
148500000          0 0
           cpll_aclk_emmc_src              0            0 
800000000          0 0

And aclk_emmcgrf(Bit 10, when high, enable clock) had gate.
localhost driver # mem r 0xff760380
0x00000415

It works sane, so

Tested-by: Ziyuan Xu <xzy.xu@rock-chips.com>

> ---
>
> Changes in v2: None
>
>   drivers/clk/rockchip/clk-rk3399.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/rockchip/clk-rk3399.c b/drivers/clk/rockchip/clk-rk3399.c
> index ede6c47..908f684 100644
> --- a/drivers/clk/rockchip/clk-rk3399.c
> +++ b/drivers/clk/rockchip/clk-rk3399.c
> @@ -934,7 +934,7 @@ static struct rockchip_clk_branch rk3399_clk_branches[] __initdata = {
>   			RK3399_CLKGATE_CON(32), 8, GFLAGS),
>   	GATE(ACLK_EMMC_NOC, "aclk_emmc_noc", "aclk_emmc", CLK_IGNORE_UNUSED,
>   			RK3399_CLKGATE_CON(32), 9, GFLAGS),
> -	GATE(ACLK_EMMC_GRF, "aclk_emmcgrf", "aclk_emmc", CLK_IGNORE_UNUSED,
> +	GATE(ACLK_EMMC_GRF, "aclk_emmcgrf", "aclk_emmc", 0,
>   			RK3399_CLKGATE_CON(32), 10, GFLAGS),
>   
>   	/* perilp0 */

[toc] | [prev] | [next] | [standalone]

#1472974 — [PATCH v2 1/4] Documentation: mmc: sdhci-of-arasan: Add clk_syscon as an optional one

We introduced soc-ctl-syscon to do several things, for instance, update
baseclk or update clkmul, etc. In odrder to access this physical block,
we need to explicitly enable its clock. Currently we don't control this
clock as we always add a CLK_IGNORE_UNUSED flag for it to indicate that
we will not gate it even if not referenced. This is not a correct way since
it is a clock parenting from clk_ahb which is used by sdhci-of-arasan now.
Without enabling clk_ahb, the flag don't guarantee we could access
soc-ctl-syscon. Moreover, we can't find a reason not to gate clk_syscon
once we remove/power-down emmc controller. So let's add clk_syscon and
enable/disable it explicitly when needed.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

Changes in v2: None

 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index 3404afa..b04eb02 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -33,6 +33,9 @@ Optional Properties:
   - clock-output-names: If specified, this will be the name of the card clock
     which will be exposed by this device.  Required if #clock-cells is
     specified.
+  - clock-names: From clock bindings: Although we treat clock-names as required
+    property, there is still one, "clk_syscon", should be optional as it depends
+    on whether we need to control soc-ctl-syscon or not.
   - #clock-cells: If specified this should be the value <0>.  With this property
     in place we will export a clock representing the Card Clock.  This clock
     is expected to be consumed by our PHY.  You must also specify
@@ -62,8 +65,8 @@ Example:
 		compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
 		reg = <0x0 0xfe330000 0x0 0x10000>;
 		interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
-		clock-names = "clk_xin", "clk_ahb";
+		clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>, <&cru ACLK_EMMC_GRF>;
+		clock-names = "clk_xin", "clk_ahb", "clk_syscon";
 		arasan,soc-ctl-syscon = <&grf>;
 		assigned-clocks = <&cru SCLK_EMMC>;
 		assigned-clock-rates = <200000000>;
-- 
2.3.7

[toc] | [prev] | [next] | [standalone]

#1472976 — [PATCH v2 3/4] arm64: dts: rockchip: add clk_syscon for sdhci on rk3399

We are intent on letting the sdhci variant driver handle this
optional clock on rk3399 platform now.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

Changes in v2: None

 arch/arm64/boot/dts/rockchip/rk3399.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index bc86e8c..d26c6ad 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -233,8 +233,9 @@
 		arasan,soc-ctl-syscon = <&grf>;
 		assigned-clocks = <&cru SCLK_EMMC>;
 		assigned-clock-rates = <200000000>;
-		clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
-		clock-names = "clk_xin", "clk_ahb";
+		clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>,
+			 <&cru ACLK_EMMC_GRF>;
+		clock-names = "clk_xin", "clk_ahb", "clk_syscon";
 		clock-output-names = "emmc_cardclock";
 		#clock-cells = <0>;
 		phys = <&emmc_phy>;
-- 
2.3.7

[toc] | [prev] | [next] | [standalone]

#1473000 — Re: [PATCH v2 3/4] arm64: dts: rockchip: add clk_syscon for sdhci on rk3399

Hi Shawn,


On 2016年08月31日 09:37, Shawn Lin wrote:
> We are intent on letting the sdhci variant driver handle this
> optional clock on rk3399 platform now.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---

Thanks for your patch, we can gate aclk_emmcgrf now as soon as sdhci 
driver suspend.

Reviewed-by: Ziyuan Xu <xzy.xu@rock-chips.com>
Tested-by: Ziyuan Xu <xzy.xu@rock-chips.com>

>
> Changes in v2: None
>
>   arch/arm64/boot/dts/rockchip/rk3399.dtsi | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> index bc86e8c..d26c6ad 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
> @@ -233,8 +233,9 @@
>   		arasan,soc-ctl-syscon = <&grf>;
>   		assigned-clocks = <&cru SCLK_EMMC>;
>   		assigned-clock-rates = <200000000>;
> -		clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
> -		clock-names = "clk_xin", "clk_ahb";
> +		clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>,
> +			 <&cru ACLK_EMMC_GRF>;
> +		clock-names = "clk_xin", "clk_ahb", "clk_syscon";
>   		clock-output-names = "emmc_cardclock";
>   		#clock-cells = <0>;
>   		phys = <&emmc_phy>;

[toc] | [prev] | [next] | [standalone]

#1472977 — [PATCH v2 2/4] mmc: sdhci-of-arasan: Control clock for accessing syscon

In the eariler commit 65820199272d ("Documentation: mmc:
sdhci-of-arasan: Add soc-ctl-syscon for corecfg regs"), we
introduced syscon to control corecfg_* stuff provided by
arasan. But given that we may need to ungate the clock for
accessing corecfg_*, it not so perfect as it depends on
whether specific clock driver disables it if not referenced.
Meanwhile, if we don't need arasan contoller to work anymore,
there is no reason to still enable it. So let's control this
clock when needed.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

Changes in v2:
- assign NULL to clk_syscon if it's not deferral error.

 drivers/mmc/host/sdhci-of-arasan.c | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 0b3a9cf..3169f81 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -78,6 +78,7 @@ struct sdhci_arasan_soc_ctl_map {
  * struct sdhci_arasan_data
  * @host:		Pointer to the main SDHCI host structure.
  * @clk_ahb:		Pointer to the AHB clock
+ * @clk_syscon:		Pointer to the optional clock for accessing syscon
  * @phy:		Pointer to the generic phy
  * @is_phy_on:		True if the PHY is on; false if not.
  * @sdcardclk_hw:	Struct for the clock we might provide to a PHY.
@@ -88,6 +89,7 @@ struct sdhci_arasan_soc_ctl_map {
 struct sdhci_arasan_data {
 	struct sdhci_host *host;
 	struct clk	*clk_ahb;
+	struct clk	*clk_syscon;
 	struct phy	*phy;
 	bool		is_phy_on;
 
@@ -290,6 +292,7 @@ static int sdhci_arasan_suspend(struct device *dev)
 
 	clk_disable(pltfm_host->clk);
 	clk_disable(sdhci_arasan->clk_ahb);
+	clk_disable(sdhci_arasan->clk_syscon);
 
 	return 0;
 }
@@ -309,6 +312,12 @@ static int sdhci_arasan_resume(struct device *dev)
 	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
 	int ret;
 
+	ret = clk_enable(sdhci_arasan->clk_syscon);
+	if (ret) {
+		dev_err(dev, "Cannot enable syscon clock.\n");
+		return ret;
+	}
+
 	ret = clk_enable(sdhci_arasan->clk_ahb);
 	if (ret) {
 		dev_err(dev, "Cannot enable AHB clock.\n");
@@ -528,26 +537,41 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
 					ret);
 			goto err_pltfm_free;
 		}
+
+		sdhci_arasan->clk_syscon = devm_clk_get(&pdev->dev,
+							"clk_syscon");
+		if (IS_ERR(sdhci_arasan->clk_syscon)) {
+			ret = PTR_ERR(sdhci_arasan->clk_syscon);
+			if (ret == -EPROBE_DEFER)
+				goto err_pltfm_free;
+			else
+				sdhci_arasan->clk_syscon = NULL;
+		}
+
+		if (clk_prepare_enable(sdhci_arasan->clk_syscon)) {
+			dev_err(&pdev->dev, "Unable to enable syscon clock.\n");
+			goto err_pltfm_free;
+		}
 	}
 
 	sdhci_arasan->clk_ahb = devm_clk_get(&pdev->dev, "clk_ahb");
 	if (IS_ERR(sdhci_arasan->clk_ahb)) {
 		dev_err(&pdev->dev, "clk_ahb clock not found.\n");
 		ret = PTR_ERR(sdhci_arasan->clk_ahb);
-		goto err_pltfm_free;
+		goto clk_dis_syscon;
 	}
 
 	clk_xin = devm_clk_get(&pdev->dev, "clk_xin");
 	if (IS_ERR(clk_xin)) {
 		dev_err(&pdev->dev, "clk_xin clock not found.\n");
 		ret = PTR_ERR(clk_xin);
-		goto err_pltfm_free;
+		goto clk_dis_syscon;
 	}
 
 	ret = clk_prepare_enable(sdhci_arasan->clk_ahb);
 	if (ret) {
 		dev_err(&pdev->dev, "Unable to enable AHB clock.\n");
-		goto err_pltfm_free;
+		goto clk_dis_syscon;
 	}
 
 	ret = clk_prepare_enable(clk_xin);
@@ -607,6 +631,8 @@ clk_disable_all:
 	clk_disable_unprepare(clk_xin);
 clk_dis_ahb:
 	clk_disable_unprepare(sdhci_arasan->clk_ahb);
+clk_dis_syscon:
+	clk_disable_unprepare(sdhci_arasan->clk_syscon);
 err_pltfm_free:
 	sdhci_pltfm_free(pdev);
 	return ret;
@@ -631,6 +657,7 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
 	ret = sdhci_pltfm_unregister(pdev);
 
 	clk_disable_unprepare(clk_ahb);
+	clk_disable_unprepare(sdhci_arasan->clk_syscon);
 
 	return ret;
 }
-- 
2.3.7

[toc] | [prev] | [next] | [standalone]

#1473004 — Re: [PATCH v2 2/4] mmc: sdhci-of-arasan: Control clock for accessing syscon

Hi Shawn,


On 2016年08月31日 09:37, Shawn Lin wrote:
> In the eariler commit 65820199272d ("Documentation: mmc:
> sdhci-of-arasan: Add soc-ctl-syscon for corecfg regs"), we
> introduced syscon to control corecfg_* stuff provided by
> arasan. But given that we may need to ungate the clock for
> accessing corecfg_*, it not so perfect as it depends on
> whether specific clock driver disables it if not referenced.
> Meanwhile, if we don't need arasan contoller to work anymore,
> there is no reason to still enable it. So let's control this
> clock when needed.
>
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> ---

Without  this series of patches, we can't gate aclk_emmcgrf even though 
driver enter suspend/remove. It meaningful to save power consumption.
It looks nice to me, and also I have tested on rk3399 board, so

Tested-by: Ziyuan Xu <xzy.xu@rock-chips.com>

>
> Changes in v2:
> - assign NULL to clk_syscon if it's not deferral error.
>
>   drivers/mmc/host/sdhci-of-arasan.c | 33 ++++++++++++++++++++++++++++++---
>   1 file changed, 30 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 0b3a9cf..3169f81 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -78,6 +78,7 @@ struct sdhci_arasan_soc_ctl_map {
>    * struct sdhci_arasan_data
>    * @host:		Pointer to the main SDHCI host structure.
>    * @clk_ahb:		Pointer to the AHB clock
> + * @clk_syscon:		Pointer to the optional clock for accessing syscon
>    * @phy:		Pointer to the generic phy
>    * @is_phy_on:		True if the PHY is on; false if not.
>    * @sdcardclk_hw:	Struct for the clock we might provide to a PHY.
> @@ -88,6 +89,7 @@ struct sdhci_arasan_soc_ctl_map {
>   struct sdhci_arasan_data {
>   	struct sdhci_host *host;
>   	struct clk	*clk_ahb;
> +	struct clk	*clk_syscon;
>   	struct phy	*phy;
>   	bool		is_phy_on;
>   
> @@ -290,6 +292,7 @@ static int sdhci_arasan_suspend(struct device *dev)
>   
>   	clk_disable(pltfm_host->clk);
>   	clk_disable(sdhci_arasan->clk_ahb);
> +	clk_disable(sdhci_arasan->clk_syscon);
>   
>   	return 0;
>   }
> @@ -309,6 +312,12 @@ static int sdhci_arasan_resume(struct device *dev)
>   	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
>   	int ret;
>   
> +	ret = clk_enable(sdhci_arasan->clk_syscon);
> +	if (ret) {
> +		dev_err(dev, "Cannot enable syscon clock.\n");
> +		return ret;
> +	}
> +
>   	ret = clk_enable(sdhci_arasan->clk_ahb);
>   	if (ret) {
>   		dev_err(dev, "Cannot enable AHB clock.\n");
> @@ -528,26 +537,41 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>   					ret);
>   			goto err_pltfm_free;
>   		}
> +
> +		sdhci_arasan->clk_syscon = devm_clk_get(&pdev->dev,
> +							"clk_syscon");
> +		if (IS_ERR(sdhci_arasan->clk_syscon)) {
> +			ret = PTR_ERR(sdhci_arasan->clk_syscon);
> +			if (ret == -EPROBE_DEFER)
> +				goto err_pltfm_free;
> +			else
> +				sdhci_arasan->clk_syscon = NULL;
> +		}
> +
> +		if (clk_prepare_enable(sdhci_arasan->clk_syscon)) {
> +			dev_err(&pdev->dev, "Unable to enable syscon clock.\n");
> +			goto err_pltfm_free;
> +		}
>   	}
>   
>   	sdhci_arasan->clk_ahb = devm_clk_get(&pdev->dev, "clk_ahb");
>   	if (IS_ERR(sdhci_arasan->clk_ahb)) {
>   		dev_err(&pdev->dev, "clk_ahb clock not found.\n");
>   		ret = PTR_ERR(sdhci_arasan->clk_ahb);
> -		goto err_pltfm_free;
> +		goto clk_dis_syscon;
>   	}
>   
>   	clk_xin = devm_clk_get(&pdev->dev, "clk_xin");
>   	if (IS_ERR(clk_xin)) {
>   		dev_err(&pdev->dev, "clk_xin clock not found.\n");
>   		ret = PTR_ERR(clk_xin);
> -		goto err_pltfm_free;
> +		goto clk_dis_syscon;
>   	}
>   
>   	ret = clk_prepare_enable(sdhci_arasan->clk_ahb);
>   	if (ret) {
>   		dev_err(&pdev->dev, "Unable to enable AHB clock.\n");
> -		goto err_pltfm_free;
> +		goto clk_dis_syscon;
>   	}
>   
>   	ret = clk_prepare_enable(clk_xin);
> @@ -607,6 +631,8 @@ clk_disable_all:
>   	clk_disable_unprepare(clk_xin);
>   clk_dis_ahb:
>   	clk_disable_unprepare(sdhci_arasan->clk_ahb);
> +clk_dis_syscon:
> +	clk_disable_unprepare(sdhci_arasan->clk_syscon);
>   err_pltfm_free:
>   	sdhci_pltfm_free(pdev);
>   	return ret;
> @@ -631,6 +657,7 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
>   	ret = sdhci_pltfm_unregister(pdev);
>   
>   	clk_disable_unprepare(clk_ahb);
> +	clk_disable_unprepare(sdhci_arasan->clk_syscon);
>   
>   	return ret;
>   }

[toc] | [prev] | [next] | [standalone]

#1474391 — Re: [PATCH v2 2/4] mmc: sdhci-of-arasan: Control clock for accessing syscon

On 31/08/16 04:37, Shawn Lin wrote:
> In the eariler commit 65820199272d ("Documentation: mmc:
> sdhci-of-arasan: Add soc-ctl-syscon for corecfg regs"), we
> introduced syscon to control corecfg_* stuff provided by
> arasan. But given that we may need to ungate the clock for
> accessing corecfg_*, it not so perfect as it depends on
> whether specific clock driver disables it if not referenced.
> Meanwhile, if we don't need arasan contoller to work anymore,
> there is no reason to still enable it. So let's control this
> clock when needed.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> 
> ---
> 
> Changes in v2:
> - assign NULL to clk_syscon if it's not deferral error.
> 
>  drivers/mmc/host/sdhci-of-arasan.c | 33 ++++++++++++++++++++++++++++++---
>  1 file changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index 0b3a9cf..3169f81 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -78,6 +78,7 @@ struct sdhci_arasan_soc_ctl_map {
>   * struct sdhci_arasan_data
>   * @host:		Pointer to the main SDHCI host structure.
>   * @clk_ahb:		Pointer to the AHB clock
> + * @clk_syscon:		Pointer to the optional clock for accessing syscon
>   * @phy:		Pointer to the generic phy
>   * @is_phy_on:		True if the PHY is on; false if not.
>   * @sdcardclk_hw:	Struct for the clock we might provide to a PHY.
> @@ -88,6 +89,7 @@ struct sdhci_arasan_soc_ctl_map {
>  struct sdhci_arasan_data {
>  	struct sdhci_host *host;
>  	struct clk	*clk_ahb;
> +	struct clk	*clk_syscon;
>  	struct phy	*phy;
>  	bool		is_phy_on;
>  
> @@ -290,6 +292,7 @@ static int sdhci_arasan_suspend(struct device *dev)
>  
>  	clk_disable(pltfm_host->clk);
>  	clk_disable(sdhci_arasan->clk_ahb);
> +	clk_disable(sdhci_arasan->clk_syscon);
>  
>  	return 0;
>  }
> @@ -309,6 +312,12 @@ static int sdhci_arasan_resume(struct device *dev)
>  	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
>  	int ret;
>  
> +	ret = clk_enable(sdhci_arasan->clk_syscon);
> +	if (ret) {
> +		dev_err(dev, "Cannot enable syscon clock.\n");
> +		return ret;
> +	}
> +
>  	ret = clk_enable(sdhci_arasan->clk_ahb);
>  	if (ret) {
>  		dev_err(dev, "Cannot enable AHB clock.\n");
> @@ -528,26 +537,41 @@ static int sdhci_arasan_probe(struct platform_device *pdev)
>  					ret);
>  			goto err_pltfm_free;
>  		}
> +
> +		sdhci_arasan->clk_syscon = devm_clk_get(&pdev->dev,
> +							"clk_syscon");
> +		if (IS_ERR(sdhci_arasan->clk_syscon)) {
> +			ret = PTR_ERR(sdhci_arasan->clk_syscon);
> +			if (ret == -EPROBE_DEFER)
> +				goto err_pltfm_free;
> +			else
> +				sdhci_arasan->clk_syscon = NULL;
> +		}
> +
> +		if (clk_prepare_enable(sdhci_arasan->clk_syscon)) {
> +			dev_err(&pdev->dev, "Unable to enable syscon clock.\n");

Do you need to set 'ret' here?

> +			goto err_pltfm_free;
> +		}
>  	}
>  
>  	sdhci_arasan->clk_ahb = devm_clk_get(&pdev->dev, "clk_ahb");
>  	if (IS_ERR(sdhci_arasan->clk_ahb)) {
>  		dev_err(&pdev->dev, "clk_ahb clock not found.\n");
>  		ret = PTR_ERR(sdhci_arasan->clk_ahb);
> -		goto err_pltfm_free;
> +		goto clk_dis_syscon;
>  	}
>  
>  	clk_xin = devm_clk_get(&pdev->dev, "clk_xin");
>  	if (IS_ERR(clk_xin)) {
>  		dev_err(&pdev->dev, "clk_xin clock not found.\n");
>  		ret = PTR_ERR(clk_xin);
> -		goto err_pltfm_free;
> +		goto clk_dis_syscon;
>  	}
>  
>  	ret = clk_prepare_enable(sdhci_arasan->clk_ahb);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Unable to enable AHB clock.\n");
> -		goto err_pltfm_free;
> +		goto clk_dis_syscon;
>  	}
>  
>  	ret = clk_prepare_enable(clk_xin);
> @@ -607,6 +631,8 @@ clk_disable_all:
>  	clk_disable_unprepare(clk_xin);
>  clk_dis_ahb:
>  	clk_disable_unprepare(sdhci_arasan->clk_ahb);
> +clk_dis_syscon:
> +	clk_disable_unprepare(sdhci_arasan->clk_syscon);
>  err_pltfm_free:
>  	sdhci_pltfm_free(pdev);
>  	return ret;
> @@ -631,6 +657,7 @@ static int sdhci_arasan_remove(struct platform_device *pdev)
>  	ret = sdhci_pltfm_unregister(pdev);
>  
>  	clk_disable_unprepare(clk_ahb);
> +	clk_disable_unprepare(sdhci_arasan->clk_syscon);
>  
>  	return ret;
>  }
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web