diff mbox series

[next,5/5] power: rk8xx: properly print all supported PMICs name

Message ID 20240524-misc-tsd-v1-5-b7bde1344b9e@cherry.de
State Superseded
Delegated to: Kever Yang
Headers show
Series rockchip: display PMIC variant properly + misc fixes for Theobroma boards | expand

Commit Message

Quentin Schulz May 24, 2024, 11:42 a.m. UTC
From: Quentin Schulz <quentin.schulz@cherry.de>

The ID of the PMIC is stored in the 2 16b registers but the only part
that matters right now is the 3 MSB, which make the 3 digits (in hex) of
the part number.

Right now, only RK808 was properly displayed, with this all currently
supported PMICs should display the proper part number.

Tested on RK806 (RK3588 Jaguar), RK808 (RK3399 Puma) and RK809 (PX30
Ringneck).

Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
---
 drivers/power/pmic/rk8xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Dragan Simic May 25, 2024, 1:35 p.m. UTC | #1
Hello Quentin,

Thanks for fixing this issue!  Please see my comments below.

On 2024-05-24 13:42, Quentin Schulz wrote:
> From: Quentin Schulz <quentin.schulz@cherry.de>
> 
> The ID of the PMIC is stored in the 2 16b registers but the only part
> that matters right now is the 3 MSB, which make the 3 digits (in hex) 
> of
> the part number.
> 
> Right now, only RK808 was properly displayed, with this all currently
> supported PMICs should display the proper part number.
> 
> Tested on RK806 (RK3588 Jaguar), RK808 (RK3399 Puma) and RK809 (PX30
> Ringneck).
> 
> Signed-off-by: Quentin Schulz <quentin.schulz@cherry.de>
> ---
>  drivers/power/pmic/rk8xx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
> index 12ff26a0855..fe85fb13543 100644
> --- a/drivers/power/pmic/rk8xx.c
> +++ b/drivers/power/pmic/rk8xx.c
> @@ -277,10 +277,10 @@ static int rk8xx_probe(struct udevice *dev)
>  		return ret;
> 
>  	priv->variant = ((msb << 8) | lsb) & RK8XX_ID_MSK;
> -	show_variant = priv->variant;
> +	/* All currently supported PMICs store the variant in the 3 MSB */
> +	show_variant = priv->variant >> 4;

It would be better to use bitfield_shift() to produce the shift
value from RK8XX_ID_MSK, instead of using 4 as the hardcoded value.
Or even better, just use bitfield_extract_by_mask() directly.

>  	switch (priv->variant) {
>  	case RK808_ID:
> -		show_variant = 0x808;	/* RK808 hardware ID is 0 */
>  		break;
>  	case RK805_ID:
>  	case RK816_ID:

I'd suggest that show_variant is also used in the following line,
instead of priv-variant, for consistency with the printing of the
known PMIC variant:

315    printf("Unknown PMIC: RK%x!!\n", priv->variant);
diff mbox series

Patch

diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
index 12ff26a0855..fe85fb13543 100644
--- a/drivers/power/pmic/rk8xx.c
+++ b/drivers/power/pmic/rk8xx.c
@@ -277,10 +277,10 @@  static int rk8xx_probe(struct udevice *dev)
 		return ret;
 
 	priv->variant = ((msb << 8) | lsb) & RK8XX_ID_MSK;
-	show_variant = priv->variant;
+	/* All currently supported PMICs store the variant in the 3 MSB */
+	show_variant = priv->variant >> 4;
 	switch (priv->variant) {
 	case RK808_ID:
-		show_variant = 0x808;	/* RK808 hardware ID is 0 */
 		break;
 	case RK805_ID:
 	case RK816_ID: