[PATCH] ARM: dts: r7s72100: fix sdhi clock define

[PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Now that all the clocks in the boot loader are disabled before booting
the kernel, and the mstp driver has been fixed for RZ/A1, here is a typo
that was missed during original testing.

Fixes: 7c8522b7047c ("ARM: dts: r7s72100: add sdhi clock to device tree")
Signed-off-by: Chris Brandt <chris.brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
---
 include/dt-bindings/clock/r7s72100-clock.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/dt-bindings/clock/r7s72100-clock.h b/include/dt-bindings/clock/r7s72100-clock.h
index 29e01ed..318ab14 100644
--- a/include/dt-bindings/clock/r7s72100-clock.h
+++ b/include/dt-bindings/clock/r7s72100-clock.h
@@ -46,6 +46,6 @@
 
 /* MSTP12 */
 #define R7S72100_CLK_SDHI0	3
-#define R7S72100_CLK_SDHI1	2
+#define R7S72100_CLK_SDHI1	1
 
 #endif /* __DT_BINDINGS_CLOCK_R7S72100_H__ */
-- 
2.10.1


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Now that all the clocks in the boot loader are disabled before booting
the kernel, and the mstp driver has been fixed for RZ/A1, here is a typo
that was missed during original testing.

Fixes: 7c8522b7047c ("ARM: dts: r7s72100: add sdhi clock to device tree")
Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 include/dt-bindings/clock/r7s72100-clock.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/dt-bindings/clock/r7s72100-clock.h b/include/dt-bindings/clock/r7s72100-clock.h
index 29e01ed..318ab14 100644
--- a/include/dt-bindings/clock/r7s72100-clock.h
+++ b/include/dt-bindings/clock/r7s72100-clock.h
@@ -46,6 +46,6 @@
 
 /* MSTP12 */
 #define R7S72100_CLK_SDHI0	3
-#define R7S72100_CLK_SDHI1	2
+#define R7S72100_CLK_SDHI1	1
 
 #endif /* __DT_BINDINGS_CLOCK_R7S72100_H__ */
-- 
2.10.1

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Chris,

On Thu, Jan 12, 2017 at 7:11 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> Now that all the clocks in the boot loader are disabled before booting
> the kernel, and the mstp driver has been fixed for RZ/A1, here is a typo
> that was missed during original testing.
>
> Fixes: 7c8522b7047c ("ARM: dts: r7s72100: add sdhi clock to device tree")
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  include/dt-bindings/clock/r7s72100-clock.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/dt-bindings/clock/r7s72100-clock.h b/include/dt-bindings/clock/r7s72100-clock.h
> index 29e01ed..318ab14 100644
> --- a/include/dt-bindings/clock/r7s72100-clock.h
> +++ b/include/dt-bindings/clock/r7s72100-clock.h
> @@ -46,6 +46,6 @@
>
>  /* MSTP12 */
>  #define R7S72100_CLK_SDHI0     3
> -#define R7S72100_CLK_SDHI1     2
> +#define R7S72100_CLK_SDHI1     1

This is strange. There are two SDHI channels, but the STBCR12 documentation
(all versions up to rev. 3.00) says the register has MSTP bits for four SD
host interfaces?

Can you please enlighten me? Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Geert,

On Thursday, January 12, 2017, Geert Uytterhoeven wrote:
> This is strange. There are two SDHI channels, but the STBCR12
> documentation (all versions up to rev. 3.00) says the register has MSTP
> bits for four SD host interfaces?
> 
> Can you please enlighten me? Thanks!

Ya, I saw that. There are 2 bits per SDHI channel. I did check and just
enabling the one works fine.

Honestly, I'm not sure why there are two clock enables.

I'll go back and ask the design team if they can tell me why there are 2.

As I said, I just re-tested and it works fine, but you can hold off on the
patch if you want until I come up with a real explanation.


Chris

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Geert,

On Thursday, January 12, 2017, Geert Uytterhoeven wrote:
> This is strange. There are two SDHI channels, but the STBCR12
> documentation (all versions up to rev. 3.00) says the register has MSTP
> bits for four SD host interfaces?
> 
> Can you please enlighten me? Thanks!

Ya, I saw that. There are 2 bits per SDHI channel. I did check and just
enabling the one works fine.

Honestly, I'm not sure why there are two clock enables.

I'll go back and ask the design team if they can tell me why there are 2.

As I said, I just re-tested and it works fine, but you can hold off on the
patch if you want until I come up with a real explanation.


Chris

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Simon Horman]

On Thu, Jan 12, 2017 at 08:34:26PM +0000, Chris Brandt wrote:
> Hi Geert,
> 
> On Thursday, January 12, 2017, Geert Uytterhoeven wrote:
> > This is strange. There are two SDHI channels, but the STBCR12
> > documentation (all versions up to rev. 3.00) says the register has MSTP
> > bits for four SD host interfaces?
> > 
> > Can you please enlighten me? Thanks!
> 
> Ya, I saw that. There are 2 bits per SDHI channel. I did check and just
> enabling the one works fine.
> 
> Honestly, I'm not sure why there are two clock enables.
> 
> I'll go back and ask the design team if they can tell me why there are 2.
> 
> As I said, I just re-tested and it works fine, but you can hold off on the
> patch if you want until I come up with a real explanation.

I'd prefer to hold off on this for now.

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2483 bytes --]

Hi Geert,

On Thursday, January 12, 2017, Geert Uytterhoeven wrote:
> This is strange. There are two SDHI channels, but the STBCR12
> documentation (all versions up to rev. 3.00) says the register has MSTP
> bits for four SD host interfaces?
> 
> Can you please enlighten me? Thanks!

If you look in the rev 3.00 manual for RZ/A1H, in section
"50.3.2 Card Detect/Write Protect", it says:


* Power-Down mode at Card removal
  SD Host Interface module is halted by the MSTP123 bit to MSTP120 bit in Standby Control Register 12
  (STBCR12). If these bits of each channel in STBCR12 are set to 10, low power consumption is achieved at Card
  removal. See section 55, Power-Down Modes


So, there are 2 clock sources for each SDHI channel, and the setting
Options are:
  SD Host Interface 0 Module Stop
  00: SD Host Interface 0 Module runs.
  01: Setting prohibited.
  10: Only card detect block in SD Host Interface 0 Module runs.
  11: Clock supply to SD Host Interface 0 Module is halted

Previously I've been running with '00' because I was turning EVERYTHING on
in u-boot.
Yesterday, I tested with just '10' and things seem to work OK
for me.

Since the sh_mobile_sdhi/tmio driver only allows for 1 clock per channel,
'10' is my only choice for the DT.

However, looking at a previous BSP for RZ/A1, the driver 
arch/arm/mach-shmobile/clock-r7s72100.c had this:


static struct clk mstp_clks[MSTP_NR] = {
        [MSTP123] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 3, 0), /* SDHI00 */
        [MSTP122] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 2,
                        CLK_ENABLE_ON_INIT),                       /* SDHI01 */
        [MSTP121] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 1, 0), /* SDHI10 */
        [MSTP120] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 0,
                CLK_ENABLE_ON_INIT),                               /* SDHI11 */


But...that would make me think on boot it would be set to '01' (setting prohibited).


I'm going to try and find if "setting prohibited" really just means:
"you can set it this way...but nothing is really going to work unless you
enable the other clock".

If that is the case, is there a DT equivalent to "CLK_ENABLE_ON_INIT" that
I can do for MSTP120(SDHI11) and MSTP120(SDHI01) so they are both cleared
on boot???


Thanks,
Chris
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Geert,

On Thursday, January 12, 2017, Geert Uytterhoeven wrote:
> This is strange. There are two SDHI channels, but the STBCR12
> documentation (all versions up to rev. 3.00) says the register has MSTP
> bits for four SD host interfaces?
> 
> Can you please enlighten me? Thanks!

If you look in the rev 3.00 manual for RZ/A1H, in section
"50.3.2 Card Detect/Write Protect", it says:


* Power-Down mode at Card removal
  SD Host Interface module is halted by the MSTP123 bit to MSTP120 bit in Standby Control Register 12
  (STBCR12). If these bits of each channel in STBCR12 are set to 10, low power consumption is achieved at Card
  removal. See section 55, Power-Down Modes


So, there are 2 clock sources for each SDHI channel, and the setting
Options are:
  SD Host Interface 0 Module Stop
  00: SD Host Interface 0 Module runs.
  01: Setting prohibited.
  10: Only card detect block in SD Host Interface 0 Module runs.
  11: Clock supply to SD Host Interface 0 Module is halted

Previously I've been running with '00' because I was turning EVERYTHING on
in u-boot.
Yesterday, I tested with just '10' and things seem to work OK
for me.

Since the sh_mobile_sdhi/tmio driver only allows for 1 clock per channel,
'10' is my only choice for the DT.

However, looking at a previous BSP for RZ/A1, the driver 
arch/arm/mach-shmobile/clock-r7s72100.c had this:


static struct clk mstp_clks[MSTP_NR] = {
        [MSTP123] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 3, 0), /* SDHI00 */
        [MSTP122] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 2,
                        CLK_ENABLE_ON_INIT),                       /* SDHI01 */
        [MSTP121] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 1, 0), /* SDHI10 */
        [MSTP120] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 0,
                CLK_ENABLE_ON_INIT),                               /* SDHI11 */


But...that would make me think on boot it would be set to '01' (setting prohibited).


I'm going to try and find if "setting prohibited" really just means:
"you can set it this way...but nothing is really going to work unless you
enable the other clock".

If that is the case, is there a DT equivalent to "CLK_ENABLE_ON_INIT" that
I can do for MSTP120(SDHI11) and MSTP120(SDHI01) so they are both cleared
on boot???


Thanks,
Chris

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Chris,

On Fri, Jan 13, 2017 at 3:44 PM, Chris Brandt <Chris.Brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> On Thursday, January 12, 2017, Geert Uytterhoeven wrote:
>> This is strange. There are two SDHI channels, but the STBCR12
>> documentation (all versions up to rev. 3.00) says the register has MSTP
>> bits for four SD host interfaces?
>>
>> Can you please enlighten me? Thanks!
>
> If you look in the rev 3.00 manual for RZ/A1H, in section
> "50.3.2 Card Detect/Write Protect", it says:
>
> * Power-Down mode at Card removal
>   SD Host Interface module is halted by the MSTP123 bit to MSTP120 bit in Standby Control Register 12
>   (STBCR12). If these bits of each channel in STBCR12 are set to 10, low power consumption is achieved at Card
>   removal. See section 55, Power-Down Modes
>
>
> So, there are 2 clock sources for each SDHI channel, and the setting
> Options are:
>   SD Host Interface 0 Module Stop
>   00: SD Host Interface 0 Module runs.
>   01: Setting prohibited.
>   10: Only card detect block in SD Host Interface 0 Module runs.
>   11: Clock supply to SD Host Interface 0 Module is halted

So typically you want to use 10 when idle, and 00 when active.

> Previously I've been running with '00' because I was turning EVERYTHING on
> in u-boot.
> Yesterday, I tested with just '10' and things seem to work OK
> for me.
>
> Since the sh_mobile_sdhi/tmio driver only allows for 1 clock per channel,
> '10' is my only choice for the DT.
>
> However, looking at a previous BSP for RZ/A1, the driver
> arch/arm/mach-shmobile/clock-r7s72100.c had this:
>
>
> static struct clk mstp_clks[MSTP_NR] = {
>         [MSTP123] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 3, 0), /* SDHI00 */
>         [MSTP122] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 2,
>                         CLK_ENABLE_ON_INIT),                       /* SDHI01 */
>         [MSTP121] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 1, 0), /* SDHI10 */
>         [MSTP120] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 0,
>                 CLK_ENABLE_ON_INIT),                               /* SDHI11 */
>
>
> But...that would make me think on boot it would be set to '01' (setting prohibited).

Yeah, running with enabled SDHI core and disabled card detect sounds silly.

> I'm going to try and find if "setting prohibited" really just means:
> "you can set it this way...but nothing is really going to work unless you
> enable the other clock".
>
> If that is the case, is there a DT equivalent to "CLK_ENABLE_ON_INIT" that
> I can do for MSTP120(SDHI11) and MSTP120(SDHI01) so they are both cleared
> on boot???

No there isn't. That's another reason why a full-fledged clock driver with
tables in C is a better idea than trying to describe all clocks in DT.
The new CPG/MSSR based driver (renesas-cpg-mssr.c) supports "critical
module clocks" through CLK_ENABLE_HAND_OFF. Unfortunately that flag hasn't
made it upstream, so I really should convert the driver to use the new
CLK_IS_CRITICAL instead...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Chris,

On Fri, Jan 13, 2017 at 3:44 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Thursday, January 12, 2017, Geert Uytterhoeven wrote:
>> This is strange. There are two SDHI channels, but the STBCR12
>> documentation (all versions up to rev. 3.00) says the register has MSTP
>> bits for four SD host interfaces?
>>
>> Can you please enlighten me? Thanks!
>
> If you look in the rev 3.00 manual for RZ/A1H, in section
> "50.3.2 Card Detect/Write Protect", it says:
>
> * Power-Down mode at Card removal
>   SD Host Interface module is halted by the MSTP123 bit to MSTP120 bit in Standby Control Register 12
>   (STBCR12). If these bits of each channel in STBCR12 are set to 10, low power consumption is achieved at Card
>   removal. See section 55, Power-Down Modes
>
>
> So, there are 2 clock sources for each SDHI channel, and the setting
> Options are:
>   SD Host Interface 0 Module Stop
>   00: SD Host Interface 0 Module runs.
>   01: Setting prohibited.
>   10: Only card detect block in SD Host Interface 0 Module runs.
>   11: Clock supply to SD Host Interface 0 Module is halted

So typically you want to use 10 when idle, and 00 when active.

> Previously I've been running with '00' because I was turning EVERYTHING on
> in u-boot.
> Yesterday, I tested with just '10' and things seem to work OK
> for me.
>
> Since the sh_mobile_sdhi/tmio driver only allows for 1 clock per channel,
> '10' is my only choice for the DT.
>
> However, looking at a previous BSP for RZ/A1, the driver
> arch/arm/mach-shmobile/clock-r7s72100.c had this:
>
>
> static struct clk mstp_clks[MSTP_NR] = {
>         [MSTP123] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 3, 0), /* SDHI00 */
>         [MSTP122] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 2,
>                         CLK_ENABLE_ON_INIT),                       /* SDHI01 */
>         [MSTP121] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 1, 0), /* SDHI10 */
>         [MSTP120] = SH_CLK_MSTP8(&peripheral1_clk, STBCR12, 0,
>                 CLK_ENABLE_ON_INIT),                               /* SDHI11 */
>
>
> But...that would make me think on boot it would be set to '01' (setting prohibited).

Yeah, running with enabled SDHI core and disabled card detect sounds silly.

> I'm going to try and find if "setting prohibited" really just means:
> "you can set it this way...but nothing is really going to work unless you
> enable the other clock".
>
> If that is the case, is there a DT equivalent to "CLK_ENABLE_ON_INIT" that
> I can do for MSTP120(SDHI11) and MSTP120(SDHI01) so they are both cleared
> on boot???

No there isn't. That's another reason why a full-fledged clock driver with
tables in C is a better idea than trying to describe all clocks in DT.
The new CPG/MSSR based driver (renesas-cpg-mssr.c) supports "critical
module clocks" through CLK_ENABLE_HAND_OFF. Unfortunately that flag hasn't
made it upstream, so I really should convert the driver to use the new
CLK_IS_CRITICAL instead...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Geert,


> > But...that would make me think on boot it would be set to '01' (setting prohibited).
> 
> Yeah, running with enabled SDHI core and disabled card detect sounds silly.

I just did some testing and with only 1 clock enabled ('01'), the core works but
card detect doesn't work. If you boot with the card in, you can read it fine, but
if you pull it...no removal is detected.
As soon as I turn the other clock on ('00'), card detect magically starts working.

So from my experiments:

  00: SD Host Interface 1 Module runs.
      >> everything works

  01: Setting prohibited.
      >> core runs, but no card detect

  10: Only card detect block in SD Host Interface 1 Module runs.
      >> SDHI address space all 00s (core not running)

  11: Clock supply to SD Host Interface 1 Module is halted
      >> nothing works (of course)


> So typically you want to use 10 when idle, and 00 when active.

I wonder if that would mean the system would get a CD interrupt. Of course
if no ISR registered (SDHI not enabled), the kernel would throw it away.


> No there isn't. That's another reason why a full-fledged clock driver with
> tables in C is a better idea than trying to describe all clocks in DT.
> The new CPG/MSSR based driver (renesas-cpg-mssr.c) supports "critical
> module clocks" through CLK_ENABLE_HAND_OFF. Unfortunately that flag hasn't
> made it upstream, so I really should convert the driver to use the new
> CLK_IS_CRITICAL instead...

So basically at the moment you're tell me it's going to stay broke (unless it's
enabled in u-boot).


In sh_mobile_sdhi.c, can we change sh_mobile_sdhi_probe() so that if there
are 2 clocks specified (in DT or platform data), it automatically enables
the 2nd clock (forever) and just uses the 1st clock as the on/off clock?



Chris

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Geert,


> > But...that would make me think on boot it would be set to '01' (setting prohibited).
> 
> Yeah, running with enabled SDHI core and disabled card detect sounds silly.

I just did some testing and with only 1 clock enabled ('01'), the core works but
card detect doesn't work. If you boot with the card in, you can read it fine, but
if you pull it...no removal is detected.
As soon as I turn the other clock on ('00'), card detect magically starts working.

So from my experiments:

  00: SD Host Interface 1 Module runs.
      >> everything works

  01: Setting prohibited.
      >> core runs, but no card detect

  10: Only card detect block in SD Host Interface 1 Module runs.
      >> SDHI address space all 00s (core not running)

  11: Clock supply to SD Host Interface 1 Module is halted
      >> nothing works (of course)


> So typically you want to use 10 when idle, and 00 when active.

I wonder if that would mean the system would get a CD interrupt. Of course
if no ISR registered (SDHI not enabled), the kernel would throw it away.


> No there isn't. That's another reason why a full-fledged clock driver with
> tables in C is a better idea than trying to describe all clocks in DT.
> The new CPG/MSSR based driver (renesas-cpg-mssr.c) supports "critical
> module clocks" through CLK_ENABLE_HAND_OFF. Unfortunately that flag hasn't
> made it upstream, so I really should convert the driver to use the new
> CLK_IS_CRITICAL instead...

So basically at the moment you're tell me it's going to stay broke (unless it's
enabled in u-boot).


In sh_mobile_sdhi.c, can we change sh_mobile_sdhi_probe() so that if there
are 2 clocks specified (in DT or platform data), it automatically enables
the 2nd clock (forever) and just uses the 1st clock as the on/off clock?



Chris

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Chris,

On Fri, Jan 13, 2017 at 6:16 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
>> > But...that would make me think on boot it would be set to '01' (setting prohibited).
>>
>> Yeah, running with enabled SDHI core and disabled card detect sounds silly.
>
> I just did some testing and with only 1 clock enabled ('01'), the core works but
> card detect doesn't work. If you boot with the card in, you can read it fine, but
> if you pull it...no removal is detected.
> As soon as I turn the other clock on ('00'), card detect magically starts working.
>
> So from my experiments:
>
>   00: SD Host Interface 1 Module runs.
>       >> everything works
>
>   01: Setting prohibited.
>       >> core runs, but no card detect
>
>   10: Only card detect block in SD Host Interface 1 Module runs.
>       >> SDHI address space all 00s (core not running)
>
>   11: Clock supply to SD Host Interface 1 Module is halted
>       >> nothing works (of course)
>
>
>> So typically you want to use 10 when idle, and 00 when active.
>
> I wonder if that would mean the system would get a CD interrupt. Of course
> if no ISR registered (SDHI not enabled), the kernel would throw it away.
>
>
>> No there isn't. That's another reason why a full-fledged clock driver with
>> tables in C is a better idea than trying to describe all clocks in DT.
>> The new CPG/MSSR based driver (renesas-cpg-mssr.c) supports "critical
>> module clocks" through CLK_ENABLE_HAND_OFF. Unfortunately that flag hasn't
>> made it upstream, so I really should convert the driver to use the new
>> CLK_IS_CRITICAL instead...
>
> So basically at the moment you're tell me it's going to stay broke (unless it's
> enabled in u-boot).
>
> In sh_mobile_sdhi.c, can we change sh_mobile_sdhi_probe() so that if there
> are 2 clocks specified (in DT or platform data), it automatically enables
> the 2nd clock (forever) and just uses the 1st clock as the on/off clock?

Of course the driver can handle the second interrupt, if you update the
binding, and add support code for that...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Geert,

On Friday, January 13, 2017, Geert Uytterhoeven wrote:
> > In sh_mobile_sdhi.c, can we change sh_mobile_sdhi_probe() so that if
> > there are 2 clocks specified (in DT or platform data), it
> > automatically enables the 2nd clock (forever) and just uses the 1st
> clock as the on/off clock?
> 
> Of course the driver can handle the second interrupt, if you update the
> binding, and add support code for that...

Of course my idea is that I would only have to update the bindings for
RZ/A1...not any other device.

My only question is, today sh_mobile_sdhi.c uses this:

   priv->clk = devm_clk_get(&pdev->dev, NULL);

to get the clock. But if there is a 2nd clock...how do I know the
string id name to look to replace NULL with??


Or...for the RZ/A1 dtsi, should I just give the 2 clocks names:

		clocks = <&mstp12_clks R7S72100_CLK_SDHI00,
				&mstp12_clks R7S72100_CLK_SDHI01>;
		clock-names = "core", "cd";


and then in the code do:

	struct *cd_clk;
	cd_clk = devm_clk_get(&pdev->dev, "cd");
	if (cd_clk) {
		clk_prepare_enable(cd_clk);
	}



  (this simple 1-line fix patch is getting a lot more complicated)


Thanks,
Chris

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Geert,

On Friday, January 13, 2017, Geert Uytterhoeven wrote:
> > In sh_mobile_sdhi.c, can we change sh_mobile_sdhi_probe() so that if
> > there are 2 clocks specified (in DT or platform data), it
> > automatically enables the 2nd clock (forever) and just uses the 1st
> clock as the on/off clock?
> 
> Of course the driver can handle the second interrupt, if you update the
> binding, and add support code for that...

Of course my idea is that I would only have to update the bindings for
RZ/A1...not any other device.

My only question is, today sh_mobile_sdhi.c uses this:

   priv->clk = devm_clk_get(&pdev->dev, NULL);

to get the clock. But if there is a 2nd clock...how do I know the
string id name to look to replace NULL with??


Or...for the RZ/A1 dtsi, should I just give the 2 clocks names:

		clocks = <&mstp12_clks R7S72100_CLK_SDHI00,
				&mstp12_clks R7S72100_CLK_SDHI01>;
		clock-names = "core", "cd";


and then in the code do:

	struct *cd_clk;
	cd_clk = devm_clk_get(&pdev->dev, "cd");
	if (cd_clk) {
		clk_prepare_enable(cd_clk);
	}



  (this simple 1-line fix patch is getting a lot more complicated)


Thanks,
Chris

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Chris,

CC Woflram

