[PATCH] staging: iio: adc: ad7192: Add spaces around minus operator

[PATCH] staging: iio: adc: ad7192: Add spaces around minus operator

[Karen Palacio]

Add spaces around minus operator to fix readibility.

Signed-off-by: Karen Palacio <karen.palacio.1994@gmail.com>
---
 drivers/staging/iio/adc/ad7192.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index acdbc07..7c632cf 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -355,7 +355,7 @@ ad7192_show_scale_available(struct device *dev,
 }
 
 static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
-			     in_voltage-voltage_scale_available,
+			     in_voltage - voltage_scale_available,
 			     0444, ad7192_show_scale_available, NULL, 0);
 
 static IIO_DEVICE_ATTR(in_voltage_scale_available, 0444,
-- 
2.7.4

Re: [PATCH] staging: iio: adc: ad7192: Add spaces around minus operator

[Alexandru Ardelean]

On Sun, Mar 10, 2019 at 11:23 PM Karen Palacio
<karen.palacio.1994@gmail.com> wrote:
>
> Add spaces around minus operator to fix readibility.
>
> Signed-off-by: Karen Palacio <karen.palacio.1994@gmail.com>
> ---
>  drivers/staging/iio/adc/ad7192.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index acdbc07..7c632cf 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -355,7 +355,7 @@ ad7192_show_scale_available(struct device *dev,
>  }
>
>  static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
> -                            in_voltage-voltage_scale_available,
> +                            in_voltage - voltage_scale_available,

This isn't broken, but I do agree it should be addressed.
I think it's the second time I see a similar patch trying to fix this.
So, obviously the code is a bit misleading.

>                              0444, ad7192_show_scale_available, NULL, 0);
>
>  static IIO_DEVICE_ATTR(in_voltage_scale_available, 0444,
> --
> 2.7.4
>

Re: [PATCH] staging: iio: adc: ad7192: Add spaces around minus operator

[Dan Carpenter]

On Mon, Mar 11, 2019 at 10:12:48AM +0200, Alexandru Ardelean wrote:
> On Sun, Mar 10, 2019 at 11:23 PM Karen Palacio
> <karen.palacio.1994@gmail.com> wrote:
> >
> > Add spaces around minus operator to fix readibility.
> >
> > Signed-off-by: Karen Palacio <karen.palacio.1994@gmail.com>
> > ---
> >  drivers/staging/iio/adc/ad7192.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > index acdbc07..7c632cf 100644
> > --- a/drivers/staging/iio/adc/ad7192.c
> > +++ b/drivers/staging/iio/adc/ad7192.c
> > @@ -355,7 +355,7 @@ ad7192_show_scale_available(struct device *dev,
> >  }
> >
> >  static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
> > -                            in_voltage-voltage_scale_available,
> > +                            in_voltage - voltage_scale_available,
> 
> This isn't broken, but I do agree it should be addressed.
> I think it's the second time I see a similar patch trying to fix this.
> So, obviously the code is a bit misleading.
> 

It's got to be more than that.  I did a simple grep of my inbox and it
says that this is the 13th time someone has tried to do this.  It's
possible that my grep is wrong, 13 seems high but not totaly impossible.

regards,
dan carpenter

Re: [PATCH] staging: iio: adc: ad7192: Add spaces around minus operator

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 2154 bytes --]

On 2019-03-11, at 11:31:59 +0300, Dan Carpenter wrote:
> On Mon, Mar 11, 2019 at 10:12:48AM +0200, Alexandru Ardelean wrote:
> > On Sun, Mar 10, 2019 at 11:23 PM Karen Palacio
> > <karen.palacio.1994@gmail.com> wrote:
> > >
> > > Add spaces around minus operator to fix readibility.
> > >
> > > Signed-off-by: Karen Palacio <karen.palacio.1994@gmail.com>
> > > ---
> > >  drivers/staging/iio/adc/ad7192.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > > index acdbc07..7c632cf 100644
> > > --- a/drivers/staging/iio/adc/ad7192.c
> > > +++ b/drivers/staging/iio/adc/ad7192.c
> > > @@ -355,7 +355,7 @@ ad7192_show_scale_available(struct device *dev,
> > >  }
> > >
> > >  static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
> > > -                            in_voltage-voltage_scale_available,
> > > +                            in_voltage - voltage_scale_available,
> >
> > This isn't broken, but I do agree it should be addressed.  I think
> > it's the second time I see a similar patch trying to fix this.  So,
> > obviously the code is a bit misleading.
> >
>
> It's got to be more than that.  I did a simple grep of my inbox and it
> says that this is the 13th time someone has tried to do this.  It's
> possible that my grep is wrong, 13 seems high but not totaly
> impossible.

The second parameter of IIO_DEVICE_ATTR_NAMED is a name which is
eventually stringified, not an arithmetic expression.

  #define IIO_DEVICE_ATTR_NAMED(_vname, _name, _mode, _show, _store, \
				_addr) \
	struct iio_dev_attr iio_dev_attr_##_vname \
	= IIO_ATTR(_name, _mode, _show, _store, _addr)

which is defined as:

  #define IIO_ATTR(_name, _mode, _show, _store, _addr)		\
	{ .dev_attr = __ATTR(_name, _mode, _show, _store),	\
	  .address = _addr }

which is defined as:

  #define __ATTR(_name, _mode, _show, _store) {             \
	.attr = {.name = __stringify(_name),                \
		 .mode = VERIFY_OCTAL_PERMISSIONS(_mode) }, \
	.show	= _show,                                    \
	.store	= _store,                                   \
  }

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] staging: iio: adc: ad7192: Add spaces around minus operator

[Jonathan Cameron]

On Mon, 11 Mar 2019 11:31:59 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:

> On Mon, Mar 11, 2019 at 10:12:48AM +0200, Alexandru Ardelean wrote:
> > On Sun, Mar 10, 2019 at 11:23 PM Karen Palacio
> > <karen.palacio.1994@gmail.com> wrote:  
> > >
> > > Add spaces around minus operator to fix readibility.
> > >
> > > Signed-off-by: Karen Palacio <karen.palacio.1994@gmail.com>
> > > ---
> > >  drivers/staging/iio/adc/ad7192.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> > > index acdbc07..7c632cf 100644
> > > --- a/drivers/staging/iio/adc/ad7192.c
> > > +++ b/drivers/staging/iio/adc/ad7192.c
> > > @@ -355,7 +355,7 @@ ad7192_show_scale_available(struct device *dev,
> > >  }
> > >
> > >  static IIO_DEVICE_ATTR_NAMED(in_v_m_v_scale_available,
> > > -                            in_voltage-voltage_scale_available,
> > > +                            in_voltage - voltage_scale_available,  
> > 
> > This isn't broken, but I do agree it should be addressed.
> > I think it's the second time I see a similar patch trying to fix this.
> > So, obviously the code is a bit misleading.
> >   
> 
> It's got to be more than that.  I did a simple grep of my inbox and it
> says that this is the 13th time someone has tried to do this.  It's
> possible that my grep is wrong, 13 seems high but not totaly impossible.

That sounds about right.

Best bet is to move this over to the read_avail callback and do it using
the core handling for available attributes which will build the name
in code so checkpatch won't find it.

Anyone want to spin up a patch to do that and save anyone else wasting
time on this?  Or we could just put a comment right next to it and hope
people read it..  

Thanks,

Jonathan

> 
> regards,
> dan carpenter
>