On 28.06.2024 11:02, claudiu beznea wrote: > > > 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? That's Biju's previous statement. Sorry for not formatting it properly. > > On the manual I've downloaded from Renesas web site the FMPE bit of > RIICnFER is not available on RZ/A1H. > > Thank you, > Claudiu Beznea > >> If it does not, then it make sense to keep the patch as it is. >> >> Cheers, >> Biju >> >>> >>> Keeping struct riic_of_data is necessary (unless I misunderstood your proposal). >>> >>> Thank you, >>> Claudiu Beznea >>> >>>> >>>> ie, replace const struct riic_of_data *info->const u8 *regs in struct >>>> riic_dev and use .data = riic_rz_xx_regs in of_match_table? >>>> >>>> Cheers, >>>> Biju >>>>> }; >>>>> >>>>> struct riic_dev { >>>>> @@ -531,36 +531,40 @@ static void riic_i2c_remove(struct platform_device *pdev) >>>>> pm_runtime_dont_use_autosuspend(dev); >>>>> } >>>>> >>>>> +static const u8 riic_rz_a_regs[RIIC_REG_END] = { >>>>> + [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 riic_of_data riic_rz_a_info = { >>>>> - .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, >>>>> - }, >>>>> + .regs = riic_rz_a_regs, >>>>> +}; >>>>> + >>>>> +static const u8 riic_rz_v2h_regs[RIIC_REG_END] = { >>>>> + [RIIC_ICCR1] = 0x00, >>>>> + [RIIC_ICCR2] = 0x01, >>>>> + [RIIC_ICMR1] = 0x02, >>>>> + [RIIC_ICMR3] = 0x04, >>>>> + [RIIC_ICSER] = 0x06, >>>>> + [RIIC_ICIER] = 0x07, >>>>> + [RIIC_ICSR2] = 0x09, >>>>> + [RIIC_ICBRL] = 0x10, >>>>> + [RIIC_ICBRH] = 0x11, >>>>> + [RIIC_ICDRT] = 0x12, >>>>> + [RIIC_ICDRR] = 0x13, >>>>> }; >>>>> >>>>> static const struct riic_of_data riic_rz_v2h_info = { >>>>> - .regs = { >>>>> - [RIIC_ICCR1] = 0x00, >>>>> - [RIIC_ICCR2] = 0x01, >>>>> - [RIIC_ICMR1] = 0x02, >>>>> - [RIIC_ICMR3] = 0x04, >>>>> - [RIIC_ICSER] = 0x06, >>>>> - [RIIC_ICIER] = 0x07, >>>>> - [RIIC_ICSR2] = 0x09, >>>>> - [RIIC_ICBRL] = 0x10, >>>>> - [RIIC_ICBRH] = 0x11, >>>>> - [RIIC_ICDRT] = 0x12, >>>>> - [RIIC_ICDRR] = 0x13, >>>>> - }, >>>>> + .regs = riic_rz_v2h_regs, >>>>> }; >>>>> >>>>> static int riic_i2c_suspend(struct device *dev) >>>>> -- >>>>> 2.39.2 >>>>> >>>>