Hi Rob, On Tue, Mar 01, 2022 at 09:04:10AM -0600, Rob Herring wrote: > On Sun, Feb 27, 2022 at 11:16:28PM +0200, Laurent Pinchart wrote: > > On Sun, Feb 27, 2022 at 11:07:23PM +0200, Sakari Ailus wrote: > > > On Sun, Feb 27, 2022 at 10:33:51PM +0200, Laurent Pinchart wrote: > > > > Add a new dt-bindings/media/video-interfaces.h header that defines > > > > macros corresponding to the bus types from media/video-interfaces.yaml. > > > > This allows avoiding hardcoded constants in device tree sources. > > > > > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > > --- > > > > include/dt-bindings/media/video-interfaces.h | 16 ++++++++++++++++ > > > > 1 file changed, 16 insertions(+) > > > > create mode 100644 include/dt-bindings/media/video-interfaces.h > > > > > > > > diff --git a/include/dt-bindings/media/video-interfaces.h b/include/dt-bindings/media/video-interfaces.h > > > > new file mode 100644 > > > > index 000000000000..e38058e1cca7 > > > > --- /dev/null > > > > +++ b/include/dt-bindings/media/video-interfaces.h > > > > @@ -0,0 +1,16 @@ > > > > +/* SPDX-License-Identifier: GPL-2.0-only */ > > > > +/* > > > > + * Copyright (C) 2022 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > > > > + */ > > > > + > > > > +#ifndef __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ > > > > +#define __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ > > > > + > > > > +#define MEDIA_BUS_TYPE_CSI2_CPHY 1 > > > > +#define MEDIA_BUS_TYPE_CSI1 2 > > > > +#define MEDIA_BUS_TYPE_CCP2 3 > > > > +#define MEDIA_BUS_TYPE_CSI2_DPHY 4 > > > > +#define MEDIA_BUS_TYPE_PARALLEL 5 > > > > > > I've been long thinkin of renaming "PARALLEL" as "BT.601" which it really > > > is. I don't mind postponing that, but I think you could as well start here. > > > Up to you. > > > > I think it's a good idea, but we then need to decide what to do with > > other types of parallel buses. Let's start this discussion now, and > > implement it in a patch on top of this series. > > 5 and what it means is an ABI. If it is ambiguous and needs to be more > specific, then you need new numbers for all of those specific types. > > If it is just a rename, I prefer it is done from the start. It's both :-) It's ambiguous, but only used to refer to BT.601-liked buses today in mainline, so I'll rename it. The number may be used to refer to different types of parallel buses out-of-tree, and we can add new types for that in mainline later when/if needed. > > > Should this be somehow visible in video-interfaces.yaml? > > > > I wish we could use macros in .yaml files instead of numerical values, > > but I don't think that's possible. I can do this: > > > > bus-type: > > $ref: /schemas/types.yaml#/definitions/uint32 > > enum: > > - - 1 # MIPI CSI-2 C-PHY > > - - 2 # MIPI CSI1 > > - - 3 # CCP2 > > - - 4 # MIPI CSI-2 D-PHY > > - - 5 # Parallel > > - - 6 # BT.656 > > + - 1 # MIPI CSI-2 C-PHY (MEDIA_BUS_TYPE_CSI2_CPHY) > > + - 2 # MIPI CSI1 (MEDIA_BUS_TYPE_CSI1) > > + - 3 # CCP2 (MEDIA_BUS_TYPE_CCP2) > > + - 4 # MIPI CSI-2 D-PHY (MEDIA_BUS_TYPE_CSI2_DPHY) > > + - 5 # Parallel (MEDIA_BUS_TYPE_PARALLEL) > > + - 6 # BT.656 (MEDIA_BUS_TYPE_BT656) > > Seems a bit redundant to have both comment text and define. The only > part missing from the defines is 'MIPI'. I agree. I'll use the macros. It would be nice if macros could be used instead of numerical values in the YAML schema, but that's certainly not high on the wishlist. > > description: > > - Data bus type. > > + Data bus type. Use the macros listed above (defined in > > + dt-bindings/video-interfaces.h) instead of numerical values. > > > > Any better proposal ? > > > > > > +#define MEDIA_BUS_TYPE_BT656 6 > > > > + > > > > +#endif /* __DT_BINDINGS_MEDIA_VIDEO_INTERFACES_H__ */ -- Regards, Laurent Pinchart
