mbox series

[v10,00/14] Add TPS25750 USB type-C PD controller support

Message ID 20231003155842.57313-1-alkuor@gmail.com
Headers show
Series Add TPS25750 USB type-C PD controller support | expand

Message

Abdel Alkuor Oct. 3, 2023, 3:58 p.m. UTC
From: Abdel Alkuor <abdelalkuor@geotab.com>

TPS25750 USB type-C PD controller has the same register offsets as
tps6598x. The following is a summary of incorporating TPS25750 into
TPS6598x driver:
- Add device data to accommodate different implementations

- 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 resume 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 following needs 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.

v10:
 - PATCH 1,2:
    - No changes
 - PATCH 3:
    - Add Reviewed-by
 - PATCH 4:
    - Move 0004-USB-typec-tps6598x-Load-TPS25750-patch-bundle.patch to patch 6
    - Reorder 0009-USB-typec-tps6598x-Refactor-tps6598x-port-registrati.patch
      to be patch 4
    - Add Reviewed-by
 - PATCH 5:
    - Move 0005-USB-typec-tps6598x-Check-for-EEPROM-present.patch to patch 6
    - Add device data for tps6598x and cd321x
 - PATCH 6:
    - Add tps25750 support and enable it by aggregating the following patches from v9:
      0004-USB-typec-tps6598x-Load-TPS25750-patch-bundle.patch
      0005-USB-typec-tps6598x-Check-for-EEPROM-present.patch
      0006-USB-typec-tps6598x-Clear-dead-battery-flag.patch
      0007-USB-typec-tps6598x-Apply-patch-again-after-power-res.patch
      0008-USB-typec-tps6598x-Add-interrupt-support-for-TPS2575.patch
      0010-USB-typec-tps6598x-Add-port-registration-for-tps2575.patch
      0011-USB-typec-tps6598x-Enable-sleep-mode-for-tps25750.patch
 - PATCH 7:
    - Move 0012-USB-typec-tps6598x-Add-trace-for-tps25750-irq.patch to patch 7
 - PATCH 8:
    - Move 0013-USB-typec-tps6598x-Add-power-status-trace-for-tps257.patch to patch 8
    - Remove power status trace prototype from device data
 - PATCH 9:
    - Move 0014-USB-typec-tps6598x-Add-status-trace-for-tps25750.patch to patch 9
    - Remove status trace prototype from device data
	
v9:
 - PATCH 1: 
    - Add Reviewed-by
 - PATCH 2..7:
    - No changes
 - PATCH 8:
    - Move of_device_id to its original place
    - Move device data structs to the top of of_device_id
    - Use device_get_match_data to get device data
 - PATCH 9:
    - No changes
 - PATCH 10:
    - Move device data structs to the top of of_device_id
 - PATCH 11,12:
    - No changes
 - PATCH 13,14:
    - Move device data structs to the top of of_device_id
v8:
 - PATCH 1: 
    - Define reg-names at top-level
 - PATCH 2:
    - Add Reviewed-by
 - PATCH 3:
    - Revert mode check return
 - PATCH 4:
    - Return mode when mode is checked
    - Use device_is_compatible instead of of_device_is_compatible
 - PATCH 5,6:
    - No changes
 - PATCH 7:
    - Use device_is_compatible instead of of_device_is_compatible
 - PATCH 8,9:
    - No changes
 - PATCH 10:
    - Change tps->cb to tps->data
 - PATCH 11,12:
    - No changes
 - PATCH 13,14:
    - Change tps->cb to tps->data

v7:
 - PATCH 1: 
    - Define reg at top-level
    - Remove description from reg-names
 - PATCH 2..7: Add tps6598x to the subject
 - PATCH 8:
    - Add tps6598x to the subject
    - Create tps25750 interrupt handler
 - PATCH 9..11: Add tps6598x to the subject
 - PATCH 12: 
    - Add driver name to commit subject
    - Call trace_tps25750_irq directly from tps25750 interrupt
      handler
 - PATCH 13-14: Add tps6598x to the subject
      
v6:
 - PATCH 1: Use reg property for patch address
 - PATCH 2: Use tps6598x_exec_cmd as a wrapper
 - PATCH 3: Return current mode and check it directly
 - PATCH 4:
    - Don't check VID for tps25750 as the VID register doesn't exist
    - Remove is_tps25750 flag from tps6598x struct
    - Get patch address from reg property
 - PATCH 5: Update eeprom macro to use TPS instead
 - PATCH 6: No changes
 - PATCH 7: Check tps25750 using is_compatiable device node
 - PATCH 8: Create tipd callbacks factory 
 - PATCH 9: No changes
 - PATCH 10: Add port registration to tipd data factory
 - PATCH 11: Use tps25750_init instead of tps25750_apply_patch in resume
    	     as it initializes sleep mode
 - PATCH 12: Add trace irq to tipd callbacks factory
 - PATCH 13: Add trace power status to tipd data factory
 - PATCH 14: Add trace status to tipd data factory
v5:
 - PATCH 1: Add tps25750 bindings to tps6598x
 - PATCH 2: Remove tps25750 driver and incorperate tps25750
 	    into tps6598x driver
 - PATCH [3..15]: Incorporating tps25750 into tps6598x driver
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

Abdel Alkuor (9):
  dt-bindings: usb: tps6598x: Add tps25750
  USB: typec: tsp6598x: Add cmd timeout and response delay
  USB: typec: tps6598x: Add patch mode to tps6598x
  USB: typec: tps6598x: Refactor tps6598x port registration
  USB: typec: tps6598x: Add device data to of_device_id
  USB: typec: tps6598x: Add TPS25750 support
  USB: typec: tps6598x: Add trace for tps25750 irq
  USB: typec: tps6598x: Add power status trace for tps25750
  USB: typec: tps6598x: Add status trace for tps25750

 .../devicetree/bindings/usb/ti,tps6598x.yaml  |  81 ++-
 drivers/usb/typec/tipd/core.c                 | 626 ++++++++++++++++--
 drivers/usb/typec/tipd/tps6598x.h             |  36 +
 drivers/usb/typec/tipd/trace.h                |  92 +++
 4 files changed, 764 insertions(+), 71 deletions(-)

Comments

Heikki Krogerus Oct. 10, 2023, 5:58 a.m. UTC | #1
On Tue, Oct 03, 2023 at 11:58:40AM -0400, Abdel Alkuor wrote:
> From: Abdel Alkuor <abdelalkuor@geotab.com>
> 
> tps25750 event1 register doesn't have all bits in tps6598x
> event registers, only show the events that are masked
> 
> Signed-off-by: Abdel Alkuor <abdelalkuor@geotab.com>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
> Changes in v10:
>   - Move 0012-USB-typec-tps6598x-Add-trace-for-tps25750-irq.patch to patch 7
> Changes in v9:
>   - No changes
> Changes in v8:
>   - Use device_is_compatible instead of of_device_is_compatible
> Changes in v7:
>   - Add driver name to commit subject
> Changes in v6:
>   - Check tps25750 using is_compatiable device node
> Changes in v5:
>   - Incorporating tps25750 into tps6598x driver
>  drivers/usb/typec/tipd/core.c  |  1 +
>  drivers/usb/typec/tipd/trace.h | 22 ++++++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
> index 892432ad80a3..e0ff6fc9f256 100644
> --- a/drivers/usb/typec/tipd/core.c
> +++ b/drivers/usb/typec/tipd/core.c
> @@ -589,6 +589,7 @@ static irqreturn_t tps25750_interrupt(int irq, void *data)
>  		dev_err(tps->dev, "%s: failed to read events\n", __func__);
>  		goto err_unlock;
>  	}
> +	trace_tps25750_irq(event[0]);
>  
>  	if (!(event[0] | event[1]))
>  		goto err_unlock;
> diff --git a/drivers/usb/typec/tipd/trace.h b/drivers/usb/typec/tipd/trace.h
> index 12cad1bde7cc..28725234a2d8 100644
> --- a/drivers/usb/typec/tipd/trace.h
> +++ b/drivers/usb/typec/tipd/trace.h
> @@ -74,6 +74,13 @@
>  		{ APPLE_CD_REG_INT_DATA_STATUS_UPDATE,		"DATA_STATUS_UPDATE" }, \
>  		{ APPLE_CD_REG_INT_STATUS_UPDATE,		"STATUS_UPDATE" })
>  
> +#define show_tps25750_irq_flags(flags) \
> +	__print_flags_u64(flags, "|", \
> +		{ TPS_REG_INT_PLUG_EVENT,			"PLUG_EVENT" }, \
> +		{ TPS_REG_INT_POWER_STATUS_UPDATE,		"POWER_STATUS_UPDATE" }, \
> +		{ TPS_REG_INT_STATUS_UPDATE,			"STATUS_UPDATE" }, \
> +		{ TPS_REG_INT_PD_STATUS_UPDATE,			"PD_STATUS_UPDATE" })
> +
>  #define TPS6598X_STATUS_FLAGS_MASK (GENMASK(31, 0) ^ (TPS_STATUS_CONN_STATE_MASK | \
>  						      TPS_STATUS_PP_5V0_SWITCH_MASK | \
>  						      TPS_STATUS_PP_HV_SWITCH_MASK | \
> @@ -230,6 +237,21 @@ TRACE_EVENT(cd321x_irq,
>  		      show_cd321x_irq_flags(__entry->event))
>  );
>  
> +TRACE_EVENT(tps25750_irq,
> +	    TP_PROTO(u64 event),
> +	    TP_ARGS(event),
> +
> +	    TP_STRUCT__entry(
> +			     __field(u64, event)
> +			     ),
> +
> +	    TP_fast_assign(
> +			   __entry->event = event;
> +			   ),
> +
> +	    TP_printk("event=%s", show_tps25750_irq_flags(__entry->event))
> +);
> +
>  TRACE_EVENT(tps6598x_status,
>  	    TP_PROTO(u32 status),
>  	    TP_ARGS(status),
> -- 
> 2.34.1
Greg Kroah-Hartman Oct. 10, 2023, 11:21 a.m. UTC | #2
On Tue, Oct 10, 2023 at 07:09:22AM -0400, Abdel Alkuor wrote:
> On Tue, Oct 10, 2023 at 09:02:55AM +0200, Greg KH wrote:
> > On Tue, Oct 03, 2023 at 11:58:33AM -0400, Abdel Alkuor wrote:
> > > From: Abdel Alkuor <abdelalkuor@geotab.com>
> >
> Hi Greg,
> > Note, your subject line says 00/14 yet your patches had 00/09.  I hope
> > you didn't dropp patches somewhere?
> >
> Sorry, it is just a copy/paste error from v9 as v9 has 14 patches.
> > I applied what is here now, but are some missing?
> >
> No, they are 9 patches in v10 as many patches from v9 got merged into
> one patch. Should I upload another cover letter to fix the subject line?

No, that would just confuse me more, all is good now, thanks.

greg k-h