diff mbox series

[v6,1/5] dt-bindings: media: Add Allwinner A10 CSI binding

Message ID 110dd9ff1784c29fa16304825a41d1603a33f166.1562847292.git-series.maxime.ripard@bootlin.com (mailing list archive)
State New, archived
Headers show
Series media: Allwinner A10 CSI support | expand

Commit Message

Maxime Ripard July 11, 2019, 12:15 p.m. UTC
The Allwinner A10 CMOS Sensor Interface is a camera capture interface also
used in later (A10s, A13, A20, R8 and GR8) SoCs.

On some SoCs, like the A10, there's multiple instances of that controller,
with one instance supporting more channels and having an ISP.

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
---
 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 94 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml

Comments

Chen-Yu Tsai Aug. 15, 2019, 8:34 a.m. UTC | #1
Hi,

Sorry for chiming in so late.

On Thu, Jul 11, 2019 at 8:15 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
>
> The Allwinner A10 CMOS Sensor Interface is a camera capture interface also
> used in later (A10s, A13, A20, R8 and GR8) SoCs.
>
> On some SoCs, like the A10, there's multiple instances of that controller,
> with one instance supporting more channels and having an ISP.
>
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> ---
>  Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 94 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
>
> diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> new file mode 100644
> index 000000000000..97c9fc3b5050
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> @@ -0,0 +1,94 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings
> +
> +maintainers:
> +  - Chen-Yu Tsai <wens@csie.org>
> +  - Maxime Ripard <maxime.ripard@bootlin.com>
> +
> +description: |-
> +  The Allwinner A10 and later has a CMOS Sensor Interface to retrieve
> +  frames from a parallel or BT656 sensor.
> +
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - allwinner,sun7i-a20-csi0
> +          - const: allwinner,sun4i-a10-csi0

CSI0 on the A10 has an ISP. Do we know if the one in the A20 does
as well? It certainly doesn't say so in the user manual. If not,
then we can't claim that A20 CSI0 is compatible with A10 CSI0.

> +
> +      - items:
> +          - const: allwinner,sun4i-a10-csi0
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: The CSI interface clock
> +      - description: The CSI module clock
> +      - description: The CSI ISP clock
> +      - description: The CSI DRAM clock
> +
> +  clock-names:
> +    items:
> +      - const: bus
> +      - const: mod

I doubt this actually is a module clock. Based on the usage in your
device tree patch, and the csi driver in the old linux-sunxi kernel,
the clock rate is set to 24 MHz, or whatever the sensor requires for
MCLK.

ChenYu

> +      - const: isp
> +      - const: ram
> +
> +  resets:
> +    description: The reset line driver this IP
> +    maxItems: 1
> +
> +  pinctrl-0:
> +    minItems: 1
> +
> +  pinctrl-names:
> +    const: default
> +
> +  port:
> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      endpoint:
> +        properties:
> +          bus-width:
> +            const: 8
> +            description: Number of data lines actively used.
> +
> +          data-active: true
> +          hsync-active: true
> +          pclk-sample: true
> +          remote-endpoint: true
> +          vsync-active: true
> +
> +        required:
> +          - bus-width
> +          - data-active
> +          - hsync-active
> +          - pclk-sample
> +          - remote-endpoint
> +          - vsync-active
> +
> +    required:
> +      - endpoint
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +additionalProperties: false
> +...
> --
> git-series 0.9.1
Chen-Yu Tsai Sept. 15, 2019, 8:54 a.m. UTC | #2
On Thu, Aug 15, 2019 at 4:34 PM Chen-Yu Tsai <wens@csie.org> wrote:
>
> Hi,
>
> Sorry for chiming in so late.
>
> On Thu, Jul 11, 2019 at 8:15 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> >
> > The Allwinner A10 CMOS Sensor Interface is a camera capture interface also
> > used in later (A10s, A13, A20, R8 and GR8) SoCs.
> >
> > On some SoCs, like the A10, there's multiple instances of that controller,
> > with one instance supporting more channels and having an ISP.
> >
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > ---
> >  Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 94 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > new file mode 100644
> > index 000000000000..97c9fc3b5050
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > @@ -0,0 +1,94 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings
> > +
> > +maintainers:
> > +  - Chen-Yu Tsai <wens@csie.org>
> > +  - Maxime Ripard <maxime.ripard@bootlin.com>
> > +
> > +description: |-
> > +  The Allwinner A10 and later has a CMOS Sensor Interface to retrieve
> > +  frames from a parallel or BT656 sensor.
> > +
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - enum:
> > +              - allwinner,sun7i-a20-csi0
> > +          - const: allwinner,sun4i-a10-csi0
>
> CSI0 on the A10 has an ISP. Do we know if the one in the A20 does
> as well? It certainly doesn't say so in the user manual. If not,
> then we can't claim that A20 CSI0 is compatible with A10 CSI0.
>
> > +
> > +      - items:
> > +          - const: allwinner,sun4i-a10-csi0
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: The CSI interface clock
> > +      - description: The CSI module clock
> > +      - description: The CSI ISP clock
> > +      - description: The CSI DRAM clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: bus
> > +      - const: mod
>
> I doubt this actually is a module clock. Based on the usage in your
> device tree patch, and the csi driver in the old linux-sunxi kernel,
> the clock rate is set to 24 MHz, or whatever the sensor requires for
> MCLK.

