| Message ID | 20230801170015.40965-1-festevam@denx.de |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | dt-bindings: trivial-devices: Remove the OV5642 entry | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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 >
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.
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.
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?
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.
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.
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.
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
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/
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
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(-)