2023-08-24 14:57:40

by Emmanuel Gil Peyrot

[permalink] [raw]
Subject: [PATCH 1/2] power: supply: bq24190: Support bq24193

From: Alexandre Courbot <[email protected]>

This charger is working with the driver as-is, so enable it to be probed.

It is used in the Nintendo Switch for instance.

Signed-off-by: Alexandre Courbot <[email protected]>
---
Documentation/devicetree/bindings/power/supply/bq24190.yaml | 1 +
drivers/power/supply/bq24190_charger.c | 2 ++
2 files changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/bq24190.yaml b/Documentation/devicetree/bindings/power/supply/bq24190.yaml
index d3ebc9de8c0b..92a28d3c3070 100644
--- a/Documentation/devicetree/bindings/power/supply/bq24190.yaml
+++ b/Documentation/devicetree/bindings/power/supply/bq24190.yaml
@@ -18,6 +18,7 @@ properties:
enum:
- ti,bq24190
- ti,bq24192
+ - ti,bq24193
- ti,bq24192i
- ti,bq24196

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
index ef8235848f56..a56122b39687 100644
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c
@@ -2018,6 +2018,7 @@ static const struct dev_pm_ops bq24190_pm_ops = {
static const struct i2c_device_id bq24190_i2c_ids[] = {
{ "bq24190" },
{ "bq24192" },
+ { "bq24193" },
{ "bq24192i" },
{ "bq24196" },
{ },
@@ -2027,6 +2028,7 @@ MODULE_DEVICE_TABLE(i2c, bq24190_i2c_ids);
static const struct of_device_id bq24190_of_match[] = {
{ .compatible = "ti,bq24190", },
{ .compatible = "ti,bq24192", },
+ { .compatible = "ti,bq24193", },
{ .compatible = "ti,bq24192i", },
{ .compatible = "ti,bq24196", },
{ },
--
2.42.0



2023-08-24 15:24:34

by Emmanuel Gil Peyrot

[permalink] [raw]
Subject: Re: [PATCH 1/2] power: supply: bq24190: Support bq24193

On Thu, Aug 24, 2023 at 02:40:10PM +0200, Krzysztof Kozlowski wrote:
[…]
> > diff --git a/Documentation/devicetree/bindings/power/supply/bq24190.yaml b/Documentation/devicetree/bindings/power/supply/bq24190.yaml
> > index d3ebc9de8c0b..92a28d3c3070 100644
> > --- a/Documentation/devicetree/bindings/power/supply/bq24190.yaml
> > +++ b/Documentation/devicetree/bindings/power/supply/bq24190.yaml
> > @@ -18,6 +18,7 @@ properties:
> > enum:
> > - ti,bq24190
> > - ti,bq24192
> > + - ti,bq24193
> > - ti,bq24192i
> > - ti,bq24196
> >
> > diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
> > index ef8235848f56..a56122b39687 100644
> > --- a/drivers/power/supply/bq24190_charger.c
> > +++ b/drivers/power/supply/bq24190_charger.c
> > @@ -2018,6 +2018,7 @@ static const struct dev_pm_ops bq24190_pm_ops = {
> > static const struct i2c_device_id bq24190_i2c_ids[] = {
> > { "bq24190" },
> > { "bq24192" },
> > + { "bq24193" },
> > { "bq24192i" },
> > { "bq24196" },
> > { },
> > @@ -2027,6 +2028,7 @@ MODULE_DEVICE_TABLE(i2c, bq24190_i2c_ids);
> > static const struct of_device_id bq24190_of_match[] = {
> > { .compatible = "ti,bq24190", },
> > { .compatible = "ti,bq24192", },
> > + { .compatible = "ti,bq24193", },
> > { .compatible = "ti,bq24192i", },
> > { .compatible = "ti,bq24196", },
>
> We should really stop doing this. All of them are compatible, aren't they?

From what I gather from the different datasheets, the main difference
between them is the maximum current they are able to provide, 1.5 A for
the bq24190 and bq24192i, 3 A for bq24192 and 4.5 A for bq24193. The
default current limit is also detected differently it seems. But yeah,
those are otherwise similar enough to not require anything different in
the driver.

What would be a good way forward for that? Adding a new ti,bq2419x
compatible and switching all devices to this one, as long as they don’t
require anything specific?

>
> Best regards,
> Krzysztof
>

Thanks,

--
Link Mauve

2023-09-05 16:22:51

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH 1/2] power: supply: bq24190: Support bq24193

On Thu, Aug 24, 2023 at 3:28 PM Krzysztof Kozlowski
<[email protected]> wrote:
> On 24/08/2023 15:00, Emmanuel Gil Peyrot wrote:
>
> >>> @@ -2027,6 +2028,7 @@ MODULE_DEVICE_TABLE(i2c, bq24190_i2c_ids);
> >>> static const struct of_device_id bq24190_of_match[] = {
> >>> { .compatible = "ti,bq24190", },
> >>> { .compatible = "ti,bq24192", },
> >>> + { .compatible = "ti,bq24193", },
> >>> { .compatible = "ti,bq24192i", },
> >>> { .compatible = "ti,bq24196", },
> >>
> >> We should really stop doing this. All of them are compatible, aren't they?
> >
> > From what I gather from the different datasheets, the main difference
> > between them is the maximum current they are able to provide, 1.5 A for
> > the bq24190 and bq24192i, 3 A for bq24192 and 4.5 A for bq24193. The
> > default current limit is also detected differently it seems. But yeah,
> > those are otherwise similar enough to not require anything different in
> > the driver.
> >
> > What would be a good way forward for that? Adding a new ti,bq2419x
> > compatible and switching all devices to this one, as long as they don’t
> > require anything specific?
>
> Not a wildcard but any of existing ones, e.g. "ti,bq24196", "ti,bq24190".

We usually encourage people to over-specify the hardware number,
because you never know when you need a quirk and then if you can't
tell them apart you are in a bad place. (But there are exceptions, such
as jedec-nor...)

The differences pointed out (charge current limit etc) can very well
result in different code paths at some point, especially if the charger
interacts with some other component.

Yours,
Linus Walleij