From mboxrd@z Thu Jan 1 00:00:00 1970 From: Geert Uytterhoeven Date: Fri, 20 Nov 2015 07:52:19 +0000 Subject: Re: [PATCH 18/25] serial: sh-sci: Prepare for multiple clocks and baud rate generators Message-Id: List-Id: References: <1447958344-836-1-git-send-email-geert+renesas@glider.be> <1447958344-836-19-git-send-email-geert+renesas@glider.be> <3383903.1R72kZGx62@avalon> In-Reply-To: <3383903.1R72kZGx62@avalon> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Laurent Pinchart Cc: Geert Uytterhoeven , Greg Kroah-Hartman , Simon Horman , Magnus Damm , Yoshinori Sato , "linux-serial@vger.kernel.org" , Linux-sh list , "linux-kernel@vger.kernel.org" Hi Laurent, On Thu, Nov 19, 2015 at 10:04 PM, Laurent Pinchart wrote: > On Thursday 19 November 2015 19:38:57 Geert Uytterhoeven wrote: >> Refactor the clock and baud rate parameter code to ease adding support >> for multiple clocks and baud rate generators later. >> sci_scbrr_calc() now returns the bit rate error, so it can be compared >> to the bit rate error for other baud rate generators. >> >> Signed-off-by: Geert Uytterhoeven >> --- >> drivers/tty/serial/sh-sci.c | 176 +++++++++++++++++++++++++++-------------- >> 1 file changed, 120 insertions(+), 56 deletions(-) >> >> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c >> index 726c96d5a511c222..12800e52f41953dc 100644 >> --- a/drivers/tty/serial/sh-sci.c >> +++ b/drivers/tty/serial/sh-sci.c >> @@ -2252,33 +2301,48 @@ static struct uart_ops sci_uart_ops = { >> >> 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 = devm_clk_get(dev, "fck"); >> - if (PTR_ERR(sci_port->fclk) = -EPROBE_DEFER) >> - return -EPROBE_DEFER; >> - if (!IS_ERR(sci_port->fclk)) >> - return 0; >> + const char *clk_names[] = { >> + [SCI_FCK] = "fck", >> + }; >> + struct clk *clk; >> + unsigned int i; >> >> - /* >> - * But it used to be called "sci_ick", and we need to maintain DT >> - * backward compatibility. >> - */ >> - sci_port->fclk = devm_clk_get(dev, "sci_ick"); >> - if (PTR_ERR(sci_port->fclk) = -EPROBE_DEFER) >> - return -EPROBE_DEFER; >> - if (!IS_ERR(sci_port->fclk)) >> - return 0; >> + for (i = 0; i < SCI_NUM_CLKS; i++) { >> + clk = devm_clk_get(dev, clk_names[i]); >> + if (PTR_ERR(clk) = -EPROBE_DEFER) >> + return -EPROBE_DEFER; >> >> - /* >> - * 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 = devm_clk_get(dev, "peripheral_clk"); >> - if (!IS_ERR(sci_port->fclk)) >> - return 0; >> + if (IS_ERR(clk) && i = SCI_FCK) { >> + /* >> + * "fck" used to be called "sci_ick", and we need to >> + * maintain DT backward compatibility. >> + */ >> + clk = devm_clk_get(dev, "sci_ick"); >> + if (PTR_ERR(clk) = -EPROBE_DEFER) >> + return -EPROBE_DEFER; >> + >> + if (!IS_ERR(clk)) >> + goto found; >> + >> + /* >> + * Not all SH platforms declare a clock lookup entry >> + * for SCI devices, in which case we need to get the >> + * global "peripheral_clk" clock. >> + */ >> + clk = devm_clk_get(dev, "peripheral_clk"); >> + if (!IS_ERR(clk)) >> + goto found; >> + >> + dev_err(dev, "failed to get functional clock\n"); >> + return PTR_ERR(clk); >> + } >> >> - dev_err(dev, "failed to get functional clock\n"); >> - return PTR_ERR(sci_port->fclk); >> +found: >> + if (!IS_ERR(clk)) >> + dev_dbg(dev, "clk %u is %pC rate %pCr\n", i, clk, clk); >> + sci_port->clks[i] = IS_ERR(clk) ? NULL : clk; > > Isn't it an issue that we can't tell apart the case where there is no clock > specified in DT and the case where we can't get the clock due to another error > ? All failures here are for optional clocks. If the real failure is that the clock wasn't specified (or misspelled) in DT, it should have been detected during the integration phase. 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759358AbbKTHwX (ORCPT ); Fri, 20 Nov 2015 02:52:23 -0500 Received: from mail-ob0-f175.google.com ([209.85.214.175]:34989 "EHLO mail-ob0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754664AbbKTHwU (ORCPT ); Fri, 20 Nov 2015 02:52:20 -0500 MIME-Version: 1.0 In-Reply-To: <3383903.1R72kZGx62@avalon> References: <1447958344-836-1-git-send-email-geert+renesas@glider.be> <1447958344-836-19-git-send-email-geert+renesas@glider.be> <3383903.1R72kZGx62@avalon> Date: Fri, 20 Nov 2015 08:52:19 +0100 X-Google-Sender-Auth: U1h-ce6hDDqh9KLsD0LovsAskpY Message-ID: Subject: Re: [PATCH 18/25] serial: sh-sci: Prepare for multiple clocks and baud rate generators From: Geert Uytterhoeven To: Laurent Pinchart Cc: Geert Uytterhoeven , Greg Kroah-Hartman , Simon Horman , Magnus Damm , Yoshinori Sato , "linux-serial@vger.kernel.org" , Linux-sh list , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Laurent, On Thu, Nov 19, 2015 at 10:04 PM, Laurent Pinchart wrote: > On Thursday 19 November 2015 19:38:57 Geert Uytterhoeven wrote: >> Refactor the clock and baud rate parameter code to ease adding support >> for multiple clocks and baud rate generators later. >> sci_scbrr_calc() now returns the bit rate error, so it can be compared >> to the bit rate error for other baud rate generators. >> >> Signed-off-by: Geert Uytterhoeven >> --- >> drivers/tty/serial/sh-sci.c | 176 +++++++++++++++++++++++++++-------------- >> 1 file changed, 120 insertions(+), 56 deletions(-) >> >> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c >> index 726c96d5a511c222..12800e52f41953dc 100644 >> --- a/drivers/tty/serial/sh-sci.c >> +++ b/drivers/tty/serial/sh-sci.c >> @@ -2252,33 +2301,48 @@ static struct uart_ops sci_uart_ops = { >> >> 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 = devm_clk_get(dev, "fck"); >> - if (PTR_ERR(sci_port->fclk) == -EPROBE_DEFER) >> - return -EPROBE_DEFER; >> - if (!IS_ERR(sci_port->fclk)) >> - return 0; >> + const char *clk_names[] = { >> + [SCI_FCK] = "fck", >> + }; >> + struct clk *clk; >> + unsigned int i; >> >> - /* >> - * But it used to be called "sci_ick", and we need to maintain DT >> - * backward compatibility. >> - */ >> - sci_port->fclk = devm_clk_get(dev, "sci_ick"); >> - if (PTR_ERR(sci_port->fclk) == -EPROBE_DEFER) >> - return -EPROBE_DEFER; >> - if (!IS_ERR(sci_port->fclk)) >> - return 0; >> + for (i = 0; i < SCI_NUM_CLKS; i++) { >> + clk = devm_clk_get(dev, clk_names[i]); >> + if (PTR_ERR(clk) == -EPROBE_DEFER) >> + return -EPROBE_DEFER; >> >> - /* >> - * 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 = devm_clk_get(dev, "peripheral_clk"); >> - if (!IS_ERR(sci_port->fclk)) >> - return 0; >> + if (IS_ERR(clk) && i == SCI_FCK) { >> + /* >> + * "fck" used to be called "sci_ick", and we need to >> + * maintain DT backward compatibility. >> + */ >> + clk = devm_clk_get(dev, "sci_ick"); >> + if (PTR_ERR(clk) == -EPROBE_DEFER) >> + return -EPROBE_DEFER; >> + >> + if (!IS_ERR(clk)) >> + goto found; >> + >> + /* >> + * Not all SH platforms declare a clock lookup entry >> + * for SCI devices, in which case we need to get the >> + * global "peripheral_clk" clock. >> + */ >> + clk = devm_clk_get(dev, "peripheral_clk"); >> + if (!IS_ERR(clk)) >> + goto found; >> + >> + dev_err(dev, "failed to get functional clock\n"); >> + return PTR_ERR(clk); >> + } >> >> - dev_err(dev, "failed to get functional clock\n"); >> - return PTR_ERR(sci_port->fclk); >> +found: >> + if (!IS_ERR(clk)) >> + dev_dbg(dev, "clk %u is %pC rate %pCr\n", i, clk, clk); >> + sci_port->clks[i] = IS_ERR(clk) ? NULL : clk; > > Isn't it an issue that we can't tell apart the case where there is no clock > specified in DT and the case where we can't get the clock due to another error > ? All failures here are for optional clocks. If the real failure is that the clock wasn't specified (or misspelled) in DT, it should have been detected during the integration phase. 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