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

[Ulf Hansson <ulf.hansson@linaro.org>]

[...]

>
> We cannot change the past.  Our only concern should be to develop the
> simplest and cleanest overall implementation possible given the facts as
> they exist today.
>

I understand, but it seems like we don't agree on what is the "cleanest".

>>
>>>
>>> The firmware containing these bindings is out in the wild.  If we
>>> deprecate
>>> some of the bindings, the driver will still have to support them in the
>>> future.
>>>
>>> In the case of OCTEON based devices, the device tree bindings are a
>>> firmware
>>> <--> kernel ABI as the device trees always come from the firmware and not
>>> some other place.
>>
>>
>> Now, I am not going spend more time arguing, instead I prefer if we
>> can be constructive.
>>
>> As the maintainer of MMC, I have tried to be helpful by providing you
>> with my view on how we can move forward.
>>
>> I don't think it's a big deal for you to implement something along
>> those lines for what I have requested, or is it?
>
>
> 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?
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.

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.

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.

Kind regards
Uffe