linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/3] add slow clock support for SAM9X60
@ 2019-02-14 12:14 Claudiu.Beznea
  2019-02-14 12:14 ` [PATCH v2 1/3] clk: at91: sckc: add support to specify registers bit offsets Claudiu.Beznea
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Claudiu.Beznea @ 2019-02-14 12:14 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, mark.rutland, Nicolas.Ferre,
	alexandre.belloni, Ludovic.Desroches
  Cc: linux-clk, devicetree, linux-arm-kernel, linux-kernel, Claudiu.Beznea

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Hi,

This series add slow clock support for SAM9X60. Apart from previous IPs, this
one uses different offsets in control register for different functionalities.
The series adapt current driver to work for all IPs using per IP
configurations initialized at probe.

Thank you,
Claudiu Beznea

Changes in v2:
- split patch 1/1 from v1 in 2 patches: one adding register bit offsets
  support (patch 1/3 from this series), one adding support for SAM9X60
  (patch 2/3 from this series)
- fix compatible string from "microchip,at91sam9x60-sckc" to
  "microchip,sam9x60-sckc"

Claudiu Beznea (3):
  clk: at91: sckc: add support to specify registers bit offsets
  clk: at91: sckc: add support for SAM9X60
  dt-bindings: clk: at91: add bindings for SAM9X60's slow clock
    controller

 .../devicetree/bindings/clock/at91-clock.txt       |   3 +-
 drivers/clk/at91/sckc.c                            | 142 ++++++++++++++++-----
 2 files changed, 109 insertions(+), 36 deletions(-)

-- 
2.7.4


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] clk: at91: sckc: add support to specify registers bit offsets
  2019-02-14 12:14 [PATCH v2 0/3] add slow clock support for SAM9X60 Claudiu.Beznea
@ 2019-02-14 12:14 ` Claudiu.Beznea
  2019-02-18 21:08   ` Alexandre Belloni
  2019-02-14 12:14 ` [PATCH v2 2/3] clk: at91: sckc: add support for SAM9X60 Claudiu.Beznea
  2019-02-14 12:14 ` [PATCH v2 3/3] dt-bindings: clk: at91: add bindings for SAM9X60's slow clock controller Claudiu.Beznea
  2 siblings, 1 reply; 10+ messages in thread
From: Claudiu.Beznea @ 2019-02-14 12:14 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, mark.rutland, Nicolas.Ferre,
	alexandre.belloni, Ludovic.Desroches
  Cc: linux-clk, devicetree, linux-arm-kernel, linux-kernel, Claudiu.Beznea

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Different IPs uses different offsets in registers for the same
functionality, thus adapt the driver to support this.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/clk/at91/sckc.c | 112 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 77 insertions(+), 35 deletions(-)

diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
index ab6ecefc49ad..b7163d3a2269 100644
--- a/drivers/clk/at91/sckc.c
+++ b/drivers/clk/at91/sckc.c
@@ -22,15 +22,25 @@
 #define SLOWCK_SW_TIME_USEC	((SLOWCK_SW_CYCLES * USEC_PER_SEC) / \
 				 SLOW_CLOCK_FREQ)
 
-#define	AT91_SCKC_CR			0x00
-#define		AT91_SCKC_RCEN		(1 << 0)
-#define		AT91_SCKC_OSC32EN	(1 << 1)
-#define		AT91_SCKC_OSC32BYP	(1 << 2)
-#define		AT91_SCKC_OSCSEL	(1 << 3)
+#define	AT91_SCKC_CR		0x00
+#define		AT91_SCKC_RCEN(off)	(1 << (off)->cr_rcen)
+#define		AT91_SCKC_OSC32EN(off)	(1 << (off)->cr_osc32en)
+#define		AT91_SCKC_OSC32BYP(off)	(1 << (off)->cr_osc32byp)
+#define		AT91_SCKC_OSCSEL(off)	(1 << (off)->cr_oscsel)
+
+#define AT91_SCKC_OFFSET_INVALID        (32)
+
+struct clk_slow_offsets {
+	u8 cr_rcen;
+	u8 cr_osc32en;
+	u8 cr_osc32byp;
+	u8 cr_oscsel;
+};
 
 struct clk_slow_osc {
 	struct clk_hw hw;
 	void __iomem *sckcr;
+	const struct clk_slow_offsets *offsets;
 	unsigned long startup_usec;
 };
 
@@ -39,6 +49,7 @@ struct clk_slow_osc {
 struct clk_sama5d4_slow_osc {
 	struct clk_hw hw;
 	void __iomem *sckcr;
+	const struct clk_slow_offsets *offsets;
 	unsigned long startup_usec;
 	bool prepared;
 };
@@ -48,6 +59,7 @@ struct clk_sama5d4_slow_osc {
 struct clk_slow_rc_osc {
 	struct clk_hw hw;
 	void __iomem *sckcr;
+	const struct clk_slow_offsets *offsets;
 	unsigned long frequency;
 	unsigned long accuracy;
 	unsigned long startup_usec;
@@ -58,6 +70,7 @@ struct clk_slow_rc_osc {
 struct clk_sam9x5_slow {
 	struct clk_hw hw;
 	void __iomem *sckcr;
+	const struct clk_slow_offsets *offsets;
 	u8 parent;
 };
 
@@ -69,10 +82,11 @@ static int clk_slow_osc_prepare(struct clk_hw *hw)
 	void __iomem *sckcr = osc->sckcr;
 	u32 tmp = readl(sckcr);
 
-	if (tmp & (AT91_SCKC_OSC32BYP | AT91_SCKC_OSC32EN))
+	if (tmp & (AT91_SCKC_OSC32BYP(osc->offsets) |
+		   AT91_SCKC_OSC32EN(osc->offsets)))
 		return 0;
 
-	writel(tmp | AT91_SCKC_OSC32EN, sckcr);
+	writel(tmp | AT91_SCKC_OSC32EN(osc->offsets), sckcr);
 
 	usleep_range(osc->startup_usec, osc->startup_usec + 1);
 
@@ -85,10 +99,10 @@ static void clk_slow_osc_unprepare(struct clk_hw *hw)
 	void __iomem *sckcr = osc->sckcr;
 	u32 tmp = readl(sckcr);
 
-	if (tmp & AT91_SCKC_OSC32BYP)
+	if (tmp & AT91_SCKC_OSC32BYP(osc->offsets))
 		return;
 
-	writel(tmp & ~AT91_SCKC_OSC32EN, sckcr);
+	writel(tmp & ~AT91_SCKC_OSC32EN(osc->offsets), sckcr);
 }
 
 static int clk_slow_osc_is_prepared(struct clk_hw *hw)
@@ -97,10 +111,10 @@ static int clk_slow_osc_is_prepared(struct clk_hw *hw)
 	void __iomem *sckcr = osc->sckcr;
 	u32 tmp = readl(sckcr);
 
-	if (tmp & AT91_SCKC_OSC32BYP)
+	if (tmp & AT91_SCKC_OSC32BYP(osc->offsets))
 		return 1;
 
-	return !!(tmp & AT91_SCKC_OSC32EN);
+	return !!(tmp & AT91_SCKC_OSC32EN(osc->offsets));
 }
 
 static const struct clk_ops slow_osc_ops = {
@@ -114,7 +128,8 @@ at91_clk_register_slow_osc(void __iomem *sckcr,
 			   const char *name,
 			   const char *parent_name,
 			   unsigned long startup,
-			   bool bypass)
+			   bool bypass,
+			   const struct clk_slow_offsets *offsets)
 {
 	struct clk_slow_osc *osc;
 	struct clk_hw *hw;
@@ -137,9 +152,11 @@ at91_clk_register_slow_osc(void __iomem *sckcr,
 	osc->hw.init = &init;
 	osc->sckcr = sckcr;
 	osc->startup_usec = startup;
+	osc->offsets = offsets;
 
 	if (bypass)
-		writel((readl(sckcr) & ~AT91_SCKC_OSC32EN) | AT91_SCKC_OSC32BYP,
+		writel((readl(sckcr) & ~AT91_SCKC_OSC32EN(osc->offsets)) |
+					AT91_SCKC_OSC32BYP(osc->offsets),
 		       sckcr);
 
 	hw = &osc->hw;
@@ -153,7 +170,8 @@ at91_clk_register_slow_osc(void __iomem *sckcr,
 }
 
 static void __init
-of_at91sam9x5_clk_slow_osc_setup(struct device_node *np, void __iomem *sckcr)
+of_at91sam9x5_clk_slow_osc_setup(struct device_node *np, void __iomem *sckcr,
+				 const struct clk_slow_offsets *offsets)
 {
 	struct clk_hw *hw;
 	const char *parent_name;
@@ -167,7 +185,7 @@ of_at91sam9x5_clk_slow_osc_setup(struct device_node *np, void __iomem *sckcr)
 	bypass = of_property_read_bool(np, "atmel,osc-bypass");
 
 	hw = at91_clk_register_slow_osc(sckcr, name, parent_name, startup,
-					 bypass);
+					bypass, offsets);
 	if (IS_ERR(hw))
 		return;
 
@@ -195,7 +213,7 @@ static int clk_slow_rc_osc_prepare(struct clk_hw *hw)
 	struct clk_slow_rc_osc *osc = to_clk_slow_rc_osc(hw);
 	void __iomem *sckcr = osc->sckcr;
 
-	writel(readl(sckcr) | AT91_SCKC_RCEN, sckcr);
+	writel(readl(sckcr) | AT91_SCKC_RCEN(osc->offsets), sckcr);
 
 	usleep_range(osc->startup_usec, osc->startup_usec + 1);
 
@@ -207,14 +225,14 @@ static void clk_slow_rc_osc_unprepare(struct clk_hw *hw)
 	struct clk_slow_rc_osc *osc = to_clk_slow_rc_osc(hw);
 	void __iomem *sckcr = osc->sckcr;
 
-	writel(readl(sckcr) & ~AT91_SCKC_RCEN, sckcr);
+	writel(readl(sckcr) & ~AT91_SCKC_RCEN(osc->offsets), sckcr);
 }
 
 static int clk_slow_rc_osc_is_prepared(struct clk_hw *hw)
 {
 	struct clk_slow_rc_osc *osc = to_clk_slow_rc_osc(hw);
 
-	return !!(readl(osc->sckcr) & AT91_SCKC_RCEN);
+	return !!(readl(osc->sckcr) & AT91_SCKC_RCEN(osc->offsets));
 }
 
 static const struct clk_ops slow_rc_osc_ops = {
@@ -230,7 +248,8 @@ at91_clk_register_slow_rc_osc(void __iomem *sckcr,
 			      const char *name,
 			      unsigned long frequency,
 			      unsigned long accuracy,
-			      unsigned long startup)
+			      unsigned long startup,
+			      const struct clk_slow_offsets *offsets)
 {
 	struct clk_slow_rc_osc *osc;
 	struct clk_hw *hw;
@@ -252,6 +271,7 @@ at91_clk_register_slow_rc_osc(void __iomem *sckcr,
 
 	osc->hw.init = &init;
 	osc->sckcr = sckcr;
+	osc->offsets = offsets;
 	osc->frequency = frequency;
 	osc->accuracy = accuracy;
 	osc->startup_usec = startup;
@@ -267,7 +287,8 @@ at91_clk_register_slow_rc_osc(void __iomem *sckcr,
 }
 
 static void __init
-of_at91sam9x5_clk_slow_rc_osc_setup(struct device_node *np, void __iomem *sckcr)
+of_at91sam9x5_clk_slow_rc_osc_setup(struct device_node *np, void __iomem *sckcr,
+				    const struct clk_slow_offsets *offsets)
 {
 	struct clk_hw *hw;
 	u32 frequency = 0;
@@ -281,7 +302,7 @@ of_at91sam9x5_clk_slow_rc_osc_setup(struct device_node *np, void __iomem *sckcr)
 	of_property_read_u32(np, "atmel,startup-time-usec", &startup);
 
 	hw = at91_clk_register_slow_rc_osc(sckcr, name, frequency, accuracy,
-					    startup);
+					   startup, offsets);
 	if (IS_ERR(hw))
 		return;
 
@@ -299,14 +320,14 @@ static int clk_sam9x5_slow_set_parent(struct clk_hw *hw, u8 index)
 
 	tmp = readl(sckcr);
 
-	if ((!index && !(tmp & AT91_SCKC_OSCSEL)) ||
-	    (index && (tmp & AT91_SCKC_OSCSEL)))
+	if ((!index && !(tmp & AT91_SCKC_OSCSEL(slowck->offsets))) ||
+	    (index && (tmp & AT91_SCKC_OSCSEL(slowck->offsets))))
 		return 0;
 
 	if (index)
-		tmp |= AT91_SCKC_OSCSEL;
+		tmp |= AT91_SCKC_OSCSEL(slowck->offsets);
 	else
-		tmp &= ~AT91_SCKC_OSCSEL;
+		tmp &= ~AT91_SCKC_OSCSEL(slowck->offsets);
 
 	writel(tmp, sckcr);
 
@@ -319,7 +340,7 @@ static u8 clk_sam9x5_slow_get_parent(struct clk_hw *hw)
 {
 	struct clk_sam9x5_slow *slowck = to_clk_sam9x5_slow(hw);
 
-	return !!(readl(slowck->sckcr) & AT91_SCKC_OSCSEL);
+	return !!(readl(slowck->sckcr) & AT91_SCKC_OSCSEL(slowck->offsets));
 }
 
 static const struct clk_ops sam9x5_slow_ops = {
@@ -331,7 +352,8 @@ static struct clk_hw * __init
 at91_clk_register_sam9x5_slow(void __iomem *sckcr,
 			      const char *name,
 			      const char **parent_names,
-			      int num_parents)
+			      int num_parents,
+			      const struct clk_slow_offsets *offsets)
 {
 	struct clk_sam9x5_slow *slowck;
 	struct clk_hw *hw;
@@ -353,7 +375,8 @@ at91_clk_register_sam9x5_slow(void __iomem *sckcr,
 
 	slowck->hw.init = &init;
 	slowck->sckcr = sckcr;
-	slowck->parent = !!(readl(sckcr) & AT91_SCKC_OSCSEL);
+	slowck->offsets = offsets;
+	slowck->parent = !!(readl(sckcr) & AT91_SCKC_OSCSEL(slowck->offsets));
 
 	hw = &slowck->hw;
 	ret = clk_hw_register(NULL, &slowck->hw);
@@ -366,7 +389,8 @@ at91_clk_register_sam9x5_slow(void __iomem *sckcr,
 }
 
 static void __init
-of_at91sam9x5_clk_slow_setup(struct device_node *np, void __iomem *sckcr)
+of_at91sam9x5_clk_slow_setup(struct device_node *np, void __iomem *sckcr,
+			     const struct clk_slow_offsets *offsets)
 {
 	struct clk_hw *hw;
 	const char *parent_names[2];
@@ -382,7 +406,7 @@ of_at91sam9x5_clk_slow_setup(struct device_node *np, void __iomem *sckcr)
 	of_property_read_string(np, "clock-output-names", &name);
 
 	hw = at91_clk_register_sam9x5_slow(sckcr, name, parent_names,
-					    num_parents);
+					   num_parents, offsets);
 	if (IS_ERR(hw))
 		return;
 
@@ -406,10 +430,18 @@ static const struct of_device_id sckc_clk_ids[] __initconst = {
 	{ /*sentinel*/ }
 };
 
+static const struct clk_slow_offsets at91sam9x5_offsets = {
+	.cr_rcen = 0,
+	.cr_osc32en = 1,
+	.cr_osc32byp = 2,
+	.cr_oscsel = 3,
+};
+
 static void __init of_at91sam9x5_sckc_setup(struct device_node *np)
 {
 	struct device_node *childnp;
-	void (*clk_setup)(struct device_node *, void __iomem *);
+	void (*clk_setup)(struct device_node *np, void __iomem *io,
+			  const struct clk_slow_offsets *offsets);
 	const struct of_device_id *clk_id;
 	void __iomem *regbase = of_iomap(np, 0);
 
@@ -421,7 +453,7 @@ static void __init of_at91sam9x5_sckc_setup(struct device_node *np)
 		if (!clk_id)
 			continue;
 		clk_setup = clk_id->data;
-		clk_setup(childnp, regbase);
+		clk_setup(childnp, regbase, &at91sam9x5_offsets);
 	}
 }
 CLK_OF_DECLARE(at91sam9x5_clk_sckc, "atmel,at91sam9x5-sckc",
@@ -438,7 +470,7 @@ static int clk_sama5d4_slow_osc_prepare(struct clk_hw *hw)
 	 * Assume that if it has already been selected (for example by the
 	 * bootloader), enough time has aready passed.
 	 */
-	if ((readl(osc->sckcr) & AT91_SCKC_OSCSEL)) {
+	if ((readl(osc->sckcr) & AT91_SCKC_OSCSEL(osc->offsets))) {
 		osc->prepared = true;
 		return 0;
 	}
@@ -461,6 +493,13 @@ static const struct clk_ops sama5d4_slow_osc_ops = {
 	.is_prepared = clk_sama5d4_slow_osc_is_prepared,
 };
 
+static const struct clk_slow_offsets at91sama5d4_offsets = {
+	.cr_rcen = AT91_SCKC_OFFSET_INVALID,
+	.cr_osc32en = AT91_SCKC_OFFSET_INVALID,
+	.cr_osc32byp = AT91_SCKC_OFFSET_INVALID,
+	.cr_oscsel = 3,
+};
+
 static void __init of_sama5d4_sckc_setup(struct device_node *np)
 {
 	void __iomem *regbase = of_iomap(np, 0);
@@ -498,9 +537,11 @@ static void __init of_sama5d4_sckc_setup(struct device_node *np)
 	osc->hw.init = &init;
 	osc->sckcr = regbase;
 	osc->startup_usec = 1200000;
+	osc->offsets = &at91sama5d4_offsets;
 
 	if (bypass)
-		writel((readl(regbase) | AT91_SCKC_OSC32BYP), regbase);
+		writel((readl(regbase) | AT91_SCKC_OSC32BYP(osc->offsets)),
+		       regbase);
 
 	hw = &osc->hw;
 	ret = clk_hw_register(NULL, &osc->hw);
@@ -509,7 +550,8 @@ static void __init of_sama5d4_sckc_setup(struct device_node *np)
 		return;
 	}
 
-	hw = at91_clk_register_sam9x5_slow(regbase, "slowck", parent_names, 2);
+	hw = at91_clk_register_sam9x5_slow(regbase, "slowck", parent_names, 2,
+					   &at91sama5d4_offsets);
 	if (IS_ERR(hw))
 		return;
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] clk: at91: sckc: add support for SAM9X60
  2019-02-14 12:14 [PATCH v2 0/3] add slow clock support for SAM9X60 Claudiu.Beznea
  2019-02-14 12:14 ` [PATCH v2 1/3] clk: at91: sckc: add support to specify registers bit offsets Claudiu.Beznea
