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

From: Krzysztof Kozlowski <krzk@kernel.org>
To: Erez <erezgeva2@gmail.com>
Cc: Erez Geva <erezgeva@nwtime.org>,
	linux-mtd@lists.infradead.org,
	Tudor Ambarus <tudor.ambarus@linaro.org>,
	Pratyush Yadav <pratyush@kernel.org>,
	Michael Walle <mwalle@kernel.org>,
	linux-kernel@vger.kernel.org,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	devicetree@vger.kernel.org, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Esben Haabendal <esben@geanix.com>
Subject: Re: [PATCH v4 3/5] dt-bindings: mtd: spi-nor: add OTP parameters
Date: Tue, 17 Sep 2024 19:17:06 +0200	[thread overview]
Message-ID: <e0db2f62-b2fd-4b61-932c-cc2caf5dd647@kernel.org> (raw)
In-Reply-To: <CANeKEMPwgtECfksgz6jXkR+bjVFwCB9DOh1q7t_3WeojReqxbA@mail.gmail.com>

On 17/09/2024 19:11, Erez wrote:
> On Tue, 17 Sept 2024 at 19:00, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> On 17/09/2024 12:42, Erez wrote:
>>> On Tue, 17 Sept 2024 at 12:36, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>
>>>> On 17/09/2024 11:49, Erez Geva wrote:
>>>>> From: Erez Geva <ErezGeva2@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@gmail.com>
>>>>> ---
>>>>>  .../bindings/mtd/jedec,spi-nor.yaml           | 37 +++++++++++++++++++
>>>>>  1 file changed, 37 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.yaml
>>>>> index 6e3afb42926e..d502b7fab2ce 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.
>>>>>
>>>>> +  opt_n_regions:
>>>>
>>>> No underscores, but hyphens.
>>>
>>> I'll fix this.
>>>
>>>>
>>>>> +    type: u32
>>>>
>>>> It does not look like you tested the bindings, at least after quick
>>>> look. Please run `make dt_binding_check` (see
>>>
>>> I run "make dt_binding_check" on kernel 6.6.
>>
>> Yeah, we are no on kernel 6.6. You can run it also on kernel v4.1 -
>> still does not matter.
>>
>> Don't develop on ancient code because then you ask us to review same
>> broken stuff we already fixed.
> 
> I test with Beaglebone black for testing, it is difficult to run the
> last vanille version.
> I did backport the spi-nor driver.
> As for "make dt_binding_check" on last kernel, it need to upgrade the tools,
>  and I did not think it could change that much.
> 

Well, it is possible to build kernel on small embedded board, but that's
quite cumbersone, slow and inefficient, considering that it's just
easier to cross compile. But anyway, binding check does not even need
cross compilation.

Sorry, the code is obviously wrong, there is no such thing as u32, so
you did not test it. I provided link which explains how to test it. You
must do it on latest mainline kernel. Just like you must develop and
generate patches on latest mainline kernel, because this is where we
apply the patches. We do not apply them to v6.6.

Best regards,
Krzysztof

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/