* [PATCH 0/2] clk: renesas: mstp: Add support for RZ/A2
@ 2018-07-11 17:03 Chris Brandt
2018-07-11 17:03 ` [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 Chris Brandt
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Chris Brandt @ 2018-07-11 17:03 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland,
Geert Uytterhoeven
Cc: linux-clk, devicetree, linux-renesas-soc, Simon Horman, Chris Brandt
Add support for RZ/A2 series.
The clock HW is similar to RZ/A1, but with different dividers
and additional clocks sources.
Chris Brandt (2):
clk: renesas: mstp: Add support for r7s9210
clk: renesas: mstp: Document R7S9210 support
.../bindings/clock/renesas,cpg-mstp-clocks.txt | 1 +
drivers/clk/renesas/Kconfig | 5 +
drivers/clk/renesas/Makefile | 1 +
drivers/clk/renesas/clk-mstp.c | 3 +
drivers/clk/renesas/clk-rz.c | 155 ++++++++++++++++-----
5 files changed, 131 insertions(+), 34 deletions(-)
--
2.16.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 2018-07-11 17:03 [PATCH 0/2] clk: renesas: mstp: Add support for RZ/A2 Chris Brandt @ 2018-07-11 17:03 ` Chris Brandt 2018-07-13 9:58 ` Geert Uytterhoeven 2018-07-11 17:03 ` [PATCH 2/2] clk: renesas: mstp: Document R7S9210 support Chris Brandt 2018-07-20 8:10 ` [PATCH 0/2] clk: renesas: mstp: Add support for RZ/A2 Geert Uytterhoeven 2 siblings, 1 reply; 11+ messages in thread From: Chris Brandt @ 2018-07-11 17:03 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Geert Uytterhoeven Cc: linux-clk, devicetree, linux-renesas-soc, Simon Horman, Chris Brandt Add support for RZ/A2 series. The clock HW is similar to RZ/A1, but with different dividers and additional clocks sources. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- drivers/clk/renesas/Kconfig | 5 ++ drivers/clk/renesas/Makefile | 1 + drivers/clk/renesas/clk-mstp.c | 3 + drivers/clk/renesas/clk-rz.c | 155 ++++++++++++++++++++++++++++++++--------- 4 files changed, 130 insertions(+), 34 deletions(-) diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig index 9022bbe1297e..b08d44b8a476 100644 --- a/drivers/clk/renesas/Kconfig +++ b/drivers/clk/renesas/Kconfig @@ -3,6 +3,7 @@ config CLK_RENESAS default y if ARCH_RENESAS select CLK_EMEV2 if ARCH_EMEV2 select CLK_RZA1 if ARCH_R7S72100 + select CLK_RZA2 if ARCH_R7S9210 select CLK_R8A73A4 if ARCH_R8A73A4 select CLK_R8A7740 if ARCH_R8A7740 select CLK_R8A7743 if ARCH_R8A7743 @@ -45,6 +46,10 @@ config CLK_RZA1 bool "RZ/A1H clock support" if COMPILE_TEST select CLK_RENESAS_CPG_MSTP +config CLK_RZA2 + bool "RZ/A2 clock support" if COMPILE_TEST + select CLK_RENESAS_CPG_MSTP + config CLK_R8A73A4 bool "R-Mobile APE6 clock support" if COMPILE_TEST select CLK_RENESAS_CPG_MSTP diff --git a/drivers/clk/renesas/Makefile b/drivers/clk/renesas/Makefile index e4aa3d6143d2..6159ee43f7ca 100644 --- a/drivers/clk/renesas/Makefile +++ b/drivers/clk/renesas/Makefile @@ -2,6 +2,7 @@ # SoC obj-$(CONFIG_CLK_EMEV2) += clk-emev2.o obj-$(CONFIG_CLK_RZA1) += clk-rz.o +obj-$(CONFIG_CLK_RZA2) += clk-rz.o obj-$(CONFIG_CLK_R8A73A4) += clk-r8a73a4.o obj-$(CONFIG_CLK_R8A7740) += clk-r8a7740.o obj-$(CONFIG_CLK_R8A7743) += r8a7743-cpg-mssr.o diff --git a/drivers/clk/renesas/clk-mstp.c b/drivers/clk/renesas/clk-mstp.c index e82adcb16a52..9470ab8acc13 100644 --- a/drivers/clk/renesas/clk-mstp.c +++ b/drivers/clk/renesas/clk-mstp.c @@ -213,6 +213,9 @@ static void __init cpg_mstp_clocks_init(struct device_node *np) if (of_device_is_compatible(np, "renesas,r7s72100-mstp-clocks")) group->width_8bit = true; + if (of_device_is_compatible(np, "renesas,r7s9210-mstp-clocks")) + group->width_8bit = true; + for (i = 0; i < MSTP_MAX_CLOCKS; ++i) clks[i] = ERR_PTR(-ENOENT); diff --git a/drivers/clk/renesas/clk-rz.c b/drivers/clk/renesas/clk-rz.c index ac2f86d626b6..199c6ae9704c 100644 --- a/drivers/clk/renesas/clk-rz.c +++ b/drivers/clk/renesas/clk-rz.c @@ -1,5 +1,5 @@ /* - * RZ/A1 Core CPG Clocks + * RZ/A Core CPG Clocks * * Copyright (C) 2013 Ideas On Board SPRL * Copyright (C) 2014 Wolfram Sang, Sang Engineering <wsa@sang-engineering.com> @@ -24,44 +24,95 @@ struct rz_cpg { #define CPG_FRQCR 0x10 #define CPG_FRQCR2 0x14 +#define SWRSTCR3 0xFCFE0468 +/* RZ/A1 */ #define PPR0 0xFCFE3200 #define PIBC0 0xFCFE7000 -#define MD_CLK(x) ((x >> 2) & 1) /* P0_2 */ +/* RZ/A2 */ +#define PORTL_PIDR 0xFCFFE074 + +#define RZA1 1 +#define RZA2 2 /* ----------------------------------------------------------------------------- * Initialization */ +int detect_rz(void) +{ + void __iomem *swrstcr3; + static int rz_device; + + if (!rz_device) { + swrstcr3 = ioremap_nocache(SWRSTCR3, 1); + BUG_ON(!swrstcr3); + if (ioread8(swrstcr3)) + rz_device = RZA1; + else + rz_device = RZA2; + iounmap(swrstcr3); + } + return rz_device; +} -static u16 __init rz_cpg_read_mode_pins(void) +static u8 __init rz_cpg_read_mode_pin(void) { - void __iomem *ppr0, *pibc0; - u16 modes; - - ppr0 = ioremap_nocache(PPR0, 2); - pibc0 = ioremap_nocache(PIBC0, 2); - BUG_ON(!ppr0 || !pibc0); - iowrite16(4, pibc0); /* enable input buffer */ - modes = ioread16(ppr0); - iounmap(ppr0); - iounmap(pibc0); - - return modes; + void __iomem *ppr0, *pibc0, *pidr; + u8 mode; + + if (detect_rz() == RZA1) { + /* RZ/A1 */ + /* MD_CLK pin is P0_2 */ + ppr0 = ioremap_nocache(PPR0, 2); + pibc0 = ioremap_nocache(PIBC0, 2); + BUG_ON(!ppr0 || !pibc0); + iowrite16(4, pibc0); /* enable input buffer */ + mode = (u8)((ioread16(ppr0) >> 2) & 1); + iounmap(ppr0); + iounmap(pibc0); + } else { + /* RZ/A2 */ + /* MD_CLK pin is PL_1 */ + pidr = ioremap_nocache(PORTL_PIDR, 1); + BUG_ON(!pidr); + mode = (ioread8(pidr) >> 1) & 1; + iounmap(pidr); + } + + return mode; } static struct clk * __init rz_cpg_register_clock(struct device_node *np, struct rz_cpg *cpg, const char *name) { u32 val; - unsigned mult; - static const unsigned frqcr_tab[4] = { 3, 2, 0, 1 }; + unsigned int mult, div; + static const unsigned int rza1_frqcr_tab[4] = { 3, 2, 0, 1 }; + static const unsigned int rza2_frqcr_tab[5][5] = { + /* I, G, B, P1, P0 */ + { 2, 4, 8, 16, 32 }, /* FRQCR = 0x012 */ + { 4, 4, 8, 16, 32 }, /* FRQCR = 0x112 */ + { 8, 4, 8, 16, 32 }, /* FRQCR = 0x212 */ + { 16, 8, 16, 16, 32 }, /* FRQCR = 0x322 */ + { 16, 16, 32, 32, 32 }, /* FRQCR = 0x333 */ + }; - if (strcmp(name, "pll") == 0) { - unsigned int cpg_mode = MD_CLK(rz_cpg_read_mode_pins()); - const char *parent_name = of_clk_get_parent_name(np, cpg_mode); + unsigned int cpg_mode = rz_cpg_read_mode_pin(); - mult = cpg_mode ? (32 / 4) : 30; + + if (strcmp(name, "pll") == 0) { + const char *parent_name; + + if (detect_rz() == RZA1) { + /* RZ/A1 */ + parent_name = of_clk_get_parent_name(np, cpg_mode); + mult = cpg_mode ? (32 / 4) : 30; + } else { + /* RZ/A2 */ + parent_name = of_clk_get_parent_name(np, 0); + mult = 88; + } return clk_register_fixed_factor(NULL, name, parent_name, 0, mult, 1); } @@ -70,19 +121,55 @@ rz_cpg_register_clock(struct device_node *np, struct rz_cpg *cpg, const char *na if (!cpg->reg) return ERR_PTR(-ENXIO); - /* FIXME:"i" and "g" are variable clocks with non-integer dividers (e.g. 2/3) - * and the constraint that always g <= i. To get the rz platform started, - * let them run at fixed current speed and implement the details later. - */ - if (strcmp(name, "i") == 0) - val = (readl(cpg->reg + CPG_FRQCR) >> 8) & 3; - else if (strcmp(name, "g") == 0) - val = readl(cpg->reg + CPG_FRQCR2) & 3; - else - return ERR_PTR(-EINVAL); - - mult = frqcr_tab[val]; - return clk_register_fixed_factor(NULL, name, "pll", 0, mult, 3); + if (detect_rz() == RZA1) { + /* FIXME:"i" and "g" are variable clocks with non-integer dividers (e.g. 2/3) + * and the constraint that always g <= i. To get the rz platform started, + * let them run at fixed current speed and implement the details later. + */ + if (strcmp(name, "i") == 0) + val = (readl(cpg->reg + CPG_FRQCR) >> 8) & 3; + else if (strcmp(name, "g") == 0) + val = readl(cpg->reg + CPG_FRQCR2) & 3; + else + return ERR_PTR(-EINVAL); + mult = rza1_frqcr_tab[val]; + div = 3; + } else { + /* RZ/A2 */ + val = clk_readl(cpg->reg + CPG_FRQCR) & 0xFFF; + if (val == 0x012) + val = 0; + else if (val == 0x112) + val = 1; + else if (val == 0x212) + val = 2; + else if (val == 0x322) + val = 3; + else if (val == 0x333) + val = 4; + else + BUG_ON(1); /* Illegal FRQCR value */ + + if (strcmp(name, "i") == 0) + div = rza2_frqcr_tab[val][0]; + else if (strcmp(name, "g") == 0) + div = rza2_frqcr_tab[val][1]; + else if (strcmp(name, "b") == 0) + div = rza2_frqcr_tab[val][2]; + else if ((strcmp(name, "p1") == 0) || + (strcmp(name, "p1c") == 0)) + div = rza2_frqcr_tab[val][3]; + else if (strcmp(name, "p0") == 0) + div = rza2_frqcr_tab[val][4]; + else + return ERR_PTR(-EINVAL); + + mult = 1; + if (cpg_mode) + div *= 2; /* div 2 circuit before PLL */ + + } + return clk_register_fixed_factor(NULL, name, "pll", 0, mult, div); } static void __init rz_cpg_clocks_init(struct device_node *np) -- 2.16.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 2018-07-11 17:03 ` [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 Chris Brandt @ 2018-07-13 9:58 ` Geert Uytterhoeven 2018-07-13 16:27 ` Chris Brandt 0 siblings, 1 reply; 11+ messages in thread From: Geert Uytterhoeven @ 2018-07-13 9:58 UTC (permalink / raw) To: Chris Brandt Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Geert Uytterhoeven, linux-clk, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux-Renesas, Simon Horman Hi Chris, Thanks for your patch! On Wed, Jul 11, 2018 at 7:03 PM Chris Brandt <chris.brandt@renesas.com> wrote: > [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 Please drop the "mstp", as the largest share of this patch is not about MSTP clocks. > Add support for RZ/A2 series. > The clock HW is similar to RZ/A1, but with different dividers > and additional clocks sources. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > drivers/clk/renesas/Kconfig | 5 ++ > drivers/clk/renesas/Makefile | 1 + > drivers/clk/renesas/clk-mstp.c | 3 + > drivers/clk/renesas/clk-rz.c | 155 ++++++++++++++++++++++++++++++++--------- You're adding ca. 100 new lines to an existing driver of 126 lines, most of which are depending on the result of detect_rz()? So I think you're best of adding a complete new driver clk-rza2.c, matching against "renesas,r7s9210-cpg-clocks". The "renesas,rz-cpg-clocks" won't be needed for RZ/A2. And perhaps rename clk-rz.c to clk-rza1.c, and change its match string to "renesas,r7s72100-cpg-clocks"? BTW, please use fcfe0020 as the base address for the CPG (which requires changing the register offsets in the driver), to avoid the warning we're seeing with "make dtbs W=1" for RZ/A1: Warning (unique_unit_address): /soc/watchdog@fcfe0000: duplicate unit-address (also used in node /soc/cpg_clocks@fcfe0000) BTW2, I guess I can't convince you to write a modern new clock driver using a single register block, describing all core and module clocks in C tables? That would avoid making mistakes in keeping the clocks/clock-indices/ clock-output-names properties in the mstp clock nodes in DT in sync. It would also make your life easier if you ever decide to support software reset using the Software Reset Control Register in the same register block. > --- a/drivers/clk/renesas/clk-mstp.c > +++ b/drivers/clk/renesas/clk-mstp.c > @@ -213,6 +213,9 @@ static void __init cpg_mstp_clocks_init(struct device_node *np) > if (of_device_is_compatible(np, "renesas,r7s72100-mstp-clocks")) > group->width_8bit = true; > > + if (of_device_is_compatible(np, "renesas,r7s9210-mstp-clocks")) You can merge the test with the test for RZ/A1 above. > + group->width_8bit = true; > + > for (i = 0; i < MSTP_MAX_CLOCKS; ++i) > clks[i] = ERR_PTR(-ENOENT); > > diff --git a/drivers/clk/renesas/clk-rz.c b/drivers/clk/renesas/clk-rz.c > index ac2f86d626b6..199c6ae9704c 100644 > --- a/drivers/clk/renesas/clk-rz.c > +++ b/drivers/clk/renesas/clk-rz.c > @@ -24,44 +24,95 @@ struct rz_cpg { > > #define CPG_FRQCR 0x10 > #define CPG_FRQCR2 0x14 > +#define SWRSTCR3 0xFCFE0468 > > +/* RZ/A1 */ > #define PPR0 0xFCFE3200 > #define PIBC0 0xFCFE7000 > > -#define MD_CLK(x) ((x >> 2) & 1) /* P0_2 */ > +/* RZ/A2 */ > +#define PORTL_PIDR 0xFCFFE074 > + > +#define RZA1 1 > +#define RZA2 2 > > /* ----------------------------------------------------------------------------- > * Initialization > */ > +int detect_rz(void) > +{ > + void __iomem *swrstcr3; > + static int rz_device; > + > + if (!rz_device) { > + swrstcr3 = ioremap_nocache(SWRSTCR3, 1); > + BUG_ON(!swrstcr3); > + if (ioread8(swrstcr3)) > + rz_device = RZA1; > + else > + rz_device = RZA2; > + iounmap(swrstcr3); > + } > + return rz_device; > +} Please use the compatible value for differentiating (issue is moot with a separate driver). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 2018-07-13 9:58 ` Geert Uytterhoeven 2018-07-13 16:27 ` Chris Brandt @ 2018-07-13 16:27 ` Chris Brandt 0 siblings, 0 replies; 11+ messages in thread From: Chris Brandt @ 2018-07-13 16:27 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Geert Uytterhoeven, linux-clk, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <devicetree@vger.kernel.org>, Linux-Renesas, Simon Horman Hi Geert, As always, thank you for your review! On Friday, July 13, 2018, Geert Uytterhoeven wrote: > Please drop the "mstp", as the largest share of this patch is not about > MSTP clocks. OK. > > drivers/clk/renesas/clk-rz.c | 155 ++++++++++++++++++++++++++++++++-- > ------- > > You're adding ca. 100 new lines to an existing driver of 126 lines, most > of > which are depending on the result of detect_rz()? > So I think you're best of adding a complete new driver clk-rza2.c, > matching > against "renesas,r7s9210-cpg-clocks". > The "renesas,rz-cpg-clocks" won't be needed for RZ/A2. > And perhaps rename clk-rz.c to clk-rza1.c, and change its match string to > "renesas,r7s72100-cpg-clocks"? OK. I was just trying to reduce the number of files, but I guess it's not that big of a deal. > BTW, please use fcfe0020 as the base address for the CPG (which requires > changing the register offsets in the driver), to avoid the warning we're > seeing with "make dtbs W=1" for RZ/A1: > > Warning (unique_unit_address): /soc/watchdog@fcfe0000: duplicate > unit-address (also used in node /soc/cpg_clocks@fcfe0000) > > BTW2, I guess I can't convince you to write a modern new clock driver > using > a single register block, describing all core and module clocks in C > tables? > That would avoid making mistakes in keeping the clocks/clock-indices/ > clock-output-names properties in the mstp clock nodes in DT in sync. > It would also make your life easier if you ever decide to support software > reset using the Software Reset Control Register in the same register > block. I'll have a look before I go back and make all of these changes. Honestly, I spent the most time on trying to figure out how to detect what RZ I was running one (which goes away once I have separate .c files) and also how to turn the new dividers in the HW Manual into code that would actual work correctly. Meaning, if I had known better, I might have started with the new method from the start. Also, I don't really want to switch later to the new method and then change all my Device Trees, so if I'm going to use it, I better start now. I've got a nice long 14 hour plane ride this weekend to Japan, so I will have a look and try to understand how I can make the switch to the new method. I might come back with questions later. I assume like last time, I'll have to add support for 8-bit MSTP registers, and also my way of waiting for the clock bit to be set since RZ/A does not have the clock status bits like R-Car. > > --- a/drivers/clk/renesas/clk-mstp.c > > +++ b/drivers/clk/renesas/clk-mstp.c > > @@ -213,6 +213,9 @@ static void __init cpg_mstp_clocks_init(struct > device_node *np) > > if (of_device_is_compatible(np, "renesas,r7s72100-mstp-clocks")) > > group->width_8bit = true; > > > > + if (of_device_is_compatible(np, "renesas,r7s9210-mstp-clocks")) > > You can merge the test with the test for RZ/A1 above. OK. > > +int detect_rz(void) > > +{ > > + void __iomem *swrstcr3; > > + static int rz_device; > > + > > + if (!rz_device) { > > + swrstcr3 = ioremap_nocache(SWRSTCR3, 1); > > + BUG_ON(!swrstcr3); > > + if (ioread8(swrstcr3)) > > + rz_device = RZA1; > > + else > > + rz_device = RZA2; > > + iounmap(swrstcr3); > > + } > > + return rz_device; > > +} > > Please use the compatible value for differentiating (issue is moot with a > separate driver). I agree. Thanks, Chris ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 @ 2018-07-13 16:27 ` Chris Brandt 0 siblings, 0 replies; 11+ messages in thread From: Chris Brandt @ 2018-07-13 16:27 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Geert Uytterhoeven, linux-clk, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux-Renesas, Simon Horman SGkgR2VlcnQsDQoNCkFzIGFsd2F5cywgdGhhbmsgeW91IGZvciB5b3VyIHJldmlldyENCg0KDQpP biBGcmlkYXksIEp1bHkgMTMsIDIwMTgsIEdlZXJ0IFV5dHRlcmhvZXZlbiB3cm90ZToNCj4gUGxl YXNlIGRyb3AgdGhlICJtc3RwIiwgYXMgdGhlIGxhcmdlc3Qgc2hhcmUgb2YgdGhpcyBwYXRjaCBp cyBub3QgYWJvdXQNCj4gTVNUUCBjbG9ja3MuDQoNCk9LLg0KDQo+ID4gIGRyaXZlcnMvY2xrL3Jl bmVzYXMvY2xrLXJ6LmMgICB8IDE1NSArKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKy0t DQo+IC0tLS0tLS0NCj4gDQo+IFlvdSdyZSBhZGRpbmcgY2EuIDEwMCBuZXcgbGluZXMgdG8gYW4g ZXhpc3RpbmcgZHJpdmVyIG9mIDEyNiBsaW5lcywgbW9zdA0KPiBvZg0KPiB3aGljaCBhcmUgZGVw ZW5kaW5nIG9uIHRoZSByZXN1bHQgb2YgZGV0ZWN0X3J6KCk/DQo+IFNvIEkgdGhpbmsgeW91J3Jl IGJlc3Qgb2YgYWRkaW5nIGEgY29tcGxldGUgbmV3IGRyaXZlciBjbGstcnphMi5jLA0KPiBtYXRj aGluZw0KPiBhZ2FpbnN0ICJyZW5lc2FzLHI3czkyMTAtY3BnLWNsb2NrcyIuDQo+IFRoZSAicmVu ZXNhcyxyei1jcGctY2xvY2tzIiB3b24ndCBiZSBuZWVkZWQgZm9yIFJaL0EyLg0KPiBBbmQgcGVy aGFwcyByZW5hbWUgY2xrLXJ6LmMgdG8gY2xrLXJ6YTEuYywgYW5kIGNoYW5nZSBpdHMgbWF0Y2gg c3RyaW5nIHRvDQo+ICJyZW5lc2FzLHI3czcyMTAwLWNwZy1jbG9ja3MiPw0KDQpPSy4gSSB3YXMg anVzdCB0cnlpbmcgdG8gcmVkdWNlIHRoZSBudW1iZXIgb2YgZmlsZXMsIGJ1dCBJIGd1ZXNzIGl0 J3MgDQpub3QgdGhhdCBiaWcgb2YgYSBkZWFsLg0KDQoNCj4gQlRXLCBwbGVhc2UgdXNlIGZjZmUw MDIwIGFzIHRoZSBiYXNlIGFkZHJlc3MgZm9yIHRoZSBDUEcgKHdoaWNoIHJlcXVpcmVzDQo+IGNo YW5naW5nIHRoZSByZWdpc3RlciBvZmZzZXRzIGluIHRoZSBkcml2ZXIpLCB0byBhdm9pZCB0aGUg d2FybmluZyB3ZSdyZQ0KPiBzZWVpbmcgd2l0aCAibWFrZSBkdGJzIFc9MSIgZm9yIFJaL0ExOg0K PiANCj4gICAgIFdhcm5pbmcgKHVuaXF1ZV91bml0X2FkZHJlc3MpOiAvc29jL3dhdGNoZG9nQGZj ZmUwMDAwOiBkdXBsaWNhdGUNCj4gdW5pdC1hZGRyZXNzIChhbHNvIHVzZWQgaW4gbm9kZSAvc29j L2NwZ19jbG9ja3NAZmNmZTAwMDApDQo+IA0KPiBCVFcyLCBJIGd1ZXNzIEkgY2FuJ3QgY29udmlu Y2UgeW91IHRvIHdyaXRlIGEgbW9kZXJuIG5ldyBjbG9jayBkcml2ZXINCj4gdXNpbmcNCj4gYSBz aW5nbGUgcmVnaXN0ZXIgYmxvY2ssIGRlc2NyaWJpbmcgYWxsIGNvcmUgYW5kIG1vZHVsZSBjbG9j a3MgaW4gQw0KPiB0YWJsZXM/DQo+IFRoYXQgd291bGQgYXZvaWQgbWFraW5nIG1pc3Rha2VzIGlu IGtlZXBpbmcgdGhlIGNsb2Nrcy9jbG9jay1pbmRpY2VzLw0KPiBjbG9jay1vdXRwdXQtbmFtZXMg cHJvcGVydGllcyBpbiB0aGUgbXN0cCBjbG9jayBub2RlcyBpbiBEVCBpbiBzeW5jLg0KPiBJdCB3 b3VsZCBhbHNvIG1ha2UgeW91ciBsaWZlIGVhc2llciBpZiB5b3UgZXZlciBkZWNpZGUgdG8gc3Vw cG9ydCBzb2Z0d2FyZQ0KPiByZXNldCB1c2luZyB0aGUgU29mdHdhcmUgUmVzZXQgQ29udHJvbCBS ZWdpc3RlciBpbiB0aGUgc2FtZSByZWdpc3Rlcg0KPiBibG9jay4NCg0KSSdsbCBoYXZlIGEgbG9v ayBiZWZvcmUgSSBnbyBiYWNrIGFuZCBtYWtlIGFsbCBvZiB0aGVzZSBjaGFuZ2VzLiANCkhvbmVz dGx5LCBJIHNwZW50IHRoZSBtb3N0IHRpbWUgb24gdHJ5aW5nIHRvIGZpZ3VyZSBvdXQgaG93IHRv IGRldGVjdCB3aGF0IFJaIEkNCndhcyBydW5uaW5nIG9uZSAod2hpY2ggZ29lcyBhd2F5IG9uY2Ug SSBoYXZlIHNlcGFyYXRlIC5jIGZpbGVzKSBhbmQgDQphbHNvIGhvdyB0byB0dXJuIHRoZSBuZXcg ZGl2aWRlcnMgaW4gdGhlIEhXIE1hbnVhbCBpbnRvIGNvZGUgdGhhdCB3b3VsZCANCmFjdHVhbCB3 b3JrIGNvcnJlY3RseS4NCk1lYW5pbmcsIGlmIEkgaGFkIGtub3duIGJldHRlciwgSSBtaWdodCBo YXZlIHN0YXJ0ZWQgd2l0aCB0aGUgbmV3IG1ldGhvZA0KZnJvbSB0aGUgc3RhcnQuDQoNCkFsc28s IEkgZG9uJ3QgcmVhbGx5IHdhbnQgdG8gc3dpdGNoIGxhdGVyIHRvIHRoZSBuZXcgbWV0aG9kIGFu ZCB0aGVuIA0KY2hhbmdlIGFsbCBteSBEZXZpY2UgVHJlZXMsIHNvIGlmIEknbSBnb2luZyB0byB1 c2UgaXQsIEkgYmV0dGVyIHN0YXJ0IG5vdy4NCg0KSSd2ZSBnb3QgYSBuaWNlIGxvbmcgMTQgaG91 ciBwbGFuZSByaWRlIHRoaXMgd2Vla2VuZCB0byBKYXBhbiwgc28gSSB3aWxsDQpoYXZlIGEgbG9v ayBhbmQgdHJ5IHRvIHVuZGVyc3RhbmQgaG93IEkgY2FuIG1ha2UgdGhlIHN3aXRjaCB0byB0aGUg bmV3IA0KbWV0aG9kLiBJIG1pZ2h0IGNvbWUgYmFjayB3aXRoIHF1ZXN0aW9ucyBsYXRlci4gSSBh c3N1bWUgbGlrZSBsYXN0IHRpbWUsDQpJJ2xsIGhhdmUgdG8gYWRkIHN1cHBvcnQgZm9yIDgtYml0 IE1TVFAgcmVnaXN0ZXJzLCBhbmQgYWxzbyBteSB3YXkgb2YgDQp3YWl0aW5nIGZvciB0aGUgY2xv Y2sgYml0IHRvIGJlIHNldCBzaW5jZSBSWi9BIGRvZXMgbm90IGhhdmUgdGhlIGNsb2NrIA0Kc3Rh dHVzIGJpdHMgbGlrZSBSLUNhci4NCg0KDQo+ID4gLS0tIGEvZHJpdmVycy9jbGsvcmVuZXNhcy9j bGstbXN0cC5jDQo+ID4gKysrIGIvZHJpdmVycy9jbGsvcmVuZXNhcy9jbGstbXN0cC5jDQo+ID4g QEAgLTIxMyw2ICsyMTMsOSBAQCBzdGF0aWMgdm9pZCBfX2luaXQgY3BnX21zdHBfY2xvY2tzX2lu aXQoc3RydWN0DQo+IGRldmljZV9ub2RlICpucCkNCj4gPiAgICAgICAgIGlmIChvZl9kZXZpY2Vf aXNfY29tcGF0aWJsZShucCwgInJlbmVzYXMscjdzNzIxMDAtbXN0cC1jbG9ja3MiKSkNCj4gPiAg ICAgICAgICAgICAgICAgZ3JvdXAtPndpZHRoXzhiaXQgPSB0cnVlOw0KPiA+DQo+ID4gKyAgICAg ICBpZiAob2ZfZGV2aWNlX2lzX2NvbXBhdGlibGUobnAsICJyZW5lc2FzLHI3czkyMTAtbXN0cC1j bG9ja3MiKSkNCj4gDQo+IFlvdSBjYW4gbWVyZ2UgdGhlIHRlc3Qgd2l0aCB0aGUgdGVzdCBmb3Ig UlovQTEgYWJvdmUuDQoNCk9LLg0KDQoNCj4gPiAraW50IGRldGVjdF9yeih2b2lkKQ0KPiA+ICt7 DQo+ID4gKyAgICAgICB2b2lkIF9faW9tZW0gKnN3cnN0Y3IzOw0KPiA+ICsgICAgICAgc3RhdGlj IGludCByel9kZXZpY2U7DQo+ID4gKw0KPiA+ICsgICAgICAgaWYgKCFyel9kZXZpY2UpIHsNCj4g PiArICAgICAgICAgICAgICAgc3dyc3RjcjMgPSBpb3JlbWFwX25vY2FjaGUoU1dSU1RDUjMsIDEp Ow0KPiA+ICsgICAgICAgICAgICAgICBCVUdfT04oIXN3cnN0Y3IzKTsNCj4gPiArICAgICAgICAg ICAgICAgaWYgKGlvcmVhZDgoc3dyc3RjcjMpKQ0KPiA+ICsgICAgICAgICAgICAgICAgICAgICAg IHJ6X2RldmljZSA9IFJaQTE7DQo+ID4gKyAgICAgICAgICAgICAgIGVsc2UNCj4gPiArICAgICAg ICAgICAgICAgICAgICAgICByel9kZXZpY2UgPSBSWkEyOw0KPiA+ICsgICAgICAgICAgICAgICBp b3VubWFwKHN3cnN0Y3IzKTsNCj4gPiArICAgICAgIH0NCj4gPiArICAgICAgIHJldHVybiByel9k ZXZpY2U7DQo+ID4gK30NCj4gDQo+IFBsZWFzZSB1c2UgdGhlIGNvbXBhdGlibGUgdmFsdWUgZm9y IGRpZmZlcmVudGlhdGluZyAoaXNzdWUgaXMgbW9vdCB3aXRoIGENCj4gc2VwYXJhdGUgZHJpdmVy KS4NCg0KSSBhZ3JlZS4NCg0KDQpUaGFua3MsDQpDaHJpcw0KDQo= ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 @ 2018-07-13 16:27 ` Chris Brandt 0 siblings, 0 replies; 11+ messages in thread From: Chris Brandt @ 2018-07-13 16:27 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Geert Uytterhoeven, linux-clk, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux-Renesas, Simon Horman Hi Geert, As always, thank you for your review! On Friday, July 13, 2018, Geert Uytterhoeven wrote: > Please drop the "mstp", as the largest share of this patch is not about > MSTP clocks. OK. > > drivers/clk/renesas/clk-rz.c | 155 ++++++++++++++++++++++++++++++++-- > ------- > > You're adding ca. 100 new lines to an existing driver of 126 lines, most > of > which are depending on the result of detect_rz()? > So I think you're best of adding a complete new driver clk-rza2.c, > matching > against "renesas,r7s9210-cpg-clocks". > The "renesas,rz-cpg-clocks" won't be needed for RZ/A2. > And perhaps rename clk-rz.c to clk-rza1.c, and change its match string to > "renesas,r7s72100-cpg-clocks"? OK. I was just trying to reduce the number of files, but I guess it's not that big of a deal. > BTW, please use fcfe0020 as the base address for the CPG (which requires > changing the register offsets in the driver), to avoid the warning we're > seeing with "make dtbs W=1" for RZ/A1: > > Warning (unique_unit_address): /soc/watchdog@fcfe0000: duplicate > unit-address (also used in node /soc/cpg_clocks@fcfe0000) > > BTW2, I guess I can't convince you to write a modern new clock driver > using > a single register block, describing all core and module clocks in C > tables? > That would avoid making mistakes in keeping the clocks/clock-indices/ > clock-output-names properties in the mstp clock nodes in DT in sync. > It would also make your life easier if you ever decide to support software > reset using the Software Reset Control Register in the same register > block. I'll have a look before I go back and make all of these changes. Honestly, I spent the most time on trying to figure out how to detect what RZ I was running one (which goes away once I have separate .c files) and also how to turn the new dividers in the HW Manual into code that would actual work correctly. Meaning, if I had known better, I might have started with the new method from the start. Also, I don't really want to switch later to the new method and then change all my Device Trees, so if I'm going to use it, I better start now. I've got a nice long 14 hour plane ride this weekend to Japan, so I will have a look and try to understand how I can make the switch to the new method. I might come back with questions later. I assume like last time, I'll have to add support for 8-bit MSTP registers, and also my way of waiting for the clock bit to be set since RZ/A does not have the clock status bits like R-Car. > > --- a/drivers/clk/renesas/clk-mstp.c > > +++ b/drivers/clk/renesas/clk-mstp.c > > @@ -213,6 +213,9 @@ static void __init cpg_mstp_clocks_init(struct > device_node *np) > > if (of_device_is_compatible(np, "renesas,r7s72100-mstp-clocks")) > > group->width_8bit = true; > > > > + if (of_device_is_compatible(np, "renesas,r7s9210-mstp-clocks")) > > You can merge the test with the test for RZ/A1 above. OK. > > +int detect_rz(void) > > +{ > > + void __iomem *swrstcr3; > > + static int rz_device; > > + > > + if (!rz_device) { > > + swrstcr3 = ioremap_nocache(SWRSTCR3, 1); > > + BUG_ON(!swrstcr3); > > + if (ioread8(swrstcr3)) > > + rz_device = RZA1; > > + else > > + rz_device = RZA2; > > + iounmap(swrstcr3); > > + } > > + return rz_device; > > +} > > Please use the compatible value for differentiating (issue is moot with a > separate driver). I agree. Thanks, Chris ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] clk: renesas: mstp: Document R7S9210 support 2018-07-11 17:03 [PATCH 0/2] clk: renesas: mstp: Add support for RZ/A2 Chris Brandt 2018-07-11 17:03 ` [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 Chris Brandt @ 2018-07-11 17:03 ` Chris Brandt 2018-07-13 9:32 ` Geert Uytterhoeven ` (2 more replies) 2018-07-20 8:10 ` [PATCH 0/2] clk: renesas: mstp: Add support for RZ/A2 Geert Uytterhoeven 2 siblings, 3 replies; 11+ messages in thread From: Chris Brandt @ 2018-07-11 17:03 UTC (permalink / raw) To: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Geert Uytterhoeven Cc: linux-clk, devicetree, linux-renesas-soc, Simon Horman, Chris Brandt Renesas R7S9210 SoC also has the CPG MSTP clocks. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt index da578ebdda28..bb470d9bc07f 100644 --- a/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt @@ -11,6 +11,7 @@ Required Properties: - compatible: Must be one of the following - "renesas,r7s72100-mstp-clocks" for R7S72100 (RZ) MSTP gate clocks + - "renesas,r7s9210-mstp-clocks" for R7S9210 (RZ A2) MSTP gate clocks - "renesas,r8a73a4-mstp-clocks" for R8A73A4 (R-Mobile APE6) MSTP gate clocks - "renesas,r8a7740-mstp-clocks" for R8A7740 (R-Mobile A1) MSTP gate clocks - "renesas,r8a7778-mstp-clocks" for R8A7778 (R-Car M1) MSTP gate clocks -- 2.16.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] clk: renesas: mstp: Document R7S9210 support 2018-07-11 17:03 ` [PATCH 2/2] clk: renesas: mstp: Document R7S9210 support Chris Brandt @ 2018-07-13 9:32 ` Geert Uytterhoeven 2018-07-20 14:20 ` Rob Herring 2018-09-05 13:37 ` Geert Uytterhoeven 2 siblings, 0 replies; 11+ messages in thread From: Geert Uytterhoeven @ 2018-07-13 9:32 UTC (permalink / raw) To: Chris Brandt Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Geert Uytterhoeven, linux-clk, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux-Renesas, Simon Horman On Wed, Jul 11, 2018 at 7:03 PM Chris Brandt <chris.brandt@renesas.com> wrote: > Renesas R7S9210 SoC also has the CPG MSTP clocks. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] clk: renesas: mstp: Document R7S9210 support 2018-07-11 17:03 ` [PATCH 2/2] clk: renesas: mstp: Document R7S9210 support Chris Brandt 2018-07-13 9:32 ` Geert Uytterhoeven @ 2018-07-20 14:20 ` Rob Herring 2018-09-05 13:37 ` Geert Uytterhoeven 2 siblings, 0 replies; 11+ messages in thread From: Rob Herring @ 2018-07-20 14:20 UTC (permalink / raw) To: Chris Brandt Cc: Michael Turquette, Stephen Boyd, Mark Rutland, Geert Uytterhoeven, linux-clk, devicetree, linux-renesas-soc, Simon Horman On Wed, Jul 11, 2018 at 12:03:13PM -0500, Chris Brandt wrote: > Renesas R7S9210 SoC also has the CPG MSTP clocks. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > --- > Documentation/devicetree/bindings/clock/renesas,cpg-mstp-clocks.txt | 1 + > 1 file changed, 1 insertion(+) Acked-by: Rob Herring <robh@kernel.org> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] clk: renesas: mstp: Document R7S9210 support 2018-07-11 17:03 ` [PATCH 2/2] clk: renesas: mstp: Document R7S9210 support Chris Brandt 2018-07-13 9:32 ` Geert Uytterhoeven 2018-07-20 14:20 ` Rob Herring @ 2018-09-05 13:37 ` Geert Uytterhoeven 2 siblings, 0 replies; 11+ messages in thread From: Geert Uytterhoeven @ 2018-09-05 13:37 UTC (permalink / raw) To: Chris Brandt Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Geert Uytterhoeven, linux-clk, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux-Renesas, Simon Horman Hi Chris, On Wed, Jul 11, 2018 at 7:03 PM Chris Brandt <chris.brandt@renesas.com> wrote: > > Renesas R7S9210 SoC also has the CPG MSTP clocks. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> FTR, I consider this superseded by "[PATCH v3] clk: renesas: cpg-mssr: Add R7S9210 support" (https://www.spinics.net/lists/linux-clk/msg29669.html). Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] clk: renesas: mstp: Add support for RZ/A2 2018-07-11 17:03 [PATCH 0/2] clk: renesas: mstp: Add support for RZ/A2 Chris Brandt 2018-07-11 17:03 ` [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 Chris Brandt 2018-07-11 17:03 ` [PATCH 2/2] clk: renesas: mstp: Document R7S9210 support Chris Brandt @ 2018-07-20 8:10 ` Geert Uytterhoeven 2 siblings, 0 replies; 11+ messages in thread From: Geert Uytterhoeven @ 2018-07-20 8:10 UTC (permalink / raw) To: Chris Brandt Cc: Michael Turquette, Stephen Boyd, Rob Herring, Mark Rutland, Geert Uytterhoeven, linux-clk, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, Linux-Renesas, Simon Horman Hi Chris, (Oops, I wrote this response, but forgot to send it) On Wed, Jul 11, 2018 at 7:03 PM Chris Brandt <chris.brandt@renesas.com> wrote: > Add support for RZ/A2 series. > The clock HW is similar to RZ/A1, but with different dividers > and additional clocks sources. Thanks for your series! > Chris Brandt (2): > clk: renesas: mstp: Add support for r7s9210 > clk: renesas: mstp: Document R7S9210 support > > .../bindings/clock/renesas,cpg-mstp-clocks.txt | 1 + > drivers/clk/renesas/Kconfig | 5 + > drivers/clk/renesas/Makefile | 1 + > drivers/clk/renesas/clk-mstp.c | 3 + > drivers/clk/renesas/clk-rz.c | 155 ++++++++++++++++----- > 5 files changed, 131 insertions(+), 34 deletions(-) No patch to update Documentation/devicetree/bindings/clock/renesas,rz-cpg-clocks.txt? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-09-05 13:37 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-11 17:03 [PATCH 0/2] clk: renesas: mstp: Add support for RZ/A2 Chris Brandt 2018-07-11 17:03 ` [PATCH 1/2] clk: renesas: mstp: Add support for r7s9210 Chris Brandt 2018-07-13 9:58 ` Geert Uytterhoeven 2018-07-13 16:27 ` Chris Brandt 2018-07-13 16:27 ` Chris Brandt 2018-07-13 16:27 ` Chris Brandt 2018-07-11 17:03 ` [PATCH 2/2] clk: renesas: mstp: Document R7S9210 support Chris Brandt 2018-07-13 9:32 ` Geert Uytterhoeven 2018-07-20 14:20 ` Rob Herring 2018-09-05 13:37 ` Geert Uytterhoeven 2018-07-20 8:10 ` [PATCH 0/2] clk: renesas: mstp: Add support for RZ/A2 Geert Uytterhoeven
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.