We currently do not have an upstream user of the x1e CAMSS schema which
allows us to make this the first platform to treat the CSI PHYs as separate
devices in much the same way as we treat the CCI block as separate devices.
Convert the embedded CSIPHY node data to simple phys = <> removing all of
the PHY specific stuff previously embedded.
I gave some serious thought to making the Test Pattern Generators TPGs into
PHY nodes also but, unlike the CSIPHYs the TPGs have no dedicated external
pins nor regulators.
The CSIPHYs OTOH have dedicated in-fact generally unmuxed pins on Qualcomm
SoCs and each CSIPHY has its own set of input power rails usually 0p8 and
1p2.
Instead of defining the CSIPHYs as children of the CAMSS block, we take the
same approach as the CCI/I2C bus dedicated to CAMSS and define the CSIPHYs
as their own nodes.
Remove the embedded CSIPHY specific data and give CAMSS regular,
bog-standard phys = <>;
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
.../bindings/media/qcom,x1e80100-camss.yaml | 80 +++++-----------------
1 file changed, 16 insertions(+), 64 deletions(-)
diff --git a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
index 2438e08b894f4a3dc577cee4ab85184a3d7232b0..c130733887e39afe51f3d2df2a5c943c6fc2ca9f 100644
--- a/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
+++ b/Documentation/devicetree/bindings/media/qcom,x1e80100-camss.yaml
@@ -17,7 +17,7 @@ properties:
const: qcom,x1e80100-camss
reg:
- maxItems: 17
+ maxItems: 13
reg-names:
items:
@@ -27,10 +27,6 @@ properties:
- const: csid2
- const: csid_lite0
- const: csid_lite1
- - const: csiphy0
- - const: csiphy1
- - const: csiphy2
- - const: csiphy4
- const: csitpg0
- const: csitpg1
- const: csitpg2
@@ -40,7 +36,7 @@ properties:
- const: vfe_lite1
clocks:
- maxItems: 29
+ maxItems: 21
clock-names:
items:
@@ -55,14 +51,6 @@ properties:
- const: cphy_rx_clk_src
- const: csid
- const: csid_csiphy_rx
- - const: csiphy0
- - const: csiphy0_timer
- - const: csiphy1
- - const: csiphy1_timer
- - const: csiphy2
- - const: csiphy2_timer
- - const: csiphy4
- - const: csiphy4_timer
- const: gcc_axi_hf
- const: gcc_axi_sf
- const: vfe0
@@ -75,7 +63,7 @@ properties:
- const: vfe_lite_csid
interrupts:
- maxItems: 13
+ maxItems: 9
interrupt-names:
items:
@@ -84,15 +72,17 @@ properties:
- const: csid2
- const: csid_lite0
- const: csid_lite1
- - const: csiphy0
- - const: csiphy1
- - const: csiphy2
- - const: csiphy4
- const: vfe0
- const: vfe1
- const: vfe_lite0
- const: vfe_lite1
+ phys:
+ maxItems: 4
+
+ phy-names:
+ maxItems: 4
+
interconnects:
maxItems: 4
@@ -118,14 +108,6 @@ properties:
- const: ife1
- const: top
- vdd-csiphy-0p8-supply:
- description:
- Phandle to a 0.8V regulator supply to a PHY.
-
- vdd-csiphy-1p2-supply:
- description:
- Phandle to 1.8V regulator supply to a PHY.
-
ports:
$ref: /schemas/graph.yaml#/properties/ports
@@ -166,13 +148,13 @@ required:
- clock-names
- interrupts
- interrupt-names
+ - phys
+ - phy-names
- interconnects
- interconnect-names
- iommus
- power-domains
- power-domain-names
- - vdd-csiphy-0p8-supply
- - vdd-csiphy-1p2-supply
- ports
additionalProperties: false
@@ -199,10 +181,6 @@ examples:
<0 0x0acbb000 0 0x2000>,
<0 0x0acc6000 0 0x1000>,
<0 0x0acca000 0 0x1000>,
- <0 0x0ace4000 0 0x1000>,
- <0 0x0ace6000 0 0x1000>,
- <0 0x0ace8000 0 0x1000>,
- <0 0x0acec000 0 0x4000>,
<0 0x0acf6000 0 0x1000>,
<0 0x0acf7000 0 0x1000>,
<0 0x0acf8000 0 0x1000>,
@@ -217,10 +195,6 @@ examples:
"csid2",
"csid_lite0",
"csid_lite1",
- "csiphy0",
- "csiphy1",
- "csiphy2",
- "csiphy4",
"csitpg0",
"csitpg1",
"csitpg2",
@@ -240,14 +214,6 @@ examples:
<&camcc CAM_CC_CPHY_RX_CLK_SRC>,
<&camcc CAM_CC_CSID_CLK>,
<&camcc CAM_CC_CSID_CSIPHY_RX_CLK>,
- <&camcc CAM_CC_CSIPHY0_CLK>,
- <&camcc CAM_CC_CSI0PHYTIMER_CLK>,
- <&camcc CAM_CC_CSIPHY1_CLK>,
- <&camcc CAM_CC_CSI1PHYTIMER_CLK>,
- <&camcc CAM_CC_CSIPHY2_CLK>,
- <&camcc CAM_CC_CSI2PHYTIMER_CLK>,
- <&camcc CAM_CC_CSIPHY4_CLK>,
- <&camcc CAM_CC_CSI4PHYTIMER_CLK>,
<&gcc GCC_CAMERA_HF_AXI_CLK>,
<&gcc GCC_CAMERA_SF_AXI_CLK>,
<&camcc CAM_CC_IFE_0_CLK>,
@@ -270,14 +236,6 @@ examples:
"cphy_rx_clk_src",
"csid",
"csid_csiphy_rx",
- "csiphy0",
- "csiphy0_timer",
- "csiphy1",
- "csiphy1_timer",
- "csiphy2",
- "csiphy2_timer",
- "csiphy4",
- "csiphy4_timer",
"gcc_axi_hf",
"gcc_axi_sf",
"vfe0",
@@ -294,10 +252,6 @@ examples:
<GIC_SPI 431 IRQ_TYPE_EDGE_RISING>,
<GIC_SPI 468 IRQ_TYPE_EDGE_RISING>,
<GIC_SPI 359 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 478 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 479 IRQ_TYPE_EDGE_RISING>,
- <GIC_SPI 122 IRQ_TYPE_EDGE_RISING>,
<GIC_SPI 465 IRQ_TYPE_EDGE_RISING>,
<GIC_SPI 467 IRQ_TYPE_EDGE_RISING>,
<GIC_SPI 469 IRQ_TYPE_EDGE_RISING>,
@@ -308,15 +262,16 @@ examples:
"csid2",
"csid_lite0",
"csid_lite1",
- "csiphy0",
- "csiphy1",
- "csiphy2",
- "csiphy4",
"vfe0",
"vfe1",
"vfe_lite0",
"vfe_lite1";
+ phys = <&csiphy0>, <&csiphy1>,
+ <&csiphy2>, <&csiphy4>;
+ phy-names = "csiphy0", "csiphy1",
+ "csiphy2", "csiphy4";
+
interconnects = <&gem_noc MASTER_APPSS_PROC QCOM_ICC_TAG_ACTIVE_ONLY
&config_noc SLAVE_CAMERA_CFG QCOM_ICC_TAG_ACTIVE_ONLY>,
<&mmss_noc MASTER_CAMNOC_HF QCOM_ICC_TAG_ALWAYS
@@ -348,9 +303,6 @@ examples:
"ife1",
"top";
- vdd-csiphy-0p8-supply = <&csiphy_0p8_supply>;
- vdd-csiphy-1p2-supply = <&csiphy_1p2_supply>;
-
ports {
#address-cells = <1>;
#size-cells = <0>;
--
2.49.0On 11/07/2025 14:57, Bryan O'Donoghue wrote: > We currently do not have an upstream user of the x1e CAMSS schema which On first glance there is, in Linus tree: git grep qcom,x1e80100-camss drivers/media/platform/qcom/camss/camss.c If this wasn't released mention it. Best regards, Krzysztof
On 13/07/2025 10:18, Krzysztof Kozlowski wrote: > On 11/07/2025 14:57, Bryan O'Donoghue wrote: >> We currently do not have an upstream user of the x1e CAMSS schema which > > On first glance there is, in Linus tree: > > git grep qcom,x1e80100-camss > drivers/media/platform/qcom/camss/camss.c > > If this wasn't released mention it. ... and then this should be marked as fixes and picked up fast, because you have only like 2 weeks to fix it. Best regards, Krzysztof
On 13/07/2025 09:20, Krzysztof Kozlowski wrote: > On 13/07/2025 10:18, Krzysztof Kozlowski wrote: >> On 11/07/2025 14:57, Bryan O'Donoghue wrote: >>> We currently do not have an upstream user of the x1e CAMSS schema which >> >> On first glance there is, in Linus tree: >> >> git grep qcom,x1e80100-camss >> drivers/media/platform/qcom/camss/camss.c >> >> If this wasn't released mention it. > ... and then this should be marked as fixes and picked up fast, because > you have only like 2 weeks to fix it. > > Best regards, > Krzysztof I thought schema changes were acceptable so long as we haven't applied dts, which we haven't done yet. --- bod
On 13/07/2025 11:14, Bryan O'Donoghue wrote: > On 13/07/2025 09:20, Krzysztof Kozlowski wrote: >> On 13/07/2025 10:18, Krzysztof Kozlowski wrote: >>> On 11/07/2025 14:57, Bryan O'Donoghue wrote: >>>> We currently do not have an upstream user of the x1e CAMSS schema which >>> >>> On first glance there is, in Linus tree: >>> >>> git grep qcom,x1e80100-camss >>> drivers/media/platform/qcom/camss/camss.c >>> >>> If this wasn't released mention it. >> ... and then this should be marked as fixes and picked up fast, because >> you have only like 2 weeks to fix it. >> >> Best regards, >> Krzysztof > > I thought schema changes were acceptable so long as we haven't applied > dts, which we haven't done yet. Accepted DTS is just one story, but following your argumentation that docs do not define ABI break, then accepted DTS also does not matter, because it is always in the kernel sources synced with the ABI. Following your argument about "accepted DTS", what is different between: 1. accepted DTS, then changed DT binding and changed DTS, 2. not accepted DTS and changed DT binding? Why can't you accept DTS and then change it? Lack of in-kernel DTS is a good argument in your case, but you must mention ALL OTHER USERS: 1. All drivers in Linux 2. All other upstream projects, BSD, U-boot, everywhere upstream 3. ... all possible other users of the ABI, so out of tree DTS and out of tree kernel folks. This one is close to impossible to prove... Luckily we assume this point does not apply here at all. No one out of upstream trees uses these new bindings. Best regards, Krzysztof
On 13/07/2025 10:39, Krzysztof Kozlowski wrote: > Lack of in-kernel DTS is a good argument in your case, but you must > mention ALL OTHER USERS: > 1. All drivers in Linux > 2. All other upstream projects, BSD, U-boot, everywhere upstream > 3. ... all possible other users of the ABI, so out of tree DTS and out > of tree kernel folks. This one is close to impossible to prove... > Luckily we assume this point does not apply here at all. No one out of > upstream trees uses these new bindings. OK, if I get your meaning here. More comprehensive commit logs and cover letter - including doing to the work to look into BSD and u-boot to make sure the change doesn't break things for them, which I'm nearly 100% sure is the case, is what you need here ? I'm fairly sure the only real dependencies here are people following my development branch for the x1e laptops, who want to drop those patches and receive upstream stuff from -next/-stable. --- bod
On 13/07/2025 11:48, Bryan O'Donoghue wrote: > On 13/07/2025 10:39, Krzysztof Kozlowski wrote: >> Lack of in-kernel DTS is a good argument in your case, but you must >> mention ALL OTHER USERS: >> 1. All drivers in Linux >> 2. All other upstream projects, BSD, U-boot, everywhere upstream >> 3. ... all possible other users of the ABI, so out of tree DTS and out >> of tree kernel folks. This one is close to impossible to prove... >> Luckily we assume this point does not apply here at all. No one out of >> upstream trees uses these new bindings. > > OK, if I get your meaning here. > > More comprehensive commit logs and cover letter - including doing to the > work to look into BSD and u-boot to make sure the change doesn't break > things for them, which I'm nearly 100% sure is the case, is what you > need here ? Yes. Just say that ABI change has no known impact because of this and that. Best regards, Krzysztof
© 2016 - 2025 Red Hat, Inc.