Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753134AbbH0RDJ (ORCPT ); Thu, 27 Aug 2015 13:03:09 -0400 Received: from mail-yk0-f172.google.com ([209.85.160.172]:36313 "EHLO mail-yk0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751700AbbH0RDG (ORCPT ); Thu, 27 Aug 2015 13:03:06 -0400 MIME-Version: 1.0 In-Reply-To: References: <1440194989-28835-1-git-send-email-ygardi@codeaurora.org> <1440194989-28835-15-git-send-email-ygardi@codeaurora.org> <1408ea96f7b8f33aef73e0f84abd8792.squirrel@www.codeaurora.org> From: Rob Herring Date: Thu, 27 Aug 2015 12:02:46 -0500 Message-ID: Subject: Re: [PATCH v1 14/15] scsi: ufs: commit descriptors before setting the doorbell To: Yaniv Gardi Cc: linux-arm-msm , linux-scsi@vger.kernel.org, Dolev Raviv , Gilad Broner , linux-scsi-owner@vger.kernel.org, Jej B , Santosh Y , Subhash Jadavani , hch@infradead.org, Paul Bolle , "James E.J. Bottomley" , Vinayak Holikatti , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1707 Lines: 48 On Thu, Aug 27, 2015 at 7:28 AM, wrote: >>> On Aug 21, 2015 3:10 PM, "Yaniv Gardi" 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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/