Message ID | 20240901040658.157425-1-swboyd@chromium.org |
---|---|
Headers | show |
Series | platform/chrome: Add DT USB/DP muxing/topology support | expand |
On Sat, Aug 31, 2024 at 09:06:41PM -0700, Stephen Boyd wrote: > Ease driver development by adding stubs for the typec_switch APIs when > CONFIG_TYPEC=n. Copy the same method used for the typec_mux APIs to be > consistent. > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: <linux-usb@vger.kernel.org> > Cc: Pin-yen Lin <treapking@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > include/linux/usb/typec_mux.h | 43 +++++++++++++++++++++++++++++++---- > 1 file changed, 38 insertions(+), 5 deletions(-) > > diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h > index 2489a7857d8e..efb5ed32b813 100644 > --- a/include/linux/usb/typec_mux.h > +++ b/include/linux/usb/typec_mux.h > @@ -3,6 +3,7 @@ > #ifndef __USB_TYPEC_MUX > #define __USB_TYPEC_MUX > > +#include <linux/err.h> > #include <linux/property.h> > #include <linux/usb/typec.h> > > @@ -24,16 +25,13 @@ struct typec_switch_desc { > void *drvdata; > }; > > +#if IS_ENABLED(CONFIG_TYPEC) > + > struct typec_switch *fwnode_typec_switch_get(struct fwnode_handle *fwnode); > void typec_switch_put(struct typec_switch *sw); > int typec_switch_set(struct typec_switch *sw, > enum typec_orientation orientation); > > -static inline struct typec_switch *typec_switch_get(struct device *dev) > -{ > - return fwnode_typec_switch_get(dev_fwnode(dev)); > -} > - > struct typec_switch_dev * > typec_switch_register(struct device *parent, > const struct typec_switch_desc *desc); > @@ -42,6 +40,41 @@ void typec_switch_unregister(struct typec_switch_dev *sw); > void typec_switch_set_drvdata(struct typec_switch_dev *sw, void *data); > void *typec_switch_get_drvdata(struct typec_switch_dev *sw); > > +#else > + > +static inline struct typec_switch * > +fwnode_typec_switch_get(struct fwnode_handle *fwnode) > +{ > + return NULL; > +} > +static inline void typec_switch_put(struct typec_switch *sw) {} > +static inline int typec_switch_set(struct typec_switch *sw, > + enum typec_orientation orientation) > +{ > + return 0; > +} > + > +static inline struct typec_switch_dev * > +typec_switch_register(struct device *parent, > + const struct typec_switch_desc *desc) > +{ > + return ERR_PTR(-EOPNOTSUPP); > +} > +static inline void typec_switch_unregister(struct typec_switch_dev *sw) {} > + > +static inline void typec_switch_set_drvdata(struct typec_switch_dev *sw, void *data) {} > +static inline void *typec_switch_get_drvdata(struct typec_switch_dev *sw) > +{ > + return ERR_PTR(-EOPNOTSUPP); > +} > + > +#endif /* CONFIG_TYPEC */ > + > +static inline struct typec_switch *typec_switch_get(struct device *dev) > +{ > + return fwnode_typec_switch_get(dev_fwnode(dev)); > +} > + > struct typec_mux_state { > struct typec_altmode *alt; > unsigned long mode;
On Sat, 31 Aug 2024, Stephen Boyd wrote: > Add a DT graph binding to google,cros-ec-typec so that it can combine > DisplayPort (DP) and USB SuperSpeed (SS) data into a USB type-c endpoint > that is connected to the usb-c-connector node's SS endpoint. This also > allows us to connect the DP and USB nodes in the graph to the USB type-c > connectors, providing the full picture of the USB type-c data flows in > the system. > > Allow there to be multiple typec nodes underneath the EC node so that > one DT graph exists per DP bridge. The EC is actually controlling TCPCs > and redrivers that combine the DP and USB signals together so this more > accurately reflects the hardware design without introducing yet another > DT node underneath the EC for USB type-c. > > If the type-c ports are being shared between a single DP controller then > the ports need to know about each other and determine a policy to drive > DP to one type-c port. If the type-c ports each have their own dedicated > DP controller then they're able to operate independently and enter/exit > DP altmode independently as well. We can't connect the DP controller's > endpoint to one usb-c-connector port@1 endpoint and the USB controller's > endpoint to another usb-c-connector port@1 endpoint either because the > DP muxing case would have DP connected to two usb-c-connector endpoints > which the graph binding doesn't support. > > Therefore, one typec node is required per the capabilities of the type-c > port(s) being managed. This also lets us indicate which type-c ports the > DP controller is wired to. For example, if DP was connected to ports 0 > and 2, while port 1 was connected to another DP controller we wouldn't > be able to implement that without having some other DT property to > indicate which output ports are connected to the DP endpoint. > > Reviewed-by: Rob Herring (Arm) <robh@kernel.org> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > Cc: Conor Dooley <conor+dt@kernel.org> > Acked-by: Lee Jones <lee@kernel.org> > Cc: Benson Leung <bleung@chromium.org> > Cc: Guenter Roeck <groeck@chromium.org> > Cc: Prashant Malani <pmalani@chromium.org> > Cc: Tzung-Bi Shih <tzungbi@kernel.org> > Cc: <devicetree@vger.kernel.org> > Cc: <chrome-platform@lists.linux.dev> > Cc: Pin-yen Lin <treapking@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > .../bindings/mfd/google,cros-ec.yaml | 7 +- Acked-by: Lee Jones <lee@kernel.org> > .../bindings/usb/google,cros-ec-typec.yaml | 229 ++++++++++++++++++ > 2 files changed, 233 insertions(+), 3 deletions(-)
Quoting Andy Shevchenko (2024-09-02 04:12:49) > On Sat, Aug 31, 2024 at 09:06:49PM -0700, Stephen Boyd wrote: > > Is it possible to move these Cc:s after --- line below? Ok. > > > /** > > * devcon_match_fn_t - device connection match function > > * @fwnode: Remote connection's device node > > + * @endpoint: Remote connection's endpoint node > > * @con_id: Identifier for the connection > > * @data: Match function caller specific data > > * > > * Implement a callback with this function signature to search a fwnode's > > * connections for a match with a function like device_connection_find_match(). > > * This function will be called possibly multiple times, once for each > > - * connection. The match function should inspect the @fwnode to look for a > > - * match. The @con_id and @data provided are the same as the @con_id and @data > > - * arguments passed to the functions that take a devcon_match_fn_t argument. > > + * connection. The match function should inspect the connection's @fwnode > > + * and/or @endpoint to look for a match. The @con_id and @data provided are the > > + * same as the @con_id and @data arguments passed to the functions that take a > > + * devcon_match_fn_t argument. > > So, struct fwnode_handle is a single-linked list. Can we utilise that instead > of adding a new parameter? I.o.w. do those objects (@fwnode and @endpoint) have > anything in common and can be chained? No, we can't use that. We need to know which endpoint in the remote fwnode is connected to the fwnode we're searching from. This is how we know which typec mux structure is associated with which type-c port so we can drive DP there. We might have two endpoints connected to the same fwnode and then we wouldn't be able to differentiate the endpoint and the typec mux to configure. > > > * Note: This function can be called multiple times. > > What does this mean? Is it idempotent? Or what is the effect of being called > multiple times? I've removed this note now.
On Sat, Aug 31, 2024 at 09:06:55PM -0700, Stephen Boyd wrote: > Most ARM based chromebooks with two usb-c-connector nodes and one DP > controller are muxing the DP lanes between the two USB ports. This is > done so that the type-c ports are at least equal in capability if not > functionality. Either an analog mux is used to steer the DP signal to > one or the other port, or a DP bridge chip has two lanes (e.g. DP > ML0/ML1) wired to one type-c port while the other two (e.g. DP ML2/ML3) > are wired to another type-c port. > > [...] > > Cc: Prashant Malani <pmalani@chromium.org> > Cc: Benson Leung <bleung@chromium.org> > Cc: Tzung-Bi Shih <tzungbi@kernel.org> > Cc: <chrome-platform@lists.linux.dev> > Cc: Pin-yen Lin <treapking@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
On Sat, Aug 31, 2024 at 09:06:41PM GMT, Stephen Boyd wrote: > Ease driver development by adding stubs for the typec_switch APIs when > CONFIG_TYPEC=n. Copy the same method used for the typec_mux APIs to be > consistent. > > Cc: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: <linux-usb@vger.kernel.org> > Cc: Pin-yen Lin <treapking@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > include/linux/usb/typec_mux.h | 43 +++++++++++++++++++++++++++++++---- > 1 file changed, 38 insertions(+), 5 deletions(-) > > diff --git a/include/linux/usb/typec_mux.h b/include/linux/usb/typec_mux.h > index 2489a7857d8e..efb5ed32b813 100644 > --- a/include/linux/usb/typec_mux.h > +++ b/include/linux/usb/typec_mux.h > @@ -3,6 +3,7 @@ > #ifndef __USB_TYPEC_MUX > #define __USB_TYPEC_MUX > > +#include <linux/err.h> > #include <linux/property.h> > #include <linux/usb/typec.h> > > @@ -24,16 +25,13 @@ struct typec_switch_desc { > void *drvdata; > }; > > +#if IS_ENABLED(CONFIG_TYPEC) > + > struct typec_switch *fwnode_typec_switch_get(struct fwnode_handle *fwnode); > void typec_switch_put(struct typec_switch *sw); > int typec_switch_set(struct typec_switch *sw, > enum typec_orientation orientation); > > -static inline struct typec_switch *typec_switch_get(struct device *dev) > -{ > - return fwnode_typec_switch_get(dev_fwnode(dev)); > -} > - > struct typec_switch_dev * > typec_switch_register(struct device *parent, > const struct typec_switch_desc *desc); > @@ -42,6 +40,41 @@ void typec_switch_unregister(struct typec_switch_dev *sw); > void typec_switch_set_drvdata(struct typec_switch_dev *sw, void *data); > void *typec_switch_get_drvdata(struct typec_switch_dev *sw); > > +#else > + > +static inline struct typec_switch * > +fwnode_typec_switch_get(struct fwnode_handle *fwnode) > +{ > + return NULL; > +} > +static inline void typec_switch_put(struct typec_switch *sw) {} > +static inline int typec_switch_set(struct typec_switch *sw, > + enum typec_orientation orientation) > +{ > + return 0; > +} Just my 2c: The stubs above look fine from my POV, they help us to cleanup the users of the API. The register/unregister callbacks are not. The switch providers should clearly know whether the API is actually available or not. Compare this to how other subsystems (regulators, interconnects, etc) provide stubs for consumer API only. In other words, please consider sending a patch that drops provider-side Type-C MUX API stubs. > + > +static inline struct typec_switch_dev * > +typec_switch_register(struct device *parent, > + const struct typec_switch_desc *desc) > +{ > + return ERR_PTR(-EOPNOTSUPP); > +} > +static inline void typec_switch_unregister(struct typec_switch_dev *sw) {} > + > +static inline void typec_switch_set_drvdata(struct typec_switch_dev *sw, void *data) {} > +static inline void *typec_switch_get_drvdata(struct typec_switch_dev *sw) > +{ > + return ERR_PTR(-EOPNOTSUPP); > +} > + > +#endif /* CONFIG_TYPEC */ > + > +static inline struct typec_switch *typec_switch_get(struct device *dev) > +{ > + return fwnode_typec_switch_get(dev_fwnode(dev)); > +} > + > struct typec_mux_state { > struct typec_altmode *alt; > unsigned long mode; > -- > https://chromeos.dev >
On Sat, Aug 31, 2024 at 09:06:51PM GMT, Stephen Boyd wrote: > Extend the usb-switch binding to support DisplayPort (DP) alternate > modes. A third port for the DP signal is necessary when a mode-switch is > muxing USB and DP together onto a usb type-c connector. Add data-lanes > to the usbc output node to allow a device using this binding to remap > the data lanes on the output. Add an example to show how this new port > can be used. > > Reviewed-by: Rob Herring (Arm) <robh@kernel.org> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> > Cc: Conor Dooley <conor+dt@kernel.org> > Cc: Benson Leung <bleung@chromium.org> > Cc: Guenter Roeck <groeck@chromium.org> > Cc: Prashant Malani <pmalani@chromium.org> > Cc: Tzung-Bi Shih <tzungbi@kernel.org> > Cc: <devicetree@vger.kernel.org> > Cc: <chrome-platform@lists.linux.dev> > Cc: Pin-yen Lin <treapking@chromium.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > .../devicetree/bindings/usb/usb-switch.yaml | 89 +++++++++++++++++++ > 1 file changed, 89 insertions(+) > > diff --git a/Documentation/devicetree/bindings/usb/usb-switch.yaml b/Documentation/devicetree/bindings/usb/usb-switch.yaml > index f5dc7e23b134..816f295f322f 100644 > --- a/Documentation/devicetree/bindings/usb/usb-switch.yaml > +++ b/Documentation/devicetree/bindings/usb/usb-switch.yaml > @@ -52,6 +52,14 @@ properties: > endpoint: > $ref: '#/$defs/usbc-in-endpoint' > > + port@2: > + $ref: /schemas/graph.yaml#/$defs/port-base > + unevaluatedProperties: false > + > + properties: > + endpoint: > + $ref: '#/$defs/dp-endpoint' Is it a separate port or is it an endpoint of the same upstream-facing (non-connector-facing) SS port? > + > oneOf: > - required: > - port > @@ -65,6 +73,19 @@ $defs: > $ref: /schemas/graph.yaml#/$defs/endpoint-base > description: Super Speed (SS) output endpoint to a type-c connector > unevaluatedProperties: false > + properties: > + data-lanes: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: | > + An array of physical USB Type-C data lane indexes. > + - 0 is SSRX1 lane > + - 1 is SSTX1 lane > + - 2 is SSTX2 lane > + - 3 is SSRX2 lane > + minItems: 4 > + maxItems: 4 > + items: > + maximum: 3 What is the usecase to delare less than 4 lanes going to the USB-C connector? > > usbc-in-endpoint: > $ref: /schemas/graph.yaml#/$defs/endpoint-base > @@ -79,7 +100,75 @@ $defs: > items: > maximum: 8 > > + dp-endpoint: > + $ref: /schemas/graph.yaml#/$defs/endpoint-base > + description: DisplayPort (DP) input from the DP PHY > + unevaluatedProperties: false > + properties: > + data-lanes: > + $ref: /schemas/types.yaml#/definitions/uint32-array > + description: | > + An array of physical DP data lane indexes > + - 0 is DP ML0 lane > + - 1 is DP ML1 lane > + - 2 is DP ML2 lane > + - 3 is DP ML3 lane > + oneOf: > + - items: > + - const: 0 > + - const: 1 > + - items: > + - const: 0 > + - const: 1 > + - const: 2 > + - const: 3 > + > examples: > + # A USB + DP mode and orientation switch which muxes DP altmode > + # and USB onto a usb-c-connector node. > + - | > + device { > + mode-switch; > + orientation-switch; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + endpoint { > + remote-endpoint = <&usb_c_connector>; > + data-lanes = <0 1 2 3>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + endpoint { > + remote-endpoint = <&usb_ss_phy>; > + }; > + }; > + > + port@2 { > + reg = <2>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + endpoint { > + remote-endpoint = <&dp_phy>; > + data-lanes = <0 1 2 3>; > + }; > + }; > + }; > + }; > + > # A USB orientation switch which flips the pin orientation > # for a usb-c-connector node. > - | > -- > https://chromeos.dev >