Re: [PATCH v12 4/7] i2c: davinci: calculate bus freq from Hz instead of kHz

[Peter Rosin]

Hi!

On 2026-07-19 16:59, Marcus Folkesson wrote:
> The bus frequency is unnecessarily converted between Hz and kHz in
> several places.
> This is probably an old legacy from the old times (pre-devicetrees)
> when the davinci_i2c_platform_data took the bus_freq in kHz.
> 
> Stick to Hz.
> 
> Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> Signed-off-by: Marcus Folkesson <marcus.folkesson@xxxxxxxxx>
> ---
>   drivers/i2c/busses/i2c-davinci.c | 16 ++++++++--------
>   1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
> index e15eef163a8f..f543997c31e9 100644
> --- a/drivers/i2c/busses/i2c-davinci.c
> +++ b/drivers/i2c/busses/i2c-davinci.c
> @@ -132,8 +132,8 @@ struct davinci_i2c_dev {
>   #ifdef CONFIG_CPU_FREQ
>   	struct notifier_block	freq_transition;
>   #endif
> -	/* standard bus frequency (kHz) */
> -	unsigned int		bus_freq;
> +	/* standard bus frequency */
> +	unsigned int		bus_freq_Hz;
>   	/* Chip has a ICPFUNC register */
>   	bool			has_pfunc;
>   };
> @@ -207,16 +207,16 @@ static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
>   	if (device_is_compatible(dev->dev, "ti,keystone-i2c"))
>   		d = 6;
>   
> -	clk = ((input_clock / (psc + 1)) / (dev->bus_freq * 1000));
> +	clk = (input_clock / (psc + 1)) / (dev->bus_freq_Hz);

You can lose some brackets, e.g.

	clk = input_clock / (psc + 1) / dev->bus_freq_Hz;

Cheers,
Peter

>   	/* Avoid driving the bus too fast because of rounding errors above */
> -	if (input_clock / (psc + 1) / clk > dev->bus_freq * 1000)
> +	if (input_clock / (psc + 1) / clk > dev->bus_freq_Hz)
>   		clk++;
>   	/*
>   	 * According to I2C-BUS Spec 2.1, in FAST-MODE LOW period should be at
>   	 * least 1.3uS, which is not the case with 50% duty cycle. Driving HIGH
>   	 * to LOW ratio as 1 to 2 is more safe.
>   	 */
> -	if (dev->bus_freq > 100)
> +	if (dev->bus_freq_Hz > I2C_MAX_STANDARD_MODE_FREQ)
>   		clkl = (clk << 1) / 3;
>   	else
>   		clkl = (clk >> 1);
> @@ -240,7 +240,7 @@ static void i2c_davinci_calc_clk_dividers(struct davinci_i2c_dev *dev)
>   	davinci_i2c_write_reg(dev, DAVINCI_I2C_CLKH_REG, clkh);
>   	davinci_i2c_write_reg(dev, DAVINCI_I2C_CLKL_REG, clkl);
>   
> -	dev_dbg(dev->dev, "input_clock = %d, CLK = %d\n", input_clock, clk);
> +	dev_dbg(dev->dev, "input_clock = %u, CLK = %u\n", input_clock, clk);
>   }
>   
>   /*
> @@ -267,7 +267,7 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
>   		davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKL_REG));
>   	dev_dbg(dev->dev, "CLKH = %d\n",
>   		davinci_i2c_read_reg(dev, DAVINCI_I2C_CLKH_REG));
> -	dev_dbg(dev->dev, "bus_freq = %dkHz\n", dev->bus_freq);
> +	dev_dbg(dev->dev, "bus_freq_Hz = %dHz\n", dev->bus_freq_Hz);
>   
>   
>   	/* Take the I2C module out of reset: */
> @@ -760,7 +760,7 @@ static int davinci_i2c_probe(struct platform_device *pdev)
>   	if (r)
>   		prop = I2C_MAX_STANDARD_MODE_FREQ;
>   
> -	dev->bus_freq = prop / 1000;
> +	dev->bus_freq_Hz = prop;
>   
>   	dev->has_pfunc = device_property_present(&pdev->dev, "ti,has-pfunc");
>   
>