| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <OS0PR01MB59225BB8DF5AE4811158563786AC9@OS0PR01MB5922.jpnprd01.prod.outlook.com>
Date: Sat, 2 Oct 2021 07:53:30 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Sergey Shtylyov <s.shtylyov@....ru>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>
CC: Sergei Shtylyov <sergei.shtylyov@...il.com>,
Geert Uytterhoeven <geert+renesas@...der.be>,
Adam Ford <aford173@...il.com>, Andrew Lunn <andrew@...n.ch>,
Yuusuke Ashizuka <ashiduka@...itsu.com>,
Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-renesas-soc@...r.kernel.org"
<linux-renesas-soc@...r.kernel.org>,
Chris Paterson <Chris.Paterson2@...esas.com>,
Biju Das <biju.das@...renesas.com>,
Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: RE: [PATCH 02/10] ravb: Rename "no_ptp_cfg_active" and
"ptp_cfg_active" variables
Hi Sergei,
> 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://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/ethernet/renesas/ravb_main.c?h=v5.15-rc3&id=f5d7837f96e53a8c9b6c49e1bc95cf0ae88b99e8
>
> > "ptp_cfg_active" with "ccc_gac" to match the HW features.
> >
> > There is no functional change.
> >
> > Signed-off-by: Biju Das <biju.das.jz@...renesas.com>
> > Suggested-by: Sergey Shtylyov <s.shtylyov@....ru>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > ---
> > 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?
I see below you are proposing to enable both "gptp" and "ccc_gac" for R-Car 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
But the existing driver code just support "gPTP support active in config mode" for R-Car Gen3.
Do you want me to do feature improvement as well, as part of Gbethernet support?
Please let me know your thoughts.
The same comments applies to all the comments you have mentioned below.
Regards,
Biju
>
> > ravb_ptp_stop(ndev);
> > /* Wait for DMA stopping */
> > error = ravb_stop_dma(ndev);
> > @@ -1306,7 +1306,7 @@ static int ravb_set_ringparam(struct net_device
> *ndev,
> > ravb_emac_init(ndev);
> >
> > /* Initialise PTP Clock driver */
> > - if (info->no_ptp_cfg_active)
> > + if (info->gptp)
> > ravb_ptp_init(ndev, priv->pdev);
>
> The same question here...
>
> > netif_device_attach(ndev);
> > @@ -1446,7 +1446,7 @@ static int ravb_open(struct net_device *ndev)
> > ravb_emac_init(ndev);
> >
> > /* Initialise PTP Clock driver */
> > - if (info->no_ptp_cfg_active)
> > + if (info->gptp)
>
> ... and here.
>
> > ravb_ptp_init(ndev, priv->pdev);
> >
> > netif_tx_start_all_queues(ndev);
> > @@ -1460,7 +1460,7 @@ static int ravb_open(struct net_device *ndev)
> >
> > out_ptp_stop:
> > /* Stop PTP Clock driver */
> > - if (info->no_ptp_cfg_active)
> > + if (info->gptp)
> > ravb_ptp_stop(ndev);
>
> ... and here.
>
> > out_free_irq_nc_tx:
> > if (!info->multi_irqs)
> > @@ -1508,7 +1508,7 @@ static void ravb_tx_timeout_work(struct
> work_struct *work)
> > netif_tx_stop_all_queues(ndev);
> >
> > /* Stop PTP Clock driver */
> > - if (info->no_ptp_cfg_active)
> > + if (info->gptp)
>
> ... and here.
>
> > ravb_ptp_stop(ndev);
> >
> > /* Wait for DMA stopping */
> > @@ -1543,7 +1543,7 @@ static void ravb_tx_timeout_work(struct
> > work_struct *work)
> >
> > out:
> > /* Initialise PTP Clock driver */
> > - if (info->no_ptp_cfg_active)
> > + if (info->gptp)
> > ravb_ptp_init(ndev, priv->pdev);
>
> ... and here.
>
> > netif_tx_start_all_queues(ndev);
> > @@ -1752,7 +1752,7 @@ static int ravb_close(struct net_device *ndev)
> > ravb_write(ndev, 0, TIC);
> >
> > /* Stop PTP Clock driver */
> > - if (info->no_ptp_cfg_active)
> > + if (info->gptp)
>
> ... and here.
>
> > ravb_ptp_stop(ndev);
> >
> > /* Set the config mode to stop the AVB-DMAC's processes */ @@
> > -2018,7 +2018,7 @@ static const struct ravb_hw_info ravb_gen3_hw_info =
> {
> > .internal_delay = 1,
> > .tx_counters = 1,
> > .multi_irqs = 1,
> > - .ptp_cfg_active = 1,
>
> Where is 'gptp'?
>
> > + .ccc_gac = 1,
> > };
> >
> > static const struct ravb_hw_info ravb_gen2_hw_info = {
> [...]
> > @@ -2080,7 +2080,7 @@ static void ravb_set_config_mode(struct net_device
> *ndev)
> > struct ravb_private *priv = netdev_priv(ndev);
> > const struct ravb_hw_info *info = priv->info;
> >
> > - if (info->no_ptp_cfg_active) {
> > + if (info->gptp) {
>
> Where have you lost !info->ccc_gac?
>
> > ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_CONFIG);
> > /* Set CSEL value */
> > ravb_modify(ndev, CCC, CCC_CSEL, CCC_CSEL_HPB);
> [...]
>
> MBR, Sergey
Powered by blists - more mailing lists