Add I2C driver for Summit Microelectronics SMB347 Battery Charger.
PATCH 1/1 Add I2C driver for Summit Microelectronics SMB347 Battery Charger.
From: Bruce E. Robertson <[email protected]>
Driver support for the Summit I²C battery charger. This is used in some Intel devices.
---
drivers/power/Kconfig | 7 drivers/power/Makefile | 1 drivers/power/smb347-charger.c | 1294 ++++++++++++++++++++++++++++++++++ include/linux/power/smb347-charger.h | 117 +++ 4 files changed, 1420 insertions(+), 0 deletions(-) create mode 100644 drivers/power/smb347-charger.c create mode 100644 include/linux/power/smb347-charger.h
drivers/power/Kconfig +7 -0
@@ -274,4 +274,11 @@ config CHARGER_MAX8998 Say Y to enable support for the battery charger control sysfs and platform data of MAX8998/LP3974 PMICs. +config CHARGER_SMB347 + tristate "Summit Microelectronics SMB347 Battery Charger" + depends on I2C + help + Say Y to include support for Summit Microelectronics SMB347 + Battery Charger. + endif # POWER_SUPPLY
drivers/power/Makefile +1 -0
@@ -41,3 +41,4 @@ obj-$(CONFIG_CHARGER_GPIO) += gpio-charger.o obj-$(CONFIG_CHARGER_MANAGER) += charger-manager.o obj-$(CONFIG_CHARGER_MAX8997) += max8997_charger.o obj-$(CONFIG_CHARGER_MAX8998) += max8998_charger.o +obj-$(CONFIG_CHARGER_SMB347) += smb347-charger.o
drivers/power/smb347-charger.c new file +1294 -0
1,294 lines not on this page · raw message
include/linux/power/smb347-charger.h new file +117 -0
117 lines not on this page · raw message
Quoted 2 lines · Alan Cox
Driver support for the Summit I??C battery charger. This is used in some Intel devices.
There's quite a lot of read/modify/write cycles and...
drivers/power/smb347-charger.c line 1111
Quoted 8 lines
drivers/power/smb347-charger.c new file +8
-0+static int smb347_debugfs_show (struct seq_file *s,void *data) +{ +struct smb347_charger *smb = s->private; +int ret; +u8 reg; + +seq_printf (s,"Control registers:\n" ); +seq_printf (s,"==================\n" );
...this which make me wonder if this might not benefit from using regmap. The read/modify/write cycles are handled by regmap_update_bits() and there's a standard debugfs file for dumping registers. Should save a bit of code I think but I'd not worry too much either way.
drivers/power/smb347-charger.c line 1288
Quoted 11 lines
drivers/power/smb347-charger.c new file +11
-0+static int __init smb347_init (void ) +{ +return i2c_add_driver (&smb347_driver); +} +module_init (smb347_init); + +static void __exit smb347_exit(void) +{ +i2c_del_driver (&smb347_driver); +} +module_exit (smb347_exit);
There's module_i2c_driver() (and similarly for SPI and platform) though that's definitely a nitpick.
Quoted 6 lines · On Mon, 6 Feb 2012 16:01:27 +0000 Mark Brown…
Quoted 2 lines · Alan Cox
Driver support for the Summit I??C battery charger. This is used in some Intel devices.
There's quite a lot of read/modify/write cycles and...
True but they are basically all in the setup side of things so I'm not sure the complexity is justified or appropriate ?
Agreed on module_i2c_driver() - I'll do a follow on patch to switch it to that.
Alan
Quoted 6 lines · Alan Cox
Quoted 1 line · Mark Brown
There's quite a lot of read/modify/write cycles and...
True but they are basically all in the setup side of things so I'm not sure the complexity is justified or appropriate ?
I'm not sure what complexity you mean? If the interface is too difficult to use we should fix that but I didn't notice anything in the driver which should be more than a straight substitution (or deletion of code in the case of the debugfs file).
Quoted 5 lines · On Mon, 6 Feb 2012 16:35:45 +0000 Mark Brown…
Quoted 3 lines · Alan Cox
Quoted 1 line · Mark Brown
There's quite a lot of read/modify/write cycles and...
Quoted 7 lines
Quoted 2 lines
True but they are basically all in the setup side of things so I'm not sure the complexity is justified or appropriate ?
I'm not sure what complexity you mean? If the interface is too difficult to use we should fix that but I didn't notice anything in the driver which should be more than a straight substitution (or deletion of code in the case of the debugfs file).
The API seems fine to me but you've then got to implement the writeable/readable/volatile reg callbacks and figure out what is cachable for the chip and specify it.
Seems like overkill for a chip you poke asynchronously during boot and then basically let it mind its own business.
Alan
Quoted 10 lines · Alan Cox
Quoted 4 lines · Mark Brown
I'm not sure what complexity you mean? If the interface is too difficult to use we should fix that but I didn't notice anything in the driver which should be more than a straight substitution (or deletion of code in the case of the debugfs file).
The API seems fine to me but you've then got to implement the writeable/readable/volatile reg callbacks and figure out what is cachable for the chip and specify it.
All those callbacks are optional, you can tell the API what the maximum register is with max_register and it should just show all registers (which is pretty much what you have there) - you only need max_register so that debugfs does something sensible, though thinking about it we can probably just assume the maximum possible register value for devices that don't specify it and have something sensible happen (at least for smaller address sizes, I'm not sure we want 65535 registers shown by default for 16 bit address devices).
The cache usage is totally optional too, it's available if you want it but it won't make sense for all devices and is disabled by default.
Quoted 21 lines · Mark Brown
Quoted 2 lines · Alan Cox
Driver support for the Summit I??C battery charger. This is used in some Intel devices.
There's quite a lot of read/modify/write cycles and...
drivers/power/smb347-charger.c line 1111
Quoted 8 lines
drivers/power/smb347-charger.c new file +8
-0+static int smb347_debugfs_show (struct seq_file *s,void *data) +{ +struct smb347_charger *smb = s->private; +int ret; +u8 reg; + +seq_printf (s,"Control registers:\n" ); +seq_printf (s,"==================\n" );...this which make me wonder if this might not benefit from using regmap. The read/modify/write cycles are handled by
regmap_update_bits()and there's a standard debugfs file for dumping registers. Should save a bit of code I think but I'd not worry too much either way.
Quoted 14 lines
drivers/power/smb347-charger.c line 1288
Quoted 11 lines
drivers/power/smb347-charger.c new file +11
-0+static int __init smb347_init (void ) +{ +return i2c_add_driver (&smb347_driver); +} +module_init (smb347_init); + +static void __exit smb347_exit(void) +{ +i2c_del_driver (&smb347_driver); +} +module_exit (smb347_exit);There's
module_i2c_driver()(and similarly for SPI and platform) though that's definitely a nitpick.
I agree with all these comments. There wasn't any update on this patch though. But it compiles and hopefully works, so I'm applying it w/ some lingering hope that it'll receive attention from the submitter. :-)
Thanks!
Signature · 2 lines
Anton Vorontsov Email: [email protected]