Re: [PATCH v1 1/3] scsi: ufs: add write booster feature support

[Stanley Chu <stanley.chu@mediatek.com>]

Hi Kyuho,

On Wed, 2020-06-24 at 11:06 +0900, Kyuho Choi wrote:
> Hi Stanley,
> 
> On 6/24/20, Stanley Chu <stanley.chu@mediatek.com> wrote:
> > Hi Steev,
> >
> > On Tue, 2020-06-23 at 20:10 -0500, Steev Klimaszewski wrote:
> >> On 6/23/20 1:51 AM, Kyuho Choi wrote:
> >> > Hi Avri,
> >> >
> >> > On 6/23/20, Avri Altman <Avri.Altman@wdc.com> wrote:
> >> >>> AFAIK, this device are ufs 2.1. It's not support writebooster.
> >> >>>
> >> >>> I'd check latest linux scsi branch and ufshcd_wb_config function's
> >> >>> called without device capability check.
> >> >> Please grep ufshcd_wb_probe.
> >> >>
> >> > I got your point, but as I mentioned, this device not support wb, this
> >> > is old products.
> >> >
> >> > I'm not sure ufshcd_wb_probe are called or not in Rob and Steev's
> >> > platform.
> >> > If it's called, hba->caps are setted with wb diable and this error not
> >> > occured.
> >> > But (it looks) not called, same query error will be occured in
> >> > ufshcd_wb_config/ctrl.
> >> >
> >> > BR,
> >> > Kyuho Choi
> >>
> >> I do show ufshcd_wb_probe in my sources - I'm based on 5.8-rc2 with a
> >> few extra patches for the c630, and the inline encryption patches.
> >>
> >> I this is the output that I see -
> >>
> >>  1.
> >>     [    0.702501] ufshcd-qcom 1d84000.ufshc: ufshcd_populate_vreg:
> >>     Unable to find vdd-hba-supply regulator, assuming enabled
> >>  2.
> >>     [    0.702506] ufshcd-qcom 1d84000.ufshc: ufshcd_populate_vreg:
> >>     Unable to find vccq-supply regulator, assuming enabled
> >>  3.
> >>     [    0.702508] ufshcd-qcom 1d84000.ufshc: ufshcd_populate_vreg:
> >>     Unable to find vccq2-supply regulator, assuming enabled
> >>  4.
> >>     [    0.703296] ufshcd-qcom 1d84000.ufshc: Found QC Inline Crypto
> >>     Engine (ICE) v3.1.75
> >>  5.
> >>     [    0.705121] scsi host0: ufshcd
> >>  6.
> >>     [    0.720163] ALSA device list:
> >>  7.
> >>     [    0.720171]   No soundcards found.
> >>  8.
> >>     [    0.731393] ufshcd-qcom 1d84000.ufshc: ufshcd_print_pwr_info:[RX,
> >>     TX]: gear=[1, 1], lane[1, 1], pwr[SLOWAUTO_MODE, SLOWAUTO_MODE],
> >>     rate = 0
> >>  9.
> >>     [    0.893738] ufshcd-qcom 1d84000.ufshc: ufshcd_print_pwr_info:[RX,
> >>     TX]: gear=[3, 3], lane[2, 2], pwr[FAST MODE, FAST MODE], rate = 2
> >> 10.
> >>     [    0.894703] ufshcd-qcom 1d84000.ufshc:
> >>     ufshcd_find_max_sup_active_icc_level: Regulator capability was not
> >>     set, actvIccLevel=0
> >> 11.
> >>     [    0.896032] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag: Sending
> >>     flag query for idn 14 failed, err = 253
> >> 12.
> >>     [    0.896919] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag: Sending
> >>     flag query for idn 14 failed, err = 253
> >> 13.
> >>     [    0.897798] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag: Sending
> >>     flag query for idn 14 failed, err = 253
> >> 14.
> >>     [    0.898227] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag_retry:
> >>     query attribute, opcode 6, idn 14, failed with error 253 after 3
> >> retires
> >> 15.
> >>     [    0.898798] ufshcd-qcom 1d84000.ufshc: ufshcd_wb_ctrl write
> >>     booster enable failed 253
> >> 16.
> >>     [    0.899150] ufshcd-qcom 1d84000.ufshc: ufshcd_wb_config: Enable
> >>     WB failed: 253
> >> 17.
> >>     [    0.899918] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag: Sending
> >>     flag query for idn 16 failed, err = 253
> >> 18.
> >>     [    0.900448] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag: Sending
> >>     flag query for idn 16 failed, err = 253
> >> 19.
> >>     [    0.901290] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag: Sending
> >>     flag query for idn 16 failed, err = 253
> >> 20.
> >>     [    0.901749] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag_retry:
> >>     query attribute, opcode 6, idn 16, failed with error 253 after 3
> >> retires
> >> 21.
> >>     [    0.902285] ufshcd-qcom 1d84000.ufshc: ufshcd_wb_config: En WB
> >>     flush during H8: failed: 253
> >> 22.
> >>     [    0.903105] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag: Sending
> >>     flag query for idn 15 failed, err = 253
> >> 23.
> >>     [    0.903988] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag: Sending
> >>     flag query for idn 15 failed, err = 253
> >> 24.
> >>     [    0.904866] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag: Sending
> >>     flag query for idn 15 failed, err = 253
> >> 25.
> >>     [    0.905294] ufshcd-qcom 1d84000.ufshc: ufshcd_query_flag_retry:
> >>     query attribute, opcode 6, idn 15, failed with error 253 after 3
> >> retires
> >> 26.
> >>     [    0.905859] ufshcd-qcom 1d84000.ufshc: ufshcd_wb_buf_flush_enable
> >>     WB - buf flush enable failed 253
> >
> > Please help try below simple patch to see if above WriteBooster messages
> > can be eliminated.
> >
> >
> > ---
> >  drivers/scsi/ufs/ufshcd.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
> > index f173ad1bd79f..089c0785f0b3 100644
> > --- a/drivers/scsi/ufs/ufshcd.c
> > +++ b/drivers/scsi/ufs/ufshcd.c
> > @@ -6985,6 +6985,8 @@ static int ufs_get_device_desc(struct ufs_hba
> > *hba)
> >  	    dev_info->wspecversion == 0x220 ||
> >  	    (hba->dev_quirks & UFS_DEVICE_QUIRK_SUPPORT_EXTENDED_FEATURES))
> >  		ufshcd_wb_probe(hba, desc_buf);
> > +	else
> > +		hba->caps &= ~UFSHCD_CAP_WB_EN;
> 
> IMO, hba->caps about WB_EN is already set in ufs-vendor.c. So for
> writebooster didn't support ufs devices, need to clear this caps.
> 
> >

Thanks for the ack. Then I'll send it as a formal patch.

Thank you,
Stanley Chu