linux-pwm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thierry Reding <thierry.reding@gmail.com>
To: Jeff LaBundy <jeff@labundy.com>
Cc: u.kleine-koenig@pengutronix.de, linux-pwm@vger.kernel.org
Subject: Re: [RESEND v5] pwm: Add support for Azoteq IQS620A PWM generator
Date: Tue, 14 Apr 2020 16:10:10 +0200	[thread overview]
Message-ID: <20200414141010.GD3593749@ulmo> (raw)
In-Reply-To: <1586744116-14157-1-git-send-email-jeff@labundy.com>

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

On Sun, Apr 12, 2020 at 09:15:16PM -0500, Jeff LaBundy wrote:
> This patch adds support for the Azoteq IQS620A, capable of generating
> a 1-kHz PWM output with duty cycle between ~0.4% and 100% (inclusive).
> 
> Signed-off-by: Jeff LaBundy <jeff@labundy.com>
> Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Note: This patch was originally part of a series that was intended to be taken
>       through mfd in its entirety. However, attempts to elicit an Ack for this
>       patch went unanswered, so it was dropped in order to unblock the series.
> 
>       The remainder of the series (including pwm bindings) landed in 5.7-rc1,
>       so this patch is being sent on its own so that it may be taken through
>       pwm during the next merge window.
> 
> Changes in v5:
>   - Replaced iqs62x->map with iqs62x->regmap throughout
>   - Updated iqs620_pwm_probe to read the values of IQS620_PWR_SETTINGS_PWM_OUT
>     and IQS620_PWM_DUTY_CYCLE so that iqs620_pwm_get_state reflects the actual
>     state of the device following any changes made by a bootloader
>   - Dropped #defines for platform_driver name and alias in favor of the actual
>     string names
> 
> Changes in v4:
>   - Updated iqs620_pwm_apply and iqs620_pwm_get_state to hold the lock around
>     only the minimum necessary code
>   - Removed the completion protecting against early use of chip->pwms[0] from
>     inside iqs620_pwm_notifier in favor of cached register values
>   - Updated iqs620_pwm_get_state to use the cached register values instead of
>     reading IQS620_PWR_SETTINGS_PWM_OUT and IQS620_PWM_DUTY_CYCLE (both equal
>     to zero by default)
>   - Added a comment in iqs620_pwm_notifier to note that the parent MFD driver
>     prints an error message in the event of a device reset
> 
> Changes in v3:
>   - Updated the commit message to say "~0.4%" instead of "0.4%"
>   - Clarified the effect of duty cycle and state changes in the 'Limitations'
>     section and added a restriction regarding 0% duty cycle
>   - Added a comment in iqs620_pwm_apply to explain how duty cycle is derived
>   - Updated iqs620_pwm_apply to disable the output first and enable it last to
>     prevent temporarily driving a stale duty cycle
>   - Rounded the calculation for duty cycle up and down in iqs620_pwm_get_state
>     and iqs620_pwm_apply, respectively
>   - Added a comment in iqs620_pwm_get_state to explain what it reports follow-
>     ing requests to set duty cycle to 0%
>   - Added a lock to prevent back-to-back access of IQS620_PWR_SETTINGS_PWM_OUT
>     and IQS620_PWM_DUTY_CYCLE from being interrupted
>   - Updated iqs620_pwm_notifier to reference pwm->state directly as opposed to
>     calling pwm_get_state
>   - Moved notifier unregistration back to a device-managed action
>   - Added a completion to prevent iqs620_pwm_notifier from referencing the
>     pwm_chip structure until it has been initialized by pwmchip_add
> 
> Changes in v2:
>   - Merged 'Copyright' and 'Author' lines into one in introductory comments
>   - Added 'Limitations' section to introductory comments
>   - Replaced 'error' with 'ret' throughout
>   - Added const qualifier to state argument of iqs620_pwm_apply and removed all
>     modifications to the variable's contents
>   - Updated iqs620_pwm_apply to return -ENOTSUPP or -EINVAL if the requested
>     polarity is inverted or the requested period is below 1 ms, respectively
>   - Updated iqs620_pwm_apply to disable the PWM output if duty cycle is zero
>   - Added iqs620_pwm_get_state
>   - Eliminated tabbed alignment of pwm_ops and platform_driver struct members
>   - Moved notifier unregistration to already present iqs620_pwm_remove, which
>     eliminated the need for a device-managed action and ready flag
>   - Added a comment in iqs620_pwm_probe to explain the order of operations
>   - Changed Kconfig "depends on" logic to MFD_IQS62X || COMPILE_TEST
> 
>  drivers/pwm/Kconfig       |  10 ++
>  drivers/pwm/Makefile      |   1 +
>  drivers/pwm/pwm-iqs620a.c | 270 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 281 insertions(+)
>  create mode 100644 drivers/pwm/pwm-iqs620a.c

Applied, thanks.

Thierry

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

      reply	other threads:[~2020-04-14 14:10 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-13  2:15 [RESEND v5] pwm: Add support for Azoteq IQS620A PWM generator Jeff LaBundy
2020-04-14 14:10 ` Thierry Reding [this message]

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=20200414141010.GD3593749@ulmo \
    --to=thierry.reding@gmail.com \
    --cc=jeff@labundy.com \
    --cc=linux-pwm@vger.kernel.org \
    --cc=u.kleine-koenig@pengutronix.de \
    /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).