Re: [PATCH v6] gpio: pca953x: Add Maxim MAX7313 PWM support - Uwe Kleine-König

From: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
To: Andy Shevchenko <andy.shevchenko@gmail.com>
Cc: Miquel Raynal <miquel.raynal@bootlin.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>,
	"open list:GPIO SUBSYSTEM" <linux-gpio@vger.kernel.org>,
	linux-pwm@vger.kernel.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v6] gpio: pca953x: Add Maxim MAX7313 PWM support
Date: Sun, 3 May 2020 21:41:26 +0200	[thread overview]
Message-ID: <20200503194126.wlwuide5q2oxzlwt@pengutronix.de> (raw)
In-Reply-To: <CAHp75Vcg9kdgr=AXxwmXO-rmL5z9nq=zJvCww8wG7i1B3BQNYg@mail.gmail.com>

Hello Andy, hello Miquel,

On Sun, May 03, 2020 at 10:20:23PM +0300, Andy Shevchenko wrote:
> On Sun, May 3, 2020 at 1:54 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > The MAX7313 chip is fully compatible with the PCA9535 on its basic
> > functions but can also manage the intensity on each of its ports with
> > PWM. Each output is independent and may be tuned with 16 values (4
> > bits per output). The period is always 32kHz, only the duty-cycle may
> > be changed. One can use any output as GPIO or PWM.
> 
> Besides the messing with parameter types (int vs. bool) it should be
> rebased on top of Bartosz's tree.
> 
> Also, it might be that we can instantiate a kind of device (MFD?) that
> will share same regmap between two and have naturally different
> drivers for GPIO and PWM.
> 
> Side note: I still think this should be a function of the pin when
> driver will be converted to pin control. Now this change delays the
> conversion and better approach. But as I said before, if GPIO
> maintainers consider this good enough to go like this, I won't object.
> 
> Some background. It's known that some pin control devices may have PWM
> function (limited, like blinking led or so, or full) and it would be
> nice to have a pin mux option which enables PWM on a requested pin. Or
> PWM tries to enable proper pin muxing (this sounds even better in
> order of sharing same API from pin control, like "pin is available for
> GPIO").

There is one thing I see as a pre-condition to the abstraction of
pin-controller + GPIO + PWM: If I configure the PWM with certain
parameters while the pin is still configured as GPIO there must be no
visible effect on the pin. The setting must then be active when the pin
is changed to PWM mode. (And vice versa: The GPIO setting must not
influence the PWM output ...) I didn't check the hardware manual (or the
patch), but if this needs caching of pwm and gpio parameters I would
take that as a strong hint that the abstraction is wrong.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |