[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <a7182597-b45e-40cf-baeb-60f69ec2365d@marvell.com>
Date: Fri, 29 Nov 2024 19:33:19 +0530
From: Amit Singh Tomar <amitsinght@...vell.com>
To: Dzmitry Sankouski <dsankouski@...il.com>,
Hans de Goede <hdegoede@...hat.com>,
Krzysztof Kozlowski <krzk@...nel.org>,
Marek Szyprowski <m.szyprowski@...sung.com>,
Sebastian Krzyszkowiak <sebastian.krzyszkowiak@...i.sm>,
Purism Kernel Team <kernel@...i.sm>,
Sebastian Reichel <sre@...nel.org>, Rob Herring <robh@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Alim Akhtar <alim.akhtar@...sung.com>, Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>
Cc: linux-pm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-samsung-soc@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-msm@...r.kernel.org
Subject: [PATCH v4 2/2] power: supply: max17042: add platform driver variant
Hi,
>
> The solution here add and option to use max17042 driver as a MFD
> sub device, thus allowing any additional functionality be implemented as
> another sub device. This will help to reduce code duplication in MFD
> fuel gauge drivers.
>
> Signed-off-by: Dzmitry Sankouski <dsankouski@...il.com>
> ---
> Changes in v4:
> - rename module_init and module_exit fuctions
> - rework max17042_init
> - assign chip_type in probe function
> - pass i2c_client as pointer on pointer, to use same pointer created in
> MFD. This allows devm_regmap_init_i2c to cleanup gracefully.
>
> Changes in v3:
> - pass dev pointer in max17042_probe
> - remove prints
> ---
> drivers/power/supply/max17042_battery.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 92 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/power/supply/max17042_battery.c b/drivers/power/supply/max17042_battery.c
> index 99bf6915aa23..6a1bfc4a7b13 100644
> --- a/drivers/power/supply/max17042_battery.c
> +++ b/drivers/power/supply/max17042_battery.c
> @@ -16,6 +16,7 @@
> #include <linux/i2c.h>
> #include <linux/delay.h>
> #include <linux/interrupt.h>
> +#include <linux/platform_device.h>
> #include <linux/pm.h>
> #include <linux/mod_devicetable.h>
> #include <linux/power_supply.h>
> @@ -1029,14 +1030,12 @@ static const struct power_supply_desc max17042_no_current_sense_psy_desc = {
> .num_properties = ARRAY_SIZE(max17042_battery_props) - 2,
> };
>
> -static int max17042_probe(struct i2c_client *client)
> +static int max17042_probe(struct i2c_client *client, struct device *dev,
> + enum max170xx_chip_type chip_type)
> {
> - const struct i2c_device_id *id = i2c_client_get_device_id(client);
> struct i2c_adapter *adapter = client->adapter;
> const struct power_supply_desc *max17042_desc = &max17042_psy_desc;
> struct power_supply_config psy_cfg = {};
> - const struct acpi_device_id *acpi_id = NULL;
> - struct device *dev = &client->dev;
> struct max17042_chip *chip;
> int ret;
> int i;
> @@ -1045,33 +1044,25 @@ static int max17042_probe(struct i2c_client *client)
> if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA))
> return -EIO;
>
> - chip = devm_kzalloc(&client->dev, sizeof(*chip), GFP_KERNEL);
> + chip = devm_kzalloc(dev, sizeof(*chip), GFP_KERNEL);
> if (!chip)
> return -ENOMEM;
>
> chip->client = client;
> - if (id) {
> - chip->chip_type = id->driver_data;
> - } else {
> - acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev);
> - if (!acpi_id)
> - return -ENODEV;
> -
> - chip->chip_type = acpi_id->driver_data;
> - }
> + chip->chip_type = chip_type;
> chip->regmap = devm_regmap_init_i2c(client, &max17042_regmap_config);
> if (IS_ERR(chip->regmap)) {
> - dev_err(&client->dev, "Failed to initialize regmap\n");
> + dev_err(dev, "Failed to initialize regmap\n");
> return -EINVAL;
> }
>
> chip->pdata = max17042_get_pdata(chip);
> if (!chip->pdata) {
> - dev_err(&client->dev, "no platform data provided\n");
> + dev_err(dev, "no platform data provided\n");
> return -EINVAL;
> }
>
> - i2c_set_clientdata(client, chip);
> + dev_set_drvdata(dev, chip);
> psy_cfg.drv_data = chip;
> psy_cfg.of_node = dev->of_node;
>
> @@ -1095,17 +1086,17 @@ static int max17042_probe(struct i2c_client *client)
> regmap_write(chip->regmap, MAX17042_LearnCFG, 0x0007);
> }
>
> - chip->battery = devm_power_supply_register(&client->dev, max17042_desc,
> + chip->battery = devm_power_supply_register(dev, max17042_desc,
> &psy_cfg);
> if (IS_ERR(chip->battery)) {
> - dev_err(&client->dev, "failed: power supply register\n");
> + dev_err(dev, "failed: power supply register\n");
> return PTR_ERR(chip->battery);
> }
>
> if (client->irq) {
> unsigned int flags = IRQF_ONESHOT | IRQF_SHARED | IRQF_PROBE_SHARED;
>
> - ret = devm_request_threaded_irq(&client->dev, client->irq,
> + ret = devm_request_threaded_irq(dev, client->irq,
> NULL,
> max17042_thread_handler, flags,
> chip->battery->desc->name,
> @@ -1118,7 +1109,7 @@ static int max17042_probe(struct i2c_client *client)
> } else {
> client->irq = 0;
> if (ret != -EBUSY)
> - dev_err(&client->dev, "Failed to get IRQ\n");
> + dev_err(dev, "Failed to get IRQ\n");
> }
> }
> /* Not able to update the charge threshold when exceeded? -> disable */
> @@ -1127,7 +1118,7 @@ static int max17042_probe(struct i2c_client *client)
>
> regmap_read(chip->regmap, MAX17042_STATUS, &val);
> if (val & STATUS_POR_BIT) {
> - ret = devm_work_autocancel(&client->dev, &chip->work,
> + ret = devm_work_autocancel(dev, &chip->work,
> max17042_init_worker);
> if (ret)
> return ret;
> @@ -1139,6 +1130,38 @@ static int max17042_probe(struct i2c_client *client)
> return 0;
> }
>
> +static int max17042_i2c_probe(struct i2c_client *client)
> +{
> + const struct i2c_device_id *id = i2c_client_get_device_id(client);
> + const struct acpi_device_id *acpi_id = NULL;
> + struct device *dev = &client->dev;
> + enum max170xx_chip_type chip_type;
> +
> + if (id) {
> + chip_type = id->driver_data;
> + } else {
> + acpi_id = acpi_match_device(dev->driver->acpi_match_table, dev);
> + if (!acpi_id)
> + return -ENODEV;
> +
> + chip_type = acpi_id->driver_data;
> + }
> +
> + return max17042_probe(client, dev, chip_type);
> +}
> +
> +static int max17042_platform_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct i2c_client **i2c = dev_get_platdata(dev);
This seems a bit unusual; can't we just use:
struct i2c_client *i2c = dev_get_platdata(&pdev->dev); instead?
> + const struct platform_device_id *id = platform_get_device_id(pdev);
> +
> + if (!i2c)
> + return -EINVAL;
> +
> + return max17042_probe(*i2c, dev, id->driver_data);
and then just pass "i2c" here ?
> +}
> +
> #ifdef CONFIG_PM_SLEEP
> static int max17042_suspend(struct device *dev)
> {
> @@ -1204,6 +1227,16 @@ static const struct i2c_device_id max17042_id[] = {
> };
> MODULE_DEVICE_TABLE(i2c, max17042_id);
>
> +static const struct platform_device_id max17042_platform_id[] = {
> + { "max17042", MAXIM_DEVICE_TYPE_MAX17042 },
> + { "max17047", MAXIM_DEVICE_TYPE_MAX17047 },
> + { "max17050", MAXIM_DEVICE_TYPE_MAX17050 },
> + { "max17055", MAXIM_DEVICE_TYPE_MAX17055 },
> + { "max77849-battery", MAXIM_DEVICE_TYPE_MAX17047 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(platform, max17042_platform_id);
> +
> static struct i2c_driver max17042_i2c_driver = {
> .driver = {
> .name = "max17042",
> @@ -1211,10 +1244,45 @@ static struct i2c_driver max17042_i2c_driver = {
> .of_match_table = of_match_ptr(max17042_dt_match),
> .pm = &max17042_pm_ops,
> },
> - .probe = max17042_probe,
> + .probe = max17042_i2c_probe,
> .id_table = max17042_id,
> };
> -module_i2c_driver(max17042_i2c_driver);
> +
> +static struct platform_driver max17042_platform_driver = {
> + .driver = {
> + .name = "max17042",
> + .acpi_match_table = ACPI_PTR(max17042_acpi_match),
> + .of_match_table = of_match_ptr(max17042_dt_match),
> + .pm = &max17042_pm_ops,
> + },
> + .probe = max17042_platform_probe,
> + .id_table = max17042_platform_id,
> +};
> +
> +static int __init max17042_init(void)
> +{
> + int ret;
> +
> + ret = platform_driver_register(&max17042_platform_driver);
> + if (ret)
> + return ret;
> +
> + ret = i2c_add_driver(&max17042_i2c_driver);
> + if (ret) {
> + platform_driver_unregister(&max17042_platform_driver);
> + return ret;
> + }
> +
> + return 0;
> +}
> +module_init(max17042_init);
> +
> +static void __exit max17042_exit(void)
> +{
> + i2c_del_driver(&max17042_i2c_driver);
> + platform_driver_unregister(&max17042_platform_driver);
> +}
> +module_exit(max17042_exit);
>
> MODULE_AUTHOR("MyungJoo Ham <myungjoo.ham@...sung.com>");
> MODULE_DESCRIPTION("MAX17042 Fuel Gauge");
>
> --
> 2.39.5
>
>
Powered by blists - more mailing lists