Hi Dario,
On 06/09/20 5:41 pm, Dario Binacchi wrote:
> The TI PWMSS driver is a simple bus driver for providing clock and power
> management for the PWM peripherals on TI AM33xx SoCs, namely eCAP,
> eHRPWM and eQEP.
>
> Based on more recent versions of the device tree inside the linux kernel,
> I added the clock domain for each subsystem in am33x.dtsi so it can be
> enabled before probing the peripheral child device and disabled after
> its removal.
>
> Signed-off-by: Dario Binacchi <[email protected]>
> ---
>
> (no changes since v1)
>
> arch/arm/dts/am33xx.dtsi | 9 +++
> doc/device-tree-bindings/pwm/ti,pwmss.txt | 58 ++++++++++++++
> drivers/pwm/Kconfig | 9 ++-
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-ti-pwmss.c | 95 +++++++++++++++++++++++
> 5 files changed, 171 insertions(+), 1 deletion(-)
> create mode 100644 doc/device-tree-bindings/pwm/ti,pwmss.txt
> create mode 100644 drivers/pwm/pwm-ti-pwmss.c
>
> diff --git a/arch/arm/dts/am33xx.dtsi b/arch/arm/dts/am33xx.dtsi
> index d3dd6a16e7..fa7492a9b6 100644
> --- a/arch/arm/dts/am33xx.dtsi
> +++ b/arch/arm/dts/am33xx.dtsi
> @@ -758,6 +758,9 @@
> 0x48300180 0x48300180 0x80 /* EQEP */
> 0x48300200 0x48300200 0x80>; /* EHRPWM */
>
> + clocks = <&l4_per_clkctrl AM3_EPWMSS0_CLKCTRL 0>;
> + clock-names = "fck";
> +
> ecap0: ecap@48300100 {
> compatible = "ti,am3352-ecap",
> "ti,am33xx-ecap";
> @@ -792,6 +795,9 @@
> 0x48302180 0x48302180 0x80 /* EQEP */
> 0x48302200 0x48302200 0x80>; /* EHRPWM */
>
> + clocks = <&l4_per_clkctrl AM3_EPWMSS1_CLKCTRL 0>;
> + clock-names = "fck";
> +
> ecap1: ecap@48302100 {
> compatible = "ti,am3352-ecap",
> "ti,am33xx-ecap";
> @@ -826,6 +832,9 @@
> 0x48304180 0x48304180 0x80 /* EQEP */
> 0x48304200 0x48304200 0x80>; /* EHRPWM */
>
> + clocks = <&l4_per_clkctrl AM3_EPWMSS2_CLKCTRL 0>;
> + clock-names = "fck";
hmm..I do not see these changes in upstream kernel v5.9-rc3 DT. Which version of
kernel are you using? I see al PWM nodes under am33xx-l4.dtsi
> +
> ecap2: ecap@48304100 {
> compatible = "ti,am3352-ecap",
> "ti,am33xx-ecap";
> diff --git a/doc/device-tree-bindings/pwm/ti,pwmss.txt
> b/doc/device-tree-bindings/pwm/ti,pwmss.txt
> new file mode 100644
> index 0000000000..4633697fbd
> --- /dev/null
> +++ b/doc/device-tree-bindings/pwm/ti,pwmss.txt
> @@ -0,0 +1,58 @@
> +TI SOC based PWM Subsystem
> +
> +Required properties:
> +- compatible: Must be "ti,<soc>-pwmss".
> + for am33xx - compatible = "ti,am33xx-pwmss";
> + for am4372 - compatible = "ti,am4372-pwmss","ti,am33xx-pwmss";
> + for dra746 - compatible = "ti,dra746-pwmss", "ti,am33xx-pwmss"
> +
> +- reg: physical base address and size of the registers map.
> +- address-cells: Specify the number of u32 entries needed in child nodes.
> + Should set to 1.
> +- size-cells: specify number of u32 entries needed to specify child nodes
> size
> + in reg property. Should set to 1.
> +- ranges: describes the address mapping of a memory-mapped bus. Should set to
> + physical address map of child's base address, physical address within
> + parent's address space and length of the address map. For am33xx,
> + 3 set of child register maps present, ECAP register space, EQEP
> + register space, EHRPWM register space.
> +
> +Also child nodes should also populated under PWMSS DT node.
> +
> +Example:
> +epwmss0: epwmss@48300000 { /* PWMSS for am33xx */
> + compatible = "ti,am33xx-pwmss";
> + reg = <0x48300000 0x10>;
> + ti,hwmods = "epwmss0";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x48300100 0x48300100 0x80 /* ECAP */
> + 0x48300180 0x48300180 0x80 /* EQEP */
> + 0x48300200 0x48300200 0x80>; /* EHRPWM */
> +
> + /* child nodes go here */
> +};
> +
> +epwmss0: epwmss@48300000 { /* PWMSS for am4372 */
> + compatible = "ti,am4372-pwmss","ti,am33xx-pwmss"
> + reg = <0x48300000 0x10>;
> + ti,hwmods = "epwmss0";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges = <0x48300100 0x48300100 0x80 /* ECAP */
> + 0x48300180 0x48300180 0x80 /* EQEP */
> + 0x48300200 0x48300200 0x80>; /* EHRPWM */
> +
> + /* child nodes go here */
> +};
> +
> +epwmss0: epwmss@4843e000 { /* PWMSS for DRA7xx */
> + compatible = "ti,dra746-pwmss", "ti,am33xx-pwmss";
> + reg = <0x4843e000 0x30>;
> + ti,hwmods = "epwmss0";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + /* child nodes go here */
> +};
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index e4ba8b3d80..129f5d77ca 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -69,8 +69,15 @@ config PWM_SUNXI
> This PWM is found on H3, A64 and other Allwinner SoCs. It supports a
> programmable period and duty cycle. A 16-bit counter is used.
>
> +config PWM_TI_PWMSS
> + bool "Enable support for the AM335x PWM Subsystem"
> + depends on DM_PWM
> + help
> + This enables the AM335x PWM subsystem driver support on TI's SOCs.
> +
> config PWM_TI_EHRPWM
> bool "Enable support for the AM335x EHRPWM"
> - depends on DM_PWM
> + depends on PWM_TI_PWMSS
> + default y
> help
> This enables the AM335x EHRPWM driver support on TI's SoCs.
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index c5f88f7501..df9c54e764 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -19,3 +19,4 @@ obj-$(CONFIG_PWM_SIFIVE) += pwm-sifive.o
> obj-$(CONFIG_PWM_TEGRA) += tegra_pwm.o
> obj-$(CONFIG_PWM_SUNXI) += sunxi_pwm.o
> obj-$(CONFIG_PWM_TI_EHRPWM) += pwm-ti-ehrpwm.o
> +obj-$(CONFIG_PWM_TI_PWMSS) += pwm-ti-pwmss.o
> diff --git a/drivers/pwm/pwm-ti-pwmss.c b/drivers/pwm/pwm-ti-pwmss.c
> new file mode 100644
> index 0000000000..57f86512d6
> --- /dev/null
> +++ b/drivers/pwm/pwm-ti-pwmss.c
> @@ -0,0 +1,95 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Pulse-Width Modulation Subsystem (pwmss)
> + *
> + * Copyright (C) 2020 Dario Binacchi <[email protected]>
> + */
> +#define DEBUG
> +#undef CONFIG_LOGLEVEL
> +#define CONFIG_LOGLEVEL 8
> +
> +#include <common.h>
> +#include <clk.h>
> +#include <dm.h>
> +
> +struct ti_pwmss_priv {
> + int child_count;
> + struct clk clkdm;
> +};
> +
> +static const struct udevice_id ti_pwmss_ids[] = {
> + {.compatible = "ti,am33xx-pwmss"},
> + {}
> +};
> +
> +static int ti_pwmss_child_post_remove(struct udevice *dev)
> +{
> + struct ti_pwmss_priv *priv = dev_get_priv(dev->parent);
> + int err;
> +
> + if (--priv->child_count > 0)
> + return 0;
> +
> + err = clk_disable(&priv->clkdm);
> + if (err) {
> + dev_err(dev->parent, "%s: failed to disable clock domain\n",
> + __func__);
> + goto clkdm_disable_err;
> + }
> +
> + err = clk_release_all(&priv->clkdm, 1);
> + if (err) {
> + dev_err(dev->parent, "%s: failed to release clock domain\n",
> + __func__);
> + goto clkdm_release_err;
> + }
> +
> + return 0;
> +
> +clkdm_release_err:
> + clk_enable(&priv->clkdm);
> +clkdm_disable_err:
> + priv->child_count++;
> + return err;
> +}
> +
> +static int ti_pwmss_child_pre_probe(struct udevice *dev)
> +{
> + struct ti_pwmss_priv *priv = dev_get_priv(dev->parent);
> + int err;
> +
> + if (++priv->child_count > 1)
> + return 0;
> +
> + err = clk_get_by_name(dev->parent, "fck", &priv->clkdm);
> + if (err) {
> + dev_err(dev->parent, "%s: failed to get clock domain\n",
> + __func__);
> + goto clkdm_get_err;
> + }
> +
> + err = clk_enable(&priv->clkdm);
> + if (err) {
> + dev_err(dev->parent, "%s: failed to enable clock domain\n",
> + __func__);
> + goto clkdm_enable_err;
> + }
Do we need a whole subsystem driver just for enabling the clocks? Can't the same
be happen in driver?
Thanks and regards,
Lokesh
