Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1713611 > unrolled thread

Re: [PATCH V2 11/12] spmi: pmic-arb: add support for HW version 5

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH V2 11/12] spmi: pmic-arb: add support for HW version 5 Shawn Guo <shawnguo@kernel.org> - 2017-08-17 09:50 +0200
    Re: [PATCH V2 11/12] spmi: pmic-arb: add support for HW version 5 kgunda@codeaurora.org - 2017-08-17 12:10 +0200
      Re: [PATCH V2 11/12] spmi: pmic-arb: add support for HW version 5 Shawn Guo <shawnguo@kernel.org> - 2017-08-17 14:40 +0200
        Re: [PATCH V2 11/12] spmi: pmic-arb: add support for HW version 5 kgunda@codeaurora.org - 2017-08-18 13:20 +0200
          Re: [PATCH V2 11/12] spmi: pmic-arb: add support for HW version 5 kgunda@codeaurora.org - 2017-08-18 13:30 +0200
          Re: [PATCH V2 11/12] spmi: pmic-arb: add support for HW version 5 Shawn Guo <shawnguo@kernel.org> - 2017-08-18 15:30 +0200

#1713611 — Re: [PATCH V2 11/12] spmi: pmic-arb: add support for HW version 5

Hi Kiran,

On Fri, Jul 28, 2017 at 12:40:46PM +0530, 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.
> 
> Signed-off-by: David Collins <collinsd@codeaurora.org>
> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/spmi/spmi-pmic-arb.c | 236 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 214 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/spmi/spmi-pmic-arb.c b/drivers/spmi/spmi-pmic-arb.c
> index bc68e08..3f46445 100644
> --- a/drivers/spmi/spmi-pmic-arb.c
> +++ b/drivers/spmi/spmi-pmic-arb.c

<snip>

> @@ -680,12 +701,19 @@ static int qpnpint_irq_domain_dt_translate(struct irq_domain *d,
>  	ppid = intspec[0] << 8 | intspec[1];
>  	rc = pmic_arb->ver_ops->ppid_to_apid(pmic_arb, ppid);
>  	if (rc < 0) {
> -		dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = 0x%x, periph = 0x%x, irq = %x rc = %d\n",
> +		dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u rc = %d\n",
>  		intspec[0], intspec[1], intspec[2], rc);
>  		return rc;
>  	}
>  
>  	apid = rc;
> +	if (pmic_arb->apid_data[apid].irq_ee != pmic_arb->ee) {
> +		dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = %#x, periph = %#x, irq = %u: ee=%u but owner=%u\n",
> +			intspec[0], intspec[1], intspec[2], pmic_arb->ee,
> +			pmic_arb->apid_data[apid].irq_ee);
> +		return -ENODEV;
> +	}
> +

It seems to me that this check breaks pm8916_gpios on db410c.  It causes
the failure of pinctrl-spmi-gpio driver probing, because npins returned
from platform_irq_count() call is 0.

Shawn

>  	/* Keep track of {max,min}_apid for bounding search during interrupt */
>  	if (apid > pmic_arb->max_apid)
>  		pmic_arb->max_apid = apid;

[toc] | [next] | [standalone]

#1713752

