On Wed, Aug 23, 2023 at 10:02:51AM +0300, Heikki Krogerus wrote: > On Sun, Aug 20, 2023 at 03:32:25PM -0400, Abdel Alkuor wrote: > > TPS25750 is USB Type-C and PD controller. The device is > > highly configurable using App Customization online Tool > > developed by TI to generate loadable binary. > > > > TPS25750 supports three modes; PTCH, APP, and BOOT. A configuration > > can only be applied when the controller is on PTCH mode. > > > > The controller attempts to load a configuration from EEPROM on > > I2Cm bus. If no EEPROM is detected, then the driver tries to load > > a configuration on I2Cs bus using a firmware file defined > > in DT. > > > > The driver implements the binary loading sequence which > > can be found on pg.53 in TPS25750 Host Interface Technical > > Reference Manual (Rev. A) https://tinyurl.com/y9rkhu8a > > > > The driver only supports resume pm callback as power management is > > automatically controlled by the device. See pg.47 in TPS25750 > > datasheet https://tinyurl.com/3vfd2k43 > > > > v4: > > - PATCH 1: No change > > - PATCH 2: Fix comments style and drop of_match_ptr > > v3: > > - PATCH 1: Fix node name > > - PATCH 2: Upload tps25750 driver patch > > v2: > > - PATCH 1: General properties clean up > > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > > > > Abdel Alkuor (2): > > dt-bindings: usb: Add ti,tps25750 > > USB: typec: Add TI TPS25750 USB Type-C controller > > > > .../devicetree/bindings/usb/ti,tps25750.yaml | 81 ++ > > drivers/usb/typec/Kconfig | 13 + > > drivers/usb/typec/Makefile | 1 + > > drivers/usb/typec/tps25750.c | 1077 +++++++++++++++++ > > drivers/usb/typec/tps25750.h | 162 +++ > > 5 files changed, 1334 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/usb/ti,tps25750.yaml > > create mode 100644 drivers/usb/typec/tps25750.c > > create mode 100644 drivers/usb/typec/tps25750.h Hi Heikki, > > TPS25750 has the same host interface as TI TPS65xxx controllers, no? > The register offsets at least are exactly the same. Correct, the register offsets are the same. That being said, TPS25750 is a subset of TPS6598x where some registers are structured differently, and even some registers are not supported. Hence, Each has its own host interface manual. > > You need to first try to incorporate support for TI25750 support into > the existing tipd driver (drivers/usb/typec/tipd/). > Incorporating TPS25750 into TPS6598x driver is doable which requires the following changes: - Only Check VID register (0x00) for TPS6598x and cd321x, as TPS25750 doesn't have VID register. - TypeC port registration will be registered differently for each PD controller. TPS6598x uses system configuration register (0x28) to get pr/dr capabilities. On the other hand, TPS25750 will use data role property and PD status register (0x40) to get pr/dr capabilities as TPS25750 doesn't have register 0x28 supported. - TPS25750 requires writing a binary configuration to switch PD controller from PTCH mode to APP mode which needs the following changes: - Add PTCH mode to the modes list. - Add an argument to tps6598x_check_mode to return the current mode. - Currently, tps6598x_exec_cmd has cmd timeout hardcoded to 1 second, and doesn't wait before checking DATA_OUT response. In TPS25750, patch 4CCs take longer than 1 second to execute and some requires a delay before checking DATA_OUT. To accommodate that, cmd_timeout and response_delay will be added as arguments to tps6598x_exec_cmd. - Implement applying patch sequence for TPS25750. - In pm suspend callback, patch mode needs to be checked and the binary configuration should be applied if needed. - For interrupt, TPS25750 has only one event register (0x14) and one mask register (0x16) of 11 bytes each, where TPS6598x has two event and two mask registers of 8 bytes each. Both TPS25750 and TPS65986x shares the same bit field offsets for events/masks/clear but many of there fields are reserved in TPS25750, the followings need to be done in tps6598x_interrupt: - Read EVENT1 register as a block of 11 bytes when tps25750 is present - Write CLEAR1 register as a block of 11 bytes when tps25750 is present - Add trace_tps25750_irq - During testing, I noticed that when a cable is plugged into the PD controller and before PD controller switches to APP mode, there is a lag between dr/pr updates and PlugInsertOrRemoval Event, so a check for dr/pr change needs to be added along TPS_REG_INT_PLUG_EVENT check - Add TPS25750 traces for status and power status registers. Trace for data register won't be added as it doesn't exist in the device. - Configure sleep mode for TPS25750. Before writing the binary configuration, PBMs command is used to set up the patch bundle which consists of binary length, timeout, and slave address. The slave address is not the device I2C address, the bundle slave address can be any value except the ones that are listed for possible device addresses which are 0x20, 0x21, 0x22, and 0x23 based on pg.44 in https://www.ti.com/lit/ds/symlink/tps25750.pdf. Currently, I'm using 0x0F for the bundle slave address which I think is not the right way of assigning such address as a conflict may arise when there is another device with 0x0F address is present on the same I2C bus. Should the bundle address be set as property in the device node? Or should we use i2c_probe_func_quick_read to scan the bus and find an available address that is not used? Thanks, Abdel