Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH v3] manpage: update FALLOC_FL_INSERT_RANGE flag in fallocate
From: Michael Kerrisk (man-pages) @ 2015-05-08  9:40 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA, Linux API, Dave Chinner,
	Theodore Ts'o
In-Reply-To: <009201d0894f$d87f0890$897d19b0$@samsung.com>

Hi Namjae,

> Update FALLOC_FL_INSERT_RANGE flag in fallocate.

Sorry -- I should have said that I already applied your V2 
patch and made the fix we discussed (and that you add below).
I just hadn't pushed to Git yet. Done now.

Cheers,

Michael

On 05/08/2015 07:28 AM, Namjae Jeon wrote:
> Signed-off-by: Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Ashish Sangwan <a.sangwan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
>  man2/fallocate.2 | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 84 insertions(+), 5 deletions(-)
> 
> diff --git a/man2/fallocate.2 b/man2/fallocate.2
> index 0cc1a00..0d31027 100644
> --- a/man2/fallocate.2
> +++ b/man2/fallocate.2
> @@ -228,6 +228,59 @@ ext4, for extent-based files (since Linux 3.15)
>  .IP *
>  SMB3 (since Linux 3.17)
>  .\" commit 30175628bf7f521e9ee31ac98fa6d6fe7441a556
> +.SS Increasing file space
> +flag (available since Linux 4.1)
> +.\" commit dd46c787788d5bf5b974729d43e4c405814a4c7d
> +Specifying the
> +.BR FALLOC_FL_INSERT_RANGE
> +flag in
> +.I mode
> +will increase the file space by inserting a hole within the file size without
> +overwriting any existing data.
> +The hole will start at
> +.I offset
> +and continue for
> +.I len
> +bytes.
> +For inserting hole inside file, the contents of the file starting at
> +.I offset
> +will be shifted towards right by
> +.I len
> +bytes.
> +Inserting a hole inside the file will increase the file size by
> +.I len
> +bytes.
> +
> +This mode has the same limitation as
> +.BR FALLOC_FL_COLLAPSE_RANGE
> +regarding the
> +granularity of the operation.
> +If the granularity requirements are not met,
> +.BR fallocate ()
> +will fail with the error
> +.BR EINVAL.
> +If the
> +.I offset
> +is greater than or equal to the end of file, an error is
> +returned.
> +For such type of operations, i.e. inserting a hole at the end of file,
> +.BR ftruncate(2)
> +should be used.
> +In case
> +.IR offset + len
> +exceeds the maximum file size, errno will be set to
> +.B EFBIG.
> +
> +No other flags may be specified in
> +.IR mode
> +in conjunction with
> +.BR FALLOC_FL_INSERT_RANGE .
> +
> +As of Linux 4.1,
> +.B FALLOC_FL_INSERT_RANGE
> +is supported by
> +XFS.
> +.\" commit a904b1ca5751faf5ece8600e18cd3b674afcca1b
>  .SH RETURN VALUE
>  On success,
>  .BR fallocate ()
> @@ -245,6 +298,12 @@ is not a valid file descriptor, or is not opened for writing.
>  .IR offset + len
>  exceeds the maximum file size.
>  .TP
> +.B EFBIG
> +.I mode
> +is
> +.BR FALLOC_FL_INSERT_RANGE ,
> +the current file size+len exceeds the maximum file size.
> +.TP
>  .B EINTR
>  A signal was caught during execution.
>  .TP
> @@ -273,7 +332,17 @@ reaches or passes the end of the file.
>  .B EINVAL
>  .I mode
>  is
> -.BR FALLOC_FL_COLLAPSE_RANGE ,
> +.BR FALLOC_FL_INSERT_RANGE
> +and the range specified by
> +.I offset
> +reaches or passes the end of the file.
> +.TP
> +.B EINVAL
> +.I mode
> +is
> +.BR FALLOC_FL_COLLAPSE_RANGE
> +or
> +.BR FALLOC_FL_INSERT_RANGE ,
>  but either
>  .I offset
>  or
> @@ -282,18 +351,24 @@ is not a multiple of the filesystem block size.
>  .TP
>  .B EINVAL
>  .I mode
> -contains both
> +contains either of
>  .B FALLOC_FL_COLLAPSE_RANGE
> +or
> +.B FALLOC_FL_INSERT_RANGE
>  and other flags;
>  no other flags are permitted with
> -.BR FALLOC_FL_COLLAPSE_RANGE .
> +.BR FALLOC_FL_COLLAPSE_RANGE
> +or
> +.BR FALLOC_FL_INSERT_RANGE .
>  .TP
>  .B EINVAL
>  .I mode
>  is
>  .BR FALLOC_FL_COLLAPSE_RANGE
>  or
> -.BR FALLOC_FL_ZERO_RANGE ,
> +.BR FALLOC_FL_ZERO_RANGE
> +or
> +.BR FALLOC_FL_INSERT_RANGE ,
>  but the file referred to by
>  .I fd
>  is not a regular file.
> @@ -345,6 +420,8 @@ specifies
>  .BR FALLOC_FL_PUNCH_HOLE
>  or
>  .BR FALLOC_FL_COLLAPSE_RANGE
> +or
> +.BR FALLOC_FL_INSERT_RANGE
>  and
>  the file referred to by
>  .I fd
> @@ -363,7 +440,9 @@ refers to a pipe or FIFO.
>  .B ETXTBSY
>  .I mode
>  specifies
> -.BR FALLOC_FL_COLLAPSE_RANGE ,
> +.BR FALLOC_FL_COLLAPSE_RANGE
> +or
> +.BR FALLOC_FL_INSERT_RANGE ,
>  but the file referred to by
>  .IR fd
>  is currently being executed.
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: [PATCH v3] manpage: update FALLOC_FL_INSERT_RANGE flag in fallocate
From: Namjae Jeon @ 2015-05-08  9:42 UTC (permalink / raw)
  To: 'Michael Kerrisk (man-pages)'
  Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-man-u79uwXL29TY76Z2rM5mHXA, 'Linux API',
	'Dave Chinner', 'Theodore Ts'o'
In-Reply-To: <554C847A.9050709-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> Hi Namjae,
Hi Michael,

> 
> > Update FALLOC_FL_INSERT_RANGE flag in fallocate.
> 
> Sorry -- I should have said that I already applied your V2
> patch and made the fix we discussed (and that you add below).
> I just hadn't pushed to Git yet. Done now.
Okay :)
Thanks very much!!
> 
> Cheers,
> 
> Michael
> 
> On 05/08/2015 07:28 AM, Namjae Jeon wrote:
> > Signed-off-by: Namjae Jeon <namjae.jeon-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > Signed-off-by: Ashish Sangwan <a.sangwan-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > ---
> >  man2/fallocate.2 | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 84 insertions(+), 5 deletions(-)
> >
> > diff --git a/man2/fallocate.2 b/man2/fallocate.2
> > index 0cc1a00..0d31027 100644
> > --- a/man2/fallocate.2
> > +++ b/man2/fallocate.2
> > @@ -228,6 +228,59 @@ ext4, for extent-based files (since Linux 3.15)
> >  .IP *
> >  SMB3 (since Linux 3.17)
> >  .\" commit 30175628bf7f521e9ee31ac98fa6d6fe7441a556
> > +.SS Increasing file space
> > +flag (available since Linux 4.1)
> > +.\" commit dd46c787788d5bf5b974729d43e4c405814a4c7d
> > +Specifying the
> > +.BR FALLOC_FL_INSERT_RANGE
> > +flag in
> > +.I mode
> > +will increase the file space by inserting a hole within the file size without
> > +overwriting any existing data.
> > +The hole will start at
> > +.I offset
> > +and continue for
> > +.I len
> > +bytes.
> > +For inserting hole inside file, the contents of the file starting at
> > +.I offset
> > +will be shifted towards right by
> > +.I len
> > +bytes.
> > +Inserting a hole inside the file will increase the file size by
> > +.I len
> > +bytes.
> > +
> > +This mode has the same limitation as
> > +.BR FALLOC_FL_COLLAPSE_RANGE
> > +regarding the
> > +granularity of the operation.
> > +If the granularity requirements are not met,
> > +.BR fallocate ()
> > +will fail with the error
> > +.BR EINVAL.
> > +If the
> > +.I offset
> > +is greater than or equal to the end of file, an error is
> > +returned.
> > +For such type of operations, i.e. inserting a hole at the end of file,
> > +.BR ftruncate(2)
> > +should be used.
> > +In case
> > +.IR offset + len
> > +exceeds the maximum file size, errno will be set to
> > +.B EFBIG.
> > +
> > +No other flags may be specified in
> > +.IR mode
> > +in conjunction with
> > +.BR FALLOC_FL_INSERT_RANGE .
> > +
> > +As of Linux 4.1,
> > +.B FALLOC_FL_INSERT_RANGE
> > +is supported by
> > +XFS.
> > +.\" commit a904b1ca5751faf5ece8600e18cd3b674afcca1b
> >  .SH RETURN VALUE
> >  On success,
> >  .BR fallocate ()
> > @@ -245,6 +298,12 @@ is not a valid file descriptor, or is not opened for writing.
> >  .IR offset + len
> >  exceeds the maximum file size.
> >  .TP
> > +.B EFBIG
> > +.I mode
> > +is
> > +.BR FALLOC_FL_INSERT_RANGE ,
> > +the current file size+len exceeds the maximum file size.
> > +.TP
> >  .B EINTR
> >  A signal was caught during execution.
> >  .TP
> > @@ -273,7 +332,17 @@ reaches or passes the end of the file.
> >  .B EINVAL
> >  .I mode
> >  is
> > -.BR FALLOC_FL_COLLAPSE_RANGE ,
> > +.BR FALLOC_FL_INSERT_RANGE
> > +and the range specified by
> > +.I offset
> > +reaches or passes the end of the file.
> > +.TP
> > +.B EINVAL
> > +.I mode
> > +is
> > +.BR FALLOC_FL_COLLAPSE_RANGE
> > +or
> > +.BR FALLOC_FL_INSERT_RANGE ,
> >  but either
> >  .I offset
> >  or
> > @@ -282,18 +351,24 @@ is not a multiple of the filesystem block size.
> >  .TP
> >  .B EINVAL
> >  .I mode
> > -contains both
> > +contains either of
> >  .B FALLOC_FL_COLLAPSE_RANGE
> > +or
> > +.B FALLOC_FL_INSERT_RANGE
> >  and other flags;
> >  no other flags are permitted with
> > -.BR FALLOC_FL_COLLAPSE_RANGE .
> > +.BR FALLOC_FL_COLLAPSE_RANGE
> > +or
> > +.BR FALLOC_FL_INSERT_RANGE .
> >  .TP
> >  .B EINVAL
> >  .I mode
> >  is
> >  .BR FALLOC_FL_COLLAPSE_RANGE
> >  or
> > -.BR FALLOC_FL_ZERO_RANGE ,
> > +.BR FALLOC_FL_ZERO_RANGE
> > +or
> > +.BR FALLOC_FL_INSERT_RANGE ,
> >  but the file referred to by
> >  .I fd
> >  is not a regular file.
> > @@ -345,6 +420,8 @@ specifies
> >  .BR FALLOC_FL_PUNCH_HOLE
> >  or
> >  .BR FALLOC_FL_COLLAPSE_RANGE
> > +or
> > +.BR FALLOC_FL_INSERT_RANGE
> >  and
> >  the file referred to by
> >  .I fd
> > @@ -363,7 +440,9 @@ refers to a pipe or FIFO.
> >  .B ETXTBSY
> >  .I mode
> >  specifies
> > -.BR FALLOC_FL_COLLAPSE_RANGE ,
> > +.BR FALLOC_FL_COLLAPSE_RANGE
> > +or
> > +.BR FALLOC_FL_INSERT_RANGE ,
> >  but the file referred to by
> >  .IR fd
> >  is currently being executed.
> >
> 
> 
> --
> Michael Kerrisk
> Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
> Linux/UNIX System Programming Training: http://man7.org/training/

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 3/3] proc: add kpageidle file
From: Vladimir Davydov @ 2015-05-08  9:56 UTC (permalink / raw)
  To: Minchan Kim
  Cc: Andrew Morton, Johannes Weiner, Michal Hocko, Greg Thelen,
	Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rik van Riel, Hugh Dickins,
	Christoph Lameter, Paul E. McKenney, Peter Zijlstra
In-Reply-To: <20150504105459.GA19384@blaptop>

On Mon, May 04, 2015 at 07:54:59PM +0900, Minchan Kim wrote:
> So, I guess once below compiler optimization happens in __page_set_anon_rmap,
> it could be corrupt in page_refernced.
> 
> __page_set_anon_rmap:
>         page->mapping = (struct address_space *) anon_vma;
>         page->mapping = (struct address_space *)((void *)page_mapping + PAGE_MAPPING_ANON);
> 
> Because page_referenced checks it with PageAnon which has no memory barrier.
> So if above compiler optimization happens, page_referenced can pass the anon
> page in rmap_walk_file, not ramp_walk_anon. It's my theory. :)

FWIW

If such splits were possible, we would have bugs all over the kernel
IMO. An example is do_wp_page() vs shrink_active_list(). In do_wp_page()
we can call page_move_anon_rmap(), which sets page->mapping in exactly
the same fashion as above-mentioned __page_set_anon_rmap():

	anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON;
	page->mapping = (struct address_space *) anon_vma;

The page in question may be on an LRU list, because nowhere in
do_wp_page() we remove it from the list, neither do we take any LRU
related locks. The page is locked, that's true, but shrink_active_list()
calls page_referenced() on an unlocked page, so according to your logic
they can race with the latter receiving a page with page->mapping equal
to anon_vma w/o PAGE_MAPPING_ANON bit set:

CPU0				CPU1
----				----
do_wp_page			shrink_active_list
 lock_page			 page_referenced
				  PageAnon->yes, so skip trylock_page
 page_move_anon_rmap
  page->mapping = anon_vma
				  rmap_walk
				   PageAnon->no
				   rmap_walk_file
				    BUG
  page->mapping = page->mapping+PAGE_MAPPING_ANON

However, this does not happen.

Thanks,
Vladimir

^ permalink raw reply

* Re: [PATCH v3 02/12] KVM: define common __KVM_GUESTDBG_USE_SW/HW_BP values
From: Paolo Bonzini @ 2015-05-08 11:09 UTC (permalink / raw)
  To: Christoffer Dall, Alex Bennée
  Cc: kvm, Alexey Kardashevskiy, Benjamin Herrenschmidt, Paul Mackerras,
	H. Peter Anvin, kvmarm, Nadav Amit, Michael Ellerman,
	maintainer:X86 ARCHITECTURE..., Gleb Natapov, Ingo Molnar,
	zhichao.huang, jan.kiszka, bp, marc.zyngier, r65777,
	Thomas Gleixner, linux-arm-kernel, list, ABI/API, open list, dahi,
	open, open list:LINUX FOR POWERPC...
In-Reply-To: <20150508092346.GB24744@cbox>



On 08/05/2015 11:23, Christoffer Dall wrote:
> On Wed, May 06, 2015 at 05:23:17PM +0100, Alex Bennée wrote:
>> Currently x86, powerpc and soon arm64 use the same two architecture
>> specific bits for guest debug support for software and hardware
>> breakpoints. This makes the shared values explicit while leaving the
>> gate open for another architecture to use some other value if they
>> really really want to.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>>
>> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>> index ab4d473..1731569 100644
>> --- a/arch/powerpc/include/uapi/asm/kvm.h
>> +++ b/arch/powerpc/include/uapi/asm/kvm.h
>> @@ -310,8 +310,8 @@ struct kvm_guest_debug_arch {
>>   * and upper 16 bits are architecture specific. Architecture specific defines
>>   * that ioctl is for setting hardware breakpoint or software breakpoint.
>>   */
>> -#define KVM_GUESTDBG_USE_SW_BP		0x00010000
>> -#define KVM_GUESTDBG_USE_HW_BP		0x00020000
>> +#define KVM_GUESTDBG_USE_SW_BP		__KVM_GUESTDBG_USE_SW_BP
>> +#define KVM_GUESTDBG_USE_HW_BP		__KVM_GUESTDBG_USE_HW_BP
>>  
>>  /* definition of registers in kvm_run */
>>  struct kvm_sync_regs {
>> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
>> index d7dcef5..1438202 100644
>> --- a/arch/x86/include/uapi/asm/kvm.h
>> +++ b/arch/x86/include/uapi/asm/kvm.h
>> @@ -250,8 +250,8 @@ struct kvm_debug_exit_arch {
>>  	__u64 dr7;
>>  };
>>  
>> -#define KVM_GUESTDBG_USE_SW_BP		0x00010000
>> -#define KVM_GUESTDBG_USE_HW_BP		0x00020000
>> +#define KVM_GUESTDBG_USE_SW_BP		__KVM_GUESTDBG_USE_SW_BP
>> +#define KVM_GUESTDBG_USE_HW_BP		__KVM_GUESTDBG_USE_HW_BP
>>  #define KVM_GUESTDBG_INJECT_DB		0x00040000
>>  #define KVM_GUESTDBG_INJECT_BP		0x00080000
>>  
>> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
>> index 70ac641..3b6252e 100644
>> --- a/include/uapi/linux/kvm.h
>> +++ b/include/uapi/linux/kvm.h
>> @@ -570,8 +570,16 @@ struct kvm_s390_irq_state {
>>  
>>  /* for KVM_SET_GUEST_DEBUG */
>>  
>> -#define KVM_GUESTDBG_ENABLE		0x00000001
>> -#define KVM_GUESTDBG_SINGLESTEP		0x00000002
>> +#define KVM_GUESTDBG_ENABLE		(1 << 0)
>> +#define KVM_GUESTDBG_SINGLESTEP	(1 << 1)
>> +
>> +/*
>> + * Architecture specific stuff uses the top 16 bits of the field,
> 
> s/stuff/<something more specific>/
> 
>> + * however there is some shared commonality for the common cases
>> + */
>> +#define __KVM_GUESTDBG_USE_SW_BP	(1 << 16)
>> +#define __KVM_GUESTDBG_USE_HW_BP	(1 << 17)
>> +
>>  
>>  struct kvm_guest_debug {
>>  	__u32 control;
> 
> We sort of left this discussion hanging with me expressing slight
> concern about the usefulness about these defines.
> 
> Paolo, what are your thoughts?

I would just lift these two KVM_GUESTDBG_* defines to
include/uapi/linux/kvm.h and say that architecture specific stuff uses
the top 14 bits of the field. :)

Paolo

^ permalink raw reply

* Re: [PATCH 02/18] media controller: deprecate entity subtype
From: Hans Verkuil @ 2015-05-08 11:39 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linux Media Mailing List
  Cc: Mauro Carvalho Chehab, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <80e0882a4194460ca232f19ebbc85fa3338eda3f.1431046915.git.mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>

Hi Mauro,

Just a typo, after that you can add my:

Acked-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Thanks,

	Hans

On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> The media controller entity subtype doesn't make much sense,
> especially since V4L2 subdevices may also have associated devnodes.
> 
> So, better to get rid of it while it is not too late.
> 
> We need, of course, to keep the old symbols to avoid userspace
> breakage, but we should avoid using them internally at the
> Kernel.
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> 
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 4e816be3de39..775c11c6b173 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -42,31 +42,45 @@ struct media_device_info {
>  
>  #define MEDIA_ENT_ID_FLAG_NEXT		(1 << 31)
>  
> +/* Used values for media_entity_desc::type */
> +
> +#define MEDIA_ENT_T_DEVNODE_V4L		(((1 << 16)) + 1)
> +#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_DEVNODE_V4L + 3)
> +#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_DEVNODE_V4L + 4)
> +#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_DEVNODE_V4L + 5)
> +#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_DEVNODE_V4L + 6)
> +#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_DEVNODE_V4L + 7)
> +
> +#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	((2 << 16) + 1)
> +#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 1)
> +#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 2)
> +/* A converter of analogue video to its digital representation. */
> +#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 3)
> +
> +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 4)
> +
> +#if 1
> +/*
> + * Legacy symbols.
> + * Kept just to avoid userspace compilation breakages.
> + * One day, the symbols bellow will be removed

bellow -> below

> + */
> +
>  #define MEDIA_ENT_TYPE_SHIFT		16
>  #define MEDIA_ENT_TYPE_MASK		0x00ff0000
>  #define MEDIA_ENT_SUBTYPE_MASK		0x0000ffff
>  
>  #define MEDIA_ENT_T_DEVNODE		(1 << MEDIA_ENT_TYPE_SHIFT)
> -#define MEDIA_ENT_T_DEVNODE_V4L		(MEDIA_ENT_T_DEVNODE + 1)
> +#define MEDIA_ENT_T_V4L2_SUBDEV		(2 << MEDIA_ENT_TYPE_SHIFT)
> +
>  #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
>  #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
> -#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_DEVNODE + 4)
> -#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_DEVNODE + 5)
> -#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_DEVNODE + 6)
> -#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_DEVNODE + 7)
> -#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_DEVNODE + 8)
>  
> -/* Legacy symbol. Use it to avoid userspace compilation breakages */
> +
>  #define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
> +#endif
>  
> -#define MEDIA_ENT_T_V4L2_SUBDEV		(2 << MEDIA_ENT_TYPE_SHIFT)
> -#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	(MEDIA_ENT_T_V4L2_SUBDEV + 1)
> -#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV + 2)
> -#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV + 3)
> -/* A converter of analogue video to its digital representation. */
> -#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV + 4)
> -
> -#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV + 5)
> +/* Used bitmasks for media_entity_desc::flags */
>  
>  #define MEDIA_ENT_FL_DEFAULT		(1 << 0)
>  
> 

^ permalink raw reply

* Re: [PATCH 03/18] media controller: use MEDIA_ENT_T_AV_DMA for A/V DMA engines
From: Hans Verkuil @ 2015-05-08 11:54 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linux Media Mailing List
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, Hyun Kwon,
	Laurent Pinchart, Michal Simek, Sören Brinkmann,
	Sakari Ailus, Sylwester Nawrocki, Ramakrishnan Muthukrishnan,
	Markus Elfring, linux-doc, linux-arm-kernel, linux-api
In-Reply-To: <afb84e3d80fc4f6f2465a123012f161b8c29f1c4.1431046915.git.mchehab@osg.samsung.com>

On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> At the Video4Linux API, the /dev/video?, /dev/vbi? and
> /dev/radio? device nodes are used for the chipset that

You should add /dev/swradio? for SDR devices.

> provides the bridge between video/radio streams and the
> USB, PCI or CPU buses.
> 
> Such bridge is also typically used to control the V4L2 device
> as a hole.

hole -> whole

> 
> For video streaming devices and SDR radio devices, they're
> also associated with the DMA engines that transfer the
> video stream (or SDR stream) to the CPU's memory.
> 
> It should be noticed, however, this is not true on non-SDR
> radio devices,

I think you forgot that SDR devices are not using /dev/radio
but /dev/swradio. They have different names. Radio devices never
stream (OK, I think there are one or two exceptions, but that's really
because nobody bothered to make an alsa driver. Those boards are
in practice out of spec.)

> and may also not be true on embedded devices
> that, due to DRM reasons, don't allow writing unencrypted
> data on a memory that could be seen by the CPU.

This actually might still work by using opaque DMABUF handles. But that's
under discussion right now in the Secure Data Path thread.

Another reason can also be that the SoC vendor re-invented the wheel
and made there own DMA streaming solution. You can still make V4L drivers
that control the video receivers/transmitters, but for the actual streaming
you are forced to use the vendor's crap code (hello TI!).

I've bitter experiences with that :-(
 
> So, we'll eventually need to add another entity for such
> bridge chipsets that have a video/vbi/radio device node
> associated, but don't have DMA engines on (some) devnodes.
> 
> As, currently, we don't have any such case,

??? Radio devices are exactly that.

> let's for now
> just rename the device nodes that are associated with a
> DMA engine as MEDIA_ENT_T_AV_DMA.
> 
> So,
> 	MEDIA_ENT_T_DEVNODE_V4L -> MEDIA_ENT_T_AV_DMA
> 
> PS.: This is not actually true for USB devices, as the
> DMA engine is an internal component, as it is up to the
> Kernel to strip the stream payload from the URB packages.

How about MEDIA_ENT_T_DATA_STREAMING? Or perhaps DATA_IO? Perhaps even just
"IO"?

That would cover USB as well, and I dislike the use of "AV", since the
data might contain other things besides audio and/or video.

Regards,

	Hans

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> index 5872f8bbf774..5b8147629159 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> @@ -183,7 +183,7 @@
>  	    <entry>Unknown device node</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_DEVNODE_V4L</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_AV_DMA</constant></entry>
>  	    <entry>V4L video, radio or vbi device node</entry>
>  	  </row>
>  	  <row>
> diff --git a/drivers/media/platform/xilinx/xilinx-dma.c b/drivers/media/platform/xilinx/xilinx-dma.c
> index efde88adf624..7fa0cc0f08f0 100644
> --- a/drivers/media/platform/xilinx/xilinx-dma.c
> +++ b/drivers/media/platform/xilinx/xilinx-dma.c
> @@ -193,7 +193,7 @@ static int xvip_pipeline_validate(struct xvip_pipeline *pipe,
>  	while ((entity = media_entity_graph_walk_next(&graph))) {
>  		struct xvip_dma *dma;
>  
> -		if (entity->type != MEDIA_ENT_T_DEVNODE_V4L)
> +		if (entity->type != MEDIA_ENT_T_AV_DMA)
>  			continue;
>  
>  		dma = to_xvip_dma(media_entity_to_video_device(entity));
> diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
> index 71a1b93b0790..9ef920221b5a 100644
> --- a/drivers/media/v4l2-core/v4l2-dev.c
> +++ b/drivers/media/v4l2-core/v4l2-dev.c
> @@ -912,7 +912,7 @@ int __video_register_device(struct video_device *vdev, int type, int nr,
>  	/* Part 5: Register the entity. */
>  	if (vdev->v4l2_dev->mdev &&
>  	    vdev->vfl_type != VFL_TYPE_SUBDEV) {
> -		vdev->entity.type = MEDIA_ENT_T_DEVNODE_V4L;
> +		vdev->entity.type = MEDIA_ENT_T_AV_DMA;
>  		vdev->entity.name = vdev->name;
>  		vdev->entity.info.dev.major = VIDEO_MAJOR;
>  		vdev->entity.info.dev.minor = vdev->minor;
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 63596063b213..9f8fc8330b3e 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -535,7 +535,7 @@ v4l2_subdev_link_validate_get_format(struct media_pad *pad,
>  		return v4l2_subdev_call(sd, pad, get_fmt, NULL, fmt);
>  	}
>  
> -	WARN(pad->entity->type != MEDIA_ENT_T_DEVNODE_V4L,
> +	WARN(pad->entity->type != MEDIA_ENT_T_AV_DMA,
>  	     "Driver bug! Wrong media entity type 0x%08x, entity %s\n",
>  	     pad->entity->type, pad->entity->name);
>  
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 775c11c6b173..a7aa2aac9c23 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -44,12 +44,12 @@ struct media_device_info {
>  
>  /* Used values for media_entity_desc::type */
>  
> -#define MEDIA_ENT_T_DEVNODE_V4L		(((1 << 16)) + 1)
> -#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_DEVNODE_V4L + 3)
> -#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_DEVNODE_V4L + 4)
> -#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_DEVNODE_V4L + 5)
> -#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_DEVNODE_V4L + 6)
> -#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_DEVNODE_V4L + 7)
> +#define MEDIA_ENT_T_AV_DMA		(((1 << 16)) + 1)
> +#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_AV_DMA + 3)
> +#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_AV_DMA + 4)
> +#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_AV_DMA + 5)
> +#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_AV_DMA + 6)
> +#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_AV_DMA + 7)
>  
>  #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	((2 << 16) + 1)
>  #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 1)
> 


^ permalink raw reply

* Re: [PATCH 04/18] media controller: Rename camera entities
From: Hans Verkuil @ 2015-05-08 12:03 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linux Media Mailing List
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, Sakari Ailus,
	Laurent Pinchart, Kyungmin Park, Heungjun Kim, Prabhakar Lad,
	Andrzej Hajda, Sylwester Nawrocki, Boris BREZILLON, linux-doc,
	linux-api
In-Reply-To: <a1a45e1b62e9dc69fd0a2d11dff57a414304c541.1431046915.git.mchehab@osg.samsung.com>

On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> As explained before, the hole idea of subtypes at entities was

hole -> whole

> not nice. All V4L2 subdevs may have a device node associated.
> 
> Also, the hole idea is to expose hardware IP blocks, so calling
> them as V4L2 is a very bad practice, as they were not designed
> for the V4L2 API. It is just the reverse.
> 
> So, instead of using V4L2_SUBDEV, let's call the camera sub-
> devices with CAM, instead:
> 
> 	MEDIA_ENT_T_V4L2_SUBDEV_SENSOR -> MEDIA_ENT_T_CAM_SENSOR
> 	MEDIA_ENT_T_V4L2_SUBDEV_FLASH  -> MEDIA_ENT_T_CAM_FLASH
> 	MEDIA_ENT_T_V4L2_SUBDEV_LENS   -> MEDIA_ENT_T_CAM_LENS

I would actually postpone this until Laurent has a properties API ready.
These entity types are fatally flawed since an entity can combine functions
in one. E.g. an i2c device (generally represented as a single entity) might
provide for both sensor and flash. Or combine tuner and video decoder, etc.

Basically an entity like this is a sub-device (as in the literal meaning
of being a part of a larger device) that has one or more functions and may
have device node(s) associated with it. That is best expressed as properties.

And you really do have to tell userspace that these entities expose a
v4l-subdev device node. Renaming them doesn't make that go away.

Regards,

	Hans

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> index 5b8147629159..759604e3529f 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> @@ -219,15 +219,15 @@
>  	    <entry>Unknown V4L sub-device</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV_SENSOR</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_CAM_SENSOR</constant></entry>
>  	    <entry>Video sensor</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV_FLASH</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_CAM_FLASH</constant></entry>
>  	    <entry>Flash controller</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV_LENS</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_CAM_LENS</constant></entry>
>  	    <entry>Lens controller</entry>
>  	  </row>
>  	  <row>
> diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c
> index c70ababce954..c12f873a8e26 100644
> --- a/drivers/media/i2c/adp1653.c
> +++ b/drivers/media/i2c/adp1653.c
> @@ -516,7 +516,7 @@ static int adp1653_probe(struct i2c_client *client,
>  	if (ret < 0)
>  		goto free_and_quit;
>  
> -	flash->subdev.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> +	flash->subdev.entity.type = MEDIA_ENT_T_CAM_FLASH;
>  
>  	return 0;
>  
> diff --git a/drivers/media/i2c/as3645a.c b/drivers/media/i2c/as3645a.c
> index 301084b07887..9a2872be11b0 100644
> --- a/drivers/media/i2c/as3645a.c
> +++ b/drivers/media/i2c/as3645a.c
> @@ -831,7 +831,7 @@ static int as3645a_probe(struct i2c_client *client,
>  	if (ret < 0)
>  		goto done;
>  
> -	flash->subdev.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> +	flash->subdev.entity.type = MEDIA_ENT_T_CAM_FLASH;
>  
>  	mutex_init(&flash->power_lock);
>  
> diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> index d9ece4b2d047..4d28cda77f4d 100644
> --- a/drivers/media/i2c/lm3560.c
> +++ b/drivers/media/i2c/lm3560.c
> @@ -368,7 +368,7 @@ static int lm3560_subdev_init(struct lm3560_flash *flash,
>  	rval = media_entity_init(&flash->subdev_led[led_no].entity, 0, NULL, 0);
>  	if (rval < 0)
>  		goto err_out;
> -	flash->subdev_led[led_no].entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> +	flash->subdev_led[led_no].entity.type = MEDIA_ENT_T_CAM_FLASH;
>  
>  	return rval;
>  
> diff --git a/drivers/media/i2c/lm3646.c b/drivers/media/i2c/lm3646.c
> index 626fb4679c02..19ee2ac00be7 100644
> --- a/drivers/media/i2c/lm3646.c
> +++ b/drivers/media/i2c/lm3646.c
> @@ -285,7 +285,7 @@ static int lm3646_subdev_init(struct lm3646_flash *flash)
>  	rval = media_entity_init(&flash->subdev_led.entity, 0, NULL, 0);
>  	if (rval < 0)
>  		goto err_out;
> -	flash->subdev_led.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> +	flash->subdev_led.entity.type = MEDIA_ENT_T_CAM_FLASH;
>  	return rval;
>  
>  err_out:
> diff --git a/drivers/media/i2c/m5mols/m5mols_core.c b/drivers/media/i2c/m5mols/m5mols_core.c
> index 6404c0d93e7a..beb519cf8be4 100644
> --- a/drivers/media/i2c/m5mols/m5mols_core.c
> +++ b/drivers/media/i2c/m5mols/m5mols_core.c
> @@ -978,7 +978,7 @@ static int m5mols_probe(struct i2c_client *client,
>  	ret = media_entity_init(&sd->entity, 1, &info->pad, 0);
>  	if (ret < 0)
>  		return ret;
> -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
>  
>  	init_waitqueue_head(&info->irq_waitq);
>  	mutex_init(&info->lock);
> diff --git a/drivers/media/i2c/noon010pc30.c b/drivers/media/i2c/noon010pc30.c
> index f197b6cbd407..a03556197405 100644
> --- a/drivers/media/i2c/noon010pc30.c
> +++ b/drivers/media/i2c/noon010pc30.c
> @@ -779,7 +779,7 @@ static int noon010_probe(struct i2c_client *client,
>  		goto np_err;
>  
>  	info->pad.flags = MEDIA_PAD_FL_SOURCE;
> -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
>  	ret = media_entity_init(&sd->entity, 1, &info->pad, 0);
>  	if (ret < 0)
>  		goto np_err;
> diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
> index d700a1d0a6f2..63b9464e813f 100644
> --- a/drivers/media/i2c/ov2659.c
> +++ b/drivers/media/i2c/ov2659.c
> @@ -1425,7 +1425,7 @@ static int ov2659_probe(struct i2c_client *client,
>  
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>  	ov2659->pad.flags = MEDIA_PAD_FL_SOURCE;
> -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
>  	ret = media_entity_init(&sd->entity, 1, &ov2659->pad, 0);
>  	if (ret < 0) {
>  		v4l2_ctrl_handler_free(&ov2659->ctrls);
> diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
> index 2bc473385c91..ed3c0573a0f8 100644
> --- a/drivers/media/i2c/ov9650.c
> +++ b/drivers/media/i2c/ov9650.c
> @@ -1500,7 +1500,7 @@ static int ov965x_probe(struct i2c_client *client,
>  		return ret;
>  
>  	ov965x->pad.flags = MEDIA_PAD_FL_SOURCE;
> -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
>  	ret = media_entity_init(&sd->entity, 1, &ov965x->pad, 0);
>  	if (ret < 0)
>  		return ret;
> diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c
> index 97084237275d..23af7f90678a 100644
> --- a/drivers/media/i2c/s5k4ecgx.c
> +++ b/drivers/media/i2c/s5k4ecgx.c
> @@ -961,7 +961,7 @@ static int s5k4ecgx_probe(struct i2c_client *client,
>  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>  
>  	priv->pad.flags = MEDIA_PAD_FL_SOURCE;
> -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
>  	ret = media_entity_init(&sd->entity, 1, &priv->pad, 0);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
> index bee73de347dc..fadd48d35a55 100644
> --- a/drivers/media/i2c/s5k5baf.c
> +++ b/drivers/media/i2c/s5k5baf.c
> @@ -408,7 +408,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
>  
>  static inline bool s5k5baf_is_cis_subdev(struct v4l2_subdev *sd)
>  {
> -	return sd->entity.type == MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +	return sd->entity.type == MEDIA_ENT_T_CAM_SENSOR;
>  }
>  
>  static inline struct s5k5baf *to_s5k5baf(struct v4l2_subdev *sd)
> @@ -1904,7 +1904,7 @@ static int s5k5baf_configure_subdevs(struct s5k5baf *state,
>  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>  
>  	state->cis_pad.flags = MEDIA_PAD_FL_SOURCE;
> -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
>  	ret = media_entity_init(&sd->entity, NUM_CIS_PADS, &state->cis_pad, 0);
>  	if (ret < 0)
>  		goto err;
> diff --git a/drivers/media/i2c/s5k6aa.c b/drivers/media/i2c/s5k6aa.c
> index d0ad6a25bdab..d2390c6e5dbe 100644
> --- a/drivers/media/i2c/s5k6aa.c
> +++ b/drivers/media/i2c/s5k6aa.c
> @@ -1577,7 +1577,7 @@ static int s5k6aa_probe(struct i2c_client *client,
>  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>  
>  	s5k6aa->pad.flags = MEDIA_PAD_FL_SOURCE;
> -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
>  	ret = media_entity_init(&sd->entity, 1, &s5k6aa->pad, 0);
>  	if (ret)
>  		return ret;
> diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
> index 636ebd6fe5dc..78f2cdd3561b 100644
> --- a/drivers/media/i2c/smiapp/smiapp-core.c
> +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> @@ -2763,7 +2763,7 @@ static int smiapp_init(struct smiapp_sensor *sensor)
>  
>  	dev_dbg(&client->dev, "profile %d\n", sensor->minfo.smiapp_profile);
>  
> -	sensor->pixel_array->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> +	sensor->pixel_array->sd.entity.type = MEDIA_ENT_T_CAM_SENSOR;
>  
>  	/* final steps */
>  	smiapp_read_frame_fmt(sensor);
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index a7aa2aac9c23..2e465ba087ba 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -51,13 +51,13 @@ struct media_device_info {
>  #define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_AV_DMA + 6)
>  #define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_AV_DMA + 7)
>  
> -#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	((2 << 16) + 1)
> -#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 1)
> -#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 2)
> +#define MEDIA_ENT_T_CAM_SENSOR	((2 << 16) + 1)
> +#define MEDIA_ENT_T_CAM_FLASH	(MEDIA_ENT_T_CAM_SENSOR + 1)
> +#define MEDIA_ENT_T_CAM_LENS	(MEDIA_ENT_T_CAM_SENSOR + 2)
>  /* A converter of analogue video to its digital representation. */
> -#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 3)
> +#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_CAM_SENSOR + 3)
>  
> -#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 4)
> +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_CAM_SENSOR + 4)
>  
>  #if 1
>  /*
> @@ -76,8 +76,10 @@ struct media_device_info {
>  #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
>  #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
>  
> -
>  #define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
> +#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	MEDIA_ENT_T_CAM_SENSOR
> +#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	MEDIA_ENT_T_CAM_FLASH
> +#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	MEDIA_ENT_T_CAM_LENS
>  #endif
>  
>  /* Used bitmasks for media_entity_desc::flags */
> 

^ permalink raw reply

* Re: [PATCH v18 0/3] sys_membarrier()
From: Michael Kerrisk @ 2015-05-08 12:06 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: Andrew Morton, Linux Kernel, Linux API
In-Reply-To: <1430940068-4326-1-git-send-email-mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org>

[CC += linux-api@]

On Wed, May 6, 2015 at 9:21 PM, Mathieu Desnoyers
<mathieu.desnoyers-vg+e7yoeK/dWk0Htik3J/w@public.gmane.org> wrote:
> Here is v18 of the sys_membarrier patchset, taking care of comments about v17.
> Let see if there are further comments on this patch round.
>
> Thanks,
>
> Mathieu
>
> Mathieu Desnoyers (2):
>   sys_membarrier(): system-wide memory barrier (generic, x86)
>   selftests: enhance membarrier syscall test
>
> Pranith Kumar (1):
>   selftests: add membarrier syscall test
>
>  MAINTAINERS                                        |    8 ++
>  arch/x86/syscalls/syscall_32.tbl                   |    1 +
>  arch/x86/syscalls/syscall_64.tbl                   |    1 +
>  include/linux/syscalls.h                           |    2 +
>  include/uapi/asm-generic/unistd.h                  |    4 +-
>  include/uapi/linux/Kbuild                          |    1 +
>  include/uapi/linux/membarrier.h                    |   53 +++++++++
>  init/Kconfig                                       |   12 ++
>  kernel/Makefile                                    |    1 +
>  kernel/membarrier.c                                |   66 +++++++++++
>  kernel/sys_ni.c                                    |    3 +
>  tools/testing/selftests/Makefile                   |    1 +
>  tools/testing/selftests/membarrier/.gitignore      |    1 +
>  tools/testing/selftests/membarrier/Makefile        |   11 ++
>  .../testing/selftests/membarrier/membarrier_test.c |  121 ++++++++++++++++++++
>  15 files changed, 285 insertions(+), 1 deletions(-)
>  create mode 100644 include/uapi/linux/membarrier.h
>  create mode 100644 kernel/membarrier.c
>  create mode 100644 tools/testing/selftests/membarrier/.gitignore
>  create mode 100644 tools/testing/selftests/membarrier/Makefile
>  create mode 100644 tools/testing/selftests/membarrier/membarrier_test.c
>
> --
> 1.7.7.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Michael Kerrisk Linux man-pages maintainer;
http://www.kernel.org/doc/man-pages/
Author of "The Linux Programming Interface", http://blog.man7.org/

^ permalink raw reply

* Re: [PATCH 05/18] media controller: rename MEDIA_ENT_T_DEVNODE_DVB entities
From: Hans Verkuil @ 2015-05-08 12:10 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linux Media Mailing List
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, linux-doc, linux-api
In-Reply-To: <f448ae9a612a6ceb05e0fd669bf252fa90aa278a.1431046915.git.mchehab@osg.samsung.com>

On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> In order to reflect that the entities are actually the hardware
> (or firmware, or in-kernel software), and are not associated
> with the DVB API, let's remove DEVNODE_ from the entity names
> and use DTV (Digital TV) for the entities.
> 
> The frontend is an special case: the frontend devnode actually
> talks directly with the DTV demodulator. It may or may not also
> talk with the SEC (Satellite Equipment Control) and with the
> tuner. For the sake of unifying the nomenclature, let's call it
> as MEDIA_ENT_T_DTV_DEMOD, because this component is always
> there.
> 
> So:
> 
> 	MEDIA_ENT_T_DEVNODE_DVB_FE    -> MEDIA_ENT_T_DTV_DEMOD
> 	MEDIA_ENT_T_DEVNODE_DVB_DEMUX -> MEDIA_ENT_T_DTV_DEMUX
> 	MEDIA_ENT_T_DEVNODE_DVB_DVR   -> MEDIA_ENT_T_DTV_DVR
> 	MEDIA_ENT_T_DEVNODE_DVB_CA    -> MEDIA_ENT_T_DTV_CA
> 	MEDIA_ENT_T_DEVNODE_DVB_NET   -> MEDIA_ENT_T_DTV_NET

I'm happy with the new names.

> 
> PS.: we could actually not keep this define:
> 	#define MEDIA_ENT_T_DEVNODE_DVB_FE MEDIA_ENT_T_DTV_DEMOD
> 
> As MEDIA_ENT_T_DEVNODE_DVB_FE symbol will not arrive any Kernel
> version (being present only at the 4.1-rc kernels), but keeping
> it helps to show that the DVB frontend node is actually associated
> with the DTV demodulator. So, keeping it for now helps to better
> document. Also, it avoids to break experimental versions of v4l-utils.
> So, better to remove this only when we remove the remaining legacy
> stuff.

I disagree with that. Let's not introduce defines that are not going to
be used. And v4l-utils is easily fixed.

Instead of keeping an unused define, why not...

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> index 759604e3529f..27082b07f4c2 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> @@ -195,23 +195,23 @@
>  	    <entry>ALSA card</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_FE</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_DTV_DEMOD</constant></entry>
>  	    <entry>DVB frontend devnode</entry>

... explain what is going on here? I.e. that this frontend always controls a demod
and optionally also a tuner and/or SEC.

Regards,

	Hans

>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_DEMUX</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_DTV_DEMUX</constant></entry>
>  	    <entry>DVB demux devnode</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_DVR</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_DTV_DVR</constant></entry>
>  	    <entry>DVB DVR devnode</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_CA</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_DTV_CA</constant></entry>
>  	    <entry>DVB CAM devnode</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_NET</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_DTV_NET</constant></entry>
>  	    <entry>DVB network devnode</entry>
>  	  </row>
>  	  <row>
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 13bb57f0457f..39846077045e 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -221,26 +221,26 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
>  
>  	switch (type) {
>  	case DVB_DEVICE_FRONTEND:
> -		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
> +		dvbdev->entity->type = MEDIA_ENT_T_DTV_DEMOD;
>  		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>  		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>  		break;
>  	case DVB_DEVICE_DEMUX:
> -		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
> +		dvbdev->entity->type = MEDIA_ENT_T_DTV_DEMUX;
>  		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>  		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>  		break;
>  	case DVB_DEVICE_DVR:
> -		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DVR;
> +		dvbdev->entity->type = MEDIA_ENT_T_DTV_DVR;
>  		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>  		break;
>  	case DVB_DEVICE_CA:
> -		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
> +		dvbdev->entity->type = MEDIA_ENT_T_DTV_CA;
>  		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
>  		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
>  		break;
>  	case DVB_DEVICE_NET:
> -		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_NET;
> +		dvbdev->entity->type = MEDIA_ENT_T_DTV_NET;
>  		break;
>  	default:
>  		kfree(dvbdev->entity);
> @@ -396,16 +396,16 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
>  		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
>  			tuner = entity;
>  			break;
> -		case MEDIA_ENT_T_DEVNODE_DVB_FE:
> +		case MEDIA_ENT_T_DTV_DEMOD:
>  			fe = entity;
>  			break;
> -		case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
> +		case MEDIA_ENT_T_DTV_DEMUX:
>  			demux = entity;
>  			break;
> -		case MEDIA_ENT_T_DEVNODE_DVB_DVR:
> +		case MEDIA_ENT_T_DTV_DVR:
>  			dvr = entity;
>  			break;
> -		case MEDIA_ENT_T_DEVNODE_DVB_CA:
> +		case MEDIA_ENT_T_DTV_CA:
>  			ca = entity;
>  			break;
>  		}
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 2e465ba087ba..0de9912411c5 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -45,11 +45,11 @@ struct media_device_info {
>  /* Used values for media_entity_desc::type */
>  
>  #define MEDIA_ENT_T_AV_DMA		(((1 << 16)) + 1)
> -#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_AV_DMA + 3)
> -#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_AV_DMA + 4)
> -#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_AV_DMA + 5)
> -#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_AV_DMA + 6)
> -#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_AV_DMA + 7)
> +#define MEDIA_ENT_T_DTV_DEMOD	(MEDIA_ENT_T_AV_DMA + 3)
> +#define MEDIA_ENT_T_DTV_DEMUX	(MEDIA_ENT_T_AV_DMA + 4)
> +#define MEDIA_ENT_T_DTV_DVR	(MEDIA_ENT_T_AV_DMA + 5)
> +#define MEDIA_ENT_T_DTV_CA	(MEDIA_ENT_T_AV_DMA + 6)
> +#define MEDIA_ENT_T_DTV_NET	(MEDIA_ENT_T_AV_DMA + 7)
>  
>  #define MEDIA_ENT_T_CAM_SENSOR	((2 << 16) + 1)
>  #define MEDIA_ENT_T_CAM_FLASH	(MEDIA_ENT_T_CAM_SENSOR + 1)
> @@ -76,7 +76,13 @@ struct media_device_info {
>  #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
>  #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
>  
> -#define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
> +#define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DTV_DEMOD
> +#define MEDIA_ENT_T_DEVNODE_DVB_FE	MEDIA_ENT_T_DTV_DEMOD
> +#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	MEDIA_ENT_T_DTV_DEMUX
> +#define MEDIA_ENT_T_DEVNODE_DVB_DVR	MEDIA_ENT_T_DTV_DVR
> +#define MEDIA_ENT_T_DEVNODE_DVB_CA	MEDIA_ENT_T_DTV_CA
> +#define MEDIA_ENT_T_DEVNODE_DVB_NET	MEDIA_ENT_T_DTV_NET
> +
>  #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	MEDIA_ENT_T_CAM_SENSOR
>  #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	MEDIA_ENT_T_CAM_FLASH
>  #define MEDIA_ENT_T_V4L2_SUBDEV_LENS	MEDIA_ENT_T_CAM_LENS
> 

^ permalink raw reply

* Re: [PATCH 06/18] media controller: rename analog TV decoder
From: Hans Verkuil @ 2015-05-08 12:12 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linux Media Mailing List
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, Lars-Peter Clausen,
	Prabhakar Lad, Sakari Ailus, Joe Perches, Boris BREZILLON,
	Laurent Pinchart, Guennadi Liakhovetski, Axel Lin,
	Matthias Schwarzott, Antti Palosaari, Olli Salonen,
	Peter Senna Tschudin, Ramakrishnan Muthukrishnan, linux-doc,
	linux-api
In-Reply-To: <404817c5796244fe30bb29c883e7e9e07cf8e06c.1431046915.git.mchehab@osg.samsung.com>

On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> To keep coherency, let's also remove V4L2_SUBDEV from the analog
> TV decoder, calling it by its function, and not by the V4L2
> API mapping.

Same issue as with patch 04/18.

	Hans

> 
> So,
> 
> 	MEDIA_ENT_T_V4L2_SUBDEV_DECODER -> MEDIA_ENT_T_ATV_DECODER
> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> index 27082b07f4c2..9b3861058f0d 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> @@ -231,7 +231,7 @@
>  	    <entry>Lens controller</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV_DECODER</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_ATV_DECODER</constant></entry>
>  	    <entry>Video decoder, the basic function of the video decoder is to
>  	    accept analogue video from a wide variety of sources such as
>  	    broadcast, DVD players, cameras and video cassette recorders, in
> diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
> index a493c0b0b5fe..e8f0b53cc253 100644
> --- a/drivers/media/i2c/adv7180.c
> +++ b/drivers/media/i2c/adv7180.c
> @@ -1212,7 +1212,7 @@ static int adv7180_probe(struct i2c_client *client,
>  		goto err_unregister_vpp_client;
>  
>  	state->pad.flags = MEDIA_PAD_FL_SOURCE;
> -	sd->entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
> +	sd->entity.flags |= MEDIA_ENT_T_ATV_DECODER;
>  	ret = media_entity_init(&sd->entity, 1, &state->pad, 0);
>  	if (ret)
>  		goto err_free_ctrl;
> diff --git a/drivers/media/i2c/cx25840/cx25840-core.c b/drivers/media/i2c/cx25840/cx25840-core.c
> index e15a789ad596..cd8a3c273ab8 100644
> --- a/drivers/media/i2c/cx25840/cx25840-core.c
> +++ b/drivers/media/i2c/cx25840/cx25840-core.c
> @@ -5208,7 +5208,7 @@ static int cx25840_probe(struct i2c_client *client,
>  	state->pads[CX25840_PAD_INPUT].flags = MEDIA_PAD_FL_SINK;
>  	state->pads[CX25840_PAD_VID_OUT].flags = MEDIA_PAD_FL_SOURCE;
>  	state->pads[CX25840_PAD_VBI_OUT].flags = MEDIA_PAD_FL_SOURCE;
> -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
> +	sd->entity.type = MEDIA_ENT_T_ATV_DECODER;
>  
>  	ret = media_entity_init(&sd->entity, ARRAY_SIZE(state->pads),
>  				state->pads, 0);
> diff --git a/drivers/media/i2c/tvp514x.c b/drivers/media/i2c/tvp514x.c
> index 24e47279e30c..77744c390941 100644
> --- a/drivers/media/i2c/tvp514x.c
> +++ b/drivers/media/i2c/tvp514x.c
> @@ -1106,7 +1106,7 @@ tvp514x_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>  	decoder->pad.flags = MEDIA_PAD_FL_SOURCE;
>  	decoder->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> -	decoder->sd.entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
> +	decoder->sd.entity.flags |= MEDIA_ENT_T_ATV_DECODER;
>  
>  	ret = media_entity_init(&decoder->sd.entity, 1, &decoder->pad, 0);
>  	if (ret < 0) {
> diff --git a/drivers/media/i2c/tvp7002.c b/drivers/media/i2c/tvp7002.c
> index 05077cffd235..3facef49aef1 100644
> --- a/drivers/media/i2c/tvp7002.c
> +++ b/drivers/media/i2c/tvp7002.c
> @@ -1019,7 +1019,7 @@ static int tvp7002_probe(struct i2c_client *c, const struct i2c_device_id *id)
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>  	device->pad.flags = MEDIA_PAD_FL_SOURCE;
>  	device->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> -	device->sd.entity.flags |= MEDIA_ENT_T_V4L2_SUBDEV_DECODER;
> +	device->sd.entity.flags |= MEDIA_ENT_T_ATV_DECODER;
>  
>  	error = media_entity_init(&device->sd.entity, 1, &device->pad, 0);
>  	if (error < 0)
> diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
> index fe00da105e77..a756f74f0adc 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-cards.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
> @@ -1216,7 +1216,7 @@ static void cx231xx_create_media_graph(struct cx231xx *dev)
>  		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
>  			tuner = entity;
>  			break;
> -		case MEDIA_ENT_T_V4L2_SUBDEV_DECODER:
> +		case MEDIA_ENT_T_ATV_DECODER:
>  			decoder = entity;
>  			break;
>  		}
> diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
> index af44f2d1c0a1..bed4ee28916d 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-video.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-video.c
> @@ -119,7 +119,7 @@ static int cx231xx_enable_analog_tuner(struct cx231xx *dev)
>  	 * this should be enough for the actual needs.
>  	 */
>  	media_device_for_each_entity(entity, mdev) {
> -		if (entity->type == MEDIA_ENT_T_V4L2_SUBDEV_DECODER) {
> +		if (entity->type == MEDIA_ENT_T_ATV_DECODER) {
>  			decoder = entity;
>  			break;
>  		}
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 0de9912411c5..9b3d80e765f0 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -54,8 +54,8 @@ struct media_device_info {
>  #define MEDIA_ENT_T_CAM_SENSOR	((2 << 16) + 1)
>  #define MEDIA_ENT_T_CAM_FLASH	(MEDIA_ENT_T_CAM_SENSOR + 1)
>  #define MEDIA_ENT_T_CAM_LENS	(MEDIA_ENT_T_CAM_SENSOR + 2)
> -/* A converter of analogue video to its digital representation. */
> -#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_CAM_SENSOR + 3)
> +
> +#define MEDIA_ENT_T_ATV_DECODER	(MEDIA_ENT_T_CAM_SENSOR + 3)
>  
>  #define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_CAM_SENSOR + 4)
>  
> @@ -86,6 +86,8 @@ struct media_device_info {
>  #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	MEDIA_ENT_T_CAM_SENSOR
>  #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	MEDIA_ENT_T_CAM_FLASH
>  #define MEDIA_ENT_T_V4L2_SUBDEV_LENS	MEDIA_ENT_T_CAM_LENS
> +
> +#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER MEDIA_ENT_T_ATV_DECODER
>  #endif
>  
>  /* Used bitmasks for media_entity_desc::flags */
> 


^ permalink raw reply

* Re: [PATCH 07/18] media controller: rename the tuner entity
From: Hans Verkuil @ 2015-05-08 12:13 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Linux Media Mailing List
  Cc: Mauro Carvalho Chehab, Jonathan Corbet, Matthias Schwarzott,
	Antti Palosaari, Olli Salonen, Prabhakar Lad, Sakari Ailus,
	Laurent Pinchart, linux-doc, linux-api
In-Reply-To: <6d88ece22cbbbaa72bbddb8b152b0d62728d6129.1431046915.git.mchehab@osg.samsung.com>

On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> Finally, let's rename the tuner entity. inside the media subsystem,
> a tuner can be used by AM/FM radio, SDR radio, analog TV and digital TV.
> It could even be used on other subsystems, like network, for wireless
> devices.
> 
> So, it is not constricted to V4L2 API, or to a subdev.
> 
> Let's then rename it as:
> 	MEDIA_ENT_T_V4L2_SUBDEV_TUNER -> MEDIA_ENT_T_TUNER

See patch 04/18.

	Hans

> 
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> 
> diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> index 9b3861058f0d..5c7f366bb1f4 100644
> --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> @@ -241,7 +241,7 @@
>  	    signals.</entry>
>  	  </row>
>  	  <row>
> -	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV_TUNER</constant></entry>
> +	    <entry><constant>MEDIA_ENT_T_TUNER</constant></entry>
>  	    <entry>TV and/or radio tuner</entry>
>  	  </row>
>  	</tbody>
> diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> index 39846077045e..d6a096495035 100644
> --- a/drivers/media/dvb-core/dvbdev.c
> +++ b/drivers/media/dvb-core/dvbdev.c
> @@ -393,7 +393,7 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
>  
>  	media_device_for_each_entity(entity, mdev) {
>  		switch (entity->type) {
> -		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
> +		case MEDIA_ENT_T_TUNER:
>  			tuner = entity;
>  			break;
>  		case MEDIA_ENT_T_DTV_DEMOD:
> diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
> index a756f74f0adc..2a7331e3c4a0 100644
> --- a/drivers/media/usb/cx231xx/cx231xx-cards.c
> +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
> @@ -1213,7 +1213,7 @@ static void cx231xx_create_media_graph(struct cx231xx *dev)
>  
>  	media_device_for_each_entity(entity, mdev) {
>  		switch (entity->type) {
> -		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
> +		case MEDIA_ENT_T_TUNER:
>  			tuner = entity;
>  			break;
>  		case MEDIA_ENT_T_ATV_DECODER:
> diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
> index abdcffabcb59..ecf4e8a543b3 100644
> --- a/drivers/media/v4l2-core/tuner-core.c
> +++ b/drivers/media/v4l2-core/tuner-core.c
> @@ -696,7 +696,7 @@ static int tuner_probe(struct i2c_client *client,
>  register_client:
>  #if defined(CONFIG_MEDIA_CONTROLLER)
>  	t->pad.flags = MEDIA_PAD_FL_SOURCE;
> -	t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
> +	t->sd.entity.type = MEDIA_ENT_T_TUNER;
>  	t->sd.entity.name = t->name;
>  
>  	ret = media_entity_init(&t->sd.entity, 1, &t->pad, 0);
> diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> index 9b3d80e765f0..6acc4be1378c 100644
> --- a/include/uapi/linux/media.h
> +++ b/include/uapi/linux/media.h
> @@ -57,7 +57,7 @@ struct media_device_info {
>  
>  #define MEDIA_ENT_T_ATV_DECODER	(MEDIA_ENT_T_CAM_SENSOR + 3)
>  
> -#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_CAM_SENSOR + 4)
> +#define MEDIA_ENT_T_TUNER	(MEDIA_ENT_T_CAM_SENSOR + 4)
>  
>  #if 1
>  /*
> @@ -88,6 +88,8 @@ struct media_device_info {
>  #define MEDIA_ENT_T_V4L2_SUBDEV_LENS	MEDIA_ENT_T_CAM_LENS
>  
>  #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER MEDIA_ENT_T_ATV_DECODER
> +
> +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	MEDIA_ENT_T_TUNER
>  #endif
>  
>  /* Used bitmasks for media_entity_desc::flags */
> 


^ permalink raw reply

* Re: [PATCH 03/18] media controller: use MEDIA_ENT_T_AV_DMA for A/V DMA engines
From: Mauro Carvalho Chehab @ 2015-05-08 12:32 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Jonathan Corbet,
	Hyun Kwon, Laurent Pinchart, Michal Simek, Sören Brinkmann,
	Sakari Ailus, Sylwester Nawrocki, Ramakrishnan Muthukrishnan,
	Markus Elfring, linux-doc, linux-arm-kernel, linux-api
In-Reply-To: <554CA3DF.9030700@xs4all.nl>

Em Fri, 08 May 2015 13:54:07 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> > At the Video4Linux API, the /dev/video?, /dev/vbi? and
> > /dev/radio? device nodes are used for the chipset that

I knew that this patch would cause some discussions ;)

> 
> You should add /dev/swradio? for SDR devices.

I will.

> 
> > provides the bridge between video/radio streams and the
> > USB, PCI or CPU buses.
> > 
> > Such bridge is also typically used to control the V4L2 device
> > as a hole.
> 
> hole -> whole

Ok.

> > 
> > For video streaming devices and SDR radio devices, they're
> > also associated with the DMA engines that transfer the
> > video stream (or SDR stream) to the CPU's memory.
> > 
> > It should be noticed, however, this is not true on non-SDR
> > radio devices,
> 
> I think you forgot that SDR devices are not using /dev/radio
> but /dev/swradio.

Yeah, true. I forgot that, in the end of the day, we used a
different naming for SDR radio devnodes. I'll fix the comments.

> They have different names. Radio devices never
> stream (OK, I think there are one or two exceptions, but that's really
> because nobody bothered to make an alsa driver. Those boards are
> in practice out of spec.)

Yes, I know.

> > and may also not be true on embedded devices
> > that, due to DRM reasons, don't allow writing unencrypted
> > data on a memory that could be seen by the CPU.
> 
> This actually might still work by using opaque DMABUF handles. But that's
> under discussion right now in the Secure Data Path thread.

Well, a DMABUF opaque handler like that actually refers to either a
buffer that is shared only between 2 devices or to a device-to-device
DMA transfer.

Such dataflow is different than the usual meaning of the video devnode,
where the devnode is used to do I/O transfers. So, it may actually 
be mapped as a different type of entity.

We'll need to discuss further when we start mapping this via MC.

> Another reason can also be that the SoC vendor re-invented the wheel
> and made there own DMA streaming solution. You can still make V4L drivers
> that control the video receivers/transmitters, but for the actual streaming
> you are forced to use the vendor's crap code (hello TI!).
> 
> I've bitter experiences with that :-(
>  
> > So, we'll eventually need to add another entity for such
> > bridge chipsets that have a video/vbi/radio device node
> > associated, but don't have DMA engines on (some) devnodes.
> > 
> > As, currently, we don't have any such case,
> 
> ??? Radio devices are exactly that.

I actually meant to say:

	"As, currently, no driver uses Media Controller on such cases,"

> > let's for now
> > just rename the device nodes that are associated with a
> > DMA engine as MEDIA_ENT_T_AV_DMA.
> > 
> > So,
> > 	MEDIA_ENT_T_DEVNODE_V4L -> MEDIA_ENT_T_AV_DMA
> > 
> > PS.: This is not actually true for USB devices, as the
> > DMA engine is an internal component, as it is up to the
> > Kernel to strip the stream payload from the URB packages.
> 
> How about MEDIA_ENT_T_DATA_STREAMING? Or perhaps DATA_IO? Perhaps even just
> "IO"?

Almost entities do I/O and data streaming (exceptions are flash controller,
SEC and similar control subdevices). So, DATA_STREAMING, DATA_IO or IO
are a way too generic.

DMA is a little bit more restrictive than we wanted.

Actually, I originally named those as MEDIA_ENT_T_AV_BRIDGE, because
the hardware component that implements the device->CPU I/O is typically
a bridge. But then I went into the drivers, and I noticed that several
devices with just one bridge have several different entities for I/O.

So, I ran this script:
	$ git filter-branch -f --msg-filter 'cat && sed s,MEDIA_ENT_T_AV_BRIDGE,MEDIA_ENT_T_AV_DMA,g' origin/patchwork..
	$ git filter-branch -f --tree-filter 'for i in $(git grep -l MEDIA_ENT_T_AV_BRIDGE); do sed s,MEDIA_ENT_T_AV_BRIDGE,MEDIA_ENT_T_AV_DMA,g $i >a && mv a $i; done' origin/patchwork..

To replace the name everywere. Provided that we decide a better name,
this can be easily fixable by doing the above scripting.

Perhaps MEDIA_ENT_T_DEV_CPU_AV_IO would be a better name?

> That would cover USB as well, and I dislike the use of "AV", since the
> data might contain other things besides audio and/or video.

True, but how to distinguish such device from an ALSA DEV/CPU IO?

Answering myself, I see one alternative for that... we could use
MEDIA_ENT_T_DEV_CPU_IO for all device->CPU I/O devices, and use
properties to tell what APIs are valid on such entity. The bad thing
is that someone could add multiple "incompatible" APIs at the same
device (like ALSA, V4L and DVB - all on the dame sevnode).

I'm running out of ideas here ;)

In the lack of a better name, I would keep MEDIA_ENT_T_AV_DMA, as
it is the closest one to what's provided by such entities (or the
less wrong one).


Regards,
Mauro

^ permalink raw reply

* Re: [PATCH 04/18] media controller: Rename camera entities
From: Mauro Carvalho Chehab @ 2015-05-08 12:53 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Jonathan Corbet,
	Sakari Ailus, Laurent Pinchart, Kyungmin Park, Heungjun Kim,
	Prabhakar Lad, Andrzej Hajda, Sylwester Nawrocki, Boris BREZILLON,
	linux-doc, linux-api
In-Reply-To: <554CA5F5.1040101@xs4all.nl>

Em Fri, 08 May 2015 14:03:01 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> > As explained before, the hole idea of subtypes at entities was
> 
> hole -> whole
> 
> > not nice. All V4L2 subdevs may have a device node associated.
> > 
> > Also, the hole idea is to expose hardware IP blocks, so calling
> > them as V4L2 is a very bad practice, as they were not designed
> > for the V4L2 API. It is just the reverse.
> > 
> > So, instead of using V4L2_SUBDEV, let's call the camera sub-
> > devices with CAM, instead:
> > 
> > 	MEDIA_ENT_T_V4L2_SUBDEV_SENSOR -> MEDIA_ENT_T_CAM_SENSOR
> > 	MEDIA_ENT_T_V4L2_SUBDEV_FLASH  -> MEDIA_ENT_T_CAM_FLASH
> > 	MEDIA_ENT_T_V4L2_SUBDEV_LENS   -> MEDIA_ENT_T_CAM_LENS
> 
> I would actually postpone this until Laurent has a properties API ready.
> These entity types are fatally flawed since an entity can combine functions
> in one. E.g. an i2c device (generally represented as a single entity) might
> provide for both sensor and flash. Or combine tuner and video decoder, etc.

Mapping one I2C address as one entity is plain wrong.

It doesn't matter if a piece of hardware has just one I2C address or
multiple ones. There are tuners that have multiple I2C addresses. This is
actually common on some RDS tuners: they have one address for control, and
another one to transfer data from the tuned station. Still, it is just one
tuner.

There are also cases where there's no I2C address at all, but still
there are multiple entities.

Just enumerating a few examples of drivers I touched recently:

- au8522 - the same I2C address controls both V4L2 analod demod
	   and DVB demod;
- as102 - it is just a single chip, with tuner, demod and bridge on it.
	  no I2C buses internally at all.
- cx231xx - It has one internal I2C bus used to control different functions
	 inside the bridge chipset.

etc.


So, if a single piece of hardware has the functions of two entities
(sensor and flash), it should be represented as two separate entities.

The I2C bus would matter if we were mapping the control plane of the
entities, adding PADs for the control lines. But last time I checked,
Laurent was still strongly opposed to that.

> Basically an entity like this is a sub-device (as in the literal meaning
> of being a part of a larger device) that has one or more functions and may
> have device node(s) associated with it. That is best expressed as properties.
> 
> And you really do have to tell userspace that these entities expose a
> v4l-subdev device node. Renaming them doesn't make that go away.

Well, we should decide if we want the namespace and the entities
representing the hardware or representing the Linux API.
V4L2_SUBDEV has nothing to do with hardware. It is just an abstraction
that we've created on one of our subsystems.

> 
> Regards,
> 
> 	Hans
> 
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > index 5b8147629159..759604e3529f 100644
> > --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > @@ -219,15 +219,15 @@
> >  	    <entry>Unknown V4L sub-device</entry>
> >  	  </row>
> >  	  <row>
> > -	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV_SENSOR</constant></entry>
> > +	    <entry><constant>MEDIA_ENT_T_CAM_SENSOR</constant></entry>
> >  	    <entry>Video sensor</entry>
> >  	  </row>
> >  	  <row>
> > -	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV_FLASH</constant></entry>
> > +	    <entry><constant>MEDIA_ENT_T_CAM_FLASH</constant></entry>
> >  	    <entry>Flash controller</entry>
> >  	  </row>
> >  	  <row>
> > -	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV_LENS</constant></entry>
> > +	    <entry><constant>MEDIA_ENT_T_CAM_LENS</constant></entry>
> >  	    <entry>Lens controller</entry>
> >  	  </row>
> >  	  <row>
> > diff --git a/drivers/media/i2c/adp1653.c b/drivers/media/i2c/adp1653.c
> > index c70ababce954..c12f873a8e26 100644
> > --- a/drivers/media/i2c/adp1653.c
> > +++ b/drivers/media/i2c/adp1653.c
> > @@ -516,7 +516,7 @@ static int adp1653_probe(struct i2c_client *client,
> >  	if (ret < 0)
> >  		goto free_and_quit;
> >  
> > -	flash->subdev.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> > +	flash->subdev.entity.type = MEDIA_ENT_T_CAM_FLASH;
> >  
> >  	return 0;
> >  
> > diff --git a/drivers/media/i2c/as3645a.c b/drivers/media/i2c/as3645a.c
> > index 301084b07887..9a2872be11b0 100644
> > --- a/drivers/media/i2c/as3645a.c
> > +++ b/drivers/media/i2c/as3645a.c
> > @@ -831,7 +831,7 @@ static int as3645a_probe(struct i2c_client *client,
> >  	if (ret < 0)
> >  		goto done;
> >  
> > -	flash->subdev.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> > +	flash->subdev.entity.type = MEDIA_ENT_T_CAM_FLASH;
> >  
> >  	mutex_init(&flash->power_lock);
> >  
> > diff --git a/drivers/media/i2c/lm3560.c b/drivers/media/i2c/lm3560.c
> > index d9ece4b2d047..4d28cda77f4d 100644
> > --- a/drivers/media/i2c/lm3560.c
> > +++ b/drivers/media/i2c/lm3560.c
> > @@ -368,7 +368,7 @@ static int lm3560_subdev_init(struct lm3560_flash *flash,
> >  	rval = media_entity_init(&flash->subdev_led[led_no].entity, 0, NULL, 0);
> >  	if (rval < 0)
> >  		goto err_out;
> > -	flash->subdev_led[led_no].entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> > +	flash->subdev_led[led_no].entity.type = MEDIA_ENT_T_CAM_FLASH;
> >  
> >  	return rval;
> >  
> > diff --git a/drivers/media/i2c/lm3646.c b/drivers/media/i2c/lm3646.c
> > index 626fb4679c02..19ee2ac00be7 100644
> > --- a/drivers/media/i2c/lm3646.c
> > +++ b/drivers/media/i2c/lm3646.c
> > @@ -285,7 +285,7 @@ static int lm3646_subdev_init(struct lm3646_flash *flash)
> >  	rval = media_entity_init(&flash->subdev_led.entity, 0, NULL, 0);
> >  	if (rval < 0)
> >  		goto err_out;
> > -	flash->subdev_led.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_FLASH;
> > +	flash->subdev_led.entity.type = MEDIA_ENT_T_CAM_FLASH;
> >  	return rval;
> >  
> >  err_out:
> > diff --git a/drivers/media/i2c/m5mols/m5mols_core.c b/drivers/media/i2c/m5mols/m5mols_core.c
> > index 6404c0d93e7a..beb519cf8be4 100644
> > --- a/drivers/media/i2c/m5mols/m5mols_core.c
> > +++ b/drivers/media/i2c/m5mols/m5mols_core.c
> > @@ -978,7 +978,7 @@ static int m5mols_probe(struct i2c_client *client,
> >  	ret = media_entity_init(&sd->entity, 1, &info->pad, 0);
> >  	if (ret < 0)
> >  		return ret;
> > -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> > +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
> >  
> >  	init_waitqueue_head(&info->irq_waitq);
> >  	mutex_init(&info->lock);
> > diff --git a/drivers/media/i2c/noon010pc30.c b/drivers/media/i2c/noon010pc30.c
> > index f197b6cbd407..a03556197405 100644
> > --- a/drivers/media/i2c/noon010pc30.c
> > +++ b/drivers/media/i2c/noon010pc30.c
> > @@ -779,7 +779,7 @@ static int noon010_probe(struct i2c_client *client,
> >  		goto np_err;
> >  
> >  	info->pad.flags = MEDIA_PAD_FL_SOURCE;
> > -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> > +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
> >  	ret = media_entity_init(&sd->entity, 1, &info->pad, 0);
> >  	if (ret < 0)
> >  		goto np_err;
> > diff --git a/drivers/media/i2c/ov2659.c b/drivers/media/i2c/ov2659.c
> > index d700a1d0a6f2..63b9464e813f 100644
> > --- a/drivers/media/i2c/ov2659.c
> > +++ b/drivers/media/i2c/ov2659.c
> > @@ -1425,7 +1425,7 @@ static int ov2659_probe(struct i2c_client *client,
> >  
> >  #if defined(CONFIG_MEDIA_CONTROLLER)
> >  	ov2659->pad.flags = MEDIA_PAD_FL_SOURCE;
> > -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> > +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
> >  	ret = media_entity_init(&sd->entity, 1, &ov2659->pad, 0);
> >  	if (ret < 0) {
> >  		v4l2_ctrl_handler_free(&ov2659->ctrls);
> > diff --git a/drivers/media/i2c/ov9650.c b/drivers/media/i2c/ov9650.c
> > index 2bc473385c91..ed3c0573a0f8 100644
> > --- a/drivers/media/i2c/ov9650.c
> > +++ b/drivers/media/i2c/ov9650.c
> > @@ -1500,7 +1500,7 @@ static int ov965x_probe(struct i2c_client *client,
> >  		return ret;
> >  
> >  	ov965x->pad.flags = MEDIA_PAD_FL_SOURCE;
> > -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> > +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
> >  	ret = media_entity_init(&sd->entity, 1, &ov965x->pad, 0);
> >  	if (ret < 0)
> >  		return ret;
> > diff --git a/drivers/media/i2c/s5k4ecgx.c b/drivers/media/i2c/s5k4ecgx.c
> > index 97084237275d..23af7f90678a 100644
> > --- a/drivers/media/i2c/s5k4ecgx.c
> > +++ b/drivers/media/i2c/s5k4ecgx.c
> > @@ -961,7 +961,7 @@ static int s5k4ecgx_probe(struct i2c_client *client,
> >  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> >  
> >  	priv->pad.flags = MEDIA_PAD_FL_SOURCE;
> > -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> > +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
> >  	ret = media_entity_init(&sd->entity, 1, &priv->pad, 0);
> >  	if (ret)
> >  		return ret;
> > diff --git a/drivers/media/i2c/s5k5baf.c b/drivers/media/i2c/s5k5baf.c
> > index bee73de347dc..fadd48d35a55 100644
> > --- a/drivers/media/i2c/s5k5baf.c
> > +++ b/drivers/media/i2c/s5k5baf.c
> > @@ -408,7 +408,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl)
> >  
> >  static inline bool s5k5baf_is_cis_subdev(struct v4l2_subdev *sd)
> >  {
> > -	return sd->entity.type == MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> > +	return sd->entity.type == MEDIA_ENT_T_CAM_SENSOR;
> >  }
> >  
> >  static inline struct s5k5baf *to_s5k5baf(struct v4l2_subdev *sd)
> > @@ -1904,7 +1904,7 @@ static int s5k5baf_configure_subdevs(struct s5k5baf *state,
> >  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> >  
> >  	state->cis_pad.flags = MEDIA_PAD_FL_SOURCE;
> > -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> > +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
> >  	ret = media_entity_init(&sd->entity, NUM_CIS_PADS, &state->cis_pad, 0);
> >  	if (ret < 0)
> >  		goto err;
> > diff --git a/drivers/media/i2c/s5k6aa.c b/drivers/media/i2c/s5k6aa.c
> > index d0ad6a25bdab..d2390c6e5dbe 100644
> > --- a/drivers/media/i2c/s5k6aa.c
> > +++ b/drivers/media/i2c/s5k6aa.c
> > @@ -1577,7 +1577,7 @@ static int s5k6aa_probe(struct i2c_client *client,
> >  	sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
> >  
> >  	s5k6aa->pad.flags = MEDIA_PAD_FL_SOURCE;
> > -	sd->entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> > +	sd->entity.type = MEDIA_ENT_T_CAM_SENSOR;
> >  	ret = media_entity_init(&sd->entity, 1, &s5k6aa->pad, 0);
> >  	if (ret)
> >  		return ret;
> > diff --git a/drivers/media/i2c/smiapp/smiapp-core.c b/drivers/media/i2c/smiapp/smiapp-core.c
> > index 636ebd6fe5dc..78f2cdd3561b 100644
> > --- a/drivers/media/i2c/smiapp/smiapp-core.c
> > +++ b/drivers/media/i2c/smiapp/smiapp-core.c
> > @@ -2763,7 +2763,7 @@ static int smiapp_init(struct smiapp_sensor *sensor)
> >  
> >  	dev_dbg(&client->dev, "profile %d\n", sensor->minfo.smiapp_profile);
> >  
> > -	sensor->pixel_array->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_SENSOR;
> > +	sensor->pixel_array->sd.entity.type = MEDIA_ENT_T_CAM_SENSOR;
> >  
> >  	/* final steps */
> >  	smiapp_read_frame_fmt(sensor);
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index a7aa2aac9c23..2e465ba087ba 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -51,13 +51,13 @@ struct media_device_info {
> >  #define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_AV_DMA + 6)
> >  #define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_AV_DMA + 7)
> >  
> > -#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	((2 << 16) + 1)
> > -#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 1)
> > -#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 2)
> > +#define MEDIA_ENT_T_CAM_SENSOR	((2 << 16) + 1)
> > +#define MEDIA_ENT_T_CAM_FLASH	(MEDIA_ENT_T_CAM_SENSOR + 1)
> > +#define MEDIA_ENT_T_CAM_LENS	(MEDIA_ENT_T_CAM_SENSOR + 2)
> >  /* A converter of analogue video to its digital representation. */
> > -#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 3)
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_DECODER	(MEDIA_ENT_T_CAM_SENSOR + 3)
> >  
> > -#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_V4L2_SUBDEV_SENSOR + 4)
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_CAM_SENSOR + 4)
> >  
> >  #if 1
> >  /*
> > @@ -76,8 +76,10 @@ struct media_device_info {
> >  #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
> >  #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
> >  
> > -
> >  #define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	MEDIA_ENT_T_CAM_SENSOR
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	MEDIA_ENT_T_CAM_FLASH
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_LENS	MEDIA_ENT_T_CAM_LENS
> >  #endif
> >  
> >  /* Used bitmasks for media_entity_desc::flags */
> > 
> 

^ permalink raw reply

* Re: [PATCH 05/18] media controller: rename MEDIA_ENT_T_DEVNODE_DVB entities
From: Mauro Carvalho Chehab @ 2015-05-08 12:56 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Jonathan Corbet,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <554CA7C8.20505-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>

Em Fri, 08 May 2015 14:10:48 +0200
Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> escreveu:

> On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> > In order to reflect that the entities are actually the hardware
> > (or firmware, or in-kernel software), and are not associated
> > with the DVB API, let's remove DEVNODE_ from the entity names
> > and use DTV (Digital TV) for the entities.
> > 
> > The frontend is an special case: the frontend devnode actually
> > talks directly with the DTV demodulator. It may or may not also
> > talk with the SEC (Satellite Equipment Control) and with the
> > tuner. For the sake of unifying the nomenclature, let's call it
> > as MEDIA_ENT_T_DTV_DEMOD, because this component is always
> > there.
> > 
> > So:
> > 
> > 	MEDIA_ENT_T_DEVNODE_DVB_FE    -> MEDIA_ENT_T_DTV_DEMOD
> > 	MEDIA_ENT_T_DEVNODE_DVB_DEMUX -> MEDIA_ENT_T_DTV_DEMUX
> > 	MEDIA_ENT_T_DEVNODE_DVB_DVR   -> MEDIA_ENT_T_DTV_DVR
> > 	MEDIA_ENT_T_DEVNODE_DVB_CA    -> MEDIA_ENT_T_DTV_CA
> > 	MEDIA_ENT_T_DEVNODE_DVB_NET   -> MEDIA_ENT_T_DTV_NET
> 
> I'm happy with the new names.
> 
> > 
> > PS.: we could actually not keep this define:
> > 	#define MEDIA_ENT_T_DEVNODE_DVB_FE MEDIA_ENT_T_DTV_DEMOD
> > 
> > As MEDIA_ENT_T_DEVNODE_DVB_FE symbol will not arrive any Kernel
> > version (being present only at the 4.1-rc kernels), but keeping
> > it helps to show that the DVB frontend node is actually associated
> > with the DTV demodulator. So, keeping it for now helps to better
> > document. Also, it avoids to break experimental versions of v4l-utils.
> > So, better to remove this only when we remove the remaining legacy
> > stuff.
> 
> I disagree with that. Let's not introduce defines that are not going to
> be used. And v4l-utils is easily fixed.
> 
> Instead of keeping an unused define, why not...

We agree to disagree here ;)

> 
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> > 
> > diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > index 759604e3529f..27082b07f4c2 100644
> > --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > @@ -195,23 +195,23 @@
> >  	    <entry>ALSA card</entry>
> >  	  </row>
> >  	  <row>
> > -	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_FE</constant></entry>
> > +	    <entry><constant>MEDIA_ENT_T_DTV_DEMOD</constant></entry>
> >  	    <entry>DVB frontend devnode</entry>
> 
> ... explain what is going on here? I.e. that this frontend always controls a demod
> and optionally also a tuner and/or SEC.

I'm actually doing just the renames on those initial patches. There are
some adjustments to be done at the documentation, not just that.

So, I opted to do such adjustment on the last patch of this series.
> 
> Regards,
> 
> 	Hans
> 
> >  	  </row>
> >  	  <row>
> > -	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_DEMUX</constant></entry>
> > +	    <entry><constant>MEDIA_ENT_T_DTV_DEMUX</constant></entry>
> >  	    <entry>DVB demux devnode</entry>
> >  	  </row>
> >  	  <row>
> > -	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_DVR</constant></entry>
> > +	    <entry><constant>MEDIA_ENT_T_DTV_DVR</constant></entry>
> >  	    <entry>DVB DVR devnode</entry>
> >  	  </row>
> >  	  <row>
> > -	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_CA</constant></entry>
> > +	    <entry><constant>MEDIA_ENT_T_DTV_CA</constant></entry>
> >  	    <entry>DVB CAM devnode</entry>
> >  	  </row>
> >  	  <row>
> > -	    <entry><constant>MEDIA_ENT_T_DEVNODE_DVB_NET</constant></entry>
> > +	    <entry><constant>MEDIA_ENT_T_DTV_NET</constant></entry>
> >  	    <entry>DVB network devnode</entry>
> >  	  </row>
> >  	  <row>
> > diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> > index 13bb57f0457f..39846077045e 100644
> > --- a/drivers/media/dvb-core/dvbdev.c
> > +++ b/drivers/media/dvb-core/dvbdev.c
> > @@ -221,26 +221,26 @@ static void dvb_register_media_device(struct dvb_device *dvbdev,
> >  
> >  	switch (type) {
> >  	case DVB_DEVICE_FRONTEND:
> > -		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_FE;
> > +		dvbdev->entity->type = MEDIA_ENT_T_DTV_DEMOD;
> >  		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> >  		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> >  		break;
> >  	case DVB_DEVICE_DEMUX:
> > -		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DEMUX;
> > +		dvbdev->entity->type = MEDIA_ENT_T_DTV_DEMUX;
> >  		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> >  		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> >  		break;
> >  	case DVB_DEVICE_DVR:
> > -		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_DVR;
> > +		dvbdev->entity->type = MEDIA_ENT_T_DTV_DVR;
> >  		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> >  		break;
> >  	case DVB_DEVICE_CA:
> > -		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_CA;
> > +		dvbdev->entity->type = MEDIA_ENT_T_DTV_CA;
> >  		dvbdev->pads[0].flags = MEDIA_PAD_FL_SINK;
> >  		dvbdev->pads[1].flags = MEDIA_PAD_FL_SOURCE;
> >  		break;
> >  	case DVB_DEVICE_NET:
> > -		dvbdev->entity->type = MEDIA_ENT_T_DEVNODE_DVB_NET;
> > +		dvbdev->entity->type = MEDIA_ENT_T_DTV_NET;
> >  		break;
> >  	default:
> >  		kfree(dvbdev->entity);
> > @@ -396,16 +396,16 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
> >  		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
> >  			tuner = entity;
> >  			break;
> > -		case MEDIA_ENT_T_DEVNODE_DVB_FE:
> > +		case MEDIA_ENT_T_DTV_DEMOD:
> >  			fe = entity;
> >  			break;
> > -		case MEDIA_ENT_T_DEVNODE_DVB_DEMUX:
> > +		case MEDIA_ENT_T_DTV_DEMUX:
> >  			demux = entity;
> >  			break;
> > -		case MEDIA_ENT_T_DEVNODE_DVB_DVR:
> > +		case MEDIA_ENT_T_DTV_DVR:
> >  			dvr = entity;
> >  			break;
> > -		case MEDIA_ENT_T_DEVNODE_DVB_CA:
> > +		case MEDIA_ENT_T_DTV_CA:
> >  			ca = entity;
> >  			break;
> >  		}
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index 2e465ba087ba..0de9912411c5 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -45,11 +45,11 @@ struct media_device_info {
> >  /* Used values for media_entity_desc::type */
> >  
> >  #define MEDIA_ENT_T_AV_DMA		(((1 << 16)) + 1)
> > -#define MEDIA_ENT_T_DEVNODE_DVB_FE	(MEDIA_ENT_T_AV_DMA + 3)
> > -#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	(MEDIA_ENT_T_AV_DMA + 4)
> > -#define MEDIA_ENT_T_DEVNODE_DVB_DVR	(MEDIA_ENT_T_AV_DMA + 5)
> > -#define MEDIA_ENT_T_DEVNODE_DVB_CA	(MEDIA_ENT_T_AV_DMA + 6)
> > -#define MEDIA_ENT_T_DEVNODE_DVB_NET	(MEDIA_ENT_T_AV_DMA + 7)
> > +#define MEDIA_ENT_T_DTV_DEMOD	(MEDIA_ENT_T_AV_DMA + 3)
> > +#define MEDIA_ENT_T_DTV_DEMUX	(MEDIA_ENT_T_AV_DMA + 4)
> > +#define MEDIA_ENT_T_DTV_DVR	(MEDIA_ENT_T_AV_DMA + 5)
> > +#define MEDIA_ENT_T_DTV_CA	(MEDIA_ENT_T_AV_DMA + 6)
> > +#define MEDIA_ENT_T_DTV_NET	(MEDIA_ENT_T_AV_DMA + 7)
> >  
> >  #define MEDIA_ENT_T_CAM_SENSOR	((2 << 16) + 1)
> >  #define MEDIA_ENT_T_CAM_FLASH	(MEDIA_ENT_T_CAM_SENSOR + 1)
> > @@ -76,7 +76,13 @@ struct media_device_info {
> >  #define MEDIA_ENT_T_DEVNODE_FB		(MEDIA_ENT_T_DEVNODE + 2)
> >  #define MEDIA_ENT_T_DEVNODE_ALSA	(MEDIA_ENT_T_DEVNODE + 3)
> >  
> > -#define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DEVNODE_DVB_FE
> > +#define MEDIA_ENT_T_DEVNODE_DVB		MEDIA_ENT_T_DTV_DEMOD
> > +#define MEDIA_ENT_T_DEVNODE_DVB_FE	MEDIA_ENT_T_DTV_DEMOD
> > +#define MEDIA_ENT_T_DEVNODE_DVB_DEMUX	MEDIA_ENT_T_DTV_DEMUX
> > +#define MEDIA_ENT_T_DEVNODE_DVB_DVR	MEDIA_ENT_T_DTV_DVR
> > +#define MEDIA_ENT_T_DEVNODE_DVB_CA	MEDIA_ENT_T_DTV_CA
> > +#define MEDIA_ENT_T_DEVNODE_DVB_NET	MEDIA_ENT_T_DTV_NET
> > +
> >  #define MEDIA_ENT_T_V4L2_SUBDEV_SENSOR	MEDIA_ENT_T_CAM_SENSOR
> >  #define MEDIA_ENT_T_V4L2_SUBDEV_FLASH	MEDIA_ENT_T_CAM_FLASH
> >  #define MEDIA_ENT_T_V4L2_SUBDEV_LENS	MEDIA_ENT_T_CAM_LENS
> > 
> 

^ permalink raw reply

* Re: [PATCH 03/18] media controller: use MEDIA_ENT_T_AV_DMA for A/V DMA engines
From: Hans Verkuil @ 2015-05-08 12:57 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Jonathan Corbet,
	Hyun Kwon, Laurent Pinchart, Michal Simek, Sören Brinkmann,
	Sakari Ailus, Sylwester Nawrocki, Ramakrishnan Muthukrishnan,
	Markus Elfring, linux-doc, linux-arm-kernel, linux-api
In-Reply-To: <20150508093239.28644ad4@recife.lan>

Hi Mauro,

On 05/08/2015 02:32 PM, Mauro Carvalho Chehab wrote:
>>> and may also not be true on embedded devices
>>> that, due to DRM reasons, don't allow writing unencrypted
>>> data on a memory that could be seen by the CPU.
>>
>> This actually might still work by using opaque DMABUF handles. But that's
>> under discussion right now in the Secure Data Path thread.
> 
> Well, a DMABUF opaque handler like that actually refers to either a
> buffer that is shared only between 2 devices or to a device-to-device
> DMA transfer.
> 
> Such dataflow is different than the usual meaning of the video devnode,
> where the devnode is used to do I/O transfers. So, it may actually 
> be mapped as a different type of entity.
> 
> We'll need to discuss further when we start mapping this via MC.

Yes, this is quite theoretical at the moment.

>>> So, we'll eventually need to add another entity for such
>>> bridge chipsets that have a video/vbi/radio device node
>>> associated, but don't have DMA engines on (some) devnodes.
>>>
>>> As, currently, we don't have any such case,
>>
>> ??? Radio devices are exactly that.
> 
> I actually meant to say:
> 
> 	"As, currently, no driver uses Media Controller on such cases,"

Ah, OK. That makes more sense :-)

> 
>>> let's for now
>>> just rename the device nodes that are associated with a
>>> DMA engine as MEDIA_ENT_T_AV_DMA.
>>>
>>> So,
>>> 	MEDIA_ENT_T_DEVNODE_V4L -> MEDIA_ENT_T_AV_DMA
>>>
>>> PS.: This is not actually true for USB devices, as the
>>> DMA engine is an internal component, as it is up to the
>>> Kernel to strip the stream payload from the URB packages.
>>
>> How about MEDIA_ENT_T_DATA_STREAMING? Or perhaps DATA_IO? Perhaps even just
>> "IO"?
> 
> Almost entities do I/O and data streaming (exceptions are flash controller,
> SEC and similar control subdevices). So, DATA_STREAMING, DATA_IO or IO
> are a way too generic.

For some of our products we have lots of video nodes that just control the
receiver or transmitter (and possibly other related blocks), but leave the
streaming to vendor code where we are forced to use that. So this can be
a lot more common than you might think, although you won't see that appearing
in the kernel.

> 
> DMA is a little bit more restrictive than we wanted.
> 
> Actually, I originally named those as MEDIA_ENT_T_AV_BRIDGE, because
> the hardware component that implements the device->CPU I/O is typically
> a bridge. But then I went into the drivers, and I noticed that several
> devices with just one bridge have several different entities for I/O.
> 
> So, I ran this script:
> 	$ git filter-branch -f --msg-filter 'cat && sed s,MEDIA_ENT_T_AV_BRIDGE,MEDIA_ENT_T_AV_DMA,g' origin/patchwork..
> 	$ git filter-branch -f --tree-filter 'for i in $(git grep -l MEDIA_ENT_T_AV_BRIDGE); do sed s,MEDIA_ENT_T_AV_BRIDGE,MEDIA_ENT_T_AV_DMA,g $i >a && mv a $i; done' origin/patchwork..
> 
> To replace the name everywere. Provided that we decide a better name,
> this can be easily fixable by doing the above scripting.
> 
> Perhaps MEDIA_ENT_T_DEV_CPU_AV_IO would be a better name?
> 
>> That would cover USB as well, and I dislike the use of "AV", since the
>> data might contain other things besides audio and/or video.
> 
> True, but how to distinguish such device from an ALSA DEV/CPU IO?
> 
> Answering myself, I see one alternative for that... we could use
> MEDIA_ENT_T_DEV_CPU_IO for all device->CPU I/O devices, and use
> properties to tell what APIs are valid on such entity. The bad thing
> is that someone could add multiple "incompatible" APIs at the same
> device (like ALSA, V4L and DVB - all on the dame sevnode).
> 
> I'm running out of ideas here ;)
> 
> In the lack of a better name, I would keep MEDIA_ENT_T_AV_DMA, as
> it is the closest one to what's provided by such entities (or the
> less wrong one).

See my reply to patch 10/18: I see whether streaming is supported or not as a
function (or feature) of the entity, just like a subdev entity can be both
a tuner and a video decoder. I.e. these are properties.

I think the naming issue here shows the problem with your approach. Whereas
having a 'streaming' property simplifies this IMHO.

Regards,

	Hans

^ permalink raw reply

* Re: [PATCH 07/18] media controller: rename the tuner entity
From: Mauro Carvalho Chehab @ 2015-05-08 12:57 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Jonathan Corbet,
	Matthias Schwarzott, Antti Palosaari, Olli Salonen, Prabhakar Lad,
	Sakari Ailus, Laurent Pinchart, linux-doc, linux-api
In-Reply-To: <554CA862.8070407@xs4all.nl>

Em Fri, 08 May 2015 14:13:22 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> > Finally, let's rename the tuner entity. inside the media subsystem,
> > a tuner can be used by AM/FM radio, SDR radio, analog TV and digital TV.
> > It could even be used on other subsystems, like network, for wireless
> > devices.
> > 
> > So, it is not constricted to V4L2 API, or to a subdev.
> > 
> > Let's then rename it as:
> > 	MEDIA_ENT_T_V4L2_SUBDEV_TUNER -> MEDIA_ENT_T_TUNER
> 
> See patch 04/18.

Mapping the tuner as a V4L2_SUBDEV is plain wrong. We can't assume
that a tuner will always be mapped via V4L2 subdev API.

> 
> 	Hans
> 
> > 
> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> > 
> > diff --git a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > index 9b3861058f0d..5c7f366bb1f4 100644
> > --- a/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > +++ b/Documentation/DocBook/media/v4l/media-ioc-enum-entities.xml
> > @@ -241,7 +241,7 @@
> >  	    signals.</entry>
> >  	  </row>
> >  	  <row>
> > -	    <entry><constant>MEDIA_ENT_T_V4L2_SUBDEV_TUNER</constant></entry>
> > +	    <entry><constant>MEDIA_ENT_T_TUNER</constant></entry>
> >  	    <entry>TV and/or radio tuner</entry>
> >  	  </row>
> >  	</tbody>
> > diff --git a/drivers/media/dvb-core/dvbdev.c b/drivers/media/dvb-core/dvbdev.c
> > index 39846077045e..d6a096495035 100644
> > --- a/drivers/media/dvb-core/dvbdev.c
> > +++ b/drivers/media/dvb-core/dvbdev.c
> > @@ -393,7 +393,7 @@ void dvb_create_media_graph(struct dvb_adapter *adap)
> >  
> >  	media_device_for_each_entity(entity, mdev) {
> >  		switch (entity->type) {
> > -		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
> > +		case MEDIA_ENT_T_TUNER:
> >  			tuner = entity;
> >  			break;
> >  		case MEDIA_ENT_T_DTV_DEMOD:
> > diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
> > index a756f74f0adc..2a7331e3c4a0 100644
> > --- a/drivers/media/usb/cx231xx/cx231xx-cards.c
> > +++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
> > @@ -1213,7 +1213,7 @@ static void cx231xx_create_media_graph(struct cx231xx *dev)
> >  
> >  	media_device_for_each_entity(entity, mdev) {
> >  		switch (entity->type) {
> > -		case MEDIA_ENT_T_V4L2_SUBDEV_TUNER:
> > +		case MEDIA_ENT_T_TUNER:
> >  			tuner = entity;
> >  			break;
> >  		case MEDIA_ENT_T_ATV_DECODER:
> > diff --git a/drivers/media/v4l2-core/tuner-core.c b/drivers/media/v4l2-core/tuner-core.c
> > index abdcffabcb59..ecf4e8a543b3 100644
> > --- a/drivers/media/v4l2-core/tuner-core.c
> > +++ b/drivers/media/v4l2-core/tuner-core.c
> > @@ -696,7 +696,7 @@ static int tuner_probe(struct i2c_client *client,
> >  register_client:
> >  #if defined(CONFIG_MEDIA_CONTROLLER)
> >  	t->pad.flags = MEDIA_PAD_FL_SOURCE;
> > -	t->sd.entity.type = MEDIA_ENT_T_V4L2_SUBDEV_TUNER;
> > +	t->sd.entity.type = MEDIA_ENT_T_TUNER;
> >  	t->sd.entity.name = t->name;
> >  
> >  	ret = media_entity_init(&t->sd.entity, 1, &t->pad, 0);
> > diff --git a/include/uapi/linux/media.h b/include/uapi/linux/media.h
> > index 9b3d80e765f0..6acc4be1378c 100644
> > --- a/include/uapi/linux/media.h
> > +++ b/include/uapi/linux/media.h
> > @@ -57,7 +57,7 @@ struct media_device_info {
> >  
> >  #define MEDIA_ENT_T_ATV_DECODER	(MEDIA_ENT_T_CAM_SENSOR + 3)
> >  
> > -#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	(MEDIA_ENT_T_CAM_SENSOR + 4)
> > +#define MEDIA_ENT_T_TUNER	(MEDIA_ENT_T_CAM_SENSOR + 4)
> >  
> >  #if 1
> >  /*
> > @@ -88,6 +88,8 @@ struct media_device_info {
> >  #define MEDIA_ENT_T_V4L2_SUBDEV_LENS	MEDIA_ENT_T_CAM_LENS
> >  
> >  #define MEDIA_ENT_T_V4L2_SUBDEV_DECODER MEDIA_ENT_T_ATV_DECODER
> > +
> > +#define MEDIA_ENT_T_V4L2_SUBDEV_TUNER	MEDIA_ENT_T_TUNER
> >  #endif
> >  
> >  /* Used bitmasks for media_entity_desc::flags */
> > 
> 

^ permalink raw reply

* Re: [PATCH v3 01/11] coresight-etm4x: Adding CoreSight ETM4x driver
From: Mathieu Poirier @ 2015-05-08 13:12 UTC (permalink / raw)
  To: Ivan T. Ivanov
  Cc: Greg KH, linux-arm-kernel@lists.infradead.org, linux-api,
	linux-kernel@vger.kernel.org, Kaixu Xia, Chunyan Zhang
In-Reply-To: <1431073649.21307.12.camel@mm-sol.com>

On 8 May 2015 at 02:27, Ivan T. Ivanov <iivanov@mm-sol.com> wrote:
>
> Hi Mathieu,
>
> On Wed, 2015-05-06 at 09:27 -0600, Mathieu Poirier wrote:
>>
>> +static struct amba_id etm4_ids[] = {
>> +       {       /* ETM 4.0 - Juno board */
>> +               .id     = 0x000bb95e,
>> +               .mask   = 0x000fffff,
>> +               .data   = "ETM 4.0",
>> +       },
>> +       { 0, 0},
>> +};
>> +
>>
>
> Any particular reason to remove device ID for the
> Hi6220 board? It happens to be the same ID used in
> Qualcomm 8x16 ETM components.

Efforts to push support for the Hi6220 board upstream have been
abandoned, hence the decision to remove the entry.  Apologies for the
inconvenience, I will add it back.

Mathieu

>
> Regards,
> Ivan

^ permalink raw reply

* Re: [PATCH 04/18] media controller: Rename camera entities
From: Hans Verkuil @ 2015-05-08 13:13 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Jonathan Corbet,
	Sakari Ailus, Laurent Pinchart, Kyungmin Park, Heungjun Kim,
	Prabhakar Lad, Andrzej Hajda, Sylwester Nawrocki, Boris BREZILLON,
	linux-doc, linux-api
In-Reply-To: <20150508095347.3f6e2a5a@recife.lan>

On 05/08/2015 02:53 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 08 May 2015 14:03:01 +0200
> Hans Verkuil <hverkuil@xs4all.nl> escreveu:
> 
>> On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
>>> As explained before, the hole idea of subtypes at entities was
>>
>> hole -> whole
>>
>>> not nice. All V4L2 subdevs may have a device node associated.
>>>
>>> Also, the hole idea is to expose hardware IP blocks, so calling
>>> them as V4L2 is a very bad practice, as they were not designed
>>> for the V4L2 API. It is just the reverse.
>>>
>>> So, instead of using V4L2_SUBDEV, let's call the camera sub-
>>> devices with CAM, instead:
>>>
>>> 	MEDIA_ENT_T_V4L2_SUBDEV_SENSOR -> MEDIA_ENT_T_CAM_SENSOR
>>> 	MEDIA_ENT_T_V4L2_SUBDEV_FLASH  -> MEDIA_ENT_T_CAM_FLASH
>>> 	MEDIA_ENT_T_V4L2_SUBDEV_LENS   -> MEDIA_ENT_T_CAM_LENS
>>
>> I would actually postpone this until Laurent has a properties API ready.
>> These entity types are fatally flawed since an entity can combine functions
>> in one. E.g. an i2c device (generally represented as a single entity) might
>> provide for both sensor and flash. Or combine tuner and video decoder, etc.
> 
> Mapping one I2C address as one entity is plain wrong.

I said 'i2c device', not i2c address. That would definitely be wrong. I'm also
not saying that each i2c device (chip) maps always to one entity, although this
is generally true. In the end it depends on the driver author to decide how
to split up the functionality of the device into entities. There is no hard
and fast rule for that.

> So, if a single piece of hardware has the functions of two entities
> (sensor and flash), it should be represented as two separate entities.

Perhaps, perhaps not. There is in the general case no clear rule at what
level you design your entities. In a complex pipeline it would be madness
to map every little HW block to an entity since you would get a zillion
entities, which is highly impractical. The cx25840 combines multiple
functions (tuner, audio/video decoders), and I am not at all sure you
would gain anything from splitting that up into smaller entities. In the
end, if you would write a block diagram of your board the cx25840 would be
a single block. And experience has shown that that is typically the right
level for deciding what will be an entity or not.

Also, in such devices the various functions are often intertwined and
generally not easy to separate.

> 
> The I2C bus would matter if we were mapping the control plane of the
> entities, adding PADs for the control lines. But last time I checked,
> Laurent was still strongly opposed to that.
> 
>> Basically an entity like this is a sub-device (as in the literal meaning
>> of being a part of a larger device) that has one or more functions and may
>> have device node(s) associated with it. That is best expressed as properties.
>>
>> And you really do have to tell userspace that these entities expose a
>> v4l-subdev device node. Renaming them doesn't make that go away.
> 
> Well, we should decide if we want the namespace and the entities
> representing the hardware or representing the Linux API.
> V4L2_SUBDEV has nothing to do with hardware. It is just an abstraction
> that we've created on one of our subsystems.

I agree. MEDIA_ENT_T_V4L2_SUBDEV_SENSOR basically contains two bits of
information: the linux API used to access the entity and the function.

Since you don't combine multiple APIs (e.g. ALSA and V4L2) for a single
device node (I would certainly never allow such code in the kernel!) there
is only one of those, but one entity can certainly combine multiple functions
as I argued for above. Hence, those should be properties.

Anyway, let's wait what Laurent thinks and setup an irc session for this.

Regards,

	Hans

^ permalink raw reply

* Re: [PATCH v4 3/3] proc: add kpageidle file
From: Vladimir Davydov @ 2015-05-08 13:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Johannes Weiner, Michal Hocko, Greg Thelen,
	Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <6af563e62c4ab5fdfe336410ad5df9d6540267cc.1431002699.git.vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

Oops, this patch is stale, the correct one is here:
---
From: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
Subject: [PATCH] proc: add kpageidle file

Knowing the portion of memory that is not used by a certain application
or memory cgroup (idle memory) can be useful for partitioning the system
efficiently, e.g. by setting memory cgroup limits appropriately.
Currently, the only means to estimate the amount of idle memory provided
by the kernel is /proc/PID/{clear_refs,smaps}: the user can clear the
access bit for all pages mapped to a particular process by writing 1 to
clear_refs, wait for some time, and then count smaps:Referenced.
However, this method has two serious shortcomings:

 - it does not count unmapped file pages
 - it affects the reclaimer logic

To overcome these drawbacks, this patch introduces two new page flags,
Idle and Young, and a new proc file, /proc/kpageidle. A page's Idle flag
can only be set from userspace by setting bit in /proc/kpageidle at the
offset corresponding to the page, and it is cleared whenever the page is
accessed either through page tables (it is cleared in page_referenced()
in this case) or using the read(2) system call (mark_page_accessed()).
Thus by setting the Idle flag for pages of a particular workload, which
can be found e.g. by reading /proc/PID/pagemap, waiting for some time to
let the workload access its working set, and then reading the kpageidle
file, one can estimate the amount of pages that are not used by the
workload.

The Young page flag is used to avoid interference with the memory
reclaimer. A page's Young flag is set whenever the Access bit of a page
table entry pointing to the page is cleared by writing to kpageidle. If
page_referenced() is called on a Young page, it will add 1 to its return
value, therefore concealing the fact that the Access bit was cleared.

Note, since there is no room for extra page flags on 32 bit, this
feature uses extended page flags when compiled on 32 bit.

Signed-off-by: Vladimir Davydov <vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

diff --git a/Documentation/vm/pagemap.txt b/Documentation/vm/pagemap.txt
index a9b7afc8fbc6..c9266340852c 100644
--- a/Documentation/vm/pagemap.txt
+++ b/Documentation/vm/pagemap.txt
@@ -5,7 +5,7 @@ pagemap is a new (as of 2.6.25) set of interfaces in the kernel that allow
 userspace programs to examine the page tables and related information by
 reading files in /proc.
 
-There are four components to pagemap:
+There are five components to pagemap:
 
  * /proc/pid/pagemap.  This file lets a userspace process find out which
    physical frame each virtual page is mapped to.  It contains one 64-bit
@@ -69,6 +69,16 @@ There are four components to pagemap:
    memory cgroup each page is charged to, indexed by PFN. Only available when
    CONFIG_MEMCG is set.
 
+ * /proc/kpageidle.  This file implements a bitmap where each bit corresponds
+   to a page, indexed by PFN. When the bit is set, the corresponding page is
+   idle. A page is considered idle if it has not been accessed since it was
+   marked idle. To mark a page idle one should set the bit corresponding to the
+   page by writing to the file. A value written to the file is OR-ed with the
+   current bitmap value. Only user memory pages can be marked idle, for other
+   page types input is silently ignored. Writing to this file beyond max PFN
+   results in the ENXIO error. Only available when CONFIG_IDLE_PAGE_TRACKING is
+   set.
+
 Short descriptions to the page flags:
 
  0. LOCKED
diff --git a/fs/proc/page.c b/fs/proc/page.c
index 70d23245dd43..5c055a7eee54 100644
--- a/fs/proc/page.c
+++ b/fs/proc/page.c
@@ -16,6 +16,7 @@
 
 #define KPMSIZE sizeof(u64)
 #define KPMMASK (KPMSIZE - 1)
+#define KPMBITS (KPMSIZE * BITS_PER_BYTE)
 
 /* /proc/kpagecount - an array exposing page counts
  *
@@ -275,6 +276,173 @@ static const struct file_operations proc_kpagecgroup_operations = {
 };
 #endif /* CONFIG_MEMCG */
 
+#ifdef CONFIG_IDLE_PAGE_TRACKING
+/*
+ * Idle page tracking only considers user memory pages, for other types of
+ * pages the idle flag is always unset and an attempt to set it is silently
+ * ignored.
+ *
+ * We treat a page as a user memory page if it has the LRU flag set, because it
+ * is always safe to pass such a page to page_referenced(), which is essential
+ * for idle page tracking. With such an indicator of user pages we can skip
+ * isolated pages, but since there are not usually many of them, it should not
+ * affect the overall result.
+ *
+ * This function tries to get a user memory page by pfn as described above.
+ */
+static struct page *kpageidle_get_page(unsigned long pfn)
+{
+	struct page *page;
+
+	if (!pfn_valid(pfn))
+		return NULL;
+	page = pfn_to_page(pfn);
+	if (!page || !PageLRU(page))
+		return NULL;
+	if (!get_page_unless_zero(page))
+		return NULL;
+	if (unlikely(!PageLRU(page))) {
+		put_page(page);
+		return NULL;
+	}
+	return page;
+}
+
+/*
+ * This function calls page_referenced() to clear the referenced bit for all
+ * mappings to a page. Since the latter also clears the page idle flag if the
+ * page was referenced, it can be used to update the idle flag of a page.
+ */
+static void kpageidle_clear_pte_refs(struct page *page)
+{
+	unsigned long dummy;
+
+	if (page_referenced(page, 0, NULL, &dummy))
+		/*
+		 * We cleared the referenced bit in a mapping to this page. To
+		 * avoid interference with the reclaimer, mark it young so that
+		 * the next call to page_referenced() will also return > 0 (see
+		 * page_referenced_one())
+		 */
+		set_page_young(page);
+}
+
+static ssize_t kpageidle_read(struct file *file, char __user *buf,
+			      size_t count, loff_t *ppos)
+{
+	u64 __user *out = (u64 __user *)buf;
+	struct page *page;
+	unsigned long pfn, end_pfn;
+	ssize_t ret = 0;
+	u64 idle_bitmap = 0;
+	int bit;
+
+	if (*ppos & KPMMASK || count & KPMMASK)
+		return -EINVAL;
+
+	pfn = *ppos * BITS_PER_BYTE;
+	if (pfn >= max_pfn)
+		return 0;
+
+	end_pfn = pfn + count * BITS_PER_BYTE;
+	if (end_pfn > max_pfn)
+		end_pfn = ALIGN(max_pfn, KPMBITS);
+
+	for (; pfn < end_pfn; pfn++) {
+		bit = pfn % KPMBITS;
+		page = kpageidle_get_page(pfn);
+		if (page) {
+			if (page_is_idle(page)) {
+				/*
+				 * The page might have been referenced via a
+				 * pte, in which case it is not idle. Clear
+				 * refs and recheck.
+				 */
+				kpageidle_clear_pte_refs(page);
+				if (page_is_idle(page))
+					idle_bitmap |= 1ULL << bit;
+			}
+			put_page(page);
+		}
+		if (bit == KPMBITS - 1) {
+			if (put_user(idle_bitmap, out)) {
+				ret = -EFAULT;
+				break;
+			}
+			idle_bitmap = 0;
+			out++;
+		}
+	}
+
+	*ppos += (char __user *)out - buf;
+	if (!ret)
+		ret = (char __user *)out - buf;
+	return ret;
+}
+
+static ssize_t kpageidle_write(struct file *file, const char __user *buf,
+			       size_t count, loff_t *ppos)
+{
+	const u64 __user *in = (const u64 __user *)buf;
+	struct page *page;
+	unsigned long pfn, end_pfn;
+	ssize_t ret = 0;
+	u64 idle_bitmap = 0;
+	int bit;
+
+	if (*ppos & KPMMASK || count & KPMMASK)
+		return -EINVAL;
+
+	pfn = *ppos * BITS_PER_BYTE;
+	if (pfn >= max_pfn)
+		return -ENXIO;
+
+	end_pfn = pfn + count * BITS_PER_BYTE;
+	if (end_pfn > max_pfn)
+		end_pfn = ALIGN(max_pfn, KPMBITS);
+
+	for (; pfn < end_pfn; pfn++) {
+		bit = pfn % KPMBITS;
+		if (bit == 0) {
+			if (get_user(idle_bitmap, in)) {
+				ret = -EFAULT;
+				break;
+			}
+			in++;
+		}
+		if (idle_bitmap >> bit & 1) {
+			page = kpageidle_get_page(pfn);
+			if (page) {
+				kpageidle_clear_pte_refs(page);
+				set_page_idle(page);
+				put_page(page);
+			}
+		}
+	}
+
+	*ppos += (const char __user *)in - buf;
+	if (!ret)
+		ret = (const char __user *)in - buf;
+	return ret;
+}
+
+static const struct file_operations proc_kpageidle_operations = {
+	.llseek = mem_lseek,
+	.read = kpageidle_read,
+	.write = kpageidle_write,
+};
+
+#ifndef CONFIG_64BIT
+static bool need_page_idle(void)
+{
+	return true;
+}
+struct page_ext_operations page_idle_ops = {
+	.need = need_page_idle,
+};
+#endif
+#endif /* CONFIG_IDLE_PAGE_TRACKING */
+
 static int __init proc_page_init(void)
 {
 	proc_create("kpagecount", S_IRUSR, NULL, &proc_kpagecount_operations);
@@ -282,6 +450,10 @@ static int __init proc_page_init(void)
 #ifdef CONFIG_MEMCG
 	proc_create("kpagecgroup", S_IRUSR, NULL, &proc_kpagecgroup_operations);
 #endif
+#ifdef CONFIG_IDLE_PAGE_TRACKING
+	proc_create("kpageidle", S_IRUSR | S_IWUSR, NULL,
+		    &proc_kpageidle_operations);
+#endif
 	return 0;
 }
 fs_initcall(proc_page_init);
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index 6dee68d013ff..ab04846f7dd5 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -458,7 +458,7 @@ static void smaps_account(struct mem_size_stats *mss, struct page *page,
 
 	mss->resident += size;
 	/* Accumulate the size in pages that have been accessed. */
-	if (young || PageReferenced(page))
+	if (young || page_is_young(page) || PageReferenced(page))
 		mss->referenced += size;
 	mapcount = page_mapcount(page);
 	if (mapcount >= 2) {
@@ -808,6 +808,7 @@ static int clear_refs_pte_range(pmd_t *pmd, unsigned long addr,
 
 		/* Clear accessed and referenced bits. */
 		pmdp_test_and_clear_young(vma, addr, pmd);
+		clear_page_young(page);
 		ClearPageReferenced(page);
 out:
 		spin_unlock(ptl);
@@ -835,6 +836,7 @@ out:
 
 		/* Clear accessed and referenced bits. */
 		ptep_test_and_clear_young(vma, addr, pte);
+		clear_page_young(page);
 		ClearPageReferenced(page);
 	}
 	pte_unmap_unlock(pte - 1, ptl);
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0755b9fd03a7..794d29aa2317 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2200,5 +2200,93 @@ void __init setup_nr_node_ids(void);
 static inline void setup_nr_node_ids(void) {}
 #endif
 
+#ifdef CONFIG_IDLE_PAGE_TRACKING
+#ifdef CONFIG_64BIT
+static inline bool page_is_young(struct page *page)
+{
+	return PageYoung(page);
+}
+
+static inline void set_page_young(struct page *page)
+{
+	SetPageYoung(page);
+}
+
+static inline void clear_page_young(struct page *page)
+{
+	ClearPageYoung(page);
+}
+
+static inline bool page_is_idle(struct page *page)
+{
+	return PageIdle(page);
+}
+
+static inline void set_page_idle(struct page *page)
+{
+	SetPageIdle(page);
+}
+
+static inline void clear_page_idle(struct page *page)
+{
+	ClearPageIdle(page);
+}
+#else /* !CONFIG_64BIT */
+/*
+ * If there is not enough space to store Idle and Young bits in page flags, use
+ * page ext flags instead.
+ */
+extern struct page_ext_operations page_idle_ops;
+
+static inline bool page_is_young(struct page *page)
+{
+	return test_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
+}
+
+static inline void set_page_young(struct page *page)
+{
+	set_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
+}
+
+static inline void clear_page_young(struct page *page)
+{
+	clear_bit(PAGE_EXT_YOUNG, &lookup_page_ext(page)->flags);
+}
+
+static inline bool page_is_idle(struct page *page)
+{
+	return test_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
+}
+
+static inline void set_page_idle(struct page *page)
+{
+	set_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
+}
+
+static inline void clear_page_idle(struct page *page)
+{
+	clear_bit(PAGE_EXT_IDLE, &lookup_page_ext(page)->flags);
+}
+#endif /* CONFIG_64BIT */
+#else /* !CONFIG_IDLE_PAGE_TRACKING */
+static inline bool page_is_young(struct page *page)
+{
+	return false;
+}
+
+static inline void clear_page_young(struct page *page)
+{
+}
+
+static inline bool page_is_idle(struct page *page)
+{
+	return false;
+}
+
+static inline void clear_page_idle(struct page *page)
+{
+}
+#endif /* CONFIG_IDLE_PAGE_TRACKING */
+
 #endif /* __KERNEL__ */
 #endif /* _LINUX_MM_H */
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index f34e040b34e9..5e7c4f50a644 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -109,6 +109,10 @@ enum pageflags {
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	PG_compound_lock,
 #endif
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
+	PG_young,
+	PG_idle,
+#endif
 	__NR_PAGEFLAGS,
 
 	/* Filesystems */
@@ -289,6 +293,11 @@ PAGEFLAG_FALSE(HWPoison)
 #define __PG_HWPOISON 0
 #endif
 
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
+PAGEFLAG(Young, young)
+PAGEFLAG(Idle, idle)
+#endif
+
 /*
  * On an anonymous page mapped into a user virtual memory area,
  * page->mapping points to its anon_vma, not to a struct address_space;
diff --git a/include/linux/page_ext.h b/include/linux/page_ext.h
index c42981cd99aa..17f118a82854 100644
--- a/include/linux/page_ext.h
+++ b/include/linux/page_ext.h
@@ -26,6 +26,10 @@ enum page_ext_flags {
 	PAGE_EXT_DEBUG_POISON,		/* Page is poisoned */
 	PAGE_EXT_DEBUG_GUARD,
 	PAGE_EXT_OWNER,
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
+	PAGE_EXT_YOUNG,
+	PAGE_EXT_IDLE,
+#endif
 };
 
 /*
diff --git a/mm/Kconfig b/mm/Kconfig
index 390214da4546..3600eace4774 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -635,3 +635,15 @@ config MAX_STACK_SIZE_MB
 	  changed to a smaller value in which case that is used.
 
 	  A sane initial value is 80 MB.
+
+config IDLE_PAGE_TRACKING
+	bool "Enable idle page tracking"
+	select PROC_PAGE_MONITOR
+	select PAGE_EXTENSION if !64BIT
+	help
+	  This feature allows to estimate the amount of user pages that have
+	  not been touched during a given period of time. This information can
+	  be useful to tune memory cgroup limits and/or for job placement
+	  within a compute cluster.
+
+	  See Documentation/vm/pagemap.txt for more details.
diff --git a/mm/debug.c b/mm/debug.c
index 3eb3ac2fcee7..bb66f9ccec03 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -48,6 +48,10 @@ static const struct trace_print_flags pageflag_names[] = {
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 	{1UL << PG_compound_lock,	"compound_lock"	},
 #endif
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && defined(CONFIG_64BIT)
+	{1UL << PG_young,		"young"		},
+	{1UL << PG_idle,		"idle"		},
+#endif
 };
 
 static void dump_flags(unsigned long flags,
diff --git a/mm/page_ext.c b/mm/page_ext.c
index d86fd2f5353f..e4b3af054bf2 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -59,6 +59,9 @@ static struct page_ext_operations *page_ext_ops[] = {
 #ifdef CONFIG_PAGE_OWNER
 	&page_owner_ops,
 #endif
+#if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
+	&page_idle_ops,
+#endif
 };
 
 static unsigned long total_usage;
diff --git a/mm/rmap.c b/mm/rmap.c
index 24dd3f9fee27..eca7416f55d7 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -781,6 +781,14 @@ static int page_referenced_one(struct page *page, struct vm_area_struct *vma,
 		pte_unmap_unlock(pte, ptl);
 	}
 
+	if (referenced && page_is_idle(page))
+		clear_page_idle(page);
+
+	if (page_is_young(page)) {
+		clear_page_young(page);
+		referenced++;
+	}
+
 	if (referenced) {
 		pra->referenced++;
 		pra->vm_flags |= vma->vm_flags;
diff --git a/mm/swap.c b/mm/swap.c
index a7251a8ed532..6bf6f293a9ea 100644
--- a/mm/swap.c
+++ b/mm/swap.c
@@ -623,6 +623,8 @@ void mark_page_accessed(struct page *page)
 	} else if (!PageReferenced(page)) {
 		SetPageReferenced(page);
 	}
+	if (page_is_idle(page))
+		clear_page_idle(page);
 }
 EXPORT_SYMBOL(mark_page_accessed);
 

^ permalink raw reply related

* Re: [PATCH 07/18] media controller: rename the tuner entity
From: Hans Verkuil @ 2015-05-08 13:21 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Jonathan Corbet,
	Matthias Schwarzott, Antti Palosaari, Olli Salonen, Prabhakar Lad,
	Sakari Ailus, Laurent Pinchart, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150508095754.1c39a276-+RedX5hVuTR+urZeOPWqwQ@public.gmane.org>

On 05/08/2015 02:57 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 08 May 2015 14:13:22 +0200
> Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> escreveu:
> 
>> On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
>>> Finally, let's rename the tuner entity. inside the media subsystem,
>>> a tuner can be used by AM/FM radio, SDR radio, analog TV and digital TV.
>>> It could even be used on other subsystems, like network, for wireless
>>> devices.
>>>
>>> So, it is not constricted to V4L2 API, or to a subdev.
>>>
>>> Let's then rename it as:
>>> 	MEDIA_ENT_T_V4L2_SUBDEV_TUNER -> MEDIA_ENT_T_TUNER
>>
>> See patch 04/18.
> 
> Mapping the tuner as a V4L2_SUBDEV is plain wrong. We can't assume
> that a tuner will always be mapped via V4L2 subdev API.

True. Today we have subdevs that have no device node to control them, so
in that case it would just be a SUBDEV entity. There are subdevs that make
a v4l-subdev device node, so those can be V4L(2)_SUBDEV entities.

The question is: what are your ideas for e.g. DVB-only tuners? Would they
get a DVB-like device node? (so DTV_SUBDEV) Would hybrid tuners have two
device nodes? One v4l-subdev, one dvb/dtv-subdev?

Just curious what your thoughts are.

Brainstorming:

It might be better to map each device node to an entity and each hardware
component (tuner, DMA engine) to an entity, and avoid this mixing of
hw entity vs device node entity.

Hmm, we need a another brainstorm meeting...

Regards,

	Hans

^ permalink raw reply

* Re: [PATCH v4 0/3] idle memory tracking
From: Vladimir Davydov @ 2015-05-08 13:22 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Minchan Kim, Johannes Weiner, Michal Hocko, Greg Thelen,
	Michel Lespinasse, David Rientjes, Pavel Emelyanov,
	Cyrill Gorcunov, Jonathan Corbet,
	linux-api-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	cgroups-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cover.1431002699.git.vdavydov-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>

On Thu, May 07, 2015 at 05:09:39PM +0300, Vladimir Davydov wrote:
> ---- SCRIPT FOR COUNTING IDLE PAGES PER CGROUP ----

Oops, this script is stale. The correct one is here:
---
#! /usr/bin/python
#

import os
import stat
import errno
import struct

CGROUP_MOUNT = "/sys/fs/cgroup/memory"
BUFSIZE = 8 * 1024  # must be multiple of 8


def set_idle():
    f = open("/proc/kpageidle", "wb", BUFSIZE)
    while True:
        try:
            f.write(struct.pack("Q", pow(2, 64) - 1))
        except IOError as err:
            if err.errno == errno.ENXIO:
                break
            raise
    f.close()


def count_idle():
    f_flags = open("/proc/kpageflags", "rb", BUFSIZE)
    f_cgroup = open("/proc/kpagecgroup", "rb", BUFSIZE)
    f_idle = open("/proc/kpageidle", "rb", BUFSIZE)

    pfn = 0
    nr_idle = {}
    while True:
        s = f_flags.read(8)
        if not s:
            break

        flags, = struct.unpack('Q', s)
        cgino, = struct.unpack('Q', f_cgroup.read(8))

        bit = pfn % 64
        if not bit:
            idle_bitmap, = struct.unpack('Q', f_idle.read(8))

        idle = idle_bitmap >> bit & 1
        pfn += 1

        unevictable = flags >> 18 & 1
        huge = flags >> 22 & 1

        if idle and not unevictable:
            nr_idle[cgino] = nr_idle.get(cgino, 0) + (512 if huge else 1)

    f_flags.close()
    f_cgroup.close()
    f_idle.close()
    return nr_idle


print "Setting the idle flag for each page..."
set_idle()

raw_input("Wait until the workload accesses its working set, then press Enter")

print "Counting idle pages..."
nr_idle = count_idle()

for dir, subdirs, files in os.walk(CGROUP_MOUNT):
    ino = os.stat(dir)[stat.ST_INO]
    print dir + ": " + str(nr_idle.get(ino, 0) * 4) + " KB"

^ permalink raw reply

* Re: [PATCH 01/11] coresight-etm4x: Adding CoreSight ETM4x driver
From: Mathieu Poirier @ 2015-05-08 14:07 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Greg KH,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kaixu Xia,
	Chunyan Zhang
In-Reply-To: <CACRpkdak5-iuWEDAvhDq3uwYobK+tW_N-t-QaY+-5TUSCa7uOg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 28 April 2015 at 06:28, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Apr 23, 2015 at 12:40 AM, Mathieu Poirier
> <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
>> From: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>>
>> This driver manages the CoreSight ETMv4 (Embedded Trace Macrocell) IP block
>> to support HW assisted tracing on ARMv7 and ARMv8 architectures.
>>
>> Signed-off-by: Pratik Patel <pratikp-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Signed-off-by: Kaixu Xia <xiakaixu-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Mathieu Poirier <mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> (...)
>> +/* The number of ETMv4 currently registered */
>> +static int etm4_count;
>> +static struct etmv4_drvdata *etmdrvdata[NR_CPUS];
>
> Don't you need a mutex or something around etm4_count so
> as to protect it? You could also make it an atomic.

Sorry for the late reply, your email got lost under the pile.

The etm4_count is only used at boot time to avoid registering the
notifiers twice and not used otherwise.  Since the discovery of
coresight devices is sequential there is no point in guarding it.

>
> Also why is it a signed int ... unsigned int?
>
> I know, lame comments, the runtime PM stuff looks perfect now,

I like it too - thanks for the review.

> someone had a refcount comment but it's doing the right thing.
>
> Yours,
> Linus Walleij

^ permalink raw reply

* Re: [PATCH 07/18] media controller: rename the tuner entity
From: Mauro Carvalho Chehab @ 2015-05-08 14:08 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Jonathan Corbet,
	Matthias Schwarzott, Antti Palosaari, Olli Salonen, Prabhakar Lad,
	Sakari Ailus, Laurent Pinchart, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <554CB863.1040006-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>

Em Fri, 08 May 2015 15:21:39 +0200
Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> escreveu:

> On 05/08/2015 02:57 PM, Mauro Carvalho Chehab wrote:
> > Em Fri, 08 May 2015 14:13:22 +0200
> > Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> escreveu:
> > 
> >> On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
> >>> Finally, let's rename the tuner entity. inside the media subsystem,
> >>> a tuner can be used by AM/FM radio, SDR radio, analog TV and digital TV.
> >>> It could even be used on other subsystems, like network, for wireless
> >>> devices.
> >>>
> >>> So, it is not constricted to V4L2 API, or to a subdev.
> >>>
> >>> Let's then rename it as:
> >>> 	MEDIA_ENT_T_V4L2_SUBDEV_TUNER -> MEDIA_ENT_T_TUNER
> >>
> >> See patch 04/18.
> > 
> > Mapping the tuner as a V4L2_SUBDEV is plain wrong. We can't assume
> > that a tuner will always be mapped via V4L2 subdev API.
> 
> True. Today we have subdevs that have no device node to control them, so
> in that case it would just be a SUBDEV entity. There are subdevs that make
> a v4l-subdev device node, so those can be V4L(2)_SUBDEV entities.
> 
> The question is: what are your ideas for e.g. DVB-only tuners? Would they
> get a DVB-like device node? (so DTV_SUBDEV)

I guess we may need DVB subdevs in the future. For now, I don't see
much usage.

> Would hybrid tuners have two
> device nodes? One v4l-subdev, one dvb/dtv-subdev?

No. A tuner is a tuner. The very same device can be used for analog or
digital TV. Ok, there are tuners that only work for digital TV (satellite
tuners, typically), because satellite requires a different tuning range,
and require an extra hardware to power up the satellite antena. So, on
most devices, the tuner is integrated with SEC.

In any case, I don't see any reason why artificially one piece of hardware
component (tuner) into one subdevice entity per API.

What it might make sense in the future is to have some DVB-specific ioctls
for a hybrid tuner,  in order to allow adjusting its internal filters to
an specific digital TV standard.

> Just curious what your thoughts are.
> 
> Brainstorming:
> 
> It might be better to map each device node to an entity and each hardware
> component (tuner, DMA engine) to an entity, and avoid this mixing of
> hw entity vs device node entity.

Ok, but then we need to properly define the namespaces for HW and for
Linux API components.

So, we would have a namespace like:
 
	- ENT_T_DEVNODE_DVB_(FE|CA|NET|DEMUX|DVR) for the DVB API device nodes;
	- ENT_T_DEVNODE_V4L for the radio/swradio/video/vbi devnodes, or,
alternatively, use ENT_T_DEVNODE_V4L_(RADIO|SWRADIO|VIDEO|VBI);
	- ENT_T_HW_(TUNER|CAM_SENSOR|ATV_DEMOD|DTV_DEMOD|...) for the
hardware components.

In other words, the namespace would actually define two subtypes:
	- devnodes;
	- hardware components

There's one advantage on this strategy: it is easier to keep backward
compatibility, IMHO, as we'll be preserving 1 << 16 for device nodes
and 2 << 16 for hardware components.

Yet, we'll need to add an entity for the V4L2 hardware DMA (with makes
sense to me), and this might break backward compatibility if not done
well.

It should be said that, in such case, hardware components will then
mean not only V4L2-specific hardware (V4L2_SUBDEV_foo), but also DVB,
ALSA, ... components.

So, we'll still need a way to identify what of those components are
V4L2 subdevs, probably using the properties API.

If you all agree with that, I'll respin the patch series to map the
entities like that.

Regards,
Mauro


> 
> Hmm, we need a another brainstorm meeting...
> 
> Regards,
> 
> 	Hans

^ permalink raw reply

* Re: [PATCH RFC] vfs: add a O_NOMTIME flag
From: John Stoffel @ 2015-05-08 14:29 UTC (permalink / raw)
  To: Sage Weil
  Cc: Zach Brown, Dave Chinner, Alexander Viro, linux-fsdevel,
	linux-kernel, linux-api
In-Reply-To: <alpine.DEB.2.00.1505071752520.28239@cobra.newdream.net>

>>>>> "Sage" == Sage Weil <sage@newdream.net> writes:

Sage> On Thu, 7 May 2015, Zach Brown wrote:
>> On Thu, May 07, 2015 at 10:26:17AM +1000, Dave Chinner wrote:
>> > On Wed, May 06, 2015 at 03:00:12PM -0700, Zach Brown wrote:
>> > > The criteria for using O_NOMTIME is the same as for using O_NOATIME:
>> > > owning the file or having the CAP_FOWNER capability.  If we're not
>> > > comfortable allowing owners to prevent mtime/ctime updates then we
>> > > should add a tunable to allow O_NOMTIME.  Maybe a mount option?
>> > 
>> > I dislike "turn off safety for performance" options because Joe
>> > SpeedRacer will always select performance over safety.
>> 
>> Well, for ceph there's no safety concern.  They never use cmtime in
>> these files.
>> 
>> So are you suggesting not implementing this and making them rework their
>> IO paths to avoid the fs maintaining mtime so that we don't give Joe
>> Speedracer more rope?  Or are we talking about adding some speed bumps
>> that ceph can flip on that might give Joe Speedracer pause?

Sage> I think this is the fundamental question: who do we give the
Sage> ammunition to, the user or app writer, or the sysadmin?

Sage> One might argue that we gave the user a similar power with
Sage> O_NOATIME (the power to break applications that assume atime is
Sage> accurate).  Here we give developers/users the power to not
Sage> update mtime and suffer the consequences (like, obviously,
Sage> breaking mtime-based backups).  It should be pretty obvious to
Sage> anyone using the flag what the consequences are.

Not modifying atime doesn't really break anything except people who
think they can tell when a file was last accessed.  Which isn't
critical (unless your in a paranoid security conscious place...) but
MTIME is another beast entirely.   Turning that off is going to break
lots of hidden assumptions.  

Sage> Note that we can suffer similar lapses in mtime with fdatasync
Sage> followed by a system crash.  And as Andy points out it's
Sage> semi-broken for writable mmap.  The crash case is obviously a
Sage> slightly different thing, but the idea that mtime can't always
Sage> be trusted certainly isn't crazy talk.

True, but after a crash... people expect and understand there might be
corruption in a filesystem.  

Sage> Or, we can be conservative and require a mount option so that
Sage> the admin has to explicitly allow behavior that might break some
Sage> existing assumptions about mtime/ctime ('-o user_noatime' I
Sage> guess?).


Sage> I'm happy either way, so long as in the end an unprivileged ceph
Sage> daemon avoids the useless work.  In our case we always own the
Sage> entire mount/disk, so a mount option is just fine.

I agree with the mount option, makes it crystal clear.  And then it's
on the sysadmin/owner of the system to understand (ha!) the problems.

This is all me speaking with my Sysadmin hat firmly on my head.

^ permalink raw reply

* Re: [PATCH 07/18] media controller: rename the tuner entity
From: Hans Verkuil @ 2015-05-08 14:32 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Jonathan Corbet,
	Matthias Schwarzott, Antti Palosaari, Olli Salonen, Prabhakar Lad,
	Sakari Ailus, Laurent Pinchart, linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150508110826.00e4e954-+RedX5hVuTR+urZeOPWqwQ@public.gmane.org>

On 05/08/2015 04:08 PM, Mauro Carvalho Chehab wrote:
> Em Fri, 08 May 2015 15:21:39 +0200
> Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> escreveu:
> 
>> On 05/08/2015 02:57 PM, Mauro Carvalho Chehab wrote:
>>> Em Fri, 08 May 2015 14:13:22 +0200
>>> Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> escreveu:
>>>
>>>> On 05/08/2015 03:12 AM, Mauro Carvalho Chehab wrote:
>>>>> Finally, let's rename the tuner entity. inside the media subsystem,
>>>>> a tuner can be used by AM/FM radio, SDR radio, analog TV and digital TV.
>>>>> It could even be used on other subsystems, like network, for wireless
>>>>> devices.
>>>>>
>>>>> So, it is not constricted to V4L2 API, or to a subdev.
>>>>>
>>>>> Let's then rename it as:
>>>>> 	MEDIA_ENT_T_V4L2_SUBDEV_TUNER -> MEDIA_ENT_T_TUNER
>>>>
>>>> See patch 04/18.
>>>
>>> Mapping the tuner as a V4L2_SUBDEV is plain wrong. We can't assume
>>> that a tuner will always be mapped via V4L2 subdev API.
>>
>> True. Today we have subdevs that have no device node to control them, so
>> in that case it would just be a SUBDEV entity. There are subdevs that make
>> a v4l-subdev device node, so those can be V4L(2)_SUBDEV entities.
>>
>> The question is: what are your ideas for e.g. DVB-only tuners? Would they
>> get a DVB-like device node? (so DTV_SUBDEV)
> 
> I guess we may need DVB subdevs in the future. For now, I don't see
> much usage.
> 
>> Would hybrid tuners have two
>> device nodes? One v4l-subdev, one dvb/dtv-subdev?
> 
> No. A tuner is a tuner. The very same device can be used for analog or
> digital TV. Ok, there are tuners that only work for digital TV (satellite
> tuners, typically), because satellite requires a different tuning range,
> and require an extra hardware to power up the satellite antena. So, on
> most devices, the tuner is integrated with SEC.
> 
> In any case, I don't see any reason why artificially one piece of hardware
> component (tuner) into one subdevice entity per API.
> 
> What it might make sense in the future is to have some DVB-specific ioctls
> for a hybrid tuner,  in order to allow adjusting its internal filters to
> an specific digital TV standard.
> 
>> Just curious what your thoughts are.
>>
>> Brainstorming:
>>
>> It might be better to map each device node to an entity and each hardware
>> component (tuner, DMA engine) to an entity, and avoid this mixing of
>> hw entity vs device node entity.
> 
> Ok, but then we need to properly define the namespaces for HW and for
> Linux API components.
> 
> So, we would have a namespace like:
>  
> 	- ENT_T_DEVNODE_DVB_(FE|CA|NET|DEMUX|DVR) for the DVB API device nodes;
> 	- ENT_T_DEVNODE_V4L for the radio/swradio/video/vbi devnodes, or,
> alternatively, use ENT_T_DEVNODE_V4L_(RADIO|SWRADIO|VIDEO|VBI);
> 	- ENT_T_HW_(TUNER|CAM_SENSOR|ATV_DEMOD|DTV_DEMOD|...) for the
> hardware components.
> 
> In other words, the namespace would actually define two subtypes:
> 	- devnodes;
> 	- hardware components
> 
> There's one advantage on this strategy: it is easier to keep backward
> compatibility, IMHO, as we'll be preserving 1 << 16 for device nodes

Right, that will work.

> and 2 << 16 for hardware components.

This remains problematic since I believe this should be done as a list
of properties. Instead the entity type would be ENT_T_HW (if it represents
actual hardware), ENT_T_SW (if it is a software implementation, for example
for the DVB demux if there is no HW demux), and perhaps ENT_T_IP for
representing IP blocks (not sure about this one).

And the properties will tell what functions it supports.

The existing 2 << 16 defines would only be used if the property list matches
the original meaning of the define to keep it backwards compatible.

> 
> Yet, we'll need to add an entity for the V4L2 hardware DMA (with makes
> sense to me), and this might break backward compatibility if not done
> well.

I see this as a property as well, but otherwise I agree with this.

> 
> It should be said that, in such case, hardware components will then
> mean not only V4L2-specific hardware (V4L2_SUBDEV_foo), but also DVB,
> ALSA, ... components.
> 
> So, we'll still need a way to identify what of those components are
> V4L2 subdevs, probably using the properties API.

Why? A hardware component that can be controlled via a v4l-subdev node
would be linked to an entity for that v4l-subdev node. That's an API
entity. The whole 'is this a v4l-subdev' question disappears, since that
is now no longer relevant. Instead you will have an ENT_T_DEVNODE_V4L_SUBDEV
entity linked to the hw entity.

Even a radio device would fit cleanly into this: the tuner entity simply
has a link to a radio device node entity.

Hmm, does this also solve the control vs DMA issue? If a DEVNODE entity
is hooked up to an entity with the DMA functionality, then you can stream,
otherwise it is just for control.

I'm not sure if this is always true, though. Of course, we can also just
add the streaming/dma property to the DEVNODE entity as well.

> If you all agree with that, I'll respin the patch series to map the
> entities like that.

We can interpret the existing ENT_T_HW_TUNER etc. as shorthand for a
property while the property API isn't there yet (we need that anyway
for backwards compat).

So we would need to add a ENT_T_HW_DMA as well (to be replaced by a
property later).

Basically I see the 1 << 16 range as device node types, the 2 << 16 range
as shorthands for what should be properties (this really defined functions
and entities can combine multiple functions), and we would need to have a
new range (4 << 16) for non-DEVNODE entity types. Although we could keep
it in range 1 << 16 as well, but I think it might make sense to keep it
separate.

And there you would get ENT_T_HW/SW/IP (not sure about the last one). And
perhaps FPGA.

Again, just brainstorming here.

Regards,

	Hans

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox