Re: [PATCH 4/8] i2c: sprd: Add IIC controller driver to support dynamic switching of 400K/1M/3.4M frequency

[Chunyan Zhang]

On Thu, 17 Aug 2023 at 17:46, Huangzheng Lai <Huangzheng.Lai@xxxxxxxxxx> wrote:
>
> When IIC-slaves supporting different frequencies use the same IIC

%s/I2C/IIC

> controller, the IIC controller usually only operates at lower frequencies.
> In order to improve the performance of IIC-slaves transmission supporting
> faster frequencies, we dynamically configure the IIC operating frequency
> based on the value of the input parameter msg ->flag.
>
> Signed-off-by: Huangzheng Lai <Huangzheng.Lai@xxxxxxxxxx>
> ---
>  drivers/i2c/busses/i2c-sprd.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-sprd.c b/drivers/i2c/busses/i2c-sprd.c
> index 549b60dd3273..02c11a9ff2da 100644
> --- a/drivers/i2c/busses/i2c-sprd.c
> +++ b/drivers/i2c/busses/i2c-sprd.c
> @@ -75,7 +75,14 @@
>  #define SPRD_I2C_PM_TIMEOUT    1000
>  /* timeout (ms) for transfer message */
>  #define I2C_XFER_TIMEOUT       1000
> -
> +/* dynamic modify clk_freq flag  */
> +#define        I2C_3M4_FLAG            0x0100

#define <space> I2C_3M4_FLAG <tab> 0x0100

> +#define        I2C_1M_FLAG             0x0080
> +#define        I2C_400K_FLAG           0x0040
> +
> +#define        I2C_FREQ_400K           400000
> +#define        I2C_FREQ_1M             1000000
> +#define        I2C_FREQ_3_4M           3400000

ditto

>  /* SPRD i2c data structure */
>  struct sprd_i2c {
>         struct i2c_adapter adap;
> @@ -94,6 +101,8 @@ struct sprd_i2c {
>         int err;
>  };
>
> +static void sprd_i2c_set_clk(struct sprd_i2c *i2c_dev, u32 freq);

I would suggest moving this whole function above its caller.

> +
>  static void sprd_i2c_set_count(struct sprd_i2c *i2c_dev, u32 count)
>  {
>         writel(count, i2c_dev->base + I2C_COUNT);
> @@ -269,6 +278,12 @@ static int sprd_i2c_handle_msg(struct i2c_adapter *i2c_adap,
>                 sprd_i2c_send_stop(i2c_dev, !!is_last_msg);
>         }
>
> +       if (msg->flags & I2C_400K_FLAG)

Where does this flag set? I'm not sure we can use msg->flags like
this. I don't know i2c very well.

Thanks,
Chunyan

> +               sprd_i2c_set_clk(i2c_dev, I2C_FREQ_400K);
> +       else if (msg->flags & I2C_1M_FLAG)
> +               sprd_i2c_set_clk(i2c_dev, I2C_FREQ_1M);
> +       else if (msg->flags & I2C_3M4_FLAG)
> +               sprd_i2c_set_clk(i2c_dev, I2C_FREQ_3_4M);
>         /*
>          * We should enable rx fifo full interrupt to get data when receiving
>          * full data.
> --
> 2.17.1
>