linux-kernel - Re: [Question] setup_data_interface() hook when switching to a different type of NAND chip

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 1 Feb 2019 16:53:13 +0100
From:   Boris Brezillon <bbrezillon@...nel.org>
To:     Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc:     Boris Brezillon <boris.brezillon@...tlin.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        linux-mtd <linux-mtd@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [Question] setup_data_interface() hook when switching to a
 different type of NAND chip

Hi Masahiro,

On Fri, 1 Feb 2019 19:27:46 +0900
Masahiro Yamada <yamada.masahiro@...ionext.com> 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.

Feel free to enhance the ->setup_data_interface() doc if you think it's
not clear enough.

Regards,

Boris