* [PATCH 1/3] regulator: fixed: add possibility to enable by clock
2019-09-03 8:03 [PATCH 0/3] Add new binding regulator-fixed-clock to regulator-fixed Philippe Schenker
@ 2019-09-03 8:03 ` Philippe Schenker
2019-09-05 18:06 ` Mark Brown
2019-09-03 8:03 ` [PATCH 2/3] ARM: dts: imx6ull-colibri: add phy-supply and respective regulator Philippe Schenker
2019-09-03 8:03 ` [PATCH 3/3] dt-bindings: regulator: add regulator-fixed-clock binding Philippe Schenker
2 siblings, 1 reply; 10+ messages in thread
From: Philippe Schenker @ 2019-09-03 8:03 UTC (permalink / raw)
To: linux-kernel, Mark Brown, Shawn Guo, Sascha Hauer, Liam Girdwood
Cc: devicetree, Max Krummenacher, Rob Herring, Stefan Agner,
Marcel Ziswiler, Mark Rutland, Luka Pivk, Philippe Schenker
This commit adds the possibility to choose the compatible
"regulator-fixed-clock" in devicetree.
This is a special regulator-fixed that has to have a clock, from which
the regulator gets switched on and off.
Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
---
drivers/regulator/fixed.c | 86 +++++++++++++++++++++++++++++++++++++--
1 file changed, 83 insertions(+), 3 deletions(-)
diff --git a/drivers/regulator/fixed.c b/drivers/regulator/fixed.c
index 999547dde99d..eadeca9a1a6c 100644
--- a/drivers/regulator/fixed.c
+++ b/drivers/regulator/fixed.c
@@ -23,14 +23,66 @@
#include <linux/gpio/consumer.h>
#include <linux/slab.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/regulator/of_regulator.h>
#include <linux/regulator/machine.h>
+#include <linux/clk.h>
+
struct fixed_voltage_data {
struct regulator_desc desc;
struct regulator_dev *dev;
+
+ struct clk *enable_clock;
+ unsigned int clk_enable_counter;
+};
+
+struct fixed_dev_type {
+ bool has_enable_clock;
+};
+
+static const struct fixed_dev_type fixed_voltage_data = {
+ .has_enable_clock = false,
};
+static const struct fixed_dev_type fixed_clkenable_data = {
+ .has_enable_clock = true,
+};
+
+static int reg_clock_enable(struct regulator_dev *rdev)
+{
+ struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
+ int ret = 0;
+
+ ret = clk_prepare_enable(priv->enable_clock);
+ if (ret)
+ return ret;
+
+ priv->clk_enable_counter++;
+
+ return ret;
+}
+
+static int reg_clock_disable(struct regulator_dev *rdev)
+{
+ struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
+
+ clk_disable_unprepare(priv->enable_clock);
+ priv->clk_enable_counter--;
+
+ return 0;
+}
+
+static int reg_clock_is_enabled(struct regulator_dev *rdev)
+{
+ struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
+
+ if (priv->clk_enable_counter > 0)
+ return 1;
+
+ return 0;
+}
+
/**
* of_get_fixed_voltage_config - extract fixed_voltage_config structure info
@@ -84,10 +136,19 @@ of_get_fixed_voltage_config(struct device *dev,
static struct regulator_ops fixed_voltage_ops = {
};
+static struct regulator_ops fixed_voltage_clkenabled_ops = {
+ .enable = reg_clock_enable,
+ .disable = reg_clock_disable,
+ .is_enabled = reg_clock_is_enabled,
+};
+
static int reg_fixed_voltage_probe(struct platform_device *pdev)
{
+ struct device *dev = &pdev->dev;
struct fixed_voltage_config *config;
struct fixed_voltage_data *drvdata;
+ const struct fixed_dev_type *drvtype =
+ of_match_device(dev->driver->of_match_table, dev)->data;
struct regulator_config cfg = { };
enum gpiod_flags gflags;
int ret;
@@ -118,7 +179,18 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
}
drvdata->desc.type = REGULATOR_VOLTAGE;
drvdata->desc.owner = THIS_MODULE;
- drvdata->desc.ops = &fixed_voltage_ops;
+
+ if (drvtype->has_enable_clock) {
+ drvdata->desc.ops = &fixed_voltage_clkenabled_ops;
+
+ drvdata->enable_clock = devm_clk_get(dev, NULL);
+ if (IS_ERR(drvdata->enable_clock)) {
+ dev_err(dev, "Cant get enable-clock from devicetree\n");
+ return -ENOENT;
+ }
+ } else {
+ drvdata->desc.ops = &fixed_voltage_ops;
+ }
drvdata->desc.enable_time = config->startup_delay;
@@ -191,8 +263,16 @@ static int reg_fixed_voltage_probe(struct platform_device *pdev)
#if defined(CONFIG_OF)
static const struct of_device_id fixed_of_match[] = {
- { .compatible = "regulator-fixed", },
- {},
+ {
+ .compatible = "regulator-fixed",
+ .data = &fixed_voltage_data,
+ },
+ {
+ .compatible = "regulator-fixed-clock",
+ .data = &fixed_clkenable_data,
+ },
+ {
+ },
};
MODULE_DEVICE_TABLE(of, fixed_of_match);
#endif
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] regulator: fixed: add possibility to enable by clock
2019-09-03 8:03 ` [PATCH 1/3] regulator: fixed: add possibility to enable by clock Philippe Schenker
@ 2019-09-05 18:06 ` Mark Brown
2019-09-10 6:08 ` Philippe Schenker
0 siblings, 1 reply; 10+ messages in thread
From: Mark Brown @ 2019-09-05 18:06 UTC (permalink / raw)
To: Philippe Schenker
Cc: linux-kernel, Shawn Guo, Sascha Hauer, Liam Girdwood, devicetree,
Max Krummenacher, Rob Herring, Stefan Agner, Marcel Ziswiler,
Mark Rutland, Luka Pivk
[-- Attachment #1: Type: text/plain, Size: 768 bytes --]
On Tue, Sep 03, 2019 at 08:03:46AM +0000, Philippe Schenker wrote:
> This commit adds the possibility to choose the compatible
> "regulator-fixed-clock" in devicetree.
>
> This is a special regulator-fixed that has to have a clock, from which
> the regulator gets switched on and off.
This seems conceptually fine. Minor issues though:
> +static int reg_clock_is_enabled(struct regulator_dev *rdev)
> +{
> + struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
> +
> + if (priv->clk_enable_counter > 0)
> + return 1;
> +
> + return 0;
> +}
This could just be return priv->clk_enable_counter > 0 - ideally the
clock API would let us query if the clock is enabled but that might be a
bit confused anyway given that it's possibly shared.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] regulator: fixed: add possibility to enable by clock
2019-09-05 18:06 ` Mark Brown
@ 2019-09-10 6:08 ` Philippe Schenker
2019-09-10 6:14 ` Philippe Schenker
0 siblings, 1 reply; 10+ messages in thread
From: Philippe Schenker @ 2019-09-10 6:08 UTC (permalink / raw)
To: broonie
Cc: Max Krummenacher, Marcel Ziswiler, Luka Pivk, s.hauer,
devicetree, mark.rutland, linux-kernel, Stefan Agner, shawnguo,
lgirdwood, robh+dt
On Thu, 2019-09-05 at 19:06 +0100, Mark Brown wrote:
> On Tue, Sep 03, 2019 at 08:03:46AM +0000, Philippe Schenker wrote:
> > This commit adds the possibility to choose the compatible
> > "regulator-fixed-clock" in devicetree.
> >
> > This is a special regulator-fixed that has to have a clock, from
> > which
> > the regulator gets switched on and off.
>
> This seems conceptually fine. Minor issues though:
Thanks for your comments and I'm glad you like it! I will send a v2
shortly, also with Rob's fixes in. Can I expect it to be pulled for 5.4?
Best regards,
Philippe
>
> > +static int reg_clock_is_enabled(struct regulator_dev *rdev)
> > +{
> > + struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
> > +
> > + if (priv->clk_enable_counter > 0)
> > + return 1;
> > +
> > + return 0;
> > +}
>
> This could just be return priv->clk_enable_counter > 0 - ideally the
> clock API would let us query if the clock is enabled but that might be
> a
> bit confused anyway given that it's possibly shared.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] regulator: fixed: add possibility to enable by clock
2019-09-10 6:08 ` Philippe Schenker
@ 2019-09-10 6:14 ` Philippe Schenker
0 siblings, 0 replies; 10+ messages in thread
From: Philippe Schenker @ 2019-09-10 6:14 UTC (permalink / raw)
To: broonie
Cc: Max Krummenacher, Marcel Ziswiler, Luka Pivk, s.hauer,
devicetree, mark.rutland, linux-kernel, Stefan Agner, shawnguo,
lgirdwood, robh+dt
On Tue, 2019-09-10 at 06:08 +0000, Philippe Schenker wrote:
> On Thu, 2019-09-05 at 19:06 +0100, Mark Brown wrote:
> > On Tue, Sep 03, 2019 at 08:03:46AM +0000, Philippe Schenker wrote:
> > > This commit adds the possibility to choose the compatible
> > > "regulator-fixed-clock" in devicetree.
> > >
> > > This is a special regulator-fixed that has to have a clock, from
> > > which
> > > the regulator gets switched on and off.
> >
> > This seems conceptually fine. Minor issues though:
>
> Thanks for your comments and I'm glad you like it! I will send a v2
> shortly, also with Rob's fixes in. Can I expect it to be pulled for
> 5.4?
I meant 5.5 of course.
>
> Best regards,
> Philippe
>
> > > +static int reg_clock_is_enabled(struct regulator_dev *rdev)
> > > +{
> > > + struct fixed_voltage_data *priv = rdev_get_drvdata(rdev);
> > > +
> > > + if (priv->clk_enable_counter > 0)
> > > + return 1;
> > > +
> > > + return 0;
> > > +}
> >
> > This could just be return priv->clk_enable_counter > 0 - ideally the
> > clock API would let us query if the clock is enabled but that might
> > be
> > a
> > bit confused anyway given that it's possibly shared.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] ARM: dts: imx6ull-colibri: add phy-supply and respective regulator
2019-09-03 8:03 [PATCH 0/3] Add new binding regulator-fixed-clock to regulator-fixed Philippe Schenker
2019-09-03 8:03 ` [PATCH 1/3] regulator: fixed: add possibility to enable by clock Philippe Schenker
@ 2019-09-03 8:03 ` Philippe Schenker
2019-09-03 8:03 ` [PATCH 3/3] dt-bindings: regulator: add regulator-fixed-clock binding Philippe Schenker
2 siblings, 0 replies; 10+ messages in thread
From: Philippe Schenker @ 2019-09-03 8:03 UTC (permalink / raw)
To: linux-kernel, Mark Brown, Shawn Guo, Sascha Hauer, Liam Girdwood
Cc: devicetree, Max Krummenacher, Rob Herring, Stefan Agner,
Marcel Ziswiler, Mark Rutland, Luka Pivk, Philippe Schenker,
Fabio Estevam, linux-arm-kernel, Pengutronix Kernel Team,
NXP Linux Team
This adds regulator-fixed-clock, a fixed-regulator that turns on and
off with a clock and add it to the phy.
Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
---
arch/arm/boot/dts/imx6ull-colibri.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/imx6ull-colibri.dtsi b/arch/arm/boot/dts/imx6ull-colibri.dtsi
index d56728f03c35..76021b842a97 100644
--- a/arch/arm/boot/dts/imx6ull-colibri.dtsi
+++ b/arch/arm/boot/dts/imx6ull-colibri.dtsi
@@ -47,6 +47,17 @@
states = <1800000 0x1 3300000 0x0>;
vin-supply = <®_module_3v3>;
};
+
+ reg_eth_phy: regulator-eth-phy {
+ compatible = "regulator-fixed-clock";
+ regulator-boot-on;
+ regulator-name = "eth_phy";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ clocks = <&clks IMX6UL_CLK_ENET2_REF_125M>;
+ startup-delay-us = <150000>;
+ vin-supply = <®_module_3v3>;
+ };
};
&adc1 {
@@ -66,6 +77,7 @@
pinctrl-0 = <&pinctrl_enet2>;
phy-mode = "rmii";
phy-handle = <ðphy1>;
+ phy-supply = <®_eth_phy>;
status = "okay";
mdio {
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/3] dt-bindings: regulator: add regulator-fixed-clock binding
2019-09-03 8:03 [PATCH 0/3] Add new binding regulator-fixed-clock to regulator-fixed Philippe Schenker
2019-09-03 8:03 ` [PATCH 1/3] regulator: fixed: add possibility to enable by clock Philippe Schenker
2019-09-03 8:03 ` [PATCH 2/3] ARM: dts: imx6ull-colibri: add phy-supply and respective regulator Philippe Schenker
@ 2019-09-03 8:03 ` Philippe Schenker
2019-09-03 8:45 ` Rob Herring
2 siblings, 1 reply; 10+ messages in thread
From: Philippe Schenker @ 2019-09-03 8:03 UTC (permalink / raw)
To: linux-kernel, Mark Brown, Shawn Guo, Sascha Hauer, Liam Girdwood
Cc: devicetree, Max Krummenacher, Rob Herring, Stefan Agner,
Marcel Ziswiler, Mark Rutland, Luka Pivk, Philippe Schenker
This adds the documentation to the compatible regulator-fixed-clock
Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
---
.../bindings/regulator/fixed-regulator.yaml | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
index a650b457085d..5fd081e80b43 100644
--- a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
+++ b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
@@ -19,9 +19,19 @@ description:
allOf:
- $ref: "regulator.yaml#"
+select:
+ properties:
+ compatible:
+ contains:
+ const: regulator-fixed-clock
+ required:
+ - clocks
+
properties:
compatible:
- const: regulator-fixed
+ items:
+ - const: regulator-fixed
+ - const: regulator-fixed-clock
regulator-name: true
@@ -29,6 +39,12 @@ properties:
description: gpio to use for enable control
maxItems: 1
+ clocks:
+ description:
+ clock to use for enable control. This binding is only available if
+ the compatible is chosen to regulator-fixed-clock. The clock binding
+ is mandatory if compatible is chosen to regulator-fixed-clock.
+
startup-delay-us:
description: startup time in microseconds
$ref: /schemas/types.yaml#/definitions/uint32
--
2.23.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] dt-bindings: regulator: add regulator-fixed-clock binding
2019-09-03 8:03 ` [PATCH 3/3] dt-bindings: regulator: add regulator-fixed-clock binding Philippe Schenker
@ 2019-09-03 8:45 ` Rob Herring
2019-09-04 8:07 ` Philippe Schenker
0 siblings, 1 reply; 10+ messages in thread
From: Rob Herring @ 2019-09-03 8:45 UTC (permalink / raw)
To: Philippe Schenker
Cc: linux-kernel, Mark Brown, Shawn Guo, Sascha Hauer, Liam Girdwood,
devicetree, Max Krummenacher, Stefan Agner, Marcel Ziswiler,
Mark Rutland, Luka Pivk
On Tue, Sep 3, 2019 at 9:03 AM Philippe Schenker
<philippe.schenker@toradex.com> wrote:
>
> This adds the documentation to the compatible regulator-fixed-clock
Please explain what that is in this patch.
>
> Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
>
> ---
>
> .../bindings/regulator/fixed-regulator.yaml | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
> index a650b457085d..5fd081e80b43 100644
> --- a/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
> +++ b/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml
> @@ -19,9 +19,19 @@ description:
> allOf:
> - $ref: "regulator.yaml#"
>
> +select:
> + properties:
> + compatible:
> + contains:
> + const: regulator-fixed-clock
> + required:
> + - clocks
You don't need this.
If you add a new compatible, then this should probably be a new schema
doc. Is the 'gpio' property valid in this case as if a clock is the
enable, can you also have a gpio enable? That said, it seems like the
new compatible is only for validating the DT in the driver. You could
just use a clock if present and default to current behavior if not.
It's not the kernel's job to validate DTs.
>
> properties:
> compatible:
> - const: regulator-fixed
> + items:
> + - const: regulator-fixed
> + - const: regulator-fixed-clock
This says you must have 'compatible = "regulator-fixed",
"regulator-fixed-clock";'.
What you want is:
enum:
- regulator-fixed
- regulator-fixed-clock
> regulator-name: true
>
> @@ -29,6 +39,12 @@ properties:
> description: gpio to use for enable control
> maxItems: 1
>
> + clocks:
> + description:
> + clock to use for enable control. This binding is only available if
> + the compatible is chosen to regulator-fixed-clock. The clock binding
> + is mandatory if compatible is chosen to regulator-fixed-clock.
Need to define how many clocks (maxItems: 1).
> +
> startup-delay-us:
> description: startup time in microseconds
> $ref: /schemas/types.yaml#/definitions/uint32
> --
> 2.23.0
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] dt-bindings: regulator: add regulator-fixed-clock binding
2019-09-03 8:45 ` Rob Herring
@ 2019-09-04 8:07 ` Philippe Schenker
2019-09-04 9:06 ` Rob Herring
0 siblings, 1 reply; 10+ messages in thread
From: Philippe Schenker @ 2019-09-04 8:07 UTC (permalink / raw)
To: robh+dt
Cc: Max Krummenacher, Marcel Ziswiler, Luka Pivk, s.hauer, broonie,
devicetree, linux-kernel, shawnguo, Stefan Agner, lgirdwood,
mark.rutland
On Tue, 2019-09-03 at 09:45 +0100, Rob Herring wrote:
> On Tue, Sep 3, 2019 at 9:03 AM Philippe Schenker
> <philippe.schenker@toradex.com> wrote:
> > This adds the documentation to the compatible regulator-fixed-clock
>
> Please explain what that is in this patch.
Hi Rob and thanks for your comments. I will change this commit message
for a possible v2 into this:
This adds the documentation to the compatible regulator-fixed-clock.
This binding is a special binding of regulator-fixed and adds the
ability to add a clock to regulator-fixed, so the regulator can be
enabled and disabled with that clock.
If the special compatible regulator-fixed-clock is used it is mandatory
to supply a clock.
> >
>
> > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> >
> > ---
> >
> > .../bindings/regulator/fixed-regulator.yaml | 18
> > +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/regulator/fixed-
> > regulator.yaml b/Documentation/devicetree/bindings/regulator/fixed-
> > regulator.yaml
> > index a650b457085d..5fd081e80b43 100644
> > --- a/Documentation/devicetree/bindings/regulator/fixed-
> > regulator.yaml
> > +++ b/Documentation/devicetree/bindings/regulator/fixed-
> > regulator.yaml
> > @@ -19,9 +19,19 @@ description:
> > allOf:
> > - $ref: "regulator.yaml#"
> >
> > +select:
> > + properties:
> > + compatible:
> > + contains:
> > + const: regulator-fixed-clock
> > + required:
> > + - clocks
>
> You don't need this.
>
> If you add a new compatible, then this should probably be a new schema
> doc. Is the 'gpio' property valid in this case as if a clock is the
> enable, can you also have a gpio enable? That said, it seems like the
> new compatible is only for validating the DT in the driver. You could
> just use a clock if present and default to current behavior if not.
> It's not the kernel's job to validate DTs.
The gpio property is valid with this compatible. I added regulator-
fixed-clock to regulator-fixed hence I also don't want to create a new
schema file.
With the above select statement I wanted to state clocks as required
when the compatible regulator-fixed-clock is given.
>
> > properties:
> > compatible:
> > - const: regulator-fixed
> > + items:
> > + - const: regulator-fixed
> > + - const: regulator-fixed-clock
>
> This says you must have 'compatible = "regulator-fixed",
> "regulator-fixed-clock";'.
>
> What you want is:
>
> enum:
> - regulator-fixed
> - regulator-fixed-clock
Thanks, this was exactly what I wanted to do.
>
> > regulator-name: true
> >
> > @@ -29,6 +39,12 @@ properties:
> > description: gpio to use for enable control
> > maxItems: 1
> >
> > + clocks:
> > + description:
> > + clock to use for enable control. This binding is only
> > available if
> > + the compatible is chosen to regulator-fixed-clock. The clock
> > binding
> > + is mandatory if compatible is chosen to regulator-fixed-
> > clock.
>
> Need to define how many clocks (maxItems: 1).
Will do for a possible v2. I want to wait what Mark Brown says about the
addition in general, maybe I have to change all over how this speciality
is added into regulator subsystem and therefore also the binding will
change.
Philippe
>
> > +
> > startup-delay-us:
> > description: startup time in microseconds
> > $ref: /schemas/types.yaml#/definitions/uint32
> > --
> > 2.23.0
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] dt-bindings: regulator: add regulator-fixed-clock binding
2019-09-04 8:07 ` Philippe Schenker
@ 2019-09-04 9:06 ` Rob Herring
0 siblings, 0 replies; 10+ messages in thread
From: Rob Herring @ 2019-09-04 9:06 UTC (permalink / raw)
To: Philippe Schenker
Cc: Max Krummenacher, Marcel Ziswiler, Luka Pivk, s.hauer, broonie,
devicetree, linux-kernel, shawnguo, Stefan Agner, lgirdwood,
mark.rutland
On Wed, Sep 4, 2019 at 9:07 AM Philippe Schenker
<philippe.schenker@toradex.com> wrote:
>
> On Tue, 2019-09-03 at 09:45 +0100, Rob Herring wrote:
> > On Tue, Sep 3, 2019 at 9:03 AM Philippe Schenker
> > <philippe.schenker@toradex.com> wrote:
> > > This adds the documentation to the compatible regulator-fixed-clock
> >
> > Please explain what that is in this patch.
>
> Hi Rob and thanks for your comments. I will change this commit message
> for a possible v2 into this:
>
> This adds the documentation to the compatible regulator-fixed-clock.
> This binding is a special binding of regulator-fixed and adds the
> ability to add a clock to regulator-fixed, so the regulator can be
> enabled and disabled with that clock.
> If the special compatible regulator-fixed-clock is used it is mandatory
> to supply a clock.
>
> > >
> >
> > > Signed-off-by: Philippe Schenker <philippe.schenker@toradex.com>
> > >
> > > ---
> > >
> > > .../bindings/regulator/fixed-regulator.yaml | 18
> > > +++++++++++++++++-
> > > 1 file changed, 17 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/regulator/fixed-
> > > regulator.yaml b/Documentation/devicetree/bindings/regulator/fixed-
> > > regulator.yaml
> > > index a650b457085d..5fd081e80b43 100644
> > > --- a/Documentation/devicetree/bindings/regulator/fixed-
> > > regulator.yaml
> > > +++ b/Documentation/devicetree/bindings/regulator/fixed-
> > > regulator.yaml
> > > @@ -19,9 +19,19 @@ description:
> > > allOf:
> > > - $ref: "regulator.yaml#"
> > >
> > > +select:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: regulator-fixed-clock
> > > + required:
> > > + - clocks
> >
> > You don't need this.
> >
> > If you add a new compatible, then this should probably be a new schema
> > doc. Is the 'gpio' property valid in this case as if a clock is the
> > enable, can you also have a gpio enable? That said, it seems like the
> > new compatible is only for validating the DT in the driver. You could
> > just use a clock if present and default to current behavior if not.
> > It's not the kernel's job to validate DTs.
>
> The gpio property is valid with this compatible. I added regulator-
> fixed-clock to regulator-fixed hence I also don't want to create a new
> schema file.
Okay, if all the other properties are valid then adding to this file is fine.
> With the above select statement I wanted to state clocks as required
> when the compatible regulator-fixed-clock is given.
select is not the right way to do that. select is for whether to apply
the schema to a node or not. What you have will silently not apply the
schema if 'clocks' is missing or compatible is 'regulator-fixed'.
Essentially what you need to do here is:
if:
properties:
compatible:
contains:
const: regulator-fixed-clock
then:
required:
- clocks
Rob
^ permalink raw reply [flat|nested] 10+ messages in thread