[PATCH] v4l2-of: fix compiler errors if CONFIG_OF is undefined

[Hans Verkuil]

You must use static inline otherwise you get these errors if CONFIG_OF is not 
defined:

In file included from drivers/media/platform/soc_camera/soc_camera.c:39:0:
include/media/v4l2-of.h:112:13: warning: 'v4l2_of_free_endpoint' defined but 
not used [-Wunused-function]
 static void v4l2_of_free_endpoint(struct v4l2_of_endpoint *endpoint)
 ^
In file included from drivers/media/platform/soc_camera/atmel-isi.c:28:0:
include/media/v4l2-of.h:112:13: warning: 'v4l2_of_free_endpoint' defined but 
not used [-Wunused-function]
 static void v4l2_of_free_endpoint(struct v4l2_of_endpoint *endpoint)
 ^
In file included from drivers/media/platform/soc_camera/rcar_vin.c:36:0:
include/media/v4l2-of.h:112:13: warning: 'v4l2_of_free_endpoint' defined but 
not used [-Wunused-function]
 static void v4l2_of_free_endpoint(struct v4l2_of_endpoint *endpoint)
 ^

Signed-off-by: Hans Verkuil 
---
diff --git a/include/media/v4l2-of.h b/include/media/v4l2-of.h
index 241e98a..4dc34b2 100644
--- a/include/media/v4l2-of.h
+++ b/include/media/v4l2-of.h
@@ -103,13 +103,13 @@ static inline int v4l2_of_parse_endpoint(const struct 
device_node *node,
return -ENOSYS;
 }
 
-struct v4l2_of_endpoint *v4l2_of_alloc_parse_endpoint(
+static inline struct v4l2_of_endpoint *v4l2_of_alloc_parse_endpoint(
const struct device_node *node)
 {
return NULL;
 }
 
-static void v4l2_of_free_endpoint(struct v4l2_of_endpoint *endpoint)
+static inline void v4l2_of_free_endpoint(struct v4l2_of_endpoint *endpoint)
 {
 }
 
--
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

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:   Tue Apr 28 04:00:17 CEST 2015
git branch: test
git hash:   cb0c9e1f6777287e81d9b48c264d980bf5014b48
gcc version:i686-linux-gcc (GCC) 5.1.0
sparse version: v0.5.0-44-g40791b9
smatch version: 0.4.1-3153-g7d56ab3
host hardware:  x86_64
host os:4.0.0-0.slh.2-amd64

linux-git-arm-at91: OK
linux-git-arm-davinci: WARNINGS
linux-git-arm-exynos: WARNINGS
linux-git-arm-mx: OK
linux-git-arm-omap: ERRORS
linux-git-arm-omap1: OK
linux-git-arm-pxa: OK
linux-git-blackfin: OK
linux-git-i686: WARNINGS
linux-git-m32r: OK
linux-git-mips: WARNINGS
linux-git-powerpc64: OK
linux-git-sh: WARNINGS
linux-git-x86_64: ERRORS
linux-2.6.32.27-i686: ERRORS
linux-2.6.33.7-i686: ERRORS
linux-2.6.34.7-i686: ERRORS
linux-2.6.35.9-i686: ERRORS
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.23-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-i686: ERRORS
linux-4.1-rc1-i686: WARNINGS
linux-2.6.32.27-x86_64: ERRORS
linux-2.6.33.7-x86_64: ERRORS
linux-2.6.34.7-x86_64: ERRORS
linux-2.6.35.9-x86_64: 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.23-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-x86_64: ERRORS
linux-4.1-rc1-x86_64: WARNINGS
apps: OK
spec-git: OK
sparse: ERRORS
smatch: ERRORS

Detailed results are available here:

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

Full logs are available here:

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

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/media.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: [PULL] For 4.2 (or even 4.1?) add support for cx24120/Technisat SkyStar S2

[Mauro Carvalho Chehab]

Em Mon, 27 Apr 2015 23:25:23 +0200
Patrick Boettcher  escreveu:

> Thank you for your review Mauro.
> 
> In total there are about 3-4 (it's a guess) users of this driver and
> this, written-once read-never, code is for a hardware which is very
> unlikely to be ever reused again ever. Sometimes I regret that there is
> no carpet in OpenSource where you could hide the dirt under ;-).

:)
 
> This driver has been reverse-engineered from a binary-only release - so
> looking for logic is not really useful - there will be sections which
> are not understandable or would require a certain amount of
> investigation and reverse-engineering (I'm thinking of the UNC and BER
> reporting, maybe SNR too).

SNR can be reported in relative mode, if you're unable to figure out
what it means in dB. I don't see much problem for UNC, as, on both DVBv3
and DVBv5, this is a counter.

BER is the only measure that could be more problematic, as, on DVBv5,
it requires the scale of the BER measure, e. g. if BER reports "1",
does that mean an error of 10^-12, 10^-9, 10^-7, 10^-6 or some other
bit rate? In other words, what's the denominator for BER?

> Tough not sure if it is worth the time. Anyone there to convince me?
> 
> I'm really surprised that checkpatch.pl hasn't seen any of the
> coding-style problems you pointed out, except the printk-usage. I ran
> it on the .c-file and not on the patch, is that the problem?

No. People complained that checkpatch was too pedantic. So, some
checks are now optional. A more pedantic test would be to run it
as:

$ checkpatch.pl --strict

There are other parameters that could also used, like --codespell,
in order to identify spelling errors.

Checkpatch can even correct some common errors, using the --fix
parameter.

If you run checkpatch in strict mode, you'll see a lot more errors,
like:

CHECK: spaces preferred around that '/' (ctx:VxV)
#1330: FILE: drivers/media/dvb-frontends/cx24120.c:1131:
+   cmd.arg[4]  = ((state->dcur.symbol_rate/1000) & 0xff00) >> 8;
...

