[PATCH 0/5] Tegra264 PWM support

[PATCH 0/5] Tegra264 PWM support

[Mikko Perttunen]

Hello,

this adds support for the PWM controller on Tegra264. The controller
is similar to previous generations, but the register fields are
widened, the depth is made configurable, and the enable bit moves
to a different spot.

This series adds only basic support with fixed depth -- configurable
depth will come later.

The series uses the nvidia,tegra264-pwm compatible string. Bindings
for this are added in Thierry's series

  https://lore.kernel.org/linux-tegra/20260320234056.2579010-1-thierry.reding@kernel.org/

Thanks,
Mikko

---
Mikko Perttunen (4):
      pwm: tegra: Modify read/write accessors for multi-register channel
      pwm: tegra: Parametrize enable register offset
      pwm: tegra: Parametrize duty and scale field widths
      pwm: tegra: Add support for Tegra264

Yi-Wei Wang (1):
      pwm: tegra: Avoid hard-coded max clock frequency

 drivers/pwm/pwm-tegra.c | 141 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 98 insertions(+), 43 deletions(-)
---
base-commit: b7cac19bc4780fe1156b217a1c5c96a3e23b275b
change-id: 20260303-t264-pwm-57e10d039df1

[PATCH 1/5] pwm: tegra: Avoid hard-coded max clock frequency

[Mikko Perttunen]

From: Yi-Wei Wang <yiweiw@nvidia.com>

The clock driving the Tegra PWM IP can be sourced from different parent
clocks. Hence, let dev_pm_opp_set_rate() set the max clock rate based
upon the current parent clock that can be specified via device-tree.

After this, the Tegra194 SoC data becomes redundant, so get rid of it.

Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
Co-developed-by: Mikko Perttunen <mperttunen@nvidia.com>
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/pwm/pwm-tegra.c | 16 +++-------------
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 172063b51d44..759b98b97b6e 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -59,9 +59,6 @@
 
 struct tegra_pwm_soc {
 	unsigned int num_channels;
-
-	/* Maximum IP frequency for given SoCs */
-	unsigned long max_frequency;
 };
 
 struct tegra_pwm_chip {
@@ -303,7 +300,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 		return ret;
 
 	/* Set maximum frequency of the IP */
-	ret = dev_pm_opp_set_rate(&pdev->dev, pc->soc->max_frequency);
+	ret = dev_pm_opp_set_rate(&pdev->dev, S64_MAX);
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Failed to set max frequency: %d\n", ret);
 		goto put_pm;
@@ -318,7 +315,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 
 	/* Set minimum limit of PWM period for the IP */
 	pc->min_period_ns =
-	    (NSEC_PER_SEC / (pc->soc->max_frequency >> PWM_DUTY_WIDTH)) + 1;
+	    (NSEC_PER_SEC / (pc->clk_rate >> PWM_DUTY_WIDTH)) + 1;
 
 	pc->rst = devm_reset_control_get_exclusive(&pdev->dev, "pwm");
 	if (IS_ERR(pc->rst)) {
@@ -397,23 +394,16 @@ static int __maybe_unused tegra_pwm_runtime_resume(struct device *dev)
 
 static const struct tegra_pwm_soc tegra20_pwm_soc = {
 	.num_channels = 4,
-	.max_frequency = 48000000UL,
 };
 
 static const struct tegra_pwm_soc tegra186_pwm_soc = {
 	.num_channels = 1,
-	.max_frequency = 102000000UL,
-};
-
-static const struct tegra_pwm_soc tegra194_pwm_soc = {
-	.num_channels = 1,
-	.max_frequency = 408000000UL,
 };
 
 static const struct of_device_id tegra_pwm_of_match[] = {
 	{ .compatible = "nvidia,tegra20-pwm", .data = &tegra20_pwm_soc },
 	{ .compatible = "nvidia,tegra186-pwm", .data = &tegra186_pwm_soc },
-	{ .compatible = "nvidia,tegra194-pwm", .data = &tegra194_pwm_soc },
+	{ .compatible = "nvidia,tegra194-pwm", .data = &tegra186_pwm_soc },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, tegra_pwm_of_match);

-- 
2.53.0

[PATCH 2/5] pwm: tegra: Modify read/write accessors for multi-register channel

[Mikko Perttunen]

On Tegra264, each PWM instance has two registers (per channel, of which
there is one). Update the pwm_readl/pwm_writel helper functions to
take channel (as struct pwm_device *) and offset separately.

Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/pwm/pwm-tegra.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 759b98b97b6e..cf54f75d92a5 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -57,6 +57,8 @@
 #define PWM_SCALE_WIDTH	13
 #define PWM_SCALE_SHIFT	0
 
+#define PWM_CSR_0	0
+
 struct tegra_pwm_soc {
 	unsigned int num_channels;
 };
@@ -78,14 +80,18 @@ static inline struct tegra_pwm_chip *to_tegra_pwm_chip(struct pwm_chip *chip)
 	return pwmchip_get_drvdata(chip);
 }
 
-static inline u32 pwm_readl(struct tegra_pwm_chip *pc, unsigned int offset)
+static inline u32 pwm_readl(struct pwm_device *dev, unsigned int offset)
 {
-	return readl(pc->regs + (offset << 4));
+	struct tegra_pwm_chip *chip = to_tegra_pwm_chip(dev->chip);
+
+	return readl(chip->regs + (dev->hwpwm * 16) + offset);
 }
 
-static inline void pwm_writel(struct tegra_pwm_chip *pc, unsigned int offset, u32 value)
+static inline void pwm_writel(struct pwm_device *dev, unsigned int offset, u32 value)
 {
-	writel(value, pc->regs + (offset << 4));
+	struct tegra_pwm_chip *chip = to_tegra_pwm_chip(dev->chip);
+
+	writel(value, chip->regs + (dev->hwpwm * 16) + offset);
 }
 
 static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
@@ -194,7 +200,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	} else
 		val |= PWM_ENABLE;
 
