lkml.org 
[lkml]   [2020]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 1/3] dt-bindings: media: imx258: Add bindings for IMX258 sensor
On Mon, Sep 21, 2020 at 5:27 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Mon, Sep 14, 2020 at 02:13:10PM -0600, Rob Herring wrote:
> > On Wed, Sep 02, 2020 at 09:18:08AM +0200, Krzysztof Kozlowski wrote:
> > > Add bindings for the IMX258 camera sensor. The bindings, just like the
> > > driver, are quite limited, e.g. do not support regulator supplies.
> >
> > Bindings should be complete, not what a driver happens to currently
> > support.
>
> I'll add then more complete picture.
>
> >
> > >
> > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> > >
> > > ---
> > >
> > > Changes since v1:
> > > 1. None
> > > ---
> > > .../devicetree/bindings/media/i2c/imx258.yaml | 92 ++++++++++++++++++++++
> > > MAINTAINERS | 1 +
> > > 2 files changed, 93 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/media/i2c/imx258.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/i2c/imx258.yaml b/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> > > new file mode 100644
> > > index 000000000000..ef789ad31143
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/i2c/imx258.yaml
> > > @@ -0,0 +1,92 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/i2c/imx258.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Sony IMX258 13 Mpixel CMOS Digital Image Sensor
> > > +
> > > +maintainers:
> > > + - Krzysztof Kozlowski <krzk@kernel.org>
> > > +
> > > +description: |-
> > > + IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel
> > > + type stacked image sensor with a square pixel array of size 4208 x 3120. It
> > > + is programmable through I2C interface. Image data is sent through MIPI
> > > + CSI-2.
> > > +
> > > +properties:
> > > + compatible:
> > > + const: sony,imx258
> > > +
> > > + clocks:
> > > + maxItems: 1
> > > +
> > > + clock-frequency:
> > > + description: Frequency of input clock if clock is not provided
> > > + deprecated: true
> >
> > Why are we adding something deprecated on a new binding?
>
> My intention was also to document it but indeed easier to skip it.
>
> >
> > > + const: 19200000
> > > +
> > > + reg:
> > > + maxItems: 1
> > > +
> > > + # See ../video-interfaces.txt for more details
> > > + port:
> > > + type: object
> > > + properties:
> > > + endpoint:
> > > + type: object
> > > + properties:
> > > + data-lanes:
> > > + items:
> > > + - const: 1
> > > + - const: 2
> > > + - const: 3
> > > + - const: 4
> >
> > If this is the only config, why does it need to be in DT?
>
> The sensor is capable of two settings: two lanes (1 and 2) and four
> lanes described above. However Linux driver requires the latter (four
> lanes, 1+2+3+4).
>
> If I were to describe the bindings for HW, someone would really be
> confused and try to use two lanes setup, which won't work. Driver won't
> allow it.

If someone has h/w with only 2 lanes connected, then they have to go
add support to the driver whether we've documented 2 lanes in the
binding or not.

> I understand that bindings document the HW and describe its interface
> but do we really want to put "theoretical" bindings which cannot be used
> in practice with Linux kernel?
>
> If yes, how to nicely document this that only one setting is currently
> working?

You don't, at least in the binding. That's a driver issue. Bindings
are separate. They are stored in the kernel tree for convenience, not
because they are part of the kernel.

Rob

\
 
 \ /
  Last update: 2020-09-22 21:24    [W:0.046 / U:0.900 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site