Hello, On Mon, Jul 29, 2019 at 02:43:23PM +0800, Chen-Yu Tsai wrote: > On Mon, Jul 29, 2019 at 2:36 PM Uwe Kleine-König > <u.kleine-koenig@xxxxxxxxxxxxxx> wrote: > > On Fri, Jul 26, 2019 at 08:40:41PM +0200, Jernej Skrabec wrote: > > > @@ -371,6 +374,14 @@ static int sun4i_pwm_probe(struct platform_device *pdev) > > > if (IS_ERR(pwm->clk)) > > > return PTR_ERR(pwm->clk); > > > > > > + if (pwm->data->has_reset) { > > > + pwm->rst = devm_reset_control_get(&pdev->dev, NULL); > > > + if (IS_ERR(pwm->rst)) > > > + return PTR_ERR(pwm->rst); > > > + > > > + reset_control_deassert(pwm->rst); > > > + } > > > + > > > > I wonder why there is a need to track if a given chip needs a reset > > line. I'd just use devm_reset_control_get_optional() and drop the > > .has_reset member in struct sun4i_pwm_data. > > Because it's not optional for this platform, i.e. it won't work if > the reset control (or clk, in the next patch) is somehow missing from > the device tree. If the device tree is wrong it is considered ok that the driver doesn't behave correctly. So this is not a problem you need (or should) care about. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |