[U-Boot] [PATCH 2/2] arm: mx6: cm_fx6: use gpio request

[Igor Grinberg]

On 10/05/14 21:32, Simon Glass wrote:
> Hi Igor,
> 
> On 5 October 2014 04:52, Igor Grinberg <grinberg at 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 <grinberg at 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 <nikita at compulab.co.il> wrote:

[...]

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

Revision GPIOs is only one case of encoding, there are multiple
possibilities for revision encoding.
Nevertheless, from my experience, only 3rd option is scaling well enough
and sorts out various production/field update by customer/etc. needs.

> 
> For base boards (and maybe even board revs) I wonder if the DT merging
> feature might be useful?

Yes. I think it is useful indeed. There were talks about "cape bus" (or
whatever it was called) in Linux kernel and dynamic DT blobs loading for
extending the one the kernel was booted with. Though I don't know what
the status of this, but it can be very useful for extension boards and
hot plugging. Less for the base boards concept (at least as we use it
now). For the base boards DT merging (if I understand the meaning of
this correctly) can prove useful, but should be handled by U-Boot as
base boards might have a serious impact on the boot process.

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

No objection on this one.
The objection was only on how to handle the current situation with cm-fx6
CoM. e.g. current driver does not handle the GPIO request stuff.
Once it does, in the same patch, we should remove it from the board(s)
code. It is just like Nikita said, until we have these in the drivers,
we prefer to solve the request problem in a better way (IMO, this is
a better way - to request once instead of request and free each time
or depend on a "inited" variable).


-- 
Regards,
Igor.