Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <2ef12e328ecdc411e7d145a331d7c8ce668bf2be.camel@gmail.com>
Subject: Re: [PATCH v2 2/3] scsi: ufs: Keep device active mode only
 fWriteBoosterBufferFlushDuringHibernate == 1
From:   Bean Huo <huobean@gmail.com>
To:     Avri Altman <Avri.Altman@wdc.com>,
        "alim.akhtar@samsung.com" <alim.akhtar@samsung.com>,
        "asutoshd@codeaurora.org" <asutoshd@codeaurora.org>,
        "jejb@linux.ibm.com" <jejb@linux.ibm.com>,
        "martin.petersen@oracle.com" <martin.petersen@oracle.com>,
        "stanley.chu@mediatek.com" <stanley.chu@mediatek.com>,
        "beanhuo@micron.com" <beanhuo@micron.com>,
        "bvanassche@acm.org" <bvanassche@acm.org>,
        "tomas.winkler@intel.com" <tomas.winkler@intel.com>,
        "cang@codeaurora.org" <cang@codeaurora.org>
Cc:     "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Date:   Mon, 07 Dec 2020 12:18:10 +0100
In-Reply-To: <DM6PR04MB6575AED736BE44CA8D4B8998FCCE0@DM6PR04MB6575.namprd04.prod.outlook.com>
References: <20201206101335.3418-1-huobean@gmail.com>
         <20201206101335.3418-3-huobean@gmail.com>
         <DM6PR04MB6575AED736BE44CA8D4B8998FCCE0@DM6PR04MB6575.namprd04.prod.outlook.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Mon, 2020-12-07 at 08:02 +0000, Avri Altman wrote:
> > According to the JEDEC UFS 3.1 Spec, If
> > fWriteBoosterBufferFlushDuringHibernate
> > is set to one, the device flushes the WriteBooster Buffer data
> > automatically
> > whenever the link enters the hibernate (HIBERN8) state. While the
> > flushing
> > operation is in progress, the device should be kept in Active power
> > mode.
> > Currently, we set this flag during the UFSHCD probe stage, but we
> > didn't deal
> > with its programming failure. Even this failure is less likely to
> > occur, but
> > still it is possible.
> > This patch is to add checkup of
> > fWriteBoosterBufferFlushDuringHibernate
> > setting,
> > keep the device as "active power mode" only when this flag be
> > successfully
> > set
> > to 1.
> > 
> > Fixes: 51dd905bd2f6 ("scsi: ufs: Fix WriteBooster flush during
> > runtime
> > suspend")
> > Signed-off-by: Bean Huo <beanhuo@micron.com>
> 
> You've added the fixes tag,

yes,it is a bug.

>  but my previous comment is still unanswered:
> you are adding protection to a single device management command.
> Why this command in particular? 
> What makes it so special that it needs this extra care?
> 

see the Spec: 
"
If fWriteBoosterBufferFlushDuringHibernate is set to one, the device
flushes the WriteBooster Buffer data automatically whenever the link
enters the hibernate (HIBERN8) state.

The device shall stop the flushing operation if
fWriteBoosterBufferFlushDuringHibernate are set to zero.
....

"
If fWriteBoosterBufferFlushDuringHibernate ==0, device will not flush
WB, even if you keep device as "active mode" and LINK in hibernate
state.

Bean
Thanks,