[RFC] musb: removing otg protocol support

[RFC] musb: removing otg protocol support

[Bin Liu]

On Sun, Mar 18, 2018 at 05:36:10PM +0100, Hans de Goede wrote:
> Hi,
> 
> On 18-03-18 14:16, Greg Kroah-Hartman wrote:
> >On Fri, Mar 16, 2018 at 02:09:51PM -0500, Bin Liu wrote:
> >>Hi,
> >>
> >>The kernel usb stack and musb drivers have gone through some changes in
> >>the past several kernel versions, such as adding otg fsm, musb runtime
> >>PM, and musb otg state moving from musb to musb->xceiv... I am wondering
> >>if the otg protocol (hnp, srp) functions are already broken in the musb
> >>drivers, but I don't have a platform to confirm it.
> >>
> >>Do we know by any chance there is still someone using the musb otg
> >>functions in any relatively newer kernel and we still need to support
> >>otg in musb?  If not, I am thinking to clean up the otg functions in
> >>musb drivers to make the code easy to read and maintain.
> >
> >By "clean up" do you mean "delete it"?  :)
> >
> >I don't know of any real OTG hardware that ever shipped, does anyone
> >else?
> 
> I'm not aware of any hardware using the fancier otg functions such as
> role-swapping, etc. either. From an Allwinner pov as long as you keep

Thanks for confirming ;)

> plain gadget and host mode as controlled by the id pin work, I'm happy.

Yes, this is not changed.

> Note I'm not really involved in Allwinnner stuff anymore, so if you
> want some testing of the cleanup, you should probably ask the
> free-electron (now bootlin.com) people.

Thanks, but I am not sure how I can find the exact individual contects.

Regards,
-Bin.
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] musb: removing otg protocol support

[Bin Liu]

On Mon, Mar 19, 2018 at 03:40:48PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Mar 19, 2018 at 09:20:23AM -0500, Bin Liu wrote:
> > On Sun, Mar 18, 2018 at 02:16:25PM +0100, Greg Kroah-Hartman wrote:
> > > On Fri, Mar 16, 2018 at 02:09:51PM -0500, Bin Liu wrote:
> > > > Hi,
> > > > 
> > > > The kernel usb stack and musb drivers have gone through some changes in
> > > > the past several kernel versions, such as adding otg fsm, musb runtime
> > > > PM, and musb otg state moving from musb to musb->xceiv... I am wondering
> > > > if the otg protocol (hnp, srp) functions are already broken in the musb
> > > > drivers, but I don't have a platform to confirm it.
> > > > 
> > > > Do we know by any chance there is still someone using the musb otg
> > > > functions in any relatively newer kernel and we still need to support
> > > > otg in musb?  If not, I am thinking to clean up the otg functions in
> > > > musb drivers to make the code easy to read and maintain.
> > > 
> > > By "clean up" do you mean "delete it"?  :)
> > 
> > Yes, delete it to make the driver state machine simpler and use less
> > flags for recording states.
> > 
> > > 
> > > I don't know of any real OTG hardware that ever shipped, does anyone
> > > else?
> > > 
> > > > If we can make the conclusion to remove it, I propose the patch below
> > > > to disable musb otg first, then clean up the driver later if nobody
> > > > complains about the otg function removal.
> > > 
> > > It will take years for people who make these types of devices to notice
> > > that OTG is removed, so be careful about this.  Refactor away, but
> > 
> > I personally think we can safely say there wasn't any true OTG products
> > where were under development in the last several years, but I am more
> > concerned if there was any such product released in the past, for
> > example omap24xx/34xx based, but was still actively maintained to the
> > latest kernel? Then deleting OTG protocol from musb drivers would break
> > them.
> 
> Wasn't the BeagleBone devices using that chipset?  If not, then you are

No, BeagleBone uses TI AM335x devices which have two MUSB controllers,
and released in around 2012. And I don't think anyone uses the true OTG
on AM335x, but omap24xx/34xx devices are much older which I don't know
the history...

> probably fine.

Sounds good. I will send a patch in a few weeks to disable OTG in musb
for v3.18-rc1, too late for v4.17 now I think...

Regards,
-Bin.
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] musb: removing otg protocol support

[Greg Kroah-Hartman]

On Mon, Mar 19, 2018 at 09:20:23AM -0500, Bin Liu wrote:
> On Sun, Mar 18, 2018 at 02:16:25PM +0100, Greg Kroah-Hartman wrote:
> > On Fri, Mar 16, 2018 at 02:09:51PM -0500, Bin Liu wrote:
> > > Hi,
> > > 
> > > The kernel usb stack and musb drivers have gone through some changes in
> > > the past several kernel versions, such as adding otg fsm, musb runtime
> > > PM, and musb otg state moving from musb to musb->xceiv... I am wondering
> > > if the otg protocol (hnp, srp) functions are already broken in the musb
> > > drivers, but I don't have a platform to confirm it.
> > > 
> > > Do we know by any chance there is still someone using the musb otg
> > > functions in any relatively newer kernel and we still need to support
> > > otg in musb?  If not, I am thinking to clean up the otg functions in
> > > musb drivers to make the code easy to read and maintain.
> > 
> > By "clean up" do you mean "delete it"?  :)
> 
> Yes, delete it to make the driver state machine simpler and use less
> flags for recording states.
> 
> > 
> > I don't know of any real OTG hardware that ever shipped, does anyone
> > else?
> > 
> > > If we can make the conclusion to remove it, I propose the patch below
> > > to disable musb otg first, then clean up the driver later if nobody
> > > complains about the otg function removal.
> > 
> > It will take years for people who make these types of devices to notice
> > that OTG is removed, so be careful about this.  Refactor away, but
> 
> I personally think we can safely say there wasn't any true OTG products
> where were under development in the last several years, but I am more
> concerned if there was any such product released in the past, for
> example omap24xx/34xx based, but was still actively maintained to the
> latest kernel? Then deleting OTG protocol from musb drivers would break
> them.

Wasn't the BeagleBone devices using that chipset?  If not, then you are
probably fine.

thanks,

greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] musb: removing otg protocol support

[Bin Liu]

On Sun, Mar 18, 2018 at 02:16:25PM +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 16, 2018 at 02:09:51PM -0500, Bin Liu wrote:
> > Hi,
> > 
> > The kernel usb stack and musb drivers have gone through some changes in
> > the past several kernel versions, such as adding otg fsm, musb runtime
> > PM, and musb otg state moving from musb to musb->xceiv... I am wondering
> > if the otg protocol (hnp, srp) functions are already broken in the musb
> > drivers, but I don't have a platform to confirm it.
> > 
> > Do we know by any chance there is still someone using the musb otg
> > functions in any relatively newer kernel and we still need to support
> > otg in musb?  If not, I am thinking to clean up the otg functions in
> > musb drivers to make the code easy to read and maintain.
> 
> By "clean up" do you mean "delete it"?  :)

Yes, delete it to make the driver state machine simpler and use less
flags for recording states.

> 
> I don't know of any real OTG hardware that ever shipped, does anyone
> else?
> 
> > If we can make the conclusion to remove it, I propose the patch below
> > to disable musb otg first, then clean up the driver later if nobody
> > complains about the otg function removal.
> 
> It will take years for people who make these types of devices to notice
> that OTG is removed, so be careful about this.  Refactor away, but

I personally think we can safely say there wasn't any true OTG products
where were under development in the last several years, but I am more
concerned if there was any such product released in the past, for
example omap24xx/34xx based, but was still actively maintained to the
latest kernel? Then deleting OTG protocol from musb drivers would break
them.

> watch out you don't break someone's stuff and get told about it in 2
> years :(

My plan is to take two steps if we decided we can delete it.

First use the patch I copied in my first email to disable OTG protocols
support and wait for 2~3 quarters, we can simply revert it back if we
got yelled for any breakage ;)

Then *gradually* cleaning out any HNP/SRP related code through several
kernel versions (I don't think I have that much time to clean up all OTG
code within one kernel version anyway.)

If we break anyone in the second step, we can handle it as for any
typical regression, shouldn't be an issue.

Regards,
-Bin.
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] musb: removing otg protocol support

[Hans de Goede]

Hi,

