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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi David,

On 11/02/16 17:35, David Daney wrote:
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@xxxxxxxxxx>

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 ?

Yes, that's true. The tree as visible in /sys/firmware is the modified version, because the modifications are performed on the DTB before it is unflattened and copied. This is what can be seen with the update:

# ls /sys/firmware/devicetree/base/soc\@0/mmc\@1180000002000/mmc-slot\@0/
bus-width  compatible  max-frequency  name  reg  voltage-ranges


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()

OK, well we can go back to that...


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.

Best to keep the support in the driver, I think.



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.
OK cool, well let's go back to that version, then.

Thanks,
Matt


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.

[...]

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux