Re: [PATCH v4 4/4] pwm: meson: make full use of common clock framework

From: Heiner Kallweit <hkallweit1@gmail.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: neil.armstrong@linaro.org, "Jerome Brunet" <jbrunet@baylibre.com>,
	"Neil Armstrong" <narmstrong@baylibre.com>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"thierry.reding@gmail.com" <thierry.reding@gmail.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"open list:ARM/Amlogic Meson..."
	<linux-amlogic@lists.infradead.org>,
	linux-pwm@vger.kernel.org
Subject: Re: [PATCH v4 4/4] pwm: meson: make full use of common clock framework
Date: Mon, 1 May 2023 15:39:45 +0200	[thread overview]
Message-ID: <7cacf0f4-099c-766d-5757-cc3813ec09a8@gmail.com> (raw)
In-Reply-To: <CAFBinCAF4oc+FoG8CtQhpSHSAkODQFXGbt5OvtprGSb4s+fWqg@mail.gmail.com>

On 23.04.2023 22:58, Martin Blumenstingl wrote:
> On Wed, Apr 19, 2023 at 9:58 PM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> [...]
>>> This is a hack based on current clock values, either explicitly support a code path
>>> where pre_div = 0 or if you can't do that with CCF implement the pinctrl way to handle this,
>>> which is the cleanest.
>>>
>> To make it explicit we could request ULONG_MAX as rate instead of 1GHz, this would imply
>> choosing mux parent with highest rate and pre_div = 0. Up to you whether this would be
>> acceptable.
> I like the idea of using ULONG_MAX as I first had to think about why
> you chose 1GHz in the driver.
> 
>> AFAICS pinctrl would need quite some DTS changes, and it's not my area of expertise.
>> So it would be open who can implement this.
> My opinion is that this can be done in a separate patch. We need to
> work on this whole thing anyways as you mentioned that newer SoCs
> (from what I understand: G12A onwards) have a dedicated "constant
> output" bit which will make the pinctrl solution unnecessary (at least
> based on how I understand it).
> 
Agree.
My understanding of the "constant output" bit is, based on the vendor driver:
W/o this bit the chip internally increments the lo and hi value. Not sure by the way
how the chip handles value 0xffff, whether it omits the increment in this case.
W/ this bit set the chip doesn't increment the values, therefore lo / hi can be
effectively zero.

> 
> Best regards,
> Martin

Heiner

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel