[PATCH v4] staging: usbip: replace pr_warning() with dev_warn().

[PATCH v4] staging: usbip: replace pr_warning() with dev_warn().

[navin patidar]

dev_warn() is preferred over pr_warning().

container_of() is used to get usb_driver pointer from usbip_device container
(stub_device or vhci_device), to get device structure required for dev_warn().

Signed-off-by: navin patidar <navinp@cdac.in>
---
 drivers/staging/usbip/usbip_event.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/usbip/usbip_event.c b/drivers/staging/usbip/usbip_event.c
index 82123be..1f3a571 100644
--- a/drivers/staging/usbip/usbip_event.c
+++ b/drivers/staging/usbip/usbip_event.c
@@ -21,6 +21,8 @@
 #include <linux/export.h>

 #include "usbip_common.h"
+#include "stub.h"
+#include "vhci.h"

 static int event_handler(struct usbip_device *ud)
 {
@@ -85,7 +87,14 @@ int usbip_start_eh(struct usbip_device *ud)

 	ud->eh = kthread_run(event_handler_loop, ud, "usbip_eh");
 	if (IS_ERR(ud->eh)) {
-		pr_warning("Unable to start control thread\n");
+		struct device *dev;
+
+		if (ud->side == USBIP_STUB)
+			dev = &container_of(ud, struct stub_device, ud)->udev->dev;
+		else
+			dev = &container_of(ud, struct vhci_device, ud)->udev->dev;
+
+		dev_warn(dev, "Unable to start control thread\n");
 		return PTR_ERR(ud->eh);
 	}

--
1.7.10.4

Re: [PATCH v4] staging: usbip: replace pr_warning() with dev_warn().

[Greg KH]

On Thu, Jun 27, 2013 at 03:34:52PM +0530, navin patidar wrote:
> dev_warn() is preferred over pr_warning().
> 
> container_of() is used to get usb_driver pointer from usbip_device container
> (stub_device or vhci_device), to get device structure required for dev_warn().
> 
> Signed-off-by: navin patidar <navinp@cdac.in>
> ---
>  drivers/staging/usbip/usbip_event.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/usbip/usbip_event.c b/drivers/staging/usbip/usbip_event.c
> index 82123be..1f3a571 100644
> --- a/drivers/staging/usbip/usbip_event.c
> +++ b/drivers/staging/usbip/usbip_event.c
> @@ -21,6 +21,8 @@
>  #include <linux/export.h>
> 
>  #include "usbip_common.h"
> +#include "stub.h"
> +#include "vhci.h"
> 
>  static int event_handler(struct usbip_device *ud)
>  {
> @@ -85,7 +87,14 @@ int usbip_start_eh(struct usbip_device *ud)
> 
>  	ud->eh = kthread_run(event_handler_loop, ud, "usbip_eh");
>  	if (IS_ERR(ud->eh)) {
> -		pr_warning("Unable to start control thread\n");
> +		struct device *dev;
> +
> +		if (ud->side == USBIP_STUB)
> +			dev = &container_of(ud, struct stub_device, ud)->udev->dev;
> +		else
> +			dev = &container_of(ud, struct vhci_device, ud)->udev->dev;

Putting '&' in front of container_of seems odd, are you sure it's needed
here?  If ud is a pointer, everything else should "just work" properly
without that.

Can you please fix this up?

thanks,

greg k-h

Re: [PATCH v4] staging: usbip: replace pr_warning() with dev_warn().

[navin patidar]

On Wed, Jul 24, 2013, Greg KH <gregkh@linuxfoundation.org> said:

> On Thu, Jun 27, 2013 at 03:34:52PM +0530, navin patidar wrote:
>> dev_warn() is preferred over pr_warning().
>>
>> container_of() is used to get usb_driver pointer from usbip_device container
>> (stub_device or vhci_device), to get device structure required for dev_warn().
>>
>> Signed-off-by: navin patidar <navinp@cdac.in>
>> ---
>>  drivers/staging/usbip/usbip_event.c |   11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/usbip/usbip_event.c b/drivers/staging/usbip/usbip_event.c
>> index 82123be..1f3a571 100644
>> --- a/drivers/staging/usbip/usbip_event.c
>> +++ b/drivers/staging/usbip/usbip_event.c
>> @@ -21,6 +21,8 @@
>>  #include <linux/export.h>
>>
>>  #include "usbip_common.h"
>> +#include "stub.h"
>> +#include "vhci.h"
>>
>>  static int event_handler(struct usbip_device *ud)
>>  {
>> @@ -85,7 +87,14 @@ int usbip_start_eh(struct usbip_device *ud)
>>
>>      ud->eh = kthread_run(event_handler_loop, ud, "usbip_eh");
>>      if (IS_ERR(ud->eh)) {
>> -        pr_warning("Unable to start control thread\n");
>> +        struct device *dev;
>> +
>> +        if (ud->side == USBIP_STUB)
>> +            dev = &container_of(ud, struct stub_device, ud)->udev->dev;
>> +        else
>> +            dev = &container_of(ud, struct vhci_device, ud)->udev->dev;
>
> Putting '&' in front of container_of seems odd, are you sure it's needed
> here?  If ud is a pointer, everything else should "just work" properly
> without that.

Removing '&'   caused following error.
drivers/staging/usbip/usbip_event.c: In function ‘usbip_start_eh’:
drivers/staging/usbip/usbip_event.c:93:8: error: incompatible types
when assigning to type ‘struct device *’ from type ‘struct device’
drivers/staging/usbip/usbip_event.c:95:8: error: incompatible types
when assigning to type ‘struct device *’ from type ‘struct device’

dev needs to be initialized with address of dev (struct device ) which
is member of struct usb_device pointed by the udev.

To make it more clear i can change it to

dev = &(container_of(ud, struct vhci_device, ud)->udev->dev);

 or

struct vhci_device *vdev = container_of(ud, struct vhci_device, ud);
dev = &(vdev->udev->dev);

regards,
--navin-patidar

Re: [PATCH v4] staging: usbip: replace pr_warning() with dev_warn().

[Greg KH]

On Thu, Jul 25, 2013 at 10:19:31AM +0530, navin patidar wrote:
> >> -        pr_warning("Unable to start control thread\n");
> >> +        struct device *dev;
> >> +
> >> +        if (ud->side == USBIP_STUB)
> >> +            dev = &container_of(ud, struct stub_device, ud)->udev->dev;
> >> +        else
> >> +            dev = &container_of(ud, struct vhci_device, ud)->udev->dev;
> >
> > Putting '&' in front of container_of seems odd, are you sure it's needed
> > here?  If ud is a pointer, everything else should "just work" properly
> > without that.
> 
> Removing '&'   caused following error.
> drivers/staging/usbip/usbip_event.c: In function ‘usbip_start_eh’:
> drivers/staging/usbip/usbip_event.c:93:8: error: incompatible types
> when assigning to type ‘struct device *’ from type ‘struct device’
> drivers/staging/usbip/usbip_event.c:95:8: error: incompatible types
> when assigning to type ‘struct device *’ from type ‘struct device’
> 
> dev needs to be initialized with address of dev (struct device ) which
> is member of struct usb_device pointed by the udev.
> 
> To make it more clear i can change it to
> 
> dev = &(container_of(ud, struct vhci_device, ud)->udev->dev);
> 
>  or
> 
> struct vhci_device *vdev = container_of(ud, struct vhci_device, ud);
> dev = &(vdev->udev->dev);

Or perhaps:
	dev = container_of(ud, struct stub_device, ud).udev->dev;

or ->udev.dev; I don't remember which, but that should work, right?

greg k-h

Re: [PATCH v4] staging: usbip: replace pr_warning() with dev_warn().

[navin patidar]

On Thu, Jul 25, 2013, Greg KH <gregkh@linuxfoundation.org> said:

> On Thu, Jul 25, 2013 at 10:19:31AM +0530, navin patidar wrote:
>> >> -        pr_warning("Unable to start control thread\n");
>> >> +        struct device *dev;
>> >> +
>> >> +        if (ud->side == USBIP_STUB)
>> >> +            dev = &container_of(ud, struct stub_device, ud)->udev->dev;
>> >> +        else
>> >> +            dev = &container_of(ud, struct vhci_device, ud)->udev->dev;
>> >
>> > Putting '&' in front of container_of seems odd, are you sure it's needed
>> > here?  If ud is a pointer, everything else should "just work" properly
>> > without that.
>>
>> Removing '&'   caused following error.
>> drivers/staging/usbip/usbip_event.c: In function ‘usbip_start_eh’:
>> drivers/staging/usbip/usbip_event.c:93:8: error: incompatible types
>> when assigning to type ‘struct device *’ from type ‘struct device’
>> drivers/staging/usbip/usbip_event.c:95:8: error: incompatible types
>> when assigning to type ‘struct device *’ from type ‘struct device’
>>
>> dev needs to be initialized with address of dev (struct device ) which
>> is member of struct usb_device pointed by the udev.
>>
>> To make it more clear i can change it to
>>
>> dev = &(container_of(ud, struct vhci_device, ud)->udev->dev);
>>
>>  or
>>
>> struct vhci_device *vdev = container_of(ud, struct vhci_device, ud);
>> dev = &(vdev->udev->dev);
>
> Or perhaps:
>     dev = container_of(ud, struct stub_device, ud).udev->dev;

 container_of() returns stub_device pointer so "container_of(ud,
struct stub_device, ud).udev->dev"  won't  work.

> or ->udev.dev; I don't remember which, but that should work, right?
udev is also a pointer to usb_device structure inside  stub_device structure.
    ->udev->dev   only will work.

v4 patch gets compiled without any error or warning.
and actually Joe Perches reviewed v3 of the patch and suggested changes.
https://lkml.org/lkml/2013/6/27/15

regards,
--navin-patidar