Hi Andrey,
See below for a quick review of the code.
I agree with Greg's comment why this is added to staging instead of
drivers/media/pci?
When you post the v2 patch, can you add the output of 'v4l2-compliance -s' to
the
cover letter? Please use the latest v4l2-compliance version from the
v4l-utils.git
repository when testing.
On 03/14/2016 02:59 AM, Andrey Utkin wrote:
> Support for boards based on Techwell TW5864 chip which provides
> multichannel video & audio grabbing and encoding (H.264, MJPEG,
> ADPCM G.726).
>
> Signed-off-by: Andrey Utkin
> Tested-by: Andrey Utkin
> ---
> MAINTAINERS |7 +
> drivers/staging/media/Kconfig|2 +
> drivers/staging/media/Makefile |1 +
> drivers/staging/media/tw5864/Kconfig | 11 +
> drivers/staging/media/tw5864/Makefile|3 +
> drivers/staging/media/tw5864/tw5864-bs.h | 154 ++
> drivers/staging/media/tw5864/tw5864-config.c | 359 +
> drivers/staging/media/tw5864/tw5864-core.c | 453 ++
> drivers/staging/media/tw5864/tw5864-h264.c | 183 +++
> drivers/staging/media/tw5864/tw5864-reg.h| 2200
> ++
> drivers/staging/media/tw5864/tw5864-tables.h | 237 +++
> drivers/staging/media/tw5864/tw5864-video.c | 1364
> drivers/staging/media/tw5864/tw5864.h| 280
> include/linux/pci_ids.h |1 +
> 14 files changed, 5255 insertions(+)
> create mode 100644 drivers/staging/media/tw5864/Kconfig
> create mode 100644 drivers/staging/media/tw5864/Makefile
> create mode 100644 drivers/staging/media/tw5864/tw5864-bs.h
> create mode 100644 drivers/staging/media/tw5864/tw5864-config.c
> create mode 100644 drivers/staging/media/tw5864/tw5864-core.c
> create mode 100644 drivers/staging/media/tw5864/tw5864-h264.c
> create mode 100644 drivers/staging/media/tw5864/tw5864-reg.h
> create mode 100644 drivers/staging/media/tw5864/tw5864-tables.h
> create mode 100644 drivers/staging/media/tw5864/tw5864-video.c
> create mode 100644 drivers/staging/media/tw5864/tw5864.h
>
> diff --git a/drivers/staging/media/tw5864/tw5864-bs.h
> b/drivers/staging/media/tw5864/tw5864-bs.h
> new file mode 100644
> index 000..8c1df7a
> --- /dev/null
> +++ b/drivers/staging/media/tw5864/tw5864-bs.h
> @@ -0,0 +1,154 @@
> +static inline void bs_direct_write(struct bs *s, u8 value)
> +{
> + *s->p = value;
> + s->p++;
> + s->i_left = 8;
> +}
> +
> +static inline void bs_write(struct bs *s, int i_count, u32 i_bits)
This one is a bit too big to be an inline IMHO.
> +{
> + if (s->p >= s->p_end - 4)
> + return;
> + while (i_count > 0) {
> + if (i_count < 32)
> + i_bits &= (1 << i_count) - 1;
> + if (i_count < s->i_left) {
> + *s->p = (*s->p << i_count) | i_bits;
> + s->i_left -= i_count;
> + break;
> + }
> + *s->p = (*s->p << s->i_left) | (i_bits >> (i_count -
> +s->i_left));
> + i_count -= s->i_left;
> + s->p++;
> + s->i_left = 8;
> + }
> +}
> +
> diff --git a/drivers/staging/media/tw5864/tw5864-config.c
> b/drivers/staging/media/tw5864/tw5864-config.c
> new file mode 100644
> index 000..ff3e308
> --- /dev/null
> +++ b/drivers/staging/media/tw5864/tw5864-config.c
> @@ -0,0 +1,359 @@
> +/*
> + * TW5864 driver - analog decoders configuration functions
> + *
> + * Copyright (C) 2015 Bluecherry, LLC
> + * Author: Andrey Utkin
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include "tw5864.h"
> +#include "tw5864-reg.h"
> +
> +#define TW5864_IIC_TIMEOUT (3)
> +
> +static unsigned char tbl_pal_tw2864_common[] __used = {
Just use static const and remove the __used.
It would be nice to have a short comment before each array that gives a bit
more information.
> + 0x00, 0x00, 0x64, 0x11,
> + 0x80, 0x80, 0x00, 0x12,
> + 0x12, 0x20, 0x0a, 0xD0,
> + 0x00, 0x00, 0x07, 0x7F,
> +};
> +
> +static int __used i2c_multi_read(struct tw5864_dev *dev, u8 devid, u8 devfn,
> + u8 *buf, u32 count)
Again, what's up with the __used? Just remove it.
> +{
> + int i = 0;
> + u32 val = 0;
> + int timeout = TW5864_IIC_TIMEOUT;
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + f