| Message ID | 20210127151217.24760-2-avri.altman@wdc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add Host control mode to HPB | expand |
On Wed, Jan 27, 2021 at 05:12:10PM +0200, Avri Altman wrote: > We will use it later, when we'll need to differentiate between device > and host control modes. > > Signed-off-by: Avri Altman <avri.altman@wdc.com> > --- > drivers/scsi/ufs/ufshpb.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c > index d3e6c5b32328..183bdf35f2d0 100644 > --- a/drivers/scsi/ufs/ufshpb.c > +++ b/drivers/scsi/ufs/ufshpb.c > @@ -26,6 +26,8 @@ static int tot_active_srgn_pages; > > static struct workqueue_struct *ufshpb_wq; > > +static enum UFSHPB_MODE ufshpb_mode; How are you allowed to have a single variable for a device-specific thing? What happens when you have two controllers or disks or whatever you are binding to here? How does this work at all? This should be per-device, right? thanks, greg k-h
> > > > +static enum UFSHPB_MODE ufshpb_mode; > > How are you allowed to have a single variable for a device-specific > thing? What happens when you have two controllers or disks or whatever > you are binding to here? How does this work at all? > > This should be per-device, right? Right. Done. Not being bickering, AFAIK, there aren't, nor will be in the foreseen future, any multi-ufs-hosts designs. There were some talks in the past about ufs cards, but this is officially off the table. Thanks, Avri
On Sun, Jan 31, 2021 at 07:08:00AM +0000, Avri Altman wrote: > > > > > > +static enum UFSHPB_MODE ufshpb_mode; > > > > How are you allowed to have a single variable for a device-specific > > thing? What happens when you have two controllers or disks or whatever > > you are binding to here? How does this work at all? > > > > This should be per-device, right? > Right. Done. > > Not being bickering, AFAIK, there aren't, nor will be in the foreseen future, any multi-ufs-hosts designs. Why not? What prevents someone from putting 2 PCI ufs host controllers in a system tomorrow? > There were some talks in the past about ufs cards, but this is officially off the table. Never say never :) Seriously, how can you somehow ensure that a random manufacturer will not do this? thanks, greg k-h
> On Sun, Jan 31, 2021 at 07:08:00AM +0000, Avri Altman wrote: > > > > > > > > +static enum UFSHPB_MODE ufshpb_mode; > > > > > > How are you allowed to have a single variable for a device-specific > > > thing? What happens when you have two controllers or disks or whatever > > > you are binding to here? How does this work at all? > > > > > > This should be per-device, right? > > Right. Done. > > > > Not being bickering, AFAIK, there aren't, nor will be in the foreseen future, > any multi-ufs-hosts designs. > > Why not? What prevents someone from putting 2 PCI ufs host controllers > in a system tomorrow? > > > There were some talks in the past about ufs cards, but this is officially off > the table. > > Never say never :) > > Seriously, how can you somehow ensure that a random manufacturer will > not do this? Better let the platform vendors answer this. As for your comment - you are obviously right - I will fix this. Thanks, Avri
diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c index d3e6c5b32328..183bdf35f2d0 100644 --- a/drivers/scsi/ufs/ufshpb.c +++ b/drivers/scsi/ufs/ufshpb.c @@ -26,6 +26,8 @@ static int tot_active_srgn_pages; static struct workqueue_struct *ufshpb_wq; +static enum UFSHPB_MODE ufshpb_mode; + bool ufshpb_is_allowed(struct ufs_hba *hba) { return !(hba->ufshpb_dev.hpb_disabled); @@ -1690,10 +1692,9 @@ void ufshpb_get_dev_info(struct ufs_hba *hba, u8 *desc_buf) { struct ufshpb_dev_info *hpb_dev_info = &hba->ufshpb_dev; int version; - u8 hpb_mode; - hpb_mode = desc_buf[DEVICE_DESC_PARAM_HPB_CONTROL]; - if (hpb_mode == HPB_HOST_CONTROL) { + ufshpb_mode = desc_buf[DEVICE_DESC_PARAM_HPB_CONTROL]; + if (ufshpb_mode == HPB_HOST_CONTROL) { dev_err(hba->dev, "%s: host control mode is not supported.\n", __func__); hpb_dev_info->hpb_disabled = true;
We will use it later, when we'll need to differentiate between device and host control modes. Signed-off-by: Avri Altman <avri.altman@wdc.com> --- drivers/scsi/ufs/ufshpb.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)