Hi Walleij, Thanks for the review. On Fri, Feb 24, 2023 at 11:53:08AM +0100, Linus Walleij wrote: > Hi Ye, > > thanks for your patch! > > On Sun, Feb 19, 2023 at 7:31 PM Ye Xiang <xiang.ye@xxxxxxxxx> wrote: > > > Add sysfs attributes Documentation entries for LJCA device > > > > Signed-off-by: Ye Xiang <xiang.ye@xxxxxxxxx> > (...) > > +What: /sys/bus/usb/.../cmd > > +Date: July 2023 > > +KernelVersion: 6.4 > > +Contact: Ye Xiang <xiang.ye@xxxxxxxxx> > > +Description: > > + Commands supported by LJCA device. > > + When read, it will return valid commands. > > + When write with a command, it will execute the command. > > + Valid commands are [dfu, reset, debug] > > + dfu: Force LJCA device to enter DFU mode. > > + reset: Trigger soft reset for LJCA device. > > + debug: Enable debug logging. > > Given that there are kernel drivers for this device, it looks pretty > dangerous to make it possible for userspace to reset the device? Agree. I would just remove the reset cmd on the next version. User access LJCA will fail during reset because the reset cmd could cause LJCA down for a short time. > > But maybe it will re-enumerate when you do this so all drivers > unload cleanly and then re-probe? No, I haven't implemented the re-probe yet. It seems quite complicated to make sure all drivers depend on LJCA re-init. So, I would just stop exporting the reset interface to userspace. > > I guess the DFU mode will use the USB standard class for updating > the firmware? Yes. > > Perhaps a short blurb on the use case for each string could be > helpful, like "echo dfu to this file so as to put the device into > DFU mode so the firmware can be updated". Thanks will add this blurb. > > Is the idea that e.g. fwupdmgr should provide a front-end for this? We haven't implemented a front-end in fwupdmgr. dfu-util is used to update LJCA firmware manually currently. Maybe we will consider implementing this in fwupdmgr later. > > Yours, > Linus Walleij -- Thanks Ye Xiang
