Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754332AbcDKLqT (ORCPT ); Mon, 11 Apr 2016 07:46:19 -0400 Received: from mail-wm0-f54.google.com ([74.125.82.54]:38456 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754150AbcDKLqQ (ORCPT ); Mon, 11 Apr 2016 07:46:16 -0400 Date: Mon, 11 Apr 2016 13:46:12 +0200 From: Thierry Reding To: Jon Hunter Cc: Mark Brown , Liam Girdwood , linux-kernel@vger.kernel.org, Javier Martinez Canillas Subject: Re: [PATCH 1/5] regulator: core: Resolve supply earlier Message-ID: <20160411114612.GD17743@ulmo.ba.sec> References: <1460038959-21592-1-git-send-email-thierry.reding@gmail.com> <570B8376.6030505@nvidia.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="RhUH2Ysw6aD5utA4" Content-Disposition: inline In-Reply-To: <570B8376.6030505@nvidia.com> User-Agent: Mutt/1.6.0 (2016-04-01) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3969 Lines: 103 --RhUH2Ysw6aD5utA4 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Apr 11, 2016 at 11:59:02AM +0100, Jon Hunter wrote: > Hi Thierry, >=20 > On 07/04/16 15:22, Thierry Reding wrote: > > From: Thierry Reding > >=20 > > Subsequent patches will need access to the parent supply from within the > > set_machine_constraints() function to properly implement bypass mode. If > > the parent supply hasn't been resolved by that time the voltage can't be > > queried. > >=20 > > Also, by making sure the supply is resolved early most of the changes in > > set_machine_constraints() don't have to be undone if resolution fails. > >=20 > > Suggested-by: Mark Brown > > Signed-off-by: Thierry Reding > > --- > > drivers/regulator/core.c | 27 +++++++++++++++++++++------ > > 1 file changed, 21 insertions(+), 6 deletions(-) > >=20 > > diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c > > index 2786d251b1cc..cc0333a79924 100644 > > --- a/drivers/regulator/core.c > > +++ b/drivers/regulator/core.c > > @@ -3972,18 +3972,27 @@ regulator_register(const struct regulator_desc = *regulator_desc, > > =20 > > dev_set_drvdata(&rdev->dev, rdev); > > =20 > > + if (init_data && init_data->supply_regulator) > > + rdev->supply_name =3D init_data->supply_regulator; > > + else if (regulator_desc->supply_name) > > + rdev->supply_name =3D regulator_desc->supply_name; > > + > > + /* > > + * set_machine_constraints() needs the supply to be resolved in order > > + * to support querying the current voltage in bypass mode. Resolve it > > + * here to more easily handle deferred probing. > > + */ > > + ret =3D regulator_resolve_supply(rdev); > > + if (ret < 0) > > + goto scrub; > > + >=20 > Thanks for sending this. However, I think that calling > regulator_resolve_supply() can cause a deadlock, because the > regulator_list_mutex is held at this point and > regulator_resolve_supply() calls regulator_dev_lookup() which may try to > request the mutex again. True... I never encountered that case in my testing. I'm not sure exactly why, though. > So may be we need to move this call after the call to > regulator_of_get_init_data() before we acquire the mutex. I don't think that'll work. regulator_resolve_supply() depends on some operations performed much later (such as rdev->dev.parent being set). Perhaps moving the locking of the regulator_list_mutex down instead could work. It seems to me like the first place where it would need to be held is set_machine_constraints(). > Also, if we add this call, then I am wondering if we still need ... >=20 > class_for_each_device(®ulator_class, NULL, NULL, > regulator_register_resolve_supply); Possibly not. That line was introduced to hook up existing orphan regulators with their parents when they were registered, but I guess since we now always defer probe if a parent isn't registered yet the line would become a no-op. Thierry --RhUH2Ysw6aD5utA4 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJXC46BAAoJEN0jrNd/PrOhieQQAIrjzQlXEmyQ9Ipynia4gzf8 /O6uzW2kvB4wnwb05RiJeW7AhVOrD3hDx24IeuebKvHc00ZGdhYoW3rMfqnaHxt1 z4SEGRQsouZMtYt7DFcQhs25SqcYiBaDARze2NOjzyICUUf6p1+bL602cLIxlLFY IGA7dnzrwpm5GABoFPyEbkLuYLH99Hcrw3SMMqBdbAH86gIz1qEWnjduwt4Lr9zy qBhHCOVhuvgAnAtRlF4CfPT2gBd5qYIu8mepGMzHi2u8wIy/YrxNvspvHeqatsT7 IrWsu0yPtipDY6gy584LozdusYGftaww/aOjUkKx1oU0S6T7Ot4XXiiQ/rt4dVrH PR5HSjZegYsUnLByybRvNcDIgACitBYJ2QrR8cDNMVELvuGgelfPiibkWzWiVBeR oH5Kxb0oVzLrNb0DGOYs+VPRT6PdNmi++dRvVDKv8m9ympkxgbA/gNv8JTZdtqOp 6zld8Cy7TGOh7u6EZh5seDeAUnLhNdrXP+fZv3G/+P6enPrrbZHg5xgPNYdya0ES 8vHqM+VjD1BrjK38SclzD7GYlzhI5W3rIvoqZHHlEm6qqFKVzg/s6J0u1jhX7IuE 3OX6xWUdP1ECIVhSwFpm2b0UaiyEo69q/CIf4jwhog0PfWSbelnW1Cfk4kIN80xw gXeQQbEpoYuDHPdwlvNO =SLSP -----END PGP SIGNATURE----- --RhUH2Ysw6aD5utA4--