| Message ID | 20220208155027.891055-4-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Add support for BCM2835 camera interface (unicam) | expand |
| Context | Check | Description |
|---|---|---|
| robh/patch-applied | success | |
| robh/checkpatch | success | |
| robh/dtbs-check | success | |
| robh/dt-meta-schema | success |
On Tue, 08 Feb 2022 16:50:19 +0100, Jean-Michel Hautbois wrote: > Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > camera interface. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > v4: > - make MAINTAINERS its own patch > - describe the reg and clocks correctly > - use a vendor entry for the number of data lanes > --- > .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++ > 1 file changed, 117 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > Reviewed-by: Rob Herring <robh@kernel.org>
Hi Maxime, as someone with a little more insight to the clocks, i like to know your opinion about the bcm2835-unicam binding. Am 08.02.22 um 16:50 schrieb Jean-Michel Hautbois: > Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > camera interface. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > v4: > - make MAINTAINERS its own patch > - describe the reg and clocks correctly > - use a vendor entry for the number of data lanes > --- > .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++ > 1 file changed, 117 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > new file mode 100644 > index 000000000000..1938ace23b3d > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > @@ -0,0 +1,117 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Broadcom BCM283x Camera Interface (Unicam) > + > +maintainers: > + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> > + > +description: |- > + 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 whose name starts with 'csi' then it will stop the firmware > + accessing the block, and it can then safely be used via the device tree > + binding. > + > +properties: > + compatible: > + const: brcm,bcm2835-unicam > + > + reg: > + items: > + - description: Unicam block. > + - description: Clock Manager Image (CMI) block. > + > + reg-names: > + items: > + - const: unicam > + - const: cmi > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: Clock to drive the LP state machine of Unicam. > + - description: Clock for the VPU (core clock). > + > + clock-names: > + items: > + - const: lp > + - const: vpu > + according to this patch [1], the unicam driver only needs the VPU clock reference just to enforce a minimum of 250 MHz. The firmware clock binding and its driver is specific to the bcm2711, but the Unicam IP exists since bcm2835. So do you think the clock part is correct or should be the VPU clock optional? Best regards [1] - https://github.com/raspberrypi/linux/commit/fc5b041b2e952dc84279dda978472a73a4b3891c
Hi, On Sun, Feb 13, 2022 at 04:48:45PM +0100, Stefan Wahren wrote: > as someone with a little more insight to the clocks, i like to know your > opinion about the bcm2835-unicam binding. > > Am 08.02.22 um 16:50 schrieb Jean-Michel Hautbois: > > Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > > camera interface. > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > --- > > v4: > > - make MAINTAINERS its own patch > > - describe the reg and clocks correctly > > - use a vendor entry for the number of data lanes > > --- > > .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++ > > 1 file changed, 117 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > new file mode 100644 > > index 000000000000..1938ace23b3d > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > @@ -0,0 +1,117 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Broadcom BCM283x Camera Interface (Unicam) > > + > > +maintainers: > > + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> > > + > > +description: |- > > + 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 whose name starts with 'csi' then it will stop the firmware > > + accessing the block, and it can then safely be used via the device tree > > + binding. > > + > > +properties: > > + compatible: > > + const: brcm,bcm2835-unicam > > + > > + reg: > > + items: > > + - description: Unicam block. > > + - description: Clock Manager Image (CMI) block. > > + > > + reg-names: > > + items: > > + - const: unicam > > + - const: cmi > > + > > + interrupts: > > + maxItems: 1 > > + > > + clocks: > > + items: > > + - description: Clock to drive the LP state machine of Unicam. > > + - description: Clock for the VPU (core clock). > > + > > + clock-names: > > + items: > > + - const: lp > > + - const: vpu > > + > > according to this patch [1], the unicam driver only needs the VPU clock > reference just to enforce a minimum of 250 MHz. The firmware clock > binding and its driver is specific to the bcm2711, but the Unicam IP > exists since bcm2835. > > So do you think the clock part is correct or should be the VPU clock > optional? I think we should keep it mandatory. Indeed, that clock is shared with the HVS that will change its rate on a regular basis, so even just enforcing that 250MHz while it's on without a clock handle will be fairly hard. Also, those are the constraints we have now, but having the clock handle all the time will allow us to add any constraint we might need in the future. And BCM2711 or not, the clock has probably always been there. Maxime
On Mon, Feb 14, 2022 at 10:39:54AM +0100, Maxime Ripard wrote: > Hi, > > On Sun, Feb 13, 2022 at 04:48:45PM +0100, Stefan Wahren wrote: > > as someone with a little more insight to the clocks, i like to know your > > opinion about the bcm2835-unicam binding. > > > > Am 08.02.22 um 16:50 schrieb Jean-Michel Hautbois: > > > Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > > > camera interface. > > > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > > > > > --- > > > v4: > > > - make MAINTAINERS its own patch > > > - describe the reg and clocks correctly > > > - use a vendor entry for the number of data lanes > > > --- > > > .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++ > > > 1 file changed, 117 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > > new file mode 100644 > > > index 000000000000..1938ace23b3d > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > > @@ -0,0 +1,117 @@ > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Broadcom BCM283x Camera Interface (Unicam) > > > + > > > +maintainers: > > > + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> > > > + > > > +description: |- > > > + 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 whose name starts with 'csi' then it will stop the firmware > > > + accessing the block, and it can then safely be used via the device tree > > > + binding. > > > + > > > +properties: > > > + compatible: > > > + const: brcm,bcm2835-unicam > > > + > > > + reg: > > > + items: > > > + - description: Unicam block. > > > + - description: Clock Manager Image (CMI) block. > > > + > > > + reg-names: > > > + items: > > > + - const: unicam > > > + - const: cmi > > > + > > > + interrupts: > > > + maxItems: 1 > > > + > > > + clocks: > > > + items: > > > + - description: Clock to drive the LP state machine of Unicam. > > > + - description: Clock for the VPU (core clock). > > > + > > > + clock-names: > > > + items: > > > + - const: lp > > > + - const: vpu > > > + > > > > according to this patch [1], the unicam driver only needs the VPU clock > > reference just to enforce a minimum of 250 MHz. The firmware clock > > binding and its driver is specific to the bcm2711, but the Unicam IP > > exists since bcm2835. > > > > So do you think the clock part is correct or should be the VPU clock > > optional? > > I think we should keep it mandatory. Indeed, that clock is shared with > the HVS that will change its rate on a regular basis, so even just > enforcing that 250MHz while it's on without a clock handle will be > fairly hard. > > Also, those are the constraints we have now, but having the clock handle > all the time will allow us to add any constraint we might need in the > future. > > And BCM2711 or not, the clock has probably always been there. Furthermore, regardless of what the driver needs, Unicam operates with the VPU clock, so I think it makes sense to reference it in the device tree.
Hi, Am 14.02.22 um 10:54 schrieb Laurent Pinchart: > On Mon, Feb 14, 2022 at 10:39:54AM +0100, Maxime Ripard wrote: >> Hi, >> >> On Sun, Feb 13, 2022 at 04:48:45PM +0100, Stefan Wahren wrote: >>> as someone with a little more insight to the clocks, i like to know your >>> opinion about the bcm2835-unicam binding. >>> >>> Am 08.02.22 um 16:50 schrieb Jean-Michel Hautbois: >>>> Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam >>>> camera interface. >>>> >>>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> >>>> Signed-off-by: Naushir Patuck <naush@raspberrypi.com> >>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>>> >>>> --- >>>> v4: >>>> - make MAINTAINERS its own patch >>>> - describe the reg and clocks correctly >>>> - use a vendor entry for the number of data lanes >>>> --- >>>> .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++ >>>> 1 file changed, 117 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml >>>> new file mode 100644 >>>> index 000000000000..1938ace23b3d >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml >>>> @@ -0,0 +1,117 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: Broadcom BCM283x Camera Interface (Unicam) >>>> + >>>> +maintainers: >>>> + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> >>>> + >>>> +description: |- >>>> + 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 whose name starts with 'csi' then it will stop the firmware >>>> + accessing the block, and it can then safely be used via the device tree >>>> + binding. >>>> + >>>> +properties: >>>> + compatible: >>>> + const: brcm,bcm2835-unicam >>>> + >>>> + reg: >>>> + items: >>>> + - description: Unicam block. >>>> + - description: Clock Manager Image (CMI) block. >>>> + >>>> + reg-names: >>>> + items: >>>> + - const: unicam >>>> + - const: cmi >>>> + >>>> + interrupts: >>>> + maxItems: 1 >>>> + >>>> + clocks: >>>> + items: >>>> + - description: Clock to drive the LP state machine of Unicam. >>>> + - description: Clock for the VPU (core clock). >>>> + >>>> + clock-names: >>>> + items: >>>> + - const: lp >>>> + - const: vpu >>>> + >>> according to this patch [1], the unicam driver only needs the VPU clock >>> reference just to enforce a minimum of 250 MHz. The firmware clock >>> binding and its driver is specific to the bcm2711, but the Unicam IP >>> exists since bcm2835. >>> >>> So do you think the clock part is correct or should be the VPU clock >>> optional? >> I think we should keep it mandatory. Indeed, that clock is shared with >> the HVS that will change its rate on a regular basis, so even just >> enforcing that 250MHz while it's on without a clock handle will be >> fairly hard. >> >> Also, those are the constraints we have now, but having the clock handle >> all the time will allow us to add any constraint we might need in the >> future. >> >> And BCM2711 or not, the clock has probably always been there. > Furthermore, regardless of what the driver needs, Unicam operates with > the VPU clock, so I think it makes sense to reference it in the device > tree. okay, as a result we need a DTS patch for bcm2835-rpi.dtsi to enable the firmware clocks and its driver in this series. Best regards
Hi Stefan, On Mon, Feb 14, 2022 at 12:32:59PM +0100, Stefan Wahren wrote: > Am 14.02.22 um 10:54 schrieb Laurent Pinchart: > > On Mon, Feb 14, 2022 at 10:39:54AM +0100, Maxime Ripard wrote: > >> On Sun, Feb 13, 2022 at 04:48:45PM +0100, Stefan Wahren wrote: > >>> as someone with a little more insight to the clocks, i like to know your > >>> opinion about the bcm2835-unicam binding. > >>> > >>> Am 08.02.22 um 16:50 schrieb Jean-Michel Hautbois: > >>>> Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > >>>> camera interface. > >>>> > >>>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > >>>> Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > >>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>>> > >>>> --- > >>>> v4: > >>>> - make MAINTAINERS its own patch > >>>> - describe the reg and clocks correctly > >>>> - use a vendor entry for the number of data lanes > >>>> --- > >>>> .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++ > >>>> 1 file changed, 117 insertions(+) > >>>> create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > >>>> new file mode 100644 > >>>> index 000000000000..1938ace23b3d > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > >>>> @@ -0,0 +1,117 @@ > >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>> +%YAML 1.2 > >>>> +--- > >>>> +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# > >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>> + > >>>> +title: Broadcom BCM283x Camera Interface (Unicam) > >>>> + > >>>> +maintainers: > >>>> + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> > >>>> + > >>>> +description: |- > >>>> + 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 whose name starts with 'csi' then it will stop the firmware > >>>> + accessing the block, and it can then safely be used via the device tree > >>>> + binding. > >>>> + > >>>> +properties: > >>>> + compatible: > >>>> + const: brcm,bcm2835-unicam > >>>> + > >>>> + reg: > >>>> + items: > >>>> + - description: Unicam block. > >>>> + - description: Clock Manager Image (CMI) block. > >>>> + > >>>> + reg-names: > >>>> + items: > >>>> + - const: unicam > >>>> + - const: cmi > >>>> + > >>>> + interrupts: > >>>> + maxItems: 1 > >>>> + > >>>> + clocks: > >>>> + items: > >>>> + - description: Clock to drive the LP state machine of Unicam. > >>>> + - description: Clock for the VPU (core clock). > >>>> + > >>>> + clock-names: > >>>> + items: > >>>> + - const: lp > >>>> + - const: vpu > >>>> + > >>> > >>> according to this patch [1], the unicam driver only needs the VPU clock > >>> reference just to enforce a minimum of 250 MHz. The firmware clock > >>> binding and its driver is specific to the bcm2711, but the Unicam IP > >>> exists since bcm2835. > >>> > >>> So do you think the clock part is correct or should be the VPU clock > >>> optional? > >> > >> I think we should keep it mandatory. Indeed, that clock is shared with > >> the HVS that will change its rate on a regular basis, so even just > >> enforcing that 250MHz while it's on without a clock handle will be > >> fairly hard. > >> > >> Also, those are the constraints we have now, but having the clock handle > >> all the time will allow us to add any constraint we might need in the > >> future. > >> > >> And BCM2711 or not, the clock has probably always been there. > > > > Furthermore, regardless of what the driver needs, Unicam operates with > > the VPU clock, so I think it makes sense to reference it in the device > > tree. > > okay, as a result we need a DTS patch for bcm2835-rpi.dtsi to enable the > firmware clocks and its driver in this series. Can't we do that on top, enabling Unicam support for bcm2711 only first ? I have no idea how to deal with firmware clocks on bcm2825, and I'm not sure Jean-Michel even has a hardware platform to test it. If you want to send a patch series to enable firmware clocks on bcm2835, we'll be happy to rebase on top.
On Mon, Feb 21, 2022 at 09:10:27AM +0200, Laurent Pinchart wrote: > Hi Stefan, > > On Mon, Feb 14, 2022 at 12:32:59PM +0100, Stefan Wahren wrote: > > Am 14.02.22 um 10:54 schrieb Laurent Pinchart: > > > On Mon, Feb 14, 2022 at 10:39:54AM +0100, Maxime Ripard wrote: > > >> On Sun, Feb 13, 2022 at 04:48:45PM +0100, Stefan Wahren wrote: > > >>> as someone with a little more insight to the clocks, i like to know your > > >>> opinion about the bcm2835-unicam binding. > > >>> > > >>> Am 08.02.22 um 16:50 schrieb Jean-Michel Hautbois: > > >>>> Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > > >>>> camera interface. > > >>>> > > >>>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > > >>>> Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > >>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > > >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > >>>> > > >>>> --- > > >>>> v4: > > >>>> - make MAINTAINERS its own patch > > >>>> - describe the reg and clocks correctly > > >>>> - use a vendor entry for the number of data lanes > > >>>> --- > > >>>> .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++ > > >>>> 1 file changed, 117 insertions(+) > > >>>> create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > >>>> > > >>>> diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > >>>> new file mode 100644 > > >>>> index 000000000000..1938ace23b3d > > >>>> --- /dev/null > > >>>> +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > >>>> @@ -0,0 +1,117 @@ > > >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > >>>> +%YAML 1.2 > > >>>> +--- > > >>>> +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# > > >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > > >>>> + > > >>>> +title: Broadcom BCM283x Camera Interface (Unicam) > > >>>> + > > >>>> +maintainers: > > >>>> + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> > > >>>> + > > >>>> +description: |- > > >>>> + 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 whose name starts with 'csi' then it will stop the firmware > > >>>> + accessing the block, and it can then safely be used via the device tree > > >>>> + binding. > > >>>> + > > >>>> +properties: > > >>>> + compatible: > > >>>> + const: brcm,bcm2835-unicam > > >>>> + > > >>>> + reg: > > >>>> + items: > > >>>> + - description: Unicam block. > > >>>> + - description: Clock Manager Image (CMI) block. > > >>>> + > > >>>> + reg-names: > > >>>> + items: > > >>>> + - const: unicam > > >>>> + - const: cmi > > >>>> + > > >>>> + interrupts: > > >>>> + maxItems: 1 > > >>>> + > > >>>> + clocks: > > >>>> + items: > > >>>> + - description: Clock to drive the LP state machine of Unicam. > > >>>> + - description: Clock for the VPU (core clock). > > >>>> + > > >>>> + clock-names: > > >>>> + items: > > >>>> + - const: lp > > >>>> + - const: vpu > > >>>> + > > >>> > > >>> according to this patch [1], the unicam driver only needs the VPU clock > > >>> reference just to enforce a minimum of 250 MHz. The firmware clock > > >>> binding and its driver is specific to the bcm2711, but the Unicam IP > > >>> exists since bcm2835. > > >>> > > >>> So do you think the clock part is correct or should be the VPU clock > > >>> optional? > > >> > > >> I think we should keep it mandatory. Indeed, that clock is shared with > > >> the HVS that will change its rate on a regular basis, so even just > > >> enforcing that 250MHz while it's on without a clock handle will be > > >> fairly hard. > > >> > > >> Also, those are the constraints we have now, but having the clock handle > > >> all the time will allow us to add any constraint we might need in the > > >> future. > > >> > > >> And BCM2711 or not, the clock has probably always been there. > > > > > > Furthermore, regardless of what the driver needs, Unicam operates with > > > the VPU clock, so I think it makes sense to reference it in the device > > > tree. > > > > okay, as a result we need a DTS patch for bcm2835-rpi.dtsi to enable the > > firmware clocks and its driver in this series. > > Can't we do that on top, enabling Unicam support for bcm2711 only first > ? I have no idea how to deal with firmware clocks on bcm2825, and I'm > not sure Jean-Michel even has a hardware platform to test it. The RPi kernel uses the firmware clocks driver for all the platforms, so it shouldn't be too bad. Maxime
Hi Laurent, Am 21.02.22 um 08:10 schrieb Laurent Pinchart: > Hi Stefan, > > On Mon, Feb 14, 2022 at 12:32:59PM +0100, Stefan Wahren wrote: >> Am 14.02.22 um 10:54 schrieb Laurent Pinchart: >>> On Mon, Feb 14, 2022 at 10:39:54AM +0100, Maxime Ripard wrote: >>>> On Sun, Feb 13, 2022 at 04:48:45PM +0100, Stefan Wahren wrote: >>>>> as someone with a little more insight to the clocks, i like to know your >>>>> opinion about the bcm2835-unicam binding. >>>>> >>>>> Am 08.02.22 um 16:50 schrieb Jean-Michel Hautbois: >>>>>> Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam >>>>>> camera interface. >>>>>> >>>>>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> >>>>>> Signed-off-by: Naushir Patuck <naush@raspberrypi.com> >>>>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >>>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> >>>>>> >>>>>> --- >>>>>> v4: >>>>>> - make MAINTAINERS its own patch >>>>>> - describe the reg and clocks correctly >>>>>> - use a vendor entry for the number of data lanes >>>>>> --- >>>>>> .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++ >>>>>> 1 file changed, 117 insertions(+) >>>>>> create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml >>>>>> new file mode 100644 >>>>>> index 000000000000..1938ace23b3d >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml >>>>>> @@ -0,0 +1,117 @@ >>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>>>> +%YAML 1.2 >>>>>> +--- >>>>>> +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# >>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>>> + >>>>>> +title: Broadcom BCM283x Camera Interface (Unicam) >>>>>> + >>>>>> +maintainers: >>>>>> + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> >>>>>> + >>>>>> +description: |- >>>>>> + 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 whose name starts with 'csi' then it will stop the firmware >>>>>> + accessing the block, and it can then safely be used via the device tree >>>>>> + binding. >>>>>> + >>>>>> +properties: >>>>>> + compatible: >>>>>> + const: brcm,bcm2835-unicam >>>>>> + >>>>>> + reg: >>>>>> + items: >>>>>> + - description: Unicam block. >>>>>> + - description: Clock Manager Image (CMI) block. >>>>>> + >>>>>> + reg-names: >>>>>> + items: >>>>>> + - const: unicam >>>>>> + - const: cmi >>>>>> + >>>>>> + interrupts: >>>>>> + maxItems: 1 >>>>>> + >>>>>> + clocks: >>>>>> + items: >>>>>> + - description: Clock to drive the LP state machine of Unicam. >>>>>> + - description: Clock for the VPU (core clock). >>>>>> + >>>>>> + clock-names: >>>>>> + items: >>>>>> + - const: lp >>>>>> + - const: vpu >>>>>> + >>>>> according to this patch [1], the unicam driver only needs the VPU clock >>>>> reference just to enforce a minimum of 250 MHz. The firmware clock >>>>> binding and its driver is specific to the bcm2711, but the Unicam IP >>>>> exists since bcm2835. >>>>> >>>>> So do you think the clock part is correct or should be the VPU clock >>>>> optional? >>>> I think we should keep it mandatory. Indeed, that clock is shared with >>>> the HVS that will change its rate on a regular basis, so even just >>>> enforcing that 250MHz while it's on without a clock handle will be >>>> fairly hard. >>>> >>>> Also, those are the constraints we have now, but having the clock handle >>>> all the time will allow us to add any constraint we might need in the >>>> future. >>>> >>>> And BCM2711 or not, the clock has probably always been there. >>> Furthermore, regardless of what the driver needs, Unicam operates with >>> the VPU clock, so I think it makes sense to reference it in the device >>> tree. >> okay, as a result we need a DTS patch for bcm2835-rpi.dtsi to enable the >> firmware clocks and its driver in this series. > Can't we do that on top, enabling Unicam support for bcm2711 only first > ? I have no idea how to deal with firmware clocks on bcm2825, and I'm > not sure Jean-Michel even has a hardware platform to test it. sorry as being a bcm2835 maintainer so long, i'm not always aware of the ambiguous meaning of bcm2835. The bcm2835-rpi.dtsi is used by all Raspberry Pi generations. So it's sufficient to test it with a Raspberry Pi 4, but we would gain support for all generations. So my request is to move the changes from bcm2711 specific dtsi to the general dtsi. There is no need to touch any driver, please have a look at this patch [1] to see what i had in my mind. Just compile tested. I hope you understand, i want to have it for all generations in one step. [1] - https://github.com/lategoodbye/rpi-zero/commit/67897cc22c03204e6464d00ff4ddac6bf5dc65dc > If you want to send a patch series to enable firmware clocks on bcm2835, > we'll be happy to rebase on top. >
Hi Stefan, On Mon, Feb 21, 2022 at 01:45:10PM +0100, Stefan Wahren wrote: > Am 21.02.22 um 08:10 schrieb Laurent Pinchart: > > On Mon, Feb 14, 2022 at 12:32:59PM +0100, Stefan Wahren wrote: > >> Am 14.02.22 um 10:54 schrieb Laurent Pinchart: > >>> On Mon, Feb 14, 2022 at 10:39:54AM +0100, Maxime Ripard wrote: > >>>> On Sun, Feb 13, 2022 at 04:48:45PM +0100, Stefan Wahren wrote: > >>>>> as someone with a little more insight to the clocks, i like to know your > >>>>> opinion about the bcm2835-unicam binding. > >>>>> > >>>>> Am 08.02.22 um 16:50 schrieb Jean-Michel Hautbois: > >>>>>> Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > >>>>>> camera interface. > >>>>>> > >>>>>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > >>>>>> Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > >>>>>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > >>>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >>>>>> > >>>>>> --- > >>>>>> v4: > >>>>>> - make MAINTAINERS its own patch > >>>>>> - describe the reg and clocks correctly > >>>>>> - use a vendor entry for the number of data lanes > >>>>>> --- > >>>>>> .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++ > >>>>>> 1 file changed, 117 insertions(+) > >>>>>> create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > >>>>>> > >>>>>> diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > >>>>>> new file mode 100644 > >>>>>> index 000000000000..1938ace23b3d > >>>>>> --- /dev/null > >>>>>> +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > >>>>>> @@ -0,0 +1,117 @@ > >>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > >>>>>> +%YAML 1.2 > >>>>>> +--- > >>>>>> +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# > >>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>>>> + > >>>>>> +title: Broadcom BCM283x Camera Interface (Unicam) > >>>>>> + > >>>>>> +maintainers: > >>>>>> + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> > >>>>>> + > >>>>>> +description: |- > >>>>>> + 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 whose name starts with 'csi' then it will stop the firmware > >>>>>> + accessing the block, and it can then safely be used via the device tree > >>>>>> + binding. > >>>>>> + > >>>>>> +properties: > >>>>>> + compatible: > >>>>>> + const: brcm,bcm2835-unicam > >>>>>> + > >>>>>> + reg: > >>>>>> + items: > >>>>>> + - description: Unicam block. > >>>>>> + - description: Clock Manager Image (CMI) block. > >>>>>> + > >>>>>> + reg-names: > >>>>>> + items: > >>>>>> + - const: unicam > >>>>>> + - const: cmi > >>>>>> + > >>>>>> + interrupts: > >>>>>> + maxItems: 1 > >>>>>> + > >>>>>> + clocks: > >>>>>> + items: > >>>>>> + - description: Clock to drive the LP state machine of Unicam. > >>>>>> + - description: Clock for the VPU (core clock). > >>>>>> + > >>>>>> + clock-names: > >>>>>> + items: > >>>>>> + - const: lp > >>>>>> + - const: vpu > >>>>>> + > >>>>> > >>>>> according to this patch [1], the unicam driver only needs the VPU clock > >>>>> reference just to enforce a minimum of 250 MHz. The firmware clock > >>>>> binding and its driver is specific to the bcm2711, but the Unicam IP > >>>>> exists since bcm2835. > >>>>> > >>>>> So do you think the clock part is correct or should be the VPU clock > >>>>> optional? > >>>> > >>>> I think we should keep it mandatory. Indeed, that clock is shared with > >>>> the HVS that will change its rate on a regular basis, so even just > >>>> enforcing that 250MHz while it's on without a clock handle will be > >>>> fairly hard. > >>>> > >>>> Also, those are the constraints we have now, but having the clock handle > >>>> all the time will allow us to add any constraint we might need in the > >>>> future. > >>>> > >>>> And BCM2711 or not, the clock has probably always been there. > >>> > >>> Furthermore, regardless of what the driver needs, Unicam operates with > >>> the VPU clock, so I think it makes sense to reference it in the device > >>> tree. > >> > >> okay, as a result we need a DTS patch for bcm2835-rpi.dtsi to enable the > >> firmware clocks and its driver in this series. > > > > Can't we do that on top, enabling Unicam support for bcm2711 only first > > ? I have no idea how to deal with firmware clocks on bcm2825, and I'm > > not sure Jean-Michel even has a hardware platform to test it. > > sorry as being a bcm2835 maintainer so long, i'm not always aware of the > ambiguous meaning of bcm2835. The bcm2835-rpi.dtsi is used by all > Raspberry Pi generations. So it's sufficient to test it with a Raspberry > Pi 4, but we would gain support for all generations. > > So my request is to move the changes from bcm2711 specific dtsi to the > general dtsi. There is no need to touch any driver, please have a look > at this patch [1] to see what i had in my mind. Just compile tested. > > I hope you understand, i want to have it for all generations in one step. > > [1] - > https://github.com/lategoodbye/rpi-zero/commit/67897cc22c03204e6464d00ff4ddac6bf5dc65dc Got it. I thought changes would be needed in the firmware clocks driver, if it's just about moving the node to a different file, it's pretty easy. Thank you for taking the time to cook up a patch to educate me :-) > > If you want to send a patch series to enable firmware clocks on bcm2835, > > we'll be happy to rebase on top.
Hi Jean-Michel, On Tue, Feb 08, 2022 at 04:50:19PM +0100, Jean-Michel Hautbois wrote: > Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam > camera interface. > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > v4: > - make MAINTAINERS its own patch > - describe the reg and clocks correctly > - use a vendor entry for the number of data lanes > --- > .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++ > 1 file changed, 117 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > > diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > new file mode 100644 > index 000000000000..1938ace23b3d > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml > @@ -0,0 +1,117 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Broadcom BCM283x Camera Interface (Unicam) > + > +maintainers: > + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> > + > +description: |- > + 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 whose name starts with 'csi' then it will stop the firmware > + accessing the block, and it can then safely be used via the device tree > + binding. > + > +properties: > + compatible: > + const: brcm,bcm2835-unicam > + > + reg: > + items: > + - description: Unicam block. > + - description: Clock Manager Image (CMI) block. > + > + reg-names: > + items: > + - const: unicam > + - const: cmi > + > + interrupts: > + maxItems: 1 > + > + clocks: > + items: > + - description: Clock to drive the LP state machine of Unicam. > + - description: Clock for the VPU (core clock). > + > + clock-names: > + items: > + - const: lp > + - const: vpu > + > + power-domains: > + items: > + - description: Unicam power domain > + > + brcm,num-data-lanes: > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [ 2, 4 ] > + description: | > + Number of CSI-2 data lanes supported by this Unicam instance. The number > + of data lanes actively used is specified with the data-lanes endpoint > + property. > + > + port: > + $ref: /schemas/graph.yaml#/$defs/port-base > + unevaluatedProperties: false > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# > + unevaluatedProperties: false > + > + properties: > + data-lanes: true > + > + required: > + - data-lanes As you support both CSI-2 D-PHY and CCP, you'll need bus-type (or make CSI-2 default), and optionally strobe property with CCP. > + > + required: > + - endpoint > + > +required: > + - compatible > + - reg > + - reg-names > + - interrupts > + - clocks > + - clock-names > + - power-domains > + - brcm,num-data-lanes > + - port > + > +additionalProperties: False > + > +examples: > + - | > + #include <dt-bindings/clock/bcm2835.h> > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + #include <dt-bindings/power/raspberrypi-power.h> > + csi1: csi@7e801000 { > + compatible = "brcm,bcm2835-unicam"; > + reg = <0x7e801000 0x800>, > + <0x7e802004 0x4>; > + reg-names = "unicam", "cmi"; > + interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>; > + clocks = <&clocks BCM2835_CLOCK_CAM1>, > + <&firmware_clocks 4>; > + clock-names = "lp", "vpu"; > + power-domains = <&power RPI_POWER_DOMAIN_UNICAM1>; > + brcm,num-data-lanes = <2>; > + port { > + csi1_ep: endpoint { > + remote-endpoint = <&imx219_0>; > + data-lanes = <1 2>; > + }; > + }; > + }; > +...
diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml new file mode 100644 index 000000000000..1938ace23b3d --- /dev/null +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml @@ -0,0 +1,117 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Broadcom BCM283x Camera Interface (Unicam) + +maintainers: + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com> + +description: |- + 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 whose name starts with 'csi' then it will stop the firmware + accessing the block, and it can then safely be used via the device tree + binding. + +properties: + compatible: + const: brcm,bcm2835-unicam + + reg: + items: + - description: Unicam block. + - description: Clock Manager Image (CMI) block. + + reg-names: + items: + - const: unicam + - const: cmi + + interrupts: + maxItems: 1 + + clocks: + items: + - description: Clock to drive the LP state machine of Unicam. + - description: Clock for the VPU (core clock). + + clock-names: + items: + - const: lp + - const: vpu + + power-domains: + items: + - description: Unicam power domain + + brcm,num-data-lanes: + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [ 2, 4 ] + description: | + Number of CSI-2 data lanes supported by this Unicam instance. The number + of data lanes actively used is specified with the data-lanes endpoint + property. + + port: + $ref: /schemas/graph.yaml#/$defs/port-base + unevaluatedProperties: false + + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: true + + required: + - data-lanes + + required: + - endpoint + +required: + - compatible + - reg + - reg-names + - interrupts + - clocks + - clock-names + - power-domains + - brcm,num-data-lanes + - port + +additionalProperties: False + +examples: + - | + #include <dt-bindings/clock/bcm2835.h> + #include <dt-bindings/interrupt-controller/arm-gic.h> + #include <dt-bindings/power/raspberrypi-power.h> + csi1: csi@7e801000 { + compatible = "brcm,bcm2835-unicam"; + reg = <0x7e801000 0x800>, + <0x7e802004 0x4>; + reg-names = "unicam", "cmi"; + interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>; + clocks = <&clocks BCM2835_CLOCK_CAM1>, + <&firmware_clocks 4>; + clock-names = "lp", "vpu"; + power-domains = <&power RPI_POWER_DOMAIN_UNICAM1>; + brcm,num-data-lanes = <2>; + port { + csi1_ep: endpoint { + remote-endpoint = <&imx219_0>; + data-lanes = <1 2>; + }; + }; + }; +...