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