On 24 May 2018 at 09:01, Kai Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> wrote:
> at 2:34 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>
>> On 24 May 2018 at 07:51, Kai Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
>> wrote:
>>>
>>> at 5:55 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>>>
>>>> On 23 May 2018 at 11:29, Kai Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
>>>> wrote:
>>>>>
>>>>> at 4:59 PM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>>>>>
>>>>>> On 23 May 2018 at 10:31, Kai Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
>>>>>> wrote:
>>>>>>>
>>>>>>> Hi Alan,
>>>>>>>
>>>>>>> I just discussed with Ritesh about commit [1].
>>>>>>>
>>>>>>> Since it was concluded to be hardware problem, is it okay to revert
>>>>>>> [1]?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> I don't think so.
>>>>>>
>>>>>>> The end goal is to keep the card reader runtime suspended to reduce
>>>>>>> power
>>>>>>> consumption.
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Sounds like a call to pm_suspend_ignore_children() should be called
>>>>>> for the USB parent device, conditionally, when the HW works as you
>>>>>> suggest.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> Does your "conditionally" mean CONFIG_PM enabled/disabled?
>>>>
>>>>
>>>>
>>>> Nope. Depending on how the HW works.
>>>>
>>>> At least to my understanding, the fixes we made were on a HW that
>>>> requires the USB leaf devices nodes (mmc/memstick) to have the USB
>>>> parent to be powered. This seems to not be the case for your
>>>> situation.
>>>
>>>
>>>
>>> Yes, when the leaf devices (rtsx_usb_{sdmmc,ms}) are in use, the parent
>>> (rtsx_usb) needs to be powered.
>>
>>
>> Right, thanks for clarifying that.
>>
>>> The original issue I had is when the slot is empty, the card detection
>>> keeps
>>> waking up the card reader, because of pm_runtime_get_sync().
>>
>>
>> Ok, now I understand a bit more of what you want to do. Let me
>> elaborate on the mmc case (the memstick works the similar).
>>
>> So, depending on what card detect mechanism the mmc host supports, it
>> may set different host capabilities to inform the mmc core about a
>> desired behavior.
>>
>> In this case, rtsx_usb_sdmmc has set MMC_CAP_NEEDS_POLL, because the
>> driver consider its HW to *not* support a remote wakeup for card
>> detect. Hence the core schedules a work for polling, meaning that it
>> will wakeup the mmc host (pm_runtime_get_sync()) and then try to
>> initialize a card. When there is no card, it will fail and let the
>> host go back to sleep (pm_runtime_put) and re-schedule a new work.
>
>
> Thanks for the explanation.
>
>>
>>> It's unnecessary because the card reader has remote wakeup capability, it
>>> wakes up when there's a card gets plugged into slot.
>>
>>
>> Can you please elaborate on how card detect works for your case.
>>
>> The common way to do that for mmc, is to use a separate GPIO card
>> detect pin. Is possibly that what is used in your case as well - or do
>> you have another external logic serving the remote wakeup?
>
>
> From what I understand from Realtek, it's USB remote wakeup signaling.
> The card reader gets woken up when a card gets plugged into its slot.
I don't know USB good enough, but I am sure the remote wakeup
signaling at least needs to be activated and configured some how.
And more precisely, who wakes up the mmc host? Things just doesn't happen. :-)
> Although the card reader is powered up, it doesn't know there's a card
> (yet).
That's okay. No other mmc host driver does and that's why we run a mmc
initialization sequence to find out.
> So MMC_CAP_NEEDS_POLL is still needed for this card reader.
No, that won't work as the mmc core will then try to activate the host
to run an initialization sequence on regular intervals. This is what
you want to avoid.
Instead we want the mmc host to signal to the mmc core when it shall
run an initialization sequence to try to find a card.
>
>>
>> [...]
>>
>>> Thanks, I'll send a patch based on your suggestion.
>>
>>
>> Hold on. Using pm_suspend_ignore_children() is not the correct
>> solution to this problem, now that I understand more.
>
>
> But looks like the parent (rtsx_usb) is the one who controls power
> management.
For sure.
> rtsx_usb_suspend() prevents the device from gets runtime suspended when
> there's a card.
I guess rtsx_usb_suspend() may be used for system suspend as well,
hence why this is needed.
For runtime PM, the driver for rtsx_sdmmc platform device should only
need to call pm_runtime_get() to avoid the parent (rtsx_usb) from
being runtime suspended, thus keeping the power to the card. At least
this is my understanding.
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
