linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Hans de Goede <hdegoede@redhat.com>
Cc: "Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Jani Nikula" <jani.nikula@linux.intel.com>,
	"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
	"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
	"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
	"Rafael J . Wysocki" <rjw@rjwysocki.net>,
	"Len Brown" <lenb@kernel.org>,
	linux-pwm@vger.kernel.org,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	dri-devel@lists.freedesktop.org,
	"Mika Westerberg" <mika.westerberg@linux.intel.com>,
	linux-acpi@vger.kernel.org
Subject: Re: [PATCH v4 06/16] pwm: lpss: Correct get_state result for base_unit == 0
Date: Sat, 11 Jul 2020 08:11:20 +0200	[thread overview]
Message-ID: <20200711061120.di53sk5utjerb72q@pengutronix.de> (raw)
In-Reply-To: <af7158c5-02bb-38ce-4b22-e469079dcccf@redhat.com>

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

On Thu, Jul 09, 2020 at 05:47:59PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 7/9/20 4:50 PM, Andy Shevchenko wrote:
> > On Wed, Jul 08, 2020 at 11:14:22PM +0200, Hans de Goede wrote:
> > > The datasheet specifies that programming the base_unit part of the
> > > ctrl register to 0 results in a contineous low signal.
> > > 
> > > Adjust the get_state method to reflect this by setting pwm_state.period
> > > to 1 and duty_cycle to 0.
> > 
> > ...
> > 
> > > +	if (freq == 0) {
> > > +		/* In this case the PWM outputs a continous low signal */
> > 
> > > +		state->period = 1;
> > 
> > I guess this should be something like half of the range (so base unit calc
> > will give 128). Because with period = 1 (too small) it will give too small
> > base unit (if apply) and as a result we get high frequency pulses.
> 
> You are right, that if after this the user only changes the duty-cycle
> things will work very poorly, we will end up with a base_unit value of
> e.g 65535 and then have almost no duty-cycle resolution at all.

Is this a problem of the consumer that we don't need to solve? Are there
known consumers running into this problem?

pwm_lpss_prepare() is buggy here, a request for a too low period should be
refused.

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 --]

  reply	other threads:[~2020-07-11  6:11 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-08 21:14 [PATCH v4 00/15] acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API Hans de Goede
2020-07-08 21:14 ` [PATCH v4 01/16] ACPI / LPSS: Resume Cherry Trail PWM controller in no-irq phase Hans de Goede
2020-07-08 21:14 ` [PATCH v4 02/16] ACPI / LPSS: Save Cherry Trail PWM ctx registers only once (at activation) Hans de Goede
2020-07-08 21:14 ` [PATCH v4 03/16] pwm: lpss: Fix off by one error in base_unit math in pwm_lpss_prepare() Hans de Goede
2020-07-08 21:14 ` [PATCH v4 04/16] pwm: lpss: Add range limit check for the base_unit register value Hans de Goede
2020-07-09 12:53   ` Andy Shevchenko
2020-07-09 13:23     ` Hans de Goede
2020-07-09 14:21       ` Andy Shevchenko
2020-07-09 14:33         ` Hans de Goede
2020-07-09 14:51           ` Andy Shevchenko
2020-07-08 21:14 ` [PATCH v4 05/16] pwm: lpss: Use pwm_lpss_apply() when restoring state on resume Hans de Goede
2020-07-09 13:36   ` Andy Shevchenko
2020-07-09 13:48     ` Hans de Goede
2020-07-08 21:14 ` [PATCH v4 06/16] pwm: lpss: Correct get_state result for base_unit == 0 Hans de Goede
2020-07-09 14:50   ` Andy Shevchenko
2020-07-09 15:47     ` Hans de Goede
2020-07-11  6:11       ` Uwe Kleine-König [this message]
2020-07-11 13:58         ` Hans de Goede
2020-07-08 21:14 ` [PATCH v4 07/16] pwm: crc: Fix period / duty_cycle times being off by a factor of 256 Hans de Goede
2020-07-08 21:14 ` [PATCH v4 08/16] pwm: crc: Fix off-by-one error in the clock-divider calculations Hans de Goede
2020-07-08 21:14 ` [PATCH v4 09/16] pwm: crc: Fix period changes not having any effect Hans de Goede
2020-07-08 21:14 ` [PATCH v4 10/16] pwm: crc: Enable/disable PWM output on enable/disable Hans de Goede
2020-07-08 21:14 ` [PATCH v4 11/16] pwm: crc: Implement apply() method to support the new atomic PWM API Hans de Goede
2020-07-08 21:14 ` [PATCH v4 12/16] pwm: crc: Implement get_state() method Hans de Goede
2020-07-08 21:14 ` [PATCH v4 13/16] drm/i915: panel: Add get_vbt_pwm_freq() helper Hans de Goede
2020-07-08 21:14 ` [PATCH v4 14/16] drm/i915: panel: Honor the VBT PWM frequency for devs with an external PWM controller Hans de Goede
2020-07-08 21:14 ` [PATCH v4 15/16] drm/i915: panel: Honor the VBT PWM min setting " Hans de Goede
2020-07-08 21:14 ` [PATCH v4 16/16] drm/i915: panel: Use atomic PWM API " Hans de Goede
2020-07-11  6:32   ` Uwe Kleine-König
2020-07-11 13:51     ` Hans de Goede
2020-07-09 14:14 ` [PATCH v4 00/15] acpi/pwm/i915: Convert pwm-crc and i915 driver's PWM code to use the atomic PWM API Sam Ravnborg
2020-07-09 14:40   ` Hans de Goede
2020-07-09 15:23     ` Sam Ravnborg
2020-07-11  6:19     ` Uwe Kleine-König
2020-07-11 13:46       ` Hans de Goede

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=20200711061120.di53sk5utjerb72q@pengutronix.de \
    --to=u.kleine-koenig@pengutronix.de \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=hdegoede@redhat.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@linux.intel.com \
    --cc=joonas.lahtinen@linux.intel.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=mika.westerberg@linux.intel.com \
    --cc=rjw@rjwysocki.net \
    --cc=rodrigo.vivi@intel.com \
    --cc=thierry.reding@gmail.com \
    --cc=ville.syrjala@linux.intel.com \
    /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).