Re: [PATCH v5 05/10] power: supply: rt5033_charger: Add RT5033 charger device driver

[Christophe JAILLET <christophe.jaillet@wanadoo.fr>]

Le 14/05/2023 à 14:31, Jakob Hauser a écrit :
> This patch adds device driver of Richtek RT5033 PMIC. The driver supports
> switching charger. rt5033 charger provides three charging modes. The charging
> modes are pre-charge mode, fast charge mode and constant voltage mode. They
> vary in charge rate, the charge parameters can be controlled by i2c interface.
> 
> Cc: Beomho Seo <beomho.seo-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Tested-by: Raymond Hackley <raymondhackley-g/b1ySJe57IN+BqQ9rBEUg@public.gmane.org>
> Signed-off-by: Jakob Hauser <jahau-ur4TIblo6goN+BqQ9rBEUg@public.gmane.org>
> Reviewed-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>   drivers/power/supply/Kconfig          |   8 +
>   drivers/power/supply/Makefile         |   1 +
>   drivers/power/supply/rt5033_charger.c | 472 ++++++++++++++++++++++++++
>   include/linux/mfd/rt5033.h            |  16 -
>   4 files changed, 481 insertions(+), 16 deletions(-)
>   create mode 100644 drivers/power/supply/rt5033_charger.c
> 

[...]

> +static int rt5033_charger_probe(struct platform_device *pdev)
> +{
> +	struct rt5033_charger *charger;
> +	struct power_supply_config psy_cfg = {};
> +	int ret;
> +
> +	charger = devm_kzalloc(&pdev->dev, sizeof(*charger), GFP_KERNEL);
> +	if (!charger)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, charger);
> +	charger->dev = &pdev->dev;
> +	charger->regmap = dev_get_regmap(pdev->dev.parent, NULL);
> +
> +	psy_cfg.of_node = pdev->dev.of_node;
> +	psy_cfg.drv_data = charger;
> +
> +	charger->psy = devm_power_supply_register(&pdev->dev,
> +						  &rt5033_charger_desc,
> +						  &psy_cfg);
> +	if (IS_ERR(charger->psy))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(charger->psy),
> +				     "Failed to register power supply\n");
> +
> +	charger->chg = rt5033_charger_dt_init(charger);
> +	if (IS_ERR_OR_NULL(charger->chg))

Hi,

Nit: charger->chg can't be NULL.

> +		return -ENODEV;

Why bother returning specific error code in rt5033_charger_dt_init() if 
they are eaten here.

return PTR_ERR(charger->chg)?


CJ

> +
> +	ret = rt5033_charger_reg_init(charger);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}

[...]