| Message ID | 20220216160128.2418025-1-eugene.shalygin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v3] hwmon: (asus-ec-sensors) merge setup functions | expand |
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 */
> 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
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 */
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(-)