Message-ID: <anawfylnc5afpljxeooruyy3lgxn3wizy57jruvnmro6akfsx7@zdzsnludbl6f>
Date: Tue, 24 Jun 2025 04:06:20 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Casey Connolly <casey.connolly@...aro.org>
Cc: Sebastian Reichel <sre@...nel.org>, Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>, Kees Cook <kees@...nel.org>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        linux-arm-msm@...r.kernel.org, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        Sebastian Reichel <sebastian.reichel@...labora.com>,
        linux-hardening@...r.kernel.org
Subject: Re: [PATCH 09/11] power: supply: qcom_smbx: add smb5 support

On Thu, Jun 19, 2025 at 04:55:17PM +0200, Casey Connolly wrote:
> Introduce support for the SMB5 charger found on pm8150b and other more
> modern Qualcomm SoCs.
> 
> SMB5 is largely similar to SMB2, with a few register differences. The
> main difference is the new Type-C hardware block which some registers
> are moved to.
> 
> Signed-off-by: Casey Connolly <casey.connolly@...aro.org>
> ---
>  drivers/power/supply/qcom_smbx.c | 367 +++++++++++++++++++++++++++++++++------
>  1 file changed, 314 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/power/supply/qcom_smbx.c b/drivers/power/supply/qcom_smbx.c
> index 10ddd33a09599decb23c0f1ccd02fa9b56602543..d902f3f43548191d3d0310ce90e699918ed0f16f 100644
> --- a/drivers/power/supply/qcom_smbx.c
> +++ b/drivers/power/supply/qcom_smbx.c
> @@ -22,18 +22,32 @@
>  #include <linux/regmap.h>
>  #include <linux/types.h>
>  #include <linux/workqueue.h>
>  
> +enum smb_generation {
> +	SMB2,
> +	SMB5,
> +};
> +
> +#define SMB_REG_OFFSET(smb) (smb->gen == SMB2 ? 0x600 : 0x100)
> +
>  /* clang-format off */
>  #define BATTERY_CHARGER_STATUS_1			0x06
>  #define BATTERY_CHARGER_STATUS_MASK			GENMASK(2, 0)
>  
>  #define BATTERY_CHARGER_STATUS_2			0x07
> -#define CHARGER_ERROR_STATUS_BAT_OV_BIT			BIT(5)
> -#define BAT_TEMP_STATUS_HOT_SOFT_LIMIT_BIT		BIT(3)
> -#define BAT_TEMP_STATUS_COLD_SOFT_LIMIT_BIT		BIT(2)
> -#define BAT_TEMP_STATUS_TOO_HOT_BIT			BIT(1)
> -#define BAT_TEMP_STATUS_TOO_COLD_BIT			BIT(0)
> +#define SMB2_CHARGER_ERROR_STATUS_BAT_OV_BIT		BIT(5)
> +#define SMB2_BAT_TEMP_STATUS_HOT_SOFT_LIMIT_BIT	BIT(3)
> +#define SMB2_BAT_TEMP_STATUS_COLD_SOFT_LIMIT_BIT	BIT(2)
> +#define SMB2_BAT_TEMP_STATUS_TOO_HOT_BIT		BIT(1)
> +#define SMB5_CHARGER_ERROR_STATUS_BAT_OV_BIT		BIT(1)
> +#define SMB2_BAT_TEMP_STATUS_TOO_COLD_BIT		BIT(0)
> +
> +#define BATTERY_CHARGER_STATUS_7			0x0D
> +#define SMB5_BAT_TEMP_STATUS_HOT_SOFT_BIT		BIT(5)
> +#define SMB5_BAT_TEMP_STATUS_COLD_SOFT_BIT		BIT(4)
> +#define SMB5_BAT_TEMP_STATUS_TOO_HOT_BIT		BIT(3)
> +#define SMB5_BAT_TEMP_STATUS_TOO_COLD_BIT		BIT(2)
>  
>  #define CHARGING_ENABLE_CMD				0x42
>  #define CHARGING_ENABLE_CMD_BIT				BIT(0)
>  
> @@ -55,11 +69,14 @@
>  
>  #define FLOAT_VOLTAGE_CFG				0x70
>  #define FLOAT_VOLTAGE_SETTING_MASK			GENMASK(7, 0)
>  
> -#define FG_UPDATE_CFG_2_SEL				0x7D
> -#define SOC_LT_CHG_RECHARGE_THRESH_SEL_BIT		BIT(2)
> -#define VBT_LT_CHG_RECHARGE_THRESH_SEL_BIT		BIT(1)
> +#define SMB2_FG_UPDATE_CFG_2_SEL			0x7D
> +#define SMB2_SOC_LT_CHG_RECHARGE_THRESH_SEL_BIT		BIT(2)
> +#define SMB2_VBT_LT_CHG_RECHARGE_THRESH_SEL_BIT		BIT(1)
> +
> +#define SMB5_CHARGE_RCHG_SOC_THRESHOLD_CFG_REG		0x7D
> +#define SMB5_CHARGE_RCHG_SOC_THRESHOLD_CFG_MASK		GENMASK(7, 0)
>  
>  #define OTG_CFG						0x153
>  #define OTG_EN_SRC_CFG_BIT				BIT(1)
>  
> @@ -73,36 +90,46 @@
>  #define CDP_CHARGER_BIT					BIT(2)
>  #define OCP_CHARGER_BIT					BIT(1)
>  #define SDP_CHARGER_BIT					BIT(0)
>  
> +#define USBIN_CMD_IL					0x340
> +#define USBIN_SUSPEND_BIT				BIT(0)
> +
>  #define CMD_APSD					0x341
>  #define APSD_RERUN_BIT					BIT(0)
>  
> +#define CMD_ICL_OVERRIDE				0x342
> +#define ICL_OVERRIDE_BIT				BIT(0)
> +
>  #define TYPE_C_CFG					0x358
> +#define APSD_START_ON_CC_BIT				BIT(7)
>  #define FACTORY_MODE_DETECTION_EN_BIT			BIT(5)
>  #define VCONN_OC_CFG_BIT				BIT(1)
>  
>  #define USBIN_OPTIONS_1_CFG				0x362
> +#define AUTO_SRC_DETECT_BIT				BIT(3)
>  #define HVDCP_EN_BIT					BIT(2)
>  
> +#define USBIN_LOAD_CFG					0x65
> +#define ICL_OVERRIDE_AFTER_APSD_BIT			BIT(4)
> +
>  #define USBIN_ICL_OPTIONS				0x366
>  #define USB51_MODE_BIT					BIT(1)
>  #define USBIN_MODE_CHG_BIT				BIT(0)
>  
>  /* PMI8998 only */
>  #define TYPE_C_INTRPT_ENB_SOFTWARE_CTRL			0x368
> -#define VCONN_EN_SRC_BIT				BIT(4)
> +#define SMB2_VCONN_EN_SRC_BIT				BIT(4)
>  #define VCONN_EN_VALUE_BIT				BIT(3)
>  #define TYPEC_POWER_ROLE_CMD_MASK			GENMASK(2, 0)
> -#define UFP_EN_CMD_BIT					BIT(2)
> -#define DFP_EN_CMD_BIT					BIT(1)
> -#define TYPEC_DISABLE_CMD_BIT				BIT(0)
> +#define SMB5_EN_SNK_ONLY_BIT				BIT(1)
>  
>  #define USBIN_CURRENT_LIMIT_CFG				0x370
>  
>  #define USBIN_AICL_OPTIONS_CFG				0x380
>  #define SUSPEND_ON_COLLAPSE_USBIN_BIT			BIT(7)
>  #define USBIN_AICL_START_AT_MAX_BIT			BIT(5)
> +#define USBIN_AICL_PERIODIC_RERUN_EN_BIT		BIT(4)
>  #define USBIN_AICL_ADC_EN_BIT				BIT(3)
>  #define USBIN_AICL_EN_BIT				BIT(2)
>  #define USBIN_HV_COLLAPSE_RESPONSE_BIT			BIT(1)
>  #define USBIN_LV_COLLAPSE_RESPONSE_BIT			BIT(0)
> @@ -113,15 +140,41 @@
>  
>  #define USBIN_CONT_AICL_THRESHOLD_CFG			0x384
>  #define USBIN_CONT_AICL_THRESHOLD_CFG_MASK		GENMASK(5, 0)
>  
> -#define ICL_STATUS					0x607
> +#define ICL_STATUS(smb)					(SMB_REG_OFFSET(smb) + 0x07)
>  #define INPUT_CURRENT_LIMIT_MASK			GENMASK(7, 0)
>  
> -#define POWER_PATH_STATUS				0x60B
> +#define POWER_PATH_STATUS(smb)				(SMB_REG_OFFSET(smb) + 0x0B)
>  #define P_PATH_USE_USBIN_BIT				BIT(4)
>  #define P_PATH_VALID_INPUT_POWER_SOURCE_STS_BIT		BIT(0)
>  
> +/* 0x5xx region is PM8150b only Type-C registers */
> +
> +/* Bits 2:0 match PMI8998 TYPE_C_INTRPT_ENB_SOFTWARE_CTRL */
> +#define SMB5_TYPE_C_MODE_CFG				0x544
> +#define SMB5_EN_TRY_SNK_BIT				BIT(4)
> +#define SMB5_EN_SNK_ONLY_BIT				BIT(1)
> +
> +#define SMB5_TYPEC_TYPE_C_VCONN_CONTROL			0x546
> +#define SMB5_VCONN_EN_ORIENTATION_BIT			BIT(2)
> +#define SMB5_VCONN_EN_VALUE_BIT				BIT(1)
> +#define SMB5_VCONN_EN_SRC_BIT				BIT(0)
> +
> +
> +#define SMB5_TYPE_C_DEBUG_ACCESS_SINK			0x54a
> +#define SMB5_TYPEC_DEBUG_ACCESS_SINK_MASK		GENMASK(4, 0)
> +
> +#define SMB5_DEBUG_ACCESS_SRC_CFG			0x54C
> +#define SMB5_EN_UNORIENTED_DEBUG_ACCESS_SRC_BIT	BIT(0)
> +
> +#define SMB5_TYPE_C_EXIT_STATE_CFG			0x550
> +#define SMB5_BYPASS_VSAFE0V_DURING_ROLE_SWAP_BIT	BIT(3)
> +#define SMB5_SEL_SRC_UPPER_REF_BIT			BIT(2)
> +#define SMB5_EXIT_SNK_BASED_ON_CC_BIT			BIT(0)
> +
> +/* Common */
> +
>  #define BARK_BITE_WDOG_PET				0x643
>  #define BARK_BITE_WDOG_PET_BIT				BIT(0)
>  
>  #define WD_CFG						0x651
> @@ -184,8 +237,9 @@ struct smb_chip {
>  	const char *name;
>  	unsigned int base;
>  	struct regmap *regmap;
>  	struct power_supply_battery_info *batt_info;
> +	enum smb_generation gen;
>  
>  	struct delayed_work status_change_work;
>  	int cable_irq;
>  	bool wakeup_enabled;
> @@ -195,8 +249,15 @@ struct smb_chip {
>  
>  	struct power_supply *chg_psy;
>  };
>  
> +struct smb_match_data {
> +	const char *name;
> +	enum smb_generation gen;
> +	size_t init_seq_len;
> +	const struct smb_init_register __counted_by(init_seq_len) *init_seq;
> +};
> +
>  static enum power_supply_property smb_properties[] = {
>  	POWER_SUPPLY_PROP_MANUFACTURER,
>  	POWER_SUPPLY_PROP_MODEL_NAME,
>  	POWER_SUPPLY_PROP_CURRENT_MAX,
> @@ -212,9 +273,9 @@ static int smb_get_prop_usb_online(struct smb_chip *chip, int *val)
>  {
>  	unsigned int stat;
>  	int rc;
>  
> -	rc = regmap_read(chip->regmap, chip->base + POWER_PATH_STATUS, &stat);
> +	rc = regmap_read(chip->regmap, chip->base + POWER_PATH_STATUS(chip), &stat);
>  	if (rc < 0) {
>  		dev_err(chip->dev, "Couldn't read power path status: %d\n", rc);
>  		return rc;
>  	}
> @@ -267,11 +328,37 @@ static int smb_apsd_get_charger_type(struct smb_chip *chip, int *val)
>  
>  	return 0;
>  }
>  
> +/* Return 1 when in overvoltage state, else 0 or -errno */
> +static int smbx_ov_status(struct smb_chip *chip)
> +{
> +	u16 reg;
> +	u8 mask;
> +	int rc;
> +	u32 val;
> +
> +	switch (chip->gen) {
> +	case SMB2:
> +		reg = BATTERY_CHARGER_STATUS_2;
> +		mask = SMB2_CHARGER_ERROR_STATUS_BAT_OV_BIT;
> +		break;
> +	case SMB5:
> +		reg = BATTERY_CHARGER_STATUS_7;
> +		mask = SMB5_CHARGER_ERROR_STATUS_BAT_OV_BIT;
> +		break;
> +	}
> +
> +	rc = regmap_read(chip->regmap, chip->base + reg, &val);
> +	if (rc)
> +		return rc;
> +
> +	return !!(reg & mask);
> +}
> +
>  static int smb_get_prop_status(struct smb_chip *chip, int *val)
>  {
> -	unsigned char stat[2];
> +	u32 stat;
>  	int usb_online = 0;
>  	int rc;
>  
>  	rc = smb_get_prop_usb_online(chip, &usb_online);
> @@ -279,24 +366,29 @@ static int smb_get_prop_status(struct smb_chip *chip, int *val)
>  		*val = POWER_SUPPLY_STATUS_DISCHARGING;
>  		return rc;
>  	}
>  
> -	rc = regmap_bulk_read(chip->regmap,
> -			      chip->base + BATTERY_CHARGER_STATUS_1, &stat, 2);
> +	rc = regmap_read(chip->regmap,
> +			      chip->base + BATTERY_CHARGER_STATUS_1, &stat);
>  	if (rc < 0) {
>  		dev_err(chip->dev, "Failed to read charging status ret=%d\n",
>  			rc);
>  		return rc;
>  	}
>  
> -	if (stat[1] & CHARGER_ERROR_STATUS_BAT_OV_BIT) {
> +	rc = smbx_ov_status(chip);
> +	if (rc < 0)
> +		return rc;
> +
> +	/* In overvoltage state */
> +	if (rc == 1) {
>  		*val = POWER_SUPPLY_STATUS_NOT_CHARGING;
>  		return 0;
>  	}
>  
> -	stat[0] = stat[0] & BATTERY_CHARGER_STATUS_MASK;
> +	stat = stat & BATTERY_CHARGER_STATUS_MASK;
>  
> -	switch (stat[0]) {
> +	switch (stat) {
>  	case TRICKLE_CHARGE:
>  	case PRE_CHARGE:
>  	case FAST_CHARGE:
>  	case FULLON_CHARGE:
> @@ -318,9 +410,9 @@ static int smb_get_prop_status(struct smb_chip *chip, int *val)
>  
>  static inline int smb_get_current_limit(struct smb_chip *chip,
>  					 unsigned int *val)
>  {
> -	int rc = regmap_read(chip->regmap, chip->base + ICL_STATUS, val);
> +	int rc = regmap_read(chip->regmap, chip->base + ICL_STATUS(chip), val);
>  
>  	if (rc >= 0)
>  		*val *= CURRENT_SCALE_FACTOR;
>  	return rc;
> @@ -413,9 +505,46 @@ static int smb_get_iio_chan(struct smb_chip *chip, struct iio_channel *chan,
>  
>  	return iio_read_channel_processed(chan, val);
>  }
>  
> -static int smb_get_prop_health(struct smb_chip *chip, int *val)
> +static int smb5_get_prop_health(struct smb_chip *chip, int *val)
> +{
> +	int rc;
> +	unsigned int stat;
> +
> +	rc = smbx_ov_status(chip);
> +
> +	/* Treat any error as if we are in the overvoltage state */
> +	if (rc < 0)
> +		dev_err(chip->dev, "Couldn't determine overvoltage status!");
> +	if (rc) {
> +		dev_err(chip->dev, "battery over-voltage");
> +		*val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
> +		return 0;
> +	}
> +
> +	rc = regmap_read(chip->regmap, chip->base + BATTERY_CHARGER_STATUS_7,
> +			 &stat);
> +	if (rc < 0) {
> +		dev_err(chip->dev, "Couldn't read charger status 7 rc=%d\n", rc);
> +		return rc;
> +	}
> +
> +	if (stat & SMB5_BAT_TEMP_STATUS_TOO_COLD_BIT)
> +		*val = POWER_SUPPLY_HEALTH_COLD;
> +	else if (stat & SMB5_BAT_TEMP_STATUS_TOO_HOT_BIT)
> +		*val = POWER_SUPPLY_HEALTH_OVERHEAT;
> +	else if (stat & SMB5_BAT_TEMP_STATUS_COLD_SOFT_BIT)
> +		*val = POWER_SUPPLY_HEALTH_COOL;
> +	else if (stat & SMB5_BAT_TEMP_STATUS_HOT_SOFT_BIT)
> +		*val = POWER_SUPPLY_HEALTH_WARM;
> +	else
> +		*val = POWER_SUPPLY_HEALTH_GOOD;
> +
> +	return 0;
> +}
> +
> +static int smb2_get_prop_health(struct smb_chip *chip, int *val)
>  {
>  	int rc;
>  	unsigned int stat;
>  
> @@ -426,34 +555,45 @@ static int smb_get_prop_health(struct smb_chip *chip, int *val)
>  		return rc;
>  	}
>  
>  	switch (stat) {
> -	case CHARGER_ERROR_STATUS_BAT_OV_BIT:
> +	case SMB2_CHARGER_ERROR_STATUS_BAT_OV_BIT:
>  		*val = POWER_SUPPLY_HEALTH_OVERVOLTAGE;
>  		return 0;
> -	case BAT_TEMP_STATUS_TOO_COLD_BIT:
> +	case SMB2_BAT_TEMP_STATUS_TOO_COLD_BIT:
>  		*val = POWER_SUPPLY_HEALTH_COLD;
>  		return 0;
> -	case BAT_TEMP_STATUS_TOO_HOT_BIT:
> +	case SMB2_BAT_TEMP_STATUS_TOO_HOT_BIT:
>  		*val = POWER_SUPPLY_HEALTH_OVERHEAT;
>  		return 0;
> -	case BAT_TEMP_STATUS_COLD_SOFT_LIMIT_BIT:
> +	case SMB2_BAT_TEMP_STATUS_COLD_SOFT_LIMIT_BIT:
>  		*val = POWER_SUPPLY_HEALTH_COOL;
>  		return 0;
> -	case BAT_TEMP_STATUS_HOT_SOFT_LIMIT_BIT:
> +	case SMB2_BAT_TEMP_STATUS_HOT_SOFT_LIMIT_BIT:
>  		*val = POWER_SUPPLY_HEALTH_WARM;
>  		return 0;
>  	default:
>  		*val = POWER_SUPPLY_HEALTH_GOOD;
>  		return 0;
>  	}
>  }
>  
> +static int smb_get_prop_health(struct smb_chip *chip, int *val)
> +{
> +	switch (chip->gen) {
> +	case SMB2:
> +		return smb2_get_prop_health(chip, val);
> +	case SMB5:
> +		return smb5_get_prop_health(chip, val);
> +	}
> +}
> +
>  static int smb_get_property(struct power_supply *psy,
>  			     enum power_supply_property psp,
>  			     union power_supply_propval *val)
>  {
>  	struct smb_chip *chip = power_supply_get_drvdata(psy);
> +	int ret;
>  
>  	switch (psp) {
>  	case POWER_SUPPLY_PROP_MANUFACTURER:
>  		val->strval = "Qualcomm";
> @@ -466,10 +606,15 @@ static int smb_get_property(struct power_supply *psy,
>  	case POWER_SUPPLY_PROP_CURRENT_NOW:
>  		return smb_get_iio_chan(chip, chip->usb_in_i_chan,
>  					 &val->intval);
>  	case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> -		return smb_get_iio_chan(chip, chip->usb_in_v_chan,
> +		ret = smb_get_iio_chan(chip, chip->usb_in_v_chan,
>  					 &val->intval);
> +		if (!ret) {
> +			if (chip->gen == SMB5)
> +				val->intval *= 16;
> +		}
> +		return ret;
>  	case POWER_SUPPLY_PROP_ONLINE:
>  		return smb_get_prop_usb_online(chip, &val->intval);
>  	case POWER_SUPPLY_PROP_STATUS:
>  		return smb_get_prop_status(chip, &val->intval);
> @@ -515,14 +660,10 @@ static int smb_property_is_writable(struct power_supply *psy,
>  
>  static irqreturn_t smb_handle_batt_overvoltage(int irq, void *data)
>  {
>  	struct smb_chip *chip = data;
> -	unsigned int status;
>  
> -	regmap_read(chip->regmap, chip->base + BATTERY_CHARGER_STATUS_2,
> -		    &status);
> -
> -	if (status & CHARGER_ERROR_STATUS_BAT_OV_BIT) {
> +	if (smbx_ov_status(chip) == 1) {
>  		/* The hardware stops charging automatically */
>  		dev_err(chip->dev, "battery overvoltage detected\n");
>  		power_supply_changed(chip->chg_psy);
>  	}
> @@ -566,9 +707,9 @@ static irqreturn_t smb_handle_wdog_bark(int irq, void *data)
>  	return IRQ_HANDLED;
>  }
>  
>  static const struct power_supply_desc smb_psy_desc = {
> -	.name = "pmi8998_charger",
> +	.name = "SMB2_charger",
>  	.type = POWER_SUPPLY_TYPE_USB,
>  	.usb_types = BIT(POWER_SUPPLY_USB_TYPE_SDP) |
>  		     BIT(POWER_SUPPLY_USB_TYPE_CDP) |
>  		     BIT(POWER_SUPPLY_USB_TYPE_DCP) |
> @@ -580,18 +721,100 @@ static const struct power_supply_desc smb_psy_desc = {
>  	.property_is_writeable = smb_property_is_writable,
>  };
>  
>  /* Init sequence derived from vendor downstream driver */
> -static const struct smb_init_register smb_init_seq[] = {
> -	{ .addr = AICL_RERUN_TIME_CFG, .mask = AICL_RERUN_TIME_MASK, .val = 0 },
> +static const struct smb_init_register smb5_init_seq[] = {
> +	{ .addr = USBIN_CMD_IL, .mask = USBIN_SUSPEND_BIT, .val = 0 },
> +	/*
> +	 * By default configure us as an upstream facing port
> +	 * FIXME: This will be handled by the type-c driver
> +	 */

All of this needs to be reworked to cooperate with the type-c driver.
Otherwise it might try to reconfigure the Type-C mode _after_ the TCPM
has negotiated some configuration. So, it can't go in in this way.

> +	{ .addr = SMB5_TYPE_C_MODE_CFG,
> +	  .mask = SMB5_EN_TRY_SNK_BIT | SMB5_EN_SNK_ONLY_BIT,
> +	  .val = SMB5_EN_TRY_SNK_BIT },
> 

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists