cron job: media_tree daily build: ERRORS

[Hans Verkuil]

This message is generated daily by a cron job that builds media_tree for
the kernels and architectures in the list below.

Results of the daily build of media_tree:

date:   Mon Nov 21 05:00:17 CET 2016
media-tree git hash:f2709c206d8a3e11729e68d80c57e7470bbe8e5e
media_build git hash:   bd61d1f4217e104f91df1ba1b5a4594e379829de
v4l-utils git hash: 046f2376ac29b3f1b8f88f094527ff65814a5c9c
gcc version:i686-linux-gcc (GCC) 6.2.0
sparse version: v0.5.0-3553-g78b2ea6
smatch version: v0.5.0-3553-g78b2ea6
host hardware:  x86_64
host os:4.7.0-164

linux-git-arm-at91: OK
linux-git-arm-davinci: WARNINGS
linux-git-arm-multi: OK
linux-git-arm-pxa: OK
linux-git-blackfin-bf561: OK
linux-git-i686: OK
linux-git-m32r: OK
linux-git-mips: OK
linux-git-powerpc64: OK
linux-git-sh: OK
linux-git-x86_64: OK
linux-2.6.36.4-i686: ERRORS
linux-2.6.37.6-i686: ERRORS
linux-2.6.38.8-i686: ERRORS
linux-2.6.39.4-i686: ERRORS
linux-3.0.60-i686: ERRORS
linux-3.1.10-i686: ERRORS
linux-3.2.37-i686: ERRORS
linux-3.3.8-i686: ERRORS
linux-3.4.27-i686: ERRORS
linux-3.5.7-i686: ERRORS
linux-3.6.11-i686: ERRORS
linux-3.7.4-i686: ERRORS
linux-3.8-i686: ERRORS
linux-3.9.2-i686: ERRORS
linux-3.10.1-i686: ERRORS
linux-3.11.1-i686: ERRORS
linux-3.12.67-i686: ERRORS
linux-3.13.11-i686: ERRORS
linux-3.14.9-i686: ERRORS
linux-3.15.2-i686: ERRORS
linux-3.16.7-i686: ERRORS
linux-3.17.8-i686: ERRORS
linux-3.18.7-i686: ERRORS
linux-3.19-i686: ERRORS
linux-4.0.9-i686: ERRORS
linux-4.1.33-i686: ERRORS
linux-4.2.8-i686: ERRORS
linux-4.3.6-i686: ERRORS
linux-4.4.22-i686: ERRORS
linux-4.5.7-i686: ERRORS
linux-4.6.7-i686: ERRORS
linux-4.7.5-i686: ERRORS
linux-4.8-i686: ERRORS
linux-4.9-rc5-i686: ERRORS
linux-2.6.36.4-x86_64: ERRORS
linux-2.6.37.6-x86_64: ERRORS
linux-2.6.38.8-x86_64: ERRORS
linux-2.6.39.4-x86_64: ERRORS
linux-3.0.60-x86_64: ERRORS
linux-3.1.10-x86_64: ERRORS
linux-3.2.37-x86_64: ERRORS
linux-3.3.8-x86_64: ERRORS
linux-3.4.27-x86_64: ERRORS
linux-3.5.7-x86_64: ERRORS
linux-3.6.11-x86_64: ERRORS
linux-3.7.4-x86_64: ERRORS
linux-3.8-x86_64: ERRORS
linux-3.9.2-x86_64: ERRORS
linux-3.10.1-x86_64: ERRORS
linux-3.11.1-x86_64: ERRORS
linux-3.12.67-x86_64: ERRORS
linux-3.13.11-x86_64: ERRORS
linux-3.14.9-x86_64: ERRORS
linux-3.15.2-x86_64: ERRORS
linux-3.16.7-x86_64: ERRORS
linux-3.17.8-x86_64: ERRORS
linux-3.18.7-x86_64: ERRORS
linux-3.19-x86_64: ERRORS
linux-4.0.9-x86_64: ERRORS
linux-4.1.33-x86_64: ERRORS
linux-4.2.8-x86_64: ERRORS
linux-4.3.6-x86_64: ERRORS
linux-4.4.22-x86_64: ERRORS
linux-4.5.7-x86_64: ERRORS
linux-4.6.7-x86_64: ERRORS
linux-4.7.5-x86_64: ERRORS
linux-4.8-x86_64: ERRORS
linux-4.9-rc5-x86_64: ERRORS
apps: WARNINGS
spec-git: OK
smatch: ERRORS
sparse: WARNINGS

Detailed results are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Monday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/3] stk1160: Check whether to use AC97 codec or internal ADC.

[Ezequiel Garcia]

On 27 October 2016 at 17:34, Marcel Hasler  wrote:
> Some STK1160-based devices use the chip's internal 8-bit ADC. This is 
> configured through a strap
> pin. The value of this and other pins can be read through the POSVA register. 
> If the internal
> ADC is used, there's no point trying to setup the unavailable AC97 codec.
>
> Signed-off-by: Marcel Hasler 
> ---
>  drivers/media/usb/stk1160/stk1160-ac97.c | 15 +++
>  drivers/media/usb/stk1160/stk1160-core.c |  3 +--
>  drivers/media/usb/stk1160/stk1160-reg.h  |  3 +++
>  3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c 
> b/drivers/media/usb/stk1160/stk1160-ac97.c
> index d3665ce..6dbc39f 100644
> --- a/drivers/media/usb/stk1160/stk1160-ac97.c
> +++ b/drivers/media/usb/stk1160/stk1160-ac97.c
> @@ -90,8 +90,23 @@ void stk1160_ac97_dump_regs(struct stk1160 *dev)
>  }
>  #endif
>
> +int stk1160_has_ac97(struct stk1160 *dev)
> +{
> +   u8 value;
> +
> +   stk1160_read_reg(dev, STK1160_POSVA, );
> +
> +   /* Bit 2 high means internal ADC */
> +   return !(value & 0x04);

How about define a macro such as:

diff --git a/drivers/media/usb/stk1160/stk1160-reg.h
b/drivers/media/usb/stk1160/stk1160-reg.h
index a4ab586fcee1..4922249d7d34 100644
--- a/drivers/media/usb/stk1160/stk1160-reg.h
+++ b/drivers/media/usb/stk1160/stk1160-reg.h
@@ -28,6 +28,7 @@

 /* Power-on Strapping Data */
 #define STK1160_POSVA  0x010
+#define  STK1160_POSVA_ACSYNC  BIT(2)

Also, the spec mentions another POSVA bit relevant
to audio ACDOUT. Should we check that too?

> +}
> +
>  void stk1160_ac97_setup(struct stk1160 *dev)
>  {
> +   if (!stk1160_has_ac97(dev)) {
> +   stk1160_info("Device uses internal 8-bit ADC, skipping AC97 
> setup.");
> +   return;
> +   }
> +
> /* Two-step reset AC97 interface and hardware codec */
> stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x94);
> stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x8c);
> diff --git a/drivers/media/usb/stk1160/stk1160-core.c 
> b/drivers/media/usb/stk1160/stk1160-core.c
> index f3c9b8a..c86eb61 100644
> --- a/drivers/media/usb/stk1160/stk1160-core.c
> +++ b/drivers/media/usb/stk1160/stk1160-core.c
> @@ -20,8 +20,7 @@
>   *
>   * TODO:
>   *
> - * 1. (Try to) detect if we must register ac97 mixer
> - * 2. Support stream at lower speed: lower frame rate or lower frame size.
> + * 1. Support stream at lower speed: lower frame rate or lower frame size.
>   *
>   */
>
> diff --git a/drivers/media/usb/stk1160/stk1160-reg.h 
> b/drivers/media/usb/stk1160/stk1160-reg.h
> index 81ff3a1..a4ab586 100644
> --- a/drivers/media/usb/stk1160/stk1160-reg.h
> +++ b/drivers/media/usb/stk1160/stk1160-reg.h
> @@ -26,6 +26,9 @@
>  /* Remote Wakup Control */
>  #define STK1160_RMCTL  0x00c
>
> +/* Power-on Strapping Data */
> +#define STK1160_POSVA  0x010
> +
>  /*
>   * Decoder Control Register:
>   * This byte controls capture start/stop
> --
> 2.10.1
>



-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] stk1160: Give the chip some time to retrieve data from AC97 codec.

[Ezequiel Garcia]

On 28 October 2016 at 05:52, Marcel Hasler  wrote:
> The STK1160 needs some time to transfer data from the AC97 registers into its 
> own. On some
> systems reading the chip's own registers to soon will return wrong values. 
> The "proper" way to
> handle this would be to poll STK1160_AC97CTL_0 after every read or write 
> command until the
> command bit has been cleared, but this may not be worth the hassle.
>
> Signed-off-by: Marcel Hasler 
> ---
>  drivers/media/usb/stk1160/stk1160-ac97.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c 
> b/drivers/media/usb/stk1160/stk1160-ac97.c
> index 31bdd60d..caa65a8 100644
> --- a/drivers/media/usb/stk1160/stk1160-ac97.c
> +++ b/drivers/media/usb/stk1160/stk1160-ac97.c
> @@ -20,6 +20,7 @@
>   *
>   */
>
> +#include 
>  #include 
>
>  #include "stk1160.h"
> @@ -61,6 +62,9 @@ static u16 stk1160_read_ac97(struct stk1160 *dev, u16 reg)
>  */
> stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x8b);
>
> +   /* Give the chip some time to transfer data */
> +   usleep_range(20, 40);
> +

I don't recall any issues with this. In any case, we only read the registers
for debugging purposes, so it's not a big deal.

Maybe it would be better to expand the comment a little bit,
using your commit log:

""
The "proper" way to
handle this would be to poll STK1160_AC97CTL_0 after
every read or write command until the command bit
has been cleared, but this may not be worth the hassle.
""

This way, if the sleep proves problematic in the future,
the "proper way" is already documented.

> /* Retrieve register value */
> stk1160_read_reg(dev, STK1160_AC97_CMD, );
> stk1160_read_reg(dev, STK1160_AC97_CMD + 1, );
> --
> 2.10.1
>



-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] stk1160: Add module param for setting the record gain.

[Ezequiel Garcia]

On 27 October 2016 at 17:35, Marcel Hasler  wrote:
> Allow setting a custom record gain for the internal AC97 codec (if 
> available). This can be
> a value between 0 and 15, 8 is the default and should be suitable for most 
> users. The Windows
> driver also sets this to 8 without any possibility for changing it.
>
> Signed-off-by: Marcel Hasler 
> ---
>  drivers/media/usb/stk1160/stk1160-ac97.c | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c 
> b/drivers/media/usb/stk1160/stk1160-ac97.c
> index 6dbc39f..31bdd60d 100644
> --- a/drivers/media/usb/stk1160/stk1160-ac97.c
> +++ b/drivers/media/usb/stk1160/stk1160-ac97.c
> @@ -25,6 +25,11 @@
>  #include "stk1160.h"
>  #include "stk1160-reg.h"
>
> +static u8 gain = 8;
> +
> +module_param(gain, byte, 0444);
> +MODULE_PARM_DESC(gain, "Set capture gain level if AC97 codec is available 
> (0-15, default: 8)");
> +
>  static void stk1160_write_ac97(struct stk1160 *dev, u16 reg, u16 value)
>  {
> /* Set codec register address */
> @@ -122,7 +127,11 @@ void stk1160_ac97_setup(struct stk1160 *dev)
> stk1160_write_ac97(dev, 0x16, 0x0808); /* Aux volume */
> stk1160_write_ac97(dev, 0x1a, 0x0404); /* Record select */
> stk1160_write_ac97(dev, 0x02, 0x); /* Master volume */
> -   stk1160_write_ac97(dev, 0x1c, 0x0808); /* Record gain */
> +
> +   /* Record gain */
> +   gain = (gain > 15) ? 15 : gain;
> +   stk1160_info("Setting capture gain to %d.", gain);

This message doesn't add anything useful, can we drop it?

> +   stk1160_write_ac97(dev, 0x1c, (gain<<8) | gain);
>
>  #ifdef DEBUG
> stk1160_ac97_dump_regs(dev);
> --
> 2.10.1
>



-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 1/3] stk1160: Remove stk1160-mixer and setup internal AC97 codec automatically.

[Ezequiel Garcia]

Marcel,

On 27 October 2016 at 17:34, Marcel Hasler  wrote:
> Exposing all the channels of the device's internal AC97 codec to userspace is 
> unnecessary and
> confusing. Instead the driver should setup the codec with proper values. This 
> patch removes the
> mixer and sets up the codec using optimal values, i.e. the same values set by 
> the Windows
> driver. This also makes the device work out-of-the-box, without the need for 
> the user to
> reconfigure the device every time it's plugged in.
>
> Signed-off-by: Marcel Hasler 

This patch is *awesome*.

You've re-written the file ;-), so if you want to put your
copyright on stk1160-ac97.c, be my guest.

Also, just a minor comment, see below.

> ---
>  drivers/media/usb/stk1160/Kconfig|  10 +--
>  drivers/media/usb/stk1160/Makefile   |   4 +-
>  drivers/media/usb/stk1160/stk1160-ac97.c | 121 
> +++
>  drivers/media/usb/stk1160/stk1160-core.c |   5 +-
>  drivers/media/usb/stk1160/stk1160.h  |   9 +--
>  5 files changed, 47 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/media/usb/stk1160/Kconfig 
> b/drivers/media/usb/stk1160/Kconfig
> index 95584c1..22dff4f 100644
> --- a/drivers/media/usb/stk1160/Kconfig
> +++ b/drivers/media/usb/stk1160/Kconfig
> @@ -8,17 +8,9 @@ config VIDEO_STK1160_COMMON
>   To compile this driver as a module, choose M here: the
>   module will be called stk1160
>
> -config VIDEO_STK1160_AC97
> -   bool "STK1160 AC97 codec support"
> -   depends on VIDEO_STK1160_COMMON && SND
> -
> -   ---help---
> - Enables AC97 codec support for stk1160 driver.
> -
>  config VIDEO_STK1160
> tristate
> -   depends on (!VIDEO_STK1160_AC97 || (SND='n') || SND) && 
> VIDEO_STK1160_COMMON
> +   depends on VIDEO_STK1160_COMMON
> default y
> select VIDEOBUF2_VMALLOC
> select VIDEO_SAA711X
> -   select SND_AC97_CODEC if SND
> diff --git a/drivers/media/usb/stk1160/Makefile 
> b/drivers/media/usb/stk1160/Makefile
> index dfe3e90..42d0546 100644
> --- a/drivers/media/usb/stk1160/Makefile
> +++ b/drivers/media/usb/stk1160/Makefile
> @@ -1,10 +1,8 @@
> -obj-stk1160-ac97-$(CONFIG_VIDEO_STK1160_AC97) := stk1160-ac97.o
> -
>  stk1160-y :=   stk1160-core.o \
> stk1160-v4l.o \
> stk1160-video.o \
> stk1160-i2c.o \
> -   $(obj-stk1160-ac97-y)
> +   stk1160-ac97.o
>
>  obj-$(CONFIG_VIDEO_STK1160) += stk1160.o
>
> diff --git a/drivers/media/usb/stk1160/stk1160-ac97.c 
> b/drivers/media/usb/stk1160/stk1160-ac97.c
> index 2dd308f..d3665ce 100644
> --- a/drivers/media/usb/stk1160/stk1160-ac97.c
> +++ b/drivers/media/usb/stk1160/stk1160-ac97.c
> @@ -21,19 +21,12 @@
>   */
>
>  #include 
> -#include 
> -#include 
> -#include 
>
>  #include "stk1160.h"
>  #include "stk1160-reg.h"
>
> -static struct snd_ac97 *stk1160_ac97;
> -
> -static void stk1160_write_ac97(struct snd_ac97 *ac97, u16 reg, u16 value)
> +static void stk1160_write_ac97(struct stk1160 *dev, u16 reg, u16 value)
>  {
> -   struct stk1160 *dev = ac97->private_data;
> -
> /* Set codec register address */
> stk1160_write_reg(dev, STK1160_AC97_ADDR, reg);
>
> @@ -48,9 +41,9 @@ static void stk1160_write_ac97(struct snd_ac97 *ac97, u16 
> reg, u16 value)
> stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x8c);
>  }
>
> -static u16 stk1160_read_ac97(struct snd_ac97 *ac97, u16 reg)
> +#ifdef DEBUG
> +static u16 stk1160_read_ac97(struct stk1160 *dev, u16 reg)
>  {
> -   struct stk1160 *dev = ac97->private_data;
> u8 vall = 0;
> u8 valh = 0;
>
> @@ -70,81 +63,53 @@ static u16 stk1160_read_ac97(struct snd_ac97 *ac97, u16 
> reg)
> return (valh << 8) | vall;
>  }
>
> -static void stk1160_reset_ac97(struct snd_ac97 *ac97)
> +void stk1160_ac97_dump_regs(struct stk1160 *dev)