I'm working on adding support for this on the R40, and it seems with
this SoC the picture is much clearer. It has the same CSI interface
block, but the CCU has the clocks correctly named. We have:

  - CSI_MCLK0
  - CSI_MCLK1
  - CSI_SCLK

in addition to the bus clocks.

The CSI section also explains the clock signals:

    6.1.3.2. Clock Sources
    Two Clocks need to be configured for CSI controller. CSI0/1_MCLK
    provides the master clock for sensor and other devices. CSI_SCLK
    is the top clock for the whole CSI module.

So it would seem the ISP clock we currently have in the DT is simply
the module clock shared by all CSI-related hardware blocks, and the
module clock is bogus.

ChenYu

> ChenYu
>
> > +      - const: isp
> > +      - const: ram
> > +
> > +  resets:
> > +    description: The reset line driver this IP
> > +    maxItems: 1
> > +
> > +  pinctrl-0:
> > +    minItems: 1
> > +
> > +  pinctrl-names:
> > +    const: default
> > +
> > +  port:
> > +    type: object
> > +    additionalProperties: false
> > +
> > +    properties:
> > +      endpoint:
> > +        properties:
> > +          bus-width:
> > +            const: 8
> > +            description: Number of data lines actively used.
> > +
> > +          data-active: true
> > +          hsync-active: true
> > +          pclk-sample: true
> > +          remote-endpoint: true
> > +          vsync-active: true
> > +
> > +        required:
> > +          - bus-width
> > +          - data-active
> > +          - hsync-active
> > +          - pclk-sample
> > +          - remote-endpoint
> > +          - vsync-active
> > +
> > +    required:
> > +      - endpoint
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +
> > +additionalProperties: false
> > +...
> > --
> > git-series 0.9.1
Maxime Ripard Oct. 1, 2019, 8:52 a.m. UTC | #3
Hi,

Thanks for looking into this.

