[PATCH v10 3/5] mtd: nand: vf610_nfc: add device tree bindings
From: stefan@agner.ch (Stefan Agner)
Date: 2015-08-26 21:16:06
Also in:
linux-devicetree, lkml
On 2015-08-26 08:39, Boris Brezillon wrote:
Hi Bill, On Wed, 26 Aug 2015 11:26:36 -0400 Bill Pringlemeir [off-list ref] wrote:quoted
On 25 Aug 2015, computersforpeace at gmail.com wrote:quoted
Sorry, I realized a potential issue here.quoted
On Mon, Aug 03, 2015 at 11:27:28AM +0200, Stefan Agner wrote:quoted
Signed-off-by: Bill Pringlemeir <redacted> Acked-by: Shawn Guo <shawnguo@kernel.org> Reviewed-by: Brian Norris <computersforpeace@gmail.com> Signed-off-by: Stefan Agner <stefan@agner.ch> --- .../devicetree/bindings/mtd/vf610-nfc.txt | 45 ++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 Documentation/devicetree/bindings/mtd/vf610-nfc.txtquoted
quoted
diff --git a/Documentation/devicetree/bindings/mtd/vf610-nfc.txtb/Documentation/devicetree/bindings/mtd/vf610-nfc.txt new file mode 100644 index 0000000..cae5f25--- /dev/null +++ b/Documentation/devicetree/bindings/mtd/vf610-nfc.txtquoted
quoted
-0,0 +1,45 @@+- nand-bus-width: see nand.txt +- nand-ecc-mode: see nand.txt +- nand-on-flash-bbt: see nand.txtquoted
Stumbling across the "multi-CS" questions on the driver reminds me: it typically makes sense to define new NAND bindings using separate NAND *controller* and *flash* device nodes. The above 3 properties, at least, would apply on a per-flash basis, not per-controller typically. See sunxi-nand, for instance:quoted
brcmnand had a similar pattern:quoted
(Perhaps it's time we standardized this a little more formally...)These would apply per chip, but the controller has to be configured to support each and every one. Every time an operation was performed, we would have to check the chip type and reconfigure the controller. Currently, the driver does not support this and it would add a lot of overhead in some cases unless a register cache was used. Is the flexibility of using a system with combined 8/16bit devices really worth all the overhead? Isn't it sort of brain dead hardware not to make all of the chips similar? Why would everyone have to pay for such a crazy setup? To separate it would at least be a lie versus the code in the current form. As well, there are only a few SOC which support multiple chip selects. The 'multi-CS' register bits of this controller varies between PowerPC, 68K/Coldfire and ARM platforms.
The DT can be a lie versus the code. The DT should reflect how the hardware is wired, afaik, if we take shortcuts in the driver code, that is fine. If we don't support a certain configuration right now (e.g. second NAND chip), the driver can just return an appropriate error code.
quoted
I looked briefly at the brcmnand.c and it seems that it is not supporting different ECC per chip even though the nodes are broken out this way. In fact, if some raw functions are called, I think it will put it in ECC mode even if it wasn't before? Well, I agree that this would be good generically, I think it puts a lot of effort in the drivers for not so much payoff?Hm, the sunxi driver supports it, and it does not add such a big overhead... The only thing you have to do is cache a bunch of register values per-chip and restore/apply them when the chip is selected (in your ->select_chip() implementation). Anyway, even if the suggested DT representation is a lie in regards to your implementation, it's actually pretty accurate from an hardware POV, and this is exactly what DT is supposed to represent.
I agree with both of you. I don't see much value implementing multi-NAND chip support, especially with different configurations, at the moment. I am not aware of any hardware making use of that now. I will update the driver to parse a NAND sub node and get the ECC properties from the per flash configuration. However, I won't add chip select or multi-NAND support right now... Any objection? -- Stefan