@ 2019-02-14 12:14 ` Claudiu.Beznea
  2019-02-18 21:20   ` Alexandre Belloni
  2019-02-14 12:14 ` [PATCH v2 3/3] dt-bindings: clk: at91: add bindings for SAM9X60's slow clock controller Claudiu.Beznea
  2 siblings, 1 reply; 10+ messages in thread
From: Claudiu.Beznea @ 2019-02-14 12:14 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, mark.rutland, Nicolas.Ferre,
	alexandre.belloni, Ludovic.Desroches
  Cc: linux-clk, devicetree, linux-arm-kernel, linux-kernel, Claudiu.Beznea

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Add support for SAM9X60.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 drivers/clk/at91/sckc.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
index b7163d3a2269..b3075c51d260 100644
--- a/drivers/clk/at91/sckc.c
+++ b/drivers/clk/at91/sckc.c
@@ -459,6 +459,36 @@ static void __init of_at91sam9x5_sckc_setup(struct device_node *np)
 CLK_OF_DECLARE(at91sam9x5_clk_sckc, "atmel,at91sam9x5-sckc",
 	       of_at91sam9x5_sckc_setup);
 
+static const struct clk_slow_offsets at91sam9x60_offsets = {
+	.cr_rcen = AT91_SCKC_OFFSET_INVALID,
+	.cr_osc32en = 1,
+	.cr_osc32byp = 2,
+	.cr_oscsel = 24,
+};
+
+static void __init of_at91sam9x60_sckc_setup(struct device_node *np)
+{
+	struct device_node *childnp;
+	void (*clk_setup)(struct device_node *np, void __iomem *io,
+			  const struct clk_slow_offsets *offsets);
+	const struct of_device_id *clk_id;
+	void __iomem *regbase = of_iomap(np, 0);
+
+	if (!regbase)
+		return;
+
+	for_each_child_of_node(np, childnp) {
+		clk_id = of_match_node(sckc_clk_ids, childnp);
+		if (!clk_id)
+			continue;
+		clk_setup = clk_id->data;
+		clk_setup(childnp, regbase, &at91sam9x60_offsets);
+	}
+}
+
+CLK_OF_DECLARE(at91sam9x60_clk_sckc, "microchip,sam9x60-sckc",
+	       of_at91sam9x60_sckc_setup);
+
 static int clk_sama5d4_slow_osc_prepare(struct clk_hw *hw)
 {
 	struct clk_sama5d4_slow_osc *osc = to_clk_sama5d4_slow_osc(hw);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] dt-bindings: clk: at91: add bindings for SAM9X60's slow clock controller
  2019-02-14 12:14 [PATCH v2 0/3] add slow clock support for SAM9X60 Claudiu.Beznea
  2019-02-14 12:14 ` [PATCH v2 1/3] clk: at91: sckc: add support to specify registers bit offsets Claudiu.Beznea
  2019-02-14 12:14 ` [PATCH v2 2/3] clk: at91: sckc: add support for SAM9X60 Claudiu.Beznea
@ 2019-02-14 12:14 ` Claudiu.Beznea
  2019-02-18 20:04   ` Rob Herring
  2 siblings, 1 reply; 10+ messages in thread
From: Claudiu.Beznea @ 2019-02-14 12:14 UTC (permalink / raw)
  To: mturquette, sboyd, robh+dt, mark.rutland, Nicolas.Ferre,
	alexandre.belloni, Ludovic.Desroches
  Cc: linux-clk, devicetree, linux-arm-kernel, linux-kernel, Claudiu.Beznea

From: Claudiu Beznea <claudiu.beznea@microchip.com>

Add bindings for SAM9X60's slow clock controller.

Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
---
 Documentation/devicetree/bindings/clock/at91-clock.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/clock/at91-clock.txt b/Documentation/devicetree/bindings/clock/at91-clock.txt
index e9f70fcdfe80..ce1d21102428 100644
--- a/Documentation/devicetree/bindings/clock/at91-clock.txt
+++ b/Documentation/devicetree/bindings/clock/at91-clock.txt
@@ -9,7 +9,8 @@ Slow Clock controller:
 Required properties:
 - compatible : shall be one of the following:
 	"atmel,at91sam9x5-sckc" or
-	"atmel,sama5d4-sckc":
+	"atmel,sama5d4-sckc" or
+	"microchip,sam9x60-sckc":
 		at91 SCKC (Slow Clock Controller)
 		This node contains the slow clock definitions.
 
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] dt-bindings: clk: at91: add bindings for SAM9X60's  slow clock controller
  2019-02-14 12:14 ` [PATCH v2 3/3] dt-bindings: clk: at91: add bindings for SAM9X60's slow clock controller Claudiu.Beznea
