Re: [PATCH v3 07/12] usb: common: Add function to get num_lanes and transfer rate

Chunfeng Yun wrote:

On Fri, 2020-07-24 at 16:39 -0700, Thinh Nguyen wrote:

quoted

Add a new common function to parse maximum_speed property string for
the specified number of lanes and transfer rate.

Signed-off-by: Thinh Nguyen <redacted>
---
Changes in v3:
- Add new function to parse "maximum-speed" for lanes and transfer rate
- Remove separate functions getting num_lanes and transfer rate properties
Changes in v2:
- New commit

  drivers/usb/common/common.c | 47 ++++++++++++++++++++++++++++++++++++++++++---
  include/linux/usb/ch9.h     | 25 ++++++++++++++++++++++++
  2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/common/common.c b/drivers/usb/common/common.c
index 1433260d99b4..53b4950c49e4 100644
--- a/drivers/usb/common/common.c
+++ b/drivers/usb/common/common.c

@@ -77,18 +77,59 @@ const char *usb_speed_string(enum usb_device_speed speed)
  }
  EXPORT_SYMBOL_GPL(usb_speed_string);
  
-enum usb_device_speed usb_get_maximum_speed(struct device *dev)
+void usb_get_maximum_speed_and_num_lanes(struct device *dev,
+					 enum usb_device_speed *speed,
+					 enum usb_phy_gen *gen,
+					 u8 *num_lanes)
  {
  	const char *maximum_speed;
+	enum usb_device_speed matched_speed = USB_SPEED_UNKNOWN;
+	enum usb_phy_gen matched_gen = USB_PHY_GEN_UNKNOWN;
+	u8 matched_num_lanes = 0;
  	int ret;
  
  	ret = device_property_read_string(dev, "maximum-speed", &maximum_speed);
  	if (ret < 0)
-		return USB_SPEED_UNKNOWN;
+		goto done;
  
  	ret = match_string(speed_names, ARRAY_SIZE(speed_names), maximum_speed);
+	if (ret >= 0) {
+		matched_speed = ret;
+		goto done;
+	}
+
+	if (strncmp("super-speed-plus-gen2x2", maximum_speed, 23) == 0) {
+		matched_speed = USB_SPEED_SUPER_PLUS;
+		matched_gen = USB_PHY_GEN_2;
+		matched_num_lanes = 2;
+	} else if (strncmp("super-speed-plus-gen2x1", maximum_speed, 23) == 0) {
+		matched_speed = USB_SPEED_SUPER_PLUS;
+		matched_gen = USB_PHY_GEN_2;
+		matched_num_lanes = 1;
+	} else if (strncmp("super-speed-plus-gen1x2", maximum_speed, 23) == 0) {
+		matched_speed = USB_SPEED_SUPER_PLUS;
+		matched_gen = USB_PHY_GEN_1;
+		matched_num_lanes = 2;
+	}
+
+done:
+	if (speed)
+		*speed = matched_speed;
+
+	if (num_lanes)
+		*num_lanes = matched_num_lanes;
+
+	if (gen)
+		*gen = matched_gen;
+}
+EXPORT_SYMBOL_GPL(usb_get_maximum_speed_and_num_lanes);
+
+enum usb_device_speed usb_get_maximum_speed(struct device *dev)
+{
+	enum usb_device_speed speed;
  
-	return (ret < 0) ? USB_SPEED_UNKNOWN : ret;
+	usb_get_maximum_speed_and_num_lanes(dev, &speed, NULL, NULL);
+	return speed;
  }
  EXPORT_SYMBOL_GPL(usb_get_maximum_speed);
  

diff --git a/include/linux/usb/ch9.h b/include/linux/usb/ch9.h
index 01191649a0ad..46cfd72e7082 100644
--- a/include/linux/usb/ch9.h
+++ b/include/linux/usb/ch9.h

@@ -57,6 +57,13 @@ enum usb_link_protocol {
  	USB_LP_SSP = 1,
  };
  
+/* USB phy signaling rate gen */
+enum usb_phy_gen {
+	USB_PHY_GEN_UNKNOWN,
+	USB_PHY_GEN_1,
+	USB_PHY_GEN_2,
+};

The GEN_1, GEN_2 will describe the capability of not only PHY but also
MAC, add _PHY_ seems a little ambiguous, I think
usb_get_maximum_speed_and_num_lanes() is mainly used to get the
capability of MAC. Another, not suitable to add property about PHY
capablity in MAC nodes.

 From USB 3.2 spec:
"Gen 1 is an adjective used to refer to the Physical layer associated 
with a 5.0 Gbps signaling rate. The original USB SuperSpeed Phy and a 
Gen 1 Phy refer to the same Phy."
"Gen 2 is an adjective used to refer to the Physical layer associated 
with a 10 Gbps signaling rate."

If you have a better name, I'm open for suggestions.

BR,
Thinh