On Fri, 16 Feb 2018 01:02:31 +0100, Lorenzo Bianconi wrote: > > On Thu, 15 Feb 2018 23:59:24 +0100, Lorenzo Bianconi wrote: > >> Introduce __mt7601u_rr and __mt7601u_vendor_single_wr routines in order > >> to make mt7601u_rmw and mt7601u_rmc atomic since it is possible that > >> read and write accesses of mt7601u_rmw/mt7601u_rmc can be interleaved > >> with a different write operation on the same register. > >> Moreover move write trace point in __mt7601u_vendor_single_wr > >> > >> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@xxxxxxxxxx> > > > > Could you provide an example of which accesses make it problematic? > > Is this fixing an actual bug? > > Hi Jakub, > > it is not an issue I had experimented, I noticed a theoretical race > reviewing the code. > AFAIU, based on the current implementation it is possible that mt7601u_rmw > (with mt7601u_rr) loads data from given register but its store access > (mt7601u_wr) is > preceded by another mt7601u_wr on the same register. In this case the > value configured by > the first mt7601u_wr executed is overwritten by the second one (the > store from mt7601u_rmw) > even if the first write is setting a different subfield respect to > mt7601u_rmw. Hm.. There should be no path in the driver where that could cause problems AFAIR. We have reg_atomic_mutex and hw_atomic_mutex to protect from concurrent access to the HW where they are possible. RMW operations are non-atomic by definition, it's supposed to work like PCIe register accesses would - 32bit reads/writes are atomic, but RMW is not. So I'm not sure what to do with this patch. Doesn't seem necessary...
