Messages in this thread | | | Subject | Re: [Patch 1/3] ARM: dts: am43xx: add support for clkout1 clock | From | Tero Kristo <> | Date | Thu, 31 Oct 2019 16:06:59 +0200 |
| |
On 30/10/2019 21:59, Benoit Parrot wrote: > Tony Lindgren <tony@atomide.com> wrote on Wed [2019-Oct-23 08:56:57 -0700]: >> * Tony Lindgren <tony@atomide.com> [191022 16:56]: >>> * Tero Kristo <t-kristo@ti.com> [191022 16:48]: >>>> On 22/10/2019 19:21, Benoit Parrot wrote: >>>>> Tony Lindgren <tony@atomide.com> wrote on Tue [2019-Oct-22 08:48:16 -0700]: >>>>>> * Benoit Parrot <bparrot@ti.com> [191016 18:47]: >>>>>>> --- a/arch/arm/boot/dts/am43xx-clocks.dtsi >>>>>>> +++ b/arch/arm/boot/dts/am43xx-clocks.dtsi >>>>>>> @@ -704,6 +704,60 @@ >>>>>>> ti,bit-shift = <8>; >>>>>>> reg = <0x2a48>; >>>>>>> }; >>>>>>> + >>>>>>> + clkout1_osc_div_ck: clkout1_osc_div_ck { >>>>>>> + #clock-cells = <0>; >>>>>>> + compatible = "ti,divider-clock"; >>>>>>> + clocks = <&sys_clkin_ck>; >>>>>>> + ti,bit-shift = <20>; >>>>>>> + ti,max-div = <4>; >>>>>>> + reg = <0x4100>; >>>>>>> + }; >>>>>> >>>>>> Here too please describe why the clock names are not generic. >>>>> >>>>> Tero originally had this patch in the kernel so this is somewhat of a >>>>> revert. Since these "clock" were removed. If the name syntax is no longer >>>>> valid for some reason, then I will need a little more informations to >>>>> proceed. >>>>> >>>>> Tero, can you assist here? >>>> >>>> This one is just following the naming convention of the rest of the clocks >>>> atm. >>>> >>>> If we need to fix all the underscore name clocks, that requires pretty much >>>> complete revamp of both the dts data + clock data under the clock driver, >>>> and it is not backwards compatible either. How should we tackle that one? >>>> >>>> We could maybe add support code in kernel to do s/-/_/g for the "new" clocks >>>> so that their parent-child relationships would be retained, and then convert >>>> the clocks in phases. >>> >>> Well some of them can be fixed by configuring things based >>> on the compatible value and then the node name can be just >>> clock like it should be. >>> >>> Here too one option would be to add custom compatibles like: >>> >>> compatible = "ti,clkout1-osc-div", "ti,divider-clock"; >>> >>> And then have match data configure the rest. >>> >>> The other option would be to have lookup tables in the clock >>> driver based on the SoC and reg address. >>> >>> This is a hidden mine though.. We've hit it already several times, >>> and any dts clean-up effort has a chance of breaking things. >> >> Hmm maybe in this case just doing this is enough: >> >> clkout1_osc_div_ck: clock@4100 { >> ... >> } > > But then we would end up with 6 clock node with the same name "clock@4100", > doesn't pose a problem somewhere?
Yeah, clk core would not know which one to use then.
> > Tero? > > Benoit > >> >> Or do all the TI clocks we have have a dependency to the >> node naming?
This is a feature of clock core. Clock parents need to have distinct names, otherwise it won't work.
-Tero -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
| |