devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: Michal Simek <michal.simek@amd.com>,
	Praveen Teja Kundanala <praveen.teja.kundanala@amd.com>,
	srinivas.kandagatla@linaro.org, robh+dt@kernel.org,
	krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml
Date: Fri, 13 Oct 2023 13:58:23 +0200	[thread overview]
Message-ID: <b5a2d37b-168c-4cc9-9dc0-68f131cdf3ad@linaro.org> (raw)
In-Reply-To: <013e44c6-45f5-44c1-94b3-536955fae78f@amd.com>

On 13/10/2023 13:51, Michal Simek wrote:
> 
> 
> On 10/13/23 13:46, Krzysztof Kozlowski wrote:
>> On 13/10/2023 13:22, Michal Simek wrote:
>>>>
>>>>> +
>>>>> +required:
>>>>> +  - compatible
>>>>
>>>> required: block goes after patternProperties: block
>>>>
>>>>> +
>>>>> +patternProperties:
>>>>> +  "^soc_revision@0$":
>>>>
>>>> Why do you define individual memory cells? Is this part of a binding?
>>>> IOW, OS/Linux requires this?
>>>
>>> nvmem has in kernel interface where you can reference to nodes. nvmem_cell_get()
>>> calls. It means you should be able to describe internal layout that's why names
>>> are used. And address in name is there because of reg property is used to
>>> describe base offset and size.
>>
>> That's not really what I am asking. Why internal layout of memory must
>> be part of the bindings?
> 
> It doesn't need to be but offsets are hardcoded inside the driver itself and 
> they can't be different.

Hm, where? I opened drivers/nvmem/zynqmp_nvmem.c and I do not see any
hard-coded offsets.

>  On different nvmem locations like MAC location in 
> eeprom this can vary across boards but in this case location has to be only like 
> this.
> I am fine if they don't need to be actually check but there is no any other way 
> how they can be composed. And also others are not valid that's why not to 
> describe only valid one.

OK, that would be valid (if I find anywhere the offsets) and answers my
questions but I wish it was documented somewhere. Because now you are
making it a binding, so it cannot change (e.g. for different devices
with same hardware but different firmware or manufacturing process, for
future hardware sharing this binding).

In any case the binding should have only items which are really fixed
and OS depends on them. Neither this nor next commit answers this.

Best regards,
Krzysztof


  reply	other threads:[~2023-10-13 11:58 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-13 10:14 [PATCH 0/5] Add ZynqMP efuse access support Praveen Teja Kundanala
2023-10-13 10:14 ` [PATCH 1/5] firmware: xilinx: Add ZynqMP efuse access API Praveen Teja Kundanala
2023-10-13 10:14 ` [PATCH 2/5] dt-bindings: nvmem: Convert xlnx,zynqmp-nvmem.txt to yaml Praveen Teja Kundanala
2023-10-13 10:30   ` Krzysztof Kozlowski
2023-10-13 11:22     ` Michal Simek
2023-10-13 11:46       ` Krzysztof Kozlowski
2023-10-13 11:51         ` Michal Simek
2023-10-13 11:58           ` Krzysztof Kozlowski [this message]
2023-10-13 12:08             ` Michal Simek
2023-10-13 12:54               ` Krzysztof Kozlowski
2023-10-13 13:06                 ` Michal Simek
2023-10-13 13:10                   ` Krzysztof Kozlowski
2023-10-13 13:12                     ` Michal Simek
2023-10-16  8:01                     ` Michal Simek
2023-10-16 10:06                       ` Kundanala, Praveen Teja
2023-10-13 11:09   ` Rob Herring
2023-10-13 16:23   ` Rob Herring
2023-10-13 10:14 ` [PATCH 3/5] dt-bindings: nvmem: Add nodes for ZynqMP efuses Praveen Teja Kundanala
2023-10-13 10:31   ` Krzysztof Kozlowski
2023-10-13 10:14 ` [PATCH 4/5] arm64: zynqmp: Add ZynqnMP nvmem nodes Praveen Teja Kundanala
2023-10-13 10:32   ` Krzysztof Kozlowski
2023-10-13 11:18     ` Michal Simek
2023-10-13 10:14 ` [PATCH 5/5] nvmem: zynqmp_nvmem: Add support to access efuse Praveen Teja Kundanala

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b5a2d37b-168c-4cc9-9dc0-68f131cdf3ad@linaro.org \
    --to=krzysztof.kozlowski@linaro.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michal.simek@amd.com \
    --cc=praveen.teja.kundanala@amd.com \
    --cc=robh+dt@kernel.org \
    --cc=srinivas.kandagatla@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).