| Message ID | 712c1acff963238e685cbd5c4a1b91f0ec7f9061.1657786765.git.vkh@melexis.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: i2c: mlx7502x ToF camera support | expand |
On 14/07/2022 10:34, Volodymyr Kharuk wrote: > Add device tree binding of the mlx7502x and update MAINTAINERS > > Signed-off-by: Volodymyr Kharuk <vkh@melexis.com> Wrong subject prefix. Remove trailing "binding". > --- > .../bindings/media/i2c/melexis,mlx7502x.yaml | 146 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 147 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > > diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > new file mode 100644 > index 000000000000..4ac91f7a26b6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > @@ -0,0 +1,146 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Melexis ToF 7502x MIPI CSI-2 Sensor > + > +maintainers: > + - Volodymyr Kharuk <vkh@melexis.com> > + > +description: |- > + Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes, > + and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12. > + Sensor 75026 is QVGA, while 75027 is VGA sensor. > + If you use compatible = "melexis,mlx7502x", then autodetect will be called. > + > +properties: > + compatible: > + items: No items. You have just one item. > + - enum: > + - melexis,mlx7502x > + - melexis,mlx75026 > + - melexis,mlx75027 > + > + reg: > + maxItems: 1 > + > + assigned-clocks: true > + assigned-clock-parents: true > + assigned-clock-rates: true These are not needed. > + > + clocks: > + description: Clock frequency 8MHz > + maxItems: 1 > + > + vdda-supply: > + description: > + Definition of the regulator used as analog power supply(2.7V). s/Definition of the regulator used as // (redundant) > + > + vddif-supply: > + description: > + Definition of the regulator used as interface power supply(1.8V). s/Definition of the regulator used as // > + > + vddd-supply: > + description: > + Definition of the regulator used as digital power supply(1.2V). s/Definition of the regulator used as // > + > + vdmix-supply: > + description: > + Definition of the regulator used as mixed driver power supply(1.2V). s/Definition of the regulator used as // > + > + reset-gpios: > + maxItems: 1 > + description: Reset Sensor GPIO Control (active low) > + > + melexis,trig-gpios: > + maxItems: 1 > + description: > + Hardware Trigger GPIO Control (active low) > + When the hardware trigger mode is enabled, this GPIO is used to generate > + a low level impulse for about 100us. The impulse triggers a new > + measurement cycle. > + > + melexis,leden-gpios: > + maxItems: 1 > + description: > + Led driver enable GPIO Control (active high) > + This GPIO notifies the illumination driver when it is safe to use led > + signals from the sensor. > + > + port: > + description: MIPI CSI-2 transmitter port > + $ref: /schemas/graph.yaml#/$defs/port-base > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# > + unevaluatedProperties: false > + > + properties: > + data-lanes: > + oneOf: > + - items: > + - const: 1 > + - const: 2 > + - items: > + - const: 1 > + - const: 2 > + - const: 3 > + - const: 4 > + > + clock-noncontinuous: true > + link-frequencies: true You do not need these two, they come from video-interfaces.yaml > + > + required: > + - data-lanes > + - link-frequencies > + > +required: > + - compatible > + - reg > + - clocks > + - port > + Best regards, Krzysztof
Hi Volodymyr, Thank you for the patch. On Thu, Jul 14, 2022 at 11:34:47AM +0300, Volodymyr Kharuk wrote: > Add device tree binding of the mlx7502x and update MAINTAINERS > > Signed-off-by: Volodymyr Kharuk <vkh@melexis.com> > --- > .../bindings/media/i2c/melexis,mlx7502x.yaml | 146 ++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 147 insertions(+) > create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > > diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > new file mode 100644 > index 000000000000..4ac91f7a26b6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > @@ -0,0 +1,146 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Melexis ToF 7502x MIPI CSI-2 Sensor > + > +maintainers: > + - Volodymyr Kharuk <vkh@melexis.com> > + > +description: |- > + Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes, > + and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12. > + Sensor 75026 is QVGA, while 75027 is VGA sensor. > + If you use compatible = "melexis,mlx7502x", then autodetect will be called. I'd move this last line as a description of the compatible property, but I'm also not sure this should be mentioned in the DT bindings, as it's a driver implementation detail. I'm actually not sure we should support it with three different compatible values as proposed, as without this documentation users will have a hard time figuring out what compatible value to pick. One option would be to support the following three compatible values: compatible = "melexis,mlx75026", "melexis,mlx7502x"; compatible = "melexis,mlx75027", "melexis,mlx7502x"; compatible = "melexis,mlx7502x"; The last one only would trigger autodetection. I'm still not sure how to document that properly in bindings though. > + > +properties: > + compatible: > + items: > + - enum: > + - melexis,mlx7502x > + - melexis,mlx75026 > + - melexis,mlx75027 > + > + reg: > + maxItems: 1 > + > + assigned-clocks: true > + assigned-clock-parents: true > + assigned-clock-rates: true > + > + clocks: > + description: Clock frequency 8MHz > + maxItems: 1 > + > + vdda-supply: > + description: > + Definition of the regulator used as analog power supply(2.7V). > + > + vddif-supply: > + description: > + Definition of the regulator used as interface power supply(1.8V). > + > + vddd-supply: > + description: > + Definition of the regulator used as digital power supply(1.2V). > + > + vdmix-supply: > + description: > + Definition of the regulator used as mixed driver power supply(1.2V). > + > + reset-gpios: > + maxItems: 1 > + description: Reset Sensor GPIO Control (active low) > + > + melexis,trig-gpios: > + maxItems: 1 > + description: > + Hardware Trigger GPIO Control (active low) > + When the hardware trigger mode is enabled, this GPIO is used to generate > + a low level impulse for about 100us. The impulse triggers a new > + measurement cycle. > + > + melexis,leden-gpios: > + maxItems: 1 > + description: > + Led driver enable GPIO Control (active high) > + This GPIO notifies the illumination driver when it is safe to use led > + signals from the sensor. As far as I understand this isn't an input of the sensor, but a signal that is driven by the driver and controls the separate illuminator. It shouldn't be specified in this binding, as it's not a property of the sensor. You should instead have a DT node for the illumination driver. > + port: > + description: MIPI CSI-2 transmitter port > + $ref: /schemas/graph.yaml#/$defs/port-base > + > + properties: > + endpoint: > + $ref: /schemas/media/video-interfaces.yaml# > + unevaluatedProperties: false > + > + properties: > + data-lanes: > + oneOf: > + - items: > + - const: 1 > + - const: 2 > + - items: > + - const: 1 > + - const: 2 > + - const: 3 > + - const: 4 > + > + clock-noncontinuous: true > + link-frequencies: true > + > + required: > + - data-lanes > + - link-frequencies > + > +required: > + - compatible > + - reg > + - clocks > + - port Let's make the supplies mandatory too, as the chip can't operate without them. > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/gpio/gpio.h> > + > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + mlx7502x: camera@57 { > + compatible = "melexis,mlx7502x"; > + reg = <0x57>; > + clocks = <&mlx7502x_clk>; > + > + assigned-clocks = <&mlx7502x_clk>; > + assigned-clock-parents = <&mlx7502x_clk_parent>; > + assigned-clock-rates = <8000000>; > + > + vddd-supply = <&mlx_7502x_reg>; > + > + reset-gpios = <&gpio_exp 6 GPIO_ACTIVE_HIGH>; > + melexis,trig-gpios = <&gpio_exp 2 GPIO_ACTIVE_HIGH>; > + melexis,leden-gpios = <&gpio_exp 7 GPIO_ACTIVE_HIGH>; > + > + port { > + mlx7502x_out_mipi_csi2: endpoint { > + remote-endpoint = <&mipi_csi2_from_mlx7502x>; > + data-lanes = <1 2 3 4>; > + link-frequencies = /bits/ 64 < 960000000 > + 904000000 > + 800000000 > + 704000000 > + 600000000 > + 300000000 >; > + }; > + }; > + }; > + }; > + > +... > diff --git a/MAINTAINERS b/MAINTAINERS > index 1a68d888ee14..b00a726bb3db 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12678,6 +12678,7 @@ M: Volodymyr Kharuk <vkh@melexis.com> > L: linux-media@vger.kernel.org > S: Supported > W: http://www.melexis.com > +F: Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > F: include/uapi/linux/mlx7502x.h > > MELFAS MIP4 TOUCHSCREEN DRIVER
On 14/07/2022 12:06, Laurent Pinchart wrote: > Hi Volodymyr, > > Thank you for the patch. > > On Thu, Jul 14, 2022 at 11:34:47AM +0300, Volodymyr Kharuk wrote: >> Add device tree binding of the mlx7502x and update MAINTAINERS >> >> Signed-off-by: Volodymyr Kharuk <vkh@melexis.com> >> --- >> .../bindings/media/i2c/melexis,mlx7502x.yaml | 146 ++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 147 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml >> >> diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml >> new file mode 100644 >> index 000000000000..4ac91f7a26b6 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml >> @@ -0,0 +1,146 @@ >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Melexis ToF 7502x MIPI CSI-2 Sensor >> + >> +maintainers: >> + - Volodymyr Kharuk <vkh@melexis.com> >> + >> +description: |- >> + Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes, >> + and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12. >> + Sensor 75026 is QVGA, while 75027 is VGA sensor. >> + If you use compatible = "melexis,mlx7502x", then autodetect will be called. > > I'd move this last line as a description of the compatible property, but > I'm also not sure this should be mentioned in the DT bindings, as it's a > driver implementation detail. I'm actually not sure we should support it > with three different compatible values as proposed, as without this > documentation users will have a hard time figuring out what compatible > value to pick. > > One option would be to support the following three compatible values: > > compatible = "melexis,mlx75026", "melexis,mlx7502x"; > compatible = "melexis,mlx75027", "melexis,mlx7502x"; > compatible = "melexis,mlx7502x"; > > The last one only would trigger autodetection. I'm still not sure how to > document that properly in bindings though. I missed that part of binding. Wildcards are not allowed in compatible, so mlx7502x has to go. Anyway what does this autodetection mean? Best regards, Krzysztof
On Thu, Jul 14, 2022 at 12:35:52PM +0200, Krzysztof Kozlowski wrote: > On 14/07/2022 12:06, Laurent Pinchart wrote: > > Hi Volodymyr, > > > > Thank you for the patch. > > > > On Thu, Jul 14, 2022 at 11:34:47AM +0300, Volodymyr Kharuk wrote: > >> Add device tree binding of the mlx7502x and update MAINTAINERS > >> > >> Signed-off-by: Volodymyr Kharuk <vkh@melexis.com> > >> --- > >> .../bindings/media/i2c/melexis,mlx7502x.yaml | 146 ++++++++++++++++++ > >> MAINTAINERS | 1 + > >> 2 files changed, 147 insertions(+) > >> create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > >> > >> diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > >> new file mode 100644 > >> index 000000000000..4ac91f7a26b6 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > >> @@ -0,0 +1,146 @@ > >> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > >> +%YAML 1.2 > >> +--- > >> +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml# > >> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >> + > >> +title: Melexis ToF 7502x MIPI CSI-2 Sensor > >> + > >> +maintainers: > >> + - Volodymyr Kharuk <vkh@melexis.com> > >> + > >> +description: |- > >> + Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes, > >> + and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12. > >> + Sensor 75026 is QVGA, while 75027 is VGA sensor. > >> + If you use compatible = "melexis,mlx7502x", then autodetect will be called. > > > > I'd move this last line as a description of the compatible property, but > > I'm also not sure this should be mentioned in the DT bindings, as it's a > > driver implementation detail. I'm actually not sure we should support it > > with three different compatible values as proposed, as without this > > documentation users will have a hard time figuring out what compatible > > value to pick. > > > > One option would be to support the following three compatible values: > > > > compatible = "melexis,mlx75026", "melexis,mlx7502x"; > > compatible = "melexis,mlx75027", "melexis,mlx7502x"; > > compatible = "melexis,mlx7502x"; > > > > The last one only would trigger autodetection. I'm still not sure how to > > document that properly in bindings though. > > I missed that part of binding. > > Wildcards are not allowed in compatible, so mlx7502x has to go. Really ? We've had fallback generic compatible strings since the beginning. > Anyway what does this autodetection mean? As far as I understand, it means that the driver will use a hardware identification register to figure out if the sensor is a 75026 or 75027. The upside is that one doesn't need to change the device tree when swapping between those two sensors. The downside is that the sensor needs to be powered up at probe time. Depending on the platform, one of those two behaviours is preferred. Auto-detection is nice, but in laptops or tablets (not a use case for this particular device, but the problem applies to camera sensors in general), it would mean that the privacy LED of the camera could be briefly lit at boot time due to the sensor being powered on, which can worry users.
On 14/07/2022 12:45, Laurent Pinchart wrote: > On Thu, Jul 14, 2022 at 12:35:52PM +0200, Krzysztof Kozlowski wrote: >> On 14/07/2022 12:06, Laurent Pinchart wrote: >>> Hi Volodymyr, >>> >>> Thank you for the patch. >>> >>> On Thu, Jul 14, 2022 at 11:34:47AM +0300, Volodymyr Kharuk wrote: >>>> Add device tree binding of the mlx7502x and update MAINTAINERS >>>> >>>> Signed-off-by: Volodymyr Kharuk <vkh@melexis.com> >>>> --- >>>> .../bindings/media/i2c/melexis,mlx7502x.yaml | 146 ++++++++++++++++++ >>>> MAINTAINERS | 1 + >>>> 2 files changed, 147 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml >>>> new file mode 100644 >>>> index 000000000000..4ac91f7a26b6 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml >>>> @@ -0,0 +1,146 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: Melexis ToF 7502x MIPI CSI-2 Sensor >>>> + >>>> +maintainers: >>>> + - Volodymyr Kharuk <vkh@melexis.com> >>>> + >>>> +description: |- >>>> + Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes, >>>> + and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12. >>>> + Sensor 75026 is QVGA, while 75027 is VGA sensor. >>>> + If you use compatible = "melexis,mlx7502x", then autodetect will be called. >>> >>> I'd move this last line as a description of the compatible property, but >>> I'm also not sure this should be mentioned in the DT bindings, as it's a >>> driver implementation detail. I'm actually not sure we should support it >>> with three different compatible values as proposed, as without this >>> documentation users will have a hard time figuring out what compatible >>> value to pick. >>> >>> One option would be to support the following three compatible values: >>> >>> compatible = "melexis,mlx75026", "melexis,mlx7502x"; >>> compatible = "melexis,mlx75027", "melexis,mlx7502x"; >>> compatible = "melexis,mlx7502x"; >>> >>> The last one only would trigger autodetection. I'm still not sure how to >>> document that properly in bindings though. >> >> I missed that part of binding. >> >> Wildcards are not allowed in compatible, so mlx7502x has to go. > > Really ? We've had fallback generic compatible strings since the > beginning. Fallback generic compatibles are allowed. Wildcards not. Wildcards were actually never explicitly allowed, they just slipped in to many bindings... We have several discussions on this on mailing list, so no real point to repeat the arguments. There is a difference between generic fallback. If the device follows clear specification and version, e.g. "foo-bar-v4", you can use it for generic compatible. This is more common in SoC components. Requirement - there is a clear mapping between versions and SoCs. > >> Anyway what does this autodetection mean? > > As far as I understand, it means that the driver will use a hardware > identification register to figure out if the sensor is a 75026 or 75027. Then there is no need to define 75027 compatible. DT is for cases where autodetection does not work... > The upside is that one doesn't need to change the device tree when > swapping between those two sensors. The downside is that the sensor > needs to be powered up at probe time. Depending on the platform, one of > those two behaviours is preferred. Auto-detection is nice, but in > laptops or tablets (not a use case for this particular device, but the > problem applies to camera sensors in general), it would mean that the > privacy LED of the camera could be briefly lit at boot time due to the > sensor being powered on, which can worry users. OK, that's reasonable argument for dedicated compatible but I don't understand why you cannot perform autodetection the moment device is actually powered up (first time). I understand it is nice and easy to make everything in the probe and most devices perform it that way. But if you don't want to do it in the probe - DT is not a workaround for this... Best regards, Krzysztof
On 14/07/2022 13:00, Krzysztof Kozlowski wrote: > On 14/07/2022 12:45, Laurent Pinchart wrote: >> On Thu, Jul 14, 2022 at 12:35:52PM +0200, Krzysztof Kozlowski wrote: >>> On 14/07/2022 12:06, Laurent Pinchart wrote: >>>> Hi Volodymyr, >>>> >>>> Thank you for the patch. >>>> >>>> On Thu, Jul 14, 2022 at 11:34:47AM +0300, Volodymyr Kharuk wrote: >>>>> Add device tree binding of the mlx7502x and update MAINTAINERS >>>>> >>>>> Signed-off-by: Volodymyr Kharuk <vkh@melexis.com> >>>>> --- >>>>> .../bindings/media/i2c/melexis,mlx7502x.yaml | 146 ++++++++++++++++++ >>>>> MAINTAINERS | 1 + >>>>> 2 files changed, 147 insertions(+) >>>>> create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml >>>>> >>>>> diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml >>>>> new file mode 100644 >>>>> index 000000000000..4ac91f7a26b6 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml >>>>> @@ -0,0 +1,146 @@ >>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) >>>>> +%YAML 1.2 >>>>> +--- >>>>> +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml# >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>>> + >>>>> +title: Melexis ToF 7502x MIPI CSI-2 Sensor >>>>> + >>>>> +maintainers: >>>>> + - Volodymyr Kharuk <vkh@melexis.com> >>>>> + >>>>> +description: |- >>>>> + Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes, >>>>> + and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12. >>>>> + Sensor 75026 is QVGA, while 75027 is VGA sensor. >>>>> + If you use compatible = "melexis,mlx7502x", then autodetect will be called. >>>> >>>> I'd move this last line as a description of the compatible property, but >>>> I'm also not sure this should be mentioned in the DT bindings, as it's a >>>> driver implementation detail. I'm actually not sure we should support it >>>> with three different compatible values as proposed, as without this >>>> documentation users will have a hard time figuring out what compatible >>>> value to pick. >>>> >>>> One option would be to support the following three compatible values: >>>> >>>> compatible = "melexis,mlx75026", "melexis,mlx7502x"; >>>> compatible = "melexis,mlx75027", "melexis,mlx7502x"; >>>> compatible = "melexis,mlx7502x"; >>>> >>>> The last one only would trigger autodetection. I'm still not sure how to >>>> document that properly in bindings though. >>> >>> I missed that part of binding. >>> >>> Wildcards are not allowed in compatible, so mlx7502x has to go. >> >> Really ? We've had fallback generic compatible strings since the >> beginning. > > Fallback generic compatibles are allowed. Wildcards not. Wildcards were > actually never explicitly allowed, they just slipped in to many > bindings... We have several discussions on this on mailing list, so no > real point to repeat the arguments. Although I forgot one more acceptable case - family of devices followed by a specific compatible. However that "family" cannot be on its own. Best regards, Krzysztof
On Thu, Jul 14, 2022 at 01:00:24PM +0200, Krzysztof Kozlowski wrote: > On 14/07/2022 12:45, Laurent Pinchart wrote: > > On Thu, Jul 14, 2022 at 12:35:52PM +0200, Krzysztof Kozlowski wrote: > >> On 14/07/2022 12:06, Laurent Pinchart wrote: > >>> Hi Volodymyr, > >>> > >>> Thank you for the patch. > >>> > >>> On Thu, Jul 14, 2022 at 11:34:47AM +0300, Volodymyr Kharuk wrote: > >>>> Add device tree binding of the mlx7502x and update MAINTAINERS > >>>> > >>>> Signed-off-by: Volodymyr Kharuk <vkh@melexis.com> > >>>> --- > >>>> .../bindings/media/i2c/melexis,mlx7502x.yaml | 146 ++++++++++++++++++ > >>>> MAINTAINERS | 1 + > >>>> 2 files changed, 147 insertions(+) > >>>> create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > >>>> new file mode 100644 > >>>> index 000000000000..4ac91f7a26b6 > >>>> --- /dev/null > >>>> +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > >>>> @@ -0,0 +1,146 @@ > >>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > >>>> +%YAML 1.2 > >>>> +--- > >>>> +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml# > >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# > >>>> + > >>>> +title: Melexis ToF 7502x MIPI CSI-2 Sensor > >>>> + > >>>> +maintainers: > >>>> + - Volodymyr Kharuk <vkh@melexis.com> > >>>> + > >>>> +description: |- > >>>> + Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes, > >>>> + and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12. > >>>> + Sensor 75026 is QVGA, while 75027 is VGA sensor. > >>>> + If you use compatible = "melexis,mlx7502x", then autodetect will be called. > >>> > >>> I'd move this last line as a description of the compatible property, but > >>> I'm also not sure this should be mentioned in the DT bindings, as it's a > >>> driver implementation detail. I'm actually not sure we should support it > >>> with three different compatible values as proposed, as without this > >>> documentation users will have a hard time figuring out what compatible > >>> value to pick. > >>> > >>> One option would be to support the following three compatible values: > >>> > >>> compatible = "melexis,mlx75026", "melexis,mlx7502x"; > >>> compatible = "melexis,mlx75027", "melexis,mlx7502x"; > >>> compatible = "melexis,mlx7502x"; > >>> > >>> The last one only would trigger autodetection. I'm still not sure how to > >>> document that properly in bindings though. > >> > >> I missed that part of binding. > >> > >> Wildcards are not allowed in compatible, so mlx7502x has to go. > > > > Really ? We've had fallback generic compatible strings since the > > beginning. > > Fallback generic compatibles are allowed. Wildcards not. Wildcards were > actually never explicitly allowed, they just slipped in to many > bindings... We have several discussions on this on mailing list, so no > real point to repeat the arguments. > > There is a difference between generic fallback. If the device follows > clear specification and version, e.g. "foo-bar-v4", you can use it for > generic compatible. This is more common in SoC components. Requirement - > there is a clear mapping between versions and SoCs. I'm not sure to see a clear difference between the two concepts. > >> Anyway what does this autodetection mean? > > > > As far as I understand, it means that the driver will use a hardware > > identification register to figure out if the sensor is a 75026 or 75027. > > Then there is no need to define 75027 compatible. DT is for cases where > autodetection does not work... It's autodetection of the exact device model, those are I2C devices so we still need DT, and we still need to know that it's one of the MLX75026 or MLX75027. > > The upside is that one doesn't need to change the device tree when > > swapping between those two sensors. The downside is that the sensor > > needs to be powered up at probe time. Depending on the platform, one of > > those two behaviours is preferred. Auto-detection is nice, but in > > laptops or tablets (not a use case for this particular device, but the > > problem applies to camera sensors in general), it would mean that the > > privacy LED of the camera could be briefly lit at boot time due to the > > sensor being powered on, which can worry users. > > OK, that's reasonable argument for dedicated compatible but I don't > understand why you cannot perform autodetection the moment device is > actually powered up (first time). I understand it is nice and easy to > make everything in the probe and most devices perform it that way. But > if you don't want to do it in the probe - DT is not a workaround for this... For cameras, we often deal with complex pipelines with multiple external devices and multiple IP cores, with drivers that need to communicate with each other to initialize the complete camera system. For instance, each camera-related component in the system registers itself in a media graph that can be queried from userspace and exposes information about all devices, including their model. There's no power up of any device when this query is being performed from userspace. It could possibly be changed (and maybe it should, for reasons unrelated to this discussion), but we're looking at pretty much a complete redesign of V4L2 and MC then.
On 14/07/2022 13:12, Laurent Pinchart wrote: >>>>> One option would be to support the following three compatible values: >>>>> >>>>> compatible = "melexis,mlx75026", "melexis,mlx7502x"; >>>>> compatible = "melexis,mlx75027", "melexis,mlx7502x"; >>>>> compatible = "melexis,mlx7502x"; >>>>> >>>>> The last one only would trigger autodetection. I'm still not sure how to >>>>> document that properly in bindings though. >>>> >>>> I missed that part of binding. >>>> >>>> Wildcards are not allowed in compatible, so mlx7502x has to go. >>> >>> Really ? We've had fallback generic compatible strings since the >>> beginning. >> >> Fallback generic compatibles are allowed. Wildcards not. Wildcards were >> actually never explicitly allowed, they just slipped in to many >> bindings... We have several discussions on this on mailing list, so no >> real point to repeat the arguments. >> >> There is a difference between generic fallback. If the device follows >> clear specification and version, e.g. "foo-bar-v4", you can use it for >> generic compatible. This is more common in SoC components. Requirement - >> there is a clear mapping between versions and SoCs. > > I'm not sure to see a clear difference between the two concepts. The clear difference is that you have a versioned and re-usable hardware block plus clear mapping which version goes to which SoC. Version numbers usually start with 1, not with 75025. 75025 is a model name. >>>> Anyway what does this autodetection mean? >>> >>> As far as I understand, it means that the driver will use a hardware >>> identification register to figure out if the sensor is a 75026 or 75027. >> >> Then there is no need to define 75027 compatible. DT is for cases where >> autodetection does not work... > > It's autodetection of the exact device model, those are I2C devices so > we still need DT, and we still need to know that it's one of the > MLX75026 or MLX75027. > >>> The upside is that one doesn't need to change the device tree when >>> swapping between those two sensors. The downside is that the sensor >>> needs to be powered up at probe time. Depending on the platform, one of >>> those two behaviours is preferred. Auto-detection is nice, but in >>> laptops or tablets (not a use case for this particular device, but the >>> problem applies to camera sensors in general), it would mean that the >>> privacy LED of the camera could be briefly lit at boot time due to the >>> sensor being powered on, which can worry users. >> >> OK, that's reasonable argument for dedicated compatible but I don't >> understand why you cannot perform autodetection the moment device is >> actually powered up (first time). I understand it is nice and easy to >> make everything in the probe and most devices perform it that way. But >> if you don't want to do it in the probe - DT is not a workaround for this... > > For cameras, we often deal with complex pipelines with multiple external > devices and multiple IP cores, with drivers that need to communicate > with each other to initialize the complete camera system. For instance, > each camera-related component in the system registers itself in a media > graph that can be queried from userspace and exposes information about > all devices, including their model. There's no power up of any device > when this query is being performed from userspace. It could possibly be > changed (and maybe it should, for reasons unrelated to this discussion), > but we're looking at pretty much a complete redesign of V4L2 and MC > then. Is then autodetection a real use case since you have to power up the sensor each time system boots and this violates privacy? Several I2C sensors do not care about this and they always do it on power up, so aren't we solving here something unimportant? Best regards, Krzysztof
On Thu, Jul 14, 2022 at 01:23:41PM +0200, Krzysztof Kozlowski wrote: > On 14/07/2022 13:12, Laurent Pinchart wrote: > >>>>> One option would be to support the following three compatible values: > >>>>> > >>>>> compatible = "melexis,mlx75026", "melexis,mlx7502x"; > >>>>> compatible = "melexis,mlx75027", "melexis,mlx7502x"; > >>>>> compatible = "melexis,mlx7502x"; > >>>>> > >>>>> The last one only would trigger autodetection. I'm still not sure how to > >>>>> document that properly in bindings though. > >>>> > >>>> I missed that part of binding. > >>>> > >>>> Wildcards are not allowed in compatible, so mlx7502x has to go. > >>> > >>> Really ? We've had fallback generic compatible strings since the > >>> beginning. > >> > >> Fallback generic compatibles are allowed. Wildcards not. Wildcards were > >> actually never explicitly allowed, they just slipped in to many > >> bindings... We have several discussions on this on mailing list, so no > >> real point to repeat the arguments. > >> > >> There is a difference between generic fallback. If the device follows > >> clear specification and version, e.g. "foo-bar-v4", you can use it for > >> generic compatible. This is more common in SoC components. Requirement - > >> there is a clear mapping between versions and SoCs. > > > > I'm not sure to see a clear difference between the two concepts. > > The clear difference is that you have a versioned and re-usable hardware > block plus clear mapping which version goes to which SoC. Version > numbers usually start with 1, not with 75025. 75025 is a model name. How about Documentation/devicetree/bindings/serial/renesas,scif.yaml for instance, where the version number isn't known and the SoC name is used instead ? Is that acceptable ? How should we deal with devices that have different models, where the model is irrelevant to the kernel driver, but relevant to userspace ? Imagine, for instance, a light sensor with 10 models than only differ by the filter they use to tune the sensitivity to different light spectrums ? They are all "compatible" from a software point of view, would the driver need to list all 10 compatible strings ? > >>>> Anyway what does this autodetection mean? > >>> > >>> As far as I understand, it means that the driver will use a hardware > >>> identification register to figure out if the sensor is a 75026 or 75027. > >> > >> Then there is no need to define 75027 compatible. DT is for cases where > >> autodetection does not work... > > > > It's autodetection of the exact device model, those are I2C devices so > > we still need DT, and we still need to know that it's one of the > > MLX75026 or MLX75027. > > > >>> The upside is that one doesn't need to change the device tree when > >>> swapping between those two sensors. The downside is that the sensor > >>> needs to be powered up at probe time. Depending on the platform, one of > >>> those two behaviours is preferred. Auto-detection is nice, but in > >>> laptops or tablets (not a use case for this particular device, but the > >>> problem applies to camera sensors in general), it would mean that the > >>> privacy LED of the camera could be briefly lit at boot time due to the > >>> sensor being powered on, which can worry users. > >> > >> OK, that's reasonable argument for dedicated compatible but I don't > >> understand why you cannot perform autodetection the moment device is > >> actually powered up (first time). I understand it is nice and easy to > >> make everything in the probe and most devices perform it that way. But > >> if you don't want to do it in the probe - DT is not a workaround for this... > > > > For cameras, we often deal with complex pipelines with multiple external > > devices and multiple IP cores, with drivers that need to communicate > > with each other to initialize the complete camera system. For instance, > > each camera-related component in the system registers itself in a media > > graph that can be queried from userspace and exposes information about > > all devices, including their model. There's no power up of any device > > when this query is being performed from userspace. It could possibly be > > changed (and maybe it should, for reasons unrelated to this discussion), > > but we're looking at pretty much a complete redesign of V4L2 and MC > > then. > > Is then autodetection a real use case since you have to power up the > sensor each time system boots and this violates privacy? Several I2C > sensors do not care about this and they always do it on power up, so > aren't we solving here something unimportant? In a laptop or tablet with a camera sensor, you likely don't want autodetection. In an industrial device, you don't care, and having the ability to auto-detect the exact sensor model when booting saves cost in the production chain as a single image can work across different models.
On 14/07/2022 13:29, Laurent Pinchart wrote: > On Thu, Jul 14, 2022 at 01:23:41PM +0200, Krzysztof Kozlowski wrote: >> On 14/07/2022 13:12, Laurent Pinchart wrote: >>>>>>> One option would be to support the following three compatible values: >>>>>>> >>>>>>> compatible = "melexis,mlx75026", "melexis,mlx7502x"; >>>>>>> compatible = "melexis,mlx75027", "melexis,mlx7502x"; >>>>>>> compatible = "melexis,mlx7502x"; >>>>>>> >>>>>>> The last one only would trigger autodetection. I'm still not sure how to >>>>>>> document that properly in bindings though. >>>>>> >>>>>> I missed that part of binding. >>>>>> >>>>>> Wildcards are not allowed in compatible, so mlx7502x has to go. >>>>> >>>>> Really ? We've had fallback generic compatible strings since the >>>>> beginning. >>>> >>>> Fallback generic compatibles are allowed. Wildcards not. Wildcards were >>>> actually never explicitly allowed, they just slipped in to many >>>> bindings... We have several discussions on this on mailing list, so no >>>> real point to repeat the arguments. >>>> >>>> There is a difference between generic fallback. If the device follows >>>> clear specification and version, e.g. "foo-bar-v4", you can use it for >>>> generic compatible. This is more common in SoC components. Requirement - >>>> there is a clear mapping between versions and SoCs. >>> >>> I'm not sure to see a clear difference between the two concepts. >> >> The clear difference is that you have a versioned and re-usable hardware >> block plus clear mapping which version goes to which SoC. Version >> numbers usually start with 1, not with 75025. 75025 is a model name. > > How about Documentation/devicetree/bindings/serial/renesas,scif.yaml for > instance, where the version number isn't known and the SoC name is used > instead ? Is that acceptable ? This is the second case I mentioned - family of devices where the family fallback is not allowed to be alone. You cannot use just "renesas,scif" in DTS. > > How should we deal with devices that have different models, where the > model is irrelevant to the kernel driver, but relevant to userspace ? > Imagine, for instance, a light sensor with 10 models than only differ by > the filter they use to tune the sensitivity to different light spectrums > ? They are all "compatible" from a software point of view, would the > driver need to list all 10 compatible strings ? I don't understand that example, I mean, what's the problem here? If they are all compatible, you can use only one comaptible, e.g. melexis,mlx75026. If you ever need to differentiate it for user-space, you add specific compatible for the model and you have: melexis,mlx75027, melexis,mlx75026 If user-space needs dedicated compatibles - add them, no one here argues to not to use specific compatibles. >>> For cameras, we often deal with complex pipelines with multiple external >>> devices and multiple IP cores, with drivers that need to communicate >>> with each other to initialize the complete camera system. For instance, >>> each camera-related component in the system registers itself in a media >>> graph that can be queried from userspace and exposes information about >>> all devices, including their model. There's no power up of any device >>> when this query is being performed from userspace. It could possibly be >>> changed (and maybe it should, for reasons unrelated to this discussion), >>> but we're looking at pretty much a complete redesign of V4L2 and MC >>> then. >> >> Is then autodetection a real use case since you have to power up the >> sensor each time system boots and this violates privacy? Several I2C >> sensors do not care about this and they always do it on power up, so >> aren't we solving here something unimportant? > > In a laptop or tablet with a camera sensor, you likely don't want > autodetection. In an industrial device, you don't care, and having the > ability to auto-detect the exact sensor model when booting saves cost in > the production chain as a single image can work across different models. We talk about the case here, not generic. Do you want to have autodetection possible here or not? Best regards, Krzysztof
Hi Laurent, Thank you for your review, On Thu, Jul 14, 2022 at 01:06:35PM +0300, Laurent Pinchart wrote: > Hi Volodymyr, > > Thank you for the patch. > > On Thu, Jul 14, 2022 at 11:34:47AM +0300, Volodymyr Kharuk wrote: > > Add device tree binding of the mlx7502x and update MAINTAINERS > > > > Signed-off-by: Volodymyr Kharuk <vkh@melexis.com> > > --- > > .../bindings/media/i2c/melexis,mlx7502x.yaml | 146 ++++++++++++++++++ > > MAINTAINERS | 1 + > > 2 files changed, 147 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > > new file mode 100644 > > index 000000000000..4ac91f7a26b6 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > > @@ -0,0 +1,146 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: Melexis ToF 7502x MIPI CSI-2 Sensor > > + > > +maintainers: > > + - Volodymyr Kharuk <vkh@melexis.com> > > + > > +description: |- > > + Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes, > > + and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12. > > + Sensor 75026 is QVGA, while 75027 is VGA sensor. > > + If you use compatible = "melexis,mlx7502x", then autodetect will be called. > > I'd move this last line as a description of the compatible property, but > I'm also not sure this should be mentioned in the DT bindings, as it's a > driver implementation detail. I'm actually not sure we should support it > with three different compatible values as proposed, as without this > documentation users will have a hard time figuring out what compatible > value to pick. > > One option would be to support the following three compatible values: > > compatible = "melexis,mlx75026", "melexis,mlx7502x"; > compatible = "melexis,mlx75027", "melexis,mlx7502x"; > compatible = "melexis,mlx7502x"; > > The last one only would trigger autodetection. I'm still not sure how to > document that properly in bindings though. > > > + > > +properties: > > + compatible: > > + items: > > + - enum: > > + - melexis,mlx7502x > > + - melexis,mlx75026 > > + - melexis,mlx75027 > > + > > + reg: > > + maxItems: 1 > > + > > + assigned-clocks: true > > + assigned-clock-parents: true > > + assigned-clock-rates: true > > + > > + clocks: > > + description: Clock frequency 8MHz > > + maxItems: 1 > > + > > + vdda-supply: > > + description: > > + Definition of the regulator used as analog power supply(2.7V). > > + > > + vddif-supply: > > + description: > > + Definition of the regulator used as interface power supply(1.8V). > > + > > + vddd-supply: > > + description: > > + Definition of the regulator used as digital power supply(1.2V). > > + > > + vdmix-supply: > > + description: > > + Definition of the regulator used as mixed driver power supply(1.2V). > > + > > + reset-gpios: > > + maxItems: 1 > > + description: Reset Sensor GPIO Control (active low) > > + > > + melexis,trig-gpios: > > + maxItems: 1 > > + description: > > + Hardware Trigger GPIO Control (active low) > > + When the hardware trigger mode is enabled, this GPIO is used to generate > > + a low level impulse for about 100us. The impulse triggers a new > > + measurement cycle. > > + > > + melexis,leden-gpios: > > + maxItems: 1 > > + description: > > + Led driver enable GPIO Control (active high) > > + This GPIO notifies the illumination driver when it is safe to use led > > + signals from the sensor. > > As far as I understand this isn't an input of the sensor, but a signal > that is driven by the driver and controls the separate illuminator. It > shouldn't be specified in this binding, as it's not a property of the > sensor. You should instead have a DT node for the illumination driver. Yes, you are right. There are illuminators, which are not ready to accept the signal from the sensor, if pin levels are not right. So I need to notify illuminator somehow. Can the illumination driver be as a V4L2 subdevice? Then I can notify the illuminator via s_stream. In another case it can damage the illuminator. What do you think? > > > + port: > > + description: MIPI CSI-2 transmitter port > > + $ref: /schemas/graph.yaml#/$defs/port-base > > + > > + properties: > > + endpoint: > > + $ref: /schemas/media/video-interfaces.yaml# > > + unevaluatedProperties: false > > + > > + properties: > > + data-lanes: > > + oneOf: > > + - items: > > + - const: 1 > > + - const: 2 > > + - items: > > + - const: 1 > > + - const: 2 > > + - const: 3 > > + - const: 4 > > + > > + clock-noncontinuous: true > > + link-frequencies: true > > + > > + required: > > + - data-lanes > > + - link-frequencies > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - port > > Let's make the supplies mandatory too, as the chip can't operate without > them. Ok > > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > + #include <dt-bindings/gpio/gpio.h> > > + > > + i2c { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + mlx7502x: camera@57 { > > + compatible = "melexis,mlx7502x"; > > + reg = <0x57>; > > + clocks = <&mlx7502x_clk>; > > + > > + assigned-clocks = <&mlx7502x_clk>; > > + assigned-clock-parents = <&mlx7502x_clk_parent>; > > + assigned-clock-rates = <8000000>; > > + > > + vddd-supply = <&mlx_7502x_reg>; > > + > > + reset-gpios = <&gpio_exp 6 GPIO_ACTIVE_HIGH>; > > + melexis,trig-gpios = <&gpio_exp 2 GPIO_ACTIVE_HIGH>; > > + melexis,leden-gpios = <&gpio_exp 7 GPIO_ACTIVE_HIGH>; > > + > > + port { > > + mlx7502x_out_mipi_csi2: endpoint { > > + remote-endpoint = <&mipi_csi2_from_mlx7502x>; > > + data-lanes = <1 2 3 4>; > > + link-frequencies = /bits/ 64 < 960000000 > > + 904000000 > > + 800000000 > > + 704000000 > > + 600000000 > > + 300000000 >; > > + }; > > + }; > > + }; > > + }; > > + > > +... > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 1a68d888ee14..b00a726bb3db 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -12678,6 +12678,7 @@ M: Volodymyr Kharuk <vkh@melexis.com> > > L: linux-media@vger.kernel.org > > S: Supported > > W: http://www.melexis.com > > +F: Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > > F: include/uapi/linux/mlx7502x.h > > > > MELFAS MIP4 TOUCHSCREEN DRIVER > > -- > Regards, > > Laurent Pinchart
Hi Krzysztof, On Thu, Jul 14, 2022 at 01:56:13PM +0200, Krzysztof Kozlowski wrote: > On 14/07/2022 13:29, Laurent Pinchart wrote: > > On Thu, Jul 14, 2022 at 01:23:41PM +0200, Krzysztof Kozlowski wrote: > >> On 14/07/2022 13:12, Laurent Pinchart wrote: > >>>>>>> One option would be to support the following three compatible values: > >>>>>>> > >>>>>>> compatible = "melexis,mlx75026", "melexis,mlx7502x"; > >>>>>>> compatible = "melexis,mlx75027", "melexis,mlx7502x"; > >>>>>>> compatible = "melexis,mlx7502x"; > >>>>>>> > >>>>>>> The last one only would trigger autodetection. I'm still not sure how to > >>>>>>> document that properly in bindings though. > >>>>>> > >>>>>> I missed that part of binding. > >>>>>> > >>>>>> Wildcards are not allowed in compatible, so mlx7502x has to go. > >>>>> > >>>>> Really ? We've had fallback generic compatible strings since the > >>>>> beginning. > >>>> > >>>> Fallback generic compatibles are allowed. Wildcards not. Wildcards were > >>>> actually never explicitly allowed, they just slipped in to many > >>>> bindings... We have several discussions on this on mailing list, so no > >>>> real point to repeat the arguments. > >>>> > >>>> There is a difference between generic fallback. If the device follows > >>>> clear specification and version, e.g. "foo-bar-v4", you can use it for > >>>> generic compatible. This is more common in SoC components. Requirement - > >>>> there is a clear mapping between versions and SoCs. > >>> > >>> I'm not sure to see a clear difference between the two concepts. > >> > >> The clear difference is that you have a versioned and re-usable hardware > >> block plus clear mapping which version goes to which SoC. Version > >> numbers usually start with 1, not with 75025. 75025 is a model name. > > > > How about Documentation/devicetree/bindings/serial/renesas,scif.yaml for > > instance, where the version number isn't known and the SoC name is used > > instead ? Is that acceptable ? > > This is the second case I mentioned - family of devices where the family > fallback is not allowed to be alone. You cannot use just "renesas,scif" > in DTS. > > > > > How should we deal with devices that have different models, where the > > model is irrelevant to the kernel driver, but relevant to userspace ? > > Imagine, for instance, a light sensor with 10 models than only differ by > > the filter they use to tune the sensitivity to different light spectrums > > ? They are all "compatible" from a software point of view, would the > > driver need to list all 10 compatible strings ? > > I don't understand that example, I mean, what's the problem here? If > they are all compatible, you can use only one comaptible, e.g. > melexis,mlx75026. > > If you ever need to differentiate it for user-space, you add specific > compatible for the model and you have: > > melexis,mlx75027, melexis,mlx75026 > > If user-space needs dedicated compatibles - add them, no one here argues > to not to use specific compatibles. Thanks for explanation. Now I understand the device tree better and the whole idea behind it. I'll remove wildcard and autodetect. Instead I will use of_match_table only. > > > >>> For cameras, we often deal with complex pipelines with multiple external > >>> devices and multiple IP cores, with drivers that need to communicate > >>> with each other to initialize the complete camera system. For instance, > >>> each camera-related component in the system registers itself in a media > >>> graph that can be queried from userspace and exposes information about > >>> all devices, including their model. There's no power up of any device > >>> when this query is being performed from userspace. It could possibly be > >>> changed (and maybe it should, for reasons unrelated to this discussion), > >>> but we're looking at pretty much a complete redesign of V4L2 and MC > >>> then. > >> > >> Is then autodetection a real use case since you have to power up the > >> sensor each time system boots and this violates privacy? Several I2C > >> sensors do not care about this and they always do it on power up, so > >> aren't we solving here something unimportant? > > > > In a laptop or tablet with a camera sensor, you likely don't want > > autodetection. In an industrial device, you don't care, and having the > > ability to auto-detect the exact sensor model when booting saves cost in > > the production chain as a single image can work across different models. > > We talk about the case here, not generic. Do you want to have > autodetection possible here or not? > > Best regards, > Krzysztof
Hi Volodymyr, I've just realized your previous e-mail felt through the cracks. Sorry about that. Please see below for comments. On Fri, Jul 15, 2022 at 06:32:43PM +0300, Volodymyr Kharuk wrote: > On Thu, Jul 14, 2022 at 01:06:35PM +0300, Laurent Pinchart wrote: > > On Thu, Jul 14, 2022 at 11:34:47AM +0300, Volodymyr Kharuk wrote: > > > Add device tree binding of the mlx7502x and update MAINTAINERS > > > > > > Signed-off-by: Volodymyr Kharuk <vkh@melexis.com> > > > --- > > > .../bindings/media/i2c/melexis,mlx7502x.yaml | 146 ++++++++++++++++++ > > > MAINTAINERS | 1 + > > > 2 files changed, 147 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > > > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > > > new file mode 100644 > > > index 000000000000..4ac91f7a26b6 > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > > > @@ -0,0 +1,146 @@ > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > > > +%YAML 1.2 > > > +--- > > > +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Melexis ToF 7502x MIPI CSI-2 Sensor > > > + > > > +maintainers: > > > + - Volodymyr Kharuk <vkh@melexis.com> > > > + > > > +description: |- > > > + Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes, > > > + and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12. > > > + Sensor 75026 is QVGA, while 75027 is VGA sensor. > > > + If you use compatible = "melexis,mlx7502x", then autodetect will be called. > > > > I'd move this last line as a description of the compatible property, but > > I'm also not sure this should be mentioned in the DT bindings, as it's a > > driver implementation detail. I'm actually not sure we should support it > > with three different compatible values as proposed, as without this > > documentation users will have a hard time figuring out what compatible > > value to pick. > > > > One option would be to support the following three compatible values: > > > > compatible = "melexis,mlx75026", "melexis,mlx7502x"; > > compatible = "melexis,mlx75027", "melexis,mlx7502x"; > > compatible = "melexis,mlx7502x"; > > > > The last one only would trigger autodetection. I'm still not sure how to > > document that properly in bindings though. > > > > > + > > > +properties: > > > + compatible: > > > + items: > > > + - enum: > > > + - melexis,mlx7502x > > > + - melexis,mlx75026 > > > + - melexis,mlx75027 > > > + > > > + reg: > > > + maxItems: 1 > > > + > > > + assigned-clocks: true > > > + assigned-clock-parents: true > > > + assigned-clock-rates: true > > > + > > > + clocks: > > > + description: Clock frequency 8MHz > > > + maxItems: 1 > > > + > > > + vdda-supply: > > > + description: > > > + Definition of the regulator used as analog power supply(2.7V). > > > + > > > + vddif-supply: > > > + description: > > > + Definition of the regulator used as interface power supply(1.8V). > > > + > > > + vddd-supply: > > > + description: > > > + Definition of the regulator used as digital power supply(1.2V). > > > + > > > + vdmix-supply: > > > + description: > > > + Definition of the regulator used as mixed driver power supply(1.2V). > > > + > > > + reset-gpios: > > > + maxItems: 1 > > > + description: Reset Sensor GPIO Control (active low) > > > + > > > + melexis,trig-gpios: > > > + maxItems: 1 > > > + description: > > > + Hardware Trigger GPIO Control (active low) > > > + When the hardware trigger mode is enabled, this GPIO is used to generate > > > + a low level impulse for about 100us. The impulse triggers a new > > > + measurement cycle. > > > + > > > + melexis,leden-gpios: > > > + maxItems: 1 > > > + description: > > > + Led driver enable GPIO Control (active high) > > > + This GPIO notifies the illumination driver when it is safe to use led > > > + signals from the sensor. > > > > As far as I understand this isn't an input of the sensor, but a signal > > that is driven by the driver and controls the separate illuminator. It > > shouldn't be specified in this binding, as it's not a property of the > > sensor. You should instead have a DT node for the illumination driver. > > Yes, you are right. There are illuminators, which are not ready to accept > the signal from the sensor, if pin levels are not right. So I need to > notify illuminator somehow. Can the illumination driver be as a V4L2 > subdevice? Then I can notify the illuminator via s_stream. In another > case it can damage the illuminator. What do you think? It can be a separate subdev, yes. There are flash-related controls in V4L2, maybe some of them would cover your use cases ? We have a few flash drivers in upstream that you can look at for inspiration, for instance drivers/media/i2c/lm3560.c or drivers/media/i2c/adp1653.c. > > > + port: > > > + description: MIPI CSI-2 transmitter port > > > + $ref: /schemas/graph.yaml#/$defs/port-base > > > + > > > + properties: > > > + endpoint: > > > + $ref: /schemas/media/video-interfaces.yaml# > > > + unevaluatedProperties: false > > > + > > > + properties: > > > + data-lanes: > > > + oneOf: > > > + - items: > > > + - const: 1 > > > + - const: 2 > > > + - items: > > > + - const: 1 > > > + - const: 2 > > > + - const: 3 > > > + - const: 4 > > > + > > > + clock-noncontinuous: true > > > + link-frequencies: true > > > + > > > + required: > > > + - data-lanes > > > + - link-frequencies > > > + > > > +required: > > > + - compatible > > > + - reg > > > + - clocks > > > + - port > > > > Let's make the supplies mandatory too, as the chip can't operate without > > them. > > Ok > > > > + > > > +additionalProperties: false > > > + > > > +examples: > > > + - | > > > + #include <dt-bindings/gpio/gpio.h> > > > + > > > + i2c { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + mlx7502x: camera@57 { > > > + compatible = "melexis,mlx7502x"; > > > + reg = <0x57>; > > > + clocks = <&mlx7502x_clk>; > > > + > > > + assigned-clocks = <&mlx7502x_clk>; > > > + assigned-clock-parents = <&mlx7502x_clk_parent>; > > > + assigned-clock-rates = <8000000>; > > > + > > > + vddd-supply = <&mlx_7502x_reg>; > > > + > > > + reset-gpios = <&gpio_exp 6 GPIO_ACTIVE_HIGH>; > > > + melexis,trig-gpios = <&gpio_exp 2 GPIO_ACTIVE_HIGH>; > > > + melexis,leden-gpios = <&gpio_exp 7 GPIO_ACTIVE_HIGH>; > > > + > > > + port { > > > + mlx7502x_out_mipi_csi2: endpoint { > > > + remote-endpoint = <&mipi_csi2_from_mlx7502x>; > > > + data-lanes = <1 2 3 4>; > > > + link-frequencies = /bits/ 64 < 960000000 > > > + 904000000 > > > + 800000000 > > > + 704000000 > > > + 600000000 > > > + 300000000 >; > > > + }; > > > + }; > > > + }; > > > + }; > > > + > > > +... > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index 1a68d888ee14..b00a726bb3db 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -12678,6 +12678,7 @@ M: Volodymyr Kharuk <vkh@melexis.com> > > > L: linux-media@vger.kernel.org > > > S: Supported > > > W: http://www.melexis.com > > > +F: Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml > > > F: include/uapi/linux/mlx7502x.h > > > > > > MELFAS MIP4 TOUCHSCREEN DRIVER
Hi Krzysztof, Very late reply, this had fallen through the cracks. On Thu, Jul 14, 2022 at 01:56:13PM +0200, Krzysztof Kozlowski wrote: > On 14/07/2022 13:29, Laurent Pinchart wrote: > > On Thu, Jul 14, 2022 at 01:23:41PM +0200, Krzysztof Kozlowski wrote: > >> On 14/07/2022 13:12, Laurent Pinchart wrote: > >>>>>>> One option would be to support the following three compatible values: > >>>>>>> > >>>>>>> compatible = "melexis,mlx75026", "melexis,mlx7502x"; > >>>>>>> compatible = "melexis,mlx75027", "melexis,mlx7502x"; > >>>>>>> compatible = "melexis,mlx7502x"; > >>>>>>> > >>>>>>> The last one only would trigger autodetection. I'm still not sure how to > >>>>>>> document that properly in bindings though. > >>>>>> > >>>>>> I missed that part of binding. > >>>>>> > >>>>>> Wildcards are not allowed in compatible, so mlx7502x has to go. > >>>>> > >>>>> Really ? We've had fallback generic compatible strings since the > >>>>> beginning. > >>>> > >>>> Fallback generic compatibles are allowed. Wildcards not. Wildcards were > >>>> actually never explicitly allowed, they just slipped in to many > >>>> bindings... We have several discussions on this on mailing list, so no > >>>> real point to repeat the arguments. > >>>> > >>>> There is a difference between generic fallback. If the device follows > >>>> clear specification and version, e.g. "foo-bar-v4", you can use it for > >>>> generic compatible. This is more common in SoC components. Requirement - > >>>> there is a clear mapping between versions and SoCs. > >>> > >>> I'm not sure to see a clear difference between the two concepts. > >> > >> The clear difference is that you have a versioned and re-usable hardware > >> block plus clear mapping which version goes to which SoC. Version > >> numbers usually start with 1, not with 75025. 75025 is a model name. > > > > How about Documentation/devicetree/bindings/serial/renesas,scif.yaml for > > instance, where the version number isn't known and the SoC name is used > > instead ? Is that acceptable ? > > This is the second case I mentioned - family of devices where the family > fallback is not allowed to be alone. You cannot use just "renesas,scif" > in DTS. OK. Does this mean you are fine with compatible = "melexis,mlx75026", "melexis,mlx7502x"; compatible = "melexis,mlx75027", "melexis,mlx7502x"; where "melexis,mlx7502x" is considered to be the family fallback, but not compatible = "melexis,mlx7502x"; alone ? > > How should we deal with devices that have different models, where the > > model is irrelevant to the kernel driver, but relevant to userspace ? > > Imagine, for instance, a light sensor with 10 models than only differ by > > the filter they use to tune the sensitivity to different light spectrums > > ? They are all "compatible" from a software point of view, would the > > driver need to list all 10 compatible strings ? > > I don't understand that example, I mean, what's the problem here? If > they are all compatible, you can use only one comaptible, e.g. > melexis,mlx75026. > > If you ever need to differentiate it for user-space, you add specific > compatible for the model and you have: > > melexis,mlx75027, melexis,mlx75026 > > If user-space needs dedicated compatibles - add them, no one here argues > to not to use specific compatibles. OK. > >>> For cameras, we often deal with complex pipelines with multiple external > >>> devices and multiple IP cores, with drivers that need to communicate > >>> with each other to initialize the complete camera system. For instance, > >>> each camera-related component in the system registers itself in a media > >>> graph that can be queried from userspace and exposes information about > >>> all devices, including their model. There's no power up of any device > >>> when this query is being performed from userspace. It could possibly be > >>> changed (and maybe it should, for reasons unrelated to this discussion), > >>> but we're looking at pretty much a complete redesign of V4L2 and MC > >>> then. > >> > >> Is then autodetection a real use case since you have to power up the > >> sensor each time system boots and this violates privacy? Several I2C > >> sensors do not care about this and they always do it on power up, so > >> aren't we solving here something unimportant? > > > > In a laptop or tablet with a camera sensor, you likely don't want > > autodetection. In an industrial device, you don't care, and having the > > ability to auto-detect the exact sensor model when booting saves cost in > > the production chain as a single image can work across different models. > > We talk about the case here, not generic. Do you want to have > autodetection possible here or not? I'd like to support auto-detection, but not make it mandatory. Assuming a family of chips supported by one driver with hardware that makes auto-detection possible, I have use cases where I specifically don't want auto-detection as it would have undesirable side effects at probe time, and other use cases where I want auto-detection as it lowers the costs in the production chain. I thus need to be able to specify, in DT, whether to use auto-detection or not, and when not using auto-detection, specify the exact chip model.
On 06/02/2023 11:45, Laurent Pinchart wrote: > Hi Krzysztof, > > Very late reply, this had fallen through the cracks. > > On Thu, Jul 14, 2022 at 01:56:13PM +0200, Krzysztof Kozlowski wrote: >> On 14/07/2022 13:29, Laurent Pinchart wrote: >>> On Thu, Jul 14, 2022 at 01:23:41PM +0200, Krzysztof Kozlowski wrote: >>>> On 14/07/2022 13:12, Laurent Pinchart wrote: >>>>>>>>> One option would be to support the following three compatible values: >>>>>>>>> >>>>>>>>> compatible = "melexis,mlx75026", "melexis,mlx7502x"; >>>>>>>>> compatible = "melexis,mlx75027", "melexis,mlx7502x"; >>>>>>>>> compatible = "melexis,mlx7502x"; >>>>>>>>> >>>>>>>>> The last one only would trigger autodetection. I'm still not sure how to >>>>>>>>> document that properly in bindings though. >>>>>>>> >>>>>>>> I missed that part of binding. >>>>>>>> >>>>>>>> Wildcards are not allowed in compatible, so mlx7502x has to go. >>>>>>> >>>>>>> Really ? We've had fallback generic compatible strings since the >>>>>>> beginning. >>>>>> >>>>>> Fallback generic compatibles are allowed. Wildcards not. Wildcards were >>>>>> actually never explicitly allowed, they just slipped in to many >>>>>> bindings... We have several discussions on this on mailing list, so no >>>>>> real point to repeat the arguments. >>>>>> >>>>>> There is a difference between generic fallback. If the device follows >>>>>> clear specification and version, e.g. "foo-bar-v4", you can use it for >>>>>> generic compatible. This is more common in SoC components. Requirement - >>>>>> there is a clear mapping between versions and SoCs. >>>>> >>>>> I'm not sure to see a clear difference between the two concepts. >>>> >>>> The clear difference is that you have a versioned and re-usable hardware >>>> block plus clear mapping which version goes to which SoC. Version >>>> numbers usually start with 1, not with 75025. 75025 is a model name. >>> >>> How about Documentation/devicetree/bindings/serial/renesas,scif.yaml for >>> instance, where the version number isn't known and the SoC name is used >>> instead ? Is that acceptable ? >> >> This is the second case I mentioned - family of devices where the family >> fallback is not allowed to be alone. You cannot use just "renesas,scif" >> in DTS. > > OK. Does this mean you are fine with > > compatible = "melexis,mlx75026", "melexis,mlx7502x"; > compatible = "melexis,mlx75027", "melexis,mlx7502x"; > > where "melexis,mlx7502x" is considered to be the family fallback, but > not > > compatible = "melexis,mlx7502x"; > > alone ? Correct. (...) >>> >>> In a laptop or tablet with a camera sensor, you likely don't want >>> autodetection. In an industrial device, you don't care, and having the >>> ability to auto-detect the exact sensor model when booting saves cost in >>> the production chain as a single image can work across different models. >> >> We talk about the case here, not generic. Do you want to have >> autodetection possible here or not? > > I'd like to support auto-detection, but not make it mandatory. Assuming > a family of chips supported by one driver with hardware that makes > auto-detection possible, I have use cases where I specifically don't > want auto-detection as it would have undesirable side effects at probe > time, and other use cases where I want auto-detection as it lowers the > costs in the production chain. I thus need to be able to specify, in DT, > whether to use auto-detection or not, and when not using auto-detection, > specify the exact chip model. OK, I understand. This however stretches the Devicetree approach - you are putting OS policy of device probing into the DT binding. What's more, it serves only Linux' purpose. If other OS/software is fine with auto-detection on first use (thus no privacy concerns), then all this discussion is not relevant. The binding is independent of OS, thus we should not align it to our specific OS behavior. Maybe Linux needs some generic runtime knob to turn on/off autodetection for all devices. It does not look like a job for DT. Best regards, Krzysztof
Hi Krzysztof, On Mon, Feb 06, 2023 at 07:20:21PM +0100, Krzysztof Kozlowski wrote: > On 06/02/2023 11:45, Laurent Pinchart wrote: > > On Thu, Jul 14, 2022 at 01:56:13PM +0200, Krzysztof Kozlowski wrote: > >> On 14/07/2022 13:29, Laurent Pinchart wrote: > >>> On Thu, Jul 14, 2022 at 01:23:41PM +0200, Krzysztof Kozlowski wrote: > >>>> On 14/07/2022 13:12, Laurent Pinchart wrote: > >>>>>>>>> One option would be to support the following three compatible values: > >>>>>>>>> > >>>>>>>>> compatible = "melexis,mlx75026", "melexis,mlx7502x"; > >>>>>>>>> compatible = "melexis,mlx75027", "melexis,mlx7502x"; > >>>>>>>>> compatible = "melexis,mlx7502x"; > >>>>>>>>> > >>>>>>>>> The last one only would trigger autodetection. I'm still not sure how to > >>>>>>>>> document that properly in bindings though. > >>>>>>>> > >>>>>>>> I missed that part of binding. > >>>>>>>> > >>>>>>>> Wildcards are not allowed in compatible, so mlx7502x has to go. > >>>>>>> > >>>>>>> Really ? We've had fallback generic compatible strings since the > >>>>>>> beginning. > >>>>>> > >>>>>> Fallback generic compatibles are allowed. Wildcards not. Wildcards were > >>>>>> actually never explicitly allowed, they just slipped in to many > >>>>>> bindings... We have several discussions on this on mailing list, so no > >>>>>> real point to repeat the arguments. > >>>>>> > >>>>>> There is a difference between generic fallback. If the device follows > >>>>>> clear specification and version, e.g. "foo-bar-v4", you can use it for > >>>>>> generic compatible. This is more common in SoC components. Requirement - > >>>>>> there is a clear mapping between versions and SoCs. > >>>>> > >>>>> I'm not sure to see a clear difference between the two concepts. > >>>> > >>>> The clear difference is that you have a versioned and re-usable hardware > >>>> block plus clear mapping which version goes to which SoC. Version > >>>> numbers usually start with 1, not with 75025. 75025 is a model name. > >>> > >>> How about Documentation/devicetree/bindings/serial/renesas,scif.yaml for > >>> instance, where the version number isn't known and the SoC name is used > >>> instead ? Is that acceptable ? > >> > >> This is the second case I mentioned - family of devices where the family > >> fallback is not allowed to be alone. You cannot use just "renesas,scif" > >> in DTS. > > > > OK. Does this mean you are fine with > > > > compatible = "melexis,mlx75026", "melexis,mlx7502x"; > > compatible = "melexis,mlx75027", "melexis,mlx7502x"; > > > > where "melexis,mlx7502x" is considered to be the family fallback, but > > not > > > > compatible = "melexis,mlx7502x"; > > > > alone ? > > Correct. > > (...) > > >>> > >>> In a laptop or tablet with a camera sensor, you likely don't want > >>> autodetection. In an industrial device, you don't care, and having the > >>> ability to auto-detect the exact sensor model when booting saves cost in > >>> the production chain as a single image can work across different models. > >> > >> We talk about the case here, not generic. Do you want to have > >> autodetection possible here or not? > > > > I'd like to support auto-detection, but not make it mandatory. Assuming > > a family of chips supported by one driver with hardware that makes > > auto-detection possible, I have use cases where I specifically don't > > want auto-detection as it would have undesirable side effects at probe > > time, and other use cases where I want auto-detection as it lowers the > > costs in the production chain. I thus need to be able to specify, in DT, > > whether to use auto-detection or not, and when not using auto-detection, > > specify the exact chip model. > > OK, I understand. This however stretches the Devicetree approach - you > are putting OS policy of device probing into the DT binding. What's > more, it serves only Linux' purpose. If other OS/software is fine with > auto-detection on first use (thus no privacy concerns), then all this > discussion is not relevant. The binding is independent of OS, thus we > should not align it to our specific OS behavior. > > Maybe Linux needs some generic runtime knob to turn on/off autodetection > for all devices. It does not look like a job for DT. If we want to be able to run without auto-detection, regardless of the operating system, we need to specify the exact model in DT, otherwise a driver wouldn't be able to identify the device. If we want to use auto-detection, that's for the purpose of simplifying system integration, with a single DT that covers multiple device variants. Those two use cases thus require DT binding that allow for both options, specifying an exact model, or being more generic. I agree that deciding to auto-detect based on what compatible strings are specified may be specific to a particular Linux driver, but the fact that we need the two options to support both use cases isn't OS-specific in my opinion.
diff --git a/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml new file mode 100644 index 000000000000..4ac91f7a26b6 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml @@ -0,0 +1,146 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/media/i2c/melexis,mlx7502x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Melexis ToF 7502x MIPI CSI-2 Sensor + +maintainers: + - Volodymyr Kharuk <vkh@melexis.com> + +description: |- + Melexis ToF 7502x sensors has a CSI-2 output. It supports 2 and 4 lanes, + and mipi speeds are 300, 600, 704, 800, 904, 960Mbs. Supported format is RAW12. + Sensor 75026 is QVGA, while 75027 is VGA sensor. + If you use compatible = "melexis,mlx7502x", then autodetect will be called. + +properties: + compatible: + items: + - enum: + - melexis,mlx7502x + - melexis,mlx75026 + - melexis,mlx75027 + + reg: + maxItems: 1 + + assigned-clocks: true + assigned-clock-parents: true + assigned-clock-rates: true + + clocks: + description: Clock frequency 8MHz + maxItems: 1 + + vdda-supply: + description: + Definition of the regulator used as analog power supply(2.7V). + + vddif-supply: + description: + Definition of the regulator used as interface power supply(1.8V). + + vddd-supply: + description: + Definition of the regulator used as digital power supply(1.2V). + + vdmix-supply: + description: + Definition of the regulator used as mixed driver power supply(1.2V). + + reset-gpios: + maxItems: 1 + description: Reset Sensor GPIO Control (active low) + + melexis,trig-gpios: + maxItems: 1 + description: + Hardware Trigger GPIO Control (active low) + When the hardware trigger mode is enabled, this GPIO is used to generate + a low level impulse for about 100us. The impulse triggers a new + measurement cycle. + + melexis,leden-gpios: + maxItems: 1 + description: + Led driver enable GPIO Control (active high) + This GPIO notifies the illumination driver when it is safe to use led + signals from the sensor. + + port: + description: MIPI CSI-2 transmitter port + $ref: /schemas/graph.yaml#/$defs/port-base + + properties: + endpoint: + $ref: /schemas/media/video-interfaces.yaml# + unevaluatedProperties: false + + properties: + data-lanes: + oneOf: + - items: + - const: 1 + - const: 2 + - items: + - const: 1 + - const: 2 + - const: 3 + - const: 4 + + clock-noncontinuous: true + link-frequencies: true + + required: + - data-lanes + - link-frequencies + +required: + - compatible + - reg + - clocks + - port + +additionalProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + + i2c { + #address-cells = <1>; + #size-cells = <0>; + + mlx7502x: camera@57 { + compatible = "melexis,mlx7502x"; + reg = <0x57>; + clocks = <&mlx7502x_clk>; + + assigned-clocks = <&mlx7502x_clk>; + assigned-clock-parents = <&mlx7502x_clk_parent>; + assigned-clock-rates = <8000000>; + + vddd-supply = <&mlx_7502x_reg>; + + reset-gpios = <&gpio_exp 6 GPIO_ACTIVE_HIGH>; + melexis,trig-gpios = <&gpio_exp 2 GPIO_ACTIVE_HIGH>; + melexis,leden-gpios = <&gpio_exp 7 GPIO_ACTIVE_HIGH>; + + port { + mlx7502x_out_mipi_csi2: endpoint { + remote-endpoint = <&mipi_csi2_from_mlx7502x>; + data-lanes = <1 2 3 4>; + link-frequencies = /bits/ 64 < 960000000 + 904000000 + 800000000 + 704000000 + 600000000 + 300000000 >; + }; + }; + }; + }; + +... diff --git a/MAINTAINERS b/MAINTAINERS index 1a68d888ee14..b00a726bb3db 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -12678,6 +12678,7 @@ M: Volodymyr Kharuk <vkh@melexis.com> L: linux-media@vger.kernel.org S: Supported W: http://www.melexis.com +F: Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml F: include/uapi/linux/mlx7502x.h MELFAS MIP4 TOUCHSCREEN DRIVER
Add device tree binding of the mlx7502x and update MAINTAINERS Signed-off-by: Volodymyr Kharuk <vkh@melexis.com> --- .../bindings/media/i2c/melexis,mlx7502x.yaml | 146 ++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 147 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/melexis,mlx7502x.yaml