Messages in this thread | | | From | Grant Likely <> | Date | Wed, 4 May 2011 14:29:46 -0600 | Subject | Re: [PATCH v4] uio/pdrv_genirq: Add OF support |
| |
On Wed, May 4, 2011 at 1:47 PM, Hans J. Koch <hjk@hansjkoch.de> wrote: > On Wed, May 04, 2011 at 03:21:55PM +0200, Michal Simek wrote: >> Hans J. Koch wrote: >> >On Tue, May 03, 2011 at 10:34:12PM +0200, Wolfram Sang wrote: >> >>On Mon, May 02, 2011 at 08:51:55AM +0200, Michal Simek wrote: >> >>>Adding OF binding to genirq. >> >>>Version string is setup to the "devicetree". >> >>> >> >>>Compatible string is not setup for now but you can add your >> >>>custom compatible string to uio_of_genirq_match structure. >> >>> >> >>>For example with "vendor,device" compatible string: >> >>>static const struct of_device_id __devinitconst uio_of_genirq_match[] = { >> >>> { .compatible = "vendor,device", }, >> >>> { /* empty for now */ }, >> >>>}; >> >>> >> >>>Signed-off-by: Michal Simek <monstr@monstr.eu> >> >>[...] >> >> >> >>>+ /* alloc uioinfo for one device */ >> >>>+ uioinfo = kzalloc(sizeof(*uioinfo), GFP_KERNEL); >> >>kfree in remove? >> > >> >Oh yes. Missed that one. It should probably look like the "bad0" case in probe(). >> >> Yes, freeing uioinfo in uio_pdrv_genirq_remove make sense for CONFIG_OF. >> >> Please correct me if I am wrong dev.of_node is not NULL for OF. I >> think yes that's why I would prefer to use this construct instead of >> #ifdef CONFIG_OF. >> >> if (pdev->dev.of_node) >> kfree(pdev->dev.platform_data); > > Huh? You didn't allocate platform_data, so you shouldn't free it. > It's uioinfo you allocated.
*never* set or modify the pdev->dev.platform_data pointer from a device driver. If the data needs to be modified, or a new structure allocated, then rework the driver to store it in it's private data structure. platform_data must be considered immutable by drivers. Otherwise you'll break unbindng and rebinding device drivers.
g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |