diff mbox series

[2/2] net: macb: Update tsu clk usage in runtime suspend/resume for Versal

Message ID 20220720112924.1096-3-harini.katakam@xilinx.com
State New
Headers show
Series None | expand

Commit Message

Harini Katakam July 20, 2022, 11:29 a.m. UTC 
On Versal TSU clock cannot be disabled irrespective of whether PTP is
used. Hence introduce a new Versal config structure with a "need tsu"
caps flag and check the same in runtime_suspend/resume before cutting
off clocks.

More information on this for future reference:
This is an IP limitation on versions 1p11 and 1p12 when Qbv is enabled
(See designcfg1, bit 3). However it is better to rely on an SoC specific
check rather than the IP version because tsu clk property itself may not
represent actual HW tsu clock on some chip designs.

Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
---
 drivers/net/ethernet/cadence/macb.h      |  1 +
 drivers/net/ethernet/cadence/macb_main.c | 17 +++++++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

Comments

Claudiu Beznea July 22, 2022, 8:25 a.m. UTC | #1 
On 20.07.2022 14:29, Harini Katakam wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Versal TSU clock cannot be disabled irrespective of whether PTP is
> used. Hence introduce a new Versal config structure with a "need tsu"
> caps flag and check the same in runtime_suspend/resume before cutting
> off clocks.
> 
> More information on this for future reference:
> This is an IP limitation on versions 1p11 and 1p12 when Qbv is enabled
> (See designcfg1, bit 3). However it is better to rely on an SoC specific
> check rather than the IP version because tsu clk property itself may not
> represent actual HW tsu clock on some chip designs.
> 
> Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> ---
>  drivers/net/ethernet/cadence/macb.h      |  1 +
>  drivers/net/ethernet/cadence/macb_main.c | 17 +++++++++++++++--
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index 7ca077b65eaa..8bf67b44b466 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -725,6 +725,7 @@
>  #define MACB_CAPS_MACB_IS_GEM                  0x80000000
>  #define MACB_CAPS_PCS                          0x01000000
>  #define MACB_CAPS_HIGH_SPEED                   0x02000000
> +#define MACB_CAPS_NEED_TSUCLK                  0x00001000

Can you keep this sorted by the bit position used?

> 
>  /* LSO settings */
>  #define MACB_LSO_UFO_ENABLE                    0x01
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 7eb7822cd184..8bbc46e8a9eb 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -4735,6 +4735,16 @@ static const struct macb_config zynqmp_config = {
>         .usrio = &macb_default_usrio,
>  };
> 
> +static const struct macb_config versal_config = {
> +       .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
> +               MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH | MACB_CAPS_NEED_TSUCLK,
> +       .dma_burst_length = 16,
> +       .clk_init = macb_clk_init,
> +       .init = init_reset_optional,
> +       .jumbo_max_len = 10240,
> +       .usrio = &macb_default_usrio,
> +};
> +

Also, could you keep this not b/w zynq configs to have a bit of sort of these?

Other than this:

Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>


>  static const struct macb_config zynq_config = {
>         .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF |
>                 MACB_CAPS_NEEDS_RSTONUBR,
> @@ -4794,6 +4804,7 @@ static const struct of_device_id macb_dt_ids[] = {
>         { .compatible = "microchip,mpfs-macb", .data = &mpfs_config },
>         { .compatible = "microchip,sama7g5-gem", .data = &sama7g5_gem_config },
>         { .compatible = "microchip,sama7g5-emac", .data = &sama7g5_emac_config },
> +       { .compatible = "cdns,versal-gem", .data = &versal_config},
>         { /* sentinel */ }
>  };
>  MODULE_DEVICE_TABLE(of, macb_dt_ids);
> @@ -5203,7 +5214,7 @@ static int __maybe_unused macb_runtime_suspend(struct device *dev)
> 
>         if (!(device_may_wakeup(dev)))
>                 macb_clks_disable(bp->pclk, bp->hclk, bp->tx_clk, bp->rx_clk, bp->tsu_clk);
> -       else
> +       else if (!(bp->caps & MACB_CAPS_NEED_TSUCLK))
>                 macb_clks_disable(NULL, NULL, NULL, NULL, bp->tsu_clk);
> 
>         return 0;
> @@ -5219,8 +5230,10 @@ static int __maybe_unused macb_runtime_resume(struct device *dev)
>                 clk_prepare_enable(bp->hclk);
>                 clk_prepare_enable(bp->tx_clk);
>                 clk_prepare_enable(bp->rx_clk);
> +               clk_prepare_enable(bp->tsu_clk);
> +       } else if (!(bp->caps & MACB_CAPS_NEED_TSUCLK)) {
> +               clk_prepare_enable(bp->tsu_clk);
>         }
> -       clk_prepare_enable(bp->tsu_clk);
> 
>         return 0;
>  }
> --
> 2.17.1
>
Harini Katakam July 22, 2022, 10:34 a.m. UTC | #2 
Hi Claudiu,

