From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42971) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WtcCp-0005LT-Mw for qemu-devel@nongnu.org; Sun, 08 Jun 2014 08:28:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WtcCk-000860-78 for qemu-devel@nongnu.org; Sun, 08 Jun 2014 08:28:11 -0400 Received: from mail-lb0-f172.google.com ([209.85.217.172]:48741) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WtcCj-00085k-W3 for qemu-devel@nongnu.org; Sun, 08 Jun 2014 08:28:06 -0400 Received: by mail-lb0-f172.google.com with SMTP id l4so2478546lbv.17 for ; Sun, 08 Jun 2014 05:28:05 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <539449AE.4090909@msgid.tls.msk.ru> References: <1402159924-13853-1-git-send-email-peter.maydell@linaro.org> <539449AE.4090909@msgid.tls.msk.ru> From: Peter Maydell Date: Sun, 8 Jun 2014 13:27:44 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [Qemu-trivial] [PATCH] hw/net/eepro100: Implement read-only bits in MDI registers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: QEMU Trivial , QEMU Developers , Patch Tracking On 8 June 2014 12:31, Michael Tokarev wrote: > 07.06.2014 20:52, Peter Maydell wrote: >> Although we defined an eepro100_mdi_mask[] array indicating which bits >> in the registers are read-only, we weren't actually doing anything with >> it. Make the MDI register-read code use it rather than manually making >> registers 2 and 3 totally read-only and the rest totally read-write. > > s/register-read/register-write/ -- can be fixed when applying. > > I'm not sure this is "trivial enough", because the side effect is > not obvious, at least not to someone not familiar with eepro100 > registers and their usage. Mmm, but do you want to suggest a better queue? I did check that Linux could still boot and talk to the network via an eepro100. > Besides, the description does not seem to be very accurate too. > From the code I see that the original code makes register 0 > "semi-writable", register 1 is unwritable and the rest fully > writable. Yes, that was wrongly worded. You're correct that it's just that 1 is unwritable in the original code. > In this context, apparently we're losing the ability to write to > register 0 completely, since its mask is 0 but the original code > allows writing something to it. Hmm? The mask is a mask of read-only bits, so if mask is zero then ANDing the register with the mask will clear it, ANDing the data with ~mask will do nothing, and then ORing the data into the register means we set every bit in the register. (This all happens after the register-specific case code, so the work that does to have some of the bits have special effects by changing the value of 'data' remains in place.) > Also, maybe updating the "missing()" calls according to the > bitmask is a good idea... That seems like a separate thing; there's a lot of missing behaviour here, I suspect. thanks -- PMM