[PATCH] staging: iio: ad5933: Fix implicit fall-through in switch()

[PATCH] staging: iio: ad5933: Fix implicit fall-through in switch()

[Akhilesh Patil]

Add default case in switch() codeblock in ad5933_read_raw().
Convert implicit error return due to switch fallthrough to explicit return
to make intent clear. Follow kernel switch fall-thorugh guidelines at
Documentation/process/deprecated.rst

Signed-off-by: Akhilesh Patil <akhilesh@ee.iitb.ac.in>
---
Checked build for 6.16.0 kernel with ad5933
---
 drivers/staging/iio/impedance-analyzer/ad5933.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 85a4223295cd..6547a259b8a0 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -533,9 +533,10 @@ static int ad5933_read_raw(struct iio_dev *indio_dev,
 		*val = 1000;
 		*val2 = 5;
 		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
 	}
 
-	return -EINVAL;
 out:
 	iio_device_release_direct(indio_dev);
 	return ret;
-- 
2.34.1

Re: [PATCH] staging: iio: ad5933: Fix implicit fall-through in switch()

[Greg KH]

On Mon, Jul 28, 2025 at 03:29:28PM +0530, Akhilesh Patil wrote:
> Add default case in switch() codeblock in ad5933_read_raw().
> Convert implicit error return due to switch fallthrough to explicit return
> to make intent clear. Follow kernel switch fall-thorugh guidelines at
> Documentation/process/deprecated.rst
> 
> Signed-off-by: Akhilesh Patil <akhilesh@ee.iitb.ac.in>
> ---
> Checked build for 6.16.0 kernel with ad5933
> ---
>  drivers/staging/iio/impedance-analyzer/ad5933.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index 85a4223295cd..6547a259b8a0 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -533,9 +533,10 @@ static int ad5933_read_raw(struct iio_dev *indio_dev,
>  		*val = 1000;
>  		*val2 = 5;
>  		return IIO_VAL_FRACTIONAL_LOG2;
> +	default:
> +		return -EINVAL;

What tool is requiring this to be added?  It's totally redundant and
needs to have the tool fixed instead.

sorry,

greg k-h

Re: [PATCH] staging: iio: ad5933: Fix implicit fall-through in switch()

[Akhilesh Patil]

On Mon, Jul 28, 2025 at 12:39:21PM +0200, Greg KH wrote:
> On Mon, Jul 28, 2025 at 03:29:28PM +0530, Akhilesh Patil wrote:
> > Add default case in switch() codeblock in ad5933_read_raw().
> > Convert implicit error return due to switch fallthrough to explicit return
> > to make intent clear. Follow kernel switch fall-thorugh guidelines at
> > Documentation/process/deprecated.rst
> > 
> > Signed-off-by: Akhilesh Patil <akhilesh@ee.iitb.ac.in>
> > ---
> > Checked build for 6.16.0 kernel with ad5933
> > ---
> >  drivers/staging/iio/impedance-analyzer/ad5933.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > index 85a4223295cd..6547a259b8a0 100644
> > --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> > +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > @@ -533,9 +533,10 @@ static int ad5933_read_raw(struct iio_dev *indio_dev,
> >  		*val = 1000;
> >  		*val2 = 5;
> >  		return IIO_VAL_FRACTIONAL_LOG2;
> > +	default:
> > +		return -EINVAL;
> 
> What tool is requiring this to be added?  It's totally redundant and
> needs to have the tool fixed instead.

This patch is not inspired by any tool as such.
I observed this code pattern while manually reading the staging area iio
code. From my eyes, there is implicit intention to return from switch block if
no match is found which can be improved in readibility by explicit
default block returning error.
I agree this is redundant and will not have any functional impact.
However, imo - this can help support kernel wide efforts to
clarify switch() blocks.

The motivation for this patch is from a035d552 which talks about
eleminating ambiguity by clearly defining swich() case blocks.

Thanks for the review, 

Akhilesh

> 
> sorry,
> 
> greg k-h

Re: [PATCH] staging: iio: ad5933: Fix implicit fall-through in switch()

[Greg KH]

On Mon, Jul 28, 2025 at 07:32:54PM +0530, Akhilesh Patil wrote:
> On Mon, Jul 28, 2025 at 12:39:21PM +0200, Greg KH wrote:
> > On Mon, Jul 28, 2025 at 03:29:28PM +0530, Akhilesh Patil wrote:
> > > Add default case in switch() codeblock in ad5933_read_raw().
> > > Convert implicit error return due to switch fallthrough to explicit return
> > > to make intent clear. Follow kernel switch fall-thorugh guidelines at
> > > Documentation/process/deprecated.rst
> > > 
> > > Signed-off-by: Akhilesh Patil <akhilesh@ee.iitb.ac.in>
> > > ---
> > > Checked build for 6.16.0 kernel with ad5933
> > > ---
> > >  drivers/staging/iio/impedance-analyzer/ad5933.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > > index 85a4223295cd..6547a259b8a0 100644
> > > --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> > > +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > > @@ -533,9 +533,10 @@ static int ad5933_read_raw(struct iio_dev *indio_dev,
> > >  		*val = 1000;
> > >  		*val2 = 5;
> > >  		return IIO_VAL_FRACTIONAL_LOG2;
> > > +	default:
> > > +		return -EINVAL;
> > 
> > What tool is requiring this to be added?  It's totally redundant and
> > needs to have the tool fixed instead.
> 
> This patch is not inspired by any tool as such.
> I observed this code pattern while manually reading the staging area iio
> code. From my eyes, there is implicit intention to return from switch block if
> no match is found which can be improved in readibility by explicit
> default block returning error.
> I agree this is redundant and will not have any functional impact.
> However, imo - this can help support kernel wide efforts to
> clarify switch() blocks.
> 
> The motivation for this patch is from a035d552 which talks about
> eleminating ambiguity by clearly defining swich() case blocks.

Yes, but the code right after this does the "default return", so that is
now dead code.

I'd recommend the "pattern" that the current code is in, it's simpler.

thanks,

greg k-h

Re: [PATCH] staging: iio: ad5933: Fix implicit fall-through in switch()

[Andy Shevchenko]

On Mon, Jul 28, 2025 at 04:23:49PM +0200, Greg KH wrote:
> On Mon, Jul 28, 2025 at 07:32:54PM +0530, Akhilesh Patil wrote:
> > On Mon, Jul 28, 2025 at 12:39:21PM +0200, Greg KH wrote:
> > > On Mon, Jul 28, 2025 at 03:29:28PM +0530, Akhilesh Patil wrote:

...

> > > > +	default:
> > > > +		return -EINVAL;
> > > 
> > > What tool is requiring this to be added?  It's totally redundant and
> > > needs to have the tool fixed instead.
> > 
> > This patch is not inspired by any tool as such.
> > I observed this code pattern while manually reading the staging area iio
> > code. From my eyes, there is implicit intention to return from switch block if
> > no match is found which can be improved in readibility by explicit
> > default block returning error.
> > I agree this is redundant and will not have any functional impact.
> > However, imo - this can help support kernel wide efforts to
> > clarify switch() blocks.
> > 
> > The motivation for this patch is from a035d552 which talks about
> > eleminating ambiguity by clearly defining swich() case blocks.
> 
> Yes, but the code right after this does the "default return", so that is
> now dead code.

Hmm... If I read the code correctly it is either already was a dead code before
that patch, or it's still accessible via goto label.

> I'd recommend the "pattern" that the current code is in, it's simpler.

The pattern to return from all switch cases, including default is commonly used
in IIO drivers.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] staging: iio: ad5933: Fix implicit fall-through in switch()

[Akhilesh Patil]

On Wed, Aug 06, 2025 at 12:16:14AM +0300, Andy Shevchenko wrote:
> On Mon, Jul 28, 2025 at 04:23:49PM +0200, Greg KH wrote:
> > On Mon, Jul 28, 2025 at 07:32:54PM +0530, Akhilesh Patil wrote:
> > > On Mon, Jul 28, 2025 at 12:39:21PM +0200, Greg KH wrote:
> > > > On Mon, Jul 28, 2025 at 03:29:28PM +0530, Akhilesh Patil wrote:
> 
> ...
> 
> > > > > +	default:
> > > > > +		return -EINVAL;
> > > > 
> > > > What tool is requiring this to be added?  It's totally redundant and
> > > > needs to have the tool fixed instead.
> > > 
> > > This patch is not inspired by any tool as such.
> > > I observed this code pattern while manually reading the staging area iio
> > > code. From my eyes, there is implicit intention to return from switch block if
> > > no match is found which can be improved in readibility by explicit
> > > default block returning error.
> > > I agree this is redundant and will not have any functional impact.
> > > However, imo - this can help support kernel wide efforts to
> > > clarify switch() blocks.
> > > 
> > > The motivation for this patch is from a035d552 which talks about
> > > eleminating ambiguity by clearly defining swich() case blocks.
> > 
> > Yes, but the code right after this does the "default return", so that is
> > now dead code.
> 
> Hmm... If I read the code correctly it is either already was a dead code before
> that patch, or it's still accessible via goto label.
> 
> > I'd recommend the "pattern" that the current code is in, it's simpler.
> 
> The pattern to return from all switch cases, including default is commonly used
> in IIO drivers.
Thanks Andy for your view here. Yes, this pattern is seen in iio drivers
including dead code as we discussed here.
For example, v6.16: drivers/iio/chemical/sgp30.c:406-411 shows same
pattern as this patch will introduce.

hence, IMO this patch can help this staging code align with coding
pattern of iio drivers. However even without this patch there is no
functional impact and it can be skipped also, I am okay either ways :)

Regards,
Akhilesh

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

Re: [PATCH] staging: iio: ad5933: Fix implicit fall-through in switch()

[Andy Shevchenko]

On Fri, Aug 8, 2025 at 11:57 AM Akhilesh Patil <akhilesh@ee.iitb.ac.in> wrote:
> On Wed, Aug 06, 2025 at 12:16:14AM +0300, Andy Shevchenko wrote:
> > On Mon, Jul 28, 2025 at 04:23:49PM +0200, Greg KH wrote:
> > > On Mon, Jul 28, 2025 at 07:32:54PM +0530, Akhilesh Patil wrote:
> > > > On Mon, Jul 28, 2025 at 12:39:21PM +0200, Greg KH wrote:
> > > > > On Mon, Jul 28, 2025 at 03:29:28PM +0530, Akhilesh Patil wrote:

...

> > > > > > +     default:
> > > > > > +             return -EINVAL;
> > > > >
> > > > > What tool is requiring this to be added?  It's totally redundant and
> > > > > needs to have the tool fixed instead.
> > > >
> > > > This patch is not inspired by any tool as such.
> > > > I observed this code pattern while manually reading the staging area iio
> > > > code. From my eyes, there is implicit intention to return from switch block if
> > > > no match is found which can be improved in readibility by explicit
> > > > default block returning error.
> > > > I agree this is redundant and will not have any functional impact.
> > > > However, imo - this can help support kernel wide efforts to
> > > > clarify switch() blocks.
> > > >
> > > > The motivation for this patch is from a035d552 which talks about
> > > > eleminating ambiguity by clearly defining swich() case blocks.
> > >
> > > Yes, but the code right after this does the "default return", so that is
> > > now dead code.
> >
> > Hmm... If I read the code correctly it is either already was a dead code before
> > that patch, or it's still accessible via goto label.
> >
> > > I'd recommend the "pattern" that the current code is in, it's simpler.
> >
> > The pattern to return from all switch cases, including default is commonly used
> > in IIO drivers.

> Thanks Andy for your view here. Yes, this pattern is seen in iio drivers
> including dead code as we discussed here.
> For example, v6.16: drivers/iio/chemical/sgp30.c:406-411 shows same
> pattern as this patch will introduce.
>
> hence, IMO this patch can help this staging code align with coding
> pattern of iio drivers. However even without this patch there is no
> functional impact and it can be skipped also, I am okay either ways :)

Still, this leaves a goto label and makes code not really easy to
read. What I recommend for all to be satisfied is to refactor the
function so that it won't have a goto label after the switch. In this
case it will be okay to use the above pattern and also for people not
questioning the correctness of the code. Without that done I would
tend to agree with Greg on the basis of unneeded churn.

-- 
With Best Regards,
Andy Shevchenko