| Message ID | 51FA6FCA.3050900@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/01/2013 05:25 PM, Nishanth Menon wrote: > On 07/31/2013 05:07 AM, Tero Kristo wrote: >> On 07/30/2013 09:40 PM, Nishanth Menon wrote: >>> On 07/23/2013 02:20 AM, Tero Kristo wrote: >>>> Some devices require their clocks to be available with a specific >>>> dev-id con-id mapping. With DT, the clocks can be found by default >>>> only with their name, or alternatively through the device node of >>>> the consumer. With drivers, that don't support DT fully yet, add >>>> mechanism to register specific clock names. >>>> >>>> Signed-off-by: Tero Kristo <t-kristo@ti.com> >>> >>> with this, should it not be enough to add clocks=<&phandle> >> >> Don't know, I am not an expert on DT. :) > > That is the usage - > Documentation/devicetree/bindings/clock/clock-bindings.txt > >> >>> >>> I am not sure I understand what specific drivers should need to carry >>> this "old hack" forward. More importantly, why is it preferable to carry >>> the hack forward rather than fixing the drivers. >> >> At least the GP timer seems to need this, and I don't want to touch / >> verify all the potential drivers touched by this so it is easier to >> provide a (semi) temporary tweak. > > I see that GP timer nodes seem to be already device tree converted, at > least I see the nodes in SoC.dtsi Even if those exist, they don't seem to work. The kernel crashes without the alias nodes as of now. > > Do we know what is going on for these that need these temporary devices? > can we do a special node property for these? > > I think the entire problem is coz of > timer->fclk = clk_get(NULL, omap_hwmod_get_main_clk(oh)); in case of > even of_populated. I guess so, clk_get tries to look for improper clk which does not exist. Might be a bug with hwmod data. > > if we can get rid of usage of omap_hwmod_get_main_clk by catching them > with [1], then we can force the drivers to pick up based on device node > clocks= property. > > It might be easier to fix 1 driver - timer, rather than introduce am33x, > omap4, omap5 dra7 specific "SoC clk driver". > > with that this entire patch becomes redundant. It is not that simple. Looking at the architectures this set supports, I see clock alias nodes at least for following drivers: - GPT timer - USB - DCAN - EMAC - VPFE - UART - SSI - DSS - security - MMC - MCBSP - MCSPI I am _not_ going to fix all of these during the initial phase. :P But yes, eventually these should go away. > > > [1] > diff --git a/arch/arm/mach-omap2/omap_hwmod.c > b/arch/arm/mach-omap2/omap_hwmod.c > index da26659..2e90ab4 100644 > --- a/arch/arm/mach-omap2/omap_hwmod.c > +++ b/arch/arm/mach-omap2/omap_hwmod.c > @@ -4153,5 +4153,10 @@ const char *omap_hwmod_get_main_clk(struct > omap_hwmod *oh) > if (!oh) > return NULL; > > + if (!of_have_populated_dt()) { > + WARN(1, "%s hwmod clk node read with OF?:FIXME!\n", > + oh->name); > + } > + > return oh->main_clk; > }
On 08/01/2013 10:18 AM, Tero Kristo wrote: > On 08/01/2013 05:25 PM, Nishanth Menon wrote: >> On 07/31/2013 05:07 AM, Tero Kristo wrote: >>> On 07/30/2013 09:40 PM, Nishanth Menon wrote: >>>> On 07/23/2013 02:20 AM, Tero Kristo wrote: [..] >> if we can get rid of usage of omap_hwmod_get_main_clk by catching them >> with [1], then we can force the drivers to pick up based on device node >> clocks= property. >> >> It might be easier to fix 1 driver - timer, rather than introduce am33x, >> omap4, omap5 dra7 specific "SoC clk driver". >> >> with that this entire patch becomes redundant. > > It is not that simple. Looking at the architectures this set supports, I > see clock alias nodes at least for following drivers: > > - GPT timer > - USB > - DCAN > - EMAC > - VPFE > - UART > - SSI > - DSS > - security > - MMC > - MCBSP > - MCSPI > > I am _not_ going to fix all of these during the initial phase. :P > > But yes, eventually these should go away. How many of these are needed to boot? what functionality do we expect with the series -> we can constraint saying that remaining drivers should fix themselves the right way, else dont have the feature - example cpufreq- fix it the right way, or wont see the feature enabled. introducing a "way out" for all of these just invites more guys to screw around claiming "it was done before - see here".. lets just fix the darned basic ones, refuse to provide "way out" and let the others fix themselves.
On 08/01/2013 06:24 PM, Nishanth Menon wrote: > On 08/01/2013 10:18 AM, Tero Kristo wrote: >> On 08/01/2013 05:25 PM, Nishanth Menon wrote: >>> On 07/31/2013 05:07 AM, Tero Kristo wrote: >>>> On 07/30/2013 09:40 PM, Nishanth Menon wrote: >>>>> On 07/23/2013 02:20 AM, Tero Kristo wrote: > [..] >>> if we can get rid of usage of omap_hwmod_get_main_clk by catching them >>> with [1], then we can force the drivers to pick up based on device node >>> clocks= property. >>> >>> It might be easier to fix 1 driver - timer, rather than introduce am33x, >>> omap4, omap5 dra7 specific "SoC clk driver". >>> >>> with that this entire patch becomes redundant. >> >> It is not that simple. Looking at the architectures this set supports, I >> see clock alias nodes at least for following drivers: >> >> - GPT timer >> - USB >> - DCAN >> - EMAC >> - VPFE >> - UART >> - SSI >> - DSS >> - security >> - MMC >> - MCBSP >> - MCSPI >> >> I am _not_ going to fix all of these during the initial phase. :P >> >> But yes, eventually these should go away. > > How many of these are needed to boot? what functionality do we expect > with the series -> we can constraint saying that remaining drivers > should fix themselves the right way, else dont have the feature - > example cpufreq- fix it the right way, or wont see the feature enabled. > > introducing a "way out" for all of these just invites more guys to screw > around claiming "it was done before - see here".. > > lets just fix the darned basic ones, refuse to provide "way out" and let > the others fix themselves. > Yea, that would be one way. I guess someone like Tony should comment on this.
* Tero Kristo <t-kristo@ti.com> [130801 08:37]: > On 08/01/2013 06:24 PM, Nishanth Menon wrote: > >On 08/01/2013 10:18 AM, Tero Kristo wrote: > >>On 08/01/2013 05:25 PM, Nishanth Menon wrote: > >>>On 07/31/2013 05:07 AM, Tero Kristo wrote: > >>>>On 07/30/2013 09:40 PM, Nishanth Menon wrote: > >>>>>On 07/23/2013 02:20 AM, Tero Kristo wrote: > >[..] > >>>if we can get rid of usage of omap_hwmod_get_main_clk by catching them > >>>with [1], then we can force the drivers to pick up based on device node > >>>clocks= property. > >>> > >>>It might be easier to fix 1 driver - timer, rather than introduce am33x, > >>>omap4, omap5 dra7 specific "SoC clk driver". > >>> > >>>with that this entire patch becomes redundant. > >> > >>It is not that simple. Looking at the architectures this set supports, I > >>see clock alias nodes at least for following drivers: > >> > >>- GPT timer > >>- USB > >>- DCAN > >>- EMAC > >>- VPFE > >>- UART > >>- SSI > >>- DSS > >>- security > >>- MMC > >>- MCBSP > >>- MCSPI > >> > >>I am _not_ going to fix all of these during the initial phase. :P > >> > >>But yes, eventually these should go away. > > > >How many of these are needed to boot? what functionality do we expect > >with the series -> we can constraint saying that remaining drivers > >should fix themselves the right way, else dont have the feature - > >example cpufreq- fix it the right way, or wont see the feature enabled. > > > >introducing a "way out" for all of these just invites more guys to screw > >around claiming "it was done before - see here".. > > > >lets just fix the darned basic ones, refuse to provide "way out" and let > >the others fix themselves. > > > > Yea, that would be one way. I guess someone like Tony should comment > on this. Well how about add some dev_warns so we can keep things working and know which ones to fix? Otherwise it seems that things will not work properly for many devices. Regards, Tony
diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index da26659..2e90ab4 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -4153,5 +4153,10 @@ const char *omap_hwmod_get_main_clk(struct omap_hwmod *oh) if (!oh) return NULL; + if (!of_have_populated_dt()) { + WARN(1, "%s hwmod clk node read with OF?:FIXME!\n", + oh->name); + } + return oh->main_clk;