All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/14] SCI clocks cleanup
@ 2015-09-14 12:14 Laurent Pinchart
  2015-09-14 12:14   ` Laurent Pinchart
                   ` (5 more replies)
  0 siblings, 6 replies; 10+ messages in thread
From: Laurent Pinchart @ 2015-09-14 12:14 UTC (permalink / raw)
  To: linux-sh

Hello,

The SCI driver currently handles two clocks, an interface clock named sci_ick
and a functional clock named sci_fck. Studying the datasheets of the SH and
ARM SoCs that incorportate (H)SCI(F)([AB]) instances showed (un)surprisingly
that the hardware doesn't have a separate controllable interface clock.

All the platforms that declare an interface clock for the SCI set it to the
clock used as the SCI functional clock. The two clocks can thus be merged on
the driver side, which is what this patch series does. The resulting clock is
called "fck", and all SH and ARM users (both DT and non-DT) are fixed to name
their SCI clocks appropriately.

Support for the "sci_ick" name is kept in the sh-sci driver to ensure DT
backward compatibility, and support for the "peripheral_clk" clock to not
break SH platforms that don't declare device-specific SCI clocks. The later
can be removed when all SH platforms will declare their SCI clocks properly.

The patches have been developed for an ancien (v3.x for those who were born)
kernel and rebased on top of Simon's master branch. I've only compile-tested
them after the rebase. Geert, I believe this series is a good preliminary
cleanup for the SCI baud rate generator clock support. Could you give it a try
as part of your work on that ?

Laurent Pinchart (14):
  serial: sh-sci: Drop the interface clock
  sh: Rename sci_ick and sci_fck clock to fck
  sh: Remove sci_ick clock alias
  ARM: shmobile: sh73a0: Rename the serial port clock to fck
  ARM: shmobile: r7s72100: Rename the serial port clock to fck
  ARM: shmobile: r8a73a4: Rename the serial port clock to fck
  ARM: shmobile: r8a7740: Rename the serial port clock to fck
  ARM: shmobile: r8a7778: Rename the serial port clock to fck
  ARM: shmobile: r8a7779: Rename the serial port clock to fck
  ARM: shmobile: r8a7790: Rename the serial port clock to fck
  ARM: shmobile: r8a7791: Rename the serial port clock to fck
  ARM: shmobile: r8a7793: Rename the serial port clock to fck
  ARM: shmobile: r8a7794: Rename the serial port clock to fck
  serial: sh-sci: Drop the sci_fck clock fallback

 .../bindings/serial/renesas,sci-serial.txt         |  4 +-
 arch/arm/boot/dts/r7s72100.dtsi                    | 16 +++----
 arch/arm/boot/dts/r8a73a4.dtsi                     | 12 ++---
 arch/arm/boot/dts/r8a7740.dtsi                     | 18 +++----
 arch/arm/boot/dts/r8a7778.dtsi                     | 12 ++---
 arch/arm/boot/dts/r8a7779.dtsi                     | 12 ++---
 arch/arm/boot/dts/r8a7790.dtsi                     | 20 ++++----
 arch/arm/boot/dts/r8a7791.dtsi                     | 36 +++++++-------
 arch/arm/boot/dts/r8a7793.dtsi                     |  4 +-
 arch/arm/boot/dts/r8a7794.dtsi                     | 36 +++++++-------
 arch/arm/boot/dts/sh73a0.dtsi                      | 18 +++----
 arch/sh/kernel/cpu/clock-cpg.c                     |  1 -
 arch/sh/kernel/cpu/sh2a/clock-sh7264.c             |  9 +++-
 arch/sh/kernel/cpu/sh2a/clock-sh7269.c             | 16 +++----
 arch/sh/kernel/cpu/sh4a/clock-sh7343.c             |  8 ++--
 arch/sh/kernel/cpu/sh4a/clock-sh7366.c             |  6 +--
 arch/sh/kernel/cpu/sh4a/clock-sh7723.c             | 12 ++---
 arch/sh/kernel/cpu/sh4a/clock-sh7734.c             | 12 ++---
 arch/sh/kernel/cpu/sh4a/clock-sh7757.c             |  6 +--
 arch/sh/kernel/cpu/sh4a/clock-sh7785.c             | 12 ++---
 arch/sh/kernel/cpu/sh4a/clock-sh7786.c             | 12 ++---
 arch/sh/kernel/cpu/sh4a/clock-shx3.c               |  8 ++--
 drivers/tty/serial/sh-sci.c                        | 55 +++++++++++++---------
 23 files changed, 180 insertions(+), 165 deletions(-)

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 01/14] serial: sh-sci: Drop the interface clock
  2015-09-14 12:14 [PATCH 00/14] SCI clocks cleanup Laurent Pinchart
@ 2015-09-14 12:14   ` Laurent Pinchart
  2015-09-14 12:26 ` [PATCH 00/14] SCI clocks cleanup Geert Uytterhoeven
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2015-09-14 12:14 UTC (permalink / raw)
  To: linux-sh; +Cc: Geert Uytterhoeven, devicetree

As no platform defines an interface clock the SCI driver always fall
back to a clock named "peripheral_clk". On SH platform that clock is the
base clock for the SCI functional clock and has the same frequency. On
ARM platforms that clock doesn't exist, and clk_get() will return the
default clock for the device. We can thus make the functional clock
mandatory and drop the interface clock.

Cc: devicetree@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/serial/renesas,sci-serial.txt         |  4 +-
 drivers/tty/serial/sh-sci.c                        | 60 +++++++++++++---------
 2 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
index e84b13a8eda3..c390860bc23f 100644
--- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
+++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
@@ -40,7 +40,7 @@ Required properties:
 
   - clocks: Must contain a phandle and clock-specifier pair for each entry
     in clock-names.
-  - clock-names: Must contain "sci_ick" for the SCIx UART interface clock.
+  - clock-names: Must contain "fck" for the SCIx UART functional clock.
 
 Note: Each enabled SCIx UART should have an alias correctly numbered in the
 "aliases" node.
@@ -61,7 +61,7 @@ Example:
 		interrupt-parent = <&gic>;
 		interrupts = <0 144 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp2_clks R8A7790_CLK_SCIFA0>;
-		clock-names = "sci_ick";
+		clock-names = "fck";
 		dmas = <&dmac0 0x21>, <&dmac0 0x22>;
 		dma-names = "tx", "rx";
 	};
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 1b2f894bdc9e..56e4b2639d99 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -91,8 +91,6 @@ struct sci_port {
 	struct timer_list	break_timer;
 	int			break_flag;
 
-	/* Interface clock */
-	struct clk		*iclk;
 	/* Function clock */
 	struct clk		*fclk;
 
@@ -465,9 +463,8 @@ static void sci_port_enable(struct sci_port *sci_port)
 
 	pm_runtime_get_sync(sci_port->port.dev);
 
-	clk_prepare_enable(sci_port->iclk);
-	sci_port->port.uartclk = clk_get_rate(sci_port->iclk);
 	clk_prepare_enable(sci_port->fclk);
+	sci_port->port.uartclk = clk_get_rate(sci_port->fclk);
 }
 
 static void sci_port_disable(struct sci_port *sci_port)
@@ -484,7 +481,6 @@ static void sci_port_disable(struct sci_port *sci_port)
 	sci_port->break_flag = 0;
 
 	clk_disable_unprepare(sci_port->fclk);
-	clk_disable_unprepare(sci_port->iclk);
 
 	pm_runtime_put_sync(sci_port->port.dev);
 }
@@ -1085,7 +1081,7 @@ static int sci_notifier(struct notifier_block *self,
 		struct uart_port *port = &sci_port->port;
 
 		spin_lock_irqsave(&port->lock, flags);
-		port->uartclk = clk_get_rate(sci_port->iclk);
+		port->uartclk = clk_get_rate(sci_port->fclk);
 		spin_unlock_irqrestore(&port->lock, flags);
 	}
 
@@ -2181,6 +2177,38 @@ static struct uart_ops sci_uart_ops = {
 #endif
 };
 
+static int sci_init_clocks(struct sci_port *sci_port, struct device *dev)
+{
+	/* Get the SCI functional clock. It's called "fck" on ARM. */
+	sci_port->fclk = clk_get(dev, "fck");
+	if (!IS_ERR(sci_port->fclk))
+		return 0;
+
+	/*
+	 * But it used to be called "sci_ick", and we need to maintain DT
+	 * backward compatibility.
+	 */
+	sci_port->fclk = clk_get(dev, "sci_ick");
+	if (!IS_ERR(sci_port->fclk))
+		return 0;
+
+	/* SH has historically named the clock "sci_fck". */
+	sci_port->fclk = clk_get(dev, "sci_fck");
+	if (!IS_ERR(sci_port->fclk))
+		return 0;
+
+	/*
+	 * Not all SH platforms declare a clock lookup entry for SCI devices, in
+	 * which case we need to get the global "peripheral_clk" clock.
+	 */
+	sci_port->fclk = clk_get(dev, "peripheral_clk");
+	if (!IS_ERR(sci_port->fclk))
+		return 0;
+
+	dev_err(dev, "failed to get functional clock\n");
+	return PTR_ERR(sci_port->fclk);
+}
+
 static int sci_init_single(struct platform_device *dev,
 			   struct sci_port *sci_port, unsigned int index,
 			   struct plat_sci_port *p, bool early)
@@ -2274,22 +2302,9 @@ static int sci_init_single(struct platform_device *dev,
 				: sampling_rate;
 
 	if (!early) {
-		sci_port->iclk = clk_get(&dev->dev, "sci_ick");
-		if (IS_ERR(sci_port->iclk)) {
-			sci_port->iclk = clk_get(&dev->dev, "peripheral_clk");
-			if (IS_ERR(sci_port->iclk)) {
-				dev_err(&dev->dev, "can't get iclk\n");
-				return PTR_ERR(sci_port->iclk);
-			}
-		}
-
-		/*
-		 * The function clock is optional, ignore it if we can't
-		 * find it.
-		 */
-		sci_port->fclk = clk_get(&dev->dev, "sci_fck");
-		if (IS_ERR(sci_port->fclk))
-			sci_port->fclk = NULL;
+		ret = sci_init_clocks(sci_port, &dev->dev);
+		if (ret < 0)
+			return ret;
 
 		port->dev = &dev->dev;
 
@@ -2339,7 +2354,6 @@ static int sci_init_single(struct platform_device *dev,
 
 static void sci_cleanup_single(struct sci_port *port)
 {
-	clk_put(port->iclk);
 	clk_put(port->fclk);
 
 	pm_runtime_disable(port->port.dev);
-- 
2.4.6


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 01/14] serial: sh-sci: Drop the interface clock
@ 2015-09-14 12:14   ` Laurent Pinchart
  0 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2015-09-14 12:14 UTC (permalink / raw)
  To: linux-sh; +Cc: Geert Uytterhoeven, devicetree

As no platform defines an interface clock the SCI driver always fall
back to a clock named "peripheral_clk". On SH platform that clock is the
base clock for the SCI functional clock and has the same frequency. On
ARM platforms that clock doesn't exist, and clk_get() will return the
default clock for the device. We can thus make the functional clock
mandatory and drop the interface clock.

Cc: devicetree@vger.kernel.org
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
 .../bindings/serial/renesas,sci-serial.txt         |  4 +-
 drivers/tty/serial/sh-sci.c                        | 60 +++++++++++++---------
 2 files changed, 39 insertions(+), 25 deletions(-)

diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
index e84b13a8eda3..c390860bc23f 100644
--- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
+++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
@@ -40,7 +40,7 @@ Required properties:
 
   - clocks: Must contain a phandle and clock-specifier pair for each entry
     in clock-names.
-  - clock-names: Must contain "sci_ick" for the SCIx UART interface clock.
+  - clock-names: Must contain "fck" for the SCIx UART functional clock.
 
 Note: Each enabled SCIx UART should have an alias correctly numbered in the
 "aliases" node.
@@ -61,7 +61,7 @@ Example:
 		interrupt-parent = <&gic>;
 		interrupts = <0 144 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp2_clks R8A7790_CLK_SCIFA0>;
-		clock-names = "sci_ick";
+		clock-names = "fck";
 		dmas = <&dmac0 0x21>, <&dmac0 0x22>;
 		dma-names = "tx", "rx";
 	};
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 1b2f894bdc9e..56e4b2639d99 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -91,8 +91,6 @@ struct sci_port {
 	struct timer_list	break_timer;
 	int			break_flag;
 
-	/* Interface clock */
-	struct clk		*iclk;
 	/* Function clock */
 	struct clk		*fclk;
 
@@ -465,9 +463,8 @@ static void sci_port_enable(struct sci_port *sci_port)
 
 	pm_runtime_get_sync(sci_port->port.dev);
 
-	clk_prepare_enable(sci_port->iclk);
-	sci_port->port.uartclk = clk_get_rate(sci_port->iclk);
 	clk_prepare_enable(sci_port->fclk);
+	sci_port->port.uartclk = clk_get_rate(sci_port->fclk);
 }
 
 static void sci_port_disable(struct sci_port *sci_port)
@@ -484,7 +481,6 @@ static void sci_port_disable(struct sci_port *sci_port)
 	sci_port->break_flag = 0;
 
 	clk_disable_unprepare(sci_port->fclk);
-	clk_disable_unprepare(sci_port->iclk);
 
 	pm_runtime_put_sync(sci_port->port.dev);
 }
@@ -1085,7 +1081,7 @@ static int sci_notifier(struct notifier_block *self,
 		struct uart_port *port = &sci_port->port;
 
 		spin_lock_irqsave(&port->lock, flags);
-		port->uartclk = clk_get_rate(sci_port->iclk);
+		port->uartclk = clk_get_rate(sci_port->fclk);
 		spin_unlock_irqrestore(&port->lock, flags);
 	}
 
@@ -2181,6 +2177,38 @@ static struct uart_ops sci_uart_ops = {
 #endif
 };
 
+static int sci_init_clocks(struct sci_port *sci_port, struct device *dev)
+{
+	/* Get the SCI functional clock. It's called "fck" on ARM. */
+	sci_port->fclk = clk_get(dev, "fck");
+	if (!IS_ERR(sci_port->fclk))
+		return 0;
+
+	/*
+	 * But it used to be called "sci_ick", and we need to maintain DT
+	 * backward compatibility.
+	 */
+	sci_port->fclk = clk_get(dev, "sci_ick");
+	if (!IS_ERR(sci_port->fclk))
+		return 0;
+
+	/* SH has historically named the clock "sci_fck". */
+	sci_port->fclk = clk_get(dev, "sci_fck");
+	if (!IS_ERR(sci_port->fclk))
+		return 0;
+
+	/*
+	 * Not all SH platforms declare a clock lookup entry for SCI devices, in
+	 * which case we need to get the global "peripheral_clk" clock.
+	 */
+	sci_port->fclk = clk_get(dev, "peripheral_clk");
+	if (!IS_ERR(sci_port->fclk))
+		return 0;
+
+	dev_err(dev, "failed to get functional clock\n");
+	return PTR_ERR(sci_port->fclk);
+}
+
 static int sci_init_single(struct platform_device *dev,
 			   struct sci_port *sci_port, unsigned int index,
 			   struct plat_sci_port *p, bool early)
@@ -2274,22 +2302,9 @@ static int sci_init_single(struct platform_device *dev,
 				: sampling_rate;
 
 	if (!early) {
-		sci_port->iclk = clk_get(&dev->dev, "sci_ick");
-		if (IS_ERR(sci_port->iclk)) {
-			sci_port->iclk = clk_get(&dev->dev, "peripheral_clk");
-			if (IS_ERR(sci_port->iclk)) {
-				dev_err(&dev->dev, "can't get iclk\n");
-				return PTR_ERR(sci_port->iclk);
-			}
-		}
-
-		/*
-		 * The function clock is optional, ignore it if we can't
-		 * find it.
-		 */
-		sci_port->fclk = clk_get(&dev->dev, "sci_fck");
-		if (IS_ERR(sci_port->fclk))
-			sci_port->fclk = NULL;
+		ret = sci_init_clocks(sci_port, &dev->dev);
+		if (ret < 0)
+			return ret;
 
 		port->dev = &dev->dev;
 
@@ -2339,7 +2354,6 @@ static int sci_init_single(struct platform_device *dev,
 
 static void sci_cleanup_single(struct sci_port *port)
 {
-	clk_put(port->iclk);
 	clk_put(port->fclk);
 
 	pm_runtime_disable(port->port.dev);
-- 
2.4.6


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/14] SCI clocks cleanup
  2015-09-14 12:14 [PATCH 00/14] SCI clocks cleanup Laurent Pinchart
  2015-09-14 12:14   ` Laurent Pinchart
@ 2015-09-14 12:26 ` Geert Uytterhoeven
  2015-09-14 12:37 ` Laurent Pinchart
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-09-14 12:26 UTC (permalink / raw)
  To: linux-sh

Hi Laurent,

On Mon, Sep 14, 2015 at 2:14 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> The SCI driver currently handles two clocks, an interface clock named sci_ick
> and a functional clock named sci_fck. Studying the datasheets of the SH and
> ARM SoCs that incorportate (H)SCI(F)([AB]) instances showed (un)surprisingly
> that the hardware doesn't have a separate controllable interface clock.
>
> All the platforms that declare an interface clock for the SCI set it to the
> clock used as the SCI functional clock. The two clocks can thus be merged on
> the driver side, which is what this patch series does. The resulting clock is
> called "fck", and all SH and ARM users (both DT and non-DT) are fixed to name
> their SCI clocks appropriately.
>
> Support for the "sci_ick" name is kept in the sh-sci driver to ensure DT
> backward compatibility, and support for the "peripheral_clk" clock to not
> break SH platforms that don't declare device-specific SCI clocks. The later
> can be removed when all SH platforms will declare their SCI clocks properly.
>
> The patches have been developed for an ancien (v3.x for those who were born)
> kernel and rebased on top of Simon's master branch. I've only compile-tested
> them after the rebase. Geert, I believe this series is a good preliminary
> cleanup for the SCI baud rate generator clock support. Could you give it a try
> as part of your work on that ?

Thanks a lot!

There are a few more to fix up:
  - The example in new
    Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt
  - New arch/arm64/boot/dts/renesas/r8a7795.dtsi
  - The new old kid on the block: arch/h8300/boot/dts

I believe we can easily handle those.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/14] SCI clocks cleanup
  2015-09-14 12:14 [PATCH 00/14] SCI clocks cleanup Laurent Pinchart
  2015-09-14 12:14   ` Laurent Pinchart
  2015-09-14 12:26 ` [PATCH 00/14] SCI clocks cleanup Geert Uytterhoeven
@ 2015-09-14 12:37 ` Laurent Pinchart
  2015-09-14 12:46 ` Geert Uytterhoeven
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 10+ messages in thread
From: Laurent Pinchart @ 2015-09-14 12:37 UTC (permalink / raw)
  To: linux-sh

Hi Geert,

On Monday 14 September 2015 14:26:45 Geert Uytterhoeven wrote:
> On Mon, Sep 14, 2015 at 2:14 PM, Laurent Pinchart wrote:
> > The SCI driver currently handles two clocks, an interface clock named
> > sci_ick and a functional clock named sci_fck. Studying the datasheets of
> > the SH and ARM SoCs that incorportate (H)SCI(F)([AB]) instances showed
> > (un)surprisingly that the hardware doesn't have a separate controllable
> > interface clock.
> > 
> > All the platforms that declare an interface clock for the SCI set it to
> > the clock used as the SCI functional clock. The two clocks can thus be
> > merged on the driver side, which is what this patch series does. The
> > resulting clock is called "fck", and all SH and ARM users (both DT and
> > non-DT) are fixed to name their SCI clocks appropriately.
> > 
> > Support for the "sci_ick" name is kept in the sh-sci driver to ensure DT
> > backward compatibility, and support for the "peripheral_clk" clock to not
> > break SH platforms that don't declare device-specific SCI clocks. The
> > later can be removed when all SH platforms will declare their SCI clocks
> > properly.
> > 
> > The patches have been developed for an ancien (v3.x for those who were
> > born) kernel and rebased on top of Simon's master branch. I've only
> > compile-tested them after the rebase. Geert, I believe this series is a
> > good preliminary cleanup for the SCI baud rate generator clock support.
> > Could you give it a try as part of your work on that ?
> 
> Thanks a lot!

You're welcome.

> There are a few more to fix up:
>   - The example in new
>     Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt
>   - New arch/arm64/boot/dts/renesas/r8a7795.dtsi

I haven't included these in the patch series as they're not present in Simon's 
devel branch.

>   - The new old kid on the block: arch/h8300/boot/dts

And I have missed that one. If you take the series in your tree for SCI BRG 
clock development, could you add a patch to handle h8300 ?

> I believe we can easily handle those.

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/14] SCI clocks cleanup
  2015-09-14 12:14 [PATCH 00/14] SCI clocks cleanup Laurent Pinchart
                   ` (2 preceding siblings ...)
  2015-09-14 12:37 ` Laurent Pinchart
@ 2015-09-14 12:46 ` Geert Uytterhoeven
  2015-09-17  7:30 ` Simon Horman
  2015-09-17  7:53 ` Simon Horman
  5 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-09-14 12:46 UTC (permalink / raw)
  To: linux-sh

On Mon, Sep 14, 2015 at 2:37 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> There are a few more to fix up:
>>   - The example in new
>>     Documentation/devicetree/bindings/clock/renesas,rcar-gen3-cpg-clocks.txt
>>   - New arch/arm64/boot/dts/renesas/r8a7795.dtsi
>
> I haven't included these in the patch series as they're not present in Simon's
> devel branch.

I understand.

>>   - The new old kid on the block: arch/h8300/boot/dts
>
> And I have missed that one. If you take the series in your tree for SCI BRG
> clock development, could you add a patch to handle h8300 ?

I handle these.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/14] SCI clocks cleanup
  2015-09-14 12:14 [PATCH 00/14] SCI clocks cleanup Laurent Pinchart
                   ` (3 preceding siblings ...)
  2015-09-14 12:46 ` Geert Uytterhoeven
@ 2015-09-17  7:30 ` Simon Horman
  2015-09-17  7:53 ` Simon Horman
  5 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2015-09-17  7:30 UTC (permalink / raw)
  To: linux-sh

On Mon, Sep 14, 2015 at 03:14:22PM +0300, Laurent Pinchart wrote:
> Hello,
> 
> The SCI driver currently handles two clocks, an interface clock named sci_ick
> and a functional clock named sci_fck. Studying the datasheets of the SH and
> ARM SoCs that incorportate (H)SCI(F)([AB]) instances showed (un)surprisingly
> that the hardware doesn't have a separate controllable interface clock.
> 
> All the platforms that declare an interface clock for the SCI set it to the
> clock used as the SCI functional clock. The two clocks can thus be merged on
> the driver side, which is what this patch series does. The resulting clock is
> called "fck", and all SH and ARM users (both DT and non-DT) are fixed to name
> their SCI clocks appropriately.
> 
> Support for the "sci_ick" name is kept in the sh-sci driver to ensure DT
> backward compatibility, and support for the "peripheral_clk" clock to not
> break SH platforms that don't declare device-specific SCI clocks. The later
> can be removed when all SH platforms will declare their SCI clocks properly.
> 
> The patches have been developed for an ancien (v3.x for those who were born)
> kernel and rebased on top of Simon's master branch. I've only compile-tested
> them after the rebase. Geert, I believe this series is a good preliminary
> cleanup for the SCI baud rate generator clock support. Could you give it a try
> as part of your work on that ?

FWIW, given the explanation above, particularly that regarding backwards
compatibility for DT, I am comfortable with these changes.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 00/14] SCI clocks cleanup
  2015-09-14 12:14 [PATCH 00/14] SCI clocks cleanup Laurent Pinchart
                   ` (4 preceding siblings ...)
  2015-09-17  7:30 ` Simon Horman
@ 2015-09-17  7:53 ` Simon Horman
  5 siblings, 0 replies; 10+ messages in thread
From: Simon Horman @ 2015-09-17  7:53 UTC (permalink / raw)
  To: linux-sh

On Thu, Sep 17, 2015 at 04:30:06PM +0900, Simon Horman wrote:
> On Mon, Sep 14, 2015 at 03:14:22PM +0300, Laurent Pinchart wrote:
> > Hello,
> > 
> > The SCI driver currently handles two clocks, an interface clock named sci_ick
> > and a functional clock named sci_fck. Studying the datasheets of the SH and
> > ARM SoCs that incorportate (H)SCI(F)([AB]) instances showed (un)surprisingly
> > that the hardware doesn't have a separate controllable interface clock.
> > 
> > All the platforms that declare an interface clock for the SCI set it to the
> > clock used as the SCI functional clock. The two clocks can thus be merged on
> > the driver side, which is what this patch series does. The resulting clock is
> > called "fck", and all SH and ARM users (both DT and non-DT) are fixed to name
> > their SCI clocks appropriately.
> > 
> > Support for the "sci_ick" name is kept in the sh-sci driver to ensure DT
> > backward compatibility, and support for the "peripheral_clk" clock to not
> > break SH platforms that don't declare device-specific SCI clocks. The later
> > can be removed when all SH platforms will declare their SCI clocks properly.
> > 
> > The patches have been developed for an ancien (v3.x for those who were born)
> > kernel and rebased on top of Simon's master branch. I've only compile-tested
> > them after the rebase. Geert, I believe this series is a good preliminary
> > cleanup for the SCI baud rate generator clock support. Could you give it a try
> > as part of your work on that ?
> 
> FWIW, given the explanation above, particularly that regarding backwards
> compatibility for DT, I am comfortable with these changes.

I have now looked over the code and it looks good to me:

Acked-by: Simon Horman <horms+renesas@verge.net.au>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 01/14] serial: sh-sci: Drop the interface clock
  2015-09-14 12:14   ` Laurent Pinchart
@ 2015-11-02 14:27     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-11-02 14:27 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Linux-sh list, Geert Uytterhoeven, devicetree@vger.kernel.org

Hi Laurent,

On Mon, Sep 14, 2015 at 2:14 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> As no platform defines an interface clock the SCI driver always fall
> back to a clock named "peripheral_clk". On SH platform that clock is the
> base clock for the SCI functional clock and has the same frequency. On
> ARM platforms that clock doesn't exist, and clk_get() will return the
> default clock for the device. We can thus make the functional clock
> mandatory and drop the interface clock.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../bindings/serial/renesas,sci-serial.txt         |  4 +-
>  drivers/tty/serial/sh-sci.c                        | 60 +++++++++++++---------
>  2 files changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> index e84b13a8eda3..c390860bc23f 100644
> --- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt

> @@ -2181,6 +2177,38 @@ static struct uart_ops sci_uart_ops = {
>  #endif
>  };
>
> +static int sci_init_clocks(struct sci_port *sci_port, struct device *dev)
> +{
> +       /* Get the SCI functional clock. It's called "fck" on ARM. */
> +       sci_port->fclk = clk_get(dev, "fck");
> +       if (!IS_ERR(sci_port->fclk))
> +               return 0;
> +
> +       /*
> +        * But it used to be called "sci_ick", and we need to maintain DT
> +        * backward compatibility.
> +        */
> +       sci_port->fclk = clk_get(dev, "sci_ick");
> +       if (!IS_ERR(sci_port->fclk))
> +               return 0;
> +
> +       /* SH has historically named the clock "sci_fck". */
> +       sci_port->fclk = clk_get(dev, "sci_fck");
> +       if (!IS_ERR(sci_port->fclk))
> +               return 0;
> +
> +       /*
> +        * Not all SH platforms declare a clock lookup entry for SCI devices, in
> +        * which case we need to get the global "peripheral_clk" clock.
> +        */
> +       sci_port->fclk = clk_get(dev, "peripheral_clk");
> +       if (!IS_ERR(sci_port->fclk))
> +               return 0;
> +
> +       dev_err(dev, "failed to get functional clock\n");
> +       return PTR_ERR(sci_port->fclk);

This doesn't handle probe deferral correctly.
-EPROBE_DEFER from an earlier clock will be overwritten by -ENOENT from a
later clock, and the driver will never be reprobed.

> +}

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 01/14] serial: sh-sci: Drop the interface clock
@ 2015-11-02 14:27     ` Geert Uytterhoeven
  0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2015-11-02 14:27 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Linux-sh list, Geert Uytterhoeven, devicetree@vger.kernel.org

Hi Laurent,

On Mon, Sep 14, 2015 at 2:14 PM, Laurent Pinchart
<laurent.pinchart+renesas@ideasonboard.com> wrote:
> As no platform defines an interface clock the SCI driver always fall
> back to a clock named "peripheral_clk". On SH platform that clock is the
> base clock for the SCI functional clock and has the same frequency. On
> ARM platforms that clock doesn't exist, and clk_get() will return the
> default clock for the device. We can thus make the functional clock
> mandatory and drop the interface clock.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
>  .../bindings/serial/renesas,sci-serial.txt         |  4 +-
>  drivers/tty/serial/sh-sci.c                        | 60 +++++++++++++---------
>  2 files changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> index e84b13a8eda3..c390860bc23f 100644
> --- a/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt
> +++ b/Documentation/devicetree/bindings/serial/renesas,sci-serial.txt

> @@ -2181,6 +2177,38 @@ static struct uart_ops sci_uart_ops = {
>  #endif
>  };
>
> +static int sci_init_clocks(struct sci_port *sci_port, struct device *dev)
> +{
> +       /* Get the SCI functional clock. It's called "fck" on ARM. */
> +       sci_port->fclk = clk_get(dev, "fck");
> +       if (!IS_ERR(sci_port->fclk))
> +               return 0;
> +
> +       /*
> +        * But it used to be called "sci_ick", and we need to maintain DT
> +        * backward compatibility.
> +        */
> +       sci_port->fclk = clk_get(dev, "sci_ick");
> +       if (!IS_ERR(sci_port->fclk))
> +               return 0;
> +
> +       /* SH has historically named the clock "sci_fck". */
> +       sci_port->fclk = clk_get(dev, "sci_fck");
> +       if (!IS_ERR(sci_port->fclk))
> +               return 0;
> +
> +       /*
> +        * Not all SH platforms declare a clock lookup entry for SCI devices, in
> +        * which case we need to get the global "peripheral_clk" clock.
> +        */
> +       sci_port->fclk = clk_get(dev, "peripheral_clk");
> +       if (!IS_ERR(sci_port->fclk))
> +               return 0;
> +
> +       dev_err(dev, "failed to get functional clock\n");
> +       return PTR_ERR(sci_port->fclk);

This doesn't handle probe deferral correctly.
-EPROBE_DEFER from an earlier clock will be overwritten by -ENOENT from a
later clock, and the driver will never be reprobed.

> +}

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-11-02 14:27 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-14 12:14 [PATCH 00/14] SCI clocks cleanup Laurent Pinchart
2015-09-14 12:14 ` [PATCH 01/14] serial: sh-sci: Drop the interface clock Laurent Pinchart
2015-09-14 12:14   ` Laurent Pinchart
2015-11-02 14:27   ` Geert Uytterhoeven
2015-11-02 14:27     ` Geert Uytterhoeven
2015-09-14 12:26 ` [PATCH 00/14] SCI clocks cleanup Geert Uytterhoeven
2015-09-14 12:37 ` Laurent Pinchart
2015-09-14 12:46 ` Geert Uytterhoeven
2015-09-17  7:30 ` Simon Horman
2015-09-17  7:53 ` Simon Horman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.