Re: [PATCH v12 1/7] i2c: core: add callback to change bus frequency

[Peter Rosin]

Hi!

On 2026-07-23 13:36, Marcus Folkesson wrote:
> Hi Peter!
> 
> On Thu, Jul 23, 2026 at 01:03:53PM +0200, Peter Rosin wrote:
>>> + *
>>> + * @set_clk_freq: Set clock frequency for the adapter. Returns the actual set frequency.
>>> + *      This function is optional.
>>>    */
>>>   struct i2c_adapter {
>>>   	struct module *owner;
>>> @@ -742,6 +745,8 @@ struct i2c_adapter {
>>>   	struct rt_mutex mux_lock;
>>>   
>>>   	int timeout;			/* in jiffies */
>>> +	u32 clock_Hz;			/* bus clock speed */
>>> +	int (*set_clk_freq)(struct i2c_adapter *adap, u32 clock_Hz); /* Optional */
>>
>> To expand on the comment from v9 by Wolfram [1], I too think it
>> would be good if this function pointer did not live directly in
>> i2c_adapter but instead in some other struct so that the actual
>> pointer could live in read-only memory. I.e. if i2c_algorithm
>> does not fit for some reason, a new function pointer struct might
>> be called for?
> 
> My objection to move it to i2c_algorithm, was mostly based on that the
> algorithm should not be aware of the current clock frequency. When I
> reread the comment from Wolfram (and you), I realize that we are only
> talking about the function pointer, which is fine to put into the
> i2c_algorithm struct.
> 
> Sorry for the misinterpretation, I will move the pointer to
> i2c_algorithm and keep clock_Hz in i2c_adapter.

Please note that the main point of moving the function pointer
into a struct is to enable instances of that struct to live in
const memory. Thus, if you add a random function pointer to some
unrelated struct, users of that struct will likely need to set
up their own instance with the needed mix of hooks. That works
poorly with the generic algos in drivers/i2c/algos that export
i2c_algorithm instances. If you need to manipulate .set_clk_freq
in one of those generic instances, then they can't be const.

Hence, it might be wise to put this new .set_clk_freq hook in
some other struct with function pointers where it fits better. If
no such struct exists, a new one might be needed?

Cheers,
Peter