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

Michal Simek <michal.simek@xxxxxxx> · Mon, 19 Sep 2022 09:01:53 +0200

Hi,

On 9/19/22 08:32, Datta, Shubhrajyoti wrote:
[AMD Official Use Only - General]

Hi Sunbo,

-----Original Message-----
From: Sungbo Eo <mans0n@xxxxxxxxxx>
Sent: Saturday, September 17, 2022 7:31 PM
To: Datta, Shubhrajyoti <shubhrajyoti.datta@xxxxxxx>
Cc: linux-gpio@xxxxxxxxxxxxxxx; git (AMD-Xilinx) <git@xxxxxxx>;
devicetree@xxxxxxxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx;
robh+dt@xxxxxxxxxx; brgl@xxxxxxxx; linus.walleij@xxxxxxxxxx; Andy
Shevchenko <andy.shevchenko@xxxxxxxxx>
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@xxxxxxx>
---
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?

You will have to solve issue with Kconfig symbols. Anyway if you want to merge 
these two drivers together it has to be done on the top of this series anyway. 
It means get this first part merged and then another device can be on the top of 
this series.

Thanks,
Michal