2022-06-08 03:56:16

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 0/7] usb: typec: Introduce typec-switch binding

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.

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 | 56 +++++++
.../devicetree/bindings/usb/typec-switch.yaml | 76 +++++++++
drivers/gpu/drm/bridge/analogix/anx7625.c | 151 ++++++++++++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 20 +++
drivers/usb/typec/mux.c | 8 +-
include/linux/usb/typec_mux.h | 38 +++++
6 files changed, 347 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/usb/typec-switch.yaml

--
2.36.1.255.ge46751e96f-goog


2022-06-08 04:10:15

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 3/7] dt-bindings: usb: Add Type-C switch binding

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).

Signed-off-by: Prashant Malani <[email protected]>
---
.../devicetree/bindings/usb/typec-switch.yaml | 76 +++++++++++++++++++
1 file changed, 76 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..60a600a63fef
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/typec-switch.yaml
@@ -0,0 +1,76 @@
+# 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:
+ items:
+ - enum:
+ - 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:
+ - |
+ anx7625 {
+ typecswitch {
+ 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.255.ge46751e96f-goog

2022-06-08 04:20:30

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 1/7] usb: typec: mux: Allow muxes to specify mode-switch

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]>
---
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.255.ge46751e96f-goog

2022-06-08 05:20:24

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 2/7] usb: typec: mux: Add CONFIG guards for functions

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.

Signed-off-by: Prashant Malani <[email protected]>
---
include/linux/usb/typec_mux.h | 38 +++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)

diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
index ee57781dcf28..758d34ced1f8 100644
--- a/include/linux/usb/typec_mux.h
+++ b/include/linux/usb/typec_mux.h
@@ -58,6 +58,8 @@ struct typec_mux_desc {
void *drvdata;
};

+#if IS_ENABLED(CONFIG_TYPEC) || IS_MODULE(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);
@@ -76,4 +78,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
+
+struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
+ const struct typec_altmode_desc *desc)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
+void typec_mux_put(struct typec_mux *mux) {}
+
+int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
+{
+ return -EOPNOTSUPP;
+}
+
+static inline struct typec_mux *
+typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
+struct typec_mux *
+typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+void typec_mux_unregister(struct typec_mux *mux) {}
+
+void typec_mux_set_drvdata(struct typec_mux *mux, void *data) {}
+void *typec_mux_get_drvdata(struct typec_mux *mux)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
+#endif /* CONFIG_TYPEC */
+
#endif /* __USB_TYPEC_MUX */
--
2.36.1.255.ge46751e96f-goog

2022-06-08 05:44:37

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 6/7] drm/bridge: anx7625: Register Type-C mode switches

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.

Signed-off-by: Prashant Malani <[email protected]>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 73 +++++++++++++++++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 6 ++
2 files changed, 79 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 07ed44c6b839..d41a21103bd3 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,9 +2582,59 @@ 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 = NULL;
+ struct device_node *sw;
int ret = 0;

of = of_get_child_by_name(device->of_node, "switches");
@@ -2594,6 +2645,26 @@ static int anx7625_register_typec_switches(struct device *device, struct anx7625
if (ctx->num_typec_switches <= 0)
return -ENODEV;

+ 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;
}

@@ -2759,6 +2830,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.255.ge46751e96f-goog

2022-06-08 05:52:57

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 5/7] drm/bridge: anx7625: Register number of Type C switches

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.

Signed-off-by: Prashant Malani <[email protected]>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 20 ++++++++++++++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 1 +
2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index 53a5da6c49dd..07ed44c6b839 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2581,6 +2581,22 @@ 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 = NULL;
+ int ret = 0;
+
+ 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 ret;
+}
+
static int anx7625_i2c_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -2686,6 +2702,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_info(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.255.ge46751e96f-goog

2022-06-08 06:00:16

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 7/7] drm/bridge: anx7625: Add typec_mux_set callback function

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.

Signed-off-by: Pin-Yen Lin <[email protected]>
Signed-off-by: Prashant Malani <[email protected]>
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 58 +++++++++++++++++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 13 +++++
2 files changed, 71 insertions(+)

diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index d41a21103bd3..2c308d12fab2 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,66 @@ 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 old_dp_connected = (ctx->typec_ports[0].dp_connected ||
+ ctx->typec_ports[1].dp_connected);
+ bool new_dp_connected;
+
+ if (ctx->num_typec_switches == 1)
+ return 0;
+
+ 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.255.ge46751e96f-goog

2022-06-08 06:09:56

by Prashant Malani

[permalink] [raw]
Subject: [PATCH 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support

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.

Signed-off-by: Prashant Malani <[email protected]>
---
.../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++
1 file changed, 56 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
index 35a48515836e..7e1f655ddfcc 100644
--- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
+++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
@@ -105,6 +105,26 @@ properties:
- port@0
- port@1

+ switches:
+ type: object
+ description: Set of switches controlling DisplayPort traffic on
+ outgoing RX/TX lanes to Type C ports.
+
+ properties:
+ switch:
+ $ref: /schemas/usb/typec-switch.yaml#
+ maxItems: 2
+
+ properties:
+ reg:
+ maxItems: 1
+
+ required:
+ - reg
+
+ required:
+ - switch@0
+
required:
- compatible
- reg
@@ -167,5 +187,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.255.ge46751e96f-goog

2022-06-08 10:22:31

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/7] dt-bindings: usb: Add Type-C switch binding

On 07/06/2022 21:00, 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).
>
> Signed-off-by: Prashant Malani <[email protected]>
> ---
> .../devicetree/bindings/usb/typec-switch.yaml | 76 +++++++++++++++++++
> 1 file changed, 76 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..60a600a63fef
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/typec-switch.yaml
> @@ -0,0 +1,76 @@
> +# 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:
> + items:

Single item, so no need for items.

> + - enum:
> + - 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

Why true? I see usb-connector has it from commit 6a0e321ea735
("dt-bindings: Explicitly allow additional properties in common schemas")
but that looks also weird - this is not a common schema, but a
complete, generic one.

