Re: [RFC PATCH 1/3] mfd: upboard: Add UP2 platform controller driver

[Lee Jones <lee.jones@xxxxxxxxxx>]

On Fri, 04 May 2018, Javier Arteaga wrote:

> On Fri, May 04, 2018 at 09:09:18AM +0100, Lee Jones wrote:
> > On Thu, 03 May 2018, Javier Arteaga wrote:
> > > On Thu, May 03, 2018 at 09:32:04AM +0100, Lee Jones wrote:

[...]

> > > > > I see your point. I was going by the reasoning that really_probe()
> > > > > already avoids printing log warnings on -ENODEV/-ENXIO (yes, literally
> > > > > an error code, but meaning "driver rejected a device match" AFAIK).
> > > > > Following that logic, it should be desirable to be just as quiet from
> > > > > our driver too.
> > > > 
> > > > Silently failing is a valid use-case.  It would depend on what the
> > > > failure is.  In the use-case to which you refer a mis-match is not
> > > > 'really' an error, they basically say "this is not the correct driver,
> > > > continue to look for the right one".  The mis-matches will also be
> > > > many and the output of the log, useless.  However in your case, an
> > > > -ENODEV here would been that someone is trying to boot the kernel on a
> > > > board which should probably be supported, but support is missing.
> > > 
> > > Hmm. Following from your comment:
> > > 
> > > - the first check ("is FPGA firmware explicitly marked as custom?") is a
> > >   valid use of dev_dbg() ; return -ENODEV, as this driver surely isn't
> > >   meant to handle arbitrary firmware, but
> > 
> > No, it's an error.
> > 
> > "Custom firmware not supported" it's something your user will always
> > want to know.  Why are you hiding this from a user?
> 
> *Theoretically* another driver (e.g. out-of-tree support for that custom
> firmware) may be able to handle that firmware just fine, at which point
> an error line from this driver in dmesg is just misleading spam.

I don't care about out-of-tree drivers. ;)

> But while typing it out, I can see is all hypothetical and hand-wavy.
> While having a re-flashed board that silently fails to bind would
> certainly confuse users. Let's make it a dev_err() then.

+1

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html