From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753132AbbJMViY (ORCPT ); Tue, 13 Oct 2015 17:38:24 -0400 Received: from mail-gw2-out.broadcom.com ([216.31.210.63]:55139 "EHLO mail-gw2-out.broadcom.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752197AbbJMViW (ORCPT ); Tue, 13 Oct 2015 17:38:22 -0400 X-IronPort-AV: E=Sophos;i="5.17,680,1437462000"; d="scan'208";a="77552865" Date: Tue, 13 Oct 2015 17:38:18 -0400 From: Jon Mason To: Hauke Mehrtens CC: Florian Fainelli , Rob Herring , Pawel Moll , Mark Rutland , "Ian Campbell" , Kumar Gala , Russell King , , , , , =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= Subject: Re: [PATCH 1/2] ARM: dts: bcm5301x: Add BCM SVK DT files Message-ID: <20151013213818.GM12512@broadcom.com> References: <1443824564-17291-1-git-send-email-jonmason@broadcom.com> <56192304.1050500@hauke-m.de> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <56192304.1050500@hauke-m.de> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Oct 10, 2015 at 04:39:00PM +0200, Hauke Mehrtens wrote: > On 10/03/2015 12:22 AM, Jon Mason wrote: > > Add device tree files for Broadcom Northstar based SVKs. Since the > > bcm5301x.dtsi already exists, all that is necessary is the dts files to > > enable the UARTs (and specify the RAM size for the 4708/9). With these > > files, the SVKs are able to boot to shell. > > > > Signed-off-by: Jon Mason > > --- > > arch/arm/boot/dts/Makefile | 5 +++- > > arch/arm/boot/dts/bcm94708.dts | 52 +++++++++++++++++++++++++++++++++++ > > arch/arm/boot/dts/bcm94709.dts | 52 +++++++++++++++++++++++++++++++++++ > > arch/arm/boot/dts/bcm953012k.dts | 59 ++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 167 insertions(+), 1 deletion(-) > > create mode 100644 arch/arm/boot/dts/bcm94708.dts > > create mode 100644 arch/arm/boot/dts/bcm94709.dts > > create mode 100644 arch/arm/boot/dts/bcm953012k.dts > > > > diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile > > index 233159d..96a1b58 100644 > > --- a/arch/arm/boot/dts/Makefile > > +++ b/arch/arm/boot/dts/Makefile > > @@ -72,7 +72,10 @@ dtb-$(CONFIG_ARCH_BCM_5301X) += \ > > bcm47081-buffalo-wzr-900dhp.dtb \ > > bcm4709-asus-rt-ac87u.dtb \ > > bcm4709-buffalo-wxr-1900dhp.dtb \ > > - bcm4709-netgear-r8000.dtb > > + bcm4709-netgear-r8000.dtb \ > > + bcm94708.dtb \ > > + bcm94709.dtb \ > > + bcm953012k.dtb > > dtb-$(CONFIG_ARCH_BCM_63XX) += \ > > bcm963138dvt.dtb > > dtb-$(CONFIG_ARCH_BCM_CYGNUS) += \ > > diff --git a/arch/arm/boot/dts/bcm94708.dts b/arch/arm/boot/dts/bcm94708.dts > > new file mode 100644 > > index 0000000..57a74c6 > > --- /dev/null > > +++ b/arch/arm/boot/dts/bcm94708.dts > > Currently the files are named beginning with the chip name something > like this should fit better the other naming schema: > arch/arm/boot/dts/bcm4708-brcm-bcm94708.dts Sorry, I was following the namign schema that we've been using on other Broadcom SVKs in the Linux device tree directory. For example, arch/arm/boot/dts/bcm911360_entphn.dts arch/arm/boot/dts/bcm911360k.dts arch/arm/boot/dts/bcm958300k.dts arch/arm/boot/dts/bcm958305k.dts arch/arm/boot/dts/bcm958625k.dts arch/arm/boot/dts/bcm963138dvt.dts Also, this is the name we have been using internally. So, it will making migrating to newer kernels much easier. > > > @@ -0,0 +1,52 @@ > > +/* > > + * BSD LICENSE > > + * > > + * Copyright(c) 2015 Broadcom Corporation. All rights reserved. > > + * > > + * Redistribution and use in source and binary forms, with or without > > + * modification, are permitted provided that the following conditions > > + * are met: > > + * > > + * * Redistributions of source code must retain the above copyright > > + * notice, this list of conditions and the following disclaimer. > > + * * Redistributions in binary form must reproduce the above copyright > > + * notice, this list of conditions and the following disclaimer in > > + * the documentation and/or other materials provided with the > > + * distribution. > > + * * Neither the name of Broadcom Corporation nor the names of its > > + * contributors may be used to endorse or promote products derived > > + * from this software without specific prior written permission. > > + * > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT > > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > + */ > > + > > +/dts-v1/; > > + > > +#include "bcm5301x.dtsi" > > + > > +/ { > > + model = "NorthStar SVK (BCM94708)"; > > + compatible = "brcm,bcm4708"; > > The compatible string should also contain this exactly board like this: > compatible = "brcm,bcm94708", "brcm,bcm4708"; Ok > > > + > > + aliases { > > + serial0 = &uart0; > > + }; > > + > > + chosen { > > + bootargs = "console=ttyS0,115200 mem=128M"; > > Instead of using mem=128M you should add an extra node like this: > > memory { > reg = <0x00000000 0x08000000>; > }; > > Does this board only has 128M of RAM? currently we restrict all of them > to that, but I just want to know. The 4708/9 SVKs have 128M, but the 53012 has 256M. > > > + }; > > +}; > > + > > +&uart0 { > > + status = "okay"; > > +}; > > diff --git a/arch/arm/boot/dts/bcm94709.dts b/arch/arm/boot/dts/bcm94709.dts > > new file mode 100644 > > index 0000000..bf4b6e1 > > --- /dev/null > > +++ b/arch/arm/boot/dts/bcm94709.dts > > @@ -0,0 +1,52 @@ > > +/* > > + * BSD LICENSE > > + * > > + * Copyright(c) 2015 Broadcom Corporation. All rights reserved. > > + * > > + * Redistribution and use in source and binary forms, with or without > > + * modification, are permitted provided that the following conditions > > + * are met: > > + * > > + * * Redistributions of source code must retain the above copyright > > + * notice, this list of conditions and the following disclaimer. > > + * * Redistributions in binary form must reproduce the above copyright > > + * notice, this list of conditions and the following disclaimer in > > + * the documentation and/or other materials provided with the > > + * distribution. > > + * * Neither the name of Broadcom Corporation nor the names of its > > + * contributors may be used to endorse or promote products derived > > + * from this software without specific prior written permission. > > + * > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT > > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > + */ > > + > > +/dts-v1/; > > + > > +#include "bcm5301x.dtsi" > > + > > +/ { > > + model = "NorthStar SVK (BCM94709)"; > > + compatible = "brcm,bcm4709", "brcm,bcm4708"; > Better this: > compatible = "brcm,bcm94709", "brcm,bcm4709", "brcm,bcm4708"; It seems to me that this who 4708 base is kind of a misnomer. They really are "Northstar". Would it not be more accurate to change it to be compatible = "brcm,bcm4709", "brcm,ns"; ? > > > + > > + aliases { > > + serial0 = &uart0; > > + }; > > + > > + chosen { > > + bootargs = "console=ttyS0,115200 mem=128M"; > same as above. Ok > > + }; > > +}; > > + > > +&uart0 { > > + status = "okay"; > > +}; > > diff --git a/arch/arm/boot/dts/bcm953012k.dts b/arch/arm/boot/dts/bcm953012k.dts > > new file mode 100644 > > index 0000000..1726ba0 > > --- /dev/null > > +++ b/arch/arm/boot/dts/bcm953012k.dts > > @@ -0,0 +1,59 @@ > > +/* > > + * BSD LICENSE > > + * > > + * Copyright(c) 2015 Broadcom Corporation. All rights reserved. > > + * > > + * Redistribution and use in source and binary forms, with or without > > + * modification, are permitted provided that the following conditions > > + * are met: > > + * > > + * * Redistributions of source code must retain the above copyright > > + * notice, this list of conditions and the following disclaimer. > > + * * Redistributions in binary form must reproduce the above copyright > > + * notice, this list of conditions and the following disclaimer in > > + * the documentation and/or other materials provided with the > > + * distribution. > > + * * Neither the name of Broadcom Corporation nor the names of its > > + * contributors may be used to endorse or promote products derived > > + * from this software without specific prior written permission. > > + * > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS > > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR > > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT > > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, > > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, > > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY > > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT > > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE > > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > + */ > > + > > +/dts-v1/; > > + > > +#include "bcm5301x.dtsi" > > + > > +/ { > > + model = "NorthStar SVK (BCM953012K)"; > > + compatible = "brcm,bcm5301k", "brcm,bcm4708"; > > same as other file. > > > + > > + aliases { > > + serial0 = &uart0; > > + serial1 = &uart1; > > + }; > > + > > + chosen { > > + bootargs = "console=ttyS0,115200"; > > Why is no memory size given here at all? I'll add the 256M. > > > + }; > > +}; > > + > > +&uart0 { > > + clock-frequency = <62499840>; > > Just out of curiosity on what does this clock frequency depend? I saw > your patch adding a real clock driver which should make this obsolete. Better to add this now, as the device tree part might be out of sync for a time. Also, is it not better to make the UARTs a static clock and not dependent on the clk driver? Thanks, Jon > > + status = "okay"; > > +}; > > + > > +&uart1 { > > + clock-frequency = <62499840>; > > + status = "okay"; > > +}; > > >