On Tue, Oct 1, 2019 at 4:52 PM Maxime Ripard <mripard@xxxxxxxxxx> 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@xxxxxxxx> wrote: > > > > > > Hi, > > > > > > Sorry for chiming in so late. > > > > > > On Thu, Jul 11, 2019 at 8:15 PM Maxime Ripard <maxime.ripard@xxxxxxxxxxx> 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@xxxxxxxxxx> > > > > Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxx> > > > > --- > > > > 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@xxxxxxxx> > > > > + - Maxime Ripard <maxime.ripard@xxxxxxxxxxx> > > > > + > > > > +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