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

Lad, Prabhakar
2024-03-08T21:00:43Z

>On Fri, Mar 8, 2024 at 6:15 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
>
>
> > Subject: Re: [PATCH 4/5] i2c: riic: Pass register offsets and chip details as OF data
> >
> > Hi Biju,
> >
> > On Fri, Mar 8, 2024 at 5:36 PM Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > >
> > > 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?
> > >
> > Yes this is added to future proof it, as the RZ/V2H SoC has some bit differences in the slave address
> > register as compared to RZ/A and RZ/G2L. Comparing the family outside probe is not always preferred
> > hence this is added as part of OF data.
>
> Ok. Got it.
> But the family variable is unused and is wasting memory.
> Simple reg layout is enough for device data??
>
Ok, I'll drop it for the current implementation.

Cheers,
Prabhakar