| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20140725081349.GB9756@kwain>
Date: Fri, 25 Jul 2014 10:13:49 +0200
From: Antoine Ténart
<antoine.tenart@...e-electrons.com>
To: Peter Chen <Peter.Chen@...escale.com>
Cc: Antoine Ténart
<antoine.tenart@...e-electrons.com>, "balbi@...com" <balbi@...com>,
"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
"kishon@...com" <kishon@...com>,
"stern@...land.harvard.edu" <stern@...land.harvard.edu>,
"sergei.shtylyov@...entembedded.com"
<sergei.shtylyov@...entembedded.com>,
"yoshihiro.shimoda.uh@...esas.com" <yoshihiro.shimoda.uh@...esas.com>,
"alexandre.belloni@...e-electrons.com"
<alexandre.belloni@...e-electrons.com>,
"thomas.petazzoni@...e-electrons.com"
<thomas.petazzoni@...e-electrons.com>,
"zmxu@...vell.com" <zmxu@...vell.com>,
"jszhang@...vell.com" <jszhang@...vell.com>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 8/8] usb: chipidea: add support to the generic PHY
framework in ChipIdea
Hi Peter,
On Fri, Jul 25, 2014 at 12:34:39AM +0000, Peter Chen wrote:
>
> > > >
> > > > - if (ci->platdata->usb_phy)
> > > > + if (ci->platdata->phy)
> > > > + ci->phy = ci->platdata->phy;
> > > > + else if (ci->platdata->usb_phy)
> > > > ci->usb_phy = ci->platdata->usb_phy;
> > > > else
> > > > - ci->usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> > > > + ci->phy = of_phy_get(dev->of_node, 0);
> > >
> > > Here, we may need to consider both usb_phy and generic_phy, and the
> > > core device is not populated from device tree.
> > >
> > > You can use devm_usb_get_phy and devm_phy_get for global phy case.
> >
> > This is the case. If no PHY is found by of_phy_get() we try to get an USB
> > PHY by calling devm_usb_get_phy().
> >
>
> Like I said above, the core device is not populated from DT, so the DT version
> phy API may not be needed.
I see, I'll switch to devm_usb_get_phy() and devm_phy_get().
> >
> > > > @@ -85,13 +88,16 @@ static int host_start(struct ci_hdrc *ci)
> > > > if (ret) {
> > > > goto disable_reg;
> > > > } else {
> > > > - struct usb_otg *otg = ci->usb_phy->otg;
> > > > + if (ci->usb_phy) {
> > > > + struct usb_otg *otg = ci->usb_phy->otg;
> > > >
> > > > - ci->hcd = hcd;
> > > > - if (otg) {
> > > > - otg->host = &hcd->self;
> > > > - hcd->self.otg_port = 1;
> > > > + if (otg) {
> > > > + otg->host = &hcd->self;
> > > > + hcd->self.otg_port = 1;
> > > > + }
> > > > }
> > >
> > > The otg port is still existed even use generic phy.
> >
> > Yes. I don't know how to retrieve the OTG pointer when we do not use an
> > USB PHY. We may have to add an OTG member into the ci_hdrc structure, but
> > I didn't understand well where does occur this OTG setup.
> >
> > I may have missed something. Do you know if CI OTG always is in FSM mode?
> > If so we could access the OTG pointer through ci_hdrc->fsm.
> >
>
> Just add OTG member into ci_hdrc structure, see below patch
>
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 9563cb5..186fb3f 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -207,6 +207,7 @@ struct ci_hdrc {
> bool id_event;
> bool b_sess_valid_event;
> bool imx28_write_fix;
> + struct usb_otg otg;
> };
>
> static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index a93d950..70d7fe2 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -85,7 +85,7 @@ static int host_start(struct ci_hdrc *ci)
> if (ret) {
> goto disable_reg;
> } else {
> - struct usb_otg *otg = ci->transceiver->otg;
> + struct usb_otg *otg = &ci->otg;
>
> ci->hcd = hcd;
> if (otg) {
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 8cb2508..fc8847e 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -778,20 +778,8 @@ void ci_hdrc_otg_fsm_start(struct ci_hdrc *ci)
> int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
> {
> int retval = 0;
> - struct usb_otg *otg;
>
> - otg = devm_kzalloc(ci->dev,
> - sizeof(struct usb_otg), GFP_KERNEL);
> - if (!otg) {
> - dev_err(ci->dev,
> - "Failed to allocate usb_otg structure for ci hdrc otg!\n");
> - return -ENOMEM;
> - }
> -
> - otg->phy = ci->transceiver;
> - otg->gadget = &ci->gadget;
> - ci->fsm.otg = otg;
> - ci->transceiver->otg = ci->fsm.otg;
> + ci->fsm.otg = &ci->otg;
> ci->fsm.power_up = 1;
> ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
> ci->fsm.otg->state = OTG_STATE_UNDEFINED;
> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
> index 33d3480..b2f7587 100644
> --- a/include/linux/usb/otg.h
> +++ b/include/linux/usb/otg.h
> @@ -14,7 +14,6 @@
> struct usb_otg {
> u8 default_a;
>
> - struct usb_phy *phy;
> struct usb_bus *host;
> struct usb_gadget *gadget;
>
> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> index ac7d791..b732541 100644
> --- a/include/linux/usb/phy.h
> +++ b/include/linux/usb/phy.h
> @@ -79,8 +79,6 @@ struct usb_phy {
> enum usb_phy_type type;
> enum usb_phy_events last_event;
>
> - struct usb_otg *otg;
> -
> struct device *io_dev;
> struct usb_phy_io_ops *io_ops;
> void __iomem *io_priv;
> b29397@...inux1:~/work/projects/upstream/usb/usb$ git diff > a.diff
> b29397@...inux1:~/work/projects/upstream/usb/usb$ cat a.diff
> diff --git a/drivers/usb/chipidea/ci.h b/drivers/usb/chipidea/ci.h
> index 9563cb5..186fb3f 100644
> --- a/drivers/usb/chipidea/ci.h
> +++ b/drivers/usb/chipidea/ci.h
> @@ -207,6 +207,7 @@ struct ci_hdrc {
> bool id_event;
> bool b_sess_valid_event;
> bool imx28_write_fix;
> + struct usb_otg otg;
> };
>
> static inline struct ci_role_driver *ci_role(struct ci_hdrc *ci)
> diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> index a93d950..70d7fe2 100644
> --- a/drivers/usb/chipidea/host.c
> +++ b/drivers/usb/chipidea/host.c
> @@ -85,7 +85,7 @@ static int host_start(struct ci_hdrc *ci)
> if (ret) {
> goto disable_reg;
> } else {
> - struct usb_otg *otg = ci->transceiver->otg;
> + struct usb_otg *otg = &ci->otg;
>
> ci->hcd = hcd;
> if (otg) {
> diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
> index 8cb2508..fc8847e 100644
> --- a/drivers/usb/chipidea/otg_fsm.c
> +++ b/drivers/usb/chipidea/otg_fsm.c
> @@ -778,20 +778,8 @@ void ci_hdrc_otg_fsm_start(struct ci_hdrc *ci)
> int ci_hdrc_otg_fsm_init(struct ci_hdrc *ci)
> {
> int retval = 0;
> - struct usb_otg *otg;
>
> - otg = devm_kzalloc(ci->dev,
> - sizeof(struct usb_otg), GFP_KERNEL);
> - if (!otg) {
> - dev_err(ci->dev,
> - "Failed to allocate usb_otg structure for ci hdrc otg!\n");
> - return -ENOMEM;
> - }
> -
> - otg->phy = ci->transceiver;
> - otg->gadget = &ci->gadget;
> - ci->fsm.otg = otg;
> - ci->transceiver->otg = ci->fsm.otg;
> + ci->fsm.otg = &ci->otg;
> ci->fsm.power_up = 1;
> ci->fsm.id = hw_read_otgsc(ci, OTGSC_ID) ? 1 : 0;
> ci->fsm.otg->state = OTG_STATE_UNDEFINED;
> diff --git a/include/linux/usb/otg.h b/include/linux/usb/otg.h
> index 33d3480..b2f7587 100644
> --- a/include/linux/usb/otg.h
> +++ b/include/linux/usb/otg.h
> @@ -14,7 +14,6 @@
> struct usb_otg {
> u8 default_a;
>
> - struct usb_phy *phy;
> struct usb_bus *host;
> struct usb_gadget *gadget;
>
> diff --git a/include/linux/usb/phy.h b/include/linux/usb/phy.h
> index ac7d791..b732541 100644
> --- a/include/linux/usb/phy.h
> +++ b/include/linux/usb/phy.h
> @@ -79,8 +79,6 @@ struct usb_phy {
> enum usb_phy_type type;
> enum usb_phy_events last_event;
>
> - struct usb_otg *otg;
> -
> struct device *io_dev;
> struct usb_phy_io_ops *io_ops;
> void __iomem *io_priv;
Where does the ci->otg setup occurs? It seems to me it's never filled.
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists