| 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: <20150911015040.GB17405@shlinux2>
Date: Fri, 11 Sep 2015 09:50:41 +0800
From: Peter Chen <peter.chen@...escale.com>
To: Roger Quadros <rogerq@...com>
CC: <stern@...land.harvard.edu>, <balbi@...com>,
<gregkh@...uxfoundation.org>, <dan.j.williams@...el.com>,
<jun.li@...escale.com>, <mathias.nyman@...ux.intel.com>,
<tony@...mide.com>, <Joao.Pinto@...opsys.com>,
<abrestic@...omium.org>, <linux-usb@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-omap@...r.kernel.org>
Subject: Re: [PATCH v4 07/13] usb: otg: add OTG core
On Thu, Sep 10, 2015 at 05:17:36PM +0300, Roger Quadros wrote:
> On 10/09/15 08:35, Peter Chen wrote:
> > On Wed, Sep 09, 2015 at 01:21:50PM +0300, Roger Quadros wrote:
> >> On 09/09/15 11:45, Peter Chen wrote:
> >>> On Wed, Sep 09, 2015 at 12:33:20PM +0300, Roger Quadros wrote:
> >>>> On 09/09/15 11:13, Peter Chen wrote:
> >>>>> On Wed, Sep 09, 2015 at 12:08:10PM +0300, Roger Quadros wrote:
> >>>>>> On 09/09/15 05:21, Peter Chen wrote:
> >>>>>>> On Tue, Sep 08, 2015 at 03:25:25PM +0300, Roger Quadros wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 08/09/15 11:31, Peter Chen wrote:
> >>>>>>>>> On Mon, Sep 07, 2015 at 01:23:01PM +0300, Roger Quadros wrote:
> >>>>>>>>>> On 07/09/15 04:23, Peter Chen wrote:
> >>>>>>>>>>> On Mon, Aug 24, 2015 at 04:21:18PM +0300, Roger Quadros wrote:
> >>>>>>>>>>>> + * This is used by the USB Host stack to register the Host controller
> >>>>>>>>>>>> + * to the OTG core. Host controller must not be started by the
> >>>>>>>>>>>> + * caller as it is left upto the OTG state machine to do so.
> >>>>>>>>>>>> + *
> >>>>>>>>>>>> + * Returns: 0 on success, error value otherwise.
> >>>>>>>>>>>> + */
> >>>>>>>>>>>> +int usb_otg_register_hcd(struct usb_hcd *hcd, unsigned int irqnum,
> >>>>>>>>>>>> + unsigned long irqflags, struct otg_hcd_ops *ops)
> >>>>>>>>>>>> +{
> >>>>>>>>>>>> + struct usb_otg *otgd;
> >>>>>>>>>>>> + struct device *hcd_dev = hcd->self.controller;
> >>>>>>>>>>>> + struct device *otg_dev = usb_otg_get_device(hcd_dev);
> >>>>>>>>>>>> +
> >>>>>>>>>>>
> >>>>>>>>>>> One big problem here is: there are two designs for current (IP) driver
> >>>>>>>>>>> code, one creates dedicated hcd device as roothub's parent, like dwc3.
> >>>>>>>>>>> Another one doesn't do this, roothub's parent is core device (or otg device
> >>>>>>>>>>> in your patch), like chipidea and dwc2.
> >>>>>>>>>>>
> >>>>>>>>>>> Then, otg_dev will be glue layer device for chipidea after that.
> >>>>>>>>>>
> >>>>>>>>>> OK. Let's add a way for the otg controller driver to provide the host and gadget
> >>>>>>>>>> information to the otg core for such devices like chipidea and dwc2.
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Roger, not only chipidea and dwc2, I think the musb uses the same
> >>>>>>>>> hierarchy. If the host, device, and otg share the same register
> >>>>>>>>> region, host part can't be a platform driver since we don't want
> >>>>>>>>> to remap the same register region again.
> >>>>>>>>>
> >>>>>>>>> So, in the design, we may need to consider both situations, one
> >>>>>>>>> is otg/host/device has its own register region, and host is a
> >>>>>>>>> separate platform device (A), the other is three parts share the
> >>>>>>>>> same register region, there is only one platform driver (B).
> >>>>>>>>>
> >>>>>>>>> A:
> >>>>>>>>>
> >>>>>>>>> IP core device
> >>>>>>>>> |
> >>>>>>>>> |
> >>>>>>>>> |-----|-----|
> >>>>>>>>> gadget host platform device
> >>>>>>>>> |
> >>>>>>>>> roothub
> >>>>>>>>>
> >>>>>>>>> B:
> >>>>>>>>>
> >>>>>>>>> IP core device
> >>>>>>>>> |
> >>>>>>>>> |
> >>>>>>>>> |-----|-----|
> >>>>>>>>> gadget roothub
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> This API must be called before the hcd/gadget-driver is added so that the otg
> >>>>>>>>>> core knows it's linked to an OTG controller.
> >>>>>>>>>>
> >>>>>>>>>> Any better idea?
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> A flag stands for this hcd controller is the same with otg controller
> >>>>>>>>> can be used, this flag can be stored at struct usb_otg_config.
> >>>>>>>>
> >>>>>>>> What if there is another architecture like so?
> >>>>>>>>
> >>>>>>>> C:
> >>>>>>>> [Parent]
> >>>>>>>> |
> >>>>>>>> |
> >>>>>>>> |------------------|--------------|
> >>>>>>>> [OTG core] [gadget] [host]
> >>>>>>>>
> >>>>>>>> We need a more flexible mechanism to link the gadget and
> >>>>>>>> host device to the otg core for non DT case.
> >>>>>>>>
> >>>>>>>> How about adding struct usb_otg parameter to usb_otg_register_hcd()?
> >>>>>>>>
> >>>>>>>> e.g.
> >>>>>>>> int usb_otg_register_hcd(struct usb_otg *otg, struct usb_hcd *hcd, ..)
> >>>>>>>>
> >>>>>>>> If otg is NULL it will try DT otg-controller property or parent to
> >>>>>>>> get the otg controller.
> >>>>>>>
> >>>>>>> How usb_otg_register_hcd get struct usb_otg, from where?
> >>>>>>
> >>>>>> This only works when the parent driver creating the hcd has registered the
> >>>>>> otg controller too.
> >>>>>>
> >>>>>
> >>>>> Sorry? So we need to find another way to solve this issue, right?
> >>>>
> >>>> For existing cases this is sufficient.
> >>>> The otg device is either the one supplied during usb_otg_register_hcd
> >>>> (cases B and C) or it is the parent device (case A).
> >>>
> >>> How we differentiate case A and case B at usb_otg_register_hcd?
> >>> Would you show me the sample code?
> >>
> >> Case A:
> >>
> >> hcd platform driver doesn't know about otg device so it calls
> >>
> >> usb_add_hcd(hcd,..)->usb_otg_register_hcd(NULL, hcd,..);
> >>
> >> Case B:
> >>
> >> core driver knows about both otg and hcd so it calls
> >> usb_otg_register_hcd(otg, hcd,...);
> >>
> >
> > Ok, Get your points, you mean invoke usb_otg_register_hcd at platform
> > driver directly instead of at hcd.c. It may be not a good solution
> > due to we use different otg APIs for two cases, it may confuse the
> > users, unless we can have some APIs (flags) are easy to read and well
> > documentation.
> >
>
> I need to think how else we can solve this problem so that it is usable
> for all scenarios. If you get some bright ideas please do share :)
>
If we want to call OTG stuff at hcd/gadget driver, we'd better store
struct usb_otg pointer at struct usb_hcd and struct usb_gadget, since
the parent/child relationship can't be used now.
The platform driver can call usb_otg_register_hcd/usb_otg_register_gadget
to achieve this, it is a little different with your current design.
--
Best Regards,
Peter Chen
--
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