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: RZ/{G2L, G2LC, G2UL, V2L, FIVE} -> riic_rz_g2_info RZ/G3S and RZ/V2H -> riic_rz_v2h_info Everything else: riic_rz_a_info I don't have anything at hand to test the "everything else" thus I enabled it for RZ/{G2L, G2LC, G2UL, V2L, FIVE}, RZ/G3S and RZ/V2H. > > Then except RZ/A1, set FMP. I cannot test all these. > > Currently register layout of RZ/A2 is not matching with > Hardware manual. I cannot test this either. Also, I think this is not the purpose of this series. Thank you, Claudiu Beznea > > Cheers, > Biju