Hi Manjunath,

Thanks for the patch.

On Tuesday 17 April 2012 14:22:59 Manjunath Hadli wrote:
> As the same interrupt is shared between capture and display devices,
> sometimes we get isr calls where the interrupt might not genuinely belong
> to capture or display. Hence, add a condition in the isr to check for
> interrupt ownership and channel number to make sure we do not
> service wrong interrupts.
> 
> Signed-off-by: Manjunath Hadli <manjunath.ha...@ti.com>
> ---
>  drivers/media/video/davinci/vpif_capture.c |    5 +++++
>  drivers/media/video/davinci/vpif_display.c |    5 +++++
>  include/media/davinci/vpif_types.h         |    2 ++
>  3 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/davinci/vpif_capture.c
> b/drivers/media/video/davinci/vpif_capture.c index 6504e40..33d865d 100644
> --- a/drivers/media/video/davinci/vpif_capture.c
> +++ b/drivers/media/video/davinci/vpif_capture.c
> @@ -333,6 +333,7 @@ static void vpif_schedule_next_buffer(struct common_obj
> *common) */
>  static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
>  {
> +     struct vpif_capture_config *config = vpif_dev->platform_data;
>       struct vpif_device *dev = &vpif_obj;
>       struct common_obj *common;
>       struct channel_obj *ch;
> @@ -341,6 +342,10 @@ static irqreturn_t vpif_channel_isr(int irq, void
> *dev_id) int fid = -1, i;
> 
>       channel_id = *(int *)(dev_id);
> +     if (!config->intr_status ||
> +                     !config->intr_status(vpif_base, channel_id))
> +             return IRQ_NONE;
> +

I don't think this is the right way to handle the situation. You should 
instead read the interrupt source register for the VPIF capture device, and 
return IRQ_NONE if you find that no interrupt source has been flagged (and 
similarly for the display device below).

>       ch = dev->dev[channel_id];
> 
>       field = ch->common[VPIF_VIDEO_INDEX].fmt.fmt.pix.field;
> diff --git a/drivers/media/video/davinci/vpif_display.c
> b/drivers/media/video/davinci/vpif_display.c index 7fa34b4..9b59e0b 100644
> --- a/drivers/media/video/davinci/vpif_display.c
> +++ b/drivers/media/video/davinci/vpif_display.c
> @@ -299,6 +299,7 @@ static void process_interlaced_mode(int fid, struct
> common_obj *common) */
>  static irqreturn_t vpif_channel_isr(int irq, void *dev_id)
>  {
> +     struct vpif_display_config *config = vpif_dev->platform_data;
>       struct vpif_device *dev = &vpif_obj;
>       struct channel_obj *ch;
>       struct common_obj *common;
> @@ -307,6 +308,10 @@ static irqreturn_t vpif_channel_isr(int irq, void
> *dev_id) int channel_id = 0;
> 
>       channel_id = *(int *)(dev_id);
> +     if (!config->intr_status ||
> +             !config->intr_status(vpif_base, channel_id + 2))
> +             return IRQ_NONE;
> +
>       ch = dev->dev[channel_id];
>       field = ch->common[VPIF_VIDEO_INDEX].fmt.fmt.pix.field;
>       for (i = 0; i < VPIF_NUMOBJECTS; i++) {
> diff --git a/include/media/davinci/vpif_types.h
> b/include/media/davinci/vpif_types.h index bd8217c..2845bda 100644
> --- a/include/media/davinci/vpif_types.h
> +++ b/include/media/davinci/vpif_types.h
> @@ -45,6 +45,7 @@ struct vpif_subdev_info {
> 
>  struct vpif_display_config {
>       int (*set_clock)(int, int);
> +     int (*intr_status)(void __iomem *vpif_base, int);
>       struct vpif_subdev_info *subdevinfo;
>       int subdev_count;
>       const char **output;
> @@ -65,6 +66,7 @@ struct vpif_capture_chan_config {
>  struct vpif_capture_config {
>       int (*setup_input_channel_mode)(int);
>       int (*setup_input_path)(int, const char *);
> +     int (*intr_status)(void __iomem *vpif_base, int);
>       struct vpif_capture_chan_config chan_config[VPIF_CAPTURE_MAX_CHANNELS];
>       struct vpif_subdev_info *subdev_info;
>       int subdev_count;

-- 
Regards,

Laurent Pinchart

_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source

Reply via email to