This series adds support for fully describing the USB/DP topology on
ChromeOS Trogdor devices in DT. Trogdor devices have a single DP phy in
the AP that is muxed to one of two usb type-c connectors depending on
which port asserts HPD first to the EC. We'd like to know which port is
connected to an external monitor to provide a better experience to the
user about things like which type-c port is displaying DP or which
type-c hub is acting up, etc. Describing the connection all the way from
the source to the connector will allow us to do this. There will be some
more work to do after this to wire up sysfs connections, but that work
has already started or finished so it should be mostly minor changes to
support DT there.
This patch series is large, unfortunately, and is ordered in logical
groups: gpio, USB, DRM, typec, and finally dts to put it all together.
There's more that could be put in here, e.g. supporting ChromeOS Corsola
devices, but I wanted to get something out there early instead of
waiting to make this work with everything that exists today and posting
it then.
Onto the patches:
First is the EC GPIO driver, which is dependency free and can be merged
at any time. It's only needed to provide information about which port
the EC is steering DP to, because the EC had a bug where it never told
the AP about which port has HPD asserted or not.
Second is the USB binding and hub patches. These are used to describe
how the USB hub is wired up on all the Trogdor devices, and make the
connect_type be something besides "unknown" on DT devices. ACPI has
supported setting a proper connect_type for some time now. These can
also be merged pretty much dependency free, except that the dt binding
will be needed to avoid DT binding check failures. I don't think those
checks are fatal though, so probably also fine to take this part
independently.
Third is the DRM bridge patches. These are used to implement lane
assignment for DP altmode configurations through the drm_bridge code.
The typec code will use this to tell the DP phy how many lanes of DP to
drive and which lanes to drive out to the USB type-c connector. Adding
support for lane assignment allows us to implement DP muxing as well,
physically splitting the DP lanes on the DP phy so that hardware doesn't
have to use an analog mux to steer two DP lanes to one or the other
type-c port. These are a hard dependency for the typec code.
Fourth is the typec patches, that ties together everything that comes
before it in this series. The EC typec switch driver implements a
drm_bridge that can signal HPD from the type-c connector through the
bridge chain, mux the DP phy in software so that we don't have to use an
analog mux, and implement orientation control for boards like Kukui that
directly connect the DP phy to the type-c port, necessitating lane
assignment to flip the lanes to match the cable orientation.
Finally, the dts patches wire everything up to fully describe the USB/DT
topology on Trogdor. This includes the USB hub, the EC gpios, the DP
controller, and the external connectors like the usb-c and usb-a
connectors.
After this initial version I will probably split this series and send
parts in pieces to more rapidly send new versions. Those parts will
refer back to this version in the cover letter so we can all get the
full context. I don't expect to merge this through one maintainer tree
immediately, so I set the 'To' line to chrome-platform to reflect the
overall target audience.
Prashant Malani (1):
platform/chrome: cros_ec_typec: Purge blocking switch devlinks
Stephen Boyd (21):
dt-bindings: gpio: Add binding for ChromeOS EC GPIO controller
gpio: Add ChromeOS EC GPIO driver
dt-bindings: usb: Add downstream facing ports to realtek binding
usb: core: Set connect_type of ports based on DT node
drm/atomic-helper: Introduce lane remapping support to bridges
drm/bridge: Verify lane assignment is going to work during
atomic_check
device property: Add remote endpoint to devcon matcher
platform/chrome: cros_typec_switch: Use read_poll_timeout helper
platform/chrome: cros_typec_switch: Move port creation code to
sub-function
platform/chrome: cros_typec_switch: Use fwnode instead of ACPI APIs
platform/chrome: cros_typec_switch: Use dev_err_probe()
dt-bindings: chrome: Add google,cros-ec-typec-switch binding
platform/chrome: cros_typec_switch: Add support for signaling HPD to
drm_bridge
platform/chrome: cros_typec_switch: Support DP muxing via DRM lane
assignment
platform/chrome: cros_typec_switch: Support orientation-switch
platform/chrome: cros_typec_switch: Handle lack of HPD information
dt-bindings: chrome: Add binding for ChromeOS Pogo pin connector
arm64: dts: qcom: sc7180: quackingstick: Disable instead of delete
usb_c1
arm64: dts: qcom: sc7180: pazquel: Add missing comment header
arm64: dts: qcom: sc7180-trogdor: Make clamshell/detachable fragments
arm64: dts: qcom: sc7180-trogdor: Wire up USB and DP to
usb-c-connectors
.../chrome/google,cros-ec-typec-switch.yaml | 365 ++++++++++++
.../chrome/google,pogo-pin-connector.yaml | 61 ++
.../bindings/gpio/google,cros-ec-gpio.yaml | 49 ++
.../bindings/mfd/google,cros-ec.yaml | 8 +
.../bindings/usb/realtek,rts5411.yaml | 50 ++
.../dts/qcom/sc7180-trogdor-clamshell.dtsi | 30 +
.../boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 52 +-
.../dts/qcom/sc7180-trogdor-detachable.dtsi | 25 +
.../dts/qcom/sc7180-trogdor-homestar.dtsi | 54 +-
.../dts/qcom/sc7180-trogdor-kingoftown.dts | 57 +-
.../boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 58 +-
.../boot/dts/qcom/sc7180-trogdor-pazquel.dtsi | 59 +-
.../boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 46 +-
.../qcom/sc7180-trogdor-quackingstick.dtsi | 46 +-
.../arm64/boot/dts/qcom/sc7180-trogdor-r1.dts | 2 +-
.../dts/qcom/sc7180-trogdor-wormdingler.dtsi | 52 +-
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 175 ++++++
drivers/base/property.c | 7 +-
drivers/gpio/Kconfig | 10 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-cros-ec.c | 218 +++++++
drivers/gpu/drm/drm_atomic_state_helper.c | 2 +
drivers/gpu/drm/drm_bridge.c | 50 ++
drivers/platform/chrome/Kconfig | 3 +-
drivers/platform/chrome/cros_ec_typec.c | 10 +
drivers/platform/chrome/cros_typec_switch.c | 555 +++++++++++++++---
drivers/usb/core/port.c | 37 ++
drivers/usb/roles/class.c | 4 +-
drivers/usb/typec/mux.c | 8 +
drivers/usb/typec/retimer.c | 7 +-
include/drm/drm_atomic.h | 31 +
include/drm/drm_bridge.h | 4 +
include/linux/property.h | 5 +-
33 files changed, 2026 insertions(+), 115 deletions(-)
create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
create mode 100644 Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
create mode 100644 Documentation/devicetree/bindings/gpio/google,cros-ec-gpio.yaml
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
create mode 100644 drivers/gpio/gpio-cros-ec.c
Cc: Alexandre Belloni <[email protected]>
Cc: Andrzej Hajda <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: <[email protected]>
Cc: Daniel Scally <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: David Airlie <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Heikki Krogerus <[email protected]>
Cc: Ivan Orlov <[email protected]>
Cc: Jernej Skrabec <[email protected]>
Cc: Jonas Karlman <[email protected]>
Cc: Konrad Dybcio <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: Linus Walleij <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: maciek swiech <[email protected]>
Cc: Matthias Kaehlcke <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Mika Westerberg <[email protected]>
Cc: Neil Armstrong <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Cc: Prashant Malani <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Robert Foss <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: Tzung-Bi Shih <[email protected]>
Cc: Vinod Koul <[email protected]>
base-commit: 0dd3ee31125508cd67f7e7172247f05b7fd1753a
--
https://chromeos.dev
The ChromeOS embedded controller (EC) supports setting the state of
GPIOs when the system is unlocked, and getting the state of GPIOs in all
cases. The GPIOs are on the EC itself, so the EC acts similar to a GPIO
expander. Add a binding to describe these GPIOs in DT so that other
devices described in DT can read the GPIOs on the EC.
Cc: Linus Walleij <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
.../bindings/gpio/google,cros-ec-gpio.yaml | 49 +++++++++++++++++++
.../bindings/mfd/google,cros-ec.yaml | 3 ++
2 files changed, 52 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpio/google,cros-ec-gpio.yaml
diff --git a/Documentation/devicetree/bindings/gpio/google,cros-ec-gpio.yaml b/Documentation/devicetree/bindings/gpio/google,cros-ec-gpio.yaml
new file mode 100644
index 000000000000..a9f1d7784070
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpio/google,cros-ec-gpio.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpio/google,cros-ec-gpio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIOs controlled by ChromeOS EC
+
+maintainers:
+ - Stephen Boyd <[email protected]>
+
+description:
+ Google's ChromeOS EC has a gpio controller inside the Embedded Controller
+ (EC) and controlled via a host-command interface. The node for this
+ device should be under a cros-ec node like google,cros-ec-spi.
+
+properties:
+ compatible:
+ const: google,cros-ec-gpio
+
+ '#gpio-cells':
+ const: 2
+
+ gpio-controller: true
+
+required:
+ - compatible
+ - '#gpio-cells'
+ - gpio-controller
+
+additionalProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cros-ec@0 {
+ compatible = "google,cros-ec-spi";
+ reg = <0>;
+ interrupts = <101 0>;
+ gpio {
+ compatible = "google,cros-ec-gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+ };
+ };
diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index e1ca4f297c6d..ded396b28fba 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -93,6 +93,9 @@ properties:
'#size-cells':
const: 0
+ gpio:
+ $ref: /schemas/gpio/google,cros-ec-gpio.yaml#
+
typec:
$ref: /schemas/chrome/google,cros-ec-typec.yaml#
--
https://chromeos.dev
The ChromeOS embedded controller (EC) supports setting the state of
GPIOs when the system is unlocked, and getting the state of GPIOs in all
cases. The GPIOs are on the EC itself, so the EC acts similar to a GPIO
expander. Add a driver to get and set the GPIOs on the EC through the
host command interface.
Cc: Linus Walleij <[email protected]>
Cc: Bartosz Golaszewski <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/gpio/Kconfig | 10 ++
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-cros-ec.c | 218 ++++++++++++++++++++++++++++++++++++
3 files changed, 229 insertions(+)
create mode 100644 drivers/gpio/gpio-cros-ec.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index b3a133ed31ee..62b0ae25a727 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -1220,6 +1220,16 @@ config GPIO_BD9571MWV
This driver can also be built as a module. If so, the module
will be called gpio-bd9571mwv.
+config GPIO_CROS_EC
+ tristate "ChromeOS EC GPIO support"
+ depends on CROS_EC
+ help
+ GPIO driver for exposing GPIOs on the ChromeOS Embedded
+ Controller.
+
+ This driver can also be built as a module. If so, the module
+ will be called gpio-cros-ec.
+
config GPIO_CRYSTAL_COVE
tristate "GPIO support for Crystal Cove PMIC"
depends on (X86 || COMPILE_TEST) && INTEL_SOC_PMIC
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index eb73b5d633eb..2e66410c1da6 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_GPIO_BT8XX) += gpio-bt8xx.o
obj-$(CONFIG_GPIO_CADENCE) += gpio-cadence.o
obj-$(CONFIG_GPIO_CLPS711X) += gpio-clps711x.o
obj-$(CONFIG_GPIO_SNPS_CREG) += gpio-creg-snps.o
+obj-$(CONFIG_GPIO_CROS_EC) += gpio-cros-ec.o
obj-$(CONFIG_GPIO_CRYSTAL_COVE) += gpio-crystalcove.o
obj-$(CONFIG_GPIO_CS5535) += gpio-cs5535.o
obj-$(CONFIG_GPIO_DA9052) += gpio-da9052.o
diff --git a/drivers/gpio/gpio-cros-ec.c b/drivers/gpio/gpio-cros-ec.c
new file mode 100644
index 000000000000..0d35558304bf
--- /dev/null
+++ b/drivers/gpio/gpio-cros-ec.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2024 Google LLC
+ *
+ * This driver provides the ability to control GPIOs on the Chrome OS EC.
+ * There isn't any direction control, and setting values on GPIOs is only
+ * possible when the system is unlocked.
+ */
+
+#include <linux/bitops.h>
+#include <linux/errno.h>
+#include <linux/gpio/driver.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* Setting gpios is only supported when the system is unlocked */
+static void cros_ec_gpio_set(struct gpio_chip *gc, unsigned int gpio, int val)
+{
+ const char *name = gc->names[gpio];
+ struct cros_ec_device *cros_ec = gpiochip_get_data(gc);
+ struct ec_params_gpio_set params = {
+ .val = val,
+ };
+ int ret;
+ ssize_t copied;
+
+ copied = strscpy(params.name, name, sizeof(params.name));
+ if (copied < 0)
+ return;
+
+ ret = cros_ec_cmd(cros_ec, 0, EC_CMD_GPIO_SET, ¶ms,
+ sizeof(params), NULL, 0);
+ if (ret < 0)
+ dev_err(gc->parent, "error setting gpio%d (%s) on EC: %d\n", gpio, name, ret);
+}
+
+static int cros_ec_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+ const char *name = gc->names[gpio];
+ struct cros_ec_device *cros_ec = gpiochip_get_data(gc);
+ struct ec_params_gpio_get params;
+ struct ec_response_gpio_get response;
+ int ret;
+ ssize_t copied;
+
+ copied = strscpy(params.name, name, sizeof(params.name));
+ if (copied < 0)
+ return -EINVAL;
+
+ ret = cros_ec_cmd(cros_ec, 0, EC_CMD_GPIO_GET, ¶ms,
+ sizeof(params), &response, sizeof(response));
+ if (ret < 0) {
+ dev_err(gc->parent, "error getting gpio%d (%s) on EC: %d\n", gpio, name, ret);
+ return ret;
+ }
+
+ return response.val;
+}
+
+#define CROS_EC_GPIO_INPUT BIT(8)
+#define CROS_EC_GPIO_OUTPUT BIT(9)
+
+static int cros_ec_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio)
+{
+ const char *name = gc->names[gpio];
+ struct cros_ec_device *cros_ec = gpiochip_get_data(gc);
+ struct ec_params_gpio_get_v1 params = {
+ .subcmd = EC_GPIO_GET_INFO,
+ .get_info.index = gpio,
+ };
+ struct ec_response_gpio_get_v1 response;
+ int ret;
+
+ ret = cros_ec_cmd(cros_ec, 1, EC_CMD_GPIO_GET, ¶ms,
+ sizeof(params), &response, sizeof(response));
+ if (ret < 0) {
+ dev_err(gc->parent, "error getting direction of gpio%d (%s) on EC: %d\n", gpio, name, ret);
+ return ret;
+ }
+
+ if (response.get_info.flags & CROS_EC_GPIO_INPUT)
+ return GPIO_LINE_DIRECTION_IN;
+
+ if (response.get_info.flags & CROS_EC_GPIO_OUTPUT)
+ return GPIO_LINE_DIRECTION_OUT;
+
+ return -EINVAL;
+}
+
+static int cros_ec_gpio_request(struct gpio_chip *chip, unsigned gpio_pin)
+{
+ if (gpio_pin < chip->ngpio)
+ return 0;
+
+ return -EINVAL;
+}
+
+/* Query EC for all gpio line names */
+static int cros_ec_gpio_init_names(struct cros_ec_device *cros_ec, struct gpio_chip *gc)
+{
+ struct ec_params_gpio_get_v1 params = {
+ .subcmd = EC_GPIO_GET_INFO,
+ };
+ struct ec_response_gpio_get_v1 response;
+ int ret, i;
+ /* EC may not NUL terminate */
+ size_t name_len = sizeof(response.get_info.name) + 1;
+ ssize_t copied;
+ const char **names;
+ char *str;
+
+ names = devm_kcalloc(gc->parent, gc->ngpio, sizeof(*names), GFP_KERNEL);
+ if (!names)
+ return -ENOMEM;
+ gc->names = names;
+
+ str = devm_kcalloc(gc->parent, gc->ngpio, name_len, GFP_KERNEL);
+ if (!str)
+ return -ENOMEM;
+
+ /* Get gpio line names one at a time */
+ for (i = 0; i < gc->ngpio; i++) {
+ params.get_info.index = i;
+ ret = cros_ec_cmd(cros_ec, 1, EC_CMD_GPIO_GET, ¶ms,
+ sizeof(params), &response, sizeof(response));
+ if (ret < 0) {
+ dev_err_probe(gc->parent, ret, "error getting gpio%d info\n", i);
+ return ret;
+ }
+
+ names[i] = str;
+ copied = strscpy(str, response.get_info.name, name_len);
+ if (copied < 0)
+ return copied;
+
+ str += copied + 1;
+ }
+
+ return 0;
+}
+
+/* Query EC for number of gpios */
+static int cros_ec_gpio_ngpios(struct cros_ec_device *cros_ec)
+{
+ struct ec_params_gpio_get_v1 params = {
+ .subcmd = EC_GPIO_GET_COUNT,
+ };
+ struct ec_response_gpio_get_v1 response;
+ int ret;
+
+ ret = cros_ec_cmd(cros_ec, 1, EC_CMD_GPIO_GET, ¶ms,
+ sizeof(params), &response, sizeof(response));
+ if (ret < 0)
+ return ret;
+
+ return response.get_count.val;
+}
+
+static int cros_ec_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct cros_ec_device *cros_ec = dev_get_drvdata(dev->parent);
+ struct gpio_chip *gc;
+ int ngpios;
+ int ret;
+
+ ngpios = cros_ec_gpio_ngpios(cros_ec);
+ if (ngpios < 0) {
+ dev_err_probe(dev, ngpios, "error getting gpio count\n");
+ return ngpios;
+ }
+
+ gc = devm_kzalloc(&pdev->dev, sizeof(*gc), GFP_KERNEL);
+ if (!gc)
+ return -ENOMEM;
+
+ gc->ngpio = ngpios;
+ gc->parent = dev;
+ ret = cros_ec_gpio_init_names(cros_ec, gc);
+ if (ret)
+ return ret;
+
+ gc->can_sleep = true;
+ gc->label = dev_name(dev);
+ gc->base = -1;
+ gc->set = cros_ec_gpio_set;
+ gc->get = cros_ec_gpio_get;
+ gc->get_direction = cros_ec_gpio_get_direction;
+ gc->request = cros_ec_gpio_request;
+
+ return devm_gpiochip_add_data(&pdev->dev, gc, cros_ec);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id cros_ec_gpio_of_match[] = {
+ { .compatible = "google,cros-ec-gpio" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, cros_ec_gpio_of_match);
+#endif
+
+static struct platform_driver cros_ec_gpio_driver = {
+ .probe = cros_ec_gpio_probe,
+ .driver = {
+ .name = "cros-ec-gpio",
+ .of_match_table = of_match_ptr(cros_ec_gpio_of_match),
+ },
+};
+module_platform_driver(cros_ec_gpio_driver);
+
+MODULE_DESCRIPTION("ChromeOS EC GPIO Driver");
+MODULE_LICENSE("GPL");
--
https://chromeos.dev
Add a graph with 4 output endpoints to this hub binding to support the
scenario where a downstream facing port is connected to a device that
isn't a connector or a USB device with a VID:PID. This will be used to
connect downstream facing ports to USB type-c switches so the USB
superspeed and high speed lanes can be put onto USB connectors.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: Matthias Kaehlcke <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Cc: maciek swiech <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
.../bindings/usb/realtek,rts5411.yaml | 50 +++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml b/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
index f0784d2e86da..5480a31698be 100644
--- a/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
+++ b/Documentation/devicetree/bindings/usb/realtek,rts5411.yaml
@@ -21,6 +21,12 @@ properties:
reg: true
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
vdd-supply:
description:
phandle to the regulator that provides power to the hub.
@@ -30,6 +36,36 @@ properties:
description:
phandle to the peer hub on the controller.
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ description:
+ 1st downstream facing USB port
+
+ port@2:
+ $ref: /schemas/graph.yaml#/properties/port
+ description:
+ 2nd downstream facing USB port
+
+ port@3:
+ $ref: /schemas/graph.yaml#/properties/port
+ description:
+ 3rd downstream facing USB port
+
+ port@4:
+ $ref: /schemas/graph.yaml#/properties/port
+ description:
+ 4th downstream facing USB port
+
+patternProperties:
+ "^.*@[1-4]$":
+ description: The hard wired USB devices
+ type: object
+ $ref: /schemas/usb/usb-device.yaml
+
required:
- peer-hub
- compatible
@@ -50,6 +86,11 @@ examples:
reg = <1>;
vdd-supply = <&pp3300_hub>;
peer-hub = <&hub_3_0>;
+ /* USB 2.0 device on port 2 */
+ device@2 {
+ compatible = "usb123,4567";
+ reg = <2>;
+ };
};
/* 3.0 hub on port 2 */
@@ -58,5 +99,14 @@ examples:
reg = <2>;
vdd-supply = <&pp3300_hub>;
peer-hub = <&hub_2_0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ remote-endpoint = <&usb_a0_ss>;
+ };
+ };
};
};
--
https://chromeos.dev
When a USB hub is described in DT, such as any device that matches the
onboard-hub driver, the connect_type is set to "unknown" or
USB_PORT_CONNECT_TYPE_UNKNOWN. This makes any device plugged into that
USB port report their 'removable' device attribute as "unknown". Improve
the connect_type attribute for ports, and in turn the removable
attribute for USB devices, by looking for child devices with a reg
property or an OF graph when the device is described in DT.
If the graph exists, endpoints that are connected to a remote node must
be something like a usb-{a,b,c}-connector compatible node, or an
intermediate node like a redriver, and not a hardwired USB device on the
board. Set the connect_type to USB_PORT_CONNECT_TYPE_HOT_PLUG in this
case because the device is going to be plugged in. Set the connect_type
to USB_PORT_CONNECT_TYPE_HARD_WIRED if there's a child node for the port
like 'device@2' for port2. Set the connect_type to USB_PORT_NOT_USED if
there isn't an endpoint or child node corresponding to the port number.
To make sure things don't change, only set the port to not used if
there are child nodes. This way an onboard hub connect_type doesn't
change until ports are added or child nodes are added to describe
hardwired devices. It's assumed that all ports or no ports will be
described for a device.
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Matthias Kaehlcke <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Cc: maciek swiech <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/usb/core/port.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index c628c1abc907..622b8ada157c 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -9,6 +9,8 @@
#include <linux/kstrtox.h>
#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/of_graph.h>
#include <linux/pm_qos.h>
#include <linux/component.h>
@@ -696,7 +698,10 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
{
struct usb_port *port_dev;
struct usb_device *hdev = hub->hdev;
+ struct device_node *np, *child, *ep, *remote_np, *port_np;
int retval;
+ enum usb_port_connect_type connect_type = USB_PORT_CONNECT_TYPE_UNKNOWN;
+ u32 reg;
port_dev = kzalloc(sizeof(*port_dev), GFP_KERNEL);
if (!port_dev)
@@ -708,6 +713,38 @@ int usb_hub_create_port_device(struct usb_hub *hub, int port1)
return -ENOMEM;
}
+ np = hdev->dev.of_node;
+ /* Only set connect_type if binding has ports/hardwired devices. */
+ if (of_get_child_count(np))
+ connect_type = USB_PORT_NOT_USED;
+
+ /* Hotplug ports are connected and available in the OF graph. */
+ if (of_graph_is_present(np)) {
+ port_np = of_graph_get_port_by_id(np, port1);
+ if (port_np) {
+ ep = of_graph_get_endpoint_by_regs(np, port1, -1);
+ if (ep) {
+ remote_np = of_graph_get_remote_port_parent(ep);
+ of_node_put(ep);
+ if (of_device_is_available(remote_np))
+ connect_type = USB_PORT_CONNECT_TYPE_HOT_PLUG;
+ of_node_put(remote_np);
+ }
+ }
+ of_node_put(port_np);
+ }
+
+ /*
+ * Hard-wired ports are child nodes with a reg property corresponding
+ * to the port number.
+ */
+ for_each_available_child_of_node(np, child) {
+ if (!of_property_read_u32(child, "reg", ®) && reg == port1)
+ connect_type = USB_PORT_CONNECT_TYPE_HARD_WIRED;
+ }
+
+ port_dev->connect_type = connect_type;
+
hub->ports[port1 - 1] = port_dev;
port_dev->portnum = port1;
set_bit(port1, hub->power_bits);
--
https://chromeos.dev
Add support to the DRM atomic logic to support lane remapping between
bridges, encoders and connectors. Typically lane mapping is handled
statically in firmware, e.g. on DT we use the data-lanes property to
assign lanes when connecting display bridges. Lane assignment is dynamic
with USB-C DisplayPort altmodes, e.g. pin conf D assigns 2 lanes of DP
to pins on the USB-C connector while pin conf C assigns 4 lanes of DP to
pins on the USB-C connector. The lane assignment can't be set statically
because the DP altmode repurposes USB-C pins for the DP lanes while also
limiting the number of DP lanes or their pin assignment at runtime.
Bridge drivers should point their 'struct drm_bus_cfg::lanes' pointer to
an allocated array of 'struct drm_lane_cfg' structures and indicate the
size of this allocated array with 'struct drm_bus_cfg::num_lanes' in
their atomic_check() callback. The previous bridge in the bridge chain
can look at this information by calling
drm_bridge_next_bridge_lane_cfg() in their atomic_check() callback to
figure out what lanes need to be logically assigned to the physical
output lanes to satisfy the next bridge's lane assignment.
Cc: Andrzej Hajda <[email protected]>
Cc: Neil Armstrong <[email protected]>
Cc: Robert Foss <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Jonas Karlman <[email protected]>
Cc: Jernej Skrabec <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/gpu/drm/drm_atomic_state_helper.c | 2 ++
drivers/gpu/drm/drm_bridge.c | 34 +++++++++++++++++++++++
include/drm/drm_atomic.h | 31 +++++++++++++++++++++
include/drm/drm_bridge.h | 4 +++
4 files changed, 71 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c
index 784e63d70a42..2e989fbeb503 100644
--- a/drivers/gpu/drm/drm_atomic_state_helper.c
+++ b/drivers/gpu/drm/drm_atomic_state_helper.c
@@ -764,6 +764,8 @@ EXPORT_SYMBOL(drm_atomic_helper_bridge_duplicate_state);
void drm_atomic_helper_bridge_destroy_state(struct drm_bridge *bridge,
struct drm_bridge_state *state)
{
+ kfree(state->input_bus_cfg.lanes);
+ kfree(state->output_bus_cfg.lanes);
kfree(state);
}
EXPORT_SYMBOL(drm_atomic_helper_bridge_destroy_state);
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 30d66bee0ec6..3fce0d8d7dcb 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -843,6 +843,40 @@ void drm_atomic_bridge_chain_enable(struct drm_bridge *bridge,
}
EXPORT_SYMBOL(drm_atomic_bridge_chain_enable);
+/**
+ * drm_bridge_next_bridge_lane_cfg - get the lane configuration of the next bridge
+ * @bridge: bridge control structure
+ * @state: new atomic state
+ * @num_lanes: will contain the size of the returned array
+ *
+ * This function is typically called from &drm_bridge_funcs.atomic_check().
+ * The @bridge driver calls this function to determine what the next bridge in
+ * the bridge chain requires for the physical to logical lane assignments.
+ *
+ * Return: Lane configuration array of size @num_lanes for the next bridge
+ * after @bridge in the bridge chain, or NULL if the lane configuration is
+ * unchanged from the default.
+ */
+const struct drm_lane_cfg *
+drm_bridge_next_bridge_lane_cfg(struct drm_bridge *bridge,
+ struct drm_atomic_state *state,
+ u8 *num_lanes)
+{
+ const struct drm_bridge_state *next_bridge_state;
+ struct drm_bridge *next_bridge = drm_bridge_get_next_bridge(bridge);
+
+ next_bridge_state = drm_atomic_get_new_bridge_state(state, next_bridge);
+ if (!next_bridge_state) {
+ *num_lanes = 0;
+ return NULL;
+ }
+
+ *num_lanes = next_bridge_state->input_bus_cfg.num_lanes;
+
+ return next_bridge_state->input_bus_cfg.lanes;
+}
+EXPORT_SYMBOL(drm_bridge_next_bridge_lane_cfg);
+
static int drm_atomic_bridge_check(struct drm_bridge *bridge,
struct drm_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index cf8e1220a4ac..b206ae2654d8 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -1074,6 +1074,27 @@ drm_atomic_crtc_effectively_active(const struct drm_crtc_state *state)
return state->active || state->self_refresh_active;
}
+/**
+ * struct drm_lane_cfg - lane configuration
+ *
+ * This structure stores the lange configuration of a physical bus between
+ * two components in an output pipeline, usually between two bridges, an
+ * encoder and a bridge, or a bridge and a connector.
+ *
+ * The lane configuration is stored in &drm_bus_cfg.
+ */
+struct drm_lane_cfg {
+ /**
+ * @logical: Logical lane number
+ */
+ u8 logical;
+
+ /**
+ * @inverted: True if lane polarity is inverted, false otherwise
+ */
+ bool inverted;
+};
+
/**
* struct drm_bus_cfg - bus configuration
*
@@ -1104,6 +1125,16 @@ struct drm_bus_cfg {
* @flags: DRM_BUS_* flags used on this bus
*/
u32 flags;
+
+ /**
+ * @lanes: Lane mapping for this bus
+ */
+ struct drm_lane_cfg *lanes;
+
+ /**
+ * @num_lanes: Number of lanes in @lanes
+ */
+ u8 num_lanes;
};
/**
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index cfb7dcdb66c4..5f64f6e822e1 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -884,6 +884,10 @@ drm_atomic_helper_bridge_propagate_bus_fmt(struct drm_bridge *bridge,
struct drm_connector_state *conn_state,
u32 output_fmt,
unsigned int *num_input_fmts);
+const struct drm_lane_cfg *
+drm_bridge_next_bridge_lane_cfg(struct drm_bridge *bridge,
+ struct drm_atomic_state *state,
+ u8 *num_lanes);
enum drm_connector_status drm_bridge_detect(struct drm_bridge *bridge);
int drm_bridge_get_modes(struct drm_bridge *bridge,
--
https://chromeos.dev
Verify during drm_atomic_bridge_check() that the lane assignment set in
a bridge's atomic_check() callback is going to be satisfied by the
previous bridge. If the next bridge is requiring something besides the
default 1:1 lane assignment on its input then there must be an output
lane assignment on the previous bridge's output. Otherwise the next
bridge won't get the lanes assigned that it needs.
Cc: Andrzej Hajda <[email protected]>
Cc: Neil Armstrong <[email protected]>
Cc: Robert Foss <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Jonas Karlman <[email protected]>
Cc: Jernej Skrabec <[email protected]>
Cc: Maarten Lankhorst <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Thomas Zimmermann <[email protected]>
Cc: David Airlie <[email protected]>
Cc: Daniel Vetter <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/gpu/drm/drm_bridge.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 3fce0d8d7dcb..5097e7c65ddf 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -881,6 +881,10 @@ static int drm_atomic_bridge_check(struct drm_bridge *bridge,
struct drm_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
{
+ u8 num_input_lanes, num_output_lanes = 0;
+ const struct drm_lane_cfg *input_lanes;
+ int i;
+
if (bridge->funcs->atomic_check) {
struct drm_bridge_state *bridge_state;
int ret;
@@ -894,12 +898,24 @@ static int drm_atomic_bridge_check(struct drm_bridge *bridge,
crtc_state, conn_state);
if (ret)
return ret;
+ num_output_lanes = bridge_state->output_bus_cfg.num_lanes;
} else if (bridge->funcs->mode_fixup) {
if (!bridge->funcs->mode_fixup(bridge, &crtc_state->mode,
&crtc_state->adjusted_mode))
return -EINVAL;
}
+ input_lanes = drm_bridge_next_bridge_lane_cfg(bridge,
+ crtc_state->state,
+ &num_input_lanes);
+ /*
+ * Ensure this bridge is aware that the next bridge wants to
+ * reassign lanes.
+ */
+ for (i = 0; i < num_input_lanes; i++)
+ if (i != input_lanes[i].logical && !num_output_lanes)
+ return -ENOTSUPP;
+
return 0;
}
--
https://chromeos.dev
From: Prashant Malani <[email protected]>
When using OF graph, the fw_devlink code will create links between the
individual port driver (cros-ec-typec here) and the parent device for
a Type-C switch (like mode-switch). Since the mode-switch will in turn
have the usb-c-connector (i.e the child of the port driver) as a
supplier, fw_devlink will not be able to resolve the cyclic dependency
correctly.
As a result, the mode-switch driver probe() never runs, so mode-switches
are never registered. Because of that, the port driver probe constantly
fails with -EPROBE_DEFER, because the Type-C connector class requires all
switch devices to be registered prior to port registration.
To break this deadlock and allow the mode-switch registration to occur,
purge all the usb-c-connector nodes' absent suppliers. This eliminates
the connector as a supplier for a switch and allows it to be probed.
Signed-off-by: Prashant Malani <[email protected]>
Signed-off-by: Pin-yen Lin <[email protected]>
Reviewed-by: Chen-Yu Tsai <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Tested-by: Chen-Yu Tsai <[email protected]>
Acked-by: Heikki Krogerus <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/platform/chrome/cros_ec_typec.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_typec.c b/drivers/platform/chrome/cros_ec_typec.c
index 2b2f14a1b711..cc5269a4b2f1 100644
--- a/drivers/platform/chrome/cros_ec_typec.c
+++ b/drivers/platform/chrome/cros_ec_typec.c
@@ -325,6 +325,16 @@ static int cros_typec_init_ports(struct cros_typec_data *typec)
return -EINVAL;
}
+ /*
+ * OF graph may have set up some device links with switches,
+ * since connectors have their own compatible. Purge these
+ * to avoid a deadlock in switch probe (the switch mistakenly
+ * assumes the connector is a supplier).
+ */
+ if (dev_of_node(dev))
+ device_for_each_child_node(dev, fwnode)
+ fw_devlink_purge_absent_suppliers(fwnode);
+
/* DT uses "reg" to specify port number. */
port_prop = dev->of_node ? "reg" : "port-number";
device_for_each_child_node(dev, fwnode) {
--
https://chromeos.dev
When a single DT node has a graph connected to more than one
usb-c-connector node we can't differentiate which typec switch
registered for the device is associated with the USB connector because
the devcon matcher code assumes a 1:1 relationship between remote node
and typec switch. Furthermore, we don't have a #typec-switch-cells
property so there can only be one node per typec switch.
Support multiple USB typec switches exposed by one node by passing the
remote endpoint node in addition to the remote node to the devcon
matcher function (devcon_match_fn_t). With this change, typec switch
drivers can register switches with the device node pointer for a graph
endpoint so that they can support more than one typec switch if
necessary. Either way, a DT property like 'mode-switch' is always in the
graph's parent node and not in the endpoint node.
Cc: Andy Shevchenko <[email protected]>
Cc: Daniel Scally <[email protected]>
Cc: Heikki Krogerus <[email protected]>
Cc: Sakari Ailus <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Vinod Koul <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Mika Westerberg <[email protected]>
Cc: Alexandre Belloni <[email protected]>
Cc: Ivan Orlov <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/base/property.c | 7 +++++--
drivers/usb/roles/class.c | 4 ++--
drivers/usb/typec/mux.c | 8 ++++++++
drivers/usb/typec/retimer.c | 7 ++++++-
include/linux/property.h | 5 +++--
5 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/base/property.c b/drivers/base/property.c
index 8c40abed7852..cae81ed4e298 100644
--- a/drivers/base/property.c
+++ b/drivers/base/property.c
@@ -1289,6 +1289,7 @@ static unsigned int fwnode_graph_devcon_matches(const struct fwnode_handle *fwno
{
struct fwnode_handle *node;
struct fwnode_handle *ep;
+ struct fwnode_handle *remote_ep;
unsigned int count = 0;
void *ret;
@@ -1304,7 +1305,9 @@ static unsigned int fwnode_graph_devcon_matches(const struct fwnode_handle *fwno
continue;
}
- ret = match(node, con_id, data);
+ remote_ep = fwnode_graph_get_remote_endpoint(ep);
+ ret = match(node, remote_ep, con_id, data);
+ fwnode_handle_put(remote_ep);
fwnode_handle_put(node);
if (ret) {
if (matches)
@@ -1334,7 +1337,7 @@ static unsigned int fwnode_devcon_matches(const struct fwnode_handle *fwnode,
if (IS_ERR(node))
break;
- ret = match(node, NULL, data);
+ ret = match(node, NULL, NULL, data);
fwnode_handle_put(node);
if (ret) {
if (matches)
diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index ae41578bd014..9a0ef5fa0a19 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -89,8 +89,8 @@ enum usb_role usb_role_switch_get_role(struct usb_role_switch *sw)
}
EXPORT_SYMBOL_GPL(usb_role_switch_get_role);
-static void *usb_role_switch_match(const struct fwnode_handle *fwnode, const char *id,
- void *data)
+static void *usb_role_switch_match(const struct fwnode_handle *fwnode, const struct fwnode_handle *endpoint,
+ const char *id, void *data)
{
struct device *dev;
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index 80dd91938d96..3eabd0d62f47 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -33,6 +33,7 @@ static int switch_fwnode_match(struct device *dev, const void *fwnode)
}
static void *typec_switch_match(const struct fwnode_handle *fwnode,
+ const struct fwnode_handle *endpoint,
const char *id, void *data)
{
struct device *dev;
@@ -55,6 +56,9 @@ static void *typec_switch_match(const struct fwnode_handle *fwnode,
*/
dev = class_find_device(&typec_mux_class, NULL, fwnode,
switch_fwnode_match);
+ if (!dev)
+ dev = class_find_device(&typec_mux_class, NULL, endpoint,
+ switch_fwnode_match);
return dev ? to_typec_switch_dev(dev) : ERR_PTR(-EPROBE_DEFER);
}
@@ -263,6 +267,7 @@ static int mux_fwnode_match(struct device *dev, const void *fwnode)
}
static void *typec_mux_match(const struct fwnode_handle *fwnode,
+ const struct fwnode_handle *endpoint,
const char *id, void *data)
{
struct device *dev;
@@ -280,6 +285,9 @@ static void *typec_mux_match(const struct fwnode_handle *fwnode,
dev = class_find_device(&typec_mux_class, NULL, fwnode,
mux_fwnode_match);
+ if (!dev)
+ dev = class_find_device(&typec_mux_class, NULL, endpoint,
+ mux_fwnode_match);
return dev ? to_typec_mux_dev(dev) : ERR_PTR(-EPROBE_DEFER);
}
diff --git a/drivers/usb/typec/retimer.c b/drivers/usb/typec/retimer.c
index 4a7d1b5c4d86..eb74abee6619 100644
--- a/drivers/usb/typec/retimer.c
+++ b/drivers/usb/typec/retimer.c
@@ -22,7 +22,9 @@ static int retimer_fwnode_match(struct device *dev, const void *fwnode)
return is_typec_retimer(dev) && device_match_fwnode(dev, fwnode);
}
-static void *typec_retimer_match(const struct fwnode_handle *fwnode, const char *id, void *data)
+static void *typec_retimer_match(const struct fwnode_handle *fwnode,
+ const struct fwnode_handle *endpoint,
+ const char *id, void *data)
{
struct device *dev;
@@ -31,6 +33,9 @@ static void *typec_retimer_match(const struct fwnode_handle *fwnode, const char
dev = class_find_device(&retimer_class, NULL, fwnode,
retimer_fwnode_match);
+ if (!dev)
+ dev = class_find_device(&retimer_class, NULL, endpoint,
+ retimer_fwnode_match);
return dev ? to_typec_retimer(dev) : ERR_PTR(-EPROBE_DEFER);
}
diff --git a/include/linux/property.h b/include/linux/property.h
index 9f2585d705a8..0f20df1f0a49 100644
--- a/include/linux/property.h
+++ b/include/linux/property.h
@@ -455,8 +455,9 @@ unsigned int fwnode_graph_get_endpoint_count(const struct fwnode_handle *fwnode,
int fwnode_graph_parse_endpoint(const struct fwnode_handle *fwnode,
struct fwnode_endpoint *endpoint);
-typedef void *(*devcon_match_fn_t)(const struct fwnode_handle *fwnode, const char *id,
- void *data);
+typedef void *(*devcon_match_fn_t)(const struct fwnode_handle *fwnode,
+ const struct fwnode_handle *endpoint,
+ const char *id, void *data);
void *fwnode_connection_find_match(const struct fwnode_handle *fwnode,
const char *con_id, void *data,
--
https://chromeos.dev
It's possible for this polling loop to get scheduled away for a long
time right after checking the status on the EC. If that happens, we will
never try to check the status at least one more time before giving up
and saying that it timed out. Let's use the read_poll_timeout() macro to
construct the loop with a proper timeout mechanism and the ability to
check that the condition is true once more when the timeout hits.
Cc: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Tzung-Bi Shih <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/platform/chrome/cros_typec_switch.c | 21 ++++++++-------------
1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
index 07a19386dc4e..a23fe80d9d4b 100644
--- a/drivers/platform/chrome/cros_typec_switch.c
+++ b/drivers/platform/chrome/cros_typec_switch.c
@@ -8,7 +8,7 @@
#include <linux/acpi.h>
#include <linux/delay.h>
-#include <linux/jiffies.h>
+#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/platform_data/cros_ec_commands.h>
#include <linux/platform_data/cros_ec_proto.h>
@@ -108,7 +108,6 @@ static bool cros_typec_check_event(struct cros_typec_switch_data *sdata, int por
static int cros_typec_configure_mux(struct cros_typec_switch_data *sdata, int port_num, int index,
unsigned long mode, struct typec_altmode *alt)
{
- unsigned long end;
u32 event_mask;
u8 mux_state;
int ret;
@@ -134,18 +133,14 @@ static int cros_typec_configure_mux(struct cros_typec_switch_data *sdata, int po
return ret;
/* Check for the mux set done event. */
- end = jiffies + msecs_to_jiffies(1000);
- do {
- if (cros_typec_check_event(sdata, port_num, event_mask))
- return 0;
+ if (read_poll_timeout(cros_typec_check_event, ret, ret == 0, 1000,
+ 1000 * 1000UL, false, sdata, port_num, event_mask)) {
+ dev_err(sdata->dev, "Timed out waiting for mux set done on index: %d, state: %d\n",
+ index, mux_state);
+ return -ETIMEDOUT;
+ }
- usleep_range(500, 1000);
- } while (time_before(jiffies, end));
-
- dev_err(sdata->dev, "Timed out waiting for mux set done on index: %d, state: %d\n",
- index, mux_state);
-
- return -ETIMEDOUT;
+ return 0;
}
static int cros_typec_mode_switch_set(struct typec_mux_dev *mode_switch,
--
https://chromeos.dev
This driver will soon support devicetree firmwares. Extract the struct
cros_typec_port creation part of cros_typec_register_switches() into a
sub-function so that we can extend it for DT, where a graph is used
instead of child nodes.
Cc: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Tzung-Bi Shih <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/platform/chrome/cros_typec_switch.c | 113 +++++++++++---------
1 file changed, 62 insertions(+), 51 deletions(-)
diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
index a23fe80d9d4b..1a718b661203 100644
--- a/drivers/platform/chrome/cros_typec_switch.c
+++ b/drivers/platform/chrome/cros_typec_switch.c
@@ -201,13 +201,69 @@ static int cros_typec_register_retimer(struct cros_typec_port *port, struct fwno
return PTR_ERR_OR_ZERO(port->retimer);
}
-static int cros_typec_register_switches(struct cros_typec_switch_data *sdata)
+static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
+ struct fwnode_handle *fwnode)
{
struct cros_typec_port *port;
struct device *dev = sdata->dev;
- struct fwnode_handle *fwnode;
struct acpi_device *adev;
unsigned long long index;
+ int ret;
+
+ port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
+ if (!port)
+ return -ENOMEM;
+
+ adev = to_acpi_device_node(fwnode);
+ if (adev) {
+ ret = acpi_evaluate_integer(adev->handle, "_ADR", NULL, &index);
+ if (ACPI_FAILURE(ret)) {
+ dev_err(fwnode->dev, "_ADR wasn't evaluated\n");
+ return -ENODATA;
+ }
+ }
+
+ if (!adev) {
+ dev_err(fwnode->dev, "Couldn't get ACPI handle\n");
+ return -ENODEV;
+ }
+
+ if (index >= EC_USB_PD_MAX_PORTS) {
+ dev_err(fwnode->dev, "Invalid port index number: %llu\n", index);
+ return -EINVAL;
+ }
+ port->sdata = sdata;
+ port->port_num = index;
+ sdata->ports[index] = port;
+
+ if (fwnode_property_present(fwnode, "retimer-switch")) {
+ ret = cros_typec_register_retimer(port, fwnode);
+ if (ret) {
+ dev_err(dev, "Retimer switch register failed\n");
+ return ret;
+ }
+
+ dev_dbg(dev, "Retimer switch registered for index %llu\n", index);
+ }
+
+ if (!fwnode_property_present(fwnode, "mode-switch"))
+ return 0;
+
+ ret = cros_typec_register_mode_switch(port, fwnode);
+ if (ret) {
+ dev_err(dev, "Mode switch register failed\n");
+ return ret;
+ }
+
+ dev_dbg(dev, "Mode switch registered for index %llu\n", index);
+
+ return ret;
+}
+
+static int cros_typec_register_switches(struct cros_typec_switch_data *sdata)
+{
+ struct device *dev = sdata->dev;
+ struct fwnode_handle *fwnode;
int nports, ret;
nports = device_get_child_node_count(dev);
@@ -217,60 +273,15 @@ static int cros_typec_register_switches(struct cros_typec_switch_data *sdata)
}
device_for_each_child_node(dev, fwnode) {
- port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
- if (!port) {
- ret = -ENOMEM;
- goto err_switch;
- }
-
- adev = to_acpi_device_node(fwnode);
- if (!adev) {
- dev_err(fwnode->dev, "Couldn't get ACPI device handle\n");
- ret = -ENODEV;
- goto err_switch;
- }
-
- ret = acpi_evaluate_integer(adev->handle, "_ADR", NULL, &index);
- if (ACPI_FAILURE(ret)) {
- dev_err(fwnode->dev, "_ADR wasn't evaluated\n");
- ret = -ENODATA;
- goto err_switch;
- }
-
- if (index >= EC_USB_PD_MAX_PORTS) {
- dev_err(fwnode->dev, "Invalid port index number: %llu\n", index);
- ret = -EINVAL;
- goto err_switch;
- }
- port->sdata = sdata;
- port->port_num = index;
- sdata->ports[index] = port;
-
- if (fwnode_property_present(fwnode, "retimer-switch")) {
- ret = cros_typec_register_retimer(port, fwnode);
- if (ret) {
- dev_err(dev, "Retimer switch register failed\n");
- goto err_switch;
- }
-
- dev_dbg(dev, "Retimer switch registered for index %llu\n", index);
- }
-
- if (!fwnode_property_present(fwnode, "mode-switch"))
- continue;
-
- ret = cros_typec_register_mode_switch(port, fwnode);
+ ret = cros_typec_register_port(sdata, fwnode);
if (ret) {
- dev_err(dev, "Mode switch register failed\n");
- goto err_switch;
+ fwnode_handle_put(fwnode);
+ goto err;
}
-
- dev_dbg(dev, "Mode switch registered for index %llu\n", index);
}
return 0;
-err_switch:
- fwnode_handle_put(fwnode);
+err:
cros_typec_unregister_switches(sdata);
return ret;
}
--
https://chromeos.dev
Use dev_err_probe() helper so we can get better diagnostics when driver
probes fails for any reason.
Cc: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Tzung-Bi Shih <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/platform/chrome/cros_typec_switch.c | 36 +++++++--------------
1 file changed, 12 insertions(+), 24 deletions(-)
diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
index 373e0e86ebfc..769de2889f2f 100644
--- a/drivers/platform/chrome/cros_typec_switch.c
+++ b/drivers/platform/chrome/cros_typec_switch.c
@@ -219,31 +219,23 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
if (adev)
prop_name = "_ADR";
- if (!adev) {
- dev_err(fwnode->dev, "Couldn't get ACPI handle\n");
- return -ENODEV;
- }
+ if (!adev)
+ return dev_err_probe(fwnode->dev, -ENODEV, "Couldn't get ACPI handle\n");
ret = fwnode_property_read_u32(fwnode, prop_name, &index);
- if (ret) {
- dev_err(fwnode->dev, "%s property wasn't found\n", prop_name);
- return ret;
- }
+ if (ret)
+ return dev_err_probe(fwnode->dev, ret, "%s property wasn't found\n", prop_name);
- if (index >= EC_USB_PD_MAX_PORTS) {
- dev_err(fwnode->dev, "Invalid port index number: %u\n", index);
- return -EINVAL;
- }
+ if (index >= EC_USB_PD_MAX_PORTS)
+ return dev_err_probe(fwnode->dev, -EINVAL, "Invalid port index number: %u\n", index);
port->sdata = sdata;
port->port_num = index;
sdata->ports[index] = port;
if (fwnode_property_present(fwnode, "retimer-switch")) {
ret = cros_typec_register_retimer(port, fwnode);
- if (ret) {
- dev_err(dev, "Retimer switch register failed\n");
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "Retimer switch register failed\n");
dev_dbg(dev, "Retimer switch registered for index %u\n", index);
}
@@ -252,10 +244,8 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
return 0;
ret = cros_typec_register_mode_switch(port, fwnode);
- if (ret) {
- dev_err(dev, "Mode switch register failed\n");
- return ret;
- }
+ if (ret)
+ return dev_err_probe(dev, ret, "Mode switch register failed\n");
dev_dbg(dev, "Mode switch registered for index %u\n", index);
@@ -269,10 +259,8 @@ static int cros_typec_register_switches(struct cros_typec_switch_data *sdata)
int nports, ret;
nports = device_get_child_node_count(dev);
- if (nports == 0) {
- dev_err(dev, "No switch devices found.\n");
- return -ENODEV;
- }
+ if (nports == 0)
+ return dev_err_probe(dev, -ENODEV, "No switch devices found\n");
device_for_each_child_node(dev, fwnode) {
ret = cros_typec_register_port(sdata, fwnode);
--
https://chromeos.dev
Add a binding for the USB type-c switch controls found on some ChromeOS
Embedded Controllers (ECs). When this device is a mode switch, it takes
one DisplayPort (DP) port as input and some number (possibly zero) of
USB SuperSpeed ports (bundles of USB SS lanes) as input, and muxes those
lanes into USB type-c SuperSpeed lanes suitable for the SSTRX1/2 pins on
a usb-c-connector. When this device is an orientation switch, it
redirects the DP lanes to the proper USB type-c SSTRX lanes.
Cc: Rob Herring <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: Lee Jones <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: Prashant Malani <[email protected]>
Cc: Tzung-Bi Shih <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
.../chrome/google,cros-ec-typec-switch.yaml | 365 ++++++++++++++++++
.../bindings/mfd/google,cros-ec.yaml | 5 +
2 files changed, 370 insertions(+)
create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
new file mode 100644
index 000000000000..17a0ba928f5d
--- /dev/null
+++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
@@ -0,0 +1,365 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/chrome/google,cros-ec-typec-switch.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Google Chrome OS EC(Embedded Controller) USB Type C Switch
+
+maintainers:
+ - Benson Leung <[email protected]>
+ - Prashant Malani <[email protected]>
+ - Stephen Boyd <[email protected]>
+
+description:
+ Chrome OS devices have an Embedded Controller(EC) which has access to USB
+ Type C switching. This node is intended to allow the OS to control Type C
+ signal muxing for USB-C orientation and alternate modes. The node for this
+ device should be under a cros-ec node like google,cros-ec-spi.
+
+properties:
+ compatible:
+ const: google,cros-ec-typec-switch
+
+ mode-switch:
+ description: Indicates this device controls altmode switching
+ type: boolean
+
+ orientation-switch:
+ description: Indicates this device controls orientation switching
+ type: boolean
+
+ mux-gpios:
+ description: GPIOs indicating which way the DP mux is steered
+
+ no-hpd:
+ description: Indicates this device doesn't signal HPD for DisplayPort
+ type: boolean
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ description: Input port to receive DisplayPort (DP) data
+ unevaluatedProperties: false
+
+ properties:
+ endpoint@0:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ description: DisplayPort data
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ An array of physical DP data lane indexes
+ - 0 is DP ML0 lane
+ - 1 is DP ML1 lane
+ - 2 is DP ML2 lane
+ - 3 is DP ML3 lane
+ oneOf:
+ - items:
+ - const: 0
+ - const: 1
+ - items:
+ - const: 0
+ - const: 1
+ - const: 2
+ - const: 3
+
+ required:
+ - endpoint@0
+
+ port@1:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Input port to receive USB SuperSpeed (SS) data
+ properties:
+ endpoint@0:
+ $ref: /schemas/graph.yaml#/properties/endpoint
+ description: USB SS data
+
+ endpoint@1:
+ $ref: /schemas/graph.yaml#/properties/endpoint
+ description: USB SS data
+
+ anyOf:
+ - required:
+ - endpoint@0
+ - required:
+ - endpoint@1
+
+ port@2:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description:
+ Output port for USB-C data
+ properties:
+ endpoint@0:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ description: USB-C data
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ An array of physical USB-C data lane indexes.
+ - 0 is SSRX1 lane
+ - 1 is SSTX1 lane
+ - 2 is SSTX2 lane
+ - 3 is SSRX2 lane
+ minItems: 4
+ maxItems: 4
+ items:
+ maximum: 3
+
+ endpoint@1:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ description: USB-C data for EC's 1st type-c port
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ An array of physical USB-C data lane indexes.
+ - 0 is SSRX1 lane
+ - 1 is SSTX1 lane
+ - 2 is SSTX2 lane
+ - 3 is SSRX2 lane
+ minItems: 4
+ maxItems: 4
+ items:
+ maximum: 3
+
+ endpoint@2:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ description: USB-C data for EC's 2nd type-c port
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ An array of physical USB-C data lane indexes.
+ - 0 is SSRX1 lane
+ - 1 is SSTX1 lane
+ - 2 is SSTX2 lane
+ - 3 is SSRX2 lane
+ minItems: 4
+ maxItems: 4
+ items:
+ maximum: 3
+
+ endpoint@3:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ description: USB-C data for EC's 3rd type-c port
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ An array of physical USB-C data lane indexes.
+ - 0 is SSRX1 lane
+ - 1 is SSTX1 lane
+ - 2 is SSTX2 lane
+ - 3 is SSRX2 lane
+ minItems: 4
+ maxItems: 4
+ items:
+ maximum: 3
+
+ endpoint@4:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ description: USB-C data for EC's 4th type-c port
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ An array of physical USB-C data lane indexes.
+ - 0 is SSRX1 lane
+ - 1 is SSTX1 lane
+ - 2 is SSTX2 lane
+ - 3 is SSRX2 lane
+ minItems: 4
+ maxItems: 4
+ items:
+ maximum: 3
+
+ endpoint@5:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ description: USB-C data for EC's 5th type-c port
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ An array of physical USB-C data lane indexes.
+ - 0 is SSRX1 lane
+ - 1 is SSTX1 lane
+ - 2 is SSTX2 lane
+ - 3 is SSRX2 lane
+ minItems: 4
+ maxItems: 4
+ items:
+ maximum: 3
+
+ endpoint@6:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ description: USB-C data for EC's 6th type-c port
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ An array of physical USB-C data lane indexes.
+ - 0 is SSRX1 lane
+ - 1 is SSTX1 lane
+ - 2 is SSTX2 lane
+ - 3 is SSRX2 lane
+ minItems: 4
+ maxItems: 4
+ items:
+ maximum: 3
+
+ endpoint@7:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ description: USB-C data for EC's 7th type-c port
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ An array of physical USB-C data lane indexes.
+ - 0 is SSRX1 lane
+ - 1 is SSTX1 lane
+ - 2 is SSTX2 lane
+ - 3 is SSRX2 lane
+ minItems: 4
+ maxItems: 4
+ items:
+ maximum: 3
+
+ anyOf:
+ - required:
+ - endpoint@0
+ - required:
+ - endpoint@1
+ - required:
+ - endpoint@2
+ - required:
+ - endpoint@3
+ - required:
+ - endpoint@4
+ - required:
+ - endpoint@5
+ - required:
+ - endpoint@6
+ - required:
+ - endpoint@7
+
+ required:
+ - port@2
+ anyOf:
+ - required:
+ - port@0
+ - required:
+ - port@1
+
+required:
+ - compatible
+ - ports
+
+allOf:
+ - if:
+ properties:
+ no-hpd: true
+ required:
+ - no-hpd
+ then:
+ properties:
+ ports:
+ required:
+ - port@0
+ - if:
+ properties:
+ mode-switch: true
+ required:
+ - mode-switch
+ then:
+ properties:
+ ports:
+ required:
+ - port@0
+
+additionalProperties: false
+
+examples:
+ - |
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cros_ec: ec@0 {
+ compatible = "google,cros-ec-spi";
+ reg = <0>;
+ interrupts = <35 0>;
+
+ typec-switch {
+ compatible = "google,cros-ec-typec-switch";
+ mode-switch;
+ orientation-switch;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ dp_in: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&dp_phy>;
+ data-lanes = <0 1>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb_in_0: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&usb_ss_0_out>;
+ };
+
+ usb_in_1: endpoint@1 {
+ reg = <1>;
+ remote-endpoint = <&usb_ss_1_out>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cros_typec_c0_ss: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&usb_c0_ss>;
+ };
+
+ cros_typec_c1_ss: endpoint@1 {
+ reg = <1>;
+ remote-endpoint = <&usb_c1_ss>;
+ };
+ };
+ };
+ };
+ };
+ };
+...
diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
index ded396b28fba..563c51a4a39c 100644
--- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
+++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
@@ -164,6 +164,10 @@ patternProperties:
type: object
$ref: /schemas/extcon/extcon-usbc-cros-ec.yaml#
+ "^typec-switch[0-9]*$":
+ type: object
+ $ref: /schemas/chrome/google,cros-ec-typec-switch.yaml#
+
required:
- compatible
@@ -227,6 +231,7 @@ allOf:
"^i2c-tunnel[0-9]*$": false
"^regulator@[0-9]+$": false
"^extcon[0-9]*$": false
+ "^typec-switch[0-9]*$": false
# Using additionalProperties: false here and
# listing true properties doesn't work
--
https://chromeos.dev
Use fwnode APIs instead of ACPI ones because this driver will soon
support devicetree firmwares. Using fwnode APIs makes it easier to
support either ACPI or DT.
Cc: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Tzung-Bi Shih <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/platform/chrome/cros_typec_switch.c | 24 +++++++++++----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
index 1a718b661203..373e0e86ebfc 100644
--- a/drivers/platform/chrome/cros_typec_switch.c
+++ b/drivers/platform/chrome/cros_typec_switch.c
@@ -207,29 +207,31 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
struct cros_typec_port *port;
struct device *dev = sdata->dev;
struct acpi_device *adev;
- unsigned long long index;
+ u32 index;
int ret;
+ const char *prop_name;
port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
if (!port)
return -ENOMEM;
adev = to_acpi_device_node(fwnode);
- if (adev) {
- ret = acpi_evaluate_integer(adev->handle, "_ADR", NULL, &index);
- if (ACPI_FAILURE(ret)) {
- dev_err(fwnode->dev, "_ADR wasn't evaluated\n");
- return -ENODATA;
- }
- }
+ if (adev)
+ prop_name = "_ADR";
if (!adev) {
dev_err(fwnode->dev, "Couldn't get ACPI handle\n");
return -ENODEV;
}
+ ret = fwnode_property_read_u32(fwnode, prop_name, &index);
+ if (ret) {
+ dev_err(fwnode->dev, "%s property wasn't found\n", prop_name);
+ return ret;
+ }
+
if (index >= EC_USB_PD_MAX_PORTS) {
- dev_err(fwnode->dev, "Invalid port index number: %llu\n", index);
+ dev_err(fwnode->dev, "Invalid port index number: %u\n", index);
return -EINVAL;
}
port->sdata = sdata;
@@ -243,7 +245,7 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
return ret;
}
- dev_dbg(dev, "Retimer switch registered for index %llu\n", index);
+ dev_dbg(dev, "Retimer switch registered for index %u\n", index);
}
if (!fwnode_property_present(fwnode, "mode-switch"))
@@ -255,7 +257,7 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
return ret;
}
- dev_dbg(dev, "Mode switch registered for index %llu\n", index);
+ dev_dbg(dev, "Mode switch registered for index %u\n", index);
return ret;
}
--
https://chromeos.dev
Register an orientation switch with the typec subsystem when the
'orientation-switch' DT property is present. In these sorts of hardware
designs, the DP phy lanes are wired directly to the usb-c-connector. The
EC signals entry and exit of DP mode on the port, and the DP phy lanes
are assigned to the pins based on the port orientation (normal or
reverse).
Stash the orientation of the port and simply wait for the atomic_check
phase to request the desired DP lane assignment (normal or reverse).
Don't restrict this to the presence of the mode-switch property because
it's possible for this device to only be an orientation-switch, in which
case the DP mode entry is signaled externally (e.g. through an HPD pin
on the DP source).
Cc: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Tzung-Bi Shih <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/platform/chrome/cros_typec_switch.c | 84 ++++++++++++++++++---
1 file changed, 72 insertions(+), 12 deletions(-)
diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
index adcbf8f44c98..c22c2531327a 100644
--- a/drivers/platform/chrome/cros_typec_switch.c
+++ b/drivers/platform/chrome/cros_typec_switch.c
@@ -38,9 +38,11 @@ struct cros_typec_dp_bridge {
struct cros_typec_port {
int port_num;
struct typec_mux_dev *mode_switch;
+ struct typec_switch_dev *orientation_switch;
struct typec_retimer *retimer;
size_t num_dp_lanes;
u32 lane_mapping[USBC_LANES_COUNT];
+ enum typec_orientation orientation;
struct cros_typec_switch_data *sdata;
};
@@ -245,6 +247,21 @@ static int cros_typec_mode_switch_set(struct typec_mux_dev *mode_switch,
return 0;
}
+static int cros_typec_dp_port_orientation_set(struct typec_switch_dev *sw,
+ enum typec_orientation orientation)
+{
+ struct cros_typec_port *port = typec_switch_get_drvdata(sw);
+
+ /*
+ * Lane remapping is in cros_typec_dp_bridge_atomic_check(). Whenever
+ * an orientation changes HPD will go low and then high again so the
+ * atomic check handles the orientation change.
+ */
+ port->orientation = orientation;
+
+ return 0;
+}
+
static int cros_typec_retimer_set(struct typec_retimer *retimer, struct typec_retimer_state *state)
{
struct cros_typec_port *port = typec_retimer_get_drvdata(retimer);
@@ -280,6 +297,21 @@ static int cros_typec_register_mode_switch(struct cros_typec_port *port,
return PTR_ERR_OR_ZERO(port->mode_switch);
}
+static int cros_typec_register_orientation_switch(struct cros_typec_port *port,
+ struct fwnode_handle *fwnode)
+{
+ struct typec_switch_desc orientation_switch_desc = {
+ .fwnode = fwnode,
+ .drvdata = port,
+ .name = fwnode_get_name(fwnode),
+ .set = cros_typec_dp_port_orientation_set,
+ };
+
+ port->orientation_switch = typec_switch_register(port->sdata->dev, &orientation_switch_desc);
+
+ return PTR_ERR_OR_ZERO(port->orientation_switch);
+}
+
static int cros_typec_register_retimer(struct cros_typec_port *port, struct fwnode_handle *fwnode)
{
struct typec_retimer_desc retimer_desc = {
@@ -328,17 +360,35 @@ static int dp_lane_to_typec_lane(unsigned int dp_lane)
return -EINVAL;
}
-static int typec_to_dp_lane(unsigned int typec_lane)
+static int typec_to_dp_lane(unsigned int typec_lane,
+ enum typec_orientation orientation)
{
- switch (typec_lane) {
- case 0:
- return 3;
- case 1:
- return 2;
- case 2:
- return 0;
- case 3:
- return 1;
+ switch (orientation) {
+ case TYPEC_ORIENTATION_NONE:
+ case TYPEC_ORIENTATION_NORMAL:
+ switch (typec_lane) {
+ case 0:
+ return 3;
+ case 1:
+ return 2;
+ case 2:
+ return 0;
+ case 3:
+ return 1;
+ }
+ break;
+ case TYPEC_ORIENTATION_REVERSE:
+ switch (typec_lane) {
+ case 0:
+ return 0;
+ case 1:
+ return 1;
+ case 2:
+ return 3;
+ case 3:
+ return 2;
+ }
+ break;
}
return -EINVAL;
@@ -381,7 +431,7 @@ static int cros_typec_dp_bridge_atomic_check(struct drm_bridge *bridge,
typec_lane = port->lane_mapping[typec_lane];
/* Map logical type-c lane to logical DP lane */
- in_lanes[i].logical = typec_to_dp_lane(typec_lane);
+ in_lanes[i].logical = typec_to_dp_lane(typec_lane, port->orientation);
}
return 0;
@@ -509,6 +559,15 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
dev_dbg(dev, "Mode switch registered for index %u\n", index);
}
+ if (fwnode_property_present(fwnode, "orientation-switch")) {
+ ret = cros_typec_register_orientation_switch(port, port_node);
+ if (ret) {
+ dev_err(dev, "Orientation switch register failed\n");
+ goto out;
+ }
+
+ dev_dbg(dev, "Orientation switch registered for index %u\n", index);
+ }
out:
if (np)
@@ -556,7 +615,8 @@ static int cros_typec_register_switches(struct cros_typec_switch_data *sdata)
}
}
- if (fwnode_property_present(devnode, "mode-switch")) {
+ if (fwnode_property_present(devnode, "mode-switch") ||
+ fwnode_property_present(devnode, "orientation-switch")) {
fwnode = fwnode_graph_get_endpoint_by_id(devnode, 0, 0, 0);
if (fwnode) {
ret = cros_typec_register_dp_bridge(sdata, fwnode);
--
https://chromeos.dev
Some EC firmwares on Trogdor/Strongbad boards don't properly indicate
the state of DP HPD on a type-c port. Instead, the EC only indicates
that DP mode is entered or exited for a type-c port. To make matters
worse, on these firmwares the DP signal is muxed between two USB type-c
connectors, so we can't use the HPD state to figure out which type-c
port is actually displaying DP.
Read the state of the EC's analog mux from the hpd notification callback
to figure out which type-c port is displaying DP. This circumvents the
entire host command/message interface, because it doesn't work all the
time. Only do this when we have the mux-gpios property in DT, indicating
that we have to read the EC gpio state to figure this out. For now we
only support a single gpio "bit", so there can only be two USB type-c
ports.
Cc: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Tzung-Bi Shih <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/platform/chrome/cros_typec_switch.c | 33 ++++++++++++++++++++-
1 file changed, 32 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
index c22c2531327a..edd628eab7da 100644
--- a/drivers/platform/chrome/cros_typec_switch.c
+++ b/drivers/platform/chrome/cros_typec_switch.c
@@ -8,6 +8,7 @@
#include <linux/acpi.h>
#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/of.h>
@@ -26,6 +27,7 @@
struct cros_typec_dp_bridge {
/* TODO: Add mutex lock to protect active_port with respect to drm/typec framework calls */
struct cros_typec_port *active_port;
+ struct gpio_desc *mux_gpio;
struct cros_typec_switch_data *sdata;
size_t max_lanes;
bool hpd_enabled;
@@ -453,6 +455,29 @@ static void cros_typec_dp_bridge_hpd_disable(struct drm_bridge *bridge)
typec_dp_bridge->hpd_enabled = false;
}
+static void cros_typec_dp_bridge_hpd_notify(struct drm_bridge *bridge,
+ enum drm_connector_status status)
+{
+ struct cros_typec_dp_bridge *typec_dp_bridge;
+ struct cros_typec_switch_data *sdata;
+ struct gpio_desc *mux_gpio;
+ int mux_val;
+
+ typec_dp_bridge = bridge_to_cros_typec_dp_bridge(bridge);
+ mux_gpio = typec_dp_bridge->mux_gpio;
+
+ /*
+ * Some ECs don't notify AP when HPD goes high or low so we have to
+ * read the EC GPIO that controls the mux to figure out which type-c
+ * port is connected to DP.
+ */
+ if (mux_gpio) {
+ sdata = typec_dp_bridge->sdata;
+ mux_val = gpiod_get_value_cansleep(mux_gpio);
+ typec_dp_bridge->active_port = sdata->ports[mux_val];
+ }
+}
+
static const struct drm_bridge_funcs cros_typec_dp_bridge_funcs = {
.attach = cros_typec_dp_bridge_attach,
.atomic_check = cros_typec_dp_bridge_atomic_check,
@@ -461,6 +486,7 @@ static const struct drm_bridge_funcs cros_typec_dp_bridge_funcs = {
.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
.hpd_enable = cros_typec_dp_bridge_hpd_enable,
.hpd_disable = cros_typec_dp_bridge_hpd_disable,
+ .hpd_notify = cros_typec_dp_bridge_hpd_notify,
};
static int cros_typec_register_dp_bridge(struct cros_typec_switch_data *sdata,
@@ -478,6 +504,10 @@ static int cros_typec_register_dp_bridge(struct cros_typec_switch_data *sdata,
typec_dp_bridge->sdata = sdata;
sdata->typec_dp_bridge = typec_dp_bridge;
+ typec_dp_bridge->mux_gpio = devm_gpiod_get_optional(dev, "mux", 0);
+ if (IS_ERR(typec_dp_bridge->mux_gpio))
+ return PTR_ERR(typec_dp_bridge->mux_gpio);
+
num_lanes = fwnode_property_count_u32(fwnode, "data-lanes");
if (num_lanes < 0)
num_lanes = 4;
@@ -488,7 +518,8 @@ static int cros_typec_register_dp_bridge(struct cros_typec_switch_data *sdata,
bridge->funcs = &cros_typec_dp_bridge_funcs;
bridge->of_node = dev->of_node;
bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
- bridge->ops |= DRM_BRIDGE_OP_HPD;
+ if (!fwnode_property_present(dev_fwnode(dev), "no-hpd"))
+ bridge->ops |= DRM_BRIDGE_OP_HPD;
return devm_drm_bridge_add(dev, bridge);
}
--
https://chromeos.dev
It's simpler to reason about things if we disable nodes instead of
deleting them. Disable the second usb type-c connector node on
quackingstick instead of deleting it so that we can reason about ports
more easily.
Cc: <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Konrad Dybcio <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
.../arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
index 5f06842c683b..b7de9fd3fa20 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
@@ -10,9 +10,6 @@
#include "sc7180-trogdor.dtsi"
#include "sc7180-trogdor-rt5682i-sku.dtsi"
-/* This board only has 1 USB Type-C port. */
-/delete-node/ &usb_c1;
-
/ {
ppvar_lcd: ppvar-lcd-regulator {
compatible = "regulator-fixed";
@@ -136,6 +133,11 @@ pp3300_disp_on: &pp3300_dx_edp {
gpio = <&tlmm 67 GPIO_ACTIVE_HIGH>;
};
+/* This board only has 1 USB Type-C port. */
+&usb_c1 {
+ status = "disabled";
+};
+
/* PINCTRL - modifications to sc7180-trogdor.dtsi */
/*
--
https://chromeos.dev
We put a header before modifying pinctrl nodes defined in
sc7180-trogdor.dtsi in every other file. Add one here so we know that
this section is for pinctrl modifications.
Cc: <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Konrad Dybcio <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi
index 8823edbb4d6e..73aa75621721 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi
@@ -83,6 +83,8 @@ &pp3300_dx_edp {
gpio = <&tlmm 67 GPIO_ACTIVE_HIGH>;
};
+/* PINCTRL - modifications to sc7180-trogdor.dtsi */
+
&en_pp3300_dx_edp {
pins = "gpio67";
};
--
https://chromeos.dev
At a high-level, detachable Trogdors (sometimes known as Strongbads)
don't have a cros_ec keyboard, while all clamshell Trogdors (only known
as Trogdors) always have a cros_ec keyboard. Looking closer though, all
clamshells have a USB type-A connector and a hardwired USB camera. And
all detachables replace the USB camera with a MIPI based one and swap
the USB type-a connector for the detachable keyboard pogo pins.
Split the detachable and clamshell bits into different files so we can
describe these differences in one place instead of in each board that
includes sc7180-trogdor.dtsi. For now this is just the keyboard part,
but eventually this will include the type-a port and the pogo pins.
Cc: <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Konrad Dybcio <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
.../boot/dts/qcom/sc7180-trogdor-clamshell.dtsi | 9 +++++++++
arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 5 +----
.../boot/dts/qcom/sc7180-trogdor-detachable.dtsi | 12 ++++++++++++
.../arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 7 +------
.../boot/dts/qcom/sc7180-trogdor-kingoftown.dts | 2 +-
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 3 +--
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi | 2 +-
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 2 +-
.../boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi | 7 +------
arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts | 2 +-
.../boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi | 5 +----
11 files changed, 30 insertions(+), 26 deletions(-)
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
new file mode 100644
index 000000000000..bcf3df463f80
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Trogdor dts framgent for clamshells
+ *
+ * Copyright 2024 Google LLC.
+ */
+
+/* This file must be included after sc7180-trogdor.dtsi */
+#include <arm/cros-ec-keyboard.dtsi>
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
index 7765c8f64905..6e6a4643c4dd 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
@@ -7,6 +7,7 @@
#include "sc7180-trogdor.dtsi"
#include "sc7180-trogdor-ti-sn65dsi86.dtsi"
+#include "sc7180-trogdor-detachable.dtsi"
/* Deleted nodes from sc7180-trogdor.dtsi */
@@ -80,10 +81,6 @@ &camcc {
};
&cros_ec {
- keyboard-controller {
- compatible = "google,cros-ec-keyb-switches";
- };
-
cros_ec_proximity: proximity {
compatible = "google,cros-ec-mkbp-proximity";
label = "proximity-wifi";
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
new file mode 100644
index 000000000000..ab0f30288871
--- /dev/null
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Google Trogdor dts framgent for detachables
+ *
+ * Copyright 2024 Google LLC.
+ */
+
+&cros_ec {
+ keyboard-controller {
+ compatible = "google,cros-ec-keyb-switches";
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
index 2ba3bbf3b9ad..a86a6c5c3f67 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
@@ -8,6 +8,7 @@
/* This file must be included after sc7180-trogdor.dtsi */
#include "sc7180-trogdor-rt5682i-sku.dtsi"
+#include "sc7180-trogdor-detachable.dtsi"
/ {
/* BOARD-SPECIFIC TOP LEVEL NODES */
@@ -135,12 +136,6 @@ &camcc {
status = "okay";
};
-&cros_ec {
- keyboard-controller {
- compatible = "google,cros-ec-keyb-switches";
- };
-};
-
&panel {
compatible = "samsung,atna33xc20";
enable-gpios = <&tlmm 12 GPIO_ACTIVE_HIGH>;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts
index d6db7d83adcf..655bea928e52 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts
@@ -9,7 +9,7 @@
#include "sc7180-trogdor.dtsi"
#include "sc7180-trogdor-parade-ps8640.dtsi"
-#include <arm/cros-ec-keyboard.dtsi>
+#include "sc7180-trogdor-clamshell.dtsi"
#include "sc7180-trogdor-lte-sku.dtsi"
#include "sc7180-trogdor-rt5682s-sku.dtsi"
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
index e9f213d27711..c3fd6760de7a 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
@@ -5,8 +5,7 @@
* Copyright 2020 Google LLC.
*/
-/* This file must be included after sc7180-trogdor.dtsi */
-#include <arm/cros-ec-keyboard.dtsi>
+#include "sc7180-trogdor-clamshell.dtsi"
&ap_sar_sensor {
semtech,cs0-ground;
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi
index 73aa75621721..60ccd3abddfc 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi
@@ -6,7 +6,7 @@
*/
/* This file must be included after sc7180-trogdor.dtsi */
-#include <arm/cros-ec-keyboard.dtsi>
+#include "sc7180-trogdor-clamshell.dtsi"
&ap_sar_sensor {
compatible = "semtech,sx9324";
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
index 0be62331f982..43b2583f0f26 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
@@ -7,7 +7,7 @@
#include "sc7180-trogdor.dtsi"
/* Must come after sc7180-trogdor.dtsi to modify cros_ec */
-#include <arm/cros-ec-keyboard.dtsi>
+#include "sc7180-trogdor-clamshell.dtsi"
#include "sc7180-trogdor-rt5682i-sku.dtsi"
#include "sc7180-trogdor-ti-sn65dsi86.dtsi"
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
index b7de9fd3fa20..00229b1515e6 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
@@ -9,6 +9,7 @@
#include "sc7180-trogdor.dtsi"
#include "sc7180-trogdor-rt5682i-sku.dtsi"
+#include "sc7180-trogdor-detachable.dtsi"
/ {
ppvar_lcd: ppvar-lcd-regulator {
@@ -44,12 +45,6 @@ &camcc {
status = "okay";
};
-&cros_ec {
- keyboard-controller {
- compatible = "google,cros-ec-keyb-switches";
- };
-};
-
&gpio_keys {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
index c9667751a990..4b43a9b273c0 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts
@@ -9,7 +9,7 @@
#include "sc7180-trogdor.dtsi"
/* Must come after sc7180-trogdor.dtsi to modify cros_ec */
-#include <arm/cros-ec-keyboard.dtsi>
+#include "sc7180-trogdor-clamshell.dtsi"
#include "sc7180-trogdor-rt5682i-sku.dtsi"
#include "sc7180-trogdor-ti-sn65dsi86.dtsi"
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
index 305ad127246e..1d9fc61b6550 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
@@ -8,6 +8,7 @@
/dts-v1/;
#include "sc7180-trogdor.dtsi"
+#include "sc7180-trogdor-detachable.dtsi"
/ {
avdd_lcd: avdd-lcd-regulator {
@@ -104,10 +105,6 @@ &cros_ec {
base_detection: cbas {
compatible = "google,cros-cbas";
};
-
- keyboard-controller {
- compatible = "google,cros-ec-keyb-switches";
- };
};
&i2c4 {
--
https://chromeos.dev
Fully describe the USB type-c and DP topology on sc7180 Trogdor devices.
Most Trogdor devices have two USB type-c ports (i.e. usb-c-connector
nodes), but quackingstick only has one. Also, clamshell devices such as
Lazor have a USB webcam connected to the USB hub, while detachable
devices such as Wormdingler don't have a webcam, or a USB type-a
connector. Instead they have the pogo pins for the detachable keyboard.
Fully describing the topology like this will let us expose information
about what devices are connected to which physical USB connector (type-A
or type-C) and which port is connected to an external display for DP.
Cc: <[email protected]>
Cc: Andy Gross <[email protected]>
Cc: Bjorn Andersson <[email protected]>
Cc: Konrad Dybcio <[email protected]>
Cc: Rob Herring <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
.../dts/qcom/sc7180-trogdor-clamshell.dtsi | 21 +++
.../boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 47 +++++
.../dts/qcom/sc7180-trogdor-detachable.dtsi | 13 ++
.../dts/qcom/sc7180-trogdor-homestar.dtsi | 47 +++++
.../dts/qcom/sc7180-trogdor-kingoftown.dts | 55 ++++++
.../boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 55 ++++++
.../boot/dts/qcom/sc7180-trogdor-pazquel.dtsi | 55 ++++++
.../boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 44 +++++
.../qcom/sc7180-trogdor-quackingstick.dtsi | 31 ++++
.../dts/qcom/sc7180-trogdor-wormdingler.dtsi | 47 +++++
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi | 175 ++++++++++++++++++
11 files changed, 590 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
index bcf3df463f80..96137202fc64 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
@@ -7,3 +7,24 @@
/* This file must be included after sc7180-trogdor.dtsi */
#include <arm/cros-ec-keyboard.dtsi>
+
+/ {
+ usb-a-connector {
+ compatible = "usb-a-connector";
+
+ port {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb_a0_hs: endpoint@0 {
+ reg = <0>;
+ /* Remote endpoint filled in by board */
+ };
+
+ usb_a0_ss: endpoint@1 {
+ reg = <1>;
+ /* Remote endpoint filled in by board */
+ };
+ };
+ };
+};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
index 6e6a4643c4dd..4cf5b1e20b27 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi
@@ -135,6 +135,17 @@ skin-temp-thermistor@1 {
};
};
+&pogo_pins {
+ keyboard@4 {
+ compatible = "usb18d1,504c";
+ reg = <4>;
+ };
+};
+
+&pogo_pins_in {
+ remote-endpoint = <&usb_hub_dfp4_hs>;
+};
+
&pp1800_uf_cam {
status = "okay";
};
@@ -176,6 +187,42 @@ &sound_multimedia0_codec {
sound-dai = <&adau7002>;
};
+&usb_c0_hs {
+ remote-endpoint = <&usb_hub_dfp1_hs>;
+};
+
+&usb_c0_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp1_ss>;
+};
+
+&usb_c1_hs {
+ remote-endpoint = <&usb_hub_dfp2_hs>;
+};
+
+&usb_c1_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp2_ss>;
+};
+
+&usb_hub_dfp1_hs {
+ remote-endpoint = <&usb_c0_hs>;
+};
+
+&usb_hub_dfp1_ss {
+ remote-endpoint = <&usb_c0_ss_rxtx>;
+};
+
+&usb_hub_dfp2_hs {
+ remote-endpoint = <&usb_c1_hs>;
+};
+
+&usb_hub_dfp2_ss {
+ remote-endpoint = <&usb_c1_ss_rxtx>;
+};
+
+&usb_hub_dfp4_hs {
+ remote-endpoint = <&pogo_pins_in>;
+};
+
/* PINCTRL - modifications to sc7180-trogdor.dtsi */
&en_pp3300_dx_edp {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
index ab0f30288871..b24a0213a477 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
@@ -5,6 +5,19 @@
* Copyright 2024 Google LLC.
*/
+/ {
+ pogo_pins: pogo-pin-connector {
+ compatible = "google,pogo-pin-connector";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ /* Detachable keyboard populated for each board */
+ port {
+ pogo_pins_in: endpoint {
+ };
+ };
+ };
+};
+
&cros_ec {
keyboard-controller {
compatible = "google,cros-ec-keyb-switches";
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
index a86a6c5c3f67..9e32c984ab32 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi
@@ -162,6 +162,17 @@ skin-temp-thermistor@1 {
};
};
+&pogo_pins {
+ keyboard@3 {
+ compatible = "usb18d1,5052";
+ reg = <3>;
+ };
+};
+
+&pogo_pins_in {
+ remote-endpoint = <&usb_hub_dfp3_hs>;
+};
+
&pp1800_uf_cam {
status = "okay";
};
@@ -190,6 +201,42 @@ &sound_multimedia1_codec {
sound-dai = <&max98360a>, <&max98360a_1>, <&max98360a_2>, <&max98360a_3> ;
};
+&usb_c0_hs {
+ remote-endpoint = <&usb_hub_dfp2_hs>;
+};
+
+&usb_c0_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp2_ss>;
+};
+
+&usb_c1_hs {
+ remote-endpoint = <&usb_hub_dfp4_hs>;
+};
+
+&usb_c1_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp4_ss>;
+};
+
+&usb_hub_dfp2_hs {
+ remote-endpoint = <&usb_c0_hs>;
+};
+
+&usb_hub_dfp2_ss {
+ remote-endpoint = <&usb_c0_ss_rxtx>;
+};
+
+&usb_hub_dfp3_hs {
+ remote-endpoint = <&pogo_pins_in>;
+};
+
+&usb_hub_dfp4_hs {
+ remote-endpoint = <&usb_c1_hs>;
+};
+
+&usb_hub_dfp4_ss {
+ remote-endpoint = <&usb_c1_ss_rxtx>;
+};
+
&wifi {
qcom,ath10k-calibration-variant = "GO_HOMESTAR";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts b/arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts
index 655bea928e52..476c0a2f30da 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts
@@ -78,6 +78,61 @@ &pp3300_dx_edp {
gpio = <&tlmm 67 GPIO_ACTIVE_HIGH>;
};
+&usb_a0_hs {
+ remote-endpoint = <&usb_hub_dfp3_hs>;
+};
+
+&usb_a0_ss {
+ remote-endpoint = <&usb_hub_dfp3_ss>;
+};
+
+&usb_c0_hs {
+ remote-endpoint = <&usb_hub_dfp1_hs>;
+};
+
+&usb_c0_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp1_ss>;
+};
+
+&usb_c1_hs {
+ remote-endpoint = <&usb_hub_dfp2_hs>;
+};
+
+&usb_c1_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp2_ss>;
+};
+
+&usb_hub_2_x {
+ camera@4 {
+ compatible = "usb4f2,b75a";
+ reg = <4>;
+ };
+};
+
+&usb_hub_dfp1_hs {
+ remote-endpoint = <&usb_c0_hs>;
+};
+
+&usb_hub_dfp1_ss {
+ remote-endpoint = <&usb_c0_ss_rxtx>;
+};
+
+&usb_hub_dfp2_hs {
+ remote-endpoint = <&usb_c1_hs>;
+};
+
+&usb_hub_dfp2_ss {
+ remote-endpoint = <&usb_c1_ss_rxtx>;
+};
+
+&usb_hub_dfp3_hs {
+ remote-endpoint = <&usb_a0_hs>;
+};
+
+&usb_hub_dfp3_ss {
+ remote-endpoint = <&usb_a0_ss>;
+};
+
&wifi {
qcom,ath10k-calibration-variant = "GO_KINGOFTOWN";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
index c3fd6760de7a..2603607ebd80 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
@@ -68,6 +68,61 @@ &trackpad {
interrupts = <58 IRQ_TYPE_EDGE_FALLING>;
};
+&usb_hub_2_x {
+ camera@1 {
+ compatible = "usb408,a092";
+ reg = <1>;
+ };
+};
+
+&usb_a0_hs {
+ remote-endpoint = <&usb_hub_dfp3_hs>;
+};
+
+&usb_a0_ss {
+ remote-endpoint = <&usb_hub_dfp3_ss>;
+};
+
+&usb_c0_hs {
+ remote-endpoint = <&usb_hub_dfp2_hs>;
+};
+
+&usb_c0_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp2_ss>;
+};
+
+&usb_c1_hs {
+ remote-endpoint = <&usb_hub_dfp4_hs>;
+};
+
+&usb_c1_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp4_ss>;
+};
+
+&usb_hub_dfp2_hs {
+ remote-endpoint = <&usb_c0_hs>;
+};
+
+&usb_hub_dfp2_ss {
+ remote-endpoint = <&usb_c0_ss_rxtx>;
+};
+
+&usb_hub_dfp3_hs {
+ remote-endpoint = <&usb_a0_hs>;
+};
+
+&usb_hub_dfp3_ss {
+ remote-endpoint = <&usb_a0_ss>;
+};
+
+&usb_hub_dfp4_hs {
+ remote-endpoint = <&usb_c1_hs>;
+};
+
+&usb_hub_dfp4_ss {
+ remote-endpoint = <&usb_c1_ss_rxtx>;
+};
+
&wifi {
qcom,ath10k-calibration-variant = "GO_LAZOR";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi
index 60ccd3abddfc..dee06c64b59a 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi
@@ -83,6 +83,61 @@ &pp3300_dx_edp {
gpio = <&tlmm 67 GPIO_ACTIVE_HIGH>;
};
+&usb_a0_hs {
+ remote-endpoint = <&usb_hub_dfp3_hs>;
+};
+
+&usb_a0_ss {
+ remote-endpoint = <&usb_hub_dfp3_ss>;
+};
+
+&usb_c0_hs {
+ remote-endpoint = <&usb_hub_dfp1_hs>;
+};
+
+&usb_c0_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp1_ss>;
+};
+
+&usb_c1_hs {
+ remote-endpoint = <&usb_hub_dfp2_hs>;
+};
+
+&usb_c1_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp2_ss>;
+};
+
+&usb_hub_2_x {
+ camera@4 {
+ compatible = "usb5c8,b03";
+ reg = <4>;
+ };
+};
+
+&usb_hub_dfp1_hs {
+ remote-endpoint = <&usb_c0_hs>;
+};
+
+&usb_hub_dfp1_ss {
+ remote-endpoint = <&usb_c0_ss_rxtx>;
+};
+
+&usb_hub_dfp2_hs {
+ remote-endpoint = <&usb_c1_hs>;
+};
+
+&usb_hub_dfp2_ss {
+ remote-endpoint = <&usb_c1_ss_rxtx>;
+};
+
+&usb_hub_dfp3_hs {
+ remote-endpoint = <&usb_a0_hs>;
+};
+
+&usb_hub_dfp3_ss {
+ remote-endpoint = <&usb_a0_ss>;
+};
+
/* PINCTRL - modifications to sc7180-trogdor.dtsi */
&en_pp3300_dx_edp {
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
index 43b2583f0f26..88ffa2331cd2 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi
@@ -180,10 +180,54 @@ &sound {
dmic-gpios = <&tlmm 86 GPIO_ACTIVE_HIGH>;
};
+&usb_a0_hs {
+ remote-endpoint = <&usb_hub_dfp4_hs>;
+};
+
+&usb_a0_ss {
+ remote-endpoint = <&usb_hub_dfp4_ss>;
+};
+
+&usb_c0_hs {
+ remote-endpoint = <&usb_hub_dfp3_hs>;
+};
+
+&usb_c0_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp3_ss>;
+};
+
&usb_c1 {
status = "disabled";
};
+&usb_hub_2_x {
+ camera@1 {
+ compatible = "usb4f2,b718";
+ reg = <1>;
+ };
+
+ camera@2 {
+ compatible = "usb13d3,56e9";
+ reg = <2>;
+ };
+};
+
+&usb_hub_dfp3_hs {
+ remote-endpoint = <&usb_c0_hs>;
+};
+
+&usb_hub_dfp3_ss {
+ remote-endpoint = <&usb_c0_ss_rxtx>;
+};
+
+&usb_hub_dfp4_hs {
+ remote-endpoint = <&usb_a0_hs>;
+};
+
+&usb_hub_dfp4_ss {
+ remote-endpoint = <&usb_a0_ss>;
+};
+
&wifi {
qcom,ath10k-calibration-variant = "GO_POMPOM";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
index 00229b1515e6..d0d9871b74cb 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi
@@ -104,6 +104,17 @@ &sdhc_2 {
status = "okay";
};
+&pogo_pins {
+ keyboard@1 {
+ compatible = "usb18d1,505b";
+ reg = <1>;
+ };
+};
+
+&pogo_pins_in {
+ remote-endpoint = <&usb_hub_dfp1_hs>;
+};
+
&pp1800_uf_cam {
status = "okay";
};
@@ -128,11 +139,31 @@ pp3300_disp_on: &pp3300_dx_edp {
gpio = <&tlmm 67 GPIO_ACTIVE_HIGH>;
};
+&usb_c0_hs {
+ remote-endpoint = <&usb_hub_dfp2_hs>;
+};
+
+&usb_c0_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp2_ss>;
+};
+
/* This board only has 1 USB Type-C port. */
&usb_c1 {
status = "disabled";
};
+&usb_hub_dfp2_hs {
+ remote-endpoint = <&usb_c0_hs>;
+};
+
+&usb_hub_dfp2_ss {
+ remote-endpoint = <&usb_c0_ss_rxtx>;
+};
+
+&usb_hub_dfp1_hs {
+ remote-endpoint = <&pogo_pins_in>;
+};
+
/* PINCTRL - modifications to sc7180-trogdor.dtsi */
/*
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
index 1d9fc61b6550..409d332fbc13 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi
@@ -180,6 +180,17 @@ skin-temp-thermistor@1 {
};
};
+&pogo_pins {
+ keyboard@3 {
+ compatible = "usb18d1,5057";
+ reg = <3>;
+ };
+};
+
+&pogo_pins_in {
+ remote-endpoint = <&usb_hub_dfp3_hs>;
+};
+
&pp1800_uf_cam {
status = "okay";
};
@@ -196,6 +207,42 @@ &pp2800_wf_cam {
status = "okay";
};
+&usb_c0_hs {
+ remote-endpoint = <&usb_hub_dfp2_hs>;
+};
+
+&usb_c0_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp2_ss>;
+};
+
+&usb_c1_hs {
+ remote-endpoint = <&usb_hub_dfp4_hs>;
+};
+
+&usb_c1_ss_rxtx {
+ remote-endpoint = <&usb_hub_dfp3_ss>;
+};
+
+&usb_hub_dfp2_hs {
+ remote-endpoint = <&usb_c0_hs>;
+};
+
+&usb_hub_dfp2_ss {
+ remote-endpoint = <&usb_c0_ss_rxtx>;
+};
+
+&usb_hub_dfp4_hs {
+ remote-endpoint = <&usb_c1_hs>;
+};
+
+&usb_hub_dfp3_ss {
+ remote-endpoint = <&usb_c1_ss_rxtx>;
+};
+
+&usb_hub_dfp3_hs {
+ remote-endpoint = <&pogo_pins_in>;
+};
+
&wifi {
qcom,ath10k-calibration-variant = "GO_WORMDINGLER";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
index 46aaeba28604..ee08a4ecade9 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi
@@ -650,6 +650,12 @@ cros_ec: ec@0 {
pinctrl-0 = <&ap_ec_int_l>;
spi-max-frequency = <3000000>;
+ cros_ec_gpio: gpio {
+ compatible = "google,cros-ec-gpio";
+ #gpio-cells = <2>;
+ gpio-controller;
+ };
+
cros_ec_pwm: pwm {
compatible = "google,cros-ec-pwm";
#pwm-cells = <1>;
@@ -662,6 +668,65 @@ i2c_tunnel: i2c-tunnel {
#size-cells = <0>;
};
+ typec-switch {
+ compatible = "google,cros-ec-typec-switch";
+ no-hpd;
+ mode-switch;
+ mux-gpios = <&cros_ec_gpio 42 GPIO_ACTIVE_HIGH>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ dp_ml0_ml1: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&mdss_dp_out>;
+ data-lanes = <0 1>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb_c0_ss_rxtx: endpoint@0 {
+ reg = <0>;
+ /* Endpoint filled in by board */
+ };
+
+ usb_c1_ss_rxtx: endpoint@1 {
+ reg = <1>;
+ /* Endpoint filled in by board */
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ cros_typec_c0_ss: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&usb_c0_ss>;
+ data-lanes = <0 1 2 3>;
+ };
+
+ cros_typec_c1_ss: endpoint@1 {
+ reg = <1>;
+ remote-endpoint = <&usb_c1_ss>;
+ data-lanes = <0 1 2 3>;
+ };
+ };
+ };
+
+ };
+
typec {
compatible = "google,cros-ec-typec";
#address-cells = <1>;
@@ -674,6 +739,25 @@ usb_c0: connector@0 {
power-role = "dual";
data-role = "host";
try-power-role = "source";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ usb_c0_hs: endpoint {
+ /* Endpoint filled in by board */
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ usb_c0_ss: endpoint {
+ remote-endpoint = <&cros_typec_c0_ss>;
+ };
+ };
+ };
};
usb_c1: connector@1 {
@@ -683,6 +767,25 @@ usb_c1: connector@1 {
power-role = "dual";
data-role = "host";
try-power-role = "source";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ usb_c1_hs: endpoint {
+ /* Endpoint filled in by board */
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ usb_c1_ss: endpoint {
+ remote-endpoint = <&cros_typec_c1_ss>;
+ };
+ };
+ };
};
};
};
@@ -794,6 +897,7 @@ &mdss_dp {
&mdss_dp_out {
data-lanes = <0 1>;
link-frequencies = /bits/ 64 <1620000000 2700000000 5400000000>;
+ remote-endpoint = <&dp_ml0_ml1>;
};
&mdss_dsi0 {
@@ -965,6 +1069,41 @@ usb_hub_2_x: hub@1 {
reg = <1>;
vdd-supply = <&pp3300_hub>;
peer-hub = <&usb_hub_3_x>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb_hub_2_x_ports: ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <1>;
+ usb_hub_dfp1_hs: endpoint {
+ /* Remote endpoint filled in by board */
+ };
+ };
+ port@2 {
+ reg = <2>;
+ usb_hub_dfp2_hs: endpoint {
+ /* Remote endpoint filled in by board */
+ };
+ };
+
+ port@3 {
+ reg = <3>;
+ usb_hub_dfp3_hs: endpoint {
+ /* Remote endpoint filled in by board */
+ };
+ };
+
+ port@4 {
+ reg = <4>;
+ usb_hub_dfp4_hs: endpoint {
+ /* Remote endpoint filled in by board */
+ };
+ };
+ };
};
/* 3.x hub on port 2 */
@@ -973,6 +1112,42 @@ usb_hub_3_x: hub@2 {
reg = <2>;
vdd-supply = <&pp3300_hub>;
peer-hub = <&usb_hub_2_x>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ usb_hub_3_x_ports: ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <1>;
+ usb_hub_dfp1_ss: endpoint {
+ /* Remote endpoint filled in by board */
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+ usb_hub_dfp2_ss: endpoint {
+ /* Remote endpoint filled in by board */
+ };
+ };
+
+ port@3 {
+ reg = <3>;
+ usb_hub_dfp3_ss: endpoint {
+ /* Remote endpoint filled in by board */
+ };
+ };
+
+ port@4 {
+ reg = <4>;
+ usb_hub_dfp4_ss: endpoint {
+ /* Remote endpoint filled in by board */
+ };
+ };
+ };
};
};
--
https://chromeos.dev
We can imagine that logically the EC is a device that has some number of
DisplayPort (DP) connector inputs, some number of USB3 connector inputs,
and some number of USB type-c connector outputs. If you squint enough it
looks like a USB type-c dock. Logically there's a crossbar pin
assignment capability within the EC that can assign USB and DP lanes to
USB type-c lanes in the connector (i.e. USB type-c pin configurations).
In reality, the EC is a microcontroller that has some TCPCs and
redrivers connected to it over something like i2c and DP/USB from the AP
is wired directly to those ICs, not the EC.
This design allows the EC to abstract many possible USB and DP hardware
configurations away from the AP (kernel) so that the AP can largely deal
with USB and DP without thinking about USB Type-C much at all. The DP
and USB data originate in the AP, not the EC, so it helps to think that
the EC takes the DP and USB data as input to mux onto USB type-c ports
even if it really doesn't do that. With this split design, the EC
forwards the DP HPD state to the AP via a GPIO that's connected to the
DP phy.
Having that HPD state signaled directly to the DP phy uses precious
hardware resources, a GPIO or two and a wire, and it also forces the
TCPM to live on the EC. If we want to save costs and move more control
of USB type-c to the kernel it's in our interest to get rid of the HPD
GPIO entirely and signal HPD to the DP phy some other way. Luckily, the
EC already exposes information about the USB Type-C stack to the kernel
via the host command interface in the "google,cros-ec-typec" compatible
driver, which parses EC messages related to USB type-c and effectively
"replays" those messages to the kernel's USB typec subsystem. This
includes the state of HPD, which can be interrogated and acted upon by
registering a 'struct typec_mux_dev' with the typec subsystem.
On DT based systems, the DP display pipeline is abstracted via a 'struct
drm_bridge'. If we want to signal HPD state within the kernel we need to
hook into the drm_bridge framework somehow to call
drm_bridge_hpd_notify() when HPD state changes in the typec framework.
Make a drm_bridge in the EC that attaches onto the end of the DP bridge
chain and logically moves the display data onto a usb-c-connector.
Signal HPD when the typec HPD state changes, as long as this new
drm_bridge is the one that's supposed to signal HPD. Do that by
registering a 'struct typec_mux_dev' with the typec framework and
associating that struct with a usb-c-connector node and a drm_bridge.
To keep this patch minimal, just signal HPD state to the drm_bridge
chain. Later patches will add more features. Eventually we'll be able to
inform userspace about which usb-c-connector node is displaying DP and
what USB devices are connected to a connector. Note that this code is
placed in the cros_typec_switch driver because that's where mode-switch
devices on the EC are controlled by the AP. Logically this drm_bridge
sits in front of the mode-switch on the EC, and if there is anything to
control on the EC the 'EC_FEATURE_TYPEC_AP_MUX_SET' feature will be set.
Cc: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Tzung-Bi Shih <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/platform/chrome/Kconfig | 3 +-
drivers/platform/chrome/cros_typec_switch.c | 218 ++++++++++++++++++--
2 files changed, 204 insertions(+), 17 deletions(-)
diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
index 7a83346bfa53..910aa8be9c84 100644
--- a/drivers/platform/chrome/Kconfig
+++ b/drivers/platform/chrome/Kconfig
@@ -287,7 +287,8 @@ config CHROMEOS_PRIVACY_SCREEN
config CROS_TYPEC_SWITCH
tristate "ChromeOS EC Type-C Switch Control"
- depends on MFD_CROS_EC_DEV && TYPEC && ACPI
+ depends on MFD_CROS_EC_DEV
+ depends on TYPEC
default MFD_CROS_EC_DEV
help
If you say Y here, you get support for configuring the ChromeOS EC Type-C
diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
index 769de2889f2f..d8fb6662cf8d 100644
--- a/drivers/platform/chrome/cros_typec_switch.c
+++ b/drivers/platform/chrome/cros_typec_switch.c
@@ -10,6 +10,7 @@
#include <linux/delay.h>
#include <linux/iopoll.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/platform_data/cros_ec_commands.h>
#include <linux/platform_data/cros_ec_proto.h>
#include <linux/platform_device.h>
@@ -18,6 +19,15 @@
#include <linux/usb/typec_mux.h>
#include <linux/usb/typec_retimer.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_print.h>
+
+struct cros_typec_dp_bridge {
+ struct cros_typec_switch_data *sdata;
+ bool hpd_enabled;
+ struct drm_bridge bridge;
+};
+
/* Handles and other relevant data required for each port's switches. */
struct cros_typec_port {
int port_num;
@@ -30,7 +40,9 @@ struct cros_typec_port {
struct cros_typec_switch_data {
struct device *dev;
struct cros_ec_device *ec;
+ bool typec_cmd_supported;
struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
+ struct cros_typec_dp_bridge *typec_dp_bridge;
};
static int cros_typec_cmd_mux_set(struct cros_typec_switch_data *sdata, int port_num, u8 index,
@@ -143,13 +155,60 @@ static int cros_typec_configure_mux(struct cros_typec_switch_data *sdata, int po
return 0;
}
+static int cros_typec_dp_port_switch_set(struct typec_mux_dev *mode_switch,
+ struct typec_mux_state *state)
+{
+ struct cros_typec_port *port;
+ const struct typec_displayport_data *dp_data;
+ struct cros_typec_dp_bridge *typec_dp_bridge;
+ struct drm_bridge *bridge;
+ bool hpd_asserted;
+
+ port = typec_mux_get_drvdata(mode_switch);
+ typec_dp_bridge = port->sdata->typec_dp_bridge;
+ if (!typec_dp_bridge)
+ return 0;
+
+ bridge = &typec_dp_bridge->bridge;
+
+ if (state->mode == TYPEC_STATE_SAFE || state->mode == TYPEC_STATE_USB) {
+ if (typec_dp_bridge->hpd_enabled)
+ drm_bridge_hpd_notify(bridge, connector_status_disconnected);
+
+ return 0;
+ }
+
+ if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
+ if (typec_dp_bridge->hpd_enabled) {
+ dp_data = state->data;
+ hpd_asserted = dp_data->status & DP_STATUS_HPD_STATE;
+
+ if (hpd_asserted)
+ drm_bridge_hpd_notify(bridge, connector_status_connected);
+ else
+ drm_bridge_hpd_notify(bridge, connector_status_disconnected);
+ }
+ }
+
+ return 0;
+}
+
static int cros_typec_mode_switch_set(struct typec_mux_dev *mode_switch,
struct typec_mux_state *state)
{
struct cros_typec_port *port = typec_mux_get_drvdata(mode_switch);
+ struct cros_typec_switch_data *sdata = port->sdata;
+ int ret;
+
+ ret = cros_typec_dp_port_switch_set(mode_switch, state);
+ if (ret)
+ return ret;
/* Mode switches have index 0. */
- return cros_typec_configure_mux(port->sdata, port->port_num, 0, state->mode, state->alt);
+ if (sdata->typec_cmd_supported)
+ return cros_typec_configure_mux(port->sdata, port->port_num, 0, state->mode, state->alt);
+
+ return 0;
}
static int cros_typec_retimer_set(struct typec_retimer *retimer, struct typec_retimer_state *state)
@@ -201,12 +260,77 @@ static int cros_typec_register_retimer(struct cros_typec_port *port, struct fwno
return PTR_ERR_OR_ZERO(port->retimer);
}
+static int
+cros_typec_dp_bridge_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
+ DRM_ERROR("Fix bridge driver to make connector optional!\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static struct cros_typec_dp_bridge *
+bridge_to_cros_typec_dp_bridge(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct cros_typec_dp_bridge, bridge);
+}
+
+static void cros_typec_dp_bridge_hpd_enable(struct drm_bridge *bridge)
+{
+ struct cros_typec_dp_bridge *typec_dp_bridge;
+
+ typec_dp_bridge = bridge_to_cros_typec_dp_bridge(bridge);
+ typec_dp_bridge->hpd_enabled = true;
+}
+
+static void cros_typec_dp_bridge_hpd_disable(struct drm_bridge *bridge)
+{
+ struct cros_typec_dp_bridge *typec_dp_bridge;
+
+ typec_dp_bridge = bridge_to_cros_typec_dp_bridge(bridge);
+ typec_dp_bridge->hpd_enabled = false;
+}
+
+static const struct drm_bridge_funcs cros_typec_dp_bridge_funcs = {
+ .attach = cros_typec_dp_bridge_attach,
+ .hpd_enable = cros_typec_dp_bridge_hpd_enable,
+ .hpd_disable = cros_typec_dp_bridge_hpd_disable,
+};
+
+static int cros_typec_register_dp_bridge(struct cros_typec_switch_data *sdata,
+ struct fwnode_handle *fwnode)
+{
+ struct cros_typec_dp_bridge *typec_dp_bridge;
+ struct drm_bridge *bridge;
+ struct device *dev = sdata->dev;
+
+ typec_dp_bridge = devm_kzalloc(dev, sizeof(*typec_dp_bridge), GFP_KERNEL);
+ if (!typec_dp_bridge)
+ return -ENOMEM;
+
+ typec_dp_bridge->sdata = sdata;
+ sdata->typec_dp_bridge = typec_dp_bridge;
+ bridge = &typec_dp_bridge->bridge;
+
+ bridge->funcs = &cros_typec_dp_bridge_funcs;
+ bridge->of_node = dev->of_node;
+ bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
+ bridge->ops |= DRM_BRIDGE_OP_HPD;
+
+ return devm_drm_bridge_add(dev, bridge);
+}
+
static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
struct fwnode_handle *fwnode)
{
struct cros_typec_port *port;
struct device *dev = sdata->dev;
struct acpi_device *adev;
+ struct device_node *np;
+ struct fwnode_handle *port_node;
u32 index;
int ret;
const char *prop_name;
@@ -218,9 +342,12 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
adev = to_acpi_device_node(fwnode);
if (adev)
prop_name = "_ADR";
+ np = to_of_node(fwnode);
+ if (np)
+ prop_name = "reg";
- if (!adev)
- return dev_err_probe(fwnode->dev, -ENODEV, "Couldn't get ACPI handle\n");
+ if (!adev && !np)
+ return dev_err_probe(fwnode->dev, -ENODEV, "Couldn't get ACPI/OF device handle\n");
ret = fwnode_property_read_u32(fwnode, prop_name, &index);
if (ret)
@@ -232,41 +359,84 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
port->port_num = index;
sdata->ports[index] = port;
+ port_node = fwnode;
+ if (np)
+ fwnode = fwnode_graph_get_port_parent(fwnode);
+
if (fwnode_property_present(fwnode, "retimer-switch")) {
- ret = cros_typec_register_retimer(port, fwnode);
- if (ret)
- return dev_err_probe(dev, ret, "Retimer switch register failed\n");
+ ret = cros_typec_register_retimer(port, port_node);
+ if (ret) {
+ dev_err_probe(dev, ret, "Retimer switch register failed\n");
+ goto out;
+ }
dev_dbg(dev, "Retimer switch registered for index %u\n", index);
}
- if (!fwnode_property_present(fwnode, "mode-switch"))
- return 0;
+ if (fwnode_property_present(fwnode, "mode-switch")) {
+ ret = cros_typec_register_mode_switch(port, port_node);
+ if (ret) {
+ dev_err_probe(dev, ret, "Mode switch register failed\n");
+ goto out;
+ }
- ret = cros_typec_register_mode_switch(port, fwnode);
- if (ret)
- return dev_err_probe(dev, ret, "Mode switch register failed\n");
+ dev_dbg(dev, "Mode switch registered for index %u\n", index);
+ }
- dev_dbg(dev, "Mode switch registered for index %u\n", index);
+out:
+ if (np)
+ fwnode_handle_put(fwnode);
return ret;
}
static int cros_typec_register_switches(struct cros_typec_switch_data *sdata)
{
struct device *dev = sdata->dev;
+ struct fwnode_handle *devnode;
struct fwnode_handle *fwnode;
+ struct fwnode_endpoint endpoint;
int nports, ret;
nports = device_get_child_node_count(dev);
if (nports == 0)
return dev_err_probe(dev, -ENODEV, "No switch devices found\n");
- device_for_each_child_node(dev, fwnode) {
- ret = cros_typec_register_port(sdata, fwnode);
- if (ret) {
+ devnode = dev_fwnode(dev);
+ if (fwnode_graph_get_endpoint_count(devnode, 0)) {
+ fwnode_graph_for_each_endpoint(devnode, fwnode) {
+ ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
+ if (ret) {
+ fwnode_handle_put(fwnode);
+ goto err;
+ }
+ /* Skip if not a type-c output port */
+ if (endpoint.port != 2)
+ continue;
+
+ ret = cros_typec_register_port(sdata, fwnode);
+ if (ret) {
+ fwnode_handle_put(fwnode);
+ goto err;
+ }
+ }
+ } else {
+ device_for_each_child_node(dev, fwnode) {
+ ret = cros_typec_register_port(sdata, fwnode);
+ if (ret) {
+ fwnode_handle_put(fwnode);
+ goto err;
+ }
+ }
+ }
+
+ if (fwnode_property_present(devnode, "mode-switch")) {
+ fwnode = fwnode_graph_get_endpoint_by_id(devnode, 0, 0, 0);
+ if (fwnode) {
+ ret = cros_typec_register_dp_bridge(sdata, fwnode);
fwnode_handle_put(fwnode);
- goto err;
+ if (ret)
+ goto err;
}
}
@@ -280,6 +450,7 @@ static int cros_typec_switch_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct cros_typec_switch_data *sdata;
+ struct cros_ec_dev *ec_dev;
sdata = devm_kzalloc(dev, sizeof(*sdata), GFP_KERNEL);
if (!sdata)
@@ -288,6 +459,12 @@ static int cros_typec_switch_probe(struct platform_device *pdev)
sdata->dev = dev;
sdata->ec = dev_get_drvdata(pdev->dev.parent);
+ ec_dev = dev_get_drvdata(&sdata->ec->ec->dev);
+ if (!ec_dev)
+ return -EPROBE_DEFER;
+
+ sdata->typec_cmd_supported = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_AP_MUX_SET);
+
platform_set_drvdata(pdev, sdata);
return cros_typec_register_switches(sdata);
@@ -308,10 +485,19 @@ static const struct acpi_device_id cros_typec_switch_acpi_id[] = {
MODULE_DEVICE_TABLE(acpi, cros_typec_switch_acpi_id);
#endif
+#ifdef CONFIG_OF
+static const struct of_device_id cros_typec_switch_of_match_table[] = {
+ { .compatible = "google,cros-ec-typec-switch" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, cros_typec_switch_of_match_table);
+#endif
+
static struct platform_driver cros_typec_switch_driver = {
.driver = {
.name = "cros-typec-switch",
.acpi_match_table = ACPI_PTR(cros_typec_switch_acpi_id),
+ .of_match_table = of_match_ptr(cros_typec_switch_of_match_table),
},
.probe = cros_typec_switch_probe,
.remove_new = cros_typec_switch_remove,
--
https://chromeos.dev
Implement DP lane assignment in the drm_bridge atomic_check() callback
so that DP altmode configurations like pinconf D can be supported and DP
muxing can be implemented. In the DP altmode spec, pinconf C assigns all
4 SuperSpeed lanes in the usb-c-connector node to DP, while pinconf D
assigns 2 SuperSpeed lanes to DP and 2 SuperSpeed lanes to USB. Use the
'data-lanes' property from the input DP graph port to calculate the
maximum number of lanes coming from the DP source (either 2 or
4) and limit the lanes requested to the smaller of this or the pin
configuration.
Once we know the maximum number of lanes that can be assigned, map the
DP lanes to the active type-c output port with the assigned pin
configuration. Use the 'data-lanes' property from the active type-c
output port to determine which logical DP lanes should be assigned to
the output of the drm_bridge. For now assume the type-c pins are in the
normal orientation.
This design supports different DP altmode pin configurations and also
allows us to effectively mux one DP phy to two different USB type-c
connectors by wiring the physical DP lanes to one or the other USB
type-c connectors in the hardware. For example, DP ML0/ML1 are hardwired
to one USB type-c connector and DP ML2/ML3 are hardwired to the other
connector. The 'data-lanes' of the first USB type-c port would be the
default "<0 1 2 3>" while the 'data-lanes' of the second USB type-c port
would be "<2 3 0 1>". Depending on the active USB type-c port, map the
logical DP lane to the logical type-c lane, and then find the physical
type-c lane corresponding to that logical lane from the type-c port's
'data-lanes' property. Once we have that, map the physical type-c lane
to the physical DP lane and request that physical DP lane as the logical
DP lane through the DRM lane assignment logic on the input of the
drm_bridge.
Cc: Prashant Malani <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Tzung-Bi Shih <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
drivers/platform/chrome/cros_typec_switch.c | 136 +++++++++++++++++++-
1 file changed, 131 insertions(+), 5 deletions(-)
diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
index d8fb6662cf8d..adcbf8f44c98 100644
--- a/drivers/platform/chrome/cros_typec_switch.c
+++ b/drivers/platform/chrome/cros_typec_switch.c
@@ -19,20 +19,28 @@
#include <linux/usb/typec_mux.h>
#include <linux/usb/typec_retimer.h>
+#include <drm/drm_atomic_state_helper.h>
#include <drm/drm_bridge.h>
#include <drm/drm_print.h>
struct cros_typec_dp_bridge {
+ /* TODO: Add mutex lock to protect active_port with respect to drm/typec framework calls */
+ struct cros_typec_port *active_port;
struct cros_typec_switch_data *sdata;
+ size_t max_lanes;
bool hpd_enabled;
struct drm_bridge bridge;
};
+#define USBC_LANES_COUNT 4
+
/* Handles and other relevant data required for each port's switches. */
struct cros_typec_port {
int port_num;
struct typec_mux_dev *mode_switch;
struct typec_retimer *retimer;
+ size_t num_dp_lanes;
+ u32 lane_mapping[USBC_LANES_COUNT];
struct cros_typec_switch_data *sdata;
};
@@ -163,6 +171,8 @@ static int cros_typec_dp_port_switch_set(struct typec_mux_dev *mode_switch,
struct cros_typec_dp_bridge *typec_dp_bridge;
struct drm_bridge *bridge;
bool hpd_asserted;
+ u8 pin_assign;
+ size_t num_lanes, max_lanes;
port = typec_mux_get_drvdata(mode_switch);
typec_dp_bridge = port->sdata->typec_dp_bridge;
@@ -172,17 +182,41 @@ static int cros_typec_dp_port_switch_set(struct typec_mux_dev *mode_switch,
bridge = &typec_dp_bridge->bridge;
if (state->mode == TYPEC_STATE_SAFE || state->mode == TYPEC_STATE_USB) {
- if (typec_dp_bridge->hpd_enabled)
- drm_bridge_hpd_notify(bridge, connector_status_disconnected);
+ /* Clear active port when port isn't in DP mode */
+ port->num_dp_lanes = 0;
+ if (typec_dp_bridge->active_port == port) {
+ typec_dp_bridge->active_port = NULL;
+ if (typec_dp_bridge->hpd_enabled)
+ drm_bridge_hpd_notify(bridge, connector_status_disconnected);
+ }
return 0;
}
if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
- if (typec_dp_bridge->hpd_enabled) {
- dp_data = state->data;
- hpd_asserted = dp_data->status & DP_STATUS_HPD_STATE;
+ dp_data = state->data;
+ hpd_asserted = dp_data->status & DP_STATUS_HPD_STATE;
+ /*
+ * Assume the first port to have HPD asserted is the one muxed
+ * to DP (i.e. active_port). When there's only one port this
+ * delays setting the active_port until HPD is asserted, but
+ * before that the drm_connector looks disconnected so
+ * active_port doesn't need to be set.
+ */
+ if (hpd_asserted && !typec_dp_bridge->active_port)
+ typec_dp_bridge->active_port = port;
+ /* Determine number of logical DP lanes from pin assignment */
+ pin_assign = DP_CONF_GET_PIN_ASSIGN(dp_data->conf);
+ if (pin_assign == DP_PIN_ASSIGN_D)
+ num_lanes = 2;
+ else
+ num_lanes = 4;
+ max_lanes = typec_dp_bridge->max_lanes;
+ port->num_dp_lanes = min(num_lanes, max_lanes);
+
+ /* Only notify hpd state for the port that has entered DP mode. */
+ if (typec_dp_bridge->hpd_enabled && typec_dp_bridge->active_port == port) {
if (hpd_asserted)
drm_bridge_hpd_notify(bridge, connector_status_connected);
else
@@ -278,6 +312,81 @@ bridge_to_cros_typec_dp_bridge(struct drm_bridge *bridge)
return container_of(bridge, struct cros_typec_dp_bridge, bridge);
}
+static int dp_lane_to_typec_lane(unsigned int dp_lane)
+{
+ switch (dp_lane) {
+ case 0:
+ return 2;
+ case 1:
+ return 3;
+ case 2:
+ return 1;
+ case 3:
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static int typec_to_dp_lane(unsigned int typec_lane)
+{
+ switch (typec_lane) {
+ case 0:
+ return 3;
+ case 1:
+ return 2;
+ case 2:
+ return 0;
+ case 3:
+ return 1;
+ }
+
+ return -EINVAL;
+}
+
+static int cros_typec_dp_bridge_atomic_check(struct drm_bridge *bridge,
+ struct drm_bridge_state *bridge_state,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ struct cros_typec_dp_bridge *typec_dp_bridge;
+ struct drm_lane_cfg *in_lanes;
+ size_t num_lanes;
+ struct cros_typec_port *port;
+ int i, typec_lane;
+
+ typec_dp_bridge = bridge_to_cros_typec_dp_bridge(bridge);
+ if (!typec_dp_bridge->active_port)
+ return -ENODEV;
+
+ port = typec_dp_bridge->active_port;
+
+ num_lanes = port->num_dp_lanes;
+ in_lanes = kcalloc(num_lanes, sizeof(*in_lanes), GFP_KERNEL);
+ if (!in_lanes)
+ return -ENOMEM;
+
+ bridge_state->input_bus_cfg.lanes = in_lanes;
+ bridge_state->input_bus_cfg.num_lanes = num_lanes;
+
+ for (i = 0; i < num_lanes; i++) {
+ /* Get physical type-c lane for DP lane */
+ typec_lane = dp_lane_to_typec_lane(i);
+ if (typec_lane < 0) {
+ DRM_ERROR("Invalid type-c lane configuration\n");
+ return -EINVAL;
+ }
+
+ /* Map to logical type-c lane */
+ typec_lane = port->lane_mapping[typec_lane];
+
+ /* Map logical type-c lane to logical DP lane */
+ in_lanes[i].logical = typec_to_dp_lane(typec_lane);
+ }
+
+ return 0;
+}
+
static void cros_typec_dp_bridge_hpd_enable(struct drm_bridge *bridge)
{
struct cros_typec_dp_bridge *typec_dp_bridge;
@@ -296,6 +405,10 @@ static void cros_typec_dp_bridge_hpd_disable(struct drm_bridge *bridge)
static const struct drm_bridge_funcs cros_typec_dp_bridge_funcs = {
.attach = cros_typec_dp_bridge_attach,
+ .atomic_check = cros_typec_dp_bridge_atomic_check,
+ .atomic_reset = drm_atomic_helper_bridge_reset,
+ .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+ .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
.hpd_enable = cros_typec_dp_bridge_hpd_enable,
.hpd_disable = cros_typec_dp_bridge_hpd_disable,
};
@@ -305,6 +418,7 @@ static int cros_typec_register_dp_bridge(struct cros_typec_switch_data *sdata,
{
struct cros_typec_dp_bridge *typec_dp_bridge;
struct drm_bridge *bridge;
+ int num_lanes;
struct device *dev = sdata->dev;
typec_dp_bridge = devm_kzalloc(dev, sizeof(*typec_dp_bridge), GFP_KERNEL);
@@ -313,6 +427,12 @@ static int cros_typec_register_dp_bridge(struct cros_typec_switch_data *sdata,
typec_dp_bridge->sdata = sdata;
sdata->typec_dp_bridge = typec_dp_bridge;
+
+ num_lanes = fwnode_property_count_u32(fwnode, "data-lanes");
+ if (num_lanes < 0)
+ num_lanes = 4;
+ typec_dp_bridge->max_lanes = num_lanes;
+
bridge = &typec_dp_bridge->bridge;
bridge->funcs = &cros_typec_dp_bridge_funcs;
@@ -333,6 +453,7 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
struct fwnode_handle *port_node;
u32 index;
int ret;
+ const u32 default_lane_mapping[] = { 0, 1, 2, 3 };
const char *prop_name;
port = devm_kzalloc(dev, sizeof(*port), GFP_KERNEL);
@@ -359,6 +480,11 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
port->port_num = index;
sdata->ports[index] = port;
+ if (fwnode_property_read_u32_array(fwnode, "data-lanes",
+ port->lane_mapping,
+ ARRAY_SIZE(port->lane_mapping)))
+ memcpy(port->lane_mapping, default_lane_mapping, sizeof(default_lane_mapping));
+
port_node = fwnode;
if (np)
fwnode = fwnode_graph_get_port_parent(fwnode);
--
https://chromeos.dev
Describe the set of pins used to connect the detachable keyboard on
detachable ChromeOS devices. The set of pins is called the "pogo pins".
It's basically USB 2.0 with an extra pin for base detection. We expect
to find a keyboard on the other side of this connector with a specific
vid/pid, so describe that as a child device at the port of the usb
device connected upstream.
Cc: Rob Herring <[email protected]>
Cc: Krzysztof Kozlowski <[email protected]>
Cc: Conor Dooley <[email protected]>
Cc: Benson Leung <[email protected]>
Cc: Guenter Roeck <[email protected]>
Cc: <[email protected]>
Cc: <[email protected]>
Cc: Pin-yen Lin <[email protected]>
Signed-off-by: Stephen Boyd <[email protected]>
---
.../chrome/google,pogo-pin-connector.yaml | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
create mode 100644 Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
diff --git a/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
new file mode 100644
index 000000000000..5ba68fd95fcd
--- /dev/null
+++ b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
@@ -0,0 +1,61 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/chrome/google,pogo-pin-connector.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Google Pogo Pin Connector
+
+maintainers:
+ - Stephen Boyd <[email protected]>
+
+properties:
+ compatible:
+ const: google,pogo-pin-connector
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ port:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Connection to USB2 port providing USB signals
+ required:
+ - endpoint
+
+patternProperties:
+ "^keyboard@[0-9a-f]{1,2}$":
+ description: The detachable keyboard
+ type: object
+ $ref: /schemas/usb/usb-device.yaml
+
+required:
+ - compatible
+ - '#address-cells'
+ - '#size-cells'
+ - port
+
+additionalProperties: false
+
+examples:
+ - |
+ connector {
+ compatible = "google,pogo-pin-connector";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ keyboard@2 {
+ compatible = "usb18d1,504c";
+ reg = <2>;
+ };
+
+ port {
+ pogo_connector_in: endpoint {
+ remote-endpoint = <&usb_hub_dsp3_hs>;
+ };
+ };
+ };
+
+...
--
https://chromeos.dev
On Fri, 09 Feb 2024 23:09:14 -0800, Stephen Boyd wrote:
> Add a graph with 4 output endpoints to this hub binding to support the
> scenario where a downstream facing port is connected to a device that
> isn't a connector or a USB device with a VID:PID. This will be used to
> connect downstream facing ports to USB type-c switches so the USB
> superspeed and high speed lanes can be put onto USB connectors.
>
> Cc: Greg Kroah-Hartman <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Conor Dooley <[email protected]>
> Cc: Matthias Kaehlcke <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Cc: maciek swiech <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../bindings/usb/realtek,rts5411.yaml | 50 +++++++++++++++++++
> 1 file changed, 50 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dts:32.21-31: Warning (reg_format): /example-0/usb/hub@1/device@2:reg: property has invalid length (4 bytes) (#address-cells == 2, #size-cells == 1)
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dtb: Warning (pci_device_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dtb: Warning (pci_device_bus_num): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dtb: Warning (simple_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dtb: Warning (i2c_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dtb: Warning (spi_bus_reg): Failed prerequisite 'reg_format'
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dts:30.26-33.19: Warning (avoid_default_addr_size): /example-0/usb/hub@1/device@2: Relying on default #address-cells value
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dts:30.26-33.19: Warning (avoid_default_addr_size): /example-0/usb/hub@1/device@2: Relying on default #size-cells value
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dtb: Warning (unique_unit_address_if_enabled): Failed prerequisite 'avoid_default_addr_size'
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dts:43.23-50.19: Warning (graph_port): /example-0/usb/hub@2/ports: graph port node name should be 'port'
Documentation/devicetree/bindings/usb/realtek,rts5411.example.dts:46.28-49.23: Warning (graph_endpoint): /example-0/usb/hub@2/ports/port@0: graph endpoint node name should be 'endpoint'
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
On Sat, 10 Feb 2024 at 09:16, Stephen Boyd <[email protected]> wrote:
>
> It's simpler to reason about things if we disable nodes instead of
> deleting them. Disable the second usb type-c connector node on
> quackingstick instead of deleting it so that we can reason about ports
> more easily.
>
> Cc: <[email protected]>
> Cc: Andy Gross <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Konrad Dybcio <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Conor Dooley <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Sat, 10 Feb 2024 at 09:16, Stephen Boyd <[email protected]> wrote:
>
> We put a header before modifying pinctrl nodes defined in
> sc7180-trogdor.dtsi in every other file. Add one here so we know that
> this section is for pinctrl modifications.
>
> Cc: <[email protected]>
> Cc: Andy Gross <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Konrad Dybcio <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Conor Dooley <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Sat, 10 Feb 2024 at 09:17, Stephen Boyd <[email protected]> wrote:
>
> At a high-level, detachable Trogdors (sometimes known as Strongbads)
> don't have a cros_ec keyboard, while all clamshell Trogdors (only known
> as Trogdors) always have a cros_ec keyboard. Looking closer though, all
> clamshells have a USB type-A connector and a hardwired USB camera. And
> all detachables replace the USB camera with a MIPI based one and swap
> the USB type-a connector for the detachable keyboard pogo pins.
>
> Split the detachable and clamshell bits into different files so we can
> describe these differences in one place instead of in each board that
> includes sc7180-trogdor.dtsi. For now this is just the keyboard part,
> but eventually this will include the type-a port and the pogo pins.
>
> Cc: <[email protected]>
> Cc: Andy Gross <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Konrad Dybcio <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Conor Dooley <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../boot/dts/qcom/sc7180-trogdor-clamshell.dtsi | 9 +++++++++
> arch/arm64/boot/dts/qcom/sc7180-trogdor-coachz.dtsi | 5 +----
> .../boot/dts/qcom/sc7180-trogdor-detachable.dtsi | 12 ++++++++++++
> .../arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi | 7 +------
> .../boot/dts/qcom/sc7180-trogdor-kingoftown.dts | 2 +-
> arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi | 3 +--
> arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi | 2 +-
> arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi | 2 +-
> .../boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi | 7 +------
> arch/arm64/boot/dts/qcom/sc7180-trogdor-r1.dts | 2 +-
> .../boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi | 5 +----
> 11 files changed, 30 insertions(+), 26 deletions(-)
> create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
> create mode 100644 arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
>
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On Sat, 10 Feb 2024 at 09:14, Stephen Boyd <[email protected]> wrote:
>
> We can imagine that logically the EC is a device that has some number of
> DisplayPort (DP) connector inputs, some number of USB3 connector inputs,
> and some number of USB type-c connector outputs. If you squint enough it
> looks like a USB type-c dock. Logically there's a crossbar pin
> assignment capability within the EC that can assign USB and DP lanes to
> USB type-c lanes in the connector (i.e. USB type-c pin configurations).
> In reality, the EC is a microcontroller that has some TCPCs and
> redrivers connected to it over something like i2c and DP/USB from the AP
> is wired directly to those ICs, not the EC.
>
> This design allows the EC to abstract many possible USB and DP hardware
> configurations away from the AP (kernel) so that the AP can largely deal
> with USB and DP without thinking about USB Type-C much at all. The DP
> and USB data originate in the AP, not the EC, so it helps to think that
> the EC takes the DP and USB data as input to mux onto USB type-c ports
> even if it really doesn't do that. With this split design, the EC
> forwards the DP HPD state to the AP via a GPIO that's connected to the
> DP phy.
>
> Having that HPD state signaled directly to the DP phy uses precious
> hardware resources, a GPIO or two and a wire, and it also forces the
> TCPM to live on the EC. If we want to save costs and move more control
> of USB type-c to the kernel it's in our interest to get rid of the HPD
> GPIO entirely and signal HPD to the DP phy some other way. Luckily, the
> EC already exposes information about the USB Type-C stack to the kernel
> via the host command interface in the "google,cros-ec-typec" compatible
> driver, which parses EC messages related to USB type-c and effectively
> "replays" those messages to the kernel's USB typec subsystem. This
> includes the state of HPD, which can be interrogated and acted upon by
> registering a 'struct typec_mux_dev' with the typec subsystem.
>
> On DT based systems, the DP display pipeline is abstracted via a 'struct
> drm_bridge'. If we want to signal HPD state within the kernel we need to
> hook into the drm_bridge framework somehow to call
> drm_bridge_hpd_notify() when HPD state changes in the typec framework.
> Make a drm_bridge in the EC that attaches onto the end of the DP bridge
> chain and logically moves the display data onto a usb-c-connector.
> Signal HPD when the typec HPD state changes, as long as this new
> drm_bridge is the one that's supposed to signal HPD. Do that by
> registering a 'struct typec_mux_dev' with the typec framework and
> associating that struct with a usb-c-connector node and a drm_bridge.
>
> To keep this patch minimal, just signal HPD state to the drm_bridge
> chain. Later patches will add more features. Eventually we'll be able to
> inform userspace about which usb-c-connector node is displaying DP and
> what USB devices are connected to a connector. Note that this code is
> placed in the cros_typec_switch driver because that's where mode-switch
> devices on the EC are controlled by the AP. Logically this drm_bridge
> sits in front of the mode-switch on the EC, and if there is anything to
> control on the EC the 'EC_FEATURE_TYPEC_AP_MUX_SET' feature will be set.
>
> Cc: Prashant Malani <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Tzung-Bi Shih <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> drivers/platform/chrome/Kconfig | 3 +-
> drivers/platform/chrome/cros_typec_switch.c | 218 ++++++++++++++++++--
> 2 files changed, 204 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/platform/chrome/Kconfig b/drivers/platform/chrome/Kconfig
> index 7a83346bfa53..910aa8be9c84 100644
> --- a/drivers/platform/chrome/Kconfig
> +++ b/drivers/platform/chrome/Kconfig
> @@ -287,7 +287,8 @@ config CHROMEOS_PRIVACY_SCREEN
>
> config CROS_TYPEC_SWITCH
> tristate "ChromeOS EC Type-C Switch Control"
> - depends on MFD_CROS_EC_DEV && TYPEC && ACPI
> + depends on MFD_CROS_EC_DEV
> + depends on TYPEC
> default MFD_CROS_EC_DEV
> help
> If you say Y here, you get support for configuring the ChromeOS EC Type-C
> diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
> index 769de2889f2f..d8fb6662cf8d 100644
> --- a/drivers/platform/chrome/cros_typec_switch.c
> +++ b/drivers/platform/chrome/cros_typec_switch.c
> @@ -10,6 +10,7 @@
> #include <linux/delay.h>
> #include <linux/iopoll.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/platform_data/cros_ec_commands.h>
> #include <linux/platform_data/cros_ec_proto.h>
> #include <linux/platform_device.h>
> @@ -18,6 +19,15 @@
> #include <linux/usb/typec_mux.h>
> #include <linux/usb/typec_retimer.h>
>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_print.h>
> +
> +struct cros_typec_dp_bridge {
> + struct cros_typec_switch_data *sdata;
> + bool hpd_enabled;
> + struct drm_bridge bridge;
> +};
Is there any chance that you can use drm_dp_hpd_bridge_register() /
drm_aux_hpd_bridge_notify() instead of implementing another
drm_bridge?
If something is missing from the existing implementation we can
probably work that out.
> +
> /* Handles and other relevant data required for each port's switches. */
> struct cros_typec_port {
> int port_num;
> @@ -30,7 +40,9 @@ struct cros_typec_port {
> struct cros_typec_switch_data {
> struct device *dev;
> struct cros_ec_device *ec;
> + bool typec_cmd_supported;
> struct cros_typec_port *ports[EC_USB_PD_MAX_PORTS];
> + struct cros_typec_dp_bridge *typec_dp_bridge;
> };
>
> static int cros_typec_cmd_mux_set(struct cros_typec_switch_data *sdata, int port_num, u8 index,
> @@ -143,13 +155,60 @@ static int cros_typec_configure_mux(struct cros_typec_switch_data *sdata, int po
> return 0;
> }
>
> +static int cros_typec_dp_port_switch_set(struct typec_mux_dev *mode_switch,
> + struct typec_mux_state *state)
> +{
> + struct cros_typec_port *port;
> + const struct typec_displayport_data *dp_data;
> + struct cros_typec_dp_bridge *typec_dp_bridge;
> + struct drm_bridge *bridge;
> + bool hpd_asserted;
> +
> + port = typec_mux_get_drvdata(mode_switch);
> + typec_dp_bridge = port->sdata->typec_dp_bridge;
> + if (!typec_dp_bridge)
> + return 0;
> +
> + bridge = &typec_dp_bridge->bridge;
> +
> + if (state->mode == TYPEC_STATE_SAFE || state->mode == TYPEC_STATE_USB) {
> + if (typec_dp_bridge->hpd_enabled)
> + drm_bridge_hpd_notify(bridge, connector_status_disconnected);
> +
> + return 0;
> + }
> +
> + if (state->alt && state->alt->svid == USB_TYPEC_DP_SID) {
> + if (typec_dp_bridge->hpd_enabled) {
> + dp_data = state->data;
> + hpd_asserted = dp_data->status & DP_STATUS_HPD_STATE;
> +
> + if (hpd_asserted)
> + drm_bridge_hpd_notify(bridge, connector_status_connected);
> + else
> + drm_bridge_hpd_notify(bridge, connector_status_disconnected);
> + }
> + }
> +
> + return 0;
> +}
> +
> static int cros_typec_mode_switch_set(struct typec_mux_dev *mode_switch,
> struct typec_mux_state *state)
> {
> struct cros_typec_port *port = typec_mux_get_drvdata(mode_switch);
> + struct cros_typec_switch_data *sdata = port->sdata;
> + int ret;
> +
> + ret = cros_typec_dp_port_switch_set(mode_switch, state);
> + if (ret)
> + return ret;
>
> /* Mode switches have index 0. */
> - return cros_typec_configure_mux(port->sdata, port->port_num, 0, state->mode, state->alt);
> + if (sdata->typec_cmd_supported)
> + return cros_typec_configure_mux(port->sdata, port->port_num, 0, state->mode, state->alt);
> +
> + return 0;
> }
>
> static int cros_typec_retimer_set(struct typec_retimer *retimer, struct typec_retimer_state *state)
> @@ -201,12 +260,77 @@ static int cros_typec_register_retimer(struct cros_typec_port *port, struct fwno
> return PTR_ERR_OR_ZERO(port->retimer);
> }
>
> +static int
> +cros_typec_dp_bridge_attach(struct drm_bridge *bridge,
> + enum drm_bridge_attach_flags flags)
> +{
> + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) {
> + DRM_ERROR("Fix bridge driver to make connector optional!\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static struct cros_typec_dp_bridge *
> +bridge_to_cros_typec_dp_bridge(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct cros_typec_dp_bridge, bridge);
> +}
> +
> +static void cros_typec_dp_bridge_hpd_enable(struct drm_bridge *bridge)
> +{
> + struct cros_typec_dp_bridge *typec_dp_bridge;
> +
> + typec_dp_bridge = bridge_to_cros_typec_dp_bridge(bridge);
> + typec_dp_bridge->hpd_enabled = true;
> +}
> +
> +static void cros_typec_dp_bridge_hpd_disable(struct drm_bridge *bridge)
> +{
> + struct cros_typec_dp_bridge *typec_dp_bridge;
> +
> + typec_dp_bridge = bridge_to_cros_typec_dp_bridge(bridge);
> + typec_dp_bridge->hpd_enabled = false;
> +}
> +
> +static const struct drm_bridge_funcs cros_typec_dp_bridge_funcs = {
> + .attach = cros_typec_dp_bridge_attach,
> + .hpd_enable = cros_typec_dp_bridge_hpd_enable,
> + .hpd_disable = cros_typec_dp_bridge_hpd_disable,
> +};
> +
> +static int cros_typec_register_dp_bridge(struct cros_typec_switch_data *sdata,
> + struct fwnode_handle *fwnode)
> +{
> + struct cros_typec_dp_bridge *typec_dp_bridge;
> + struct drm_bridge *bridge;
> + struct device *dev = sdata->dev;
> +
> + typec_dp_bridge = devm_kzalloc(dev, sizeof(*typec_dp_bridge), GFP_KERNEL);
> + if (!typec_dp_bridge)
> + return -ENOMEM;
> +
> + typec_dp_bridge->sdata = sdata;
> + sdata->typec_dp_bridge = typec_dp_bridge;
> + bridge = &typec_dp_bridge->bridge;
> +
> + bridge->funcs = &cros_typec_dp_bridge_funcs;
> + bridge->of_node = dev->of_node;
> + bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
> + bridge->ops |= DRM_BRIDGE_OP_HPD;
> +
> + return devm_drm_bridge_add(dev, bridge);
> +}
> +
> static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
> struct fwnode_handle *fwnode)
> {
> struct cros_typec_port *port;
> struct device *dev = sdata->dev;
> struct acpi_device *adev;
> + struct device_node *np;
> + struct fwnode_handle *port_node;
> u32 index;
> int ret;
> const char *prop_name;
> @@ -218,9 +342,12 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
> adev = to_acpi_device_node(fwnode);
> if (adev)
> prop_name = "_ADR";
> + np = to_of_node(fwnode);
> + if (np)
> + prop_name = "reg";
>
> - if (!adev)
> - return dev_err_probe(fwnode->dev, -ENODEV, "Couldn't get ACPI handle\n");
> + if (!adev && !np)
> + return dev_err_probe(fwnode->dev, -ENODEV, "Couldn't get ACPI/OF device handle\n");
>
> ret = fwnode_property_read_u32(fwnode, prop_name, &index);
> if (ret)
> @@ -232,41 +359,84 @@ static int cros_typec_register_port(struct cros_typec_switch_data *sdata,
> port->port_num = index;
> sdata->ports[index] = port;
>
> + port_node = fwnode;
> + if (np)
> + fwnode = fwnode_graph_get_port_parent(fwnode);
> +
> if (fwnode_property_present(fwnode, "retimer-switch")) {
> - ret = cros_typec_register_retimer(port, fwnode);
> - if (ret)
> - return dev_err_probe(dev, ret, "Retimer switch register failed\n");
> + ret = cros_typec_register_retimer(port, port_node);
> + if (ret) {
> + dev_err_probe(dev, ret, "Retimer switch register failed\n");
> + goto out;
> + }
>
> dev_dbg(dev, "Retimer switch registered for index %u\n", index);
> }
>
> - if (!fwnode_property_present(fwnode, "mode-switch"))
> - return 0;
> + if (fwnode_property_present(fwnode, "mode-switch")) {
> + ret = cros_typec_register_mode_switch(port, port_node);
> + if (ret) {
> + dev_err_probe(dev, ret, "Mode switch register failed\n");
> + goto out;
> + }
>
> - ret = cros_typec_register_mode_switch(port, fwnode);
> - if (ret)
> - return dev_err_probe(dev, ret, "Mode switch register failed\n");
> + dev_dbg(dev, "Mode switch registered for index %u\n", index);
> + }
>
> - dev_dbg(dev, "Mode switch registered for index %u\n", index);
>
> +out:
> + if (np)
> + fwnode_handle_put(fwnode);
> return ret;
> }
>
> static int cros_typec_register_switches(struct cros_typec_switch_data *sdata)
> {
> struct device *dev = sdata->dev;
> + struct fwnode_handle *devnode;
> struct fwnode_handle *fwnode;
> + struct fwnode_endpoint endpoint;
> int nports, ret;
>
> nports = device_get_child_node_count(dev);
> if (nports == 0)
> return dev_err_probe(dev, -ENODEV, "No switch devices found\n");
>
> - device_for_each_child_node(dev, fwnode) {
> - ret = cros_typec_register_port(sdata, fwnode);
> - if (ret) {
> + devnode = dev_fwnode(dev);
> + if (fwnode_graph_get_endpoint_count(devnode, 0)) {
> + fwnode_graph_for_each_endpoint(devnode, fwnode) {
> + ret = fwnode_graph_parse_endpoint(fwnode, &endpoint);
> + if (ret) {
> + fwnode_handle_put(fwnode);
> + goto err;
> + }
> + /* Skip if not a type-c output port */
> + if (endpoint.port != 2)
> + continue;
> +
> + ret = cros_typec_register_port(sdata, fwnode);
> + if (ret) {
> + fwnode_handle_put(fwnode);
> + goto err;
> + }
> + }
> + } else {
> + device_for_each_child_node(dev, fwnode) {
> + ret = cros_typec_register_port(sdata, fwnode);
> + if (ret) {
> + fwnode_handle_put(fwnode);
> + goto err;
> + }
> + }
> + }
> +
> + if (fwnode_property_present(devnode, "mode-switch")) {
> + fwnode = fwnode_graph_get_endpoint_by_id(devnode, 0, 0, 0);
> + if (fwnode) {
> + ret = cros_typec_register_dp_bridge(sdata, fwnode);
> fwnode_handle_put(fwnode);
> - goto err;
> + if (ret)
> + goto err;
> }
> }
>
> @@ -280,6 +450,7 @@ static int cros_typec_switch_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> struct cros_typec_switch_data *sdata;
> + struct cros_ec_dev *ec_dev;
>
> sdata = devm_kzalloc(dev, sizeof(*sdata), GFP_KERNEL);
> if (!sdata)
> @@ -288,6 +459,12 @@ static int cros_typec_switch_probe(struct platform_device *pdev)
> sdata->dev = dev;
> sdata->ec = dev_get_drvdata(pdev->dev.parent);
>
> + ec_dev = dev_get_drvdata(&sdata->ec->ec->dev);
> + if (!ec_dev)
> + return -EPROBE_DEFER;
> +
> + sdata->typec_cmd_supported = cros_ec_check_features(ec_dev, EC_FEATURE_TYPEC_AP_MUX_SET);
> +
> platform_set_drvdata(pdev, sdata);
>
> return cros_typec_register_switches(sdata);
> @@ -308,10 +485,19 @@ static const struct acpi_device_id cros_typec_switch_acpi_id[] = {
> MODULE_DEVICE_TABLE(acpi, cros_typec_switch_acpi_id);
> #endif
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id cros_typec_switch_of_match_table[] = {
> + { .compatible = "google,cros-ec-typec-switch" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, cros_typec_switch_of_match_table);
> +#endif
> +
> static struct platform_driver cros_typec_switch_driver = {
> .driver = {
> .name = "cros-typec-switch",
> .acpi_match_table = ACPI_PTR(cros_typec_switch_acpi_id),
> + .of_match_table = of_match_ptr(cros_typec_switch_of_match_table),
> },
> .probe = cros_typec_switch_probe,
> .remove_new = cros_typec_switch_remove,
> --
> https://chromeos.dev
>
>
--
With best wishes
Dmitry
Quoting Dmitry Baryshkov (2024-02-10 06:10:31)
> On Sat, 10 Feb 2024 at 09:14, Stephen Boyd <[email protected]> wrote:
> > diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
> > index 769de2889f2f..d8fb6662cf8d 100644
> > --- a/drivers/platform/chrome/cros_typec_switch.c
> > +++ b/drivers/platform/chrome/cros_typec_switch.c
> > @@ -18,6 +19,15 @@
> > #include <linux/usb/typec_mux.h>
> > #include <linux/usb/typec_retimer.h>
> >
> > +#include <drm/drm_bridge.h>
> > +#include <drm/drm_print.h>
> > +
> > +struct cros_typec_dp_bridge {
> > + struct cros_typec_switch_data *sdata;
> > + bool hpd_enabled;
> > + struct drm_bridge bridge;
> > +};
>
> Is there any chance that you can use drm_dp_hpd_bridge_register() /
> drm_aux_hpd_bridge_notify() instead of implementing another
> drm_bridge?
> If something is missing from the existing implementation we can
> probably work that out.
Yeah I think that can work. I had put the drm_bridge in this driver
because I needed a 'struct device' per DP phy, but I think that problem
goes away with an auxiliary device, so that is nicely solved.
I'll have to add logic about typec ports to that auxiliary driver
though, like mapping data-lanes and handling lane assignments. And then
I'll move this code from the cros_typec_switch driver to the
cros_ec_typec driver so it can be called outside of the typec mux set
path. That's probably better because it's sort of bolted on to the
cros_typec_switch driver. We'll need to know if the DP phy needs to
handle orientation or if the EC is doing that somehow, so probably I'll
need to add a DT property to the google,cros-ec-typec binding to
indicate that orientation control is needed.
It looks like I should add a new auxiliary device, like
'dp_typec_bridge', and have some other function like
drm_dp_typec_bridge_register() for that. I can wrap the 'struct
drm_aux_hpd_bridge_data' with a 'struct drm_aux_typec_bridge_data' and
then the typec port information can live there. HPD can still be
signaled with drm_aux_hpd_bridge_notify() but other functions can be
used to set the active typec port, e.g.
drm_aux_typec_bridge_set_active_port(), and then get orientation with
typec_get_orientation() in the atomic_check().
On Sun, 11 Feb 2024 at 10:52, Stephen Boyd <[email protected]> wrote:
>
> Quoting Dmitry Baryshkov (2024-02-10 06:10:31)
> > On Sat, 10 Feb 2024 at 09:14, Stephen Boyd <[email protected]> wrote:
> > > diff --git a/drivers/platform/chrome/cros_typec_switch.c b/drivers/platform/chrome/cros_typec_switch.c
> > > index 769de2889f2f..d8fb6662cf8d 100644
> > > --- a/drivers/platform/chrome/cros_typec_switch.c
> > > +++ b/drivers/platform/chrome/cros_typec_switch.c
> > > @@ -18,6 +19,15 @@
> > > #include <linux/usb/typec_mux.h>
> > > #include <linux/usb/typec_retimer.h>
> > >
> > > +#include <drm/drm_bridge.h>
> > > +#include <drm/drm_print.h>
> > > +
> > > +struct cros_typec_dp_bridge {
> > > + struct cros_typec_switch_data *sdata;
> > > + bool hpd_enabled;
> > > + struct drm_bridge bridge;
> > > +};
> >
> > Is there any chance that you can use drm_dp_hpd_bridge_register() /
> > drm_aux_hpd_bridge_notify() instead of implementing another
> > drm_bridge?
> > If something is missing from the existing implementation we can
> > probably work that out.
>
> Yeah I think that can work. I had put the drm_bridge in this driver
> because I needed a 'struct device' per DP phy, but I think that problem
> goes away with an auxiliary device, so that is nicely solved.
>
> I'll have to add logic about typec ports to that auxiliary driver
> though, like mapping data-lanes and handling lane assignments. And then
> I'll move this code from the cros_typec_switch driver to the
> cros_ec_typec driver so it can be called outside of the typec mux set
> path. That's probably better because it's sort of bolted on to the
> cros_typec_switch driver. We'll need to know if the DP phy needs to
> handle orientation or if the EC is doing that somehow, so probably I'll
> need to add a DT property to the google,cros-ec-typec binding to
> indicate that orientation control is needed.
I still haven't fully got into your usage of data-lanes. I hope to be
able to comment on that part and on the ports / endpoints tomorrow.
>
> It looks like I should add a new auxiliary device, like
> 'dp_typec_bridge', and have some other function like
> drm_dp_typec_bridge_register() for that. I can wrap the 'struct
> drm_aux_hpd_bridge_data' with a 'struct drm_aux_typec_bridge_data' and
> then the typec port information can live there. HPD can still be
> signaled with drm_aux_hpd_bridge_notify() but other functions can be
> used to set the active typec port, e.g.
> drm_aux_typec_bridge_set_active_port(), and then get orientation with
> typec_get_orientation() in the atomic_check().
--
With best wishes
Dmitry
On 10/02/2024 08:09, Stephen Boyd wrote:
> The ChromeOS embedded controller (EC) supports setting the state of
> GPIOs when the system is unlocked, and getting the state of GPIOs in all
> cases. The GPIOs are on the EC itself, so the EC acts similar to a GPIO
> expander. Add a binding to describe these GPIOs in DT so that other
> devices described in DT can read the GPIOs on the EC.
..
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cros-ec@0 {
> + compatible = "google,cros-ec-spi";
> + reg = <0>;
> + interrupts = <101 0>;
This is should be proper define but then are you sure interrupt is type
NONE? Does not look right.
Best regards,
Krzysztof
On 10/02/2024 08:09, Stephen Boyd wrote:
> Add a binding for the USB type-c switch controls found on some ChromeOS
> Embedded Controllers (ECs). When this device is a mode switch, it takes
> one DisplayPort (DP) port as input and some number (possibly zero) of
> USB SuperSpeed ports (bundles of USB SS lanes) as input, and muxes those
> lanes into USB type-c SuperSpeed lanes suitable for the SSTRX1/2 pins on
> a usb-c-connector. When this device is an orientation switch, it
> redirects the DP lanes to the proper USB type-c SSTRX lanes.
>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Conor Dooley <[email protected]>
> Cc: Lee Jones <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Prashant Malani <[email protected]>
> Cc: Tzung-Bi Shih <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../chrome/google,cros-ec-typec-switch.yaml | 365 ++++++++++++++++++
> .../bindings/mfd/google,cros-ec.yaml | 5 +
> 2 files changed, 370 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
>
> diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> new file mode 100644
> index 000000000000..17a0ba928f5d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> @@ -0,0 +1,365 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/chrome/google,cros-ec-typec-switch.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Google Chrome OS EC(Embedded Controller) USB Type C Switch
> +
> +maintainers:
> + - Benson Leung <[email protected]>
> + - Prashant Malani <[email protected]>
> + - Stephen Boyd <[email protected]>
> +
> +description:
> + Chrome OS devices have an Embedded Controller(EC) which has access to USB
> + Type C switching. This node is intended to allow the OS to control Type C
> + signal muxing for USB-C orientation and alternate modes. The node for this
> + device should be under a cros-ec node like google,cros-ec-spi.
> +
If this is USB Type C switch, then you miss reference to
usb-switch.yaml, but then ports look a bit different.
> +properties:
> + compatible:
> + const: google,cros-ec-typec-switch
> +
> + mode-switch:
> + description: Indicates this device controls altmode switching
> + type: boolean
> +
> + orientation-switch:
> + description: Indicates this device controls orientation switching
> + type: boolean
> +
> + mux-gpios:
> + description: GPIOs indicating which way the DP mux is steered
missing maxItems
> +
> + no-hpd:
> + description: Indicates this device doesn't signal HPD for DisplayPort
> + type: boolean
> +
> + ports:
> + $ref: /schemas/graph.yaml#/properties/ports
> +
> + properties:
> + port@0:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + description: Input port to receive DisplayPort (DP) data
> + unevaluatedProperties: false
> +
> + properties:
> + endpoint@0:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + description: DisplayPort data
> + unevaluatedProperties: false
> + properties:
> + data-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + An array of physical DP data lane indexes
> + - 0 is DP ML0 lane
> + - 1 is DP ML1 lane
> + - 2 is DP ML2 lane
> + - 3 is DP ML3 lane
> + oneOf:
> + - items:
> + - const: 0
> + - const: 1
> + - items:
> + - const: 0
> + - const: 1
> + - const: 2
> + - const: 3
> +
> + required:
> + - endpoint@0
> +
> + port@1:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Input port to receive USB SuperSpeed (SS) data
> + properties:
> + endpoint@0:
> + $ref: /schemas/graph.yaml#/properties/endpoint
> + description: USB SS data
> +
> + endpoint@1:
> + $ref: /schemas/graph.yaml#/properties/endpoint
> + description: USB SS data
> +
> + anyOf:
> + - required:
> + - endpoint@0
> + - required:
> + - endpoint@1
> +
> + port@2:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + unevaluatedProperties: false
> + description:
> + Output port for USB-C data
> + properties:
> + endpoint@0:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + description: USB-C data
> + unevaluatedProperties: false
> + properties:
> + data-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + An array of physical USB-C data lane indexes.
> + - 0 is SSRX1 lane
> + - 1 is SSTX1 lane
> + - 2 is SSTX2 lane
> + - 3 is SSRX2 lane
> + minItems: 4
> + maxItems: 4
> + items:
> + maximum: 3
> +
> + endpoint@1:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + description: USB-C data for EC's 1st type-c port
> + unevaluatedProperties: false
> + properties:
> + data-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + An array of physical USB-C data lane indexes.
> + - 0 is SSRX1 lane
> + - 1 is SSTX1 lane
> + - 2 is SSTX2 lane
> + - 3 is SSRX2 lane
> + minItems: 4
> + maxItems: 4
> + items:
> + maximum: 3
> +
> + endpoint@2:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + description: USB-C data for EC's 2nd type-c port
> + unevaluatedProperties: false
> + properties:
> + data-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + An array of physical USB-C data lane indexes.
> + - 0 is SSRX1 lane
> + - 1 is SSTX1 lane
> + - 2 is SSTX2 lane
> + - 3 is SSRX2 lane
> + minItems: 4
> + maxItems: 4
> + items:
> + maximum: 3
> +
> + endpoint@3:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + description: USB-C data for EC's 3rd type-c port
> + unevaluatedProperties: false
> + properties:
> + data-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + An array of physical USB-C data lane indexes.
> + - 0 is SSRX1 lane
> + - 1 is SSTX1 lane
> + - 2 is SSTX2 lane
> + - 3 is SSRX2 lane
> + minItems: 4
> + maxItems: 4
> + items:
> + maximum: 3
> +
> + endpoint@4:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + description: USB-C data for EC's 4th type-c port
> + unevaluatedProperties: false
> + properties:
> + data-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + An array of physical USB-C data lane indexes.
> + - 0 is SSRX1 lane
> + - 1 is SSTX1 lane
> + - 2 is SSTX2 lane
> + - 3 is SSRX2 lane
> + minItems: 4
> + maxItems: 4
> + items:
> + maximum: 3
> +
> + endpoint@5:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + description: USB-C data for EC's 5th type-c port
> + unevaluatedProperties: false
> + properties:
> + data-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + An array of physical USB-C data lane indexes.
> + - 0 is SSRX1 lane
> + - 1 is SSTX1 lane
> + - 2 is SSTX2 lane
> + - 3 is SSRX2 lane
> + minItems: 4
> + maxItems: 4
> + items:
> + maximum: 3
> +
> + endpoint@6:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + description: USB-C data for EC's 6th type-c port
> + unevaluatedProperties: false
> + properties:
> + data-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + An array of physical USB-C data lane indexes.
> + - 0 is SSRX1 lane
> + - 1 is SSTX1 lane
> + - 2 is SSTX2 lane
> + - 3 is SSRX2 lane
> + minItems: 4
> + maxItems: 4
> + items:
> + maximum: 3
> +
> + endpoint@7:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + description: USB-C data for EC's 7th type-c port
> + unevaluatedProperties: false
> + properties:
> + data-lanes:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + An array of physical USB-C data lane indexes.
> + - 0 is SSRX1 lane
> + - 1 is SSTX1 lane
> + - 2 is SSTX2 lane
> + - 3 is SSRX2 lane
> + minItems: 4
> + maxItems: 4
> + items:
> + maximum: 3
> +
> + anyOf:
> + - required:
> + - endpoint@0
I don't get what you want to say here. This anyOf should have no effect.
> + - required:
> + - endpoint@1
> + - required:
> + - endpoint@2
> + - required:
> + - endpoint@3
> + - required:
> + - endpoint@4
> + - required:
> + - endpoint@5
> + - required:
> + - endpoint@6
> + - required:
> + - endpoint@7
> +
> + required:
> + - port@2
> + anyOf:
> + - required:
> + - port@0
> + - required:
> + - port@1
Same problem here
> +
> +required:
> + - compatible
> + - ports
> +
> +allOf:
> + - if:
> + properties:
> + no-hpd: true
I don't understand this either. What is it for? Where did you see such
syntax?
> + required:
> + - no-hpd> + then:
> + properties:
> + ports:
> + required:
> + - port@0
> + - if:
> + properties:
> + mode-switch: true
> + required:
> + - mode-switch
> + then:
> + properties:
> + ports:
> + required:
> + - port@0
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cros_ec: ec@0 {
> + compatible = "google,cros-ec-spi";
> + reg = <0>;
> + interrupts = <35 0>;
Same concerns about interrupts.
> +
> + typec-switch {
> + compatible = "google,cros-ec-typec-switch";
> + mode-switch;
> + orientation-switch;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + dp_in: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&dp_phy>;
> + data-lanes = <0 1>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + usb_in_0: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&usb_ss_0_out>;
> + };
> +
> + usb_in_1: endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <&usb_ss_1_out>;
> + };
> + };
> +
> + port@2 {
> + reg = <2>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cros_typec_c0_ss: endpoint@0 {
> + reg = <0>;
> + remote-endpoint = <&usb_c0_ss>;
> + };
> +
> + cros_typec_c1_ss: endpoint@1 {
> + reg = <1>;
> + remote-endpoint = <&usb_c1_ss>;
> + };
> + };
> + };
> + };
> + };
> + };
> +...
> diff --git a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> index ded396b28fba..563c51a4a39c 100644
> --- a/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> +++ b/Documentation/devicetree/bindings/mfd/google,cros-ec.yaml
> @@ -164,6 +164,10 @@ patternProperties:
> type: object
> $ref: /schemas/extcon/extcon-usbc-cros-ec.yaml#
>
> + "^typec-switch[0-9]*$":
Missing - after typec-switch (e.g. typec-switch-1)
> + type: object
> + $ref: /schemas/chrome/google,cros-ec-typec-switch.yaml#
Best regards,
Krzysztof
On 10/02/2024 08:09, Stephen Boyd wrote:
> Add a binding for the USB type-c switch controls found on some ChromeOS
> Embedded Controllers (ECs). When this device is a mode switch, it takes
> one DisplayPort (DP) port as input and some number (possibly zero) of
> USB SuperSpeed ports (bundles of USB SS lanes) as input, and muxes those
> lanes into USB type-c SuperSpeed lanes suitable for the SSTRX1/2 pins on
> a usb-c-connector. When this device is an orientation switch, it
> redirects the DP lanes to the proper USB type-c SSTRX lanes.
>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Conor Dooley <[email protected]>
> Cc: Lee Jones <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: Prashant Malani <[email protected]>
> Cc: Tzung-Bi Shih <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../chrome/google,cros-ec-typec-switch.yaml | 365 ++++++++++++++++++
> .../bindings/mfd/google,cros-ec.yaml | 5 +
> 2 files changed, 370 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
Ah, and wrong placement. There is no hardware called "chrome", please
don't stuff things there. USB switches go to other USB switches (git
grep usb-switch.yaml will give you hints).
Best regards,
Krzysztof
On 10/02/2024 08:09, Stephen Boyd wrote:
> Describe the set of pins used to connect the detachable keyboard on
> detachable ChromeOS devices. The set of pins is called the "pogo pins".
> It's basically USB 2.0 with an extra pin for base detection. We expect
> to find a keyboard on the other side of this connector with a specific
> vid/pid, so describe that as a child device at the port of the usb
> device connected upstream.
>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Conor Dooley <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../chrome/google,pogo-pin-connector.yaml | 61 +++++++++++++++++++
> 1 file changed, 61 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
>
> diff --git a/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
> new file mode 100644
> index 000000000000..5ba68fd95fcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
> @@ -0,0 +1,61 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/chrome/google,pogo-pin-connector.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Google Pogo Pin Connector
> +
> +maintainers:
> + - Stephen Boyd <[email protected]>
> +
Missing description describing the hardware.
> +properties:
> + compatible:
> + const: google,pogo-pin-connector
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + port:
> + $ref: /schemas/graph.yaml#/properties/port
> + description: Connection to USB2 port providing USB signals
> + required:
> + - endpoint
Drop required.
> +
> +patternProperties:
> + "^keyboard@[0-9a-f]{1,2}$":
> + description: The detachable keyboard
If this is detachable why do you define it in DT? Only hard-wired USB
devices, which need some sort of special handling. are described in DT.
> + type: object
> + $ref: /schemas/usb/usb-device.yaml
On this level:
unevaluatedProperties: false
> +
> +required:
> + - compatible
> + - '#address-cells'
> + - '#size-cells'
Use one type of quotes, either ' or ".
> + - port
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + connector {
> + compatible = "google,pogo-pin-connector";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + keyboard@2 {
> + compatible = "usb18d1,504c";
Messed indentation.
> + reg = <2>;
> + };
> +
Best regards,
Krzysztof
Hi Stephen,
thanks for your patch!
Overall it looks good I have a few nitpicks
On Sat, Feb 10, 2024 at 8:09 AM Stephen Boyd <[email protected]> wrote:
> The ChromeOS embedded controller (EC) supports setting the state of
> GPIOs when the system is unlocked, and getting the state of GPIOs in all
> cases. The GPIOs are on the EC itself, so the EC acts similar to a GPIO
> expander. Add a driver to get and set the GPIOs on the EC through the
> host command interface.
>
> Cc: Linus Walleij <[email protected]>
> Cc: Bartosz Golaszewski <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
(...)
> +config GPIO_CROS_EC
> + tristate "ChromeOS EC GPIO support"
> + depends on CROS_EC
> + help
> + GPIO driver for exposing GPIOs on the ChromeOS Embedded
> + Controller.
> +
> + This driver can also be built as a module. If so, the module
> + will be called gpio-cros-ec.
Classified as "MFD Expander" but I honestly don't know anything better.
> +#include <linux/bitops.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/init.h>
Do you need init.h?
I guess maybe... I thought module would be enough for this.
> +static int cros_ec_gpio_request(struct gpio_chip *chip, unsigned gpio_pin)
> +{
> + if (gpio_pin < chip->ngpio)
> + return 0;
> +
> + return -EINVAL;
> +}
If this check is needed, it should be in gpiolib I think?
> +#ifdef CONFIG_OF
This ifdef should not be needed these days.
> +static const struct of_device_id cros_ec_gpio_of_match[] = {
> + { .compatible = "google,cros-ec-gpio" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, cros_ec_gpio_of_match);
> +#endif
> +
> +static struct platform_driver cros_ec_gpio_driver = {
> + .probe = cros_ec_gpio_probe,
> + .driver = {
> + .name = "cros-ec-gpio",
> + .of_match_table = of_match_ptr(cros_ec_gpio_of_match),
Nor the of_match_ptr() business.
With these fixed/addressed:
Reviewed-by: Linus Walleij <[email protected]>
Yours,
Linus Walleij
Hi,
On Fri, Feb 9, 2024 at 11:10 PM Stephen Boyd <[email protected]> wrote:
>
> It's simpler to reason about things if we disable nodes instead of
> deleting them. Disable the second usb type-c connector node on
> quackingstick instead of deleting it so that we can reason about ports
> more easily.
>
> Cc: <[email protected]>
> Cc: Andy Gross <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Konrad Dybcio <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Conor Dooley <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../arm64/boot/dts/qcom/sc7180-trogdor-quackingstick.dtsi | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
Bjorn: happy to see this landed any time to shorten Stephen's series.
Reviewed-by: Douglas Anderson <[email protected]>
Hi,
On Fri, Feb 9, 2024 at 11:10 PM Stephen Boyd <[email protected]> wrote:
>
> We put a header before modifying pinctrl nodes defined in
> sc7180-trogdor.dtsi in every other file. Add one here so we know that
> this section is for pinctrl modifications.
>
> Cc: <[email protected]>
> Cc: Andy Gross <[email protected]>
> Cc: Bjorn Andersson <[email protected]>
> Cc: Konrad Dybcio <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Conor Dooley <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
Bjorn: happy to see this landed any time to shorten Stephen's series.
Reviewed-by: Douglas Anderson <[email protected]>
Hi,
On Fri, Feb 9, 2024 at 11:10 PM Stephen Boyd <[email protected]> wrote:
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
> new file mode 100644
> index 000000000000..bcf3df463f80
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-clamshell.dtsi
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Trogdor dts framgent for clamshells
s/framgent/fragment
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
> new file mode 100644
> index 000000000000..ab0f30288871
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-detachable.dtsi
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Google Trogdor dts framgent for detachables
s/framgent/fragment
> + * Copyright 2024 Google LLC.
> + */
> +
Tiny nit: should this file have a comment like "/* This file must be
included after sc7180-trogdor.dtsi */" like the clamshell file?
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> index e9f213d27711..c3fd6760de7a 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> @@ -5,8 +5,7 @@
> * Copyright 2020 Google LLC.
> */
>
> -/* This file must be included after sc7180-trogdor.dtsi */
> -#include <arm/cros-ec-keyboard.dtsi>
> +#include "sc7180-trogdor-clamshell.dtsi"
nit: Not that it was terribly consistent before, but in lazor you
remove the "This file must be included after sc7180-trogdor.dtsi"
because (I guess) it moved to the clamshell file. However, in other
dts files you don't remove it. pazquel has the exact same comment and
it's not removed. Pompom has a slight variant of the comment where it
explains the reason (to modify cros_ec) and it's not removed. Could
make it more consistent...
Everything above is either tiny typos or nits, so happy enough with:
Reviewed-by: Douglas Anderson <[email protected]>
Hi,
On Fri, Feb 9, 2024 at 11:09 PM Stephen Boyd <[email protected]> wrote:
>
> When a USB hub is described in DT, such as any device that matches the
> onboard-hub driver, the connect_type is set to "unknown" or
> USB_PORT_CONNECT_TYPE_UNKNOWN. This makes any device plugged into that
> USB port report their 'removable' device attribute as "unknown". Improve
> the connect_type attribute for ports, and in turn the removable
> attribute for USB devices, by looking for child devices with a reg
> property or an OF graph when the device is described in DT.
>
> If the graph exists, endpoints that are connected to a remote node must
> be something like a usb-{a,b,c}-connector compatible node, or an
> intermediate node like a redriver, and not a hardwired USB device on the
> board. Set the connect_type to USB_PORT_CONNECT_TYPE_HOT_PLUG in this
> case because the device is going to be plugged in. Set the connect_type
> to USB_PORT_CONNECT_TYPE_HARD_WIRED if there's a child node for the port
> like 'device@2' for port2. Set the connect_type to USB_PORT_NOT_USED if
> there isn't an endpoint or child node corresponding to the port number.
The above sounds good, but then I look at patch #18 ("dt-bindings:
chrome: Add binding for ChromeOS Pogo pin connector") and patch #22
("arm64: dts: qcom: sc7180-trogdor: Wire up USB and DP to
usb-c-connectors") and it makes my Spidey Sense tingle.
Specifically, I _think_ if a port is "hard wired" that can sometimes
tell the system that the port is a bit more trusted. In the case of
the "pogo" pins on detachables, though, I don't _think_ there's
anything that prevents someone from making a "pogo to USB A port"
adapter and then you could plug anything you wanted into the pogo
port. If there's any extra trust given to these "internal" ports a
nefarious attacker could presumably abuse that trust for the pogo
pins.
I have no idea if this is a realistic concern or not. I'm about 95%
sure that hardwired "PCIe" ports get extra trust and get "deferred
IOMMU flush" enabled and, in the case of PCIe, that actually is a real
security hole. For USB, though, I think the system is more isolated by
the USB host controller so I'm not sure that there is any extra trust
given to "hard wired" ports. ...so maybe the answer here is to just
ignore my rambling. ...or maybe the answer here is that everything is
fine but patches #18 and #22 should be modified not to cause the pogo
pins to be considered as "hard wired" since they really aren't...
-Doug
Hi,
On Fri, Feb 9, 2024 at 11:10 PM Stephen Boyd <[email protected]> wrote:
>
> Describe the set of pins used to connect the detachable keyboard on
> detachable ChromeOS devices. The set of pins is called the "pogo pins".
> It's basically USB 2.0 with an extra pin for base detection. We expect
> to find a keyboard on the other side of this connector with a specific
> vid/pid, so describe that as a child device at the port of the usb
> device connected upstream.
Can you remind me what the side effects would be if a different
VID/PID shows up there? I know it's not an end-user scenario, but I
have a pre-production "coachz" keyboard that's actually programmed
incorrectly and shows up as the wrong PID. Presumably I could either
throw the old hardware away or figure out a way to re-program it and
it's really not a big deal, but just curious what happens...
-Doug
Quoting Doug Anderson (2024-02-13 16:03:11)
> Hi,
>
> On Fri, Feb 9, 2024 at 11:09 PM Stephen Boyd <[email protected]> wrote:
> >
> > When a USB hub is described in DT, such as any device that matches the
> > onboard-hub driver, the connect_type is set to "unknown" or
> > USB_PORT_CONNECT_TYPE_UNKNOWN. This makes any device plugged into that
> > USB port report their 'removable' device attribute as "unknown". Improve
> > the connect_type attribute for ports, and in turn the removable
> > attribute for USB devices, by looking for child devices with a reg
> > property or an OF graph when the device is described in DT.
> >
> > If the graph exists, endpoints that are connected to a remote node must
> > be something like a usb-{a,b,c}-connector compatible node, or an
> > intermediate node like a redriver, and not a hardwired USB device on the
> > board. Set the connect_type to USB_PORT_CONNECT_TYPE_HOT_PLUG in this
> > case because the device is going to be plugged in. Set the connect_type
> > to USB_PORT_CONNECT_TYPE_HARD_WIRED if there's a child node for the port
> > like 'device@2' for port2. Set the connect_type to USB_PORT_NOT_USED if
> > there isn't an endpoint or child node corresponding to the port number.
>
> The above sounds good, but then I look at patch #18 ("dt-bindings:
> chrome: Add binding for ChromeOS Pogo pin connector") and patch #22
> ("arm64: dts: qcom: sc7180-trogdor: Wire up USB and DP to
> usb-c-connectors") and it makes my Spidey Sense tingle.
>
> Specifically, I _think_ if a port is "hard wired" that can sometimes
> tell the system that the port is a bit more trusted. In the case of
> the "pogo" pins on detachables, though, I don't _think_ there's
> anything that prevents someone from making a "pogo to USB A port"
> adapter and then you could plug anything you wanted into the pogo
> port. If there's any extra trust given to these "internal" ports a
> nefarious attacker could presumably abuse that trust for the pogo
> pins.
The pogo pins on detachables are connected via the OF graph, so they are
only ever going to be either "not connected" or "hotplug". They can't
become "hard wired" because they're not a child node with a reg property
corresponding to the USB port.
>
> I have no idea if this is a realistic concern or not. I'm about 95%
> sure that hardwired "PCIe" ports get extra trust and get "deferred
> IOMMU flush" enabled and, in the case of PCIe, that actually is a real
> security hole. For USB, though, I think the system is more isolated by
> the USB host controller so I'm not sure that there is any extra trust
> given to "hard wired" ports. ...so maybe the answer here is to just
> ignore my rambling. ...or maybe the answer here is that everything is
> fine but patches #18 and #22 should be modified not to cause the pogo
> pins to be considered as "hard wired" since they really aren't...
>
Pogo pins should be considered hot plug with this patch series, but I
will double check that I didn't mess up that logic.
Quoting Krzysztof Kozlowski (2024-02-11 05:39:36)
> On 10/02/2024 08:09, Stephen Boyd wrote:
> > diff --git a/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
> > new file mode 100644
> > index 000000000000..5ba68fd95fcd
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
> > @@ -0,0 +1,61 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
[...]
>
> > +properties:
> > + compatible:
> > + const: google,pogo-pin-connector
> > +
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + port:
> > + $ref: /schemas/graph.yaml#/properties/port
> > + description: Connection to USB2 port providing USB signals
> > + required:
> > + - endpoint
>
> Drop required.
Why? I'd like to make it so you can't have the node defined without
connecting it up to the rest of the system. Is that bad?
>
>
> > +
> > +patternProperties:
> > + "^keyboard@[0-9a-f]{1,2}$":
> > + description: The detachable keyboard
>
> If this is detachable why do you define it in DT? Only hard-wired USB
> devices, which need some sort of special handling. are described in DT.
From the commit text:
We expect to find a keyboard on the other side of this connector with a
specific vid/pid, so describe that as a child device at the port of the
usb device connected upstream.
ChromeOS userspace is checking that the connected device downstream of
this port has the expected vid/pid to quickly rule out USB keyboards
that aren't the detachable keyboard. I wanted to express this in DT so
that it didn't live in ChromeOS userspace forever.
Quoting Doug Anderson (2024-02-13 17:17:34)
> Hi,
>
> On Fri, Feb 9, 2024 at 11:10 PM Stephen Boyd <[email protected]> wrote:
> >
> > Describe the set of pins used to connect the detachable keyboard on
> > detachable ChromeOS devices. The set of pins is called the "pogo pins".
> > It's basically USB 2.0 with an extra pin for base detection. We expect
> > to find a keyboard on the other side of this connector with a specific
> > vid/pid, so describe that as a child device at the port of the usb
> > device connected upstream.
>
> Can you remind me what the side effects would be if a different
> VID/PID shows up there? I know it's not an end-user scenario, but I
> have a pre-production "coachz" keyboard that's actually programmed
> incorrectly and shows up as the wrong PID. Presumably I could either
> throw the old hardware away or figure out a way to re-program it and
> it's really not a big deal, but just curious what happens...
As far as I know nothing happens besides ChromeOS userspace treats the
keyboard as "external" so things like smarter base detection, e.g.
wraparound keyboard detection or kickstand mode, may not work. I think
you get a popup box telling you the keyboard isn't the trusted one.
Quoting Doug Anderson (2024-02-13 15:34:38)
> Hi,
>
> On Fri, Feb 9, 2024 at 11:10 PM Stephen Boyd <[email protected]> wrote:
> >
> > + * Copyright 2024 Google LLC.
> > + */
> > +
>
> Tiny nit: should this file have a comment like "/* This file must be
> included after sc7180-trogdor.dtsi */" like the clamshell file?
I was copying the comment for the keyboard include file, but the
detachable doesn't have that include. I can add it here though, that's
fine.
>
>
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> > index e9f213d27711..c3fd6760de7a 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi
> > @@ -5,8 +5,7 @@
> > * Copyright 2020 Google LLC.
> > */
> >
> > -/* This file must be included after sc7180-trogdor.dtsi */
> > -#include <arm/cros-ec-keyboard.dtsi>
> > +#include "sc7180-trogdor-clamshell.dtsi"
>
> nit: Not that it was terribly consistent before, but in lazor you
> remove the "This file must be included after sc7180-trogdor.dtsi"
> because (I guess) it moved to the clamshell file. However, in other
> dts files you don't remove it. pazquel has the exact same comment and
> it's not removed. Pompom has a slight variant of the comment where it
> explains the reason (to modify cros_ec) and it's not removed. Could
> make it more consistent...
Sure I can make it more consistent with the explanation. Which way to go
though? Remove it from the boards and put it into the fragment files?
Hi,
On Wed, Feb 14, 2024 at 4:35 PM Stephen Boyd <[email protected]> wrote:
>
> > > -/* This file must be included after sc7180-trogdor.dtsi */
> > > -#include <arm/cros-ec-keyboard.dtsi>
> > > +#include "sc7180-trogdor-clamshell.dtsi"
> >
> > nit: Not that it was terribly consistent before, but in lazor you
> > remove the "This file must be included after sc7180-trogdor.dtsi"
> > because (I guess) it moved to the clamshell file. However, in other
> > dts files you don't remove it. pazquel has the exact same comment and
> > it's not removed. Pompom has a slight variant of the comment where it
> > explains the reason (to modify cros_ec) and it's not removed. Could
> > make it more consistent...
>
> Sure I can make it more consistent with the explanation. Which way to go
> though? Remove it from the boards and put it into the fragment files?
I don't care too much. I guess I'd lean toward just putting it in the
fragment files just to copy it fewer times.
-Doug
Quoting Krzysztof Kozlowski (2024-02-11 05:26:33)
> On 10/02/2024 08:09, Stephen Boyd wrote:
> > The ChromeOS embedded controller (EC) supports setting the state of
> > GPIOs when the system is unlocked, and getting the state of GPIOs in all
> > cases. The GPIOs are on the EC itself, so the EC acts similar to a GPIO
> > expander. Add a binding to describe these GPIOs in DT so that other
> > devices described in DT can read the GPIOs on the EC.
>
> ...
>
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + spi {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + cros-ec@0 {
> > + compatible = "google,cros-ec-spi";
> > + reg = <0>;
> > + interrupts = <101 0>;
>
> This is should be proper define but then are you sure interrupt is type
> NONE? Does not look right.
>
I copied this from cros-ec-typec.yaml so I'll have to fix them all!
Quoting Krzysztof Kozlowski (2024-02-11 05:34:25)
> On 10/02/2024 08:09, Stephen Boyd wrote:
> > Add a binding for the USB type-c switch controls found on some ChromeOS
> > Embedded Controllers (ECs). When this device is a mode switch, it takes
> > one DisplayPort (DP) port as input and some number (possibly zero) of
> > USB SuperSpeed ports (bundles of USB SS lanes) as input, and muxes those
> > lanes into USB type-c SuperSpeed lanes suitable for the SSTRX1/2 pins on
> > a usb-c-connector. When this device is an orientation switch, it
> > redirects the DP lanes to the proper USB type-c SSTRX lanes.
> >
> > Cc: Rob Herring <[email protected]>
> > Cc: Krzysztof Kozlowski <[email protected]>
> > Cc: Conor Dooley <[email protected]>
> > Cc: Lee Jones <[email protected]>
> > Cc: Benson Leung <[email protected]>
> > Cc: Guenter Roeck <[email protected]>
> > Cc: Prashant Malani <[email protected]>
> > Cc: Tzung-Bi Shih <[email protected]>
> > Cc: <[email protected]>
> > Cc: <[email protected]>
> > Cc: Pin-yen Lin <[email protected]>
> > Signed-off-by: Stephen Boyd <[email protected]>
> > ---
> > .../chrome/google,cros-ec-typec-switch.yaml | 365 ++++++++++++++++++
> > .../bindings/mfd/google,cros-ec.yaml | 5 +
> > 2 files changed, 370 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> > new file mode 100644
> > index 000000000000..17a0ba928f5d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
> > @@ -0,0 +1,365 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/chrome/google,cros-ec-typec-switch.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Google Chrome OS EC(Embedded Controller) USB Type C Switch
> > +
> > +maintainers:
> > + - Benson Leung <[email protected]>
> > + - Prashant Malani <[email protected]>
> > + - Stephen Boyd <[email protected]>
> > +
> > +description:
> > + Chrome OS devices have an Embedded Controller(EC) which has access to USB
> > + Type C switching. This node is intended to allow the OS to control Type C
> > + signal muxing for USB-C orientation and alternate modes. The node for this
> > + device should be under a cros-ec node like google,cros-ec-spi.
> > +
>
> If this is USB Type C switch, then you miss reference to
> usb-switch.yaml, but then ports look a bit different.
Thanks for the pointer. I suspect that's in linux-next only? I'm going
to move this to the cros-ec-typec.yaml file but I'll still need some
sort of property like 'mode-switch' or 'orientation-switch' to describe
what needs to be done in the kernel by changing lane assignments in the
drm_bridge.
One problem with those properties is that they're boolean for the whole
device. If I have a google,cros-ec-typec node that has two input DP
ports and two output USB-C ports then it may be that one port needs
orientation switching and the other only needs to do mode switching.
This needs to be expressed somehow and a top-level boolean property
doesn't do that. It could be part of the DP endpoint node itself.
Also I don't know how to indicate that mode switching can't be changed
here directly. For example, on Trogdor the mode is switched by the EC,
and the kernel can't change the mode. Changing DP lane assignments isn't
going to change the situation either. The mode is still going to be
something like DP+USB, or just DP, etc. Maybe there needs to be a
different property, like 'dp-mode-lane-assignment = <array of ports>',
that indicates which DP ports need to do lane assignment or
'dp-orientation-lane-assignment = <array of ports>' for orientation
control. Either way, I'm saying that 'mode-switch' and
'orientation-switch' properties don't make any sense here. I was using
them to wedge the code into the typec switch callbacks, but if I move
this into the cros-ec-typec driver then I don't need them.
>
> > +
> > + no-hpd:
> > + description: Indicates this device doesn't signal HPD for DisplayPort
> > + type: boolean
> > +
> > + ports:
[...]
> > +
> > + endpoint@7:
> > + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> > + description: USB-C data for EC's 7th type-c port
> > + unevaluatedProperties: false
> > + properties:
> > + data-lanes:
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > + description: |
> > + An array of physical USB-C data lane indexes.
> > + - 0 is SSRX1 lane
> > + - 1 is SSTX1 lane
> > + - 2 is SSTX2 lane
> > + - 3 is SSRX2 lane
> > + minItems: 4
> > + maxItems: 4
> > + items:
> > + maximum: 3
> > +
> > + anyOf:
> > + - required:
> > + - endpoint@0
>
> I don't get what you want to say here. This anyOf should have no effect.
I'm trying to say that there should be at least one usb-c data lane
output endpoint if there's a port@2 (usb-c output).
>
> > + - required:
> > + - endpoint@1
> > + - required:
> > + - endpoint@2
[...]
>
> > +
> > +required:
> > + - compatible
> > + - ports
> > +
> > +allOf:
> > + - if:
> > + properties:
> > + no-hpd: true
>
> I don't understand this either. What is it for? Where did you see such
> syntax?
This is saying that if the no-hpd property is present then the port@0
port (DP input port) is required. Otherwise no-hpd doesn't really make
any sense because this device isn't muxing DP onto USB type-c connectors.
I found this syntax by searching the schema website and reading this
page[1]. The last yellow note about "country" not being a required
property led me to this syntax.
[1] https://json-schema.org/understanding-json-schema/reference/conditionals#ifthenelse
On 15/02/2024 01:07, Stephen Boyd wrote:
> Quoting Krzysztof Kozlowski (2024-02-11 05:39:36)
>> On 10/02/2024 08:09, Stephen Boyd wrote:
>>> diff --git a/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
>>> new file mode 100644
>>> index 000000000000..5ba68fd95fcd
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/chrome/google,pogo-pin-connector.yaml
>>> @@ -0,0 +1,61 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
> [...]
>>
>>> +properties:
>>> + compatible:
>>> + const: google,pogo-pin-connector
>>> +
>>> + "#address-cells":
>>> + const: 1
>>> +
>>> + "#size-cells":
>>> + const: 0
>>> +
>>> + port:
>>> + $ref: /schemas/graph.yaml#/properties/port
>>> + description: Connection to USB2 port providing USB signals
>>> + required:
>>> + - endpoint
>>
>> Drop required.
>
> Why? I'd like to make it so you can't have the node defined without
> connecting it up to the rest of the system. Is that bad?
Hm, I double checked and you're right. I thought endpoint is required
anyway by graph.yaml in dtschema, but it seems it is not.
>
>>
>>
>>> +
>>> +patternProperties:
>>> + "^keyboard@[0-9a-f]{1,2}$":
>>> + description: The detachable keyboard
>>
>> If this is detachable why do you define it in DT? Only hard-wired USB
>> devices, which need some sort of special handling. are described in DT.
>
> From the commit text:
>
> We expect to find a keyboard on the other side of this connector with a
> specific vid/pid, so describe that as a child device at the port of the
> usb device connected upstream.
>
> ChromeOS userspace is checking that the connected device downstream of
> this port has the expected vid/pid to quickly rule out USB keyboards
> that aren't the detachable keyboard. I wanted to express this in DT so
> that it didn't live in ChromeOS userspace forever.
OK,
Best regards,
Krzysztof
On 15/02/2024 02:52, Stephen Boyd wrote:
> Quoting Krzysztof Kozlowski (2024-02-11 05:34:25)
>> On 10/02/2024 08:09, Stephen Boyd wrote:
>>> Add a binding for the USB type-c switch controls found on some ChromeOS
>>> Embedded Controllers (ECs). When this device is a mode switch, it takes
>>> one DisplayPort (DP) port as input and some number (possibly zero) of
>>> USB SuperSpeed ports (bundles of USB SS lanes) as input, and muxes those
>>> lanes into USB type-c SuperSpeed lanes suitable for the SSTRX1/2 pins on
>>> a usb-c-connector. When this device is an orientation switch, it
>>> redirects the DP lanes to the proper USB type-c SSTRX lanes.
>>>
>>> Cc: Rob Herring <[email protected]>
>>> Cc: Krzysztof Kozlowski <[email protected]>
>>> Cc: Conor Dooley <[email protected]>
>>> Cc: Lee Jones <[email protected]>
>>> Cc: Benson Leung <[email protected]>
>>> Cc: Guenter Roeck <[email protected]>
>>> Cc: Prashant Malani <[email protected]>
>>> Cc: Tzung-Bi Shih <[email protected]>
>>> Cc: <[email protected]>
>>> Cc: <[email protected]>
>>> Cc: Pin-yen Lin <[email protected]>
>>> Signed-off-by: Stephen Boyd <[email protected]>
>>> ---
>>> .../chrome/google,cros-ec-typec-switch.yaml | 365 ++++++++++++++++++
>>> .../bindings/mfd/google,cros-ec.yaml | 5 +
>>> 2 files changed, 370 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
>>> new file mode 100644
>>> index 000000000000..17a0ba928f5d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/chrome/google,cros-ec-typec-switch.yaml
>>> @@ -0,0 +1,365 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/chrome/google,cros-ec-typec-switch.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: Google Chrome OS EC(Embedded Controller) USB Type C Switch
>>> +
>>> +maintainers:
>>> + - Benson Leung <[email protected]>
>>> + - Prashant Malani <[email protected]>
>>> + - Stephen Boyd <[email protected]>
>>> +
>>> +description:
>>> + Chrome OS devices have an Embedded Controller(EC) which has access to USB
>>> + Type C switching. This node is intended to allow the OS to control Type C
>>> + signal muxing for USB-C orientation and alternate modes. The node for this
>>> + device should be under a cros-ec node like google,cros-ec-spi.
>>> +
>>
>> If this is USB Type C switch, then you miss reference to
>> usb-switch.yaml, but then ports look a bit different.
>
> Thanks for the pointer. I suspect that's in linux-next only? I'm going
Yes, Greg's tree.
> to move this to the cros-ec-typec.yaml file but I'll still need some
> sort of property like 'mode-switch' or 'orientation-switch' to describe
> what needs to be done in the kernel by changing lane assignments in the
> drm_bridge.
>
> One problem with those properties is that they're boolean for the whole
> device. If I have a google,cros-ec-typec node that has two input DP
> ports and two output USB-C ports then it may be that one port needs
> orientation switching and the other only needs to do mode switching.
> This needs to be expressed somehow and a top-level boolean property
> doesn't do that. It could be part of the DP endpoint node itself.
Maybe it can be added to the connector binding as well.
>
> Also I don't know how to indicate that mode switching can't be changed
> here directly. For example, on Trogdor the mode is switched by the EC,
> and the kernel can't change the mode. Changing DP lane assignments isn't
> going to change the situation either. The mode is still going to be
> something like DP+USB, or just DP, etc. Maybe there needs to be a
> different property, like 'dp-mode-lane-assignment = <array of ports>',
> that indicates which DP ports need to do lane assignment or
> 'dp-orientation-lane-assignment = <array of ports>' for orientation
> control. Either way, I'm saying that 'mode-switch' and
> 'orientation-switch' properties don't make any sense here. I was using
> them to wedge the code into the typec switch callbacks, but if I move
> this into the cros-ec-typec driver then I don't need them.
OK
>
>>
>>> +
>>> + no-hpd:
>>> + description: Indicates this device doesn't signal HPD for DisplayPort
>>> + type: boolean
>>> +
>>> + ports:
> [...]
>>> +
>>> + endpoint@7:
>>> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
>>> + description: USB-C data for EC's 7th type-c port
>>> + unevaluatedProperties: false
>>> + properties:
>>> + data-lanes:
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>> + description: |
>>> + An array of physical USB-C data lane indexes.
>>> + - 0 is SSRX1 lane
>>> + - 1 is SSTX1 lane
>>> + - 2 is SSTX2 lane
>>> + - 3 is SSRX2 lane
>>> + minItems: 4
>>> + maxItems: 4
>>> + items:
>>> + maximum: 3
>>> +
>>> + anyOf:
>>> + - required:
>>> + - endpoint@0
>>
>> I don't get what you want to say here. This anyOf should have no effect.
>
> I'm trying to say that there should be at least one usb-c data lane
> output endpoint if there's a port@2 (usb-c output).
Wait, my bad, this is good. At least one endpoint will be required.
>
>>
>>> + - required:
>>> + - endpoint@1
>>> + - required:
>>> + - endpoint@2
> [...]
>>
>>> +
>>> +required:
>>> + - compatible
>>> + - ports
>>> +
>>> +allOf:
>>> + - if:
>>> + properties:
>>> + no-hpd: true
>>
>> I don't understand this either. What is it for? Where did you see such
>> syntax?
>
> This is saying that if the no-hpd property is present then the port@0
> port (DP input port) is required. Otherwise no-hpd doesn't really make
> any sense because this device isn't muxing DP onto USB type-c connectors.
Then you only need if: required: like here:
https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/net/qcom,ipa.yaml#L174
>
> I found this syntax by searching the schema website and reading this
> page[1]. The last yellow note about "country" not being a required
> property led me to this syntax.
>
> [1] https://json-schema.org/understanding-json-schema/reference/conditionals#ifthenelse
Best regards,
Krzysztof
On Fri, Feb 09, 2024 at 11:09:12PM -0800, Stephen Boyd wrote:
> The ChromeOS embedded controller (EC) supports setting the state of
> GPIOs when the system is unlocked, and getting the state of GPIOs in all
> cases. The GPIOs are on the EC itself, so the EC acts similar to a GPIO
> expander. Add a binding to describe these GPIOs in DT so that other
> devices described in DT can read the GPIOs on the EC.
>
> Cc: Linus Walleij <[email protected]>
> Cc: Bartosz Golaszewski <[email protected]>
> Cc: Rob Herring <[email protected]>
> Cc: Krzysztof Kozlowski <[email protected]>
> Cc: Conor Dooley <[email protected]>
> Cc: Lee Jones <[email protected]>
> Cc: Benson Leung <[email protected]>
> Cc: Guenter Roeck <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: Pin-yen Lin <[email protected]>
> Signed-off-by: Stephen Boyd <[email protected]>
> ---
> .../bindings/gpio/google,cros-ec-gpio.yaml | 49 +++++++++++++++++++
> .../bindings/mfd/google,cros-ec.yaml | 3 ++
> 2 files changed, 52 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/gpio/google,cros-ec-gpio.yaml
>
> diff --git a/Documentation/devicetree/bindings/gpio/google,cros-ec-gpio.yaml b/Documentation/devicetree/bindings/gpio/google,cros-ec-gpio.yaml
> new file mode 100644
> index 000000000000..a9f1d7784070
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/google,cros-ec-gpio.yaml
> @@ -0,0 +1,49 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpio/google,cros-ec-gpio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIOs controlled by ChromeOS EC
> +
> +maintainers:
> + - Stephen Boyd <[email protected]>
> +
> +description:
> + Google's ChromeOS EC has a gpio controller inside the Embedded Controller
> + (EC) and controlled via a host-command interface. The node for this
> + device should be under a cros-ec node like google,cros-ec-spi.
Why do we need a child node here?
Rob
Quoting Rob Herring (2024-02-15 06:06:38)
> On Fri, Feb 09, 2024 at 11:09:12PM -0800, Stephen Boyd wrote:
> > +
> > +description:
> > + Google's ChromeOS EC has a gpio controller inside the Embedded Controller
> > + (EC) and controlled via a host-command interface. The node for this
> > + device should be under a cros-ec node like google,cros-ec-spi.
>
> Why do we need a child node here?
>
When in Rome... but I get your point. I will work on moving #gpio-cells
and gpio-controller into the cros-ec binding and populating some child
device from the mfd driver.