diff mbox series

[v2,1/3] dt-bindings: Add bindings for aspeed pwm-tach.

Message ID 20221101095156.30591-2-billy_tsai@aspeedtech.com
State Superseded, archived
Headers show
Series upport pwm/tach driver for aspeed ast26xx | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Billy Tsai Nov. 1, 2022, 9:51 a.m. UTC
This patch adds device binding for aspeed pwm-tach device which is a
multi-function device include pwm and tach function and pwm/tach device
bindings which should be the child-node of pwm-tach device.

Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
---
 .../bindings/hwmon/aspeed,ast2600-tach.yaml   | 48 ++++++++++++
 .../bindings/mfd/aspeed,ast2600-pwm-tach.yaml | 76 +++++++++++++++++++
 .../bindings/pwm/aspeed,ast2600-pwm.yaml      | 64 ++++++++++++++++
 3 files changed, 188 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed,ast2600-tach.yaml
 create mode 100644 Documentation/devicetree/bindings/mfd/aspeed,ast2600-pwm-tach.yaml
 create mode 100644 Documentation/devicetree/bindings/pwm/aspeed,ast2600-pwm.yaml

Comments

Rob Herring Nov. 1, 2022, 6:40 p.m. UTC | #1
On Tue, Nov 01, 2022 at 05:51:54PM +0800, Billy Tsai wrote:
> This patch adds device binding for aspeed pwm-tach device which is a
> multi-function device include pwm and tach function and pwm/tach device
> bindings which should be the child-node of pwm-tach device.
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  .../bindings/hwmon/aspeed,ast2600-tach.yaml   | 48 ++++++++++++
>  .../bindings/mfd/aspeed,ast2600-pwm-tach.yaml | 76 +++++++++++++++++++
>  .../bindings/pwm/aspeed,ast2600-pwm.yaml      | 64 ++++++++++++++++
>  3 files changed, 188 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed,ast2600-tach.yaml
>  create mode 100644 Documentation/devicetree/bindings/mfd/aspeed,ast2600-pwm-tach.yaml
>  create mode 100644 Documentation/devicetree/bindings/pwm/aspeed,ast2600-pwm.yaml

I'm pretty sure I've said this before, but I'm not taking more fan 
controller bindings without comming up with a common binding. Please see 
this series[1] and help ensure it meets your needs.

Rob

[1] https://lore.kernel.org/all/20221013094838.1529153-2-Naresh.Solanki@9elements.com/
Billy Tsai Nov. 2, 2022, 3:21 a.m. UTC | #2
Hi Rob,

On 2022/11/2, 2:40 AM, "Rob Herring" <robh@kernel.org> wrote:

  >  On Tue, Nov 01, 2022 at 05:51:54PM +0800, Billy Tsai wrote:
  >  > This patch adds device binding for aspeed pwm-tach device which is a
  >  > multi-function device include pwm and tach function and pwm/tach device
  >  > bindings which should be the child-node of pwm-tach device.
  >  > 
  >  > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
  >  > ---
  >  >  .../bindings/hwmon/aspeed,ast2600-tach.yaml   | 48 ++++++++++++
  >  >  .../bindings/mfd/aspeed,ast2600-pwm-tach.yaml | 76 +++++++++++++++++++
  >  >  .../bindings/pwm/aspeed,ast2600-pwm.yaml      | 64 ++++++++++++++++
  >  >  3 files changed, 188 insertions(+)
  >  >  create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed,ast2600-tach.yaml
  >  >  create mode 100644 Documentation/devicetree/bindings/mfd/aspeed,ast2600-pwm-tach.yaml
  >  >  create mode 100644 Documentation/devicetree/bindings/pwm/aspeed,ast2600-pwm.yaml

  >  I'm pretty sure I've said this before, but I'm not taking more fan 
  >  controller bindings without comming up with a common binding. Please see 
  >  this series[1] and help ensure it meets your needs.

  >  Rob

  >  [1] 20221013094838.1529153-2-Naresh.Solanki@9elements.com <https://lore.kernel.org/all/<a href=>/">https://lore.kernel.org/all/20221013094838.1529153-2-Naresh.Solanki@9elements.com/

The link you provide doesn't meet my needs. This is fan binding.
As I told before 
"This patch doesn't use to binding the fan control h/w. It is used to binding the two independent h/w blocks.
One is used to provide pwm output and another is used to monitor the speed of the input." 
My patch is used to point out that the pwm and the tach is the different function and don't need to
bind together. You can not only combine them as the fan usage but also treat them as the individual module for
use. For example: the pwm can use to be the beeper (pwm-beeper.c), the tach can be used to monitor any device's speed.

Thanks

