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
