linux-kernel - Re: [PATCH v10 3/5] mtd: nand: vf610

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]

Message-ID: <20150826212842.GT81844@google.com>
Date:	Wed, 26 Aug 2015 14:28:42 -0700
From:	Brian Norris <computersforpeace@...il.com>
To:	Stefan Agner <stefan@...er.ch>
Cc:	Boris Brezillon <boris.brezillon@...e-electrons.com>,
	Bill Pringlemeir <bpringle@...patico.ca>, dwmw2@...radead.org,
	sebastian@...akpoint.cc, robh+dt@...nel.org, pawel.moll@....com,
	mark.rutland@....com, ijc+devicetree@...lion.org.uk,
	galak@...eaurora.org, shawn.guo@...aro.org, kernel@...gutronix.de,
	marb@...at.de, aaron@...tycactus.com, bpringlemeir@...il.com,
	linux-mtd@...ts.infradead.org, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	albert.aribaud@...ev.fr, klimov.linux@...il.com,
	Bill Pringlemeir <bpringlemeir@...ps.com>
Subject: Re: [PATCH v10 3/5] mtd: nand: vf610_nfc: add device tree bindings

On Wed, Aug 26, 2015 at 02:15:45PM -0700, Stefan Agner wrote:
> On 2015-08-26 08:39, Boris Brezillon wrote:
> > On Wed, 26 Aug 2015 11:26:36 -0400
> > Bill Pringlemeir <bpringle@...patico.ca> wrote:
> >> 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.

Right, I was only asking for:
(1) a more accurate DT and
(2) clarity in the driver; the clarity might just be "we don't support
    multi-CS"

> >> 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?

Nope, sounds good to me.

A few tips:
 * be defensive; i.e., error out if someone specifies 2 flash in the DT
 * use the 'reg' property to be the addressing index in the flash
   sub-node; i.e., the chip-select. This fits the practice done by most
   others, I think.

Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/