> On Thu, 2020-12-03 at 10:46 +0000, Avri Altman wrote: > > > > > From: Bean Huo <beanhuo@xxxxxxxxxx> > > > > > > > > > > Keep device power mode as active power mode and VCC supply only > > > > > if > > > > > fWriteBoosterBufferFlushDuringHibernate setting 1 is > > > > > successful. > > > > > > Hi Avri > > > Thanks so much taking time reiew. > > > > > > > Why would it fail? > > > > > > During the reliability testing in harsh environments, such as: > > > EMS testing, in the high/low-temperature environment. The system > > > would > > > reboot itself, there will be programming failure very likely. > > > If we assume failure will never hit, why we capture its result > > > following with dev_err(). If you keep using your phone in a harsh > > > environment, you will see this print message. > > > > > > Of course, in a normal environment, the chance of failure likes you > > > to > > > win a lottery, but the possibility still exists. > > > > Exactly. > > so, you agree the possiblity of failure exists. I was more relating to the lottery part. > > > Hence we need-not any extra logic protecting device management > > command failures. > > what extra logic? > > > > > if reading the configuration pass correctly, and UFSHCD_CAP_WB_EN is > > set, > > > UFSHCD_CAP_WB_EN set is DRAM level. still in the cache. > > > one should expect that any other functionality would work. > > > No, The programming will consume more power than reading, the > later setting will more possbile fail than reading. > > > Otherwise, any non-standard behavior should be added with a quirk. > > > > NO, this is not what is standard or non-standard. This is independent > of UFS device/controller. It is a software design. IMO, we didn't deal > with programming status that is a potential bug. If having to impose to > a component, do you think should be controller or device? Instead of > addin a quirk, I prefer dropping this patch. It seems you are adding some special treatment in case some device management command failed, A vanishingly unlikely event but a one that has significant impact over power consumption. If a device is not responding properly to some specific device management command, It should be treated accordingly. Thanks, Avri > > > > > > Thanks, > > Avri > > > > > > > > > > Since UFSHCD_CAP_WB_EN is toggled off on ufshcd_wb_probe If the > > > > device doesn't support wb, > > > > The check ufshcd_is_wb_allowed should suffice, isn't it? > > > > > > > > > > No, UFSHCD_CAP_WB_EN only tells us if the platform supports WB, > > > doesn't tell us fWriteBoosterBufferFlushDuringHibernate status. > > > > > > Thanks, > > > Bean
