lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