| Message ID | 20180802205841.22039-1-mb@lightnvm.io (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | lightnvm: pblk: remove unnecessary drive version check | expand |
> On 2 Aug 2018, at 22.58, Matias Bjørling <mb@lightnvm.io> wrote: > > The nvme driver checks for 1.2 and 2.0 compatibility. If an unsupported > version is reported, the device will not be initialized. > > Signed-off-by: Matias Bjørling <mb@lightnvm.io> > --- > drivers/lightnvm/pblk-init.c | 8 -------- > 1 file changed, 8 deletions(-) > > diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c > index 537e98f2b24a..e9e2fedff387 100644 > --- a/drivers/lightnvm/pblk-init.c > +++ b/drivers/lightnvm/pblk-init.c > @@ -1202,14 +1202,6 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, > pblk->state = PBLK_STATE_RUNNING; > pblk->gc.gc_enabled = 0; > > - if (!(geo->version == NVM_OCSSD_SPEC_12 || > - geo->version == NVM_OCSSD_SPEC_20)) { > - pblk_err(pblk, "OCSSD version not supported (%u)\n", > - geo->version); > - kfree(pblk); > - return ERR_PTR(-EINVAL); > - } > - > if (geo->version == NVM_OCSSD_SPEC_12 && geo->dom & NVM_RSP_L2P) { > pblk_err(pblk, "host-side L2P table not supported. (%x)\n", > geo->dom); > -- > 2.11.0 The same comment goes for this patch. pblk does support 2.0 and 1.2 today, but it is not guaranteed it will support a new revision straight away. So I think that a check wither here or through the .capabilities I proposed on the other patch is necessary. Javier
On 08/03/2018 02:18 PM, Javier Gonzalez wrote: >> On 2 Aug 2018, at 22.58, Matias Bjørling <mb@lightnvm.io> wrote: >> >> The nvme driver checks for 1.2 and 2.0 compatibility. If an unsupported >> version is reported, the device will not be initialized. >> >> Signed-off-by: Matias Bjørling <mb@lightnvm.io> >> --- >> drivers/lightnvm/pblk-init.c | 8 -------- >> 1 file changed, 8 deletions(-) >> >> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c >> index 537e98f2b24a..e9e2fedff387 100644 >> --- a/drivers/lightnvm/pblk-init.c >> +++ b/drivers/lightnvm/pblk-init.c >> @@ -1202,14 +1202,6 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, >> pblk->state = PBLK_STATE_RUNNING; >> pblk->gc.gc_enabled = 0; >> >> - if (!(geo->version == NVM_OCSSD_SPEC_12 || >> - geo->version == NVM_OCSSD_SPEC_20)) { >> - pblk_err(pblk, "OCSSD version not supported (%u)\n", >> - geo->version); >> - kfree(pblk); >> - return ERR_PTR(-EINVAL); >> - } >> - >> if (geo->version == NVM_OCSSD_SPEC_12 && geo->dom & NVM_RSP_L2P) { >> pblk_err(pblk, "host-side L2P table not supported. (%x)\n", >> geo->dom); >> -- >> 2.11.0 > > The same comment goes for this patch. pblk does support 2.0 and 1.2 > today, but it is not guaranteed it will support a new revision straight > away. So I think that a check wither here or through the .capabilities I > proposed on the other patch is necessary. > Works for me. I'll send another patch that enables pblk target to tell which version it supports.
On 08/03/2018 02:39 PM, Matias Bjørling wrote: > On 08/03/2018 02:18 PM, Javier Gonzalez wrote: >>> On 2 Aug 2018, at 22.58, Matias Bjørling <mb@lightnvm.io> wrote: >>> >>> The nvme driver checks for 1.2 and 2.0 compatibility. If an unsupported >>> version is reported, the device will not be initialized. >>> >>> Signed-off-by: Matias Bjørling <mb@lightnvm.io> >>> --- >>> drivers/lightnvm/pblk-init.c | 8 -------- >>> 1 file changed, 8 deletions(-) >>> >>> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c >>> index 537e98f2b24a..e9e2fedff387 100644 >>> --- a/drivers/lightnvm/pblk-init.c >>> +++ b/drivers/lightnvm/pblk-init.c >>> @@ -1202,14 +1202,6 @@ static void *pblk_init(struct nvm_tgt_dev >>> *dev, struct gendisk *tdisk, >>> pblk->state = PBLK_STATE_RUNNING; >>> pblk->gc.gc_enabled = 0; >>> >>> - if (!(geo->version == NVM_OCSSD_SPEC_12 || >>> - geo->version == NVM_OCSSD_SPEC_20)) { >>> - pblk_err(pblk, "OCSSD version not supported (%u)\n", >>> - geo->version); >>> - kfree(pblk); >>> - return ERR_PTR(-EINVAL); >>> - } >>> - >>> if (geo->version == NVM_OCSSD_SPEC_12 && geo->dom & NVM_RSP_L2P) { >>> pblk_err(pblk, "host-side L2P table not supported. (%x)\n", >>> geo->dom); >>> -- >>> 2.11.0 >> >> The same comment goes for this patch. pblk does support 2.0 and 1.2 >> today, but it is not guaranteed it will support a new revision straight >> away. So I think that a check wither here or through the .capabilities I >> proposed on the other patch is necessary. >> > > Works for me. I'll send another patch that enables pblk target to tell > which version it supports. On second thought. The idea is that pblk will not have to think about 1.2/2.0. The core will expose a general address format, etc., such that pblk doesn't have to be complicated by that. I am working on the various pieces and moving them to core. The patch may have gone out a bit early. I will send this again when the 1.2/2.0 conversions have been applied.
> On 3 Aug 2018, at 14.55, Matias Bjørling <mb@lightnvm.io> wrote: > > On 08/03/2018 02:39 PM, Matias Bjørling wrote: >> On 08/03/2018 02:18 PM, Javier Gonzalez wrote: >>>> On 2 Aug 2018, at 22.58, Matias Bjørling <mb@lightnvm.io> wrote: >>>> >>>> The nvme driver checks for 1.2 and 2.0 compatibility. If an unsupported >>>> version is reported, the device will not be initialized. >>>> >>>> Signed-off-by: Matias Bjørling <mb@lightnvm.io> >>>> --- >>>> drivers/lightnvm/pblk-init.c | 8 -------- >>>> 1 file changed, 8 deletions(-) >>>> >>>> diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c >>>> index 537e98f2b24a..e9e2fedff387 100644 >>>> --- a/drivers/lightnvm/pblk-init.c >>>> +++ b/drivers/lightnvm/pblk-init.c >>>> @@ -1202,14 +1202,6 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, >>>> pblk->state = PBLK_STATE_RUNNING; >>>> pblk->gc.gc_enabled = 0; >>>> >>>> - if (!(geo->version == NVM_OCSSD_SPEC_12 || >>>> - geo->version == NVM_OCSSD_SPEC_20)) { >>>> - pblk_err(pblk, "OCSSD version not supported (%u)\n", >>>> - geo->version); >>>> - kfree(pblk); >>>> - return ERR_PTR(-EINVAL); >>>> - } >>>> - >>>> if (geo->version == NVM_OCSSD_SPEC_12 && geo->dom & NVM_RSP_L2P) { >>>> pblk_err(pblk, "host-side L2P table not supported. (%x)\n", >>>> geo->dom); >>>> -- >>>> 2.11.0 >>> >>> The same comment goes for this patch. pblk does support 2.0 and 1.2 >>> today, but it is not guaranteed it will support a new revision straight >>> away. So I think that a check wither here or through the .capabilities I >>> proposed on the other patch is necessary. >> Works for me. I'll send another patch that enables pblk target to tell which version it supports. > > On second thought. The idea is that pblk will not have to think about > 1.2/2.0. The core will expose a general address format, etc., such > that pblk doesn't have to be complicated by that. > Yes! This was the main motivation for me starting looking into moving things down. An issue you might hit is that some conversions are very tightly couple with pblk (e.g., 32/64 bit L2P conversions). In the code I have on my end, I ended up having helpers for it, but it does not feel right. i'm happy to share some thoughts if you want... > I am working on the various pieces and moving them to core. The patch > may have gone out a bit early. I will send this again when the 1.2/2.0 > conversions have been applied. Cool. Thanks! Javier
diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c index 537e98f2b24a..e9e2fedff387 100644 --- a/drivers/lightnvm/pblk-init.c +++ b/drivers/lightnvm/pblk-init.c @@ -1202,14 +1202,6 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, pblk->state = PBLK_STATE_RUNNING; pblk->gc.gc_enabled = 0; - if (!(geo->version == NVM_OCSSD_SPEC_12 || - geo->version == NVM_OCSSD_SPEC_20)) { - pblk_err(pblk, "OCSSD version not supported (%u)\n", - geo->version); - kfree(pblk); - return ERR_PTR(-EINVAL); - } - if (geo->version == NVM_OCSSD_SPEC_12 && geo->dom & NVM_RSP_L2P) { pblk_err(pblk, "host-side L2P table not supported. (%x)\n", geo->dom);
The nvme driver checks for 1.2 and 2.0 compatibility. If an unsupported version is reported, the device will not be initialized. Signed-off-by: Matias Bjørling <mb@lightnvm.io> --- drivers/lightnvm/pblk-init.c | 8 -------- 1 file changed, 8 deletions(-)