All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support
@ 2021-05-19 14:12 Mika Westerberg
  2021-05-19 14:12 ` [PATCH 1/9] thunderbolt: Log the link as TBT instead of TBT3 Mika Westerberg
                   ` (9 more replies)
  0 siblings, 10 replies; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 14:12 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Rafael J. Wysocki, linux-acpi, Casey G Bowman, Rajmohan Mani,
	Christian Kellner, Greg Kroah-Hartman, Jonathan Corbet,
	Mika Westerberg

Hi all,

USB4 retimers are only accessible when the USB4 is up. However, sometimes
it may be useful to be able to upgrade on-board retimers even if the link
is not up. For instance if the user simply does not have any USB4 devices.

Making retimers accessible in "offline" mode requires some help from the
platform firmware (ACPI in our case) to turn on power to the retimers and
cycle them through different modes to get the sideband link up. This may
also involve other firmwares such as Embedded Controller (as it is the case
with recent Chromebooks).

This series adds support for "offline" retimer NVM upgrade so that it first
exposes each USB4 port to the userspace. If the platform firmware provides
a special _DSM-method (Device Specific Method) under the USB4 port ACPI
description, we expose two attributes under the port that the userspace can
use to put the port to offline mode and rescan for the retimers. Otherwise
the NVM upgrade works the same way than with the online mode. We also add
documentation to the admin-guide how this can be done.

In addition to this, at least Intel USB4 devices (and retimers) allow
running NVM authenticate (upgrade) separately from write so we make it
possible for the userspace to run the write and authenticate in two steps.
This allows userspace to trigger the authentication at later time, like
when the user logs out.

Mika Westerberg (4):
  thunderbolt: Log the link as TBT instead of TBT3
  thunderbolt: Add USB4 port devices
  thunderbolt: Allow router NVM authenticate separately
  thunderbolt: Check for NVM authentication status after the operation started

Rajmohan Mani (5):
  thunderbolt: Add support for ACPI _DSM to power on/off retimers
  thunderbolt: Add additional USB4 port operations for retimer access
  thunderbolt: Add support for retimer NVM upgrade when there is no link
  thunderbolt: Move nvm_write_ops to tb.h
  thunderbolt: Add WRITE_ONLY and AUTHENTICATE_ONLY NVM operations for retimers

 .../ABI/testing/sysfs-bus-thunderbolt         |  38 ++-
 Documentation/admin-guide/thunderbolt.rst     |  29 ++
 drivers/thunderbolt/Makefile                  |   2 +-
 drivers/thunderbolt/acpi.c                    | 206 +++++++++++++
 drivers/thunderbolt/domain.c                  |   9 +-
 drivers/thunderbolt/retimer.c                 | 108 +++++--
 drivers/thunderbolt/sb_regs.h                 |   2 +
 drivers/thunderbolt/switch.c                  | 120 +++++---
 drivers/thunderbolt/tb.c                      |   4 +-
 drivers/thunderbolt/tb.h                      |  60 +++-
 drivers/thunderbolt/usb4.c                    | 153 +++++++++-
 drivers/thunderbolt/usb4_port.c               | 281 ++++++++++++++++++
 12 files changed, 930 insertions(+), 82 deletions(-)
 create mode 100644 drivers/thunderbolt/usb4_port.c

-- 
2.30.2


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

* [PATCH 1/9] thunderbolt: Log the link as TBT instead of TBT3
  2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
@ 2021-05-19 14:12 ` Mika Westerberg
  2021-05-19 14:12 ` [PATCH 2/9] thunderbolt: Add USB4 port devices Mika Westerberg
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 14:12 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Rafael J. Wysocki, linux-acpi, Casey G Bowman, Rajmohan Mani,
	Christian Kellner, Greg Kroah-Hartman, Jonathan Corbet,
	Mika Westerberg

The upstream port can be connected to any previous generation
Thunderbolt port so logging as "TBT" is more accurate than "TBT3.

No functional changes.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/usb4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/usb4.c b/drivers/thunderbolt/usb4.c
index edab8ea63c0b..94cc25cc6388 100644
--- a/drivers/thunderbolt/usb4.c
+++ b/drivers/thunderbolt/usb4.c
@@ -260,7 +260,7 @@ int usb4_switch_setup(struct tb_switch *sw)
 	parent = tb_switch_parent(sw);
 	downstream_port = tb_port_at(tb_route(sw), parent);
 	sw->link_usb4 = link_is_usb4(downstream_port);
-	tb_sw_dbg(sw, "link: %s\n", sw->link_usb4 ? "USB4" : "TBT3");
+	tb_sw_dbg(sw, "link: %s\n", sw->link_usb4 ? "USB4" : "TBT");
 
 	xhci = val & ROUTER_CS_6_HCI;
 	tbt3 = !(val & ROUTER_CS_6_TNS);
-- 
2.30.2


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

* [PATCH 2/9] thunderbolt: Add USB4 port devices
  2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
  2021-05-19 14:12 ` [PATCH 1/9] thunderbolt: Log the link as TBT instead of TBT3 Mika Westerberg
@ 2021-05-19 14:12 ` Mika Westerberg
  2021-05-19 15:14   ` Heikki Krogerus
  2021-05-19 14:12 ` [PATCH 3/9] thunderbolt: Add support for ACPI _DSM to power on/off retimers Mika Westerberg
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 14:12 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Rafael J. Wysocki, linux-acpi, Casey G Bowman, Rajmohan Mani,
	Christian Kellner, Greg Kroah-Hartman, Jonathan Corbet,
	Mika Westerberg

Create devices for each USB4 port. This is needed when we add retimer
access when there is no device connected but may be useful for other
purposes too following what USB subsystem does. This exports a single
attribute "link" that shows the type of the USB4 link (or "none" if
there is no cable connected).

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 .../ABI/testing/sysfs-bus-thunderbolt         |   7 ++
 drivers/thunderbolt/Makefile                  |   2 +-
 drivers/thunderbolt/retimer.c                 |  15 ++-
 drivers/thunderbolt/switch.c                  |  17 ++-
 drivers/thunderbolt/tb.h                      |  30 +++++
 drivers/thunderbolt/usb4.c                    |  54 +++++++++
 drivers/thunderbolt/usb4_port.c               | 112 ++++++++++++++++++
 7 files changed, 229 insertions(+), 8 deletions(-)
 create mode 100644 drivers/thunderbolt/usb4_port.c

diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
index 05afeee05538..3537ba1ba892 100644
--- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
+++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
@@ -290,6 +290,13 @@ Contact:	thunderbolt-software@lists.01.org
 Description:	This contains XDomain service specific settings as
 		bitmask. Format: %x
 
+What:		/sys/bus/thunderbolt/devices/usb4_portX/link
+Date:		Sep 2021
+KernelVersion:	v5.14
+Contact:	Mika Westerberg <mika.westerberg@linux.intel.com>
+Description:	Returns the current link mode. Possible values are
+		"usb4", "tbt" and "none".
+
 What:		/sys/bus/thunderbolt/devices/<device>:<port>.<index>/device
 Date:		Oct 2020
 KernelVersion:	v5.9
diff --git a/drivers/thunderbolt/Makefile b/drivers/thunderbolt/Makefile
index 7aa48f6c41d9..da19d7987d00 100644
--- a/drivers/thunderbolt/Makefile
+++ b/drivers/thunderbolt/Makefile
@@ -2,7 +2,7 @@
 obj-${CONFIG_USB4} := thunderbolt.o
 thunderbolt-objs := nhi.o nhi_ops.o ctl.o tb.o switch.o cap.o path.o tunnel.o eeprom.o
 thunderbolt-objs += domain.o dma_port.o icm.o property.o xdomain.o lc.o tmu.o usb4.o
-thunderbolt-objs += nvm.o retimer.o quirks.o
+thunderbolt-objs += usb4_port.o nvm.o retimer.o quirks.o
 
 thunderbolt-${CONFIG_ACPI} += acpi.o
 thunderbolt-$(CONFIG_DEBUG_FS) += debugfs.o
diff --git a/drivers/thunderbolt/retimer.c b/drivers/thunderbolt/retimer.c
index c44fad2b9fbb..2e5188fb1150 100644
--- a/drivers/thunderbolt/retimer.c
+++ b/drivers/thunderbolt/retimer.c
@@ -283,11 +283,13 @@ struct device_type tb_retimer_type = {
 
 static int tb_retimer_add(struct tb_port *port, u8 index, u32 auth_status)
 {
+	struct usb4_port *usb4;
 	struct tb_retimer *rt;
 	u32 vendor, device;
 	int ret;
 
-	if (!port->cap_usb4)
+	usb4 = port->usb4;
+	if (!usb4)
 		return -EINVAL;
 
 	ret = usb4_port_retimer_read(port, index, USB4_SB_VENDOR_ID, &vendor,
@@ -331,7 +333,7 @@ static int tb_retimer_add(struct tb_port *port, u8 index, u32 auth_status)
 	rt->port = port;
 	rt->tb = port->sw->tb;
 
-	rt->dev.parent = &port->sw->dev;
+	rt->dev.parent = &usb4->dev;
 	rt->dev.bus = &tb_bus_type;
 	rt->dev.type = &tb_retimer_type;
 	dev_set_name(&rt->dev, "%s:%u.%u", dev_name(&port->sw->dev),
@@ -389,7 +391,7 @@ static struct tb_retimer *tb_port_find_retimer(struct tb_port *port, u8 index)
 	struct tb_retimer_lookup lookup = { .port = port, .index = index };
 	struct device *dev;
 
-	dev = device_find_child(&port->sw->dev, &lookup, retimer_match);
+	dev = device_find_child(&port->usb4->dev, &lookup, retimer_match);
 	if (dev)
 		return tb_to_retimer(dev);
 
@@ -479,7 +481,10 @@ static int remove_retimer(struct device *dev, void *data)
  */
 void tb_retimer_remove_all(struct tb_port *port)
 {
-	if (port->cap_usb4)
-		device_for_each_child_reverse(&port->sw->dev, port,
+	struct usb4_port *usb4;
+
+	usb4 = port->usb4;
+	if (usb4)
+		device_for_each_child_reverse(&usb4->dev, port,
 					      remove_retimer);
 }
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index e015dc93a916..7303c61a891a 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -2741,11 +2741,16 @@ int tb_switch_add(struct tb_switch *sw)
 				 sw->device_name);
 	}
 
+	ret = usb4_switch_add_ports(sw);
+	if (ret) {
+		dev_err(&sw->dev, "failed to add USB4 ports\n");
+		goto err_del;
+	}
+
 	ret = tb_switch_nvm_add(sw);
 	if (ret) {
 		dev_err(&sw->dev, "failed to add NVM devices\n");
-		device_del(&sw->dev);
-		return ret;
+		goto err_ports;
 	}
 
 	/*
@@ -2766,6 +2771,13 @@ int tb_switch_add(struct tb_switch *sw)
 
 	tb_switch_debugfs_init(sw);
 	return 0;
+
+err_ports:
+	usb4_switch_remove_ports(sw);
+err_del:
+	device_del(&sw->dev);
+
+	return ret;
 }
 
 /**
@@ -2805,6 +2817,7 @@ void tb_switch_remove(struct tb_switch *sw)
 		tb_plug_events_active(sw, false);
 
 	tb_switch_nvm_remove(sw);
+	usb4_switch_remove_ports(sw);
 
 	if (tb_route(sw))
 		dev_info(&sw->dev, "device disconnected\n");
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 89e38aeea52b..948e7601428f 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -202,6 +202,7 @@ struct tb_switch {
  * @cap_tmu: Offset of the adapter specific TMU capability (%0 if not present)
  * @cap_adap: Offset of the adapter specific capability (%0 if not present)
  * @cap_usb4: Offset to the USB4 port capability (%0 if not present)
+ * @usb4: Pointer to the USB4 port structure (only if @cap_usb4 is != %0)
  * @port: Port number on switch
  * @disabled: Disabled by eeprom or enabled but not implemented
  * @bonded: true if the port is bonded (two lanes combined as one)
@@ -228,6 +229,7 @@ struct tb_port {
 	int cap_tmu;
 	int cap_adap;
 	int cap_usb4;
+	struct usb4_port *usb4;
 	u8 port;
 	bool disabled;
 	bool bonded;
@@ -241,6 +243,16 @@ struct tb_port {
 	unsigned int dma_credits;
 };
 
+/**
+ * struct usb4_port - USB4 port device
+ * @dev: Device for the port
+ * @port: Pointer to the lane 0 adapter
+ */
+struct usb4_port {
+	struct device dev;
+	struct tb_port *port;
+};
+
 /**
  * tb_retimer: Thunderbolt retimer
  * @dev: Device for the retimer
@@ -645,6 +657,7 @@ struct tb *tb_probe(struct tb_nhi *nhi);
 extern struct device_type tb_domain_type;
 extern struct device_type tb_retimer_type;
 extern struct device_type tb_switch_type;
+extern struct device_type usb4_port_device_type;
 
 int tb_domain_init(void);
 void tb_domain_exit(void);
@@ -1038,6 +1051,8 @@ struct tb_port *usb4_switch_map_pcie_down(struct tb_switch *sw,
 					  const struct tb_port *port);
 struct tb_port *usb4_switch_map_usb3_down(struct tb_switch *sw,
 					  const struct tb_port *port);
+int usb4_switch_add_ports(struct tb_switch *sw);
+void usb4_switch_remove_ports(struct tb_switch *sw);
 
 int usb4_port_unlock(struct tb_port *port);
 int usb4_port_configure(struct tb_port *port);
@@ -1070,6 +1085,21 @@ int usb4_usb3_port_allocate_bandwidth(struct tb_port *port, int *upstream_bw,
 int usb4_usb3_port_release_bandwidth(struct tb_port *port, int *upstream_bw,
 				     int *downstream_bw);
 
+static inline bool tb_is_usb4_port_device(const struct device *dev)
+{
+	return dev->type == &usb4_port_device_type;
+}
+
+static inline struct usb4_port *tb_to_usb4_port_device(struct device *dev)
+{
+	if (tb_is_usb4_port_device(dev))
+		return container_of(dev, struct usb4_port, dev);
+	return NULL;
+}
+
+struct usb4_port *usb4_port_device_add(struct tb_port *port);
+void usb4_port_device_remove(struct usb4_port *usb4);
+
 /* Keep link controller awake during update */
 #define QUIRK_FORCE_POWER_LINK_CONTROLLER		BIT(0)
 
diff --git a/drivers/thunderbolt/usb4.c b/drivers/thunderbolt/usb4.c
index 94cc25cc6388..1f82af35328e 100644
--- a/drivers/thunderbolt/usb4.c
+++ b/drivers/thunderbolt/usb4.c
@@ -985,6 +985,60 @@ struct tb_port *usb4_switch_map_usb3_down(struct tb_switch *sw,
 	return NULL;
 }
 
+/**
+ * usb4_switch_add_ports() - Add USB4 ports for this router
+ * @sw: USB4 router
+ *
+ * For USB4 router finds all USB4 ports and registers devices for each.
+ * Can be called to any router.
+ *
+ * Return %0 in case of success and negative errno in case of failure.
+ */
+int usb4_switch_add_ports(struct tb_switch *sw)
+{
+	struct tb_port *port;
+
+	if (tb_switch_is_icm(sw) || !tb_switch_is_usb4(sw))
+		return 0;
+
+	tb_switch_for_each_port(sw, port) {
+		struct usb4_port *usb4;
+
+		if (!tb_port_is_null(port))
+			continue;
+		if (!port->cap_usb4)
+			continue;
+
+		usb4 = usb4_port_device_add(port);
+		if (IS_ERR(usb4)) {
+			usb4_switch_remove_ports(sw);
+			return PTR_ERR(usb4);
+		}
+
+		port->usb4 = usb4;
+	}
+
+	return 0;
+}
+
+/**
+ * usb4_switch_remove_ports() - Removes USB4 ports from this router
+ * @sw: USB4 router
+ *
+ * Unregisters previously registered USB4 ports.
+ */
+void usb4_switch_remove_ports(struct tb_switch *sw)
+{
+	struct tb_port *port;
+
+	tb_switch_for_each_port(sw, port) {
+		if (port->usb4) {
+			usb4_port_device_remove(port->usb4);
+			port->usb4 = NULL;
+		}
+	}
+}
+
 /**
  * usb4_port_unlock() - Unlock USB4 downstream port
  * @port: USB4 port to unlock
diff --git a/drivers/thunderbolt/usb4_port.c b/drivers/thunderbolt/usb4_port.c
new file mode 100644
index 000000000000..520bbfd7bf33
--- /dev/null
+++ b/drivers/thunderbolt/usb4_port.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * USB4 port device
+ *
+ * Copyright (C) 2021, Intel Corporation
+ * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
+ */
+
+#include <linux/acpi.h>
+#include <linux/pm_runtime.h>
+
+#include "tb.h"
+
+static ssize_t link_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
+{
+	struct usb4_port *usb4 = tb_to_usb4_port_device(dev);
+	struct tb_port *port = usb4->port;
+	struct tb *tb = port->sw->tb;
+	const char *link;
+
+	if (mutex_lock_interruptible(&tb->lock))
+		return -ERESTARTSYS;
+
+	if (tb_is_upstream_port(port))
+		link = port->sw->link_usb4 ? "usb4" : "tbt";
+	else if (tb_port_has_remote(port))
+		link = port->remote->sw->link_usb4 ? "usb4" : "tbt";
+	else
+		link = "none";
+
+	mutex_unlock(&tb->lock);
+
+	return sysfs_emit(buf, "%s\n", link);
+}
+static DEVICE_ATTR_RO(link);
+
+static struct attribute *common_attrs[] = {
+	&dev_attr_link.attr,
+	NULL
+};
+
+static const struct attribute_group common_group = {
+	.attrs = common_attrs,
+};
+
+static const struct attribute_group *usb4_port_device_groups[] = {
+	&common_group,
+	NULL
+};
+
+static void usb4_port_device_release(struct device *dev)
+{
+	struct usb4_port *usb4 = container_of(dev, struct usb4_port, dev);
+
+	kfree(usb4);
+}
+
+struct device_type usb4_port_device_type = {
+	.name = "usb4_port",
+	.groups = usb4_port_device_groups,
+	.release = usb4_port_device_release,
+};
+
+/**
+ * usb4_port_device_add() - Add USB4 port device
+ * @port: Lane 0 adapter port to add the USB4 port
+ *
+ * Creates and registers a USB4 port device for @port. Returns the new
+ * USB4 port device pointer or ERR_PTR() in case of error.
+ */
+struct usb4_port *usb4_port_device_add(struct tb_port *port)
+{
+	struct usb4_port *usb4;
+	int ret;
+
+	usb4 = kzalloc(sizeof(*usb4), GFP_KERNEL);
+	if (!usb4)
+		return ERR_PTR(-ENOMEM);
+
+	usb4->port = port;
+	usb4->dev.type = &usb4_port_device_type;
+	usb4->dev.parent = &port->sw->dev;
+	dev_set_name(&usb4->dev, "usb4_port%d", port->port);
+
+	ret = device_register(&usb4->dev);
+	if (ret) {
+		put_device(&usb4->dev);
+		return ERR_PTR(ret);
+	}
+
+	pm_runtime_no_callbacks(&usb4->dev);
+	pm_runtime_set_active(&usb4->dev);
+	pm_runtime_enable(&usb4->dev);
+	pm_runtime_set_autosuspend_delay(&usb4->dev, TB_AUTOSUSPEND_DELAY);
+	pm_runtime_mark_last_busy(&usb4->dev);
+	pm_runtime_use_autosuspend(&usb4->dev);
+
+	return usb4;
+}
+
+/**
+ * usb4_port_device_remove() - Removes USB4 port device
+ * @usb4: USB4 port device
+ *
+ * Unregisters the USB4 port device from the system. The device will be
+ * released when the last reference is dropped.
+ */
+void usb4_port_device_remove(struct usb4_port *usb4)
+{
+	device_unregister(&usb4->dev);
+}
-- 
2.30.2


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

* [PATCH 3/9] thunderbolt: Add support for ACPI _DSM to power on/off retimers
  2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
  2021-05-19 14:12 ` [PATCH 1/9] thunderbolt: Log the link as TBT instead of TBT3 Mika Westerberg
  2021-05-19 14:12 ` [PATCH 2/9] thunderbolt: Add USB4 port devices Mika Westerberg
@ 2021-05-19 14:12 ` Mika Westerberg
  2021-05-19 14:12 ` [PATCH 4/9] thunderbolt: Add additional USB4 port operations for retimer access Mika Westerberg
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 14:12 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Rafael J. Wysocki, linux-acpi, Casey G Bowman, Rajmohan Mani,
	Christian Kellner, Greg Kroah-Hartman, Jonathan Corbet,
	Mika Westerberg

From: Rajmohan Mani <rajmohan.mani@intel.com>

Typically retimers can be accessed only when the USB4 link is up (e.g
there is a cable connected). However, sometimes it is useful to be able
to access retimers even if there is nothing connected to the USB4 port.
For instance we may still want to be able to upgrade the retimer NVM
firmware even if the user does not have any USB4 devices. This is
something that USB4 spec leaves to implementers.

In case of ACPI based systems, we can support this by providing a
special _DSM method under each USB4 port. This _DSM can be used to turn
on power to on-board retimers (and cycle it through different modes so
that the sideband becomes usable).

This patch adds support for this _DSM and makes the functionality
available to the rest of the driver through tb_acpi_power_[on|off]_retimers().

Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
Co-developed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/acpi.c   | 206 +++++++++++++++++++++++++++++++++++
 drivers/thunderbolt/domain.c |   9 +-
 drivers/thunderbolt/tb.h     |  13 +++
 3 files changed, 225 insertions(+), 3 deletions(-)

diff --git a/drivers/thunderbolt/acpi.c b/drivers/thunderbolt/acpi.c
index 35fa17f7e599..b67e72d5644b 100644
--- a/drivers/thunderbolt/acpi.c
+++ b/drivers/thunderbolt/acpi.c
@@ -180,3 +180,209 @@ bool tb_acpi_is_xdomain_allowed(void)
 		return osc_sb_native_usb4_control & OSC_USB_XDOMAIN;
 	return true;
 }
+
+/* UUID for retimer _DSM: e0053122-795b-4122-8a5e-57be1d26acb3 */
+static const guid_t retimer_dsm_guid =
+	GUID_INIT(0xe0053122, 0x795b, 0x4122,
+		  0x8a, 0x5e, 0x57, 0xbe, 0x1d, 0x26, 0xac, 0xb3);
+
+#define RETIMER_DSM_QUERY_ONLINE_STATE	1
+#define RETIMER_DSM_SET_ONLINE_STATE	2
+
+static int tb_acpi_retimer_set_power(struct tb_port *port, bool power)
+{
+	struct usb4_port *usb4 = port->usb4;
+	union acpi_object argv4[2];
+	struct acpi_device *adev;
+	union acpi_object *obj;
+	int ret;
+
+	if (!usb4->can_offline)
+		return 0;
+
+	adev = ACPI_COMPANION(&usb4->dev);
+	if (WARN_ON(!adev))
+		return 0;
+
+	/* Check if we are already powered on (and in correct mode) */
+	obj = acpi_evaluate_dsm_typed(adev->handle, &retimer_dsm_guid, 1,
+				      RETIMER_DSM_QUERY_ONLINE_STATE, NULL,
+				      ACPI_TYPE_INTEGER);
+	if (!obj) {
+		tb_port_warn(port, "ACPI: query online _DSM failed\n");
+		return -EIO;
+	}
+
+	ret = obj->integer.value;
+	ACPI_FREE(obj);
+
+	if (power == ret)
+		return 0;
+
+	tb_port_dbg(port, "ACPI: calling _DSM to power %s retimers\n",
+		    power ? "on" : "off");
+
+	argv4[0].type = ACPI_TYPE_PACKAGE;
+	argv4[0].package.count = 1;
+	argv4[0].package.elements = &argv4[1];
+	argv4[1].integer.type = ACPI_TYPE_INTEGER;
+	argv4[1].integer.value = power;
+
+	obj = acpi_evaluate_dsm_typed(adev->handle, &retimer_dsm_guid, 1,
+				      RETIMER_DSM_SET_ONLINE_STATE, argv4,
+				      ACPI_TYPE_INTEGER);
+	if (!obj) {
+		tb_port_warn(port,
+			     "ACPI: set online state _DSM evaluation failed\n");
+		return -EIO;
+	}
+
+	ret = obj->integer.value;
+	ACPI_FREE(obj);
+
+	if (ret >= 0) {
+		if (power)
+			return ret == 1 ? 0 : -EBUSY;
+		return 0;
+	}
+
+	tb_port_warn(port, "ACPI: set online state _DSM failed with error %d\n", ret);
+	return -EIO;
+}
+
+/**
+ * tb_acpi_power_on_retimers() - Call platform to power on retimers
+ * @port: USB4 port
+ *
+ * Calls platform to turn on power to all retimers behind this USB4
+ * port. After this function returns successfully the caller can
+ * continue with the normal retimer flows (as specified in the USB4
+ * spec). Note if this returns %-EBUSY it means the type-C port is in
+ * non-USB4/TBT mode (there is non-USB4/TBT device connected).
+ *
+ * This should only be called if the USB4/TBT link is not up.
+ *
+ * Returns %0 on success.
+ */
+int tb_acpi_power_on_retimers(struct tb_port *port)
+{
+	return tb_acpi_retimer_set_power(port, true);
+}
+
+/**
+ * tb_acpi_power_off_retimers() - Call platform to power off retimers
+ * @port: USB4 port
+ *
+ * This is the opposite of tb_acpi_power_on_retimers(). After returning
+ * successfully the normal operations with the @port can continue.
+ *
+ * Returns %0 on success.
+ */
+int tb_acpi_power_off_retimers(struct tb_port *port)
+{
+	return tb_acpi_retimer_set_power(port, false);
+}
+
+static bool tb_acpi_bus_match(struct device *dev)
+{
+	return tb_is_switch(dev) || tb_is_usb4_port_device(dev);
+}
+
+static struct acpi_device *tb_acpi_find_port(struct acpi_device *adev,
+					     const struct tb_port *port)
+{
+	struct acpi_device *port_adev;
+
+	if (!adev)
+		return NULL;
+
+	/*
+	 * Device routers exists under the downstream facing USB4 port
+	 * of the parent router. Their _ADR is always 0.
+	 */
+	list_for_each_entry(port_adev, &adev->children, node) {
+		if (acpi_device_adr(port_adev) == port->port)
+			return port_adev;
+	}
+
+	return NULL;
+}
+
+static struct acpi_device *tb_acpi_switch_find_companion(struct tb_switch *sw)
+{
+	struct acpi_device *adev = NULL;
+	struct tb_switch *parent_sw;
+
+	parent_sw = tb_switch_parent(sw);
+	if (parent_sw) {
+		struct tb_port *port = tb_port_at(tb_route(sw), parent_sw);
+		struct acpi_device *port_adev;
+
+		port_adev = tb_acpi_find_port(ACPI_COMPANION(&parent_sw->dev), port);
+		if (port_adev)
+			adev = acpi_find_child_device(port_adev, 0, false);
+	} else {
+		struct tb_nhi *nhi = sw->tb->nhi;
+		struct acpi_device *parent_adev;
+
+		parent_adev = ACPI_COMPANION(&nhi->pdev->dev);
+		if (parent_adev)
+			adev = acpi_find_child_device(parent_adev, 0, false);
+	}
+
+	return adev;
+}
+
+static struct acpi_device *tb_acpi_find_companion(struct device *dev)
+{
+	/*
+	 * The Thunderbolt/USB4 hierarchy looks like following:
+	 *
+	 * Device (NHI)
+	 *   Device (HR)		// Host router _ADR == 0
+	 *      Device (DFP0)		// Downstream port _ADR == lane 0 adapter
+	 *        Device (DR)		// Device router _ADR == 0
+	 *          Device (UFP)	// Upstream port _ADR == lane 0 adapter
+	 *      Device (DFP1)		// Downstream port _ADR == lane 0 adapter number
+	 *
+	 * At the moment we bind the host router to the corresponding
+	 * Linux device.
+	 */
+	if (tb_is_switch(dev))
+		return tb_acpi_switch_find_companion(tb_to_switch(dev));
+	else if (tb_is_usb4_port_device(dev))
+		return tb_acpi_find_port(ACPI_COMPANION(dev->parent),
+					 tb_to_usb4_port_device(dev)->port);
+	return NULL;
+}
+
+static void tb_acpi_setup(struct device *dev)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	struct usb4_port *usb4 = tb_to_usb4_port_device(dev);
+
+	if (!adev || !usb4)
+		return;
+
+	if (acpi_check_dsm(adev->handle, &retimer_dsm_guid, 1,
+			   BIT(RETIMER_DSM_QUERY_ONLINE_STATE) |
+			   BIT(RETIMER_DSM_SET_ONLINE_STATE)))
+		usb4->can_offline = true;
+}
+
+static struct acpi_bus_type tb_acpi_bus = {
+	.name = "thunderbolt",
+	.match = tb_acpi_bus_match,
+	.find_companion = tb_acpi_find_companion,
+	.setup = tb_acpi_setup,
+};
+
+int tb_acpi_init(void)
+{
+	return register_acpi_bus_type(&tb_acpi_bus);
+}
+
+void tb_acpi_exit(void)
+{
+	unregister_acpi_bus_type(&tb_acpi_bus);
+}
diff --git a/drivers/thunderbolt/domain.c b/drivers/thunderbolt/domain.c
index 98f4056f89ff..a062befcb3b2 100644
--- a/drivers/thunderbolt/domain.c
+++ b/drivers/thunderbolt/domain.c
@@ -881,11 +881,12 @@ int tb_domain_init(void)
 	int ret;
 
 	tb_test_init();
-
 	tb_debugfs_init();
+	tb_acpi_init();
+
 	ret = tb_xdomain_init();
 	if (ret)
-		goto err_debugfs;
+		goto err_acpi;
 	ret = bus_register(&tb_bus_type);
 	if (ret)
 		goto err_xdomain;
@@ -894,7 +895,8 @@ int tb_domain_init(void)
 
 err_xdomain:
 	tb_xdomain_exit();
-err_debugfs:
+err_acpi:
+	tb_acpi_exit();
 	tb_debugfs_exit();
 	tb_test_exit();
 
@@ -907,6 +909,7 @@ void tb_domain_exit(void)
 	ida_destroy(&tb_domain_ida);
 	tb_nvm_exit();
 	tb_xdomain_exit();
+	tb_acpi_exit();
 	tb_debugfs_exit();
 	tb_test_exit();
 }
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 948e7601428f..c5704f495afa 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -247,10 +247,13 @@ struct tb_port {
  * struct usb4_port - USB4 port device
  * @dev: Device for the port
  * @port: Pointer to the lane 0 adapter
+ * @can_offline: Does the port have necessary platform support to moved
+ *		 it into offline mode and back
  */
 struct usb4_port {
 	struct device dev;
 	struct tb_port *port;
+	bool can_offline;
 };
 
 /**
@@ -1113,6 +1116,11 @@ bool tb_acpi_may_tunnel_usb3(void);
 bool tb_acpi_may_tunnel_dp(void);
 bool tb_acpi_may_tunnel_pcie(void);
 bool tb_acpi_is_xdomain_allowed(void);
+
+int tb_acpi_init(void);
+void tb_acpi_exit(void);
+int tb_acpi_power_on_retimers(struct tb_port *port);
+int tb_acpi_power_off_retimers(struct tb_port *port);
 #else
 static inline void tb_acpi_add_links(struct tb_nhi *nhi) { }
 
@@ -1121,6 +1129,11 @@ static inline bool tb_acpi_may_tunnel_usb3(void) { return true; }
 static inline bool tb_acpi_may_tunnel_dp(void) { return true; }
 static inline bool tb_acpi_may_tunnel_pcie(void) { return true; }
 static inline bool tb_acpi_is_xdomain_allowed(void) { return true; }
+
+static inline int tb_acpi_init(void) { return 0; }
+static inline void tb_acpi_exit(void) { }
+static inline int tb_acpi_power_on_retimers(struct tb_port *port) { return 0; }
+static inline int tb_acpi_power_off_retimers(struct tb_port *port) { return 0; }
 #endif
 
 #ifdef CONFIG_DEBUG_FS
-- 
2.30.2


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

* [PATCH 4/9] thunderbolt: Add additional USB4 port operations for retimer access
  2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
                   ` (2 preceding siblings ...)
  2021-05-19 14:12 ` [PATCH 3/9] thunderbolt: Add support for ACPI _DSM to power on/off retimers Mika Westerberg
@ 2021-05-19 14:12 ` Mika Westerberg
  2021-05-19 14:12 ` [PATCH 5/9] thunderbolt: Add support for retimer NVM upgrade when there is no link Mika Westerberg
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 14:12 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Rafael J. Wysocki, linux-acpi, Casey G Bowman, Rajmohan Mani,
	Christian Kellner, Greg Kroah-Hartman, Jonathan Corbet,
	Mika Westerberg

From: Rajmohan Mani <rajmohan.mani@intel.com>

When accessing retimers when there is no cable connected we are going to
need additional USB4 port operations. First the port needs to be put
into offline mode, and then the sideband channel transactions must be
enabled on the SBTX line. This adds support for these operations.

Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/sb_regs.h |  2 +
 drivers/thunderbolt/tb.h      |  3 ++
 drivers/thunderbolt/usb4.c    | 69 +++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+)

diff --git a/drivers/thunderbolt/sb_regs.h b/drivers/thunderbolt/sb_regs.h
index 9dafd696612f..bda889ff3bda 100644
--- a/drivers/thunderbolt/sb_regs.h
+++ b/drivers/thunderbolt/sb_regs.h
@@ -17,7 +17,9 @@
 enum usb4_sb_opcode {
 	USB4_SB_OPCODE_ERR = 0x20525245,			/* "ERR " */
 	USB4_SB_OPCODE_ONS = 0x444d4321,			/* "!CMD" */
+	USB4_SB_OPCODE_ROUTER_OFFLINE = 0x4e45534c,		/* "LSEN" */
 	USB4_SB_OPCODE_ENUMERATE_RETIMERS = 0x4d554e45,		/* "ENUM" */
+	USB4_SB_OPCODE_SET_INBOUND_SBTX = 0x5055534c,		/* "LSUP" */
 	USB4_SB_OPCODE_QUERY_LAST_RETIMER = 0x5453414c,		/* "LAST" */
 	USB4_SB_OPCODE_GET_NVM_SECTOR_SIZE = 0x53534e47,	/* "GNSS" */
 	USB4_SB_OPCODE_NVM_SET_OFFSET = 0x53504f42,		/* "BOPS" */
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index c5704f495afa..936518adca74 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -1062,8 +1062,11 @@ int usb4_port_configure(struct tb_port *port);
 void usb4_port_unconfigure(struct tb_port *port);
 int usb4_port_configure_xdomain(struct tb_port *port);
 void usb4_port_unconfigure_xdomain(struct tb_port *port);
+int usb4_port_router_offline(struct tb_port *port);
+int usb4_port_router_online(struct tb_port *port);
 int usb4_port_enumerate_retimers(struct tb_port *port);
 
+int usb4_port_retimer_set_inbound_sbtx(struct tb_port *port, u8 index);
 int usb4_port_retimer_read(struct tb_port *port, u8 index, u8 reg, void *buf,
 			   u8 size);
 int usb4_port_retimer_write(struct tb_port *port, u8 index, u8 reg,
diff --git a/drivers/thunderbolt/usb4.c b/drivers/thunderbolt/usb4.c
index 1f82af35328e..8af96dbaa7a7 100644
--- a/drivers/thunderbolt/usb4.c
+++ b/drivers/thunderbolt/usb4.c
@@ -1318,6 +1318,48 @@ static int usb4_port_sb_op(struct tb_port *port, enum usb4_sb_target target,
 	return -ETIMEDOUT;
 }
 
+static int usb4_port_set_router_offline(struct tb_port *port, bool offline)
+{
+	u32 val = !offline;
+	int ret;
+
+	ret = usb4_port_sb_write(port, USB4_SB_TARGET_ROUTER, 0,
+				  USB4_SB_METADATA, &val, sizeof(val));
+	if (ret)
+		return ret;
+
+	val = USB4_SB_OPCODE_ROUTER_OFFLINE;
+	return usb4_port_sb_write(port, USB4_SB_TARGET_ROUTER, 0,
+				  USB4_SB_OPCODE, &val, sizeof(val));
+}
+
+/**
+ * usb4_port_router_offline() - Put the USB4 port to offline mode
+ * @port: USB4 port
+ *
+ * This function puts the USB4 port into offline mode. In this mode the
+ * port does not react on hotplug events anymore. This needs to be
+ * called before retimer access is done when the USB4 links is not up.
+ *
+ * Returns %0 in case of success and negative errno if there was an
+ * error.
+ */
+int usb4_port_router_offline(struct tb_port *port)
+{
+	return usb4_port_set_router_offline(port, true);
+}
+
+/**
+ * usb4_port_router_online() - Put the USB4 port back to online
+ * @port: USB4 port
+ *
+ * Makes the USB4 port functional again.
+ */
+int usb4_port_router_online(struct tb_port *port)
+{
+	return usb4_port_set_router_offline(port, false);
+}
+
 /**
  * usb4_port_enumerate_retimers() - Send RT broadcast transaction
  * @port: USB4 port
@@ -1343,6 +1385,33 @@ static inline int usb4_port_retimer_op(struct tb_port *port, u8 index,
 			       timeout_msec);
 }
 
+/**
+ * usb4_port_retimer_set_inbound_sbtx() - Enable sideband channel transactions
+ * @port: USB4 port
+ * @index: Retimer index
+ *
+ * Enables sideband channel transations on SBTX. Can be used when USB4
+ * link does not go up, for example if there is no device connected.
+ */
+int usb4_port_retimer_set_inbound_sbtx(struct tb_port *port, u8 index)
+{
+	int ret;
+
+	ret = usb4_port_retimer_op(port, index, USB4_SB_OPCODE_SET_INBOUND_SBTX,
+				   500);
+
+	if (ret != -ENODEV)
+		return ret;
+
+	/*
+	 * Per the USB4 retimer spec, the retimer is not required to
+	 * send an RT (Retimer Transaction) response for the first
+	 * SET_INBOUND_SBTX command
+	 */
+	return usb4_port_retimer_op(port, index, USB4_SB_OPCODE_SET_INBOUND_SBTX,
+				    500);
+}
+
 /**
  * usb4_port_retimer_read() - Read from retimer sideband registers
  * @port: USB4 port
-- 
2.30.2


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

* [PATCH 5/9] thunderbolt: Add support for retimer NVM upgrade when there is no link
  2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
                   ` (3 preceding siblings ...)
  2021-05-19 14:12 ` [PATCH 4/9] thunderbolt: Add additional USB4 port operations for retimer access Mika Westerberg
@ 2021-05-19 14:12 ` Mika Westerberg
  2021-05-19 14:12 ` [PATCH 6/9] thunderbolt: Move nvm_write_ops to tb.h Mika Westerberg
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 14:12 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Rafael J. Wysocki, linux-acpi, Casey G Bowman, Rajmohan Mani,
	Christian Kellner, Greg Kroah-Hartman, Jonathan Corbet,
	Mika Westerberg

From: Rajmohan Mani <rajmohan.mani@intel.com>

With help from platform firmware (ACPI) it is possible to power on
retimers even when there is no USB4 link (e.g nothing is connected to
the USB4 ports). This allows us to bring the USB4 sideband up so that we
can access retimers and upgrade their NVM firmware.

If the platform has support for this, we expose two additional
attributes under USB4 ports: offline and rescan. These can be used to
bring the port offline, rescan for the retimers and put the port online
again. The retimer NVM upgrade itself works the same way than with cable
connected.

Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
Co-developed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 .../ABI/testing/sysfs-bus-thunderbolt         |  26 +++
 Documentation/admin-guide/thunderbolt.rst     |  29 +++
 drivers/thunderbolt/retimer.c                 |  24 ++-
 drivers/thunderbolt/switch.c                  |  48 +++--
 drivers/thunderbolt/tb.c                      |   4 +-
 drivers/thunderbolt/tb.h                      |   5 +-
 drivers/thunderbolt/usb4_port.c               | 169 ++++++++++++++++++
 7 files changed, 277 insertions(+), 28 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
index 3537ba1ba892..f6743dc33aac 100644
--- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
+++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
@@ -297,6 +297,32 @@ Contact:	Mika Westerberg <mika.westerberg@linux.intel.com>
 Description:	Returns the current link mode. Possible values are
 		"usb4", "tbt" and "none".
 
+What:		/sys/bus/thunderbolt/devices/usb4_portX/offline
+Date:		Sep 2021
+KernelVersion:	v5.14
+Contact:	Rajmohan Mani <rajmohan.mani@intel.com>
+Description:	Writing 1 to this attribute puts the USB4 port into
+		offline mode. Only allowed when there is nothing
+		connected to the port (link attribute returns "none").
+		Once the port is in offline mode it does not receive any
+		hotplug events. This is used to update NVM firmware of
+		on-board retimers. Writing 0 puts the port back to
+		online mode.
+
+		This attribute is only visible if the platform supports
+		powering on retimers when there is no cable connected.
+
+What:		/sys/bus/thunderbolt/devices/usb4_portX/rescan
+Date:		Sep 2021
+KernelVersion:	v5.14
+Contact:	Rajmohan Mani <rajmohan.mani@intel.com>
+Description:	When the USB4 port is in offline mode writing 1 to this
+		attribute forces rescan of the sideband for on-board
+		retimers. Each retimer appear under the USB4 port as if
+		the USB4 link was up. These retimers act in the same way
+		as if the cable was connected so upgrading their NVM
+		firmware can be done the usual way.
+
 What:		/sys/bus/thunderbolt/devices/<device>:<port>.<index>/device
 Date:		Oct 2020
 KernelVersion:	v5.9
diff --git a/Documentation/admin-guide/thunderbolt.rst b/Documentation/admin-guide/thunderbolt.rst
index f18e881373c4..2ed79f41a411 100644
--- a/Documentation/admin-guide/thunderbolt.rst
+++ b/Documentation/admin-guide/thunderbolt.rst
@@ -256,6 +256,35 @@ Note names of the NVMem devices ``nvm_activeN`` and ``nvm_non_activeN``
 depend on the order they are registered in the NVMem subsystem. N in
 the name is the identifier added by the NVMem subsystem.
 
+Upgrading on-board retimer NVM when there is no cable connected
+---------------------------------------------------------------
+If the platform supports, it may be possible to upgrade the retimer NVM
+firmware even when there is nothing connected to the USB4
+ports. When this is the case the ``usb4_portX`` devices have two special
+attributes: ``offline`` and ``rescan``. The way to upgrade the firmware
+is to first put the USB4 port into offline mode::
+
+  # echo 1 > /sys/bus/thunderbolt/devices/0-0/usb4_port1/offline
+
+This step makes sure the port does not respond to any hotplug events,
+and also ensures the retimers are powered on. The next step is to scan
+for the retimers::
+
+  # echo 1 > /sys/bus/thunderbolt/devices/0-0/usb4_port1/rescan
+
+This enumerates and adds the on-board retimers. Now retimer NVM can be
+upgraded in the same way than with cable connected (see previous
+section). However, the retimer is not disconnected as we are offline
+mode) so after writing ``1`` to ``nvm_authenticate`` one should wait for
+5 or more seconds before running rescan again::
+
+  # echo 1 > /sys/bus/thunderbolt/devices/0-0/usb4_port1/rescan
+
+This point if everything went fine, the port can be put back to
+functional state again::
+
+  # echo 0 > /sys/bus/thunderbolt/devices/0-0/usb4_port1/offline
+
 Upgrading NVM when host controller is in safe mode
 --------------------------------------------------
 If the existing NVM is not properly authenticated (or is missing) the
diff --git a/drivers/thunderbolt/retimer.c b/drivers/thunderbolt/retimer.c
index 2e5188fb1150..05af0feefe84 100644
--- a/drivers/thunderbolt/retimer.c
+++ b/drivers/thunderbolt/retimer.c
@@ -401,19 +401,18 @@ static struct tb_retimer *tb_port_find_retimer(struct tb_port *port, u8 index)
 /**
  * tb_retimer_scan() - Scan for on-board retimers under port
  * @port: USB4 port to scan
+ * @add: If true also registers found retimers
  *
- * Tries to enumerate on-board retimers connected to @port. Found
- * retimers are registered as children of @port. Does not scan for cable
- * retimers for now.
+ * Brings the sideband into a state where retimers can be accessed.
+ * Then Tries to enumerate on-board retimers connected to @port. Found
+ * retimers are registered as children of @port if @add is set.  Does
+ * not scan for cable retimers for now.
  */
-int tb_retimer_scan(struct tb_port *port)
+int tb_retimer_scan(struct tb_port *port, bool add)
 {
 	u32 status[TB_MAX_RETIMER_INDEX + 1] = {};
 	int ret, i, last_idx = 0;
 
-	if (!port->cap_usb4)
-		return 0;
-
 	/*
 	 * Send broadcast RT to make sure retimer indices facing this
 	 * port are set.
@@ -422,6 +421,13 @@ int tb_retimer_scan(struct tb_port *port)
 	if (ret)
 		return ret;
 
+	/*
+	 * Enable sideband channel for each retimer. We can do this
+	 * regardless whether there is device connected or not.
+	 */
+	for (i = 1; i <= TB_MAX_RETIMER_INDEX; i++)
+		usb4_port_retimer_set_inbound_sbtx(port, i);
+
 	/*
 	 * Before doing anything else, read the authentication status.
 	 * If the retimer has it set, store it for the new retimer
@@ -453,10 +459,10 @@ int tb_retimer_scan(struct tb_port *port)
 		rt = tb_port_find_retimer(port, i);
 		if (rt) {
 			put_device(&rt->dev);
-		} else {
+		} else if (add) {
 			ret = tb_retimer_add(port, i, status[i]);
 			if (ret && ret != -EOPNOTSUPP)
-				return ret;
+				break;
 		}
 	}
 
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index 7303c61a891a..dae59919e2bf 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -1153,6 +1153,33 @@ static int tb_port_start_lane_initialization(struct tb_port *port)
 	return ret == -EINVAL ? 0 : ret;
 }
 
+/*
+ * Returns true if the port had something (router, XDomain) connected
+ * before suspend.
+ */
+static bool tb_port_resume(struct tb_port *port)
+{
+	bool has_remote = tb_port_has_remote(port);
+
+	if (port->usb4) {
+		usb4_port_device_resume(port->usb4);
+	} else if (!has_remote) {
+		/*
+		 * For disconnected downstream lane adapters start lane
+		 * initialization now so we detect future connects.
+		 *
+		 * For XDomain start the lane initialzation now so the
+		 * link gets re-established.
+		 *
+		 * This is only needed for non-USB4 ports.
+		 */
+		if (!tb_is_upstream_port(port) || port->xdomain)
+			tb_port_start_lane_initialization(port);
+	}
+
+	return has_remote || port->xdomain;
+}
+
 /**
  * tb_port_is_enabled() - Is the adapter port enabled
  * @port: Port to check
@@ -2915,22 +2942,11 @@ int tb_switch_resume(struct tb_switch *sw)
 
 	/* check for surviving downstream switches */
 	tb_switch_for_each_port(sw, port) {
-		if (!tb_port_has_remote(port) && !port->xdomain) {
-			/*
-			 * For disconnected downstream lane adapters
-			 * start lane initialization now so we detect
-			 * future connects.
-			 */
-			if (!tb_is_upstream_port(port) && tb_port_is_null(port))
-				tb_port_start_lane_initialization(port);
+		if (!tb_port_is_null(port))
+			continue;
+
+		if (!tb_port_resume(port))
 			continue;
-		} else if (port->xdomain) {
-			/*
-			 * Start lane initialization for XDomain so the
-			 * link gets re-established.
-			 */
-			tb_port_start_lane_initialization(port);
-		}
 
 		if (tb_wait_for_port(port, true) <= 0) {
 			tb_port_warn(port,
@@ -2939,7 +2955,7 @@ int tb_switch_resume(struct tb_switch *sw)
 				tb_sw_set_unplugged(port->remote->sw);
 			else if (port->xdomain)
 				port->xdomain->is_unplugged = true;
-		} else if (tb_port_has_remote(port) || port->xdomain) {
+		} else {
 			/*
 			 * Always unlock the port so the downstream
 			 * switch/domain is accessible.
diff --git a/drivers/thunderbolt/tb.c b/drivers/thunderbolt/tb.c
index 7e6dc2b03bed..bc6d568dbb89 100644
--- a/drivers/thunderbolt/tb.c
+++ b/drivers/thunderbolt/tb.c
@@ -595,7 +595,7 @@ static void tb_scan_port(struct tb_port *port)
 		return;
 	}
 
-	tb_retimer_scan(port);
+	tb_retimer_scan(port, true);
 
 	sw = tb_switch_alloc(port->sw->tb, &port->sw->dev,
 			     tb_downstream_route(port));
@@ -662,7 +662,7 @@ static void tb_scan_port(struct tb_port *port)
 		tb_sw_warn(sw, "failed to enable TMU\n");
 
 	/* Scan upstream retimers */
-	tb_retimer_scan(upstream_port);
+	tb_retimer_scan(upstream_port, true);
 
 	/*
 	 * Create USB 3.x tunnels only when the switch is plugged to the
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 936518adca74..341e8443a22d 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -249,11 +249,13 @@ struct tb_port {
  * @port: Pointer to the lane 0 adapter
  * @can_offline: Does the port have necessary platform support to moved
  *		 it into offline mode and back
+ * @offline: The port is currently in offline mode
  */
 struct usb4_port {
 	struct device dev;
 	struct tb_port *port;
 	bool can_offline;
+	bool offline;
 };
 
 /**
@@ -1017,7 +1019,7 @@ void tb_xdomain_remove(struct tb_xdomain *xd);
 struct tb_xdomain *tb_xdomain_find_by_link_depth(struct tb *tb, u8 link,
 						 u8 depth);
 
-int tb_retimer_scan(struct tb_port *port);
+int tb_retimer_scan(struct tb_port *port, bool add);
 void tb_retimer_remove_all(struct tb_port *port);
 
 static inline bool tb_is_retimer(const struct device *dev)
@@ -1105,6 +1107,7 @@ static inline struct usb4_port *tb_to_usb4_port_device(struct device *dev)
 
 struct usb4_port *usb4_port_device_add(struct tb_port *port);
 void usb4_port_device_remove(struct usb4_port *usb4);
+int usb4_port_device_resume(struct usb4_port *usb4);
 
 /* Keep link controller awake during update */
 #define QUIRK_FORCE_POWER_LINK_CONTROLLER		BIT(0)
diff --git a/drivers/thunderbolt/usb4_port.c b/drivers/thunderbolt/usb4_port.c
index 520bbfd7bf33..765c74179598 100644
--- a/drivers/thunderbolt/usb4_port.c
+++ b/drivers/thunderbolt/usb4_port.c
@@ -44,8 +44,166 @@ static const struct attribute_group common_group = {
 	.attrs = common_attrs,
 };
 
+static int usb4_port_offline(struct usb4_port *usb4)
+{
+	struct tb_port *port = usb4->port;
+	int ret;
+
+	ret = tb_acpi_power_on_retimers(port);
+	if (ret)
+		return ret;
+
+	ret = usb4_port_router_offline(port);
+	if (ret) {
+		tb_acpi_power_off_retimers(port);
+		return ret;
+	}
+
+	ret = tb_retimer_scan(port, false);
+	if (ret) {
+		usb4_port_router_online(port);
+		tb_acpi_power_off_retimers(port);
+	}
+
+	return ret;
+}
+
+static void usb4_port_online(struct usb4_port *usb4)
+{
+	struct tb_port *port = usb4->port;
+
+	usb4_port_router_online(port);
+	tb_acpi_power_off_retimers(port);
+}
+
+static ssize_t offline_show(struct device *dev,
+	struct device_attribute *attr, char *buf)
+{
+	struct usb4_port *usb4 = tb_to_usb4_port_device(dev);
+
+	return sysfs_emit(buf, "%d\n", usb4->offline);
+}
+
+static ssize_t offline_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct usb4_port *usb4 = tb_to_usb4_port_device(dev);
+	struct tb_port *port = usb4->port;
+	struct tb *tb = port->sw->tb;
+	bool val;
+	int ret;
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		return ret;
+
+	pm_runtime_get_sync(&usb4->dev);
+
+	if (mutex_lock_interruptible(&tb->lock)) {
+		ret = -ERESTARTSYS;
+		goto out_rpm;
+	}
+
+	if (val == usb4->offline)
+		goto out_unlock;
+
+	/* Offline mode works only for ports that are not connected */
+	if (tb_port_has_remote(port)) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
+	if (val) {
+		ret = usb4_port_offline(usb4);
+		if (ret)
+			goto out_unlock;
+	} else {
+		usb4_port_online(usb4);
+		tb_retimer_remove_all(port);
+	}
+
+	usb4->offline = val;
+	tb_port_dbg(port, "%s offline mode\n", val ? "enter" : "exit");
+
+out_unlock:
+	mutex_unlock(&tb->lock);
+out_rpm:
+	pm_runtime_mark_last_busy(&usb4->dev);
+	pm_runtime_put_autosuspend(&usb4->dev);
+
+	return ret ? ret : count;
+}
+static DEVICE_ATTR_RW(offline);
+
+static ssize_t rescan_store(struct device *dev,
+	struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct usb4_port *usb4 = tb_to_usb4_port_device(dev);
+	struct tb_port *port = usb4->port;
+	struct tb *tb = port->sw->tb;
+	bool val;
+	int ret;
+
+	ret = kstrtobool(buf, &val);
+	if (ret)
+		return ret;
+
+	if (!val)
+		return count;
+
+	pm_runtime_get_sync(&usb4->dev);
+
+	if (mutex_lock_interruptible(&tb->lock)) {
+		ret = -ERESTARTSYS;
+		goto out_rpm;
+	}
+
+	/* Must be in offline mode already */
+	if (!usb4->offline) {
+		ret = -EINVAL;
+		goto out_unlock;
+	}
+
+	tb_retimer_remove_all(port);
+	ret = tb_retimer_scan(port, true);
+
+out_unlock:
+	mutex_unlock(&tb->lock);
+out_rpm:
+	pm_runtime_mark_last_busy(&usb4->dev);
+	pm_runtime_put_autosuspend(&usb4->dev);
+
+	return ret ? ret : count;
+}
+static DEVICE_ATTR_WO(rescan);
+
+static struct attribute *service_attrs[] = {
+	&dev_attr_offline.attr,
+	&dev_attr_rescan.attr,
+	NULL
+};
+
+static umode_t service_attr_is_visible(struct kobject *kobj,
+				       struct attribute *attr, int n)
+{
+	struct device *dev = kobj_to_dev(kobj);
+	struct usb4_port *usb4 = tb_to_usb4_port_device(dev);
+
+	/*
+	 * Always need some platform help to cycle the modes so that
+	 * retimers can be accessed through the sideband.
+	 */
+	return usb4->can_offline ? attr->mode : 0;
+}
+
+static const struct attribute_group service_group = {
+	.attrs = service_attrs,
+	.is_visible = service_attr_is_visible,
+};
+
 static const struct attribute_group *usb4_port_device_groups[] = {
 	&common_group,
+	&service_group,
 	NULL
 };
 
@@ -110,3 +268,14 @@ void usb4_port_device_remove(struct usb4_port *usb4)
 {
 	device_unregister(&usb4->dev);
 }
+
+/**
+ * usb4_port_device_resume() - Resumes USB4 port device
+ * @usb4: USB4 port device
+ *
+ * Used to resume USB4 port device after sleep state.
+ */
+int usb4_port_device_resume(struct usb4_port *usb4)
+{
+	return usb4->offline ? usb4_port_offline(usb4) : 0;
+}
-- 
2.30.2


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

* [PATCH 6/9] thunderbolt: Move nvm_write_ops to tb.h
  2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
                   ` (4 preceding siblings ...)
  2021-05-19 14:12 ` [PATCH 5/9] thunderbolt: Add support for retimer NVM upgrade when there is no link Mika Westerberg
@ 2021-05-19 14:12 ` Mika Westerberg
  2021-05-19 14:12 ` [PATCH 7/9] thunderbolt: Allow router NVM authenticate separately Mika Westerberg
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 14:12 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Rafael J. Wysocki, linux-acpi, Casey G Bowman, Rajmohan Mani,
	Christian Kellner, Greg Kroah-Hartman, Jonathan Corbet,
	Mika Westerberg

From: Rajmohan Mani <rajmohan.mani@intel.com>

Currently these write ops are used for updating router firmware images
only. Moving to tb.h helps the retimers also to use the same ops.

Also add tb_ prefix to the enum while there.

Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/switch.c | 5 -----
 drivers/thunderbolt/tb.h     | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index dae59919e2bf..bf4821d3bbab 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -26,11 +26,6 @@ struct nvm_auth_status {
 	u32 status;
 };
 
-enum nvm_write_ops {
-	WRITE_AND_AUTHENTICATE = 1,
-	WRITE_ONLY = 2,
-};
-
 /*
  * Hold NVM authentication failure status per switch This information
  * needs to stay around even when the switch gets power cycled so we
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 341e8443a22d..863d80ad44ab 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -58,6 +58,11 @@ struct tb_nvm {
 	bool flushed;
 };
 
+enum tb_nvm_write_ops {
+	WRITE_AND_AUTHENTICATE = 1,
+	WRITE_ONLY = 2,
+};
+
 #define TB_SWITCH_KEY_SIZE		32
 #define TB_SWITCH_MAX_DEPTH		6
 #define USB4_SWITCH_MAX_DEPTH		5
-- 
2.30.2


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

* [PATCH 7/9] thunderbolt: Allow router NVM authenticate separately
  2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
                   ` (5 preceding siblings ...)
  2021-05-19 14:12 ` [PATCH 6/9] thunderbolt: Move nvm_write_ops to tb.h Mika Westerberg
@ 2021-05-19 14:12 ` Mika Westerberg
  2021-05-19 14:12 ` [PATCH 8/9] thunderbolt: Add WRITE_ONLY and AUTHENTICATE_ONLY NVM operations for retimers Mika Westerberg
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 14:12 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Rafael J. Wysocki, linux-acpi, Casey G Bowman, Rajmohan Mani,
	Christian Kellner, Greg Kroah-Hartman, Jonathan Corbet,
	Mika Westerberg

It may be useful if the actual NVM authentication can be delayed to be
run later, for instance when the user logs out. For this reason add a
new NVM operation (AUHENTICATE_ONLY) that just triggers the authentication
procedure over whatever was written to the NVM storage.

This is not supported with Thunderbolt 1-3 devices, though.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 .../ABI/testing/sysfs-bus-thunderbolt         |  5 +-
 drivers/thunderbolt/switch.c                  | 50 ++++++++++++-------
 drivers/thunderbolt/tb.h                      |  2 +
 drivers/thunderbolt/usb4.c                    | 13 ++++-
 4 files changed, 49 insertions(+), 21 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
index f6743dc33aac..da580b504c87 100644
--- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
+++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
@@ -213,12 +213,15 @@ Description:	When new NVM image is written to the non-active NVM
 		restarted with the new NVM firmware. If the image
 		verification fails an error code is returned instead.
 
-		This file will accept writing values "1" or "2"
+		This file will accept writing values "1", "2" or "3".
 
 		- Writing "1" will flush the image to the storage
 		  area and authenticate the image in one action.
 		- Writing "2" will run some basic validation on the image
 		  and flush it to the storage area.
+		- Writing "3" will authenticate the image that is
+		  currently written in the storage area. This is only
+		  supported with USB4 devices.
 
 		When read holds status of the last authentication
 		operation if an error occurred during the process. This
diff --git a/drivers/thunderbolt/switch.c b/drivers/thunderbolt/switch.c
index bf4821d3bbab..83b1ef3d5d03 100644
--- a/drivers/thunderbolt/switch.c
+++ b/drivers/thunderbolt/switch.c
@@ -303,13 +303,23 @@ static inline int nvm_read(struct tb_switch *sw, unsigned int address,
 	return dma_port_flash_read(sw->dma_port, address, buf, size);
 }
 
-static int nvm_authenticate(struct tb_switch *sw)
+static int nvm_authenticate(struct tb_switch *sw, bool auth_only)
 {
 	int ret;
 
-	if (tb_switch_is_usb4(sw))
+	if (tb_switch_is_usb4(sw)) {
+		if (auth_only) {
+			ret = usb4_switch_nvm_set_offset(sw, 0);
+			if (ret)
+				return ret;
+		}
+		sw->nvm->authenticating = true;
 		return usb4_switch_nvm_authenticate(sw);
+	} else if (auth_only) {
+		return -EOPNOTSUPP;
+	}
 
+	sw->nvm->authenticating = true;
 	if (!tb_route(sw)) {
 		nvm_authenticate_start_dma_port(sw);
 		ret = nvm_authenticate_host_dma_port(sw);
@@ -1713,8 +1723,7 @@ static ssize_t nvm_authenticate_sysfs(struct device *dev, const char *buf,
 				      bool disconnect)
 {
 	struct tb_switch *sw = tb_to_switch(dev);
-	int val;
-	int ret;
+	int val, ret;
 
 	pm_runtime_get_sync(&sw->dev);
 
@@ -1737,22 +1746,27 @@ static ssize_t nvm_authenticate_sysfs(struct device *dev, const char *buf,
 	nvm_clear_auth_status(sw);
 
 	if (val > 0) {
-		if (!sw->nvm->flushed) {
-			if (!sw->nvm->buf) {
+		if (val == AUTHENTICATE_ONLY) {
+			if (disconnect)
 				ret = -EINVAL;
-				goto exit_unlock;
+			else
+				ret = nvm_authenticate(sw, true);
+		} else {
+			if (!sw->nvm->flushed) {
+				if (!sw->nvm->buf) {
+					ret = -EINVAL;
+					goto exit_unlock;
+				}
+
+				ret = nvm_validate_and_write(sw);
+				if (ret || val == WRITE_ONLY)
+					goto exit_unlock;
 			}
-
-			ret = nvm_validate_and_write(sw);
-			if (ret || val == WRITE_ONLY)
-				goto exit_unlock;
-		}
-		if (val == WRITE_AND_AUTHENTICATE) {
-			if (disconnect) {
-				ret = tb_lc_force_power(sw);
-			} else {
-				sw->nvm->authenticating = true;
-				ret = nvm_authenticate(sw);
+			if (val == WRITE_AND_AUTHENTICATE) {
+				if (disconnect)
+					ret = tb_lc_force_power(sw);
+				else
+					ret = nvm_authenticate(sw, false);
 			}
 		}
 	}
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 863d80ad44ab..53f6bb85b178 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -61,6 +61,7 @@ struct tb_nvm {
 enum tb_nvm_write_ops {
 	WRITE_AND_AUTHENTICATE = 1,
 	WRITE_ONLY = 2,
+	AUTHENTICATE_ONLY = 3,
 };
 
 #define TB_SWITCH_KEY_SIZE		32
@@ -1049,6 +1050,7 @@ int usb4_switch_set_sleep(struct tb_switch *sw);
 int usb4_switch_nvm_sector_size(struct tb_switch *sw);
 int usb4_switch_nvm_read(struct tb_switch *sw, unsigned int address, void *buf,
 			 size_t size);
+int usb4_switch_nvm_set_offset(struct tb_switch *sw, unsigned int address);
 int usb4_switch_nvm_write(struct tb_switch *sw, unsigned int address,
 			  const void *buf, size_t size);
 int usb4_switch_nvm_authenticate(struct tb_switch *sw);
diff --git a/drivers/thunderbolt/usb4.c b/drivers/thunderbolt/usb4.c
index 8af96dbaa7a7..76d7335aa440 100644
--- a/drivers/thunderbolt/usb4.c
+++ b/drivers/thunderbolt/usb4.c
@@ -547,8 +547,17 @@ int usb4_switch_nvm_read(struct tb_switch *sw, unsigned int address, void *buf,
 				usb4_switch_nvm_read_block, sw);
 }
 
-static int usb4_switch_nvm_set_offset(struct tb_switch *sw,
-				      unsigned int address)
+/**
+ * usb4_switch_nvm_set_offset() - Set NVM write offset
+ * @sw: USB4 router
+ * @address: Start offset
+ *
+ * Explicitly sets NVM write offset. Normally when writing to NVM this
+ * is done automatically by usb4_switch_nvm_write().
+ *
+ * Returns %0 in success and negative errno if there was a failure.
+ */
+int usb4_switch_nvm_set_offset(struct tb_switch *sw, unsigned int address)
 {
 	u32 metadata, dwaddress;
 	u8 status = 0;
-- 
2.30.2


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

* [PATCH 8/9] thunderbolt: Add WRITE_ONLY and AUTHENTICATE_ONLY NVM operations for retimers
  2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
                   ` (6 preceding siblings ...)
  2021-05-19 14:12 ` [PATCH 7/9] thunderbolt: Allow router NVM authenticate separately Mika Westerberg
@ 2021-05-19 14:12 ` Mika Westerberg
  2021-05-19 14:12 ` [PATCH 9/9] thunderbolt: Check for NVM authentication status after the operation started Mika Westerberg
  2021-05-20  8:59 ` [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Greg Kroah-Hartman
  9 siblings, 0 replies; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 14:12 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Rafael J. Wysocki, linux-acpi, Casey G Bowman, Rajmohan Mani,
	Christian Kellner, Greg Kroah-Hartman, Jonathan Corbet,
	Mika Westerberg

From: Rajmohan Mani <rajmohan.mani@intel.com>

The same way we support these two operations for USB4 routers we can
extend the retimer NVM operations to support retimers also.

Signed-off-by: Rajmohan Mani <rajmohan.mani@intel.com>
Co-developed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 .../ABI/testing/sysfs-bus-thunderbolt         |  2 +-
 drivers/thunderbolt/retimer.c                 | 51 ++++++++++++++-----
 drivers/thunderbolt/tb.h                      |  2 +
 drivers/thunderbolt/usb4.c                    | 15 +++++-
 4 files changed, 53 insertions(+), 17 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-thunderbolt b/Documentation/ABI/testing/sysfs-bus-thunderbolt
index da580b504c87..95c21d6c9a84 100644
--- a/Documentation/ABI/testing/sysfs-bus-thunderbolt
+++ b/Documentation/ABI/testing/sysfs-bus-thunderbolt
@@ -221,7 +221,7 @@ Description:	When new NVM image is written to the non-active NVM
 		  and flush it to the storage area.
 		- Writing "3" will authenticate the image that is
 		  currently written in the storage area. This is only
-		  supported with USB4 devices.
+		  supported with USB4 devices and retimers.
 
 		When read holds status of the last authentication
 		operation if an error occurred during the process. This
diff --git a/drivers/thunderbolt/retimer.c b/drivers/thunderbolt/retimer.c
index 05af0feefe84..3aa790aa6500 100644
--- a/drivers/thunderbolt/retimer.c
+++ b/drivers/thunderbolt/retimer.c
@@ -103,6 +103,7 @@ static int tb_retimer_nvm_validate_and_write(struct tb_retimer *rt)
 	unsigned int image_size, hdr_size;
 	const u8 *buf = rt->nvm->buf;
 	u16 ds_size, device;
+	int ret;
 
 	image_size = rt->nvm->buf_data_size;
 	if (image_size < NVM_MIN_SIZE || image_size > NVM_MAX_SIZE)
@@ -140,8 +141,25 @@ static int tb_retimer_nvm_validate_and_write(struct tb_retimer *rt)
 	buf += hdr_size;
 	image_size -= hdr_size;
 
-	return usb4_port_retimer_nvm_write(rt->port, rt->index, 0, buf,
-					   image_size);
+	ret = usb4_port_retimer_nvm_write(rt->port, rt->index, 0, buf,
+					 image_size);
+	if (!ret)
+		rt->nvm->flushed = true;
+
+	return ret;
+}
+
+static int tb_retimer_nvm_authenticate(struct tb_retimer *rt, bool auth_only)
+{
+	int ret;
+
+	if (auth_only) {
+		ret = usb4_port_retimer_nvm_set_offset(rt->port, rt->index, 0);
+		if (ret)
+			return ret;
+	}
+
+	return usb4_port_retimer_nvm_authenticate(rt->port, rt->index);
 }
 
 static ssize_t device_show(struct device *dev, struct device_attribute *attr,
@@ -176,8 +194,7 @@ static ssize_t nvm_authenticate_store(struct device *dev,
 	struct device_attribute *attr, const char *buf, size_t count)
 {
 	struct tb_retimer *rt = tb_to_retimer(dev);
-	bool val;
-	int ret;
+	int val, ret;
 
 	pm_runtime_get_sync(&rt->dev);
 
@@ -191,7 +208,7 @@ static ssize_t nvm_authenticate_store(struct device *dev,
 		goto exit_unlock;
 	}
 
-	ret = kstrtobool(buf, &val);
+	ret = kstrtoint(buf, 10, &val);
 	if (ret)
 		goto exit_unlock;
 
@@ -199,16 +216,22 @@ static ssize_t nvm_authenticate_store(struct device *dev,
 	rt->auth_status = 0;
 
 	if (val) {
-		if (!rt->nvm->buf) {
-			ret = -EINVAL;
-			goto exit_unlock;
+		if (val == AUTHENTICATE_ONLY) {
+			ret = tb_retimer_nvm_authenticate(rt, true);
+		} else {
+			if (!rt->nvm->flushed) {
+				if (!rt->nvm->buf) {
+					ret = -EINVAL;
+					goto exit_unlock;
+				}
+
+				ret = tb_retimer_nvm_validate_and_write(rt);
+				if (ret || val == WRITE_ONLY)
+					goto exit_unlock;
+			}
+			if (val == WRITE_AND_AUTHENTICATE)
+				ret = tb_retimer_nvm_authenticate(rt, false);
 		}
-
-		ret = tb_retimer_nvm_validate_and_write(rt);
-		if (ret)
-			goto exit_unlock;
-
-		ret = usb4_port_retimer_nvm_authenticate(rt->port, rt->index);
 	}
 
 exit_unlock:
diff --git a/drivers/thunderbolt/tb.h b/drivers/thunderbolt/tb.h
index 53f6bb85b178..725104c83e3d 100644
--- a/drivers/thunderbolt/tb.h
+++ b/drivers/thunderbolt/tb.h
@@ -1082,6 +1082,8 @@ int usb4_port_retimer_write(struct tb_port *port, u8 index, u8 reg,
 			    const void *buf, u8 size);
 int usb4_port_retimer_is_last(struct tb_port *port, u8 index);
 int usb4_port_retimer_nvm_sector_size(struct tb_port *port, u8 index);
+int usb4_port_retimer_nvm_set_offset(struct tb_port *port, u8 index,
+				     unsigned int address);
 int usb4_port_retimer_nvm_write(struct tb_port *port, u8 index,
 				unsigned int address, const void *buf,
 				size_t size);
diff --git a/drivers/thunderbolt/usb4.c b/drivers/thunderbolt/usb4.c
index 76d7335aa440..ceddbe7e9f93 100644
--- a/drivers/thunderbolt/usb4.c
+++ b/drivers/thunderbolt/usb4.c
@@ -1513,8 +1513,19 @@ int usb4_port_retimer_nvm_sector_size(struct tb_port *port, u8 index)
 	return ret ? ret : metadata & USB4_NVM_SECTOR_SIZE_MASK;
 }
 
-static int usb4_port_retimer_nvm_set_offset(struct tb_port *port, u8 index,
-					    unsigned int address)
+/**
+ * usb4_port_retimer_nvm_set_offset() - Set NVM write offset
+ * @port: USB4 port
+ * @index: Retimer index
+ * @address: Start offset
+ *
+ * Exlicitly sets NVM write offset. Normally when writing to NVM this is
+ * done automatically by usb4_port_retimer_nvm_write().
+ *
+ * Returns %0 in success and negative errno if there was a failure.
+ */
+int usb4_port_retimer_nvm_set_offset(struct tb_port *port, u8 index,
+				     unsigned int address)
 {
 	u32 metadata, dwaddress;
 	int ret;
-- 
2.30.2


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

* [PATCH 9/9] thunderbolt: Check for NVM authentication status after the operation started
  2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
                   ` (7 preceding siblings ...)
  2021-05-19 14:12 ` [PATCH 8/9] thunderbolt: Add WRITE_ONLY and AUTHENTICATE_ONLY NVM operations for retimers Mika Westerberg
@ 2021-05-19 14:12 ` Mika Westerberg
  2021-05-20  8:59 ` [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Greg Kroah-Hartman
  9 siblings, 0 replies; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 14:12 UTC (permalink / raw)
  To: linux-usb
  Cc: Yehezkel Bernat, Michael Jamet, Andreas Noever, Lukas Wunner,
	Rafael J. Wysocki, linux-acpi, Casey G Bowman, Rajmohan Mani,
	Christian Kellner, Greg Kroah-Hartman, Jonathan Corbet,
	Mika Westerberg

If the NVM authentication fails immediately, like if the firmware
detects that the image is not valid for some reason, better to read the
status once and if set to non-zero fail the operation accordingly.

Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
 drivers/thunderbolt/retimer.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/thunderbolt/retimer.c b/drivers/thunderbolt/retimer.c
index 3aa790aa6500..722694052f4a 100644
--- a/drivers/thunderbolt/retimer.c
+++ b/drivers/thunderbolt/retimer.c
@@ -151,6 +151,7 @@ static int tb_retimer_nvm_validate_and_write(struct tb_retimer *rt)
 
 static int tb_retimer_nvm_authenticate(struct tb_retimer *rt, bool auth_only)
 {
+	u32 status;
 	int ret;
 
 	if (auth_only) {
@@ -159,7 +160,24 @@ static int tb_retimer_nvm_authenticate(struct tb_retimer *rt, bool auth_only)
 			return ret;
 	}
 
-	return usb4_port_retimer_nvm_authenticate(rt->port, rt->index);
+	ret = usb4_port_retimer_nvm_authenticate(rt->port, rt->index);
+	if (ret)
+		return ret;
+
+	usleep_range(100, 150);
+
+	/*
+	 * Check the status now if we still can access the retimer. It
+	 * is expected that the below fails.
+	 */
+	ret = usb4_port_retimer_nvm_authenticate_status(rt->port, rt->index,
+							&status);
+	if (!ret) {
+		rt->auth_status = status;
+		return status ? -EINVAL : 0;
+	}
+
+	return 0;
 }
 
 static ssize_t device_show(struct device *dev, struct device_attribute *attr,
-- 
2.30.2


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

* Re: [PATCH 2/9] thunderbolt: Add USB4 port devices
  2021-05-19 14:12 ` [PATCH 2/9] thunderbolt: Add USB4 port devices Mika Westerberg
@ 2021-05-19 15:14   ` Heikki Krogerus
  2021-05-19 15:30     ` Heikki Krogerus
  2021-05-19 15:40     ` Mika Westerberg
  0 siblings, 2 replies; 16+ messages in thread
From: Heikki Krogerus @ 2021-05-19 15:14 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: linux-usb, Yehezkel Bernat, Michael Jamet, Andreas Noever,
	Lukas Wunner, Rafael J. Wysocki, linux-acpi, Casey G Bowman,
	Rajmohan Mani, Christian Kellner, Greg Kroah-Hartman,
	Jonathan Corbet

On Wed, May 19, 2021 at 05:12:52PM +0300, Mika Westerberg wrote:
> Create devices for each USB4 port. This is needed when we add retimer
> access when there is no device connected but may be useful for other
> purposes too following what USB subsystem does. This exports a single
> attribute "link" that shows the type of the USB4 link (or "none" if
> there is no cable connected).

<snip>

> +/*
> + * USB4 port device
> + *
> + * Copyright (C) 2021, Intel Corporation
> + * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "tb.h"
> +
> +static ssize_t link_show(struct device *dev, struct device_attribute *attr,
> +			 char *buf)
> +{
> +	struct usb4_port *usb4 = tb_to_usb4_port_device(dev);
> +	struct tb_port *port = usb4->port;
> +	struct tb *tb = port->sw->tb;
> +	const char *link;
> +
> +	if (mutex_lock_interruptible(&tb->lock))
> +		return -ERESTARTSYS;
> +
> +	if (tb_is_upstream_port(port))
> +		link = port->sw->link_usb4 ? "usb4" : "tbt";
> +	else if (tb_port_has_remote(port))
> +		link = port->remote->sw->link_usb4 ? "usb4" : "tbt";
> +	else
> +		link = "none";
> +
> +	mutex_unlock(&tb->lock);
> +
> +	return sysfs_emit(buf, "%s\n", link);
> +}
> +static DEVICE_ATTR_RO(link);
> +
> +static struct attribute *common_attrs[] = {
> +	&dev_attr_link.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group common_group = {
> +	.attrs = common_attrs,
> +};
> +
> +static const struct attribute_group *usb4_port_device_groups[] = {
> +	&common_group,
> +	NULL
> +};
> +
> +static void usb4_port_device_release(struct device *dev)
> +{
> +	struct usb4_port *usb4 = container_of(dev, struct usb4_port, dev);
> +
> +	kfree(usb4);
> +}
> +
> +struct device_type usb4_port_device_type = {
> +	.name = "usb4_port",
> +	.groups = usb4_port_device_groups,
> +	.release = usb4_port_device_release,
> +};

I noticed that in the next patch you add acpi_bus_type for these
ports, but is that really necessary? Why not just:

int usb4_port_fwnode_match(struct tb_port *port, struct fwnode_handle *fwnode)
{
        if (is_acpi_device_node(fwnode))
                return acpi_device_adr(to_acpi_device_node(fwnode)) == port->port;

        return 0;
}

> +/**
> + * usb4_port_device_add() - Add USB4 port device
> + * @port: Lane 0 adapter port to add the USB4 port
> + *
> + * Creates and registers a USB4 port device for @port. Returns the new
> + * USB4 port device pointer or ERR_PTR() in case of error.
> + */
> +struct usb4_port *usb4_port_device_add(struct tb_port *port)
> +{

        struct fwnode_handle *child;

> +	struct usb4_port *usb4;
> +	int ret;
> +
> +	usb4 = kzalloc(sizeof(*usb4), GFP_KERNEL);
> +	if (!usb4)
> +		return ERR_PTR(-ENOMEM);
> +
> +	usb4->port = port;
> +	usb4->dev.type = &usb4_port_device_type;
> +	usb4->dev.parent = &port->sw->dev;
> +	dev_set_name(&usb4->dev, "usb4_port%d", port->port);

and then here something like this (feel free to improve this part):

        device_for_each_child_node(&port->sw->dev, child) {
                if (usb4_port_fwnode_match(port, child)) {
                        usb4->dev.fwnode = child;
                        break;
                }
        }

Or maybe I'm missing something?

> +	ret = device_register(&usb4->dev);
> +	if (ret) {
> +		put_device(&usb4->dev);
> +		return ERR_PTR(ret);
> +	}
> +
> +	pm_runtime_no_callbacks(&usb4->dev);
> +	pm_runtime_set_active(&usb4->dev);
> +	pm_runtime_enable(&usb4->dev);
> +	pm_runtime_set_autosuspend_delay(&usb4->dev, TB_AUTOSUSPEND_DELAY);
> +	pm_runtime_mark_last_busy(&usb4->dev);
> +	pm_runtime_use_autosuspend(&usb4->dev);
> +
> +	return usb4;
> +}

thanks,

-- 
heikki

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

* Re: [PATCH 2/9] thunderbolt: Add USB4 port devices
  2021-05-19 15:14   ` Heikki Krogerus
@ 2021-05-19 15:30     ` Heikki Krogerus
  2021-05-19 15:40     ` Mika Westerberg
  1 sibling, 0 replies; 16+ messages in thread
From: Heikki Krogerus @ 2021-05-19 15:30 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: linux-usb, Yehezkel Bernat, Michael Jamet, Andreas Noever,
	Lukas Wunner, Rafael J. Wysocki, linux-acpi, Casey G Bowman,
	Rajmohan Mani, Christian Kellner, Greg Kroah-Hartman,
	Jonathan Corbet

On Wed, May 19, 2021 at 06:14:47PM +0300, Heikki Krogerus wrote:
> On Wed, May 19, 2021 at 05:12:52PM +0300, Mika Westerberg wrote:
> > Create devices for each USB4 port. This is needed when we add retimer
> > access when there is no device connected but may be useful for other
> > purposes too following what USB subsystem does. This exports a single
> > attribute "link" that shows the type of the USB4 link (or "none" if
> > there is no cable connected).
> 
> <snip>
> 
> > +/*
> > + * USB4 port device
> > + *
> > + * Copyright (C) 2021, Intel Corporation
> > + * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/pm_runtime.h>
> > +
> > +#include "tb.h"
> > +
> > +static ssize_t link_show(struct device *dev, struct device_attribute *attr,
> > +			 char *buf)
> > +{
> > +	struct usb4_port *usb4 = tb_to_usb4_port_device(dev);
> > +	struct tb_port *port = usb4->port;
> > +	struct tb *tb = port->sw->tb;
> > +	const char *link;
> > +
> > +	if (mutex_lock_interruptible(&tb->lock))
> > +		return -ERESTARTSYS;
> > +
> > +	if (tb_is_upstream_port(port))
> > +		link = port->sw->link_usb4 ? "usb4" : "tbt";
> > +	else if (tb_port_has_remote(port))
> > +		link = port->remote->sw->link_usb4 ? "usb4" : "tbt";
> > +	else
> > +		link = "none";
> > +
> > +	mutex_unlock(&tb->lock);
> > +
> > +	return sysfs_emit(buf, "%s\n", link);
> > +}
> > +static DEVICE_ATTR_RO(link);
> > +
> > +static struct attribute *common_attrs[] = {
> > +	&dev_attr_link.attr,
> > +	NULL
> > +};
> > +
> > +static const struct attribute_group common_group = {
> > +	.attrs = common_attrs,
> > +};
> > +
> > +static const struct attribute_group *usb4_port_device_groups[] = {
> > +	&common_group,
> > +	NULL
> > +};
> > +
> > +static void usb4_port_device_release(struct device *dev)
> > +{
> > +	struct usb4_port *usb4 = container_of(dev, struct usb4_port, dev);
> > +
> > +	kfree(usb4);
> > +}
> > +
> > +struct device_type usb4_port_device_type = {
> > +	.name = "usb4_port",
> > +	.groups = usb4_port_device_groups,
> > +	.release = usb4_port_device_release,
> > +};
> 
> I noticed that in the next patch you add acpi_bus_type for these
> ports, but is that really necessary? Why not just:
> 
> int usb4_port_fwnode_match(struct tb_port *port, struct fwnode_handle *fwnode)
> {
>         if (is_acpi_device_node(fwnode))
>                 return acpi_device_adr(to_acpi_device_node(fwnode)) == port->port;
> 
>         return 0;
> }
> 
> > +/**
> > + * usb4_port_device_add() - Add USB4 port device
> > + * @port: Lane 0 adapter port to add the USB4 port
> > + *
> > + * Creates and registers a USB4 port device for @port. Returns the new
> > + * USB4 port device pointer or ERR_PTR() in case of error.
> > + */
> > +struct usb4_port *usb4_port_device_add(struct tb_port *port)
> > +{
> 
>         struct fwnode_handle *child;
> 
> > +	struct usb4_port *usb4;
> > +	int ret;
> > +
> > +	usb4 = kzalloc(sizeof(*usb4), GFP_KERNEL);
> > +	if (!usb4)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	usb4->port = port;
> > +	usb4->dev.type = &usb4_port_device_type;
> > +	usb4->dev.parent = &port->sw->dev;
> > +	dev_set_name(&usb4->dev, "usb4_port%d", port->port);
> 
> and then here something like this (feel free to improve this part):
> 
>         device_for_each_child_node(&port->sw->dev, child) {
>                 if (usb4_port_fwnode_match(port, child)) {
>                         usb4->dev.fwnode = child;
>                         break;
>                 }
>         }
> 
> Or maybe I'm missing something?

Ah, the node hierarchy is much more complex than I though.

Sorry for the noise.

thanks,

-- 
heikki

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

* Re: [PATCH 2/9] thunderbolt: Add USB4 port devices
  2021-05-19 15:14   ` Heikki Krogerus
  2021-05-19 15:30     ` Heikki Krogerus
@ 2021-05-19 15:40     ` Mika Westerberg
  2021-05-20  9:23       ` Heikki Krogerus
  1 sibling, 1 reply; 16+ messages in thread
From: Mika Westerberg @ 2021-05-19 15:40 UTC (permalink / raw)
  To: Heikki Krogerus
  Cc: linux-usb, Yehezkel Bernat, Michael Jamet, Andreas Noever,
	Lukas Wunner, Rafael J. Wysocki, linux-acpi, Casey G Bowman,
	Rajmohan Mani, Christian Kellner, Greg Kroah-Hartman,
	Jonathan Corbet

Hi Heikki,

On Wed, May 19, 2021 at 06:14:47PM +0300, Heikki Krogerus wrote:
> On Wed, May 19, 2021 at 05:12:52PM +0300, Mika Westerberg wrote:
> > Create devices for each USB4 port. This is needed when we add retimer
> > access when there is no device connected but may be useful for other
> > purposes too following what USB subsystem does. This exports a single
> > attribute "link" that shows the type of the USB4 link (or "none" if
> > there is no cable connected).
> 
> <snip>
> 
> > +/*
> > + * USB4 port device
> > + *
> > + * Copyright (C) 2021, Intel Corporation
> > + * Author: Mika Westerberg <mika.westerberg@linux.intel.com>
> > + */
> > +
> > +#include <linux/acpi.h>
> > +#include <linux/pm_runtime.h>
> > +
> > +#include "tb.h"
> > +
> > +static ssize_t link_show(struct device *dev, struct device_attribute *attr,
> > +			 char *buf)
> > +{
> > +	struct usb4_port *usb4 = tb_to_usb4_port_device(dev);
> > +	struct tb_port *port = usb4->port;
> > +	struct tb *tb = port->sw->tb;
> > +	const char *link;
> > +
> > +	if (mutex_lock_interruptible(&tb->lock))
> > +		return -ERESTARTSYS;
> > +
> > +	if (tb_is_upstream_port(port))
> > +		link = port->sw->link_usb4 ? "usb4" : "tbt";
> > +	else if (tb_port_has_remote(port))
> > +		link = port->remote->sw->link_usb4 ? "usb4" : "tbt";
> > +	else
> > +		link = "none";
> > +
> > +	mutex_unlock(&tb->lock);
> > +
> > +	return sysfs_emit(buf, "%s\n", link);
> > +}
> > +static DEVICE_ATTR_RO(link);
> > +
> > +static struct attribute *common_attrs[] = {
> > +	&dev_attr_link.attr,
> > +	NULL
> > +};
> > +
> > +static const struct attribute_group common_group = {
> > +	.attrs = common_attrs,
> > +};
> > +
> > +static const struct attribute_group *usb4_port_device_groups[] = {
> > +	&common_group,
> > +	NULL
> > +};
> > +
> > +static void usb4_port_device_release(struct device *dev)
> > +{
> > +	struct usb4_port *usb4 = container_of(dev, struct usb4_port, dev);
> > +
> > +	kfree(usb4);
> > +}
> > +
> > +struct device_type usb4_port_device_type = {
> > +	.name = "usb4_port",
> > +	.groups = usb4_port_device_groups,
> > +	.release = usb4_port_device_release,
> > +};
> 
> I noticed that in the next patch you add acpi_bus_type for these
> ports, but is that really necessary? Why not just:
> 
> int usb4_port_fwnode_match(struct tb_port *port, struct fwnode_handle *fwnode)
> {
>         if (is_acpi_device_node(fwnode))
>                 return acpi_device_adr(to_acpi_device_node(fwnode)) == port->port;
> 
>         return 0;
> }

I have to say I might be missing some new additions to fwnode front but
the acpi_bus_type is used to match the ACPI nodes to usb4_ports and also
to routers. I see that this one may work for the former but not sure
about the latter.

I guess we could do similar for routers too in switch.c.

However, I would like to keep ACPI specific code in acpi.c if possible
but if this is the preferred way then no problem doing what you say :)

> > +/**
> > + * usb4_port_device_add() - Add USB4 port device
> > + * @port: Lane 0 adapter port to add the USB4 port
> > + *
> > + * Creates and registers a USB4 port device for @port. Returns the new
> > + * USB4 port device pointer or ERR_PTR() in case of error.
> > + */
> > +struct usb4_port *usb4_port_device_add(struct tb_port *port)
> > +{
> 
>         struct fwnode_handle *child;
> 
> > +	struct usb4_port *usb4;
> > +	int ret;
> > +
> > +	usb4 = kzalloc(sizeof(*usb4), GFP_KERNEL);
> > +	if (!usb4)
> > +		return ERR_PTR(-ENOMEM);
> > +
> > +	usb4->port = port;
> > +	usb4->dev.type = &usb4_port_device_type;
> > +	usb4->dev.parent = &port->sw->dev;
> > +	dev_set_name(&usb4->dev, "usb4_port%d", port->port);
> 
> and then here something like this (feel free to improve this part):
> 
>         device_for_each_child_node(&port->sw->dev, child) {
>                 if (usb4_port_fwnode_match(port, child)) {
>                         usb4->dev.fwnode = child;
>                         break;
>                 }
>         }
> 
> Or maybe I'm missing something?
> 
> > +	ret = device_register(&usb4->dev);
> > +	if (ret) {
> > +		put_device(&usb4->dev);
> > +		return ERR_PTR(ret);
> > +	}
> > +
> > +	pm_runtime_no_callbacks(&usb4->dev);
> > +	pm_runtime_set_active(&usb4->dev);
> > +	pm_runtime_enable(&usb4->dev);
> > +	pm_runtime_set_autosuspend_delay(&usb4->dev, TB_AUTOSUSPEND_DELAY);
> > +	pm_runtime_mark_last_busy(&usb4->dev);
> > +	pm_runtime_use_autosuspend(&usb4->dev);
> > +
> > +	return usb4;
> > +}
> 
> thanks,
> 
> -- 
> heikki

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

* Re: [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support
  2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
                   ` (8 preceding siblings ...)
  2021-05-19 14:12 ` [PATCH 9/9] thunderbolt: Check for NVM authentication status after the operation started Mika Westerberg
@ 2021-05-20  8:59 ` Greg Kroah-Hartman
  2021-06-01  7:56   ` Mika Westerberg
  9 siblings, 1 reply; 16+ messages in thread
From: Greg Kroah-Hartman @ 2021-05-20  8:59 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: linux-usb, Yehezkel Bernat, Michael Jamet, Andreas Noever,
	Lukas Wunner, Rafael J. Wysocki, linux-acpi, Casey G Bowman,
	Rajmohan Mani, Christian Kellner, Jonathan Corbet

On Wed, May 19, 2021 at 05:12:50PM +0300, Mika Westerberg wrote:
> Hi all,
> 
> USB4 retimers are only accessible when the USB4 is up. However, sometimes
> it may be useful to be able to upgrade on-board retimers even if the link
> is not up. For instance if the user simply does not have any USB4 devices.
> 
> Making retimers accessible in "offline" mode requires some help from the
> platform firmware (ACPI in our case) to turn on power to the retimers and
> cycle them through different modes to get the sideband link up. This may
> also involve other firmwares such as Embedded Controller (as it is the case
> with recent Chromebooks).
> 
> This series adds support for "offline" retimer NVM upgrade so that it first
> exposes each USB4 port to the userspace. If the platform firmware provides
> a special _DSM-method (Device Specific Method) under the USB4 port ACPI
> description, we expose two attributes under the port that the userspace can
> use to put the port to offline mode and rescan for the retimers. Otherwise
> the NVM upgrade works the same way than with the online mode. We also add
> documentation to the admin-guide how this can be done.
> 
> In addition to this, at least Intel USB4 devices (and retimers) allow
> running NVM authenticate (upgrade) separately from write so we make it
> possible for the userspace to run the write and authenticate in two steps.
> This allows userspace to trigger the authentication at later time, like
> when the user logs out.
> 
> Mika Westerberg (4):
>   thunderbolt: Log the link as TBT instead of TBT3
>   thunderbolt: Add USB4 port devices
>   thunderbolt: Allow router NVM authenticate separately
>   thunderbolt: Check for NVM authentication status after the operation started
> 
> Rajmohan Mani (5):
>   thunderbolt: Add support for ACPI _DSM to power on/off retimers
>   thunderbolt: Add additional USB4 port operations for retimer access
>   thunderbolt: Add support for retimer NVM upgrade when there is no link
>   thunderbolt: Move nvm_write_ops to tb.h
>   thunderbolt: Add WRITE_ONLY and AUTHENTICATE_ONLY NVM operations for retimers

Looks good:

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

* Re: [PATCH 2/9] thunderbolt: Add USB4 port devices
  2021-05-19 15:40     ` Mika Westerberg
@ 2021-05-20  9:23       ` Heikki Krogerus
  0 siblings, 0 replies; 16+ messages in thread
From: Heikki Krogerus @ 2021-05-20  9:23 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: linux-usb, Yehezkel Bernat, Michael Jamet, Andreas Noever,
	Lukas Wunner, Rafael J. Wysocki, linux-acpi, Casey G Bowman,
	Rajmohan Mani, Christian Kellner, Greg Kroah-Hartman,
	Jonathan Corbet

Hi,

On Wed, May 19, 2021 at 06:40:05PM +0300, Mika Westerberg wrote:
> > I noticed that in the next patch you add acpi_bus_type for these
> > ports, but is that really necessary? Why not just:
> > 
> > int usb4_port_fwnode_match(struct tb_port *port, struct fwnode_handle *fwnode)
> > {
> >         if (is_acpi_device_node(fwnode))
> >                 return acpi_device_adr(to_acpi_device_node(fwnode)) == port->port;
> > 
> >         return 0;
> > }
> 
> I have to say I might be missing some new additions to fwnode front but
> the acpi_bus_type is used to match the ACPI nodes to usb4_ports and also
> to routers. I see that this one may work for the former but not sure
> about the latter.

Yeah, I noticed that afterwards.

> I guess we could do similar for routers too in switch.c.

Forget about it.

> However, I would like to keep ACPI specific code in acpi.c if possible
> but if this is the preferred way then no problem doing what you say :)

Well, that's actually the thing. The matching is the only ACPI (or any
firmware type) specific thing that we need when assigning the firmware
nodes to the device. Note also that the above function works already
even with CONFIG_ACPI=n. That's why I though that there is no reason
not to just make this firmware node type agnostic from the start.

But since your acpi_bus_type handles also the routers, and also since
the node hierarchy with the separate UFP and DFP ports and everything
did not look straightforward, it's probable easier to just use that.

So sorry again for the noise.

> > > +/**
> > > + * usb4_port_device_add() - Add USB4 port device
> > > + * @port: Lane 0 adapter port to add the USB4 port
> > > + *
> > > + * Creates and registers a USB4 port device for @port. Returns the new
> > > + * USB4 port device pointer or ERR_PTR() in case of error.
> > > + */
> > > +struct usb4_port *usb4_port_device_add(struct tb_port *port)
> > > +{
> > 
> >         struct fwnode_handle *child;
> > 
> > > +	struct usb4_port *usb4;
> > > +	int ret;
> > > +
> > > +	usb4 = kzalloc(sizeof(*usb4), GFP_KERNEL);
> > > +	if (!usb4)
> > > +		return ERR_PTR(-ENOMEM);
> > > +
> > > +	usb4->port = port;
> > > +	usb4->dev.type = &usb4_port_device_type;
> > > +	usb4->dev.parent = &port->sw->dev;
> > > +	dev_set_name(&usb4->dev, "usb4_port%d", port->port);
> > 
> > and then here something like this (feel free to improve this part):
> > 
> >         device_for_each_child_node(&port->sw->dev, child) {
> >                 if (usb4_port_fwnode_match(port, child)) {
> >                         usb4->dev.fwnode = child;
> >                         break;
> >                 }
> >         }
> > 
> > Or maybe I'm missing something?
> > 
> > > +	ret = device_register(&usb4->dev);
> > > +	if (ret) {
> > > +		put_device(&usb4->dev);
> > > +		return ERR_PTR(ret);
> > > +	}
> > > +
> > > +	pm_runtime_no_callbacks(&usb4->dev);
> > > +	pm_runtime_set_active(&usb4->dev);
> > > +	pm_runtime_enable(&usb4->dev);
> > > +	pm_runtime_set_autosuspend_delay(&usb4->dev, TB_AUTOSUSPEND_DELAY);
> > > +	pm_runtime_mark_last_busy(&usb4->dev);
> > > +	pm_runtime_use_autosuspend(&usb4->dev);
> > > +
> > > +	return usb4;
> > > +}

thanks,

-- 
heikki

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

* Re: [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support
  2021-05-20  8:59 ` [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Greg Kroah-Hartman
@ 2021-06-01  7:56   ` Mika Westerberg
  0 siblings, 0 replies; 16+ messages in thread
From: Mika Westerberg @ 2021-06-01  7:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-usb, Yehezkel Bernat, Michael Jamet, Andreas Noever,
	Lukas Wunner, Rafael J. Wysocki, linux-acpi, Casey G Bowman,
	Rajmohan Mani, Christian Kellner, Jonathan Corbet

On Thu, May 20, 2021 at 10:59:12AM +0200, Greg Kroah-Hartman wrote:
> On Wed, May 19, 2021 at 05:12:50PM +0300, Mika Westerberg wrote:
> > Hi all,
> > 
> > USB4 retimers are only accessible when the USB4 is up. However, sometimes
> > it may be useful to be able to upgrade on-board retimers even if the link
> > is not up. For instance if the user simply does not have any USB4 devices.
> > 
> > Making retimers accessible in "offline" mode requires some help from the
> > platform firmware (ACPI in our case) to turn on power to the retimers and
> > cycle them through different modes to get the sideband link up. This may
> > also involve other firmwares such as Embedded Controller (as it is the case
> > with recent Chromebooks).
> > 
> > This series adds support for "offline" retimer NVM upgrade so that it first
> > exposes each USB4 port to the userspace. If the platform firmware provides
> > a special _DSM-method (Device Specific Method) under the USB4 port ACPI
> > description, we expose two attributes under the port that the userspace can
> > use to put the port to offline mode and rescan for the retimers. Otherwise
> > the NVM upgrade works the same way than with the online mode. We also add
> > documentation to the admin-guide how this can be done.
> > 
> > In addition to this, at least Intel USB4 devices (and retimers) allow
> > running NVM authenticate (upgrade) separately from write so we make it
> > possible for the userspace to run the write and authenticate in two steps.
> > This allows userspace to trigger the authentication at later time, like
> > when the user logs out.
> > 
> > Mika Westerberg (4):
> >   thunderbolt: Log the link as TBT instead of TBT3
> >   thunderbolt: Add USB4 port devices
> >   thunderbolt: Allow router NVM authenticate separately
> >   thunderbolt: Check for NVM authentication status after the operation started
> > 
> > Rajmohan Mani (5):
> >   thunderbolt: Add support for ACPI _DSM to power on/off retimers
> >   thunderbolt: Add additional USB4 port operations for retimer access
> >   thunderbolt: Add support for retimer NVM upgrade when there is no link
> >   thunderbolt: Move nvm_write_ops to tb.h
> >   thunderbolt: Add WRITE_ONLY and AUTHENTICATE_ONLY NVM operations for retimers
> 
> Looks good:
> 
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks!

All applied to thunderbolt.git/next.

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

end of thread, other threads:[~2021-06-01  7:58 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19 14:12 [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Mika Westerberg
2021-05-19 14:12 ` [PATCH 1/9] thunderbolt: Log the link as TBT instead of TBT3 Mika Westerberg
2021-05-19 14:12 ` [PATCH 2/9] thunderbolt: Add USB4 port devices Mika Westerberg
2021-05-19 15:14   ` Heikki Krogerus
2021-05-19 15:30     ` Heikki Krogerus
2021-05-19 15:40     ` Mika Westerberg
2021-05-20  9:23       ` Heikki Krogerus
2021-05-19 14:12 ` [PATCH 3/9] thunderbolt: Add support for ACPI _DSM to power on/off retimers Mika Westerberg
2021-05-19 14:12 ` [PATCH 4/9] thunderbolt: Add additional USB4 port operations for retimer access Mika Westerberg
2021-05-19 14:12 ` [PATCH 5/9] thunderbolt: Add support for retimer NVM upgrade when there is no link Mika Westerberg
2021-05-19 14:12 ` [PATCH 6/9] thunderbolt: Move nvm_write_ops to tb.h Mika Westerberg
2021-05-19 14:12 ` [PATCH 7/9] thunderbolt: Allow router NVM authenticate separately Mika Westerberg
2021-05-19 14:12 ` [PATCH 8/9] thunderbolt: Add WRITE_ONLY and AUTHENTICATE_ONLY NVM operations for retimers Mika Westerberg
2021-05-19 14:12 ` [PATCH 9/9] thunderbolt: Check for NVM authentication status after the operation started Mika Westerberg
2021-05-20  8:59 ` [PATCH 0/9] thunderbolt: Offline on-board retimer NVM upgrade support Greg Kroah-Hartman
2021-06-01  7:56   ` Mika Westerberg

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.