Re: [RESEND PATCH v7 1/2] mmc: OCTEON: Add DT bindings for OCTEON MMC controller

[David Daney <ddaney@caviumnetworks.com>]

On 04/20/2016 02:32 AM, Ulf Hansson wrote:
[...]
>>
>> It is a matter of how much manipulation of the device tree we want to do
>> before it is handed off to the driver core for device creation.  I would
>> like to not do any.
>>
>> There is a global cost to changing the device tree in early boot.  It may
>> not be borne by the MMC sub-system, but it exists none the less.
>
> I don't think your concern is right.
>
> What I request are *small* updates to the DTB from arch/SoC specific
> code, those should be really simple to fix.
>
> The benefit is that the driver becomes more portable and it don't have
> to carry around supporting legacy bindings.
>
> Moreover, for new SoCs revisions, which still may re-use the same MMC
> controller, the DTB patching isn't needed.
>
>>
>> Given that:
>>
>>   A) The MMC core doesn't contain the concept of one bus controller with
>> multiple "slots".
>>
>>   B) The existing OCTEON device tree bindings should continue to be
>> supported.
>>
>> There are several options.
>>
>>    1) Invent new MMC device tree bindings that are different from what we
>> currently have, but that convey the same information.
>>
>>    1a) Change the OCTEON MMC driver to use only these new bindings, and write
>> some sort of device tree fix up code that runs in early boot to rewrite the
>> device tree MMC properties.   This results in the code supporting the OCTEON
>> MMC devices being split between the driver and system early boot code.  The
>> cost is an increase in system complexity to generate the device tree nodes
>> with new bindings.
>>
>>    1b) Change the OCTEON MMC driver to use either these new bindings or
>> legacy bindings.  In this case all OCTEON MMC code is localized to a single
>> driver file.  There is a small increase in complexity to carry code to
>> decode both new and legacy device tree bindings.
>>
>>    2) Use existing OCTEON MMC device tree bindings, as they are sufficient to
>> achieve a working driver.  Since the code is all specific to the OCTEON MMC
>> driver, any ugliness is contained lexicographically near to the features
>> being implemented.  Any feedback related to the architecture and style of
>> the driver code would be addressed.
>>
>> The current state is #2.  My interpretation of your desires is #1a.
>>
>> I am fine with introducing a new device tree binding.  But, I don't think
>> the kernel as a whole nor this specific OCTEON MMC driver will be improved
>> by adding more device tree synthesis code in early boot.  Any thing that is
>> there should be limited to supporting very old (pre OCTEON MMC) firmware
>> that doesn't supply a device tree.  So I would strongly support either
>> approach #1b or #2.
>
> Let me elaborate once more on how I see the way forward.
>
> For A):
> I have suggested a solution that I think can be generic, see my earlier email.
>
>  From the DTB point of view, I request you to update the slot
> compatible string to a generic one. Is that a difficult task to patch
> the DTB with?

It depends on the length of the new compatible property.  If it is 
longer than the old property, then it is much more difficult.


> If so, let's keep yours as well, but make sure it's documented as deprecated.
>
> Regarding the changes needed to the mmc core, as to enable it to know
> about mmc-slots, this should be quite easy to implement. I even
> volunteer to can help, if you think it's needed.
>
> So to summarize regarding A). I want a generic solution for slot nodes!
>
> For B), there are two cases:
> 1. Legacy bindings that already has a corresponding generic MMC
> binding. Renaming these properties by patching the DTB is an easy
> operation.

It is not so easy to rename things in the DTB.  Any renaming causes the 
string table to grow, so you have to have to allocate extra space for 
it.  Currently everything we do with the DTB is done in-place, so you 
would have to rewrite the early DTB handling code to allocate memory and 
make a copy of the DTB.

>
> 2. Regarding the DT bindings for "power-gpios" and "voltage-ranges".
> Under *no* circumstances I won't accept any similar bindings. Instead
> a GPIO regulator shall be used and I have explained why in an earlier
> response.
>

power-gpios is somewhat of a misnomer.  It may not even control the 
device power supply, on some boards it just isolates the data lines so 
the MMC/SD cannot influence the bus.

voltage-ranges I almost think we can drop, and not deal with.


> I do realize that patching the DTB to create the GPIO regulator is not
> an easy task, that was my initial thought but let's just avoid that!
>
> Instead, you can parse the DTB from arch/SoC specific code to find the
> slot compatible string. Then continue to search for the
> power-gpios/voltage-ranges DT properties and register a GPIO regulator
> via the regulator API for the slot-node device. I believe this should
> be an easy task for you to implement, right!?
>
> Then the mmc octeon driver can ignore the power-gpios and
> voltage-ranges DT bindings and instead just use
> mmc_regulator_get_supply() (and other mmc regulator APIs from the mmc
> core) to deal with power to the card.
>

We will consider that.

David Daney