-	pwm_writel(pc, pwm->hwpwm, val);
+	pwm_writel(pwm, PWM_CSR_0, val);
 
 	/*
 	 * If the PWM is not enabled, turn the clock off again to save power.
@@ -207,7 +213,6 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static int tegra_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
 	int rc = 0;
 	u32 val;
 
@@ -215,21 +220,20 @@ static int tegra_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (rc)
 		return rc;
 
-	val = pwm_readl(pc, pwm->hwpwm);
+	val = pwm_readl(pwm, PWM_CSR_0);
 	val |= PWM_ENABLE;
-	pwm_writel(pc, pwm->hwpwm, val);
+	pwm_writel(pwm, PWM_CSR_0, val);
 
 	return 0;
 }
 
 static void tegra_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
-	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
 	u32 val;
 
-	val = pwm_readl(pc, pwm->hwpwm);
+	val = pwm_readl(pwm, PWM_CSR_0);
 	val &= ~PWM_ENABLE;
-	pwm_writel(pc, pwm->hwpwm, val);
+	pwm_writel(pwm, PWM_CSR_0, val);
 
 	pm_runtime_put_sync(pwmchip_parent(chip));
 }

-- 
2.53.0

[PATCH 3/5] pwm: tegra: Parametrize enable register offset

[Mikko Perttunen]

On Tegra264, the PWM enablement bit is not located at the base address
of the PWM controller. Hence, introduce an enablement offset field in
the tegra_pwm_soc structure to describe the offset of the register.

Co-developed-by: Yi-Wei Wang <yiweiw@nvidia.com>
Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/pwm/pwm-tegra.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index cf54f75d92a5..22d709986e8c 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -61,6 +61,7 @@
 
 struct tegra_pwm_soc {
 	unsigned int num_channels;
+	unsigned int enable_reg;
 };
 
 struct tegra_pwm_chip {
@@ -197,8 +198,9 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		err = pm_runtime_resume_and_get(pwmchip_parent(chip));
 		if (err)
 			return err;
-	} else
+	} else if (pc->soc->enable_reg == PWM_CSR_0) {
 		val |= PWM_ENABLE;
+	}
 
 	pwm_writel(pwm, PWM_CSR_0, val);
 
@@ -213,6 +215,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 static int tegra_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
+	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
 	int rc = 0;
 	u32 val;
 
@@ -220,20 +223,22 @@ static int tegra_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm)
 	if (rc)
 		return rc;
 
-	val = pwm_readl(pwm, PWM_CSR_0);
+
+	val = pwm_readl(pwm, pc->soc->enable_reg);
 	val |= PWM_ENABLE;
-	pwm_writel(pwm, PWM_CSR_0, val);
+	pwm_writel(pwm, pc->soc->enable_reg, val);
 
 	return 0;
 }
 
 static void tegra_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
 {
+	struct tegra_pwm_chip *pc = to_tegra_pwm_chip(chip);
 	u32 val;
 
-	val = pwm_readl(pwm, PWM_CSR_0);
+	val = pwm_readl(pwm, pc->soc->enable_reg);
 	val &= ~PWM_ENABLE;
-	pwm_writel(pwm, PWM_CSR_0, val);
+	pwm_writel(pwm, pc->soc->enable_reg, val);
 
 	pm_runtime_put_sync(pwmchip_parent(chip));
 }
@@ -398,10 +403,12 @@ static int __maybe_unused tegra_pwm_runtime_resume(struct device *dev)
 
 static const struct tegra_pwm_soc tegra20_pwm_soc = {
 	.num_channels = 4,
+	.enable_reg = PWM_CSR_0,
 };
 
 static const struct tegra_pwm_soc tegra186_pwm_soc = {
 	.num_channels = 1,
+	.enable_reg = PWM_CSR_0,
 };
 
 static const struct of_device_id tegra_pwm_of_match[] = {

-- 
2.53.0

[PATCH 4/5] pwm: tegra: Parametrize duty and scale field widths

[Mikko Perttunen]

Tegra264 has wider fields for the duty and scale register fields.
Parameterize the driver in preparation. The depth value also
becomes disconnected from the width of the duty field, so define
it separately.

Co-developed-by: Yi-Wei Wang <yiweiw@nvidia.com>
Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/pwm/pwm-tegra.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 22d709986e8c..857301baad51 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -52,16 +52,19 @@
 #include <soc/tegra/common.h>
 
 #define PWM_ENABLE	(1 << 31)
-#define PWM_DUTY_WIDTH	8
 #define PWM_DUTY_SHIFT	16
-#define PWM_SCALE_WIDTH	13
 #define PWM_SCALE_SHIFT	0
 
 #define PWM_CSR_0	0
 
+#define PWM_DEPTH	256
+
 struct tegra_pwm_soc {
 	unsigned int num_channels;
 	unsigned int enable_reg;
+
+	unsigned int duty_width;
+	unsigned int scale_width;
 };
 
 struct tegra_pwm_chip {
@@ -106,22 +109,22 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	/*
 	 * Convert from duty_ns / period_ns to a fixed number of duty ticks
-	 * per (1 << PWM_DUTY_WIDTH) cycles and make sure to round to the
+	 * per PWM_DEPTH cycles and make sure to round to the
 	 * nearest integer during division.
 	 */
-	c *= (1 << PWM_DUTY_WIDTH);
+	c *= PWM_DEPTH;
 	c = DIV_ROUND_CLOSEST_ULL(c, period_ns);
 
 	val = (u32)c << PWM_DUTY_SHIFT;
 
 	/*
-	 *  min period = max clock limit >> PWM_DUTY_WIDTH
+	 *  min period = max clock limit / PWM_DEPTH
 	 */
 	if (period_ns < pc->min_period_ns)
 		return -EINVAL;
 
 	/*
-	 * Compute the prescaler value for which (1 << PWM_DUTY_WIDTH)
+	 * Compute the prescaler value for which PWM_DEPTH
 	 * cycles at the PWM clock rate will take period_ns nanoseconds.
 	 *
 	 * num_channels: If single instance of PWM controller has multiple
@@ -135,7 +138,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 */
 	if (pc->soc->num_channels == 1) {
 		/*
-		 * Rate is multiplied with 2^PWM_DUTY_WIDTH so that it matches
+		 * Rate is multiplied with PWM_DEPTH so that it matches
 		 * with the maximum possible rate that the controller can
 		 * provide. Any further lower value can be derived by setting
 		 * PFM bits[0:12].
@@ -145,7 +148,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 		 * source clock rate as required_clk_rate, PWM controller will
 		 * be able to configure the requested period.
 		 */
-		required_clk_rate = DIV_ROUND_UP_ULL((u64)NSEC_PER_SEC << PWM_DUTY_WIDTH,
+		required_clk_rate = DIV_ROUND_UP_ULL((u64)NSEC_PER_SEC * PWM_DEPTH,
 						     period_ns);
 
 		if (required_clk_rate > clk_round_rate(pc->clk, required_clk_rate))
@@ -169,7 +172,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 
 	/* Consider precision in PWM_SCALE_WIDTH rate calculation */
 	rate = mul_u64_u64_div_u64(pc->clk_rate, period_ns,
-				   (u64)NSEC_PER_SEC << PWM_DUTY_WIDTH);
+				   (u64)NSEC_PER_SEC * PWM_DEPTH);
 
 	/*
 	 * Since the actual PWM divider is the register's frequency divider
@@ -185,7 +188,7 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
 	 * Make sure that the rate will fit in the register's frequency
 	 * divider field.
 	 */
-	if (rate >> PWM_SCALE_WIDTH)
+	if (rate >> pc->soc->scale_width)
 		return -EINVAL;
 
 	val |= rate << PWM_SCALE_SHIFT;
@@ -324,7 +327,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
 
 	/* Set minimum limit of PWM period for the IP */
 	pc->min_period_ns =
-	    (NSEC_PER_SEC / (pc->clk_rate >> PWM_DUTY_WIDTH)) + 1;
+	    (NSEC_PER_SEC / (pc->clk_rate / PWM_DEPTH)) + 1;
 
 	pc->rst = devm_reset_control_get_exclusive(&pdev->dev, "pwm");
 	if (IS_ERR(pc->rst)) {
@@ -404,11 +407,15 @@ static int __maybe_unused tegra_pwm_runtime_resume(struct device *dev)
 static const struct tegra_pwm_soc tegra20_pwm_soc = {
 	.num_channels = 4,
 	.enable_reg = PWM_CSR_0,
+	.duty_width = 8,
+	.scale_width = 13,
 };
 
 static const struct tegra_pwm_soc tegra186_pwm_soc = {
 	.num_channels = 1,
 	.enable_reg = PWM_CSR_0,
+	.duty_width = 8,
+	.scale_width = 13,
 };
 
 static const struct of_device_id tegra_pwm_of_match[] = {

-- 
2.53.0

[PATCH 5/5] pwm: tegra: Add support for Tegra264

[Mikko Perttunen]

Tegra264 changes the register layout to accommodate wider fields
for duty and scale, and adds configurable depth which will be
supported in a later patch.

Add SoC data and update top comment to describe register layout
in more detail.

Co-developed-by: Yi-Wei Wang <yiweiw@nvidia.com>
Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/pwm/pwm-tegra.c | 75 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 61 insertions(+), 14 deletions(-)

diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
index 857301baad51..c1e8a804d783 100644
--- a/drivers/pwm/pwm-tegra.c
+++ b/drivers/pwm/pwm-tegra.c
@@ -7,22 +7,60 @@
  * Copyright (c) 2010-2020, NVIDIA Corporation.
  * Based on arch/arm/plat-mxc/pwm.c by Sascha Hauer <s.hauer@pengutronix.de>
  *
- * Overview of Tegra Pulse Width Modulator Register:
- * 1. 13-bit: Frequency division (SCALE)
- * 2. 8-bit : Pulse division (DUTY)
- * 3. 1-bit : Enable bit
+ * Overview of Tegra Pulse Width Modulator Register
+ * CSR_0 of Tegra20, Tegra186, and Tegra194:
+ * +-------+-------+-----------------------------------------------------------+
+ * | Bit   | Field | Description                                               |
+ * +-------+-------+-----------------------------------------------------------+
+ * | 31    | ENB   | Enable Pulse width modulator.                             |
+ * |       |       | 0 = DISABLE, 1 = ENABLE.                                  |
+ * +-------+-------+-----------------------------------------------------------+
+ * | 30:16 | PWM_0 | Pulse width that needs to be programmed.                  |
+ * |       |       | 0 = Always low.                                           |
+ * |       |       | 1 = 1 / 256 pulse high.                                   |
+ * |       |       | 2 = 2 / 256 pulse high.                                   |
+ * |       |       | N = N / 256 pulse high.                                   |
+ * |       |       | Only 8 bits are usable [23:16].                           |
+ * |       |       | Bit[24] can be programmed to 1 to achieve 100% duty       |
+ * |       |       | cycle. In this case the other bits [23:16] are set to     |
+ * |       |       | don’t care.                                               |
+ * +-------+-------+-----------------------------------------------------------+
+ * | 12:0  | PFM_0 | Frequency divider that needs to be programmed, also known |
+ * |       |       | as SCALE. Division by (1 + PFM_0).                        |
+ * +-------+-------+-----------------------------------------------------------+
  *
- * The PWM clock frequency is divided by 256 before subdividing it based
- * on the programmable frequency division value to generate the required
- * frequency for PWM output. The maximum output frequency that can be
- * achieved is (max rate of source clock) / 256.
- * e.g. if source clock rate is 408 MHz, maximum output frequency can be:
- * 408 MHz/256 = 1.6 MHz.
- * This 1.6 MHz frequency can further be divided using SCALE value in PWM.
+ * CSR_0 of Tegra264:
+ * +-------+-------+-----------------------------------------------------------+
+ * | Bit   | Field | Description                                               |
+ * +-------+-------+-----------------------------------------------------------+
+ * | 31:16 | PWM_0 | Pulse width that needs to be programmed.                  |
+ * |       |       | 0 = Always low.                                           |
+ * |       |       | 1 = 1 / (1 + CSR_1.DEPTH) pulse high.                     |
+ * |       |       | 2 = 2 / (1 + CSR_1.DEPTH) pulse high.                     |
+ * |       |       | N = N / (1 + CSR_1.DEPTH) pulse high.                     |
+ * +-------+-------+-----------------------------------------------------------+
+ * | 15:0  | PFM_0 | Frequency divider that needs to be programmed, also known |
+ * |       |       | as SCALE. Division by (1 + PFM_0).                        |
+ * +-------+-------+-----------------------------------------------------------+
+ *
+ * CSR_1 of Tegra264:
+ * +-------+-------+-----------------------------------------------------------+
+ * | Bit   | Field | Description                                               |
+ * +-------+-------+-----------------------------------------------------------+
+ * | 31    | ENB   | Enable Pulse width modulator.                             |
+ * |       |       | 0 = DISABLE, 1 = ENABLE.                                  |
+ * +-------+-------+-----------------------------------------------------------+
+ * | 30:15 | DEPTH | Depth for pulse width modulator. This controls the pulse  |
+ * |       |       | time generated. Division by (1 + CSR_1.DEPTH).            |
+ * +-------+-------+-----------------------------------------------------------+
  *
- * PWM pulse width: 8 bits are usable [23:16] for varying pulse width.
- * To achieve 100% duty cycle, program Bit [24] of this register to
- * 1’b1. In which case the other bits [23:16] are set to don't care.
+ * The PWM clock frequency is divided by DEPTH = (1 + CSR_1.DEPTH) before subdividing it
+ * based on the programmable frequency division value to generate the required frequency
+ * for PWM output. DEPTH is fixed to 256 before Tegra264. The maximum output frequency
+ * that can be achieved is (max rate of source clock) / DEPTH.
+ * e.g. if source clock rate is 408 MHz, and DEPTH = 256, maximum output frequency can be:
+ * 408 MHz / 256 ~= 1.6 MHz.
+ * This 1.6 MHz frequency can further be divided using SCALE value in PWM.
  *
  * Limitations:
  * -	When PWM is disabled, the output is driven to inactive.
@@ -56,6 +94,7 @@
 #define PWM_SCALE_SHIFT	0
 
 #define PWM_CSR_0	0
+#define PWM_CSR_1	4
 
 #define PWM_DEPTH	256
 
@@ -418,10 +457,18 @@ static const struct tegra_pwm_soc tegra186_pwm_soc = {
 	.scale_width = 13,
 };
 
+static const struct tegra_pwm_soc tegra264_pwm_soc = {
+	.num_channels = 1,
+	.enable_reg = PWM_CSR_1,
+	.duty_width = 16,
+	.scale_width = 16,
+};
+
 static const struct of_device_id tegra_pwm_of_match[] = {
 	{ .compatible = "nvidia,tegra20-pwm", .data = &tegra20_pwm_soc },
 	{ .compatible = "nvidia,tegra186-pwm", .data = &tegra186_pwm_soc },
 	{ .compatible = "nvidia,tegra194-pwm", .data = &tegra186_pwm_soc },
+	{ .compatible = "nvidia,tegra264-pwm", .data = &tegra264_pwm_soc },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, tegra_pwm_of_match);

-- 
2.53.0

Re: [PATCH 5/5] pwm: tegra: Add support for Tegra264

[Krzysztof Kozlowski]

On 23/03/2026 03:36, Mikko Perttunen wrote:
> +
>  static const struct of_device_id tegra_pwm_of_match[] = {
>  	{ .compatible = "nvidia,tegra20-pwm", .data = &tegra20_pwm_soc },
>  	{ .compatible = "nvidia,tegra186-pwm", .data = &tegra186_pwm_soc },
>  	{ .compatible = "nvidia,tegra194-pwm", .data = &tegra186_pwm_soc },
> +	{ .compatible = "nvidia,tegra264-pwm", .data = &tegra264_pwm_soc },

Undocumented ABI.

Please run scripts/checkpatch.pl on the patches and fix reported
warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
patches and (probably) fix more warnings. Some warnings can be ignored,
especially from --strict run, but the code here looks like it needs a
fix. Feel free to get in touch if the warning is not clear.


Best regards,
Krzysztof

Re: [PATCH 0/5] Tegra264 PWM support

[Krzysztof Kozlowski]

On 23/03/2026 03:36, Mikko Perttunen wrote:
> Hello,
> 
> this adds support for the PWM controller on Tegra264. The controller
> is similar to previous generations, but the register fields are
> widened, the depth is made configurable, and the enable bit moves
> to a different spot.
> 
> This series adds only basic support with fixed depth -- configurable
> depth will come later.
> 
> The series uses the nvidia,tegra264-pwm compatible string. Bindings
> for this are added in Thierry's series
> 
>   https://lore.kernel.org/linux-tegra/20260320234056.2579010-1-thierry.reding@kernel.org/

NAK, that's not how you send driver code.

Best regards,
Krzysztof

Re: [PATCH 5/5] pwm: tegra: Add support for Tegra264

[Mikko Perttunen]

On Monday, March 23, 2026 4:24 PM Krzysztof Kozlowski wrote:
> On 23/03/2026 03:36, Mikko Perttunen wrote:
> > +
> > 
> >  static const struct of_device_id tegra_pwm_of_match[] = {
> >  
> >  	{ .compatible = "nvidia,tegra20-pwm", .data = &tegra20_pwm_soc },
> >  	{ .compatible = "nvidia,tegra186-pwm", .data = &tegra186_pwm_soc },
> >  	{ .compatible = "nvidia,tegra194-pwm", .data = &tegra186_pwm_soc },
> > 
> > +	{ .compatible = "nvidia,tegra264-pwm", .data = &tegra264_pwm_soc },
> 
> Undocumented ABI.
> 
> Please run scripts/checkpatch.pl on the patches and fix reported
> warnings. After that, run also 'scripts/checkpatch.pl --strict' on the
> patches and (probably) fix more warnings. Some warnings can be ignored,
> especially from --strict run, but the code here looks like it needs a
> fix. Feel free to get in touch if the warning is not clear.
> 
> 
> Best regards,
> Krzysztof

So, this series was checkpatch --strict clean (modulo one false positive) when 
I submitted. I agree that that is strange, so I looked into it and looks like 
I had an old processed-schema.json file containing tegra264-pwm, so checkpatch 
didn't complain. It is of course in .gitignore so I didn't notice.

Mikko

Re: [PATCH 0/5] Tegra264 PWM support

[Mikko Perttunen]

On Monday, March 23, 2026 4:24 PM Krzysztof Kozlowski wrote:
> On 23/03/2026 03:36, Mikko Perttunen wrote:
> > Hello,
> > 
> > this adds support for the PWM controller on Tegra264. The controller
> > is similar to previous generations, but the register fields are
> > widened, the depth is made configurable, and the enable bit moves
> > to a different spot.
> > 
> > This series adds only basic support with fixed depth -- configurable
> > depth will come later.
> > 
> > The series uses the nvidia,tegra264-pwm compatible string. Bindings
> > for this are added in Thierry's series
> > 
> >   https://lore.kernel.org/linux-tegra/20260320234056.2579010-1-thierry.red
> >   ing@kernel.org/
> NAK, that's not how you send driver code.
> 
> Best regards,
> Krzysztof

I will pick up Thierry's patches for subsequent revisions.

Mikko

Re: [PATCH 1/5] pwm: tegra: Avoid hard-coded max clock frequency

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2483 bytes --]

On Mon, Mar 23, 2026 at 11:36:37AM +0900, Mikko Perttunen wrote:
> From: Yi-Wei Wang <yiweiw@nvidia.com>
> 
> The clock driving the Tegra PWM IP can be sourced from different parent
> clocks. Hence, let dev_pm_opp_set_rate() set the max clock rate based
> upon the current parent clock that can be specified via device-tree.
> 
> After this, the Tegra194 SoC data becomes redundant, so get rid of it.
> 
> Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
> Co-developed-by: Mikko Perttunen <mperttunen@nvidia.com>
> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  drivers/pwm/pwm-tegra.c | 16 +++-------------
>  1 file changed, 3 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> index 172063b51d44..759b98b97b6e 100644
> --- a/drivers/pwm/pwm-tegra.c
> +++ b/drivers/pwm/pwm-tegra.c
> @@ -59,9 +59,6 @@
>  
>  struct tegra_pwm_soc {
>  	unsigned int num_channels;
> -
> -	/* Maximum IP frequency for given SoCs */
> -	unsigned long max_frequency;
>  };
>  
>  struct tegra_pwm_chip {
> @@ -303,7 +300,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	/* Set maximum frequency of the IP */
> -	ret = dev_pm_opp_set_rate(&pdev->dev, pc->soc->max_frequency);
> +	ret = dev_pm_opp_set_rate(&pdev->dev, S64_MAX);

The documentation comment for dev_pm_opp_set_rate() reads:

	Device wanting to run at fmax provided by the opp, should have
	already rounded to the target OPP's frequency.

I think using S64_MAX is technically fine (assuming there are no issues
with big numbers in that function), but still it feels wrong to use
something simpler than the comment suggests. Am I missing something?

>  	if (ret < 0) {
>  		dev_err(&pdev->dev, "Failed to set max frequency: %d\n", ret);
>  		goto put_pm;
> @@ -318,7 +315,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)
>  
>  	/* Set minimum limit of PWM period for the IP */
>  	pc->min_period_ns =
> -	    (NSEC_PER_SEC / (pc->soc->max_frequency >> PWM_DUTY_WIDTH)) + 1;
> +	    (NSEC_PER_SEC / (pc->clk_rate >> PWM_DUTY_WIDTH)) + 1;

Orthogonal to this patch: Should this be

	DIV_ROUND_UP(NSEC_PER_SEC, pc->clk_rate >> PWM_DUTY_WIDTH)

? Or even

	DIV_ROUND_UP(NSEC_PER_SEC < PWM_DUTY_WIDTH, pc->clk_rate);

? (Note, the latter doesn't work as is, as the first parameter has an
overflow, I guess you're still getting my question.)

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/5] Tegra264 PWM support

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1035 bytes --]

Hello Mikko,

On Mon, Mar 23, 2026 at 11:36:36AM +0900, Mikko Perttunen wrote:
> this adds support for the PWM controller on Tegra264. The controller
> is similar to previous generations, but the register fields are
> widened, the depth is made configurable, and the enable bit moves
> to a different spot.

looking at the driver it would be great if you could provide a
get_state() callback (or even convert to the waveform callbacks) and
fix:

	static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
				    int duty_ns, int period_ns)
	{
		...
	}

	static int tegra_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
				   const struct pwm_state *state)
	{
		...
		err = tegra_pwm_config(chip, pwm, state->duty_cycle, state->period);
		...
	}

where state->duty_cycle and state->period are u64 and thus big values
are not passed correctly to tegra_pwm_config().

The former helps a lot for testing the driver, and the latter for fixing
the fallout that you then will probably notice :-)

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 0/5] Tegra264 PWM support

