On Fri, 10 Jun 2016 18:46:24 +0200 Ricard Wanderlof <ricard.wanderlof@xxxxxxxx> wrote: > On Fri, 10 Jun 2016, Boris Brezillon wrote: > > > > The use cases as I see are as follows: > > > > > > a) Two identical chips sharing all but the CS lines, in order to implement > > > a seemingly-larger address space. (e.g. two 256 Mbit chips implementing a > > > 4 GB area). In this case, for certain operations, the controller does not > > > have to wait for one device to complete before issuing a command to > > > another. I'm not sure how the controller keeps track of the two devices > > > though. > > > > I think it's the chip-enable use case, isn't it? > > > > > > > > b) Two different chips with the same connection, which provide disjunct > > > functions, e.g. one (small) flash for program storage and one (large) for > > > data. > > > > Then they can't share the CS line (or be actived at the same time), at > > least it doesn't make any sense to me. > > Sorry if I haven't been clear enough, but in neither case are the CS lines > shared between the devices. There is still a separate CS for each flash > chip. The question is how the controller handles the CS lines. > > Basically, in the general case, the controller can handle a matrix of nand > flash chips. There can be a number of banks, each of which can have a > number of individual CS lines. For the (in this case academic) case of 3 > banks and 4 chip selects per bank, there would be a total of 3 x 4 = 12 CS > lines. > > For the IP configuration the driver was written for, there are only 2 CS > lines, and we can configure if they are to be viewed by the controller as > 2 CS lines within the same single bank, or 2 separate banks with one CS > each. This is what the DT property is intended to express. It basically > translates directly into a register write in the IP. Okay, got it. I guess we should just expose the chip select in a linear way (3 banks of 4 CS means the controller should support up to 12 chips), unless you really have a way to change the CS pins routing internally. > > But, as you are driving at below, the bindings should really cover the > more general case, for which a simple use-bank-select property doesn't > really cut it. Since the driver only supports a single CS at the time > being, I'm really proposing to drop it completely, alternatively we could > have two: 'banks' and 'devices-per-bank', which reflect what the general > IP case would be able to handle. For the version of the IP in use these > would have the permitted values of 1 and 2, with some combinations being > illegal. Unfortunately, the IP configuration cannot be read out (neither > the version of it), so it's not possible for the driver to verify the > DT settings against the actual IP configuration. I don't really know how > to solve that. Yes, let's drop the property for now. Just trying to understand how the IP works ;). > > > > > > I didn't really want to have the added flexibility (and complexity) of > > > > > being able to use any R/B line for any connected flash chip. It seems an > > > > > unnecessary complication for the driver without much gain. > > > > > > > > Not really, at least if you properly separate the chip and controller > > > > objects it's quite easy to deal with, and I'll ask you to do this clean > > > > separation anyway (even if you say you only have a single chip per > > > > controller) :P. > > > > > > Yes, the separation of chip and controller is needed, but the R/B line > > > flexibility requires an additional mapping functionality within the > > > driver. Not rocket science of course, but I can't see much point in it > > > (other than to cover up a potential routing mistake done by a PCB > > > designer). > > > > You seem to argue on all the minor things I'm asking. Honestly, it > > should be hard or error-prone to do it. And let's say you don't support > > it in your driver, you should still think about that when designing > > your bindings. > > Sorry, it's just that over the years I've seen too much code that > introduces various flexibilities just because someone thought it might be > 'nice to have at some point' or 'because the hardware supports it', but > which in reality is never used and still must be maintained. Sure, one > small mapping function is certainly not going to break the bank, far from > it, but over time the matrix of things that can be configured can grow to > awkward proportions, and often something thought of as a minor issue can > turn out to be complex to support in the end. And given a stable API rule, > it's too late to simplify things once they have been implemented. > > Of course, the code should not be written in a way to limit future > expansion either. > > > > I mean, an IP may be capable of a lot of things, and we don't necessarily > > > want to implement them all in the driver to start with and have DT > > > propertes for them all do we? > > > > Hm, you should at least take the capabilities you know about into > > account when defining a new binding, and you already know that some > > SoCs might decide to expose 2 or more R/B pins, so it should already be > > designed this way. > > Ok. I'll give it some more thought then. > > > I'm opposed to the idea of putting information that can be > > automatically deduced in the DT. For this specific case, the main > > reason is that a board vendor can decide to use different NAND chips > > on the same design, and they might not all share the same > > capabilities (and you don't want to have a dts for each NAND). > > Yes, that makes sense of course, but what if someone would want to > override the automatic settings, for whatever reason, using an optional DT > property? I can think of several reasons either way, that's why I'm > asking. You mean reducing the timings because the board design prevents using the highest supported mode for example? That would actually be a valid use case, and I guess we could make a generic property for that (without the vendor prefix). -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
