linux-kernel - Re: [PATCH v5 3/5] dt-bindings: mtd: spi-nor: add OTP parameters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANeKEMPSfGNLs+dwd0LWQUGpkGkShu5zOJhDbtMWyg-8SQaJKA@mail.gmail.com>
Date: Mon, 23 Sep 2024 17:49:54 +0200
From: Erez <erezgeva2@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Erez Geva <erezgeva@...ime.org>, linux-mtd@...ts.infradead.org, 
	Tudor Ambarus <tudor.ambarus@...aro.org>, Pratyush Yadav <pratyush@...nel.org>, 
	Michael Walle <mwalle@...nel.org>, linux-kernel@...r.kernel.org, 
	Miquel Raynal <miquel.raynal@...tlin.com>, Richard Weinberger <richard@....at>, 
	Vignesh Raghavendra <vigneshr@...com>, devicetree@...r.kernel.org, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Esben Haabendal <esben@...nix.com>
Subject: Re: [PATCH v5 3/5] dt-bindings: mtd: spi-nor: add OTP parameters

On Mon, 23 Sept 2024 at 17:42, Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On 23/09/2024 11:21, Erez wrote:
> > On Sun, 22 Sept 2024 at 22:40, Krzysztof Kozlowski <krzk@...nel.org> wrote:
> >>
> >> On Fri, Sep 20, 2024 at 08:12:29PM +0200, Erez Geva wrote:
> >>> From: Erez Geva <ErezGeva2@...il.com>
> >>>
> >>> Some flash devices need OTP parameters in device tree.
> >>> As we can not deduce the parameters based on JEDEC ID or SFDP.
> >>>
> >>> Signed-off-by: Erez Geva <ErezGeva2@...il.com>
> >>> ---
> >>>  .../bindings/mtd/jedec,spi-nor.yaml           | 39 +++++++++++++++++++
> >>>  1 file changed, 39 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> >>> index 6e3afb42926e..4f7bb3f41cb1 100644
> >>> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> >>> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
> >>> @@ -90,6 +90,43 @@ properties:
> >>>        the SRWD bit while writing the status register. WP# signal hard strapped to GND
> >>>        can be a valid use case.
> >>>
> >>> +  otp-n-regions:
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>> +    description:
> >>> +      Some flash devices need OTP parameters in the device tree.
> >>> +      As we can not deduce the parameters based on JEDEC ID or SFDP.
> >>> +      This parameter indicates the number of OTP regions.
> >>
> >> OTP regions where? In DTS? On flash itself?
> >
> > Where can OTP regions be?
> > Can you please be serious?
> > If you have any suggestions, I am happy to hear.
> > I did ask before.
>
> Yes, I am serious, imagine that we do not know what you wanted to say.
> At first this just sounded like you mix nvmem-cells here.
>
> Out of blue this binding starts mentioning OTP and you add bunch of
> generic properties not really matching anything so far. Instead of being
> sarcastic about reviewers confusion, rather improve your description

Sorry, no disrespect.
I just prefer to be explicit, same as you ask me to be.

The patch seria is under review in the MTD-SPI-NOR.
This patch is under hold at the moment.

Thanks for the feedback and your time
Erez
.
>
> Otherwise, good luck.
>
>
>
> >
> >>
> >>> +      The value must be larger or equal to 1 and mandatory for OTP.
> >>
> >> Don't repeat constraints in free form text. Add proper minimum and
> >
> > Sure, I will add a minimum.
> >
> >> default, although it is confusing - property is not required but it is
> >> mandatory for OTP?
> >
> > You are welcome to suggest a better rephrase.
> > Using OTP settings is optional.
> > If you set OTP then the number of regions and region length are mandatory.
> > While offset and base are optional for OTP settings.
>
> So properties should be required?
>
> >
> >
> >>
> >>
> >>
> >>> +
> >>> +  otp-len:
> >>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>> +    description:
> >>> +      Some flash devices need OTP parameters in the device tree.
> >>> +      As we can not deduce the parameters based on JEDEC ID or SFDP.
> >>
> >> Don't repeat the same.
> >
> > Is there a grouping description?
> >
> >>
> >>> +      This parameter indicates the size (length) in bytes of an OTP region.
> >>
> >> What if each region has different length? Is it possible?
> >
> > Yes, there are. Old Mactronix have chips with the first region bigger
> > than the second region.
> > As these are old chips, we may skip the support of them.
>
> Other devices can come later re-introducing this approach.
>
>
>
> Best regards,
> Krzysztof
>