Messages in this thread | ![/](/images/icornerl.gif) | | From | Chen-Yu Tsai <> | Date | Mon, 9 Jan 2023 18:10:01 +0800 | Subject | Re: [PATCH v9 3/9] drm/display: Add Type-C switch helpers |
| |
On Mon, Jan 9, 2023 at 4:41 PM Pin-yen Lin <treapking@chromium.org> wrote: > > Add helpers to register and unregister Type-C "switches" for bridges > capable of switching their output between two downstream devices. > > The helper registers USB Type-C mode switches when the "mode-switch" > and the "data-lanes" properties are available in Device Tree. > > Signed-off-by: Pin-yen Lin <treapking@chromium.org>
Tested-by: Chen-Yu Tsai <wenst@chromium.org>
on MT8192 based Hayato (ASUS Chromebook Flip CM3200).
> --- > > (no changes since v8) > > Changes in v8: > - Fixed the build issue when CONFIG_TYPEC=m > - Fixed some style issues > > Changes in v7: > - Extracted the common codes to a helper function > - New in v7 > > drivers/gpu/drm/display/drm_dp_helper.c | 132 ++++++++++++++++++++++++ > include/drm/display/drm_dp_helper.h | 16 +++ > 2 files changed, 148 insertions(+) > > diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c > index 16565a0a5da6..fb9e23744c08 100644 > --- a/drivers/gpu/drm/display/drm_dp_helper.c > +++ b/drivers/gpu/drm/display/drm_dp_helper.c > @@ -30,11 +30,13 @@ > #include <linux/sched.h> > #include <linux/seq_file.h> > #include <linux/string_helpers.h> > +#include <linux/usb/typec_mux.h> > #include <linux/dynamic_debug.h> > > #include <drm/display/drm_dp_helper.h> > #include <drm/display/drm_dp_mst_helper.h> > #include <drm/drm_edid.h> > +#include <drm/drm_of.h> > #include <drm/drm_print.h> > #include <drm/drm_vblank.h> > #include <drm/drm_panel.h> > @@ -3891,3 +3893,133 @@ int drm_panel_dp_aux_backlight(struct drm_panel *panel, struct drm_dp_aux *aux) > EXPORT_SYMBOL(drm_panel_dp_aux_backlight); > > #endif > + > +#if IS_REACHABLE(CONFIG_TYPEC) > +static int drm_dp_register_mode_switch(struct device *dev, struct device_node *node, > + struct drm_dp_typec_switch_desc *switch_desc, > + void *data, void *mux_set) > +{ > + struct drm_dp_typec_port_data *port_data; > + struct typec_mux_desc mux_desc = {}; > + char name[32]; > + u32 dp_lanes[2]; > + int ret, num_lanes, port_num = -1; > + > + num_lanes = drm_of_get_data_lanes_count(node, 0, 2); > + if (num_lanes <= 0) { > + dev_err(dev, "Error on getting data lanes count: %d\n", > + num_lanes);
Also printing out the full node name (endpoint@N) would be more helpful.
> + return num_lanes; > + } > + > + ret = of_property_read_u32_array(node, "data-lanes", dp_lanes, num_lanes); > + if (ret) { > + dev_err(dev, "Failed to read the data-lanes variable: %d\n", > + ret);
Same here.
> + return ret; > + } > + > + port_num = dp_lanes[0] / 2; > + > + port_data = &switch_desc->typec_ports[port_num]; > + port_data->data = data; > + 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 = 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\n", > + port_num, ret); > + } > + > + return ret; > +} > + > +/** > + * drm_dp_register_typec_switches() - register Type-C switches > + * @dev: Device that registers Type-C switches > + * @port: Device node for the switch > + * @switch_desc: A Type-C switch descriptor > + * @data: Private data for the switches > + * @mux_set: Callback function for typec_mux_set > + * > + * This function registers USB Type-C switches for DP bridges that can switch > + * the output signal between their output pins. > + * > + * Currently only mode switches are implemented, and the function assumes the > + * given @port device node has endpoints with "mode-switch" property. > + * Register the endpoint as port 0 if the "data-lanes" property falls in 0/1, > + * and register it as port 1 if "data-lanes" falls in 2/3. > + */ > +int drm_dp_register_typec_switches(struct device *dev, struct device_node *port, > + struct drm_dp_typec_switch_desc *switch_desc, > + void *data, void *mux_set) > +{ > + struct device_node *sw; > + int ret; > + > + for_each_child_of_node(port, sw) { > + if (of_property_read_bool(sw, "mode-switch")) > + switch_desc->num_typec_switches++; > + } > + > + if (!switch_desc->num_typec_switches) { > + dev_warn(dev, "No Type-C switches node found\n");
Maybe change this to dev_info or even dev_debug? A warning would be too noisy if the bridge drivers are calling this helper unconditionally.
Otherwise,
Reviewed-by: Chen-Yu Tsai <wenst@chromium.org>
| ![\](/images/icornerr.gif) |