Re: [PATCH v4 9/9] scsi: ufs: Make .get_hba_mac() optional

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Wed, Jul 03, 2024 at 01:36:46PM -0700, Bart Van Assche wrote:
> On 7/3/24 6:22 AM, Manivannan Sadhasivam wrote:
> > On Tue, Jul 02, 2024 at 01:39:17PM -0700, Bart Van Assche wrote:
> > > -	mac = hba->vops->get_hba_mac(hba);
> > > +	if (!hba->vops || !hba->vops->get_hba_mac) {
> > > +		hba->capabilities =
> > > +			ufshcd_readl(hba, REG_CONTROLLER_CAPABILITIES);
> > > +		mac = hba->capabilities & MASK_TRANSFER_REQUESTS_SLOTS_MCQ;
> > > +		mac++;
> > 
> > Can you add a comment to state that the MAC value is 0 based?
> 
> Sure.
> 
> > > diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> > > index b3444f9ce130..9e0290c6c2d3 100644
> > > --- a/drivers/ufs/core/ufshcd.c
> > > +++ b/drivers/ufs/core/ufshcd.c
> > > @@ -8753,13 +8753,15 @@ static int ufshcd_device_init(struct ufs_hba *hba, bool init_dev_params)
> > >   		if (ret)
> > >   			return ret;
> > >   		if (is_mcq_supported(hba) && !hba->scsi_host_added) {
> > > +			ufshcd_mcq_enable(hba);
> > > +			hba->mcq_enabled = true;
> > 
> > If the 'mcq_enabled' assignment goes hand in hand with
> > ufshcd_mcq_{enable/disable}, why shouldn't it be moved inside?
> 
> If an UFSHCI controller is reset, the controller is reset from MCQ mode
> to SDB mode and it is derived from the hba->mcq_enabled structure member
> that MCQ was enabled before the reset. In other words, moving all
> hba->mcq_enabled assignments into ufshcd_mcq_{enable/disable}() would
> break the code that resets the UFSHCI controller.
> 

Hmm, could you please point me to the code that does this? I tried looking for
it but couldn't spot.

- Mani

-- 
மணிவண்ணன் சதாசிவம்