| Message ID | 20190623123850.22584-1-Anson.Huang@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RESEND,V2,1/3] clocksource/drivers/sysctr: Add optional clock-frequency property | expand |
On 23/06/2019 14:38, Anson.Huang@nxp.com wrote: > From: Anson Huang <Anson.Huang@nxp.com> > > Systems which use platform driver model for clock driver require the > clock frequency to be supplied via device tree when system counter > driver is enabled. > > This is necessary as in the platform driver model the of_clk operations > do not work correctly because system counter driver is initialized in > early phase of system boot up, and clock driver using platform driver > model is NOT ready at that time, it will cause system counter driver > initialization failed. > > Add the optinal clock-frequency to the device tree bindings of the NXP > system counter, so the frequency can be handed in and the of_clk > operations can be skipped. Isn't it possible to create a fixed-clock and refer to it? So no need to create a specific action before calling timer_of_init() ? > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > --- > Changes since V1: > - improve commit log, no content change. > --- > Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt > index d576599..c9907a0 100644 > --- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt > +++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt > @@ -14,6 +14,11 @@ Required properties: > - clocks : Specifies the counter clock. > - clock-names: Specifies the clock's name of this module > > +Optional properties: > + > +- clock-frequency : Specifies system counter clock frequency and indicates system > + counter driver to skip clock operations. > + > Example: > > system_counter: timer@306a0000 { > @@ -22,4 +27,5 @@ Example: > clocks = <&clk_8m>; > clock-names = "per"; > interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>; > + clock-frequency = <8333333>; > }; >
Hi, Daniel > On 23/06/2019 14:38, Anson.Huang@nxp.com wrote: > > From: Anson Huang <Anson.Huang@nxp.com> > > > > Systems which use platform driver model for clock driver require the > > clock frequency to be supplied via device tree when system counter > > driver is enabled. > > > > This is necessary as in the platform driver model the of_clk > > operations do not work correctly because system counter driver is > > initialized in early phase of system boot up, and clock driver using > > platform driver model is NOT ready at that time, it will cause system > > counter driver initialization failed. > > > > Add the optinal clock-frequency to the device tree bindings of the NXP > > system counter, so the frequency can be handed in and the of_clk > > operations can be skipped. > > Isn't it possible to create a fixed-clock and refer to it? So no need to create a > specific action before calling timer_of_init() ? > As the clock must be ready before the TIMER_OF_DECLARE, so adding a CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock? The system counter's frequency are different on different platforms, so adding fixed clock in system counter driver is NOT a good idea, ONLY the DT node or the clock driver can create this fixed clock according to platforms, can you advise where to create this fixed clock is better? Thanks, Anson
On 26/06/2019 03:42, Anson Huang wrote: > Hi, Daniel > >> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote: >>> From: Anson Huang <Anson.Huang@nxp.com> >>> >>> Systems which use platform driver model for clock driver require the >>> clock frequency to be supplied via device tree when system counter >>> driver is enabled. >>> >>> This is necessary as in the platform driver model the of_clk >>> operations do not work correctly because system counter driver is >>> initialized in early phase of system boot up, and clock driver using >>> platform driver model is NOT ready at that time, it will cause system >>> counter driver initialization failed. >>> >>> Add the optinal clock-frequency to the device tree bindings of the NXP >>> system counter, so the frequency can be handed in and the of_clk >>> operations can be skipped. >> >> Isn't it possible to create a fixed-clock and refer to it? So no need to create a >> specific action before calling timer_of_init() ? >> > > As the clock must be ready before the TIMER_OF_DECLARE, so adding a CLK_OF_DECLARE_DRIVER in > clock driver to ONLY register a fixed-clock? The system counter's frequency are different on different > platforms, so adding fixed clock in system counter driver is NOT a good idea, ONLY the DT node or the > clock driver can create this fixed clock according to platforms, can you advise where to create this fixed > clock is better? Can you point me to a DT with the "nxp,sysctr-timer" ?
Hi, Daniel > On 26/06/2019 03:42, Anson Huang wrote: > > Hi, Daniel > > > >> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote: > >>> From: Anson Huang <Anson.Huang@nxp.com> > >>> > >>> Systems which use platform driver model for clock driver require the > >>> clock frequency to be supplied via device tree when system counter > >>> driver is enabled. > >>> > >>> This is necessary as in the platform driver model the of_clk > >>> operations do not work correctly because system counter driver is > >>> initialized in early phase of system boot up, and clock driver using > >>> platform driver model is NOT ready at that time, it will cause > >>> system counter driver initialization failed. > >>> > >>> Add the optinal clock-frequency to the device tree bindings of the > >>> NXP system counter, so the frequency can be handed in and the of_clk > >>> operations can be skipped. > >> > >> Isn't it possible to create a fixed-clock and refer to it? So no need > >> to create a specific action before calling timer_of_init() ? > >> > > > > As the clock must be ready before the TIMER_OF_DECLARE, so adding a > > CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock? > > The system counter's frequency are different on different platforms, > > so adding fixed clock in system counter driver is NOT a good idea, > > ONLY the DT node or the clock driver can create this fixed clock according to > platforms, can you advise where to create this fixed clock is better? > > Can you point me to a DT with the "nxp,sysctr-timer" ? The DT node of system counter is new added in 3/3 of this patch series, also can be found from below link: https://patchwork.kernel.org/patch/11011703/ thanks, Anson
On 27/06/2019 02:43, Anson Huang wrote: > Hi, Daniel > >> On 26/06/2019 03:42, Anson Huang wrote: >>> Hi, Daniel >>> >>>> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote: >>>>> From: Anson Huang <Anson.Huang@nxp.com> >>>>> >>>>> Systems which use platform driver model for clock driver require the >>>>> clock frequency to be supplied via device tree when system counter >>>>> driver is enabled. >>>>> >>>>> This is necessary as in the platform driver model the of_clk >>>>> operations do not work correctly because system counter driver is >>>>> initialized in early phase of system boot up, and clock driver using >>>>> platform driver model is NOT ready at that time, it will cause >>>>> system counter driver initialization failed. >>>>> >>>>> Add the optinal clock-frequency to the device tree bindings of the >>>>> NXP system counter, so the frequency can be handed in and the of_clk >>>>> operations can be skipped. >>>> >>>> Isn't it possible to create a fixed-clock and refer to it? So no need >>>> to create a specific action before calling timer_of_init() ? >>>> >>> >>> As the clock must be ready before the TIMER_OF_DECLARE, so adding a >>> CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock? >>> The system counter's frequency are different on different platforms, >>> so adding fixed clock in system counter driver is NOT a good idea, >>> ONLY the DT node or the clock driver can create this fixed clock according to >> platforms, can you advise where to create this fixed clock is better? >> >> Can you point me to a DT with the "nxp,sysctr-timer" ? > > The DT node of system counter is new added in 3/3 of this patch series, also can be found > from below link: > https://patchwork.kernel.org/patch/11011703/ Sorry, I was unclear. I meant a patch with the timer defined using a clock as defined currently in the binding (no clock-frequency).
Hi, Daniel > On 27/06/2019 02:43, Anson Huang wrote: > > Hi, Daniel > > > >> On 26/06/2019 03:42, Anson Huang wrote: > >>> Hi, Daniel > >>> > >>>> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote: > >>>>> From: Anson Huang <Anson.Huang@nxp.com> > >>>>> > >>>>> Systems which use platform driver model for clock driver require > >>>>> the clock frequency to be supplied via device tree when system > >>>>> counter driver is enabled. > >>>>> > >>>>> This is necessary as in the platform driver model the of_clk > >>>>> operations do not work correctly because system counter driver is > >>>>> initialized in early phase of system boot up, and clock driver > >>>>> using platform driver model is NOT ready at that time, it will > >>>>> cause system counter driver initialization failed. > >>>>> > >>>>> Add the optinal clock-frequency to the device tree bindings of the > >>>>> NXP system counter, so the frequency can be handed in and the > >>>>> of_clk operations can be skipped. > >>>> > >>>> Isn't it possible to create a fixed-clock and refer to it? So no > >>>> need to create a specific action before calling timer_of_init() ? > >>>> > >>> > >>> As the clock must be ready before the TIMER_OF_DECLARE, so adding a > >>> CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock? > >>> The system counter's frequency are different on different platforms, > >>> so adding fixed clock in system counter driver is NOT a good idea, > >>> ONLY the DT node or the clock driver can create this fixed clock > >>> according to > >> platforms, can you advise where to create this fixed clock is better? > >> > >> Can you point me to a DT with the "nxp,sysctr-timer" ? > > > > The DT node of system counter is new added in 3/3 of this patch > > series, also can be found from below link: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc > > > hwork.kernel.org%2Fpatch%2F11011703%2F&data=02%7C01%7Canson. > huang% > > > 40nxp.com%7C8b9519ecceb346712be808d6fad675e4%7C686ea1d3bc2b4c6f > a92cd99 > > > c5c301635%7C0%7C0%7C636972196338405582&sdata=sOQQzDFxoCqe > VuHFuYPHh > > F8Bdj2Zu9WS7Go%2FV9lrWa8%3D&reserved=0 > > Sorry, I was unclear. I meant a patch with the timer defined using a clock as > defined currently in the binding (no clock-frequency). OK, for i.MX8MM, we use clocks, check below patch series: https://patchwork.kernel.org/patch/11008519/ code piece as below: + system_counter: timer@306a0000 { + compatible = "nxp,sysctr-timer"; + reg = <0x306a0000 0x30000>; + interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>, + <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clk IMX8MM_CLK_SYS_CTR>; + clock-names = "per"; + }; Thanks, Anson.
Hi Anson, On 27/06/2019 10:11, Anson Huang wrote: > Hi, Daniel > >> On 27/06/2019 02:43, Anson Huang wrote: >>> Hi, Daniel >>> >>>> On 26/06/2019 03:42, Anson Huang wrote: >>>>> Hi, Daniel >>>>> >>>>>> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote: >>>>>>> From: Anson Huang <Anson.Huang@nxp.com> >>>>>>> >>>>>>> Systems which use platform driver model for clock driver require >>>>>>> the clock frequency to be supplied via device tree when system >>>>>>> counter driver is enabled. >>>>>>> >>>>>>> This is necessary as in the platform driver model the of_clk >>>>>>> operations do not work correctly because system counter driver is >>>>>>> initialized in early phase of system boot up, and clock driver >>>>>>> using platform driver model is NOT ready at that time, it will >>>>>>> cause system counter driver initialization failed. >>>>>>> >>>>>>> Add the optinal clock-frequency to the device tree bindings of the >>>>>>> NXP system counter, so the frequency can be handed in and the >>>>>>> of_clk operations can be skipped. >>>>>> >>>>>> Isn't it possible to create a fixed-clock and refer to it? So no >>>>>> need to create a specific action before calling timer_of_init() ? >>>>>> >>>>> >>>>> As the clock must be ready before the TIMER_OF_DECLARE, so adding a >>>>> CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock? >>>>> The system counter's frequency are different on different platforms, >>>>> so adding fixed clock in system counter driver is NOT a good idea, >>>>> ONLY the DT node or the clock driver can create this fixed clock >>>>> according to >>>> platforms, can you advise where to create this fixed clock is better? >>>> >>>> Can you point me to a DT with the "nxp,sysctr-timer" ? >>> >>> The DT node of system counter is new added in 3/3 of this patch >>> series, also can be found from below link: >>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc >>> >> hwork.kernel.org%2Fpatch%2F11011703%2F&data=02%7C01%7Canson. >> huang% >>> >> 40nxp.com%7C8b9519ecceb346712be808d6fad675e4%7C686ea1d3bc2b4c6f >> a92cd99 >>> >> c5c301635%7C0%7C0%7C636972196338405582&sdata=sOQQzDFxoCqe >> VuHFuYPHh >>> F8Bdj2Zu9WS7Go%2FV9lrWa8%3D&reserved=0 >> >> Sorry, I was unclear. I meant a patch with the timer defined using a clock as >> defined currently in the binding (no clock-frequency). > > OK, for i.MX8MM, we use clocks, check below patch series: > > https://patchwork.kernel.org/patch/11008519/ > > code piece as below: > > + system_counter: timer@306a0000 { > + compatible = "nxp,sysctr-timer"; > + reg = <0x306a0000 0x30000>; > + interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>, > + <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clk IMX8MM_CLK_SYS_CTR>; > + clock-names = "per"; > + }; Thanks, The fixed-clock can help to keep the code and the DT definition for the timer untouched as the 'clocks' above will refer to it. But that means we describe a fake clock. So it is up to you to decide if you want to stick the clock-frequency or use a fixed-clock.
On 23/06/2019 14:38, Anson.Huang@nxp.com wrote: > From: Anson Huang <Anson.Huang@nxp.com> > > Systems which use platform driver model for clock driver require the > clock frequency to be supplied via device tree when system counter > driver is enabled. > > This is necessary as in the platform driver model the of_clk operations > do not work correctly because system counter driver is initialized in > early phase of system boot up, and clock driver using platform driver > model is NOT ready at that time, it will cause system counter driver > initialization failed. > > Add the optinal clock-frequency to the device tree bindings of the NXP > system counter, so the frequency can be handed in and the of_clk > operations can be skipped. > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > --- > Changes since V1: > - improve commit log, no content change. > --- > Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt > index d576599..c9907a0 100644 > --- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt > +++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt > @@ -14,6 +14,11 @@ Required properties: > - clocks : Specifies the counter clock. > - clock-names: Specifies the clock's name of this module > > +Optional properties: > + > +- clock-frequency : Specifies system counter clock frequency and indicates system > + counter driver to skip clock operations. > + Shouldn't it be required and mutually exclusive with clocks/clock-names? > Example: > > system_counter: timer@306a0000 { > @@ -22,4 +27,5 @@ Example: > clocks = <&clk_8m>; > clock-names = "per"; > interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>; > + clock-frequency = <8333333>; > }; >
Hi, Daniel > On 23/06/2019 14:38, Anson.Huang@nxp.com wrote: > > From: Anson Huang <Anson.Huang@nxp.com> > > > > Systems which use platform driver model for clock driver require the > > clock frequency to be supplied via device tree when system counter > > driver is enabled. > > > > This is necessary as in the platform driver model the of_clk > > operations do not work correctly because system counter driver is > > initialized in early phase of system boot up, and clock driver using > > platform driver model is NOT ready at that time, it will cause system > > counter driver initialization failed. > > > > Add the optinal clock-frequency to the device tree bindings of the NXP > > system counter, so the frequency can be handed in and the of_clk > > operations can be skipped. > > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com> > > --- > > Changes since V1: > > - improve commit log, no content change. > > --- > > Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6 > > ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt > > b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt > > index d576599..c9907a0 100644 > > --- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt > > +++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt > > @@ -14,6 +14,11 @@ Required properties: > > - clocks : Specifies the counter clock. > > - clock-names: Specifies the clock's name of this module > > > > +Optional properties: > > + > > +- clock-frequency : Specifies system counter clock frequency and indicates > system > > + counter driver to skip clock operations. > > + > > Shouldn't it be required and mutually exclusive with clocks/clock-names? > Yes, make sense, I ever thought about it when doing this patch, but eventually I picked the optional...will fix it in next version. Thanks, Anson > > Example: > > > > system_counter: timer@306a0000 { > > @@ -22,4 +27,5 @@ Example: > > clocks = <&clk_8m>; > > clock-names = "per"; > > interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>; > > + clock-frequency = <8333333>; > > }; > > > >
diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt index d576599..c9907a0 100644 --- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt +++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt @@ -14,6 +14,11 @@ Required properties: - clocks : Specifies the counter clock. - clock-names: Specifies the clock's name of this module +Optional properties: + +- clock-frequency : Specifies system counter clock frequency and indicates system + counter driver to skip clock operations. + Example: system_counter: timer@306a0000 { @@ -22,4 +27,5 @@ Example: clocks = <&clk_8m>; clock-names = "per"; interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>; + clock-frequency = <8333333>; };