[Mikko Perttunen]

On Wednesday, March 25, 2026 1:45 AM Uwe Kleine-König wrote:
> Hello Mikko,
> 
> On Mon, Mar 23, 2026 at 11:36:36AM +0900, Mikko Perttunen wrote:
> > this adds support for the PWM controller on Tegra264. The controller
> > is similar to previous generations, but the register fields are
> > widened, the depth is made configurable, and the enable bit moves
> > to a different spot.
> 
> looking at the driver it would be great if you could provide a
> get_state() callback (or even convert to the waveform callbacks) and
> fix:
> 
> 	static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device 
*pwm,
> 				    int duty_ns, int period_ns)
> 	{
> 		...
> 	}
> 
> 	static int tegra_pwm_apply(struct pwm_chip *chip, struct pwm_device 
*pwm,
> 				   const struct pwm_state *state)
> 	{
> 		...
> 		err = tegra_pwm_config(chip, pwm, state->duty_cycle, state-
>period);
> 		...
> 	}
> 
> where state->duty_cycle and state->period are u64 and thus big values
> are not passed correctly to tegra_pwm_config().
> 
> The former helps a lot for testing the driver, and the latter for fixing
> the fallout that you then will probably notice :-)

I agree, there are certainly some improvements to be done here. I was planning 
to do some refactoring as part of the followup (adding support for the 
configurable depth value), so if you're OK with it I'll fix those then. For 
now this basic support is needed to make it possible to keep the board with 
its fan/jet engine in the same room.. :)