static void stk1160_ac97_dump_regs ?

>  {
> -   struct stk1160 *dev = ac97->private_data;
> -   /* Two-step reset AC97 interface and hardware codec */
> -   stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x94);
> -   stk1160_write_reg(dev, STK1160_AC97CTL_0, 0x88);
> +   u16 value;
>
> -   /* Set 16-bit audio data and choose L channel*/
> -   stk1160_write_reg(dev, STK1160_AC97CTL_1 + 2, 0x01);
> -}
> +   value = stk1160_read_ac97(dev, 0x12); /* CD volume */
> +   stk1160_dbg("0x12 == 0x%04x", value);
>
> -static struct snd_ac97_bus_ops stk1160_ac97_ops = {
> -   .read   = stk1160_read_ac97,
> -   .write  = stk1160_write_ac97,
> -   .reset  = stk1160_reset_ac97,
> -};
> +   value = stk1160_read_ac97(dev, 0x10); /* Line-in volume */
> +   stk1160_dbg("0x10 == 0x%04x", value);
>
> -int stk1160_ac97_register(struct stk1160 *dev)
> -{
> -   struct snd_card *card = NULL;
> -   struct snd_ac97_bus *ac97_bus;
> -   struct snd_ac97_template ac97_template;
> -   int rc;
> +   value = 

Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor

[Pavel Machek]

Hi!

> > +   /* V4L2_CID_EXPOSURE */
> > +   min = et8ek8_exposure_rows_to_us(sensor, 1);
> > +   max = et8ek8_exposure_rows_to_us(sensor,
> > +   sensor->current_reglist->mode.max_exp);
> 
> Haven't I suggested to use lines instead? I vaguely remember doing so...
> this would remove quite some code from the driver.

Lines ... lines ... no, I don't think I understand how to use lines
here. I guess I could switch units from us to rows here...?

Is it good idea? For userspace, microseconds are really a nice
interface, because ... well, that's what photographers are used to
think about (ISO 400, time 1/100). fcam also uses usec internally.

In the current camera code, I do autogain in small resolution, then
use same parameters (gain, time) at higher resolution. I guess I could
do the same with the non-microseconds interface, but then I'd have to
move the microsecond computation into userspace. And userspace is
not really good place to do that, as it does not know (and should not
have to know!) such low level details.

So... can we keep the interface as it is?

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor

[Pavel Machek]

Hi!

> > +static void et8ek8_reglist_to_mbus(const struct et8ek8_reglist *reglist,
> > +  struct v4l2_mbus_framefmt *fmt)
> > +{
> > +   fmt->width = reglist->mode.window_width;
> > +   fmt->height = reglist->mode.window_height;
> > +
> > +   if (reglist->mode.pixel_format == V4L2_PIX_FMT_SGRBG10DPCM8)
> 
> The driver doesn't really need to deal with pixel formats. Could you use
> media bus formats instead, and rename the fields accordingly?
> 
> The reason why it did use pixel formats was that (V4L2) media bus formats
> did not exist when the driver was written. :-)

Makes sense...

Something like this? [untested, will test complete changes.]

Pavel

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c 
b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index 0301e81..eb131b2 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -395,11 +395,7 @@ static void et8ek8_reglist_to_mbus(const struct 
et8ek8_reglist *reglist,
 {
fmt->width = reglist->mode.window_width;
fmt->height = reglist->mode.window_height;
-
-   if (reglist->mode.pixel_format == V4L2_PIX_FMT_SGRBG10DPCM8)
-   fmt->code = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8;
-   else
-   fmt->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+   fmt->code = reglist->mode.bus_format;
 }
 
 static struct et8ek8_reglist *et8ek8_reglist_find_mode_fmt(
@@ -538,7 +534,7 @@ static int et8ek8_reglist_import(struct i2c_client *client,
   __func__,
   list->type,
   list->mode.window_width, list->mode.window_height,
-  list->mode.pixel_format,
+  list->mode.bus_format,
   list->mode.timeperframe.numerator,
   list->mode.timeperframe.denominator,
   (void *)meta->reglist[nlists].ptr);
@@ -967,21 +963,18 @@ static int et8ek8_enum_mbus_code(struct v4l2_subdev 
*subdev,
continue;
 
for (i = 0; i < npixelformat; i++) {
-   if (pixelformat[i] == mode->pixel_format)
+   if (pixelformat[i] == mode->bus_format)
break;
}
if (i != npixelformat)
continue;
 
if (code->index == npixelformat) {
-   if (mode->pixel_format == V4L2_PIX_FMT_SGRBG10DPCM8)
-   code->code = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8;
-   else
-   code->code = MEDIA_BUS_FMT_SGRBG10_1X10;
+   code->code = mode->bus_format;
return 0;
}
 
-   pixelformat[npixelformat] = mode->pixel_format;
+   pixelformat[npixelformat] = mode->bus_format;
npixelformat++;
}
 
diff --git a/drivers/media/i2c/et8ek8/et8ek8_mode.c 
b/drivers/media/i2c/et8ek8/et8ek8_mode.c
index 956fc60..12998d8 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_mode.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_mode.c
@@ -59,7 +59,7 @@ static struct et8ek8_reglist 
mode1_poweron_mode2_16vga_2592x1968_12_07fps = {
},
.max_exp = 2012,
/* .max_gain = 0, */
-   .pixel_format = V4L2_PIX_FMT_SGRBG10,
+   .bus_format = MEDIA_BUS_FMT_SGRBG10_1X10,
.sensitivity = 65536
},
.regs = {
@@ -160,7 +160,7 @@ static struct et8ek8_reglist 
mode1_16vga_2592x1968_13_12fps_dpcm10_8 = {
},
.max_exp = 2012,
/* .max_gain = 0, */
-   .pixel_format = V4L2_PIX_FMT_SGRBG10DPCM8,
+   .bus_format = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8,
.sensitivity = 65536
},
.regs = {
@@ -216,7 +216,7 @@ static struct et8ek8_reglist 
mode3_4vga_1296x984_29_99fps_dpcm10_8 = {
},
.max_exp = 1004,
/* .max_gain = 0, */
-   .pixel_format = V4L2_PIX_FMT_SGRBG10DPCM8,
+   .bus_format = MEDIA_BUS_FMT_SGRBG10_DPCM8_1X8,
.sensitivity = 65536
},
.regs = {
@@ -272,7 +272,7 @@ static struct et8ek8_reglist mode4_svga_864x656_29_88fps = {
},
.max_exp = 668,
/* .max_gain = 0, */
-   .pixel_format = V4L2_PIX_FMT_SGRBG10,
+   .bus_format = MEDIA_BUS_FMT_SGRBG10_1X10,
.sensitivity = 65536
},
.regs = {
@@ -328,7 +328,7 @@ static struct et8ek8_reglist mode5_vga_648x492_29_93fps = {
},
.max_exp = 500,
/* .max_gain = 0, */
-   .pixel_format = V4L2_PIX_FMT_SGRBG10,
+   .bus_format = MEDIA_BUS_FMT_SGRBG10_1X10,
.sensitivity = 65536

Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor

[Pavel Machek]

Hi!

> > +   u32 min, max;
> > +
> > +   v4l2_ctrl_handler_init(>ctrl_handler, 4);
> > +
> > +   /* V4L2_CID_GAIN */
> > +   v4l2_ctrl_new_std(>ctrl_handler, _ctrl_ops,
> > + V4L2_CID_GAIN, 0, ARRAY_SIZE(et8ek8_gain_table) - 1,
> > + 1, 0);
> > +
> > +   /* V4L2_CID_EXPOSURE */
> > +   min = et8ek8_exposure_rows_to_us(sensor, 1);
> > +   max = et8ek8_exposure_rows_to_us(sensor,
> > +   sensor->current_reglist->mode.max_exp);
> 
> Haven't I suggested to use lines instead? I vaguely remember doing so...
> this would remove quite some code from the driver.

Do you have some more hints? I'll try to figure it out...

> > +#ifdef CONFIG_PM
> > +
> > +static int et8ek8_suspend(struct device *dev)
> 
> static int __maybe_unused ...
> 
> Please check the smiapp patches I just sent to the list. The smiapp driver
> had similar issues.

Ok, I guess I figured it out from other code (no network at the
moment).

> > +   if (sensor->power_count) {
> > +   gpiod_set_value(sensor->reset, 0);
> > +   clk_disable_unprepare(sensor->ext_clk);
> > +   sensor->power_count = 0;
> > +   }
> > +
> 
> You're missing v4l2_async_unregister_subdev() here.

Added.

> > +   v4l2_device_unregister_subdev(>subdev);
> > +   device_remove_file(>dev, _attr_priv_mem);
> > +   v4l2_ctrl_handler_free(>ctrl_handler);
> > +   media_entity_cleanup(>subdev.entity);
> > +
> > +   return 0;
> > +}

> > +MODULE_DEVICE_TABLE(i2c, et8ek8_id_table);
> > +
> > +static const struct dev_pm_ops et8ek8_pm_ops = {
> > +   .suspend= et8ek8_suspend,
> > +   .resume = et8ek8_resume,
> 
> How about using  SET_SYSTEM_SLEEP_PM_OPS() here?

Ok, I guess that saves few lines.

> > +module_i2c_driver(et8ek8_i2c_driver);
> > +
> > +MODULE_AUTHOR("Sakari Ailus ");
> 
> You should put your name here as well. :-)
> 
> It's been a long time I even tried to use it. :-i

Me? Ok, I can list myself there, but I don't really know much about
that driver.

> > + * Contact: Sakari Ailus 
> > + *  Tuukka Toivonen 
> 
> Tuukka's e-mail is wrong here (the correct address is elsewhere in the
> patch).

Fixed.

Ok, these cleanups are here.

Pavel

diff --git a/drivers/media/i2c/et8ek8/et8ek8_driver.c 
b/drivers/media/i2c/et8ek8/et8ek8_driver.c
index eb131b2..eb8c1b4 100644
--- a/drivers/media/i2c/et8ek8/et8ek8_driver.c
+++ b/drivers/media/i2c/et8ek8/et8ek8_driver.c
@@ -5,6 +5,7 @@
  *
  * Contact: Sakari Ailus 
  *  Tuukka Toivonen 
+ *  Pavel Machek 
  *
  * Based on code from Toni Leinonen .
  *
@@ -1435,9 +1436,7 @@ static const struct v4l2_subdev_internal_ops 
et8ek8_internal_ops = {
 /* --
  * I2C driver
  */
-#ifdef CONFIG_PM
-
-static int et8ek8_suspend(struct device *dev)
+static int __maybe_unused et8ek8_suspend(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct v4l2_subdev *subdev = i2c_get_clientdata(client);
@@ -1449,7 +1448,7 @@ static int et8ek8_suspend(struct device *dev)
return __et8ek8_set_power(sensor, false);
 }
 
-static int et8ek8_resume(struct device *dev)
+static int __maybe_unused et8ek8_resume(struct device *dev)
 {
struct i2c_client *client = to_i2c_client(dev);
struct v4l2_subdev *subdev = i2c_get_clientdata(client);
@@ -1461,13 +1460,6 @@ static int et8ek8_resume(struct device *dev)
return __et8ek8_set_power(sensor, true);
 }
 
-#else
-
-#define et8ek8_suspend NULL
-#define et8ek8_resume NULL
-
-#endif /* CONFIG_PM */
-
 static int et8ek8_probe(struct i2c_client *client,
const struct i2c_device_id *devid)
 {
@@ -1542,6 +1534,7 @@ static int __exit et8ek8_remove(struct i2c_client *client)
v4l2_device_unregister_subdev(>subdev);
device_remove_file(>dev, _attr_priv_mem);
v4l2_ctrl_handler_free(>ctrl_handler);
+   v4l2_async_unregister_subdev(>subdev);
media_entity_cleanup(>subdev.entity);
 
return 0;
@@ -1559,8 +1552,7 @@ static const struct i2c_device_id et8ek8_id_table[] = {
 MODULE_DEVICE_TABLE(i2c, et8ek8_id_table);
 
 static const struct dev_pm_ops et8ek8_pm_ops = {
-   .suspend= et8ek8_suspend,
-   .resume = et8ek8_resume,
+   SET_SYSTEM_SLEEP_PM_OPS(et8ek8_suspend, et8ek8_resume)
 };
 
 static struct i2c_driver et8ek8_i2c_driver = {
@@ -1576,6 +1568,6 @@ static struct i2c_driver et8ek8_i2c_driver = {
 
 module_i2c_driver(et8ek8_i2c_driver);
 
-MODULE_AUTHOR("Sakari Ailus ");
+MODULE_AUTHOR("Sakari Ailus , Pavel Machek 

Re: [Ksummit-discuss] Including images on Sphinx documents

[Mauro Carvalho Chehab]

Em Sat, 19 Nov 2016 22:59:01 -
"David Woodhouse"  escreveu:

> > I think that graphviz and svg are the reasonable modern formats. Let's
> > try to avoid bitmaps in today's world, except perhaps as intermediate
> > generated things for what we can't avoid.  

Ok, I got rid of all bitmap images:
https://git.linuxtv.org/mchehab/experimental.git/log/?h=svg-images

Now, all images are in SVG (one is actually a .dot file - while we don't
have an extension to handle it, I opted to keep both .dot and .svg on
my development tree - I'll likely add a Makefile rule for it too).

I converted the ones from pdf/xfig to SVG, and I rewrote the other ones
on SVG. The most complex one was cropping a bitmap image. Instead, I took
the "Tuz" image - e. g. the one from commit 8032b526d1a3 
("linux.conf.au 2009: Tuz") and use it for image crop. The file size is
a way bigger than the previous one (the PNG had 11K; the SVG now has 563K),
but the end result looked nice, IMHO.

> Sure, SVG makes sense. It's a text-based format (albeit XML) and it *can*
> be edited with a text editor and reasonably kept in version control, at
> least if the common tools store it in a diff-friendly way (with some line
> breaks occasionally, and maybe no indenting). Do they?

Inkscape does a good job on breaking lines for a diff-friendly output.

Yet, some lines violate the maximum limit for e-mails defined by
IETF RFC 2821. The problem is that sending such patches to the mailing
lists could make them be ignored.

Not sure what would be the best way to solve such issues.


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

Re: ir-keytable: infinite loops, segfaults

[Sean Young]

On Sat, Nov 19, 2016 at 09:01:10AM +1100, Vincent McIntyre wrote:
> On Fri, Nov 18, 2016 at 05:40:34PM +, Sean Young wrote:
> > >  
> > > So are you saying that the hex codes in the rc_map_dvico_mce_table
> > > struct are invalid (at least in some cases)?
> > 
> > Most likely the remote produces IR in a standard protocol (e.g. rc5, rc6). 
> > If we first get the keymap right then the remote can be used with any 
> > IR receiver that can deal with its protocol; conversely, if we know 
> > what protocol the IR receiver can receive, and we make it produce the 
> > scancodes in the right format, it can then be used with any remote that 
> > uses the protocol it understands.
> > 
> > So at the moment we don't know what protocol it is, and even if it is 
> > rc5 then some bit shifting might be needed to make it into a proper
> > rc5 scancode (both driver and keymap).
> > 
> > > How can I tell what protocol is in use?
> > > 0x00010001 doesn't mean much to me; I did search the linux source
> > > for the code but didn't find any helpful matches.
> > 
> > At the moment it's not easy to determine what protocol an remote uses;
> > I would like to change that but for now, the following is probably
> > the easiest way.
> > 
> > cd /sys/class/rc/rc1 # replace rc1 with your receiver
> > for i in $( protocols; done
> > echo 3 > /sys/module/rc_core/parameters/debug
> > journal -f -k
>  
> Ah. Here we have a problem. The device (/dev/input/event15)
> doesn't have a corresponding rcX node, see ir-keytable output below.
> I had it explained to me like this:

As I said you would need to use a raw IR receiver which has rc-core support
to determine the protocol, so never mind. Please can you try this patch:

I don't have the hardware to test this so your input would be appreciated.


Sean

From: Sean Young 
Subject: [PATCH] [media] cxusb: port to rc-core

The d680_dmb keymap has some new new mappings.

Signed-off-by: Sean Young 
---
 drivers/media/rc/keymaps/Makefile|   3 +
 drivers/media/rc/keymaps/rc-d680-dmb.c   |  75 +++
 drivers/media/rc/keymaps/rc-dvico-mce.c  |  85 
 drivers/media/rc/keymaps/rc-dvico-portable.c |  76 +++
 drivers/media/usb/dvb-usb/cxusb.c| 298 ++-
 include/media/rc-map.h   |   3 +
 6 files changed, 308 insertions(+), 232 deletions(-)
 create mode 100644 drivers/media/rc/keymaps/rc-d680-dmb.c
 create mode 100644 drivers/media/rc/keymaps/rc-dvico-mce.c
 create mode 100644 drivers/media/rc/keymaps/rc-dvico-portable.c

diff --git a/drivers/media/rc/keymaps/Makefile 
b/drivers/media/rc/keymaps/Makefile
index d7b13fa..11d5d5a 100644
--- a/drivers/media/rc/keymaps/Makefile
+++ b/drivers/media/rc/keymaps/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \
rc-cec.o \
rc-cinergy-1400.o \
rc-cinergy.o \
+   rc-d680-dmb.o \
rc-delock-61959.o \
rc-dib0700-nec.o \
rc-dib0700-rc5.o \
@@ -31,6 +32,8 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \
rc-dntv-live-dvbt-pro.o \
rc-dtt200u.o \
rc-dvbsky.o \
+   rc-dvico-mce.o \
+   rc-dvico-portable.o \
rc-em-terratec.o \
rc-encore-enltv2.o \
rc-encore-enltv.o \
diff --git a/drivers/media/rc/keymaps/rc-d680-dmb.c 
b/drivers/media/rc/keymaps/rc-d680-dmb.c
new file mode 100644
index 000..bb5745d
--- /dev/null
+++ b/drivers/media/rc/keymaps/rc-d680-dmb.c
@@ -0,0 +1,75 @@
+/*
+ * keymap imported from cxusb.c
+ *
+ * Copyright (C) 2016 Sean Young
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2.
+ */
+
+#include 
+#include 
+
+static struct rc_map_table rc_map_d680_dmb_table[] = {
+   { 0x0038, KEY_SWITCHVIDEOMODE },/* TV/AV */
+   { 0x080c, KEY_ZOOM },
+   { 0x0800, KEY_0 },
+   { 0x0001, KEY_1 },
+   { 0x0802, KEY_2 },
+   { 0x0003, KEY_3 },
+   { 0x0804, KEY_4 },
+   { 0x0005, KEY_5 },
+   { 0x0806, KEY_6 },
+   { 0x0007, KEY_7 },
+   { 0x0808, KEY_8 },
+   { 0x0009, KEY_9 },
+   { 0x000a, KEY_MUTE },
+   { 0x0829, KEY_BACK },
+   { 0x0012, KEY_CHANNELUP },
+   { 0x0813, KEY_CHANNELDOWN },
+   { 0x002b, KEY_VOLUMEUP },
+   { 0x082c, KEY_VOLUMEDOWN },
+   { 0x0020, KEY_UP },
+   { 0x0821, KEY_DOWN },
+   { 0x0011, KEY_LEFT },
+   { 0x0810, KEY_RIGHT },
+   { 0x000d, KEY_OK },
+   { 0x081f, KEY_RECORD },
+   { 0x0017, KEY_PLAYPAUSE },
+   { 0x0816, KEY_PLAYPAUSE },
+   { 0x000b, KEY_STOP },
+   { 

Re: [PATCH v4] media: Driver for Toshiba et8ek8 5MP sensor

[Pavel Machek]

Hi!

> Just a few more comments...
> 
> Please check my other review as well. I believe you may have missed the
> comments in between in that one.

Sorry for that. Would you have a link for that email or a copy? (I
can't seem to find it.)

Thanks,
Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature