On Sun, 8 Jan 2006 21:50:21 -0500
Dmitry Torokhov <[email protected]> wrote:
Hi,
I will reply in two different emails as I need
to do some research on some questions of yours.
> > + if (down_trylock(&rtc->char_sem))
> > + return -EBUSY;
> > +
>
> Does the device have to be opened for exclusively? Can it support
> concurrent reads?
I'm trying to make the things work the same way as with the old
interface. Once this new code will be as stable as the old one, new
features can be added.
Concurrent reads are certainly possible, but we'd need to lock
out writes in proper places, and I do not feel safe
to do it at this stage.
[..]
> > + ret = -ERESTARTSYS;
> > + break;
> > + }
> > + schedule();
> > + } while (1);
> > + set_current_state(TASK_RUNNING);
> > + remove_wait_queue(&rtc->irq_queue, &wait);
> > +
>
>
> The above looks very much like open-coded wait_event_interruptible();
Ditto, plus there's the irq data inside
> > + kobject_hotplug(&class_dev->kobj, KOBJ_ADD);
> > +
>
> This is kobject_hotplug abuse; you are not adding a new object here.
Yes. But I've checked the code and should not harm. I've asked
for that a couple of weeks ago in this same mailing list
and got no answer. If there's a working alternative to obtain
the same result, I'm obviously willing to try.
> > +/* interface registration */
> > +
> > +struct class_interface rtc_dev_interface = {
> > + .add = &rtc_dev_add_device,
> > + .remove = &rtc_dev_remove_device,
> > +};
> > +
>
> I wonder if doing rtc dev as a class device interface is a good idea.
> It may be cleaner to fold it into the core.
What the code implements is actually an interface, so this should
be the riht place. It is also fully optional, everything could work
without it. Probably the interface implementation hasn't all the primitives
to handle this kind of work, but I'm not willing to go into that right now ;)
Thank you for your hard work Dmitry, after weeks viewing the same
code I'm going to be lost in all of those locks issues ;)
--
Best regards,
Alessandro Zummo,
Tower Technologies - Turin, Italy
http://www.towertech.it
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
