| Message ID | 20210224115146.9131-4-aford173@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [V3,1/5] dt-bindings: net: renesas,etheravb: Add additional clocks | expand |
> @@ -2260,6 +2267,9 @@ static int ravb_remove(struct platform_device *pdev) > if (priv->chip_id != RCAR_GEN2) > ravb_ptp_stop(ndev); > > + if (priv->refclk) > + clk_disable_unprepare(priv->refclk); > + Hi Adam You don't need the if (). The clk API is happy with a NULL pointer and will do the right thing. Otherwise: Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
Hi Adam, On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote: > For devices that use a programmable clock for the AVB reference clock, > the driver may need to enable them. Add code to find the optional clock > and enable it when available. > > Signed-off-by: Adam Ford <aford173@gmail.com> Thanks for your patch! > --- a/drivers/net/ethernet/renesas/ravb_main.c > +++ b/drivers/net/ethernet/renesas/ravb_main.c > @@ -2148,6 +2148,13 @@ static int ravb_probe(struct platform_device *pdev) > goto out_release; > } > > + priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk"); > + if (IS_ERR(priv->refclk)) { > + error = PTR_ERR(priv->refclk); > + goto out_release; > + } > + clk_prepare_enable(priv->refclk); > + Shouldn't the reference clock be disabled in case of any failure below? > ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN); > ndev->min_mtu = ETH_MIN_MTU; > > @@ -2260,6 +2267,9 @@ static int ravb_remove(struct platform_device *pdev) > if (priv->chip_id != RCAR_GEN2) > ravb_ptp_stop(ndev); > > + if (priv->refclk) > + clk_disable_unprepare(priv->refclk); > + > dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat, > priv->desc_bat_dma); > /* Set reset mode */ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Adam, On Mon, Mar 29, 2021 at 2:45 PM Adam Ford <aford173@gmail.com> wrote: > On Thu, Mar 4, 2021 at 2:08 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Wed, Feb 24, 2021 at 12:52 PM Adam Ford <aford173@gmail.com> wrote: > > > For devices that use a programmable clock for the AVB reference clock, > > > the driver may need to enable them. Add code to find the optional clock > > > and enable it when available. > > > > > > Signed-off-by: Adam Ford <aford173@gmail.com> > > > > Thanks for your patch! > > > > > --- a/drivers/net/ethernet/renesas/ravb_main.c > > > +++ b/drivers/net/ethernet/renesas/ravb_main.c > > > @@ -2148,6 +2148,13 @@ static int ravb_probe(struct platform_device *pdev) > > > goto out_release; > > > } > > > > > > + priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk"); > > > + if (IS_ERR(priv->refclk)) { > > > + error = PTR_ERR(priv->refclk); > > > + goto out_release; > > > + } > > > + clk_prepare_enable(priv->refclk); > > > + > > > > Shouldn't the reference clock be disabled in case of any failure below? > > > I'll generate a V4. > > Should I just regenerate this patch since it seems like the rest are > OK, or should I regenerate the whole series? As the DT bindings haven't been applied yet, I think it would be best if you would send a v4 with just the patches for the netdev tree (i.e. DT bindings patch 1 and driver patch 4). I will take the DT patches from this series, once the bindings have been accepted. Thank you! Gr{oetje,eeting}s, Geert
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h index 7453b17a37a2..ff363797bd2b 100644 --- a/drivers/net/ethernet/renesas/ravb.h +++ b/drivers/net/ethernet/renesas/ravb.h @@ -994,6 +994,7 @@ struct ravb_private { struct platform_device *pdev; void __iomem *addr; struct clk *clk; + struct clk *refclk; 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 bd30505fbc57..614448e6eb24 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -2148,6 +2148,13 @@ static int ravb_probe(struct platform_device *pdev) goto out_release; } + priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk"); + if (IS_ERR(priv->refclk)) { + error = PTR_ERR(priv->refclk); + goto out_release; + } + clk_prepare_enable(priv->refclk); + ndev->max_mtu = 2048 - (ETH_HLEN + VLAN_HLEN + ETH_FCS_LEN); ndev->min_mtu = ETH_MIN_MTU; @@ -2260,6 +2267,9 @@ static int ravb_remove(struct platform_device *pdev) if (priv->chip_id != RCAR_GEN2) ravb_ptp_stop(ndev); + if (priv->refclk) + clk_disable_unprepare(priv->refclk); + dma_free_coherent(ndev->dev.parent, priv->desc_bat_size, priv->desc_bat, priv->desc_bat_dma); /* Set reset mode */
For devices that use a programmable clock for the AVB reference clock, the driver may need to enable them. Add code to find the optional clock and enable it when available. Signed-off-by: Adam Ford <aford173@gmail.com> --- V3: Change 'avb' to 'AVB' Remove unnessary else statement and pointer maniupluation when enabling the refclock. Add disable_unprepare call in remove funtion. V2: The previous patch to fetch the fclk was dropped. In its place is code to enable the refclk