Re: [PATCH 2/2] gpio: Add support for SLG7XL45106 I2C GPO expander

[Michal Simek <michal.simek@amd.com>]

On 6/29/22 12:15, Andy Shevchenko wrote:
> On Wed, Jun 29, 2022 at 9:14 AM Michal Simek <michal.simek@amd.com> wrote:
>> On 6/29/22 03:00, Sungbo Eo wrote:
>>> On 2022-06-29 04:21, Andy Shevchenko wrote:
>>>> On Tue, Jun 28, 2022 at 9:13 PM Andy Shevchenko
>>>> <andy.shevchenko@gmail.com> wrote:
> 
> ...
> 
>>>> Actually, why can't pca9570 be amended to support this?
> 
>>> It seems the slg7xl45106 driver reads/writes a reg at 0xDB so it is not
>>> compatible with pca9570 driver (in the current state), and (I suppose)
>>> it could be converted to use gpio-regmap.
>>>
>>> [1]
>>> https://lore.kernel.org/linux-gpio/69f5d1a1970838b8c4bd8d6e8dba6cac@walle.cc/
>>
>> As was mentioned driver is based on pca9570 and the only important difference is
>> with i2c_smbus_read_byte/i2c_smbus_read_byte_data and especially
>> i2c_smbus_write_byte/i2c_smbus_write_byte_data.
>>
>> Read can be aligned without any issue but write will have if/else because of
>> i2c_smbus_write_byte_data. Example below.
>>
>> Something like this. If this change is fine I think there won't be any issue to
>> just merge it with pca9570.
> 
> Thanks, I also would like to see something as below in the result.

ok. Good.

> 
>> diff --git a/drivers/gpio/gpio-slg7xl45106.c b/drivers/gpio/gpio-slg7xl45106.c
>> index bf25e6fb6782..b90950ae38c1 100644
>> --- a/drivers/gpio/gpio-slg7xl45106.c
>> +++ b/drivers/gpio/gpio-slg7xl45106.c
>> @@ -22,20 +22,24 @@
>>    struct slg7xl45106 {
>>           struct gpio_chip chip;
>>           struct mutex lock;      /* To protect writes */
>> +       u32 command;
>>    };
>>
>>    static int slg7xl45106_read(struct slg7xl45106 *gpio)
>>    {
>>           struct i2c_client *client = to_i2c_client(gpio->chip.parent);
>>
>> -       return i2c_smbus_read_byte_data(client, SLG7XL45106_GPO_REG);
>> +       return i2c_smbus_read_byte_data(client, gpio->command);
>>    }
>>
>>    static int slg7xl45106_write(struct slg7xl45106 *gpio, u8 value)
>>    {
>>           struct i2c_client *client = to_i2c_client(gpio->chip.parent);
>>
>> -       return i2c_smbus_write_byte_data(client, SLG7XL45106_GPO_REG, value);
>> +       if (gpio->command)
>> +               return i2c_smbus_write_byte_data(client, SLG7XL45106_GPO_REG,
>> value);
> 
> Missed change to gpio->command :-)

I found it too. :-)

Shubhrajyoti: Can you please merge that slg driver to 9570? That dt-binding will 
require small massage too.

Thanks,
Michal