Message ID | 20240525102914.22634-1-ansuelsmth@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [1/3] dt-bindings: hwmon: g762: Convert to yaml schema | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | warning | total: 0 errors, 2 warnings, 83 lines checked |
robh/patch-applied | success | |
robh/dtbs-check | warning | build log |
robh/dt-meta-schema | success |
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 <ansuelsmth@gmail.com> > > --- > > 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) >
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 <ansuelsmth@gmail.com> > --- > 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)
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 <ansuelsmth@gmail.com> + +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>; + }; + };
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 <ansuelsmth@gmail.com> --- .../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