Best Regards,
Billy Tsai
Rob Herring Nov. 2, 2022, 6:19 p.m. UTC | #3
On Tue, Nov 1, 2022 at 10:21 PM Billy Tsai <billy_tsai@aspeedtech.com> wrote:
>
> Hi Rob,
>
> On 2022/11/2, 2:40 AM, "Rob Herring" <robh@kernel.org> wrote:
>
>   >  On Tue, Nov 01, 2022 at 05:51:54PM +0800, Billy Tsai wrote:
>   >  > This patch adds device binding for aspeed pwm-tach device which is a
>   >  > multi-function device include pwm and tach function and pwm/tach device
>   >  > bindings which should be the child-node of pwm-tach device.
>   >  >
>   >  > Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
>   >  > ---
>   >  >  .../bindings/hwmon/aspeed,ast2600-tach.yaml   | 48 ++++++++++++
>   >  >  .../bindings/mfd/aspeed,ast2600-pwm-tach.yaml | 76 +++++++++++++++++++
>   >  >  .../bindings/pwm/aspeed,ast2600-pwm.yaml      | 64 ++++++++++++++++
>   >  >  3 files changed, 188 insertions(+)
>   >  >  create mode 100644 Documentation/devicetree/bindings/hwmon/aspeed,ast2600-tach.yaml
>   >  >  create mode 100644 Documentation/devicetree/bindings/mfd/aspeed,ast2600-pwm-tach.yaml
>   >  >  create mode 100644 Documentation/devicetree/bindings/pwm/aspeed,ast2600-pwm.yaml
>
>   >  I'm pretty sure I've said this before, but I'm not taking more fan
>   >  controller bindings without comming up with a common binding. Please see
>   >  this series[1] and help ensure it meets your needs.
>
>   >  Rob
>
>   >  [1] 20221013094838.1529153-2-Naresh.Solanki@9elements.com <https://lore.kernel.org/all/<a href=>/">https://lore.kernel.org/all/20221013094838.1529153-2-Naresh.Solanki@9elements.com/
>
> The link you provide doesn't meet my needs. This is fan binding.

A PWM and Tach controller is for fans, no?

As I said, contribute to it so that it does meet your needs.

> As I told before
> "This patch doesn't use to binding the fan control h/w. It is used to binding the two independent h/w blocks.
> One is used to provide pwm output and another is used to monitor the speed of the input."
> My patch is used to point out that the pwm and the tach is the different function and don't need to
> bind together. You can not only combine them as the fan usage but also treat them as the individual module for
> use. For example: the pwm can use to be the beeper (pwm-beeper.c), the tach can be used to monitor any device's speed.

That all sounds like requirements that you have which you should
ensure the fan binding can support.

I've already said to use the PWM binding in the fan binding exactly
for the purpose of hooking up the PWMs to other things. Whether the
tach controller is useful for something other than fans, I don't know.
Seems less likely. The max6639 also has a tach controller. So if other
uses are possible for you, then it could be possible for any other h/w
like the max6639.

Rob
Billy Tsai Nov. 3, 2022, 7:39 a.m. UTC | #4
On 2022/11/3, 2:19 AM, "Rob Herring" <robh@kernel.org> wrote:

    On Tue, Nov 1, 2022 at 10:21 PM Billy Tsai <billy_tsai@aspeedtech.com> wrote:
    
    > That all sounds like requirements that you have which you should
    > ensure the fan binding can support.

    > I've already said to use the PWM binding in the fan binding exactly
    > for the purpose of hooking up the PWMs to other things. Whether the
    > tach controller is useful for something other than fans, I don't know.
    > Seems less likely. The max6639 also has a tach controller. So if other
    > uses are possible for you, then it could be possible for any other h/w
    > like the max6639.

The linux kernel already have the similar binding:
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/hwmon/pwm-fan.txt 
Therefore, I want to reuse it and the pwm-fan.c instead of creating another similar fan binding and driver.
I am referring to the following files:
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/mfd/kontron%2Csl28cpld.yaml 
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pwm/kontron%2Csl28cpld-pwm.yaml 
https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/hwmon/kontron%2Csl28cpld-hwmon.yaml 
It will also reuse the pwm-fan instead creating it own pwm-fan binding for their hwmon(tach).
https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts#L20 

Thanks

Best Regards,
Billy Tsai
Billy Tsai Nov. 14, 2022, 8:26 a.m. UTC | #5
Hi Rob,

On 2022/11/3, 3:39 PM, "Billy Tsai" <billy_tsai@aspeedtech.com> wrote:

    On 2022/11/3, 2:19 AM, "Rob Herring" <robh@kernel.org> wrote:

        On Tue, Nov 1, 2022 at 10:21 PM Billy Tsai <billy_tsai@aspeedtech.com> wrote:

    >    > That all sounds like requirements that you have which you should
    >    > ensure the fan binding can support.

    >    > I've already said to use the PWM binding in the fan binding exactly
    >    > for the purpose of hooking up the PWMs to other things. Whether the
    >    > tach controller is useful for something other than fans, I don't know.
    >    > Seems less likely. The max6639 also has a tach controller. So if other
    >    > uses are possible for you, then it could be possible for any other h/w
    >    > like the max6639.

    > The linux kernel already have the similar binding:
    > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/hwmon/pwm-fan.txt 
    > Therefore, I want to reuse it and the pwm-fan.c instead of creating another similar fan binding and driver.
    > I am referring to the following files:
    > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/mfd/kontron%2Csl28cpld.yaml 
    > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/pwm/kontron%2Csl28cpld-pwm.yaml 
    > https://github.com/torvalds/linux/blob/master/Documentation/devicetree/bindings/hwmon/kontron%2Csl28cpld-hwmon.yaml 
    > It will also reuse the pwm-fan instead creating it own pwm-fan binding for their hwmon(tach).
    > https://github.com/torvalds/linux/blob/master/arch/arm64/boot/dts/freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts#L20 

Do you have any sugguest about the binding like the link above?
We can just creat the pure PWM provider and use the pwm-fan as the common fan binding instead of creating another fan-common.yaml.

Thanks

Best Regards,
Billy Tsai
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/hwmon/aspeed,ast2600-tach.yaml b/Documentation/devicetree/bindings/hwmon/aspeed,ast2600-tach.yaml
new file mode 100644
index 000000000000..838200fae30e
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/aspeed,ast2600-tach.yaml
@@ -0,0 +1,48 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2021 Aspeed, Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/hwmon/aspeed,ast2600-tach.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aspeed Ast2600 Tach controller
+
+maintainers:
+  - Billy Tsai <billy_tsai@aspeedtech.com>
+
+description: |
+  The Aspeed Tach controller can support upto 16 fan input.
+  This module is part of the ast2600-pwm-tach multi-function device. For more
+  details see ../mfd/aspeed,ast2600-pwm-tach.yaml.
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2600-tach
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  pinctrl-0: true
+
+  pinctrl-names:
+    const: default
+
+required:
+  - compatible
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties:
+  type: object
+  properties:
+    reg:
+      description:
+        The tach channel used for this node.
+      maxItems: 1
+
+  required:
+    - reg
diff --git a/Documentation/devicetree/bindings/mfd/aspeed,ast2600-pwm-tach.yaml b/Documentation/devicetree/bindings/mfd/aspeed,ast2600-pwm-tach.yaml
new file mode 100644
index 000000000000..1eaf6fab2752
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/aspeed,ast2600-pwm-tach.yaml
@@ -0,0 +1,76 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2021 Aspeed, Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/aspeed,ast2600-pwm-tach.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: PWM Tach controller Device Tree Bindings
+
+description: |
+  The PWM Tach controller is represented as a multi-function device which
+  includes:
+    PWM
+    Tach
+
+maintainers:
+  - Billy Tsai <billy_tsai@aspeedtech.com>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - aspeed,ast2600-pwm-tach
+      - const: syscon
+      - const: simple-mfd
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - resets
+
+patternProperties:
+  "^pwm(@[0-9a-f]+)?$":
+    $ref: ../pwm/aspeed,ast2600-pwm.yaml
+
+  "^tach(@[0-9a-f]+)?$":
+    $ref: ../hwmon/aspeed,ast2600-tach.yaml
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/ast2600-clock.h>
+    pwm_tach: pwm_tach@1e610000 {
+      compatible = "aspeed,ast2600-pwm-tach", "syscon", "simple-mfd";
+      reg = <0x1e610000 0x100>;
+      clocks = <&syscon ASPEED_CLK_AHB>;
+      resets = <&syscon ASPEED_RESET_PWM>;
+
+      pwm: pwm {
+        compatible = "aspeed,ast2600-pwm";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        #pwm-cells = <3>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_pwm0_default>;
+      };
+
+      tach: tach {
+        compatible = "aspeed,ast2600-tach";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&pinctrl_tach0_default>;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/pwm/aspeed,ast2600-pwm.yaml b/Documentation/devicetree/bindings/pwm/aspeed,ast2600-pwm.yaml
new file mode 100644
index 000000000000..f501f8a769df
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/aspeed,ast2600-pwm.yaml
@@ -0,0 +1,64 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2021 Aspeed, Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/aspeed,ast2600-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Aspeed Ast2600 PWM controller
+
+maintainers:
+  - Billy Tsai <billy_tsai@aspeedtech.com>
+
+description: |
+  The Aspeed PWM controller can support upto 16 PWM outputs.
+  This module is part of the ast2600-pwm-tach multi-function device. For more
+  details see ../mfd/aspeed,ast2600-pwm-tach.yaml.
+
+properties:
+  compatible:
+    enum:
+      - aspeed,ast2600-pwm
+
+  "#pwm-cells":
+    const: 3
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  pinctrl-0: true
+
+  pinctrl-names:
+    const: default
+
+required:
+  - compatible
+  - "#pwm-cells"
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties:
+  description: Set extend properties for each pwm channel.
+  type: object
+  properties:
+    reg:
+      description:
+        The pwm channel index.
+      maxItems: 1
+
+    aspeed,wdt-reload-enable:
+      type: boolean
+      description:
+        Enable the function of wdt reset reload duty point.
+
+    aspeed,wdt-reload-duty-point:
+      description:
+        Define the duty point after wdt reset, 0 = 100%
+      minimum: 0
+      maximum: 255
+
+  required:
+    - reg