diff mbox series

[1/3] regulator: rk8xx: fix incorrect device used for _ldo_[sg]et_suspend_value

Message ID 20240605-pmic-rk8xx-v1-1-2349fdf68aa0@cherry.de
State Accepted
Commit 7bc5c3ea62471bca435a6365cd7c070653b3a794
Delegated to: Kever Yang
Headers show
Series rockchip: rk8xx: fix broken [np]ldo callbacks | expand

Commit Message

Quentin Schulz June 5, 2024, 9:33 a.m. UTC
From: Quentin Schulz <quentin.schulz@cherry.de>

_ldo_get_suspend_value and _ldo_set_suspend_value get passed the parent
of the regulator (so the pmic) as first argument, therefore this udevice
should be used for pmic_* callbacks instead of using the parent of the
pmic.

To avoid further confusion, let's rename the argument to pmic instead of
dev, highlighting which kind of device we expect as argument.

Fixes: f047e4ab9762 ("regulator: rk8xx: add indirection level for some ldo callbacks")
Reported-by: Simon Glass <sjg@chromium.org>
Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 drivers/power/regulator/rk8xx.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Kever Yang June 6, 2024, 6:45 a.m. UTC | #1
On 2024/6/5 17:33, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
>
> _ldo_get_suspend_value and _ldo_set_suspend_value get passed the parent
> of the regulator (so the pmic) as first argument, therefore this udevice
> should be used for pmic_* callbacks instead of using the parent of the
> pmic.
>
> To avoid further confusion, let's rename the argument to pmic instead of
> dev, highlighting which kind of device we expect as argument.
>
> Fixes: f047e4ab9762 ("regulator: rk8xx: add indirection level for some ldo callbacks")
> Reported-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   drivers/power/regulator/rk8xx.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c
> index 1bd4605d43a..cce3502f89c 100644
> --- a/drivers/power/regulator/rk8xx.c
> +++ b/drivers/power/regulator/rk8xx.c
> @@ -1216,7 +1216,7 @@ static int pldo_set_value(struct udevice *dev, int uvolt)
>   	return _ldo_set_value(dev, info, uvolt);
>   }
>   
> -static int _ldo_set_suspend_value(struct udevice *dev, const struct rk8xx_reg_info *info, int uvolt)
> +static int _ldo_set_suspend_value(struct udevice *pmic, const struct rk8xx_reg_info *info, int uvolt)
>   {
>   	int mask = info->vsel_mask;
>   	int val;
> @@ -1232,7 +1232,7 @@ static int _ldo_set_suspend_value(struct udevice *dev, const struct rk8xx_reg_in
>   	debug("%s: volt=%d, reg=0x%x, mask=0x%x, val=0x%x\n",
>   	      __func__, uvolt, info->vsel_sleep_reg, mask, val);
>   
> -	return pmic_clrsetbits(dev->parent, info->vsel_sleep_reg, mask, val);
> +	return pmic_clrsetbits(pmic, info->vsel_sleep_reg, mask, val);
>   }
>   
>   static int ldo_set_suspend_value(struct udevice *dev, int uvolt)
> @@ -1259,7 +1259,7 @@ static int pldo_set_suspend_value(struct udevice *dev, int uvolt)
>   	return _ldo_set_suspend_value(dev->parent, info, uvolt);
>   }
>   
> -static int _ldo_get_suspend_value(struct udevice *dev, const struct rk8xx_reg_info *info)
> +static int _ldo_get_suspend_value(struct udevice *pmic, const struct rk8xx_reg_info *info)
>   {
>   	int mask = info->vsel_mask;
>   	int val, ret;
> @@ -1267,7 +1267,7 @@ static int _ldo_get_suspend_value(struct udevice *dev, const struct rk8xx_reg_in
>   	if (info->vsel_sleep_reg == NA)
>   		return -ENOSYS;
>   
> -	ret = pmic_reg_read(dev->parent, info->vsel_sleep_reg);
> +	ret = pmic_reg_read(pmic, info->vsel_sleep_reg);
>   	if (ret < 0)
>   		return ret;
>   
>
Simon Glass June 6, 2024, 3:04 p.m. UTC | #2
On Wed, 5 Jun 2024 at 03:33, Quentin Schulz <foss+uboot@0leil.net> wrote:
>
> From: Quentin Schulz <quentin.schulz@cherry.de>
>
> _ldo_get_suspend_value and _ldo_set_suspend_value get passed the parent
> of the regulator (so the pmic) as first argument, therefore this udevice
> should be used for pmic_* callbacks instead of using the parent of the
> pmic.
>
> To avoid further confusion, let's rename the argument to pmic instead of
> dev, highlighting which kind of device we expect as argument.
>
> Fixes: f047e4ab9762 ("regulator: rk8xx: add indirection level for some ldo callbacks")
> Reported-by: Simon Glass <sjg@chromium.org>
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>  drivers/power/regulator/rk8xx.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>
Tested-by: Simon Glass <sjg@chromium.org>  # chromebook-bob
diff mbox series

Patch

diff --git a/drivers/power/regulator/rk8xx.c b/drivers/power/regulator/rk8xx.c
index 1bd4605d43a..cce3502f89c 100644
--- a/drivers/power/regulator/rk8xx.c
+++ b/drivers/power/regulator/rk8xx.c
@@ -1216,7 +1216,7 @@  static int pldo_set_value(struct udevice *dev, int uvolt)
 	return _ldo_set_value(dev, info, uvolt);
 }
 
-static int _ldo_set_suspend_value(struct udevice *dev, const struct rk8xx_reg_info *info, int uvolt)
+static int _ldo_set_suspend_value(struct udevice *pmic, const struct rk8xx_reg_info *info, int uvolt)
 {
 	int mask = info->vsel_mask;
 	int val;
@@ -1232,7 +1232,7 @@  static int _ldo_set_suspend_value(struct udevice *dev, const struct rk8xx_reg_in
 	debug("%s: volt=%d, reg=0x%x, mask=0x%x, val=0x%x\n",
 	      __func__, uvolt, info->vsel_sleep_reg, mask, val);
 
-	return pmic_clrsetbits(dev->parent, info->vsel_sleep_reg, mask, val);
+	return pmic_clrsetbits(pmic, info->vsel_sleep_reg, mask, val);
 }
 
 static int ldo_set_suspend_value(struct udevice *dev, int uvolt)
@@ -1259,7 +1259,7 @@  static int pldo_set_suspend_value(struct udevice *dev, int uvolt)
 	return _ldo_set_suspend_value(dev->parent, info, uvolt);
 }
 
-static int _ldo_get_suspend_value(struct udevice *dev, const struct rk8xx_reg_info *info)
+static int _ldo_get_suspend_value(struct udevice *pmic, const struct rk8xx_reg_info *info)
 {
 	int mask = info->vsel_mask;
 	int val, ret;
@@ -1267,7 +1267,7 @@  static int _ldo_get_suspend_value(struct udevice *dev, const struct rk8xx_reg_in
 	if (info->vsel_sleep_reg == NA)
 		return -ENOSYS;
 
-	ret = pmic_reg_read(dev->parent, info->vsel_sleep_reg);
+	ret = pmic_reg_read(pmic, info->vsel_sleep_reg);
 	if (ret < 0)
 		return ret;