Convert content of the altera socfpga.txt to match clock bindings for
the Arria10 SoC devicetrees. Currently all altr,* bindings appear as
error at dtbs_check, since these bindings are only written in .txt
format.
Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
---
.../bindings/clock/altr,socfpga-a10.yaml | 107 ++++++++++++++++++
1 file changed, 107 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml
diff --git a/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml
new file mode 100644
index 000000000..795826f53
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml
@@ -0,0 +1,107 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/altr,socfpga-a10.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Device Tree Clock bindings for Altera's SoCFPGA platform
+
+maintainers:
+ - TODO
+
+description:
+ This binding uses the common clock binding[1].
+
+ [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+properties:
+ compatible:
+ description: |
+ shall be one of the following
+ - "altr,socfpga-a10-pll-clock" - for a PLL clock
+ - "altr,socfpga-a10-perip-clk" - The peripheral clock divided from the
+ PLL clock.
+ - "altr,socfpga-a10-gate-clk" - Clocks that directly feed peripherals
+ and can get gated.
+ enum:
+ - altr,socfpga-a10-pll-clock
+ - altr,socfpga-a10-perip-clk
+ - altr,socfpga-a10-gate-clk
+
+ reg:
+ description: |
+ shall be the control register offset from CLOCK_MANAGER's base for the
+ clock.
+ maxItems: 1
+
+ clocks:
+ description: |
+ shall be the input parent clock phandle for the clock. This is either an
+ oscillator or a pll output.
+ minItems: 1
+ maxItems: 5
+
+ '#clock-cells':
+ description: from common clock binding, shall be set to 0.
+ maxItems: 1
+
+ fixed-divider:
+ description: if clocks have a fixed divider value, use this property.
+ minimum: 1
+ maximum: 16
+
+ clk-gate:
+ description: |
+ for "socfpga-a10-gate-clk", clk-gate contains the gating register and the
+ bit index.
+ minItems: 2
+
+ div-reg:
+ description: |
+ for "socfpga-a10-gate-clk" and "socfpga-a10-periph-clk", div-reg contains
+ the divider register, bit shift, and width.
+ minItems: 3
+ maxItems: 3
+
+ clk-phase:
+ description: |
+ for the sdmmc_clk, contains the value of the clock phase that controls
+ the SDMMC CIU clock. The first value is the clk_sample(smpsel), and the
+ second value is the cclk_in_drv(drvsel). The clk-phase is used to enable
+ the correct hold/delay times that is needed for the SD/MMC CIU clock. The
+ values of both can be 0-315 degrees, in 45 degree increments.
+ minItems: 1
+
+required:
+ - compatible
+ - clocks
+ - '#clock-cells'
+
+oneOf:
+ - items:
+ - required:
+ - reg
+ - required:
+ - div-reg
+
+additionalProperties: false
+
+examples:
+ - |
+ main_pll: main_pll@40 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #clock-cells = <0>;
+ compatible = "altr,socfpga-a10-pll-clock";
+ clocks = <&osc1>, <&cb_intosc_ls_clk>,
+ <&f2s_free_clk>;
+ reg = <0x40>;
+
+ main_noc_base_clk: main_noc_base_clk {
+ compatible = "altr,socfpga-a10-perip-clk";
+ div-reg = <0x140 16 11>;
+ clocks = <&periph_pll>;
+ #clock-cells = <0>;
+ };
+ };
+...
--
2.25.1On Sun, Oct 20, 2024 at 07:40:17PM +0000, Lothar Rubusch wrote: > Convert content of the altera socfpga.txt to match clock bindings for > the Arria10 SoC devicetrees. Currently all altr,* bindings appear as > error at dtbs_check, since these bindings are only written in .txt > format. > Please use subject prefixes matching the subsystem. You can get them for example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com> > --- > .../bindings/clock/altr,socfpga-a10.yaml | 107 ++++++++++++++++++ > 1 file changed, 107 insertions(+) > create mode 100644 Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml > > diff --git a/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml > new file mode 100644 > index 000000000..795826f53 > --- /dev/null > +++ b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml > @@ -0,0 +1,107 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/clock/altr,socfpga-a10.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Device Tree Clock bindings for Altera's SoCFPGA platform This wasn't tested or you have some very, very old dtschema. > + > +maintainers: > + - TODO We should not be taking unmaintained stuff. > + > +description: > + This binding uses the common clock binding[1]. > + > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt Drop description or describe the hardware. > + > +properties: > + compatible: > + description: | > + shall be one of the following > + - "altr,socfpga-a10-pll-clock" - for a PLL clock > + - "altr,socfpga-a10-perip-clk" - The peripheral clock divided from the > + PLL clock. > + - "altr,socfpga-a10-gate-clk" - Clocks that directly feed peripherals > + and can get gated. Drop description. > + enum: > + - altr,socfpga-a10-pll-clock > + - altr,socfpga-a10-perip-clk > + - altr,socfpga-a10-gate-clk Why are you adding bindings per clock? Usually that's a no-go, you should be describing here clock controller unit. Best regards, Krzysztof
On Mon, Oct 21, 2024 at 9:05 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Sun, Oct 20, 2024 at 07:40:17PM +0000, Lothar Rubusch wrote:
> > Convert content of the altera socfpga.txt to match clock bindings for
> > the Arria10 SoC devicetrees. Currently all altr,* bindings appear as
> > error at dtbs_check, since these bindings are only written in .txt
> > format.
> >
>
> Please use subject prefixes matching the subsystem. You can get them for
> example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
>
> > Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> > ---
> > .../bindings/clock/altr,socfpga-a10.yaml | 107 ++++++++++++++++++
> > 1 file changed, 107 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml
> > new file mode 100644
> > index 000000000..795826f53
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml
> > @@ -0,0 +1,107 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/clock/altr,socfpga-a10.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Device Tree Clock bindings for Altera's SoCFPGA platform
>
> This wasn't tested or you have some very, very old dtschema.
>
>
> > +
> > +maintainers:
> > + - TODO
>
> We should not be taking unmaintained stuff.
>
This is just a trigger here. Basically, I have no probelm in placing
my own name here. AFAIR Mr. Dinh Nguyen has his name on the other
intel/altera related files, so I'm not sure who decides that. Please
let me know.
Basically this particular patch is related to my initial questions
(cover letter):
1.) Documentation/devicetree/bindings:
Executing the following find...
$ find ./Documentation/devicetree/bindings -name socfpga-\*.txt
...shows 4 text files describing "altr," bindings. I sketch-implemented
the clock binding and could reduce some of my dtbs_check warnings. So, my
questions is, if this is the right way? Shall I try to write .yaml files
for all 4 of them, too? Related to that, who will be maintainer?
2.) Some bindings, e.g. the Silabs clock generator seem to have no
driver, thus show up as warning:
compatible = "silabs,si5338";
IMHO it is most likely rather to be probed/loaded in the SPL of the
bootloader. Is it problematic to keep those declarations (showing up as
warning in dtbs_check) or how to deal with them?
3.) Please, give me some feedback if the DT and binding adjustments are
going into total wrong direction, or where I may do better. If it is ok,
and acceptable, or what is still missing. I tried to split them, to
allow for better single integration / discussion let me know if this is
ok, too.
> > +
> > +description:
> > + This binding uses the common clock binding[1].
> > +
> > + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
>
> Drop description or describe the hardware.
Ok (the description was taken as content from the corresponding .txt file)
> > +
> > +properties:
> > + compatible:
> > + description: |
> > + shall be one of the following
> > + - "altr,socfpga-a10-pll-clock" - for a PLL clock
> > + - "altr,socfpga-a10-perip-clk" - The peripheral clock divided from the
> > + PLL clock.
> > + - "altr,socfpga-a10-gate-clk" - Clocks that directly feed peripherals
> > + and can get gated.
>
> Drop description.
OK (dito)
> > + enum:
> > + - altr,socfpga-a10-pll-clock
> > + - altr,socfpga-a10-perip-clk
> > + - altr,socfpga-a10-gate-clk
>
> Why are you adding bindings per clock? Usually that's a no-go, you
> should be describing here clock controller unit.
as above, taken from .txt - So, I see there seems to be some interest
to convert the .txt files into .yaml files. I will have a look at the
other altr, ....txt -> .yaml cases, but will need to do more readings
first. This may become a different patch set, then.
On 24/10/2024 08:10, Lothar Rubusch wrote: > On Mon, Oct 21, 2024 at 9:05 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >> >> On Sun, Oct 20, 2024 at 07:40:17PM +0000, Lothar Rubusch wrote: >>> Convert content of the altera socfpga.txt to match clock bindings for >>> the Arria10 SoC devicetrees. Currently all altr,* bindings appear as >>> error at dtbs_check, since these bindings are only written in .txt >>> format. >>> >> >> Please use subject prefixes matching the subsystem. You can get them for >> example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory >> your patch is touching. For bindings, the preferred subjects are >> explained here: >> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters >> >>> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com> >>> --- >>> .../bindings/clock/altr,socfpga-a10.yaml | 107 ++++++++++++++++++ >>> 1 file changed, 107 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml >>> new file mode 100644 >>> index 000000000..795826f53 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml >>> @@ -0,0 +1,107 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/clock/altr,socfpga-a10.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Device Tree Clock bindings for Altera's SoCFPGA platform >> >> This wasn't tested or you have some very, very old dtschema. >> >> >>> + >>> +maintainers: >>> + - TODO >> >> We should not be taking unmaintained stuff. >> > > This is just a trigger here. Basically, I have no probelm in placing > my own name here. AFAIR Mr. Dinh Nguyen has his name on the other > intel/altera related files, so I'm not sure who decides that. Please > let me know. > > Basically this particular patch is related to my initial questions > (cover letter): > 1.) Documentation/devicetree/bindings: > Executing the following find... > $ find ./Documentation/devicetree/bindings -name socfpga-\*.txt > ...shows 4 text files describing "altr," bindings. I sketch-implemented > the clock binding and could reduce some of my dtbs_check warnings. So, my > questions is, if this is the right way? Shall I try to write .yaml files > for all 4 of them, too? Related to that, who will be maintainer? Whoever is interested in that hardware. Platform maintainer, device maintainer. > > 2.) Some bindings, e.g. the Silabs clock generator seem to have no > driver, thus show up as warning: > compatible = "silabs,si5338"; > IMHO it is most likely rather to be probed/loaded in the SPL of the > bootloader. Is it problematic to keep those declarations (showing up as > warning in dtbs_check) or how to deal with them? Sorry, I don't get the problem. > > 3.) Please, give me some feedback if the DT and binding adjustments are > going into total wrong direction, or where I may do better. If it is ok, > and acceptable, or what is still missing. I tried to split them, to > allow for better single integration / discussion let me know if this is > ok, too. I still don't understand. Nothing here is different than with every other platform. > > >>> + >>> +description: >>> + This binding uses the common clock binding[1]. >>> + >>> + [1] Documentation/devicetree/bindings/clock/clock-bindings.txt >> >> Drop description or describe the hardware. > > Ok (the description was taken as content from the corresponding .txt file) What corresponding txt file? You are adding new binding. Are you saying you duplicated bindings instead of doing conversion? git log -p -- Documentation/devicetree | grep -i convert Best regards, Krzysztof
On Thu, Oct 24, 2024 at 8:24 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 24/10/2024 08:10, Lothar Rubusch wrote: > > On Mon, Oct 21, 2024 at 9:05 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > >> [...] > >>> diff --git a/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml > >>> new file mode 100644 > >>> index 000000000..795826f53 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml > >>> @@ -0,0 +1,107 @@ > >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>> +%YAML 1.2 [...] > What corresponding txt file? You are adding new binding. Are you saying > you duplicated bindings instead of doing conversion? > > git log -p -- Documentation/devicetree | grep -i convert Please, try the following: $ find ./Documentation/devicetree/bindings -name socfpga-\*.txt ./Documentation/devicetree/bindings/net/socfpga-dwmac.txt ./Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt ./Documentation/devicetree/bindings/arm/altera/socfpga-sdram-edac.txt ./Documentation/devicetree/bindings/arm/altera/socfpga-system.txt Currently, bindings described in these .txt files are not covered by bindings check. Is it supposed to be like that, or is this just something "historical"? I would appreciate to take the opportunity to learn more about bindings and bindings check, and may try to convert the files to .yaml. Is this ok or do you have plans with those files? If noone speaks up, I'll put my name under the maintainers in the .yaml. Let's see what happens (I hope it's ok, due to the high demand in cyclone5 these days...). Best regards, Lothar
On 25/10/2024 08:59, Lothar Rubusch wrote: > On Thu, Oct 24, 2024 at 8:24 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >> >> On 24/10/2024 08:10, Lothar Rubusch wrote: >>> On Mon, Oct 21, 2024 at 9:05 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >>>> > [...] >>>>> diff --git a/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml >>>>> new file mode 100644 >>>>> index 000000000..795826f53 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml >>>>> @@ -0,0 +1,107 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>> +%YAML 1.2 > [...] >> What corresponding txt file? You are adding new binding. Are you saying >> you duplicated bindings instead of doing conversion? >> >> git log -p -- Documentation/devicetree | grep -i convert This would answer you... You would see hundreds of patches showing what to do, including patches from me, Rob or Conor. > > Please, try the following: I know this, what are you asking about? > $ find ./Documentation/devicetree/bindings -name socfpga-\*.txt > ./Documentation/devicetree/bindings/net/socfpga-dwmac.txt > ./Documentation/devicetree/bindings/edac/socfpga-eccmgr.txt > ./Documentation/devicetree/bindings/arm/altera/socfpga-sdram-edac.txt > ./Documentation/devicetree/bindings/arm/altera/socfpga-system.txt > > Currently, bindings described in these .txt files are not covered by > bindings check. Is it supposed to be like that, or is this just > something "historical"? These are TXT files, that's nothing to do with dtschema. Everything in TXT is just unconverted binding. Best regards, Krzysztof
On Sun, 20 Oct 2024 19:40:17 +0000, Lothar Rubusch wrote:
> Convert content of the altera socfpga.txt to match clock bindings for
> the Arria10 SoC devicetrees. Currently all altr,* bindings appear as
> error at dtbs_check, since these bindings are only written in .txt
> format.
>
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
> ---
> .../bindings/clock/altr,socfpga-a10.yaml | 107 ++++++++++++++++++
> 1 file changed, 107 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml: fixed-divider: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml: clk-gate: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml: div-reg: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml: clk-phase: missing type definition
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml: title: "Device Tree Clock bindings for Altera's SoCFPGA platform" should not be valid under {'pattern': '([Bb]inding| [Ss]chema)'}
hint: Everything is a binding/schema, no need to say it. Describe what hardware the binding is for.
from schema $id: http://devicetree.org/meta-schemas/base.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/altr,socfpga-a10.yaml: properties:#clock-cells: 'maxItems' is not one of ['description', 'deprecated', 'const', 'enum', 'minimum', 'maximum', 'multipleOf', 'default', '$ref', 'oneOf']
from schema $id: http://devicetree.org/meta-schemas/core.yaml#
Documentation/devicetree/bindings/clock/altr,socfpga-a10.example.dts:25.11-24: Warning (reg_format): /example-0/main_pll@40:reg: property has invalid length (4 bytes) (#address-cells == 1, #size-cells == 1)
Documentation/devicetree/bindings/clock/altr,socfpga-a10.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/clock/altr,socfpga-a10.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/clock/altr,socfpga-a10.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/clock/altr,socfpga-a10.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/clock/altr,socfpga-a10.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/clock/altr,socfpga-a10.example.dtb: main_pll@40: '#address-cells', '#size-cells', 'main_noc_base_clk' do not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/clock/altr,socfpga-a10.yaml#
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241020194028.2272371-13-l.rubusch@gmail.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
© 2016 - 2024 Red Hat, Inc.