Re: [PATCH v1 1/5] ARM: imx28: add basic dt support

From: Dong Aisheng
Date: Wed Mar 14 2012 - 22:55:38 EST


On Wed, Mar 14, 2012 at 09:05:34PM +0800, Rob Herring wrote:
> On 03/14/2012 01:23 AM, Dong Aisheng wrote:
> > 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@xxxxxxxxxx>
> >>>
> >>> This patch includes basic dt support which can boot via nfs rootfs.
> >>>
> >>> Signed-off-by: Dong Aisheng <dong.aisheng@xxxxxxxxxx>
> >>> ---
> >>> 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@80080000 {
> >>> + compatible = "simple-bus";
> >>> + #address-cells = <1>;
> >>> + #size-cells = <1>;
> >>> + reg = <0x80080000 0x80000>;
> >>> + ranges;
> >>> +
> >>> + fec@800f0000 {
> >>> + compatible = "fsl,imx28-fec";
> >>> + reg = <0x800f0000 0x4000>;
> >>
> >> This too IIRC.
> >>
> > The size is 16KB/0x4000 in iMX28 spec. :)
>
> Yes, but the module only uses <4KB. You waste virtual memory space by
> mapping all the unused area. It's not really an issue when you have
> <512MB of RAM, but does become an issue.
>
Yes, it seems an issue of spec.
And all other places are using the same size.
The simplest way now may be just keep align with spec first.

> >>> + interrupts = <101>;
> >>> + status = "disabled";
> >>> + };
> >>> +
> >>> + fec@800f4000 {
> >>> + 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.
>
> Okay either way, but my vote would be add it as needed.
>
Your vote is ok to me.

> >>> + * 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?
>
> Yes, you can add more clock look-ups with the DT name.
>
Got it.

Regards
Dong Aisheng

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/