linux-kernel - Re: [PATCH 23/25] arm64: renesas: r8a7795 dtsi: Add BRG support for (H)SCIF

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: <CAMuHMdXVuaWj_y6mLMi7+r5yXO2dn4i3k+Vr5qPrNc36sHWsWg@mail.gmail.com>
Date:	Fri, 20 Nov 2015 09:17:11 +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 23/25] arm64: renesas: r8a7795 dtsi: Add BRG support for (H)SCIF

Hi Laurent,

On Thu, Nov 19, 2015 at 10:07 PM, Laurent Pinchart
<laurent.pinchart@...asonboard.com> wrote:
> On Thursday 19 November 2015 19:39:02 Geert Uytterhoeven wrote:
>> Add the device node for the external SCIF_CLK.
>> The presence of the SCIF_CLK crystal and its clock frequency depend on
>> the actual board.
>>
>> Add the two optional clock sources (ZS_CLK and SCIF_CLK for the internal
>> resp. external clock) for the Baud Rate Generator for External Clock
>> (BRG) to all SCIF and HSCIF device nodes.
>>
>> This increases the range and accuracy of supported baud rates on
>> (H)SCIF.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@...der.be>
>> ---
>>  arch/arm64/boot/dts/renesas/r8a7795.dtsi | 74 +++++++++++++++++++----------
>>  1 file changed, 52 insertions(+), 22 deletions(-)
>>
>> diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index
>> 53a2a8fb42b7480c..25900761cfde201e 100644
>> --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi
>> @@ -84,6 +84,14 @@
>>               status = "disabled";
>>       };
>>
>> +     /* External SCIF clock - to be overridden by boards that provide it */
>> +     scif_clk: scif {
>> +             compatible = "fixed-clock";
>> +             #clock-cells = <0>;
>> +             clock-frequency = <0>;
>> +             status = "disabled";
>> +     };
>
> I have mixed feelings about this. Defining an external clock that isn't
> present on the board isn't very clean, even more so when the clock has such a

We have precedence of optional external clocks (can_clk, audio_clk_*).
The SoC datasheet clearly calls it "scif_clk", so that makes it an ABI, IMHO.

> generic name. Wouldn't it be better to let board files define the clock when
> they need it ? I know it would require board files to override the clocks and
> clock-names property too. Maybe we need to extend the DTS syntax to allow
> extending list properties instead of overriding them completely ?

As scif_clk is shared between all (H)SCIF instances, that would mean overriding
the clock and clock-names for all of them, which is quite a tedious task.
Most boards seem to provide a SCIF_CLK, to allow having "perfect" standard
baud rates.

Combined all of the above, I think it's sufficiently generic to keep
it that way.

Note that it's different for (H)SCK: these are per-(H)SCIF inputs, and depend
even more on board layout. Adding individual zero-frequency clock nodes for
them would preclude e.g. connecting all (H)SCK inputs to the same crystal.
Hence I didn't add them, and you do have to override all clocks and
clock-names of a node if you want to add an (H)SCK clock input (been there,
done that for testing; long live DT overlays).

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/