* [PATCH] net: macb: ensure the device is available before accessing GEMGXL control registers
@ 2021-05-21 12:48 ` Zong Li
0 siblings, 0 replies; 6+ messages in thread
From: Zong Li @ 2021-05-21 12:48 UTC (permalink / raw)
To: nicolas.ferre, claudiu.beznea, davem, kuba, palmer,
paul.walmsley, schwab, sboyd, aou, mturquette, geert, yixun.lan,
netdev, linux-kernel, linux-riscv
Cc: Zong Li
If runtime power menagement is enabled, the gigabit ethernet PLL would
be disabled after macb_probe(). During this period of time, the system
would hang up if we try to access GEMGXL control registers.
We can't put runtime_pm_get/runtime_pm_put/ there due to the issue of
sleep inside atomic section (7fa2955ff70ce453 ("sh_eth: Fix sleeping
function called from invalid context"). Add the similar flag to ensure
the device is available before accessing GEMGXL device.
Signed-off-by: Zong Li <zong.li@sifive.com>
---
drivers/net/ethernet/cadence/macb.h | 2 ++
drivers/net/ethernet/cadence/macb_main.c | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d8d87213697c..acf5242ce715 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1309,6 +1309,8 @@ struct macb {
u32 rx_intr_mask;
+ unsigned int is_opened;
+
struct macb_pm_data pm_data;
const struct macb_usrio_config *usrio;
};
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6bc7d41d519b..e079ed10ad91 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2781,6 +2781,8 @@ static int macb_open(struct net_device *dev)
if (bp->ptp_info)
bp->ptp_info->ptp_init(dev);
+ bp->is_opened = 1;
+
return 0;
reset_hw:
@@ -2818,6 +2820,8 @@ static int macb_close(struct net_device *dev)
if (bp->ptp_info)
bp->ptp_info->ptp_remove(dev);
+ bp->is_opened = 0;
+
pm_runtime_put(&bp->pdev->dev);
return 0;
@@ -2867,6 +2871,9 @@ static struct net_device_stats *gem_get_stats(struct macb *bp)
struct gem_stats *hwstat = &bp->hw_stats.gem;
struct net_device_stats *nstat = &bp->dev->stats;
+ if (!bp->is_opened)
+ return nstat;
+
gem_update_stats(bp);
nstat->rx_errors = (hwstat->rx_frame_check_sequence_errors +
--
2.31.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] net: macb: ensure the device is available before accessing GEMGXL control registers
@ 2021-05-21 12:48 ` Zong Li
0 siblings, 0 replies; 6+ messages in thread
From: Zong Li @ 2021-05-21 12:48 UTC (permalink / raw)
To: nicolas.ferre, claudiu.beznea, davem, kuba, palmer,
paul.walmsley, schwab, sboyd, aou, mturquette, geert, yixun.lan,
netdev, linux-kernel, linux-riscv
Cc: Zong Li
If runtime power menagement is enabled, the gigabit ethernet PLL would
be disabled after macb_probe(). During this period of time, the system
would hang up if we try to access GEMGXL control registers.
We can't put runtime_pm_get/runtime_pm_put/ there due to the issue of
sleep inside atomic section (7fa2955ff70ce453 ("sh_eth: Fix sleeping
function called from invalid context"). Add the similar flag to ensure
the device is available before accessing GEMGXL device.
Signed-off-by: Zong Li <zong.li@sifive.com>
---
drivers/net/ethernet/cadence/macb.h | 2 ++
drivers/net/ethernet/cadence/macb_main.c | 7 +++++++
2 files changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index d8d87213697c..acf5242ce715 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -1309,6 +1309,8 @@ struct macb {
u32 rx_intr_mask;
+ unsigned int is_opened;
+
struct macb_pm_data pm_data;
const struct macb_usrio_config *usrio;
};
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6bc7d41d519b..e079ed10ad91 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -2781,6 +2781,8 @@ static int macb_open(struct net_device *dev)
if (bp->ptp_info)
bp->ptp_info->ptp_init(dev);
+ bp->is_opened = 1;
+
return 0;
reset_hw:
@@ -2818,6 +2820,8 @@ static int macb_close(struct net_device *dev)
if (bp->ptp_info)
bp->ptp_info->ptp_remove(dev);
+ bp->is_opened = 0;
+
pm_runtime_put(&bp->pdev->dev);
return 0;
@@ -2867,6 +2871,9 @@ static struct net_device_stats *gem_get_stats(struct macb *bp)
struct gem_stats *hwstat = &bp->hw_stats.gem;
struct net_device_stats *nstat = &bp->dev->stats;
+ if (!bp->is_opened)
+ return nstat;
+
gem_update_stats(bp);
nstat->rx_errors = (hwstat->rx_frame_check_sequence_errors +
--
2.31.1
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] net: macb: ensure the device is available before accessing GEMGXL control registers
2021-05-21 12:48 ` Zong Li
@ 2021-05-21 16:51 ` Florian Fainelli
-1 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2021-05-21 16:51 UTC (permalink / raw)
To: Zong Li, nicolas.ferre, claudiu.beznea, davem, kuba, palmer,
paul.walmsley, schwab, sboyd, aou, mturquette, geert, yixun.lan,
netdev, linux-kernel, linux-riscv
On 5/21/2021 5:48 AM, Zong Li wrote:
> If runtime power menagement is enabled, the gigabit ethernet PLL would
> be disabled after macb_probe(). During this period of time, the system
> would hang up if we try to access GEMGXL control registers.
>
> We can't put runtime_pm_get/runtime_pm_put/ there due to the issue of
> sleep inside atomic section (7fa2955ff70ce453 ("sh_eth: Fix sleeping
> function called from invalid context"). Add the similar flag to ensure
> the device is available before accessing GEMGXL device.
>
> Signed-off-by: Zong Li <zong.li@sifive.com>
> ---
> drivers/net/ethernet/cadence/macb.h | 2 ++
> drivers/net/ethernet/cadence/macb_main.c | 7 +++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index d8d87213697c..acf5242ce715 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -1309,6 +1309,8 @@ struct macb {
>
> u32 rx_intr_mask;
>
> + unsigned int is_opened;
> +
> struct macb_pm_data pm_data;
> const struct macb_usrio_config *usrio;
> };
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 6bc7d41d519b..e079ed10ad91 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2781,6 +2781,8 @@ static int macb_open(struct net_device *dev)
> if (bp->ptp_info)
> bp->ptp_info->ptp_init(dev);
>
> + bp->is_opened = 1;
> +
> return 0;
>
> reset_hw:
> @@ -2818,6 +2820,8 @@ static int macb_close(struct net_device *dev)
> if (bp->ptp_info)
> bp->ptp_info->ptp_remove(dev);
>
> + bp->is_opened = 0;
> +
> pm_runtime_put(&bp->pdev->dev);
>
> return 0;
> @@ -2867,6 +2871,9 @@ static struct net_device_stats *gem_get_stats(struct macb *bp)
> struct gem_stats *hwstat = &bp->hw_stats.gem;
> struct net_device_stats *nstat = &bp->dev->stats;
>
> + if (!bp->is_opened)
> + return nstat;
The canonical way to do this check is to use netif_running(), and not
open code a boolean tracking whether a network device is opened or not.
--
Florian
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: macb: ensure the device is available before accessing GEMGXL control registers
@ 2021-05-21 16:51 ` Florian Fainelli
0 siblings, 0 replies; 6+ messages in thread
From: Florian Fainelli @ 2021-05-21 16:51 UTC (permalink / raw)
To: Zong Li, nicolas.ferre, claudiu.beznea, davem, kuba, palmer,
paul.walmsley, schwab, sboyd, aou, mturquette, geert, yixun.lan,
netdev, linux-kernel, linux-riscv
On 5/21/2021 5:48 AM, Zong Li wrote:
> If runtime power menagement is enabled, the gigabit ethernet PLL would
> be disabled after macb_probe(). During this period of time, the system
> would hang up if we try to access GEMGXL control registers.
>
> We can't put runtime_pm_get/runtime_pm_put/ there due to the issue of
> sleep inside atomic section (7fa2955ff70ce453 ("sh_eth: Fix sleeping
> function called from invalid context"). Add the similar flag to ensure
> the device is available before accessing GEMGXL device.
>
> Signed-off-by: Zong Li <zong.li@sifive.com>
> ---
> drivers/net/ethernet/cadence/macb.h | 2 ++
> drivers/net/ethernet/cadence/macb_main.c | 7 +++++++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> index d8d87213697c..acf5242ce715 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -1309,6 +1309,8 @@ struct macb {
>
> u32 rx_intr_mask;
>
> + unsigned int is_opened;
> +
> struct macb_pm_data pm_data;
> const struct macb_usrio_config *usrio;
> };
> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 6bc7d41d519b..e079ed10ad91 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -2781,6 +2781,8 @@ static int macb_open(struct net_device *dev)
> if (bp->ptp_info)
> bp->ptp_info->ptp_init(dev);
>
> + bp->is_opened = 1;
> +
> return 0;
>
> reset_hw:
> @@ -2818,6 +2820,8 @@ static int macb_close(struct net_device *dev)
> if (bp->ptp_info)
> bp->ptp_info->ptp_remove(dev);
>
> + bp->is_opened = 0;
> +
> pm_runtime_put(&bp->pdev->dev);
>
> return 0;
> @@ -2867,6 +2871,9 @@ static struct net_device_stats *gem_get_stats(struct macb *bp)
> struct gem_stats *hwstat = &bp->hw_stats.gem;
> struct net_device_stats *nstat = &bp->dev->stats;
>
> + if (!bp->is_opened)
> + return nstat;
The canonical way to do this check is to use netif_running(), and not
open code a boolean tracking whether a network device is opened or not.
--
Florian
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: macb: ensure the device is available before accessing GEMGXL control registers
2021-05-21 16:51 ` Florian Fainelli
@ 2021-05-22 9:08 ` Zong Li
-1 siblings, 0 replies; 6+ messages in thread
From: Zong Li @ 2021-05-22 9:08 UTC (permalink / raw)
To: Florian Fainelli
Cc: nicolas.ferre, claudiu.beznea, davem, kuba, Palmer Dabbelt,
Paul Walmsley, Andreas Schwab, Stephen Boyd, Albert Ou,
Michael Turquette, geert, Yixun Lan, netdev,
linux-kernel@vger.kernel.org List, linux-riscv
On Sat, May 22, 2021 at 12:51 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 5/21/2021 5:48 AM, Zong Li wrote:
> > If runtime power menagement is enabled, the gigabit ethernet PLL would
> > be disabled after macb_probe(). During this period of time, the system
> > would hang up if we try to access GEMGXL control registers.
> >
> > We can't put runtime_pm_get/runtime_pm_put/ there due to the issue of
> > sleep inside atomic section (7fa2955ff70ce453 ("sh_eth: Fix sleeping
> > function called from invalid context"). Add the similar flag to ensure
> > the device is available before accessing GEMGXL device.
> >
> > Signed-off-by: Zong Li <zong.li@sifive.com>
> > ---
> > drivers/net/ethernet/cadence/macb.h | 2 ++
> > drivers/net/ethernet/cadence/macb_main.c | 7 +++++++
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> > index d8d87213697c..acf5242ce715 100644
> > --- a/drivers/net/ethernet/cadence/macb.h
> > +++ b/drivers/net/ethernet/cadence/macb.h
> > @@ -1309,6 +1309,8 @@ struct macb {
> >
> > u32 rx_intr_mask;
> >
> > + unsigned int is_opened;
> > +
> > struct macb_pm_data pm_data;
> > const struct macb_usrio_config *usrio;
> > };
> > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> > index 6bc7d41d519b..e079ed10ad91 100644
> > --- a/drivers/net/ethernet/cadence/macb_main.c
> > +++ b/drivers/net/ethernet/cadence/macb_main.c
> > @@ -2781,6 +2781,8 @@ static int macb_open(struct net_device *dev)
> > if (bp->ptp_info)
> > bp->ptp_info->ptp_init(dev);
> >
> > + bp->is_opened = 1;
> > +
> > return 0;
> >
> > reset_hw:
> > @@ -2818,6 +2820,8 @@ static int macb_close(struct net_device *dev)
> > if (bp->ptp_info)
> > bp->ptp_info->ptp_remove(dev);
> >
> > + bp->is_opened = 0;
> > +
> > pm_runtime_put(&bp->pdev->dev);
> >
> > return 0;
> > @@ -2867,6 +2871,9 @@ static struct net_device_stats *gem_get_stats(struct macb *bp)
> > struct gem_stats *hwstat = &bp->hw_stats.gem;
> > struct net_device_stats *nstat = &bp->dev->stats;
> >
> > + if (!bp->is_opened)
> > + return nstat;
>
> The canonical way to do this check is to use netif_running(), and not
> open code a boolean tracking whether a network device is opened or not.
Yes, I have tried this and it worked. Let me change it in the next version.
> --
> Florian
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] net: macb: ensure the device is available before accessing GEMGXL control registers
@ 2021-05-22 9:08 ` Zong Li
0 siblings, 0 replies; 6+ messages in thread
From: Zong Li @ 2021-05-22 9:08 UTC (permalink / raw)
To: Florian Fainelli
Cc: nicolas.ferre, claudiu.beznea, davem, kuba, Palmer Dabbelt,
Paul Walmsley, Andreas Schwab, Stephen Boyd, Albert Ou,
Michael Turquette, geert, Yixun Lan, netdev,
linux-kernel@vger.kernel.org List, linux-riscv
On Sat, May 22, 2021 at 12:51 AM Florian Fainelli <f.fainelli@gmail.com> wrote:
>
>
>
> On 5/21/2021 5:48 AM, Zong Li wrote:
> > If runtime power menagement is enabled, the gigabit ethernet PLL would
> > be disabled after macb_probe(). During this period of time, the system
> > would hang up if we try to access GEMGXL control registers.
> >
> > We can't put runtime_pm_get/runtime_pm_put/ there due to the issue of
> > sleep inside atomic section (7fa2955ff70ce453 ("sh_eth: Fix sleeping
> > function called from invalid context"). Add the similar flag to ensure
> > the device is available before accessing GEMGXL device.
> >
> > Signed-off-by: Zong Li <zong.li@sifive.com>
> > ---
> > drivers/net/ethernet/cadence/macb.h | 2 ++
> > drivers/net/ethernet/cadence/macb_main.c | 7 +++++++
> > 2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
> > index d8d87213697c..acf5242ce715 100644
> > --- a/drivers/net/ethernet/cadence/macb.h
> > +++ b/drivers/net/ethernet/cadence/macb.h
> > @@ -1309,6 +1309,8 @@ struct macb {
> >
> > u32 rx_intr_mask;
> >
> > + unsigned int is_opened;
> > +
> > struct macb_pm_data pm_data;
> > const struct macb_usrio_config *usrio;
> > };
> > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> > index 6bc7d41d519b..e079ed10ad91 100644
> > --- a/drivers/net/ethernet/cadence/macb_main.c
> > +++ b/drivers/net/ethernet/cadence/macb_main.c
> > @@ -2781,6 +2781,8 @@ static int macb_open(struct net_device *dev)
> > if (bp->ptp_info)
> > bp->ptp_info->ptp_init(dev);
> >
> > + bp->is_opened = 1;
> > +
> > return 0;
> >
> > reset_hw:
> > @@ -2818,6 +2820,8 @@ static int macb_close(struct net_device *dev)
> > if (bp->ptp_info)
> > bp->ptp_info->ptp_remove(dev);
> >
> > + bp->is_opened = 0;
> > +
> > pm_runtime_put(&bp->pdev->dev);
> >
> > return 0;
> > @@ -2867,6 +2871,9 @@ static struct net_device_stats *gem_get_stats(struct macb *bp)
> > struct gem_stats *hwstat = &bp->hw_stats.gem;
> > struct net_device_stats *nstat = &bp->dev->stats;
> >
> > + if (!bp->is_opened)
> > + return nstat;
>
> The canonical way to do this check is to use netif_running(), and not
> open code a boolean tracking whether a network device is opened or not.
Yes, I have tried this and it worked. Let me change it in the next version.
> --
> Florian
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-05-22 9:09 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-21 12:48 [PATCH] net: macb: ensure the device is available before accessing GEMGXL control registers Zong Li
2021-05-21 12:48 ` Zong Li
2021-05-21 16:51 ` Florian Fainelli
2021-05-21 16:51 ` Florian Fainelli
2021-05-22 9:08 ` Zong Li
2021-05-22 9:08 ` Zong Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.