diff mbox

[V1,04/15] spmi: pmic-arb: optimize table lookups

Message ID 1496147943-25822-5-git-send-email-kgunda@codeaurora.org (mailing list archive)
State Not Applicable, archived
Delegated to: Andy Gross
Headers show

Commit Message

Kiran Gunda May 30, 2017, 12:38 p.m. UTC 
From: Abhijeet Dharmapurikar <adharmap@codeaurora.org>

The current driver uses a mix of radix tree and a fwd lookup
table to translate between apid and ppid. It is buggy and confusing.

Instead simply use a radix tree for v1 hardware and use the
forward lookup table for v2.

Signed-off-by: Abhijeet Dharmapurikar <adharmap@codeaurora.org>
Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
---
 drivers/spmi/spmi-pmic-arb.c | 144 ++++++++++++++++++++++++++-----------------
 1 file changed, 88 insertions(+), 56 deletions(-)

Comments

Stephen Boyd May 31, 2017, 1:44 a.m. UTC | #1 
On 05/30, Kiran Gunda wrote:
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index 7201611..6320f1f 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c
> @@ -164,6 +164,8 @@ struct spmi_pmic_arb {
>   *			on v2 offset of SPMI_PIC_IRQ_CLEARn.
>   */
>  struct pmic_arb_ver_ops {
> +	int (*ppid_to_apid)(struct spmi_pmic_arb *pa, u8 sid, u16 addr,
> +			u8 *apid);

Nitpick: It's called "pa" but "dev" in the next line.

>  	int (*mode)(struct spmi_pmic_arb *dev, u8 sid, u16 addr,
>  			mode_t *mode);
>  	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
> @@ -657,42 +659,6 @@ struct spmi_pmic_arb_irq_spec {
>  	unsigned irq:3;
>  };
>  
> -static int search_mapping_table(struct spmi_pmic_arb *pa,
> -				struct spmi_pmic_arb_irq_spec *spec,

Perhaps the spec should be removed at some point if this was the
only place it was passed to.

> -				u8 *apid)

This code looks mostly unchanged, so please leave it as it is and
move the pmic_arb_ppid_to_apid_v1() function here so we can see
what actually changed.

> -{
> -	u16 ppid = spec->slave << 8 | spec->per;
> -	u32 *mapping_table = pa->mapping_table;
> -	int index = 0, i;
> -	u32 data;
> -
> -	for (i = 0; i < SPMI_MAPPING_TABLE_TREE_DEPTH; ++i) {
> -		if (!test_and_set_bit(index, pa->mapping_table_valid))
> -			mapping_table[index] = readl_relaxed(pa->cnfg +
> -						SPMI_MAPPING_TABLE_REG(index));
> -
> -		data = mapping_table[index];
> -
> -		if (ppid & BIT(SPMI_MAPPING_BIT_INDEX(data))) {
> -			if (SPMI_MAPPING_BIT_IS_1_FLAG(data)) {
> -				index = SPMI_MAPPING_BIT_IS_1_RESULT(data);
> -			} else {
> -				*apid = SPMI_MAPPING_BIT_IS_1_RESULT(data);
> -				return 0;
> -			}
> -		} else {
> -			if (SPMI_MAPPING_BIT_IS_0_FLAG(data)) {
> -				index = SPMI_MAPPING_BIT_IS_0_RESULT(data);
> -			} else {
> -				*apid = SPMI_MAPPING_BIT_IS_0_RESULT(data);
> -				return 0;
> -			}
> -		}
> -	}
> -
> -	return -ENODEV;
> -}
> -
>  static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
>  					   struct device_node *controller,
>  					   const u32 *intspec,
> @@ -702,7 +668,7 @@ static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
>  {
>  	struct spmi_pmic_arb *pa = d->host_data;
>  	struct spmi_pmic_arb_irq_spec spec;
> -	int err;
> +	int rc;
>  	u8 apid;
>  
>  	dev_dbg(&pa->spmic->dev,
> @@ -720,11 +686,14 @@ static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
>  	spec.per   = intspec[1];
>  	spec.irq   = intspec[2];
>  
> -	err = search_mapping_table(pa, &spec, &apid);
> -	if (err)
> -		return err;
> -
> -	pa->apid_to_ppid[apid] = spec.slave << 8 | spec.per;
> +	rc = pa->ver_ops->ppid_to_apid(pa, intspec[0],
> +			(intspec[1] << 8), &apid);
> +	if (rc < 0) {
> +		dev_err(&pa->spmic->dev,
> +		"failed to xlate sid = 0x%x, periph = 0x%x, irq = %x rc = %d\n",
> +		intspec[0], intspec[1], intspec[2], rc);
> +		return rc;
> +	}
>  
>  	/* Keep track of {max,min}_apid for bounding search during interrupt */
>  	if (apid > pa->max_apid)
> @@ -758,6 +727,54 @@ static int qpnpint_irq_domain_map(struct irq_domain *d,
>  }
>  
>  static int
> +pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pa, u8 sid, u16 addr, u8 *apid)
> +{
> +	u16 ppid = sid << 8 | ((addr >> 8) & 0xFF);

The function is called ppid_to_apid, but we pass in a sid and
addr. Just pass a ppid instead of both split out?

> +	u32 *mapping_table = pa->mapping_table;
> +	int index = 0, i;
> +	u16 apid_valid;
> +	u32 data;
> +
> +	apid_valid = pa->ppid_to_apid[ppid];
> +	if (apid_valid & PMIC_ARB_CHAN_VALID) {
> +		*apid = (apid_valid & ~PMIC_ARB_CHAN_VALID);
> +		return 0;
> +	}
> +

From here to...

> +	for (i = 0; i < SPMI_MAPPING_TABLE_TREE_DEPTH; ++i) {
> +		if (!test_and_set_bit(index, pa->mapping_table_valid))
> +			mapping_table[index] = readl_relaxed(pa->cnfg +
> +						SPMI_MAPPING_TABLE_REG(index));
> +
> +		data = mapping_table[index];
> +
> +		if (ppid & BIT(SPMI_MAPPING_BIT_INDEX(data))) {
> +			if (SPMI_MAPPING_BIT_IS_1_FLAG(data)) {
> +				index = SPMI_MAPPING_BIT_IS_1_RESULT(data);
> +			} else {
> +				*apid = SPMI_MAPPING_BIT_IS_1_RESULT(data);
> +				pa->ppid_to_apid[ppid]
> +					= *apid | PMIC_ARB_CHAN_VALID;
> +				pa->apid_to_ppid[*apid] = ppid;
> +				return 0;
> +			}
> +		} else {
> +			if (SPMI_MAPPING_BIT_IS_0_FLAG(data)) {
> +				index = SPMI_MAPPING_BIT_IS_0_RESULT(data);
> +			} else {
> +				*apid = SPMI_MAPPING_BIT_IS_0_RESULT(data);
> +				pa->ppid_to_apid[ppid]
> +					= *apid | PMIC_ARB_CHAN_VALID;
> +				pa->apid_to_ppid[*apid] = ppid;
> +				return 0;
> +			}
> +		}
> +	}
> +
> +	return -ENODEV;
> +}

here is all unchanged? Oh except apid_to_ppid/ppid_to_apid is
inserted.

> +
> +static int
>  pmic_arb_mode_v1(struct spmi_pmic_arb *pa, u8 sid, u16 addr, mode_t *mode)
>  {
>  	*mode = S_IRUSR | S_IWUSR;
> @@ -797,6 +814,7 @@ static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pa, u16 ppid)
>  
>  		id = (regval >> 8) & PMIC_ARB_PPID_MASK;
>  		pa->ppid_to_apid[id] = apid | PMIC_ARB_CHAN_VALID;
> +		pa->apid_to_ppid[apid] = id;
>  		if (id == ppid) {
>  			apid |= PMIC_ARB_CHAN_VALID;
>  			break;
> @@ -809,20 +827,35 @@ static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pa, u16 ppid)
>  
>  
>  static int
> -pmic_arb_mode_v2(struct spmi_pmic_arb *pa, u8 sid, u16 addr, mode_t *mode)
> +pmic_arb_ppid_to_apid_v2(struct spmi_pmic_arb *pa, u8 sid, u16 addr, u8 *apid)
>  {
>  	u16 ppid = (sid << 8) | (addr >> 8);
> -	u16 apid;
> -	u8 owner;
> +	u16 apid_valid;
>  
> -	apid = pa->ppid_to_apid[ppid];
> -	if (!(apid & PMIC_ARB_CHAN_VALID))
> +	apid_valid = pa->ppid_to_apid[ppid];
> +	if (!(apid_valid & PMIC_ARB_CHAN_VALID))
> +		apid_valid = pmic_arb_find_apid(pa, ppid);
> +	if (!(apid_valid & PMIC_ARB_CHAN_VALID))
>  		return -ENODEV;
>  
> +	*apid = (apid_valid & ~PMIC_ARB_CHAN_VALID);

Useless parenthesis. Please remove.

Why can't we just return a number that's >= 0 for apid and < 0
for an error from this op? Pointer passing is odd style.

> +	return 0;
> +}
> +
Kiran Gunda June 1, 2017, 4:53 p.m. UTC | #2 
On 2017-05-31 07:14, Stephen Boyd wrote:
> On 05/30, Kiran Gunda wrote:
>> diff --git a/drivers/spmi/spmi-pmic-arb.c 
>> b/drivers/spmi/spmi-pmic-arb.c
>> index 7201611..6320f1f 100644
>> --- a/drivers/spmi/spmi-pmic-arb.c
>> +++ b/drivers/spmi/spmi-pmic-arb.c
>> @@ -164,6 +164,8 @@ struct spmi_pmic_arb {
>>   *			on v2 offset of SPMI_PIC_IRQ_CLEARn.
>>   */
>>  struct pmic_arb_ver_ops {
>> +	int (*ppid_to_apid)(struct spmi_pmic_arb *pa, u8 sid, u16 addr,
>> +			u8 *apid);
> 
> Nitpick: It's called "pa" but "dev" in the next line.
> 
Will rename the pa -> dev in the next patch.
>>  	int (*mode)(struct spmi_pmic_arb *dev, u8 sid, u16 addr,
>>  			mode_t *mode);
>>  	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
>> @@ -657,42 +659,6 @@ struct spmi_pmic_arb_irq_spec {
>>  	unsigned irq:3;
>>  };
>> 
>> -static int search_mapping_table(struct spmi_pmic_arb *pa,
>> -				struct spmi_pmic_arb_irq_spec *spec,
> 
> Perhaps the spec should be removed at some point if this was the
> only place it was passed to.
> 
Yes. This is passed only in this function. You want to remove this
structure and pass the slave, per and irq as function parameters?
If so we will do it in the next patch.

>> -				u8 *apid)
> 
> This code looks mostly unchanged, so please leave it as it is and
> move the pmic_arb_ppid_to_apid_v1() function here so we can see
> what actually changed.
> 
Sure. Will do that in the next patch.
>> -{
>> -	u16 ppid = spec->slave << 8 | spec->per;
>> -	u32 *mapping_table = pa->mapping_table;
>> -	int index = 0, i;
>> -	u32 data;
>> -
>> -	for (i = 0; i < SPMI_MAPPING_TABLE_TREE_DEPTH; ++i) {
>> -		if (!test_and_set_bit(index, pa->mapping_table_valid))
>> -			mapping_table[index] = readl_relaxed(pa->cnfg +
>> -						SPMI_MAPPING_TABLE_REG(index));
>> -
>> -		data = mapping_table[index];
>> -
>> -		if (ppid & BIT(SPMI_MAPPING_BIT_INDEX(data))) {
>> -			if (SPMI_MAPPING_BIT_IS_1_FLAG(data)) {
>> -				index = SPMI_MAPPING_BIT_IS_1_RESULT(data);
>> -			} else {
>> -				*apid = SPMI_MAPPING_BIT_IS_1_RESULT(data);
>> -				return 0;
>> -			}
>> -		} else {
>> -			if (SPMI_MAPPING_BIT_IS_0_FLAG(data)) {
>> -				index = SPMI_MAPPING_BIT_IS_0_RESULT(data);
>> -			} else {
>> -				*apid = SPMI_MAPPING_BIT_IS_0_RESULT(data);
>> -				return 0;
>> -			}
>> -		}
>> -	}
>> -
>> -	return -ENODEV;
>> -}
>> -
>>  static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
>>  					   struct device_node *controller,
>>  					   const u32 *intspec,
>> @@ -702,7 +668,7 @@ static int qpnpint_irq_domain_dt_translate(struct 
>> irq_domain *d,
>>  {
>>  	struct spmi_pmic_arb *pa = d->host_data;
>>  	struct spmi_pmic_arb_irq_spec spec;
>> -	int err;
>> +	int rc;
>>  	u8 apid;
>> 
>>  	dev_dbg(&pa->spmic->dev,
>> @@ -720,11 +686,14 @@ static int 
>> qpnpint_irq_domain_dt_translate(struct irq_domain *d,
>>  	spec.per   = intspec[1];
>>  	spec.irq   = intspec[2];
>> 
>> -	err = search_mapping_table(pa, &spec, &apid);
>> -	if (err)
>> -		return err;
>> -
>> -	pa->apid_to_ppid[apid] = spec.slave << 8 | spec.per;
>> +	rc = pa->ver_ops->ppid_to_apid(pa, intspec[0],
>> +			(intspec[1] << 8), &apid);
>> +	if (rc < 0) {
>> +		dev_err(&pa->spmic->dev,
>> +		"failed to xlate sid = 0x%x, periph = 0x%x, irq = %x rc = %d\n",
>> +		intspec[0], intspec[1], intspec[2], rc);
>> +		return rc;
>> +	}
>> 
>>  	/* Keep track of {max,min}_apid for bounding search during interrupt 
>> */
>>  	if (apid > pa->max_apid)
>> @@ -758,6 +727,54 @@ static int qpnpint_irq_domain_map(struct 
>> irq_domain *d,
>>  }
>> 
>>  static int
>> +pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pa, u8 sid, u16 addr, 
>> u8 *apid)
>> +{
>> +	u16 ppid = sid << 8 | ((addr >> 8) & 0xFF);
> 
> The function is called ppid_to_apid, but we pass in a sid and
> addr. Just pass a ppid instead of both split out?
> 
Sure .. Will do that in the next patch.
>> +	u32 *mapping_table = pa->mapping_table;
>> +	int index = 0, i;
>> +	u16 apid_valid;
>> +	u32 data;
>> +
>> +	apid_valid = pa->ppid_to_apid[ppid];
>> +	if (apid_valid & PMIC_ARB_CHAN_VALID) {
>> +		*apid = (apid_valid & ~PMIC_ARB_CHAN_VALID);
>> +		return 0;
>> +	}
>> +
> 
> From here to...
> 
>> +	for (i = 0; i < SPMI_MAPPING_TABLE_TREE_DEPTH; ++i) {
>> +		if (!test_and_set_bit(index, pa->mapping_table_valid))
>> +			mapping_table[index] = readl_relaxed(pa->cnfg +
>> +						SPMI_MAPPING_TABLE_REG(index));
>> +
>> +		data = mapping_table[index];
>> +
>> +		if (ppid & BIT(SPMI_MAPPING_BIT_INDEX(data))) {
>> +			if (SPMI_MAPPING_BIT_IS_1_FLAG(data)) {
>> +				index = SPMI_MAPPING_BIT_IS_1_RESULT(data);
>> +			} else {
>> +				*apid = SPMI_MAPPING_BIT_IS_1_RESULT(data);
>> +				pa->ppid_to_apid[ppid]
>> +					= *apid | PMIC_ARB_CHAN_VALID;
>> +				pa->apid_to_ppid[*apid] = ppid;
>> +				return 0;
>> +			}
>> +		} else {
>> +			if (SPMI_MAPPING_BIT_IS_0_FLAG(data)) {
>> +				index = SPMI_MAPPING_BIT_IS_0_RESULT(data);
>> +			} else {
>> +				*apid = SPMI_MAPPING_BIT_IS_0_RESULT(data);
>> +				pa->ppid_to_apid[ppid]
>> +					= *apid | PMIC_ARB_CHAN_VALID;
>> +				pa->apid_to_ppid[*apid] = ppid;
>> +				return 0;
>> +			}
>> +		}
>> +	}
>> +
>> +	return -ENODEV;
>> +}
> 
> here is all unchanged? Oh except apid_to_ppid/ppid_to_apid is
> inserted.
> 
Yes. correct. Will move this part to the original place in the next 
patch.
>> +
>> +static int
>>  pmic_arb_mode_v1(struct spmi_pmic_arb *pa, u8 sid, u16 addr, mode_t 
>> *mode)
>>  {
>>  	*mode = S_IRUSR | S_IWUSR;
>> @@ -797,6 +814,7 @@ static u16 pmic_arb_find_apid(struct spmi_pmic_arb 
>> *pa, u16 ppid)
>> 
>>  		id = (regval >> 8) & PMIC_ARB_PPID_MASK;
>>  		pa->ppid_to_apid[id] = apid | PMIC_ARB_CHAN_VALID;
>> +		pa->apid_to_ppid[apid] = id;
>>  		if (id == ppid) {
>>  			apid |= PMIC_ARB_CHAN_VALID;
>>  			break;
>> @@ -809,20 +827,35 @@ static u16 pmic_arb_find_apid(struct 
>> spmi_pmic_arb *pa, u16 ppid)
>> 
>> 
>>  static int
>> -pmic_arb_mode_v2(struct spmi_pmic_arb *pa, u8 sid, u16 addr, mode_t 
>> *mode)
>> +pmic_arb_ppid_to_apid_v2(struct spmi_pmic_arb *pa, u8 sid, u16 addr, 
>> u8 *apid)
>>  {
>>  	u16 ppid = (sid << 8) | (addr >> 8);
>> -	u16 apid;
>> -	u8 owner;
>> +	u16 apid_valid;
>> 
>> -	apid = pa->ppid_to_apid[ppid];
>> -	if (!(apid & PMIC_ARB_CHAN_VALID))
>> +	apid_valid = pa->ppid_to_apid[ppid];
>> +	if (!(apid_valid & PMIC_ARB_CHAN_VALID))
>> +		apid_valid = pmic_arb_find_apid(pa, ppid);
>> +	if (!(apid_valid & PMIC_ARB_CHAN_VALID))
>>  		return -ENODEV;
>> 
>> +	*apid = (apid_valid & ~PMIC_ARB_CHAN_VALID);
> 
> Useless parenthesis. Please remove.
> 
Sure. Will remove it in the next patch.
> Why can't we just return a number that's >= 0 for apid and < 0
> for an error from this op? Pointer passing is odd style.
> 
Sure. Will change it in the next patch.
>> +	return 0;
>> +}
>> +
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephen Boyd June 2, 2017, 6:31 p.m. UTC | #3 
On 06/01, kgunda@codeaurora.org wrote:
> On 2017-05-31 07:14, Stephen Boyd wrote:
> >On 05/30, Kiran Gunda wrote:
> >>diff --git a/drivers/spmi/spmi-pmic-arb.c
> >>b/drivers/spmi/spmi-pmic-arb.c
> >>index 7201611..6320f1f 100644
> >>--- a/drivers/spmi/spmi-pmic-arb.c
> >>+++ b/drivers/spmi/spmi-pmic-arb.c
> >>@@ -657,42 +659,6 @@ struct spmi_pmic_arb_irq_spec {
> >> 	unsigned irq:3;
> >> };
> >>
> >>-static int search_mapping_table(struct spmi_pmic_arb *pa,
> >>-				struct spmi_pmic_arb_irq_spec *spec,
> >
> >Perhaps the spec should be removed at some point if this was the
> >only place it was passed to.
> >
> Yes. This is passed only in this function. You want to remove this
> structure and pass the slave, per and irq as function parameters?
> If so we will do it in the next patch.

We don't pass it anywhere though? So I'm just saying have local
variables instead of the structure.
Kiran Gunda June 5, 2017, 6:33 a.m. UTC | #4 
On 2017-06-03 00:01, Stephen Boyd wrote:
> On 06/01, kgunda@codeaurora.org wrote:
>> On 2017-05-31 07:14, Stephen Boyd wrote:
>> >On 05/30, Kiran Gunda wrote:
>> >>diff --git a/drivers/spmi/spmi-pmic-arb.c
>> >>b/drivers/spmi/spmi-pmic-arb.c
>> >>index 7201611..6320f1f 100644
>> >>--- a/drivers/spmi/spmi-pmic-arb.c
>> >>+++ b/drivers/spmi/spmi-pmic-arb.c
>> >>@@ -657,42 +659,6 @@ struct spmi_pmic_arb_irq_spec {
>> >> 	unsigned irq:3;
>> >> };
>> >>
>> >>-static int search_mapping_table(struct spmi_pmic_arb *pa,
>> >>-				struct spmi_pmic_arb_irq_spec *spec,
>> >
>> >Perhaps the spec should be removed at some point if this was the
>> >only place it was passed to.
>> >
>> Yes. This is passed only in this function. You want to remove this
>> structure and pass the slave, per and irq as function parameters?
>> If so we will do it in the next patch.
> 
> We don't pass it anywhere though? So I'm just saying have local
> variables instead of the structure.
Sure. Thanks for the clarification.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
index 7201611..6320f1f 100644
--- a/drivers/spmi/spmi-pmic-arb.c
+++ b/drivers/spmi/spmi-pmic-arb.c
@@ -164,6 +164,8 @@  struct spmi_pmic_arb {
  *			on v2 offset of SPMI_PIC_IRQ_CLEARn.
  */
 struct pmic_arb_ver_ops {
+	int (*ppid_to_apid)(struct spmi_pmic_arb *pa, u8 sid, u16 addr,
+			u8 *apid);
 	int (*mode)(struct spmi_pmic_arb *dev, u8 sid, u16 addr,
 			mode_t *mode);
 	/* spmi commands (read_cmd, write_cmd, cmd) functionality */
@@ -657,42 +659,6 @@  struct spmi_pmic_arb_irq_spec {
 	unsigned irq:3;
 };
 
-static int search_mapping_table(struct spmi_pmic_arb *pa,
-				struct spmi_pmic_arb_irq_spec *spec,
-				u8 *apid)
-{
-	u16 ppid = spec->slave << 8 | spec->per;
-	u32 *mapping_table = pa->mapping_table;
-	int index = 0, i;
-	u32 data;
-
-	for (i = 0; i < SPMI_MAPPING_TABLE_TREE_DEPTH; ++i) {
-		if (!test_and_set_bit(index, pa->mapping_table_valid))
-			mapping_table[index] = readl_relaxed(pa->cnfg +
-						SPMI_MAPPING_TABLE_REG(index));
-
-		data = mapping_table[index];
-
-		if (ppid & BIT(SPMI_MAPPING_BIT_INDEX(data))) {
-			if (SPMI_MAPPING_BIT_IS_1_FLAG(data)) {
-				index = SPMI_MAPPING_BIT_IS_1_RESULT(data);
-			} else {
-				*apid = SPMI_MAPPING_BIT_IS_1_RESULT(data);
-				return 0;
-			}
-		} else {
-			if (SPMI_MAPPING_BIT_IS_0_FLAG(data)) {
-				index = SPMI_MAPPING_BIT_IS_0_RESULT(data);
-			} else {
-				*apid = SPMI_MAPPING_BIT_IS_0_RESULT(data);
-				return 0;
-			}
-		}
-	}
-
-	return -ENODEV;
-}
-
 static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
 					   struct device_node *controller,
 					   const u32 *intspec,
@@ -702,7 +668,7 @@  static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
 {
 	struct spmi_pmic_arb *pa = d->host_data;
 	struct spmi_pmic_arb_irq_spec spec;
-	int err;
+	int rc;
 	u8 apid;
 
 	dev_dbg(&pa->spmic->dev,
@@ -720,11 +686,14 @@  static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
 	spec.per   = intspec[1];
 	spec.irq   = intspec[2];
 
-	err = search_mapping_table(pa, &spec, &apid);
-	if (err)
-		return err;
-
-	pa->apid_to_ppid[apid] = spec.slave << 8 | spec.per;
+	rc = pa->ver_ops->ppid_to_apid(pa, intspec[0],
+			(intspec[1] << 8), &apid);
+	if (rc < 0) {
+		dev_err(&pa->spmic->dev,
+		"failed to xlate sid = 0x%x, periph = 0x%x, irq = %x rc = %d\n",
+		intspec[0], intspec[1], intspec[2], rc);
+		return rc;
+	}
 
 	/* Keep track of {max,min}_apid for bounding search during interrupt */
 	if (apid > pa->max_apid)
@@ -758,6 +727,54 @@  static int qpnpint_irq_domain_map(struct irq_domain *d,
 }
 
 static int
+pmic_arb_ppid_to_apid_v1(struct spmi_pmic_arb *pa, u8 sid, u16 addr, u8 *apid)
+{
+	u16 ppid = sid << 8 | ((addr >> 8) & 0xFF);
+	u32 *mapping_table = pa->mapping_table;
+	int index = 0, i;
+	u16 apid_valid;
+	u32 data;
+
+	apid_valid = pa->ppid_to_apid[ppid];
+	if (apid_valid & PMIC_ARB_CHAN_VALID) {
+		*apid = (apid_valid & ~PMIC_ARB_CHAN_VALID);
+		return 0;
+	}
+
+	for (i = 0; i < SPMI_MAPPING_TABLE_TREE_DEPTH; ++i) {
+		if (!test_and_set_bit(index, pa->mapping_table_valid))
+			mapping_table[index] = readl_relaxed(pa->cnfg +
+						SPMI_MAPPING_TABLE_REG(index));
+
+		data = mapping_table[index];
+
+		if (ppid & BIT(SPMI_MAPPING_BIT_INDEX(data))) {
+			if (SPMI_MAPPING_BIT_IS_1_FLAG(data)) {
+				index = SPMI_MAPPING_BIT_IS_1_RESULT(data);
+			} else {
+				*apid = SPMI_MAPPING_BIT_IS_1_RESULT(data);
+				pa->ppid_to_apid[ppid]
+					= *apid | PMIC_ARB_CHAN_VALID;
+				pa->apid_to_ppid[*apid] = ppid;
+				return 0;
+			}
+		} else {
+			if (SPMI_MAPPING_BIT_IS_0_FLAG(data)) {
+				index = SPMI_MAPPING_BIT_IS_0_RESULT(data);
+			} else {
+				*apid = SPMI_MAPPING_BIT_IS_0_RESULT(data);
+				pa->ppid_to_apid[ppid]
+					= *apid | PMIC_ARB_CHAN_VALID;
+				pa->apid_to_ppid[*apid] = ppid;
+				return 0;
+			}
+		}
+	}
+
+	return -ENODEV;
+}
+
+static int
 pmic_arb_mode_v1(struct spmi_pmic_arb *pa, u8 sid, u16 addr, mode_t *mode)
 {
 	*mode = S_IRUSR | S_IWUSR;
@@ -797,6 +814,7 @@  static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pa, u16 ppid)
 
 		id = (regval >> 8) & PMIC_ARB_PPID_MASK;
 		pa->ppid_to_apid[id] = apid | PMIC_ARB_CHAN_VALID;
+		pa->apid_to_ppid[apid] = id;
 		if (id == ppid) {
 			apid |= PMIC_ARB_CHAN_VALID;
 			break;
@@ -809,20 +827,35 @@  static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pa, u16 ppid)
 
 
 static int
-pmic_arb_mode_v2(struct spmi_pmic_arb *pa, u8 sid, u16 addr, mode_t *mode)
+pmic_arb_ppid_to_apid_v2(struct spmi_pmic_arb *pa, u8 sid, u16 addr, u8 *apid)
 {
 	u16 ppid = (sid << 8) | (addr >> 8);
-	u16 apid;
-	u8 owner;
+	u16 apid_valid;
 
-	apid = pa->ppid_to_apid[ppid];
-	if (!(apid & PMIC_ARB_CHAN_VALID))
+	apid_valid = pa->ppid_to_apid[ppid];
+	if (!(apid_valid & PMIC_ARB_CHAN_VALID))
+		apid_valid = pmic_arb_find_apid(pa, ppid);
+	if (!(apid_valid & PMIC_ARB_CHAN_VALID))
 		return -ENODEV;
 
+	*apid = (apid_valid & ~PMIC_ARB_CHAN_VALID);
+	return 0;
+}
+
+static int
+pmic_arb_mode_v2(struct spmi_pmic_arb *pa, u8 sid, u16 addr, mode_t *mode)
+{
+	u8 apid;
+	u8 owner;
+	int rc;
+
+	rc = pmic_arb_ppid_to_apid_v2(pa, sid, addr, &apid);
+	if (rc < 0)
+		return rc;
+
 	*mode = 0;
 	*mode |= S_IRUSR;
 
-	apid &= ~PMIC_ARB_CHAN_VALID;
 	owner = pa->apid_to_owner[apid];
 	if (owner == pa->ee)
 		*mode |= S_IWUSR;
@@ -833,15 +866,12 @@  static u16 pmic_arb_find_apid(struct spmi_pmic_arb *pa, u16 ppid)
 static int
 pmic_arb_offset_v2(struct spmi_pmic_arb *pa, u8 sid, u16 addr, u32 *offset)
 {
-	u16 ppid = (sid << 8) | (addr >> 8);
-	u16 apid;
+	u8 apid;
+	int rc;
 
-	apid = pa->ppid_to_apid[ppid];
-	if (!(apid & PMIC_ARB_CHAN_VALID))
-		apid = pmic_arb_find_apid(pa, ppid);
-	if (!(apid & PMIC_ARB_CHAN_VALID))
-		return -ENODEV;
-	apid &= ~PMIC_ARB_CHAN_VALID;
+	rc = pmic_arb_ppid_to_apid_v2(pa, sid, addr, &apid);
+	if (rc < 0)
+		return rc;
 
 	*offset = 0x1000 * pa->ee + 0x8000 * apid;
 	return 0;
@@ -898,6 +928,7 @@  static u32 pmic_arb_irq_clear_v2(u8 n)
 }
 
 static const struct pmic_arb_ver_ops pmic_arb_v1 = {
+	.ppid_to_apid		= pmic_arb_ppid_to_apid_v1,
 	.mode			= pmic_arb_mode_v1,
 	.non_data_cmd		= pmic_arb_non_data_cmd_v1,
 	.offset			= pmic_arb_offset_v1,
@@ -909,6 +940,7 @@  static u32 pmic_arb_irq_clear_v2(u8 n)
 };
 
 static const struct pmic_arb_ver_ops pmic_arb_v2 = {
+	.ppid_to_apid		= pmic_arb_ppid_to_apid_v2,
 	.mode			= pmic_arb_mode_v2,
 	.non_data_cmd		= pmic_arb_non_data_cmd_v2,
 	.offset			= pmic_arb_offset_v2,