From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
Cc: Rob Herring <robh+dt@kernel.org>,
Thierry Reding <thierry.reding@gmail.com>,
Lee Jones <lee.jones@linaro.org>,
devicetree@vger.kernel.org, linux-pwm@vger.kernel.org,
punit1.agrawal@toshiba.co.jp, yuji2.ishikawa@toshiba.co.jp,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 2/2] pwm: visconti: Add Toshiba Visconti SoC PWM support
Date: Sun, 18 Apr 2021 15:44:11 +0200 [thread overview]
Message-ID: <20210418134411.vfltokielrwuygqa@pengutronix.de> (raw)
In-Reply-To: <20210418110904.1942806-3-nobuhiro1.iwamatsu@toshiba.co.jp>
[-- Attachment #1: Type: text/plain, Size: 2544 bytes --]
Hello,
just a few smaller issues left to fix.
On Sun, Apr 18, 2021 at 08:09:04PM +0900, Nobuhiro Iwamatsu wrote:
> diff --git a/drivers/pwm/pwm-visconti.c b/drivers/pwm/pwm-visconti.c
> new file mode 100644
> index 000000000000..166b18ac1a3a
> --- /dev/null
> +++ b/drivers/pwm/pwm-visconti.c
> @@ -0,0 +1,188 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Toshiba Visconti pulse-width-modulation controller driver
> + *
> + * Copyright (c) 2020 TOSHIBA CORPORATION
> + * Copyright (c) 2020 Toshiba Electronic Devices & Storage Corporation
We're in 2021, so you might want to adapt the year in the copy right
notice.
> + *
> + * Authors: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>
> + *
> + * Limitations:
> + * - PIPGM_PWMC is a 2-bit divider (00: 1, 01: 2, 10: 4, 11: 8).
This is too detailed for the purpose of this section. Please either drop
it or make this:
- The fixed input clock is running at 1 MHz and is divided by either 1,
2, 4 or 8.
> + * - Fixed input clock running at 1 MHz.
> + * - When the settings of the PWM are modified, the new values are shadowed
> + * in hardware until the PIPGM_PCSR register is written and the currently
> + * running period is completed. This way the hardware switches atomically
> + * from the old setting to the new.
> + * - Disabling the hardware completes the currently running period and keeps
> + * the output at low level at all times.
> + */
> +
> [...]
> + /*
> + * PWMC controls a divider that divides the input clk by a
> + * power of two between 1 and 8. As a smaller divider yields
> + * higher precision, pick the smallest possible one.
> + */
> + if (period > 0xffff) {
> + pwmc0 = ilog2(period >> 16);
> + BUG_ON(pwmc0 > 3);
> + } else
> + pwmc0 = 0;
The linux coding style mandates that you should use braces for both
branches. (i.e.
+ if (period > 0xffff) {
+ pwmc0 = ilog2(period >> 16);
+ BUG_ON(pwmc0 > 3);
+ } else {
+ pwmc0 = 0;
+ }
)
> + period >>= pwmc0;
> + duty_cycle >>= pwmc0;
> +
> + if (state->polarity == PWM_POLARITY_INVERSED)
> + pwmc0 |= PIPGM_PWMC_PWMACT;
> + writel(pwmc0, priv->base + PIPGM_PWMC(pwm->hwpwm));
> + writel(duty_cycle, priv->base + PIPGM_PDUT(pwm->hwpwm));
> + writel(period, priv->base + PIPGM_PCSR(pwm->hwpwm));
> +
> + return 0;
> +}
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2021-04-18 13:44 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-18 11:09 [PATCH v5 0/2] pwm: visconti: Add Toshiba Visconti SoC PWM support Nobuhiro Iwamatsu
2021-04-18 11:09 ` [PATCH v5 1/2] dt-bindings: pwm: Add bindings for Toshiba Visconti PWM Controller Nobuhiro Iwamatsu
2021-04-18 11:09 ` [PATCH v5 2/2] pwm: visconti: Add Toshiba Visconti SoC PWM support Nobuhiro Iwamatsu
2021-04-18 13:44 ` Uwe Kleine-König [this message]
2021-04-18 23:20 ` Nobuhiro Iwamatsu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210418134411.vfltokielrwuygqa@pengutronix.de \
--to=u.kleine-koenig@pengutronix.de \
--cc=devicetree@vger.kernel.org \
--cc=lee.jones@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pwm@vger.kernel.org \
--cc=nobuhiro1.iwamatsu@toshiba.co.jp \
--cc=punit1.agrawal@toshiba.co.jp \
--cc=robh+dt@kernel.org \
--cc=thierry.reding@gmail.com \
--cc=yuji2.ishikawa@toshiba.co.jp \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).