Re: [PATCH v2] i2c: ls2x: Add clocks property parsing and adjust bus speed

[Hongliang Wang <wanghongliang@loongson.cn>]

Hi, Huacai,

On 2026/5/8 上午11:06, Huacai Chen wrote:
> Hi, Hongliang,
>
> On Thu, May 7, 2026 at 4:11 PM Hongliang Wang <wanghongliang@loongson.cn> wrote:
>> From: wanghongliang <wanghongliang@loongson.cn>
>>
>> The i2c-ls2x driver supports dts and acpi parameter passing.
>> In dts, uses clock framework, by parsing clocks property to
>> get i2c bus reference clock, and define factor by device data.
>> In acpi, by passing clocks property to describe i2c bus reference
>> clock and clock-div property to describe factor.
>> Based on i2c bus reference clock(clock_a), i2c bus speed(clock_s)
>> and factor, calculate the prcescale of i2c divider register.
>> The calculation formula is
>> prcescale = clock_a/(factor*clock_s)-1
>>
>> Signed-off-by: wanghongliang <wanghongliang@loongson.cn>
>> ---
>>   drivers/i2c/busses/i2c-ls2x.c | 42 ++++++++++++++++++++++++++++++++---
>>   1 file changed, 39 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-ls2x.c b/drivers/i2c/busses/i2c-ls2x.c
>> index b475dd27b7af..7db91e7a5d78 100644
>> --- a/drivers/i2c/busses/i2c-ls2x.c
>> +++ b/drivers/i2c/busses/i2c-ls2x.c
>> @@ -13,6 +13,7 @@
>>   #include <linux/bitfield.h>
>>   #include <linux/bits.h>
>>   #include <linux/completion.h>
>> +#include <linux/clk.h>
> This should be before completion.h.
>
OK.
>>   #include <linux/device.h>
>>   #include <linux/iopoll.h>
>>   #include <linux/i2c.h>
>> @@ -63,11 +64,16 @@
>>   /* The default bus frequency, which is an empirical value */
>>   #define LS2X_I2C_FREQ_STD      (33 * HZ_PER_KHZ)
>>
>> +struct ls2x_i2c_chip_data {
>> +       unsigned int    factor;
>> +};
>> +
>>   struct ls2x_i2c_priv {
>>          struct i2c_adapter      adapter;
>>          void __iomem            *base;
>>          struct i2c_timings      i2c_t;
>>          struct completion       cmd_complete;
>> +       const struct ls2x_i2c_chip_data *chip_data;
> Use "unsigned int    factor" directly?
>
OK,  I will adjust it
>>   };
>>
>>   /*
>> @@ -96,6 +102,8 @@ static irqreturn_t ls2x_i2c_isr(int this_irq, void *dev_id)
>>   static void ls2x_i2c_adjust_bus_speed(struct ls2x_i2c_priv *priv)
>>   {
>>          u16 val;
>> +       u32 pclk, factor;
> factor or div? If factor is better, then rename clock-div to
> clock-factor; if div is better, then rename factor to div.
div is better,  I will adjust it
>> +       struct clk *clk;
>>          struct i2c_timings *t = &priv->i2c_t;
>>          struct device *dev = priv->adapter.dev.parent;
>>          u32 acpi_speed = i2c_acpi_find_bus_speed(dev);
>> @@ -107,12 +115,30 @@ static void ls2x_i2c_adjust_bus_speed(struct ls2x_i2c_priv *priv)
>>          else
>>                  t->bus_freq_hz = LS2X_I2C_FREQ_STD;
>>
>> +       if (dev_of_node(dev)) {
>> +               clk = devm_clk_get_optional_enabled(dev, NULL);
>> +               if (IS_ERR(clk) || !clk)
>> +                       pclk = LS2X_I2C_PCLK_FREQ;
>> +               else
>> +                       pclk = clk_get_rate(clk);
> Reverse the "if & else" so the default case will be the last one, this
> is also the same as the ACPI case below.
OK
> Huacai
>
>> +
>> +               factor = priv->chip_data->factor;
>> +
>> +               val = (pclk * 10) / (factor * t->bus_freq_hz) - 1;
>> +       } else {
>> +               if (!device_property_read_u32(dev, "clocks", &pclk) &&
>> +                   !device_property_read_u32(dev, "clock-div", &factor) &&
>> +                   factor != 0)
>> +                       val = (pclk * 10) / (factor * t->bus_freq_hz) - 1;
>> +               else
>> +                       val = LS2X_I2C_PCLK_FREQ / (5 * t->bus_freq_hz) - 1;
>> +       }
>> +
>>          /*
>>           * According to the chip manual, we can only access the registers as bytes,
>>           * otherwise the high bits will be truncated.
>>           * So set the I2C frequency with a sequential writeb() instead of writew().
>>           */
>> -       val = LS2X_I2C_PCLK_FREQ / (5 * t->bus_freq_hz) - 1;
>>          writeb(FIELD_GET(GENMASK(7, 0), val), priv->base + I2C_LS2X_PRER_LO);
>>          writeb(FIELD_GET(GENMASK(15, 8), val), priv->base + I2C_LS2X_PRER_HI);
>>   }
>> @@ -295,6 +321,8 @@ static int ls2x_i2c_probe(struct platform_device *pdev)
>>          if (!priv)
>>                  return -ENOMEM;
>>
>> +       priv->chip_data = device_get_match_data(dev);
>> +
>>          /* Map hardware registers */
>>          priv->base = devm_platform_ioremap_resource(pdev, 0);
>>          if (IS_ERR(priv->base))
>> @@ -348,9 +376,17 @@ static int ls2x_i2c_resume(struct device *dev)
>>   static DEFINE_RUNTIME_DEV_PM_OPS(ls2x_i2c_pm_ops,
>>                                   ls2x_i2c_suspend, ls2x_i2c_resume, NULL);
>>
>> +static const struct ls2x_i2c_chip_data ls2x_i2c_2k_data = {
>> +       .factor = 4,
>> +};
>> +
>> +static const struct ls2x_i2c_chip_data ls2x_i2c_7a_data = {
>> +       .factor = 5,
>> +};
>> +
>>   static const struct of_device_id ls2x_i2c_id_table[] = {
>> -       { .compatible = "loongson,ls2k-i2c" },
>> -       { .compatible = "loongson,ls7a-i2c" },
>> +       { .compatible = "loongson,ls2k-i2c", .data = &ls2x_i2c_2k_data, },
>> +       { .compatible = "loongson,ls7a-i2c", .data = &ls2x_i2c_7a_data, },
>>          { /* sentinel */ }
>>   };
>>   MODULE_DEVICE_TABLE(of, ls2x_i2c_id_table);
>> --
>> 2.47.2
>>
>>
Best regards,
Hongliang Wang