USB: mxs-phy: add basic otg support

PATCH Superseded Richard Zhao 4 participants 7 messages Sep 17, 2012 – Sep 19, 2012

Subsystems (2)

USB Arm i.MX and Layerscape

lore

Share & tools

lore

Richard Zhao

author

2012-09-17 09:05 UTC #0 lore raw

PATCH v3 1/1 USB: mxs-phy: add basic otg support

Signed-off-by Richard Zhao <[email protected]>

Acked-by Felipe Balbi <[email protected]>

---
Changes from v2:
- assign host/gadget in mxs_phy_set_host/mxs_phy_set_peripheral

 drivers/usb/otg/mxs-phy.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

drivers/usb/otg/mxs-phy.c +25 -0

index 88db976..3255112 100644
@@ -129,12 +129,28 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy, int port)
 	return 0;
 }
 
+static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+	otg->host = host;
+
+	return 0;
+}
+
+static int mxs_phy_set_peripheral(struct usb_otg *otg,
+					struct usb_gadget *gadget)
+{
+	otg->gadget = gadget;
+
+	return 0;
+}
+
 static int mxs_phy_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
+	struct usb_otg *otg;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {

@@ -171,6 +187,15 @@ static int mxs_phy_probe(struct platform_device *pdev)
 
 	mxs_phy->clk = clk;
 
+	otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
+	if (!otg)
+		return -ENOMEM;
+	otg->phy = &mxs_phy->phy;
+	otg->set_host = mxs_phy_set_host;
+	otg->set_peripheral = mxs_phy_set_peripheral;
+
+	mxs_phy->phy.otg = otg;
+
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
 	return 0;

Signature · 1 line

1.7.9.5

Alexander Shishkin

2012-09-17 10:06 UTC #1 lore raw

PATCH v3 1/1 USB: mxs-phy: add basic otg support

Quoted 2 lines · Richard Zhao

Signed-off-by Richard Zhao <[email protected]>
Acked-by Felipe Balbi <[email protected]>

Felipe said,

Quoted 3 lines

if you add a commit log you can add my:
Acked-by Felipe Balbi <[email protected]>

but the commit message is still totally missing. I would like to ask you to pay attention to the commit messages in the patches that you submit. They should explain the problem that your patch is solving, how you are solving it and why, so that anyone immediately knows what the patch is about without digging up mailing list conversations. There's also a nice blog entry [1] on how to write good commit messages.

[1] http://who-t.blogspot.com/2009/12/on-commit-messages.html

Quoted 58 lines

---
Changes from v2:
- assign host/gadget in mxs_phy_set_host/mxs_phy_set_peripheral

 drivers/usb/otg/mxs-phy.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

drivers/usb/otg/mxs-phy.c +25 -0

index 88db976..3255112 100644
@@ -129,12 +129,28 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy, int port)
 	return 0;
 }
 
+static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+	otg->host = host;
+
+	return 0;
+}
+
+static int mxs_phy_set_peripheral(struct usb_otg *otg,
+					struct usb_gadget *gadget)
+{
+	otg->gadget = gadget;
+
+	return 0;
+}
+
 static int mxs_phy_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
+	struct usb_otg *otg;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {

@@ -171,6 +187,15 @@ static int mxs_phy_probe(struct platform_device *pdev)
 
 	mxs_phy->clk = clk;
 
+	otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
+	if (!otg)
+		return -ENOMEM;
+	otg->phy = &mxs_phy->phy;
+	otg->set_host = mxs_phy_set_host;
+	otg->set_peripheral = mxs_phy_set_peripheral;
+
+	mxs_phy->phy.otg = otg;
+
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
 	return 0;

Signature · 1 line

1.7.9.5

Richard Zhao

author

2012-09-19 01:11 UTC #2 lore raw

PATCH v3 1/1 USB: mxs-phy: add basic otg support

Quoted 11 lines · Alexander Shishkin

Quoted 2 lines · Richard Zhao
Signed-off-by Richard Zhao <[email protected]>
Acked-by Felipe Balbi <[email protected]>
Felipe said,
Quoted 3 lines
if you add a commit log you can add my:
Acked-by Felipe Balbi <[email protected]>

Quoted 8 lines

but the commit message is still totally missing. I would like to ask you to pay attention to the commit messages in the patches that you submit. They should explain the problem that your patch is solving, how you are solving it and why, so that anyone immediately knows what the patch is about without digging up mailing list conversations. There's also a nice blog entry [1] on how to write good commit messages.
[1] http://who-t.blogspot.com/2009/12/on-commit-messages.html

Will you agree to give ack if commit message changed to below?

USB: mxs-phy: add fake otg support

It has nothing to do with phy driver, but for now chipidea driver calls struct usb_otg callbacks. So we implement a fake one.

When chipidea implement its own otg driver, this commit can be reverted.

Signed-off-by Richard Zhao <[email protected]>

Acked-by Felipe Balbi <[email protected]

Thanks
Richard

Quoted 60 lines

Quoted 59 lines

---
Changes from v2:
- assign host/gadget in mxs_phy_set_host/mxs_phy_set_peripheral

 drivers/usb/otg/mxs-phy.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

drivers/usb/otg/mxs-phy.c +25 -0

index 88db976..3255112 100644
@@ -129,12 +129,28 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy, int port)
 	return 0;
 }
 
