* [PATCH] net: bcmgenet: Add MDIO_INTR in GENETv2
@ 2016-02-15 8:42 Jaedon Shin
2016-02-15 18:18 ` Florian Fainelli
0 siblings, 1 reply; 4+ messages in thread
From: Jaedon Shin @ 2016-02-15 8:42 UTC (permalink / raw)
To: netdev; +Cc: Florian Fainelli, David Miller, Petri Gynther, Jaedon Shin
The GENETv2 chipsets has MDIO interrupt like the GENETv3+ chipsets.
The previous commit d5c3d84657db ("net: phy: Avoid polling PHY with
PHY_IGNORE_INTERRUPTS") and commit 49f7a471e4d1 ("net: bcmgenet: Properly
configure PHY to ignore interrupt") cause link-down PHY always in some
40nm generation chipsets.
Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++--
drivers/net/ethernet/broadcom/genet/bcmgenet.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index b15a60d787c7..8e9aa8f6390d 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -1904,7 +1904,7 @@ static int init_umac(struct bcmgenet_priv *priv)
bcmgenet_bp_mc_set(priv, reg);
}
- /* Enable MDIO interrupts on GENET v3+ */
+ /* Enable MDIO interrupts on GENET v2+ */
if (priv->hw_params->flags & GENET_HAS_MDIO_INTR)
int0_enable |= (UMAC_IRQ_MDIO_DONE | UMAC_IRQ_MDIO_ERROR);
@@ -3168,7 +3168,7 @@ static struct bcmgenet_hw_params bcmgenet_hw_params[] = {
.rdma_offset = 0x3000,
.tdma_offset = 0x4000,
.words_per_bd = 2,
- .flags = GENET_HAS_EXT,
+ .flags = GENET_HAS_EXT | GENET_HAS_MDIO_INTR,
},
[GENET_V3] = {
.tx_queues = 4,
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
index 967367557309..c14bfbfbe06a 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
@@ -310,7 +310,7 @@ struct bcmgenet_mib_counters {
#define UMAC_IRQ_TXDMA_BDONE (1 << 18)
#define UMAC_IRQ_TXDMA_DONE UMAC_IRQ_TXDMA_MBDONE
-/* Only valid for GENETv3+ */
+/* Only valid for GENETv2+ */
#define UMAC_IRQ_MDIO_DONE (1 << 23)
#define UMAC_IRQ_MDIO_ERROR (1 << 24)
--
2.7.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: bcmgenet: Add MDIO_INTR in GENETv2
2016-02-15 8:42 [PATCH] net: bcmgenet: Add MDIO_INTR in GENETv2 Jaedon Shin
@ 2016-02-15 18:18 ` Florian Fainelli
2016-02-16 3:12 ` Jaedon Shin
0 siblings, 1 reply; 4+ messages in thread
From: Florian Fainelli @ 2016-02-15 18:18 UTC (permalink / raw)
To: Jaedon Shin, netdev; +Cc: David Miller, Petri Gynther, opendmb
Hi Jaedon,
On 15/02/2016 00:42, Jaedon Shin wrote:
> The GENETv2 chipsets has MDIO interrupt like the GENETv3+ chipsets.
>
> The previous commit d5c3d84657db ("net: phy: Avoid polling PHY with
> PHY_IGNORE_INTERRUPTS") and commit 49f7a471e4d1 ("net: bcmgenet: Properly
> configure PHY to ignore interrupt") cause link-down PHY always in some
> 40nm generation chipsets.
Humm, these are two different things here:
- GENET_HAS_MDIO_INTR is about telling the driver whether the hardware
supports MDIO_INTR_DONE and MDIO_INTR_ERROR
- eliminating PHY polling is about utilizing LINK_UP and LINK_DOWN to
avoid polling the PHY
So the original problem is actually here:
bcmgenet_irq_task():
/* Link UP/DOWN event */
if ((priv->hw_params->flags & GENET_HAS_MDIO_INTR) &&
(priv->irq0_stat & UMAC_IRQ_LINK_EVENT)) {
These two checks are actually orthogonal, so we should remove the first
check on GENET_HAS_MDIO_INTR.
Your patch remains valid though, just the explanation needs a bit
tweaking, thanks!
>
> Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com>
> ---
> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++--
> drivers/net/ethernet/broadcom/genet/bcmgenet.h | 2 +-
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> index b15a60d787c7..8e9aa8f6390d 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
> @@ -1904,7 +1904,7 @@ static int init_umac(struct bcmgenet_priv *priv)
> bcmgenet_bp_mc_set(priv, reg);
> }
>
> - /* Enable MDIO interrupts on GENET v3+ */
> + /* Enable MDIO interrupts on GENET v2+ */
> if (priv->hw_params->flags & GENET_HAS_MDIO_INTR)
> int0_enable |= (UMAC_IRQ_MDIO_DONE | UMAC_IRQ_MDIO_ERROR);
>
> @@ -3168,7 +3168,7 @@ static struct bcmgenet_hw_params bcmgenet_hw_params[] = {
> .rdma_offset = 0x3000,
> .tdma_offset = 0x4000,
> .words_per_bd = 2,
> - .flags = GENET_HAS_EXT,
> + .flags = GENET_HAS_EXT | GENET_HAS_MDIO_INTR,
> },
> [GENET_V3] = {
> .tx_queues = 4,
> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> index 967367557309..c14bfbfbe06a 100644
> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
> @@ -310,7 +310,7 @@ struct bcmgenet_mib_counters {
> #define UMAC_IRQ_TXDMA_BDONE (1 << 18)
> #define UMAC_IRQ_TXDMA_DONE UMAC_IRQ_TXDMA_MBDONE
>
> -/* Only valid for GENETv3+ */
> +/* Only valid for GENETv2+ */
> #define UMAC_IRQ_MDIO_DONE (1 << 23)
> #define UMAC_IRQ_MDIO_ERROR (1 << 24)
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: bcmgenet: Add MDIO_INTR in GENETv2
2016-02-15 18:18 ` Florian Fainelli
@ 2016-02-16 3:12 ` Jaedon Shin
2016-02-19 5:47 ` Florian Fainelli
0 siblings, 1 reply; 4+ messages in thread
From: Jaedon Shin @ 2016-02-16 3:12 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, David Miller, Petri Gynther, opendmb
Hi Florian,
> On Feb 16, 2016, at 3:18 AM, Florian Fainelli <f.fainelli@gmail.com> wrote:
>
> Hi Jaedon,
>
> On 15/02/2016 00:42, Jaedon Shin wrote:
>> The GENETv2 chipsets has MDIO interrupt like the GENETv3+ chipsets.
>>
>> The previous commit d5c3d84657db ("net: phy: Avoid polling PHY with
>> PHY_IGNORE_INTERRUPTS") and commit 49f7a471e4d1 ("net: bcmgenet: Properly
>> configure PHY to ignore interrupt") cause link-down PHY always in some
>> 40nm generation chipsets.
>
> Humm, these are two different things here:
>
> - GENET_HAS_MDIO_INTR is about telling the driver whether the hardware
> supports MDIO_INTR_DONE and MDIO_INTR_ERROR
> - eliminating PHY polling is about utilizing LINK_UP and LINK_DOWN to
> avoid polling the PHY
>
> So the original problem is actually here:
>
> bcmgenet_irq_task():
>
> /* Link UP/DOWN event */
> if ((priv->hw_params->flags & GENET_HAS_MDIO_INTR) &&
> (priv->irq0_stat & UMAC_IRQ_LINK_EVENT)) {
>
> These two checks are actually orthogonal, so we should remove the first
> check on GENET_HAS_MDIO_INTR.
>
As you said, the part in bcmgenet_irq_task() is a problem.
The bcmgenet using internal PHY should use phy_mac_interrupt() cause it has not
PHY_POLL, and it depends on Ethernet MAC ISR.
UMAC_IRQ_LINK_EVENT(LINK_UP and LINK_DOWN) was working correctly in GENETv2,
but (priv->hw_params->flags & GENET_HAS_MDIO_INTR) was blocking to call
phy_mac_interrupt(). I didn't find a reason through datasheet without MDIO_INTR
in GENETv2. However, I'm not sure using MDIO_INTR.
Therefore if MDIO_INTR is not valid in GENETv2, I will send the patch again
to remove the first chicken GENET_HAS_MDIO_INTR after your confirm.
Thanks,
Jaedon
> Your patch remains valid though, just the explanation needs a bit
> tweaking, thanks!
>
>>
>> Signed-off-by: Jaedon Shin <jaedon.shin@gmail.com>
>> ---
>> drivers/net/ethernet/broadcom/genet/bcmgenet.c | 4 ++--
>> drivers/net/ethernet/broadcom/genet/bcmgenet.h | 2 +-
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> index b15a60d787c7..8e9aa8f6390d 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
>> @@ -1904,7 +1904,7 @@ static int init_umac(struct bcmgenet_priv *priv)
>> bcmgenet_bp_mc_set(priv, reg);
>> }
>>
>> - /* Enable MDIO interrupts on GENET v3+ */
>> + /* Enable MDIO interrupts on GENET v2+ */
>> if (priv->hw_params->flags & GENET_HAS_MDIO_INTR)
>> int0_enable |= (UMAC_IRQ_MDIO_DONE | UMAC_IRQ_MDIO_ERROR);
>>
>> @@ -3168,7 +3168,7 @@ static struct bcmgenet_hw_params bcmgenet_hw_params[] = {
>> .rdma_offset = 0x3000,
>> .tdma_offset = 0x4000,
>> .words_per_bd = 2,
>> - .flags = GENET_HAS_EXT,
>> + .flags = GENET_HAS_EXT | GENET_HAS_MDIO_INTR,
>> },
>> [GENET_V3] = {
>> .tx_queues = 4,
>> diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.h b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
>> index 967367557309..c14bfbfbe06a 100644
>> --- a/drivers/net/ethernet/broadcom/genet/bcmgenet.h
>> +++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.h
>> @@ -310,7 +310,7 @@ struct bcmgenet_mib_counters {
>> #define UMAC_IRQ_TXDMA_BDONE (1 << 18)
>> #define UMAC_IRQ_TXDMA_DONE UMAC_IRQ_TXDMA_MBDONE
>>
>> -/* Only valid for GENETv3+ */
>> +/* Only valid for GENETv2+ */
>> #define UMAC_IRQ_MDIO_DONE (1 << 23)
>> #define UMAC_IRQ_MDIO_ERROR (1 << 24)
>>
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: bcmgenet: Add MDIO_INTR in GENETv2
2016-02-16 3:12 ` Jaedon Shin
@ 2016-02-19 5:47 ` Florian Fainelli
0 siblings, 0 replies; 4+ messages in thread
From: Florian Fainelli @ 2016-02-19 5:47 UTC (permalink / raw)
To: Jaedon Shin; +Cc: netdev, David Miller, Petri Gynther, opendmb
Hi Jaedon
Le 15/02/2016 19:12, Jaedon Shin a écrit :
>
> As you said, the part in bcmgenet_irq_task() is a problem.
>
> The bcmgenet using internal PHY should use phy_mac_interrupt() cause it has not
> PHY_POLL, and it depends on Ethernet MAC ISR.
>
> UMAC_IRQ_LINK_EVENT(LINK_UP and LINK_DOWN) was working correctly in GENETv2,
> but (priv->hw_params->flags & GENET_HAS_MDIO_INTR) was blocking to call
> phy_mac_interrupt(). I didn't find a reason through datasheet without MDIO_INTR
> in GENETv2. However, I'm not sure using MDIO_INTR.
>
> Therefore if MDIO_INTR is not valid in GENETv2, I will send the patch again
> to remove the first chicken GENET_HAS_MDIO_INTR after your confirm.
MDIO interrupts are wired in GENETv2, so the second part of your patch
is correct, I see now that you have submitted a proper fix for the Link
UP/DOWN event condition, and thank you for doing that. FWIW, all GENET
versions have link UP/DOWN interrupts.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-19 5:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-02-15 8:42 [PATCH] net: bcmgenet: Add MDIO_INTR in GENETv2 Jaedon Shin
2016-02-15 18:18 ` Florian Fainelli
2016-02-16 3:12 ` Jaedon Shin
2016-02-19 5:47 ` Florian Fainelli
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).