Re: [PATCH dev-4.7 2/2] net/faraday: Clear stale interrupts

[Gavin Shan <gwshan@linux.vnet.ibm.com>]

On Mon, Aug 01, 2016 at 05:56:01PM +0930, Joel Stanley wrote:
>On Thu, 2016-07-28 at 11:04 +1000, Gavin Shan wrote:
>> There is stale interrupt (PHYSTS_CHG in ISR, bit#6 in 0x0) from
>> the bootloader (uboot) when enabling the MAC. The stale interrupts
>> aren't part of kernel and should be cleared.
>> 
>> This clears the stale interrupts in ISR (0x0) when enabling the MAC.
>
>Thanks Gavin. A question below.
>
>> 
>> Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
>> ---
>>  drivers/net/ethernet/faraday/ftgmac100.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>> 
>> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c
>> b/drivers/net/ethernet/faraday/ftgmac100.c
>> index f00911a..fc2852f 100644
>> --- a/drivers/net/ethernet/faraday/ftgmac100.c
>> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
>> @@ -1119,6 +1119,7 @@ static int ftgmac100_open(struct net_device
>> *netdev)
>>  {
>>  	struct ftgmac100 *priv = netdev_priv(netdev);
>>  	struct phy_device *phydev = netdev->phydev;
>> +	unsigned int status;
>>  	int err;
>>  
>>  	err = ftgmac100_alloc_buffers(priv);
>> @@ -1145,6 +1146,10 @@ static int ftgmac100_open(struct net_device
>> *netdev)
>>  	ftgmac100_init_hw(priv);
>>  	ftgmac100_start_hw(priv, priv->use_ncsi ? 100 : 10);
>>  
>> +	/* Clear stale interrupts */
>> +	status = ioread32(priv->base + FTGMAC100_OFFSET_ISR);
>> +	iowrite32(status, priv->base + FTGMAC100_OFFSET_ISR);
>
>By the time we get down here we've called ftgmac100_reset_hw. This has
>the following:
>
>/* NOTE: reset clears all registers */
>iowrite32(FTGMAC100_MACCR_SW_RST, priv->base + FTGMAC100_OFFSET_MACCR); 
>
>We are seeing that it does not actually clear all the registers?
>

Yep, it clears all registers including ISR/IER/MACCR, meaning MACCR[11]
is cleared. ISR[9] (PHYSTS_CHG) is set right after it even IER is cleared
that time. So ISR[9] cannot be cleared if MACCR[11] isn't set. Note the
MACCR[11] is set in PATCH[1/2] in ftgmac100_start_hw(). It also means
ISR[9] cannot be cleared successfully until ftgmac100_start_hw() sets
MACCR[11].

>Secondly, if we're seeing stale interrupts is there a risk that they
>will fire again between now and the call below where we
>set FTGMAC100_OFFSET_IER?
>
>Should we be masking them all off, clearing the stale ones, and then
>re-enabling them?
>

IER is cleared by reset done in ftgmac100_reset_hw(), meaning all interrupts
are disabled that time. We're clearing the stale interrupts (mainly ISR[9] -
PHYSTS_CHG) before the interrupts are enabled.

Thanks,
Gavin

>Cheers,
>
>Joel
>
>
>> +
>>  	if (phydev)
>>  		phy_start(phydev);
>>  	else if (priv->use_ncsi)
>