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@xxxxxxxxxxx> >>> --- >>> .../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@xxxxxxxxxxx> >>> @@ -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