On 2017-08-17 13:11, Shawn Guo wrote:
> Hi Kiran,
> 
> On Fri, Jul 28, 2017 at 12:40:46PM +0530, 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.
>> 
>> Signed-off-by: David Collins <collinsd@codeaurora.org>
>> Signed-off-by: Kiran Gunda <kgunda@codeaurora.org>
>> Reviewed-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  drivers/spmi/spmi-pmic-arb.c | 236 
>> +++++++++++++++++++++++++++++++++++++++----
>>  1 file changed, 214 insertions(+), 22 deletions(-)
>> 
>> diff --git a/drivers/spmi/spmi-pmic-arb.c 
>> b/drivers/spmi/spmi-pmic-arb.c
>> index bc68e08..3f46445 100644
>> --- a/drivers/spmi/spmi-pmic-arb.c
>> +++ b/drivers/spmi/spmi-pmic-arb.c
> 
> <snip>
> 
>> @@ -680,12 +701,19 @@ static int 
>> qpnpint_irq_domain_dt_translate(struct irq_domain *d,
>>  	ppid = intspec[0] << 8 | intspec[1];
>>  	rc = pmic_arb->ver_ops->ppid_to_apid(pmic_arb, ppid);
>>  	if (rc < 0) {
>> -		dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = 0x%x, periph 
>> = 0x%x, irq = %x rc = %d\n",
>> +		dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = %#x, periph = 
>> %#x, irq = %u rc = %d\n",
>>  		intspec[0], intspec[1], intspec[2], rc);
>>  		return rc;
>>  	}
>> 
>>  	apid = rc;
>> +	if (pmic_arb->apid_data[apid].irq_ee != pmic_arb->ee) {
>> +		dev_err(&pmic_arb->spmic->dev, "failed to xlate sid = %#x, periph = 
>> %#x, irq = %u: ee=%u but owner=%u\n",
>> +			intspec[0], intspec[1], intspec[2], pmic_arb->ee,
>> +			pmic_arb->apid_data[apid].irq_ee);
>> +		return -ENODEV;
>> +	}
>> +
> 
> It seems to me that this check breaks pm8916_gpios on db410c.  It 
> causes
> the failure of pinctrl-spmi-gpio driver probing, because npins returned
> from platform_irq_count() call is 0.
> 
> Shawn
> 
Hi Shawn,
The intention of this check is to avoid the access to the peripherals 
those are
not owned by the current EE (APSS) and it is expected to return a 
failure if the
peripheral that is not owned by the current EE is trying to be accessed.

Looks like you trying to access the GPIOs 0xc000 and 0xc100,
which are owned by modem subsystem but not the APSS. That is why you 
seeing the failure for that.
Please change the ownership of those GPIOs to APSS (id '0') if you are 
working on a non-modem device (APQ).

Thanks,
Kiran
>>  	/* Keep track of {max,min}_apid for bounding search during interrupt 
>> */
>>  	if (apid > pmic_arb->max_apid)
>>  		pmic_arb->max_apid = apid;
> --
> 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

[toc] | [prev] | [next] | [standalone]

#1713959

On Thu, Aug 17, 2017 at 03:38:30PM +0530, kgunda@codeaurora.org wrote:
> The intention of this check is to avoid the access to the
> peripherals those are
> not owned by the current EE (APSS) and it is expected to return a
> failure if the
> peripheral that is not owned by the current EE is trying to be accessed.

Thanks for the background of this check.

> Looks like you trying to access the GPIOs 0xc000 and 0xc100,
> which are owned by modem subsystem but not the APSS. That is why you
> seeing the failure for that.
> Please change the ownership of those GPIOs to APSS (id '0') if you
> are working on a non-modem device (APQ).

Yes, it's device apq8016-sbc, and the error message is like below.

[    1.317186] spmi spmi-0: PMIC arbiter version v2 (0x20010000)
[    1.324784] spmi spmi-0: failed to xlate sid = 0x0, periph = 0xc0, irq = 0: ee=0 but owner=2
[    1.329430] spmi spmi-0: failed to xlate sid = 0x0, periph = 0xc0, irq = 0: ee=0 but owner=2
[    1.337626] qcom-spmi-gpio: probe of 200f000.spmi:pm8916@0:gpios@c000 failed with error -22

I see that 'qcom,ee' property of spmi_bus is already <0>.  Is that what
you mean by ownership of those GPIOs?

The pm8916_gpio is working fine on apq8016-sbc device with the current
mainline kernel.  It's not good to break the existing device without
a very good.  IMO, we should get this fixed before the patch is merged.

Shawn

[toc] | [prev] | [next] | [standalone]

#1714801

On 2017-08-17 17:58, Shawn Guo wrote:
> On Thu, Aug 17, 2017 at 03:38:30PM +0530, kgunda@codeaurora.org wrote:
>> The intention of this check is to avoid the access to the
>> peripherals those are
>> not owned by the current EE (APSS) and it is expected to return a
>> failure if the
>> peripheral that is not owned by the current EE is trying to be 
>> accessed.
> 
> Thanks for the background of this check.
> 
>> Looks like you trying to access the GPIOs 0xc000 and 0xc100,
>> which are owned by modem subsystem but not the APSS. That is why you
>> seeing the failure for that.
>> Please change the ownership of those GPIOs to APSS (id '0') if you
>> are working on a non-modem device (APQ).
> 
> Yes, it's device apq8016-sbc, and the error message is like below.
> 
> [    1.317186] spmi spmi-0: PMIC arbiter version v2 (0x20010000)
> [    1.324784] spmi spmi-0: failed to xlate sid = 0x0, periph = 0xc0,
> irq = 0: ee=0 but owner=2
> [    1.329430] spmi spmi-0: failed to xlate sid = 0x0, periph = 0xc0,
> irq = 0: ee=0 but owner=2
> [    1.337626] qcom-spmi-gpio: probe of
> 200f000.spmi:pm8916@0:gpios@c000 failed with error -22
> 
> I see that 'qcom,ee' property of spmi_bus is already <0>.  Is that what
> you mean by ownership of those GPIOs?
> 
> The pm8916_gpio is working fine on apq8016-sbc device with the current
> mainline kernel.  It's not good to break the existing device without
> a very good.  IMO, we should get this fixed before the patch is merged.
> 
> Shawn
> --

 From the logs the ownership for these GPIOs is not assigned to the 
