* [PATCH 0/2] Add support for Smart Battery System Manager @ 2016-06-22 19:07 Karl-Heinz Schneider 2016-06-22 19:07 ` [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation Karl-Heinz Schneider 2016-06-22 19:07 ` [PATCH 2/2] power: Adds support for Smart Battery System Manager Karl-Heinz Schneider 0 siblings, 2 replies; 13+ messages in thread From: Karl-Heinz Schneider @ 2016-06-22 19:07 UTC (permalink / raw) To: devicetree, linux-pm, linux-acpi, linux-i2c Cc: Rob Herring, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Peter Rosin, Phil Reid, Karl-Heinz Schneider This patch series adds support for Smart Battery System Manager. A SBSM is a device listening at I2C/SMBus address 0x0a and is capable of communicating up to four I2C smart battery devices. All smart battery devices are listening at address 0x0b, so the SBSM muliplexes between them. The driver makes use of the I2C-Mux framework to allow smart batteries to be bound via device tree, i.e. the sbs-battery driver. Via sysfs interface the online state and charge type are presented. If the driver is bound as ltc1760 (a Dual Smart Battery System Manager) the charge type can also be changed from trickle to fast. Patch set generated against v4.7rc-4. Karl-Heinz Schneider (2): Documentation: Add sbs-manager device tree node documentation power: Adds support for Smart Battery System Manager .../devicetree/bindings/power/sbs,sbs-manager.txt | 57 ++++ drivers/power/Kconfig | 13 + drivers/power/Makefile | 1 + drivers/power/sbs-manager.c | 346 +++++++++++++++++++++ 4 files changed, 417 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt create mode 100644 drivers/power/sbs-manager.c -- 1.9.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-22 19:07 [PATCH 0/2] Add support for Smart Battery System Manager Karl-Heinz Schneider @ 2016-06-22 19:07 ` Karl-Heinz Schneider 2016-06-24 17:50 ` Rob Herring 2016-06-22 19:07 ` [PATCH 2/2] power: Adds support for Smart Battery System Manager Karl-Heinz Schneider 1 sibling, 1 reply; 13+ messages in thread From: Karl-Heinz Schneider @ 2016-06-22 19:07 UTC (permalink / raw) To: devicetree, linux-pm, linux-acpi, linux-i2c Cc: Rob Herring, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Peter Rosin, Phil Reid, Karl-Heinz Schneider This patch adds device tree documentation for the sbs-manager Reviewed-by: Phil Reid <preid@electromag.com.au> Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de> --- .../devicetree/bindings/power/sbs,sbs-manager.txt | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt new file mode 100644 index 0000000..d52b466 --- /dev/null +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt @@ -0,0 +1,58 @@ +Binding for sbs-manager + +Required properties: +- compatible: should be "sbs,sbs-manager" or "lltc,ltc1760" if device is a + ltc1760. +- reg: integer, i2c address of the device. Should be <0xa>. + +Optional properties: +- sbsm,i2c-retry-count: integer, number of retries for trying to read or write + to registers. Default: 1 + +From OS view the device is basically an i2c-mux used to communicate with up to +four smart battery devices at address 0xb. The driver actually implements this +behaviour. So standard i2c-mux nodes can be used to register up to four slave +batteries. Channels will be numerated as 1, 2, 4 and 8. + +Example: + +batman@0a { + compatible = "sbs,sbs-manager"; + reg = <0x0a>; + sbsm,i2c-retry-count = <3>; + #address-cells = <1>; + #size-cells = <0>; + + channel1@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + + battery1@0b { + compatible = "sbs-battery"; + reg = <0x0b>; + }; + }; + + channel2@2 { + #address-cells = <1>; + #size-cells = <0>; + reg = <2>; + + battery2@0b { + compatible = "sbs-battery"; + reg = <0x0b>; + }; + }; + + channel3@4 { + #address-cells = <1>; + #size-cells = <0>; + reg = <4>; + + battery3@0b { + compatible = "sbs-battery"; + reg = <0x0b>; + }; + }; +}; -- 1.9.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-22 19:07 ` [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation Karl-Heinz Schneider @ 2016-06-24 17:50 ` Rob Herring 2016-06-26 5:21 ` Phil Reid ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Rob Herring @ 2016-06-24 17:50 UTC (permalink / raw) To: Karl-Heinz Schneider Cc: devicetree, linux-pm, linux-acpi, linux-i2c, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Peter Rosin, Phil Reid On Wed, Jun 22, 2016 at 09:07:15PM +0200, Karl-Heinz Schneider wrote: > This patch adds device tree documentation for the sbs-manager > > Reviewed-by: Phil Reid <preid@electromag.com.au> > Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de> > --- > .../devicetree/bindings/power/sbs,sbs-manager.txt | 58 ++++++++++++++++++++++ > 1 file changed, 58 insertions(+) > create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt > > diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt > new file mode 100644 > index 0000000..d52b466 > --- /dev/null > +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt > @@ -0,0 +1,58 @@ > +Binding for sbs-manager > + > +Required properties: > +- compatible: should be "sbs,sbs-manager" or "lltc,ltc1760" if device is a > + ltc1760. sbs is not a vendor. What chip is sbs-manager? I suspect you should drop it and only list specific chips. > +- reg: integer, i2c address of the device. Should be <0xa>. > + > +Optional properties: > +- sbsm,i2c-retry-count: integer, number of retries for trying to read or write > + to registers. Default: 1 Seems like a driver setting. Is having a retry in the driver a problem if the h/w works and never actually needs it? > + > +From OS view the device is basically an i2c-mux used to communicate with up to > +four smart battery devices at address 0xb. The driver actually implements this > +behaviour. So standard i2c-mux nodes can be used to register up to four slave > +batteries. Channels will be numerated as 1, 2, 4 and 8. > + > +Example: > + > +batman@0a { > + compatible = "sbs,sbs-manager"; > + reg = <0x0a>; > + sbsm,i2c-retry-count = <3>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + channel1@1 { channel@1 Do we have a standard node name for mux nodes? If not, we should. > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <1>; > + > + battery1@0b { battery@b > + compatible = "sbs-battery"; This should be an actual battery model. Or all this information is generic, you don't really need it in DT. > + reg = <0x0b>; > + }; > + }; > + > + channel2@2 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <2>; > + > + battery2@0b { > + compatible = "sbs-battery"; > + reg = <0x0b>; > + }; > + }; > + > + channel3@4 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <4>; > + > + battery3@0b { > + compatible = "sbs-battery"; > + reg = <0x0b>; > + }; > + }; > +}; > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-24 17:50 ` Rob Herring @ 2016-06-26 5:21 ` Phil Reid 2016-06-26 14:05 ` Rob Herring 2016-06-26 7:10 ` Karl-Heinz Schneider 2016-06-26 22:35 ` Peter Rosin 2 siblings, 1 reply; 13+ messages in thread From: Phil Reid @ 2016-06-26 5:21 UTC (permalink / raw) To: Rob Herring, Karl-Heinz Schneider Cc: devicetree, linux-pm, linux-acpi, linux-i2c, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Peter Rosin G'day Rob Couple of thoughts / question below. On 25/06/2016 01:50, Rob Herring wrote: > On Wed, Jun 22, 2016 at 09:07:15PM +0200, Karl-Heinz Schneider wrote: >> This patch adds device tree documentation for the sbs-manager >> >> Reviewed-by: Phil Reid <preid@electromag.com.au> >> Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de> >> --- >> .../devicetree/bindings/power/sbs,sbs-manager.txt | 58 ++++++++++++++++++++++ >> 1 file changed, 58 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt >> >> diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt >> new file mode 100644 >> index 0000000..d52b466 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt >> @@ -0,0 +1,58 @@ >> +Binding for sbs-manager >> + >> +Required properties: >> +- compatible: should be "sbs,sbs-manager" or "lltc,ltc1760" if device is a >> + ltc1760. > > sbs is not a vendor. What chip is sbs-manager? I suspect you should drop > it and only list specific chips. This follows the interface to the existing paired sbs,sbs-battery driver defined in power/sbs-battery.c It implements a generic driver for the Smart Battery System Manager Specification. Spec available here: http://sbs-forum.org/specs/sbsm100b.pdf In addition the ltc1760 extends the spec. > >> +- reg: integer, i2c address of the device. Should be <0xa>. >> + >> +Optional properties: >> +- sbsm,i2c-retry-count: integer, number of retries for trying to read or write >> + to registers. Default: 1 > > Seems like a driver setting. Is having a retry in the driver a problem > if the h/w works and never actually needs it? Similarly the sbs-battery driver specifies the same same retry behaviour. And is a model for this implementation. I've found the ltc1760 and sbs batteries to be problematic when communicating to them. A lot of drivers (and the associated hardware) don't handle multiple bus masters well. The bus arbitation doesn't seem to work correctly. Retries where the only thing I could do to to get things to work reliably. Mostly means the driver needs fixing, but in one case the designware core hardware seemed to be the problem for me. > >> + >> +From OS view the device is basically an i2c-mux used to communicate with up to >> +four smart battery devices at address 0xb. The driver actually implements this >> +behaviour. So standard i2c-mux nodes can be used to register up to four slave >> +batteries. Channels will be numerated as 1, 2, 4 and 8. >> + >> +Example: >> + >> +batman@0a { >> + compatible = "sbs,sbs-manager"; >> + reg = <0x0a>; >> + sbsm,i2c-retry-count = <3>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + channel1@1 { > > channel@1 > > Do we have a standard node name for mux nodes? If not, we should. > >> + #address-cells = <1>; >> + #size-cells = <0>; >> + reg = <1>; >> + >> + battery1@0b { > > battery@b > >> + compatible = "sbs-battery"; > > This should be an actual battery model. Or all this information is > generic, you don't really need it in DT. Do we really want to restrict to battery model? I have hardware where complete different sbs compliant batteries can be plugged in. Without the compatible flag here how do you get the sbs-battery driver to load and manage the battery itself? Or are you suggesting the manager should do this somehow? I'm still learning... > >> + reg = <0x0b>; >> + }; >> + }; >> + >> + channel2@2 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + reg = <2>; >> + >> + battery2@0b { >> + compatible = "sbs-battery"; >> + reg = <0x0b>; >> + }; >> + }; >> + >> + channel3@4 { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + reg = <4>; >> + >> + battery3@0b { >> + compatible = "sbs-battery"; >> + reg = <0x0b>; >> + }; >> + }; >> +}; >> -- >> 1.9.1 >> -- Regards Phil Reid ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-26 5:21 ` Phil Reid @ 2016-06-26 14:05 ` Rob Herring 2016-06-27 21:10 ` Karl-Heinz Schneider 0 siblings, 1 reply; 13+ messages in thread From: Rob Herring @ 2016-06-26 14:05 UTC (permalink / raw) To: Phil Reid Cc: Karl-Heinz Schneider, devicetree, linux-pm, linux-acpi, linux-i2c, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Peter Rosin On Sun, Jun 26, 2016 at 12:21 AM, Phil Reid <preid@electromag.com.au> wrote: > G'day Rob > > Couple of thoughts / question below. > > On 25/06/2016 01:50, Rob Herring wrote: >> >> On Wed, Jun 22, 2016 at 09:07:15PM +0200, Karl-Heinz Schneider wrote: >>> >>> This patch adds device tree documentation for the sbs-manager >>> >>> Reviewed-by: Phil Reid <preid@electromag.com.au> >>> Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de> >>> --- >>> .../devicetree/bindings/power/sbs,sbs-manager.txt | 58 >>> ++++++++++++++++++++++ >>> 1 file changed, 58 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/power/sbs,sbs-manager.txt >>> >>> diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt >>> b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt >>> new file mode 100644 >>> index 0000000..d52b466 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt >>> @@ -0,0 +1,58 @@ >>> +Binding for sbs-manager >>> + >>> +Required properties: >>> +- compatible: should be "sbs,sbs-manager" or "lltc,ltc1760" if device is >>> a >>> + ltc1760. >> >> >> sbs is not a vendor. What chip is sbs-manager? I suspect you should drop >> it and only list specific chips. > > This follows the interface to the existing paired sbs,sbs-battery driver > defined in power/sbs-battery.c > It implements a generic driver for the Smart Battery System Manager > Specification. > Spec available here: http://sbs-forum.org/specs/sbsm100b.pdf > > In addition the ltc1760 extends the spec. Chips will always vary from specs in some way either on purpose or by accident. sbs,sbs-manager is fine as a fallback string, but there should always be a chip specific string first. >>> +- reg: integer, i2c address of the device. Should be <0xa>. >>> + >>> +Optional properties: >>> +- sbsm,i2c-retry-count: integer, number of retries for trying to read or >>> write >>> + to registers. Default: 1 >> >> >> Seems like a driver setting. Is having a retry in the driver a problem >> if the h/w works and never actually needs it? > > Similarly the sbs-battery driver specifies the same same retry behaviour. > And is a model for this implementation. > > I've found the ltc1760 and sbs batteries to be problematic when > communicating to them. > A lot of drivers (and the associated hardware) don't handle multiple bus > masters well. > The bus arbitation doesn't seem to work correctly. > Retries where the only thing I could do to to get things to work reliably. > Mostly means the driver needs fixing, but in one case the designware core > hardware seemed to be the problem for me. I'm not questioning the need for a retry. I'm questioning the need to limit the retries and tune per platform. What would be the issue if the driver hardcodes the number of retries to 10? This will work for any h/w that needs 0, 1, 2, ..., or 10 retries. The only issue would be how long until it errors out. And yes, I can confirm DW i2c h/w is a POS at least for some versions. >>> +From OS view the device is basically an i2c-mux used to communicate with >>> up to >>> +four smart battery devices at address 0xb. The driver actually >>> implements this >>> +behaviour. So standard i2c-mux nodes can be used to register up to four >>> slave >>> +batteries. Channels will be numerated as 1, 2, 4 and 8. >>> + >>> +Example: >>> + >>> +batman@0a { >>> + compatible = "sbs,sbs-manager"; >>> + reg = <0x0a>; >>> + sbsm,i2c-retry-count = <3>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + channel1@1 { >> >> >> channel@1 >> >> Do we have a standard node name for mux nodes? If not, we should. >> >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + reg = <1>; >>> + >>> + battery1@0b { >> >> >> battery@b >> >>> + compatible = "sbs-battery"; >> >> >> This should be an actual battery model. Or all this information is >> generic, you don't really need it in DT. > > Do we really want to restrict to battery model? You're not. It is just being explicit in case some battery needs special handling and you only find that out after writing the binding. The DT model is such that the kernel can be updated with fixes without updating the DT. Specific compatible strings are needed for that to work. > I have hardware where complete different sbs compliant batteries can be > plugged in. > Without the compatible flag here how do you get the sbs-battery driver to > load and manage the battery itself? > Or are you suggesting the manager should do this somehow? > I'm still learning... sbs,sbs-battery can still be a fall-back compatible string. Rob ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-26 14:05 ` Rob Herring @ 2016-06-27 21:10 ` Karl-Heinz Schneider 2016-06-28 1:13 ` Phil Reid 0 siblings, 1 reply; 13+ messages in thread From: Karl-Heinz Schneider @ 2016-06-27 21:10 UTC (permalink / raw) To: Rob Herring Cc: Phil Reid, devicetree, linux-pm, linux-acpi, linux-i2c, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Peter Rosin Am Sonntag, den 26.06.2016, 09:05 -0500 schrieb Rob Herring: > On Sun, Jun 26, 2016 at 12:21 AM, Phil Reid <preid@electromag.com.au> wrote: > > G'day Rob > > > > Couple of thoughts / question below. > > > > On 25/06/2016 01:50, Rob Herring wrote: > >> > >> On Wed, Jun 22, 2016 at 09:07:15PM +0200, Karl-Heinz Schneider wrote: > >>> > >>> This patch adds device tree documentation for the sbs-manager > >>> > >>> Reviewed-by: Phil Reid <preid@electromag.com.au> > >>> Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de> > >>> --- > >>> .../devicetree/bindings/power/sbs,sbs-manager.txt | 58 > >>> ++++++++++++++++++++++ > >>> 1 file changed, 58 insertions(+) > >>> create mode 100644 > >>> Documentation/devicetree/bindings/power/sbs,sbs-manager.txt > >>> > >>> diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt > >>> b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt > >>> new file mode 100644 > >>> index 0000000..d52b466 > >>> --- /dev/null > >>> +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt > >>> @@ -0,0 +1,58 @@ > >>> +Binding for sbs-manager > >>> + > >>> +Required properties: > >>> +- compatible: should be "sbs,sbs-manager" or "lltc,ltc1760" if device is > >>> a > >>> + ltc1760. > >> > >> > >> sbs is not a vendor. What chip is sbs-manager? I suspect you should drop > >> it and only list specific chips. > > > > This follows the interface to the existing paired sbs,sbs-battery driver > > defined in power/sbs-battery.c > > It implements a generic driver for the Smart Battery System Manager > > Specification. > > Spec available here: http://sbs-forum.org/specs/sbsm100b.pdf > > > > In addition the ltc1760 extends the spec. > > Chips will always vary from specs in some way either on purpose or by > accident. sbs,sbs-manager is fine as a fallback string, but there > should always be a chip specific string first. All right. Will change compatible string to "lltc,ltc1760" and mention in the Required properties section that "sbs,sbs-manager" is usable as fallback. > > >>> +- reg: integer, i2c address of the device. Should be <0xa>. > >>> + > >>> +Optional properties: > >>> +- sbsm,i2c-retry-count: integer, number of retries for trying to read or > >>> write > >>> + to registers. Default: 1 > >> > >> > >> Seems like a driver setting. Is having a retry in the driver a problem > >> if the h/w works and never actually needs it? > > > > Similarly the sbs-battery driver specifies the same same retry behaviour. > > And is a model for this implementation. > > > > I've found the ltc1760 and sbs batteries to be problematic when > > communicating to them. > > A lot of drivers (and the associated hardware) don't handle multiple bus > > masters well. > > The bus arbitation doesn't seem to work correctly. > > Retries where the only thing I could do to to get things to work reliably. > > Mostly means the driver needs fixing, but in one case the designware core > > hardware seemed to be the problem for me. > > I'm not questioning the need for a retry. I'm questioning the need to > limit the retries and tune per platform. What would be the issue if > the driver hardcodes the number of retries to 10? This will work for > any h/w that needs 0, 1, 2, ..., or 10 retries. The only issue would > be how long until it errors out. > > And yes, I can confirm DW i2c h/w is a POS at least for some versions. > > >>> +From OS view the device is basically an i2c-mux used to communicate with > >>> up to > >>> +four smart battery devices at address 0xb. The driver actually > >>> implements this > >>> +behaviour. So standard i2c-mux nodes can be used to register up to four > >>> slave > >>> +batteries. Channels will be numerated as 1, 2, 4 and 8. > >>> + > >>> +Example: > >>> + > >>> +batman@0a { > >>> + compatible = "sbs,sbs-manager"; > >>> + reg = <0x0a>; > >>> + sbsm,i2c-retry-count = <3>; > >>> + #address-cells = <1>; > >>> + #size-cells = <0>; > >>> + > >>> + channel1@1 { > >> > >> > >> channel@1 > >> > >> Do we have a standard node name for mux nodes? If not, we should. > >> > >>> + #address-cells = <1>; > >>> + #size-cells = <0>; > >>> + reg = <1>; > >>> + > >>> + battery1@0b { > >> > >> > >> battery@b > >> > >>> + compatible = "sbs-battery"; > >> > >> > >> This should be an actual battery model. Or all this information is > >> generic, you don't really need it in DT. > > > > Do we really want to restrict to battery model? > > You're not. It is just being explicit in case some battery needs > special handling and you only find that out after writing the binding. > The DT model is such that the kernel can be updated with fixes without > updating the DT. Specific compatible strings are needed for that to > work. > > > I have hardware where complete different sbs compliant batteries can be > > plugged in. > > Without the compatible flag here how do you get the sbs-battery driver to > > load and manage the battery itself? > > Or are you suggesting the manager should do this somehow? > > I'm still learning... > > sbs,sbs-battery can still be a fall-back compatible string. Consider this scenario: compatible = "ti,bq2060", "sbs,sbs-battery"; By now the there is no special "bq2060" driver. But my battery might be on of those. I code the DT this way everything should work find (sbs-battery driver will bind). If battery is removable and battery device changes everything should still work fine hence sbs-battery works on subset of registers of the standard. But what happens if a kernel update is done and a bq2060 driver appears which might want access to registers not part of the standard and the (now changed) device doesn't provide? > > Rob Karl-Heinz ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-27 21:10 ` Karl-Heinz Schneider @ 2016-06-28 1:13 ` Phil Reid 2016-06-28 20:54 ` Rob Herring 0 siblings, 1 reply; 13+ messages in thread From: Phil Reid @ 2016-06-28 1:13 UTC (permalink / raw) To: Karl-Heinz Schneider, Rob Herring Cc: devicetree, linux-pm, linux-acpi, linux-i2c, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Peter Rosin G'day Rob, On 28/06/2016 05:10, Karl-Heinz Schneider wrote: > Am Sonntag, den 26.06.2016, 09:05 -0500 schrieb Rob Herring: >> On Sun, Jun 26, 2016 at 12:21 AM, Phil Reid <preid@electromag.com.au> wrote: *Snip* >>>>> +- reg: integer, i2c address of the device. Should be <0xa>. >>>>> + >>>>> +Optional properties: >>>>> +- sbsm,i2c-retry-count: integer, number of retries for trying to read or >>>>> write >>>>> + to registers. Default: 1 >>>> >>>> >>>> Seems like a driver setting. Is having a retry in the driver a problem >>>> if the h/w works and never actually needs it? >>> >>> Similarly the sbs-battery driver specifies the same same retry behaviour. >>> And is a model for this implementation. >>> >>> I've found the ltc1760 and sbs batteries to be problematic when >>> communicating to them. >>> A lot of drivers (and the associated hardware) don't handle multiple bus >>> masters well. >>> The bus arbitation doesn't seem to work correctly. >>> Retries where the only thing I could do to to get things to work reliably. >>> Mostly means the driver needs fixing, but in one case the designware core >>> hardware seemed to be the problem for me. >> >> I'm not questioning the need for a retry. I'm questioning the need to >> limit the retries and tune per platform. What would be the issue if >> the driver hardcodes the number of retries to 10? This will work for >> any h/w that needs 0, 1, 2, ..., or 10 retries. The only issue would >> be how long until it errors out. >> >> And yes, I can confirm DW i2c h/w is a POS at least for some versions. >> So your suggesting we hardcode the retry value in the driver and not provide a configuration option in the binding? My only thought with allowing a dt setitng to customise the value is it allows the integrator to select how many retires they want to try before failing, which kind of limits the elapse time until a failure is reported. -- Regards Phil Reid ElectroMagnetic Imaging Technology Pty Ltd Development of Geophysical Instrumentation & Software www.electromag.com.au 3 The Avenue, Midland WA 6056, AUSTRALIA Ph: +61 8 9250 8100 Fax: +61 8 9250 7100 Email: preid@electromag.com.au ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-28 1:13 ` Phil Reid @ 2016-06-28 20:54 ` Rob Herring 0 siblings, 0 replies; 13+ messages in thread From: Rob Herring @ 2016-06-28 20:54 UTC (permalink / raw) To: Phil Reid Cc: Karl-Heinz Schneider, devicetree, linux-pm, linux-acpi, linux-i2c, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Peter Rosin On Tue, Jun 28, 2016 at 09:13:21AM +0800, Phil Reid wrote: > G'day Rob, > > On 28/06/2016 05:10, Karl-Heinz Schneider wrote: > >Am Sonntag, den 26.06.2016, 09:05 -0500 schrieb Rob Herring: > >>On Sun, Jun 26, 2016 at 12:21 AM, Phil Reid <preid@electromag.com.au> wrote: > > *Snip* > > > >>>>>+- reg: integer, i2c address of the device. Should be <0xa>. > >>>>>+ > >>>>>+Optional properties: > >>>>>+- sbsm,i2c-retry-count: integer, number of retries for trying to read or > >>>>>write > >>>>>+ to registers. Default: 1 > >>>> > >>>> > >>>>Seems like a driver setting. Is having a retry in the driver a problem > >>>>if the h/w works and never actually needs it? > >>> > >>>Similarly the sbs-battery driver specifies the same same retry behaviour. > >>>And is a model for this implementation. > >>> > >>>I've found the ltc1760 and sbs batteries to be problematic when > >>>communicating to them. > >>>A lot of drivers (and the associated hardware) don't handle multiple bus > >>>masters well. > >>>The bus arbitation doesn't seem to work correctly. > >>>Retries where the only thing I could do to to get things to work reliably. > >>>Mostly means the driver needs fixing, but in one case the designware core > >>>hardware seemed to be the problem for me. > >> > >>I'm not questioning the need for a retry. I'm questioning the need to > >>limit the retries and tune per platform. What would be the issue if > >>the driver hardcodes the number of retries to 10? This will work for > >>any h/w that needs 0, 1, 2, ..., or 10 retries. The only issue would > >>be how long until it errors out. > >> > >>And yes, I can confirm DW i2c h/w is a POS at least for some versions. > >> > > So your suggesting we hardcode the retry value in the driver and not provide a > configuration option in the binding? Yes. > My only thought with allowing a dt setitng to customise the value is it allows the > integrator to select how many retires they want to try before failing, which kind of > limits the elapse time until a failure is reported. It is easier to add later and can't be removed later. If we do want something like this, then it should be a common property for i2c devices/buses. Rob ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-24 17:50 ` Rob Herring 2016-06-26 5:21 ` Phil Reid @ 2016-06-26 7:10 ` Karl-Heinz Schneider 2016-06-26 22:35 ` Peter Rosin 2 siblings, 0 replies; 13+ messages in thread From: Karl-Heinz Schneider @ 2016-06-26 7:10 UTC (permalink / raw) To: Rob Herring Cc: devicetree, linux-pm, linux-acpi, linux-i2c, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Peter Rosin, Phil Reid Hi Rob, sorry for resending, forgot the Cc. Am Freitag, den 24.06.2016, 12:50 -0500 schrieb Rob Herring: > On Wed, Jun 22, 2016 at 09:07:15PM +0200, Karl-Heinz Schneider wrote: > > This patch adds device tree documentation for the sbs-manager > > > > Reviewed-by: Phil Reid <preid@electromag.com.au> > > Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de> > > --- > > .../devicetree/bindings/power/sbs,sbs-manager.txt | 58 ++++++++++++++++++++++ > > 1 file changed, 58 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/power/sbs,sbs-manager.txt > > > > diff --git a/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt > > new file mode 100644 > > index 0000000..d52b466 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/power/sbs,sbs-manager.txt > > @@ -0,0 +1,58 @@ > > +Binding for sbs-manager > > + > > +Required properties: > > +- compatible: should be "sbs,sbs-manager" or "lltc,ltc1760" if device is a > > + ltc1760. > > sbs is not a vendor. What chip is sbs-manager? I suspect you should drop > it and only list specific chips. Yes it's not a specific chip. But it's the specification of the interface. See http://sbs-forum.org/specs/sbsm100b.pdf. One implementation (with special features) is the LTC1760. I choose this because the sbs-battery-driver did it the same way. You can find this in sbs-battery.c: static const struct of_device_id sbs_dt_ids[] = { { .compatible = "sbs,sbs-battery" }, { .compatible = "ti,bq20z75" }, { } }; Why not keep a generic compatibility string? > > > +- reg: integer, i2c address of the device. Should be <0xa>. > > + > > +Optional properties: > > +- sbsm,i2c-retry-count: integer, number of retries for trying to read or write > > + to registers. Default: 1 > > Seems like a driver setting. Is having a retry in the driver a problem > if the h/w works and never actually needs it? It could also be passed as a module parameter. But again the sbs-battery driver did it the same way, it just acted as template in this case... The retry problematic arises because the SBSM is itself an i2c master. And (at least the LTC) stretches hosts i2c clock if it does communicate with the connected batteries. This will likely disturb the hosts communication to any i2c chip connected on this bus sooner or later, including to the SBSM. > > > + > > +From OS view the device is basically an i2c-mux used to communicate with up to > > +four smart battery devices at address 0xb. The driver actually implements this > > +behaviour. So standard i2c-mux nodes can be used to register up to four slave > > +batteries. Channels will be numerated as 1, 2, 4 and 8. > > + > > +Example: > > + > > +batman@0a { > > + compatible = "sbs,sbs-manager"; > > + reg = <0x0a>; > > + sbsm,i2c-retry-count = <3>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + channel1@1 { > > channel@1 > > Do we have a standard node name for mux nodes? If not, we should. I don't know. Other mux binding docs name the cannels just "i2c" or "port". Spec names them (the battery) "Smart Battery [A,B..]". Can at least drop the number in front of the @. > > > + #address-cells = <1>; > > + #size-cells = <0>; > > + reg = <1>; > > + > > + battery1@0b { > > battery@b > > > + compatible = "sbs-battery"; > > This should be an actual battery model. Or all this information is > generic, you don't really need it in DT. A solution could be to bind the sbs-battery driver within the sbs-manager driver for all detected channels per default, if not otherwise state in device tree? Current implementation doesn't bind anything by itself. Don't know what is the preferred behaviour? > > > + reg = <0x0b>; > > + }; > > + }; > > + > > + channel2@2 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + reg = <2>; > > + > > + battery2@0b { > > + compatible = "sbs-battery"; > > + reg = <0x0b>; > > + }; > > + }; > > + > > + channel3@4 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + reg = <4>; > > + > > + battery3@0b { > > + compatible = "sbs-battery"; > > + reg = <0x0b>; > > + }; > > + }; > > +}; > > -- > > 1.9.1 > > Tanks for review. -- Greetings ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-24 17:50 ` Rob Herring 2016-06-26 5:21 ` Phil Reid 2016-06-26 7:10 ` Karl-Heinz Schneider @ 2016-06-26 22:35 ` Peter Rosin 2016-06-27 15:28 ` Rob Herring 2 siblings, 1 reply; 13+ messages in thread From: Peter Rosin @ 2016-06-26 22:35 UTC (permalink / raw) To: Rob Herring, Karl-Heinz Schneider Cc: devicetree, linux-pm, linux-acpi, linux-i2c, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Phil Reid On 2016-06-24 19:50, Rob Herring wrote: > On Wed, Jun 22, 2016 at 09:07:15PM +0200, Karl-Heinz Schneider wrote: >> This patch adds device tree documentation for the sbs-manager *snip* >> + >> +From OS view the device is basically an i2c-mux used to communicate with up to >> +four smart battery devices at address 0xb. The driver actually implements this >> +behaviour. So standard i2c-mux nodes can be used to register up to four slave >> +batteries. Channels will be numerated as 1, 2, 4 and 8. >> + >> +Example: >> + >> +batman@0a { >> + compatible = "sbs,sbs-manager"; >> + reg = <0x0a>; >> + sbsm,i2c-retry-count = <3>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + channel1@1 { > > channel@1 > > Do we have a standard node name for mux nodes? If not, we should. No name is enforced by the i2c mux support code, but I think "i2c" dominates, and quite possibly it is the only documented name? Cheers, Peter ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-26 22:35 ` Peter Rosin @ 2016-06-27 15:28 ` Rob Herring 2016-06-27 20:37 ` Karl-Heinz Schneider 0 siblings, 1 reply; 13+ messages in thread From: Rob Herring @ 2016-06-27 15:28 UTC (permalink / raw) To: Peter Rosin Cc: Karl-Heinz Schneider, devicetree, linux-pm, linux-acpi, linux-i2c, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Phil Reid On Sun, Jun 26, 2016 at 5:35 PM, Peter Rosin <peda@axentia.se> wrote: > On 2016-06-24 19:50, Rob Herring wrote: >> On Wed, Jun 22, 2016 at 09:07:15PM +0200, Karl-Heinz Schneider wrote: >>> This patch adds device tree documentation for the sbs-manager > > *snip* > >>> + >>> +From OS view the device is basically an i2c-mux used to communicate with up to >>> +four smart battery devices at address 0xb. The driver actually implements this >>> +behaviour. So standard i2c-mux nodes can be used to register up to four slave >>> +batteries. Channels will be numerated as 1, 2, 4 and 8. >>> + >>> +Example: >>> + >>> +batman@0a { >>> + compatible = "sbs,sbs-manager"; >>> + reg = <0x0a>; >>> + sbsm,i2c-retry-count = <3>; >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + channel1@1 { >> >> channel@1 >> >> Do we have a standard node name for mux nodes? If not, we should. > > No name is enforced by the i2c mux support code, but I think "i2c" > dominates, and quite possibly it is the only documented name? The kernel generally doesn't care what node names are, but standard naming is convention. If "i2c" is most common, then go with that. Rob ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation 2016-06-27 15:28 ` Rob Herring @ 2016-06-27 20:37 ` Karl-Heinz Schneider 0 siblings, 0 replies; 13+ messages in thread From: Karl-Heinz Schneider @ 2016-06-27 20:37 UTC (permalink / raw) To: Rob Herring Cc: Peter Rosin, devicetree, linux-pm, linux-acpi, linux-i2c, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Phil Reid Am Montag, den 27.06.2016, 10:28 -0500 schrieb Rob Herring: > On Sun, Jun 26, 2016 at 5:35 PM, Peter Rosin <peda@axentia.se> wrote: > > On 2016-06-24 19:50, Rob Herring wrote: > >> On Wed, Jun 22, 2016 at 09:07:15PM +0200, Karl-Heinz Schneider wrote: > >>> This patch adds device tree documentation for the sbs-manager > > > > *snip* > > > >>> + > >>> +From OS view the device is basically an i2c-mux used to communicate with up to > >>> +four smart battery devices at address 0xb. The driver actually implements this > >>> +behaviour. So standard i2c-mux nodes can be used to register up to four slave > >>> +batteries. Channels will be numerated as 1, 2, 4 and 8. > >>> + > >>> +Example: > >>> + > >>> +batman@0a { > >>> + compatible = "sbs,sbs-manager"; > >>> + reg = <0x0a>; > >>> + sbsm,i2c-retry-count = <3>; > >>> + #address-cells = <1>; > >>> + #size-cells = <0>; > >>> + > >>> + channel1@1 { > >> > >> channel@1 > >> > >> Do we have a standard node name for mux nodes? If not, we should. > > > > No name is enforced by the i2c mux support code, but I think "i2c" > > dominates, and quite possibly it is the only documented name? > > The kernel generally doesn't care what node names are, but standard > naming is convention. If "i2c" is most common, then go with that. Will do so. "channelx" will go "i2c" "batteryx" will go "battery" > > Rob Karl-Heinz ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] power: Adds support for Smart Battery System Manager 2016-06-22 19:07 [PATCH 0/2] Add support for Smart Battery System Manager Karl-Heinz Schneider 2016-06-22 19:07 ` [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation Karl-Heinz Schneider @ 2016-06-22 19:07 ` Karl-Heinz Schneider 1 sibling, 0 replies; 13+ messages in thread From: Karl-Heinz Schneider @ 2016-06-22 19:07 UTC (permalink / raw) To: devicetree, linux-pm, linux-acpi, linux-i2c Cc: Rob Herring, Mark Rutland, Sebastian Reichel, Dmitry Eremin-Solenikov, David Woodhouse, Rafael J. Wysocki, Peter Rosin, Phil Reid, Karl-Heinz Schneider This patch adds support for Smart Battery System Manager. A SBSM is a device listening at I2C/SMBus address 0x0a and is capable of communicating up to four I2C smart battery devices. All smart battery devices are listening at address 0x0b, so the SBSM muliplexes between them. The driver makes use of the I2C-Mux framework to allow smart batteries to be bound via device tree, i.e. the sbs-battery driver. Via sysfs interface the online state and charge type are presented. If the driver is bound as ltc1760 (an implementation of a Dual Smart Battery System Manager) the charge type can also be changed from trickle to fast. Tested-by: Phil Reid <preid@electromag.com.au> Reviewed-by: Phil Reid <preid@electromag.com.au> Signed-off-by: Karl-Heinz Schneider <karl-heinz@schneider-inet.de> --- drivers/power/Kconfig | 13 ++ drivers/power/Makefile | 1 + drivers/power/sbs-manager.c | 347 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 361 insertions(+) create mode 100644 drivers/power/sbs-manager.c diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig index 421770d..7c561bf 100644 --- a/drivers/power/Kconfig +++ b/drivers/power/Kconfig @@ -157,6 +157,19 @@ config BATTERY_WM97XX help Say Y to enable support for battery measured by WM97xx aux port. +config MANAGER_SBS + tristate "Smart Battery System Manager" + depends on I2C && I2C_MUX + help + Say Y here to include support for Smart Battery System Manager + ICs. The driver reports online and charging status via sysfs. + It presents itself also as I2C mux which allows to bind + smart battery driver to its ports. + Supported is for example LTC1760. + + This driver can also be built as a module. If so, the module will be + called sbs-manager. + config BATTERY_SBS tristate "SBS Compliant gas gauge" depends on I2C diff --git a/drivers/power/Makefile b/drivers/power/Makefile index e46b75d..5378e65 100644 --- a/drivers/power/Makefile +++ b/drivers/power/Makefile @@ -22,6 +22,7 @@ obj-$(CONFIG_BATTERY_DS2760) += ds2760_battery.o obj-$(CONFIG_BATTERY_DS2780) += ds2780_battery.o obj-$(CONFIG_BATTERY_DS2781) += ds2781_battery.o obj-$(CONFIG_BATTERY_DS2782) += ds2782_battery.o +obj-$(CONFIG_MANAGER_SBS) += sbs-manager.o obj-$(CONFIG_BATTERY_GAUGE_LTC2941) += ltc2941-battery-gauge.o obj-$(CONFIG_BATTERY_GOLDFISH) += goldfish_battery.o obj-$(CONFIG_BATTERY_PMU) += pmu_battery.o diff --git a/drivers/power/sbs-manager.c b/drivers/power/sbs-manager.c new file mode 100644 index 0000000..228bab8 --- /dev/null +++ b/drivers/power/sbs-manager.c @@ -0,0 +1,347 @@ +/* + * Driver for SBS compliant Smart Battery System Managers + * + * The device communicates via i2c at address 0x0a and multiplexes access to up + * to four smart batteries at address 0x0b. + * + * Via sysfs interface the online state and charge type are presented. + * + * Datasheet SBSM: http://sbs-forum.org/specs/sbsm100b.pdf + * Datasheet LTC1760: http://cds.linear.com/docs/en/datasheet/1760fb.pdf + * + * Karl-Heinz Schneider <karl-heinz@schneider-inet.de> + * + * 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. + */ + +#include <linux/module.h> +#include <linux/i2c.h> +#include <linux/i2c-mux.h> +#include <linux/power_supply.h> + +#define SBSM_MAX_BATS 4 + +/* registers addresses */ +#define SBSM_CMD_BATSYSSTATE 0x01 +#define SBSM_CMD_BATSYSSTATECONT 0x02 +#define SBSM_CMD_BATSYSINFO 0x04 +#define SBSM_CMD_LTC 0x3c + +struct sbsm_data { + struct i2c_client *client; + struct i2c_mux_core *muxc; + + struct power_supply *psy; + + int retries; /* number of i2c transfer retries */ + int cur_chan; /* currently selected channel */ + bool is_ltc1760; /* special capabilities */ +}; + +static enum power_supply_property sbsm_props[] = { + POWER_SUPPLY_PROP_ONLINE, + POWER_SUPPLY_PROP_CHARGE_TYPE, +}; + +static int sbsm_read_word(struct i2c_client *client, u8 address) +{ + struct sbsm_data *data = i2c_get_clientdata(client); + int reg, retries = max(data->retries + 1, 1); + + while (retries > 0) { + reg = i2c_smbus_read_word_data(client, address); + if (reg >= 0) + break; + --retries; + } + + if (reg < 0) { + dev_err(&client->dev, "failed to read register %i\n", + (int)address); + return reg; + } + + return le16_to_cpu(reg); +} + +static int sbsm_write_word(struct i2c_client *client, u8 address, u16 word) +{ + struct sbsm_data *data = i2c_get_clientdata(client); + int ret, retries = max(data->retries + 1, 1); + + word = cpu_to_le16(word); + while (retries > 0) { + ret = i2c_smbus_write_word_data(client, address, word); + if (ret >= 0) + break; + --retries; + } + if (ret < 0) + dev_err(&client->dev, "failed to write to register %i\n", + (int)address); + + return ret; +} + +static int sbsm_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val) +{ + struct sbsm_data *data = power_supply_get_drvdata(psy); + int regval = 0; + + switch (psp) { + case POWER_SUPPLY_PROP_ONLINE: + regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATECONT); + if (regval < 0) + return regval; + val->intval = !!(regval & 0x1); + break; + + case POWER_SUPPLY_PROP_CHARGE_TYPE: + regval = sbsm_read_word(data->client, SBSM_CMD_BATSYSSTATE); + if (regval < 0) + return regval; + + if ((regval & 0x00f0) == 0) { + val->intval = POWER_SUPPLY_CHARGE_TYPE_NONE; + return 0; + } + val->intval = POWER_SUPPLY_CHARGE_TYPE_TRICKLE; + + if (data->is_ltc1760) { + /* charge mode fast if turbo is active */ + regval = sbsm_read_word(data->client, SBSM_CMD_LTC); + if (regval < 0) + return regval; + else if (regval & 0x80) + val->intval = POWER_SUPPLY_CHARGE_TYPE_FAST; + } + break; + + default: + return -EINVAL; + } + + return 0; +} + +static int sbsm_prop_is_writeable(struct power_supply *psy, + enum power_supply_property psp) +{ + struct sbsm_data *data = power_supply_get_drvdata(psy); + + return (psp == POWER_SUPPLY_PROP_CHARGE_TYPE) && data->is_ltc1760; +} + +static int sbsm_set_property(struct power_supply *psy, + enum power_supply_property psp, + const union power_supply_propval *val) +{ + struct sbsm_data *data = power_supply_get_drvdata(psy); + int ret = -EINVAL; + + switch (psp) { + case POWER_SUPPLY_PROP_CHARGE_TYPE: + /* write 1 to TURBO if type fast is given */ + if (data->is_ltc1760) { + u16 regval = val->intval == + POWER_SUPPLY_CHARGE_TYPE_FAST ? (0x1 << 7) : 0; + ret = sbsm_write_word(data->client, SBSM_CMD_LTC, + regval); + } + break; + + default: + break; + } + + return ret; +} + +/* + * Switch to battery + * Parameter chan is directly the content of SMB_BAT* nibble + */ +static int sbsm_select(struct i2c_mux_core *muxc, u32 chan) +{ + struct sbsm_data *data = i2c_mux_priv(muxc); + int ret = 0; + + if (data->cur_chan == chan) + return ret; + + ret = sbsm_write_word(data->client, SBSM_CMD_BATSYSSTATE, chan << 12); + if (ret) + dev_err(&data->client->dev, "Failed to select channel %i\n", + chan); + else + data->cur_chan = chan; + + return ret; +} + +#if defined(CONFIG_OF) + +#include <linux/of_device.h> + +static const struct of_device_id sbsm_dt_ids[] = { + { .compatible = "sbs,sbs-manager" }, + { .compatible = "lltc,ltc1760" }, + { } +}; +MODULE_DEVICE_TABLE(of, sbsm_dt_ids); + +static void sbsm_properties_from_of(struct sbsm_data *data) +{ + struct device_node *of_node = data->client->dev.of_node; + int rc; + u32 value = 0; + + if (!of_node) + return; + + rc = of_property_read_u32(of_node, "sbsm,i2c-retry-count", &value); + if (!rc) + data->retries = (int)max(1u, value); +} + +#else + +static void sbsm_properties_from_of(struct sbsm_data *data) +{ } + +#endif + +static const struct power_supply_desc sbsm_default_psy_desc = { + .type = POWER_SUPPLY_TYPE_MAINS, + .properties = sbsm_props, + .num_properties = ARRAY_SIZE(sbsm_props), + .get_property = &sbsm_get_property, + .set_property = &sbsm_set_property, + .property_is_writeable = &sbsm_prop_is_writeable, +}; + +static int sbsm_probe(struct i2c_client *client, + const struct i2c_device_id *id) +{ + struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent); + struct sbsm_data *data; + struct device *dev = &client->dev; + struct power_supply_desc *psy_desc; + struct power_supply_config psy_cfg = {}; + int ret = 0, i, supported_bats; + + /* Device listens only at address 0x0a */ + if (client->addr != 0x0a) + return -ENODEV; + + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_WORD_DATA)) + return -EPFNOSUPPORT; + + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + + i2c_set_clientdata(client, data); + + data->client = client; + data->retries = 1; + sbsm_properties_from_of(data); + data->is_ltc1760 = !!strstr(id->name, "ltc1760"); + + ret = sbsm_read_word(client, SBSM_CMD_BATSYSINFO); + if (ret < 0) + return ret; + supported_bats = le16_to_cpu(ret) & 0xf; + + data->muxc = i2c_mux_alloc(adapter, dev, SBSM_MAX_BATS, 0, + I2C_MUX_LOCKED, &sbsm_select, NULL); + if (!data->muxc) { + dev_err(dev, "failed to alloc i2c mux\n"); + ret = -ENOMEM; + goto err_mux_alloc; + } + data->muxc->priv = data; + + /* register muxed i2c channels. One for each supported battery */ + for (i = 0; i < SBSM_MAX_BATS; ++i) { + uint chan = 1 << i; + + if (chan & supported_bats) { + ret = i2c_mux_add_adapter(data->muxc, 0, chan, 0); + if (ret) { + dev_err(dev, + "failed to register i2c mux channel %d\n", + chan); + goto err_mux_register; + } + } + } + + psy_desc = devm_kmemdup(dev, &sbsm_default_psy_desc, + sizeof(struct power_supply_desc), + GFP_KERNEL); + if (!psy_desc) { + ret = -ENOMEM; + goto err_psy; + } + + psy_desc->name = devm_kasprintf(dev, GFP_KERNEL, "sbsm-%s", + dev_name(&client->dev)); + if (!psy_desc->name) { + ret = -ENOMEM; + goto err_psy; + } + + psy_cfg.drv_data = data; + data->psy = devm_power_supply_register(dev, psy_desc, &psy_cfg); + if (IS_ERR(data->psy)) { + ret = PTR_ERR(data->psy); + dev_err(dev, "failed to register power supply %s\n", + psy_desc->name); + goto err_psy; + } + + dev_info(dev, "sbsm registered\n"); + return 0; + +err_psy: +err_mux_register: + i2c_mux_del_adapters(data->muxc); + +err_mux_alloc: + return ret; +} + +static int sbsm_remove(struct i2c_client *client) +{ + struct sbsm_data *data = i2c_get_clientdata(client); + + i2c_mux_del_adapters(data->muxc); + return 0; +} + +static const struct i2c_device_id sbsm_ids[] = { + { "sbs-manager", 0 }, + { "ltc1760", 0 }, + { } +}; +MODULE_DEVICE_TABLE(i2c, sbsm_ids); + +static struct i2c_driver sbsm_driver = { + .driver = { + .name = "sbsm", + .owner = THIS_MODULE, + }, + .probe = sbsm_probe, + .remove = sbsm_remove, + .id_table = sbsm_ids +}; +module_i2c_driver(sbsm_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Karl-Heinz Schneider <karl-heinz@schneider-inet.de>"); +MODULE_DESCRIPTION("SBSM Smart Battery System Manager"); -- 1.9.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-06-28 20:55 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-22 19:07 [PATCH 0/2] Add support for Smart Battery System Manager Karl-Heinz Schneider 2016-06-22 19:07 ` [PATCH 1/2] Documentation: Add sbs-manager device tree node documentation Karl-Heinz Schneider 2016-06-24 17:50 ` Rob Herring 2016-06-26 5:21 ` Phil Reid 2016-06-26 14:05 ` Rob Herring 2016-06-27 21:10 ` Karl-Heinz Schneider 2016-06-28 1:13 ` Phil Reid 2016-06-28 20:54 ` Rob Herring 2016-06-26 7:10 ` Karl-Heinz Schneider 2016-06-26 22:35 ` Peter Rosin 2016-06-27 15:28 ` Rob Herring 2016-06-27 20:37 ` Karl-Heinz Schneider 2016-06-22 19:07 ` [PATCH 2/2] power: Adds support for Smart Battery System Manager Karl-Heinz Schneider
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.