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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 31 Aug 2023 15:44:20 +0800
From:   Chunyan Zhang <zhang.lyra@...il.com>
To:     Huangzheng Lai <Huangzheng.Lai@...soc.com>
Cc:     Andi Shyti <andi.shyti@...nel.org>,
        Orson Zhai <orsonzhai@...il.com>,
        Baolin Wang <baolin.wang@...ux.alibaba.com>,
        linux-i2c@...r.kernel.org, linux-kernel@...r.kernel.org,
        huangzheng lai <laihuangzheng@...il.com>,
        Xiongpeng Wu <xiongpeng.wu@...soc.com>
Subject: Re: [PATCH 4/8] i2c: sprd: Add IIC controller driver to support
 dynamic switching of 400K/1M/3.4M frequency

On Thu, 17 Aug 2023 at 17:46, Huangzheng Lai <Huangzheng.Lai@...soc.com> 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@...soc.com>
> ---
>  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
>