> Subject: RE: [PATCH v2 07/12] i2c: riic: Define individual arrays to describe the register offsets > > Hi Claudiu, > > > -----Original Message----- > > From: claudiu beznea <claudiu.beznea@xxxxxxxxx> > > Sent: Friday, June 28, 2024 12:25 PM > > Subject: Re: [PATCH v2 07/12] i2c: riic: Define individual arrays to > > describe the register offsets > > > > > > > > On 28.06.2024 13:49, Biju Das wrote: > > > Hi Claudiu, > > > > > >> -----Original Message----- > > >> From: claudiu beznea <claudiu.beznea@xxxxxxxxx> > > >> Sent: Friday, June 28, 2024 11:25 AM > > >> Subject: Re: [PATCH v2 07/12] i2c: riic: Define individual arrays > > >> to describe the register offsets > > >> > > >> > > >> > > >> On 28.06.2024 11:24, Biju Das wrote: > > >>> Hi Claudiu, > > >>> > > >>>> -----Original Message----- > > >>>> From: claudiu beznea <claudiu.beznea@xxxxxxxxx> > > >>>> Sent: Friday, June 28, 2024 9:13 AM > > >>>> Subject: Re: [PATCH v2 07/12] i2c: riic: Define individual arrays > > >>>> to describe the register offsets > > >>>> > > >>>> > > >>>> > > >>>> On 28.06.2024 11:09, Biju Das wrote: > > >>>>> > > >>>>> Hi Claudiu, > > >>>>> > > >>>>>> -----Original Message----- > > >>>>>> From: claudiu beznea <claudiu.beznea@xxxxxxxxx> > > >>>>>> Sent: Friday, June 28, 2024 9:03 AM > > >>>>>> Subject: Re: [PATCH v2 07/12] i2c: riic: Define individual > > >>>>>> arrays to describe the register offsets > > >>>>>> > > >>>>>> > > >>>>>> > > >>>>>> On 28.06.2024 10:55, Biju Das wrote: > > >>>>>>> Hi Claudiu, > > >>>>>>> > > >>>>>>>> -----Original Message----- > > >>>>>>>> From: claudiu beznea <claudiu.beznea@xxxxxxxxx> > > >>>>>>>> Sent: Friday, June 28, 2024 8:32 AM > > >>>>>>>> Subject: Re: [PATCH v2 07/12] i2c: riic: Define individual > > >>>>>>>> arrays to describe the register offsets > > >>>>>>>> > > >>>>>>>> Hi, Biju, > > >>>>>>>> > > >>>>>>>> On 28.06.2024 08:59, Biju Das wrote: > > >>>>>>>>> Hi Claudiu, > > >>>>>>>>> > > >>>>>>>>>> -----Original Message----- > > >>>>>>>>>> From: Claudiu <claudiu.beznea@xxxxxxxxx> > > >>>>>>>>>> Sent: Tuesday, June 25, 2024 1:14 PM > > >>>>>>>>>> Subject: [PATCH v2 07/12] i2c: riic: Define individual > > >>>>>>>>>> arrays to describe the register offsets > > >>>>>>>>>> > > >>>>>>>>>> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > > >>>>>>>>>> > > >>>>>>>>>> Define individual arrays to describe the register offsets. > > >>>>>>>>>> In this way we can describe different IP variants that > > >>>>>>>>>> share the same register offsets but have differences in other characteristics. > > >>>>>>>>>> Commit prepares for the addition > > >>>>>>>> of fast mode plus. > > >>>>>>>>>> > > >>>>>>>>>> Signed-off-by: Claudiu Beznea > > >>>>>>>>>> <claudiu.beznea.uj@xxxxxxxxxxxxxx> > > >>>>>>>>>> --- > > >>>>>>>>>> > > >>>>>>>>>> Changes in v2: > > >>>>>>>>>> - none > > >>>>>>>>>> > > >>>>>>>>>> drivers/i2c/busses/i2c-riic.c | 58 > > >>>>>>>>>> +++++++++++++++++++---------------- > > >>>>>>>>>> 1 file changed, 31 insertions(+), 27 deletions(-) > > >>>>>>>>>> > > >>>>>>>>>> diff --git a/drivers/i2c/busses/i2c-riic.c > > >>>>>>>>>> b/drivers/i2c/busses/i2c-riic.c index > > >>>>>>>>>> 9fe007609076..8ffbead95492 100644 > > >>>>>>>>>> --- a/drivers/i2c/busses/i2c-riic.c > > >>>>>>>>>> +++ b/drivers/i2c/busses/i2c-riic.c > > >>>>>>>>>> @@ -91,7 +91,7 @@ enum riic_reg_list { }; > > >>>>>>>>>> > > >>>>>>>>>> struct riic_of_data { > > >>>>>>>>>> - u8 regs[RIIC_REG_END]; > > >>>>>>>>>> + const u8 *regs; > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>>>> Since you are touching this part, can we drop struct and Use > > >>>>>>>>> u8* as device_data instead? > > >>>>>>>> > > >>>>>>>> Patch 09/12 "i2c: riic: Add support for fast mode plus" adds > > >>>>>>>> a new member to struct > > >>>>>> riic_of_data. > > >>>>>>>> That new member is needed to differentiate b/w hardware > > >>>>>>>> versions supporting fast mode plus based on compatible. > > >>>>>>> > > >>>>>>> Are we sure RZ/A does not support fast mode plus? > > >>>>>> > > >>>>>> From commit description of patch 09/12: > > >>>>>> > > >>>>>> Fast mode plus is available on most of the IP variants that > > >>>>>> RIIC driver is working with. The exception is (according to HW > > >>>>>> manuals of the SoCs where this IP is > > >>>> available) the Renesas RZ/A1H. > > >>>>>> For this, patch introduces the struct riic_of_data::fast_mode_plus. > > >>>>>> > > >>>>>> I checked the manuals of all the SoCs where this driver is used. > > >>>>>> > > >>>>>> I haven't checked the H/W manual? > > >>>>>> > > >>>>>> On the manual I've downloaded from Renesas web site the FMPE > > >>>>>> bit of RIICnFER is not available on RZ/A1H. > > >>>>> > > >>>>> I just found RZ/A2M manual, it supports FMP and register layout looks similar to RZ/G2L. > > >>>> > > >>>> I introduced struct riic_of_data::fast_mode_plus because of RZ/A1H. > > >>> > > >>> Maybe make the register layout as per SoC > > >>> > > >>> RZ/A1 --> &riic_rz_a_info > > >>> RZ/A2 and RZ/{G2L,G2LC,V2L,G2UL,FIVE} --> &riic_rz_g2_info RZ/G3S > > >>> and RZ/V2H --> &riic_rz_v2h_info > > >> > > >> Sorry, but I don't understand. Patch 09/12 already does that but a > > >> bit > > >> differently: > > > > > > Now register layout is added to differentiate the SoCs for adding > > > support to RZ/G3S and this layout should match with the hardware manual for all supported SoCs. > > > Currently it is wrong for RZ/A2 SoC, while you fixed it for all other SoCs. > > > > I checked RZ/A2M. There is nothing broken. The only thing that I see > > is that the FP+ is not enabled on RZ/A2M (please let me know if there > > is anything else I missed). I don't see this broken. It is the same behavior that was before this > patch. > > As per RZ/A2M hardware manual, ICFER register is present > > While as per [1], you don't have this register. So according to me RZ/A2 SoC register layout is > broken and it is same as RZ/A1. > Oops, ICFER register is present in RZ/A1 as well. Currently same compatible used for RZ/A1 and RZ/A2 that needs to be updated in patch#9 as the later has FMP capability like RZ/G2L. Cheers, Biju