RE: [PATCH v2 2/2] gpio: pca9570: add slg7xl45106 support

Datta, Shubhrajyoti
2022-09-19T06:32:15Z

[AMD Official Use Only - General]

Hi Sunbo,

> -----Original Message-----
> From: Sungbo Eo <mans0n@gorani.run>
> Sent: Saturday, September 17, 2022 7:31 PM
> To: Datta, Shubhrajyoti <shubhrajyoti.datta@amd.com>
> Cc: linux-gpio@vger.kernel.org; git (AMD-Xilinx) <git@amd.com>;
> devicetree@vger.kernel.org; krzysztof.kozlowski+dt@linaro.org;
> robh+dt@kernel.org; brgl@bgdev.pl; linus.walleij@linaro.org; Andy
> Shevchenko <andy.shevchenko@gmail.com>
> Subject: Re: [PATCH v2 2/2] gpio: pca9570: add slg7xl45106 support
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Hi,
>
> Thanks for the update.
> I was thinking I should reply to your patch in the last month, but I was a little
> busy at the time and I forgot to do so...
>
> On 2022-09-15 20:48, Shubhrajyoti Datta wrote:
> > slg7xl45106 is a I2C GPO expander.
> > Add a compatible string for the same. Also update the driver to write
> > and read from it.
> >
> > Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
> > ---
> > v2:
> > Use platform data insted of compatible
>
> Moving the command property into the new platform structure is nice.
> And please add more description about the device in the commit message.
> We don't even know the full name of the vendor from your patch.
> I like the older version of your patch in that perspective.
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Fall%2F1656426829-1008-3-git-send-email-
> shubhrajyoti.datta%40xilinx.com%2F&amp;data=05%7C01%7Cshubhrajyoti.d
> atta%40amd.com%7C9758241b75fc461113b608da98b50869%7C3dd8961fe488
> 4e608e11a82d994e183d%7C0%7C0%7C637990201003357055%7CUnknown%7
> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi
> LCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=x0RHFhr9X0L3VBzTRyRy
> VfLhm74gx7jBqUs2NEFhKcI%3D&amp;reserved=0
> And a link to the device datasheet would be also nice (if possible).
>

Will update the description.

> >
> > drivers/gpio/gpio-pca9570.c | 39
> > +++++++++++++++++++++++++++++++++----
> > 1 file changed, 35 insertions(+), 4 deletions(-)
>
> And I was also thinking that tpic2810 driver might be more appropriate then
> this pca9570 driver for a device with one command byte.
> Actually I had forked tpic2810 to create pca9570 to support a device without
> any command byte.
> Come to think of it, the two drivers may even be consolidated into a single
> generic one... What do you think?

I agree.
It looks to me that the current driver should work for the tpic2810 also by adding the compatible.
Do you agree?

>
> Thanks,
> Sungbo