Re: [PATCH v3 5/6] regulator: mt6359: Add regulator supply names

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Redundant memory allocation for `vbbck` regulator description due to hardcoded supply name mismatch.
--

commit 71212b8ad317d34fab479d1bb93b102697d370d4
Author: Chen-Yu Tsai <wenst@chromium.org>

regulator: mt6359: Add regulator supply names

This commit adds .supply_name field settings for each MT6359 regulator to
match the DT binding definitions. It also handles dynamic supply name
assignment for the vbbck regulator which is fed by vio18.

> diff --git a/drivers/regulator/mt6359-regulator.c b/drivers/regulator/mt6359-regulator.c
> index bcf9a476a34e1..fa97c3189df5d 100644
> --- a/drivers/regulator/mt6359-regulator.c
> +++ b/drivers/regulator/mt6359-regulator.c
[ ... ]
> @@ -641,11 +649,12 @@ static const struct mt6359_regulator_info mt6359_regulators[] = {
[ ... ]
> -	MT6359_LDO("ldo_vbbck", VBBCK, vbbck_voltages,
> +	/* vbbck is fed from vio18 internally. */
> +	MT6359_LDO("ldo_vbbck", VBBCK, "LDO_VIO18", vbbck_voltages,
>  		   MT6359_RG_LDO_VBBCK_EN_ADDR, MT6359_RG_LDO_VBBCK_EN_SHIFT,

Would setting the supply name here to "VIO18" instead of "LDO_VIO18" be
more accurate?

The MT6359_LDO macro sets the default name for the VIO18 regulator to
"VIO18" using the stringified #_name argument. Because this is initialized
to "LDO_VIO18", it seems to guarantee a mismatch during the probe check:

[ ... ]
> @@ -951,6 +959,7 @@ static int mt6359_regulator_probe(struct platform_device *pdev)
[ ... ]
> +	vio18_name = mt6359_info[MT6359_ID_VIO18].desc.name;
> +
>  	config.dev = mt6397->dev;
>  	config.regmap = mt6397->regmap;
>  	for (i = 0; i < MT6359_MAX_REGULATOR; i++, mt6359_info++) {
> +		const struct regulator_desc *desc = &mt6359_info->desc;
> +		struct regulator_desc *_desc;
> +
>  		/* drop const here, but all uses in the driver are const */
>  		config.driver_data = (void *)mt6359_info;
> -		rdev = devm_regulator_register(&pdev->dev, &mt6359_info->desc, &config);
> +
> +		/* Use vio18's actual name as supply_name for vbbck */
> +		if (i == MT6359_ID_VBBCK && strcmp(desc->supply_name, vio18_name) != 0) {
> +			_desc = devm_kzalloc(&pdev->dev, sizeof(*_desc), GFP_KERNEL);

Since desc->supply_name is "LDO_VIO18" and vio18_name defaults to "VIO18",
does this cause an unnecessary allocation of a new regulator_desc on every
boot, even when the device tree doesn't override the VIO18 name?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512085358.1693208-1-wenst@chromium.org?part=5