Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1424796 > unrolled thread

[PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag for SPI3 related clocks

Back to article view | Back to linux.kernel

Contents

  [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag for  SPI3 related clocks Andi Shyti <andi.shyti@samsung.com> - 2016-06-17 10:20 +0200
    Re: [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag  for SPI3 related clocks Tomasz Figa <tomasz.figa@gmail.com> - 2016-06-18 16:40 +0200
      Re: [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag  for SPI3 related clocks Andi Shyti <andi@etezian.org> - 2016-06-19 08:00 +0200
        Re: [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag  for SPI3 related clocks Tomasz Figa <tomasz.figa@gmail.com> - 2016-06-19 15:50 +0200
          Re: [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag for  SPI3 related clocks Andi Shyti <andi.shyti@samsung.com> - 2016-06-20 07:50 +0200
            Re: [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag  for SPI3 related clocks Tomasz Figa <tomasz.figa@gmail.com> - 2016-06-20 11:10 +0200
              Re: [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag for  SPI3 related clocks Andi Shyti <andi.shyti@samsung.com> - 2016-06-20 12:30 +0200

#1424796 — [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag for SPI3 related clocks

From: Jaehoon Chung <jh80.chung@samsung.com>

The SPI 3 bus uses two clocks, a bus clock and an input clock.
Do not disable the clocks when unused in order to allow access to
the SPI 3 device.

Signed-off-by: Jaehoon Chung <jh80.chung@samsung.com>
Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
---
 drivers/clk/samsung/clk-exynos5433.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c
index 128527b..7c1fa09 100644
--- a/drivers/clk/samsung/clk-exynos5433.c
+++ b/drivers/clk/samsung/clk-exynos5433.c
@@ -1641,11 +1641,12 @@ static struct samsung_gate_clock peric_gate_clks[] __initdata = {
 	GATE(CLK_SCLK_IOCLK_SPI4, "sclk_ioclk_spi4", "ioclk_spi4_clk_in",
 			ENABLE_SCLK_PERIC, 21, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_SCLK_IOCLK_SPI3, "sclk_ioclk_spi3", "ioclk_spi3_clk_in",
-			ENABLE_SCLK_PERIC, 20, CLK_SET_RATE_PARENT, 0),
+			ENABLE_SCLK_PERIC, 20,
+			CLK_IGNORE_UNUSED | CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_SCLK_SPI4, "sclk_spi4", "sclk_spi4_peric", ENABLE_SCLK_PERIC,
 			19, CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_SCLK_SPI3, "sclk_spi3", "sclk_spi3_peric", ENABLE_SCLK_PERIC,
-			18, CLK_SET_RATE_PARENT, 0),
+			18, CLK_IGNORE_UNUSED | CLK_SET_RATE_PARENT, 0),
 	GATE(CLK_SCLK_SCI, "sclk_sci", "div_sclk_sci", ENABLE_SCLK_PERIC,
 			17, 0, 0),
 	GATE(CLK_SCLK_SC_IN, "sclk_sc_in", "div_sclk_sc_in", ENABLE_SCLK_PERIC,
-- 
2.8.1

[toc] | [next] | [standalone]

#1425729 — Re: [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag for SPI3 related clocks

Hi Andi, Jaehoon,

2016-06-17 17:16 GMT+09:00 Andi Shyti <andi.shyti@samsung.com>:
> From: Jaehoon Chung <jh80.chung@samsung.com>
>
> The SPI 3 bus uses two clocks, a bus clock and an input clock.
> Do not disable the clocks when unused in order to allow access to
> the SPI 3 device.

If unused, why would access to SPI 3 device needed?

Best regards,
Tomasz

[toc] | [prev] | [next] | [standalone]

#1425936 — Re: [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag for SPI3 related clocks

Hi Tomasz,

> > The SPI 3 bus uses two clocks, a bus clock and an input clock.
> > Do not disable the clocks when unused in order to allow access to
> > the SPI 3 device.
> 
> If unused, why would access to SPI 3 device needed?

because next I will submit a small driver which uses the SPI3.
Actually in the exynos5433 boards all the SPI are used but not all
the drivers are ported to mainline.

Thanks,
Andi

[toc] | [prev] | [next] | [standalone]

#1425973 — Re: [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag for SPI3 related clocks

2016-06-19 14:59 GMT+09:00 Andi Shyti <andi@etezian.org>:
> Hi Tomasz,
>
>> > The SPI 3 bus uses two clocks, a bus clock and an input clock.
>> > Do not disable the clocks when unused in order to allow access to
>> > the SPI 3 device.
>>
>> If unused, why would access to SPI 3 device needed?
>
> because next I will submit a small driver which uses the SPI3.
> Actually in the exynos5433 boards all the SPI are used but not all
> the drivers are ported to mainline.

Then shouldn't the driver request the clocks and enable them? Or I'm
missing something obvious? :)

Best regards,
Tomasz

[toc] | [prev] | [next] | [standalone]

#1426215

Hi Tomasz,

> >> > The SPI 3 bus uses two clocks, a bus clock and an input clock.
> >> > Do not disable the clocks when unused in order to allow access to
> >> > the SPI 3 device.
> >>
> >> If unused, why would access to SPI 3 device needed?
> >
> > because next I will submit a small driver which uses the SPI3.
> > Actually in the exynos5433 boards all the SPI are used but not all
> > the drivers are ported to mainline.
> 
> Then shouldn't the driver request the clocks and enable them? Or I'm
> missing something obvious? :)

the reason is that...

[ from the patch ]

>         GATE(CLK_SCLK_IOCLK_SPI3, "sclk_ioclk_spi3", "ioclk_spi3_clk_in",
> -                       ENABLE_SCLK_PERIC, 20, CLK_SET_RATE_PARENT, 0),
> +                       ENABLE_SCLK_PERIC, 20,
> +                       CLK_IGNORE_UNUSED | CLK_SET_RATE_PARENT, 0),

... the sclk_ioclk_spi3 is new in exynos5433 and there is no
implementation for enabling/disabling that particular clock...

>         GATE(CLK_SCLK_SPI3, "sclk_spi3", "sclk_spi3_peric", ENABLE_SCLK_PERIC,
> -                       18, CLK_SET_RATE_PARENT, 0),
> +                       18, CLK_IGNORE_UNUSED | CLK_SET_RATE_PARENT, 0),

... while in this case your question makes sense, but it depends
on which clock the device (s3c64xx) is requesting (from the DTS).
In any case, I kept it consistent with the SPI1, which falls in
the same case, as in mainline we don't have any DTS for
exynos5433 (yet!).

Thanks,
Andi

[toc] | [prev] | [next] | [standalone]

#1426358 — Re: [PATCH] clk: samsung: exynos5433: use clock_ignore_unused flag for SPI3 related clocks

2016-06-20 14:41 GMT+09:00 Andi Shyti <andi.shyti@samsung.com>:
> Hi Tomasz,
>
>> >> > The SPI 3 bus uses two clocks, a bus clock and an input clock.
>> >> > Do not disable the clocks when unused in order to allow access to
>> >> > the SPI 3 device.
>> >>
>> >> If unused, why would access to SPI 3 device needed?
>> >
>> > because next I will submit a small driver which uses the SPI3.
>> > Actually in the exynos5433 boards all the SPI are used but not all
>> > the drivers are ported to mainline.
>>
>> Then shouldn't the driver request the clocks and enable them? Or I'm
>> missing something obvious? :)
>
> the reason is that...
>
> [ from the patch ]
>
>>         GATE(CLK_SCLK_IOCLK_SPI3, "sclk_ioclk_spi3", "ioclk_spi3_clk_in",
>> -                       ENABLE_SCLK_PERIC, 20, CLK_SET_RATE_PARENT, 0),
>> +                       ENABLE_SCLK_PERIC, 20,
>> +                       CLK_IGNORE_UNUSED | CLK_SET_RATE_PARENT, 0),
>
> ... the sclk_ioclk_spi3 is new in exynos5433 and there is no
> implementation for enabling/disabling that particular clock...

I'm not convinced that it's a good reason to make boards on which SPI3
is unused not gate the clock by default, then. The correct solution
would be to make the SPI driver request this clock and enable it.

>
>>         GATE(CLK_SCLK_SPI3, "sclk_spi3", "sclk_spi3_peric", ENABLE_SCLK_PERIC,
>> -                       18, CLK_SET_RATE_PARENT, 0),
>> +                       18, CLK_IGNORE_UNUSED | CLK_SET_RATE_PARENT, 0),
>
> ... while in this case your question makes sense, but it depends
> on which clock the device (s3c64xx) is requesting (from the DTS).
> In any case, I kept it consistent with the SPI1, which falls in
> the same case, as in mainline we don't have any DTS for
> exynos5433 (yet!).

Same here.

In general CLK_IGNORE_UNUSED should be avoided, because the semantics
of CLK_IGNORE_UNUSED are really weak and protect only from
clk_disable_unused(). It doesn't cause the clock to be enabled if it
was disabled at boot-up and it doesn't cause its parents to be enabled
(not even stay enabled if they don't have CLK_IGNORE_UNUSED, but have
gating capability). The only reasonable use case for it that I can
think of now is a display controller that needs certain clocks to
remain running to keep showing something on the screen until the
proper driver is loaded and takes over.

Unfortunately I have to NAK this patch. Please add proper support for
them to the SPI driver.

Best regards,
Tomasz

[toc] | [prev] | [next] | [standalone]

#1426472

> Unfortunately I have to NAK this patch. Please add proper support for
> them to the SPI driver.

I agree, this is quite a lazy solution.

But your comment is giving me more work than what I was willing
to have, also because in this driver the SPI1 is aslo using
CLK_IGNORE_UNUSED :)

Thanks for your review,
Andi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web