Re: [PATCH] mtd: rawnand: tegra: fix error handling of subop helpers

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

Hi Boris,

Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 17 Jul 2018
14:38:41 +0200:

> On Tue, 17 Jul 2018 14:22:54 +0200
> Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> 
> > Hi Boris,
> > 
> > Boris Brezillon <boris.brezillon@bootlin.com> wrote on Tue, 17 Jul 2018
> > 14:17:33 +0200:
> >   
> > > On Sat, 14 Jul 2018 18:32:51 +0200
> > > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> > >     
> > > > A report from Colin Ian King pointed a CoverityScan issue where error
> > > > values on these helpers where not checked in the drivers. These
> > > > helpers could error out only in case of a software bug in driver code,
> > > > not because of a runtime/hardware error but in any cases it is safer
> > > > to handle these errors properly.
> > > > 
> > > > Fix the Tegra NAND controller driver implementation by checking
> > > > potential negative error values coming from these helpers.      
> > > 
> > > Hm, ok. I thought you were opting for a return 0 + WARN_ON() approach,
> > > what made you change your mind?    
> > 
> > Wise people told me WARN_ON() should be avoided as much as possible.  
> 
> I think I mentioned BUG_ON(), not WARN_ON() :P.

I've never been a good listener :)

> 
> > Hence after more discussion with myself I choose to implement the most
> > standard C solution: check the returned value...
> > 
> > But if you think a return 0 + WARN_ON() would be better I'm ready to
> > change this as it was my initial idea :)  
> 
> Well, if this cannot happen without a SW bug, then I'd recommend the
> WARN_ON() + unsigned int ret approach. That should force people debug
> their implementation while keeping drivers code simple.

I'm fine with this approach, I'll send a v2.

Thanks,
Miquèl