From patchwork Fri Feb 19 23:28:23 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roel Kluin X-Patchwork-Id: 80845 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter.kernel.org (8.14.3/8.14.3) with ESMTP id o1JNL9Wv024727 for ; Fri, 19 Feb 2010 23:21:09 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756169Ab0BSXUm (ORCPT ); Fri, 19 Feb 2010 18:20:42 -0500 Received: from mail-ew0-f228.google.com ([209.85.219.228]:48265 "EHLO mail-ew0-f228.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755671Ab0BSXUk (ORCPT ); Fri, 19 Feb 2010 18:20:40 -0500 Received: by ewy28 with SMTP id 28so751916ewy.28 for ; Fri, 19 Feb 2010 15:20:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=Wc0HjZgQmt8GEhltjsyzr7VTiu8mAS8J6kSZ5aB3IB4=; b=SjFr0A3fd58TWAZjK9UUCKQ/3efqFP1qCGLV7XvT76V3UtaQz8U8a8dQkD9LiDG+cK n301IttxunKkBPIfHGU+8uFay9GG8z4B4IL2Omafxc+9lNUZCgd2sDM+Q9p3llYOOflf n6F6qTCFJe+TplQFBrYNZFFS50gtO50YCuWs0= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:user-agent:mime-version:to:cc:subject :references:in-reply-to:content-type:content-transfer-encoding; b=Xn4MGGU+UiVnLZ9h8+t9tL89gY1ZLprh6KajJ5tenjjlZyMHVK42+ZHLLpWMRwHAHr xM1P1sBjeKHnhmjzfKsmdhcMaBqeIkxXrWBEar5Uw2iMp2TXmzcUgZ5MHD3ZzSTPZSYC QIDV4HcGCPAlYRGpWC6/7VcOlduZSK/7mJmus= Received: by 10.213.68.139 with SMTP id v11mr1484642ebi.32.1266621637895; Fri, 19 Feb 2010 15:20:37 -0800 (PST) Received: from zoinx.mars (d133062.upc-d.chello.nl [213.46.133.62]) by mx.google.com with ESMTPS id 23sm2361427eya.27.2010.02.19.15.20.35 (version=SSLv3 cipher=RC4-MD5); Fri, 19 Feb 2010 15:20:35 -0800 (PST) Message-ID: <4B7F1E97.8000602@gmail.com> Date: Sat, 20 Feb 2010 00:28:23 +0100 From: Roel Kluin User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.7) Gecko/20100120 Fedora/3.0.1-1.fc12 Thunderbird/3.0.1 MIME-Version: 1.0 To: Roel Kluin CC: "Karicheri, Muralidharan" , Mauro Carvalho Chehab , "linux-media@vger.kernel.org" , Andrew Morton , LKML , davinci-linux-open-source@linux.davincidsp.com Subject: Re: [PATCH] video_device: don't free_irq() an element past array vpif_obj.dev[] and fix test References: <4B714E15.4020909@gmail.com> <25e057c01002181202v346f488bk571d099f679fea83@mail.gmail.com> <4B7EEB6B.5070400@gmail.com> In-Reply-To: <4B7EEB6B.5070400@gmail.com> Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.3 (demeter.kernel.org [140.211.167.41]); Fri, 19 Feb 2010 23:21:09 +0000 (UTC) diff --git a/drivers/media/video/davinci/vpif_capture.c b/drivers/media/video/davinci/vpif_capture.c index 7813072..096e727 100644 --- a/drivers/media/video/davinci/vpif_capture.c +++ b/drivers/media/video/davinci/vpif_capture.c @@ -1915,7 +1915,7 @@ static __init int vpif_probe(struct platform_device *pdev) { struct vpif_subdev_info *subdevdata; struct vpif_capture_config *config; - int i, j, k, m, q, err; + int i, j, k, err; struct i2c_adapter *i2c_adap; struct channel_obj *ch; struct common_obj *common; @@ -1936,14 +1936,18 @@ static __init int vpif_probe(struct platform_device *pdev) for (i = res->start; i <= res->end; i++) { if (request_irq(i, vpif_channel_isr, IRQF_DISABLED, "DM646x_Capture", - (void *)(&vpif_obj.dev[k]->channel_id))) { - err = -EBUSY; + &vpif_obj.dev[k]->channel_id)) { i--; + err = -EBUSY; goto vpif_int_err; } } k++; + if (k >= VPIF_DISPLAY_MAX_DEVICES) + break; } + if (k == 0) + return -ENODEV; for (i = 0; i < VPIF_CAPTURE_MAX_DEVICES; i++) { /* Get the pointer to the channel object */ @@ -1972,16 +1976,16 @@ static __init int vpif_probe(struct platform_device *pdev) ch->video_dev = vfd; } - for (j = 0; j < VPIF_CAPTURE_MAX_DEVICES; j++) { - ch = vpif_obj.dev[j]; - ch->channel_id = j; + for (i = 0; i < VPIF_CAPTURE_MAX_DEVICES; i++) { + ch = vpif_obj.dev[i]; + ch->channel_id = i; common = &(ch->common[VPIF_VIDEO_INDEX]); spin_lock_init(&common->irqlock); mutex_init(&common->lock); /* Initialize prio member of channel object */ v4l2_prio_init(&ch->prio); err = video_register_device(ch->video_dev, - VFL_TYPE_GRABBER, (j ? 1 : 0)); + VFL_TYPE_GRABBER, (i ? 1 : 0)); if (err) goto probe_out; @@ -2007,24 +2011,24 @@ static __init int vpif_probe(struct platform_device *pdev) goto probe_subdev_out; } - for (i = 0; i < subdev_count; i++) { - subdevdata = &config->subdev_info[i]; - vpif_obj.sd[i] = + for (j = 0; j < subdev_count; j++) { + subdevdata = &config->subdev_info[j]; + vpif_obj.sd[j] = v4l2_i2c_new_subdev_board(&vpif_obj.v4l2_dev, i2c_adap, subdevdata->name, &subdevdata->board_info, NULL); - if (!vpif_obj.sd[i]) { + if (!vpif_obj.sd[j]) { vpif_err("Error registering v4l2 subdevice\n"); goto probe_subdev_out; } v4l2_info(&vpif_obj.v4l2_dev, "registered sub device %s\n", subdevdata->name); - if (vpif_obj.sd[i]) - vpif_obj.sd[i]->grp_id = 1 << i; + if (vpif_obj.sd[j]) + vpif_obj.sd[j]->grp_id = 1 << j; } v4l2_info(&vpif_obj.v4l2_dev, "DM646x VPIF Capture driver" " initialized\n"); @@ -2034,30 +2038,37 @@ static __init int vpif_probe(struct platform_device *pdev) probe_subdev_out: /* free sub devices memory */ kfree(vpif_obj.sd); - - j = VPIF_CAPTURE_MAX_DEVICES; probe_out: v4l2_device_unregister(&vpif_obj.v4l2_dev); - for (k = 0; k < j; k++) { + for (j = 0; j < i; j++) { /* Get the pointer to the channel object */ - ch = vpif_obj.dev[k]; + ch = vpif_obj.dev[j]; /* Unregister video device */ video_unregister_device(ch->video_dev); } vpif_dev_alloc_err: - k = VPIF_CAPTURE_MAX_DEVICES-1; - res = platform_get_resource(pdev, IORESOURCE_IRQ, k); + while (k--) { + res = platform_get_resource(pdev, IORESOURCE_IRQ, k); + if (res != NULL) + break; + vpif_err("Couldn't free irqs for resource %d.\n", k); + } + if (res == NULL) + return err; i = res->end; - vpif_int_err: - for (q = k; q >= 0; q--) { - for (m = i; m >= (int)res->start; m--) - free_irq(m, (void *)(&vpif_obj.dev[q]->channel_id)); + for (j = i; j >= (int)res->start; j--) + free_irq(j, &vpif_obj.dev[k]->channel_id); - res = platform_get_resource(pdev, IORESOURCE_IRQ, q-1); - if (res) - i = res->end; + while (k--) { + res = platform_get_resource(pdev, IORESOURCE_IRQ, k); + if (res == NULL) { + vpif_err("Couldn't free irqs for resource %d.\n", k); + continue; + } + for (j = res->end; j >= (int)res->start; j--) + free_irq(j, &vpif_obj.dev[k]->channel_id); } return err; } diff --git a/drivers/media/video/davinci/vpif_display.c b/drivers/media/video/davinci/vpif_display.c index dfddef7..6bcedbf 100644 --- a/drivers/media/video/davinci/vpif_display.c +++ b/drivers/media/video/davinci/vpif_display.c @@ -383,7 +383,7 @@ static int vpif_get_std_info(struct channel_obj *ch) int index; std_info->stdid = vid_ch->stdid; - if (!std_info) + if (!std_info->stdid) return -1; for (index = 0; index < ARRAY_SIZE(ch_params); index++) { @@ -1423,7 +1423,7 @@ static __init int vpif_probe(struct platform_device *pdev) { struct vpif_subdev_info *subdevdata; struct vpif_display_config *config; - int i, j = 0, k, q, m, err = 0; + int i, j, k, err; struct i2c_adapter *i2c_adap; struct common_obj *common; struct channel_obj *ch; @@ -1452,12 +1452,18 @@ static __init int vpif_probe(struct platform_device *pdev) if (request_irq(i, vpif_channel_isr, IRQF_DISABLED, "DM646x_Display", (void *)(&vpif_obj.dev[k]->channel_id))) { + i--; err = -EBUSY; + vpif_err("VPIF IRQ request failed\n"); goto vpif_int_err; } } k++; + if (k >= VPIF_DISPLAY_MAX_DEVICES) + break; } + if (k == 0) + return -ENODEV; for (i = 0; i < VPIF_DISPLAY_MAX_DEVICES; i++) { @@ -1472,7 +1478,7 @@ static __init int vpif_probe(struct platform_device *pdev) video_device_release(ch->video_dev); } err = -ENOMEM; - goto vpif_int_err; + goto alloc_vid_fail; } /* Initialize field of video device */ @@ -1489,13 +1495,13 @@ static __init int vpif_probe(struct platform_device *pdev) ch->video_dev = vfd; } - for (j = 0; j < VPIF_DISPLAY_MAX_DEVICES; j++) { - ch = vpif_obj.dev[j]; + for (i = 0; i < VPIF_DISPLAY_MAX_DEVICES; i++) { + ch = vpif_obj.dev[i]; /* Initialize field of the channel objects */ atomic_set(&ch->usrs, 0); - for (k = 0; k < VPIF_NUMOBJECTS; k++) { - ch->common[k].numbuffers = 0; - common = &ch->common[k]; + for (j = 0; j < VPIF_NUMOBJECTS; j++) { + ch->common[j].numbuffers = 0; + common = &ch->common[j]; common->io_usrs = 0; common->started = 0; spin_lock_init(&common->irqlock); @@ -1506,12 +1512,12 @@ static __init int vpif_probe(struct platform_device *pdev) common->ctop_off = common->cbtm_off = 0; common->cur_frm = common->next_frm = NULL; memset(&common->fmt, 0, sizeof(common->fmt)); - common->numbuffers = config_params.numbuffers[k]; + common->numbuffers = config_params.numbuffers[j]; } ch->initialized = 0; - ch->channel_id = j; - if (j < 2) + ch->channel_id = i; + if (i < 2) ch->common[VPIF_VIDEO_INDEX].numbuffers = config_params.numbuffers[ch->channel_id]; else @@ -1529,7 +1535,7 @@ static __init int vpif_probe(struct platform_device *pdev) (int)ch, (int)&ch->video_dev); err = video_register_device(ch->video_dev, - VFL_TYPE_GRABBER, (j ? 3 : 2)); + VFL_TYPE_GRABBER, (i ? 3 : 2)); if (err < 0) goto probe_out; @@ -1548,18 +1554,18 @@ static __init int vpif_probe(struct platform_device *pdev) goto probe_out; } - for (i = 0; i < subdev_count; i++) { - vpif_obj.sd[i] = v4l2_i2c_new_subdev_board(&vpif_obj.v4l2_dev, - i2c_adap, subdevdata[i].name, - &subdevdata[i].board_info, + for (j = 0; j < subdev_count; j++) { + vpif_obj.sd[j] = v4l2_i2c_new_subdev_board(&vpif_obj.v4l2_dev, + i2c_adap, subdevdata[j].name, + &subdevdata[j].board_info, NULL); - if (!vpif_obj.sd[i]) { + if (!vpif_obj.sd[j]) { vpif_err("Error registering v4l2 subdevice\n"); goto probe_subdev_out; } - if (vpif_obj.sd[i]) - vpif_obj.sd[i]->grp_id = 1 << i; + if (vpif_obj.sd[j]) + vpif_obj.sd[j]->grp_id = 1 << j; } return 0; @@ -1567,20 +1573,36 @@ static __init int vpif_probe(struct platform_device *pdev) probe_subdev_out: kfree(vpif_obj.sd); probe_out: - for (k = 0; k < j; k++) { - ch = vpif_obj.dev[k]; + for (j = 0; j < i; j++) { + ch = vpif_obj.dev[j]; video_unregister_device(ch->video_dev); video_device_release(ch->video_dev); ch->video_dev = NULL; } +alloc_vid_fail: + while (k--) { + res = platform_get_resource(pdev, IORESOURCE_IRQ, k); + if (res != NULL) + break; + vpif_err("Couldn't free irqs for resource %d.\n", k); + } + if (res == NULL) + return err; + i = res->end; vpif_int_err: v4l2_device_unregister(&vpif_obj.v4l2_dev); - vpif_err("VPIF IRQ request failed\n"); - for (q = k; k >= 0; k--) { - for (m = i; m >= res->start; m--) - free_irq(m, (void *)(&vpif_obj.dev[k]->channel_id)); - res = platform_get_resource(pdev, IORESOURCE_IRQ, k-1); - m = res->end; + + for (j = i; j >= res->start; j--) + free_irq(j, (void *)(&vpif_obj.dev[k]->channel_id)); + + while (k--) { + res = platform_get_resource(pdev, IORESOURCE_IRQ, k); + if (res == NULL) { + vpif_err("Couldn't free irqs for resource %d.\n", k); + continue; + } + for (j = res->end; j >= res->start; j--) + free_irq(j, (void *)(&vpif_obj.dev[k]->channel_id)); } return err; diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c index 33c4fe2..2faa9d3 100644 --- a/drivers/net/davinci_emac.c +++ b/drivers/net/davinci_emac.c @@ -2369,12 +2369,12 @@ static int emac_devioctl(struct net_device *ndev, struct ifreq *ifrq, int cmd) static int emac_dev_open(struct net_device *ndev) { struct device *emac_dev = &ndev->dev; - u32 rc, cnt, ch; + u32 cnt, ch; int phy_addr; struct resource *res; - int q, m; - int i = 0; + int i, j, k, err; int k = 0; + int err; struct emac_priv *priv = netdev_priv(ndev); netif_carrier_off(ndev); @@ -2398,15 +2398,15 @@ static int emac_dev_open(struct net_device *ndev) emac_write(EMAC_MACHASH2, 0); /* multi ch not supported - open 1 TX, 1RX ch by default */ - rc = emac_init_txch(priv, EMAC_DEF_TX_CH); - if (0 != rc) { + err = emac_init_txch(priv, EMAC_DEF_TX_CH); + if (err) { dev_err(emac_dev, "DaVinci EMAC: emac_init_txch() failed"); - return rc; + return err; } - rc = emac_init_rxch(priv, EMAC_DEF_RX_CH, priv->mac_addr); - if (0 != rc) { + err = emac_init_rxch(priv, EMAC_DEF_RX_CH, priv->mac_addr); + if (err) { dev_err(emac_dev, "DaVinci EMAC: emac_init_rxch() failed"); - return rc; + return err; } /* Request IRQ */ @@ -2414,11 +2414,17 @@ static int emac_dev_open(struct net_device *ndev) while ((res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k))) { for (i = res->start; i <= res->end; i++) { if (request_irq(i, emac_irq, IRQF_DISABLED, - ndev->name, ndev)) + ndev->name, ndev)) { + i--; + err = -EBUSY; + dev_err(emac_dev, "DaVinci EMAC: request_irq() failed"); goto rollback; + } } k++; } + if (k == 0) + return -ENODEV; /* Start/Enable EMAC hardware */ emac_hw_enable(priv); @@ -2436,7 +2442,8 @@ static int emac_dev_open(struct net_device *ndev) if (!priv->phydev) { printk(KERN_ERR "%s: no PHY found\n", ndev->name); - return -1; + err = -ENODEV; + goto phydev_error; } priv->phydev = phy_connect(ndev, dev_name(&priv->phydev->dev), @@ -2445,7 +2452,8 @@ static int emac_dev_open(struct net_device *ndev) if (IS_ERR(priv->phydev)) { printk(KERN_ERR "%s: Could not attach to PHY\n", ndev->name); - return PTR_ERR(priv->phydev); + err = PTR_ERR(priv->phydev); + goto phydev_error; } priv->link = 0; @@ -2456,7 +2464,7 @@ static int emac_dev_open(struct net_device *ndev) "(mii_bus:phy_addr=%s, id=%x)\n", ndev->name, priv->phydev->drv->name, dev_name(&priv->phydev->dev), priv->phydev->phy_id); - } else{ + } else { /* No PHY , fix the link, speed and duplex settings */ priv->link = 1; priv->speed = SPEED_100; @@ -2475,15 +2483,29 @@ static int emac_dev_open(struct net_device *ndev) return 0; +phydev_error: + while (k--) { + res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k); + if (res != NULL) + break; + dev_err(emac_dev, "Couldn't free irqs for resource %d.\n", k); + } + if (res == NULL) + return err; + i = res->end; rollback: - - dev_err(emac_dev, "DaVinci EMAC: request_irq() failed"); - - for (q = k; k >= 0; k--) { - for (m = i; m >= res->start; m--) - free_irq(m, ndev); - res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k-1); - m = res->end; + for (j = i; j >= res->start; j--) + free_irq(j, ndev); + + while (k--) { + res = platform_get_resource(priv->pdev, IORESOURCE_IRQ, k); + if (res == NULL) { + dev_err(emac_dev, "Couldn't free irqs for resource %d.\n", + k); + continue; + } + for (j = res->end; j >= res->start; j--) + free_irq(j, ndev); } return -EBUSY; }