| Message ID | 20200828130602.42203-1-andre.przywara@arm.com |
|---|---|
| Headers | show |
| Series | dt-bindings: Convert SP805 to Json-schema (and fix users) | expand |
On Fri, Aug 28, 2020 at 3:06 PM Andre Przywara <andre.przywara@arm.com> wrote: > The SP805 binding sets the name for the actual watchdog clock to > "wdog_clk" (with an underscore). > > Change the name in the DTs for ARM Ltd. platforms to match that. The > Linux and U-Boot driver use the *first* clock for this purpose anyway, > so it does not break anything. > > For MPS2 we only specify one clock so far, but the binding requires > two clocks to be named. > > In practice, Linux would pick a clock named "apb_pclk" for the bus > clock, and the Linux and U-Boot SP805 driver would use the first clock > to derive the actual watchdog counter frequency. So since currently both > are the very same clock, we can just double the clock reference, and add > the correct clock-names, to match the binding. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On 8/28/2020 6:05 AM, Andre Przywara wrote: > The SP805 binding sets the name for the actual watchdog clock to > "wdog_clk" (with an underscore). > > Change the name in the DTs for Broadcom platforms to match that. The > Linux and U-Boot driver use the *first* clock for this purpose anyway, > so it does not break anything. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi | 2 +- > arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi b/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi > index 15f7b0ed3836..6a5fc55f0a4e 100644 > --- a/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi > +++ b/arch/arm64/boot/dts/broadcom/northstar2/ns2.dtsi > @@ -576,7 +576,7 @@ > reg = <0x66090000 0x1000>; > interrupts = <GIC_SPI 406 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&iprocslow>, <&iprocslow>; > - clock-names = "wdogclk", "apb_pclk"; > + clock-names = "wdog_clk", "apb_pclk"; > }; > > gpio_g: gpio@660a0000 { > diff --git a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi > index 0098dfdef96c..b425b12c3ed2 100644 > --- a/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi > +++ b/arch/arm64/boot/dts/broadcom/stingray/stingray.dtsi > @@ -438,7 +438,7 @@ > reg = <0x000c0000 0x1000>; > interrupts = <GIC_SPI 189 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&hsls_25m_div2_clk>, <&hsls_div4_clk>; > - clock-names = "wdogclk", "apb_pclk"; > + clock-names = "wdog_clk", "apb_pclk"; > timeout-sec = <60>; > }; > > Although not currently used in the driver, this indeed should be fixed to match DT documentation and be ready for future clock support in the driver (i.e., getting clock by name). Reviewed-by: Ray Jui <ray.jui@broadcom.com>
On 8/28/2020 6:06 AM, Andre Przywara wrote: > The SP805 DT binding requires two clocks to be specified, but the > Broadcom Cygnus DT currently only specifies one clock. > > In practice, Linux would pick a clock named "apb_pclk" for the bus > clock, and the Linux and U-Boot SP805 driver would use the first clock > to derive the actual watchdog counter frequency. > > Since currently both are the very same clock, we can just double the > clock reference, and add the correct clock-names, to match the binding. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > arch/arm/boot/dts/bcm-cygnus.dtsi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi > index 35bdd0969f0a..dacaef2c14ca 100644 > --- a/arch/arm/boot/dts/bcm-cygnus.dtsi > +++ b/arch/arm/boot/dts/bcm-cygnus.dtsi > @@ -234,8 +234,8 @@ > compatible = "arm,sp805" , "arm,primecell"; > reg = <0x18009000 0x1000>; > interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&axi81_clk>; > - clock-names = "apb_pclk"; > + clocks = <&axi81_clk>, <&axi81_clk>; > + clock-names = "wdog_clk", "apb_pclk"; > }; > > gpio_ccm: gpio@1800a000 { > Reviewed-by: Ray Jui <ray.jui@broadcom.com>
On 8/28/2020 6:06 AM, Andre Przywara wrote: > The SP805 binding sets the name for the actual watchdog clock to > "wdog_clk" (with an underscore). > > Change the name in the DTs for the Broadcom NSP platform to match that. > The Linux and U-Boot driver use the *first* clock for this purpose > anyway, so it does not break anything. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > arch/arm/boot/dts/bcm-nsp.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/bcm-nsp.dtsi b/arch/arm/boot/dts/bcm-nsp.dtsi > index 1333ef8be0a2..351908b4a39c 100644 > --- a/arch/arm/boot/dts/bcm-nsp.dtsi > +++ b/arch/arm/boot/dts/bcm-nsp.dtsi > @@ -438,7 +438,7 @@ > reg = <0x39000 0x1000>; > interrupts = <GIC_SPI 126 IRQ_TYPE_LEVEL_HIGH>; > clocks = <&iprocslow>, <&iprocslow>; > - clock-names = "wdogclk", "apb_pclk"; > + clock-names = "wdog_clk", "apb_pclk"; > }; > > lcpll0: lcpll0@3f100 { > Reviewed-by: Ray Jui <ray.jui@broadcom.com>
On 8/28/20 6:06 AM, Andre Przywara wrote: > The SP805 DT binding requires two clocks to be specified, but the > Broadcom Cygnus DT currently only specifies one clock. > > In practice, Linux would pick a clock named "apb_pclk" for the bus > clock, and the Linux and U-Boot SP805 driver would use the first clock > to derive the actual watchdog counter frequency. > > Since currently both are the very same clock, we can just double the > clock reference, and add the correct clock-names, to match the binding. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
On 8/28/20 6:06 AM, Andre Przywara wrote: > The SP805 binding sets the name for the actual watchdog clock to > "wdog_clk" (with an underscore). > > Change the name in the DTs for the Broadcom NSP platform to match that. > The Linux and U-Boot driver use the *first* clock for this purpose > anyway, so it does not break anything. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
On 8/28/20 6:05 AM, Andre Przywara wrote: > The SP805 binding sets the name for the actual watchdog clock to > "wdog_clk" (with an underscore). > > Change the name in the DTs for Broadcom platforms to match that. The > Linux and U-Boot driver use the *first* clock for this purpose anyway, > so it does not break anything. Not that it really matters because the driver does a devm_clk_get(&adev->dev, NULL), but yes, this should be fixed to conform to the binding. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
On 8/28/20 6:05 AM, Andre Przywara wrote: > This is an attempt to convert the SP805 watchdog DT binding to yaml. > This is done in the first patch, the remaining nine fix some DT users. > > I couldn't test any of those DT files on actual machines, but tried > to make the changes in a way that would be transparent to at least the > Linux driver. The only other SP805 DT user I could find is U-Boot, which > seems to only use a very minimal subset of the binding (just the first > clock). > I only tried to fix those DTs that were easily and reliably fixable. > AFAICT, a missing primecell compatible string, for instance, would > prevent the Linux driver from probing the device at all, so I didn't > dare to touch those DTs at all. Missing clocks are equally fatal. What is the plan for merging this series? Should Rob pick up all changes or since those are non critical changes, should we just leave it to the SoC maintainers to pick up the changes in their tree? Likewise for the SP804 timer series, what's the plan?
On Fri, Aug 28, 2020 at 1:34 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > > On 8/28/20 6:05 AM, Andre Przywara wrote: > > This is an attempt to convert the SP805 watchdog DT binding to yaml. > > This is done in the first patch, the remaining nine fix some DT users. > > > > I couldn't test any of those DT files on actual machines, but tried > > to make the changes in a way that would be transparent to at least the > > Linux driver. The only other SP805 DT user I could find is U-Boot, which > > seems to only use a very minimal subset of the binding (just the first > > clock). > > I only tried to fix those DTs that were easily and reliably fixable. > > AFAICT, a missing primecell compatible string, for instance, would > > prevent the Linux driver from probing the device at all, so I didn't > > dare to touch those DTs at all. Missing clocks are equally fatal. > > What is the plan for merging this series? Should Rob pick up all changes > or since those are non critical changes, should we just leave it to the > SoC maintainers to pick up the changes in their tree? I don't take .dts files. Either subarch maintainers can pick up individual patches or send a PR to SoC maintainers. Rob
On 8/28/20 2:28 PM, Rob Herring wrote: > On Fri, Aug 28, 2020 at 1:34 PM Florian Fainelli <f.fainelli@gmail.com> wrote: >> >> On 8/28/20 6:05 AM, Andre Przywara wrote: >>> This is an attempt to convert the SP805 watchdog DT binding to yaml. >>> This is done in the first patch, the remaining nine fix some DT users. >>> >>> I couldn't test any of those DT files on actual machines, but tried >>> to make the changes in a way that would be transparent to at least the >>> Linux driver. The only other SP805 DT user I could find is U-Boot, which >>> seems to only use a very minimal subset of the binding (just the first >>> clock). >>> I only tried to fix those DTs that were easily and reliably fixable. >>> AFAICT, a missing primecell compatible string, for instance, would >>> prevent the Linux driver from probing the device at all, so I didn't >>> dare to touch those DTs at all. Missing clocks are equally fatal. >> >> What is the plan for merging this series? Should Rob pick up all changes >> or since those are non critical changes, should we just leave it to the >> SoC maintainers to pick up the changes in their tree? > > I don't take .dts files. Either subarch maintainers can pick up > individual patches or send a PR to SoC maintainers. OK, so we are fine, to say make sure this all lands in v5.10-rc1 at some point and the warnings should no longer exist by then?
On Fri, 28 Aug 2020 14:06:00 +0100, Andre Przywara <andre.przywara@arm.com> wrote: > The SP805 DT binding requires two clocks to be specified, but the > Broadcom Cygnus DT currently only specifies one clock. > > In practice, Linux would pick a clock named "apb_pclk" for the bus > clock, and the Linux and U-Boot SP805 driver would use the first clock > to derive the actual watchdog counter frequency. > > Since currently both are the very same clock, we can just double the > clock reference, and add the correct clock-names, to match the binding. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- Applied to qspi-fixes, thanks! -- Florian
On Fri, 28 Aug 2020 14:06:01 +0100, Andre Przywara <andre.przywara@arm.com> wrote: > The SP805 binding sets the name for the actual watchdog clock to > "wdog_clk" (with an underscore). > > Change the name in the DTs for the Broadcom NSP platform to match that. > The Linux and U-Boot driver use the *first* clock for this purpose > anyway, so it does not break anything. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- Applied to qspi-fixes, thanks! -- Florian
On Fri, 28 Aug 2020 14:05:55 +0100, Andre Przywara <andre.przywara@arm.com> wrote: > The SP805 binding sets the name for the actual watchdog clock to > "wdog_clk" (with an underscore). > > Change the name in the DTs for Broadcom platforms to match that. The > Linux and U-Boot driver use the *first* clock for this purpose anyway, > so it does not break anything. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- Applied to devicetree-arm64/next, thanks! -- Florian
On 8/30/2020 8:55 PM, Florian Fainelli wrote: > On Fri, 28 Aug 2020 14:06:00 +0100, Andre Przywara <andre.przywara@arm.com> wrote: >> The SP805 DT binding requires two clocks to be specified, but the >> Broadcom Cygnus DT currently only specifies one clock. >> >> In practice, Linux would pick a clock named "apb_pclk" for the bus >> clock, and the Linux and U-Boot SP805 driver would use the first clock >> to derive the actual watchdog counter frequency. >> >> Since currently both are the very same clock, we can just double the >> clock reference, and add the correct clock-names, to match the binding. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- > > Applied to qspi-fixes, thanks! Applied to devicetree/next actually, likewise for the next patch.
On Fri, Aug 28, 2020 at 02:05:56PM +0100, Andre Przywara wrote: > The SP805 binding sets the order of the clock-names to be: "wdog_clk", > "apb_pclk" (in exactly that order). > > Change the order in the DTs for Freescale platforms to match that. The > two clocks given in all nodes are actually the same, so that does not > change any behaviour. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> Applied, thanks.
On 28/08/2020 22:32, Florian Fainelli wrote: Hi, Florian, thanks for queueing the Broadcom specific patches! > On 8/28/20 2:28 PM, Rob Herring wrote: >> On Fri, Aug 28, 2020 at 1:34 PM Florian Fainelli <f.fainelli@gmail.com> wrote: >>> >>> On 8/28/20 6:05 AM, Andre Przywara wrote: >>>> This is an attempt to convert the SP805 watchdog DT binding to yaml. >>>> This is done in the first patch, the remaining nine fix some DT users. >>>> >>>> I couldn't test any of those DT files on actual machines, but tried >>>> to make the changes in a way that would be transparent to at least the >>>> Linux driver. The only other SP805 DT user I could find is U-Boot, which >>>> seems to only use a very minimal subset of the binding (just the first >>>> clock). >>>> I only tried to fix those DTs that were easily and reliably fixable. >>>> AFAICT, a missing primecell compatible string, for instance, would >>>> prevent the Linux driver from probing the device at all, so I didn't >>>> dare to touch those DTs at all. Missing clocks are equally fatal. >>> >>> What is the plan for merging this series? Should Rob pick up all changes >>> or since those are non critical changes, should we just leave it to the >>> SoC maintainers to pick up the changes in their tree? >> >> I don't take .dts files. Either subarch maintainers can pick up >> individual patches or send a PR to SoC maintainers. > > OK, so we are fine, to say make sure this all lands in v5.10-rc1 at some > point and the warnings should no longer exist by then? So yes, I would be very grateful if subsystem maintainers take this at their discretion. For once, I didn't actually change anything in the binding, so most things were already slightly wrong according to the .txt binding, just nobody realised or cared. So those .dts files changes are actually independent and justified even without patch 01/10. Secondly, there are already so many warnings in many .dts files at the moment, that (in the worst case) a few more - for a brief period of time - do not really matter. But at the end it will improve the situation. Rob, if you are fine with the actual binding, I would try to pursue the remaining subsystem maintainers to get the .dts changes merged. Thanks, Andre.
On Fri, Aug 28, 2020 at 9:34 PM Florian Fainelli <f.fainelli@gmail.com> wrote: > On 8/28/20 6:05 AM, Andre Przywara wrote: > What is the plan for merging this series? Should Rob pick up all changes > or since those are non critical changes, should we just leave it to the > SoC maintainers to pick up the changes in their tree? What about André just send a pull request to the ARM SoC maintainers for the whole thing? Yours, Linus Walleij
On 9/4/2020 1:58 AM, Linus Walleij wrote: > On Fri, Aug 28, 2020 at 9:34 PM Florian Fainelli <f.fainelli@gmail.com> wrote: >> On 8/28/20 6:05 AM, Andre Przywara wrote: > >> What is the plan for merging this series? Should Rob pick up all changes >> or since those are non critical changes, should we just leave it to the >> SoC maintainers to pick up the changes in their tree? > > What about André just send a pull request to the ARM SoC maintainers > for the whole thing? I already applied some of the patches, if we got that route please CC me so I can drop them from my local queue. Thanks
On 04/09/2020 16:29, Florian Fainelli wrote: Hi, > On 9/4/2020 1:58 AM, Linus Walleij wrote:>> On Fri, Aug 28, 2020 at 9:34 PM Florian Fainelli >> <f.fainelli@gmail.com> wrote: >>> On 8/28/20 6:05 AM, Andre Przywara wrote: >> >>> What is the plan for merging this series? Should Rob pick up all changes >>> or since those are non critical changes, should we just leave it to the >>> SoC maintainers to pick up the changes in their tree? >> >> What about André just send a pull request to the ARM SoC maintainers >> for the whole thing? > > I already applied some of the patches, if we got that route please CC me > so I can drop them from my local queue. Thanks I would for sure drop these from any PR. Rob, are you happy with the actual binding conversion? If you are willing to take it as it is (Viresh has already acked), I could then split off the DT fixes and either chase the maintainers or send ARM SoC a PR. But this really depends on the binding being good. Cheers, Andre.
On 9/4/2020 8:35 AM, André Przywara wrote: > On 04/09/2020 16:29, Florian Fainelli wrote: > > Hi, > >> On 9/4/2020 1:58 AM, Linus Walleij wrote:>> On Fri, Aug 28, 2020 at 9:34 PM Florian Fainelli >>> <f.fainelli@gmail.com> wrote: >>>> On 8/28/20 6:05 AM, Andre Przywara wrote: >>> >>>> What is the plan for merging this series? Should Rob pick up all changes >>>> or since those are non critical changes, should we just leave it to the >>>> SoC maintainers to pick up the changes in their tree? >>> >>> What about André just send a pull request to the ARM SoC maintainers >>> for the whole thing? >> >> I already applied some of the patches, if we got that route please CC me >> so I can drop them from my local queue. Thanks > > I would for sure drop these from any PR. > > Rob, are you happy with the actual binding conversion? If you are > willing to take it as it is (Viresh has already acked), I could then > split off the DT fixes and either chase the maintainers or send ARM SoC > a PR. But this really depends on the binding being good. We had discussed this in an another leg of this thread that starts here: https://lore.kernel.org/linux-devicetree/CAL_JsqKvcGAotS6xL7pu+wM8X33PLCQCuoaXYmWrA3j3OdoR5A@mail.gmail.com/ -- Florian
On Fri, 28 Aug 2020 14:05:52 +0100, Andre Przywara wrote: > This is an attempt to convert the SP805 watchdog DT binding to yaml. > This is done in the first patch, the remaining nine fix some DT users. > > I couldn't test any of those DT files on actual machines, but tried > to make the changes in a way that would be transparent to at least the > Linux driver. The only other SP805 DT user I could find is U-Boot, which > seems to only use a very minimal subset of the binding (just the first > clock). > I only tried to fix those DTs that were easily and reliably fixable. > AFAICT, a missing primecell compatible string, for instance, would > prevent the Linux driver from probing the device at all, so I didn't > dare to touch those DTs at all. Missing clocks are equally fatal. > > [...] I have picked 2 patches for Arm Ltd boards/models. Applied to sudeep.holla/linux (for-next/juno), thanks! [1/2] (korg_sudeep/for-next/juno, for-next/juno) arm64: dts: arm: Fix SP805 clock-names https://git.kernel.org/sudeep.holla/c/b83ded8a31 [2/2] ARM: dts: arm: Fix SP805 clocks https://git.kernel.org/sudeep.holla/c/a894c6dd56 -- Regards, Sudeep