| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <4F60979E.3060704@gmail.com>
Date: Wed, 14 Mar 2012 08:05:34 -0500
From: Rob Herring <robherring2@...il.com>
To: Dong Aisheng <aisheng.dong@...escale.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 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@...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. :)
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.
>>> + 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.
Okay either way, but my vote would be add it as needed.
>>> + * 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.
Rob
>
>>> + { /* 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