Message ID | 1439844336-21596-1-git-send-email-linux@roeck-us.net |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
From: Guenter Roeck <linux@roeck-us.net> Date: Mon, 17 Aug 2015 13:45:36 -0700 > Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes > the call to smsc911x_probe_config() unconditional, and no longer fails if > there is no device node. device_get_phy_mode() is called unconditionally, > and if there is no phy node configured returns an error code. This error > code is assigned to phy_interface, and interpreted elsewhere in the code > as valid phy mode. This in turn causes qemu to crash when running a > variant of realview_pb_defconfig. > > qemu: hardware error: lan9118_read: Bad reg 0x86 > > Fixes: 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") > Cc: Jeremy Linton <jeremy.linton@arm.com> > Cc Graeme Gregory <graeme.gregory@linaro.org> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> Applied, thanks. -- 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
On 08/17/2015 03:45 PM, Guenter Roeck wrote: > Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes > the call to smsc911x_probe_config() unconditional, and no longer fails if > there is no device node. device_get_phy_mode() is called unconditionally, > and if there is no phy node configured returns an error code. This error > code is assigned to phy_interface, and interpreted elsewhere in the code > as valid phy mode. This in turn causes qemu to crash when running a > variant of realview_pb_defconfig. Thanks for catching that! I have a couple other minor cleanups per the reviewers. Jeremy -- 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
On 08/17/2015 02:19 PM, Jeremy Linton wrote: > On 08/17/2015 03:45 PM, Guenter Roeck wrote: >> Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes >> the call to smsc911x_probe_config() unconditional, and no longer fails if >> there is no device node. device_get_phy_mode() is called unconditionally, >> and if there is no phy node configured returns an error code. This error >> code is assigned to phy_interface, and interpreted elsewhere in the code >> as valid phy mode. This in turn causes qemu to crash when running a >> variant of realview_pb_defconfig. > > Thanks for catching that! I have a couple other minor cleanups per the reviewers. > One nitpick I noticed after sending my patch: dev can never be NULL in smsc911x_probe_config(), so it does not really make sense to check if it is NULL. Guenter -- 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
On 08/17/2015 05:14 PM, Guenter Roeck wrote: > One nitpick I noticed after sending my patch: dev can never be NULL in > smsc911x_probe_config(), so it does not really make sense to check if it is NULL. No, it doesn't... it should really be something like if (dev_fwnode(dev)) But dev_fwnode is static inline in property.c, and i'm pretty sure that still isn't 100% correct.The best plan might just be to remove the check, and abort on failure to find the phy property per your 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
Hi, * Guenter Roeck <linux@roeck-us.net> [150817 13:48]: > Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes > the call to smsc911x_probe_config() unconditional, and no longer fails if > there is no device node. device_get_phy_mode() is called unconditionally, > and if there is no phy node configured returns an error code. This error > code is assigned to phy_interface, and interpreted elsewhere in the code > as valid phy mode. This in turn causes qemu to crash when running a > variant of realview_pb_defconfig. > > qemu: hardware error: lan9118_read: Bad reg 0x86 > > Fixes: 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") > Cc: Jeremy Linton <jeremy.linton@arm.com> > Cc Graeme Gregory <graeme.gregory@linaro.org> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/net/ethernet/smsc/smsc911x.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c > index 0f21aa3bb537..34f97684506b 100644 > --- a/drivers/net/ethernet/smsc/smsc911x.c > +++ b/drivers/net/ethernet/smsc/smsc911x.c > @@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops = { > static int smsc911x_probe_config(struct smsc911x_platform_config *config, > struct device *dev) > { > + int phy_interface; > u32 width = 0; > > if (!dev) > return -ENODEV; > > - config->phy_interface = device_get_phy_mode(dev); > + phy_interface = device_get_phy_mode(dev); > + if (phy_interface < 0) > + return phy_interface; > + > + config->phy_interface = phy_interface; > > device_get_mac_address(dev, config->mac, ETH_ALEN); Looks like this change makes at least omap boards using smsc911x fail with -22 for me in Linux next. Do any of the the device tree configured smsc911x devices actually have a phy configured? Regards, Tony -- 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
On 08/26/2015 12:04 PM, Tony Lindgren wrote: > * Guenter Roeck <linux@roeck-us.net> [150817 13:48]: >> Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes > Looks like this change makes at least omap boards using smsc911x > fail with -22 for me in Linux next. > > Do any of the the device tree configured smsc911x devices actually > have a phy configured? Tony, Looks like all the ones in the kernel boot/dts directory have a phy including the omap3-lilly except for the ste-snowball.dts. Do you have smsc,force-internal-phy set instead? Thanks, -- 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
Hi Tony, On 08/26/2015 10:04 AM, Tony Lindgren wrote: > Hi, > > * Guenter Roeck <linux@roeck-us.net> [150817 13:48]: >> Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes >> the call to smsc911x_probe_config() unconditional, and no longer fails if >> there is no device node. device_get_phy_mode() is called unconditionally, >> and if there is no phy node configured returns an error code. This error >> code is assigned to phy_interface, and interpreted elsewhere in the code >> as valid phy mode. This in turn causes qemu to crash when running a >> variant of realview_pb_defconfig. >> >> qemu: hardware error: lan9118_read: Bad reg 0x86 >> >> Fixes: 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") >> Cc: Jeremy Linton <jeremy.linton@arm.com> >> Cc Graeme Gregory <graeme.gregory@linaro.org> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> drivers/net/ethernet/smsc/smsc911x.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c >> index 0f21aa3bb537..34f97684506b 100644 >> --- a/drivers/net/ethernet/smsc/smsc911x.c >> +++ b/drivers/net/ethernet/smsc/smsc911x.c >> @@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops = { >> static int smsc911x_probe_config(struct smsc911x_platform_config *config, >> struct device *dev) >> { >> + int phy_interface; >> u32 width = 0; >> >> if (!dev) >> return -ENODEV; >> >> - config->phy_interface = device_get_phy_mode(dev); >> + phy_interface = device_get_phy_mode(dev); >> + if (phy_interface < 0) >> + return phy_interface; >> + >> + config->phy_interface = phy_interface; >> >> device_get_mac_address(dev, config->mac, ETH_ALEN); > > Looks like this change makes at least omap boards using smsc911x > fail with -22 for me in Linux next. > What do you see if you revert my patch ? It should assign -22, or its unsigned representation, to phy_interface, which isn't such a good idea either. > Do any of the the device tree configured smsc911x devices actually > have a phy configured? > Good question, and beats me. Looking into the original code, it didn't check for an error return from of_get_phy_mode() either, and thus _would_ dutifully assign the error code to phy_interface. Wonder how was this supposed to work to start with. I'll do some debugging and try to find out what exactly is going on. Guenter -- 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
* Jeremy Linton <jeremy.linton@arm.com> [150826 10:35]: > On 08/26/2015 12:04 PM, Tony Lindgren wrote: > >* Guenter Roeck <linux@roeck-us.net> [150817 13:48]: > >>Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes > >Looks like this change makes at least omap boards using smsc911x > >fail with -22 for me in Linux next. > > > >Do any of the the device tree configured smsc911x devices actually > >have a phy configured? > > Tony, > > Looks like all the ones in the kernel boot/dts directory have a phy > including the omap3-lilly except for the ste-snowball.dts. > > Do you have smsc,force-internal-phy set instead? Hmm most of them are using omap-gpmc-smsc911x.dtsi and omap-gpmc-smsc9221.dtsi which are set up the same way as omap3-lilly. So no phy. Regards, Tony -- 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
* Guenter Roeck <linux@roeck-us.net> [150826 10:40]: > Hi Tony, > > On 08/26/2015 10:04 AM, Tony Lindgren wrote: > >Hi, > > > >* Guenter Roeck <linux@roeck-us.net> [150817 13:48]: > >>Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes > >>the call to smsc911x_probe_config() unconditional, and no longer fails if > >>there is no device node. device_get_phy_mode() is called unconditionally, > >>and if there is no phy node configured returns an error code. This error > >>code is assigned to phy_interface, and interpreted elsewhere in the code > >>as valid phy mode. This in turn causes qemu to crash when running a > >>variant of realview_pb_defconfig. > >> > >> qemu: hardware error: lan9118_read: Bad reg 0x86 > >> > >>Fixes: 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") > >>Cc: Jeremy Linton <jeremy.linton@arm.com> > >>Cc Graeme Gregory <graeme.gregory@linaro.org> > >>Signed-off-by: Guenter Roeck <linux@roeck-us.net> > >>--- > >> drivers/net/ethernet/smsc/smsc911x.c | 7 ++++++- > >> 1 file changed, 6 insertions(+), 1 deletion(-) > >> > >>diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c > >>index 0f21aa3bb537..34f97684506b 100644 > >>--- a/drivers/net/ethernet/smsc/smsc911x.c > >>+++ b/drivers/net/ethernet/smsc/smsc911x.c > >>@@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops = { > >> static int smsc911x_probe_config(struct smsc911x_platform_config *config, > >> struct device *dev) > >> { > >>+ int phy_interface; > >> u32 width = 0; > >> > >> if (!dev) > >> return -ENODEV; > >> > >>- config->phy_interface = device_get_phy_mode(dev); > >>+ phy_interface = device_get_phy_mode(dev); > >>+ if (phy_interface < 0) > >>+ return phy_interface; > >>+ > >>+ config->phy_interface = phy_interface; > >> > >> device_get_mac_address(dev, config->mac, ETH_ALEN); > > > >Looks like this change makes at least omap boards using smsc911x > >fail with -22 for me in Linux next. > > > > What do you see if you revert my patch ? It should assign -22, or its > unsigned representation, to phy_interface, which isn't such a good idea > either. If I revert patch "smsc911x: Fix crash seen if neither ACPI nor OF is configured or used" things work as just assign config->phy_interface directly without returning early. It's -22 in that case also. > >Do any of the the device tree configured smsc911x devices actually > >have a phy configured? > > > Good question, and beats me. Looking into the original code, > it didn't check for an error return from of_get_phy_mode() either, > and thus _would_ dutifully assign the error code to phy_interface. > Wonder how was this supposed to work to start with. > > I'll do some debugging and try to find out what exactly is going on. Looks like adding "smsc,force-internal-phy" to omap-gpmc-smsc9221.dtsi does not help. Somehow the default behavior is now different. Regards, Tony -- 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
On 08/26/2015 10:04 AM, Tony Lindgren wrote: > Hi, > > * Guenter Roeck <linux@roeck-us.net> [150817 13:48]: >> Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes >> the call to smsc911x_probe_config() unconditional, and no longer fails if >> there is no device node. device_get_phy_mode() is called unconditionally, >> and if there is no phy node configured returns an error code. This error >> code is assigned to phy_interface, and interpreted elsewhere in the code >> as valid phy mode. This in turn causes qemu to crash when running a >> variant of realview_pb_defconfig. >> >> qemu: hardware error: lan9118_read: Bad reg 0x86 >> >> Fixes: 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") >> Cc: Jeremy Linton <jeremy.linton@arm.com> >> Cc Graeme Gregory <graeme.gregory@linaro.org> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> drivers/net/ethernet/smsc/smsc911x.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c >> index 0f21aa3bb537..34f97684506b 100644 >> --- a/drivers/net/ethernet/smsc/smsc911x.c >> +++ b/drivers/net/ethernet/smsc/smsc911x.c >> @@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops = { >> static int smsc911x_probe_config(struct smsc911x_platform_config *config, >> struct device *dev) >> { >> + int phy_interface; >> u32 width = 0; >> >> if (!dev) >> return -ENODEV; >> >> - config->phy_interface = device_get_phy_mode(dev); >> + phy_interface = device_get_phy_mode(dev); >> + if (phy_interface < 0) >> + return phy_interface; >> + >> + config->phy_interface = phy_interface; >> >> device_get_mac_address(dev, config->mac, ETH_ALEN); > > Looks like this change makes at least omap boards using smsc911x > fail with -22 for me in Linux next. > > Do any of the the device tree configured smsc911x devices actually > have a phy configured? > Ok, this is more subtle than I thought. Previously, the code would not attempt any devicetree configuration if devicetree was not configured. Now it does. The error return from device_get_phy_mode() isn't the actual problem. Apparently it doesn't really matter if a nonsensical value is assigned to phy_interface. The problem is that the reg-io-width property is obviously not present in the non-dt and non-acpi case. This overwrites the existing platform data configuration and selects 16 bit mode, to which the (simulated) hardware obviously reacts less than enthusiastic. Fixing this properly won't be easy. If the "reg-io-width" property is not present or wrong, the default register width is 16 bit. Obviously, if neither DT nor ACPI is available, it won't be present. This causes the crash I had observed. Bad part is that there does not seem to be a reliable means to detect that platform data should be used in that situation. Other device_get_XXX functions return -ENXIO if that happens, but not device_property_read_u32(). It is _supposed_ to return it per its API, but it doesn't (it returns -ENODATA). We may need two separate patches, one to fix up device_property_read_u32() to return -ENXIO, and one to fix smsc911x_probe_config() to ignore the error from device_get_phy_mode(), and to bail out if device_property_read_u32() returns -ENXIO. The simpler alternative would be to check the return value from device_property_read_u32() for both -ENXIO and -ENODATA. This would make the code independent of the necessary core changes (which may take a while). I tested this variant, and it works, at least for the non-DT case. Does this make sense ? Thanks, Guenter -- 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
* Guenter Roeck <linux@roeck-us.net> [150826 11:37]: > On 08/26/2015 10:04 AM, Tony Lindgren wrote: > >Hi, > > > >* Guenter Roeck <linux@roeck-us.net> [150817 13:48]: > >>Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes > >>the call to smsc911x_probe_config() unconditional, and no longer fails if > >>there is no device node. device_get_phy_mode() is called unconditionally, > >>and if there is no phy node configured returns an error code. This error > >>code is assigned to phy_interface, and interpreted elsewhere in the code > >>as valid phy mode. This in turn causes qemu to crash when running a > >>variant of realview_pb_defconfig. > >> > >> qemu: hardware error: lan9118_read: Bad reg 0x86 > >> > >>Fixes: 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") > >>Cc: Jeremy Linton <jeremy.linton@arm.com> > >>Cc Graeme Gregory <graeme.gregory@linaro.org> > >>Signed-off-by: Guenter Roeck <linux@roeck-us.net> > >>--- > >> drivers/net/ethernet/smsc/smsc911x.c | 7 ++++++- > >> 1 file changed, 6 insertions(+), 1 deletion(-) > >> > >>diff --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c > >>index 0f21aa3bb537..34f97684506b 100644 > >>--- a/drivers/net/ethernet/smsc/smsc911x.c > >>+++ b/drivers/net/ethernet/smsc/smsc911x.c > >>@@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops = { > >> static int smsc911x_probe_config(struct smsc911x_platform_config *config, > >> struct device *dev) > >> { > >>+ int phy_interface; > >> u32 width = 0; > >> > >> if (!dev) > >> return -ENODEV; > >> > >>- config->phy_interface = device_get_phy_mode(dev); > >>+ phy_interface = device_get_phy_mode(dev); > >>+ if (phy_interface < 0) > >>+ return phy_interface; > >>+ > >>+ config->phy_interface = phy_interface; > >> > >> device_get_mac_address(dev, config->mac, ETH_ALEN); > > > >Looks like this change makes at least omap boards using smsc911x > >fail with -22 for me in Linux next. > > > >Do any of the the device tree configured smsc911x devices actually > >have a phy configured? > > > > Ok, this is more subtle than I thought. > > Previously, the code would not attempt any devicetree configuration > if devicetree was not configured. > > Now it does. > > The error return from device_get_phy_mode() isn't the actual problem. > Apparently it doesn't really matter if a nonsensical value is assigned > to phy_interface. > > The problem is that the reg-io-width property is obviously not present > in the non-dt and non-acpi case. This overwrites the existing platform data > configuration and selects 16 bit mode, to which the (simulated) hardware > obviously reacts less than enthusiastic. > > Fixing this properly won't be easy. If the "reg-io-width" property > is not present or wrong, the default register width is 16 bit. Obviously, > if neither DT nor ACPI is available, it won't be present. This causes > the crash I had observed. Heh OK :) > Bad part is that there does not seem to be a reliable means to detect > that platform data should be used in that situation. Other device_get_XXX > functions return -ENXIO if that happens, but not device_property_read_u32(). > It is _supposed_ to return it per its API, but it doesn't (it returns > -ENODATA). > > We may need two separate patches, one to fix up device_property_read_u32() > to return -ENXIO, and one to fix smsc911x_probe_config() to ignore the error > from device_get_phy_mode(), and to bail out if device_property_read_u32() > returns -ENXIO. I guess the device_property_read_u32() change needs to be discussed separately.. So probably best to fix up the regression to smsc911x first. > The simpler alternative would be to check the return value from > device_property_read_u32() for both -ENXIO and -ENODATA. > This would make the code independent of the necessary core changes > (which may take a while). I tested this variant, and it works, at least > for the non-DT case. > > Does this make sense ? Yeh I think that would allow fixing up the smsc911x regression while discussing the device_property_read_u32() change. Got a test patch for me to try? Regards, Tony -- 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
Hi Tony, On 08/26/2015 01:16 PM, Tony Lindgren wrote: [ ... ] >> We may need two separate patches, one to fix up device_property_read_u32() >> to return -ENXIO, and one to fix smsc911x_probe_config() to ignore the error >> from device_get_phy_mode(), and to bail out if device_property_read_u32() >> returns -ENXIO. > > I guess the device_property_read_u32() change needs to be discussed > separately.. So probably best to fix up the regression to smsc911x > first. > Not sure myself. Jeremy has a point - we don't really know for sure how safe it is to check for -ENODATA (in addition to -ENXIO). Also, fixing device_property_read_u32() turned out to be much easier than I thought. >> The simpler alternative would be to check the return value from >> device_property_read_u32() for both -ENXIO and -ENODATA. >> This would make the code independent of the necessary core changes >> (which may take a while). I tested this variant, and it works, at least >> for the non-DT case. >> >> Does this make sense ? > > Yeh I think that would allow fixing up the smsc911x regression while > discussing the device_property_read_u32() change. Got a test patch > for me to try? > You should have two by now to choose from. Guenter -- 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
* Guenter Roeck <linux@roeck-us.net> [150826 13:58]: > Hi Tony, > > On 08/26/2015 01:16 PM, Tony Lindgren wrote: > [ ... ] > > >>We may need two separate patches, one to fix up device_property_read_u32() > >>to return -ENXIO, and one to fix smsc911x_probe_config() to ignore the error > >>from device_get_phy_mode(), and to bail out if device_property_read_u32() > >>returns -ENXIO. > > > >I guess the device_property_read_u32() change needs to be discussed > >separately.. So probably best to fix up the regression to smsc911x > >first. > > > Not sure myself. Jeremy has a point - we don't really know for sure how > safe it is to check for -ENODATA (in addition to -ENXIO). Also, fixing > device_property_read_u32() turned out to be much easier than I thought. > > >>The simpler alternative would be to check the return value from > >>device_property_read_u32() for both -ENXIO and -ENODATA. > >>This would make the code independent of the necessary core changes > >>(which may take a while). I tested this variant, and it works, at least > >>for the non-DT case. > >> > >>Does this make sense ? > > > >Yeh I think that would allow fixing up the smsc911x regression while > >discussing the device_property_read_u32() change. Got a test patch > >for me to try? > > > > You should have two by now to choose from. Acked the second version thanks :) Tony -- 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 --git a/drivers/net/ethernet/smsc/smsc911x.c b/drivers/net/ethernet/smsc/smsc911x.c index 0f21aa3bb537..34f97684506b 100644 --- a/drivers/net/ethernet/smsc/smsc911x.c +++ b/drivers/net/ethernet/smsc/smsc911x.c @@ -2367,12 +2367,17 @@ static const struct smsc911x_ops shifted_smsc911x_ops = { static int smsc911x_probe_config(struct smsc911x_platform_config *config, struct device *dev) { + int phy_interface; u32 width = 0; if (!dev) return -ENODEV; - config->phy_interface = device_get_phy_mode(dev); + phy_interface = device_get_phy_mode(dev); + if (phy_interface < 0) + return phy_interface; + + config->phy_interface = phy_interface; device_get_mac_address(dev, config->mac, ETH_ALEN);
Commit 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") makes the call to smsc911x_probe_config() unconditional, and no longer fails if there is no device node. device_get_phy_mode() is called unconditionally, and if there is no phy node configured returns an error code. This error code is assigned to phy_interface, and interpreted elsewhere in the code as valid phy mode. This in turn causes qemu to crash when running a variant of realview_pb_defconfig. qemu: hardware error: lan9118_read: Bad reg 0x86 Fixes: 0b50dc4fc971 ("Convert smsc911x to use ACPI as well as DT") Cc: Jeremy Linton <jeremy.linton@arm.com> Cc Graeme Gregory <graeme.gregory@linaro.org> Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/net/ethernet/smsc/smsc911x.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)