clk: at91: Improve the clock implementation

PATCH Superseded Wenyou Yang 4 participants 12 messages Sep 18, 2016 – Sep 20, 2016

Subsystems (7)

USB GPIO Common clk MMC, SD and SDIO +3 more subsystems

lore

Share & tools

lore

Wenyou Yang

author

2016-09-18 06:53 UTC #0 lore raw

v2 cover clk: at91: Improve the clock implementation

Add the clock ops for such as spi0_clk, which is the real clock provider, instead of periph32ck, which only recursively bind its children as clk devices. Also update the clocks called in the drivers.

Changes in v2:
- Add Acked-by tag.
- For the periph32ck, periph64ck, gck, systemck nodes, they aren't the clock providers, are to house various actual clock providers, use UCLASS_MISC, instead of UCLASS_CLK.
- For *_of_xlate(), add argument check.
- Fix the implementation of the *_get_rate().
- Use documentation-wise variables for *_clk_probe().
- Remove the duplicated code, use the common functions.
- Add Acked-by tag for gpio/atmel_pio4.

Wenyou Yang (7):
clk: clk-uclass: Assign clk->dev before call .of_xlate
clk: at91: Improve the clock implementation
gpio: atmel_pio4: Remove unneccessary clock calling
i2c: at91_i2c: Remove unneccessary clock calling
i2c: at91_i2c: Change error return -ENODEV to -EINVAL
usb: ehci-atmel: Remove unneccessary clock calling
mmc: atmel_sdhci: Remove unneccessary clock calling

 drivers/clk/at91/Kconfig          |  1 +
 drivers/clk/at91/clk-generated.c  | 92 +++++++++++++++++++++++++--------------
 drivers/clk/at91/clk-peripheral.c | 83 ++++++++++++++++++++++++++---------
 drivers/clk/at91/clk-system.c     | 62 +++++++++++++++++---------
 drivers/clk/at91/pmc.c            | 62 +++++++++++++++++++++++---
 drivers/clk/at91/pmc.h            |  5 ++-
 drivers/clk/clk-uclass.c          |  3 ++
 drivers/gpio/atmel_pio4.c         | 12 -----
 drivers/i2c/at91_i2c.c            | 18 +-------
 drivers/mmc/atmel_sdhci.c         | 27 +-----------
 drivers/usb/host/ehci-atmel.c     | 15 -------
 11 files changed, 228 insertions(+), 152 deletions(-)

Signature · 1 line

2.7.4

Wenyou Yang

author

2016-09-18 06:53 UTC #1 lore raw

PATCH v2 1/7 clk: clk-uclass: Assign clk->dev before call .of_xlate

In order to make clk->dev available in ops->of_xlate() to get the clock ID from the 'reg' property of the clock node, assign the clk->dev before calling ops->of_xlate().

Signed-off-by Wenyou Yang <[email protected]>

Acked-by Stephen Warren <[email protected]>

---

Changes in v2:
- Add Acked-by tag.

 drivers/clk/clk-uclass.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/clk/clk-uclass.c +3 -0

index 4d78e3f..4a3248b 100644
@@ -80,6 +80,9 @@ int clk_get_by_index(struct udevice *dev, int index, struct clk *clk)
 		      __func__, ret);
 		return ret;
 	}
+
+	clk->dev = dev_clk;
+
 	ops = clk_dev_ops(dev_clk);
 
 	if (ops->of_xlate)

Signature · 1 line

2.7.4

Wenyou Yang

author

2016-09-18 06:53 UTC #2 lore raw

PATCH v2 2/7 clk: at91: Improve the clock implementation

In reply to Wenyou Yang

For the peripheral clock, provide the clock ops for the clock provider, such as spi0_clk. The .of_xlate is to get the clk->id, the .enable is to enable the spi0 peripheral clock, the .get_rate is to get the clock frequency.

The driver for periph32ck node is responsible for recursively binding its children as clk devices, not provide the clock ops.

So do the generated clock and system clock.

Signed-off-by Wenyou Yang <[email protected]>

---

