RE: [PATCH v16] pwm: opencores: Add PWM driver support

[William Qiu]

> -----Original Message-----
> From: Uwe Kleine-König <ukleinek@xxxxxxxxxx>
> Sent: 2024年12月27日 16:38
> To: William Qiu <william.qiu@xxxxxxxxxxxxxxxx>
> Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-pwm@xxxxxxxxxxxxxxx; Hal Feng
> <hal.feng@xxxxxxxxxxxxxxxx>; Philipp Zabel <p.zabel@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH v16] pwm: opencores: Add PWM driver support
> 
> On Tue, Dec 24, 2024 at 01:53:03AM +0000, William Qiu wrote:
> > > -----Original Message-----
> > > From: Uwe Kleine-König <ukleinek@xxxxxxxxxx>
> > > Sent: 2024年12月24日 1:12
> > > To: William Qiu <william.qiu@xxxxxxxxxxxxxxxx>
> > > Cc: linux-kernel@xxxxxxxxxxxxxxx; linux-pwm@xxxxxxxxxxxxxxx; Hal
> > > Feng <hal.feng@xxxxxxxxxxxxxxxx>; Philipp Zabel
> > > <p.zabel@xxxxxxxxxxxxxx>
> > > Subject: Re: [PATCH v16] pwm: opencores: Add PWM driver support
> > >
> > > Hello William,
> > >
> > > On Mon, Dec 23, 2024 at 02:47:59AM +0000, William Qiu wrote:
> > > > > > +	duty_data = mul_u64_u32_div(state->duty_cycle,
> > > > > > +ddata->clk_rate,
> > > > > NSEC_PER_SEC);
> > > > > > +	if (!duty_data)
> > > > > > +		return -EINVAL;
> > > > >
> > > > > I can understand that period_data == 0 is an error, but
> > > > > duty_data ==
> > > > > 0 could/should just work?!
> > > >
> > > > It means no need to check whether the duty is valid?
> > >
> > > No, it means that I expect that duty_data == 0 is a valid setting
> > > and most controllers support it.
> > >
> > > Best regards
> > > Uwe
> >
> > So we just need to check duty < 0?
> 
> I'm not sure I understand this question. You can assume that
> state->duty_cycle is >= 0 in the driver callback.
> 
> If the hardware doesn't support duty_cycle == 0, add a comment and proper
> error handling. (i.e. return with -EINVAL if the requested duty_cycle is too low
> to be implemented.)
> 
> Best regards
> Uwe

I see. I'll check it and send v17 soon.

Thanks,
William