On 8 June 2014 12:31, Michael Tokarev <m...@tls.msk.ru> 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