linux-kernel - Re: [PATCH 2/5] ARM: dts: Device tree for AXM55xx.

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 22 Apr 2014 23:31:41 +0200
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Arnd Bergmann <arnd@...db.de>,
	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	Anders Berg <anders.berg@....com>,
	Olof Johansson <olof@...om.net>,
	Mike Turquette <mturquette@...aro.org>,
	Mark Rutland <mark.rutland@....com>,
	Dmitry Eremin-Solenikov <dbaryshkov@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/5] ARM: dts: Device tree for AXM55xx.

On Tue, Apr 15, 2014 at 2:45 PM, Arnd Bergmann <arnd@...db.de> wrote:
> On Tuesday 15 April 2014 14:06:11 Anders Berg wrote:

>> +                     serial0: uart@...0080000 {
>> +                             compatible = "arm,pl011", "arm,primecell";
>> +                             reg = <0x20 0x10080000 0 0x1000>;
>> +                             interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
>> +                             clocks = <&clk_per>, <&clk_per>;
>> +                             clock-names = "uartclk", "apb_pclk";
>> +                             status = "disabled";
>> +                     };
>
> "uartclk" is not a valid string for pl011, as per binding:
>
> | - clocks:  When present, must refer to exactly one clock named
> |           "apb_pclk"
>
> I do see that a lot of platforms do the same thing you have here, not
> sure who is wrong.

OK the PL011 is tricksier. It has two clocks, the APB clock driving
the logic and another clock input that is divided down to generate
the external data clock (baud rate).

The bus will do:
pcdev->pclk = clk_get(&pcdev->dev, "apb_pclk");

The driver will do:
uap->clk = devm_clk_get(&dev->dev, NULL);

So the driver relies on the anonymous clock being "first"
in a clock retrieveal operation, and it's a bit fragile indeed.
That leads to that either you give this clock a sensible
name or you list the empty string, as you have to have
clock-names there to name the last entry so the bus can
retrieve it.

This clock is *indeed* named "UARTCLK" (capital letters)
in the TRM for PL011. So I think this is perfectly reasonable.

The situation is the same as with the PL022 SPI driver we have
discussed before, it also retrieves the anonymous clock but we
consistently named it "SSPCLK" in commits:
dfc9832ca3f7a10d573e29e46d6ba03a54fbe580
80fbe30f63c9ba2ab62eb2c1f7ef607ce0721a95

As everyone else was using that. And it was the name from
the TRM.

Yours,
Linus Walleij
--
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/