Re: [PATCH 0/3] mtd: rawnand: Get rid of the cmx270 driver

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Robert Jarzmik <robert.jarzmik@free.fr>
Cc: Vignesh Raghavendra <vigneshr@ti.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Tudor Ambarus <tudor.ambarus@microchip.com>,
	Richard Weinberger <richard@nod.at>,
	Haojian Zhuang <haojian.zhuang@gmail.com>,
	linux-mtd@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Daniel Mack <daniel@zonque.org>
Subject: Re: [PATCH 0/3] mtd: rawnand: Get rid of the cmx270 driver
Date: Wed, 13 May 2020 15:17:37 +0200	[thread overview]
Message-ID: <20200513151737.757513cf@collabora.com> (raw)
In-Reply-To: <87lflwngey.fsf@belgarion.home>

Hi Robert,

On Wed, 13 May 2020 14:55:01 +0200
Robert Jarzmik <robert.jarzmik@free.fr> wrote:

> Miquel Raynal <miquel.raynal@bootlin.com> writes:
> 
> > Hi Robert,  
> 
> Mi Miquel,
> 
> >> I hope someone still has a board to test that.  
> No, unfortunately I don't have this board, nor do I know of anyone having
> one. It's the second time I see patches on cmx270, and the question to whether
> we shoud keep this board in kernel is still in my mind ... given that cm-x300 is
> fully supported and testable, and no one I know has a cm-x2700 ...

What's the point of keeping support for a board no one has or no one
cares about? I know I don't have my word in this decision, but I would
strongly recommend getting rid of it, especially when I see such
crappy/unmaintained code lurking around in the drivers/ tree.

> 
> Now for your series, I have 2 comments :
>  - dsb() : can you explain the rationale of each of the 3 instances I saw
>  please.

I didn't add any dsb(), just copied what was done before.

>  - the +2 IOMEM offset
>    I don't like it at all. I don't mind the offset, I disklike the use of
>    readb() or readw() where before there was a readl().. Same thing for writeb()
>    against writel().
> 
>    The bus semantics are not the same, the alignment is not the same as well
>    (and PXA is very old and doesn't cope well with alignment), and without a
>    proper board to test, I would be very wary to have that change.

Well, given the core uses {read,write}{b,w}() [1] to read/write data
and this driver doesn't provide its own
->{read_byte,write_buf,read_buf}() implementation, I'd expect things to
work just fine if we use byte/word accessors for the rest. This being
said, I'm fine switching back to 32bit accessors if that's a hard
requirement.

Thanks,

Boris

[1]https://elixir.bootlin.com/linux/v5.7-rc5/source/drivers/mtd/nand/raw/nand_legacy.c#L28

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel