| Message ID | 20201114135044.724385-3-daniel@0x0f.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ARM: mstar: Basic MPLL support | expand |
On Sat, 14 Nov 2020 22:50:40 +0900, Daniel Palmer wrote: > Add a binding description for the MStar/SigmaStar MPLL clock block. > > Signed-off-by: Daniel Palmer <daniel@0x0f.com> > --- > .../bindings/clock/mstar,msc313-mpll.yaml | 58 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/mstar,msc313-mpll.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
Quoting Daniel Palmer (2020-11-14 05:50:40) > Add a binding description for the MStar/SigmaStar MPLL clock block. > > Signed-off-by: Daniel Palmer <daniel@0x0f.com> > --- > .../bindings/clock/mstar,msc313-mpll.yaml | 58 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/mstar,msc313-mpll.yaml > > diff --git a/Documentation/devicetree/bindings/clock/mstar,msc313-mpll.yaml b/Documentation/devicetree/bindings/clock/mstar,msc313-mpll.yaml > new file mode 100644 > index 000000000000..9ddc1163b31b > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/mstar,msc313-mpll.yaml > @@ -0,0 +1,58 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/mstar,msc313-mpll.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MStar/Sigmastar MSC313 MPLL > + > +maintainers: > + - Daniel Palmer <daniel@thingy.jp> > + > +description: | > + The MStar/SigmaStar MSC313 and later ARMv7 chips have an MPLL block that > + takes the external xtal input and multiplies it to create a high > + frequency clock and divides that down into a number of clocks that > + peripherals use. > + > +properties: > + compatible: > + const: mstar,msc313-mpll > + > + "#clock-cells": > + const: 1 > + > + clocks: > + maxItems: 1 > + > + clock-output-names: > + minItems: 8 > + maxItems: 8 > + description: | > + This should provide a name for the internal PLL clock and then > + a name for each of the divided outputs. Is this necessary? > + > + reg: > + maxItems: 1 > + > +required: > + - compatible > + - "#clock-cells" > + - clocks > + - clock-output-names > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + mpll@206000 { > + compatible = "mstar,msc313-mpll"; > + reg = <0x206000 0x200>; > + #clock-cells = <1>; > + clocks = <&xtal>; > + clock-output-names = "mpll", "mpll_div_2", > + "mpll_div_3", "mpll_div_4", > + "mpll_div_5", "mpll_div_6", > + "mpll_div_7", "mpll_div_10"; It looks like it can be derived in the driver.
Hi Stephen, On Sun, 20 Dec 2020 at 12:39, Stephen Boyd <sboyd@kernel.org> wrote: > > + clock-output-names: > > + minItems: 8 > > + maxItems: 8 > > + description: | > > + This should provide a name for the internal PLL clock and then > > + a name for each of the divided outputs. > > Is this necessary? I found without the names specified in the dt probing of muxes that depend on the outputs but appear earlier didn't work. Also this same PLL layout seems to be used in some other places so eventually I was thinking this driver would get used for those PLLs with different output names. Thanks, Daniel
Quoting Daniel Palmer (2020-12-19 22:35:41) > Hi Stephen, > > On Sun, 20 Dec 2020 at 12:39, Stephen Boyd <sboyd@kernel.org> wrote: > > > + clock-output-names: > > > + minItems: 8 > > > + maxItems: 8 > > > + description: | > > > + This should provide a name for the internal PLL clock and then > > > + a name for each of the divided outputs. > > > > Is this necessary? > > I found without the names specified in the dt probing of muxes that > depend on the outputs but appear earlier didn't work. > Also this same PLL layout seems to be used in some other places so > eventually I was thinking this driver would get used for those PLLs > with different output names. Still seems like it could be auto-generated based on dev_name() + number. Now that we have a way to specify clk parents via the clocks property in DT (without any clock-names required) we should be able to avoid needing clock-output-names in general.
Hi Stephen, On Mon, 21 Dec 2020 at 03:44, Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Daniel Palmer (2020-12-19 22:35:41) > > Hi Stephen, > > > > On Sun, 20 Dec 2020 at 12:39, Stephen Boyd <sboyd@kernel.org> wrote: > > > > + clock-output-names: > > > > + minItems: 8 > > > > + maxItems: 8 > > > > + description: | > > > > + This should provide a name for the internal PLL clock and then > > > > + a name for each of the divided outputs. > > > > > > Is this necessary? > > > > I found without the names specified in the dt probing of muxes that > > depend on the outputs but appear earlier didn't work. > > Also this same PLL layout seems to be used in some other places so > > eventually I was thinking this driver would get used for those PLLs > > with different output names. > > Still seems like it could be auto-generated based on dev_name() + > number. At one point I had something similar to that where the output names were generated at probe. Without the clock outputs listed in the device tree clock muxes that source clocks from the mpll couldn't probe properly as they couldn't look up all of their parents if they probed before the mpll. Maybe I'm doing something wrong there? I couldn't find a way to always resolve all of the parents or defer the probe of the muxes until the mpll clocks are registered. Cheers, Daniel
Quoting Daniel Palmer (2020-12-21 00:51:56) > Hi Stephen, > > On Mon, 21 Dec 2020 at 03:44, Stephen Boyd <sboyd@kernel.org> wrote: > > > > Quoting Daniel Palmer (2020-12-19 22:35:41) > > > Hi Stephen, > > > > > > On Sun, 20 Dec 2020 at 12:39, Stephen Boyd <sboyd@kernel.org> wrote: > > > > > + clock-output-names: > > > > > + minItems: 8 > > > > > + maxItems: 8 > > > > > + description: | > > > > > + This should provide a name for the internal PLL clock and then > > > > > + a name for each of the divided outputs. > > > > > > > > Is this necessary? > > > > > > I found without the names specified in the dt probing of muxes that > > > depend on the outputs but appear earlier didn't work. > > > Also this same PLL layout seems to be used in some other places so > > > eventually I was thinking this driver would get used for those PLLs > > > with different output names. > > > > Still seems like it could be auto-generated based on dev_name() + > > number. > > At one point I had something similar to that where the output names > were generated at probe. > Without the clock outputs listed in the device tree clock muxes that > source clocks from the mpll couldn't probe properly as they couldn't > look up all of their parents if they probed before the mpll. > Maybe I'm doing something wrong there? I couldn't find a way to always > resolve all of the parents or defer the probe of the muxes until the > mpll clocks are registered. > The child clks should be using clk_parent_data to point to the parent clks through DT. That way the name of the clk doesn't matter except for debug purposes.
Hi Stephen, On Wed, 10 Feb 2021 at 11:29, Stephen Boyd <sboyd@kernel.org> wrote: > The child clks should be using clk_parent_data to point to the parent > clks through DT. That way the name of the clk doesn't matter except for > debug purposes. I think I get it now. I was using of_clk_parent_fill() to get the parent clocks sourced from the mpll but I seems like I should be filling out an array of struct clk_parent_data with the indices of the parents and using clk_register_composite_pdata() etc instead. Thanks! Daniel
Quoting Daniel Palmer (2021-02-10 18:28:40) > Hi Stephen, > > On Wed, 10 Feb 2021 at 11:29, Stephen Boyd <sboyd@kernel.org> wrote: > > The child clks should be using clk_parent_data to point to the parent > > clks through DT. That way the name of the clk doesn't matter except for > > debug purposes. > > I think I get it now. I was using of_clk_parent_fill() to get the > parent clocks sourced > from the mpll but I seems like I should be filling out an array of > struct clk_parent_data > with the indices of the parents and using > clk_register_composite_pdata() etc instead. > Yes that's the idea. Thanks!
diff --git a/Documentation/devicetree/bindings/clock/mstar,msc313-mpll.yaml b/Documentation/devicetree/bindings/clock/mstar,msc313-mpll.yaml new file mode 100644 index 000000000000..9ddc1163b31b --- /dev/null +++ b/Documentation/devicetree/bindings/clock/mstar,msc313-mpll.yaml @@ -0,0 +1,58 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/clock/mstar,msc313-mpll.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MStar/Sigmastar MSC313 MPLL + +maintainers: + - Daniel Palmer <daniel@thingy.jp> + +description: | + The MStar/SigmaStar MSC313 and later ARMv7 chips have an MPLL block that + takes the external xtal input and multiplies it to create a high + frequency clock and divides that down into a number of clocks that + peripherals use. + +properties: + compatible: + const: mstar,msc313-mpll + + "#clock-cells": + const: 1 + + clocks: + maxItems: 1 + + clock-output-names: + minItems: 8 + maxItems: 8 + description: | + This should provide a name for the internal PLL clock and then + a name for each of the divided outputs. + + reg: + maxItems: 1 + +required: + - compatible + - "#clock-cells" + - clocks + - clock-output-names + - reg + +additionalProperties: false + +examples: + - | + mpll@206000 { + compatible = "mstar,msc313-mpll"; + reg = <0x206000 0x200>; + #clock-cells = <1>; + clocks = <&xtal>; + clock-output-names = "mpll", "mpll_div_2", + "mpll_div_3", "mpll_div_4", + "mpll_div_5", "mpll_div_6", + "mpll_div_7", "mpll_div_10"; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 1e874fda810e..d1c98a6f9f24 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2131,6 +2131,7 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers) S: Maintained W: http://linux-chenxing.org/ F: Documentation/devicetree/bindings/arm/mstar/* +F: Documentation/devicetree/bindings/clock/mstar,msc313-mpll.yaml F: Documentation/devicetree/bindings/gpio/mstar,msc313-gpio.yaml F: arch/arm/boot/dts/mstar-* F: arch/arm/mach-mstar/
Add a binding description for the MStar/SigmaStar MPLL clock block. Signed-off-by: Daniel Palmer <daniel@0x0f.com> --- .../bindings/clock/mstar,msc313-mpll.yaml | 58 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/mstar,msc313-mpll.yaml