From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: [PATCH v3 2/4] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver Date: Tue, 21 Nov 2017 11:26:19 -0800 Message-ID: <877euje8mc.fsf@anholt.net> References: <1814950930.414004.1506062733728@email.1und1.de> <20170927215124.6k3j54qf2qscnzc2@rob-hp-laptop> Mime-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Return-path: In-Reply-To: Sender: devicetree-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Dave Stevenson , Rob Herring Cc: Stefan Wahren , Mauro Carvalho Chehab , linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Sakari Ailus , Hans Verkuil , linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org --=-=-= Content-Type: text/plain Dave Stevenson writes: > Hi Rob > > On 27 September 2017 at 22:51, Rob Herring wrote: >> On Fri, Sep 22, 2017 at 05:07:22PM +0100, Dave Stevenson wrote: >>> Hi Stefan >>> >>> On 22 September 2017 at 07:45, Stefan Wahren wrote: >>> > Hi Dave, >>> > >>> >> Dave Stevenson hat am 20. September 2017 um 18:07 geschrieben: >>> >> >>> >> >>> >> Document the DT bindings for the CSI2/CCP2 receiver peripheral >>> >> (known as Unicam) on BCM283x SoCs. >>> >> >>> >> Signed-off-by: Dave Stevenson >>> >> --- >>> >> >>> >> Changes since v2 >>> >> - Removed all references to Linux drivers. >>> >> - Reworded section about disabling the firmware driver. >>> >> - Renamed clock from "lp_clock" to "lp" in description and example. >>> >> - Referred to video-interfaces.txt and stated requirements on remote-endpoint >>> >> and data-lanes. >>> >> - Corrected typo in example from csi to csi1. >>> >> - Removed unnecessary #address-cells and #size-cells in example. >>> >> - Removed setting of status from the example. >>> >> >>> >> .../devicetree/bindings/media/bcm2835-unicam.txt | 85 ++++++++++++++++++++++ >>> >> 1 file changed, 85 insertions(+) >>> >> create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt >>> >> >>> >> diff --git a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt >>> >> new file mode 100644 >>> >> index 0000000..7714fb3 >>> >> --- /dev/null >>> >> +++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt >>> >> @@ -0,0 +1,85 @@ >>> >> +Broadcom BCM283x Camera Interface (Unicam) >>> >> +------------------------------------------ >>> >> + >>> >> +The Unicam block on BCM283x SoCs is the receiver for either >>> >> +CSI-2 or CCP2 data from image sensors or similar devices. >>> >> + >>> >> +The main platform using this SoC is the Raspberry Pi family of boards. >>> >> +On the Pi the VideoCore firmware can also control this hardware block, >>> >> +and driving it from two different processors will cause issues. >>> >> +To avoid this, the firmware checks the device tree configuration >>> >> +during boot. If it finds device tree nodes called csi0 or csi1 then >>> >> +it will stop the firmware accessing the block, and it can then >>> >> +safely be used via the device tree binding. >>> >> + >>> >> +Required properties: >>> >> +=================== >>> >> +- compatible : must be "brcm,bcm2835-unicam". >>> >> +- reg : physical base address and length of the register sets for the >>> >> + device. >>> >> +- interrupts : should contain the IRQ line for this Unicam instance. >>> >> +- clocks : list of clock specifiers, corresponding to entries in >>> >> + clock-names property. >>> >> +- clock-names : must contain an "lp" entry, matching entries in the >>> >> + clocks property. >>> >> + >>> >> +Unicam supports a single port node. It should contain one 'port' child node >>> >> +with child 'endpoint' node. Please refer to the bindings defined in >>> >> +Documentation/devicetree/bindings/media/video-interfaces.txt. >>> >> + >>> >> +Within the endpoint node the "remote-endpoint" and "data-lanes" properties >>> >> +are mandatory. >>> >> +Data lane reordering is not supported so the data lanes must be in order, >>> >> +starting at 1. The number of data lanes should represent the number of >>> >> +usable lanes for the hardware block. That may be limited by either the SoC or >>> >> +how the platform presents the interface, and the lower value must be used. >>> >> + >>> >> +Lane reordering is not supported on the clock lane either, so the optional >>> >> +property "clock-lane" will implicitly be <0>. >>> >> +Similarly lane inversion is not supported, therefore "lane-polarities" will >>> >> +implicitly be <0 0 0 0 0>. >>> >> +Neither of these values will be checked. >>> >> + >>> >> +Example: >>> >> + csi1: csi1@7e801000 { >>> >> + compatible = "brcm,bcm2835-unicam"; >>> >> + reg = <0x7e801000 0x800>, >>> >> + <0x7e802004 0x4>; >>> > >>> > sorry, i didn't noticed this before. I'm afraid this is using a small range of the CMI. Are there possible other users of this range? Does it make sense to handle this by a separate clock driver? >>> >>> CMI (Clock Manager Image) consists of a total of 4 registers. >>> 0x7e802000 is CMI_CAM0, with only bits 0-5 used for gating and >>> inversion of the clock and data lanes (2 data lanes available on >>> CAM0). >>> 0x7e802004 is CMI_CAM1, with only bits 0-9 used for gating and >>> inversion of the clock and data lanes (4 data lanes available on >>> CAM1). >>> 0x7e802008 is CMI_CAMTEST which I have no documentation or drivers for. >>> 0x7e802010 is CMI_USBCTL. Only bit 6 is documented and is a reset. The >>> default value is the required value. Nothing touches it that I can >>> find. >>> >>> The range listed only covers the one register associated with that >>> Unicam instance, so no other users. The other two aren't touched. >>> Do 16 active register bits solely for camera clock gating really >>> warrant a full clock driver? >> >> You should describe all the registers in DT, not just what the driver >> (currently) uses. > > I'm not clear what you're asking for here. > > This binding is for the Unicam block, not for CMI (Clock Manager > Imaging). In order for a Unicam instance to work, it needs to enable > the relevant clock gating via 1 CMI register, and it will only ever be > one register. Rob, the CMI just a small bit of glue required by the crossing of a power domain in a unicam instance, and the two unicam instances in this HW have their CMI regs next to each other. It's not really a separate block, and I think describing the unicam's CMI reg in the unicam binding is appropriate. What would you need from Dave to ack this binding? --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAloUfdsACgkQtdYpNtH8 nujSpQ//YCu66AFK9LhCX0IOxXyLYNsz4H8apQSg97UxQmwwitO1Ishdg40viXKQ 7J7e0R68nduoTSaLEz+TIbOsXJ2gogyR7yVzanuoy2EFOXD8SkyVbvbGvV1qbvOE crTU0iqBdmXiKbz+P9J2btDJPWqoDnaZuI/xBRCplozsrBAtN74244gI4wKYj8mE pYxHjmIf7cHBE1KJopzpjK3GDhP2J9a5JhqpHDX1EG+9g+3gGBWY8s5Ostka2MJb 6bGdZv5Qu44fF/x9LSj5ILJt6O4lMnDmvj13sLh/qLFMLDzM6yq9HAyMbL4msbQO KNRspUA4Zh4NU0eVtHufcrf9X6DYDVfrQp/VEhG9K9qStTQgfDOobbZWIVi/VwFJ 3kt4CUV+vN70AFfbILX1W+Ik+nPSpZF3A+6QDhyFj4by3bKRqGd583jGU1Rkvw2Q IOjZApSFzaHWiQj3lwQytw+UHW+SGWgfa1vNI1OsoTBmwSLcrctPNVEnbDo2HLer AFkQ2/mhcNgpz9VEsdkbBMyqN/c2KXquZZHzPMWW4Z75hR10B1YBEktJnYfPHGLw 8f19kqgnOfcjeAF+FqLOxGvQzL5fFLWY1970PzMgtn+oJ5KqyDy81EIn/fw0goNj x0Gaq3agfQN+ssxW0B3ZUz15GRGb5gjwD0rByaW1mojDv5BUZ5c= =W2Sf -----END PGP SIGNATURE----- --=-=-=-- -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from anholt.net ([50.246.234.109]:45890 "EHLO anholt.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751154AbdKUT0X (ORCPT ); Tue, 21 Nov 2017 14:26:23 -0500 From: Eric Anholt To: Dave Stevenson , Rob Herring Cc: Stefan Wahren , Mauro Carvalho Chehab , linux-media@vger.kernel.org, Sakari Ailus , Hans Verkuil , linux-rpi-kernel@lists.infradead.org, devicetree@vger.kernel.org Subject: Re: [PATCH v3 2/4] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver In-Reply-To: References: <1814950930.414004.1506062733728@email.1und1.de> <20170927215124.6k3j54qf2qscnzc2@rob-hp-laptop> Date: Tue, 21 Nov 2017 11:26:19 -0800 Message-ID: <877euje8mc.fsf@anholt.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Sender: linux-media-owner@vger.kernel.org List-ID: --=-=-= Content-Type: text/plain Dave Stevenson writes: > Hi Rob > > On 27 September 2017 at 22:51, Rob Herring wrote: >> On Fri, Sep 22, 2017 at 05:07:22PM +0100, Dave Stevenson wrote: >>> Hi Stefan >>> >>> On 22 September 2017 at 07:45, Stefan Wahren wrote: >>> > Hi Dave, >>> > >>> >> Dave Stevenson hat am 20. September 2017 um 18:07 geschrieben: >>> >> >>> >> >>> >> Document the DT bindings for the CSI2/CCP2 receiver peripheral >>> >> (known as Unicam) on BCM283x SoCs. >>> >> >>> >> Signed-off-by: Dave Stevenson >>> >> --- >>> >> >>> >> Changes since v2 >>> >> - Removed all references to Linux drivers. >>> >> - Reworded section about disabling the firmware driver. >>> >> - Renamed clock from "lp_clock" to "lp" in description and example. >>> >> - Referred to video-interfaces.txt and stated requirements on remote-endpoint >>> >> and data-lanes. >>> >> - Corrected typo in example from csi to csi1. >>> >> - Removed unnecessary #address-cells and #size-cells in example. >>> >> - Removed setting of status from the example. >>> >> >>> >> .../devicetree/bindings/media/bcm2835-unicam.txt | 85 ++++++++++++++++++++++ >>> >> 1 file changed, 85 insertions(+) >>> >> create mode 100644 Documentation/devicetree/bindings/media/bcm2835-unicam.txt >>> >> >>> >> diff --git a/Documentation/devicetree/bindings/media/bcm2835-unicam.txt b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt >>> >> new file mode 100644 >>> >> index 0000000..7714fb3 >>> >> --- /dev/null >>> >> +++ b/Documentation/devicetree/bindings/media/bcm2835-unicam.txt >>> >> @@ -0,0 +1,85 @@ >>> >> +Broadcom BCM283x Camera Interface (Unicam) >>> >> +------------------------------------------ >>> >> + >>> >> +The Unicam block on BCM283x SoCs is the receiver for either >>> >> +CSI-2 or CCP2 data from image sensors or similar devices. >>> >> + >>> >> +The main platform using this SoC is the Raspberry Pi family of boards. >>> >> +On the Pi the VideoCore firmware can also control this hardware block, >>> >> +and driving it from two different processors will cause issues. >>> >> +To avoid this, the firmware checks the device tree configuration >>> >> +during boot. If it finds device tree nodes called csi0 or csi1 then >>> >> +it will stop the firmware accessing the block, and it can then >>> >> +safely be used via the device tree binding. >>> >> + >>> >> +Required properties: >>> >> +=================== >>> >> +- compatible : must be "brcm,bcm2835-unicam". >>> >> +- reg : physical base address and length of the register sets for the >>> >> + device. >>> >> +- interrupts : should contain the IRQ line for this Unicam instance. >>> >> +- clocks : list of clock specifiers, corresponding to entries in >>> >> + clock-names property. >>> >> +- clock-names : must contain an "lp" entry, matching entries in the >>> >> + clocks property. >>> >> + >>> >> +Unicam supports a single port node. It should contain one 'port' child node >>> >> +with child 'endpoint' node. Please refer to the bindings defined in >>> >> +Documentation/devicetree/bindings/media/video-interfaces.txt. >>> >> + >>> >> +Within the endpoint node the "remote-endpoint" and "data-lanes" properties >>> >> +are mandatory. >>> >> +Data lane reordering is not supported so the data lanes must be in order, >>> >> +starting at 1. The number of data lanes should represent the number of >>> >> +usable lanes for the hardware block. That may be limited by either the SoC or >>> >> +how the platform presents the interface, and the lower value must be used. >>> >> + >>> >> +Lane reordering is not supported on the clock lane either, so the optional >>> >> +property "clock-lane" will implicitly be <0>. >>> >> +Similarly lane inversion is not supported, therefore "lane-polarities" will >>> >> +implicitly be <0 0 0 0 0>. >>> >> +Neither of these values will be checked. >>> >> + >>> >> +Example: >>> >> + csi1: csi1@7e801000 { >>> >> + compatible = "brcm,bcm2835-unicam"; >>> >> + reg = <0x7e801000 0x800>, >>> >> + <0x7e802004 0x4>; >>> > >>> > sorry, i didn't noticed this before. I'm afraid this is using a small range of the CMI. Are there possible other users of this range? Does it make sense to handle this by a separate clock driver? >>> >>> CMI (Clock Manager Image) consists of a total of 4 registers. >>> 0x7e802000 is CMI_CAM0, with only bits 0-5 used for gating and >>> inversion of the clock and data lanes (2 data lanes available on >>> CAM0). >>> 0x7e802004 is CMI_CAM1, with only bits 0-9 used for gating and >>> inversion of the clock and data lanes (4 data lanes available on >>> CAM1). >>> 0x7e802008 is CMI_CAMTEST which I have no documentation or drivers for. >>> 0x7e802010 is CMI_USBCTL. Only bit 6 is documented and is a reset. The >>> default value is the required value. Nothing touches it that I can >>> find. >>> >>> The range listed only covers the one register associated with that >>> Unicam instance, so no other users. The other two aren't touched. >>> Do 16 active register bits solely for camera clock gating really >>> warrant a full clock driver? >> >> You should describe all the registers in DT, not just what the driver >> (currently) uses. > > I'm not clear what you're asking for here. > > This binding is for the Unicam block, not for CMI (Clock Manager > Imaging). In order for a Unicam instance to work, it needs to enable > the relevant clock gating via 1 CMI register, and it will only ever be > one register. Rob, the CMI just a small bit of glue required by the crossing of a power domain in a unicam instance, and the two unicam instances in this HW have their CMI regs next to each other. It's not really a separate block, and I think describing the unicam's CMI reg in the unicam binding is appropriate. What would you need from Dave to ack this binding? --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAloUfdsACgkQtdYpNtH8 nujSpQ//YCu66AFK9LhCX0IOxXyLYNsz4H8apQSg97UxQmwwitO1Ishdg40viXKQ 7J7e0R68nduoTSaLEz+TIbOsXJ2gogyR7yVzanuoy2EFOXD8SkyVbvbGvV1qbvOE crTU0iqBdmXiKbz+P9J2btDJPWqoDnaZuI/xBRCplozsrBAtN74244gI4wKYj8mE pYxHjmIf7cHBE1KJopzpjK3GDhP2J9a5JhqpHDX1EG+9g+3gGBWY8s5Ostka2MJb 6bGdZv5Qu44fF/x9LSj5ILJt6O4lMnDmvj13sLh/qLFMLDzM6yq9HAyMbL4msbQO KNRspUA4Zh4NU0eVtHufcrf9X6DYDVfrQp/VEhG9K9qStTQgfDOobbZWIVi/VwFJ 3kt4CUV+vN70AFfbILX1W+Ik+nPSpZF3A+6QDhyFj4by3bKRqGd583jGU1Rkvw2Q IOjZApSFzaHWiQj3lwQytw+UHW+SGWgfa1vNI1OsoTBmwSLcrctPNVEnbDo2HLer AFkQ2/mhcNgpz9VEsdkbBMyqN/c2KXquZZHzPMWW4Z75hR10B1YBEktJnYfPHGLw 8f19kqgnOfcjeAF+FqLOxGvQzL5fFLWY1970PzMgtn+oJ5KqyDy81EIn/fw0goNj x0Gaq3agfQN+ssxW0B3ZUz15GRGb5gjwD0rByaW1mojDv5BUZ5c= =W2Sf -----END PGP SIGNATURE----- --=-=-=--