Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add optional '#nvmem-cell-cells' #89

Closed
wants to merge 2 commits into from

Conversation

mwalle
Copy link

@mwalle mwalle commented Oct 23, 2022

As requested in https://lore.kernel.org/linux-devicetree/20220912192038.GA1661550-robh@kernel.org/, add the property in dtschema.

The second patch is my try to also add support to the dtschema tooling. I'm not sure, if the entry for the 'nvmem-cells' in phandle_args ever worked, because the type of nvmem-cells wasn't a phandle-array. Thus fixup_phandles() seems to skip that property.

Add support for having arguments to a phandle to a nvmem-cell. To be
backwards compatible, we have to make the '#nvmem-cell-cells' property
optional. If it's missing no arguments are expected.

Signed-off-by: Michael Walle <michael@walle.cc>
'nvmem-cells' can now have a '#nvmem-cell-cells' property. Handle that
case correctly. If it is missing, no arguments are expected.
_get_phandle_arg_size() already handles both cases correctly.

Signed-off-by: Michael Walle <michael@walle.cc>
@rmilecki
Copy link

I wasn't aware of this work and meanwhile sent a patch, see:
[PATCH dt-schema.git] schemas: add NVMEM cell with #nvmem-cell-cells

Hopefully we can get that #nvmem-cell-cells shaped & accepted in some form :)

$ref: "cell.yaml#/array"
oneOf:
- $ref: "cell.yaml#/array"
- $ref: "/schemas/types.yaml#/definitions/phandle-array"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

meta-schemas are what check the schemas. What you are adding is what belongs in the schema, and is already there (in the kernel tree).

@@ -12,6 +12,8 @@ properties:
nvmem-cells:
$ref: "cell.yaml#/array"

'#nvmem-cell-cells': true

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed in meta-schema. What you do need is:

#nvmem-cell-cells:
enum: [ 0, 1 ]

In nvmem.yaml in the kernel. (Really, I'd like to move those here.)

The exact constraints depends on if you want to globally define how many and what the cells contain.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any plans for working on this? I guess after almost 2 years not really.

@rmilecki
Copy link

@robherring: I'm confused by your mention & suggestion to move that stuff into kernel.

I believe (unless I got wrong impression) that you actually suggested adding #nvmem-cell-cells to the dt-schema, please see:
Re: [PATCH v2 15/20] dt-bindings: nvmem: add YAML schema for the sl28 vpd layout

I'd prefer to see the schema in dtschema rather than the kernel.

Does my
[PATCH V2 dt-schema.git] schemas: add NVMEM cell with #nvmem-cell-cells
by any chance handles #nvmem-cell-cells the right way? I decided to add dtschema/schemas/nvmem/nvmem-cell.yaml binding (so out of meta-schema).

@robherring
Copy link
Member

@rmilecki My comment was the dtb.py change is what is needed here. I would like to see nvmem.yaml and nvmem-consumer.yaml moved from the kernel to dtschema, but that's a separate issue. For now, it's fine to add #nvmem-cell-cells to kernel's nvmem.yaml.

@rmilecki
Copy link

We now have #nvmem-cell-cells in multiple in-Linux bindings (fixed-layout.yaml, u-boot,env.yaml, onie,tlv-layout.yaml, kontron,sl28-vpd.yaml, brcm,nvram.yaml).

Does this pull request still make any sense? Or should it be closed?

@robherring
Copy link
Member

I believe at least the dtb.py part is needed still.

@@ -262,7 +262,6 @@ def fdt_scan_node(fdt, nodename, offset):
'msi-ranges': '#interrupt-cells',
'gpio-ranges': 3,

'nvmem-cells': None,
Copy link

@krzk krzk Jan 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Independently, Rafał Miłecki reported a bug regarding nvmem-cells and I narrowed it to this line. Later I found this pull request.

I think this part is needed along with Rob's comment for last hunk...

@krzk
Copy link

krzk commented Jan 9, 2024

I'll try to sort it out.

@krzk krzk mentioned this pull request Jan 9, 2024
krzk added a commit to krzk/linux that referenced this pull request Jan 9, 2024
Linux kernel NVMEM consumer bindings define phandle to NVMEM cells
("nvmem-cells"), thus we also want the common definition of property
defining number of cells encoding that specifier, so the

Suggested-by: Rob Herring <robh@kernel.org>
Reported-by: Michael Walle <michael@walle.cc>
Closes: devicetree-org/dt-schema#89
Reported-by: Rafał Miłecki <zajec5@gmail.com>
Closes: https://lore.kernel.org/linux-arm-kernel/20221121105830.7411-1-zajec5@gmail.com/#r
Closes: https://lore.kernel.org/all/bdf7751b-0421-485d-8382-26c084f09d7d@gmail.com/
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
krzk added a commit to krzk/linux that referenced this pull request Jan 22, 2024
Linux kernel NVMEM consumer bindings define phandle to NVMEM cells
("nvmem-cells"), thus we also want the common definition of property
defining number of cells encoding that specifier, so the

Suggested-by: Rob Herring <robh@kernel.org>
Reported-by: Michael Walle <michael@walle.cc>
Closes: devicetree-org/dt-schema#89
Reported-by: Rafał Miłecki <zajec5@gmail.com>
Closes: https://lore.kernel.org/linux-arm-kernel/20221121105830.7411-1-zajec5@gmail.com/#r
Closes: https://lore.kernel.org/all/bdf7751b-0421-485d-8382-26c084f09d7d@gmail.com/
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
@robherring
Copy link
Member

Fixed in commit 0053468 ("dtb: expect nvmem-cell-cells")

@robherring robherring closed this Jan 24, 2024
krzk added a commit to krzk/linux that referenced this pull request Feb 12, 2024
Linux kernel NVMEM consumer bindings define phandle to NVMEM cells
("nvmem-cells"), thus we also want the common definition of property
defining number of cells encoding that specifier, so the

Suggested-by: Rob Herring <robh@kernel.org>
Reported-by: Michael Walle <michael@walle.cc>
Closes: devicetree-org/dt-schema#89
Reported-by: Rafał Miłecki <zajec5@gmail.com>
Closes: https://lore.kernel.org/linux-arm-kernel/20221121105830.7411-1-zajec5@gmail.com/#r
Closes: https://lore.kernel.org/all/bdf7751b-0421-485d-8382-26c084f09d7d@gmail.com/
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
staging-kernelci-org pushed a commit to kernelci/linux that referenced this pull request Feb 14, 2024
Linux kernel NVMEM consumer bindings define phandle to NVMEM cells
("nvmem-cells"), thus we also want the common definition of property
defining number of cells encoding that specifier, so the

Suggested-by: Rob Herring <robh@kernel.org>
Reported-by: Michael Walle <michael@walle.cc>
Closes: devicetree-org/dt-schema#89
Reported-by: Rafał Miłecki <zajec5@gmail.com>
Closes: https://lore.kernel.org/linux-arm-kernel/20221121105830.7411-1-zajec5@gmail.com/#r
Closes: https://lore.kernel.org/all/bdf7751b-0421-485d-8382-26c084f09d7d@gmail.com/
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
intel-lab-lkp pushed a commit to intel-lab-lkp/linux that referenced this pull request Feb 15, 2024
Linux kernel NVMEM consumer bindings define phandle to NVMEM cells
("nvmem-cells"), thus we also want the common definition of property
defining number of cells encoding that specifier, so the

Suggested-by: Rob Herring <robh@kernel.org>
Reported-by: Michael Walle <michael@walle.cc>
Closes: devicetree-org/dt-schema#89
Reported-by: Rafał Miłecki <zajec5@gmail.com>
Closes: https://lore.kernel.org/linux-arm-kernel/20221121105830.7411-1-zajec5@gmail.com/#r
Closes: https://lore.kernel.org/all/bdf7751b-0421-485d-8382-26c084f09d7d@gmail.com/
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
ColinIanKing pushed a commit to ColinIanKing/linux-next that referenced this pull request Mar 12, 2024
Linux kernel NVMEM consumer bindings define phandle to NVMEM cells
("nvmem-cells"), thus we also want the common definition of property
defining number of cells encoding that specifier, so the

Suggested-by: Rob Herring <robh@kernel.org>
Reported-by: Michael Walle <michael@walle.cc>
Closes: devicetree-org/dt-schema#89
Reported-by: Rafał Miłecki <zajec5@gmail.com>
Closes: https://lore.kernel.org/linux-arm-kernel/20221121105830.7411-1-zajec5@gmail.com/#r
Closes: https://lore.kernel.org/all/bdf7751b-0421-485d-8382-26c084f09d7d@gmail.com/
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Link: https://lore.kernel.org/r/20240224114516.86365-5-srinivas.kandagatla@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
ijc pushed a commit to ijc/devicetree-conversion-state-v3 that referenced this pull request Mar 26, 2024
Linux kernel NVMEM consumer bindings define phandle to NVMEM cells
("nvmem-cells"), thus we also want the common definition of property
defining number of cells encoding that specifier, so the

Suggested-by: Rob Herring <robh@kernel.org>
Reported-by: Michael Walle <michael@walle.cc>
Closes: devicetree-org/dt-schema#89
Reported-by: Rafał Miłecki <zajec5@gmail.com>
Closes: https://lore.kernel.org/linux-arm-kernel/20221121105830.7411-1-zajec5@gmail.com/#r
Closes: https://lore.kernel.org/all/bdf7751b-0421-485d-8382-26c084f09d7d@gmail.com/
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
Link: https://lore.kernel.org/r/20240224114516.86365-5-srinivas.kandagatla@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

[ upstream commit: d28c853 ]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants