linux-kernel - Re: [PATCH 18/25] serial: sh-sci: Prepare for multiple clocks and baud rate generators

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdWZVZ-iVRKOjzszoJiBMxC-p=OTGWRb-EYHFV=eC3DiWQ@mail.gmail.com>
Date:	Fri, 20 Nov 2015 08:52:19 +0100
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:	Geert Uytterhoeven <geert+renesas@...der.be>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Simon Horman <horms@...ge.net.au>,
	Magnus Damm <magnus.damm@...il.com>,
	Yoshinori Sato <ysato@...rs.sourceforge.jp>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
	Linux-sh list <linux-sh@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 18/25] serial: sh-sci: Prepare for multiple clocks and
 baud rate generators

Hi Laurent,

On Thu, Nov 19, 2015 at 10:04 PM, Laurent Pinchart
<laurent.pinchart@...asonboard.com> 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 <geert+renesas@...der.be>
>> ---
>>  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@...ux-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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/