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 <samuel@xxxxxxxxxxxx> >>> --- >>> >>> 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 >>> >>> > >