On Fri, Jul 22, 2022 at 1:55 PM <Claudiu.Beznea@microchip.com> wrote:
>
> On 20.07.2022 14:29, Harini Katakam wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > On Versal TSU clock cannot be disabled irrespective of whether PTP is
> > used. Hence introduce a new Versal config structure with a "need tsu"
> > caps flag and check the same in runtime_suspend/resume before cutting
> > off clocks.
> >
> > More information on this for future reference:
> > This is an IP limitation on versions 1p11 and 1p12 when Qbv is enabled
> > (See designcfg1, bit 3). However it is better to rely on an SoC specific
> > check rather than the IP version because tsu clk property itself may not
> > represent actual HW tsu clock on some chip designs.
> >
> > Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
> > ---
> >  drivers/net/ethernet/cadence/macb.h      |  1 +
> >  drivers/net/ethernet/cadence/macb_main.c | 17 +++++++++++++++--
> >  2 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> > index 7ca077b65eaa..8bf67b44b466 100644
> > --- a/drivers/net/ethernet/cadence/macb.h
> > +++ b/drivers/net/ethernet/cadence/macb.h
> > @@ -725,6 +725,7 @@
> >  #define MACB_CAPS_MACB_IS_GEM                  0x80000000
> >  #define MACB_CAPS_PCS                          0x01000000
> >  #define MACB_CAPS_HIGH_SPEED                   0x02000000
> > +#define MACB_CAPS_NEED_TSUCLK                  0x00001000
>
> Can you keep this sorted by the bit position used?

Thanks for the review.
Sure, I'll sort these in a separate patch first in the same series.

>
> >
> >  /* LSO settings */
> >  #define MACB_LSO_UFO_ENABLE                    0x01
> > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> > index 7eb7822cd184..8bbc46e8a9eb 100644
> > --- a/drivers/net/ethernet/cadence/macb_main.c
> > +++ b/drivers/net/ethernet/cadence/macb_main.c
> > @@ -4735,6 +4735,16 @@ static const struct macb_config zynqmp_config = {
> >         .usrio = &macb_default_usrio,
> >  };
> >
> > +static const struct macb_config versal_config = {
> > +       .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
> > +               MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH | MACB_CAPS_NEED_TSUCLK,
> > +       .dma_burst_length = 16,
> > +       .clk_init = macb_clk_init,
> > +       .init = init_reset_optional,
> > +       .jumbo_max_len = 10240,
> > +       .usrio = &macb_default_usrio,
> > +};
> > +
>
> Also, could you keep this not b/w zynq configs to have a bit of sort of these?
>
> Other than this:
>
> Reviewed-by: Claudiu Beznea <claudiu.beznea@microchip.com>

Thanks.

Regards,
Harini
diff mbox series

Patch

diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 7ca077b65eaa..8bf67b44b466 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -725,6 +725,7 @@ 
 #define MACB_CAPS_MACB_IS_GEM			0x80000000
 #define MACB_CAPS_PCS				0x01000000
 #define MACB_CAPS_HIGH_SPEED			0x02000000
+#define MACB_CAPS_NEED_TSUCLK			0x00001000
 
 /* LSO settings */
 #define MACB_LSO_UFO_ENABLE			0x01
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 7eb7822cd184..8bbc46e8a9eb 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4735,6 +4735,16 @@  static const struct macb_config zynqmp_config = {
 	.usrio = &macb_default_usrio,
 };
 
+static const struct macb_config versal_config = {
+	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO |
+		MACB_CAPS_GEM_HAS_PTP | MACB_CAPS_BD_RD_PREFETCH | MACB_CAPS_NEED_TSUCLK,
+	.dma_burst_length = 16,
+	.clk_init = macb_clk_init,
+	.init = init_reset_optional,
+	.jumbo_max_len = 10240,
+	.usrio = &macb_default_usrio,
+};
+
 static const struct macb_config zynq_config = {
 	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF |
 		MACB_CAPS_NEEDS_RSTONUBR,
@@ -4794,6 +4804,7 @@  static const struct of_device_id macb_dt_ids[] = {
 	{ .compatible = "microchip,mpfs-macb", .data = &mpfs_config },
 	{ .compatible = "microchip,sama7g5-gem", .data = &sama7g5_gem_config },
 	{ .compatible = "microchip,sama7g5-emac", .data = &sama7g5_emac_config },
+	{ .compatible = "cdns,versal-gem", .data = &versal_config},
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, macb_dt_ids);
@@ -5203,7 +5214,7 @@  static int __maybe_unused macb_runtime_suspend(struct device *dev)
 
 	if (!(device_may_wakeup(dev)))
 		macb_clks_disable(bp->pclk, bp->hclk, bp->tx_clk, bp->rx_clk, bp->tsu_clk);
-	else
+	else if (!(bp->caps & MACB_CAPS_NEED_TSUCLK))
 		macb_clks_disable(NULL, NULL, NULL, NULL, bp->tsu_clk);
 
 	return 0;
@@ -5219,8 +5230,10 @@  static int __maybe_unused macb_runtime_resume(struct device *dev)
 		clk_prepare_enable(bp->hclk);
 		clk_prepare_enable(bp->tx_clk);
 		clk_prepare_enable(bp->rx_clk);
+		clk_prepare_enable(bp->tsu_clk);
+	} else if (!(bp->caps & MACB_CAPS_NEED_TSUCLK)) {
+		clk_prepare_enable(bp->tsu_clk);
 	}
-	clk_prepare_enable(bp->tsu_clk);
 
 	return 0;
 }