2020-07-06 14:27:48

by Sergey Organov

Subject: [PATCH 4/5] net: fec: get rid of redundant code in fec_ptp_set()

Code of the form "if(x) x = 0" replaced with "x = 0".

Code of the form "if(x == a) x = a" removed.

Signed-off-by: Sergey Organov <[email protected]>
---
drivers/net/ethernet/freescale/fec_ptp.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c
index e455343..4152cae 100644
--- a/drivers/net/ethernet/freescale/fec_ptp.c
+++ b/drivers/net/ethernet/freescale/fec_ptp.c
@@ -485,9 +485,7 @@ int fec_ptp_set(struct net_device *ndev, struct ifreq *ifr)

switch (config.rx_filter) {
case HWTSTAMP_FILTER_NONE:
- if (fep->hwts_rx_en)
- fep->hwts_rx_en = 0;
- config.rx_filter = HWTSTAMP_FILTER_NONE;
+ fep->hwts_rx_en = 0;
break;

default:
--
2.10.0.1.g57b01a3

2020-07-07 04:09:00

by Andy Duan

Subject: RE: [EXT] [PATCH 4/5] net: fec: get rid of redundant code in fec_ptp_set()

From: Sergey Organov <[email protected]> Sent: Monday, July 6, 2020 10:26 PM
> Code of the form "if(x) x = 0" replaced with "x = 0".
>
> Code of the form "if(x == a) x = a" removed.
>
> Signed-off-by: Sergey Organov <[email protected]>
> ---
> drivers/net/ethernet/freescale/fec_ptp.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
> b/drivers/net/ethernet/freescale/fec_ptp.c
> index e455343..4152cae 100644
> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> @@ -485,9 +485,7 @@ int fec_ptp_set(struct net_device *ndev, struct ifreq
> *ifr)
>
> switch (config.rx_filter) {
> case HWTSTAMP_FILTER_NONE:
> - if (fep->hwts_rx_en)
> - fep->hwts_rx_en = 0;
> - config.rx_filter = HWTSTAMP_FILTER_NONE;
The line should keep according your commit log.

> + fep->hwts_rx_en = 0;
> break;
>
> default:
> --
> 2.10.0.1.g57b01a3

2020-07-07 14:43:55

by Sergey Organov

Subject: Re: [EXT] [PATCH 4/5] net: fec: get rid of redundant code in fec_ptp_set()

Andy Duan <[email protected]> writes:

> From: Sergey Organov <[email protected]> Sent: Monday, July 6, 2020 10:26 PM
>> Code of the form "if(x) x = 0" replaced with "x = 0".
>>
>> Code of the form "if(x == a) x = a" removed.
>>
>> Signed-off-by: Sergey Organov <[email protected]>
>> ---
>> drivers/net/ethernet/freescale/fec_ptp.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
>> b/drivers/net/ethernet/freescale/fec_ptp.c
>> index e455343..4152cae 100644
>> --- a/drivers/net/ethernet/freescale/fec_ptp.c
>> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
>> @@ -485,9 +485,7 @@ int fec_ptp_set(struct net_device *ndev, struct ifreq
>> *ifr)
>>
>> switch (config.rx_filter) {
>> case HWTSTAMP_FILTER_NONE:
>> - if (fep->hwts_rx_en)
>> - fep->hwts_rx_en = 0;
>> - config.rx_filter = HWTSTAMP_FILTER_NONE;
> The line should keep according your commit log.

You mean I should fix commit log like this:

Code of the form "switch(x) case a: x = a; break" removed.

?

I'll do if it's cleaner that way.

Thanks,
-- Sergey


>
>> + fep->hwts_rx_en = 0;
>> break;
>>
>> default:
>> --
>> 2.10.0.1.g57b01a3

2020-07-08 05:41:24

by Andy Duan

Subject: RE: [EXT] [PATCH 4/5] net: fec: get rid of redundant code in fec_ptp_set()

From: Sergey Organov <[email protected]> Sent: Tuesday, July 7, 2020 10:43 PM
> Andy Duan <[email protected]> writes:
>
> > From: Sergey Organov <[email protected]> Sent: Monday, July 6, 2020
> 10:26 PM
> >> Code of the form "if(x) x = 0" replaced with "x = 0".
> >>
> >> Code of the form "if(x == a) x = a" removed.
> >>
> >> Signed-off-by: Sergey Organov <[email protected]>
> >> ---
> >> drivers/net/ethernet/freescale/fec_ptp.c | 4 +---
> >> 1 file changed, 1 insertion(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
> >> b/drivers/net/ethernet/freescale/fec_ptp.c
> >> index e455343..4152cae 100644
> >> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> >> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> >> @@ -485,9 +485,7 @@ int fec_ptp_set(struct net_device *ndev, struct
> ifreq
> >> *ifr)
> >>
> >> switch (config.rx_filter) {
> >> case HWTSTAMP_FILTER_NONE:
> >> - if (fep->hwts_rx_en)
> >> - fep->hwts_rx_en = 0;
> >> - config.rx_filter = HWTSTAMP_FILTER_NONE;
> > The line should keep according your commit log.
>
> You mean I should fix commit log like this:
>
> Code of the form "switch(x) case a: x = a; break" removed.
>
> ?
Like this:

case HWTSTAMP_FILTER_NONE:
fep->hwts_rx_en = 0;
config.rx_filter = HWTSTAMP_FILTER_NONE;
break;
>
> I'll do if it's cleaner that way.
>
> Thanks,
> -- Sergey
>
>
> >
> >> + fep->hwts_rx_en = 0;
> >> break;
> >>
> >> default:
> >> --
> >> 2.10.0.1.g57b01a3

2020-07-08 08:49:25

by Sergey Organov

Subject: Re: [EXT] [PATCH 4/5] net: fec: get rid of redundant code in fec_ptp_set()

Andy Duan <[email protected]> writes:

> From: Sergey Organov <[email protected]> Sent: Tuesday, July 7, 2020 10:43 PM
>> Andy Duan <[email protected]> writes:
>>
>> > From: Sergey Organov <[email protected]> Sent: Monday, July 6, 2020
>> 10:26 PM
>> >> Code of the form "if(x) x = 0" replaced with "x = 0".
>> >>
>> >> Code of the form "if(x == a) x = a" removed.
>> >>
>> >> Signed-off-by: Sergey Organov <[email protected]>
>> >> ---
>> >> drivers/net/ethernet/freescale/fec_ptp.c | 4 +---
>> >> 1 file changed, 1 insertion(+), 3 deletions(-)
>> >>
>> >> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
>> >> b/drivers/net/ethernet/freescale/fec_ptp.c
>> >> index e455343..4152cae 100644
>> >> --- a/drivers/net/ethernet/freescale/fec_ptp.c
>> >> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
>> >> @@ -485,9 +485,7 @@ int fec_ptp_set(struct net_device *ndev, struct
>> ifreq
>> >> *ifr)
>> >>
>> >> switch (config.rx_filter) {
>> >> case HWTSTAMP_FILTER_NONE:
>> >> - if (fep->hwts_rx_en)
>> >> - fep->hwts_rx_en = 0;
>> >> - config.rx_filter = HWTSTAMP_FILTER_NONE;
>> > The line should keep according your commit log.
>>
>> You mean I should fix commit log like this:
>>
>> Code of the form "switch(x) case a: x = a; break" removed.
>>
>> ?
> Like this:
>
> case HWTSTAMP_FILTER_NONE:
> fep->hwts_rx_en = 0;
> config.rx_filter = HWTSTAMP_FILTER_NONE;

This last line is redundant, as it's part of the switch that reads:

switch (config.rx_filter) {
case HWTSTAMP_FILTER_NONE: config.rx_filter = HWTSTAMP_FILTER_NONE;

that effectively reduces to:

if(x == a) x = a;

that is a no-op (provided 'x' is a usual memory reference),
and that is exactly what I've described in the commit log.

What do I miss?

Thanks,
-- Sergey

2020-07-08 08:58:11

by Andy Duan

Subject: RE: [EXT] [PATCH 4/5] net: fec: get rid of redundant code in fec_ptp_set()

From: Sergey Organov <[email protected]> Sent: Wednesday, July 8, 2020 4:49 PM
> Andy Duan <[email protected]> writes:
>
> > From: Sergey Organov <[email protected]> Sent: Tuesday, July 7, 2020
> > 10:43 PM
> >> Andy Duan <[email protected]> writes:
> >>
> >> > From: Sergey Organov <[email protected]> Sent: Monday, July 6,
> >> > 2020
> >> 10:26 PM
> >> >> Code of the form "if(x) x = 0" replaced with "x = 0".
> >> >>
> >> >> Code of the form "if(x == a) x = a" removed.
> >> >>
> >> >> Signed-off-by: Sergey Organov <[email protected]>
> >> >> ---
> >> >> drivers/net/ethernet/freescale/fec_ptp.c | 4 +---
> >> >> 1 file changed, 1 insertion(+), 3 deletions(-)
> >> >>
> >> >> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
> >> >> b/drivers/net/ethernet/freescale/fec_ptp.c
> >> >> index e455343..4152cae 100644
> >> >> --- a/drivers/net/ethernet/freescale/fec_ptp.c
> >> >> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
> >> >> @@ -485,9 +485,7 @@ int fec_ptp_set(struct net_device *ndev,
> >> >> struct
> >> ifreq
> >> >> *ifr)
> >> >>
> >> >> switch (config.rx_filter) {
> >> >> case HWTSTAMP_FILTER_NONE:
> >> >> - if (fep->hwts_rx_en)
> >> >> - fep->hwts_rx_en = 0;
> >> >> - config.rx_filter = HWTSTAMP_FILTER_NONE;

The original patch seems fine. Thanks!

For the patch: Acked-by: Fugang Duan <[email protected]>

2020-07-08 12:28:08

by Sergey Organov

Subject: Re: [EXT] [PATCH 4/5] net: fec: get rid of redundant code in fec_ptp_set()

Andy Duan <[email protected]> writes:

> From: Sergey Organov <[email protected]> Sent: Wednesday, July 8, 2020 4:49 PM
>> Andy Duan <[email protected]> writes:
>>
>> > From: Sergey Organov <[email protected]> Sent: Tuesday, July 7, 2020
>> > 10:43 PM
>> >> Andy Duan <[email protected]> writes:
>> >>
>> >> > From: Sergey Organov <[email protected]> Sent: Monday, July 6,
>> >> > 2020
>> >> 10:26 PM
>> >> >> Code of the form "if(x) x = 0" replaced with "x = 0".
>> >> >>
>> >> >> Code of the form "if(x == a) x = a" removed.
>> >> >>
>> >> >> Signed-off-by: Sergey Organov <[email protected]>
>> >> >> ---
>> >> >> drivers/net/ethernet/freescale/fec_ptp.c | 4 +---
>> >> >> 1 file changed, 1 insertion(+), 3 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/net/ethernet/freescale/fec_ptp.c
>> >> >> b/drivers/net/ethernet/freescale/fec_ptp.c
>> >> >> index e455343..4152cae 100644
>> >> >> --- a/drivers/net/ethernet/freescale/fec_ptp.c
>> >> >> +++ b/drivers/net/ethernet/freescale/fec_ptp.c
>> >> >> @@ -485,9 +485,7 @@ int fec_ptp_set(struct net_device *ndev,
>> >> >> struct
>> >> ifreq
>> >> >> *ifr)
>> >> >>
>> >> >> switch (config.rx_filter) {
>> >> >> case HWTSTAMP_FILTER_NONE:
>> >> >> - if (fep->hwts_rx_en)
>> >> >> - fep->hwts_rx_en = 0;
>> >> >> - config.rx_filter = HWTSTAMP_FILTER_NONE;
>
> The original patch seems fine. Thanks!
>
> For the patch: Acked-by: Fugang Duan <[email protected]>

OK, thanks for reviewing!

-- Sergey