Re: [PATCH v4 2/3] mtd: cfi_cmdset_0002: Use chip_ready() for write on S29GL064N

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Thorsten,

regressions@leemhuis.info wrote on Mon, 21 Mar 2022 15:17:50 +0100:

> On 21.03.22 14:41, Miquel Raynal wrote:
> > regressions@leemhuis.info wrote on Mon, 21 Mar 2022 13:51:10 +0100:  
> >> On 21.03.22 13:35, Miquel Raynal wrote:  
> >>> regressions@leemhuis.info wrote on Mon, 21 Mar 2022 12:48:11 +0100:
> >>>  
> >>>> On 16.03.22 16:54, Tokunori Ikegami wrote:  
> >>>>> As pointed out by this bug report [1], buffered writes are now broken on
> >>>>> S29GL064N. This issue comes from a rework which switched from using chip_good()
> >>>>> to chip_ready(), because DQ true data 0xFF is read on S29GL064N and an error
> >>>>> returned by chip_good(). One way to solve the issue is to revert the change
> >>>>> partially to use chip_ready for S29GL064N.
> >>>>>
> >>>>> [1] https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/    
> >>>>
> >>>> Why did you switch from the documented format for links you added on my
> >>>> request (see
> >>>> https://lore.kernel.org/stable/f1b44e87-e457-7783-d46e-0d577cea3b72@leemhuis.info/
> >>>>
> >>>> ) to v2 to something else that is not recognized by tools and scripts
> >>>> that rely on proper link tags? You are making my and maybe other peoples
> >>>> life unnecessary hard. :-((
> >>>>
> >>>> FWIW, the proper style should support footnote style like this:
> >>>>
> >>>> Link:
> >>>> https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
> >>>>  [1]
> >>>>
> >>>> Ciao, Thorsten
> >>>>
> >>>> #regzbot ^backmonitor:
> >>>> https://lore.kernel.org/r/b687c259-6413-26c9-d4c9-b3afa69ea124@pengutronix.de/
> >>>>  
> >>>
> >>> Because today's requirement from maintainers is to provide a Link
> >>> tag that points to the mail discussion of the patch being applied.  
> >>
> >> That can be an additional Link tag, that is done all the time.
> >>  
> >>> I
> >>> then asked to use the above form instead to point to the bug report
> >>> because I don't see the point of having a "Link" tag for it?  
> > 
> > Perhaps I should emphasize that I don't remember your initial request
> > regarding the use of a Link tag  
> 
> Happen, no worries.
> 
> > and my original idea was to help this
> > contributor, not kill your tools which I actually know very little
> > about.  
> >>> But it's not your own project, we are all working with thousands of  
> >> people together on this project on various different fronts. That needs
> >> coordination, as some things otherwise become hard or impossible. That's
> >> why we have documentation that explains how to do some things. Not
> >> following it just because you don't like it is not helpful and in this
> >> case makes my life as a volunteer a lot harder.  
> > 
> > Let's be honest, you are referring to a Documentation patch that *you*
> > wrote  
> 
> Correct, but in case of submitting-patches it was just a clarification
> how to place links; why the whole aspect was missing in the other is
> kinda odd and likely lost in history...
> 
> > and was merged into Linus' tree mid January. How often do you
> > think people used to the contribution workflow monitor these files?  
> 
> Not often, that's why I have no problem pointing it out, even if that's
> slightly annoying. But you can imagine that it felt kinda odd on my side
> when asking someone to set the links (with references to the docs
> explaining how to set them) and seeing them added then in v2, just so
> see they vanished again in v3 of the same patch. :-/

I fully understand. I actually learned that these tags had to be used
for this purpose, so I will actually enforce their use in my next
reviews.

Just a side question, should the Documentation also mention how
to refer to links for people not used to it? Something like
[5.Posting.rst]:

	"Link: <link> [1]
	 Link: <link> [2]"

My original point was that maintainers would almost always add
a Link tag at the end, containing the mailing-list thread about the
patch being applied. Just saying in the commit log "see the link below"
then becomes misleading.

> > I am totally fine enforcing the use of Link: tags if this is what has
> > been decided, just don't expect everybody to switch to a style rather
> > than another over a night.  
> 
> I don't.
> 
> >> If you don't like the approach explained by the documentation, submit a
> >> patch adjusting the documentation and then we can talk about this. But
> >> until that is applied please stick to the format explained by the
> >> documentation.  
> > This is uselessly condescending.  
> 
> I apologize, it wasn't meant that way.

No worries, thanks :-)

> I had to many discussions already
> where people were not setting any links and it seems the topic is slowly
> hitting a nerve here. Sorry.

I also feel like I am repeating myself sometimes. And then I remember
Rob and the ton of e-mails where he has to repeat himself hundreds of
times a day and I feel slightly better :-p

Cheers, Miquèl