> The MFD driver has now been added, so this driver is now being adopted to be a
> subdevice driver on top of it. This means, the i2c driver usage is being
> converted to platform driver usage all around.
>
> Signed-off-by: Laszlo Papp <lpapp@xxxxxxx>
> ---
> drivers/hwmon/Kconfig | 2 +-
> drivers/hwmon/max6650.c | 125 ++++++++++++++++++++++++------------------------
> 2 files changed, 63 insertions(+), 64 deletions(-)
>
<snip>
> @@ -39,6 +39,9 @@
> #include <linux/hwmon.h>
> #include <linux/hwmon-sysfs.h>
> #include <linux/err.h>
> +#include <linux/platform_device.h>
> +
Not really any need for this.
> +#include <linux/mfd/max665x-private.h>
<snip>
> struct max6650_data {
> struct i2c_client *client;
> const struct attribute_group *groups[3];
> + struct max665x_dev *iodev;
I find the name iodev a bit confusing, why not max665x_dev?
<snip>
> - data->speed = i2c_smbus_read_byte_data(client,
> - MAX6650_REG_SPEED);
> - data->config = i2c_smbus_read_byte_data(client,
> - MAX6650_REG_CONFIG);
> + regmap_read(map, MAX6650_REG_SPEED, &data->speed);
> + regmap_read(map, MAX6650_REG_CONFIG, &data->config);
I'd like Mark to look over your Regmap implementation if possible.
> if (config < 0) {
> - dev_err(dev, "Error reading config, aborting.\n");
> + dev_err(&pdev->dev, "Error reading config, aborting.\n");
Rather than make all these changes, just do:
struct device *dev = &pdev->dev;
... at the top.
<snip>
> +static int max6650_probe(struct platform_device *pdev)
> {
> - struct device *dev = &client->dev;
> - struct max6650_data *data;
> + struct max6650_data *data = platform_get_drvdata(pdev);
Don't do this here, it's messy. Declare it here and initialise it later.
> + const struct platform_device_id *id = platform_get_device_id(pdev);
Same here.
> struct device *hwmon_dev;
> int err;
>
> - data = devm_kzalloc(dev, sizeof(struct max6650_data), GFP_KERNEL);
> + data = devm_kzalloc(&pdev->dev, sizeof(struct max6650_data),
> + GFP_KERNEL);
Eh? didn't you already initialise 'data'?
<snip>
> - hwmon_dev = devm_hwmon_device_register_with_groups(dev,
> - client->name, data,
> - data->groups);
> + hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev,
> + data->client->name,
> + data, data->groups);
Leave it as 'dev'.
> -static const struct i2c_device_id max6650_id[] = {
> +static const struct platform_device_id max6650_id[] = {
> { "max6650", 1 },
> { "max6651", 4 },
I'm still pretty keen to have these magic numbers #defined.
Not yet though, let's get this sorted first.
> +static struct platform_driver max6650_driver = {
> .driver = {
> .name = "max6650",
Guenter, Jean,
Can this be changed now it's a platform device?
Laszio,
Next thing to do is to get this working and runtime test it. No more
mid-term reviews until it's fully functional.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
