| Message ID | 20230605094402.85071-1-csokas.bence@prolan.hu (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [resubmit] net: fec: Refactor: rename `adapter` to `fep` | expand |
On 05.06.2023 11:44:03, Csókás Bence wrote:
> Signed-off-by: Csókás Bence <csokas.bence@prolan.hu>
You probably want to add a patch description.
regards,
Marc
On 2023. 06. 05. 11:51, Marc Kleine-Budde wrote: > On 05.06.2023 11:44:03, Csókás Bence wrote: >> Signed-off-by: Csókás Bence <csokas.bence@prolan.hu> > > You probably want to add a patch description. Is it necessary for such a trivial refactor commit? I thought the commit msg already said it all. What else do you think I should include still? Would something like this be sufficient? "Rename local `struct fec_enet_private *adapter` to `fep` in `fec_ptp_gettime()` to match the rest of the driver" > > regards, > Marc > Thanks, Bence
On 05.06.2023 14:35:33, Csókás Bence wrote: > On 2023. 06. 05. 11:51, Marc Kleine-Budde wrote: > > On 05.06.2023 11:44:03, Csókás Bence wrote: > > > Signed-off-by: Csókás Bence <csokas.bence@prolan.hu> > > > > You probably want to add a patch description. > > Is it necessary for such a trivial refactor commit? This is considered good practice. > I thought the commit msg > already said it all. What else do you think I should include still? > > Would something like this be sufficient? > "Rename local `struct fec_enet_private *adapter` to `fep` in > `fec_ptp_gettime()` to match the rest of the driver" Looks good to me. Thanks, Marc
On 05.06.23 14:35, Csókás Bence wrote: > On 2023. 06. 05. 11:51, Marc Kleine-Budde wrote: >> On 05.06.2023 11:44:03, Csókás Bence wrote: >>> Signed-off-by: Csókás Bence <csokas.bence@prolan.hu> >> >> You probably want to add a patch description. > > Is it necessary for such a trivial refactor commit? I thought the commit msg already said it all. What else do you think I should include still? > > > Would something like this be sufficient? > "Rename local `struct fec_enet_private *adapter` to `fep` in `fec_ptp_gettime()` to match the rest of the driver" The "to match the rest of the driver" is the interesting part. I see what the commit is doing, but the title alone doesn't tell me why you'd want to change it. Cheers, Ahmad > >> >> regards, >> Marc >> > > Thanks, > Bence > > >
On Mon, Jun 05, 2023 at 11:44:03AM +0200, Csókás Bence wrote: please provide a motivation behind this rename in the commit message. > Signed-off-by: Csókás Bence <csokas.bence@prolan.hu> > --- > drivers/net/ethernet/freescale/fec_ptp.c | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c > index ab86bb8562ef..afc658d2c271 100644 > --- a/drivers/net/ethernet/freescale/fec_ptp.c > +++ b/drivers/net/ethernet/freescale/fec_ptp.c > @@ -443,21 +443,21 @@ static int fec_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta) > */ > static int fec_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts) > { > - struct fec_enet_private *adapter = > + struct fec_enet_private *fep = > container_of(ptp, struct fec_enet_private, ptp_caps); > u64 ns; > unsigned long flags; > > - mutex_lock(&adapter->ptp_clk_mutex); > + mutex_lock(&fep->ptp_clk_mutex); > /* Check the ptp clock */ > - if (!adapter->ptp_clk_on) { > - mutex_unlock(&adapter->ptp_clk_mutex); > + if (!fep->ptp_clk_on) { > + mutex_unlock(&fep->ptp_clk_mutex); > return -EINVAL; > } > - spin_lock_irqsave(&adapter->tmreg_lock, flags); > - ns = timecounter_read(&adapter->tc); > - spin_unlock_irqrestore(&adapter->tmreg_lock, flags); > - mutex_unlock(&adapter->ptp_clk_mutex); > + spin_lock_irqsave(&fep->tmreg_lock, flags); > + ns = timecounter_read(&fep->tc); > + spin_unlock_irqrestore(&fep->tmreg_lock, flags); > + mutex_unlock(&fep->ptp_clk_mutex); > > *ts = ns_to_timespec64(ns); > > -- > 2.25.1 > > >
diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index ab86bb8562ef..afc658d2c271 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -443,21 +443,21 @@ static int fec_ptp_adjtime(struct ptp_clock_info *ptp, s64 delta) */ static int fec_ptp_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts) { - struct fec_enet_private *adapter = + struct fec_enet_private *fep = container_of(ptp, struct fec_enet_private, ptp_caps); u64 ns; unsigned long flags; - mutex_lock(&adapter->ptp_clk_mutex); + mutex_lock(&fep->ptp_clk_mutex); /* Check the ptp clock */ - if (!adapter->ptp_clk_on) { - mutex_unlock(&adapter->ptp_clk_mutex); + if (!fep->ptp_clk_on) { + mutex_unlock(&fep->ptp_clk_mutex); return -EINVAL; } - spin_lock_irqsave(&adapter->tmreg_lock, flags); - ns = timecounter_read(&adapter->tc); - spin_unlock_irqrestore(&adapter->tmreg_lock, flags); - mutex_unlock(&adapter->ptp_clk_mutex); + spin_lock_irqsave(&fep->tmreg_lock, flags); + ns = timecounter_read(&fep->tc); + spin_unlock_irqrestore(&fep->tmreg_lock, flags); + mutex_unlock(&fep->ptp_clk_mutex); *ts = ns_to_timespec64(ns);
Signed-off-by: Csókás Bence <csokas.bence@prolan.hu> --- drivers/net/ethernet/freescale/fec_ptp.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)