Message ID | 20220817085550.18887-3-shubhrajyoti.datta@amd.com |
---|---|
State | New |
Headers | show |
Series | gpio: pca9570: add slg7xl45106 support | expand |
On 17/08/2022 11:55, 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. Please fix your wrapping to match coding style. > > > @@ -94,6 +100,7 @@ static void pca9570_set(struct gpio_chip *chip, unsigned int offset, int value) > static int pca9570_probe(struct i2c_client *client) > { > struct pca9570 *gpio; > + struct device_node *np = client->dev.of_node; > > gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL); > if (!gpio) > @@ -109,6 +116,9 @@ static int pca9570_probe(struct i2c_client *client) > gpio->chip.ngpio = (uintptr_t)device_get_match_data(&client->dev); > gpio->chip.can_sleep = true; > > + if (of_device_is_compatible(np, "dlg,slg7xl45106")) Matching is done once and you have driver data for this. No compatibles in the code, this does not scale. > + gpio->command = SLG7XL45106_GPO_REG; > + > mutex_init(&gpio->lock); > > /* Read the current output level */ > @@ -121,12 +131,14 @@ static int pca9570_probe(struct i2c_client *client) > > static const struct i2c_device_id pca9570_id_table[] = { > { "pca9570", 4 }, > + { "slg7xl45106", 8 }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(i2c, pca9570_id_table); > > static const struct of_device_id pca9570_of_match_table[] = { > { .compatible = "nxp,pca9570", .data = (void *)4 }, This is some old tree, isn't it? > + { .compatible = "dlg,slg7xl45106", .data = (void *)8 }, > { /* sentinel */ } > }; > MODULE_DEVICE_TABLE(of, pca9570_of_match_table); Best regards, Krzysztof
On Wed, Aug 17, 2022 at 12:10 PM Shubhrajyoti Datta <shubhrajyoti.datta@amd.com> wrote: > > slg7xl45106 is a I2C GPO expander. > Add a compatible string for the same. Also update the > driver to write and read from it. ... > + struct device_node *np = client->dev.of_node; > + if (of_device_is_compatible(np, "dlg,slg7xl45106")) > + gpio->command = SLG7XL45106_GPO_REG; Please, avoid making the driver OF dependent when there is no reason to do that. Note, we have device property and fwnode APIs that may cover a lot of cases for different property providers.
diff --git a/drivers/gpio/gpio-pca9570.c b/drivers/gpio/gpio-pca9570.c index 2735cd6addcf..b4e422b25ea6 100644 --- a/drivers/gpio/gpio-pca9570.c +++ b/drivers/gpio/gpio-pca9570.c @@ -15,6 +15,8 @@ #include <linux/mutex.h> #include <linux/property.h> +#define SLG7XL45106_GPO_REG 0xDB + /** * struct pca9570 - GPIO driver data * @chip: GPIO controller chip @@ -25,6 +27,7 @@ struct pca9570 { struct gpio_chip chip; struct mutex lock; u8 out; + u8 command; }; static int pca9570_read(struct pca9570 *gpio, u8 *value) @@ -32,7 +35,7 @@ static int pca9570_read(struct pca9570 *gpio, u8 *value) struct i2c_client *client = to_i2c_client(gpio->chip.parent); int ret; - ret = i2c_smbus_read_byte(client); + ret = i2c_smbus_read_byte_data(client, gpio->command); if (ret < 0) return ret; @@ -44,6 +47,9 @@ static int pca9570_write(struct pca9570 *gpio, u8 value) { struct i2c_client *client = to_i2c_client(gpio->chip.parent); + if (gpio->command) + return i2c_smbus_write_byte_data(client, gpio->command, value); + return i2c_smbus_write_byte(client, value); } @@ -94,6 +100,7 @@ static void pca9570_set(struct gpio_chip *chip, unsigned int offset, int value) static int pca9570_probe(struct i2c_client *client) { struct pca9570 *gpio; + struct device_node *np = client->dev.of_node; gpio = devm_kzalloc(&client->dev, sizeof(*gpio), GFP_KERNEL); if (!gpio) @@ -109,6 +116,9 @@ static int pca9570_probe(struct i2c_client *client) gpio->chip.ngpio = (uintptr_t)device_get_match_data(&client->dev); gpio->chip.can_sleep = true; + if (of_device_is_compatible(np, "dlg,slg7xl45106")) + gpio->command = SLG7XL45106_GPO_REG; + mutex_init(&gpio->lock); /* Read the current output level */ @@ -121,12 +131,14 @@ static int pca9570_probe(struct i2c_client *client) static const struct i2c_device_id pca9570_id_table[] = { { "pca9570", 4 }, + { "slg7xl45106", 8 }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(i2c, pca9570_id_table); static const struct of_device_id pca9570_of_match_table[] = { { .compatible = "nxp,pca9570", .data = (void *)4 }, + { .compatible = "dlg,slg7xl45106", .data = (void *)8 }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, pca9570_of_match_table);
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> --- drivers/gpio/gpio-pca9570.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)