Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1714527 > unrolled thread

Re: [PATCH 2/5] iio: Add support for LMP92001 ADC

Started byjmondi <jacopo@jmondi.org>
First post2017-08-18 05:00 +0200
Last post2017-08-21 19:40 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/5] iio: Add support for LMP92001 ADC jmondi <jacopo@jmondi.org> - 2017-08-18 05:00 +0200
    Re: [PATCH 2/5] iio: Add support for LMP92001 ADC Jonathan Cameron <jic23@kernel.org> - 2017-08-20 12:40 +0200
      Re: [PATCH 2/5] iio: Add support for LMP92001 ADC jmondi <jacopo@jmondi.org> - 2017-08-21 19:40 +0200

#1714527 — Re: [PATCH 2/5] iio: Add support for LMP92001 ADC

Fromjmondi <jacopo@jmondi.org>
Date2017-08-18 05:00 +0200
SubjectRe: [PATCH 2/5] iio: Add support for LMP92001 ADC
Message-ID<ufFFU-5xy-3@gated-at.bofh.it>
Hi Abhisit,

On Fri, Aug 18, 2017 at 09:34:16AM +0700, Abhisit Sangjan wrote:
> Hi Jmondi,
>
> Thank you for your recommend, I am testing the code will be send the new
> patch in soon.

[snip]

