linux-kernel - Re: [PATCH] mmc: dw_mmc: rockchip: Keep controller working for card detect

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAPDyKFonhL3JwN1TDrXW-0=VgZowBqTpHsi99GSRmW+aN+4ELQ@mail.gmail.com>
Date: Fri, 1 Nov 2024 12:04:13 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Kever Yang <kever.yang@...k-chips.com>
Cc: Robin Murphy <robin.murphy@....com>, Heiko Stübner <heiko@...ech.de>, 
	linux-rockchip@...ts.infradead.org, Jaehoon Chung <jh80.chung@...sung.com>, 
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	linux-mmc@...r.kernel.org
Subject: Re: [PATCH] mmc: dw_mmc: rockchip: Keep controller working for card detect

On Fri, 1 Nov 2024 at 08:59, Kever Yang <kever.yang@...k-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@....com> wrote:
> >> On 02/10/2024 10:55 pm, Ulf Hansson wrote:
> >>> On Sat, 14 Sept 2024 at 13:52, Heiko Stübner <heiko@...ech.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@...k-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