> -----Original Message----- > From: Greg KH [mailto:gregkh@xxxxxxxxxxxxxxxxxxx] > Sent: Wednesday, December 07, 2016 9:53 AM > To: Stuart Yoder <stuart.yoder@xxxxxxx> > Cc: devel@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; agraf@xxxxxxx; arnd@xxxxxxxx; Leo Li > <leoyang.li@xxxxxxx>; Ioana Ciornei <ioana.ciornei@xxxxxxx>; Catalin Horghidan > <catalin.horghidan@xxxxxxx>; Laurentiu Tudor <laurentiu.tudor@xxxxxxx>; Ruxandra Ioana Radulescu > <ruxandra.radulescu@xxxxxxx> > Subject: Re: [PATCH v3 1/9] staging: fsl-mc: move bus driver out of staging > > On Thu, Dec 01, 2016 at 04:41:26PM -0600, Stuart Yoder wrote: > > Move the source files out of staging into their final locations: > > -include files in drivers/staging/fsl-mc/include go to include/linux/fsl > > -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip > > -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc > > -README.txt, providing and overview of DPAA goes to > > Documentation/dpaa2/overview.txt > > -update MAINTAINERS with new location > > > > Delete other remaining staging files-- Makefile, Kconfig, TODO > > Ok, given that I haven't ever reviewed this code, I had a few questions > that I couldn't easily figure out by looking at your code: > - what is the lifecycle of your 'struct device' usage? Who > creates it, who frees it, and who accesses it? We embed a 'struct device' inside our bus specific device struct 'struct fsl_mc_device'. So, when a new fsl-mc object is discovered on the bus during initial enumeration or hotplug we create a new 'struct fsl_mc_device' and do a device_initialize()/device_add(). (see fsl_mc_device_add() for where this is done) 'struct device' is freed when a device is removed-- the reverse of the above. As far as who accesses it... fsl-mc device drivers will reference the 'struct device' when registering interrupts, when calling functions like devm*, dev_err(), and for maintaining driver private data in 'driver_data'. Example of registering an irq where you can see the embedded struct dev (dpio_dev->dev) referenced: error = devm_request_irq(&dpio_dev->dev, irq->msi_desc->irq, dpio_irq_handler, 0, dev_name(&dpio_dev->dev), &dpio_dev->dev); > - do you have any Documentation/ABI entries? Not currently, but it looks like we need ones for bind/unbind. I will submit a patch to document these. > - root_dprc_count, why are you using an atomic variable for > this? What is it for other than "look, I'm running!"? There can be multiple root buses, and this variable simply tracks the count of them. It's is atomic there might be a theoretical race condition where 2 buses might be added at the same time. The root buses are found in the device tree and so if there is no chance that device tree processing happens in parallel on multiple cores then we could remove the atomic. > - don't call pr_info() in fsl_mc_bus_driver_init(), no need to > say anything if all goes well. Same goes with random > dev_info() calls, please remove. Ok, will do. Thanks, Stuart _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
