Re: [PATCH v4 2/2] power: supply: mt6360_charger: add MT6360 charger support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 

On Mon, 2021-01-18 at 20:41 +0800, Gene Chen wrote:
> From: Gene Chen <gene_chen@xxxxxxxxxxx>
> 
> Add basic support for the battery charger for MT6360 PMIC
> 
> Signed-off-by: Gene Chen <gene_chen@xxxxxxxxxxx>
> ---
>  drivers/power/supply/Kconfig          |  10 +
>  drivers/power/supply/Makefile         |   1 +
>  drivers/power/supply/mt6360_charger.c | 914
> ++++++++++++++++++++++++++++++++++
>  3 files changed, 925 insertions(+)
>  create mode 100644 drivers/power/supply/mt6360_charger.c
> 

Thanks for the contribution :)

Few comments which I am not demanding to be 'fixed' - but which might
be good to be checked. Eg, please consider my comments as 'nit's.

...

> +static unsigned int mt6360_map_reg_sel(u32 data, u32 min, u32 max,
> u32 step)
> +{
> +	u32 target = 0, max_sel;
> +
> +	if (data >= min) {
> +		target = (data - min) / step;
> +		max_sel = (max - min) / step;
> +		if (target > max_sel)
> +			target = max_sel;
> +	}
> +	return target;
> +}

lib/linear_ranges.c might already implement this ...

> +
> +static u32 mt6360_map_real_val(u32 sel, u32 min, u32 max, u32 step)
> +{
> +	u32 target = 0;
> +
> +	target = min + (sel * step);
> +	if (target > max)
> +		target = max;
> +	return target;
> +}

...and this.

> +static int mt6360_charger_get_ichg(struct mt6360_chg_info *mci,
> +				   union power_supply_propval *val)
> +{
> +	int ret;
> +	unsigned int regval;
> +
> +	ret = regmap_read(mci->regmap, MT6360_PMU_CHG_CTRL7, &regval);
> +	if (ret < 0)
> +		return ret;
> +	regval = (regval & MT6360_ICHG_MASK) >> MT6360_ICHG_SHFT;
> +	val->intval = mt6360_map_real_val(regval,
> +					  MT6360_ICHG_MIN,
> +					  MT6360_ICHG_MAX,
> +					  MT6360_ICHG_STEP);

linear_ranges?

> +	return 0;
> +}
> +
> +static int mt6360_charger_get_max_ichg(struct mt6360_chg_info *mci,
> +				       union power_supply_propval *val)
> +{
> +	val->intval = MT6360_ICHG_MAX;
> +	return 0;
> +}
> +
> +static int mt6360_charger_get_cv(struct mt6360_chg_info *mci,
> +				 union power_supply_propval *val)
> +{
> +	int ret;
> +	unsigned int regval;
> +
> +	ret = regmap_read(mci->regmap, MT6360_PMU_CHG_CTRL4, &regval);
> +	if (ret < 0)
> +		return ret;
> +	regval = (regval & MT6360_VOREG_MASK) >> MT6360_VOREG_SHFT;
> +	val->intval = mt6360_map_real_val(regval,
> +					  MT6360_VOREG_MIN,
> +					  MT6360_VOREG_MAX,
> +					  MT6360_VOREG_STEP);

linear_ranges?

> +	return 0;
> +}
> +
> +static int mt6360_charger_get_max_cv(struct mt6360_chg_info *mci,
> +				     union power_supply_propval *val)
> +{
> +	val->intval = MT6360_VOREG_MAX;
> +	return 0;
> +}
> +
> +static int mt6360_charger_get_aicr(struct mt6360_chg_info *mci,
> +				   union power_supply_propval *val)
> +{
> +	int ret;
> +	unsigned int regval;
> +
> +	ret = regmap_read(mci->regmap, MT6360_PMU_CHG_CTRL3, &regval);
> +	if (ret < 0)
> +		return ret;
> +	regval = (regval & MT6360_IAICR_MASK) >> MT6360_IAICR_SHFT;
> +	val->intval = mt6360_map_real_val(regval,
> +					  MT6360_AICR_MIN,
> +					  MT6360_AICR_MAX,
> +					  MT6360_AICR_STEP);

linear_ranges?

> +	return 0;
> +}
> +
> +static int mt6360_charger_get_mivr(struct mt6360_chg_info *mci,
> +				   union power_supply_propval *val)
> +{
> +	int ret;
> +	unsigned int regval;
> +
> +	ret = regmap_read(mci->regmap, MT6360_PMU_CHG_CTRL6, &regval);
> +	if (ret < 0)
> +		return ret;
> +	regval = (regval & MT6360_VMIVR_MASK) >> MT6360_VMIVR_SHFT;
> +	val->intval = mt6360_map_real_val(regval,
> +					  MT6360_VMIVR_MIN,
> +					  MT6360_VMIVR_MAX,
> +					  MT6360_VMIVR_STEP);

linear_ranges?

> +	return 0;
> +}
> +
> +static int mt6360_charger_get_iprechg(struct mt6360_chg_info *mci,
> +				      union power_supply_propval *val)
> +{
> +	int ret;
> +	unsigned int regval;
> +
> +	ret = regmap_read(mci->regmap, MT6360_PMU_CHG_CTRL8, &regval);
> +	if (ret < 0)
> +		return ret;
> +	regval = (regval & MT6360_IPREC_MASK) >> MT6360_IPREC_SHFT;
> +	val->intval = mt6360_map_real_val(regval,
> +					  MT6360_IPREC_MIN,
> +					  MT6360_IPREC_MAX,
> +					  MT6360_IPREC_STEP);

linear_ranges?

> +	return 0;
> +}
> +
> +static int mt6360_charger_get_ieoc(struct mt6360_chg_info *mci,
> +				   union power_supply_propval *val)
> +{
> +	int ret;
> +	unsigned int regval;
> +
> +	ret = regmap_read(mci->regmap, MT6360_PMU_CHG_CTRL9, &regval);
> +	if (ret < 0)
> +		return ret;
> +	regval = (regval & MT6360_IEOC_MASK) >> MT6360_IEOC_SHFT;
> +	val->intval = mt6360_map_real_val(regval,
> +					  MT6360_IEOC_MIN,
> +					  MT6360_IEOC_MAX,
> +					  MT6360_IEOC_STEP);

linear_ranges?

> +	return 0;
> +}
> +
> +static int mt6360_charger_set_online(struct mt6360_chg_info *mci,
> +				     const union power_supply_propval
> *val)
> +{
> +	u8 force_sleep = val->intval ? 0 : 1;
> +
> +	return regmap_update_bits(mci->regmap,
> +				  MT6360_PMU_CHG_CTRL1,
> +				  MT6360_FSLP_MASK,
> +				  force_sleep << MT6360_FSLP_SHFT);
> +}
> +
> +static int mt6360_charger_set_ichg(struct mt6360_chg_info *mci,
> +				   const union power_supply_propval
> *val)
> +{
> +	u8 sel;
> +
> +	sel = mt6360_map_reg_sel(val->intval,
> +				 MT6360_ICHG_MIN,
> +				 MT6360_ICHG_MAX,
> +				 MT6360_ICHG_STEP);

linear_ranges?

> +	return regmap_update_bits(mci->regmap,
> +				  MT6360_PMU_CHG_CTRL7,
> +				  MT6360_ICHG_MASK,
> +				  sel << MT6360_ICHG_SHFT);
> +}
> +
> +static int mt6360_charger_set_cv(struct mt6360_chg_info *mci,
> +				 const union power_supply_propval *val)
> +{
> +	u8 sel;
> +
> +	sel = mt6360_map_reg_sel(val->intval,
> +				 MT6360_VOREG_MIN,
> +				 MT6360_VOREG_MAX,
> +				 MT6360_VOREG_STEP);

linear_ranges?

> +	return regmap_update_bits(mci->regmap,
> +				  MT6360_PMU_CHG_CTRL4,
> +				  MT6360_VOREG_MASK,
> +				  sel << MT6360_VOREG_SHFT);
> +}
> +
> +static int mt6360_charger_set_aicr(struct mt6360_chg_info *mci,
> +				   const union power_supply_propval
> *val)
> +{
> +	u8 sel;
> +
> +	sel = mt6360_map_reg_sel(val->intval,
> +				 MT6360_AICR_MIN,
> +				 MT6360_AICR_MAX,
> +				 MT6360_AICR_STEP);

linear_ranges?

> +	return regmap_update_bits(mci->regmap,
> +				  MT6360_PMU_CHG_CTRL3,
> +				  MT6360_IAICR_MASK,
> +				  sel << MT6360_IAICR_SHFT);
> +}
> +
> +static int mt6360_charger_set_mivr(struct mt6360_chg_info *mci,
> +				   const union power_supply_propval
> *val)
> +{
> +	u8 sel;
> +
> +	sel = mt6360_map_reg_sel(val->intval,
> +				 MT6360_VMIVR_MIN,
> +				 MT6360_VMIVR_MAX,
> +				 MT6360_VMIVR_STEP);

linear_ranges?

> +	return regmap_update_bits(mci->regmap,
> +				  MT6360_PMU_CHG_CTRL3,
> +				  MT6360_VMIVR_MASK,
> +				  sel << MT6360_VMIVR_SHFT);
> +}
> +
> +static int mt6360_charger_set_iprechg(struct mt6360_chg_info *mci,
> +				      const union power_supply_propval
> *val)
> +{
> +	u8 sel;
> +
> +	sel = mt6360_map_reg_sel(val->intval,
> +				 MT6360_IPREC_MIN,
> +				 MT6360_IPREC_MAX,
> +				 MT6360_IPREC_STEP);

linear_ranges?

> +	return regmap_update_bits(mci->regmap,
> +				  MT6360_PMU_CHG_CTRL8,
> +				  MT6360_IPREC_MASK,
> +				  sel << MT6360_IPREC_SHFT);
> +}
> +
> +static int mt6360_charger_set_ieoc(struct mt6360_chg_info *mci,
> +				   const union power_supply_propval
> *val)
> +{
> +	u8 sel;
> +
> +	sel = mt6360_map_reg_sel(val->intval,
> +				 MT6360_IEOC_MIN,
> +				 MT6360_IEOC_MAX,
> +				 MT6360_IEOC_STEP);

linear_ranges?

> +	return regmap_update_bits(mci->regmap,
> +				  MT6360_PMU_CHG_CTRL9,
> +				  MT6360_IEOC_MASK,
> +				  sel << MT6360_IEOC_SHFT);
> +}
> +
> +



