Hi,
On 6/29/22 03:00, Sungbo Eo wrote:
Hi,
On 2022-06-29 04:21, Andy Shevchenko wrote:
On Tue, Jun 28, 2022 at 9:13 PM Andy Shevchenko
<andy.shevchenko@xxxxxxxxx> wrote:
On Tue, Jun 28, 2022 at 4:35 PM Shubhrajyoti Datta
<shubhrajyoti.datta@xxxxxxxxxx> wrote:
From: Raviteja Narayanam <raviteja.narayanam@xxxxxxxxxx>
Dialog semiconductors SLG7XL45106 is an 8-bit I2C GPO expander.
The output port is controlled by a data byte with register
address.
1/ Have you checked if there is a driver that sounds very similar to
this already upstream?
Actually, why can't pca9570 be amended to support this?
2/ Have you tried to use gpio-regmap? Why can it not be used?
Same Q for PCA9570 driver. (Maybe gpio-regmap wasn't existed that time?)
IIRC regmap could not be used for pca9570 as pca9570 doesn't have a
concept of "address"; it only accepts a data value.[1]
Please let me know if the situation has changed in the meantime.
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@xxxxxxxx/
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,
Michal
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);
+
+ return i2c_smbus_write_byte(client, value);
}
static int slg7xl45106_get_direction(struct gpio_chip *chip,
@@ -93,6 +97,9 @@ static int slg7xl45106_probe(struct i2c_client *client)
gpio->chip.ngpio = (uintptr_t)device_get_match_data(&client->dev);
gpio->chip.can_sleep = true;
+ /* will be filled based on compatible string, 0 for pca9570 */
+ gpio->command = SLG7XL45106_GPO_REG;
+
mutex_init(&gpio->lock);
i2c_set_clientdata(client, gpio);
