| Message ID | 1435332230.23818.12.camel@codethink.co.uk (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Simon Horman |
| Headers | show |
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-sh" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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>;
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(+)