[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20140624.161241.1533958645479291631.davem@davemloft.net>
Date: Tue, 24 Jun 2014 16:12:41 -0700 (PDT)
From: David Miller <davem@...emloft.net>
To: f.fainelli@...il.com
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net 2/2] net: systemport: fix UniMAC reset logic
From: Florian Fainelli <f.fainelli@...il.com>
Date: Fri, 20 Jun 2014 13:01:40 -0700
> The UniMAC CMD_SW_RESET bit is not a self-clearing bit, so we need to
> assert it, wait a bit and clear it manually. As a result, umac_reset()
> is updated not to return any value.
>
> By setting CMD_SW_RESET and only that bit, we were also clearing the
> other 31 bits, overriding the hardware reset defaults which are
> correctly set on purpose.
>
> Signed-off-by: Florian Fainelli <f.fainelli@...il.com>
I agree with Sergei, this is really suspicious that the existing code
never wrote the sw reset bit into the register at all, it just writes
a zero.
And you don't mention this at all in this commit message.
If I were to guess, I would fathom that the sequence was partially
copied from the other broadcom driver, the genet one. There it
issues CMD_SW_RESET by first clearing the entire regiser to zero,
then writing CMD_SW_RESET, and then polling for that bit to clear.
I'm assuming that is being done for a good reason, and I'd like
you to make sure that same reason does not apply here.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists