Hi Igor, On 5 October 2014 04:52, Igor Grinberg <grinb...@compulab.co.il> wrote: > Hi Simon, > > On 10/03/14 17:04, Simon Glass wrote: >> Hi Igor, >> >> >> On 3 October 2014 01:41, Igor Grinberg <grinb...@compulab.co.il> wrote: >>> Hi Simon, >>> >>> On 10/02/14 22:22, Simon Glass wrote: >>>> Hi Nikita, >>>> >>>> On 2 October 2014 08:17, Nikita Kiryanov <nik...@compulab.co.il> wrote: >>>>> Use gpio_request for all the gpios that are utilized by various >>>>> subsystems in cm-fx6, and refactor the relevant init functions >>>>> so that all gpios are requested during board_init(), not during >>>>> subsystem init, thus avoiding the need to manage gpio ownership >>>>> each time a subsystem is initialized. >>>>> >>>>> The new division of labor is: >>>>> During board_init() muxes are setup and gpios are requested. >>>>> During subsystem init gpios are toggled. >>>>> >>>>> Cc: Igor Grinberg <grinb...@compulab.co.il> >>>>> Cc: Simon Glass <s...@chromium.org> >>>>> Signed-off-by: Nikita Kiryanov <nik...@compulab.co.il> >>>> >>>> Copying my comments from the other patch: >>> >>> Please, don't get me wrong, we have of course read and thought about >>> this also considering your comments. >>> The thing is that currently, some of those GPIOs are way too board >>> specific and may be the right thing would be to leave it that way. >>> Also some are not board specific, and I will be very glad to pass >>> them to the drivers. >>> Yet, I think Nikita's patch is very sane for now. >>> Once we can pass the GPIOs to the drivers we will of course do this. >>> We will also be glad to help working on this as we always did (when >>> the schedule permits us). >>> >>>> >>>> I've thought about that quite a lot as part of the driver model work. >>>> Claiming GPIOs in board code doesn't feel right to me: >>>> >>>> 1. If using device tree, the GPIOs are in there, and it means the >>>> board code needs to go looking there as well as the driver. The board >>>> code actually needs to sniff around in the driver's device tree nodes. >>>> That just seems honky. >>> >>> I think this is case dependent. Really we're in the boot loader >>> world, things here are board specific in many cases. >> >> Yes it is important to retain that flexibility. >> >>> >>>> >>>> 2. In the driver model world, we hope that board init will fade away >>>> to a large extent. Certainly it should be possible to specify most of >>>> what a driver needs in device tree or platform data. Getting rid of >>>> the explicit init calls in U-Boot (board_init_f(), board_init(), >>>> board_late_init(), board_early_init_f(), ...) is a nice effect of >>>> driver model I hope. >>> >>> I don't think it is possible. >>> There will always be boards that are not by the reference design >>> and there will have to be stuff done in the board files as it will >>> not concern any other boards or drivers. >> >> Let's see how this pans out. I feel we can more the pendulum a long >> way towards less board-specific init. For example, for MMC we have a >> card detect. If we just specify which GPIO it is on, then we don't >> need all the callbacks. > > That is totally agreed for cases where it is possible.
OK good. > >> >>> >>>> >>>> 3. Even if not using device tree, and using platform data, where the >>>> board code may specify the platform data, it still feels honky for the >>>> board to be parsing its own data (designed for use by the driver) to >>>> claim GPIOs. >>> >>> Why even? Not using DT is not a bad practice at all! >>> DT has been designed as an API/ABI to the OS and not for the boot loader! >>> Boot loaders are board specific, period. >>> I don't mind using DT in the boot loader for ease and abstraction, but >>> don't be obsessed with it, because it will only lead to another, >>> pre-bootloader boot loader which will accommodate all the stuff you >>> are trying to avoid. >>> >>> Regarding your feeling honky about parsing data by the board code: >>> There are so many cases, that I don't think you have considered, >>> where using DT _instead_ of run time initializations is a total >>> madness. >>> Here is one: >>> Same board, different configuration/revision/extension/variation/etc. >>> Instead of parsing stuff at runtime and adjusting things according >>> to detection, you propose an army of DT blobs? This sounds insane. >> >> We are talking here about the code/data trade-off. I feel that U-Boot >> currently requires lots of code to be written where with device >> tree/platform data it could be data, and the benefit is fewer code >> paths and easier integration of new boards. What are these revisions >> doing? If they are changing the MMC card detect line then you can have >> two different platform data blobs for the board, or two device trees. >> It's not mandatory but it certainly works. > > Right. Yet, you still need code that will detect the revision > and make the correct choice for platform data/DT blob. > What I'm saying is that in this case, you don't really need several > DT blobs or platform data structs, but only one which can be > adjusted by the same (or not the same) revision detection code. > > Sometimes, it can be 3, 4, 5 revisions of the CoM, plus even more of > a base board. You can't really have that much DT blobs in a pocket > to pull out... Sure you can. But if you have board rev GPIOs you can do at least three things: 1. Use them only during production to configure the image to flash (an image that only works with a single board rev) 2. Use them at run time to select which platform data or device tree info to use 3. Use them at run-time to adjust the platform data or device tree (even if just to change the 'status' property from "okay" to "disabled") I feel all of these have their uses depending on the situation. There a lots of trade-offs to be had. For base boards (and maybe even board revs) I wonder if the DT merging feature might be useful? > >> >> If it is easier to check a few GPIOs to find the board ID and then >> adjust things at runtime then that works too. That sort of code should >> live in the board files though, not the drivers. > > So, it merely repeats what I'm saying... > >> >>> >>>> >>>> 4. I don't really see why pre-claiming enforces anything. If two >>>> subsystems claim the same GPIO you are going to get an error somewhere >>>> in any case. >>> >>> Two subsystems should never own the same GPIO at the same time. >>> If you follow that rule, there will be errors only in case when there >>> should errors. >> >> Yes. My point was that pre-claiming would still result in an error in this >> case. > > It will have the benefit of _not_ playing the request - free game. > IMO, request should be done once (for GPIOs that are designed to be used > in only one subsystem) and then the subsystem can toggle the GPIO as > it likes - w/o the need for requesting once again. OK. In general I prefer the device to claim the GPIOs rather than the board. With driver model this is pretty easy as there is a formal 'probe' function which will only be called once (until after 'remove' is called at least). I think request/free is is only a problem because of the ad-hoc driver approach. But I see that as a general direction to take, and not a hard and fast rule. > >> >>> >>>> >>>> 5. If you look at the calls that drivers make to find out information >>>> from the board file, or perform some init (mmc does this, USB, >>>> ethernet, etc.) it is mostly because the driver has no idea of the >>>> specifics of the board. Device tree and platform data fix exactly this >>>> problem. The driver has the best idea of when it needs to start up, >>>> when it needs this resource of that. In some cases the driver have the >>>> ability to operate in two modes (e.g. 4 bit or 8 bit SDIO, UART >>>> with/without flow control) and this means the init depends on the >>>> driver's mode. >>> >>> This is correct. No doubt about this. >>> Yet, generic driver may have prerequisite on a custom board and >>> don't even know about this prerequisite. >> >> Or in fact the driver may request that the board be set up a >> particular way. This is effectively what happens with MMC - 'please >> set up the card-detect line for me to use'. >> >>> >>>> >>>> Having said that, it's your board and if you really want to go this >>>> way in the interim, then I'm not going to strongly object. >>> >>> Thanks! >>> I do really like the idea of DM and I think this should be developed >>> year ago. Yet, any framework should be flexible enough to give some >>> place on the stage to the board specific code. >> >> I strongly agree with this statement and have been careful so far to >> maintain this flexibility in the framework. Let's keep an eye on it. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot