Some NVMEM devices contain cells which do not start at a multiple of the
device's stride. However, the "reg" property of a cell must be aligned
to its provider device's stride.
These cells can be represented in the DT using the "bits" property if
that property allows offsets up to the full stride. 63 is chosen
assuming that NVMEM devices will not have strides larger than 8 bytes.
Signed-off-by: Samuel Holland <[email protected]>
---
Documentation/devicetree/bindings/nvmem/nvmem.yaml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 3bb349c634cb..4f440ab6a13c 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -53,7 +53,7 @@ patternProperties:
$ref: /schemas/types.yaml#/definitions/uint32-array
items:
- minimum: 0
- maximum: 7
+ maximum: 63
description:
Offset in bit within the address range specified by reg.
- minimum: 1
--
2.35.1
On Sun, Aug 14, 2022 at 12:36:54PM -0500, Samuel Holland wrote:
> Some NVMEM devices contain cells which do not start at a multiple of the
> device's stride. However, the "reg" property of a cell must be aligned
> to its provider device's stride.
How is a DT author supposed to know this?
I would lean toward it's the OS's problem to deal with (your option 1 I
guess). I worry that one client could expect it one way and another
client the other. Or folks making DT changes to 'fix' things.
>
> These cells can be represented in the DT using the "bits" property if
> that property allows offsets up to the full stride. 63 is chosen
> assuming that NVMEM devices will not have strides larger than 8 bytes.
>
> Signed-off-by: Samuel Holland <[email protected]>
> ---
>
> Documentation/devicetree/bindings/nvmem/nvmem.yaml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> index 3bb349c634cb..4f440ab6a13c 100644
> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
> @@ -53,7 +53,7 @@ patternProperties:
> $ref: /schemas/types.yaml#/definitions/uint32-array
> items:
> - minimum: 0
> - maximum: 7
> + maximum: 63
> description:
> Offset in bit within the address range specified by reg.
> - minimum: 1
> --
> 2.35.1
>
>
On 8/25/22 4:02 PM, Rob Herring wrote:
> On Sun, Aug 14, 2022 at 12:36:54PM -0500, Samuel Holland wrote:
>> Some NVMEM devices contain cells which do not start at a multiple of the
>> device's stride. However, the "reg" property of a cell must be aligned
>> to its provider device's stride.
>
> How is a DT author supposed to know this?
To know the stride? They know the compatible string of the NVMEM provider
device, and the stride is a property of the hardware.
To know that alignment to the stride is required? That's not documented in the
binding. I can add that to the "reg" description in this file.
> I would lean toward it's the OS's problem to deal with (your option 1 I
> guess). I worry that one client could expect it one way and another
> client the other. Or folks making DT changes to 'fix' things.
After this binding change, the meaning of
reg = <0x2a 1>;
bits = <0 8>; // optional
and
reg = <0x28 4>;
bits = <16 8>;
would be identical.
With option 1, only the first representation would be valid in the DT.
With this series (option 2), either representation would validate in the DT, but
only the second representation would be accepted by Linux for the driver in
question (sunxi-sid).
With option 3, either representation would work in the DT and Linux.
Note that there is no restriction on the bit length, so the following are
already equivalent today:
reg = <0x28 1>;
bits = <0 8>; // optional
and
reg = <0x28 2>;
bits = <0 8>;
So there are already multiple possible representations. I don't really think
that is a problem, since the meaning is still unambiguous.
Regards,
Samuel
>> These cells can be represented in the DT using the "bits" property if
>> that property allows offsets up to the full stride. 63 is chosen
>> assuming that NVMEM devices will not have strides larger than 8 bytes.
>>
>> Signed-off-by: Samuel Holland <[email protected]>
>> ---
>>
>> Documentation/devicetree/bindings/nvmem/nvmem.yaml | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> index 3bb349c634cb..4f440ab6a13c 100644
>> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>> @@ -53,7 +53,7 @@ patternProperties:
>> $ref: /schemas/types.yaml#/definitions/uint32-array
>> items:
>> - minimum: 0
>> - maximum: 7
>> + maximum: 63
>> description:
>> Offset in bit within the address range specified by reg.
>> - minimum: 1
>> --
>> 2.35.1
>>
>>
On 9/8/22 22:29, Samuel Holland wrote:
> On 8/25/22 4:02 PM, Rob Herring wrote:
>> On Sun, Aug 14, 2022 at 12:36:54PM -0500, Samuel Holland wrote:
>>> Some NVMEM devices contain cells which do not start at a multiple of the
>>> device's stride. However, the "reg" property of a cell must be aligned
>>> to its provider device's stride.
>>
>> How is a DT author supposed to know this?
>
> To know the stride? They know the compatible string of the NVMEM provider
> device, and the stride is a property of the hardware.
>
> To know that alignment to the stride is required? That's not documented in the
> binding. I can add that to the "reg" description in this file.
>
>> I would lean toward it's the OS's problem to deal with (your option 1 I
>> guess). I worry that one client could expect it one way and another
>> client the other. Or folks making DT changes to 'fix' things.
>
> After this binding change, the meaning of
>
> reg = <0x2a 1>;
> bits = <0 8>; // optional
>
> and
>
> reg = <0x28 4>;
> bits = <16 8>;
>
> would be identical.
>
> With option 1, only the first representation would be valid in the DT.
>
> With this series (option 2), either representation would validate in the DT, but
> only the second representation would be accepted by Linux for the driver in
> question (sunxi-sid).
>
> With option 3, either representation would work in the DT and Linux.
>
> Note that there is no restriction on the bit length, so the following are
> already equivalent today:
>
> reg = <0x28 1>;
> bits = <0 8>; // optional
>
> and
>
> reg = <0x28 2>;
> bits = <0 8>;
>
> So there are already multiple possible representations. I don't really think
> that is a problem, since the meaning is still unambiguous.
Does anyone have further thoughts on this?
From Rob's comment on the alignment being "the OS's problem", it sounds
like I should implement option 3 -- have the NVMEM core transform the
cell to match the driver's alignment/size requirements. Before I start
implementing it, does that sound workable?
Regards,
Samuel
>>> These cells can be represented in the DT using the "bits" property if
>>> that property allows offsets up to the full stride. 63 is chosen
>>> assuming that NVMEM devices will not have strides larger than 8 bytes.
>>>
>>> Signed-off-by: Samuel Holland <[email protected]>
>>> ---
>>>
>>> Documentation/devicetree/bindings/nvmem/nvmem.yaml | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>>> index 3bb349c634cb..4f440ab6a13c 100644
>>> --- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
>>> @@ -53,7 +53,7 @@ patternProperties:
>>> $ref: /schemas/types.yaml#/definitions/uint32-array
>>> items:
>>> - minimum: 0
>>> - maximum: 7
>>> + maximum: 63
>>> description:
>>> Offset in bit within the address range specified by reg.
>>> - minimum: 1
>>> --
>>> 2.35.1
>>>
>>>
>
>