On Sun, Sep 15, 2019 at 04:54:16PM +0800, Chen-Yu Tsai wrote:
> On Thu, Aug 15, 2019 at 4:34 PM Chen-Yu Tsai <wens@csie.org> wrote:
> >
> > Hi,
> >
> > Sorry for chiming in so late.
> >
> > On Thu, Jul 11, 2019 at 8:15 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > >
> > > The Allwinner A10 CMOS Sensor Interface is a camera capture interface also
> > > used in later (A10s, A13, A20, R8 and GR8) SoCs.
> > >
> > > On some SoCs, like the A10, there's multiple instances of that controller,
> > > with one instance supporting more channels and having an ISP.
> > >
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > ---
> > >  Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  1 file changed, 94 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > new file mode 100644
> > > index 000000000000..97c9fc3b5050
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > @@ -0,0 +1,94 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Chen-Yu Tsai <wens@csie.org>
> > > +  - Maxime Ripard <maxime.ripard@bootlin.com>
> > > +
> > > +description: |-
> > > +  The Allwinner A10 and later has a CMOS Sensor Interface to retrieve
> > > +  frames from a parallel or BT656 sensor.
> > > +
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - items:
> > > +          - enum:
> > > +              - allwinner,sun7i-a20-csi0
> > > +          - const: allwinner,sun4i-a10-csi0
> >
> > CSI0 on the A10 has an ISP. Do we know if the one in the A20 does
> > as well? It certainly doesn't say so in the user manual. If not,
> > then we can't claim that A20 CSI0 is compatible with A10 CSI0.
> >
> > > +
> > > +      - items:
> > > +          - const: allwinner,sun4i-a10-csi0
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    items:
> > > +      - description: The CSI interface clock
> > > +      - description: The CSI module clock
> > > +      - description: The CSI ISP clock
> > > +      - description: The CSI DRAM clock
> > > +
> > > +  clock-names:
> > > +    items:
> > > +      - const: bus
> > > +      - const: mod
> >
> > I doubt this actually is a module clock. Based on the usage in your
> > device tree patch, and the csi driver in the old linux-sunxi kernel,
> > the clock rate is set to 24 MHz, or whatever the sensor requires for
> > MCLK.
>
> I'm working on adding support for this on the R40, and it seems with
> this SoC the picture is much clearer. It has the same CSI interface
> block, but the CCU has the clocks correctly named. We have:
>
>   - CSI_MCLK0
>   - CSI_MCLK1
>   - CSI_SCLK
>
> in addition to the bus clocks.
>
> The CSI section also explains the clock signals:
>
>     6.1.3.2. Clock Sources
>     Two Clocks need to be configured for CSI controller. CSI0/1_MCLK
>     provides the master clock for sensor and other devices. CSI_SCLK
>     is the top clock for the whole CSI module.
>
> So it would seem the ISP clock we currently have in the DT is simply
> the module clock shared by all CSI-related hardware blocks, and the
> module clock is bogus.

I don't think it is. It looks like there's no ISP in the R40 CSI
controllers, so that would mean that we don't have an ISP clock, and
the SCLK is the module clock.

Does that make sense?

Maxime
Chen-Yu Tsai Oct. 1, 2019, 9:04 a.m. UTC | #4
On Tue, Oct 1, 2019 at 4:52 PM Maxime Ripard <mripard@kernel.org> wrote:
>
> Hi,
>
> Thanks for looking into this.
>
> On Sun, Sep 15, 2019 at 04:54:16PM +0800, Chen-Yu Tsai wrote:
> > On Thu, Aug 15, 2019 at 4:34 PM Chen-Yu Tsai <wens@csie.org> wrote:
> > >
> > > Hi,
> > >
> > > Sorry for chiming in so late.
> > >
> > > On Thu, Jul 11, 2019 at 8:15 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > >
> > > > The Allwinner A10 CMOS Sensor Interface is a camera capture interface also
> > > > used in later (A10s, A13, A20, R8 and GR8) SoCs.
> > > >
> > > > On some SoCs, like the A10, there's multiple instances of that controller,
> > > > with one instance supporting more channels and having an ISP.
> > > >
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > > ---
> > > >  Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 94 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > new file mode 100644
> > > > index 000000000000..97c9fc3b5050
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > @@ -0,0 +1,94 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings
> > > > +
> > > > +maintainers:
> > > > +  - Chen-Yu Tsai <wens@csie.org>
> > > > +  - Maxime Ripard <maxime.ripard@bootlin.com>
> > > > +
> > > > +description: |-
> > > > +  The Allwinner A10 and later has a CMOS Sensor Interface to retrieve
> > > > +  frames from a parallel or BT656 sensor.
> > > > +
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    oneOf:
> > > > +      - items:
> > > > +          - enum:
> > > > +              - allwinner,sun7i-a20-csi0
> > > > +          - const: allwinner,sun4i-a10-csi0
> > >
> > > CSI0 on the A10 has an ISP. Do we know if the one in the A20 does
> > > as well? It certainly doesn't say so in the user manual. If not,
> > > then we can't claim that A20 CSI0 is compatible with A10 CSI0.
> > >
> > > > +
> > > > +      - items:
> > > > +          - const: allwinner,sun4i-a10-csi0
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: The CSI interface clock
> > > > +      - description: The CSI module clock
> > > > +      - description: The CSI ISP clock
> > > > +      - description: The CSI DRAM clock
> > > > +
> > > > +  clock-names:
> > > > +    items:
> > > > +      - const: bus
> > > > +      - const: mod
> > >
> > > I doubt this actually is a module clock. Based on the usage in your
> > > device tree patch, and the csi driver in the old linux-sunxi kernel,
> > > the clock rate is set to 24 MHz, or whatever the sensor requires for
> > > MCLK.
> >
> > I'm working on adding support for this on the R40, and it seems with
> > this SoC the picture is much clearer. It has the same CSI interface
> > block, but the CCU has the clocks correctly named. We have:
> >
> >   - CSI_MCLK0
> >   - CSI_MCLK1
> >   - CSI_SCLK
> >
> > in addition to the bus clocks.
> >
> > The CSI section also explains the clock signals:
> >
> >     6.1.3.2. Clock Sources
> >     Two Clocks need to be configured for CSI controller. CSI0/1_MCLK
> >     provides the master clock for sensor and other devices. CSI_SCLK
> >     is the top clock for the whole CSI module.
> >
> > So it would seem the ISP clock we currently have in the DT is simply
> > the module clock shared by all CSI-related hardware blocks, and the
> > module clock is bogus.
>
> I don't think it is. It looks like there's no ISP in the R40 CSI
> controllers, so that would mean that we don't have an ISP clock, and
> the SCLK is the module clock.
>
> Does that make sense?

