linux-kernel - Re: [PATCH v3 2/2] pwm: add T-HEAD PWM driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 13 Nov 2023 22:56:02 +0100
From:   Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
To:     Jisheng Zhang <jszhang@...nel.org>
Cc:     Thierry Reding <thierry.reding@...il.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Guo Ren <guoren@...nel.org>, Fu Wei <wefu@...hat.com>,
        linux-pwm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v3 2/2] pwm: add T-HEAD PWM driver

Hello,

On Thu, Oct 05, 2023 at 09:05:19PM +0800, Jisheng Zhang wrote:
> diff --git a/drivers/pwm/pwm-thead.c b/drivers/pwm/pwm-thead.c
> new file mode 100644
> index 000000000000..3b9ffddfe33d
> --- /dev/null
> +++ b/drivers/pwm/pwm-thead.c
> @@ -0,0 +1,270 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * T-HEAD PWM driver
> + *
> + * Copyright (C) 2021 Alibaba Group Holding Limited.
> + * Copyright (C) 2023 Jisheng Zhang <jszhang@...nel.org>
> + *
> + * Limitations:
> + * - The THEAD_PWM_CTRL_START bit is only effective when 0 -> 1, which is used
> + *   to start the channel, 1 -> 0 doesn't change anything. so 0 % duty cycle
> + *   is used to "disable" the channel.
> + * - The THEAD_PWM_CTRL_START bit is automatically cleared once PWM channel is
> + *   started.
> + * - The THEAD_PWM_CFG_UPDATE atomically updates and only updates period and duty.
> + * - To update period and duty, THEAD_PWM_CFG_UPDATE needs to go through 0 -> 1
> + *   step, I.E if THEAD_PWM_CFG_UPDATE is already 1, it's necessary to clear it
> + *   to 0 beforehand.
> + * - Polarity can only be changed if never started before.

Questions I'd like to have answered here additionally are:

 - How does the hardware behave when it's disabled? Typical canditates
   are:
    - drives the inactive level
    - freezes where it just happens to be when disable gets effective
    - disable drivers (HIGH-Z)
    - drives low

 - Does the hardware complete the current running period when apply is
   called with enabled=false?

 - Does the hardware complete the current running period when apply is
   called with enabled=true?

> +static const struct dev_pm_ops thead_pwm_pm_ops = {
> +	SET_RUNTIME_PM_OPS(thead_pwm_runtime_suspend, thead_pwm_runtime_resume, NULL)
> +};

Please see
https://lore.kernel.org/linux-pwm/20231023174616.2282067-13-u.kleine-koenig@pengutronix.de/t/
how this can be improved.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)