From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> To: Vivek Gautam <gautam.vivek@samsung.com> Cc: Julius Werner <jwerner@chromium.org>, "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>, "linux-samsung-soc@vger.kernel.org" <linux-samsung-soc@vger.kernel.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, kishon <kishon@ti.com>, Mathias Nyman <mathias.nyman@intel.com>, LKML <linux-kernel@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org>, Kukjin Kim <kgene.kim@samsung.com> Subject: Re: [PATCH 2/4] usb: host: xhci-plat: Add support to get PHYs Date: Fri, 04 Jul 2014 02:39:53 +0400 [thread overview] Message-ID: <53B5DBB9.3080202@cogentembedded.com> (raw) In-Reply-To: <CAFp+6iGYZdRL17UqPhskyDDhfD++His3mO2v7uzG5OF1Hd3TrQ@mail.gmail.com> Hello. On 06/25/2014 12:44 PM, Vivek Gautam wrote: >>>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h >>>> index 9ffecd5..453d89e 100644 >>>> --- a/drivers/usb/host/xhci.h >>>> +++ b/drivers/usb/host/xhci.h >>>> @@ -1582,6 +1582,9 @@ struct xhci_hcd { >>>> u32 port_status_u0; >>>> /* Compliance Mode Timer Triggered every 2 seconds */ >>>> #define COMP_MODE_RCVRY_MSECS 2000 >>>> + /* phys for the controller */ >>>> + struct phy *phy2_gen; >>>> + struct phy *phy3_gen; >>>> }; >>> I don't think adding new variables here and restricting most of this >>> logic to xhci-plat.c (in the next patch) is the best way to do it. >> Indeed. Actually, I haven't given enough thought to this... >>> There's no conceptual reason why other host controllers (e.g. xhci-pci >>> or even EHCI) could not have a similar need to tune their PHY after >>> reset. PHYs are universal to all host controllers. >>> There is already a 'phy' member in struct usb_hcd which I think is >>> mostly unused right now. I think it would be much less >>> confusing/redundant to reuse that member for this purpose (you could >>> still set it up from xhci_plat_probe(), and then call it from >>> hcd_bus_resume() or something like that). >> That member has type 'struct usb_phy *' while here we have 'struct phy *' >> -- feel the difference. >> I have already tried adding 'struct phy *gen_phy' to 'struct usb_hcd', > So the 'struct phy *' available in the usb_hcd is requested in usb_add_hcd(). > This is requested with the constant string 'usb' : > struct phy *phy = phy_get(hcd->self.controller, "usb"); > This can get the phy with string 'usb' only if, either the host > controller has a device node wherein the phys are given. Yes, that was the plan. Currently, the PHY driver for which this was written can be probed from the device tree only. > Even in this case one can't give same constant string for two > different phys, UTMI+ and PIPE3 phy, isn't it ? Yes, you're right. I didn't really consider the case of some host needing 2 distinct PHY. > Or, the other way can be when host gets a lookup table to look into to > find the relevant phys, something like > Heikki has suggested: > usb: dwc3: host: convey the PHYs to xhci > (https://lkml.org/lkml/2014/6/5/585) and related patch series. Well, AFAIK the look-up table method is already provided by the current PHY core for non-DT use cases; this doesn't seem to have changed with that patch set, does it? > So if we use this second approach, we would need to override the > 'phy_get()' that has been done in usb_add_hcd() > in xhci_plat_probe(), and then use them in later operations. I guess it'd probably be simpler to ignore the 'gen_phy' member of 'struct usb_hcd' in your case (if it gets eventually added). WBR, Sergei
WARNING: multiple messages have this Message-ID (diff)
From: sergei.shtylyov@cogentembedded.com (Sergei Shtylyov) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 2/4] usb: host: xhci-plat: Add support to get PHYs Date: Fri, 04 Jul 2014 02:39:53 +0400 [thread overview] Message-ID: <53B5DBB9.3080202@cogentembedded.com> (raw) In-Reply-To: <CAFp+6iGYZdRL17UqPhskyDDhfD++His3mO2v7uzG5OF1Hd3TrQ@mail.gmail.com> Hello. On 06/25/2014 12:44 PM, Vivek Gautam wrote: >>>> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h >>>> index 9ffecd5..453d89e 100644 >>>> --- a/drivers/usb/host/xhci.h >>>> +++ b/drivers/usb/host/xhci.h >>>> @@ -1582,6 +1582,9 @@ struct xhci_hcd { >>>> u32 port_status_u0; >>>> /* Compliance Mode Timer Triggered every 2 seconds */ >>>> #define COMP_MODE_RCVRY_MSECS 2000 >>>> + /* phys for the controller */ >>>> + struct phy *phy2_gen; >>>> + struct phy *phy3_gen; >>>> }; >>> I don't think adding new variables here and restricting most of this >>> logic to xhci-plat.c (in the next patch) is the best way to do it. >> Indeed. Actually, I haven't given enough thought to this... >>> There's no conceptual reason why other host controllers (e.g. xhci-pci >>> or even EHCI) could not have a similar need to tune their PHY after >>> reset. PHYs are universal to all host controllers. >>> There is already a 'phy' member in struct usb_hcd which I think is >>> mostly unused right now. I think it would be much less >>> confusing/redundant to reuse that member for this purpose (you could >>> still set it up from xhci_plat_probe(), and then call it from >>> hcd_bus_resume() or something like that). >> That member has type 'struct usb_phy *' while here we have 'struct phy *' >> -- feel the difference. >> I have already tried adding 'struct phy *gen_phy' to 'struct usb_hcd', > So the 'struct phy *' available in the usb_hcd is requested in usb_add_hcd(). > This is requested with the constant string 'usb' : > struct phy *phy = phy_get(hcd->self.controller, "usb"); > This can get the phy with string 'usb' only if, either the host > controller has a device node wherein the phys are given. Yes, that was the plan. Currently, the PHY driver for which this was written can be probed from the device tree only. > Even in this case one can't give same constant string for two > different phys, UTMI+ and PIPE3 phy, isn't it ? Yes, you're right. I didn't really consider the case of some host needing 2 distinct PHY. > Or, the other way can be when host gets a lookup table to look into to > find the relevant phys, something like > Heikki has suggested: > usb: dwc3: host: convey the PHYs to xhci > (https://lkml.org/lkml/2014/6/5/585) and related patch series. Well, AFAIK the look-up table method is already provided by the current PHY core for non-DT use cases; this doesn't seem to have changed with that patch set, does it? > So if we use this second approach, we would need to override the > 'phy_get()' that has been done in usb_add_hcd() > in xhci_plat_probe(), and then use them in later operations. I guess it'd probably be simpler to ignore the 'gen_phy' member of 'struct usb_hcd' in your case (if it gets eventually added). WBR, Sergei
next prev parent reply other threads:[~2014-07-03 22:39 UTC|newest] Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top 2014-06-06 12:12 [PATCH v1 0/4] Fine tune USB 3.0 PHY on exynos5420 Vivek Gautam 2014-06-06 12:12 ` Vivek Gautam 2014-06-06 12:12 ` Vivek Gautam 2014-06-06 12:12 ` [PATCH 1/4] phy: Add provision for calibrating phy Vivek Gautam 2014-06-06 12:12 ` Vivek Gautam 2014-06-09 3:49 ` Pratyush Anand 2014-06-09 3:49 ` Pratyush Anand 2014-06-09 3:49 ` Pratyush Anand 2014-07-09 9:02 ` Vivek Gautam 2014-07-09 9:02 ` Vivek Gautam 2014-07-09 9:02 ` Vivek Gautam 2014-06-06 12:12 ` [PATCH 2/4] usb: host: xhci-plat: Add support to get PHYs Vivek Gautam 2014-06-06 12:12 ` Vivek Gautam 2014-06-09 20:22 ` Julius Werner 2014-06-09 20:22 ` Julius Werner 2014-06-09 20:22 ` Julius Werner 2014-06-24 6:10 ` Vivek Gautam 2014-06-24 6:10 ` Vivek Gautam 2014-06-24 6:10 ` Vivek Gautam 2014-06-24 22:34 ` Sergei Shtylyov 2014-06-24 22:34 ` Sergei Shtylyov 2014-06-24 22:34 ` Sergei Shtylyov 2014-06-25 5:49 ` Vivek Gautam 2014-06-25 5:49 ` Vivek Gautam 2014-06-25 5:49 ` Vivek Gautam 2014-06-25 8:44 ` Vivek Gautam 2014-06-25 8:44 ` Vivek Gautam 2014-06-25 8:44 ` Vivek Gautam 2014-07-03 22:39 ` Sergei Shtylyov [this message] 2014-07-03 22:39 ` Sergei Shtylyov 2014-07-03 22:39 ` Sergei Shtylyov 2014-06-06 12:12 ` [PATCH 3/4] usb: host: xhci-plat: Caibrate PHY post host reset Vivek Gautam 2014-06-06 12:12 ` Vivek Gautam 2014-06-06 12:12 ` [PATCH 4/4] phy: exynos5-usbdrd: Calibrate LOS levels for exynos5420/5800 Vivek Gautam 2014-06-06 12:12 ` Vivek Gautam
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=53B5DBB9.3080202@cogentembedded.com \ --to=sergei.shtylyov@cogentembedded.com \ --cc=gautam.vivek@samsung.com \ --cc=gregkh@linuxfoundation.org \ --cc=jwerner@chromium.org \ --cc=kgene.kim@samsung.com \ --cc=kishon@ti.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-samsung-soc@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=mathias.nyman@intel.com \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.