Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757082AbcDGTOe (ORCPT ); Thu, 7 Apr 2016 15:14:34 -0400 Received: from mail-lf0-f46.google.com ([209.85.215.46]:33588 "EHLO mail-lf0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751516AbcDGTOb (ORCPT ); Thu, 7 Apr 2016 15:14:31 -0400 Subject: Re: [PATCH] ARM: dts: r8a7791: Don't disable referenced optional clocks To: Sjoerd Simons , Simon Horman , Phil Edworthy References: <1459947173-6664-1-git-send-email-sjoerd.simons@collabora.co.uk> <57059874.1070309@cogentembedded.com> <1460012450.13819.38.camel@collabora.co.uk> Cc: linux-renesas-soc@vger.kernel.org, devicetree@vger.kernel.org, Geert Uytterhoeven , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org From: Sergei Shtylyov Organization: Cogent Embedded Message-ID: <5706B191.3060903@cogentembedded.com> Date: Thu, 7 Apr 2016 22:14:25 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1460012450.13819.38.camel@collabora.co.uk> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4181 Lines: 128 Hello. On 04/07/2016 10:00 AM, Sjoerd Simons wrote: > Hey Sergei, > > Thanks for your review. You're welcome. :-) > On Thu, 2016-04-07 at 02:15 +0300, Sergei Shtylyov wrote: >> On 04/06/2016 03:52 PM, Sjoerd Simons wrote: >> >>> >>> clk_get on a disabled clock node will return EPROBE_DEFER, which >>> can >>> cause drivers to be deferred forever if such clocks are referenced >>> in >>> their clocks property. >>> >>> Update the various disabled external clock nodes to default to a >>> frequency of 0, but don't disable them to prevent this. >>> >>> Signed-off-by: Sjoerd Simons >>> >>> --- >>> >>> arch/arm/boot/dts/r8a7791-koelsch.dts | 1 + >>> arch/arm/boot/dts/r8a7791-porter.dts | 1 + >>> arch/arm/boot/dts/r8a7791.dtsi | 5 +---- >>> 3 files changed, 3 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts >>> b/arch/arm/boot/dts/r8a7791-koelsch.dts >>> index 1adf877..da59c28 100644 >>> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts >>> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts >>> @@ -660,6 +660,7 @@ >>> }; >>> >>> &pcie_bus_clk { >>> + clock-frequency = <100000000>; >> Hmmm, looking at the Koelsch schematics, I don't see this clock. >> :-/ > > I don't have the schematics so i was simply keeping the current state. I understand. I was just surprised by what checking the code against the schematics revealed. > I've added Phil Edworthy to the list as he was the one originally I should've CC'ed Phil myself... > enable the bus clk for Koelsh according to git. Hopefully he can > clarify :) Let's hope he'd reply... >>> status = "okay"; >>> }; >>> >>> diff --git a/arch/arm/boot/dts/r8a7791-porter.dts >>> b/arch/arm/boot/dts/r8a7791-porter.dts >>> index 9554d13..19b257e 100644 >>> --- a/arch/arm/boot/dts/r8a7791-porter.dts >>> +++ b/arch/arm/boot/dts/r8a7791-porter.dts >>> @@ -413,6 +413,7 @@ >>> }; >>> >>> &pcie_bus_clk { >>> + clock-frequency = <100000000>; >>> status = "okay"; >>> }; >>> >> Again, looking at the Porter schematics, I don't see this clock >> either. :-/ > > You were the one enabling this clock for Porter ;) I don't have PCIE Yes, of course. I don't remember the gory details already -- I might have blindly copied what was in the Koelsch's .dts. > hardware to test with on my porter board, might be worth if you could > disable this clock and see if PCI-E still fucntions as expected (maybe > in practise it just happens to prefer the internal clock?) ? I know the PCIe driver does require this clock in order to function -- it would be the same eternal -EPROBE_DEFER condition you'd already described; see drivers/pci/host/pcie-rcar.c yourself... >>> diff --git a/arch/arm/boot/dts/r8a7791.dtsi >>> b/arch/arm/boot/dts/r8a7791.dtsi >>> index 8693888..676df63 100644 >>> --- a/arch/arm/boot/dts/r8a7791.dtsi >>> +++ b/arch/arm/boot/dts/r8a7791.dtsi >>> @@ -1104,8 +1104,7 @@ >>> pcie_bus_clk: pcie_bus { >>> compatible = "fixed-clock"; >>> #clock-cells = <0>; >>> - clock-frequency = <100000000>; >>> - status = "disabled"; >>> + clock-frequency = <0>; >> If the clock has a good default frequency, I don't think you need >> to >> remove it. Otherwise you missed USB_EXTAL which is 48 MHz (and can be >> overridden). > > I did that as it was by default disabled, so if i do enable it but > don't drop the default frequency to 0 board swithout that clock will > suddenly have it added to their dtb. Zero frequency is hardly any better then the default... > For the usb external clock I didn't touch it as it was already enabled > by default with a proper frequency, so it wouldn't hit the issue i was > trying to fix here. But i agree, both looking at other Renesas dtsis > and how all other external clocks are done in this dtsi, this node is > odd. It's not that odd given that the R-Car gen2 manual specifically says it should be 48 MHz. Looking into the manual again, the PCIe bus clock must be the onne presented on the CICREF[PN]1_SATA/PCIe input signals and it indeed should be 100 MHz... So Phil was correct. MBR, Sergei