From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758221AbcFHRQP (ORCPT ); Wed, 8 Jun 2016 13:16:15 -0400 Received: from mezzanine.sirena.org.uk ([106.187.55.193]:50996 "EHLO mezzanine.sirena.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757575AbcFHRQO (ORCPT ); Wed, 8 Jun 2016 13:16:14 -0400 Date: Wed, 8 Jun 2016 18:16:08 +0100 From: Mark Brown To: WEN Pingbo Cc: linux-kernel@vger.kernel.org, lgirdwood@gmail.com, vincent.guittot@linaro.org, stephen.boyd@linaro.org Message-ID: <20160608171608.GM7510@sirena.org.uk> References: <1462777508-24934-1-git-send-email-pingbo.wen@linaro.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="DMotDPdpQlD4ewOK" Content-Disposition: inline In-Reply-To: <1462777508-24934-1-git-send-email-pingbo.wen@linaro.org> X-Cookie: We've upped our standards, so up yours! User-Agent: Mutt/1.6.0 (2016-04-01) X-SA-Exim-Connect-IP: 2a01:348:6:8808:fab::3 X-SA-Exim-Mail-From: broonie@sirena.org.uk Subject: Re: [RFC PATCH] regulator: introduce boot protection flag X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on mezzanine.sirena.org.uk) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --DMotDPdpQlD4ewOK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Mon, May 09, 2016 at 03:05:08PM +0800, WEN Pingbo wrote: > In some platforms, critical shared regulator is initialized in > bootloader. But during kernel booting, the driver probing order and > conflicting operations from other regulator consumers, may set the > regulator in a undefined state, which will cause serious problem. > This patch try to add a boot_protection flag in regulator constraints. This still feels like a short term hack that doesn't belong in an ABI. It's all very implementation specific and not very robust, it's not describing the outcome we're looking for but rather a very specific behaviour which won't work outside of a fairly narrow system configuration. The difficulty in coherently explaining what the end of boot is and what protection means is a big warning sign here. I think you need to be looking at some combination of getting the devices you're interested in started up early and more precisely describing the end result you're trying to achieve. The issues with probe deferral do seem related here, it's another symptom of not really making any decisions about init ordering and so sometimes making bad ones. > And regulator core will postpone all operations until all consumers > have taked their place. It doesn't, it postpones them until late_initacall(). This is both after the consumers have loaded if they are built in and before any consumers built as modules come up. > The boot_protection flag only work before late_initicall. And as other > constraints liked, you can specify this flag in a board file, or in > dts file. Anything added to the DT ABI needs a binding. > + /* constraints check has already done */ > + if (rdev->boot_mode) > + rdev->desc->ops->set_mode(rdev, rdev->boot_mode); This whole sequence of code ignores errors - that's not great. We should at least log them. > + mutex_unlock(&rdev->mutex); > + > + if (regulator) > + regulator_set_voltage(regulator, regulator->min_uV, > + regulator->max_uV); That's... exciting. There's a couple of issues here. One is that this is not operating on the rdev but rather on a consumer regulator device, the other is that we drop out of the lock before doing the update which tends to be a warning sign that something fun is going on and at least an internal function should be used. These two most likely come down to the same issue. --DMotDPdpQlD4ewOK Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJXWFLWAAoJECTWi3JdVIfQaYUH/1DqFliml3STfq07w9I9oYOX ctwpK+9QG7L3zURReg52gmH7I5cmHJrKM9DZNzMWLfBbK8KQjShVRaoUgZGBvT05 PVdPoK9fSfPXEooCrUqHFzbHUoUNM54rubwKwauNM1hHb7Qlmwwx9LSqZW8T1QBb S2uS3qKT3qUT+t7dYlvSC67jSKs5QpFiVxleOT5rRBk69p4zV8iyQNrTXVXS19Ft p6iGv3/lK9XR8tmBzvHFFM46IejhQwvgUZB71XS+wL5wLw9uBgkpqezHHk2YDeWv rdQ6efe8kfXdusj4Rixc8S6gn+316k9oDjVxsj51aJDHSa5DBDGSxy7v+RB4Efk= =sZK2 -----END PGP SIGNATURE----- --DMotDPdpQlD4ewOK--