Re: [PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist

[Greg Kroah-Hartman]

On Thu, May 21, 2015 at 04:57:52PM +0800, Lu Baolu wrote:
 ULPI registers its bus at module_init, so if the bus fails to register, the
 module will fail to load and all will be well in the world.
 
 However, if the ULPI code is built-in rather than a module, the bus
 initialization may fail, but we'd still try to register drivers later onto
 a non-existent bus, which will panic the kernel.
 
 Fix that by checking that the bus was indeed initialized before trying to
 register drivers on top of it.
 
 Signed-off-by: Sasha Levin sasha.le...@oracle.com
 Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
 Signed-off-by: Lu Baolu baolu...@linux.intel.com
 Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
 Reviewed-by: David Cohen david.a.co...@linux.intel.com
 ---
  drivers/usb/common/ulpi.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
 index 0e6f968..af52b46 100644
 --- a/drivers/usb/common/ulpi.c
 +++ b/drivers/usb/common/ulpi.c
 @@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv)
   if (!drv-probe)
   return -EINVAL;
  
 + /* Was the bus registered successfully? */
 + if (unlikely(WARN_ON(!ulpi_bus.p)))

never use unlikely/likely unless you can actually measure the difference
if the marking is not used.

Hint, on driver probe time, you can't.

thanks,

greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist

[Lu Baolu]

ULPI registers its bus at module_init, so if the bus fails to register, the
module will fail to load and all will be well in the world.

However, if the ULPI code is built-in rather than a module, the bus
initialization may fail, but we'd still try to register drivers later onto
a non-existent bus, which will panic the kernel.

Fix that by checking that the bus was indeed initialized before trying to
register drivers on top of it.

Signed-off-by: Sasha Levin sasha.le...@oracle.com
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
Reviewed-by: David Cohen david.a.co...@linux.intel.com
---
 drivers/usb/common/ulpi.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..af52b46 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv)
if (!drv-probe)
return -EINVAL;
 
+   /* Was the bus registered successfully? */
+   if (unlikely(WARN_ON(!ulpi_bus.p)))
+   return -ENODEV;
+
drv-driver.bus = ulpi_bus;
 
return driver_register(drv-driver);
@@ -242,7 +246,7 @@ static int __init ulpi_init(void)
 {
return bus_register(ulpi_bus);
 }
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
 
 static void __exit ulpi_exit(void)
 {
-- 
2.1.4

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

Re: [PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist

[Lu, Baolu]

Hi Heikki, Sasha and others,

Please ignore this patch. I should not squash these two patches into one and
sign it off behalf of other people. I apologize for this and I'm sorry 
if this lets

you feel affronted.

Thanks,
Baolu


On 05/21/2015 04:57 PM, Lu Baolu wrote:

ULPI registers its bus at module_init, so if the bus fails to register, the
module will fail to load and all will be well in the world.

However, if the ULPI code is built-in rather than a module, the bus
initialization may fail, but we'd still try to register drivers later onto
a non-existent bus, which will panic the kernel.

Fix that by checking that the bus was indeed initialized before trying to
register drivers on top of it.

Signed-off-by: Sasha Levin sasha.le...@oracle.com
Signed-off-by: Heikki Krogerus heikki.kroge...@linux.intel.com
Signed-off-by: Lu Baolu baolu...@linux.intel.com
Reported-by: Zhuo Qiuxu qiuxu.z...@intel.com
Reviewed-by: David Cohen david.a.co...@linux.intel.com
---
  drivers/usb/common/ulpi.c | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/common/ulpi.c b/drivers/usb/common/ulpi.c
index 0e6f968..af52b46 100644
--- a/drivers/usb/common/ulpi.c
+++ b/drivers/usb/common/ulpi.c
@@ -132,6 +132,10 @@ int ulpi_register_driver(struct ulpi_driver *drv)
if (!drv-probe)
return -EINVAL;
  
+	/* Was the bus registered successfully? */

+   if (unlikely(WARN_ON(!ulpi_bus.p)))
+   return -ENODEV;
+
drv-driver.bus = ulpi_bus;
  
  	return driver_register(drv-driver);

@@ -242,7 +246,7 @@ static int __init ulpi_init(void)
  {
return bus_register(ulpi_bus);
  }
-module_init(ulpi_init);
+subsys_initcall(ulpi_init);
  
  static void __exit ulpi_exit(void)

  {


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

Re: [PATCH 1/1] usb: ulpi: don't register drivers if bus doesn't exist

[Heikki Krogerus]

On Thu, May 21, 2015 at 06:07:25PM +0800, Lu, Baolu wrote:
 Hi Heikki, Sasha and others,
 
 Please ignore this patch. I should not squash these two patches into one and
 sign it off behalf of other people. I apologize for this and I'm sorry if
 this lets
 you feel affronted.

No worries. So we'll use patch from Sasha to fix the panic when the
bus is missing, and your's to fix the execution order.

If you prepare v2 of your original patch, I'll ACK it.


Thanks,

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