Re: [PATCH V1 3/4] spmi: pmic-arb: add support for HW version 5

kgunda
2017-07-06T10:48:59Z

On 2017-07-06 12:29, Stephen Boyd wrote:
> On 07/03, Kiran Gunda wrote:
>> From: David Collins <collinsd@codeaurora.org>
>>
>> Add support for version 5 of the SPMI PMIC arbiter. It utilizes
>> different offsets for registers than those found on version 3.
>> Also, the procedure to determine if writing and IRQ access is
>> allowed for a given PPID changes for version 5.
>>
>
> David's sign-off is missing?
>
Looks like mistakenly removed along with change-id. Will add it in the
next patch.
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> ---
>> drivers/spmi/spmi-pmic-arb.c | 233
>> +++++++++++++++++++++++++++++++++++++++----
>> 1 file changed, 214 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/spmi/spmi-pmic-arb.c
>> b/drivers/spmi/spmi-pmic-arb.c
>> index 86affb0..bc88c19 100644
>> --- a/drivers/spmi/spmi-pmic-arb.c
>> +++ b/drivers/spmi/spmi-pmic-arb.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2012-2015, The Linux Foundation. All rights
>> reserved.
>> + * Copyright (c) 2012-2017, The Linux Foundation. All rights
>> reserved.
>> *
>> * This program is free software; you can redistribute it and/or
>> modify
>> * it under the terms of the GNU General Public License version 2 and
>> @@ -29,6 +29,7 @@
>> #define PMIC_ARB_VERSION 0x0000
>> #define PMIC_ARB_VERSION_V2_MIN 0x20010000
>> #define PMIC_ARB_VERSION_V3_MIN 0x30000000
>> +#define PMIC_ARB_VERSION_V5_MIN 0x50000000
>> #define PMIC_ARB_INT_EN 0x0004
>>
>> /* PMIC Arbiter channel registers offsets */
>> @@ -39,7 +40,6 @@
>> #define PMIC_ARB_WDATA1 0x14
>> #define PMIC_ARB_RDATA0 0x18
>> #define PMIC_ARB_RDATA1 0x1C
>> -#define PMIC_ARB_REG_CHNL(N) (0x800 + 0x4 * (N))
>>
>> /* Mapping Table */
>> #define SPMI_MAPPING_TABLE_REG(N) (0x0B00 + (4 * (N)))
>> @@ -52,6 +52,8 @@
>> #define SPMI_MAPPING_TABLE_TREE_DEPTH 16 /* Maximum of 16-bits */
>> #define PMIC_ARB_MAX_PPID BIT(12) /* PPID is 12bit */
>> #define PMIC_ARB_APID_VALID BIT(15)
>> +#define PMIC_ARB_CHAN_IS_IRQ_OWNER(reg) ((reg) & BIT(24))
>> +#define INVALID (-1)
>
> INVALID_EE? And then define it to be 0xff?
>
ok... will modify it.
>>
>> /* Ownership Table */
>> #define SPMI_OWNERSHIP_TABLE_REG(N) (0x0700 + (4 * (N)))
>> @@ -86,6 +88,15 @@ enum pmic_arb_cmd_op_code {
>> PMIC_ARB_OP_ZERO_WRITE = 16,
>> };
>>
>> +/*
>> + * PMIC arbiter version 5 uses different register offsets for
>> read/write vs
>> + * observer channels.
>> + */
>> +enum pmic_arb_channel {
>> + PMIC_ARB_CHANNEL_RW,
>> + PMIC_ARB_CHANNEL_OBS,
>> +};
>> +
>> /* Maximum number of support PMIC peripherals */
>> #define PMIC_ARB_MAX_PERIPHS 512
> [...]
>> @@ -112,7 +123,8 @@ enum pmic_arb_cmd_op_code {
>>
>> +
>> /* v2 offset per ppid and per ee */
>> static int pmic_arb_offset_v2(struct spmi_pmic_arb *pmic_arb, u8 sid,
>> u16 addr,
>> - u32 *offset)
>> + enum pmic_arb_channel ch_type, u32 *offset)
>> {
>> u16 apid;
>> u16 ppid;
>> @@ -841,6 +947,34 @@ static int pmic_arb_offset_v2(struct
>> spmi_pmic_arb *pmic_arb, u8 sid, u16 addr,
>> return 0;
>> }
>>
>> +/*
>> + * v5 offset per ee and per apid for observer channels and per apid
>> for
>> + * read/write channels.
>> + */
>> +static int pmic_arb_offset_v5(struct spmi_pmic_arb *pmic_arb, u8 sid,
>> u16 addr,
>> + enum pmic_arb_channel ch_type, u32 *offset)
>> +{
>> + u16 apid;
>> + int rc;
>> + u16 ppid = (sid << 8) | (addr >> 8);
>> +
>> + rc = pmic_arb_ppid_to_apid_v5(pmic_arb, ppid);
>> + if (rc < 0)
>> + return rc;
>> +
>> + apid = rc;
>> + switch (ch_type) {
>> + case PMIC_ARB_CHANNEL_OBS:
>> + *offset = 0x10000 * pmic_arb->ee + 0x80 * apid;
>> + break;
>> + case PMIC_ARB_CHANNEL_RW:
>> + *offset = 0x10000 * apid;
>> + break;
>> + }
>> +
>> + return 0;
>
> Why doesn't this also return an iomem pointer?
Actually the purpose of this function is to find the offset of the APID.
That's why we are not returning the iomem pointer. But we can return the
offset
from this function, instead of a pass by pointer. Will change it to
return the offset
in next patch.