[PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[Fabio Estevam]

As explained in the description text:

"This is a list of trivial I2C and SPI devices that have simple device tree
bindings, consisting only of a compatible field, an address and possibly an
interrupt line."
 
A camera device does not fall into this category as it needs other
properties such as regulators, reset and powerdown GPIOs, clocks,
media endpoint.

Remove the OV5642 entry.

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 Documentation/devicetree/bindings/trivial-devices.yaml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
index 40bc475ee7e1..ab1423a4aa7f 100644
--- a/Documentation/devicetree/bindings/trivial-devices.yaml
+++ b/Documentation/devicetree/bindings/trivial-devices.yaml
@@ -313,8 +313,6 @@ properties:
           - nuvoton,w83773g
             # OKI ML86V7667 video decoder
           - oki,ml86v7667
-            # OV5642: Color CMOS QSXGA (5-megapixel) Image Sensor with OmniBSI and Embedded TrueFocus
-          - ovti,ov5642
             # 48-Lane, 12-Port PCI Express Gen 2 (5.0 GT/s) Switch
           - plx,pex8648
             # Pulsedlight LIDAR range-finding sensor
-- 
2.34.1

Re: [PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1541 bytes --]

On Tue, Aug 01, 2023 at 02:00:15PM -0300, Fabio Estevam wrote:
> As explained in the description text:
> 
> "This is a list of trivial I2C and SPI devices that have simple device tree
> bindings, consisting only of a compatible field, an address and possibly an
> interrupt line."
>  
> A camera device does not fall into this category as it needs other
> properties such as regulators, reset and powerdown GPIOs, clocks,
> media endpoint.
> 
> Remove the OV5642 entry.
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>

Removing it without re-adding it elsewhere does not seem right, since
there'll now be some undocumented compatibles in the tree, no?

> ---
>  Documentation/devicetree/bindings/trivial-devices.yaml | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/trivial-devices.yaml b/Documentation/devicetree/bindings/trivial-devices.yaml
> index 40bc475ee7e1..ab1423a4aa7f 100644
> --- a/Documentation/devicetree/bindings/trivial-devices.yaml
> +++ b/Documentation/devicetree/bindings/trivial-devices.yaml
> @@ -313,8 +313,6 @@ properties:
>            - nuvoton,w83773g
>              # OKI ML86V7667 video decoder
>            - oki,ml86v7667
> -            # OV5642: Color CMOS QSXGA (5-megapixel) Image Sensor with OmniBSI and Embedded TrueFocus
> -          - ovti,ov5642
>              # 48-Lane, 12-Port PCI Express Gen 2 (5.0 GT/s) Switch
>            - plx,pex8648
>              # Pulsedlight LIDAR range-finding sensor
> -- 
> 2.34.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[Fabio Estevam]

On 01/08/2023 17:47, Conor Dooley wrote:

> Removing it without re-adding it elsewhere does not seem right, since
> there'll now be some undocumented compatibles in the tree, no?

Currently, there is no ov5642 support in the kernel.

If someone adds the support for the ov5642 camera, then a specific 
binding
will have to be created.

I prefer to remove it from trivial-devices to avoid confusion.

As is, it gives a false impression that ov5642 is supported and that it
is a trivial device.

Re: [PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

On Tue, Aug 01, 2023 at 06:10:52PM -0300, Fabio Estevam wrote:
> On 01/08/2023 17:47, Conor Dooley wrote:
> 
> > Removing it without re-adding it elsewhere does not seem right, since
> > there'll now be some undocumented compatibles in the tree, no?
> 
> Currently, there is no ov5642 support in the kernel.

It is present in devicetrees.

> If someone adds the support for the ov5642 camera, then a specific binding
> will have to be created.
> 
> I prefer to remove it from trivial-devices to avoid confusion.
> 
> As is, it gives a false impression that ov5642 is supported and that it
> is a trivial device.

The latter only do I agree with.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[Fabio Estevam]

On 01/08/2023 18:13, Conor Dooley wrote:
> On Tue, Aug 01, 2023 at 06:10:52PM -0300, Fabio Estevam wrote:
>> On 01/08/2023 17:47, Conor Dooley wrote:
>> 
>> > Removing it without re-adding it elsewhere does not seem right, since
>> > there'll now be some undocumented compatibles in the tree, no?
>> 
>> Currently, there is no ov5642 support in the kernel.
> 
> It is present in devicetrees.

Yes, and none of them have the ov5642 camera functional:

arch/arm/boot/dts/nxp/imx/imx53-smd.dts
arch/arm/boot/dts/nxp/imx/imx6qdl-sabrelite.dtsi
arch/arm/boot/dts/nxp/imx/imx6qdl-sabresd.dtsi

>> If someone adds the support for the ov5642 camera, then a specific 
>> binding
>> will have to be created.
>> 
>> I prefer to remove it from trivial-devices to avoid confusion.
>> 
>> As is, it gives a false impression that ov5642 is supported and that 
>> it
>> is a trivial device.
> 
> The latter only do I agree with.

Care to explain how ov5642 is supported by the current mainline kernel?

Re: [PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1215 bytes --]

On Tue, Aug 01, 2023 at 06:18:29PM -0300, Fabio Estevam wrote:
> On 01/08/2023 18:13, Conor Dooley wrote:
> > On Tue, Aug 01, 2023 at 06:10:52PM -0300, Fabio Estevam wrote:
> > > On 01/08/2023 17:47, Conor Dooley wrote:
> > > 
> > > > Removing it without re-adding it elsewhere does not seem right, since
> > > > there'll now be some undocumented compatibles in the tree, no?
> > > 
> > > Currently, there is no ov5642 support in the kernel.
> > 
> > It is present in devicetrees.
> 
> Yes, and none of them have the ov5642 camera functional:
> 
> arch/arm/boot/dts/nxp/imx/imx53-smd.dts
> arch/arm/boot/dts/nxp/imx/imx6qdl-sabrelite.dtsi
> arch/arm/boot/dts/nxp/imx/imx6qdl-sabresd.dtsi
> 
> > > If someone adds the support for the ov5642 camera, then a specific
> > > binding
> > > will have to be created.
> > > 
> > > I prefer to remove it from trivial-devices to avoid confusion.
> > > 
> > > As is, it gives a false impression that ov5642 is supported and that
> > > it
> > > is a trivial device.
> > 
> > The latter only do I agree with.
> 
> Care to explain how ov5642 is supported by the current mainline kernel?

I never said it was chief. Please re-read the quoted text.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[Fabio Estevam]

Hi Conor,

On 01/08/2023 18:28, Conor Dooley wrote:

> I never said it was chief. Please re-read the quoted text.

trivial-devices.yaml throws the following warning:

imx6q-sabrelite.dtb: camera@42: 'clock-names', 'clocks', 'gp-gpios', 
'port', 'powerdown-gpios', 'reset-gpios' do not match any of the 
regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/trivial-devices.yaml#

Would it make sense to remove ovti,ov5642 from the trivial-devices 
bindings as well as from the
following devicetrees?

arch/arm/boot/dts/nxp/imx/imx53-smd.dts
arch/arm/boot/dts/nxp/imx/imx6qdl-sabrelite.dtsi
arch/arm/boot/dts/nxp/imx/imx6qdl-sabresd.dtsi

Please advise.

Re: [PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

On Tue, Aug 01, 2023 at 06:43:58PM -0300, Fabio Estevam wrote:
> Hi Conor,
> 
> On 01/08/2023 18:28, Conor Dooley wrote:
> 
> > I never said it was chief. Please re-read the quoted text.
> 
> trivial-devices.yaml throws the following warning:
> 
> imx6q-sabrelite.dtb: camera@42: 'clock-names', 'clocks', 'gp-gpios', 'port',
> 'powerdown-gpios', 'reset-gpios' do not match any of the regexes:
> 'pinctrl-[0-9]+'
> 	from schema $id: http://devicetree.org/schemas/trivial-devices.yaml#
> 
> Would it make sense to remove ovti,ov5642 from the trivial-devices bindings
> as well as from the
> following devicetrees?

I would rather that you documented it, rather than removed it, please.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[Krzysztof Kozlowski]

On 01/08/2023 23:57, Conor Dooley wrote:
> On Tue, Aug 01, 2023 at 06:43:58PM -0300, Fabio Estevam wrote:
>> Hi Conor,
>>
>> On 01/08/2023 18:28, Conor Dooley wrote:
>>
>>> I never said it was chief. Please re-read the quoted text.
>>
>> trivial-devices.yaml throws the following warning:
>>
>> imx6q-sabrelite.dtb: camera@42: 'clock-names', 'clocks', 'gp-gpios', 'port',
>> 'powerdown-gpios', 'reset-gpios' do not match any of the regexes:
>> 'pinctrl-[0-9]+'
>> 	from schema $id: http://devicetree.org/schemas/trivial-devices.yaml#
>>
>> Would it make sense to remove ovti,ov5642 from the trivial-devices bindings
>> as well as from the
>> following devicetrees?
> 
> I would rather that you documented it, rather than removed it, please.

Removal from DTS is rather discouraged, because DTS and these nodes
could be used in other systems. The best is to fully document the
device, regardless whether Linux supports it or not.

Dropping from trivial-devices could work, because it does not change
much for our schema - one warning goes to other warning...

Best regards,
Krzysztof

Re: [PATCH] dt-bindings: trivial-devices: Remove the OV5642 entry

[Fabio Estevam]

On 20/08/2023 05:14, Krzysztof Kozlowski wrote:

> Removal from DTS is rather discouraged, because DTS and these nodes
> could be used in other systems. The best is to fully document the
> device, regardless whether Linux supports it or not.

Right, this was the same recommendation I got from Conor.

Here is v3 that addresses it:

https://lore.kernel.org/linux-media/20230802160326.293420-1-festevam@denx.de/