FWIW, I'm also considering adding some KUnit tests for the duty/scale/depth 
configuration. Please let me know if you have any thoughts on that.

Thank you
Mikko

> 
> Best regards
> Uwe

Re: [PATCH 1/5] pwm: tegra: Avoid hard-coded max clock frequency

[Mikko Perttunen]

On Wednesday, March 25, 2026 1:45 AM Uwe Kleine-König wrote:
> On Mon, Mar 23, 2026 at 11:36:37AM +0900, Mikko Perttunen wrote:
> > From: Yi-Wei Wang <yiweiw@nvidia.com>
> > 
> > The clock driving the Tegra PWM IP can be sourced from different parent
> > clocks. Hence, let dev_pm_opp_set_rate() set the max clock rate based
> > upon the current parent clock that can be specified via device-tree.
> > 
> > After this, the Tegra194 SoC data becomes redundant, so get rid of it.
> > 
> > Signed-off-by: Yi-Wei Wang <yiweiw@nvidia.com>
> > Co-developed-by: Mikko Perttunen <mperttunen@nvidia.com>
> > Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
> > ---
> > 
> >  drivers/pwm/pwm-tegra.c | 16 +++-------------
> >  1 file changed, 3 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/pwm/pwm-tegra.c b/drivers/pwm/pwm-tegra.c
> > index 172063b51d44..759b98b97b6e 100644
> > --- a/drivers/pwm/pwm-tegra.c
> > +++ b/drivers/pwm/pwm-tegra.c
> > @@ -59,9 +59,6 @@
> > 
> >  struct tegra_pwm_soc {
> >  
> >  	unsigned int num_channels;
> > 
> > -
> > -	/* Maximum IP frequency for given SoCs */
> > -	unsigned long max_frequency;
> > 
> >  };
> >  
> >  struct tegra_pwm_chip {
> > 
> > @@ -303,7 +300,7 @@ static int tegra_pwm_probe(struct platform_device
> > *pdev)> 
> >  		return ret;
> >  	
> >  	/* Set maximum frequency of the IP */
> > 
> > -	ret = dev_pm_opp_set_rate(&pdev->dev, pc->soc->max_frequency);
> > +	ret = dev_pm_opp_set_rate(&pdev->dev, S64_MAX);
> 
> The documentation comment for dev_pm_opp_set_rate() reads:
> 
> 	Device wanting to run at fmax provided by the opp, should have
> 	already rounded to the target OPP's frequency.
> 
> I think using S64_MAX is technically fine (assuming there are no issues
> with big numbers in that function), but still it feels wrong to use
> something simpler than the comment suggests. Am I missing something?

Looking at the history of the function, the comment was added in the commit 
below. It seems like it used to be that the opp framework always used the fmax 
of each OPP even if a lower rate was specified, but after the change, the 
caller has to specify the fmax rate if they want that rate specifically. As 
such I don't think it should be an issue in our case, as we're just using the 
rate to find an OPP and don't have a specific one in mind.

commit b3e3759ee4abd72bedbf4b109ff1749d3aea6f21
Author: Stephen Boyd <swboyd@chromium.org>
Date:   Wed Mar 20 15:19:08 2019 +0530

    opp: Don't overwrite rounded clk rate
    
    The OPP table normally contains 'fmax' values corresponding to the
    voltage or performance levels of each OPP, but we don't necessarily want
    all the devices to run at fmax all the time. Running at fmax makes sense
    for devices like CPU/GPU, which have a finite amount of work to do and
    since a specific amount of energy is consumed at an OPP, its better to
    run at the highest possible frequency for that voltage value.
    
    On the other hand, we have IO devices which need to run at specific
    frequencies only for their proper functioning, instead of maximum
    possible frequency.
    
    The OPP core currently roundup to the next possible OPP for a frequency
    and select the fmax value. To support the IO devices by the OPP core,
    lets do the roundup to fetch the voltage or performance state values,
    but not use the OPP frequency value. Rather use the value returned by
    clk_round_rate().
    
    The current user, cpufreq, of dev_pm_opp_set_rate() already does the
    rounding to the next OPP before calling this routine and it won't
    have any side affects because of this change.
    
    Signed-off-by: Stephen Boyd <swboyd@chromium.org>
    Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
    [ Viresh: Massaged changelog, added comment and use temp_opp variable
              instead ]
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>


> 
> >  	if (ret < 0) {
> >  	
> >  		dev_err(&pdev->dev, "Failed to set max frequency: %d\n", 
ret);
> >  		goto put_pm;
> > 
> > @@ -318,7 +315,7 @@ static int tegra_pwm_probe(struct platform_device
> > *pdev)> 
> >  	/* Set minimum limit of PWM period for the IP */
> >  	pc->min_period_ns =
> > 
> > -	    (NSEC_PER_SEC / (pc->soc->max_frequency >> PWM_DUTY_WIDTH)) + 1;
> > +	    (NSEC_PER_SEC / (pc->clk_rate >> PWM_DUTY_WIDTH)) + 1;
> 
> Orthogonal to this patch: Should this be
> 
> 	DIV_ROUND_UP(NSEC_PER_SEC, pc->clk_rate >> PWM_DUTY_WIDTH)
> 
> ? Or even
> 
> 	DIV_ROUND_UP(NSEC_PER_SEC < PWM_DUTY_WIDTH, pc->clk_rate);
> 
> ? (Note, the latter doesn't work as is, as the first parameter has an
> overflow, I guess you're still getting my question.)

Indeed, it would be overestimating the minimum period right now. It's not 
quite part of Tegra264 support but I can include a patch in the next revision 
if you'd like. Otherwise I could include it in the followup series or as a 
separate patch.

> 
> Best regards
> Uwe

Thanks!
Mikko

Re: [PATCH 1/5] pwm: tegra: Avoid hard-coded max clock frequency

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 4525 bytes --]

[ Adding OPP maintainers to Cc: ]

Helle Mikko,

On Wed, Mar 25, 2026 at 09:34:55AM +0900, Mikko Perttunen wrote:
> On Wednesday, March 25, 2026 1:45 AM Uwe Kleine-König wrote:
> > On Mon, Mar 23, 2026 at 11:36:37AM +0900, Mikko Perttunen wrote:
> > > @@ -303,7 +300,7 @@ static int tegra_pwm_probe(struct platform_device
> > > *pdev)> 
> > >  		return ret;
> > >  	
> > >  	/* Set maximum frequency of the IP */
> > > 
> > > -	ret = dev_pm_opp_set_rate(&pdev->dev, pc->soc->max_frequency);
> > > +	ret = dev_pm_opp_set_rate(&pdev->dev, S64_MAX);
> > 
> > The documentation comment for dev_pm_opp_set_rate() reads:
> > 
> > 	Device wanting to run at fmax provided by the opp, should have
> > 	already rounded to the target OPP's frequency.
> > 
> > I think using S64_MAX is technically fine (assuming there are no issues
> > with big numbers in that function), but still it feels wrong to use
> > something simpler than the comment suggests. Am I missing something?
> 
> Looking at the history of the function, the comment was added in the commit 
> below. It seems like it used to be that the opp framework always used the fmax 
> of each OPP even if a lower rate was specified, but after the change, the 
> caller has to specify the fmax rate if they want that rate specifically. As 
> such I don't think it should be an issue in our case, as we're just using the 
> rate to find an OPP and don't have a specific one in mind.
> 
> commit b3e3759ee4abd72bedbf4b109ff1749d3aea6f21
> Author: Stephen Boyd <swboyd@chromium.org>
> Date:   Wed Mar 20 15:19:08 2019 +0530
> 
>     opp: Don't overwrite rounded clk rate
>     
>     The OPP table normally contains 'fmax' values corresponding to the
>     voltage or performance levels of each OPP, but we don't necessarily want
>     all the devices to run at fmax all the time. Running at fmax makes sense
>     for devices like CPU/GPU, which have a finite amount of work to do and
>     since a specific amount of energy is consumed at an OPP, its better to
>     run at the highest possible frequency for that voltage value.
>     
>     On the other hand, we have IO devices which need to run at specific
>     frequencies only for their proper functioning, instead of maximum
>     possible frequency.
>     
>     The OPP core currently roundup to the next possible OPP for a frequency
>     and select the fmax value. To support the IO devices by the OPP core,
>     lets do the roundup to fetch the voltage or performance state values,
>     but not use the OPP frequency value. Rather use the value returned by
>     clk_round_rate().
>     
>     The current user, cpufreq, of dev_pm_opp_set_rate() already does the
>     rounding to the next OPP before calling this routine and it won't
>     have any side affects because of this change.
>     
>     Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>     Signed-off-by: Rajendra Nayak <rnayak@codeaurora.org>
>     [ Viresh: Massaged changelog, added comment and use temp_opp variable
>               instead ]
>     Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

So the comment describing dev_pm_opp_set_rate() needs an update, right?

> > >  	if (ret < 0) {
> > >  	
> > >  		dev_err(&pdev->dev, "Failed to set max frequency: %d\n", 
> ret);
> > >  		goto put_pm;
> > > 
> > > @@ -318,7 +315,7 @@ static int tegra_pwm_probe(struct platform_device
> > > *pdev)> 
> > >  	/* Set minimum limit of PWM period for the IP */
> > >  	pc->min_period_ns =
> > > 
> > > -	    (NSEC_PER_SEC / (pc->soc->max_frequency >> PWM_DUTY_WIDTH)) + 1;
> > > +	    (NSEC_PER_SEC / (pc->clk_rate >> PWM_DUTY_WIDTH)) + 1;
> > 
> > Orthogonal to this patch: Should this be
> > 
> > 	DIV_ROUND_UP(NSEC_PER_SEC, pc->clk_rate >> PWM_DUTY_WIDTH)
> > 
> > ? Or even
> > 
> > 	DIV_ROUND_UP(NSEC_PER_SEC < PWM_DUTY_WIDTH, pc->clk_rate);
> > 
> > ? (Note, the latter doesn't work as is, as the first parameter has an
> > overflow, I guess you're still getting my question.)
> 
> Indeed, it would be overestimating the minimum period right now. It's not 
> quite part of Tegra264 support but I can include a patch in the next revision 
> if you'd like. Otherwise I could include it in the followup series or as a 
> separate patch.

If you know it and feel responsible to address it at some point that's
fine. We lived with that issue for some time now, so a separate and if
you prefer later series is fine for me.

Best regards
Uwe

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 1/5] pwm: tegra: Avoid hard-coded max clock frequency

[Viresh Kumar]

On 25-03-26, 07:12, Uwe Kleine-König wrote:
> On Wed, Mar 25, 2026 at 09:34:55AM +0900, Mikko Perttunen wrote:
> > On Wednesday, March 25, 2026 1:45 AM Uwe Kleine-König wrote:
> > > On Mon, Mar 23, 2026 at 11:36:37AM +0900, Mikko Perttunen wrote:
> > > > @@ -303,7 +300,7 @@ static int tegra_pwm_probe(struct platform_device
> > > > *pdev)> 
> > > >  		return ret;
> > > >  	
> > > >  	/* Set maximum frequency of the IP */
> > > > 
> > > > -	ret = dev_pm_opp_set_rate(&pdev->dev, pc->soc->max_frequency);
> > > > +	ret = dev_pm_opp_set_rate(&pdev->dev, S64_MAX);
> > > 
> > > The documentation comment for dev_pm_opp_set_rate() reads:
> > > 
> > > 	Device wanting to run at fmax provided by the opp, should have
> > > 	already rounded to the target OPP's frequency.

And that is correct, right ? This comment is talking about the max freq possible
with each OPP and not the highest freq possible with the device. If a device
supports 5 OPPs (0-4) and if we want to run at the freq mentioned in the OPP3
entry in DT, then the caller must send a frequency such that clk_round_rate()
returns the frequency of the OPP3.

In the above case though, we will end up running at the highest freq returned
by clk_round_rate() and an OPP corresponding to that.

> > > I think using S64_MAX is technically fine (assuming there are no issues
> > > with big numbers in that function), but still it feels wrong to use
> > > something simpler than the comment suggests. Am I missing something?

I think S64_MAX will work as well, unless clk_round_rate() returns a frequency
higher than what the OPP table mentions. It may still work, but the values may
be confusing and inconsistent.

> > Looking at the history of the function, the comment was added in the commit 
> > below. It seems like it used to be that the opp framework always used the fmax 
> > of each OPP even if a lower rate was specified, but after the change, the 
> > caller has to specify the fmax rate if they want that rate specifically. As 
> > such I don't think it should be an issue in our case, as we're just using the 
> > rate to find an OPP and don't have a specific one in mind.

Right.

> So the comment describing dev_pm_opp_set_rate() needs an update, right?

Maybe, not sure. But as I mentioned earlier, it is written with the context of
each OPP's highest freq.

-- 
viresh