[PATCH 0/6] Introduce MAX77759 charger driver

[PATCH 0/6] Introduce MAX77759 charger driver

[Amit Sunil Dhamne via B4 Relay]

MAX77759 PMIC is used in Pixel 6 and 6 Pro (Oriole/Raven) boards.
One of the functions of the MAX77759 PMIC is a battery charger. This
patchset introduces a driver for this function. One of the unique
features of this charger driver is that it works with a USB input where
the Type-C controller is TCPCI based.

Changes to the board files will follow soon once this patchset is reviewed.

For reference to the MAX77759 MFD based patchset (present in upstream):
https://lore.kernel.org/all/20250509-max77759-mfd-v10-0-962ac15ee3ef@linaro.org/

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
Amit Sunil Dhamne (6):
      dt-bindings: power: supply: Add Maxim MAX77759 charger
      dt-bindings: mfd: maxim,max77759: add charger child node
      dt-bindings: usb: maxim,max33359: Add supply property for VBUS in OTG mode
      mfd: max77759: modify irq configs
      power: supply: max77759: add charger driver
      usb: typec: tcpm/tcpci_maxim: deprecate WAR for setting charger mode

 .../devicetree/bindings/mfd/maxim,max77759.yaml    |  12 +
 .../power/supply/maxim,max77759-charger.yaml       |  36 +
 .../devicetree/bindings/usb/maxim,max33359.yaml    |   4 +
 MAINTAINERS                                        |   7 +
 drivers/mfd/max77759.c                             |  27 +-
 drivers/power/supply/Kconfig                       |  11 +
 drivers/power/supply/Makefile                      |   1 +
 drivers/power/supply/max77759_charger.c            | 866 +++++++++++++++++++++
 drivers/usb/typec/tcpm/tcpci_maxim.h               |   1 +
 drivers/usb/typec/tcpm/tcpci_maxim_core.c          |  48 +-
 include/linux/mfd/max77759.h                       |   9 +
 11 files changed, 999 insertions(+), 23 deletions(-)
---
base-commit: 39f90c1967215375f7d87b81d14b0f3ed6b40c29
change-id: 20251105-max77759-charger-852b626d661a

Best regards,
-- 
Amit Sunil Dhamne <amitsd@google.com>

[PATCH 1/6] dt-bindings: power: supply: Add Maxim MAX77759 charger

[Amit Sunil Dhamne via B4 Relay]

From: Amit Sunil Dhamne <amitsd@google.com>

Add bindings for Maxim max77759 charger device.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
 .../power/supply/maxim,max77759-charger.yaml       | 36 ++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
new file mode 100644
index 000000000000..71f866419774
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
@@ -0,0 +1,36 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/power/supply/maxim,max77759-charger.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Maxim Integrated MAX77759 Battery charger
+
+maintainers:
+  - Amit Sunil Dhamne <amitsd@google.com>
+
+description: |
+  This module is part of the MAX77759 PMIC. For additional information, see
+  Documentation/devicetree/bindings/mfd/maxim,max77759.yaml.
+
+  The Maxim MAX77759 is a dual input switch mode battery charger for portable
+  applications. It supports wired and wireless charging and can operate in buck
+  and boost mode.
+
+allOf:
+  - $ref: power-supply.yaml#
+
+properties:
+  compatible:
+    const: maxim,max77759-charger
+
+  usb-otg-vbus-regulator:
+    type: object
+    description: Provides Boost for sourcing VBUS.
+    $ref: /schemas/regulator/regulator.yaml#
+    unevaluatedProperties: false
+
+required:
+  - compatible
+
+unevaluatedProperties: false

-- 
2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH 1/6] dt-bindings: power: supply: Add Maxim MAX77759 charger

[Krzysztof Kozlowski]

On 23/11/2025 09:35, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
> 
> Add bindings for Maxim max77759 charger device.
> 
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
>  .../power/supply/maxim,max77759-charger.yaml       | 36 ++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
> new file mode 100644
> index 000000000000..71f866419774
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
> @@ -0,0 +1,36 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/supply/maxim,max77759-charger.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Maxim Integrated MAX77759 Battery charger
> +
> +maintainers:
> +  - Amit Sunil Dhamne <amitsd@google.com>
> +
> +description: |
> +  This module is part of the MAX77759 PMIC. For additional information, see
> +  Documentation/devicetree/bindings/mfd/maxim,max77759.yaml.
> +
> +  The Maxim MAX77759 is a dual input switch mode battery charger for portable
> +  applications. It supports wired and wireless charging and can operate in buck
> +  and boost mode.
> +
> +allOf:
> +  - $ref: power-supply.yaml#
> +
> +properties:
> +  compatible:
> +    const: maxim,max77759-charger
> +

This should be just folded into parent node, no need for separate
charger device or is just incomplete.

> +  usb-otg-vbus-regulator:
> +    type: object
> +    description: Provides Boost for sourcing VBUS.
> +    $ref: /schemas/regulator/regulator.yaml#
> +    unevaluatedProperties: false
> +

Best regards,
Krzysztof

Re: [PATCH 1/6] dt-bindings: power: supply: Add Maxim MAX77759 charger

[Amit Sunil Dhamne]

Hi Krzysztof,

On 11/23/25 1:28 AM, Krzysztof Kozlowski wrote:
> On 23/11/2025 09:35, Amit Sunil Dhamne via B4 Relay wrote:
>> From: Amit Sunil Dhamne <amitsd@google.com>
>>
>> Add bindings for Maxim max77759 charger device.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>>  .../power/supply/maxim,max77759-charger.yaml       | 36 ++++++++++++++++++++++
>>  1 file changed, 36 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
>> new file mode 100644
>> index 000000000000..71f866419774
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
>> @@ -0,0 +1,36 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/power/supply/maxim,max77759-charger.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Maxim Integrated MAX77759 Battery charger
>> +
>> +maintainers:
>> +  - Amit Sunil Dhamne <amitsd@google.com>
>> +
>> +description: |
>> +  This module is part of the MAX77759 PMIC. For additional information, see
>> +  Documentation/devicetree/bindings/mfd/maxim,max77759.yaml.
>> +
>> +  The Maxim MAX77759 is a dual input switch mode battery charger for portable
>> +  applications. It supports wired and wireless charging and can operate in buck
>> +  and boost mode.
>> +
>> +allOf:
>> +  - $ref: power-supply.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: maxim,max77759-charger
>> +
> This should be just folded into parent node, no need for separate
> charger device or is just incomplete.

Thanks for the review! You are right, the binding is incomplete. This
charger block actually listens on its own I2C address, distinct from the
main PMIC.

I will update v2 to include the reg property. I will also add the
standard properties `constant-charge-current-max-microamp` and
`constant-charge-voltage-max-microvolt` to configure the hardware
limits, as this charger device does not manage the battery profile
directly (that is handled by a separate fuel gauge).


Thanks,

Amit

>
>> +  usb-otg-vbus-regulator:
>> +    type: object
>> +    description: Provides Boost for sourcing VBUS.
>> +    $ref: /schemas/regulator/regulator.yaml#
>> +    unevaluatedProperties: false
>> +
> Best regards,
> Krzysztof

Re: [PATCH 1/6] dt-bindings: power: supply: Add Maxim MAX77759 charger

[Krzysztof Kozlowski]

On Sun, Nov 23, 2025 at 06:34:05PM -0800, Amit Sunil Dhamne wrote:
> Hi Krzysztof,
> 
> On 11/23/25 1:28 AM, Krzysztof Kozlowski wrote:
> > On 23/11/2025 09:35, Amit Sunil Dhamne via B4 Relay wrote:
> >> From: Amit Sunil Dhamne <amitsd@google.com>
> >>
> >> Add bindings for Maxim max77759 charger device.
> >>
> >> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> >> ---
> >>  .../power/supply/maxim,max77759-charger.yaml       | 36 ++++++++++++++++++++++
> >>  1 file changed, 36 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
> >> new file mode 100644
> >> index 000000000000..71f866419774
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
> >> @@ -0,0 +1,36 @@
> >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/power/supply/maxim,max77759-charger.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Maxim Integrated MAX77759 Battery charger
> >> +
> >> +maintainers:
> >> +  - Amit Sunil Dhamne <amitsd@google.com>
> >> +
> >> +description: |
> >> +  This module is part of the MAX77759 PMIC. For additional information, see
> >> +  Documentation/devicetree/bindings/mfd/maxim,max77759.yaml.
> >> +
> >> +  The Maxim MAX77759 is a dual input switch mode battery charger for portable
> >> +  applications. It supports wired and wireless charging and can operate in buck
> >> +  and boost mode.
> >> +
> >> +allOf:
> >> +  - $ref: power-supply.yaml#
> >> +
> >> +properties:
> >> +  compatible:
> >> +    const: maxim,max77759-charger
> >> +
> > This should be just folded into parent node, no need for separate
> > charger device or is just incomplete.
> 
> Thanks for the review! You are right, the binding is incomplete. This
> charger block actually listens on its own I2C address, distinct from the
> main PMIC.
> 
> I will update v2 to include the reg property. I will also add the

AFAIK, the main (parent) device schema does not reference children via
any sort of addressing, so reg here would not be suitable.

> standard properties `constant-charge-current-max-microamp` and
> `constant-charge-voltage-max-microvolt` to configure the hardware
> limits, as this charger device does not manage the battery profile
> directly (that is handled by a separate fuel gauge).

Well, still, what's the benefit for the bindings to have it as a
separate child? Kind of depends on your example, which is quite small -
one regulator and supply. Grow the example with battery and other
independent resources (if they are) to justify it. Or show arguments why
this is re-usable.

Best regards,
Krzysztof

Re: [PATCH 1/6] dt-bindings: power: supply: Add Maxim MAX77759 charger

[Amit Sunil Dhamne]

On 11/25/25 1:56 AM, Krzysztof Kozlowski wrote:
> On Sun, Nov 23, 2025 at 06:34:05PM -0800, Amit Sunil Dhamne wrote:
>> Hi Krzysztof,
>>
>> On 11/23/25 1:28 AM, Krzysztof Kozlowski wrote:
>>> On 23/11/2025 09:35, Amit Sunil Dhamne via B4 Relay wrote:
>>>> From: Amit Sunil Dhamne <amitsd@google.com>
>>>>
>>>> Add bindings for Maxim max77759 charger device.
>>>>
>>>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>>>> ---
>>>>   .../power/supply/maxim,max77759-charger.yaml       | 36 ++++++++++++++++++++++
>>>>   1 file changed, 36 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
>>>> new file mode 100644
>>>> index 000000000000..71f866419774
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
>>>> @@ -0,0 +1,36 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/power/supply/maxim,max77759-charger.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: Maxim Integrated MAX77759 Battery charger
>>>> +
>>>> +maintainers:
>>>> +  - Amit Sunil Dhamne <amitsd@google.com>
>>>> +
>>>> +description: |
>>>> +  This module is part of the MAX77759 PMIC. For additional information, see
>>>> +  Documentation/devicetree/bindings/mfd/maxim,max77759.yaml.
>>>> +
>>>> +  The Maxim MAX77759 is a dual input switch mode battery charger for portable
>>>> +  applications. It supports wired and wireless charging and can operate in buck
>>>> +  and boost mode.
>>>> +
>>>> +allOf:
>>>> +  - $ref: power-supply.yaml#
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: maxim,max77759-charger
>>>> +
>>> This should be just folded into parent node, no need for separate
>>> charger device or is just incomplete.
>> Thanks for the review! You are right, the binding is incomplete. This
>> charger block actually listens on its own I2C address, distinct from the
>> main PMIC.
>>
>> I will update v2 to include the reg property. I will also add the
> AFAIK, the main (parent) device schema does not reference children via
> any sort of addressing, so reg here would not be suitable.

I agree that currently nvmem and gpio devices (which are children of 
PMIC device) are not referenced using any address. But I was guessing 
that's because they share the i2c client id with the PMIC and sharing 
its address space (implied).

The charger device while being part of the MAX77759 PMIC package has 
it's own i2c client id and address space that's why I proposed "reg". 
The underlying assumption I made was separate client id implies that a 
"reg" property required. But maybe that's incorrect.

I can understand the argument against having a "reg" property. As the 
i2c client id will remain same for a max77759 charger device (as it's a 
chip property and not a board property) it will always remain a 
constant. I will drop the "reg" proposal.


>
>> standard properties `constant-charge-current-max-microamp` and
>> `constant-charge-voltage-max-microvolt` to configure the hardware
>> limits, as this charger device does not manage the battery profile
>> directly (that is handled by a separate fuel gauge).
> Well, still, what's the benefit for the bindings to have it as a
> separate child? Kind of depends on your example, which is quite small -
> one regulator and supply. Grow the example with battery and other
> independent resources (if they are) to justify it. Or show arguments why
> this is re-usable.

The primary reasons for keeping the charger as a distinct child node are 
to model the hardware topology for the power supply subsystem and to 
house the OTG regulator provided by the charger block.
The charger needs to be referenced by the Fuel Gauge (which handles the 
battery profile) via power-supplies. Additionally, the charger block 
provides a regulator for USB OTG VBUS, which is cleaner to represent as 
a child node of the charger rather than mixing it into the top-level 
PMIC node.

The final goal is to correctly represent the hardware connections so 
that I can use it for [2]. The dts would ideally look like this:

```

maxtcpc: usb-typec@25 {
         compatible = "maxim,max77759-tcpci";
         ...
         otg-vbus-supply = <&otg_vbus_reg>;
};

pmic@66 {

         compatible = "maxim,max77759";
         ....
         chg: charger {
                 compatible = "maxim,max77759-charger";
                 power-supplies = <&maxtcpc>;
                 otg_vbus_reg: usb-otg-vbus-regulator {
                         regulator-name = "usb-otg-vbus"

                 };

         };

};

battery: battery {
     compatible = "simple-battery";
     ....

};

fuel-guage@36 {
         compatible = "maxim,max77759-fg";
         ....
         power-supplies = <&chg>;
         monitored-battery = <&battery>;

};

```

[1] 
https://lore.kernel.org/all/20250915-b4-gs101_max77759_fg-v6-0-31d08581500f@uclouvain.be/

[2] 
https://lore.kernel.org/all/20250507-batt_ops-v2-0-8d06130bffe6@google.com/


BR,

Amit

>
> Best regards,
> Krzysztof
>

Re: [PATCH 1/6] dt-bindings: power: supply: Add Maxim MAX77759 charger

[Krzysztof Kozlowski]

On 26/11/2025 00:48, Amit Sunil Dhamne wrote:
> 
> On 11/25/25 1:56 AM, Krzysztof Kozlowski wrote:
>> On Sun, Nov 23, 2025 at 06:34:05PM -0800, Amit Sunil Dhamne wrote:
>>> Hi Krzysztof,
>>>
>>> On 11/23/25 1:28 AM, Krzysztof Kozlowski wrote:
>>>> On 23/11/2025 09:35, Amit Sunil Dhamne via B4 Relay wrote:
>>>>> From: Amit Sunil Dhamne <amitsd@google.com>
>>>>>
>>>>> Add bindings for Maxim max77759 charger device.
>>>>>
>>>>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>>>>> ---
>>>>>   .../power/supply/maxim,max77759-charger.yaml       | 36 ++++++++++++++++++++++
>>>>>   1 file changed, 36 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..71f866419774
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
>>>>> @@ -0,0 +1,36 @@
>>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/power/supply/maxim,max77759-charger.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Maxim Integrated MAX77759 Battery charger
>>>>> +
>>>>> +maintainers:
>>>>> +  - Amit Sunil Dhamne <amitsd@google.com>
>>>>> +
>>>>> +description: |
>>>>> +  This module is part of the MAX77759 PMIC. For additional information, see
>>>>> +  Documentation/devicetree/bindings/mfd/maxim,max77759.yaml.
>>>>> +
>>>>> +  The Maxim MAX77759 is a dual input switch mode battery charger for portable
>>>>> +  applications. It supports wired and wireless charging and can operate in buck
>>>>> +  and boost mode.
>>>>> +
>>>>> +allOf:
>>>>> +  - $ref: power-supply.yaml#
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: maxim,max77759-charger
>>>>> +
>>>> This should be just folded into parent node, no need for separate
>>>> charger device or is just incomplete.
>>> Thanks for the review! You are right, the binding is incomplete. This
>>> charger block actually listens on its own I2C address, distinct from the
>>> main PMIC.
>>>
>>> I will update v2 to include the reg property. I will also add the
>> AFAIK, the main (parent) device schema does not reference children via
>> any sort of addressing, so reg here would not be suitable.
> 
> I agree that currently nvmem and gpio devices (which are children of 
> PMIC device) are not referenced using any address. But I was guessing 
> that's because they share the i2c client id with the PMIC and sharing 
> its address space (implied).
> 
> The charger device while being part of the MAX77759 PMIC package has 
> it's own i2c client id and address space that's why I proposed "reg". 
> The underlying assumption I made was separate client id implies that a 
> "reg" property required. But maybe that's incorrect.
> 
> I can understand the argument against having a "reg" property. As the 
> i2c client id will remain same for a max77759 charger device (as it's a 
> chip property and not a board property) it will always remain a 
> constant. I will drop the "reg" proposal.
> 
> 
>>
>>> standard properties `constant-charge-current-max-microamp` and
>>> `constant-charge-voltage-max-microvolt` to configure the hardware
>>> limits, as this charger device does not manage the battery profile
>>> directly (that is handled by a separate fuel gauge).
>> Well, still, what's the benefit for the bindings to have it as a
>> separate child? Kind of depends on your example, which is quite small -
>> one regulator and supply. Grow the example with battery and other
>> independent resources (if they are) to justify it. Or show arguments why
>> this is re-usable.
> 
> The primary reasons for keeping the charger as a distinct child node are 
> to model the hardware topology for the power supply subsystem and to 

You do not need children for that at all.

> house the OTG regulator provided by the charger block.
> The charger needs to be referenced by the Fuel Gauge (which handles the 
> battery profile) via power-supplies. Additionally, the charger block 
> provides a regulator for USB OTG VBUS, which is cleaner to represent as 
> a child node of the charger rather than mixing it into the top-level 
> PMIC node.

Sorry but argument that you need a child device to be able to construct
a phandle is just wrong. You can create phandles on every other way as well.


Best regards,
Krzysztof

Re: [PATCH 1/6] dt-bindings: power: supply: Add Maxim MAX77759 charger

[Amit Sunil Dhamne]

On 12/2/25 5:00 AM, Krzysztof Kozlowski wrote:
> On 26/11/2025 00:48, Amit Sunil Dhamne wrote:
>> On 11/25/25 1:56 AM, Krzysztof Kozlowski wrote:
>>> On Sun, Nov 23, 2025 at 06:34:05PM -0800, Amit Sunil Dhamne wrote:
>>>> Hi Krzysztof,
>>>>
>>>> On 11/23/25 1:28 AM, Krzysztof Kozlowski wrote:
>>>>> On 23/11/2025 09:35, Amit Sunil Dhamne via B4 Relay wrote:
>>>>>> From: Amit Sunil Dhamne <amitsd@google.com>
>>>>>>
>>>>>> Add bindings for Maxim max77759 charger device.
>>>>>>
>>>>>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>>>>>> ---
>>>>>>    .../power/supply/maxim,max77759-charger.yaml       | 36 ++++++++++++++++++++++
>>>>>>    1 file changed, 36 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..71f866419774
>>>>>> --- /dev/null
>>>>>> +++ b/Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
>>>>>> @@ -0,0 +1,36 @@
>>>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id: http://devicetree.org/schemas/power/supply/maxim,max77759-charger.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Maxim Integrated MAX77759 Battery charger
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Amit Sunil Dhamne <amitsd@google.com>
>>>>>> +
>>>>>> +description: |
>>>>>> +  This module is part of the MAX77759 PMIC. For additional information, see
>>>>>> +  Documentation/devicetree/bindings/mfd/maxim,max77759.yaml.
>>>>>> +
>>>>>> +  The Maxim MAX77759 is a dual input switch mode battery charger for portable
>>>>>> +  applications. It supports wired and wireless charging and can operate in buck
>>>>>> +  and boost mode.
>>>>>> +
>>>>>> +allOf:
>>>>>> +  - $ref: power-supply.yaml#
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    const: maxim,max77759-charger
>>>>>> +
>>>>> This should be just folded into parent node, no need for separate
>>>>> charger device or is just incomplete.
>>>> Thanks for the review! You are right, the binding is incomplete. This
>>>> charger block actually listens on its own I2C address, distinct from the
>>>> main PMIC.
>>>>
>>>> I will update v2 to include the reg property. I will also add the
>>> AFAIK, the main (parent) device schema does not reference children via
>>> any sort of addressing, so reg here would not be suitable.
>> I agree that currently nvmem and gpio devices (which are children of
>> PMIC device) are not referenced using any address. But I was guessing
>> that's because they share the i2c client id with the PMIC and sharing
>> its address space (implied).
>>
>> The charger device while being part of the MAX77759 PMIC package has
>> it's own i2c client id and address space that's why I proposed "reg".
>> The underlying assumption I made was separate client id implies that a
>> "reg" property required. But maybe that's incorrect.
>>
>> I can understand the argument against having a "reg" property. As the
>> i2c client id will remain same for a max77759 charger device (as it's a
>> chip property and not a board property) it will always remain a
>> constant. I will drop the "reg" proposal.
>>
>>
>>>> standard properties `constant-charge-current-max-microamp` and
>>>> `constant-charge-voltage-max-microvolt` to configure the hardware
>>>> limits, as this charger device does not manage the battery profile
>>>> directly (that is handled by a separate fuel gauge).
>>> Well, still, what's the benefit for the bindings to have it as a
>>> separate child? Kind of depends on your example, which is quite small -
>>> one regulator and supply. Grow the example with battery and other
>>> independent resources (if they are) to justify it. Or show arguments why
>>> this is re-usable.
>> The primary reasons for keeping the charger as a distinct child node are
>> to model the hardware topology for the power supply subsystem and to
> You do not need children for that at all.

Actually what you said makes sense. I will fold the charger's schema 
into mfd/maxim,max77759's schema.


Thanks,

Amit

>> house the OTG regulator provided by the charger block.
>> The charger needs to be referenced by the Fuel Gauge (which handles the
>> battery profile) via power-supplies. Additionally, the charger block
>> provides a regulator for USB OTG VBUS, which is cleaner to represent as
>> a child node of the charger rather than mixing it into the top-level
>> PMIC node.
> Sorry but argument that you need a child device to be able to construct
> a phandle is just wrong. You can create phandles on every other way as well.
>
>
> Best regards,
> Krzysztof

[PATCH 2/6] dt-bindings: mfd: maxim,max77759: add charger child node

[Amit Sunil Dhamne via B4 Relay]

From: Amit Sunil Dhamne <amitsd@google.com>

The Maxim MAX77759 MFD includes a charger function, hence add its
binding as a property. Also, update the example to include charger.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
 Documentation/devicetree/bindings/mfd/maxim,max77759.yaml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
index 525de9ab3c2b..29132f73f2c8 100644
--- a/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
+++ b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
@@ -37,6 +37,9 @@ properties:
   nvmem-0:
     $ref: /schemas/nvmem/maxim,max77759-nvmem.yaml
 
+  charger:
+    $ref: /schemas/power/supply/maxim,max77759-charger.yaml
+
 required:
   - compatible
   - interrupts
@@ -95,5 +98,14 @@ examples:
                     };
                 };
             };
+
+            charger {
+                compatible = "maxim,max77759-charger";
+                power-supplies = <&maxtcpc>;
+
+                usb-otg-vbus-regulator {
+                    regulator-name = "usb-otg-vbus";
+                };
+            };
         };
     };

-- 
2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH 2/6] dt-bindings: mfd: maxim,max77759: add charger child node

[Krzysztof Kozlowski]

On 23/11/2025 09:35, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
> 
> The Maxim MAX77759 MFD includes a charger function, hence add its
> binding as a property. Also, update the example to include charger.
> 
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
>  Documentation/devicetree/bindings/mfd/maxim,max77759.yaml | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
> index 525de9ab3c2b..29132f73f2c8 100644
> --- a/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
> +++ b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
> @@ -37,6 +37,9 @@ properties:
>    nvmem-0:
>      $ref: /schemas/nvmem/maxim,max77759-nvmem.yaml
>  
> +  charger:
> +    $ref: /schemas/power/supply/maxim,max77759-charger.yaml

You need to explain dependencies/merging in the cover letter. This patch
now cannot be merged alone through MFD.

Or just decouple the dependency and list here only compatible, assuming
this child node even stays.

