mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [RFC] device probe order
@ 2015-12-23 16:10 Peter Mamonov
  2015-12-23 16:35 ` Alexander Aring
  2015-12-23 19:46 ` Sascha Hauer
  0 siblings, 2 replies; 12+ messages in thread
From: Peter Mamonov @ 2015-12-23 16:10 UTC (permalink / raw)
  To: barebox

Dear All,

I've ported an UHCI driver from the u-boot to the barebox (WIP). To
interoperate with the EHCI driver, the UHCI driver should be probed
ater the EHCI driver. Both drivers are binded via the device tree
mechanism. How can i achieve the correct probe order?

Regards,
Peter

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-23 16:10 [RFC] device probe order Peter Mamonov
@ 2015-12-23 16:35 ` Alexander Aring
  2015-12-23 16:56   ` Peter Mamonov
  2015-12-23 19:46 ` Sascha Hauer
  1 sibling, 1 reply; 12+ messages in thread
From: Alexander Aring @ 2015-12-23 16:35 UTC (permalink / raw)
  To: Peter Mamonov; +Cc: barebox

On Wed, Dec 23, 2015 at 07:10:58PM +0300, Peter Mamonov wrote:
> Dear All,
> 
> I've ported an UHCI driver from the u-boot to the barebox (WIP). To
> interoperate with the EHCI driver, the UHCI driver should be probed
> ater the EHCI driver. Both drivers are binded via the device tree
> mechanism. How can i achieve the correct probe order?
> 

Normally this should done by returning "-EPROBE_DEFER" inside the probe
function. There was some RFC last years for supporting EPROBE_DEFER [0]
and it seems these are mainline.

However you need some bool which indicates that the EHCI driver is probed.

int uhci_probe(foobar) {

if (!indicate_ehci_is_probed(foobar)
	return -EPROBE_DEFER;
}

- Alex

[0] http://barebox.infradead.narkive.com/ZWIXXU0R/patch-v2-0-6-introduce-deferred-probing

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-23 16:35 ` Alexander Aring
@ 2015-12-23 16:56   ` Peter Mamonov
  2015-12-23 17:04     ` Alexander Aring
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Mamonov @ 2015-12-23 16:56 UTC (permalink / raw)
  To: Alexander Aring, Sascha Hauer; +Cc: barebox

On Wed, 23 Dec 2015 17:35:51 +0100
Alexander Aring <alex.aring@gmail.com> wrote:

> On Wed, Dec 23, 2015 at 07:10:58PM +0300, Peter Mamonov wrote:
> > Dear All,
> > 
> > I've ported an UHCI driver from the u-boot to the barebox (WIP). To
> > interoperate with the EHCI driver, the UHCI driver should be probed
> > ater the EHCI driver. Both drivers are binded via the device tree
> > mechanism. How can i achieve the correct probe order?
> > 
> 
> Normally this should done by returning "-EPROBE_DEFER" inside the
> probe function. There was some RFC last years for supporting
> EPROBE_DEFER [0] and it seems these are mainline.
> 
> However you need some bool which indicates that the EHCI driver is
> probed.

Thanks, Alex. As i understand, this is the linux-way solution.

Sasha, is it ok to add a global variable to indicate the EHCI presence?
Or should we follow the way proposed by the mentioned RFCs, i.e.
introduce dependencies between drivers?

> 
> int uhci_probe(foobar) {
> 
> if (!indicate_ehci_is_probed(foobar)
> 	return -EPROBE_DEFER;
> }
> 
> - Alex
> 
> [0]
> http://barebox.infradead.narkive.com/ZWIXXU0R/patch-v2-0-6-introduce-deferred-probing


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-23 16:56   ` Peter Mamonov
@ 2015-12-23 17:04     ` Alexander Aring
  2015-12-24  9:48       ` Peter Mamonov
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Aring @ 2015-12-23 17:04 UTC (permalink / raw)
  To: Peter Mamonov; +Cc: barebox

On Wed, Dec 23, 2015 at 07:56:44PM +0300, Peter Mamonov wrote:
> On Wed, 23 Dec 2015 17:35:51 +0100
> Alexander Aring <alex.aring@gmail.com> wrote:
> 
> > On Wed, Dec 23, 2015 at 07:10:58PM +0300, Peter Mamonov wrote:
> > > Dear All,
> > > 
> > > I've ported an UHCI driver from the u-boot to the barebox (WIP). To
> > > interoperate with the EHCI driver, the UHCI driver should be probed
> > > ater the EHCI driver. Both drivers are binded via the device tree
> > > mechanism. How can i achieve the correct probe order?
> > > 
> > 
> > Normally this should done by returning "-EPROBE_DEFER" inside the
> > probe function. There was some RFC last years for supporting
> > EPROBE_DEFER [0] and it seems these are mainline.
> > 
> > However you need some bool which indicates that the EHCI driver is
> > probed.
> 
> Thanks, Alex. As i understand, this is the linux-way solution.
> 
> Sasha, is it ok to add a global variable to indicate the EHCI presence?
> Or should we follow the way proposed by the mentioned RFCs, i.e.
> introduce dependencies between drivers?
> 

mhhh, maybe a simple "get_device_by_name" works here.

If returning NULL then return -EPROBE_DEFER. Don't know if this is a
good solution, name need to be unique then.


btw:
Just found that "of_find_device_by_node" returns -EPROBE_DEFER when
nothing was found. This was introduced by the patch series.

Maybe it helps to look how the current use-cases deals with
-EPROBE_DEFER or get_device_by_name is enough.

- Alex


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-23 16:10 [RFC] device probe order Peter Mamonov
  2015-12-23 16:35 ` Alexander Aring
@ 2015-12-23 19:46 ` Sascha Hauer
  2015-12-24 10:46   ` Peter Mamonov
  1 sibling, 1 reply; 12+ messages in thread
From: Sascha Hauer @ 2015-12-23 19:46 UTC (permalink / raw)
  To: Peter Mamonov; +Cc: barebox

Hi Peter,

On Wed, Dec 23, 2015 at 07:10:58PM +0300, Peter Mamonov wrote:
> Dear All,
> 
> I've ported an UHCI driver from the u-boot to the barebox (WIP). To
> interoperate with the EHCI driver, the UHCI driver should be probed
> ater the EHCI driver. Both drivers are binded via the device tree
> mechanism. How can i achieve the correct probe order?

Do you have an example binding to look at? Normally I would assume that
the binding makes sure somehow that the uhci driver has to be probed.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-23 17:04     ` Alexander Aring
@ 2015-12-24  9:48       ` Peter Mamonov
  2015-12-24 13:42         ` Peter Mamonov
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Mamonov @ 2015-12-24  9:48 UTC (permalink / raw)
  To: Alexander Aring; +Cc: barebox

On Wed, 23 Dec 2015 18:04:43 +0100
Alexander Aring <alex.aring@gmail.com> wrote:

> On Wed, Dec 23, 2015 at 07:56:44PM +0300, Peter Mamonov wrote:
> > On Wed, 23 Dec 2015 17:35:51 +0100
> > Alexander Aring <alex.aring@gmail.com> wrote:
> > 
> > > On Wed, Dec 23, 2015 at 07:10:58PM +0300, Peter Mamonov wrote:
> > > > Dear All,
> > > > 
> > > > I've ported an UHCI driver from the u-boot to the barebox
> > > > (WIP). To interoperate with the EHCI driver, the UHCI driver
> > > > should be probed ater the EHCI driver. Both drivers are binded
> > > > via the device tree mechanism. How can i achieve the correct
> > > > probe order?
> > > > 
> > > 
> > > Normally this should done by returning "-EPROBE_DEFER" inside the
> > > probe function. There was some RFC last years for supporting
> > > EPROBE_DEFER [0] and it seems these are mainline.
> > > 
> > > However you need some bool which indicates that the EHCI driver is
> > > probed.
> > 
> > Thanks, Alex. As i understand, this is the linux-way solution.
> > 
> > Sasha, is it ok to add a global variable to indicate the EHCI
> > presence? Or should we follow the way proposed by the mentioned
> > RFCs, i.e. introduce dependencies between drivers?
> > 
> 
> mhhh, maybe a simple "get_device_by_name" works here.
> 
> If returning NULL then return -EPROBE_DEFER. Don't know if this is a
> good solution, name need to be unique then.
> 
> 
> btw:
> Just found that "of_find_device_by_node" returns -EPROBE_DEFER when
> nothing was found. This was introduced by the patch series.

I like this approach better, than introducing a global variable.
Will look further into it.

> 
> Maybe it helps to look how the current use-cases deals with
> -EPROBE_DEFER or get_device_by_name is enough.
> 
> - Alex
> 


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-23 19:46 ` Sascha Hauer
@ 2015-12-24 10:46   ` Peter Mamonov
  2015-12-24 14:35     ` Alexander Aring
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Mamonov @ 2015-12-24 10:46 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Wed, 23 Dec 2015 20:46:02 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi Peter,
> 
> On Wed, Dec 23, 2015 at 07:10:58PM +0300, Peter Mamonov wrote:
> > Dear All,
> > 
> > I've ported an UHCI driver from the u-boot to the barebox (WIP). To
> > interoperate with the EHCI driver, the UHCI driver should be probed
> > ater the EHCI driver. Both drivers are binded via the device tree
> > mechanism. How can i achieve the correct probe order?
> 
> Do you have an example binding to look at? Normally I would assume
> that the binding makes sure somehow that the uhci driver has to be
> probed.


At the moment the binding is quite straightforward:

		ehci: ehci@1ba00200 {
			compatible = "generic-ehci";
			reg = <0x00000000 0x20 0x00000000 0x100>;
			status = "disabled";
		};

		uhci: uhci@1ba00000 {
			compatible = "generic-uhci";
			reg = <0x00000000 0x200>;
			status = "disabled";
		};

Probably, we can add "companion = <&ehci>;" into the uhci node and
check if the ehci has been probed by calling of_find_device_by_node(),
as  Alexander Aring proposed.

> 
> Sascha
> 
> 


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-24  9:48       ` Peter Mamonov
@ 2015-12-24 13:42         ` Peter Mamonov
  2016-01-04  8:56           ` Sascha Hauer
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Mamonov @ 2015-12-24 13:42 UTC (permalink / raw)
  To: Alexander Aring; +Cc: barebox

On Thu, 24 Dec 2015 12:48:37 +0300
Peter Mamonov <pmamonov@gmail.com> wrote:

> On Wed, 23 Dec 2015 18:04:43 +0100
> Alexander Aring <alex.aring@gmail.com> wrote:
> 
> > On Wed, Dec 23, 2015 at 07:56:44PM +0300, Peter Mamonov wrote:
> > > On Wed, 23 Dec 2015 17:35:51 +0100
> > > Alexander Aring <alex.aring@gmail.com> wrote:
> > > 
> > > > On Wed, Dec 23, 2015 at 07:10:58PM +0300, Peter Mamonov wrote:
> > > > > Dear All,
> > > > > 
> > > > > I've ported an UHCI driver from the u-boot to the barebox
> > > > > (WIP). To interoperate with the EHCI driver, the UHCI driver
> > > > > should be probed ater the EHCI driver. Both drivers are binded
> > > > > via the device tree mechanism. How can i achieve the correct
> > > > > probe order?
> > > > > 
> > > > 
> > > > Normally this should done by returning "-EPROBE_DEFER" inside
> > > > the probe function. There was some RFC last years for supporting
> > > > EPROBE_DEFER [0] and it seems these are mainline.
> > > > 
> > > > However you need some bool which indicates that the EHCI driver
> > > > is probed.
> > > 
> > > Thanks, Alex. As i understand, this is the linux-way solution.
> > > 
> > > Sasha, is it ok to add a global variable to indicate the EHCI
> > > presence? Or should we follow the way proposed by the mentioned
> > > RFCs, i.e. introduce dependencies between drivers?
> > > 
> > 
> > mhhh, maybe a simple "get_device_by_name" works here.
> > 
> > If returning NULL then return -EPROBE_DEFER. Don't know if this is a
> > good solution, name need to be unique then.
> > 
> > 
> > btw:
> > Just found that "of_find_device_by_node" returns -EPROBE_DEFER when
> > nothing was found. This was introduced by the patch series.
> 
> I like this approach better, than introducing a global variable.
> Will look further into it.

Unfortunately of_find_device_by_node() returns a valid pointer to
the device before the device probe function is called. I guess
get_device_by_name() behaves in the same way.

> 
> > 
> > Maybe it helps to look how the current use-cases deals with
> > -EPROBE_DEFER or get_device_by_name is enough.
> > 
> > - Alex
> > 
> 


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-24 10:46   ` Peter Mamonov
@ 2015-12-24 14:35     ` Alexander Aring
  2015-12-24 16:10       ` Peter Mamonov
  0 siblings, 1 reply; 12+ messages in thread
From: Alexander Aring @ 2015-12-24 14:35 UTC (permalink / raw)
  To: Peter Mamonov; +Cc: barebox

On Thu, Dec 24, 2015 at 01:46:53PM +0300, Peter Mamonov wrote:
> On Wed, 23 Dec 2015 20:46:02 +0100
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Hi Peter,
> > 
> > On Wed, Dec 23, 2015 at 07:10:58PM +0300, Peter Mamonov wrote:
> > > Dear All,
> > > 
> > > I've ported an UHCI driver from the u-boot to the barebox (WIP). To
> > > interoperate with the EHCI driver, the UHCI driver should be probed
> > > ater the EHCI driver. Both drivers are binded via the device tree
> > > mechanism. How can i achieve the correct probe order?
> > 
> > Do you have an example binding to look at? Normally I would assume
> > that the binding makes sure somehow that the uhci driver has to be
> > probed.
> 
> 
> At the moment the binding is quite straightforward:
> 
> 		ehci: ehci@1ba00200 {
> 			compatible = "generic-ehci";
> 			reg = <0x00000000 0x20 0x00000000 0x100>;
> 			status = "disabled";
> 		};
> 
> 		uhci: uhci@1ba00000 {
> 			compatible = "generic-uhci";
> 			reg = <0x00000000 0x200>;
> 			status = "disabled";
> 		};
> 
> Probably, we can add "companion = <&ehci>;" into the uhci node and
> check if the ehci has been probed by calling of_find_device_by_node(),
> as  Alexander Aring proposed.
> 

I mentioned the -EPROBE_DEFER because we do the same way at handling rpi
power domains, which requires the firmware module at first. See [0].

There we use:

fw_np = of_parse_phandle(pdev->dev.of_node, "firmware", 0);

in you case it would be:

ehci_np = of_parse_phandle(pdev->dev.of_node, "companion", 0);
where "pdev->dev.of_node" is uhci device node.

If this fails then we return -ENODEV, but you better don't do nothing
then because "companion" is optional then.

Back to Linux solution at [0]:

After that function "rpi_firmware_get" tries to get some driver data and
this driver data is available only when the firmware is successful probed.
Means: return -EPROBE_DEFER when function "rpi_firmware_get" returns
NULL, otherwise you can be sure that ehci is probed.

Note:
This is the linux way and I don't know if this works under barebox. :-)
Maybe there exists a better way, what Sascha said, that the device-tree
will do low-level handling of -EPROBE_DEFER somehow. I know that some
subsystems e.g. power domains will do that somehow in special
power-domain device-tree handling and magic handle the device probing in
the correct order (but at the end it's really some handling with
-EPROBE_DEFER).

- Alex

[0] https://github.com/anholt/linux/blob/b936d16077b18a575c5b892c8fe21a6ca67fc31a/arch/arm/mach-bcm/raspberrypi-power.c#L175
[1] https://github.com/anholt/linux/blob/rpi-4.2.y/drivers/firmware/raspberrypi.c#L251

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-24 14:35     ` Alexander Aring
@ 2015-12-24 16:10       ` Peter Mamonov
  2015-12-25 10:21         ` Alexander Aring
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Mamonov @ 2015-12-24 16:10 UTC (permalink / raw)
  To: Alexander Aring, Sascha Hauer; +Cc: barebox

Let me summarize my efforts:

"Global variable" solution works fine, however it is not elegant enough:

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 862444b..d06e001 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -138,0 +139,2 @@ static struct descriptor {
+int ehci_probed = 0;
+
@@ -1346,0 +1349,2 @@ static int ehci_probe(struct device_d *dev)
+       ehci_probed = 1;
+
diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
index 5a5314f..c99426c 100644
--- a/drivers/usb/host/uhci-hcd.c
+++ b/drivers/usb/host/uhci-hcd.c
@@ -81,0 +82,2 @@
+extern int ehci_probed;
+
@@ -1232,0 +1235,4 @@ static int uhci_probe(struct device_d *dev)
+       if (!ehci_probed) {
+               dev_err(dev, "PROBE_DEFER\n");
+               return -EPROBE_DEFER;
+       }



"Device tree" solution doesn't work, because of_find_device_by_node()
returns pointer to the device even though the device probe function
wasn't called. The question is: how to tell if the device probe
function was called?

diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
index c99426c..44eca36 100644
--- a/drivers/usb/host/uhci-hcd.c
+++ b/drivers/usb/host/uhci-hcd.c
@@ -1234,0 +1235,20 @@ static int uhci_probe(struct device_d *dev)
+       struct device_node *dn = dev->device_node, *companion;
+
+       if (dn) {
+               companion = of_parse_phandle(dn, "companion", 0);
+               if (companion && !of_find_device_by_node(companion)) {
+                       dev_err(dev, "PROBE_DEFER\n");
+                       return -EPROBE_DEFER;
+               }
+       }


On Thu, 24 Dec 2015 15:35:15 +0100
Alexander Aring <alex.aring@gmail.com> wrote:

> On Thu, Dec 24, 2015 at 01:46:53PM +0300, Peter Mamonov wrote:
> > On Wed, 23 Dec 2015 20:46:02 +0100
> > Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > 
> > > Hi Peter,
> > > 
> > > On Wed, Dec 23, 2015 at 07:10:58PM +0300, Peter Mamonov wrote:
> > > > Dear All,
> > > > 
> > > > I've ported an UHCI driver from the u-boot to the barebox
> > > > (WIP). To interoperate with the EHCI driver, the UHCI driver
> > > > should be probed ater the EHCI driver. Both drivers are binded
> > > > via the device tree mechanism. How can i achieve the correct
> > > > probe order?
> > > 
> > > Do you have an example binding to look at? Normally I would assume
> > > that the binding makes sure somehow that the uhci driver has to be
> > > probed.
> > 
> > 
> > At the moment the binding is quite straightforward:
> > 
> > 		ehci: ehci@1ba00200 {
> > 			compatible = "generic-ehci";
> > 			reg = <0x00000000 0x20 0x00000000 0x100>;
> > 			status = "disabled";
> > 		};
> > 
> > 		uhci: uhci@1ba00000 {
> > 			compatible = "generic-uhci";
> > 			reg = <0x00000000 0x200>;
> > 			status = "disabled";
> > 		};
> > 
> > Probably, we can add "companion = <&ehci>;" into the uhci node and
> > check if the ehci has been probed by calling
> > of_find_device_by_node(), as  Alexander Aring proposed.
> > 
> 
> I mentioned the -EPROBE_DEFER because we do the same way at handling
> rpi power domains, which requires the firmware module at first. See
> [0].
> 
> There we use:
> 
> fw_np = of_parse_phandle(pdev->dev.of_node, "firmware", 0);
> 
> in you case it would be:
> 
> ehci_np = of_parse_phandle(pdev->dev.of_node, "companion", 0);
> where "pdev->dev.of_node" is uhci device node.
> 
> If this fails then we return -ENODEV, but you better don't do nothing
> then because "companion" is optional then.
> 
> Back to Linux solution at [0]:
> 
> After that function "rpi_firmware_get" tries to get some driver data
> and this driver data is available only when the firmware is
> successful probed. Means: return -EPROBE_DEFER when function
> "rpi_firmware_get" returns NULL, otherwise you can be sure that ehci
> is probed.
> 
> Note:
> This is the linux way and I don't know if this works under
> barebox. :-) Maybe there exists a better way, what Sascha said, that
> the device-tree will do low-level handling of -EPROBE_DEFER somehow.
> I know that some subsystems e.g. power domains will do that somehow
> in special power-domain device-tree handling and magic handle the
> device probing in the correct order (but at the end it's really some
> handling with -EPROBE_DEFER).
> 
> - Alex
> 
> [0]
> https://github.com/anholt/linux/blob/b936d16077b18a575c5b892c8fe21a6ca67fc31a/arch/arm/mach-bcm/raspberrypi-power.c#L175
> [1]
> https://github.com/anholt/linux/blob/rpi-4.2.y/drivers/firmware/raspberrypi.c#L251


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-24 16:10       ` Peter Mamonov
@ 2015-12-25 10:21         ` Alexander Aring
  0 siblings, 0 replies; 12+ messages in thread
From: Alexander Aring @ 2015-12-25 10:21 UTC (permalink / raw)
  To: Peter Mamonov; +Cc: barebox

On Thu, Dec 24, 2015 at 07:10:29PM +0300, Peter Mamonov wrote:
> Let me summarize my efforts:
> 
> "Global variable" solution works fine, however it is not elegant enough:
> 
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 862444b..d06e001 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -138,0 +139,2 @@ static struct descriptor {
> +int ehci_probed = 0;
> +
> @@ -1346,0 +1349,2 @@ static int ehci_probe(struct device_d *dev)
> +       ehci_probed = 1;
> +
> diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
> index 5a5314f..c99426c 100644
> --- a/drivers/usb/host/uhci-hcd.c
> +++ b/drivers/usb/host/uhci-hcd.c
> @@ -81,0 +82,2 @@
> +extern int ehci_probed;
> +
> @@ -1232,0 +1235,4 @@ static int uhci_probe(struct device_d *dev)
> +       if (!ehci_probed) {
> +               dev_err(dev, "PROBE_DEFER\n");
> +               return -EPROBE_DEFER;
> +       }
> 
> 

I think one of the big disadvantages here to this solution is that it
doesn't work when you have multiple ehci/uhci devices.

In linux it would be non-go, but barebox... don't know if barebox can
handle such setup.

> 
> "Device tree" solution doesn't work, because of_find_device_by_node()
> returns pointer to the device even though the device probe function
> wasn't called. The question is: how to tell if the device probe
> function was called?

I would say the easiest way is to check on "dev-priv", there you also
could get the "struct ehci_priv" if needed in uhci when companion is
there. The "dev->priv" will be set when running probe.

> 
> diff --git a/drivers/usb/host/uhci-hcd.c b/drivers/usb/host/uhci-hcd.c
> index c99426c..44eca36 100644
> --- a/drivers/usb/host/uhci-hcd.c
> +++ b/drivers/usb/host/uhci-hcd.c
> @@ -1234,0 +1235,20 @@ static int uhci_probe(struct device_d *dev)
> +       struct device_node *dn = dev->device_node, *companion;
> +
> +       if (dn) {
> +               companion = of_parse_phandle(dn, "companion", 0);
> +               if (companion && !of_find_device_by_node(companion)) {
> +                       dev_err(dev, "PROBE_DEFER\n");
> +                       return -EPROBE_DEFER;
> +               }
> +       }
> 

if (dn) {
	companion = of_parse_phandle(dn, "companion", 0);
	if (companion) {
		dev = of_find_device_by_node(companion);
		if (!dev || !dev->priv) {
			dev_err(dev, "PROBE_DEFER\n");
			return -EPROBE_DEFER;
		}
	}
}

if you need "struct ehci_priv" inside of uhci driver, then I would do
some static inline function for casting and doing:

"!ehci_get_priv(dev))" instead "dev->priv", but the access should be
then readonly only.

Don't know if this is a good and acceptable solution.

- Alex

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC] device probe order
  2015-12-24 13:42         ` Peter Mamonov
@ 2016-01-04  8:56           ` Sascha Hauer
  0 siblings, 0 replies; 12+ messages in thread
From: Sascha Hauer @ 2016-01-04  8:56 UTC (permalink / raw)
  To: Peter Mamonov; +Cc: barebox

On Thu, Dec 24, 2015 at 04:42:25PM +0300, Peter Mamonov wrote:
> On Thu, 24 Dec 2015 12:48:37 +0300
> Peter Mamonov <pmamonov@gmail.com> wrote:
> 
> > On Wed, 23 Dec 2015 18:04:43 +0100
> > Alexander Aring <alex.aring@gmail.com> wrote:
> > 
> > > On Wed, Dec 23, 2015 at 07:56:44PM +0300, Peter Mamonov wrote:
> > > > On Wed, 23 Dec 2015 17:35:51 +0100
> > > > Alexander Aring <alex.aring@gmail.com> wrote:
> > > > 
> > > > > On Wed, Dec 23, 2015 at 07:10:58PM +0300, Peter Mamonov wrote:
> > > > > > Dear All,
> > > > > > 
> > > > > > I've ported an UHCI driver from the u-boot to the barebox
> > > > > > (WIP). To interoperate with the EHCI driver, the UHCI driver
> > > > > > should be probed ater the EHCI driver. Both drivers are binded
> > > > > > via the device tree mechanism. How can i achieve the correct
> > > > > > probe order?
> > > > > > 
> > > > > 
> > > > > Normally this should done by returning "-EPROBE_DEFER" inside
> > > > > the probe function. There was some RFC last years for supporting
> > > > > EPROBE_DEFER [0] and it seems these are mainline.
> > > > > 
> > > > > However you need some bool which indicates that the EHCI driver
> > > > > is probed.
> > > > 
> > > > Thanks, Alex. As i understand, this is the linux-way solution.
> > > > 
> > > > Sasha, is it ok to add a global variable to indicate the EHCI
> > > > presence? Or should we follow the way proposed by the mentioned
> > > > RFCs, i.e. introduce dependencies between drivers?
> > > > 
> > > 
> > > mhhh, maybe a simple "get_device_by_name" works here.
> > > 
> > > If returning NULL then return -EPROBE_DEFER. Don't know if this is a
> > > good solution, name need to be unique then.
> > > 
> > > 
> > > btw:
> > > Just found that "of_find_device_by_node" returns -EPROBE_DEFER when
> > > nothing was found. This was introduced by the patch series.
> > 
> > I like this approach better, than introducing a global variable.
> > Will look further into it.
> 
> Unfortunately of_find_device_by_node() returns a valid pointer to
> the device before the device probe function is called. I guess
> get_device_by_name() behaves in the same way.

This looks buggy. There should be a way to tell if a device has been
probed or not before working with the device returned by
of_find_device_by_node() or get_device_by_name(). The easiest way is
probably to check for dev->driver. This is not enough though to tell if
the device probe has failed, not yet executed, or deferred. Maybe we
could store the probe status of a device in struct device_d itself.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-01-04  8:56 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-23 16:10 [RFC] device probe order Peter Mamonov
2015-12-23 16:35 ` Alexander Aring
2015-12-23 16:56   ` Peter Mamonov
2015-12-23 17:04     ` Alexander Aring
2015-12-24  9:48       ` Peter Mamonov
2015-12-24 13:42         ` Peter Mamonov
2016-01-04  8:56           ` Sascha Hauer
2015-12-23 19:46 ` Sascha Hauer
2015-12-24 10:46   ` Peter Mamonov
2015-12-24 14:35     ` Alexander Aring
2015-12-24 16:10       ` Peter Mamonov
2015-12-25 10:21         ` Alexander Aring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox