linux-kernel - Re: [PATCH v4 03/10] pwm: max7360: Add MAX7360 PWM support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <D8IL9KSYUQZK.MB94IIL9FUL1@bootlin.com>
Date: Mon, 17 Mar 2025 14:51:30 +0100
From: "Mathieu Dubois-Briand" <mathieu.dubois-briand@...tlin.com>
To: Uwe Kleine-König <ukleinek@...nel.org>, "Linus Walleij"
 <linus.walleij@...aro.org>
Cc: "Lee Jones" <lee@...nel.org>, "Rob Herring" <robh@...nel.org>,
 "Krzysztof Kozlowski" <krzk+dt@...nel.org>, "Conor Dooley"
 <conor+dt@...nel.org>, "Kamel Bouhara" <kamel.bouhara@...tlin.com>,
 "Bartosz Golaszewski" <brgl@...ev.pl>, "Dmitry Torokhov"
 <dmitry.torokhov@...il.com>, "Michael Walle" <mwalle@...nel.org>, "Mark
 Brown" <broonie@...nel.org>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
 "Danilo Krummrich" <dakr@...nel.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <linux-gpio@...r.kernel.org>,
 <linux-input@...r.kernel.org>, <linux-pwm@...r.kernel.org>,
 <andriy.shevchenko@...el.com>, Grégory Clement
 <gregory.clement@...tlin.com>, "Thomas Petazzoni"
 <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v4 03/10] pwm: max7360: Add MAX7360 PWM support

On Thu Mar 13, 2025 at 10:03 PM CET, Uwe Kleine-König wrote:
> Hello Mathieu,
>
> On Fri, Feb 14, 2025 at 12:49:53PM +0100, mathieu.dubois-briand@...tlin.com wrote:
> > diff --git a/drivers/pwm/pwm-max7360.c b/drivers/pwm/pwm-max7360.c
> > new file mode 100644
> > index 000000000000..f1257c20add2
> ...
> > +
> > +static void max7360_pwm_free(struct pwm_chip *chip, struct pwm_device *pwm)
> > +{
> > +	struct max7360_pwm *max7360_pwm;
> > +
> > +	max7360_pwm = max7360_pwm_from_chip(chip);
> > +	max7360_port_pin_request(max7360_pwm->parent, pwm->hwpwm, false);
>
> Would be nice if pinmuxing would be abstracted as a pinctrl driver. Not
> sure how much effort that is. Maybe Linus has an idea?
>

Yes, I got some similar comments previously and I've been working on it:
the next version will gain a pinctrl driver.

> > +}
> > +
> > [...]
> > +
> > +static int max7360_pwm_write_waveform(struct pwm_chip *chip,
> > +				      struct pwm_device *pwm,
> > +				      const void *_wfhw)
> > +{
> > +	const struct max7360_pwm_waveform *wfhw = _wfhw;
> > +	struct max7360_pwm *max7360_pwm;
> > +	unsigned int val;
> > +	int ret;
> > +
> > +	max7360_pwm = max7360_pwm_from_chip(chip);
> > +
> > +	val = (wfhw->duty_steps == 0) ? 0 : MAX7360_PWM_CTRL_ENABLE(pwm->hwpwm);
>
> Does not setting MAX7360_PWM_CTRL_ENABLE result in the pin going to
> Hi-Z? If yes: That's wrong. You're only supposed to do that if
> period_length_ns = 0 was requested. If no: This needs a comment why
> duty_steps = 0 is special here.
>

Ok, I confirm this does set the pin in Hi-Z, I'm fixing it.

...

>
> Best regards
> Uwe

Thanks for your review.

-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com