| 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
| ||
|
Message-ID: <87d8b81a-2ab9-49f2-9319-99d74f84081e@linaro.org>
Date: Mon, 30 Jun 2025 01:06:24 +0200
From: Caleb Connolly <caleb.connolly@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
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 24/06/2025 03:06, Dmitry Baryshkov wrote:
> 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.
Hi Dmitry,
this is the smb2/pmi8998 init sequence, we don't have any type-c support
for this yet hence this. When we add role switching/type-c support it
will make sense to rework this but until then we need this.
Kind regards,
>
>> + { .addr = SMB5_TYPE_C_MODE_CFG,
>> + .mask = SMB5_EN_TRY_SNK_BIT | SMB5_EN_SNK_ONLY_BIT,
>> + .val = SMB5_EN_TRY_SNK_BIT },
>>
>
--
// Casey (she/her)
Powered by blists - more mailing lists