Hello, sorry for late reply.
wow, there is many comments...
> But anyway, I think that in your case you don't need it. Why don't you
> update directly your sensor values when you receive the data in
> ugold_intr() instead of copying to a intermediate buffer?
The device always works command (control pipe) -> response (interrupt
pipe) cycle. I tried to replace uhidev_get_report() instead of reading
interrupt pipe, but it didn't work. (why?)
If only temperature data (as read temperature data command's response)
comes from interrupt pipe, I think it will be able to remove
intermediate buffer. But, other responses by initialize commands are
also using interrupt pipe. Is there any good idea to distinguish them?
>> + /*
>> + * interrupt pipe is opened but data comes after ugold_attach()
>> + * is finished. simply attach sensors here and the device will be
>> + * initialized at ugold_refresh().
>> + *
>> + * at this point, the number of sensors is unknown. setup maximum
>> + * sensors here and detach unused sensor later.
>> + */
>
> Why don't you initialize the device before opening the interrupt pipe?
ugold_init_device() gets the number of sensors and offset parameter,
they come from interrupt pipe. And I couldn't get any interrupt response
when issuing commands in ugold_attach().
After (exiting) ugold_attach(), I could get interrupt response.
>> +int
>> +ugold_issue_cmd(struct ugold_softc *sc, uint8_t *cmd, int len, int delay)
>> +{
>> + usb_device_request_t req;
>> +
>> + bzero(sc->sc_ibuf, sc->sc_ilen);
>> +
>> + req.bmRequestType = UT_WRITE_CLASS_INTERFACE;
>> + req.bRequest = UR_SET_REPORT;
>> + USETW(req.wValue, 0x0200);
>> + USETW(req.wIndex, 0x0001);
>> + USETW(req.wLength, len);
>> + if (usbd_do_request(sc->sc_udev, &req, cmd))
>> + return EIO;
>
> I would suggest you to have a look at uhidev_set_report{,_async}() instead of
> writing your own version here.
I think this can replace with uhidev_set_report(), I will try rewrite.
>> +int
>> +ugold_init_device(struct ugold_softc *sc)
>> +{
>> + usb_device_request_t req;
>> + usbd_status error;
>> +
>> + /* send SetReport request to another (Keyboard) interface */
>> + req.bmRequestType = UT_WRITE_CLASS_INTERFACE;
>> + req.bRequest = UR_SET_REPORT;
>> + USETW(req.wValue, 0x0201);
>> + USETW(req.wIndex, 0x0000);
>> + USETW(req.wLength, sizeof(cmd_led_off));
>> + error = usbd_do_request(sc->sc_udev, &req, cmd_led_off);
>> + if (error)
>> + return EIO;
>
> Same here, this is likely to be another uhidev_set_report() with a
> different interface, no?
Current ugold_attach() ignores keyboard interface, so sc_hdev in
ugold_softc points mouse inerface.
Maybe this SetReport will be able to replace issuing
uhidev_set_report() when detecting keyboard interface in
ugold_attach().
>> + /* init process */
>> + if (ugold_issue_cmd(sc, cmd_get_offset, sizeof(cmd_get_offset), 200))
>> + return EIO;
>> +
>> + /* received one interrupt message, it contains offset parameter */
>> + sc->sc_num_sensors = sc->sc_ibuf[1];
>
> How can you be sure sc_ibuf contains the data you asked for?
Well, sanity check will be required.
Cheers,
--
SASANO Takayoshi <[email protected]>
