diff mbox series

[5/5] Documentation: Add ABI doc for attributes of LJCA device

Message ID 20230219183059.1029525-6-xiang.ye@intel.com
State Superseded
Headers show
Series Add Intel LJCA device driver | expand

Commit Message

Ye Xiang Feb. 19, 2023, 6:30 p.m. UTC
Add sysfs attributes Documentation entries for LJCA device

Signed-off-by: Ye Xiang <xiang.ye@intel.com>
---
 .../ABI/testing/sysfs-bus-usb-devices-ljca    | 22 +++++++++++++++++++
 1 file changed, 22 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-usb-devices-ljca

Comments

Linus Walleij Feb. 24, 2023, 10:53 a.m. UTC | #1
Hi Ye,

thanks for your patch!

On Sun, Feb 19, 2023 at 7:31 PM Ye Xiang <xiang.ye@intel.com> wrote:

> Add sysfs attributes Documentation entries for LJCA device
>
> Signed-off-by: Ye Xiang <xiang.ye@intel.com>
(...)
> +What:          /sys/bus/usb/.../cmd
> +Date:          July 2023
> +KernelVersion: 6.4
> +Contact:       Ye Xiang <xiang.ye@intel.com>
> +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?

But maybe it will re-enumerate when you do this so all drivers
unload cleanly and then re-probe?

I guess the DFU mode will use the USB standard class for updating
the firmware?

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".

Is the idea that e.g. fwupdmgr should provide a front-end for this?

Yours,
Linus Walleij
Ye Xiang Feb. 24, 2023, 6:01 p.m. UTC | #2
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@intel.com> wrote:
> 
> > Add sysfs attributes Documentation entries for LJCA device
> >
> > Signed-off-by: Ye Xiang <xiang.ye@intel.com>
> (...)
> > +What:          /sys/bus/usb/.../cmd
> > +Date:          July 2023
> > +KernelVersion: 6.4
> > +Contact:       Ye Xiang <xiang.ye@intel.com>
> > +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
Linus Walleij March 6, 2023, 2 p.m. UTC | #3
On Fri, Feb 24, 2023 at 7:02 PM Ye, Xiang <xiang.ye@intel.com> wrote:
> On Fri, Feb 24, 2023 at 11:53:08AM +0100, Linus Walleij wrote:

> > 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.

It's a matter of process rather than implementation really, but if you can
please communicate upward to the project that if this is to go into
consumer hands (i.e. be used by random people on random laptops)
the firmware update should come via fwupdmgr.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/Documentation/ABI/testing/sysfs-bus-usb-devices-ljca b/Documentation/ABI/testing/sysfs-bus-usb-devices-ljca
new file mode 100644
index 000000000000..ae270c02b66c
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-usb-devices-ljca
@@ -0,0 +1,22 @@ 
+What:		/sys/bus/usb/.../version
+Date:		July 2023
+KernelVersion:	6.4
+Contact:	Ye Xiang <xiang.ye@intel.com>
+Description:
+		Provides the current firmware version of LJCA device.
+		The format is Major.Minor.Patch.Build, where
+		Major, Minor, Patch, and Build are decimal numbers.
+		For example: 1.0.0.256
+
+What:		/sys/bus/usb/.../cmd
+Date:		July 2023
+KernelVersion:	6.4
+Contact:	Ye Xiang <xiang.ye@intel.com>
+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.