linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* BQ27xxx registers
@ 2016-12-20  6:00 Chris Lapa
  2016-12-20 11:34 ` Pali Rohár
  0 siblings, 1 reply; 8+ messages in thread
From: Chris Lapa @ 2016-12-20  6:00 UTC (permalink / raw)
  To: pali.rohar, afd, Sebastian Reichel, linux-pm, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1255 bytes --]

Hi,

I'm testing out the 4.9 kernel on a AM3359 board fitted with a 
BQ27510-G3 fuel gauge. The board previously worked on the 4.1 kernel, 
however on the 4.9 kernel the bq27xxx_battery.c driver is spitting out 
this error continuously:

power_supply bq27510-0: driver failed to report `charge_full_design' 
property: -121

I have narrowed down the problem to commit 099867a16 where the 
BQ27XXX_REG_DCAP value was changed from 0x2e to 0x3c.

I can generate a patch to fix this issue, however the bigger problem 
exists as to which revision fuel gauge the bq27xxx_battery.c driver is 
intended to support for each family.

Attached is a table I put together of all the register addresses for 
each supported device under the BQ27500 definition and what the current 
driver utilizes (I didn't paste it here as the word wrapping messes with 
the formatting).

There isn't really an ideal solution I can see where we keep the single 
BQ27500 definition and support all the functionality of each revision.

We can try and just support the latest revisions (BQ27510-G3 and 
BQ27520-G4) but I think it could hurt backwards compatibility for 
existing hardware.

I'm happy to work on a fix but just wanted to get some thoughts before 
proceeding.

Cheers,
Chris

[-- Attachment #2: BQ275xx-Gx.txt --]
[-- Type: text/plain, Size: 1255 bytes --]

Register		BQ27500/1	BQ27510-G1	BQ27510-G2	BQ27510-G3	BQ27520-G1	BQ27520-G2	BQ27520-G3	BQ27520-G4 	bq27xxx_battery.c
BQ27XXX_REG_CTRL	0x00		0x00		0x00		0x00		0x00		0x00		0x00		0x00		0x00
BQ27XXX_REG_TEMP	0x06		0x06		0x06		0x06		0x06		0x06		0x06		0x06		0x06
BQ27XXX_REG_INT_TEMP	—		-		-		0x28		-		0x36		0x36		0x28		0x28
BQ27XXX_REG_VOLT	0x08		0x08		0x08		0x08		0x08		0x08		0x08		0x08		0x08
BQ27XXX_REG_AI		0x14		0x14		0x14		0x14		0x14		0x14		0x14		0x14		0x14
BQ27XXX_REG_FLAGS	0x0a		0x0a		0x0a		0x0a		0x0a		0x0a		0x0a		0x0a		0x0a
BQ27XXX_REG_TTE		0x16		0x16		0x16		0x16		0x16		0x16		0x16		0x16		0x16
BQ27XXX_REG_TTF		0x18		0x18		0x18		—		0x18		0x18		-		-		-
BQ27XXX_REG_TTES	0x1c		0x1c		0x1c		0x1a		0x1c		0x1c		0x1c		0x1a		0x1a
BQ27XXX_REG_TTECP	0x26		0x26		0x26		-		0x26		0x26		0x26		-		-
BQ27XXX_REG_NAC		0x0c		0x0c		0x0c		0x0c		0x0c		0x0c		0x0c		0x0c		0x0c
BQ27XXX_REG_FCC		0x12		0x12		0x12		0x12		0x12		0x12		0x12		0x12		0x12
BQ27XXX_REG_CYCT	0x2a		0x2a		0x2a		0x1e		-		0x2a		0x2a		0x1e		0x2a
BQ27XXX_REG_AE		0x22		0x22		0x22		-		0x22		0x22		0x22		-		-
BQ27XXX_REG_SOC		0x2c		0x2c		0x2c		0x20		0x2c		0x2c		0x2c		0x20		0x2c
BQ27XXX_REG_DCAP	0x3c		0x3c		0x3c		0x2e		0x3c		0x3c		0x3c		-		0x3c
BQ27XXX_REG_AP		0x24		0x24		0x24		-		0x24		0x24		0x24		-		-

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: BQ27xxx registers
  2016-12-20  6:00 BQ27xxx registers Chris Lapa
@ 2016-12-20 11:34 ` Pali Rohár
  2016-12-21  2:49   ` Chris Lapa
  0 siblings, 1 reply; 8+ messages in thread
From: Pali Rohár @ 2016-12-20 11:34 UTC (permalink / raw)
  To: chris; +Cc: afd, Sebastian Reichel, linux-pm, linux-kernel

[-- Attachment #1: Type: Text/Plain, Size: 458 bytes --]

On Tuesday 20 December 2016 07:00:41 Chris Lapa wrote:
> I can generate a patch to fix this issue, however the bigger problem
> exists as to which revision fuel gauge the bq27xxx_battery.c driver
> is intended to support for each family.

Hi! I think driver should support all revisions. There can be (and 
probably really is) hardware which uses old revision and such hardware 
should be still supported...

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: BQ27xxx registers
  2016-12-20 11:34 ` Pali Rohár
@ 2016-12-21  2:49   ` Chris Lapa
  2016-12-21 12:46     ` Pali Rohár
  0 siblings, 1 reply; 8+ messages in thread
From: Chris Lapa @ 2016-12-21  2:49 UTC (permalink / raw)
  To: Pali Rohár; +Cc: afd, Sebastian Reichel, linux-pm, linux-kernel

On 20/12/16 10:34 pm, Pali Rohár wrote:
> On Tuesday 20 December 2016 07:00:41 Chris Lapa wrote:
>> I can generate a patch to fix this issue, however the bigger problem
>> exists as to which revision fuel gauge the bq27xxx_battery.c driver
>> is intended to support for each family.
>
> Hi! I think driver should support all revisions. There can be (and
> probably really is) hardware which uses old revision and such hardware
> should be still supported...
>

I agree. However due to the register address changes across the spectrum 
of revisions, each revision will have to be specified individually. For 
example, we will need to implement a BQ27510G1, BQ27510G2, BQ27510G3, 
BQ27520G1, BQ27520G2, BQ27520G3, BQ27520G4 definitions and prospective 
device tree additions ti,bq27510g1, ti,bq27510g2 etc.

The other option is to aim for bottom of the barrel support for all the 
devices under the BQ27500 definition but my feeling is it would get 
messier fast and be less maintainable.

My preference is to go with the first option if you agree?

Thanks,
Chris

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: BQ27xxx registers
  2016-12-21  2:49   ` Chris Lapa
@ 2016-12-21 12:46     ` Pali Rohár
  2016-12-21 13:19       ` Sebastian Reichel
  2016-12-21 23:37       ` Chris Lapa
  0 siblings, 2 replies; 8+ messages in thread
From: Pali Rohár @ 2016-12-21 12:46 UTC (permalink / raw)
  To: chris; +Cc: afd, Sebastian Reichel, linux-pm, linux-kernel

[-- Attachment #1: Type: Text/Plain, Size: 1612 bytes --]

On Wednesday 21 December 2016 03:49:10 Chris Lapa wrote:
> On 20/12/16 10:34 pm, Pali Rohár wrote:
> > On Tuesday 20 December 2016 07:00:41 Chris Lapa wrote:
> >> I can generate a patch to fix this issue, however the bigger
> >> problem exists as to which revision fuel gauge the
> >> bq27xxx_battery.c driver is intended to support for each family.
> > 
> > Hi! I think driver should support all revisions. There can be (and
> > probably really is) hardware which uses old revision and such
> > hardware should be still supported...
> 
> I agree. However due to the register address changes across the
> spectrum of revisions, each revision will have to be specified
> individually. For example, we will need to implement a BQ27510G1,
> BQ27510G2, BQ27510G3, BQ27520G1, BQ27520G2, BQ27520G3, BQ27520G4
> definitions and prospective device tree additions ti,bq27510g1,
> ti,bq27510g2 etc.
> 
> The other option is to aim for bottom of the barrel support for all
> the devices under the BQ27500 definition but my feeling is it would
> get messier fast and be less maintainable.
> 
> My preference is to go with the first option if you agree?

Yes. If those chips have different register addresses, then those chips 
are different. Name, generation or suffix does not matter here.

Similarly there could be chips with different name, but same addresses, 
so can use one driver/configuration without any change.

So I'm for different name in device tree (or platform data or what is 
being used) to distinguish between different revisions.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: BQ27xxx registers
  2016-12-21 12:46     ` Pali Rohár
@ 2016-12-21 13:19       ` Sebastian Reichel
  2016-12-21 23:37       ` Chris Lapa
  1 sibling, 0 replies; 8+ messages in thread
From: Sebastian Reichel @ 2016-12-21 13:19 UTC (permalink / raw)
  To: Pali Rohár; +Cc: chris, afd, linux-pm, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

Hi,

On Wed, Dec 21, 2016 at 01:46:17PM +0100, Pali Rohár wrote:
> On Wednesday 21 December 2016 03:49:10 Chris Lapa wrote:
> > On 20/12/16 10:34 pm, Pali Rohár wrote:
> > > On Tuesday 20 December 2016 07:00:41 Chris Lapa wrote:
> > >> I can generate a patch to fix this issue, however the bigger
> > >> problem exists as to which revision fuel gauge the
> > >> bq27xxx_battery.c driver is intended to support for each family.
> > > 
> > > Hi! I think driver should support all revisions. There can be (and
> > > probably really is) hardware which uses old revision and such
> > > hardware should be still supported...
> > 
> > I agree. However due to the register address changes across the
> > spectrum of revisions, each revision will have to be specified
> > individually. For example, we will need to implement a BQ27510G1,
> > BQ27510G2, BQ27510G3, BQ27520G1, BQ27520G2, BQ27520G3, BQ27520G4
> > definitions and prospective device tree additions ti,bq27510g1,
> > ti,bq27510g2 etc.
> > 
> > The other option is to aim for bottom of the barrel support for all
> > the devices under the BQ27500 definition but my feeling is it would
> > get messier fast and be less maintainable.
> > 
> > My preference is to go with the first option if you agree?
> 
> Yes. If those chips have different register addresses, then those chips 
> are different. Name, generation or suffix does not matter here.
> 
> Similarly there could be chips with different name, but same addresses, 
> so can use one driver/configuration without any change.
> 
> So I'm for different name in device tree (or platform data or what is 
> being used) to distinguish between different revisions.

Sounds fine to me. Let's keep the compatible string without revision
as deprecated alias for the version currently implemented by the
driver (for backward compatibility).

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: BQ27xxx registers
  2016-12-21 12:46     ` Pali Rohár
  2016-12-21 13:19       ` Sebastian Reichel
@ 2016-12-21 23:37       ` Chris Lapa
  2017-01-16 17:43         ` Andrew F. Davis
  1 sibling, 1 reply; 8+ messages in thread
From: Chris Lapa @ 2016-12-21 23:37 UTC (permalink / raw)
  To: Pali Rohár; +Cc: afd, Sebastian Reichel, linux-pm, linux-kernel

On 21/12/16 11:46 pm, Pali Rohár wrote:
> On Wednesday 21 December 2016 03:49:10 Chris Lapa wrote:
>> On 20/12/16 10:34 pm, Pali Rohár wrote:
>>> On Tuesday 20 December 2016 07:00:41 Chris Lapa wrote:
>>>> I can generate a patch to fix this issue, however the bigger
>>>> problem exists as to which revision fuel gauge the
>>>> bq27xxx_battery.c driver is intended to support for each family.
>>>
>>> Hi! I think driver should support all revisions. There can be (and
>>> probably really is) hardware which uses old revision and such
>>> hardware should be still supported...
>>
>> I agree. However due to the register address changes across the
>> spectrum of revisions, each revision will have to be specified
>> individually. For example, we will need to implement a BQ27510G1,
>> BQ27510G2, BQ27510G3, BQ27520G1, BQ27520G2, BQ27520G3, BQ27520G4
>> definitions and prospective device tree additions ti,bq27510g1,
>> ti,bq27510g2 etc.
>>
>> The other option is to aim for bottom of the barrel support for all
>> the devices under the BQ27500 definition but my feeling is it would
>> get messier fast and be less maintainable.
>>
>> My preference is to go with the first option if you agree?
>
> Yes. If those chips have different register addresses, then those chips
> are different. Name, generation or suffix does not matter here.
>
> Similarly there could be chips with different name, but same addresses,
> so can use one driver/configuration without any change.
>
> So I'm for different name in device tree (or platform data or what is
> being used) to distinguish between different revisions.
>

