Message ID | fbd52f5f5253b382b8d7b3e8046134de29f965b8.1666710197.git.mazziesaccount@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:6687:0:0:0:0:0 with SMTP id l7csp1064832wru; Tue, 25 Oct 2022 08:20:00 -0700 (PDT) X-Google-Smtp-Source: AMsMyM6bKsTriTa53cDEzSR3zDuZ4bQ9Zw5Hmm37hC6HGwOtlwS5Q+CnVwczVPCOTOrNSuVqqdlL X-Received: by 2002:a05:6402:294f:b0:461:f5ce:31e8 with SMTP id ed15-20020a056402294f00b00461f5ce31e8mr6424278edb.363.1666711200606; Tue, 25 Oct 2022 08:20:00 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1666711200; cv=none; d=google.com; s=arc-20160816; b=y6edJ9SSoHsYfDk4BjQlTRPXpgaEFiwmo6Jo1tVwu/55KUqzjZ+TKrc8Q+JxQjDz90 m/U10DJpMHaPe8viefSATRah+9+P9F2NINyaOyQgvHo97Fnb26rANb5q1sEJqvJZgLDg Pkz2dgbkDCzLp2hBnrd0SJpB7t01fVC3+QcvhSuapsNgANXK+h9gT3b2bNyn3nqjRwP9 cWnsESdQSffGcWYwU7Vt8zz6ktRWDasgrF3bodlveoaOMeWd5uVEGZ4mPdkU8gS80E/2 e5nPDr9ukll+uwd7fiAdRysNlFXN2xApsZX+/ZFPBCYppFKxp3Dm08GWDhZxS9ILi7U9 xULA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=i3VCaLUSkP0MHn9g1Bdf8SvWr5beC5vVRYzG/hEHOwM=; b=aHeZGQOfaG97zESEjUaccSJJuhx0x64tbxnTNeMktl0GEMSgiDcew4dtbwxQvfVBiI Ahi/zAOs7nhp2NhLAwaLbW8uRZJ+DMNdX4Sz6HuqCkMkvO5n21hTd5leyOt9eKlLI13O 7V9Q3ONjIq1eC9stJe8L5B8AW0Ds0IfU6/CVW4RTZh+xW5J27lv8547qN8kmnzMldM7H HTlqlde4dMmJcZhkjLizAZcE3CzfQGHltgKabtzbYtS7pa/YWNKamGr1s9oOTs+aWbvG TZekOCUmWIvnpvR3eYrW6XI1uqq+tIyO3Uhwyex9zo53+bSnMR6ZCd6Y5Jwu3ANrcOiF D64Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=KTXHatYh; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id va2-20020a17090711c200b0077f4a46a179si2822576ejb.454.2022.10.25.08.19.35; Tue, 25 Oct 2022 08:20:00 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20210112 header.b=KTXHatYh; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232960AbiJYPMY (ORCPT <rfc822;pwkd43@gmail.com> + 99 others); Tue, 25 Oct 2022 11:12:24 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51734 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232946AbiJYPMU (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 25 Oct 2022 11:12:20 -0400 Received: from mail-lj1-x236.google.com (mail-lj1-x236.google.com [IPv6:2a00:1450:4864:20::236]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 10A9740BD9; Tue, 25 Oct 2022 08:12:19 -0700 (PDT) Received: by mail-lj1-x236.google.com with SMTP id u2so8169642ljl.3; Tue, 25 Oct 2022 08:12:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=i3VCaLUSkP0MHn9g1Bdf8SvWr5beC5vVRYzG/hEHOwM=; b=KTXHatYhfbJ2a1sMmM8whproz8b9Ke5Q7Nnzl12OSpOZiXx2lNMEkbn0qSlv0WvCbd 7T0E/h+cX2gK+AVbtSMBG/I1CuRTDL1tMME+E5KkPVn+oghs3XSqj+RKpjkBSBxQZbMW 94mZIyqeZRnuHH/3uSzyyGuEVGrL77eXiScoBqnSO978mmuxRreEpOIEqnoFXqaCrYxy g5VQLqd6SQoo6P48TPIWMstK2atLw0NASknSIC7DdswGQuZATKNYy7sdqSDg02C+/ZTB KnEmC+dkqLARf7ALRt2jx96PYE9aqWBguXjkGmu19mONskqZDIfC3CcnmI+/PoDOtgdY kMog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=i3VCaLUSkP0MHn9g1Bdf8SvWr5beC5vVRYzG/hEHOwM=; b=4g4Y4vgumlDH7SvqBzY64abRzb1C16FTjwCbDLxMe5Cu1o9qmQmfy8tGnVPnENzMRi ogrXUve8ClgJ667S+m6nPTEZy71WbosCBIwZplsY9lQ3Owvp/b14DgdiIm3kBOL5/L1g 4iptTQ6rXTVCsyRLlHOlviuQKuaf2rC7s5QEIH5qp9pKlot/Sg7WC+TdzHGinu4Y13aa SaC+IiUXup4G3eR+ateH61nX4BzkfrjGl0ZT0UXrjwXrY356Wr0m2b59eKtIXtpMlGw8 CPKM8MvTzv9JiM/hNrey276DdCDGf1LMzxct4xEu+yqi68X5BxvgJ0XNPG+gj7EY+8W5 ZpBg== X-Gm-Message-State: ACrzQf0ChNU/aeXdPwvb2zTdsHfQEjeLMEPdeq3yj5vLy+1o2xR8LBvf NXQ5siF4r5EnFDbWH3g5pnA= X-Received: by 2002:a05:651c:1241:b0:261:9313:9cb9 with SMTP id h1-20020a05651c124100b0026193139cb9mr14393521ljh.213.1666710737399; Tue, 25 Oct 2022 08:12:17 -0700 (PDT) Received: from dc75zzyyyyyyyyyyyyyby-3.rev.dnainternet.fi (dc75zzyyyyyyyyyyyyyby-3.rev.dnainternet.fi. [2001:14ba:16f3:4a00::2]) by smtp.gmail.com with ESMTPSA id r7-20020a2eb607000000b0026befa96249sm545053ljn.8.2022.10.25.08.12.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Oct 2022 08:12:16 -0700 (PDT) Date: Tue, 25 Oct 2022 18:12:11 +0300 From: Matti Vaittinen <mazziesaccount@gmail.com> To: Matti Vaittinen <mazziesaccount@gmail.com>, Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>, Daniel Scally <djrscally@gmail.com>, Heikki Krogerus <heikki.krogerus@linux.intel.com>, Sakari Ailus <sakari.ailus@linux.intel.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, "Rafael J. Wysocki" <rafael@kernel.org>, Wolfram Sang <wsa@kernel.org>, Akhil R <akhilrajeev@nvidia.com>, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-i2c@vger.kernel.org Subject: [PATCH v2 2/2] i2c: i2c-smbus: fwnode_irq_get_byname() return value fix Message-ID: <fbd52f5f5253b382b8d7b3e8046134de29f965b8.1666710197.git.mazziesaccount@gmail.com> References: <cover.1666710197.git.mazziesaccount@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="l4NgxrvU12sZ6SXP" Content-Disposition: inline In-Reply-To: <cover.1666710197.git.mazziesaccount@gmail.com> X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1747673363566833105?= X-GMAIL-MSGID: =?utf-8?q?1747673363566833105?= |
Series |
fix fwnode_irq_get_byname() returnvalue
|
|
Commit Message
Matti Vaittinen
Oct. 25, 2022, 3:12 p.m. UTC
The fwnode_irq_get_byname() was changed to not return 0 upon failure so return value check can be adjusted to reflect the change. Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- Depends on the mentioned return value change which is in patch 1/2. The return value change does also cause a functional change here. Eg. when IRQ mapping fails, the fwnode_irq_get_byname() no longer returns zero. This will cause also the probe here to return nonzero failure. I guess this is desired behaviour. --- drivers/i2c/i2c-smbus.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On Tue, Oct 25, 2022 at 06:12:11PM +0300, Matti Vaittinen wrote: > The fwnode_irq_get_byname() was changed to not return 0 upon failure so > return value check can be adjusted to reflect the change. ... > Depends on the mentioned return value change which is in patch 1/2. The > return value change does also cause a functional change here. Eg. when > IRQ mapping fails, the fwnode_irq_get_byname() no longer returns zero. > This will cause also the probe here to return nonzero failure. I guess > this is desired behaviour. The entire error handling there looks suspicious. The 'struct i2c_smbus_alert_setup' description says: "If irq is not specified, the smbus_alert driver doesn't take care of interrupt handling. In that case it is up to the I2C bus driver to either handle the interrupts or to poll for alerts." So, the question is, shouldn't we just drop the check completely?
On 10/25/22 19:30, Andy Shevchenko wrote: > On Tue, Oct 25, 2022 at 06:12:11PM +0300, Matti Vaittinen wrote: >> The fwnode_irq_get_byname() was changed to not return 0 upon failure so >> return value check can be adjusted to reflect the change. > > ... > >> Depends on the mentioned return value change which is in patch 1/2. The >> return value change does also cause a functional change here. Eg. when >> IRQ mapping fails, the fwnode_irq_get_byname() no longer returns zero. >> This will cause also the probe here to return nonzero failure. I guess >> this is desired behaviour. > > The entire error handling there looks suspicious. > > The 'struct i2c_smbus_alert_setup' description says: > > "If irq is not specified, the smbus_alert driver doesn't take care of > interrupt handling. In that case it is up to the I2C bus driver to > either handle the interrupts or to poll for alerts." > > So, the question is, shouldn't we just drop the check completely? I don't really know what this means. Does it mean that if IRQ is not provided, the driver needs to take care of alerts (in which case the check here is very valid because IRQ is required for smbus_alert driver). Or does it mean that only the IRQ handling is omitted while the smbus_alert driver should do all the other stuff (what ever that is) as usual. In this case this check indeed feels wrong. I would appreciate someone with more insight to this driver to take a look at it. Yours -- Matti -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
On 10/27/22 08:40, Matti Vaittinen wrote: > On 10/25/22 19:30, Andy Shevchenko wrote: >> On Tue, Oct 25, 2022 at 06:12:11PM +0300, Matti Vaittinen wrote: >>> The fwnode_irq_get_byname() was changed to not return 0 upon failure so >>> return value check can be adjusted to reflect the change. >> >> ... >> >>> Depends on the mentioned return value change which is in patch 1/2. The >>> return value change does also cause a functional change here. Eg. when >>> IRQ mapping fails, the fwnode_irq_get_byname() no longer returns zero. >>> This will cause also the probe here to return nonzero failure. I guess >>> this is desired behaviour. >> >> The entire error handling there looks suspicious. >> >> The 'struct i2c_smbus_alert_setup' description says: >> >> "If irq is not specified, the smbus_alert driver doesn't take care of >> interrupt handling. In that case it is up to the I2C bus driver to >> either handle the interrupts or to poll for alerts." >> >> So, the question is, shouldn't we just drop the check completely? > > I don't really know what this means. Does it mean that if IRQ is not > provided, the driver needs to take care of alerts (in which case the > check here is very valid because IRQ is required for smbus_alert > driver). Or does it mean that only the IRQ handling is omitted while the > smbus_alert driver should do all the other stuff (what ever that is) as > usual. In this case this check indeed feels wrong. > > I would appreciate someone with more insight to this driver to take a > look at it. Wolfram, do you have the required insight? What would be the best way to proceed? I see 3 options: 1. fix the return value as is done by this series. https://lore.kernel.org/all/cover.1666710197.git.mazziesaccount@gmail.com/ => Will cause the i2c-smbus probe to return failure also if IRQ mapping fails. 2. apply the 1/1 from the series "as is" - but drop the return value check for fwnode_irq_get_byname() altogether as was suggested by Andy 3. drop this series and apply the documentation fix suggested in: https://lore.kernel.org/all/Y1dzCCMCDswQFVvO@dc75zzyyyyyyyyyyyyyby-3.rev.dnainternet.fi/ Thoughts anyone? Yours -- Matti -- Matti Vaittinen Linux kernel developer at ROHM Semiconductors Oulu Finland ~~ When things go utterly wrong vim users can always type :help! ~~
diff --git a/drivers/i2c/i2c-smbus.c b/drivers/i2c/i2c-smbus.c index 07c92c8495a3..d0cc4b7903ed 100644 --- a/drivers/i2c/i2c-smbus.c +++ b/drivers/i2c/i2c-smbus.c @@ -130,7 +130,7 @@ static int smbalert_probe(struct i2c_client *ara, } else { irq = fwnode_irq_get_byname(dev_fwnode(adapter->dev.parent), "smbus_alert"); - if (irq <= 0) + if (irq < 0) return irq; }