All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/4] USB Power Delivery character device interface
@ 2021-10-26 14:33 Heikki Krogerus
  2021-10-26 14:33 ` [RFC PATCH 1/4] usb: pd: uapi header split Heikki Krogerus
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Heikki Krogerus @ 2021-10-26 14:33 UTC (permalink / raw)
  To: Prashant Malani, Benson Leung
  Cc: Adam Thomson, Guenter Roeck, Badhri Jagan Sridharan, Jack Pham,
	Gopal, Saranya, Regupathy, Rajaram, linux-usb, linux-kernel

Hi,

This is the proposal for USB PD character devices that we could use to
communicate USB PD messages directly with the USB PD capable partners,
ports and cable plugs from user space. Originally I proposed this idea
here as a better way to get the PDOs from the partners (and ports and
plugs): https://lkml.org/lkml/2021/10/8/331

Initially you could read the PDOs with this, but it of course is not
only limited to PDOs - any messages should potentetially be suported.

This is in any case really just a draft, and most likely does not work
if you try it, but it should give an idea about what I'm proposing
(hopefully).

In this proposal I'm placing each device that you can have
behind a single USB PD capable Type-C port (so port, partner
and plugs) under its own folder, so you would end up with
something like this if also both plugs are supported:

	/dev/pd0/port
	/dev/pd0/plug0
	/dev/pd0/plug1
	/dev/pd0/partner

I'm also including an ugly test tool that you can try out to dump the
PDOs depending on the role of the device - tools/usb/pd-test.c. This
is what I got from a charger (and the port at the same time):

	% pd-test /dev/pd0/port
	Sink Capabilities:
	  PDO1: 0x3601912c
	  PDO2: 0x000640e1
	  PDO3: 0x9901912c

	% pd-test /dev/pd0/partner
	Source Capabilities:
	  PDO1: 0x0801912c
	  PDO2: 0x0003c12c

But please note that this whole series is really just meant to be PoC!
Don't assume it will work!

The core code that adds the character devices is here:

	drivers/usb/typec/pd-dev.c.

It's actually really simple, and I don't think it would ever need to
be much more complicated than that.

Example driver support I added to UCSI:

	drivers/usb/typec/ucsi/pd-dev.c

thanks,

Heikki Krogerus (4):
  usb: pd: uapi header split
  usb: typec: Character device for USB Power Delivery devices
  usb: typec: ucsi: Add support for PD cdev
  tools: usb: Hideous test tool for USB PD char device

 .../userspace-api/ioctl/ioctl-number.rst      |   1 +
 drivers/usb/typec/Makefile                    |   2 +-
 drivers/usb/typec/class.c                     |  42 ++
 drivers/usb/typec/class.h                     |   4 +
 drivers/usb/typec/pd-dev.c                    | 210 ++++++++
 drivers/usb/typec/pd-dev.h                    |  15 +
 drivers/usb/typec/ucsi/Makefile               |   2 +-
 drivers/usb/typec/ucsi/pd-dev.c               | 125 +++++
 drivers/usb/typec/ucsi/ucsi.c                 |  37 +-
 drivers/usb/typec/ucsi/ucsi.h                 |   7 +
 include/linux/usb/pd.h                        | 487 +----------------
 include/linux/usb/pd_dev.h                    |  22 +
 include/linux/usb/typec.h                     |   8 +
 include/uapi/linux/usb/pd.h                   | 496 ++++++++++++++++++
 include/uapi/linux/usb/pd_dev.h               |  55 ++
 tools/usb/Build                               |   1 +
 tools/usb/Makefile                            |   8 +-
 tools/usb/pd-test.c                           | 123 +++++
 18 files changed, 1147 insertions(+), 498 deletions(-)
 create mode 100644 drivers/usb/typec/pd-dev.c
 create mode 100644 drivers/usb/typec/pd-dev.h
 create mode 100644 drivers/usb/typec/ucsi/pd-dev.c
 create mode 100644 include/linux/usb/pd_dev.h
 create mode 100644 include/uapi/linux/usb/pd.h
 create mode 100644 include/uapi/linux/usb/pd_dev.h
 create mode 100644 tools/usb/pd-test.c

-- 
2.33.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [RFC PATCH 1/4] usb: pd: uapi header split
  2021-10-26 14:33 [RFC PATCH 0/4] USB Power Delivery character device interface Heikki Krogerus
@ 2021-10-26 14:33 ` Heikki Krogerus
  2021-10-26 16:05   ` kernel test robot
  2021-10-26 14:33 ` [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices Heikki Krogerus
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Heikki Krogerus @ 2021-10-26 14:33 UTC (permalink / raw)
  To: Prashant Malani, Benson Leung
  Cc: Adam Thomson, Guenter Roeck, Badhri Jagan Sridharan, Jack Pham,
	Gopal, Saranya, Regupathy, Rajaram, linux-usb, linux-kernel

WIP. This still needs tuning.

Exporting the USB Power Delivery definitions to user space.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 include/linux/usb/pd.h      | 487 +----------------------------------
 include/uapi/linux/usb/pd.h | 496 ++++++++++++++++++++++++++++++++++++
 2 files changed, 497 insertions(+), 486 deletions(-)
 create mode 100644 include/uapi/linux/usb/pd.h

diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 96b7ff66f074b..86802d59ca510 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -6,493 +6,8 @@
 #ifndef __LINUX_USB_PD_H
 #define __LINUX_USB_PD_H
 
-#include <linux/kernel.h>
-#include <linux/types.h>
-#include <linux/usb/typec.h>
+#include <uapi/linux/usb/pd.h>
 
-/* USB PD Messages */
-enum pd_ctrl_msg_type {
-	/* 0 Reserved */
-	PD_CTRL_GOOD_CRC = 1,
-	PD_CTRL_GOTO_MIN = 2,
-	PD_CTRL_ACCEPT = 3,
-	PD_CTRL_REJECT = 4,
-	PD_CTRL_PING = 5,
-	PD_CTRL_PS_RDY = 6,
-	PD_CTRL_GET_SOURCE_CAP = 7,
-	PD_CTRL_GET_SINK_CAP = 8,
-	PD_CTRL_DR_SWAP = 9,
-	PD_CTRL_PR_SWAP = 10,
-	PD_CTRL_VCONN_SWAP = 11,
-	PD_CTRL_WAIT = 12,
-	PD_CTRL_SOFT_RESET = 13,
-	/* 14-15 Reserved */
-	PD_CTRL_NOT_SUPP = 16,
-	PD_CTRL_GET_SOURCE_CAP_EXT = 17,
-	PD_CTRL_GET_STATUS = 18,
-	PD_CTRL_FR_SWAP = 19,
-	PD_CTRL_GET_PPS_STATUS = 20,
-	PD_CTRL_GET_COUNTRY_CODES = 21,
-	/* 22-31 Reserved */
-};
-
-enum pd_data_msg_type {
-	/* 0 Reserved */
-	PD_DATA_SOURCE_CAP = 1,
-	PD_DATA_REQUEST = 2,
-	PD_DATA_BIST = 3,
-	PD_DATA_SINK_CAP = 4,
-	PD_DATA_BATT_STATUS = 5,
-	PD_DATA_ALERT = 6,
-	PD_DATA_GET_COUNTRY_INFO = 7,
-	PD_DATA_ENTER_USB = 8,
-	/* 9-14 Reserved */
-	PD_DATA_VENDOR_DEF = 15,
-	/* 16-31 Reserved */
-};
-
-enum pd_ext_msg_type {
-	/* 0 Reserved */
-	PD_EXT_SOURCE_CAP_EXT = 1,
-	PD_EXT_STATUS = 2,
-	PD_EXT_GET_BATT_CAP = 3,
-	PD_EXT_GET_BATT_STATUS = 4,
-	PD_EXT_BATT_CAP = 5,
-	PD_EXT_GET_MANUFACTURER_INFO = 6,
-	PD_EXT_MANUFACTURER_INFO = 7,
-	PD_EXT_SECURITY_REQUEST = 8,
-	PD_EXT_SECURITY_RESPONSE = 9,
-	PD_EXT_FW_UPDATE_REQUEST = 10,
-	PD_EXT_FW_UPDATE_RESPONSE = 11,
-	PD_EXT_PPS_STATUS = 12,
-	PD_EXT_COUNTRY_INFO = 13,
-	PD_EXT_COUNTRY_CODES = 14,
-	/* 15-31 Reserved */
-};
-
-#define PD_REV10	0x0
-#define PD_REV20	0x1
-#define PD_REV30	0x2
 #define PD_MAX_REV	PD_REV30
 
-#define PD_HEADER_EXT_HDR	BIT(15)
-#define PD_HEADER_CNT_SHIFT	12
-#define PD_HEADER_CNT_MASK	0x7
-#define PD_HEADER_ID_SHIFT	9
-#define PD_HEADER_ID_MASK	0x7
-#define PD_HEADER_PWR_ROLE	BIT(8)
-#define PD_HEADER_REV_SHIFT	6
-#define PD_HEADER_REV_MASK	0x3
-#define PD_HEADER_DATA_ROLE	BIT(5)
-#define PD_HEADER_TYPE_SHIFT	0
-#define PD_HEADER_TYPE_MASK	0x1f
-
-#define PD_HEADER(type, pwr, data, rev, id, cnt, ext_hdr)		\
-	((((type) & PD_HEADER_TYPE_MASK) << PD_HEADER_TYPE_SHIFT) |	\
-	 ((pwr) == TYPEC_SOURCE ? PD_HEADER_PWR_ROLE : 0) |		\
-	 ((data) == TYPEC_HOST ? PD_HEADER_DATA_ROLE : 0) |		\
-	 (rev << PD_HEADER_REV_SHIFT) |					\
-	 (((id) & PD_HEADER_ID_MASK) << PD_HEADER_ID_SHIFT) |		\
-	 (((cnt) & PD_HEADER_CNT_MASK) << PD_HEADER_CNT_SHIFT) |	\
-	 ((ext_hdr) ? PD_HEADER_EXT_HDR : 0))
-
-#define PD_HEADER_LE(type, pwr, data, rev, id, cnt) \
-	cpu_to_le16(PD_HEADER((type), (pwr), (data), (rev), (id), (cnt), (0)))
-
-static inline unsigned int pd_header_cnt(u16 header)
-{
-	return (header >> PD_HEADER_CNT_SHIFT) & PD_HEADER_CNT_MASK;
-}
-
-static inline unsigned int pd_header_cnt_le(__le16 header)
-{
-	return pd_header_cnt(le16_to_cpu(header));
-}
-
-static inline unsigned int pd_header_type(u16 header)
-{
-	return (header >> PD_HEADER_TYPE_SHIFT) & PD_HEADER_TYPE_MASK;
-}
-
-static inline unsigned int pd_header_type_le(__le16 header)
-{
-	return pd_header_type(le16_to_cpu(header));
-}
-
-static inline unsigned int pd_header_msgid(u16 header)
-{
-	return (header >> PD_HEADER_ID_SHIFT) & PD_HEADER_ID_MASK;
-}
-
-static inline unsigned int pd_header_msgid_le(__le16 header)
-{
-	return pd_header_msgid(le16_to_cpu(header));
-}
-
-static inline unsigned int pd_header_rev(u16 header)
-{
-	return (header >> PD_HEADER_REV_SHIFT) & PD_HEADER_REV_MASK;
-}
-
-static inline unsigned int pd_header_rev_le(__le16 header)
-{
-	return pd_header_rev(le16_to_cpu(header));
-}
-
-#define PD_EXT_HDR_CHUNKED		BIT(15)
-#define PD_EXT_HDR_CHUNK_NUM_SHIFT	11
-#define PD_EXT_HDR_CHUNK_NUM_MASK	0xf
-#define PD_EXT_HDR_REQ_CHUNK		BIT(10)
-#define PD_EXT_HDR_DATA_SIZE_SHIFT	0
-#define PD_EXT_HDR_DATA_SIZE_MASK	0x1ff
-
-#define PD_EXT_HDR(data_size, req_chunk, chunk_num, chunked)				\
-	((((data_size) & PD_EXT_HDR_DATA_SIZE_MASK) << PD_EXT_HDR_DATA_SIZE_SHIFT) |	\
-	 ((req_chunk) ? PD_EXT_HDR_REQ_CHUNK : 0) |					\
-	 (((chunk_num) & PD_EXT_HDR_CHUNK_NUM_MASK) << PD_EXT_HDR_CHUNK_NUM_SHIFT) |	\
-	 ((chunked) ? PD_EXT_HDR_CHUNKED : 0))
-
-#define PD_EXT_HDR_LE(data_size, req_chunk, chunk_num, chunked) \
-	cpu_to_le16(PD_EXT_HDR((data_size), (req_chunk), (chunk_num), (chunked)))
-
-static inline unsigned int pd_ext_header_chunk_num(u16 ext_header)
-{
-	return (ext_header >> PD_EXT_HDR_CHUNK_NUM_SHIFT) &
-		PD_EXT_HDR_CHUNK_NUM_MASK;
-}
-
-static inline unsigned int pd_ext_header_data_size(u16 ext_header)
-{
-	return (ext_header >> PD_EXT_HDR_DATA_SIZE_SHIFT) &
-		PD_EXT_HDR_DATA_SIZE_MASK;
-}
-
-static inline unsigned int pd_ext_header_data_size_le(__le16 ext_header)
-{
-	return pd_ext_header_data_size(le16_to_cpu(ext_header));
-}
-
-#define PD_MAX_PAYLOAD		7
-#define PD_EXT_MAX_CHUNK_DATA	26
-
-/**
-  * struct pd_chunked_ext_message_data - PD chunked extended message data as
-  *					 seen on wire
-  * @header:    PD extended message header
-  * @data:      PD extended message data
-  */
-struct pd_chunked_ext_message_data {
-	__le16 header;
-	u8 data[PD_EXT_MAX_CHUNK_DATA];
-} __packed;
-
-/**
-  * struct pd_message - PD message as seen on wire
-  * @header:    PD message header
-  * @payload:   PD message payload
-  * @ext_msg:   PD message chunked extended message data
-  */
-struct pd_message {
-	__le16 header;
-	union {
-		__le32 payload[PD_MAX_PAYLOAD];
-		struct pd_chunked_ext_message_data ext_msg;
-	};
-} __packed;
-
-/* PDO: Power Data Object */
-#define PDO_MAX_OBJECTS		7
-
-enum pd_pdo_type {
-	PDO_TYPE_FIXED = 0,
-	PDO_TYPE_BATT = 1,
-	PDO_TYPE_VAR = 2,
-	PDO_TYPE_APDO = 3,
-};
-
-#define PDO_TYPE_SHIFT		30
-#define PDO_TYPE_MASK		0x3
-
-#define PDO_TYPE(t)	((t) << PDO_TYPE_SHIFT)
-
-#define PDO_VOLT_MASK		0x3ff
-#define PDO_CURR_MASK		0x3ff
-#define PDO_PWR_MASK		0x3ff
-
-#define PDO_FIXED_DUAL_ROLE		BIT(29)	/* Power role swap supported */
-#define PDO_FIXED_SUSPEND		BIT(28) /* USB Suspend supported (Source) */
-#define PDO_FIXED_HIGHER_CAP		BIT(28) /* Requires more than vSafe5V (Sink) */
-#define PDO_FIXED_EXTPOWER		BIT(27) /* Externally powered */
-#define PDO_FIXED_USB_COMM		BIT(26) /* USB communications capable */
-#define PDO_FIXED_DATA_SWAP		BIT(25) /* Data role swap supported */
-#define PDO_FIXED_UNCHUNK_EXT		BIT(24) /* Unchunked Extended Message supported (Source) */
-#define PDO_FIXED_FRS_CURR_MASK		(BIT(24) | BIT(23)) /* FR_Swap Current (Sink) */
-#define PDO_FIXED_FRS_CURR_SHIFT	23
-#define PDO_FIXED_VOLT_SHIFT		10	/* 50mV units */
-#define PDO_FIXED_CURR_SHIFT		0	/* 10mA units */
-
-#define PDO_FIXED_VOLT(mv)	((((mv) / 50) & PDO_VOLT_MASK) << PDO_FIXED_VOLT_SHIFT)
-#define PDO_FIXED_CURR(ma)	((((ma) / 10) & PDO_CURR_MASK) << PDO_FIXED_CURR_SHIFT)
-
-#define PDO_FIXED(mv, ma, flags)			\
-	(PDO_TYPE(PDO_TYPE_FIXED) | (flags) |		\
-	 PDO_FIXED_VOLT(mv) | PDO_FIXED_CURR(ma))
-
-#define VSAFE5V 5000 /* mv units */
-
-#define PDO_BATT_MAX_VOLT_SHIFT	20	/* 50mV units */
-#define PDO_BATT_MIN_VOLT_SHIFT	10	/* 50mV units */
-#define PDO_BATT_MAX_PWR_SHIFT	0	/* 250mW units */
-
-#define PDO_BATT_MIN_VOLT(mv) ((((mv) / 50) & PDO_VOLT_MASK) << PDO_BATT_MIN_VOLT_SHIFT)
-#define PDO_BATT_MAX_VOLT(mv) ((((mv) / 50) & PDO_VOLT_MASK) << PDO_BATT_MAX_VOLT_SHIFT)
-#define PDO_BATT_MAX_POWER(mw) ((((mw) / 250) & PDO_PWR_MASK) << PDO_BATT_MAX_PWR_SHIFT)
-
-#define PDO_BATT(min_mv, max_mv, max_mw)			\
-	(PDO_TYPE(PDO_TYPE_BATT) | PDO_BATT_MIN_VOLT(min_mv) |	\
-	 PDO_BATT_MAX_VOLT(max_mv) | PDO_BATT_MAX_POWER(max_mw))
-
-#define PDO_VAR_MAX_VOLT_SHIFT	20	/* 50mV units */
-#define PDO_VAR_MIN_VOLT_SHIFT	10	/* 50mV units */
-#define PDO_VAR_MAX_CURR_SHIFT	0	/* 10mA units */
-
-#define PDO_VAR_MIN_VOLT(mv) ((((mv) / 50) & PDO_VOLT_MASK) << PDO_VAR_MIN_VOLT_SHIFT)
-#define PDO_VAR_MAX_VOLT(mv) ((((mv) / 50) & PDO_VOLT_MASK) << PDO_VAR_MAX_VOLT_SHIFT)
-#define PDO_VAR_MAX_CURR(ma) ((((ma) / 10) & PDO_CURR_MASK) << PDO_VAR_MAX_CURR_SHIFT)
-
-#define PDO_VAR(min_mv, max_mv, max_ma)				\
-	(PDO_TYPE(PDO_TYPE_VAR) | PDO_VAR_MIN_VOLT(min_mv) |	\
-	 PDO_VAR_MAX_VOLT(max_mv) | PDO_VAR_MAX_CURR(max_ma))
-
-enum pd_apdo_type {
-	APDO_TYPE_PPS = 0,
-};
-
-#define PDO_APDO_TYPE_SHIFT	28	/* Only valid value currently is 0x0 - PPS */
-#define PDO_APDO_TYPE_MASK	0x3
-
-#define PDO_APDO_TYPE(t)	((t) << PDO_APDO_TYPE_SHIFT)
-
-#define PDO_PPS_APDO_MAX_VOLT_SHIFT	17	/* 100mV units */
-#define PDO_PPS_APDO_MIN_VOLT_SHIFT	8	/* 100mV units */
-#define PDO_PPS_APDO_MAX_CURR_SHIFT	0	/* 50mA units */
-
-#define PDO_PPS_APDO_VOLT_MASK	0xff
-#define PDO_PPS_APDO_CURR_MASK	0x7f
-
-#define PDO_PPS_APDO_MIN_VOLT(mv)	\
-	((((mv) / 100) & PDO_PPS_APDO_VOLT_MASK) << PDO_PPS_APDO_MIN_VOLT_SHIFT)
-#define PDO_PPS_APDO_MAX_VOLT(mv)	\
-	((((mv) / 100) & PDO_PPS_APDO_VOLT_MASK) << PDO_PPS_APDO_MAX_VOLT_SHIFT)
-#define PDO_PPS_APDO_MAX_CURR(ma)	\
-	((((ma) / 50) & PDO_PPS_APDO_CURR_MASK) << PDO_PPS_APDO_MAX_CURR_SHIFT)
-
-#define PDO_PPS_APDO(min_mv, max_mv, max_ma)				\
-	(PDO_TYPE(PDO_TYPE_APDO) | PDO_APDO_TYPE(APDO_TYPE_PPS) |	\
-	PDO_PPS_APDO_MIN_VOLT(min_mv) | PDO_PPS_APDO_MAX_VOLT(max_mv) |	\
-	PDO_PPS_APDO_MAX_CURR(max_ma))
-
-static inline enum pd_pdo_type pdo_type(u32 pdo)
-{
-	return (pdo >> PDO_TYPE_SHIFT) & PDO_TYPE_MASK;
-}
-
-static inline unsigned int pdo_fixed_voltage(u32 pdo)
-{
-	return ((pdo >> PDO_FIXED_VOLT_SHIFT) & PDO_VOLT_MASK) * 50;
-}
-
-static inline unsigned int pdo_min_voltage(u32 pdo)
-{
-	return ((pdo >> PDO_VAR_MIN_VOLT_SHIFT) & PDO_VOLT_MASK) * 50;
-}
-
-static inline unsigned int pdo_max_voltage(u32 pdo)
-{
-	return ((pdo >> PDO_VAR_MAX_VOLT_SHIFT) & PDO_VOLT_MASK) * 50;
-}
-
-static inline unsigned int pdo_max_current(u32 pdo)
-{
-	return ((pdo >> PDO_VAR_MAX_CURR_SHIFT) & PDO_CURR_MASK) * 10;
-}
-
-static inline unsigned int pdo_max_power(u32 pdo)
-{
-	return ((pdo >> PDO_BATT_MAX_PWR_SHIFT) & PDO_PWR_MASK) * 250;
-}
-
-static inline enum pd_apdo_type pdo_apdo_type(u32 pdo)
-{
-	return (pdo >> PDO_APDO_TYPE_SHIFT) & PDO_APDO_TYPE_MASK;
-}
-
-static inline unsigned int pdo_pps_apdo_min_voltage(u32 pdo)
-{
-	return ((pdo >> PDO_PPS_APDO_MIN_VOLT_SHIFT) &
-		PDO_PPS_APDO_VOLT_MASK) * 100;
-}
-
-static inline unsigned int pdo_pps_apdo_max_voltage(u32 pdo)
-{
-	return ((pdo >> PDO_PPS_APDO_MAX_VOLT_SHIFT) &
-		PDO_PPS_APDO_VOLT_MASK) * 100;
-}
-
-static inline unsigned int pdo_pps_apdo_max_current(u32 pdo)
-{
-	return ((pdo >> PDO_PPS_APDO_MAX_CURR_SHIFT) &
-		PDO_PPS_APDO_CURR_MASK) * 50;
-}
-
-/* RDO: Request Data Object */
-#define RDO_OBJ_POS_SHIFT	28
-#define RDO_OBJ_POS_MASK	0x7
-#define RDO_GIVE_BACK		BIT(27)	/* Supports reduced operating current */
-#define RDO_CAP_MISMATCH	BIT(26) /* Not satisfied by source caps */
-#define RDO_USB_COMM		BIT(25) /* USB communications capable */
-#define RDO_NO_SUSPEND		BIT(24) /* USB Suspend not supported */
-
-#define RDO_PWR_MASK			0x3ff
-#define RDO_CURR_MASK			0x3ff
-
-#define RDO_FIXED_OP_CURR_SHIFT		10
-#define RDO_FIXED_MAX_CURR_SHIFT	0
-
-#define RDO_OBJ(idx) (((idx) & RDO_OBJ_POS_MASK) << RDO_OBJ_POS_SHIFT)
-
-#define PDO_FIXED_OP_CURR(ma) ((((ma) / 10) & RDO_CURR_MASK) << RDO_FIXED_OP_CURR_SHIFT)
-#define PDO_FIXED_MAX_CURR(ma) ((((ma) / 10) & RDO_CURR_MASK) << RDO_FIXED_MAX_CURR_SHIFT)
-
-#define RDO_FIXED(idx, op_ma, max_ma, flags)			\
-	(RDO_OBJ(idx) | (flags) |				\
-	 PDO_FIXED_OP_CURR(op_ma) | PDO_FIXED_MAX_CURR(max_ma))
-
-#define RDO_BATT_OP_PWR_SHIFT		10	/* 250mW units */
-#define RDO_BATT_MAX_PWR_SHIFT		0	/* 250mW units */
-
-#define RDO_BATT_OP_PWR(mw) ((((mw) / 250) & RDO_PWR_MASK) << RDO_BATT_OP_PWR_SHIFT)
-#define RDO_BATT_MAX_PWR(mw) ((((mw) / 250) & RDO_PWR_MASK) << RDO_BATT_MAX_PWR_SHIFT)
-
-#define RDO_BATT(idx, op_mw, max_mw, flags)			\
-	(RDO_OBJ(idx) | (flags) |				\
-	 RDO_BATT_OP_PWR(op_mw) | RDO_BATT_MAX_PWR(max_mw))
-
-#define RDO_PROG_VOLT_MASK	0x7ff
-#define RDO_PROG_CURR_MASK	0x7f
-
-#define RDO_PROG_VOLT_SHIFT	9
-#define RDO_PROG_CURR_SHIFT	0
-
-#define RDO_PROG_VOLT_MV_STEP	20
-#define RDO_PROG_CURR_MA_STEP	50
-
-#define PDO_PROG_OUT_VOLT(mv)	\
-	((((mv) / RDO_PROG_VOLT_MV_STEP) & RDO_PROG_VOLT_MASK) << RDO_PROG_VOLT_SHIFT)
-#define PDO_PROG_OP_CURR(ma)	\
-	((((ma) / RDO_PROG_CURR_MA_STEP) & RDO_PROG_CURR_MASK) << RDO_PROG_CURR_SHIFT)
-
-#define RDO_PROG(idx, out_mv, op_ma, flags)			\
-	(RDO_OBJ(idx) | (flags) |				\
-	 PDO_PROG_OUT_VOLT(out_mv) | PDO_PROG_OP_CURR(op_ma))
-
-static inline unsigned int rdo_index(u32 rdo)
-{
-	return (rdo >> RDO_OBJ_POS_SHIFT) & RDO_OBJ_POS_MASK;
-}
-
-static inline unsigned int rdo_op_current(u32 rdo)
-{
-	return ((rdo >> RDO_FIXED_OP_CURR_SHIFT) & RDO_CURR_MASK) * 10;
-}
-
-static inline unsigned int rdo_max_current(u32 rdo)
-{
-	return ((rdo >> RDO_FIXED_MAX_CURR_SHIFT) &
-		RDO_CURR_MASK) * 10;
-}
-
-static inline unsigned int rdo_op_power(u32 rdo)
-{
-	return ((rdo >> RDO_BATT_OP_PWR_SHIFT) & RDO_PWR_MASK) * 250;
-}
-
-static inline unsigned int rdo_max_power(u32 rdo)
-{
-	return ((rdo >> RDO_BATT_MAX_PWR_SHIFT) & RDO_PWR_MASK) * 250;
-}
-
-/* Enter_USB Data Object */
-#define EUDO_USB_MODE_MASK		GENMASK(30, 28)
-#define EUDO_USB_MODE_SHIFT		28
-#define   EUDO_USB_MODE_USB2		0
-#define   EUDO_USB_MODE_USB3		1
-#define   EUDO_USB_MODE_USB4		2
-#define EUDO_USB4_DRD			BIT(26)
-#define EUDO_USB3_DRD			BIT(25)
-#define EUDO_CABLE_SPEED_MASK		GENMASK(23, 21)
-#define EUDO_CABLE_SPEED_SHIFT		21
-#define   EUDO_CABLE_SPEED_USB2		0
-#define   EUDO_CABLE_SPEED_USB3_GEN1	1
-#define   EUDO_CABLE_SPEED_USB4_GEN2	2
-#define   EUDO_CABLE_SPEED_USB4_GEN3	3
-#define EUDO_CABLE_TYPE_MASK		GENMASK(20, 19)
-#define EUDO_CABLE_TYPE_SHIFT		19
-#define   EUDO_CABLE_TYPE_PASSIVE	0
-#define   EUDO_CABLE_TYPE_RE_TIMER	1
-#define   EUDO_CABLE_TYPE_RE_DRIVER	2
-#define   EUDO_CABLE_TYPE_OPTICAL	3
-#define EUDO_CABLE_CURRENT_MASK		GENMASK(18, 17)
-#define EUDO_CABLE_CURRENT_SHIFT	17
-#define   EUDO_CABLE_CURRENT_NOTSUPP	0
-#define   EUDO_CABLE_CURRENT_3A		2
-#define   EUDO_CABLE_CURRENT_5A		3
-#define EUDO_PCIE_SUPPORT		BIT(16)
-#define EUDO_DP_SUPPORT			BIT(15)
-#define EUDO_TBT_SUPPORT		BIT(14)
-#define EUDO_HOST_PRESENT		BIT(13)
-
-/* USB PD timers and counters */
-#define PD_T_NO_RESPONSE	5000	/* 4.5 - 5.5 seconds */
-#define PD_T_DB_DETECT		10000	/* 10 - 15 seconds */
-#define PD_T_SEND_SOURCE_CAP	150	/* 100 - 200 ms */
-#define PD_T_SENDER_RESPONSE	60	/* 24 - 30 ms, relaxed */
-#define PD_T_RECEIVER_RESPONSE	15	/* 15ms max */
-#define PD_T_SOURCE_ACTIVITY	45
-#define PD_T_SINK_ACTIVITY	135
-#define PD_T_SINK_WAIT_CAP	310	/* 310 - 620 ms */
-#define PD_T_PS_TRANSITION	500
-#define PD_T_SRC_TRANSITION	35
-#define PD_T_DRP_SNK		40
-#define PD_T_DRP_SRC		30
-#define PD_T_PS_SOURCE_OFF	920
-#define PD_T_PS_SOURCE_ON	480
-#define PD_T_PS_SOURCE_ON_PRS	450	/* 390 - 480ms */
-#define PD_T_PS_HARD_RESET	30
-#define PD_T_SRC_RECOVER	760
-#define PD_T_SRC_RECOVER_MAX	1000
-#define PD_T_SRC_TURN_ON	275
-#define PD_T_SAFE_0V		650
-#define PD_T_VCONN_SOURCE_ON	100
-#define PD_T_SINK_REQUEST	100	/* 100 ms minimum */
-#define PD_T_ERROR_RECOVERY	100	/* minimum 25 is insufficient */
-#define PD_T_SRCSWAPSTDBY	625	/* Maximum of 650ms */
-#define PD_T_NEWSRC		250	/* Maximum of 275ms */
-#define PD_T_SWAP_SRC_START	20	/* Minimum of 20ms */
-#define PD_T_BIST_CONT_MODE	50	/* 30 - 60 ms */
-#define PD_T_SINK_TX		16	/* 16 - 20 ms */
-#define PD_T_CHUNK_NOT_SUPP	42	/* 40 - 50 ms */
-
-#define PD_T_DRP_TRY		100	/* 75 - 150 ms */
-#define PD_T_DRP_TRYWAIT	600	/* 400 - 800 ms */
-
-#define PD_T_CC_DEBOUNCE	200	/* 100 - 200 ms */
-#define PD_T_PD_DEBOUNCE	20	/* 10 - 20 ms */
-#define PD_T_TRY_CC_DEBOUNCE	15	/* 10 - 20 ms */
-
-#define PD_N_CAPS_COUNT		(PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
-#define PD_N_HARD_RESET_COUNT	2
-
-#define PD_P_SNK_STDBY_MW	2500	/* 2500 mW */
-
 #endif /* __LINUX_USB_PD_H */
diff --git a/include/uapi/linux/usb/pd.h b/include/uapi/linux/usb/pd.h
new file mode 100644
index 0000000000000..407d8deea3ea9
--- /dev/null
+++ b/include/uapi/linux/usb/pd.h
@@ -0,0 +1,496 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright 2015-2017 Google, Inc
+ */
+
+#ifndef _UAPI__LINUX_USB_PD_H
+#define _UAPI__LINUX_USB_PD_H
+
+#include <linux/types.h>
+#include <asm/byteorder.h>	/* le16_to_cpu */
+
+/* USB PD Messages */
+enum pd_ctrl_msg_type {
+	/* 0 Reserved */
+	PD_CTRL_GOOD_CRC = 1,
+	PD_CTRL_GOTO_MIN = 2,
+	PD_CTRL_ACCEPT = 3,
+	PD_CTRL_REJECT = 4,
+	PD_CTRL_PING = 5,
+	PD_CTRL_PS_RDY = 6,
+	PD_CTRL_GET_SOURCE_CAP = 7,
+	PD_CTRL_GET_SINK_CAP = 8,
+	PD_CTRL_DR_SWAP = 9,
+	PD_CTRL_PR_SWAP = 10,
+	PD_CTRL_VCONN_SWAP = 11,
+	PD_CTRL_WAIT = 12,
+	PD_CTRL_SOFT_RESET = 13,
+	/* 14-15 Reserved */
+	PD_CTRL_NOT_SUPP = 16,
+	PD_CTRL_GET_SOURCE_CAP_EXT = 17,
+	PD_CTRL_GET_STATUS = 18,
+	PD_CTRL_FR_SWAP = 19,
+	PD_CTRL_GET_PPS_STATUS = 20,
+	PD_CTRL_GET_COUNTRY_CODES = 21,
+	/* 22-31 Reserved */
+};
+
+enum pd_data_msg_type {
+	/* 0 Reserved */
+	PD_DATA_SOURCE_CAP = 1,
+	PD_DATA_REQUEST = 2,
+	PD_DATA_BIST = 3,
+	PD_DATA_SINK_CAP = 4,
+	PD_DATA_BATT_STATUS = 5,
+	PD_DATA_ALERT = 6,
+	PD_DATA_GET_COUNTRY_INFO = 7,
+	PD_DATA_ENTER_USB = 8,
+	/* 9-14 Reserved */
+	PD_DATA_VENDOR_DEF = 15,
+	/* 16-31 Reserved */
+};
+
+enum pd_ext_msg_type {
+	/* 0 Reserved */
+	PD_EXT_SOURCE_CAP_EXT = 1,
+	PD_EXT_STATUS = 2,
+	PD_EXT_GET_BATT_CAP = 3,
+	PD_EXT_GET_BATT_STATUS = 4,
+	PD_EXT_BATT_CAP = 5,
+	PD_EXT_GET_MANUFACTURER_INFO = 6,
+	PD_EXT_MANUFACTURER_INFO = 7,
+	PD_EXT_SECURITY_REQUEST = 8,
+	PD_EXT_SECURITY_RESPONSE = 9,
+	PD_EXT_FW_UPDATE_REQUEST = 10,
+	PD_EXT_FW_UPDATE_RESPONSE = 11,
+	PD_EXT_PPS_STATUS = 12,
+	PD_EXT_COUNTRY_INFO = 13,
+	PD_EXT_COUNTRY_CODES = 14,
+	/* 15-31 Reserved */
+};
+
+#define PD_REV10	0x0
+#define PD_REV20	0x1
+#define PD_REV30	0x2
+
+#define PD_HEADER_EXT_HDR	BIT(15)
+#define PD_HEADER_CNT_SHIFT	12
+#define PD_HEADER_CNT_MASK	0x7
+#define PD_HEADER_ID_SHIFT	9
+#define PD_HEADER_ID_MASK	0x7
+#define PD_HEADER_PWR_ROLE	BIT(8)
+#define PD_HEADER_REV_SHIFT	6
+#define PD_HEADER_REV_MASK	0x3
+#define PD_HEADER_DATA_ROLE	BIT(5)
+#define PD_HEADER_TYPE_SHIFT	0
+#define PD_HEADER_TYPE_MASK	0x1f
+
+#define PD_HEADER(type, pwr, data, rev, id, cnt, ext_hdr)		\
+	((((type) & PD_HEADER_TYPE_MASK) << PD_HEADER_TYPE_SHIFT) |	\
+	 ((pwr) ? PD_HEADER_PWR_ROLE : 0) |				\
+	 ((data) ? PD_HEADER_DATA_ROLE : 0) |				\
+	 ((rev) << PD_HEADER_REV_SHIFT) |					\
+	 (((id) & PD_HEADER_ID_MASK) << PD_HEADER_ID_SHIFT) |		\
+	 (((cnt) & PD_HEADER_CNT_MASK) << PD_HEADER_CNT_SHIFT) |	\
+	 ((ext_hdr) ? PD_HEADER_EXT_HDR : 0))
+
+#define PD_HEADER_LE(type, pwr, data, rev, id, cnt) \
+	cpu_to_le16(PD_HEADER((type), (pwr), (data), (rev), (id), (cnt), (0)))
+
+static inline unsigned int pd_header_cnt(__u16 header)
+{
+	return (header >> PD_HEADER_CNT_SHIFT) & PD_HEADER_CNT_MASK;
+}
+
+static inline unsigned int pd_header_cnt_le(__le16 header)
+{
+	return pd_header_cnt(le16_to_cpu(header));
+}
+
+static inline unsigned int pd_header_type(__u16 header)
+{
+	return (header >> PD_HEADER_TYPE_SHIFT) & PD_HEADER_TYPE_MASK;
+}
+
+static inline unsigned int pd_header_type_le(__le16 header)
+{
+	return pd_header_type(le16_to_cpu(header));
+}
+
+static inline unsigned int pd_header_msgid(__u16 header)
+{
+	return (header >> PD_HEADER_ID_SHIFT) & PD_HEADER_ID_MASK;
+}
+
+static inline unsigned int pd_header_msgid_le(__le16 header)
+{
+	return pd_header_msgid(le16_to_cpu(header));
+}
+
+static inline unsigned int pd_header_rev(__u16 header)
+{
+	return (header >> PD_HEADER_REV_SHIFT) & PD_HEADER_REV_MASK;
+}
+
+static inline unsigned int pd_header_rev_le(__le16 header)
+{
+	return pd_header_rev(le16_to_cpu(header));
+}
+
+#define PD_EXT_HDR_CHUNKED		BIT(15)
+#define PD_EXT_HDR_CHUNK_NUM_SHIFT	11
+#define PD_EXT_HDR_CHUNK_NUM_MASK	0xf
+#define PD_EXT_HDR_REQ_CHUNK		BIT(10)
+#define PD_EXT_HDR_DATA_SIZE_SHIFT	0
+#define PD_EXT_HDR_DATA_SIZE_MASK	0x1ff
+
+#define PD_EXT_HDR(data_size, req_chunk, chunk_num, chunked)				\
+	((((data_size) & PD_EXT_HDR_DATA_SIZE_MASK) << PD_EXT_HDR_DATA_SIZE_SHIFT) |	\
+	 ((req_chunk) ? PD_EXT_HDR_REQ_CHUNK : 0) |					\
+	 (((chunk_num) & PD_EXT_HDR_CHUNK_NUM_MASK) << PD_EXT_HDR_CHUNK_NUM_SHIFT) |	\
+	 ((chunked) ? PD_EXT_HDR_CHUNKED : 0))
+
+#define PD_EXT_HDR_LE(data_size, req_chunk, chunk_num, chunked) \
+	cpu_to_le16(PD_EXT_HDR((data_size), (req_chunk), (chunk_num), (chunked)))
+
+static inline unsigned int pd_ext_header_chunk_num(__u16 ext_header)
+{
+	return (ext_header >> PD_EXT_HDR_CHUNK_NUM_SHIFT) &
+		PD_EXT_HDR_CHUNK_NUM_MASK;
+}
+
+static inline unsigned int pd_ext_header_data_size(__u16 ext_header)
+{
+	return (ext_header >> PD_EXT_HDR_DATA_SIZE_SHIFT) &
+		PD_EXT_HDR_DATA_SIZE_MASK;
+}
+
+static inline unsigned int pd_ext_header_data_size_le(__le16 ext_header)
+{
+	return pd_ext_header_data_size(le16_to_cpu(ext_header));
+}
+
+#define PD_MAX_PAYLOAD		7
+#define PD_EXT_MAX_CHUNK_DATA	26
+
+/**
+ * struct pd_chunked_ext_message_data - PD chunked extended message data as
+ *					 seen on wire
+ * @header:    PD extended message header
+ * @data:      PD extended message data
+ */
+struct pd_chunked_ext_message_data {
+	__le16 header;
+	__u8 data[PD_EXT_MAX_CHUNK_DATA];
+} __attribute__ ((packed));
+
+/**
+ * struct pd_message - PD message as seen on wire
+ * @header:    PD message header
+ * @payload:   PD message payload
+ * @ext_msg:   PD message chunked extended message data
+ */
+struct pd_message {
+	__le16 header;
+	union {
+		__le32 payload[PD_MAX_PAYLOAD];
+		struct pd_chunked_ext_message_data ext_msg;
+	};
+} __attribute__ ((packed));
+
+/* PDO: Power Data Object */
+#define PDO_MAX_OBJECTS		7
+
+enum pd_pdo_type {
+	PDO_TYPE_FIXED = 0,
+	PDO_TYPE_BATT = 1,
+	PDO_TYPE_VAR = 2,
+	PDO_TYPE_APDO = 3,
+};
+
+#define PDO_TYPE_SHIFT		30
+#define PDO_TYPE_MASK		0x3
+
+#define PDO_TYPE(t)	((t) << PDO_TYPE_SHIFT)
+
+#define PDO_VOLT_MASK		0x3ff
+#define PDO_CURR_MASK		0x3ff
+#define PDO_PWR_MASK		0x3ff
+
+#define PDO_FIXED_DUAL_ROLE		BIT(29)	/* Power role swap supported */
+#define PDO_FIXED_SUSPEND		BIT(28) /* USB Suspend supported (Source) */
+#define PDO_FIXED_HIGHER_CAP		BIT(28) /* Requires more than vSafe5V (Sink) */
+#define PDO_FIXED_EXTPOWER		BIT(27) /* Externally powered */
+#define PDO_FIXED_USB_COMM		BIT(26) /* USB communications capable */
+#define PDO_FIXED_DATA_SWAP		BIT(25) /* Data role swap supported */
+#define PDO_FIXED_UNCHUNK_EXT		BIT(24) /* Unchunked Extended Message supported (Source) */
+#define PDO_FIXED_FRS_CURR_MASK		(BIT(24) | BIT(23)) /* FR_Swap Current (Sink) */
+#define PDO_FIXED_FRS_CURR_SHIFT	23
+#define PDO_FIXED_VOLT_SHIFT		10	/* 50mV units */
+#define PDO_FIXED_CURR_SHIFT		0	/* 10mA units */
+
+#define PDO_FIXED_VOLT(mv)	((((mv) / 50) & PDO_VOLT_MASK) << PDO_FIXED_VOLT_SHIFT)
+#define PDO_FIXED_CURR(ma)	((((ma) / 10) & PDO_CURR_MASK) << PDO_FIXED_CURR_SHIFT)
+
+#define PDO_FIXED(mv, ma, flags)			\
+	(PDO_TYPE(PDO_TYPE_FIXED) | (flags) |		\
+	 PDO_FIXED_VOLT(mv) | PDO_FIXED_CURR(ma))
+
+#define VSAFE5V 5000 /* mv units */
+
+#define PDO_BATT_MAX_VOLT_SHIFT	20	/* 50mV units */
+#define PDO_BATT_MIN_VOLT_SHIFT	10	/* 50mV units */
+#define PDO_BATT_MAX_PWR_SHIFT	0	/* 250mW units */
+
+#define PDO_BATT_MIN_VOLT(mv) ((((mv) / 50) & PDO_VOLT_MASK) << PDO_BATT_MIN_VOLT_SHIFT)
+#define PDO_BATT_MAX_VOLT(mv) ((((mv) / 50) & PDO_VOLT_MASK) << PDO_BATT_MAX_VOLT_SHIFT)
+#define PDO_BATT_MAX_POWER(mw) ((((mw) / 250) & PDO_PWR_MASK) << PDO_BATT_MAX_PWR_SHIFT)
+
+#define PDO_BATT(min_mv, max_mv, max_mw)			\
+	(PDO_TYPE(PDO_TYPE_BATT) | PDO_BATT_MIN_VOLT(min_mv) |	\
+	 PDO_BATT_MAX_VOLT(max_mv) | PDO_BATT_MAX_POWER(max_mw))
+
+#define PDO_VAR_MAX_VOLT_SHIFT	20	/* 50mV units */
+#define PDO_VAR_MIN_VOLT_SHIFT	10	/* 50mV units */
+#define PDO_VAR_MAX_CURR_SHIFT	0	/* 10mA units */
+
+#define PDO_VAR_MIN_VOLT(mv) ((((mv) / 50) & PDO_VOLT_MASK) << PDO_VAR_MIN_VOLT_SHIFT)
+#define PDO_VAR_MAX_VOLT(mv) ((((mv) / 50) & PDO_VOLT_MASK) << PDO_VAR_MAX_VOLT_SHIFT)
+#define PDO_VAR_MAX_CURR(ma) ((((ma) / 10) & PDO_CURR_MASK) << PDO_VAR_MAX_CURR_SHIFT)
+
+#define PDO_VAR(min_mv, max_mv, max_ma)				\
+	(PDO_TYPE(PDO_TYPE_VAR) | PDO_VAR_MIN_VOLT(min_mv) |	\
+	 PDO_VAR_MAX_VOLT(max_mv) | PDO_VAR_MAX_CURR(max_ma))
+
+enum pd_apdo_type {
+	APDO_TYPE_PPS = 0,
+};
+
+#define PDO_APDO_TYPE_SHIFT	28	/* Only valid value currently is 0x0 - PPS */
+#define PDO_APDO_TYPE_MASK	0x3
+
+#define PDO_APDO_TYPE(t)	((t) << PDO_APDO_TYPE_SHIFT)
+
+#define PDO_PPS_APDO_MAX_VOLT_SHIFT	17	/* 100mV units */
+#define PDO_PPS_APDO_MIN_VOLT_SHIFT	8	/* 100mV units */
+#define PDO_PPS_APDO_MAX_CURR_SHIFT	0	/* 50mA units */
+
+#define PDO_PPS_APDO_VOLT_MASK	0xff
+#define PDO_PPS_APDO_CURR_MASK	0x7f
+
+#define PDO_PPS_APDO_MIN_VOLT(mv)	\
+	((((mv) / 100) & PDO_PPS_APDO_VOLT_MASK) << PDO_PPS_APDO_MIN_VOLT_SHIFT)
+#define PDO_PPS_APDO_MAX_VOLT(mv)	\
+	((((mv) / 100) & PDO_PPS_APDO_VOLT_MASK) << PDO_PPS_APDO_MAX_VOLT_SHIFT)
+#define PDO_PPS_APDO_MAX_CURR(ma)	\
+	((((ma) / 50) & PDO_PPS_APDO_CURR_MASK) << PDO_PPS_APDO_MAX_CURR_SHIFT)
+
+#define PDO_PPS_APDO(min_mv, max_mv, max_ma)				\
+	(PDO_TYPE(PDO_TYPE_APDO) | PDO_APDO_TYPE(APDO_TYPE_PPS) |	\
+	PDO_PPS_APDO_MIN_VOLT(min_mv) | PDO_PPS_APDO_MAX_VOLT(max_mv) |	\
+	PDO_PPS_APDO_MAX_CURR(max_ma))
+
+static inline enum pd_pdo_type pdo_type(__u32 pdo)
+{
+	return (pdo >> PDO_TYPE_SHIFT) & PDO_TYPE_MASK;
+}
+
+static inline unsigned int pdo_fixed_voltage(__u32 pdo)
+{
+	return ((pdo >> PDO_FIXED_VOLT_SHIFT) & PDO_VOLT_MASK) * 50;
+}
+
+static inline unsigned int pdo_min_voltage(__u32 pdo)
+{
+	return ((pdo >> PDO_VAR_MIN_VOLT_SHIFT) & PDO_VOLT_MASK) * 50;
+}
+
+static inline unsigned int pdo_max_voltage(__u32 pdo)
+{
+	return ((pdo >> PDO_VAR_MAX_VOLT_SHIFT) & PDO_VOLT_MASK) * 50;
+}
+
+static inline unsigned int pdo_max_current(__u32 pdo)
+{
+	return ((pdo >> PDO_VAR_MAX_CURR_SHIFT) & PDO_CURR_MASK) * 10;
+}
+
+static inline unsigned int pdo_max_power(__u32 pdo)
+{
+	return ((pdo >> PDO_BATT_MAX_PWR_SHIFT) & PDO_PWR_MASK) * 250;
+}
+
+static inline enum pd_apdo_type pdo_apdo_type(__u32 pdo)
+{
+	return (pdo >> PDO_APDO_TYPE_SHIFT) & PDO_APDO_TYPE_MASK;
+}
+
+static inline unsigned int pdo_pps_apdo_min_voltage(__u32 pdo)
+{
+	return ((pdo >> PDO_PPS_APDO_MIN_VOLT_SHIFT) &
+		PDO_PPS_APDO_VOLT_MASK) * 100;
+}
+
+static inline unsigned int pdo_pps_apdo_max_voltage(__u32 pdo)
+{
+	return ((pdo >> PDO_PPS_APDO_MAX_VOLT_SHIFT) &
+		PDO_PPS_APDO_VOLT_MASK) * 100;
+}
+
+static inline unsigned int pdo_pps_apdo_max_current(__u32 pdo)
+{
+	return ((pdo >> PDO_PPS_APDO_MAX_CURR_SHIFT) &
+		PDO_PPS_APDO_CURR_MASK) * 50;
+}
+
+/* RDO: Request Data Object */
+#define RDO_OBJ_POS_SHIFT	28
+#define RDO_OBJ_POS_MASK	0x7
+#define RDO_GIVE_BACK		BIT(27)	/* Supports reduced operating current */
+#define RDO_CAP_MISMATCH	BIT(26) /* Not satisfied by source caps */
+#define RDO_USB_COMM		BIT(25) /* USB communications capable */
+#define RDO_NO_SUSPEND		BIT(24) /* USB Suspend not supported */
+
+#define RDO_PWR_MASK			0x3ff
+#define RDO_CURR_MASK			0x3ff
+
+#define RDO_FIXED_OP_CURR_SHIFT		10
+#define RDO_FIXED_MAX_CURR_SHIFT	0
+
+#define RDO_OBJ(idx) (((idx) & RDO_OBJ_POS_MASK) << RDO_OBJ_POS_SHIFT)
+
+#define PDO_FIXED_OP_CURR(ma) ((((ma) / 10) & RDO_CURR_MASK) << RDO_FIXED_OP_CURR_SHIFT)
+#define PDO_FIXED_MAX_CURR(ma) ((((ma) / 10) & RDO_CURR_MASK) << RDO_FIXED_MAX_CURR_SHIFT)
+
+#define RDO_FIXED(idx, op_ma, max_ma, flags)			\
+	(RDO_OBJ(idx) | (flags) |				\
+	 PDO_FIXED_OP_CURR(op_ma) | PDO_FIXED_MAX_CURR(max_ma))
+
+#define RDO_BATT_OP_PWR_SHIFT		10	/* 250mW units */
+#define RDO_BATT_MAX_PWR_SHIFT		0	/* 250mW units */
+
+#define RDO_BATT_OP_PWR(mw) ((((mw) / 250) & RDO_PWR_MASK) << RDO_BATT_OP_PWR_SHIFT)
+#define RDO_BATT_MAX_PWR(mw) ((((mw) / 250) & RDO_PWR_MASK) << RDO_BATT_MAX_PWR_SHIFT)
+
+#define RDO_BATT(idx, op_mw, max_mw, flags)			\
+	(RDO_OBJ(idx) | (flags) |				\
+	 RDO_BATT_OP_PWR(op_mw) | RDO_BATT_MAX_PWR(max_mw))
+
+#define RDO_PROG_VOLT_MASK	0x7ff
+#define RDO_PROG_CURR_MASK	0x7f
+
+#define RDO_PROG_VOLT_SHIFT	9
+#define RDO_PROG_CURR_SHIFT	0
+
+#define RDO_PROG_VOLT_MV_STEP	20
+#define RDO_PROG_CURR_MA_STEP	50
+
+#define PDO_PROG_OUT_VOLT(mv)	\
+	((((mv) / RDO_PROG_VOLT_MV_STEP) & RDO_PROG_VOLT_MASK) << RDO_PROG_VOLT_SHIFT)
+#define PDO_PROG_OP_CURR(ma)	\
+	((((ma) / RDO_PROG_CURR_MA_STEP) & RDO_PROG_CURR_MASK) << RDO_PROG_CURR_SHIFT)
+
+#define RDO_PROG(idx, out_mv, op_ma, flags)			\
+	(RDO_OBJ(idx) | (flags) |				\
+	 PDO_PROG_OUT_VOLT(out_mv) | PDO_PROG_OP_CURR(op_ma))
+
+static inline unsigned int rdo_index(__u32 rdo)
+{
+	return (rdo >> RDO_OBJ_POS_SHIFT) & RDO_OBJ_POS_MASK;
+}
+
+static inline unsigned int rdo_op_current(__u32 rdo)
+{
+	return ((rdo >> RDO_FIXED_OP_CURR_SHIFT) & RDO_CURR_MASK) * 10;
+}
+
+static inline unsigned int rdo_max_current(__u32 rdo)
+{
+	return ((rdo >> RDO_FIXED_MAX_CURR_SHIFT) &
+		RDO_CURR_MASK) * 10;
+}
+
+static inline unsigned int rdo_op_power(__u32 rdo)
+{
+	return ((rdo >> RDO_BATT_OP_PWR_SHIFT) & RDO_PWR_MASK) * 250;
+}
+
+static inline unsigned int rdo_max_power(__u32 rdo)
+{
+	return ((rdo >> RDO_BATT_MAX_PWR_SHIFT) & RDO_PWR_MASK) * 250;
+}
+
+/* Enter_USB Data Object */
+#define EUDO_USB_MODE_MASK		GENMASK(30, 28)
+#define EUDO_USB_MODE_SHIFT		28
+#define   EUDO_USB_MODE_USB2		0
+#define   EUDO_USB_MODE_USB3		1
+#define   EUDO_USB_MODE_USB4		2
+#define EUDO_USB4_DRD			BIT(26)
+#define EUDO_USB3_DRD			BIT(25)
+#define EUDO_CABLE_SPEED_MASK		GENMASK(23, 21)
+#define EUDO_CABLE_SPEED_SHIFT		21
+#define   EUDO_CABLE_SPEED_USB2		0
+#define   EUDO_CABLE_SPEED_USB3_GEN1	1
+#define   EUDO_CABLE_SPEED_USB4_GEN2	2
+#define   EUDO_CABLE_SPEED_USB4_GEN3	3
+#define EUDO_CABLE_TYPE_MASK		GENMASK(20, 19)
+#define EUDO_CABLE_TYPE_SHIFT		19
+#define   EUDO_CABLE_TYPE_PASSIVE	0
+#define   EUDO_CABLE_TYPE_RE_TIMER	1
+#define   EUDO_CABLE_TYPE_RE_DRIVER	2
+#define   EUDO_CABLE_TYPE_OPTICAL	3
+#define EUDO_CABLE_CURRENT_MASK		GENMASK(18, 17)
+#define EUDO_CABLE_CURRENT_SHIFT	17
+#define   EUDO_CABLE_CURRENT_NOTSUPP	0
+#define   EUDO_CABLE_CURRENT_3A		2
+#define   EUDO_CABLE_CURRENT_5A		3
+#define EUDO_PCIE_SUPPORT		BIT(16)
+#define EUDO_DP_SUPPORT			BIT(15)
+#define EUDO_TBT_SUPPORT		BIT(14)
+#define EUDO_HOST_PRESENT		BIT(13)
+
+/* USB PD timers and counters */
+#define PD_T_NO_RESPONSE	5000	/* 4.5 - 5.5 seconds */
+#define PD_T_DB_DETECT		10000	/* 10 - 15 seconds */
+#define PD_T_SEND_SOURCE_CAP	150	/* 100 - 200 ms */
+#define PD_T_SENDER_RESPONSE	60	/* 24 - 30 ms, relaxed */
+#define PD_T_RECEIVER_RESPONSE	15	/* 15ms max */
+#define PD_T_SOURCE_ACTIVITY	45
+#define PD_T_SINK_ACTIVITY	135
+#define PD_T_SINK_WAIT_CAP	310	/* 310 - 620 ms */
+#define PD_T_PS_TRANSITION	500
+#define PD_T_SRC_TRANSITION	35
+#define PD_T_DRP_SNK		40
+#define PD_T_DRP_SRC		30
+#define PD_T_PS_SOURCE_OFF	920
+#define PD_T_PS_SOURCE_ON	480
+#define PD_T_PS_SOURCE_ON_PRS	450	/* 390 - 480ms */
+#define PD_T_PS_HARD_RESET	30
+#define PD_T_SRC_RECOVER	760
+#define PD_T_SRC_RECOVER_MAX	1000
+#define PD_T_SRC_TURN_ON	275
+#define PD_T_SAFE_0V		650
+#define PD_T_VCONN_SOURCE_ON	100
+#define PD_T_SINK_REQUEST	100	/* 100 ms minimum */
+#define PD_T_ERROR_RECOVERY	100	/* minimum 25 is insufficient */
+#define PD_T_SRCSWAPSTDBY	625	/* Maximum of 650ms */
+#define PD_T_NEWSRC		250	/* Maximum of 275ms */
+#define PD_T_SWAP_SRC_START	20	/* Minimum of 20ms */
+#define PD_T_BIST_CONT_MODE	50	/* 30 - 60 ms */
+#define PD_T_SINK_TX		16	/* 16 - 20 ms */
+#define PD_T_CHUNK_NOT_SUPP	42	/* 40 - 50 ms */
+
+#define PD_T_DRP_TRY		100	/* 75 - 150 ms */
+#define PD_T_DRP_TRYWAIT	600	/* 400 - 800 ms */
+
+#define PD_T_CC_DEBOUNCE	200	/* 100 - 200 ms */
+#define PD_T_PD_DEBOUNCE	20	/* 10 - 20 ms */
+#define PD_T_TRY_CC_DEBOUNCE	15	/* 10 - 20 ms */
+
+#define PD_N_CAPS_COUNT		(PD_T_NO_RESPONSE / PD_T_SEND_SOURCE_CAP)
+#define PD_N_HARD_RESET_COUNT	2
+
+#define PD_P_SNK_STDBY_MW	2500	/* 2500 mW */
+
+#endif /* _UAPI__LINUX_USB_PD_H */
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices
  2021-10-26 14:33 [RFC PATCH 0/4] USB Power Delivery character device interface Heikki Krogerus
  2021-10-26 14:33 ` [RFC PATCH 1/4] usb: pd: uapi header split Heikki Krogerus
@ 2021-10-26 14:33 ` Heikki Krogerus
  2021-10-26 15:08   ` Greg KH
                     ` (3 more replies)
  2021-10-26 14:33 ` [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev Heikki Krogerus
                   ` (2 subsequent siblings)
  4 siblings, 4 replies; 19+ messages in thread
From: Heikki Krogerus @ 2021-10-26 14:33 UTC (permalink / raw)
  To: Prashant Malani, Benson Leung
  Cc: Adam Thomson, Guenter Roeck, Badhri Jagan Sridharan, Jack Pham,
	Gopal, Saranya, Regupathy, Rajaram, linux-usb, linux-kernel

Interim.

TODO/ideas:
- Figure out a proper magic value for the ioctl and check if
  the ioctl range is OK.
- Register separate PD device for the cdev, and register it
  only if the device (port, plug or partner) actually
  supports USB PD (or come up with some other solution?).
- Introduce something like

	struct pd_request {
		struct pd_message request;
		struct pd_message __user *response;
	};

  and use it instead of only single struct pd_messages everywhere.

- Add compat support.
- What do we do with Alerts and Attentions?

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 .../userspace-api/ioctl/ioctl-number.rst      |   1 +
 drivers/usb/typec/Makefile                    |   2 +-
 drivers/usb/typec/class.c                     |  42 ++++
 drivers/usb/typec/class.h                     |   4 +
 drivers/usb/typec/pd-dev.c                    | 210 ++++++++++++++++++
 drivers/usb/typec/pd-dev.h                    |  15 ++
 include/linux/usb/pd_dev.h                    |  22 ++
 include/linux/usb/typec.h                     |   8 +
 include/uapi/linux/usb/pd_dev.h               |  55 +++++
 9 files changed, 358 insertions(+), 1 deletion(-)
 create mode 100644 drivers/usb/typec/pd-dev.c
 create mode 100644 drivers/usb/typec/pd-dev.h
 create mode 100644 include/linux/usb/pd_dev.h
 create mode 100644 include/uapi/linux/usb/pd_dev.h

diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 0ba463be6c588..fd443fd21f62a 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -175,6 +175,7 @@ Code  Seq#    Include File                                           Comments
 'P'   60-6F  sound/sscape_ioctl.h                                    conflict!
 'P'   00-0F  drivers/usb/class/usblp.c                               conflict!
 'P'   01-09  drivers/misc/pci_endpoint_test.c                        conflict!
+'P'   70-7F  uapi/linux/usb/pd_dev.h                                 <mailto:linux-usb@vger.kernel.org>
 'Q'   all    linux/soundcard.h
 'R'   00-1F  linux/random.h                                          conflict!
 'R'   01     linux/rfkill.h                                          conflict!
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index a0adb8947a301..be44528168013 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_TYPEC)		+= typec.o
-typec-y				:= class.o mux.o bus.o port-mapper.o
+typec-y				:= class.o mux.o bus.o port-mapper.o pd-dev.o
 obj-$(CONFIG_TYPEC)		+= altmodes/
 obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
 obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index aeef453aa6585..19fcc5da175d7 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -15,6 +15,7 @@
 
 #include "bus.h"
 #include "class.h"