> +
> +examples:
> + - |
> + anx7625 {

Generic node name.

> + typecswitch {

I guess here as well, so "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>;
> + };
> + };
> + };
> + };
> + };


Best regards,
Krzysztof

2022-06-08 10:23:09

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/7] dt-bindings: usb: Add Type-C switch binding

On 08/06/2022 11:18, Krzysztof Kozlowski wrote:
>> +anyOf:
>> + - required:
>> + - mode-switch
>> + - required:
>> + - orientation-switch
>> +
>> +additionalProperties: true
>
> Why true? I see usb-connector has it from commit 6a0e321ea735
> ("dt-bindings: Explicitly allow additional properties in common schemas")
> but that looks also weird - this is not a common schema, but a
> complete, generic one.
>

Ah, I see now, the parent device uses the additional properties. It's
ok, skip the comment.


Best regards,
Krzysztof

2022-06-08 10:26:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support

On 07/06/2022 21:00, 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.
>
> Signed-off-by: Prashant Malani <[email protected]>
> ---
> .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> index 35a48515836e..7e1f655ddfcc 100644
> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> @@ -105,6 +105,26 @@ properties:
> - port@0
> - port@1
>
> + switches:
> + type: object
> + description: Set of switches controlling DisplayPort traffic on
> + outgoing RX/TX lanes to Type C ports.
> +
> + properties:
> + switch:

You allow only one switch with such schema, so no need for "switches"...

> + $ref: /schemas/usb/typec-switch.yaml#
> + maxItems: 2

Are you sure this works? what are you limiting here with maxItems? I
think you wanted patternProperties...

> +
> + properties:
> + reg:
> + maxItems: 1
> +
> + required:
> + - reg
> +
> + required:
> + - switch@0

This does not match the property.

You also need unevaluatedProperties:false


> +
> required:
> - compatible
> - reg
> @@ -167,5 +187,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>;

Messed up indentation. Your previous patch should also switch to 4-space
as recommended by schema coding style.

> + };
> + };
> + };
> + };
> + 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>;

Ditto.

> + };
> + };
> + };
> + };
> + };
> };
> };


Best regards,
Krzysztof

2022-06-08 17:26:04

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH 3/7] dt-bindings: usb: Add Type-C switch binding

Hi Krzysztof,

On Jun 08 11:18, Krzysztof Kozlowski wrote:
> On 07/06/2022 21:00, 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).
> >
> > Signed-off-by: Prashant Malani <[email protected]>
> > ---
> > .../devicetree/bindings/usb/typec-switch.yaml | 76 +++++++++++++++++++
> > 1 file changed, 76 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..60a600a63fef
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/typec-switch.yaml
> > @@ -0,0 +1,76 @@
> > +# 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:
> > + items:
>
> Single item, so no need for items.

I tried this initially, but dt_binding_check throws the following
error:
$ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/usb/typec-switch.yaml

LINT Documentation/devicetree/bindings
CHKDT Documentation/devicetree/bindings/processed-schema.json
/foo/linux/Documentation/devicetree/bindings/usb/typec-switch.yaml: properties:compatible: [{'enum': ['typec-switch']}] is not of type 'object', 'boolean'
from schema $id: http://json-schema.org/draft-07/schema#
SCHEMA Documentation/devicetree/bindings/processed-schema.json
/foo/linux/Documentation/devicetree/bindings/usb/typec-switch.yaml: ignoring, error in schema: properties: compatible

>
> > + - enum:
> > + - 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
>
> Why true? I see usb-connector has it from commit 6a0e321ea735
> ("dt-bindings: Explicitly allow additional properties in common schemas")
> but that looks also weird - this is not a common schema, but a
> complete, generic one.
>
> > +
> > +examples:
> > + - |
> > + anx7625 {
>
> Generic node name.

Will fix in v2.
>
> > + typecswitch {
>
> I guess here as well, so "usb-switch"?

Same here, will fix with v2, along with 4-space indentation.

>
> > + 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>;
> > + };
> > + };
> > + };
> > + };
> > + };
>
>
> Best regards,
> Krzysztof

2022-06-08 18:44:18

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support

Hi Krzysztof,

Thank you for looking at the patch.

On Jun 08 11:24, Krzysztof Kozlowski wrote:
> On 07/06/2022 21:00, 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.
> >
> > Signed-off-by: Prashant Malani <[email protected]>
> > ---
> > .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++
> > 1 file changed, 56 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > index 35a48515836e..7e1f655ddfcc 100644
> > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > @@ -105,6 +105,26 @@ properties:
> > - port@0
> > - port@1
> >
> > + switches:
> > + type: object
> > + description: Set of switches controlling DisplayPort traffic on
> > + outgoing RX/TX lanes to Type C ports.
> > +
> > + properties:
> > + switch:
>
> You allow only one switch with such schema, so no need for "switches"...

See below comment (summary: we'd like to allow 1 or 2 switches).
>
> > + $ref: /schemas/usb/typec-switch.yaml#
> > + maxItems: 2
>
> Are you sure this works? what are you limiting here with maxItems? I
> think you wanted patternProperties...

Yeah, I might not have used the DT syntax correctly here.
What I'm aiming for is:
"switches" should can contain 1 or 2 "switch" nodes.
2 is the maximum (limitation of the hardware).

>
> > +
> > + properties:
> > + reg:
> > + maxItems: 1
> > +
> > + required:
> > + - reg
> > +
> > + required:
> > + - switch@0
>
> This does not match the property.
>
> You also need unevaluatedProperties:false

Ack, will update this in the next version.

>
>
> > +
> > required:
> > - compatible
> > - reg
> > @@ -167,5 +187,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>;
>
> Messed up indentation. Your previous patch should also switch to 4-space
> as recommended by schema coding style.

Sorry about that, will fix up the indentation in the next version.

>
> > + };
> > + };
> > + };
> > + };
> > + 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>;
>
> Ditto.
>
> > + };
> > + };
> > + };
> > + };
> > + };
> > };
> > };
>
>
> Best regards,
> Krzysztof

2022-06-08 18:44:36

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 6/7] drm/bridge: anx7625: Register Type-C mode switches

Hi Prashant,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm/drm-next]
[also build test WARNING on usb/usb-testing v5.19-rc1 next-20220608]
[cannot apply to balbi-usb/testing/next peter-chen-usb/for-usb-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Prashant-Malani/usb-typec-Introduce-typec-switch-binding/20220608-042545
base: git://anongit.freedesktop.org/drm/drm drm-next
config: i386-randconfig-a006 (https://download.01.org/0day-ci/archive/20220609/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project b92436efcb7813fc481b30f2593a4907568d917a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2ac4609c73d7bb4d1a585dae84559967ced3bad6
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Prashant-Malani/usb-typec-Introduce-typec-switch-binding/20220608-042545
git checkout 2ac4609c73d7bb4d1a585dae84559967ced3bad6
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/bridge/analogix/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

In file included from drivers/gpu/drm/bridge/analogix/anx7625.c:18:
>> include/linux/usb/typec_mux.h:83:19: warning: no previous prototype for function 'fwnode_typec_mux_get' [-Wmissing-prototypes]
struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
^
include/linux/usb/typec_mux.h:83:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
^
static
>> include/linux/usb/typec_mux.h:89:6: warning: no previous prototype for function 'typec_mux_put' [-Wmissing-prototypes]
void typec_mux_put(struct typec_mux *mux) {}
^
include/linux/usb/typec_mux.h:89:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void typec_mux_put(struct typec_mux *mux) {}
^
static
>> include/linux/usb/typec_mux.h:91:5: warning: no previous prototype for function 'typec_mux_set' [-Wmissing-prototypes]
int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
^
include/linux/usb/typec_mux.h:91:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
^
static
>> include/linux/usb/typec_mux.h:103:1: warning: no previous prototype for function 'typec_mux_register' [-Wmissing-prototypes]
typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
^
include/linux/usb/typec_mux.h:102:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
struct typec_mux *
^
static
>> include/linux/usb/typec_mux.h:107:6: warning: no previous prototype for function 'typec_mux_unregister' [-Wmissing-prototypes]
void typec_mux_unregister(struct typec_mux *mux) {}
^
include/linux/usb/typec_mux.h:107:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void typec_mux_unregister(struct typec_mux *mux) {}
^
static
>> include/linux/usb/typec_mux.h:109:6: warning: no previous prototype for function 'typec_mux_set_drvdata' [-Wmissing-prototypes]
void typec_mux_set_drvdata(struct typec_mux *mux, void *data) {}
^
include/linux/usb/typec_mux.h:109:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void typec_mux_set_drvdata(struct typec_mux *mux, void *data) {}
^
static
>> include/linux/usb/typec_mux.h:110:7: warning: no previous prototype for function 'typec_mux_get_drvdata' [-Wmissing-prototypes]
void *typec_mux_get_drvdata(struct typec_mux *mux)
^
include/linux/usb/typec_mux.h:110:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void *typec_mux_get_drvdata(struct typec_mux *mux)
^
static
drivers/gpu/drm/bridge/analogix/anx7625.c:2617:23: error: incompatible pointer types assigning to 'struct typec_mux_dev *' from 'struct typec_mux *' [-Werror,-Wincompatible-pointer-types]
port_data->typec_mux = typec_mux_register(dev, &mux_desc);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/bridge/analogix/anx7625.c:2631:24: error: incompatible pointer types passing 'struct typec_mux_dev *' to parameter of type 'struct typec_mux *' [-Werror,-Wincompatible-pointer-types]
typec_mux_unregister(ctx->typec_ports[i].typec_mux);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/usb/typec_mux.h:107:45: note: passing argument to parameter 'mux' here
void typec_mux_unregister(struct typec_mux *mux) {}
^
7 warnings and 2 errors generated.


vim +/fwnode_typec_mux_get +83 include/linux/usb/typec_mux.h

0835afe6b807ae Prashant Malani 2022-06-07 82
0835afe6b807ae Prashant Malani 2022-06-07 @83 struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
0835afe6b807ae Prashant Malani 2022-06-07 84 const struct typec_altmode_desc *desc)
0835afe6b807ae Prashant Malani 2022-06-07 85 {
0835afe6b807ae Prashant Malani 2022-06-07 86 return ERR_PTR(-EOPNOTSUPP);
0835afe6b807ae Prashant Malani 2022-06-07 87 }
0835afe6b807ae Prashant Malani 2022-06-07 88
0835afe6b807ae Prashant Malani 2022-06-07 @89 void typec_mux_put(struct typec_mux *mux) {}
0835afe6b807ae Prashant Malani 2022-06-07 90
0835afe6b807ae Prashant Malani 2022-06-07 @91 int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
0835afe6b807ae Prashant Malani 2022-06-07 92 {
0835afe6b807ae Prashant Malani 2022-06-07 93 return -EOPNOTSUPP;
0835afe6b807ae Prashant Malani 2022-06-07 94 }
0835afe6b807ae Prashant Malani 2022-06-07 95
0835afe6b807ae Prashant Malani 2022-06-07 96 static inline struct typec_mux *
0835afe6b807ae Prashant Malani 2022-06-07 97 typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc)
0835afe6b807ae Prashant Malani 2022-06-07 98 {
0835afe6b807ae Prashant Malani 2022-06-07 99 return ERR_PTR(-EOPNOTSUPP);
0835afe6b807ae Prashant Malani 2022-06-07 100 }
0835afe6b807ae Prashant Malani 2022-06-07 101
0835afe6b807ae Prashant Malani 2022-06-07 102 struct typec_mux *
0835afe6b807ae Prashant Malani 2022-06-07 @103 typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
0835afe6b807ae Prashant Malani 2022-06-07 104 {
0835afe6b807ae Prashant Malani 2022-06-07 105 return ERR_PTR(-EOPNOTSUPP);
0835afe6b807ae Prashant Malani 2022-06-07 106 }
0835afe6b807ae Prashant Malani 2022-06-07 @107 void typec_mux_unregister(struct typec_mux *mux) {}
0835afe6b807ae Prashant Malani 2022-06-07 108
0835afe6b807ae Prashant Malani 2022-06-07 @109 void typec_mux_set_drvdata(struct typec_mux *mux, void *data) {}
0835afe6b807ae Prashant Malani 2022-06-07 @110 void *typec_mux_get_drvdata(struct typec_mux *mux)
0835afe6b807ae Prashant Malani 2022-06-07 111 {
0835afe6b807ae Prashant Malani 2022-06-07 112 return ERR_PTR(-EOPNOTSUPP);
0835afe6b807ae Prashant Malani 2022-06-07 113 }
0835afe6b807ae Prashant Malani 2022-06-07 114

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-06-08 18:58:16

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 6/7] drm/bridge: anx7625: Register Type-C mode switches

Hi Prashant,

I love your patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on usb/usb-testing v5.19-rc1 next-20220608]
[cannot apply to balbi-usb/testing/next peter-chen-usb/for-usb-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Prashant-Malani/usb-typec-Introduce-typec-switch-binding/20220608-042545
base: git://anongit.freedesktop.org/drm/drm drm-next
config: hexagon-buildonly-randconfig-r012-20220608 (https://download.01.org/0day-ci/archive/20220609/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project b92436efcb7813fc481b30f2593a4907568d917a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2ac4609c73d7bb4d1a585dae84559967ced3bad6
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Prashant-Malani/usb-typec-Introduce-typec-switch-binding/20220608-042545
git checkout 2ac4609c73d7bb4d1a585dae84559967ced3bad6
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/gpu/drm/bridge/analogix/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

In file included from drivers/gpu/drm/bridge/analogix/anx7625.c:18:
>> include/linux/usb/typec_mux.h:83:19: warning: no previous prototype for function 'fwnode_typec_mux_get' [-Wmissing-prototypes]
struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
^
include/linux/usb/typec_mux.h:83:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
^
static
>> include/linux/usb/typec_mux.h:89:6: warning: no previous prototype for function 'typec_mux_put' [-Wmissing-prototypes]
void typec_mux_put(struct typec_mux *mux) {}
^
include/linux/usb/typec_mux.h:89:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void typec_mux_put(struct typec_mux *mux) {}
^
static
>> include/linux/usb/typec_mux.h:91:5: warning: no previous prototype for function 'typec_mux_set' [-Wmissing-prototypes]
int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
^
include/linux/usb/typec_mux.h:91:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
^
static
>> include/linux/usb/typec_mux.h:103:1: warning: no previous prototype for function 'typec_mux_register' [-Wmissing-prototypes]
typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
^
include/linux/usb/typec_mux.h:102:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
struct typec_mux *
^
static
>> include/linux/usb/typec_mux.h:107:6: warning: no previous prototype for function 'typec_mux_unregister' [-Wmissing-prototypes]
void typec_mux_unregister(struct typec_mux *mux) {}
^
include/linux/usb/typec_mux.h:107:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void typec_mux_unregister(struct typec_mux *mux) {}
^
static
>> include/linux/usb/typec_mux.h:109:6: warning: no previous prototype for function 'typec_mux_set_drvdata' [-Wmissing-prototypes]
void typec_mux_set_drvdata(struct typec_mux *mux, void *data) {}
^
include/linux/usb/typec_mux.h:109:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void typec_mux_set_drvdata(struct typec_mux *mux, void *data) {}
^
static
>> include/linux/usb/typec_mux.h:110:7: warning: no previous prototype for function 'typec_mux_get_drvdata' [-Wmissing-prototypes]
void *typec_mux_get_drvdata(struct typec_mux *mux)
^
include/linux/usb/typec_mux.h:110:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void *typec_mux_get_drvdata(struct typec_mux *mux)
^
static
>> drivers/gpu/drm/bridge/analogix/anx7625.c:2617:23: error: incompatible pointer types assigning to 'struct typec_mux_dev *' from 'struct typec_mux *' [-Werror,-Wincompatible-pointer-types]
port_data->typec_mux = typec_mux_register(dev, &mux_desc);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/gpu/drm/bridge/analogix/anx7625.c:2631:24: error: incompatible pointer types passing 'struct typec_mux_dev *' to parameter of type 'struct typec_mux *' [-Werror,-Wincompatible-pointer-types]
typec_mux_unregister(ctx->typec_ports[i].typec_mux);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/usb/typec_mux.h:107:45: note: passing argument to parameter 'mux' here
void typec_mux_unregister(struct typec_mux *mux) {}
^
7 warnings and 2 errors generated.


vim +2617 drivers/gpu/drm/bridge/analogix/anx7625.c

2590
2591 static int anx7625_register_mode_switch(struct device *dev, struct device_node *node,
2592 struct anx7625_data *ctx)
2593 {
2594 struct anx7625_port_data *port_data;
2595 struct typec_mux_desc mux_desc = {};
2596 char name[32];
2597 u32 port_num;
2598 int ret;
2599
2600 ret = of_property_read_u32(node, "reg", &port_num);
2601 if (ret)
2602 return ret;
2603
2604 if (port_num >= ctx->num_typec_switches) {
2605 dev_err(dev, "Invalid port number specified: %d\n", port_num);
2606 return -EINVAL;
2607 }
2608
2609 port_data = &ctx->typec_ports[port_num];
2610 port_data->ctx = ctx;
2611 mux_desc.fwnode = &node->fwnode;
2612 mux_desc.drvdata = port_data;
2613 snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
2614 mux_desc.name = name;
2615 mux_desc.set = anx7625_typec_mux_set;
2616
> 2617 port_data->typec_mux = typec_mux_register(dev, &mux_desc);
2618 if (IS_ERR(port_data->typec_mux)) {
2619 ret = PTR_ERR(port_data->typec_mux);
2620 dev_err(dev, "Mode switch register for port %d failed: %d", port_num, ret);
2621 }
2622
2623 return ret;
2624 }
2625
2626 static void anx7625_unregister_typec_switches(struct anx7625_data *ctx)
2627 {
2628 int i;
2629
2630 for (i = 0; i < ctx->num_typec_switches; i++)
> 2631 typec_mux_unregister(ctx->typec_ports[i].typec_mux);
2632 }
2633

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-06-08 20:37:45

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH 2/7] usb: typec: mux: Add CONFIG guards for functions

On Jun 07 19:00, 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.
>
> Signed-off-by: Prashant Malani <[email protected]>
> ---
> include/linux/usb/typec_mux.h | 38 +++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h
> index ee57781dcf28..758d34ced1f8 100644
> --- a/include/linux/usb/typec_mux.h
> +++ b/include/linux/usb/typec_mux.h
> @@ -58,6 +58,8 @@ struct typec_mux_desc {
> void *drvdata;
> };
>
> +#if IS_ENABLED(CONFIG_TYPEC) || IS_MODULE(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);
> @@ -76,4 +78,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
> +
> +struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
> + const struct typec_altmode_desc *desc)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +
> +void typec_mux_put(struct typec_mux *mux) {}
> +
> +int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline struct typec_mux *
> +typec_mux_get(struct device *dev, const struct typec_altmode_desc *desc)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +
> +struct typec_mux *
> +typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}
> +void typec_mux_unregister(struct typec_mux *mux) {}
> +
> +void typec_mux_set_drvdata(struct typec_mux *mux, void *data) {}
> +void *typec_mux_get_drvdata(struct typec_mux *mux)
> +{
> + return ERR_PTR(-EOPNOTSUPP);
> +}