application processor but to another master. Considering this is a 
(apq8016-sbc) APSS only platform ideally the fix for this would be to 
update the ownership table in the boot loader. The other option would be 
to make the  existing ownership check in SW optional for a platform via 
DT. I will push a patch shortly for this. Could you please give that a 
try.

Thanks,
Kiran
> 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

[toc] | [prev] | [next] | [standalone]

#1714809

On 2017-08-18 16:43, kgunda@codeaurora.org wrote:
> On 2017-08-17 17:58, Shawn Guo wrote:
>> On Thu, Aug 17, 2017 at 03:38:30PM +0530, kgunda@codeaurora.org wrote:
>>> The intention of this check is to avoid the access to the
>>> peripherals those are
>>> not owned by the current EE (APSS) and it is expected to return a
>>> failure if the
>>> peripheral that is not owned by the current EE is trying to be 
>>> accessed.
>> 
>> Thanks for the background of this check.
>> 
>>> Looks like you trying to access the GPIOs 0xc000 and 0xc100,
>>> which are owned by modem subsystem but not the APSS. That is why you
>>> seeing the failure for that.
>>> Please change the ownership of those GPIOs to APSS (id '0') if you
>>> are working on a non-modem device (APQ).
>> 
>> Yes, it's device apq8016-sbc, and the error message is like below.
>> 
>> [    1.317186] spmi spmi-0: PMIC arbiter version v2 (0x20010000)
>> [    1.324784] spmi spmi-0: failed to xlate sid = 0x0, periph = 0xc0,
>> irq = 0: ee=0 but owner=2
>> [    1.329430] spmi spmi-0: failed to xlate sid = 0x0, periph = 0xc0,
>> irq = 0: ee=0 but owner=2
>> [    1.337626] qcom-spmi-gpio: probe of
>> 200f000.spmi:pm8916@0:gpios@c000 failed with error -22
>> 
>> I see that 'qcom,ee' property of spmi_bus is already <0>.  Is that 
>> what
>> you mean by ownership of those GPIOs?
>> 
I do not mean that. The SPMI arbiter ownership register is programmed by 
the boot-loader,
where the actual ownership is set. The same is shown in the above logs 
as owner = 2.

>> The pm8916_gpio is working fine on apq8016-sbc device with the current
>> mainline kernel.  It's not good to break the existing device without
>> a very good.  IMO, we should get this fixed before the patch is 
>> merged.
>> 
>> Shawn
>> --
> 
> From the logs the ownership for these GPIOs is not assigned to the
> application processor but to another master. Considering this is a
> (apq8016-sbc) APSS only platform ideally the fix for this would be to
> update the ownership table in the boot loader. The other option would
> be to make the  existing ownership check in SW optional for a platform
> via DT. I will push a patch shortly for this. Could you please give
> that a try.
> 
> Thanks,
> Kiran
>> 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
> --
> 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

[toc] | [prev] | [next] | [standalone]

#1714954

On Fri, Aug 18, 2017 at 04:43:23PM +0530, kgunda@codeaurora.org wrote:
> From the logs the ownership for these GPIOs is not assigned to the
> application processor but to another master. Considering this is a
> (apq8016-sbc) APSS only platform ideally the fix for this would be
> to update the ownership table in the boot loader. The other option
> would be to make the  existing ownership check in SW optional for a
> platform via DT. I will push a patch shortly for this. Could you
> please give that a try.

I just tested it on db410c board, and it fixes the problem for me.
Thanks for the patch.

Shawn

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web