| Message ID | 20200429223134.789322-1-boris.brezillon@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | mtd: rawnand: Get rid of the cmx270 driver | expand |
Hi Robert, Boris Brezillon <boris.brezillon@collabora.com> wrote on Thu, 30 Apr 2020 00:31:31 +0200: > Hello, > > As part of my attempt to convert all NAND drivers to exec_op() I noticed > the cmx270 board didn't really deserve a custom driver since other boards > using the same SoC (em-x270 to name one) are using the gen_nand driver. > I think the only issue with the CM-X270 is that the chip is connected > to D[16:23] (or D[16:31] if it's a 16bit bus). Adjusting the mem > resource offset should do the trick. > > I hope someone still has a board to test that. > > Regards, > > Boris > > Boris Brezillon (3): > ARM: pxa: cm-x270: Use gen_nand to expose the NAND device > ARM: pxa: Stop selecting CONFIG_MTD_NAND_CM_X270 > mtd: rawnand: Remove the cmx270 NAND controller driver > > arch/arm/configs/cm_x2xx_defconfig | 1 - > arch/arm/configs/pxa_defconfig | 1 - > arch/arm/mach-pxa/cm-x270.c | 131 ++++++++++++++++ > drivers/mtd/nand/raw/Kconfig | 4 - > drivers/mtd/nand/raw/Makefile | 1 - > drivers/mtd/nand/raw/cmx270_nand.c | 236 ----------------------------- > 6 files changed, 131 insertions(+), 243 deletions(-) > delete mode 100644 drivers/mtd/nand/raw/cmx270_nand.c > Any chance you give this series a try? I plan to merge several series like that during this release, but of course if you can test it beforehands it's even better! Thanks, Miquèl
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 ... Now for your series, I have 2 comments : - dsb() : can you explain the rationale of each of the 3 instances I saw please. - 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. Cheers.
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
Hello, Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 13 May 2020 15:17:37 +0200: > 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. I also agree on the fact that spending time on maintain unused boards is lost time. We have so many drivers to handle, maybe it's time to get rid of these "too" old drivers.
On Wed, May 13, 2020 at 3:23 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed, 13 May 2020 15:17:37 +0200: > > On Wed, 13 May 2020 14:55:01 +0200 Robert Jarzmik <robert.jarzmik@free.fr> wrote: > > > Miquel Raynal <miquel.raynal@bootlin.com> writes: > > > > > > >> 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. > > I also agree on the fact that spending time on maintain unused boards > is lost time. We have so many drivers to handle, maybe it's time to get > rid of these "too" old drivers. The cm-x255/cm-x270 came up in another discussion last year because of its unusual PCI_HOST_ITE8152 PCI support that I'd like to kill off. We did not see a strong reason to keep the board support at that time, but nobody sent any patches. I think it would be reasonable to just kill off MACH_ARMCORE, PCI_HOST_ITE8152 and MTD_NAND_CM_X270 along with anything else that is no longer used after those are gone, such as the pcmcia support. Arnd