2022-11-01 10:22:16

by Billy Tsai

[permalink] [raw]
Subject: [v2 0/3] upport pwm/tach driver for aspeed ast26xx

Unlike the old design that the register setting of the TACH should based
on the configure of the PWM. In ast26xx, the dependency between pwm and
tach controller is eliminated and becomes a separate hardware block. One
is used to provide pwm output and another is used to monitor the frequency
of the input. Therefore, this patch serials implements them by writing the
two driver "pwm-aspeed-ast2600.c" and "tach-aspeed-ast2600.c". The former
is following the pwm subsystem which can apply the existed driver to
controller the fan(pwm-fan.c), beeper(pwm-beeper.c) and so on. The latter
is following the sysfs interface of hwmon to creat the node for fan
monitor.

Changes since v1:
- tach:
- Add the document tach-aspeed-ast2600.rst
- Use devm_* api to simplify the error cleanup.
- Change hwmon register api to devm_hwmon_device_register_with_info

Billy Tsai (3):
dt-bindings: Add bindings for aspeed pwm-tach.
pwm: Add Aspeed ast2600 PWM support
hwmon: Add Aspeed ast2600 TACH support

.../bindings/hwmon/aspeed,ast2600-tach.yaml | 48 ++
.../bindings/mfd/aspeed,ast2600-pwm-tach.yaml | 76 +++
.../bindings/pwm/aspeed,ast2600-pwm.yaml | 64 +++
Documentation/hwmon/tach-aspeed-ast2600.rst | 28 +
drivers/hwmon/Kconfig | 9 +
drivers/hwmon/Makefile | 1 +
drivers/hwmon/tach-aspeed-ast2600.c | 484 ++++++++++++++++++
drivers/pwm/Kconfig | 10 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-aspeed-ast2600.c | 325 ++++++++++++
10 files changed, 1046 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
create mode 100644 Documentation/hwmon/tach-aspeed-ast2600.rst
create mode 100644 drivers/hwmon/tach-aspeed-ast2600.c
create mode 100644 drivers/pwm/pwm-aspeed-ast2600.c

--
2.25.1



2022-11-01 10:24:55

by Billy Tsai

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

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 <[email protected]>
---
.../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

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 <[email protected]>
+
+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 <[email protected]>
+
+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 <[email protected]>
+
+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
--
2.25.1


2022-11-01 19:31:50

by Rob Herring

[permalink] [raw]
Subject: Re: [v2 1/3] dt-bindings: Add bindings for aspeed pwm-tach.

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 <[email protected]>
> ---
> .../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/[email protected]/

2022-11-02 04:12:01

by Billy Tsai

[permalink] [raw]
Subject: Re: [v2 1/3] dt-bindings: Add bindings for aspeed pwm-tach.

Hi Rob,

On 2022/11/2, 2:40 AM, "Rob Herring" <[email protected]> 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 <[email protected]>
> > ---
> > .../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] [email protected] <https://lore.kernel.org/all/<a href=>/">https://lore.kernel.org/all/[email protected]/

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

2022-11-02 20:02:42

by Rob Herring

[permalink] [raw]
Subject: Re: [v2 1/3] dt-bindings: Add bindings for aspeed pwm-tach.

On Tue, Nov 1, 2022 at 10:21 PM Billy Tsai <[email protected]> wrote:
>
> Hi Rob,
>
> On 2022/11/2, 2:40 AM, "Rob Herring" <[email protected]> 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 <[email protected]>
> > > ---
> > > .../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] [email protected] <https://lore.kernel.org/all/<a href=>/">https://lore.kernel.org/all/[email protected]/
>
> 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

2022-11-03 07:53:20

by Billy Tsai

[permalink] [raw]
Subject: Re: [v2 1/3] dt-bindings: Add bindings for aspeed pwm-tach.

On 2022/11/3, 2:19 AM, "Rob Herring" <[email protected]> wrote:

On Tue, Nov 1, 2022 at 10:21 PM Billy Tsai <[email protected]> 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


2022-11-14 08:49:04

by Billy Tsai

[permalink] [raw]
Subject: Re: [v2 1/3] dt-bindings: Add bindings for aspeed pwm-tach.

Hi Rob,

On 2022/11/3, 3:39 PM, "Billy Tsai" <[email protected]> wrote:

On 2022/11/3, 2:19 AM, "Rob Herring" <[email protected]> wrote:

On Tue, Nov 1, 2022 at 10:21 PM Billy Tsai <[email protected]> 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