From: balbi@ti.com (Felipe Balbi)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V2] USB: initialize or shutdown PHY when add or remove host controller
Date: Thu, 20 Jun 2013 15:17:49 +0300 [thread overview]
Message-ID: <20130620121749.GF9817@arwen.pp.htv.fi> (raw)
In-Reply-To: <CADApbej+qcvV2EmcB9g_k=DGp7R-1H_dEaA7StimMoR+G6z0Cg@mail.gmail.com>
Hi,
On Thu, Jun 20, 2013 at 08:53:05AM +0800, Chao Xie wrote:
> >> @@ -2674,6 +2693,9 @@ void usb_remove_hcd(struct usb_hcd *hcd)
> >> free_irq(hcd->irq, hcd);
> >> }
> >>
> >> + if (hcd->phy)
> >> + usb_phy_shutdown(hcd->phy);
> >> +
> >> usb_put_dev(hcd->self.root_hub);
> >> usb_deregister_bus(&hcd->self);
> >> hcd_buffer_destroy(hcd);
> >>
> >
> > I still think that we shouldn't do this because it adds more confusion and is not
> > still a generic enough solution.
> >
> > 1) It is better for the piece of code that does usb_phy_get() to do usb_phy_init/shutdown,
> > else there will be lot of confusion. (Felipe pointed this out earlier).
> >
> > 2) There is no standard way of getting phy for different controllers. It is mostly platform
> > dependent and it is best to leave this to the controller drivers. (Pointed out by Alan).
> >
> > 3) Controllers can have multiple PHYs. e.g. ehci-omap has one PHY per port and it supports
> > 3 ports. This is also platform specific and difficult to handle generically.
> >
> > 4) Controllers can have specific timing requirements as to when the PHY is initialized relative
> > to the controller being initialized. I've pointed OMAP specific stuff in the earlier patch.
> >
> > Considering all these points, I think we should leave things as they are. It isn't that hard
> > for controllers to manage phy_init() and phy_shutdown(), and there is not much code space saved
> > when compared to the complexity it creates if we move them to HCD layer.
> >
> In fact, the PHY setting and handling is related to platform or SOC,
> and for different SOC they can
> have same EHCI HCD but they PHY handling can be different.
> Omap'a case is the example, and i think some other vendors may have
> silimar cases.
> From above point, It is better to leave the PHY initialization and
> shutdown to be done by each echi-xxx driver.
>
> So Alan and Felipe
> What are your ideas about it?
If we have so many exceptions, then sure. But eventually, the common
case should be added generically with a flag so that non-generic cases
(like OMAP) can request to handle the PHY by themselves.
Alan ?
--
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130620/00677a79/attachment.sig>
WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi@ti.com>
To: Chao Xie <xiechao.mail@gmail.com>
Cc: Roger Quadros <rogerq@ti.com>, Chao Xie <chao.xie@marvell.com>,
Alan Stern <stern@rowland.harvard.edu>,
"balbi@ti.com" <balbi@ti.com>,
Greg KH <gregkh@linuxfoundation.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH V2] USB: initialize or shutdown PHY when add or remove host controller
Date: Thu, 20 Jun 2013 15:17:49 +0300 [thread overview]
Message-ID: <20130620121749.GF9817@arwen.pp.htv.fi> (raw)
In-Reply-To: <CADApbej+qcvV2EmcB9g_k=DGp7R-1H_dEaA7StimMoR+G6z0Cg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2259 bytes --]
Hi,
On Thu, Jun 20, 2013 at 08:53:05AM +0800, Chao Xie wrote:
> >> @@ -2674,6 +2693,9 @@ void usb_remove_hcd(struct usb_hcd *hcd)
> >> free_irq(hcd->irq, hcd);
> >> }
> >>
> >> + if (hcd->phy)
> >> + usb_phy_shutdown(hcd->phy);
> >> +
> >> usb_put_dev(hcd->self.root_hub);
> >> usb_deregister_bus(&hcd->self);
> >> hcd_buffer_destroy(hcd);
> >>
> >
> > I still think that we shouldn't do this because it adds more confusion and is not
> > still a generic enough solution.
> >
> > 1) It is better for the piece of code that does usb_phy_get() to do usb_phy_init/shutdown,
> > else there will be lot of confusion. (Felipe pointed this out earlier).
> >
> > 2) There is no standard way of getting phy for different controllers. It is mostly platform
> > dependent and it is best to leave this to the controller drivers. (Pointed out by Alan).
> >
> > 3) Controllers can have multiple PHYs. e.g. ehci-omap has one PHY per port and it supports
> > 3 ports. This is also platform specific and difficult to handle generically.
> >
> > 4) Controllers can have specific timing requirements as to when the PHY is initialized relative
> > to the controller being initialized. I've pointed OMAP specific stuff in the earlier patch.
> >
> > Considering all these points, I think we should leave things as they are. It isn't that hard
> > for controllers to manage phy_init() and phy_shutdown(), and there is not much code space saved
> > when compared to the complexity it creates if we move them to HCD layer.
> >
> In fact, the PHY setting and handling is related to platform or SOC,
> and for different SOC they can
> have same EHCI HCD but they PHY handling can be different.
> Omap'a case is the example, and i think some other vendors may have
> silimar cases.
> From above point, It is better to leave the PHY initialization and
> shutdown to be done by each echi-xxx driver.
>
> So Alan and Felipe
> What are your ideas about it?
If we have so many exceptions, then sure. But eventually, the common
case should be added generically with a flag so that non-generic cases
(like OMAP) can request to handle the PHY by themselves.
Alan ?
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
next prev parent reply other threads:[~2013-06-20 12:17 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-19 2:31 [PATCH V2] USB: initialize or shutdown PHY when add or remove host controller Chao Xie
2013-06-19 2:31 ` Chao Xie
2013-06-19 2:48 ` Greg KH
2013-06-19 2:48 ` Greg KH
2013-06-19 3:23 ` Chao Xie
2013-06-19 3:23 ` Chao Xie
2013-06-19 7:51 ` Roger Quadros
2013-06-19 7:51 ` Roger Quadros
2013-06-20 0:53 ` Chao Xie
2013-06-20 0:53 ` Chao Xie
2013-06-20 12:17 ` Felipe Balbi [this message]
2013-06-20 12:17 ` Felipe Balbi
2013-06-20 17:25 ` Alan Stern
2013-06-20 17:25 ` Alan Stern
2013-06-21 1:07 ` Chao Xie
2013-06-21 1:07 ` Chao Xie
2013-06-21 1:27 ` Chao Xie
2013-06-21 1:27 ` Chao Xie
2013-06-24 19:45 ` Felipe Balbi
2013-06-24 19:45 ` Felipe Balbi
2013-06-25 1:25 ` Chao Xie
2013-06-25 1:25 ` Chao Xie
2013-06-25 3:33 ` Felipe Balbi
2013-06-25 3:33 ` Felipe Balbi
2013-06-24 19:36 ` Felipe Balbi
2013-06-24 19:36 ` Felipe Balbi
2013-06-25 13:37 ` Roger Quadros
2013-06-25 13:37 ` Roger Quadros
2013-06-25 13:43 ` Felipe Balbi
2013-06-25 13:43 ` Felipe Balbi
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=20130620121749.GF9817@arwen.pp.htv.fi \
--to=balbi@ti.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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
Be 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.