mbox series

[v2,0/2] gpio: pca9570: add slg7xl45106 support

Message ID 20220915114803.26185-1-shubhrajyoti.datta@amd.com
Headers show
Series gpio: pca9570: add slg7xl45106 support | expand

Message

Shubhrajyoti Datta Sept. 15, 2022, 11:48 a.m. UTC 
Add SLG7XL45106 GPO expander
 
v2:
Use the platform data check instead of compatible
arrange alphabetically
rebase to the latest kernel


Shubhrajyoti Datta (2):
  dt-bindings: gpio: pca9570: Add compatible for slg7xl45106
  gpio: pca9570: add slg7xl45106 support

 .../bindings/gpio/gpio-pca9570.yaml           |  1 +
 drivers/gpio/gpio-pca9570.c                   | 39 +++++++++++++++++--
 2 files changed, 36 insertions(+), 4 deletions(-)

Comments

Sungbo Eo Sept. 17, 2022, 2 p.m. UTC | #1 
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://lore.kernel.org/all/1656426829-1008-3-git-send-email-shubhrajyoti.datta@xilinx.com/
And a link to the device datasheet would be also nice (if possible).

> 
>  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?

Thanks,
Sungbo