* [PATCH] clk: s3c64xx: Correct spi bus clock hookups
@ 2013-10-15 12:26 Charles Keepax
2013-10-15 22:52 ` Tomasz Figa
0 siblings, 1 reply; 5+ messages in thread
From: Charles Keepax @ 2013-10-15 12:26 UTC (permalink / raw)
To: linux-arm-kernel
Correct the SPI bus clock hookups to match their state before the switch
to this driver. With out this patch my system (based on an s3c6410)
can't locate any spibus clock.
Note, I only have a passing familiarity with this driver and subsystem
so this patch could probably use a careful eye.
Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
drivers/clk/samsung/clk-s3c64xx.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/samsung/clk-s3c64xx.c b/drivers/clk/samsung/clk-s3c64xx.c
index 7d2c842..f6ac974 100644
--- a/drivers/clk/samsung/clk-s3c64xx.c
+++ b/drivers/clk/samsung/clk-s3c64xx.c
@@ -356,8 +356,12 @@ static struct samsung_clock_alias s3c64xx_clock_aliases[] = {
ALIAS(SCLK_MMC2, "s3c-sdhci.2", "mmc_busclk.2"),
ALIAS(SCLK_MMC1, "s3c-sdhci.1", "mmc_busclk.2"),
ALIAS(SCLK_MMC0, "s3c-sdhci.0", "mmc_busclk.2"),
- ALIAS(SCLK_SPI1, "s3c6410-spi.1", "spi-bus"),
- ALIAS(SCLK_SPI0, "s3c6410-spi.0", "spi-bus"),
+ ALIAS(PCLK_SPI1, "s3c6410-spi.1", "spi_busclk0"),
+ ALIAS(SCLK_SPI1, "s3c6410-spi.1", "spi_busclk1"),
+ ALIAS(SCLK_SPI1_48, "s3c6410-spi.1", "spi_busclk2"),
+ ALIAS(PCLK_SPI0, "s3c6410-spi.0", "spi_busclk0"),
+ ALIAS(SCLK_SPI0, "s3c6410-spi.0", "spi_busclk1"),
+ ALIAS(SCLK_SPI0_48, "s3c6410-spi.0", "spi_busclk2"),
ALIAS(SCLK_AUDIO1, "samsung-pcm.1", "audio-bus"),
ALIAS(SCLK_AUDIO1, "samsung-i2s.1", "audio-bus"),
ALIAS(SCLK_AUDIO0, "samsung-pcm.0", "audio-bus"),
--
1.7.2.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH] clk: s3c64xx: Correct spi bus clock hookups
2013-10-15 12:26 [PATCH] clk: s3c64xx: Correct spi bus clock hookups Charles Keepax
@ 2013-10-15 22:52 ` Tomasz Figa
2013-10-16 8:10 ` Charles Keepax
0 siblings, 1 reply; 5+ messages in thread
From: Tomasz Figa @ 2013-10-15 22:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi Charles,
On Tuesday 15 of October 2013 13:26:22 Charles Keepax wrote:
> Correct the SPI bus clock hookups to match their state before the switch
> to this driver. With out this patch my system (based on an s3c6410)
> can't locate any spibus clock.
That's right, a patch like this is needed indeed, however...
> Note, I only have a passing familiarity with this driver and subsystem
> so this patch could probably use a careful eye.
>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
> drivers/clk/samsung/clk-s3c64xx.c | 8 ++++++--
> 1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/clk/samsung/clk-s3c64xx.c
> b/drivers/clk/samsung/clk-s3c64xx.c index 7d2c842..f6ac974 100644
> --- a/drivers/clk/samsung/clk-s3c64xx.c
> +++ b/drivers/clk/samsung/clk-s3c64xx.c
> @@ -356,8 +356,12 @@ static struct samsung_clock_alias
> s3c64xx_clock_aliases[] = { ALIAS(SCLK_MMC2, "s3c-sdhci.2",
> "mmc_busclk.2"),
> ALIAS(SCLK_MMC1, "s3c-sdhci.1", "mmc_busclk.2"),
> ALIAS(SCLK_MMC0, "s3c-sdhci.0", "mmc_busclk.2"),
> - ALIAS(SCLK_SPI1, "s3c6410-spi.1", "spi-bus"),
> - ALIAS(SCLK_SPI0, "s3c6410-spi.0", "spi-bus"),
> + ALIAS(PCLK_SPI1, "s3c6410-spi.1", "spi_busclk0"),
> + ALIAS(SCLK_SPI1, "s3c6410-spi.1", "spi_busclk1"),
> + ALIAS(SCLK_SPI1_48, "s3c6410-spi.1", "spi_busclk2"),
...according to the documentation, the order is different. The SPI_CLKSEL
field of CLK_CFG register of the SPI block can have following values:
0 - PCLK (aka PCLK_SPIx)
1 - USBCLK (aka SCLK_SPIx_48)
2 - Epll clock (aka SCLK_SPIx)
The index after spi_busclk corresponds to the value written to SPI_CLKSEL
field, so your patch should be adjusted accordingly.
By the way, the USBCLK case is a bit strange, because it requires USB
signal mask to be unmasked, which in turn needs USB PHY to be enabled, as
otherwise some "unwanted leakage" can occur. Having to enable USB just to
use SPI seems rather inconvenient (especially in terms of power
consumption), so the usability of this clock is rather limited and it
might be better not to let the driver know about it.
Best regards,
Tomasz
P.S. Please keep linux-samsung-soc mailing list on Cc when sending patches
for Samsung hardware.
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] clk: s3c64xx: Correct spi bus clock hookups
2013-10-15 22:52 ` Tomasz Figa
@ 2013-10-16 8:10 ` Charles Keepax
2013-10-16 9:10 ` Tomasz Figa
0 siblings, 1 reply; 5+ messages in thread
From: Charles Keepax @ 2013-10-16 8:10 UTC (permalink / raw)
To: linux-arm-kernel
+ samsung soc mailing list
On Wed, Oct 16, 2013 at 12:52:42AM +0200, Tomasz Figa wrote:
> Hi Charles,
>
> On Tuesday 15 of October 2013 13:26:22 Charles Keepax wrote:
> > ALIAS(SCLK_MMC1, "s3c-sdhci.1", "mmc_busclk.2"),
> > ALIAS(SCLK_MMC0, "s3c-sdhci.0", "mmc_busclk.2"),
> > - ALIAS(SCLK_SPI1, "s3c6410-spi.1", "spi-bus"),
> > - ALIAS(SCLK_SPI0, "s3c6410-spi.0", "spi-bus"),
> > + ALIAS(PCLK_SPI1, "s3c6410-spi.1", "spi_busclk0"),
> > + ALIAS(SCLK_SPI1, "s3c6410-spi.1", "spi_busclk1"),
> > + ALIAS(SCLK_SPI1_48, "s3c6410-spi.1", "spi_busclk2"),
>
> ...according to the documentation, the order is different. The SPI_CLKSEL
> field of CLK_CFG register of the SPI block can have following values:
> 0 - PCLK (aka PCLK_SPIx)
> 1 - USBCLK (aka SCLK_SPIx_48)
> 2 - Epll clock (aka SCLK_SPIx)
>
> The index after spi_busclk corresponds to the value written to SPI_CLKSEL
> field, so your patch should be adjusted accordingly.
Hmm... will probably need to test this to see what happens the
old clock setup was this:
CLKDEV_INIT(NULL, "spi_busclk0", &clk_p),
CLKDEV_INIT("s3c6410-spi.0", "spi_busclk1", &clk_sclk_spi0.clk),
CLKDEV_INIT("s3c6410-spi.0", "spi_busclk2", &clk_48m_spi0),
CLKDEV_INIT("s3c6410-spi.1", "spi_busclk1", &clk_sclk_spi1.clk),
CLKDEV_INIT("s3c6410-spi.1", "spi_busclk2", &clk_48m_spi1),
Which appears to differ from the documentation, that said though
I would wager that only the first of those has really had much
testing.
>
> By the way, the USBCLK case is a bit strange, because it requires USB
> signal mask to be unmasked, which in turn needs USB PHY to be enabled, as
> otherwise some "unwanted leakage" can occur. Having to enable USB just to
> use SPI seems rather inconvenient (especially in terms of power
> consumption), so the usability of this clock is rather limited and it
> might be better not to let the driver know about it.
Seems reasonable to drop this for now and it could be added back
in if someone specifically needed to use it.
Thanks,
Charles
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] clk: s3c64xx: Correct spi bus clock hookups
2013-10-16 8:10 ` Charles Keepax
@ 2013-10-16 9:10 ` Tomasz Figa
2013-10-16 10:26 ` Charles Keepax
0 siblings, 1 reply; 5+ messages in thread
From: Tomasz Figa @ 2013-10-16 9:10 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 16 of October 2013 09:10:35 Charles Keepax wrote:
> + samsung soc mailing list
>
> On Wed, Oct 16, 2013 at 12:52:42AM +0200, Tomasz Figa wrote:
> > Hi Charles,
> >
> > On Tuesday 15 of October 2013 13:26:22 Charles Keepax wrote:
> > > ALIAS(SCLK_MMC1, "s3c-sdhci.1", "mmc_busclk.2"),
> > > ALIAS(SCLK_MMC0, "s3c-sdhci.0", "mmc_busclk.2"),
> > > - ALIAS(SCLK_SPI1, "s3c6410-spi.1", "spi-bus"),
> > > - ALIAS(SCLK_SPI0, "s3c6410-spi.0", "spi-bus"),
> > > + ALIAS(PCLK_SPI1, "s3c6410-spi.1", "spi_busclk0"),
> > > + ALIAS(SCLK_SPI1, "s3c6410-spi.1", "spi_busclk1"),
> > > + ALIAS(SCLK_SPI1_48, "s3c6410-spi.1", "spi_busclk2"),
> >
> > ...according to the documentation, the order is different. The SPI_CLKSEL
> > field of CLK_CFG register of the SPI block can have following values:
> > 0 - PCLK (aka PCLK_SPIx)
> > 1 - USBCLK (aka SCLK_SPIx_48)
> > 2 - Epll clock (aka SCLK_SPIx)
> >
> > The index after spi_busclk corresponds to the value written to SPI_CLKSEL
> > field, so your patch should be adjusted accordingly.
>
> Hmm... will probably need to test this to see what happens the
> old clock setup was this:
>
> CLKDEV_INIT(NULL, "spi_busclk0", &clk_p),
> CLKDEV_INIT("s3c6410-spi.0", "spi_busclk1", &clk_sclk_spi0.clk),
> CLKDEV_INIT("s3c6410-spi.0", "spi_busclk2", &clk_48m_spi0),
> CLKDEV_INIT("s3c6410-spi.1", "spi_busclk1", &clk_sclk_spi1.clk),
> CLKDEV_INIT("s3c6410-spi.1", "spi_busclk2", &clk_48m_spi1),
>
> Which appears to differ from the documentation, that said though
> I would wager that only the first of those has really had much
> testing.
I believe in 90% cases the driver simply used first available clock,
which would be the PCLK, so I wouldn't be really surprised if operation
on other clock sources weren't even tested.
If you have hardware to test this, especially with possibility of checking
the SPI frequency I would really appreciate this, as I unfortunately don't
have such.
Best regards,
Tomasz
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] clk: s3c64xx: Correct spi bus clock hookups
2013-10-16 9:10 ` Tomasz Figa
@ 2013-10-16 10:26 ` Charles Keepax
0 siblings, 0 replies; 5+ messages in thread
From: Charles Keepax @ 2013-10-16 10:26 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Oct 16, 2013 at 11:10:59AM +0200, Tomasz Figa wrote:
> If you have hardware to test this, especially with possibility of checking
> the SPI frequency I would really appreciate this, as I unfortunately don't
> have such.
I might not be able to get around to it this week, but I
certainly should be able to test this on the hardware I have and
measure the SPI speed.
Thanks,
Charles
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-10-16 10:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-15 12:26 [PATCH] clk: s3c64xx: Correct spi bus clock hookups Charles Keepax
2013-10-15 22:52 ` Tomasz Figa
2013-10-16 8:10 ` Charles Keepax
2013-10-16 9:10 ` Tomasz Figa
2013-10-16 10:26 ` Charles Keepax
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).