Convert g762 Documentation to yaml schema.
Since it supports various device, change the name to g76x and add the
vendor prefix.
Signed-off-by: Christian Marangi <[email protected]>
---
.../devicetree/bindings/hwmon/g762.txt | 47 -----------
.../devicetree/bindings/hwmon/gmt,g76x.yaml | 83 +++++++++++++++++++
2 files changed, 83 insertions(+), 47 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/hwmon/g762.txt
create mode 100644 Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
diff --git a/Documentation/devicetree/bindings/hwmon/g762.txt b/Documentation/devicetree/bindings/hwmon/g762.txt
deleted file mode 100644
index 6d154c4923de..000000000000
--- a/Documentation/devicetree/bindings/hwmon/g762.txt
+++ /dev/null
@@ -1,47 +0,0 @@
-GMT G762/G763 PWM Fan controller
-
-Required node properties:
-
- - "compatible": must be either "gmt,g762" or "gmt,g763"
- - "reg": I2C bus address of the device
- - "clocks": a fixed clock providing input clock frequency
- on CLK pin of the chip.
-
-Optional properties:
-
- - "fan_startv": fan startup voltage. Accepted values are 0, 1, 2 and 3.
- The higher the more.
-
- - "pwm_polarity": pwm polarity. Accepted values are 0 (positive duty)
- and 1 (negative duty).
-
- - "fan_gear_mode": fan gear mode. Supported values are 0, 1 and 2.
-
-If an optional property is not set in .dts file, then current value is kept
-unmodified (e.g. u-boot installed value).
-
-Additional information on operational parameters for the device is available
-in Documentation/hwmon/g762.rst. A detailed datasheet for the device is available
-at http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf.
-
-Example g762 node:
-
- clocks {
- #address-cells = <1>;
- #size-cells = <0>;
-
- g762_clk: fixedclk {
- compatible = "fixed-clock";
- #clock-cells = <0>;
- clock-frequency = <8192>;
- }
- }
-
- g762: g762@3e {
- compatible = "gmt,g762";
- reg = <0x3e>;
- clocks = <&g762_clk>
- fan_gear_mode = <0>; /* chip default */
- fan_startv = <1>; /* chip default */
- pwm_polarity = <0>; /* chip default */
- };
diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
new file mode 100644
index 000000000000..bfefe49f54bf
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
@@ -0,0 +1,83 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GMT G762/G763 PWM Fan controller
+
+maintainers:
+ - Christian Marangi <[email protected]>
+
+description: |
+ GMT G762/G763 PWM Fan controller.
+
+ If an optional property is not set in DT, then current value is kept
+ unmodified (e.g. bootloader installed value).
+
+ Additional information on operational parameters for the device is available
+ in Documentation/hwmon/g762.rst. A detailed datasheet for the device is available
+ at http://natisbad.org/NAS/refs/GMT_EDS-762_763-080710-0.2.pdf.
+
+properties:
+ compatible:
+ enum:
+ - gmt,g762
+ - gmt,g763
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ description: a fixed clock providing input clock frequency on CLK
+ pin of the chip.
+ maxItems: 1
+
+ fan_startv:
+ description: Fan startup voltage step
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2, 3]
+
+ pwm_polarity:
+ description: PWM polarity (psotivie or negative duty)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1]
+
+ fan_gear_mode:
+ description: FAN gear mode. Configure High speed fan setting factor
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [0, 1, 2]
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ clocks {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ g762_clk: fixedclk {
+ compatible = "fixed-clock";
+ #clock-cells = <0>;
+ clock-frequency = <8192>;
+ };
+ };
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ g762@3e {
+ compatible = "gmt,g762";
+ reg = <0x3e>;
+ clocks = <&g762_clk>;
+ fan_gear_mode = <0>;
+ fan_startv = <1>;
+ pwm_polarity = <0>;
+ };
+ };
--
2.43.0
Add support for g761 PWM Fan controller. This is an exact copy of g763
with the difference that it does also support an internal clock
oscillators.
Add required logic to support this additional feature with the property
gmt,internal-clock and reject invalid schema that define both
internal-clock property and external clocks.
Signed-off-by: Christian Marangi <[email protected]>
---
.../devicetree/bindings/hwmon/gmt,g76x.yaml | 43 +++++++++++++++++--
1 file changed, 40 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
index bfefe49f54bf..d6e80392d045 100644
--- a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
+++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
@@ -4,13 +4,13 @@
$id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: GMT G762/G763 PWM Fan controller
+title: GMT G761/G762/G763 PWM Fan controller
maintainers:
- Christian Marangi <[email protected]>
description: |
- GMT G762/G763 PWM Fan controller.
+ GMT G761/G762/G763 PWM Fan controller.
If an optional property is not set in DT, then current value is kept
unmodified (e.g. bootloader installed value).
@@ -22,6 +22,7 @@ description: |
properties:
compatible:
enum:
+ - gmt,g761
- gmt,g762
- gmt,g763
@@ -48,10 +49,37 @@ properties:
$ref: /schemas/types.yaml#/definitions/uint32
enum: [0, 1, 2]
+ gmt,internal-clock:
+ description: Use the Internal clock instead of externally attached one
+ via the CLK pin.
+ type: boolean
+
required:
- compatible
- reg
- - clocks
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - gmt,g762
+ - gmt,g763
+ then:
+ properties:
+ gmt,internal-clock: false
+
+ required:
+ - clocks
+
+ - if:
+ required:
+ - gmt,internal-clock
+
+ then:
+ properties:
+ clocks: false
additionalProperties: false
@@ -80,4 +108,13 @@ examples:
fan_startv = <1>;
pwm_polarity = <0>;
};
+
+ g761@1e {
+ compatible = "gmt,g761";
+ reg = <0x1e>;
+ gmt,internal-clock;
+ fan_gear_mode = <0>;
+ fan_startv = <1>;
+ pwm_polarity = <0>;
+ };
};
--
2.43.0
Add support for g761 PWM Fan Controller.
The g761 is a copy of the g763 with the only difference of supporting
and internal clock. This can be configured with the gmt,internal-clock
property and in such case clock handling is skipped.
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/hwmon/g762.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/g762.c b/drivers/hwmon/g762.c
index af1228708e25..1629a3141c11 100644
--- a/drivers/hwmon/g762.c
+++ b/drivers/hwmon/g762.c
@@ -69,6 +69,7 @@ enum g762_regs {
#define G762_REG_FAN_CMD1_PWM_POLARITY 0x02 /* PWM polarity */
#define G762_REG_FAN_CMD1_PULSE_PER_REV 0x01 /* pulse per fan revolution */
+#define G761_REG_FAN_CMD2_FAN_CLOCK 0x20 /* choose internal clock*/
#define G762_REG_FAN_CMD2_GEAR_MODE_1 0x08 /* fan gear mode */
#define G762_REG_FAN_CMD2_GEAR_MODE_0 0x04
#define G762_REG_FAN_CMD2_FAN_STARTV_1 0x02 /* fan startup voltage */
@@ -115,6 +116,7 @@ enum g762_regs {
struct g762_data {
struct i2c_client *client;
+ bool internal_clock;
struct clk *clk;
/* update mutex */
@@ -566,6 +568,7 @@ static int do_set_fan_startv(struct device *dev, unsigned long val)
#ifdef CONFIG_OF
static const struct of_device_id g762_dt_match[] = {
+ { .compatible = "gmt,g761" },
{ .compatible = "gmt,g762" },
{ .compatible = "gmt,g763" },
{ },
@@ -597,6 +600,16 @@ static int g762_of_clock_enable(struct i2c_client *client)
if (!client->dev.of_node)
return 0;
+ data = i2c_get_clientdata(client);
+
+ /* Skip CLK detection and handling if we use internal clock */
+ data->internal_clock = of_property_read_bool(client->dev.of_node,
+ "gmt,internal-clock");
+ if (data->internal_clock) {
+ do_set_clk_freq(&client->dev, 32768);
+ return 0;
+ }
+
clk = of_clk_get(client->dev.of_node, 0);
if (IS_ERR(clk)) {
dev_err(&client->dev, "failed to get clock\n");
@@ -616,7 +629,6 @@ static int g762_of_clock_enable(struct i2c_client *client)
goto clk_unprep;
}
- data = i2c_get_clientdata(client);
data->clk = clk;
ret = devm_add_action(&client->dev, g762_of_clock_disable, data);
@@ -1029,12 +1041,17 @@ static inline int g762_fan_init(struct device *dev)
if (IS_ERR(data))
return PTR_ERR(data);
+ if (data->internal_clock)
+ data->fan_cmd2 |= G761_REG_FAN_CMD2_FAN_CLOCK;
+
data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_FAIL;
data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_OOC;
data->valid = false;
- return i2c_smbus_write_byte_data(data->client, G762_REG_FAN_CMD1,
- data->fan_cmd1);
+ return (i2c_smbus_write_byte_data(data->client, G762_REG_FAN_CMD1,
+ data->fan_cmd1) |
+ i2c_smbus_write_byte_data(data->client, G762_REG_FAN_CMD2,
+ data->fan_cmd2));
}
static int g762_probe(struct i2c_client *client)
--
2.43.0
On 5/25/24 03:29, Christian Marangi wrote:
> Add support for g761 PWM Fan Controller.
>
> The g761 is a copy of the g763 with the only difference of supporting
> and internal clock. This can be configured with the gmt,internal-clock
> property and in such case clock handling is skipped.
>
Do you happen to have a datasheet ? The datasheet is not available from GMT,
making it impossible to validate the changes.
> Signed-off-by: Christian Marangi <[email protected]>
> ---
> drivers/hwmon/g762.c | 23 ++++++++++++++++++++---
> 1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwmon/g762.c b/drivers/hwmon/g762.c
> index af1228708e25..1629a3141c11 100644
> --- a/drivers/hwmon/g762.c
> +++ b/drivers/hwmon/g762.c
> @@ -69,6 +69,7 @@ enum g762_regs {
> #define G762_REG_FAN_CMD1_PWM_POLARITY 0x02 /* PWM polarity */
> #define G762_REG_FAN_CMD1_PULSE_PER_REV 0x01 /* pulse per fan revolution */
>
> +#define G761_REG_FAN_CMD2_FAN_CLOCK 0x20 /* choose internal clock*/
> #define G762_REG_FAN_CMD2_GEAR_MODE_1 0x08 /* fan gear mode */
> #define G762_REG_FAN_CMD2_GEAR_MODE_0 0x04
> #define G762_REG_FAN_CMD2_FAN_STARTV_1 0x02 /* fan startup voltage */
> @@ -115,6 +116,7 @@ enum g762_regs {
>
> struct g762_data {
> struct i2c_client *client;
> + bool internal_clock;
> struct clk *clk;
>
> /* update mutex */
> @@ -566,6 +568,7 @@ static int do_set_fan_startv(struct device *dev, unsigned long val)
>
> #ifdef CONFIG_OF
> static const struct of_device_id g762_dt_match[] = {
> + { .compatible = "gmt,g761" },
> { .compatible = "gmt,g762" },
> { .compatible = "gmt,g763" },
> { },
> @@ -597,6 +600,16 @@ static int g762_of_clock_enable(struct i2c_client *client)
> if (!client->dev.of_node)
> return 0;
>
> + data = i2c_get_clientdata(client);
> +
> + /* Skip CLK detection and handling if we use internal clock */
> + data->internal_clock = of_property_read_bool(client->dev.of_node,
> + "gmt,internal-clock");
> + if (data->internal_clock) {
> + do_set_clk_freq(&client->dev, 32768); > + return 0;
> + }:
> +
> clk = of_clk_get(client->dev.of_node, 0);
> if (IS_ERR(clk)) {
> dev_err(&client->dev, "failed to get clock\n");
> @@ -616,7 +629,6 @@ static int g762_of_clock_enable(struct i2c_client *client)
> goto clk_unprep;
> }
>
> - data = i2c_get_clientdata(client);
> data->clk = clk;
>
> ret = devm_add_action(&client->dev, g762_of_clock_disable, data);
> @@ -1029,12 +1041,17 @@ static inline int g762_fan_init(struct device *dev)
> if (IS_ERR(data))
> return PTR_ERR(data);
>
> + if (data->internal_clock)
> + data->fan_cmd2 |= G761_REG_FAN_CMD2_FAN_CLOCK;
> +
This and the property must only be accepted for G761.
> data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_FAIL;
> data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_OOC;
> data->valid = false;
>
> - return i2c_smbus_write_byte_data(data->client, G762_REG_FAN_CMD1,
> - data->fan_cmd1);
> + return (i2c_smbus_write_byte_data(data->client, G762_REG_FAN_CMD1,
> + data->fan_cmd1) |
> + i2c_smbus_write_byte_data(data->client, G762_REG_FAN_CMD2,
> + data->fan_cmd2));
This is wrong. It would logically combine error codes, and execute
the second write even after the first failed.
Guenter
> }
>
> static int g762_probe(struct i2c_client *client)
On 5/25/24 03:29, Christian Marangi wrote:
> Add support for g761 PWM Fan controller. This is an exact copy of g763
> with the difference that it does also support an internal clock
> oscillators.
>
> Add required logic to support this additional feature with the property
> gmt,internal-clock and reject invalid schema that define both
> internal-clock property and external clocks.
>
> Signed-off-by: Christian Marangi <[email protected]>
> ---
> .../devicetree/bindings/hwmon/gmt,g76x.yaml | 43 +++++++++++++++++--
> 1 file changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
> index bfefe49f54bf..d6e80392d045 100644
> --- a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
> @@ -4,13 +4,13 @@
> $id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml#
> $schema: http://devicetree.org/meta-schemas/core.yaml#
>
> -title: GMT G762/G763 PWM Fan controller
> +title: GMT G761/G762/G763 PWM Fan controller
>
> maintainers:
> - Christian Marangi <[email protected]>
>
> description: |
> - GMT G762/G763 PWM Fan controller.
> + GMT G761/G762/G763 PWM Fan controller.
>
> If an optional property is not set in DT, then current value is kept
> unmodified (e.g. bootloader installed value).
> @@ -22,6 +22,7 @@ description: |
> properties:
> compatible:
> enum:
> + - gmt,g761
> - gmt,g762
> - gmt,g763
>
> @@ -48,10 +49,37 @@ properties:
> $ref: /schemas/types.yaml#/definitions/uint32
> enum: [0, 1, 2]
>
> + gmt,internal-clock:
> + description: Use the Internal clock instead of externally attached one
> + via the CLK pin.
> + type: boolean
> +
> required:
> - compatible
> - reg
> - - clocks
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - gmt,g762
> + - gmt,g763
> + then:
> + properties:
> + gmt,internal-clock: false
> +
> + required:
> + - clocks
Is the new property even necessary ? The absence of an external clock on G761
could be used to imply that the internal clock is used.
Guenter
> +
> + - if:
> + required:
> + - gmt,internal-clock
> +
> + then:
> + properties:
> + clocks: false
>
> additionalProperties: false
>
> @@ -80,4 +108,13 @@ examples:
> fan_startv = <1>;
> pwm_polarity = <0>;
> };
> +
> + g761@1e {
> + compatible = "gmt,g761";
> + reg = <0x1e>;
> + gmt,internal-clock;
> + fan_gear_mode = <0>;
> + fan_startv = <1>;
> + pwm_polarity = <0>;
> + };
> };
On Sat, May 25, 2024 at 07:29:55AM -0700, Guenter Roeck wrote:
> On 5/25/24 03:29, Christian Marangi wrote:
> > Add support for g761 PWM Fan Controller.
> >
> > The g761 is a copy of the g763 with the only difference of supporting
> > and internal clock. This can be configured with the gmt,internal-clock
> > property and in such case clock handling is skipped.
> >
>
> Do you happen to have a datasheet ? The datasheet is not available from GMT,
> making it impossible to validate the changes.
>
No datasheet, online there is only the first page that describe the
features.
This internal clock feature is the only difference to g763 and is
present in a downstream driver from a Asus ipq807x router.
I verified that all the regs match.
> > Signed-off-by: Christian Marangi <[email protected]>
> > ---
> > drivers/hwmon/g762.c | 23 ++++++++++++++++++++---
> > 1 file changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hwmon/g762.c b/drivers/hwmon/g762.c
> > index af1228708e25..1629a3141c11 100644
> > --- a/drivers/hwmon/g762.c
> > +++ b/drivers/hwmon/g762.c
> > @@ -69,6 +69,7 @@ enum g762_regs {
> > #define G762_REG_FAN_CMD1_PWM_POLARITY 0x02 /* PWM polarity */
> > #define G762_REG_FAN_CMD1_PULSE_PER_REV 0x01 /* pulse per fan revolution */
> > +#define G761_REG_FAN_CMD2_FAN_CLOCK 0x20 /* choose internal clock*/
> > #define G762_REG_FAN_CMD2_GEAR_MODE_1 0x08 /* fan gear mode */
> > #define G762_REG_FAN_CMD2_GEAR_MODE_0 0x04
> > #define G762_REG_FAN_CMD2_FAN_STARTV_1 0x02 /* fan startup voltage */
> > @@ -115,6 +116,7 @@ enum g762_regs {
> > struct g762_data {
> > struct i2c_client *client;
> > + bool internal_clock;
> > struct clk *clk;
> > /* update mutex */
> > @@ -566,6 +568,7 @@ static int do_set_fan_startv(struct device *dev, unsigned long val)
> > #ifdef CONFIG_OF
> > static const struct of_device_id g762_dt_match[] = {
> > + { .compatible = "gmt,g761" },
> > { .compatible = "gmt,g762" },
> > { .compatible = "gmt,g763" },
> > { },
> > @@ -597,6 +600,16 @@ static int g762_of_clock_enable(struct i2c_client *client)
> > if (!client->dev.of_node)
> > return 0;
> > + data = i2c_get_clientdata(client);
> > +
> > + /* Skip CLK detection and handling if we use internal clock */
> > + data->internal_clock = of_property_read_bool(client->dev.of_node,
> > + "gmt,internal-clock");
> > + if (data->internal_clock) {
> > + do_set_clk_freq(&client->dev, 32768); > + return 0;
> > + }:
> > +
> > clk = of_clk_get(client->dev.of_node, 0);
> > if (IS_ERR(clk)) {
> > dev_err(&client->dev, "failed to get clock\n");
> > @@ -616,7 +629,6 @@ static int g762_of_clock_enable(struct i2c_client *client)
> > goto clk_unprep;
> > }
> > - data = i2c_get_clientdata(client);
> > data->clk = clk;
> > ret = devm_add_action(&client->dev, g762_of_clock_disable, data);
> > @@ -1029,12 +1041,17 @@ static inline int g762_fan_init(struct device *dev)
> > if (IS_ERR(data))
> > return PTR_ERR(data);
> > + if (data->internal_clock)
> > + data->fan_cmd2 |= G761_REG_FAN_CMD2_FAN_CLOCK;
> > +
>
> This and the property must only be accepted for G761.
>
Yes you are right. I limit this only in Documentation but as a failsafe
I should also verify this here. Will do in V2.
> > data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_FAIL;
> > data->fan_cmd1 |= G762_REG_FAN_CMD1_DET_FAN_OOC;
> > data->valid = false;
> > - return i2c_smbus_write_byte_data(data->client, G762_REG_FAN_CMD1,
> > - data->fan_cmd1);
> > + return (i2c_smbus_write_byte_data(data->client, G762_REG_FAN_CMD1,
> > + data->fan_cmd1) |
> > + i2c_smbus_write_byte_data(data->client, G762_REG_FAN_CMD2,
> > + data->fan_cmd2));
>
> This is wrong. It would logically combine error codes, and execute
> the second write even after the first failed.
>
Ok will change the thing.
> > }
> > static int g762_probe(struct i2c_client *client)
>
--
Ansuel
On Sat, May 25, 2024 at 07:32:04AM -0700, Guenter Roeck wrote:
> On 5/25/24 03:29, Christian Marangi wrote:
> > Add support for g761 PWM Fan controller. This is an exact copy of g763
> > with the difference that it does also support an internal clock
> > oscillators.
> >
> > Add required logic to support this additional feature with the property
> > gmt,internal-clock and reject invalid schema that define both
> > internal-clock property and external clocks.
> >
> > Signed-off-by: Christian Marangi <[email protected]>
> > ---
> > .../devicetree/bindings/hwmon/gmt,g76x.yaml | 43 +++++++++++++++++--
> > 1 file changed, 40 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
> > index bfefe49f54bf..d6e80392d045 100644
> > --- a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
> > +++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
> > @@ -4,13 +4,13 @@
> > $id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml#
> > $schema: http://devicetree.org/meta-schemas/core.yaml#
> > -title: GMT G762/G763 PWM Fan controller
> > +title: GMT G761/G762/G763 PWM Fan controller
> > maintainers:
> > - Christian Marangi <[email protected]>
> > description: |
> > - GMT G762/G763 PWM Fan controller.
> > + GMT G761/G762/G763 PWM Fan controller.
> > If an optional property is not set in DT, then current value is kept
> > unmodified (e.g. bootloader installed value).
> > @@ -22,6 +22,7 @@ description: |
> > properties:
> > compatible:
> > enum:
> > + - gmt,g761
> > - gmt,g762
> > - gmt,g763
> > @@ -48,10 +49,37 @@ properties:
> > $ref: /schemas/types.yaml#/definitions/uint32
> > enum: [0, 1, 2]
> > + gmt,internal-clock:
> > + description: Use the Internal clock instead of externally attached one
> > + via the CLK pin.
> > + type: boolean
> > +
> > required:
> > - compatible
> > - reg
> > - - clocks
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + enum:
> > + - gmt,g762
> > + - gmt,g763
> > + then:
> > + properties:
> > + gmt,internal-clock: false
> > +
> > + required:
> > + - clocks
>
> Is the new property even necessary ? The absence of an external clock on G761
> could be used to imply that the internal clock is used.
>
Well with how the driver works, if a property is not defined, then the
value is not set and the one set by the bootloader or from device reset
is keept.
This conflicts with the logic of no clock = internal.
But yes if asked I can drop that, I can totally see your point since the
clocks is a required property anyway so it's always set.
>
> > +
> > + - if:
> > + required:
> > + - gmt,internal-clock
> > +
> > + then:
> > + properties:
> > + clocks: false
> > additionalProperties: false
> > @@ -80,4 +108,13 @@ examples:
> > fan_startv = <1>;
> > pwm_polarity = <0>;
> > };
> > +
> > + g761@1e {
> > + compatible = "gmt,g761";
> > + reg = <0x1e>;
> > + gmt,internal-clock;
> > + fan_gear_mode = <0>;
> > + fan_startv = <1>;
> > + pwm_polarity = <0>;
> > + };
> > };
>
--
Ansuel
On 5/25/24 03:50, Christian Marangi wrote:
> On Sat, May 25, 2024 at 07:32:04AM -0700, Guenter Roeck wrote:
>> On 5/25/24 03:29, Christian Marangi wrote:
>>> Add support for g761 PWM Fan controller. This is an exact copy of g763
>>> with the difference that it does also support an internal clock
>>> oscillators.
>>>
>>> Add required logic to support this additional feature with the property
>>> gmt,internal-clock and reject invalid schema that define both
>>> internal-clock property and external clocks.
>>>
>>> Signed-off-by: Christian Marangi <[email protected]>
>>> ---
>>> .../devicetree/bindings/hwmon/gmt,g76x.yaml | 43 +++++++++++++++++--
>>> 1 file changed, 40 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
>>> index bfefe49f54bf..d6e80392d045 100644
>>> --- a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
>>> +++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
>>> @@ -4,13 +4,13 @@
>>> $id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml#
>>> $schema: http://devicetree.org/meta-schemas/core.yaml#
>>> -title: GMT G762/G763 PWM Fan controller
>>> +title: GMT G761/G762/G763 PWM Fan controller
>>> maintainers:
>>> - Christian Marangi <[email protected]>
>>> description: |
>>> - GMT G762/G763 PWM Fan controller.
>>> + GMT G761/G762/G763 PWM Fan controller.
>>> If an optional property is not set in DT, then current value is kept
>>> unmodified (e.g. bootloader installed value).
>>> @@ -22,6 +22,7 @@ description: |
>>> properties:
>>> compatible:
>>> enum:
>>> + - gmt,g761
>>> - gmt,g762
>>> - gmt,g763
>>> @@ -48,10 +49,37 @@ properties:
>>> $ref: /schemas/types.yaml#/definitions/uint32
>>> enum: [0, 1, 2]
>>> + gmt,internal-clock:
>>> + description: Use the Internal clock instead of externally attached one
>>> + via the CLK pin.
>>> + type: boolean
>>> +
>>> required:
>>> - compatible
>>> - reg
>>> - - clocks
>>> +
>>> +allOf:
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + enum:
>>> + - gmt,g762
>>> + - gmt,g763
>>> + then:
>>> + properties:
>>> + gmt,internal-clock: false
>>> +
>>> + required:
>>> + - clocks
>>
>> Is the new property even necessary ? The absence of an external clock on G761
>> could be used to imply that the internal clock is used.
>>
>
> Well with how the driver works, if a property is not defined, then the
> value is not set and the one set by the bootloader or from device reset
> is keept.
>
> This conflicts with the logic of no clock = internal.
>
Not sure I understand the problem. It would be a simple change in the driver
to add "if the chip is G761 and the clock is not provided in DT, use the
internal clock".
Guenter
On Sat, May 25, 2024 at 07:53:57AM -0700, Guenter Roeck wrote:
> On 5/25/24 03:50, Christian Marangi wrote:
> > On Sat, May 25, 2024 at 07:32:04AM -0700, Guenter Roeck wrote:
> > > On 5/25/24 03:29, Christian Marangi wrote:
> > > > Add support for g761 PWM Fan controller. This is an exact copy of g763
> > > > with the difference that it does also support an internal clock
> > > > oscillators.
> > > >
> > > > Add required logic to support this additional feature with the property
> > > > gmt,internal-clock and reject invalid schema that define both
> > > > internal-clock property and external clocks.
> > > >
> > > > Signed-off-by: Christian Marangi <[email protected]>
> > > > ---
> > > > .../devicetree/bindings/hwmon/gmt,g76x.yaml | 43 +++++++++++++++++--
> > > > 1 file changed, 40 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
> > > > index bfefe49f54bf..d6e80392d045 100644
> > > > --- a/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
> > > > +++ b/Documentation/devicetree/bindings/hwmon/gmt,g76x.yaml
> > > > @@ -4,13 +4,13 @@
> > > > $id: http://devicetree.org/schemas/hwmon/gmt,g76x.yaml#
> > > > $schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > -title: GMT G762/G763 PWM Fan controller
> > > > +title: GMT G761/G762/G763 PWM Fan controller
> > > > maintainers:
> > > > - Christian Marangi <[email protected]>
> > > > description: |
> > > > - GMT G762/G763 PWM Fan controller.
> > > > + GMT G761/G762/G763 PWM Fan controller.
> > > > If an optional property is not set in DT, then current value is kept
> > > > unmodified (e.g. bootloader installed value).
> > > > @@ -22,6 +22,7 @@ description: |
> > > > properties:
> > > > compatible:
> > > > enum:
> > > > + - gmt,g761
> > > > - gmt,g762
> > > > - gmt,g763
> > > > @@ -48,10 +49,37 @@ properties:
> > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > enum: [0, 1, 2]
> > > > + gmt,internal-clock:
> > > > + description: Use the Internal clock instead of externally attached one
> > > > + via the CLK pin.
> > > > + type: boolean
> > > > +
> > > > required:
> > > > - compatible
> > > > - reg
> > > > - - clocks
> > > > +
> > > > +allOf:
> > > > + - if:
> > > > + properties:
> > > > + compatible:
> > > > + contains:
> > > > + enum:
> > > > + - gmt,g762
> > > > + - gmt,g763
> > > > + then:
> > > > + properties:
> > > > + gmt,internal-clock: false
> > > > +
> > > > + required:
> > > > + - clocks
> > >
> > > Is the new property even necessary ? The absence of an external clock on G761
> > > could be used to imply that the internal clock is used.
> > >
> >
> > Well with how the driver works, if a property is not defined, then the
> > value is not set and the one set by the bootloader or from device reset
> > is keept.
> >
> > This conflicts with the logic of no clock = internal.
> >
>
> Not sure I understand the problem. It would be a simple change in the driver
> to add "if the chip is G761 and the clock is not provided in DT, use the
> internal clock".
>
Yes sure code wise is pretty easy, my concern is more of losing this
info in DT. But anyway ok will drop in V2. Thanks a lot for the review!
--
Ansuel
On 5/25/24 04:12, Christian Marangi wrote:
>>> Well with how the driver works, if a property is not defined, then the
>>> value is not set and the one set by the bootloader or from device reset
>>> is keept.
>>>
>>> This conflicts with the logic of no clock = internal.
>>>
>>
>> Not sure I understand the problem. It would be a simple change in the driver
>> to add "if the chip is G761 and the clock is not provided in DT, use the
>> internal clock".
>>
>
> Yes sure code wise is pretty easy, my concern is more of losing this
> info in DT. But anyway ok will drop in V2. Thanks a lot for the review!
>
Not sure I understand. This is added support for a chip which is not currently
supported. There should not be any information to lose. What am I missing ?
Guenter