Jean Tourrilhes <[EMAIL PROTECTED]> writes:
> Hi everybody...
>
> A few weeks ago, I was complaining that the timming in the
> Actisys dongle driver in Linux IrDA were absurd. Today I received my
> batch of Actisys dongle, and guess what, I could not make it work with
> Linux IrDA.
> So, I wiped out the initialisation code and the code to change
> speed in the old driver and replaced it with something simpler, faster
> and more robust. I have to thanks Lichen Wang from Actisys who sent me
Well, I'm not so sure it's more robust ;-)
> a nice e-mail detailing the secret of their dongle.
> Anyway, the result is a new dongle driver that work for me.
> Enjoy...
>
> static int actisys_change_speed(struct irda_task *task)
> {
> dongle_t *self = (dongle_t *) task->instance;
> __u32 speed = (__u32) task->param; /* Target speed */
> int index = 0;
> int ret = 0;
>
> IRDA_DEBUG(4, __FUNCTION__ "(), speed=%d (was %d)\n",
> speed, self->speed);
>
> /* Go to a known state by reseting the dongle */
>
> /* Reset the dongle : set DTR low for 10 us */
> self->set_dtr_rts(self->dev, FALSE, TRUE);
> udelay(MIN_DELAY);
>
> /* Go back to normal mode (we are now at 9600 b/s) */
> self->set_dtr_rts(self->dev, TRUE, TRUE);
>
> /* Now, we can set the speed requested */
>
> /* Send RTS pulses until we reach the target speed */
> while((index < 5) && (speed != baud_rates[index++]))
I think it's very dangerous to increase the index here! Anyway, the index
will be one higher than the current baudrate when this loop is finished.
> {
> /* Make sure previous pulse is finished */
> udelay(MIN_DELAY);
>
> /* Set RTS low for 10 us */
> self->set_dtr_rts(self->dev, TRUE, FALSE);
> udelay(MIN_DELAY);
>
> /* Set RTS high for 10 us */
> self->set_dtr_rts(self->dev, TRUE, TRUE);
> }
>
> /* Check if life is sweet... */
> if(speed != baud_rates[index])
... and that is very dangerous here, if index becomes 5 (which it can be if
something goes wrong), you will index outside the array. Well, I cannot
find any reason why this should match at all, so I'm confused that you
actually got this code to work!? You find a match, but increase the index
at the same time, which means that the index is pointing to the next
entry (which should not match).
> ret = -1; /* This should not happen */
> self->speed = baud_rates[index]; /* Do we care ? */
>
> /* Basta lavoro, on se casse d'ici... */
> irda_task_next_state(task, IRDA_TASK_DONE);
>
> return ret;
> }
I think it would be better (and safer) to do it like this:
/* Send RTS pulses until we reach the target speed */
for (i=0; i<5; i++) {
if (speed == baud_rates[i]) {
self->speed = baud_rates[i];
break;
}
/* Make sure previous pulse is finished */
udelay(MIN_DELAY);
/* Set RTS low for 10 us */
self->set_dtr_rts(self->dev, TRUE, FALSE);
udelay(MIN_DELAY);
/* Set RTS high for 10 us */
self->set_dtr_rts(self->dev, TRUE, TRUE);
}
/* Check if life is sweet... */
if (i == 5)
ret = -1; /* This should not happen */
I have fixed this in my code, and I'll be releasing a new patch later ...
-- Dag
--
/ Dag Brattli | The Linux-IrDA Project /
// University of Tromsoe, Norway | Infrared communication for Linux //
/// http://www.cs.uit.no/~dagb | http://www.cs.uit.no/linux-irda/ ///
_______________________________________________
Linux-IrDA mailing list - [EMAIL PROTECTED]
http://www4.pasta.cs.UiT.No/mailman/listinfo/linux-irda
