RE: [PATCH 4/5] i2c: riic: Pass register offsets and chip details as OF data

Biju Das
2024-03-08T17:36:43Z

Hi Prabhakar,

Thanks for the patch.

> -----Original Message-----
> From: Prabhakar <prabhakar.csengg@gmail.com>
> Sent: Friday, March 8, 2024 5:27 PM
> Subject: [PATCH 4/5] i2c: riic: Pass register offsets and chip details as OF data
>
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> With an increasing number of SoCs reusing this driver, each with slight variations in the RIIC IP, it
> becomes necessary to support passing these details as OF data. This approach simplifies the extension
> of the driver for other SoCs.
>
> This patch lays the groundwork for adding support for the Renesas RZ/V2H SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> Reviewed-by: Fabrizio Castro <fabrizio.castro.jz@renesas.com>
> ---
> drivers/i2c/busses/i2c-riic.c | 59 ++++++++++++++++++++++++++---------
> 1 file changed, 44 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c index
> 49a12f1ecdf9..3398d639b754 100644
> --- a/drivers/i2c/busses/i2c-riic.c
> +++ b/drivers/i2c/busses/i2c-riic.c
> @@ -46,18 +46,6 @@
> #include <linux/pm_runtime.h>
> #include <linux/reset.h>
>
> -#define RIIC_ICCR1 0x00
> -#define RIIC_ICCR2 0x04
> -#define RIIC_ICMR1 0x08
> -#define RIIC_ICMR3 0x10
> -#define RIIC_ICSER 0x18
> -#define RIIC_ICIER 0x1c
> -#define RIIC_ICSR2 0x24
> -#define RIIC_ICBRL 0x34
> -#define RIIC_ICBRH 0x38
> -#define RIIC_ICDRT 0x3c
> -#define RIIC_ICDRR 0x40
> -
> #define ICCR1_ICE 0x80
> #define ICCR1_IICRST 0x40
> #define ICCR1_SOWP 0x10
> @@ -87,6 +75,13 @@
>
> #define RIIC_INIT_MSG -1
>
> +#define RIIC_RZ_A_TYPE 0

> +
> +struct riic_of_data {
> + u8 family;

Do you need family as compatible have this information?

> + u8 regs[];
> +};
> +
> struct riic_dev {
> void __iomem *base;
> u8 *buf;
> @@ -94,6 +89,7 @@ struct riic_dev {
> int bytes_left;
> int err;
> int is_last;
> + const struct riic_of_data *info;
> struct completion msg_done;
> struct i2c_adapter adapter;
> struct clk *clk;
> @@ -105,14 +101,28 @@ struct riic_irq_desc {
> char *name;
> };
>
> +enum riic_reg_list {
> + RIIC_ICCR1 = 0,
> + RIIC_ICCR2,
> + RIIC_ICMR1,
> + RIIC_ICMR3,
> + RIIC_ICSER,
> + RIIC_ICIER,
> + RIIC_ICSR2,
> + RIIC_ICBRL,
> + RIIC_ICBRH,
> + RIIC_ICDRT,
> + RIIC_ICDRR,
> +};
> +
> static inline void riic_writeb_reg(u8 val, struct riic_dev *riic, u8 offset) {
> - writeb(val, riic->base + offset);
> + writeb(val, riic->base + riic->info->regs[offset]);
> }
>
> static inline u8 riic_readb_reg(struct riic_dev *riic, u8 offset) {
> - return readb(riic->base + offset);
> + return readb(riic->base + riic->info->regs[offset]);
> }
>
> static inline void riic_clear_set_bit(struct riic_dev *riic, u8 clear, u8 set, u8 reg) @@ -453,6
> +463,8 @@ static int riic_i2c_probe(struct platform_device *pdev)
> }
> }
>
> + riic->info = of_device_get_match_data(&pdev->dev);
> +
> adap = &riic->adapter;
> i2c_set_adapdata(adap, riic);
> strscpy(adap->name, "Renesas RIIC adapter", sizeof(adap->name)); @@ -497,8 +509,25 @@ static void
> riic_i2c_remove(struct platform_device *pdev)
> pm_runtime_disable(&pdev->dev);
> }
>
> +static const struct riic_of_data riic_rz_a_info = {
> + .family = RIIC_RZ_A_TYPE,
> + .regs = {
> + [RIIC_ICCR1] = 0x00,
> + [RIIC_ICCR2] = 0x04,
> + [RIIC_ICMR1] = 0x08,
> + [RIIC_ICMR3] = 0x10,
> + [RIIC_ICSER] = 0x18,
> + [RIIC_ICIER] = 0x1c,
> + [RIIC_ICSR2] = 0x24,
> + [RIIC_ICBRL] = 0x34,
> + [RIIC_ICBRH] = 0x38,
> + [RIIC_ICDRT] = 0x3c,
> + [RIIC_ICDRR] = 0x40,
> + },
> +};
> +
> static const struct of_device_id riic_i2c_dt_ids[] = {
> - { .compatible = "renesas,riic-rz", },
> + { .compatible = "renesas,riic-rz", .data = &riic_rz_a_info },
> { /* Sentinel */ },
> };
>
> --
> 2.34.1
>