On Thu, Aug 27, 2015 at 7:28 AM, <ygardi@xxxxxxxxxxxxxx> wrote: >>> On Aug 21, 2015 3:10 PM, "Yaniv Gardi" <ygardi@xxxxxxxxxxxxxx> wrote: >>>> >>>> Add a write memory barrier to make sure descriptors prepared are >>>> actually >>>> written to memory before ringing the doorbell. We have also added the >>>> write memory barrier after ringing the doorbell register so that >>>> controller sees the new request immediately. [...] >>>> + /* Make sure descriptors are ready before ringing the doorbell >>>> */ >>>> + wmb(); >>> >>> The writel for the doorbell will do a barrier first. (I didn't check >>> what exactly ufshcd_writel does, but that is why I don't like these >>> private access wrappers.) > > the barrier here is important and a must. > before it we prepare descriptors. > after it we write to DOOR-BELL. > (and after the DOOR-BELL we have another one.) > if we remove it, we might get the DOOR-BELL written, before the > descriptors written. If you dig into what writel does, you will see that what the code here ends up being is: descriptor setup wmb() __iomwb() -> wmb on arm64 doorbell register write So explain why you need 2 barriers. Barriers in a driver are a red flag. Usually they are not needed, but sometimes they are. You have to be able to explain why if they are. Pretty much every descriptor based DMA device works as you describe and has the ordering problem. Because of that, the core code takes care of this. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
