lkml.org 
[lkml]   [2012]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [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@linaro.org>
    > >
    > > This patch includes basic dt support which can boot via nfs rootfs.
    > >
    > > Signed-off-by: Dong Aisheng <dong.aisheng@linaro.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@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. :)

    > > + 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.

    > > + * 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



    \
     
     \ /
      Last update: 2012-03-14 07:25    [W:0.030 / U:0.760 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site