El Sun, Sep 01, 2024 at 05:38:39PM GMT Radhey Shyam Pandey ha dit: > usb5744 supports SMBus Configuration and it may be configured via the > SMBus slave interface during the hub start-up configuration stage. > > To program it driver uses i2c-bus phandle (added in commit '02be19e914b8 > dt-bindings: usb: Add support for Microchip usb5744 hub controller') to > get i2c client device and then based on usb5744 compatible check calls > usb5744 i2c default initialization sequence. > > Apart from the USB command attach, prevent the hub from suspend. > when the USB Attach with SMBus (0xAA56) command is issued to the hub, > the hub is getting enumerated and then it puts in a suspend mode. > This causes the hub to NAK any SMBus access made by the SMBus Master > during this period and not able to see the hub's slave address while > running the "i2c probe" command. > > Prevent the MCU from putting the HUB in suspend mode through register > write. The BYPASS_UDC_SUSPEND bit (Bit 3) of the RuntimeFlags2 > register at address 0x411D controls this aspect of the hub. The > BYPASS_UDC_SUSPEND bit in register 0x411Dh must be set to ensure that the > MCU is always enabled and ready to respond to SMBus runtime commands. > This register needs to be written before the USB attach command is issued. > > The byte sequence is as follows: > Slave addr: 0x2d 00 00 05 00 01 41 1D 08 > Slave addr: 0x2d 99 37 00 > Slave addr: 0x2d AA 56 00 > > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@xxxxxxx> > --- > Changes for v4: > - Fix error: implicit declaration of function 'i2c_smbus_*' APIs by > introducing a dependency on I2C_CONFIG. This error is reported > by kernel test on v3 series and usb:usb-testing 20/25 branch. > https://lore.kernel.org/all/2024082503-uncoated-chaperone-7f70@gregkh > > Changes for v3: > - Add comment for UDC suspend sequence. > - Drop USB5744_CREG_MEM_NBYTES and USB5744_CREG_NBYTES and replace > it with literal + comment. > - Move microchip defines to source file. > > Changes for v2: > - Move power on reset delay to separate patch. > - Switch to compatible based check for calling usb5755 > onboard_dev_5744_i2c_init(). This is done to make > onboard_dev_5744_i2c_init() as static. > - Fix subsystem "usb: misc: onboard_usb_dev:..." > - Use #define for different register bits instead of magic values. > - Use err_power_off label name. > - Modified commit description to be in sync with v2 changes. > --- > drivers/usb/misc/Kconfig | 2 +- > drivers/usb/misc/onboard_usb_dev.c | 73 ++++++++++++++++++++++++++++++ > 2 files changed, 74 insertions(+), 1 deletion(-) > > diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig > index 50b86d531701..cb5e47d439ab 100644 > --- a/drivers/usb/misc/Kconfig > +++ b/drivers/usb/misc/Kconfig > @@ -318,7 +318,7 @@ config BRCM_USB_PINMAP > > config USB_ONBOARD_DEV > tristate "Onboard USB device support" > - depends on OF > + depends on OF && I2C > help > Say Y here if you want to support discrete onboard USB devices > that don't require an additional control bus for initialization, > diff --git a/drivers/usb/misc/onboard_usb_dev.c b/drivers/usb/misc/onboard_usb_dev.c > index da27c48fc11d..247600015bca 100644 > --- a/drivers/usb/misc/onboard_usb_dev.c > +++ b/drivers/usb/misc/onboard_usb_dev.c > @@ -11,6 +11,7 @@ > #include <linux/err.h> > #include <linux/gpio/consumer.h> > #include <linux/init.h> > +#include <linux/i2c.h> > #include <linux/kernel.h> > #include <linux/list.h> > #include <linux/module.h> > @@ -29,6 +30,17 @@ > > #include "onboard_usb_dev.h" > > +/* USB5744 register offset and mask */ > +#define USB5744_CMD_ATTACH 0xAA > +#define USB5744_CMD_ATTACH_LSB 0x56 > +#define USB5744_CMD_CREG_ACCESS 0x99 > +#define USB5744_CMD_CREG_ACCESS_LSB 0x37 > +#define USB5744_CREG_MEM_ADDR 0x00 > +#define USB5744_CREG_WRITE 0x00 > +#define USB5744_CREG_RUNTIMEFLAGS2 0x41 > +#define USB5744_CREG_RUNTIMEFLAGS2_LSB 0x1D > +#define USB5744_CREG_BYPASS_UDC_SUSPEND BIT(3) > + > static void onboard_dev_attach_usb_driver(struct work_struct *work); > > static struct usb_device_driver onboard_dev_usbdev_driver; > @@ -297,10 +309,46 @@ static void onboard_dev_attach_usb_driver(struct work_struct *work) > pr_err("Failed to attach USB driver: %pe\n", ERR_PTR(err)); > } > > +static int onboard_dev_5744_i2c_init(struct i2c_client *client) > +{ > + struct device *dev = &client->dev; > + int ret; > + > + /* > + * Set BYPASS_UDC_SUSPEND bit to ensure MCU is always enabled > + * and ready to respond to SMBus runtime commands. > + * The command writes 5 bytes to memory and single data byte in nit: s/in/to the/ > + * configuration register. > + */ > + char wr_buf[7] = {USB5744_CREG_MEM_ADDR, 5, > + USB5744_CREG_WRITE, 1, > + USB5744_CREG_RUNTIMEFLAGS2, > + USB5744_CREG_RUNTIMEFLAGS2_LSB, > + USB5744_CREG_BYPASS_UDC_SUSPEND}; > + > + ret = i2c_smbus_write_block_data(client, 0, sizeof(wr_buf), wr_buf); > + if (ret) > + return dev_err_probe(dev, ret, "BYPASS_UDC_SUSPEND bit configuration failed\n"); > + > + ret = i2c_smbus_write_word_data(client, USB5744_CMD_CREG_ACCESS, > + USB5744_CMD_CREG_ACCESS_LSB); > + if (ret) > + return dev_err_probe(dev, ret, "Configuration Register Access Command failed\n"); nit: s/Command/command/ Uppercase for the register name is ok, but command isn't a name. > + > + /* Send SMBus command to boot hub. */ nit: from the code it's obvious that an SMBus command is sent, no need to spell that out. I suggest to change the comment to "Attach/boot the hub". > + ret = i2c_smbus_write_word_data(client, USB5744_CMD_ATTACH, > + USB5744_CMD_ATTACH_LSB); > + if (ret < 0) > + return dev_err_probe(dev, ret, "USB Attach with SMBus command failed\n"); nit: s/.../USB Attach command failed/ > + > + return ret; return 0; > +}
