usb:typec:mux: remove indentation for common path
Subsystems (2)
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
@@ -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
Quoted 115 lines · R Sundar
Nitpick, Mostly common path will not be indented. so rewritten this function to check
device_nodepointer is null and removed common path indentation.Signed-off-by R Sundar <[email protected]>---
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@@ -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
Heikki Krogerus
maintainerHi Sundar,
Quoted 5 lines · R Sundar
Nitpick, Mostly common path will not be indented. so rewritten this function to check
device_nodepointer is null and removed common path indentation.Signed-off-by R Sundar <[email protected]>
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
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_nodepointer is null and removed common path indentation.Signed-off-by R Sundar <[email protected]>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