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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <9cad7b42dbfea351fb3b708736bf6f6715bcf694.camel@linaro.org>
Date: Mon, 24 Nov 2025 09:09:37 +0000
From: André Draszik <andre.draszik@...aro.org>
To: amitsd@...gle.com, Sebastian Reichel <sre@...nel.org>, Rob Herring	
 <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley	
 <conor+dt@...nel.org>, Lee Jones <lee@...nel.org>, Greg Kroah-Hartman	
 <gregkh@...uxfoundation.org>, Badhri Jagan Sridharan <badhri@...gle.com>, 
 Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Peter Griffin
 <peter.griffin@...aro.org>, Tudor Ambarus	 <tudor.ambarus@...aro.org>, Alim
 Akhtar <alim.akhtar@...sung.com>
Cc: linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-usb@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-samsung-soc@...r.kernel.org, RD
 Babiera <rdbabiera@...gle.com>, Kyle Tso <kyletso@...gle.com>
Subject: Re: [PATCH 6/6] usb: typec: tcpm/tcpci_maxim: deprecate WAR for
 setting charger mode

Hi Amit,

On Sun, 2025-11-23 at 08:35 +0000, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@...gle.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@...gle.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'