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 source vbus when typec is in
source power mode. In other power modes, this regulator will be turned
off if active.
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 | 54 +++++++++++++++++++------------
2 files changed, 34 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
index b33540a42a95..b314606eb0f6 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 *vbus_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..e9e2405c5ca0 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>
@@ -35,12 +36,6 @@
*/
#define TCPC_RECEIVE_BUFFER_LEN 32
-#define MAX_BUCK_BOOST_SID 0x69
-#define MAX_BUCK_BOOST_OP 0xb9
-#define MAX_BUCK_BOOST_OFF 0
-#define MAX_BUCK_BOOST_SOURCE 0xa
-#define MAX_BUCK_BOOST_SINK 0x5
-
static const struct regmap_range max_tcpci_tcpci_range[] = {
regmap_reg_range(0x00, 0x95)
};
@@ -202,32 +197,49 @@ static void process_rx(struct max_tcpci_chip *chip, u16 status)
tcpm_pd_receive(chip->port, &msg, rx_type);
}
+static int get_vbus_regulator_handle(struct max_tcpci_chip *chip)
+{
+ if (IS_ERR_OR_NULL(chip->vbus_reg)) {
+ chip->vbus_reg = devm_regulator_get_exclusive(chip->dev,
+ "vbus");
+ if (IS_ERR_OR_NULL(chip->vbus_reg)) {
+ dev_err(chip->dev,
+ "Failed to get vbus 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_vbus_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->vbus_reg))
+ ret = regulator_enable(chip->vbus_reg);
+ } else {
+ if (regulator_is_enabled(chip->vbus_reg))
+ ret = regulator_disable(chip->vbus_reg);
+ }
- return ret < 0 ? ret : 1;
+ return ret < 0 ? ret : 1;
}
static void process_power_status(struct max_tcpci_chip *chip)
--
2.52.0.351.gbe84eed79e-googHi,
> + if (source) {
> + if (!regulator_is_enabled(chip->vbus_reg))
> + ret = regulator_enable(chip->vbus_reg);
> + } else {
> + if (regulator_is_enabled(chip->vbus_reg))
> + ret = regulator_disable(chip->vbus_reg);
> + }
It looks like you have to do one more round, so can drop the
regulator_is_enabled() checks and just always enable/disable it
unconditionally.
if (source)
ret = regulator_enable(chip->vbus_reg);
else
ret = regulator_disable(chip->vbus_reg);
I don't think you need the check in any case, but if I've understood
this correctly, you should not use that check when the regulator does
not support that check because then the API claims it's always
enabled. So I guess in that case "if (!regulator_is_enabled())" may
not work as expected, and you may actually be left with a disabled
regulator. This may not be a problem on current platforms, but who
knows what happens in the future.
thanks,
--
heikki
Hi Heikki,
Thanks for the review!
On 1/9/26 5:14 AM, Heikki Krogerus wrote:
> Hi,
>
>> + if (source) {
>> + if (!regulator_is_enabled(chip->vbus_reg))
>> + ret = regulator_enable(chip->vbus_reg);
>> + } else {
>> + if (regulator_is_enabled(chip->vbus_reg))
>> + ret = regulator_disable(chip->vbus_reg);
>> + }
> It looks like you have to do one more round, so can drop the
> regulator_is_enabled() checks and just always enable/disable it
> unconditionally.
>
> if (source)
> ret = regulator_enable(chip->vbus_reg);
> else
> ret = regulator_disable(chip->vbus_reg);
The regulator framework uses refcounting on the number of enables. If
the number of times regulator is disabled > enabled, a warning will be
thrown. Also, I don't want to call regulator_enable more than once for
the same refcounting reason (will have to call disable those many number
of times to actually disable).
> I don't think you need the check in any case, but if I've understood
> this correctly, you should not use that check when the regulator does
> not support that check because then the API claims it's always
> enabled. So I guess in that case "if (!regulator_is_enabled())" may
> not work as expected, and you may actually be left with a disabled
> regulator. This may not be a problem on current platforms, but who
> knows what happens in the future.
I don't think this should be an issue in the future as this driver is
specifically meant for max77759_tcpci device and should only be used
with max77759 charger (they both exist only in the same package). And
that the max77759_charger driver does implement the callback. However,
if you think that regulator_is_enabled() is unreliable, I could track
the state within the tcpci driver instead of calling
regulator_is_enabled() and call enable/disable regulator accordingly.
Let me know wdyt and I'll update the next revision accordingly.
BR,
Amit
>
> thanks,
>
Fri, Jan 09, 2026 at 06:16:57PM -0800, Amit Sunil Dhamne kirjoitti:
> Hi Heikki,
>
> Thanks for the review!
>
> On 1/9/26 5:14 AM, Heikki Krogerus wrote:
> > Hi,
> >
> >> + if (source) {
> >> + if (!regulator_is_enabled(chip->vbus_reg))
> >> + ret = regulator_enable(chip->vbus_reg);
> >> + } else {
> >> + if (regulator_is_enabled(chip->vbus_reg))
> >> + ret = regulator_disable(chip->vbus_reg);
> >> + }
> > It looks like you have to do one more round, so can drop the
> > regulator_is_enabled() checks and just always enable/disable it
> > unconditionally.
> >
> > if (source)
> > ret = regulator_enable(chip->vbus_reg);
> > else
> > ret = regulator_disable(chip->vbus_reg);
>
> The regulator framework uses refcounting on the number of enables. If
> the number of times regulator is disabled > enabled, a warning will be
> thrown. Also, I don't want to call regulator_enable more than once for
> the same refcounting reason (will have to call disable those many number
> of times to actually disable).
>
> > I don't think you need the check in any case, but if I've understood
> > this correctly, you should not use that check when the regulator does
> > not support that check because then the API claims it's always
> > enabled. So I guess in that case "if (!regulator_is_enabled())" may
> > not work as expected, and you may actually be left with a disabled
> > regulator. This may not be a problem on current platforms, but who
> > knows what happens in the future.
>
> I don't think this should be an issue in the future as this driver is
> specifically meant for max77759_tcpci device and should only be used
> with max77759 charger (they both exist only in the same package). And
> that the max77759_charger driver does implement the callback. However,
> if you think that regulator_is_enabled() is unreliable, I could track
> the state within the tcpci driver instead of calling
> regulator_is_enabled() and call enable/disable regulator accordingly.
>
> Let me know wdyt and I'll update the next revision accordingly.
Let's go with this then as is.
thanks,
--
heikki
On 1/12/26 5:20 AM, Heikki Krogerus wrote:
> Fri, Jan 09, 2026 at 06:16:57PM -0800, Amit Sunil Dhamne kirjoitti:
>> Hi Heikki,
>>
>> Thanks for the review!
>>
>> On 1/9/26 5:14 AM, Heikki Krogerus wrote:
>>> Hi,
>>>
>>>> + if (source) {
>>>> + if (!regulator_is_enabled(chip->vbus_reg))
>>>> + ret = regulator_enable(chip->vbus_reg);
>>>> + } else {
>>>> + if (regulator_is_enabled(chip->vbus_reg))
>>>> + ret = regulator_disable(chip->vbus_reg);
>>>> + }
>>> It looks like you have to do one more round, so can drop the
>>> regulator_is_enabled() checks and just always enable/disable it
>>> unconditionally.
>>>
>>> if (source)
>>> ret = regulator_enable(chip->vbus_reg);
>>> else
>>> ret = regulator_disable(chip->vbus_reg);
>> The regulator framework uses refcounting on the number of enables. If
>> the number of times regulator is disabled > enabled, a warning will be
>> thrown. Also, I don't want to call regulator_enable more than once for
>> the same refcounting reason (will have to call disable those many number
>> of times to actually disable).
>>
>>> I don't think you need the check in any case, but if I've understood
>>> this correctly, you should not use that check when the regulator does
>>> not support that check because then the API claims it's always
>>> enabled. So I guess in that case "if (!regulator_is_enabled())" may
>>> not work as expected, and you may actually be left with a disabled
>>> regulator. This may not be a problem on current platforms, but who
>>> knows what happens in the future.
>> I don't think this should be an issue in the future as this driver is
>> specifically meant for max77759_tcpci device and should only be used
>> with max77759 charger (they both exist only in the same package). And
>> that the max77759_charger driver does implement the callback. However,
>> if you think that regulator_is_enabled() is unreliable, I could track
>> the state within the tcpci driver instead of calling
>> regulator_is_enabled() and call enable/disable regulator accordingly.
>>
>> Let me know wdyt and I'll update the next revision accordingly.
> Let's go with this then as is.
Sounds good. Thanks!
>
> thanks,
>
On Sat, 2025-12-27 at 00:04 +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 source vbus when typec is in > source power mode. In other power modes, this regulator will be turned > off if active. > > 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 | 54 +++++++++++++++++++------------ > 2 files changed, 34 insertions(+), 21 deletions(-) Reviewed-by: André Draszik <andre.draszik@linaro.org>
Sat, Dec 27, 2025 at 12:04:25AM +0000, Amit Sunil Dhamne via B4 Relay kirjoitti:
> 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 source vbus when typec is in
> source power mode. In other power modes, this regulator will be turned
> off if active.
>
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/typec/tcpm/tcpci_maxim.h | 1 +
> drivers/usb/typec/tcpm/tcpci_maxim_core.c | 54 +++++++++++++++++++------------
> 2 files changed, 34 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm/tcpci_maxim.h b/drivers/usb/typec/tcpm/tcpci_maxim.h
> index b33540a42a95..b314606eb0f6 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 *vbus_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..e9e2405c5ca0 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>
> @@ -35,12 +36,6 @@
> */
> #define TCPC_RECEIVE_BUFFER_LEN 32
>
> -#define MAX_BUCK_BOOST_SID 0x69
> -#define MAX_BUCK_BOOST_OP 0xb9
> -#define MAX_BUCK_BOOST_OFF 0
> -#define MAX_BUCK_BOOST_SOURCE 0xa
> -#define MAX_BUCK_BOOST_SINK 0x5
> -
> static const struct regmap_range max_tcpci_tcpci_range[] = {
> regmap_reg_range(0x00, 0x95)
> };
> @@ -202,32 +197,49 @@ static void process_rx(struct max_tcpci_chip *chip, u16 status)
> tcpm_pd_receive(chip->port, &msg, rx_type);
> }
>
> +static int get_vbus_regulator_handle(struct max_tcpci_chip *chip)
> +{
> + if (IS_ERR_OR_NULL(chip->vbus_reg)) {
> + chip->vbus_reg = devm_regulator_get_exclusive(chip->dev,
> + "vbus");
> + if (IS_ERR_OR_NULL(chip->vbus_reg)) {
> + dev_err(chip->dev,
> + "Failed to get vbus 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_vbus_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->vbus_reg))
> + ret = regulator_enable(chip->vbus_reg);
> + } else {
> + if (regulator_is_enabled(chip->vbus_reg))
> + ret = regulator_disable(chip->vbus_reg);
> + }
>
> - return ret < 0 ? ret : 1;
> + return ret < 0 ? ret : 1;
> }
>
> static void process_power_status(struct max_tcpci_chip *chip)
>
> --
> 2.52.0.351.gbe84eed79e-goog
>
--
heikki
© 2016 - 2026 Red Hat, Inc.