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. Thanks, Thomas. _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