LKP discovered some issues with static inlining as well as older
(incorrect struct). [1]

I will fix this in the next version.

[1]
https://lore.kernel.org/linux-usb/[email protected]/T/#m571c46dce2339186967216bd5af25bcf9e6d1380

> +
> +#endif /* CONFIG_TYPEC */
> +
> #endif /* __USB_TYPEC_MUX */
> --
> 2.36.1.255.ge46751e96f-goog
>

2022-06-08 22:31:23

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support

On Wed, Jun 8, 2022 at 10:08 AM Prashant Malani <[email protected]> wrote:
>
> Hi Krzysztof,
>
> Thank you for looking at the patch.
>
> On Jun 08 11:24, Krzysztof Kozlowski wrote:
> > On 07/06/2022 21:00, 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.
> > >
> > > Signed-off-by: Prashant Malani <[email protected]>
> > > ---
> > > .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++
> > > 1 file changed, 56 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > index 35a48515836e..7e1f655ddfcc 100644
> > > --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> > > @@ -105,6 +105,26 @@ properties:
> > > - port@0
> > > - port@1
> > >
> > > + switches:
> > > + type: object
> > > + description: Set of switches controlling DisplayPort traffic on
> > > + outgoing RX/TX lanes to Type C ports.
> > > +
> > > + properties:
> > > + switch:
> >
> > You allow only one switch with such schema, so no need for "switches"...
>
> See below comment (summary: we'd like to allow 1 or 2 switches).
> >
> > > + $ref: /schemas/usb/typec-switch.yaml#
> > > + maxItems: 2
> >
> > Are you sure this works? what are you limiting here with maxItems? I
> > think you wanted patternProperties...
>
> Yeah, I might not have used the DT syntax correctly here.
> What I'm aiming for is:
> "switches" should can contain 1 or 2 "switch" nodes.
> 2 is the maximum (limitation of the hardware).
>
> >
> > > +
> > > + properties:
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + required:
> > > + - reg
> > > +
> > > + required:
> > > + - switch@0
> >
> > This does not match the property.
> >
> > You also need unevaluatedProperties:false
>
> Ack, will update this in the next version.

Actually, could you kindly clarify which of the two needs this?
"switches" or "switch" ?
I interpreted "switch" as requiring it, but I thought it better to confirm.

>
> >
> >
> > > +
> > > required:
> > > - compatible
> > > - reg
> > > @@ -167,5 +187,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>;
> >
> > Messed up indentation. Your previous patch should also switch to 4-space
> > as recommended by schema coding style.
>
> Sorry about that, will fix up the indentation in the next version.
>
> >
> > > + };
> > > + };
> > > + };
> > > + };
> > > + 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>;
> >
> > Ditto.
> >
> > > + };
> > > + };
> > > + };
> > > + };
> > > + };
> > > };
> > > };
> >
> >
> > Best regards,
> > Krzysztof

2022-06-08 22:40:18

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 6/7] drm/bridge: anx7625: Register Type-C mode switches

Hi Prashant,

I love your patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on usb/usb-testing v5.19-rc1 next-20220608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Prashant-Malani/usb-typec-Introduce-typec-switch-binding/20220608-042545
base: git://anongit.freedesktop.org/drm/drm drm-next
config: nios2-buildonly-randconfig-r006-20220608 (https://download.01.org/0day-ci/archive/20220609/[email protected]/config)
compiler: nios2-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/2ac4609c73d7bb4d1a585dae84559967ced3bad6
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Prashant-Malani/usb-typec-Introduce-typec-switch-binding/20220608-042545
git checkout 2ac4609c73d7bb4d1a585dae84559967ced3bad6
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=nios2 SHELL=/bin/bash drivers/gpu/drm/bridge/analogix/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All error/warnings (new ones prefixed by >>):

In file included from drivers/gpu/drm/bridge/analogix/anx7625.c:18:
>> include/linux/usb/typec_mux.h:83:19: warning: no previous prototype for 'fwnode_typec_mux_get' [-Wmissing-prototypes]
83 | struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
| ^~~~~~~~~~~~~~~~~~~~
>> include/linux/usb/typec_mux.h:89:6: warning: no previous prototype for 'typec_mux_put' [-Wmissing-prototypes]
89 | void typec_mux_put(struct typec_mux *mux) {}
| ^~~~~~~~~~~~~
>> include/linux/usb/typec_mux.h:91:5: warning: no previous prototype for 'typec_mux_set' [-Wmissing-prototypes]
91 | int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
| ^~~~~~~~~~~~~
>> include/linux/usb/typec_mux.h:103:1: warning: no previous prototype for 'typec_mux_register' [-Wmissing-prototypes]
103 | typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
| ^~~~~~~~~~~~~~~~~~
>> include/linux/usb/typec_mux.h:107:6: warning: no previous prototype for 'typec_mux_unregister' [-Wmissing-prototypes]
107 | void typec_mux_unregister(struct typec_mux *mux) {}
| ^~~~~~~~~~~~~~~~~~~~
>> include/linux/usb/typec_mux.h:109:6: warning: no previous prototype for 'typec_mux_set_drvdata' [-Wmissing-prototypes]
109 | void typec_mux_set_drvdata(struct typec_mux *mux, void *data) {}
| ^~~~~~~~~~~~~~~~~~~~~
>> include/linux/usb/typec_mux.h:110:7: warning: no previous prototype for 'typec_mux_get_drvdata' [-Wmissing-prototypes]
110 | void *typec_mux_get_drvdata(struct typec_mux *mux)
| ^~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/bridge/analogix/anx7625.c: In function 'anx7625_register_mode_switch':
>> drivers/gpu/drm/bridge/analogix/anx7625.c:2617:30: error: assignment to 'struct typec_mux_dev *' from incompatible pointer type 'struct typec_mux *' [-Werror=incompatible-pointer-types]
2617 | port_data->typec_mux = typec_mux_register(dev, &mux_desc);
| ^
drivers/gpu/drm/bridge/analogix/anx7625.c: In function 'anx7625_unregister_typec_switches':
>> drivers/gpu/drm/bridge/analogix/anx7625.c:2631:57: error: passing argument 1 of 'typec_mux_unregister' from incompatible pointer type [-Werror=incompatible-pointer-types]
2631 | typec_mux_unregister(ctx->typec_ports[i].typec_mux);
| ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
| |
| struct typec_mux_dev *
In file included from drivers/gpu/drm/bridge/analogix/anx7625.c:18:
include/linux/usb/typec_mux.h:107:45: note: expected 'struct typec_mux *' but argument is of type 'struct typec_mux_dev *'
107 | void typec_mux_unregister(struct typec_mux *mux) {}
| ~~~~~~~~~~~~~~~~~~^~~
cc1: some warnings being treated as errors


vim +2617 drivers/gpu/drm/bridge/analogix/anx7625.c

2590
2591 static int anx7625_register_mode_switch(struct device *dev, struct device_node *node,
2592 struct anx7625_data *ctx)
2593 {
2594 struct anx7625_port_data *port_data;
2595 struct typec_mux_desc mux_desc = {};
2596 char name[32];
2597 u32 port_num;
2598 int ret;
2599
2600 ret = of_property_read_u32(node, "reg", &port_num);
2601 if (ret)
2602 return ret;
2603
2604 if (port_num >= ctx->num_typec_switches) {
2605 dev_err(dev, "Invalid port number specified: %d\n", port_num);
2606 return -EINVAL;
2607 }
2608
2609 port_data = &ctx->typec_ports[port_num];
2610 port_data->ctx = ctx;
2611 mux_desc.fwnode = &node->fwnode;
2612 mux_desc.drvdata = port_data;
2613 snprintf(name, sizeof(name), "%s-%u", node->name, port_num);
2614 mux_desc.name = name;
2615 mux_desc.set = anx7625_typec_mux_set;
2616
> 2617 port_data->typec_mux = typec_mux_register(dev, &mux_desc);
2618 if (IS_ERR(port_data->typec_mux)) {
2619 ret = PTR_ERR(port_data->typec_mux);
2620 dev_err(dev, "Mode switch register for port %d failed: %d", port_num, ret);
2621 }
2622
2623 return ret;
2624 }
2625
2626 static void anx7625_unregister_typec_switches(struct anx7625_data *ctx)
2627 {
2628 int i;
2629
2630 for (i = 0; i < ctx->num_typec_switches; i++)
> 2631 typec_mux_unregister(ctx->typec_ports[i].typec_mux);
2632 }
2633

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-06-09 00:18:09

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 7/7] drm/bridge: anx7625: Add typec_mux_set callback function