> +
>  required:
>    - compatible
>    - interrupts
> @@ -95,5 +98,14 @@ examples:
>                      };
>                  };
>              };
> +
> +            charger {
> +                compatible = "maxim,max77759-charger";
> +                power-supplies = <&maxtcpc>;

Feels like you miss here battery.


Best regards,
Krzysztof

Re: [PATCH 2/6] dt-bindings: mfd: maxim,max77759: add charger child node

[Amit Sunil Dhamne]

On 11/23/25 1:30 AM, Krzysztof Kozlowski wrote:
> On 23/11/2025 09:35, Amit Sunil Dhamne via B4 Relay wrote:
>> From: Amit Sunil Dhamne <amitsd@google.com>
>>
>> The Maxim MAX77759 MFD includes a charger function, hence add its
>> binding as a property. Also, update the example to include charger.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>>   Documentation/devicetree/bindings/mfd/maxim,max77759.yaml | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
>> index 525de9ab3c2b..29132f73f2c8 100644
>> --- a/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/maxim,max77759.yaml
>> @@ -37,6 +37,9 @@ properties:
>>     nvmem-0:
>>       $ref: /schemas/nvmem/maxim,max77759-nvmem.yaml
>>   
>> +  charger:
>> +    $ref: /schemas/power/supply/maxim,max77759-charger.yaml
> You need to explain dependencies/merging in the cover letter. This patch
> now cannot be merged alone through MFD.
>
> Or just decouple the dependency and list here only compatible, assuming
> this child node even stays.

Would it be okay if I drop this patch from this series and re-send it 
while mentioning the dependency?


>> +
>>   required:
>>     - compatible
>>     - interrupts
>> @@ -95,5 +98,14 @@ examples:
>>                       };
>>                   };
>>               };
>> +
>> +            charger {
>> +                compatible = "maxim,max77759-charger";
>> +                power-supplies = <&maxtcpc>;
> Feels like you miss here battery.

I have added the example in [1]. We can discuss there in case there are 
further concerns.

[1] 
https://lore.kernel.org/all/7ad91325-e881-461d-b39e-6ff15d98b3c5@google.com/T/#u


BR,

Amit

>
>
> Best regards,
> Krzysztof

[PATCH 3/6] dt-bindings: usb: maxim,max33359: Add supply property for VBUS in OTG mode

[Amit Sunil Dhamne via B4 Relay]

From: Amit Sunil Dhamne <amitsd@google.com>

Add a regulator supply property for VBUS when usb is in OTG mode.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
---
 Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
index 3de4dc40b791..a529f18c4918 100644
--- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
+++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
@@ -32,6 +32,9 @@ properties:
     description:
       Properties for usb c connector.
 
+  otg-vbus-supply:
+    description: Regulator to control OTG VBUS supply.
+
 required:
   - compatible
   - reg
@@ -53,6 +56,7 @@ examples:
             reg = <0x25>;
             interrupt-parent = <&gpa8>;
             interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
+            otg-vbus-supply = <&otg_vbus_reg>;
 
             connector {
                 compatible = "usb-c-connector";

-- 
2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH 3/6] dt-bindings: usb: maxim,max33359: Add supply property for VBUS in OTG mode

[Krzysztof Kozlowski]

On Sun, Nov 23, 2025 at 08:35:50AM +0000, Amit Sunil Dhamne wrote:
> Add a regulator supply property for VBUS when usb is in OTG mode.
> 
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
>  Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> index 3de4dc40b791..a529f18c4918 100644
> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> @@ -32,6 +32,9 @@ properties:
>      description:
>        Properties for usb c connector.
>  
> +  otg-vbus-supply:

How is the pin or supply called in the datasheet?

Best regards,
Krzysztof

Re: [PATCH 3/6] dt-bindings: usb: maxim,max33359: Add supply property for VBUS in OTG mode

[Amit Sunil Dhamne]

Hi Krzysztof,

On 11/23/25 11:53 PM, Krzysztof Kozlowski wrote:
> On Sun, Nov 23, 2025 at 08:35:50AM +0000, Amit Sunil Dhamne wrote:
>> Add a regulator supply property for VBUS when usb is in OTG mode.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
>> ---
>>   Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> index 3de4dc40b791..a529f18c4918 100644
>> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> @@ -32,6 +32,9 @@ properties:
>>       description:
>>         Properties for usb c connector.
>>   
>> +  otg-vbus-supply:
> How is the pin or supply called in the datasheet?

The pin that supplies the VBUS power in OTG is referred to as Vchgin in 
the datasheet.

>
> Best regards,
> Krzysztof
>

Re: [PATCH 3/6] dt-bindings: usb: maxim,max33359: Add supply property for VBUS in OTG mode

[André Draszik]

On Tue, 2025-11-25 at 12:13 -0800, Amit Sunil Dhamne wrote:
> Hi Krzysztof,
> 
> On 11/23/25 11:53 PM, Krzysztof Kozlowski wrote:
> > On Sun, Nov 23, 2025 at 08:35:50AM +0000, Amit Sunil Dhamne wrote:
> > > Add a regulator supply property for VBUS when usb is in OTG mode.
> > > 
> > > Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> > > Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> > > ---
> > >   Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> > > b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> > > index 3de4dc40b791..a529f18c4918 100644
> > > --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> > > @@ -32,6 +32,9 @@ properties:
> > >       description:
> > >         Properties for usb c connector.
> > >   
> > > +  otg-vbus-supply:
> > How is the pin or supply called in the datasheet?
> 
> The pin that supplies the VBUS power in OTG is referred to as Vchgin in 

I think that should be chgin (without V prefix)

> the datasheet.

Cheers,
Andre'

Re: [PATCH 3/6] dt-bindings: usb: maxim,max33359: Add supply property for VBUS in OTG mode

[Amit Sunil Dhamne]

On 11/26/25 8:18 AM, André Draszik wrote:
> On Tue, 2025-11-25 at 12:13 -0800, Amit Sunil Dhamne wrote:
>> Hi Krzysztof,
>>
>> On 11/23/25 11:53 PM, Krzysztof Kozlowski wrote:
>>> On Sun, Nov 23, 2025 at 08:35:50AM +0000, Amit Sunil Dhamne wrote:
>>>> Add a regulator supply property for VBUS when usb is in OTG mode.
>>>>
>>>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>>>> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
>>>> ---
>>>>    Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 4 ++++
>>>>    1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>>>> b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>>>> index 3de4dc40b791..a529f18c4918 100644
>>>> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>>>> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>>>> @@ -32,6 +32,9 @@ properties:
>>>>        description:
>>>>          Properties for usb c connector.
>>>>    
>>>> +  otg-vbus-supply:
>>> How is the pin or supply called in the datasheet?
>> The pin that supplies the VBUS power in OTG is referred to as Vchgin in
> I think that should be chgin (without V prefix)

Right, it's just CHGIN. These CHGIN pins source the USB VBUS power in 
OTG mode.

>
>> the datasheet.
> Cheers,
> Andre'

Re: [PATCH 3/6] dt-bindings: usb: maxim,max33359: Add supply property for VBUS in OTG mode

[Heikki Krogerus]

Sun, Nov 23, 2025 at 08:35:50AM +0000, Amit Sunil Dhamne via B4 Relay kirjoitti:
> From: Amit Sunil Dhamne <amitsd@google.com>
> 
> Add a regulator supply property for VBUS when usb is in OTG mode.

What is "OTG mode"?

OTG is usually used to refer to the USB in device role, even though the
specification actually defines OTG device as a device capable of both
host and device roles. So the term was confusing already before.
Nevertheless, the emphasis is always on data-role, _not_ power-role.

Here it seems MAX33359 uses the term OTG as a synonym for "source", so
power-role?

Please don't use the term OTG unless you really have to - it's too
confusing. I know the MAX33359 datasheet uses it, but what you really
do here is regulate VBUS. So please:

        s/otg-vbus/vbus/

thanks,

> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
> ---
>  Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> index 3de4dc40b791..a529f18c4918 100644
> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> @@ -32,6 +32,9 @@ properties:
>      description:
>        Properties for usb c connector.
>  
> +  otg-vbus-supply:
> +    description: Regulator to control OTG VBUS supply.
> +
>  required:
>    - compatible
>    - reg
> @@ -53,6 +56,7 @@ examples:
>              reg = <0x25>;
>              interrupt-parent = <&gpa8>;
>              interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
> +            otg-vbus-supply = <&otg_vbus_reg>;
>  
>              connector {
>                  compatible = "usb-c-connector";
> 
> -- 
> 2.52.0.rc2.455.g230fcf2819-goog
> 

-- 
heikki

Re: [PATCH 3/6] dt-bindings: usb: maxim,max33359: Add supply property for VBUS in OTG mode

[Amit Sunil Dhamne]

Hi Heikki,

On 11/26/25 2:01 AM, Heikki Krogerus wrote:
> Sun, Nov 23, 2025 at 08:35:50AM +0000, Amit Sunil Dhamne via B4 Relay kirjoitti:
>> From: Amit Sunil Dhamne <amitsd@google.com>
>>
>> Add a regulator supply property for VBUS when usb is in OTG mode.
> What is "OTG mode"?
>
> OTG is usually used to refer to the USB in device role, even though the
> specification actually defines OTG device as a device capable of both
> host and device roles. So the term was confusing already before.
> Nevertheless, the emphasis is always on data-role, _not_ power-role.

Thanks for the insight!


>
> Here it seems MAX33359 uses the term OTG as a synonym for "source", so
> power-role?

Essentially. The datasheet refers to the mode where VBUS is sourced as 
OTG mode.


> Please don't use the term OTG unless you really have to - it's too
> confusing. I know the MAX33359 datasheet uses it, but what you really
> do here is regulate VBUS. So please:
>
>          s/otg-vbus/vbus/

I will drop OTG term at least in the USB world and restrict it to the 
charger driver.


BR,

Amit

>
> thanks,
>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> Reviewed-by: Badhri Jagan Sridharan <badhri@google.com>
>> ---
>>   Documentation/devicetree/bindings/usb/maxim,max33359.yaml | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> index 3de4dc40b791..a529f18c4918 100644
>> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> @@ -32,6 +32,9 @@ properties:
>>       description:
>>         Properties for usb c connector.
>>   
>> +  otg-vbus-supply:
>> +    description: Regulator to control OTG VBUS supply.
>> +
>>   required:
>>     - compatible
>>     - reg
>> @@ -53,6 +56,7 @@ examples:
>>               reg = <0x25>;
>>               interrupt-parent = <&gpa8>;
>>               interrupts = <2 IRQ_TYPE_LEVEL_LOW>;
>> +            otg-vbus-supply = <&otg_vbus_reg>;
>>   
>>               connector {
>>                   compatible = "usb-c-connector";
>>
>> -- 
>> 2.52.0.rc2.455.g230fcf2819-goog
>>

[PATCH 4/6] mfd: max77759: modify irq configs

[Amit Sunil Dhamne via B4 Relay]

From: Amit Sunil Dhamne <amitsd@google.com>

Define specific bit-level masks for charger's registers and modify the
irq mask for charger irq_chip. Also, configure the max77759 interrupt
lines as active low to all interrupt registrations to ensure the
interrupt controllers are configured with the correct trigger type.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
 drivers/mfd/max77759.c       | 24 +++++++++++++++++-------
 include/linux/mfd/max77759.h |  9 +++++++++
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
index 6cf6306c4a3b..5fe22884f362 100644
--- a/drivers/mfd/max77759.c
+++ b/drivers/mfd/max77759.c
@@ -256,8 +256,17 @@ static const struct regmap_irq max77759_topsys_irqs[] = {
 };
 
 static const struct regmap_irq max77759_chgr_irqs[] = {
-	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0, GENMASK(7, 0)),
-	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1, GENMASK(7, 0)),
+	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0,
+		       MAX77759_CHGR_REG_CHG_INT_AICL |
+		       MAX77759_CHGR_REG_CHG_INT_CHGIN |
+		       MAX77759_CHGR_REG_CHG_INT_CHG |
+		       MAX77759_CHGR_REG_CHG_INT_INLIM),
+	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1,
+		       MAX77759_CHGR_REG_CHG_INT2_BAT_OILO |
+		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC |
+		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV |
+		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO |
+		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE),
 };
 
 static const struct regmap_irq_chip max77759_pmic_irq_chip = {
@@ -486,8 +495,8 @@ static int max77759_add_chained_irq_chip(struct device *dev,
 				     "failed to get parent vIRQ(%d) for chip %s\n",
 				     pirq, chip->name);
 
-	ret = devm_regmap_add_irq_chip(dev, regmap, irq,
-				       IRQF_ONESHOT | IRQF_SHARED, 0, chip,
+	ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT |
+				       IRQF_SHARED | IRQF_TRIGGER_LOW, 0, chip,
 				       data);
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to add %s IRQ chip\n",
@@ -519,8 +528,9 @@ static int max77759_add_chained_maxq(struct i2c_client *client,
 
 	ret = devm_request_threaded_irq(&client->dev, apcmdres_irq,
 					NULL, apcmdres_irq_handler,
-					IRQF_ONESHOT | IRQF_SHARED,
-					dev_name(&client->dev), max77759);
+					IRQF_ONESHOT | IRQF_SHARED |
+					IRQF_TRIGGER_LOW, dev_name(&client->dev),
+					max77759);
 	if (ret)
 		return dev_err_probe(&client->dev, ret,
 				     "MAX77759_MAXQ_INT_APCMDRESI failed\n");
@@ -633,7 +643,7 @@ static int max77759_probe(struct i2c_client *client)
 		return dev_err_probe(&client->dev, -EINVAL,
 				     "invalid IRQ: %d\n", client->irq);
 
-	irq_flags = IRQF_ONESHOT | IRQF_SHARED;
+	irq_flags = IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW;
 	irq_flags |= irqd_get_trigger_type(irq_data);
 
 	ret = devm_regmap_add_irq_chip(&client->dev, max77759->regmap_top,
diff --git a/include/linux/mfd/max77759.h b/include/linux/mfd/max77759.h
index c6face34e385..0ef29a48deec 100644
--- a/include/linux/mfd/max77759.h
+++ b/include/linux/mfd/max77759.h
@@ -62,7 +62,16 @@
 #define MAX77759_CHGR_REG_CHG_INT               0xb0
 #define MAX77759_CHGR_REG_CHG_INT2              0xb1
 #define MAX77759_CHGR_REG_CHG_INT_MASK          0xb2
+#define MAX77759_CHGR_REG_CHG_INT_AICL          BIT(7)
+#define MAX77759_CHGR_REG_CHG_INT_CHGIN         BIT(6)
+#define MAX77759_CHGR_REG_CHG_INT_CHG           BIT(4)
+#define MAX77759_CHGR_REG_CHG_INT_INLIM         BIT(2)
 #define MAX77759_CHGR_REG_CHG_INT2_MASK         0xb3
+#define MAX77759_CHGR_REG_CHG_INT2_BAT_OILO     BIT(4)
+#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC   BIT(3)
+#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV   BIT(2)
+#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO   BIT(1)
+#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE BIT(0)
 #define MAX77759_CHGR_REG_CHG_INT_OK            0xb4
 #define MAX77759_CHGR_REG_CHG_DETAILS_00        0xb5
 #define MAX77759_CHGR_REG_CHG_DETAILS_01        0xb6

-- 
2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH 4/6] mfd: max77759: modify irq configs

[André Draszik]

Hi Amit,

Thanks for your patches to enable the charger!

> From: Amit Sunil Dhamne <amitsd@google.com>
> 
> Define specific bit-level masks for charger's registers and modify the
> irq mask for charger irq_chip. Also, configure the max77759 interrupt
> lines as active low to all interrupt registrations to ensure the
> interrupt controllers are configured with the correct trigger type.
> 
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
>  drivers/mfd/max77759.c       | 24 +++++++++++++++++-------
>  include/linux/mfd/max77759.h |  9 +++++++++
>  2 files changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
> index 6cf6306c4a3b..5fe22884f362 100644
> --- a/drivers/mfd/max77759.c
> +++ b/drivers/mfd/max77759.c
> @@ -256,8 +256,17 @@ static const struct regmap_irq max77759_topsys_irqs[] = {
>  };
>  
>  static const struct regmap_irq max77759_chgr_irqs[] = {
> -	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0, GENMASK(7, 0)),
> -	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1, GENMASK(7, 0)),
> +	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0,
> +		       MAX77759_CHGR_REG_CHG_INT_AICL |
> +		       MAX77759_CHGR_REG_CHG_INT_CHGIN |
> +		       MAX77759_CHGR_REG_CHG_INT_CHG |
> +		       MAX77759_CHGR_REG_CHG_INT_INLIM),
> +	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1,
> +		       MAX77759_CHGR_REG_CHG_INT2_BAT_OILO |
> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC |
> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV |
> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO |
> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE),
>  };
>  
>  static const struct regmap_irq_chip max77759_pmic_irq_chip = {
> @@ -486,8 +495,8 @@ static int max77759_add_chained_irq_chip(struct device *dev,
>  				     "failed to get parent vIRQ(%d) for chip %s\n",
>  				     pirq, chip->name);
>  
> -	ret = devm_regmap_add_irq_chip(dev, regmap, irq,
> -				       IRQF_ONESHOT | IRQF_SHARED, 0, chip,
> +	ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT |
> +				       IRQF_SHARED | IRQF_TRIGGER_LOW, 0, chip,
>  				       data);

Please correct me if I'm wrong, but I don't think this makes sense for a
chained IRQ in this case. What problem does this change fix?

>  	if (ret)
>  		return dev_err_probe(dev, ret, "failed to add %s IRQ chip\n",
> @@ -519,8 +528,9 @@ static int max77759_add_chained_maxq(struct i2c_client *client,
>  
>  	ret = devm_request_threaded_irq(&client->dev, apcmdres_irq,
>  					NULL, apcmdres_irq_handler,
> -					IRQF_ONESHOT | IRQF_SHARED,
> -					dev_name(&client->dev), max77759);
> +					IRQF_ONESHOT | IRQF_SHARED |
> +					IRQF_TRIGGER_LOW, dev_name(&client->dev),
> +					max77759);

dito.

>  	if (ret)
>  		return dev_err_probe(&client->dev, ret,
>  				     "MAX77759_MAXQ_INT_APCMDRESI failed\n");
> @@ -633,7 +643,7 @@ static int max77759_probe(struct i2c_client *client)
>  		return dev_err_probe(&client->dev, -EINVAL,
>  				     "invalid IRQ: %d\n", client->irq);
>  
> -	irq_flags = IRQF_ONESHOT | IRQF_SHARED;
> +	irq_flags = IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW;

I don't believe IRQF_TRIGGER_LOW should be added here, as this is board-specific.
The polarity is meant to be set via DT (and the only current user of this driver
does so).

>  	irq_flags |= irqd_get_trigger_type(irq_data);

That's what gets us the config from DT.

>  
>  	ret = devm_regmap_add_irq_chip(&client->dev, max77759->regmap_top,
> diff --git a/include/linux/mfd/max77759.h b/include/linux/mfd/max77759.h
> index c6face34e385..0ef29a48deec 100644
> --- a/include/linux/mfd/max77759.h
> +++ b/include/linux/mfd/max77759.h
> @@ -62,7 +62,16 @@
>  #define MAX77759_CHGR_REG_CHG_INT               0xb0
>  #define MAX77759_CHGR_REG_CHG_INT2              0xb1
>  #define MAX77759_CHGR_REG_CHG_INT_MASK          0xb2
> +#define MAX77759_CHGR_REG_CHG_INT_AICL          BIT(7)
> +#define MAX77759_CHGR_REG_CHG_INT_CHGIN         BIT(6)
> +#define MAX77759_CHGR_REG_CHG_INT_CHG           BIT(4)
> +#define MAX77759_CHGR_REG_CHG_INT_INLIM         BIT(2)
>  #define MAX77759_CHGR_REG_CHG_INT2_MASK         0xb3
> +#define MAX77759_CHGR_REG_CHG_INT2_BAT_OILO     BIT(4)
> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC   BIT(3)
> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV   BIT(2)
> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO   BIT(1)
> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE BIT(0)

Even if wireless out of scope, it'd still be nice to add macros for
the remaining bits to make this complete and avoid having to update
these again in case wireless support gets added in the future.

Also, would be nice to keep existing style and indent the bits from
the registers (see existing bit definitions in this file a few lines
further up).

Finally, can you add the bits below the respective register (0xb0 / 0xb1)
please, to keep suffix meaningful, and to follow existing style for cases
like this (see MAX77759_MAXQ_REG_UIC_INT1).


Cheers,
Andre'

Re: [PATCH 4/6] mfd: max77759: modify irq configs

[Krzysztof Kozlowski]

On 24/11/2025 07:21, André Draszik wrote:
> 
>>  	if (ret)
>>  		return dev_err_probe(&client->dev, ret,
>>  				     "MAX77759_MAXQ_INT_APCMDRESI failed\n");
>> @@ -633,7 +643,7 @@ static int max77759_probe(struct i2c_client *client)
>>  		return dev_err_probe(&client->dev, -EINVAL,
>>  				     "invalid IRQ: %d\n", client->irq);
>>  
>> -	irq_flags = IRQF_ONESHOT | IRQF_SHARED;
>> +	irq_flags = IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW;
> 
> I don't believe IRQF_TRIGGER_LOW should be added here, as this is board-specific.
> The polarity is meant to be set via DT (and the only current user of this driver
> does so).
> 


If this is the main chip interrupt, then you are right and the code is
obviously wrong. What's more, it is completely unexplained in the commit
msg, because that vague statement cannot be taken as any reasonable
explanation.

Best regards,
Krzysztof

Re: [PATCH 4/6] mfd: max77759: modify irq configs

[Amit Sunil Dhamne]

On 11/23/25 11:39 PM, Krzysztof Kozlowski wrote:
> On 24/11/2025 07:21, André Draszik wrote:
>>>   	if (ret)
>>>   		return dev_err_probe(&client->dev, ret,
>>>   				     "MAX77759_MAXQ_INT_APCMDRESI failed\n");
>>> @@ -633,7 +643,7 @@ static int max77759_probe(struct i2c_client *client)
>>>   		return dev_err_probe(&client->dev, -EINVAL,
>>>   				     "invalid IRQ: %d\n", client->irq);
>>>   
>>> -	irq_flags = IRQF_ONESHOT | IRQF_SHARED;
>>> +	irq_flags = IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW;
>> I don't believe IRQF_TRIGGER_LOW should be added here, as this is board-specific.
>> The polarity is meant to be set via DT (and the only current user of this driver
>> does so).
>>
> If this is the main chip interrupt, then you are right and the code is
> obviously wrong. What's more, it is completely unexplained in the commit
> msg, because that vague statement cannot be taken as any reasonable
> explanation.

You are right. As discussed in the thread with André, I will drop this 
particular change in the next rev.

BR,

Amit

>
> Best regards,
> Krzysztof

Re: [PATCH 4/6] mfd: max77759: modify irq configs

[Amit Sunil Dhamne]

Hi André,

On 11/23/25 10:21 PM, André Draszik wrote:
> Hi Amit,
>
> Thanks for your patches to enable the charger!

Ack!


>> From: Amit Sunil Dhamne <amitsd@google.com>
>>
>> Define specific bit-level masks for charger's registers and modify the
>> irq mask for charger irq_chip. Also, configure the max77759 interrupt
>> lines as active low to all interrupt registrations to ensure the
>> interrupt controllers are configured with the correct trigger type.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>>   drivers/mfd/max77759.c       | 24 +++++++++++++++++-------
>>   include/linux/mfd/max77759.h |  9 +++++++++
>>   2 files changed, 26 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
>> index 6cf6306c4a3b..5fe22884f362 100644
>> --- a/drivers/mfd/max77759.c
>> +++ b/drivers/mfd/max77759.c
>> @@ -256,8 +256,17 @@ static const struct regmap_irq max77759_topsys_irqs[] = {
>>   };
>>   
>>   static const struct regmap_irq max77759_chgr_irqs[] = {
>> -	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0, GENMASK(7, 0)),
>> -	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1, GENMASK(7, 0)),
>> +	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0,
>> +		       MAX77759_CHGR_REG_CHG_INT_AICL |
>> +		       MAX77759_CHGR_REG_CHG_INT_CHGIN |
>> +		       MAX77759_CHGR_REG_CHG_INT_CHG |
>> +		       MAX77759_CHGR_REG_CHG_INT_INLIM),
>> +	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1,
>> +		       MAX77759_CHGR_REG_CHG_INT2_BAT_OILO |
>> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC |
>> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV |
>> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO |
>> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE),
>>   };
>>   
>>   static const struct regmap_irq_chip max77759_pmic_irq_chip = {
>> @@ -486,8 +495,8 @@ static int max77759_add_chained_irq_chip(struct device *dev,
>>   				     "failed to get parent vIRQ(%d) for chip %s\n",
>>   				     pirq, chip->name);
>>   
>> -	ret = devm_regmap_add_irq_chip(dev, regmap, irq,
>> -				       IRQF_ONESHOT | IRQF_SHARED, 0, chip,
>> +	ret = devm_regmap_add_irq_chip(dev, regmap, irq, IRQF_ONESHOT |
>> +				       IRQF_SHARED | IRQF_TRIGGER_LOW, 0, chip,
>>   				       data);
> Please correct me if I'm wrong, but I don't think this makes sense for a
> chained IRQ in this case. What problem does this change fix?

While this patch was meant only for modifying the interrupt mask, I 
added this for consistency because the main SoC -> PMIC line is active 
low. However, I agree it is not strictly necessary here. I will drop it 
in the next revision.


>
>>   	if (ret)
>>   		return dev_err_probe(dev, ret, "failed to add %s IRQ chip\n",
>> @@ -519,8 +528,9 @@ static int max77759_add_chained_maxq(struct i2c_client *client,
>>   
>>   	ret = devm_request_threaded_irq(&client->dev, apcmdres_irq,
>>   					NULL, apcmdres_irq_handler,
>> -					IRQF_ONESHOT | IRQF_SHARED,
>> -					dev_name(&client->dev), max77759);
>> +					IRQF_ONESHOT | IRQF_SHARED |
>> +					IRQF_TRIGGER_LOW, dev_name(&client->dev),
>> +					max77759);
> dito.

Agreed, will drop.


>
>>   	if (ret)
>>   		return dev_err_probe(&client->dev, ret,
>>   				     "MAX77759_MAXQ_INT_APCMDRESI failed\n");
>> @@ -633,7 +643,7 @@ static int max77759_probe(struct i2c_client *client)
>>   		return dev_err_probe(&client->dev, -EINVAL,
>>   				     "invalid IRQ: %d\n", client->irq);
>>   
>> -	irq_flags = IRQF_ONESHOT | IRQF_SHARED;
>> +	irq_flags = IRQF_ONESHOT | IRQF_SHARED | IRQF_TRIGGER_LOW;
> I don't believe IRQF_TRIGGER_LOW should be added here, as this is board-specific.
> The polarity is meant to be set via DT (and the only current user of this driver
> does so).

You are correct. Since I am already retrieving the trigger type from DT 
via irqd_get_trigger_type() below, I will drop this change.


>>   	irq_flags |= irqd_get_trigger_type(irq_data);
> That's what gets us the config from DT.
>
>>   
>>   	ret = devm_regmap_add_irq_chip(&client->dev, max77759->regmap_top,
>> diff --git a/include/linux/mfd/max77759.h b/include/linux/mfd/max77759.h
>> index c6face34e385..0ef29a48deec 100644
>> --- a/include/linux/mfd/max77759.h
>> +++ b/include/linux/mfd/max77759.h
>> @@ -62,7 +62,16 @@
>>   #define MAX77759_CHGR_REG_CHG_INT               0xb0
>>   #define MAX77759_CHGR_REG_CHG_INT2              0xb1
>>   #define MAX77759_CHGR_REG_CHG_INT_MASK          0xb2
>> +#define MAX77759_CHGR_REG_CHG_INT_AICL          BIT(7)
>> +#define MAX77759_CHGR_REG_CHG_INT_CHGIN         BIT(6)
>> +#define MAX77759_CHGR_REG_CHG_INT_CHG           BIT(4)
>> +#define MAX77759_CHGR_REG_CHG_INT_INLIM         BIT(2)
>>   #define MAX77759_CHGR_REG_CHG_INT2_MASK         0xb3
>> +#define MAX77759_CHGR_REG_CHG_INT2_BAT_OILO     BIT(4)
>> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC   BIT(3)
>> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV   BIT(2)
>> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO   BIT(1)
>> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE BIT(0)
> Even if wireless out of scope, it'd still be nice to add macros for
> the remaining bits to make this complete and avoid having to update
> these again in case wireless support gets added in the future.

I would prefer to only define the macros I am currently using to keep 
the review focused, unless you consider this a strict requirement.


>
> Also, would be nice to keep existing style and indent the bits from
> the registers (see existing bit definitions in this file a few lines
> further up).
> Finally, can you add the bits below the respective register (0xb0 / 0xb1)
> please, to keep suffix meaningful, and to follow existing style for cases
> like this (see MAX77759_MAXQ_REG_UIC_INT1).

I will fix the indentation and ordering in the next revision.

Regarding bit definitions: In [PATCH 5/6], the max77759_charger.c driver 
defines bits for the register addresses defined in this file. Currently, 
those macros are only used locally by the max77759 charger driver. Would 
you prefer I move those definitions to this header file as well?

BR,

Amit


>
>
> Cheers,
> Andre'

Re: [PATCH 4/6] mfd: max77759: modify irq configs

[André Draszik]

Hi Amit,

On Tue, 2025-11-25 at 17:10 -0800, Amit Sunil Dhamne wrote:
> Hi André,
> 
> On 11/23/25 10:21 PM, André Draszik wrote:
> > Hi Amit,
> > 
> > Thanks for your patches to enable the charger!
> 
> Ack!
> 
> 
> > > From: Amit Sunil Dhamne <amitsd@google.com>
> > > 
> > > Define specific bit-level masks for charger's registers and modify the
> > > irq mask for charger irq_chip. Also, configure the max77759 interrupt
> > > lines as active low to all interrupt registrations to ensure the
> > > interrupt controllers are configured with the correct trigger type.
> > > 
> > > Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> > > ---
> > >   drivers/mfd/max77759.c       | 24 +++++++++++++++++-------
> > >   include/linux/mfd/max77759.h |  9 +++++++++
> > >   2 files changed, 26 insertions(+), 7 deletions(-)
> > > 
> > > diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
> > > index 6cf6306c4a3b..5fe22884f362 100644
> > > --- a/drivers/mfd/max77759.c
> > > +++ b/drivers/mfd/max77759.c
> > > @@ -256,8 +256,17 @@ static const struct regmap_irq max77759_topsys_irqs[] = {
> > >   };
> > >   
> > >   static const struct regmap_irq max77759_chgr_irqs[] = {
> > > -	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0, GENMASK(7, 0)),
> > > -	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1, GENMASK(7, 0)),
> > > +	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0,
> > > +		       MAX77759_CHGR_REG_CHG_INT_AICL |
> > > +		       MAX77759_CHGR_REG_CHG_INT_CHGIN |
> > > +		       MAX77759_CHGR_REG_CHG_INT_CHG |
> > > +		       MAX77759_CHGR_REG_CHG_INT_INLIM),
> > > +	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1,
> > > +		       MAX77759_CHGR_REG_CHG_INT2_BAT_OILO |
> > > +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC |
> > > +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV |
> > > +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO |
> > > +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE),
> > >   };

You should also add the remaining bits in each register here, so that the
regulator-irq can mask them when no user exists. It will only touch the
bits it knows about, so the state of the mask register is non-
deterministic with this change as-is (depends on how the bootloader
configured it).

[...]

> > 
> > 
> > > diff --git a/include/linux/mfd/max77759.h b/include/linux/mfd/max77759.h
> > > index c6face34e385..0ef29a48deec 100644
> > > --- a/include/linux/mfd/max77759.h
> > > +++ b/include/linux/mfd/max77759.h
> > > @@ -62,7 +62,16 @@
> > >   #define MAX77759_CHGR_REG_CHG_INT               0xb0
> > >   #define MAX77759_CHGR_REG_CHG_INT2              0xb1
> > >   #define MAX77759_CHGR_REG_CHG_INT_MASK          0xb2
> > > +#define MAX77759_CHGR_REG_CHG_INT_AICL          BIT(7)
> > > +#define MAX77759_CHGR_REG_CHG_INT_CHGIN         BIT(6)
> > > +#define MAX77759_CHGR_REG_CHG_INT_CHG           BIT(4)
> > > +#define MAX77759_CHGR_REG_CHG_INT_INLIM         BIT(2)
> > >   #define MAX77759_CHGR_REG_CHG_INT2_MASK         0xb3
> > > +#define MAX77759_CHGR_REG_CHG_INT2_BAT_OILO     BIT(4)
> > > +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC   BIT(3)
> > > +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV   BIT(2)
> > > +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO   BIT(1)
> > > +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE BIT(0)
> > Even if wireless out of scope, it'd still be nice to add macros for
> > the remaining bits to make this complete and avoid having to update
> > these again in case wireless support gets added in the future.
> 
> I would prefer to only define the macros I am currently using to keep 
> the review focused, unless you consider this a strict requirement.

It makes sense to add them now, as per above.



> > Also, would be nice to keep existing style and indent the bits from
> > the registers (see existing bit definitions in this file a few lines
> > further up).
> > Finally, can you add the bits below the respective register (0xb0 / 0xb1)
> > please, to keep suffix meaningful, and to follow existing style for cases
> > like this (see MAX77759_MAXQ_REG_UIC_INT1).
> 
> I will fix the indentation and ordering in the next revision.
> 
> Regarding bit definitions: In [PATCH 5/6], the max77759_charger.c driver 
> defines bits for the register addresses defined in this file. Currently, 
> those macros are only used locally by the max77759 charger driver. Would 
> you prefer I move those definitions to this header file as well?

Yes, would be nice to have them all in one place (in this file). That said,
CHG_INT, CHG_INT_MASK, and CHG_INT_OK all have the same layout and share
the same bits, so I personally would probably reuse the ones you added for
INT for all three of them - MASK (as you did already), and also for the OK
register. But up to you.

Cheers,
Andre'

Re: [PATCH 4/6] mfd: max77759: modify irq configs

[André Draszik]

On Wed, 2025-11-26 at 06:44 +0000, André Draszik wrote:
> Hi Amit,
> 
> On Tue, 2025-11-25 at 17:10 -0800, Amit Sunil Dhamne wrote:
> > Hi André,
> > 
> > On 11/23/25 10:21 PM, André Draszik wrote:
> > > Hi Amit,
> > > 
> > > Thanks for your patches to enable the charger!
> > 
> > Ack!
> > 
> > 
> > > > From: Amit Sunil Dhamne <amitsd@google.com>
> > > > 
> > > > Define specific bit-level masks for charger's registers and modify the
> > > > irq mask for charger irq_chip. Also, configure the max77759 interrupt
> > > > lines as active low to all interrupt registrations to ensure the
> > > > interrupt controllers are configured with the correct trigger type.
> > > > 
> > > > Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> > > > ---
> > > >   drivers/mfd/max77759.c       | 24 +++++++++++++++++-------
> > > >   include/linux/mfd/max77759.h |  9 +++++++++
> > > >   2 files changed, 26 insertions(+), 7 deletions(-)
> > > > 
> > > > diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
> > > > index 6cf6306c4a3b..5fe22884f362 100644
> > > > --- a/drivers/mfd/max77759.c
> > > > +++ b/drivers/mfd/max77759.c
> > > > @@ -256,8 +256,17 @@ static const struct regmap_irq max77759_topsys_irqs[] = {
> > > >   };
> > > >   
> > > >   static const struct regmap_irq max77759_chgr_irqs[] = {
> > > > -	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0, GENMASK(7, 0)),
> > > > -	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1, GENMASK(7, 0)),
> > > > +	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0,
> > > > +		       MAX77759_CHGR_REG_CHG_INT_AICL |
> > > > +		       MAX77759_CHGR_REG_CHG_INT_CHGIN |
> > > > +		       MAX77759_CHGR_REG_CHG_INT_CHG |
> > > > +		       MAX77759_CHGR_REG_CHG_INT_INLIM),
> > > > +	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1,
> > > > +		       MAX77759_CHGR_REG_CHG_INT2_BAT_OILO |
> > > > +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC |
> > > > +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV |
> > > > +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO |
> > > > +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE),
> > > >   };
> 
> You should also add the remaining bits in each register here, so that the
> regulator-irq can mask them when no user exists. It will only touch the
  ^^^^^^^^^^^^^
  regmap-irq

A.

Re: [PATCH 4/6] mfd: max77759: modify irq configs

[Amit Sunil Dhamne]

On 11/25/25 10:44 PM, André Draszik wrote:
> Hi Amit,
>
> On Tue, 2025-11-25 at 17:10 -0800, Amit Sunil Dhamne wrote:
>> Hi André,
>>
>> On 11/23/25 10:21 PM, André Draszik wrote:
>>> Hi Amit,
>>>
>>> Thanks for your patches to enable the charger!
>> Ack!
>>
>>
>>>> From: Amit Sunil Dhamne <amitsd@google.com>
>>>>
>>>> Define specific bit-level masks for charger's registers and modify the
>>>> irq mask for charger irq_chip. Also, configure the max77759 interrupt
>>>> lines as active low to all interrupt registrations to ensure the
>>>> interrupt controllers are configured with the correct trigger type.
>>>>
>>>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>>>> ---
>>>>    drivers/mfd/max77759.c       | 24 +++++++++++++++++-------
>>>>    include/linux/mfd/max77759.h |  9 +++++++++
>>>>    2 files changed, 26 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
>>>> index 6cf6306c4a3b..5fe22884f362 100644
>>>> --- a/drivers/mfd/max77759.c
>>>> +++ b/drivers/mfd/max77759.c
>>>> @@ -256,8 +256,17 @@ static const struct regmap_irq max77759_topsys_irqs[] = {
>>>>    };
>>>>    
>>>>    static const struct regmap_irq max77759_chgr_irqs[] = {
>>>> -	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0, GENMASK(7, 0)),
>>>> -	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1, GENMASK(7, 0)),
>>>> +	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_1, 0,
>>>> +		       MAX77759_CHGR_REG_CHG_INT_AICL |
>>>> +		       MAX77759_CHGR_REG_CHG_INT_CHGIN |
>>>> +		       MAX77759_CHGR_REG_CHG_INT_CHG |
>>>> +		       MAX77759_CHGR_REG_CHG_INT_INLIM),
>>>> +	REGMAP_IRQ_REG(MAX77759_CHARGER_INT_2, 1,
>>>> +		       MAX77759_CHGR_REG_CHG_INT2_BAT_OILO |
>>>> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC |
>>>> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV |
>>>> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO |
>>>> +		       MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE),
>>>>    };
> You should also add the remaining bits in each register here, so that the
> regulator-irq can mask them when no user exists. It will only touch the
> bits it knows about, so the state of the mask register is non-
> deterministic with this change as-is (depends on how the bootloader
> configured it).
>
> [...]

I see what you're saying. I will remove this and keep it the way it was 
before.


>
>>>
>>>> diff --git a/include/linux/mfd/max77759.h b/include/linux/mfd/max77759.h
>>>> index c6face34e385..0ef29a48deec 100644
>>>> --- a/include/linux/mfd/max77759.h
>>>> +++ b/include/linux/mfd/max77759.h
>>>> @@ -62,7 +62,16 @@
>>>>    #define MAX77759_CHGR_REG_CHG_INT               0xb0
>>>>    #define MAX77759_CHGR_REG_CHG_INT2              0xb1
>>>>    #define MAX77759_CHGR_REG_CHG_INT_MASK          0xb2
>>>> +#define MAX77759_CHGR_REG_CHG_INT_AICL          BIT(7)
>>>> +#define MAX77759_CHGR_REG_CHG_INT_CHGIN         BIT(6)
>>>> +#define MAX77759_CHGR_REG_CHG_INT_CHG           BIT(4)
>>>> +#define MAX77759_CHGR_REG_CHG_INT_INLIM         BIT(2)
>>>>    #define MAX77759_CHGR_REG_CHG_INT2_MASK         0xb3
>>>> +#define MAX77759_CHGR_REG_CHG_INT2_BAT_OILO     BIT(4)
>>>> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC   BIT(3)
>>>> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV   BIT(2)
>>>> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO   BIT(1)
>>>> +#define MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE BIT(0)
>>> Even if wireless out of scope, it'd still be nice to add macros for
>>> the remaining bits to make this complete and avoid having to update
>>> these again in case wireless support gets added in the future.
>> I would prefer to only define the macros I am currently using to keep
>> the review focused, unless you consider this a strict requirement.
> It makes sense to add them now, as per above.

