| Message ID | 20240410-mbly-olb-v1-3-335e496d7be3@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add Mobileye EyeQ system controller support (clk, reset, pinctrl) | expand |
On 10/04/2024 19:12, Théo Lebrun wrote: > Add bindings for EyeQ6L and EyeQ6H reset controllers. > > Some controllers host a single domain, meaning a single cell is enough. > We do not enforce reg-names for such nodes. > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > --- > .../bindings/reset/mobileye,eyeq5-reset.yaml | 88 ++++++++++++++++++---- > MAINTAINERS | 1 + > 2 files changed, 74 insertions(+), 15 deletions(-) > > diff --git a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml > index 062b4518347b..799bcf15bed9 100644 > --- a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml > +++ b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml > @@ -4,11 +4,13 @@ > $id: http://devicetree.org/schemas/reset/mobileye,eyeq5-reset.yaml# > $schema: http://devicetree.org/meta-schemas/core.yaml# > > -title: Mobileye EyeQ5 reset controller > +title: Mobileye EyeQ reset controller > > description: > - The EyeQ5 reset driver handles three reset domains. Its registers live in a > - shared region called OLB. > + EyeQ reset controller handles one or more reset domains. They live in shared > + regions called OLB. EyeQ5 and EyeQ6L host one OLB each, each with one reset > + instance. EyeQ6H hosts 7 OLB regions; three of those (west, east, > + accelerator) host reset controllers. West and east are duplicates. > > maintainers: > - Grégory Clement <gregory.clement@bootlin.com> > @@ -17,27 +19,83 @@ maintainers: > > properties: > compatible: > - const: mobileye,eyeq5-reset > + enum: > + - mobileye,eyeq5-reset > + - mobileye,eyeq6l-reset > + - mobileye,eyeq6h-we-reset > + - mobileye,eyeq6h-acc-reset > > - reg: > - maxItems: 3 > + reg: true Same mistakes. Please open existing bindings with multiple variants, e.g. some Qualcomm, and take a look how it is done there. Best regards, Krzysztof
Hello, On Thu Apr 11, 2024 at 8:14 AM CEST, Krzysztof Kozlowski wrote: > On 10/04/2024 19:12, Théo Lebrun wrote: > > Add bindings for EyeQ6L and EyeQ6H reset controllers. > > > > Some controllers host a single domain, meaning a single cell is enough. > > We do not enforce reg-names for such nodes. > > > > Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> > > --- > > .../bindings/reset/mobileye,eyeq5-reset.yaml | 88 ++++++++++++++++++---- > > MAINTAINERS | 1 + > > 2 files changed, 74 insertions(+), 15 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml > > index 062b4518347b..799bcf15bed9 100644 > > --- a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml > > +++ b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml > > @@ -4,11 +4,13 @@ > > $id: http://devicetree.org/schemas/reset/mobileye,eyeq5-reset.yaml# > > $schema: http://devicetree.org/meta-schemas/core.yaml# > > > > -title: Mobileye EyeQ5 reset controller > > +title: Mobileye EyeQ reset controller > > > > description: > > - The EyeQ5 reset driver handles three reset domains. Its registers live in a > > - shared region called OLB. > > + EyeQ reset controller handles one or more reset domains. They live in shared > > + regions called OLB. EyeQ5 and EyeQ6L host one OLB each, each with one reset > > + instance. EyeQ6H hosts 7 OLB regions; three of those (west, east, > > + accelerator) host reset controllers. West and east are duplicates. > > > > maintainers: > > - Grégory Clement <gregory.clement@bootlin.com> > > @@ -17,27 +19,83 @@ maintainers: > > > > properties: > > compatible: > > - const: mobileye,eyeq5-reset > > + enum: > > + - mobileye,eyeq5-reset > > + - mobileye,eyeq6l-reset > > + - mobileye,eyeq6h-we-reset > > + - mobileye,eyeq6h-acc-reset > > > > - reg: > > - maxItems: 3 > > + reg: true > > Same mistakes. Please open existing bindings with multiple variants, > e.g. some Qualcomm, and take a look how it is done there. Thanks for the pointer to good example, that is useful! So if we take one random binding matching Documentation/devicetree/bindings/clock/qcom,*.yaml and that contains the "reg-names" string, we see: reg: items: - description: LPASS qdsp6ss register - description: LPASS top-cc register reg-names: items: - const: qdsp6ss - const: top_cc I don't understand one thing; this doesn't tell you: You can provide 2 MMIO blocks, which must be qdsp6ss and top_cc. But it tells you: Block zero must be qdsp6ss. Block one must be top_cc. If we do that I do not get the point of reg-names; we put more information in our devicetree that is in any case imposed. This is why I went with a different approach looking like: reg: minItems: 2 maxItems: 2 reg-names: minItems: 2 maxItems: 2 items: enum: [ d0, d1 ] I know this is not perfect, but at least you don't enforce an order for no reason. If "items: const..." approach should be taken, then I'll remove reg-names which bring no benefit. Thanks Krzysztof, -- Théo Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
On 11/04/2024 16:04, Théo Lebrun wrote: > Hello, > > On Thu Apr 11, 2024 at 8:14 AM CEST, Krzysztof Kozlowski wrote: >> On 10/04/2024 19:12, Théo Lebrun wrote: >>> Add bindings for EyeQ6L and EyeQ6H reset controllers. >>> >>> Some controllers host a single domain, meaning a single cell is enough. >>> We do not enforce reg-names for such nodes. >>> >>> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> >>> --- >>> .../bindings/reset/mobileye,eyeq5-reset.yaml | 88 ++++++++++++++++++---- >>> MAINTAINERS | 1 + >>> 2 files changed, 74 insertions(+), 15 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml >>> index 062b4518347b..799bcf15bed9 100644 >>> --- a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml >>> +++ b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml >>> @@ -4,11 +4,13 @@ >>> $id: http://devicetree.org/schemas/reset/mobileye,eyeq5-reset.yaml# >>> $schema: http://devicetree.org/meta-schemas/core.yaml# >>> >>> -title: Mobileye EyeQ5 reset controller >>> +title: Mobileye EyeQ reset controller >>> >>> description: >>> - The EyeQ5 reset driver handles three reset domains. Its registers live in a >>> - shared region called OLB. >>> + EyeQ reset controller handles one or more reset domains. They live in shared >>> + regions called OLB. EyeQ5 and EyeQ6L host one OLB each, each with one reset >>> + instance. EyeQ6H hosts 7 OLB regions; three of those (west, east, >>> + accelerator) host reset controllers. West and east are duplicates. >>> >>> maintainers: >>> - Grégory Clement <gregory.clement@bootlin.com> >>> @@ -17,27 +19,83 @@ maintainers: >>> >>> properties: >>> compatible: >>> - const: mobileye,eyeq5-reset >>> + enum: >>> + - mobileye,eyeq5-reset >>> + - mobileye,eyeq6l-reset >>> + - mobileye,eyeq6h-we-reset >>> + - mobileye,eyeq6h-acc-reset >>> >>> - reg: >>> - maxItems: 3 >>> + reg: true >> >> Same mistakes. Please open existing bindings with multiple variants, >> e.g. some Qualcomm, and take a look how it is done there. > > Thanks for the pointer to good example, that is useful! So if we take > one random binding matching > Documentation/devicetree/bindings/clock/qcom,*.yaml and that contains > the "reg-names" string, we see: > > reg: > items: > - description: LPASS qdsp6ss register > - description: LPASS top-cc register > > reg-names: > items: > - const: qdsp6ss > - const: top_cc > > I don't understand one thing; this doesn't tell you: > > You can provide 2 MMIO blocks, which must be qdsp6ss and top_cc. No, it tells you exactly this, with difference: s/can/must/ > > But it tells you: > > Block zero must be qdsp6ss. > Block one must be top_cc. > > If we do that I do not get the point of reg-names; we put more > information in our devicetree that is in any case imposed. Same old argument. Order is not flexible. Order is fixed. Why do you need names? I don't need, it's purely your optional choice. Maybe you find it more readable, up to you. > > This is why I went with a different approach looking like: > > reg: > minItems: 2 > maxItems: 2 > reg-names: > minItems: 2 > maxItems: 2 > items: > enum: [ d0, d1 ] No, order is fixed. > > I know this is not perfect, but at least you don't enforce an order for > no reason. If "items: const..." approach should be taken, then I'll > remove reg-names which bring no benefit. You can remove it, you can keep it, whatever makes code more readable, but order is fixed. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml index 062b4518347b..799bcf15bed9 100644 --- a/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml +++ b/Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml @@ -4,11 +4,13 @@ $id: http://devicetree.org/schemas/reset/mobileye,eyeq5-reset.yaml# $schema: http://devicetree.org/meta-schemas/core.yaml# -title: Mobileye EyeQ5 reset controller +title: Mobileye EyeQ reset controller description: - The EyeQ5 reset driver handles three reset domains. Its registers live in a - shared region called OLB. + EyeQ reset controller handles one or more reset domains. They live in shared + regions called OLB. EyeQ5 and EyeQ6L host one OLB each, each with one reset + instance. EyeQ6H hosts 7 OLB regions; three of those (west, east, + accelerator) host reset controllers. West and east are duplicates. maintainers: - Grégory Clement <gregory.clement@bootlin.com> @@ -17,27 +19,83 @@ maintainers: properties: compatible: - const: mobileye,eyeq5-reset + enum: + - mobileye,eyeq5-reset + - mobileye,eyeq6l-reset + - mobileye,eyeq6h-we-reset + - mobileye,eyeq6h-acc-reset - reg: - maxItems: 3 + reg: true - reg-names: - items: - - const: d0 - - const: d1 - - const: d2 + reg-names: true "#reset-cells": - const: 2 description: - The first cell is the domain (0 to 2 inclusive) and the second one is the - reset index inside that domain. + First cell is domain, second is reset index inside that domain. If + controller has a single domain, first cell is implicitly zero. + enum: [ 1, 2 ] required: - compatible - reg - - reg-names - "#reset-cells" +allOf: + # EyeQ5 and EyeQ6L have multiple domains, other compatibles have one. + # Multiple domains means named resources and two reset cells. + # Single domain means a single unnamed resource and one reset cell. + - if: + properties: + compatible: + enum: + - mobileye,eyeq5-reset + - mobileye,eyeq6l-reset + then: + properties: + "#reset-cells": + const: 2 + required: + - reg-names + else: + properties: + reg: + maxItems: 1 + reg-names: false + "#reset-cells": + const: 1 + + # EyeQ5 has three domains. + - if: + properties: + compatible: + contains: + const: mobileye,eyeq5-reset + then: + properties: + reg: + minItems: 3 + maxItems: 3 + reg-names: + minItems: 3 + maxItems: 3 + items: + enum: [ d0, d1, d2 ] + + # EyeQ6L has two domains. + - if: + properties: + compatible: + contains: + const: mobileye,eyeq6l-reset + then: + properties: + reg: + minItems: 2 + maxItems: 2 + reg-names: + minItems: 2 + maxItems: 2 + items: + enum: [ d0, d1 ] + additionalProperties: false diff --git a/MAINTAINERS b/MAINTAINERS index f5a488331b38..42553da10be9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -14927,6 +14927,7 @@ L: linux-mips@vger.kernel.org S: Maintained F: Documentation/devicetree/bindings/clock/mobileye,eyeq5-clk.yaml F: Documentation/devicetree/bindings/mips/mobileye.yaml +F: Documentation/devicetree/bindings/reset/mobileye,eyeq5-reset.yaml F: Documentation/devicetree/bindings/soc/mobileye/ F: arch/mips/boot/dts/mobileye/ F: arch/mips/configs/eyeq5_defconfig
Add bindings for EyeQ6L and EyeQ6H reset controllers. Some controllers host a single domain, meaning a single cell is enough. We do not enforce reg-names for such nodes. Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com> --- .../bindings/reset/mobileye,eyeq5-reset.yaml | 88 ++++++++++++++++++---- MAINTAINERS | 1 + 2 files changed, 74 insertions(+), 15 deletions(-)