Changes in v2:
- For the periph32ck, periph64ck, gck, systemck nodes, they aren't the clock providers, are to house various actual clock providers, use UCLASS_MISC, instead of UCLASS_CLK.
- For *_of_xlate(), add argument check.
- Fix the implementation of the *_get_rate().
- Use documentation-wise variables for *_clk_probe().
- Remove the duplicated code, use the common functions.

 drivers/clk/at91/Kconfig          |  1 +
 drivers/clk/at91/clk-generated.c  | 92 +++++++++++++++++++++++++--------------
 drivers/clk/at91/clk-peripheral.c | 83 ++++++++++++++++++++++++++---------
 drivers/clk/at91/clk-system.c     | 62 +++++++++++++++++---------
 drivers/clk/at91/pmc.c            | 62 +++++++++++++++++++++++---
 drivers/clk/at91/pmc.h            |  5 ++-
 6 files changed, 222 insertions(+), 83 deletions(-)

drivers/clk/at91/Kconfig +1 -0

index 10050d8..904ed48 100644
@@ -1,6 +1,7 @@
 config CLK_AT91
 	bool "AT91 clock drivers"
 	depends on CLK
+	select MISC
 	help
 	  This option is used to enable the AT91 clock driver.
 	  The driver supports the AT91 clock generator, including

drivers/clk/at91/clk-generated.c +59 ~~-33~~

146 lines not on this page · raw message

drivers/clk/at91/clk-peripheral.c +62 ~~-21~~

100 lines not on this page · raw message

drivers/clk/at91/clk-system.c +41 ~~-21~~

84 lines not on this page · raw message

drivers/clk/at91/pmc.c +55 -7

89 lines not on this page · raw message

drivers/clk/at91/pmc.h +4 -1

index 5444c84..f222fce 100644
@@ -13,6 +13,9 @@ struct pmc_platdata {
 };
 
 int at91_pmc_core_probe(struct udevice *dev);
-int at91_pmc_clk_node_bind(struct udevice *dev);
+int at91_clk_sub_device_bind(struct udevice *dev, const char *drv_name);
+
+int at91_clk_of_xlate(struct clk *clk, struct fdtdec_phandle_args *args);
+int at91_clk_probe(struct udevice *dev);
 
 #endif

Signature · 1 line

2.7.4

Stephen Warren

2016-09-20 21:27 UTC #3 lore raw

PATCH v2 2/7 clk: at91: Improve the clock implementation

Quoted 11 lines · Wenyou Yang

For the peripheral clock, provide the clock ops for the clock provider, such as spi0_clk. The .of_xlate is to get the clk->id, the .enable is to enable the spi0 peripheral clock, the .get_rate is to get the clock frequency.
The driver for periph32ck node is responsible for recursively binding its children as clk devices, not provide the clock ops.
So do the generated clock and system clock.

drivers/clk/at91/clk-generated.c DECLARE_GLOBAL_DATA_PTR; · line 53

Quoted 7 lines

drivers/clk/at91/clk-generated.c +5 -1

 
-static ulong generated_clk_get_rate(struct clk *clk)
+static int generic_clk_of_xlate(struct clk *clk,
+			       struct fdtdec_phandle_args *args)
+{
+	return at91_clk_of_xlate(clk, args);
+}

You don't need this wrapper function; simply do the following:

static struct clk_ops generic_clk_ops = {
	.of_xlate = at91_clk_of_xlate,

The same comment applies to the other places at91_clk_of_xlate is used.

drivers/clk/at91/clk-generated.c static int generated_clk_ofdata_to_platdata(struct udevice *dev) · line 178

Quoted 5 lines

drivers/clk/at91/clk-generated.c +2 -1

+static int generic_clk_probe(struct udevice *dev)
 {
-	return at91_pmc_core_probe(dev);
+	return at91_clk_probe(dev);
 }

Same comment here.

drivers/clk/at91/pmc.c int at91_pmc_clk_node_bind(struct udevice *dev) · line 89

Quoted 8 lines

drivers/clk/at91/pmc.c +6 -0

 
+int at91_clk_of_xlate(struct clk *clk, struct fdtdec_phandle_args *args)
+{
+	int periph;
+
+	if (args->args_count) {
+		debug("Invaild args_count: %d\n", args->args_count);

s/Invaild/Invalid/

Aside from those issues,

Acked-by Stephen Warren <[email protected]>

Wenyou Yang

author

2016-09-18 06:53 UTC #4 lore raw

PATCH v2 3/7 gpio: atmel_pio4: Remove unneccessary clock calling

In reply to Wenyou Yang

Due to the peripheral clock driver improvement, remove the unneccessary clock calling.

Signed-off-by Wenyou Yang <[email protected]>

Acked-by Stephen Warren <[email protected]>

---

Changes in v2:
- Add Acked-by tag for gpio/atmel_pio4.

 drivers/gpio/atmel_pio4.c | 12 ------------
 1 file changed, 12 deletions(-)

drivers/gpio/atmel_pio4.c +0 ~~-12~~

index 7adea88..cb90b02 100644
@@ -284,27 +284,15 @@ static int atmel_pio4_probe(struct udevice *dev)
 	struct atmel_pio4_platdata *plat = dev_get_platdata(dev);
 	struct gpio_dev_priv *uc_priv = dev_get_uclass_priv(dev);
 	struct atmel_pioctrl_data *pioctrl_data;
-	struct udevice *dev_clk;
 	struct clk clk;
 	fdt_addr_t addr_base;
 	u32 nbanks;
-	int periph;
 	int ret;
 
 	ret = clk_get_by_index(dev, 0, &clk);
 	if (ret)
 		return ret;
 
-	periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
-	if (periph < 0)
-		return -EINVAL;
-
-	dev_clk = dev_get_parent(clk.dev);
-	ret = clk_request(dev_clk, &clk);
-	if (ret)
-		return ret;
-
-	clk.id = periph;
 	ret = clk_enable(&clk);
 	if (ret)
 		return ret;

Signature · 1 line

2.7.4

Wenyou Yang

author

2016-09-18 06:53 UTC #5 lore raw

PATCH v2 4/7 i2c: at91_i2c: Remove unneccessary clock calling

In reply to Wenyou Yang

Due to the peripheral clock driver improvement, remove the unneccessary clock calling.

Signed-off-by Wenyou Yang <[email protected]>

---

Changes in v2: None

 drivers/i2c/at91_i2c.c | 16 ----------------
 1 file changed, 16 deletions(-)

drivers/i2c/at91_i2c.c +0 ~~-16~~

index 8e9c3ad..73f29e3 100644
@@ -176,34 +176,18 @@ static void at91_calc_i2c_clock(struct udevice *dev, int i2c_clk)
 static int at91_i2c_enable_clk(struct udevice *dev)
 {
 	struct at91_i2c_bus *bus = dev_get_priv(dev);
-	struct udevice *dev_clk;
 	struct clk clk;
 	ulong clk_rate;
-	int periph;
 	int ret;
 
 	ret = clk_get_by_index(dev, 0, &clk);
 	if (ret)
 		return -EINVAL;
 
-	periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
-	if (periph < 0)
-		return -EINVAL;
-
-	dev_clk = dev_get_parent(clk.dev);
-	ret = clk_request(dev_clk, &clk);
-	if (ret)
-		return ret;
-
-	clk.id = periph;
 	ret = clk_enable(&clk);
 	if (ret)
 		return ret;
 
-	ret = clk_get_by_index(dev_clk, 0, &clk);
-	if (ret)
-		return ret;
-
 	clk_rate = clk_get_rate(&clk);
 	if (!clk_rate)
 		return -ENODEV;

Signature · 1 line

2.7.4

Heiko Schocher

2016-09-19 05:58 UTC #6 lore raw

PATCH v2 4/7 i2c: at91_i2c: Remove unneccessary clock calling

Hello Wenyou Yang,

Am 18.09.2016 um 08:53 schrieb Wenyou Yang:

Quoted 10 lines

Due to the peripheral clock driver improvement, remove the unneccessary clock calling.
Signed-off-by Wenyou Yang <[email protected]>
---
Changes in v2: None
  drivers/i2c/at91_i2c.c | 16 ----------------
  1 file changed, 16 deletions(-)

Reviewed-by Heiko Schocher <[email protected]>

bye,
Heiko

Quoted 41 lines

drivers/i2c/at91_i2c.c +0 ~~-16~~

index 8e9c3ad..73f29e3 100644
@@ -176,34 +176,18 @@ static void at91_calc_i2c_clock(struct udevice *dev, int i2c_clk)
  static int at91_i2c_enable_clk(struct udevice *dev)
  {
  	struct at91_i2c_bus *bus = dev_get_priv(dev);
-	struct udevice *dev_clk;
  	struct clk clk;
  	ulong clk_rate;
-	int periph;
  	int ret;
 
  	ret = clk_get_by_index(dev, 0, &clk);
  	if (ret)
  		return -EINVAL;
 
-	periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
-	if (periph < 0)
-		return -EINVAL;
-
-	dev_clk = dev_get_parent(clk.dev);
-	ret = clk_request(dev_clk, &clk);
-	if (ret)
-		return ret;
-
-	clk.id = periph;
  	ret = clk_enable(&clk);
  	if (ret)
  		return ret;
 
-	ret = clk_get_by_index(dev_clk, 0, &clk);
-	if (ret)
-		return ret;
-
  	clk_rate = clk_get_rate(&clk);
  	if (!clk_rate)
  		return -ENODEV;

Signature · 2 lines

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Wenyou Yang

author

2016-09-18 06:53 UTC #7 lore raw

PATCH v2 5/7 i2c: at91_i2c: Change error return -ENODEV to -EINVAL

In reply to Wenyou Yang

Change the error return value -ENODEV from to -EINVAL for more reasonable.

Signed-off-by Wenyou Yang <[email protected]>

---

Changes in v2: None

 drivers/i2c/at91_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/i2c/at91_i2c.c +1 -1

index 73f29e3..472420b 100644
@@ -190,7 +190,7 @@ static int at91_i2c_enable_clk(struct udevice *dev)
 
 	clk_rate = clk_get_rate(&clk);
 	if (!clk_rate)
-		return -ENODEV;
+		return -EINVAL;
 
 	bus->bus_clk_rate = clk_rate;

Signature · 1 line

2.7.4

Heiko Schocher

2016-09-19 05:58 UTC #8 lore raw

PATCH v2 5/7 i2c: at91_i2c: Change error return -ENODEV to -EINVAL

Hello Wenyou Yang,

Am 18.09.2016 um 08:53 schrieb Wenyou Yang:

Quoted 10 lines

Change the error return value -ENODEV from to -EINVAL for more reasonable.
Signed-off-by Wenyou Yang <[email protected]>
---
Changes in v2: None
  drivers/i2c/at91_i2c.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by Heiko Schocher <[email protected]>

bye,
Heiko

Quoted 15 lines

drivers/i2c/at91_i2c.c +1 -1

index 73f29e3..472420b 100644
@@ -190,7 +190,7 @@ static int at91_i2c_enable_clk(struct udevice *dev)
 
  	clk_rate = clk_get_rate(&clk);
  	if (!clk_rate)
-		return -ENODEV;
+		return -EINVAL;
 
  	bus->bus_clk_rate = clk_rate;

Signature · 2 lines

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Wenyou Yang

author

2016-09-18 06:53 UTC #9 lore raw

PATCH v2 6/7 usb: ehci-atmel: Remove unneccessary clock calling

In reply to Wenyou Yang

Due to the peripheral clock driver improvement, remove the unneccessary clock calling.

Signed-off-by Wenyou Yang <[email protected]>

---

Changes in v2: None

 drivers/usb/host/ehci-atmel.c | 15 ---------------
 1 file changed, 15 deletions(-)

drivers/usb/host/ehci-atmel.c +0 ~~-15~~

index d65bbe9..6cb5286 100644
@@ -56,9 +56,7 @@ struct ehci_atmel_priv {
 
 static int ehci_atmel_enable_clk(struct udevice *dev)
 {
-	struct udevice *dev_clk;
 	struct clk clk;
-	int periph;
 	int ret;
 
 	ret = clk_get_by_index(dev, 0, &clk);

@@ -73,19 +71,6 @@ static int ehci_atmel_enable_clk(struct udevice *dev)
 	if (ret)
 		return -EINVAL;
 
-	periph = fdtdec_get_uint(gd->fdt_blob, clk.dev->of_offset, "reg", -1);
-	if (periph < 0)
-		return -EINVAL;
-
-	dev_clk = dev_get_parent(clk.dev);
-	if (!dev_clk)
-		return -ENODEV;
-
-	ret = clk_request(dev_clk, &clk);
-	if (ret)
-		return ret;
-
-	clk.id = periph;
 	ret = clk_enable(&clk);
 	if (ret)
 		return ret;

Signature · 1 line

2.7.4

Wenyou Yang

author

2016-09-18 06:53 UTC #10 lore raw

PATCH v2 7/7 mmc: atmel_sdhci: Remove unneccessary clock calling

In reply to Wenyou Yang

Due to the peripheral and generated clock driver improvement, remove the unneccessary clock calling.

Signed-off-by Wenyou Yang <[email protected]>

---

Changes in v2: None

 drivers/mmc/atmel_sdhci.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

drivers/mmc/atmel_sdhci.c +2 ~~-25~~

index dd6bd33..437a652 100644
@@ -51,29 +51,6 @@ struct atmel_sdhci_plat {
 	struct mmc mmc;
 };
 
-static int atmel_sdhci_get_clk(struct udevice *dev, int index, struct clk *clk)
-{
-	struct udevice *dev_clk;
-	int periph, ret;
-
-	ret = clk_get_by_index(dev, index, clk);
-	if (ret)
-		return ret;
-
-	periph = fdtdec_get_uint(gd->fdt_blob, clk->dev->of_offset, "reg", -1);
-	if (periph < 0)
-		return -EINVAL;
-
-	dev_clk = dev_get_parent(clk->dev);
-	ret = clk_request(dev_clk, clk);
-	if (ret)
-		return ret;
-
-	clk->id = periph;
-
-	return 0;
-}
-
 static int atmel_sdhci_probe(struct udevice *dev)
 {
 	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);

@@ -86,7 +63,7 @@ static int atmel_sdhci_probe(struct udevice *dev)
 	struct clk clk;
 	int ret;
 
-	ret = atmel_sdhci_get_clk(dev, 0, &clk);
+	ret = clk_get_by_index(dev, 0, &clk);
 	if (ret)
 		return ret;

@@ -107,7 +84,7 @@ static int atmel_sdhci_probe(struct udevice *dev)
 	clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
 	gck_rate = clk_base * 1000000 * (clk_mul + 1);
 
-	ret = atmel_sdhci_get_clk(dev, 1, &clk);
+	ret = clk_get_by_index(dev, 1, &clk);
 	if (ret)
 		return ret;

Signature · 1 line

2.7.4

Jaehoon Chung

2016-09-20 07:20 UTC #11 lore raw

PATCH v2 7/7 mmc: atmel_sdhci: Remove unneccessary clock calling

Quoted 2 lines · Wenyou Yang

Due to the peripheral and generated clock driver improvement, remove the unneccessary clock calling.

s/unneccessary/unnecessary

Also subject.. :)

Quoted 2 lines

Signed-off-by Wenyou Yang <[email protected]>

Reviewed-by Jaehoon Chung <[email protected]>

Best Regards,
Jaehoon Chung

Quoted 60 lines

---

Changes in v2: None

 drivers/mmc/atmel_sdhci.c | 27 ++-------------------------
 1 file changed, 2 insertions(+), 25 deletions(-)

drivers/mmc/atmel_sdhci.c +2 ~~-25~~

index dd6bd33..437a652 100644
@@ -51,29 +51,6 @@ struct atmel_sdhci_plat {
 	struct mmc mmc;
 };
 
-static int atmel_sdhci_get_clk(struct udevice *dev, int index, struct clk *clk)
-{
-	struct udevice *dev_clk;
-	int periph, ret;
-
-	ret = clk_get_by_index(dev, index, clk);
-	if (ret)
-		return ret;
-
-	periph = fdtdec_get_uint(gd->fdt_blob, clk->dev->of_offset, "reg", -1);
-	if (periph < 0)
-		return -EINVAL;
-
-	dev_clk = dev_get_parent(clk->dev);
-	ret = clk_request(dev_clk, clk);
-	if (ret)
-		return ret;
-
-	clk->id = periph;
-
-	return 0;
-}
-
 static int atmel_sdhci_probe(struct udevice *dev)
 {
 	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);

@@ -86,7 +63,7 @@ static int atmel_sdhci_probe(struct udevice *dev)
 	struct clk clk;
 	int ret;
 
-	ret = atmel_sdhci_get_clk(dev, 0, &clk);
+	ret = clk_get_by_index(dev, 0, &clk);
 	if (ret)
 		return ret;

@@ -107,7 +84,7 @@ static int atmel_sdhci_probe(struct udevice *dev)
 	clk_mul = (caps_1 & SDHCI_CLOCK_MUL_MASK) >> SDHCI_CLOCK_MUL_SHIFT;
 	gck_rate = clk_base * 1000000 * (clk_mul + 1);
 
-	ret = atmel_sdhci_get_clk(dev, 1, &clk);
+	ret = clk_get_by_index(dev, 1, &clk);
 	if (ret)
 		return ret;