diff mbox series

[RFT,v2,4/5] i3c: mipi-i3c-hci: use get_parity8 helper instead of open coding it

Message ID 20241229101234.2896-5-wsa+renesas@sang-engineering.com (mailing list archive)
State New
Delegated to: Geert Uytterhoeven
Headers show
Series i3c: introduce and use generic parity helper | expand

Commit Message

Wolfram Sang Dec. 29, 2024, 10:12 a.m. UTC
The kernel has now a generic helper for getting parity with easier to
understand semantics. Make use of it.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Changes since v1:

* renamed from 'get_parity8' to 'parity8'
* rebased to 6.13-rc4

 drivers/i3c/master/mipi-i3c-hci/dat_v1.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

Comments

David Laight Jan. 1, 2025, 12:14 p.m. UTC | #1
On Sun, 29 Dec 2024 11:12:32 +0100
Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:

> The kernel has now a generic helper for getting parity with easier to
> understand semantics. Make use of it.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> Changes since v1:
> 
> * renamed from 'get_parity8' to 'parity8'
> * rebased to 6.13-rc4
> 
>  drivers/i3c/master/mipi-i3c-hci/dat_v1.c | 11 +----------
>  1 file changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
> index 47b9b4d4ed3f..85c4916972e4 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
> @@ -40,15 +40,6 @@
>  #define dat_w0_write(i, v)	writel(v, hci->DAT_regs + (i) * 8)
>  #define dat_w1_write(i, v)	writel(v, hci->DAT_regs + (i) * 8 + 4)
>  
> -static inline bool dynaddr_parity(unsigned int addr)
> -{
> -	addr |= 1 << 7;
> -	addr += addr >> 4;
> -	addr += addr >> 2;
> -	addr += addr >> 1;
> -	return (addr & 1);
> -}
> -
>  static int hci_dat_v1_init(struct i3c_hci *hci)
>  {
>  	unsigned int dat_idx;
> @@ -123,7 +114,7 @@ static void hci_dat_v1_set_dynamic_addr(struct i3c_hci *hci,
>  	dat_w0 = dat_w0_read(dat_idx);
>  	dat_w0 &= ~(DAT_0_DYNAMIC_ADDRESS | DAT_0_DYNADDR_PARITY);
>  	dat_w0 |= FIELD_PREP(DAT_0_DYNAMIC_ADDRESS, address) |
> -		  (dynaddr_parity(address) ? DAT_0_DYNADDR_PARITY : 0);
> +		  (parity8(address) ? 0 : DAT_0_DYNADDR_PARITY);

NAK - that isn't the same code at all.

	David

>  	dat_w0_write(dat_idx, dat_w0);
>  }
>
Wolfram Sang Jan. 2, 2025, 9:01 a.m. UTC | #2
> > @@ -123,7 +114,7 @@ static void hci_dat_v1_set_dynamic_addr(struct i3c_hci *hci,
> >  	dat_w0 = dat_w0_read(dat_idx);
> >  	dat_w0 &= ~(DAT_0_DYNAMIC_ADDRESS | DAT_0_DYNADDR_PARITY);
> >  	dat_w0 |= FIELD_PREP(DAT_0_DYNAMIC_ADDRESS, address) |
> > -		  (dynaddr_parity(address) ? DAT_0_DYNADDR_PARITY : 0);
> > +		  (parity8(address) ? 0 : DAT_0_DYNADDR_PARITY);
> 
> NAK - that isn't the same code at all.

But the same algorithm? Please elaborate where you think the new code
will fail compared to the old one. And frankly, are you aware of
different parity calculations? Have you read the link which was in the
kdocs of my new function?
diff mbox series

Patch

diff --git a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
index 47b9b4d4ed3f..85c4916972e4 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dat_v1.c
@@ -40,15 +40,6 @@ 
 #define dat_w0_write(i, v)	writel(v, hci->DAT_regs + (i) * 8)
 #define dat_w1_write(i, v)	writel(v, hci->DAT_regs + (i) * 8 + 4)
 
-static inline bool dynaddr_parity(unsigned int addr)
-{
-	addr |= 1 << 7;
-	addr += addr >> 4;
-	addr += addr >> 2;
-	addr += addr >> 1;
-	return (addr & 1);
-}
-
 static int hci_dat_v1_init(struct i3c_hci *hci)
 {
 	unsigned int dat_idx;
@@ -123,7 +114,7 @@  static void hci_dat_v1_set_dynamic_addr(struct i3c_hci *hci,
 	dat_w0 = dat_w0_read(dat_idx);
 	dat_w0 &= ~(DAT_0_DYNAMIC_ADDRESS | DAT_0_DYNADDR_PARITY);
 	dat_w0 |= FIELD_PREP(DAT_0_DYNAMIC_ADDRESS, address) |
-		  (dynaddr_parity(address) ? DAT_0_DYNADDR_PARITY : 0);
+		  (parity8(address) ? 0 : DAT_0_DYNADDR_PARITY);
 	dat_w0_write(dat_idx, dat_w0);
 }