Right. That's another way to put it. The point is I believe the
CSI[01]_CLK clocks on the A10/A20 are simply the MCLK outputs.

ChenYu
Maxime Ripard Oct. 2, 2019, 2:49 p.m. UTC | #5
Hi,

On Tue, Oct 01, 2019 at 05:04:40PM +0800, Chen-Yu Tsai wrote:
> On Tue, Oct 1, 2019 at 4:52 PM Maxime Ripard <mripard@kernel.org> wrote:
> >
> > Hi,
> >
> > Thanks for looking into this.
> >
> > On Sun, Sep 15, 2019 at 04:54:16PM +0800, Chen-Yu Tsai wrote:
> > > On Thu, Aug 15, 2019 at 4:34 PM Chen-Yu Tsai <wens@csie.org> wrote:
> > > >
> > > > Hi,
> > > >
> > > > Sorry for chiming in so late.
> > > >
> > > > On Thu, Jul 11, 2019 at 8:15 PM Maxime Ripard <maxime.ripard@bootlin.com> wrote:
> > > > >
> > > > > The Allwinner A10 CMOS Sensor Interface is a camera capture interface also
> > > > > used in later (A10s, A13, A20, R8 and GR8) SoCs.
> > > > >
> > > > > On some SoCs, like the A10, there's multiple instances of that controller,
> > > > > with one instance supporting more channels and having an ISP.
> > > > >
> > > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > > > Signed-off-by: Maxime Ripard <maxime.ripard@bootlin.com>
> > > > > ---
> > > > >  Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml | 94 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > > > >  1 file changed, 94 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..97c9fc3b5050
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
> > > > > @@ -0,0 +1,94 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings
> > > > > +
> > > > > +maintainers:
> > > > > +  - Chen-Yu Tsai <wens@csie.org>
> > > > > +  - Maxime Ripard <maxime.ripard@bootlin.com>
> > > > > +
> > > > > +description: |-
> > > > > +  The Allwinner A10 and later has a CMOS Sensor Interface to retrieve
> > > > > +  frames from a parallel or BT656 sensor.
> > > > > +
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    oneOf:
> > > > > +      - items:
> > > > > +          - enum:
> > > > > +              - allwinner,sun7i-a20-csi0
> > > > > +          - const: allwinner,sun4i-a10-csi0
> > > >
> > > > CSI0 on the A10 has an ISP. Do we know if the one in the A20 does
> > > > as well? It certainly doesn't say so in the user manual. If not,
> > > > then we can't claim that A20 CSI0 is compatible with A10 CSI0.
> > > >
> > > > > +
> > > > > +      - items:
> > > > > +          - const: allwinner,sun4i-a10-csi0
> > > > > +
> > > > > +  reg:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  interrupts:
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  clocks:
> > > > > +    items:
> > > > > +      - description: The CSI interface clock
> > > > > +      - description: The CSI module clock
> > > > > +      - description: The CSI ISP clock
> > > > > +      - description: The CSI DRAM clock
> > > > > +
> > > > > +  clock-names:
> > > > > +    items:
> > > > > +      - const: bus
> > > > > +      - const: mod
> > > >
> > > > I doubt this actually is a module clock. Based on the usage in your
> > > > device tree patch, and the csi driver in the old linux-sunxi kernel,
> > > > the clock rate is set to 24 MHz, or whatever the sensor requires for
> > > > MCLK.
> > >
> > > I'm working on adding support for this on the R40, and it seems with
> > > this SoC the picture is much clearer. It has the same CSI interface
> > > block, but the CCU has the clocks correctly named. We have:
> > >
> > >   - CSI_MCLK0
> > >   - CSI_MCLK1
> > >   - CSI_SCLK
> > >
> > > in addition to the bus clocks.
> > >
> > > The CSI section also explains the clock signals:
> > >
> > >     6.1.3.2. Clock Sources
> > >     Two Clocks need to be configured for CSI controller. CSI0/1_MCLK
> > >     provides the master clock for sensor and other devices. CSI_SCLK
> > >     is the top clock for the whole CSI module.
> > >
> > > So it would seem the ISP clock we currently have in the DT is simply
> > > the module clock shared by all CSI-related hardware blocks, and the
> > > module clock is bogus.
> >
> > I don't think it is. It looks like there's no ISP in the R40 CSI
> > controllers, so that would mean that we don't have an ISP clock, and
> > the SCLK is the module clock.
> >
> > Does that make sense?
>
> Right. That's another way to put it. The point is I believe the
> CSI[01]_CLK clocks on the A10/A20 are simply the MCLK outputs.

Looking at the Allwinner BSP again, it looks like you're right. I'll
send some patches to remove that clock from the binding.

Thanks!
Maxime
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
new file mode 100644
index 000000000000..97c9fc3b5050
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/allwinner,sun4i-a10-csi.yaml
@@ -0,0 +1,94 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/allwinner,sun4i-a10-csi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Allwinner A10 CMOS Sensor Interface (CSI) Device Tree Bindings
+
+maintainers:
+  - Chen-Yu Tsai <wens@csie.org>
+  - Maxime Ripard <maxime.ripard@bootlin.com>
+
+description: |-
+  The Allwinner A10 and later has a CMOS Sensor Interface to retrieve
+  frames from a parallel or BT656 sensor.
+
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - allwinner,sun7i-a20-csi0
+          - const: allwinner,sun4i-a10-csi0
+
+      - items:
+          - const: allwinner,sun4i-a10-csi0
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: The CSI interface clock
+      - description: The CSI module clock
+      - description: The CSI ISP clock
+      - description: The CSI DRAM clock
+
+  clock-names:
+    items:
+      - const: bus
+      - const: mod
+      - const: isp
+      - const: ram
+
+  resets:
+    description: The reset line driver this IP
+    maxItems: 1
+
+  pinctrl-0:
+    minItems: 1
+
+  pinctrl-names:
+    const: default
+
+  port:
+    type: object
+    additionalProperties: false
+
+    properties:
+      endpoint:
+        properties:
+          bus-width:
+            const: 8
+            description: Number of data lines actively used.
+
+          data-active: true
+          hsync-active: true
+          pclk-sample: true
+          remote-endpoint: true
+          vsync-active: true
+
+        required:
+          - bus-width
+          - data-active
+          - hsync-active
+          - pclk-sample
+          - remote-endpoint
+          - vsync-active
+
+    required:
+      - endpoint
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+additionalProperties: false
+...