2022-05-11 16:50:00

by Min Li

[permalink] [raw]
Subject: [PATCH net v5 3/3] ptp: clockmatrix: miscellaneous cosmetic change

suggested by Jakub Kicinski

Signed-off-by: Min Li <[email protected]>
---
drivers/ptp/ptp_clockmatrix.c | 69 +++++++++++++------------------------------
1 file changed, 20 insertions(+), 49 deletions(-)

diff --git a/drivers/ptp/ptp_clockmatrix.c b/drivers/ptp/ptp_clockmatrix.c
index ea87487..4461635 100644
--- a/drivers/ptp/ptp_clockmatrix.c
+++ b/drivers/ptp/ptp_clockmatrix.c
@@ -261,17 +261,13 @@ static int arm_tod_read_trig_sel_refclk(struct idtcm_channel *channel, u8 ref)

if (err)
dev_err(idtcm->dev, "%s: err = %d", __func__, err);
-
return err;
}

static bool is_single_shot(u8 mask)
{
/* Treat single bit ToD masks as continuous trigger */
- if ((mask == 1) || (mask == 2) || (mask == 4) || (mask == 8))
- return false;
- else
- return true;
+ return mask <= 8 && is_power_of_2(mask);
}

static int idtcm_extts_enable(struct idtcm_channel *channel,
@@ -418,13 +414,10 @@ static int _idtcm_gettime_triggered(struct idtcm_channel *channel,

err = idtcm_read(idtcm, channel->tod_read_secondary,
TOD_READ_SECONDARY_BASE, buf, sizeof(buf));
-
if (err)
return err;

- err = char_array_to_timespec(buf, sizeof(buf), ts);
-
- return err;
+ return char_array_to_timespec(buf, sizeof(buf), ts);
}

static int _idtcm_gettime(struct idtcm_channel *channel,
@@ -456,9 +449,7 @@ static int _idtcm_gettime(struct idtcm_channel *channel,
if (err)
return err;

- err = char_array_to_timespec(buf, sizeof(buf), ts);
-
- return err;
+ return char_array_to_timespec(buf, sizeof(buf), ts);
}

static int idtcm_extts_check_channel(struct idtcm *idtcm, u8 todn)
@@ -500,13 +491,10 @@ static int _idtcm_gettime_immediate(struct idtcm_channel *channel,

err = idtcm_write(idtcm, channel->tod_read_primary,
tod_read_cmd, &val, sizeof(val));
-
if (err)
return err;

- err = _idtcm_gettime(channel, ts, 10);
-
- return err;
+ return _idtcm_gettime(channel, ts, 10);
}

static int _sync_pll_output(struct idtcm *idtcm,
@@ -631,9 +619,7 @@ static int _sync_pll_output(struct idtcm *idtcm,

/* Place master sync out of reset */
val &= ~(SYNCTRL1_MASTER_SYNC_RST);
- err = idtcm_write(idtcm, 0, sync_ctrl1, &val, sizeof(val));
-
- return err;
+ return idtcm_write(idtcm, 0, sync_ctrl1, &val, sizeof(val));
}

static int idtcm_sync_pps_output(struct idtcm_channel *channel)
@@ -917,7 +903,6 @@ static int _idtcm_settime(struct idtcm_channel *channel,
static int idtcm_set_phase_pull_in_offset(struct idtcm_channel *channel,
s32 offset_ns)
{
- int err;
int i;
struct idtcm *idtcm = channel->idtcm;
u8 buf[4];
@@ -927,16 +912,13 @@ static int idtcm_set_phase_pull_in_offset(struct idtcm_channel *channel,
offset_ns >>= 8;
}

- err = idtcm_write(idtcm, channel->dpll_phase_pull_in, PULL_IN_OFFSET,
- buf, sizeof(buf));
-
- return err;
+ return idtcm_write(idtcm, channel->dpll_phase_pull_in, PULL_IN_OFFSET,
+ buf, sizeof(buf));
}

static int idtcm_set_phase_pull_in_slope_limit(struct idtcm_channel *channel,
u32 max_ffo_ppb)
{
- int err;
u8 i;
struct idtcm *idtcm = channel->idtcm;
u8 buf[3];
@@ -949,10 +931,8 @@ static int idtcm_set_phase_pull_in_slope_limit(struct idtcm_channel *channel,
max_ffo_ppb >>= 8;
}

- err = idtcm_write(idtcm, channel->dpll_phase_pull_in,
- PULL_IN_SLOPE_LIMIT, buf, sizeof(buf));
-
- return err;
+ return idtcm_write(idtcm, channel->dpll_phase_pull_in,
+ PULL_IN_SLOPE_LIMIT, buf, sizeof(buf));
}

static int idtcm_start_phase_pull_in(struct idtcm_channel *channel)
@@ -991,9 +971,7 @@ static int do_phase_pull_in_fw(struct idtcm_channel *channel,
if (err)
return err;

- err = idtcm_start_phase_pull_in(channel);
-
- return err;
+ return idtcm_start_phase_pull_in(channel);
}

static int set_tod_write_overhead(struct idtcm_channel *channel)
@@ -1417,10 +1395,9 @@ static int idtcm_set_pll_mode(struct idtcm_channel *channel,

dpll_mode |= (mode << PLL_MODE_SHIFT);

- err = idtcm_write(idtcm, channel->dpll_n,
- IDTCM_FW_REG(idtcm->fw_ver, V520, DPLL_MODE),
- &dpll_mode, sizeof(dpll_mode));
- return err;
+ return idtcm_write(idtcm, channel->dpll_n,
+ IDTCM_FW_REG(idtcm->fw_ver, V520, DPLL_MODE),
+ &dpll_mode, sizeof(dpll_mode));
}

static int idtcm_get_manual_reference(struct idtcm_channel *channel,
@@ -1460,11 +1437,9 @@ static int idtcm_set_manual_reference(struct idtcm_channel *channel,

dpll_manu_ref_cfg |= (ref << MANUAL_REFERENCE_SHIFT);

- err = idtcm_write(idtcm, channel->dpll_ctrl_n,
- DPLL_CTRL_DPLL_MANU_REF_CFG,
- &dpll_manu_ref_cfg, sizeof(dpll_manu_ref_cfg));
-
- return err;
+ return idtcm_write(idtcm, channel->dpll_ctrl_n,
+ DPLL_CTRL_DPLL_MANU_REF_CFG,
+ &dpll_manu_ref_cfg, sizeof(dpll_manu_ref_cfg));
}

static int configure_dpll_mode_write_frequency(struct idtcm_channel *channel)
@@ -1746,10 +1721,8 @@ static int _idtcm_adjphase(struct idtcm_channel *channel, s32 delta_ns)
phase_50ps >>= 8;
}

- err = idtcm_write(idtcm, channel->dpll_phase, DPLL_WR_PHASE,
- buf, sizeof(buf));
-
- return err;
+ return idtcm_write(idtcm, channel->dpll_phase, DPLL_WR_PHASE,
+ buf, sizeof(buf));
}

static int _idtcm_adjfine(struct idtcm_channel *channel, long scaled_ppm)
@@ -1790,10 +1763,8 @@ static int _idtcm_adjfine(struct idtcm_channel *channel, long scaled_ppm)
fcw >>= 8;
}

- err = idtcm_write(idtcm, channel->dpll_freq, DPLL_WR_FREQ,
- buf, sizeof(buf));
-
- return err;
+ return idtcm_write(idtcm, channel->dpll_freq, DPLL_WR_FREQ,
+ buf, sizeof(buf));
}

static int idtcm_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
--
2.7.4



2022-05-12 22:28:21

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net v5 3/3] ptp: clockmatrix: miscellaneous cosmetic change

