Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-30 Thread Laurent Pinchart
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.

-- 
Regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-29 Thread Geert Uytterhoeven
On Thu, Oct 22, 2015 at 2:58 PM, Geert Uytterhoeven
 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 -- ge...@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 devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-26 Thread Geert Uytterhoeven
Hi Laurent,

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.

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 -- ge...@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 devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-25 Thread Laurent Pinchart
Hi Geert,

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:
> >> 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().

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-24 Thread Geert Uytterhoeven
Hi Stephen,

On Sat, Oct 24, 2015 at 3:10 AM, Stephen Boyd  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().
>>
>>   - 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 -- ge...@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 devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-23 Thread Stephen Boyd
On 10/22, Geert Uytterhoeven wrote:
> Hi Mike,
> 
> 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().
> 
>   - 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.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-22 Thread Geert Uytterhoeven
Hi Mike,

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:
>>> > 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 -- ge...@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 devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-20 Thread Geert Uytterhoeven
Hi Mike,

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:
>> > 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 -- ge...@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 devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-20 Thread Geert Uytterhoeven
Hi Mike,

On Tue, Oct 20, 2015 at 2:24 PM, Michael Turquette
 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 -- ge...@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 devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-20 Thread Michael Turquette
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 devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 5/5] [RFC] clk: shmobile: r8a7795: Add new CPG/MSSR driver

2015-10-16 Thread Geert Uytterhoeven
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 
---
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

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 ..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#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,