Hi Prashant,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on usb/usb-testing v5.19-rc1 next-20220608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/intel-lab-lkp/linux/commits/Prashant-Malani/usb-typec-Introduce-typec-switch-binding/20220608-042545
base: git://anongit.freedesktop.org/drm/drm drm-next
config: nios2-buildonly-randconfig-r006-20220608 (https://download.01.org/0day-ci/archive/20220609/[email protected]/config)
compiler: nios2-linux-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/49c4c89ed5b50cbd623e611c8f4eba4b5ca9dd02
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Prashant-Malani/usb-typec-Introduce-typec-switch-binding/20220608-042545
git checkout 49c4c89ed5b50cbd623e611c8f4eba4b5ca9dd02
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 O=build_dir ARCH=nios2 SHELL=/bin/bash drivers/gpu/drm/bridge/analogix/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

In file included from drivers/gpu/drm/bridge/analogix/anx7625.c:19:
include/linux/usb/typec_mux.h:83:19: warning: no previous prototype for 'fwnode_typec_mux_get' [-Wmissing-prototypes]
83 | struct typec_mux *fwnode_typec_mux_get(struct fwnode_handle *fwnode,
| ^~~~~~~~~~~~~~~~~~~~
include/linux/usb/typec_mux.h:89:6: warning: no previous prototype for 'typec_mux_put' [-Wmissing-prototypes]
89 | void typec_mux_put(struct typec_mux *mux) {}
| ^~~~~~~~~~~~~
include/linux/usb/typec_mux.h:91:5: warning: no previous prototype for 'typec_mux_set' [-Wmissing-prototypes]
91 | int typec_mux_set(struct typec_mux *mux, struct typec_mux_state *state)
| ^~~~~~~~~~~~~
include/linux/usb/typec_mux.h:103:1: warning: no previous prototype for 'typec_mux_register' [-Wmissing-prototypes]
103 | typec_mux_register(struct device *parent, const struct typec_mux_desc *desc)
| ^~~~~~~~~~~~~~~~~~
include/linux/usb/typec_mux.h:107:6: warning: no previous prototype for 'typec_mux_unregister' [-Wmissing-prototypes]
107 | void typec_mux_unregister(struct typec_mux *mux) {}
| ^~~~~~~~~~~~~~~~~~~~
include/linux/usb/typec_mux.h:109:6: warning: no previous prototype for 'typec_mux_set_drvdata' [-Wmissing-prototypes]
109 | void typec_mux_set_drvdata(struct typec_mux *mux, void *data) {}
| ^~~~~~~~~~~~~~~~~~~~~
include/linux/usb/typec_mux.h:110:7: warning: no previous prototype for 'typec_mux_get_drvdata' [-Wmissing-prototypes]
110 | void *typec_mux_get_drvdata(struct typec_mux *mux)
| ^~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/bridge/analogix/anx7625.c: In function 'anx7625_typec_mux_set':
>> drivers/gpu/drm/bridge/analogix/anx7625.c:2616:64: error: passing argument 1 of 'typec_mux_get_drvdata' from incompatible pointer type [-Werror=incompatible-pointer-types]
2616 | struct anx7625_port_data *data = typec_mux_get_drvdata(mux);
| ^~~
| |
| struct typec_mux_dev *
In file included from drivers/gpu/drm/bridge/analogix/anx7625.c:19:
include/linux/usb/typec_mux.h:110:47: note: expected 'struct typec_mux *' but argument is of type 'struct typec_mux_dev *'
110 | void *typec_mux_get_drvdata(struct typec_mux *mux)
| ~~~~~~~~~~~~~~~~~~^~~
drivers/gpu/drm/bridge/analogix/anx7625.c: In function 'anx7625_register_mode_switch':
drivers/gpu/drm/bridge/analogix/anx7625.c:2675:30: error: assignment to 'struct typec_mux_dev *' from incompatible pointer type 'struct typec_mux *' [-Werror=incompatible-pointer-types]
2675 | port_data->typec_mux = typec_mux_register(dev, &mux_desc);
| ^
drivers/gpu/drm/bridge/analogix/anx7625.c: In function 'anx7625_unregister_typec_switches':
drivers/gpu/drm/bridge/analogix/anx7625.c:2689:57: error: passing argument 1 of 'typec_mux_unregister' from incompatible pointer type [-Werror=incompatible-pointer-types]
2689 | typec_mux_unregister(ctx->typec_ports[i].typec_mux);
| ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
| |
| struct typec_mux_dev *
In file included from drivers/gpu/drm/bridge/analogix/anx7625.c:19:
include/linux/usb/typec_mux.h:107:45: note: expected 'struct typec_mux *' but argument is of type 'struct typec_mux_dev *'
107 | void typec_mux_unregister(struct typec_mux *mux) {}
| ~~~~~~~~~~~~~~~~~~^~~
cc1: some warnings being treated as errors


vim +/typec_mux_get_drvdata +2616 drivers/gpu/drm/bridge/analogix/anx7625.c

2612
2613 static int anx7625_typec_mux_set(struct typec_mux_dev *mux,
2614 struct typec_mux_state *state)
2615 {
> 2616 struct anx7625_port_data *data = typec_mux_get_drvdata(mux);
2617 struct anx7625_data *ctx = data->ctx;
2618 struct device *dev = &ctx->client->dev;
2619
2620 bool old_dp_connected = (ctx->typec_ports[0].dp_connected ||
2621 ctx->typec_ports[1].dp_connected);
2622 bool new_dp_connected;
2623
2624 if (ctx->num_typec_switches == 1)
2625 return 0;
2626
2627 dev_dbg(dev, "mux_set dp_connected: c0=%d, c1=%d\n",
2628 ctx->typec_ports[0].dp_connected, ctx->typec_ports[1].dp_connected);
2629
2630 data->dp_connected = (state->alt && state->alt->svid == USB_TYPEC_DP_SID &&
2631 state->alt->mode == USB_TYPEC_DP_MODE);
2632
2633 new_dp_connected = (ctx->typec_ports[0].dp_connected ||
2634 ctx->typec_ports[1].dp_connected);
2635
2636 /* dp on, power on first */
2637 if (!old_dp_connected && new_dp_connected)
2638 pm_runtime_get_sync(dev);
2639
2640 anx7625_typec_two_ports_update(ctx);
2641
2642 /* dp off, power off last */
2643 if (old_dp_connected && !new_dp_connected)
2644 pm_runtime_put_sync(dev);
2645
2646 return 0;
2647 }
2648

--
0-DAY CI Kernel Test Service
https://01.org/lkp

2022-06-09 06:51:44

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support

On 08/06/2022 23:56, Prashant Malani wrote:
> On Wed, Jun 8, 2022 at 10:08 AM Prashant Malani <[email protected]> wrote:
>>
>> Hi Krzysztof,
>>
>> Thank you for looking at the patch.
>>
>> On Jun 08 11:24, Krzysztof Kozlowski wrote:
>>> On 07/06/2022 21:00, 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.
>>>>
>>>> Signed-off-by: Prashant Malani <[email protected]>
>>>> ---
>>>> .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++
>>>> 1 file changed, 56 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
>>>> index 35a48515836e..7e1f655ddfcc 100644
>>>> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
>>>> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
>>>> @@ -105,6 +105,26 @@ properties:
>>>> - port@0
>>>> - port@1
>>>>
>>>> + switches:
>>>> + type: object
>>>> + description: Set of switches controlling DisplayPort traffic on
>>>> + outgoing RX/TX lanes to Type C ports.
>>>> +
>>>> + properties:
>>>> + switch:
>>>
>>> You allow only one switch with such schema, so no need for "switches"...
>>
>> See below comment (summary: we'd like to allow 1 or 2 switches).
>>>
>>>> + $ref: /schemas/usb/typec-switch.yaml#
>>>> + maxItems: 2
>>>
>>> Are you sure this works? what are you limiting here with maxItems? I
>>> think you wanted patternProperties...
>>
>> Yeah, I might not have used the DT syntax correctly here.
>> What I'm aiming for is:
>> "switches" should can contain 1 or 2 "switch" nodes.
>> 2 is the maximum (limitation of the hardware).
>>
>>>
>>>> +
>>>> + properties:
>>>> + reg:
>>>> + maxItems: 1
>>>> +
>>>> + required:
>>>> + - reg
>>>> +
>>>> + required:
>>>> + - switch@0
>>>
>>> This does not match the property.
>>>
>>> You also need unevaluatedProperties:false
>>
>> Ack, will update this in the next version.
>
> Actually, could you kindly clarify which of the two needs this?
> "switches" or "switch" ?
> I interpreted "switch" as requiring it, but I thought it better to confirm.

Depends what do you want to have there. If two properties called
"switch", then "switches" is ok. However old code had only one property
thus switches with maximum one switch is a bit weird.

Looking at example you wanted to switch@[01], so you need to use
patternProperties.


Best regards,
Krzysztof

2022-06-09 07:09:07

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 3/7] dt-bindings: usb: Add Type-C switch binding

On 08/06/2022 19:15, Prashant Malani wrote:
> Hi Krzysztof,
>
> On Jun 08 11:18, Krzysztof Kozlowski wrote:
>> On 07/06/2022 21:00, 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).
>>>
>>> Signed-off-by: Prashant Malani <[email protected]>
>>> ---
>>> .../devicetree/bindings/usb/typec-switch.yaml | 76 +++++++++++++++++++
>>> 1 file changed, 76 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..60a600a63fef
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/usb/typec-switch.yaml
>>> @@ -0,0 +1,76 @@
>>> +# 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:
>>> + items:
>>
>> Single item, so no need for items.
>
> I tried this initially, but dt_binding_check throws the following
> error:
> $ make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/usb/typec-switch.yaml
>
> LINT Documentation/devicetree/bindings
> CHKDT Documentation/devicetree/bindings/processed-schema.json
> /foo/linux/Documentation/devicetree/bindings/usb/typec-switch.yaml: properties:compatible: [{'enum': ['typec-switch']}] is not of type 'object', 'boolean'
> from schema $id: http://json-schema.org/draft-07/schema#
> SCHEMA Documentation/devicetree/bindings/processed-schema.json
> /foo/linux/Documentation/devicetree/bindings/usb/typec-switch.yaml: ignoring, error in schema: properties: compatible

Probably you messed up the syntax in the enum. Just open some examples:

git grep -C 3 'properties:' -- Documentation/devicetree/bindings/*/*yaml

Documentation/devicetree/bindings/ata/intel,ixp4xx-compact-flash.yaml

Best regards,
Krzysztof

2022-06-09 19:41:22

by Prashant Malani

[permalink] [raw]
Subject: Re: [PATCH 4/7] dt-bindings: drm/bridge: anx7625: Add mode-switch support

On Wed, Jun 8, 2022 at 11:41 PM Krzysztof Kozlowski
<[email protected]> wrote:
>
> On 08/06/2022 23:56, Prashant Malani wrote:
> > On Wed, Jun 8, 2022 at 10:08 AM Prashant Malani <[email protected]> wrote:
> >>
> >> Hi Krzysztof,
> >>
> >> Thank you for looking at the patch.
> >>
> >> On Jun 08 11:24, Krzysztof Kozlowski wrote:
> >>> On 07/06/2022 21:00, 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.
> >>>>
> >>>> Signed-off-by: Prashant Malani <[email protected]>
> >>>> ---
> >>>> .../display/bridge/analogix,anx7625.yaml | 56 +++++++++++++++++++
> >>>> 1 file changed, 56 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> >>>> index 35a48515836e..7e1f655ddfcc 100644
> >>>> --- a/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> >>>> +++ b/Documentation/devicetree/bindings/display/bridge/analogix,anx7625.yaml
> >>>> @@ -105,6 +105,26 @@ properties:
> >>>> - port@0
> >>>> - port@1
> >>>>
> >>>> + switches:
> >>>> + type: object
> >>>> + description: Set of switches controlling DisplayPort traffic on
> >>>> + outgoing RX/TX lanes to Type C ports.
> >>>> +
> >>>> + properties:
> >>>> + switch:
> >>>
> >>> You allow only one switch with such schema, so no need for "switches"...
> >>
> >> See below comment (summary: we'd like to allow 1 or 2 switches).
> >>>
> >>>> + $ref: /schemas/usb/typec-switch.yaml#
> >>>> + maxItems: 2
> >>>
> >>> Are you sure this works? what are you limiting here with maxItems? I
> >>> think you wanted patternProperties...
> >>
> >> Yeah, I might not have used the DT syntax correctly here.
> >> What I'm aiming for is:
> >> "switches" should can contain 1 or 2 "switch" nodes.
> >> 2 is the maximum (limitation of the hardware).
> >>
> >>>
> >>>> +
> >>>> + properties:
> >>>> + reg:
> >>>> + maxItems: 1
> >>>> +
> >>>> + required:
> >>>> + - reg
> >>>> +
> >>>> + required:
> >>>> + - switch@0
> >>>
> >>> This does not match the property.
> >>>
> >>> You also need unevaluatedProperties:false
> >>
> >> Ack, will update this in the next version.
> >
> > Actually, could you kindly clarify which of the two needs this?
> > "switches" or "switch" ?
> > I interpreted "switch" as requiring it, but I thought it better to confirm.
>
> Depends what do you want to have there. If two properties called
> "switch", then "switches" is ok. However old code had only one property
> thus switches with maximum one switch is a bit weird.
>
> Looking at example you wanted to switch@[01], so you need to use
> patternProperties.

Thanks for the suggestion. I've made the change in v2.

Regards,

-Prashant