CHECK: Alignment should match open parenthesis
#1486: FILE: drivers/media/dvb-frontends/cx24120.c:1287:
+   err("Could not load firmware (%s): %d\n",
+   CX24120_FIRMWARE, ret);
...
CHECK: Prefer kzalloc(sizeof(*state)...) over kzalloc(sizeof(struct 
cx24120_state)...)
#466: FILE: drivers/media/dvb-frontends/cx24120.c:267:
+   state = kzalloc(sizeof(struct cx24120_state),
...

total: 2 errors, 5 warnings, 107 checks, 1755 lines checked

E. g., 107 additional reports will pop up. Ok, not all of them are
relevant. That's why this is disabled by default. For example, it
works for me if you don't fix this warning:

CHECK: Avoid CamelCase: 
#106: FILE: drivers/media/common/b2c2/flexcop-fe-tuner.c:635:
+static int skystarS2_rev33_attach(struct flexcop_device *fc,

It is probably worth to run checkpatch in strict mode for new drivers,
in order to see if something relevant pops up.

I generally don't run it in strict mode when reviewing patches,
as it would generate too much overhead on my review process, as
it would take me more time reviewing those things, and I may
miss something more relevant.

> Could we send an additional patch for coding-style or would you prefer
> a new patch which has everything inside? This would maintain the
> author-attribution of the initial commit.

An additional patch is fine.

> Sorry for the top-posting.
> 
> best regards,
> --
> Patrick.
> 
> 
> On Mon, 27 Apr 2015 17:16:28 -0300
> Mauro Carvalho Chehab  wrote:
> 
> > Em Mon, 20 Apr 2015 09:27:20 +0200
> > Patrick Boettcher  escreveu:
> > 
> > > Hi Mauro,
> > > 
> > > Would you please pull the following two patches for finally
> > > mainlining the Technisat SkyStar S2 (and its frontend cx24120).
> > > 
> > > Ideally for 4.1, but I assume it is too late. So for 4.2.
> > 
> > Hi Patrick,
> > 
> > It was too late for 4.1. We typically close the merge for the next
> > Kernel one week before the open of a merge window.
> > 
> > > Please also tell whether a pull-request is OK for you or whether you
> > > prefer patches.
> > 
> > Pull requests work best for me, as it warrants that the patches
> > will be applied in order. Also, I priorize pull requests over usual
> > patches.
> > 
> > However, if you send a pull request, don't forget to also post the
> > patch series, as it helps people to review and comment about the code.
> > 
> > > I'm based on the current media-tree's master. But can rebase myself
> > > on anything you wish for your convenience.
> > 
> > That's fine. You can base on it or on any tag at the Linus tree.
> > 
> > My script will actually convert the pull request into a quilt series
> > of patches, getting only the patches between the range specified at
> > the pull request e-mail.
> > 
> > It is slow, however, if the origin branch is not here, as it needs to
> > download a larger amount of objects, and then use a somewhat complex
> > heuristics to detect the origin branch. 
> > 
> > That's why the be

[linuxtv-media:master 731/732] s5k5baf.c:(.text+0x2073): multiple definition of `v4l2_of_alloc_parse_endpoint'

[kbuild test robot]

tree:   git://linuxtv.org/media_tree.git master
head:   cb0c9e1f6777287e81d9b48c264d980bf5014b48
commit: 698da18e082c8fdfa675bee6338e3f9864d5d7ee [731/732] [media] v4l: of: 
Parse variable length properties --- link-frequencies
config: x86_64-randconfig-ib0-04280406 (attached as .config)
reproduce:
  git checkout 698da18e082c8fdfa675bee6338e3f9864d5d7ee
  # save the attached .config to linux build tree
  make ARCH=x86_64 

All error/warnings:

   drivers/media/i2c/s5k5baf.o: In function `v4l2_of_alloc_parse_endpoint':
>> s5k5baf.c:(.text+0x2073): multiple definition of 
>> `v4l2_of_alloc_parse_endpoint'
   drivers/media/i2c/adv7604.o:adv7604.c:(.text+0x4537): first defined here
   drivers/media/i2c/s5c73m3/built-in.o: In function 
`v4l2_of_alloc_parse_endpoint':
>> (.text+0xddf): multiple definition of `v4l2_of_alloc_parse_endpoint'
   drivers/media/i2c/adv7604.o:adv7604.c:(.text+0x4537): first defined here

---
0-DAY kernel test infrastructureOpen Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
#
# Automatically generated file; DO NOT EDIT.
# Linux/x86_64 4.1.0-rc1 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_X86_64=y
CONFIG_X86=y
CONFIG_INSTRUCTION_DECODER=y
CONFIG_PERF_EVENTS_INTEL_UNCORE=y
CONFIG_OUTPUT_FORMAT="elf64-x86-64"
CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig"
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_MMU=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_GENERIC_ISA_DMA=y
CONFIG_GENERIC_BUG=y
CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y
CONFIG_GENERIC_HWEIGHT=y
CONFIG_ARCH_MAY_HAVE_PC_FDC=y
CONFIG_RWSEM_XCHGADD_ALGORITHM=y
CONFIG_GENERIC_CALIBRATE_DELAY=y
CONFIG_ARCH_HAS_CPU_RELAX=y
CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_ARCH_SUSPEND_POSSIBLE=y
CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y
CONFIG_ARCH_WANT_GENERAL_HUGETLB=y
CONFIG_ZONE_DMA32=y
CONFIG_AUDIT_ARCH=y
CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_X86_64_SMP=y
CONFIG_X86_HT=y
CONFIG_ARCH_HWEIGHT_CFLAGS="-fcall-saved-rdi -fcall-saved-rsi -fcall-saved-rdx 
-fcall-saved-rcx -fcall-saved-r8 -fcall-saved-r9 -fcall-saved-r10 
-fcall-saved-r11"
CONFIG_ARCH_SUPPORTS_UPROBES=y
CONFIG_FIX_EARLYCON_MEM=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y

#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
CONFIG_LOCALVERSION_AUTO=y
CONFIG_HAVE_KERNEL_GZIP=y
CONFIG_HAVE_KERNEL_BZIP2=y
CONFIG_HAVE_KERNEL_LZMA=y
CONFIG_HAVE_KERNEL_XZ=y
CONFIG_HAVE_KERNEL_LZO=y
CONFIG_HAVE_KERNEL_LZ4=y
# CONFIG_KERNEL_GZIP is not set
# CONFIG_KERNEL_BZIP2 is not set
# CONFIG_KERNEL_LZMA is not set
CONFIG_KERNEL_XZ=y
# CONFIG_KERNEL_LZO is not set
# CONFIG_KERNEL_LZ4 is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
# CONFIG_SWAP is not set
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
# CONFIG_POSIX_MQUEUE is not set
CONFIG_CROSS_MEMORY_ATTACH=y
CONFIG_FHANDLE=y
CONFIG_USELIB=y
# CONFIG_AUDIT is not set
CONFIG_HAVE_ARCH_AUDITSYSCALL=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_PROBE=y
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_GENERIC_IRQ_LEGACY_ALLOC_HWIRQ=y
CONFIG_GENERIC_PENDING_IRQ=y
CONFIG_IRQ_DOMAIN=y
# CONFIG_IRQ_DOMAIN_DEBUG is not set
CONFIG_IRQ_FORCED_THREADING=y
CONFIG_SPARSE_IRQ=y
CONFIG_CLOCKSOURCE_WATCHDOG=y
CONFIG_ARCH_CLOCKSOURCE_DATA=y
CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y
CONFIG_GENERIC_TIME_VSYSCALL=y
CONFIG_GENERIC_CLOCKEVENTS=y
CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y
CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y
CONFIG_GENERIC_CMOS_UPDATE=y

#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
# CONFIG_NO_HZ_FULL is not set
# CONFIG_NO_HZ is not set
# CONFIG_HIGH_RES_TIMERS is not set

#
# CPU/Task time and stats accounting
#
CONFIG_VIRT_CPU_ACCOUNTING=y
# CONFIG_TICK_CPU_ACCOUNTING is not set
CONFIG_VIRT_CPU_ACCOUNTING_GEN=y
# CONFIG_IRQ_TIME_ACCOUNTING is not set

#
# RCU Subsystem
#
CONFIG_PREEMPT_RCU=y
CONFIG_SRCU=y
# CONFIG_TASKS_RCU is not set
CONFIG_RCU_STALL_COMMON=y
CONFIG_CONTEXT_TRACKING=y
CONFIG_RCU_USER_QS=y
# CONFIG_CONTEXT_TRACKING_FORCE is not set
CONFIG_RCU_FANOUT=64
CONFIG_RCU_FANOUT_LEAF=16
# CONFIG_RCU_FANOUT_EXACT is not set
# CONFIG_RCU_FAST_NO_HZ is not set
CONFIG_TREE_RCU_TRACE=y
CONFIG_RCU_BOOST=y
CONFIG_RCU_KTHREAD_PRIO=1
CONFIG_RCU_BOOST_DELAY=500
# CONFIG_RCU_NOCB_CPU is not set
# CONFIG_RCU_EXPEDITE_BOOT is not set
CONFIG_BUILD_BIN2C=y
CONFIG_IKCONFIG=y
# CONFIG_IKCONFIG_PROC is not set
CONFIG_LOG_BUF_SHIFT=17
CONFIG_LOG_CPU_MAX_BUF_SHIFT=12
CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y
CONFIG_ARCH_SUPPORTS_NUMA_BALANCING=y
CONFIG_ARCH_SUPPORTS_INT128=y
CONFIG_NUMA_BALANCING=y
# CONFIG_NUMA_BALANCING_DEFAULT_ENABL

Re: [PULL] For 4.2 (or even 4.1?) add support for cx24120/Technisat SkyStar S2

[Jemma Denson]

(Resending to fix reply-all)

Hi Mauro,

Thanks for taking the time to look into this. I'll let Patrick respond 
to the first part regards the pull request - I'll just respond to the 
points you've made about the driver itself.


On 27/04/15 21:16, Mauro Carvalho Chehab wrote:
> +
> +
> +/* Write multiple registers */
> +static int cx24120_writeregN(struct cx24120_state *state,
> +u8 reg, const u8 *values, u16 len, u8 incr)
> +{
> +int ret;
> +u8 buf[5]; /* maximum 4 data bytes at once - flexcop limitation
> +(very limited i2c-interface this one) */
> Hmm... if the limit is at flexcop, then the best is to not be add such
> restriction here, but at the flexcop code, and passing the max limit that
> used for the I2C transfer as a parameter at the attach structure, just
> like other frontend and tuner drivers do.
I had considered doing that - the cx24116 driver does have i2c_wr_max as 
part of it's config struct. The only reason I didn't however was that I 
did consider that it's now quite unlikely this demod would be used in 
anything else so it could probably safely stay fixed for a while yet.


As you say though it would be nicer as a config item, and it shouldn't 
be too much to add in so I'll look into doing it that way.

>
> Also, this limit is hardcoded here. Please use a define instead.
So, if I do have this as a config item then this and the following 
hardcoded values should all disappear...

>
>> +
>> +struct i2c_msg msg = {
>> +.addr = state->config->i2c_addr,
>> +.flags = 0,
>> +.buf = buf,
>> +.len = len };
>> +
>> +while (len) {
>> +buf[0] = reg;
>> +msg.len = len > 4 ? 4 : len;
> Again, don't hardcode the max buf size here.

>
> +
> +/* Wait for tuning */
> +while (delay_cnt >= 0) {
> +cx24120_read_status(fe, &status);
> +if (status & FE_HAS_LOCK)
> +goto tuned;
> +msleep(20);
> +delay_cnt -= 20;
> +}
> I don't see any need for waiting for tune here. This is generally done in
> userspace and at the kthread inside dvb_frontend.c.
>
> Any reason why this has to be done here?

Some point after tuning the cx24120_set_clock_ratios function needs to 
be called and the firmware command CMD_CLOCK_SET sent. The ratios sent 
to this command depend on delivery system, fec & pilot - the latter two 
only available to read after tuning. If this isn't done then the mpeg 
stream doesn't appear.


Is there a better point to set the ratios? Or for another way of asking 
that, is there some other function that will always get hit after tuning?


> +
> +/* Calculate vco from config */
> +static u64 cx24120_calculate_vco(struct cx24120_state *state)
> +{
> +u32 vco;
> +u64 inv_vco, res, xxyyzz;
> +u32 xtal_khz = state->config->xtal_khz;
> +
> +xxyyzz = 0x4ULL;
> xxyyzz? Weird name for a var.
Yes... one of the oddities left from the dissasembled driver I hadn't 
ironed out yet! I'll look into what it's supposed to be doing and rename 
that to something far more sensible.



As time allows I'll get this fixed up as required - would it be best if 
I send patches to here made against Patrick's tree? I presume also one 
patch for each of the coding styles, and then one patch for each of the 
other issues?



Jemma.


--
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: [PULL] For 4.2 (or even 4.1?) add support for cx24120/Technisat SkyStar S2

[Patrick Boettcher]

Thank you for your review Mauro.

In total there are about 3-4 (it's a guess) users of this driver and
this, written-once read-never, code is for a hardware which is very
unlikely to be ever reused again ever. Sometimes I regret that there is
no carpet in OpenSource where you could hide the dirt under ;-).

This driver has been reverse-engineered from a binary-only release - so
looking for logic is not really useful - there will be sections which
are not understandable or would require a certain amount of
investigation and reverse-engineering (I'm thinking of the UNC and BER
reporting, maybe SNR too).

Tough not sure if it is worth the time. Anyone there to convince me?

I'm really surprised that checkpatch.pl hasn't seen any of the
coding-style problems you pointed out, except the printk-usage. I ran
it on the .c-file and not on the patch, is that the problem?

Could we send an additional patch for coding-style or would you prefer
a new patch which has everything inside? This would maintain the
author-attribution of the initial commit.

Sorry for the top-posting.

best regards,
--
Patrick.


On Mon, 27 Apr 2015 17:16:28 -0300
Mauro Carvalho Chehab  wrote:

> Em Mon, 20 Apr 2015 09:27:20 +0200
> Patrick Boettcher  escreveu:
> 
> > Hi Mauro,
> > 
> > Would you please pull the following two patches for finally
> > mainlining the Technisat SkyStar S2 (and its frontend cx24120).
> > 
> > Ideally for 4.1, but I assume it is too late. So for 4.2.
> 
> Hi Patrick,
> 
> It was too late for 4.1. We typically close the merge for the next
> Kernel one week before the open of a merge window.
> 
> > Please also tell whether a pull-request is OK for you or whether you
> > prefer patches.
> 
> Pull requests work best for me, as it warrants that the patches
> will be applied in order. Also, I priorize pull requests over usual
> patches.
> 
> However, if you send a pull request, don't forget to also post the
> patch series, as it helps people to review and comment about the code.
> 
> > I'm based on the current media-tree's master. But can rebase myself
> > on anything you wish for your convenience.
> 
> That's fine. You can base on it or on any tag at the Linus tree.
> 
> My script will actually convert the pull request into a quilt series
> of patches, getting only the patches between the range specified at
> the pull request e-mail.
> 
> It is slow, however, if the origin branch is not here, as it needs to
> download a larger amount of objects, and then use a somewhat complex
> heuristics to detect the origin branch. 
> 
> That's why the best is to either base on media-tree master or on a 
> Linus tag.
> 
> > 
> > Thanks,
> > --
> > Patrick.
> > 
> > 
> > The following changes since commit
> > e183201b9e917daf2530b637b2f34f1d5afb934d:
> > 
> >   [media] uvcvideo: add support for VIDIOC_QUERY_EXT_CTRL
> > (2015-04-10 10:29:27 -0300)
> > 
> > are available in the git repository at:
> > 
> >   https://github.com/pboettch/linux.git cx24120-v2
> > 
> > for you to fetch changes up to
> > 3a6500da369a632c9fd405b1191dcbf5e5e07504:
> > 
> >   [media] cx24120: minor checkpatch fixes (2015-04-17 11:11:40
> > +0200)
> 
> As complained by checkpatch:
> 
> WARNING: added, moved or deleted file(s), does MAINTAINERS need
> updating? #195: 
> new file mode 100644
> 
> Please add an entry at MAINTAINERS for the one that will be
> maintaining it.
> 
> Other comments follow.
> 
> > From c6c7a0454adacfda1ecbd62ae46b23d8af857539 Mon Sep 17 00:00:00
> > 2001 From: Jemma Denson 
> > Date: Tue, 14 Apr 2015 14:04:50 +0200
> > Subject: [media] Add support for TechniSat Skystar S2
> > To: Linux Media Mailing List 
> > Cc: Mauro Carvalho Chehab 
> > 
> > This patch adds support for the Technisat Skystar S2 - this
> > has been tried before but the cx24120 driver was a bit out of shape
> > and it didn't got any further:
> > 
> > https://patchwork.linuxtv.org/patch/10575/
> > 
> > It is an old card, but currently being sold off for next to nothing,
> > so it's proving quite popular of late. Noticing it's quite similar
> > to the cx24116 and cx24117 I've rewritten the driver in a similar
> > way.
> > 
> > There were a few registers and commands from those drivers
> > missing from this one I've tested out and found they do something so
> > they've been added in to speed up tuning and to make get_frontend
> > return something useful.
> > 
> > Signed-off-by: Jemma Denson 
> > Signed-off-by: Patrick.Boettcher 
> > ---
> >  drivers/media/common/b2c2/Kconfig|1 +
> >  drivers/media/common/b2c2/flexcop-fe-tuner.c |   51 +-
> >  drivers/media/common/b2c2/flexcop-misc.c |1 +
> >  drivers/media/common/b2c2/flexcop-reg.h  |1 +
> >  drivers/media/dvb-frontends/Kconfig  |7 +
> >  drivers/media/dvb-frontends/Makefile |1 +
> >  drivers/media/dvb-frontends/cx24120.c| 1577
> > ++
> > drivers/media/dvb-frontends/cx24120.h|   56 + 8 files
> > changed, 1688 insertions(+), 7 dele

Re: [PULL] For 4.2 (or even 4.1?) add support for cx24120/Technisat SkyStar S2

[Mauro Carvalho Chehab]

Em Mon, 20 Apr 2015 09:27:20 +0200
Patrick Boettcher  escreveu:

> Hi Mauro,
> 
> Would you please pull the following two patches for finally
> mainlining the Technisat SkyStar S2 (and its frontend cx24120).
> 
> Ideally for 4.1, but I assume it is too late. So for 4.2.

Hi Patrick,

It was too late for 4.1. We typically close the merge for the next Kernel
one week before the open of a merge window.

> Please also tell whether a pull-request is OK for you or whether you
> prefer patches.

Pull requests work best for me, as it warrants that the patches
will be applied in order. Also, I priorize pull requests over usual
patches.

However, if you send a pull request, don't forget to also post the
patch series, as it helps people to review and comment about the code.

> I'm based on the current media-tree's master. But can rebase myself
> on anything you wish for your convenience.

That's fine. You can base on it or on any tag at the Linus tree.

My script will actually convert the pull request into a quilt series
of patches, getting only the patches between the range specified at
the pull request e-mail.

It is slow, however, if the origin branch is not here, as it needs to
download a larger amount of objects, and then use a somewhat complex
heuristics to detect the origin branch. 

That's why the best is to either base on media-tree master or on a 
Linus tag.

> 
> Thanks,
> --
> Patrick.
> 
> 
> The following changes since commit e183201b9e917daf2530b637b2f34f1d5afb934d:
> 
>   [media] uvcvideo: add support for VIDIOC_QUERY_EXT_CTRL (2015-04-10 
> 10:29:27 -0300)
> 
> are available in the git repository at:
> 
>   https://github.com/pboettch/linux.git cx24120-v2
> 
> for you to fetch changes up to 3a6500da369a632c9fd405b1191dcbf5e5e07504:
> 
>   [media] cx24120: minor checkpatch fixes (2015-04-17 11:11:40 +0200)

As complained by checkpatch:

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#195: 
new file mode 100644

Please add an entry at MAINTAINERS for the one that will be maintaining
it.

Other comments follow.

> From c6c7a0454adacfda1ecbd62ae46b23d8af857539 Mon Sep 17 00:00:00 2001
> From: Jemma Denson 
> Date: Tue, 14 Apr 2015 14:04:50 +0200
> Subject: [media] Add support for TechniSat Skystar S2
> To: Linux Media Mailing List 
> Cc: Mauro Carvalho Chehab 
> 
> This patch adds support for the Technisat Skystar S2 - this
> has been tried before but the cx24120 driver was a bit out of shape
> and it didn't got any further:
> 
> https://patchwork.linuxtv.org/patch/10575/
> 
> It is an old card, but currently being sold off for next to nothing,
> so it's proving quite popular of late. Noticing it's quite similar
> to the cx24116 and cx24117 I've rewritten the driver in a similar way.
> 
> There were a few registers and commands from those drivers
> missing from this one I've tested out and found they do something so
> they've been added in to speed up tuning and to make get_frontend
> return something useful.
> 
> Signed-off-by: Jemma Denson 
> Signed-off-by: Patrick.Boettcher 
> ---
>  drivers/media/common/b2c2/Kconfig|1 +
>  drivers/media/common/b2c2/flexcop-fe-tuner.c |   51 +-
>  drivers/media/common/b2c2/flexcop-misc.c |1 +
>  drivers/media/common/b2c2/flexcop-reg.h  |1 +
>  drivers/media/dvb-frontends/Kconfig  |7 +
>  drivers/media/dvb-frontends/Makefile |1 +
>  drivers/media/dvb-frontends/cx24120.c| 1577 
> ++
>  drivers/media/dvb-frontends/cx24120.h|   56 +
>  8 files changed, 1688 insertions(+), 7 deletions(-)
>  create mode 100644 drivers/media/dvb-frontends/cx24120.c
>  create mode 100644 drivers/media/dvb-frontends/cx24120.h
> 
> diff --git a/drivers/media/common/b2c2/Kconfig 
> b/drivers/media/common/b2c2/Kconfig
> index a8c6cdfaa2f5..e5936380b1e5 100644
> --- a/drivers/media/common/b2c2/Kconfig
> +++ b/drivers/media/common/b2c2/Kconfig
> @@ -14,6 +14,7 @@ config DVB_B2C2_FLEXCOP
>   select DVB_S5H1420 if MEDIA_SUBDRV_AUTOSELECT
>   select DVB_TUNER_ITD1000 if MEDIA_SUBDRV_AUTOSELECT
>   select DVB_ISL6421 if MEDIA_SUBDRV_AUTOSELECT
> + select DVB_CX24120 if MEDIA_SUBDRV_AUTOSELECT
>   select DVB_CX24123 if MEDIA_SUBDRV_AUTOSELECT
>   select MEDIA_TUNER_SIMPLE if MEDIA_SUBDRV_AUTOSELECT
>   select DVB_TUNER_CX24113 if MEDIA_SUBDRV_AUTOSELECT
> diff --git a/drivers/media/common/b2c2/flexcop-fe-tuner.c 
> b/drivers/media/common/b2c2/flexcop-fe-tuner.c
> index 7e14e90d2922..66f6910a1810 100644
> --- a/drivers/media/common/b2c2/flexcop-fe-tuner.c
> +++ b/drivers/media/common/b2c2/flexcop-fe-tuner.c
> @@ -12,6 +12,7 @@
>  #include "cx24113.h"
>  #include "cx24123.h"
>  #include "isl6421.h"
> +#include "cx24120.h"
>  #include "mt352.h"
>  #include "bcm3510.h"
>  #include "nxt200x.h"
> @@ -26,6 +27,16 @@
>  #define FE_SUPPORTED(fe) (defined(CONFIG_DVB_##fe) || \
>   (defined(CONFIG_DVB_##fe##_MODULE) && defined(MODULE)))
>  
> +#if FE_SUPPORTED(B

Re: [PATCH v2] ivtv: use arch_phys_wc_add() and require PAT disabled

[Andy Walls]

On Mon, 2015-04-27 at 09:43 -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" 
> 
> We are burrying direct access to MTRR code support on
> x86 in order to take advantage of PAT. In the future we
> also want to make the default behaviour of ioremap_nocache()
> to use strong UC, use of mtrr_add() on those systems
> would make write-combining void.
> 
> In order to help both enable us to later make strong
> UC default and in order to phase out direct MTRR access
> code port the driver over to arch_phys_wc_add() and
> annotate that the device driver requires systems to
> boot with PAT disabled, with the nopat kernel parameter.
> 
> This is a worthy comprmise given that the hardware is
> really rare these days, and perhaps only some lost souls
> in some third world country are expected to be using this
> feature of the device driver.
> 
> Cc: Andy Walls 

Acked-by: Andy Walls 

Regards,
Andy

> Cc: Mauro Carvalho Chehab 
> Cc: Andy Lutomirski 
> Cc: Suresh Siddha 
> Cc: Ingo Molnar 
> Cc: Thomas Gleixner 
> Cc: Juergen Gross 
> Cc: Daniel Vetter 
> Cc: Dave Airlie 
> Cc: Bjorn Helgaas 
> Cc: Antonino Daplas 
> Cc: Jean-Christophe Plagniol-Villard 
> Cc: Tomi Valkeinen 
> Cc: Dave Hansen 
> Cc: Arnd Bergmann 
> Cc: Michael S. Tsirkin 
> Cc: Stefan Bader 
> Cc: Ville Syrjälä 
> Cc: Mel Gorman 
> Cc: Vlastimil Babka 
> Cc: Borislav Petkov 
> Cc: Davidlohr Bueso 
> Cc: konrad.w...@oracle.com
> Cc: ville.syrj...@linux.intel.com
> Cc: david.vra...@citrix.com
> Cc: jbeul...@suse.com
> Cc: toshi.k...@hp.com
> Cc: Roger Pau Monné 
> Cc: linux-fb...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: ivtv-de...@ivtvdriver.org
> Cc: linux-media@vger.kernel.org
> Cc: xen-de...@lists.xensource.com
> Signed-off-by: Luis R. Rodriguez 
> ---
> 
> This v2 moves the PAT bail out error check on to ivtvfb_init()
> as per Andy's request. It also removes some comment about TODO
> items for PAT.
> 
>  drivers/media/pci/ivtv/Kconfig  |  3 +++
>  drivers/media/pci/ivtv/ivtvfb.c | 58 
> -
>  2 files changed, 26 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
> index dd6ee57e..b2a7f88 100644
> --- a/drivers/media/pci/ivtv/Kconfig
> +++ b/drivers/media/pci/ivtv/Kconfig
> @@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
> This is used in the Hauppauge PVR-350 card. There is a driver
> homepage at .
>  
> +   If you have this hardware you will need to boot with PAT disabled
> +   on your x86 systems, use the nopat kernel parameter.
> +
> To compile this driver as a module, choose M here: the
> module will be called ivtvfb.
> diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
> index 9ff1230..8761e3e 100644
> --- a/drivers/media/pci/ivtv/ivtvfb.c
> +++ b/drivers/media/pci/ivtv/ivtvfb.c
> @@ -44,8 +44,8 @@
>  #include 
>  #include 
>  
> -#ifdef CONFIG_MTRR
> -#include 
> +#ifdef CONFIG_X86_64
> +#include 
>  #endif
>  
>  #include "ivtv-driver.h"
> @@ -155,12 +155,11 @@ struct osd_info {
>   /* Buffer size */
>   u32 video_buffer_size;
>  
> -#ifdef CONFIG_MTRR
>   /* video_base rounded down as required by hardware MTRRs */
>   unsigned long fb_start_aligned_physaddr;
>   /* video_base rounded up as required by hardware MTRRs */
>   unsigned long fb_end_aligned_physaddr;
> -#endif
> + int wc_cookie;
>  
>   /* Store the buffer offset */
>   int set_osd_coords_x;
> @@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
>  static int ivtvfb_init_io(struct ivtv *itv)
>  {
>   struct osd_info *oi = itv->osd_info;
> + /* Find the largest power of two that maps the whole buffer */
> + int size_shift = 31;
>  
>   mutex_lock(&itv->serialize_lock);
>   if (ivtv_init_on_first_open(itv)) {
> @@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
>   oi->video_pbase, oi->video_vbase,
>   oi->video_buffer_size / 1024);
>  
> -#ifdef CONFIG_MTRR
> - {
> - /* Find the largest power of two that maps the whole buffer */
> - int size_shift = 31;
> -
> - while (!(oi->video_buffer_size & (1 << size_shift))) {
> - size_shift--;
> - }
> - size_shift++;
> - oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << 
> size_shift) - 1);
> - oi->fb_end_aligned_physaddr = oi->video_pbase + 
> oi->video_buffer_size;
> - oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
> - oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
> - if (mtrr_add(oi->fb_start_aligned_physaddr,
> - oi->fb_end_aligned_physaddr - 
> oi->fb_start_aligned_physaddr,
> -  MTRR_TYPE_WRCOMB, 1) < 0) {
> - IVTVFB_INFO("disabled mttr\n");
> - o

[PATCH v2] ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

From: "Luis R. Rodriguez" 

We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.

In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.

This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.

Cc: Andy Walls 
Cc: Mauro Carvalho Chehab 
Cc: Andy Lutomirski 
Cc: Suresh Siddha 
Cc: Ingo Molnar 
Cc: Thomas Gleixner 
Cc: Juergen Gross 
Cc: Daniel Vetter 
Cc: Dave Airlie 
Cc: Bjorn Helgaas 
Cc: Antonino Daplas 
Cc: Jean-Christophe Plagniol-Villard 
Cc: Tomi Valkeinen 
Cc: Dave Hansen 
Cc: Arnd Bergmann 
Cc: Michael S. Tsirkin 
Cc: Stefan Bader 
Cc: Ville Syrjälä 
Cc: Mel Gorman 
Cc: Vlastimil Babka 
Cc: Borislav Petkov 
Cc: Davidlohr Bueso 
Cc: konrad.w...@oracle.com
Cc: ville.syrj...@linux.intel.com
Cc: david.vra...@citrix.com
Cc: jbeul...@suse.com
Cc: toshi.k...@hp.com
Cc: Roger Pau Monné 
Cc: linux-fb...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: ivtv-de...@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Signed-off-by: Luis R. Rodriguez 
---

This v2 moves the PAT bail out error check on to ivtvfb_init()
as per Andy's request. It also removes some comment about TODO
items for PAT.

 drivers/media/pci/ivtv/Kconfig  |  3 +++
 drivers/media/pci/ivtv/ivtvfb.c | 58 -
 2 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
  This is used in the Hauppauge PVR-350 card. There is a driver
  homepage at .
 
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86 systems, use the nopat kernel parameter.
+
  To compile this driver as a module, choose M here: the
  module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..8761e3e 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
 #include 
 #include 
 
-#ifdef CONFIG_MTRR
-#include 
+#ifdef CONFIG_X86_64
+#include 
 #endif
 
 #include "ivtv-driver.h"
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
 
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+   int wc_cookie;
 
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
 static int ivtvfb_init_io(struct ivtv *itv)
 {
struct osd_info *oi = itv->osd_info;
+   /* Find the largest power of two that maps the whole buffer */
+   int size_shift = 31;
 
mutex_lock(&itv->serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi->video_pbase, oi->video_vbase,
oi->video_buffer_size / 1024);
 
-#ifdef CONFIG_MTRR
-   {
-   /* Find the largest power of two that maps the whole buffer */
-   int size_shift = 31;
-
-   while (!(oi->video_buffer_size & (1 << size_shift))) {
-   size_shift--;
-   }
-   size_shift++;
-   oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << 
size_shift) - 1);
-   oi->fb_end_aligned_physaddr = oi->video_pbase + 
oi->video_buffer_size;
-   oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
-   oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
-   if (mtrr_add(oi->fb_start_aligned_physaddr,
-   oi->fb_end_aligned_physaddr - 
oi->fb_start_aligned_physaddr,
-MTRR_TYPE_WRCOMB, 1) < 0) {
-   IVTVFB_INFO("disabled mttr\n");
-   oi->fb_start_aligned_physaddr = 0;
-   oi->fb_end_aligned_physaddr = 0;
-   }
-   }
-#endif
-
+   while (!(oi->video_buffer_size & (1 << size_shift)))
+   size_shift--;
+   size_shift++;
+   oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1

Re: [PATCH] [media] ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

On Sat, Apr 25, 2015 at 07:12:05AM -0400, Andy Walls wrote:
> Hi Luis,
> 
> Sorry for the late reply.
> 
> Thank you for the patch! See my comments below:
> 
> On Wed, 2015-04-22 at 12:33 -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" 
> > 
> > We are burrying direct access to MTRR code support on
> > x86 in order to take advantage of PAT. In the future we
> > also want to make the default behaviour of ioremap_nocache()
> > to use strong UC, use of mtrr_add() on those systems
> > would make write-combining void.
> > 
> > In order to help both enable us to later make strong
> > UC default and in order to phase out direct MTRR access
> > code port the driver over to arch_phys_wc_add() and
> > annotate that the device driver requires systems to
> > boot with PAT disabled, with the nopat kernel parameter.
> > 
> > This is a worthy comprmise given that the hardware is
> > really rare these days,
> 
> I'm OK with the compromise solution.  It makes sense.

OK great!

> > diff --git a/drivers/media/pci/ivtv/ivtvfb.c 
> > b/drivers/media/pci/ivtv/ivtvfb.c
> > index 9ff1230..552408b 100644
> > --- a/drivers/media/pci/ivtv/ivtvfb.c
> > +++ b/drivers/media/pci/ivtv/ivtvfb.c
> > @@ -1120,6 +1121,7 @@ static int ivtvfb_init_io(struct ivtv *itv)
> > oi->video_buffer_size = 1704960;
> >  
> > oi->video_pbase = itv->base_addr + IVTV_DECODER_OFFSET + 
> > oi->video_rbase;
> > +   /* XXX: split this for PAT */
> 
> Please remove this comment. 

Done.

> > @@ -1190,6 +1172,13 @@ static int ivtvfb_init_card(struct ivtv *itv)
> >  {
> > int rc;
> >  
> > +#ifdef CONFIG_X86_64
> > +   if (WARN(pat_enabled,
> 
> This check might be better placed in ivtvfb_init().  This check is going
> to have the same result for every PVR-350 card in the system that is
> found by ivtvfb.

OK moved!

> > +"ivtv needs PAT disabled, boot with nopat kernel 
> > parameter\n")) {
> 
> This needs to read "ivtvfb needs [...]" to avoid user confusion with the
> main ivtv driver module.

OK!

> This change is the only one I really care about.  Then I can give my
> Ack.

OK!

  Luis
--
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 0/3] Add media bdisp driver for stihxxx platforms

[Hans Verkuil]

Hi Fabien,

Thank you for this driver! Good to see V4L2 support for this SoC.

I did a quick initial scan over the driver and there are a few things that
need to be addressed:

- I think bdisp as the driver name is a bit generic, perhaps something like
  stih4xx-bdisp might be more appropriate. Similar to the exynos-* drivers.

- Replace cropcap/g_crop/s_crop by the g/s_selection ioctls. The old ioctls
  are no longer supported for new drivers (the v4l2 core will automatically
  add support for those ioctls if g/s_selection is implemented in the driver).
  Read careful how crop and compose rectangles are used in a m2m device. I
  would expect that you implement cropping for the BUF_TYPE_VIDEO_OUTPUT side
  (i.e. memory to hardware) and implement composing for the 
BUF_TYPE_VIDEO_CAPTURE
  side (i.e. hardware to memory).

  If the hardware also support composition for output or cropping for capture,
  then let me know: in that case you will likely have to implement support for
  V4L2_SEL_TGT_NATIVE_SIZE as well.

- Several ioctl and fop helpers were added to media/v4l2-mem2mem.h (e.g.
  v4l2_m2m_ioctl_reqbufs, v4l2_m2m_fop_mmap, etc.). Use these instead of
  rolling your own.

- I would like to see the output of these v4l2-compliance commands:

v4l2-compliance
v4l2-compliance -s
v4l2-compliance -f

  In all fairness: mem2mem devices are not often tested using v4l2-compliance
  and there may be problems testing this (-f will likely fail), but I still
  like to see the output so I know what works and what doesn't.

  Please use the latest v4l2-compliance code from the v4l-utils.git repository.
  I won't accept the driver unless I see the results of these compliance tests:
  running this is required for new drivers since it is a great way of verifying
  the completeness of your driver.

Regards,

Hans

On 04/27/2015 05:56 PM, Fabien Dessenne wrote:
> This series of patches adds the support of v4l2 2D blitter driver for
> STMicroelectronics SOC.
> 
> The following features are supported and tested:
> - Color format conversion (RGB32, RGB24, RGB16, NV12, YUV420P)
> - Copy
> - Scale
> - Flip
> - Deinterlace
> - Wide (4K) picture support
> - Crop
> 
> This driver uses the v4l2 mem2mem framework and its implementation was largely
> inspired by the Exynos G-Scaler (exynos-gsc) driver.
> 
> The driver is mainly implemented across two files:
> - bdisp-v4l2.c
> - bdisp-hw.c
> bdisp-v4l2.c uses v4l2_m2m to manage the V4L2 interface with the userland. It
> calls the HW services that are implemented in bdisp-hw.c.
> 
> The additional bdisp-debug.c file manages some debugfs entries.
> 
> Fabien Dessenne (3):
>   [media] bdisp: add DT bindings documentation
>   [media] bdisp: 2D blitter driver using v4l2 mem2mem framework
>   [media] bdisp: add debug file system
> 
>  .../devicetree/bindings/media/st,stih4xx.txt   |   32 +
>  drivers/media/platform/Kconfig |   10 +
>  drivers/media/platform/Makefile|2 +
>  drivers/media/platform/bdisp/Kconfig   |9 +
>  drivers/media/platform/bdisp/Makefile  |3 +
>  drivers/media/platform/bdisp/bdisp-debug.c |  668 +
>  drivers/media/platform/bdisp/bdisp-filter.h|  346 +
>  drivers/media/platform/bdisp/bdisp-hw.c|  823 +++
>  drivers/media/platform/bdisp/bdisp-reg.h   |  235 +++
>  drivers/media/platform/bdisp/bdisp-v4l2.c  | 1492 
> 
>  drivers/media/platform/bdisp/bdisp.h   |  220 +++
>  11 files changed, 3840 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/st,stih4xx.txt
>  create mode 100644 drivers/media/platform/bdisp/Kconfig
>  create mode 100644 drivers/media/platform/bdisp/Makefile
>  create mode 100644 drivers/media/platform/bdisp/bdisp-debug.c
>  create mode 100644 drivers/media/platform/bdisp/bdisp-filter.h
>  create mode 100644 drivers/media/platform/bdisp/bdisp-hw.c
>  create mode 100644 drivers/media/platform/bdisp/bdisp-reg.h
>  create mode 100644 drivers/media/platform/bdisp/bdisp-v4l2.c
>  create mode 100644 drivers/media/platform/bdisp/bdisp.h
> 

--
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

[PATCH 2/3] [media] bdisp: 2D blitter driver using v4l2 mem2mem framework

[Fabien Dessenne]

This v4l2 mem2mem driver is a 2D blitter for STMicroelectronics SoC.
It uses the v4l2 mem2mem framework.

The following features are supported and tested:
- Color format conversion (RGB32, RGB24, RGB16, NV12, YUV420P)
- Copy
- Scale
- Flip
- Deinterlace
- Wide (4K) picture support
- Crop

Signed-off-by: Fabien Dessenne 
---
 drivers/media/platform/Kconfig  |   10 +
 drivers/media/platform/Makefile |2 +
 drivers/media/platform/bdisp/Kconfig|9 +
 drivers/media/platform/bdisp/Makefile   |3 +
 drivers/media/platform/bdisp/bdisp-filter.h |  346 +++
 drivers/media/platform/bdisp/bdisp-hw.c |  783 ++
 drivers/media/platform/bdisp/bdisp-reg.h|  235 +
 drivers/media/platform/bdisp/bdisp-v4l2.c   | 1477 +++
 drivers/media/platform/bdisp/bdisp.h|  190 
 9 files changed, 3055 insertions(+)
 create mode 100644 drivers/media/platform/bdisp/Kconfig
 create mode 100644 drivers/media/platform/bdisp/Makefile
 create mode 100644 drivers/media/platform/bdisp/bdisp-filter.h
 create mode 100644 drivers/media/platform/bdisp/bdisp-hw.c
 create mode 100644 drivers/media/platform/bdisp/bdisp-reg.h
 create mode 100644 drivers/media/platform/bdisp/bdisp-v4l2.c
 create mode 100644 drivers/media/platform/bdisp/bdisp.h

diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 2e30be5..bf149c2 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -210,6 +210,16 @@ config VIDEO_SAMSUNG_EXYNOS_GSC
help
  This is a v4l2 driver for Samsung EXYNOS5 SoC G-Scaler.
 
+config VIDEO_BDISP
+   tristate "STMicroelectronics BDISP 2D blitter driver"
+   depends on VIDEO_DEV && VIDEO_V4L2
+   depends on ARCH_STI || COMPILE_TEST
+   depends on HAS_DMA
+   select VIDEOBUF2_DMA_CONTIG
+   select V4L2_MEM2MEM_DEV
+   help
+ This v4l2 mem2mem driver is a 2D blitter for STMicroelectronics SoC.
+
 config VIDEO_SH_VEU
tristate "SuperH VEU mem2mem video processing driver"
depends on VIDEO_DEV && VIDEO_V4L2 && HAS_DMA
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 3ec1547..cad3c0f 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -34,6 +34,8 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_TV)+= s5p-tv/
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D)+= s5p-g2d/
 obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC) += exynos-gsc/
 
+obj-$(CONFIG_VIDEO_BDISP)  += bdisp/
+
 obj-$(CONFIG_BLACKFIN)  += blackfin/
 
 obj-$(CONFIG_ARCH_DAVINCI) += davinci/
diff --git a/drivers/media/platform/bdisp/Kconfig 
b/drivers/media/platform/bdisp/Kconfig
new file mode 100644
index 000..3f6de4f
--- /dev/null
+++ b/drivers/media/platform/bdisp/Kconfig
@@ -0,0 +1,9 @@
+config VIDEO_BDISP
+   tristate "STMicroelectronics BDISP 2D blitter driver"
+   depends on VIDEO_DEV && VIDEO_V4L2
+   select VIDEOBUF2_DMA_CONTIG
+   select V4L2_MEM2MEM_DEV
+   help
+This v4l2 mem2mem driver is a 2D blitter for STMicroelectronics SoC.
+To compile this driver as a module, choose M here: the module will
+be called bdisp.ko.
diff --git a/drivers/media/platform/bdisp/Makefile 
b/drivers/media/platform/bdisp/Makefile
new file mode 100644
index 000..ae347a4
--- /dev/null
+++ b/drivers/media/platform/bdisp/Makefile
@@ -0,0 +1,3 @@
+obj-$(CONFIG_VIDEO_BDISP) := bdisp.o
+
+bdisp-objs := bdisp-v4l2.o bdisp-hw.o
diff --git a/drivers/media/platform/bdisp/bdisp-filter.h 
b/drivers/media/platform/bdisp/bdisp-filter.h
new file mode 100644
index 000..fc8c54f
--- /dev/null
+++ b/drivers/media/platform/bdisp/bdisp-filter.h
@@ -0,0 +1,346 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2014
+ * Authors: Fabien Dessenne  for STMicroelectronics.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#define BDISP_HF_NB 64
+#define BDISP_VF_NB 40
+
+/**
+ * struct bdisp_filter_h_spec - Horizontal filter specification
+ *
+ * @min:min scale factor for this filter (6.10 fixed point)
+ * @max:max scale factor for this filter (6.10 fixed point)
+ * coef:filter coefficients
+ */
+struct bdisp_filter_h_spec {
+   const u16 min;
+   const u16 max;
+   const u8 coef[BDISP_HF_NB];
+};
+
+static const struct bdisp_filter_h_spec bdisp_h_spec[] = {
+   {
+   .min = 0,
+   .max = 921,
+   .coef = {
+   0x00, 0x00, 0x00, 0x00, 0x40, 0x00, 0x00, 0x00,
+   0x00, 0x00, 0xff, 0x07, 0x3d, 0xfc, 0x01, 0x00,
+   0x00, 0x01, 0xfd, 0x11, 0x36, 0xf9, 0x02, 0x00,
+   0x00, 0x01, 0xfb, 0x1b, 0x2e, 0xf9, 0x02, 0x00,
+   0x00, 0x01, 0xf9, 0x26, 0x26, 0xf9, 0x01, 0x00,
+   0x00, 0x02, 0xf9, 0x30, 0x19, 0xfb, 0x01, 0x00,
+   0x00, 

[PATCH 3/3] [media] bdisp: add debug file system

[Fabien Dessenne]

Creates 5 debugfs entries to dump the last HW request, the last HW node
(=command), the HW registers and the recent HW performance (time & fps)

Signed-off-by: Fabien Dessenne 
---
 drivers/media/platform/bdisp/Makefile  |   2 +-
 drivers/media/platform/bdisp/bdisp-debug.c | 668 +
 drivers/media/platform/bdisp/bdisp-hw.c|  40 ++
 drivers/media/platform/bdisp/bdisp-v4l2.c  |  17 +-
 drivers/media/platform/bdisp/bdisp.h   |  30 ++
 5 files changed, 755 insertions(+), 2 deletions(-)
 create mode 100644 drivers/media/platform/bdisp/bdisp-debug.c

diff --git a/drivers/media/platform/bdisp/Makefile 
b/drivers/media/platform/bdisp/Makefile
index ae347a4..2cc2e67 100644
--- a/drivers/media/platform/bdisp/Makefile
+++ b/drivers/media/platform/bdisp/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_VIDEO_BDISP) := bdisp.o
 
-bdisp-objs := bdisp-v4l2.o bdisp-hw.o
+bdisp-objs := bdisp-v4l2.o bdisp-hw.o bdisp-debug.o
diff --git a/drivers/media/platform/bdisp/bdisp-debug.c 
b/drivers/media/platform/bdisp/bdisp-debug.c
new file mode 100644
index 000..7c3a632
--- /dev/null
+++ b/drivers/media/platform/bdisp/bdisp-debug.c
@@ -0,0 +1,668 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2014
+ * Authors: Fabien Dessenne  for STMicroelectronics.
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include 
+#include 
+
+#include "bdisp.h"
+#include "bdisp-filter.h"
+#include "bdisp-reg.h"
+
+void bdisp_dbg_perf_begin(struct bdisp_dev *bdisp)
+{
+   bdisp->dbg.hw_start = ktime_get();
+}
+
+void bdisp_dbg_perf_end(struct bdisp_dev *bdisp)
+{
+   s64 time_us;
+
+   time_us = ktime_us_delta(ktime_get(), bdisp->dbg.hw_start);
+
+   if (!bdisp->dbg.min_duration)
+   bdisp->dbg.min_duration = time_us;
+   else
+   bdisp->dbg.min_duration = min(time_us, bdisp->dbg.min_duration);
+
+   bdisp->dbg.last_duration = time_us;
+   bdisp->dbg.max_duration = max(time_us, bdisp->dbg.max_duration);
+   bdisp->dbg.tot_duration += time_us;
+}
+
+static void bdisp_dbg_dump_ins(struct seq_file *s, u32 val)
+{
+   seq_printf(s, "INS\t0x%08X\t", val);
+
+   switch (val & BLT_INS_S1_MASK) {
+   case BLT_INS_S1_OFF:
+   break;
+   case BLT_INS_S1_MEM:
+   seq_puts(s, "SRC1=mem - ");
+   break;
+   case BLT_INS_S1_CF:
+   seq_puts(s, "SRC1=ColorFill - ");
+   break;
+   case BLT_INS_S1_COPY:
+   seq_puts(s, "SRC1=copy - ");
+   break;
+   case BLT_INS_S1_FILL:
+   seq_puts(s, "SRC1=fil - ");
+   break;
+   default:
+   seq_puts(s, "SRC1=??? - ");
+   break;
+   }
+
+   switch (val & BLT_INS_S2_MASK) {
+   case BLT_INS_S2_OFF:
+   break;
+   case BLT_INS_S2_MEM:
+   seq_puts(s, "SRC2=mem - ");
+   break;
+   case BLT_INS_S2_CF:
+   seq_puts(s, "SRC2=ColorFill - ");
+   break;
+   default:
+   seq_puts(s, "SRC2=??? - ");
+   break;
+   }
+
+   if ((val & BLT_INS_S3_MASK) == BLT_INS_S3_MEM)
+   seq_puts(s, "SRC3=mem - ");
+
+   if (val & BLT_INS_IVMX)
+   seq_puts(s, "IVMX - ");
+   if (val & BLT_INS_CLUT)
+   seq_puts(s, "CLUT - ");
+   if (val & BLT_INS_SCALE)
+   seq_puts(s, "Scale - ");
+   if (val & BLT_INS_FLICK)
+   seq_puts(s, "Flicker - ");
+   if (val & BLT_INS_CLIP)
+   seq_puts(s, "Clip - ");
+   if (val & BLT_INS_CKEY)
+   seq_puts(s, "ColorKey - ");
+   if (val & BLT_INS_OVMX)
+   seq_puts(s, "OVMX - ");
+   if (val & BLT_INS_DEI)
+   seq_puts(s, "Deint - ");
+   if (val & BLT_INS_PMASK)
+   seq_puts(s, "PlaneMask - ");
+   if (val & BLT_INS_VC1R)
+   seq_puts(s, "VC1R - ");
+   if (val & BLT_INS_ROTATE)
+   seq_puts(s, "Rotate - ");
+   if (val & BLT_INS_GRAD)
+   seq_puts(s, "GradFill - ");
+   if (val & BLT_INS_AQLOCK)
+   seq_puts(s, "AQLock - ");
+   if (val & BLT_INS_PACE)
+   seq_puts(s, "Pace - ");
+   if (val & BLT_INS_IRQ)
+   seq_puts(s, "IRQ - ");
+
+   seq_puts(s, "\n");
+}
+
+static void bdisp_dbg_dump_tty(struct seq_file *s, u32 val)
+{
+   seq_printf(s, "TTY\t0x%08X\t", val);
+   seq_printf(s, "Pitch=%d - ", val & 0x);
+
+   switch ((val & BLT_TTY_COL_MASK) >> BLT_TTY_COL_SHIFT) {
+   case BDISP_RGB565:
+   seq_puts(s, "RGB565 - ");
+   break;
+   case BDISP_XRGB:
+   seq_puts(s, "xRGB888 - ");
+   break;
+   case BDISP_ARGB:
+   seq_puts(s, "ARGB - ");
+   break;
+   case BDISP_NV12:
+   seq_puts(s, "NV12 - ");
+   break;
+   case BDISP_YUV_3B:
+

[PATCH 0/3] Add media bdisp driver for stihxxx platforms

[Fabien Dessenne]

This series of patches adds the support of v4l2 2D blitter driver for
STMicroelectronics SOC.

The following features are supported and tested:
- Color format conversion (RGB32, RGB24, RGB16, NV12, YUV420P)
- Copy
- Scale
- Flip
- Deinterlace
- Wide (4K) picture support
- Crop

This driver uses the v4l2 mem2mem framework and its implementation was largely
inspired by the Exynos G-Scaler (exynos-gsc) driver.

The driver is mainly implemented across two files:
- bdisp-v4l2.c
- bdisp-hw.c
bdisp-v4l2.c uses v4l2_m2m to manage the V4L2 interface with the userland. It
calls the HW services that are implemented in bdisp-hw.c.

The additional bdisp-debug.c file manages some debugfs entries.

Fabien Dessenne (3):
  [media] bdisp: add DT bindings documentation
  [media] bdisp: 2D blitter driver using v4l2 mem2mem framework
  [media] bdisp: add debug file system

 .../devicetree/bindings/media/st,stih4xx.txt   |   32 +
 drivers/media/platform/Kconfig |   10 +
 drivers/media/platform/Makefile|2 +
 drivers/media/platform/bdisp/Kconfig   |9 +
 drivers/media/platform/bdisp/Makefile  |3 +
 drivers/media/platform/bdisp/bdisp-debug.c |  668 +
 drivers/media/platform/bdisp/bdisp-filter.h|  346 +
 drivers/media/platform/bdisp/bdisp-hw.c|  823 +++
 drivers/media/platform/bdisp/bdisp-reg.h   |  235 +++
 drivers/media/platform/bdisp/bdisp-v4l2.c  | 1492 
 drivers/media/platform/bdisp/bdisp.h   |  220 +++
 11 files changed, 3840 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/st,stih4xx.txt
 create mode 100644 drivers/media/platform/bdisp/Kconfig
 create mode 100644 drivers/media/platform/bdisp/Makefile
 create mode 100644 drivers/media/platform/bdisp/bdisp-debug.c
 create mode 100644 drivers/media/platform/bdisp/bdisp-filter.h
 create mode 100644 drivers/media/platform/bdisp/bdisp-hw.c
 create mode 100644 drivers/media/platform/bdisp/bdisp-reg.h
 create mode 100644 drivers/media/platform/bdisp/bdisp-v4l2.c
 create mode 100644 drivers/media/platform/bdisp/bdisp.h

-- 
1.9.1

--
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

[PATCH 1/3] [media] bdisp: add DT bindings documentation

[Fabien Dessenne]

This adds DT binding documentation for STMicroelectronics bdisp driver.

Signed-off-by: Fabien Dessenne 
---
 .../devicetree/bindings/media/st,stih4xx.txt   | 32 ++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/st,stih4xx.txt

diff --git a/Documentation/devicetree/bindings/media/st,stih4xx.txt 
b/Documentation/devicetree/bindings/media/st,stih4xx.txt
new file mode 100644
index 000..df655cd
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/st,stih4xx.txt
@@ -0,0 +1,32 @@
+STMicroelectronics stih4xx platforms
+
+bdisp: 2D blitter for STMicroelectronics SoC.
+
+Required properties:
+- compatible: should be "st,stih407-bdisp".
+- reg: BDISP physical address location and length.
+- interrupts: BDISP interrupt number.
+- clocks: from common clock binding: handle hardware IP needed clocks, the
+  number of clocks may depend on the SoC type.
+  See ../clocks/clock-bindings.txt for details.
+- clock-names: names of the clocks listed in clocks property in the same order.
+
+Example:
+
+   bdisp0:bdisp@9f1 {
+   compatible = "st,stih407-bdisp";
+   reg = <0x9f1 0x1000>;
+   interrupts = ;
+   clock-names = "bdisp";
+   clocks = <&clk_s_c0_flexgen CLK_IC_BDISP_0>;
+   };
+
+Aliases:
+Each BDISP should have a numbered alias in the aliases node, in the form of
+bdispN, N = 0 or 1.
+
+Example:
+
+   aliases {
+   bdisp0 = &bdisp0;
+   };
-- 
1.9.1

--
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

gute Neuigkeiten

[nmzdr]

gute Neuigkeiten
Handy, Laptop, TV-Raum, Goultard .
Versand ist kostenlos
Alle unsere Produkte können Preferred Rabatt von 50% angeboten werden
si  te:   weacaoo . com

[GIT PULL FOR v4.2] Fix compiler warnings, vivid improvements

[Hans Verkuil]

The following changes since commit b3e5ced63e051e8f911b795ac5b06229a5328f7b:

  Merge tag 'v4.1-rc1' into patchwork (2015-04-27 10:32:45 -0300)

are available in the git repository at:

  git://linuxtv.org/hverkuil/media_tree.git for-v4.2f

for you to fetch changes up to c2abe92094034a3bdd8698ca5992b1b8239e975d:

  radio-bcm2048: fix compiler warning (2015-04-27 16:19:41 +0200)


Hans Verkuil (11):
  vivid-tpg: add tpg_log_status()
  vivid-tpg: add full range SMPTE 240M support
  vivid-tpg: add full range BT.2020 support
  vivid-tpg: add full range BT.2020C support
  vivid-tpg: fix XV601/709 Y'CbCr encoding
  DocBook/media: attemps -> attempts
  s5c73m3/s5k5baf/s5k6aa: fix compiler warnings
  s3c-camif: fix compiler warnings
  cx24123/mb86a20s/s921: fix compiler warnings
  dib8000: fix compiler warning
  radio-bcm2048: fix compiler warning

Philipp Zabel (1):
  vivid: add 1080p capture at 2 fps and 5 fps to webcam emulation

 Documentation/DocBook/media/v4l/media-func-open.xml |   2 +-
 drivers/media/dvb-frontends/cx24123.h   |   2 +-
 drivers/media/dvb-frontends/dib8000.h   |   2 +-
 drivers/media/dvb-frontends/mb86a20s.h  |   2 +-
 drivers/media/dvb-frontends/s921.h  |   2 +-
 drivers/media/i2c/s5c73m3/s5c73m3-core.c|   2 +-
 drivers/media/i2c/s5k5baf.c |   2 +-
 drivers/media/i2c/s5k6aa.c  |   2 +-
 drivers/media/platform/s3c-camif/camif-capture.c|   4 +--
 drivers/media/platform/vivid/vivid-core.c   |  13 -
 drivers/media/platform/vivid/vivid-tpg.c| 109 
+++-
 drivers/media/platform/vivid/vivid-tpg.h|   1 +
 drivers/media/platform/vivid/vivid-vid-cap.c|  13 +
 drivers/staging/media/bcm2048/radio-bcm2048.c   |   3 +-
 14 files changed, 122 insertions(+), 37 deletions(-)
--
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 v5 3/5] [media] videobuf2: return -EPIPE from DQBUF after the last buffer

[Hans Verkuil]

On 04/20/2015 10:28 AM, Philipp Zabel wrote:
> If the last buffer was dequeued from a capture queue, let poll return
> immediately and let DQBUF return -EPIPE to signal there will no more
> buffers to dequeue until STREAMOFF.
> The driver signals the last buffer by setting the V4L2_BUF_FLAG_LAST.
> To reenable dequeuing on the capture queue, the driver must explicitly
> call vb2_clear_last_buffer_queued. The last buffer queued flag is
> cleared automatically during STREAMOFF.
> 
> Signed-off-by: Philipp Zabel 

Acked-by: Hans Verkuil 

Thanks!

Hans


> ---
> Changes since v4:
>  - Split out DocBook changes into a separate patch.
>  - Documented last_buffer_dequeued flag in vb2_queue comment.
>  - Moved last_buffer_dequeued check from vb2_internal_dqbuf
>into __vb2_wait_for_done_vb.
>  - Dropped superfluous check in vb2_poll.
> ---
>  drivers/media/v4l2-core/v4l2-mem2mem.c   | 10 +-
>  drivers/media/v4l2-core/videobuf2-core.c | 19 ++-
>  include/media/videobuf2-core.h   | 13 +
>  3 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c 
> b/drivers/media/v4l2-core/v4l2-mem2mem.c
> index 80c588f..1b5b432 100644
> --- a/drivers/media/v4l2-core/v4l2-mem2mem.c
> +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c
> @@ -564,8 +564,16 @@ unsigned int v4l2_m2m_poll(struct file *file, struct 
> v4l2_m2m_ctx *m2m_ctx,
>  
>   if (list_empty(&src_q->done_list))
>   poll_wait(file, &src_q->done_wq, wait);
> - if (list_empty(&dst_q->done_list))
> + if (list_empty(&dst_q->done_list)) {
> + /*
> +  * If the last buffer was dequeued from the capture queue,
> +  * return immediately. DQBUF will return -EPIPE.
> +  */
> + if (dst_q->last_buffer_dequeued)
> + return rc | POLLIN | POLLRDNORM;
> +
>   poll_wait(file, &dst_q->done_wq, wait);
> + }
>  
>   if (m2m_ctx->m2m_dev->m2m_ops->lock)
>   m2m_ctx->m2m_dev->m2m_ops->lock(m2m_ctx->priv);
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c 
> b/drivers/media/v4l2-core/videobuf2-core.c
> index cc16e76..3b2188e 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1918,6 +1918,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, 
> int nonblocking)
>   return -EIO;
>   }
>  
> + if (q->last_buffer_dequeued) {
> + dprintk(3, "last buffer dequeued already, will not wait 
> for buffers\n");
> + return -EPIPE;
> + }
> +
>   if (!list_empty(&q->done_list)) {
>   /*
>* Found a buffer that we were waiting for.
> @@ -2073,6 +2078,9 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, 
> struct v4l2_buffer *b, bool n
>   /* Remove from videobuf queue */
>   list_del(&vb->queued_entry);
>   q->queued_count--;
> + if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
> + vb->v4l2_buf.flags & V4L2_BUF_FLAG_LAST)
> + q->last_buffer_dequeued = true;
>   /* go back to dequeued state */
>   __vb2_dqbuf(vb);
>  
> @@ -2286,6 +2294,7 @@ static int vb2_internal_streamoff(struct vb2_queue *q, 
> enum v4l2_buf_type type)
>*/
>   __vb2_queue_cancel(q);
>   q->waiting_for_buffers = !V4L2_TYPE_IS_OUTPUT(q->type);
> + q->last_buffer_dequeued = false;
>  
>   dprintk(3, "successful\n");
>   return 0;
> @@ -2628,8 +2637,16 @@ unsigned int vb2_poll(struct vb2_queue *q, struct file 
> *file, poll_table *wait)
>   if (V4L2_TYPE_IS_OUTPUT(q->type) && q->queued_count < q->num_buffers)
>   return res | POLLOUT | POLLWRNORM;
>  
> - if (list_empty(&q->done_list))
> + if (list_empty(&q->done_list)) {
> + /*
> +  * If the last buffer was dequeued from a capture queue,
> +  * return immediately. DQBUF will return -EPIPE.
> +  */
> + if (q->last_buffer_dequeued)
> + return res | POLLIN | POLLRDNORM;
> +
>   poll_wait(file, &q->done_wq, wait);
> + }
>  
>   /*
>* Take first buffer available for dequeuing.
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index bd2cec2..a689d25 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -390,6 +390,9 @@ struct v4l2_fh;
>   * @waiting_for_buffers: used in poll() to check if vb2 is still waiting for
>   *   buffers. Only set for capture queues if qbuf has not yet been
>   *   called since poll() needs to return POLLERR in that situation.
> + * @last_buffer_dequeued: used in poll() and DQBUF to immediately return if 
> the
> + *   last decoded buffer was already dequeued. Set for capture queues
> + *   when a buffer with the V4L2_BUF_FLAG_LAST 

Re: [PATCH v5 2/5] [media] videodev2: Add V4L2_BUF_FLAG_LAST

[Hans Verkuil]

On 04/20/2015 10:28 AM, Philipp Zabel wrote:
> From: Peter Seiderer 
> 
> This v4l2_buffer flag can be used by drivers to mark a capture buffer
> as the last generated buffer, for example after a V4L2_DEC_CMD_STOP
> command was issued.
> 
> Signed-off-by: Peter Seiderer 
> Signed-off-by: Philipp Zabel 

Acked-by: Hans Verkuil 

Thanks!

Hans

> ---
> Changes since v4:
>  - Split out DocBook changes into a separate patch.
> ---
>  include/trace/events/v4l2.h| 3 ++-
>  include/uapi/linux/videodev2.h | 2 ++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h
> index b9bb1f2..32c33aa 100644
> --- a/include/trace/events/v4l2.h
> +++ b/include/trace/events/v4l2.h
> @@ -58,7 +58,8 @@
>   { V4L2_BUF_FLAG_TIMESTAMP_MASK,  "TIMESTAMP_MASK" },  \
>   { V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN,   "TIMESTAMP_UNKNOWN" },   \
>   { V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC, "TIMESTAMP_MONOTONIC" }, \
> - { V4L2_BUF_FLAG_TIMESTAMP_COPY,  "TIMESTAMP_COPY" })
> + { V4L2_BUF_FLAG_TIMESTAMP_COPY,  "TIMESTAMP_COPY" },  \
> + { V4L2_BUF_FLAG_LAST,"LAST" })
>  
>  #define show_timecode_flags(flags) \
>   __print_flags(flags, "|", \
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index fbdc360..c642c10 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -809,6 +809,8 @@ struct v4l2_buffer {
>  #define V4L2_BUF_FLAG_TSTAMP_SRC_MASK0x0007
>  #define V4L2_BUF_FLAG_TSTAMP_SRC_EOF 0x
>  #define V4L2_BUF_FLAG_TSTAMP_SRC_SOE 0x0001
> +/* mem2mem encoder/decoder */
> +#define V4L2_BUF_FLAG_LAST   0x0010
>  
>  /**
>   * struct v4l2_exportbuffer - export of video buffer as DMABUF file 
> descriptor
> 

--
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 v5 1/5] [media] DocBook media: document mem2mem draining flow

[Hans Verkuil]

On 04/20/2015 10:28 AM, Philipp Zabel wrote:
> Document the interaction between VIDIOC_DECODER_CMD V4L2_DEC_CMD_STOP and
> VIDIOC_ENCODER_CMD V4L2_ENC_CMD_STOP to start the draining, the V4L2_EVENT_EOS
> event signalling all capture buffers are finished and ready to be dequeud,
> the new V4L2_BUF_FLAG_LAST buffer flag indicating the last buffer being 
> dequeued
> from the capture queue, and the poll and VIDIOC_DQBUF ioctl return values once
> the queue is drained.

Actually, we document the m2m draining flow, but the same thing is true for
MPEG capture devices like hdpvr that support ENCODER_CMD. So I don't think this
is specific to m2m devices.

Regards,

Hans

> 
> Signed-off-by: Philipp Zabel 
> ---
> Changes since v4:
>  - Split out documentation changes into a separate patch
>  - Changed wording following Pawel's suggestions.
> ---
>  Documentation/DocBook/media/v4l/io.xml | 10 ++
>  Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml |  9 -
>  Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml |  8 +++-
>  Documentation/DocBook/media/v4l/vidioc-qbuf.xml|  8 
>  4 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml 
> b/Documentation/DocBook/media/v4l/io.xml
> index 1c17f80..f561037 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -1129,6 +1129,16 @@ in this buffer has not been created by the CPU but by 
> some DMA-capable unit,
>  in which case caches have not been used.
> 
> 
> + V4L2_BUF_FLAG_LAST
> + 0x0010
> + Last buffer produced by the hardware. mem2mem codec drivers
> +set this flag on the capture queue for the last buffer when the
> +VIDIOC_QUERYBUF or
> +VIDIOC_DQBUF ioctl is called. Any 
> subsequent
> +call to the VIDIOC_DQBUF ioctl will not 
> block
> +anymore, but return an &EPIPE;.
> +   
> +   
>   V4L2_BUF_FLAG_TIMESTAMP_MASK
>   0xe000
>   Mask for timestamp types below. To test the
> diff --git a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml 
> b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> index 9215627..6502d82 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> @@ -197,7 +197,14 @@ be muted when playing back at a non-standard speed.
>  this command does nothing. This command has two flags:
>  if V4L2_DEC_CMD_STOP_TO_BLACK is set, then the decoder 
> will
>  set the picture to black after it stopped decoding. Otherwise the last image 
> will
> -repeat. If V4L2_DEC_CMD_STOP_IMMEDIATELY is set, then 
> the decoder
> +repeat. mem2mem decoders will stop producing new frames altogether. They 
> will send
> +a V4L2_EVENT_EOS event when the last frame has been 
> decoded
> +and all frames are ready to be dequeued and will set the
> +V4L2_BUF_FLAG_LAST buffer flag on the last buffer of the
> +capture queue to indicate there will be no new buffers produced to dequeue. 
> Once
> +this flag was set, the VIDIOC_DQBUF ioctl
> +will not block anymore, but return an &EPIPE;.
> +If V4L2_DEC_CMD_STOP_IMMEDIATELY is set, then the 
> decoder
>  stops immediately (ignoring the pts value), 
> otherwise it
>  will keep decoding until timestamp >= pts or until the last of the pending 
> data from
>  its internal buffers was decoded.
> diff --git a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml 
> b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> index 0619ca5..3cdb841 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> @@ -129,7 +129,13 @@ this command.
>  encoding will continue until the end of the current Group
>  Of Pictures, otherwise encoding will stop immediately.
>  When the encoder is already stopped, this command does
> -nothing.
> +nothing. mem2mem encoders will send a V4L2_EVENT_EOS 
> event
> +when the last frame has been decoded and all frames are ready to be dequeued 
> and
> +will set the V4L2_BUF_FLAG_LAST buffer flag on the last
> +buffer of the capture queue to indicate there will be no new buffers 
> produced to
> +dequeue. Once this flag was set, the
> +VIDIOC_DQBUF ioctl will not block anymore,
> +but return an &EPIPE;.
> 
> 
>   V4L2_ENC_CMD_PAUSE
> diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml 
> b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> index 3504a7f..6cfc53b 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> @@ -186,6 +186,14 @@ In that case the application should be able to safely 
> reuse the buffer and
>  continue streaming.
>   
>   
> + EPIPE
> + 
> +   VIDIOC_DQBUF returns this on an empty
> +capture queue for mem2mem codecs if a buffer with the
> +V4L2_BUF_FLAG_LAST was already dequ

Re: [PATCH v5 1/5] [media] DocBook media: document mem2mem draining flow

[Hans Verkuil]

Hi Philipp,

I finally got around to reviewing this patch series. Sorry for the delay, but
here are my comments:

On 04/20/2015 10:28 AM, Philipp Zabel wrote:
> Document the interaction between VIDIOC_DECODER_CMD V4L2_DEC_CMD_STOP and
> VIDIOC_ENCODER_CMD V4L2_ENC_CMD_STOP to start the draining, the V4L2_EVENT_EOS
> event signalling all capture buffers are finished and ready to be dequeud,
> the new V4L2_BUF_FLAG_LAST buffer flag indicating the last buffer being 
> dequeued
> from the capture queue, and the poll and VIDIOC_DQBUF ioctl return values once
> the queue is drained.
> 
> Signed-off-by: Philipp Zabel 
> ---
> Changes since v4:
>  - Split out documentation changes into a separate patch
>  - Changed wording following Pawel's suggestions.
> ---
>  Documentation/DocBook/media/v4l/io.xml | 10 ++
>  Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml |  9 -
>  Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml |  8 +++-
>  Documentation/DocBook/media/v4l/vidioc-qbuf.xml|  8 
>  4 files changed, 33 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/DocBook/media/v4l/io.xml 
> b/Documentation/DocBook/media/v4l/io.xml
> index 1c17f80..f561037 100644
> --- a/Documentation/DocBook/media/v4l/io.xml
> +++ b/Documentation/DocBook/media/v4l/io.xml
> @@ -1129,6 +1129,16 @@ in this buffer has not been created by the CPU but by 
> some DMA-capable unit,
>  in which case caches have not been used.
> 
> 
> + V4L2_BUF_FLAG_LAST
> + 0x0010
> + Last buffer produced by the hardware. mem2mem codec drivers
> +set this flag on the capture queue for the last buffer when the
> +VIDIOC_QUERYBUF or
> +VIDIOC_DQBUF ioctl is called. Any 
> subsequent
> +call to the VIDIOC_DQBUF ioctl will not 
> block
> +anymore, but return an &EPIPE;.

As Kamil mentioned in his review, we should allow for bytesused == 0 here.

> +   
> +   
>   V4L2_BUF_FLAG_TIMESTAMP_MASK
>   0xe000
>   Mask for timestamp types below. To test the
> diff --git a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml 
> b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> index 9215627..6502d82 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-decoder-cmd.xml
> @@ -197,7 +197,14 @@ be muted when playing back at a non-standard speed.
>  this command does nothing. This command has two flags:
>  if V4L2_DEC_CMD_STOP_TO_BLACK is set, then the decoder 
> will
>  set the picture to black after it stopped decoding. Otherwise the last image 
> will
> -repeat. If V4L2_DEC_CMD_STOP_IMMEDIATELY is set, then 
> the decoder
> +repeat. mem2mem decoders will stop producing new frames altogether. They 
> will send
> +a V4L2_EVENT_EOS event when the last frame has been 
> decoded
> +and all frames are ready to be dequeued and will set the
> +V4L2_BUF_FLAG_LAST buffer flag on the last buffer of the

Make a note here as well that the last buffer might be an empty buffer.

> +capture queue to indicate there will be no new buffers produced to dequeue. 
> Once
> +this flag was set, the VIDIOC_DQBUF ioctl
> +will not block anymore, but return an &EPIPE;.
> +If V4L2_DEC_CMD_STOP_IMMEDIATELY is set, then the 
> decoder
>  stops immediately (ignoring the pts value), 
> otherwise it
>  will keep decoding until timestamp >= pts or until the last of the pending 
> data from
>  its internal buffers was decoded.
> diff --git a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml 
> b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> index 0619ca5..3cdb841 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-encoder-cmd.xml
> @@ -129,7 +129,13 @@ this command.
>  encoding will continue until the end of the current Group
>  Of Pictures, otherwise encoding will stop immediately.
>  When the encoder is already stopped, this command does
> -nothing.
> +nothing. mem2mem encoders will send a V4L2_EVENT_EOS 
> event
> +when the last frame has been decoded and all frames are ready to be dequeued 
> and
> +will set the V4L2_BUF_FLAG_LAST buffer flag on the last
> +buffer of the capture queue to indicate there will be no new buffers 
> produced to

Ditto.

> +dequeue. Once this flag was set, the
> +VIDIOC_DQBUF ioctl will not block anymore,
> +but return an &EPIPE;.
> 
> 
>   V4L2_ENC_CMD_PAUSE
> diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml 
> b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> index 3504a7f..6cfc53b 100644
> --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> @@ -186,6 +186,14 @@ In that case the application should be able to safely 
> reuse the buffer and
>  continue streaming.
>   
>   
> + EPIPE
> + 
> +   VIDIOC_DQBUF returns this on an empty
> +capture queue for mem2mem co

RE: [PATCH v4 06/10] cec: add HDMI CEC framework

[Kamil Debski]

Hi Hans,

Thank you so much for all today's comments. I will consider them when
preparing the next version.

From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
ow...@vger.kernel.org] On Behalf Of Hans Verkuil
Sent: Monday, April 27, 2015 1:27 PM
> 
> On 04/27/2015 12:25 PM, Hans Verkuil wrote:
> > On 04/23/2015 03:03 PM, Kamil Debski wrote:
> >> From: Hans Verkuil 
> >>
> >> The added HDMI CEC framework provides a generic kernel interface for
> >> HDMI CEC devices.
> >>
> >> Signed-off-by: Hans Verkuil 
> >> [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
> >> [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
> >> [k.deb...@samsung.com: change kthread handling when setting logical
> >> address]
> >> [k.deb...@samsung.com: code cleanup and fixes]
> >> [k.deb...@samsung.com: add missing CEC commands to match spec]
> >> [k.deb...@samsung.com: add RC framework support]
> >> [k.deb...@samsung.com: move and edit documentation]
> >> [k.deb...@samsung.com: add vendor id reporting]
> >> [k.deb...@samsung.com: add possibility to clear assigned logical
> >> addresses]
> >> [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
> >> [k.deb...@samsung.com: reorder of API structs and add reserved
> >> fields]
> >> [k.deb...@samsung.com: fix handling of events and fix 32/64bit
> >> timespec problem]
> >> [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
> >> Signed-off-by: Kamil Debski 
> >> ---
> >>  Documentation/cec.txt |  396 
> >>  drivers/media/Kconfig |6 +
> >>  drivers/media/Makefile|2 +
> >>  drivers/media/cec.c   | 1161
> +
> >>  include/media/cec.h   |  140 ++
> >>  include/uapi/linux/Kbuild |1 +
> >>  include/uapi/linux/cec.h  |  303 
> >>  7 files changed, 2009 insertions(+)
> >>  create mode 100644 Documentation/cec.txt  create mode 100644
> >> drivers/media/cec.c  create mode 100644 include/media/cec.h  create
> >> mode 100644 include/uapi/linux/cec.h
> >>
> >
> >> +  case CEC_S_ADAP_LOG_ADDRS: {
> >> +  struct cec_log_addrs log_addrs;
> >> +
> >> +  if (!(adap->capabilities & CEC_CAP_LOG_ADDRS))
> >> +  return -ENOTTY;
> >> +  if (copy_from_user(&log_addrs, parg, sizeof(log_addrs)))
> >> +  return -EFAULT;
> >> +  err = cec_claim_log_addrs(adap, &log_addrs, true);
> >
> > Currently CEC_S_ADAP_LOG_ADDRS is always blocking, but since we have
> > CEC_EVENT_READY I think it makes sense to just return in non-blocking
> > mode and have cec_claim_log_addrs generate CEC_EVENT_READY when done.
> > Userspace can then call G_ADAP_LOG_ADDRS to discover the result.
> >
> > What do you think?
> >

I am looking into this now. If I see this correctly this involves:
- adding cec_post_event(cla_int->adap, CEC_EVENT_READY); to
cec_config_thread_func
- adding O_NONBLOCK check in CEC_S_ADAP_LOG_ADDRS 
Right?
 
> On a related topic: non-blocking behavior for CEC_RECEIVE is well
> defined, but for CEC_TRA NSMIT it isn't. If reply == 0, then we need a
> way to inform userspace that the transmit finished (with a possible
> non-zero status code). An event would be suitable for that, but we
> would need a way to associate a transmit message with the event.
> 
> One possibility might be to have the CEC framework assign a sequence
> number to a transmit message which is returned by CEC_TRANSMIT and used
> in the event.
> 
> If reply != 0, then I think the received message should be queued up in
> the receive queue, but with a non-zero reply field and with the
> sequence number of the transmit message it is a reply of.

A sequence number is a good solution, I believe. To recap:
- a sequence number should be set by the framework and returned in the
CEC_TRANSMIT ioctl
- a new event should be added CEC_EVENT_TX_DONE and it should be posted on
each transmission
  finish 
- event struct has to include a sequence field as well
Is this ok?

> 
> Regards,
> 
>   Hans

Best wishes,
-- 
Kamil Debski
Samsung R&D Institute Poland

--
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

[PATCH] DocBook/media: add CEC documentation

[Hans Verkuil]

(Kamil, here is the DocBook documentation for the CEC API. Please add this to 
your
patch series when you post the next version of that.)
---

Add the documentation for the public CEC API.

Signed-off-by: Hans Verkuil 
---
 Documentation/DocBook/media/Makefile   |   4 +-
 Documentation/DocBook/media/v4l/biblio.xml |  10 +
 Documentation/DocBook/media/v4l/cec-api.xml|  73 ++
 Documentation/DocBook/media/v4l/cec-func-close.xml |  59 +
 Documentation/DocBook/media/v4l/cec-func-ioctl.xml |  73 ++
 Documentation/DocBook/media/v4l/cec-func-open.xml  |  94 +++
 Documentation/DocBook/media/v4l/cec-func-poll.xml  |  89 +++
 .../DocBook/media/v4l/cec-ioc-g-adap-log-addrs.xml | 275 +
 .../DocBook/media/v4l/cec-ioc-g-adap-phys-addr.xml |  78 ++
 .../DocBook/media/v4l/cec-ioc-g-adap-state.xml |  87 +++
 Documentation/DocBook/media/v4l/cec-ioc-g-caps.xml | 167 +
 .../DocBook/media/v4l/cec-ioc-g-event.xml  | 128 ++
 .../DocBook/media/v4l/cec-ioc-g-vendor-id.xml  |  70 ++
 .../DocBook/media/v4l/cec-ioc-receive.xml  | 176 +
 Documentation/DocBook/media_api.tmpl   |   6 +-
 15 files changed, 1386 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/DocBook/media/v4l/cec-api.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-func-close.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-func-ioctl.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-func-open.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-func-poll.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-adap-log-addrs.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-adap-phys-addr.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-adap-state.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-caps.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-event.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-g-vendor-id.xml
 create mode 100644 Documentation/DocBook/media/v4l/cec-ioc-receive.xml

diff --git a/Documentation/DocBook/media/Makefile 
b/Documentation/DocBook/media/Makefile
index 8bf7c61..9650332 100644
--- a/Documentation/DocBook/media/Makefile
+++ b/Documentation/DocBook/media/Makefile
@@ -64,6 +64,7 @@ IOCTLS = \
$(shell perl -ne 'print "$$1 " if /\#define\s+([A-Z][^\s]+)\s+_IO/' 
$(srctree)/include/uapi/linux/dvb/net.h) \
$(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' 
$(srctree)/include/uapi/linux/dvb/video.h) \
$(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' 
$(srctree)/include/uapi/linux/media.h) \
+   $(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' 
$(srctree)/include/uapi/linux/cec.h) \
$(shell perl -ne 'print "$$1 " if /\#define\s+([^\s]+)\s+_IO/' 
$(srctree)/include/uapi/linux/v4l2-subdev.h) \
VIDIOC_SUBDEV_G_FRAME_INTERVAL \
VIDIOC_SUBDEV_S_FRAME_INTERVAL \
@@ -98,6 +99,7 @@ STRUCTS = \
$(shell perl -ne 'print "$$1 " if (/^struct\s+([A-Z][^\s]+)\s+/)' 
$(srctree)/include/uapi/linux/dvb/net.h) \
$(shell perl -ne 'print "$$1 " if (/^struct\s+([^\s]+)\s+/)' 
$(srctree)/include/uapi/linux/dvb/video.h) \
$(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' 
$(srctree)/include/uapi/linux/media.h) \
+   $(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' 
$(srctree)/include/uapi/linux/cec.h) \
$(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' 
$(srctree)/include/uapi/linux/v4l2-subdev.h) \
$(shell perl -ne 'print "$$1 " if /^struct\s+([^\s]+)\s+/' 
$(srctree)/include/uapi/linux/v4l2-mediabus.h)
 
@@ -300,7 +302,7 @@ $(MEDIA_OBJ_DIR)/media-entities.tmpl: 
$(MEDIA_OBJ_DIR)/v4l2.xml
@(  \
for ident in $(IOCTLS) ; do \
  entity=`echo $$ident | tr _ -` ;  \
- id=`grep "$$ident" $(MEDIA_OBJ_DIR)/vidioc-*.xml 
$(MEDIA_OBJ_DIR)/media-ioc-*.xml | sed -r s,"^.*/(.*).xml.*","\1",` ; \
+ id=`grep "$$ident" $(MEDIA_OBJ_DIR)/vidioc-*.xml 
$(MEDIA_OBJ_DIR)/media-ioc-*.xml $(MEDIA_OBJ_DIR)/cec-ioc-*.xml | sed -r 
s,"^.*/(.*).xml.*","\1",` ; \
  echo "$$ident\">" \
  >>$@ ;\
diff --git a/Documentation/DocBook/media/v4l/biblio.xml 
b/Documentation/DocBook/media/v4l/biblio.xml
index fdee6b3..bed940b 100644
--- a/Documentation/DocBook/media/v4l/biblio.xml
+++ b/Documentation/DocBook/media/v4l/biblio.xml
@@ -324,6 +324,16 @@ in the frequency range from 87,5 to 108,0 MHz
   Specification Version 1.4a
 
 
+
+  HDMI2
+  
+   HDMI Licensing LLC
+(http://www.hdmi.org";>http://www.hdmi.org)
+  
+  High-Definition Multimedia Interface
+  Specification Version 2.0
+

Re: [PATCH 02/12] dvbsky: use si2168 config option ts_clock_gapped

[Olli Salonen]

Thank you Steven,

That's helpful to know. I've been bumping into some issues with
another Si2168-based device and certain DVB-C streams. Will try to see
if this could help in that case...

Cheers,
-olli

On 24 April 2015 at 15:16, Steven Toth  wrote:
>> I've also seen that the Hauppauge HVR-2205 Windows driver enables this
>> option, but it seems to me that that board works ok also without this.
>
> Olli, I found out why this is, I thought you'd appreciate the comment
>
> Apparently the issue only occurs with DVB streams faster than
> approximately 50Mbps, which standard DVB-T/T2, ATSC and QAM-B never
> are.
>
> The issue apparently, is with some QAM-A (DVB-C) streams in
> Europe. This explains why I've never seen it. That's being said, I
> do plan to add the gapped clock patch to the SAA7164 shortly - for
> safety.
>
> --
> Steven Toth - Kernel Labs
> http://www.kernellabs.com
> --
> 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
--
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 v4 06/10] cec: add HDMI CEC framework

[Hans Verkuil]

On 04/27/2015 11:22 AM, Hans Verkuil wrote:
> Hi Kamil,
> 
> Sorry for all the replies, but I'm writing the DocBook documentation, so
> whenever I find something missing I'll just reply to this patch.
>> +/* The CEC version */
> 
> Add support for version 1.3a here:
> 
> #define CEC_VERSION_1_3A  4
> 
>> +#define CEC_VERSION_1_4B5

Hmm, reading up on this I see that '5' is used for HDMI 1.4, 1.4a or 1.4b.

I think it should be renamed to VERSION_1_4 (i.e. drop the 'B').

Regards,

Hans

>> +#define CEC_VERSION_2_0 6
> 
> Regards,
> 
>   Hans
> --
> 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
> 

--
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 v4 06/10] cec: add HDMI CEC framework

[Hans Verkuil]

On 04/27/2015 12:25 PM, Hans Verkuil wrote:
> On 04/23/2015 03:03 PM, Kamil Debski wrote:
>> From: Hans Verkuil 
>>
>> The added HDMI CEC framework provides a generic kernel interface for
>> HDMI CEC devices.
>>
>> Signed-off-by: Hans Verkuil 
>> [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
>> [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
>> [k.deb...@samsung.com: change kthread handling when setting logical
>> address]
>> [k.deb...@samsung.com: code cleanup and fixes]
>> [k.deb...@samsung.com: add missing CEC commands to match spec]
>> [k.deb...@samsung.com: add RC framework support]
>> [k.deb...@samsung.com: move and edit documentation]
>> [k.deb...@samsung.com: add vendor id reporting]
>> [k.deb...@samsung.com: add possibility to clear assigned logical
>> addresses]
>> [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
>> [k.deb...@samsung.com: reorder of API structs and add reserved fields]
>> [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
>> problem]
>> [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
>> Signed-off-by: Kamil Debski 
>> ---
>>  Documentation/cec.txt |  396 
>>  drivers/media/Kconfig |6 +
>>  drivers/media/Makefile|2 +
>>  drivers/media/cec.c   | 1161 
>> +
>>  include/media/cec.h   |  140 ++
>>  include/uapi/linux/Kbuild |1 +
>>  include/uapi/linux/cec.h  |  303 
>>  7 files changed, 2009 insertions(+)
>>  create mode 100644 Documentation/cec.txt
>>  create mode 100644 drivers/media/cec.c
>>  create mode 100644 include/media/cec.h
>>  create mode 100644 include/uapi/linux/cec.h
>>
> 
>> +case CEC_S_ADAP_LOG_ADDRS: {
>> +struct cec_log_addrs log_addrs;
>> +
>> +if (!(adap->capabilities & CEC_CAP_LOG_ADDRS))
>> +return -ENOTTY;
>> +if (copy_from_user(&log_addrs, parg, sizeof(log_addrs)))
>> +return -EFAULT;
>> +err = cec_claim_log_addrs(adap, &log_addrs, true);
> 
> Currently CEC_S_ADAP_LOG_ADDRS is always blocking, but since we have 
> CEC_EVENT_READY
> I think it makes sense to just return in non-blocking mode and have 
> cec_claim_log_addrs
> generate CEC_EVENT_READY when done. Userspace can then call G_ADAP_LOG_ADDRS 
> to discover
> the result.
> 
> What do you think?
> 

On a related topic: non-blocking behavior for CEC_RECEIVE is well defined, but 
for
CEC_TRANSMIT it isn't. If reply == 0, then we need a way to inform userspace 
that
the transmit finished (with a possible non-zero status code). An event would be
suitable for that, but we would need a way to associate a transmit message with
the event.

One possibility might be to have the CEC framework assign a sequence number to
a transmit message which is returned by CEC_TRANSMIT and used in the event.

If reply != 0, then I think the received message should be queued up in the
receive queue, but with a non-zero reply field and with the sequence number of 
the
transmit message it is a reply of.

Regards,

Hans
--
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 v4 06/10] cec: add HDMI CEC framework

[Hans Verkuil]

On 04/23/2015 03:03 PM, Kamil Debski wrote:
> From: Hans Verkuil 
> 
> The added HDMI CEC framework provides a generic kernel interface for
> HDMI CEC devices.
> 
> Signed-off-by: Hans Verkuil 
> [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
> [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
> [k.deb...@samsung.com: change kthread handling when setting logical
> address]
> [k.deb...@samsung.com: code cleanup and fixes]
> [k.deb...@samsung.com: add missing CEC commands to match spec]
> [k.deb...@samsung.com: add RC framework support]
> [k.deb...@samsung.com: move and edit documentation]
> [k.deb...@samsung.com: add vendor id reporting]
> [k.deb...@samsung.com: add possibility to clear assigned logical
> addresses]
> [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
> [k.deb...@samsung.com: reorder of API structs and add reserved fields]
> [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
> problem]
> [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
> Signed-off-by: Kamil Debski 
> ---
>  Documentation/cec.txt |  396 
>  drivers/media/Kconfig |6 +
>  drivers/media/Makefile|2 +
>  drivers/media/cec.c   | 1161 
> +
>  include/media/cec.h   |  140 ++
>  include/uapi/linux/Kbuild |1 +
>  include/uapi/linux/cec.h  |  303 
>  7 files changed, 2009 insertions(+)
>  create mode 100644 Documentation/cec.txt
>  create mode 100644 drivers/media/cec.c
>  create mode 100644 include/media/cec.h
>  create mode 100644 include/uapi/linux/cec.h
> 

> + case CEC_S_ADAP_LOG_ADDRS: {
> + struct cec_log_addrs log_addrs;
> +
> + if (!(adap->capabilities & CEC_CAP_LOG_ADDRS))
> + return -ENOTTY;
> + if (copy_from_user(&log_addrs, parg, sizeof(log_addrs)))
> + return -EFAULT;
> + err = cec_claim_log_addrs(adap, &log_addrs, true);

Currently CEC_S_ADAP_LOG_ADDRS is always blocking, but since we have 
CEC_EVENT_READY
I think it makes sense to just return in non-blocking mode and have 
cec_claim_log_addrs
generate CEC_EVENT_READY when done. Userspace can then call G_ADAP_LOG_ADDRS to 
discover
the result.

What do you think?

Regards,

Hans

> + if (err)
> + return err;
> +
> + if (copy_to_user(parg, &log_addrs, sizeof(log_addrs)))
> + return -EFAULT;
> + break;
> + }

--
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 v4 06/10] cec: add HDMI CEC framework

[Hans Verkuil]

On 04/23/2015 03:03 PM, Kamil Debski wrote:
> From: Hans Verkuil 
> 
> The added HDMI CEC framework provides a generic kernel interface for
> HDMI CEC devices.
> 
> Signed-off-by: Hans Verkuil 
> [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
> [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
> [k.deb...@samsung.com: change kthread handling when setting logical
> address]
> [k.deb...@samsung.com: code cleanup and fixes]
> [k.deb...@samsung.com: add missing CEC commands to match spec]
> [k.deb...@samsung.com: add RC framework support]
> [k.deb...@samsung.com: move and edit documentation]
> [k.deb...@samsung.com: add vendor id reporting]
> [k.deb...@samsung.com: add possibility to clear assigned logical
> addresses]
> [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
> [k.deb...@samsung.com: reorder of API structs and add reserved fields]
> [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
> problem]
> [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
> Signed-off-by: Kamil Debski 
> ---
>  Documentation/cec.txt |  396 
>  drivers/media/Kconfig |6 +
>  drivers/media/Makefile|2 +
>  drivers/media/cec.c   | 1161 
> +
>  include/media/cec.h   |  140 ++
>  include/uapi/linux/Kbuild |1 +
>  include/uapi/linux/cec.h  |  303 
>  7 files changed, 2009 insertions(+)
>  create mode 100644 Documentation/cec.txt
>  create mode 100644 drivers/media/cec.c
>  create mode 100644 include/media/cec.h
>  create mode 100644 include/uapi/linux/cec.h
> 
> diff --git a/Documentation/cec.txt b/Documentation/cec.txt
> new file mode 100644
> index 000..2b6c08a
> --- /dev/null
> +++ b/Documentation/cec.txt
> @@ -0,0 +1,396 @@
> +- CEC_G_ADAP_LOG_ADDRS and CEC_S_ADAP_LOG_ADDRS
> +
> +These ioctl are used to configure the logical addresses of the CEC adapter.
> +
> +#define CEC_G_ADAP_LOG_ADDRS _IOR('a', 3, struct cec_log_addrs)
> +#define CEC_S_ADAP_LOG_ADDRS _IOWR('a', 4, struct cec_log_addrs)
> +
> +The struct cec_log_addrs is following:
> +
> +struct cec_log_addrs {
> + __u8 cec_version;
> + __u8 num_log_addrs;
> + __u8 primary_device_type[CEC_MAX_LOG_ADDRS];
> + __u8 log_addr_type[CEC_MAX_LOG_ADDRS];
> + __u8 log_addr[CEC_MAX_LOG_ADDRS];
> +
> + /* CEC 2.0 */
> + __u8 all_device_types;
> + __u8 features[CEC_MAX_LOG_ADDRS][12];
> +
> + __u8 reserved[9];
> +};
> +
> +The cec_version determines which CEC version should be used.
> +
> +/* The CEC version */
> +#define CEC_VERSION_1_4B 5
> +#define CEC_VERSION_2_0  6
> +
> +It will try to claim num_log_addrs devices. The log_addr_type array has
> +the logical address type that needs to be claimed for that device, and
> +the log_addr array will receive the actual logical address that was
> +claimed for that device or 0xff if no address could be claimed.
> +
> +The primary_device_type contains the primary device for each logical
> +address.
> +
> +For CEC 2.0 devices fill in the all_device_types parameter to use with the
> +Report Features command, and fill in the 'features' which contains the
> +remaining parameters (RC Profile and Device Features) to use in Report
> +Features.
> +
> +An error is returned if the adapter is disabled or if there
> +is no physical address assigned or if the cec_version is unknown.
> +
> +If no logical address of one or more of the given types could be claimed,
> +then log_addr will be set to CEC_LOG_ADDR_INVALID.

This does not appear to be the case looking at the cec_config_log_addrs 
function.
I don't see it being set to INVALID if it couldn't be claimed. I think that is
missing in the cec_config_log_addrs function.

Regards,

Hans

--
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 v4 06/10] cec: add HDMI CEC framework

[Hans Verkuil]

On 04/27/2015 11:22 AM, Hans Verkuil wrote:
> Hi Kamil,
> 
> Sorry for all the replies, but I'm writing the DocBook documentation, so
> whenever I find something missing I'll just reply to this patch.
>> +/* The CEC version */
> 
> Add support for version 1.3a here:
> 
> #define CEC_VERSION_1_3A  4

To clarify: the core CEC framework will not support 1.3A, but of course
other devices can report it, so we do need the define.

Regards,

Hans

> 
>> +#define CEC_VERSION_1_4B5
>> +#define CEC_VERSION_2_0 6
> 
> Regards,
> 
>   Hans
> --
> 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
> 

--
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 v4 06/10] cec: add HDMI CEC framework

[Hans Verkuil]

Hi Kamil,

Sorry for all the replies, but I'm writing the DocBook documentation, so
whenever I find something missing I'll just reply to this patch.
> +/* The CEC version */

Add support for version 1.3a here:

#define CEC_VERSION_1_3A4

> +#define CEC_VERSION_1_4B 5
> +#define CEC_VERSION_2_0  6

Regards,

Hans
--
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 v4 06/10] cec: add HDMI CEC framework

[Hans Verkuil]

Hi Kamil,

I've added some missing HDMI 2.0 commands:

On 04/23/2015 03:03 PM, Kamil Debski wrote:
> From: Hans Verkuil 
> 
> The added HDMI CEC framework provides a generic kernel interface for
> HDMI CEC devices.
> 
> Signed-off-by: Hans Verkuil 
> [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
> [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
> [k.deb...@samsung.com: change kthread handling when setting logical
> address]
> [k.deb...@samsung.com: code cleanup and fixes]
> [k.deb...@samsung.com: add missing CEC commands to match spec]
> [k.deb...@samsung.com: add RC framework support]
> [k.deb...@samsung.com: move and edit documentation]
> [k.deb...@samsung.com: add vendor id reporting]
> [k.deb...@samsung.com: add possibility to clear assigned logical
> addresses]
> [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
> [k.deb...@samsung.com: reorder of API structs and add reserved fields]
> [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
> problem]
> [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
> Signed-off-by: Kamil Debski 
> ---
>  Documentation/cec.txt |  396 
>  drivers/media/Kconfig |6 +
>  drivers/media/Makefile|2 +
>  drivers/media/cec.c   | 1161 
> +
>  include/media/cec.h   |  140 ++
>  include/uapi/linux/Kbuild |1 +
>  include/uapi/linux/cec.h  |  303 
>  7 files changed, 2009 insertions(+)
>  create mode 100644 Documentation/cec.txt
>  create mode 100644 drivers/media/cec.c
>  create mode 100644 include/media/cec.h
>  create mode 100644 include/uapi/linux/cec.h
> 
> diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
> new file mode 100644
> index 000..bb6d66c
> --- /dev/null
> +++ b/include/uapi/linux/cec.h
> @@ -0,0 +1,303 @@
> +#ifndef _CEC_H
> +#define _CEC_H
> +
> +#include 
> +
> +struct cec_time {
> + __u64 sec;
> + __u64 nsec;
> +};
> +
> +struct cec_msg {
> + struct cec_time ts;
> + __u32 len;
> + __u32 status;
> + __u32 timeout;
> + /* timeout (in ms) is used to timeout CEC_RECEIVE.
> +Set to 0 if you want to wait forever. */
> + __u8  msg[16];
> + __u8  reply;
> + /* If non-zero, then wait for a reply with this opcode.
> +If there was an error when sending the msg or FeatureAbort
> +was returned, then reply is set to 0.
> +If reply is non-zero upon return, then len/msg are set to
> +the received message.
> +If reply is zero upon return and status has the
> +CEC_TX_STATUS_FEATURE_ABORT bit set, then len/msg are set to the
> +received feature abort message.
> +If reply is zero upon return and status has the
> +CEC_TX_STATUS_REPLY_TIMEOUT
> +bit set, then no reply was seen at all.
> +This field is ignored with CEC_RECEIVE.
> +If reply is non-zero for CEC_TRANSMIT and the message is a broadcast,
> +then -EINVAL is returned.
> +if reply is non-zero, then timeout is set to 1000 (the required
> +maximum response time).
> +  */
> + __u8 reserved[31];
> +};
> +
> +static inline __u8 cec_msg_initiator(const struct cec_msg *msg)
> +{
> + return msg->msg[0] >> 4;
> +}
> +
> +static inline __u8 cec_msg_destination(const struct cec_msg *msg)
> +{
> + return msg->msg[0] & 0xf;
> +}
> +
> +static inline bool cec_msg_is_broadcast(const struct cec_msg *msg)
> +{
> + return (msg->msg[0] & 0xf) == 0xf;
> +}
> +
> +/* cec status field */
> +#define CEC_TX_STATUS_OK(0)
> +#define CEC_TX_STATUS_ARB_LOST  (1 << 0)
> +#define CEC_TX_STATUS_RETRY_TIMEOUT (1 << 1)
> +#define CEC_TX_STATUS_FEATURE_ABORT (1 << 2)
> +#define CEC_TX_STATUS_REPLY_TIMEOUT (1 << 3)
> +#define CEC_RX_STATUS_READY (0)
> +
> +#define CEC_LOG_ADDR_INVALID 0xff
> +
> +/* The maximum number of logical addresses one device can be assigned to.
> + * The CEC 2.0 spec allows for only 2 logical addresses at the moment. The
> + * Analog Devices CEC hardware supports 3. So let's go wild and go for 4. */
> +#define CEC_MAX_LOG_ADDRS 4
> +
> +/* The "Primary Device Type" */
> +#define CEC_PRIM_DEVTYPE_TV  0
> +#define CEC_PRIM_DEVTYPE_RECORD  1
> +#define CEC_PRIM_DEVTYPE_TUNER   3
> +#define CEC_PRIM_DEVTYPE_PLAYBACK4
> +#define CEC_PRIM_DEVTYPE_AUDIOSYSTEM 5
> +#define CEC_PRIM_DEVTYPE_SWITCH  6
> +#define CEC_PRIM_DEVTYPE_VIDEOPROC   7
> +
> +/* The "All Device Types" flags (CEC 2.0) */
> +#define CEC_FL_ALL_DEVTYPE_TV(1 << 7)
> +#define CEC_FL_ALL_DEVTYPE_RECORD(1 << 6)
> +#define CEC_FL_ALL_DEVTYPE_TUNER (1 << 5)
> +#define CEC_FL_ALL_DEVTYPE_PLAYBACK  (1 << 4)
> +#define CEC_FL_ALL_DEVTYPE_AUDIOSYSTEM   (1 << 3)
> +#define CEC_FL_ALL_DEVTYPE_SWITCH(1 << 2)
> +/* And if you wondering what happened to VIDEOPROC devices: those should
> + * 

Re: [PATCH v4 06/10] cec: add HDMI CEC framework

[Hans Verkuil]

Hi Lars,

My thanks as well for your comments.

I'd like to add some background information as well as to why we move
the core CEC support into the kernel: the main reason for doing this
is to support the HEAC part of the CEC protocol. Specifically the ARC
support and handling the hotplug detect CEC/HEAC messages. This has to
be handled in the kernel and cannot be left to userspace. While the
current framework does not yet handle these messages support for this
will appear, probably later this year since I will have to work on ARC.

Out of curiosity: have you ever seen CEC adapters that implement the
ethernet part of HEAC? My understanding is that nobody uses that part
since wifi is the standard these days. But perhaps you know of examples
where it was actually implemented.

Regards,

Hans

On 04/27/2015 10:09 AM, Kamil Debski wrote:
> Hi Lars, 
> 
> Thank you for your comments.
> 
> From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
> ow...@vger.kernel.org] On Behalf Of Lars Op den Kamp
> Sent: Friday, April 24, 2015 12:04 PM
>  
>> Hi Kamil, Hans,
>>
>> I'm the main developer of libCEC
>> (https://github.com/Pulse-Eight/libcec). Sorry for the late time to
>> jump
>> in here, but I wasn't signed up to this mailing list and someone
>> pointed
>> me to this discussion.
>>
>> Unfortunately this approach will not work with half the TVs that are
>> out
>> there. I'll explain why:
>>
>> * because of how some (common) brands implemented CEC in their TVs,
>> this
>> implementation will not work, as the TV will just reject it. In libCEC,
>> we've created work arounds for brands like this. Without these work
>> arounds, your in-kernel implementation will be very vendor specific.
>> e.g. this implementation will work for Samsung's TVs, but not for the
>> TVs made by another big TV brand. All commands, including CEC_OP_ABORT,
>> should be passed to userspace to make it work with all brands.
>>
>> * it should be made possible to not have the kernel send any CEC
>> message, try to process any received CEC message, or ack to any logical
>> address at all, to allow libraries like libCEC to fully handle all CEC
>> traffic. Some brands only enable routing of some CEC keys when a
>> specific device type is used. libCEC will allocate a logical address of
>> the correct type for the brand that's used. If another address is first
>> allocated by the kernel, and the TV communicates with it to find out
>> it's name and things like that, and libCEC allocates another address a
>> bit later when initialised, then you'll end up with multiple entries in
>> the device list on the TV, and only one of them will work.
> 
> Adding a special mode in the CEC framework that disables parsing and
> processing seems like a good idea for me. This way libCEC could be
> completely
> in charge of how the communication is handled. 
> 
> I discussed this with Hans and he is for this solution. This way there would
> be two modes:
> - One with handling of CEC messages enabled in the kernel, in idea behind
>   this is to have processing adhere to the CEC spec as closely as possible.
>   It should work with equipment that also follows the spec and has little
>   vendor specific quirks.
> - Second, the passthrough mode, in which the handling of CEC messages would
>   be left to userspace application. Kernel would not be sending or
>   receiving messages, unless specifically told to do so. Below you mentioned
>   that allocating logical addresses and sending ACKs could be done in
> kernel.
> 
>   The way I see it is following: If allocation of a logical address is made
>   then ACKs will be handled by the framework. If no allocation is made then
>   the userspace can still send and receive messages. However no filtering is
>   done based on the logical address - all received messages are sent to the
>   userspace.
> 
>>
>> * CEC is *very* vendor specific. The main reason is, in my opinion, the
>> use of the word "should" instead of "shall" in the spec. It's addressed
>> in the new version, but it'll take years before all the non 2.x devices
>> are gone. What works for vendor A will simply not work for vendor B.
>> libCEC aims to address this, in a library that can be used on all major
>> platforms and by all major programming languages. You could duplicate
>> the work done there in the kernel to make make the implementation work
>> with all brands, but I think that this does simply not belong in the
>> kernel when it can be handled in userspace perfectly.
> 
> CEC being very vendor specific is a huge problem. I agree with you that
> there is no need to duplicate the effort to mitigate all the vendor quirks.
> Especially that a working implementation (libCEC) is already done.
> 
>> So I suggest that you limit the in-kernel implementation to handling
>> raw
>> traffic only, to have it do this (and nothing more):
>> * allocate one or more logical addresses, and ack CEC traffic sent to
>> those logical addresses
>> * receive CEC traffic and f

RE: [PATCH v4 06/10] cec: add HDMI CEC framework

[Kamil Debski]

Hi Lars, 

Thank you for your comments.

From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
ow...@vger.kernel.org] On Behalf Of Lars Op den Kamp
Sent: Friday, April 24, 2015 12:04 PM
 
> Hi Kamil, Hans,
> 
> I'm the main developer of libCEC
> (https://github.com/Pulse-Eight/libcec). Sorry for the late time to
> jump
> in here, but I wasn't signed up to this mailing list and someone
> pointed
> me to this discussion.
> 
> Unfortunately this approach will not work with half the TVs that are
> out
> there. I'll explain why:
> 
> * because of how some (common) brands implemented CEC in their TVs,
> this
> implementation will not work, as the TV will just reject it. In libCEC,
> we've created work arounds for brands like this. Without these work
> arounds, your in-kernel implementation will be very vendor specific.
> e.g. this implementation will work for Samsung's TVs, but not for the
> TVs made by another big TV brand. All commands, including CEC_OP_ABORT,
> should be passed to userspace to make it work with all brands.
>
> * it should be made possible to not have the kernel send any CEC
> message, try to process any received CEC message, or ack to any logical
> address at all, to allow libraries like libCEC to fully handle all CEC
> traffic. Some brands only enable routing of some CEC keys when a
> specific device type is used. libCEC will allocate a logical address of
> the correct type for the brand that's used. If another address is first
> allocated by the kernel, and the TV communicates with it to find out
> it's name and things like that, and libCEC allocates another address a
> bit later when initialised, then you'll end up with multiple entries in
> the device list on the TV, and only one of them will work.

Adding a special mode in the CEC framework that disables parsing and
processing seems like a good idea for me. This way libCEC could be
completely
in charge of how the communication is handled. 

I discussed this with Hans and he is for this solution. This way there would
be two modes:
- One with handling of CEC messages enabled in the kernel, in idea behind
  this is to have processing adhere to the CEC spec as closely as possible.
  It should work with equipment that also follows the spec and has little
  vendor specific quirks.
- Second, the passthrough mode, in which the handling of CEC messages would
  be left to userspace application. Kernel would not be sending or
  receiving messages, unless specifically told to do so. Below you mentioned
  that allocating logical addresses and sending ACKs could be done in
kernel.

  The way I see it is following: If allocation of a logical address is made
  then ACKs will be handled by the framework. If no allocation is made then
  the userspace can still send and receive messages. However no filtering is
  done based on the logical address - all received messages are sent to the
  userspace.

> 
> * CEC is *very* vendor specific. The main reason is, in my opinion, the
> use of the word "should" instead of "shall" in the spec. It's addressed
> in the new version, but it'll take years before all the non 2.x devices
> are gone. What works for vendor A will simply not work for vendor B.
> libCEC aims to address this, in a library that can be used on all major
> platforms and by all major programming languages. You could duplicate
> the work done there in the kernel to make make the implementation work
> with all brands, but I think that this does simply not belong in the
> kernel when it can be handled in userspace perfectly.

CEC being very vendor specific is a huge problem. I agree with you that
there is no need to duplicate the effort to mitigate all the vendor quirks.
Especially that a working implementation (libCEC) is already done.

> So I suggest that you limit the in-kernel implementation to handling
> raw
> traffic only, to have it do this (and nothing more):
> * allocate one or more logical addresses, and ack CEC traffic sent to
> those logical addresses
> * receive CEC traffic and forward it to userspace (traffic sent to all
> addresses is preferred, not just traffic sent to the logical address
> used by the device running this code)
> * transmit CEC traffic initiated by userspace

As mentioned above, I propose a "passthorugh" mode in which handling of
CEC messages by the kernel CEC framework will be very limited. I think
that the three functions listed above should be enough.

Any comments on this solution?

> 
> thanks,
> 
> Lars Op den Kamp

Best wishes,
-- 
Kamil Debski
Samsung R&D Institute Poland

> 
> 
> On 23-04-15 15:03, Kamil Debski wrote:
> > From: Hans Verkuil 
> >
> > The added HDMI CEC framework provides a generic kernel interface for
> > HDMI CEC devices.
> >
> > Signed-off-by: Hans Verkuil 
> > [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
> > [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
> > [k.deb...@samsung.com: change kthread handling when setting logical
> > address]
> > [k

Re: [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question

[Hans Verkuil]

Added the y2038 mailinglist since I would like to get their input for
this API.

Y2038 experts, can you take a look at my comments in the code below?

Thanks!

On 04/23/2015 03:03 PM, Kamil Debski wrote:
> From: Hans Verkuil 
> 
> The added HDMI CEC framework provides a generic kernel interface for
> HDMI CEC devices.
> 
> Signed-off-by: Hans Verkuil 
> [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
> [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
> [k.deb...@samsung.com: change kthread handling when setting logical
> address]
> [k.deb...@samsung.com: code cleanup and fixes]
> [k.deb...@samsung.com: add missing CEC commands to match spec]
> [k.deb...@samsung.com: add RC framework support]
> [k.deb...@samsung.com: move and edit documentation]
> [k.deb...@samsung.com: add vendor id reporting]
> [k.deb...@samsung.com: add possibility to clear assigned logical
> addresses]
> [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
> [k.deb...@samsung.com: reorder of API structs and add reserved fields]
> [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
> problem]
> [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
> Signed-off-by: Kamil Debski 
> ---
>  Documentation/cec.txt |  396 
>  drivers/media/Kconfig |6 +
>  drivers/media/Makefile|2 +
>  drivers/media/cec.c   | 1161 
> +
>  include/media/cec.h   |  140 ++
>  include/uapi/linux/Kbuild |1 +
>  include/uapi/linux/cec.h  |  303 
>  7 files changed, 2009 insertions(+)
>  create mode 100644 Documentation/cec.txt
>  create mode 100644 drivers/media/cec.c
>  create mode 100644 include/media/cec.h
>  create mode 100644 include/uapi/linux/cec.h
> 



> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index 4842a98..5854cfd 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -81,6 +81,7 @@ header-y += capi.h
>  header-y += cciss_defs.h
>  header-y += cciss_ioctl.h
>  header-y += cdrom.h
> +header-y += cec.h
>  header-y += cgroupstats.h
>  header-y += chio.h
>  header-y += cm4000_cs.h
> diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
> new file mode 100644
> index 000..bb6d66c
> --- /dev/null
> +++ b/include/uapi/linux/cec.h
> @@ -0,0 +1,303 @@
> +#ifndef _CEC_H
> +#define _CEC_H
> +
> +#include 
> +
> +struct cec_time {
> + __u64 sec;
> + __u64 nsec;
> +};

I don't like having to introduce a new struct for time here. Basically we are
only doing this because there is still no public struct timespec64.

When will that struct become available for use in a public API? If it is 4.2,
then we can start using it. If it will take longer, then what alternative can
we use to prevent having to change the API later?

One alternative might be to drop it for now and just reserve space in the
structs to add it later.

Input from the y2038 experts will be welcome!

Regards,

Hans

> +
> +struct cec_msg {
> + struct cec_time ts;
> + __u32 len;
> + __u32 status;
> + __u32 timeout;
> + /* timeout (in ms) is used to timeout CEC_RECEIVE.
> +Set to 0 if you want to wait forever. */
> + __u8  msg[16];
> + __u8  reply;
> + /* If non-zero, then wait for a reply with this opcode.
> +If there was an error when sending the msg or FeatureAbort
> +was returned, then reply is set to 0.
> +If reply is non-zero upon return, then len/msg are set to
> +the received message.
> +If reply is zero upon return and status has the
> +CEC_TX_STATUS_FEATURE_ABORT bit set, then len/msg are set to the
> +received feature abort message.
> +If reply is zero upon return and status has the
> +CEC_TX_STATUS_REPLY_TIMEOUT
> +bit set, then no reply was seen at all.
> +This field is ignored with CEC_RECEIVE.
> +If reply is non-zero for CEC_TRANSMIT and the message is a broadcast,
> +then -EINVAL is returned.
> +if reply is non-zero, then timeout is set to 1000 (the required
> +maximum response time).
> +  */
> + __u8 reserved[31];
> +};
> +
> +static inline __u8 cec_msg_initiator(const struct cec_msg *msg)
> +{
> + return msg->msg[0] >> 4;
> +}
> +
> +static inline __u8 cec_msg_destination(const struct cec_msg *msg)
> +{
> + return msg->msg[0] & 0xf;
> +}
> +
> +static inline bool cec_msg_is_broadcast(const struct cec_msg *msg)
> +{
> + return (msg->msg[0] & 0xf) == 0xf;
> +}
> +
> +/* cec status field */
> +#define CEC_TX_STATUS_OK(0)
> +#define CEC_TX_STATUS_ARB_LOST  (1 << 0)
> +#define CEC_TX_STATUS_RETRY_TIMEOUT (1 << 1)
> +#define CEC_TX_STATUS_FEATURE_ABORT (1 << 2)
> +#define CEC_TX_STATUS_REPLY_TIMEOUT (1 << 3)
> +#define CEC_RX_STATUS_READY (0)
> +
> +#define CEC_LOG_ADDR_INVALID 0xff
> +
> +/* The maximum number of logical addresses 

[PATCH 5/5] radio-bcm2048: fix compiler warning

[Hans Verkuil]

From: Hans Verkuil 

radio-bcm2048.c: In function 'bcm2048_i2c_driver_probe':
radio-bcm2048.c:2597:11: warning: variable 'skip_release' set but not used 
[-Wunused-but-set-variable]
  int err, skip_release = 0;
 ^

Signed-off-by: Hans Verkuil 
---
 drivers/staging/media/bcm2048/radio-bcm2048.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/media/bcm2048/radio-bcm2048.c 
b/drivers/staging/media/bcm2048/radio-bcm2048.c
index bd50fb2..dbed3a2 100644
--- a/drivers/staging/media/bcm2048/radio-bcm2048.c
+++ b/drivers/staging/media/bcm2048/radio-bcm2048.c
@@ -2594,7 +2594,7 @@ static int bcm2048_i2c_driver_probe(struct i2c_client 
*client,
const struct i2c_device_id *id)
 {
struct bcm2048_device *bdev;
-   int err, skip_release = 0;
+   int err;
 
bdev = kzalloc(sizeof(*bdev), GFP_KERNEL);
if (!bdev) {
@@ -2647,7 +2647,6 @@ free_sysfs:
bcm2048_sysfs_unregister_properties(bdev, ARRAY_SIZE(attrs));
 free_registration:
video_unregister_device(&bdev->videodev);
-   skip_release = 1;
 free_irq:
if (client->irq)
free_irq(client->irq, bdev);
-- 
2.1.4

--
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

[PATCH 3/5] cx24123/mb86a20s/s921: fix compiler warnings

[Hans Verkuil]

From: Hans Verkuil 

In file included from drivers/media/common/b2c2/flexcop-fe-tuner.c:13:0:
drivers/media/dvb-frontends/cx24123.h:54:2: warning: 
'cx24123_get_tuner_i2c_adapter' defined but not used [-Wunused-function]
  cx24123_get_tuner_i2c_adapter(struct dvb_frontend *fe)
  ^
In file included from drivers/media/usb/em28xx/em28xx-dvb.c:46:0:
drivers/media/dvb-frontends/s921.h:40:2: warning: 's921_get_tuner_i2c_adapter' 
defined but not used [-Wunused-function]
  s921_get_tuner_i2c_adapter(struct dvb_frontend *fe)
  ^
In file included from drivers/media/usb/em28xx/em28xx-dvb.c:55:0:
drivers/media/dvb-frontends/mb86a20s.h:49:2: warning: 
'mb86a20s_get_tuner_i2c_adapter' defined but not used [-Wunused-function]
  mb86a20s_get_tuner_i2c_adapter(struct dvb_frontend *fe)
  ^
In file included from drivers/media/usb/cx231xx/cx231xx-dvb.c:35:0:
drivers/media/dvb-frontends/mb86a20s.h:49:2: warning: 
'mb86a20s_get_tuner_i2c_adapter' defined but not used [-Wunused-function]
  mb86a20s_get_tuner_i2c_adapter(struct dvb_frontend *fe)
  ^

Signed-off-by: Hans Verkuil 
---
 drivers/media/dvb-frontends/cx24123.h  | 2 +-
 drivers/media/dvb-frontends/mb86a20s.h | 2 +-
 drivers/media/dvb-frontends/s921.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/dvb-frontends/cx24123.h 
b/drivers/media/dvb-frontends/cx24123.h
index 758aee5..975f3c9 100644
--- a/drivers/media/dvb-frontends/cx24123.h
+++ b/drivers/media/dvb-frontends/cx24123.h
@@ -50,7 +50,7 @@ static inline struct dvb_frontend *cx24123_attach(
printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
return NULL;
 }
-static struct i2c_adapter *
+static inline struct i2c_adapter *
cx24123_get_tuner_i2c_adapter(struct dvb_frontend *fe)
 {
printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
diff --git a/drivers/media/dvb-frontends/mb86a20s.h 
b/drivers/media/dvb-frontends/mb86a20s.h
index f749c8a..a113282 100644
--- a/drivers/media/dvb-frontends/mb86a20s.h
+++ b/drivers/media/dvb-frontends/mb86a20s.h
@@ -45,7 +45,7 @@ static inline struct dvb_frontend *mb86a20s_attach(
printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
return NULL;
 }
-static struct i2c_adapter *
+static inline struct i2c_adapter *
mb86a20s_get_tuner_i2c_adapter(struct dvb_frontend *fe)
 {
printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
diff --git a/drivers/media/dvb-frontends/s921.h 
b/drivers/media/dvb-frontends/s921.h
index 7d3999a..f5b722d 100644
--- a/drivers/media/dvb-frontends/s921.h
+++ b/drivers/media/dvb-frontends/s921.h
@@ -36,7 +36,7 @@ static inline struct dvb_frontend *s921_attach(
printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
return NULL;
 }
-static struct i2c_adapter *
+static inline struct i2c_adapter *
s921_get_tuner_i2c_adapter(struct dvb_frontend *fe)
 {
printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
-- 
2.1.4

--
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

[PATCH 4/5] dib8000: fix compiler warning

[Hans Verkuil]

From: Hans Verkuil 

In file included from include/uapi/linux/posix_types.h:4:0,
 from include/uapi/linux/types.h:13,
 from include/linux/types.h:5,
 from include/uapi/linux/sysinfo.h:4,
 from include/uapi/linux/kernel.h:4,
 from include/linux/cache.h:4,
 from include/linux/time.h:4,
 from include/linux/input.h:11,
 from drivers/media/usb/dvb-usb/dvb-usb.h:13,
 from drivers/media/usb/dvb-usb/dib0700.h:13,
 from drivers/media/usb/dvb-usb/dib0700_devices.c:9:
drivers/media/dvb-frontends/dib8000.h: In function 'dib8000_attach':
include/linux/stddef.h:8:14: warning: return makes integer from pointer without 
a cast [-Wint-conversion]
 #define NULL ((void *)0)
  ^
drivers/media/dvb-frontends/dib8000.h:72:9: note: in expansion of macro 'NULL'
  return NULL;
 ^

Signed-off-by: Hans Verkuil 
---
 drivers/media/dvb-frontends/dib8000.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/dvb-frontends/dib8000.h 
b/drivers/media/dvb-frontends/dib8000.h
index 780c37b..25e97b4 100644
--- a/drivers/media/dvb-frontends/dib8000.h
+++ b/drivers/media/dvb-frontends/dib8000.h
@@ -69,7 +69,7 @@ void *dib8000_attach(struct dib8000_ops *ops);
 static inline int dib8000_attach(struct dib8000_ops *ops)
 {
printk(KERN_WARNING "%s: driver disabled by Kconfig\n", __func__);
-   return NULL;
+   return 0;
 }
 #endif
 
-- 
2.1.4

--
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

[PATCH 1/5] s5c73m3/s5k5baf/s5k6aa: fix compiler warnings

[Hans Verkuil]

From: Hans Verkuil 

Fix these compiler warnings that appeared after switching to gcc-5.1.0:

drivers/media/i2c/s5k5baf.c: In function 's5k5baf_set_power':
drivers/media/i2c/s5k5baf.c:1057:10: warning: logical not is only applied to 
the left hand side of comparison [-Wlogical-not-parentheses]
  if (!on != state->power)
  ^
drivers/media/i2c/s5k6aa.c: In function 's5k6aa_set_power':
drivers/media/i2c/s5k6aa.c:878:10: warning: logical not is only applied to the 
left hand side of comparison [-Wlogical-not-parentheses]
  if (!on == s5k6aa->power) {
  ^
drivers/media/i2c/s5c73m3/s5c73m3-core.c: In function 's5c73m3_oif_set_power':
drivers/media/i2c/s5c73m3/s5c73m3-core.c:1456:17: warning: logical not is only 
applied to the left hand side of comparison [-Wlogical-not-parentheses]
  } else if (!on == state->power) {
 ^

Signed-off-by: Hans Verkuil 
Cc: Kamil Debski 
---
 drivers/media/i2c/s5c73m3/s5c73m3-core.c | 2 +-
 drivers/media/i2c/s5k5baf.c  | 2 +-
 drivers/media/i2c/s5k6aa.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/i2c/s5c73m3/s5c73m3-core.c 
b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
index 08b234b..53c5ea8 100644
--- a/drivers/media/i2c/s5c73m3/s5c73m3-core.c
+++ b/drivers/media/i2c/s5c73m3/s5c73m3-core.c
@@ -1453,7 +1453,7 @@ static int s5c73m3_oif_set_power(struct v4l2_subdev *sd, 
int on)
state->apply_fiv = 1;
state->apply_fmt = 1;
}
-   } else if (!on == state->power) {
+   } else if (state->power == !on) {
ret = s5c73m3_set_af_softlanding(state);
if (!ret)
ret = __s5c73m3_power_off(state);
diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
index 297ef04..bee73de 100644
--- a/drivers/media/i2c/s5k5baf.c
+++ b/drivers/media/i2c/s5k5baf.c
@@ -1054,7 +1054,7 @@ static int s5k5baf_set_power(struct v4l2_subdev *sd, int 
on)
 
mutex_lock(&state->lock);
 
-   if (!on != state->power)
+   if (state->power != !on)
goto out;
 
if (on) {
diff --git a/drivers/media/i2c/s5k6aa.c b/drivers/media/i2c/s5k6aa.c
index de803a1..d0ad6a2 100644
--- a/drivers/media/i2c/s5k6aa.c
+++ b/drivers/media/i2c/s5k6aa.c
@@ -875,7 +875,7 @@ static int s5k6aa_set_power(struct v4l2_subdev *sd, int on)
 
mutex_lock(&s5k6aa->lock);
 
-   if (!on == s5k6aa->power) {
+   if (s5k6aa->power == !on) {
if (on) {
ret = __s5k6aa_power_on(s5k6aa);
if (!ret)
-- 
2.1.4

--
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

[PATCH 0/5] Fix compiler warnings

[Hans Verkuil]

From: Hans Verkuil 

Various patches to fix compiler warnings. Some of these appeared after
I switched to gcc-5.1.0 for the daily build.

There is one more warning in a davinci driver remaining, I've asked
Prabhakar to look at that one.

Regards,

Hans

Hans Verkuil (5):
  s5c73m3/s5k5baf/s5k6aa: fix compiler warnings
  s3c-camif: fix compiler warnings
  cx24123/mb86a20s/s921: fix compiler warnings
  dib8000: fix compiler warning
  radio-bcm2048: fix compiler warning

 drivers/media/dvb-frontends/cx24123.h| 2 +-
 drivers/media/dvb-frontends/dib8000.h| 2 +-
 drivers/media/dvb-frontends/mb86a20s.h   | 2 +-
 drivers/media/dvb-frontends/s921.h   | 2 +-
 drivers/media/i2c/s5c73m3/s5c73m3-core.c | 2 +-
 drivers/media/i2c/s5k5baf.c  | 2 +-
 drivers/media/i2c/s5k6aa.c   | 2 +-
 drivers/media/platform/s3c-camif/camif-capture.c | 4 ++--
 drivers/staging/media/bcm2048/radio-bcm2048.c| 3 +--
 9 files changed, 10 insertions(+), 11 deletions(-)

-- 
2.1.4

--
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

[PATCH 2/5] s3c-camif: fix compiler warnings

[Hans Verkuil]

From: Hans Verkuil 

Fix these compiler warnings that appeared after switching to gcc-5.1.0:

drivers/media/platform/s3c-camif/camif-capture.c: In function 
'sensor_set_power':
drivers/media/platform/s3c-camif/camif-capture.c:118:10: warning: logical not 
is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
  if (!on == camif->sensor.power_count)
  ^
drivers/media/platform/s3c-camif/camif-capture.c: In function 
'sensor_set_streaming':
drivers/media/platform/s3c-camif/camif-capture.c:134:10: warning: logical not 
is only applied to the left hand side of comparison [-Wlogical-not-parentheses]
  if (!on == camif->sensor.stream_count)
  ^

Signed-off-by: Hans Verkuil 
Cc: Kamil Debski 
---
 drivers/media/platform/s3c-camif/camif-capture.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/s3c-camif/camif-capture.c 
b/drivers/media/platform/s3c-camif/camif-capture.c
index f6a61b9..db4d7d2 100644
--- a/drivers/media/platform/s3c-camif/camif-capture.c
+++ b/drivers/media/platform/s3c-camif/camif-capture.c
@@ -115,7 +115,7 @@ static int sensor_set_power(struct camif_dev *camif, int on)
struct cam_sensor *sensor = &camif->sensor;
int err = 0;
 
-   if (!on == camif->sensor.power_count)
+   if (camif->sensor.power_count == !on)
err = v4l2_subdev_call(sensor->sd, core, s_power, on);
if (!err)
sensor->power_count += on ? 1 : -1;
@@ -131,7 +131,7 @@ static int sensor_set_streaming(struct camif_dev *camif, 
int on)
struct cam_sensor *sensor = &camif->sensor;
int err = 0;
 
-   if (!on == camif->sensor.stream_count)
+   if (camif->sensor.stream_count == !on)
err = v4l2_subdev_call(sensor->sd, video, s_stream, on);
if (!err)
sensor->stream_count += on ? 1 : -1;
-- 
2.1.4

--
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