diff mbox

[PATCHv2,12/14,media] cx231xx: use dev_info() for extension load/unload

Message ID 58369096f1fee7d71942eae7a40db6d7c1c368bf.1414929816.git.mchehab@osg.samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mauro Carvalho Chehab Nov. 2, 2014, 12:32 p.m. UTC
Now that we're using dev_foo, the logs become like:

	usb 1-2: DVB: registering adapter 0 frontend 0 (Fujitsu mb86A20s)...
	usb 1-2: Successfully loaded cx231xx-dvb
	cx231xx: Cx231xx dvb Extension initialized

It is not clear, by the logs, that usb 1-2 name is an alias for
cx231xx. So, we also need to use dvb_info() at extension load/unload.

After the patch, it will print:
	usb 1-2: Cx231xx dvb Extension initialized

With is coherent with the other logs.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

Comments

Antti Palosaari Nov. 2, 2014, 5:32 p.m. UTC | #1
On 11/02/2014 02:32 PM, Mauro Carvalho Chehab wrote:
> Now that we're using dev_foo, the logs become like:
>
> 	usb 1-2: DVB: registering adapter 0 frontend 0 (Fujitsu mb86A20s)...
> 	usb 1-2: Successfully loaded cx231xx-dvb
> 	cx231xx: Cx231xx dvb Extension initialized
>
> It is not clear, by the logs, that usb 1-2 name is an alias for
> cx231xx. So, we also need to use dvb_info() at extension load/unload.
>
> After the patch, it will print:
> 	usb 1-2: Cx231xx dvb Extension initialized
>
> With is coherent with the other logs.


That is not correct as wrong device pointer passed to dev_. Go cx231xx 
usb driver probe function and add following test log to see how is 
should look like:
dev_info(&intf->dev, "Hello World\n");

It prints something like
cx231xx 1-2-1: Hello World

regards
Antti

>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/drivers/media/usb/cx231xx/cx231xx-core.c b/drivers/media/usb/cx231xx/cx231xx-core.c
> index 36c3ecf204c1..64e907f02a02 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-core.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-core.c
> @@ -98,10 +98,10 @@ int cx231xx_register_extension(struct cx231xx_ops *ops)
>
>   	mutex_lock(&cx231xx_devlist_mutex);
>   	list_add_tail(&ops->next, &cx231xx_extension_devlist);
> -	list_for_each_entry(dev, &cx231xx_devlist, devlist)
> +	list_for_each_entry(dev, &cx231xx_devlist, devlist) {
>   		ops->init(dev);
> -
> -	printk(KERN_INFO DRIVER_NAME ": %s initialized\n", ops->name);
> +		dev_info(&dev->udev->dev, "%s initialized\n", ops->name);
> +	}
>   	mutex_unlock(&cx231xx_devlist_mutex);
>   	return 0;
>   }
> @@ -112,11 +112,11 @@ void cx231xx_unregister_extension(struct cx231xx_ops *ops)
>   	struct cx231xx *dev = NULL;
>
>   	mutex_lock(&cx231xx_devlist_mutex);
> -	list_for_each_entry(dev, &cx231xx_devlist, devlist)
> +	list_for_each_entry(dev, &cx231xx_devlist, devlist) {
>   		ops->fini(dev);
> +		dev_info(&dev->udev->dev, "%s removed\n", ops->name);
> +	}
>
> -
> -	printk(KERN_INFO DRIVER_NAME ": %s removed\n", ops->name);
>   	list_del(&ops->next);
>   	mutex_unlock(&cx231xx_devlist_mutex);
>   }
>
Mauro Carvalho Chehab Nov. 3, 2014, 8:51 a.m. UTC | #2
Em Sun, 02 Nov 2014 19:32:37 +0200
Antti Palosaari <crope@iki.fi> escreveu:

> 
> 
> On 11/02/2014 02:32 PM, Mauro Carvalho Chehab wrote:
> > Now that we're using dev_foo, the logs become like:
> >
> > 	usb 1-2: DVB: registering adapter 0 frontend 0 (Fujitsu mb86A20s)...
> > 	usb 1-2: Successfully loaded cx231xx-dvb
> > 	cx231xx: Cx231xx dvb Extension initialized
> >
> > It is not clear, by the logs, that usb 1-2 name is an alias for
> > cx231xx. So, we also need to use dvb_info() at extension load/unload.
> >
> > After the patch, it will print:
> > 	usb 1-2: Cx231xx dvb Extension initialized
> >
> > With is coherent with the other logs.
> 
> 
> That is not correct as wrong device pointer passed to dev_. Go cx231xx 
> usb driver probe function and add following test log to see how is 
> should look like:
> dev_info(&intf->dev, "Hello World\n");

I changed the probe to be:

static int cx231xx_usb_probe(struct usb_interface *interface,
                             const struct usb_device_id *id)
{
        struct usb_device *udev;
	...
        struct usb_interface_assoc_descriptor *assoc_desc;

        udev = usb_get_dev(interface_to_usbdev(interface));
        ifnum = interface->altsetting[0].desc.bInterfaceNumber;

        dev_info(&interface->dev, "intf Hello World\n");
        dev_info(&udev->dev, "udev Hello World\n");

The result is:

[54915.036082] cx231xx 1-2:1.2: intf Hello World
[54915.036090] usb 1-2: udev Hello World
[54915.036163] cx231xx 1-2:1.3: intf Hello World
[54915.036171] usb 1-2: udev Hello World
[54915.036197] cx231xx 1-2:1.4: intf Hello World
[54915.036204] usb 1-2: udev Hello World
[54915.036228] cx231xx 1-2:1.5: intf Hello World
[54915.036234] usb 1-2: udev Hello World
[54915.036258] cx231xx 1-2:1.6: intf Hello World
[54915.036264] usb 1-2: udev Hello World

Devices with multiple interfaces seem to have intf->dev filled with
a different device than udev->dev. The cx231xx is likely the most
complex device we have at media, as it has lots of interfaces, each
with lots of alternates, plus its 3 I2C physical buses and one I2C
mux internally.

I may work on a patch that would be storing intf->dev into
cx231xx dev struct. That's probably the easiest way for this
device.

Do you have any other idea?

Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" 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/drivers/media/usb/cx231xx/cx231xx-core.c b/drivers/media/usb/cx231xx/cx231xx-core.c
index 36c3ecf204c1..64e907f02a02 100644
--- a/drivers/media/usb/cx231xx/cx231xx-core.c
+++ b/drivers/media/usb/cx231xx/cx231xx-core.c
@@ -98,10 +98,10 @@  int cx231xx_register_extension(struct cx231xx_ops *ops)
 
 	mutex_lock(&cx231xx_devlist_mutex);
 	list_add_tail(&ops->next, &cx231xx_extension_devlist);
-	list_for_each_entry(dev, &cx231xx_devlist, devlist)
+	list_for_each_entry(dev, &cx231xx_devlist, devlist) {
 		ops->init(dev);
-
-	printk(KERN_INFO DRIVER_NAME ": %s initialized\n", ops->name);
+		dev_info(&dev->udev->dev, "%s initialized\n", ops->name);
+	}
 	mutex_unlock(&cx231xx_devlist_mutex);
 	return 0;
 }
@@ -112,11 +112,11 @@  void cx231xx_unregister_extension(struct cx231xx_ops *ops)
 	struct cx231xx *dev = NULL;
 
 	mutex_lock(&cx231xx_devlist_mutex);
-	list_for_each_entry(dev, &cx231xx_devlist, devlist)
+	list_for_each_entry(dev, &cx231xx_devlist, devlist) {
 		ops->fini(dev);
+		dev_info(&dev->udev->dev, "%s removed\n", ops->name);
+	}
 
-
-	printk(KERN_INFO DRIVER_NAME ": %s removed\n", ops->name);
 	list_del(&ops->next);
 	mutex_unlock(&cx231xx_devlist_mutex);
 }