[0/3] Support 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.

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 ++
 drivers/hwmon/Kconfig                         |   9 +
 drivers/hwmon/Makefile                        |   1 +
 drivers/hwmon/tach-aspeed-ast2600.c           | 692 ++++++++++++++++++
 drivers/pwm/Kconfig                           |  10 +
 drivers/pwm/Makefile                          |   1 +
 drivers/pwm/pwm-aspeed-ast2600.c              | 325 ++++++++
 9 files changed, 1226 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 drivers/hwmon/tach-aspeed-ast2600.c
 create mode 100644 drivers/pwm/pwm-aspeed-ast2600.c

On Mon, Oct 31, 2022 at 06:38:08PM +0800, Billy Tsai wrote:
> This patch add the support of Tachometer which can use to monitor the
> frequency of the input. The tach supports up to 16 channels and it's part
> function of multi-function device "pwm-tach controller".
> 
> Signed-off-by: Billy Tsai <billy_tsai@aspeedtech.com>
> ---
>  drivers/hwmon/Kconfig               |   9 +
>  drivers/hwmon/Makefile              |   1 +
>  drivers/hwmon/tach-aspeed-ast2600.c | 692 ++++++++++++++++++++++++++++

Please also provide Documentation/hwmon/tach-aspeed-ast2600.rst.

[ ... ]

> +	hwmon = devm_hwmon_device_register_with_groups(dev, "aspeed_tach", priv,
> +						       priv->groups);

Please use the new hwmon api (devm_hwmon_device_register_with_info).

> +	ret = PTR_ERR_OR_ZERO(hwmon);
> +	if (ret) {
> +		dev_err_probe(dev, ret, "Failed to register hwmon device\n");
> +		goto err_assert_reset;
> +	}
> +	platform_set_drvdata(pdev, priv);
> +	return 0;
> +err_assert_reset:
> +	reset_control_assert(priv->reset);
> +err_disable_clk:
> +	clk_disable_unprepare(priv->clk);

You should be able to use devm_clk_get_enabled() to handle
the clock. If reset handling has to come first, you could use
devm_add_action_or_reset() for it. This way you would
not need the remove function, and error cleanup would
be much simplified.

Thanks,
Guenter

> +	return ret;
> +}
> +
> +static int aspeed_tach_remove(struct platform_device *pdev)
> +{
> +	struct aspeed_tach_data *priv = platform_get_drvdata(pdev);
> +
> +	reset_control_assert(priv->reset);
> +	clk_disable_unprepare(priv->clk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id of_stach_match_table[] = {
> +	{
> +		.compatible = "aspeed,ast2600-tach",
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, of_stach_match_table);
> +
> +static struct platform_driver aspeed_tach_driver = {
> +	.probe		= aspeed_tach_probe,
> +	.remove		= aspeed_tach_remove,
> +	.driver		= {
> +		.name	= "aspeed_tach",
> +		.of_match_table = of_stach_match_table,
> +	},
> +};
> +
> +module_platform_driver(aspeed_tach_driver);
> +
> +MODULE_AUTHOR("Billy Tsai <billy_tsai@aspeedtech.com>");
> +MODULE_DESCRIPTION("Aspeed ast2600 TACH device driver");
> +MODULE_LICENSE("GPL v2");
> +
> -- 
> 2.25.1
>