* [PATCH v2] remove indentation for common path [linux-next]
@ 2024-04-20 16:49 sundar
2024-04-20 17:07 ` Joe Perches
2024-04-20 20:47 ` Dmitry Baryshkov
0 siblings, 2 replies; 6+ messages in thread
From: sundar @ 2024-04-20 16:49 UTC (permalink / raw
To: heikki.krogerus, gregkh, neil.armstrong, dmitry.baryshkov,
u.kleine-koenig, christophe.jaillet
Cc: linux-usb, linux-kernel, skhan, javier.carrasco.cruz, sundar
Added check if pointer is null and removed indentation for common path
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: sundar <prosunofficial@gmail.com>
---
Fixed nitpicks in code according to comments received on other patch.
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
v1 patch link - https://lore.kernel.org/all/20240420145522.15018-1-prosunofficial@gmail.com/
drivers/usb/typec/mux/nb7vpq904m.c | 49 +++++++++++++++---------------
1 file changed, 25 insertions(+), 24 deletions(-)
diff --git a/drivers/usb/typec/mux/nb7vpq904m.c b/drivers/usb/typec/mux/nb7vpq904m.c
index b17826713753..fe0257840dd5 100644
--- a/drivers/usb/typec/mux/nb7vpq904m.c
+++ b/drivers/usb/typec/mux/nb7vpq904m.c
@@ -321,35 +321,37 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
ep = of_graph_get_endpoint_by_regs(nb7->client->dev.of_node, 1, 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;
- }
+ if (!ep)
+ return 0;
- 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) {
+ if (j == DATA_LANES_COUNT)
+ break;
+ }
+
+ switch (i) {
case NORMAL_LANE_MAPPING:
break;
case INVERT_LANE_MAPPING:
@@ -360,7 +362,6 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
dev_err(&nb7->client->dev, "invalid data lanes mapping\n");
ret = -EINVAL;
goto out_error;
- }
}
out_done:
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] remove indentation for common path [linux-next]
2024-04-20 16:49 [PATCH v2] remove indentation for common path [linux-next] sundar
@ 2024-04-20 17:07 ` Joe Perches
2024-04-21 0:43 ` sundar
2024-04-20 20:47 ` Dmitry Baryshkov
1 sibling, 1 reply; 6+ messages in thread
From: Joe Perches @ 2024-04-20 17:07 UTC (permalink / raw
To: sundar, heikki.krogerus, gregkh, neil.armstrong, dmitry.baryshkov,
u.kleine-koenig, christophe.jaillet
Cc: linux-usb, linux-kernel, skhan, javier.carrasco.cruz
On Sat, 2024-04-20 at 22:19 +0530, sundar wrote:
> ```
Added check if pointer is null and removed indentation for common path
```
[]
> ```
diff --git a/drivers/usb/typec/mux/nb7vpq904m.c b/drivers/usb/typec/mux/nb7vpq904m.c
```
[]
> ```
@@ -321,35 +321,37 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
ep = of_graph_get_endpoint_by_regs(nb7->client->dev.of_node, 1, 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;
- }
+ if (!ep)
+ return 0;
```
Not equivalent code as the out_error:
of_node_put(ep);
isn't done
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] remove indentation for common path [linux-next]
2024-04-20 16:49 [PATCH v2] remove indentation for common path [linux-next] sundar
2024-04-20 17:07 ` Joe Perches
@ 2024-04-20 20:47 ` Dmitry Baryshkov
2024-04-21 0:52 ` sundar
1 sibling, 1 reply; 6+ messages in thread
From: Dmitry Baryshkov @ 2024-04-20 20:47 UTC (permalink / raw
To: sundar
Cc: heikki.krogerus, gregkh, neil.armstrong, u.kleine-koenig,
christophe.jaillet, linux-usb, linux-kernel, skhan,
javier.carrasco.cruz
On Sat, 20 Apr 2024 at 19:49, sundar <prosunofficial@gmail.com> wrote:
>
> Added check if pointer is null and removed indentation for common path
>
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: sundar <prosunofficial@gmail.com>
> ---
>
> Fixed nitpicks in code according to comments received on other patch.
>
> 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
>
> v1 patch link - https://lore.kernel.org/all/20240420145522.15018-1-prosunofficial@gmail.com/
>
> drivers/usb/typec/mux/nb7vpq904m.c | 49 +++++++++++++++---------------
> 1 file changed, 25 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/usb/typec/mux/nb7vpq904m.c b/drivers/usb/typec/mux/nb7vpq904m.c
> index b17826713753..fe0257840dd5 100644
> --- a/drivers/usb/typec/mux/nb7vpq904m.c
> +++ b/drivers/usb/typec/mux/nb7vpq904m.c
> @@ -321,35 +321,37 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
>
> ep = of_graph_get_endpoint_by_regs(nb7->client->dev.of_node, 1, 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;
> - }
> + if (!ep)
> + return 0;
>
> - 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) {
> + if (j == DATA_LANES_COUNT)
> + break;
> + }
> +
> + switch (i) {
> case NORMAL_LANE_MAPPING:
switch-cases should also be shifted one level to the left, see
Documentation/process/coding-style.rst
> break;
> case INVERT_LANE_MAPPING:
> @@ -360,7 +362,6 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
> dev_err(&nb7->client->dev, "invalid data lanes mapping\n");
> ret = -EINVAL;
> goto out_error;
> - }
> }
>
> out_done:
> --
> 2.34.1
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] remove indentation for common path [linux-next]
2024-04-20 17:07 ` Joe Perches
@ 2024-04-21 0:43 ` sundar
2024-04-21 1:27 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: sundar @ 2024-04-21 0:43 UTC (permalink / raw
To: Joe Perches, heikki.krogerus, gregkh, neil.armstrong,
dmitry.baryshkov, u.kleine-koenig, christophe.jaillet
Cc: linux-usb, linux-kernel, skhan, javier.carrasco.cruz
On 20/04/24 22:37, Joe Perches wrote:
> On Sat, 2024-04-20 at 22:19 +0530, sundar wrote:
>
>
>> ```
> @@ -321,35 +321,37 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
>
> ep = of_graph_get_endpoint_by_regs(nb7->client->dev.of_node, 1, 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;
> - }
> + if (!ep)
> + return 0;
> ```
>
>
> Not equivalent code as the out_error:
>
> of_node_put(ep);
>
> isn't done
>
>
Hi joe perches,
If ep is null, I believe we dont need to call of_node_put. Because
passing null pointer to of_node_put() make no difference.
In of_node_put() definition, if pointer is null, there is no operation.
Thanks,
Sundar
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] remove indentation for common path [linux-next]
2024-04-20 20:47 ` Dmitry Baryshkov
@ 2024-04-21 0:52 ` sundar
0 siblings, 0 replies; 6+ messages in thread
From: sundar @ 2024-04-21 0:52 UTC (permalink / raw
To: Dmitry Baryshkov
Cc: heikki.krogerus, gregkh, neil.armstrong, u.kleine-koenig,
christophe.jaillet, linux-usb, linux-kernel, skhan,
javier.carrasco.cruz
On 21/04/24 02:17, Dmitry Baryshkov wrote:
> On Sat, 20 Apr 2024 at 19:49, sundar <prosunofficial@gmail.com> wrote:
>>
>> Added check if pointer is null and removed indentation for common path
>>
>> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Signed-off-by: sundar <prosunofficial@gmail.com>
>> ---
>>
>> Fixed nitpicks in code according to comments received on other patch.
>>
>> 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
>>
>> v1 patch link - https://lore.kernel.org/all/20240420145522.15018-1-prosunofficial@gmail.com/
>>
>> drivers/usb/typec/mux/nb7vpq904m.c | 49 +++++++++++++++---------------
>> 1 file changed, 25 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/usb/typec/mux/nb7vpq904m.c b/drivers/usb/typec/mux/nb7vpq904m.c
>> index b17826713753..fe0257840dd5 100644
>> --- a/drivers/usb/typec/mux/nb7vpq904m.c
>> +++ b/drivers/usb/typec/mux/nb7vpq904m.c
>> @@ -321,35 +321,37 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
>>
>> ep = of_graph_get_endpoint_by_regs(nb7->client->dev.of_node, 1, 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;
>> - }
>> + if (!ep)
>> + return 0;
>>
>> - 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) {
>> + if (j == DATA_LANES_COUNT)
>> + break;
>> + }
>> +
>> + switch (i) {
>> case NORMAL_LANE_MAPPING:
>
> switch-cases should also be shifted one level to the left, see
> Documentation/process/coding-style.rst
>
>> break;
>> case INVERT_LANE_MAPPING:
>> @@ -360,7 +362,6 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
>> dev_err(&nb7->client->dev, "invalid data lanes mapping\n");
>> ret = -EINVAL;
>> goto out_error;
>> - }
>> }
>>
>> out_done:
>> --
>> 2.34.1
>>
>
>
Hi Dmitry,
Thanks for the review.will fix my code.
Thanks,
Sundar
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] remove indentation for common path [linux-next]
2024-04-21 0:43 ` sundar
@ 2024-04-21 1:27 ` Joe Perches
0 siblings, 0 replies; 6+ messages in thread
From: Joe Perches @ 2024-04-21 1:27 UTC (permalink / raw
To: sundar, heikki.krogerus, gregkh, neil.armstrong, dmitry.baryshkov,
u.kleine-koenig, christophe.jaillet
Cc: linux-usb, linux-kernel, skhan, javier.carrasco.cruz
On Sun, 2024-04-21 at 06:13 +0530, sundar wrote:
> On 20/04/24 22:37, Joe Perches wrote:
> > On Sat, 2024-04-20 at 22:19 +0530, sundar wrote:
> >
> >
> > > ```
> > @@ -321,35 +321,37 @@ static int nb7vpq904m_parse_data_lanes_mapping(struct nb7vpq904m *nb7)
> >
> > ep = of_graph_get_endpoint_by_regs(nb7->client->dev.of_node, 1, 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;
> > - }
> > + if (!ep)
> > + return 0;
> > ```
> >
> >
> > Not equivalent code as the out_error:
> >
> > of_node_put(ep);
> >
> > isn't done
> >
> >
>
> Hi joe perches,
>
> If ep is null, I believe we dont need to call of_node_put. Because
> passing null pointer to of_node_put() make no difference.
>
> In of_node_put() definition, if pointer is null, there is no operation.
>
Fine, but you should explain that in the changelog
and not make reviewers look it up.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-21 1:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-20 16:49 [PATCH v2] remove indentation for common path [linux-next] sundar
2024-04-20 17:07 ` Joe Perches
2024-04-21 0:43 ` sundar
2024-04-21 1:27 ` Joe Perches
2024-04-20 20:47 ` Dmitry Baryshkov
2024-04-21 0:52 ` sundar
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 read-only IMAP folder(s) and NNTP newsgroup(s).