Hi Boris, On Sat, Feb 2, 2019 at 12:53 AM Boris Brezillon <bbrezillon@xxxxxxxxxx> wrote: > > Hi Masahiro, > > On Fri, 1 Feb 2019 19:27:46 +0900 > Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote: > > > Hi. > > > > > > When I was looking into the NAND controller/chips separation, > > this question popped up in my mind. > > > > > > Commit 2d472aba15ff169 provides us a more flexibility > > about the controller/chips connection. > > The connected NAND chips do not need to be homogeneous > > any more. > > > > > > My question is about the ->setup_data_interface() hook > > when switching between NAND chips with different speed (timing mode). > > > > > > Think about the case below: > > > > { > > compatible = "foo-nand-controller"; > > reg = <...>; > > #address-cells = <1>; > > #size-cells = <0>; > > > > nand@0 { > > reg = <0>; > > /* Slow NAND chip */ > > } > > > > nand@1 { > > reg = <1>; > > /* Fast NAND chip */ > > } > > > > } > > > > > > > > In this case, two devices /dev/mtdblock0 and /dev/mtdblock1 > > will appear. > > > > If a user gets access to those two devices in turns, > > I think ->setup_data_interface() should be invoked somehow > > in order to update the timing registers on the controller side. > > > > Currently, ->setup_data_interface() is invoked in nand_scan_tail() > > and that's it. > > > > So, both nand@0 and nand@1 are accessed by the timing mode of nand@1 > > (assuming nand@0 and nand@1 are initialized in this order) > > > > > > Of course, it depends on the controller. > > > > If a controller has a register set for every chip select, > > the hardware will be able to change the access speed automatically. > > > > > > I think most of controllers just have a single set of timing registers. > > So, when switching between different types of chips, > > the driver must update the timing registers. > > > > > > If this is a worthwhile usecase, > > should it be taken care of by the NAND framework, > > or by drivers ? > > > > > > I just thought we could do something in > > nand_get_device() / nand_release_device(). > > > > > > If this should be done per driver, > > drivers can update registers in select_target or select_chip. > > > > > > Thought? > > It should be done by the driver in its select_target() handler. > Actually, it's already done like that in several drivers (marvell, > sunxi, ...). > ->setup_data_interface() is not required to apply timings right away. > What it should do is convert the NAND timings into controller timings. > Of course, if the controller has one set of timing regs per CS, the > driver can apply timings right away, but when that's not the case, > controller timings should be stored in the private nand data and > applied when the chip is selected. I see. I will do so because the Denali controller has just a single set of timing registers. I will need some more time, but I am working on it. Thanks for your advice! > Feel free to enhance the ->setup_data_interface() doc if you think it's > not clear enough. > > Regards, > > Boris > > ______________________________________________________ > Linux MTD discussion mailing list > http://lists.infradead.org/mailman/listinfo/linux-mtd/ -- Best Regards Masahiro Yamada ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/