Date:   Sat, 25 Feb 2023 02:01:20 +0800
From:   "Ye, Xiang" <xiang.ye@intel.com>
To:     Linus Walleij <linus.walleij@linaro.org>
CC:     Lee Jones <lee@kernel.org>, Wolfram Sang <wsa@kernel.org>,
        Tyrone Ting <kfting@nuvoton.com>,
        Mark Brown <broonie@kernel.org>,
        Bartosz Golaszewski <brgl@bgdev.pl>,
        <linux-i2c@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
        <linux-spi@vger.kernel.org>, <linux-gpio@vger.kernel.org>,
        <srinivas.pandruvada@intel.com>, <heikki.krogerus@linux.intel.com>,
        <andriy.shevchenko@linux.intel.com>,
        <sakari.ailus@linux.intel.com>, <zhifeng.wang@intel.com>,
        <wentong.wu@intel.com>, <lixu.zhang@intel.com>
Subject: Re: [PATCH 5/5] Documentation: Add ABI doc for attributes of LJCA
 device
Message-ID: <Y/j7cAQaaCXXYe6s@ye-NUC7i7DNHE>
References: <20230219183059.1029525-1-xiang.ye@intel.com>
 <20230219183059.1029525-6-xiang.ye@intel.com>
 <CACRpkdbAve++nA0zwHvOm3fy0t9J9g0fR_FO71TTv=TwM6CJYA@mail.gmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <CACRpkdbAve++nA0zwHvOm3fy0t9J9g0fR_FO71TTv=TwM6CJYA@mail.gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?YXOSurLEF8WdZ3PWIC1lsxh+4v+Hamc24wSgDYplXAy/vJPW/QXgQXLY1y7N?=
 =?us-ascii?Q?Rmmcfsv6yLBSP7h6MBTPDyhPvrP0+93JRWDDxsls3/8hfKBf/s1Qj0kMEw/5?=
 =?us-ascii?Q?yEwWUixlh8kNkWZrsKo9tUkM7uUF63OFY19sW/P37hndfAkhkW6QQ4xT1AKP?=
 =?us-ascii?Q?LH4L3CA/CuxDCZh5IBUUBnj2aOSdlZAG/b6goNF/kKx9PLK6IhyIlkE/bpJv?=
 =?us-ascii?Q?MFjUB8RTJc6hOJMaciHta0Ys4UlLaHVKOIkWDlkCc3tuRo0aUaq+n98dWBRm?=
 =?us-ascii?Q?pzScdt7XxQaQspkgHJA3vQzMCUuU0zPzlWNNOCxRwpzMXM3wEoRIIeoi0v79?=
 =?us-ascii?Q?G7ja4XRye8ZtueWigvR2CuYG0hMlspWkK41UFk98qoGls4y/AoVbMrpLWntu?=
 =?us-ascii?Q?szO0QIHZvlw8K63Et392Or7SAAETGY319aXBQMNWDm34pwL0I9LYcgm7yD23?=
 =?us-ascii?Q?FNI44L+Gz2EgvVGJ1k44ep1AGkqpCbV2ZiYRZuCSHK/dNuIlBYIMyeNHtMpB?=
 =?us-ascii?Q?UPriidqiy8wZ0DfJLgWdIXkBE9ih9OzO+3uYVeoQopdOEO9QyBmwfRN4GLRU?=
 =?us-ascii?Q?O68KWNf0x88l8NrEhBQlLYC7CjlEKH4fqNu112/BwrqoSL/5ai5BwYw85b8H?=
 =?us-ascii?Q?rcwK26A9Fe8z8Bxl2WRJ1j3eZng2BBA2CPqTxTk+mdbQfJ1OUNvmArmBwsi2?=
 =?us-ascii?Q?M/ZIxoD1WXeTGOJn1wHynKOHanft67NcOSPOHZh/hgUuXamGrPj/Ge419pu3?=
 =?us-ascii?Q?HfI9yqjtterC1YzqNE82JBy9l+LunojO8ew/MLlBzTktuVWq4iqPJP8qzalq?=
 =?us-ascii?Q?hho65K1Rnt/mg9BZTM306Ymeg3uOSbz+M5XHPXubvb6msoune53IiCUW3y/2?=
 =?us-ascii?Q?4UUBDId7oWC6jrBCvSDhpJDbFBacwCvz81p/LWIEpBYvFJUEZsxLAV+PX0NB?=
 =?us-ascii?Q?0FCgb9ZjVYszpsCj6WXL318LmSe3zYZPZN+0lvT3R92tSey6PIgu1CNUcwPc?=
 =?us-ascii?Q?cf6o2M9dfNkrwptQq4VeezQkO3wfnSiRgFVNPqLB6IbFkmHX2W4mVVkujsow?=
 =?us-ascii?Q?jECRs+XwIBX1AuTwPuuC6fAuzRAwfYPTz8Q8zHc0kJh0Y+tsimiZamLkK54b?=
 =?us-ascii?Q?1KRVt2a2Bu7W50Wc53sGQj0K43GG+mx0WPWp16v9uG6lt/NH7y59Xl/o1zZb?=
 =?us-ascii?Q?e8C+Dw0ymu1DKb9XHEYjAYPMUD3gNgARnNxmA/4GsZtP1q0WynJN4PVOGiNC?=
 =?us-ascii?Q?B3WJ8k/mOZLS8+JliWxuw72GALTSnTuVgT03h9hyVRc7duC7AZx2alwk6dsS?=
 =?us-ascii?Q?d43PRTFkypbgloc9DOLwe331cjMc1a58BsVhnleNRoJYqJKEwc3ovWIZBR52?=
 =?us-ascii?Q?U7Hz1RSZiwQTmBFW7yRyN+UBmqEK/ny70CEsG8oHeoZ/SMpRxl6dQBC97A51?=
 =?us-ascii?Q?hEyuodsMluKnST9oSKfg8ZuDzYBu1Ble1F/n/bgqMSVuVaO2EnJeJxqCrTCs?=
 =?us-ascii?Q?c4tYtTphr3+QlQU+f693uCMlimpN9FzU9VgrRFVAUh6ZxYHTDlIgFPmCe67a?=
 =?us-ascii?Q?73KPat4pDTTqCq0F9HMwi69uVeFliucmC24HqJPT?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 63c8c0b7-8404-4f05-bb16-08db1691284d
X-MS-Exchange-CrossTenant-AuthSource: DM5PR11MB1418.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Feb 2023 18:01:30.7496
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Ft6N4eOY76e3YVEiSgD1buCZ3KtN0lZR27sAwJU4ZEP09Fb7YPhztRatfCtihfrXoXmUv2l1M6MYqVZYDGolHA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB5019
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

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