All of lore.kernel.org
 help / color / mirror / Atom feed
From: andrew@lunn.ch (Andrew Lunn)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 2/3] usb: chipidea: host: let the hcd know's parent device node
Date: Fri, 4 Mar 2016 03:17:30 +0100	[thread overview]
Message-ID: <20160304021730.GA20597@lunn.ch> (raw)
In-Reply-To: <20160304015326.GA30272@shlinux2.ap.freescale.net>

> > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > > index 053bac9..55120ef 100644
> > > --- a/drivers/usb/chipidea/host.c
> > > +++ b/drivers/usb/chipidea/host.c
> > > @@ -109,15 +109,25 @@ static int host_start(struct ci_hdrc *ci)
> > >  	struct ehci_hcd *ehci;
> > >  	struct ehci_ci_priv *priv;
> > >  	int ret;
> > > +	struct device *dev = ci->dev;
> > >  
> > > -	if (usb_disabled())
> > > +	if (usb_disabled() || !dev)
> > >  		return -ENODEV;
> > >  
> > > -	hcd = usb_create_hcd(&ci_ehci_hc_driver, ci->dev, dev_name(ci->dev));
> > > +	/*
> > > +	 * USB Core will try to get child node under roothub,
> > > +	 * but chipidea core has no of_node, and the child node
> > > +	 * for controller is located at glue layer's node which
> > > +	 * is chipidea core's parent.
> > > +	 */
> > > +	if (dev->parent && dev->parent->of_node)
> > > +		dev->of_node = dev->parent->of_node;
> > 
> > Is this a good idea? Two devices with the same of_node?
> > 
> 
> This is only for chipidea driver whose host controller device
> doesn't have entry at dts, but other host controller driver which
> supports device tree should have its entry at dts.
> 
> > I know the networking code assumes of_node values are unique, and uses
> > it to find a device. Are you 100% sure the USB code does not make this
> > assumption.
> > 
> 
> The controller device is the root for USB device, the common
> USB code will not touch its glue layer device (controller's parent).

I'm just thinking about code like:

of_find_spi_master_by_node(), of_find_net_device_by_node(),
of_find_backlight_by_node(), etc.

If somebody was to implement an of_find_usb_host_by_node() are you
100% sure the right node will be found? This seems like a bug waiting
to happen.

    Andrew

WARNING: multiple messages have this Message-ID (diff)
From: Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>
To: Peter Chen <hzpeterchen-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Peter Chen <peter.chen-3arQi8VN3Tc@public.gmane.org>,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org,
	stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz@public.gmane.org,
	mark.rutland-5wv7dgnIgG8@public.gmane.org,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	mail-APzI5cXaD1zVlRWJc41N0YvC60bnQu0Y@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	pawel.moll-5wv7dgnIgG8@public.gmane.org,
	s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
	linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
	Peter Chen <peter.chen-KZfg59tc24xl57MIdRCFDg@public.gmane.org>,
	p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org,
	festevam-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 2/3] usb: chipidea: host: let the hcd know's parent device node
Date: Fri, 4 Mar 2016 03:17:30 +0100	[thread overview]
Message-ID: <20160304021730.GA20597@lunn.ch> (raw)
In-Reply-To: <20160304015326.GA30272-Fb7DQEYuewWctlrPMvKcciBecyulp+rMXqFh9Ls21Oc@public.gmane.org>

