Message-ID: <20230514224709.ltefid7yvjtzz5nj@mercury.elektranox.org>
Date:   Mon, 15 May 2023 00:47:09 +0200
From:   Sebastian Reichel <sebastian.reichel@...labora.com>
To:     Jakob Hauser <jahau@...ketmail.com>
Cc:     Lee Jones <lee@...nel.org>, Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Beomho Seo <beomho.seo@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Stephan Gerhold <stephan@...hold.net>,
        Raymond Hackley <raymondhackley@...tonmail.com>,
        Pavel Machek <pavel@....cz>, Axel Lin <axel.lin@...ics.com>,
        ChiYuan Huang <cy_huang@...htek.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Henrik Grimler <henrik@...mler.se>, linux-pm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        phone-devel@...r.kernel.org, ~postmarketos/upstreaming@...ts.sr.ht
Subject: Re: [PATCH v5 06/10] power: supply: rt5033_charger: Add cable
 detection and USB OTG supply

Hi,

On Sun, May 14, 2023 at 02:31:26PM +0200, Jakob Hauser wrote:
> Implement cable detection by extcon and handle the driver according to the
> connector type.
> 
> There are basically three types of action: "set_charging", "set_otg" and
> "set_disconnect".
> 
> A forth helper function to "unset_otg" was added because this is used in both
> "set_charging" and "set_disconnect". In the first case it covers the rather
> rare event that someone changes from OTG to charging without disconnect. In
> the second case, when disconnecting, the values are set back to the ones from
> initialization to return into a defined state.
> 
> Additionally, there is "set_mivr". When connecting to e.g. a laptop/PC, the
> minimum input voltage regulation (MIVR) shall prevent a voltage drop if the
> cable or the supply is weak. The MIVR value is set to 4600MV, same as in the
> Android driver [1]. When disconnecting, MIVR is set back to DISABLED.
> 
> In the function rt5033_get_charger_state(): When in OTG mode, the chip
> reports status "charging". Change this to "discharging" because there is
> no charging going on in OTG mode [2].
> 
> [1] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L499
> [2] https://github.com/msm8916-mainline/linux-downstream/blob/GT-I9195I/drivers/battery/rt5033_charger.c#L686-L687
> 
> Tested-by: Raymond Hackley <raymondhackley@...tonmail.com>
> Signed-off-by: Jakob Hauser <jahau@...ketmail.com>
> ---

Acked-by: Sebastian Reichel <sebastian.reichel@...labora.com>

-- Sebastian

>  drivers/power/supply/rt5033_charger.c | 276 +++++++++++++++++++++++++-
>  1 file changed, 274 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/supply/rt5033_charger.c b/drivers/power/supply/rt5033_charger.c
> index 1aa346dd0679..7c7fd4cf0623 100644
> --- a/drivers/power/supply/rt5033_charger.c
> +++ b/drivers/power/supply/rt5033_charger.c
> @@ -6,7 +6,10 @@
>   * Author: Beomho Seo <beomho.seo@...sung.com>
>   */
>  
> +#include <linux/devm-helpers.h>
> +#include <linux/extcon.h>
>  #include <linux/module.h>
> +#include <linux/mutex.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/power_supply.h>
> @@ -26,6 +29,14 @@ struct rt5033_charger {
>  	struct regmap			*regmap;
>  	struct power_supply		*psy;
>  	struct rt5033_charger_data	*chg;
> +	struct extcon_dev		*edev;
> +	struct notifier_block		extcon_nb;
> +	struct work_struct		extcon_work;
> +	struct mutex			lock;
> +	bool online;
> +	bool otg;
> +	bool mivr_enabled;
> +	u8 cv_regval;
>  };
>  
>  static int rt5033_get_charger_state(struct rt5033_charger *charger)
> @@ -56,6 +67,10 @@ static int rt5033_get_charger_state(struct rt5033_charger *charger)
>  		state = POWER_SUPPLY_STATUS_UNKNOWN;
>  	}
>  
> +	/* For OTG mode, RT5033 would still report "charging" */
> +	if (charger->otg)
> +		state = POWER_SUPPLY_STATUS_DISCHARGING;
> +
>  	return state;
>  }
>  
> @@ -147,6 +162,9 @@ static inline int rt5033_init_const_charge(struct rt5033_charger *charger)
>  		return -EINVAL;
>  	}
>  
> +	/* Store that value for later usage */
> +	charger->cv_regval = reg_data;
> +
>  	/* Set end of charge current */
>  	if (chg->eoc_uamp < RT5033_CHARGER_EOC_MIN ||
>  	    chg->eoc_uamp > RT5033_CHARGER_EOC_MAX) {
> @@ -330,6 +348,152 @@ static int rt5033_charger_reg_init(struct rt5033_charger *charger)
>  	return 0;
>  }
>  
> +static int rt5033_charger_set_otg(struct rt5033_charger *charger)
> +{
> +	int ret;
> +
> +	mutex_lock(&charger->lock);
> +
> +	/* Set OTG boost v_out to 5 volts */
> +	ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL2,
> +			RT5033_CHGCTRL2_CV_MASK,
> +			0x37 << RT5033_CHGCTRL2_CV_SHIFT);
> +	if (ret) {
> +		dev_err(charger->dev, "Failed set OTG boost v_out\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Set operation mode to OTG */
> +	ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL1,
> +			RT5033_CHGCTRL1_MODE_MASK, RT5033_BOOST_MODE);
> +	if (ret) {
> +		dev_err(charger->dev, "Failed to update OTG mode.\n");
> +		return -EINVAL;
> +	}
> +
> +	/* In case someone switched from charging to OTG directly */
> +	if (charger->online)
> +		charger->online = false;
> +
> +	charger->otg = true;
> +
> +	mutex_unlock(&charger->lock);
> +
> +	return 0;
> +}
> +
> +static int rt5033_charger_unset_otg(struct rt5033_charger *charger)
> +{
> +	int ret;
> +	u8 data;
> +
> +	/* Restore constant voltage for charging */
> +	data = charger->cv_regval;
> +	ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL2,
> +			RT5033_CHGCTRL2_CV_MASK,
> +			data << RT5033_CHGCTRL2_CV_SHIFT);
> +	if (ret) {
> +		dev_err(charger->dev, "Failed to restore constant voltage\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Set operation mode to charging */
> +	ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL1,
> +			RT5033_CHGCTRL1_MODE_MASK, RT5033_CHARGER_MODE);
> +	if (ret) {
> +		dev_err(charger->dev, "Failed to update charger mode.\n");
> +		return -EINVAL;
> +	}
> +
> +	charger->otg = false;
> +
> +	return 0;
> +}
> +
> +static int rt5033_charger_set_charging(struct rt5033_charger *charger)
> +{
> +	int ret;
> +
> +	mutex_lock(&charger->lock);
> +
> +	/* In case someone switched from OTG to charging directly */
> +	if (charger->otg) {
> +		ret = rt5033_charger_unset_otg(charger);
> +		if (ret)
> +			return -EINVAL;
> +	}
> +
> +	charger->online = true;
> +
> +	mutex_unlock(&charger->lock);
> +
> +	return 0;
> +}
> +
> +static int rt5033_charger_set_mivr(struct rt5033_charger *charger)
> +{
> +	int ret;
> +
> +	mutex_lock(&charger->lock);
> +
> +	/*
> +	 * When connected via USB connector type SDP (Standard Downstream Port),
> +	 * the minimum input voltage regulation (MIVR) should be enabled. It
> +	 * prevents an input voltage drop due to insufficient current provided
> +	 * by the adapter or USB input. As a downside, it may reduces the
> +	 * charging current and thus slows the charging.
> +	 */
> +	ret = regmap_update_bits(charger->regmap, RT5033_REG_CHG_CTRL4,
> +			RT5033_CHGCTRL4_MIVR_MASK, RT5033_CHARGER_MIVR_4600MV);
> +	if (ret) {
> +		dev_err(charger->dev, "Failed to set MIVR level.\n");
> +		return -EINVAL;
> +	}
> +
> +	charger->mivr_enabled = true;
> +
> +	mutex_unlock(&charger->lock);
> +
> +	/* Beyond this, do the same steps like setting charging */
> +	rt5033_charger_set_charging(charger);
> +
> +	return 0;
> +}
> +
> +static int rt5033_charger_set_disconnect(struct rt5033_charger *charger)
> +{
> +	int ret;
> +
> +	mutex_lock(&charger->lock);
> +
> +	/* Disable MIVR if enabled */
> +	if (charger->mivr_enabled) {
> +		ret = regmap_update_bits(charger->regmap,
> +				RT5033_REG_CHG_CTRL4,
> +				RT5033_CHGCTRL4_MIVR_MASK,
> +				RT5033_CHARGER_MIVR_DISABLE);
> +		if (ret) {
> +			dev_err(charger->dev, "Failed to disable MIVR.\n");
> +			return -EINVAL;
> +		}
> +
> +		charger->mivr_enabled = false;
> +	}
> +
> +	if (charger->otg) {
> +		ret = rt5033_charger_unset_otg(charger);
> +		if (ret)
> +			return -EINVAL;
> +	}
> +
> +	if (charger->online)
> +		charger->online = false;
> +
> +	mutex_unlock(&charger->lock);
> +
> +	return 0;
> +}
> +
>  static enum power_supply_property rt5033_charger_props[] = {
>  	POWER_SUPPLY_PROP_STATUS,
>  	POWER_SUPPLY_PROP_CHARGE_TYPE,
> @@ -366,8 +530,7 @@ static int rt5033_charger_get_property(struct power_supply *psy,
>  		val->strval = RT5033_MANUFACTURER;
>  		break;
>  	case POWER_SUPPLY_PROP_ONLINE:
> -		val->intval = (rt5033_get_charger_state(charger) ==
> -				POWER_SUPPLY_STATUS_CHARGING);
> +		val->intval = charger->online;
>  		break;
>  	default:
>  		return -EINVAL;
> @@ -402,6 +565,86 @@ static struct rt5033_charger_data *rt5033_charger_dt_init(
>  	return chg;
>  }
>  
> +static void rt5033_charger_extcon_work(struct work_struct *work)
> +{
> +	struct rt5033_charger *charger =
> +		container_of(work, struct rt5033_charger, extcon_work);
> +	struct extcon_dev *edev = charger->edev;
> +	int connector, state;
> +	int ret;
> +
> +	for (connector = EXTCON_USB_HOST; connector <= EXTCON_CHG_USB_PD;
> +	     connector++) {
> +		state = extcon_get_state(edev, connector);
> +		if (state == 1)
> +			break;
> +	}
> +
> +	/*
> +	 * Adding a delay between extcon notification and extcon action. This
> +	 * makes extcon action execution more reliable. Without the delay the
> +	 * execution sometimes fails, possibly because the chip is busy or not
> +	 * ready.
> +	 */
> +	msleep(100);
> +
> +	switch (connector) {
> +	case EXTCON_CHG_USB_SDP:
> +		ret = rt5033_charger_set_mivr(charger);
> +		if (ret) {
> +			dev_err(charger->dev, "failed to set USB mode\n");
> +			break;
> +		}
> +		dev_info(charger->dev, "USB mode. connector type: %d\n",
> +			 connector);
> +		break;
> +	case EXTCON_CHG_USB_DCP:
> +	case EXTCON_CHG_USB_CDP:
> +	case EXTCON_CHG_USB_ACA:
> +	case EXTCON_CHG_USB_FAST:
> +	case EXTCON_CHG_USB_SLOW:
> +	case EXTCON_CHG_WPT:
> +	case EXTCON_CHG_USB_PD:
> +		ret = rt5033_charger_set_charging(charger);
> +		if (ret) {
> +			dev_err(charger->dev, "failed to set charging\n");
> +			break;
> +		}
> +		dev_info(charger->dev, "charging. connector type: %d\n",
> +			 connector);
> +		break;
> +	case EXTCON_USB_HOST:
> +		ret = rt5033_charger_set_otg(charger);
> +		if (ret) {
> +			dev_err(charger->dev, "failed to set OTG\n");
> +			break;
> +		}
> +		dev_info(charger->dev, "OTG enabled\n");
> +		break;
> +	default:
> +		ret = rt5033_charger_set_disconnect(charger);
> +		if (ret) {
> +			dev_err(charger->dev, "failed to set disconnect\n");
> +			break;
> +		}
> +		dev_info(charger->dev, "disconnected\n");
> +		break;
> +	}
> +
> +	power_supply_changed(charger->psy);
> +}
> +
> +static int rt5033_charger_extcon_notifier(struct notifier_block *nb,
> +					  unsigned long event, void *param)
> +{
> +	struct rt5033_charger *charger =
> +		container_of(nb, struct rt5033_charger, extcon_nb);
> +
> +	schedule_work(&charger->extcon_work);
> +
> +	return NOTIFY_OK;
> +}
> +
>  static const struct power_supply_desc rt5033_charger_desc = {
>  	.name = "rt5033-charger",
>  	.type = POWER_SUPPLY_TYPE_USB,
> @@ -414,6 +657,7 @@ static int rt5033_charger_probe(struct platform_device *pdev)
>  {
>  	struct rt5033_charger *charger;
>  	struct power_supply_config psy_cfg = {};
> +	struct device_node *np_conn, *np_edev;
>  	int ret;
>  
>  	charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> @@ -423,6 +667,7 @@ static int rt5033_charger_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, charger);
>  	charger->dev = &pdev->dev;
>  	charger->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	mutex_init(&charger->lock);
>  
>  	psy_cfg.of_node = pdev->dev.of_node;
>  	psy_cfg.drv_data = charger;
> @@ -442,6 +687,33 @@ static int rt5033_charger_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * Extcon support is not vital for the charger to work. If no extcon
> +	 * is available, just emit a warning and leave the probe function.
> +	 */
> +	np_conn = of_parse_phandle(pdev->dev.of_node, "richtek,usb-connector", 0);
> +	np_edev = of_get_parent(np_conn);
> +	charger->edev = extcon_find_edev_by_node(np_edev);
> +	if (IS_ERR(charger->edev)) {
> +		dev_warn(&pdev->dev, "no extcon device found in device-tree\n");
> +		goto out;
> +	}
> +
> +	ret = devm_work_autocancel(&pdev->dev, &charger->extcon_work,
> +				   rt5033_charger_extcon_work);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to initialize extcon work\n");
> +		return ret;
> +	}
> +
> +	charger->extcon_nb.notifier_call = rt5033_charger_extcon_notifier;
> +	ret = devm_extcon_register_notifier_all(&pdev->dev, charger->edev,
> +						&charger->extcon_nb);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to register extcon notifier\n");
> +		return ret;
> +	}
> +out:
>  	return 0;
>  }
>  
> -- 
> 2.39.2
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)

Powered by blists - more mailing lists