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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <76c0b6bc5b7f477998462d8b48d96272@BN1PR0301MB0772.namprd03.prod.outlook.com>
Date:	Fri, 25 Jul 2014 00:34:39 +0000
From:	Peter Chen <Peter.Chen@...escale.com>
To:	Antoine Ténart 
	<antoine.tenart@...e-electrons.com>
CC:	"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

 
> > >
> > > -	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. 

> >
> > >
> > > -	if (IS_ERR(ci->usb_phy)) {
> > > -		ret = PTR_ERR(ci->usb_phy);
> > > +	if (IS_ERR(ci->phy)) {
> > >  		/*
> > >  		 * if -ENXIO is returned, it means PHY layer wasn't
> > >  		 * enabled, so it makes no sense to return -EPROBE_DEFER
> > >  		 * in that case, since no PHY driver will ever probe.
> > >  		 */
> > > -		if (ret == -ENXIO)
> > > -			return ret;
> > > +		if (PTR_ERR(ci->phy) == -ENXIO)
> > > +			return -ENXIO;
> > >
> > > -		dev_err(dev, "no usb2 phy configured\n");
> > > -		return -EPROBE_DEFER;
> > > +		ci->usb_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
> > > +		if (IS_ERR(ci->usb_phy)) {
> > > +			dev_err(dev, "no usb2 phy configured\n");
> > > +			return -EPROBE_DEFER;
> > > +		}
> > >  	}
> 
> [snip]
> 
> > > diff --git a/drivers/usb/chipidea/debug.c
> > > b/drivers/usb/chipidea/debug.c index d47cddd38e4a..77881a5ce48f
> > > 100644
> > > --- a/drivers/usb/chipidea/debug.c
> > > +++ b/drivers/usb/chipidea/debug.c
> > > @@ -219,7 +219,9 @@ int ci_otg_show(struct seq_file *s, void *unused)
> > >  	fsm = &ci->fsm;
> > >
> > >  	/* ------ State ----- */
> > > -		usb_otg_state_string(ci->usb_phy->otg.state));
> > > +	if (ci->usb_phy)
> > > +		seq_printf(s, "OTG state: %s\n\n",
> > > +			usb_otg_state_string(ci->usb_phy->otg->state));
> >
> > You may change wrongly here.
> 
> I thought a pointer to the OTG info wasn't available from there. I just
> checked, and it seems we can retrieve if from the otg_fsm structure here.
> I'll test and update.
> 
> > > @@ -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;


Peter
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