[PATCH v3 0/3] usb: typec: Separate USB PD from USB Type-C

[PATCH v3 0/3] usb: typec: Separate USB PD from USB Type-C

[Heikki Krogerus]

Hi,

Now everything is a device. There are now other changes.


v2 cover letter:

In this version the USB Power Delivery support is now completely
separated into its own little subsystem. The USB Power Delivery
objects are not devices, but they are also no longer tied to any
device by default. This change makes it possible to share the USB PD
objects between multiple devices on top of being able to select the
objects that we want the device to use.

The USB Power Delivery objects are now placed under
/sys/kernel/usb_power_delivery directory. As an example:

        /sys/kernel/usb_power_delivery/pd0

So now that pd0 can be linked to a device, or devices, that want (or
can) use it to negotiate the USB PD contract with. An example where
two devices share the PD:

        /sys/class/typec/port0/usb_power_delivery -> ../../../../../../../kernel/usb_power_delivery/pd0
        /sys/class/typec/port1/usb_power_delivery -> ../../../../../../../kernel/usb_power_delivery/pd0

I did not change the directory hierarchy at all, because I'm assuming
that it is not a problem anymore:

        pd0/<message>/<object>/<field>

On top of that change, I also switched to tcpm.c from ucsi.c as
the first user of this thing.


v1 cover letter:

Ideally after this there should be no need to add any new USB Power
Delivery specific attribute files directly to the USB Type-C devices
in sysfs. They now have their own directory.

The idea of the series is that any device (so not just USB Type-C
connectors and the partners attached to them) that supports USB Power
Delivery can have this separate sub-directory "usb_power_delivery" in
sysfs, and that sub-directory will have all the USB Power Delivery
objects and all the other USB Power Delivery details.

There are already ways that allow us to read the USB Power Delivery
capabilities from potentially any USB PD capable USB device attached
to the bus - one way is defined in the USB Type-C Bridge
Specification.

Initially the Capability Messages (i.e. PDOs) are exposed.

This is an example (tree view) of the capabilities that the ports on a
normal x86 system advertise to the partner. First you have the message
directory (source_capabilities and sink_capabilities), and that will
have a sub-directory for each PDO that capability message has. The PDO
sub-directories are named by their type. The number in front of the
name is the object position of the PDO:

/sys/class/typec/port0/usb_power_delivery
|-- revision
|-- sink_capabilities/
|   |-- 1:fixed_supply/
|   |   |-- dual_role_data
|   |   |-- dual_role_power
|   |   |-- fast_role_swap_current
|   |   |-- operational_current
|   |   |-- unchunked_extended_messages_supported
|   |   |-- unconstrained_power
|   |   |-- usb_communication_capable
|   |   |-- usb_suspend_supported
|   |   `-- voltage
|   |-- 2:variable_supply/
|   |   |-- maximum_voltage
|   |   |-- minimum_voltage
|   |   `-- operational_current
|   `-- 3:battery/
|       |-- maximum_voltage
|       |-- minimum_voltage
|       `-- operational_power
`-- source_capabilities/
    `-- 1:fixed_supply/
        |-- dual_role_data
        |-- dual_role_power
        |-- maximum_current
        |-- unchunked_extended_messages_supported
        |-- unconstrained_power
        |-- usb_communication_capable
        |-- usb_suspend_supported
        `-- voltage

And these are the capabilities of my Thunderbolt3 dock:

/sys/class/typec/port0-partner/usb_power_delivery
|-- revision
|-- sink_capabilities/
|   `-- 1:fixed_supply/
|       |-- dual_role_data
|       |-- dual_role_power
|       |-- fast_role_swap_current
|       |-- operational_current
|       |-- unchunked_extended_messages_supported
|       |-- unconstrained_power
|       |-- usb_communication_capable
|       |-- usb_suspend_supported
|       `-- voltage
`-- source_capabilities/
    |-- 1:fixed_supply/
    |   |-- dual_role_data
    |   |-- dual_role_power
    |   |-- maximum_current
    |   |-- unchunked_extended_messages_supported
    |   |-- unconstrained_power
    |   |-- usb_communication_capable
    |   |-- usb_suspend_supported
    |   `-- voltage
    |-- 2:fixed_supply/
    |   |-- maximum_current
    |   `-- voltage
    |-- 3:fixed_supply/
    |   |-- maximum_current
    |   `-- voltage
    |-- 4:fixed_supply/
    |   |-- maximum_current
    |   `-- voltage
    `-- 5:fixed_supply/
        |-- maximum_current
        `-- voltage


Heikki Krogerus (3):
  usb: typec: Separate USB Power Delivery from USB Type-C
  usb: typec: USB Power Deliver helpers for ports and partners
  usb: typec: tcpm: Register USB Power Delivery Capabilities

 Documentation/ABI/testing/sysfs-class-typec   |   8 +
 .../testing/sysfs-class-usb_power_delivery    | 240 ++++++
 drivers/usb/typec/Makefile                    |   2 +-
 drivers/usb/typec/class.c                     | 148 ++++
 drivers/usb/typec/class.h                     |   4 +
 drivers/usb/typec/pd.c                        | 721 ++++++++++++++++++
 drivers/usb/typec/pd.h                        |  30 +
 drivers/usb/typec/tcpm/tcpm.c                 | 142 +++-
 include/linux/usb/pd.h                        |  35 +
 include/linux/usb/typec.h                     |  22 +
 10 files changed, 1350 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-usb_power_delivery
 create mode 100644 drivers/usb/typec/pd.c
 create mode 100644 drivers/usb/typec/pd.h

-- 
2.35.1

[PATCH v3 1/3] usb: typec: Separate USB Power Delivery from USB Type-C

[Heikki Krogerus]

Introducing a small device class for USB Power Delivery.
The idea with it is that we do not mix any more USB Power
Delivery information into the USB Type-C connectors only.
This separation will make it possible to register USB Power
Delivery devices also from other places, for example from
USB Type-C Bridges (see USB Type-C Bridge Specification).

The device class will not always deal with only the messages
and objects that were negotiated with the partner, but
instead messages and objects that can be used in the
negotiation. That allows the USB PD devices to be shared and
reconfigured. The ports can decide which objects are to be
advertised to the partner before the contract is negotiated.
It is also possible to allow the user space to make that
decision if needed.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 .../testing/sysfs-class-usb_power_delivery    | 240 ++++++
 drivers/usb/typec/Makefile                    |   2 +-
 drivers/usb/typec/pd.c                        | 721 ++++++++++++++++++
 drivers/usb/typec/pd.h                        |  30 +
 include/linux/usb/pd.h                        |  35 +
 include/linux/usb/typec.h                     |  10 +
 6 files changed, 1037 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/ABI/testing/sysfs-class-usb_power_delivery
 create mode 100644 drivers/usb/typec/pd.c
 create mode 100644 drivers/usb/typec/pd.h

diff --git a/Documentation/ABI/testing/sysfs-class-usb_power_delivery b/Documentation/ABI/testing/sysfs-class-usb_power_delivery
new file mode 100644
index 0000000000000..ce2b1b563cb34
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-usb_power_delivery
@@ -0,0 +1,240 @@
+What:		/sys/class/usb_power_delivery
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Directory for USB Power Delivery devices.
+
+What:		/sys/class/usb_power_delivery/.../revision
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		File showing the USB Power Delivery Specification Revision used
+		in communication.
+
+What:		/sys/class/usb_power_delivery/.../version
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		This is an optional attribute file showing the version of the
+		specific revision of the USB Power Delivery Specification. In
+		most cases the specification version is not known and the file
+		is not available.
+
+What:		/sys/class/usb_power_delivery/.../source-capabilities
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		The source capabilities message "Source_Capabilities" contains a
+		set of Power Data Objects (PDO), each representing a type of
+		power supply. The order of the PDO objects is defined in the USB
+		Power Delivery Specification. Each PDO - power supply - will
+		have its own device, and the PDO device name will start with the
+		object position number as the first character followed by the
+		power supply type name (":" as delimiter).
+
+			/sys/class/usb_power_delivery/.../source_capabilities/<position>:<type>
+
+What:		/sys/class/usb_power_delivery/.../sink-capabilities
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		The sink capability message "Sink_Capabilities" contains a set
+		of Power Data Objects (PDO) just like with source capabilities,
+		but instead of describing the power capabilities, these objects
+		describe the power requirements.
+
+		The order of the objects in the sink capability message is the
+		same as with the source capabilities message.
+
+Fixed Supplies
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:fixed_supply
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Devices containing the attributes (the bit fields) defined for
+		Fixed Supplies.
+
+		The device "1:fixed_supply" is special. USB Power Delivery
+		Specification dictates that the first PDO (at object position
+		1), and the only mandatory PDO, is always the vSafe5V Fixed
+		Supply Object. vSafe5V Object has additional fields defined for
+		it that the other Fixed Supply Objects do not have and that are
+		related to the USB capabilities rather than power capabilities.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/1:fixed_supply/dual_role_power
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		This file contains boolean value that tells does the device
+		support both source and sink power roles.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/1:fixed_supply/usb_suspend_supported
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		This file shows the value of the USB Suspend Supported bit in
+		vSafe5V Fixed Supply Object. If the bit is set then the device
+		will follow the USB 2.0 and USB 3.2 rules for suspend and
+		resume.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/1:fixed_supply/unconstrained_power
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		This file shows the value of the Unconstrained Power bit in
+		vSafe5V Fixed Supply Object. The bit is set when an external
+		source of power, powerful enough to power the entire system on
+		its own, is available for the device.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/1:fixed_supply/usb_communication_capable
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		This file shows the value of the USB Communication Capable bit in
+		vSafe5V Fixed Supply Object.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/1:fixed_supply/dual_role_data
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		This file shows the value of the Dual-Role Data bit in vSafe5V
+		Fixed Supply Object. Dual role data means ability act as both
+		USB host and USB device.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/1:fixed_supply/unchunked_extended_messages_supported
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		This file shows the value of the Unchunked Extended Messages
+		Supported bit in vSafe5V Fixed Supply Object.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:fixed_supply/voltage
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		The voltage the supply supports in millivolts.
+
+What:		/sys/class/usb_power_delivery/.../source-capabilities/<position>:fixed_supply/maximum_current
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Maximum current of the fixed source supply in milliamperes.
+
+What:		/sys/class/usb_power_delivery/.../sink-capabilities/<position>:fixed_supply/operational_current
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Operational current of the sink in milliamperes.
+
+What:		/sys/class/usb_power_delivery/.../sink-capabilities/<position>:fixed_supply/fast_role_swap_current
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		This file contains the value of the "Fast Role Swap USB Type-C
+		Current" field that tells the current level the sink requires
+		after a Fast Role Swap.
+		0 - Fast Swap not supported"
+		1 - Default USB Power"
+		2 - 1.5A@5V"
+		3 - 3.0A@5V"
+
+Variable Supplies
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:variable_supply
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Variable Power Supply PDO.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:variable_supply/maximum_voltage
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Maximum Voltage in millivolts.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:variable_supply/minimum_voltage
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Minimum Voltage in millivolts.
+
+What:		/sys/class/usb_power_delivery/.../source-capabilities/<position>:variable_supply/maximum_current
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		The maximum current in milliamperes that the source can supply
+		at the given Voltage range.
+
+What:		/sys/class/usb_power_delivery/.../sink-capabilities/<position>:variable_supply/operational_current
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		The operational current in milliamperes that the sink requires
+		at the given Voltage range.
+
+Battery Supplies
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:battery
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Battery PDO.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:battery/maximum_voltage
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Maximum Voltage in millivolts.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:battery/minimum_voltage
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Minimum Voltage in millivolts.
+
+What:		/sys/class/usb_power_delivery/.../source-capabilities/<position>:battery/maximum_power
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Maximum allowable Power in milliwatts.
+
+What:		/sys/class/usb_power_delivery/.../sink-capabilities/<position>:battery/operational_power
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		The operational power that the sink requires at the given
+		voltage range.
+
+Standard Power Range (SPR) Programmable Power Supplies
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:programmable_supply
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Programmable Power Supply (PPS) Augmented PDO (APDO).
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:programmable_supply/maximum_voltage
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Maximum Voltage in millivolts.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:programmable_supply/minimum_voltage
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Minimum Voltage in millivolts.
+
+What:		/sys/class/usb_power_delivery/.../<capability>/<position>:programmable_supply/maximum_current
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Maximum Current in milliamperes.
+
+What:		/sys/class/usb_power_delivery/.../source-capabilities/<position>:programmable_supply/pps_power_limited
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		The PPS Power Limited bit indicates whether or not the source
+		supply will exceed the rated output power if requested.
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 43626acc0aafb..2f174cd3e5df1 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
+typec-y				:= class.o mux.o bus.o pd.o
 typec-$(CONFIG_ACPI)		+= port-mapper.o
 obj-$(CONFIG_TYPEC)		+= altmodes/
 obj-$(CONFIG_TYPEC_TCPM)	+= tcpm/
diff --git a/drivers/usb/typec/pd.c b/drivers/usb/typec/pd.c
new file mode 100644
index 0000000000000..a2fd1bdc4c5e9
--- /dev/null
+++ b/drivers/usb/typec/pd.c
@@ -0,0 +1,721 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB Power Delivery sysfs entries
+ *
+ * Copyright (C) 2022, Intel Corporation
+ * Author: Heikki Krogerus <heikki.krogerus@linux.intel.com>
+ */
+
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/usb/pd.h>
+
+#include "pd.h"
+
+static DEFINE_IDA(pd_ida);
+
+static struct class pd_class = {
+	.name = "usb_power_delivery",
+	.owner = THIS_MODULE,
+};
+
+#define to_pdo(o) container_of(o, struct pdo, dev)
+
+struct pdo {
+	struct device dev;
+	int object_position;
+	u32 pdo;
+};
+
+static void pdo_release(struct device *dev)
+{
+	kfree(to_pdo(dev));
+}
+
+/* -------------------------------------------------------------------------- */
+/* Fixed Supply */
+
+static ssize_t
+dual_role_power_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%u\n", !!(to_pdo(kobj_to_dev(kobj))->pdo & PDO_FIXED_DUAL_ROLE));
+}
+
+static ssize_t
+usb_suspend_supported_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%u\n", !!(to_pdo(kobj_to_dev(kobj))->pdo & PDO_FIXED_SUSPEND));
+}
+
+static ssize_t
+unconstrained_power_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%u\n", !!(to_pdo(kobj_to_dev(kobj))->pdo & PDO_FIXED_EXTPOWER));
+}
+
+static ssize_t
+usb_communication_capable_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%u\n", !!(to_pdo(kobj_to_dev(kobj))->pdo & PDO_FIXED_USB_COMM));
+}
+
+static ssize_t
+dual_role_data_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%u\n", !!(to_pdo(kobj_to_dev(kobj))->pdo & PDO_FIXED_DATA_SWAP));
+}
+
+static ssize_t
+unchunked_extended_messages_supported_show(struct kobject *kobj,
+					   struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%u\n", !!(to_pdo(kobj_to_dev(kobj))->pdo & PDO_FIXED_UNCHUNK_EXT));
+}
+
+/*
+ * REVISIT: Peak Current requires access also to the RDO.
+static ssize_t
+peak_current_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	...
+}
+*/
+
+static ssize_t
+fast_role_swap_current_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%u\n",
+			  to_pdo(kobj_to_dev(kobj))->pdo >> PDO_FIXED_FRS_CURR_SHIFT) & 3;
+}
+
+static ssize_t
+voltage_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%umV\n", pdo_fixed_voltage(to_pdo(kobj_to_dev(kobj))->pdo));
+}
+
+/* Shared with Variable supplies, both source and sink */
+static ssize_t current_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%umA\n", pdo_max_current(to_pdo(kobj_to_dev(kobj))->pdo));
+}
+
+/* These additional details are only available with vSafe5V supplies */
+static struct kobj_attribute dual_role_power_attr = __ATTR_RO(dual_role_power);
+static struct kobj_attribute usb_suspend_supported_attr = __ATTR_RO(usb_suspend_supported);
+static struct kobj_attribute unconstrained_power_attr = __ATTR_RO(unconstrained_power);
+static struct kobj_attribute usb_communication_capable_attr = __ATTR_RO(usb_communication_capable);
+static struct kobj_attribute dual_role_data_attr = __ATTR_RO(dual_role_data);
+static struct kobj_attribute
+unchunked_extended_messages_supported_attr = __ATTR_RO(unchunked_extended_messages_supported);
+
+/* Visible on all Fixed type source supplies */
+/*static struct kobj_attribute peak_current_attr = __ATTR_RO(peak_current);*/
+/* Visible on Fixed type sink supplies */
+static struct kobj_attribute fast_role_swap_current_attr = __ATTR_RO(fast_role_swap_current);
+
+/* Shared with Variable type supplies */
+static struct kobj_attribute maximum_current_attr = {
+	.attr = {
+		.name = "maximum_current",
+		.mode = 0444,
+	},
+	.show = current_show,
+};
+
+static struct kobj_attribute operational_current_attr = {
+	.attr = {
+		.name = "operational_current",
+		.mode = 0444,
+	},
+	.show = current_show,
+};
+
+/* Visible on all Fixed type supplies */
+static struct kobj_attribute voltage_attr = __ATTR_RO(voltage);
+
+static struct attribute *source_fixed_supply_attrs[] = {
+	&dual_role_power_attr.attr,
+	&usb_suspend_supported_attr.attr,
+	&unconstrained_power_attr.attr,
+	&usb_communication_capable_attr.attr,
+	&dual_role_data_attr.attr,
+	&unchunked_extended_messages_supported_attr.attr,
+	/*&peak_current_attr.attr,*/
+	&voltage_attr.attr,
+	&maximum_current_attr.attr,
+	NULL
+};
+
+static umode_t fixed_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+	if (to_pdo(kobj_to_dev(kobj))->object_position &&
+	    /*attr != &peak_current_attr.attr &&*/
+	    attr != &voltage_attr.attr &&
+	    attr != &maximum_current_attr.attr &&
+	    attr != &operational_current_attr.attr)
+		return 0;
+
+	return attr->mode;
+}
+
+static const struct attribute_group source_fixed_supply_group = {
+	.is_visible = fixed_attr_is_visible,
+	.attrs = source_fixed_supply_attrs,
+};
+__ATTRIBUTE_GROUPS(source_fixed_supply);
+
+static struct device_type source_fixed_supply_type = {
+	.name = "pdo",
+	.release = pdo_release,
+	.groups = source_fixed_supply_groups,
+};
+
+static struct attribute *sink_fixed_supply_attrs[] = {
+	&dual_role_power_attr.attr,
+	&usb_suspend_supported_attr.attr,
+	&unconstrained_power_attr.attr,
+	&usb_communication_capable_attr.attr,
+	&dual_role_data_attr.attr,
+	&unchunked_extended_messages_supported_attr.attr,
+	&fast_role_swap_current_attr.attr,
+	&voltage_attr.attr,
+	&operational_current_attr.attr,
+	NULL
+};
+
+static const struct attribute_group sink_fixed_supply_group = {
+	.is_visible = fixed_attr_is_visible,
+	.attrs = sink_fixed_supply_attrs,
+};
+__ATTRIBUTE_GROUPS(sink_fixed_supply);
+
+static struct device_type sink_fixed_supply_type = {
+	.name = "pdo",
+	.release = pdo_release,
+	.groups = sink_fixed_supply_groups,
+};
+
+/* -------------------------------------------------------------------------- */
+/* Variable Supply */
+
+static ssize_t
+maximum_voltage_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%umV\n", pdo_max_voltage(to_pdo(kobj_to_dev(kobj))->pdo));
+}
+
+static ssize_t
+minimum_voltage_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%umV\n", pdo_min_voltage(to_pdo(kobj_to_dev(kobj))->pdo));
+}
+
+/* Shared with Battery */
+static struct kobj_attribute maximum_voltage_attr = __ATTR_RO(maximum_voltage);
+static struct kobj_attribute minimum_voltage_attr = __ATTR_RO(minimum_voltage);
+
+static struct attribute *source_variable_supply_attrs[] = {
+	&maximum_voltage_attr.attr,
+	&minimum_voltage_attr.attr,
+	&maximum_current_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(source_variable_supply);
+
+static struct device_type source_variable_supply_type = {
+	.name = "pdo",
+	.release = pdo_release,
+	.groups = source_variable_supply_groups,
+};
+
+static struct attribute *sink_variable_supply_attrs[] = {
+	&maximum_voltage_attr.attr,
+	&minimum_voltage_attr.attr,
+	&operational_current_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(sink_variable_supply);
+
+static struct device_type sink_variable_supply_type = {
+	.name = "pdo",
+	.release = pdo_release,
+	.groups = sink_variable_supply_groups,
+};
+
+/* -------------------------------------------------------------------------- */
+/* Battery */
+
+static ssize_t
+maximum_power_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%umW\n", pdo_max_power(to_pdo(kobj_to_dev(kobj))->pdo));
+}
+
+static ssize_t
+operational_power_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%umW\n", pdo_max_power(to_pdo(kobj_to_dev(kobj))->pdo));
+}
+
+static struct kobj_attribute maximum_power_attr = __ATTR_RO(maximum_power);
+static struct kobj_attribute operational_power_attr = __ATTR_RO(operational_power);
+
+static struct attribute *source_battery_attrs[] = {
+	&maximum_voltage_attr.attr,
+	&minimum_voltage_attr.attr,
+	&maximum_power_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(source_battery);
+
+static struct device_type source_battery_type = {
+	.name = "pdo",
+	.release = pdo_release,
+	.groups = source_battery_groups,
+};
+
+static struct attribute *sink_battery_attrs[] = {
+	&maximum_voltage_attr.attr,
+	&minimum_voltage_attr.attr,
+	&operational_power_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(sink_battery);
+
+static struct device_type sink_battery_type = {
+	.name = "pdo",
+	.release = pdo_release,
+	.groups = sink_battery_groups,
+};
+
+/* -------------------------------------------------------------------------- */
+/* Standard Power Range (SPR) Programmable Power Supply (PPS) */
+
+static ssize_t
+pps_power_limited_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%u\n", !!(to_pdo(kobj_to_dev(kobj))->pdo & BIT(27)));
+}
+
+static ssize_t
+pps_max_voltage_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%umV\n", pdo_pps_apdo_max_voltage(to_pdo(kobj_to_dev(kobj))->pdo));
+}
+
+static ssize_t
+pps_min_voltage_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%umV\n", pdo_pps_apdo_min_voltage(to_pdo(kobj_to_dev(kobj))->pdo));
+}
+
+static ssize_t
+pps_max_current_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%umA\n", pdo_pps_apdo_max_current(to_pdo(kobj_to_dev(kobj))->pdo));
+}
+
+static struct kobj_attribute pps_power_limited_attr = __ATTR_RO(pps_power_limited);
+
+static struct kobj_attribute pps_max_voltage_attr = {
+	.attr = {
+		.name = "maximum_voltage",
+		.mode = 0444,
+	},
+	.show = pps_max_voltage_show,
+};
+
+static struct kobj_attribute pps_min_voltage_attr = {
+	.attr = {
+		.name = "minimum_voltage",
+		.mode = 0444,
+	},
+	.show = pps_min_voltage_show,
+};
+
+static struct kobj_attribute pps_max_current_attr = {
+	.attr = {
+		.name = "maximum_current",
+		.mode = 0444,
+	},
+	.show = pps_max_current_show,
+};
+
+static struct attribute *source_pps_attrs[] = {
+	&pps_power_limited_attr.attr,
+	&pps_max_voltage_attr.attr,
+	&pps_min_voltage_attr.attr,
+	&pps_max_current_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(source_pps);
+
+static struct device_type source_pps_type = {
+	.name = "pdo",
+	.release = pdo_release,
+	.groups = source_pps_groups,
+};
+
+static struct attribute *sink_pps_attrs[] = {
+	&pps_max_voltage_attr.attr,
+	&pps_min_voltage_attr.attr,
+	&pps_max_current_attr.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(sink_pps);
+
+static struct device_type sink_pps_type = {
+	.name = "pdo",
+	.release = pdo_release,
+	.groups = sink_pps_groups,
+};
+
+/* -------------------------------------------------------------------------- */
+
+static const char * const supply_name[] = {
+	[PDO_TYPE_FIXED] = "fixed_supply",
+	[PDO_TYPE_BATT]  = "battery",
+	[PDO_TYPE_VAR]	 = "variable_supply",
+};
+
+static const char * const apdo_supply_name[] = {
+	[APDO_TYPE_PPS]  = "programmable_supply",
+};
+
+static struct device_type *source_type[] = {
+	[PDO_TYPE_FIXED] = &source_fixed_supply_type,
+	[PDO_TYPE_BATT]  = &source_battery_type,
+	[PDO_TYPE_VAR]   = &source_variable_supply_type,
+};
+
+static struct device_type *source_apdo_type[] = {
+	[APDO_TYPE_PPS]  = &source_pps_type,
+};
+
+static struct device_type *sink_type[] = {
+	[PDO_TYPE_FIXED] = &sink_fixed_supply_type,
+	[PDO_TYPE_BATT]  = &sink_battery_type,
+	[PDO_TYPE_VAR]   = &sink_variable_supply_type,
+};
+
+static struct device_type *sink_apdo_type[] = {
+	[APDO_TYPE_PPS]  = &sink_pps_type,
+};
+
+/* REVISIT: Export when EPR_*_Capabilities need to be supported. */
+static int add_pdo(struct pd_capabilities *cap, u32 pdo, int position)
+{
+	struct device_type *type;
+	const char *name;
+	struct pdo *p;
+	int ret;
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	p->pdo = pdo;
+	p->object_position = position;
+
+	if (pdo_type(pdo) == PDO_TYPE_APDO) {
+		/* FIXME: Only PPS supported for now! Skipping others. */
+		if (pdo_apdo_type(pdo) > APDO_TYPE_PPS) {
+			dev_warn(&cap->dev, "Unknown APDO type. PDO 0x%08x\n", pdo);
+			kfree(p);
+			return 0;
+		}
+
+		if (is_source(cap->role))
+			type = source_apdo_type[pdo_apdo_type(pdo)];
+		else
+			type = sink_apdo_type[pdo_apdo_type(pdo)];
+
+		name = apdo_supply_name[pdo_apdo_type(pdo)];
+	} else {
+		if (is_source(cap->role))
+			type = source_type[pdo_type(pdo)];
+		else
+			type = sink_type[pdo_type(pdo)];
+
+		name = supply_name[pdo_type(pdo)];
+	}
+
+	p->dev.parent = &cap->dev;
+	p->dev.type = type;
+	dev_set_name(&p->dev, "%u:%s", position + 1, name);
+
+	ret = device_register(&p->dev);
+	if (ret) {
+		put_device(&p->dev);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int remove_pdo(struct device *dev, void *data)
+{
+	device_unregister(dev);
+	return 0;
+}
+
+/* -------------------------------------------------------------------------- */
+
+static const char * const cap_name[] = {
+	[TYPEC_SINK]    = "sink-capabilities",
+	[TYPEC_SOURCE]  = "source-capabilities",
+};
+
+static void pd_capabilities_release(struct device *dev)
+{
+	kfree(to_pd_capabilities(dev));
+}
+
+static struct device_type pd_capabilities_type = {
+	.name = "capabilities",
+	.release = pd_capabilities_release,
+};
+
+/**
+ * pd_register_capabilities - Register a set of capabilities.
+ * @pd: The USB PD instance that the capabilities belong to.
+ * @desc: Description of the Capablities Message.
+ *
+ * This function registers a Capabilities Message described in @desc. The
+ * capabilities will have their own sub-directory under @pd in sysfs.
+ *
+ * The function returns pointer to struct pd_capabilities, or ERR_PRT(errno).
+ */
+struct pd_capabilities *pd_register_capabilities(struct pd *pd, struct pd_caps_desc *desc)
+{
+	struct pd_capabilities *cap;
+	int ret;
+	int i;
+
+	cap = kzalloc(sizeof(*cap), GFP_KERNEL);
+	if (!cap)
+		return ERR_PTR(-ENOMEM);
+
+	cap->pd = pd;
+	cap->role = desc->role;
+
+	cap->dev.parent = &pd->dev;
+	cap->dev.type = &pd_capabilities_type;
+	dev_set_name(&cap->dev, "%s", cap_name[cap->role]);
+
+	ret = device_register(&cap->dev);
+	if (ret) {
+		put_device(&cap->dev);
+		return ERR_PTR(ret);
+	}
+
+	for (i = 0; i < PDO_MAX_OBJECTS && desc->pdo[i]; i++) {
+		ret = add_pdo(cap, desc->pdo[i], i);
+		if (ret) {
+			pd_unregister_capabilities(cap);
+			return ERR_PTR(ret);
+		}
+	}
+
+	return cap;
+}
+EXPORT_SYMBOL_GPL(pd_register_capabilities);
+
+/**
+ * pd_unregister_capabilities - Unregister a set of capabilities
+ * @cap: The capabilities
+ */
+void pd_unregister_capabilities(struct pd_capabilities *cap)
+{
+	if (!cap)
+		return;
+
+	device_for_each_child(&cap->dev, NULL, remove_pdo);
+	device_unregister(&cap->dev);
+}
+EXPORT_SYMBOL_GPL(pd_unregister_capabilities);
+
+/* -------------------------------------------------------------------------- */
+
+static ssize_t revision_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	struct pd *pd = to_pd(kobj_to_dev(kobj));
+
+	return sysfs_emit(buf, "%u.%u\n", (pd->revision >> 8) & 0xff, (pd->revision >> 4) & 0xf);
+}
+
+static ssize_t version_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf)
+{
+	struct pd *pd = to_pd(kobj_to_dev(kobj));
+
+	return sysfs_emit(buf, "%u.%u\n", (pd->version >> 8) & 0xff, (pd->version >> 4) & 0xf);
+}
+
+static struct kobj_attribute revision_attr = __ATTR_RO(revision);
+static struct kobj_attribute version_attr = __ATTR_RO(version);
+
+static struct attribute *pd_attrs[] = {
+	&revision_attr.attr,
+	&version_attr.attr,
+	NULL
+};
+
+static umode_t pd_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+	struct pd *pd = to_pd(kobj_to_dev(kobj));
+
+	if (attr == &version_attr.attr && !pd->version)
+		return 0;
+
+	return attr->mode;
+}
+
+static const struct attribute_group pd_group = {
+	.is_visible = pd_attr_is_visible,
+	.attrs = pd_attrs,
+};
+__ATTRIBUTE_GROUPS(pd);
+
+static void pd_release(struct device *dev)
+{
+	struct pd *pd = to_pd(dev);
+
+	ida_simple_remove(&pd_ida, pd->id);
+	kfree(pd);
+}
+
+static struct device_type pd_type = {
+	.name = "usb_power_delivery",
+	.release = pd_release,
+	.groups = pd_groups,
+};
+
+struct pd *pd_find(const char *name)
+{
+	struct device *dev;
+
+	dev = class_find_device_by_name(&pd_class, name);
+
+	return dev ? to_pd(dev) : NULL;
+}
+
+/**
+ * pd_register - Register USB Power Delivery Support.
+ * @parent: Parent device.
+ * @desc: Description of the USB PD contract.
+ *
+ * This routine can be used to register USB Power Delivery capabilities that a
+ * device or devices can support. These capabilities represent all the
+ * capabilities that can be negotiated with a partner, so not only the Power
+ * Capabilities that are negotiated using the USB PD Capabilities Message.
+ *
+ * The USB Power Delivery Support object that this routine generates can be used
+ * as the parent object for all the actual USB Power Delivery Messages and
+ * objects that can be negotiated with the partner.
+ *
+ * Returns handle to struct pd or ERR_PTR.
+ */
+struct pd *pd_register(struct device *parent, struct pd_desc *desc)
+{
+	struct pd *pd;
+	int ret;
+
+	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+	if (!pd)
+		return ERR_PTR(-ENOMEM);
+
+	ret = ida_simple_get(&pd_ida, 0, 0, GFP_KERNEL);
+	if (ret < 0) {
+		kfree(pd);
+		return ERR_PTR(ret);
+	}
+
+	pd->id = ret;
+	pd->revision = desc->revision;
+	pd->version = desc->version;
+
+	pd->dev.parent = parent;
+	pd->dev.type = &pd_type;
+	pd->dev.class = &pd_class;
+	dev_set_name(&pd->dev, "pd%d", pd->id);
+
+	ret = device_register(&pd->dev);
+	if (ret) {
+		put_device(&pd->dev);
+		return ERR_PTR(ret);
+	}
+
+	return pd;
+}
+EXPORT_SYMBOL_GPL(pd_register);
+
+/**
+ * pd_unregister - Unregister USB Power Delivery Support.
+ * @pd: The USB PD contract.
+ */
+void pd_unregister(struct pd *pd)
+{
+	if (IS_ERR_OR_NULL(pd))
+		return;
+
+	device_unregister(&pd->dev);
+}
+EXPORT_SYMBOL_GPL(pd_unregister);
+
+/**
+ * pd_link_device - Link device to its USB PD object.
+ * @pd: The USB PD instance.
+ * @dev: The device.
+ *
+ * This function can be used to create a symlink named "usb_power_delivery" for
+ * @dev that points to @pd.
+ */
+int pd_link_device(struct pd *pd, struct device *dev)
+{
+	int ret;
+
+	if (IS_ERR_OR_NULL(pd) || !dev)
+		return 0;
+
+	ret = sysfs_create_link(&dev->kobj, &pd->dev.kobj, "usb_power_delivery");
+	if (ret)
+		return ret;
+
+	get_device(&pd->dev);
+	get_device(dev);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pd_link_device);
+
+/**
+ * pd_unlink_device - Unlink device from its USB PD object.
+ * @pd: The USB PD instance.
+ * @dev: The device.
+ *
+ * Remove the symlink that was previously created with pd_link_device().
+ */
+void pd_unlink_device(struct pd *pd, struct device *dev)
+{
+	if (IS_ERR_OR_NULL(pd) || !dev)
+		return;
+
+	sysfs_remove_link(&dev->kobj, "usb_power_delivery");
+	put_device(&pd->dev);
+	put_device(dev);
+}
+EXPORT_SYMBOL_GPL(pd_unlink_device);
+
+/* -------------------------------------------------------------------------- */
+
+int __init pd_init(void)
+{
+	return class_register(&pd_class);
+}
+
+void __exit pd_exit(void)
+{
+	ida_destroy(&pd_ida);
+	class_unregister(&pd_class);
+}
diff --git a/drivers/usb/typec/pd.h b/drivers/usb/typec/pd.h
new file mode 100644
index 0000000000000..6fe2426b9b2ab
--- /dev/null
+++ b/drivers/usb/typec/pd.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __USB_POWER_DELIVERY__
+#define __USB_POWER_DELIVERY__
+
+#include <linux/kobject.h>
+
+struct pd_capabilities {
+	struct device dev;
+	struct pd *pd;
+	enum typec_role role;
+};
+
+struct pd {
+	struct device		dev;
+	int			id;
+
+	u16			revision; /* 0300H = "3.0" */
+	u16			version;
+};
+
+#define to_pd_capabilities(o) container_of(o, struct pd_capabilities, dev)
+#define to_pd(o) container_of(o, struct pd, dev)
+
+struct pd *pd_find(const char *name);
+
+int pd_init(void);
+void pd_exit(void);
+
+#endif /* __USB_POWER_DELIVERY__ */
diff --git a/include/linux/usb/pd.h b/include/linux/usb/pd.h
index 96b7ff66f074b..42a58efebd419 100644
--- a/include/linux/usb/pd.h
+++ b/include/linux/usb/pd.h
@@ -495,4 +495,39 @@ static inline unsigned int rdo_max_power(u32 rdo)
 
 #define PD_P_SNK_STDBY_MW	2500	/* 2500 mW */
 
+#if IS_ENABLED(CONFIG_TYPEC)
+
+struct pd;
+
+/**
+ * pd_desc - USB Power Delivery Descriptor
+ * @revision: USB Power Delivery Specification Revision
+ * @version: USB Power Delivery Specicication Version - optional
+ */
+struct pd_desc {
+	u16 revision;
+	u16 version;
+};
+
+/**
+ * pd_caps_desc - Description of USB Power Delivery Capabilities Message
+ * @pdo: The Power Data Objects in the Capability Message
+ * @role: Power role of the capabilities
+ */
+struct pd_caps_desc {
+	u32 pdo[PDO_MAX_OBJECTS];
+	enum typec_role role;
+};
+
+struct pd_capabilities *pd_register_capabilities(struct pd *pd, struct pd_caps_desc *desc);
+void pd_unregister_capabilities(struct pd_capabilities *cap);
+
+struct pd *pd_register(struct device *parent, struct pd_desc *desc);
+void pd_unregister(struct pd *pd);
+
+int pd_link_device(struct pd *pd, struct device *dev);
+void pd_unlink_device(struct pd *pd, struct device *dev);
+
+#endif /* CONFIG_TYPEC */
+
 #endif /* __LINUX_USB_PD_H */
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index fdf737d48b3bf..45e28d14ae56e 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -52,6 +52,16 @@ enum typec_role {
 	TYPEC_SOURCE,
 };
 
+static inline int is_sink(enum typec_role role)
+{
+	return role == TYPEC_SINK;
+}
+
+static inline int is_source(enum typec_role role)
+{
+	return role == TYPEC_SOURCE;
+}
+
 enum typec_pwr_opmode {
 	TYPEC_PWR_MODE_USB,
 	TYPEC_PWR_MODE_1_5A,
-- 
2.35.1

[PATCH v3 2/3] usb: typec: USB Power Deliver helpers for ports and partners

[Heikki Krogerus]

All the USB Type-C Connector Class devices are protected, so
the drivers can not directly access them. This will adds a
few helpers that can be used to link the ports and partners
to the correct USB Power Delivery objects.

For ports a new optional sysfs attribute file is also added
that can be used to select the USB Power Delivery
capabilities that the port will advertise to the partner.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 Documentation/ABI/testing/sysfs-class-typec |   8 ++
 drivers/usb/typec/class.c                   | 148 ++++++++++++++++++++
 drivers/usb/typec/class.h                   |   4 +
 include/linux/usb/typec.h                   |  12 ++
 4 files changed, 172 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index 75088ecad2029..281b995beb05a 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -141,6 +141,14 @@ Description:
 		- "reverse": CC2 orientation
 		- "unknown": Orientation cannot be determined.
 
+What:		/sys/class/typec/<port>/select_usb_power_delivery
+Date:		May 2022
+Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
+Description:
+		Lists the USB Power Delivery Capabilities that the port can
+		advertise to the partner. The currently used capabilities are in
+		brackets. Selection happens by writing to the file.
+
 USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
 
 What:		/sys/class/typec/<port>-partner/accessory_mode
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index ee0e520707dd7..bd087389fc510 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.h"
 
 static DEFINE_IDA(typec_index_ida);
 
@@ -720,6 +721,38 @@ void typec_partner_set_pd_revision(struct typec_partner *partner, u16 pd_revisio
 }
 EXPORT_SYMBOL_GPL(typec_partner_set_pd_revision);
 
+/**
+ * typec_partner_set_pd - Declare USB Power Delivery Contract.
+ * @partner: The partner device.
+ * @pd: The USB PD instance.
+ *
+ * This routine can be used to declare USB Power Delivery Contract with @partner
+ * by linking @partner to @pd which contains the objects that were used during the
+ * negotiation of the contract.
+ *
+ * If @pd is NULL, the link is removed and the contract with @partner has ended.
+ */
+int typec_partner_set_pd(struct typec_partner *partner, struct pd *pd)
+{
+	int ret;
+
+	if (IS_ERR_OR_NULL(partner) || partner->pd == pd)
+		return 0;
+
+	if (pd) {
+		ret = pd_link_device(pd, &partner->dev);
+		if (ret)
+			return ret;
+	} else {
+		pd_unlink_device(partner->pd, &partner->dev);
+	}
+
+	partner->pd = pd;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(typec_partner_set_pd);
+
 /**
  * typec_partner_set_num_altmodes - Set the number of available partner altmodes
  * @partner: The partner to be updated.
@@ -1170,6 +1203,104 @@ EXPORT_SYMBOL_GPL(typec_unregister_cable);
 /* ------------------------------------------------------------------------- */
 /* USB Type-C ports */
 
+/**
+ * typec_port_set_pd - Assign USB PD for port.
+ * @port: USB Type-C port.
+ * @pd: USB PD instance.
+ *
+ * This routine can be used to set the USB Power Delivery Capabilities for @port
+ * that it will advertise to the partner.
+ *
+ * If @pd is NULL, the assignment is removed.
+ */
+int typec_port_set_pd(struct typec_port *port, struct pd *pd)
+{
+	int ret;
+
+	if (IS_ERR_OR_NULL(port) || port->pd == pd)
+		return 0;
+
+	if (pd) {
+		ret = pd_link_device(pd, &port->dev);
+		if (ret)
+			return ret;
+	} else {
+		pd_unlink_device(port->pd, &port->dev);
+	}
+
+	port->pd = pd;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(typec_port_set_pd);
+
+static ssize_t select_usb_power_delivery_store(struct device *dev,
+					       struct device_attribute *attr,
+					       const char *buf, size_t size)
+{
+	struct typec_port *port = to_typec_port(dev);
+	struct pd *pd;
+
+	if (!port->ops || !port->ops->pd_set)
+		return -EOPNOTSUPP;
+
+	pd = pd_find(buf);
+	if (!pd)
+		return -EINVAL;
+
+	return port->ops->pd_set(port, pd);
+}
+
+static ssize_t select_usb_power_delivery_show(struct device *dev,
+					      struct device_attribute *attr, char *buf)
+{
+	struct typec_port *port = to_typec_port(dev);
+	struct pd **pds;
+	struct pd *pd;
+	int ret = 0;
+
+	if (!port->ops || !port->ops->pd_get)
+		return -EOPNOTSUPP;
+
+	pds = port->ops->pd_get(port);
+	if (!pds)
+		return 0;
+
+	for (pd = pds[0]; pd; pd++) {
+		if (pd == port->pd)
+			ret += sysfs_emit(buf + ret, "[%s] ", dev_name(&pd->dev));
+		else
+			ret += sysfs_emit(buf + ret, "%s ", dev_name(&pd->dev));
+	}
+
+	buf[ret - 1] = '\n';
+
+	return ret;
+}
+static DEVICE_ATTR_RW(select_usb_power_delivery);
+
+static struct attribute *port_attrs[] = {
+	&dev_attr_select_usb_power_delivery.attr,
+	NULL
+};
+
+static umode_t port_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n)
+{
+	struct typec_port *port = to_typec_port(kobj_to_dev(kobj));
+
+	if (!port->pd || !port->ops || !port->ops->pd_get)
+		return 0;
+	if (!port->ops->pd_set)
+		return 0444;
+
+	return attr->mode;
+}
+
+static const struct attribute_group pd_group = {
+	.is_visible = port_attr_is_visible,
+	.attrs = port_attrs,
+};
+
 static const char * const typec_orientations[] = {
 	[TYPEC_ORIENTATION_NONE]	= "unknown",
 	[TYPEC_ORIENTATION_NORMAL]	= "normal",
@@ -1581,6 +1712,7 @@ static const struct attribute_group typec_group = {
 
 static const struct attribute_group *typec_groups[] = {
 	&typec_group,
+	&pd_group,
 	NULL
 };
 
@@ -2123,6 +2255,13 @@ struct typec_port *typec_register_port(struct device *parent,
 		return ERR_PTR(ret);
 	}
 
+	ret = typec_port_set_pd(port, cap->pd);
+	if (ret) {
+		dev_err(&port->dev, "failed to link pd\n");
+		device_unregister(&port->dev);
+		return ERR_PTR(ret);
+	}
+
 	ret = typec_link_ports(port);
 	if (ret)
 		dev_warn(&port->dev, "failed to create symlinks (%d)\n", ret);
@@ -2141,6 +2280,7 @@ void typec_unregister_port(struct typec_port *port)
 {
 	if (!IS_ERR_OR_NULL(port)) {
 		typec_unlink_ports(port);
+		typec_port_set_pd(port, NULL);
 		device_unregister(&port->dev);
 	}
 }
@@ -2162,8 +2302,15 @@ static int __init typec_init(void)
 	if (ret)
 		goto err_unregister_mux_class;
 
+	ret = pd_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);
 
@@ -2176,6 +2323,7 @@ subsys_initcall(typec_init);
 
 static void __exit typec_exit(void)
 {
+	pd_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 0f1bd6d19d67e..6c638fd8d85cb 100644
--- a/drivers/usb/typec/class.h
+++ b/drivers/usb/typec/class.h
@@ -33,6 +33,8 @@ struct typec_partner {
 	int				num_altmodes;
 	u16				pd_revision; /* 0300H = "3.0" */
 	enum usb_pd_svdm_ver		svdm_version;
+
+	struct pd			*pd;
 };
 
 struct typec_port {
@@ -40,6 +42,8 @@ struct typec_port {
 	struct device			dev;
 	struct ida			mode_ids;
 
+	struct pd			*pd;
+
 	int				prefer_role;
 	enum typec_data_role		data_role;
 	enum typec_role			pwr_role;
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 45e28d14ae56e..3855cd07925d3 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -22,6 +22,8 @@ struct typec_altmode_ops;
 struct fwnode_handle;
 struct device;
 
+struct pd;
+
 enum typec_port_type {
 	TYPEC_PORT_SRC,
 	TYPEC_PORT_SNK,
@@ -223,6 +225,8 @@ struct typec_partner_desc {
  * @pr_set: Set Power Role
  * @vconn_set: Source VCONN
  * @port_type_set: Set port type
+ * @pd_get: Get available USB Power Delivery Capabilities.
+ * @pd_set: Set USB Power Delivery Capabilities.
  */
 struct typec_operations {
 	int (*try_role)(struct typec_port *port, int role);
@@ -231,6 +235,8 @@ struct typec_operations {
 	int (*vconn_set)(struct typec_port *port, enum typec_role role);
 	int (*port_type_set)(struct typec_port *port,
 			     enum typec_port_type type);
+	struct pd **(*pd_get)(struct typec_port *port);
+	int (*pd_set)(struct typec_port *port, struct pd *pd);
 };
 
 enum usb_pd_svdm_ver {
@@ -250,6 +256,7 @@ enum usb_pd_svdm_ver {
  * @accessory: Supported Accessory Modes
  * @fwnode: Optional fwnode of the port
  * @driver_data: Private pointer for driver specific info
+ * @pd: Optional USB Power Delivery Support
  * @ops: Port operations vector
  *
  * Static capabilities of a single USB Type-C port.
@@ -267,6 +274,8 @@ struct typec_capability {
 	struct fwnode_handle	*fwnode;
 	void			*driver_data;
 
+	struct pd		*pd;
+
 	const struct typec_operations	*ops;
 };
 
@@ -318,4 +327,7 @@ void typec_partner_set_svdm_version(struct typec_partner *partner,
 				    enum usb_pd_svdm_ver svdm_version);
 int typec_get_negotiated_svdm_version(struct typec_port *port);
 
+int typec_port_set_pd(struct typec_port *port, struct pd *pd);
+int typec_partner_set_pd(struct typec_partner *partner, struct pd *pd);
+
 #endif /* __LINUX_USB_TYPEC_H */
-- 
2.35.1

[PATCH v3 3/3] usb: typec: tcpm: Register USB Power Delivery Capabilities

[Heikki Krogerus]

Register both the port and partner USB Power Delivery
Capabilities so they are exposed to the user space.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/tcpm/tcpm.c | 142 +++++++++++++++++++++++++++++++++-
 1 file changed, 141 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 3bc2f4ebd1feb..5785f174a3ce6 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -394,6 +394,14 @@ struct tcpm_port {
 	bool explicit_contract;
 	unsigned int rx_msgid;
 
+	/* USB PD objects */
+	struct pd *pd;
+	struct pd_capabilities *port_source_caps;
+	struct pd_capabilities *port_sink_caps;
+	struct pd *partner_pd;
+	struct pd_capabilities *partner_source_caps;
+	struct pd_capabilities *partner_sink_caps;
+
 	/* Partner capabilities/requests */
 	u32 sink_request;
 	u32 source_caps[PDO_MAX_OBJECTS];
@@ -2352,6 +2360,52 @@ static void tcpm_pd_handle_msg(struct tcpm_port *port,
 	}
 }
 
+static int tcpm_register_source_caps(struct tcpm_port *port)
+{
+	struct pd_desc desc = { port->negotiated_rev };
+	struct pd_caps_desc caps = { };
+	struct pd_capabilities *cap;
+
+	if (!port->partner_pd)
+		port->partner_pd = pd_register(NULL, &desc);
+	if (IS_ERR(port->partner_pd))
+		return PTR_ERR(port->partner_pd);
+
+	memcpy(caps.pdo, port->source_caps, sizeof(u32) * port->nr_source_caps);
+	caps.role = TYPEC_SOURCE;
+
+	cap = pd_register_capabilities(port->partner_pd, &caps);
+	if (IS_ERR(cap))
+		return PTR_ERR(cap);
+
+	port->partner_source_caps = cap;
+
+	return 0;
+}
+
+static int tcpm_register_sink_caps(struct tcpm_port *port)
+{
+	struct pd_desc desc = { port->negotiated_rev };
+	struct pd_caps_desc caps = { };
+	struct pd_capabilities *cap;
+
+	if (!port->partner_pd)
+		port->partner_pd = pd_register(NULL, &desc);
+	if (IS_ERR(port->partner_pd))
+		return PTR_ERR(port->partner_pd);
+
+	memcpy(caps.pdo, port->sink_caps, sizeof(u32) * port->nr_sink_caps);
+	caps.role = TYPEC_SINK;
+
+	cap = pd_register_capabilities(port->partner_pd, &caps);
+	if (IS_ERR(cap))
+		return PTR_ERR(cap);
+
+	port->partner_sink_caps = cap;
+
+	return 0;
+}
+
 static void tcpm_pd_data_request(struct tcpm_port *port,
 				 const struct pd_message *msg)
 {
@@ -2381,6 +2435,8 @@ static void tcpm_pd_data_request(struct tcpm_port *port,
 		tcpm_validate_caps(port, port->source_caps,
 				   port->nr_source_caps);
 
+		tcpm_register_source_caps(port);
+
 		/*
 		 * Adjust revision in subsequent message headers, as required,
 		 * to comply with 6.2.1.1.5 of the USB PD 3.0 spec. We don't
@@ -2488,6 +2544,8 @@ static void tcpm_pd_data_request(struct tcpm_port *port,
 
 		port->nr_sink_caps = cnt;
 		port->sink_cap_done = true;
+		tcpm_register_sink_caps(port);
+
 		if (port->ams == GET_SINK_CAPABILITIES)
 			tcpm_set_state(port, ready_state(port), 0);
 		/* Unexpected Sink Capabilities */
@@ -3554,6 +3612,7 @@ static void tcpm_typec_connect(struct tcpm_port *port)
 		port->partner = typec_register_partner(port->typec_port,
 						       &port->partner_desc);
 		port->connected = true;
+		typec_partner_set_pd(port->partner, port->partner_pd);
 	}
 }
 
@@ -3622,6 +3681,7 @@ static int tcpm_src_attach(struct tcpm_port *port)
 static void tcpm_typec_disconnect(struct tcpm_port *port)
 {
 	if (port->connected) {
+		typec_partner_set_pd(port->partner, NULL);
 		typec_unregister_partner(port->partner);
 		port->partner = NULL;
 		port->connected = false;
@@ -3684,6 +3744,13 @@ static void tcpm_reset_port(struct tcpm_port *port)
 	port->sink_cap_done = false;
 	if (port->tcpc->enable_frs)
 		port->tcpc->enable_frs(port->tcpc, false);
+
+	pd_unregister_capabilities(port->partner_sink_caps);
+	port->partner_sink_caps = NULL;
+	pd_unregister_capabilities(port->partner_source_caps);
+	port->partner_source_caps = NULL;
+	pd_unregister(port->partner_pd);
+	port->partner_pd = NULL;
 }
 
 static void tcpm_detach(struct tcpm_port *port)
@@ -5924,6 +5991,68 @@ void tcpm_tcpc_reset(struct tcpm_port *port)
 }
 EXPORT_SYMBOL_GPL(tcpm_tcpc_reset);
 
+static void tcpm_port_unregister_pd(struct tcpm_port *port)
+{
+	pd_unregister_capabilities(port->port_sink_caps);
+	port->port_sink_caps = NULL;
+	pd_unregister_capabilities(port->port_source_caps);
+	port->port_source_caps = NULL;
+	pd_unregister(port->pd);
+	port->pd = NULL;
+}
+
+static int tcpm_port_register_pd(struct tcpm_port *port)
+{
+	struct pd_desc desc = { port->typec_caps.pd_revision };
+	struct pd_caps_desc caps = { };
+	struct pd_capabilities *cap;
+	int ret;
+
+	if (!port->nr_src_pdo && !port->nr_snk_pdo)
+		return 0;
+
+	port->pd = pd_register(port->dev, &desc);
+	if (IS_ERR(port->pd)) {
+		ret = PTR_ERR(port->pd);
+		goto err_unregister;
+	}
+
+	if (port->nr_src_pdo) {
+		memcpy_and_pad(caps.pdo, sizeof(caps.pdo), port->src_pdo,
+			       port->nr_src_pdo * sizeof(u32), 0);
+		caps.role = TYPEC_SOURCE;
+
+		cap = pd_register_capabilities(port->pd, &caps);
+		if (IS_ERR(cap)) {
+			ret = PTR_ERR(cap);
+			goto err_unregister;
+		}
+
+		port->port_source_caps = cap;
+	}
+
+	if (port->nr_snk_pdo) {
+		memcpy_and_pad(caps.pdo, sizeof(caps.pdo), port->snk_pdo,
+			       port->nr_snk_pdo * sizeof(u32), 0);
+		caps.role = TYPEC_SINK;
+
+		cap = pd_register_capabilities(port->pd, &caps);
+		if (IS_ERR(cap)) {
+			ret = PTR_ERR(cap);
+			goto err_unregister;
+		}
+
+		port->port_sink_caps = cap;
+	}
+
+	return 0;
+
+err_unregister:
+	tcpm_port_unregister_pd(port);
+
+	return ret;
+}
+
 static int tcpm_fw_get_caps(struct tcpm_port *port,
 			    struct fwnode_handle *fwnode)
 {
@@ -6382,10 +6511,16 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 		goto out_role_sw_put;
 	power_supply_changed(port->psy);
 
+	err = tcpm_port_register_pd(port);
+	if (err)
+		goto out_role_sw_put;
+
+	port->typec_caps.pd = port->pd;
+
 	port->typec_port = typec_register_port(port->dev, &port->typec_caps);
 	if (IS_ERR(port->typec_port)) {
 		err = PTR_ERR(port->typec_port);
-		goto out_role_sw_put;
+		goto out_unregister_pd;
 	}
 
 	typec_port_register_altmodes(port->typec_port,
@@ -6400,6 +6535,8 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	tcpm_log(port, "%s: registered", dev_name(dev));
 	return port;
 
+out_unregister_pd:
+	tcpm_port_unregister_pd(port);
 out_role_sw_put:
 	usb_role_switch_put(port->role_sw);
 out_destroy_wq:
@@ -6422,6 +6559,9 @@ void tcpm_unregister_port(struct tcpm_port *port)
 	hrtimer_cancel(&port->state_machine_timer);
 
 	tcpm_reset_port(port);
+
+	tcpm_port_unregister_pd(port);
+
 	for (i = 0; i < ARRAY_SIZE(port->port_altmode); i++)
 		typec_unregister_altmode(port->port_altmode[i]);
 	typec_unregister_port(port->typec_port);
-- 
2.35.1

Re: [PATCH v3 1/3] usb: typec: Separate USB Power Delivery from USB Type-C

[Greg Kroah-Hartman]

On Mon, Apr 25, 2022 at 03:49:44PM +0300, Heikki Krogerus wrote:
> --- /dev/null
> +++ b/drivers/usb/typec/pd.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __USB_POWER_DELIVERY__
> +#define __USB_POWER_DELIVERY__
> +
> +#include <linux/kobject.h>

Why kobject.h when:

> +
> +struct pd_capabilities {
> +	struct device dev;

This is a device?

> +	struct pd *pd;
> +	enum typec_role role;
> +};
> +
> +struct pd {
> +	struct device		dev;
> +	int			id;
> +
> +	u16			revision; /* 0300H = "3.0" */

So BCD?

> +	u16			version;
> +};



> +
> +#define to_pd_capabilities(o) container_of(o, struct pd_capabilities, dev)
> +#define to_pd(o) container_of(o, struct pd, dev)
> +
> +struct pd *pd_find(const char *name);

"struct pd" is just about the shortest structure name I've seen in the
kernel so far.  How about using some more letters?  :)

> +
> +int pd_init(void);
> +void pd_exit(void);

The kobject question above goes to the code as well.  You are creating a
bunch of raw kobjects still, why?  This should all fit into the driver
model and kobjects shouldn't be needed.  Are you trying to nest too deep
in the attributes?  If so, kobjects will not work as userspace tools
will not realize they are there and are attributes at all.

Try running some tests using libudev and see the lack of attributes with
this patch, tools are not going to be able to see them at all this way.

thanks,

greg k-h

Re: [PATCH v3 1/3] usb: typec: Separate USB Power Delivery from USB Type-C

[Heikki Krogerus]

Hi,

On Tue, Apr 26, 2022 at 02:07:25PM +0200, Greg Kroah-Hartman wrote:
> On Mon, Apr 25, 2022 at 03:49:44PM +0300, Heikki Krogerus wrote:
> > --- /dev/null
> > +++ b/drivers/usb/typec/pd.h
> > @@ -0,0 +1,30 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#ifndef __USB_POWER_DELIVERY__
> > +#define __USB_POWER_DELIVERY__
> > +
> > +#include <linux/kobject.h>
> 
> Why kobject.h when:

Oops, that should now be "#include <linux/device.h>".

> > +
> > +struct pd_capabilities {
> > +	struct device dev;
> 
> This is a device?
> 
> > +	struct pd *pd;
> > +	enum typec_role role;
> > +};
> > +
> > +struct pd {
> > +	struct device		dev;
> > +	int			id;
> > +
> > +	u16			revision; /* 0300H = "3.0" */
> 
> So BCD?

Yes.

> > +	u16			version;
> > +};
> 
> > +
> > +#define to_pd_capabilities(o) container_of(o, struct pd_capabilities, dev)
> > +#define to_pd(o) container_of(o, struct pd, dev)
> > +
> > +struct pd *pd_find(const char *name);
> 
> "struct pd" is just about the shortest structure name I've seen in the
> kernel so far.  How about using some more letters?  :)

Okay, I'll make it usbpd.

> > +
> > +int pd_init(void);
> > +void pd_exit(void);
> 
> The kobject question above goes to the code as well.  You are creating a
> bunch of raw kobjects still, why?  This should all fit into the driver
> model and kobjects shouldn't be needed.  Are you trying to nest too deep
> in the attributes?  If so, kobjects will not work as userspace tools
> will not realize they are there and are attributes at all.

They are not raw kobjects, they are all devices now. That header just
needs to be fixed.

Br,

-- 
heikki

Re: [PATCH v3 1/3] usb: typec: Separate USB Power Delivery from USB Type-C

[Greg Kroah-Hartman]

On Wed, Apr 27, 2022 at 09:42:19AM +0300, Heikki Krogerus wrote:
> Hi,
> 
> On Tue, Apr 26, 2022 at 02:07:25PM +0200, Greg Kroah-Hartman wrote:
> > On Mon, Apr 25, 2022 at 03:49:44PM +0300, Heikki Krogerus wrote:
> > > --- /dev/null
> > > +++ b/drivers/usb/typec/pd.h
> > > @@ -0,0 +1,30 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +
> > > +#ifndef __USB_POWER_DELIVERY__
> > > +#define __USB_POWER_DELIVERY__
> > > +
> > > +#include <linux/kobject.h>
> > 
> > Why kobject.h when:
> 
> Oops, that should now be "#include <linux/device.h>".
> 
> > > +
> > > +struct pd_capabilities {
> > > +	struct device dev;
> > 
> > This is a device?
> > 
> > > +	struct pd *pd;
> > > +	enum typec_role role;
> > > +};
> > > +
> > > +struct pd {
> > > +	struct device		dev;
> > > +	int			id;
> > > +
> > > +	u16			revision; /* 0300H = "3.0" */
> > 
> > So BCD?
> 
> Yes.
> 
> > > +	u16			version;
> > > +};
> > 
> > > +
> > > +#define to_pd_capabilities(o) container_of(o, struct pd_capabilities, dev)
> > > +#define to_pd(o) container_of(o, struct pd, dev)
> > > +
> > > +struct pd *pd_find(const char *name);
> > 
> > "struct pd" is just about the shortest structure name I've seen in the
> > kernel so far.  How about using some more letters?  :)
> 
> Okay, I'll make it usbpd.

How about some more vowels: "struct usb_power_delivery" please.  This
isn't the 1980's :)

> > > +
> > > +int pd_init(void);
> > > +void pd_exit(void);
> > 
> > The kobject question above goes to the code as well.  You are creating a
> > bunch of raw kobjects still, why?  This should all fit into the driver
> > model and kobjects shouldn't be needed.  Are you trying to nest too deep
> > in the attributes?  If so, kobjects will not work as userspace tools
> > will not realize they are there and are attributes at all.
> 
> They are not raw kobjects, they are all devices now. That header just
> needs to be fixed.

You have loads of kobject attributes in the .c file.  Either I read it
wrong, or you are doing something wrong, as that should never be the
case for a driver or device.

thanks,

greg k-h

Re: [PATCH v3 1/3] usb: typec: Separate USB Power Delivery from USB Type-C

[Heikki Krogerus]

On Wed, Apr 27, 2022 at 09:30:30AM +0200, Greg Kroah-Hartman wrote:
> > > "struct pd" is just about the shortest structure name I've seen in the
> > > kernel so far.  How about using some more letters?  :)
> > 
> > Okay, I'll make it usbpd.
> 
> How about some more vowels: "struct usb_power_delivery" please.  This
> isn't the 1980's :)

"struct usb_power_delivery" is fine, but I would still really really
want to use "struct usbpd_capabilities" instead of
"struct usb_power_delivery_capabilities" - it's just too long.
Is that okay?

> > > > +
> > > > +int pd_init(void);
> > > > +void pd_exit(void);
> > > 
> > > The kobject question above goes to the code as well.  You are creating a
> > > bunch of raw kobjects still, why?  This should all fit into the driver
> > > model and kobjects shouldn't be needed.  Are you trying to nest too deep
> > > in the attributes?  If so, kobjects will not work as userspace tools
> > > will not realize they are there and are attributes at all.
> > 
> > They are not raw kobjects, they are all devices now. That header just
> > needs to be fixed.
> 
> You have loads of kobject attributes in the .c file.  Either I read it
> wrong, or you are doing something wrong, as that should never be the
> case for a driver or device.

Hmm, I'm probable still doing something wrong...

thanks,

-- 
heikki

Re: [PATCH v3 1/3] usb: typec: Separate USB Power Delivery from USB Type-C

[Greg Kroah-Hartman]

On Wed, Apr 27, 2022 at 11:17:49AM +0300, Heikki Krogerus wrote:
> On Wed, Apr 27, 2022 at 09:30:30AM +0200, Greg Kroah-Hartman wrote:
> > > > "struct pd" is just about the shortest structure name I've seen in the
> > > > kernel so far.  How about using some more letters?  :)
> > > 
> > > Okay, I'll make it usbpd.
> > 
> > How about some more vowels: "struct usb_power_delivery" please.  This
> > isn't the 1980's :)
> 
> "struct usb_power_delivery" is fine, but I would still really really
> want to use "struct usbpd_capabilities" instead of
> "struct usb_power_delivery_capabilities" - it's just too long.
> Is that okay?

Nah, spell it out please, we don't use "usbpd" anywhere, and again, we
don't have a limit of characters.  Most editors should auto-complete
anyway :)

> > > > The kobject question above goes to the code as well.  You are creating a
> > > > bunch of raw kobjects still, why?  This should all fit into the driver
> > > > model and kobjects shouldn't be needed.  Are you trying to nest too deep
> > > > in the attributes?  If so, kobjects will not work as userspace tools
> > > > will not realize they are there and are attributes at all.
> > > 
> > > They are not raw kobjects, they are all devices now. That header just
> > > needs to be fixed.
> > 
> > You have loads of kobject attributes in the .c file.  Either I read it
> > wrong, or you are doing something wrong, as that should never be the
> > case for a driver or device.
> 
> Hmm, I'm probable still doing something wrong...

All of your sysfs callbacks should have a struct device, not a kobject.
You might just be getting lucky in that we are casting around a pointer
of the correct layout.  But the compiler should have caught that
somewhere, please look into it.

thanks,

greg k-h

Re: [PATCH v3 1/3] usb: typec: Separate USB Power Delivery from USB Type-C

[Heikki Krogerus]

On Wed, Apr 27, 2022 at 10:50:35AM +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 27, 2022 at 11:17:49AM +0300, Heikki Krogerus wrote:
> > On Wed, Apr 27, 2022 at 09:30:30AM +0200, Greg Kroah-Hartman wrote:
> > > > > "struct pd" is just about the shortest structure name I've seen in the
> > > > > kernel so far.  How about using some more letters?  :)
> > > > 
> > > > Okay, I'll make it usbpd.
> > > 
> > > How about some more vowels: "struct usb_power_delivery" please.  This
> > > isn't the 1980's :)
> > 
> > "struct usb_power_delivery" is fine, but I would still really really
> > want to use "struct usbpd_capabilities" instead of
> > "struct usb_power_delivery_capabilities" - it's just too long.
> > Is that okay?
> 
> Nah, spell it out please, we don't use "usbpd" anywhere, and again, we
> don't have a limit of characters.  Most editors should auto-complete
> anyway :)

Very well.

> > > > > The kobject question above goes to the code as well.  You are creating a
> > > > > bunch of raw kobjects still, why?  This should all fit into the driver
> > > > > model and kobjects shouldn't be needed.  Are you trying to nest too deep
> > > > > in the attributes?  If so, kobjects will not work as userspace tools
> > > > > will not realize they are there and are attributes at all.
> > > > 
> > > > They are not raw kobjects, they are all devices now. That header just
> > > > needs to be fixed.
> > > 
> > > You have loads of kobject attributes in the .c file.  Either I read it
> > > wrong, or you are doing something wrong, as that should never be the
> > > case for a driver or device.
> > 
> > Hmm, I'm probable still doing something wrong...
> 
> All of your sysfs callbacks should have a struct device, not a kobject.
> You might just be getting lucky in that we are casting around a pointer
> of the correct layout.  But the compiler should have caught that
> somewhere, please look into it.

I'll fix this.

thanks,

-- 
heikki