Hi, On Wed, Jul 19, 2017 at 12:04:07PM -0500, Andrew F. Davis wrote: > When the BQ27xxx driver was originally written the w1 subsystem only > allowed device drivers for w1 attached devices to live in the w1 > subsystem. Kernel driver subsystems expect that the driver for a device > live in the directory of the subsystem for which it implements > functionality, not in the directory of the bus that it is attached. To > work around this, the BQ27xxx driver was implemented as a platform device > driver and the interface driver would instantiate this device from within > the w1 directory, then pass a w1 read callback as platform data. > > As we can now have the w1 interface driver in the power/supply directory > (like we do already with the i2c interface driver) we can remove this > middle-layer platform driver. > > Signed-off-by: Andrew F. Davis <a...@ti.com> > Acked-by: Pali Rohár <pali.ro...@gmail.com> > Acked-by: Sebastian Reichel <s...@kernel.org>
Thanks, queued to power-supply's for-next branch. -- Sebastian > drivers/power/supply/bq27xxx_battery.c | 104 ------------ > drivers/power/supply/bq27xxx_battery_hdq.c | 250 > ++++++++++++++++------------- > include/linux/power/bq27xxx_battery.h | 17 -- > 3 files changed, 135 insertions(+), 236 deletions(-) > rewrite drivers/power/supply/bq27xxx_battery_hdq.c (72%) > > diff --git a/drivers/power/supply/bq27xxx_battery.c > b/drivers/power/supply/bq27xxx_battery.c > index ed44439d0112..079083b31293 100644 > --- a/drivers/power/supply/bq27xxx_battery.c > +++ b/drivers/power/supply/bq27xxx_battery.c > @@ -1938,110 +1938,6 @@ void bq27xxx_battery_teardown(struct > bq27xxx_device_info *di) > } > EXPORT_SYMBOL_GPL(bq27xxx_battery_teardown); > > -static int bq27xxx_battery_platform_read(struct bq27xxx_device_info *di, u8 > reg, > - bool single) > -{ > - struct device *dev = di->dev; > - struct bq27xxx_platform_data *pdata = dev->platform_data; > - unsigned int timeout = 3; > - int upper, lower; > - int temp; > - > - if (!single) { > - /* Make sure the value has not changed in between reading the > - * lower and the upper part */ > - upper = pdata->read(dev, reg + 1); > - do { > - temp = upper; > - if (upper < 0) > - return upper; > - > - lower = pdata->read(dev, reg); > - if (lower < 0) > - return lower; > - > - upper = pdata->read(dev, reg + 1); > - } while (temp != upper && --timeout); > - > - if (timeout == 0) > - return -EIO; > - > - return (upper << 8) | lower; > - } > - > - return pdata->read(dev, reg); > -} > - > -static int bq27xxx_battery_platform_probe(struct platform_device *pdev) > -{ > - struct bq27xxx_device_info *di; > - struct bq27xxx_platform_data *pdata = pdev->dev.platform_data; > - > - if (!pdata) { > - dev_err(&pdev->dev, "no platform_data supplied\n"); > - return -EINVAL; > - } > - > - if (!pdata->read) { > - dev_err(&pdev->dev, "no hdq read callback supplied\n"); > - return -EINVAL; > - } > - > - if (!pdata->chip) { > - dev_err(&pdev->dev, "no device supplied\n"); > - return -EINVAL; > - } > - > - di = devm_kzalloc(&pdev->dev, sizeof(*di), GFP_KERNEL); > - if (!di) > - return -ENOMEM; > - > - platform_set_drvdata(pdev, di); > - > - di->dev = &pdev->dev; > - di->chip = pdata->chip; > - di->name = pdata->name ?: dev_name(&pdev->dev); > - di->bus.read = bq27xxx_battery_platform_read; > - > - return bq27xxx_battery_setup(di); > -} > - > -static int bq27xxx_battery_platform_remove(struct platform_device *pdev) > -{ > - struct bq27xxx_device_info *di = platform_get_drvdata(pdev); > - > - bq27xxx_battery_teardown(di); > - > - return 0; > -} > - > -static const struct platform_device_id bq27xxx_battery_platform_id_table[] = > { > - { "bq27000-battery", }, > - { /* sentinel */ } > -}; > -MODULE_DEVICE_TABLE(platform, bq27xxx_battery_platform_id_table); > - > -#ifdef CONFIG_OF > -static const struct of_device_id bq27xxx_battery_platform_of_match_table[] = > { > - { .compatible = "ti,bq27000" }, > - {}, > -}; > -MODULE_DEVICE_TABLE(of, bq27xxx_battery_platform_of_match_table); > -#endif > - > -static struct platform_driver bq27xxx_battery_platform_driver = { > - .probe = bq27xxx_battery_platform_probe, > - .remove = bq27xxx_battery_platform_remove, > - .driver = { > - .name = "bq27000-battery", > - .of_match_table = > of_match_ptr(bq27xxx_battery_platform_of_match_table), > - }, > - .id_table = bq27xxx_battery_platform_id_table, > -}; > -module_platform_driver(bq27xxx_battery_platform_driver); > - > -MODULE_ALIAS("platform:bq27000-battery"); > - > MODULE_AUTHOR("Rodolfo Giometti <giome...@linux.it>"); > MODULE_DESCRIPTION("BQ27xxx battery monitor driver"); > MODULE_LICENSE("GPL"); > diff --git a/drivers/power/supply/bq27xxx_battery_hdq.c > b/drivers/power/supply/bq27xxx_battery_hdq.c > dissimilarity index 72% > index f4df67eb9d2c..9aff896c9802 100644 > --- a/drivers/power/supply/bq27xxx_battery_hdq.c > +++ b/drivers/power/supply/bq27xxx_battery_hdq.c > @@ -1,115 +1,135 @@ > -/* > - * Copyright (C) 2007 Texas Instruments, Inc. > - * > - * This file is licensed under the terms of the GNU General Public License > - * version 2. This program is licensed "as is" without any warranty of any > - * kind, whether express or implied. > - * > - */ > - > -#include <linux/kernel.h> > -#include <linux/module.h> > -#include <linux/device.h> > -#include <linux/types.h> > -#include <linux/platform_device.h> > -#include <linux/mutex.h> > -#include <linux/power/bq27xxx_battery.h> > - > -#include <linux/w1.h> > - > -#define W1_FAMILY_BQ27000 0x01 > - > -#define HDQ_CMD_READ (0) > -#define HDQ_CMD_WRITE (1<<7) > - > -static int F_ID; > -module_param(F_ID, int, S_IRUSR); > -MODULE_PARM_DESC(F_ID, "1-wire slave FID for BQ device"); > - > -static int w1_bq27000_read(struct device *dev, unsigned int reg) > -{ > - u8 val; > - struct w1_slave *sl = container_of(dev->parent, struct w1_slave, dev); > - > - mutex_lock(&sl->master->bus_mutex); > - w1_write_8(sl->master, HDQ_CMD_READ | reg); > - val = w1_read_8(sl->master); > - mutex_unlock(&sl->master->bus_mutex); > - > - return val; > -} > - > -static struct bq27xxx_platform_data bq27000_battery_info = { > - .read = w1_bq27000_read, > - .name = "bq27000-battery", > - .chip = BQ27000, > -}; > - > -static int w1_bq27000_add_slave(struct w1_slave *sl) > -{ > - int ret; > - struct platform_device *pdev; > - > - pdev = platform_device_alloc("bq27000-battery", -1); > - if (!pdev) { > - ret = -ENOMEM; > - return ret; > - } > - ret = platform_device_add_data(pdev, > - &bq27000_battery_info, > - sizeof(bq27000_battery_info)); > - if (ret) > - goto pdev_add_failed; > - pdev->dev.parent = &sl->dev; > - > - ret = platform_device_add(pdev); > - if (ret) > - goto pdev_add_failed; > - > - dev_set_drvdata(&sl->dev, pdev); > - > - goto success; > - > -pdev_add_failed: > - platform_device_put(pdev); > -success: > - return ret; > -} > - > -static void w1_bq27000_remove_slave(struct w1_slave *sl) > -{ > - struct platform_device *pdev = dev_get_drvdata(&sl->dev); > - > - platform_device_unregister(pdev); > -} > - > -static struct w1_family_ops w1_bq27000_fops = { > - .add_slave = w1_bq27000_add_slave, > - .remove_slave = w1_bq27000_remove_slave, > -}; > - > -static struct w1_family w1_bq27000_family = { > - .fid = W1_FAMILY_BQ27000, > - .fops = &w1_bq27000_fops, > -}; > - > -static int __init w1_bq27000_init(void) > -{ > - if (F_ID) > - w1_bq27000_family.fid = F_ID; > - > - return w1_register_family(&w1_bq27000_family); > -} > - > -static void __exit w1_bq27000_exit(void) > -{ > - w1_unregister_family(&w1_bq27000_family); > -} > - > -module_init(w1_bq27000_init); > -module_exit(w1_bq27000_exit); > - > -MODULE_AUTHOR("Texas Instruments Ltd"); > -MODULE_DESCRIPTION("HDQ/1-wire slave driver bq27000 battery monitor chip"); > -MODULE_LICENSE("GPL"); > -MODULE_ALIAS("w1-family-" __stringify(W1_FAMILY_BQ27000)); > +/* > + * BQ27xxx battery monitor HDQ/1-wire driver > + * > + * Copyright (C) 2007-2017 Texas Instruments Incorporated - > http://www.ti.com/ > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + * > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any > + * kind, whether express or implied; without even the implied warranty > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/device.h> > +#include <linux/types.h> > +#include <linux/platform_device.h> > +#include <linux/mutex.h> > +#include <linux/power/bq27xxx_battery.h> > + > +#include <linux/w1.h> > + > +#define W1_FAMILY_BQ27000 0x01 > + > +#define HDQ_CMD_READ (0 << 7) > +#define HDQ_CMD_WRITE (1 << 7) > + > +static int F_ID; > +module_param(F_ID, int, S_IRUSR); > +MODULE_PARM_DESC(F_ID, "1-wire slave FID for BQ27xxx device"); > + > +static int w1_bq27000_read(struct w1_slave *sl, unsigned int reg) > +{ > + u8 val; > + > + mutex_lock(&sl->master->bus_mutex); > + w1_write_8(sl->master, HDQ_CMD_READ | reg); > + val = w1_read_8(sl->master); > + mutex_unlock(&sl->master->bus_mutex); > + > + return val; > +} > + > +static int bq27xxx_battery_hdq_read(struct bq27xxx_device_info *di, u8 reg, > + bool single) > +{ > + struct w1_slave *sl = dev_to_w1_slave(di->dev); > + unsigned int timeout = 3; > + int upper, lower; > + int temp; > + > + if (!single) { > + /* > + * Make sure the value has not changed in between reading the > + * lower and the upper part > + */ > + upper = w1_bq27000_read(sl, reg + 1); > + do { > + temp = upper; > + if (upper < 0) > + return upper; > + > + lower = w1_bq27000_read(sl, reg); > + if (lower < 0) > + return lower; > + > + upper = w1_bq27000_read(sl, reg + 1); > + } while (temp != upper && --timeout); > + > + if (timeout == 0) > + return -EIO; > + > + return (upper << 8) | lower; > + } > + > + return w1_bq27000_read(sl, reg); > +} > + > +static int bq27xxx_battery_hdq_add_slave(struct w1_slave *sl) > +{ > + struct bq27xxx_device_info *di; > + > + di = devm_kzalloc(&sl->dev, sizeof(*di), GFP_KERNEL); > + if (!di) > + return -ENOMEM; > + > + dev_set_drvdata(&sl->dev, di); > + > + di->dev = &sl->dev; > + di->chip = BQ27000; > + di->name = "bq27000-battery"; > + di->bus.read = bq27xxx_battery_hdq_read; > + > + return bq27xxx_battery_setup(di); > +} > + > +static void bq27xxx_battery_hdq_remove_slave(struct w1_slave *sl) > +{ > + struct bq27xxx_device_info *di = dev_get_drvdata(&sl->dev); > + > + bq27xxx_battery_teardown(di); > +} > + > +static struct w1_family_ops bq27xxx_battery_hdq_fops = { > + .add_slave = bq27xxx_battery_hdq_add_slave, > + .remove_slave = bq27xxx_battery_hdq_remove_slave, > +}; > + > +static struct w1_family bq27xxx_battery_hdq_family = { > + .fid = W1_FAMILY_BQ27000, > + .fops = &bq27xxx_battery_hdq_fops, > +}; > + > +static int __init bq27xxx_battery_hdq_init(void) > +{ > + if (F_ID) > + bq27xxx_battery_hdq_family.fid = F_ID; > + > + return w1_register_family(&bq27xxx_battery_hdq_family); > +} > +module_init(bq27xxx_battery_hdq_init); > + > +static void __exit bq27xxx_battery_hdq_exit(void) > +{ > + w1_unregister_family(&bq27xxx_battery_hdq_family); > +} > +module_exit(bq27xxx_battery_hdq_exit); > + > +MODULE_AUTHOR("Texas Instruments Ltd"); > +MODULE_DESCRIPTION("BQ27xxx battery monitor HDQ/1-wire driver"); > +MODULE_LICENSE("GPL"); > +MODULE_ALIAS("w1-family-" __stringify(W1_FAMILY_BQ27000)); > diff --git a/include/linux/power/bq27xxx_battery.h > b/include/linux/power/bq27xxx_battery.h > index 11e11685dd1d..89890437f9ab 100644 > --- a/include/linux/power/bq27xxx_battery.h > +++ b/include/linux/power/bq27xxx_battery.h > @@ -20,23 +20,6 @@ enum bq27xxx_chip { > BQ27421, /* bq27421, bq27425, bq27441, bq27621 */ > }; > > -/** > - * struct bq27xxx_plaform_data - Platform data for bq27xxx devices > - * @name: Name of the battery. > - * @chip: Chip class number of this device. > - * @read: HDQ read callback. > - * This function should provide access to the HDQ bus the battery is > - * connected to. > - * The first parameter is a pointer to the battery device, the second the > - * register to be read. The return value should either be the content of > - * the passed register or an error value. > - */ > -struct bq27xxx_platform_data { > - const char *name; > - enum bq27xxx_chip chip; > - int (*read)(struct device *dev, unsigned int); > -}; > - > struct bq27xxx_device_info; > struct bq27xxx_access_methods { > int (*read)(struct bq27xxx_device_info *di, u8 reg, bool single); > -- > 2.13.0 >
signature.asc
Description: PGP signature