lkml.org 
[lkml]   [2015]   [May]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 1/7] staging: fsl-mc: MC bus IRQ support
    On Wed, May 06, 2015 at 04:28:22PM -0500, J. German Rivera wrote:
    > @@ -350,17 +324,280 @@ int dprc_scan_container(struct fsl_mc_device *mc_bus_dev)
    > * Discover objects in the DPRC:
    > */
    > mutex_lock(&mc_bus->scan_mutex);
    > - error = dprc_scan_objects(mc_bus_dev);
    > + error = dprc_scan_objects(mc_bus_dev, &irq_count);
    > mutex_unlock(&mc_bus->scan_mutex);
    > if (error < 0)
    > - goto error;
    > + return error;
    > +
    > + if (!mc_bus->irq_resources) {

    This can never be true. It confused me for a while because otherwise it
    looks like there is an obvious bug here. I also think
    dprc_scan_container() should be named something alloc because that's
    really what it does.

    > + irq_count += FSL_MC_IRQ_POOL_MAX_EXTRA_IRQS;
    > + error = fsl_mc_populate_irq_pool(mc_bus, irq_count);
    > + if (error < 0)
    > + return error;
    > + }
    > +
    > + return 0;
    > +}

    [ snip ]

    > @@ -415,10 +652,20 @@ static int dprc_probe(struct fsl_mc_device *mc_dev)
    > if (error < 0)
    > goto error_cleanup_open;
    >
    > + /*
    > + * Configure interrupts for the DPRC object associated with this MC bus:
    > + */
    > + error = dprc_setup_irqs(mc_dev);
    > + if (error < 0)
    > + goto error_cleanup_open;
    > +
    > dev_info(&mc_dev->dev, "DPRC device bound to driver");
    > return 0;
    >
    > error_cleanup_open:
    > + if (mc_bus->irq_resources)
    > + fsl_mc_cleanup_irq_pool(mc_bus);

    This isn't beautiful beause we're not cleaning up open here. We're
    de-allocating dprc_scan_container(). Introduce a new label, and remove
    the if statement so it looks like this:

    error_free_scan:
    unscan_the_container(mc_bus);
    error_cleanup_open:
    (void)dprc_close(mc_dev->mc_io, mc_dev->mc_handle);


    > +
    > (void)dprc_close(mc_dev->mc_io, mc_dev->mc_handle);
    >
    > error_cleanup_mc_io:

    regards,
    dan carpenter



    \
     
     \ /
      Last update: 2015-05-07 16:41    [W:3.267 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site