Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1651488 > unrolled thread

[PATCH v2 13/27] thunderbolt: Read vendor and device name from DROM

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v2 13/27] thunderbolt: Read vendor and device name from DROM Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-05-26 18:20 +0200
    Re: [PATCH v2 13/27] thunderbolt: Read vendor and device name from DROM Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-27 18:00 +0200
    Re: [PATCH v2 13/27] thunderbolt: Read vendor and device name from DROM Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-05-27 18:00 +0200
      Re: [PATCH v2 13/27] thunderbolt: Read vendor and device name from  DROM Mika Westerberg <mika.westerberg@linux.intel.com> - 2017-05-28 10:50 +0200

#1651488 — [PATCH v2 13/27] thunderbolt: Read vendor and device name from DROM

The device DROM contains name of the vendor and device among other
things. Extract this information and expose it to the userspace via two
new attributes.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Yehezkel Bernat <yehezkel.bernat@intel.com>
Reviewed-by: Michael Jamet <michael.jamet@intel.com>
---
 Documentation/ABI/testing/sysfs-bus-thunderbolt | 14 +++++++++
 drivers/thunderbolt/eeprom.c                    | 42 +++++++++++++++++++++++++
 drivers/thunderbolt/switch.c                    | 22 +++++++++++++
 drivers/thunderbolt/tb.h                        |  4 +++
 4 files changed, 82 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
index a3dac3becd1e..2f352c787431 100644
--- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
+++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
@@ -5,6 +5,13 @@ Contact:	thunderbolt-software@lists.01.org
 Description:	This attribute contains id of this device extracted from
 		the device DROM.
 
+What:		/sys/bus/thunderbolt/devices/.../device_name
+Date:		Sep 2017
+KernelVersion:	4.13
+Contact:	thunderbolt-software@lists.01.org
+Description:	This attribute contains name of this device extracted from
+		the device DROM.
+
 What:		/sys/bus/thunderbolt/devices/.../vendor
 Date:		Sep 2017
 KernelVersion:	4.13
@@ -12,6 +19,13 @@ Contact:	thunderbolt-software@lists.01.org
 Description:	This attribute contains vendor id of this device extracted
 		from the device DROM.
 
+What:		/sys/bus/thunderbolt/devices/.../vendor_name
+Date:		Sep 2017
+KernelVersion:	4.13
+Contact:	thunderbolt-software@lists.01.org
+Description:	This attribute contains vendor name of this device extracted
+		from the device DROM.
+
 What:		/sys/bus/thunderbolt/devices/.../unique_id
 Date:		Sep 2017
 KernelVersion:	4.13
diff --git a/drivers/thunderbolt/eeprom.c b/drivers/thunderbolt/eeprom.c
index 5c7d80a109b1..8070f5966aac 100644
--- a/drivers/thunderbolt/eeprom.c
+++ b/drivers/thunderbolt/eeprom.c
@@ -204,6 +204,11 @@ struct tb_drom_entry_header {
 	enum tb_drom_entry_type type:1;
 } __packed;
 
+struct tb_drom_entry_generic {
+	struct tb_drom_entry_header header;
+	u8 data[0];
+} __packed;
+
 struct tb_drom_entry_port {
 	/* BYTES 0-1 */
 	struct tb_drom_entry_header header;
@@ -295,6 +300,42 @@ int tb_drom_read_uid_only(struct tb_switch *sw, u64 *uid)
 	return 0;
 }
 
