lkml.org 
[lkml]   [2018]   [Apr]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patches in this message
    /
    SubjectRe: [PATCH] net: phy: marvell: clear wol event before setting it
    From
    Date
    Hi,

    On 4/26/2018 11:56 AM, Bhadram Varka wrote:
    > Hi,
    > On 4/26/2018 11:45 AM, Jisheng Zhang wrote:
    >> Hi,
    >>
    >> On Thu, 26 Apr 2018 11:10:21 +0530 Bhadram Varka wrote:
    >>
    >>> Hi,
    >>>
    >>> On 4/19/2018 5:48 PM, Andrew Lunn wrote:
    >>>> On Thu, Apr 19, 2018 at 04:02:32PM +0800, Jisheng Zhang wrote:
    >>>>> From: Jingju Hou <Jingju.Hou@synaptics.com>
    >>>>>
    >>>>> If WOL event happened once, the LED[2] interrupt pin will not be
    >>>>> cleared unless reading the CSISR register. So clear the WOL event
    >>>>> before enabling it.
    >>>>>
    >>>>> Signed-off-by: Jingju Hou <Jingju.Hou@synaptics.com>
    >>>>> Signed-off-by: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
    >>>>> ---
    >>>>>    drivers/net/phy/marvell.c | 9 +++++++++
    >>>>>    1 file changed, 9 insertions(+)
    >>>>>
    >>>>> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
    >>>>> index c22e8e383247..b6abe1cbc84b 100644
    >>>>> --- a/drivers/net/phy/marvell.c
    >>>>> +++ b/drivers/net/phy/marvell.c
    >>>>> @@ -115,6 +115,9 @@
    >>>>>    /* WOL Event Interrupt Enable */
    >>>>>    #define MII_88E1318S_PHY_CSIER_WOL_EIE BIT(7)
    >>>>>    +/* Copper Specific Interrupt Status Register */
    >>>>> +#define MII_88E1318S_PHY_CSISR                0x13
    >>>>> +
    >>>>>    /* LED Timer Control Register */
    >>>>>    #define MII_88E1318S_PHY_LED_TCR            0x12
    >>>>>    #define MII_88E1318S_PHY_LED_TCR_FORCE_INT BIT(15)
    >>>>> @@ -1393,6 +1396,12 @@ static int m88e1318_set_wol(struct
    >>>>> phy_device *phydev,
    >>>>>            if (err < 0)
    >>>>>                goto error;
    >>>>>    +        /* If WOL event happened once, the LED[2] interrupt pin
    >>>>> +         * will not be cleared unless reading the CSISR register.
    >>>>> +         * So clear the WOL event first before enabling it.
    >>>>> +         */
    >>>>> +        phy_read(phydev, MII_88E1318S_PHY_CSISR);
    >>>>> +
    >>>> Hi Jisheng
    >>>>
    >>>> The problem with this is, you could be clearing a real interrupt, link
    >>>> down/up etc. If interrupts are in use, i think the normal interrupt
    >>>> handling will clear the WOL interrupt? So can you make this read
    >>>> conditional on !phy_interrupt_is_valid()?
    >>> So this will clear WoL interrupt bit from Copper Interrupt status
    >>> register.
    >>>
    >>> How about clearing WoL status (Page 17, register 17) for every WOL
    >>> event ?
    >>>
    >> This is already properly done by setting
    >> MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS
    >> in m88e1318_set_wol()
    > This part of the code executes only when we enable WOL through ethtool
    > (ethtool -s eth0 wol g)
    >
    > Lets say once WOL enabled through magic packet - HW generates WOL
    > interrupt once magic packet received.
    > The problem that I see here is that for the next immediate magic
    > packet I don't see WOL interrupt generated by the HW.
    > I need to explicitly clear WOL status for HW to generate WOL interrupt.

    With the below patch I see WOL event interrupt for every magic packet
    that HW receives...

    diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
    index ed8a67d..5d3d138 100644
    --- a/drivers/net/phy/marvell.c
    +++ b/drivers/net/phy/marvell.c
    @@ -55,6 +55,7 @@

     #define MII_M1011_IEVENT               0x13
     #define MII_M1011_IEVENT_CLEAR         0x0000
    +#define MII_M1011_IEVENT_WOL_EVENT     BIT(7)

     #define MII_M1011_IMASK                        0x12
    - #define MII_M1011_IMASK_INIT           0x6400
    + #define MII_M1011_IMASK_INIT           0x6480
    @@ -195,13 +196,40 @@ struct marvell_priv {
            bool copper;
     };

    +static int marvell_clear_wol_status(struct phy_device *phydev)
    +{
    +       int err, temp, oldpage;
    +
    +       oldpage = phy_read(phydev, MII_MARVELL_PHY_PAGE);
    +       if (oldpage < 0)
    +               return oldpage;
    +
    +       err = phy_write(phydev, MII_MARVELL_PHY_PAGE,
    +                        MII_88E1318S_PHY_WOL_PAGE);
    +       if (err < 0)
    +               return err;
    +
    +       /*
    +        * Clear WOL status so that for next WOL event
    +        * interrupt will be generated by HW
    +        */
    +       temp = phy_read(phydev, MII_88E1318S_PHY_WOL_CTRL);
    +       temp |= MII_88E1318S_PHY_WOL_CTRL_CLEAR_WOL_STATUS;
    +       err = phy_write(phydev, MII_88E1318S_PHY_WOL_CTRL, temp);
    +       if (err < 0)
    +               return err;
    +
    +
    +       phy_write(phydev, MII_MARVELL_PHY_PAGE, oldpage);
    +
    +       return 0;
    +}
    +
     static int marvell_ack_interrupt(struct phy_device *phydev)
     {
            int err;

            /* Clear the interrupts by reading the reg */
            err = phy_read(phydev, MII_M1011_IEVENT);
    -
            if (err < 0)
                    return err;

    @@ -1454,12 +1482,18 @@ static int marvell_aneg_done(struct phy_device
    *phydev)

     static int m88e1121_did_interrupt(struct phy_device *phydev)
     {
    -       int imask;
    +       int imask, err;

            imask = phy_read(phydev, MII_M1011_IEVENT);

    -       if (imask & MII_M1011_IMASK_INIT)
    +       if (imask & MII_M1011_IMASK_INIT) {
    +               if (imask & MII_M1011_IEVENT_WOL_EVENT) {
    +                       err = marvell_clear_wol_status(phydev);
    +                       if (err < 0)
    +                               return 0;
    +               }
                    return 1;
    +       }

            return 0;
     }
    \
     
     \ /
      Last update: 2018-04-26 09:10    [W:7.626 / U:0.052 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site