Messages in this thread Patch in this message | | | Date | Thu, 13 Feb 2014 09:58:17 +0000 | From | Lee Jones <> | Subject | Re: [RFC PATCH] hwmon: (max6650) Convert to be a platform driver |
| |
> 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@kde.org> > --- > This patch has been compile tested only and will be tested with real hardware, > but early reviews to catch any trivial issues would be welcome. > drivers/hwmon/Kconfig | 2 +- > drivers/hwmon/max6650.c | 155 ++++++++++++++++++++++++------------------------ > 2 files changed, 79 insertions(+), 78 deletions(-)
<snip>
> /* > * Insmod parameters > @@ -105,24 +108,23 @@ module_param(clock, int, S_IRUGO); > > #define DIV_FROM_REG(reg) (1 << (reg & 7)) > > -static int max6650_probe(struct i2c_client *client, > - const struct i2c_device_id *id); > -static int max6650_init_client(struct i2c_client *client); > -static int max6650_remove(struct i2c_client *client); > +static int max6650_probe(struct platform_device *pdev); > +static int max6650_init_client(struct platform_device *pdev); > +static int max6650_remove(struct platform_device *pdev); > static struct max6650_data *max6650_update_device(struct device *dev);
It would be good to remove these forward declarations in the future.
If no one volunteers I'll happily do it.
> /* > * Driver data (common to all clients) > */ > > -static const struct i2c_device_id max6650_id[] = { > +static const struct platform_device_id max6650_id[] = { > { "max6650", 1 }, > { "max6651", 4 }, > { } > }; > -MODULE_DEVICE_TABLE(i2c, max6650_id); > +MODULE_DEVICE_TABLE(platform, max6650_id);
I thought you were going to do the matching in the MFD driver?
If not, what's the point in the exported 'type' attribute?
> -static struct i2c_driver max6650_driver = { > +static struct platform_driver max6650_driver = { > .driver = { > .name = "max6650",
This should be changed as it now supports max665{0,1} right?
<snip>
> - i2c_smbus_write_byte_data(client, MAX6650_REG_SPEED, data->speed); > + regmap_write(data->iodev->map, MAX6650_REG_SPEED, data->speed);
Ensure all of the regmap stuff is fully tested.
<snip>
> @@ -484,10 +482,12 @@ static umode_t max6650_attrs_visible(struct kobject *kobj, struct attribute *a, > int n) > { > struct device *dev = container_of(kobj, struct device, kobj); > - struct i2c_client *client = to_i2c_client(dev); > - u8 alarm_en = i2c_smbus_read_byte_data(client, MAX6650_REG_ALARM_EN); > + struct max6650_data *data = dev_get_drvdata(dev); > + int alarm_en; > struct device_attribute *devattr; > > + regmap_read(data->iodev->map, MAX6650_REG_ALARM_EN, &alarm_en); > +
Where is this then used?
> -static int max6650_probe(struct i2c_client *client, > - const struct i2c_device_id *id) > +static int max6650_probe(struct platform_device *pdev) > { > + struct max665x_dev *max665x = dev_get_drvdata(pdev->dev.parent); > struct max6650_data *data; > + const struct platform_device_id *id = platform_get_device_id(pdev);
What's the point in 'type' in the global container?
It's looking as though you're not going to need it to be global after all, right?
<snip>
-- Lee Jones Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |