Thanks for your inputs. > > +/* Serial interfaces (I2C, SPI, UART) differ in interface subclass */ > > +enum cy_scb_modes { > > + CY_USBS_SCB_DISABLED = 0, > > + CY_USBS_SCB_UART = 1, > > + CY_USBS_SCB_SPI = 2, > > + CY_USBS_SCB_I2C = 3 > > Why have you numbered these individually? I have changed them as #define's now. These values are not arbitrary. > > + /* Serial interfaces (I2C, SPI, UART) differ in interface subclass */ > > + switch (interface->cur_altsetting->desc.bInterfaceSubClass) { > > + case CY_USBS_SCB_I2C: > > + dev_info(&interface->dev, "using I2C interface\n"); > > + cyusbs23x_devs = cyusbs23x_i2c_gpio_devs; > > + ndevs = ARRAY_SIZE(cyusbs23x_i2c_gpio_devs); > > Not sure I get this. Why do you have local variables for these instead of using > them directly? We are planning to support other protocols in future. To facilitate that, local variables are used. > > + return 0; > > + > > +error: > > + usb_put_dev(cyusbs->usb_dev); > > Isn't there a devm_* version of usb_set_intfdata()? No. Thanks, Muthu This message and any attachments may contain Cypress (or its subsidiaries) confidential information. If it has been received in error, please advise the sender and immediately delete this message. ��.n��������+%������w��{.n�����{���)��jg��������ݢj����G�������j:+v���w�m������w�������h�����٥