[PATCH] mmc: dw_mmc: rockchip: Keep controller working for card detect

[Ulf Hansson]

On Fri, 1 Nov 2024 at 08:59, Kever Yang <kever.yang at rock-chips.com> wrote:
>
> Hi Ulf, Robin, Heiko,
>
> On 2024/10/7 17:49, Ulf Hansson wrote:
> > On Fri, 4 Oct 2024 at 19:34, Robin Murphy <robin.murphy at arm.com> wrote:
> >> On 02/10/2024 10:55 pm, Ulf Hansson wrote:
> >>> On Sat, 14 Sept 2024 at 13:52, Heiko Stübner <heiko at sntech.de> wrote:
> >>>> Am Donnerstag, 12. September 2024, 09:26:14 CEST schrieb Kever Yang:
> >>>>> In order to make the SD card hotplug working we need the card detect
> >>>>> function logic inside the controller always working. The runtime PM will
> >>>>> gate the clock and the power domain, which stops controller working when
> >>>>> no data transfer happen.
> >>>>>
> >>>>> So lets skip enable runtime PM when the card needs to detected by the
> >>>>> controller and the card is removable.
> >>>>>
> >>>>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
> >>>> So for the change itself this looks good, i.e. it fixes an issue for baords relying
> >>>> on the on-chip-card-detect.
> >>>>
> >>>>
> >>>> But for boards doing that, the controller will be running _all the time_
> >>>> even if there is never any card inserted.
> >>>>
> >>>> So relying on the on-soc card-detect will effectively increase the power-
> >>>> consumption of the board - even it it'll never use any sd-card?
> Yes, this is how the controller works, the controller needs the clock to
> make the detect logic work.
> If we use gpio to implement this card-detect, it works because the GPIO
> controller/clock keeps working.

Right. On embedded battery driven platforms it's quite common that
there is some always-on logic (maybe via a PMIC) that helps to take
care of these GPIO irqs.

> For the dw_mmc driver support, we should support both kind of implement
> due to the controller has this function,
> so this patch is for the card-detect implement by the dwmmc controller,
> the controller need to keep working
> - only for sd-card (so not include the "non-removable " device)
> - also not disable rpm when "cd-gpios" is used.
>
> For the power consumption, I believe it will increase, but very very
> small, we can't even monitor the change

Right, that may be perfectly correct on the platform you are using. On
others it may not.

In general, it's a bad idea to keep devices runtime resumed, unless we
really have too. In particular when the are shared power-rails being
managed by a power-domain, for example.

> if we use the normal equipment. The driver should make function works
> first, and then consider the power.
>
> This patch is to make the dwmmc function works without gpio's help in
> dwmmc driver,
>   and has no affect to the gpio option, people still able to use gpio to
> do the cd.
>

I understand and you certainly have a point.

However, it sounds like you think there is a drawback involved to use
MMC_CAP_NEEDS_POLL in this case? In that case, can you please
elaborate why preventing runtime suspend would be better?

[...]

Kind regards
Uffe