@ 2019-02-18 20:04   ` Rob Herring
  0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2019-02-18 20:04 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: mturquette, sboyd, robh+dt, mark.rutland, Nicolas.Ferre,
	alexandre.belloni, Ludovic.Desroches, linux-clk, devicetree,
	linux-arm-kernel, linux-kernel, Claudiu.Beznea

On Thu, 14 Feb 2019 12:14:36 +0000, <Claudiu.Beznea@microchip.com> wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Add bindings for SAM9X60's slow clock controller.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  Documentation/devicetree/bindings/clock/at91-clock.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] clk: at91: sckc: add support to specify registers bit offsets
  2019-02-14 12:14 ` [PATCH v2 1/3] clk: at91: sckc: add support to specify registers bit offsets Claudiu.Beznea
@ 2019-02-18 21:08   ` Alexandre Belloni
  2019-02-19  9:17     ` Claudiu.Beznea
  0 siblings, 1 reply; 10+ messages in thread
From: Alexandre Belloni @ 2019-02-18 21:08 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: mturquette, sboyd, robh+dt, mark.rutland, Nicolas.Ferre,
	Ludovic.Desroches, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel

Hi,

On 14/02/2019 12:14:28+0000, Claudiu.Beznea@microchip.com wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Different IPs uses different offsets in registers for the same
> functionality, thus adapt the driver to support this.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  drivers/clk/at91/sckc.c | 112 +++++++++++++++++++++++++++++++++---------------
>  1 file changed, 77 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
> index ab6ecefc49ad..b7163d3a2269 100644
> --- a/drivers/clk/at91/sckc.c
> +++ b/drivers/clk/at91/sckc.c
> @@ -22,15 +22,25 @@
>  #define SLOWCK_SW_TIME_USEC	((SLOWCK_SW_CYCLES * USEC_PER_SEC) / \
>  				 SLOW_CLOCK_FREQ)
>  
> -#define	AT91_SCKC_CR			0x00
> -#define		AT91_SCKC_RCEN		(1 << 0)
> -#define		AT91_SCKC_OSC32EN	(1 << 1)
> -#define		AT91_SCKC_OSC32BYP	(1 << 2)
> -#define		AT91_SCKC_OSCSEL	(1 << 3)
> +#define	AT91_SCKC_CR		0x00
> +#define		AT91_SCKC_RCEN(off)	(1 << (off)->cr_rcen)
> +#define		AT91_SCKC_OSC32EN(off)	(1 << (off)->cr_osc32en)
> +#define		AT91_SCKC_OSC32BYP(off)	(1 << (off)->cr_osc32byp)
> +#define		AT91_SCKC_OSCSEL(off)	(1 << (off)->cr_oscsel)

You don't need those macros if you store BIT(off) instead of off in the
members of clk_slow_offsets.

> +
> +#define AT91_SCKC_OFFSET_INVALID        (32)
> +

This would also remove the need for that define as 0 would mean that the
bit doesn't exist.

