This series introduces a binding for Type-C data lane switches. These
control the routing and operating modes of USB Type-C data lanes based
on the PD messaging from the Type-C port driver regarding connected
peripherals.
The first patch introduces a change to the Type-C mux class mode-switch
matching code, while the second adds a config guard to a Type-C header.
The next couple of patches introduce the new "typec-switch" binding as
well as one user of it (the ANX7625 drm bridge).
The remaining patches add functionality to the anx7625 driver to
register the mode-switches, as well as program its crosspoint
switch depending on which Type-C port has a DisplayPort (DP) peripheral
connected to it.
v3: https://lore.kernel.org/linux-usb/[email protected]/
Changes since v3:
- Some more modifications to the anx7625 binding patch (4/7).
- Picked up 1 more Reviewed-by tag.
Pin-Yen Lin (1):
drm/bridge: anx7625: Add typec_mux_set callback function
Prashant Malani (6):
usb: typec: mux: Allow muxes to specify mode-switch
usb: typec: mux: Add CONFIG guards for functions
dt-bindings: usb: Add Type-C switch binding
dt-bindings: drm/bridge: anx7625: Add mode-switch support
drm/bridge: anx7625: Register number of Type C switches
drm/bridge: anx7625: Register Type-C mode switches
.../display/bridge/analogix,anx7625.yaml | 64 ++++++++
.../devicetree/bindings/usb/typec-switch.yaml | 74 +++++++++
drivers/gpu/drm/bridge/analogix/anx7625.c | 148 ++++++++++++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 20 +++
drivers/usb/typec/mux.c | 8 +-
include/linux/usb/typec_mux.h | 44 +++++-
6 files changed, 350 insertions(+), 8 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/typec-switch.yaml
--
2.36.1.476.g0c4daa206d-goog
Loosen the typec_mux_match() requirements so that searches where an
alt mode is not specified, but the target mux device lists the
"mode-switch" property, return a success.
This is helpful in Type C port drivers which would like to get a pointer
to the mux switch associated with a Type C port, but don't want to
specify a particular alt mode.
Signed-off-by: Prashant Malani <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
Tested-by: Nícolas F. R. A. Prado <[email protected]>
---
Changes since v3:
- No changes.
Changes since v2:
- Included Reviewed-by and Tested-by tags from v2.
Changes since v1:
- No changes.
drivers/usb/typec/mux.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index fd55c2c516a5..464330776cd6 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -281,9 +281,13 @@ static void *typec_mux_match(struct fwnode_handle *fwnode, const char *id,
if (match)
goto find_mux;
- /* Accessory Mode muxes */
if (!desc) {
- match = fwnode_property_present(fwnode, "accessory");
+ /*
+ * Accessory Mode muxes & muxes which explicitly specify
+ * the required identifier can avoid SVID matching.
+ */
+ match = fwnode_property_present(fwnode, "accessory") ||
+ fwnode_property_present(fwnode, id);
if (match)
goto find_mux;
return NULL;
--
2.36.1.476.g0c4daa206d-goog
Parse the "switches" node, if available, and count and store the number
of Type-C switches within it. Since we currently don't do anything with
this info, no functional changes are expected from this change.
This patch sets a foundation for the actual registering of Type-C
switches with the Type-C connector class framework.
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
Tested-by: Nícolas F. R. A. Prado <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---
Changes since v3:
- No changes.
Changes since v2:
- Move ret variable to Patch v3 6/7.
- Make error print a dev_dbg, since it is noisy.
- Added Reviewed-by and Tested-by tags.
Changes since v1:
- No changes.
drivers/gpu/drm/bridge/analogix/anx7625.c | 18 ++++++++++++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 1 +
2 files changed, 19 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 53a5da6c49dd..e3d4c2738b8c 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2581,6 +2581,20 @@ static void anx7625_runtime_disable(void *data)
pm_runtime_disable(data);
}
+static int anx7625_register_typec_switches(struct device *device, struct anx7625_data *ctx)
+{
+ struct device_node *of = of_get_child_by_name(device->of_node, "switches");
+
+ if (!of)
+ return -ENODEV;
+
+ ctx->num_typec_switches = of_get_child_count(of);
+ if (ctx->num_typec_switches <= 0)
+ return -ENODEV;
+
+ return 0;
+}
+
static int anx7625_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -2686,6 +2700,10 @@ static int anx7625_i2c_probe(struct i2c_client *client,
if (platform->pdata.intp_irq)
queue_work(platform->workqueue, &platform->work);
+ ret = anx7625_register_typec_switches(dev, platform);
+ if (ret)
+ dev_dbg(dev, "Didn't register Type C switches, err: %d\n", ret);
+
platform->bridge.funcs = &anx7625_bridge_funcs;
platform->bridge.of_node = client->dev.of_node;
if (!anx7625_of_panel_on_aux_bus(&client->dev))
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h
index e257a84db962..d5cbca708842 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -473,6 +473,7 @@ struct anx7625_data {
struct drm_connector *connector;
struct mipi_dsi_device *dsi;
struct drm_dp_aux aux;
+ int num_typec_switches;
};
#endif /* __ANX7625_H__ */
--
2.36.1.476.g0c4daa206d-goog
Analogix 7625 can be used in systems to switch USB Type-C DisplayPort
alternate mode lane traffic between 2 Type-C ports.
Update the binding to accommodate this usage by introducing a switch
property.
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
Tested-by: Nícolas F. R. A. Prado <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---
Changes since v3:
- Fix unevaluatedProperties usage.
- Add additionalProperties to top level "switches" nodes.
- Make quotes consistent.
- Add '^..$' to regex.
(All suggested by Krzysztof Kozlowski)
Changes since v2:
- Added Reviewed-by and Tested-by tags.
Changes since v1:
- Introduced patternProperties for "switch" children (suggested by
Krzysztof Kozlowski).
- Added unevaluatedProperties descriptor (suggested by Krzysztof
Kozlowski).
- Added "address-cells" and "size-cells" properties to "switches".
.../display/bridge/analogix,anx7625.yaml | 64 +++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
index 35a48515836e..bc6f7644db31 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
@@ -105,6 +105,34 @@ properties:
- port@0
- port@1
+ switches:
+ type: object
+ description: Set of switches controlling DisplayPort traffic on
+ outgoing RX/TX lanes to Type C ports.
+ additionalProperties: false
+
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ patternProperties:
+ '^switch@[01]$':
+ $ref: /schemas/usb/typec-switch.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ required:
+ - switch@0
+
required:
- compatible
- reg
@@ -167,5 +195,41 @@ examples:
};
};
};
+ switches {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ switch@0 {
+ compatible = "typec-switch";
+ reg = <0>;
+ mode-switch;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ anx_typec0: endpoint {
+ remote-endpoint = <&typec_port0>;
+ };
+ };
+ };
+ };
+ switch@1 {
+ compatible = "typec-switch";
+ reg = <1>;
+ mode-switch;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ anx_typec1: endpoint {
+ remote-endpoint = <&typec_port1>;
+ };
+ };
+ };
+ };
+ };
};
};
--
2.36.1.476.g0c4daa206d-goog
There are some drivers that can use the Type C mux API, but don't have
to. Introduce CONFIG guards for the mux functions so that drivers can
include the header file and not run into compilation errors on systems
which don't have CONFIG_TYPEC enabled. When CONFIG_TYPEC is not enabled,
the Type C mux functions will be stub versions of the original calls.
Reported-by: kernel test robot <[email protected]>
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
Tested-by: Nícolas F. R. A. Prado <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---
Changes since v3:
- No changes.
Changes since v2:
- Fix up return types for some of the stubs. Remove 1 unnecessary stub
in the else condition.
- Remove unnecessary IS_MODULE config guard.
- Added Reviewed-by and Tested-by tags.
Changes since v1:
- Added static inline to stub functions.
- Updated function signature of stub functions from "struct typec_mux"
to "struct typec_mux_dev" in accordance with updates from commit
713fd49b430c ("usb: typec: mux: Introduce indirection")
include/linux/usb/typec_mux.h | 44 ++++++++++++++++++++++++++++++-----
1 file changed, 38 insertions(+), 6 deletions(-)
diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
index ee57781dcf28..9292f0e07846 100644
--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h
@@ -58,17 +58,13 @@ struct typec_mux_desc {
void *drvdata;
};
+#if IS_ENABLED(CONFIG_TYPEC)
+
struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
const struct typec_altmode_desc *desc);
void typec_mux_put(struct typec_mux *mux);
int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state);
-static inline struct typec_mux *
-typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc)
-{
- return fwnode_typec_mux_get(dev_fwnode(dev), desc);
-}
-
struct typec_mux_dev *
typec_mux_register(struct device *parent, const struct typec_mux_desc *desc);
void typec_mux_unregister(struct typec_mux_dev *mux);
@@ -76,4 +72,40 @@ void typec_mux_unregister(struct typec_mux_dev *mux);
void typec_mux_set_drvdata(struct typec_mux_dev *mux, void *data);
void *typec_mux_get_drvdata(struct typec_mux_dev *mux);
+#else
+
+static inline struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
+ const struct typec_altmode_desc *desc)
+{
+ return NULL;
+}
+
+static inline void typec_mux_put(struct typec_mux *mux) {}
+
+static inline int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
+{
+ return 0;
+}
+
+static inline struct typec_mux_dev *
+typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+static inline void typec_mux_unregister(struct typec_mux_dev *mux) {}
+
+static inline void typec_mux_set_drvdata(struct typec_mux_dev *mux, void *data) {}
+static inline void *typec_mux_get_drvdata(struct typec_mux_dev *mux)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
+#endif /* CONFIG_TYPEC */
+
+static inline struct typec_mux *
+typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc)
+{
+ return fwnode_typec_mux_get(dev_fwnode(dev), desc);
+}
+
#endif /* __USB_TYPEC_MUX */
--
2.36.1.476.g0c4daa206d-goog
From: Pin-Yen Lin <[email protected]>
Add the callback function when the driver receives state
changes of the Type-C port. The callback function configures the
crosspoint switch of the anx7625 bridge chip, which can change the
output pins of the signals according to the port state.
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
Tested-by: Nícolas F. R. A. Prado <[email protected]>
Signed-off-by: Pin-Yen Lin <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---
Changes since v3:
- Added Reviewed-by tag from Angelo.
Changes since v2:
- Moved num_typec_switches check to beginning of function
- Made dp_connected assignments fit on one line (and removed unnecessary
parentheses)
- Added Reviewed-by and Tested-by tags.
Changes since v1:
- No changes.
drivers/gpu/drm/bridge/analogix/anx7625.c | 56 +++++++++++++++++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 13 ++++++
2 files changed, 69 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index bd21f159b973..5992fc8beeeb 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -15,6 +15,7 @@
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/usb/typec_dp.h>
#include <linux/usb/typec_mux.h>
#include <linux/workqueue.h>
@@ -2582,9 +2583,64 @@ static void anx7625_runtime_disable(void *data)
pm_runtime_disable(data);
}
+static void anx7625_set_crosspoint_switch(struct anx7625_data *ctx,
+ enum typec_orientation orientation)
+{
+ if (orientation == TYPEC_ORIENTATION_NORMAL) {
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0,
+ SW_SEL1_SSRX_RX1 | SW_SEL1_DPTX0_RX2);
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1,
+ SW_SEL2_SSTX_TX1 | SW_SEL2_DPTX1_TX2);
+ } else if (orientation == TYPEC_ORIENTATION_REVERSE) {
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_0,
+ SW_SEL1_SSRX_RX2 | SW_SEL1_DPTX0_RX1);
+ anx7625_reg_write(ctx, ctx->i2c.tcpc_client, TCPC_SWITCH_1,
+ SW_SEL2_SSTX_TX2 | SW_SEL2_DPTX1_TX1);
+ }
+}
+
+static void anx7625_typec_two_ports_update(struct anx7625_data *ctx)
+{
+ if (ctx->typec_ports[0].dp_connected && ctx->typec_ports[1].dp_connected)
+ /* Both ports available, do nothing to retain the current one. */
+ return;
+ else if (ctx->typec_ports[0].dp_connected)
+ anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_NORMAL);
+ else if (ctx->typec_ports[1].dp_connected)
+ anx7625_set_crosspoint_switch(ctx, TYPEC_ORIENTATION_REVERSE);
+}
+
static int anx7625_typec_mux_set(struct typec_mux_dev *mux,
struct typec_mux_state *state)
{
+ struct anx7625_port_data *data = typec_mux_get_drvdata(mux);
+ struct anx7625_data *ctx = data->ctx;
+ struct device *dev = &ctx->client->dev;
+ bool new_dp_connected, old_dp_connected;
+
+ if (ctx->num_typec_switches == 1)
+ return 0;
+
+ old_dp_connected = ctx->typec_ports[0].dp_connected || ctx->typec_ports[1].dp_connected;
+
+ dev_dbg(dev, "mux_set dp_connected: c0=%d, c1=%d\n",
+ ctx->typec_ports[0].dp_connected, ctx->typec_ports[1].dp_connected);
+
+ data->dp_connected = (state->alt && state->alt->svid == USB_TYPEC_DP_SID &&
+ state->alt->mode == USB_TYPEC_DP_MODE);
+
+ new_dp_connected = ctx->typec_ports[0].dp_connected || ctx->typec_ports[1].dp_connected;
+
+ /* dp on, power on first */
+ if (!old_dp_connected && new_dp_connected)
+ pm_runtime_get_sync(dev);
+
+ anx7625_typec_two_ports_update(ctx);
+
+ /* dp off, power off last */
+ if (old_dp_connected && !new_dp_connected)
+ pm_runtime_put_sync(dev);
+
return 0;
}
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 76cfc64f7574..7d6c6fdf9a3a 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -55,6 +55,18 @@
#define HPD_STATUS_CHANGE 0x80
#define HPD_STATUS 0x80
+#define TCPC_SWITCH_0 0xB4
+#define SW_SEL1_DPTX0_RX2 BIT(0)
+#define SW_SEL1_DPTX0_RX1 BIT(1)
+#define SW_SEL1_SSRX_RX2 BIT(4)
+#define SW_SEL1_SSRX_RX1 BIT(5)
+
+#define TCPC_SWITCH_1 0xB5
+#define SW_SEL2_DPTX1_TX2 BIT(0)
+#define SW_SEL2_DPTX1_TX1 BIT(1)
+#define SW_SEL2_SSTX_TX2 BIT(4)
+#define SW_SEL2_SSTX_TX1 BIT(5)
+
/******** END of I2C Address 0x58 ********/
/***************************************************************/
@@ -444,6 +456,7 @@ struct anx7625_i2c_client {
};
struct anx7625_port_data {
+ bool dp_connected;
struct typec_mux_dev *typec_mux;
struct anx7625_data *ctx;
};
--
2.36.1.476.g0c4daa206d-goog
Introduce a binding which represents a component that can control the
routing of USB Type-C data lines as well as address data line
orientation (based on CC lines' orientation).
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
Tested-by: Nícolas F. R. A. Prado <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---
Changes since v3:
- No changes.
Changes since v2:
- Added Reviewed-by and Tested-by tags.
Changes since v1:
- Removed "items" from compatible.
- Fixed indentation in example.
.../devicetree/bindings/usb/typec-switch.yaml | 74 +++++++++++++++++++
1 file changed, 74 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/typec-switch.yaml
diff --git a/Documentation/devicetree/bindings/usb/typec-switch.yaml b/Documentation/devicetree/bindings/usb/typec-switch.yaml
new file mode 100644
index 000000000000..78b0190c8543
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/typec-switch.yaml
@@ -0,0 +1,74 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/typec-switch.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: USB Type-C Switch
+
+maintainers:
+ - Prashant Malani <[email protected]>
+
+description:
+ A USB Type-C switch represents a component which routes USB Type-C data
+ lines to various protocol host controllers (e.g USB, VESA DisplayPort,
+ Thunderbolt etc.) depending on which mode the Type-C port, port partner
+ and cable are operating in. It can also modify lane routing based on
+ the orientation of a connected Type-C peripheral.
+
+properties:
+ compatible:
+ const: typec-switch
+
+ mode-switch:
+ type: boolean
+ description: Specify that this switch can handle alternate mode switching.
+
+ orientation-switch:
+ type: boolean
+ description: Specify that this switch can handle orientation switching.
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+ description: OF graph binding modelling data lines to the Type-C switch.
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/properties/port
+ description: Link between the switch and a Type-C connector.
+
+ required:
+ - port@0
+
+required:
+ - compatible
+ - ports
+
+anyOf:
+ - required:
+ - mode-switch
+ - required:
+ - orientation-switch
+
+additionalProperties: true
+
+examples:
+ - |
+ drm-bridge {
+ usb-switch {
+ compatible = "typec-switch";
+ mode-switch;
+ orientation-switch;
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ anx_ep: endpoint {
+ remote-endpoint = <&typec_controller>;
+ };
+ };
+ };
+ };
+ };
--
2.36.1.476.g0c4daa206d-goog
When the DT node has "switches" available, register a Type-C mode-switch
for each listed "switch". This allows the driver to receive state
information about what operating mode a Type-C port and its connected
peripherals are in, as well as status information (like VDOs) related to
that state.
The callback function is currently a stub, but subsequent patches will
implement the required functionality.
Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
Tested-by: Nícolas F. R. A. Prado <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---
Changes since v3:
- No changes.
Changes since v2:
- Updated dev_info() to dev_warn() print, but added a check to ensure it
only triggers on non -ENODEV errors.
- Made conflict resolutions resulting from changes introduced in
Patch v3 5/7 (add ret variable here instead of in Patch v3 5/7).
- Added Reviewed-by and Tested-by tags.
Changes since v1:
- No changes.
drivers/gpu/drm/bridge/analogix/anx7625.c | 82 +++++++++++++++++++++--
drivers/gpu/drm/bridge/analogix/anx7625.h | 6 ++
2 files changed, 84 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index e3d4c2738b8c..bd21f159b973 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -15,6 +15,7 @@
#include <linux/regulator/consumer.h>
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/usb/typec_mux.h>
#include <linux/workqueue.h>
#include <linux/of_gpio.h>
@@ -2581,10 +2582,61 @@ static void anx7625_runtime_disable(void *data)
pm_runtime_disable(data);
}
+static int anx7625_typec_mux_set(struct typec_mux_dev *mux,
+ struct typec_mux_state *state)
+{
+ return 0;
+}
+
+static int anx7625_register_mode_switch(struct device *dev, struct device_node *node,
+ struct anx7625_data *ctx)
+{
+ struct anx7625_port_data *port_data;
+ struct typec_mux_desc mux_desc = {};
+ char name[32];
+ u32 port_num;
+ int ret;
+
+ ret = of_property_read_u32(node, "reg", &port_num);
+ if (ret)
+ return ret;
+
+ if (port_num >= ctx->num_typec_switches) {
+ dev_err(dev, "Invalid port number specified: %d\n", port_num);
+ return -EINVAL;
+ }
+
+ port_data = &ctx->typec_ports[port_num];
+ port_data->ctx = ctx;
+ mux_desc.fwnode = &node->fwnode;
+ mux_desc.drvdata = port_data;
+ snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
+ mux_desc.name = name;
+ mux_desc.set = anx7625_typec_mux_set;
+
+ port_data->typec_mux = typec_mux_register(dev, &mux_desc);
+ if (IS_ERR(port_data->typec_mux)) {
+ ret = PTR_ERR(port_data->typec_mux);
+ dev_err(dev, "Mode switch register for port %d failed: %d", port_num, ret);
+ }
+
+ return ret;
+}
+
+static void anx7625_unregister_typec_switches(struct anx7625_data *ctx)
+{
+ int i;
+
+ for (i = 0; i < ctx->num_typec_switches; i++)
+ typec_mux_unregister(ctx->typec_ports[i].typec_mux);
+}
+
static int anx7625_register_typec_switches(struct device *device, struct anx7625_data *ctx)
{
- struct device_node *of = of_get_child_by_name(device->of_node, "switches");
+ struct device_node *of, *sw;
+ int ret = 0;
+ of = of_get_child_by_name(device->of_node, "switches");
if (!of)
return -ENODEV;
@@ -2592,7 +2644,27 @@ static int anx7625_register_typec_switches(struct device *device, struct anx7625
if (ctx->num_typec_switches <= 0)
return -ENODEV;
- return 0;
+ ctx->typec_ports = devm_kzalloc(device,
+ ctx->num_typec_switches * sizeof(struct anx7625_port_data),
+ GFP_KERNEL);
+ if (!ctx->typec_ports)
+ return -ENOMEM;
+
+ /* Register switches for each connector. */
+ for_each_available_child_of_node(of, sw) {
+ if (!of_property_read_bool(sw, "mode-switch"))
+ continue;
+ ret = anx7625_register_mode_switch(device, sw, ctx);
+ if (ret) {
+ dev_err(device, "Failed to register mode switch: %d\n", ret);
+ break;
+ }
+ }
+
+ if (ret)
+ anx7625_unregister_typec_switches(ctx);
+
+ return ret;
}
static int anx7625_i2c_probe(struct i2c_client *client,
@@ -2701,8 +2773,8 @@ static int anx7625_i2c_probe(struct i2c_client *client,
queue_work(platform->workqueue, &platform->work);
ret = anx7625_register_typec_switches(dev, platform);
- if (ret)
- dev_dbg(dev, "Didn't register Type C switches, err: %d\n", ret);
+ if (ret && ret != -ENODEV)
+ dev_warn(dev, "Didn't register Type C switches, err: %d\n", ret);
platform->bridge.funcs = &anx7625_bridge_funcs;
platform->bridge.of_node = client->dev.of_node;
@@ -2757,6 +2829,8 @@ static int anx7625_i2c_remove(struct i2c_client *client)
drm_bridge_remove(&platform->bridge);
+ anx7625_unregister_typec_switches(platform);
+
if (platform->pdata.intp_irq)
destroy_workqueue(platform->workqueue);
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h
index d5cbca708842..76cfc64f7574 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -443,6 +443,11 @@ struct anx7625_i2c_client {
struct i2c_client *tcpc_client;
};
+struct anx7625_port_data {
+ struct typec_mux_dev *typec_mux;
+ struct anx7625_data *ctx;
+};
+
struct anx7625_data {
struct anx7625_platform_data pdata;
struct platform_device *audio_pdev;
@@ -474,6 +479,7 @@ struct anx7625_data {
struct mipi_dsi_device *dsi;
struct drm_dp_aux aux;
int num_typec_switches;
+ struct anx7625_port_data *typec_ports;
};
#endif /* __ANX7625_H__ */
--
2.36.1.476.g0c4daa206d-goog
On 15/06/2022 10:20, Prashant Malani wrote:
> Analogix 7625 can be used in systems to switch USB Type-C DisplayPort
> alternate mode lane traffic between 2 Type-C ports.
>
> Update the binding to accommodate this usage by introducing a switch
> property.
>
> Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
> Tested-by: Nícolas F. R. A. Prado <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>
> ---
>
> Changes since v3:
> - Fix unevaluatedProperties usage.
> - Add additionalProperties to top level "switches" nodes.
> - Make quotes consistent.
> - Add '^..$' to regex.
> (All suggested by Krzysztof Kozlowski)
Looks good to me.
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
I should add:
Series submission suggestions (of course, open to better suggestions too):
- Patches 1-3 can go through the USB repo.
- Patches 4-7 can:
+ also go through the USB repo after the maintainer acks have been
received <or>
+ go through the DRM repo (after creating a branch from USB repo's
usb-next branch after Patches 1-3 have been applied).
(I"ll add the above to the cover letter if there is a v5).
Thanks!
On Wed, Jun 15, 2022 at 10:21 AM Prashant Malani <[email protected]> wrote:
>
> This series introduces a binding for Type-C data lane switches. These
> control the routing and operating modes of USB Type-C data lanes based
> on the PD messaging from the Type-C port driver regarding connected
> peripherals.
>
> The first patch introduces a change to the Type-C mux class mode-switch
> matching code, while the second adds a config guard to a Type-C header.
> The next couple of patches introduce the new "typec-switch" binding as
> well as one user of it (the ANX7625 drm bridge).
>
> The remaining patches add functionality to the anx7625 driver to
> register the mode-switches, as well as program its crosspoint
> switch depending on which Type-C port has a DisplayPort (DP) peripheral
> connected to it.
>
> v3: https://lore.kernel.org/linux-usb/[email protected]/
>
> Changes since v3:
> - Some more modifications to the anx7625 binding patch (4/7).
> - Picked up 1 more Reviewed-by tag.
>
> Pin-Yen Lin (1):
> drm/bridge: anx7625: Add typec_mux_set callback function
>
> Prashant Malani (6):
> usb: typec: mux: Allow muxes to specify mode-switch
> usb: typec: mux: Add CONFIG guards for functions
> dt-bindings: usb: Add Type-C switch binding
> dt-bindings: drm/bridge: anx7625: Add mode-switch support
> drm/bridge: anx7625: Register number of Type C switches
> drm/bridge: anx7625: Register Type-C mode switches
>
> .../display/bridge/analogix,anx7625.yaml | 64 ++++++++
> .../devicetree/bindings/usb/typec-switch.yaml | 74 +++++++++
> drivers/gpu/drm/bridge/analogix/anx7625.c | 148 ++++++++++++++++++
> drivers/gpu/drm/bridge/analogix/anx7625.h | 20 +++
> drivers/usb/typec/mux.c | 8 +-
> include/linux/usb/typec_mux.h | 44 +++++-
> 6 files changed, 350 insertions(+), 8 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/usb/typec-switch.yaml
>
> --
> 2.36.1.476.g0c4daa206d-goog
>
On Wed, Jun 15, 2022 at 05:20:18PM +0000, Prashant Malani wrote:
> There are some drivers that can use the Type C mux API, but don't have
> to. Introduce CONFIG guards for the mux functions so that drivers can
> include the header file and not run into compilation errors on systems
> which don't have CONFIG_TYPEC enabled. When CONFIG_TYPEC is not enabled,
> the Type C mux functions will be stub versions of the original calls.
>
> Reported-by: kernel test robot <[email protected]>
> Reviewed-by: N?colas F. R. A. Prado <[email protected]>
> Tested-by: N?colas F. R. A. Prado <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>
Reviewed-by: Heikki Krogerus <[email protected]>
> ---
>
> Changes since v3:
> - No changes.
>
> Changes since v2:
> - Fix up return types for some of the stubs. Remove 1 unnecessary stub
> in the else condition.
> - Remove unnecessary IS_MODULE config guard.
> - Added Reviewed-by and Tested-by tags.
>
> Changes since v1:
> - Added static inline to stub functions.
> - Updated function signature of stub functions from "struct typec_mux"
> to "struct typec_mux_dev" in accordance with updates from commit
> 713fd49b430c ("usb: typec: mux: Introduce indirection")
>
> include/linux/usb/typec_mux.h | 44 ++++++++++++++++++++++++++++++-----
> 1 file changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
> index ee57781dcf28..9292f0e07846 100644
> --- a/include/linux/usb/typec_mux.h
> +++ b/include/linux/usb/typec_mux.h
> @@ -58,17 +58,13 @@ struct typec_mux_desc {
> void *drvdata;
> };
>
> +#if IS_ENABLED(CONFIG_TYPEC)
> +
> struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
> const struct typec_altmode_desc *desc);
> void typec_mux_put(struct typec_mux *mux);
> int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state);
>
> -static inline struct typec_mux *
> -typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc)
> -{
> - return fwnode_typec_mux_get(dev_fwnode(dev), desc);
> -}
> -
> struct typec_mux_dev *
> typec_mux_register(struct device *parent, const struct typec_mux_desc *desc);
> void typec_mux_unregister(struct typec_mux_dev *mux);
> @@ -76,4 +72,40 @@ void typec_mux_unregister(struct typec_mux_dev *mux);
> void typec_mux_set_drvdata(struct typec_mux_dev *mux, void *data);
> void *typec_mux_get_drvdata(struct typec_mux_dev *mux);
>
> +#else
> +
> +static inline struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
> + const struct typec_altmode_desc *desc)
> +{
> + return NULL;
> +}
> +
> +static inline void typec_mux_put(struct typec_mux *mux) {}
> +
> +static inline int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
> +{
> + return 0;
> +}
> +
> +static inline struct typec_mux_dev *
> +typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +static inline void typec_mux_unregister(struct typec_mux_dev *mux) {}
> +
> +static inline void typec_mux_set_drvdata(struct typec_mux_dev *mux, void *data) {}
> +static inline void *typec_mux_get_drvdata(struct typec_mux_dev *mux)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +
> +#endif /* CONFIG_TYPEC */
> +
> +static inline struct typec_mux *
> +typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc)
> +{
> + return fwnode_typec_mux_get(dev_fwnode(dev), desc);
> +}
> +
> #endif /* __USB_TYPEC_MUX */
> --
> 2.36.1.476.g0c4daa206d-goog
--
heikki
Quoting Prashant Malani (2022-06-15 10:20:20)
>
> .../display/bridge/analogix,anx7625.yaml | 64 +++++++++++++++++++
> 1 file changed, 64 insertions(+)
Can this file get a link to the product brief[1]? It helps to quickly
find the block diagram.
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index 35a48515836e..bc6f7644db31 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -105,6 +105,34 @@ properties:
> - port@0
> - port@1
>
> + switches:
> + type: object
> + description: Set of switches controlling DisplayPort traffic on
> + outgoing RX/TX lanes to Type C ports.
> + additionalProperties: false
> +
> + properties:
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> + patternProperties:
> + '^switch@[01]$':
> + $ref: /schemas/usb/typec-switch.yaml#
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + maxItems: 1
> +
> + required:
> + - reg
> +
> + required:
> + - switch@0
> +
> required:
> - compatible
> - reg
> @@ -167,5 +195,41 @@ examples:
> };
> };
> };
> + switches {
Is "switches" a bus?
> + #address-cells = <1>;
> + #size-cells = <0>;
> + switch@0 {
> + compatible = "typec-switch";
Is this compatible matched against a driver that's populated on this
"switches" bus?
> + reg = <0>;
> + mode-switch;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + anx_typec0: endpoint {
> + remote-endpoint = <&typec_port0>;
> + };
> + };
> + };
I was expecting to see these simply be more ports in the existing graph
binding of this device, and then have the 'mode-switch' or
'orientation-switch' properties be at the same level as the compatible
string "analogix,anx7625". Here's the reasoning, based on looking at the
product brief and the existing binding/implementation.
Looking at the only existing implementation of this binding upstream in
mt8183-kukui-jacuzzi.dtsi it looks like one of these typec ports is
actually the same physically as the 'anx7625_out' endpoint (reg address
of 1) that is already defined in the binding. It seems that MIPI DSI/DPI
comes in and is output through 2 lanes, SSRX2 and SSTX2 according to the
product brief[1], and that is connected to some eDP panel
("auo,b116xw03"). Presumably that is the same as anx_typec1 in this
patch? I suspect the USB3.1 input is not connected on this board, and
thus the crosspoint switch is never used, nor the SSRX1/SSTX1 pins.
The existing binding defines the MIPI DSI/DPI input as port0 and two of
the four lanes of output that is probably by default connected to the
"DisplayPort Transmitter" as port1 because that's how the crosspoint
switch comes out of reset. That leaves the USB3.1 input possibly needing
a port in the ports binding, and the other two lanes of output needing a
port in the ports binding to describe their connection to the downstream
device. And finally information about if the crosspoint switch needs to
be registered with the typec framework to do typec things, which can be
achieved by the presence of the 'mode-switch' property.
On a board like kukui-jacuzzi these new properties and ports wouldn't be
specified, because what is there is already sufficient. If this chip is
connected to a usb-c-connector then I'd expect to see a connection from
the output ports in the graph binding to the connector node's ports.
There aren't any ports in the usb-c-connector binding though from what I
see.
I believe there's also one more use case here where USB3.1 or MIPI
DSI/DPI is connected on the input side and this device is used to steer
USB3.1 or DP through the crosspoint switch to either of the two output
pairs. This last scenario means that we have to describe both output
pairs, SSRX1/SSTX1 and SSRX2/SSTX2, as different ports in the binding so
they can be connected to different usb-c-connectors if the hardware
engineer wired the output pins that way.
TL;DR: Can we add 'mode-switch' as an optional property and two more
ports at address 2 and 3 for the USB3.1 input and the SSRX1/SSTX1 pair
respectively to the existing graph part of this binding?
> + };
> + switch@1 {
> + compatible = "typec-switch";
> + reg = <1>;
> + mode-switch;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + anx_typec1: endpoint {
> + remote-endpoint = <&typec_port1>;
> + };
> + };
> + };
> + };
> + };
> };
[1] https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf
On Thu, Jun 16, 2022 at 12:42 AM Stephen Boyd <[email protected]> wrote:
>
> Quoting Prashant Malani (2022-06-15 10:20:20)
> >
> > .../display/bridge/analogix,anx7625.yaml | 64 +++++++++++++++++++
> > 1 file changed, 64 insertions(+)
>
> Can this file get a link to the product brief[1]? It helps to quickly
> find the block diagram.
Sure, but I don't really think that should be included in this patch
(or series).
I'd be happy to submit a separate patch once this series is resolved.
>
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > index 35a48515836e..bc6f7644db31 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > @@ -105,6 +105,34 @@ properties:
> > - port@0
> > - port@1
> >
> > + switches:
> > + type: object
> > + description: Set of switches controlling DisplayPort traffic on
> > + outgoing RX/TX lanes to Type C ports.
> > + additionalProperties: false
> > +
> > + properties:
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > + patternProperties:
> > + '^switch@[01]$':
> > + $ref: /schemas/usb/typec-switch.yaml#
> > + unevaluatedProperties: false
> > +
> > + properties:
> > + reg:
> > + maxItems: 1
> > +
> > + required:
> > + - reg
> > +
> > + required:
> > + - switch@0
> > +
> > required:
> > - compatible
> > - reg
> > @@ -167,5 +195,41 @@ examples:
> > };
> > };
> > };
> > + switches {
>
> Is "switches" a bus?
No.
>
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + switch@0 {
> > + compatible = "typec-switch";
>
> Is this compatible matched against a driver that's populated on this
> "switches" bus?
No. Patch 6/7 has the implementation details on how the anx driver
performs the enumeration of switches.
>
> > + reg = <0>;
> > + mode-switch;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + port@0 {
> > + reg = <0>;
> > + anx_typec0: endpoint {
> > + remote-endpoint = <&typec_port0>;
> > + };
> > + };
> > + };
>
> I was expecting to see these simply be more ports in the existing graph
> binding of this device, and then have the 'mode-switch' or
> 'orientation-switch' properties be at the same level as the compatible
> string "analogix,anx7625". Here's the reasoning, based on looking at the
> product brief and the existing binding/implementation.
>
> Looking at the only existing implementation of this binding upstream in
> mt8183-kukui-jacuzzi.dtsi it looks like one of these typec ports is
> actually the same physically as the 'anx7625_out' endpoint (reg address
> of 1) that is already defined in the binding. It seems that MIPI DSI/DPI
> comes in and is output through 2 lanes, SSRX2 and SSTX2 according to the
> product brief[1], and that is connected to some eDP panel
> ("auo,b116xw03"). Presumably that is the same as anx_typec1 in this
> patch? I suspect the USB3.1 input is not connected on this board, and
> thus the crosspoint switch is never used, nor the SSRX1/SSTX1 pins.
>
> The existing binding defines the MIPI DSI/DPI input as port0 and two of
> the four lanes of output that is probably by default connected to the
> "DisplayPort Transmitter" as port1 because that's how the crosspoint
> switch comes out of reset. That leaves the USB3.1 input possibly needing
> a port in the ports binding, and the other two lanes of output needing a
> port in the ports binding to describe their connection to the downstream
> device. And finally information about if the crosspoint switch needs to
> be registered with the typec framework to do typec things, which can be
> achieved by the presence of the 'mode-switch' property.
>
> On a board like kukui-jacuzzi these new properties and ports wouldn't be
> specified, because what is there is already sufficient. If this chip is
> connected to a usb-c-connector then I'd expect to see a connection from
> the output ports in the graph binding to the connector node's ports.
> There aren't any ports in the usb-c-connector binding though from what I
> see.
>
> I believe there's also one more use case here where USB3.1 or MIPI
> DSI/DPI is connected on the input side and this device is used to steer
> USB3.1 or DP through the crosspoint switch to either of the two output
> pairs. This last scenario means that we have to describe both output
> pairs, SSRX1/SSTX1 and SSRX2/SSTX2, as different ports in the binding so
> they can be connected to different usb-c-connectors if the hardware
> engineer wired the output pins that way.
>
> TL;DR: Can we add 'mode-switch' as an optional property and two more
> ports at address 2 and 3 for the USB3.1 input and the SSRX1/SSTX1 pair
> respectively to the existing graph part of this binding?
Sorry, but I got lost midway through the preceding explanation. The binding
can always add additional ports to each "switch" to accomplish the
graph connections
you are alluding to (if the driver needs/uses it, which I don't think
this one does at present).
Adding extra ports to existing ports gets tricky from a mode-switch
enumeration perspective (which
ports should have the modes switches, which shouldn't? Do you follow
the remote end points for each port
and see which one is a Type C connector? What if we add an
intermediate switch device in the future?)
Having a dedicated "switch" binding makes this consistent and easy
(port0 will always have the end-point for the switch).
While there may be more than 1 valid approach here, I believe the
current one is appropriate.
>
> > + };
> > + switch@1 {
> > + compatible = "typec-switch";
> > + reg = <1>;
> > + mode-switch;
> > +
> > + ports {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + port@0 {
> > + reg = <0>;
> > + anx_typec1: endpoint {
> > + remote-endpoint = <&typec_port1>;
> > + };
> > + };
> > + };
> > + };
> > + };
> > };
>
> [1] https://www.analogix.com/en/system/files/AA-002291-PB-6-ANX7625_ProductBrief.pdf
Il 15/06/22 19:20, Prashant Malani ha scritto:
> This series introduces a binding for Type-C data lane switches. These
> control the routing and operating modes of USB Type-C data lanes based
> on the PD messaging from the Type-C port driver regarding connected
> peripherals.
>
> The first patch introduces a change to the Type-C mux class mode-switch
> matching code, while the second adds a config guard to a Type-C header.
> The next couple of patches introduce the new "typec-switch" binding as
> well as one user of it (the ANX7625 drm bridge).
>
> The remaining patches add functionality to the anx7625 driver to
> register the mode-switches, as well as program its crosspoint
> switch depending on which Type-C port has a DisplayPort (DP) peripheral
> connected to it.
>
> v3: https://lore.kernel.org/linux-usb/[email protected]/
>
For the entire series:
Reviewed-by: AngeloGioacchino Del Regno <[email protected]>
Regards,
Angelo
On Thu, Jun 16, 2022 at 01:54:36AM -0700, Prashant Malani wrote:
> On Thu, Jun 16, 2022 at 12:42 AM Stephen Boyd <[email protected]> wrote:
> >
> > Quoting Prashant Malani (2022-06-15 10:20:20)
> > >
> > > .../display/bridge/analogix,anx7625.yaml | 64 +++++++++++++++++++
> > > 1 file changed, 64 insertions(+)
> >
> > Can this file get a link to the product brief[1]? It helps to quickly
> > find the block diagram.
>
> Sure, but I don't really think that should be included in this patch
> (or series).
> I'd be happy to submit a separate patch once this series is resolved.
>
> >
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > index 35a48515836e..bc6f7644db31 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > @@ -105,6 +105,34 @@ properties:
> > > - port@0
> > > - port@1
> > >
> > > + switches:
> > > + type: object
> > > + description: Set of switches controlling DisplayPort traffic on
> > > + outgoing RX/TX lanes to Type C ports.
> > > + additionalProperties: false
> > > +
> > > + properties:
> > > + '#address-cells':
> > > + const: 1
> > > +
> > > + '#size-cells':
> > > + const: 0
> > > +
> > > + patternProperties:
> > > + '^switch@[01]$':
> > > + $ref: /schemas/usb/typec-switch.yaml#
> > > + unevaluatedProperties: false
> > > +
> > > + properties:
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + required:
> > > + - reg
> > > +
> > > + required:
> > > + - switch@0
> > > +
> > > required:
> > > - compatible
> > > - reg
> > > @@ -167,5 +195,41 @@ examples:
> > > };
> > > };
> > > };
> > > + switches {
> >
> > Is "switches" a bus?
>
> No.
>
> >
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + switch@0 {
> > > + compatible = "typec-switch";
> >
> > Is this compatible matched against a driver that's populated on this
> > "switches" bus?
>
> No. Patch 6/7 has the implementation details on how the anx driver
> performs the enumeration of switches.
>
> >
> > > + reg = <0>;
> > > + mode-switch;
> > > +
> > > + ports {
> > > + #address-cells = <1>;
> > > + #size-cells = <0>;
> > > + port@0 {
> > > + reg = <0>;
> > > + anx_typec0: endpoint {
> > > + remote-endpoint = <&typec_port0>;
> > > + };
> > > + };
> > > + };
> >
> > I was expecting to see these simply be more ports in the existing graph
> > binding of this device, and then have the 'mode-switch' or
> > 'orientation-switch' properties be at the same level as the compatible
> > string "analogix,anx7625". Here's the reasoning, based on looking at the
> > product brief and the existing binding/implementation.
> >
> > Looking at the only existing implementation of this binding upstream in
> > mt8183-kukui-jacuzzi.dtsi it looks like one of these typec ports is
> > actually the same physically as the 'anx7625_out' endpoint (reg address
> > of 1) that is already defined in the binding. It seems that MIPI DSI/DPI
> > comes in and is output through 2 lanes, SSRX2 and SSTX2 according to the
> > product brief[1], and that is connected to some eDP panel
> > ("auo,b116xw03"). Presumably that is the same as anx_typec1 in this
> > patch? I suspect the USB3.1 input is not connected on this board, and
> > thus the crosspoint switch is never used, nor the SSRX1/SSTX1 pins.
> >
> > The existing binding defines the MIPI DSI/DPI input as port0 and two of
> > the four lanes of output that is probably by default connected to the
> > "DisplayPort Transmitter" as port1 because that's how the crosspoint
> > switch comes out of reset. That leaves the USB3.1 input possibly needing
> > a port in the ports binding, and the other two lanes of output needing a
> > port in the ports binding to describe their connection to the downstream
> > device. And finally information about if the crosspoint switch needs to
> > be registered with the typec framework to do typec things, which can be
> > achieved by the presence of the 'mode-switch' property.
> >
> > On a board like kukui-jacuzzi these new properties and ports wouldn't be
> > specified, because what is there is already sufficient. If this chip is
> > connected to a usb-c-connector then I'd expect to see a connection from
> > the output ports in the graph binding to the connector node's ports.
> > There aren't any ports in the usb-c-connector binding though from what I
> > see.
> >
> > I believe there's also one more use case here where USB3.1 or MIPI
> > DSI/DPI is connected on the input side and this device is used to steer
> > USB3.1 or DP through the crosspoint switch to either of the two output
> > pairs. This last scenario means that we have to describe both output
> > pairs, SSRX1/SSTX1 and SSRX2/SSTX2, as different ports in the binding so
> > they can be connected to different usb-c-connectors if the hardware
> > engineer wired the output pins that way.
> >
> > TL;DR: Can we add 'mode-switch' as an optional property and two more
> > ports at address 2 and 3 for the USB3.1 input and the SSRX1/SSTX1 pair
> > respectively to the existing graph part of this binding?
>
> Sorry, but I got lost midway through the preceding explanation.
Made sense to me.
> The binding
> can always add additional ports to each "switch" to accomplish the
> graph connections
> you are alluding to (if the driver needs/uses it, which I don't think
> this one does at present).
Why is the switch special? If I just look at this from a block diagram
perspective, I just see a list of interfaces that need to be described
in the graph.
> Adding extra ports to existing ports gets tricky from a mode-switch
> enumeration perspective (which
> ports should have the modes switches, which shouldn't? Do you follow
> the remote end points for each port
> and see which one is a Type C connector?
The driver knows which port is which because the binding has to define
it. So you have to check 2 of them (SSRX1/SSTX1 and SSRX2/SSTX2) to find
usb C connectors.
> What if we add an
> intermediate switch device in the future?)
> Having a dedicated "switch" binding makes this consistent and easy
> (port0 will always have the end-point for the switch).
>
> While there may be more than 1 valid approach here, I believe the
> current one is appropriate.
To put it simply, if you want to define a generic binding, I want to see
at least 2 users of it. What I really want to see is someone looking at
all the Type-C related bindings and h/w possibilities, not just 1
problem or their own h/w. IOW, a Type-C binding czar.
Rob
On Thu, Jun 16, 2022 at 12:34 PM Rob Herring <[email protected]> wrote:
>
> On Thu, Jun 16, 2022 at 01:54:36AM -0700, Prashant Malani wrote:
> > On Thu, Jun 16, 2022 at 12:42 AM Stephen Boyd <[email protected]> wrote:
> > >
> > > Quoting Prashant Malani (2022-06-15 10:20:20)
> > > >
> > > > .../display/bridge/analogix,anx7625.yaml | 64 +++++++++++++++++++
> > > > 1 file changed, 64 insertions(+)
> > >
> > > Can this file get a link to the product brief[1]? It helps to quickly
> > > find the block diagram.
> >
> > Sure, but I don't really think that should be included in this patch
> > (or series).
> > I'd be happy to submit a separate patch once this series is resolved.
> >
> > >
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > index 35a48515836e..bc6f7644db31 100644
> > > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > @@ -105,6 +105,34 @@ properties:
> > > > - port@0
> > > > - port@1
> > > >
> > > > + switches:
> > > > + type: object
> > > > + description: Set of switches controlling DisplayPort traffic on
> > > > + outgoing RX/TX lanes to Type C ports.
> > > > + additionalProperties: false
> > > > +
> > > > + properties:
> > > > + '#address-cells':
> > > > + const: 1
> > > > +
> > > > + '#size-cells':
> > > > + const: 0
> > > > +
> > > > + patternProperties:
> > > > + '^switch@[01]$':
> > > > + $ref: /schemas/usb/typec-switch.yaml#
> > > > + unevaluatedProperties: false
> > > > +
> > > > + properties:
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + required:
> > > > + - reg
> > > > +
> > > > + required:
> > > > + - switch@0
> > > > +
> > > > required:
> > > > - compatible
> > > > - reg
> > > > @@ -167,5 +195,41 @@ examples:
> > > > };
> > > > };
> > > > };
> > > > + switches {
> > >
> > > Is "switches" a bus?
> >
> > No.
> >
> > >
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + switch@0 {
> > > > + compatible = "typec-switch";
> > >
> > > Is this compatible matched against a driver that's populated on this
> > > "switches" bus?
> >
> > No. Patch 6/7 has the implementation details on how the anx driver
> > performs the enumeration of switches.
> >
> > >
> > > > + reg = <0>;
> > > > + mode-switch;
> > > > +
> > > > + ports {
> > > > + #address-cells = <1>;
> > > > + #size-cells = <0>;
> > > > + port@0 {
> > > > + reg = <0>;
> > > > + anx_typec0: endpoint {
> > > > + remote-endpoint = <&typec_port0>;
> > > > + };
> > > > + };
> > > > + };
> > >
> > > I was expecting to see these simply be more ports in the existing graph
> > > binding of this device, and then have the 'mode-switch' or
> > > 'orientation-switch' properties be at the same level as the compatible
> > > string "analogix,anx7625". Here's the reasoning, based on looking at the
> > > product brief and the existing binding/implementation.
> > >
> > > Looking at the only existing implementation of this binding upstream in
> > > mt8183-kukui-jacuzzi.dtsi it looks like one of these typec ports is
> > > actually the same physically as the 'anx7625_out' endpoint (reg address
> > > of 1) that is already defined in the binding. It seems that MIPI DSI/DPI
> > > comes in and is output through 2 lanes, SSRX2 and SSTX2 according to the
> > > product brief[1], and that is connected to some eDP panel
> > > ("auo,b116xw03"). Presumably that is the same as anx_typec1 in this
> > > patch? I suspect the USB3.1 input is not connected on this board, and
> > > thus the crosspoint switch is never used, nor the SSRX1/SSTX1 pins.
> > >
> > > The existing binding defines the MIPI DSI/DPI input as port0 and two of
> > > the four lanes of output that is probably by default connected to the
> > > "DisplayPort Transmitter" as port1 because that's how the crosspoint
> > > switch comes out of reset. That leaves the USB3.1 input possibly needing
> > > a port in the ports binding, and the other two lanes of output needing a
> > > port in the ports binding to describe their connection to the downstream
> > > device. And finally information about if the crosspoint switch needs to
> > > be registered with the typec framework to do typec things, which can be
> > > achieved by the presence of the 'mode-switch' property.
> > >
> > > On a board like kukui-jacuzzi these new properties and ports wouldn't be
> > > specified, because what is there is already sufficient. If this chip is
> > > connected to a usb-c-connector then I'd expect to see a connection from
> > > the output ports in the graph binding to the connector node's ports.
> > > There aren't any ports in the usb-c-connector binding though from what I
> > > see.
> > >
> > > I believe there's also one more use case here where USB3.1 or MIPI
> > > DSI/DPI is connected on the input side and this device is used to steer
> > > USB3.1 or DP through the crosspoint switch to either of the two output
> > > pairs. This last scenario means that we have to describe both output
> > > pairs, SSRX1/SSTX1 and SSRX2/SSTX2, as different ports in the binding so
> > > they can be connected to different usb-c-connectors if the hardware
> > > engineer wired the output pins that way.
> > >
> > > TL;DR: Can we add 'mode-switch' as an optional property and two more
> > > ports at address 2 and 3 for the USB3.1 input and the SSRX1/SSTX1 pair
> > > respectively to the existing graph part of this binding?
> >
> > Sorry, but I got lost midway through the preceding explanation.
>
> Made sense to me.
>
> > The binding
> > can always add additional ports to each "switch" to accomplish the
> > graph connections
> > you are alluding to (if the driver needs/uses it, which I don't think
> > this one does at present).
>
> Why is the switch special? If I just look at this from a block diagram
> perspective, I just see a list of interfaces that need to be described
> in the graph.
Because it is specific to Type-C connectors. The anx7625.h does
contain a cross-point
switch which controls data lines coming from 1 (or more) Type-C
connectors, so it seems reasonable
to have a dedicated binding for such types of hardware sub-components,
which helps define the graph connections
in a more uniform manner. That's not to say:
- this can only be used by this hardware. The typec-switch binding is
generic enough to accommodate other hardware.
- there is only 1 way to do this. The interfaces could be described
using existing port OF graph bindings, but I don't
see that as reason enough to not include a dedicated switch binding if
it makes the overall binding more logically organized (IMO) and
makes driver registration code mode clean.
>
> > Adding extra ports to existing ports gets tricky from a mode-switch
> > enumeration perspective (which
> > ports should have the modes switches, which shouldn't? Do you follow
> > the remote end points for each port
> > and see which one is a Type C connector?
>
> The driver knows which port is which because the binding has to define
> it. So you have to check 2 of them (SSRX1/SSTX1 and SSRX2/SSTX2) to find
> usb C connectors.
Right, but with the switch binding you no longer need to check. If
there is a typec-switch, you know
it is coming from a Type-C connector, so you can just register the
switches with the Type-C framework.
>
> > What if we add an
> > intermediate switch device in the future?)
> > Having a dedicated "switch" binding makes this consistent and easy
> > (port0 will always have the end-point for the switch).
> >
> > While there may be more than 1 valid approach here, I believe the
> > current one is appropriate.
>
> To put it simply, if you want to define a generic binding, I want to see
> at least 2 users of it. What I really want to see is someone looking at
> all the Type-C related bindings and h/w possibilities, not just 1
> problem or their own h/w. IOW, a Type-C binding czar.
As I mentioned above, the typec-switch binding is generic enough to allow usage
by other hardware. I can think of at least 1 example which could
utilize this switch-binding [1], but I'd defer to the maintainer
of that binding to adopt the changes or not.
[1] https://elixir.bootlin.com/linux/v5.19-rc2/source/Documentation/devicetree/bindings/usb/fcs,fsa4480.yaml
Thanks,
>
> Rob
On Wed, Jun 15, 2022 at 11:13:33AM -0700, Prashant Malani wrote:
> I should add:
>
> Series submission suggestions (of course, open to better suggestions too):
> - Patches 1-3 can go through the USB repo.
I will take patches 1 and 2 now.
seems the others need reworks or acks from the DT people.
thanks,
greg k-h
On Thu, Jun 16, 2022 at 12:57 PM Prashant Malani <[email protected]> wrote:
>
> On Thu, Jun 16, 2022 at 12:34 PM Rob Herring <[email protected]> wrote:
> >
> > On Thu, Jun 16, 2022 at 01:54:36AM -0700, Prashant Malani wrote:
> > > On Thu, Jun 16, 2022 at 12:42 AM Stephen Boyd <[email protected]> wrote:
> > > >
> > > > Quoting Prashant Malani (2022-06-15 10:20:20)
> > > > >
> > > > > .../display/bridge/analogix,anx7625.yaml | 64 +++++++++++++++++++
> > > > > 1 file changed, 64 insertions(+)
> > > >
> > > > Can this file get a link to the product brief[1]? It helps to quickly
> > > > find the block diagram.
> > >
> > > Sure, but I don't really think that should be included in this patch
> > > (or series).
> > > I'd be happy to submit a separate patch once this series is resolved.
> > >
> > > >
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > > index 35a48515836e..bc6f7644db31 100644
> > > > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > > > @@ -105,6 +105,34 @@ properties:
> > > > > - port@0
> > > > > - port@1
> > > > >
> > > > > + switches:
> > > > > + type: object
> > > > > + description: Set of switches controlling DisplayPort traffic on
> > > > > + outgoing RX/TX lanes to Type C ports.
> > > > > + additionalProperties: false
> > > > > +
> > > > > + properties:
> > > > > + '#address-cells':
> > > > > + const: 1
> > > > > +
> > > > > + '#size-cells':
> > > > > + const: 0
> > > > > +
> > > > > + patternProperties:
> > > > > + '^switch@[01]$':
> > > > > + $ref: /schemas/usb/typec-switch.yaml#
> > > > > + unevaluatedProperties: false
> > > > > +
> > > > > + properties:
> > > > > + reg:
> > > > > + maxItems: 1
> > > > > +
> > > > > + required:
> > > > > + - reg
> > > > > +
> > > > > + required:
> > > > > + - switch@0
> > > > > +
> > > > > required:
> > > > > - compatible
> > > > > - reg
> > > > > @@ -167,5 +195,41 @@ examples:
> > > > > };
> > > > > };
> > > > > };
> > > > > + switches {
> > > >
> > > > Is "switches" a bus?
> > >
> > > No.
> > >
> > > >
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + switch@0 {
> > > > > + compatible = "typec-switch";
> > > >
> > > > Is this compatible matched against a driver that's populated on this
> > > > "switches" bus?
> > >
> > > No. Patch 6/7 has the implementation details on how the anx driver
> > > performs the enumeration of switches.
> > >
> > > >
> > > > > + reg = <0>;
> > > > > + mode-switch;
> > > > > +
> > > > > + ports {
> > > > > + #address-cells = <1>;
> > > > > + #size-cells = <0>;
> > > > > + port@0 {
> > > > > + reg = <0>;
> > > > > + anx_typec0: endpoint {
> > > > > + remote-endpoint = <&typec_port0>;
> > > > > + };
> > > > > + };
> > > > > + };
> > > >
> > > > I was expecting to see these simply be more ports in the existing graph
> > > > binding of this device, and then have the 'mode-switch' or
> > > > 'orientation-switch' properties be at the same level as the compatible
> > > > string "analogix,anx7625". Here's the reasoning, based on looking at the
> > > > product brief and the existing binding/implementation.
> > > >
> > > > Looking at the only existing implementation of this binding upstream in
> > > > mt8183-kukui-jacuzzi.dtsi it looks like one of these typec ports is
> > > > actually the same physically as the 'anx7625_out' endpoint (reg address
> > > > of 1) that is already defined in the binding. It seems that MIPI DSI/DPI
> > > > comes in and is output through 2 lanes, SSRX2 and SSTX2 according to the
> > > > product brief[1], and that is connected to some eDP panel
> > > > ("auo,b116xw03"). Presumably that is the same as anx_typec1 in this
> > > > patch? I suspect the USB3.1 input is not connected on this board, and
> > > > thus the crosspoint switch is never used, nor the SSRX1/SSTX1 pins.
> > > >
> > > > The existing binding defines the MIPI DSI/DPI input as port0 and two of
> > > > the four lanes of output that is probably by default connected to the
> > > > "DisplayPort Transmitter" as port1 because that's how the crosspoint
> > > > switch comes out of reset. That leaves the USB3.1 input possibly needing
> > > > a port in the ports binding, and the other two lanes of output needing a
> > > > port in the ports binding to describe their connection to the downstream
> > > > device. And finally information about if the crosspoint switch needs to
> > > > be registered with the typec framework to do typec things, which can be
> > > > achieved by the presence of the 'mode-switch' property.
> > > >
> > > > On a board like kukui-jacuzzi these new properties and ports wouldn't be
> > > > specified, because what is there is already sufficient. If this chip is
> > > > connected to a usb-c-connector then I'd expect to see a connection from
> > > > the output ports in the graph binding to the connector node's ports.
> > > > There aren't any ports in the usb-c-connector binding though from what I
> > > > see.
> > > >
> > > > I believe there's also one more use case here where USB3.1 or MIPI
> > > > DSI/DPI is connected on the input side and this device is used to steer
> > > > USB3.1 or DP through the crosspoint switch to either of the two output
> > > > pairs. This last scenario means that we have to describe both output
> > > > pairs, SSRX1/SSTX1 and SSRX2/SSTX2, as different ports in the binding so
> > > > they can be connected to different usb-c-connectors if the hardware
> > > > engineer wired the output pins that way.
> > > >
> > > > TL;DR: Can we add 'mode-switch' as an optional property and two more
> > > > ports at address 2 and 3 for the USB3.1 input and the SSRX1/SSTX1 pair
> > > > respectively to the existing graph part of this binding?
> > >
> > > Sorry, but I got lost midway through the preceding explanation.
> >
> > Made sense to me.
> >
> > > The binding
> > > can always add additional ports to each "switch" to accomplish the
> > > graph connections
> > > you are alluding to (if the driver needs/uses it, which I don't think
> > > this one does at present).
> >
> > Why is the switch special? If I just look at this from a block diagram
> > perspective, I just see a list of interfaces that need to be described
> > in the graph.
>
> Because it is specific to Type-C connectors. The anx7625.h does
> contain a cross-point
> switch which controls data lines coming from 1 (or more) Type-C
> connectors, so it seems reasonable
> to have a dedicated binding for such types of hardware sub-components,
> which helps define the graph connections
> in a more uniform manner. That's not to say:
> - this can only be used by this hardware. The typec-switch binding is
> generic enough to accommodate other hardware.
> - there is only 1 way to do this. The interfaces could be described
> using existing port OF graph bindings, but I don't
> see that as reason enough to not include a dedicated switch binding if
> it makes the overall binding more logically organized (IMO) and
> makes driver registration code mode clean.
>
> >
> > > Adding extra ports to existing ports gets tricky from a mode-switch
> > > enumeration perspective (which
> > > ports should have the modes switches, which shouldn't? Do you follow
> > > the remote end points for each port
> > > and see which one is a Type C connector?
> >
> > The driver knows which port is which because the binding has to define
> > it. So you have to check 2 of them (SSRX1/SSTX1 and SSRX2/SSTX2) to find
> > usb C connectors.
>
> Right, but with the switch binding you no longer need to check. If
> there is a typec-switch, you know
> it is coming from a Type-C connector, so you can just register the
> switches with the Type-C framework.
>
> >
> > > What if we add an
> > > intermediate switch device in the future?)
> > > Having a dedicated "switch" binding makes this consistent and easy
> > > (port0 will always have the end-point for the switch).
> > >
> > > While there may be more than 1 valid approach here, I believe the
> > > current one is appropriate.
> >
> > To put it simply, if you want to define a generic binding, I want to see
> > at least 2 users of it.
Pin-Yen and I will work on adding another user for the binding to v5 of
this patch series.
Best regards,
- Prashant
On 15/06/2022 19:20, Prashant Malani wrote:
> Introduce a binding which represents a component that can control the
> routing of USB Type-C data lines as well as address data line
> orientation (based on CC lines' orientation).
>
> Reviewed-by: Nícolas F. R. A. Prado <[email protected]>
> Tested-by: Nícolas F. R. A. Prado <[email protected]>
> Signed-off-by: Prashant Malani <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On Wed, Jun 22, 2022 at 04:53:40PM +0200, Krzysztof Kozlowski wrote:
> On 21/06/2022 15:17, Greg Kroah-Hartman wrote:
> > On Wed, Jun 15, 2022 at 11:13:33AM -0700, Prashant Malani wrote:
> >> I should add:
> >>
> >> Series submission suggestions (of course, open to better suggestions too):
> >> - Patches 1-3 can go through the USB repo.
> >
> > I will take patches 1 and 2 now.
> >
> > seems the others need reworks or acks from the DT people.
>
> I just gave for patch 3 and before for 4, so you can grab these as well.
They are gone from my queue, a resend with that ack would be good so
that I can pick it up easier.
thanks,
gre gk-h
On 21/06/2022 15:17, Greg Kroah-Hartman wrote:
> On Wed, Jun 15, 2022 at 11:13:33AM -0700, Prashant Malani wrote:
>> I should add:
>>
>> Series submission suggestions (of course, open to better suggestions too):
>> - Patches 1-3 can go through the USB repo.
>
> I will take patches 1 and 2 now.
>
> seems the others need reworks or acks from the DT people.
I just gave for patch 3 and before for 4, so you can grab these as well.
Thanks!
Best regards,
Krzysztof
On Wed, Jun 22, 2022 at 8:11 AM Greg Kroah-Hartman
<[email protected]> wrote:
>
> On Wed, Jun 22, 2022 at 04:53:40PM +0200, Krzysztof Kozlowski wrote:
> > On 21/06/2022 15:17, Greg Kroah-Hartman wrote:
> > > On Wed, Jun 15, 2022 at 11:13:33AM -0700, Prashant Malani wrote:
> > >> I should add:
> > >>
> > >> Series submission suggestions (of course, open to better suggestions too):
> > >> - Patches 1-3 can go through the USB repo.
> > >
> > > I will take patches 1 and 2 now.
> > >
> > > seems the others need reworks or acks from the DT people.
> >
> > I just gave for patch 3 and before for 4, so you can grab these as well.
>
> They are gone from my queue, a resend with that ack would be good so
> that I can pick it up easier.
Thanks Greg. I've sent out v5 [1] which has the Reviewed-by tags from
Krzysztof.
[1] https://lore.kernel.org/linux-usb/[email protected]/
Best regards,
-Prashant