> +static const struct regulator_ops mt6360_chg_otg_ops = {
> +	.list_voltage = regulator_list_voltage_linear,
> +	.enable = regulator_enable_regmap,
> +	.disable = regulator_disable_regmap,
> +	.is_enabled = regulator_is_enabled_regmap,
> +	.set_voltage_sel = regulator_set_voltage_sel_regmap,
> +	.get_voltage_sel = regulator_get_voltage_sel_regmap,
> +};
> +
> +static const struct regulator_desc mt6360_otg_rdesc = {
> +	.of_match = "usb-otg-vbus",
> +	.name = "usb-otg-vbus",
> +	.ops = &mt6360_chg_otg_ops,
> +	.owner = THIS_MODULE,
> +	.type = REGULATOR_VOLTAGE,
> +	.min_uV = 4425000,
> +	.uV_step = 25000,
> +	.n_voltages = 57,
> +	.vsel_reg = MT6360_PMU_CHG_CTRL5,
> +	.vsel_mask = MT6360_VOBST_MASK,
> +	.enable_reg = MT6360_PMU_CHG_CTRL1,
> +	.enable_mask = MT6360_OPA_MODE_MASK,
> +};

Any particular reason why these are here and not in a regulator driver?

...

> +static int mt6360_charger_probe(struct platform_device *pdev)
> +{
> +	struct mt6360_chg_info *mci;
> +	struct power_supply_config charger_cfg = {};
> +	struct regulator_config config = { };
> +	int ret;
> +
> +	mci = devm_kzalloc(&pdev->dev, sizeof(*mci), GFP_KERNEL);
> +	if (!mci)
> +		return -ENOMEM;
> +
> +	ret = mt6360_parse_dt(pdev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to parse
> dt\n");
> +
> +	mci->dev = &pdev->dev;
> +	mci->vinovp = 6500000;
> +	mutex_init(&mci->chgdet_lock);
> +	platform_set_drvdata(pdev, mci);
> +	INIT_WORK(&mci->chrdet_work, &mt6360_chrdet_work);
> +
> +	mci->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +	if (!mci->regmap)
> +		return dev_err_probe(&pdev->dev, -ENODEV, "Failed to
> get parent regmap\n");
> +
> +	ret = mt6360_apply_dt(pdev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to apply
> dt\n");
> +
> +	memcpy(&mci->psy_desc, &mt6360_charger_desc, sizeof(mci-
> >psy_desc));
> +	mci->psy_desc.name = dev_name(&pdev->dev);
> +	charger_cfg.drv_data = mci;
> +	charger_cfg.of_node = pdev->dev.of_node;
> +	mci->psy = devm_power_supply_register(&pdev->dev,
> +					      &mci->psy_desc,
> &charger_cfg);
> +	if (IS_ERR(mci->psy))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(mci->psy),
> +				     "Failed to register power supply
> dev\n");
> +
> +	ret = mt6360_chg_init_setting(mci);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to
> initial setting\n");
> +
> +	schedule_work(&mci->chrdet_work);

Is this work scheduled anywhere else? If not - why doing this in wq
context? If yes - does this wq need cancellation upon exit?

> +
> +	ret = mt6360_chg_irq_register(pdev);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Failed to
> register irqs\n");
> +
> +	config.dev = &pdev->dev;
> +	config.regmap = mci->regmap;
> +	mci->otg_rdev = devm_regulator_register(&pdev->dev,
> &mt6360_otg_rdesc,
> +						&config);
> +	if (IS_ERR(mci->otg_rdev))
> +		return PTR_ERR(mci->otg_rdev);
> +
> +	return 0;
> +}
> +

Best Regards
	Matti Vaittinen
[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]

  Powered by Linux