[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120314062354.GA8494@shlinux2.ap.freescale.net>
Date: Wed, 14 Mar 2012 14:23:55 +0800
From: Dong Aisheng <aisheng.dong@...escale.com>
To: Rob Herring <robherring2@...il.com>
CC: Dong Aisheng <b29396@...escale.com>,
<devicetree-discuss@...ts.ozlabs.org>,
<linux-kernel@...r.kernel.org>, <linux-mmc@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<vinod.koul@...ux.intel.com>, <s.hauer@...gutronix.de>,
<rob.herring@...xeda.com>, <grant.likely@...retlab.ca>,
<rdunlap@...otime.net>, <kernel@...gutronix.de>, <cjb@...top.org>,
<shawn.guo@...escale.com>
Subject: Re: [PATCH v1 1/5] ARM: imx28: add basic dt support
On Tue, Mar 13, 2012 at 09:35:43AM -0500, Rob Herring wrote:
> On 03/13/2012 03:47 AM, Dong Aisheng wrote:
> > From: Dong Aisheng <dong.aisheng@...aro.org>
> >
> > This patch includes basic dt support which can boot via nfs rootfs.
> >
> > Signed-off-by: Dong Aisheng <dong.aisheng@...aro.org>
> > ---
> > Documentation/devicetree/bindings/arm/fsl.txt | 4 +
> > arch/arm/boot/dts/imx28-evk.dts | 31 +++++++++
> > arch/arm/boot/dts/imx28.dtsi | 88 +++++++++++++++++++++++++
> > arch/arm/mach-mxs/Kconfig | 9 +++
> > arch/arm/mach-mxs/Makefile | 1 +
> > arch/arm/mach-mxs/imx28-dt.c | 67 +++++++++++++++++++
> > 6 files changed, 200 insertions(+), 0 deletions(-)
....
> > + interrupts = <47>;
> > + };
> > + };
> > + };
> > +
> > + ahb@...80000 {
> > + compatible = "simple-bus";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + reg = <0x80080000 0x80000>;
> > + ranges;
> > +
> > + fec@...f0000 {
> > + compatible = "fsl,imx28-fec";
> > + reg = <0x800f0000 0x4000>;
>
> This too IIRC.
>
The size is 16KB/0x4000 in iMX28 spec. :)
> > + interrupts = <101>;
> > + status = "disabled";
> > + };
> > +
> > + fec@...f4000 {
> > + compatible = "fsl,imx28-fec";
> > + reg = <0x800f4000 0x4000>;
> > + interrupts = <102>;
> > + status = "disabled";
> > + };
> > +
> > + };
> > +};
> > diff --git a/arch/arm/mach-mxs/Kconfig b/arch/arm/mach-mxs/Kconfig
> > index c57f996..6ab66af 100644
> > --- a/arch/arm/mach-mxs/Kconfig
> > +++ b/arch/arm/mach-mxs/Kconfig
> > @@ -17,6 +17,15 @@ config SOC_IMX28
> >
> > comment "MXS platforms:"
> >
> > +config MACH_IMX28_DT
>
> Perhaps this should be more generic like MACH_MXS_DT to eventually
> support MX23 as well?
>
I just did like the imx ways.
But yes if we see the need when do imx23 dt support.
> > + bool "Support i.MX28 platforms from device tree"
> > + select SOC_IMX28
> > + select USE_OF
> > + select MACH_MX28EVK
>
> This should not be selected here.
>
Like other imx soc dt board files, the imx28-dt.c may need to use some bits
like pinmux in mx28evk.c board file.
Yes, currently i can remove it since it is using uboot pinmux setting.
But when add other devices support which is not covered in u-boot like flexcan,
i may need to use non-dt board's pinmux setting.
So maybe we can keep it first and remove it when totally not dependent
on non-dt files.
> > + * The code contained herein is licensed under the GNU General Public
> > + * License. You may obtain a copy of the GNU General Public License
> > + * Version 2 or later at the following locations:
> > + *
> > + * http://www.opensource.org/licenses/gpl-license.html
> > + * http://www.gnu.org/copyleft/gpl.html
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/irqdomain.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> > +#include <asm/mach/arch.h>
> > +#include <asm/mach/time.h>
> > +#include <mach/common.h>
> > +#include <mach/mx28.h>
> > +
> > +static const struct of_dev_auxdata imx28_auxdata_lookup[] __initconst = {
> > + OF_DEV_AUXDATA("arm,pl011", MX28_DUART_BASE_ADDR, "duart", NULL),
> > + OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC0_BASE_ADDR, "imx28-fec.0", NULL),
> > + OF_DEV_AUXDATA("fsl,imx28-fec", MX28_ENET_MAC1_BASE_ADDR, "imx28-fec.1", NULL),
>
> Why do you need the old names?
>
To keep align with the old clocks.
See arch/arm/mach-mxs/clock-mx28.c:
_REGISTER_CLOCK("imx28-fec.0", NULL, fec_clk)
_REGISTER_CLOCK("imx28-fec.1", NULL, fec_clk)
Is there any better way to avoid this?
> > + { /* sentinel */ }
> > +};
> > +
> > +static void __init imx28_init(void)
> > +{
> > + of_platform_populate(NULL, of_default_bus_match_table,
> > + imx28_auxdata_lookup, NULL);
> > +}
> > +
> > +static const struct of_device_id icoll_of_match[] __initconst = {
> > + { .compatible = "fsl,imx28-icoll", },
> > + {},
> > +};
> > +
> > +static void __init imx28_dt_init_irq(void)
> > +{
> > + irq_domain_generate_simple(icoll_of_match, MX28_ICOLL_BASE_ADDR, 0);
>
> Please do "proper" domain support for the interrupt controller and use
> of_irq_init.
>
Ok, i will see it.
For others, will fix them all.
Thanks for the review.
Regards
Dong Aisheng
--
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/
Powered by blists - more mailing lists