Re: [hg:v4l-dvb] video: initial support for ADV7180

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em Mon, 21 Sep 2009 09:28:51 +0200
Richard Röjfors  <richard.rojfors@xxxxxxxxxxxxxxx> escreveu:

> Patch from Richard Röjfors wrote:
> > The patch number 13019 was added via Douglas Schilling Landgraf <dougsland@xxxxxxxxxx>
> > to http://linuxtv.org/hg/v4l-dvb master development tree.
> > 
> > Kernel patches in this development tree may be modified to be backward
> > compatible with older kernels. Compatibility modifications will be
> > removed before inclusion into the mainstream Kernel
> > 
> > If anyone has any objections, please let us know by sending a message to:
> > 	Linux Media Mailing List <linux-media@xxxxxxxxxxxxxxx>
> > 
> > ------
> 
> Hi,
> 
> There is a newer version of the driver that has support for beeing
> interrupt driver and setting standard, and checking the signal status.
> 
> I would be very happy if that gets committed instead.

Hi Richard,

The previous version were already committed on our trees. So, please send us
diff patches, instead of a completely new version.

As stated on Kernel Documentation/SubmittingPatches:

"If your changes produce a lot of deltas, you may want to look into
splitting them into individual patches which modify things in
logical stages.  This will facilitate easier reviewing by other
kernel developers, very important if you want your patch accepted."

So, if you are adding 3 new functionalities (interrupt, standard setting,
signal status), the better is if you send us 3 patches.

Thanks,
Mauro.

> +static int v4l2_std_to_adv7180(v4l2_std_id std)
> +{
> +	/* pal is a combination of several variants */
> +	if (std & V4L2_STD_PAL)
> +		return ADV7180_INPUT_CONTROL_PAL_BG;
> +	if (std & V4L2_STD_NTSC)
> +		return ADV7180_INPUT_CONTROL_NTSC_M;
> +
> +	switch (std) {
> +	case V4L2_STD_PAL_60:
> +		return ADV7180_INPUT_CONTROL_PAL60;
> +	case V4L2_STD_NTSC_443:
> +		return ADV7180_INPUT_CONTROL_NTSC_443;
> +	case V4L2_STD_PAL_N:
> +		return ADV7180_INPUT_CONTROL_PAL_N;
> +	case V4L2_STD_PAL_M:
> +		return ADV7180_INPUT_CONTROL_PAL_M;
> +	case V4L2_STD_PAL_Nc:
> +		return ADV7180_INPUT_CONTROL_PAL_COMB_N;
> +	case V4L2_STD_SECAM:
> +		return ADV7180_INPUT_CONTROL_PAL_SECAM;
> +	default:
> +		return -EINVAL;
> +	}
> +}

Btw, this code is not right. 

All standards are bit masks. As such, it is valid that an userspace application
to send a request like, for example, V4L2_STD_SECAM_K. This standard
seems to be supported, but your driver will return -EINVAL.

What we generally do is to handle first the special cases where just one standard
that requires especial treatment is defined (like PAL/60, PAL/M, PAL/N, ...) and then
at the bottom, we handle the masks that covers more than one standard, like:

	if (std == V4L_STD_PAL_60)
		return ADV7180_INPUT_CONTROL_PAL60;
	...

	if (std & V4L2_STD_SECAM)
		return ADV7180_INPUT_CONTROL_PAL_SECAM;
	if (std & V4L2_STD_NTSC)
		return ADV7180_INPUT_CONTROL_NTSC_M;
	/* If it is none of the above, it is PAL */
	return ADV7180_INPUT_CONTROL_PAL_BG;



Cheers,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux