Re: [PATCH v10 01/14] dt-bindings: connector: analog: add sdtv standards property
Hi Hans,
On 19-09-27 11:09, Hans Verkuil wrote:
> Hi Marco,
>
> On 8/30/19 12:16 PM, Marco Felsch wrote:
> > Some connectors no matter if in- or output supports only a limited
> > range of sdtv standards. 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.
>
> I got this warnings:
>
> WARNING: DT binding docs and includes should be a separate patch. See:
> Documentation/devicetree/bindings/submitting-patches.txt
>
> I do think it makes sense to split this up into three patches: first
> adding the sdtv-standards.h, then the update to analog-tv-connector.txt
> and finally the videodev.h patch.
I knew this error but I ignored it becuase it mades no sense to me. I
can split it if you want.
Regards,
Marco
> Regards,
>
> Hans
>
> >
> > Signed-off-by: Marco Felsch
> > Reviewed-by: Rob Herring
> > ---
> > [1] https://patchwork.kernel.org/cover/10794703/
> >
> > v10:
> > - fix typo within comment s/TV_STD_*/SDTV_STD_*/
> >
> > v8:
> > Hi Rob,
> >
> > I dropped your r b tag becuase of the changes I made in this version.
> > Please can you have look on it again? Luckily this would be the last
> > time ;-)
> >
> > - move definition to include/dt-bindings/display
> > - rename tvnorms.h to sdtv-standards.h
> > - TVORMS_* -> SDTV_STD_*
> > - add sync comments
> > - adapt commit message
> > - fix bindings documentation
> >
> > 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 | 6 ++
> > include/dt-bindings/display/sdtv-standards.h | 76 +++
> > include/uapi/linux/videodev2.h| 4 +
> > 3 files changed, 86 insertions(+)
> > create mode 100644 include/dt-bindings/display/sdtv-standards.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..883bcb2604c7 100644
> > ---
> > a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> > +++
> > b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> > @@ -6,16 +6,22 @@ Required properties:
> >
> > Optional properties:
> > - label: a symbolic name for the connector
> > +- sdtv-standards: limit the supported TV standards on a connector to the
> > given
> > + ones. If not specified all TV standards are allowed.
> > + Possible TV standards are defined in
> > + include/dt-bindings/display/sdtv-standards.h.
> >
> > Required nodes:
> > - Video port for TV input
> >
> > Example
> > ---
> > +#include
> >
> > tv: connector {
> > compatible = "composite-video-connector";
> > label = "tv";
> > + sdtv-standards = <(SDTV_STD_PAL | SDTV_STD_NTSC)>;
> >
> > port {
> > tv_connector_in: endpoint {
> > diff --git a/include/dt-bindings/display/sdtv-standards.h
> > b/include/dt-bindings/display/sdtv-standards.h
> > new file mode 100644
> > index ..fbc1a3db2ea7
> > --- /dev/null
> > +++ b/include/dt-bindings/display/sdtv-standards.h
> > @@ -0,0 +1,76 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only or X11 */
> > +/*
> > + * Copyright 2019 Pengutronix, Marco Felsch
> > + */
> > +
> > +#ifndef _DT_BINDINGS_DISPLAY_SDTV_STDS_H
> > +#define _DT_BINDINGS_DISPLAY_SDTV_STDS_H
> > +
> > +/*
> > + * Attention: Keep the SDTV_STD_* bit definitions in sync with
> > + * include/uapi/linux/videodev2.h V4L2_STD_* bit definitions.
> > + */
> > +/* One bit for each standard */
> > +#define SDTV_STD_PAL_B 0x0001
> > +#define SDTV_STD_PAL_B10x0002
> > +#define SDTV_STD_PAL_G 0x0004
> > +#define SDTV_STD_PAL_H 0x0008
> > +#define SDTV_STD_PAL_I 0x0010
> > +#define SDTV_STD_PAL_D 0x0020
> > +#define SDTV_STD_PAL_D10x0040
> > +#define SDTV_STD_PAL_K 0x0080
> > +
> > +#define SDTV_STD_PAL (SDTV_STD_PAL_B | \
> > +SDTV_STD_PAL_B1| \
> > +SDTV_STD_PAL_G | \
> > +SDTV_STD_PAL_H | \
> > +
Re: [PATCH v10 01/14] dt-bindings: connector: analog: add sdtv standards property
Hi Marco,
On 8/30/19 12:16 PM, Marco Felsch wrote:
> Some connectors no matter if in- or output supports only a limited
> range of sdtv standards. 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.
I got this warnings:
WARNING: DT binding docs and includes should be a separate patch. See:
Documentation/devicetree/bindings/submitting-patches.txt
I do think it makes sense to split this up into three patches: first
adding the sdtv-standards.h, then the update to analog-tv-connector.txt
and finally the videodev.h patch.
Regards,
Hans
>
> Signed-off-by: Marco Felsch
> Reviewed-by: Rob Herring
> ---
> [1] https://patchwork.kernel.org/cover/10794703/
>
> v10:
> - fix typo within comment s/TV_STD_*/SDTV_STD_*/
>
> v8:
> Hi Rob,
>
> I dropped your r b tag becuase of the changes I made in this version.
> Please can you have look on it again? Luckily this would be the last
> time ;-)
>
> - move definition to include/dt-bindings/display
> - rename tvnorms.h to sdtv-standards.h
> - TVORMS_* -> SDTV_STD_*
> - add sync comments
> - adapt commit message
> - fix bindings documentation
>
> 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 | 6 ++
> include/dt-bindings/display/sdtv-standards.h | 76 +++
> include/uapi/linux/videodev2.h| 4 +
> 3 files changed, 86 insertions(+)
> create mode 100644 include/dt-bindings/display/sdtv-standards.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..883bcb2604c7 100644
> ---
> a/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> +++
> b/Documentation/devicetree/bindings/display/connector/analog-tv-connector.txt
> @@ -6,16 +6,22 @@ Required properties:
>
> Optional properties:
> - label: a symbolic name for the connector
> +- sdtv-standards: limit the supported TV standards on a connector to the
> given
> + ones. If not specified all TV standards are allowed.
> + Possible TV standards are defined in
> + include/dt-bindings/display/sdtv-standards.h.
>
> Required nodes:
> - Video port for TV input
>
> Example
> ---
> +#include
>
> tv: connector {
> compatible = "composite-video-connector";
> label = "tv";
> + sdtv-standards = <(SDTV_STD_PAL | SDTV_STD_NTSC)>;
>
> port {
> tv_connector_in: endpoint {
> diff --git a/include/dt-bindings/display/sdtv-standards.h
> b/include/dt-bindings/display/sdtv-standards.h
> new file mode 100644
> index ..fbc1a3db2ea7
> --- /dev/null
> +++ b/include/dt-bindings/display/sdtv-standards.h
> @@ -0,0 +1,76 @@
> +/* SPDX-License-Identifier: GPL-2.0-only or X11 */
> +/*
> + * Copyright 2019 Pengutronix, Marco Felsch
> + */
> +
> +#ifndef _DT_BINDINGS_DISPLAY_SDTV_STDS_H
> +#define _DT_BINDINGS_DISPLAY_SDTV_STDS_H
> +
> +/*
> + * Attention: Keep the SDTV_STD_* bit definitions in sync with
> + * include/uapi/linux/videodev2.h V4L2_STD_* bit definitions.
> + */
> +/* One bit for each standard */
> +#define SDTV_STD_PAL_B 0x0001
> +#define SDTV_STD_PAL_B1 0x0002
> +#define SDTV_STD_PAL_G 0x0004
> +#define SDTV_STD_PAL_H 0x0008
> +#define SDTV_STD_PAL_I 0x0010
> +#define SDTV_STD_PAL_D 0x0020
> +#define SDTV_STD_PAL_D1 0x0040
> +#define SDTV_STD_PAL_K 0x0080
> +
> +#define SDTV_STD_PAL (SDTV_STD_PAL_B | \
> + SDTV_STD_PAL_B1| \
> + SDTV_STD_PAL_G | \
> + SDTV_STD_PAL_H | \
> + SDTV_STD_PAL_I | \
> + SDTV_STD_PAL_D | \
> + SDTV_STD_PAL_D1| \
> + SDTV_STD_PAL_K)
> +
> +#define SDTV_STD_PAL_M 0x0100
> +#define SDTV_STD_PAL_N 0x0200
> +#define SDTV_STD_PAL_Nc 0x0400
> +#define SDTV_STD_PAL_60
