Re: [PATCH v4 6/7] pwm: tegra: Add support for Tegra264

[Thierry Reding]

On Tue, Mar 31, 2026 at 11:12:18AM +0900, Mikko Perttunen wrote:
> 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@xxxxxxxxxx>
> Signed-off-by: Yi-Wei Wang <yiweiw@xxxxxxxxxx>
> Signed-off-by: Mikko Perttunen <mperttunen@xxxxxxxxxx>
> ---
>  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 d7968521fbfd..c9d30724e339 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@xxxxxxxxxxxxxx>
>   *
> - * 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.

This paragraph exceeds the 80 character limit. Technically checkpatch
now has a limit of 100 characters, so it probably doesn't warn about
this, but I've seen some people say that we should still stay within
the 80 character limit if easily doable (which would be the case here).

I don't care much either way, so it's ultimately up to Uwe. Other than
that looks good:

Reviewed-by: Thierry Reding <treding@xxxxxxxxxx>

Attachment: signature.asc
Description: PGP signature