Re: [PATCH 4/4] iio: dac: Convert powerdown read callbacks to sysfs_emit()

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 20 Mar 2021 08:13:55 -0700
Joe Perches <joe@perches.com> wrote:

> On Sat, 2021-03-20 at 13:52 +0100, Lars-Peter Clausen wrote:
> > On 3/20/21 12:01 PM, Joe Perches wrote:  
> > > On Sat, 2021-03-20 at 08:14 +0100, Lars-Peter Clausen wrote:  
> > > > Update DAC drivers powerdown attribute show callback to use the new
> > > > sysfs_emit() function.
> > > > 
> > > > sysfs_emit() is preferred over raw s*printf() for sysfs attributes since it
> > > > knows about the sysfs buffer specifics and has some built-in sanity checks.  
> > > Thanks.
> > > 
> > > unrelated trivia:
> > >   
> > > > diff --git a/drivers/iio/dac/ad5360.c b/drivers/iio/dac/ad5360.c  
> > > []  
> > > > @@ -255,7 +255,7 @@ static ssize_t ad5360_read_dac_powerdown(struct device *dev,
> > > >   	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > > >   	struct ad5360_state *st = iio_priv(indio_dev);
> > > >   
> > > > 
> > > > -	return sprintf(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));
> > > > +	return sysfs_emit(buf, "%d\n", (bool)(st->ctrl & AD5360_SF_CTRL_PWR_DOWN));  
> > > rather than cast to bool, perhaps standardize to use !!(val & test)  
> > I very much prefer the cast to bool since it semantically stronger. You 
> > don't have to know that the !! idiom is used to cast an int to bool.  
> 
> Using !! does not cast to bool, it's an int.
> 
> casting to bool and using %d in a printf equivalent ends up with an
> integer promotion/implicit type conversion from bool to int.
> 
> Anyway, it's not my code so it's author's choice, but similar
> code using different styles is, at a minimum, inconsistent.
> 
I'm certainly not against cleaning this up at somepoint, but it's not strictly
part of what this particular patch set is doing, so I'd rather do
it separately anyway.

Applied to the togreg branch of iio.git and pushed out as testing
for all the normal reasons.

Thanks,

Jonathan