2019-02-01 10:31:50

by Masahiro Yamada

[permalink] [raw]
Subject: [Question] setup_data_interface() hook when switching to a different type of NAND chip

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?


--
Best Regards
Masahiro Yamada


2019-02-01 15:54:55

by Boris Brezillon

[permalink] [raw]
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 <[email protected]> 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

2019-02-02 07:44:29

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [Question] setup_data_interface() hook when switching to a different type of NAND chip

Hi Boris,

On Sat, Feb 2, 2019 at 12:53 AM Boris Brezillon <[email protected]> wrote:
>
> Hi Masahiro,
>
> On Fri, 1 Feb 2019 19:27:46 +0900
> Masahiro Yamada <[email protected]> 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