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=-5.0 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 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 E0ABEC2D0E5 for ; Sun, 29 Mar 2020 18:22:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B5CBA2073E for ; Sun, 29 Mar 2020 18:22:51 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="fazfmfEZ" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728637AbgC2SWv (ORCPT ); Sun, 29 Mar 2020 14:22:51 -0400 Received: from pandora.armlinux.org.uk ([78.32.30.218]:40694 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728467AbgC2SWu (ORCPT ); Sun, 29 Mar 2020 14:22:50 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=ul5mPuoITZiwsLENljEvGuoEiNdCieYNeoF+CgVtynY=; b=fazfmfEZAjksByCU8DCpoS9F+ Nr90jbUdAi42KYu8HacKRPIMwtSxS8gPV08oocVVKktH55e304YKnr90bFNgyYt2QdC6Xo/x1JCLe MeRgQnh2C3QeyvqYQ8+L1rm2X7XpkYeZf2LSddGAxZZwyi/MC5qfNUH7IICpLRcznIUswY/hP9LbD G4Sr5nykVqVuGxItgtmgYImpYs/uMsRUiZ77JSOE+KLbqGB6hWPDdTpsOER/1oj+L3kuk1YV4S2MR F37blPxMy0YhqGydl4ZS6VnWEQ9q7PCOx7sAvRpULKz8Nt04SRvjE0A+hPeGf4wkCK6cklh8gZUlC etqkiKGaA==; Received: from shell.armlinux.org.uk ([2001:4d48:ad52:3201:5054:ff:fe00:4ec]:59598) by pandora.armlinux.org.uk with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jIca0-0006Mb-Bc; Sun, 29 Mar 2020 19:22:40 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.92) (envelope-from ) id 1jIcZw-0006QA-JB; Sun, 29 Mar 2020 19:22:36 +0100 Date: Sun, 29 Mar 2020 19:22:36 +0100 From: Russell King - ARM Linux admin To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= Cc: Mark Rutland , Andrew Lunn , Jason Cooper , devicetree@vger.kernel.org, Linus Walleij , linux-pwm@vger.kernel.org, Bartosz Golaszewski , Rob Herring , Thierry Reding , linux-gpio@vger.kernel.org, Gregory Clement , linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth Subject: Re: [PATCH RFC 2/6] gpio: mvebu: honour EPROBE_DEFER for devm_clk_get() Message-ID: <20200329182236.GC25745@shell.armlinux.org.uk> References: <20200329104549.GX25745@shell.armlinux.org.uk> <20200329131659.4hbshjst4ccvje2n@pengutronix.de> <20200329133400.GA25745@shell.armlinux.org.uk> <20200329180056.cwju3zqviwnwwjd6@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20200329180056.cwju3zqviwnwwjd6@pengutronix.de> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-gpio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org On Sun, Mar 29, 2020 at 08:00:56PM +0200, Uwe Kleine-König wrote: > Hello Russell, > > On Sun, Mar 29, 2020 at 02:34:00PM +0100, Russell King - ARM Linux admin wrote: > > On Sun, Mar 29, 2020 at 03:16:59PM +0200, Uwe Kleine-König wrote: > > > On Sun, Mar 29, 2020 at 11:48:09AM +0100, Russell King wrote: > > > > diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c > > > > index fa5641615db6..ee13b11c5298 100644 > > > > --- a/drivers/gpio/gpio-mvebu.c > > > > +++ b/drivers/gpio/gpio-mvebu.c > > > > @@ -1132,6 +1132,9 @@ static int mvebu_gpio_probe(struct platform_device *pdev) > > > > } > > > > > > > > mvchip->clk = devm_clk_get(&pdev->dev, NULL); > > > > + if (mvchip->clk == ERR_PTR(-EPROBE_DEFER)) > > > > + return -EPROBE_DEFER; > > > > + > > > > /* Not all SoCs require a clock.*/ > > > > if (!IS_ERR(mvchip->clk)) > > > > clk_prepare_enable(mvchip->clk); > > > > > > I'd say the following is the right thing to do here: > > > > > > mvchip->clk = devm_clk_get_optional(...); > > > if (IS_ERR(mvchip->clk)) > > > return ... > > > > It's not that simple. The clock is required for Armada 370, and is > > optional for Armada 8040. > > I'd say it is still the right approach here. On Armada 370 the dtb then > has a clk and on Armada 8040 it doesn't. So if with > devm_clk_get_optional() something goes wrong that's because the dtb is > wrong. And in fact the handling is even better than with your suggested > patch as every error (but EPROBE_DEFER) is ignored instead of passed to > the caller with your (and the existing) approach. Sort of. Every error is currently treated as "no clock", and only later does such an error become fatal in the driver _if_ PWM is configured into the kernel and we're running on Armada 370. If PWM is disabled in the kernel, or on some other SoC, then the driver doesn't care whether getting the clock reported any kind of error. Your proposal is to always treat any error getting the clock, irrespective of whether there is PWM or not, as a fatal error for the driver. That is an entirely seperate functional change. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up From mboxrd@z Thu Jan 1 00:00:00 1970 From: Russell King - ARM Linux admin Subject: Re: [PATCH RFC 2/6] gpio: mvebu: honour EPROBE_DEFER for devm_clk_get() Date: Sun, 29 Mar 2020 19:22:36 +0100 Message-ID: <20200329182236.GC25745@shell.armlinux.org.uk> References: <20200329104549.GX25745@shell.armlinux.org.uk> <20200329131659.4hbshjst4ccvje2n@pengutronix.de> <20200329133400.GA25745@shell.armlinux.org.uk> <20200329180056.cwju3zqviwnwwjd6@pengutronix.de> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Return-path: Content-Disposition: inline In-Reply-To: <20200329180056.cwju3zqviwnwwjd6-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= Cc: Mark Rutland , Andrew Lunn , Jason Cooper , devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linus Walleij , linux-pwm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Bartosz Golaszewski , Rob Herring , Thierry Reding , linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Gregory Clement , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Sebastian Hesselbarth List-Id: linux-pwm@vger.kernel.org On Sun, Mar 29, 2020 at 08:00:56PM +0200, Uwe Kleine-König wrote: > Hello Russell, > > On Sun, Mar 29, 2020 at 02:34:00PM +0100, Russell King - ARM Linux admin wrote: > > On Sun, Mar 29, 2020 at 03:16:59PM +0200, Uwe Kleine-König wrote: > > > On Sun, Mar 29, 2020 at 11:48:09AM +0100, Russell King wrote: > > > > diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c > > > > index fa5641615db6..ee13b11c5298 100644 > > > > --- a/drivers/gpio/gpio-mvebu.c > > > > +++ b/drivers/gpio/gpio-mvebu.c > > > > @@ -1132,6 +1132,9 @@ static int mvebu_gpio_probe(struct platform_device *pdev) > > > > } > > > > > > > > mvchip->clk = devm_clk_get(&pdev->dev, NULL); > > > > + if (mvchip->clk == ERR_PTR(-EPROBE_DEFER)) > > > > + return -EPROBE_DEFER; > > > > + > > > > /* Not all SoCs require a clock.*/ > > > > if (!IS_ERR(mvchip->clk)) > > > > clk_prepare_enable(mvchip->clk); > > > > > > I'd say the following is the right thing to do here: > > > > > > mvchip->clk = devm_clk_get_optional(...); > > > if (IS_ERR(mvchip->clk)) > > > return ... > > > > It's not that simple. The clock is required for Armada 370, and is > > optional for Armada 8040. > > I'd say it is still the right approach here. On Armada 370 the dtb then > has a clk and on Armada 8040 it doesn't. So if with > devm_clk_get_optional() something goes wrong that's because the dtb is > wrong. And in fact the handling is even better than with your suggested > patch as every error (but EPROBE_DEFER) is ignored instead of passed to > the caller with your (and the existing) approach. Sort of. Every error is currently treated as "no clock", and only later does such an error become fatal in the driver _if_ PWM is configured into the kernel and we're running on Armada 370. If PWM is disabled in the kernel, or on some other SoC, then the driver doesn't care whether getting the clock reported any kind of error. Your proposal is to always treat any error getting the clock, irrespective of whether there is PWM or not, as a fatal error for the driver. That is an entirely seperate functional change. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps up 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=-5.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 8D6F1C43331 for ; Sun, 29 Mar 2020 18:23:09 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 6018C20732 for ; Sun, 29 Mar 2020 18:23:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="hcVAj0px"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=armlinux.org.uk header.i=@armlinux.org.uk header.b="fazfmfEZ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6018C20732 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=armlinux.org.uk Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=hulYu44uqDURwWvmvXogCQO6lCUDQDt1YXkwI6pVOOc=; b=hcVAj0px7SuRKy O8fvvk12TijlgopEy5kPqgj8sbwPfs89Mftciay2lxEvmwkGxnGcFgIU1rwcu0d6WAx6XeJl5NiA/ 6sUxz1LQElwaf2kZu/85esmz1gWuZp+0C9OyIgw4iUIUPpXnULNQ/LTs8UCJdNSTsJxDlNeI5XODe aHk3ZASyQydf5NwDkw+KdNoA2KyOvyHNqjiWiJlAwuxvlOeDlWqipfaLrZR01OV6KiCnxpiOnMufo 6/peMXQVvzkmzW6x2W11hrucvjpQCmhgZJrWJpdc3bOjrJszNQi61KW9B+vbfjBTuHvd7vcWo/y2z X66UrmaG+V5lcYjjP5/Q==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jIcaO-0006bM-PI; Sun, 29 Mar 2020 18:23:04 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jIcaK-0006aZ-KC for linux-arm-kernel@lists.infradead.org; Sun, 29 Mar 2020 18:23:02 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To: Content-Transfer-Encoding:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=ul5mPuoITZiwsLENljEvGuoEiNdCieYNeoF+CgVtynY=; b=fazfmfEZAjksByCU8DCpoS9F+ Nr90jbUdAi42KYu8HacKRPIMwtSxS8gPV08oocVVKktH55e304YKnr90bFNgyYt2QdC6Xo/x1JCLe MeRgQnh2C3QeyvqYQ8+L1rm2X7XpkYeZf2LSddGAxZZwyi/MC5qfNUH7IICpLRcznIUswY/hP9LbD G4Sr5nykVqVuGxItgtmgYImpYs/uMsRUiZ77JSOE+KLbqGB6hWPDdTpsOER/1oj+L3kuk1YV4S2MR F37blPxMy0YhqGydl4ZS6VnWEQ9q7PCOx7sAvRpULKz8Nt04SRvjE0A+hPeGf4wkCK6cklh8gZUlC etqkiKGaA==; Received: from shell.armlinux.org.uk ([2001:4d48:ad52:3201:5054:ff:fe00:4ec]:59598) by pandora.armlinux.org.uk with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jIca0-0006Mb-Bc; Sun, 29 Mar 2020 19:22:40 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.92) (envelope-from ) id 1jIcZw-0006QA-JB; Sun, 29 Mar 2020 19:22:36 +0100 Date: Sun, 29 Mar 2020 19:22:36 +0100 From: Russell King - ARM Linux admin To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= Subject: Re: [PATCH RFC 2/6] gpio: mvebu: honour EPROBE_DEFER for devm_clk_get() Message-ID: <20200329182236.GC25745@shell.armlinux.org.uk> References: <20200329104549.GX25745@shell.armlinux.org.uk> <20200329131659.4hbshjst4ccvje2n@pengutronix.de> <20200329133400.GA25745@shell.armlinux.org.uk> <20200329180056.cwju3zqviwnwwjd6@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200329180056.cwju3zqviwnwwjd6@pengutronix.de> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200329_112300_831562_88DCC02E X-CRM114-Status: GOOD ( 19.74 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Andrew Lunn , Jason Cooper , linux-pwm@vger.kernel.org, Linus Walleij , Bartosz Golaszewski , devicetree@vger.kernel.org, Rob Herring , Thierry Reding , linux-gpio@vger.kernel.org, Gregory Clement , linux-arm-kernel@lists.infradead.org, Sebastian Hesselbarth Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Sun, Mar 29, 2020 at 08:00:56PM +0200, Uwe Kleine-K=F6nig wrote: > Hello Russell, > = > On Sun, Mar 29, 2020 at 02:34:00PM +0100, Russell King - ARM Linux admin = wrote: > > On Sun, Mar 29, 2020 at 03:16:59PM +0200, Uwe Kleine-K=F6nig wrote: > > > On Sun, Mar 29, 2020 at 11:48:09AM +0100, Russell King wrote: > > > > diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c > > > > index fa5641615db6..ee13b11c5298 100644 > > > > --- a/drivers/gpio/gpio-mvebu.c > > > > +++ b/drivers/gpio/gpio-mvebu.c > > > > @@ -1132,6 +1132,9 @@ static int mvebu_gpio_probe(struct platform_d= evice *pdev) > > > > } > > > > = > > > > mvchip->clk =3D devm_clk_get(&pdev->dev, NULL); > > > > + if (mvchip->clk =3D=3D ERR_PTR(-EPROBE_DEFER)) > > > > + return -EPROBE_DEFER; > > > > + > > > > /* Not all SoCs require a clock.*/ > > > > if (!IS_ERR(mvchip->clk)) > > > > clk_prepare_enable(mvchip->clk); > > > = > > > I'd say the following is the right thing to do here: > > > = > > > mvchip->clk =3D devm_clk_get_optional(...); > > > if (IS_ERR(mvchip->clk)) > > > return ... > > = > > It's not that simple. The clock is required for Armada 370, and is > > optional for Armada 8040. > = > I'd say it is still the right approach here. On Armada 370 the dtb then > has a clk and on Armada 8040 it doesn't. So if with > devm_clk_get_optional() something goes wrong that's because the dtb is > wrong. And in fact the handling is even better than with your suggested > patch as every error (but EPROBE_DEFER) is ignored instead of passed to > the caller with your (and the existing) approach. Sort of. Every error is currently treated as "no clock", and only later does such an error become fatal in the driver _if_ PWM is configured into the kernel and we're running on Armada 370. If PWM is disabled in the kernel, or on some other SoC, then the driver doesn't care whether getting the clock reported any kind of error. Your proposal is to always treat any error getting the clock, irrespective of whether there is PWM or not, as a fatal error for the driver. That is an entirely seperate functional change. -- = RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 10.2Mbps down 587kbps = up _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel