Re: [PATCH 4/11] LED: Add LED Timer Trigger

[Richard Purdie]

On Tue, 2006-01-31 at 16:01 +0100, Jan-Benedict Glaw wrote:
> On Tue, 2006-01-31 13:41:37 +0000, Richard Purdie <rpurdie@xxxxxxxxx> wrote:
> > +static void led_timer_setdata(struct led_device *led_dev, unsigned long duty, unsigned long frequency)
> > +{
> > +	struct timer_trig_data *timer_data = led_dev->trigger_data;
> > +	signed long duty1;
> > +
> > +	if (frequency > 500)
> > +		frequency = 500;
> 
> Why? 

We're dealing with msec delays. Any frequency > 1000 will just cause
problems. There was a reason for using half that but it escapes me and
might be unneeded now. 500Hz/1000Hz is above the frequency the human eye
can see so is unlikely to present a problem.

> ...and especially: why, without complaining?

This is the important bit. It should return an -EINVAL back to
userspace.

> > +	if (duty > 100)
> > +		duty = 100;
> 
> Dito.

Duty cycles > 100 make no sense and would break the subsequent
calculation. Same problem/solution as above.

Thanks,

Richard


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/