From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752659AbcAGOqM (ORCPT ); Thu, 7 Jan 2016 09:46:12 -0500 Received: from mout.kundenserver.de ([212.227.126.187]:59092 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750722AbcAGOqI (ORCPT ); Thu, 7 Jan 2016 09:46:08 -0500 From: Arnd Bergmann To: Jon Mason Cc: linux-arm-kernel@lists.infradead.org, Florian Fainelli , Hauke Mehrtens , bcm-kernel-feedback-list@broadcom.com, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org Subject: Re: [PATCH 3/3] arm: dts: bcm5301x: Add syscon based reboot in DT Date: Thu, 07 Jan 2016 15:45:32 +0100 Message-ID: <2695666.f4UPYAJLqz@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20160105222605.GL31867@broadcom.com> References: <1450474676-10210-1-git-send-email-jonmason@broadcom.com> <2180042.V6T2Tnxp6B@wuerfel> <20160105222605.GL31867@broadcom.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:0W4ca0eKCvxR9tJayTpTRcWhAat7xMEsqoRXZDvwnQrHn8rUmZY Cbi0GUPJnMI0EHSiL/R9y/QP9tTFrdJ9H9sTHlkTbvscYRGg7AzP/uXEKGJcJy1nEMkQKKw OVRMz4ujs6AysgUR8/a7NHEktwSQ+NpVn6oxlQ9veYDK22SzMMRB6Xyy2Vx8q3tj+AIOJoq ad9RFahV7jDRur+3iisPg== X-UI-Out-Filterresults: notjunk:1;V01:K0:leqit/rkCNY=:10UPvNNjVEtkhgrI1hw3xx wMRSj/GQYL/iVebBIIzUEKxqOKPg5DAvGggHR8VG034KEbU7CfCmpIQz3qZXs2oOROdscBIHt NKqhTnkZk9IBaHIKa9HgTmNU8zYEENaFSHS9yPe2mr2hWrjjVcwfDtzjLQCqI61HpLf8meOoX aPscz9DROjkOu47X+YrhqvUHlUVZsxus0B6XUI2z5NF4BX4QG7h/giZHaOnyOSdIhUM0G14vQ sB2UW/xVTnm3ll0KeStgbWg9b8N/aWuA7q7Ki52EwJiWC+vAICyWIlMcgHK/GRs2kN0JvYRzR ri2o/xBbbIAqfTYchFXM1Hd3Kc44+0dTfVuFOr7QprTiEz5mJAjSOA/BLtMGTzuyoSSh4b5Qy y0sfIhgsf2aL2jZdbeiadF1fdhXVfcAsyzbZWy//0UBtQIJZZtqLal4Boe+3Y4AyHZXM5RjCr Sl6EQ+UH8dcH6YKM19p5ATN/GY3k4ViI5A6pC6Q9mCFpDVgjiY2hYzRUHNtkTejA1zF6Ia3Ay mbB5LqUqP+//1vi62DDWxXydX6TykniT0eVQimzVyzk7tt0BkCCi4wedJiPgCo0dUmFh42R87 Q9UMzexjyZK0Pu8UpbtRg108xwSVhsnc1R2Fzurb9XgGzh7+2tyOB708sq3zTNZVJjHe3Pr7b nYpZuY5/QOFrK9ovb12SJaopIQ8sMVg9TzvrDIJ4OikqgMA6ZPFRqagpIxcpgc9ZODg1uD8XC JVA14lkt9HymLYNZ Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tuesday 05 January 2016 17:26:06 Jon Mason wrote: > On Fri, Dec 18, 2015 at 10:44:28PM +0100, Arnd Bergmann wrote: > > On Friday 18 December 2015 16:37:56 Jon Mason wrote: > > > + cru: cru@1800c184 { > > > + compatible = "syscon"; > > > + reg = <0x1800c184 0xc>; > > > + }; > > > > It's unusual for a device to start at such an odd address. Are you sure > > it's not a larger device starting at 0x1800c000 or 0x18000000? > > The CRU (Clock and Reset Unit) starts at 0x1800c100, with the > following layout: > > CRU Clock Management at 0x1800c100-0x1800c180 > CRU Reset at 0x1800c184 > CRU Period Sample Clock at 0x1800c188 > CRU Interrupt register at 0x1800c18c > CRU MDIO Control at 0x1800c190 > CRU GPIO at 0x1800c1c0-0x1800c1e0 > CRU SDIO 0x1800c200-0x1800c214 > CRU RoboSW Interrupt at 0x1800c280 > CRU Straps Control at 0x1800c2a0 > > The clock driver is already referencing the registers between > 0x1800c100-0x1800c180, and the GPIO driver is referencing registers > between 0x1800c1c0-0x1800c1e0. > > The reset part of the syscon seems to be the only useful thing in this > block. Am I approaching this incorrectly? I think the problem is how the gpio controller has a device node that overlaps with one of the devices of the chip. If the data sheet lists a "Clock and Reset Unit" at those addresses, that is a strong indication that this is actually a specific piece of hardware, and it should be represented as one device node in DT, with the sub-registers being exposed by that driver in some way. A typical way would be to have a syscon node like cru: syscon@1800c100 { compatible = "brcm,bcm53010-clock-reset-unit", "syscon"; reg = <0x1800c100 100>; }; that represents the entire unit. You can then have syscon references in each driver that uses it, and/or create a high-level driver that binds to the "brcm,bcm53010-clock-reset-unit" compatible string and that exports a set of functions for other drivers to be used if you prefer to do this as functional abstraction rather than register based. > > Also, please provide a more specific compatible string based on the > > name of the device in the data sheet. The node name in contrast should > > be more generic, e.g. > > > > cru: system-controller@1800c000 { > > compatible = "brcm,bcm53010-cru", "syscon"; > > This is very similar between the NS and NSP (and NS2) platforms. I'll > verify the layout and see if this can't be "brcm,iproc-cru" or > something similar. If it's only "very similar" but not identical, don't use the same compatible string, at least not as the only one. You should be able to identify the specific device by looking at its compatible string in case you want to write a high-level driver that knows about the differences later (the driver should not need to inquire the chip name, it should only look at one device node). Arnd