| 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: <20120124105653.GU5446@pengutronix.de>
Date: Tue, 24 Jan 2012 11:56:53 +0100
From: Sascha Hauer <s.hauer@...gutronix.de>
To: Jaccon Bastiaansen <jaccon.bastiaansen@...il.com>
Cc: kernel@...gutronix.de, u.kleine-koenig@...gutronix.de,
davem@...emloft.net, cavokz@...il.com, linux@....linux.org.uk,
linux-arm-kernel@...ts.infradead.org, netdev@...r.kernel.org
Subject: Re: [PATCH V4 2/4] CS89x0 : add CS89x0 platform device to the
iMX21ADS board
On Tue, Jan 24, 2012 at 12:34:24AM +0100, Jaccon Bastiaansen wrote:
> Hello Sascha,
>
> 2012/1/23 Sascha Hauer <s.hauer@...gutronix.de>:
> > On Thu, Jan 19, 2012 at 07:13:50PM +0100, Jaccon Bastiaansen wrote:
> >> Add CS89x0 networking support to the iMX21ADS board by using the
> >> platform driver support in the CS89x0 driver.
> >>
> >> Signed-off-by: Jaccon Bastiaansen <jaccon.bastiaansen@...il.com>
> >> ---
> >> arch/arm/configs/imx_v4_v5_defconfig | 2 ++
> >> arch/arm/mach-imx/mach-mx21ads.c | 31 ++++++++++++++++++++++++++++---
> >> 2 files changed, 30 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/arm/configs/imx_v4_v5_defconfig b/arch/arm/configs/imx_v4_v5_defconfig
> >> index 11a4192..fc0106f 100644
> >> --- a/arch/arm/configs/imx_v4_v5_defconfig
> >> +++ b/arch/arm/configs/imx_v4_v5_defconfig
> >> @@ -81,6 +81,8 @@ CONFIG_NET_ETHERNET=y
> >> CONFIG_SMC91X=y
> >> CONFIG_DM9000=y
> >> CONFIG_SMC911X=y
> >> +CONFIG_CS89x0=y
> >> +CONFIG_CS89x0_PLATFORM=y
> >> # CONFIG_NETDEV_1000 is not set
> >> # CONFIG_NETDEV_10000 is not set
> >> # CONFIG_INPUT_MOUSEDEV is not set
> >> diff --git a/arch/arm/mach-imx/mach-mx21ads.c b/arch/arm/mach-imx/mach-mx21ads.c
> >> index 25f8402..4fd37fc 100644
> >> --- a/arch/arm/mach-imx/mach-mx21ads.c
> >> +++ b/arch/arm/mach-imx/mach-mx21ads.c
> >> @@ -37,8 +37,8 @@
> >> #define MX21ADS_REG_ADDR(offset) (void __force __iomem *) \
> >> (MX21ADS_MMIO_BASE_ADDR + (offset))
> >>
> >> +#define MX21ADS_CS8900A_MMIO_SIZE 0x200000
> >> #define MX21ADS_CS8900A_IRQ IRQ_GPIOE(11)
> >> -#define MX21ADS_CS8900A_IOBASE_REG MX21ADS_REG_ADDR(0x000000)
> >> #define MX21ADS_ST16C255_IOBASE_REG MX21ADS_REG_ADDR(0x200000)
> >> #define MX21ADS_VERSION_REG MX21ADS_REG_ADDR(0x400000)
> >> #define MX21ADS_IO_REG MX21ADS_REG_ADDR(0x800000)
> >> @@ -159,6 +159,26 @@ static struct platform_device mx21ads_nor_mtd_device = {
> >> .resource = &mx21ads_flash_resource,
> >> };
> >>
> >> +static const struct resource mx21ads_cs8900_resources[] __initconst = {
> >> + {
> >> + .start = MX21_CS1_BASE_ADDR,
> >> + .end = MX21_CS1_BASE_ADDR + MX21ADS_CS8900A_MMIO_SIZE - 1,
> >> + .flags = IORESOURCE_MEM,
> >> + },
> >> + {
> >> + .start = MX21ADS_CS8900A_IRQ,
> >> + .end = MX21ADS_CS8900A_IRQ,
> >> + .flags = IORESOURCE_IRQ,
> >> + }
> >> +};
> >> +
> >> +static const struct platform_device_info mx21ads_cs8900_devinfo __initconst = {
> >> + .name = "cs89x0",
> >> + .id = 0,
> >> + .res = mx21ads_cs8900_resources,
> >> + .num_res = ARRAY_SIZE(mx21ads_cs8900_resources),
> >> +};
> >> +
> >> static const struct imxuart_platform_data uart_pdata_rts __initconst = {
> >> .flags = IMXUART_HAVE_RTSCTS,
> >> };
> >> @@ -254,14 +274,17 @@ mx21ads_nand_board_info __initconst = {
> >> static struct map_desc mx21ads_io_desc[] __initdata = {
> >> /*
> >> * Memory-mapped I/O on MX21ADS Base board:
> >> - * - CS8900A Ethernet controller
> >> * - ST16C2552CJ UART
> >> * - CPU and Base board version
> >> * - Base board I/O register
> >> + *
> >> + * Note that the CS8900 uses the first MX21ADS_CS8900A_MMIO_SIZE
> >> + * bytes of the CS1 memory range.
> >> */
> >> {
> >> .virtual = MX21ADS_MMIO_BASE_ADDR,
> >> - .pfn = __phys_to_pfn(MX21_CS1_BASE_ADDR),
> >> + .pfn = __phys_to_pfn(MX21_CS1_BASE_ADDR +
> >> + MX21ADS_CS8900A_MMIO_SIZE),
> >
> > Since you now use ioremap the cs89x0 driver this mapping is not used
> > for the cs89x0 anymore, so it's strange to have references to it
> > here.
> > You still need the mapping though, but only for MX21ADS_IO_REG.
> >
>
> The ST16C255 UART is the first peripheral in the CS1 memory range for
> which this fixed mapping is now required. I could create another
> constant (for example: MX21ADS_ST16C255_OFFSET) to indicate the offset
> in the CS1 memory range where the mapping should start. But since that
> would create another constant with the same value as
> MX21ADS_CS8900A_MMIO_SIZE, I decided to use MX21ADS_CS8900A_MMIO_SIZE.
>
> Do you prefer to a constant with another name here?
Ok, looking again at it I understand what you are doing here and it
seems wrong. Instead of mapping MX21_CS1_BASE_ADDR you now map
MX21_CS1_BASE_ADDR + MX21ADS_CS8900A_MMIO_SIZE. This means that the
offset to MX21ADS_IO_REG is no longer 0x800000 but
0x800000 - MX21ADS_CS8900A_MMIO_SIZE.
Please just keep the mapping like it was before, it won't hurt.
BTW the 8250 serial driver also ioremaps the memory if you add
UPF_IOREMAP to the platform data.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists