| Message ID | 20220614151722.2194936-3-sravanhome@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | [1/6] iio: adc: mp2629: fix wrong comparison of channel | expand |
On Tue, Jun 14, 2022 at 5:17 PM Saravanan Sekar <sravanhome@gmail.com> wrote: > > mp2733 is updated version of mp2629 battery charge management > device for single-cell Li-ion or Li-polymer battery. Additionally > supports usb fast-charge and higher range of input voltage. ... > +#include <linux/of_device.h> What the original code misses is the mod_devicetable.h, and also see below. ... > +static const struct of_device_id mp2629_of_match[] = { > + { .compatible = "mps,mp2629", .data = (void *)CHIP_ID_MP2629 }, > + { .compatible = "mps,mp2733", .data = (void *)CHIP_ID_MP2733 }, > + { } > +}; > +MODULE_DEVICE_TABLE(of, mp2629_of_match); No need to move, see below. ... > +static int mp2629_probe(struct i2c_client *client, > + const struct i2c_device_id *id) Why out of a sudden you moved from ->probe_new() to ->probe()? > + enum mp2xx_chip_id chip_id; > + const struct of_device_id *of_id; > int ret; > > + if (client->dev.of_node) { > + of_id = of_match_device(mp2629_of_match, &client->dev); > + if (!of_id) { > + dev_err(&client->dev, "Failed to match device\n"); > + return -ENODEV; > + } > + chip_id = (enum mp2xx_chip_id)of_id->data; > + } This all is a single LoC only + property.h: #include <linux/property.h> enum mp2xx_chip_id chip_id; chip_id = (uintptr_t)device_get_match_data(&client->dev);
Hello Andy, Thanks for your time to review, I try fix all the review comments On 14/06/22 18:05, Andy Shevchenko wrote: > On Tue, Jun 14, 2022 at 5:17 PM Saravanan Sekar <sravanhome@gmail.com> wrote: >> >> mp2733 is updated version of mp2629 battery charge management >> device for single-cell Li-ion or Li-polymer battery. Additionally >> supports usb fast-charge and higher range of input voltage. > > ... > >> +#include <linux/of_device.h> > > What the original code misses is the mod_devicetable.h, and also see below. > > ... > >> +static const struct of_device_id mp2629_of_match[] = { >> + { .compatible = "mps,mp2629", .data = (void *)CHIP_ID_MP2629 }, >> + { .compatible = "mps,mp2733", .data = (void *)CHIP_ID_MP2733 }, >> + { } >> +}; >> +MODULE_DEVICE_TABLE(of, mp2629_of_match); > > No need to move, see below. > > ... > >> +static int mp2629_probe(struct i2c_client *client, >> + const struct i2c_device_id *id) > > Why out of a sudden you moved from ->probe_new() to ->probe()? > I was experiment to pass i2c_device_id table to differentiate, the used compatible. I will switch back to probe_new. >> + enum mp2xx_chip_id chip_id; >> + const struct of_device_id *of_id; >> int ret; >> >> + if (client->dev.of_node) { >> + of_id = of_match_device(mp2629_of_match, &client->dev); >> + if (!of_id) { >> + dev_err(&client->dev, "Failed to match device\n"); >> + return -ENODEV; >> + } >> + chip_id = (enum mp2xx_chip_id)of_id->data; >> + } > > This all is a single LoC only + property.h: > > #include <linux/property.h> > > enum mp2xx_chip_id chip_id; > > chip_id = (uintptr_t)device_get_match_data(&client->dev); > sure. Thanks, Saravanan
diff --git a/drivers/mfd/mp2629.c b/drivers/mfd/mp2629.c index 16840ec5fd1c..e0bbba9ca853 100644 --- a/drivers/mfd/mp2629.c +++ b/drivers/mfd/mp2629.c @@ -12,6 +12,7 @@ #include <linux/mfd/core.h> #include <linux/mfd/mp2629.h> #include <linux/module.h> +#include <linux/of_device.h> #include <linux/platform_device.h> #include <linux/regmap.h> #include <linux/slab.h> @@ -33,16 +34,36 @@ static const struct regmap_config mp2629_regmap_config = { .max_register = 0x17, }; -static int mp2629_probe(struct i2c_client *client) +static const struct of_device_id mp2629_of_match[] = { + { .compatible = "mps,mp2629", .data = (void *)CHIP_ID_MP2629 }, + { .compatible = "mps,mp2733", .data = (void *)CHIP_ID_MP2733 }, + { } +}; +MODULE_DEVICE_TABLE(of, mp2629_of_match); + +static int mp2629_probe(struct i2c_client *client, + const struct i2c_device_id *id) { struct mp2629_data *ddata; + enum mp2xx_chip_id chip_id; + const struct of_device_id *of_id; int ret; + if (client->dev.of_node) { + of_id = of_match_device(mp2629_of_match, &client->dev); + if (!of_id) { + dev_err(&client->dev, "Failed to match device\n"); + return -ENODEV; + } + chip_id = (enum mp2xx_chip_id)of_id->data; + } + ddata = devm_kzalloc(&client->dev, sizeof(*ddata), GFP_KERNEL); if (!ddata) return -ENOMEM; ddata->dev = &client->dev; + ddata->chip_id = chip_id; i2c_set_clientdata(client, ddata); ddata->regmap = devm_regmap_init_i2c(client, &mp2629_regmap_config); @@ -59,18 +80,12 @@ static int mp2629_probe(struct i2c_client *client) return ret; } -static const struct of_device_id mp2629_of_match[] = { - { .compatible = "mps,mp2629"}, - { } -}; -MODULE_DEVICE_TABLE(of, mp2629_of_match); - static struct i2c_driver mp2629_driver = { .driver = { .name = "mp2629", .of_match_table = mp2629_of_match, }, - .probe_new = mp2629_probe, + .probe = mp2629_probe, }; module_i2c_driver(mp2629_driver); diff --git a/include/linux/mfd/mp2629.h b/include/linux/mfd/mp2629.h index 89b706900b57..ee0e65720c75 100644 --- a/include/linux/mfd/mp2629.h +++ b/include/linux/mfd/mp2629.h @@ -9,9 +9,15 @@ #include <linux/device.h> #include <linux/regmap.h> +enum mp2xx_chip_id { + CHIP_ID_MP2629, + CHIP_ID_MP2733, +}; + struct mp2629_data { struct device *dev; struct regmap *regmap; + enum mp2xx_chip_id chip_id; }; enum mp2629_adc_chan {
mp2733 is updated version of mp2629 battery charge management device for single-cell Li-ion or Li-polymer battery. Additionally supports usb fast-charge and higher range of input voltage. Signed-off-by: Saravanan Sekar <sravanhome@gmail.com> --- drivers/mfd/mp2629.c | 31 +++++++++++++++++++++++-------- include/linux/mfd/mp2629.h | 6 ++++++ 2 files changed, 29 insertions(+), 8 deletions(-)