netdev - Re: [PATCH V4 2/4] CS89x0 : add CS89x0 platform device to the iMX21ADS board

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20120125170622.GX5446@pengutronix.de>
Date:	Wed, 25 Jan 2012 18:06:22 +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 09:34:09PM +0100, Jaccon Bastiaansen wrote:
> >> >
> >> > 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
> >
> 
> You are right, this mapping is now wrong.
> 
> The reason I didn't keep the mapping as is, is because then the
> physical pages from MX21_CS1_BASE_ADDR up to MX21_CS1_BASE_ADDR +
> MX21ADS_CS8900A_MMIO_SIZE would be mapped twice to different virtual
> addresses (once by this static mapping and once by the ioremap in the
> cs89x0 driver).

This won't hurt, you will even get the same virtual address you have in
the static mapping returned from ioremap since this commit:

commit 576d2f2525612ecb5af029a76f21f22a3b82563d
Author: Nicolas Pitre <nicolas.pitre@...aro.org>
Date:   Fri Sep 16 01:14:23 2011 -0400

    ARM: add generic ioremap optimization by reusing static mappings
    
    Now that we have all the static mappings from iotable_init() located
    in the vmalloc area, it is trivial to optimize ioremap by reusing those
    static mappings when the requested physical area fits in one ofi them,
    and so in a generic way for all platforms.
    
    Signed-off-by: Nicolas Pitre <nicolas.pitre@...aro.org>
    Tested-by: Stephen Warren <swarren@...dia.com>
    Tested-by: Kevin Hilman <khilman@...com>
    Tested-by: Jamie Iles <jamie@...ieiles.com>



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