Stijn, If you could provide me the details of tweakings that you have made, it would help me to improve the driver.
Murali Karicheri Software Design Engineer Texas Instruments Inc. Germantown, MD 20874 new phone: 301-407-9583 Old Phone : 301-515-3736 (will be deprecated) email: m-kariche...@ti.com ________________________________ From: Stijn Devriendt [mailto:high...@gmail.com] Sent: Saturday, August 15, 2009 2:19 AM To: Karicheri, Muralidharan; Davinci-linux-open-source@linux.davincidsp.com Subject: Re: [PATCH v1 - 3/5] V4L : vpif capture driver for DM6467 This code is so EVM specific, instead of SOC-specific. In order to connect 2 DM646x chips directly via VPIF this driver requires quite a bit of tweaking. (I know because I had to do this before ;) ) Please share the details so that I could explain if that is an issue with this driver and if so, we could update the driver with another patch to take care of the same. I'd appreciate a more loosely coupled subdevice layer. The sub device layer is implemented using v4l2 sub device framework. If you check all of the video drivers that uses sub device framework, you would see similar implementation. There is a well defined interface for bridge driver to interface with sub devices. So I don't understand what is your concern here. I suggest you discuss your concern in the linux-media mailing list. VPIF itself doesn't use I2C so this stuff should belong elsewhere. The driver consists of 2 parts. vpif_capture.c has the bridge driver. As per sub device framework, the bridge driver is aware of all sub devices it works with. The i2c adapter information is required to load up an i2c sub device. So we can't help on that. If the sub device is not i2c specific, then there are other APIs to load up the module. So this driver will be able to work any sub device that implements the v4l2 sub device APIs. The sub device part which in this is tvp514x module. It is written for v4l2 sub device interface. If you could discuss specific issues, it will be easy to understand your concerns. Secondly I can imagine any board other than the EVM uses other codec chips (either I2C or not). So basically the whole codec stuff would preferable be split out of the core VPIF driver. Not sure what you mean here. Can you clarify what codec part you are talking about? Regards, Stijn +/** + * vpif_probe : This function probes the vpif capture driver + * @pdev: platform device pointer + * + * This creates device entries by register itself to the V4L2 driver and + * initializes fields of each channel objects + */ +static __init int vpif_probe(struct platform_device *pdev) +{ + struct vpif_subdev_info *subdevdata; + struct vpif_capture_config *config; + int i, j, k, m, q, err; + struct i2c_adapter *i2c_adap; + struct channel_obj *ch; + struct common_obj *common; + struct video_device *vfd; + struct resource *res; + int subdev_count; + + vpif_dev = &pdev->dev; + + err = initialize_vpif(); + if (err) { + v4l2_err(vpif_dev->driver, "Error initializing vpif\n"); + return err; + } + + k = 0; + while ((res = platform_get_resource(pdev, IORESOURCE_IRQ, k))) { + for (i = res->start; i <= res->end; i++) { + if (request_irq(i, vpif_channel_isr, IRQF_DISABLED, + "DM646x_Capture", + (void *)(&vpif_obj.dev[k]->channel_id))) { + err = -EBUSY; + i--; + goto vpif_int_err; + } + } + k++; + } + + for (i = 0; i < VPIF_CAPTURE_MAX_DEVICES; i++) { + /* Get the pointer to the channel object */ + ch = vpif_obj.dev[i]; + /* Allocate memory for video device */ + vfd = video_device_alloc(); + if (NULL == vfd) { + for (j = 0; j < i; j++) { + ch = vpif_obj.dev[j]; + video_device_release(ch->video_dev); + } + err = -ENOMEM; + goto vpif_dev_alloc_err; + } + + /* Initialize field of video device */ + *vfd = vpif_video_template; + vfd->v4l2_dev = &vpif_obj.v4l2_dev; + vfd->release = video_device_release; + snprintf(vfd->name, sizeof(vfd->name), + "DM646x_VPIFCapture_DRIVER_V%d.%d.%d", + (VPIF_CAPTURE_VERSION_CODE >> 16) & 0xff, + (VPIF_CAPTURE_VERSION_CODE >> 8) & 0xff, + (VPIF_CAPTURE_VERSION_CODE) & 0xff); + /* Set video_dev to the video device */ + ch->video_dev = vfd; + } + + for (j = 0; j < VPIF_CAPTURE_MAX_DEVICES; j++) { + ch = vpif_obj.dev[j]; + ch->channel_id = j; + common = &(ch->common[VPIF_VIDEO_INDEX]); + spin_lock_init(&common->irqlock); + mutex_init(&common->lock); + /* Initialize prio member of channel object */ + v4l2_prio_init(&ch->prio); + err = video_register_device(ch->video_dev, + VFL_TYPE_GRABBER, (j ? 1 : 0)); + if (err) + goto probe_out; + + video_set_drvdata(ch->video_dev, ch); + + } + + i2c_adap = i2c_get_adapter(1); + config = pdev->dev.platform_data; + + subdev_count = config->subdev_count; + vpif_obj.sd<http://vpif_obj.sd> = kmalloc(sizeof(struct v4l2_subdev *) * subdev_count, + GFP_KERNEL); + if (vpif_obj.sd<http://vpif_obj.sd> == NULL) { + vpif_err("unable to allocate memory for subdevice pointers\n"); + err = -ENOMEM; + goto probe_out; + } + + err = v4l2_device_register(vpif_dev, &vpif_obj.v4l2_dev); + if (err) { + v4l2_err(vpif_dev->driver, "Error registering v4l2 device\n"); + goto probe_subdev_out; + } + + for (i = 0; i < subdev_count; i++) { + subdevdata = &config->subdev_info[i]; + vpif_obj.sd<http://vpif_obj.sd>[i] = + v4l2_i2c_new_subdev_board(&vpif_obj.v4l2_dev, + i2c_adap, + subdevdata->name, + &subdevdata->board_info, + NULL); + + if (!vpif_obj.sd<http://vpif_obj.sd>[i]) { + vpif_err("Error registering v4l2 subdevice\n"); + goto probe_subdev_out; + } + v4l2_info(&vpif_obj.v4l2_dev, "registered sub device %s\n", + subdevdata->name); + + if (vpif_obj.sd<http://vpif_obj.sd>[i]) + vpif_obj.sd<http://vpif_obj.sd>[i]->grp_id = 1 << i; + } + v4l2_info(&vpif_obj.v4l2_dev, "DM646x VPIF Capture driver" + " initialized\n"); + + return 0; + +probe_subdev_out: + /* free sub devices memory */ + kfree(vpif_obj.sd<http://vpif_obj.sd>); + + j = VPIF_CAPTURE_MAX_DEVICES; +probe_out: + v4l2_device_unregister(&vpif_obj.v4l2_dev); + for (k = 0; k < j; k++) { + /* Get the pointer to the channel object */ + ch = vpif_obj.dev[k]; + /* Unregister video device */ + video_unregister_device(ch->video_dev); + } + +vpif_dev_alloc_err: + k = VPIF_CAPTURE_MAX_DEVICES-1; + res = platform_get_resource(pdev, IORESOURCE_IRQ, k); + i = res->end; + +vpif_int_err: + for (q = k; q >= 0; q--) { + for (m = i; m >= (int)res->start; m--) + free_irq(m, (void *)(&vpif_obj.dev[q]->channel_id)); + + res = platform_get_resource(pdev, IORESOURCE_IRQ, q-1); + if (res) + i = res->end; + } + return err; +} +
_______________________________________________ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
