Skip to contentSkip to conversation

usb:typec:mux: remove indentation for common path

PATCH In review R Sundar 2 participants 4 messages Apr 26, 2024 – May 6, 2024

Subsystems (2)
lore
Share & tools
lore

R Sundar

author
#0 lore raw

PATCH v5 1/1 usb:typec:mux: remove indentation for common path

Nitpick, Mostly common path will not be indented. so rewritten this function to check device_node pointer is null and removed common path indentation.

---

Fixed nitpicks in code according to comments received on other patch as below:

[ Nit, this function should be rewritten to not work like this, the "common" path should not be indented, but only the exception (i.e. bail if ep is not allocated properly.) ]
https://lore.kernel.org/all/2024041103-doornail-professor-7c1e@gregkh/

Goal is to get rid of of_node_put,but sending this patch first to do one thing at a time.

Changes since v1 - fixed the typo error for spell from identation to indentation

Changes since v2 - Shifted the indentation to one level left for the switch cases as per coding style.

Changes since v3 - Added descriptive subject for the patch and checked from and sign-off having same name.

Changes since v4 - Fixed name in signed-off-by as in documents.

Patches link:
------------

v1  - https://lore.kernel.org/all/[email protected]/
v2  - https://lore.kernel.org/linux-usb/[email protected]/
v3  - https://lore.kernel.org/all/[email protected]/
v4  - https://lore.kernel.org/all/[email protected]/
 drivers/usb/typec/mux/nb7vpq904m.c | 68 +++++++++++++++---------------
 1 file changed, 34 insertions(+), 34 deletions(-)
drivers/usb/typec/mux/nb7vpq904m.c +34 -34
index b17826713753..f7a00b388876 100644
@@ -320,47 +320,47 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
 	int ret, i, j;
 
 	ep = of_graph_get_endpoint_by_regs(nb7->client->dev.of_node, 1, 0);
+	if (!ep)
+		return 0;
 
-	if (ep) {
-		ret = of_property_count_u32_elems(ep, "data-lanes");
-		if (ret == -EINVAL)
-			/* Property isn't here, consider default mapping */
-			goto out_done;
-		if (ret < 0)
-			goto out_error;
-
-		if (ret != DATA_LANES_COUNT) {
-			dev_err(&nb7->client->dev, "expected 4 data lanes\n");
-			ret = -EINVAL;
-			goto out_error;
-		}
-
-		ret = of_property_read_u32_array(ep, "data-lanes", data_lanes, DATA_LANES_COUNT);
-		if (ret)
-			goto out_error;
+	ret = of_property_count_u32_elems(ep, "data-lanes");
+	if (ret == -EINVAL)
+		/* Property isn't here, consider default mapping */
+		goto out_done;
+	if (ret < 0)
+		goto out_error;
+
+	if (ret != DATA_LANES_COUNT) {
+		dev_err(&nb7->client->dev, "expected 4 data lanes\n");
+		ret = -EINVAL;
+		goto out_error;
+	}
 
-		for (i = 0; i < ARRAY_SIZE(supported_data_lane_mapping); i++) {
-			for (j = 0; j < DATA_LANES_COUNT; j++) {
-				if (data_lanes[j] != supported_data_lane_mapping[i][j])
-					break;
-			}
+	ret = of_property_read_u32_array(ep, "data-lanes", data_lanes, DATA_LANES_COUNT);
+	if (ret)
+		goto out_error;
 
-			if (j == DATA_LANES_COUNT)
+	for (i = 0; i < ARRAY_SIZE(supported_data_lane_mapping); i++) {
+		for (j = 0; j < DATA_LANES_COUNT; j++) {
+			if (data_lanes[j] != supported_data_lane_mapping[i][j])
 				break;
 		}
 
-		switch (i) {
-		case NORMAL_LANE_MAPPING:
-			break;
-		case INVERT_LANE_MAPPING:
-			nb7->swap_data_lanes = true;
-			dev_info(&nb7->client->dev, "using inverted data lanes mapping\n");
+		if (j == DATA_LANES_COUNT)
 			break;
-		default:
-			dev_err(&nb7->client->dev, "invalid data lanes mapping\n");
-			ret = -EINVAL;
-			goto out_error;
-		}
+	}
+
+	switch (i) {
+	case NORMAL_LANE_MAPPING:
+		break;
+	case INVERT_LANE_MAPPING:
+		nb7->swap_data_lanes = true;
+		dev_info(&nb7->client->dev, "using inverted data lanes mapping\n");
+		break;
+	default:
+		dev_err(&nb7->client->dev, "invalid data lanes mapping\n");
+		ret = -EINVAL;
+		goto out_error;
 	}
 
 out_done:
Signature · 1 line
2.34.1
author
#1 lore raw
Quoted 115 lines · R Sundar

Nitpick, Mostly common path will not be indented. so rewritten this function to check device_node pointer is null and removed common path indentation.

---

Fixed nitpicks in code according to comments received on other patch as below:

[ Nit, this function should be rewritten to not work like this, the "common" path should not be indented, but only the exception (i.e. bail if ep is not allocated properly.) ]
https://lore.kernel.org/all/2024041103-doornail-professor-7c1e@gregkh/

Goal is to get rid of of_node_put,but sending this patch first to do one thing at a time.

Changes since v1 - fixed the typo error for spell from identation to indentation

Changes since v2 - Shifted the indentation to one level left for the switch cases as per coding style.

Changes since v3 - Added descriptive subject for the patch and checked from and sign-off having same name.

Changes since v4 - Fixed name in signed-off-by as in documents.

Patches link:
------------

v1  - https://lore.kernel.org/all/[email protected]/
v2  - https://lore.kernel.org/linux-usb/[email protected]/
v3  - https://lore.kernel.org/all/[email protected]/
v4  - https://lore.kernel.org/all/[email protected]/
  drivers/usb/typec/mux/nb7vpq904m.c | 68 +++++++++++++++---------------
  1 file changed, 34 insertions(+), 34 deletions(-)
drivers/usb/typec/mux/nb7vpq904m.c +27 -34
index b17826713753..f7a00b388876 100644
@@ -320,47 +320,47 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
  	int ret, i, j;
 
  	ep = of_graph_get_endpoint_by_regs(nb7->client->dev.of_node, 1, 0);
+	if (!ep)
+		return 0;
 
-	if (ep) {
-		ret = of_property_count_u32_elems(ep, "data-lanes");
-		if (ret == -EINVAL)
-			/* Property isn't here, consider default mapping */
-			goto out_done;
-		if (ret < 0)
-			goto out_error;
-
-		if (ret != DATA_LANES_COUNT) {
-			dev_err(&nb7->client->dev, "expected 4 data lanes\n");
-			ret = -EINVAL;
-			goto out_error;
-		}
-
-		ret = of_property_read_u32_array(ep, "data-lanes", data_lanes, DATA_LANES_COUNT);
-		if (ret)
-			goto out_error;
+	ret = of_property_count_u32_elems(ep, "data-lanes");
+	if (ret == -EINVAL)
+		/* Property isn't here, consider default mapping */
+		goto out_done;
+	if (ret < 0)
+		goto out_error;
+
+	if (ret != DATA_LANES_COUNT) {
+		dev_err(&nb7->client->dev, "expected 4 data lanes\n");
+		ret = -EINVAL;
+		goto out_error;
+	}
 
-		for (i = 0; i < ARRAY_SIZE(supported_data_lane_mapping); i++) {
-			for (j = 0; j < DATA_LANES_COUNT; j++) {
-				if (data_lanes[j] != supported_data_lane_mapping[i][j])
-					break;
-			}
+	ret = of_property_read_u32_array(ep, "data-lanes", data_lanes, DATA_LANES_COUNT);
+	if (ret)
+		goto out_error;
 
-			if (j == DATA_LANES_COUNT)
+	for (i = 0; i < ARRAY_SIZE(supported_data_lane_mapping); i++) {
+		for (j = 0; j < DATA_LANES_COUNT; j++) {
+			if (data_lanes[j] != supported_data_lane_mapping[i][j])
  				break;
  		}
 
-		switch (i) {
-		case NORMAL_LANE_MAPPING:
-			break;
-		case INVERT_LANE_MAPPING:
-			nb7->swap_data_lanes = true;
-			dev_info(&nb7->client->dev, "using inverted data lanes mapping\n");
+		if (j == DATA_LANES_COUNT)
  			break;
-		default:
-			dev_err(&nb7->client->dev, "invalid data lanes mapping\n");
-			ret = -EINVAL;
-			goto out_error;
-		}
+	}
+
+	switch (i) {
+	case NORMAL_LANE_MAPPING:
+		break;
+	case INVERT_LANE_MAPPING:

drivers/usb/typec/mux/nb7vpq904m.c static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7) · line 366

Quoted 10 lines
drivers/usb/typec/mux/nb7vpq904m.c +7 -0
+		nb7->swap_data_lanes = true;
+		dev_info(&nb7->client->dev, "using inverted data lanes mapping\n");
+		break;
+	default:
+		dev_err(&nb7->client->dev, "invalid data lanes mapping\n");
+		ret = -EINVAL;
+		goto out_error;
  	}
 
  out_done:

Hi,

Any feedback for this patch?

Regards,
Sundar

maintainer
#2 lore raw
In reply to R Sundar

Hi Sundar,

Quoted 5 lines · R Sundar

Nitpick, Mostly common path will not be indented. so rewritten this function to check device_node pointer is null and removed common path indentation.

For the record, I'm still uncomfortable with the name - why not just spell out your whole name?

Quoted 26 lines

---

Fixed nitpicks in code according to comments received on other patch as below:

[ Nit, this function should be rewritten to not work like this, the "common" path should not be indented, but only the exception (i.e. bail if ep is not allocated properly.) ]
https://lore.kernel.org/all/2024041103-doornail-professor-7c1e@gregkh/

Goal is to get rid of of_node_put,but sending this patch first to do one thing at a time.

Changes since v1 - fixed the typo error for spell from identation to indentation

Changes since v2 - Shifted the indentation to one level left for the switch cases as per coding style.

Changes since v3 - Added descriptive subject for the patch and checked from and sign-off having same name.

Changes since v4 - Fixed name in signed-off-by as in documents.

Patches link:
------------

Quoted 7 lines
v1  - https://lore.kernel.org/all/[email protected]/
v2  - https://lore.kernel.org/linux-usb/[email protected]/
v3  - https://lore.kernel.org/all/[email protected]/
v4  - https://lore.kernel.org/all/[email protected]/
 drivers/usb/typec/mux/nb7vpq904m.c | 68 +++++++++++++++---------------
 1 file changed, 34 insertions(+), 34 deletions(-)

Sorry for missing this earlier, but it looks like this patch only modifies the nb7vpq904m driver, so I think you should specify that already in the subject.
While at it, you could also specify the only function that is being modified in the commit message (this is just a suggestion):

usb: typec: nb7vpq904m: Remove uneeded indentation

In function nb7vpq904m_parse_data_lanes_mapping(), the "if (ep)" condition is basically the entire function. Making the code a bit more readable by inverting the condition so that the function returns immedately if there is no "ep".

thanks,

Signature · 1 line
heikki
author
#3 lore raw

Hi Heikki,

Quoted 12 lines · Heikki Krogerus

Hi Sundar,

Quoted 5 lines · R Sundar

Nitpick, Mostly common path will not be indented. so rewritten this function to check device_node pointer is null and removed common path indentation.

For the record, I'm still uncomfortable with the name - why not just spell out your whole name?

I understand your concern. " R Sundar ", is my whole name.

Quoted 12 lines
Quoted 5 lines

---

Fixed nitpicks in code according to comments received on other patch as below:

Sorry for missing this earlier, but it looks like this patch only modifies the nb7vpq904m driver, so I think you should specify that already in the subject.
While at it, you could also specify the only function that is being modified in the commit message (this is just a suggestion):

Quoted 9 lines

usb: typec: nb7vpq904m: Remove uneeded indentation

In function nb7vpq904m_parse_data_lanes_mapping(), the "if (ep)" condition is basically the entire function. Making the code a bit more readable by inverting the condition so that the function returns immedately if there is no "ep".

thanks,

Thanks for the suggestion provided, will modify the commit message and send the updated one.

Thanks,
Sundar