+static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+	otg->host = host;
+
+	return 0;
+}
+
+static int mxs_phy_set_peripheral(struct usb_otg *otg,
+					struct usb_gadget *gadget)
+{
+	otg->gadget = gadget;
+
+	return 0;
+}
+
 static int mxs_phy_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
+	struct usb_otg *otg;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {

@@ -171,6 +187,15 @@ static int mxs_phy_probe(struct platform_device *pdev)
 
 	mxs_phy->clk = clk;
 
+	otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
+	if (!otg)
+		return -ENOMEM;
+	otg->phy = &mxs_phy->phy;
+	otg->set_host = mxs_phy_set_host;
+	otg->set_peripheral = mxs_phy_set_peripheral;
+
+	mxs_phy->phy.otg = otg;
+
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
 	return 0;

Signature · 1 line

1.7.9.5

Marc Kleine-Budde

2012-09-19 07:27 UTC #3 lore raw

PATCH v3 1/1 USB: mxs-phy: add basic otg support

Quoted 73 lines · Richard Zhao

Quoted 11 lines · Alexander Shishkin
Quoted 2 lines · Richard Zhao
Signed-off-by Richard Zhao <[email protected]>
Acked-by Felipe Balbi <[email protected]>
Felipe said,
Quoted 3 lines
if you add a commit log you can add my:
Acked-by Felipe Balbi <[email protected]>
Quoted 8 lines
but the commit message is still totally missing. I would like to ask you to pay attention to the commit messages in the patches that you submit. They should explain the problem that your patch is solving, how you are solving it and why, so that anyone immediately knows what the patch is about without digging up mailing list conversations. There's also a nice blog entry [1] on how to write good commit messages.
[1] http://who-t.blogspot.com/2009/12/on-commit-messages.html
Will you agree to give ack if commit message changed to below?
USB: mxs-phy: add fake otg support
It has nothing to do with phy driver, but for now chipidea driver calls struct usb_otg callbacks. So we implement a fake one.
When chipidea implement its own otg driver, this commit can be reverted.
Signed-off-by Richard Zhao <[email protected]>
Acked-by Felipe Balbi <[email protected]
Thanks
Richard
Quoted 39 lines
Quoted 39 lines
---
Changes from v2:
- assign host/gadget in mxs_phy_set_host/mxs_phy_set_peripheral
 drivers/usb/otg/mxs-phy.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
drivers/usb/otg/mxs-phy.c +16 -0
index 88db976..3255112 100644
@@ -129,12 +129,28 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy, int port)
 	return 0;
 }
 
+static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+	otg->host = host;
+
+	return 0;
+}
+
+static int mxs_phy_set_peripheral(struct usb_otg *otg,
+					struct usb_gadget *gadget)
+{
+	otg->gadget = gadget;
+
+	return 0;
+}
+
 static int mxs_phy_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
+	struct usb_otg *otg;
 

Quoted 7 lines

Quoted 7 lines
drivers/usb/otg/mxs-phy.c static int mxs_phy_probe(struct platform_device *pdev) · line 190
Quoted 7 lines
drivers/usb/otg/mxs-phy.c +1 -0
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!res) {
@@ -171,6 +187,15 @@ static int mxs_phy_probe(struct platform_device *pdev)
 
 	mxs_phy->clk = clk;
 
+	otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);

In order to save some overhead you can embed the struct usb_otg in the struct mxs_phy.

Marc

Quoted 15 lines

Quoted 14 lines

Quoted 13 lines

drivers/usb/otg/mxs-phy.c +8 -0

+	if (!otg)
+		return -ENOMEM;
+	otg->phy = &mxs_phy->phy;
+	otg->set_host = mxs_phy_set_host;
+	otg->set_peripheral = mxs_phy_set_peripheral;
+
+	mxs_phy->phy.otg = otg;
+
 	platform_set_drvdata(pdev, &mxs_phy->phy);
 
 	return 0;

Signature · 1 line

1.7.9.5

Signature · 12 lines

Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 259 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120919/170e54b2/attachment.sig>

Chen Peter-B29397

2012-09-17 10:08 UTC #4 lore raw

PATCH v3 1/1 USB: mxs-phy: add basic otg support

In reply to Richard Zhao

The purpose for this patch is to have set_peripheral implementation, but in current chipidea code, we don't need to know gadget at all, as when id switch occurs, the core code know its role (device or host) very well, and will call related stop/start function.

We have already many code bind struct otg with struct phy, we'd better split them at later code.

A better approach to fix this problem is to either not call set_peripheral if both device and host use chipidea driver, or implement otg struct at chipidea driver.

Quoted 59 lines

Changes from v2:
- assign host/gadget in mxs_phy_set_host/mxs_phy_set_peripheral

 drivers/usb/otg/mxs-phy.c |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

drivers/usb/otg/mxs-phy.c +16 -0

index 88db976..3255112 100644
@@ -129,12 +129,28 @@ static int mxs_phy_on_disconnect(struct usb_phy
 *phy, int port)
 	return 0;
 }
 
+static int mxs_phy_set_host(struct usb_otg *otg, struct usb_bus *host)
+{
+	otg->host = host;
+
+	return 0;
+}
+
+static int mxs_phy_set_peripheral(struct usb_otg *otg,
+					struct usb_gadget *gadget)
+{
+	otg->gadget = gadget;
+
+	return 0;
+}
+
 static int mxs_phy_probe(struct platform_device *pdev)
 {
 	struct resource *res;
 	void __iomem *base;
 	struct clk *clk;
 	struct mxs_phy *mxs_phy;
+	struct usb_otg *otg;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);

drivers/usb/otg/mxs-phy.c +9 -0

 	if (!res) {

@@ -171,6 +187,15 @@ static int mxs_phy_probe(struct platform_device
 *pdev)
 
 	mxs_phy->clk = clk;
 
+	otg = devm_kzalloc(&pdev->dev, sizeof(struct usb_otg), GFP_KERNEL);
+	if (!otg)
+		return -ENOMEM;
+	otg->phy = &mxs_phy->phy;
+	otg->set_host = mxs_phy_set_host;
+	otg->set_peripheral = mxs_phy_set_peripheral;
+
+	mxs_phy->phy.otg = otg;
+
 	platform_set_drvdata(pdev, &mxs_phy->phy);

return 0;
--
1.7.9.5

Alexander Shishkin

2012-09-17 10:18 UTC #5 lore raw

PATCH v3 1/1 USB: mxs-phy: add basic otg support

Quoted 11 lines · Chen Peter-B29397 <[email protected]> writes:

The purpose for this patch is to have set_peripheral implementation, but in current chipidea code, we don't need to know gadget at all, as when id switch occurs, the core code know its role (device or host) very well, and will call related stop/start function.
We have already many code bind struct otg with struct phy, we'd better split them at later code.
A better approach to fix this problem is to either not call set_peripheral if both device and host use chipidea driver, or implement otg struct at chipidea driver.

Yes, I think the otg driver should be part of the chipidea driver, the only concern is the msm, although it can probably be converted once we have an otg driver. I'll have a look at it soon unless someone beats me to it.

The bigger plan was to implement a generic otg framework and base the chipidea's otg driver on that, instead of dragging in one more state machine and whatnot.

Regards,
--
Alex

Chen Peter-B29397

2012-09-17 23:39 UTC #6 lore raw

PATCH v3 1/1 USB: mxs-phy: add basic otg support

Quoted 11 lines

Quoted 11 lines
The purpose for this patch is to have set_peripheral implementation, but in current chipidea code, we don't need to know gadget at all, as when id switch occurs, the core code know its role (device or host) very well, and will call related stop/start function.
We have already many code bind struct otg with struct phy, we'd better split them at later code.
A better approach to fix this problem is to either not call

Quoted 8 lines

set_peripheral
Quoted 2 lines
if both device and host use chipidea driver, or implement otg struct at chipidea driver.
Yes, I think the otg driver should be part of the chipidea driver, the only concern is the msm, although it can probably be converted once we have an otg driver. I'll have a look at it soon unless someone beats me to it.

Currently, we still have no auto-suspend and wakeup support, but msm supports them, it may need much effort to move msm to current whole chipidea framework. As it affects we go on implement id-switch function and other usb functions at chipidea, I hope we can have it soon, thanks. I also would like to help it if you are busy on other things.

Quoted 5 lines

The bigger plan was to implement a generic otg framework and base the chipidea's otg driver on that, instead of dragging in one more state machine and whatnot.

Yes, we can transfer it to use generic otg framework once it is ready, but first, it is better has own otg driver at chipidea.

Quoted 3 lines

Regards,
--
Alex