Re: [PATCH 2/2] i2c: tegra: Add SMBus block read and SMBus alert functions

[Dmitry Osipenko]

10.12.2021 12:38, Akhil R пишет:
>>> +struct tegra_i2c_smbalert {
>> smbalert isn't a word, should be smbus_alert. Same for the GPIO name and
>> other places.
> The actual notation, I suppose, is SMBALERT#. If you see it is better to make it 
> smbus_alert, I can update it. 'smbalert' also looks good to me though.

smbus_alert is better

>>> +     struct i2c_smbus_alert_setup alert_data;
>>> +     struct i2c_client *ara;
>> What "ara" stands for? Please use meaningful names, like alert_dev for
>> example.
> i2c-smbus.c uses the same name to refer to the smbus alert i2c_client.
> I thought to use the same for consistency with the core and other drivers.
> It stands for "Alert response address".

Please add doc comment for the struct with the explained name then.

>> I don't see where this member is used at all, please remove it.
> Wouldn't it be good to have it stored so that it is handy if using any other 
> function later, like i2c_handle_smbus_alert. Moreover, I thought to keep
> the usage consistent with other drivers having similar function.

Please add it only once it will be actually needed.