+#include "pd-dev.h"
 
 static DEFINE_IDA(typec_index_ida);
 
@@ -665,6 +666,11 @@ static const struct attribute_group *typec_partner_groups[] = {
 	NULL
 };
 
+char *typec_partner_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
+{
+	return kasprintf(GFP_KERNEL, "pd%u/partner", to_typec_port(dev->parent)->id);
+}
+
 static void typec_partner_release(struct device *dev)
 {
 	struct typec_partner *partner = to_typec_partner(dev);
@@ -676,6 +682,7 @@ static void typec_partner_release(struct device *dev)
 const struct device_type typec_partner_dev_type = {
 	.name = "typec_partner",
 	.groups = typec_partner_groups,
+	.devnode = typec_partner_devnode,
 	.release = typec_partner_release,
 };
 
@@ -807,6 +814,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
 
 	ida_init(&partner->mode_ids);
 	partner->usb_pd = desc->usb_pd;
+	partner->pd_dev = desc->pd_dev;
 	partner->accessory = desc->accessory;
 	partner->num_altmodes = -1;
 	partner->pd_revision = desc->pd_revision;
@@ -826,6 +834,9 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
 	partner->dev.type = &typec_partner_dev_type;
 	dev_set_name(&partner->dev, "%s-partner", dev_name(&port->dev));
 
+	if (partner->pd_dev)
+		partner->dev.devt = MKDEV(PD_DEV_MAJOR, port->id * 4 + 3);
+
 	ret = device_register(&partner->dev);
 	if (ret) {
 		dev_err(&port->dev, "failed to register partner (%d)\n", ret);
@@ -853,6 +864,13 @@ EXPORT_SYMBOL_GPL(typec_unregister_partner);
 /* ------------------------------------------------------------------------- */
 /* Type-C Cable Plugs */
 
+char *typec_plug_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
+{
+	return kasprintf(GFP_KERNEL, "pd%u/plug%d",
+			 to_typec_port(dev->parent->parent)->id,
+			 to_typec_plug(dev)->index);
+}
+
 static void typec_plug_release(struct device *dev)
 {
 	struct typec_plug *plug = to_typec_plug(dev);
@@ -891,6 +909,7 @@ static const struct attribute_group *typec_plug_groups[] = {
 const struct device_type typec_plug_dev_type = {
 	.name = "typec_plug",
 	.groups = typec_plug_groups,
+	.devnode = typec_plug_devnode,
 	.release = typec_plug_release,
 };
 
@@ -973,11 +992,16 @@ struct typec_plug *typec_register_plug(struct typec_cable *cable,
 	ida_init(&plug->mode_ids);
 	plug->num_altmodes = -1;
 	plug->index = desc->index;
+	plug->pd_dev = desc->pd_dev;
 	plug->dev.class = &typec_class;
 	plug->dev.parent = &cable->dev;
 	plug->dev.type = &typec_plug_dev_type;
 	dev_set_name(&plug->dev, "%s-%s", dev_name(cable->dev.parent), name);
 
+	if (plug->pd_dev)
+		plug->dev.devt = MKDEV(PD_DEV_MAJOR,
+				       to_typec_port(cable->dev.parent)->id * 4 + 1 + plug->index);
+
 	ret = device_register(&plug->dev);
 	if (ret) {
 		dev_err(&cable->dev, "failed to register plug (%d)\n", ret);
@@ -1595,6 +1619,11 @@ static int typec_uevent(struct device *dev, struct kobj_uevent_env *env)
 	return ret;
 }
 
+char *typec_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
+{
+	return kasprintf(GFP_KERNEL, "pd%u/port", to_typec_port(dev)->id);
+}
+
 static void typec_release(struct device *dev)
 {
 	struct typec_port *port = to_typec_port(dev);
@@ -1611,6 +1640,7 @@ const struct device_type typec_port_dev_type = {
 	.name = "typec_port",
 	.groups = typec_groups,
 	.uevent = typec_uevent,
+	.devnode = typec_devnode,
 	.release = typec_release,
 };
 
@@ -2044,6 +2074,7 @@ struct typec_port *typec_register_port(struct device *parent,
 
 	port->id = id;
 	port->ops = cap->ops;
+	port->pd_dev = cap->pd_dev;
 	port->port_type = cap->type;
 	port->prefer_role = cap->prefer_role;
 
@@ -2055,6 +2086,9 @@ struct typec_port *typec_register_port(struct device *parent,
 	dev_set_name(&port->dev, "port%d", id);
 	dev_set_drvdata(&port->dev, cap->driver_data);
 
+	if (port->pd_dev)
+		port->dev.devt = MKDEV(PD_DEV_MAJOR, id * 4);
+
 	port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL);
 	if (!port->cap) {
 		put_device(&port->dev);
@@ -2121,8 +2155,15 @@ static int __init typec_init(void)
 	if (ret)
 		goto err_unregister_mux_class;
 
+	ret = usbpd_dev_init();
+	if (ret)
+		goto err_unregister_class;
+
 	return 0;
 
+err_unregister_class:
+	class_unregister(&typec_class);
+
 err_unregister_mux_class:
 	class_unregister(&typec_mux_class);
 
@@ -2135,6 +2176,7 @@ subsys_initcall(typec_init);
 
 static void __exit typec_exit(void)
 {
+	usbpd_dev_exit();
 	class_unregister(&typec_class);
 	ida_destroy(&typec_index_ida);
 	bus_unregister(&typec_bus);
diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
index aef03eb7e1523..87c072f2ad753 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -14,6 +14,7 @@ struct typec_plug {
 	enum typec_plug_index		index;
 	struct ida			mode_ids;
 	int				num_altmodes;
+	const struct pd_dev		*pd_dev;
 };
 
 struct typec_cable {
@@ -33,6 +34,7 @@ struct typec_partner {
 	int				num_altmodes;
 	u16				pd_revision; /* 0300H = "3.0" */
 	enum usb_pd_svdm_ver		svdm_version;
+	const struct pd_dev		*pd_dev;
 };
 
 struct typec_port {
@@ -59,6 +61,8 @@ struct typec_port {
 	struct mutex			port_list_lock; /* Port list lock */
 
 	void				*pld;
+
+	const struct pd_dev		*pd_dev;
 };
 
 #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
diff --git a/drivers/usb/typec/pd-dev.c b/drivers/usb/typec/pd-dev.c
new file mode 100644
index 0000000000000..436853e046ce4
--- /dev/null
+++ b/drivers/usb/typec/pd-dev.c
@@ -0,0 +1,210 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Power Delivery /dev entries
+ *
+ * Copyright (C) 2021 Intel Corporation
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ */
+
+#include <linux/cdev.h>
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/usb/pd_dev.h>
+
+#include "class.h"
+
+#define PD_DEV_MAX (MINORMASK + 1)
+
+dev_t usbpd_devt;
+static struct cdev usb_pd_cdev;
+
+struct pddev {
+	struct device *dev;
+	struct typec_port *port;
+	const struct pd_dev *pd_dev;
+};
+
+static ssize_t usbpd_read(struct file *file, char __user *buf, size_t count, loff_t *offset)
+{
+	/* FIXME TODO XXX */
+
+	/* Alert and Attention handling here (with poll) ? */
+
+	return 0;
+}
+
+static long usbpd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+	struct pddev *pd = file->private_data;
+	void __user *p = (void __user *)arg;
+	unsigned int pwr_role;
+	struct pd_message msg;
+	u32 configuration;
+	int ret = 0;
+
+	switch (cmd) {
+	case USBPDDEV_INFO:
+		if (copy_to_user(p, pd->pd_dev->info, sizeof(*pd->pd_dev->info)))
+			return -EFAULT;
+		break;
+	case USBPDDEV_CONFIGURE:
+		if (!pd->pd_dev->ops->configure)
+			return -ENOTTY;
+
+		if (copy_from_user(&configuration, p, sizeof(configuration)))
+			return -EFAULT;
+
+		ret = pd->pd_dev->ops->configure(pd->pd_dev, configuration);
+		if (ret)
+			return ret;
+		break;
+	case USBPDDEV_PWR_ROLE:
+		if (is_typec_plug(pd->dev))
+			return -ENOTTY;
+
+		if (is_typec_partner(pd->dev)) {
+			if (pd->port->pwr_role == TYPEC_SINK)
+				pwr_role = TYPEC_SOURCE;
+			else
+				pwr_role = TYPEC_SINK;
+		} else {
+			pwr_role = pd->port->pwr_role;
+		}
+
+		if (copy_to_user(p, &pwr_role, sizeof(unsigned int)))
+			return -EFAULT;
+		break;
+	case USBPDDEV_GET_MESSAGE:
+		if (!pd->pd_dev->ops->get_message)
+			return -ENOTTY;
+
+		if (copy_from_user(&msg, p, sizeof(msg)))
+			return -EFAULT;
+
+		ret = pd->pd_dev->ops->get_message(pd->pd_dev, &msg);
+		if (ret)
+			return ret;
+
+		if (copy_to_user(p, &msg, sizeof(msg)))
+			return -EFAULT;
+		break;
+	case USBPDDEV_SET_MESSAGE:
+		if (!pd->pd_dev->ops->set_message)
+			return -ENOTTY;
+
+		ret = pd->pd_dev->ops->set_message(pd->pd_dev, &msg);
+		if (ret)
+			return ret;
+
+		if (copy_to_user(p, &msg, sizeof(msg)))
+			return -EFAULT;
+		break;
+	case USBPDDEV_SUBMIT_MESSAGE:
+		if (!pd->pd_dev->ops->submit)
+			return -ENOTTY;
+
+		if (copy_from_user(&msg, p, sizeof(msg)))
+			return -EFAULT;
+
+		ret = pd->pd_dev->ops->submit(pd->pd_dev, &msg);
+		if (ret)
+			return ret;
+
+		if (copy_to_user(p, &msg, sizeof(msg)))
+			return -EFAULT;
+		break;
+	default:
+		return -ENOTTY;
+	}
+
+	return 0;
+}
+
+static int usbpd_open(struct inode *inode, struct file *file)
+{
+	struct device *dev;
+	struct pddev *pd;
+
+	dev = class_find_device_by_devt(&typec_class, inode->i_rdev);
+	if (!dev)
+		return -ENODEV;
+
+	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+	if (!pd) {
+		put_device(dev);
+		return -ENOMEM;
+	}
+
+	if (is_typec_partner(dev)) {
+		if (!to_typec_partner(dev)->usb_pd) {
+			put_device(dev);
+			kfree(pd);
+			return -ENODEV;
+		}
+		pd->port = to_typec_port(dev->parent);
+		pd->pd_dev = to_typec_partner(dev)->pd_dev;
+	} else if (is_typec_plug(dev)) {
+		pd->port = to_typec_port(dev->parent->parent);
+		pd->pd_dev = to_typec_plug(dev)->pd_dev;
+	} else {
+		pd->port = to_typec_port(dev);
+		pd->pd_dev = to_typec_port(dev)->pd_dev;
+	}
+
+	pd->dev = dev;
+	file->private_data = pd;
+
+	return 0;
+}
+
+static int usbpd_release(struct inode *inode, struct file *file)
+{
+	struct pddev *pd = file->private_data;
+
+	put_device(pd->dev);
+	kfree(pd);
+
+	return 0;
+}
+
+const struct file_operations usbpd_file_operations = {
+	.owner		= THIS_MODULE,
+	.llseek		= no_llseek,
+	.read		= usbpd_read,
+	.unlocked_ioctl = usbpd_ioctl,
+	.compat_ioctl	= compat_ptr_ioctl,
+	.open		= usbpd_open,
+	.release	= usbpd_release,
+};
+
+int __init usbpd_dev_init(void)
+{
+	int ret;
+
+	ret = alloc_chrdev_region(&usbpd_devt, 0, PD_DEV_MAX, "usb_pd");
+	if (ret)
+		return ret;
+
+	/*
+	 * FIXME!
+	 *
+	 * Now the cdev is always created, even when the device does not support
+	 * USB PD.
+	 */
+
+	cdev_init(&usb_pd_cdev, &usbpd_file_operations);
+
+	ret = cdev_add(&usb_pd_cdev, usbpd_devt, PD_DEV_MAX);
+	if (ret) {
+		unregister_chrdev_region(usbpd_devt, PD_DEV_MAX);
+		return ret;
+	}
+
+	return 0;
+}
+
+void __exit usbpd_dev_exit(void)
+{
+	cdev_del(&usb_pd_cdev);
+	unregister_chrdev_region(usbpd_devt, PD_DEV_MAX);
+}
diff --git a/drivers/usb/typec/pd-dev.h b/drivers/usb/typec/pd-dev.h
new file mode 100644
index 0000000000000..2d817167c4042
--- /dev/null
+++ b/drivers/usb/typec/pd-dev.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __USB_TYPEC_PDDEV__
+#define __USB_TYPEC_PDDEV__
+
+#include <linux/kdev_t.h>
+
+#define PD_DEV_MAJOR	MAJOR(usbpd_devt)
+
+extern dev_t usbpd_devt;
+
+int usbpd_dev_init(void);
+void usbpd_dev_exit(void);
+
+#endif /* __USB_TYPEC_PDDEV__ */
diff --git a/include/linux/usb/pd_dev.h b/include/linux/usb/pd_dev.h
new file mode 100644
index 0000000000000..d451928f94e78
--- /dev/null
+++ b/include/linux/usb/pd_dev.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __LINUX_USB_PDDEV_H
+#define __LINUX_USB_PDDEV_H
+
+#include <uapi/linux/usb/pd_dev.h>
+
+struct pd_dev;
+
+struct pd_ops {
+	int (*configure)(const struct pd_dev *dev, u32 flags);
+	int (*get_message)(const struct pd_dev *dev, struct pd_message *msg);
+	int (*set_message)(const struct pd_dev *dev, struct pd_message *msg);
+	int (*submit)(const struct pd_dev *dev, struct pd_message *msg);
+};
+
+struct pd_dev {
+	const struct pd_info *info;
+	const struct pd_ops *ops;
+};
+
+#endif /* __LINUX_USB_PDDEV_H */
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index e2e44bb1dad85..6df7b096f769c 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -20,6 +20,7 @@ struct typec_port;
 struct typec_altmode_ops;
 
 struct fwnode_handle;
+struct pd_dev;
 struct device;
 
 enum typec_port_type {
@@ -159,11 +160,13 @@ enum typec_plug_index {
  * struct typec_plug_desc - USB Type-C Cable Plug Descriptor
  * @index: SOP Prime for the plug connected to DFP and SOP Double Prime for the
  *         plug connected to UFP
+ * @pd_dev: USB Power Delivery Character Device
  *
  * Represents USB Type-C Cable Plug.
  */
 struct typec_plug_desc {
 	enum typec_plug_index	index;
+	const struct pd_dev	*pd_dev;
 };
 
 /*
@@ -189,6 +192,7 @@ struct typec_cable_desc {
  * @accessory: Audio, Debug or none.
  * @identity: Discover Identity command data
  * @pd_revision: USB Power Delivery Specification Revision if supported
+ * @pd_dev: USB Power Delivery Character Device
  *
  * Details about a partner that is attached to USB Type-C port. If @identity
  * member exists when partner is registered, a directory named "identity" is
@@ -204,6 +208,7 @@ struct typec_partner_desc {
 	enum typec_accessory	accessory;
 	struct usb_pd_identity	*identity;
 	u16			pd_revision; /* 0300H = "3.0" */
+	const struct pd_dev	*pd_dev;
 };
 
 /**
@@ -241,6 +246,7 @@ enum usb_pd_svdm_ver {
  * @fwnode: Optional fwnode of the port
  * @driver_data: Private pointer for driver specific info
  * @ops: Port operations vector
+ * @pd_dev: USB Power Delivery Character Device
  *
  * Static capabilities of a single USB Type-C port.
  */
@@ -258,6 +264,8 @@ struct typec_capability {
 	void			*driver_data;
 
 	const struct typec_operations	*ops;
+
+	const struct pd_dev	*pd_dev;
 };
 
 /* Specific to try_role(). Indicates the user want's to clear the preference. */
diff --git a/include/uapi/linux/usb/pd_dev.h b/include/uapi/linux/usb/pd_dev.h
new file mode 100644
index 0000000000000..44027bc6b6339
--- /dev/null
+++ b/include/uapi/linux/usb/pd_dev.h
@@ -0,0 +1,55 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _UAPI__LINUX_USB_PDDEV_H
+#define _UAPI__LINUX_USB_PDDEV_H
+
+#include <linux/types.h>
+#include <linux/usb/pd.h>
+
+/*
+ * struct pd_info - USB Power Delivery Device Information
+ * @specification_revision: USB Power Delivery Specification Revision
+ * @supported_ctrl_msgs: Supported Control Messages
+ * @supported_data_msgs: Supported Data Messages
+ * @supported_ext_msgs: Supported Extended Messages
+ *
+ * @specification_revision is in the same format as the Specification Revision
+ * Field in the Message Header. @supported_ctrl_msgs, @supported_data_msgs and
+ * @supported_ext_msgs list the messages, a bit for each, that can be used with
+ * USBPDDEV_SUBMIT_MESSAGE ioctl.
+ */
+struct pd_info {
+	__u8 specification_revision; /* XXX I don't know if this is useful? */
+	__u32 ctrl_msgs_supported;
+	__u32 data_msgs_supported;
+	__u32 ext_msgs_supported;
+} __attribute__ ((packed));
+
+/* Example configuration flags for ports. */
+#define USBPDDEV_CFPORT_ENTER_MODES	BIT(0) /* Automatic alt mode entry. */
+
+/*
+ * For basic communication use USBPDDEV_SUBMIT_MESSAGE ioctl. GoodCRC is not
+ * supported. Response will also never be GoodCRC.
+ *
+ * To check cached objects (if they are cached) use USBPDDEV_GET_MESSAGE ioctl.
+ * Useful most likely with RDO and EUDO, but also with Identity etc.
+ * USBPDDEV_SET_MESSAGE is primarily meant to be used with ports. If supported,
+ * it can be used to assign the values for objects like EUDO that the port should
+ * use in future communication.
+ *
+ * The idea with USBPDDEV_CONFIGURE is that you could modify the behaviour of
+ * the underlying TCPM (or what ever interface you have) with some things. So
+ * for example, you could disable automatic alternate mode entry with it with
+ * that USBPDDEV_CFPORT_ENTER_MODES - It's just an example! - so basically, you
+ * could take over some things from TCPM with it.
+ */
+
+#define USBPDDEV_INFO		_IOR('P', 0x70, struct pd_info)
+#define USBPDDEV_CONFIGURE	_IOW('P', 0x71, __u32)
+#define USBPDDEV_PWR_ROLE	_IOR('P', 0x72, int) /* The *current* role! */
+#define USBPDDEV_GET_MESSAGE	_IOWR('P', 0x73, struct pd_message)
+#define USBPDDEV_SET_MESSAGE	_IOW('P', 0x74, struct pd_message)
+#define USBPDDEV_SUBMIT_MESSAGE	_IOWR('P', 0x75, struct pd_message)
+
+#endif /* _UAPI__LINUX_USB_PDDEV_H */
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev
  2021-10-26 14:33 [RFC PATCH 0/4] USB Power Delivery character device interface Heikki Krogerus
  2021-10-26 14:33 ` [RFC PATCH 1/4] usb: pd: uapi header split Heikki Krogerus
  2021-10-26 14:33 ` [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices Heikki Krogerus
@ 2021-10-26 14:33 ` Heikki Krogerus
  2021-10-27  1:00   ` Jack Pham
  2021-10-27  8:06   ` kernel test robot
  2021-10-26 14:33 ` [RFC PATCH 4/4] tools: usb: Hideous test tool for USB PD char device Heikki Krogerus
  2021-10-26 15:06 ` [RFC PATCH 0/4] USB Power Delivery character device interface Greg KH
  4 siblings, 2 replies; 19+ messages in thread
From: Heikki Krogerus @ 2021-10-26 14:33 UTC (permalink / raw)
  To: Prashant Malani, Benson Leung
  Cc: Adam Thomson, Guenter Roeck, Badhri Jagan Sridharan, Jack Pham,
	Gopal, Saranya, Regupathy, Rajaram, linux-usb, linux-kernel

Interim.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/ucsi/Makefile |   2 +-
 drivers/usb/typec/ucsi/pd-dev.c | 125 ++++++++++++++++++++++++++++++++
 drivers/usb/typec/ucsi/ucsi.c   |  37 +++++++---
 drivers/usb/typec/ucsi/ucsi.h   |   7 ++
 4 files changed, 161 insertions(+), 10 deletions(-)
 create mode 100644 drivers/usb/typec/ucsi/pd-dev.c

diff --git a/drivers/usb/typec/ucsi/Makefile b/drivers/usb/typec/ucsi/Makefile
index 8a8eb5cb8e0f0..097af32468f96 100644
--- a/drivers/usb/typec/ucsi/Makefile
+++ b/drivers/usb/typec/ucsi/Makefile
@@ -3,7 +3,7 @@ CFLAGS_trace.o				:= -I$(src)
 
 obj-$(CONFIG_TYPEC_UCSI)		+= typec_ucsi.o
 
-typec_ucsi-y				:= ucsi.o
+typec_ucsi-y				:= ucsi.o pd-dev.o
 
 typec_ucsi-$(CONFIG_TRACING)		+= trace.o
 
diff --git a/drivers/usb/typec/ucsi/pd-dev.c b/drivers/usb/typec/ucsi/pd-dev.c
new file mode 100644
index 0000000000000..3c03d3d859198
--- /dev/null
+++ b/drivers/usb/typec/ucsi/pd-dev.c
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * UCSI USB Power Delivery Device
+ *
+ * Copyright (C) 2021, Intel Corporation
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ */
+
+#include "ucsi.h"
+
+static struct ucsi_connector *pd_dev_to_connector(const struct pd_dev *dev);
+
+static int ucsi_pd_get_objects(const struct pd_dev *dev, struct pd_message *msg)
+{
+	struct ucsi_connector *con = pd_dev_to_connector(dev);
+	int partner = dev == &con->pd_partner_dev;
+	int ret = -ENOTTY;
+
+	mutex_lock(&con->lock);
+
+	if (le16_to_cpu(msg->header) & PD_HEADER_EXT_HDR)
+		goto err;
+
+	switch (pd_header_type_le(msg->header)) {
+	case PD_DATA_SOURCE_CAP:
+		ret = ucsi_read_pdos(con, partner, 1, msg->payload);
+		if (ret < 0)
+			goto err;
+
+		msg->header = PD_HEADER_LE(PD_DATA_SOURCE_CAP, 0, 0, 0, 0, ret);
+		break;
+	case PD_DATA_REQUEST:
+		msg->header = PD_HEADER_LE(PD_DATA_REQUEST, 0, 0, 0, 0, 1);
+		msg->payload[0] = con->status.request_data_obj;
+		break;
+	case PD_DATA_SINK_CAP:
+		ret = ucsi_read_pdos(con, partner, 0, msg->payload);
+		if (ret < 0)
+			goto err;
+
+		msg->header = PD_HEADER_LE(PD_DATA_SINK_CAP, 0, 0, 0, 0, ret);
+		break;
+	default:
+		goto err;
+	}
+
+	ret = 0;
+err:
+	mutex_unlock(&con->lock);
+
+	return ret;
+}
+
+/*
+ * This function is here just as an example for now.
+ */
+static int ucsi_pd_submit(const struct pd_dev *dev, struct pd_message *msg)
+{
+	struct ucsi_connector *con = pd_dev_to_connector(dev);
+	int ret;
+
+	mutex_lock(&con->lock);
+
+	switch (pd_header_type_le(msg->header)) {
+	case PD_CTRL_GET_SOURCE_CAP:
+		ret = ucsi_read_pdos(con, 1, 1, msg->payload);
+		if (ret < 0)
+			goto err;
+
+		msg->header = PD_HEADER_LE(PD_DATA_SOURCE_CAP, 0, 0, 0, 0, ret);
+		break;
+	case PD_CTRL_GET_SINK_CAP:
+		ret = ucsi_read_pdos(con, 1, 0, msg->payload);
+		if (ret < 0)
+			goto err;
+
+		msg->header = PD_HEADER_LE(PD_DATA_SINK_CAP, 0, 0, 0, 0, ret);
+		break;
+	default:
+		ret = -ENOTTY;
+		goto err;
+	}
+
+	ret = 0;
+err:
+	mutex_unlock(&con->lock);
+
+	return ret;
+}
+
+static const struct pd_ops ucsi_pd_partner_ops = {
+	.get_message = ucsi_pd_get_objects,
+	.submit = ucsi_pd_submit,
+};
+
+static struct pd_info ucsi_pd_partner_info = {
+	.specification_revision = 2,
+	.ctrl_msgs_supported = BIT(PD_CTRL_GET_SOURCE_CAP) |
+			       BIT(PD_CTRL_GET_SINK_CAP),
+};
+
+static const struct pd_ops ucsi_pd_port_ops = {
+	.get_message = ucsi_pd_get_objects,
+};
+
+static struct pd_info ucsi_pd_port_info = {
+	.specification_revision = 2,
+};
+
+static struct ucsi_connector *pd_dev_to_connector(const struct pd_dev *dev)
+{
+	if (dev->info == &ucsi_pd_port_info)
+		return container_of(dev, struct ucsi_connector, pd_port_dev);
+	if (dev->info == &ucsi_pd_partner_info)
+		return container_of(dev, struct ucsi_connector, pd_partner_dev);
+	return NULL;
+}
+
+void ucsi_init_pd_dev(struct ucsi_connector *con)
+{
+	con->pd_port_dev.info = &ucsi_pd_port_info;
+	con->pd_port_dev.ops = &ucsi_pd_port_ops;
+	con->pd_partner_dev.info = &ucsi_pd_partner_info;
+	con->pd_partner_dev.ops = &ucsi_pd_partner_ops;
+}
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 6aa28384f77f1..e9e6d7608bdaf 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -557,7 +557,7 @@ static void ucsi_unregister_altmodes(struct ucsi_connector *con, u8 recipient)
 	}
 }
 
-static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner,
+static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner, int source,
 			 u32 *pdos, int offset, int num_pdos)
 {
 	struct ucsi *ucsi = con->ucsi;
@@ -568,7 +568,7 @@ static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner,
 	command |= UCSI_GET_PDOS_PARTNER_PDO(is_partner);
 	command |= UCSI_GET_PDOS_PDO_OFFSET(offset);
 	command |= UCSI_GET_PDOS_NUM_PDOS(num_pdos - 1);
-	command |= UCSI_GET_PDOS_SRC_PDOS;
+	command |= source ? UCSI_GET_PDOS_SRC_PDOS : 0;
 	ret = ucsi_send_command(ucsi, command, pdos + offset,
 				num_pdos * sizeof(u32));
 	if (ret < 0 && ret != -ETIMEDOUT)
@@ -579,27 +579,43 @@ static int ucsi_get_pdos(struct ucsi_connector *con, int is_partner,
 	return ret;
 }
 
-static int ucsi_get_src_pdos(struct ucsi_connector *con)
+int ucsi_read_pdos(struct ucsi_connector *con, int partner, int source, u32 *pdos)
 {
+	u32 pdo[PDO_MAX_OBJECTS];
+	int num_pdos;
 	int ret;
 
 	/* UCSI max payload means only getting at most 4 PDOs at a time */
-	ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
+	ret = ucsi_get_pdos(con, partner, source, pdo, 0, UCSI_MAX_PDOS);
 	if (ret < 0)
 		return ret;
 
-	con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
-	if (con->num_pdos < UCSI_MAX_PDOS)
-		return 0;
+	num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
+	if (num_pdos < UCSI_MAX_PDOS)
+		goto done;
 
 	/* get the remaining PDOs, if any */
-	ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
+	ret = ucsi_get_pdos(con, partner, source, pdo, UCSI_MAX_PDOS,
 			    PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
 	if (ret < 0)
 		return ret;
 
-	con->num_pdos += ret / sizeof(u32);
+	num_pdos += ret / sizeof(u32);
+done:
+	memcpy(pdos, pdo, num_pdos * sizeof(pdo));
+
+	return num_pdos;
+}
+
+static int ucsi_get_src_pdos(struct ucsi_connector *con)
+{
+	int ret;
+
+	ret = ucsi_read_pdos(con, 0, 1, con->src_pdos);
+	if (ret < 0)
+		return ret;
 
+	con->num_pdos = ret;
 	ucsi_port_psy_changed(con);
 
 	return 0;
@@ -671,6 +687,7 @@ static int ucsi_register_partner(struct ucsi_connector *con)
 	}
 
 	desc.usb_pd = pwr_opmode == UCSI_CONSTAT_PWR_OPMODE_PD;
+	desc.pd_dev = &con->pd_partner_dev;
 
 	partner = typec_register_partner(con->port, &desc);
 	if (IS_ERR(partner)) {
@@ -1038,6 +1055,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 	mutex_init(&con->lock);
 	con->num = index + 1;
 	con->ucsi = ucsi;
+	ucsi_init_pd_dev(con);
 
 	/* Delay other interactions with the con until registration is complete */
 	mutex_lock(&con->lock);
@@ -1077,6 +1095,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 	cap->fwnode = ucsi_find_fwnode(con);
 	cap->driver_data = con;
 	cap->ops = &ucsi_ops;
+	cap->pd_dev = &con->pd_port_dev;
 
 	ret = ucsi_register_port_psy(con);
 	if (ret)
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index 280f1e1bda2c9..1da97ddedf363 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -9,6 +9,7 @@
 #include <linux/types.h>
 #include <linux/usb/typec.h>
 #include <linux/usb/pd.h>
+#include <linux/usb/pd_dev.h>
 #include <linux/usb/role.h>
 
 /* -------------------------------------------------------------------------- */
@@ -335,6 +336,9 @@ struct ucsi_connector {
 	int num_pdos;
 
 	struct usb_role_switch *usb_role_sw;
+
+	struct pd_dev pd_port_dev;
+	struct pd_dev pd_partner_dev;
 };
 
 int ucsi_send_command(struct ucsi *ucsi, u64 command,
@@ -343,6 +347,9 @@ int ucsi_send_command(struct ucsi *ucsi, u64 command,
 void ucsi_altmode_update_active(struct ucsi_connector *con);
 int ucsi_resume(struct ucsi *ucsi);
 
+void ucsi_init_pd_dev(struct ucsi_connector *con);
+int ucsi_read_pdos(struct ucsi_connector *con, int partner, int source, u32 *pdos);
+
 #if IS_ENABLED(CONFIG_POWER_SUPPLY)
 int ucsi_register_port_psy(struct ucsi_connector *con);
 void ucsi_unregister_port_psy(struct ucsi_connector *con);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [RFC PATCH 4/4] tools: usb: Hideous test tool for USB PD char device
  2021-10-26 14:33 [RFC PATCH 0/4] USB Power Delivery character device interface Heikki Krogerus
                   ` (2 preceding siblings ...)
  2021-10-26 14:33 ` [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev Heikki Krogerus
@ 2021-10-26 14:33 ` Heikki Krogerus
  2021-10-26 15:06 ` [RFC PATCH 0/4] USB Power Delivery character device interface Greg KH
  4 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2021-10-26 14:33 UTC (permalink / raw)
  To: Prashant Malani, Benson Leung
  Cc: Adam Thomson, Guenter Roeck, Badhri Jagan Sridharan, Jack Pham,
	Gopal, Saranya, Regupathy, Rajaram, linux-usb, linux-kernel

Interim.

The Makefile needs to be tuned so we can include to correct
files.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 tools/usb/Build     |   1 +
 tools/usb/Makefile  |   8 ++-
 tools/usb/pd-test.c | 123 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 131 insertions(+), 1 deletion(-)
 create mode 100644 tools/usb/pd-test.c

diff --git a/tools/usb/Build b/tools/usb/Build
index 2ad6f97458168..7116198533a75 100644
--- a/tools/usb/Build
+++ b/tools/usb/Build
@@ -1,2 +1,3 @@
 testusb-y += testusb.o
 ffs-test-y += ffs-test.o
+pd-test-y += pd-test.o
diff --git a/tools/usb/Makefile b/tools/usb/Makefile
index 1b128e551b2e4..e3e41a3397f23 100644
--- a/tools/usb/Makefile
+++ b/tools/usb/Makefile
@@ -16,7 +16,7 @@ MAKEFLAGS += -r
 override CFLAGS += -O2 -Wall -Wextra -g -D_GNU_SOURCE -I$(OUTPUT)include -I$(srctree)/tools/include
 override LDFLAGS += -lpthread
 
-ALL_TARGETS := testusb ffs-test
+ALL_TARGETS := testusb ffs-test pd-test
 ALL_PROGRAMS := $(patsubst %,$(OUTPUT)%,$(ALL_TARGETS))
 
 all: $(ALL_PROGRAMS)
@@ -36,6 +36,12 @@ $(FFS_TEST_IN): FORCE
 $(OUTPUT)ffs-test: $(FFS_TEST_IN)
 	$(QUIET_LINK)$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
 
+PD_TEST_IN := $(OUTPUT)pd-test-in.o
+$(PD_TEST_IN): FORCE
+	$(Q)$(MAKE) $(build)=pd-test
+$(OUTPUT)pd-test: $(PD_TEST_IN)
+	$(QUIET_LINK)$(CC) $(CFLAGS) $< -o $@
+
 clean:
 	rm -f $(ALL_PROGRAMS)
 	find $(if $(OUTPUT),$(OUTPUT),.) -name '*.o' -delete -o -name '\.*.d' -delete -o -name '\.*.o.cmd' -delete
diff --git a/tools/usb/pd-test.c b/tools/usb/pd-test.c
new file mode 100644
index 0000000000000..bb38dd4134581
--- /dev/null
+++ b/tools/usb/pd-test.c
@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * USB Power Delivery device tester.
+ *
+ * Copyright (C) 2021 Intel Corporation
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ */
+
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <linux/types.h>
+
+struct pd_message {
+	__le16 header;
+	__le32 payload[7];
+} __attribute__((packed));
+
+struct pd_info {
+	__u8 specification_revision;
+	__u32 ctrl_msgs_supported;
+	__u32 data_msgs_supported;
+	__u32 ext_msgs_supported;
+} __attribute__((packed));
+
+#define USBPDDEV_INFO		_IOR('P', 0x70, struct pd_info)
+#define USBPDDEV_CONFIGURE	_IOW('P', 0x71, __u32)
+#define USBPDDEV_PWR_ROLE	_IOR('P', 0x72, int)
+#define USBPDDEV_GET_MESSAGE	_IOWR('P', 0x73, struct pd_message)
+#define USBPDDEV_SET_MESSAGE	_IOW('P', 0x74, struct pd_message)
+#define USBPDDEV_SUBMIT_MESSAGE	_IOWR('P', 0x75, struct pd_message)
+
+enum pd_data_msg_type {
+	/* 0 Reserved */
+	PD_DATA_SOURCE_CAP = 1,
+	PD_DATA_REQUEST = 2,
+	PD_DATA_BIST = 3,
+	PD_DATA_SINK_CAP = 4,
+	PD_DATA_BATT_STATUS = 5,
+	PD_DATA_ALERT = 6,
+	PD_DATA_GET_COUNTRY_INFO = 7,
+	PD_DATA_ENTER_USB = 8,
+	/* 9-14 Reserved */
+	PD_DATA_VENDOR_DEF = 15,
+	/* 16-31 Reserved */
+};
+
+int dump_source_pdos(int fd)
+{
+	struct pd_message msg = {};
+	int ret;
+	int i;
+
+	msg.header = PD_DATA_SOURCE_CAP;
+	ret = ioctl(fd, USBPDDEV_GET_MESSAGE, &msg);
+	if (ret < 0) {
+		printf("No cached Source Capabilities %d\n", ret);
+		return ret;
+	}
+
+	printf("Source Capabilities:\n");
+
+	for (i = 0; i < (msg.header >> 12 & 7); i++)
+		printf("  PDO%d: 0x%08x\n", i + 1, msg.payload[i]);
+
+	return 0;
+}
+
+int dump_sink_pdos(int fd)
+{
+	struct pd_message msg = {};
+	int ret;
+	int i;
+
+	msg.header = PD_DATA_SINK_CAP;
+	ret = ioctl(fd, USBPDDEV_GET_MESSAGE, &msg);
+	if (ret < 0) {
+		printf("No cached Sink Capabilities %d\n", ret);
+		return ret;
+	}
+
+	printf("Sink Capabilities:\n");
+
+	for (i = 0; i < (msg.header >> 12 & 7); i++)
+		printf("  PDO%d: 0x%08x\n", i + 1, msg.payload[i]);
+
+	return 0;
+}
+
+int main(int argc, char **argv)
+{
+	unsigned int role;
+	int ret;
+	int fd;
+
+	if (argc != 2) {
+		fprintf(stderr, "Usage: %s [DEV]\n"
+				"       %% %s /dev/pd0/port\n\n",
+				argv[0], argv[0]);
+		return -1;
+	}
+
+	fd = open(argv[1], O_RDWR);
+	if (fd < 0)
+		return fd;
+
+	ret = ioctl(fd, USBPDDEV_PWR_ROLE, &role);
+	if (ret < 0) {
+		printf("USBPDDEV_PWR_ROLE failed %d\n", ret);
+		goto err;
+	}
+
+	if (role)
+		ret = dump_source_pdos(fd);
+	else
+		ret = dump_sink_pdos(fd);
+err:
+	close(fd);
+
+	return ret;
+}
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 0/4] USB Power Delivery character device interface
  2021-10-26 14:33 [RFC PATCH 0/4] USB Power Delivery character device interface Heikki Krogerus
                   ` (3 preceding siblings ...)
  2021-10-26 14:33 ` [RFC PATCH 4/4] tools: usb: Hideous test tool for USB PD char device Heikki Krogerus
@ 2021-10-26 15:06 ` Greg KH
  2021-10-27 11:02   ` Heikki Krogerus
  4 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2021-10-26 15:06 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Prashant Malani, Benson Leung, Adam Thomson, Guenter Roeck,
	Badhri Jagan Sridharan, Jack Pham, Gopal, Saranya, Regupathy,
	Rajaram, linux-usb, linux-kernel

On Tue, Oct 26, 2021 at 05:33:48PM +0300, Heikki Krogerus wrote:
> Hi,
> 
> This is the proposal for USB PD character devices that we could use to
> communicate USB PD messages directly with the USB PD capable partners,
> ports and cable plugs from user space. Originally I proposed this idea
> here as a better way to get the PDOs from the partners (and ports and
> plugs): https://lkml.org/lkml/2021/10/8/331

You should put the info there (and please use lore.kernel.org in the
future, not lkml.org as we have no control over that site), into this
0/X message as I have no idea _why_ you need a char device and why the
sysfs interface will not work.

So, why not sysfs?  :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices
  2021-10-26 14:33 ` [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices Heikki Krogerus
@ 2021-10-26 15:08   ` Greg KH
  2021-10-26 16:30   ` kernel test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 19+ messages in thread
From: Greg KH @ 2021-10-26 15:08 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Prashant Malani, Benson Leung, Adam Thomson, Guenter Roeck,
	Badhri Jagan Sridharan, Jack Pham, Gopal, Saranya, Regupathy,
	Rajaram, linux-usb, linux-kernel

On Tue, Oct 26, 2021 at 05:33:50PM +0300, Heikki Krogerus wrote:
> Interim.
> 
> TODO/ideas:
> - Figure out a proper magic value for the ioctl and check if
>   the ioctl range is OK.
> - Register separate PD device for the cdev, and register it
>   only if the device (port, plug or partner) actually
>   supports USB PD (or come up with some other solution?).
> - Introduce something like
> 
> 	struct pd_request {
> 		struct pd_message request;
> 		struct pd_message __user *response;
> 	};
> 
>   and use it instead of only single struct pd_messages everywhere.
> 
> - Add compat support.

Ick, no, new ioctls should never need compat support if you create them
properly.  That is only for "old" ones.

Also, why not use the miscdev api instead?  That should remove some code
of yours and make things simpler, if you really want to stick with a
char device node...

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 1/4] usb: pd: uapi header split
  2021-10-26 14:33 ` [RFC PATCH 1/4] usb: pd: uapi header split Heikki Krogerus
@ 2021-10-26 16:05   ` kernel test robot
  0 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-10-26 16:05 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5071 bytes --]

Hi Heikki,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on chrome-platform/for-next peter-chen-usb/for-usb-next linus/master v5.15-rc7 next-20211026]
[cannot apply to balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Heikki-Krogerus/USB-Power-Delivery-character-device-interface/20211026-224629
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/7fd78c91aab51d0602826a1ba76fdf9280e75c0a
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Heikki-Krogerus/USB-Power-Delivery-character-device-interface/20211026-224629
        git checkout 7fd78c91aab51d0602826a1ba76fdf9280e75c0a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
   scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
>> error: include/uapi/linux/usb/pd.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
   make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/usb/pd.h] Error 1
   make[2]: Target '__headers' not remade because of errors.
   make[1]: *** [Makefile:1305: headers] Error 2
   arch/mips/kernel/asm-offsets.c:26:6: error: no previous prototype for 'output_ptreg_defines' [-Werror=missing-prototypes]
      26 | void output_ptreg_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:78:6: error: no previous prototype for 'output_task_defines' [-Werror=missing-prototypes]
      78 | void output_task_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:92:6: error: no previous prototype for 'output_thread_info_defines' [-Werror=missing-prototypes]
      92 | void output_thread_info_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:108:6: error: no previous prototype for 'output_thread_defines' [-Werror=missing-prototypes]
     108 | void output_thread_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:136:6: error: no previous prototype for 'output_thread_fpu_defines' [-Werror=missing-prototypes]
     136 | void output_thread_fpu_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:179:6: error: no previous prototype for 'output_mm_defines' [-Werror=missing-prototypes]
     179 | void output_mm_defines(void)
         |      ^~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:218:6: error: no previous prototype for 'output_sc_defines' [-Werror=missing-prototypes]
     218 | void output_sc_defines(void)
         |      ^~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:253:6: error: no previous prototype for 'output_signal_defined' [-Werror=missing-prototypes]
     253 | void output_signal_defined(void)
         |      ^~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:320:6: error: no previous prototype for 'output_pbe_defines' [-Werror=missing-prototypes]
     320 | void output_pbe_defines(void)
         |      ^~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:332:6: error: no previous prototype for 'output_pm_defines' [-Werror=missing-prototypes]
     332 | void output_pm_defines(void)
         |      ^~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:346:6: error: no previous prototype for 'output_kvm_defines' [-Werror=missing-prototypes]
     346 | void output_kvm_defines(void)
         |      ^~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:390:6: error: no previous prototype for 'output_cps_defines' [-Werror=missing-prototypes]
     390 | void output_cps_defines(void)
         |      ^~~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors
   make[2]: *** [scripts/Makefile.build:121: arch/mips/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1219: prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:219: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 71734 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices
  2021-10-26 14:33 ` [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices Heikki Krogerus
  2021-10-26 15:08   ` Greg KH
@ 2021-10-26 16:30   ` kernel test robot
  2021-10-26 18:45   ` kernel test robot
  2021-10-28  1:03   ` Prashant Malani
  3 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-10-26 16:30 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 5268 bytes --]

Hi Heikki,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on usb/usb-testing]
[also build test ERROR on chrome-platform/for-next peter-chen-usb/for-usb-next linus/master v5.15-rc7 next-20211026]
[cannot apply to balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Heikki-Krogerus/USB-Power-Delivery-character-device-interface/20211026-224629
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: mips-allyesconfig (attached as .config)
compiler: mips-linux-gcc (GCC) 11.2.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/ece16be8ea7588344679c4cbf7f674e5e1e250f8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Heikki-Krogerus/USB-Power-Delivery-character-device-interface/20211026-224629
        git checkout ece16be8ea7588344679c4cbf7f674e5e1e250f8
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross ARCH=mips 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
   scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
>> error: include/uapi/linux/usb/pd_dev.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
   error: include/uapi/linux/usb/pd.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
   make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/usb/pd_dev.h] Error 1
   make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/usb/pd.h] Error 1
   make[2]: Target '__headers' not remade because of errors.
   make[1]: *** [Makefile:1305: headers] Error 2
   arch/mips/kernel/asm-offsets.c:26:6: error: no previous prototype for 'output_ptreg_defines' [-Werror=missing-prototypes]
      26 | void output_ptreg_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:78:6: error: no previous prototype for 'output_task_defines' [-Werror=missing-prototypes]
      78 | void output_task_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:92:6: error: no previous prototype for 'output_thread_info_defines' [-Werror=missing-prototypes]
      92 | void output_thread_info_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:108:6: error: no previous prototype for 'output_thread_defines' [-Werror=missing-prototypes]
     108 | void output_thread_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:136:6: error: no previous prototype for 'output_thread_fpu_defines' [-Werror=missing-prototypes]
     136 | void output_thread_fpu_defines(void)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:179:6: error: no previous prototype for 'output_mm_defines' [-Werror=missing-prototypes]
     179 | void output_mm_defines(void)
         |      ^~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:218:6: error: no previous prototype for 'output_sc_defines' [-Werror=missing-prototypes]
     218 | void output_sc_defines(void)
         |      ^~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:253:6: error: no previous prototype for 'output_signal_defined' [-Werror=missing-prototypes]
     253 | void output_signal_defined(void)
         |      ^~~~~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:320:6: error: no previous prototype for 'output_pbe_defines' [-Werror=missing-prototypes]
     320 | void output_pbe_defines(void)
         |      ^~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:332:6: error: no previous prototype for 'output_pm_defines' [-Werror=missing-prototypes]
     332 | void output_pm_defines(void)
         |      ^~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:346:6: error: no previous prototype for 'output_kvm_defines' [-Werror=missing-prototypes]
     346 | void output_kvm_defines(void)
         |      ^~~~~~~~~~~~~~~~~~
   arch/mips/kernel/asm-offsets.c:390:6: error: no previous prototype for 'output_cps_defines' [-Werror=missing-prototypes]
     390 | void output_cps_defines(void)
         |      ^~~~~~~~~~~~~~~~~~
   cc1: all warnings being treated as errors
   make[2]: *** [scripts/Makefile.build:121: arch/mips/kernel/asm-offsets.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [Makefile:1219: prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:219: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 71734 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices
  2021-10-26 14:33 ` [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices Heikki Krogerus
  2021-10-26 15:08   ` Greg KH
  2021-10-26 16:30   ` kernel test robot
@ 2021-10-26 18:45   ` kernel test robot
  2021-10-28  1:03   ` Prashant Malani
  3 siblings, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-10-26 18:45 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 10617 bytes --]

Hi Heikki,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on chrome-platform/for-next peter-chen-usb/for-usb-next linus/master v5.15-rc7 next-20211026]
[cannot apply to balbi-usb/testing/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Heikki-Krogerus/USB-Power-Delivery-character-device-interface/20211026-224629
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: i386-debian-10.3 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/ece16be8ea7588344679c4cbf7f674e5e1e250f8
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Heikki-Krogerus/USB-Power-Delivery-character-device-interface/20211026-224629
        git checkout ece16be8ea7588344679c4cbf7f674e5e1e250f8
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/usb/typec/class.c:669:7: warning: no previous prototype for 'typec_partner_devnode' [-Wmissing-prototypes]
     669 | char *typec_partner_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
         |       ^~~~~~~~~~~~~~~~~~~~~
>> drivers/usb/typec/class.c:867:7: warning: no previous prototype for 'typec_plug_devnode' [-Wmissing-prototypes]
     867 | char *typec_plug_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
         |       ^~~~~~~~~~~~~~~~~~
>> drivers/usb/typec/class.c:1622:7: warning: no previous prototype for 'typec_devnode' [-Wmissing-prototypes]
    1622 | char *typec_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
         |       ^~~~~~~~~~~~~
--
>> drivers/usb/typec/pd-dev.c:180:12: warning: no previous prototype for 'usbpd_dev_init' [-Wmissing-prototypes]
     180 | int __init usbpd_dev_init(void)
         |            ^~~~~~~~~~~~~~
>> drivers/usb/typec/pd-dev.c:206:13: warning: no previous prototype for 'usbpd_dev_exit' [-Wmissing-prototypes]
     206 | void __exit usbpd_dev_exit(void)
         |             ^~~~~~~~~~~~~~


vim +/typec_partner_devnode +669 drivers/usb/typec/class.c

   668	
 > 669	char *typec_partner_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
   670	{
   671		return kasprintf(GFP_KERNEL, "pd%u/partner", to_typec_port(dev->parent)->id);
   672	}
   673	
   674	static void typec_partner_release(struct device *dev)
   675	{
   676		struct typec_partner *partner = to_typec_partner(dev);
   677	
   678		ida_destroy(&partner->mode_ids);
   679		kfree(partner);
   680	}
   681	
   682	const struct device_type typec_partner_dev_type = {
   683		.name = "typec_partner",
   684		.groups = typec_partner_groups,
   685		.devnode = typec_partner_devnode,
   686		.release = typec_partner_release,
   687	};
   688	
   689	/**
   690	 * typec_partner_set_identity - Report result from Discover Identity command
   691	 * @partner: The partner updated identity values
   692	 *
   693	 * This routine is used to report that the result of Discover Identity USB power
   694	 * delivery command has become available.
   695	 */
   696	int typec_partner_set_identity(struct typec_partner *partner)
   697	{
   698		if (!partner->identity)
   699			return -EINVAL;
   700	
   701		typec_report_identity(&partner->dev);
   702		return 0;
   703	}
   704	EXPORT_SYMBOL_GPL(typec_partner_set_identity);
   705	
   706	/**
   707	 * typec_partner_set_pd_revision - Set the PD revision supported by the partner
   708	 * @partner: The partner to be updated.
   709	 * @pd_revision:  USB Power Delivery Specification Revision supported by partner
   710	 *
   711	 * This routine is used to report that the PD revision of the port partner has
   712	 * become available.
   713	 */
   714	void typec_partner_set_pd_revision(struct typec_partner *partner, u16 pd_revision)
   715	{
   716		if (partner->pd_revision == pd_revision)
   717			return;
   718	
   719		partner->pd_revision = pd_revision;
   720		sysfs_notify(&partner->dev.kobj, NULL, "usb_power_delivery_revision");
   721		if (pd_revision != 0 && !partner->usb_pd) {
   722			partner->usb_pd = 1;
   723			sysfs_notify(&partner->dev.kobj, NULL,
   724				     "supports_usb_power_delivery");
   725		}
   726		kobject_uevent(&partner->dev.kobj, KOBJ_CHANGE);
   727	}
   728	EXPORT_SYMBOL_GPL(typec_partner_set_pd_revision);
   729	
   730	/**
   731	 * typec_partner_set_num_altmodes - Set the number of available partner altmodes
   732	 * @partner: The partner to be updated.
   733	 * @num_altmodes: The number of altmodes we want to specify as available.
   734	 *
   735	 * This routine is used to report the number of alternate modes supported by the
   736	 * partner. This value is *not* enforced in alternate mode registration routines.
   737	 *
   738	 * @partner.num_altmodes is set to -1 on partner registration, denoting that
   739	 * a valid value has not been set for it yet.
   740	 *
   741	 * Returns 0 on success or negative error number on failure.
   742	 */
   743	int typec_partner_set_num_altmodes(struct typec_partner *partner, int num_altmodes)
   744	{
   745		int ret;
   746	
   747		if (num_altmodes < 0)
   748			return -EINVAL;
   749	
   750		partner->num_altmodes = num_altmodes;
   751		ret = sysfs_update_group(&partner->dev.kobj, &typec_partner_group);
   752		if (ret < 0)
   753			return ret;
   754	
   755		sysfs_notify(&partner->dev.kobj, NULL, "number_of_alternate_modes");
   756		kobject_uevent(&partner->dev.kobj, KOBJ_CHANGE);
   757	
   758		return 0;
   759	}
   760	EXPORT_SYMBOL_GPL(typec_partner_set_num_altmodes);
   761	
   762	/**
   763	 * typec_partner_register_altmode - Register USB Type-C Partner Alternate Mode
   764	 * @partner: USB Type-C Partner that supports the alternate mode
   765	 * @desc: Description of the alternate mode
   766	 *
   767	 * This routine is used to register each alternate mode individually that
   768	 * @partner has listed in response to Discover SVIDs command. The modes for a
   769	 * SVID listed in response to Discover Modes command need to be listed in an
   770	 * array in @desc.
   771	 *
   772	 * Returns handle to the alternate mode on success or ERR_PTR on failure.
   773	 */
   774	struct typec_altmode *
   775	typec_partner_register_altmode(struct typec_partner *partner,
   776				       const struct typec_altmode_desc *desc)
   777	{
   778		return typec_register_altmode(&partner->dev, desc);
   779	}
   780	EXPORT_SYMBOL_GPL(typec_partner_register_altmode);
   781	
   782	/**
   783	 * typec_partner_set_svdm_version - Set negotiated Structured VDM (SVDM) Version
   784	 * @partner: USB Type-C Partner that supports SVDM
   785	 * @svdm_version: Negotiated SVDM Version
   786	 *
   787	 * This routine is used to save the negotiated SVDM Version.
   788	 */
   789	void typec_partner_set_svdm_version(struct typec_partner *partner,
   790					   enum usb_pd_svdm_ver svdm_version)
   791	{
   792		partner->svdm_version = svdm_version;
   793	}
   794	EXPORT_SYMBOL_GPL(typec_partner_set_svdm_version);
   795	
   796	/**
   797	 * typec_register_partner - Register a USB Type-C Partner
   798	 * @port: The USB Type-C Port the partner is connected to
   799	 * @desc: Description of the partner
   800	 *
   801	 * Registers a device for USB Type-C Partner described in @desc.
   802	 *
   803	 * Returns handle to the partner on success or ERR_PTR on failure.
   804	 */
   805	struct typec_partner *typec_register_partner(struct typec_port *port,
   806						     struct typec_partner_desc *desc)
   807	{
   808		struct typec_partner *partner;
   809		int ret;
   810	
   811		partner = kzalloc(sizeof(*partner), GFP_KERNEL);
   812		if (!partner)
   813			return ERR_PTR(-ENOMEM);
   814	
   815		ida_init(&partner->mode_ids);
   816		partner->usb_pd = desc->usb_pd;
   817		partner->pd_dev = desc->pd_dev;
   818		partner->accessory = desc->accessory;
   819		partner->num_altmodes = -1;
   820		partner->pd_revision = desc->pd_revision;
   821		partner->svdm_version = port->cap->svdm_version;
   822	
   823		if (desc->identity) {
   824			/*
   825			 * Creating directory for the identity only if the driver is
   826			 * able to provide data to it.
   827			 */
   828			partner->dev.groups = usb_pd_id_groups;
   829			partner->identity = desc->identity;
   830		}
   831	
   832		partner->dev.class = &typec_class;
   833		partner->dev.parent = &port->dev;
   834		partner->dev.type = &typec_partner_dev_type;
   835		dev_set_name(&partner->dev, "%s-partner", dev_name(&port->dev));
   836	
   837		if (partner->pd_dev)
   838			partner->dev.devt = MKDEV(PD_DEV_MAJOR, port->id * 4 + 3);
   839	
   840		ret = device_register(&partner->dev);
   841		if (ret) {
   842			dev_err(&port->dev, "failed to register partner (%d)\n", ret);
   843			put_device(&partner->dev);
   844			return ERR_PTR(ret);
   845		}
   846	
   847		return partner;
   848	}
   849	EXPORT_SYMBOL_GPL(typec_register_partner);
   850	
   851	/**
   852	 * typec_unregister_partner - Unregister a USB Type-C Partner
   853	 * @partner: The partner to be unregistered
   854	 *
   855	 * Unregister device created with typec_register_partner().
   856	 */
   857	void typec_unregister_partner(struct typec_partner *partner)
   858	{
   859		if (!IS_ERR_OR_NULL(partner))
   860			device_unregister(&partner->dev);
   861	}
   862	EXPORT_SYMBOL_GPL(typec_unregister_partner);
   863	
   864	/* ------------------------------------------------------------------------- */
   865	/* Type-C Cable Plugs */
   866	
 > 867	char *typec_plug_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
   868	{
   869		return kasprintf(GFP_KERNEL, "pd%u/plug%d",
   870				 to_typec_port(dev->parent->parent)->id,
   871				 to_typec_plug(dev)->index);
   872	}
   873	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38478 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev
  2021-10-26 14:33 ` [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev Heikki Krogerus
@ 2021-10-27  1:00   ` Jack Pham
  2021-10-27 11:10     ` Heikki Krogerus
  2021-10-27  8:06   ` kernel test robot
  1 sibling, 1 reply; 19+ messages in thread
From: Jack Pham @ 2021-10-27  1:00 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Prashant Malani, Benson Leung, Adam Thomson, Guenter Roeck,
	Badhri Jagan Sridharan, Gopal, Saranya, Regupathy, Rajaram,
	linux-usb, linux-kernel

On Tue, Oct 26, 2021 at 05:33:51PM +0300, Heikki Krogerus wrote:
>
> -static int ucsi_get_src_pdos(struct ucsi_connector *con)
> +int ucsi_read_pdos(struct ucsi_connector *con, int partner, int source, u32 *pdos)
>  {
> +	u32 pdo[PDO_MAX_OBJECTS];
> +	int num_pdos;
>  	int ret;
>  
>  	/* UCSI max payload means only getting at most 4 PDOs at a time */
> -	ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
> +	ret = ucsi_get_pdos(con, partner, source, pdo, 0, UCSI_MAX_PDOS);
>  	if (ret < 0)
>  		return ret;
>  
> -	con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> -	if (con->num_pdos < UCSI_MAX_PDOS)
> -		return 0;
> +	num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> +	if (num_pdos < UCSI_MAX_PDOS)
> +		goto done;
>  
>  	/* get the remaining PDOs, if any */
> -	ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
> +	ret = ucsi_get_pdos(con, partner, source, pdo, UCSI_MAX_PDOS,
>  			    PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
>  	if (ret < 0)
>  		return ret;
>  
> -	con->num_pdos += ret / sizeof(u32);
> +	num_pdos += ret / sizeof(u32);
> +done:
> +	memcpy(pdos, pdo, num_pdos * sizeof(pdo));
> +
> +	return num_pdos;
> +}
> +
> +static int ucsi_get_src_pdos(struct ucsi_connector *con)
> +{
> +	int ret;
> +
> +	ret = ucsi_read_pdos(con, 0, 1, con->src_pdos);

Second parameter should be 1 right?  Original intent of get_src_pdos()
is to retrieve the partner's source capabilities in order to populate
the power_supply.  Passing 0 as the partner param here changes the
behavior to retrieve the source PDOs of the port.

(BTW I'm going to send a quick patch for this to since this assumes that
port is sink and partner is source; when it's the other way around we
end up calling GET_PDOS on the sink partner when it might not even be
source capable).

Jack
-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev
  2021-10-26 14:33 ` [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev Heikki Krogerus
  2021-10-27  1:00   ` Jack Pham
@ 2021-10-27  8:06   ` kernel test robot
  1 sibling, 0 replies; 19+ messages in thread
From: kernel test robot @ 2021-10-27  8:06 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4781 bytes --]

Hi Heikki,

[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on usb/usb-testing]
[also build test WARNING on next-20211026]
[cannot apply to chrome-platform/for-next peter-chen-usb/for-usb-next linus/master balbi-usb/testing/next v5.15-rc7]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Heikki-Krogerus/USB-Power-Delivery-character-device-interface/20211026-224629
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: x86_64-rhel-8.3-kselftests (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/9eaf262732a6e75ba994550fa5d04daedf762ce6
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Heikki-Krogerus/USB-Power-Delivery-character-device-interface/20211026-224629
        git checkout 9eaf262732a6e75ba994550fa5d04daedf762ce6
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/usb/typec/ucsi/pd-dev.c:26:55: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected unsigned int [usertype] *pdos @@     got restricted __le32 * @@
   drivers/usb/typec/ucsi/pd-dev.c:26:55: sparse:     expected unsigned int [usertype] *pdos
   drivers/usb/typec/ucsi/pd-dev.c:26:55: sparse:     got restricted __le32 *
>> drivers/usb/typec/ucsi/pd-dev.c:34:33: sparse: sparse: incorrect type in assignment (different base types) @@     expected restricted __le32 @@     got unsigned int [usertype] request_data_obj @@
   drivers/usb/typec/ucsi/pd-dev.c:34:33: sparse:     expected restricted __le32
   drivers/usb/typec/ucsi/pd-dev.c:34:33: sparse:     got unsigned int [usertype] request_data_obj
   drivers/usb/typec/ucsi/pd-dev.c:37:55: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected unsigned int [usertype] *pdos @@     got restricted __le32 * @@
   drivers/usb/typec/ucsi/pd-dev.c:37:55: sparse:     expected unsigned int [usertype] *pdos
   drivers/usb/typec/ucsi/pd-dev.c:37:55: sparse:     got restricted __le32 *
   drivers/usb/typec/ucsi/pd-dev.c:66:49: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected unsigned int [usertype] *pdos @@     got restricted __le32 * @@
   drivers/usb/typec/ucsi/pd-dev.c:66:49: sparse:     expected unsigned int [usertype] *pdos
   drivers/usb/typec/ucsi/pd-dev.c:66:49: sparse:     got restricted __le32 *
   drivers/usb/typec/ucsi/pd-dev.c:73:49: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected unsigned int [usertype] *pdos @@     got restricted __le32 * @@
   drivers/usb/typec/ucsi/pd-dev.c:73:49: sparse:     expected unsigned int [usertype] *pdos
   drivers/usb/typec/ucsi/pd-dev.c:73:49: sparse:     got restricted __le32 *

vim +26 drivers/usb/typec/ucsi/pd-dev.c

    12	
    13	static int ucsi_pd_get_objects(const struct pd_dev *dev, struct pd_message *msg)
    14	{
    15		struct ucsi_connector *con = pd_dev_to_connector(dev);
    16		int partner = dev == &con->pd_partner_dev;
    17		int ret = -ENOTTY;
    18	
    19		mutex_lock(&con->lock);
    20	
    21		if (le16_to_cpu(msg->header) & PD_HEADER_EXT_HDR)
    22			goto err;
    23	
    24		switch (pd_header_type_le(msg->header)) {
    25		case PD_DATA_SOURCE_CAP:
  > 26			ret = ucsi_read_pdos(con, partner, 1, msg->payload);
    27			if (ret < 0)
    28				goto err;
    29	
    30			msg->header = PD_HEADER_LE(PD_DATA_SOURCE_CAP, 0, 0, 0, 0, ret);
    31			break;
    32		case PD_DATA_REQUEST:
    33			msg->header = PD_HEADER_LE(PD_DATA_REQUEST, 0, 0, 0, 0, 1);
  > 34			msg->payload[0] = con->status.request_data_obj;
    35			break;
    36		case PD_DATA_SINK_CAP:
    37			ret = ucsi_read_pdos(con, partner, 0, msg->payload);
    38			if (ret < 0)
    39				goto err;
    40	
    41			msg->header = PD_HEADER_LE(PD_DATA_SINK_CAP, 0, 0, 0, 0, ret);
    42			break;
    43		default:
    44			goto err;
    45		}
    46	
    47		ret = 0;
    48	err:
    49		mutex_unlock(&con->lock);
    50	
    51		return ret;
    52	}
    53	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 42115 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 0/4] USB Power Delivery character device interface
  2021-10-26 15:06 ` [RFC PATCH 0/4] USB Power Delivery character device interface Greg KH
@ 2021-10-27 11:02   ` Heikki Krogerus
  2021-10-27 12:53     ` Greg KH
  0 siblings, 1 reply; 19+ messages in thread
From: Heikki Krogerus @ 2021-10-27 11:02 UTC (permalink / raw)
  To: Greg KH
  Cc: Prashant Malani, Benson Leung, Adam Thomson, Guenter Roeck,
	Badhri Jagan Sridharan, Jack Pham, Gopal, Saranya, Regupathy,
	Rajaram, linux-usb, linux-kernel

Hi Greg,

On Tue, Oct 26, 2021 at 05:06:28PM +0200, Greg KH wrote:
> So, why not sysfs?  :)

This is about allowing the user space to take over the USB Power
Delivery communication and policy decisions in some cases. The user
space needs to be able to send and receive raw USB Power Delivery
messages one way or the other. I don't care about what's the interface
that we use.

Here we are talking about the PDOs, so basically the power contract.
Even if we figured out a way how to expose all the information from
the Capability, Status, Alert and what ever messages you need to the
user space via sysfs, and then allow the user to separately send the
Request Message, we would have only covered the power contract. That
does not cover everything, but it would also be unnecessarily
complicated to handle with separate sysfs files IMO.

Even with the power contract it would make more sense to me to just
allow the user space to simply read and write the raw messages, but
when we go the other things like Vendor Specific Messages, I don't
think there is any other way.

So we really do need to be able to tap into the USB Power Delivery
protocol layer directly from user space. I don't care about how we do
that - character device is just a suggestion, although, it does still
feel correct to me. Is there some other way we could do this?


thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev
  2021-10-27  1:00   ` Jack Pham
@ 2021-10-27 11:10     ` Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2021-10-27 11:10 UTC (permalink / raw)
  To: Jack Pham
  Cc: Prashant Malani, Benson Leung, Adam Thomson, Guenter Roeck,
	Badhri Jagan Sridharan, Gopal, Saranya, Regupathy, Rajaram,
	linux-usb, linux-kernel

On Tue, Oct 26, 2021 at 06:00:35PM -0700, Jack Pham wrote:
> On Tue, Oct 26, 2021 at 05:33:51PM +0300, Heikki Krogerus wrote:
> >
> > -static int ucsi_get_src_pdos(struct ucsi_connector *con)
> > +int ucsi_read_pdos(struct ucsi_connector *con, int partner, int source, u32 *pdos)
> >  {
> > +	u32 pdo[PDO_MAX_OBJECTS];
> > +	int num_pdos;
> >  	int ret;
> >  
> >  	/* UCSI max payload means only getting at most 4 PDOs at a time */
> > -	ret = ucsi_get_pdos(con, 1, con->src_pdos, 0, UCSI_MAX_PDOS);
> > +	ret = ucsi_get_pdos(con, partner, source, pdo, 0, UCSI_MAX_PDOS);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	con->num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> > -	if (con->num_pdos < UCSI_MAX_PDOS)
> > -		return 0;
> > +	num_pdos = ret / sizeof(u32); /* number of bytes to 32-bit PDOs */
> > +	if (num_pdos < UCSI_MAX_PDOS)
> > +		goto done;
> >  
> >  	/* get the remaining PDOs, if any */
> > -	ret = ucsi_get_pdos(con, 1, con->src_pdos, UCSI_MAX_PDOS,
> > +	ret = ucsi_get_pdos(con, partner, source, pdo, UCSI_MAX_PDOS,
> >  			    PDO_MAX_OBJECTS - UCSI_MAX_PDOS);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	con->num_pdos += ret / sizeof(u32);
> > +	num_pdos += ret / sizeof(u32);
> > +done:
> > +	memcpy(pdos, pdo, num_pdos * sizeof(pdo));
> > +
> > +	return num_pdos;
> > +}
> > +
> > +static int ucsi_get_src_pdos(struct ucsi_connector *con)
> > +{
> > +	int ret;
> > +
> > +	ret = ucsi_read_pdos(con, 0, 1, con->src_pdos);
> 
> Second parameter should be 1 right?  Original intent of get_src_pdos()
> is to retrieve the partner's source capabilities in order to populate
> the power_supply.  Passing 0 as the partner param here changes the
> behavior to retrieve the source PDOs of the port.

Sounds like there is a bug in the existing code. I'm not changing the
behviour in this patch.

> (BTW I'm going to send a quick patch for this to since this assumes that
> port is sink and partner is source; when it's the other way around we
> end up calling GET_PDOS on the sink partner when it might not even be
> source capable).

I'll check it...

thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 0/4] USB Power Delivery character device interface
  2021-10-27 11:02   ` Heikki Krogerus
@ 2021-10-27 12:53     ` Greg KH
  2021-10-28  7:17       ` Heikki Krogerus
  0 siblings, 1 reply; 19+ messages in thread
From: Greg KH @ 2021-10-27 12:53 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Prashant Malani, Benson Leung, Adam Thomson, Guenter Roeck,
	Badhri Jagan Sridharan, Jack Pham, Gopal, Saranya, Regupathy,
	Rajaram, linux-usb, linux-kernel

On Wed, Oct 27, 2021 at 02:02:42PM +0300, Heikki Krogerus wrote:
> Hi Greg,
> 
> On Tue, Oct 26, 2021 at 05:06:28PM +0200, Greg KH wrote:
> > So, why not sysfs?  :)
> 
> This is about allowing the user space to take over the USB Power
> Delivery communication and policy decisions in some cases. The user
> space needs to be able to send and receive raw USB Power Delivery
> messages one way or the other. I don't care about what's the interface
> that we use.
> 
> Here we are talking about the PDOs, so basically the power contract.
> Even if we figured out a way how to expose all the information from
> the Capability, Status, Alert and what ever messages you need to the
> user space via sysfs, and then allow the user to separately send the
> Request Message, we would have only covered the power contract. That
> does not cover everything, but it would also be unnecessarily
> complicated to handle with separate sysfs files IMO.
> 
> Even with the power contract it would make more sense to me to just
> allow the user space to simply read and write the raw messages, but
> when we go the other things like Vendor Specific Messages, I don't
> think there is any other way.
> 
> So we really do need to be able to tap into the USB Power Delivery
> protocol layer directly from user space. I don't care about how we do
> that - character device is just a suggestion, although, it does still
> feel correct to me. Is there some other way we could do this?

Ok, a char device sounds fine, but _what_ userspace code is going to be
using this interface?  We need to have a working version of that as well
before we could take this new interface, otherwise it wouldn't make much
sense.

And why does userspace have to do this, what is wrong with the kernel
doing it as it does today?  I.e. what is broken that adding a new api to
the kernel is going to fix?

That needs to be documented really really well.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices
  2021-10-26 14:33 ` [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices Heikki Krogerus
                     ` (2 preceding siblings ...)
  2021-10-26 18:45   ` kernel test robot
@ 2021-10-28  1:03   ` Prashant Malani
  2021-10-28  7:36     ` Heikki Krogerus
  3 siblings, 1 reply; 19+ messages in thread
From: Prashant Malani @ 2021-10-28  1:03 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Benson Leung, Adam Thomson, Guenter Roeck,
	Badhri Jagan Sridharan, Jack Pham, Gopal, Saranya, Regupathy,
	Rajaram, linux-usb, linux-kernel

Hi Heikki,

Thanks for sending the RFC.

On Tue, Oct 26, 2021 at 05:33:50PM +0300, Heikki Krogerus wrote:
> Interim.
> 
> TODO/ideas:
> - Figure out a proper magic value for the ioctl and check if
>   the ioctl range is OK.
> - Register separate PD device for the cdev, and register it
>   only if the device (port, plug or partner) actually
>   supports USB PD (or come up with some other solution?).
> - Introduce something like
> 
> 	struct pd_request {
> 		struct pd_message request;
> 		struct pd_message __user *response;
> 	};
> 
>   and use it instead of only single struct pd_messages everywhere.
> 
> - Add compat support.
> - What do we do with Alerts and Attentions?
> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
>  .../userspace-api/ioctl/ioctl-number.rst      |   1 +
>  drivers/usb/typec/Makefile                    |   2 +-
>  drivers/usb/typec/class.c                     |  42 ++++
>  drivers/usb/typec/class.h                     |   4 +
>  drivers/usb/typec/pd-dev.c                    | 210 ++++++++++++++++++
>  drivers/usb/typec/pd-dev.h                    |  15 ++
>  include/linux/usb/pd_dev.h                    |  22 ++
>  include/linux/usb/typec.h                     |   8 +
>  include/uapi/linux/usb/pd_dev.h               |  55 +++++
>  9 files changed, 358 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/usb/typec/pd-dev.c
>  create mode 100644 drivers/usb/typec/pd-dev.h
>  create mode 100644 include/linux/usb/pd_dev.h
>  create mode 100644 include/uapi/linux/usb/pd_dev.h
> 
> diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
> index 0ba463be6c588..fd443fd21f62a 100644
> --- a/Documentation/userspace-api/ioctl/ioctl-number.rst
> +++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
> @@ -175,6 +175,7 @@ Code  Seq#    Include File                                           Comments
>  'P'   60-6F  sound/sscape_ioctl.h                                    conflict!
>  'P'   00-0F  drivers/usb/class/usblp.c                               conflict!
>  'P'   01-09  drivers/misc/pci_endpoint_test.c                        conflict!
> +'P'   70-7F  uapi/linux/usb/pd_dev.h                                 <mailto:linux-usb@vger.kernel.org>
>  'Q'   all    linux/soundcard.h
>  'R'   00-1F  linux/random.h                                          conflict!
>  'R'   01     linux/rfkill.h                                          conflict!
> diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
> index a0adb8947a301..be44528168013 100644
> --- a/drivers/usb/typec/Makefile
> +++ b/drivers/usb/typec/Makefile
> @@ -1,6 +1,6 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_TYPEC)		+= typec.o
> -typec-y				:= class.o mux.o bus.o port-mapper.o
> +typec-y				:= class.o mux.o bus.o port-mapper.o pd-dev.o
>  obj-$(CONFIG_TYPEC)		+= altmodes/
>  obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
>  obj-$(CONFIG_TYPEC_UCSI)	+= ucsi/
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index aeef453aa6585..19fcc5da175d7 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -15,6 +15,7 @@
>  
>  #include "bus.h"
>  #include "class.h"
> +#include "pd-dev.h"
>  
>  static DEFINE_IDA(typec_index_ida);
>  
> @@ -665,6 +666,11 @@ static const struct attribute_group *typec_partner_groups[] = {
>  	NULL
>  };
>  
> +char *typec_partner_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
> +{
> +	return kasprintf(GFP_KERNEL, "pd%u/partner", to_typec_port(dev->parent)->id);
> +}
> +
>  static void typec_partner_release(struct device *dev)
>  {
>  	struct typec_partner *partner = to_typec_partner(dev);
> @@ -676,6 +682,7 @@ static void typec_partner_release(struct device *dev)
>  const struct device_type typec_partner_dev_type = {
>  	.name = "typec_partner",
>  	.groups = typec_partner_groups,
> +	.devnode = typec_partner_devnode,
>  	.release = typec_partner_release,
>  };
>  
> @@ -807,6 +814,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
>  
>  	ida_init(&partner->mode_ids);
>  	partner->usb_pd = desc->usb_pd;
> +	partner->pd_dev = desc->pd_dev;
>  	partner->accessory = desc->accessory;
>  	partner->num_altmodes = -1;
>  	partner->pd_revision = desc->pd_revision;
> @@ -826,6 +834,9 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
>  	partner->dev.type = &typec_partner_dev_type;
>  	dev_set_name(&partner->dev, "%s-partner", dev_name(&port->dev));
>  
> +	if (partner->pd_dev)
> +		partner->dev.devt = MKDEV(PD_DEV_MAJOR, port->id * 4 + 3);
> +
>  	ret = device_register(&partner->dev);
>  	if (ret) {
>  		dev_err(&port->dev, "failed to register partner (%d)\n", ret);
> @@ -853,6 +864,13 @@ EXPORT_SYMBOL_GPL(typec_unregister_partner);
>  /* ------------------------------------------------------------------------- */
>  /* Type-C Cable Plugs */
>  
> +char *typec_plug_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
> +{
> +	return kasprintf(GFP_KERNEL, "pd%u/plug%d",
> +			 to_typec_port(dev->parent->parent)->id,
> +			 to_typec_plug(dev)->index);
> +}
> +
>  static void typec_plug_release(struct device *dev)
>  {
>  	struct typec_plug *plug = to_typec_plug(dev);
> @@ -891,6 +909,7 @@ static const struct attribute_group *typec_plug_groups[] = {
>  const struct device_type typec_plug_dev_type = {
>  	.name = "typec_plug",
>  	.groups = typec_plug_groups,
> +	.devnode = typec_plug_devnode,
>  	.release = typec_plug_release,
>  };
>  
> @@ -973,11 +992,16 @@ struct typec_plug *typec_register_plug(struct typec_cable *cable,
>  	ida_init(&plug->mode_ids);
>  	plug->num_altmodes = -1;
>  	plug->index = desc->index;
> +	plug->pd_dev = desc->pd_dev;
>  	plug->dev.class = &typec_class;
>  	plug->dev.parent = &cable->dev;
>  	plug->dev.type = &typec_plug_dev_type;
>  	dev_set_name(&plug->dev, "%s-%s", dev_name(cable->dev.parent), name);
>  
> +	if (plug->pd_dev)
> +		plug->dev.devt = MKDEV(PD_DEV_MAJOR,
> +				       to_typec_port(cable->dev.parent)->id * 4 + 1 + plug->index);
> +
>  	ret = device_register(&plug->dev);
>  	if (ret) {
>  		dev_err(&cable->dev, "failed to register plug (%d)\n", ret);
> @@ -1595,6 +1619,11 @@ static int typec_uevent(struct device *dev, struct kobj_uevent_env *env)
>  	return ret;
>  }
>  
> +char *typec_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
> +{
> +	return kasprintf(GFP_KERNEL, "pd%u/port", to_typec_port(dev)->id);
> +}
> +
>  static void typec_release(struct device *dev)
>  {
>  	struct typec_port *port = to_typec_port(dev);
> @@ -1611,6 +1640,7 @@ const struct device_type typec_port_dev_type = {
>  	.name = "typec_port",
>  	.groups = typec_groups,
>  	.uevent = typec_uevent,
> +	.devnode = typec_devnode,
>  	.release = typec_release,
>  };
>  
> @@ -2044,6 +2074,7 @@ struct typec_port *typec_register_port(struct device *parent,
>  
>  	port->id = id;
>  	port->ops = cap->ops;
> +	port->pd_dev = cap->pd_dev;
>  	port->port_type = cap->type;
>  	port->prefer_role = cap->prefer_role;
>  
> @@ -2055,6 +2086,9 @@ struct typec_port *typec_register_port(struct device *parent,
>  	dev_set_name(&port->dev, "port%d", id);
>  	dev_set_drvdata(&port->dev, cap->driver_data);
>  
> +	if (port->pd_dev)
> +		port->dev.devt = MKDEV(PD_DEV_MAJOR, id * 4);
> +
>  	port->cap = kmemdup(cap, sizeof(*cap), GFP_KERNEL);
>  	if (!port->cap) {
>  		put_device(&port->dev);
> @@ -2121,8 +2155,15 @@ static int __init typec_init(void)
>  	if (ret)
>  		goto err_unregister_mux_class;
>  
> +	ret = usbpd_dev_init();
> +	if (ret)
> +		goto err_unregister_class;
> +
>  	return 0;
>  
> +err_unregister_class:
> +	class_unregister(&typec_class);
> +
>  err_unregister_mux_class:
>  	class_unregister(&typec_mux_class);
>  
> @@ -2135,6 +2176,7 @@ subsys_initcall(typec_init);
>  
>  static void __exit typec_exit(void)
>  {
> +	usbpd_dev_exit();
>  	class_unregister(&typec_class);
>  	ida_destroy(&typec_index_ida);
>  	bus_unregister(&typec_bus);
> diff --git a/drivers/usb/typec/class.h b/drivers/usb/typec/class.h
> index aef03eb7e1523..87c072f2ad753 100644
> --- a/drivers/usb/typec/class.h
> +++ b/drivers/usb/typec/class.h
> @@ -14,6 +14,7 @@ struct typec_plug {
>  	enum typec_plug_index		index;
>  	struct ida			mode_ids;
>  	int				num_altmodes;
> +	const struct pd_dev		*pd_dev;
>  };
>  
>  struct typec_cable {
> @@ -33,6 +34,7 @@ struct typec_partner {
>  	int				num_altmodes;
>  	u16				pd_revision; /* 0300H = "3.0" */
>  	enum usb_pd_svdm_ver		svdm_version;
> +	const struct pd_dev		*pd_dev;
>  };
>  
>  struct typec_port {
> @@ -59,6 +61,8 @@ struct typec_port {
>  	struct mutex			port_list_lock; /* Port list lock */
>  
>  	void				*pld;
> +
> +	const struct pd_dev		*pd_dev;
>  };
>  
>  #define to_typec_port(_dev_) container_of(_dev_, struct typec_port, dev)
> diff --git a/drivers/usb/typec/pd-dev.c b/drivers/usb/typec/pd-dev.c
> new file mode 100644
> index 0000000000000..436853e046ce4
> --- /dev/null
> +++ b/drivers/usb/typec/pd-dev.c
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * USB Power Delivery /dev entries
> + *
> + * Copyright (C) 2021 Intel Corporation
> + * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> + */
> +
> +#include <linux/cdev.h>
> +#include <linux/fs.h>
> +#include <linux/slab.h>
> +#include <linux/usb/pd_dev.h>
> +
> +#include "class.h"
> +
> +#define PD_DEV_MAX (MINORMASK + 1)
> +
> +dev_t usbpd_devt;
> +static struct cdev usb_pd_cdev;
> +
> +struct pddev {
> +	struct device *dev;
> +	struct typec_port *port;
> +	const struct pd_dev *pd_dev;
> +};
> +
> +static ssize_t usbpd_read(struct file *file, char __user *buf, size_t count, loff_t *offset)
> +{
> +	/* FIXME TODO XXX */
> +
> +	/* Alert and Attention handling here (with poll) ? */
> +
> +	return 0;
> +}
> +
> +static long usbpd_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> +{
> +	struct pddev *pd = file->private_data;
> +	void __user *p = (void __user *)arg;
> +	unsigned int pwr_role;
> +	struct pd_message msg;
> +	u32 configuration;
> +	int ret = 0;
> +
> +	switch (cmd) {
> +	case USBPDDEV_INFO:
> +		if (copy_to_user(p, pd->pd_dev->info, sizeof(*pd->pd_dev->info)))
> +			return -EFAULT;
> +		break;
> +	case USBPDDEV_CONFIGURE:
> +		if (!pd->pd_dev->ops->configure)
> +			return -ENOTTY;
> +
> +		if (copy_from_user(&configuration, p, sizeof(configuration)))
> +			return -EFAULT;
> +
> +		ret = pd->pd_dev->ops->configure(pd->pd_dev, configuration);
> +		if (ret)
> +			return ret;
> +		break;
> +	case USBPDDEV_PWR_ROLE:
> +		if (is_typec_plug(pd->dev))
> +			return -ENOTTY;
> +
> +		if (is_typec_partner(pd->dev)) {
> +			if (pd->port->pwr_role == TYPEC_SINK)
> +				pwr_role = TYPEC_SOURCE;
> +			else
> +				pwr_role = TYPEC_SINK;
> +		} else {
> +			pwr_role = pd->port->pwr_role;
> +		}
> +
> +		if (copy_to_user(p, &pwr_role, sizeof(unsigned int)))
> +			return -EFAULT;
> +		break;
> +	case USBPDDEV_GET_MESSAGE:
> +		if (!pd->pd_dev->ops->get_message)
> +			return -ENOTTY;
> +
> +		if (copy_from_user(&msg, p, sizeof(msg)))
> +			return -EFAULT;
> +
> +		ret = pd->pd_dev->ops->get_message(pd->pd_dev, &msg);
> +		if (ret)
> +			return ret;
> +
> +		if (copy_to_user(p, &msg, sizeof(msg)))
> +			return -EFAULT;
> +		break;
> +	case USBPDDEV_SET_MESSAGE:
> +		if (!pd->pd_dev->ops->set_message)
> +			return -ENOTTY;
> +
> +		ret = pd->pd_dev->ops->set_message(pd->pd_dev, &msg);
> +		if (ret)
> +			return ret;
> +
> +		if (copy_to_user(p, &msg, sizeof(msg)))
> +			return -EFAULT;
> +		break;
> +	case USBPDDEV_SUBMIT_MESSAGE:
> +		if (!pd->pd_dev->ops->submit)
> +			return -ENOTTY;
> +
> +		if (copy_from_user(&msg, p, sizeof(msg)))
> +			return -EFAULT;
> +
> +		ret = pd->pd_dev->ops->submit(pd->pd_dev, &msg);
> +		if (ret)
> +			return ret;
> +
> +		if (copy_to_user(p, &msg, sizeof(msg)))
> +			return -EFAULT;
> +		break;

Why is USBPDDEV_SUBMIT_MESSAGE different from USBPDDEV_SET_MESSAGE?
Shouldn't "setting" a PDO or property automatically "submit" it (using TCPM
or whatever interface is actually performing the PD messaging) if
appropriate (e.g Source Caps?). Is there a situation where one would
want to "set" a property but not "send" it?

It seems to me that the two can be combined into 1 rather than having
a separate command just for ports.

Best regards,

-Prashant


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 0/4] USB Power Delivery character device interface
  2021-10-27 12:53     ` Greg KH
@ 2021-10-28  7:17       ` Heikki Krogerus
  0 siblings, 0 replies; 19+ messages in thread
From: Heikki Krogerus @ 2021-10-28  7:17 UTC (permalink / raw)
  To: Greg KH
  Cc: Prashant Malani, Benson Leung, Adam Thomson, Guenter Roeck,
	Badhri Jagan Sridharan, Jack Pham, Gopal, Saranya, Regupathy,
	Rajaram, linux-usb, linux-kernel

On Wed, Oct 27, 2021 at 02:53:57PM +0200, Greg KH wrote:
> On Wed, Oct 27, 2021 at 02:02:42PM +0300, Heikki Krogerus wrote:
> > Hi Greg,
> > 
> > On Tue, Oct 26, 2021 at 05:06:28PM +0200, Greg KH wrote:
> > > So, why not sysfs?  :)
> > 
> > This is about allowing the user space to take over the USB Power
> > Delivery communication and policy decisions in some cases. The user
> > space needs to be able to send and receive raw USB Power Delivery
> > messages one way or the other. I don't care about what's the interface
> > that we use.
> > 
> > Here we are talking about the PDOs, so basically the power contract.
> > Even if we figured out a way how to expose all the information from
> > the Capability, Status, Alert and what ever messages you need to the
> > user space via sysfs, and then allow the user to separately send the
> > Request Message, we would have only covered the power contract. That
> > does not cover everything, but it would also be unnecessarily
> > complicated to handle with separate sysfs files IMO.
> > 
> > Even with the power contract it would make more sense to me to just
> > allow the user space to simply read and write the raw messages, but
> > when we go the other things like Vendor Specific Messages, I don't
> > think there is any other way.
> > 
> > So we really do need to be able to tap into the USB Power Delivery
> > protocol layer directly from user space. I don't care about how we do
> > that - character device is just a suggestion, although, it does still
> > feel correct to me. Is there some other way we could do this?
> 
> Ok, a char device sounds fine, but _what_ userspace code is going to be
> using this interface?  We need to have a working version of that as well
> before we could take this new interface, otherwise it wouldn't make much
> sense.
> 
> And why does userspace have to do this, what is wrong with the kernel
> doing it as it does today?  I.e. what is broken that adding a new api to
> the kernel is going to fix?
> 
> That needs to be documented really really well.

Sure.

thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices
  2021-10-28  1:03   ` Prashant Malani
@ 2021-10-28  7:36     ` Heikki Krogerus
  2021-11-09  0:27       ` Prashant Malani
  0 siblings, 1 reply; 19+ messages in thread
From: Heikki Krogerus @ 2021-10-28  7:36 UTC (permalink / raw)
  To: Prashant Malani
  Cc: Benson Leung, Adam Thomson, Guenter Roeck,
	Badhri Jagan Sridharan, Jack Pham, Gopal, Saranya, Regupathy,
	Rajaram, linux-usb, linux-kernel

Hi,

On Wed, Oct 27, 2021 at 06:03:08PM -0700, Prashant Malani wrote:
> Why is USBPDDEV_SUBMIT_MESSAGE different from USBPDDEV_SET_MESSAGE?
> Shouldn't "setting" a PDO or property automatically "submit" it (using TCPM
> or whatever interface is actually performing the PD messaging) if
> appropriate (e.g Source Caps?). Is there a situation where one would
> want to "set" a property but not "send" it?
> 
> It seems to me that the two can be combined into 1 rather than having
> a separate command just for ports.

USBPDDEV_SUBMIT_MESSAGE you use to send message directly to the partner.

USBPDDEV_SET_MESSAGE is meant to be used to store the values to a
cached message that the port manager should use next time there is
communication, but it does not send the message to the partner. So you
can use it even when there is no connection with a port, for example,
to store the values like the initial USB mode that should be used by
setting the EUDO message. Maybe the ioctl should be named
USBPDDEV_STORE_MESSAGE... I used "set" because it is sort of a
counterpart to USBPDDEV_GET_MESSAGE.

There is an explanation in include/uapi/linux/usb/pd_dev.h, please
check it. I'm curious also what you think about the idea with
USBPDDEV_CONFIGURE.

thanks,

-- 
heikki

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices
  2021-10-28  7:36     ` Heikki Krogerus
@ 2021-11-09  0:27       ` Prashant Malani
  0 siblings, 0 replies; 19+ messages in thread
From: Prashant Malani @ 2021-11-09  0:27 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: Benson Leung, Adam Thomson, Guenter Roeck,
	Badhri Jagan Sridharan, Jack Pham, Gopal, Saranya, Regupathy,
	Rajaram, linux-usb, linux-kernel

Hi Heikki,

On Thu, Oct 28, 2021 at 12:36 AM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> Hi,
>
> On Wed, Oct 27, 2021 at 06:03:08PM -0700, Prashant Malani wrote:
> > Why is USBPDDEV_SUBMIT_MESSAGE different from USBPDDEV_SET_MESSAGE?
> > Shouldn't "setting" a PDO or property automatically "submit" it (using TCPM
> > or whatever interface is actually performing the PD messaging) if
> > appropriate (e.g Source Caps?). Is there a situation where one would
> > want to "set" a property but not "send" it?
> >
> > It seems to me that the two can be combined into 1 rather than having
> > a separate command just for ports.
>
> USBPDDEV_SUBMIT_MESSAGE you use to send message directly to the partner.
>
> USBPDDEV_SET_MESSAGE is meant to be used to store the values to a
> cached message that the port manager should use next time there is
> communication, but it does not send the message to the partner. So you
> can use it even when there is no connection with a port, for example,
> to store the values like the initial USB mode that should be used by
> setting the EUDO message. Maybe the ioctl should be named
> USBPDDEV_STORE_MESSAGE... I used "set" because it is sort of a
> counterpart to USBPDDEV_GET_MESSAGE.
>
> There is an explanation in include/uapi/linux/usb/pd_dev.h, please
> check it.

Thanks for the further clarification. I guess I still don't see enough
need to differentiate SET/STORE
from SUBMIT; is there a situation where one would want to store the
source/sink caps for a port,
but not send/submit them immediately? When a partner is not connected
to a port, a set would
automatically just update the cached values and not perform a "submit"
(since there is nothing to
submit to). Perhaps there are (situations which require separate store
and submit), but I'm unable
to come up with one on the spot.

I'm curious also what you think about the idea with
> USBPDDEV_CONFIGURE.

It is indeed interesting. It seems like the specific interface for
this needs to be fleshed out more (will we
define a standard set of features which can be represented by the
|flags| and made configurable?). At present
I can't think of TCPM features which we'd want to toggle at runtime,
but I'm looking at it from a Chrome OS
perspective, so could be missing a bunch of use cases.

BTW, does poll work with this character device?

Best regards,

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2021-11-09  0:28 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-26 14:33 [RFC PATCH 0/4] USB Power Delivery character device interface Heikki Krogerus
2021-10-26 14:33 ` [RFC PATCH 1/4] usb: pd: uapi header split Heikki Krogerus
2021-10-26 16:05   ` kernel test robot
2021-10-26 14:33 ` [RFC PATCH 2/4] usb: typec: Character device for USB Power Delivery devices Heikki Krogerus
2021-10-26 15:08   ` Greg KH
2021-10-26 16:30   ` kernel test robot
2021-10-26 18:45   ` kernel test robot
2021-10-28  1:03   ` Prashant Malani
2021-10-28  7:36     ` Heikki Krogerus
2021-11-09  0:27       ` Prashant Malani
2021-10-26 14:33 ` [RFC PATCH 3/4] usb: typec: ucsi: Add support for PD cdev Heikki Krogerus
2021-10-27  1:00   ` Jack Pham
2021-10-27 11:10     ` Heikki Krogerus
2021-10-27  8:06   ` kernel test robot
2021-10-26 14:33 ` [RFC PATCH 4/4] tools: usb: Hideous test tool for USB PD char device Heikki Krogerus
2021-10-26 15:06 ` [RFC PATCH 0/4] USB Power Delivery character device interface Greg KH
2021-10-27 11:02   ` Heikki Krogerus
2021-10-27 12:53     ` Greg KH
2021-10-28  7:17       ` Heikki Krogerus

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.