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

[Erez]

On Mon, 23 Sept 2024 at 17:42, Krzysztof Kozlowski <krzk at kernel.org> wrote:
>
> On 23/09/2024 11:21, Erez wrote:
> > On Sun, 22 Sept 2024 at 22:40, Krzysztof Kozlowski <krzk at kernel.org> wrote:
> >>
> >> On Fri, Sep 20, 2024 at 08:12:29PM +0200, Erez Geva wrote:
> >>> From: Erez Geva <ErezGeva2 at gmail.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 at gmail.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
>