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

Groups > linux.kernel > #1682093 > unrolled thread

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

Started byStephen Boyd <sboyd@codeaurora.org>
First post2017-07-06 09:00 +0200
Last post2017-07-06 12:50 +0200
Articles 2 — 2 participants

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 V1 3/4] spmi: pmic-arb: add support for HW version 5 Stephen Boyd <sboyd@codeaurora.org> - 2017-07-06 09:00 +0200
    Re: [PATCH V1 3/4] spmi: pmic-arb: add support for HW version 5 kgunda@codeaurora.org - 2017-07-06 12:50 +0200

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

FromStephen Boyd <sboyd@codeaurora.org>
Date2017-07-06 09:00 +0200
SubjectRe: [PATCH V1 3/4] spmi: pmic-arb: add support for HW version 5
Message-ID<u08VA-Sf-17@gated-at.bofh.it>
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?

> 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?

>  
>  /* 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?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[toc] | [next] | [standalone]

#1682340

Fromkgunda@codeaurora.org
Date2017-07-06 12:50 +0200
Message-ID<u0cwa-38K-23@gated-at.bofh.it>
In reply to#1682093
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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web