Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / Atom feed
From: Sakari Ailus <sakari.ailus@iki.fi>
To: Robert Foss <robert.foss@linaro.org>
Cc: "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
	<devicetree@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Dongchun Zhu <dongchun.zhu@mediatek.com>,
	Tomasz Figa <tfiga@chromium.org>,
	Maxime Ripard <maxime@cerno.tech>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Fabio Estevam <festevam@gmail.com>,
	"moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE"
	<linux-arm-kernel@lists.infradead.org>,
	linux-media <linux-media@vger.kernel.org>
Subject: Re: [PATCH v6 1/3] media: dt-bindings: ov8856: Document YAML bindings
Date: Sat, 4 Apr 2020 02:27:36 +0300
Message-ID: <20200403232736.GA6127@valkosipuli.retiisi.org.uk> (raw)
In-Reply-To: <CAG3jFyvUd08U9yNVPUD9Y=nd5Xpcx34GcHJRhtvAAycoq3qimg@mail.gmail.com>

Hi Robert,

On Thu, Apr 02, 2020 at 12:10:00PM +0200, Robert Foss wrote:
> Hey Maxime,
> 
> On Wed, 1 Apr 2020 at 10:07, Maxime Ripard <maxime@cerno.tech> wrote:
> >
> > Hi,
> >
> > On Tue, Mar 31, 2020 at 03:33:44PM +0200, Robert Foss wrote:
> > > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > >
> > > This patch adds documentation of device tree in YAML schema for the
> > > OV8856 CMOS image sensor.
> > >
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > Signed-off-by: Robert Foss <robert.foss@linaro.org>
> > > ---
> > >
> > > - Changes since v5:
> > >   * Add assigned-clocks and assigned-clock-rates
> > >   * robher: dt-schema errors
> > >
> > > - Changes since v4:
> > >   * Fabio: Change reset-gpio to GPIO_ACTIVE_LOW, explain in description
> > >   * Add clock-lanes property to example
> > >   * robher: Fix syntax error in devicetree example
> > >
> > > - Changes since v3:
> > >   * robher: Fix syntax error
> > >   * robher: Removed maxItems
> > >   * Fixes yaml 'make dt-binding-check' errors
> > >
> > > - Changes since v2:
> > >   Fixes comments from from Andy, Tomasz, Sakari, Rob.
> > >   * Convert text documentation to YAML schema.
> > >
> > > - Changes since v1:
> > >   Fixes comments from Sakari, Tomasz
> > >   * Add clock-frequency and link-frequencies in DT
> > >
> > >  .../devicetree/bindings/media/i2c/ov8856.yaml | 150 ++++++++++++++++++
> > >  MAINTAINERS                                   |   1 +
> > >  2 files changed, 151 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/media/i2c/ov8856.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/ov8856.yaml b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml
> > > new file mode 100644
> > > index 000000000000..beeddfbb8709
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/ov8856.yaml
> > > @@ -0,0 +1,150 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +# Copyright (c) 2019 MediaTek Inc.
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/i2c/ov8856.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Omnivision OV8856 CMOS Sensor Device Tree Bindings
> > > +
> > > +maintainers:
> > > +  - Ben Kao <ben.kao@intel.com>
> > > +  - Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > +
> > > +description: |-
> > > +  The Omnivision OV8856 is a high performance, 1/4-inch, 8 megapixel, CMOS
> > > +  image sensor that delivers 3264x2448 at 30fps. It provides full-frame,
> > > +  sub-sampled, and windowed 10-bit MIPI images in various formats via the
> > > +  Serial Camera Control Bus (SCCB) interface. This chip is programmable
> > > +  through I2C and two-wire SCCB. The sensor output is available via CSI-2
> > > +  serial data output (up to 4-lane).
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: ovti,ov8856
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    maxItems: 1
> > > +
> > > +  clock-names:
> > > +    description:
> > > +      Input clock for the sensor.
> > > +    items:
> > > +      - const: xvclk
> > > +
> > > +  clock-frequency:
> > > +    description:
> > > +      Frequency of the xvclk clock in Hertz.
> >
> > We also had that discussion recently for another omnivision sensor
> > (ov5645 iirc), but what is clock-frequency useful for?
> >
> > It seems that the sensor is passed in clocks, so if you need to
> > retrieve the clock rate you should use the clock API instead.
> >
> > Looking at the driver, it looks like it first retrieves the clock, set
> > it to clock-frequency, and then checks that this is OV8856_XVCLK_19_2
> > (19.2 MHz).
> 
> As far as I understand it, 19.2MHz is requirement for the sensor mode
> that currently defaults to. Some modes require higher clock speeds
> than this however.

It's very system specific. Either way, bindings should not assume a
particular driver implementation.

> 
> >
> > The datasheet says that the sensor can have any frequency in the 6 -
> > 27 MHz range, so this is a driver limitation and should be set in the
> > driver using the clock API, and you can always bail out if it doesn't
> > provide a rate that is not acceptable for the drivers assumption.
> >
> > In any case, you don't need clock-frequency here...
> 
> So your suggestion is that we remove all clocks-rate properties, and
> replace the clk_get_rate() calls in the driver with clk_set_rate()
> calls for the desired frequencies?

The driver shouldn't set the rate here unless it gets it from DT (but that
was not the intention). So the driver should get the frequency instead.

-- 
Regards,

Sakari Ailus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply index

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-31 13:33 [PATCH v3 0/3] media: ov8856: Add devicetree support Robert Foss
2020-03-31 13:33 ` [PATCH v6 1/3] media: dt-bindings: ov8856: Document YAML bindings Robert Foss
2020-03-31 15:12   ` Marco Felsch
2020-04-02  9:57     ` Robert Foss
2020-04-03 19:21       ` Marco Felsch
2020-04-01  8:07   ` Maxime Ripard
2020-04-02 10:10     ` Robert Foss
2020-04-03 23:27       ` Sakari Ailus [this message]
2020-04-04  9:34         ` Maxime Ripard
2020-04-06  8:25           ` Robert Foss
2020-04-06  8:35           ` Sakari Ailus
2020-04-07  8:36             ` Maxime Ripard
2020-04-07 11:29               ` Robert Foss
2020-04-07 12:32                 ` Maxime Ripard
2020-04-07 15:47                   ` Robert Foss
2020-04-07 16:39                     ` Sakari Ailus
2020-04-07 16:46                       ` Tomasz Figa
2020-04-07 17:20                         ` Sakari Ailus
2020-04-08 12:21                           ` Maxime Ripard
2020-04-08 12:35                             ` Tomasz Figa
2020-04-08 13:43                               ` Maxime Ripard
2020-04-08 15:28                                 ` Sakari Ailus
2020-04-08 15:30                                   ` Sakari Ailus
2020-04-08 16:34                                     ` Andy Shevchenko
2020-04-15 10:18                                     ` Maxime Ripard
2020-04-15 11:10                                       ` Robert Foss
2020-04-15 16:16                                       ` Sakari Ailus
2020-04-20 15:02                                         ` Maxime Ripard
2020-04-09  8:32                                   ` Robert Foss
2020-04-07 16:20                   ` Sakari Ailus
2020-04-04  9:23       ` Maxime Ripard
2020-03-31 13:33 ` [PATCH v3 2/3] media: ov8856: Add devicetree support Robert Foss
2020-03-31 14:01   ` Andy Shevchenko
2020-04-06 13:37     ` Robert Foss
2020-04-06 15:06       ` Andy Shevchenko
2020-04-06 15:25         ` Robert Foss
2020-04-03 23:33   ` Sakari Ailus
2020-03-31 13:33 ` [PATCH v3 3/3] media: ov8856: Implement sensor module revision identification Robert Foss

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200403232736.GA6127@valkosipuli.retiisi.org.uk \
    --to=sakari.ailus@iki.fi \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dongchun.zhu@mediatek.com \
    --cc=festevam@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=maxime@cerno.tech \
    --cc=robert.foss@linaro.org \
    --cc=tfiga@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-ARM-Kernel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/0 linux-arm-kernel/git/0.git
	git clone --mirror https://lore.kernel.org/linux-arm-kernel/1 linux-arm-kernel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-arm-kernel linux-arm-kernel/ https://lore.kernel.org/linux-arm-kernel \
		linux-arm-kernel@lists.infradead.org
	public-inbox-index linux-arm-kernel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.infradead.lists.linux-arm-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git