Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CAK7LNAQLzvW-zDaR8qkDaxEXxmiCj7XQDK-G57sPhvZu1_eBdA@mail.gmail.com>
 <20190201165218.22a0ac21@bbrezillon>
In-Reply-To: <20190201165218.22a0ac21@bbrezillon>
From:   Masahiro Yamada <yamada.masahiro@socionext.com>
Date:   Sat, 2 Feb 2019 16:43:14 +0900
Message-ID: <CAK7LNASQrd_1qT8BQJBewBA65KeeETXdFpap00JuhhBV277Syw@mail.gmail.com>
Subject: Re: [Question] setup_data_interface() hook when switching to a
 different type of NAND chip
To:     Boris Brezillon <bbrezillon@kernel.org>
Cc:     Boris Brezillon <boris.brezillon@bootlin.com>,
        linux-mtd <linux-mtd@lists.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Miquel Raynal <miquel.raynal@bootlin.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Boris,

On Sat, Feb 2, 2019 at 12:53 AM Boris Brezillon <bbrezillon@kernel.org> wrote:
>
> Hi Masahiro,
>
> On Fri, 1 Feb 2019 19:27:46 +0900
> Masahiro Yamada <yamada.masahiro@socionext.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.


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