> > > diff --git a/drivers/usb/chipidea/host.c b/drivers/usb/chipidea/host.c
> > > index 053bac9..55120ef 100644
> > > --- a/drivers/usb/chipidea/host.c
> > > +++ b/drivers/usb/chipidea/host.c
> > > @@ -109,15 +109,25 @@ static int host_start(struct ci_hdrc *ci)
> > >  	struct ehci_hcd *ehci;
> > >  	struct ehci_ci_priv *priv;
> > >  	int ret;
> > > +	struct device *dev = ci->dev;
> > >  
> > > -	if (usb_disabled())
> > > +	if (usb_disabled() || !dev)
> > >  		return -ENODEV;
> > >  
> > > -	hcd = usb_create_hcd(&ci_ehci_hc_driver, ci->dev, dev_name(ci->dev));
> > > +	/*
> > > +	 * USB Core will try to get child node under roothub,
> > > +	 * but chipidea core has no of_node, and the child node
> > > +	 * for controller is located at glue layer's node which
> > > +	 * is chipidea core's parent.
> > > +	 */
> > > +	if (dev->parent && dev->parent->of_node)
> > > +		dev->of_node = dev->parent->of_node;
> > 
> > Is this a good idea? Two devices with the same of_node?
> > 
> 
> This is only for chipidea driver whose host controller device
> doesn't have entry at dts, but other host controller driver which
> supports device tree should have its entry at dts.
> 
> > I know the networking code assumes of_node values are unique, and uses
> > it to find a device. Are you 100% sure the USB code does not make this
> > assumption.
> > 
> 
> The controller device is the root for USB device, the common
> USB code will not touch its glue layer device (controller's parent).

I'm just thinking about code like:

of_find_spi_master_by_node(), of_find_net_device_by_node(),
of_find_backlight_by_node(), etc.

If somebody was to implement an of_find_usb_host_by_node() are you
100% sure the right node will be found? This seems like a bug waiting
to happen.

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

  reply	other threads:[~2016-03-04  2:17 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-03 10:01 [PATCH 0/3] Add power sequence for hard-wired USB devices Peter Chen
2016-03-03 10:01 ` Peter Chen
2016-03-03 10:01 ` [PATCH 1/3] usb: core: add power sequence for " Peter Chen
2016-03-03 10:01   ` Peter Chen
2016-03-03 18:31   ` Alan Stern
2016-03-03 18:31     ` Alan Stern
2016-03-04  2:07     ` Peter Chen
2016-03-04  2:07       ` Peter Chen
2016-03-03 20:54   ` Rob Herring
2016-03-03 20:54     ` Rob Herring
2016-03-04  2:02     ` Peter Chen
2016-03-04  2:02       ` Peter Chen
2016-03-04  2:23       ` Andrew Lunn
2016-03-04  2:23         ` Andrew Lunn
2016-03-04  2:37         ` Peter Chen
2016-03-04  2:37           ` Peter Chen
2016-03-05  4:28           ` Rob Herring
2016-03-05  4:28             ` Rob Herring
2016-03-05  8:33             ` Peter Chen
2016-03-05  8:33               ` Peter Chen
2016-03-05 14:10               ` Andrew Lunn
2016-03-05 14:10                 ` Andrew Lunn
2016-03-14 10:42                 ` Peter Chen
2016-03-14 10:42                   ` Peter Chen
2016-04-05  9:35                   ` Peter Chen
2016-04-05  9:35                     ` Peter Chen
2016-03-03 10:01 ` [PATCH 2/3] usb: chipidea: host: let the hcd know's parent device node Peter Chen
2016-03-03 10:01   ` Peter Chen
2016-03-03 14:42   ` Andrew Lunn
2016-03-03 14:42     ` Andrew Lunn
2016-03-04  1:53     ` Peter Chen
2016-03-04  1:53       ` Peter Chen
2016-03-04  2:17       ` Andrew Lunn [this message]
2016-03-04  2:17         ` Andrew Lunn
2016-03-04  2:32         ` Peter Chen
2016-03-04  2:32           ` Peter Chen
2016-03-03 10:01 ` [PATCH 3/3] ARM: dts: imx6qdl-udoo.dtsi: fix onboard USB HUB property Peter Chen
2016-03-03 10:01   ` Peter Chen
2016-03-03 22:30   ` Maciej S. Szmigiero
2016-03-03 22:30     ` Maciej S. Szmigiero
2016-03-04  2:04     ` Peter Chen
2016-03-04  2:04       ` Peter Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160304021730.GA20597@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.