Re: [PATCH 2/2] spmi: spmi-pmic-arb: add support for PMIC arbiter v8.5

[Fenglin Wu <fenglin.wu@oss.qualcomm.com>]

On 4/2/2026 12:18 PM, Fenglin Wu wrote:
>
> On 4/1/2026 7:22 PM, Dmitry Baryshkov wrote:
>> On Wed, Apr 01, 2026 at 02:41:24AM -0700, Fenglin Wu wrote:
>>> PMIC arbiter v8.5 is an extension of PMIC arbiter v8 that updated
>>> the definition of the channel status register bit fields. Add support
>>> to handle this difference.
>>>
>>> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
>>> ---
>>>   drivers/spmi/spmi-pmic-arb.c | 69 
>>> ++++++++++++++++++++++++++++++++++++++------
>>>   1 file changed, 60 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/drivers/spmi/spmi-pmic-arb.c 
>>> b/drivers/spmi/spmi-pmic-arb.c
>>> index 69f8d456324a..deeaa39bb647 100644
>>> --- a/drivers/spmi/spmi-pmic-arb.c
>>> +++ b/drivers/spmi/spmi-pmic-arb.c
>>> @@ -28,6 +28,7 @@
>>>   #define PMIC_ARB_VERSION_V5_MIN        0x50000000
>>>   #define PMIC_ARB_VERSION_V7_MIN        0x70000000
>>>   #define PMIC_ARB_VERSION_V8_MIN        0x80000000
>>> +#define PMIC_ARB_VERSION_V8P5_MIN    0x80050000
>>>   #define PMIC_ARB_INT_EN            0x0004
>>>     #define PMIC_ARB_FEATURES        0x0004
>>> @@ -63,11 +64,34 @@
>>>   #define SPMI_OWNERSHIP_PERIPH2OWNER(X)    ((X) & 0x7)
>>>     /* Channel Status fields */
>>> -enum pmic_arb_chnl_status {
>>> -    PMIC_ARB_STATUS_DONE    = BIT(0),
>>> -    PMIC_ARB_STATUS_FAILURE    = BIT(1),
>>> -    PMIC_ARB_STATUS_DENIED    = BIT(2),
>>> -    PMIC_ARB_STATUS_DROPPED    = BIT(3),
>>> +struct pmic_arb_chnl_status_mask {
>>> +    u8    done;
>>> +    u8    failure;
>>> +    u8    crc;
>>> +    u8    parity;
>>> +    u8    nack;
>>> +    u8    denied;
>>> +    u8    dropped;
>>> +};
>>> +
>>> +static const struct pmic_arb_chnl_status_mask chnl_status_mask = {
>>> +    .done        = BIT(0),
>>> +    .failure    = BIT(1),
>>> +    .crc        = 0,
>>> +    .parity        = 0,
>>> +    .nack        = 0,
>>> +    .denied        = BIT(2),
>>> +    .dropped    = BIT(3),
>>> +};
>>> +
>>> +static const struct pmic_arb_chnl_status_mask chnl_status_mask_v8p5 
>>> = {
>>> +    .done        = BIT(0),
>>> +    .failure    = BIT(1),
>>> +    .crc        = BIT(2),
>>> +    .parity        = BIT(3),
>>> +    .nack        = BIT(4),
>>> +    .denied        = BIT(5),
>>> +    .dropped    = BIT(6),
>> Would it be better to extract generation-specific callback to decode the
>> error rather than defining the list of masks?
>
> Are you proposing to add a callback in pmic_arb_ver_ops, like 
> '*check_chnl_status', and create separate implementations for PMIC 
> arbiter versions before and after v8.5?
>
> This approach would add more extensive code changes with some code 
> duplication, especially for handling common error bits shared across 
> all versions—even if they only print error messages and return an 
> error code. Is that a concern?
>
> Fenglin 

Hi Dmitry,

Please let me know if this your preferred way and if you are fine with 
the concern that I mentioned.

I can come up with this approach and post a new patch.

Thanks

Fenglin