I've been working my way through the revision migration datasheets and 
noticed this could be simplified with the FW_VERSION parameter. It is 
always located at the same address and is distinctly different between 
each chip revision. Unfortunately the migration datasheets vs individual 
revision datasheets firmware version information directly contradict 
each other. Which makes me wary of committing to using it.

Given that I don't have every single variant of this device to test 
with, its probably still safest to have the user manually specify each 
device. I should have some patches ready soon.

Thanks,
Chris

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: BQ27xxx registers
  2016-12-21 23:37       ` Chris Lapa
@ 2017-01-16 17:43         ` Andrew F. Davis
  2017-01-17  4:47           ` Chris Lapa
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew F. Davis @ 2017-01-16 17:43 UTC (permalink / raw)
  To: chris, Pali Rohár; +Cc: Sebastian Reichel, linux-pm, linux-kernel

On 12/21/2016 05:37 PM, Chris Lapa wrote:
> On 21/12/16 11:46 pm, Pali Rohár wrote:
>> On Wednesday 21 December 2016 03:49:10 Chris Lapa wrote:
>>> On 20/12/16 10:34 pm, Pali Rohár wrote:
>>>> On Tuesday 20 December 2016 07:00:41 Chris Lapa wrote:
>>>>> I can generate a patch to fix this issue, however the bigger
>>>>> problem exists as to which revision fuel gauge the
>>>>> bq27xxx_battery.c driver is intended to support for each family.
>>>>
>>>> Hi! I think driver should support all revisions. There can be (and
>>>> probably really is) hardware which uses old revision and such
>>>> hardware should be still supported...
>>>
>>> I agree. However due to the register address changes across the
>>> spectrum of revisions, each revision will have to be specified
>>> individually. For example, we will need to implement a BQ27510G1,
>>> BQ27510G2, BQ27510G3, BQ27520G1, BQ27520G2, BQ27520G3, BQ27520G4
>>> definitions and prospective device tree additions ti,bq27510g1,
>>> ti,bq27510g2 etc.
>>>
>>> The other option is to aim for bottom of the barrel support for all
>>> the devices under the BQ27500 definition but my feeling is it would
>>> get messier fast and be less maintainable.
>>>
>>> My preference is to go with the first option if you agree?
>>
>> Yes. If those chips have different register addresses, then those chips
>> are different. Name, generation or suffix does not matter here.
>>
>> Similarly there could be chips with different name, but same addresses,
>> so can use one driver/configuration without any change.
>>
>> So I'm for different name in device tree (or platform data or what is
>> being used) to distinguish between different revisions.
>>
> 
> I've been working my way through the revision migration datasheets and
> noticed this could be simplified with the FW_VERSION parameter. It is
> always located at the same address and is distinctly different between
> each chip revision. Unfortunately the migration datasheets vs individual
> revision datasheets firmware version information directly contradict
> each other. Which makes me wary of committing to using it.
> 

BTW, could you give some specific examples of this? I can work with the
HW teams to get any documentation problems fixed, so we can in the
future use this FW_VERSION parameter if needed.

Thanks,
Andrew

> Given that I don't have every single variant of this device to test
> with, its probably still safest to have the user manually specify each
> device. I should have some patches ready soon.
> 
> Thanks,
> Chris
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: BQ27xxx registers
  2017-01-16 17:43         ` Andrew F. Davis
