Re: [PATCH 2/3] mfd: qcom-rpm: Driver for the Qualcomm RPM

[Bjorn Andersson]

On Thu, May 29, 2014 at 2:41 PM, Srinivas Kandagatla
<srinivas.kandagatla@xxxxxxxxxx> wrote:
>> Although we will not have more than one rpm in a system and therefore not
>> instatiate this driver multiple times I do not want to run it off the
>> global
>> state.
>>
> I agree.
>
> Why not make a separate data structure for the qcom_of_data?
>

That might make sense, definitely worth a spin.

>>> Should't you use the platform_get_resource_byname here?
[...]
> But my point on platform_get_resource_byname is to remove the dependency on
> the resource ordering, It is very difficult to catch errors resulting in
> wrong ordered resources in DT.
>

Sorry, forgot to answer the first part of your question.

The order in dt must be fixed, hence there isn't really any reason to
use the byname; as long as it doesn't add any context or such here. We
had a long long discussion with the dt guys about this a few months
back.

>>>> +       ret = irq_set_irq_wake(irq_ack, 1);
>>>> +       if (ret)
>>>> +               dev_warn(&pdev->dev, "failed to mark ack irq as
>>>> wakeup\n");
>>>> +
>>>
>>>
>>> ..]
>>>
>>> Shouln't these be set as part of the pm suspend call, if the device is
>>> wakeup capable?
>>>
>>>
>>
>> Is there any reason to toggle this?
>>
>> I'm not sure when this interrupt will actually be fired, but I don't see
>> any
>> harm in keeping it wakup enabled at all times.
>
>
> Typically the wake-up source is driven/enabled by the user. When the system
> goes to low-power state it would enable the wakeup on the irq. And when
> there is an interrupt it would wake up the system as part of resuming from
> low-power state.
>

But the RPM is the user; the ack irq is used to acknowledge requests
or to notify the Linux system about some event. If these happens when
the system is asleep it's always expected (in current implementation
at least) to wake up the system; hence I mark it as a wake irq.

> Again if you what this interrupt to wakeup the system, I would expect
> pm_wakeup_event/related calls in the interrupt handler too.
>

There is no power management here. I'll have to look into what value
this would add. I have to get back to you on this.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/