netdev - RE: [PATCH v3 4/5] ravb: Use separate clock for gPTP

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 12 May 2022 10:12:55 +0000
From:   Phil Edworthy <phil.edworthy@...esas.com>
To:     Paolo Abeni <pabeni@...hat.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>
CC:     Geert Uytterhoeven <geert+renesas@...der.be>,
        Sergey Shtylyov <s.shtylyov@....ru>,
        Biju Das <biju.das.jz@...renesas.com>,
        Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-renesas-soc@...r.kernel.org" 
        <linux-renesas-soc@...r.kernel.org>
Subject: RE: [PATCH v3 4/5] ravb: Use separate clock for gPTP

Hi Paolo,

On 12 May 2022 08:03 Paolo Abeni wrote:
> On Tue, 2022-05-10 at 10:03 +0100, Phil Edworthy wrote:
> > RZ/V2M has a separate gPTP reference clock that is used when the
> > AVB-DMAC Mode Register (CCC) gPTP Clock Select (CSEL) bits are set to
> > "01: High-speed peripheral bus clock".
> > Therefore, add a feature that allows this clock to be used for gPTP.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@...esas.com>
> > Reviewed-by: Biju Das <biju.das.jz@...renesas.com>
> > Reviewed-by: Sergey Shtylyov <s.shtylyov@....ru>
> > ---
> > v3:
> >  - No change
> > v2:
> >  - Added Reviewed-by tags
> > ---
> >  drivers/net/ethernet/renesas/ravb.h      |  2 ++
> >  drivers/net/ethernet/renesas/ravb_main.c | 15 ++++++++++++++-
> >  2 files changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/renesas/ravb.h
> > b/drivers/net/ethernet/renesas/ravb.h
> > index e505e8088445..b980bce763d3 100644
> > --- a/drivers/net/ethernet/renesas/ravb.h
> > +++ b/drivers/net/ethernet/renesas/ravb.h
> > @@ -1031,6 +1031,7 @@ struct ravb_hw_info {
> >  	unsigned err_mgmt_irqs:1;	/* Line1 (Err) and Line2 (Mgmt) irqs
> are separate */
> >  	unsigned gptp:1;		/* AVB-DMAC has gPTP support */
> >  	unsigned ccc_gac:1;		/* AVB-DMAC has gPTP support active in
> config mode */
> > +	unsigned gptp_ref_clk:1;	/* gPTP has separate reference clock
> */
> >  	unsigned nc_queues:1;		/* AVB-DMAC has RX and TX NC queues */
> >  	unsigned magic_pkt:1;		/* E-MAC supports magic packet
> detection */
> >  	unsigned half_duplex:1;		/* E-MAC supports half duplex mode */
> > @@ -1042,6 +1043,7 @@ struct ravb_private {
> >  	void __iomem *addr;
> >  	struct clk *clk;
> >  	struct clk *refclk;
> > +	struct clk *gptp_clk;
> >  	struct mdiobb_ctrl mdiobb;
> >  	u32 num_rx_ring[NUM_RX_QUEUE];
> >  	u32 num_tx_ring[NUM_TX_QUEUE];
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c
> > b/drivers/net/ethernet/renesas/ravb_main.c
> > index 8ccc817b8b5d..ef6967731263 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -2495,11 +2495,15 @@ MODULE_DEVICE_TABLE(of, ravb_match_table);
> > static int ravb_set_gti(struct net_device *ndev)  {
> >  	struct ravb_private *priv = netdev_priv(ndev);
> > +	const struct ravb_hw_info *info = priv->info;
> >  	struct device *dev = ndev->dev.parent;
> >  	unsigned long rate;
> >  	uint64_t inc;
> >
> > -	rate = clk_get_rate(priv->clk);
> > +	if (info->gptp_ref_clk)
> > +		rate = clk_get_rate(priv->gptp_clk);
> > +	else
> > +		rate = clk_get_rate(priv->clk);
> >  	if (!rate)
> >  		return -EINVAL;
> >
> > @@ -2721,6 +2725,15 @@ static int ravb_probe(struct platform_device
> *pdev)
> >  	}
> >  	clk_prepare_enable(priv->refclk);
> >
> > +	if (info->gptp_ref_clk) {
> > +		priv->gptp_clk = devm_clk_get(&pdev->dev, "gptp");
> > +		if (IS_ERR(priv->gptp_clk)) {
> > +			error = PTR_ERR(priv->gptp_clk);
> > +			goto out_release;
> > +		}
> > +		clk_prepare_enable(priv->gptp_clk);
> > +	}
> > +
> 
> I guess you need to a conditional
> 
> 	clk_disable_unprepare(info->gptp_ref_clk)
> 
> in the error path? And even in ravb_remove()?
Oops, how did I miss that?

Thanks
Phil

> >  	ndev->max_mtu = info->rx_max_buf_size - (ETH_HLEN + VLAN_HLEN +
> ETH_FCS_LEN);
> >  	ndev->min_mtu = ETH_MIN_MTU;
> >