Re: [PATCH v6 2/3] MIPS: OCTEON: Rename legacy properties in internal device trees.

[David Daney <ddaney@caviumnetworks.com>]

On 02/11/2016 08:53 AM, Matt Redfearn wrote:
> Hi David,
>
> On 11/02/16 16:32, David Daney wrote:
>> On 02/11/2016 08:26 AM, Matt Redfearn wrote:
>>> Many OCTEON devices have been shipped in products with fixed DTBs. These
>>> DTBS contain properties which are not compatible with newer kernels with
>>> upstream drivers.
>>> Therefore some mechanism is necessary to convert legacy naming into
>>> upstream naming. In the first instance this is to support the OCTEON MMC
>>> controller, which is in a later patch of this series.
>>> This patch adds a octeon_handle_legacy_device_tree() function which is
>>> always called from device_tree_init() to fix up the device tree so that
>>> drivers need have no knowledge of the legacy naming or properties.
>>>
>>> Signed-off-by: Matt Redfearn <matt.redfearn@imgtec.com>
>>
>> NAK...
>>
>> I already sent e-mail on this, but it crossed in flight.
>
> Yeah, unfortunate timing.
>
>>
>> Basically, this patch is much more complex than the original code
>> which was just a few lines to check the alternate "legacy" names.
>
> This code is functionally equivalent to the previous version, just
> located in platform code rather than the driver itself.

I know, one thing I really don't like about it, is that we are modifying 
the kernel's view of the device that was passed in.  How does that 
effect what is seen in /sys/firmware ?

I would rather see code that calls mmc_of_parse(), and then, if the two 
properties in question (bus-width, and max-frequency) have not been 
filled in, attempt to read them with the legacy names using 
of_property_read_u32()

The implementation of mmc_of_parse() already contains support for 
parsing legacy properties, so we could also add a couple more there, 
which would be the simplest change of all.


> In terms of LOC
> it's not much different. Doing it this was does allow future flexibility
> to change other bindings that are fixed in firmware without having to
> support each set in the individual drivers.

I think the controversy is limited to the MMC driver.  As far as I know, 
we are in good shape with the bindings for the other drivers.


> Leaving this patch out will mean having to get any legacy bindings
> accepted into each driver via their maintainer.
> But for the moment we're just talking about the MMC driver - if this
> patch is not accepted then the only way to support legacy devices is
> with Ulf's signoff of handling both binding versions in the driver.
>
[...]