Hi Grant, On 18 March 2011 00:49, Grant Likely <[email protected]> wrote: > On Thu, Feb 24, 2011 at 05:43:31PM +0530, Thomas Abraham wrote: >> Hi Grant, >> >> On 23 February 2011 23:33, Grant Likely <[email protected]> wrote: >> > Hi Thomas, >> > >> > On Wed, Feb 23, 2011 at 10:48:37PM +0530, Thomas Abraham wrote: >> >> Hi, >> >> >> >> I am adding support for device tree based probe for the s5pv310 serial >> >> driver for a platform that has 4 instances of the uart port. I have >> >> few questions on this and appreciate any help for the following >> >> questions. >> >> >> >> 1. The driver is based on the usage of pdev->id in several parts of >> >> the driver. But the platform_device created using the >> >> of_platform_bus_probe function assigns -1 to pdev->id. Is the use of >> >> pdev->id not advisable or is it okay if the driver assigns a pdev->id >> >> value during the probe. >> > >> > No, the driver should *not* write a value to pdev->id at probe time. >> > Doing so breaks the device model. Instead, any enumeration that the >> > driver cares about should be stored in the driver's private data >> > structure. I would recommend modifying the driver to copy pdev->id >> > into its private structure. If it is -1, then dynamically assign an >> > id. >> > >> >> >> >> 2. The driver uses pdev->dev.platform_data even after the probe is >> >> complete. I read in one of the posts from Grant Likely that drivers >> >> should not assign pdev->dev.platfrom_data. >> > >> > Correct. >> > >> >> Is it okay if the driver >> >> parse the platform data related information from the device node and >> >> assign it to pdev->dev.platform_data. >> > >> > No. The driver *must not* modify or assign platform_data. That >> > pointer provides information to the driver, but there are memory >> > allocation lifecycle issues if it tries to store something there. >> > (Okay, I it *can* be done if the driver very carefully cleaned up >> > after itself; but I strongly recommend against it; that isn't what >> > that pointer is for) Modifying it also causes issues with drivers >> > that can accept either platform_data or device tree data. >> > >> > Instead, the driver should store all data it needs in its private data >> > structure. That's exactly why drivers have private data structures. >> > >> > g. >> > >> >> Thanks for your explanation. It was very helpful. I am now modifying >> the uart driver to keep a copy of pdev->dev.platform_data and use it. >> Also, trying to eliminate all uses of pdev->id. >> >> Could you help me with another question. The clk_get api for s5pv310 >> uses the pdev->id to find the clock defined in platform code. Since >> the instance of 'struct platform_device' that the driver gets when >> probed using device tree will have pdev->id as -1, the clk_get fails. >> Is it okay to assign a instance value to pdev->id in the probe >> function. > > When clock connections are described in the device tree, it is > intended (hoped?) that code in drivers/of/clock.c will take care of > any decoding device tree references to struct clks. Unfortunately, > the design of the clk api is such that it really is non-trivial and I > don't have an easy answer for you about how to reconcile the different > way that DT and non-DT platforms want to get their clock information. > > However, I do have a workaround. Take a look at the > of_platform_prepare() patch that I sent around a couple of days ago. > That patch will allow DT platforms to continue using the static > platform_device registrations used by non-DT platforms with the same > SoC, while still getting links to the device tree nodes. It may not > be the long term solution, but I'm comfortable merging code that uses > it because it sidesteps a lot of these issues (the static > registrations can keep using the current .id and clk_get() code).
Thanks. I will check with of_platform_prepare() and of_platform_populate() Regards, Thomas. > > g. > > _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