Okay. I will add them.


>
>
>
>>> Also, would be nice to keep existing style and indent the bits from
>>> the registers (see existing bit definitions in this file a few lines
>>> further up).
>>> Finally, can you add the bits below the respective register (0xb0 / 0xb1)
>>> please, to keep suffix meaningful, and to follow existing style for cases
>>> like this (see MAX77759_MAXQ_REG_UIC_INT1).
>> I will fix the indentation and ordering in the next revision.
>>
>> Regarding bit definitions: In [PATCH 5/6], the max77759_charger.c driver
>> defines bits for the register addresses defined in this file. Currently,
>> those macros are only used locally by the max77759 charger driver. Would
>> you prefer I move those definitions to this header file as well?
> Yes, would be nice to have them all in one place (in this file). That said,
> CHG_INT, CHG_INT_MASK, and CHG_INT_OK all have the same layout and share
> the same bits, so I personally would probably reuse the ones you added for
> INT for all three of them - MASK (as you did already), and also for the OK
> register. But up to you.

Sounds good.


Thanks,

Amit

> Cheers,
> Andre'

Re: [PATCH 4/6] mfd: max77759: modify irq configs

[Krzysztof Kozlowski]

On 23/11/2025 09:35, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
> 
> Define specific bit-level masks for charger's registers and modify the
> irq mask for charger irq_chip. Also, configure the max77759 interrupt
> lines as active low to all interrupt registrations to ensure the
> interrupt controllers are configured with the correct trigger type.

This is just redundant explanation. You did not actually explain why you
need to set the trigger.

Best regards,
Krzysztof

[PATCH 5/6] power: supply: max77759: add charger driver

[Amit Sunil Dhamne via B4 Relay]

From: Amit Sunil Dhamne <amitsd@google.com>

Add support for MAX77759 battery charger driver. This is a 4A 1-Cell
Li+/LiPoly dual input switch mode charger. While the device can support
USB & wireless charger inputs, this implementation only supports USB
input. This implementation supports both buck and boost modes.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
 MAINTAINERS                             |   7 +
 drivers/mfd/max77759.c                  |   3 +-
 drivers/power/supply/Kconfig            |  11 +
 drivers/power/supply/Makefile           |   1 +
 drivers/power/supply/max77759_charger.c | 866 ++++++++++++++++++++++++++++++++
 5 files changed, 887 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index fed6cd812d79..f1b1015c08b5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15053,6 +15053,13 @@ F:	drivers/mfd/max77759.c
 F:	drivers/nvmem/max77759-nvmem.c
 F:	include/linux/mfd/max77759.h
 
+MAXIM MAX77759 BATTERY CHARGER DRIVER
+M:	Amit Sunil Dhamne <amitsd@google.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
+F:	drivers/power/supply/max77759_charger.c
+
 MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
 M:	Javier Martinez Canillas <javier@dowhile0.org>
 L:	linux-kernel@vger.kernel.org
diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
index 5fe22884f362..8a22838be1b0 100644
--- a/drivers/mfd/max77759.c
+++ b/drivers/mfd/max77759.c
@@ -349,7 +349,8 @@ static const struct mfd_cell max77759_maxq_cells[] = {
 };
 
 static const struct mfd_cell max77759_charger_cells[] = {
-	MFD_CELL_RES("max77759-charger", max77759_charger_resources),
+	MFD_CELL_OF("max77759-charger", max77759_charger_resources, NULL, 0, 0,
+		    "maxim,max77759-charger"),
 };
 
 int max77759_maxq_command(struct max77759 *max77759,
diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
index 79ddb006e2da..b97990cc0b92 100644
--- a/drivers/power/supply/Kconfig
+++ b/drivers/power/supply/Kconfig
@@ -1074,4 +1074,15 @@ config FUEL_GAUGE_MM8013
 	  the state of charge, temperature, cycle count, actual and design
 	  capacity, etc.
 
+config CHARGER_MAX77759
+	tristate "MAX77759 Charger Driver"
+	depends on MFD_MAX77759
+	default MFD_MAX77759
+	help
+	  Say M or Y here to enable the MAX77759 Charger Driver. MAX77759
+	  charger is a function of the MAX77759 PMIC. This is a dual input
+	  switch-mode charger. This driver supports buck and OTG boost modes.
+
+	  If built as a module, it will be called max77759_charger.
+
 endif # POWER_SUPPLY
diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
index f943c9150b32..12669734cfe3 100644
--- a/drivers/power/supply/Makefile
+++ b/drivers/power/supply/Makefile
@@ -122,3 +122,4 @@ obj-$(CONFIG_CHARGER_SURFACE)	+= surface_charger.o
 obj-$(CONFIG_BATTERY_UG3105)	+= ug3105_battery.o
 obj-$(CONFIG_CHARGER_QCOM_SMB2)	+= qcom_smbx.o
 obj-$(CONFIG_FUEL_GAUGE_MM8013)	+= mm8013.o
+obj-$(CONFIG_CHARGER_MAX77759)	+= max77759_charger.o
diff --git a/drivers/power/supply/max77759_charger.c b/drivers/power/supply/max77759_charger.c
new file mode 100644
index 000000000000..51637e87182b
--- /dev/null
+++ b/drivers/power/supply/max77759_charger.c
@@ -0,0 +1,866 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * max77759_charger.c - Battery charger driver for MAX77759 charger device.
+ *
+ * Copyright 2025 Google LLC.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/devm-helpers.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/math64.h>
+#include <linux/mfd/max77759.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/regmap.h>
+#include <linux/regulator/driver.h>
+#include <linux/string_choices.h>
+
+/* CHG_INT_OK */
+#define CHG_INT_OK_AICL_OK			BIT(7)
+#define CHG_INT_OK_CHGIN_OK			BIT(6)
+#define CHG_INT_OK_CHG_OK			BIT(4)
+#define CHG_INT_OK_INLIM_OK			BIT(2)
+
+/* CHG_DETAILS_00 */
+#define CHG_DETAILS_OO_CHGIN_DTLS		GENMASK(6, 5)
+
+/*
+ * Charger Input Status
+ * @CHGIN_DTLS_VBUS_UNDERVOLTAGE: Charger input voltage (Vchgin) < Under Voltage
+ *     Threshold (Vuvlo)
+ * @CHGIN_DTLS_VBUS_MARGINAL_VOLTAGE: Vchgin > Vuvlo and
+ *     Vchgin < (Battery Voltage (Vbatt) + system voltage (Vsys))
+ * @CHGIN_DTLS_VBUS_OVERVOLTAGE: Vchgin > Over Voltage threshold (Vovlo)
+ * @CHGIN_DTLS_VBUS_VALID: Vchgin > Vuvlo, Vchgin < Vovlo and
+ *     Vchgin > (Vsys + Vbatt)
+ */
+enum chgin_dtls_status {
+	CHGIN_DTLS_VBUS_UNDERVOLTAGE,
+	CHGIN_DTLS_VBUS_MARGINAL_VOLTAGE,
+	CHGIN_DTLS_VBUS_OVERVOLTAGE,
+	CHGIN_DTLS_VBUS_VALID,
+};
+
+/* CHG_DETAILS_01 */
+#define CHG_DETAILS_01_BAT_DTLS			GENMASK(6, 4)
+
+/*
+ * Battery Details
+ * @BAT_DTLS_NO_BATT_CHG_SUSP:    No battery and the charger suspended
+ * @BAT_DTLS_DEAD_BATTERY:        Vbatt < Vtrickle
+ * @BAT_DTLS_BAT_CHG_TIMER_FAULT: Charging suspended due to timer fault
+ * @BAT_DTLS_BAT_OKAY: Battery okay and Vbatt > Min Sys Voltage (Vsysmin)
+ * @BAT_DTLS_BAT_UNDERVOLTAGE:    Battery is okay. Vtrickle < Vbatt < Vsysmin
+ * @BAT_DTLS_BAT_OVERVOLTAGE:     Battery voltage > Overvoltage threshold
+ * @BAT_DTLS_BAT_OVERCURRENT: Battery current exceeds overcurrent threshold
+ * @BAT_DTLS_BAT_ONLY_MODE:   Battery only mode and battery level not available
+ */
+enum bat_dtls_states {
+	BAT_DTLS_NO_BATT_CHG_SUSP,
+	BAT_DTLS_DEAD_BATTERY,
+	BAT_DTLS_BAT_CHG_TIMER_FAULT,
+	BAT_DTLS_BAT_OKAY,
+	BAT_DTLS_BAT_UNDERVOLTAGE,
+	BAT_DTLS_BAT_OVERVOLTAGE,
+	BAT_DTLS_BAT_OVERCURRENT,
+	BAT_DTLS_BAT_ONLY_MODE,
+};
+
+#define CHG_DETAILS_01_CHG_DTLS			GENMASK(3, 0)
+
+/*
+ * Charger Details
+ * @CHG_DTLS_PREQUAL: Charger in prequalification mode
+ * @CHG_DTLS_CC:      Charger in fast charge const curr mode
+ * @CHG_DTLS_CV:      Charger in fast charge const voltage mode
+ * @CHG_DTLS_TO:      Charger is in top off mode
+ * @CHG_DTLS_DONE:    Charger is done
+ * @CHG_DTLS_RSVD_1:  Reserved
+ * @CHG_DTLS_TIMER_FAULT:   Charger is in timer fault mode
+ * @CHG_DTLS_SUSP_BATT_THM: Charger is suspended as bettery removal detected
+ * @CHG_DTLS_OFF:     Charger is off. Input invalid or charger disabled
+ * @CHG_DTLS_RSVD_2:  Reserved
+ * @CHG_DTLS_RSVD_3:   Reserved
+ * @CHG_DTLS_OFF_WDOG_TIMER:    Charger is off as watchdog timer expired
+ * @CHG_DTLS_SUSP_JEITA:        Charger is in JEITA control mode
+ */
+enum chg_dtls_states {
+	CHG_DTLS_PREQUAL,
+	CHG_DTLS_CC,
+	CHG_DTLS_CV,
+	CHG_DTLS_TO,
+	CHG_DTLS_DONE,
+	CHG_DTLS_RSVD_1,
+	CHG_DTLS_TIMER_FAULT,
+	CHG_DTLS_SUSP_BATT_THM,
+	CHG_DTLS_OFF,
+	CHG_DTLS_RSVD_2,
+	CHG_DTLS_RSVD_3,
+	CHG_DTLS_OFF_WDOG_TIMER,
+	CHG_DTLS_SUSP_JEITA,
+};
+
+/* CHG_DETAILS_02 */
+#define CHG_DETAILS_02_CHGIN_STS		BIT(5)
+
+/* CHG_CNFG_00 */
+#define CHG_CNFG_00_MODE			GENMASK(3, 0)
+
+enum chg_mode {
+	CHG_MODE_OFF,
+	CHG_MODE_CHG_BUCK_ON = 0x5,
+	CHG_MODE_OTG_BOOST_ON = 0xA,
+};
+
+/* CHG_CNFG_02 */
+/* Fast Charge Current Selection (in uA) */
+#define CHG_CNFG_02_CHGCC			GENMASK(5, 0)
+#define CHGCC_LIMIT_MIN_UA			133330
+#define CHGCC_LIMIT_MAX_UA			4000000
+#define CHGCC_LIMIT_STEP_UA			66670
+#define CHGCC_LIMIT_REG_OFFSET			0x2
+
+/* CHG_CNFG_04 */
+/* Charge Termination Voltage Setting (in mV) */
+#define CHG_CNFG_04_CHG_CV_PRM			GENMASK(5, 0)
+/* [3.8, 3.9] V range */
+#define CHG_CV_PRM_LIMIT_LO_MIN_MV		3800
+#define CHG_CV_PRM_LIMIT_LO_MAX_MV		3900
+#define CHG_CV_PRM_LIMIT_LO_STEP_MV		100
+#define CHG_CV_PRM_LIMIT_LO_MIN_REG		0x38
+#define CHG_CV_PRM_LIMIT_LO_MAX_REG		0x39
+/* [4, 4.5] V range */
+#define CHG_CV_PRM_LIMIT_HI_MIN_MV		4000
+#define CHG_CV_PRM_LIMIT_HI_MAX_MV		4500
+#define CHG_CV_PRM_LIMIT_HI_STEP_MV		10
+#define CHG_CV_PRM_LIMIT_HI_MIN_REG		0x0
+#define CHG_CV_PRM_LIMIT_HI_MAX_REG		0x32
+
+/* CHG_CNFG_06 */
+#define CHG_CNFG_06_CHGPROT			GENMASK(3, 2)
+
+/* CHG_CNFG_09 */
+/* CHGIN Input Current Limit (in uA) */
+#define CHG_CNFG_09_CHGIN_ILIM			GENMASK(6, 0)
+#define CHGIN_ILIM_MIN_UA			100000
+#define CHGIN_ILIM_MAX_UA			3200000
+#define CHGIN_ILIM_STEP_UA			25000
+#define CHGIN_ILIM_REG_OFFSET			0x3
+
+/* CHG_CNFG_12 (Protected) */
+/* Wireless Charging input channel select */
+#define CHG_CNFG_12_WCINSEL			BIT(6)
+/* CHGIN/USB input channel select */
+#define CHG_CNFG_12_CHGINSEL			BIT(5)
+
+/* CHG_CNFG_18 (Protected) */
+/* Watchdog Timer Enable Bit */
+#define CHG_CNFG_18_WDTEN			BIT(0)
+
+/* Default values for Fast Charge Current & Float Voltage */
+#define CHG_CC_DEFAULT_UA			2266770
+#define CHG_FV_DEFAULT_MV			4300
+
+struct max77759_charger {
+	struct device *dev;
+	struct regmap *regmap;
+	struct power_supply *psy;
+	int irq[2];
+	struct regulator_dev *usb_otg_rdev;
+	struct notifier_block nb;
+	struct power_supply *tcpm_psy;
+	struct work_struct psy_work;
+	struct mutex lock; /* protects the state below */
+	enum chg_mode mode;
+};
+
+static inline int regval_to_val(int reg, int reg_offset, int step, int minval)
+{
+	return ((reg - reg_offset) * step) + minval;
+}
+
+static inline int val_to_regval(int val, int minval, int step, int reg_offset)
+{
+	s64 dividend;
+
+	if (unlikely(step == 0))
+		return reg_offset;
+
+	dividend = (s64)val - minval;
+	return DIV_S64_ROUND_CLOSEST(dividend, step) + reg_offset;
+}
+
+static inline int unlock_prot_regs(struct max77759_charger *chg, bool unlock)
+{
+	return regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_06,
+				  CHG_CNFG_06_CHGPROT,
+				  unlock ? CHG_CNFG_06_CHGPROT : 0);
+}
+
+static int charger_input_valid(struct max77759_charger *chg)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_INT_OK, &val);
+	if (ret)
+		return ret;
+
+	return (val & CHG_INT_OK_CHG_OK) && (val & CHG_INT_OK_CHGIN_OK);
+}
+
+static int get_online(struct max77759_charger *chg)
+{
+	u32 val;
+	int ret;
+
+	ret = charger_input_valid(chg);
+	if (ret <= 0)
+		return ret;
+
+	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_DETAILS_02, &val);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&chg->lock);
+	return (val & CHG_DETAILS_02_CHGIN_STS) &&
+		(chg->mode == CHG_MODE_CHG_BUCK_ON);
+}
+
+static int get_status(struct max77759_charger *chg)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_DETAILS_01,
+			  &val);
+	if (ret)
+		return ret;
+
+	switch (FIELD_GET(CHG_DETAILS_01_CHG_DTLS, val)) {
+	case CHG_DTLS_PREQUAL:
+	case CHG_DTLS_CC:
+	case CHG_DTLS_CV:
+	case CHG_DTLS_TO:
+		return POWER_SUPPLY_STATUS_CHARGING;
+	case CHG_DTLS_DONE:
+		return POWER_SUPPLY_STATUS_FULL;
+	case CHG_DTLS_TIMER_FAULT:
+	case CHG_DTLS_SUSP_BATT_THM:
+	case CHG_DTLS_OFF_WDOG_TIMER:
+	case CHG_DTLS_SUSP_JEITA:
+		return POWER_SUPPLY_STATUS_NOT_CHARGING;
+	case CHG_DTLS_OFF:
+		return POWER_SUPPLY_STATUS_DISCHARGING;
+	default:
+		break;
+	}
+
+	return POWER_SUPPLY_STATUS_UNKNOWN;
+}
+
+static int get_charge_type(struct max77759_charger *chg)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_DETAILS_01,
+			  &val);
+	if (ret)
+		return ret;
+
+	switch (FIELD_GET(CHG_DETAILS_01_CHG_DTLS, val)) {
+	case CHG_DTLS_PREQUAL:
+		return POWER_SUPPLY_CHARGE_TYPE_TRICKLE;
+	case CHG_DTLS_CC:
+		return POWER_SUPPLY_CHARGE_TYPE_FAST;
+	case CHG_DTLS_CV:
+	case CHG_DTLS_TO:
+		return POWER_SUPPLY_CHARGE_TYPE_STANDARD;
+	case CHG_DTLS_DONE:
+	case CHG_DTLS_TIMER_FAULT:
+	case CHG_DTLS_SUSP_BATT_THM:
+	case CHG_DTLS_OFF_WDOG_TIMER:
+	case CHG_DTLS_SUSP_JEITA:
+	case CHG_DTLS_OFF:
+		return POWER_SUPPLY_CHARGE_TYPE_NONE;
+	default:
+		break;
+	}
+
+	return POWER_SUPPLY_CHARGE_TYPE_UNKNOWN;
+}
+
+static int get_chg_health(struct max77759_charger *chg)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_DETAILS_00,
+			  &val);
+	if (ret)
+		return ret;
+
+	switch (FIELD_GET(CHG_DETAILS_OO_CHGIN_DTLS, val)) {
+	case CHGIN_DTLS_VBUS_UNDERVOLTAGE:
+	case CHGIN_DTLS_VBUS_MARGINAL_VOLTAGE:
+		return POWER_SUPPLY_HEALTH_UNDERVOLTAGE;
+	case CHGIN_DTLS_VBUS_OVERVOLTAGE:
+		return POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+	case CHGIN_DTLS_VBUS_VALID:
+		return POWER_SUPPLY_HEALTH_GOOD;
+	default:
+		break;
+	}
+
+	return POWER_SUPPLY_HEALTH_UNKNOWN;
+}
+
+static int get_batt_health(struct max77759_charger *chg)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_DETAILS_01,
+			  &val);
+	if (ret)
+		return ret;
+
+	switch (FIELD_GET(CHG_DETAILS_01_BAT_DTLS, val)) {
+	case BAT_DTLS_NO_BATT_CHG_SUSP:
+		return POWER_SUPPLY_HEALTH_NO_BATTERY;
+	case BAT_DTLS_DEAD_BATTERY:
+		return POWER_SUPPLY_HEALTH_DEAD;
+	case BAT_DTLS_BAT_CHG_TIMER_FAULT:
+		return POWER_SUPPLY_HEALTH_SAFETY_TIMER_EXPIRE;
+	case BAT_DTLS_BAT_OKAY:
+	case BAT_DTLS_BAT_ONLY_MODE:
+		return POWER_SUPPLY_HEALTH_GOOD;
+	case BAT_DTLS_BAT_UNDERVOLTAGE:
+		return POWER_SUPPLY_HEALTH_UNDERVOLTAGE;
+	case BAT_DTLS_BAT_OVERVOLTAGE:
+		return POWER_SUPPLY_HEALTH_OVERVOLTAGE;
+	case BAT_DTLS_BAT_OVERCURRENT:
+		return POWER_SUPPLY_HEALTH_OVERCURRENT;
+	default:
+		break;
+	}
+
+	return POWER_SUPPLY_HEALTH_UNKNOWN;
+}
+
+static int get_health(struct max77759_charger *chg)
+{
+	int ret;
+
+	ret = get_online(chg);
+	if (ret < 0)
+		return ret;
+
+	if (ret) {
+		ret = get_chg_health(chg);
+		if (ret < 0 || ret != POWER_SUPPLY_HEALTH_GOOD)
+			return ret;
+	}
+
+	return get_batt_health(chg);
+}
+
+static int get_fast_charge_current(struct max77759_charger *chg)
+{
+	u32 regval;
+	int ret;
+
+	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_02, &regval);
+	if (ret)
+		return ret;
+
+	ret = FIELD_GET(CHG_CNFG_02_CHGCC, regval);
+	if (ret <= CHGCC_LIMIT_REG_OFFSET)
+		return CHGCC_LIMIT_MIN_UA;
+
+	return regval_to_val(ret, CHGCC_LIMIT_REG_OFFSET, CHGCC_LIMIT_STEP_UA,
+			     CHGCC_LIMIT_MIN_UA);
+}
+
+static int set_fast_charge_current_limit(struct max77759_charger *chg, u32 cc_max_ua)
+{
+	u32 val;
+
+	if (cc_max_ua < CHGCC_LIMIT_MIN_UA || cc_max_ua > CHGCC_LIMIT_MAX_UA)
+		return -EINVAL;
+
+	val = val_to_regval(cc_max_ua, CHGCC_LIMIT_MIN_UA, CHGCC_LIMIT_STEP_UA,
+			    CHGCC_LIMIT_REG_OFFSET);
+	return regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_02,
+				  CHG_CNFG_02_CHGCC, val);
+}
+
+static int get_float_voltage(struct max77759_charger *chg)
+{
+	u32 regval;
+	int ret;
+
+	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_04, &regval);
+	if (ret)
+		return ret;
+
+	ret = FIELD_GET(CHG_CNFG_04_CHG_CV_PRM, regval);
+	switch (ret) {
+	case CHG_CV_PRM_LIMIT_HI_MIN_REG ... CHG_CV_PRM_LIMIT_HI_MAX_REG:
+		return regval_to_val(ret, CHG_CV_PRM_LIMIT_HI_MIN_REG,
+				     CHG_CV_PRM_LIMIT_HI_STEP_MV,
+				     CHG_CV_PRM_LIMIT_HI_MIN_MV);
+	case CHG_CV_PRM_LIMIT_LO_MIN_REG ... CHG_CV_PRM_LIMIT_LO_MAX_REG:
+		return regval_to_val(ret, CHG_CV_PRM_LIMIT_LO_MIN_REG,
+				     CHG_CV_PRM_LIMIT_LO_STEP_MV,
+				     CHG_CV_PRM_LIMIT_LO_MIN_MV);
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int set_float_voltage_limit(struct max77759_charger *chg, u32 fv_mv)
+{
+	u32 regval;
+
+	if (fv_mv >= CHG_CV_PRM_LIMIT_LO_MIN_MV &&
+	    fv_mv <= CHG_CV_PRM_LIMIT_LO_MAX_MV) {
+		regval = val_to_regval(fv_mv, CHG_CV_PRM_LIMIT_LO_MIN_MV,
+				       CHG_CV_PRM_LIMIT_LO_STEP_MV,
+				       CHG_CV_PRM_LIMIT_LO_MIN_REG);
+	} else if (fv_mv >= CHG_CV_PRM_LIMIT_HI_MIN_MV &&
+		   fv_mv <= CHG_CV_PRM_LIMIT_HI_MAX_MV) {
+		regval = val_to_regval(fv_mv, CHG_CV_PRM_LIMIT_HI_MIN_MV,
+				       CHG_CV_PRM_LIMIT_HI_STEP_MV,
+				       CHG_CV_PRM_LIMIT_HI_MIN_REG);
+	} else {
+		return -EINVAL;
+	}
+
+	return regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_04,
+				  CHG_CNFG_04_CHG_CV_PRM, regval);
+}
+
+static int get_input_current_limit(struct max77759_charger *chg)
+{
+	u32 regval;
+	int ret;
+
+	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_09, &regval);
+	if (ret)
+		return ret;
+
+	ret = FIELD_GET(CHG_CNFG_09_CHGIN_ILIM, regval);
+	if (ret <= CHGIN_ILIM_REG_OFFSET)
+		return CHGIN_ILIM_MIN_UA;
+
+	return regval_to_val(ret, CHGIN_ILIM_REG_OFFSET, CHGIN_ILIM_STEP_UA,
+			     CHGIN_ILIM_MIN_UA);
+}
+
+static int set_input_current_limit(struct max77759_charger *chg, int ilim_ua)
+{
+	u32 regval;
+
+	if (ilim_ua < 0)
+		return -EINVAL;
+
+	if (ilim_ua == 0)
+		ilim_ua = CHGIN_ILIM_MIN_UA;
+	else if (ilim_ua > CHGIN_ILIM_MAX_UA)
+		ilim_ua = CHGIN_ILIM_MAX_UA;
+
+	regval = val_to_regval(ilim_ua, CHGIN_ILIM_MIN_UA,
+			       CHGIN_ILIM_STEP_UA, CHGIN_ILIM_REG_OFFSET);
+	return regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_09,
+				  CHG_CNFG_09_CHGIN_ILIM, regval);
+}
+
+static const enum power_supply_property max77759_charger_props[] = {
+	POWER_SUPPLY_PROP_ONLINE,
+	POWER_SUPPLY_PROP_PRESENT,
+	POWER_SUPPLY_PROP_STATUS,
+	POWER_SUPPLY_PROP_CHARGE_TYPE,
+	POWER_SUPPLY_PROP_HEALTH,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX,
+	POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX,
+	POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT,
+};
+
+static int max77759_charger_get_property(struct power_supply *psy,
+					 enum power_supply_property psp,
+					 union power_supply_propval *pval)
+{
+	struct max77759_charger *chg = power_supply_get_drvdata(psy);
+	int ret;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_ONLINE:
+		ret = get_online(chg);
+		break;
+	case POWER_SUPPLY_PROP_PRESENT:
+		ret = charger_input_valid(chg);
+		break;
+	case POWER_SUPPLY_PROP_STATUS:
+		ret = get_status(chg);
+		break;
+	case POWER_SUPPLY_PROP_CHARGE_TYPE:
+		ret = get_charge_type(chg);
+		break;
+	case POWER_SUPPLY_PROP_HEALTH:
+		ret = get_health(chg);
+		break;
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_CURRENT_MAX:
+		ret = get_fast_charge_current(chg);
+		break;
+	case POWER_SUPPLY_PROP_CONSTANT_CHARGE_VOLTAGE_MAX:
+		ret = get_float_voltage(chg);
+		break;
+	case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT:
+		ret = get_input_current_limit(chg);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
+	pval->intval = ret;
+	return ret < 0 ? ret : 0;
+}
+
+static const struct power_supply_desc max77759_charger_desc = {
+	.name = "max77759-charger",
+	.type = POWER_SUPPLY_TYPE_USB,
+	.properties = max77759_charger_props,
+	.num_properties = ARRAY_SIZE(max77759_charger_props),
+	.get_property = max77759_charger_get_property,
+};
+
+static int charger_set_mode(struct max77759_charger *chg, enum chg_mode mode)
+{
+	int ret;
+
+	guard(mutex)(&chg->lock);
+
+	if (chg->mode == mode)
+		return 0;
+
+	if ((mode == CHG_MODE_CHG_BUCK_ON || mode == CHG_MODE_OTG_BOOST_ON) &&
+	    chg->mode != CHG_MODE_OFF) {
+		dev_err(chg->dev, "Invalid mode transition from %d to %d",
+			chg->mode, mode);
+		return -EINVAL;
+	}
+
+	ret = regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_00,
+				 CHG_CNFG_00_MODE, mode);
+	if (ret)
+		return ret;
+
+	chg->mode = mode;
+	return 0;
+}
+
+static int enable_usb_otg(struct regulator_dev *rdev)
+{
+	struct max77759_charger *chg = rdev_get_drvdata(rdev);
+
+	return charger_set_mode(chg, CHG_MODE_OTG_BOOST_ON);
+}
+
+static int disable_usb_otg(struct regulator_dev *rdev)
+{
+	struct max77759_charger *chg = rdev_get_drvdata(rdev);
+
+	return charger_set_mode(chg, CHG_MODE_OFF);
+}
+
+static int usb_otg_status(struct regulator_dev *rdev)
+{
+	struct max77759_charger *chg = rdev_get_drvdata(rdev);
+
+	guard(mutex)(&chg->lock);
+	return chg->mode == CHG_MODE_OTG_BOOST_ON;
+}
+
+static const struct regulator_ops usb_otg_reg_ops = {
+	.enable = enable_usb_otg,
+	.disable = disable_usb_otg,
+	.is_enabled = usb_otg_status,
+};
+
+static const struct regulator_desc usb_otg_reg_desc = {
+	.name = "usb-otg-vbus",
+	.of_match = of_match_ptr("usb-otg-vbus-regulator"),
+	.owner = THIS_MODULE,
+	.ops = &usb_otg_reg_ops,
+	.fixed_uV = 5000000,
+	.n_voltages = 1,
+};
+
+static irqreturn_t irq_handler(int irq, void *data)
+{
+	struct max77759_charger *chg = data;
+	u32 irq_status, chgint_ok, idx = 0;
+	int ret;
+
+	if (irq == chg->irq[0])
+		idx = 0;
+	else
+		idx = 1;
+
+	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_INT + idx,
+			  &irq_status);
+	if (ret) {
+		dev_err(chg->dev, "regmap_read_error idx=%d ret=%d", idx, ret);
+		return IRQ_HANDLED;
+	}
+
+	regmap_write(chg->regmap, MAX77759_CHGR_REG_CHG_INT + idx,
+		     irq_status);
+	regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_INT_OK, &chgint_ok);
+
+	if (idx == 0) {
+		if (irq_status & MAX77759_CHGR_REG_CHG_INT_AICL)
+			dev_dbg(chg->dev, "AICL mode: %s",
+				str_no_yes(chgint_ok & CHG_INT_OK_AICL_OK));
+
+		if (irq_status & MAX77759_CHGR_REG_CHG_INT_CHGIN)
+			dev_dbg(chg->dev, "CHGIN input valid: %s",
+				str_yes_no(chgint_ok & CHG_INT_OK_CHGIN_OK));
+
+		if (irq_status & MAX77759_CHGR_REG_CHG_INT_CHG)
+			dev_dbg(chg->dev, "CHG status okay/off: %s",
+				str_yes_no(chgint_ok & CHG_INT_OK_CHG_OK));
+
+		if (irq_status & MAX77759_CHGR_REG_CHG_INT_INLIM)
+			dev_dbg(chg->dev, "Current Limit reached: %s",
+				str_no_yes(chgint_ok & CHG_INT_OK_INLIM_OK));
+	} else {
+		if (irq_status & MAX77759_CHGR_REG_CHG_INT2_BAT_OILO)
+			dev_dbg(chg->dev,
+				"Battery over-current threshold crossed");
+
+		if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC)
+			dev_dbg(chg->dev, "Charger reached CC stage");
+
+		if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV)
+			dev_dbg(chg->dev, "Charger reached CV stage");
+
+		if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO)
+			dev_dbg(chg->dev, "Charger reached TO stage");
+
+		if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE)
+			dev_dbg(chg->dev, "Charger reached Done stage");
+	}
+
+	power_supply_changed(chg->psy);
+	return IRQ_HANDLED;
+}
+
+static int max77759_init_irqhandler(struct max77759_charger *chg)
+{
+	static const char * const irq_res_names[] = { "INT1", "INT2" };
+	struct device *dev = chg->dev;
+	unsigned long irq_flags;
+	struct irq_data *irqd;
+	int *irq = chg->irq;
+	int ret, i;
+
+	for (i = 0; i < 2; i++) {
+		irq[i] = platform_get_irq_byname(to_platform_device(dev),
+						 irq_res_names[i]);
+		if (irq[i] < 0) {
+			dev_err(dev, "unable to find %s irq", irq_res_names[i]);
+			return irq[i];
+		}
+
+		irq_flags = IRQF_ONESHOT | IRQF_SHARED;
+		irqd = irq_get_irq_data(irq[i]);
+		if (irqd)
+			irq_flags |= irqd_get_trigger_type(irqd);
+
+		ret = devm_request_threaded_irq(dev, irq[i], NULL, irq_handler,
+						irq_flags, dev_name(dev), chg);
+		if (ret) {
+			dev_err(dev, "Unable to register threaded irq handler");
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int max77759_charger_init(struct max77759_charger *chg)
+{
+	int ret;
+	u32 regval;
+
+	regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_00, &regval);
+	chg->mode = FIELD_GET(CHG_CNFG_00_MODE, regval);
+	ret = charger_set_mode(chg, CHG_MODE_OFF);
+	if (ret)
+		return ret;
+
+	ret = set_fast_charge_current_limit(chg, CHG_CC_DEFAULT_UA);
+	if (ret)
+		return ret;
+
+	ret = set_float_voltage_limit(chg, CHG_FV_DEFAULT_MV);
+	if (ret)
+		return ret;
+
+	ret = unlock_prot_regs(chg, true);
+	if (ret)
+		return ret;
+
+	/* Disable wireless charging input */
+	regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_12,
+			   CHG_CNFG_12_WCINSEL, 0);
+
+	regmap_update_bits(chg->regmap, MAX77759_CHGR_REG_CHG_CNFG_18,
+			   CHG_CNFG_18_WDTEN, 0);
+
+	return unlock_prot_regs(chg, false);
+}
+
+static void psy_work_item(struct work_struct *work)
+{
+	struct max77759_charger *chg =
+		container_of(work, struct max77759_charger, psy_work);
+	union power_supply_propval current_limit = { 0 }, online = { 0 };
+	int ret;
+
+	power_supply_get_property(chg->tcpm_psy, POWER_SUPPLY_PROP_CURRENT_MAX,
+				  &current_limit);
+	power_supply_get_property(chg->tcpm_psy, POWER_SUPPLY_PROP_ONLINE,
+				  &online);
+
+	if (online.intval && current_limit.intval) {
+		ret = set_input_current_limit(chg, current_limit.intval);
+		if (ret)
+			dev_err(chg->dev,
+				"Unable to set current limit, ret=%d", ret);
+
+		charger_set_mode(chg, CHG_MODE_CHG_BUCK_ON);
+	} else {
+		charger_set_mode(chg, CHG_MODE_OFF);
+	}
+}
+
+static int psy_changed(struct notifier_block *nb, unsigned long evt, void *data)
+{
+	struct max77759_charger *chg = container_of(nb, struct max77759_charger,
+						    nb);
+	const char *psy_name = "tcpm-source";
+	struct power_supply *psy = data;
+
+	if (!strnstr(psy->desc->name, psy_name, strlen(psy_name)) ||
+	    evt != PSY_EVENT_PROP_CHANGED)
+		return NOTIFY_OK;
+
+	chg->tcpm_psy = psy;
+	schedule_work(&chg->psy_work);
+	return NOTIFY_OK;
+}
+
+static void max_tcpci_unregister_psy_notifier(void *nb)
+{
+	power_supply_unreg_notifier(nb);
+}
+
+static int max77759_charger_probe(struct platform_device *pdev)
+{
+	struct regulator_config usb_otg_reg_cfg;
+	struct power_supply_config psy_cfg;
+	struct device *dev = &pdev->dev;
+	struct max77759_charger *chg;
+	int ret;
+
+	chg = devm_kzalloc(dev, sizeof(*chg), GFP_KERNEL);
+	if (!chg)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, chg);
+	chg->dev = dev;
+	chg->regmap = dev_get_regmap(dev->parent, "charger");
+	if (!chg->regmap)
+		return dev_err_probe(dev, -ENODEV, "Missing regmap");
+
+	ret = devm_mutex_init(dev, &chg->lock);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize lock");
+
+	ret = max77759_charger_init(chg);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to initialize max77759 charger");
+
+	usb_otg_reg_cfg.dev = dev;
+	usb_otg_reg_cfg.driver_data = chg;
+	usb_otg_reg_cfg.of_node = dev_of_node(dev);
+	chg->usb_otg_rdev = devm_regulator_register(dev, &usb_otg_reg_desc,
+						    &usb_otg_reg_cfg);
+	if (IS_ERR(chg->usb_otg_rdev))
+		return dev_err_probe(dev, PTR_ERR(chg->usb_otg_rdev),
+				     "Failed to register usb otg regulator");
+
+	psy_cfg.fwnode = dev_fwnode(dev);
+	psy_cfg.drv_data = chg;
+	chg->psy = devm_power_supply_register(dev, &max77759_charger_desc,
+					      &psy_cfg);
+	if (IS_ERR(chg->psy))
+		return dev_err_probe(dev, -EPROBE_DEFER,
+				     "Failed to register psy, ret=%ld",
+				     PTR_ERR(chg->psy));
+
+	ret = devm_work_autocancel(dev, &chg->psy_work, psy_work_item);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to initialize psy work");
+
+	chg->nb.notifier_call = psy_changed;
+	ret = power_supply_reg_notifier(&chg->nb);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Unable to register psy notifier");
+
+	ret = devm_add_action_or_reset(dev, max_tcpci_unregister_psy_notifier,
+				       &chg->nb);
+	if (ret)
+		return ret;
+
+	ret = max77759_init_irqhandler(chg);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Unable to initialize irq handler");
+	return 0;
+}
+
+static const struct of_device_id max77759_charger_ids[] = {
+	{ .compatible = "maxim,max77759-charger", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, max77759_charger_ids);
+
+static struct platform_driver max77759_charger_driver = {
+	.driver = {
+		.name = "max77759-charger",
+		.of_match_table = max77759_charger_ids,
+	},
+	.probe = max77759_charger_probe,
+};
+module_platform_driver(max77759_charger_driver);
+
+MODULE_AUTHOR("Amit Sunil Dhamne <amitsd@google.com>");
+MODULE_DESCRIPTION("Maxim MAX77759 charger driver");
+MODULE_LICENSE("GPL");

-- 
2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH 5/6] power: supply: max77759: add charger driver