On 18-03-18 14:16, Greg Kroah-Hartman wrote:
> On Fri, Mar 16, 2018 at 02:09:51PM -0500, Bin Liu wrote:
>> Hi,
>>
>> The kernel usb stack and musb drivers have gone through some changes in
>> the past several kernel versions, such as adding otg fsm, musb runtime
>> PM, and musb otg state moving from musb to musb->xceiv... I am wondering
>> if the otg protocol (hnp, srp) functions are already broken in the musb
>> drivers, but I don't have a platform to confirm it.
>>
>> Do we know by any chance there is still someone using the musb otg
>> functions in any relatively newer kernel and we still need to support
>> otg in musb?  If not, I am thinking to clean up the otg functions in
>> musb drivers to make the code easy to read and maintain.
> 
> By "clean up" do you mean "delete it"?  :)
> 
> I don't know of any real OTG hardware that ever shipped, does anyone
> else?

I'm not aware of any hardware using the fancier otg functions such as
role-swapping, etc. either. From an Allwinner pov as long as you keep
plain gadget and host mode as controlled by the id pin work, I'm happy.

Note I'm not really involved in Allwinnner stuff anymore, so if you
want some testing of the cleanup, you should probably ask the
free-electron (now bootlin.com) people.

Regards,

Hans
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] musb: removing otg protocol support

[Greg Kroah-Hartman]

On Fri, Mar 16, 2018 at 02:09:51PM -0500, Bin Liu wrote:
> Hi,
> 
> The kernel usb stack and musb drivers have gone through some changes in
> the past several kernel versions, such as adding otg fsm, musb runtime
> PM, and musb otg state moving from musb to musb->xceiv... I am wondering
> if the otg protocol (hnp, srp) functions are already broken in the musb
> drivers, but I don't have a platform to confirm it.
> 
> Do we know by any chance there is still someone using the musb otg
> functions in any relatively newer kernel and we still need to support
> otg in musb?  If not, I am thinking to clean up the otg functions in
> musb drivers to make the code easy to read and maintain.

By "clean up" do you mean "delete it"?  :)

I don't know of any real OTG hardware that ever shipped, does anyone
else?

> If we can make the conclusion to remove it, I propose the patch below
> to disable musb otg first, then clean up the driver later if nobody
> complains about the otg function removal.

It will take years for people who make these types of devices to notice
that OTG is removed, so be careful about this.  Refactor away, but
watch out you don't break someone's stuff and get told about it in 2
years :(

good luck!

greg k-h
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[RFC] musb: removing otg protocol support

[Bin Liu]

Hi,

The kernel usb stack and musb drivers have gone through some changes in
the past several kernel versions, such as adding otg fsm, musb runtime
PM, and musb otg state moving from musb to musb->xceiv... I am wondering
if the otg protocol (hnp, srp) functions are already broken in the musb
drivers, but I don't have a platform to confirm it.

Do we know by any chance there is still someone using the musb otg
functions in any relatively newer kernel and we still need to support
otg in musb?  If not, I am thinking to clean up the otg functions in
musb drivers to make the code easy to read and maintain.

If we can make the conclusion to remove it, I propose the patch below
to disable musb otg first, then clean up the driver later if nobody
complains about the otg function removal.

Regards,
-Bin.

8<---

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 86066328e480..4403b4cdd0b8 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1865,11 +1865,8 @@ int musb_gadget_setup(struct musb *musb)
 
 	/* this "gadget" abstracts/virtualizes the controller */
 	musb->g.name = musb_driver_name;
-#if IS_ENABLED(CONFIG_USB_MUSB_DUAL_ROLE)
-	musb->g.is_otg = 1;
-#elif IS_ENABLED(CONFIG_USB_MUSB_GADGET)
+	/* don't support otg protocols */
 	musb->g.is_otg = 0;
-#endif
 	INIT_DELAYED_WORK(&musb->gadget_work, musb_gadget_work);
 	musb_g_init_endpoints(musb);
 
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 069a1f775048..a3a5bd36ee98 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -2795,7 +2795,8 @@ int musb_host_setup(struct musb *musb, int power_budget)
 		musb->xceiv->otg->state = OTG_STATE_A_IDLE;
 	}
 	otg_set_host(musb->xceiv->otg, &hcd->self);
-	hcd->self.otg_port = 1;
+	/* don't support otg protocols */
+	hcd->self.otg_port = 0;
 	musb->xceiv->otg->host = &hcd->self;
 	hcd->power_budget = 2 * (power_budget ? : 250);