linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] bus: sunxi-rsb: Return correct data when mixing 16-bit and 8-bit reads
@ 2020-02-19  1:09 Ondrej Jirman
  2020-02-19  2:49 ` Chen-Yu Tsai
  2020-02-20 17:32 ` Maxime Ripard
  0 siblings, 2 replies; 6+ messages in thread
From: Ondrej Jirman @ 2020-02-19  1:09 UTC (permalink / raw)
  To: linux-sunxi
  Cc: Ondrej Jirman, Maxime Ripard, Chen-Yu Tsai, Samuel Holland,
	Stephen Boyd, moderated list:ARM/Allwinner sunXi SoC support,
	open list

When doing a 16-bit read that returns data in the MSB byte, the
RSB_DATA register will keep the MSB byte unchanged when doing
the following 8-bit read. sunxi_rsb_read() will then return
a result that contains high byte from 16-bit read mixed with
the 8-bit result.

The consequence is that after this happens the PMIC's regmap will
look like this: (0x33 is the high byte from the 16-bit read)

% cat /sys/kernel/debug/regmap/sunxi-rsb-3a3/registers
00: 33
01: 33
02: 33
03: 33
04: 33
05: 33
06: 33
07: 33
08: 33
09: 33
0a: 33
0b: 33
0c: 33
0d: 33
0e: 33
[snip]

Fix this by masking the result of the read with the correct mask
based on the size of the read. There are no 16-bit users in the
mainline kernel, so this doesn't need to get into the stable tree.

Signed-off-by: Ondrej Jirman <megous@megous.com>
---
 drivers/bus/sunxi-rsb.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
