All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mikko Perttunen <mperttunen@nvidia.com>
To: "Thierry Reding" <thierry.reding@kernel.org>,
	"Uwe Kleine-König" <u.kleine-koenig@baylibre.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	linux-pwm@vger.kernel.org, linux-tegra@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	"Ola Chr. Vaage" <ola.christoffer.vage@scoutdi.com>
Subject: Re: [PATCH v2 3/3] pwm: tegra: Implement .get_state()
Date: Mon, 05 Oct 2026 00:33:14 +0900	[thread overview]
Message-ID: <oG8AGa5CQQi_5H_Ter5mEA@nvidia.com> (raw)
In-Reply-To: <ar9vT4FY3QxEvRZH@monoceros>

On Friday, October 2, 2026 5:51 PM Uwe Kleine-König wrote:
> Hello Thierry,
> 
> On Wed, Sep 30, 2026 at 12:29:42PM +0200, Thierry Reding wrote:
> > On Wed, Sep 30, 2026 at 11:54:03AM +0200, Uwe Kleine-König wrote:
> > > On Tue, Sep 22, 2026 at 12:07:26PM +0200, Thierry Reding wrote:
> > > > On Mon, Sep 21, 2026 at 04:26:03PM +0200, Uwe Kleine-König wrote:
> > > > > As long as .apply() also hardcodes TEGRA_PWM_DEPTH, it's IMO fine that
> > > > > .get_state() does so, too.
> > > > 
> > > > Okay, fair enough.
> > > 
> > > Is that an Ack then?
> > 
> > I've been thinking about this some more and I don't know if it really
> > makes sense to keep hard-coding TEGRA_PWM_DEPTH.
> 
> Full ack, ideally this implementation gap would be closed. Compared to
> implementing .get_state() I don't feel confident to do that without
> testing though. (Though I could make the driver return an error code if
> the register setting doesn't match.)
> 
> > If only .apply() uses it, then it's mostly fine, I suppose, because we
> > don't care what the current (or initial) state is/was. So we either
> > don't use the device or we overwrite it with a custom set of values.
> 
> I don't agree here. If the TEGRA_PWM_DEPTH setting is different in
> hardware than the driver assumes, I'd say .apply() being wrong is worse
> than .get_state() being wrong. So I'd either go with .get_state() as it
> is now, or rely on someone with hardware to correct the depth setting
> first.
> 
> Best regards
> Uwe

My state of mind when posting the Tegra264 support series was that the 
driver -- for the time being -- intentionally relies on the DEPTH 
register being at reset value. As such I think it's fine to rely on that 
everywhere for now. Once we add support for setting the register at 
runtime, I think it makes sense to update all code paths at that time.

(Of course, I had intended to follow up with those patches sooner, but 
..)

Mikko




  reply	other threads:[~2026-10-04 15:33 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-18 14:33 [PATCH v2 0/3] pwm: tegra: Cleanups and .get_state() Uwe Kleine-König
2026-09-18 14:33 ` [PATCH v2 1/3] pwm: tegra: Make use of dev_err_probe() Uwe Kleine-König
2026-09-21  9:38   ` Thierry Reding
2026-09-21 12:46     ` Uwe Kleine-König
2026-09-21 16:14       ` Thierry Reding
2026-10-02  8:45         ` Uwe Kleine-König
2026-09-18 14:33 ` [PATCH v2 2/3] pwm: tegra: Check for match_data being NULL Uwe Kleine-König
2026-09-21  9:47   ` Thierry Reding
2026-09-21 14:34     ` Uwe Kleine-König
2026-09-21 16:24       ` Thierry Reding
2026-09-21 20:10         ` Uwe Kleine-König
2026-09-22  8:33         ` Uwe Kleine-König
2026-09-22 10:06           ` Thierry Reding
2026-09-30 11:55             ` Uwe Kleine-König
2026-09-30 13:14               ` Thierry Reding
2026-09-30 17:05                 ` Uwe Kleine-König
2026-09-18 14:33 ` [PATCH v2 3/3] pwm: tegra: Implement .get_state() Uwe Kleine-König
2026-09-21 10:18   ` Thierry Reding
2026-09-21 14:26     ` Uwe Kleine-König
2026-09-22 10:07       ` Thierry Reding
2026-09-30  9:54         ` Uwe Kleine-König
2026-09-30 10:29           ` Thierry Reding
2026-10-02  8:51             ` Uwe Kleine-König
2026-10-04 15:33               ` Mikko Perttunen [this message]
2026-09-21 10:22 ` [PATCH v2 0/3] pwm: tegra: Cleanups and .get_state() Thierry Reding

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=oG8AGa5CQQi_5H_Ter5mEA@nvidia.com \
    --to=mperttunen@nvidia.com \
    --cc=jonathanh@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pwm@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=ola.christoffer.vage@scoutdi.com \
    --cc=p.zabel@pengutronix.de \
    --cc=thierry.reding@kernel.org \
    --cc=u.kleine-koenig@baylibre.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.