diff mbox series

[net,v5,3/3] ptp: clockmatrix: miscellaneous cosmetic change

Message ID 1652279114-25939-3-git-send-email-min.li.xe@renesas.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series [net,v5,1/3] ptp: ptp_clockmatrix: Add PTP_CLK_REQ_EXTTS support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net
netdev/fixes_present fail Series targets non-next tree, but doesn't contain any Fixes tags
netdev/subject_prefix success Link
netdev/cover_letter warning Series does not have a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 2 of 2 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 164 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Min Li May 11, 2022, 2:25 p.m. UTC 
suggested by Jakub Kicinski

Signed-off-by: Min Li <min.li.xe@renesas.com>
---
 drivers/ptp/ptp_clockmatrix.c | 69 +++++++++++++------------------------------
 1 file changed, 20 insertions(+), 49 deletions(-)

Comments

Jakub Kicinski May 12, 2022, 12:37 a.m. UTC | #1 
On Wed, 11 May 2022 10:25:14 -0400 Min Li wrote:
> suggested by Jakub Kicinski
> 
> Signed-off-by: Min Li <min.li.xe@renesas.com>
> ---
>  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.
Min Li May 12, 2022, 3:12 a.m. UTC | #2 
> 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?
Jakub Kicinski May 12, 2022, 4:03 p.m. UTC | #3 
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
Min Li May 13, 2022, 7:57 p.m. UTC | #4 
> 
> 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
Jakub Kicinski May 13, 2022, 9:33 p.m. UTC | #5 
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.
diff mbox series

Patch

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)