* [PATCH 0/2] media: i2c: Cleanup assigned-clocks and endpoint: properties: unevaluatedProperties: false
@ 2024-10-12 15:02 Bryan O'Donoghue
2024-10-12 15:02 ` [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema Bryan O'Donoghue
2024-10-12 15:02 ` [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: Bryan O'Donoghue
0 siblings, 2 replies; 31+ messages in thread
From: Bryan O'Donoghue @ 2024-10-12 15:02 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy,
Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao,
Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham,
Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy,
Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart
Cc: Krzysztof Kozlowski, linux-media, devicetree, linux-kernel, imx,
linux-arm-kernel, Bryan O'Donoghue
On a recent schema submission I did what most well adjusted schema writers
do and tried to find a base file to work from to copy/paste and forget.
Confusingly/predictably I received feedback to remove or alter several of
the properties included in my devious copy/paste plan.
The first bit of feedback was that assigned-clock-* was to be dropped.
Removing assigned-clock-* as assigned-clock-* is a core property which
doesn't need to be listed in a schema.
The second bit of feedback landed on use of additionalProperties:false
along with enumeration of all required endpoint properties instead of an
implied list of valid properties from unevaluatedProperties:false.
Link: https://lore.kernel.org/linux-media/20241010-b4-master-24-11-25-ov08x40-v6-0-cf966e34e685@linaro.org
This series removes the assigned-clock-* from upstream sensor property
schemas and applies additionalProperities:false to properties: endpoint:.
A few missing properties: or required: are added to the schemas based on
output of DT checkers.
The one new DT complaint I didn't fix with the move to
additionalProperties: false is:
/home/deckard/Development/qualcomm/qlt-kernel-tools/qlt-kernel/build/x1e80100-crd_qlt_integration/arch/arm64/boot/dts/renesas/r8a774c0-ek874-mipi-2.1.dtb: imx219@10: port:endpoint: 'remote-endpoint' is a required property
Since this appears to be some sort of temporary/commented thing upstream
which I don't know the provenance of.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
Bryan O'Donoghue (2):
media: dt-bindings: Remove assigned-clock-* from various schema
media: dt-bindings: Use additionalProperties: false for endpoint: properties:
.../bindings/media/i2c/alliedvision,alvium-csi2.yaml | 5 ++++-
.../devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml | 4 +++-
.../devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml | 4 +++-
.../devicetree/bindings/media/i2c/galaxycore,gc2145.yaml | 6 +++++-
.../devicetree/bindings/media/i2c/hynix,hi846.yaml | 12 +++---------
Documentation/devicetree/bindings/media/i2c/imx219.yaml | 6 +++++-
Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml | 4 +++-
.../devicetree/bindings/media/i2c/ovti,og01a1b.yaml | 4 +++-
.../devicetree/bindings/media/i2c/ovti,ov02a10.yaml | 4 +++-
.../devicetree/bindings/media/i2c/ovti,ov4689.yaml | 4 +++-
.../devicetree/bindings/media/i2c/ovti,ov5648.yaml | 13 ++++---------
.../devicetree/bindings/media/i2c/ovti,ov5675.yaml | 3 ++-
.../devicetree/bindings/media/i2c/ovti,ov7251.yaml | 4 +++-
.../devicetree/bindings/media/i2c/ovti,ov8865.yaml | 13 ++++---------
.../devicetree/bindings/media/i2c/ovti,ov9282.yaml | 8 +++-----
.../devicetree/bindings/media/i2c/sony,imx214.yaml | 4 +++-
.../devicetree/bindings/media/i2c/sony,imx258.yaml | 8 +++-----
.../devicetree/bindings/media/i2c/sony,imx283.yaml | 4 +++-
.../devicetree/bindings/media/i2c/sony,imx290.yaml | 4 +++-
.../devicetree/bindings/media/i2c/sony,imx334.yaml | 8 +++-----
.../devicetree/bindings/media/i2c/sony,imx335.yaml | 8 +++-----
.../devicetree/bindings/media/i2c/sony,imx412.yaml | 8 +++-----
.../devicetree/bindings/media/i2c/toshiba,tc358746.yaml | 4 +++-
23 files changed, 75 insertions(+), 67 deletions(-)
---
base-commit: 58ca61c1a866bfdaa5e19fb19a2416764f847d75
change-id: 20241005-b4-linux-next-202041004-i2c-media-yaml-fixes-fcf5c0c1e08d
Best regards,
--
Bryan O'Donoghue <bryan.odonoghue@linaro.org>
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-12 15:02 [PATCH 0/2] media: i2c: Cleanup assigned-clocks and endpoint: properties: unevaluatedProperties: false Bryan O'Donoghue @ 2024-10-12 15:02 ` Bryan O'Donoghue 2024-10-12 17:24 ` Laurent Pinchart 2024-10-14 7:43 ` Krzysztof Kozlowski 2024-10-12 15:02 ` [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: Bryan O'Donoghue 1 sibling, 2 replies; 31+ messages in thread From: Bryan O'Donoghue @ 2024-10-12 15:02 UTC (permalink / raw) To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart Cc: Krzysztof Kozlowski, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel, Bryan O'Donoghue Remove extraneous assigned-clock* from media/i2c/* schemas, retain in the relevant examples. Link: https://lore.kernel.org/linux-media/j7kgz2lyxnler5qwd7yiazdq6fmsv77kyozdrxf33h54ydakjz@uqjhwhoyv6re Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 8 -------- Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml | 8 -------- Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml | 8 -------- Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 4 ---- Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ---- Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml | 4 ---- Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 ---- Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml | 4 ---- 8 files changed, 44 deletions(-) diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml @@ -28,12 +28,6 @@ properties: items: - description: Reference to the mclk clock. - assigned-clocks: - maxItems: 1 - - assigned-clock-rates: - maxItems: 1 - reset-gpios: description: Reference to the GPIO connected to the RESETB pin. Active low. maxItems: 1 @@ -82,8 +76,6 @@ required: - compatible - reg - clocks - - assigned-clocks - - assigned-clock-rates - vddio-supply - vdda-supply - vddd-supply diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml index 1f497679168c8395a94b3867beb49b251ef621fc..622243cae03caa5d14aa312df40ef5907e190d2c 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml @@ -20,12 +20,6 @@ properties: items: - description: XVCLK Clock - assigned-clocks: - maxItems: 1 - - assigned-clock-rates: - maxItems: 1 - dvdd-supply: description: Digital Domain Power Supply @@ -68,8 +62,6 @@ required: - compatible - reg - clocks - - assigned-clocks - - assigned-clock-rates - dvdd-supply - dovdd-supply - port diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml index 8a70e23ba6abed67d8b61c33bd7a261089bddda2..382d7de7a89bcea11be384a2a3800512994f9346 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml @@ -20,12 +20,6 @@ properties: items: - description: EXTCLK Clock - assigned-clocks: - maxItems: 1 - - assigned-clock-rates: - maxItems: 1 - dvdd-supply: description: Digital Domain Power Supply @@ -68,8 +62,6 @@ required: - compatible - reg - clocks - - assigned-clocks - - assigned-clock-rates - dvdd-supply - avdd-supply - dovdd-supply diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml index 79a7658f6d0547e4d6fb2267e5757eedf49fd416..38325cf318f7bd2cd60a4c7bbb6a65b54b855e26 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml @@ -27,10 +27,6 @@ properties: description: I2C address maxItems: 1 - assigned-clocks: true - assigned-clock-parents: true - assigned-clock-rates: true - clocks: description: Clock frequency from 6 to 27MHz maxItems: 1 diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml index c978abc0cdb35cfe2b85069946cf1be435a58cb8..f0f9726a2add89492b8c56e17ed607841baa3a0d 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml @@ -24,10 +24,6 @@ properties: - sony,imx258 - sony,imx258-pdaf - assigned-clocks: true - assigned-clock-parents: true - assigned-clock-rates: true - clocks: description: Clock frequency from 6 to 27 MHz. diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml index bce57b22f7b63bd73f08d8831d9bb04858ef03e0..872b8288948b2bba743f2365a55165181df156ae 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml @@ -24,10 +24,6 @@ properties: description: I2C address maxItems: 1 - assigned-clocks: true - assigned-clock-parents: true - assigned-clock-rates: true - clocks: description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz maxItems: 1 diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml index 77bf3a4ee89db3b5d16149470c0380ef8f1aeac1..38bd1c7304a59bb5fea90954c1e4e626a7c86f2f 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml @@ -24,10 +24,6 @@ properties: description: I2C address maxItems: 1 - assigned-clocks: true - assigned-clock-parents: true - assigned-clock-rates: true - clocks: description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz maxItems: 1 diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml index d9b7815650fdb890418fc96c828acc9147dfb6e8..ece1e17fe34553671e61c965eb1833c5eb08131b 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml @@ -26,10 +26,6 @@ properties: description: I2C address maxItems: 1 - assigned-clocks: true - assigned-clock-parents: true - assigned-clock-rates: true - clocks: description: Clock frequency 6MHz, 12MHz, 18MHz, 24MHz or 27MHz maxItems: 1 -- 2.46.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-12 15:02 ` [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema Bryan O'Donoghue @ 2024-10-12 17:24 ` Laurent Pinchart 2024-10-14 7:43 ` Krzysztof Kozlowski 1 sibling, 0 replies; 31+ messages in thread From: Laurent Pinchart @ 2024-10-12 17:24 UTC (permalink / raw) To: Bryan O'Donoghue Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Krzysztof Kozlowski, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel Hi Bryan, Thank you for the patch. On Sat, Oct 12, 2024 at 04:02:50PM +0100, Bryan O'Donoghue wrote: > Remove extraneous assigned-clock* from media/i2c/* schemas, retain in the > relevant examples. You may want to explain in the commit message *why* you're doing this. The reason may not be straightforward for everybody. With that, Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Link: https://lore.kernel.org/linux-media/j7kgz2lyxnler5qwd7yiazdq6fmsv77kyozdrxf33h54ydakjz@uqjhwhoyv6re > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 8 -------- > Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml | 8 -------- > Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml | 8 -------- > Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 4 ---- > Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ---- > Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml | 4 ---- > Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 ---- > Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml | 4 ---- > 8 files changed, 44 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 > --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > @@ -28,12 +28,6 @@ properties: > items: > - description: Reference to the mclk clock. > > - assigned-clocks: > - maxItems: 1 > - > - assigned-clock-rates: > - maxItems: 1 > - > reset-gpios: > description: Reference to the GPIO connected to the RESETB pin. Active low. > maxItems: 1 > @@ -82,8 +76,6 @@ required: > - compatible > - reg > - clocks > - - assigned-clocks > - - assigned-clock-rates > - vddio-supply > - vdda-supply > - vddd-supply > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml > index 1f497679168c8395a94b3867beb49b251ef621fc..622243cae03caa5d14aa312df40ef5907e190d2c 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml > @@ -20,12 +20,6 @@ properties: > items: > - description: XVCLK Clock > > - assigned-clocks: > - maxItems: 1 > - > - assigned-clock-rates: > - maxItems: 1 > - > dvdd-supply: > description: Digital Domain Power Supply > > @@ -68,8 +62,6 @@ required: > - compatible > - reg > - clocks > - - assigned-clocks > - - assigned-clock-rates > - dvdd-supply > - dovdd-supply > - port > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > index 8a70e23ba6abed67d8b61c33bd7a261089bddda2..382d7de7a89bcea11be384a2a3800512994f9346 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > @@ -20,12 +20,6 @@ properties: > items: > - description: EXTCLK Clock > > - assigned-clocks: > - maxItems: 1 > - > - assigned-clock-rates: > - maxItems: 1 > - > dvdd-supply: > description: Digital Domain Power Supply > > @@ -68,8 +62,6 @@ required: > - compatible > - reg > - clocks > - - assigned-clocks > - - assigned-clock-rates > - dvdd-supply > - avdd-supply > - dovdd-supply > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml > index 79a7658f6d0547e4d6fb2267e5757eedf49fd416..38325cf318f7bd2cd60a4c7bbb6a65b54b855e26 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml > @@ -27,10 +27,6 @@ properties: > description: I2C address > maxItems: 1 > > - assigned-clocks: true > - assigned-clock-parents: true > - assigned-clock-rates: true > - > clocks: > description: Clock frequency from 6 to 27MHz > maxItems: 1 > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > index c978abc0cdb35cfe2b85069946cf1be435a58cb8..f0f9726a2add89492b8c56e17ed607841baa3a0d 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > @@ -24,10 +24,6 @@ properties: > - sony,imx258 > - sony,imx258-pdaf > > - assigned-clocks: true > - assigned-clock-parents: true > - assigned-clock-rates: true > - > clocks: > description: > Clock frequency from 6 to 27 MHz. > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml > index bce57b22f7b63bd73f08d8831d9bb04858ef03e0..872b8288948b2bba743f2365a55165181df156ae 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml > @@ -24,10 +24,6 @@ properties: > description: I2C address > maxItems: 1 > > - assigned-clocks: true > - assigned-clock-parents: true > - assigned-clock-rates: true > - > clocks: > description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz > maxItems: 1 > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml > index 77bf3a4ee89db3b5d16149470c0380ef8f1aeac1..38bd1c7304a59bb5fea90954c1e4e626a7c86f2f 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml > @@ -24,10 +24,6 @@ properties: > description: I2C address > maxItems: 1 > > - assigned-clocks: true > - assigned-clock-parents: true > - assigned-clock-rates: true > - > clocks: > description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz > maxItems: 1 > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml > index d9b7815650fdb890418fc96c828acc9147dfb6e8..ece1e17fe34553671e61c965eb1833c5eb08131b 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml > @@ -26,10 +26,6 @@ properties: > description: I2C address > maxItems: 1 > > - assigned-clocks: true > - assigned-clock-parents: true > - assigned-clock-rates: true > - > clocks: > description: Clock frequency 6MHz, 12MHz, 18MHz, 24MHz or 27MHz > maxItems: 1 -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-12 15:02 ` [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema Bryan O'Donoghue 2024-10-12 17:24 ` Laurent Pinchart @ 2024-10-14 7:43 ` Krzysztof Kozlowski 2024-10-14 8:29 ` Bryan O'Donoghue ` (2 more replies) 1 sibling, 3 replies; 31+ messages in thread From: Krzysztof Kozlowski @ 2024-10-14 7:43 UTC (permalink / raw) To: Bryan O'Donoghue Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On Sat, Oct 12, 2024 at 04:02:50PM +0100, Bryan O'Donoghue wrote: > Remove extraneous assigned-clock* from media/i2c/* schemas, retain in the > relevant examples. > > Link: https://lore.kernel.org/linux-media/j7kgz2lyxnler5qwd7yiazdq6fmsv77kyozdrxf33h54ydakjz@uqjhwhoyv6re > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 8 -------- > Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml | 8 -------- > Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml | 8 -------- > Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 4 ---- > Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ---- > Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml | 4 ---- > Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 ---- > Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml | 4 ---- > 8 files changed, 44 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 > --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > @@ -28,12 +28,6 @@ properties: > items: > - description: Reference to the mclk clock. > > - assigned-clocks: > - maxItems: 1 > - > - assigned-clock-rates: > - maxItems: 1 > - > reset-gpios: > description: Reference to the GPIO connected to the RESETB pin. Active low. > maxItems: 1 > @@ -82,8 +76,6 @@ required: > - compatible > - reg > - clocks > - - assigned-clocks > - - assigned-clock-rates That's not extraneous, but has a meaning that without assigned-clocks this device or driver will not operate. File should rather stay as is. > - vddio-supply > - vdda-supply > - vddd-supply > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml > index 1f497679168c8395a94b3867beb49b251ef621fc..622243cae03caa5d14aa312df40ef5907e190d2c 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml > @@ -20,12 +20,6 @@ properties: > items: > - description: XVCLK Clock > > - assigned-clocks: > - maxItems: 1 > - > - assigned-clock-rates: > - maxItems: 1 > - > dvdd-supply: > description: Digital Domain Power Supply > > @@ -68,8 +62,6 @@ required: > - compatible > - reg > - clocks > - - assigned-clocks > - - assigned-clock-rates Same > - dvdd-supply > - dovdd-supply > - port > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > index 8a70e23ba6abed67d8b61c33bd7a261089bddda2..382d7de7a89bcea11be384a2a3800512994f9346 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > @@ -20,12 +20,6 @@ properties: > items: > - description: EXTCLK Clock > > - assigned-clocks: > - maxItems: 1 > - > - assigned-clock-rates: > - maxItems: 1 > - > dvdd-supply: > description: Digital Domain Power Supply > > @@ -68,8 +62,6 @@ required: > - compatible > - reg > - clocks > - - assigned-clocks > - - assigned-clock-rates Same > - dvdd-supply > - avdd-supply > - dovdd-supply > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml > index 79a7658f6d0547e4d6fb2267e5757eedf49fd416..38325cf318f7bd2cd60a4c7bbb6a65b54b855e26 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml > @@ -27,10 +27,6 @@ properties: > description: I2C address > maxItems: 1 > > - assigned-clocks: true > - assigned-clock-parents: true > - assigned-clock-rates: true > - This is ok. > clocks: > description: Clock frequency from 6 to 27MHz > maxItems: 1 > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > index c978abc0cdb35cfe2b85069946cf1be435a58cb8..f0f9726a2add89492b8c56e17ed607841baa3a0d 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > @@ -24,10 +24,6 @@ properties: > - sony,imx258 > - sony,imx258-pdaf > > - assigned-clocks: true > - assigned-clock-parents: true > - assigned-clock-rates: true > - This is ok. > clocks: > description: > Clock frequency from 6 to 27 MHz. > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml > index bce57b22f7b63bd73f08d8831d9bb04858ef03e0..872b8288948b2bba743f2365a55165181df156ae 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml > @@ -24,10 +24,6 @@ properties: > description: I2C address > maxItems: 1 > > - assigned-clocks: true > - assigned-clock-parents: true > - assigned-clock-rates: true > - This is ok. > clocks: > description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz > maxItems: 1 > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml > index 77bf3a4ee89db3b5d16149470c0380ef8f1aeac1..38bd1c7304a59bb5fea90954c1e4e626a7c86f2f 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml > @@ -24,10 +24,6 @@ properties: > description: I2C address > maxItems: 1 > > - assigned-clocks: true > - assigned-clock-parents: true > - assigned-clock-rates: true > - This is ok. > clocks: > description: Clock frequency from 6 to 27 MHz, 37.125MHz, 74.25MHz > maxItems: 1 > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml > index d9b7815650fdb890418fc96c828acc9147dfb6e8..ece1e17fe34553671e61c965eb1833c5eb08131b 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml > @@ -26,10 +26,6 @@ properties: > description: I2C address > maxItems: 1 > > - assigned-clocks: true > - assigned-clock-parents: true > - assigned-clock-rates: true > - This is ok. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-14 7:43 ` Krzysztof Kozlowski @ 2024-10-14 8:29 ` Bryan O'Donoghue 2024-10-14 8:44 ` Krzysztof Kozlowski 2024-10-14 10:20 ` Sakari Ailus 2024-10-14 20:34 ` Laurent Pinchart 2 siblings, 1 reply; 31+ messages in thread From: Bryan O'Donoghue @ 2024-10-14 8:29 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 14/10/2024 08:43, Krzysztof Kozlowski wrote: >> - - assigned-clocks >> - - assigned-clock-rates > That's not extraneous, but has a meaning that without assigned-clocks > this device or driver will not operate. > > File should rather stay as is. Hmm, I've obviously missed a trick here. I'll check it out. --- bod ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-14 8:29 ` Bryan O'Donoghue @ 2024-10-14 8:44 ` Krzysztof Kozlowski 2024-10-14 10:08 ` Bryan O'Donoghue 0 siblings, 1 reply; 31+ messages in thread From: Krzysztof Kozlowski @ 2024-10-14 8:44 UTC (permalink / raw) To: Bryan O'Donoghue Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 14/10/2024 10:29, Bryan O'Donoghue wrote: > On 14/10/2024 08:43, Krzysztof Kozlowski wrote: >>> - - assigned-clocks >>> - - assigned-clock-rates >> That's not extraneous, but has a meaning that without assigned-clocks >> this device or driver will not operate. >> >> File should rather stay as is. > > Hmm, I've obviously missed a trick here. > > I'll check it out. My response was probably not complete: this still might be extraneous, because maybe the driver/device do not care. But in general requiring assigned-clocks could have a meaning. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-14 8:44 ` Krzysztof Kozlowski @ 2024-10-14 10:08 ` Bryan O'Donoghue 0 siblings, 0 replies; 31+ messages in thread From: Bryan O'Donoghue @ 2024-10-14 10:08 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 14/10/2024 09:44, Krzysztof Kozlowski wrote: > On 14/10/2024 10:29, Bryan O'Donoghue wrote: >> On 14/10/2024 08:43, Krzysztof Kozlowski wrote: >>>> - - assigned-clocks >>>> - - assigned-clock-rates >>> That's not extraneous, but has a meaning that without assigned-clocks >>> this device or driver will not operate. >>> >>> File should rather stay as is. >> >> Hmm, I've obviously missed a trick here. >> >> I'll check it out. > > My response was probably not complete: this still might be extraneous, > because maybe the driver/device do not care. But in general requiring > assigned-clocks could have a meaning. No I see what you mean Even though assigned-clock* is a property of the SoC this driver.. drivers/media/i2c/hi846.c mclk_freq = clk_get_rate(hi846->clock); if (mclk_freq != 25000000) dev_warn(&client->dev, "External clock freq should be 25000000, not %u.\n", mclk_freq); doesn't support setting the clock. So it actually is a requirement, yes. --- bod ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-14 7:43 ` Krzysztof Kozlowski 2024-10-14 8:29 ` Bryan O'Donoghue @ 2024-10-14 10:20 ` Sakari Ailus 2024-10-14 10:34 ` Krzysztof Kozlowski 2024-10-14 20:34 ` Laurent Pinchart 2 siblings, 1 reply; 31+ messages in thread From: Sakari Ailus @ 2024-10-14 10:20 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel Hi Krzysztof, On Mon, Oct 14, 2024 at 09:43:07AM +0200, Krzysztof Kozlowski wrote: > On Sat, Oct 12, 2024 at 04:02:50PM +0100, Bryan O'Donoghue wrote: > > diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > @@ -28,12 +28,6 @@ properties: > > items: > > - description: Reference to the mclk clock. > > > > - assigned-clocks: > > - maxItems: 1 > > - > > - assigned-clock-rates: > > - maxItems: 1 > > - > > reset-gpios: > > description: Reference to the GPIO connected to the RESETB pin. Active low. > > maxItems: 1 > > @@ -82,8 +76,6 @@ required: > > - compatible > > - reg > > - clocks > > - - assigned-clocks > > - - assigned-clock-rates > > That's not extraneous, but has a meaning that without assigned-clocks > this device or driver will not operate. > > File should rather stay as is. ... > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > > index c978abc0cdb35cfe2b85069946cf1be435a58cb8..f0f9726a2add89492b8c56e17ed607841baa3a0d 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > > @@ -24,10 +24,6 @@ properties: > > - sony,imx258 > > - sony,imx258-pdaf > > > > - assigned-clocks: true > > - assigned-clock-parents: true > > - assigned-clock-rates: true > > - > > This is ok. Basically the clock related requirements for these devices are the same: they all need a clock configured at a board specific frequency. Shouldn't we treat them the same way? -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-14 10:20 ` Sakari Ailus @ 2024-10-14 10:34 ` Krzysztof Kozlowski 2024-10-14 10:46 ` Sakari Ailus 0 siblings, 1 reply; 31+ messages in thread From: Krzysztof Kozlowski @ 2024-10-14 10:34 UTC (permalink / raw) To: Sakari Ailus Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 14/10/2024 12:20, Sakari Ailus wrote: > Hi Krzysztof, > > On Mon, Oct 14, 2024 at 09:43:07AM +0200, Krzysztof Kozlowski wrote: >> On Sat, Oct 12, 2024 at 04:02:50PM +0100, Bryan O'Donoghue wrote: >>> diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml >>> index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 >>> --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml >>> +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml >>> @@ -28,12 +28,6 @@ properties: >>> items: >>> - description: Reference to the mclk clock. >>> >>> - assigned-clocks: >>> - maxItems: 1 >>> - >>> - assigned-clock-rates: >>> - maxItems: 1 >>> - >>> reset-gpios: >>> description: Reference to the GPIO connected to the RESETB pin. Active low. >>> maxItems: 1 >>> @@ -82,8 +76,6 @@ required: >>> - compatible >>> - reg >>> - clocks >>> - - assigned-clocks >>> - - assigned-clock-rates >> >> That's not extraneous, but has a meaning that without assigned-clocks >> this device or driver will not operate. >> >> File should rather stay as is. > > ... > >>> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml >>> index c978abc0cdb35cfe2b85069946cf1be435a58cb8..f0f9726a2add89492b8c56e17ed607841baa3a0d 100644 >>> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml >>> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml >>> @@ -24,10 +24,6 @@ properties: >>> - sony,imx258 >>> - sony,imx258-pdaf >>> >>> - assigned-clocks: true >>> - assigned-clock-parents: true >>> - assigned-clock-rates: true >>> - >> >> This is ok. > > Basically the clock related requirements for these devices are the same: > they all need a clock configured at a board specific frequency. Shouldn't > we treat them the same way? I don't know these devices, but binding did not express such requirement, so according to current binding the properties are 100% redundant. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-14 10:34 ` Krzysztof Kozlowski @ 2024-10-14 10:46 ` Sakari Ailus 0 siblings, 0 replies; 31+ messages in thread From: Sakari Ailus @ 2024-10-14 10:46 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel Hi Krzysztof, On Mon, Oct 14, 2024 at 12:34:51PM +0200, Krzysztof Kozlowski wrote: > On 14/10/2024 12:20, Sakari Ailus wrote: > > Hi Krzysztof, > > > > On Mon, Oct 14, 2024 at 09:43:07AM +0200, Krzysztof Kozlowski wrote: > >> On Sat, Oct 12, 2024 at 04:02:50PM +0100, Bryan O'Donoghue wrote: > >>> diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > >>> index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 > >>> --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > >>> +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > >>> @@ -28,12 +28,6 @@ properties: > >>> items: > >>> - description: Reference to the mclk clock. > >>> > >>> - assigned-clocks: > >>> - maxItems: 1 > >>> - > >>> - assigned-clock-rates: > >>> - maxItems: 1 > >>> - > >>> reset-gpios: > >>> description: Reference to the GPIO connected to the RESETB pin. Active low. > >>> maxItems: 1 > >>> @@ -82,8 +76,6 @@ required: > >>> - compatible > >>> - reg > >>> - clocks > >>> - - assigned-clocks > >>> - - assigned-clock-rates > >> > >> That's not extraneous, but has a meaning that without assigned-clocks > >> this device or driver will not operate. > >> > >> File should rather stay as is. > > > > ... > > > >>> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > >>> index c978abc0cdb35cfe2b85069946cf1be435a58cb8..f0f9726a2add89492b8c56e17ed607841baa3a0d 100644 > >>> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > >>> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > >>> @@ -24,10 +24,6 @@ properties: > >>> - sony,imx258 > >>> - sony,imx258-pdaf > >>> > >>> - assigned-clocks: true > >>> - assigned-clock-parents: true > >>> - assigned-clock-rates: true > >>> - > >> > >> This is ok. > > > > Basically the clock related requirements for these devices are the same: > > they all need a clock configured at a board specific frequency. Shouldn't > > we treat them the same way? > > I don't know these devices, but binding did not express such > requirement, so according to current binding the properties are 100% > redundant. It sounds like to me we should have both assigned-clocks and assigned-clock-rates on all of them. I recall from the earlier discussion setting the rate could have been a problem but based on the schema it looks like providing zero for the rate ot indicate not setting it is possible. Maybe we could have "csi2-sensor" and "parallel-sensor" bindings that would require, among other things, just this. -- Kind regards, Sakari Ailus ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-14 7:43 ` Krzysztof Kozlowski 2024-10-14 8:29 ` Bryan O'Donoghue 2024-10-14 10:20 ` Sakari Ailus @ 2024-10-14 20:34 ` Laurent Pinchart 2024-10-15 6:13 ` Krzysztof Kozlowski 2 siblings, 1 reply; 31+ messages in thread From: Laurent Pinchart @ 2024-10-14 20:34 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On Mon, Oct 14, 2024 at 09:43:07AM +0200, Krzysztof Kozlowski wrote: > On Sat, Oct 12, 2024 at 04:02:50PM +0100, Bryan O'Donoghue wrote: > > Remove extraneous assigned-clock* from media/i2c/* schemas, retain in the > > relevant examples. > > > > Link: https://lore.kernel.org/linux-media/j7kgz2lyxnler5qwd7yiazdq6fmsv77kyozdrxf33h54ydakjz@uqjhwhoyv6re > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > --- > > Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 8 -------- > > Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml | 8 -------- > > Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml | 8 -------- > > Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 4 ---- > > Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ---- > > Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml | 4 ---- > > Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 ---- > > Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml | 4 ---- > > 8 files changed, 44 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 > > --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > @@ -28,12 +28,6 @@ properties: > > items: > > - description: Reference to the mclk clock. > > > > - assigned-clocks: > > - maxItems: 1 > > - > > - assigned-clock-rates: > > - maxItems: 1 > > - > > reset-gpios: > > description: Reference to the GPIO connected to the RESETB pin. Active low. > > maxItems: 1 > > @@ -82,8 +76,6 @@ required: > > - compatible > > - reg > > - clocks > > - - assigned-clocks > > - - assigned-clock-rates > > That's not extraneous, but has a meaning that without assigned-clocks > this device or driver will not operate. How so ? Even if we assume that the device requires a specific clock frequency (which is often not the case for camera sensors, the limitation usually comes from drivers, so the constraint shouldn't be expressed in the bindings in that case), there is no overall requirement to assign a clock rate as in many cases the clock will come from a fixed-frequency oscillator. This seems to be a constraint that is outside of the scope of DT bindings. It is similar to regulators, where the regulator consumer doesn't have a way to express supported voltages in its DT bindings. > File should rather stay as is. > > > - vddio-supply > > - vdda-supply > > - vddd-supply [snip] -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-14 20:34 ` Laurent Pinchart @ 2024-10-15 6:13 ` Krzysztof Kozlowski 2024-10-15 11:29 ` Laurent Pinchart 0 siblings, 1 reply; 31+ messages in thread From: Krzysztof Kozlowski @ 2024-10-15 6:13 UTC (permalink / raw) To: Laurent Pinchart Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 14/10/2024 22:34, Laurent Pinchart wrote: > On Mon, Oct 14, 2024 at 09:43:07AM +0200, Krzysztof Kozlowski wrote: >> On Sat, Oct 12, 2024 at 04:02:50PM +0100, Bryan O'Donoghue wrote: >>> Remove extraneous assigned-clock* from media/i2c/* schemas, retain in the >>> relevant examples. >>> >>> Link: https://lore.kernel.org/linux-media/j7kgz2lyxnler5qwd7yiazdq6fmsv77kyozdrxf33h54ydakjz@uqjhwhoyv6re >>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> >>> --- >>> Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 8 -------- >>> Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml | 8 -------- >>> Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml | 8 -------- >>> Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 4 ---- >>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ---- >>> Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml | 4 ---- >>> Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 ---- >>> Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml | 4 ---- >>> 8 files changed, 44 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml >>> index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 >>> --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml >>> +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml >>> @@ -28,12 +28,6 @@ properties: >>> items: >>> - description: Reference to the mclk clock. >>> >>> - assigned-clocks: >>> - maxItems: 1 >>> - >>> - assigned-clock-rates: >>> - maxItems: 1 >>> - >>> reset-gpios: >>> description: Reference to the GPIO connected to the RESETB pin. Active low. >>> maxItems: 1 >>> @@ -82,8 +76,6 @@ required: >>> - compatible >>> - reg >>> - clocks >>> - - assigned-clocks >>> - - assigned-clock-rates >> >> That's not extraneous, but has a meaning that without assigned-clocks >> this device or driver will not operate. > > How so ? Even if we assume that the device requires a specific clock > frequency (which is often not the case for camera sensors, the > limitation usually comes from drivers, so the constraint shouldn't be > expressed in the bindings in that case), there is no overall requirement > to assign a clock rate as in many cases the clock will come from a > fixed-frequency oscillator. This seems to be a constraint that is > outside of the scope of DT bindings. It is similar to regulators, where > the regulator consumer doesn't have a way to express supported voltages > in its DT bindings. This property does not say it comes from a fixed-frequency oscillator, so I do not understand why you think it is unreasonable constraint. I have no clue what the author wanted to say here, so I just explained that there is a meaning behind requiring such properties. If you claim device or implementations do not have such requirement, after investigating each case, feel free to drop this. I think I also stated this already in other reply. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-15 6:13 ` Krzysztof Kozlowski @ 2024-10-15 11:29 ` Laurent Pinchart 2024-10-15 16:46 ` Rob Herring 0 siblings, 1 reply; 31+ messages in thread From: Laurent Pinchart @ 2024-10-15 11:29 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On Tue, Oct 15, 2024 at 08:13:19AM +0200, Krzysztof Kozlowski wrote: > On 14/10/2024 22:34, Laurent Pinchart wrote: > > On Mon, Oct 14, 2024 at 09:43:07AM +0200, Krzysztof Kozlowski wrote: > >> On Sat, Oct 12, 2024 at 04:02:50PM +0100, Bryan O'Donoghue wrote: > >>> Remove extraneous assigned-clock* from media/i2c/* schemas, retain in the > >>> relevant examples. > >>> > >>> Link: https://lore.kernel.org/linux-media/j7kgz2lyxnler5qwd7yiazdq6fmsv77kyozdrxf33h54ydakjz@uqjhwhoyv6re > >>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > >>> --- > >>> Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 8 -------- > >>> Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml | 8 -------- > >>> Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml | 8 -------- > >>> Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 4 ---- > >>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ---- > >>> Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml | 4 ---- > >>> Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 ---- > >>> Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml | 4 ---- > >>> 8 files changed, 44 deletions(-) > >>> > >>> diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > >>> index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 > >>> --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > >>> +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > >>> @@ -28,12 +28,6 @@ properties: > >>> items: > >>> - description: Reference to the mclk clock. > >>> > >>> - assigned-clocks: > >>> - maxItems: 1 > >>> - > >>> - assigned-clock-rates: > >>> - maxItems: 1 > >>> - > >>> reset-gpios: > >>> description: Reference to the GPIO connected to the RESETB pin. Active low. > >>> maxItems: 1 > >>> @@ -82,8 +76,6 @@ required: > >>> - compatible > >>> - reg > >>> - clocks > >>> - - assigned-clocks > >>> - - assigned-clock-rates > >> > >> That's not extraneous, but has a meaning that without assigned-clocks > >> this device or driver will not operate. > > > > How so ? Even if we assume that the device requires a specific clock > > frequency (which is often not the case for camera sensors, the > > limitation usually comes from drivers, so the constraint shouldn't be > > expressed in the bindings in that case), there is no overall requirement > > to assign a clock rate as in many cases the clock will come from a > > fixed-frequency oscillator. This seems to be a constraint that is > > outside of the scope of DT bindings. It is similar to regulators, where > > the regulator consumer doesn't have a way to express supported voltages > > in its DT bindings. > > This property does not say it comes from a fixed-frequency oscillator, > so I do not understand why you think it is unreasonable constraint. I > have no clue what the author wanted to say here, so I just explained > that there is a meaning behind requiring such properties. If you claim > device or implementations do not have such requirement, after > investigating each case, feel free to drop this. I think I also stated > this already in other reply. For camera sensor drivers I'm pretty sure we can drop those properties when they're marked as required, as there's no intrinsic characteristics of camera sensors that would require assigned-clock*. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-15 11:29 ` Laurent Pinchart @ 2024-10-15 16:46 ` Rob Herring 2024-10-16 7:47 ` Krzysztof Kozlowski 0 siblings, 1 reply; 31+ messages in thread From: Rob Herring @ 2024-10-15 16:46 UTC (permalink / raw) To: Laurent Pinchart Cc: Krzysztof Kozlowski, Bryan O'Donoghue, Mauro Carvalho Chehab, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On Tue, Oct 15, 2024 at 02:29:22PM +0300, Laurent Pinchart wrote: > On Tue, Oct 15, 2024 at 08:13:19AM +0200, Krzysztof Kozlowski wrote: > > On 14/10/2024 22:34, Laurent Pinchart wrote: > > > On Mon, Oct 14, 2024 at 09:43:07AM +0200, Krzysztof Kozlowski wrote: > > >> On Sat, Oct 12, 2024 at 04:02:50PM +0100, Bryan O'Donoghue wrote: > > >>> Remove extraneous assigned-clock* from media/i2c/* schemas, retain in the > > >>> relevant examples. > > >>> > > >>> Link: https://lore.kernel.org/linux-media/j7kgz2lyxnler5qwd7yiazdq6fmsv77kyozdrxf33h54ydakjz@uqjhwhoyv6re > > >>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > > >>> --- > > >>> Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 8 -------- > > >>> Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml | 8 -------- > > >>> Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml | 8 -------- > > >>> Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 4 ---- > > >>> Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 ---- > > >>> Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml | 4 ---- > > >>> Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 ---- > > >>> Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml | 4 ---- > > >>> 8 files changed, 44 deletions(-) > > >>> > > >>> diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > >>> index 60f19e1152b33128cf3baa15b8c70a874ca6d52e..d18ead8f7fc43bfacc291aed85b5ca9166c46edb 100644 > > >>> --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > >>> +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > > >>> @@ -28,12 +28,6 @@ properties: > > >>> items: > > >>> - description: Reference to the mclk clock. > > >>> > > >>> - assigned-clocks: > > >>> - maxItems: 1 > > >>> - > > >>> - assigned-clock-rates: > > >>> - maxItems: 1 > > >>> - > > >>> reset-gpios: > > >>> description: Reference to the GPIO connected to the RESETB pin. Active low. > > >>> maxItems: 1 > > >>> @@ -82,8 +76,6 @@ required: > > >>> - compatible > > >>> - reg > > >>> - clocks > > >>> - - assigned-clocks > > >>> - - assigned-clock-rates > > >> > > >> That's not extraneous, but has a meaning that without assigned-clocks > > >> this device or driver will not operate. > > > > > > How so ? Even if we assume that the device requires a specific clock > > > frequency (which is often not the case for camera sensors, the > > > limitation usually comes from drivers, so the constraint shouldn't be > > > expressed in the bindings in that case), there is no overall requirement > > > to assign a clock rate as in many cases the clock will come from a > > > fixed-frequency oscillator. This seems to be a constraint that is > > > outside of the scope of DT bindings. It is similar to regulators, where > > > the regulator consumer doesn't have a way to express supported voltages > > > in its DT bindings. > > > > This property does not say it comes from a fixed-frequency oscillator, > > so I do not understand why you think it is unreasonable constraint. I > > have no clue what the author wanted to say here, so I just explained > > that there is a meaning behind requiring such properties. If you claim > > device or implementations do not have such requirement, after > > investigating each case, feel free to drop this. I think I also stated > > this already in other reply. > > For camera sensor drivers I'm pretty sure we can drop those properties > when they're marked as required, as there's no intrinsic characteristics > of camera sensors that would require assigned-clock*. I tend to agree, and would take it one step further to say that applies everywhere. Whatever clock setup needed is outside the scope of a binding. The simplest case is all input clocks are fixed. The next simplest case is firmware did all clock setup needed for a specific board and the boot time default works. Rob ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema 2024-10-15 16:46 ` Rob Herring @ 2024-10-16 7:47 ` Krzysztof Kozlowski 0 siblings, 0 replies; 31+ messages in thread From: Krzysztof Kozlowski @ 2024-10-16 7:47 UTC (permalink / raw) To: Rob Herring, Laurent Pinchart Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 15/10/2024 18:46, Rob Herring wrote: >>>> >>>> How so ? Even if we assume that the device requires a specific clock >>>> frequency (which is often not the case for camera sensors, the >>>> limitation usually comes from drivers, so the constraint shouldn't be >>>> expressed in the bindings in that case), there is no overall requirement >>>> to assign a clock rate as in many cases the clock will come from a >>>> fixed-frequency oscillator. This seems to be a constraint that is >>>> outside of the scope of DT bindings. It is similar to regulators, where >>>> the regulator consumer doesn't have a way to express supported voltages >>>> in its DT bindings. >>> >>> This property does not say it comes from a fixed-frequency oscillator, >>> so I do not understand why you think it is unreasonable constraint. I >>> have no clue what the author wanted to say here, so I just explained >>> that there is a meaning behind requiring such properties. If you claim >>> device or implementations do not have such requirement, after >>> investigating each case, feel free to drop this. I think I also stated >>> this already in other reply. >> >> For camera sensor drivers I'm pretty sure we can drop those properties >> when they're marked as required, as there's no intrinsic characteristics >> of camera sensors that would require assigned-clock*. > > I tend to agree, and would take it one step further to say that applies > everywhere. Whatever clock setup needed is outside the scope of a > binding. The simplest case is all input clocks are fixed. The next > simplest case is firmware did all clock setup needed for a specific > board and the boot time default works. Ack. Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-12 15:02 [PATCH 0/2] media: i2c: Cleanup assigned-clocks and endpoint: properties: unevaluatedProperties: false Bryan O'Donoghue 2024-10-12 15:02 ` [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema Bryan O'Donoghue @ 2024-10-12 15:02 ` Bryan O'Donoghue 2024-10-12 18:09 ` Laurent Pinchart 2024-10-14 7:45 ` Krzysztof Kozlowski 1 sibling, 2 replies; 31+ messages in thread From: Bryan O'Donoghue @ 2024-10-12 15:02 UTC (permalink / raw) To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart Cc: Krzysztof Kozlowski, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel, Bryan O'Donoghue Some of our sensor schemas use unevaluatedProperities: false for endpoint: properties: while other schemas use additionalProperties: false. The effect of using unevaluatedProperities: false in this instance is that any property in media/video-interfaces.yaml can be considered in a dts for an endpoint. Converting to additionalProperties: false and running DT checkers show that such a liberal policy is unnecessary. We should have a consistent way of defining these properties if for no other reason than aid other developers in the preferred way of writing these schemas for media/i2c in the future. Convert to additionalProperties: as a result remote-endpoint needs to be added to the property list for most sensors. In a few cases some additional properties clock data-lanes or clock-lanes need to be added too but, for-the-most-part remote-endpoint is the only missing property. Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> --- .../devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml | 5 ++++- Documentation/devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml | 6 +++++- Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/imx219.yaml | 6 +++++- Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml | 5 ++++- Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml | 3 ++- Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml | 5 ++++- Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/sony,imx283.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml | 4 +++- Documentation/devicetree/bindings/media/i2c/toshiba,tc358746.yaml | 4 +++- 23 files changed, 75 insertions(+), 23 deletions(-) diff --git a/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml index d3329e991d1652936fcf671012b8018e4317ea40..ba166ecf4fcbb77efab69ebcbdb46f5666af8e77 100644 --- a/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml +++ b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml @@ -32,7 +32,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: link-frequencies: true @@ -45,9 +45,12 @@ properties: - const: 3 - const: 4 + remote-endpoint: true + required: - data-lanes - link-frequencies + - remote-endpoint required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml index 0e7a7b5ac89f618e6cba0d86f6f7ea853e59ae1e..8b42440586aa8c853d8bf6046ccab0c3b23cb907 100644 --- a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml +++ b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml @@ -44,7 +44,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -59,10 +59,12 @@ properties: - const: 2 link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - endpoint diff --git a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml index 51b8ece09c722e057fdb01b38d3e360e7604f39a..c15169ef901139411273e110523a311d87b4322e 100644 --- a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml +++ b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml @@ -44,7 +44,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -59,10 +59,12 @@ properties: - const: 2 link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - endpoint diff --git a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml index 9eac588de0bc28d85f44663afe5472e35f1e652c..702625962d90ea7fafb4f4f4f865659097b51406 100644 --- a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml +++ b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml @@ -56,13 +56,17 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: + data-lanes: true link-frequencies: true + remote-endpoint: true required: + - data-lanes - link-frequencies + - remote-endpoint required: - endpoint diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml index d18ead8f7fc43bfacc291aed85b5ca9166c46edb..52bb089bd67fd0f9b5464e068b8db0b8e4406b3d 100644 --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml @@ -52,7 +52,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -67,10 +67,12 @@ properties: - const: 2 link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml index 07d088cf66e0bde362b12d3494e5c91a1dd96bf3..5f395cf04b95ca47d5e685b8c43b8265db6910ae 100644 --- a/Documentation/devicetree/bindings/media/i2c/imx219.yaml +++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml @@ -52,7 +52,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -65,10 +65,14 @@ properties: - const: 2 clock-noncontinuous: true + clock-lanes: true link-frequencies: true + remote-endpoint: true required: + - data-lanes - link-frequencies + - remote-endpoint required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml b/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml index f8ace8cbccdbac482ffba733d5b28a3a38aaf822..ce45bd8409597fa6989f632d68cd4aa1a468d152 100644 --- a/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml +++ b/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml @@ -77,7 +77,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: link-frequencies: true @@ -87,11 +87,13 @@ properties: - 1 # CSI-2 C-PHY - 3 # CCP2 - 4 # CSI-2 D-PHY + remote-endpoint: true required: - link-frequencies - data-lanes - bus-type + - remote-endpoint required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml index ca57c01739d2b93100a37db56255ab717c1197ff..9b3738956c482d8826bf64f557c2e91630ea9799 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml @@ -55,7 +55,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -65,10 +65,12 @@ properties: enum: [1, 2] link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml index 67c1c291327b7febb6a039bf6f28c8dc1f32ed7f..b8db4be137085fe31ec2f076c7dc66b30bf0b66c 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml @@ -77,7 +77,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: link-frequencies: true @@ -88,9 +88,11 @@ properties: the link speed defined by the 'link-frequencies' property. If present, the value shall be in the range of 0-4. default: 4 + remote-endpoint: true required: - link-frequencies + - remote-endpoint required: - endpoint diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml index d96199031b66c5c162a034824f195e277f2a1795..7499523a6e0fbd64b9b980333adaa14a0c40a33b 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml @@ -61,7 +61,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -77,10 +77,12 @@ properties: - items: - const: 1 link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml index 622243cae03caa5d14aa312df40ef5907e190d2c..358c0422451f7faa8e0ebfc9226a5cfb087e3598 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml @@ -45,7 +45,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: link-frequencies: true @@ -54,9 +54,12 @@ properties: minItems: 1 maxItems: 2 + remote-endpoint: true + required: - data-lanes - link-frequencies + - remote-endpoint required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml index ad07204057f979ade534d29c99c3aff7372bd332..eff212524bf3c7b1ec6aa7e826d4318d58ba53a5 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml @@ -60,7 +60,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -69,6 +69,7 @@ properties: # Supports max data transfer of 900 Mbps per lane link-frequencies: true + remote-endpoint: true required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml index 2e5187acbbb89728cbb8a402559d24766198a3da..cbbe3c9ce151eb33d2b0cc1a44e6ebf66d9b59fa 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml @@ -53,7 +53,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: clock-lanes: @@ -63,10 +63,12 @@ properties: maxItems: 1 link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml index 382d7de7a89bcea11be384a2a3800512994f9346..dd5c5715fdcfc00e6d851f375f41e4d077b92bc0 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml @@ -45,7 +45,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: link-frequencies: true @@ -54,9 +54,12 @@ properties: minItems: 1 maxItems: 4 + remote-endpoint: true + required: - data-lanes - link-frequencies + - remote-endpoint required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml index 38325cf318f7bd2cd60a4c7bbb6a65b54b855e26..dde4e7426bf00920f1bd9ed1bf4d8594932dedaf 100644 --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml @@ -51,15 +51,17 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: true link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - endpoint diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml index 0162eec8ca993a7614d29908f89fa9fe6d4b545d..9b78ff6bd5f114c7f63ac90e71fa677445ddf702 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml @@ -58,7 +58,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -73,10 +73,12 @@ properties: - const: 4 link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint additionalProperties: false diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml index f0f9726a2add89492b8c56e17ed607841baa3a0d..4cf49472c24f1b800f6d2e41b8716e2ac32f959a 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml @@ -56,7 +56,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -71,10 +71,12 @@ properties: - const: 2 link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - compatible diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx283.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx283.yaml index e4f49f1435a5c2e6e1507d250662ea6ecbf3c7dc..75b78a3e925ed2fd09f56c8349d234a32739f141 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx283.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx283.yaml @@ -48,7 +48,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -60,10 +60,12 @@ properties: - const: 4 link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - endpoint diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml index bf05ca48601abda53d60a3d03aa556e7b8fd825b..e6aec7a1ba2b22a11111d19a61384f1200041df5 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml @@ -71,7 +71,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -86,10 +86,12 @@ properties: - const: 4 link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint additionalProperties: false diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml index 872b8288948b2bba743f2365a55165181df156ae..d30ef330e5af225728d1a6c40b26050cd33ba4be 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml @@ -38,15 +38,17 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: true link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - endpoint diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml index 38bd1c7304a59bb5fea90954c1e4e626a7c86f2f..36c3a0ba7822475770cd903cec3343d31bb66520 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml @@ -48,15 +48,17 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: true link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - endpoint diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml index ece1e17fe34553671e61c965eb1833c5eb08131b..0bbdf657a8c0643ffe512ae04c14dfc8e6b4fc94 100644 --- a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml @@ -50,15 +50,17 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: true link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - endpoint diff --git a/Documentation/devicetree/bindings/media/i2c/toshiba,tc358746.yaml b/Documentation/devicetree/bindings/media/i2c/toshiba,tc358746.yaml index 1c476b635b690865cff0882578d72b1db2dc7c50..367d669ad864ed6b2a8762f953f58e206c8c8194 100644 --- a/Documentation/devicetree/bindings/media/i2c/toshiba,tc358746.yaml +++ b/Documentation/devicetree/bindings/media/i2c/toshiba,tc358746.yaml @@ -96,7 +96,7 @@ properties: properties: endpoint: $ref: /schemas/media/video-interfaces.yaml# - unevaluatedProperties: false + additionalProperties: false properties: data-lanes: @@ -105,10 +105,12 @@ properties: clock-noncontinuous: true link-frequencies: true + remote-endpoint: true required: - data-lanes - link-frequencies + - remote-endpoint required: - port@0 -- 2.46.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-12 15:02 ` [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: Bryan O'Donoghue @ 2024-10-12 18:09 ` Laurent Pinchart 2024-10-13 10:44 ` Bryan O'Donoghue 2024-10-14 7:45 ` Krzysztof Kozlowski 1 sibling, 1 reply; 31+ messages in thread From: Laurent Pinchart @ 2024-10-12 18:09 UTC (permalink / raw) To: Bryan O'Donoghue Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Krzysztof Kozlowski, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel Hi Bryan, Thank you for the patch. On Sat, Oct 12, 2024 at 04:02:51PM +0100, Bryan O'Donoghue wrote: > Some of our sensor schemas use unevaluatedProperities: false for endpoint: s/unevaluatedProperities/unevaluatedProperties/ Same below. > properties: while other schemas use additionalProperties: false. > > The effect of using unevaluatedProperities: false in this instance is that > any property in media/video-interfaces.yaml can be considered in a dts for > an endpoint. > > Converting to additionalProperties: false and running DT checkers show that > such a liberal policy is unnecessary. > > We should have a consistent way of defining these properties if for no > other reason than aid other developers in the preferred way of writing > these schemas for media/i2c in the future. > > Convert to additionalProperties: as a result remote-endpoint needs to be > added to the property list for most sensors. In a few cases some > additional properties clock data-lanes or clock-lanes need to be added too > but, for-the-most-part remote-endpoint is the only missing property. > > Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org> > --- > .../devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml | 5 ++++- > Documentation/devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml | 6 +++++- > Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/imx219.yaml | 6 +++++- > Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml | 5 ++++- > Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml | 3 ++- > Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml | 5 ++++- > Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/sony,imx283.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml | 4 +++- > Documentation/devicetree/bindings/media/i2c/toshiba,tc358746.yaml | 4 +++- > 23 files changed, 75 insertions(+), 23 deletions(-) > > diff --git a/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml > index d3329e991d1652936fcf671012b8018e4317ea40..ba166ecf4fcbb77efab69ebcbdb46f5666af8e77 100644 > --- a/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/alliedvision,alvium-csi2.yaml > @@ -32,7 +32,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > link-frequencies: true > @@ -45,9 +45,12 @@ properties: > - const: 3 > - const: 4 > > + remote-endpoint: true > + > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml > index 0e7a7b5ac89f618e6cba0d86f6f7ea853e59ae1e..8b42440586aa8c853d8bf6046ccab0c3b23cb907 100644 > --- a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc05a2.yaml > @@ -44,7 +44,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -59,10 +59,12 @@ properties: > - const: 2 > > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - endpoint > diff --git a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml > index 51b8ece09c722e057fdb01b38d3e360e7604f39a..c15169ef901139411273e110523a311d87b4322e 100644 > --- a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc08a3.yaml > @@ -44,7 +44,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -59,10 +59,12 @@ properties: > - const: 2 > > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - endpoint > diff --git a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml > index 9eac588de0bc28d85f44663afe5472e35f1e652c..702625962d90ea7fafb4f4f4f865659097b51406 100644 > --- a/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/galaxycore,gc2145.yaml > @@ -56,13 +56,17 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > + data-lanes: true We should have a more precise constraint here. The sensor supports single or dual data lanes operation, so you can write data-lanes: minItems: 1 items: - const: 1 - const: 2 > link-frequencies: true > + remote-endpoint: true > > required: > + - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - endpoint > diff --git a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > index d18ead8f7fc43bfacc291aed85b5ca9166c46edb..52bb089bd67fd0f9b5464e068b8db0b8e4406b3d 100644 > --- a/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/hynix,hi846.yaml > @@ -52,7 +52,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -67,10 +67,12 @@ properties: > - const: 2 > > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/imx219.yaml b/Documentation/devicetree/bindings/media/i2c/imx219.yaml > index 07d088cf66e0bde362b12d3494e5c91a1dd96bf3..5f395cf04b95ca47d5e685b8c43b8265db6910ae 100644 > --- a/Documentation/devicetree/bindings/media/i2c/imx219.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/imx219.yaml > @@ -52,7 +52,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -65,10 +65,14 @@ properties: > - const: 2 > > clock-noncontinuous: true > + clock-lanes: true This shouldn't be needed, as the sensor doesn't support clock lane remapping. Could we drop the clock-lanes property from upstream device tree sources instead ? > link-frequencies: true > + remote-endpoint: true > > required: > + - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml b/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml > index f8ace8cbccdbac482ffba733d5b28a3a38aaf822..ce45bd8409597fa6989f632d68cd4aa1a468d152 100644 > --- a/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/mipi-ccs.yaml > @@ -77,7 +77,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > link-frequencies: true > @@ -87,11 +87,13 @@ properties: > - 1 # CSI-2 C-PHY > - 3 # CCP2 > - 4 # CSI-2 D-PHY > + remote-endpoint: true > > required: > - link-frequencies > - data-lanes > - bus-type > + - remote-endpoint > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml > index ca57c01739d2b93100a37db56255ab717c1197ff..9b3738956c482d8826bf64f557c2e91630ea9799 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,og01a1b.yaml > @@ -55,7 +55,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -65,10 +65,12 @@ properties: > enum: [1, 2] > > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > index 67c1c291327b7febb6a039bf6f28c8dc1f32ed7f..b8db4be137085fe31ec2f076c7dc66b30bf0b66c 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov02a10.yaml > @@ -77,7 +77,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > link-frequencies: true > @@ -88,9 +88,11 @@ properties: > the link speed defined by the 'link-frequencies' property. > If present, the value shall be in the range of 0-4. > default: 4 > + remote-endpoint: true > > required: > - link-frequencies > + - remote-endpoint > > required: > - endpoint > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml > index d96199031b66c5c162a034824f195e277f2a1795..7499523a6e0fbd64b9b980333adaa14a0c40a33b 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov4689.yaml > @@ -61,7 +61,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -77,10 +77,12 @@ properties: > - items: > - const: 1 > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml > index 622243cae03caa5d14aa312df40ef5907e190d2c..358c0422451f7faa8e0ebfc9226a5cfb087e3598 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5648.yaml > @@ -45,7 +45,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > link-frequencies: true > @@ -54,9 +54,12 @@ properties: > minItems: 1 > maxItems: 2 > > + remote-endpoint: true > + > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml > index ad07204057f979ade534d29c99c3aff7372bd332..eff212524bf3c7b1ec6aa7e826d4318d58ba53a5 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov5675.yaml > @@ -60,7 +60,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -69,6 +69,7 @@ properties: > > # Supports max data transfer of 900 Mbps per lane > link-frequencies: true > + remote-endpoint: true > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml > index 2e5187acbbb89728cbb8a402559d24766198a3da..cbbe3c9ce151eb33d2b0cc1a44e6ebf66d9b59fa 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov7251.yaml > @@ -53,7 +53,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > clock-lanes: > @@ -63,10 +63,12 @@ properties: > maxItems: 1 > > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > index 382d7de7a89bcea11be384a2a3800512994f9346..dd5c5715fdcfc00e6d851f375f41e4d077b92bc0 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov8865.yaml > @@ -45,7 +45,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > link-frequencies: true > @@ -54,9 +54,12 @@ properties: > minItems: 1 > maxItems: 4 > > + remote-endpoint: true > + > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml > index 38325cf318f7bd2cd60a4c7bbb6a65b54b855e26..dde4e7426bf00920f1bd9ed1bf4d8594932dedaf 100644 > --- a/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/ovti,ov9282.yaml > @@ -51,15 +51,17 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: true > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - endpoint > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml > index 0162eec8ca993a7614d29908f89fa9fe6d4b545d..9b78ff6bd5f114c7f63ac90e71fa677445ddf702 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx214.yaml > @@ -58,7 +58,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -73,10 +73,12 @@ properties: > - const: 4 > > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > additionalProperties: false > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > index f0f9726a2add89492b8c56e17ed607841baa3a0d..4cf49472c24f1b800f6d2e41b8716e2ac32f959a 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml > @@ -56,7 +56,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -71,10 +71,12 @@ properties: > - const: 2 > > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - compatible > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx283.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx283.yaml > index e4f49f1435a5c2e6e1507d250662ea6ecbf3c7dc..75b78a3e925ed2fd09f56c8349d234a32739f141 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx283.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx283.yaml > @@ -48,7 +48,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -60,10 +60,12 @@ properties: > - const: 4 > > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - endpoint > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml > index bf05ca48601abda53d60a3d03aa556e7b8fd825b..e6aec7a1ba2b22a11111d19a61384f1200041df5 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx290.yaml > @@ -71,7 +71,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -86,10 +86,12 @@ properties: > - const: 4 > > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > additionalProperties: false > > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml > index 872b8288948b2bba743f2365a55165181df156ae..d30ef330e5af225728d1a6c40b26050cd33ba4be 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx334.yaml > @@ -38,15 +38,17 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: true > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - endpoint > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml > index 38bd1c7304a59bb5fea90954c1e4e626a7c86f2f..36c3a0ba7822475770cd903cec3343d31bb66520 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx335.yaml > @@ -48,15 +48,17 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: true > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - endpoint > diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml > index ece1e17fe34553671e61c965eb1833c5eb08131b..0bbdf657a8c0643ffe512ae04c14dfc8e6b4fc94 100644 > --- a/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx412.yaml > @@ -50,15 +50,17 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: true > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - endpoint > diff --git a/Documentation/devicetree/bindings/media/i2c/toshiba,tc358746.yaml b/Documentation/devicetree/bindings/media/i2c/toshiba,tc358746.yaml > index 1c476b635b690865cff0882578d72b1db2dc7c50..367d669ad864ed6b2a8762f953f58e206c8c8194 100644 > --- a/Documentation/devicetree/bindings/media/i2c/toshiba,tc358746.yaml > +++ b/Documentation/devicetree/bindings/media/i2c/toshiba,tc358746.yaml > @@ -96,7 +96,7 @@ properties: > properties: > endpoint: > $ref: /schemas/media/video-interfaces.yaml# > - unevaluatedProperties: false > + additionalProperties: false > > properties: > data-lanes: > @@ -105,10 +105,12 @@ properties: > > clock-noncontinuous: true > link-frequencies: true > + remote-endpoint: true > > required: > - data-lanes > - link-frequencies > + - remote-endpoint > > required: > - port@0 > -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-12 18:09 ` Laurent Pinchart @ 2024-10-13 10:44 ` Bryan O'Donoghue 0 siblings, 0 replies; 31+ messages in thread From: Bryan O'Donoghue @ 2024-10-13 10:44 UTC (permalink / raw) To: Laurent Pinchart Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Krzysztof Kozlowski, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 12/10/2024 19:09, Laurent Pinchart wrote: >> + clock-lanes: true > This shouldn't be needed, as the sensor doesn't support clock lane > remapping. Could we drop the clock-lanes property from upstream device > tree sources instead ? Yes probably. --- bod ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-12 15:02 ` [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: Bryan O'Donoghue 2024-10-12 18:09 ` Laurent Pinchart @ 2024-10-14 7:45 ` Krzysztof Kozlowski 2024-10-14 8:31 ` Bryan O'Donoghue 1 sibling, 1 reply; 31+ messages in thread From: Krzysztof Kozlowski @ 2024-10-14 7:45 UTC (permalink / raw) To: Bryan O'Donoghue Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On Sat, Oct 12, 2024 at 04:02:51PM +0100, Bryan O'Donoghue wrote: > Some of our sensor schemas use unevaluatedProperities: false for endpoint: > properties: while other schemas use additionalProperties: false. > > The effect of using unevaluatedProperities: false in this instance is that > any property in media/video-interfaces.yaml can be considered in a dts for > an endpoint. ... which is what we want and what is expected. You change the code from expected to less expected variant, so please clearly document why. > > Converting to additionalProperties: false and running DT checkers show that > such a liberal policy is unnecessary. > > We should have a consistent way of defining these properties if for no > other reason than aid other developers in the preferred way of writing > these schemas for media/i2c in the future. There is consistent way - common schema defines them. I do not understand the reasoning behind this change at all. I don't think DT maintainers ever suggested it (in fact, rather opposite: suggested using unevaluatedProps) and I think is not a consensus of any talks. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-14 7:45 ` Krzysztof Kozlowski @ 2024-10-14 8:31 ` Bryan O'Donoghue 2024-10-14 8:47 ` Krzysztof Kozlowski 0 siblings, 1 reply; 31+ messages in thread From: Bryan O'Donoghue @ 2024-10-14 8:31 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 14/10/2024 08:45, Krzysztof Kozlowski wrote: > I do not understand the reasoning behind this change at all. I don't > think DT maintainers ever suggested it (in fact, rather opposite: > suggested using unevaluatedProps) and I think is not a consensus of any > talks. No there is not but then, how do you give consistent feedback except proposing something to be a baseline. On the one hand you have upstream additionalProperties: false and unevaluatedProperites: false - it'd be better to have a consistent message on which is to be used. --- bod ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-14 8:31 ` Bryan O'Donoghue @ 2024-10-14 8:47 ` Krzysztof Kozlowski 2024-10-14 9:03 ` Bryan O'Donoghue 2024-10-14 20:29 ` Laurent Pinchart 0 siblings, 2 replies; 31+ messages in thread From: Krzysztof Kozlowski @ 2024-10-14 8:47 UTC (permalink / raw) To: Bryan O'Donoghue Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 14/10/2024 10:31, Bryan O'Donoghue wrote: > On 14/10/2024 08:45, Krzysztof Kozlowski wrote: >> I do not understand the reasoning behind this change at all. I don't >> think DT maintainers ever suggested it (in fact, rather opposite: >> suggested using unevaluatedProps) and I think is not a consensus of any >> talks. > > No there is not but then, how do you give consistent feedback except > proposing something to be a baseline. > > On the one hand you have upstream additionalProperties: false and > unevaluatedProperites: false - it'd be better to have a consistent > message on which is to be used. Well, I am afraid that push towards additionalProps will lead to grow common schema (video-interface-devices or video-interfaces) into huge one-fit-all binding. And that's not good. If a common binding for a group of devices encourages you to list its subset, then it is not that common. Solution is to fix that, e.g. split it per classes of devices. Or don't care and use unevaluatedProps because it makes people's life easier and is still correct. If it is not correct, then this should be used as an argument. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-14 8:47 ` Krzysztof Kozlowski @ 2024-10-14 9:03 ` Bryan O'Donoghue 2024-10-14 10:37 ` Krzysztof Kozlowski 2024-10-14 20:29 ` Laurent Pinchart 1 sibling, 1 reply; 31+ messages in thread From: Bryan O'Donoghue @ 2024-10-14 9:03 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 14/10/2024 09:47, Krzysztof Kozlowski wrote: > If a common binding for a group of devices encourages you to list its > subset, then it is not that common. > > Solution is to fix that, e.g. split it per classes of devices. It might be possible to have $ref: /schemas/media/video-interfaces-endpoint-defaults.yaml# which declares the typical list -> $ref: /schemas/media/video-interfaces.yaml# additonalProperties:false properties: data-lanes: true link-frequencies: true remote-endpoints: true required: data-lanes link-frequencies remote-endpoints and then if you need say clock-noncontinuous you'd just include $ref: /schemas/media/video-interfaces.yaml# unevaluatedProperties: false and then list whatever you need > Or don't care and use unevaluatedProps because it makes people's life > easier and is still correct. If it is not correct, then this should be > used as an argument. I'll wait to see what people think before progressing this patch further. --- bod ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-14 9:03 ` Bryan O'Donoghue @ 2024-10-14 10:37 ` Krzysztof Kozlowski 0 siblings, 0 replies; 31+ messages in thread From: Krzysztof Kozlowski @ 2024-10-14 10:37 UTC (permalink / raw) To: Bryan O'Donoghue Cc: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, Laurent Pinchart, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 14/10/2024 11:03, Bryan O'Donoghue wrote: > On 14/10/2024 09:47, Krzysztof Kozlowski wrote: >> If a common binding for a group of devices encourages you to list its >> subset, then it is not that common. >> >> Solution is to fix that, e.g. split it per classes of devices. > > It might be possible to have > > $ref: /schemas/media/video-interfaces-endpoint-defaults.yaml# > > which declares the typical list -> > > $ref: /schemas/media/video-interfaces.yaml# > additonalProperties:false I meant something else - define common schema matching this class of devices. Not a schema for some defaults. This is supposed to reflect how we look at hardware, not some library of schemas or library of functions. > > properties: > data-lanes: true > link-frequencies: true > remote-endpoints: true and I still don't like all these. I rather expect people to list the hardware constraints or just drop it. I mentioned it some time ago in the first patch you sent, which I think started this entire discussion. > > required: > data-lanes > link-frequencies > remote-endpoints > > and then if you need say clock-noncontinuous you'd just include > > $ref: /schemas/media/video-interfaces.yaml# > unevaluatedProperties: false > > and then list whatever you need > >> Or don't care and use unevaluatedProps because it makes people's life >> easier and is still correct. If it is not correct, then this should be >> used as an argument. > > I'll wait to see what people think before progressing this patch further. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-14 8:47 ` Krzysztof Kozlowski 2024-10-14 9:03 ` Bryan O'Donoghue @ 2024-10-14 20:29 ` Laurent Pinchart 2024-10-15 6:11 ` Krzysztof Kozlowski 1 sibling, 1 reply; 31+ messages in thread From: Laurent Pinchart @ 2024-10-14 20:29 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On Mon, Oct 14, 2024 at 10:47:31AM +0200, Krzysztof Kozlowski wrote: > On 14/10/2024 10:31, Bryan O'Donoghue wrote: > > On 14/10/2024 08:45, Krzysztof Kozlowski wrote: > >> I do not understand the reasoning behind this change at all. I don't > >> think DT maintainers ever suggested it (in fact, rather opposite: > >> suggested using unevaluatedProps) and I think is not a consensus of any > >> talks. > > > > No there is not but then, how do you give consistent feedback except > > proposing something to be a baseline. > > > > On the one hand you have upstream additionalProperties: false and > > unevaluatedProperites: false - it'd be better to have a consistent > > message on which is to be used. > > Well, I am afraid that push towards additionalProps will lead to grow > common schema (video-interface-devices or video-interfaces) into huge > one-fit-all binding. And that's not good. > > If a common binding for a group of devices encourages you to list its > subset, then it is not that common. > > Solution is to fix that, e.g. split it per classes of devices. I think splitting large schemas per class is a good idea, but the problem will still exist. For instance, if we were to move the CSI-2-specific properties to a separate schema, that schema would define clock-lanes, data-lanes and clock-noncontinuous. The clock-lanes and clock-noncontinuous properties do not apply to every device, how would we then handle that ? I see three options: - Use "additionalProperties: false" and explicitly list the properties that apply to the device with "$prop: true". This is what this series does. - Use "unevaluatedProperites: false" and explicitly list the properties that do not apply to the device with "$prop: false". The drawback is that any property being added to the common schema will require modifications to all bindings that use the schema. - Use "unevaluatedProperites: false" and don't list any property with "$prop: false". This is what is being done today in many bindings. The drawback is that device tree sources that specify invalid properties for the device will validate. Among those options, my preference goes to the first one. It catches the most issues in device tree sources, while not having the drawback of the second option. It requires explicitly listing the valid properties, but I don't consider that as a drawback, I think it's actually a good thing as it clearly shows to the developers which properties are valid. Are there other options I haven't considered ? > Or don't care and use unevaluatedProps because it makes people's life > easier and is still correct. If it is not correct, then this should be > used as an argument. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-14 20:29 ` Laurent Pinchart @ 2024-10-15 6:11 ` Krzysztof Kozlowski 2024-10-15 11:28 ` Laurent Pinchart 0 siblings, 1 reply; 31+ messages in thread From: Krzysztof Kozlowski @ 2024-10-15 6:11 UTC (permalink / raw) To: Laurent Pinchart Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 14/10/2024 22:29, Laurent Pinchart wrote: > On Mon, Oct 14, 2024 at 10:47:31AM +0200, Krzysztof Kozlowski wrote: >> On 14/10/2024 10:31, Bryan O'Donoghue wrote: >>> On 14/10/2024 08:45, Krzysztof Kozlowski wrote: >>>> I do not understand the reasoning behind this change at all. I don't >>>> think DT maintainers ever suggested it (in fact, rather opposite: >>>> suggested using unevaluatedProps) and I think is not a consensus of any >>>> talks. >>> >>> No there is not but then, how do you give consistent feedback except >>> proposing something to be a baseline. >>> >>> On the one hand you have upstream additionalProperties: false and >>> unevaluatedProperites: false - it'd be better to have a consistent >>> message on which is to be used. >> >> Well, I am afraid that push towards additionalProps will lead to grow >> common schema (video-interface-devices or video-interfaces) into huge >> one-fit-all binding. And that's not good. >> >> If a common binding for a group of devices encourages you to list its >> subset, then it is not that common. >> >> Solution is to fix that, e.g. split it per classes of devices. > > I think splitting large schemas per class is a good idea, but the > problem will still exist. For instance, if we were to move the > CSI-2-specific properties to a separate schema, that schema would define > clock-lanes, data-lanes and clock-noncontinuous. The clock-lanes and > clock-noncontinuous properties do not apply to every device, how would > we then handle that ? I see three options: Why is this a problem? Why is this a problem here, but not in other subsystems having exactly the same case? Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-15 6:11 ` Krzysztof Kozlowski @ 2024-10-15 11:28 ` Laurent Pinchart 2024-10-15 11:51 ` Krzysztof Kozlowski 2024-10-15 19:44 ` Rob Herring 0 siblings, 2 replies; 31+ messages in thread From: Laurent Pinchart @ 2024-10-15 11:28 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel Hi Krzysztof, On Tue, Oct 15, 2024 at 08:11:18AM +0200, Krzysztof Kozlowski wrote: > On 14/10/2024 22:29, Laurent Pinchart wrote: > > On Mon, Oct 14, 2024 at 10:47:31AM +0200, Krzysztof Kozlowski wrote: > >> On 14/10/2024 10:31, Bryan O'Donoghue wrote: > >>> On 14/10/2024 08:45, Krzysztof Kozlowski wrote: > >>>> I do not understand the reasoning behind this change at all. I don't > >>>> think DT maintainers ever suggested it (in fact, rather opposite: > >>>> suggested using unevaluatedProps) and I think is not a consensus of any > >>>> talks. > >>> > >>> No there is not but then, how do you give consistent feedback except > >>> proposing something to be a baseline. > >>> > >>> On the one hand you have upstream additionalProperties: false and > >>> unevaluatedProperites: false - it'd be better to have a consistent > >>> message on which is to be used. > >> > >> Well, I am afraid that push towards additionalProps will lead to grow > >> common schema (video-interface-devices or video-interfaces) into huge > >> one-fit-all binding. And that's not good. > >> > >> If a common binding for a group of devices encourages you to list its > >> subset, then it is not that common. > >> > >> Solution is to fix that, e.g. split it per classes of devices. > > > > I think splitting large schemas per class is a good idea, but the > > problem will still exist. For instance, if we were to move the > > CSI-2-specific properties to a separate schema, that schema would define > > clock-lanes, data-lanes and clock-noncontinuous. The clock-lanes and > > clock-noncontinuous properties do not apply to every device, how would > > we then handle that ? I see three options: > > Why is this a problem? Why is this a problem here, but not in other > subsystems having exactly the same case? I won't talk for other subsystems, but I can say I see value in explicitly expressing what properties are valid for a device in DT bindings both to inform DT authors and to perform validation on DT sources. That's the whole point of YAML schemas, and I can't see a good reason not to use the tooling we have developed when it has an easy way to do the job. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-15 11:28 ` Laurent Pinchart @ 2024-10-15 11:51 ` Krzysztof Kozlowski 2024-10-16 9:27 ` Laurent Pinchart 2024-10-15 19:44 ` Rob Herring 1 sibling, 1 reply; 31+ messages in thread From: Krzysztof Kozlowski @ 2024-10-15 11:51 UTC (permalink / raw) To: Laurent Pinchart Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 15/10/2024 13:28, Laurent Pinchart wrote: > Hi Krzysztof, > > On Tue, Oct 15, 2024 at 08:11:18AM +0200, Krzysztof Kozlowski wrote: >> On 14/10/2024 22:29, Laurent Pinchart wrote: >>> On Mon, Oct 14, 2024 at 10:47:31AM +0200, Krzysztof Kozlowski wrote: >>>> On 14/10/2024 10:31, Bryan O'Donoghue wrote: >>>>> On 14/10/2024 08:45, Krzysztof Kozlowski wrote: >>>>>> I do not understand the reasoning behind this change at all. I don't >>>>>> think DT maintainers ever suggested it (in fact, rather opposite: >>>>>> suggested using unevaluatedProps) and I think is not a consensus of any >>>>>> talks. >>>>> >>>>> No there is not but then, how do you give consistent feedback except >>>>> proposing something to be a baseline. >>>>> >>>>> On the one hand you have upstream additionalProperties: false and >>>>> unevaluatedProperites: false - it'd be better to have a consistent >>>>> message on which is to be used. >>>> >>>> Well, I am afraid that push towards additionalProps will lead to grow >>>> common schema (video-interface-devices or video-interfaces) into huge >>>> one-fit-all binding. And that's not good. >>>> >>>> If a common binding for a group of devices encourages you to list its >>>> subset, then it is not that common. >>>> >>>> Solution is to fix that, e.g. split it per classes of devices. >>> >>> I think splitting large schemas per class is a good idea, but the >>> problem will still exist. For instance, if we were to move the >>> CSI-2-specific properties to a separate schema, that schema would define >>> clock-lanes, data-lanes and clock-noncontinuous. The clock-lanes and >>> clock-noncontinuous properties do not apply to every device, how would >>> we then handle that ? I see three options: >> >> Why is this a problem? Why is this a problem here, but not in other >> subsystems having exactly the same case? > > I won't talk for other subsystems, but I can say I see value in > explicitly expressing what properties are valid for a device in DT > bindings both to inform DT authors and to perform validation on DT > sources. That's the whole point of YAML schemas, and I can't see a good > reason not to use the tooling we have developed when it has an easy way > to do the job. I understand. The benefit, which you see, comes with complexity of the binding and need of listing properties. We do not enforce such rules (narrowing common schema in very strict way) in other subsystems, maybe with exception of input and touchscreen devices, but there common schema is quite big. And DT maintainers suggested to drop such code even for these, BTW. Best regards, Krzysztof ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-15 11:51 ` Krzysztof Kozlowski @ 2024-10-16 9:27 ` Laurent Pinchart 0 siblings, 0 replies; 31+ messages in thread From: Laurent Pinchart @ 2024-10-16 9:27 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Bryan O'Donoghue, Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel Hi Krzysztof, On Tue, Oct 15, 2024 at 01:51:43PM +0200, Krzysztof Kozlowski wrote: > On 15/10/2024 13:28, Laurent Pinchart wrote: > > On Tue, Oct 15, 2024 at 08:11:18AM +0200, Krzysztof Kozlowski wrote: > >> On 14/10/2024 22:29, Laurent Pinchart wrote: > >>> On Mon, Oct 14, 2024 at 10:47:31AM +0200, Krzysztof Kozlowski wrote: > >>>> On 14/10/2024 10:31, Bryan O'Donoghue wrote: > >>>>> On 14/10/2024 08:45, Krzysztof Kozlowski wrote: > >>>>>> I do not understand the reasoning behind this change at all. I don't > >>>>>> think DT maintainers ever suggested it (in fact, rather opposite: > >>>>>> suggested using unevaluatedProps) and I think is not a consensus of any > >>>>>> talks. > >>>>> > >>>>> No there is not but then, how do you give consistent feedback except > >>>>> proposing something to be a baseline. > >>>>> > >>>>> On the one hand you have upstream additionalProperties: false and > >>>>> unevaluatedProperites: false - it'd be better to have a consistent > >>>>> message on which is to be used. > >>>> > >>>> Well, I am afraid that push towards additionalProps will lead to grow > >>>> common schema (video-interface-devices or video-interfaces) into huge > >>>> one-fit-all binding. And that's not good. > >>>> > >>>> If a common binding for a group of devices encourages you to list its > >>>> subset, then it is not that common. > >>>> > >>>> Solution is to fix that, e.g. split it per classes of devices. > >>> > >>> I think splitting large schemas per class is a good idea, but the > >>> problem will still exist. For instance, if we were to move the > >>> CSI-2-specific properties to a separate schema, that schema would define > >>> clock-lanes, data-lanes and clock-noncontinuous. The clock-lanes and > >>> clock-noncontinuous properties do not apply to every device, how would > >>> we then handle that ? I see three options: > >> > >> Why is this a problem? Why is this a problem here, but not in other > >> subsystems having exactly the same case? > > > > I won't talk for other subsystems, but I can say I see value in > > explicitly expressing what properties are valid for a device in DT > > bindings both to inform DT authors and to perform validation on DT > > sources. That's the whole point of YAML schemas, and I can't see a good > > reason not to use the tooling we have developed when it has an easy way > > to do the job. > > I understand. The benefit, which you see, comes with complexity of the > binding and need of listing properties. I agree, the benefit comes at a cost of additional complexity in the bindings. For me the benefit outweights the cost here as I find the cost to be relatively small, but I understand that this is a personal opinion. > We do not enforce such rules (narrowing common schema in very strict > way) in other subsystems, maybe with exception of input and touchscreen > devices, but there common schema is quite big. And DT maintainers > suggested to drop such code even for these, BTW. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-15 11:28 ` Laurent Pinchart 2024-10-15 11:51 ` Krzysztof Kozlowski @ 2024-10-15 19:44 ` Rob Herring 2024-10-15 22:45 ` Bryan O'Donoghue 2024-10-16 9:31 ` Laurent Pinchart 1 sibling, 2 replies; 31+ messages in thread From: Rob Herring @ 2024-10-15 19:44 UTC (permalink / raw) To: Laurent Pinchart Cc: Krzysztof Kozlowski, Bryan O'Donoghue, Mauro Carvalho Chehab, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On Tue, Oct 15, 2024 at 02:28:06PM +0300, Laurent Pinchart wrote: > Hi Krzysztof, > > On Tue, Oct 15, 2024 at 08:11:18AM +0200, Krzysztof Kozlowski wrote: > > On 14/10/2024 22:29, Laurent Pinchart wrote: > > > On Mon, Oct 14, 2024 at 10:47:31AM +0200, Krzysztof Kozlowski wrote: > > >> On 14/10/2024 10:31, Bryan O'Donoghue wrote: > > >>> On 14/10/2024 08:45, Krzysztof Kozlowski wrote: > > >>>> I do not understand the reasoning behind this change at all. I don't > > >>>> think DT maintainers ever suggested it (in fact, rather opposite: > > >>>> suggested using unevaluatedProps) and I think is not a consensus of any > > >>>> talks. > > >>> > > >>> No there is not but then, how do you give consistent feedback except > > >>> proposing something to be a baseline. > > >>> > > >>> On the one hand you have upstream additionalProperties: false and > > >>> unevaluatedProperites: false - it'd be better to have a consistent > > >>> message on which is to be used. There are 3 options: - no $ref => additionalProperties - has a $ref: - additionalProperties and list ref-ed properties - unevaluatedProperties and don't list ref-ed properties I do debate (with myself) that that is too complicated as many don't understand the difference. We could go back to always using additionalProperties which is what we had before unevaluatedProperties was added. The other option is always use unevaluatedProperties. 2 things have stopped me from going that route. I don't care to fix 'additionalProperties' treewide which would be necessary to implement a meta-schema or check that unevaluatedProperties is used. It's not something I want to manually check in reviews. The other reason is just to not change what the rules are again. > > >> > > >> Well, I am afraid that push towards additionalProps will lead to grow > > >> common schema (video-interface-devices or video-interfaces) into huge > > >> one-fit-all binding. And that's not good. > > >> > > >> If a common binding for a group of devices encourages you to list its > > >> subset, then it is not that common. > > >> > > >> Solution is to fix that, e.g. split it per classes of devices. > > > > > > I think splitting large schemas per class is a good idea, but the > > > problem will still exist. For instance, if we were to move the > > > CSI-2-specific properties to a separate schema, that schema would define > > > clock-lanes, data-lanes and clock-noncontinuous. The clock-lanes and > > > clock-noncontinuous properties do not apply to every device, how would > > > we then handle that ? I see three options: > > > > Why is this a problem? Why is this a problem here, but not in other > > subsystems having exactly the same case? > > I won't talk for other subsystems, but I can say I see value in > explicitly expressing what properties are valid for a device in DT > bindings both to inform DT authors and to perform validation on DT > sources. That's the whole point of YAML schemas, and I can't see a good > reason not to use the tooling we have developed when it has an easy way > to do the job. This topic is just one piece of validation. A property being used that's defined, but meaningless for a device is low on the list of what I care about validating. I can't see how it would cause an actual problem? A driver is going to read the property and do what with it? Could it be an ABI issue ever? I can't see how other than a driver failing for some reason if it finds the property, but that seems a bit far fetched. Rob ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-15 19:44 ` Rob Herring @ 2024-10-15 22:45 ` Bryan O'Donoghue 2024-10-16 9:31 ` Laurent Pinchart 1 sibling, 0 replies; 31+ messages in thread From: Bryan O'Donoghue @ 2024-10-15 22:45 UTC (permalink / raw) To: Rob Herring, Laurent Pinchart Cc: Krzysztof Kozlowski, Mauro Carvalho Chehab, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel On 15/10/2024 20:44, Rob Herring wrote: > On Tue, Oct 15, 2024 at 02:28:06PM +0300, Laurent Pinchart wrote: >> Hi Krzysztof, >> >> On Tue, Oct 15, 2024 at 08:11:18AM +0200, Krzysztof Kozlowski wrote: >>> On 14/10/2024 22:29, Laurent Pinchart wrote: >>>> On Mon, Oct 14, 2024 at 10:47:31AM +0200, Krzysztof Kozlowski wrote: >>>>> On 14/10/2024 10:31, Bryan O'Donoghue wrote: >>>>>> On 14/10/2024 08:45, Krzysztof Kozlowski wrote: >>>>>>> I do not understand the reasoning behind this change at all. I don't >>>>>>> think DT maintainers ever suggested it (in fact, rather opposite: >>>>>>> suggested using unevaluatedProps) and I think is not a consensus of any >>>>>>> talks. >>>>>> >>>>>> No there is not but then, how do you give consistent feedback except >>>>>> proposing something to be a baseline. >>>>>> >>>>>> On the one hand you have upstream additionalProperties: false and >>>>>> unevaluatedProperites: false - it'd be better to have a consistent >>>>>> message on which is to be used. > > There are 3 options: > > - no $ref => additionalProperties I interpret this to mean that omitting additionalProperties/unevaluatedProperties is logically the same as having additionalProperties as you will then need to list the properties explicitly. > - has a $ref: > - additionalProperties and list ref-ed properties > - unevaluatedProperties and don't list ref-ed properties > > I do debate (with myself) that that is too complicated as many don't > understand the difference. We could go back to always using > additionalProperties which is what we had before unevaluatedProperties > was added. The other option is always use unevaluatedProperties. 2 > things have stopped me from going that route. I don't care to fix > 'additionalProperties' treewide which would be necessary to implement a > meta-schema or check that unevaluatedProperties is used. It's not > something I want to manually check in reviews. The other reason is just > to not change what the rules are again. Right so I received feedback to change link-frequencies if I recall. I thought I had been very-clever (tm) by copying an upstream source and when I received feedback to change assumed the upstream source I had copied had bit-rot w/r/t the current preferred way. Some additional discussion shows there really isn't a preferred way at present. Is there a place to meaningfully document that conclusion instead both for reviewers and implementers ? Should I already know the answer to that question ? --- bod ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: 2024-10-15 19:44 ` Rob Herring 2024-10-15 22:45 ` Bryan O'Donoghue @ 2024-10-16 9:31 ` Laurent Pinchart 1 sibling, 0 replies; 31+ messages in thread From: Laurent Pinchart @ 2024-10-16 9:31 UTC (permalink / raw) To: Rob Herring Cc: Krzysztof Kozlowski, Bryan O'Donoghue, Mauro Carvalho Chehab, Krzysztof Kozlowski, Conor Dooley, Dave Stevenson, Sakari Ailus, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Martin Kepplinger, Paul Kocialkowski, Paul J. Murphy, Daniele Alessandrelli, Tommaso Merciai, Martin Hecht, Zhi Mao, Alain Volmat, Mikhail Rudenko, Ricardo Ribalda, Kieran Bingham, Umang Jain, Manivannan Sadhasivam, Vladimir Zapolskiy, Dongchun Zhu, Quentin Schulz, Todor Tomov, linux-media, devicetree, linux-kernel, imx, linux-arm-kernel Hi Rob, On Tue, Oct 15, 2024 at 02:44:18PM -0500, Rob Herring wrote: > On Tue, Oct 15, 2024 at 02:28:06PM +0300, Laurent Pinchart wrote: > > On Tue, Oct 15, 2024 at 08:11:18AM +0200, Krzysztof Kozlowski wrote: > > > On 14/10/2024 22:29, Laurent Pinchart wrote: > > > > On Mon, Oct 14, 2024 at 10:47:31AM +0200, Krzysztof Kozlowski wrote: > > > >> On 14/10/2024 10:31, Bryan O'Donoghue wrote: > > > >>> On 14/10/2024 08:45, Krzysztof Kozlowski wrote: > > > >>>> I do not understand the reasoning behind this change at all. I don't > > > >>>> think DT maintainers ever suggested it (in fact, rather opposite: > > > >>>> suggested using unevaluatedProps) and I think is not a consensus of any > > > >>>> talks. > > > >>> > > > >>> No there is not but then, how do you give consistent feedback except > > > >>> proposing something to be a baseline. > > > >>> > > > >>> On the one hand you have upstream additionalProperties: false and > > > >>> unevaluatedProperites: false - it'd be better to have a consistent > > > >>> message on which is to be used. > > There are 3 options: > > - no $ref => additionalProperties > - has a $ref: > - additionalProperties and list ref-ed properties > - unevaluatedProperties and don't list ref-ed properties > > I do debate (with myself) Those are the best and worst debates at the same time :-) > that that is too complicated as many don't > understand the difference. We could go back to always using > additionalProperties which is what we had before unevaluatedProperties > was added. The other option is always use unevaluatedProperties. 2 > things have stopped me from going that route. I don't care to fix > 'additionalProperties' treewide which would be necessary to implement a > meta-schema or check that unevaluatedProperties is used. It's not > something I want to manually check in reviews. The other reason is just > to not change what the rules are again. > > > > >> > > > >> Well, I am afraid that push towards additionalProps will lead to grow > > > >> common schema (video-interface-devices or video-interfaces) into huge > > > >> one-fit-all binding. And that's not good. > > > >> > > > >> If a common binding for a group of devices encourages you to list its > > > >> subset, then it is not that common. > > > >> > > > >> Solution is to fix that, e.g. split it per classes of devices. > > > > > > > > I think splitting large schemas per class is a good idea, but the > > > > problem will still exist. For instance, if we were to move the > > > > CSI-2-specific properties to a separate schema, that schema would define > > > > clock-lanes, data-lanes and clock-noncontinuous. The clock-lanes and > > > > clock-noncontinuous properties do not apply to every device, how would > > > > we then handle that ? I see three options: > > > > > > Why is this a problem? Why is this a problem here, but not in other > > > subsystems having exactly the same case? > > > > I won't talk for other subsystems, but I can say I see value in > > explicitly expressing what properties are valid for a device in DT > > bindings both to inform DT authors and to perform validation on DT > > sources. That's the whole point of YAML schemas, and I can't see a good > > reason not to use the tooling we have developed when it has an easy way > > to do the job. > > This topic is just one piece of validation. A property being used that's > defined, but meaningless for a device is low on the list of what I care > about validating. I can't see how it would cause an actual problem? A > driver is going to read the property and do what with it? Could it be an > ABI issue ever? I can't see how other than a driver failing for some > reason if it finds the property, but that seems a bit far fetched. I agree the risk of issues at runtime is quite low. My personal take on this is that the additional complexity of specifying "$prop: true" in the bindings is low (for me at least), and the increased correctness in DT sources to avoid confusion for DT readers is worth it. I also like how more explicit bindings cleary show in a single place what properties are expected, making it easier for DT authors. That's a personal opinion though. -- Regards, Laurent Pinchart ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2024-10-16 9:42 UTC | newest] Thread overview: 31+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-12 15:02 [PATCH 0/2] media: i2c: Cleanup assigned-clocks and endpoint: properties: unevaluatedProperties: false Bryan O'Donoghue 2024-10-12 15:02 ` [PATCH 1/2] media: dt-bindings: Remove assigned-clock-* from various schema Bryan O'Donoghue 2024-10-12 17:24 ` Laurent Pinchart 2024-10-14 7:43 ` Krzysztof Kozlowski 2024-10-14 8:29 ` Bryan O'Donoghue 2024-10-14 8:44 ` Krzysztof Kozlowski 2024-10-14 10:08 ` Bryan O'Donoghue 2024-10-14 10:20 ` Sakari Ailus 2024-10-14 10:34 ` Krzysztof Kozlowski 2024-10-14 10:46 ` Sakari Ailus 2024-10-14 20:34 ` Laurent Pinchart 2024-10-15 6:13 ` Krzysztof Kozlowski 2024-10-15 11:29 ` Laurent Pinchart 2024-10-15 16:46 ` Rob Herring 2024-10-16 7:47 ` Krzysztof Kozlowski 2024-10-12 15:02 ` [PATCH 2/2] media: dt-bindings: Use additionalProperties: false for endpoint: properties: Bryan O'Donoghue 2024-10-12 18:09 ` Laurent Pinchart 2024-10-13 10:44 ` Bryan O'Donoghue 2024-10-14 7:45 ` Krzysztof Kozlowski 2024-10-14 8:31 ` Bryan O'Donoghue 2024-10-14 8:47 ` Krzysztof Kozlowski 2024-10-14 9:03 ` Bryan O'Donoghue 2024-10-14 10:37 ` Krzysztof Kozlowski 2024-10-14 20:29 ` Laurent Pinchart 2024-10-15 6:11 ` Krzysztof Kozlowski 2024-10-15 11:28 ` Laurent Pinchart 2024-10-15 11:51 ` Krzysztof Kozlowski 2024-10-16 9:27 ` Laurent Pinchart 2024-10-15 19:44 ` Rob Herring 2024-10-15 22:45 ` Bryan O'Donoghue 2024-10-16 9:31 ` Laurent Pinchart
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).