RESEND [PATCH v1] iio: potentiometer: mcp4131: fix double application of wiper shift

Posted by Lukas Schmid 4 days, 18 hours ago

The MCP4131 wiper address is shifted twice when preparing the SPI
command in mcp4131_write_raw().

The address is already shifted when assigned to the local variable
"address", but is then shifted again when written to data->buf[0].
This results in an incorrect command being sent to the device and
breaks wiper writes to the second channel.

Remove the second shift and use the pre-shifted address directly
when composing the SPI transfer.

Signed-off-by: Lukas Schmid <lukas.schmid@netcube.li>
---
 drivers/iio/potentiometer/mcp4131.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/potentiometer/mcp4131.c b/drivers/iio/potentiometer/mcp4131.c
index ad082827aad5..56c9111ef5e8 100644
--- a/drivers/iio/potentiometer/mcp4131.c
+++ b/drivers/iio/potentiometer/mcp4131.c
@@ -221,7 +221,7 @@ static int mcp4131_write_raw(struct iio_dev *indio_dev,
 
 	mutex_lock(&data->lock);
 
-	data->buf[0] = address << MCP4131_WIPER_SHIFT;
+	data->buf[0] = address;
 	data->buf[0] |= MCP4131_WRITE | (val >> 8);
 	data->buf[1] = val & 0xFF; /* 8 bits here */
 
-- 
2.47.3

Re: RESEND [PATCH v1] iio: potentiometer: mcp4131: fix double application of wiper shift

Posted by Jonathan Cameron 4 days, 18 hours ago

On Mon,  2 Feb 2026 21:15:35 +0100
Lukas Schmid <lukas.schmid@netcube.li> wrote:

> The MCP4131 wiper address is shifted twice when preparing the SPI
> command in mcp4131_write_raw().
> 
> The address is already shifted when assigned to the local variable
> "address", but is then shifted again when written to data->buf[0].
> This results in an incorrect command being sent to the device and
> breaks wiper writes to the second channel.
> 
> Remove the second shift and use the pre-shifted address directly
> when composing the SPI transfer.
> 
> Signed-off-by: Lukas Schmid <lukas.schmid@netcube.li>
Hi Lukas,

Good find.  
Needs a fixes tag though so that we can know how far to backport it.
looks like this one goes all the way, so I'll just reply here and
pick this up in a minute once my email reaches lore.

Fixes: 22d199a53910 ("iio: potentiometer: add driver for Microchip MCP413X/414X/415X/416X/423X/424X/425X/426X")


Thanks,

Jonathan

> ---
>  drivers/iio/potentiometer/mcp4131.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/potentiometer/mcp4131.c b/drivers/iio/potentiometer/mcp4131.c
> index ad082827aad5..56c9111ef5e8 100644
> --- a/drivers/iio/potentiometer/mcp4131.c
> +++ b/drivers/iio/potentiometer/mcp4131.c
> @@ -221,7 +221,7 @@ static int mcp4131_write_raw(struct iio_dev *indio_dev,
>  
>  	mutex_lock(&data->lock);
>  
> -	data->buf[0] = address << MCP4131_WIPER_SHIFT;
> +	data->buf[0] = address;
>  	data->buf[0] |= MCP4131_WRITE | (val >> 8);
>  	data->buf[1] = val & 0xFF; /* 8 bits here */
>

Re: RESEND [PATCH v1] iio: potentiometer: mcp4131: fix double application of wiper shift

Posted by Jonathan Cameron 4 days, 18 hours ago

On Mon, 2 Feb 2026 20:29:20 +0000
Jonathan Cameron <jic23@kernel.org> wrote:

> On Mon,  2 Feb 2026 21:15:35 +0100
> Lukas Schmid <lukas.schmid@netcube.li> wrote:
> 
> > The MCP4131 wiper address is shifted twice when preparing the SPI
> > command in mcp4131_write_raw().
> > 
> > The address is already shifted when assigned to the local variable
> > "address", but is then shifted again when written to data->buf[0].
> > This results in an incorrect command being sent to the device and
> > breaks wiper writes to the second channel.
> > 
> > Remove the second shift and use the pre-shifted address directly
> > when composing the SPI transfer.
> > 
> > Signed-off-by: Lukas Schmid <lukas.schmid@netcube.li>  
> Hi Lukas,
> 
> Good find.  
> Needs a fixes tag though so that we can know how far to backport it.
> looks like this one goes all the way, so I'll just reply here and
> pick this up in a minute once my email reaches lore.
> 
> Fixes: 22d199a53910 ("iio: potentiometer: add driver for Microchip MCP413X/414X/415X/416X/423X/424X/425X/426X")

I picked it up by hand.  A few things to note for next time.
If you do add RESEND then inside the [] so that the tooling maintainers
tend to use to pick stuff up (b4 / git am etc) doesn't included the
RESEND [PATCH v1] in the title of the commit.

Also, if you resend, please include a comment below the --- on why you
did so.

Now applied to my local tree as I'll be rebasing the fixes branch after
the release next weekend and am not planning to do a pull request before
then.

Thanks,

Jonathan

> 
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/iio/potentiometer/mcp4131.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/potentiometer/mcp4131.c b/drivers/iio/potentiometer/mcp4131.c
> > index ad082827aad5..56c9111ef5e8 100644
> > --- a/drivers/iio/potentiometer/mcp4131.c
> > +++ b/drivers/iio/potentiometer/mcp4131.c
> > @@ -221,7 +221,7 @@ static int mcp4131_write_raw(struct iio_dev *indio_dev,
> >  
> >  	mutex_lock(&data->lock);
> >  
> > -	data->buf[0] = address << MCP4131_WIPER_SHIFT;
> > +	data->buf[0] = address;
> >  	data->buf[0] |= MCP4131_WRITE | (val >> 8);
> >  	data->buf[1] = val & 0xFF; /* 8 bits here */
> >    
>

Re: RESEND [PATCH v1] iio: potentiometer: mcp4131: fix double application of wiper shift

Posted by Lukas Schmid 4 days, 17 hours ago

On Montag, 2. Februar 2026 21:34:43 Mitteleuropäische Normalzeit Jonathan 
Cameron wrote:
> On Mon, 2 Feb 2026 20:29:20 +0000
> 
> Jonathan Cameron <jic23@kernel.org> wrote:
> > On Mon,  2 Feb 2026 21:15:35 +0100
> > 
> > Lukas Schmid <lukas.schmid@netcube.li> wrote:
> > > The MCP4131 wiper address is shifted twice when preparing the SPI
> > > command in mcp4131_write_raw().
> > > 
> > > The address is already shifted when assigned to the local variable
> > > "address", but is then shifted again when written to data->buf[0].
> > > This results in an incorrect command being sent to the device and
> > > breaks wiper writes to the second channel.
> > > 
> > > Remove the second shift and use the pre-shifted address directly
> > > when composing the SPI transfer.
> > > 
> > > Signed-off-by: Lukas Schmid <lukas.schmid@netcube.li>
> > 
> > Hi Lukas,
> > 
> > Good find.
> > Needs a fixes tag though so that we can know how far to backport it.
> > looks like this one goes all the way, so I'll just reply here and
> > pick this up in a minute once my email reaches lore.
> > 
> > Fixes: 22d199a53910 ("iio: potentiometer: add driver for Microchip
> > MCP413X/414X/415X/416X/423X/424X/425X/426X")
> I picked it up by hand.  A few things to note for next time.
> If you do add RESEND then inside the [] so that the tooling maintainers
> tend to use to pick stuff up (b4 / git am etc) doesn't included the
> RESEND [PATCH v1] in the title of the commit.
> 
> Also, if you resend, please include a comment below the --- on why you
> did so.
> 
> Now applied to my local tree as I'll be rebasing the fixes branch after
> the release next weekend and am not planning to do a pull request before
> then.
> 
Hi Jonathan,

thank you for your hints. Will make sure to follow them next time.

Best regards,
Lukas

> Thanks,
> 
> Jonathan
> 
> > Thanks,
> > 
> > Jonathan
> > 
> > > ---
> > > 
> > >  drivers/iio/potentiometer/mcp4131.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/iio/potentiometer/mcp4131.c
> > > b/drivers/iio/potentiometer/mcp4131.c index ad082827aad5..56c9111ef5e8
> > > 100644
> > > --- a/drivers/iio/potentiometer/mcp4131.c
> > > +++ b/drivers/iio/potentiometer/mcp4131.c
> > > @@ -221,7 +221,7 @@ static int mcp4131_write_raw(struct iio_dev
> > > *indio_dev,
> > > 
> > >  	mutex_lock(&data->lock);
> > > 
> > > -	data->buf[0] = address << MCP4131_WIPER_SHIFT;
> > > +	data->buf[0] = address;
> > > 
> > >  	data->buf[0] |= MCP4131_WRITE | (val >> 8);
> > >  	data->buf[1] = val & 0xFF; /* 8 bits here */