On Fri, Jan 13, 2017 at 6:56 PM, Chris Brandt <Chris.Brandt-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org> wrote:
> On Friday, January 13, 2017, Geert Uytterhoeven wrote:
>> > In sh_mobile_sdhi.c, can we change sh_mobile_sdhi_probe() so that if
>> > there are 2 clocks specified (in DT or platform data), it
>> > automatically enables the 2nd clock (forever) and just uses the 1st
>> clock as the on/off clock?
>>
>> Of course the driver can handle the second interrupt, if you update the
>> binding, and add support code for that...
>
> Of course my idea is that I would only have to update the bindings for
> RZ/A1...not any other device.
>
> My only question is, today sh_mobile_sdhi.c uses this:
>
>    priv->clk = devm_clk_get(&pdev->dev, NULL);
>
> to get the clock. But if there is a 2nd clock...how do I know the
> string id name to look to replace NULL with??

That should be specified in the bindings. "NULL" will get you the first clock.

> Or...for the RZ/A1 dtsi, should I just give the 2 clocks names:
>
>                 clocks = <&mstp12_clks R7S72100_CLK_SDHI00,
>                                 &mstp12_clks R7S72100_CLK_SDHI01>;
>                 clock-names = "core", "cd";

Something like that.
If devm_clk_get(&pdev->dev, "core") fails, you can assume a single clock
and retry with NULL.

> and then in the code do:
>
>         struct *cd_clk;
>         cd_clk = devm_clk_get(&pdev->dev, "cd");
>         if (cd_clk) {
>                 clk_prepare_enable(cd_clk);
>         }
>
>   (this simple 1-line fix patch is getting a lot more complicated)

Disclaimer: I don't know how/if the SDHI core manages clocks, and may
interfere. Adding Wolfram.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Chris,

CC Woflram

On Fri, Jan 13, 2017 at 6:56 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Friday, January 13, 2017, Geert Uytterhoeven wrote:
>> > In sh_mobile_sdhi.c, can we change sh_mobile_sdhi_probe() so that if
>> > there are 2 clocks specified (in DT or platform data), it
>> > automatically enables the 2nd clock (forever) and just uses the 1st
>> clock as the on/off clock?
>>
>> Of course the driver can handle the second interrupt, if you update the
>> binding, and add support code for that...
>
> Of course my idea is that I would only have to update the bindings for
> RZ/A1...not any other device.
>
> My only question is, today sh_mobile_sdhi.c uses this:
>
>    priv->clk = devm_clk_get(&pdev->dev, NULL);
>
> to get the clock. But if there is a 2nd clock...how do I know the
> string id name to look to replace NULL with??

That should be specified in the bindings. "NULL" will get you the first clock.

> Or...for the RZ/A1 dtsi, should I just give the 2 clocks names:
>
>                 clocks = <&mstp12_clks R7S72100_CLK_SDHI00,
>                                 &mstp12_clks R7S72100_CLK_SDHI01>;
>                 clock-names = "core", "cd";

Something like that.
If devm_clk_get(&pdev->dev, "core") fails, you can assume a single clock
and retry with NULL.

> and then in the code do:
>
>         struct *cd_clk;
>         cd_clk = devm_clk_get(&pdev->dev, "cd");
>         if (cd_clk) {
>                 clk_prepare_enable(cd_clk);
>         }
>
>   (this simple 1-line fix patch is getting a lot more complicated)

Disclaimer: I don't know how/if the SDHI core manages clocks, and may
interfere. Adding Wolfram.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]


> > and then in the code do:
> >
> >         struct *cd_clk;
> >         cd_clk = devm_clk_get(&pdev->dev, "cd");
> >         if (cd_clk) {
> >                 clk_prepare_enable(cd_clk);
> >         }
> >
> >   (this simple 1-line fix patch is getting a lot more complicated)
> 
> Disclaimer: I don't know how/if the SDHI core manages clocks, and may
> interfere. Adding Wolfram.

Thanks for the heads up.

We have special callbacks for en-/disabling clocks:
sh_mobile_sdhi_clk_enable() and sh_mobile_sdhi_clk_disable().

I think those functions should get the above if-blocks (without curly
braces) to ensure we always have consistent 00 or 11 settings.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Wolfram and Geert,

On Monday, January 16, 2017, Wolfram Sang:
> > > and then in the code do:
> > >
> > >         struct *cd_clk;
> > >         cd_clk = devm_clk_get(&pdev->dev, "cd");
> > >         if (cd_clk) {
> > >                 clk_prepare_enable(cd_clk);
> > >         }
> > >
> > >   (this simple 1-line fix patch is getting a lot more complicated)
> >
> > Disclaimer: I don't know how/if the SDHI core manages clocks, and may
> > interfere. Adding Wolfram.
> 
> Thanks for the heads up.
> 
> We have special callbacks for en-/disabling clocks:
> sh_mobile_sdhi_clk_enable() and sh_mobile_sdhi_clk_disable().
> 
> I think those functions should get the above if-blocks (without curly
> braces) to ensure we always have consistent 00 or 11 settings.


Well....
I was going to try and cheat and in the probe, enable both the core
and card-detect clocks, but, not save the card-detect clock pointer.
Basically, once it is turned on, it is never turned off (since it was intended
to be used for low power operation anyway).

The reason is that would then keep me from having to modify the existing
functions sh_mobile_sdhi_clk_enable/disable.

Is anyone going to have an issue if I turn the card-detect clock on but never
turn it off???? That was the patch that I was going to test out and submit.



Chris

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Wolfram and Geert,

On Monday, January 16, 2017, Wolfram Sang:
> > > and then in the code do:
> > >
> > >         struct *cd_clk;
> > >         cd_clk = devm_clk_get(&pdev->dev, "cd");
> > >         if (cd_clk) {
> > >                 clk_prepare_enable(cd_clk);
> > >         }
> > >
> > >   (this simple 1-line fix patch is getting a lot more complicated)
> >
> > Disclaimer: I don't know how/if the SDHI core manages clocks, and may
> > interfere. Adding Wolfram.
> 
> Thanks for the heads up.
> 
> We have special callbacks for en-/disabling clocks:
> sh_mobile_sdhi_clk_enable() and sh_mobile_sdhi_clk_disable().
> 
> I think those functions should get the above if-blocks (without curly
> braces) to ensure we always have consistent 00 or 11 settings.


Well....
I was going to try and cheat and in the probe, enable both the core
and card-detect clocks, but, not save the card-detect clock pointer.
Basically, once it is turned on, it is never turned off (since it was intended
to be used for low power operation anyway).

The reason is that would then keep me from having to modify the existing
functions sh_mobile_sdhi_clk_enable/disable.

Is anyone going to have an issue if I turn the card-detect clock on but never
turn it off???? That was the patch that I was going to test out and submit.



Chris

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]


> The reason is that would then keep me from having to modify the existing
> functions sh_mobile_sdhi_clk_enable/disable.

Why do you prefer this? I may be missing something but a small if-block
per function are not expensive IMO.

> Is anyone going to have an issue if I turn the card-detect clock on but never
> turn it off???? That was the patch that I was going to test out and submit.

It smells a bit hacky to me. And while hacky things are sometimes
needed, IMO this doesn't hold true here. With just 3 more lines (an
if-block in disable and a variable for the new clock) we can have it
proper.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]


> The reason is that would then keep me from having to modify the existing
> functions sh_mobile_sdhi_clk_enable/disable.

Why do you prefer this? I may be missing something but a small if-block
per function are not expensive IMO.

> Is anyone going to have an issue if I turn the card-detect clock on but never
> turn it off???? That was the patch that I was going to test out and submit.

It smells a bit hacky to me. And while hacky things are sometimes
needed, IMO this doesn't hold true here. With just 3 more lines (an
if-block in disable and a variable for the new clock) we can have it
proper.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Jan 17, 2017 at 9:21 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>> The reason is that would then keep me from having to modify the existing
>> functions sh_mobile_sdhi_clk_enable/disable.
>
> Why do you prefer this? I may be missing something but a small if-block
> per function are not expensive IMO.
>
>> Is anyone going to have an issue if I turn the card-detect clock on but never
>> turn it off???? That was the patch that I was going to test out and submit.
>
> It smells a bit hacky to me. And while hacky things are sometimes
> needed, IMO this doesn't hold true here. With just 3 more lines (an
> if-block in disable and a variable for the new clock) we can have it
> proper.

I have no idea if the SDHI driver disables the module clock when it is
idle, but shouldn't the card detect clock be running all the time when the
driver is bound to the device?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Jan 17, 2017 at 9:21 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> The reason is that would then keep me from having to modify the existing
>> functions sh_mobile_sdhi_clk_enable/disable.
>
> Why do you prefer this? I may be missing something but a small if-block
> per function are not expensive IMO.
>
>> Is anyone going to have an issue if I turn the card-detect clock on but never
>> turn it off???? That was the patch that I was going to test out and submit.
>
> It smells a bit hacky to me. And while hacky things are sometimes
> needed, IMO this doesn't hold true here. With just 3 more lines (an
> if-block in disable and a variable for the new clock) we can have it
> proper.

I have no idea if the SDHI driver disables the module clock when it is
idle, but shouldn't the card detect clock be running all the time when the
driver is bound to the device?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 662 bytes --]


> I have no idea if the SDHI driver disables the module clock when it is
> idle, but shouldn't the card detect clock be running all the time when the
> driver is bound to the device?

Yes, it should. And for all instances with just one clock, this means
this main clock must be running. So, en-/disable functions are all about
suspend/resume and bind/unbind. (Huh, looks like the unbind part is
missing, though. Need to look closer).

