linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -next] iio: st_accel: Fix unused variable warning
@ 2019-11-01 13:47 YueHaibing
  2019-11-02 10:41 ` Ladislav Michl
  2019-11-11  3:21 ` [PATCH v2 " YueHaibing
  0 siblings, 2 replies; 10+ messages in thread
From: YueHaibing @ 2019-11-01 13:47 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, denis.ciocca, yuehaibing,
	rfontana, tglx, heiko.stuebner, rjones, drake, colin.king
  Cc: linux-iio, linux-kernel

drivers/iio/accel/st_accel_core.c:1005:44: warning:
 mount_matrix_ext_info defined but not used [-Wunused-const-variable=]

Move it to ifdef to mute this warning.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/iio/accel/st_accel_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index 2e37f8a..bba0717 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -1002,10 +1002,12 @@ get_mount_matrix(const struct iio_dev *indio_dev,
 	return adata->mount_matrix;
 }
 
+#ifdef CONFIG_ACPI
 static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
 	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),
 	{ },
 };
+#endif
 
 /* Read ST-specific _ONT orientation data from ACPI and generate an
  * appropriate mount matrix.
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] iio: st_accel: Fix unused variable warning
  2019-11-01 13:47 [PATCH -next] iio: st_accel: Fix unused variable warning YueHaibing
@ 2019-11-02 10:41 ` Ladislav Michl
  2019-11-02 14:08   ` Jonathan Cameron
  2019-11-11  3:21 ` [PATCH v2 " YueHaibing
  1 sibling, 1 reply; 10+ messages in thread
From: Ladislav Michl @ 2019-11-02 10:41 UTC (permalink / raw)
  To: YueHaibing
  Cc: jic23, knaack.h, lars, pmeerw, denis.ciocca, rfontana, tglx,
	heiko.stuebner, rjones, drake, colin.king, linux-iio,
	linux-kernel

On Fri, Nov 01, 2019 at 09:47:41PM +0800, YueHaibing wrote:
> drivers/iio/accel/st_accel_core.c:1005:44: warning:
>  mount_matrix_ext_info defined but not used [-Wunused-const-variable=]
> 
> Move it to ifdef to mute this warning.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
>  drivers/iio/accel/st_accel_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index 2e37f8a..bba0717 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -1002,10 +1002,12 @@ get_mount_matrix(const struct iio_dev *indio_dev,
>  	return adata->mount_matrix;
>  }
>  
> +#ifdef CONFIG_ACPI
>  static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
>  	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),

So now you do not get any warning for unused get_mount_matrix?
(Then it would make more sense to put all that stuff under one ifdef
and provide empty apply_acpi_orientation for non ACPI case)

>  	{ },
>  };
> +#endif
>  
>  /* Read ST-specific _ONT orientation data from ACPI and generate an
>   * appropriate mount matrix.
> -- 
> 2.7.4
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] iio: st_accel: Fix unused variable warning
  2019-11-02 10:41 ` Ladislav Michl
@ 2019-11-02 14:08   ` Jonathan Cameron
  2019-11-02 20:15     ` Ladislav Michl
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2019-11-02 14:08 UTC (permalink / raw)
  To: Ladislav Michl
  Cc: YueHaibing, knaack.h, lars, pmeerw, denis.ciocca, rfontana, tglx,
	heiko.stuebner, rjones, drake, colin.king, linux-iio,
	linux-kernel

On Sat, 2 Nov 2019 11:41:25 +0100
Ladislav Michl <ladis@linux-mips.org> wrote:

> On Fri, Nov 01, 2019 at 09:47:41PM +0800, YueHaibing wrote:
> > drivers/iio/accel/st_accel_core.c:1005:44: warning:
> >  mount_matrix_ext_info defined but not used [-Wunused-const-variable=]
> > 
> > Move it to ifdef to mute this warning.
> > 
> > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > ---
> >  drivers/iio/accel/st_accel_core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> > index 2e37f8a..bba0717 100644
> > --- a/drivers/iio/accel/st_accel_core.c
> > +++ b/drivers/iio/accel/st_accel_core.c
> > @@ -1002,10 +1002,12 @@ get_mount_matrix(const struct iio_dev *indio_dev,
> >  	return adata->mount_matrix;
> >  }
> >  
> > +#ifdef CONFIG_ACPI
> >  static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
> >  	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),  
> 
> So now you do not get any warning for unused get_mount_matrix?
> (Then it would make more sense to put all that stuff under one ifdef
> and provide empty apply_acpi_orientation for non ACPI case)

Does the __maybe_unused marking make this go away?

I'd assume that the compiler will manage to drop this either way
but I guess we should check that.

ifdef magic is always harder to read and potentially fragile in the
long run.  Here we simply want to indicate that in some build
configurations we might not use this.

Thanks,

Jonathan


> 
> >  	{ },
> >  };
> > +#endif
> >  
> >  /* Read ST-specific _ONT orientation data from ACPI and generate an
> >   * appropriate mount matrix.
> > -- 
> > 2.7.4
> >   


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] iio: st_accel: Fix unused variable warning
  2019-11-02 14:08   ` Jonathan Cameron
@ 2019-11-02 20:15     ` Ladislav Michl
  2019-11-03 11:01       ` Jonathan Cameron
  2019-11-04  1:29       ` Yuehaibing
  0 siblings, 2 replies; 10+ messages in thread
From: Ladislav Michl @ 2019-11-02 20:15 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: YueHaibing, knaack.h, lars, pmeerw, denis.ciocca, rfontana, tglx,
	heiko.stuebner, rjones, drake, colin.king, linux-iio,
	linux-kernel

On Sat, Nov 02, 2019 at 02:08:10PM +0000, Jonathan Cameron wrote:
> On Sat, 2 Nov 2019 11:41:25 +0100
> Ladislav Michl <ladis@linux-mips.org> wrote:
> 
> > On Fri, Nov 01, 2019 at 09:47:41PM +0800, YueHaibing wrote:
> > > drivers/iio/accel/st_accel_core.c:1005:44: warning:
> > >  mount_matrix_ext_info defined but not used [-Wunused-const-variable=]
> > > 
> > > Move it to ifdef to mute this warning.
> > > 
> > > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > > ---
> > >  drivers/iio/accel/st_accel_core.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> > > index 2e37f8a..bba0717 100644
> > > --- a/drivers/iio/accel/st_accel_core.c
> > > +++ b/drivers/iio/accel/st_accel_core.c
> > > @@ -1002,10 +1002,12 @@ get_mount_matrix(const struct iio_dev *indio_dev,
> > >  	return adata->mount_matrix;
> > >  }
> > >  
> > > +#ifdef CONFIG_ACPI
> > >  static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
> > >  	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),  
> > 
> > So now you do not get any warning for unused get_mount_matrix?
> > (Then it would make more sense to put all that stuff under one ifdef
> > and provide empty apply_acpi_orientation for non ACPI case)
> 
> Does the __maybe_unused marking make this go away?
> 
> I'd assume that the compiler will manage to drop this either way
> but I guess we should check that.
> 
> ifdef magic is always harder to read and potentially fragile in the
> long run.  Here we simply want to indicate that in some build
> configurations we might not use this.

This suggestion implies we'll get rid of CONFIG_ACPI completely, which
seems inapproriate looking at size of apply_acpi_orientation function.
And having both CONFIG_ACPI and __maybe_unused does not make much
sense. I had something like that in mind (+COMPILE_TEST perhaps):

diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index 2e37f8a6d8cf..0e7eac62d618 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -993,6 +993,7 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
 #define ST_ACCEL_TRIGGER_OPS NULL
 #endif
 
+#ifdef CONFIG_ACPI
 static const struct iio_mount_matrix *
 get_mount_matrix(const struct iio_dev *indio_dev,
 		 const struct iio_chan_spec *chan)
@@ -1013,7 +1014,6 @@ static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
 static int apply_acpi_orientation(struct iio_dev *indio_dev,
 				  struct iio_chan_spec *channels)
 {
-#ifdef CONFIG_ACPI
 	struct st_sensor_data *adata = iio_priv(indio_dev);
 	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
 	struct acpi_device *adev;
@@ -1141,10 +1141,14 @@ static int apply_acpi_orientation(struct iio_dev *indio_dev,
 out:
 	kfree(buffer.pointer);
 	return ret;
-#else /* !CONFIG_ACPI */
+}
+#else
+static int apply_acpi_orientation(struct iio_dev *indio_dev,
+				  struct iio_chan_spec *channels)
+{
 	return 0;
-#endif
 }
+#endif
 
 /*
  * st_accel_get_settings() - get sensor settings from device name
> Thanks,
> 
> Jonathan
> 
> 
> > 
> > >  	{ },
> > >  };
> > > +#endif
> > >  
> > >  /* Read ST-specific _ONT orientation data from ACPI and generate an
> > >   * appropriate mount matrix.
> > > -- 
> > > 2.7.4
> > >   

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] iio: st_accel: Fix unused variable warning
  2019-11-02 20:15     ` Ladislav Michl
@ 2019-11-03 11:01       ` Jonathan Cameron
  2019-11-04  1:29       ` Yuehaibing
  1 sibling, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2019-11-03 11:01 UTC (permalink / raw)
  To: Ladislav Michl
  Cc: YueHaibing, knaack.h, lars, pmeerw, denis.ciocca, rfontana, tglx,
	heiko.stuebner, rjones, drake, colin.king, linux-iio,
	linux-kernel

On Sat, 2 Nov 2019 21:15:35 +0100
Ladislav Michl <ladis@linux-mips.org> wrote:

> On Sat, Nov 02, 2019 at 02:08:10PM +0000, Jonathan Cameron wrote:
> > On Sat, 2 Nov 2019 11:41:25 +0100
> > Ladislav Michl <ladis@linux-mips.org> wrote:
> >   
> > > On Fri, Nov 01, 2019 at 09:47:41PM +0800, YueHaibing wrote:  
> > > > drivers/iio/accel/st_accel_core.c:1005:44: warning:
> > > >  mount_matrix_ext_info defined but not used [-Wunused-const-variable=]
> > > > 
> > > > Move it to ifdef to mute this warning.
> > > > 
> > > > Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> > > > ---
> > > >  drivers/iio/accel/st_accel_core.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > > 
> > > > diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> > > > index 2e37f8a..bba0717 100644
> > > > --- a/drivers/iio/accel/st_accel_core.c
> > > > +++ b/drivers/iio/accel/st_accel_core.c
> > > > @@ -1002,10 +1002,12 @@ get_mount_matrix(const struct iio_dev *indio_dev,
> > > >  	return adata->mount_matrix;
> > > >  }
> > > >  
> > > > +#ifdef CONFIG_ACPI
> > > >  static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
> > > >  	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),    
> > > 
> > > So now you do not get any warning for unused get_mount_matrix?
> > > (Then it would make more sense to put all that stuff under one ifdef
> > > and provide empty apply_acpi_orientation for non ACPI case)  
> > 
> > Does the __maybe_unused marking make this go away?
> > 
> > I'd assume that the compiler will manage to drop this either way
> > but I guess we should check that.
> > 
> > ifdef magic is always harder to read and potentially fragile in the
> > long run.  Here we simply want to indicate that in some build
> > configurations we might not use this.  
> 
> This suggestion implies we'll get rid of CONFIG_ACPI completely, which
> seems inapproriate looking at size of apply_acpi_orientation function.
> And having both CONFIG_ACPI and __maybe_unused does not make much
> sense. I had something like that in mind (+COMPILE_TEST perhaps):

One additional oddity here is why we have explicit support for
the mount matrix only in the ACPI paths.  There are standard DT bindings
for it as well which we should be using.

So let us take your approach below for now, but it would be good to
support this properly on non acpi platforms as well.  Where it isn't
specified it will fall back to an identity matrix which is as good as
we have now with no information supplied.

Please send a v2 with the below.

Thanks,

Jonathan

> 
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index 2e37f8a6d8cf..0e7eac62d618 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -993,6 +993,7 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
>  #define ST_ACCEL_TRIGGER_OPS NULL
>  #endif
>  
> +#ifdef CONFIG_ACPI
>  static const struct iio_mount_matrix *
>  get_mount_matrix(const struct iio_dev *indio_dev,
>  		 const struct iio_chan_spec *chan)
> @@ -1013,7 +1014,6 @@ static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
>  static int apply_acpi_orientation(struct iio_dev *indio_dev,
>  				  struct iio_chan_spec *channels)
>  {
> -#ifdef CONFIG_ACPI
>  	struct st_sensor_data *adata = iio_priv(indio_dev);
>  	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
>  	struct acpi_device *adev;
> @@ -1141,10 +1141,14 @@ static int apply_acpi_orientation(struct iio_dev *indio_dev,
>  out:
>  	kfree(buffer.pointer);
>  	return ret;
> -#else /* !CONFIG_ACPI */
> +}
> +#else
> +static int apply_acpi_orientation(struct iio_dev *indio_dev,
> +				  struct iio_chan_spec *channels)
> +{
>  	return 0;
> -#endif
>  }
> +#endif
>  
>  /*
>   * st_accel_get_settings() - get sensor settings from device name
> > Thanks,
> > 
> > Jonathan
> > 
> >   
> > >   
> > > >  	{ },
> > > >  };
> > > > +#endif
> > > >  
> > > >  /* Read ST-specific _ONT orientation data from ACPI and generate an
> > > >   * appropriate mount matrix.
> > > > -- 
> > > > 2.7.4
> > > >     


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] iio: st_accel: Fix unused variable warning
  2019-11-02 20:15     ` Ladislav Michl
  2019-11-03 11:01       ` Jonathan Cameron
@ 2019-11-04  1:29       ` Yuehaibing
  2019-11-09 11:50         ` Jonathan Cameron
  1 sibling, 1 reply; 10+ messages in thread
From: Yuehaibing @ 2019-11-04  1:29 UTC (permalink / raw)
  To: Ladislav Michl, Jonathan Cameron
  Cc: knaack.h, lars, pmeerw, denis.ciocca, rfontana, tglx,
	heiko.stuebner, rjones, drake, colin.king, linux-iio,
	linux-kernel

On 2019/11/3 4:15, Ladislav Michl wrote:
> On Sat, Nov 02, 2019 at 02:08:10PM +0000, Jonathan Cameron wrote:
>> On Sat, 2 Nov 2019 11:41:25 +0100
>> Ladislav Michl <ladis@linux-mips.org> wrote:
>>
>>> On Fri, Nov 01, 2019 at 09:47:41PM +0800, YueHaibing wrote:
>>>> drivers/iio/accel/st_accel_core.c:1005:44: warning:
>>>>  mount_matrix_ext_info defined but not used [-Wunused-const-variable=]
>>>>
>>>> Move it to ifdef to mute this warning.
>>>>
>>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
>>>> ---
>>>>  drivers/iio/accel/st_accel_core.c | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
>>>> index 2e37f8a..bba0717 100644
>>>> --- a/drivers/iio/accel/st_accel_core.c
>>>> +++ b/drivers/iio/accel/st_accel_core.c
>>>> @@ -1002,10 +1002,12 @@ get_mount_matrix(const struct iio_dev *indio_dev,
>>>>  	return adata->mount_matrix;
>>>>  }
>>>>  
>>>> +#ifdef CONFIG_ACPI
>>>>  static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
>>>>  	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),  
>>>
>>> So now you do not get any warning for unused get_mount_matrix?
>>> (Then it would make more sense to put all that stuff under one ifdef
>>> and provide empty apply_acpi_orientation for non ACPI case)
>>
>> Does the __maybe_unused marking make this go away?
>>
>> I'd assume that the compiler will manage to drop this either way
>> but I guess we should check that.
>>
>> ifdef magic is always harder to read and potentially fragile in the
>> long run.  Here we simply want to indicate that in some build
>> configurations we might not use this.
> 
> This suggestion implies we'll get rid of CONFIG_ACPI completely, which
> seems inapproriate looking at size of apply_acpi_orientation function.
> And having both CONFIG_ACPI and __maybe_unused does not make much
> sense. I had something like that in mind (+COMPILE_TEST perhaps):
> 
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index 2e37f8a6d8cf..0e7eac62d618 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -993,6 +993,7 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
>  #define ST_ACCEL_TRIGGER_OPS NULL
>  #endif
>  
> +#ifdef CONFIG_ACPI
>  static const struct iio_mount_matrix *
>  get_mount_matrix(const struct iio_dev *indio_dev,
>  		 const struct iio_chan_spec *chan)
> @@ -1013,7 +1014,6 @@ static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
>  static int apply_acpi_orientation(struct iio_dev *indio_dev,
>  				  struct iio_chan_spec *channels)
>  {
> -#ifdef CONFIG_ACPI
>  	struct st_sensor_data *adata = iio_priv(indio_dev);
>  	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
>  	struct acpi_device *adev;
> @@ -1141,10 +1141,14 @@ static int apply_acpi_orientation(struct iio_dev *indio_dev,
>  out:
>  	kfree(buffer.pointer);
>  	return ret;
> -#else /* !CONFIG_ACPI */
> +}
> +#else
> +static int apply_acpi_orientation(struct iio_dev *indio_dev,

Does this need inline?

> +				  struct iio_chan_spec *channels)
> +{
>  	return 0;
> -#endif
>  }
> +#endif
>  
>  /*
>   * st_accel_get_settings() - get sensor settings from device name
>> Thanks,
>>
>> Jonathan
>>
>>
>>>
>>>>  	{ },
>>>>  };
>>>> +#endif
>>>>  
>>>>  /* Read ST-specific _ONT orientation data from ACPI and generate an
>>>>   * appropriate mount matrix.
>>>> -- 
>>>> 2.7.4
>>>>   
> 
> .
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH -next] iio: st_accel: Fix unused variable warning
  2019-11-04  1:29       ` Yuehaibing
@ 2019-11-09 11:50         ` Jonathan Cameron
  0 siblings, 0 replies; 10+ messages in thread
From: Jonathan Cameron @ 2019-11-09 11:50 UTC (permalink / raw)
  To: Yuehaibing
  Cc: Ladislav Michl, knaack.h, lars, pmeerw, denis.ciocca, rfontana,
	tglx, heiko.stuebner, rjones, drake, colin.king, linux-iio,
	linux-kernel

On Mon, 4 Nov 2019 09:29:21 +0800
Yuehaibing <yuehaibing@huawei.com> wrote:

> On 2019/11/3 4:15, Ladislav Michl wrote:
> > On Sat, Nov 02, 2019 at 02:08:10PM +0000, Jonathan Cameron wrote:  
> >> On Sat, 2 Nov 2019 11:41:25 +0100
> >> Ladislav Michl <ladis@linux-mips.org> wrote:
> >>  
> >>> On Fri, Nov 01, 2019 at 09:47:41PM +0800, YueHaibing wrote:  
> >>>> drivers/iio/accel/st_accel_core.c:1005:44: warning:
> >>>>  mount_matrix_ext_info defined but not used [-Wunused-const-variable=]
> >>>>
> >>>> Move it to ifdef to mute this warning.
> >>>>
> >>>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> >>>> ---
> >>>>  drivers/iio/accel/st_accel_core.c | 2 ++
> >>>>  1 file changed, 2 insertions(+)
> >>>>
> >>>> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> >>>> index 2e37f8a..bba0717 100644
> >>>> --- a/drivers/iio/accel/st_accel_core.c
> >>>> +++ b/drivers/iio/accel/st_accel_core.c
> >>>> @@ -1002,10 +1002,12 @@ get_mount_matrix(const struct iio_dev *indio_dev,
> >>>>  	return adata->mount_matrix;
> >>>>  }
> >>>>  
> >>>> +#ifdef CONFIG_ACPI
> >>>>  static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
> >>>>  	IIO_MOUNT_MATRIX(IIO_SHARED_BY_ALL, get_mount_matrix),    
> >>>
> >>> So now you do not get any warning for unused get_mount_matrix?
> >>> (Then it would make more sense to put all that stuff under one ifdef
> >>> and provide empty apply_acpi_orientation for non ACPI case)  
> >>
> >> Does the __maybe_unused marking make this go away?
> >>
> >> I'd assume that the compiler will manage to drop this either way
> >> but I guess we should check that.
> >>
> >> ifdef magic is always harder to read and potentially fragile in the
> >> long run.  Here we simply want to indicate that in some build
> >> configurations we might not use this.  
> > 
> > This suggestion implies we'll get rid of CONFIG_ACPI completely, which
> > seems inapproriate looking at size of apply_acpi_orientation function.
> > And having both CONFIG_ACPI and __maybe_unused does not make much
> > sense. I had something like that in mind (+COMPILE_TEST perhaps):
> > 
> > diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> > index 2e37f8a6d8cf..0e7eac62d618 100644
> > --- a/drivers/iio/accel/st_accel_core.c
> > +++ b/drivers/iio/accel/st_accel_core.c
> > @@ -993,6 +993,7 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
> >  #define ST_ACCEL_TRIGGER_OPS NULL
> >  #endif
> >  
> > +#ifdef CONFIG_ACPI
> >  static const struct iio_mount_matrix *
> >  get_mount_matrix(const struct iio_dev *indio_dev,
> >  		 const struct iio_chan_spec *chan)
> > @@ -1013,7 +1014,6 @@ static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
> >  static int apply_acpi_orientation(struct iio_dev *indio_dev,
> >  				  struct iio_chan_spec *channels)
> >  {
> > -#ifdef CONFIG_ACPI
> >  	struct st_sensor_data *adata = iio_priv(indio_dev);
> >  	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
> >  	struct acpi_device *adev;
> > @@ -1141,10 +1141,14 @@ static int apply_acpi_orientation(struct iio_dev *indio_dev,
> >  out:
> >  	kfree(buffer.pointer);
> >  	return ret;
> > -#else /* !CONFIG_ACPI */
> > +}
> > +#else
> > +static int apply_acpi_orientation(struct iio_dev *indio_dev,  
> 
> Does this need inline?

Not that I can see. It's in the c file not the header so there should
be no issue with letting the compiler make the decision.
It will almost certainly be optimized out anyway.

Thanks,

Jonathan


> 
> > +				  struct iio_chan_spec *channels)
> > +{
> >  	return 0;
> > -#endif
> >  }
> > +#endif
> >  
> >  /*
> >   * st_accel_get_settings() - get sensor settings from device name  
> >> Thanks,
> >>
> >> Jonathan
> >>
> >>  
> >>>  
> >>>>  	{ },
> >>>>  };
> >>>> +#endif
> >>>>  
> >>>>  /* Read ST-specific _ONT orientation data from ACPI and generate an
> >>>>   * appropriate mount matrix.
> >>>> -- 
> >>>> 2.7.4
> >>>>     
> > 
> > .
> >   
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 -next] iio: st_accel: Fix unused variable warning
  2019-11-01 13:47 [PATCH -next] iio: st_accel: Fix unused variable warning YueHaibing
  2019-11-02 10:41 ` Ladislav Michl
@ 2019-11-11  3:21 ` YueHaibing
  2019-11-16 14:58   ` Jonathan Cameron
  1 sibling, 1 reply; 10+ messages in thread
From: YueHaibing @ 2019-11-11  3:21 UTC (permalink / raw)
  To: jic23, knaack.h, lars, pmeerw, denis.ciocca, tglx,
	alexios.zavras, allison, yuehaibing, linus.walleij, ladis
  Cc: linux-iio, linux-kernel

drivers/iio/accel/st_accel_core.c:1005:44: warning:
 mount_matrix_ext_info defined but not used [-Wunused-const-variable=]

Using stub helper while CONFIG_ACPI is disabled to fix it.

Suggested-by: Ladislav Michl <ladis@linux-mips.org>
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/iio/accel/st_accel_core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
index 7b83764..7320275 100644
--- a/drivers/iio/accel/st_accel_core.c
+++ b/drivers/iio/accel/st_accel_core.c
@@ -992,6 +992,7 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
 #define ST_ACCEL_TRIGGER_OPS NULL
 #endif
 
+#ifdef CONFIG_ACPI
 static const struct iio_mount_matrix *
 get_mount_matrix(const struct iio_dev *indio_dev,
 		 const struct iio_chan_spec *chan)
@@ -1012,7 +1013,6 @@ static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
 static int apply_acpi_orientation(struct iio_dev *indio_dev,
 				  struct iio_chan_spec *channels)
 {
-#ifdef CONFIG_ACPI
 	struct st_sensor_data *adata = iio_priv(indio_dev);
 	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
 	struct acpi_device *adev;
@@ -1140,10 +1140,14 @@ static int apply_acpi_orientation(struct iio_dev *indio_dev,
 out:
 	kfree(buffer.pointer);
 	return ret;
+}
 #else /* !CONFIG_ACPI */
+static int apply_acpi_orientation(struct iio_dev *indio_dev,
+				  struct iio_chan_spec *channels)
+{
 	return 0;
-#endif
 }
+#endif
 
 /*
  * st_accel_get_settings() - get sensor settings from device name
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 -next] iio: st_accel: Fix unused variable warning
  2019-11-11  3:21 ` [PATCH v2 " YueHaibing
@ 2019-11-16 14:58   ` Jonathan Cameron
  2019-11-18  1:41     ` Yuehaibing
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Cameron @ 2019-11-16 14:58 UTC (permalink / raw)
  To: YueHaibing
  Cc: knaack.h, lars, pmeerw, denis.ciocca, tglx, alexios.zavras,
	allison, linus.walleij, ladis, linux-iio, linux-kernel

On Mon, 11 Nov 2019 11:21:15 +0800
YueHaibing <yuehaibing@huawei.com> wrote:

> drivers/iio/accel/st_accel_core.c:1005:44: warning:
>  mount_matrix_ext_info defined but not used [-Wunused-const-variable=]
> 
> Using stub helper while CONFIG_ACPI is disabled to fix it.
> 
> Suggested-by: Ladislav Michl <ladis@linux-mips.org>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied to the fixes-togreg branch of iio.git as I've now closed for new
things for the coming merge window.  This will go upstream after the merge
window closes and probably hit around rc2.  Thanks for cleaning this up.

Jonathan

> ---
>  drivers/iio/accel/st_accel_core.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
> index 7b83764..7320275 100644
> --- a/drivers/iio/accel/st_accel_core.c
> +++ b/drivers/iio/accel/st_accel_core.c
> @@ -992,6 +992,7 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
>  #define ST_ACCEL_TRIGGER_OPS NULL
>  #endif
>  
> +#ifdef CONFIG_ACPI
>  static const struct iio_mount_matrix *
>  get_mount_matrix(const struct iio_dev *indio_dev,
>  		 const struct iio_chan_spec *chan)
> @@ -1012,7 +1013,6 @@ static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
>  static int apply_acpi_orientation(struct iio_dev *indio_dev,
>  				  struct iio_chan_spec *channels)
>  {
> -#ifdef CONFIG_ACPI
>  	struct st_sensor_data *adata = iio_priv(indio_dev);
>  	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
>  	struct acpi_device *adev;
> @@ -1140,10 +1140,14 @@ static int apply_acpi_orientation(struct iio_dev *indio_dev,
>  out:
>  	kfree(buffer.pointer);
>  	return ret;
> +}
>  #else /* !CONFIG_ACPI */
> +static int apply_acpi_orientation(struct iio_dev *indio_dev,
> +				  struct iio_chan_spec *channels)
> +{
>  	return 0;
> -#endif
>  }
> +#endif
>  
>  /*
>   * st_accel_get_settings() - get sensor settings from device name


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 -next] iio: st_accel: Fix unused variable warning
  2019-11-16 14:58   ` Jonathan Cameron
@ 2019-11-18  1:41     ` Yuehaibing
  0 siblings, 0 replies; 10+ messages in thread
From: Yuehaibing @ 2019-11-18  1:41 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: knaack.h, lars, pmeerw, denis.ciocca, tglx, alexios.zavras,
	allison, linus.walleij, ladis, linux-iio, linux-kernel

On 2019/11/16 22:58, Jonathan Cameron wrote:
> On Mon, 11 Nov 2019 11:21:15 +0800
> YueHaibing <yuehaibing@huawei.com> wrote:
> 
>> drivers/iio/accel/st_accel_core.c:1005:44: warning:
>>  mount_matrix_ext_info defined but not used [-Wunused-const-variable=]
>>
>> Using stub helper while CONFIG_ACPI is disabled to fix it.
>>
>> Suggested-by: Ladislav Michl <ladis@linux-mips.org>
>> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Applied to the fixes-togreg branch of iio.git as I've now closed for new
> things for the coming merge window.  This will go upstream after the merge
> window closes and probably hit around rc2.  Thanks for cleaning this up.

Thanks!

> 
> Jonathan
> 
>> ---
>>  drivers/iio/accel/st_accel_core.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iio/accel/st_accel_core.c b/drivers/iio/accel/st_accel_core.c
>> index 7b83764..7320275 100644
>> --- a/drivers/iio/accel/st_accel_core.c
>> +++ b/drivers/iio/accel/st_accel_core.c
>> @@ -992,6 +992,7 @@ static const struct iio_trigger_ops st_accel_trigger_ops = {
>>  #define ST_ACCEL_TRIGGER_OPS NULL
>>  #endif
>>  
>> +#ifdef CONFIG_ACPI
>>  static const struct iio_mount_matrix *
>>  get_mount_matrix(const struct iio_dev *indio_dev,
>>  		 const struct iio_chan_spec *chan)
>> @@ -1012,7 +1013,6 @@ static const struct iio_chan_spec_ext_info mount_matrix_ext_info[] = {
>>  static int apply_acpi_orientation(struct iio_dev *indio_dev,
>>  				  struct iio_chan_spec *channels)
>>  {
>> -#ifdef CONFIG_ACPI
>>  	struct st_sensor_data *adata = iio_priv(indio_dev);
>>  	struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
>>  	struct acpi_device *adev;
>> @@ -1140,10 +1140,14 @@ static int apply_acpi_orientation(struct iio_dev *indio_dev,
>>  out:
>>  	kfree(buffer.pointer);
>>  	return ret;
>> +}
>>  #else /* !CONFIG_ACPI */
>> +static int apply_acpi_orientation(struct iio_dev *indio_dev,
>> +				  struct iio_chan_spec *channels)
>> +{
>>  	return 0;
>> -#endif
>>  }
>> +#endif
>>  
>>  /*
>>   * st_accel_get_settings() - get sensor settings from device name
> 
> 
> .
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-11-18  1:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-01 13:47 [PATCH -next] iio: st_accel: Fix unused variable warning YueHaibing
2019-11-02 10:41 ` Ladislav Michl
2019-11-02 14:08   ` Jonathan Cameron
2019-11-02 20:15     ` Ladislav Michl
2019-11-03 11:01       ` Jonathan Cameron
2019-11-04  1:29       ` Yuehaibing
2019-11-09 11:50         ` Jonathan Cameron
2019-11-11  3:21 ` [PATCH v2 " YueHaibing
2019-11-16 14:58   ` Jonathan Cameron
2019-11-18  1:41     ` Yuehaibing

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).