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

Christophe JAILLET
2023-05-14T14:32:02Z

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;
> +} …