Re: [RFC PATCH nvme-cli 2/2] nvme-cli/plugins/mi:add support

[Mohit Kapoor]

On Fri, Jul 09, 2021 at 09:04:30AM -0700, Keith Busch wrote:
> > +int hal_init()
> > +{
> > +    int retval = -1;
> > +    switch (GetSidebandInterface()) {
> > +    case qemu_nvme_mi:
> > +        retval = qemu_mi_init();
> > +        break;
> > +    default:
> > +        break;
> > +    }
> > +    return retval;
> > +}
> > +
> > +int hal_open()
> > +{
> > +    int retval = -1;
> > +    switch (GetSidebandInterface()) {
> > +    case qemu_nvme_mi:
> > +        retval = qemu_mi_open();
> > +        break;
> > +    default:
> > +        break;
> > +    }
> > +    return retval;
> > +}
> > +
> > +int hal_close()
> > +{
> > +    int retval = -1;
> > +    switch (GetSidebandInterface()) {
> > +    case qemu_nvme_mi:
> > +        retval = qemu_mi_close();
> > +        break;
> > +    default:
> > +        break;
> > +    }
> > +    return retval;
> > +}
> > +
> > +int hal_i2c_write(uint8_t *data_out, uint16_t num_bytes)
> > +{
> > +    int retval = -1;
> > +    switch (GetSidebandInterface()) {
> > +    case qemu_nvme_mi:
> > +        retval = qemu_mi_write(data_out, num_bytes);
> > +        break;
> > +    default:
> > +        break;
> > +    }
> > +    return retval;
> > +}
> > +
> > +int hal_i2c_read(uint8_t *data_in, uint16_t num_bytes)
> > +{
> > +    uint32_t retval = -1;
> > +    switch (GetSidebandInterface()) {
> > +    case qemu_nvme_mi:
> > +        retval = qemu_mi_read(data_in, num_bytes);
> > +        break;
> > +    default:
> > +        break;
> > +    }
> > +    return retval;
> > +}
>
> I'm really not a fan of having non-standard interfaces. If you were
> going to do that, though, you should create a struct of function
> pointers so that you don't need these repetitive "switch (...)"
> statements.
>
> But if we're going to have OOB MI support in toolign, they should all
> use the same standard defined interface.

Thanks for your comments.
We are planning to incorporate smbus/i2c interface in nvme-cli & qemu
and the patch will be shared in a few days.
While sharing the above patch, we will take care of the hardware
interface layer based on function pointer implementation as well.