+static const char *parse_name(const u8 *data, u8 len)
+{
+	char *name;
+
+	name = kmemdup(data, len, GFP_KERNEL);
+	if (name)
+		name[len - 1] = '\0';
+	return name;
+}
+
+static int tb_drom_parse_entry_generic(struct tb_switch *sw,
+		struct tb_drom_entry_header *header)
+{
+	const struct tb_drom_entry_generic *entry =
+		(const struct tb_drom_entry_generic *)header;
+
+	switch (header->index) {
+	case 1:
+		/* Length includes 2 bytes header so remove it before copy */
+		sw->vendor_name = parse_name(entry->data,
+					     header->len - sizeof(*header));
+		if (!sw->vendor_name)
+			return -ENOMEM;
+		break;
+
+	case 2:
+		sw->device_name = parse_name(entry->data,
+					     header->len - sizeof(*header));
+		if (!sw->device_name)
+			return -ENOMEM;
+		break;
+	}
+
+	return 0;
+}
+
 static int tb_drom_parse_entry_port(struct tb_switch *sw,
 				    struct tb_drom_entry_header *header)
 {
@@ -350,6 +391,7 @@ static int tb_drom_parse_entries(struct tb_switch *sw)
 
 		switch (entry->type) {
 		case TB_DROM_ENTRY_GENERIC:
+			res = tb_drom_parse_entry_generic(sw, entry);
 			break;
 		case TB_DROM_ENTRY_PORT:
 			res = tb_drom_parse_entry_port(sw, entry);
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 86ebce28e257..da63a75eebf4 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -319,6 +319,15 @@ static ssize_t device_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(device);
 
+static ssize_t
+device_name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct tb_switch *sw = tb_to_switch(dev);
+
+	return sprintf(buf, "%s\n", sw->device_name ? sw->device_name : "");
+}
+static DEVICE_ATTR_RO(device_name);
+
 static ssize_t vendor_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
@@ -328,6 +337,15 @@ static ssize_t vendor_show(struct device *dev, struct device_attribute *attr,
 }
 static DEVICE_ATTR_RO(vendor);
 
+static ssize_t
+vendor_name_show(struct device *dev, struct device_attribute *attr, char *buf)
+{
+	struct tb_switch *sw = tb_to_switch(dev);
+
+	return sprintf(buf, "%s\n", sw->vendor_name ? sw->vendor_name : "");
+}
+static DEVICE_ATTR_RO(vendor_name);
+
 static ssize_t unique_id_show(struct device *dev, struct device_attribute *attr,
 			      char *buf)
 {
@@ -339,7 +357,9 @@ static DEVICE_ATTR_RO(unique_id);
 
 static struct attribute *switch_attrs[] = {
 	&dev_attr_device.attr,
+	&dev_attr_device_name.attr,
 	&dev_attr_vendor.attr,
+	&dev_attr_vendor_name.attr,
 	&dev_attr_unique_id.attr,
 	NULL,
 };
@@ -358,6 +378,8 @@ static void tb_switch_release(struct device *dev)
 	struct tb_switch *sw = tb_to_switch(dev);
 
 	kfree(sw->uuid);
+	kfree(sw->device_name);
+	kfree(sw->vendor_name);
 	kfree(sw->ports);
 	kfree(sw->drom);
 	kfree(sw);
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 350c3f21924e..5e66dce53c65 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -23,6 +23,8 @@
  * @uuid: UUID of the switch (or %NULL if not supported)
  * @vendor: Vendor ID of the switch
  * @device: Device ID of the switch
+ * @vendor_name: Name of the vendor (or %NULL if not known)
+ * @device_name: Name of the device (or %NULL if not known)
  * @cap_plug_events: Offset to the plug events capability (%0 if not found)
  * @is_unplugged: The switch is going away
  * @drom: DROM of the switch (%NULL if not found)
@@ -36,6 +38,8 @@ struct tb_switch {
 	uuid_be *uuid;
 	u16 vendor;
 	u16 device;
+	const char *vendor_name;
+	const char *device_name;
 	int cap_plug_events;
 	bool is_unplugged;
 	u8 *drom;
-- 
2.11.0

[toc] | [next] | [standalone]

#1651905

On Sat, May 27, 2017 at 6:57 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, May 26, 2017 at 7:09 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
>> The device DROM contains name of the vendor and device among other
>> things. Extract this information and expose it to the userspace via two
>> new attributes.
>
>> +static const char *parse_name(const u8 *data, u8 len)
>
> Hmm... (name)
>
>> +{
>> +       char *name;
>> +
>> +       name = kmemdup(data, len, GFP_KERNEL);
>
> Since it's ASCII by specification it may make sense to use
>
> sw->..._name = kstrndup(entry->data, sizeof(*header), GFP_KERNEL);

sizeof(*header) - 1, of course.

> if (!sw->..._name)
>  return -ENOMEM;
>
> just in place, instead of this entire function.
>
>> +       if (name)
>> +               name[len - 1] = '\0';
>> +       return name;
>> +}
>> +
>> +static int tb_drom_parse_entry_generic(struct tb_switch *sw,
>> +               struct tb_drom_entry_header *header)
>> +{
>> +       const struct tb_drom_entry_generic *entry =
>> +               (const struct tb_drom_entry_generic *)header;
>> +
>> +       switch (header->index) {
>> +       case 1:
>> +               /* Length includes 2 bytes header so remove it before copy */
>> +               sw->vendor_name = parse_name(entry->data,
>> +                                            header->len - sizeof(*header));
>> +               if (!sw->vendor_name)
>> +                       return -ENOMEM;
>> +               break;
>> +
>> +       case 2:
>> +               sw->device_name = parse_name(entry->data,
>> +                                            header->len - sizeof(*header));
>> +               if (!sw->device_name)
>> +                       return -ENOMEM;
>> +               break;
>> +       }
>> +
>> +       return 0;
>> +}
>
> --
> With Best Regards,
> Andy Shevchenko



-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1651906

On Fri, May 26, 2017 at 7:09 PM, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
> The device DROM contains name of the vendor and device among other
> things. Extract this information and expose it to the userspace via two
> new attributes.

> +static const char *parse_name(const u8 *data, u8 len)

Hmm... (name)

> +{
> +       char *name;
> +
> +       name = kmemdup(data, len, GFP_KERNEL);

Since it's ASCII by specification it may make sense to use

sw->..._name = kstrndup(entry->data, sizeof(*header), GFP_KERNEL);
if (!sw->..._name)
 return -ENOMEM;

just in place, instead of this entire function.

> +       if (name)
> +               name[len - 1] = '\0';
> +       return name;
> +}
> +
> +static int tb_drom_parse_entry_generic(struct tb_switch *sw,
> +               struct tb_drom_entry_header *header)
> +{
> +       const struct tb_drom_entry_generic *entry =
> +               (const struct tb_drom_entry_generic *)header;
> +
> +       switch (header->index) {
> +       case 1:
> +               /* Length includes 2 bytes header so remove it before copy */
> +               sw->vendor_name = parse_name(entry->data,
> +                                            header->len - sizeof(*header));
> +               if (!sw->vendor_name)
> +                       return -ENOMEM;
> +               break;
> +
> +       case 2:
> +               sw->device_name = parse_name(entry->data,
> +                                            header->len - sizeof(*header));
> +               if (!sw->device_name)
> +                       return -ENOMEM;
> +               break;
> +       }
> +
> +       return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko

[toc] | [prev] | [next] | [standalone]

#1652058 — Re: [PATCH v2 13/27] thunderbolt: Read vendor and device name from DROM

On Sat, May 27, 2017 at 06:57:38PM +0300, Andy Shevchenko wrote:
> On Fri, May 26, 2017 at 7:09 PM, Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> > The device DROM contains name of the vendor and device among other
> > things. Extract this information and expose it to the userspace via two
> > new attributes.
> 
> > +static const char *parse_name(const u8 *data, u8 len)
> 
> Hmm... (name)
> 
> > +{
> > +       char *name;
> > +
> > +       name = kmemdup(data, len, GFP_KERNEL);
> 
> Since it's ASCII by specification it may make sense to use
> 
> sw->..._name = kstrndup(entry->data, sizeof(*header), GFP_KERNEL);
> if (!sw->..._name)
>  return -ENOMEM;
> 
> just in place, instead of this entire function.

I did not even know we have kstrndup(). Cool, I'll change that but the
length needs to be header->len - sizeof(*header) instead of
sizeof(*header).

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web