[PATCH] hwmon: (asus-ec-sensors) merge setup functions

[PATCH] hwmon: (asus-ec-sensors) merge setup functions

[Eugene Shalygin]

Merge configure_sensor_setup() into probe().

Signed-off-by: Eugene Shalygin <eugene.shalygin@gmail.com>
---
 drivers/hwmon/asus-ec-sensors.c | 55 ++++++++++++++-------------------
 1 file changed, 24 insertions(+), 31 deletions(-)

diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
index bfac08a5dc57..45de8d34a588 100644
--- a/drivers/hwmon/asus-ec-sensors.c
+++ b/drivers/hwmon/asus-ec-sensors.c
@@ -589,31 +589,40 @@ get_board_sensors(const struct device *dev)
 	return (unsigned long)dmi_entry->driver_data;
 }
 
-static int __init configure_sensor_setup(struct device *dev)
+static int __init asus_ec_probe(struct platform_device *pdev)
 {
-	struct ec_sensors_data *ec_data = dev_get_drvdata(dev);
+	const struct hwmon_channel_info **ptr_asus_ec_ci;
 	int nr_count[hwmon_max] = { 0 }, nr_types = 0;
-	struct device *hwdev;
 	struct hwmon_channel_info *asus_ec_hwmon_chan;
-	const struct hwmon_channel_info **ptr_asus_ec_ci;
 	const struct hwmon_chip_info *chip_info;
 	const struct ec_sensor_info *si;
+	struct ec_sensors_data *ec_data;
 	enum hwmon_sensor_types type;
+	unsigned long board_sensors;
+	struct device *hwdev;
 	unsigned int i;
 
-	ec_data->board_sensors = get_board_sensors(dev);
-	if (!ec_data->board_sensors) {
+	board_sensors = get_board_sensors(&pdev->dev);
+	if (!board_sensors) {
 		return -ENODEV;
 	}
 
+	ec_data = devm_kzalloc(&pdev->dev, sizeof(struct ec_sensors_data),
+			     GFP_KERNEL);
+	if (!ec_data) {
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata(&pdev->dev, ec_data);
+	ec_data->board_sensors = board_sensors;
 	ec_data->nr_sensors = board_sensors_count(ec_data->board_sensors);
-	ec_data->sensors = devm_kcalloc(dev, ec_data->nr_sensors,
+	ec_data->sensors = devm_kcalloc(&pdev->dev, ec_data->nr_sensors,
 					sizeof(struct ec_sensor), GFP_KERNEL);
 
 	setup_sensor_data(ec_data);
-	ec_data->registers = devm_kcalloc(dev, ec_data->nr_registers,
+	ec_data->registers = devm_kcalloc(&pdev->dev, ec_data->nr_registers,
 					  sizeof(u16), GFP_KERNEL);
-	ec_data->read_buffer = devm_kcalloc(dev, ec_data->nr_registers,
+	ec_data->read_buffer = devm_kcalloc(&pdev->dev, ec_data->nr_registers,
 					    sizeof(u8), GFP_KERNEL);
 
 	if (!ec_data->registers || !ec_data->read_buffer) {
@@ -622,7 +631,7 @@ static int __init configure_sensor_setup(struct device *dev)
 
 	fill_ec_registers(ec_data);
 
-	ec_data->aml_mutex = asus_hw_access_mutex(dev);
+	ec_data->aml_mutex = asus_hw_access_mutex(&pdev->dev);
 
 	for (i = 0; i < ec_data->nr_sensors; ++i) {
 		si = get_sensor_info(ec_data, i);
@@ -635,11 +644,11 @@ static int __init configure_sensor_setup(struct device *dev)
 		nr_count[hwmon_chip]++, nr_types++;
 
 	asus_ec_hwmon_chan = devm_kcalloc(
-		dev, nr_types, sizeof(*asus_ec_hwmon_chan), GFP_KERNEL);
+		&pdev->dev, nr_types, sizeof(*asus_ec_hwmon_chan), GFP_KERNEL);
 	if (!asus_ec_hwmon_chan)
 		return -ENOMEM;
 
-	ptr_asus_ec_ci = devm_kcalloc(dev, nr_types + 1,
+	ptr_asus_ec_ci = devm_kcalloc(&pdev->dev, nr_types + 1,
 				       sizeof(*ptr_asus_ec_ci), GFP_KERNEL);
 	if (!ptr_asus_ec_ci)
 		return -ENOMEM;
@@ -651,37 +660,21 @@ static int __init configure_sensor_setup(struct device *dev)
 		if (!nr_count[type])
 			continue;
 
-		asus_ec_hwmon_add_chan_info(asus_ec_hwmon_chan, dev,
+		asus_ec_hwmon_add_chan_info(asus_ec_hwmon_chan, &pdev->dev,
 					     nr_count[type], type,
 					     hwmon_attributes[type]);
 		*ptr_asus_ec_ci++ = asus_ec_hwmon_chan++;
 	}
 
-	dev_info(dev, "board has %d EC sensors that span %d registers",
+	dev_info(&pdev->dev, "board has %d EC sensors that span %d registers",
 		 ec_data->nr_sensors, ec_data->nr_registers);
 
-	hwdev = devm_hwmon_device_register_with_info(dev, "asusec",
+	hwdev = devm_hwmon_device_register_with_info(&pdev->dev, "asusec",
 						     ec_data, chip_info, NULL);
 
 	return PTR_ERR_OR_ZERO(hwdev);
 }
 
-static int __init asus_ec_probe(struct platform_device *pdev)
-{
-	struct ec_sensors_data *state;
-	int status = 0;
-
-	state = devm_kzalloc(&pdev->dev, sizeof(struct ec_sensors_data),
-			     GFP_KERNEL);
-
-	if (!state) {
-		return -ENOMEM;
-	}
-
-	dev_set_drvdata(&pdev->dev, state);
-	status = configure_sensor_setup(&pdev->dev);
-	return status;
-}
 
 static const struct acpi_device_id acpi_ec_ids[] = {
 	/* Embedded Controller Device */
-- 
2.35.1

Re: [PATCH] hwmon: (asus-ec-sensors) merge setup functions

[Guenter Roeck]

On 2/16/22 00:19, Eugene Shalygin wrote:
> Merge configure_sensor_setup() into probe().
> 
> Signed-off-by: Eugene Shalygin <eugene.shalygin@gmail.com>
> ---
>   drivers/hwmon/asus-ec-sensors.c | 55 ++++++++++++++-------------------
>   1 file changed, 24 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
> index bfac08a5dc57..45de8d34a588 100644
> --- a/drivers/hwmon/asus-ec-sensors.c
> +++ b/drivers/hwmon/asus-ec-sensors.c
> @@ -589,31 +589,40 @@ get_board_sensors(const struct device *dev)
>   	return (unsigned long)dmi_entry->driver_data;
>   }
>   
> -static int __init configure_sensor_setup(struct device *dev)
> +static int __init asus_ec_probe(struct platform_device *pdev)
>   {
> -	struct ec_sensors_data *ec_data = dev_get_drvdata(dev);
> +	const struct hwmon_channel_info **ptr_asus_ec_ci;
>   	int nr_count[hwmon_max] = { 0 }, nr_types = 0;
> -	struct device *hwdev;
>   	struct hwmon_channel_info *asus_ec_hwmon_chan;
> -	const struct hwmon_channel_info **ptr_asus_ec_ci;
>   	const struct hwmon_chip_info *chip_info;
>   	const struct ec_sensor_info *si;
> +	struct ec_sensors_data *ec_data;
>   	enum hwmon_sensor_types type;
> +	unsigned long board_sensors;
> +	struct device *hwdev;
>   	unsigned int i;
>   
> -	ec_data->board_sensors = get_board_sensors(dev);
> -	if (!ec_data->board_sensors) {
> +	board_sensors = get_board_sensors(&pdev->dev);
> +	if (!board_sensors) {
>   		return -ENODEV;
>   	}
>   
> +	ec_data = devm_kzalloc(&pdev->dev, sizeof(struct ec_sensors_data),
> +			     GFP_KERNEL);
> +	if (!ec_data) {
> +		return -ENOMEM;
> +	}
> +
> +	dev_set_drvdata(&pdev->dev, ec_data);
> +	ec_data->board_sensors = board_sensors;
>   	ec_data->nr_sensors = board_sensors_count(ec_data->board_sensors);
> -	ec_data->sensors = devm_kcalloc(dev, ec_data->nr_sensors,
> +	ec_data->sensors = devm_kcalloc(&pdev->dev, ec_data->nr_sensors,
>   					sizeof(struct ec_sensor), GFP_KERNEL);

Almost the entire change consists of replacing "dev" with "&pdev->dev".
Please add
	struct device *dev = &pdev->dev;
at the beginning of this function to avoid this.

Guenter

>   
>   	setup_sensor_data(ec_data);
> -	ec_data->registers = devm_kcalloc(dev, ec_data->nr_registers,
> +	ec_data->registers = devm_kcalloc(&pdev->dev, ec_data->nr_registers,
>   					  sizeof(u16), GFP_KERNEL);
> -	ec_data->read_buffer = devm_kcalloc(dev, ec_data->nr_registers,
> +	ec_data->read_buffer = devm_kcalloc(&pdev->dev, ec_data->nr_registers,
>   					    sizeof(u8), GFP_KERNEL);
>   
>   	if (!ec_data->registers || !ec_data->read_buffer) {
> @@ -622,7 +631,7 @@ static int __init configure_sensor_setup(struct device *dev)
>   
>   	fill_ec_registers(ec_data);
>   
> -	ec_data->aml_mutex = asus_hw_access_mutex(dev);
> +	ec_data->aml_mutex = asus_hw_access_mutex(&pdev->dev);
>   
>   	for (i = 0; i < ec_data->nr_sensors; ++i) {
>   		si = get_sensor_info(ec_data, i);
> @@ -635,11 +644,11 @@ static int __init configure_sensor_setup(struct device *dev)
>   		nr_count[hwmon_chip]++, nr_types++;
>   
>   	asus_ec_hwmon_chan = devm_kcalloc(
> -		dev, nr_types, sizeof(*asus_ec_hwmon_chan), GFP_KERNEL);
> +		&pdev->dev, nr_types, sizeof(*asus_ec_hwmon_chan), GFP_KERNEL);
>   	if (!asus_ec_hwmon_chan)
>   		return -ENOMEM;
>   
> -	ptr_asus_ec_ci = devm_kcalloc(dev, nr_types + 1,
> +	ptr_asus_ec_ci = devm_kcalloc(&pdev->dev, nr_types + 1,
>   				       sizeof(*ptr_asus_ec_ci), GFP_KERNEL);
>   	if (!ptr_asus_ec_ci)
>   		return -ENOMEM;
> @@ -651,37 +660,21 @@ static int __init configure_sensor_setup(struct device *dev)
>   		if (!nr_count[type])
>   			continue;
>   
> -		asus_ec_hwmon_add_chan_info(asus_ec_hwmon_chan, dev,
> +		asus_ec_hwmon_add_chan_info(asus_ec_hwmon_chan, &pdev->dev,
>   					     nr_count[type], type,
>   					     hwmon_attributes[type]);
>   		*ptr_asus_ec_ci++ = asus_ec_hwmon_chan++;
>   	}
>   
> -	dev_info(dev, "board has %d EC sensors that span %d registers",
> +	dev_info(&pdev->dev, "board has %d EC sensors that span %d registers",
>   		 ec_data->nr_sensors, ec_data->nr_registers);
>   
> -	hwdev = devm_hwmon_device_register_with_info(dev, "asusec",
> +	hwdev = devm_hwmon_device_register_with_info(&pdev->dev, "asusec",
>   						     ec_data, chip_info, NULL);
>   
>   	return PTR_ERR_OR_ZERO(hwdev);
>   }
>   
> -static int __init asus_ec_probe(struct platform_device *pdev)
> -{
> -	struct ec_sensors_data *state;
> -	int status = 0;
> -
> -	state = devm_kzalloc(&pdev->dev, sizeof(struct ec_sensors_data),
> -			     GFP_KERNEL);
> -
> -	if (!state) {
> -		return -ENOMEM;
> -	}
> -
> -	dev_set_drvdata(&pdev->dev, state);
> -	status = configure_sensor_setup(&pdev->dev);
> -	return status;
> -}
>   
>   static const struct acpi_device_id acpi_ec_ids[] = {
>   	/* Embedded Controller Device */

[PATCH v2] hwmon: (asus-ec-sensors) merge setup functions

[Eugene Shalygin]

Merge configure_sensor_setup() into probe().

Change in v2: add local struct device *dev = &pdev->dev;

Signed-off-by: Eugene Shalygin <eugene.shalygin@gmail.com>
---
 drivers/hwmon/asus-ec-sensors.c | 39 ++++++++++++++-------------------
 1 file changed, 17 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
index bfac08a5dc57..300328d97a0c 100644
--- a/drivers/hwmon/asus-ec-sensors.c
+++ b/drivers/hwmon/asus-ec-sensors.c
@@ -589,23 +589,34 @@ get_board_sensors(const struct device *dev)
 	return (unsigned long)dmi_entry->driver_data;
 }
 
-static int __init configure_sensor_setup(struct device *dev)
+static int __init asus_ec_probe(struct platform_device *pdev)
 {
-	struct ec_sensors_data *ec_data = dev_get_drvdata(dev);
+	const struct hwmon_channel_info **ptr_asus_ec_ci;
 	int nr_count[hwmon_max] = { 0 }, nr_types = 0;
-	struct device *hwdev;
 	struct hwmon_channel_info *asus_ec_hwmon_chan;
-	const struct hwmon_channel_info **ptr_asus_ec_ci;
 	const struct hwmon_chip_info *chip_info;
 	const struct ec_sensor_info *si;
+	struct ec_sensors_data *ec_data;
 	enum hwmon_sensor_types type;
+	unsigned long board_sensors;
+	struct device *hwdev;
+	struct device *dev;
 	unsigned int i;
 
-	ec_data->board_sensors = get_board_sensors(dev);
-	if (!ec_data->board_sensors) {
+	dev = &pdev->dev;
+	board_sensors = get_board_sensors(dev);
+	if (!board_sensors) {
 		return -ENODEV;
 	}
 
+	ec_data = devm_kzalloc(dev, sizeof(struct ec_sensors_data),
+			     GFP_KERNEL);
+	if (!ec_data) {
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata(dev, ec_data);
+	ec_data->board_sensors = board_sensors;
 	ec_data->nr_sensors = board_sensors_count(ec_data->board_sensors);
 	ec_data->sensors = devm_kcalloc(dev, ec_data->nr_sensors,
 					sizeof(struct ec_sensor), GFP_KERNEL);
@@ -666,22 +677,6 @@ static int __init configure_sensor_setup(struct device *dev)
 	return PTR_ERR_OR_ZERO(hwdev);
 }
 
-static int __init asus_ec_probe(struct platform_device *pdev)
-{
-	struct ec_sensors_data *state;
-	int status = 0;
-
-	state = devm_kzalloc(&pdev->dev, sizeof(struct ec_sensors_data),
-			     GFP_KERNEL);
-
-	if (!state) {
-		return -ENOMEM;
-	}
-
-	dev_set_drvdata(&pdev->dev, state);
-	status = configure_sensor_setup(&pdev->dev);
-	return status;
-}
 
 static const struct acpi_device_id acpi_ec_ids[] = {
 	/* Embedded Controller Device */
-- 
2.35.1

[PATCH v3] hwmon: (asus-ec-sensors) merge setup functions

[Eugene Shalygin]

Merge configure_sensor_setup() into probe().

Changes:
 - v2: add local struct device *dev = &pdev->dev;
 - v3: initialize dev at declaration

Signed-off-by: Eugene Shalygin <eugene.shalygin@gmail.com>
---
 drivers/hwmon/asus-ec-sensors.c | 38 ++++++++++++++-------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
index bfac08a5dc57..b9eb9126e433 100644
--- a/drivers/hwmon/asus-ec-sensors.c
+++ b/drivers/hwmon/asus-ec-sensors.c
@@ -589,23 +589,33 @@ get_board_sensors(const struct device *dev)
 	return (unsigned long)dmi_entry->driver_data;
 }
 
-static int __init configure_sensor_setup(struct device *dev)
+static int __init asus_ec_probe(struct platform_device *pdev)
 {
-	struct ec_sensors_data *ec_data = dev_get_drvdata(dev);
+	const struct hwmon_channel_info **ptr_asus_ec_ci;
 	int nr_count[hwmon_max] = { 0 }, nr_types = 0;
-	struct device *hwdev;
 	struct hwmon_channel_info *asus_ec_hwmon_chan;
-	const struct hwmon_channel_info **ptr_asus_ec_ci;
 	const struct hwmon_chip_info *chip_info;
 	const struct ec_sensor_info *si;
+	struct ec_sensors_data *ec_data;
 	enum hwmon_sensor_types type;
+	unsigned long board_sensors;
+	struct device *hwdev;
 	unsigned int i;
 
-	ec_data->board_sensors = get_board_sensors(dev);
-	if (!ec_data->board_sensors) {
+	struct device *dev = &pdev->dev;
+	board_sensors = get_board_sensors(dev);
+	if (!board_sensors) {
 		return -ENODEV;
 	}
 
+	ec_data = devm_kzalloc(dev, sizeof(struct ec_sensors_data),
+			     GFP_KERNEL);
+	if (!ec_data) {
+		return -ENOMEM;
+	}
+
+	dev_set_drvdata(dev, ec_data);
+	ec_data->board_sensors = board_sensors;
 	ec_data->nr_sensors = board_sensors_count(ec_data->board_sensors);
 	ec_data->sensors = devm_kcalloc(dev, ec_data->nr_sensors,
 					sizeof(struct ec_sensor), GFP_KERNEL);
@@ -666,22 +676,6 @@ static int __init configure_sensor_setup(struct device *dev)
 	return PTR_ERR_OR_ZERO(hwdev);
 }
 
-static int __init asus_ec_probe(struct platform_device *pdev)
-{
-	struct ec_sensors_data *state;
-	int status = 0;
-
-	state = devm_kzalloc(&pdev->dev, sizeof(struct ec_sensors_data),
-			     GFP_KERNEL);
-
-	if (!state) {
-		return -ENOMEM;
-	}
-
-	dev_set_drvdata(&pdev->dev, state);
-	status = configure_sensor_setup(&pdev->dev);
-	return status;
-}
 
 static const struct acpi_device_id acpi_ec_ids[] = {
 	/* Embedded Controller Device */
-- 
2.35.1

Re: [PATCH v3] hwmon: (asus-ec-sensors) merge setup functions

[Guenter Roeck]

On 2/16/22 08:01, Eugene Shalygin wrote:
> Merge configure_sensor_setup() into probe().
> 
> Changes:
>   - v2: add local struct device *dev = &pdev->dev;
>   - v3: initialize dev at declaration
> 
> Signed-off-by: Eugene Shalygin <eugene.shalygin@gmail.com>


Please use checkpatch and fix what it reports,, and please
do not send patches as reply to previous versions of a patch
or patch series.

> ---
>   drivers/hwmon/asus-ec-sensors.c | 38 ++++++++++++++-------------------
>   1 file changed, 16 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/hwmon/asus-ec-sensors.c b/drivers/hwmon/asus-ec-sensors.c
> index bfac08a5dc57..b9eb9126e433 100644
> --- a/drivers/hwmon/asus-ec-sensors.c
> +++ b/drivers/hwmon/asus-ec-sensors.c
> @@ -589,23 +589,33 @@ get_board_sensors(const struct device *dev)
>   	return (unsigned long)dmi_entry->driver_data;
>   }
>   
> -static int __init configure_sensor_setup(struct device *dev)
> +static int __init asus_ec_probe(struct platform_device *pdev)
>   {
> -	struct ec_sensors_data *ec_data = dev_get_drvdata(dev);
> +	const struct hwmon_channel_info **ptr_asus_ec_ci;
>   	int nr_count[hwmon_max] = { 0 }, nr_types = 0;
> -	struct device *hwdev;
>   	struct hwmon_channel_info *asus_ec_hwmon_chan;
> -	const struct hwmon_channel_info **ptr_asus_ec_ci;
>   	const struct hwmon_chip_info *chip_info;
>   	const struct ec_sensor_info *si;
> +	struct ec_sensors_data *ec_data;
>   	enum hwmon_sensor_types type;
> +	unsigned long board_sensors;
> +	struct device *hwdev;
>   	unsigned int i;

I don't really see the point of reordering (most of) the variables
above, only to ...

>   
> -	ec_data->board_sensors = get_board_sensors(dev);
> -	if (!ec_data->board_sensors) {
> +	struct device *dev = &pdev->dev;

... add another variable at the and, after an empty line,
with no following empty line, and completely out of order with
the other variable declarations (if reverse christmas tree order
was the idea).

> +	board_sensors = get_board_sensors(dev);
> +	if (!board_sensors) {
>   		return -ENODEV;
>   	}
>   
> +	ec_data = devm_kzalloc(dev, sizeof(struct ec_sensors_data),
> +			     GFP_KERNEL);
> +	if (!ec_data) {
> +		return -ENOMEM;
> +	}
> +
> +	dev_set_drvdata(dev, ec_data);
> +	ec_data->board_sensors = board_sensors;
>   	ec_data->nr_sensors = board_sensors_count(ec_data->board_sensors);
>   	ec_data->sensors = devm_kcalloc(dev, ec_data->nr_sensors,
>   					sizeof(struct ec_sensor), GFP_KERNEL);
> @@ -666,22 +676,6 @@ static int __init configure_sensor_setup(struct device *dev)
>   	return PTR_ERR_OR_ZERO(hwdev);
>   }
>   
> -static int __init asus_ec_probe(struct platform_device *pdev)
> -{
> -	struct ec_sensors_data *state;
> -	int status = 0;
> -
> -	state = devm_kzalloc(&pdev->dev, sizeof(struct ec_sensors_data),
> -			     GFP_KERNEL);
> -
> -	if (!state) {
> -		return -ENOMEM;
> -	}
> -
> -	dev_set_drvdata(&pdev->dev, state);
> -	status = configure_sensor_setup(&pdev->dev);
> -	return status;
> -}
>   
>   static const struct acpi_device_id acpi_ec_ids[] = {
>   	/* Embedded Controller Device */

Re: [PATCH v3] hwmon: (asus-ec-sensors) merge setup functions

[Eugene Shalygin]

> Please use checkpatch and fix what it reports,, and please
> do not send patches as reply to previous versions of a patch
> or patch series.

Sorry, I don't really understand what are the preferences for the case
when some of the local variables are declared with initializers, and
yes, I was told once to use the reverse christmas tree arrangement.
Hope this time it's OK.

Regards,
Eugene