Message-ID: <0b51a86ad6ee7e143506501937863cd8559244ec.camel@toradex.com>
Date:   Mon, 5 Aug 2019 11:07:58 +0000
From:   Philippe Schenker <philippe.schenker@...adex.com>
To:     "broonie@...nel.org" <broonie@...nel.org>
CC:     "lgirdwood@...il.com" <lgirdwood@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH 1/2] Regulator: Core: Add clock-enable to
 fixed-regulator

On Wed, 2019-07-31 at 22:23 +0100, Mark Brown wrote:
> 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?

No, it doesn't keep it latched. To make things clear here a status table:

+----------+-----------+
| Clock    | Switch    |
+----------+-----------+
| enabled  | +3.3V on  |
+----------+-----------+
| disabled | +3.3V off |
+--------- +-----------+

I tried to do schematics in ASCII, for documentation purpose on ML:

+V3.3 >-----------------------+-------+--┐ +-----------> +3.3V_ETH
                              |       |  ^ |              switched
                              |       ---┴--         supply to PHY
                              |       |¯¯¯¯¯
                             +-+      | p-FET
                           R | |      |
                             | |      |
                             +-+      |
                              |       |
                      n-FET   +-------+
                          ||--┘       |
RMII_CLK_50MHz  | |       ||<-┐       |
----------------| |---+___||  |     -----
                | |   |       |     -----
              C       |       |   C   |
                     +-+      |       |
                   R | |      |       |
                     | |      |       |
                     +-+      |       |
                      |       |       |
                      |       |       |
                     +++     +++     +++
                     GND     GND     GND


If it shouldn't be clear here a picture:
https://share.toradex.com/v9d7a97mvq1ks62

> 
> > 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.

I thought of that too, but the problem with that approach is that I can't reflect the actual switching behavior. What would happen is if you turnethernet off with 'ip link set eth0 down', the clock would stop and therefore
no more supply voltage to the PHY. But the ethernet driverwould in that case
let the regulator enabled preventing, switching off the clock.

Anyway I feel that to solve this with a quirk would be a little hackish, plus I'd anyway need to mess around with the Ethernet/PHY drivers. So why not solve it properly with a regulator that supports clocks?

> 
> > 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.

I agree that they are commonly and widely used. To add support for clocks in
regulator-core was really easy to do as I did it the same way as it is done with
gpio's. If I don't need to touch regulator-core I don't want to. But as I said
it was really easy for me to integrate it in there in a way without even
understanding the whole regulator API.

If it makes more sense to do it in a new file like clock-regulator.c and
creating a new compatible that is what I'm trying to find out here. I'd be happy
to write also a new clock-regulator driver for that purpose.

> 
> > 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.

In my opinion this is not only about to solve my problem with startup-delay. I
think that this is really a behavior that can be generic. That's also why I'm
asking here how we want to solve that so not only I solve my little problem with
a board quirk but in a broader view for possible future usage by others.

It is possible that a regulator needs a clock. That exactly is, what we have on
our board and works better than expected (at least by myself :-)).

Powered by blists - more mailing lists