Hi Sergei, > Subject: Re: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and > "ptp_cfg_active" variables > > Hello! > > Damn, DaveM continues ignoring my review efforts... :-( will finish > reviewing the series anyway. > > On 10/2/21 10:53 AM, Biju Das wrote: > > >> Subject: Re: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and > >> "ptp_cfg_active" variables > >> > >> On 10/1/21 6:06 PM, Biju Das wrote: > >> > >>> Rename the variable "no_ptp_cfg_active" with "gptp" and > >> > >> This shouldn't be a rename but the extension of the meaning > instead... > > > > This is the original ptp support for both R-Car Gen3 and R-Car Gen2 > without config in active mode. Later we added feature support active in > config mode for R-Car Gen3 by patch[1]. > > [1] > > https://jpn01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit. > > kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2 > > Fcommit%2Fdrivers%2Fnet%2Fethernet%2Frenesas%2Fravb_main.c%3Fh%3Dv5.15 > > -rc3%26id%3Df5d7837f96e53a8c9b6c49e1bc95cf0ae88b99e8&data=04%7C01% > > 7Cbiju.das.jz%40bp.renesas.com%7Cb4a62982865a4f7cf38408d985d11fef%7C53 > > d82571da1947e49cb4625a166a4a2a%7C0%7C0%7C637687955521294093%7CUnknown% > > 7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJX > > VCI6Mn0%3D%7C1000&sdata=6rpdh6hEAUl1yMng2ruFrKflBiDGmq6RylI90Ije3t > > 4%3D&reserved=0 > > And? Do you think I don't remember the driver development history? :-) > > >>> "ptp_cfg_active" with "ccc_gac" to match the HW features. > >>> > >>> There is no functional change. > >>> > >>> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx> > >>> Suggested-by: Sergey Shtylyov <s.shtylyov@xxxxxx> > >>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx> > >>> --- > >>> RFc->v1: > >>> * Renamed the variable "no_ptp_cfg_active" with "gptp" and > >>> "ptp_cfg_active" with "ccc_gac > >>> --- > >>> drivers/net/ethernet/renesas/ravb.h | 4 ++-- > >>> drivers/net/ethernet/renesas/ravb_main.c | 26 > >>> ++++++++++++------------ > >>> 2 files changed, 15 insertions(+), 15 deletions(-) > >> > >> [...] > >>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c > >>> b/drivers/net/ethernet/renesas/ravb_main.c > >>> index 8f2358caef34..dc7654abfe55 100644 > >>> --- a/drivers/net/ethernet/renesas/ravb_main.c > >>> +++ b/drivers/net/ethernet/renesas/ravb_main.c > >>> @@ -1274,7 +1274,7 @@ static int ravb_set_ringparam(struct > >>> net_device > >> *ndev, > >>> if (netif_running(ndev)) { > >>> netif_device_detach(ndev); > >>> /* Stop PTP Clock driver */ > >>> - if (info->no_ptp_cfg_active) > >>> + if (info->gptp) > >> > >> Where have you lost !info->ccc_gac? > > > > As per patch[1], the check is for R-Car Gen2. Why do you need > > additional check as per the current driver? > > Because the driver now supports not only gen2, but also gen3, and > RZ/G2L, finally. > > > I see below you are proposing to enable both "gptp" and "ccc_gac" for > > R-Car Gen3, > > Yes, this is how the hardware evolved. gPTP hardware can (optionally) > be active outside the config mode, otherwise there's no difference b/w > gen2 and gen3. > > > According to me it is a feature improvement for R-Car Gen3 in which, > > you can have > > > > 1) gPTP support active in config mode > > 2) gPTP support not active in config mode > > Right. > > > But the existing driver code just support "gPTP support active in config > mode" for R-Car Gen3. > > And? > > > Do you want me to do feature improvement as well, as part of Gbethernet > support? > > I thought we agreed on this patch in the previous iteration, To be more > clear, by asking to remove the "double negation", I meant using: I never thought of adding feature improvements as part of Gbethernet support. Any feature improvements after adding GbEthernet support. If you expressed your ideas like adding gptp, ccc_gac for R-Car Gen3 earlier, then I should have responded it is feature improvement. So please share your ideas in advance. Regards, Biju > > if (info->gptp && !info->ccc_gac) > > versus your: > > if (!info->no_gptp && !info->ccc_gac) > > > Please let me know your thoughts. > > > > The same comments applies to all the comments you have mentioned below. > > > > Regards, > > Biju > > [...] > > MBR, Sergey