@ 2017-01-17  4:47           ` Chris Lapa
  0 siblings, 0 replies; 8+ messages in thread
From: Chris Lapa @ 2017-01-17  4:47 UTC (permalink / raw)
  To: Andrew F. Davis, Pali Rohár
  Cc: Sebastian Reichel, linux-pm, linux-kernel

On 17/1/17 4:43 am, Andrew F. Davis wrote:
> On 12/21/2016 05:37 PM, Chris Lapa wrote:
>> On 21/12/16 11:46 pm, Pali Rohár wrote:
>>> On Wednesday 21 December 2016 03:49:10 Chris Lapa wrote:
>>>> On 20/12/16 10:34 pm, Pali Rohár wrote:
>>>>> On Tuesday 20 December 2016 07:00:41 Chris Lapa wrote:
>>>>>> I can generate a patch to fix this issue, however the bigger
>>>>>> problem exists as to which revision fuel gauge the
>>>>>> bq27xxx_battery.c driver is intended to support for each family.
>>>>>
>>>>> Hi! I think driver should support all revisions. There can be (and
>>>>> probably really is) hardware which uses old revision and such
>>>>> hardware should be still supported...
>>>>
>>>> I agree. However due to the register address changes across the
>>>> spectrum of revisions, each revision will have to be specified
>>>> individually. For example, we will need to implement a BQ27510G1,
>>>> BQ27510G2, BQ27510G3, BQ27520G1, BQ27520G2, BQ27520G3, BQ27520G4
>>>> definitions and prospective device tree additions ti,bq27510g1,
>>>> ti,bq27510g2 etc.
>>>>
>>>> The other option is to aim for bottom of the barrel support for all
>>>> the devices under the BQ27500 definition but my feeling is it would
>>>> get messier fast and be less maintainable.
>>>>
>>>> My preference is to go with the first option if you agree?
>>>
>>> Yes. If those chips have different register addresses, then those chips
>>> are different. Name, generation or suffix does not matter here.
>>>
>>> Similarly there could be chips with different name, but same addresses,
>>> so can use one driver/configuration without any change.
>>>
>>> So I'm for different name in device tree (or platform data or what is
>>> being used) to distinguish between different revisions.
>>>
>>
>> I've been working my way through the revision migration datasheets and
>> noticed this could be simplified with the FW_VERSION parameter. It is
>> always located at the same address and is distinctly different between
>> each chip revision. Unfortunately the migration datasheets vs individual
>> revision datasheets firmware version information directly contradict
>> each other. Which makes me wary of committing to using it.
>>
>
> BTW, could you give some specific examples of this? I can work with the
> HW teams to get any documentation problems fixed, so we can in the
> future use this FW_VERSION parameter if needed.
>
> Thanks,
> Andrew
>
>> Given that I don't have every single variant of this device to test
>> with, its probably still safest to have the user manually specify each
>> device. I should have some patches ready soon.
>>
>> Thanks,
>> Chris
>>

Hi Andrew,

I've gone through and made a table based on the migration datasheets and
user manuals TI has provided.

CHIP		Migration D/S	User Manual
BQ27500/1 	N/A		1.06, 1.08
BQ27500-V100	1.08		1.06, 1.08
BQ27500-V120	1.20		1.20
BQ27500-V130	1.30		1.30
BQ27510-G1	1.12		Not listed
BQ27510-G2	1.23		Not listed
BQ27510-G3	4.00		4.00
BQ27520-G1	3.02 		3.01
BQ27520-G2	3.11		3.11
BQ27520-G3	3.24		3.23
BQ27520-G4	3.29		3.29

I suspect the BQ27500/1 and BQ27500-V100 are the same product but they
have separate product pages so I treated them separately. I also suspect
the different firmware revisions are probably legitimate due bugs being
fixed between when the user manual was released vs when the migration
datasheet was released.

Using the FW_VERSION parameter would be ideal, but some quick googling 
on golden images indicates that they include firmware. Which might 
introduce some more firmware variants?

Thanks,
Chris

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-01-17  4:47 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-20  6:00 BQ27xxx registers Chris Lapa
2016-12-20 11:34 ` Pali Rohár
2016-12-21  2:49   ` Chris Lapa
2016-12-21 12:46     ` Pali Rohár
2016-12-21 13:19       ` Sebastian Reichel
2016-12-21 23:37       ` Chris Lapa
2017-01-16 17:43         ` Andrew F. Davis
2017-01-17  4:47           ` Chris Lapa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).