Message-ID: <mafs0ikgnn07u.fsf@kernel.org>
Date: Fri, 10 Oct 2025 01:07:17 +0200
From: Pratyush Yadav <pratyush@...nel.org>
To: Sean Anderson <sean.anderson@...ux.dev>
Cc: Pratyush Yadav <pratyush@...nel.org>,  Tudor Ambarus
 <tudor.ambarus@...aro.org>,  Michael Walle <mwalle@...nel.org>,
  linux-mtd@...ts.infradead.org,  Richard Weinberger <richard@....at>,
  linux-kernel@...r.kernel.org,  Miquel Raynal <miquel.raynal@...tlin.com>,
  Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH] mtd: spi-nor: Enable locking for n25q00a

On Thu, Oct 09 2025, Sean Anderson wrote:

> On 10/8/25 08:30, Pratyush Yadav wrote:
>> On Tue, Oct 07 2025, Sean Anderson wrote:
>> 
>>> On 10/7/25 09:15, Pratyush Yadav wrote:
>>>> On Mon, Oct 06 2025, Sean Anderson wrote:
>>>> 
>>>>> The datasheet for n25q00a shows that the status register has the same
>>>>> layout as for n25q00, so use the same flags to enable locking support.
>>>>> These flags should have been added back in commit 150ccc181588 ("mtd:
>>>>> spi-nor: Enable locking for n25q128a11"), but they were removed by the
>>>>> maintainer...
>>>> 
>>>> This makes it sound like the maintainer did something wrong, which is
>>>> not true. Tudor had a good reason for removing them.
>>>
>>> I disagree. The maintainer used his position of authority to make the
>>> submitter second-guess their correct patch.
>> 
>> Sean, you are being very combative over such a small issue. You must
>> test your changes. This is one of the most basic principles in software
>> engineering. It was perfectly reasonable from Tudor to push back on
>> untested changes.
>> 
>> There is no abuse of "position of authority" here. When things break, we
>> get to do the work of putting the pieces back together. So of course, we
>> are reluctant to take things that increase this burden for us. Having
>> contributors test their changes is the simplest of things we ask for to
>> keep the quality bar.
>> 
>> Beyond that, I'd say that a little politeness goes a long way in life.
>> Especially towards the people maintaining the software for free that you
>> (or your employer) use. We are both wasting our energy on this debate.
>> Please stop. Take a step back and think from the other side's
>> perspective. And try to work _with_ people, not against them.
>> 
>>>
>>> These flashes have capacity of greater than the 8 MiB that can be
>>> protected using 3 BP bits. Micron (and ST before them?) addressed this
>>> by adding a fourth BP bit. This is consistent across every flash in this
>>> series, and is clearly documented in every datasheet. Defaulting to 3
>>> bits is buggy behavior: we should assume flashes behave per their
>>> datasheets until proven otherwise, especially for less-popular features
>> 
>> If I had a euro every time I found a bug in a datasheet, well, I would
>> have enough money to at least buy a nice dinner. My point is, datasheets
>> are not perfect. Only running on real hardware gets you the true
>> picture.
>
> Well, it's even *more* buggy to pretend that the datasheet doesn't exist
> and just do whatever you please. Might as well reverse-engineer every
> chip that comes across your desk from first principles with that
> attitude.

... or, you know, read the data sheet, write the driver, and _test_ if
it actually works?

>
> The locking doesn't work on any of these flashes without these flags. If
> you don't believe me you can try it yourself. The people who submitted
> the original patches certainly didn't test it.

Right. So can you send the patches you _did_ test on the hardware you do
have? So this time we are sure we got it right. And reply to the other
review comments? Without that, I don't think this patch can make
progress.

-- 
Regards,
Pratyush Yadav

Powered by blists - more mailing lists