Re: [PATCH 2/8] i2c: i2c-qcom-geni: Signify successful driver probe
From: Ard Biesheuvel
Date: Wed Jun 05 2019 - 03:27:12 EST
On Wed, 5 Jun 2019 at 09:16, Lee Jones <lee.jones@xxxxxxxxxx> wrote:
> On Tue, 04 Jun 2019, Bjorn Andersson wrote:
> > On Tue 04 Jun 03:44 PDT 2019, Lee Jones wrote:
> > > The Qualcomm Geni I2C driver currently probes silently which can be
> > > confusing when debugging potential issues. Add a low level (INFO)
> > > print when each I2C controller is successfully initially set-up.
> > >
> > > Signed-off-by: Lee Jones <lee.jones@xxxxxxxxxx>
> > > ---
> > > drivers/i2c/busses/i2c-qcom-geni.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> > > index 0fa93b448e8d..e27466d77767 100644
> > > --- a/drivers/i2c/busses/i2c-qcom-geni.c
> > > +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> > > @@ -598,6 +598,8 @@ static int geni_i2c_probe(struct platform_device *pdev)
> > > return ret;
> > > }
> > >
> > > + dev_info(&pdev->dev, "Geni-I2C adaptor successfully added\n");
> > > +
> > I would prefer that we do not add such prints, as it would be to accept
> > the downstream behaviour of spamming the log to the point where no one
> > will ever look through it.
> We should be able to find a middle ground. Spamming the log with all
> sorts of device specific information/debug is obviously not
> constructive, but a single liner to advertise that an important
> device/controller has been successfully initialised is more helpful
> than it is hinderous.
> This print was added due to the silent initialisation costing me
> several hours of debugging ACPI device/driver code (albeit learning a
> lot about ACPI as I go) just to find out that it was already doing the
> right thing - just very quietly.
There are numerous EHCI drivers IIRC which, if compiled in,
unconditionally print some blurb, whether you have the hardware or
not, which is pretty annoying.
In this case, however, having a single line per successfully probed
device (containing the dev_name and perhaps the MMIO base address or
some other identifying feature) is pretty useful, and shouldn't be
regarded as log spamming imo. dev_info() honours the 'quiet' kernel
command line parameter, and so you will only see the message if you
actually look at the log.