Re: [PATCH 02/10] pwm: Add SI-EN SN3112 PWM support

Uwe Kleine-König
2024-04-25T06:15:03Z

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

Hello,

On Wed, Apr 24, 2024 at 09:37:25PM +0200, Konrad Dybcio wrote:
> On 4/24/24 17:29, Xilin Wu via B4 Relay wrote:
> > +
> > + /* use random value to apply changes */
> > + ret = sn3112_write_reg(priv, SN3112_REG_APPLY, 0x66);
>
> "a random value"? sounds suspicious..

I smiled about that one, too, remembering https://xkcd.com/221/

> [...]
> > +#if IS_ENABLED(CONFIG_GPIOLIB)
> > + /* enable hardware shutdown pin */
> > + if (priv->sdb)
> > + gpiod_set_value(priv->sdb, 1);
> > +#endif
> > +
> > + /* power-off sn5112 power vdd */
> > + regulator_disable(priv->vdd);
> > +
> > + pwmchip_remove(chip);
>
> devm_pwmchip_add?

Note using devm_xyz only works if all requests before are also using
devm. (There are a few exceptions, but these need proper thinking and
extensive commenting.)

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