Re: [PATCH 1/2] iio: adc: Add MAX1241 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Lars,

Thank you very much for your comments! I'll send a version with the
fixes in a day or two (ar as soon as there's no more feedback, anyways)
-- in the meantime I have a question about this one:

> > +	if (!adc->shdn)
> > +		dev_info(&spi->dev, "no shdn pin passed, low-power mode disabled");
> > +	else
> > +		dev_info(&spi->dev, "shdn pin passed, low-power mode enabled");
> 
> I can see how these message above are useful during development, but I'd
> remove them or turn them into dev_dbg() for the "production" version of the
> driver. Imagine every driver printed something like this, there would be a
> lot of spam in the bootlog.

I thought this should go under info, rather than debug, because it's
(possibly) relevant runtime information. It doesn't require any action,
but it's something that a user of this driver may want to be aware
of. The timing (and power consumption, of course) in low-power mode are
different. It's akin to e.g.:

./at91_adc.c:782: dev_info(&idev->dev, "Resolution used: %u bits\n", st->res);
./at91_adc.c:842: dev_info(dev, "ADC Touch screen is disabled.\n");
./at91_adc.c:955: dev_info(&idev->dev, "not support touchscreen in the adc compatible string.\n");

or:

./ti-ads124s08.c:320: dev_info(&spi->dev, "Reset GPIO not defined\n");

which is why I figured I'd do the same in my code (what can I say, I
wear my code monkey badge with pride!).

Needless to say, since you've seen more IIO subsystem drivers than I've
seen, I totally trust your assessment of whether this is debug or info
more than I trust mine. If this was a false positive on your "looks like
a leftover debug message", let me know (and while I'm at it I might make
the message more useful/friendly...). Otherwise it'll get bumped down to
dev_dbg in my next revision.

Thanks,
Alex



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux