| Message ID | 1444999760-15750-6-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Geert, Quoting Geert Uytterhoeven (2015-10-16 05:49:20) > +static void __init r8a7795_cpg_mssr_init(struct device_node *np) > +{ > + struct regmap *regmap; > + u32 reg, cpg_mode; > + > + regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr"); > + if (IS_ERR(regmap) || > + of_property_read_u32_index(np, "renesas,modemr", 1, ®) || > + regmap_read(regmap, reg, &cpg_mode)) { > + pr_err("%s: failed to parse renesas,modemr\n", np->full_name); > + return; > + } > + > + cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)]; > + if (!cpg_pll_config->extal_div) { > + pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n", > + __func__, cpg_mode); > + return; > + } > + > + cpg_mssr_probe(np, &r8a7795_cpg_mssr_info); > +} > +CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr", > + r8a7795_cpg_mssr_init); Is CLK_OF_DECLARE needed? Is it possible to make this a real platform_driver à la drivers/clk/qcom/gcc-apq8084.c? Sorry if I already asked this in a previous version, but a quick search of my email didn't reveal anything. Regards, Mike > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mike, On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette <mturquette@baylibre.com> wrote: > Quoting Geert Uytterhoeven (2015-10-16 05:49:20) >> +static void __init r8a7795_cpg_mssr_init(struct device_node *np) >> +{ >> + struct regmap *regmap; >> + u32 reg, cpg_mode; >> + >> + regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr"); >> + if (IS_ERR(regmap) || >> + of_property_read_u32_index(np, "renesas,modemr", 1, ®) || >> + regmap_read(regmap, reg, &cpg_mode)) { >> + pr_err("%s: failed to parse renesas,modemr\n", np->full_name); >> + return; >> + } >> + >> + cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)]; >> + if (!cpg_pll_config->extal_div) { >> + pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n", >> + __func__, cpg_mode); >> + return; >> + } >> + >> + cpg_mssr_probe(np, &r8a7795_cpg_mssr_info); >> +} >> +CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr", >> + r8a7795_cpg_mssr_init); > > Is CLK_OF_DECLARE needed? Is it possible to make this a real > platform_driver à la drivers/clk/qcom/gcc-apq8084.c? I tried making it a real platform driver, but it failed: devices that are part of the Clock Domain failed to get their clock (error -2, IIRC, which is -ENOENT), and thus couldn't be instantiated. I didn't look deeper at that time. [... reading code ...] Aha, this may be caused by __of_clk_get_from_provider() returning hardcoded -ENOENT instead of propagating the error returned by __clk_create_clk()? 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mike, On Tue, Oct 20, 2015 at 3:00 PM, Michael Turquette <mturquette@baylibre.com> wrote: > Quoting Geert Uytterhoeven (2015-10-20 05:31:12) >> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette >> <mturquette@baylibre.com> wrote: >> > Quoting Geert Uytterhoeven (2015-10-16 05:49:20) >> >> +static void __init r8a7795_cpg_mssr_init(struct device_node *np) >> >> +{ >> >> + struct regmap *regmap; >> >> + u32 reg, cpg_mode; >> >> + >> >> + regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr"); >> >> + if (IS_ERR(regmap) || >> >> + of_property_read_u32_index(np, "renesas,modemr", 1, ®) || >> >> + regmap_read(regmap, reg, &cpg_mode)) { >> >> + pr_err("%s: failed to parse renesas,modemr\n", np->full_name); >> >> + return; >> >> + } >> >> + >> >> + cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)]; >> >> + if (!cpg_pll_config->extal_div) { >> >> + pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n", >> >> + __func__, cpg_mode); >> >> + return; >> >> + } >> >> + >> >> + cpg_mssr_probe(np, &r8a7795_cpg_mssr_info); >> >> +} >> >> +CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr", >> >> + r8a7795_cpg_mssr_init); >> > >> > Is CLK_OF_DECLARE needed? Is it possible to make this a real >> > platform_driver à la drivers/clk/qcom/gcc-apq8084.c? >> >> I tried making it a real platform driver, but it failed: devices that are >> part of the Clock Domain failed to get their clock (error -2, IIRC, which is >> -ENOENT), and thus couldn't be instantiated. >> I didn't look deeper at that time. >> >> [... reading code ...] >> >> Aha, this may be caused by __of_clk_get_from_provider() returning >> hardcoded -ENOENT instead of propagating the error returned by >> __clk_create_clk()? > > Well the only other error thrown by __clk_create_clk is -ENOMEM, so I'm > not sure how that would help things. Hmm, you're right. > The bindings should go in for 4.4, but if the driver is slated for 4.5 > then can you investigate this some more? Stephen and I are on a mission > to have _real_ clk drivers. Sure, I'll have a deeper look. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mike, On Tue, Oct 20, 2015 at 3:07 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Tue, Oct 20, 2015 at 3:00 PM, Michael Turquette > <mturquette@baylibre.com> wrote: >> Quoting Geert Uytterhoeven (2015-10-20 05:31:12) >>> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette >>> <mturquette@baylibre.com> wrote: >>> > Quoting Geert Uytterhoeven (2015-10-16 05:49:20) >>> >> +static void __init r8a7795_cpg_mssr_init(struct device_node *np) >>> >> +{ >>> >> + struct regmap *regmap; >>> >> + u32 reg, cpg_mode; >>> >> + >>> >> + regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr"); >>> >> + if (IS_ERR(regmap) || >>> >> + of_property_read_u32_index(np, "renesas,modemr", 1, ®) || >>> >> + regmap_read(regmap, reg, &cpg_mode)) { >>> >> + pr_err("%s: failed to parse renesas,modemr\n", np->full_name); >>> >> + return; >>> >> + } >>> >> + >>> >> + cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)]; >>> >> + if (!cpg_pll_config->extal_div) { >>> >> + pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n", >>> >> + __func__, cpg_mode); >>> >> + return; >>> >> + } >>> >> + >>> >> + cpg_mssr_probe(np, &r8a7795_cpg_mssr_info); >>> >> +} >>> >> +CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr", >>> >> + r8a7795_cpg_mssr_init); >>> > >>> > Is CLK_OF_DECLARE needed? Is it possible to make this a real >>> > platform_driver à la drivers/clk/qcom/gcc-apq8084.c? >>> >>> I tried making it a real platform driver, but it failed: devices that are >>> part of the Clock Domain failed to get their clock (error -2, IIRC, which is >>> -ENOENT), and thus couldn't be instantiated. >>> I didn't look deeper at that time. >>> >>> [... reading code ...] >>> >>> Aha, this may be caused by __of_clk_get_from_provider() returning >>> hardcoded -ENOENT instead of propagating the error returned by >>> __clk_create_clk()? >> >> Well the only other error thrown by __clk_create_clk is -ENOMEM, so I'm >> not sure how that would help things. > > Hmm, you're right. > >> The bindings should go in for 4.4, but if the driver is slated for 4.5 >> then can you investigate this some more? Stephen and I are on a mission >> to have _real_ clk drivers. > > Sure, I'll have a deeper look. And so I did (on r8a7791/koelsch). As I want to have as much clock data/code __init as possible (think multi-platform kernels --- pinmux data is a disaster here), I have to use platform_driver_probe(). - Calling platform_driver_probe() from core_initcall() or postcore_initcall() is too early, as the platform device for the CPG hasn't been created yet. Hence the CPG Clock Domain isn't registered, and all devices fail to probe as they can't be attached to their Clock Domain. -> This is where the -ENOENT came from (I incorrectly assumed it came from the clock code; sorry for that), and it's converted into -EPROBE_DEFER by genpd_dev_pm_attach(). - Calling platform_driver_probe() from arch_initcall() is too late, as the IRQC is initialized first (it's located before the CPG in .dtsi). Hence the IRQC can't find it's Clock Domain, and its probe is deferred. IRQC will be reprobed later, but in the mean time the Ethernet PHY can't find its IRQ, as the of_mdio code uses irq_of_parse_and_map(), which plainly ignores EPROBE_DEFER :-( Nevertheless, Ethernet works... - Using subsys_initcall() and later causes even more probe deferral. So that's why I went with CLK_OF_DECLARE() again... 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/22, Geert Uytterhoeven wrote: > Hi Mike, > > On Tue, Oct 20, 2015 at 3:07 PM, Geert Uytterhoeven > <geert@linux-m68k.org> wrote: > > On Tue, Oct 20, 2015 at 3:00 PM, Michael Turquette > > <mturquette@baylibre.com> wrote: > >> Quoting Geert Uytterhoeven (2015-10-20 05:31:12) > >>> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette > >>> <mturquette@baylibre.com> wrote: > > > >> The bindings should go in for 4.4, but if the driver is slated for 4.5 > >> then can you investigate this some more? Stephen and I are on a mission > >> to have _real_ clk drivers. > > > > Sure, I'll have a deeper look. > > And so I did (on r8a7791/koelsch). > > As I want to have as much clock data/code __init as possible (think > multi-platform kernels --- pinmux data is a disaster here), I have to use > platform_driver_probe(). > > - Calling platform_driver_probe() from core_initcall() or postcore_initcall() > is too early, as the platform device for the CPG hasn't been created yet. > Hence the CPG Clock Domain isn't registered, and all devices fail to probe > as they can't be attached to their Clock Domain. > -> This is where the -ENOENT came from (I incorrectly assumed it came > from the clock code; sorry for that), and it's converted into > -EPROBE_DEFER by genpd_dev_pm_attach(). > > - Calling platform_driver_probe() from arch_initcall() is too late, as the > IRQC is initialized first (it's located before the CPG in .dtsi). > Hence the IRQC can't find it's Clock Domain, and its probe is deferred. > IRQC will be reprobed later, but in the mean time the Ethernet PHY can't > find its IRQ, as the of_mdio code uses irq_of_parse_and_map(), which > plainly ignores EPROBE_DEFER :-( > Nevertheless, Ethernet works... > > - Using subsys_initcall() and later causes even more probe deferral. > > So that's why I went with CLK_OF_DECLARE() again... > Understandable for the few clocks that are used by the interrupt controller, but for the other clocks, could those be registered from a real platform device driver probe path? I've been considering making some API that lets devices associate with the clocks that the file had to register with CLK_OF_DECLARE(). The driver would have to be builtin then (no modules) but otherwise we would be able to benefit from the device driver model for most other clocks.
Hi Stephen, On Sat, Oct 24, 2015 at 3:10 AM, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 10/22, Geert Uytterhoeven wrote: >> On Tue, Oct 20, 2015 at 3:07 PM, Geert Uytterhoeven >> <geert@linux-m68k.org> wrote: >> > On Tue, Oct 20, 2015 at 3:00 PM, Michael Turquette >> > <mturquette@baylibre.com> wrote: >> >> Quoting Geert Uytterhoeven (2015-10-20 05:31:12) >> >>> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette >> >>> <mturquette@baylibre.com> wrote: >> > >> >> The bindings should go in for 4.4, but if the driver is slated for 4.5 >> >> then can you investigate this some more? Stephen and I are on a mission >> >> to have _real_ clk drivers. >> > >> > Sure, I'll have a deeper look. >> >> And so I did (on r8a7791/koelsch). >> >> As I want to have as much clock data/code __init as possible (think >> multi-platform kernels --- pinmux data is a disaster here), I have to use >> platform_driver_probe(). >> >> - Calling platform_driver_probe() from core_initcall() or postcore_initcall() >> is too early, as the platform device for the CPG hasn't been created yet. >> Hence the CPG Clock Domain isn't registered, and all devices fail to probe >> as they can't be attached to their Clock Domain. >> -> This is where the -ENOENT came from (I incorrectly assumed it came >> from the clock code; sorry for that), and it's converted into >> -EPROBE_DEFER by genpd_dev_pm_attach(). >> >> - Calling platform_driver_probe() from arch_initcall() is too late, as the >> IRQC is initialized first (it's located before the CPG in .dtsi). >> Hence the IRQC can't find it's Clock Domain, and its probe is deferred. >> IRQC will be reprobed later, but in the mean time the Ethernet PHY can't >> find its IRQ, as the of_mdio code uses irq_of_parse_and_map(), which >> plainly ignores EPROBE_DEFER :-( >> Nevertheless, Ethernet works... >> >> - Using subsys_initcall() and later causes even more probe deferral. >> >> So that's why I went with CLK_OF_DECLARE() again... > > Understandable for the few clocks that are used by the interrupt > controller, but for the other clocks, could those be registered > from a real platform device driver probe path? I've been In this particular case, that would be the IRQC module clocks and its parent, the CP clock, which is derived directly from the external crystal. If we ever have to do this for the GIC, that's gonna be several more clocks (INTC-SYS / ZS / PLL1 / MAIN / external crystal). > considering making some API that lets devices associate with the > clocks that the file had to register with CLK_OF_DECLARE(). The > driver would have to be builtin then (no modules) but otherwise > we would be able to benefit from the device driver model for most > other clocks. To be honest, that still feels like a hack to me. I hope dependencies and probe order, and obscure drivers not handling deferred probe correctly, will be solved later sooner or later, so hacks like that are no longer needed. For new SoCs like r8a7795 we can probably just make it a real platform driver, and make sure the irqc node is located after the cpg node in the .dtsi. Upon second thought, that may even be feasible for existing SoCs, as they would need a DTS update to make use of the new bindings/driver anyway, so the irqc node can be moved at the same time. Backwards compatibility through the old bindings/drivers would keep on using CLK_OF_DECLARE(). 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, On Saturday 24 October 2015 19:34:03 Geert Uytterhoeven wrote: > On Sat, Oct 24, 2015 at 3:10 AM, Stephen Boyd <sboyd@codeaurora.org> wrote: > > On 10/22, Geert Uytterhoeven wrote: > >> On Tue, Oct 20, 2015 at 3:07 PM, Geert Uytterhoeven wrote: > >>> On Tue, Oct 20, 2015 at 3:00 PM, Michael Turquette wrote: > >>>> Quoting Geert Uytterhoeven (2015-10-20 05:31:12) > >>>>> On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette wrote: > >>>> The bindings should go in for 4.4, but if the driver is slated for 4.5 > >>>> then can you investigate this some more? Stephen and I are on a > >>>> mission to have _real_ clk drivers. > >>> > >>> Sure, I'll have a deeper look. > >> > >> And so I did (on r8a7791/koelsch). > >> > >> As I want to have as much clock data/code __init as possible (think > >> multi-platform kernels --- pinmux data is a disaster here), I have to use > >> platform_driver_probe(). That sounds like an __init issue, doesn't it ? The CPG driver will always be builtin and probed during the init process, what's preventing us from using normal driver probing ? > >> - Calling platform_driver_probe() from core_initcall() or > >> postcore_initcall() is too early, as the platform device for the CPG > >> hasn't been created yet. Hence the CPG Clock Domain isn't registered, > >> and all devices fail to probe as they can't be attached to their > >> Clock Domain. > >> > >> -> This is where the -ENOENT came from (I incorrectly assumed it > >> came from the clock code; sorry for that), and it's converted > >> into -EPROBE_DEFER by genpd_dev_pm_attach(). > >> > >> - Calling platform_driver_probe() from arch_initcall() is too late, as > >> the IRQC is initialized first (it's located before the CPG in .dtsi). > >> Hence the IRQC can't find it's Clock Domain, and its probe is > >> deferred. IRQC will be reprobed later, but in the mean time the > >> Ethernet PHY can't find its IRQ, as the of_mdio code uses > >> irq_of_parse_and_map(), which plainly ignores EPROBE_DEFER :-( > >> Nevertheless, Ethernet works... > >> > >> - Using subsys_initcall() and later causes even more probe deferral. > >> > >> So that's why I went with CLK_OF_DECLARE() again... > > > > Understandable for the few clocks that are used by the interrupt > > controller, but for the other clocks, could those be registered > > from a real platform device driver probe path? I've been > > In this particular case, that would be the IRQC module clocks and its > parent, the CP clock, which is derived directly from the external crystal. > > If we ever have to do this for the GIC, that's gonna be several more > clocks (INTC-SYS / ZS / PLL1 / MAIN / external crystal). > > > considering making some API that lets devices associate with the > > clocks that the file had to register with CLK_OF_DECLARE(). The > > driver would have to be builtin then (no modules) but otherwise > > we would be able to benefit from the device driver model for most > > other clocks. > > To be honest, that still feels like a hack to me. > > I hope dependencies and probe order, and obscure drivers not handling > deferred probe correctly, will be solved later sooner or later, > so hacks like that are no longer needed. > > For new SoCs like r8a7795 we can probably just make it a real platform > driver, and make sure the irqc node is located after the cpg node in the > .dtsi. That's another hack :-) We really shouldn't depend on DT nodes order. I'm all for getting rid of CLK_OF_DECLARE > Upon second thought, that may even be feasible for existing SoCs, as > they would need a DTS update to make use of the new bindings/driver anyway, > so the irqc node can be moved at the same time. Backwards compatibility > through the old bindings/drivers would keep on using CLK_OF_DECLARE().
Hi Laurent, On Mon, Oct 26, 2015 at 3:25 AM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > On Saturday 24 October 2015 19:34:03 Geert Uytterhoeven wrote: >> On Sat, Oct 24, 2015 at 3:10 AM, Stephen Boyd <sboyd@codeaurora.org> wrote: >> > On 10/22, Geert Uytterhoeven wrote: >> >> As I want to have as much clock data/code __init as possible (think >> >> multi-platform kernels --- pinmux data is a disaster here), I have to use >> >> platform_driver_probe(). > > That sounds like an __init issue, doesn't it ? The CPG driver will always be > builtin and probed during the init process, what's preventing us from using > normal driver probing ? When using platform_driver_register(), the tables cannot be __init, as that would cause a section type mismatch. Remember, the driver core handles platform devices appearing later, so .probe() should continue to be available. Note: in theory it should be possible to compile the CPG/MSSR driver as a module, and have the module in your initramfs. But I don't think anyone really wants to do that? >> For new SoCs like r8a7795 we can probably just make it a real platform >> driver, and make sure the irqc node is located after the cpg node in the >> .dtsi. > > That's another hack :-) We really shouldn't depend on DT nodes order. I agree. But if there's an unfixed bug somewhere else, we cannot introduce regressions (for already supported SoCs). > I'm all for getting rid of CLK_OF_DECLARE Me too. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Oct 22, 2015 at 2:58 PM, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > - Calling platform_driver_probe() from arch_initcall() is too late, as the > IRQC is initialized first (it's located before the CPG in .dtsi). > Hence the IRQC can't find it's Clock Domain, and its probe is deferred. > IRQC will be reprobed later, but in the mean time the Ethernet PHY can't > find its IRQ, as the of_mdio code uses irq_of_parse_and_map(), which > plainly ignores EPROBE_DEFER :-( > Nevertheless, Ethernet works... To correct myself: renesas-irqc is initialized first because it uses postcore_initcall(). The of_mdio issue on R-Car Gen2 boards can be worked around by changing that to device_initcall(). That would cause a few more probe deferrals on R-Mobile APE6 (r8a73a4), where IRQC is not only the external interrupt controller, but also the parent interrupt controller of the PFC/GPIO combo. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Geert, On Monday 26 October 2015 09:03:44 Geert Uytterhoeven wrote: > On Mon, Oct 26, 2015 at 3:25 AM, Laurent Pinchart wrote: > > On Saturday 24 October 2015 19:34:03 Geert Uytterhoeven wrote: > >> On Sat, Oct 24, 2015 at 3:10 AM, Stephen Boyd wrote: > >> > On 10/22, Geert Uytterhoeven wrote: > >> >> As I want to have as much clock data/code __init as possible (think > >> >> multi-platform kernels --- pinmux data is a disaster here), I have to > >> >> use platform_driver_probe(). > > > > That sounds like an __init issue, doesn't it ? The CPG driver will always > > be builtin and probed during the init process, what's preventing us from > > using normal driver probing ? > > When using platform_driver_register(), the tables cannot be __init, as that > would cause a section type mismatch. Remember, the driver core handles > platform devices appearing later, so .probe() should continue to be > available. Of course, my bad. > Note: in theory it should be possible to compile the CPG/MSSR driver as a > module, and have the module in your initramfs. But I don't think anyone > really wants to do that? I don't think we should allow that, no. > >> For new SoCs like r8a7795 we can probably just make it a real platform > >> driver, and make sure the irqc node is located after the cpg node in the > >> .dtsi. > > > > That's another hack :-) We really shouldn't depend on DT nodes order. > > I agree. But if there's an unfixed bug somewhere else, we cannot introduce > regressions (for already supported SoCs). Sure, and I'm fine relying on DT nodes order as a short term hack, but we need to design a proper solution for the longer term. > > I'm all for getting rid of CLK_OF_DECLARE > > Me too.
diff --git a/drivers/clk/shmobile/Makefile b/drivers/clk/shmobile/Makefile index 77b70b070e396a65..bbeb4d4ff715ff74 100644 --- a/drivers/clk/shmobile/Makefile +++ b/drivers/clk/shmobile/Makefile @@ -8,5 +8,6 @@ obj-$(CONFIG_ARCH_R8A7790) += clk-rcar-gen2.o clk-mstp.o clk-div6.o obj-$(CONFIG_ARCH_R8A7791) += clk-rcar-gen2.o clk-mstp.o clk-div6.o obj-$(CONFIG_ARCH_R8A7793) += clk-rcar-gen2.o clk-mstp.o clk-div6.o obj-$(CONFIG_ARCH_R8A7794) += clk-rcar-gen2.o clk-mstp.o clk-div6.o -obj-$(CONFIG_ARCH_R8A7795) += clk-rcar-gen3.o clk-mssr.o clk-div6.o +obj-$(CONFIG_ARCH_R8A7795) += clk-cpg-mssr.o \ + clk-r8a7795-cpg-mssr.o clk-div6.o obj-$(CONFIG_ARCH_SH73A0) += clk-sh73a0.o clk-mstp.o clk-div6.o diff --git a/drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c b/drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c new file mode 100644 index 0000000000000000..18444a13e5722e2b --- /dev/null +++ b/drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c @@ -0,0 +1,373 @@ +/* + * r8a7795 Clock Pulse Generator / Module Standby and Software Reset + * + * Copyright (C) 2015 Glider bvba + * + * Based on clk-rcar-gen3.c + * + * Copyright (C) 2015 Renesas Electronics Corp. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + */ + +#include <linux/bug.h> +#include <linux/clk-provider.h> +#include <linux/err.h> +#include <linux/init.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/mfd/syscon.h> +#include <linux/of.h> +#include <linux/regmap.h> + +#include <dt-bindings/clock/r8a7795-cpg-mssr.h> + +#include "clk-cpg-mssr.h" + + +enum clk_ids { + /* Core Clock Outputs exported to DT */ + LAST_DT_CORE_CLOCK = R8A7795_CLK_OSC, + + /* External Input Clocks */ + CLK_EXTAL, + CLK_EXTALR, + + /* Internal Core Clocks */ + CLK_MAIN, + CLK_PLL0, + CLK_PLL1, + CLK_PLL2, + CLK_PLL3, + CLK_PLL4, + CLK_PLL1_DIV2, + CLK_PLL1_DIV4, + CLK_S0, + CLK_S1, + CLK_S2, + CLK_S3, + CLK_SDSRC, + CLK_SSPSRC, + + /* Module Clocks */ + MOD_CLK_BASE +}; + +enum r8a7795_clk_types { + CLK_TYPE_GEN3_MAIN = CLK_TYPE_CUSTOM, + CLK_TYPE_GEN3_PLL0, + CLK_TYPE_GEN3_PLL1, + CLK_TYPE_GEN3_PLL2, + CLK_TYPE_GEN3_PLL3, + CLK_TYPE_GEN3_PLL4, +}; + +static const struct cpg_core_clk r8a7795_core_clks[] __initconst = { + /* External Clock Inputs */ + DEF_INPUT("extal", CLK_EXTAL), + DEF_INPUT("extalr", CLK_EXTALR), + + /* Internal Core Clocks */ + DEF_BASE(".main", CLK_MAIN, CLK_TYPE_GEN3_MAIN, CLK_EXTAL), + DEF_BASE(".pll0", CLK_PLL0, CLK_TYPE_GEN3_PLL0, CLK_MAIN), + DEF_BASE(".pll1", CLK_PLL1, CLK_TYPE_GEN3_PLL1, CLK_MAIN), + DEF_BASE(".pll2", CLK_PLL2, CLK_TYPE_GEN3_PLL2, CLK_MAIN), + DEF_BASE(".pll3", CLK_PLL3, CLK_TYPE_GEN3_PLL3, CLK_MAIN), + DEF_BASE(".pll4", CLK_PLL4, CLK_TYPE_GEN3_PLL4, CLK_MAIN), + + DEF_FIXED(".pll1_div2", CLK_PLL1_DIV2, CLK_PLL1, 2, 1), + DEF_FIXED(".pll1_div4", CLK_PLL1_DIV4, CLK_PLL1_DIV2, 2, 1), + DEF_FIXED(".s0", CLK_S0, CLK_PLL1_DIV2, 2, 1), + DEF_FIXED(".s1", CLK_S1, CLK_PLL1_DIV2, 3, 1), + DEF_FIXED(".s2", CLK_S2, CLK_PLL1_DIV2, 4, 1), + DEF_FIXED(".s3", CLK_S3, CLK_PLL1_DIV2, 6, 1), + + /* Core Clock Outputs */ + DEF_FIXED("ztr", R8A7795_CLK_ZTR, CLK_PLL1_DIV2, 6, 1), + DEF_FIXED("ztrd2", R8A7795_CLK_ZTRD2, CLK_PLL1_DIV2, 12, 1), + DEF_FIXED("zt", R8A7795_CLK_ZT, CLK_PLL1_DIV2, 4, 1), + DEF_FIXED("zx", R8A7795_CLK_ZX, CLK_PLL1_DIV2, 2, 1), + DEF_FIXED("s0d1", R8A7795_CLK_S0D1, CLK_S0, 1, 1), + DEF_FIXED("s0d4", R8A7795_CLK_S0D4, CLK_S0, 4, 1), + DEF_FIXED("s1d1", R8A7795_CLK_S1D1, CLK_S1, 1, 1), + DEF_FIXED("s1d2", R8A7795_CLK_S1D2, CLK_S1, 2, 1), + DEF_FIXED("s1d4", R8A7795_CLK_S1D4, CLK_S1, 4, 1), + DEF_FIXED("s2d1", R8A7795_CLK_S2D1, CLK_S2, 1, 1), + DEF_FIXED("s2d2", R8A7795_CLK_S2D2, CLK_S2, 2, 1), + DEF_FIXED("s2d4", R8A7795_CLK_S2D4, CLK_S2, 4, 1), + DEF_FIXED("s3d1", R8A7795_CLK_S3D1, CLK_S3, 1, 1), + DEF_FIXED("s3d2", R8A7795_CLK_S3D2, CLK_S3, 2, 1), + DEF_FIXED("s3d4", R8A7795_CLK_S3D4, CLK_S3, 4, 1), + DEF_FIXED("cl", R8A7795_CLK_CL, CLK_PLL1_DIV2, 48, 1), + DEF_FIXED("cp", R8A7795_CLK_CP, CLK_EXTAL, 2, 1), + DEF_DIV6P1("mso", R8A7795_CLK_MSO, CLK_PLL1_DIV4, 0x014), + DEF_DIV6P1("hdmi", R8A7795_CLK_HDMI, CLK_PLL1_DIV2, 0x250), +}; + +static const struct mssr_mod_clk r8a7795_mod_clks[] __initconst = { + { "scif5", 202, R8A7795_CLK_S3D4 }, + { "scif4", 203, R8A7795_CLK_S3D4 }, + { "scif3", 204, R8A7795_CLK_S3D4 }, + { "scif1", 206, R8A7795_CLK_S3D4 }, + { "scif0", 207, R8A7795_CLK_S3D4 }, + { "msiof3", 208, R8A7795_CLK_MSO }, + { "msiof2", 209, R8A7795_CLK_MSO }, + { "msiof1", 210, R8A7795_CLK_MSO }, + { "msiof0", 211, R8A7795_CLK_MSO }, + { "sys-dmac2", 217, R8A7795_CLK_S3D1 }, + { "sys-dmac1", 218, R8A7795_CLK_S3D1 }, + { "sys-dmac0", 219, R8A7795_CLK_S3D1 }, + { "scif2", 310, R8A7795_CLK_S3D4 }, + { "intc-ap", 408, R8A7795_CLK_S3D1 }, + { "audmac0", 502, R8A7795_CLK_S3D4 }, + { "audmac1", 501, R8A7795_CLK_S3D4 }, + { "hscif4", 516, R8A7795_CLK_S3D1 }, + { "hscif3", 517, R8A7795_CLK_S3D1 }, + { "hscif2", 518, R8A7795_CLK_S3D1 }, + { "hscif1", 519, R8A7795_CLK_S3D1 }, + { "hscif0", 520, R8A7795_CLK_S3D1 }, + { "vspd3", 620, R8A7795_CLK_S2D1 }, + { "vspd2", 621, R8A7795_CLK_S2D1 }, + { "vspd1", 622, R8A7795_CLK_S2D1 }, + { "vspd0", 623, R8A7795_CLK_S2D1 }, + { "vspbc", 624, R8A7795_CLK_S2D1 }, + { "vspbd", 626, R8A7795_CLK_S2D1 }, + { "vspi2", 629, R8A7795_CLK_S2D1 }, + { "vspi1", 630, R8A7795_CLK_S2D1 }, + { "vspi0", 631, R8A7795_CLK_S2D1 }, + { "ehci2", 701, R8A7795_CLK_S3D4 }, + { "ehci1", 702, R8A7795_CLK_S3D4 }, + { "ehci0", 703, R8A7795_CLK_S3D4 }, + { "hsusb", 704, R8A7795_CLK_S3D4 }, + { "du3", 721, R8A7795_CLK_S2D1 }, + { "du2", 722, R8A7795_CLK_S2D1 }, + { "du1", 723, R8A7795_CLK_S2D1 }, + { "du0", 724, R8A7795_CLK_S2D1 }, + { "hdmi1", 728, R8A7795_CLK_HDMI }, + { "hdmi0", 729, R8A7795_CLK_HDMI }, + { "etheravb", 812, R8A7795_CLK_S3D2 }, + { "gpio7", 905, R8A7795_CLK_CP }, + { "gpio6", 906, R8A7795_CLK_CP }, + { "gpio5", 907, R8A7795_CLK_CP }, + { "gpio4", 908, R8A7795_CLK_CP }, + { "gpio3", 909, R8A7795_CLK_CP }, + { "gpio2", 910, R8A7795_CLK_CP }, + { "gpio1", 911, R8A7795_CLK_CP }, + { "gpio0", 912, R8A7795_CLK_CP }, + { "i2c6", 918, R8A7795_CLK_S3D2 }, + { "i2c5", 919, R8A7795_CLK_S3D2 }, + { "i2c4", 927, R8A7795_CLK_S3D2 }, + { "i2c3", 928, R8A7795_CLK_S3D2 }, + { "i2c2", 929, R8A7795_CLK_S3D2 }, + { "i2c1", 930, R8A7795_CLK_S3D2 }, + { "i2c0", 931, R8A7795_CLK_S3D2 }, + { "ssi-all", 1005, R8A7795_CLK_S3D4 }, + { "ssi9", 1006, MOD_CLK_BASE + 1005 }, + { "ssi8", 1007, MOD_CLK_BASE + 1005 }, + { "ssi7", 1008, MOD_CLK_BASE + 1005 }, + { "ssi6", 1009, MOD_CLK_BASE + 1005 }, + { "ssi5", 1010, MOD_CLK_BASE + 1005 }, + { "ssi4", 1011, MOD_CLK_BASE + 1005 }, + { "ssi3", 1012, MOD_CLK_BASE + 1005 }, + { "ssi2", 1013, MOD_CLK_BASE + 1005 }, + { "ssi1", 1014, MOD_CLK_BASE + 1005 }, + { "ssi0", 1015, MOD_CLK_BASE + 1005 }, + { "scu-all", 1017, R8A7795_CLK_S3D4 }, + { "scu-dvc1", 1018, MOD_CLK_BASE + 1017 }, + { "scu-dvc0", 1019, MOD_CLK_BASE + 1017 }, + { "scu-ctu1-mix1", 1020, MOD_CLK_BASE + 1017 }, + { "scu-ctu0-mix0", 1021, MOD_CLK_BASE + 1017 }, + { "scu-src9", 1022, MOD_CLK_BASE + 1017 }, + { "scu-src8", 1023, MOD_CLK_BASE + 1017 }, + { "scu-src7", 1024, MOD_CLK_BASE + 1017 }, + { "scu-src6", 1025, MOD_CLK_BASE + 1017 }, + { "scu-src5", 1026, MOD_CLK_BASE + 1017 }, + { "scu-src4", 1027, MOD_CLK_BASE + 1017 }, + { "scu-src3", 1028, MOD_CLK_BASE + 1017 }, + { "scu-src2", 1029, MOD_CLK_BASE + 1017 }, + { "scu-src1", 1030, MOD_CLK_BASE + 1017 }, + { "scu-src0", 1031, MOD_CLK_BASE + 1017 }, +}; + +static const unsigned int r8a7795_crit_mod_clks[] __initconst = { + 408, /* INTC-AP (GIC) */ +}; + + +#define CPG_PLL0CR 0x00d8 +#define CPG_PLL2CR 0x002c + +/* + * CPG Clock Data + */ + +/* + * MD EXTAL PLL0 PLL1 PLL2 PLL3 PLL4 + * 14 13 19 17 (MHz) + *------------------------------------------------------------------- + * 0 0 0 0 16.66 x 1 x180 x192 x144 x192 x144 + * 0 0 0 1 16.66 x 1 x180 x192 x144 x128 x144 + * 0 0 1 0 Prohibited setting + * 0 0 1 1 16.66 x 1 x180 x192 x144 x192 x144 + * 0 1 0 0 20 x 1 x150 x156 x120 x156 x120 + * 0 1 0 1 20 x 1 x150 x156 x120 x106 x120 + * 0 1 1 0 Prohibited setting + * 0 1 1 1 20 x 1 x150 x156 x120 x156 x120 + * 1 0 0 0 25 x 1 x120 x128 x96 x128 x96 + * 1 0 0 1 25 x 1 x120 x128 x96 x84 x96 + * 1 0 1 0 Prohibited setting + * 1 0 1 1 25 x 1 x120 x128 x96 x128 x96 + * 1 1 0 0 33.33 / 2 x180 x192 x144 x192 x144 + * 1 1 0 1 33.33 / 2 x180 x192 x144 x128 x144 + * 1 1 1 0 Prohibited setting + * 1 1 1 1 33.33 / 2 x180 x192 x144 x192 x144 + */ +#define CPG_PLL_CONFIG_INDEX(md) ((((md) & BIT(14)) >> 11) | \ + (((md) & BIT(13)) >> 11) | \ + (((md) & BIT(19)) >> 18) | \ + (((md) & BIT(17)) >> 17)) + +struct cpg_pll_config { + unsigned int extal_div; + unsigned int pll1_mult; + unsigned int pll3_mult; + unsigned int pll4_mult; +}; + +static const struct cpg_pll_config cpg_pll_configs[16] __initconst = { +/* EXTAL div PLL1 PLL3 PLL4 */ + { 1, 192, 192, 144, }, + { 1, 192, 128, 144, }, + { 0, 0, 0, 0, }, /* Prohibited setting */ + { 1, 192, 192, 144, }, + { 1, 156, 156, 120, }, + { 1, 156, 106, 120, }, + { 0, 0, 0, 0, }, /* Prohibited setting */ + { 1, 156, 156, 120, }, + { 1, 128, 128, 96, }, + { 1, 128, 84, 96, }, + { 0, 0, 0, 0, }, /* Prohibited setting */ + { 1, 128, 128, 96, }, + { 2, 192, 192, 144, }, + { 2, 192, 128, 144, }, + { 0, 0, 0, 0, }, /* Prohibited setting */ + { 2, 192, 192, 144, }, +}; + +static const struct cpg_pll_config *cpg_pll_config __initdata; + +static +struct clk * __init r8a7795_cpg_clk_register(const struct cpg_core_clk *core, + const struct cpg_mssr_info *info, + struct clk **clks, + void __iomem *base) +{ + unsigned int idx = core->id; + const struct clk *parent; + unsigned int mult = 1; + unsigned int div = 1; + u32 value; + + pr_debug("Registering r8a7795 core clock %s id %u type %u\n", + core->name, idx, core->type); + WARN_ON(idx >= info->num_total_core_clks); + WARN_ON(PTR_ERR(clks[idx]) != -ENOENT); + + parent = clks[core->parent]; + if (IS_ERR(parent)) + return ERR_CAST(parent); + + switch (core->type) { + case CLK_TYPE_GEN3_MAIN: + div = cpg_pll_config->extal_div; + break; + + case CLK_TYPE_GEN3_PLL0: + /* + * PLL0 is a configurable multiplier clock. Register it as a + * fixed factor clock for now as there's no generic multiplier + * clock implementation and we currently have no need to change + * the multiplier value. + */ + value = readl(base + CPG_PLL0CR); + mult = ((value >> 24) & 0x3f) + 1; + break; + + case CLK_TYPE_GEN3_PLL1: + mult = cpg_pll_config->pll1_mult; + break; + + case CLK_TYPE_GEN3_PLL2: + /* + * PLL2 is a configurable multiplier clock. Register it as a + * fixed factor clock for now as there's no generic multiplier + * clock implementation and we currently have no need to change + * the multiplier value. + */ + value = readl(base + CPG_PLL2CR); + mult = ((value >> 24) & 0x3f) + 1; + break; + + case CLK_TYPE_GEN3_PLL3: + mult = cpg_pll_config->pll3_mult; + break; + + case CLK_TYPE_GEN3_PLL4: + mult = cpg_pll_config->pll4_mult; + break; + + default: + pr_err("%s: Unsupported clock type %u\n", __func__, core->type); + return ERR_PTR(-EINVAL); + } + + return clk_register_fixed_factor(NULL, core->name, + __clk_get_name(parent), 0, mult, div); +} + +static const struct cpg_mssr_info r8a7795_cpg_mssr_info __initconst = { + /* Core Clocks */ + .core_clks = r8a7795_core_clks, + .num_core_clks = ARRAY_SIZE(r8a7795_core_clks), + .last_dt_core_clk = LAST_DT_CORE_CLOCK, + .num_total_core_clks = MOD_CLK_BASE, + + /* Module Clocks */ + .mod_clks = r8a7795_mod_clks, + .num_mod_clks = ARRAY_SIZE(r8a7795_mod_clks), + .num_hw_mod_clks = 12 * 32, + + /* Critical Module Clocks */ + .crit_mod_clks = r8a7795_crit_mod_clks, + .num_crit_mod_clks = ARRAY_SIZE(r8a7795_crit_mod_clks), + + /* Callbacks */ + .cpg_clk_register = r8a7795_cpg_clk_register, +}; + +static void __init r8a7795_cpg_mssr_init(struct device_node *np) +{ + struct regmap *regmap; + u32 reg, cpg_mode; + + regmap = syscon_regmap_lookup_by_phandle(np, "renesas,modemr"); + if (IS_ERR(regmap) || + of_property_read_u32_index(np, "renesas,modemr", 1, ®) || + regmap_read(regmap, reg, &cpg_mode)) { + pr_err("%s: failed to parse renesas,modemr\n", np->full_name); + return; + } + + cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)]; + if (!cpg_pll_config->extal_div) { + pr_err("%s: Prohibited setting (cpg_mode=0x%x)\n", + __func__, cpg_mode); + return; + } + + cpg_mssr_probe(np, &r8a7795_cpg_mssr_info); +} +CLK_OF_DECLARE(r8a7795_cpg_mssr, "renesas,r8a7795-cpg-mssr", + r8a7795_cpg_mssr_init);
Add a new R-Car H3 Clock Pulse Generator / Module Standby and Software Reset driver, using the new CPG/MSSR driver core. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- v4: - New. --- drivers/clk/shmobile/Makefile | 3 +- drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c | 373 ++++++++++++++++++++++++++++ 2 files changed, 375 insertions(+), 1 deletion(-) create mode 100644 drivers/clk/shmobile/clk-r8a7795-cpg-mssr.c