From: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-can@vger.kernel.org, Marc Kleine-Budde <mkl@pengutronix.de>,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
Subject: Re: [PATCH v2 3/3] can: etas_es58x: report the firmware version through ethtool
Date: Sun, 6 Nov 2022 14:34:58 +0900 [thread overview]
Message-ID: <CAMZ6Rq+hc+YObxCAFL8Go3gvc8budzdaxLZY2b4GexcG1xHzdQ@mail.gmail.com> (raw)
In-Reply-To: <Y2cDlrNjL5YSAPm2@rowland.harvard.edu>
On Sun. 6 Nov. 2022 at 09:48, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Sat, Nov 05, 2022 at 06:38:35PM +0100, Greg Kroah-Hartman wrote:
> > On Sun, Nov 06, 2022 at 02:21:11AM +0900, Vincent MAILHOL wrote:
> > > On Sat. 5 Nov. 2022 at 18:27, Vincent MAILHOL
> > > <mailhol.vincent@wanadoo.fr> wrote:
> > > > On Sat. 5 Nov. 2022 at 17:36, Greg Kroah-Hartman
> > > > <gregkh@linuxfoundation.org> wrote:
> > It's late right now, and I can't remember the whole USB spec, but I
> > think the device provides a list of the string ids that are valid for
> > it. If so, we can add that to sysfs for any USB device out there, no
> > matter the string descriptor number.
>
> No, there is no such list.
>
> > If not, maybe we can just iterate the 255 values and populate sysfs
> > files if they are present? I'll dig up the USB spec tomorrow...
>
> Yes, we could do that. But the filename would have to be the string
> id, which is not meaningful. We wouldn't be able to have labels like
> "product-info" unless somehow a driver could provide the label.
My shot on this would be like this:
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 549590e9c644..d0a4fc3ffe07 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -77,6 +77,19 @@ struct ieee1394_device_id {
* Use the flag values to control which fields are compared.
*/
+/**
+ * struct custom_string - information of custom string and their indexes
+ * @idx: Index of the custom string descriptor.
+ * @label: Mnemotechnic, will be used as a filename for the sysfs entry.
+ *
+ * USB devices might expose some information in some customs strings. Drivers
+ * can use this structure to inform the USB core of where to find these.
+ */
+struct custom_string {
+ __u8 idx;
+ const char *label;
+};
+
/**
* struct usb_device_id - identifies USB devices for probing and hotplugging
* @match_flags: Bit mask controlling which of the other fields are used to
@@ -110,6 +123,9 @@ struct ieee1394_device_id {
* @driver_info: Holds information used by the driver. Usually it holds
* a pointer to a descriptor understood by the driver, or perhaps
* device flags.
+ * @customs_strings_table: devices using customs strings can use this table to
+ * inform the USB core of how to retrieve them. If used, must
contained an
+ * empty terminating entry.
*
* In most cases, drivers will create a table of device IDs by using
* USB_DEVICE(), or similar macros designed for that purpose.
@@ -150,6 +166,7 @@ struct usb_device_id {
/* not matched against */
kernel_ulong_t driver_info
__attribute__((aligned(sizeof(kernel_ulong_t))));
+ const struct custom_string *custom_strings_table;
};
/* Some useful macros to use to create struct usb_device_id */
Then the driver would declare its custom stings like this:
static const struct custom_string es58x_custom_strings_table[] = {
{ .idx = 6, .label = "product_info" },
{ /* Terminating entry */ }
};
Finally, the USB core can iterate through it and populate the sysfs
entries using the provided label.
> Also, there's the matter of language. Devices can have string
> descriptors in multiple languages; which one should we show in sysfs?
> All of them? Right now we use just the default language for the strings
> that we put in sysfs.
I do not have the knowledge to comment on the multiple languages
issue. FYI, the device which I maintain does not have multiple
languages.
> > I say do this at the USB core level, that way it works for any USB
> > device, and you don't have a device-specific sysfs file and custom
> > userspace code just for this.
>
> This is unavoidable to some extent. Without device-specific information
> or userspace code, there is no way to know which string descriptor
> contains the data you want.
ACK. I also do not want any userspace code for that. Users should not
need to know a magic number to retrieve the thing.
> Alan Stern
>
> > Sound reasonable?
> >
> > greg k-h
next prev parent reply other threads:[~2022-11-06 5:35 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20221104073659.414147-1-mailhol.vincent@wanadoo.fr>
2022-11-04 17:16 ` [PATCH v2 0/3] can: etas_es58x: report firmware version Vincent Mailhol
2022-11-04 17:16 ` [PATCH v2 1/3] USB: core: export usb_cache_string() Vincent Mailhol
2022-11-05 8:22 ` Greg Kroah-Hartman
2022-11-04 17:16 ` [PATCH v2 2/3] can: etas_es58x: use usb_cache_string() to retrieve the product info string Vincent Mailhol
2022-11-05 8:21 ` Greg Kroah-Hartman
2022-11-04 17:16 ` [PATCH v2 3/3] can: etas_es58x: report the firmware version through ethtool Vincent Mailhol
2022-11-05 8:23 ` Greg Kroah-Hartman
2022-11-05 9:27 ` Vincent MAILHOL
2022-11-05 17:21 ` Vincent MAILHOL
2022-11-05 17:38 ` Greg Kroah-Hartman
2022-11-06 0:45 ` Alan Stern
2022-11-06 5:34 ` Vincent MAILHOL [this message]
2022-11-06 11:21 ` Greg Kroah-Hartman
2022-11-06 12:47 ` Vincent MAILHOL
2022-11-06 14:22 ` Greg Kroah-Hartman
2022-11-06 14:44 ` Vincent MAILHOL
2022-11-06 16:02 ` Greg Kroah-Hartman
2022-11-12 15:40 ` Vincent MAILHOL
2022-11-06 15:18 ` Alan Stern
2022-11-13 4:01 ` [PATCH v3 0/3] can: etas_es58x: report firmware, bootloader and hardware version Vincent Mailhol
2022-11-13 4:01 ` [PATCH v3 1/3] USB: core: export usb_cache_string() Vincent Mailhol
2022-11-22 16:00 ` Greg Kroah-Hartman
2022-11-13 4:01 ` [PATCH v3 2/3] can: etas_es58x: export firmware, bootloader and hardware versions in sysfs Vincent Mailhol
2022-11-15 22:47 ` Saeed Mahameed
2022-11-16 0:36 ` Vincent MAILHOL
2022-11-16 23:20 ` Saeed Mahameed
2022-11-13 4:01 ` [PATCH v3 3/3] can: etas_es58x: report firmware-version through ethtool Vincent Mailhol
2022-11-13 16:48 ` [PATCH v3 0/3] can: etas_es58x: report firmware, bootloader and hardware version Andrew Lunn
2022-11-14 16:49 ` Vincent MAILHOL
2022-11-14 17:08 ` Andrew Lunn
2022-11-26 16:22 ` [PATCH v4 0/6] " Vincent Mailhol
2022-11-26 16:22 ` [PATCH v4 1/6] USB: core: export usb_cache_string() Vincent Mailhol
2022-11-26 16:22 ` [PATCH v4 2/6] can: etas_es58x: add devlink support Vincent Mailhol
2022-11-26 16:51 ` Andrew Lunn
2022-11-27 5:10 ` Vincent MAILHOL
2022-11-27 15:36 ` Alan Stern
2022-11-28 1:34 ` Vincent MAILHOL
2022-11-28 5:32 ` Vincent MAILHOL
2022-11-28 15:50 ` Alan Stern
2022-11-28 23:30 ` Vincent MAILHOL
2022-11-28 13:42 ` Andrew Lunn
2022-11-28 14:29 ` Vincent MAILHOL
2022-11-26 16:22 ` [PATCH v4 3/6] can: etas_es58x: export product information through devlink_ops::info_get() Vincent Mailhol
2022-11-26 17:16 ` Andrew Lunn
2022-11-27 3:42 ` Vincent MAILHOL
2022-11-27 4:31 ` Vincent MAILHOL
2022-11-27 15:07 ` Andrew Lunn
2022-11-28 1:21 ` Vincent MAILHOL
2022-11-28 13:43 ` Andrew Lunn
2022-11-28 13:47 ` Andrew Lunn
2022-11-28 14:43 ` Vincent MAILHOL
2022-11-28 22:27 ` Jakub Kicinski
2022-11-28 23:17 ` Vincent MAILHOL
2022-11-26 16:22 ` [PATCH v4 4/6] can: etas_es58x: remove es58x_get_product_info() Vincent Mailhol
2022-11-28 13:44 ` Andrew Lunn
2022-11-28 14:36 ` Vincent MAILHOL
2022-11-26 16:22 ` [PATCH v4 5/6] can: etas_es58x: report the firmware version through ethtool Vincent Mailhol
2022-11-28 13:44 ` Andrew Lunn
2022-11-28 22:29 ` Jakub Kicinski
2022-11-29 17:12 ` Vincent MAILHOL
2022-11-30 2:31 ` Jakub Kicinski
2022-11-26 16:22 ` [PATCH v4 6/6] Documentation: devlink: add devlink documentation for the etas_es58x driver Vincent Mailhol
2022-11-30 17:46 ` [PATCH v5 0/7] can: etas_es58x: report firmware, bootloader and hardware version Vincent Mailhol
2022-11-30 17:46 ` [PATCH v5 1/7] can: etas_es58x: add devlink support Vincent Mailhol
2022-11-30 17:46 ` [PATCH v5 2/7] can: etas_es58x: add devlink port support Vincent Mailhol
2022-11-30 17:46 ` [PATCH v5 3/7] USB: core: export usb_cache_string() Vincent Mailhol
2022-11-30 17:46 ` [PATCH v5 4/7] net: devlink: add DEVLINK_INFO_VERSION_GENERIC_FW_BOOTLOADER Vincent Mailhol
2022-11-30 17:46 ` [PATCH v5 5/7] can: etas_es58x: export product information through devlink_ops::info_get() Vincent Mailhol
2022-11-30 17:46 ` [PATCH v5 6/7] can: etas_es58x: remove es58x_get_product_info() Vincent Mailhol
2022-11-30 17:46 ` [PATCH v5 7/7] Documentation: devlink: add devlink documentation for the etas_es58x driver Vincent Mailhol
2022-12-02 12:27 ` Marc Kleine-Budde
2022-12-02 13:15 ` Vincent MAILHOL
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAMZ6Rq+hc+YObxCAFL8Go3gvc8budzdaxLZY2b4GexcG1xHzdQ@mail.gmail.com \
--to=mailhol.vincent@wanadoo.fr \
--cc=gregkh@linuxfoundation.org \
--cc=linux-can@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=mkl@pengutronix.de \
--cc=stern@rowland.harvard.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).