From: Wolfram Sang <hidden> Date: 2017-01-17 08:21:19
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.
Hi Wolfram,
On Tue, Jan 17, 2017 at 9:21 AM, Wolfram Sang [off-list ref] wrote:
quoted
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.
quoted
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
From: Wolfram Sang <hidden> Date: 2017-01-17 09:45:30
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?
Hi Wolfram,
On Tue, Jan 17, 2017 at 10:45 AM, Wolfram Sang [off-list ref] wrote:
quoted
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
From: Chris Brandt <Chris.Brandt@renesas.com> Date: 2017-01-17 14:48:10
Hi Wolfram,
On Tuesday, January 17, 2017, Wolfram Sang wrote:
quoted
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
From: Chris Brandt <Chris.Brandt@renesas.com> Date: 2017-01-17 18:42:52
Hi Wolfram,
On Tuesday, January 17, 2017, Wolfram Sang wrote:
quoted
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