index b8043b58568ac..8ab6a3865f569 100644
--- a/drivers/bus/sunxi-rsb.c
+++ b/drivers/bus/sunxi-rsb.c
@@ -316,6 +316,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
 {
 	u32 cmd;
 	int ret;
+	u32 mask;
 
 	if (!buf)
 		return -EINVAL;
@@ -323,12 +324,15 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
 	switch (len) {
 	case 1:
 		cmd = RSB_CMD_RD8;
+		mask = 0xffu;
 		break;
 	case 2:
 		cmd = RSB_CMD_RD16;
+		mask = 0xffffu;
 		break;
 	case 4:
 		cmd = RSB_CMD_RD32;
+		mask = 0xffffffffu;
 		break;
 	default:
 		dev_err(rsb->dev, "Invalid access width: %zd\n", len);
@@ -345,7 +349,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
 	if (ret)
 		goto unlock;
 
-	*buf = readl(rsb->regs + RSB_DATA);
+	*buf = readl(rsb->regs + RSB_DATA) & mask;
 
 unlock:
 	mutex_unlock(&rsb->lock);
-- 
2.25.1


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

* Re: [PATCH] bus: sunxi-rsb: Return correct data when mixing 16-bit and 8-bit reads
  2020-02-19  1:09 [PATCH] bus: sunxi-rsb: Return correct data when mixing 16-bit and 8-bit reads Ondrej Jirman
@ 2020-02-19  2:49 ` Chen-Yu Tsai
  2020-02-19 12:14   ` Ondřej Jirman
  2020-02-20 17:32 ` Maxime Ripard
  1 sibling, 1 reply; 6+ messages in thread
From: Chen-Yu Tsai @ 2020-02-19  2:49 UTC (permalink / raw)
  To: Ondrej Jirman
  Cc: linux-sunxi, Maxime Ripard, Samuel Holland, Stephen Boyd,
	moderated list:ARM/Allwinner sunXi SoC support, open list

On Wed, Feb 19, 2020 at 9:10 AM Ondrej Jirman <megous@megous.com> wrote:
>
> When doing a 16-bit read that returns data in the MSB byte, the
> RSB_DATA register will keep the MSB byte unchanged when doing
> the following 8-bit read. sunxi_rsb_read() will then return
> a result that contains high byte from 16-bit read mixed with
> the 8-bit result.
>
> The consequence is that after this happens the PMIC's regmap will
> look like this: (0x33 is the high byte from the 16-bit read)
>
> % cat /sys/kernel/debug/regmap/sunxi-rsb-3a3/registers
> 00: 33
> 01: 33
> 02: 33
> 03: 33
> 04: 33
> 05: 33
> 06: 33
> 07: 33
> 08: 33
> 09: 33
> 0a: 33
> 0b: 33
> 0c: 33
> 0d: 33
> 0e: 33
> [snip]
>
> Fix this by masking the result of the read with the correct mask
> based on the size of the read. There are no 16-bit users in the
> mainline kernel, so this doesn't need to get into the stable tree.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>

Acked-by: Chen-Yu Tsai <wens@csie.org>

for the fix, however it's not entirely clear to me how the MSB 0x33
value got into the regmap. Looks like I expected the regmap core to
handle any overflows, or didn't expect the lingering MSB from 16-bit
reads, as I didn't have any 16-bit register devices back when I wrote
this.

ChenYu


> ---
>  drivers/bus/sunxi-rsb.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> index b8043b58568ac..8ab6a3865f569 100644
> --- a/drivers/bus/sunxi-rsb.c
> +++ b/drivers/bus/sunxi-rsb.c
> @@ -316,6 +316,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
>  {
>         u32 cmd;
>         int ret;
> +       u32 mask;
>
>         if (!buf)
>                 return -EINVAL;
> @@ -323,12 +324,15 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
>         switch (len) {
>         case 1:
>                 cmd = RSB_CMD_RD8;
> +               mask = 0xffu;
>                 break;
>         case 2:
>                 cmd = RSB_CMD_RD16;
> +               mask = 0xffffu;
>                 break;
>         case 4:
>                 cmd = RSB_CMD_RD32;
> +               mask = 0xffffffffu;
>                 break;
>         default:
>                 dev_err(rsb->dev, "Invalid access width: %zd\n", len);
> @@ -345,7 +349,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
>         if (ret)
>                 goto unlock;
>
> -       *buf = readl(rsb->regs + RSB_DATA);
> +       *buf = readl(rsb->regs + RSB_DATA) & mask;
>
>  unlock:
>         mutex_unlock(&rsb->lock);
> --
> 2.25.1
>

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

* Re: [PATCH] bus: sunxi-rsb: Return correct data when mixing 16-bit and 8-bit reads
  2020-02-19  2:49 ` Chen-Yu Tsai
@ 2020-02-19 12:14   ` Ondřej Jirman
  2020-02-21  3:04     ` [linux-sunxi] " Chen-Yu Tsai
  0 siblings, 1 reply; 6+ messages in thread
From: Ondřej Jirman @ 2020-02-19 12:14 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: linux-sunxi, Maxime Ripard, Samuel Holland, Stephen Boyd,
	moderated list:ARM/Allwinner sunXi SoC support, open list

On Wed, Feb 19, 2020 at 10:49:18AM +0800, Chen-Yu Tsai wrote:
> On Wed, Feb 19, 2020 at 9:10 AM Ondrej Jirman <megous@megous.com> wrote:
> >
> > When doing a 16-bit read that returns data in the MSB byte, the
> > RSB_DATA register will keep the MSB byte unchanged when doing
> > the following 8-bit read. sunxi_rsb_read() will then return
> > a result that contains high byte from 16-bit read mixed with
> > the 8-bit result.
> >
> > The consequence is that after this happens the PMIC's regmap will
> > look like this: (0x33 is the high byte from the 16-bit read)
> >
> > % cat /sys/kernel/debug/regmap/sunxi-rsb-3a3/registers
> > 00: 33
> > 01: 33
> > 02: 33
> > 03: 33
> > 04: 33
> > 05: 33
> > 06: 33
> > 07: 33
> > 08: 33
> > 09: 33
> > 0a: 33
> > 0b: 33
> > 0c: 33
> > 0d: 33
> > 0e: 33
> > [snip]
> >
> > Fix this by masking the result of the read with the correct mask
> > based on the size of the read. There are no 16-bit users in the
> > mainline kernel, so this doesn't need to get into the stable tree.
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> 
> Acked-by: Chen-Yu Tsai <wens@csie.org>
> 
> for the fix, however it's not entirely clear to me how the MSB 0x33
> value got into the regmap. Looks like I expected the regmap core to
> handle any overflows, or didn't expect the lingering MSB from 16-bit
> reads, as I didn't have any 16-bit register devices back when I wrote
> this.

Now I feel like I masked some other bug. Though explanation may be quite simple.

For example when AXP core does regmap_read on some values for showing charging
current or battery voltage, because regmap_read works with unsigned int, it
will simply get a number that's too big. And that was the major symptom
I observed. I got readings from sysfs that my tablet is consuming 600A or 200A,
etc. And this value was jumping around based on AC100 activity (as the MSB
byte got changed).

In other places where the driver does regmap_update_bits, I think nothing bad
happened. The write after the read would simply discard the MSB byte.

And for the debugfs/regmap/*/registers, those are printed such:

https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regmap-debugfs.c#L256

	snprintf(buf + buf_pos, count - buf_pos,
		"%.*x", map->debugfs_val_len, val);
	
This doesn't truncate values, so the larger number gets printed to (for example):

        33fe

But regmap debugfs code truncates this by cutting off the formatted string to
this length:

  https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regmap-debugfs.c#L189

So in the end, only:

        00: 33

remains, instead of the correct value of:

        00: fe

So in the case of debugfs this is just a cosmetic/formatting issue, that didn't
affect anything else.

I think regmap_bus->reg_read API simply expects the returned value to not exceed
the sensible range.

regards,
	o.


> ChenYu
> 
> 
> > ---
> >  drivers/bus/sunxi-rsb.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> > index b8043b58568ac..8ab6a3865f569 100644
> > --- a/drivers/bus/sunxi-rsb.c
> > +++ b/drivers/bus/sunxi-rsb.c
> > @@ -316,6 +316,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
> >  {
> >         u32 cmd;
> >         int ret;
> > +       u32 mask;
> >
> >         if (!buf)
> >                 return -EINVAL;
> > @@ -323,12 +324,15 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
> >         switch (len) {
> >         case 1:
> >                 cmd = RSB_CMD_RD8;
> > +               mask = 0xffu;
> >                 break;
> >         case 2:
> >                 cmd = RSB_CMD_RD16;
> > +               mask = 0xffffu;
> >                 break;
> >         case 4:
> >                 cmd = RSB_CMD_RD32;
> > +               mask = 0xffffffffu;
> >                 break;
> >         default:
> >                 dev_err(rsb->dev, "Invalid access width: %zd\n", len);
> > @@ -345,7 +349,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
> >         if (ret)
> >                 goto unlock;
> >
> > -       *buf = readl(rsb->regs + RSB_DATA);
> > +       *buf = readl(rsb->regs + RSB_DATA) & mask;
> >
> >  unlock:
> >         mutex_unlock(&rsb->lock);
> > --
> > 2.25.1
> >

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

* Re: [PATCH] bus: sunxi-rsb: Return correct data when mixing 16-bit and 8-bit reads
  2020-02-19  1:09 [PATCH] bus: sunxi-rsb: Return correct data when mixing 16-bit and 8-bit reads Ondrej Jirman
  2020-02-19  2:49 ` Chen-Yu Tsai
@ 2020-02-20 17:32 ` Maxime Ripard
  2020-02-20 17:43   ` Ondřej Jirman
  1 sibling, 1 reply; 6+ messages in thread
From: Maxime Ripard @ 2020-02-20 17:32 UTC (permalink / raw)
  To: Ondrej Jirman
  Cc: linux-sunxi, Chen-Yu Tsai, Samuel Holland, Stephen Boyd,
	moderated list:ARM/Allwinner sunXi SoC support, open list

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

On Wed, Feb 19, 2020 at 02:09:50AM +0100, Ondrej Jirman wrote:
> When doing a 16-bit read that returns data in the MSB byte, the
> RSB_DATA register will keep the MSB byte unchanged when doing
> the following 8-bit read. sunxi_rsb_read() will then return
> a result that contains high byte from 16-bit read mixed with
> the 8-bit result.
>
> The consequence is that after this happens the PMIC's regmap will
> look like this: (0x33 is the high byte from the 16-bit read)
>
> % cat /sys/kernel/debug/regmap/sunxi-rsb-3a3/registers
> 00: 33
> 01: 33
> 02: 33
> 03: 33
> 04: 33
> 05: 33
> 06: 33
> 07: 33
> 08: 33
> 09: 33
> 0a: 33
> 0b: 33
> 0c: 33
> 0d: 33
> 0e: 33
> [snip]
>
> Fix this by masking the result of the read with the correct mask
> based on the size of the read. There are no 16-bit users in the
> mainline kernel, so this doesn't need to get into the stable tree.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> ---
>  drivers/bus/sunxi-rsb.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> index b8043b58568ac..8ab6a3865f569 100644
> --- a/drivers/bus/sunxi-rsb.c
> +++ b/drivers/bus/sunxi-rsb.c
> @@ -316,6 +316,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
>  {
>  	u32 cmd;
>  	int ret;
> +	u32 mask;
>
>  	if (!buf)
>  		return -EINVAL;
> @@ -323,12 +324,15 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
>  	switch (len) {
>  	case 1:
>  		cmd = RSB_CMD_RD8;
> +		mask = 0xffu;
>  		break;
>  	case 2:
>  		cmd = RSB_CMD_RD16;
> +		mask = 0xffffu;
>  		break;
>  	case 4:
>  		cmd = RSB_CMD_RD32;
> +		mask = 0xffffffffu;
>  		break;
>  	default:
>  		dev_err(rsb->dev, "Invalid access width: %zd\n", len);
> @@ -345,7 +349,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
>  	if (ret)
>  		goto unlock;
>
> -	*buf = readl(rsb->regs + RSB_DATA);
> +	*buf = readl(rsb->regs + RSB_DATA) & mask;

Thanks for debugging this and the extensive commit log.

I guess it would be cleaner to just use GENMASK(len * 8, 0) here?

Maxime

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

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

* Re: [PATCH] bus: sunxi-rsb: Return correct data when mixing 16-bit and 8-bit reads
  2020-02-20 17:32 ` Maxime Ripard
@ 2020-02-20 17:43   ` Ondřej Jirman
  0 siblings, 0 replies; 6+ messages in thread
From: Ondřej Jirman @ 2020-02-20 17:43 UTC (permalink / raw)
  To: Maxime Ripard
  Cc: linux-sunxi, Chen-Yu Tsai, Samuel Holland, Stephen Boyd,
	moderated list:ARM/Allwinner sunXi SoC support, open list

Hi,

On Thu, Feb 20, 2020 at 06:32:13PM +0100, Maxime Ripard wrote:
> On Wed, Feb 19, 2020 at 02:09:50AM +0100, Ondrej Jirman wrote:
> > When doing a 16-bit read that returns data in the MSB byte, the
> > RSB_DATA register will keep the MSB byte unchanged when doing
> > the following 8-bit read. sunxi_rsb_read() will then return
> > a result that contains high byte from 16-bit read mixed with
> > the 8-bit result.
> >
> > The consequence is that after this happens the PMIC's regmap will
> > look like this: (0x33 is the high byte from the 16-bit read)
> >
> > % cat /sys/kernel/debug/regmap/sunxi-rsb-3a3/registers
> > 00: 33
> > 01: 33
> > 02: 33
> > 03: 33
> > 04: 33
> > 05: 33
> > 06: 33
> > 07: 33
> > 08: 33
> > 09: 33
> > 0a: 33
> > 0b: 33
> > 0c: 33
> > 0d: 33
> > 0e: 33
> > [snip]
> >
> > Fix this by masking the result of the read with the correct mask
> > based on the size of the read. There are no 16-bit users in the
> > mainline kernel, so this doesn't need to get into the stable tree.
> >
> > Signed-off-by: Ondrej Jirman <megous@megous.com>
> > ---
> >  drivers/bus/sunxi-rsb.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> > index b8043b58568ac..8ab6a3865f569 100644
> > --- a/drivers/bus/sunxi-rsb.c
> > +++ b/drivers/bus/sunxi-rsb.c
> > @@ -316,6 +316,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
> >  {
> >  	u32 cmd;
> >  	int ret;
> > +	u32 mask;
> >
> >  	if (!buf)
> >  		return -EINVAL;
> > @@ -323,12 +324,15 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
> >  	switch (len) {
> >  	case 1:
> >  		cmd = RSB_CMD_RD8;
> > +		mask = 0xffu;
> >  		break;
> >  	case 2:
> >  		cmd = RSB_CMD_RD16;
> > +		mask = 0xffffu;
> >  		break;
> >  	case 4:
> >  		cmd = RSB_CMD_RD32;
> > +		mask = 0xffffffffu;
> >  		break;
> >  	default:
> >  		dev_err(rsb->dev, "Invalid access width: %zd\n", len);
> > @@ -345,7 +349,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
> >  	if (ret)
> >  		goto unlock;
> >
> > -	*buf = readl(rsb->regs + RSB_DATA);
> > +	*buf = readl(rsb->regs + RSB_DATA) & mask;
> 
> Thanks for debugging this and the extensive commit log.
> 
> I guess it would be cleaner to just use GENMASK(len * 8, 0) here?

GENMASK most probably fails with value (32,0) I think.

#define GENMASK(h, l) \
	(((~UL(0)) - (UL(1) << (l)) + 1) & \
	 (~UL(0) >> (BITS_PER_LONG - 1 - (h))))

would give ~0 >> -1

regards,
	o.

> Maxime



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

* Re: [linux-sunxi] Re: [PATCH] bus: sunxi-rsb: Return correct data when mixing 16-bit and 8-bit reads
  2020-02-19 12:14   ` Ondřej Jirman
@ 2020-02-21  3:04     ` Chen-Yu Tsai
  0 siblings, 0 replies; 6+ messages in thread
From: Chen-Yu Tsai @ 2020-02-21  3:04 UTC (permalink / raw)
  To: Ondřej Jirman, Chen-Yu Tsai, linux-sunxi, Maxime Ripard,
	Samuel Holland, Stephen Boyd,
	moderated list:ARM/Allwinner sunXi SoC support, open list

On Wed, Feb 19, 2020 at 8:14 PM Ondřej Jirman <megous@megous.com> wrote:
>
> On Wed, Feb 19, 2020 at 10:49:18AM +0800, Chen-Yu Tsai wrote:
> > On Wed, Feb 19, 2020 at 9:10 AM Ondrej Jirman <megous@megous.com> wrote:
> > >
> > > When doing a 16-bit read that returns data in the MSB byte, the
> > > RSB_DATA register will keep the MSB byte unchanged when doing
> > > the following 8-bit read. sunxi_rsb_read() will then return
> > > a result that contains high byte from 16-bit read mixed with
> > > the 8-bit result.
> > >
> > > The consequence is that after this happens the PMIC's regmap will
> > > look like this: (0x33 is the high byte from the 16-bit read)
> > >
> > > % cat /sys/kernel/debug/regmap/sunxi-rsb-3a3/registers
> > > 00: 33
> > > 01: 33
> > > 02: 33
> > > 03: 33
> > > 04: 33
> > > 05: 33
> > > 06: 33
> > > 07: 33
> > > 08: 33
> > > 09: 33
> > > 0a: 33
> > > 0b: 33
> > > 0c: 33
> > > 0d: 33
> > > 0e: 33
> > > [snip]
> > >
> > > Fix this by masking the result of the read with the correct mask
> > > based on the size of the read. There are no 16-bit users in the
> > > mainline kernel, so this doesn't need to get into the stable tree.
> > >
> > > Signed-off-by: Ondrej Jirman <megous@megous.com>
> >
> > Acked-by: Chen-Yu Tsai <wens@csie.org>
> >
> > for the fix, however it's not entirely clear to me how the MSB 0x33
> > value got into the regmap. Looks like I expected the regmap core to
> > handle any overflows, or didn't expect the lingering MSB from 16-bit
> > reads, as I didn't have any 16-bit register devices back when I wrote
> > this.
>
> Now I feel like I masked some other bug. Though explanation may be quite simple.
>
> For example when AXP core does regmap_read on some values for showing charging
> current or battery voltage, because regmap_read works with unsigned int, it
> will simply get a number that's too big. And that was the major symptom
> I observed. I got readings from sysfs that my tablet is consuming 600A or 200A,
> etc. And this value was jumping around based on AC100 activity (as the MSB
> byte got changed).
>
> In other places where the driver does regmap_update_bits, I think nothing bad
> happened. The write after the read would simply discard the MSB byte.
>
> And for the debugfs/regmap/*/registers, those are printed such:
>
> https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regmap-debugfs.c#L256
>
>         snprintf(buf + buf_pos, count - buf_pos,
>                 "%.*x", map->debugfs_val_len, val);
>
> This doesn't truncate values, so the larger number gets printed to (for example):
>
>         33fe
>
> But regmap debugfs code truncates this by cutting off the formatted string to
> this length:
>
>   https://elixir.bootlin.com/linux/latest/source/drivers/base/regmap/regmap-debugfs.c#L189
>
> So in the end, only:
>
>         00: 33
>
> remains, instead of the correct value of:
>
>         00: fe
>
> So in the case of debugfs this is just a cosmetic/formatting issue, that didn't
> affect anything else.
>
> I think regmap_bus->reg_read API simply expects the returned value to not exceed
> the sensible range.

Sounds good. Thanks for digging through this. My ack still stands.

ChenYu

> regards,
>         o.
>
>
> > ChenYu
> >
> >
> > > ---
> > >  drivers/bus/sunxi-rsb.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/bus/sunxi-rsb.c b/drivers/bus/sunxi-rsb.c
> > > index b8043b58568ac..8ab6a3865f569 100644
> > > --- a/drivers/bus/sunxi-rsb.c
> > > +++ b/drivers/bus/sunxi-rsb.c
> > > @@ -316,6 +316,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
> > >  {
> > >         u32 cmd;
> > >         int ret;
> > > +       u32 mask;
> > >
> > >         if (!buf)
> > >                 return -EINVAL;
> > > @@ -323,12 +324,15 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
> > >         switch (len) {
> > >         case 1:
> > >                 cmd = RSB_CMD_RD8;
> > > +               mask = 0xffu;
> > >                 break;
> > >         case 2:
> > >                 cmd = RSB_CMD_RD16;
> > > +               mask = 0xffffu;
> > >                 break;
> > >         case 4:
> > >                 cmd = RSB_CMD_RD32;
> > > +               mask = 0xffffffffu;
> > >                 break;
> > >         default:
> > >                 dev_err(rsb->dev, "Invalid access width: %zd\n", len);
> > > @@ -345,7 +349,7 @@ static int sunxi_rsb_read(struct sunxi_rsb *rsb, u8 rtaddr, u8 addr,
> > >         if (ret)
> > >                 goto unlock;
> > >
> > > -       *buf = readl(rsb->regs + RSB_DATA);
> > > +       *buf = readl(rsb->regs + RSB_DATA) & mask;
> > >
> > >  unlock:
> > >         mutex_unlock(&rsb->lock);
> > > --
> > > 2.25.1
> > >
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> To view this discussion on the web, visit https://groups.google.com/d/msgid/linux-sunxi/20200219121424.dfvrwfcavupmwbvw%40core.my.home.

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

end of thread, other threads:[~2020-02-21  3:04 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-19  1:09 [PATCH] bus: sunxi-rsb: Return correct data when mixing 16-bit and 8-bit reads Ondrej Jirman
2020-02-19  2:49 ` Chen-Yu Tsai
2020-02-19 12:14   ` Ondřej Jirman
2020-02-21  3:04     ` [linux-sunxi] " Chen-Yu Tsai
2020-02-20 17:32 ` Maxime Ripard
2020-02-20 17:43   ` Ondřej Jirman

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).