RE: [PATCH v2 09/12] i2c: riic: Add support for fast mode plus

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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






[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux