Message ID | fae3d29bba67825030c0077dd9c79534b6888512.1505916622.git.dave.stevenson@raspberrypi.org (mailing list archive) |
---|---|
State | Changes Requested, archived |
Delegated to: | Hans Verkuil |
Headers |
Received: from vger.kernel.org ([209.132.180.67]) by www.linuxtv.org with esmtp (Exim 4.84_2) (envelope-from <linux-media-owner@vger.kernel.org>) id 1duhYO-0002tc-Cz; Wed, 20 Sep 2017 16:08:48 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751813AbdITQIq (ORCPT <rfc822;mkrufky@linuxtv.org> + 1 other); Wed, 20 Sep 2017 12:08:46 -0400 Received: from mx08-00252a01.pphosted.com ([91.207.212.211]:41826 "EHLO mx08-00252a01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751683AbdITQIo (ORCPT <rfc822;linux-media@vger.kernel.org>); Wed, 20 Sep 2017 12:08:44 -0400 Received: from pps.filterd (m0102629.ppops.net [127.0.0.1]) by mx08-00252a01.pphosted.com (8.16.0.21/8.16.0.21) with SMTP id v8KG8WDf006761 for <linux-media@vger.kernel.org>; Wed, 20 Sep 2017 17:08:43 +0100 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.org; h=from : to : cc : subject : date : message-id : in-reply-to : references : in-reply-to : references; s=pp; bh=lfPIlJ03XB/Fn5G9rnrHV4Fk11TTZiWkqgzo7w6fKow=; b=tEp7RHBg00CFxsF8GoARBHQ6Cs/nse0/I0ZrM+GhQ7UkFb5hkHMYk8zt1OSNxb9BycFm F7up4XRQ2PT33ALEcpkR7Z5iIYk3xvlqIFhv4a9Y9Z+/25kXzzT/pw4ltx7wAtYQIJzq RxlcvQQl6whGshnUJzBQIa4Q0iEI9xzjG5sOvgVp93nFp2nLK6B8/AcYDjqe694t0cek rdZnlTQ/+/cLMhWpXpKVrwui0PmEOwqdStv8Bb7+y1Wj9/2wIY59QgZtimDojIsEGTPb o7nLCL7Bw5CkpOrOuBSe9lgSiB0Tss97x3KyuPh+zpVRn7GqSI+5rsjw+uwKnZqnZVYL yQ== Received: from mail-wm0-f69.google.com (mail-wm0-f69.google.com [74.125.82.69]) by mx08-00252a01.pphosted.com with ESMTP id 2d0reg260q-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=OK) for <linux-media@vger.kernel.org>; Wed, 20 Sep 2017 17:08:43 +0100 Received: by mail-wm0-f69.google.com with SMTP id u138so3289718wmu.2 for <linux-media@vger.kernel.org>; Wed, 20 Sep 2017 09:08:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi-org.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=lfPIlJ03XB/Fn5G9rnrHV4Fk11TTZiWkqgzo7w6fKow=; b=Lo7KzYzaEbMvrLg09oyz8vrEhl40nk/XkTB+j08XLzzJ8rJ1Y5R7iVCCVqZhNyQfBo 1bGP/xlX3hW8H0PJKjPVX2gBvP5wqvsp3LfwG5EvMYc8Y6xrjFqhS8YwlNNKApFO92UK QL5z0uck33mPKvBNdD2YApA7JNgm/mSCdkMuZrn2MK0l2VwDHfSXPOUzuDPnSypTfFzq JqGPQtZpu9jLCct73snJRu98WZ42kOUsE+6Ovk6+fJEyfVqp6kIZnurt14//H3SKAiQ6 b+aUMbfDLGj0zTHyrwItF3+5q5wDv5V3DPvpktnfxL77KY1zIARJZzJEmRY1sq0a6xOV tHUA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=lfPIlJ03XB/Fn5G9rnrHV4Fk11TTZiWkqgzo7w6fKow=; b=Czy79xA+NKrVfI6ICWbFOhEQ6V7bG4u200SMbAPFshLMK1t7JYVLk908mWP7zBHxog QD4ngoDv822ymZvuHAkLxmAUqDuPrdF+wvuM8fZCtSbDZywZaHxmWwcbyXYGXP368RnR W9a8W7XK3dmqeJdomo+Z6B+e8+TAb+tv+fWvKsF2I83Fyk57xhezziFEcrCraa09rytp 40WT0xTVAN4pAQ6+CEf+kcuBIXJptzs29RhqfhjiWXBk5pYXQqYxRnj1NAyWixbZDHaS g6YmXabQ4sLeo8Vtkla6RFFbY5BNf1Kw1Cx2/MrxAhtIpPlgDBjqDqucaIJzhlYOi9mH cOBg== X-Gm-Message-State: AHPjjUh1DLghzGSNoPOQtgyDy5XxlUUEDMd2T66KkL+PIA7DisQ7fuhX xBA7tr3/ImxoieG1guYcP9ZotcgaJQLQz6xpJSnVuPRoShniOvCktA5yGA6nrqRIo5q+A1Y+0WE +dLO3e0rDTXCmBaRfYQWVIQ== X-Received: by 10.28.57.215 with SMTP id g206mr4756032wma.117.1505923723024; Wed, 20 Sep 2017 09:08:43 -0700 (PDT) X-Google-Smtp-Source: AOwi7QC8IC69gr2U9RLnDtpSIEG/wcnO7Cr7M1RQBkPwl6yhmeHONgn63qjZ8WgPaZA1Ab1SjrdpTg== X-Received: by 10.28.57.215 with SMTP id g206mr4756021wma.117.1505923722772; Wed, 20 Sep 2017 09:08:42 -0700 (PDT) Received: from dave-VirtualBox.pitowers.org ([217.33.127.173]) by smtp.googlemail.com with ESMTPSA id p15sm1937467wmi.2.2017.09.20.09.08.41 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 20 Sep 2017 09:08:42 -0700 (PDT) From: Dave Stevenson <dave.stevenson@raspberrypi.org> To: Mauro Carvalho Chehab <mchehab@kernel.org>, Rob Herring <robh+dt@kernel.org>, Hans Verkuil <hans.verkuil@cisco.com>, Eric Anholt <eric@anholt.net>, Stefan Wahren <stefan.wahren@i2se.com>, Sakari Ailus <sakari.ailus@iki.fi>, linux-media@vger.kernel.org, linux-rpi-kernel@lists.infradead.org, devicetree@vger.kernel.org Cc: Dave Stevenson <dave.stevenson@raspberrypi.org> Subject: [PATCH v3 2/4] [media] dt-bindings: Document BCM283x CSI2/CCP2 receiver Date: Wed, 20 Sep 2017 17:07:55 +0100 Message-Id: <fae3d29bba67825030c0077dd9c79534b6888512.1505916622.git.dave.stevenson@raspberrypi.org> X-Mailer: git-send-email 2.7.4 In-Reply-To: <cover.1505916622.git.dave.stevenson@raspberrypi.org> References: <cover.1505916622.git.dave.stevenson@raspberrypi.org> In-Reply-To: <cover.1505916622.git.dave.stevenson@raspberrypi.org> References: <cover.1505916622.git.dave.stevenson@raspberrypi.org> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:, , definitions=2017-09-20_04:, , signatures=0 X-Proofpoint-Spam-Details: rule=outbound_spam_notspam policy=outbound_spam score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1709200217 Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org |
Commit Message
Dave Stevenson
Sept. 20, 2017, 4:07 p.m. UTC
Document the DT bindings for the CSI2/CCP2 receiver peripheral
(known as Unicam) on BCM283x SoCs.
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
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
Comments
Hi Dave, > Dave Stevenson <dave.stevenson@raspberrypi.org> 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 <dave.stevenson@raspberrypi.org> > --- > > 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? Regards
Hi Stefan On 22 September 2017 at 07:45, Stefan Wahren <stefan.wahren@i2se.com> wrote: > Hi Dave, > >> Dave Stevenson <dave.stevenson@raspberrypi.org> 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 <dave.stevenson@raspberrypi.org> >> --- >> >> 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? Dave
Hi Dave, > Dave Stevenson <dave.stevenson@raspberrypi.org> hat am 22. September 2017 um 18:07 geschrieben: > > > Hi Stefan > > On 22 September 2017 at 07:45, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > Hi Dave, > > > >> Dave Stevenson <dave.stevenson@raspberrypi.org> 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 <dave.stevenson@raspberrypi.org> > >> --- > >> > >> 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. thanks for providing these information. > > 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? As long as there no new driver in the future, which also needs to touch these gates i'm fine. Stefan > > Dave
Dave Stevenson <dave.stevenson@raspberrypi.org> writes: > Hi Stefan > > On 22 September 2017 at 07:45, Stefan Wahren <stefan.wahren@i2se.com> wrote: >> Hi Dave, >> >>> Dave Stevenson <dave.stevenson@raspberrypi.org> 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 <dave.stevenson@raspberrypi.org> >>> --- >>> >>> 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. Yeah, my conclusion previously was that it's appropriate to consider this register as part of the unicam instance. No other HW block could want to talk to it.
On Fri, Sep 22, 2017 at 05:07:22PM +0100, Dave Stevenson wrote: > Hi Stefan > > On 22 September 2017 at 07:45, Stefan Wahren <stefan.wahren@i2se.com> wrote: > > Hi Dave, > > > >> Dave Stevenson <dave.stevenson@raspberrypi.org> 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 <dave.stevenson@raspberrypi.org> > >> --- > >> > >> 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. Rob
Hi Rob On 27 September 2017 at 22:51, Rob Herring <robh@kernel.org> 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 <stefan.wahren@i2se.com> wrote: >> > Hi Dave, >> > >> >> Dave Stevenson <dave.stevenson@raspberrypi.org> 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 <dave.stevenson@raspberrypi.org> >> >> --- >> >> >> >> 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. Eric and Stefan as the platform maintainers have already both acknowledged that a full clock driver for the CMI block is not warranted - the only user would be this driver, and the other registers in the CMI block are not useful. Do you want a random text description of a different block within this binding? Or are you requesting a full clock driver for the CMI block? Or that all Unicam instances should be mapping the whole 4 register range of CMI, and then somehow working out which register within that block they should be mapping? Regards, Dave
Dave Stevenson <dave.stevenson@raspberrypi.org> writes: > Hi Rob > > On 27 September 2017 at 22:51, Rob Herring <robh@kernel.org> 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 <stefan.wahren@i2se.com> wrote: >>> > Hi Dave, >>> > >>> >> Dave Stevenson <dave.stevenson@raspberrypi.org> 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 <dave.stevenson@raspberrypi.org> >>> >> --- >>> >> >>> >> 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?
On Tue, Nov 21, 2017 at 1:26 PM, Eric Anholt <eric@anholt.net> wrote: > Dave Stevenson <dave.stevenson@raspberrypi.org> writes: > >> Hi Rob >> >> On 27 September 2017 at 22:51, Rob Herring <robh@kernel.org> 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 <stefan.wahren@i2se.com> wrote: >>>> > Hi Dave, >>>> > >>>> >> Dave Stevenson <dave.stevenson@raspberrypi.org> 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 <dave.stevenson@raspberrypi.org> >>>> >> --- >>>> >> >>>> >> 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? Sorry, had started to reply on this and got distracted. I guess since there seems to be only 1 other bit that could possibly be used (CMI_USBCTL) it is fine like this. However, my concern would be if the CMI registers are integrated in a different way in some future chip that has the same unicam instances. Or just if the register bits are rearranged. Those are not an uncommon occurrence. You would have to provide access to those registers in some other way. It can be dealt with, but then you have to support both cases in the driver. If you all are fine with that, then: Acked-by: Rob Herring <robh@kernel.org>
Rob Herring <robh@kernel.org> writes: > On Tue, Nov 21, 2017 at 1:26 PM, Eric Anholt <eric@anholt.net> wrote: >> Dave Stevenson <dave.stevenson@raspberrypi.org> writes: >> >>> Hi Rob >>> >>> On 27 September 2017 at 22:51, Rob Herring <robh@kernel.org> 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 <stefan.wahren@i2se.com> wrote: >>>>> > Hi Dave, >>>>> > >>>>> >> Dave Stevenson <dave.stevenson@raspberrypi.org> 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 <dave.stevenson@raspberrypi.org> >>>>> >> --- >>>>> >> >>>>> >> 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? > > Sorry, had started to reply on this and got distracted. > > I guess since there seems to be only 1 other bit that could possibly > be used (CMI_USBCTL) it is fine like this. However, my concern would > be if the CMI registers are integrated in a different way in some > future chip that has the same unicam instances. Or just if the > register bits are rearranged. Those are not an uncommon occurrence. > You would have to provide access to those registers in some other way. > It can be dealt with, but then you have to support both cases in the > driver. If you all are fine with that, then: The bigisland chips match bcm2835. For capri the lane enables are shifted down by two and the clock is up at bit 20. That would be trivial to handle based on the compatible string, except that we can't talk to capri's hardware from ARM anyway :(
On 21 November 2017 at 20:54, Eric Anholt <eric@anholt.net> wrote: > Rob Herring <robh@kernel.org> writes: > >> On Tue, Nov 21, 2017 at 1:26 PM, Eric Anholt <eric@anholt.net> wrote: >>> Dave Stevenson <dave.stevenson@raspberrypi.org> writes: >>> >>>> Hi Rob >>>> >>>> On 27 September 2017 at 22:51, Rob Herring <robh@kernel.org> 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 <stefan.wahren@i2se.com> wrote: >>>>>> > Hi Dave, >>>>>> > >>>>>> >> Dave Stevenson <dave.stevenson@raspberrypi.org> 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 <dave.stevenson@raspberrypi.org> >>>>>> >> --- >>>>>> >> >>>>>> >> 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? >> >> Sorry, had started to reply on this and got distracted. >> >> I guess since there seems to be only 1 other bit that could possibly >> be used (CMI_USBCTL) it is fine like this. However, my concern would >> be if the CMI registers are integrated in a different way in some >> future chip that has the same unicam instances. Or just if the >> register bits are rearranged. Those are not an uncommon occurrence. >> You would have to provide access to those registers in some other way. >> It can be dealt with, but then you have to support both cases in the >> driver. If you all are fine with that, then: > > The bigisland chips match bcm2835. For capri the lane enables are > shifted down by two and the clock is up at bit 20. That would be > trivial to handle based on the compatible string, except that we can't > talk to capri's hardware from ARM anyway :( Thank you both. The Java and Hawaii chips also have the same Unicam block, but appear to be missing CMI totally based on the BCM Android kernel source. They aren't chips I have any interest in, but as Eric says it can be supported easily via the compatible string, or by making the resource optional. The latter is easy to do, so I'll add that to v4 of the patch set. Cheers, Dave
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>; + interrupts = <2 7>; + clocks = <&clocks BCM2835_CLOCK_CAM1>; + clock-names = "lp"; + + port { + csi1_ep: endpoint { + remote-endpoint = <&tc358743_0>; + data-lanes = <1 2>; + }; + }; + }; + + i2c0: i2c@7e205000 { + tc358743: csi-hdmi-bridge@0f { + compatible = "toshiba,tc358743"; + reg = <0x0f>; + + clocks = <&tc358743_clk>; + clock-names = "refclk"; + + tc358743_clk: bridge-clk { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <27000000>; + }; + + port { + tc358743_0: endpoint { + remote-endpoint = <&csi1_ep>; + clock-lanes = <0>; + data-lanes = <1 2>; + clock-noncontinuous; + link-frequencies = + /bits/ 64 <297000000>; + }; + }; + }; + };