RE: Regarding SDHI clocks on RZ/G2L

[Biju Das <biju.das.jz@bp.renesas.com>]

Hi Chris,

> Subject: RE: Regarding SDHI clocks on RZ/G2L
> 
> Hi Biju,
> 
> > But on the SDHI driver, we are handing cd clocks for RZ/A devices.
> > Unfortunately we are turning of this clock during suspend state.
> 
> I think it is fine to shut off the cd clock in a suspend state because
> plug/unplug a SD card is not really something that should wake up a
> system.

Some of the Mobile SoC's, SDHI is  a wakeup source for deep sleep. 
For that use case, We need to configure CD/WP pin as GPIO rather than function.
So looks like we don't have any use case like that here.


> SHDI should not be a 'wake up' event source in a system, so we don't need
> it running during a suspend state.

Agreed, if cd pin is configured as function. But for some use case It can be a wakeup source,
If it is configured as GPIO.

Run time PM enabled is in SDHI. So it will turn off both the clocks during suspend.

RZ/G2L document says we should not turn off cd clock during suspend. Otherwise card detection
Won't work. So whether SDHI can reliably work in this case? Basically if there is no activity,
this module can go into suspend state. If the cd clock turned off and card detection fails
during resume, how the SDHI functionality going to work? 


> Things like interrupt pins and GPIO are wake up triggers.
> 
> From a power perspective, I don't think it will save much power to turn
> all the clocks off. So your #1 or #2 would be fine.
> 
> As for #2, it's not really a 'critical clock' for the system.....it is
> more of a 'clock we don't care about'.

OK.

> I would say go with the one that is the easiest and makes the least code
> changes. If adding these to the SDHI driver make it very ugly, 

Easiest is to handle it in SDHI driver. May be to address "cd" clock for RZ/G2L SoC
we need special handling for suspend/resume callbacks.

@Wolfram Sang, What do you think?


Note:-
Currently I am configuring CD pin as gpio rather than function due to some reset issue[1]. After reset
cd detection fails. But it works ok, if it configured as GPIO.

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/mmc/host/renesas_sdhi_core.c?h=v5.14-rc2#n580

Cheers,
Biju


> should just add them as critical clock and be done with it.





> > -----Original Message-----
> > From: Biju Das
> > Sent: Sunday, July 18, 2021 6:26 AM
> > To: Wolfram Sang <wsa+renesas@sang-engineering.com>; Geert
> > Uytterhoeven <geert+renesas@glider.be>; Linux-Renesas
> > <linux-renesas-soc@vger.kernel.org>
> > Cc: Phil Edworthy <phil.edworthy@renesas.com>; Chris Brandt
> > <Chris.Brandt@renesas.com>; Chris Paterson
> > <Chris.Paterson2@renesas.com>
> > Subject: Regarding SDHI clocks on RZ/G2L
> >
> > Hi All,
> >
> > As per the RZ/G2L clock list document, SDHI has 4 clocks and  IMCLK2
> > should not be turn off during suspend.
> >
> > 1)IMCLK  Main Clock 1
> > 2)IMCLK2 Main Clock 2, this clk should be not be turned off during
> > suspend state, otherwise card detection won't work.
> > 3)CLK_HS High speed clock
> > 4)ACLK	Bus clock
> >
> > Multi clock PM domain support for SDHI is available in RZ/G2L and we
> > are filtering this clock not to add PM domain.
> >
> > But on the SDHI driver, we are handing cd clocks for RZ/A devices.
> > Unfortunately we are turning of this clock during suspend state.
> >
> > Q1) Is it expected behaviour for this device?
> >
> > Q2)What is the best way to handle cd clocks for RZ/G2L?
> >
> > 1) Handle it in SDHI driver? ie, enable it during probe only once and
> > avoid turning it off
> >
> > or
> >
> > 2) Add this clock as critical clock, so it will be turned on
> > permanently and don't handle it in SDHI driver?
> >
> >
> > Regards,
> > Biju