Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752856AbbLZULJ (ORCPT ); Sat, 26 Dec 2015 15:11:09 -0500 Received: from mail-lb0-f174.google.com ([209.85.217.174]:34658 "EHLO mail-lb0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751876AbbLZULH (ORCPT ); Sat, 26 Dec 2015 15:11:07 -0500 Subject: Re: [PATCH 6/7] drivers: net: cpsw: fix error return code To: Julia Lawall References: <1451143726-28195-1-git-send-email-Julia.Lawall@lip6.fr> <1451143726-28195-7-git-send-email-Julia.Lawall@lip6.fr> <567ED019.3000407@cogentembedded.com> <567ED36A.9000701@cogentembedded.com> <567EEFC2.4080800@cogentembedded.com> Cc: netdev@vger.kernel.org, kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org From: Sergei Shtylyov Organization: Cogent Embedded Message-ID: <567EF457.8020602@cogentembedded.com> Date: Sat, 26 Dec 2015 23:11:03 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2128 Lines: 68 On 12/26/2015 11:07 PM, Julia Lawall wrote: >>>>>> diff --git a/drivers/net/ethernet/ti/cpsw.c >>>>>> b/drivers/net/ethernet/ti/cpsw.c >>>>>> index 3409e80..6a76992 100644 >>>>>> --- a/drivers/net/ethernet/ti/cpsw.c >>>>>> +++ b/drivers/net/ethernet/ti/cpsw.c >>>>>> @@ -2448,8 +2448,10 @@ static int cpsw_probe(struct platform_device >>>>>> *pdev) >>>>>> >>>>>> /* RX IRQ */ >>>>>> irq = platform_get_irq(pdev, 1); >>>>>> - if (irq < 0) >>>>>> + if (irq < 0) { >>>>>> + ret = -ENOENT; >>>>> >>>>> Why not just propagate an error returned by that function? >>>> >>>> OK, I did what was done a few lines before in the same function: >>>> >>>> ndev->irq = platform_get_irq(pdev, 1); >>>> if (ndev->irq < 0) { >>>> dev_err(priv->dev, "error getting irq resource\n"); >>>> ret = -ENOENT; >>>> goto clean_ale_ret; >>>> } >>>> >>>> Maybe they should all be changed? >>> >>> Yeah, I'd vote for it. I'm seeing no sense in overriding an actual >>> error. >> >> Hm, I decided to check drivers/base/dd.c and I think I maybe know the >> reason now: -ENXIO, usually returned by platform_get_irq(), is silently >> "swallowed" by really_probe(); to be precise, -ENODEV and -ENXIO are only >> reported with pr_debug(), while -ENOENT causes printk(KERN_WARNING, ...)... > Sorry, I'm confused... What should it be? v1 or v2? Here are the counts > of the different constants returned on failure of platform_get_irq: I was somewhat confused myself but then I remembered about the deferred probing -- overriding error code basically just disables it in this case. > ENODEV: 84 > ENXIO: 67 Those 2 totally make no sense. :-) > EINVAL: 61 > ENOENT: 29 > EBUSY: 11 Hm... > EIO: 2 > EPROBE_DEFER: 1 Hm, and that last one is unconditional? > julia MBR, Sergei -- 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/