Hi Edouard,
On 6/1/05, Edouard TISSERANT <[EMAIL PROTECTED]> wrote:
> Hi all.
>
> Patch should be now OK.
>
> It should fit to last Dimitry's requests.
>
Yes, it is much better now. I still have couple of concerns though:
> + acecad->dev.absmin[ABS_MISC] = 0;
> + acecad->dev.absmax[ABS_MISC] = 4294967295U;
What this is for? As far as I can see Acecad does not report any MISC
events, why does it claim to have ABS_MISC?
> +static ssize_t show_tabletSize(struct device *dev, char *buf)
> +{
> + struct usb_acecad *acecad = dev_get_drvdata(dev);
> +
> + if (acecad == NULL)
> + return 0;
This check - is it really needed? Attributes are deleted in
disconnect... Wait, this needs locking because if someone happens to
read one of the attributes while usb_acecad_disconnect is running you
may end up accessing just freed memory, no?
> +static struct attribute *acecad_attrs[] = {
> + &dev_attr_size.attr,
> + &dev_attr_product_id.attr,
> + &dev_attr_vendor_id.attr,
> + &dev_attr_vendor.attr,
> + &dev_attr_product.attr,
You know, I have been looking at these attributes and wondered if it
would be better if we'd just let them go... We have numeric vendor ID
and product ID data exported in /proc/bus/input/devices and it will be
also exported within /sys/class/input_dev - uniformly - so tools can
pick it from the standard place with a standard name. Also userland
(X) drivers should probably just query /dev/eventX devices with
EVIOCGID to get entire input_id structure and latch onto device(s)
they support. Is there any tools/users that rely on character
representation of vendor string? That leaves up with tablet size.
Again, is there any users or is was mostly a debug thing? We can have
2 options here - have drivers use EVIOCGABS to query devices or again
attempt to export it at input_dev class level. What do you think?
> + acecad->dev.name = acecad->name;
> + acecad->dev.phys = acecad->phys;
> + acecad->dev.id.bustype = BUS_USB;
> + acecad->dev.id.vendor = dev->descriptor.idVendor;
> + acecad->dev.id.product = dev->descriptor.idProduct;
> + acecad->dev.id.version = dev->descriptor.bcdDevice;
It also needs:
acecad->dev.dev = &intf->dev;
so there is a ling from /sys/class/input/eventX -> /sys/bus/usb/devices/.....
Please making these changes and I will add the driver to my tree and
hopefully Vojtech and Andrew will pick it from there.
--
Dmitry
-------------------------------------------------------
This SF.Net email is sponsored by Yahoo.
Introducing Yahoo! Search Developer Network - Create apps using Yahoo!
Search APIs Find out how you can build Yahoo! directly into your own
Applications - visit http://developer.yahoo.net/?fr=offad-ysdn-ostg-q22005
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
