| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 27 Oct 2020 10:52:21 +0100
From: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
To: Maxime Ripard <maxime@...no.tech>
Cc: linux-media@...r.kernel.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
devel@...verdev.osuosl.org, linux-sunxi@...glegroups.com,
Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh+dt@...nel.org>, Chen-Yu Tsai <wens@...e.org>,
Yong Deng <yong.deng@...ewell.com>,
Kishon Vijay Abraham I <kishon@...com>,
Vinod Koul <vkoul@...nel.org>,
Helen Koike <helen.koike@...labora.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Hans Verkuil <hans.verkuil@...co.com>,
Sakari Ailus <sakari.ailus@...ux.intel.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
Hans Verkuil <hverkuil@...all.nl>, kevin.lhopital@...mail.com
Subject: Re: [PATCH 07/14] dt-bindings: media: i2c: Add A31 MIPI CSI-2
bindings documentation
Hi,
On Mon 26 Oct 20, 17:14, Maxime Ripard wrote:
> i2c? :)
Oops, good catch!
> On Fri, Oct 23, 2020 at 07:45:39PM +0200, Paul Kocialkowski wrote:
> > This introduces YAML bindings documentation for the A31 MIPI CSI-2
> > controller.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@...tlin.com>
> > ---
> > .../media/allwinner,sun6i-a31-mipi-csi2.yaml | 168 ++++++++++++++++++
> > 1 file changed, 168 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > new file mode 100644
> > index 000000000000..9adc0bc27033
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/allwinner,sun6i-a31-mipi-csi2.yaml
> > @@ -0,0 +1,168 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/media/allwinner,sun6i-a31-mipi-csi2.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allwinner A31 MIPI CSI-2 Device Tree Bindings
> > +
> > +maintainers:
> > + - Paul Kocialkowski <paul.kocialkowski@...tlin.com>
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - const: allwinner,sun6i-a31-mipi-csi2
> > + - items:
> > + - const: allwinner,sun8i-v3s-mipi-csi2
> > + - const: allwinner,sun6i-a31-mipi-csi2
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + clocks:
> > + items:
> > + - description: Bus Clock
> > + - description: Module Clock
> > +
> > + clock-names:
> > + items:
> > + - const: bus
> > + - const: mod
> > +
> > + phys:
> > + items:
> > + - description: MIPI D-PHY
> > +
> > + phy-names:
> > + items:
> > + - const: dphy
> > +
> > + resets:
> > + maxItems: 1
> > +
> > + # See ./video-interfaces.txt for details
> > + ports:
> > + type: object
> > +
> > + properties:
> > + port@0:
> > + type: object
> > + description: Input port, connect to a MIPI CSI-2 sensor
> > +
> > + properties:
> > + reg:
> > + const: 0
> > +
> > + endpoint:
> > + type: object
> > +
> > + properties:
> > + remote-endpoint: true
> > +
> > + bus-type:
> > + const: 4
> > +
> > + clock-lanes:
> > + maxItems: 1
> > +
> > + data-lanes:
> > + minItems: 1
> > + maxItems: 4
> > +
> > + required:
> > + - bus-type
> > + - data-lanes
> > + - remote-endpoint
> > +
> > + additionalProperties: false
> > +
> > + required:
> > + - endpoint
> > +
> > + additionalProperties: false
> > +
> > + port@1:
> > + type: object
> > + description: Output port, connect to a CSI controller
> > +
> > + properties:
> > + reg:
> > + const: 1
> > +
> > + endpoint:
> > + type: object
> > +
> > + properties:
> > + remote-endpoint: true
> > +
> > + bus-type:
> > + const: 4
>
> That one seems a bit weird. If the input and output ports are using the
> same format, what is that "bridge" supposed to be doing?
Fair enough. What this represents is the internal link (likely a FIFO) between
the two controllers. It is definitely not a MIPI CSI-2 bus but there's no
mbus type for an internal link (probably because it's not a bus after all).
Note that on the CSI controller side, we need the bus-type to be set to 4 for it
to properly select the MIPI CSI-2 input. So it just felt more logical to have
the same on the other side of the endpoint. On the other hand, we can just
remove it on the MIPI CSI-2 controller side since it won't check it and have it
fallback to the unknown mbus type.
But that would make the types inconsistent on the two sides of the link.
I don't think V4L2 will complain about it at the moment, but it would also make
sense that it does eventually.
What do you think?
> > + additionalProperties: false
> > +
> > + required:
> > + - endpoint
> > +
> > + additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - interrupts
> > + - clocks
> > + - clock-names
> > + - resets
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/clock/sun8i-v3s-ccu.h>
> > + #include <dt-bindings/reset/sun8i-v3s-ccu.h>
> > +
> > + mipi_csi2: mipi-csi2@...1000 {
>
> The unit name should be pretty standard, with the list here:
>
> https://github.com/devicetree-org/devicetree-specification/blob/master/source/chapter2-devicetree-basics.rst#generic-names-recommendation
>
> there's nothing really standing out for us in that list, but given that
> there's dsi, we should stick with csi
Then what really surprises me is that the CSI controllers are called "camera",
not "csi". If "camera" is supposed to cover both image sensor and camera sensor
interfaces, it would probably fit MIPI CSI-2 as well.
I see lots of names with -controller for controllers with specific devices
attached, like "nand-controller" or "lcd-controller". Maybe using
"camera-controller" for the CSI and MIPI CSI-2 controllers would make the most
sense, while keeping "camera" for the actual image sensors.
What do you think?
Cheers,
Paul
--
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists