| Message ID | 20180829132841.64787-1-chris.brandt@renesas.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v3] clk: renesas: cpg-mssr: Add R7S9210 support | expand |
Hi Geert, On Wednesday, August 29, 2018, Chris Brandt wrote: > > Add support for the R7S9210 (RZ/A2) Clock Pulse Generator and Module > Standby. I ran into an issue with this driver. The issue is that the registers needed for this driver are scattered throughout the address space. I can't just reserve one big block of addresses that cover everything because then I can't reserve any of the addresses that sit in the middle. For example, my watchdog timer driver doesn't work with this driver because it cannot allocate its register region. So, I think I need to rework this driver to add the ability to add multiple address regions. Chris
Hi Chris, On Wed, Sep 5, 2018 at 4:12 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Wednesday, August 29, 2018, Chris Brandt wrote: > > Add support for the R7S9210 (RZ/A2) Clock Pulse Generator and Module > > Standby. > > I ran into an issue with this driver. > > The issue is that the registers needed for this driver are scattered > throughout the address space. > > I can't just reserve one big block of addresses that cover everything > because then I can't reserve any of the addresses that sit in the middle. Yeah, this address range is kind of a "syscon" block. > For example, my watchdog timer driver doesn't work with this driver > because it cannot allocate its register region. > > So, I think I need to rework this driver to add the ability to add > multiple address regions. Your main driver for this block (clock?) can register the watchdog. Either directly with the watchdog subsystem, or by registering an "rza_wdt" platform device. One more vote for a separate rza2-cpg-stb-wdt driver?... Gr{oetje,eeting}s, Geert
Hi Geert, On Wednesday, September 05, 2018 1, Geert Uytterhoeven wrote: > > So, I think I need to rework this driver to add the ability to add > > multiple address regions. > > Your main driver for this block (clock?) can register the watchdog. > Either directly with the watchdog subsystem, or by registering an > "rza_wdt" platform device. It would seem strange that the clock driver would register the watchdog timer for no other reason other than where the chip designer happen to put the register addresses. Now that I look at it: CPG is at FCFE_0010 - FCFE_0104 MSTP is at FCFE_0400 - FCFE_0464 Nothing sits in between CPG and MSTP. In the hardware manual, there is other 'random other stuff' that they put in that chapter that is located in multiple address places, but that doesn't mean I have to map them all in this driver. I simply changed the DTS to just map CPG and MSTP and now everything is fine. My watchdog driver is happy and this CPG/MSTP driver works the same. So....I guess I didn't really have an issue after all. Chris
Hi Chris, On Wed, Sep 5, 2018 at 5:02 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Wednesday, September 05, 2018 1, Geert Uytterhoeven wrote: > > > So, I think I need to rework this driver to add the ability to add > > > multiple address regions. > > > > Your main driver for this block (clock?) can register the watchdog. > > Either directly with the watchdog subsystem, or by registering an > > "rza_wdt" platform device. > > It would seem strange that the clock driver would register the watchdog > timer for no other reason other than where the chip designer happen to > put the register addresses. That not that uncommon for "syscon" drivers... > Now that I look at it: > CPG is at FCFE_0010 - FCFE_0104 > MSTP is at FCFE_0400 - FCFE_0464 > > Nothing sits in between CPG and MSTP. OK. > In the hardware manual, there is other 'random other stuff' that they > put in that chapter that is located in multiple address places, but that > doesn't mean I have to map them all in this driver. OK. > I simply changed the DTS to just map CPG and MSTP and now everything is > fine. My watchdog driver is happy and this CPG/MSTP driver works the > same. Good, I will resume review of the patch in $subject ;-) > So....I guess I didn't really have an issue after all. You do want to: 1. Document the two register ranges in the DT bindings, 2. Update the driver to map both ranges on RZ/A2. Right now it works by luck, as ioremap()'s granularity is PAGE_SIZE. Gr{oetje,eeting}s, Geert
Hi Geert, On Wednesday, September 05, 2018 1, Geert Uytterhoeven wrote: > > So....I guess I didn't really have an issue after all. > > You do want to: > 1. Document the two register ranges in the DT bindings, > 2. Update the driver to map both ranges on RZ/A2. The driver does not need to be updated. Given this mapping: cpg: clock-controller@fcfe0020 { compatible = "renesas,r7s9210-cpg-mssr"; reg = <0xfcfe0010 0x455>; /* FCFE0010 - FCFE0465 */ clocks = <&extal_clk>; clock-names = "extal"; #clock-cells = <2>; #power-domain-cells = <0>; #reset-cells = <1>; }; This covers all the CPG, MSTP, power down and reset registers. So I'm just going to map them all at once. And, you can see that some CPG and MSTP register are mixed together. Here is the list of registers in address order: FRQCR FCFE_0010 (CPG) STBCR1 FCFE_0020 (MSTP) STBCR2 FCFE_0024 (MSTP) CKIOSEL FCFE_0100 (CPG) SCLKSEL FCFE_0104 (CPG) STBCR3 FCFE_0420 (MSTP) STBCR4 FCFE_0424 (MSTP) STBCR5 FCFE_0428 (MSTP) STBCR6 FCFE_042C (MSTP) STBCR7 FCFE_0430 (MSTP) STBCR8 FCFE_0434 (MSTP) STBCR9 FCFE_0438 (MSTP) STBCR10 FCFE_043C (MSTP) SWRSTCR1 FCFE_0460 (RESET) SWRSTCR2 FCFE_0464 (RESET) etc... So this should be all one memory block in this driver. When you get to FCFF_C000+, that's where the 'specialty' registers are. I think FCFF_C000+ should be a separate block, maybe in a separate driver. Chris
Hi Chris, On Wed, Aug 29, 2018 at 3:29 PM Chris Brandt <chris.brandt@renesas.com> wrote: > Add support for the R7S9210 (RZ/A2) Clock Pulse Generator and Module > Standby. Thanks for your patch! > The Module Standby HW in the RZ/A series is very close to R-Car HW, except > for how the registers are laid out. > The MSTP registers are only 8-bits wide, there is no status registers > (MSTPST), and the register offsets are a little different. Since the RZ/A MSTPSR > hardware manuals refer to these registers as the Standby Control Registers, > we'll use that name to distinguish the RZ/A type for the R-Car type. from the R-Car type. > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > --- /dev/null > +++ b/drivers/clk/renesas/r7s9210-cpg-mssr.c > @@ -0,0 +1,193 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * R7S9210 Clock Pulse Generator / Module Standby > + * > + * Based on r8a7795-cpg-mssr.c > + * > + * Copyright (C) 2018 Chris Brandt > + * Copyright (C) 2018 Renesas Electronics Corp. > + * > + */ > + > +#include <linux/clk.h> > +#include <linux/clk-provider.h> > +#include <dt-bindings/clock/r7s9210-cpg-mssr.h> > +#include "renesas-cpg-mssr.h" > + > +#define CPG_FRQCR 0x00 > +#define CPG_CKIOSEL 0xF0 > +#define CPG_SCLKSEL 0xF4 The last two are unused? > + > +#define PORTL_PIDR 0xFCFFE074 Unused? > +static u8 cpg_mode; > + > +/* Internal Clock ratio table */ > +static const unsigned int ratio_tab[5][5] = { > + /* I, G, B, P1, P0 */ Use a struct instead? struct { unsigned int i; unsigned int g; unsigned int ib unsigned int p1; unsigned int p0; } ratio_tab[5] = { ... } > + { 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 */ The P0 divider is fixed to 32, so you can remove it from the table? > + }; > + > +enum rz_clk_types { > + CLK_TYPE_RZA_MAIN = CLK_TYPE_CUSTOM, > + CLK_TYPE_RZA_PLL, > +}; > + > +enum clk_ids { > + /* Core Clock Outputs exported to DT */ > + LAST_DT_CORE_CLK = R7S9210_CLK_P0, > + > + /* External Input Clocks */ > + CLK_EXTAL, > + > + /* Internal Core Clocks */ > + CLK_MAIN, > + CLK_PLL, > + CLK_I, > + CLK_G, > + CLK_B, > + CLK_P1, > + CLK_P1C, > + CLK_P0, The last six are not used and can be removed (the driver uses R7S9210_CLK_* instead). > + > + /* Module Clocks */ > + MOD_CLK_BASE > +}; > + > +static struct cpg_core_clk r7s9210_core_clks[] = { > + /* External Clock Inputs */ > + DEF_INPUT("extal", CLK_EXTAL), > + > + /* Internal Core Clocks */ > + DEF_BASE(".main", CLK_MAIN, CLK_TYPE_RZA_MAIN, CLK_EXTAL), > + DEF_BASE(".pll", CLK_PLL, CLK_TYPE_RZA_PLL, CLK_MAIN), > + > + /* Core Clock Outputs */ > + DEF_FIXED("i", R7S9210_CLK_I, CLK_PLL, 2, 1), > + DEF_FIXED("g", R7S9210_CLK_G, CLK_PLL, 4, 1), > + DEF_FIXED("b", R7S9210_CLK_B, CLK_PLL, 8, 1), > + DEF_FIXED("p1", R7S9210_CLK_P1, CLK_PLL, 16, 1), > + DEF_FIXED("p1c", R7S9210_CLK_P1C, CLK_PLL, 16, 1), > + DEF_FIXED("p0", R7S9210_CLK_P0, CLK_PLL, 32, 1), > +}; > + > +static const struct mssr_mod_clk r7s9210_mod_clks[] __initconst = { > + DEF_MOD_STB("ostm0", 36, R7S9210_CLK_P1C), > + DEF_MOD_STB("ostm1", 35, R7S9210_CLK_P1C), > + DEF_MOD_STB("ostm2", 34, R7S9210_CLK_P1C), I think the table is easier to read if you sort by MSTP number. > + > + DEF_MOD_STB("scif0", 47, R7S9210_CLK_P1C), > + DEF_MOD_STB("scif1", 46, R7S9210_CLK_P1C), > + DEF_MOD_STB("scif2", 45, R7S9210_CLK_P1C), > + DEF_MOD_STB("scif3", 44, R7S9210_CLK_P1C), > + DEF_MOD_STB("scif4", 43, R7S9210_CLK_P1C), > + > + DEF_MOD_STB("ether0", 65, R7S9210_CLK_B), > + DEF_MOD_STB("ether1", 64, R7S9210_CLK_B), > + > + DEF_MOD_STB("i2c0", 87, R7S9210_CLK_P1), > + DEF_MOD_STB("i2c1", 86, R7S9210_CLK_P1), > + DEF_MOD_STB("i2c2", 85, R7S9210_CLK_P1), > + DEF_MOD_STB("i2c3", 84, R7S9210_CLK_P1), > + > +}; > + > +struct clk * __init rza2_cpg_clk_register(struct device *dev, > + const struct cpg_core_clk *core, const struct cpg_mssr_info *info, > + struct clk **clks, void __iomem *base, > + struct raw_notifier_head *notifiers) > +{ > + const struct clk *parent; > + unsigned int mult = 1; > + unsigned int div = 1; > + u16 frqcr; > + u8 index; > + int i; > + > + parent = clks[core->parent]; > + if (IS_ERR(parent)) > + return ERR_CAST(parent); > + > + switch (core->id) { > + case CLK_MAIN: > + break; > + > + case CLK_PLL: > + if (cpg_mode) > + mult = 44; /* Divider 1 is 1/2 */ > + else > + mult = 88; /* Divider 1 is 1 */ > + break; > + > + default: > + return ERR_PTR(-EINVAL); > + } > + > + /* Adjust the dividers based on the current FRQCR setting */ > + if (core->id == CLK_MAIN) { > + > + /* If EXTAL is above 12MHz, then we know it is Mode 1 */ > + if (clk_get_rate((struct clk *)parent) > 12000000) Why the cast? > + cpg_mode = 1; > + > + frqcr = clk_readl(base + CPG_FRQCR) & 0xFFF; > + if (frqcr == 0x012) > + index = 0; > + else if (frqcr == 0x112) > + index = 1; > + else if (frqcr == 0x212) > + index = 2; > + else if (frqcr == 0x322) > + index = 3; > + else if (frqcr == 0x333) > + index = 4; > + else > + BUG_ON(1); /* Illegal FRQCR value */ > + > + for (i = 0; i < ARRAY_SIZE(r7s9210_core_clks); i++) { > + switch (r7s9210_core_clks[i].id) { > + case R7S9210_CLK_I: > + r7s9210_core_clks[i].div = ratio_tab[index][0]; > + break; > + case R7S9210_CLK_G: > + r7s9210_core_clks[i].div = ratio_tab[index][1]; > + break; > + case R7S9210_CLK_B: > + r7s9210_core_clks[i].div = ratio_tab[index][2]; > + break; > + case R7S9210_CLK_P1: > + case R7S9210_CLK_P1C: > + r7s9210_core_clks[i].div = ratio_tab[index][3]; > + break; > + case R7S9210_CLK_P0: > + r7s9210_core_clks[i].div = ratio_tab[index][4]; > + break; > + } > + } > + } > + > + return clk_register_fixed_factor(NULL, core->name, > + __clk_get_name(parent), 0, mult, div); > +} > + > +const struct cpg_mssr_info r7s9210_cpg_mssr_info __initconst = { > + /* Core Clocks */ > + .core_clks = r7s9210_core_clks, > + .num_core_clks = ARRAY_SIZE(r7s9210_core_clks), > + .last_dt_core_clk = LAST_DT_CORE_CLK, > + .num_total_core_clks = MOD_CLK_BASE, > + > + /* Module Clocks */ > + .mod_clks = r7s9210_mod_clks, > + .num_mod_clks = ARRAY_SIZE(r7s9210_mod_clks), > + .num_hw_mod_clks = 11 * 32, /* includes STBCR0/1/2 which don't exist */ According to the HW manual, STBCR1/2 do not exist? > + > + /* Callbacks */ > + .cpg_clk_register = rza2_cpg_clk_register, > + > + /* RZ/A2 has Standby Control Registers */ > + .stbyctrl = true, > +}; > --- a/drivers/clk/renesas/renesas-cpg-mssr.c > +++ b/drivers/clk/renesas/renesas-cpg-mssr.c > @@ -73,6 +73,17 @@ static const u16 smstpcr[] = { > > #define SMSTPCR(i) smstpcr[i] > > +/* > + * Standby Control Register offsets (RZ/A) > + * Base address is FRQCR register > + */ > + > +static const u16 stbcr[] = { > + 0x000, 0x000, 0x014, 0x410, 0x414, 0x418, 0x41C, 0x420, stbcr[1] should be 0x010? > + 0x424, 0x428, 0x42C, 0x430, 0x434, 0x460, The last 3 don't exist? > +}; > @@ -240,8 +270,14 @@ struct clk *cpg_mssr_clk_src_twocell_get(struct of_phandle_args *clkspec, > > case CPG_MOD: > type = "module"; > - idx = MOD_CLK_PACK(clkidx); > - if (clkidx % 100 > 31 || idx >= priv->num_mod_clks) { > + if (priv->stbyctrl) { > + idx = MOD_CLK_PACK_10(clkidx); > + range_check = 7 - (clkidx % 10); > + } else { > + idx = MOD_CLK_PACK_10(clkidx); MOD_CLK_PACK() > + range_check = 31 - (clkidx % 100); > + } > + if (range_check < 0 || idx >= priv->num_mod_clks) { > dev_err(dev, "Invalid %s clock index %u\n", type, > clkidx); > return ERR_PTR(-EINVAL); > @@ -740,6 +776,12 @@ static const struct of_device_id cpg_mssr_match[] = { > .compatible = "renesas,r8a77995-cpg-mssr", > .data = &r8a77995_cpg_mssr_info, > }, > +#endif > +#ifdef CONFIG_CLK_R7S9210 > + { > + .compatible = "renesas,r7s9210-cpg-mssr", > + .data = &r7s9210_cpg_mssr_info, > + }, Please preserve sort order. > --- a/drivers/clk/renesas/renesas-cpg-mssr.h > +++ b/drivers/clk/renesas/renesas-cpg-mssr.h > @@ -78,6 +78,13 @@ struct mssr_mod_clk { > #define DEF_MOD(_name, _mod, _parent...) \ > { .name = _name, .id = MOD_CLK_ID(_mod), .parent = _parent } > > +/* Convert from sparse base-10 to packed index space */ > +#define MOD_CLK_PACK_10(x) ((x / 10) * 32 + (x % 10)) "PACK" is a bit of a misnomer, as it no longer packs, but expands from base-10 to base-32 ;-) > @@ -111,6 +122,7 @@ struct cpg_mssr_info { > unsigned int num_core_clks; > unsigned int last_dt_core_clk; > unsigned int num_total_core_clks; > + bool stbyctrl; > > /* Module Clocks */ > const struct mssr_mod_clk *mod_clks; > @@ -149,6 +161,7 @@ extern const struct cpg_mssr_info r8a77970_cpg_mssr_info; > extern const struct cpg_mssr_info r8a77980_cpg_mssr_info; > extern const struct cpg_mssr_info r8a77990_cpg_mssr_info; > extern const struct cpg_mssr_info r8a77995_cpg_mssr_info; > +extern const struct cpg_mssr_info r7s9210_cpg_mssr_info; Please preserve sort order. > --- /dev/null > +++ b/include/dt-bindings/clock/r7s9210-cpg-mssr.h > @@ -0,0 +1,21 @@ > +/* SPDX-License-Identifier: GPL-2.0 > + * > + * Copyright (C) 2018 Renesas Electronics Corp. > + * > + */ > + > +#ifndef __DT_BINDINGS_CLOCK_R7S9210_CPG_MSSR_H__ > +#define __DT_BINDINGS_CLOCK_R7S9210_CPG_MSSR_H__ > + > +#include <dt-bindings/clock/renesas-cpg-mssr.h> > + > +/* R7S9210 CPG Core Clocks */ > +#define R7S9210_CLK_PLL 0 Should that be an internal clock, not referred to from DT? There's already the internal CLK_PLL clock. > +#define R7S9210_CLK_I 1 > +#define R7S9210_CLK_G 2 > +#define R7S9210_CLK_B 3 > +#define R7S9210_CLK_P1 4 > +#define R7S9210_CLK_P1C 5 > +#define R7S9210_CLK_P0 6 The comment in Figure 6.1 suggests there's also P0C, but that may be a mistake, as I can find no other references to it? What about other clocks: BSCCLK, OCTCLK, HYPCLK, and SPICLK? 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
Hi Geert, Thanks for your review. On Thursday, September 06, 2018, Geert Uytterhoeven wrote: > > +#define CPG_FRQCR 0x00 > > +#define CPG_CKIOSEL 0xF0 > > +#define CPG_SCLKSEL 0xF4 > > The last two are unused? In this driver they are not. I can remove them. > > + > > +#define PORTL_PIDR 0xFCFFE074 > > Unused? Oops. That was left over from when I first was reading the port pin to find out the mode. But then I realize I could get the info from DT. > > > +static u8 cpg_mode; > > + > > +/* Internal Clock ratio table */ > > +static const unsigned int ratio_tab[5][5] = { > > + /* I, G, B, P1, P0 */ > > Use a struct instead? > > struct { > unsigned int i; > unsigned int g; > unsigned int ib > unsigned int p1; > unsigned int p0; > } ratio_tab[5] = { ... } That's a good idea. Thanks. > > > + { 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 */ > > The P0 divider is fixed to 32, so you can remove it from the table? OK, I can do that. I was just doing it to be consistent. > > + /* Internal Core Clocks */ > > + CLK_MAIN, > > + CLK_PLL, > > + CLK_I, > > + CLK_G, > > + CLK_B, > > + CLK_P1, > > + CLK_P1C, > > + CLK_P0, > > The last six are not used and can be removed > (the driver uses R7S9210_CLK_* instead). OK. > > +static const struct mssr_mod_clk r7s9210_mod_clks[] __initconst = { > > + DEF_MOD_STB("ostm0", 36, R7S9210_CLK_P1C), > > + DEF_MOD_STB("ostm1", 35, R7S9210_CLK_P1C), > > + DEF_MOD_STB("ostm2", 34, R7S9210_CLK_P1C), > > I think the table is easier to read if you sort by MSTP number. OK. I will switch them all around. > > + /* Adjust the dividers based on the current FRQCR setting */ > > + if (core->id == CLK_MAIN) { > > + > > + /* If EXTAL is above 12MHz, then we know it is Mode 1 */ > > + if (clk_get_rate((struct clk *)parent) > 12000000) > > Why the cast? I was getting compiler warning because parent was const. const struct clk *parent; ../drivers/clk/renesas/r7s9210-cpg-mssr.c:144:20: warning: passing argument 1 of ‘clk_get_rate’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] if (clk_get_rate(parent) > 12000000) ^ In file included from ../drivers/clk/renesas/r7s9210-cpg-mssr.c:12:0: ../include/linux/clk.h:463:15: note: expected ‘struct clk *’ but argument is of type ‘const struct clk *’ unsigned long clk_get_rate(struct clk *clk); ^ make[1]: Leaving directory '/home/renesas/tools/upstream/renesas-drivers/.out_rza2m' I can remove const, then I don't need the cast anymore. > > + /* Module Clocks */ > > + .mod_clks = r7s9210_mod_clks, > > + .num_mod_clks = ARRAY_SIZE(r7s9210_mod_clks), > > + .num_hw_mod_clks = 11 * 32, /* includes STBCR0/1/2 which don't > exist */ > > According to the HW manual, STBCR1/2 do not exist? The problem is that there are registers named "STBCR1" and "STBCR2", but they are not pure MSTP registers, and they sit at a different address location. Would it be better if I say the MSTP registers start at STBCR3, and just subtract "30" from the numbers in the device tree? > > +static const u16 stbcr[] = { > > + 0x000, 0x000, 0x014, 0x410, 0x414, 0x418, 0x41C, 0x420, > > stbcr[1] should be 0x010? Technically, stbcr[0] = 0x10, stbcr[1] = 0x14, I was just thinking that I would never be access it anyway since they are not really MSTP registers. > > > + 0x424, 0x428, 0x42C, 0x430, 0x434, 0x460, > > The last 3 don't exist? Opps, you're right! They are left over from when I change the table around. Thanks. > > + } else { > > + idx = MOD_CLK_PACK_10(clkidx); > > MOD_CLK_PACK() Good find! I would have broken existing drivers! > > +#ifdef CONFIG_CLK_R7S9210 > > + { > > + .compatible = "renesas,r7s9210-cpg-mssr", > > + .data = &r7s9210_cpg_mssr_info, > > + }, > > Please preserve sort order. > > extern const struct cpg_mssr_info r8a77980_cpg_mssr_info; > > extern const struct cpg_mssr_info r8a77990_cpg_mssr_info; > > extern const struct cpg_mssr_info r8a77995_cpg_mssr_info; > > +extern const struct cpg_mssr_info r7s9210_cpg_mssr_info; > > Please preserve sort order. OK > > +/* R7S9210 CPG Core Clocks */ > > +#define R7S9210_CLK_PLL 0 > > Should that be an internal clock, not referred to from DT? > There's already the internal CLK_PLL clock. OK, I see what you mean. I will leave it out of this header file. > > +#define R7S9210_CLK_I 1 > > +#define R7S9210_CLK_G 2 > > +#define R7S9210_CLK_B 3 > > +#define R7S9210_CLK_P1 4 > > +#define R7S9210_CLK_P1C 5 > > +#define R7S9210_CLK_P0 6 > > The comment in Figure 6.1 suggests there's also P0C, but that may be a > mistake, as I can find no other references to it? Funny, I didn't see that in there before. Like you said, it's probably just a mistake. I'll point that out to the device team. > What about other clocks: BSCCLK, OCTCLK, HYPCLK, and SPICLK? At this time, I'm considering them 'out of scope' from this driver as they really need to be set up early in device boot (ie, u-boot). Unless you were just thinking of adding them as #defines, but never really using them. Chris
Hi Chris, On Thu, Sep 6, 2018 at 4:31 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > > What about other clocks: BSCCLK, OCTCLK, HYPCLK, and SPICLK? > > At this time, I'm considering them 'out of scope' from this driver as > they really need to be set up early in device boot (ie, u-boot). > Unless you were just thinking of adding them as #defines, but never > really using them. OK, you can always add them later, when the need arises. include/dt-bindings/clock/*.h is append-only. Gr{oetje,eeting}s, Geert
diff --git a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt index 42d0f83d812b..012416b33d2d 100644 --- a/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt +++ b/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt @@ -13,6 +13,7 @@ They provide the following functionalities: Required Properties: - compatible: Must be one of: + - "renesas,r7s9210-cpg-mssr" for the r7s9210 SoC (RZ/A2) - "renesas,r8a7743-cpg-mssr" for the r8a7743 SoC (RZ/G1M) - "renesas,r8a7745-cpg-mssr" for the r8a7745 SoC (RZ/G1E) - "renesas,r8a77470-cpg-mssr" for the r8a77470 SoC (RZ/G1C) @@ -38,7 +39,7 @@ Required Properties: - clock-names: List of external parent clock names. Valid names are: - "extal" (r8a7743, r8a7745, r8a77470, r8a774a1, r8a7790, r8a7791, r8a7792, r8a7793, r8a7794, r8a7795, r8a7796, r8a77965, - r8a77970, r8a77980, r8a77990, r8a77995) + r8a77970, r8a77980, r8a77990, r8a77995, r7s9210) - "extalr" (r8a774a1, r8a7795, r8a7796, r8a77965, r8a77970, r8a77980) - "usb_extal" (r8a7743, r8a7745, r8a77470, r8a7790, r8a7791, r8a7793, r8a7794) diff --git a/drivers/clk/renesas/Kconfig b/drivers/clk/renesas/Kconfig index f998a7333acb..2edcb1bdb487 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_R7S9210 if ARCH_R7S9210 select CLK_R8A73A4 if ARCH_R8A73A4 select CLK_R8A7740 if ARCH_R8A7740 select CLK_R8A7743 if ARCH_R8A7743 @@ -46,6 +47,10 @@ config CLK_RZA1 bool "RZ/A1H clock support" if COMPILE_TEST select CLK_RENESAS_CPG_MSTP +config CLK_R7S9210 + bool "RZ/A2 clock support" if COMPILE_TEST + select CLK_RENESAS_CPG_MSSR + 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 71d4cafe15c0..dbbfd0b0742b 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_R7S9210) += r7s9210-cpg-mssr.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/r7s9210-cpg-mssr.c b/drivers/clk/renesas/r7s9210-cpg-mssr.c new file mode 100644 index 000000000000..faba66cea7d4 --- /dev/null +++ b/drivers/clk/renesas/r7s9210-cpg-mssr.c @@ -0,0 +1,193 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * R7S9210 Clock Pulse Generator / Module Standby + * + * Based on r8a7795-cpg-mssr.c + * + * Copyright (C) 2018 Chris Brandt + * Copyright (C) 2018 Renesas Electronics Corp. + * + */ + +#include <linux/clk.h> +#include <linux/clk-provider.h> +#include <dt-bindings/clock/r7s9210-cpg-mssr.h> +#include "renesas-cpg-mssr.h" + +#define CPG_FRQCR 0x00 +#define CPG_CKIOSEL 0xF0 +#define CPG_SCLKSEL 0xF4 + +#define PORTL_PIDR 0xFCFFE074 +static u8 cpg_mode; + +/* Internal Clock ratio table */ +static const unsigned int ratio_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 */ + }; + +enum rz_clk_types { + CLK_TYPE_RZA_MAIN = CLK_TYPE_CUSTOM, + CLK_TYPE_RZA_PLL, +}; + +enum clk_ids { + /* Core Clock Outputs exported to DT */ + LAST_DT_CORE_CLK = R7S9210_CLK_P0, + + /* External Input Clocks */ + CLK_EXTAL, + + /* Internal Core Clocks */ + CLK_MAIN, + CLK_PLL, + CLK_I, + CLK_G, + CLK_B, + CLK_P1, + CLK_P1C, + CLK_P0, + + /* Module Clocks */ + MOD_CLK_BASE +}; + +static struct cpg_core_clk r7s9210_core_clks[] = { + /* External Clock Inputs */ + DEF_INPUT("extal", CLK_EXTAL), + + /* Internal Core Clocks */ + DEF_BASE(".main", CLK_MAIN, CLK_TYPE_RZA_MAIN, CLK_EXTAL), + DEF_BASE(".pll", CLK_PLL, CLK_TYPE_RZA_PLL, CLK_MAIN), + + /* Core Clock Outputs */ + DEF_FIXED("i", R7S9210_CLK_I, CLK_PLL, 2, 1), + DEF_FIXED("g", R7S9210_CLK_G, CLK_PLL, 4, 1), + DEF_FIXED("b", R7S9210_CLK_B, CLK_PLL, 8, 1), + DEF_FIXED("p1", R7S9210_CLK_P1, CLK_PLL, 16, 1), + DEF_FIXED("p1c", R7S9210_CLK_P1C, CLK_PLL, 16, 1), + DEF_FIXED("p0", R7S9210_CLK_P0, CLK_PLL, 32, 1), +}; + +static const struct mssr_mod_clk r7s9210_mod_clks[] __initconst = { + DEF_MOD_STB("ostm0", 36, R7S9210_CLK_P1C), + DEF_MOD_STB("ostm1", 35, R7S9210_CLK_P1C), + DEF_MOD_STB("ostm2", 34, R7S9210_CLK_P1C), + + DEF_MOD_STB("scif0", 47, R7S9210_CLK_P1C), + DEF_MOD_STB("scif1", 46, R7S9210_CLK_P1C), + DEF_MOD_STB("scif2", 45, R7S9210_CLK_P1C), + DEF_MOD_STB("scif3", 44, R7S9210_CLK_P1C), + DEF_MOD_STB("scif4", 43, R7S9210_CLK_P1C), + + DEF_MOD_STB("ether0", 65, R7S9210_CLK_B), + DEF_MOD_STB("ether1", 64, R7S9210_CLK_B), + + DEF_MOD_STB("i2c0", 87, R7S9210_CLK_P1), + DEF_MOD_STB("i2c1", 86, R7S9210_CLK_P1), + DEF_MOD_STB("i2c2", 85, R7S9210_CLK_P1), + DEF_MOD_STB("i2c3", 84, R7S9210_CLK_P1), + +}; + +struct clk * __init rza2_cpg_clk_register(struct device *dev, + const struct cpg_core_clk *core, const struct cpg_mssr_info *info, + struct clk **clks, void __iomem *base, + struct raw_notifier_head *notifiers) +{ + const struct clk *parent; + unsigned int mult = 1; + unsigned int div = 1; + u16 frqcr; + u8 index; + int i; + + parent = clks[core->parent]; + if (IS_ERR(parent)) + return ERR_CAST(parent); + + switch (core->id) { + case CLK_MAIN: + break; + + case CLK_PLL: + if (cpg_mode) + mult = 44; /* Divider 1 is 1/2 */ + else + mult = 88; /* Divider 1 is 1 */ + break; + + default: + return ERR_PTR(-EINVAL); + } + + /* Adjust the dividers based on the current FRQCR setting */ + if (core->id == CLK_MAIN) { + + /* If EXTAL is above 12MHz, then we know it is Mode 1 */ + if (clk_get_rate((struct clk *)parent) > 12000000) + cpg_mode = 1; + + frqcr = clk_readl(base + CPG_FRQCR) & 0xFFF; + if (frqcr == 0x012) + index = 0; + else if (frqcr == 0x112) + index = 1; + else if (frqcr == 0x212) + index = 2; + else if (frqcr == 0x322) + index = 3; + else if (frqcr == 0x333) + index = 4; + else + BUG_ON(1); /* Illegal FRQCR value */ + + for (i = 0; i < ARRAY_SIZE(r7s9210_core_clks); i++) { + switch (r7s9210_core_clks[i].id) { + case R7S9210_CLK_I: + r7s9210_core_clks[i].div = ratio_tab[index][0]; + break; + case R7S9210_CLK_G: + r7s9210_core_clks[i].div = ratio_tab[index][1]; + break; + case R7S9210_CLK_B: + r7s9210_core_clks[i].div = ratio_tab[index][2]; + break; + case R7S9210_CLK_P1: + case R7S9210_CLK_P1C: + r7s9210_core_clks[i].div = ratio_tab[index][3]; + break; + case R7S9210_CLK_P0: + r7s9210_core_clks[i].div = ratio_tab[index][4]; + break; + } + } + } + + return clk_register_fixed_factor(NULL, core->name, + __clk_get_name(parent), 0, mult, div); +} + +const struct cpg_mssr_info r7s9210_cpg_mssr_info __initconst = { + /* Core Clocks */ + .core_clks = r7s9210_core_clks, + .num_core_clks = ARRAY_SIZE(r7s9210_core_clks), + .last_dt_core_clk = LAST_DT_CORE_CLK, + .num_total_core_clks = MOD_CLK_BASE, + + /* Module Clocks */ + .mod_clks = r7s9210_mod_clks, + .num_mod_clks = ARRAY_SIZE(r7s9210_mod_clks), + .num_hw_mod_clks = 11 * 32, /* includes STBCR0/1/2 which don't exist */ + + /* Callbacks */ + .cpg_clk_register = rza2_cpg_clk_register, + + /* RZ/A2 has Standby Control Registers */ + .stbyctrl = true, +}; diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c index f90b0d0ba46a..14cde80716e4 100644 --- a/drivers/clk/renesas/renesas-cpg-mssr.c +++ b/drivers/clk/renesas/renesas-cpg-mssr.c @@ -73,6 +73,17 @@ static const u16 smstpcr[] = { #define SMSTPCR(i) smstpcr[i] +/* + * Standby Control Register offsets (RZ/A) + * Base address is FRQCR register + */ + +static const u16 stbcr[] = { + 0x000, 0x000, 0x014, 0x410, 0x414, 0x418, 0x41C, 0x420, + 0x424, 0x428, 0x42C, 0x430, 0x434, 0x460, +}; + +#define STBCR(i) stbcr[i] /* * Software Reset Register offsets @@ -110,6 +121,7 @@ static const u16 srcr[] = { * @notifiers: Notifier chain to save/restore clock state for system resume * @smstpcr_saved[].mask: Mask of SMSTPCR[] bits under our control * @smstpcr_saved[].val: Saved values of SMSTPCR[] + * @stbyctrl: This device has Standby Control Registers */ struct cpg_mssr_priv { #ifdef CONFIG_RESET_CONTROLLER @@ -123,6 +135,7 @@ struct cpg_mssr_priv { unsigned int num_core_clks; unsigned int num_mod_clks; unsigned int last_dt_core_clk; + bool stbyctrl; struct raw_notifier_head notifiers; struct { @@ -162,16 +175,29 @@ static int cpg_mstp_clock_endisable(struct clk_hw *hw, bool enable) enable ? "ON" : "OFF"); spin_lock_irqsave(&priv->rmw_lock, flags); - value = readl(priv->base + SMSTPCR(reg)); - if (enable) - value &= ~bitmask; - else - value |= bitmask; - writel(value, priv->base + SMSTPCR(reg)); + if (priv->stbyctrl) { + value = readb(priv->base + STBCR(reg)); + if (enable) + value &= ~bitmask; + else + value |= bitmask; + writeb(value, priv->base + STBCR(reg)); + + /* dummy read to ensure write has completed */ + readb(priv->base + STBCR(reg)); + barrier_data(priv->base + STBCR(reg)); + } else { + value = readl(priv->base + SMSTPCR(reg)); + if (enable) + value &= ~bitmask; + else + value |= bitmask; + writel(value, priv->base + SMSTPCR(reg)); + } spin_unlock_irqrestore(&priv->rmw_lock, flags); - if (!enable) + if (!enable || priv->stbyctrl) return 0; for (i = 1000; i > 0; --i) { @@ -205,7 +231,10 @@ static int cpg_mstp_clock_is_enabled(struct clk_hw *hw) struct cpg_mssr_priv *priv = clock->priv; u32 value; - value = readl(priv->base + MSTPSR(clock->index / 32)); + if (priv->stbyctrl) + value = readb(priv->base + STBCR(clock->index / 32)); + else + value = readl(priv->base + MSTPSR(clock->index / 32)); return !(value & BIT(clock->index % 32)); } @@ -226,6 +255,7 @@ struct clk *cpg_mssr_clk_src_twocell_get(struct of_phandle_args *clkspec, unsigned int idx; const char *type; struct clk *clk; + int range_check; switch (clkspec->args[0]) { case CPG_CORE: @@ -240,8 +270,14 @@ struct clk *cpg_mssr_clk_src_twocell_get(struct of_phandle_args *clkspec, case CPG_MOD: type = "module"; - idx = MOD_CLK_PACK(clkidx); - if (clkidx % 100 > 31 || idx >= priv->num_mod_clks) { + if (priv->stbyctrl) { + idx = MOD_CLK_PACK_10(clkidx); + range_check = 7 - (clkidx % 10); + } else { + idx = MOD_CLK_PACK_10(clkidx); + range_check = 31 - (clkidx % 100); + } + if (range_check < 0 || idx >= priv->num_mod_clks) { dev_err(dev, "Invalid %s clock index %u\n", type, clkidx); return ERR_PTR(-EINVAL); @@ -740,6 +776,12 @@ static const struct of_device_id cpg_mssr_match[] = { .compatible = "renesas,r8a77995-cpg-mssr", .data = &r8a77995_cpg_mssr_info, }, +#endif +#ifdef CONFIG_CLK_R7S9210 + { + .compatible = "renesas,r7s9210-cpg-mssr", + .data = &r7s9210_cpg_mssr_info, + }, #endif { /* sentinel */ } }; @@ -791,13 +833,23 @@ static int cpg_mssr_resume_noirq(struct device *dev) if (!mask) continue; - oldval = readl(priv->base + SMSTPCR(reg)); + if (priv->stbyctrl) + oldval = readb(priv->base + STBCR(reg)); + else + oldval = readl(priv->base + SMSTPCR(reg)); newval = oldval & ~mask; newval |= priv->smstpcr_saved[reg].val & mask; if (newval == oldval) continue; - writel(newval, priv->base + SMSTPCR(reg)); + if (priv->stbyctrl) { + writeb(newval, priv->base + STBCR(reg)); + /* dummy read to ensure write has completed */ + readb(priv->base + STBCR(reg)); + barrier_data(priv->base + STBCR(reg)); + continue; + } else + writel(newval, priv->base + SMSTPCR(reg)); /* Wait until enabled clocks are really enabled */ mask &= ~priv->smstpcr_saved[reg].val; @@ -869,6 +921,7 @@ static int __init cpg_mssr_probe(struct platform_device *pdev) priv->num_mod_clks = info->num_hw_mod_clks; priv->last_dt_core_clk = info->last_dt_core_clk; RAW_INIT_NOTIFIER_HEAD(&priv->notifiers); + priv->stbyctrl = info->stbyctrl; for (i = 0; i < nclks; i++) clks[i] = ERR_PTR(-ENOENT); @@ -894,6 +947,10 @@ static int __init cpg_mssr_probe(struct platform_device *pdev) if (error) return error; + /* Reset Controller not supported for Standby Control SoCs */ + if (info->stbyctrl) + return 0; + error = cpg_mssr_reset_controller_register(priv); if (error) return error; diff --git a/drivers/clk/renesas/renesas-cpg-mssr.h b/drivers/clk/renesas/renesas-cpg-mssr.h index 2e1730bc5ef2..58f32b38e99a 100644 --- a/drivers/clk/renesas/renesas-cpg-mssr.h +++ b/drivers/clk/renesas/renesas-cpg-mssr.h @@ -78,6 +78,13 @@ struct mssr_mod_clk { #define DEF_MOD(_name, _mod, _parent...) \ { .name = _name, .id = MOD_CLK_ID(_mod), .parent = _parent } +/* Convert from sparse base-10 to packed index space */ +#define MOD_CLK_PACK_10(x) ((x / 10) * 32 + (x % 10)) + +#define MOD_CLK_ID_10(x) (MOD_CLK_BASE + MOD_CLK_PACK_10(x)) + +#define DEF_MOD_STB(_name, _mod, _parent...) \ + { .name = _name, .id = MOD_CLK_ID_10(_mod), .parent = _parent } struct device_node; @@ -103,6 +110,10 @@ struct device_node; * * @init: Optional callback to perform SoC-specific initialization * @cpg_clk_register: Optional callback to handle special Core Clock types + * + * @stbyctrl: This device has Standby Control Registers which are 8-bits + * wide, no status registers (MSTPSR) and have different address + * offsets. */ struct cpg_mssr_info { @@ -111,6 +122,7 @@ struct cpg_mssr_info { unsigned int num_core_clks; unsigned int last_dt_core_clk; unsigned int num_total_core_clks; + bool stbyctrl; /* Module Clocks */ const struct mssr_mod_clk *mod_clks; @@ -149,6 +161,7 @@ extern const struct cpg_mssr_info r8a77970_cpg_mssr_info; extern const struct cpg_mssr_info r8a77980_cpg_mssr_info; extern const struct cpg_mssr_info r8a77990_cpg_mssr_info; extern const struct cpg_mssr_info r8a77995_cpg_mssr_info; +extern const struct cpg_mssr_info r7s9210_cpg_mssr_info; /* diff --git a/include/dt-bindings/clock/r7s9210-cpg-mssr.h b/include/dt-bindings/clock/r7s9210-cpg-mssr.h new file mode 100644 index 000000000000..676cc6321d8f --- /dev/null +++ b/include/dt-bindings/clock/r7s9210-cpg-mssr.h @@ -0,0 +1,21 @@ +/* SPDX-License-Identifier: GPL-2.0 + * + * Copyright (C) 2018 Renesas Electronics Corp. + * + */ + +#ifndef __DT_BINDINGS_CLOCK_R7S9210_CPG_MSSR_H__ +#define __DT_BINDINGS_CLOCK_R7S9210_CPG_MSSR_H__ + +#include <dt-bindings/clock/renesas-cpg-mssr.h> + +/* R7S9210 CPG Core Clocks */ +#define R7S9210_CLK_PLL 0 +#define R7S9210_CLK_I 1 +#define R7S9210_CLK_G 2 +#define R7S9210_CLK_B 3 +#define R7S9210_CLK_P1 4 +#define R7S9210_CLK_P1C 5 +#define R7S9210_CLK_P0 6 + +#endif /* __DT_BINDINGS_CLOCK_R7S9210_CPG_MSSR_H__ */
Add support for the R7S9210 (RZ/A2) Clock Pulse Generator and Module Standby. The Module Standby HW in the RZ/A series is very close to R-Car HW, except for how the registers are laid out. The MSTP registers are only 8-bits wide, there is no status registers (MSTPST), and the register offsets are a little different. Since the RZ/A hardware manuals refer to these registers as the Standby Control Registers, we'll use that name to distinguish the RZ/A type for the R-Car type. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- v3: * Use actual register bit names and numbers from manual for both DT and tables ("36" instead of "306") * Do not register reset controller for stbyctrl (RZ/A) SoCs * Changed SPDX from "GPL-2.0+" to "GPL-2.0" v2: * num_hw_mod_clks was wrong * added ethernet clocks --- .../devicetree/bindings/clock/renesas,cpg-mssr.txt | 3 +- drivers/clk/renesas/Kconfig | 5 + drivers/clk/renesas/Makefile | 1 + drivers/clk/renesas/r7s9210-cpg-mssr.c | 193 +++++++++++++++++++++ drivers/clk/renesas/renesas-cpg-mssr.c | 81 +++++++-- drivers/clk/renesas/renesas-cpg-mssr.h | 13 ++ include/dt-bindings/clock/r7s9210-cpg-mssr.h | 21 +++ 7 files changed, 304 insertions(+), 13 deletions(-) create mode 100644 drivers/clk/renesas/r7s9210-cpg-mssr.c create mode 100644 include/dt-bindings/clock/r7s9210-cpg-mssr.h