On Wed, 11 May 2022 10:25:14 -0400 Min Li wrote:
> suggested by Jakub Kicinski
>
> Signed-off-by: Min Li <[email protected]>
> ---
> drivers/ptp/ptp_clockmatrix.c | 69 +++++++++++++------------------------------
> 1 file changed, 20 insertions(+), 49 deletions(-)

Not what I meant. I guess you don't speak English so no point trying to
explain. Please resend v4 and we'll merge that. v5 is not better.

2022-05-13 17:28:06

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net v5 3/3] ptp: clockmatrix: miscellaneous cosmetic change

On Thu, 12 May 2022 03:12:13 +0000 Min Li wrote:
> > Not what I meant. I guess you don't speak English so no point trying to
> > explain. Please resend v4 and we'll merge that. v5 is not better.
>
> Where do you want me to do it then? PATCH 2?

First of all, I don't understand why you keep sending these patches for
net. Please add more information about the changes to the commit
messages.

For the formatting I was complaining about - you should fold updates to
the code you're _already_modifying_ into the relevant patches.

You can clean up the rest of the code but definitely not in net. Code
refactoring goes to net-next.

Perhaps a read of the netdev FAQ will elucidate what I'm on about:
https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

2022-05-14 01:01:22

by Jakub Kicinski

[permalink] [raw]
Subject: Re: [PATCH net v5 3/3] ptp: clockmatrix: miscellaneous cosmetic change

On Fri, 13 May 2022 19:57:26 +0000 Min Li wrote:
> There are multiple places where "no empty line between call and error
> check" and "return directly" like you pointed out before. Some are
> related to this change and some are not. Do you prefer to fix only
> the related ones in this patch or do them all in another patch to
> net-next

Let's forget cleaning up the code not touched by patches 1 and 2.

Within the lines of code patches 1 and 2 touch - by which I mean the
lines listed with a '+' at the start - please make sure there are
no instances of the formatting issues there.

2022-05-14 01:43:07

by Min Li

[permalink] [raw]
Subject: RE: [PATCH net v5 3/3] ptp: clockmatrix: miscellaneous cosmetic change

>
> First of all, I don't understand why you keep sending these patches for net.
> Please add more information about the changes to the commit messages.
>
> For the formatting I was complaining about - you should fold updates to the
> code you're _already_modifying_ into the relevant patches.
>
> You can clean up the rest of the code but definitely not in net. Code
> refactoring goes to net-next.
>
> Perhaps a read of the netdev FAQ will elucidate what I'm on about:
> https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.
> kernel.org%2Fdoc%2Fhtml%2Flatest%2Fprocess%2Fmaintainer-
> netdev.html&amp;data=05%7C01%7Cmin.li.xe%40renesas.com%7C3cbe9c7
> 3bb2e4e4765d608da3430e6c8%7C53d82571da1947e49cb4625a166a4a2
> a%7C0%7C0%7C637879681870307714%7CUnknown%7CTWFpbGZsb3d8ey
> JWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> %7C3000%7C%7C%7C&amp;sdata=cYeTkW596blMW6amKHFPk7cgv9G%2B
> R7%2B0zZP72DJebDA%3D&amp;reserved=0

Hi Jakub

There are multiple places where "no empty line between call and error check" and "return directly"
like you pointed out before. Some are related to this change and some are not. Do you prefer to fix only
the related ones in this patch or do them all in another patch to net-next

Min

2022-05-14 03:28:13

by Min Li

[permalink] [raw]
Subject: RE: [PATCH net v5 3/3] ptp: clockmatrix: miscellaneous cosmetic change

> Not what I meant. I guess you don't speak English so no point trying to
> explain. Please resend v4 and we'll merge that. v5 is not better.

Where do you want me to do it then? PATCH 2?