Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 31 Jul 2019 22:23:35 +0100
From:   Mark Brown <broonie@kernel.org>
To:     Philippe Schenker <philippe.schenker@toradex.com>
Cc:     "lgirdwood@gmail.com" <lgirdwood@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RFC PATCH 1/2] Regulator: Core: Add clock-enable to
 fixed-regulator
Message-ID: <20190731212335.GL4369@sirena.org.uk>
References: <20190730173006.15823-1-dev@pschenker.ch>
 <20190730173006.15823-2-dev@pschenker.ch>
 <20190730181038.GK4264@sirena.org.uk>
 <b5e1cc3fb5838d9ea4160078402bff95903ba0da.camel@toradex.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
        protocol="application/pgp-signature"; boundary="TdMwOTenGjBWB1uY"
Content-Disposition: inline
In-Reply-To: <b5e1cc3fb5838d9ea4160078402bff95903ba0da.camel@toradex.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--TdMwOTenGjBWB1uY
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Jul 30, 2019 at 09:00:01PM +0000, Philippe Schenker wrote:
> On Tue, 2019-07-30 at 19:10 +0100, Mark Brown wrote:
> > On Tue, Jul 30, 2019 at 07:30:04PM +0200, Philippe Schenker wrote:

> > > This adds the possibility to enable a fixed-regulator with a clock.

> > Why?  What does the hardware which makes this make sense look like?

> Tomorrow I can provide some schematics if needed. But its just a simple
> switch that is switched by a clock (on when clock is on and off when
> clock is off). This clock is the RGMII 50MHz clock for the ethernet
> PHY.

So it's not switching with the clock, the circuit somehow keeps the
switch latched?

> Why is a regulator even needed?
> - On power up of the PHY there is a huge time I have to wait for
> voltage rail to settle. In the range of 100ms.
> - Because there is a switch in the circuit I abstract it with a
> regulator-fixed in devicetree to make use of the startup-delay.
> - This regulator/switch is enabled with a clock. So to be able to use
> the startup delay I need an enable-by-clock on regulator-fixed.

It does feel like it might be simpler to just handle this as a quirk in
the PHY or ethernet driver, this feels like an awful lot of work to
add a sleep on what's probably only going to ever be one system.

> Why do I think this should be in core?
> - Normally this task is done with gpio that is already in regulator-
> core.
> - Because that is already there I added the functionality for enabled-
> by-clock-functionality.
> - I thought of creating a new regulator-clock driver but that would
> hold a lot of code duplication from regulator-fixed.

Hopefully not a *lot* of duplication.  The GPIOs are handled in the core
because they're really common and used by many regulator devices, the
same will I hope not be true for clocks.

> Why is this a good Idea at all?
> - Well I'm here for the software part and should just support our
> hardware. If that is a good Idea at all I don't know, for sure it is
> not a solution that is from some school-book. But I tried it and
> measured it out and it seems to work pretty fine.
> - The reason behind all of that is limited GPIO availability from the
> iMX6ULL.

I guess my question here is what the trip through the regulator API buys
us - it's a bit of a sledgehammer to crack a nut thing.

--TdMwOTenGjBWB1uY
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAl1CBtcACgkQJNaLcl1U
h9BslQf/aeF1Faxrh/xk/9g7IdzY8k5d73wxX6BrfKWrHMJNi4YHOUw4uCeHs/kh
W5z5AoEaqzPNIxENappg2KULLrNNMqtXqArS0TXeaqLvm0u1zXuAYVDxXzQ+S6R1
CH1EyZbHb7e56AKunLAfrxrOVy/NS0jMGIGjLslrxJYCwA5BAJwkGlTdK358u7OG
UrrDOjoLxQxDZBzbnx0+BtlNkJoaPgjXNTSYuQRekdN19vG3+7s8/cOil6K7dF+d
tCM6+yiI/32YVHvOlrdiSeYW3iaHqK3DoUhETPUCiEu3W3FYeB1hT35uiFxKyn8J
KCaHwVksAVm6z6O0840d8QSoPJDPoA==
=HHd2
-----END PGP SIGNATURE-----

--TdMwOTenGjBWB1uY--