My take is: either we implement the power saving and handle the cd clock
seperately. Or we ignore the power saving for now and handle both clocks
as virtually one, i.e. en-/disable them at the same time.

Doesn't make sense?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Jan 17, 2017 at 10:45 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> I have no idea if the SDHI driver disables the module clock when it is
>> idle, but shouldn't the card detect clock be running all the time when the
>> driver is bound to the device?
>
> Yes, it should. And for all instances with just one clock, this means
> this main clock must be running. So, en-/disable functions are all about
> suspend/resume and bind/unbind. (Huh, looks like the unbind part is
> missing, though. Need to look closer).
>
> My take is: either we implement the power saving and handle the cd clock
> seperately. Or we ignore the power saving for now and handle both clocks
> as virtually one, i.e. en-/disable them at the same time.
>
> Doesn't make sense?

If we handle them as one, won't we miss card detect events due to the
card detect clock being disabled while SDHI is idle?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]


> If we handle them as one, won't we miss card detect events due to the
> card detect clock being disabled while SDHI is idle?

You mean this?

1208         /*
1209          * While using internal tmio hardware logic for card detection, we need
1210          * to ensure it stays powered for it to work.
1211          */
1212         if (_host->native_hotplug)
1213                 pm_runtime_get_noresume(&pdev->dev);



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 422 bytes --]


> If we handle them as one, won't we miss card detect events due to the
> card detect clock being disabled while SDHI is idle?

You mean this?

1208         /*
1209          * While using internal tmio hardware logic for card detection, we need
1210          * to ensure it stays powered for it to work.
1211          */
1212         if (_host->native_hotplug)
1213                 pm_runtime_get_noresume(&pdev->dev);



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Jan 17, 2017 at 10:57 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
>> If we handle them as one, won't we miss card detect events due to the
>> card detect clock being disabled while SDHI is idle?
>
> You mean this?
>
> 1208         /*
> 1209          * While using internal tmio hardware logic for card detection, we need
> 1210          * to ensure it stays powered for it to work.
> 1211          */
> 1212         if (_host->native_hotplug)
> 1213                 pm_runtime_get_noresume(&pdev->dev);

OK. So that will keep the core module clock running.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Jan 17, 2017 at 10:57 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
>> If we handle them as one, won't we miss card detect events due to the
>> card detect clock being disabled while SDHI is idle?
>
> You mean this?
>
> 1208         /*
> 1209          * While using internal tmio hardware logic for card detection, we need
> 1210          * to ensure it stays powered for it to work.
> 1211          */
> 1212         if (_host->native_hotplug)
> 1213                 pm_runtime_get_noresume(&pdev->dev);

OK. So that will keep the core module clock running.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Wolfram,

On Tuesday, January 17, 2017, Wolfram Sang wrote:
> > I have no idea if the SDHI driver disables the module clock when it is
> > idle, but shouldn't the card detect clock be running all the time when
> > the driver is bound to the device?
> 
> Yes, it should. And for all instances with just one clock, this means this
> main clock must be running. So, en-/disable functions are all about
> suspend/resume and bind/unbind. (Huh, looks like the unbind part is
> missing, though. Need to look closer).
> 
> My take is: either we implement the power saving and handle the cd clock
> seperately. Or we ignore the power saving for now and handle both clocks
> as virtually one, i.e. en-/disable them at the same time.
> 
> Doesn't make sense?

I'll go with "ignore the power saving for now and handle both clocks
as virtually one".

I'll go give it a try and see how it works.

Thanks,
Chris

RE: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Chris Brandt]

Hi Wolfram,

On Tuesday, January 17, 2017, Wolfram Sang wrote:
> > I have no idea if the SDHI driver disables the module clock when it is
> > idle, but shouldn't the card detect clock be running all the time when
> > the driver is bound to the device?
> 
> Yes, it should. And for all instances with just one clock, this means this
> main clock must be running. So, en-/disable functions are all about
> suspend/resume and bind/unbind. (Huh, looks like the unbind part is
> missing, though. Need to look closer).

I just did an unbind on my system:

$ echo e804e800.sd > /sys/bus/platform/drivers/sh_mobile_sdhi/unbind


Looks like sh_mobile_sdhi_remove() is called, but sh_mobile_sdhi_clk_disable()
is not.

So, should sh_mobile_sdhi_remove() be changed to call sh_mobile_sdhi_clk_disable()?


Chris

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 141 bytes --]


> So, should sh_mobile_sdhi_remove() be changed to call sh_mobile_sdhi_clk_disable()?

Give me a minute, I already did a patch for this :)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] ARM: dts: r7s72100: fix sdhi clock define

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 141 bytes --]


> So, should sh_mobile_sdhi_remove() be changed to call sh_mobile_sdhi_clk_disable()?

Give me a minute, I already did a patch for this :)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]