[André Draszik]

Hi Amit,

Just a quick comment below for something I noticed during a brief look.

On Sun, 2025-11-23 at 08:35 +0000, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
> 
> Add support for MAX77759 battery charger driver. This is a 4A 1-Cell
> Li+/LiPoly dual input switch mode charger. While the device can support
> USB & wireless charger inputs, this implementation only supports USB
> input. This implementation supports both buck and boost modes.
> 
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
>  MAINTAINERS                             |   7 +
>  drivers/mfd/max77759.c                  |   3 +-
>  drivers/power/supply/Kconfig            |  11 +
>  drivers/power/supply/Makefile           |   1 +
>  drivers/power/supply/max77759_charger.c | 866 ++++++++++++++++++++++++++++++++
>  5 files changed, 887 insertions(+), 1 deletion(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index fed6cd812d79..f1b1015c08b5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15053,6 +15053,13 @@ F:	drivers/mfd/max77759.c
>  F:	drivers/nvmem/max77759-nvmem.c
>  F:	include/linux/mfd/max77759.h
>  
> +MAXIM MAX77759 BATTERY CHARGER DRIVER
> +M:	Amit Sunil Dhamne <amitsd@google.com>
> +L:	linux-kernel@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/power/supply/maxim,max77759-charger.yaml
> +F:	drivers/power/supply/max77759_charger.c
> +
>  MAXIM MAX77802 PMIC REGULATOR DEVICE DRIVER
>  M:	Javier Martinez Canillas <javier@dowhile0.org>
>  L:	linux-kernel@vger.kernel.org
> diff --git a/drivers/mfd/max77759.c b/drivers/mfd/max77759.c
> index 5fe22884f362..8a22838be1b0 100644
> --- a/drivers/mfd/max77759.c
> +++ b/drivers/mfd/max77759.c
> @@ -349,7 +349,8 @@ static const struct mfd_cell max77759_maxq_cells[] = {
>  };
>  
>  static const struct mfd_cell max77759_charger_cells[] = {
> -	MFD_CELL_RES("max77759-charger", max77759_charger_resources),
> +	MFD_CELL_OF("max77759-charger", max77759_charger_resources, NULL, 0, 0,
> +		    "maxim,max77759-charger"),
>  };
>  
>  int max77759_maxq_command(struct max77759 *max77759,
> diff --git a/drivers/power/supply/Kconfig b/drivers/power/supply/Kconfig
> index 79ddb006e2da..b97990cc0b92 100644
> --- a/drivers/power/supply/Kconfig
> +++ b/drivers/power/supply/Kconfig
> @@ -1074,4 +1074,15 @@ config FUEL_GAUGE_MM8013
>  	  the state of charge, temperature, cycle count, actual and design
>  	  capacity, etc.
>  
> +config CHARGER_MAX77759
> +	tristate "MAX77759 Charger Driver"
> +	depends on MFD_MAX77759
> +	default MFD_MAX77759
> +	help
> +	  Say M or Y here to enable the MAX77759 Charger Driver. MAX77759
> +	  charger is a function of the MAX77759 PMIC. This is a dual input
> +	  switch-mode charger. This driver supports buck and OTG boost modes.
> +
> +	  If built as a module, it will be called max77759_charger.
> +
>  endif # POWER_SUPPLY
> diff --git a/drivers/power/supply/Makefile b/drivers/power/supply/Makefile
> index f943c9150b32..12669734cfe3 100644
> --- a/drivers/power/supply/Makefile
> +++ b/drivers/power/supply/Makefile
> @@ -122,3 +122,4 @@ obj-$(CONFIG_CHARGER_SURFACE)	+= surface_charger.o
>  obj-$(CONFIG_BATTERY_UG3105)	+= ug3105_battery.o
>  obj-$(CONFIG_CHARGER_QCOM_SMB2)	+= qcom_smbx.o
>  obj-$(CONFIG_FUEL_GAUGE_MM8013)	+= mm8013.o
> +obj-$(CONFIG_CHARGER_MAX77759)	+= max77759_charger.o
> diff --git a/drivers/power/supply/max77759_charger.c b/drivers/power/supply/max77759_charger.c
> new file mode 100644
> index 000000000000..51637e87182b
> --- /dev/null
> +++ b/drivers/power/supply/max77759_charger.c
> @@ -0,0 +1,866 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * max77759_charger.c - Battery charger driver for MAX77759 charger device.
> + *
> + * Copyright 2025 Google LLC.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> +#include <linux/device.h>
> +#include <linux/devm-helpers.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/math64.h>
> +#include <linux/mfd/max77759.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/power_supply.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/driver.h>
> +#include <linux/string_choices.h>
> +
> +/* CHG_INT_OK */
> +#define CHG_INT_OK_AICL_OK			BIT(7)
> +#define CHG_INT_OK_CHGIN_OK			BIT(6)
> +#define CHG_INT_OK_CHG_OK			BIT(4)
> +#define CHG_INT_OK_INLIM_OK			BIT(2)
> +

[...]

> +static irqreturn_t irq_handler(int irq, void *data)
> +{
> +	struct max77759_charger *chg = data;
> +	u32 irq_status, chgint_ok, idx = 0;
> +	int ret;
> +
> +	if (irq == chg->irq[0])
> +		idx = 0;
> +	else
> +		idx = 1;
> +
> +	ret = regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_INT + idx,
> +			  &irq_status);
> +	if (ret) {
> +		dev_err(chg->dev, "regmap_read_error idx=%d ret=%d", idx, ret);
> +		return IRQ_HANDLED;
> +	}
> +
> +	regmap_write(chg->regmap, MAX77759_CHGR_REG_CHG_INT + idx,
> +		     irq_status);
> +	regmap_read(chg->regmap, MAX77759_CHGR_REG_CHG_INT_OK, &chgint_ok);
> +
> +	if (idx == 0) {
> +		if (irq_status & MAX77759_CHGR_REG_CHG_INT_AICL)
> +			dev_dbg(chg->dev, "AICL mode: %s",
> +				str_no_yes(chgint_ok & CHG_INT_OK_AICL_OK));
> +
> +		if (irq_status & MAX77759_CHGR_REG_CHG_INT_CHGIN)
> +			dev_dbg(chg->dev, "CHGIN input valid: %s",
> +				str_yes_no(chgint_ok & CHG_INT_OK_CHGIN_OK));
> +
> +		if (irq_status & MAX77759_CHGR_REG_CHG_INT_CHG)
> +			dev_dbg(chg->dev, "CHG status okay/off: %s",
> +				str_yes_no(chgint_ok & CHG_INT_OK_CHG_OK));
> +
> +		if (irq_status & MAX77759_CHGR_REG_CHG_INT_INLIM)
> +			dev_dbg(chg->dev, "Current Limit reached: %s",
> +				str_no_yes(chgint_ok & CHG_INT_OK_INLIM_OK));
> +	} else {
> +		if (irq_status & MAX77759_CHGR_REG_CHG_INT2_BAT_OILO)
> +			dev_dbg(chg->dev,
> +				"Battery over-current threshold crossed");
> +
> +		if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CC)
> +			dev_dbg(chg->dev, "Charger reached CC stage");
> +
> +		if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_CV)
> +			dev_dbg(chg->dev, "Charger reached CV stage");
> +
> +		if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_TO)
> +			dev_dbg(chg->dev, "Charger reached TO stage");
> +
> +		if (irq_status & MAX77759_CHGR_REG_CHG_INT2_CHG_STA_DONE)
> +			dev_dbg(chg->dev, "Charger reached Done stage");
> +	}
> +
> +	power_supply_changed(chg->psy);
> +	return IRQ_HANDLED;
> +}
> +
> +static int max77759_init_irqhandler(struct max77759_charger *chg)
> +{
> +	static const char * const irq_res_names[] = { "INT1", "INT2" };
> +	struct device *dev = chg->dev;
> +	unsigned long irq_flags;
> +	struct irq_data *irqd;
> +	int *irq = chg->irq;
> +	int ret, i;
> +
> +	for (i = 0; i < 2; i++) {
> +		irq[i] = platform_get_irq_byname(to_platform_device(dev),
> +						 irq_res_names[i]);
> +		if (irq[i] < 0) {
> +			dev_err(dev, "unable to find %s irq", irq_res_names[i]);
> +			return irq[i];
> +		}
> +
> +		irq_flags = IRQF_ONESHOT | IRQF_SHARED;
> +		irqd = irq_get_irq_data(irq[i]);
> +		if (irqd)
> +			irq_flags |= irqd_get_trigger_type(irqd);
> +
> +		ret = devm_request_threaded_irq(dev, irq[i], NULL, irq_handler,
> +						irq_flags, dev_name(dev), chg);
> +		if (ret) {
> +			dev_err(dev, "Unable to register threaded irq handler");
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +}

The way you're requesting the IRQ here I don't think you need to modify
max77759_chgr_irqs[] in patch 4 at all. You're requesting the interrupt
and are manually demultiplexing and acknowledging the actual event in your
IRQ handler.

Your change in patch 4 would instead allow your charger driver to call
request_irq for each individual bit of the interrupt register(s), letting
regmap-irq demultiplex and acknowledge the event for you before calling
your handler(s). As an example, see how the ACPM result interrupt is
requested in max77759_add_chained_maxq().

It's probably cleaner to let regmap-irq deal with demultiplexing etc.


Cheers,
Andre

Re: [PATCH 5/6] power: supply: max77759: add charger driver

[kernel test robot]

Hi Amit,

kernel test robot noticed the following build errors:

[auto build test ERROR on 39f90c1967215375f7d87b81d14b0f3ed6b40c29]

url:    https://github.com/intel-lab-lkp/linux/commits/Amit-Sunil-Dhamne-via-B4-Relay/dt-bindings-power-supply-Add-Maxim-MAX77759-charger/20251123-163840
base:   39f90c1967215375f7d87b81d14b0f3ed6b40c29
patch link:    https://lore.kernel.org/r/20251123-max77759-charger-v1-5-6b2e4b8f7f54%40google.com
patch subject: [PATCH 5/6] power: supply: max77759: add charger driver
config: um-randconfig-001-20251126 (https://download.01.org/0day-ci/archive/20251126/202511261521.hSYp4ttf-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251126/202511261521.hSYp4ttf-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511261521.hSYp4ttf-lkp@intel.com/

All errors (new ones prefixed by >>):

   /usr/bin/ld: warning: .tmp_vmlinux1 has a LOAD segment with RWX permissions
   /usr/bin/ld: drivers/power/supply/max77759_charger.o: in function `max77759_charger_probe':
>> max77759_charger.c:(.ltext+0x27b): undefined reference to `devm_regulator_register'
   /usr/bin/ld: drivers/power/supply/max77759_charger.o: in function `enable_usb_otg':
>> max77759_charger.c:(.ltext+0x983): undefined reference to `rdev_get_drvdata'
   /usr/bin/ld: drivers/power/supply/max77759_charger.o: in function `disable_usb_otg':
   max77759_charger.c:(.ltext+0x9c3): undefined reference to `rdev_get_drvdata'
   /usr/bin/ld: drivers/power/supply/max77759_charger.o: in function `usb_otg_status':
   max77759_charger.c:(.ltext+0xa06): undefined reference to `rdev_get_drvdata'
   clang: error: linker command failed with exit code 1 (use -v to see invocation)

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 5/6] power: supply: max77759: add charger driver

[Amit Sunil Dhamne]

On 11/25/25 11:38 PM, kernel test robot wrote:
> Hi Amit,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on 39f90c1967215375f7d87b81d14b0f3ed6b40c29]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Amit-Sunil-Dhamne-via-B4-Relay/dt-bindings-power-supply-Add-Maxim-MAX77759-charger/20251123-163840
> base:   39f90c1967215375f7d87b81d14b0f3ed6b40c29
> patch link:    https://lore.kernel.org/r/20251123-max77759-charger-v1-5-6b2e4b8f7f54%40google.com
> patch subject: [PATCH 5/6] power: supply: max77759: add charger driver
> config: um-randconfig-001-20251126 (https://download.01.org/0day-ci/archive/20251126/202511261521.hSYp4ttf-lkp@intel.com/config)
> compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251126/202511261521.hSYp4ttf-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202511261521.hSYp4ttf-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>     /usr/bin/ld: warning: .tmp_vmlinux1 has a LOAD segment with RWX permissions
>     /usr/bin/ld: drivers/power/supply/max77759_charger.o: in function `max77759_charger_probe':
>>> max77759_charger.c:(.ltext+0x27b): undefined reference to `devm_regulator_register'
>     /usr/bin/ld: drivers/power/supply/max77759_charger.o: in function `enable_usb_otg':
>>> max77759_charger.c:(.ltext+0x983): undefined reference to `rdev_get_drvdata'
>     /usr/bin/ld: drivers/power/supply/max77759_charger.o: in function `disable_usb_otg':
>     max77759_charger.c:(.ltext+0x9c3): undefined reference to `rdev_get_drvdata'
>     /usr/bin/ld: drivers/power/supply/max77759_charger.o: in function `usb_otg_status':
>     max77759_charger.c:(.ltext+0xa06): undefined reference to `rdev_get_drvdata'
>     clang: error: linker command failed with exit code 1 (use -v to see invocation)
I believe this is because of a missing "depends on REGULATOR". Will fix 
it in the next revision.

[PATCH 6/6] usb: typec: tcpm/tcpci_maxim: deprecate WAR for setting charger mode

[Amit Sunil Dhamne via B4 Relay]

From: Amit Sunil Dhamne <amitsd@google.com>

TCPCI maxim driver directly writes to the charger's register space to
set charger mode depending on the power role. As MAX77759 chg driver
exists, this WAR is not required.

Instead, use a regulator interface to set OTG Boost mode.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
 drivers/usb/typec/tcpm/tcpci_maxim.h      |  1 +
 drivers/usb/typec/tcpm/tcpci_maxim_core.c | 48 +++++++++++++++++++++----------
 2 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
index b33540a42a95..6c82a61efe46 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim.h
+++ b/drivers/usb/typec/tcpm/tcpci_maxim.h
@@ -60,6 +60,7 @@ struct max_tcpci_chip {
 	struct tcpm_port *port;
 	enum contamiant_state contaminant_state;
 	bool veto_vconn_swap;
+	struct regulator *otg_reg;
 };
 
 static inline int max_tcpci_read16(struct max_tcpci_chip *chip, unsigned int reg, u16 *val)
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
index 19f638650796..6d819a762fa1 100644
--- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
+++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
 #include <linux/usb/pd.h>
 #include <linux/usb/tcpci.h>
 #include <linux/usb/tcpm.h>
@@ -202,32 +203,49 @@ static void process_rx(struct max_tcpci_chip *chip, u16 status)
 	tcpm_pd_receive(chip->port, &msg, rx_type);
 }
 
+static int get_otg_regulator_handle(struct max_tcpci_chip *chip)
+{
+	if (IS_ERR_OR_NULL(chip->otg_reg)) {
+		chip->otg_reg = devm_regulator_get_exclusive(chip->dev,
+							     "otg-vbus");
+		if (IS_ERR_OR_NULL(chip->otg_reg)) {
+			dev_err(chip->dev,
+				"Failed to get otg regulator handle");
+			return -ENODEV;
+		}
+	}
+
+	return 0;
+}
+
 static int max_tcpci_set_vbus(struct tcpci *tcpci, struct tcpci_data *tdata, bool source, bool sink)
 {
 	struct max_tcpci_chip *chip = tdata_to_max_tcpci(tdata);
-	u8 buffer_source[2] = {MAX_BUCK_BOOST_OP, MAX_BUCK_BOOST_SOURCE};
-	u8 buffer_sink[2] = {MAX_BUCK_BOOST_OP, MAX_BUCK_BOOST_SINK};
-	u8 buffer_none[2] = {MAX_BUCK_BOOST_OP, MAX_BUCK_BOOST_OFF};
-	struct i2c_client *i2c = chip->client;
 	int ret;
 
-	struct i2c_msg msgs[] = {
-		{
-			.addr = MAX_BUCK_BOOST_SID,
-			.flags = i2c->flags & I2C_M_TEN,
-			.len = 2,
-			.buf = source ? buffer_source : sink ? buffer_sink : buffer_none,
-		},
-	};
-
 	if (source && sink) {
 		dev_err(chip->dev, "Both source and sink set\n");
 		return -EINVAL;
 	}
 
-	ret = i2c_transfer(i2c->adapter, msgs, 1);
+	ret = get_otg_regulator_handle(chip);
+	if (ret) {
+		/*
+		 * Regulator is not necessary for sink only applications. Return
+		 * success in cases where sink mode is being modified.
+		 */
+		return source ? ret : 1;
+	}
+
+	if (source) {
+		if (!regulator_is_enabled(chip->otg_reg))
+			ret = regulator_enable(chip->otg_reg);
+	} else {
+		if (regulator_is_enabled(chip->otg_reg))
+			ret = regulator_disable(chip->otg_reg);
+	}
 
-	return  ret < 0 ? ret : 1;
+	return ret < 0 ? ret : 1;
 }
 
 static void process_power_status(struct max_tcpci_chip *chip)

-- 
2.52.0.rc2.455.g230fcf2819-goog

Re: [PATCH 6/6] usb: typec: tcpm/tcpci_maxim: deprecate WAR for setting charger mode

[André Draszik]

Hi Amit,

On Sun, 2025-11-23 at 08:35 +0000, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
> 
> TCPCI maxim driver directly writes to the charger's register space to
> set charger mode depending on the power role. As MAX77759 chg driver
> exists, this WAR is not required.
> 
> Instead, use a regulator interface to set OTG Boost mode.
> 
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
>  drivers/usb/typec/tcpm/tcpci_maxim.h      |  1 +
>  drivers/usb/typec/tcpm/tcpci_maxim_core.c | 48 +++++++++++++++++++++----------
>  2 files changed, 34 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
> index b33540a42a95..6c82a61efe46 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim.h
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim.h
> @@ -60,6 +60,7 @@ struct max_tcpci_chip {
>  	struct tcpm_port *port;
>  	enum contamiant_state contaminant_state;
>  	bool veto_vconn_swap;
> +	struct regulator *otg_reg;
>  };
>  
>  static inline int max_tcpci_read16(struct max_tcpci_chip *chip, unsigned int reg, u16 *val)
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> index 19f638650796..6d819a762fa1 100644
> --- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> +++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
> @@ -10,6 +10,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
>  #include <linux/usb/pd.h>
>  #include <linux/usb/tcpci.h>
>  #include <linux/usb/tcpm.h>
> @@ -202,32 +203,49 @@ static void process_rx(struct max_tcpci_chip *chip, u16 status)
>  	tcpm_pd_receive(chip->port, &msg, rx_type);
>  }
>  
> +static int get_otg_regulator_handle(struct max_tcpci_chip *chip)
> +{
> +	if (IS_ERR_OR_NULL(chip->otg_reg)) {
> +		chip->otg_reg = devm_regulator_get_exclusive(chip->dev,
> +							     "otg-vbus");
> +		if (IS_ERR_OR_NULL(chip->otg_reg)) {
> +			dev_err(chip->dev,
> +				"Failed to get otg regulator handle");
> +			return -ENODEV;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int max_tcpci_set_vbus(struct tcpci *tcpci, struct tcpci_data *tdata, bool source, bool sink)
>  {
>  	struct max_tcpci_chip *chip = tdata_to_max_tcpci(tdata);
> -	u8 buffer_source[2] = {MAX_BUCK_BOOST_OP, MAX_BUCK_BOOST_SOURCE};
> -	u8 buffer_sink[2] = {MAX_BUCK_BOOST_OP, MAX_BUCK_BOOST_SINK};
> -	u8 buffer_none[2] = {MAX_BUCK_BOOST_OP, MAX_BUCK_BOOST_OFF};

You should also remove the corresponding #defines at the top of this file.

> -	struct i2c_client *i2c = chip->client;
>  	int ret;
>  
> -	struct i2c_msg msgs[] = {
> -		{
> -			.addr = MAX_BUCK_BOOST_SID,
> -			.flags = i2c->flags & I2C_M_TEN,
> -			.len = 2,
> -			.buf = source ? buffer_source : sink ? buffer_sink : buffer_none,
> -		},
> -	};
> -
>  	if (source && sink) {
>  		dev_err(chip->dev, "Both source and sink set\n");
>  		return -EINVAL;
>  	}
>  
> -	ret = i2c_transfer(i2c->adapter, msgs, 1);
> +	ret = get_otg_regulator_handle(chip);
> +	if (ret) {
> +		/*
> +		 * Regulator is not necessary for sink only applications. Return
> +		 * success in cases where sink mode is being modified.
> +		 */
> +		return source ? ret : 1;
> +	}
> +
> +	if (source) {
> +		if (!regulator_is_enabled(chip->otg_reg))
> +			ret = regulator_enable(chip->otg_reg);
> +	} else {
> +		if (regulator_is_enabled(chip->otg_reg))
> +			ret = regulator_disable(chip->otg_reg);
> +	}

Given otg_reg is the fake regulator created by a previous patch in this
series, this means that the regulator API is really used to configure two
out of 16 possible charger modes here. That doesn't look right.

Cheers,
Andre'

Re: [PATCH 6/6] usb: typec: tcpm/tcpci_maxim: deprecate WAR for setting charger mode

[Amit Sunil Dhamne]

On 11/24/25 1:09 AM, André Draszik wrote:
> Hi Amit,
>
> On Sun, 2025-11-23 at 08:35 +0000, Amit Sunil Dhamne via B4 Relay wrote:
>> From: Amit Sunil Dhamne <amitsd@google.com>
>>
>> TCPCI maxim driver directly writes to the charger's register space to
>> set charger mode depending on the power role. As MAX77759 chg driver
>> exists, this WAR is not required.
>>
>> Instead, use a regulator interface to set OTG Boost mode.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>>   drivers/usb/typec/tcpm/tcpci_maxim.h      |  1 +
>>   drivers/usb/typec/tcpm/tcpci_maxim_core.c | 48 +++++++++++++++++++++----------
>>   2 files changed, 34 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
>> index b33540a42a95..6c82a61efe46 100644
>> --- a/drivers/usb/typec/tcpm/tcpci_maxim.h
>> +++ b/drivers/usb/typec/tcpm/tcpci_maxim.h
>> @@ -60,6 +60,7 @@ struct max_tcpci_chip {
>>   	struct tcpm_port *port;
>>   	enum contamiant_state contaminant_state;
>>   	bool veto_vconn_swap;
>> +	struct regulator *otg_reg;
>>   };
>>   
>>   static inline int max_tcpci_read16(struct max_tcpci_chip *chip, unsigned int reg, u16 *val)
>> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim_core.c b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
>> index 19f638650796..6d819a762fa1 100644
>> --- a/drivers/usb/typec/tcpm/tcpci_maxim_core.c
>> +++ b/drivers/usb/typec/tcpm/tcpci_maxim_core.c
>> @@ -10,6 +10,7 @@
>>   #include <linux/kernel.h>
>>   #include <linux/module.h>
>>   #include <linux/regmap.h>
>> +#include <linux/regulator/consumer.h>
>>   #include <linux/usb/pd.h>
>>   #include <linux/usb/tcpci.h>
>>   #include <linux/usb/tcpm.h>
>> @@ -202,32 +203,49 @@ static void process_rx(struct max_tcpci_chip *chip, u16 status)
>>   	tcpm_pd_receive(chip->port, &msg, rx_type);
>>   }
>>   
>> +static int get_otg_regulator_handle(struct max_tcpci_chip *chip)
>> +{
>> +	if (IS_ERR_OR_NULL(chip->otg_reg)) {
>> +		chip->otg_reg = devm_regulator_get_exclusive(chip->dev,
>> +							     "otg-vbus");
>> +		if (IS_ERR_OR_NULL(chip->otg_reg)) {
>> +			dev_err(chip->dev,
>> +				"Failed to get otg regulator handle");
>> +			return -ENODEV;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>>   static int max_tcpci_set_vbus(struct tcpci *tcpci, struct tcpci_data *tdata, bool source, bool sink)
>>   {
>>   	struct max_tcpci_chip *chip = tdata_to_max_tcpci(tdata);
>> -	u8 buffer_source[2] = {MAX_BUCK_BOOST_OP, MAX_BUCK_BOOST_SOURCE};
>> -	u8 buffer_sink[2] = {MAX_BUCK_BOOST_OP, MAX_BUCK_BOOST_SINK};
>> -	u8 buffer_none[2] = {MAX_BUCK_BOOST_OP, MAX_BUCK_BOOST_OFF};
> You should also remove the corresponding #defines at the top of this file.

Ack!


>
>> -	struct i2c_client *i2c = chip->client;
>>   	int ret;
>>   
>> -	struct i2c_msg msgs[] = {
>> -		{
>> -			.addr = MAX_BUCK_BOOST_SID,
>> -			.flags = i2c->flags & I2C_M_TEN,
>> -			.len = 2,
>> -			.buf = source ? buffer_source : sink ? buffer_sink : buffer_none,
>> -		},
>> -	};
>> -
>>   	if (source && sink) {
>>   		dev_err(chip->dev, "Both source and sink set\n");
>>   		return -EINVAL;
>>   	}
>>   
>> -	ret = i2c_transfer(i2c->adapter, msgs, 1);
>> +	ret = get_otg_regulator_handle(chip);
>> +	if (ret) {
>> +		/*
>> +		 * Regulator is not necessary for sink only applications. Return
>> +		 * success in cases where sink mode is being modified.
>> +		 */
>> +		return source ? ret : 1;
>> +	}
>> +
>> +	if (source) {
>> +		if (!regulator_is_enabled(chip->otg_reg))
>> +			ret = regulator_enable(chip->otg_reg);
>> +	} else {
>> +		if (regulator_is_enabled(chip->otg_reg))
>> +			ret = regulator_disable(chip->otg_reg);
>> +	}
> Given otg_reg is the fake regulator created by a previous patch in this
> series, this means that the regulator API is really used to configure two
> out of 16 possible charger modes here. That doesn't look right.


I understand the concern, but I believe the regulator framework is the 
correct interface here for three reasons:

1.  The TCPCI driver's responsibility is only to signal the intent to 
source VBUS (OTG) or not. It should not be aware of the specific 
register values or mode bits of the companion charger chip. The charger 
driver (via the regulator enable op) is responsible for translating that 
intent into the correct hardware-specific register value (e.g., 
selecting the correct OTG mode).

2. While the charger supports multiple modes, sourcing VBUS via OTG and 
sinking VBUS (charging) are mutually exclusive states on Type-C and 
there are no modes to support both. If complex scenarios arise (like 
wireless charging + USB OTG), the logic to select the specific register 
mode belongs in the charger driver (the regulator provider), not the 
TCPCI consumer. Now say theoretically, we support wireless charging and 
want to turn it on (sink) while USB OTG mode (source) is on. The charger 
driver would set an appropriate mode based on this info (wireless 
power-supply on, USB otg regulator on, so select XX mode). We can game 
this for any 10 of the *valid* modes (refer [1]) supported by the chip 
and this would work. Using a regulator framework here will not pose a 
restriction on the number of charger modes now or in the future.

3. This design pattern is standard in the kernel from my study. Drivers 
such as bq24190_charger and rt9471 expose USB OTG functionality via the 
regulator framework.

[1] 
https://android.googlesource.com/kernel/google-modules/bms/+/refs/heads/android-gs-raviole-mainline/max77759_regs.h#52

BR,

Amit

>
> Cheers,
> Andre'