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

[Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>]

Hi Robert,

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

> Miquel Raynal <miquel.raynal@xxxxxxxxxxx> 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 MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/