Re: [PATCH 3/5] i2c: riic: Introduce helper functions for I2C read/write operations

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Prabhakar,

On Fri, Mar 8, 2024 at 6:28 PM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Introduce helper functions for performing I2C read and write operations
> in the RIIC driver.
>
> These helper functions lay the groundwork for adding support for the
> RZ/V2H SoC. This is essential because the register offsets for the RZ/V2H
> SoC differ from those of the RZ/A SoC. By abstracting the read and write
> operations, we can seamlessly adapt the driver to support different SoC
> variants without extensive modifications.
>
> This patch is part of the preparation process for integrating support for
> the RZ/V2H SoC into the RIIC driver.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> Reviewed-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>

Thanks for your patch!

> --- a/drivers/i2c/busses/i2c-riic.c
> +++ b/drivers/i2c/busses/i2c-riic.c
> @@ -105,9 +105,19 @@ struct riic_irq_desc {
>         char *name;
>  };
>
> +static inline void riic_writeb_reg(u8 val, struct riic_dev *riic, u8 offset)

Having "riic" in the middle is definitely the wrong order of parameters ;-)
Please make "riic" the first parameter.

> +{
> +       writeb(val, riic->base + offset);
> +}
> +
> +static inline u8 riic_readb_reg(struct riic_dev *riic, u8 offset)
> +{
> +       return readb(riic->base + offset);
> +}


> -       writeb(0, riic->base + RIIC_ICSR2);
> +       riic_writeb_reg(0, riic, RIIC_ICSR2);

This clearly shows that the new accessors involve more typing work.
Why not just call them riic_writeb() and riic_readb()?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds





[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux