| Message ID | 20231227-mbly-clk-v2-3-a05db63c380f@bootlin.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Mobileye EyeQ5 clock controller | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On Wed, Dec 27, 2023 at 10:24 AM Théo Lebrun <theo.lebrun@bootlin.com> wrote: > > Add documentation to describe the "Other Logic Block" syscon. > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > --- > .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml | 44 ++++++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 45 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml > new file mode 100644 > index 000000000000..b148a49b08f1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml > @@ -0,0 +1,44 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mobileye EyeQ5 SoC system controller > + > +maintainers: > + - Grégory Clement <gregory.clement@bootlin.com> > + - Théo Lebrun <theo.lebrun@bootlin.com> > + - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> > + > +description: > + OLB ("Other Logic Block") is a hardware block grouping smaller blocks. Clocks, > + resets, pinctrl are being handled from here. I don't see resets or pinctrl in the binding. Please make it complete whether you have the driver or not. As-is, you don't need clocks to be a child node. > + > +properties: > + compatible: > + items: > + - const: mobileye,eyeq5-olb > + - const: syscon > + - const: simple-mfd > + > + reg: > + maxItems: 1 > + > + reg-io-width: > + const: 4 Why do you need this? It is not a generic block and can only ever be 1 value. > + > +required: > + - compatible > + - reg > + - reg-io-width > + > +additionalProperties: false > + > +examples: > + - | > + olb@e00000 { > + compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd"; > + reg = <0xe00000 0x400>; > + reg-io-width = <4>; Make this example complete and drop the child node example. Rob
On 27/12/2023 17:23, Théo Lebrun wrote: > Add documentation to describe the "Other Logic Block" syscon. > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > --- > .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml | 44 ++++++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 45 insertions(+) > > diff --git a/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml > new file mode 100644 > index 000000000000..b148a49b08f1 > --- /dev/null > +++ b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml > @@ -0,0 +1,44 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Mobileye EyeQ5 SoC system controller > + > +maintainers: > + - Grégory Clement <gregory.clement@bootlin.com> > + - Théo Lebrun <theo.lebrun@bootlin.com> > + - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> > + > +description: > + OLB ("Other Logic Block") is a hardware block grouping smaller blocks. Clocks, > + resets, pinctrl are being handled from here. > + > +properties: > + compatible: > + items: > + - const: mobileye,eyeq5-olb > + - const: syscon > + - const: simple-mfd > + > + reg: > + maxItems: 1 > + > + reg-io-width: > + const: 4 > + > +required: > + - compatible > + - reg > + - reg-io-width It's still wrong order of the patches. Binding should be complete, so you miss clock-controller in this patch. If you ordered them hoping there is no dependency between patches, it will not work. You still have dependency! Your next patch still depends on this, which should be clearly expressed either in cover letter or next patch. Best regards, Krzysztof
Hello, On Wed Dec 27, 2023 at 7:27 PM CET, Rob Herring wrote: > On Wed, Dec 27, 2023 at 10:24 AM Théo Lebrun <theo.lebrun@bootlin.com> wrote: > > > > Add documentation to describe the "Other Logic Block" syscon. > > > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > > --- > > .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml | 44 ++++++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 45 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml > > new file mode 100644 > > index 000000000000..b148a49b08f1 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml > > @@ -0,0 +1,44 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Mobileye EyeQ5 SoC system controller > > + > > +maintainers: > > + - Grégory Clement <gregory.clement@bootlin.com> > > + - Théo Lebrun <theo.lebrun@bootlin.com> > > + - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> > > + > > +description: > > + OLB ("Other Logic Block") is a hardware block grouping smaller blocks. Clocks, > > + resets, pinctrl are being handled from here. > > I don't see resets or pinctrl in the binding. Please make it complete > whether you have the driver or not. > > As-is, you don't need clocks to be a child node. Will do. Would it make sense to have the three drivers be a single series? Else we could have the dt-bindings be part of the base platform support series[1]. [1]: https://lore.kernel.org/lkml/20231212163459.1923041-1-gregory.clement@bootlin.com/ > > +properties: > > + compatible: > > + items: > > + - const: mobileye,eyeq5-olb > > + - const: syscon > > + - const: simple-mfd > > + > > + reg: > > + maxItems: 1 > > + > > + reg-io-width: > > + const: 4 > > Why do you need this? It is not a generic block and can only ever be 1 > value. This block is still a syscon in the end. I wanted to explicit that access width must be 4 bytes and nothing else. Does you question mean you think I should be removing it? > > +required: > > + - compatible > > + - reg > > + - reg-io-width > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + olb@e00000 { > > + compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd"; > > + reg = <0xe00000 0x400>; > > + reg-io-width = <4>; > > Make this example complete and drop the child node example. I hesitated inbetween the two options, I'll fix that on the next revision. Thanks! -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Hello, On Thu Dec 28, 2023 at 8:21 AM CET, Krzysztof Kozlowski wrote: > On 27/12/2023 17:23, Théo Lebrun wrote: > > Add documentation to describe the "Other Logic Block" syscon. > > > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > > --- > > .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml | 44 ++++++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 45 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml > > new file mode 100644 > > index 000000000000..b148a49b08f1 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml > > @@ -0,0 +1,44 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Mobileye EyeQ5 SoC system controller > > + > > +maintainers: > > + - Grégory Clement <gregory.clement@bootlin.com> > > + - Théo Lebrun <theo.lebrun@bootlin.com> > > + - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> > > + > > +description: > > + OLB ("Other Logic Block") is a hardware block grouping smaller blocks. Clocks, > > + resets, pinctrl are being handled from here. > > + > > +properties: > > + compatible: > > + items: > > + - const: mobileye,eyeq5-olb > > + - const: syscon > > + - const: simple-mfd > > + > > + reg: > > + maxItems: 1 > > + > > + reg-io-width: > > + const: 4 > > + > > +required: > > + - compatible > > + - reg > > + - reg-io-width > > It's still wrong order of the patches. Binding should be complete, so > you miss clock-controller in this patch. If you ordered them hoping > there is no dependency between patches, it will not work. You still have > dependency! Your next patch still depends on this, which should be > clearly expressed either in cover letter or next patch. Ah you read me right, I did indeed expect ordering to be enough. I'll squash all changes to mobileye,eyeq5-olb.yaml into a single commit to avoid any dependency. Thanks, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On Thu, Dec 28, 2023 at 03:57:55PM +0100, Théo Lebrun wrote: > Hello, > > On Wed Dec 27, 2023 at 7:27 PM CET, Rob Herring wrote: > > On Wed, Dec 27, 2023 at 10:24 AM Théo Lebrun <theo.lebrun@bootlin.com> wrote: > > > > > > Add documentation to describe the "Other Logic Block" syscon. > > > > > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > > > --- > > > .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml | 44 ++++++++++++++++++++++ > > > MAINTAINERS | 1 + > > > 2 files changed, 45 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml > > > new file mode 100644 > > > index 000000000000..b148a49b08f1 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml > > > @@ -0,0 +1,44 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Mobileye EyeQ5 SoC system controller > > > + > > > +maintainers: > > > + - Grégory Clement <gregory.clement@bootlin.com> > > > + - Théo Lebrun <theo.lebrun@bootlin.com> > > > + - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> > > > + > > > +description: > > > + OLB ("Other Logic Block") is a hardware block grouping smaller blocks. Clocks, > > > + resets, pinctrl are being handled from here. > > > > I don't see resets or pinctrl in the binding. Please make it complete > > whether you have the driver or not. > > > > As-is, you don't need clocks to be a child node. > > Will do. Would it make sense to have the three drivers be a single > series? You could, but doesn't have to be. Just make the binding complete. Whether you have the drivers done is up to you. > Else we could have the dt-bindings be part of the base platform > support series[1]. > > [1]: https://lore.kernel.org/lkml/20231212163459.1923041-1-gregory.clement@bootlin.com/ > > > > +properties: > > > + compatible: > > > + items: > > > + - const: mobileye,eyeq5-olb > > > + - const: syscon > > > + - const: simple-mfd > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + reg-io-width: > > > + const: 4 > > > > Why do you need this? It is not a generic block and can only ever be 1 > > value. > > This block is still a syscon in the end. I wanted to explicit that > access width must be 4 bytes and nothing else. > > Does you question mean you think I should be removing it? Yes. It can be implied by compatible. Rob
diff --git a/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml new file mode 100644 index 000000000000..b148a49b08f1 --- /dev/null +++ b/Documentation/devicetree/bindings/soc/mobileye/mobileye,eyeq5-olb.yaml @@ -0,0 +1,44 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/soc/mobileye/mobileye,eyeq5-olb.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Mobileye EyeQ5 SoC system controller + +maintainers: + - Grégory Clement <gregory.clement@bootlin.com> + - Théo Lebrun <theo.lebrun@bootlin.com> + - Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> + +description: + OLB ("Other Logic Block") is a hardware block grouping smaller blocks. Clocks, + resets, pinctrl are being handled from here. + +properties: + compatible: + items: + - const: mobileye,eyeq5-olb + - const: syscon + - const: simple-mfd + + reg: + maxItems: 1 + + reg-io-width: + const: 4 + +required: + - compatible + - reg + - reg-io-width + +additionalProperties: false + +examples: + - | + olb@e00000 { + compatible = "mobileye,eyeq5-olb", "syscon", "simple-mfd"; + reg = <0xe00000 0x400>; + reg-io-width = <4>; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 4a7bd6b40d74..d955b1e80e53 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14553,6 +14553,7 @@ M: Théo Lebrun <theo.lebrun@bootlin.com> L: linux-mips@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/mips/mobileye.yaml +F: Documentation/devicetree/bindings/soc/mobileye/ F: arch/mips/boot/dts/mobileye/ F: arch/mips/configs/generic/board-eyeq5.config F: arch/mips/generic/board-epm5.its.S
Add documentation to describe the "Other Logic Block" syscon. Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> --- .../bindings/soc/mobileye/mobileye,eyeq5-olb.yaml | 44 ++++++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 45 insertions(+)