On Tue, Jun 23, 2020 at 11:25:53AM +0000, Niklas Cassel wrote:
> On Tue, Jun 23, 2020 at 01:53:47AM -0700, Sagi Grimberg wrote:
> > > static int nvme_setup_streams_ns(struct nvme_ctrl *ctrl, struct nvme_ns *ns,
> > > @@ -1930,6 +1950,15 @@ static int __nvme_revalidate_disk(struct gendisk *disk, struct nvme_id_ns *id)
> > > if (ns->lba_shift == 0)
> > > ns->lba_shift = 9;
> > > + switch (ns->head->ids.csi) {
> > > + case NVME_CSI_NVM:
> > > + break;
> > > + default:
> > > + dev_warn(ctrl->device, "unknown csi:%d ns:%d\n",
> > > + ns->head->ids.csi, ns->head->ns_id);
> > > + return -ENODEV;
> > > + }
> >
> > Not sure we need a switch-case statement for a single case target...
>
> I would consider it two cases. A supported CSI or a non-supported CSI
> (which means any CSI value != NVME_CSI_NVM).
>
> However, a follow up patch (patch 5/5 in this series) adds another case
> to this switch-case statement (NVME_CSI_ZNS).
>
> I guess this patch could have used an if-else statement, and patch 5/5
> replaced the if-statement with a switch-case.
> However, since a patch in the same series actually adds another case,
> I think that it is more clear this way.
> (A switch-case with only two cases added, in a patch that is not the last
> one in the series, suggests (at least to me), that it will most likely be
> extended in a following patch.)
Yeah, this patch is laying the foundation for future command sets.
