>>> On 13.10.10 at 18:16, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:
> On Wed, Oct 13, 2010 at 09:53:44AM -0400, Konrad Rzeszutek Wilk wrote:
>> Hey Jan,
>>
>> Thank you for taking your time to look at this patch. Will fix up, test it,
>> and if there are no issues, have it ready tomorrow.
>
> Attached (and inline) is the updated version of this patch. If I missed
> anything
> please do point it out to me!
There's one more missing "static", and one incorrect change to
free_pdev() you did.
Also, any word on the pdev_lock you dropped from the original
implementation?
> If this is to your satisfaction, can I put a Reviewed-by tag on the patch?
Feel free to do so.
> --- /dev/null
> +++ b/drivers/pci/xen-pcifront.c
>...
> +int __devinit pcifront_scan_bus(struct pcifront_device *pdev,
static?
>...
> +static void free_pdev(struct pcifront_device *pdev)
> +{
> + dev_dbg(&pdev->xdev->dev, "freeing pdev @ 0x%p\n", pdev);
> +
> + pcifront_free_roots(pdev);
> +
> + /*For PCIE_AER error handling job*/
> + flush_scheduled_work();
> +
> + if (pdev->irq)
if (pdev->irq > 0)
It gets initialized to -1 in alloc_pdev(). It may be debatable whether
it should be >= 0 - I'm not sure if the pv-ops code allows IRQ 0 to
be used. If it doesn't, initializing to 0 in alloc_pdev() would be an
alternative.
> + unbind_from_irqhandler(pdev->irq, pdev);
> +
> + if (pdev->evtchn != INVALID_EVTCHN)
> + xenbus_free_evtchn(pdev->xdev, pdev->evtchn);
> +
> + if (pdev->gnt_ref != INVALID_GRANT_REF)
> + gnttab_end_foreign_access(pdev->gnt_ref, 0 /* r/w page */,
> + (unsigned long)pdev->sh_info);
> + else
> + free_page((unsigned long)pdev->sh_info);
> +
> + dev_set_drvdata(&pdev->xdev->dev, NULL);
> +
> + kfree(pdev);
> +}
> +
> +static int pcifront_publish_info(struct pcifront_device *pdev)
> +{
> + int err = 0;
> + struct xenbus_transaction trans;
> +
> + err = xenbus_grant_ring(pdev->xdev, virt_to_mfn(pdev->sh_info));
> + if (err < 0)
> + goto out;
> +
> + pdev->gnt_ref = err;
> +
> + err = xenbus_alloc_evtchn(pdev->xdev, &pdev->evtchn);
> + if (err)
> + goto out;
> +
> + err = bind_evtchn_to_irqhandler(pdev->evtchn, pcifront_handler_aer,
> + 0, "pcifront", pdev);
> + if (err < 0) {
> + /*
> + xenbus_free_evtchn(pdev->xdev, pdev->evtchn);
> + xenbus_dev_fatal(pdev->xdev, err, "Failed to bind evtchn to "
> + "irqhandler.\n");
> + */
Why are you commenting it out rather than removing it?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home