Received-SPF: pass (google.com: domain of linux-kernel+bounces-97434-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240308172726.225357-1-prabhakar.mahadev-lad.rj@bp.renesas.com>
 <20240308172726.225357-5-prabhakar.mahadev-lad.rj@bp.renesas.com> <OSAPR01MB1587CFE774834B31820E7D5486272@OSAPR01MB1587.jpnprd01.prod.outlook.com>
In-Reply-To: <OSAPR01MB1587CFE774834B31820E7D5486272@OSAPR01MB1587.jpnprd01.prod.outlook.com>
From: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Date: Fri, 8 Mar 2024 18:03:39 +0000
Message-ID: <CA+V-a8uJO_qz6CaD2zUXC1Kf7tuM+Nzq6W+WHjy09NN2FAn+fA@mail.gmail.com>
Subject: Re: [PATCH 4/5] i2c: riic: Pass register offsets and chip details as
 OF data
To: Biju Das <biju.das.jz@bp.renesas.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>, Chris Brandt <Chris.Brandt@renesas.com>, 
	Andi Shyti <andi.shyti@kernel.org>, Rob Herring <robh+dt@kernel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Conor Dooley <conor+dt@kernel.org>, 
	Magnus Damm <magnus.damm@gmail.com>, Wolfram Sang <wsa+renesas@sang-engineering.com>, 
	"linux-renesas-soc@vger.kernel.org" <linux-renesas-soc@vger.kernel.org>, 
	"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>, 
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>, 
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, 
	Fabrizio Castro <fabrizio.castro.jz@renesas.com>, 
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Biju,

On Fri, Mar 8, 2024 at 5:36=E2=80=AFPM 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 app=
roach simplifies the extension
> > of the driver for other SoCs.
> >
> > This patch lays the groundwork for adding support for the Renesas RZ/V2=
H 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-rii=
c.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.

Cheers,
Prabhakar

> > +     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 =3D 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 o=
ffset)  {
> > -     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 =3D of_device_get_match_data(&pdev->dev);
> > +
> >       adap =3D &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 =3D {
> > +     .family =3D RIIC_RZ_A_TYPE,
> > +     .regs =3D {
> > +             [RIIC_ICCR1] =3D 0x00,
> > +             [RIIC_ICCR2] =3D 0x04,
> > +             [RIIC_ICMR1] =3D 0x08,
> > +             [RIIC_ICMR3] =3D 0x10,
> > +             [RIIC_ICSER] =3D 0x18,
> > +             [RIIC_ICIER] =3D 0x1c,
> > +             [RIIC_ICSR2] =3D 0x24,
> > +             [RIIC_ICBRL] =3D 0x34,
> > +             [RIIC_ICBRH] =3D 0x38,
> > +             [RIIC_ICDRT] =3D 0x3c,
> > +             [RIIC_ICDRR] =3D 0x40,
> > +     },
> > +};
> > +
> >  static const struct of_device_id riic_i2c_dt_ids[] =3D {
> > -     { .compatible =3D "renesas,riic-rz", },
> > +     { .compatible =3D "renesas,riic-rz", .data =3D &riic_rz_a_info },
> >       { /* Sentinel */ },
> >  };
> >
> > --
> > 2.34.1
> >
>