> +struct clk_slow_offsets {
> +	u8 cr_rcen;
> +	u8 cr_osc32en;
> +	u8 cr_osc32byp;
> +	u8 cr_oscsel;
> +};
>  
>  struct clk_slow_osc {
>  	struct clk_hw hw;
>  	void __iomem *sckcr;
> +	const struct clk_slow_offsets *offsets;
>  	unsigned long startup_usec;
>  };
>  
> @@ -39,6 +49,7 @@ struct clk_slow_osc {
>  struct clk_sama5d4_slow_osc {
>  	struct clk_hw hw;
>  	void __iomem *sckcr;
> +	const struct clk_slow_offsets *offsets;
>  	unsigned long startup_usec;
>  	bool prepared;
>  };
> @@ -48,6 +59,7 @@ struct clk_sama5d4_slow_osc {
>  struct clk_slow_rc_osc {
>  	struct clk_hw hw;
>  	void __iomem *sckcr;
> +	const struct clk_slow_offsets *offsets;
>  	unsigned long frequency;
>  	unsigned long accuracy;
>  	unsigned long startup_usec;
> @@ -58,6 +70,7 @@ struct clk_slow_rc_osc {
>  struct clk_sam9x5_slow {
>  	struct clk_hw hw;
>  	void __iomem *sckcr;
> +	const struct clk_slow_offsets *offsets;
>  	u8 parent;
>  };
>  
> @@ -69,10 +82,11 @@ static int clk_slow_osc_prepare(struct clk_hw *hw)
>  	void __iomem *sckcr = osc->sckcr;
>  	u32 tmp = readl(sckcr);
>  
> -	if (tmp & (AT91_SCKC_OSC32BYP | AT91_SCKC_OSC32EN))
> +	if (tmp & (AT91_SCKC_OSC32BYP(osc->offsets) |
> +		   AT91_SCKC_OSC32EN(osc->offsets)))
>  		return 0;
>  
> -	writel(tmp | AT91_SCKC_OSC32EN, sckcr);
> +	writel(tmp | AT91_SCKC_OSC32EN(osc->offsets), sckcr);
>  
>  	usleep_range(osc->startup_usec, osc->startup_usec + 1);
>  
> @@ -85,10 +99,10 @@ static void clk_slow_osc_unprepare(struct clk_hw *hw)
>  	void __iomem *sckcr = osc->sckcr;
>  	u32 tmp = readl(sckcr);
>  
> -	if (tmp & AT91_SCKC_OSC32BYP)
> +	if (tmp & AT91_SCKC_OSC32BYP(osc->offsets))
>  		return;
>  
> -	writel(tmp & ~AT91_SCKC_OSC32EN, sckcr);
> +	writel(tmp & ~AT91_SCKC_OSC32EN(osc->offsets), sckcr);
>  }
>  
>  static int clk_slow_osc_is_prepared(struct clk_hw *hw)
> @@ -97,10 +111,10 @@ static int clk_slow_osc_is_prepared(struct clk_hw *hw)
>  	void __iomem *sckcr = osc->sckcr;
>  	u32 tmp = readl(sckcr);
>  
> -	if (tmp & AT91_SCKC_OSC32BYP)
> +	if (tmp & AT91_SCKC_OSC32BYP(osc->offsets))
>  		return 1;
>  
> -	return !!(tmp & AT91_SCKC_OSC32EN);
> +	return !!(tmp & AT91_SCKC_OSC32EN(osc->offsets));
>  }
>  
>  static const struct clk_ops slow_osc_ops = {
> @@ -114,7 +128,8 @@ at91_clk_register_slow_osc(void __iomem *sckcr,
>  			   const char *name,
>  			   const char *parent_name,
>  			   unsigned long startup,
> -			   bool bypass)
> +			   bool bypass,
> +			   const struct clk_slow_offsets *offsets)
>  {
>  	struct clk_slow_osc *osc;
>  	struct clk_hw *hw;
> @@ -137,9 +152,11 @@ at91_clk_register_slow_osc(void __iomem *sckcr,
>  	osc->hw.init = &init;
>  	osc->sckcr = sckcr;
>  	osc->startup_usec = startup;
> +	osc->offsets = offsets;
>  
>  	if (bypass)
> -		writel((readl(sckcr) & ~AT91_SCKC_OSC32EN) | AT91_SCKC_OSC32BYP,
> +		writel((readl(sckcr) & ~AT91_SCKC_OSC32EN(osc->offsets)) |
> +					AT91_SCKC_OSC32BYP(osc->offsets),
>  		       sckcr);
>  
>  	hw = &osc->hw;
> @@ -153,7 +170,8 @@ at91_clk_register_slow_osc(void __iomem *sckcr,
>  }
>  
>  static void __init
> -of_at91sam9x5_clk_slow_osc_setup(struct device_node *np, void __iomem *sckcr)
> +of_at91sam9x5_clk_slow_osc_setup(struct device_node *np, void __iomem *sckcr,
> +				 const struct clk_slow_offsets *offsets)
>  {
>  	struct clk_hw *hw;
>  	const char *parent_name;
> @@ -167,7 +185,7 @@ of_at91sam9x5_clk_slow_osc_setup(struct device_node *np, void __iomem *sckcr)
>  	bypass = of_property_read_bool(np, "atmel,osc-bypass");
>  
>  	hw = at91_clk_register_slow_osc(sckcr, name, parent_name, startup,
> -					 bypass);
> +					bypass, offsets);
>  	if (IS_ERR(hw))
>  		return;
>  
> @@ -195,7 +213,7 @@ static int clk_slow_rc_osc_prepare(struct clk_hw *hw)
>  	struct clk_slow_rc_osc *osc = to_clk_slow_rc_osc(hw);
>  	void __iomem *sckcr = osc->sckcr;
>  
> -	writel(readl(sckcr) | AT91_SCKC_RCEN, sckcr);
> +	writel(readl(sckcr) | AT91_SCKC_RCEN(osc->offsets), sckcr);
>  
>  	usleep_range(osc->startup_usec, osc->startup_usec + 1);
>  
> @@ -207,14 +225,14 @@ static void clk_slow_rc_osc_unprepare(struct clk_hw *hw)
>  	struct clk_slow_rc_osc *osc = to_clk_slow_rc_osc(hw);
>  	void __iomem *sckcr = osc->sckcr;
>  
> -	writel(readl(sckcr) & ~AT91_SCKC_RCEN, sckcr);
> +	writel(readl(sckcr) & ~AT91_SCKC_RCEN(osc->offsets), sckcr);
>  }
>  
>  static int clk_slow_rc_osc_is_prepared(struct clk_hw *hw)
>  {
>  	struct clk_slow_rc_osc *osc = to_clk_slow_rc_osc(hw);
>  
> -	return !!(readl(osc->sckcr) & AT91_SCKC_RCEN);
> +	return !!(readl(osc->sckcr) & AT91_SCKC_RCEN(osc->offsets));
>  }
>  
>  static const struct clk_ops slow_rc_osc_ops = {
> @@ -230,7 +248,8 @@ at91_clk_register_slow_rc_osc(void __iomem *sckcr,
>  			      const char *name,
>  			      unsigned long frequency,
>  			      unsigned long accuracy,
> -			      unsigned long startup)
> +			      unsigned long startup,
> +			      const struct clk_slow_offsets *offsets)
>  {
>  	struct clk_slow_rc_osc *osc;
>  	struct clk_hw *hw;
> @@ -252,6 +271,7 @@ at91_clk_register_slow_rc_osc(void __iomem *sckcr,
>  
>  	osc->hw.init = &init;
>  	osc->sckcr = sckcr;
> +	osc->offsets = offsets;
>  	osc->frequency = frequency;
>  	osc->accuracy = accuracy;
>  	osc->startup_usec = startup;
> @@ -267,7 +287,8 @@ at91_clk_register_slow_rc_osc(void __iomem *sckcr,
>  }
>  
>  static void __init
> -of_at91sam9x5_clk_slow_rc_osc_setup(struct device_node *np, void __iomem *sckcr)
> +of_at91sam9x5_clk_slow_rc_osc_setup(struct device_node *np, void __iomem *sckcr,
> +				    const struct clk_slow_offsets *offsets)
>  {
>  	struct clk_hw *hw;
>  	u32 frequency = 0;
> @@ -281,7 +302,7 @@ of_at91sam9x5_clk_slow_rc_osc_setup(struct device_node *np, void __iomem *sckcr)
>  	of_property_read_u32(np, "atmel,startup-time-usec", &startup);
>  
>  	hw = at91_clk_register_slow_rc_osc(sckcr, name, frequency, accuracy,
> -					    startup);
> +					   startup, offsets);
>  	if (IS_ERR(hw))
>  		return;
>  
> @@ -299,14 +320,14 @@ static int clk_sam9x5_slow_set_parent(struct clk_hw *hw, u8 index)
>  
>  	tmp = readl(sckcr);
>  
> -	if ((!index && !(tmp & AT91_SCKC_OSCSEL)) ||
> -	    (index && (tmp & AT91_SCKC_OSCSEL)))
> +	if ((!index && !(tmp & AT91_SCKC_OSCSEL(slowck->offsets))) ||
> +	    (index && (tmp & AT91_SCKC_OSCSEL(slowck->offsets))))
>  		return 0;
>  
>  	if (index)
> -		tmp |= AT91_SCKC_OSCSEL;
> +		tmp |= AT91_SCKC_OSCSEL(slowck->offsets);
>  	else
> -		tmp &= ~AT91_SCKC_OSCSEL;
> +		tmp &= ~AT91_SCKC_OSCSEL(slowck->offsets);
>  
>  	writel(tmp, sckcr);
>  
> @@ -319,7 +340,7 @@ static u8 clk_sam9x5_slow_get_parent(struct clk_hw *hw)
>  {
>  	struct clk_sam9x5_slow *slowck = to_clk_sam9x5_slow(hw);
>  
> -	return !!(readl(slowck->sckcr) & AT91_SCKC_OSCSEL);
> +	return !!(readl(slowck->sckcr) & AT91_SCKC_OSCSEL(slowck->offsets));
>  }
>  
>  static const struct clk_ops sam9x5_slow_ops = {
> @@ -331,7 +352,8 @@ static struct clk_hw * __init
>  at91_clk_register_sam9x5_slow(void __iomem *sckcr,
>  			      const char *name,
>  			      const char **parent_names,
> -			      int num_parents)
> +			      int num_parents,
> +			      const struct clk_slow_offsets *offsets)
>  {
>  	struct clk_sam9x5_slow *slowck;
>  	struct clk_hw *hw;
> @@ -353,7 +375,8 @@ at91_clk_register_sam9x5_slow(void __iomem *sckcr,
>  
>  	slowck->hw.init = &init;
>  	slowck->sckcr = sckcr;
> -	slowck->parent = !!(readl(sckcr) & AT91_SCKC_OSCSEL);
> +	slowck->offsets = offsets;
> +	slowck->parent = !!(readl(sckcr) & AT91_SCKC_OSCSEL(slowck->offsets));
>  
>  	hw = &slowck->hw;
>  	ret = clk_hw_register(NULL, &slowck->hw);
> @@ -366,7 +389,8 @@ at91_clk_register_sam9x5_slow(void __iomem *sckcr,
>  }
>  
>  static void __init
> -of_at91sam9x5_clk_slow_setup(struct device_node *np, void __iomem *sckcr)
> +of_at91sam9x5_clk_slow_setup(struct device_node *np, void __iomem *sckcr,
> +			     const struct clk_slow_offsets *offsets)
>  {
>  	struct clk_hw *hw;
>  	const char *parent_names[2];
> @@ -382,7 +406,7 @@ of_at91sam9x5_clk_slow_setup(struct device_node *np, void __iomem *sckcr)
>  	of_property_read_string(np, "clock-output-names", &name);
>  
>  	hw = at91_clk_register_sam9x5_slow(sckcr, name, parent_names,
> -					    num_parents);
> +					   num_parents, offsets);
>  	if (IS_ERR(hw))
>  		return;
>  
> @@ -406,10 +430,18 @@ static const struct of_device_id sckc_clk_ids[] __initconst = {
>  	{ /*sentinel*/ }
>  };
>  
> +static const struct clk_slow_offsets at91sam9x5_offsets = {
> +	.cr_rcen = 0,
> +	.cr_osc32en = 1,
> +	.cr_osc32byp = 2,
> +	.cr_oscsel = 3,
> +};
> +
>  static void __init of_at91sam9x5_sckc_setup(struct device_node *np)
>  {
>  	struct device_node *childnp;
> -	void (*clk_setup)(struct device_node *, void __iomem *);
> +	void (*clk_setup)(struct device_node *np, void __iomem *io,
> +			  const struct clk_slow_offsets *offsets);
>  	const struct of_device_id *clk_id;
>  	void __iomem *regbase = of_iomap(np, 0);
>  
> @@ -421,7 +453,7 @@ static void __init of_at91sam9x5_sckc_setup(struct device_node *np)
>  		if (!clk_id)
>  			continue;
>  		clk_setup = clk_id->data;
> -		clk_setup(childnp, regbase);
> +		clk_setup(childnp, regbase, &at91sam9x5_offsets);
>  	}
>  }
>  CLK_OF_DECLARE(at91sam9x5_clk_sckc, "atmel,at91sam9x5-sckc",
> @@ -438,7 +470,7 @@ static int clk_sama5d4_slow_osc_prepare(struct clk_hw *hw)
>  	 * Assume that if it has already been selected (for example by the
>  	 * bootloader), enough time has aready passed.
>  	 */
> -	if ((readl(osc->sckcr) & AT91_SCKC_OSCSEL)) {
> +	if ((readl(osc->sckcr) & AT91_SCKC_OSCSEL(osc->offsets))) {
>  		osc->prepared = true;
>  		return 0;
>  	}
> @@ -461,6 +493,13 @@ static const struct clk_ops sama5d4_slow_osc_ops = {
>  	.is_prepared = clk_sama5d4_slow_osc_is_prepared,
>  };
>  
> +static const struct clk_slow_offsets at91sama5d4_offsets = {
> +	.cr_rcen = AT91_SCKC_OFFSET_INVALID,
> +	.cr_osc32en = AT91_SCKC_OFFSET_INVALID,
> +	.cr_osc32byp = AT91_SCKC_OFFSET_INVALID,
> +	.cr_oscsel = 3,
> +};
> +

I don't think you need to change the sama5d4 part as it is not affected
by any change in the sam9x5 part.

>  static void __init of_sama5d4_sckc_setup(struct device_node *np)
>  {
>  	void __iomem *regbase = of_iomap(np, 0);
> @@ -498,9 +537,11 @@ static void __init of_sama5d4_sckc_setup(struct device_node *np)
>  	osc->hw.init = &init;
>  	osc->sckcr = regbase;
>  	osc->startup_usec = 1200000;
> +	osc->offsets = &at91sama5d4_offsets;
>  
>  	if (bypass)
> -		writel((readl(regbase) | AT91_SCKC_OSC32BYP), regbase);
> +		writel((readl(regbase) | AT91_SCKC_OSC32BYP(osc->offsets)),
> +		       regbase);
>  
>  	hw = &osc->hw;
>  	ret = clk_hw_register(NULL, &osc->hw);
> @@ -509,7 +550,8 @@ static void __init of_sama5d4_sckc_setup(struct device_node *np)
>  		return;
>  	}
>  
> -	hw = at91_clk_register_sam9x5_slow(regbase, "slowck", parent_names, 2);
> +	hw = at91_clk_register_sam9x5_slow(regbase, "slowck", parent_names, 2,
> +					   &at91sama5d4_offsets);
>  	if (IS_ERR(hw))
>  		return;
>  
> -- 
> 2.7.4
> 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/3] clk: at91: sckc: add support for SAM9X60
  2019-02-14 12:14 ` [PATCH v2 2/3] clk: at91: sckc: add support for SAM9X60 Claudiu.Beznea
@ 2019-02-18 21:20   ` Alexandre Belloni
  2019-02-19  9:17     ` Claudiu.Beznea
  0 siblings, 1 reply; 10+ messages in thread
From: Alexandre Belloni @ 2019-02-18 21:20 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: mturquette, sboyd, robh+dt, mark.rutland, Nicolas.Ferre,
	Ludovic.Desroches, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel

On 14/02/2019 12:14:32+0000, Claudiu.Beznea@microchip.com wrote:
> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> 
> Add support for SAM9X60.
> 
> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> ---
>  drivers/clk/at91/sckc.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
> index b7163d3a2269..b3075c51d260 100644
> --- a/drivers/clk/at91/sckc.c
> +++ b/drivers/clk/at91/sckc.c
> @@ -459,6 +459,36 @@ static void __init of_at91sam9x5_sckc_setup(struct device_node *np)
>  CLK_OF_DECLARE(at91sam9x5_clk_sckc, "atmel,at91sam9x5-sckc",
>  	       of_at91sam9x5_sckc_setup);
>  
> +static const struct clk_slow_offsets at91sam9x60_offsets = {
> +	.cr_rcen = AT91_SCKC_OFFSET_INVALID,
> +	.cr_osc32en = 1,
> +	.cr_osc32byp = 2,
> +	.cr_oscsel = 24,
> +};
> +
> +static void __init of_at91sam9x60_sckc_setup(struct device_node *np)
> +{
> +	struct device_node *childnp;
> +	void (*clk_setup)(struct device_node *np, void __iomem *io,
> +			  const struct clk_slow_offsets *offsets);
> +	const struct of_device_id *clk_id;
> +	void __iomem *regbase = of_iomap(np, 0);
> +
> +	if (!regbase)
> +		return;
> +
> +	for_each_child_of_node(np, childnp) {
> +		clk_id = of_match_node(sckc_clk_ids, childnp);
> +		if (!clk_id)
> +			continue;
> +		clk_setup = clk_id->data;
> +		clk_setup(childnp, regbase, &at91sam9x60_offsets);
> +	}

You actually need to have new bindings. The sam9x60 registration should
look more like the sama5d4 registration. I have a rework for the sam9x5
sckc that I will send this week to have a proper binding (i.e: no
children).

However, there is a fundamental change in the sam9x60, previously, the
sckc had only one output clock. the sam9x60 has both td_slck and
md_slck. Both need to be accessible because they are input to the PMC.

This means you will have to register the sckc with of_clk_hw_onecell_get
as the get callback.

We could still decide to do the same with sam9x5 even if it has only one
output clock.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/3] clk: at91: sckc: add support for SAM9X60
  2019-02-18 21:20   ` Alexandre Belloni
@ 2019-02-19  9:17     ` Claudiu.Beznea
  2019-02-20 11:06       ` Alexandre Belloni
  0 siblings, 1 reply; 10+ messages in thread
From: Claudiu.Beznea @ 2019-02-19  9:17 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: mturquette, sboyd, robh+dt, mark.rutland, Nicolas.Ferre,
	Ludovic.Desroches, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel



On 18.02.2019 23:20, Alexandre Belloni wrote:
> On 14/02/2019 12:14:32+0000, Claudiu.Beznea@microchip.com wrote:
>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>
>> Add support for SAM9X60.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  drivers/clk/at91/sckc.c | 30 ++++++++++++++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>>
>> diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
>> index b7163d3a2269..b3075c51d260 100644
>> --- a/drivers/clk/at91/sckc.c
>> +++ b/drivers/clk/at91/sckc.c
>> @@ -459,6 +459,36 @@ static void __init of_at91sam9x5_sckc_setup(struct device_node *np)
>>  CLK_OF_DECLARE(at91sam9x5_clk_sckc, "atmel,at91sam9x5-sckc",
>>  	       of_at91sam9x5_sckc_setup);
>>  
>> +static const struct clk_slow_offsets at91sam9x60_offsets = {
>> +	.cr_rcen = AT91_SCKC_OFFSET_INVALID,
>> +	.cr_osc32en = 1,
>> +	.cr_osc32byp = 2,
>> +	.cr_oscsel = 24,
>> +};
>> +
>> +static void __init of_at91sam9x60_sckc_setup(struct device_node *np)
>> +{
>> +	struct device_node *childnp;
>> +	void (*clk_setup)(struct device_node *np, void __iomem *io,
>> +			  const struct clk_slow_offsets *offsets);
>> +	const struct of_device_id *clk_id;
>> +	void __iomem *regbase = of_iomap(np, 0);
>> +
>> +	if (!regbase)
>> +		return;
>> +
>> +	for_each_child_of_node(np, childnp) {
>> +		clk_id = of_match_node(sckc_clk_ids, childnp);
>> +		if (!clk_id)
>> +			continue;
>> +		clk_setup = clk_id->data;
>> +		clk_setup(childnp, regbase, &at91sam9x60_offsets);
>> +	}
> 
> You actually need to have new bindings. The sam9x60 registration should
> look more like the sama5d4 registration. I have a rework for the sam9x5
> sckc that I will send this week to have a proper binding (i.e: no
> children).

Does this means that this would also solve the problem I tried to address
with this patch?

> 
> However, there is a fundamental change in the sam9x60, previously, the
> sckc had only one output clock. the sam9x60 has both td_slck and
> md_slck. Both need to be accessible because they are input to the PMC.

I was guided by the fact that md_slck is generated by the always on slow RC
oscillator (part of slow clock controller) and since there is no control
for it on slow clock controller there is no need to be described by this
driver.

> 
> This means you will have to register the sckc with of_clk_hw_onecell_get
> as the get callback.

Ok, I'll look into it.

> 
> We could still decide to do the same with sam9x5 even if it has only one
> output clock.
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/3] clk: at91: sckc: add support to specify registers bit offsets
  2019-02-18 21:08   ` Alexandre Belloni
@ 2019-02-19  9:17     ` Claudiu.Beznea
  0 siblings, 0 replies; 10+ messages in thread
From: Claudiu.Beznea @ 2019-02-19  9:17 UTC (permalink / raw)
  To: alexandre.belloni
  Cc: mturquette, sboyd, robh+dt, mark.rutland, Nicolas.Ferre,
	Ludovic.Desroches, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel

Hi,

On 18.02.2019 23:08, Alexandre Belloni wrote:
> Hi,
> 
> On 14/02/2019 12:14:28+0000, Claudiu.Beznea@microchip.com wrote:
>> From: Claudiu Beznea <claudiu.beznea@microchip.com>
>>
>> Different IPs uses different offsets in registers for the same
>> functionality, thus adapt the driver to support this.
>>
>> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
>> ---
>>  drivers/clk/at91/sckc.c | 112 +++++++++++++++++++++++++++++++++---------------
>>  1 file changed, 77 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
>> index ab6ecefc49ad..b7163d3a2269 100644
>> --- a/drivers/clk/at91/sckc.c
>> +++ b/drivers/clk/at91/sckc.c
>> @@ -22,15 +22,25 @@
>>  #define SLOWCK_SW_TIME_USEC	((SLOWCK_SW_CYCLES * USEC_PER_SEC) / \
>>  				 SLOW_CLOCK_FREQ)
>>  
>> -#define	AT91_SCKC_CR			0x00
>> -#define		AT91_SCKC_RCEN		(1 << 0)
>> -#define		AT91_SCKC_OSC32EN	(1 << 1)
>> -#define		AT91_SCKC_OSC32BYP	(1 << 2)
>> -#define		AT91_SCKC_OSCSEL	(1 << 3)
>> +#define	AT91_SCKC_CR		0x00
>> +#define		AT91_SCKC_RCEN(off)	(1 << (off)->cr_rcen)
>> +#define		AT91_SCKC_OSC32EN(off)	(1 << (off)->cr_osc32en)
>> +#define		AT91_SCKC_OSC32BYP(off)	(1 << (off)->cr_osc32byp)
>> +#define		AT91_SCKC_OSCSEL(off)	(1 << (off)->cr_oscsel)
> 
> You don't need those macros if you store BIT(off) instead of off in the
> members of clk_slow_offsets.

Ok, I'll take care of it.

> 
>> +
>> +#define AT91_SCKC_OFFSET_INVALID        (32)
>> +
> 
> This would also remove the need for that define as 0 would mean that the
> bit doesn't exist.

Agree.

> 
>> +struct clk_slow_offsets {
>> +	u8 cr_rcen;
>> +	u8 cr_osc32en;
>> +	u8 cr_osc32byp;
>> +	u8 cr_oscsel;
>> +};
>>  
>>  struct clk_slow_osc {
>>  	struct clk_hw hw;
>>  	void __iomem *sckcr;
>> +	const struct clk_slow_offsets *offsets;
>>  	unsigned long startup_usec;
>>  };
>>  
>> @@ -39,6 +49,7 @@ struct clk_slow_osc {
>>  struct clk_sama5d4_slow_osc {
>>  	struct clk_hw hw;
>>  	void __iomem *sckcr;
>> +	const struct clk_slow_offsets *offsets;
>>  	unsigned long startup_usec;
>>  	bool prepared;
>>  };
>> @@ -48,6 +59,7 @@ struct clk_sama5d4_slow_osc {
>>  struct clk_slow_rc_osc {
>>  	struct clk_hw hw;
>>  	void __iomem *sckcr;
>> +	const struct clk_slow_offsets *offsets;
>>  	unsigned long frequency;
>>  	unsigned long accuracy;
>>  	unsigned long startup_usec;
>> @@ -58,6 +70,7 @@ struct clk_slow_rc_osc {
>>  struct clk_sam9x5_slow {
>>  	struct clk_hw hw;
>>  	void __iomem *sckcr;
>> +	const struct clk_slow_offsets *offsets;
>>  	u8 parent;
>>  };
>>  
>> @@ -69,10 +82,11 @@ static int clk_slow_osc_prepare(struct clk_hw *hw)
>>  	void __iomem *sckcr = osc->sckcr;
>>  	u32 tmp = readl(sckcr);
>>  
>> -	if (tmp & (AT91_SCKC_OSC32BYP | AT91_SCKC_OSC32EN))
>> +	if (tmp & (AT91_SCKC_OSC32BYP(osc->offsets) |
>> +		   AT91_SCKC_OSC32EN(osc->offsets)))
>>  		return 0;
>>  
>> -	writel(tmp | AT91_SCKC_OSC32EN, sckcr);
>> +	writel(tmp | AT91_SCKC_OSC32EN(osc->offsets), sckcr);
>>  
>>  	usleep_range(osc->startup_usec, osc->startup_usec + 1);
>>  
>> @@ -85,10 +99,10 @@ static void clk_slow_osc_unprepare(struct clk_hw *hw)
>>  	void __iomem *sckcr = osc->sckcr;
>>  	u32 tmp = readl(sckcr);
>>  
>> -	if (tmp & AT91_SCKC_OSC32BYP)
>> +	if (tmp & AT91_SCKC_OSC32BYP(osc->offsets))
>>  		return;
>>  
>> -	writel(tmp & ~AT91_SCKC_OSC32EN, sckcr);
>> +	writel(tmp & ~AT91_SCKC_OSC32EN(osc->offsets), sckcr);
>>  }
>>  
>>  static int clk_slow_osc_is_prepared(struct clk_hw *hw)
>> @@ -97,10 +111,10 @@ static int clk_slow_osc_is_prepared(struct clk_hw *hw)
>>  	void __iomem *sckcr = osc->sckcr;
>>  	u32 tmp = readl(sckcr);
>>  
>> -	if (tmp & AT91_SCKC_OSC32BYP)
>> +	if (tmp & AT91_SCKC_OSC32BYP(osc->offsets))
>>  		return 1;
>>  
>> -	return !!(tmp & AT91_SCKC_OSC32EN);
>> +	return !!(tmp & AT91_SCKC_OSC32EN(osc->offsets));
>>  }
>>  
>>  static const struct clk_ops slow_osc_ops = {
>> @@ -114,7 +128,8 @@ at91_clk_register_slow_osc(void __iomem *sckcr,
>>  			   const char *name,
>>  			   const char *parent_name,
>>  			   unsigned long startup,
>> -			   bool bypass)
>> +			   bool bypass,
>> +			   const struct clk_slow_offsets *offsets)
>>  {
>>  	struct clk_slow_osc *osc;
>>  	struct clk_hw *hw;
>> @@ -137,9 +152,11 @@ at91_clk_register_slow_osc(void __iomem *sckcr,
>>  	osc->hw.init = &init;
>>  	osc->sckcr = sckcr;
>>  	osc->startup_usec = startup;
>> +	osc->offsets = offsets;
>>  
>>  	if (bypass)
>> -		writel((readl(sckcr) & ~AT91_SCKC_OSC32EN) | AT91_SCKC_OSC32BYP,
>> +		writel((readl(sckcr) & ~AT91_SCKC_OSC32EN(osc->offsets)) |
>> +					AT91_SCKC_OSC32BYP(osc->offsets),
>>  		       sckcr);
>>  
>>  	hw = &osc->hw;
>> @@ -153,7 +170,8 @@ at91_clk_register_slow_osc(void __iomem *sckcr,
>>  }
>>  
>>  static void __init
>> -of_at91sam9x5_clk_slow_osc_setup(struct device_node *np, void __iomem *sckcr)
>> +of_at91sam9x5_clk_slow_osc_setup(struct device_node *np, void __iomem *sckcr,
>> +				 const struct clk_slow_offsets *offsets)
>>  {
>>  	struct clk_hw *hw;
>>  	const char *parent_name;
>> @@ -167,7 +185,7 @@ of_at91sam9x5_clk_slow_osc_setup(struct device_node *np, void __iomem *sckcr)
>>  	bypass = of_property_read_bool(np, "atmel,osc-bypass");
>>  
>>  	hw = at91_clk_register_slow_osc(sckcr, name, parent_name, startup,
>> -					 bypass);
>> +					bypass, offsets);
>>  	if (IS_ERR(hw))
>>  		return;
>>  
>> @@ -195,7 +213,7 @@ static int clk_slow_rc_osc_prepare(struct clk_hw *hw)
>>  	struct clk_slow_rc_osc *osc = to_clk_slow_rc_osc(hw);
>>  	void __iomem *sckcr = osc->sckcr;
>>  
>> -	writel(readl(sckcr) | AT91_SCKC_RCEN, sckcr);
>> +	writel(readl(sckcr) | AT91_SCKC_RCEN(osc->offsets), sckcr);
>>  
>>  	usleep_range(osc->startup_usec, osc->startup_usec + 1);
>>  
>> @@ -207,14 +225,14 @@ static void clk_slow_rc_osc_unprepare(struct clk_hw *hw)
>>  	struct clk_slow_rc_osc *osc = to_clk_slow_rc_osc(hw);
>>  	void __iomem *sckcr = osc->sckcr;
>>  
>> -	writel(readl(sckcr) & ~AT91_SCKC_RCEN, sckcr);
>> +	writel(readl(sckcr) & ~AT91_SCKC_RCEN(osc->offsets), sckcr);
>>  }
>>  
>>  static int clk_slow_rc_osc_is_prepared(struct clk_hw *hw)
>>  {
>>  	struct clk_slow_rc_osc *osc = to_clk_slow_rc_osc(hw);
>>  
>> -	return !!(readl(osc->sckcr) & AT91_SCKC_RCEN);
>> +	return !!(readl(osc->sckcr) & AT91_SCKC_RCEN(osc->offsets));
>>  }
>>  
>>  static const struct clk_ops slow_rc_osc_ops = {
>> @@ -230,7 +248,8 @@ at91_clk_register_slow_rc_osc(void __iomem *sckcr,
>>  			      const char *name,
>>  			      unsigned long frequency,
>>  			      unsigned long accuracy,
>> -			      unsigned long startup)
>> +			      unsigned long startup,
>> +			      const struct clk_slow_offsets *offsets)
>>  {
>>  	struct clk_slow_rc_osc *osc;
>>  	struct clk_hw *hw;
>> @@ -252,6 +271,7 @@ at91_clk_register_slow_rc_osc(void __iomem *sckcr,
>>  
>>  	osc->hw.init = &init;
>>  	osc->sckcr = sckcr;
>> +	osc->offsets = offsets;
>>  	osc->frequency = frequency;
>>  	osc->accuracy = accuracy;
>>  	osc->startup_usec = startup;
>> @@ -267,7 +287,8 @@ at91_clk_register_slow_rc_osc(void __iomem *sckcr,
>>  }
>>  
>>  static void __init
>> -of_at91sam9x5_clk_slow_rc_osc_setup(struct device_node *np, void __iomem *sckcr)
>> +of_at91sam9x5_clk_slow_rc_osc_setup(struct device_node *np, void __iomem *sckcr,
>> +				    const struct clk_slow_offsets *offsets)
>>  {
>>  	struct clk_hw *hw;
>>  	u32 frequency = 0;
>> @@ -281,7 +302,7 @@ of_at91sam9x5_clk_slow_rc_osc_setup(struct device_node *np, void __iomem *sckcr)
>>  	of_property_read_u32(np, "atmel,startup-time-usec", &startup);
>>  
>>  	hw = at91_clk_register_slow_rc_osc(sckcr, name, frequency, accuracy,
>> -					    startup);
>> +					   startup, offsets);
>>  	if (IS_ERR(hw))
>>  		return;
>>  
>> @@ -299,14 +320,14 @@ static int clk_sam9x5_slow_set_parent(struct clk_hw *hw, u8 index)
>>  
>>  	tmp = readl(sckcr);
>>  
>> -	if ((!index && !(tmp & AT91_SCKC_OSCSEL)) ||
>> -	    (index && (tmp & AT91_SCKC_OSCSEL)))
>> +	if ((!index && !(tmp & AT91_SCKC_OSCSEL(slowck->offsets))) ||
>> +	    (index && (tmp & AT91_SCKC_OSCSEL(slowck->offsets))))
>>  		return 0;
>>  
>>  	if (index)
>> -		tmp |= AT91_SCKC_OSCSEL;
>> +		tmp |= AT91_SCKC_OSCSEL(slowck->offsets);
>>  	else
>> -		tmp &= ~AT91_SCKC_OSCSEL;
>> +		tmp &= ~AT91_SCKC_OSCSEL(slowck->offsets);
>>  
>>  	writel(tmp, sckcr);
>>  
>> @@ -319,7 +340,7 @@ static u8 clk_sam9x5_slow_get_parent(struct clk_hw *hw)
>>  {
>>  	struct clk_sam9x5_slow *slowck = to_clk_sam9x5_slow(hw);
>>  
>> -	return !!(readl(slowck->sckcr) & AT91_SCKC_OSCSEL);
>> +	return !!(readl(slowck->sckcr) & AT91_SCKC_OSCSEL(slowck->offsets));
>>  }
>>  
>>  static const struct clk_ops sam9x5_slow_ops = {
>> @@ -331,7 +352,8 @@ static struct clk_hw * __init
>>  at91_clk_register_sam9x5_slow(void __iomem *sckcr,
>>  			      const char *name,
>>  			      const char **parent_names,
>> -			      int num_parents)
>> +			      int num_parents,
>> +			      const struct clk_slow_offsets *offsets)
>>  {
>>  	struct clk_sam9x5_slow *slowck;
>>  	struct clk_hw *hw;
>> @@ -353,7 +375,8 @@ at91_clk_register_sam9x5_slow(void __iomem *sckcr,
>>  
>>  	slowck->hw.init = &init;
>>  	slowck->sckcr = sckcr;
>> -	slowck->parent = !!(readl(sckcr) & AT91_SCKC_OSCSEL);
>> +	slowck->offsets = offsets;
>> +	slowck->parent = !!(readl(sckcr) & AT91_SCKC_OSCSEL(slowck->offsets));
>>  
>>  	hw = &slowck->hw;
>>  	ret = clk_hw_register(NULL, &slowck->hw);
>> @@ -366,7 +389,8 @@ at91_clk_register_sam9x5_slow(void __iomem *sckcr,
>>  }
>>  
>>  static void __init
>> -of_at91sam9x5_clk_slow_setup(struct device_node *np, void __iomem *sckcr)
>> +of_at91sam9x5_clk_slow_setup(struct device_node *np, void __iomem *sckcr,
>> +			     const struct clk_slow_offsets *offsets)
>>  {
>>  	struct clk_hw *hw;
>>  	const char *parent_names[2];
>> @@ -382,7 +406,7 @@ of_at91sam9x5_clk_slow_setup(struct device_node *np, void __iomem *sckcr)
>>  	of_property_read_string(np, "clock-output-names", &name);
>>  
>>  	hw = at91_clk_register_sam9x5_slow(sckcr, name, parent_names,
>> -					    num_parents);
>> +					   num_parents, offsets);
>>  	if (IS_ERR(hw))
>>  		return;
>>  
>> @@ -406,10 +430,18 @@ static const struct of_device_id sckc_clk_ids[] __initconst = {
>>  	{ /*sentinel*/ }
>>  };
>>  
>> +static const struct clk_slow_offsets at91sam9x5_offsets = {
>> +	.cr_rcen = 0,
>> +	.cr_osc32en = 1,
>> +	.cr_osc32byp = 2,
>> +	.cr_oscsel = 3,
>> +};
>> +
>>  static void __init of_at91sam9x5_sckc_setup(struct device_node *np)
>>  {
>>  	struct device_node *childnp;
>> -	void (*clk_setup)(struct device_node *, void __iomem *);
>> +	void (*clk_setup)(struct device_node *np, void __iomem *io,
>> +			  const struct clk_slow_offsets *offsets);
>>  	const struct of_device_id *clk_id;
>>  	void __iomem *regbase = of_iomap(np, 0);
>>  
>> @@ -421,7 +453,7 @@ static void __init of_at91sam9x5_sckc_setup(struct device_node *np)
>>  		if (!clk_id)
>>  			continue;
>>  		clk_setup = clk_id->data;
>> -		clk_setup(childnp, regbase);
>> +		clk_setup(childnp, regbase, &at91sam9x5_offsets);
>>  	}
>>  }
>>  CLK_OF_DECLARE(at91sam9x5_clk_sckc, "atmel,at91sam9x5-sckc",
>> @@ -438,7 +470,7 @@ static int clk_sama5d4_slow_osc_prepare(struct clk_hw *hw)
>>  	 * Assume that if it has already been selected (for example by the
>>  	 * bootloader), enough time has aready passed.
>>  	 */
>> -	if ((readl(osc->sckcr) & AT91_SCKC_OSCSEL)) {
>> +	if ((readl(osc->sckcr) & AT91_SCKC_OSCSEL(osc->offsets))) {
>>  		osc->prepared = true;
>>  		return 0;
>>  	}
>> @@ -461,6 +493,13 @@ static const struct clk_ops sama5d4_slow_osc_ops = {
>>  	.is_prepared = clk_sama5d4_slow_osc_is_prepared,
>>  };
>>  
>> +static const struct clk_slow_offsets at91sama5d4_offsets = {
>> +	.cr_rcen = AT91_SCKC_OFFSET_INVALID,
>> +	.cr_osc32en = AT91_SCKC_OFFSET_INVALID,
>> +	.cr_osc32byp = AT91_SCKC_OFFSET_INVALID,
>> +	.cr_oscsel = 3,
>> +};
>> +
> 
> I don't think you need to change the sama5d4 part as it is not affected
> by any change in the sam9x5 part.

Ok.

> 
>>  static void __init of_sama5d4_sckc_setup(struct device_node *np)
>>  {
>>  	void __iomem *regbase = of_iomap(np, 0);
>> @@ -498,9 +537,11 @@ static void __init of_sama5d4_sckc_setup(struct device_node *np)
>>  	osc->hw.init = &init;
>>  	osc->sckcr = regbase;
>>  	osc->startup_usec = 1200000;
>> +	osc->offsets = &at91sama5d4_offsets;
>>  
>>  	if (bypass)
>> -		writel((readl(regbase) | AT91_SCKC_OSC32BYP), regbase);
>> +		writel((readl(regbase) | AT91_SCKC_OSC32BYP(osc->offsets)),
>> +		       regbase);
>>  
>>  	hw = &osc->hw;
>>  	ret = clk_hw_register(NULL, &osc->hw);
>> @@ -509,7 +550,8 @@ static void __init of_sama5d4_sckc_setup(struct device_node *np)
>>  		return;
>>  	}
>>  
>> -	hw = at91_clk_register_sam9x5_slow(regbase, "slowck", parent_names, 2);
>> +	hw = at91_clk_register_sam9x5_slow(regbase, "slowck", parent_names, 2,
>> +					   &at91sama5d4_offsets);
>>  	if (IS_ERR(hw))
>>  		return;
>>  
>> -- 
>> 2.7.4
>>
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/3] clk: at91: sckc: add support for SAM9X60
  2019-02-19  9:17     ` Claudiu.Beznea
@ 2019-02-20 11:06       ` Alexandre Belloni
  0 siblings, 0 replies; 10+ messages in thread
From: Alexandre Belloni @ 2019-02-20 11:06 UTC (permalink / raw)
  To: Claudiu.Beznea
  Cc: mturquette, sboyd, robh+dt, mark.rutland, Nicolas.Ferre,
	Ludovic.Desroches, linux-clk, devicetree, linux-arm-kernel,
	linux-kernel

On 19/02/2019 09:17:43+0000, Claudiu.Beznea@microchip.com wrote:
> 
> 
> On 18.02.2019 23:20, Alexandre Belloni wrote:
> > On 14/02/2019 12:14:32+0000, Claudiu.Beznea@microchip.com wrote:
> >> From: Claudiu Beznea <claudiu.beznea@microchip.com>
> >>
> >> Add support for SAM9X60.
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com>
> >> ---
> >>  drivers/clk/at91/sckc.c | 30 ++++++++++++++++++++++++++++++
> >>  1 file changed, 30 insertions(+)
> >>
> >> diff --git a/drivers/clk/at91/sckc.c b/drivers/clk/at91/sckc.c
> >> index b7163d3a2269..b3075c51d260 100644
> >> --- a/drivers/clk/at91/sckc.c
> >> +++ b/drivers/clk/at91/sckc.c
> >> @@ -459,6 +459,36 @@ static void __init of_at91sam9x5_sckc_setup(struct device_node *np)
> >>  CLK_OF_DECLARE(at91sam9x5_clk_sckc, "atmel,at91sam9x5-sckc",
> >>  	       of_at91sam9x5_sckc_setup);
> >>  
> >> +static const struct clk_slow_offsets at91sam9x60_offsets = {
> >> +	.cr_rcen = AT91_SCKC_OFFSET_INVALID,
> >> +	.cr_osc32en = 1,
> >> +	.cr_osc32byp = 2,
> >> +	.cr_oscsel = 24,
> >> +};
> >> +
> >> +static void __init of_at91sam9x60_sckc_setup(struct device_node *np)
> >> +{
> >> +	struct device_node *childnp;
> >> +	void (*clk_setup)(struct device_node *np, void __iomem *io,
> >> +			  const struct clk_slow_offsets *offsets);
> >> +	const struct of_device_id *clk_id;
> >> +	void __iomem *regbase = of_iomap(np, 0);
> >> +
> >> +	if (!regbase)
> >> +		return;
> >> +
> >> +	for_each_child_of_node(np, childnp) {
> >> +		clk_id = of_match_node(sckc_clk_ids, childnp);
> >> +		if (!clk_id)
> >> +			continue;
> >> +		clk_setup = clk_id->data;
> >> +		clk_setup(childnp, regbase, &at91sam9x60_offsets);
> >> +	}
> > 
> > You actually need to have new bindings. The sam9x60 registration should
> > look more like the sama5d4 registration. I have a rework for the sam9x5
> > sckc that I will send this week to have a proper binding (i.e: no
> > children).
> 
> Does this means that this would also solve the problem I tried to address
> with this patch?
> 

I've sent the series now. It doesn't solve this particular issue.

> > 
> > However, there is a fundamental change in the sam9x60, previously, the
> > sckc had only one output clock. the sam9x60 has both td_slck and
> > md_slck. Both need to be accessible because they are input to the PMC.
> 
> I was guided by the fact that md_slck is generated by the always on slow RC
> oscillator (part of slow clock controller) and since there is no control
> for it on slow clock controller there is no need to be described by this
> driver.
> 

Well, then it would need to be added as an input to the sckc which would
mean that you still need to change the binding. This doesn't matter too
much but else your clock tree would be wrong.

> > 
> > This means you will have to register the sckc with of_clk_hw_onecell_get
> > as the get callback.
> 
> Ok, I'll look into it.
> 
> > 
> > We could still decide to do the same with sam9x5 even if it has only one
> > output clock.
> > 

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-02-20 11:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-14 12:14 [PATCH v2 0/3] add slow clock support for SAM9X60 Claudiu.Beznea
2019-02-14 12:14 ` [PATCH v2 1/3] clk: at91: sckc: add support to specify registers bit offsets Claudiu.Beznea
2019-02-18 21:08   ` Alexandre Belloni
2019-02-19  9:17     ` Claudiu.Beznea
2019-02-14 12:14 ` [PATCH v2 2/3] clk: at91: sckc: add support for SAM9X60 Claudiu.Beznea
2019-02-18 21:20   ` Alexandre Belloni
2019-02-19  9:17     ` Claudiu.Beznea
2019-02-20 11:06       ` Alexandre Belloni
2019-02-14 12:14 ` [PATCH v2 3/3] dt-bindings: clk: at91: add bindings for SAM9X60's slow clock controller Claudiu.Beznea
2019-02-18 20:04   ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).