diff mbox

[RFC:PATCH,3.6.0-] net/ipconfig: Extend ipconfig retries to device open

Message ID 1349365123-25826-1-git-send-email-srinivas.kandagatla@st.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Srinivas KANDAGATLA Oct. 4, 2012, 3:38 p.m. UTC 
From: Srinivas Kandagatla <srinivas.kandagatla@st.com>

This patch adds retries to ipconfig at device open, the reason to do
this is: Lets say If some mdio bus driver decide to use defered probe
when it does not find any phys on the bus. The same mdio-bus driver is
re-probed as part of lateinit calls. However ipconfig also fits into
lateinit calls, so if ipconfig is called before the re-probe of mdio-bus
driver, the mac driver will fail to find a valid PHY on the mdio-bus.

Real use case here, is that for phy modes like RMII where the clock to
the PHY should be supplied by MAC which can only happen once MAC driver
is setup. In my setup Am using stmmac MAC driver and mdio-gpio driver.

Moreover ipconfig code already uses retries in dynamic ip case, so
extending it to device open would help cases as described.

Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.com>
---
Hi All, 
Recently I did try defered probe to address a sequencing issue with 
our STMMAC and mdio-gpio driver when PHY is in RMII mode.

Things did not work as expected, because ipconfig and 
defered probe both of them are called at lateinit call level.
So I was back to sequencing problem again.

As, ipconfig code already uses retries in dynamic ip case, so
extending it to device open helped in my case.

Comments?
Am open for any suggestions to address this issue.

Thanks,
srini


 net/ipv4/ipconfig.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

Comments

David Miller Oct. 7, 2012, 4:11 a.m. UTC | #1 
From: Srinivas KANDAGATLA <srinivas.kandagatla@st.com>
Date: Thu,  4 Oct 2012 16:38:43 +0100

> This patch adds retries to ipconfig at device open, the reason to do
> this is: Lets say If some mdio bus driver decide to use defered probe
> when it does not find any phys on the bus. The same mdio-bus driver is
> re-probed as part of lateinit calls. However ipconfig also fits into
> lateinit calls, so if ipconfig is called before the re-probe of mdio-bus
> driver, the mac driver will fail to find a valid PHY on the mdio-bus.

Real device drivers for real devices should not probe using late
initcalls.

The whole point of late initcalls is that you can be certain that they
run after such things.

Fix the virus not the symptom.

I'm not applying this patch.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Srinivas KANDAGATLA Oct. 8, 2012, 12:38 p.m. UTC | #2 
On 07/10/12 05:11, David Miller wrote:
> From: Srinivas KANDAGATLA <srinivas.kandagatla@st.com>
> Date: Thu,  4 Oct 2012 16:38:43 +0100
>
>> This patch adds retries to ipconfig at device open, the reason to do
>> this is: Lets say If some mdio bus driver decide to use defered probe
>> when it does not find any phys on the bus. The same mdio-bus driver is
>> re-probed as part of lateinit calls. However ipconfig also fits into
>> lateinit calls, so if ipconfig is called before the re-probe of mdio-bus
>> driver, the mac driver will fail to find a valid PHY on the mdio-bus.
> Real device drivers for real devices should not probe using late
> initcalls.
I agree,
Let me summarize what I did try.

I wanted to use "Defered Probe Feature" which went in 3.4 kernel to
solve a sequencing issue.
So modified Mdio-driver accordingly, mdio-driver decided to defer its
probe for the first-time when It could not detect any PHY's on the BUS.
(second time) device probe is actually called in lateinit call sequence
by "Defered Probe Code".

>
> The whole point of late initcalls is that you can be certain that they
> run after such things.
Yes I agree.
>
> Fix the virus not the symptom.
>
> I'm not applying this patch.

This use-case here is totally possible given that mdio bus can be
independent driver to MAC driver and Vice-versa.
So there might be situation at times that mdio bus driver might depend
on MAC driver, like in my case MAC driver has to setup itself to provide
clock to PHY's on MDIO bus before mdio-bus driver scan phys.
So using "Defered probe" feature for mdio-bus drivers will never work.

Thanks,
srini

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller Oct. 8, 2012, 6:14 p.m. UTC | #3 
From: Srinivas KANDAGATLA <srinivas.kandagatla@st.com>
Date: Mon, 08 Oct 2012 13:38:57 +0100

> On 07/10/12 05:11, David Miller wrote:
>> From: Srinivas KANDAGATLA <srinivas.kandagatla@st.com>
>> Date: Thu,  4 Oct 2012 16:38:43 +0100
>>
>>> This patch adds retries to ipconfig at device open, the reason to do
>>> this is: Lets say If some mdio bus driver decide to use defered probe
>>> when it does not find any phys on the bus. The same mdio-bus driver is
>>> re-probed as part of lateinit calls. However ipconfig also fits into
>>> lateinit calls, so if ipconfig is called before the re-probe of mdio-bus
>>> driver, the mac driver will fail to find a valid PHY on the mdio-bus.
>> Real device drivers for real devices should not probe using late
>> initcalls.
> I agree,
> Let me summarize what I did try.
> 
> I wanted to use "Defered Probe Feature" which went in 3.4 kernel to
> solve a sequencing issue.
> So modified Mdio-driver accordingly, mdio-driver decided to defer its
> probe for the first-time when It could not detect any PHY's on the BUS.
> (second time) device probe is actually called in lateinit call sequence
> by "Defered Probe Code".

That's not how we probe "devices on a bus" which is the model you
should be following here to fix this bug.

Just delaying is going to run into corner cases where things do not
work.  You need cooperation between bus provider and bus consumer,
full stop.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/ipv4/ipconfig.c b/net/ipv4/ipconfig.c
index 67e8a6b..c30505e 100644
--- a/net/ipv4/ipconfig.c
+++ b/net/ipv4/ipconfig.c
@@ -1375,9 +1375,7 @@  static int __init wait_for_devices(void)
 static int __init ip_auto_config(void)
 {
 	__be32 addr;
-#ifdef IPCONFIG_DYNAMIC
 	int retries = CONF_OPEN_RETRIES;
-#endif
 	int err;
 
 #ifdef CONFIG_PROC_FS
@@ -1398,12 +1396,22 @@  static int __init ip_auto_config(void)
 
 	/* Setup all network devices */
 	err = ic_open_devs();
+
+	if (err && --retries) {
+		pr_info("IP-Config: Reopening network devices...\n");
+		ssleep(1);
+		goto try_try_again;
+	}
+
 	if (err)
 		return err;
 
 	/* Give drivers a chance to settle */
 	msleep(CONF_POST_OPEN);
 
+#ifdef IPCONFIG_DYNAMIC
+	retries = CONF_OPEN_RETRIES;
+#endif
 	/*
 	 * If the config information is insufficient (e.g., our IP address or
 	 * IP address of the boot server is missing or we have multiple network