* Re: [PATCH v2 1/4] dt-bindings: hwmon: Add Aspeed AST2600 PWM/Fan
[not found] ` <20210113070850.1184506-2-troy_lee@aspeedtech.com>
@ 2021-01-13 15:45 ` Rob Herring
2021-01-14 14:13 ` Rob Herring
1 sibling, 0 replies; 5+ messages in thread
From: Rob Herring @ 2021-01-13 15:45 UTC (permalink / raw)
To: Troy Lee
Cc: open list:HARDWARE MONITORING,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jean Delvare, Ryan Chen, Guenter Roeck,
moderated list:ARM/ASPEED MACHINE SUPPORT, Jonathan Corbet,
Andrew Jeffery, openbmc, open list:DOCUMENTATION, open list,
leetroy, Rob Herring, Joel Stanley, Philipp Zabel, chiawei_wang,
moderated list:ARM/ASPEED MACHINE SUPPORT
On Wed, 13 Jan 2021 07:08:45 +0000, Troy Lee wrote:
> We add binding for supporting a new AST2600 PWM/Fan hwmon driver.
>
> Changes since v1:
> - dt binding with DT schema format
>
> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> ---
> .../hwmon/aspeed,ast2600-pwm-tachometer.yaml | 137 ++++++++++++++++++
> 1 file changed, 137 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed,ast2600-pwm-tachometer.yaml
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
./Documentation/devicetree/bindings/hwmon/aspeed,ast2600-pwm-tachometer.yaml:108:2: [warning] wrong indentation: expected 2 but found 1 (indentation)
dtschema/dtc warnings/errors:
See https://patchwork.ozlabs.org/patch/1425628
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: hwmon: Add Aspeed AST2600 PWM/Fan
[not found] ` <20210113070850.1184506-2-troy_lee@aspeedtech.com>
2021-01-13 15:45 ` [PATCH v2 1/4] dt-bindings: hwmon: Add Aspeed AST2600 PWM/Fan Rob Herring
@ 2021-01-14 14:13 ` Rob Herring
2021-01-15 1:46 ` Troy Lee
1 sibling, 1 reply; 5+ messages in thread
From: Rob Herring @ 2021-01-14 14:13 UTC (permalink / raw)
To: Troy Lee
Cc: open list:HARDWARE MONITORING,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jean Delvare, Ryan Chen,
moderated list:ARM/ASPEED MACHINE SUPPORT, Jonathan Corbet,
Andrew Jeffery, openbmc, open list:DOCUMENTATION, open list,
leetroy, Joel Stanley, Philipp Zabel, chiawei_wang,
Guenter Roeck, moderated list:ARM/ASPEED MACHINE SUPPORT
On Wed, Jan 13, 2021 at 07:08:45AM +0000, Troy Lee wrote:
> We add binding for supporting a new AST2600 PWM/Fan hwmon driver.
>
> Changes since v1:
> - dt binding with DT schema format
>
> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> ---
> .../hwmon/aspeed,ast2600-pwm-tachometer.yaml | 137 ++++++++++++++++++
> 1 file changed, 137 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed,ast2600-pwm-tachometer.yaml
>
> diff --git a/Documentation/devicetree/bindings/hwmon/aspeed,ast2600-pwm-tachometer.yaml b/Documentation/devicetree/bindings/hwmon/aspeed,ast2600-pwm-tachometer.yaml
> new file mode 100644
> index 000000000000..b84076a4a338
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/aspeed,ast2600-pwm-tachometer.yaml
> @@ -0,0 +1,137 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/hwmon/aspeed,ast2600-pwm-tachometer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ASPEED AST2600 PWM and Fan Tacho controller device driver
> +
> +maintainers:
> + - Ryan Chen <ryan_chen@aspeedtech.com>
> +
> +description: |
> + The ASPEED PWM controller can support upto 16 PWM outputs. The ASPEED Fan Tacho
> + controller can support upto 16 Fan tachometer inputs.
> + There can be upto 16 fans supported. Each fan can have one PWM output and
> + one Fan tach inputs.
> +
> +properties:
> + compatible:
> + const: aspeed,ast2600-pwm-tachometer
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + "#cooling-cells":
> + const: 2
> +
> + reg:
> + description:
> + Address and length of the register set for the device.
No need for generic descriptions. That's every 'reg'.
What you need is how many entries and what each one is if more than 1.
If only 1, then just 'maxItems: 1'
> +
> + clocks:
> + description:
> + phandle to clock provider with the clock number in the second cell
Same here.
> +
> + resets:
> + description:
> + phandle to reset controller with the reset number in the second cell
And here.
> +
> +patternProperties:
> + "@[0-9]+$":
If every node is a fan and there are up to 16:
^fan@[0-9a-f]$
> + type: object
> + description:
> + Under fan subnode there can upto 16 child nodes, with each child node
> + representing a fan. There are 16 fans each fan can have one PWM port and one
> + Fan tach inputs.
> + For PWM port can be configured cooling-levels to create cooling device.
> + Cooling device could be bound to a thermal zone for the thermal control.
> +
> + properties:
> + reg:
> + minimum: 0
> + maximum: 15
> + description:
> + This property identify the PWM control channel of this fan.
> +
> + fan-tach-ch:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + minimum: 0
> + maximum: 15
> + description:
> + This property identify the fan tach input channel.
> +
> + pulses-per-revolution:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 2
> + minimum: 1
> + description:
> + Specify tacho pulse per revolution of the fan.
> +
> + cooling-levels:
> + description:
> + PWM duty cycle values in a range from 0 to 255
> + which correspond to thermal cooling states.
> +
> + aspeed,pwm-freq:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 25000
> + minimum: 24
> + maximum: 780000
> + description:
> + Specify the frequency of PWM.
Units? Use a unit suffix and then drop the $ref.
> +
> + aspeed,inverse-pin:
> + type: boolean
> + description:
> + Inverse PWM output signal.
> +
> + aspeed,falling-point:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + default: 10
> + minimum: 0
> + maximum: 255
0-255 is already the range of uint8, so drop.
> + description:
> + Initialize the pulse width.
> +
> + required:
> + - fan-tach-ch
> + - reg
> +
> + additionalProperties: true
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + pwm_tacho: pwm-tacho-controller@1e610000 {
> + compatible = "aspeed,ast2600-pwm-tachometer";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x1e610000 0x100>;
> +
> + fan@1 {
> + reg = <0x00>;
> + aspeed,pwm-freq = <25000>;
> + cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
> + fan-tach-ch = /bits/ 8 <0x00>;
> + pulses-per-revolution = <2>;
> + };
> +
> + fan@2 {
> + reg = <0x01>;
> + aspeed,pwm-freq = <25000>;
> + cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
> + fan-tach-ch = /bits/ 8 <0x01>;
> + pulses-per-revolution = <2>;
> + };
> + };
> +...
> --
> 2.25.1
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: hwmon: Add Aspeed AST2600 PWM/Fan
2021-01-14 14:13 ` Rob Herring
@ 2021-01-15 1:46 ` Troy Lee
0 siblings, 0 replies; 5+ messages in thread
From: Troy Lee @ 2021-01-15 1:46 UTC (permalink / raw)
To: Rob Herring
Cc: open list:HARDWARE MONITORING,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jean Delvare, Ryan Chen,
moderated list:ARM/ASPEED MACHINE SUPPORT, Jonathan Corbet,
Andrew Jeffery, openbmc, open list:DOCUMENTATION, open list,
leetroy, Joel Stanley, Philipp Zabel, ChiaWei Wang,
Guenter Roeck, moderated list:ARM/ASPEED MACHINE SUPPORT
Hi Rob,
Thanks for reviewing.
The 01/14/2021 22:13, Rob Herring wrote:
> On Wed, Jan 13, 2021 at 07:08:45AM +0000, Troy Lee wrote:
> > We add binding for supporting a new AST2600 PWM/Fan hwmon driver.
> >
> > Changes since v1:
> > - dt binding with DT schema format
> >
> > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
> > ---
> > .../hwmon/aspeed,ast2600-pwm-tachometer.yaml | 137 ++++++++++++++++++
> > 1 file changed, 137 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed,ast2600-pwm-tachometer.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/aspeed,ast2600-pwm-tachometer.yaml b/Documentation/devicetree/bindings/hwmon/aspeed,ast2600-pwm-tachometer.yaml
> > new file mode 100644
> > index 000000000000..b84076a4a338
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/aspeed,ast2600-pwm-tachometer.yaml
> > @@ -0,0 +1,137 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +
> > +$id: http://devicetree.org/schemas/hwmon/aspeed,ast2600-pwm-tachometer.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ASPEED AST2600 PWM and Fan Tacho controller device driver
> > +
> > +maintainers:
> > + - Ryan Chen <ryan_chen@aspeedtech.com>
> > +
> > +description: |
> > + The ASPEED PWM controller can support upto 16 PWM outputs. The ASPEED Fan Tacho
> > + controller can support upto 16 Fan tachometer inputs.
> > + There can be upto 16 fans supported. Each fan can have one PWM output and
> > + one Fan tach inputs.
> > +
> > +properties:
> > + compatible:
> > + const: aspeed,ast2600-pwm-tachometer
> > +
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + "#cooling-cells":
> > + const: 2
> > +
> > + reg:
> > + description:
> > + Address and length of the register set for the device.
>
> No need for generic descriptions. That's every 'reg'.
>
> What you need is how many entries and what each one is if more than 1.
> If only 1, then just 'maxItems: 1'
>
> > +
> > + clocks:
> > + description:
> > + phandle to clock provider with the clock number in the second cell
>
> Same here.
>
> > +
> > + resets:
> > + description:
> > + phandle to reset controller with the reset number in the second cell
>
> And here.
>
> > +
> > +patternProperties:
> > + "@[0-9]+$":
>
> If every node is a fan and there are up to 16:
>
> ^fan@[0-9a-f]$
>
I will update these in v3 patch set.
> > + type: object
> > + description:
> > + Under fan subnode there can upto 16 child nodes, with each child node
> > + representing a fan. There are 16 fans each fan can have one PWM port and one
> > + Fan tach inputs.
> > + For PWM port can be configured cooling-levels to create cooling device.
> > + Cooling device could be bound to a thermal zone for the thermal control.
> > +
> > + properties:
> > + reg:
> > + minimum: 0
> > + maximum: 15
> > + description:
> > + This property identify the PWM control channel of this fan.
> > +
> > + fan-tach-ch:
> > + $ref: /schemas/types.yaml#/definitions/uint8
> > + minimum: 0
> > + maximum: 15
> > + description:
> > + This property identify the fan tach input channel.
> > +
> > + pulses-per-revolution:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + default: 2
> > + minimum: 1
> > + description:
> > + Specify tacho pulse per revolution of the fan.
> > +
> > + cooling-levels:
> > + description:
> > + PWM duty cycle values in a range from 0 to 255
> > + which correspond to thermal cooling states.
> > +
> > + aspeed,pwm-freq:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + default: 25000
> > + minimum: 24
> > + maximum: 780000
> > + description:
> > + Specify the frequency of PWM.
>
> Units? Use a unit suffix and then drop the $ref.
>
I'll change it to pwm-freq-hz.
> > +
> > + aspeed,inverse-pin:
> > + type: boolean
> > + description:
> > + Inverse PWM output signal.
> > +
> > + aspeed,falling-point:
> > + $ref: /schemas/types.yaml#/definitions/uint8
> > + default: 10
> > + minimum: 0
> > + maximum: 255
>
> 0-255 is already the range of uint8, so drop.
>
I'll drop it in v3.
Thanks,
Troy Lee
> > + description:
> > + Initialize the pulse width.
> > +
> > + required:
> > + - fan-tach-ch
> > + - reg
> > +
> > + additionalProperties: true
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + pwm_tacho: pwm-tacho-controller@1e610000 {
> > + compatible = "aspeed,ast2600-pwm-tachometer";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <0x1e610000 0x100>;
> > +
> > + fan@1 {
> > + reg = <0x00>;
> > + aspeed,pwm-freq = <25000>;
> > + cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
> > + fan-tach-ch = /bits/ 8 <0x00>;
> > + pulses-per-revolution = <2>;
> > + };
> > +
> > + fan@2 {
> > + reg = <0x01>;
> > + aspeed,pwm-freq = <25000>;
> > + cooling-levels = /bits/ 8 <125 151 177 203 229 255>;
> > + fan-tach-ch = /bits/ 8 <0x01>;
> > + pulses-per-revolution = <2>;
> > + };
> > + };
> > +...
> > --
> > 2.25.1
> >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 4/4] hwmon: Support Aspeed AST2600 PWM/Fan tachometer
[not found] ` <20210113070850.1184506-5-troy_lee@aspeedtech.com>
@ 2021-01-23 16:14 ` Guenter Roeck
2021-01-25 10:01 ` Troy Lee
0 siblings, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2021-01-23 16:14 UTC (permalink / raw)
To: Troy Lee
Cc: open list:HARDWARE MONITORING,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jean Delvare, Ryan Chen,
moderated list:ARM/ASPEED MACHINE SUPPORT, Jonathan Corbet,
Andrew Jeffery, openbmc, open list:DOCUMENTATION, open list,
leetroy, Rob Herring, Joel Stanley, Philipp Zabel, chiawei_wang,
moderated list:ARM/ASPEED MACHINE SUPPORT
On Wed, Jan 13, 2021 at 07:08:48AM +0000, Troy Lee wrote:
> Add Aspeed AST2600 PWM/Fan tacho driver. AST2600 has 16 PWM channel and
> 16 FAN tacho channel.
>
> Changes since v1:
> - fixed review comments
> - fixed double-looped calculation of div_h and div_l
> - moving configuration to device tree
> - register hwmon driver with devm_hwmon_device_register_with_info()
>
> Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
checkpatch says:
total: 0 errors, 9 warnings, 26 checks, 779 lines checked
This is a bit much. Please run checkpatch --strict and fix the issues
it reports. Please also fix the issues reported by 0-day as well as
the issues reported by the bindings robot, and resubmit.
Thanks,
Guenter
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 4/4] hwmon: Support Aspeed AST2600 PWM/Fan tachometer
2021-01-23 16:14 ` [PATCH v2 4/4] hwmon: Support Aspeed AST2600 PWM/Fan tachometer Guenter Roeck
@ 2021-01-25 10:01 ` Troy Lee
0 siblings, 0 replies; 5+ messages in thread
From: Troy Lee @ 2021-01-25 10:01 UTC (permalink / raw)
To: Guenter Roeck
Cc: open list:HARDWARE MONITORING,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Jean Delvare, Ryan Chen,
moderated list:ARM/ASPEED MACHINE SUPPORT, Jonathan Corbet,
Andrew Jeffery, openbmc, open list:DOCUMENTATION, open list,
leetroy, Rob Herring, Joel Stanley, Philipp Zabel, ChiaWei Wang,
moderated list:ARM/ASPEED MACHINE SUPPORT
Hi Guenter,
The 01/24/2021 00:14, Guenter Roeck wrote:
> On Wed, Jan 13, 2021 at 07:08:48AM +0000, Troy Lee wrote:
> > Add Aspeed AST2600 PWM/Fan tacho driver. AST2600 has 16 PWM channel and
> > 16 FAN tacho channel.
> >
> > Changes since v1:
> > - fixed review comments
> > - fixed double-looped calculation of div_h and div_l
> > - moving configuration to device tree
> > - register hwmon driver with devm_hwmon_device_register_with_info()
> >
> > Signed-off-by: Troy Lee <troy_lee@aspeedtech.com>
>
> checkpatch says:
>
> total: 0 errors, 9 warnings, 26 checks, 779 lines checked
>
> This is a bit much. Please run checkpatch --strict and fix the issues
> it reports. Please also fix the issues reported by 0-day as well as
> the issues reported by the bindings robot, and resubmit.
>
> Thanks,
> Guenter
I'll fix the WARNINGs and CHECKs.
Thanks,
Troy Lee
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-25 10:05 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20210113070850.1184506-1-troy_lee@aspeedtech.com>
[not found] ` <20210113070850.1184506-2-troy_lee@aspeedtech.com>
2021-01-13 15:45 ` [PATCH v2 1/4] dt-bindings: hwmon: Add Aspeed AST2600 PWM/Fan Rob Herring
2021-01-14 14:13 ` Rob Herring
2021-01-15 1:46 ` Troy Lee
[not found] ` <20210113070850.1184506-5-troy_lee@aspeedtech.com>
2021-01-23 16:14 ` [PATCH v2 4/4] hwmon: Support Aspeed AST2600 PWM/Fan tachometer Guenter Roeck
2021-01-25 10:01 ` Troy Lee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).