From mboxrd@z Thu Jan 1 00:00:00 1970 From: Greg Kroah-Hartman Subject: Re: [PATCH] driver: core: Allow subsystems to continue deferring probe Date: Wed, 5 Jun 2019 19:36:13 +0200 Message-ID: <20190605173613.GD27700@kroah.com> References: <20190605142312.6072-1-thierry.reding@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20190605142312.6072-1-thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Thierry Reding Cc: Ulf Hansson , linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kevin Hilman , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, "Rafael J . Wysocki" , Linus Walleij List-Id: linux-gpio@vger.kernel.org On Wed, Jun 05, 2019 at 04:23:12PM +0200, Thierry Reding wrote: > From: Thierry Reding > > Some subsystems, such as pinctrl, allow continuing to defer probe > indefinitely. This is useful for devices that depend on resources > provided by devices that are only probed after the init stage. > > One example of this can be seen on Tegra, where the DPAUX hardware > contains pinmuxing controls for pins that it shares with an I2C > controller. The I2C controller is typically used for communication > with a monitor over HDMI (DDC). However, other instances of the I2C > controller are used to access system critical components, such as a > PMIC. The I2C controller driver will therefore usually be a builtin > driver, whereas the DPAUX driver is part of the display driver that > is loaded from a module to avoid bloating the kernel image with all > of the DRM/KMS subsystem. > > In this particular case the pins used by this I2C/DDC controller > become accessible very late in the boot process. However, since the > controller is only used in conjunction with display, that's not an > issue. > > Unfortunately the driver core currently outputs a warning message > when a device fails to get the pinctrl before the end of the init > stage. That can be confusing for the user because it may sound like > an unwanted error occurred, whereas it's really an expected and > harmless situation. > > In order to eliminate this warning, this patch allows callers of the > driver_deferred_probe_check_state() helper to specify that they want > to continue deferring probe, regardless of whether we're past the > init stage or not. All of the callers of that function are updated > for the new signature, but only the pinctrl subsystem passes a true > value in the new persist parameter if appropriate. > > Signed-off-by: Thierry Reding > --- > drivers/base/dd.c | 17 ++++++++++++----- > drivers/base/power/domain.c | 2 +- > drivers/iommu/of_iommu.c | 2 +- > drivers/pinctrl/devicetree.c | 10 ++++++---- > include/linux/device.h | 2 +- > 5 files changed, 21 insertions(+), 12 deletions(-) > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 0df9b4461766..25ffbadf4187 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -238,23 +238,30 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup); > /** > * driver_deferred_probe_check_state() - Check deferred probe state > * @dev: device to check > + * @persist: Boolean flag indicating whether drivers should keep trying to > + * probe after built-in drivers have had a chance to probe. This is useful > + * for built-in drivers that rely on resources provided by modular drivers. Functionality aside, having random boolean flags in a function parameter list is a horrid way to have an api. Now when you see a call to this function with either "true" or "false" as an option, you have no idea what that means. So you need to go look up this definition and then work backwards. So even if the idea is sane (I'm not saying that), this implementation is not acceptable at all. thanks, greg k-h 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 X-Spam-Level: X-Spam-Status: No, score=-8.3 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 44FB9C28CC5 for ; Wed, 5 Jun 2019 17:36:18 +0000 (UTC) Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1BB6320866 for ; Wed, 5 Jun 2019 17:36:18 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="Pl7N7Ngq" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1BB6320866 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=linuxfoundation.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=iommu-bounces@lists.linux-foundation.org Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id E3AFEDB6; Wed, 5 Jun 2019 17:36:17 +0000 (UTC) Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id E66E3DA1 for ; Wed, 5 Jun 2019 17:36:16 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 750206D6 for ; Wed, 5 Jun 2019 17:36:16 +0000 (UTC) Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B43C420717; Wed, 5 Jun 2019 17:36:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1559756176; bh=f9NghZSrRd84WnvqePdJTbqihcTlrKdRTvbApOR66MM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Pl7N7Ngq0jBLiU77CCBuTQuGdITFzCAYXtsahILz8VNRCUU/PoY6R2aw8gvCtOTlN Ij3OOkU4KO4BFERevp0S/pFBlW4liaT5yNjjd2A4DFB3hpH9pmLaS4Rk6Xfb374BJ6 d4SrzGH9IkREaGu0+RSBi5V0uLW5Y6J1qkztsBLs= Date: Wed, 5 Jun 2019 19:36:13 +0200 From: Greg Kroah-Hartman To: Thierry Reding Subject: Re: [PATCH] driver: core: Allow subsystems to continue deferring probe Message-ID: <20190605173613.GD27700@kroah.com> References: <20190605142312.6072-1-thierry.reding@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190605142312.6072-1-thierry.reding@gmail.com> User-Agent: Mutt/1.12.0 (2019-05-25) Cc: Ulf Hansson , linux-pm@vger.kernel.org, Kevin Hilman , linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org, iommu@lists.linux-foundation.org, "Rafael J . Wysocki" , Linus Walleij X-BeenThere: iommu@lists.linux-foundation.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: Development issues for Linux IOMMU support List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: iommu-bounces@lists.linux-foundation.org Errors-To: iommu-bounces@lists.linux-foundation.org On Wed, Jun 05, 2019 at 04:23:12PM +0200, Thierry Reding wrote: > From: Thierry Reding > > Some subsystems, such as pinctrl, allow continuing to defer probe > indefinitely. This is useful for devices that depend on resources > provided by devices that are only probed after the init stage. > > One example of this can be seen on Tegra, where the DPAUX hardware > contains pinmuxing controls for pins that it shares with an I2C > controller. The I2C controller is typically used for communication > with a monitor over HDMI (DDC). However, other instances of the I2C > controller are used to access system critical components, such as a > PMIC. The I2C controller driver will therefore usually be a builtin > driver, whereas the DPAUX driver is part of the display driver that > is loaded from a module to avoid bloating the kernel image with all > of the DRM/KMS subsystem. > > In this particular case the pins used by this I2C/DDC controller > become accessible very late in the boot process. However, since the > controller is only used in conjunction with display, that's not an > issue. > > Unfortunately the driver core currently outputs a warning message > when a device fails to get the pinctrl before the end of the init > stage. That can be confusing for the user because it may sound like > an unwanted error occurred, whereas it's really an expected and > harmless situation. > > In order to eliminate this warning, this patch allows callers of the > driver_deferred_probe_check_state() helper to specify that they want > to continue deferring probe, regardless of whether we're past the > init stage or not. All of the callers of that function are updated > for the new signature, but only the pinctrl subsystem passes a true > value in the new persist parameter if appropriate. > > Signed-off-by: Thierry Reding > --- > drivers/base/dd.c | 17 ++++++++++++----- > drivers/base/power/domain.c | 2 +- > drivers/iommu/of_iommu.c | 2 +- > drivers/pinctrl/devicetree.c | 10 ++++++---- > include/linux/device.h | 2 +- > 5 files changed, 21 insertions(+), 12 deletions(-) > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 0df9b4461766..25ffbadf4187 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -238,23 +238,30 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup); > /** > * driver_deferred_probe_check_state() - Check deferred probe state > * @dev: device to check > + * @persist: Boolean flag indicating whether drivers should keep trying to > + * probe after built-in drivers have had a chance to probe. This is useful > + * for built-in drivers that rely on resources provided by modular drivers. Functionality aside, having random boolean flags in a function parameter list is a horrid way to have an api. Now when you see a call to this function with either "true" or "false" as an option, you have no idea what that means. So you need to go look up this definition and then work backwards. So even if the idea is sane (I'm not saying that), this implementation is not acceptable at all. thanks, greg k-h _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu 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 X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,T_DKIMWL_WL_HIGH,URIBL_BLOCKED,USER_AGENT_MUTT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 707CBC28D18 for ; Wed, 5 Jun 2019 17:36:18 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 4E5C020866 for ; Wed, 5 Jun 2019 17:36:18 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1559756178; bh=f9NghZSrRd84WnvqePdJTbqihcTlrKdRTvbApOR66MM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:List-ID:From; b=xZbwOFCKSk0J9MwUorkLkeUgkFNfS7QsWQHNBC6TbvoSLOO8S6xeVWRpFw9h7u8aK BkuP74NEW/Nskv6iOJtczLfzr92UcDqBziaR50r/upXuasUYxmTg+okJq988cKlcfi H0OBs2IgqBg0csP5J8OSfuGNR/REvt6i1SV74I1w= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726305AbfFERgR (ORCPT ); Wed, 5 Jun 2019 13:36:17 -0400 Received: from mail.kernel.org ([198.145.29.99]:56282 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726240AbfFERgR (ORCPT ); Wed, 5 Jun 2019 13:36:17 -0400 Received: from localhost (83-86-89-107.cable.dynamic.v4.ziggo.nl [83.86.89.107]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B43C420717; Wed, 5 Jun 2019 17:36:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1559756176; bh=f9NghZSrRd84WnvqePdJTbqihcTlrKdRTvbApOR66MM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Pl7N7Ngq0jBLiU77CCBuTQuGdITFzCAYXtsahILz8VNRCUU/PoY6R2aw8gvCtOTlN Ij3OOkU4KO4BFERevp0S/pFBlW4liaT5yNjjd2A4DFB3hpH9pmLaS4Rk6Xfb374BJ6 d4SrzGH9IkREaGu0+RSBi5V0uLW5Y6J1qkztsBLs= Date: Wed, 5 Jun 2019 19:36:13 +0200 From: Greg Kroah-Hartman To: Thierry Reding Cc: "Rafael J . Wysocki" , Kevin Hilman , Ulf Hansson , Joerg Roedel , Linus Walleij , linux-pm@vger.kernel.org, linux-gpio@vger.kernel.org, iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] driver: core: Allow subsystems to continue deferring probe Message-ID: <20190605173613.GD27700@kroah.com> References: <20190605142312.6072-1-thierry.reding@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190605142312.6072-1-thierry.reding@gmail.com> User-Agent: Mutt/1.12.0 (2019-05-25) Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On Wed, Jun 05, 2019 at 04:23:12PM +0200, Thierry Reding wrote: > From: Thierry Reding > > Some subsystems, such as pinctrl, allow continuing to defer probe > indefinitely. This is useful for devices that depend on resources > provided by devices that are only probed after the init stage. > > One example of this can be seen on Tegra, where the DPAUX hardware > contains pinmuxing controls for pins that it shares with an I2C > controller. The I2C controller is typically used for communication > with a monitor over HDMI (DDC). However, other instances of the I2C > controller are used to access system critical components, such as a > PMIC. The I2C controller driver will therefore usually be a builtin > driver, whereas the DPAUX driver is part of the display driver that > is loaded from a module to avoid bloating the kernel image with all > of the DRM/KMS subsystem. > > In this particular case the pins used by this I2C/DDC controller > become accessible very late in the boot process. However, since the > controller is only used in conjunction with display, that's not an > issue. > > Unfortunately the driver core currently outputs a warning message > when a device fails to get the pinctrl before the end of the init > stage. That can be confusing for the user because it may sound like > an unwanted error occurred, whereas it's really an expected and > harmless situation. > > In order to eliminate this warning, this patch allows callers of the > driver_deferred_probe_check_state() helper to specify that they want > to continue deferring probe, regardless of whether we're past the > init stage or not. All of the callers of that function are updated > for the new signature, but only the pinctrl subsystem passes a true > value in the new persist parameter if appropriate. > > Signed-off-by: Thierry Reding > --- > drivers/base/dd.c | 17 ++++++++++++----- > drivers/base/power/domain.c | 2 +- > drivers/iommu/of_iommu.c | 2 +- > drivers/pinctrl/devicetree.c | 10 ++++++---- > include/linux/device.h | 2 +- > 5 files changed, 21 insertions(+), 12 deletions(-) > > diff --git a/drivers/base/dd.c b/drivers/base/dd.c > index 0df9b4461766..25ffbadf4187 100644 > --- a/drivers/base/dd.c > +++ b/drivers/base/dd.c > @@ -238,23 +238,30 @@ __setup("deferred_probe_timeout=", deferred_probe_timeout_setup); > /** > * driver_deferred_probe_check_state() - Check deferred probe state > * @dev: device to check > + * @persist: Boolean flag indicating whether drivers should keep trying to > + * probe after built-in drivers have had a chance to probe. This is useful > + * for built-in drivers that rely on resources provided by modular drivers. Functionality aside, having random boolean flags in a function parameter list is a horrid way to have an api. Now when you see a call to this function with either "true" or "false" as an option, you have no idea what that means. So you need to go look up this definition and then work backwards. So even if the idea is sane (I'm not saying that), this implementation is not acceptable at all. thanks, greg k-h