Re: [PATCH v7 01/13] dt-bindings: connector: analog: add tv norms property

2019-08-19 Thread Hans Verkuil
On 8/19/19 11:01 AM, Marco Felsch wrote:
> On 19-08-19 10:39, Hans Verkuil wrote:
>> On 8/19/19 10:09 AM, Marco Felsch wrote:
>>> Hi Hans,
>>>
>>> On 19-08-16 13:11, Hans Verkuil wrote:
 On 8/15/19 1:57 PM, Marco Felsch wrote:
> Some connectors no matter if in- or output supports only a limited
> range of tv norms. It doesn't matter if the hardware behind that
> connector supports more than the listed formats since the users are
> restriced by a label e.g. to plug only a camera into this connector
> which uses the PAL format.
>
> This patch adds the capability to describe such limitation within the
> firmware. There are no format restrictions if the property isn't
> present, so it's completely backward compatible.
>
> Signed-off-by: Marco Felsch 
> Reviewed-by: Rob Herring 
> ---
> [1] https://patchwork.kernel.org/cover/10794703/
>
> v7:
> I kept Robs r b tag because I only changed the example and extended
> TVNORM_* macros.
>
> - fix some style issues
> - add TVNORM_NTSC, TVNORM_525_60 and TVNORM_625_50
>
> v6:
> - tvnorms.h: use tabs instead of spaces
> - tvnorms.h: add TVNORM_PAL and TVNORM_SECAM
> - tvnorms.h: drop rarely used TVNORM_ATSC_* norms
>
> v2-v4:
> - nothing since the patch was squashed from series [1] into this
>   series.
> ---
>  .../display/connector/analog-tv-connector.txt |  4 ++
>  include/dt-bindings/media/tvnorms.h   | 72 +++
>  2 files changed, 76 insertions(+)
>  create mode 100644 include/dt-bindings/media/tvnorms.h
>
> diff --git 
> a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>  
> b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> index 0c0970c210ab..434e8aa0398b 100644
> --- 
> a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> +++ 
> b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> @@ -6,6 +6,9 @@ Required properties:
>  
>  Optional properties:
>  - label: a symbolic name for the connector
> +- tvnorms: limit the supported tv norms on a connector to the given ones 
> else

 tv -> TV

> +   all tv norms are allowed. Possible video standards are 
> defined in

 tv -> TV
>>>
>>> Changed both thanks.
>>>
 And a more high-level question: I think tvnorm is a very vague name. It's 
 a term
 used in media, but what does 'norm' mean anyway? 'tv_standards' or 
 'video_standards'
 would be a lot more descriptive.
>>>
>>> I'm with you *_standards would be more descriptive. Now the question is:
>>> is it a video or a tv standard? Wikipedia says that SECAM, PAL, NTSC is
>>> a television standard. So I prefer the 'tv_standards' or better the
>>> 'tv-standards' approach.
>>
>> It's a bit of both, actually. But in the context of S-Video and Composite 
>> connectors
>> it is a video standard. In the context of a tuner/modulator it would be a TV 
>> standard.
>>
>> I.e. there is no difference between PAL_B and PAL_[GHIDJ] when it comes to 
>> S-Video and
>> Composite connectors, but in the context of a tuner/modulator these defines 
>> give
>> additional information about transmission bands, etc.
>>
>> That said, I think video-standards is too vague, but so is tv-standards. How 
>> about
>> sdtv-standards? I think that makes a lot of sense since this is really just 
>> for SDTV.
> 
> Yes, thats a good name.
> 
>> So tvnorms.h becomes sdtv-standards.h.
> 
> Of course.
> 
>>>
 'tvnorm' is, I think, a term used only internally in the media subsystem 
 for no
 clear reason. In the V4L2 spec it talks about 'video standard'.

 Sorry for being so late with raising this issue.

> +   include/dt-bindings/media/tvnorms.h.
>  
>  Required nodes:
>  - Video port for TV input
> @@ -16,6 +19,7 @@ Example
>  tv: connector {
>   compatible = "composite-video-connector";
>   label = "tv";
> + tvnorms = <(TVNORM_PAL | TVNORM_NTSC)>;
>  
>   port {
>   tv_connector_in: endpoint {
> diff --git a/include/dt-bindings/media/tvnorms.h 
> b/include/dt-bindings/media/tvnorms.h
> new file mode 100644
> index ..e1275673c4d6
> --- /dev/null
> +++ b/include/dt-bindings/media/tvnorms.h
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: GPL-2.0-only or X11 */
> +/*
> + * Copyright 2019 Pengutronix, Marco Felsch 
> + */
> +
> +#ifndef _DT_BINDINGS_MEDIA_TVNORMS_H
> +#define _DT_BINDINGS_MEDIA_TVNORMS_H
> +
> +/* One bit for each standard */

 I would add a comment here and in videodev2.h where you mention that
 the two headers should remain in sync.
>>>
>>> Yes, good point.
>>>
> +#define TVNORM_PAL_B 0x0001
> +#define TVNORM_P

Re: [PATCH v7 01/13] dt-bindings: connector: analog: add tv norms property

2019-08-19 Thread Marco Felsch
On 19-08-19 10:39, Hans Verkuil wrote:
> On 8/19/19 10:09 AM, Marco Felsch wrote:
> > Hi Hans,
> > 
> > On 19-08-16 13:11, Hans Verkuil wrote:
> >> On 8/15/19 1:57 PM, Marco Felsch wrote:
> >>> Some connectors no matter if in- or output supports only a limited
> >>> range of tv norms. It doesn't matter if the hardware behind that
> >>> connector supports more than the listed formats since the users are
> >>> restriced by a label e.g. to plug only a camera into this connector
> >>> which uses the PAL format.
> >>>
> >>> This patch adds the capability to describe such limitation within the
> >>> firmware. There are no format restrictions if the property isn't
> >>> present, so it's completely backward compatible.
> >>>
> >>> Signed-off-by: Marco Felsch 
> >>> Reviewed-by: Rob Herring 
> >>> ---
> >>> [1] https://patchwork.kernel.org/cover/10794703/
> >>>
> >>> v7:
> >>> I kept Robs r b tag because I only changed the example and extended
> >>> TVNORM_* macros.
> >>>
> >>> - fix some style issues
> >>> - add TVNORM_NTSC, TVNORM_525_60 and TVNORM_625_50
> >>>
> >>> v6:
> >>> - tvnorms.h: use tabs instead of spaces
> >>> - tvnorms.h: add TVNORM_PAL and TVNORM_SECAM
> >>> - tvnorms.h: drop rarely used TVNORM_ATSC_* norms
> >>>
> >>> v2-v4:
> >>> - nothing since the patch was squashed from series [1] into this
> >>>   series.
> >>> ---
> >>>  .../display/connector/analog-tv-connector.txt |  4 ++
> >>>  include/dt-bindings/media/tvnorms.h   | 72 +++
> >>>  2 files changed, 76 insertions(+)
> >>>  create mode 100644 include/dt-bindings/media/tvnorms.h
> >>>
> >>> diff --git 
> >>> a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> >>>  
> >>> b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> >>> index 0c0970c210ab..434e8aa0398b 100644
> >>> --- 
> >>> a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> >>> +++ 
> >>> b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> >>> @@ -6,6 +6,9 @@ Required properties:
> >>>  
> >>>  Optional properties:
> >>>  - label: a symbolic name for the connector
> >>> +- tvnorms: limit the supported tv norms on a connector to the given ones 
> >>> else
> >>
> >> tv -> TV
> >>
> >>> +   all tv norms are allowed. Possible video standards are 
> >>> defined in
> >>
> >> tv -> TV
> > 
> > Changed both thanks.
> > 
> >> And a more high-level question: I think tvnorm is a very vague name. It's 
> >> a term
> >> used in media, but what does 'norm' mean anyway? 'tv_standards' or 
> >> 'video_standards'
> >> would be a lot more descriptive.
> > 
> > I'm with you *_standards would be more descriptive. Now the question is:
> > is it a video or a tv standard? Wikipedia says that SECAM, PAL, NTSC is
> > a television standard. So I prefer the 'tv_standards' or better the
> > 'tv-standards' approach.
> 
> It's a bit of both, actually. But in the context of S-Video and Composite 
> connectors
> it is a video standard. In the context of a tuner/modulator it would be a TV 
> standard.
> 
> I.e. there is no difference between PAL_B and PAL_[GHIDJ] when it comes to 
> S-Video and
> Composite connectors, but in the context of a tuner/modulator these defines 
> give
> additional information about transmission bands, etc.
> 
> That said, I think video-standards is too vague, but so is tv-standards. How 
> about
> sdtv-standards? I think that makes a lot of sense since this is really just 
> for SDTV.

Yes, thats a good name.

> So tvnorms.h becomes sdtv-standards.h.

Of course.

> > 
> >> 'tvnorm' is, I think, a term used only internally in the media subsystem 
> >> for no
> >> clear reason. In the V4L2 spec it talks about 'video standard'.
> >>
> >> Sorry for being so late with raising this issue.
> >>
> >>> +   include/dt-bindings/media/tvnorms.h.
> >>>  
> >>>  Required nodes:
> >>>  - Video port for TV input
> >>> @@ -16,6 +19,7 @@ Example
> >>>  tv: connector {
> >>>   compatible = "composite-video-connector";
> >>>   label = "tv";
> >>> + tvnorms = <(TVNORM_PAL | TVNORM_NTSC)>;
> >>>  
> >>>   port {
> >>>   tv_connector_in: endpoint {
> >>> diff --git a/include/dt-bindings/media/tvnorms.h 
> >>> b/include/dt-bindings/media/tvnorms.h
> >>> new file mode 100644
> >>> index ..e1275673c4d6
> >>> --- /dev/null
> >>> +++ b/include/dt-bindings/media/tvnorms.h
> >>> @@ -0,0 +1,72 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0-only or X11 */
> >>> +/*
> >>> + * Copyright 2019 Pengutronix, Marco Felsch 
> >>> + */
> >>> +
> >>> +#ifndef _DT_BINDINGS_MEDIA_TVNORMS_H
> >>> +#define _DT_BINDINGS_MEDIA_TVNORMS_H
> >>> +
> >>> +/* One bit for each standard */
> >>
> >> I would add a comment here and in videodev2.h where you mention that
> >> the two headers should remain in sync.
> > 
> > Yes, good point.
> > 
> >>> +#define TVNORM_PAL_B 0x0001
> >>> +#define TVNORM_PAL_B10x0002
> >>> +#define TVNORM_PAL_G

Re: [PATCH v7 01/13] dt-bindings: connector: analog: add tv norms property

2019-08-19 Thread Hans Verkuil
On 8/19/19 10:18 AM, Marco Felsch wrote:
> Hi,
> 
> On 19-08-19 10:09, Marco Felsch wrote:
>> Hi Hans,
>>
>> On 19-08-16 13:11, Hans Verkuil wrote:
>>> On 8/15/19 1:57 PM, Marco Felsch wrote:
 Some connectors no matter if in- or output supports only a limited
 range of tv norms. It doesn't matter if the hardware behind that
 connector supports more than the listed formats since the users are
 restriced by a label e.g. to plug only a camera into this connector
 which uses the PAL format.

 This patch adds the capability to describe such limitation within the
 firmware. There are no format restrictions if the property isn't
 present, so it's completely backward compatible.

 Signed-off-by: Marco Felsch 
 Reviewed-by: Rob Herring 
 ---
 [1] https://patchwork.kernel.org/cover/10794703/

 v7:
 I kept Robs r b tag because I only changed the example and extended
 TVNORM_* macros.

 - fix some style issues
 - add TVNORM_NTSC, TVNORM_525_60 and TVNORM_625_50

 v6:
 - tvnorms.h: use tabs instead of spaces
 - tvnorms.h: add TVNORM_PAL and TVNORM_SECAM
 - tvnorms.h: drop rarely used TVNORM_ATSC_* norms

 v2-v4:
 - nothing since the patch was squashed from series [1] into this
   series.
 ---
  .../display/connector/analog-tv-connector.txt |  4 ++
  include/dt-bindings/media/tvnorms.h   | 72 +++
  2 files changed, 76 insertions(+)
  create mode 100644 include/dt-bindings/media/tvnorms.h

 diff --git 
 a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
  
 b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
 index 0c0970c210ab..434e8aa0398b 100644
 --- 
 a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
 +++ 
 b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
 @@ -6,6 +6,9 @@ Required properties:
  
  Optional properties:
  - label: a symbolic name for the connector
 +- tvnorms: limit the supported tv norms on a connector to the given ones 
 else
>>>
>>> tv -> TV
>>>
 +   all tv norms are allowed. Possible video standards are defined 
 in
>>>
>>> tv -> TV
>>
>> Changed both thanks.
>>
>>> And a more high-level question: I think tvnorm is a very vague name. It's a 
>>> term
>>> used in media, but what does 'norm' mean anyway? 'tv_standards' or 
>>> 'video_standards'
>>> would be a lot more descriptive.
>>
>> I'm with you *_standards would be more descriptive. Now the question is:
>> is it a video or a tv standard? Wikipedia says that SECAM, PAL, NTSC is
>> a television standard. So I prefer the 'tv_standards' or better the
>> 'tv-standards' approach.
> 
> While on it I noticed that the v4l_std_id is u64. Should we add the
> support for u64 here too?

No need. SDTV standards are frozen (i.e. no new standards will ever be added),
so everything fits nicely in 32 bits.

Regards,

Hans

> 
> Regards,
>   Marco
> 
>>> 'tvnorm' is, I think, a term used only internally in the media subsystem 
>>> for no
>>> clear reason. In the V4L2 spec it talks about 'video standard'.
>>>
>>> Sorry for being so late with raising this issue.
>>>
 +   include/dt-bindings/media/tvnorms.h.
  
  Required nodes:
  - Video port for TV input
 @@ -16,6 +19,7 @@ Example
  tv: connector {
compatible = "composite-video-connector";
label = "tv";
 +  tvnorms = <(TVNORM_PAL | TVNORM_NTSC)>;
  
port {
tv_connector_in: endpoint {
 diff --git a/include/dt-bindings/media/tvnorms.h 
 b/include/dt-bindings/media/tvnorms.h
 new file mode 100644
 index ..e1275673c4d6
 --- /dev/null
 +++ b/include/dt-bindings/media/tvnorms.h
 @@ -0,0 +1,72 @@
 +/* SPDX-License-Identifier: GPL-2.0-only or X11 */
 +/*
 + * Copyright 2019 Pengutronix, Marco Felsch 
 + */
 +
 +#ifndef _DT_BINDINGS_MEDIA_TVNORMS_H
 +#define _DT_BINDINGS_MEDIA_TVNORMS_H
 +
 +/* One bit for each standard */
>>>
>>> I would add a comment here and in videodev2.h where you mention that
>>> the two headers should remain in sync.
>>
>> Yes, good point.
>>
 +#define TVNORM_PAL_B  0x0001
 +#define TVNORM_PAL_B1 0x0002
 +#define TVNORM_PAL_G  0x0004
 +#define TVNORM_PAL_H  0x0008
 +#define TVNORM_PAL_I  0x0010
 +#define TVNORM_PAL_D  0x0020
 +#define TVNORM_PAL_D1 0x0040
 +#define TVNORM_PAL_K  0x0080
 +
 +#define TVNORM_PAL(TVNORM_PAL_B   | \
 +   TVNORM_PAL_B1  | \
 +   TVNORM_PAL_G   | \
 +   TVNORM_PAL_H   | \

Re: [PATCH v7 01/13] dt-bindings: connector: analog: add tv norms property

2019-08-19 Thread Hans Verkuil
On 8/19/19 10:09 AM, Marco Felsch wrote:
> Hi Hans,
> 
> On 19-08-16 13:11, Hans Verkuil wrote:
>> On 8/15/19 1:57 PM, Marco Felsch wrote:
>>> Some connectors no matter if in- or output supports only a limited
>>> range of tv norms. It doesn't matter if the hardware behind that
>>> connector supports more than the listed formats since the users are
>>> restriced by a label e.g. to plug only a camera into this connector
>>> which uses the PAL format.
>>>
>>> This patch adds the capability to describe such limitation within the
>>> firmware. There are no format restrictions if the property isn't
>>> present, so it's completely backward compatible.
>>>
>>> Signed-off-by: Marco Felsch 
>>> Reviewed-by: Rob Herring 
>>> ---
>>> [1] https://patchwork.kernel.org/cover/10794703/
>>>
>>> v7:
>>> I kept Robs r b tag because I only changed the example and extended
>>> TVNORM_* macros.
>>>
>>> - fix some style issues
>>> - add TVNORM_NTSC, TVNORM_525_60 and TVNORM_625_50
>>>
>>> v6:
>>> - tvnorms.h: use tabs instead of spaces
>>> - tvnorms.h: add TVNORM_PAL and TVNORM_SECAM
>>> - tvnorms.h: drop rarely used TVNORM_ATSC_* norms
>>>
>>> v2-v4:
>>> - nothing since the patch was squashed from series [1] into this
>>>   series.
>>> ---
>>>  .../display/connector/analog-tv-connector.txt |  4 ++
>>>  include/dt-bindings/media/tvnorms.h   | 72 +++
>>>  2 files changed, 76 insertions(+)
>>>  create mode 100644 include/dt-bindings/media/tvnorms.h
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>>>  
>>> b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>>> index 0c0970c210ab..434e8aa0398b 100644
>>> --- 
>>> a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>>> +++ 
>>> b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
>>> @@ -6,6 +6,9 @@ Required properties:
>>>  
>>>  Optional properties:
>>>  - label: a symbolic name for the connector
>>> +- tvnorms: limit the supported tv norms on a connector to the given ones 
>>> else
>>
>> tv -> TV
>>
>>> +   all tv norms are allowed. Possible video standards are defined 
>>> in
>>
>> tv -> TV
> 
> Changed both thanks.
> 
>> And a more high-level question: I think tvnorm is a very vague name. It's a 
>> term
>> used in media, but what does 'norm' mean anyway? 'tv_standards' or 
>> 'video_standards'
>> would be a lot more descriptive.
> 
> I'm with you *_standards would be more descriptive. Now the question is:
> is it a video or a tv standard? Wikipedia says that SECAM, PAL, NTSC is
> a television standard. So I prefer the 'tv_standards' or better the
> 'tv-standards' approach.

It's a bit of both, actually. But in the context of S-Video and Composite 
connectors
it is a video standard. In the context of a tuner/modulator it would be a TV 
standard.

I.e. there is no difference between PAL_B and PAL_[GHIDJ] when it comes to 
S-Video and
Composite connectors, but in the context of a tuner/modulator these defines give
additional information about transmission bands, etc.

That said, I think video-standards is too vague, but so is tv-standards. How 
about
sdtv-standards? I think that makes a lot of sense since this is really just for 
SDTV.

So tvnorms.h becomes sdtv-standards.h.

> 
>> 'tvnorm' is, I think, a term used only internally in the media subsystem for 
>> no
>> clear reason. In the V4L2 spec it talks about 'video standard'.
>>
>> Sorry for being so late with raising this issue.
>>
>>> +   include/dt-bindings/media/tvnorms.h.
>>>  
>>>  Required nodes:
>>>  - Video port for TV input
>>> @@ -16,6 +19,7 @@ Example
>>>  tv: connector {
>>> compatible = "composite-video-connector";
>>> label = "tv";
>>> +   tvnorms = <(TVNORM_PAL | TVNORM_NTSC)>;
>>>  
>>> port {
>>> tv_connector_in: endpoint {
>>> diff --git a/include/dt-bindings/media/tvnorms.h 
>>> b/include/dt-bindings/media/tvnorms.h
>>> new file mode 100644
>>> index ..e1275673c4d6
>>> --- /dev/null
>>> +++ b/include/dt-bindings/media/tvnorms.h
>>> @@ -0,0 +1,72 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only or X11 */
>>> +/*
>>> + * Copyright 2019 Pengutronix, Marco Felsch 
>>> + */
>>> +
>>> +#ifndef _DT_BINDINGS_MEDIA_TVNORMS_H
>>> +#define _DT_BINDINGS_MEDIA_TVNORMS_H
>>> +
>>> +/* One bit for each standard */
>>
>> I would add a comment here and in videodev2.h where you mention that
>> the two headers should remain in sync.
> 
> Yes, good point.
> 
>>> +#define TVNORM_PAL_B   0x0001
>>> +#define TVNORM_PAL_B1  0x0002
>>> +#define TVNORM_PAL_G   0x0004
>>> +#define TVNORM_PAL_H   0x0008
>>> +#define TVNORM_PAL_I   0x0010
>>> +#define TVNORM_PAL_D   0x0020
>>> +#define TVNORM_PAL_D1  0x0040
>>> +#define TVNORM_PAL_K   0x0080
>>> +
>>> +#define TVNORM_PAL (TVNORM_PAL_B   | 

Re: [PATCH v7 01/13] dt-bindings: connector: analog: add tv norms property

2019-08-19 Thread Marco Felsch
Hi,

On 19-08-19 10:09, Marco Felsch wrote:
> Hi Hans,
> 
> On 19-08-16 13:11, Hans Verkuil wrote:
> > On 8/15/19 1:57 PM, Marco Felsch wrote:
> > > Some connectors no matter if in- or output supports only a limited
> > > range of tv norms. It doesn't matter if the hardware behind that
> > > connector supports more than the listed formats since the users are
> > > restriced by a label e.g. to plug only a camera into this connector
> > > which uses the PAL format.
> > > 
> > > This patch adds the capability to describe such limitation within the
> > > firmware. There are no format restrictions if the property isn't
> > > present, so it's completely backward compatible.
> > > 
> > > Signed-off-by: Marco Felsch 
> > > Reviewed-by: Rob Herring 
> > > ---
> > > [1] https://patchwork.kernel.org/cover/10794703/
> > > 
> > > v7:
> > > I kept Robs r b tag because I only changed the example and extended
> > > TVNORM_* macros.
> > > 
> > > - fix some style issues
> > > - add TVNORM_NTSC, TVNORM_525_60 and TVNORM_625_50
> > > 
> > > v6:
> > > - tvnorms.h: use tabs instead of spaces
> > > - tvnorms.h: add TVNORM_PAL and TVNORM_SECAM
> > > - tvnorms.h: drop rarely used TVNORM_ATSC_* norms
> > > 
> > > v2-v4:
> > > - nothing since the patch was squashed from series [1] into this
> > >   series.
> > > ---
> > >  .../display/connector/analog-tv-connector.txt |  4 ++
> > >  include/dt-bindings/media/tvnorms.h   | 72 +++
> > >  2 files changed, 76 insertions(+)
> > >  create mode 100644 include/dt-bindings/media/tvnorms.h
> > > 
> > > diff --git 
> > > a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> > >  
> > > b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> > > index 0c0970c210ab..434e8aa0398b 100644
> > > --- 
> > > a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> > > +++ 
> > > b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> > > @@ -6,6 +6,9 @@ Required properties:
> > >  
> > >  Optional properties:
> > >  - label: a symbolic name for the connector
> > > +- tvnorms: limit the supported tv norms on a connector to the given ones 
> > > else
> > 
> > tv -> TV
> > 
> > > +   all tv norms are allowed. Possible video standards are 
> > > defined in
> > 
> > tv -> TV
> 
> Changed both thanks.
> 
> > And a more high-level question: I think tvnorm is a very vague name. It's a 
> > term
> > used in media, but what does 'norm' mean anyway? 'tv_standards' or 
> > 'video_standards'
> > would be a lot more descriptive.
> 
> I'm with you *_standards would be more descriptive. Now the question is:
> is it a video or a tv standard? Wikipedia says that SECAM, PAL, NTSC is
> a television standard. So I prefer the 'tv_standards' or better the
> 'tv-standards' approach.

While on it I noticed that the v4l_std_id is u64. Should we add the
support for u64 here too?

Regards,
  Marco

> > 'tvnorm' is, I think, a term used only internally in the media subsystem 
> > for no
> > clear reason. In the V4L2 spec it talks about 'video standard'.
> > 
> > Sorry for being so late with raising this issue.
> > 
> > > +   include/dt-bindings/media/tvnorms.h.
> > >  
> > >  Required nodes:
> > >  - Video port for TV input
> > > @@ -16,6 +19,7 @@ Example
> > >  tv: connector {
> > >   compatible = "composite-video-connector";
> > >   label = "tv";
> > > + tvnorms = <(TVNORM_PAL | TVNORM_NTSC)>;
> > >  
> > >   port {
> > >   tv_connector_in: endpoint {
> > > diff --git a/include/dt-bindings/media/tvnorms.h 
> > > b/include/dt-bindings/media/tvnorms.h
> > > new file mode 100644
> > > index ..e1275673c4d6
> > > --- /dev/null
> > > +++ b/include/dt-bindings/media/tvnorms.h
> > > @@ -0,0 +1,72 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only or X11 */
> > > +/*
> > > + * Copyright 2019 Pengutronix, Marco Felsch 
> > > + */
> > > +
> > > +#ifndef _DT_BINDINGS_MEDIA_TVNORMS_H
> > > +#define _DT_BINDINGS_MEDIA_TVNORMS_H
> > > +
> > > +/* One bit for each standard */
> > 
> > I would add a comment here and in videodev2.h where you mention that
> > the two headers should remain in sync.
> 
> Yes, good point.
> 
> > > +#define TVNORM_PAL_B 0x0001
> > > +#define TVNORM_PAL_B10x0002
> > > +#define TVNORM_PAL_G 0x0004
> > > +#define TVNORM_PAL_H 0x0008
> > > +#define TVNORM_PAL_I 0x0010
> > > +#define TVNORM_PAL_D 0x0020
> > > +#define TVNORM_PAL_D10x0040
> > > +#define TVNORM_PAL_K 0x0080
> > > +
> > > +#define TVNORM_PAL   (TVNORM_PAL_B   | \
> > > +  TVNORM_PAL_B1  | \
> > > +  TVNORM_PAL_G   | \
> > > +  TVNORM_PAL_H   | \
> > > +  TVNORM_PAL_I   | \
> > > +  TVNORM_PAL_D   | \
>

Re: [PATCH v7 01/13] dt-bindings: connector: analog: add tv norms property

2019-08-19 Thread Marco Felsch
Hi Hans,

On 19-08-16 13:11, Hans Verkuil wrote:
> On 8/15/19 1:57 PM, Marco Felsch wrote:
> > Some connectors no matter if in- or output supports only a limited
> > range of tv norms. It doesn't matter if the hardware behind that
> > connector supports more than the listed formats since the users are
> > restriced by a label e.g. to plug only a camera into this connector
> > which uses the PAL format.
> > 
> > This patch adds the capability to describe such limitation within the
> > firmware. There are no format restrictions if the property isn't
> > present, so it's completely backward compatible.
> > 
> > Signed-off-by: Marco Felsch 
> > Reviewed-by: Rob Herring 
> > ---
> > [1] https://patchwork.kernel.org/cover/10794703/
> > 
> > v7:
> > I kept Robs r b tag because I only changed the example and extended
> > TVNORM_* macros.
> > 
> > - fix some style issues
> > - add TVNORM_NTSC, TVNORM_525_60 and TVNORM_625_50
> > 
> > v6:
> > - tvnorms.h: use tabs instead of spaces
> > - tvnorms.h: add TVNORM_PAL and TVNORM_SECAM
> > - tvnorms.h: drop rarely used TVNORM_ATSC_* norms
> > 
> > v2-v4:
> > - nothing since the patch was squashed from series [1] into this
> >   series.
> > ---
> >  .../display/connector/analog-tv-connector.txt |  4 ++
> >  include/dt-bindings/media/tvnorms.h   | 72 +++
> >  2 files changed, 76 insertions(+)
> >  create mode 100644 include/dt-bindings/media/tvnorms.h
> > 
> > diff --git 
> > a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> >  
> > b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> > index 0c0970c210ab..434e8aa0398b 100644
> > --- 
> > a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> > +++ 
> > b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> > @@ -6,6 +6,9 @@ Required properties:
> >  
> >  Optional properties:
> >  - label: a symbolic name for the connector
> > +- tvnorms: limit the supported tv norms on a connector to the given ones 
> > else
> 
> tv -> TV
> 
> > +   all tv norms are allowed. Possible video standards are defined 
> > in
> 
> tv -> TV

Changed both thanks.

> And a more high-level question: I think tvnorm is a very vague name. It's a 
> term
> used in media, but what does 'norm' mean anyway? 'tv_standards' or 
> 'video_standards'
> would be a lot more descriptive.

I'm with you *_standards would be more descriptive. Now the question is:
is it a video or a tv standard? Wikipedia says that SECAM, PAL, NTSC is
a television standard. So I prefer the 'tv_standards' or better the
'tv-standards' approach.

> 'tvnorm' is, I think, a term used only internally in the media subsystem for 
> no
> clear reason. In the V4L2 spec it talks about 'video standard'.
> 
> Sorry for being so late with raising this issue.
> 
> > +   include/dt-bindings/media/tvnorms.h.
> >  
> >  Required nodes:
> >  - Video port for TV input
> > @@ -16,6 +19,7 @@ Example
> >  tv: connector {
> > compatible = "composite-video-connector";
> > label = "tv";
> > +   tvnorms = <(TVNORM_PAL | TVNORM_NTSC)>;
> >  
> > port {
> > tv_connector_in: endpoint {
> > diff --git a/include/dt-bindings/media/tvnorms.h 
> > b/include/dt-bindings/media/tvnorms.h
> > new file mode 100644
> > index ..e1275673c4d6
> > --- /dev/null
> > +++ b/include/dt-bindings/media/tvnorms.h
> > @@ -0,0 +1,72 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only or X11 */
> > +/*
> > + * Copyright 2019 Pengutronix, Marco Felsch 
> > + */
> > +
> > +#ifndef _DT_BINDINGS_MEDIA_TVNORMS_H
> > +#define _DT_BINDINGS_MEDIA_TVNORMS_H
> > +
> > +/* One bit for each standard */
> 
> I would add a comment here and in videodev2.h where you mention that
> the two headers should remain in sync.

Yes, good point.

> > +#define TVNORM_PAL_B   0x0001
> > +#define TVNORM_PAL_B1  0x0002
> > +#define TVNORM_PAL_G   0x0004
> > +#define TVNORM_PAL_H   0x0008
> > +#define TVNORM_PAL_I   0x0010
> > +#define TVNORM_PAL_D   0x0020
> > +#define TVNORM_PAL_D1  0x0040
> > +#define TVNORM_PAL_K   0x0080
> > +
> > +#define TVNORM_PAL (TVNORM_PAL_B   | \
> > +TVNORM_PAL_B1  | \
> > +TVNORM_PAL_G   | \
> > +TVNORM_PAL_H   | \
> > +TVNORM_PAL_I   | \
> > +TVNORM_PAL_D   | \
> > +TVNORM_PAL_D1  | \
> > +TVNORM_PAL_K)
> > +
> > +#define TVNORM_PAL_M   0x0100
> > +#define TVNORM_PAL_N   0x0200
> > +#define TVNORM_PAL_Nc  0x0400
> > +#define TVNORM_PAL_60  0x0800
> > +
> > +#define TVNORM_NTSC_M  0x1000  /* BTSC *

Re: [PATCH v7 01/13] dt-bindings: connector: analog: add tv norms property

2019-08-16 Thread Hans Verkuil
On 8/15/19 1:57 PM, Marco Felsch wrote:
> Some connectors no matter if in- or output supports only a limited
> range of tv norms. It doesn't matter if the hardware behind that
> connector supports more than the listed formats since the users are
> restriced by a label e.g. to plug only a camera into this connector
> which uses the PAL format.
> 
> This patch adds the capability to describe such limitation within the
> firmware. There are no format restrictions if the property isn't
> present, so it's completely backward compatible.
> 
> Signed-off-by: Marco Felsch 
> Reviewed-by: Rob Herring 
> ---
> [1] https://patchwork.kernel.org/cover/10794703/
> 
> v7:
> I kept Robs r b tag because I only changed the example and extended
> TVNORM_* macros.
> 
> - fix some style issues
> - add TVNORM_NTSC, TVNORM_525_60 and TVNORM_625_50
> 
> v6:
> - tvnorms.h: use tabs instead of spaces
> - tvnorms.h: add TVNORM_PAL and TVNORM_SECAM
> - tvnorms.h: drop rarely used TVNORM_ATSC_* norms
> 
> v2-v4:
> - nothing since the patch was squashed from series [1] into this
>   series.
> ---
>  .../display/connector/analog-tv-connector.txt |  4 ++
>  include/dt-bindings/media/tvnorms.h   | 72 +++
>  2 files changed, 76 insertions(+)
>  create mode 100644 include/dt-bindings/media/tvnorms.h
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt 
> b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> index 0c0970c210ab..434e8aa0398b 100644
> --- 
> a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> +++ 
> b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> @@ -6,6 +6,9 @@ Required properties:
>  
>  Optional properties:
>  - label: a symbolic name for the connector
> +- tvnorms: limit the supported tv norms on a connector to the given ones else

tv -> TV

> +   all tv norms are allowed. Possible video standards are defined in

tv -> TV

And a more high-level question: I think tvnorm is a very vague name. It's a term
used in media, but what does 'norm' mean anyway? 'tv_standards' or 
'video_standards'
would be a lot more descriptive.

'tvnorm' is, I think, a term used only internally in the media subsystem for no
clear reason. In the V4L2 spec it talks about 'video standard'.

Sorry for being so late with raising this issue.

> +   include/dt-bindings/media/tvnorms.h.
>  
>  Required nodes:
>  - Video port for TV input
> @@ -16,6 +19,7 @@ Example
>  tv: connector {
>   compatible = "composite-video-connector";
>   label = "tv";
> + tvnorms = <(TVNORM_PAL | TVNORM_NTSC)>;
>  
>   port {
>   tv_connector_in: endpoint {
> diff --git a/include/dt-bindings/media/tvnorms.h 
> b/include/dt-bindings/media/tvnorms.h
> new file mode 100644
> index ..e1275673c4d6
> --- /dev/null
> +++ b/include/dt-bindings/media/tvnorms.h
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: GPL-2.0-only or X11 */
> +/*
> + * Copyright 2019 Pengutronix, Marco Felsch 
> + */
> +
> +#ifndef _DT_BINDINGS_MEDIA_TVNORMS_H
> +#define _DT_BINDINGS_MEDIA_TVNORMS_H
> +
> +/* One bit for each standard */

I would add a comment here and in videodev2.h where you mention that
the two headers should remain in sync.

> +#define TVNORM_PAL_B 0x0001
> +#define TVNORM_PAL_B10x0002
> +#define TVNORM_PAL_G 0x0004
> +#define TVNORM_PAL_H 0x0008
> +#define TVNORM_PAL_I 0x0010
> +#define TVNORM_PAL_D 0x0020
> +#define TVNORM_PAL_D10x0040
> +#define TVNORM_PAL_K 0x0080
> +
> +#define TVNORM_PAL   (TVNORM_PAL_B   | \
> +  TVNORM_PAL_B1  | \
> +  TVNORM_PAL_G   | \
> +  TVNORM_PAL_H   | \
> +  TVNORM_PAL_I   | \
> +  TVNORM_PAL_D   | \
> +  TVNORM_PAL_D1  | \
> +  TVNORM_PAL_K)
> +
> +#define TVNORM_PAL_M 0x0100
> +#define TVNORM_PAL_N 0x0200
> +#define TVNORM_PAL_Nc0x0400
> +#define TVNORM_PAL_600x0800
> +
> +#define TVNORM_NTSC_M0x1000  /* BTSC */
> +#define TVNORM_NTSC_M_JP 0x2000  /* EIA-J */
> +#define TVNORM_NTSC_443  0x4000
> +#define TVNORM_NTSC_M_KR 0x8000  /* FM A2 */
> +
> +#define TVNORM_NTSC  (TVNORM_NTSC_M  |\
> +  TVNORM_NTSC_M_JP   |\

Add space before \

> +  TVNORM_NTSC_M_KR)
> +
> +#define TVNORM_SECAM_B   0x0001
> +#define TVNORM_SECAM_D   0x0002
> +#define TVNORM_SECAM_G   0x0004
> +#define TVNORM_SECAM_H   0x0008
> +#define TVNORM_SECAM_K 

[PATCH v7 01/13] dt-bindings: connector: analog: add tv norms property

2019-08-15 Thread Marco Felsch
Some connectors no matter if in- or output supports only a limited
range of tv norms. It doesn't matter if the hardware behind that
connector supports more than the listed formats since the users are
restriced by a label e.g. to plug only a camera into this connector
which uses the PAL format.

This patch adds the capability to describe such limitation within the
firmware. There are no format restrictions if the property isn't
present, so it's completely backward compatible.

Signed-off-by: Marco Felsch 
Reviewed-by: Rob Herring 
---
[1] https://patchwork.kernel.org/cover/10794703/

v7:
I kept Robs r b tag because I only changed the example and extended
TVNORM_* macros.

- fix some style issues
- add TVNORM_NTSC, TVNORM_525_60 and TVNORM_625_50

v6:
- tvnorms.h: use tabs instead of spaces
- tvnorms.h: add TVNORM_PAL and TVNORM_SECAM
- tvnorms.h: drop rarely used TVNORM_ATSC_* norms

v2-v4:
- nothing since the patch was squashed from series [1] into this
  series.
---
 .../display/connector/analog-tv-connector.txt |  4 ++
 include/dt-bindings/media/tvnorms.h   | 72 +++
 2 files changed, 76 insertions(+)
 create mode 100644 include/dt-bindings/media/tvnorms.h

diff --git 
a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt 
b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
index 0c0970c210ab..434e8aa0398b 100644
--- 
a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
+++ 
b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
@@ -6,6 +6,9 @@ Required properties:
 
 Optional properties:
 - label: a symbolic name for the connector
+- tvnorms: limit the supported tv norms on a connector to the given ones else
+   all tv norms are allowed. Possible video standards are defined in
+   include/dt-bindings/media/tvnorms.h.
 
 Required nodes:
 - Video port for TV input
@@ -16,6 +19,7 @@ Example
 tv: connector {
compatible = "composite-video-connector";
label = "tv";
+   tvnorms = <(TVNORM_PAL | TVNORM_NTSC)>;
 
port {
tv_connector_in: endpoint {
diff --git a/include/dt-bindings/media/tvnorms.h 
b/include/dt-bindings/media/tvnorms.h
new file mode 100644
index ..e1275673c4d6
--- /dev/null
+++ b/include/dt-bindings/media/tvnorms.h
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: GPL-2.0-only or X11 */
+/*
+ * Copyright 2019 Pengutronix, Marco Felsch 
+ */
+
+#ifndef _DT_BINDINGS_MEDIA_TVNORMS_H
+#define _DT_BINDINGS_MEDIA_TVNORMS_H
+
+/* One bit for each standard */
+#define TVNORM_PAL_B   0x0001
+#define TVNORM_PAL_B1  0x0002
+#define TVNORM_PAL_G   0x0004
+#define TVNORM_PAL_H   0x0008
+#define TVNORM_PAL_I   0x0010
+#define TVNORM_PAL_D   0x0020
+#define TVNORM_PAL_D1  0x0040
+#define TVNORM_PAL_K   0x0080
+
+#define TVNORM_PAL (TVNORM_PAL_B   | \
+TVNORM_PAL_B1  | \
+TVNORM_PAL_G   | \
+TVNORM_PAL_H   | \
+TVNORM_PAL_I   | \
+TVNORM_PAL_D   | \
+TVNORM_PAL_D1  | \
+TVNORM_PAL_K)
+
+#define TVNORM_PAL_M   0x0100
+#define TVNORM_PAL_N   0x0200
+#define TVNORM_PAL_Nc  0x0400
+#define TVNORM_PAL_60  0x0800
+
+#define TVNORM_NTSC_M  0x1000  /* BTSC */
+#define TVNORM_NTSC_M_JP   0x2000  /* EIA-J */
+#define TVNORM_NTSC_4430x4000
+#define TVNORM_NTSC_M_KR   0x8000  /* FM A2 */
+
+#define TVNORM_NTSC(TVNORM_NTSC_M  |\
+TVNORM_NTSC_M_JP   |\
+TVNORM_NTSC_M_KR)
+
+#define TVNORM_SECAM_B 0x0001
+#define TVNORM_SECAM_D 0x0002
+#define TVNORM_SECAM_G 0x0004
+#define TVNORM_SECAM_H 0x0008
+#define TVNORM_SECAM_K 0x0010
+#define TVNORM_SECAM_K10x0020
+#define TVNORM_SECAM_L 0x0040
+#define TVNORM_SECAM_LC0x0080
+
+#define TVNORM_SECAM   (TVNORM_SECAM_B | \
+TVNORM_SECAM_D | \
+TVNORM_SECAM_G | \
+TVNORM_SECAM_H | \
+TVNORM_SECAM_K | \
+TVNORM_SECAM_K1| \
+TVNORM_SECAM_L | \
+TVNORM_SECAM_LC)
+
+/* Standards for Countries with 60Hz Line frequency */
+#define TVNORM_525_60  (TVNORM_PAL_M   | \
+TVNORM_PAL_60  | \
+