Linux-Hwmon Archive on lore.kernel.org
 help / color / Atom feed
From: Michael Walle <michael@walle.cc>
To: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Cc: Thierry Reding <thierry.reding@gmail.com>,
	linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org,
	linux-pwm@vger.kernel.org, linux-watchdog@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	Rob Herring <robh+dt@kernel.org>,
	Jean Delvare <jdelvare@suse.com>,
	Guenter Roeck <linux@roeck-us.net>,
	Lee Jones <lee.jones@linaro.org>,
	Wim Van Sebroeck <wim@linux-watchdog.org>,
	Shawn Guo <shawnguo@kernel.org>, Li Yang <leoyang.li@nxp.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Jason Cooper <jason@lakedaemon.net>,
	Marc Zyngier <maz@kernel.org>, Mark Brown <broonie@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: Re: [PATCH v5 07/13] pwm: add support for sl28cpld PWM controller
Date: Wed, 15 Jul 2020 22:41:25 +0200
Message-ID: <8debe0ee9aff2c49a7567069d7bb9477@walle.cc> (raw)
In-Reply-To: <20200715181803.nmgi32tugpbuqvjg@pengutronix.de>

Hi Uwe,

Am 2020-07-15 20:18, schrieb Uwe Kleine-König:
> On Wed, Jul 15, 2020 at 07:45:10PM +0200, Michael Walle wrote:
>> 
>> Am 2020-07-15 18:36, schrieb Uwe Kleine-König:
>> > On Tue, Jul 14, 2020 at 11:09:28PM +0200, Michael Walle wrote:
>> > > > My wishlist (just as it comes to my mind, so no guarantee of
>> > > > completeness):
>> > > >
>> > > >  - can do 0% duty cycle for all supported period lengths
>> > > >  - can do 100% duty cycle for all supported period lengths
>> > > >  - supports both polarities
>> > > >  - supports immediate change of configuration and after completion of
>> > > >    the currently running period
>> > > >  - atomic update (i.e. if you go from configuration A to configuration B
>> > > >    the hardware guarantees to only emit periods of type A and then type
>> > > >    B. (Depending on the item above, the last A period might be cut off.)
>> > >
>> > > We actually discussed this, because the implementation would be
>> > > easier. But
>> > > if the change takes place immediately you might end up with a longer
>> > > duty
>> > > cycle. Assume the PWM runs at 80% duty cycle and starts with the
>> > > on-period.
>> > > If you now change that to 50% you might end up with one successive
>> > > duty
>> > > cycle of "130%". Eg. the 80% of the old and right after that you
>> > > switch to
>> > > the new 50% and then you'd have a high output which corresponds to a
>> > > 130%
>> > > cycle. I don't know if that is acceptable for all applications.
>> >
>> > I thought this is a "change takes place immediately" implementation?! So
>> > these problems are actually real here. (And this not happening is
>> > exactly
>> > my wish here. Is there a mis-understanding?)
>> 
>> I wasn't talking about the sl28cpld btw. What is the difference 
>> between
>> your proposed "change take place immediately" and "after the cycle".
>> I understand how the after the cycle should work. But how would the
>> immediate change work in your ideal PWM?
> 
> If the PWM is running at 1/3 duty cycle and reconfigured for 2/3, then
> the two scenarios are (the * marks the moment where pwm_apply_state() 
> is
> called, ^ marks the start of a period):
> 
> immediately:
> 
>   __       __    _____    _____
>  /  \_____/  \__/     \__/
>  ^        ^     ^        ^
>                 *

Ok lets assume 2/3 and change it to 1/3:

    ____     ______      __
   /    \___/      \____/  \____
   ^        ^   ^       ^
                *
This will then have a longer on period than any of the settings.

> and with my ideal PWM I can choose which of the two behaviours I want.

Ahh, that I've missed.

>> > > > > > What about disable()?
>> > > > >
>> > > > > Mhh well, it would do one 100% cycle.. mhh ;) Lets see if there we can
>> > > > > fix that (in hardware), not much we can do in the driver here. We are
>> > > > > _very_ constraint in size, therefore all that little edge cases fall
>> > > > > off
>> > > > > the table.
>> > > >
>> > > > You're saying that on disable the hardware emits a constant high level
>> > > > for one cycle? I hope not ...
>> > >
>> > > Mh, I was mistaken, disabling the PWM will turn it off immediately,
>> > > but
>> >
>> > And does turn off mean, the output gets inactive?
>> > If so you might also disable the hardware if a 0% duty cycle is
>> > configured assuming this saves some energy without modifying the
>> > resulting wave form.
>> 
>> Disabling it has some side effects like switching to another function
>> for this multi function pin. So I'd rather keep it on ;)
> 
> So IMHO you should also keep it on when pwm_apply_state is called with
> state.enabled = false to ensure a low output.

That won't work either, because that is how you would turn on that multi
function. Ie. it is GPIO (default input) as long as the PWM is not 
enabled,
otherwise its PWM.

-michael

  reply index

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-06 17:53 [PATCH v5 00/13] Add support for Kontron sl28cpld Michael Walle
2020-07-06 17:53 ` [PATCH v5 01/13] regmap-irq: use fwnode instead of device node in add_irq_chip() Michael Walle
2020-07-08 10:22   ` Mark Brown
2020-07-06 17:53 ` [PATCH v5 02/13] mfd: add simple regmap based I2C driver Michael Walle
2020-07-17  9:04   ` Lee Jones
2020-07-19 22:25     ` Michael Walle
2020-07-17  9:06   ` Lee Jones
2020-07-19 22:31     ` Michael Walle
2020-07-20  7:43       ` Lee Jones
2020-07-06 17:53 ` [PATCH v5 03/13] dt-bindings: mfd: Add bindings for sl28cpld Michael Walle
2020-07-13 16:04   ` Rob Herring
2020-07-06 17:53 ` [PATCH v5 04/13] mfd: simple-mfd-i2c: add sl28cpld support Michael Walle
2020-07-06 17:53 ` [PATCH v5 05/13] irqchip: add sl28cpld interrupt controller support Michael Walle
2020-07-08  9:37   ` Marc Zyngier
2020-07-06 17:53 ` [PATCH v5 06/13] watchdog: add support for sl28cpld watchdog Michael Walle
2020-07-06 17:53 ` [PATCH v5 07/13] pwm: add support for sl28cpld PWM controller Michael Walle
2020-07-09  8:50   ` Uwe Kleine-König
2020-07-11 17:28     ` Michael Walle
2020-07-13  8:47       ` Uwe Kleine-König
2020-07-14 11:31         ` Michael Walle
2020-07-14 16:08           ` Uwe Kleine-König
2020-07-14 21:09             ` Michael Walle
2020-07-15 16:36               ` Uwe Kleine-König
2020-07-15 17:45                 ` Michael Walle
2020-07-15 18:18                   ` Uwe Kleine-König
2020-07-15 20:41                     ` Michael Walle [this message]
2020-07-16  6:10                       ` Uwe Kleine-König
2020-07-06 17:53 ` [PATCH v5 08/13] gpio: add support for the sl28cpld GPIO controller Michael Walle
2020-07-08  7:35   ` Linus Walleij
2020-07-06 17:53 ` [PATCH v5 09/13] hwmon: add support for the sl28cpld hardware monitoring controller Michael Walle
2020-07-06 17:53 ` [PATCH v5 10/13] arm64: dts: freescale: sl28: enable sl28cpld Michael Walle
2020-07-06 17:53 ` [PATCH v5 11/13] arm64: dts: freescale: sl28: map GPIOs to input events Michael Walle
2020-07-06 17:53 ` [PATCH v5 12/13] arm64: dts: freescale: sl28: enable LED support Michael Walle
2020-07-17  8:36   ` Pavel Machek
2020-07-17 21:54     ` Michael Walle
2020-07-06 17:53 ` [PATCH v5 13/13] arm64: dts: freescale: sl28: enable fan support Michael Walle
2020-07-08  7:39   ` Linus Walleij
2020-07-08  7:56     ` Michael Walle

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=8debe0ee9aff2c49a7567069d7bb9477@walle.cc \
    --to=michael@walle.cc \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jason@lakedaemon.net \
    --cc=jdelvare@suse.com \
    --cc=lee.jones@linaro.org \
    --cc=leoyang.li@nxp.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=maz@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=shawnguo@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=thierry.reding@gmail.com \
    --cc=u.kleine-koenig@pengutronix.de \
    --cc=wim@linux-watchdog.org \
    /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

Linux-Hwmon Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-hwmon/0 linux-hwmon/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-hwmon linux-hwmon/ https://lore.kernel.org/linux-hwmon \
		linux-hwmon@vger.kernel.org
	public-inbox-index linux-hwmon

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-hwmon


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git