From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F394BC001E0 for ; Mon, 31 Jul 2023 12:59:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232295AbjGaM7r (ORCPT ); Mon, 31 Jul 2023 08:59:47 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43270 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231794AbjGaM7q (ORCPT ); Mon, 31 Jul 2023 08:59:46 -0400 Received: from mail-ej1-x631.google.com (mail-ej1-x631.google.com [IPv6:2a00:1450:4864:20::631]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D240810E3 for ; Mon, 31 Jul 2023 05:59:44 -0700 (PDT) Received: by mail-ej1-x631.google.com with SMTP id a640c23a62f3a-99c1d03e124so87479266b.2 for ; Mon, 31 Jul 2023 05:59:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1690808383; x=1691413183; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=OjNUiGbK+r1sXqp9L+BG2Nwgnyi0n65xn60LuBTJ+Lo=; b=ipE2GeHmGakEq92kZfr0zFwxnvW8Bm1NtgHlVo+VScWEywfWmiDQZ30e6NG4kzr8Nf l0HE/lQrsj5Jw9gxLxsslNaqp3ue3LjUCFI3vNJ/mtEqCWHys0ED+BtfsA72/y3BZN1t 5NsQiqcsJSsiyrQdFUOpbUGliSd8ViYY+SiE6hZhgqWiIxApRkzMzWxjSB/ZnkMBB6BB PvwHuEHVwhryClOh22M2lQ9jJHEeQYrDD0TzSFip7j8EyukgjkANeivDoC3zFJk9yYLn 9IiFw18Gu7fwb/OswNV5n1JITJ2O+Ia054m+SwYrVGFaZauSsFthBZlaknIg+WZR/+3I dSaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690808383; x=1691413183; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=OjNUiGbK+r1sXqp9L+BG2Nwgnyi0n65xn60LuBTJ+Lo=; b=TdRnwPYoPMFpd1RpMnOGBHmLic+QUmE6GRqlt7N49DGif7oXaAZLrTvwax/adWrFRy v4tzbPRWcK2RtX0cB0FfGAYGnOcbb366/KNsh6DyFjXsbFBQX+5GL7P6XUETwi+lTQJc y+4nY3KNlCY1S7C8MuOqQqlvap+wmq6b+9hq5LFUq+ZJbKopSBXsAJdbmINf7NvaAjlw blJomskhCDEPNtQJwMoLdU2PkRwTiNZsuYQHE6uYqKy27gkxthpFwPpkY3i8A8if3x+D eFmlFVaucAxaTSKKgG8B5768dO24Q45VXx/S6kRqTKolsGSU79NW6aQ7cQ8MhHczAf3Z KXLA== X-Gm-Message-State: ABy/qLbrumQOyzGYFDqAxiYtOVU9pImT8YXc3VPfju0xfXMzVbkgXwud MXEWiyrnTkEJILMw9p3Q05/DkQ== X-Google-Smtp-Source: APBJJlH0RcpsHNdXqhQRW836eEcHu4DeD0tlvaSETRKyEWM2zSPKYNVvCssy+KLB5iMuHppPVtBekw== X-Received: by 2002:a17:906:768d:b0:99b:cd0e:a805 with SMTP id o13-20020a170906768d00b0099bcd0ea805mr5645412ejm.37.1690808383337; Mon, 31 Jul 2023 05:59:43 -0700 (PDT) Received: from [192.168.1.20] ([178.197.222.183]) by smtp.gmail.com with ESMTPSA id um6-20020a170906cf8600b009930c61dc0esm6112815ejb.92.2023.07.31.05.59.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 31 Jul 2023 05:59:42 -0700 (PDT) Message-ID: Date: Mon, 31 Jul 2023 14:59:41 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.1 Subject: Re: [PATCH v3 2/2] i2c: Add GPIO-based hotplug gate Content-Language: en-US To: =?UTF-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= Cc: Svyatoslav Ryhel , Andi Shyti , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Wolfram Sang , linux-i2c@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org References: <20230729160857.6332-1-clamor95@gmail.com> <20230729160857.6332-3-clamor95@gmail.com> <25858c22-ef92-2136-67ef-0d27364c1600@linaro.org> From: Krzysztof Kozlowski In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: devicetree@vger.kernel.org On 31/07/2023 10:49, Michał Mirosław wrote: > On Mon, Jul 31, 2023 at 08:58:14AM +0200, Krzysztof Kozlowski wrote: >> On 30/07/2023 23:55, Michał Mirosław wrote: >>> On Sun, Jul 30, 2023 at 10:30:56PM +0200, Krzysztof Kozlowski wrote: >>>> On 29/07/2023 18:08, Svyatoslav Ryhel wrote: >>>>> From: Michał Mirosław >>>>> >>>>> Implement driver for hot-plugged I2C busses, where some devices on >>>>> a bus are hot-pluggable and their presence is indicated by GPIO line. >>> [...] >>>>> + priv->irq = platform_get_irq(pdev, 0); >>>>> + if (priv->irq < 0) >>>>> + return dev_err_probe(&pdev->dev, priv->irq, >>>>> + "failed to get IRQ %d\n", priv->irq); >>>>> + >>>>> + ret = devm_request_threaded_irq(&pdev->dev, priv->irq, NULL, >>>>> + i2c_hotplug_interrupt, >>>>> + IRQF_ONESHOT | IRQF_SHARED, >>>> >>>> Shared IRQ with devm is a recipe for disaster. Are you sure this is a >>>> shared one? You have a remove() function which also points that it is >>>> not safe. You can: >>>> 1. investigate to be sure it is 100% safe (please document why do you >>>> think it is safe) >>> >>> Could you elaborate on what is unsafe in using devm with shared >>> interrupts (as compared to non-shared or not devm-managed)? >>> >>> The remove function is indeed reversing the order of cleanup. The >>> shutdown path can be fixed by removing `remove()` and adding >>> `devm_add_action_or_reset(...deactivate)` before the IRQ is registered. >> Shared interrupt might be triggered easily by other device between >> remove() and irq release function (devm_free_irq() or whatever it is >> called). > > This is no different tham a non-shared interrupt that can be triggered > by the device being removed. Since devres will release the IRQ first, > before freeing the driver data, the interrupt hander will see consistent > driver-internal state. (The difference between remove() and devres > release phase is that for the latter sysfs files are already removed.) True, therefore non-devm interrupts are recommended also in such case. Maybe one of my solutions is actually not recommended. However if done right, driver with non-shared interrupts, is expected to disable interrupts in remove(), thus there is no risk. We have big discussions in the past about it, so feel free to dig through LKML to read more about. Anyway shared and devm is a clear no go. Best regards, Krzysztof