> > > > +
> > > > +        switch (mask)
> > > > +        {
> > > > +        case IIO_CHAN_INFO_RAW:
> > > > +                switch (channel->type) {
> > > > +                case IIO_VOLTAGE:
> > > > +                case IIO_TEMP:
> > > > +                        *val = code;
> > > > +                        return IIO_VAL_INT;
> > > > +                default:
> > > > +                        break;
> > > > +                }
> > > > +                break;
> > > > +        default:
> > > > +                break;
> >
> > You can remove these default cases or return -EINVAL here.
> >
>
> Abhisit: Okay, I will remove it.
>              Could you tell me in detail. Sorry, I do not understand the
> Technical.

This can potentially be reduced to

        switch (mask) {
        case IIO_CHAN_INFO_RAW:
                switch (channel->type) {
                case IIO_VOLTAGE:
                case IIO_TEMP:
                        *val = code;
                        return IIO_VAL_INT;
                }
         }

         return -EINVAL;


But that's definitely not a big deal, there are no optimization in
this code change, just less typing and less default: and break; here
and there

[toc] | [next] | [standalone]

#1715916

FromJonathan Cameron <jic23@kernel.org>
Date2017-08-20 12:40 +0200
Message-ID<ugvOa-5NO-25@gated-at.bofh.it>
In reply to#1714527
On Fri, 18 Aug 2017 14:42:59 +0700
Abhisit Sangjan <s.abhisit@gmail.com> wrote:

> Hi Jmondi,
> 
> After I removed those cases, I got warnings "no handled in switch".
> 
> On Fri, Aug 18, 2017 at 10:15 AM, Abhisit Sangjan <s.abhisit@gmail.com>
> wrote:
> 
> > Hi Jmondi,
> >
> > On Fri, Aug 18, 2017 at 9:58 AM, jmondi <jacopo@jmondi.org> wrote:
> >  
> >> Hi Abhisit,
> >>
> >> On Fri, Aug 18, 2017 at 09:34:16AM +0700, Abhisit Sangjan wrote:  
> >> > Hi Jmondi,
> >> >
> >> > Thank you for your recommend, I am testing the code will be send the new
> >> > patch in soon.  
> >>
> >> [snip]
> >>  
> >> > > > > +
> >> > > > > +        switch (mask)
> >> > > > > +        {
> >> > > > > +        case IIO_CHAN_INFO_RAW:
> >> > > > > +                switch (channel->type) {
> >> > > > > +                case IIO_VOLTAGE:
> >> > > > > +                case IIO_TEMP:
> >> > > > > +                        *val = code;
> >> > > > > +                        return IIO_VAL_INT;
> >> > > > > +                default:
> >> > > > > +                        break;
> >> > > > > +                }
> >> > > > > +                break;
> >> > > > > +        default:
> >> > > > > +                break;  
> >> > >
> >> > > You can remove these default cases or return -EINVAL here.
> >> > >  
> >> >
> >> > Abhisit: Okay, I will remove it.
> >> >              Could you tell me in detail. Sorry, I do not understand the
> >> > Technical.  
> >>
> >> This can potentially be reduced to
> >>
> >>         switch (mask) {
> >>         case IIO_CHAN_INFO_RAW:
> >>                 switch (channel->type) {
> >>                 case IIO_VOLTAGE:
> >>                 case IIO_TEMP:
> >>                         *val = code;
> >>                         return IIO_VAL_INT;
> >>                 }
            default:
	         return -EINVAL;
> >>          }
> >>
get rid of the below.
> >>          return -EINVAL;

Sorry, had forgotten about that warning!

Jonathan

> >>
> >>
> >> But that's definitely not a big deal, there are no optimization in
> >> this code change, just less typing and less default: and break; here
> >> and there
> >>  
> >
> > Abhisit: Thank you so much.
> >  
> 
> Abhisit: If I remove those default cases, I got the warning. How do I would
> do next to fix warning?  Should I leave this code as it?
> 
> # What have I changed.
> diff --git a/drivers/iio/adc/lmp92001-adc.c b/drivers/iio/adc/lmp92001-adc.c
> index 68f7a6c..ebc6423 100644
> --- a/drivers/iio/adc/lmp92001-adc.c
> +++ b/drivers/iio/adc/lmp92001-adc.c
> @@ -92,12 +92,7 @@ static int lmp92001_read_raw(struct iio_dev *indio_dev,
>                 case IIO_TEMP:
>                         *val = code;
>                         return IIO_VAL_INT;
> -               default:
> -                       break;
>                 }
> -               break;
> -       default:
> -               break;
>         }
> 
>         return -EINVAL;
> 
> # Compilation.
>   CC      drivers/iio/adc/lmp92001-adc.o
> drivers/iio/adc/lmp92001-adc.c: In function ‘lmp92001_read_raw’:
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_CURRENT’ not handled in switch [-Wswitch]
>    switch (channel->type) {
>    ^
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_POWER’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_ACCEL’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_ANGL_VEL’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_MAGN’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_LIGHT’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_INTENSITY’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_PROXIMITY’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_INCLI’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_ROT’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_ANGL’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_TIMESTAMP’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_CAPACITANCE’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_ALTVOLTAGE’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_CCT’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_PRESSURE’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_HUMIDITYRELATIVE’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_ACTIVITY’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_STEPS’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_ENERGY’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_DISTANCE’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_VELOCITY’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_CONCENTRATION’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_RESISTANCE’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_PH’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_UVINDEX’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_ELECTRICALCONDUCTIVITY’ not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_COUNT’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_INDEX’
> not handled in switch [-Wswitch]
> drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> ‘IIO_GRAVITY’ not handled in switch [-Wswitch]
> 
> 
> >
> >
> >

[toc] | [prev] | [next] | [standalone]

#1716751

Fromjmondi <jacopo@jmondi.org>
Date2017-08-21 19:40 +0200
Message-ID<ugYQa-7jc-19@gated-at.bofh.it>
In reply to#1715916
Hi Abhisit,

On Sun, Aug 20, 2017 at 11:31:41AM +0100, Jonathan Cameron wrote:
> On Fri, 18 Aug 2017 14:42:59 +0700
> Abhisit Sangjan <s.abhisit@gmail.com> wrote:
>
> > Hi Jmondi,
> >
> > After I removed those cases, I got warnings "no handled in switch".
> >
> > On Fri, Aug 18, 2017 at 10:15 AM, Abhisit Sangjan <s.abhisit@gmail.com>
> > wrote:
> >
> > > Hi Jmondi,
> > >
> > > On Fri, Aug 18, 2017 at 9:58 AM, jmondi <jacopo@jmondi.org> wrote:
> > >
> > >> Hi Abhisit,
> > >>
> > >> On Fri, Aug 18, 2017 at 09:34:16AM +0700, Abhisit Sangjan wrote:
> > >> > Hi Jmondi,
> > >> >
> > >> > Thank you for your recommend, I am testing the code will be send the new
> > >> > patch in soon.
> > >>
> > >> [snip]
> > >>
> > >> > > > > +
> > >> > > > > +        switch (mask)
> > >> > > > > +        {
> > >> > > > > +        case IIO_CHAN_INFO_RAW:
> > >> > > > > +                switch (channel->type) {
> > >> > > > > +                case IIO_VOLTAGE:
> > >> > > > > +                case IIO_TEMP:
> > >> > > > > +                        *val = code;
> > >> > > > > +                        return IIO_VAL_INT;
> > >> > > > > +                default:
> > >> > > > > +                        break;
> > >> > > > > +                }
> > >> > > > > +                break;
> > >> > > > > +        default:
> > >> > > > > +                break;
> > >> > >
> > >> > > You can remove these default cases or return -EINVAL here.
> > >> > >
> > >> >
> > >> > Abhisit: Okay, I will remove it.
> > >> >              Could you tell me in detail. Sorry, I do not understand the
> > >> > Technical.
> > >>
> > >> This can potentially be reduced to
> > >>
> > >>         switch (mask) {
> > >>         case IIO_CHAN_INFO_RAW:
> > >>                 switch (channel->type) {
> > >>                 case IIO_VOLTAGE:
> > >>                 case IIO_TEMP:
> > >>                         *val = code;
> > >>                         return IIO_VAL_INT;
> > >>                 }
>             default:
> 	         return -EINVAL;
> > >>          }
> > >>
> get rid of the below.
> > >>          return -EINVAL;
>
> Sorry, had forgotten about that warning!

My bad, as I have suggested but not compiled that code.
Thanks Jonathan!

>
> Jonathan
>
> > >>
> > >>
> > >> But that's definitely not a big deal, there are no optimization in
> > >> this code change, just less typing and less default: and break; here
> > >> and there
> > >>
> > >
> > > Abhisit: Thank you so much.
> > >
> >
> > Abhisit: If I remove those default cases, I got the warning. How do I would
> > do next to fix warning?  Should I leave this code as it?
> >
> > # What have I changed.
> > diff --git a/drivers/iio/adc/lmp92001-adc.c b/drivers/iio/adc/lmp92001-adc.c
> > index 68f7a6c..ebc6423 100644
> > --- a/drivers/iio/adc/lmp92001-adc.c
> > +++ b/drivers/iio/adc/lmp92001-adc.c
> > @@ -92,12 +92,7 @@ static int lmp92001_read_raw(struct iio_dev *indio_dev,
> >                 case IIO_TEMP:
> >                         *val = code;
> >                         return IIO_VAL_INT;
> > -               default:
> > -                       break;
> >                 }
> > -               break;
> > -       default:
> > -               break;
> >         }
> >
> >         return -EINVAL;
> >
> > # Compilation.
> >   CC      drivers/iio/adc/lmp92001-adc.o
> > drivers/iio/adc/lmp92001-adc.c: In function ‘lmp92001_read_raw’:
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_CURRENT’ not handled in switch [-Wswitch]
> >    switch (channel->type) {
> >    ^
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_POWER’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_ACCEL’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_ANGL_VEL’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_MAGN’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_LIGHT’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_INTENSITY’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_PROXIMITY’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_INCLI’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_ROT’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_ANGL’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_TIMESTAMP’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_CAPACITANCE’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_ALTVOLTAGE’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_CCT’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_PRESSURE’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_HUMIDITYRELATIVE’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_ACTIVITY’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_STEPS’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_ENERGY’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_DISTANCE’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_VELOCITY’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_CONCENTRATION’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_RESISTANCE’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_PH’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_UVINDEX’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_ELECTRICALCONDUCTIVITY’ not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_COUNT’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value ‘IIO_INDEX’
> > not handled in switch [-Wswitch]
> > drivers/iio/adc/lmp92001-adc.c:90:3: warning: enumeration value
> > ‘IIO_GRAVITY’ not handled in switch [-Wswitch]
> >
> >
> > >
> > >
> > >
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web