Hi Claudiu,
Thanks for the patch.
> -----Original Message-----
> From: Claudiu <claudiu.beznea@xxxxxxxxx>
> Sent: Tuesday, June 25, 2024 1:14 PM
> Subject: [PATCH v2 09/12] i2c: riic: Add support for fast mode plus
>
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> 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.
>
> Fast mode plus was tested on RZ/G3S, RZ/G2{L,UL,LC}, RZ/Five by instantiating the RIIC frequency to
> 1MHz and issuing i2c reads on the fast mode plus capable devices (and the i2c clock frequency was
> checked on RZ/G3S).
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> ---
>
> Changes in v2:
> - dropped code that handles the renesas,riic-no-fast-mode-plus
> - updated commit description
>
> drivers/i2c/busses/i2c-riic.c | 28 ++++++++++++++++++++++++----
> 1 file changed, 24 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c index
> 8ffbead95492..c07317f95e82 100644
> --- a/drivers/i2c/busses/i2c-riic.c
> +++ b/drivers/i2c/busses/i2c-riic.c
> @@ -63,6 +63,8 @@
> #define ICMR3_ACKWP 0x10
> #define ICMR3_ACKBT 0x08
>
> +#define ICFER_FMPE 0x80
> +
> #define ICIER_TIE 0x80
> #define ICIER_TEIE 0x40
> #define ICIER_RIE 0x20
> @@ -80,6 +82,7 @@ enum riic_reg_list {
> RIIC_ICCR2,
> RIIC_ICMR1,
> RIIC_ICMR3,
> + RIIC_ICFER,
> RIIC_ICSER,
> RIIC_ICIER,
> RIIC_ICSR2,
> @@ -92,6 +95,7 @@ enum riic_reg_list {
>
> struct riic_of_data {
> const u8 *regs;
> + bool fast_mode_plus;
> };
>
> struct riic_dev {
> @@ -315,11 +319,13 @@ static int riic_init_hw(struct riic_dev *riic)
> int total_ticks, cks, brl, brh;
> struct i2c_timings *t = &riic->i2c_t;
> struct device *dev = riic->adapter.dev.parent;
> + const struct riic_of_data *info = riic->info;
>
> - if (t->bus_freq_hz > I2C_MAX_FAST_MODE_FREQ) {
> - dev_err(dev,
> - "unsupported bus speed (%dHz). %d max\n",
> - t->bus_freq_hz, I2C_MAX_FAST_MODE_FREQ);
> + if ((!info->fast_mode_plus && t->bus_freq_hz > I2C_MAX_FAST_MODE_FREQ) ||
> + (info->fast_mode_plus && t->bus_freq_hz > I2C_MAX_FAST_MODE_PLUS_FREQ)) {
> + dev_err(dev, "unsupported bus speed (%dHz). %d max\n", t->bus_freq_hz,
> + info->fast_mode_plus ? I2C_MAX_FAST_MODE_PLUS_FREQ :
> + I2C_MAX_FAST_MODE_FREQ);
> return -EINVAL;
> }
>
> @@ -407,6 +413,9 @@ static int riic_init_hw(struct riic_dev *riic)
> riic_writeb(riic, 0, RIIC_ICSER);
> riic_writeb(riic, ICMR3_ACKWP | ICMR3_RDRFS, RIIC_ICMR3);
>
> + if (info->fast_mode_plus && t->bus_freq_hz == I2C_MAX_FAST_MODE_PLUS_FREQ)
> + riic_clear_set_bit(riic, 0, ICFER_FMPE, RIIC_ICFER);
> +
> riic_clear_set_bit(riic, ICCR1_IICRST, 0, RIIC_ICCR1);
>
> pm_runtime_mark_last_busy(dev);
> @@ -536,6 +545,7 @@ static const u8 riic_rz_a_regs[RIIC_REG_END] = {
> [RIIC_ICCR2] = 0x04,
> [RIIC_ICMR1] = 0x08,
> [RIIC_ICMR3] = 0x10,
> + [RIIC_ICFER] = 0x14,
> [RIIC_ICSER] = 0x18,
> [RIIC_ICIER] = 0x1c,
> [RIIC_ICSR2] = 0x24,
> @@ -549,11 +559,17 @@ static const struct riic_of_data riic_rz_a_info = {
> .regs = riic_rz_a_regs,
> };
>
> +static const struct riic_of_data riic_rz_g2_info = {
> + .regs = riic_rz_a_regs,
> + .fast_mode_plus = true,
> +};
> +
> static const u8 riic_rz_v2h_regs[RIIC_REG_END] = {
> [RIIC_ICCR1] = 0x00,
> [RIIC_ICCR2] = 0x01,
> [RIIC_ICMR1] = 0x02,
> [RIIC_ICMR3] = 0x04,
> + [RIIC_ICFER] = 0x05,
> [RIIC_ICSER] = 0x06,
> [RIIC_ICIER] = 0x07,
> [RIIC_ICSR2] = 0x09,
> @@ -565,6 +581,7 @@ static const u8 riic_rz_v2h_regs[RIIC_REG_END] = {
>
> static const struct riic_of_data riic_rz_v2h_info = {
> .regs = riic_rz_v2h_regs,
> + .fast_mode_plus = true,
> };
>
> static int riic_i2c_suspend(struct device *dev) @@ -613,6 +630,9 @@ static const struct dev_pm_ops
> riic_i2c_pm_ops = {
>
> static const struct of_device_id riic_i2c_dt_ids[] = {
> { .compatible = "renesas,riic-rz", .data = &riic_rz_a_info },
> + { .compatible = "renesas,riic-r9a07g043", .data = &riic_rz_g2_info, },
> + { .compatible = "renesas,riic-r9a07g044", .data = &riic_rz_g2_info, },
> + { .compatible = "renesas,riic-r9a07g054", .data = &riic_rz_g2_info,
> +},
I feel, the better way is
{ .compatible = "renesas, renesas,r7s72100", .data = &riic_rz_a_info },--> As this SoC does not support FMP
{ .compatible = "renesas,riic-rz", .data = &riic_rz_g2_info, },--> As this SoCs has FMP+ support
{ .compatible = "renesas,riic-r9a09g057", .data = &riic_rz_v2h_info },--> As this SoCs has different register layout and FMP+
With this the number of compatible entries in the device tables reduced from 5 to 3.
Cheers,
Biju
