Re: [PATCH] spmi: pmic_arb: add support for hw version 2

[Gilad Avidov <gavidov@codeaurora.org>]

On 1/23/2015 10:03 AM, Stanimir Varbanov wrote:
> Hi Gilad,
>
> On 01/20/2015 03:10 AM, Gilad Avidov wrote:
>> Qualcomm PMIC Arbiter version-2 changes from version-1 are:
>>
>> - Some diffrent register offsets.
>> - New channel register space, one per PMIC peripheral (ppid).
>>    All tx tarffic uses these channels.
>> - New observer register space. All rx trafic uses this space.
>> - Diffrent command format for spmi command registers.
>>
>> Signed-off-by: Gilad Avidov <gavidov@codeaurora.org>
>> Acked-by: Sagar Dharia <sdharia@codeaurora.org>
>> ---
>>   .../bindings/spmi/qcom,spmi-pmic-arb.txt           |  11 +-
>>   drivers/spmi/spmi-pmic-arb.c                       | 295 ++++++++++++++++++---
>>   2 files changed, 263 insertions(+), 43 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
>> index 715d099..827bd21 100644
>> --- a/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
>> +++ b/Documentation/devicetree/bindings/spmi/qcom,spmi-pmic-arb.txt
>> @@ -1,11 +1,11 @@
>>   Qualcomm SPMI Controller (PMIC Arbiter)
>>   
>> -The SPMI PMIC Arbiter is found on the Snapdragon 800 Series.  It is an SPMI
>> +The SPMI PMIC Arbiter is found on Snapdragon chipsets.  It is an SPMI
>>   controller with wrapping arbitration logic to allow for multiple on-chip
>>   devices to control a single SPMI master.
>>   
>> -The PMIC Arbiter can also act as an interrupt controller, providing interrupts
>> -to slave devices.
>> +The PMIC Arbiter is also an interrupt controller, interrupting the Snapdragon
>> +on dtection of a sequence initiated by a request-capable-slave to the master.
>>   
> <snip>
>
>>   
>> -/* Non-data command */
>> -static int pmic_arb_cmd(struct spmi_controller *ctrl, u8 opc, u8 sid)
>> +static int
>> +pmic_arb_non_data_cmd_v1(struct spmi_controller *ctrl, u8 opc, u8 sid)
>>   {
>>   	struct spmi_pmic_arb_dev *pmic_arb = spmi_controller_get_drvdata(ctrl);
>>   	unsigned long flags;
>>   	u32 cmd;
>>   	int rc;
>> -
>> -	/* Check for valid non-data command */
>> -	if (opc < SPMI_CMD_RESET || opc > SPMI_CMD_WAKEUP)
>> -		return -EINVAL;
>> +	u32 offset = pmic_arb->ver->offset(pmic_arb, sid, 0);
>>   
>>   	cmd = ((opc | 0x40) << 27) | ((sid & 0xf) << 20);
>>   
>>   	raw_spin_lock_irqsave(&pmic_arb->lock, flags);
>> -	pmic_arb_base_write(pmic_arb, PMIC_ARB_CMD(pmic_arb->channel), cmd);
>> -	rc = pmic_arb_wait_for_done(ctrl);
>> +	pmic_arb_base_write(pmic_arb, offset + PMIC_ARB_CMD, cmd);
>> +	rc = pmic_arb_wait_for_done(ctrl, pmic_arb->wr_base, sid, 0);
>>   	raw_spin_unlock_irqrestore(&pmic_arb->lock, flags);
>>   
>>   	return rc;
>>   }
>>   
>> +/* Unsupported by HW */
>> +static int
>> +pmic_arb_non_data_cmd_v2(struct spmi_controller *ctrl, u8 opc, u8 sid)
>> +{
>> +	return -EOPNOTSUPP;
>> +}

Hi Stanimir,

> Does pmic arbiter v2 supports SPMI_CMD_WAKEUP and SPMI_CMD_SHUTDOWN
> commands? If so how we send those commands to the arbiter when the
pmic-arbiter v2 does not support non-data commands including the two 
that you have mentioned above.
> .non_data_cmd operation returns EOPNOTSUPP. If we returning EOPNOTSUPP
> the spmi bus .probe method will not call spmi driver .probe. See spmi.c
> spmi_drv_probe().
Very keen observation!

The slaves that I'm working on do not need nor support the wakeup command.
I'll add a patch to add a new device tree boolean property to the 
framework, maybe "skip-wakeup", for similar slaves.
Then change spmi_drv_probe() to skip the wakeup if the property is there.
> <snip>
Thanks,
Gilad

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