From: Jun Li <email@example.com> To: Heikki Krogerus <firstname.lastname@example.org> Cc: "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, "email@example.com" <firstname.lastname@example.org>, dl-linux-imx <email@example.com>, "firstname.lastname@example.org" <email@example.com>, "firstname.lastname@example.org" <email@example.com>, "firstname.lastname@example.org" <email@example.com>, Hans de Goede <firstname.lastname@example.org> Subject: RE: [PATCH 3/4] usb: typec: add typec orientation switch support via mux controller Date: Mon, 31 May 2021 11:58:48 +0000 [thread overview] Message-ID: <VI1PR04MB5935CF87B5DB6BC62BD69B10893F9@VI1PR04MB5935.eurprd04.prod.outlook.com> (raw) In-Reply-To: <YK4R4q1cZqQDS1qy@kuha.fi.intel.com> > -----Original Message----- > From: Heikki Krogerus <email@example.com> > Sent: Wednesday, May 26, 2021 5:16 PM > To: Jun Li <firstname.lastname@example.org> > Cc: email@example.com; firstname.lastname@example.org; email@example.com; > firstname.lastname@example.org; email@example.com; dl-linux-imx > <firstname.lastname@example.org>; email@example.com; > firstname.lastname@example.org > Subject: Re: [PATCH 3/4] usb: typec: add typec orientation switch support > via mux controller > > On Tue, May 25, 2021 at 11:46:18AM +0000, Jun Li wrote: > > Hi Heikki, > > > > > -----Original Message----- > > > From: Heikki Krogerus <email@example.com> > > > Sent: Friday, May 21, 2021 4:38 PM > > > To: Jun Li <firstname.lastname@example.org> > > > Cc: email@example.com; firstname.lastname@example.org; > > > email@example.com; firstname.lastname@example.org; > > > email@example.com; dl-linux-imx <firstname.lastname@example.org>; > > > email@example.com; firstname.lastname@example.org > > > Subject: Re: [PATCH 3/4] usb: typec: add typec orientation switch > > > support via mux controller > > > > > > Hi, > > > > > > On Thu, May 20, 2021 at 03:33:36PM +0300, Heikki Krogerus wrote: > > > > Why not just do that inside fwnode_typec_switch_get() and handle > > > > the whole thing in drivers/usb/typec/mux.c (or in its own file if > > > > you prefer)? > > > > > > > > You'll just need to register a "wrapper" Type-C switch object for > > > > the OF mux controller, but that should not be a problem. That way > > > > you don't need to export any new functions, touch this file or > > > > anything else. > > > > > > I wrote a bit of code just to see how that would look. I'm attaching > > > you the hack I made. I guess something like that would not be too bad. > > > A wrapper is probable always a bit clumsy, but I'm not sure that in > > > this case it's a huge problem. Of course if there are any better > > > ideas, let's here them :-) > > > > Thanks for your patch, I am pasting the patch as below. > > > > seems we need consider more than that. > > > > diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile > > index a0adb8947a301..d85231b2fe10b 100644 > > --- a/drivers/usb/typec/Makefile > > +++ b/drivers/usb/typec/Makefile > > @@ -1,6 +1,7 @@ > > # SPDX-License-Identifier: GPL-2.0 > > obj-$(CONFIG_TYPEC) += typec.o > > typec-y := class.o mux.o bus.o port-mapper.o > > +typec-$(MULTIPLEXER) += of_mux.o > > obj-$(CONFIG_TYPEC) += altmodes/ > > obj-$(CONFIG_TYPEC_TCPM) += tcpm/ > > obj-$(CONFIG_TYPEC_UCSI) += ucsi/ > > diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c index > > 9da22ae3006c9..282622276d97b 100644 > > --- a/drivers/usb/typec/mux.c > > +++ b/drivers/usb/typec/mux.c > > @@ -63,6 +63,9 @@ struct typec_switch *fwnode_typec_switch_get(struct > > fwnode_handle *fwnode) > > > > sw = fwnode_connection_find_match(fwnode, "orientation-switch", NULL, > > typec_switch_match); > > + if (!sw) > > + sw = of_switch_register(fwnode); > > + > > > > How to support multiple typec_switch_get() for of_mux case? > > the second call to fwnode_typec_switch_get() will get the switch via > > fwnode_connection_find_match()? This means we still need a property > > "orientation-switch" for mux controller node, this seems not the > > expected way for a mux consumer, even with this property, we will get > > a EPROBE_DEFER for the first call. > > > > If we use mux_control_get() for multiple caller/consumer, then we need > > some other device as input. > > > > typec_switch object looks to me is a provider, if we create and > > maintain it in consumer side, I have not come out a better way:-). > > Sorry, but can we rewind a bit: Why can't you just register the orientation > switch from your mux driver and be done with it? You should then be able > to use OF graph, and no special bindings should be needed, no? So we still need a special property for OF graph per discussion on another thread(use device type other than device name for match), and this has to be a mux controller core binding for possible different mux chips (GPIO/MMIO...), register a typec switch if this property exist, but this is the user specific thing from mux controller point view, I feed this is again against DT binding's expectation. > > If you want to reuse a mux-controller driver, then you do need to modify > it (but only a little), and what ever mux-controller specific bindings there > are, you will not use those when the mux supplies the orientation switching > function, instead you'll use the OF graph for that. But surely that is not > a problem? > > The mux-controller framework expects the "consumers" of the muxes to > understand the final function that the mux is used for. The Type-C "mux" > framework (I should not even talk about muxes with those) works the other > way around. Fully agree. > The driver for the component that supplies the orientation switch > function must understand that it is handling that function, and there is > a good reason for doing it that way with the USB Type-C switches. I understand yes if the switch is only part function of the driver. > The > orientation switch for example quite simply is _not_ always a mux. In fact, > it's seems to be rarely a mux these days. With USB4 for example the orientation > is handled almost always by the first on-board retimer. If the mux is only part function of a new driver, use the tyepc "mux" framework and create new binding for the new driver is fine. But if the typec switch control need a dedicated driver to handle, on DT platforms, now mux-controller is the only proposed way to go from binding point view. I am not sure if my case is a normal HW design, but I guess I should not the only user of this kind of situation. > > There are actually also some technical reasons why Hans failed to get the > mux-controller thing to work, which is the original reason why we introduced > the dedicated framework for the Type-C "muxes" (I really should stop talking > about muxes), but I don't remember what was the reason. I checked the patches Hans did, that was mainly to address non-DT platform, I don't see a clear reason why it can't fit DT platform, maybe I missed something. +Hans, It would be great if you can comment on this, thanks. > > In any case, to summarise: the orientation switch is a function. A mux is > a device that can supply that function, and if it does, then the driver for > it really needs to register the dedicated orientation switch. Understand your point, if register the dedicated orientation switch is a must, I feel using general mux control can't make much sense. Thanks Li Jun > > thanks, > > -- > heikki _______________________________________________ linux-arm-kernel mailing list email@example.com http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-05-31 12:00 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-19 7:14 [PATCH 0/4] typec switch " Li Jun 2021-05-19 7:14 ` [PATCH 1/4] dt-bindings: connector: Add typec orientation switch properties Li Jun 2021-05-21 1:30 ` Rob Herring 2021-05-25 11:48 ` Jun Li 2021-05-19 7:14 ` [PATCH 2/4] usb: typec: use typec cap fwnode's of_node for typec port Li Jun 2021-05-20 12:38 ` Heikki Krogerus 2021-05-19 7:14 ` [PATCH 3/4] usb: typec: add typec orientation switch support via mux controller Li Jun 2021-05-20 12:33 ` Heikki Krogerus 2021-05-21 8:37 ` Heikki Krogerus 2021-05-25 11:46 ` Jun Li 2021-05-26 9:16 ` Heikki Krogerus 2021-05-31 11:58 ` Jun Li [this message] 2021-05-21 13:02 ` Jun Li 2021-05-19 7:14 ` [PATCH 4/4] arm64: dts: imx8mp-evk: enable usb0 with typec connector Li Jun
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=VI1PR04MB5935CF87B5DB6BC62BD69B10893F9@VI1PR04MB5935.eurprd04.prod.outlook.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='RE: [PATCH 3/4] usb: typec: add typec orientation switch support via mux controller' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).