[PATCH next v3 5/6] power: rk8xx: properly print all supported PMICs name

[Dragan Simic]

On 2024-06-17 18:54, Quentin Schulz wrote:
> On 6/17/24 4:58 PM, Dragan Simic wrote:
>> On 2024-06-17 16:10, Quentin Schulz wrote:
>>> On 6/6/24 10:45 AM, Quentin Schulz wrote:
>>>> From: Quentin Schulz <quentin.schulz at 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.
>>>> 
>>>> Additionally, when the PMIC variant is not found, print that value
>>>> instead of the masked unshifted value as all PMICs we support for 
>>>> now
>>>> have their LSB ignored to represent the actual part number.
>>>> 
>>>> Tested on RK806 (RK3588 Jaguar), RK808 (RK3399 Puma) and RK809 (PX30
>>>> Ringneck).
>>>> 
>>>> Reviewed-by: Kever Yang <kever.yang at rock-chips.com>
>>>> Signed-off-by: Quentin Schulz <quentin.schulz at cherry.de>
>>>> ---
>>>>   drivers/power/pmic/rk8xx.c | 6 +++---
>>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>> 
>>>> diff --git a/drivers/power/pmic/rk8xx.c b/drivers/power/pmic/rk8xx.c
>>>> index 12ff26a0855..617bb511e4e 100644
>>>> --- a/drivers/power/pmic/rk8xx.c
>>>> +++ b/drivers/power/pmic/rk8xx.c
>>>> @@ -6,6 +6,7 @@
>>>>     #include <dm.h>
>>>>   #include <dm/lists.h>
>>>> +#include <bitfield.h>
>>>>   #include <errno.h>
>>>>   #include <log.h>
>>>>   #include <linux/bitfield.h>
>>>> @@ -277,10 +278,9 @@ static int rk8xx_probe(struct udevice *dev)
>>>>           return ret;
>>>>         priv->variant = ((msb << 8) | lsb) & RK8XX_ID_MSK;
>>>> -    show_variant = priv->variant;
>>>> +    show_variant = bitfield_extract_by_mask(priv->variant, 
>>>> RK8XX_ID_MSK);
>>>>       switch (priv->variant) {
>>>>       case RK808_ID:
>>>> -        show_variant = 0x808;    /* RK808 hardware ID is 0 */
>>> 
>>> This line removal is actually incorrect, I should have left this in 
>>> as
>>> we cannot use the same logic as other PMICs for RK808 as it returns 
>>> 0,
>>> so 0 masked/shifted is still zero.
>> 
>> Thanks for catching this!  Moreover, I think we should skip reading
>> the msb and lsb values entirely for the RK808, because its datasheet
>> lists the default ID_MSB (0x17) and ID_LSB (0x18) registers as 
>> reserved,
>> and provides no information about gathering the chip variant.
> 
> We've been reading those registers on RK808 in our production lines
> for probably what's half a decade now, I think it's probably safe to
> use :)

True, it's obviously safe, but not reading those reserved RK808
registers would be more about accuracy and following the datasheets
as precisely as possible. :)