From: Philippe Schenker <[email protected]>
On our Colibri iMX6ULL board there is a circuit for switching the
power supply of the ethernet PHY with the 50MHz RMII clock.
This works quite fine but has one big problem. It is quite slow when
switching the supply, so Linux has to wait there. I think this switch
is at best represented as a fixed-regulator. In that way I can use
"startup-delay-us" to represent slow switching regulator.
But there's no current possibility to enable fixed-regulator with a
clock. In this RFC I send a patch after we would be able to add a clock
to a fixed-regulator in devicetree and then add the "startup-delay-us"
which would solve all my problems relatively elegant.
This works also the other way, if the PHY now needs power the
clock is not powered off because regulator depends on it.
Because this would need to change code in regulator core I like to
ask for your oppinion first, or if anyone has another idea how
I could solve that problem.
Thanks in advance for your feedback!
Philippe
Philippe Schenker (2):
Regulator: Core: Add clock-enable to fixed-regulator
ARM: dts: imx6ull: Add phy-supply to fec
arch/arm/boot/dts/imx6ull-colibri.dtsi | 12 ++++++++++++
drivers/regulator/core.c | 15 +++++++++++++++
drivers/regulator/fixed.c | 6 ++++++
include/linux/regulator/driver.h | 3 +++
include/linux/regulator/fixed.h | 1 +
5 files changed, 37 insertions(+)
--
2.22.0
From: Philippe Schenker <[email protected]>
This adds the possibility to enable a fixed-regulator with a clock.
Signed-off-by: <[email protected]>
Signed-off-by: Philippe Schenker <[email protected]>
---
drivers/regulator/core.c | 15 +++++++++++++++
drivers/regulator/fixed.c | 6 ++++++
include/linux/regulator/driver.h | 3 +++
include/linux/regulator/fixed.h | 1 +
4 files changed, 25 insertions(+)
diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e0c0cf462004..f16f6c147858 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -26,6 +26,7 @@
#include <linux/regulator/driver.h>
#include <linux/regulator/machine.h>
#include <linux/module.h>
+#include <linux/clk.h>
#define CREATE_TRACE_POINTS
#include <trace/events/regulator.h>
@@ -2385,6 +2386,11 @@ static int _regulator_do_enable(struct regulator_dev *rdev)
ret = rdev->desc->ops->enable(rdev);
if (ret < 0)
return ret;
+ } else if (rdev->ena_clk) {
+ ret = clk_prepare_enable(rdev->ena_clk);
+ if (ret)
+ return ret;
+ rdev->ena_clk_state++;
} else {
return -EINVAL;
}
@@ -2565,6 +2571,9 @@ static int _regulator_do_disable(struct regulator_dev *rdev)
ret = rdev->desc->ops->disable(rdev);
if (ret != 0)
return ret;
+ } else if (rdev->ena_clk) {
+ clk_disable_unprepare(rdev->ena_clk);
+ rdev->ena_clk_state--;
}
/* cares about last_off_jiffy only if off_on_delay is required by
@@ -2796,6 +2805,9 @@ static int _regulator_is_enabled(struct regulator_dev *rdev)
if (rdev->ena_pin)
return rdev->ena_gpio_state;
+ if (rdev->ena_clk)
+ return (rdev->ena_clk_state > 0) ? 1 : 0;
+
/* If we don't know then assume that the regulator is always on */
if (!rdev->desc->ops->is_enabled)
return 1;
@@ -5098,6 +5110,9 @@ regulator_register(const struct regulator_desc *regulator_desc,
dangling_of_gpiod = false;
}
+ if (cfg->ena_clk)
+ rdev->ena_clk = cfg->ena_clk;
+
/* register with sysfs */
rdev->dev.class = ®ulator_class;
rdev->dev.parent = dev;
diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 999547dde99d..0093c26cda3c 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -25,6 +25,7 @@
#include <linux/of.h>
#include <linux/regulator/of_regulator.h>
#include <linux/regulator/machine.h>
+#include <linux/clk.h>
struct fixed_voltage_data {
struct regulator_desc desc;
@@ -78,6 +79,10 @@ of_get_fixed_voltage_config(struct device *dev,
if (of_find_property(np, "vin-supply", NULL))
config->input_supply = "vin";
+ config->ena_clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(config->ena_clk))
+ config->ena_clk = NULL;
+
return config;
}
@@ -172,6 +177,7 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
cfg.init_data = config->init_data;
cfg.driver_data = drvdata;
cfg.of_node = pdev->dev.of_node;
+ cfg.ena_clk = config->ena_clk;
drvdata->dev = devm_regulator_register(&pdev->dev, &drvdata->desc,
&cfg);
diff --git a/include/linux/regulator/driver.h b/include/linux/regulator/driver.h
index 9a911bb5fb61..335ac1272bbd 100644
--- a/include/linux/regulator/driver.h
+++ b/include/linux/regulator/driver.h
@@ -414,6 +414,7 @@ struct regulator_config {
void *driver_data;
struct device_node *of_node;
struct regmap *regmap;
+ struct clk *ena_clk;
struct gpio_desc *ena_gpiod;
};
@@ -477,6 +478,8 @@ struct regulator_dev {
struct regulator_enable_gpio *ena_pin;
unsigned int ena_gpio_state:1;
+ struct clk *ena_clk;
+ unsigned int ena_clk_state;
unsigned int is_switch:1;
diff --git a/include/linux/regulator/fixed.h b/include/linux/regulator/fixed.h
index d44ce5f18a56..c291e1130381 100644
--- a/include/linux/regulator/fixed.h
+++ b/include/linux/regulator/fixed.h
@@ -38,6 +38,7 @@ struct fixed_voltage_config {
unsigned startup_delay;
unsigned enabled_at_boot:1;
struct regulator_init_data *init_data;
+ struct clk *ena_clk;
};
struct regulator_consumer_supply;
--
2.22.0
On Tue, Jul 30, 2019 at 07:30:04PM +0200, Philippe Schenker wrote:
> From: Philippe Schenker <[email protected]>
>
> This adds the possibility to enable a fixed-regulator with a clock.
Why? What does the hardware which makes this make sense look like?
Your cover letter didn't explain at all clearly, it just said that
there's a circuit that is connected to a clock which somehow switches
something but it's not clear. It's certainly not clear that this should
be in the core, the circuit doesn't sound like a good idea at all.
> Signed-off-by: <[email protected]>
> Signed-off-by: Philippe Schenker <[email protected]>
This needs a cleanup.
>
> /* cares about last_off_jiffy only if off_on_delay is required by
> @@ -2796,6 +2805,9 @@ static int _regulator_is_enabled(struct regulator_dev *rdev)
> if (rdev->ena_pin)
> return rdev->ena_gpio_state;
>
> + if (rdev->ena_clk)
> + return (rdev->ena_clk_state > 0) ? 1 : 0;
> +
Please write normal conditional statements, this isn't helping
legibility. Though in this case the ternery operator is totally
redundant anyway...
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:
> > From: Philippe Schenker <[email protected]>
> >
> > 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.
That switch switches power rail of a KSZ8041 ethernet PHY. So the power
rail of the KSZ8041 PHY is switched by its own clock.
> Your cover letter didn't explain at all clearly, it just said that
> there's a circuit that is connected to a clock which somehow switches
> something but it's not clear. It's certainly not clear that this
> should
> be in the core, the circuit doesn't sound like a good idea at all.
Sorry if I didn't explain it clear enough. I hope the hardware part is
clear now from the explanation above. Otherwise let me know I will
provide further explanations/schematics.
To your other questions, I will split those for better understanding:
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.
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.
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.
>
> > Signed-off-by: <[email protected]>
> > Signed-off-by: Philippe Schenker <[email protected]>
>
> This needs a cleanup.
Of course, sorry I didn't saw that beforehand. Some mess created with
cherry-picking...
>
> >
> > /* cares about last_off_jiffy only if off_on_delay is required
> > by
> > @@ -2796,6 +2805,9 @@ static int _regulator_is_enabled(struct
> > regulator_dev *rdev)
> > if (rdev->ena_pin)
> > return rdev->ena_gpio_state;
> >
> > + if (rdev->ena_clk)
> > + return (rdev->ena_clk_state > 0) ? 1 : 0;
> > +
>
> Please write normal conditional statements, this isn't helping
> legibility. Though in this case the ternery operator is totally
> redundant anyway...
Yeah now that I look at it you're right. I have in mind that I copied
that from somewhere to get the same coding style. I developed that in
an old kernel so could be that it's from there.
Anyway, this is just a concept for now and would need some more
thinking...
With this patch I want to put off a discussion, how we can support our
hardware in mainline Linux. This is my first proposal for that.
Philippe
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.
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 :-)).
On Mon, Aug 05, 2019 at 11:07:58AM +0000, Philippe Schenker wrote:
> On Wed, 2019-07-31 at 22:23 +0100, Mark Brown wrote:
Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns. Doing this makes your messages much
easier to read and reply to.
> > 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:
So the capacitor on the input of the p-FET is keeping the switch on?
When I say it's not switching with the clock I mean it's not constantly
bouncing on and off at whatever rate the clock is going at.
> > 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.
You could include that in your quirk?
> 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?
I think you are going to end up with a hack no matter what.
> > 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.
It would be better if it wasn't in the core, that keeps everything
partitioned off nicely.
> > 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 :-)).
The majority of regulators that need clocks are PWM devices which is a
whole other thing that we already support. This is a highly unusual
hardware design, we don't have the regmap stuff in the core and that's a
lot more common.
On Mon, 2019-08-05 at 17:37 +0100, Mark Brown wrote:
> On Mon, Aug 05, 2019 at 11:07:58AM +0000, Philippe Schenker wrote:
> > On Wed, 2019-07-31 at 22:23 +0100, Mark Brown wrote:
>
> Please fix your mail client to word wrap within paragraphs at something
> substantially less than 80 columns. Doing this makes your messages much
> easier to read and reply to.
That was a mistake, sorry. It actually is and was set to 80. Not sure
what happened there.
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:
>
> So the capacitor on the input of the p-FET is keeping the switch on?
> When I say it's not switching with the clock I mean it's not constantly
> bouncing on and off at whatever rate the clock is going at.
Ah, that's what you mean. Yes, the capacitor gets slowly charged with
the
resistor but nearly instantly discharged with the n-FET. So this
capacitor
is used as a Low-Pass filter to get the p-FET to be constantly switched.
It is not bouncing on and off with the clock but rather it is switched
constantly.
>
> > > 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.
>
> You could include that in your quirk?
Yes, this could be done as a quirk in the ethernet driver. But there the
clock gets enabled/disabled in different places. So I would have to sort
out where I need it and where not and basically go through the whole
driver. Plus it gets intensive to maintain that solution.
>
> > 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?
>
> I think you are going to end up with a hack no matter what.
That's exactly what I'm trying to prevent. To introduce a fixed
regulator that can have a clock is not a hack for me.
That the hardware solution is a hack is debatable yes, but why should I
not try to solve it properly in software?
>
> > > 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.
>
> It would be better if it wasn't in the core, that keeps everything
> partitioned off nicely.
I agree that our hardware design is somewhat special and is not widely
used. But I still think that this is valuable as a generic function and
could possibly be of benefit to someone else.
>
> > > 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 :-)).
>
> The majority of regulators that need clocks are PWM devices which is a
> whole other thing that we already support. This is a highly unusual
> hardware design, we don't have the regmap stuff in the core and that's a
> lot more common.
I agree with you, because of our unusual hardware design that this
shouldn't be in core. But I do not agree on solving that with quirks.
In the end I just want to represent our hardware in software. Would you
agree to create a new clock-regulator.c driver?
Or would it make more sense to extend fixed.c to support clocks-enable
without touching core?
On Tue, Aug 06, 2019 at 12:57:32PM +0000, Philippe Schenker wrote:
> On Mon, 2019-08-05 at 17:37 +0100, Mark Brown wrote:
> > So the capacitor on the input of the p-FET is keeping the switch on?
> > When I say it's not switching with the clock I mean it's not constantly
> > bouncing on and off at whatever rate the clock is going at.
> Ah, that's what you mean. Yes, the capacitor gets slowly charged with
> the
> resistor but nearly instantly discharged with the n-FET. So this
> capacitor
> is used as a Low-Pass filter to get the p-FET to be constantly switched.
> It is not bouncing on and off with the clock but rather it is switched
> constantly.
Good, I guess this might be part of why it's got this poor ramp time.
> > I think you are going to end up with a hack no matter what.
> That's exactly what I'm trying to prevent. To introduce a fixed
> regulator that can have a clock is not a hack for me.
> That the hardware solution is a hack is debatable yes, but why should I
> not try to solve it properly in software?
A lot of this discussion is around the definition of terms like "hack"
and "proper".
> In the end I just want to represent our hardware in software. Would you
> agree to create a new clock-regulator.c driver?
> Or would it make more sense to extend fixed.c to support clocks-enable
> without touching core?
At least a separate compatible makes sense, I'd have to see the code to
be clear if a completely separate driver makes sense but it'll need
separate ops at least. There'd definitely be a lot of overlap though so
it's worth looking at.
On Tue, 2019-08-06 at 19:26 +0100, Mark Brown wrote:
> On Tue, Aug 06, 2019 at 12:57:32PM +0000, Philippe Schenker wrote:
> > On Mon, 2019-08-05 at 17:37 +0100, Mark Brown wrote:
> > > So the capacitor on the input of the p-FET is keeping the switch
> > > on?
> > > When I say it's not switching with the clock I mean it's not
> > > constantly
> > > bouncing on and off at whatever rate the clock is going at.
> > Ah, that's what you mean. Yes, the capacitor gets slowly charged
> > with
> > the
> > resistor but nearly instantly discharged with the n-FET. So this
> > capacitor
> > is used as a Low-Pass filter to get the p-FET to be constantly
> > switched.
> > It is not bouncing on and off with the clock but rather it is
> > switched
> > constantly.
>
> Good, I guess this might be part of why it's got this poor ramp time.
Yes, I think so too.
>
> > > I think you are going to end up with a hack no matter what.
> > That's exactly what I'm trying to prevent. To introduce a fixed
> > regulator that can have a clock is not a hack for me.
> > That the hardware solution is a hack is debatable yes, but why
> > should I
> > not try to solve it properly in software?
>
> A lot of this discussion is around the definition of terms like "hack"
> and "proper".
>
> > In the end I just want to represent our hardware in software. Would
> > you
> > agree to create a new clock-regulator.c driver?
> > Or would it make more sense to extend fixed.c to support clocks-
> > enable
> > without touching core?
>
> At least a separate compatible makes sense, I'd have to see the code
> to
> be clear if a completely separate driver makes sense but it'll need
> separate ops at least. There'd definitely be a lot of overlap though
> so
> it's worth looking at.
Okay, thanks for discussion! I will try to make something that will fit
in mainline kernel and I will learn more about the regulator subsystem
in general so I can make a solution that fits.
But I'll need some time to do that. I will for sure link to that
discussion when I send the patch.
Philippe