| Message ID | 20170812184318.10144-12-linus.walleij@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | watchdog: Consolidate FTWDT010 derivatives | expand |
On Sat, Aug 12, 2017 at 8:43 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > This adds the PCLK clock to the Aspeed watchdog blocks. > I am not directly familiar with the Aspeed clocking, but > since the IP is derived from Faraday FTWDT010 it probably > has the ability to run the watchdog on the PCLK if > desired so to obtain the frequency from it, it needs to > be present in the device tree, and for completeness the > PCLK should also be referenced and enabled anyways. > > Take this opportunity to add the "faraday,ftwdt010" > compatible as fallback to the watchdog IP blocks. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> Joel could you merge this through the Aspeed tree? I think the compatible string is completely uncontroversial (binding ACKed) to add and all should just work fine so we can slap in "EXTCLK" later as well. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 2017-08-12 at 20:43 +0200, Linus Walleij wrote: > This adds the PCLK clock to the Aspeed watchdog blocks. > I am not directly familiar with the Aspeed clocking, but > since the IP is derived from Faraday FTWDT010 it probably > has the ability to run the watchdog on the PCLK if > desired This is true for the AST2400, but not the AST2500 where the only option is EXTCLK (1MHz). > so to obtain the frequency from it, it needs to > be present in the device tree, and for completeness the > PCLK should also be referenced and enabled anyways. > > Take this opportunity to add the "faraday,ftwdt010" > compatible as fallback to the watchdog IP blocks. > > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > arch/arm/boot/dts/aspeed-g4.dtsi | 7 +++++-- > arch/arm/boot/dts/aspeed-g5.dtsi | 12 +++++++++--- > 2 files changed, 14 insertions(+), 5 deletions(-) > > diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi > index 8a04c7e2d818..23b100383c15 100644 > --- a/arch/arm/boot/dts/aspeed-g4.dtsi > +++ b/arch/arm/boot/dts/aspeed-g4.dtsi > @@ -895,16 +895,19 @@ > > }; > > > > wdt1: wdt@1e785000 { > > - compatible = "aspeed,ast2400-wdt"; > > + compatible = "aspeed,ast2400-wdt", "faraday,ftwdt010"; > > reg = <0x1e785000 0x1c>; > > interrupts = <27>; > > + clocks = <&clk_apb>; > > + clock-names = "PCLK"; > > }; > > > > wdt2: wdt@1e785020 { > > - compatible = "aspeed,ast2400-wdt"; > > + compatible = "aspeed,ast2400-wdt", "faraday,ftwdt010"; > > reg = <0x1e785020 0x1c>; > > interrupts = <27>; > > clocks = <&clk_apb>; > > + clock-names = "PCLK"; > > status = "disabled"; > > }; > > diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi > index 9cffe347b828..2322d72cd8a9 100644 > --- a/arch/arm/boot/dts/aspeed-g5.dtsi > +++ b/arch/arm/boot/dts/aspeed-g5.dtsi > @@ -1003,21 +1003,27 @@ > > > > > wdt1: wdt@1e785000 { > > - compatible = "aspeed,ast2500-wdt"; > > + compatible = "aspeed,ast2500-wdt", "faraday,ftwdt010"; > > reg = <0x1e785000 0x20>; > > interrupts = <27>; > > + clocks = <&clk_apb>; > + clock-names = "PCLK"; Given the comment above, shouldn't we be doing something like the following instead for each of the watchdogs? + faraday,use-extclk; + clock-names = "EXTCLK"; Andrew > }; > > > > wdt2: wdt@1e785020 { > > - compatible = "aspeed,ast2500-wdt"; > > + compatible = "aspeed,ast2500-wdt", "faraday,ftwdt010"; > > reg = <0x1e785020 0x20>; > > interrupts = <27>; > > + clocks = <&clk_apb>; > > + clock-names = "PCLK"; > > status = "disabled"; > > }; > > > > wdt3: wdt@1e785040 { > > - compatible = "aspeed,ast2500-wdt"; > > + compatible = "aspeed,ast2500-wdt", "faraday,ftwdt010"; > > reg = <0x1e785040 0x20>; > > + clocks = <&clk_apb>; > > + clock-names = "PCLK"; > > status = "disabled"; > > }; >
On Wed, Oct 11, 2017 at 5:48 AM, Andrew Jeffery <andrew@aj.id.au> wrote: > On Sat, 2017-08-12 at 20:43 +0200, Linus Walleij wrote: >> This adds the PCLK clock to the Aspeed watchdog blocks. >> I am not directly familiar with the Aspeed clocking, but >> since the IP is derived from Faraday FTWDT010 it probably >> has the ability to run the watchdog on the PCLK if >> desired > > This is true for the AST2400, but not the AST2500 where the only option > is EXTCLK (1MHz). The IP block/cell certainly has a PCLK even if it cannot be used to drive the watchdog timer. It is necessary to interface the SoC interconnect. >> > + clocks = <&clk_apb>; >> + clock-names = "PCLK"; > > Given the comment above, shouldn't we be doing something like the > following instead for each of the watchdogs? > > + faraday,use-extclk; > + clock-names = "EXTCLK"; So that will be added too, later, when there is a 1MHz clock to reference in the device tree. I guess after Joel's patches. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2017-10-11 at 08:32 +0200, Linus Walleij wrote: > > On Wed, Oct 11, 2017 at 5:48 AM, Andrew Jeffery <andrew@aj.id.au> wrote: > > On Sat, 2017-08-12 at 20:43 +0200, Linus Walleij wrote: > > > This adds the PCLK clock to the Aspeed watchdog blocks. > > > I am not directly familiar with the Aspeed clocking, but > > > since the IP is derived from Faraday FTWDT010 it probably > > > has the ability to run the watchdog on the PCLK if > > > desired > > > > This is true for the AST2400, but not the AST2500 where the only option > > is EXTCLK (1MHz). > > The IP block/cell certainly has a PCLK even if it cannot be used > to drive the watchdog timer. It is necessary to interface the > SoC interconnect. Hah, yep, brain-fade. Cheers, Andrew > > > > > + clocks = <&clk_apb>; > > > > > > + clock-names = "PCLK"; > > > > Given the comment above, shouldn't we be doing something like the > > following instead for each of the watchdogs? > > > > + faraday,use-extclk; > > + clock-names = "EXTCLK"; > > So that will be added too, later, when there is a 1MHz clock > to reference in the device tree. I guess after Joel's patches. > > Yours, > Linus Walleij
On Wed, Oct 11, 2017 at 4:09 AM, Linus Walleij <linus.walleij@linaro.org> wrote: > On Sat, Aug 12, 2017 at 8:43 PM, Linus Walleij <linus.walleij@linaro.org> wrote: > >> This adds the PCLK clock to the Aspeed watchdog blocks. >> I am not directly familiar with the Aspeed clocking, but >> since the IP is derived from Faraday FTWDT010 it probably >> has the ability to run the watchdog on the PCLK if >> desired so to obtain the frequency from it, it needs to >> be present in the device tree, and for completeness the >> PCLK should also be referenced and enabled anyways. >> >> Take this opportunity to add the "faraday,ftwdt010" >> compatible as fallback to the watchdog IP blocks. >> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > > Joel could you merge this through the Aspeed tree? I think > the compatible string is completely uncontroversial > (binding ACKed) to add and all should just work fine so we > can slap in "EXTCLK" later as well. How do the clock-names work? I have been writing the aspeed clk driver and updating the bindings without clock names, and instead using a identifier in phandle to reference which clock the device wants. eg: clocks = <&syscon 10>; (I'm travelling over the next few weeks so my replies might be delayed) Cheers, Joel -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" 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 12, 2017 at 5:37 AM, Joel Stanley <joel@jms.id.au> wrote: > On Wed, Oct 11, 2017 at 4:09 AM, Linus Walleij <linus.walleij@linaro.org> wrote: >> Joel could you merge this through the Aspeed tree? I think >> the compatible string is completely uncontroversial >> (binding ACKed) to add and all should just work fine so we >> can slap in "EXTCLK" later as well. > > How do the clock-names work? I have been writing the aspeed clk driver > and updating the bindings without clock names, and instead using a > identifier in phandle to reference which clock the device wants. > > eg: > > clocks = <&syscon 10>; This works fine as long as there is just one clock. clocks = <&syscon 10>, <&syscon 11>, <&syscon 12>; becomes a problem, right? clk_get() has this signature: struct clk *clk_get(struct device *dev, const char *id); So clk_get(dev, NULL); will return <&syscon 10>; How to get the rest? clocks = <&syscon 10>, <&syscon 11>, <&syscon 12>; clock-names = "FOO", "BAR", "BAZ"; clk_get(dev, "BAR"); gets the second clock. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/arm/boot/dts/aspeed-g4.dtsi b/arch/arm/boot/dts/aspeed-g4.dtsi index 8a04c7e2d818..23b100383c15 100644 --- a/arch/arm/boot/dts/aspeed-g4.dtsi +++ b/arch/arm/boot/dts/aspeed-g4.dtsi @@ -895,16 +895,19 @@ }; wdt1: wdt@1e785000 { - compatible = "aspeed,ast2400-wdt"; + compatible = "aspeed,ast2400-wdt", "faraday,ftwdt010"; reg = <0x1e785000 0x1c>; interrupts = <27>; + clocks = <&clk_apb>; + clock-names = "PCLK"; }; wdt2: wdt@1e785020 { - compatible = "aspeed,ast2400-wdt"; + compatible = "aspeed,ast2400-wdt", "faraday,ftwdt010"; reg = <0x1e785020 0x1c>; interrupts = <27>; clocks = <&clk_apb>; + clock-names = "PCLK"; status = "disabled"; }; diff --git a/arch/arm/boot/dts/aspeed-g5.dtsi b/arch/arm/boot/dts/aspeed-g5.dtsi index 9cffe347b828..2322d72cd8a9 100644 --- a/arch/arm/boot/dts/aspeed-g5.dtsi +++ b/arch/arm/boot/dts/aspeed-g5.dtsi @@ -1003,21 +1003,27 @@ wdt1: wdt@1e785000 { - compatible = "aspeed,ast2500-wdt"; + compatible = "aspeed,ast2500-wdt", "faraday,ftwdt010"; reg = <0x1e785000 0x20>; interrupts = <27>; + clocks = <&clk_apb>; + clock-names = "PCLK"; }; wdt2: wdt@1e785020 { - compatible = "aspeed,ast2500-wdt"; + compatible = "aspeed,ast2500-wdt", "faraday,ftwdt010"; reg = <0x1e785020 0x20>; interrupts = <27>; + clocks = <&clk_apb>; + clock-names = "PCLK"; status = "disabled"; }; wdt3: wdt@1e785040 { - compatible = "aspeed,ast2500-wdt"; + compatible = "aspeed,ast2500-wdt", "faraday,ftwdt010"; reg = <0x1e785040 0x20>; + clocks = <&clk_apb>; + clock-names = "PCLK"; status = "disabled"; };
This adds the PCLK clock to the Aspeed watchdog blocks. I am not directly familiar with the Aspeed clocking, but since the IP is derived from Faraday FTWDT010 it probably has the ability to run the watchdog on the PCLK if desired so to obtain the frequency from it, it needs to be present in the device tree, and for completeness the PCLK should also be referenced and enabled anyways. Take this opportunity to add the "faraday,ftwdt010" compatible as fallback to the watchdog IP blocks. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- arch/arm/boot/dts/aspeed-g4.dtsi | 7 +++++-- arch/arm/boot/dts/aspeed-g5.dtsi | 12 +++++++++--- 2 files changed, 14 insertions(+), 5 deletions(-) -- 2.13.4 -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html