Re: [PATCH v5 6/6] i2c: Add driver for the RTL9300 I2C controller

Re: [PATCH v5 6/6] i2c: Add driver for the RTL9300 I2C controller

[Markus Elfring]

>> …
>>> +++ b/drivers/i2c/busses/i2c-rtl9300.c
>>> @@ -0,0 +1,422 @@
>> …
>>> +static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned short flags,
>>> +                  char read_write, u8 command, int size,
>>> +                  union i2c_smbus_data *data)
>>> +{
>> …
>>> +    mutex_lock(&i2c->lock);
>>> +    if (chan->sda_pin != i2c->sda_pin) {
>> …
>>> +out_unlock:
>>> +    mutex_unlock(&i2c->lock);
>>> +
>>> +    return ret;
>>> +}
>> …
>>
>> Under which circumstances would you become interested to apply a statement
>> like “guard(mutex)(&i2c->lock);”?
>> https://elixir.bootlin.com/linux/v6.11/source/include/linux/mutex.h#L196
>
> At this stage I don't what to change unless Andi insists that I do.
>
> I can't find much mention of using guard() on https://www.kernel.org/doc/html/latest/

Do you find any other information sources more encouraging?


> but I can see enough examples (although notably none in drivers/i2c) that I _think_ I can see how I could use it.

See also (for example):
Article “Linux Kernel Development - Automatic Cleanup”
by Javier Carrasco Cruz
2024-06-17
https://javiercarrascocruz.github.io/kernel-auto-cleanup-2#2-automatic-mutex-handling

Regards,
Markus

Re: [PATCH v5 6/6] i2c: Add driver for the RTL9300 I2C controller

[Javier Carrasco]

On 30/09/2024 09:55, Markus Elfring wrote:
>>> …
>>>> +++ b/drivers/i2c/busses/i2c-rtl9300.c
>>>> @@ -0,0 +1,422 @@
>>> …
>>>> +static int rtl9300_i2c_smbus_xfer(struct i2c_adapter *adap, u16 addr, unsigned short flags,
>>>> +                  char read_write, u8 command, int size,
>>>> +                  union i2c_smbus_data *data)
>>>> +{
>>> …
>>>> +    mutex_lock(&i2c->lock);
>>>> +    if (chan->sda_pin != i2c->sda_pin) {
>>> …
>>>> +out_unlock:
>>>> +    mutex_unlock(&i2c->lock);
>>>> +
>>>> +    return ret;
>>>> +}
>>> …
>>>
>>> Under which circumstances would you become interested to apply a statement
>>> like “guard(mutex)(&i2c->lock);”?
>>> https://elixir.bootlin.com/linux/v6.11/source/include/linux/mutex.h#L196
>>
>> At this stage I don't what to change unless Andi insists that I do.
>>
>> I can't find much mention of using guard() on https://www.kernel.org/doc/html/latest/
> 
> Do you find any other information sources more encouraging?
> 
> 
>> but I can see enough examples (although notably none in drivers/i2c) that I _think_ I can see how I could use it.
> 
> See also (for example):
> Article “Linux Kernel Development - Automatic Cleanup”
> by Javier Carrasco Cruz
> 2024-06-17
> https://javiercarrascocruz.github.io/kernel-auto-cleanup-2#2-automatic-mutex-handling
> 
> Regards,
> Markus

My personal blog is definitely NOT an official or even reliable source
of information.

Thanks for referencing it, but please look for examples of guard() in
the kernel, because there are several examples for different kind of
mutexes. For example, IIO uses them widely. And they are really nice, so
I would recommend anyone using them whenever it makes sense.

Best regards,
Javier Carrasco