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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 45FC0C433FE for ; Thu, 28 Oct 2021 11:50:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 2D9C4610FC for ; Thu, 28 Oct 2021 11:50:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230261AbhJ1Lwp (ORCPT ); Thu, 28 Oct 2021 07:52:45 -0400 Received: from mail.kernel.org ([198.145.29.99]:41868 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229578AbhJ1Lwl (ORCPT ); Thu, 28 Oct 2021 07:52:41 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 97A0D60FF2; Thu, 28 Oct 2021 11:50:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1635421814; bh=hgapdR6qmDmcLaknmcSzOPWpHEWQJN4oJ2A60mUz22E=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KgMLQjA40UB0Oac51b5wK4aZ7xx0UMxlA4RAJ16ovhhwvvbxhk7D4b70Pz/+S8uSz r+i/F2HTqDF09Ofl7Es6xWKpmGTaaJWS2Yg/KMpUNvmbDwt8XF8U4q4ithCUrDt4Q3 umCl434jYw4vBlhgxhsJLAGCqWZ0GruZvTfwBWhaWkNCbci8o0/GWsPcc+dciVY8PN LgnycatH/yG9IgJSfAODHTqc3cIw6zTU7MEngZgPi6+JrylHJg5ffOC67ARaq3N7B9 Zyy/B2tYwUvjZ4DjksV3jOREt/J9yGIViU5jqEkpH36q0mOD5SuHVXU+qs2tdHWYno mBNIa7ZW6WJ/g== Date: Thu, 28 Oct 2021 12:50:08 +0100 From: Mark Brown To: Richard Zhu Cc: Francesco Dolcini , "l.stach@pengutronix.de" , "bhelgaas@google.com" , "lorenzo.pieralisi@arm.com" , "jingoohan1@gmail.com" , "linux-pci@vger.kernel.org" , dl-linux-imx , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "kernel@pengutronix.de" Subject: Re: [PATCH v3 3/7] PCI: imx6: Fix the regulator dump when link never came up Message-ID: References: <1634886750-13861-1-git-send-email-hongxing.zhu@nxp.com> <1634886750-13861-4-git-send-email-hongxing.zhu@nxp.com> <20211025111312.GA31419@francesco-nb.int.toradex.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="RKFvMGWaPRDm/WJh" Content-Disposition: inline In-Reply-To: X-Cookie: try again Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --RKFvMGWaPRDm/WJh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Oct 28, 2021 at 06:50:58AM +0000, Richard Zhu wrote: > > I would be really surprised to see PCI hardware that was able to support a > > supply being physically absent, and this use of _is_enabled() is quite > > simply not how any of this is supposed to work in the regulator API even > > for regulators that can be optional. > [Richard Zhu] Actually, this regulator is one GPIO fixed regulator. > Controlled by SW to turn on (GPIO high) or turn off (GPIO low) the supply. > In some boards designs, this supply might be always on(GPIO high). > So, in point of SW driver view, this regulator is optional. No, it's not. The regulator API supports the systems where the regualtor is always on perfectly well, the client driver should not need to do anything to support them. > > Perhaps it's not causing problems in this design but if the supply is ever > > shared with anything else then the software will run into trouble. > > There will also be problems with the error handling on a system where > > the regulator needs to be controlled. > [Richard Zhu] This GPIO fixed regulator is only used by controller driver. > It makes sense to disable the enabled regulator when driver probe is failed. The driver should undo any enables it did itself, it should not undo any enables that anything else did which means it should never be basing decisions on regulator_is_enabled(). While the regulator may not be shared in the particular board you're looking at it may be shared in other systems. --RKFvMGWaPRDm/WJh Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmF6jm8ACgkQJNaLcl1U h9BLxgf/Zm+IGEbLXWh7e1XuPxfanEkwKtL8XvVb+F4KlVIKego05HBRd7BLknFd /HzU7BxR6iGIJE9KlLgl9p7qL6n6RZXatqZTFdkqElP/LS0WdwofBU0mYngVwUoJ Bt9LDQBMLxU/elYGbWCDTEQ1OmZc54if7ty5z+UQuZib/L8qQqF3hDKci5jcUsOx lPr2+lvxzDlbHRGZxseE7TZMcKtdUri58cBttYQTOgptD5HPwgD1gyx/Vg0rdimo xfCY52Hrv7C3UToRss7CySnSZV29AY3lONKh3IjtvMqpvNo2gDWIyFARtpM/5Nob ZJEZ+qQg+AjCsze/u0ox6CQgajm+SA== =HgqY -----END PGP SIGNATURE----- --RKFvMGWaPRDm/WJh-- 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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9DB2CC433F5 for ; Thu, 28 Oct 2021 11:51:41 +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 596FD61056 for ; Thu, 28 Oct 2021 11:51:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 596FD61056 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: List-Subscribe:List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: In-Reply-To:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Reply-To:Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date :Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=bC0sVQW76SdfAqtjogyGiiCwOjoyQ8ItGTtaLbMSQnY=; b=Ljg4q42tPLDmdrn9TcJiBoKV0S eVDaUvsjUqqxPf4xLkNpuPGpphaYgXKfHCLBoveDVB0ICZD7G58y38eJqYDd3Cbxu+AVOipbsrDG+ aI1ib6QZAaYou9/hOn82OZg+PAXEcXh9mm81aPiicqm5eyRLsATsaOaCqm9oPRkBvh4uzS8Uk5fug QmlYcOcY8VOfpahkHN4O+mLcCWYNwZ/euHB0pKo6JXXxldJXhksJca2BQcA6qRU2as973Iu0glcWU g1PFMDtoFKDyECq2uaxthV9noM2vkx8C0EY9rDWgluq52tdmQyk854HXQe0hRcHpjiYw6KN4vIc+h e8H4EQDQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mg3vH-007jEF-06; Thu, 28 Oct 2021 11:50:19 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mg3vD-007jDt-Iu for linux-arm-kernel@lists.infradead.org; Thu, 28 Oct 2021 11:50:16 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 97A0D60FF2; Thu, 28 Oct 2021 11:50:12 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1635421814; bh=hgapdR6qmDmcLaknmcSzOPWpHEWQJN4oJ2A60mUz22E=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=KgMLQjA40UB0Oac51b5wK4aZ7xx0UMxlA4RAJ16ovhhwvvbxhk7D4b70Pz/+S8uSz r+i/F2HTqDF09Ofl7Es6xWKpmGTaaJWS2Yg/KMpUNvmbDwt8XF8U4q4ithCUrDt4Q3 umCl434jYw4vBlhgxhsJLAGCqWZ0GruZvTfwBWhaWkNCbci8o0/GWsPcc+dciVY8PN LgnycatH/yG9IgJSfAODHTqc3cIw6zTU7MEngZgPi6+JrylHJg5ffOC67ARaq3N7B9 Zyy/B2tYwUvjZ4DjksV3jOREt/J9yGIViU5jqEkpH36q0mOD5SuHVXU+qs2tdHWYno mBNIa7ZW6WJ/g== Date: Thu, 28 Oct 2021 12:50:08 +0100 From: Mark Brown To: Richard Zhu Cc: Francesco Dolcini , "l.stach@pengutronix.de" , "bhelgaas@google.com" , "lorenzo.pieralisi@arm.com" , "jingoohan1@gmail.com" , "linux-pci@vger.kernel.org" , dl-linux-imx , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "kernel@pengutronix.de" Subject: Re: [PATCH v3 3/7] PCI: imx6: Fix the regulator dump when link never came up Message-ID: References: <1634886750-13861-1-git-send-email-hongxing.zhu@nxp.com> <1634886750-13861-4-git-send-email-hongxing.zhu@nxp.com> <20211025111312.GA31419@francesco-nb.int.toradex.com> MIME-Version: 1.0 In-Reply-To: X-Cookie: try again X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211028_045015_712370_F2FE6B4B X-CRM114-Status: GOOD ( 24.36 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: multipart/mixed; boundary="===============2090591176431135847==" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org --===============2090591176431135847== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="RKFvMGWaPRDm/WJh" Content-Disposition: inline --RKFvMGWaPRDm/WJh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Oct 28, 2021 at 06:50:58AM +0000, Richard Zhu wrote: > > I would be really surprised to see PCI hardware that was able to support a > > supply being physically absent, and this use of _is_enabled() is quite > > simply not how any of this is supposed to work in the regulator API even > > for regulators that can be optional. > [Richard Zhu] Actually, this regulator is one GPIO fixed regulator. > Controlled by SW to turn on (GPIO high) or turn off (GPIO low) the supply. > In some boards designs, this supply might be always on(GPIO high). > So, in point of SW driver view, this regulator is optional. No, it's not. The regulator API supports the systems where the regualtor is always on perfectly well, the client driver should not need to do anything to support them. > > Perhaps it's not causing problems in this design but if the supply is ever > > shared with anything else then the software will run into trouble. > > There will also be problems with the error handling on a system where > > the regulator needs to be controlled. > [Richard Zhu] This GPIO fixed regulator is only used by controller driver. > It makes sense to disable the enabled regulator when driver probe is failed. The driver should undo any enables it did itself, it should not undo any enables that anything else did which means it should never be basing decisions on regulator_is_enabled(). While the regulator may not be shared in the particular board you're looking at it may be shared in other systems. --RKFvMGWaPRDm/WJh Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmF6jm8ACgkQJNaLcl1U h9BLxgf/Zm+IGEbLXWh7e1XuPxfanEkwKtL8XvVb+F4KlVIKego05HBRd7BLknFd /HzU7BxR6iGIJE9KlLgl9p7qL6n6RZXatqZTFdkqElP/LS0WdwofBU0mYngVwUoJ Bt9LDQBMLxU/elYGbWCDTEQ1OmZc54if7ty5z+UQuZib/L8qQqF3hDKci5jcUsOx lPr2+lvxzDlbHRGZxseE7TZMcKtdUri58cBttYQTOgptD5HPwgD1gyx/Vg0rdimo xfCY52Hrv7C3UToRss7CySnSZV29AY3lONKh3IjtvMqpvNo2gDWIyFARtpM/5Nob ZJEZ+qQg+AjCsze/u0ox6CQgajm+SA== =HgqY -----END PGP SIGNATURE----- --RKFvMGWaPRDm/WJh-- --===============2090591176431135847== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel --===============2090591176431135847==--