diff mbox

[v3,5/6] ARM: shmobile: lager: Set clock rates for SDHI

Message ID 1435332230.23818.12.camel@codethink.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Hutchings June 26, 2015, 3:23 p.m. UTC
Set the input clocks to the highest supported speeds.

Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
---
 arch/arm/boot/dts/r8a7790-lager.dts | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Kuninori Morimoto June 29, 2015, 6:23 a.m. UTC | #1
Hi Ben 
Cc Laurent, Geert, Magnus

> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> index aaa4f258e279..5f68e53c58ae 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -488,6 +488,9 @@
>  	pinctrl-0 = <&sdhi0_pins>;
>  	pinctrl-names = "default";
>  
> +	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
> +	assigned-clock-rates = <156000000>;
> +
>  	vmmc-supply = <&vcc_sdhi0>;
>  	vqmmc-supply = <&vccq_sdhi0>;
>  	cd-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
> @@ -498,6 +501,9 @@
>  	pinctrl-0 = <&sdhi2_pins>;
>  	pinctrl-names = "default";
>  
> +	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
> +	assigned-clock-rates = <97500000>;
> +
>  	vmmc-supply = <&vcc_sdhi2>;
>  	vqmmc-supply = <&vccq_sdhi2>;
>  	cd-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;

Thank you for your patch, but I still wandering about this.
Please correct me if I'm misunderstanding.

Above seetings is for SDHI IP, and it can divide it ?
The image is

	[CPG] -> 156 MHz -> [SDHI] -> 1/x -> [CARD]
	         ~~~~~~~

If so, why we can't use max-frequency ?
We can calculate/set SDHI IP clocks via 
max-frequency / clk_round_rate() / clk_set_rate()
since we know SDHI's divider capability.

SH-MMC is using this style. and I think it is flexible for every speed.
Please check sh_mmcif_clock_control(), sh_mmcif_clk_setup()
on ${LINUX}/drivers/mmc/host/sh_mmcif.c

Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings June 30, 2015, 12:02 a.m. UTC | #2
On Mon, 2015-06-29 at 06:23 +0000, Kuninori Morimoto wrote:
> Hi Ben 
> Cc Laurent, Geert, Magnus
> 
> > diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
> > index aaa4f258e279..5f68e53c58ae 100644
> > --- a/arch/arm/boot/dts/r8a7790-lager.dts
> > +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> > @@ -488,6 +488,9 @@
> >  	pinctrl-0 = <&sdhi0_pins>;
> >  	pinctrl-names = "default";
> >  
> > +	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
> > +	assigned-clock-rates = <156000000>;
> > +
> >  	vmmc-supply = <&vcc_sdhi0>;
> >  	vqmmc-supply = <&vccq_sdhi0>;
> >  	cd-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
> > @@ -498,6 +501,9 @@
> >  	pinctrl-0 = <&sdhi2_pins>;
> >  	pinctrl-names = "default";
> >  
> > +	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
> > +	assigned-clock-rates = <97500000>;
> > +
> >  	vmmc-supply = <&vcc_sdhi2>;
> >  	vqmmc-supply = <&vccq_sdhi2>;
> >  	cd-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;
> 
> Thank you for your patch, but I still wandering about this.
> Please correct me if I'm misunderstanding.
> 
> Above seetings is for SDHI IP, and it can divide it ?
> The image is
> 
> 	[CPG] -> 156 MHz -> [SDHI] -> 1/x -> [CARD]
> 	         ~~~~~~~
> 
> If so, why we can't use max-frequency ?
> We can calculate/set SDHI IP clocks via 
> max-frequency / clk_round_rate() / clk_set_rate()
> since we know SDHI's divider capability.
> 
> SH-MMC is using this style. and I think it is flexible for every speed.
> Please check sh_mmcif_clock_control(), sh_mmcif_clk_setup()
> on ${LINUX}/drivers/mmc/host/sh_mmcif.c

That's certainly a nicer way of doing this.  The difficulty I see is
that tmio_mmc doesn't know anything about the input clock, and not all
of the drivers using it actually use the clock framework.

Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings June 30, 2015, 1:29 a.m. UTC | #3
On Tue, 2015-06-30 at 01:02 +0100, Ben Hutchings wrote:
> On Mon, 2015-06-29 at 06:23 +0000, Kuninori Morimoto wrote:
[...]
> > If so, why we can't use max-frequency ?
> > We can calculate/set SDHI IP clocks via 
> > max-frequency / clk_round_rate() / clk_set_rate()
> > since we know SDHI's divider capability.
> > 
> > SH-MMC is using this style. and I think it is flexible for every speed.
> > Please check sh_mmcif_clock_control(), sh_mmcif_clk_setup()
> > on ${LINUX}/drivers/mmc/host/sh_mmcif.c
> 
> That's certainly a nicer way of doing this.  The difficulty I see is
> that tmio_mmc doesn't know anything about the input clock, and not all
> of the drivers using it actually use the clock framework.

More importantly, that algrithm can result in overclocking the card.

Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings June 30, 2015, 1:31 a.m. UTC | #4
On Tue, 2015-06-30 at 02:29 +0100, Ben Hutchings wrote:
> On Tue, 2015-06-30 at 01:02 +0100, Ben Hutchings wrote:
> > On Mon, 2015-06-29 at 06:23 +0000, Kuninori Morimoto wrote:
> [...]
> > > If so, why we can't use max-frequency ?
> > > We can calculate/set SDHI IP clocks via 
> > > max-frequency / clk_round_rate() / clk_set_rate()
> > > since we know SDHI's divider capability.
> > > 
> > > SH-MMC is using this style. and I think it is flexible for every speed.
> > > Please check sh_mmcif_clock_control(), sh_mmcif_clk_setup()
> > > on ${LINUX}/drivers/mmc/host/sh_mmcif.c
> > 
> > That's certainly a nicer way of doing this.  The difficulty I see is
> > that tmio_mmc doesn't know anything about the input clock, and not all
> > of the drivers using it actually use the clock framework.
> 
> More importantly, that algrithm can result in overclocking the card.

...and some of the multiplications overflow!

Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kuninori Morimoto June 30, 2015, 1:45 a.m. UTC | #5
Hi Ben

> > > > SH-MMC is using this style. and I think it is flexible for every speed.
> > > > Please check sh_mmcif_clock_control(), sh_mmcif_clk_setup()
> > > > on ${LINUX}/drivers/mmc/host/sh_mmcif.c
> > > 
> > > That's certainly a nicer way of doing this.  The difficulty I see is
> > > that tmio_mmc doesn't know anything about the input clock, and not all
> > > of the drivers using it actually use the clock framework.
> > 
> > More importantly, that algrithm can result in overclocking the card.
> 
> ...and some of the multiplications overflow!

Can you show us more detail ?
It seems bug...

Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings June 30, 2015, 2:06 a.m. UTC | #6
On Tue, 2015-06-30 at 01:45 +0000, Kuninori Morimoto wrote:
> Hi Ben
> 
> > > > > SH-MMC is using this style. and I think it is flexible for every speed.
> > > > > Please check sh_mmcif_clock_control(), sh_mmcif_clk_setup()
> > > > > on ${LINUX}/drivers/mmc/host/sh_mmcif.c
> > > > 
> > > > That's certainly a nicer way of doing this.  The difficulty I see is
> > > > that tmio_mmc doesn't know anything about the input clock, and not all
> > > > of the drivers using it actually use the clock framework.
> > > 
> > > More importantly, that algrithm can result in overclocking the card.
> > 
> > ...and some of the multiplications overflow!
> 
> Can you show us more detail ?
> It seems bug...

The requested clock rate can be up to 52 MHz and the divider can be up
to 1024.  With div == 1024 and clk == 52000000, clk * div overflows.

Ben.


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kuninori Morimoto June 30, 2015, 4:43 a.m. UTC | #7
Hi Ben

> > Can you show us more detail ?
> > It seems bug...
> 
> The requested clock rate can be up to 52 MHz and the divider can be up
> to 1024.  With div == 1024 and clk == 52000000, clk * div overflows.

IMO, In such case, driver should find
 (input) 52 MHz / (divide) 1 = (request) 52 MHz
or similar.

52 GHz / 1024 = 52 MHz can be possible (from HW point),
but...
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index aaa4f258e279..5f68e53c58ae 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -488,6 +488,9 @@ 
 	pinctrl-0 = <&sdhi0_pins>;
 	pinctrl-names = "default";
 
+	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI0>;
+	assigned-clock-rates = <156000000>;
+
 	vmmc-supply = <&vcc_sdhi0>;
 	vqmmc-supply = <&vccq_sdhi0>;
 	cd-gpios = <&gpio3 6 GPIO_ACTIVE_LOW>;
@@ -498,6 +501,9 @@ 
 	pinctrl-0 = <&sdhi2_pins>;
 	pinctrl-names = "default";
 
+	assigned-clocks = <&mstp3_clks R8A7790_CLK_SDHI2>;
+	assigned-clock-rates = <97500000>;
+
 	vmmc-supply = <&vcc_sdhi2>;
 	vqmmc-supply = <&vccq_sdhi2>;
 	cd-gpios = <&gpio3 22 GPIO_ACTIVE_LOW>;