Re: [PATCH v4] dt-bindings: display: mediatek: split: add subschema property constraints
On Fri, Oct 04, 2024 at 11:12:45AM +0800, Moudy Ho wrote:
> The display node in mt8195.dtsi was triggering a CHECK_DTBS error due
> to an excessively long 'clocks' property:
> display@14f06000: clocks: [[31, 14], [31, 43], [31, 44]] is too long
>
> To resolve this issue, the constraints for 'clocks' and
> other properties within the subschema will be reinforced.
>
> Fixes: 739058a9c5c3 ("dt-bindings: display: mediatek: split: add compatible
> for MT8195")
> Signed-off-by: Macpaul Lin
> Signed-off-by: Moudy Ho
>
> --
> This is based on [v2] dt-bindings: display: mediatek: split: add clocks count
> constraint for MT8195
>
> Changes since v3:
> - Correct the compatible name for the mt8173 split in the subschema.
>
> Changes since v2:
> - Revise the commit message.
> - Enhance the descriptions of 'clocks'.
> - Strengthen the conditions within the subschema.
>
> Changes since v1:
> - Adding functional descriptions and quantity restrictions.
> ---
> .../display/mediatek/mediatek,split.yaml | 24 +++
> 1 file changed, 24 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> index e4affc854f3d..87f8477a7be8 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> @@ -57,6 +57,9 @@ properties:
>clocks:
> items:
>- description: SPLIT Clock
> + - description: Used for interfacing with the HDMI RX signal source.
> + - description: Paired with receiving HDMI RX metadata.
> +minItems: 1
>
> required:
>- compatible
> @@ -72,9 +75,30 @@ allOf:
> const: mediatek,mt8195-mdp3-split
>
> then:
> + properties:
> +clocks:
minItems, nothing in your commit msg says that clocks are optional
> + maxItems: 3
> +
> +power-domains:
> + maxItems: 1
This should be in top-level, not here.
Best regards,
Krzysztof
[PATCH 2/2] drm/bridge: tc358768: switch to bus-width
"data-lines" property is way too similar to "data-lanes". It is also
duplicating "bus-width" from video-interfaces.yaml schema. "data-lines"
was deprecated in the bindings and "bus-width" is preferred, so parse it
instead while keeping things backwards compatible.
Signed-off-by: Krzysztof Kozlowski
---
drivers/gpu/drm/bridge/tc358768.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/tc358768.c
b/drivers/gpu/drm/bridge/tc358768.c
index 0e8813278a2f..fc96fa5aab54 100644
--- a/drivers/gpu/drm/bridge/tc358768.c
+++ b/drivers/gpu/drm/bridge/tc358768.c
@@ -443,7 +443,9 @@ static int tc358768_dsi_host_attach(struct mipi_dsi_host
*host,
ret = -EINVAL;
ep = of_graph_get_endpoint_by_regs(host->dev->of_node, 0, 0);
if (ep) {
- ret = of_property_read_u32(ep, "data-lines", &priv->pd_lines);
+ ret = of_property_read_u32(ep, "bus-width", &priv->pd_lines);
+ if (ret)
+ ret = of_property_read_u32(ep, "data-lines",
&priv->pd_lines);
of_node_put(ep);
}
--
2.43.0
[PATCH 1/2] dt-bindings: display: bridge: tc358768: switch to bus-width
"data-lines" property is way too similar to "data-lanes". It is also duplicating "bus-width" from video-interfaces.yaml schema. Deprecate "data-lines" and use the common property. Signed-off-by: Krzysztof Kozlowski --- .../devicetree/bindings/display/bridge/toshiba,tc358768.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml index 779d8c57f854..bb5d3b543800 100644 --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358768.yaml @@ -60,6 +60,10 @@ properties: data-lines: $ref: /schemas/types.yaml#/definitions/uint32 enum: [ 16, 18, 24 ] +deprecated: true + + bus-width: +enum: [ 16, 18, 24 ] port@1: $ref: /schemas/graph.yaml#/properties/port -- 2.43.0
Re: [PATCH 2/2] dt-bindings: display: bridge: sil,sii9022: Add data-lines
On 03/10/2024 13:56, Wadim Egorov wrote: > > > Am 03.10.24 um 12:03 schrieb Krzysztof Kozlowski: >> On 03/10/2024 10:20, Wadim Egorov wrote: >>> The SI9022 HDMI transmitter can be configured with 16, 18, or 24 input >>> data lines. This commit introduces the data-lines property to the input >> >> lines? lanes? What are lines? like pins? > > Yes, "lines" in this context refers to the number of pins used for the > input pixel data bus, which can support 16, 18, or 24-bit wide data > buses. These are parallel data lines (or pins) that carry uncompressed > digital video to the HDMI transmitter. > >> >>> endpoint, specifying the number of parallel RGB input pins connected >>> to the transmitter. >>> >>> Signed-off-by: Wadim Egorov >>> --- >>> .../bindings/display/bridge/sil,sii9022.yaml| 13 - >>> 1 file changed, 12 insertions(+), 1 deletion(-) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml >>> b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml >>> index 5a69547ad3d7..24306f8eb107 100644 >>> --- a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml >>> +++ b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml >>> @@ -81,9 +81,20 @@ properties: >>> >>> properties: >>> port@0: >>> -$ref: /schemas/graph.yaml#/properties/port >>> +unevaluatedProperties: false >>> +$ref: /schemas/graph.yaml#/$defs/port-base >>> description: Parallel RGB input port >>> >>> +properties: >>> + endpoint: >>> +$ref: /schemas/graph.yaml#/$defs/endpoint-base >>> +unevaluatedProperties: false >>> + >>> +properties: >>> + data-lines: >> >> No, this will confuse everyone. Considering lack of description how >> anyone would figure out what this means? > > I guess from working with the hardware/reference manual and using this chip? > > I don't think it is overly confusing, especially since the port is > already described as the "Parallel RGB input port" which clearly implies > the use of pins for data transmission. I am surprised you do not find data-lanes and data-lines confusing. For non-native English speakers this even might sound the same. You used earlier pins and bits, so maybe it's the same as bus-width, which is already used all over the bindings, including one of the bridges. Anyway a generic property should go to video-interfaces. > > I am open to other suggestions if you believe a different name would > improve clarity. > > Btw, bridge/toshiba,tc358768.yaml, which performs a similar function, > also uses the term data-lines. Then this has to go to common schema. Oh, wait, video-interfaces already have it! Best regards, Krzysztof
Re: [PATCH v2] dt-bindings: display: mediatek: split: add clocks count constraint for MT8195
On 30/09/2024 05:28, Moudy Ho (何宗原) wrote: >>> required: >>>- compatible >>> @@ -72,6 +75,9 @@ allOf: >>> const: mediatek,mt8195-mdp3-split >>> >>> then: >>> + properties: >>> +clocks: >> >> missing minItems >> >> Missing constraints for all the variants. >> > > Does this mean that a 'maxItems:1' condition needs to be added for > mt8173 clock property under the 'allOf' seciton? This means that each variant must have clearly defined, fixed list of clocks (other properties as well) Best regards, Krzysztof
Re: [PATCH 2/2] dt-bindings: display: bridge: sil,sii9022: Add data-lines
On 03/10/2024 10:20, Wadim Egorov wrote: > The SI9022 HDMI transmitter can be configured with 16, 18, or 24 input > data lines. This commit introduces the data-lines property to the input lines? lanes? What are lines? like pins? > endpoint, specifying the number of parallel RGB input pins connected > to the transmitter. > > Signed-off-by: Wadim Egorov > --- > .../bindings/display/bridge/sil,sii9022.yaml| 13 - > 1 file changed, 12 insertions(+), 1 deletion(-) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml > b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml > index 5a69547ad3d7..24306f8eb107 100644 > --- a/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/sil,sii9022.yaml > @@ -81,9 +81,20 @@ properties: > > properties: >port@0: > -$ref: /schemas/graph.yaml#/properties/port > +unevaluatedProperties: false > +$ref: /schemas/graph.yaml#/$defs/port-base > description: Parallel RGB input port > > +properties: > + endpoint: > +$ref: /schemas/graph.yaml#/$defs/endpoint-base > +unevaluatedProperties: false > + > +properties: > + data-lines: No, this will confuse everyone. Considering lack of description how anyone would figure out what this means? I don't know much about RGB parallel port to advice how this should be called. Can you describe the hardware more? > +$ref: /schemas/types.yaml#/definitions/uint32 > +enum: [ 16, 18, 24 ] > + >port@1: > $ref: /schemas/graph.yaml#/properties/port > description: HDMI output port Best regards, Krzysztof
[PATCH 5/5] dt-bindings: display/msm: merge SM8550 DPU into SC7280
Split of the bindings was artificial and not helping - we end up with
multiple binding files for very similar devices thus increasing the
chances of using different order of reg and clocks entries.
Unify DPU bindings of SC7280 and SM8550, because they are the same.
Signed-off-by: Krzysztof Kozlowski
---
.../bindings/display/msm/qcom,sc7280-dpu.yaml | 2 +
.../bindings/display/msm/qcom,sm8550-dpu.yaml | 133 -
2 files changed, 2 insertions(+), 133 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
index 750230839fc9..6902795b4e2c 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
@@ -8,6 +8,7 @@ title: Qualcomm Display DPU on SC7280
maintainers:
- Bjorn Andersson
+ - Neil Armstrong
- Dmitry Baryshkov
- Krishna Manikandan
@@ -20,6 +21,7 @@ properties:
- qcom,sc8280xp-dpu
- qcom,sm8350-dpu
- qcom,sm8450-dpu
+ - qcom,sm8550-dpu
reg:
items:
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm8550-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sm8550-dpu.yaml
deleted file mode 100644
index 16a541fca66f..
--- a/Documentation/devicetree/bindings/display/msm/qcom,sm8550-dpu.yaml
+++ /dev/null
@@ -1,133 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
-%YAML 1.2
-$id: http://devicetree.org/schemas/display/msm/qcom,sm8550-dpu.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Qualcomm SM8550 Display DPU
-
-maintainers:
- - Neil Armstrong
-
-$ref: /schemas/display/msm/dpu-common.yaml#
-
-properties:
- compatible:
-const: qcom,sm8550-dpu
-
- reg:
-items:
- - description: Address offset and size for mdp register set
- - description: Address offset and size for vbif register set
-
- reg-names:
-items:
- - const: mdp
- - const: vbif
-
- clocks:
-items:
- - description: Display AHB
- - description: Display hf axi
- - description: Display MDSS ahb
- - description: Display lut
- - description: Display core
- - description: Display vsync
-
- clock-names:
-items:
- - const: bus
- - const: nrt_bus
- - const: iface
- - const: lut
- - const: core
- - const: vsync
-
-required:
- - compatible
- - reg
- - reg-names
- - clocks
- - clock-names
-
-unevaluatedProperties: false
-
-examples:
- - |
-#include
-#include
-#include
-#include
-
-display-controller@ae01000 {
-compatible = "qcom,sm8550-dpu";
-reg = <0x0ae01000 0x8f000>,
- <0x0aeb 0x2008>;
-reg-names = "mdp", "vbif";
-
-clocks = <&gcc GCC_DISP_AHB_CLK>,
-<&gcc GCC_DISP_HF_AXI_CLK>,
-<&dispcc DISP_CC_MDSS_AHB_CLK>,
-<&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
-<&dispcc DISP_CC_MDSS_MDP_CLK>,
-<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
-clock-names = "bus",
- "nrt_bus",
- "iface",
- "lut",
- "core",
- "vsync";
-
-assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
-assigned-clock-rates = <1920>;
-
-operating-points-v2 = <&mdp_opp_table>;
-power-domains = <&rpmhpd RPMHPD_MMCX>;
-
-interrupt-parent = <&mdss>;
-interrupts = <0>;
-
-ports {
-#address-cells = <1>;
-#size-cells = <0>;
-
-port@0 {
-reg = <0>;
-dpu_intf1_out: endpoint {
-remote-endpoint = <&dsi0_in>;
-};
-};
-
-port@1 {
-reg = <1>;
-dpu_intf2_out: endpoint {
-remote-endpoint = <&dsi1_in>;
-};
-};
-};
-
-mdp_opp_table: opp-table {
-compatible = "operating-points-v2";
-
-opp-2 {
-opp-hz = /bits/ 64 <2>;
-required-opps = <&rpmhpd_opp_low_svs>;
-};
-
-opp-32500 {
-opp-hz = /bits/ 64 <32500>;
-required-opps = <&rpmhpd_opp_svs>;
-};
-
-opp-37500 {
-opp-hz = /bits/ 64 <37500>;
-required-opps = <&rpmhpd_opp_svs_l1>;
-};
-
-opp-51400 {
-opp-hz = /bits/ 64 <51400>;
-required-opps = <&rpmhpd_opp_nom>;
-};
-};
-};
-...
--
2.43.0
[PATCH 1/5] dt-bindings: display/msm: merge SC8280XP DPU into SC7280
Split of the bindings was artificial and not helping - we end up with
multiple binding files for very similar devices thus increasing the
chances of using different order of reg and clocks entries.
Unify DPU bindings of SC7280 and SC8280XP, because they are the same.
Signed-off-by: Krzysztof Kozlowski
---
.../bindings/display/msm/qcom,sc7280-dpu.yaml | 5 +-
.../bindings/display/msm/qcom,sc8280xp-dpu.yaml| 122 -
2 files changed, 4 insertions(+), 123 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
index b0fbe86219d1..fab7a3b9a20e 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
@@ -7,13 +7,16 @@ $schema: http://devicetree.org/meta-schemas/core.yaml#
title: Qualcomm Display DPU on SC7280
maintainers:
+ - Bjorn Andersson
- Krishna Manikandan
$ref: /schemas/display/msm/dpu-common.yaml#
properties:
compatible:
-const: qcom,sc7280-dpu
+enum:
+ - qcom,sc7280-dpu
+ - qcom,sc8280xp-dpu
reg:
items:
diff --git
a/Documentation/devicetree/bindings/display/msm/qcom,sc8280xp-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sc8280xp-dpu.yaml
deleted file mode 100644
index d19e3bec4600..
--- a/Documentation/devicetree/bindings/display/msm/qcom,sc8280xp-dpu.yaml
+++ /dev/null
@@ -1,122 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
-%YAML 1.2
-$id: http://devicetree.org/schemas/display/msm/qcom,sc8280xp-dpu.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Qualcomm SC8280XP Display Processing Unit
-
-maintainers:
- - Bjorn Andersson
-
-description:
- Device tree bindings for SC8280XP Display Processing Unit.
-
-$ref: /schemas/display/msm/dpu-common.yaml#
-
-properties:
- compatible:
-const: qcom,sc8280xp-dpu
-
- reg:
-items:
- - description: Address offset and size for mdp register set
- - description: Address offset and size for vbif register set
-
- reg-names:
-items:
- - const: mdp
- - const: vbif
-
- clocks:
-items:
- - description: Display hf axi clock
- - description: Display sf axi clock
- - description: Display ahb clock
- - description: Display lut clock
- - description: Display core clock
- - description: Display vsync clock
-
- clock-names:
-items:
- - const: bus
- - const: nrt_bus
- - const: iface
- - const: lut
- - const: core
- - const: vsync
-
-unevaluatedProperties: false
-
-examples:
- - |
-#include
-#include
-#include
-#include
-#include
-
-display-controller@ae01000 {
-compatible = "qcom,sc8280xp-dpu";
-reg = <0x0ae01000 0x8f000>,
- <0x0aeb 0x2008>;
-reg-names = "mdp", "vbif";
-
-clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
- <&gcc GCC_DISP_SF_AXI_CLK>,
- <&dispcc0 DISP_CC_MDSS_AHB_CLK>,
- <&dispcc0 DISP_CC_MDSS_MDP_LUT_CLK>,
- <&dispcc0 DISP_CC_MDSS_MDP_CLK>,
- <&dispcc0 DISP_CC_MDSS_VSYNC_CLK>;
-clock-names = "bus",
- "nrt_bus",
- "iface",
- "lut",
- "core",
- "vsync";
-
-assigned-clocks = <&dispcc0 DISP_CC_MDSS_MDP_CLK>,
- <&dispcc0 DISP_CC_MDSS_VSYNC_CLK>;
-assigned-clock-rates = <46000>,
- <1920>;
-
-operating-points-v2 = <&mdp_opp_table>;
-power-domains = <&rpmhpd SC8280XP_MMCX>;
-
-interrupt-parent = <&mdss0>;
-interrupts = <0>;
-
-ports {
-#address-cells = <1>;
-#size-cells = <0>;
-
-port@0 {
-reg = <0>;
-endpoint {
-remote-endpoint = <&mdss0_dp0_in>;
-};
-};
-
-port@4 {
-reg = <4>;
-endpoint {
-remote-endpoint = <&mdss0_dp1_in>;
-};
-};
-
-port@5 {
-reg = <5>;
-endpoint {
-remote-endpoint = <&mdss0_dp3_in>;
-};
-};
-
-port@6 {
-reg = <6>;
-endpoint {
-remote-endpoint = <&mdss0_dp2_in>;
-};
-};
-};
-};
-...
--
2.43.0
[PATCH 4/5] dt-bindings: display/msm: merge SM8450 DPU into SC7280
Split of the bindings was artificial and not helping - we end up with
multiple binding files for very similar devices thus increasing the
chances of using different order of reg and clocks entries.
Unify DPU bindings of SC7280 and SM8450, because they are the same.
Signed-off-by: Krzysztof Kozlowski
---
.../bindings/display/msm/qcom,sc7280-dpu.yaml | 2 +
.../bindings/display/msm/qcom,sm8450-dpu.yaml | 139 -
2 files changed, 2 insertions(+), 139 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
index 3d69a573b450..750230839fc9 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
@@ -8,6 +8,7 @@ title: Qualcomm Display DPU on SC7280
maintainers:
- Bjorn Andersson
+ - Dmitry Baryshkov
- Krishna Manikandan
$ref: /schemas/display/msm/dpu-common.yaml#
@@ -18,6 +19,7 @@ properties:
- qcom,sc7280-dpu
- qcom,sc8280xp-dpu
- qcom,sm8350-dpu
+ - qcom,sm8450-dpu
reg:
items:
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm8450-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sm8450-dpu.yaml
deleted file mode 100644
index 2a5d3daed0e1..
--- a/Documentation/devicetree/bindings/display/msm/qcom,sm8450-dpu.yaml
+++ /dev/null
@@ -1,139 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
-%YAML 1.2
-$id: http://devicetree.org/schemas/display/msm/qcom,sm8450-dpu.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Qualcomm SM8450 Display DPU
-
-maintainers:
- - Dmitry Baryshkov
-
-$ref: /schemas/display/msm/dpu-common.yaml#
-
-properties:
- compatible:
-const: qcom,sm8450-dpu
-
- reg:
-items:
- - description: Address offset and size for mdp register set
- - description: Address offset and size for vbif register set
-
- reg-names:
-items:
- - const: mdp
- - const: vbif
-
- clocks:
-items:
- - description: Display hf axi
- - description: Display sf axi
- - description: Display ahb
- - description: Display lut
- - description: Display core
- - description: Display vsync
-
- clock-names:
-items:
- - const: bus
- - const: nrt_bus
- - const: iface
- - const: lut
- - const: core
- - const: vsync
-
-required:
- - compatible
- - reg
- - reg-names
- - clocks
- - clock-names
-
-unevaluatedProperties: false
-
-examples:
- - |
-#include
-#include
-#include
-#include
-#include
-
-display-controller@ae01000 {
-compatible = "qcom,sm8450-dpu";
-reg = <0x0ae01000 0x8f000>,
- <0x0aeb 0x2008>;
-reg-names = "mdp", "vbif";
-
-clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
-<&gcc GCC_DISP_SF_AXI_CLK>,
-<&dispcc DISP_CC_MDSS_AHB_CLK>,
-<&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
-<&dispcc DISP_CC_MDSS_MDP_CLK>,
-<&dispcc DISP_CC_MDSS_VSYNC_CLK>;
-clock-names = "bus",
- "nrt_bus",
- "iface",
- "lut",
- "core",
- "vsync";
-
-assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
-assigned-clock-rates = <1920>;
-
-operating-points-v2 = <&mdp_opp_table>;
-power-domains = <&rpmhpd RPMHPD_MMCX>;
-
-interrupt-parent = <&mdss>;
-interrupts = <0>;
-
-ports {
-#address-cells = <1>;
-#size-cells = <0>;
-
-port@0 {
-reg = <0>;
-dpu_intf1_out: endpoint {
-remote-endpoint = <&dsi0_in>;
-};
-};
-
-port@1 {
-reg = <1>;
-dpu_intf2_out: endpoint {
-remote-endpoint = <&dsi1_in>;
-};
-};
-};
-
-mdp_opp_table: opp-table {
-compatible = "operating-points-v2";
-
-opp-17200{
-opp-hz = /bits/ 64 <17200>;
-required-opps = <&rpmhpd_opp_low_svs_d1>;
-};
-
-opp-2 {
-opp-hz = /bits/ 64 <2>;
-required-opps = <&rpmhpd_opp_low_svs>;
-};
-
-opp-32500 {
-opp-hz = /bits/ 64 <32500>;
-required-opps = <&rpmhpd_opp_svs>;
-};
-
-opp-37500 {
-opp-hz = /bits/ 64 <37500>;
-required-opps = <&rpmhpd_opp_svs_l1>;
-};
-
-opp-5 {
-opp-hz = /bits/ 64 <5>;
-required-opps = <&rpmhpd_opp_nom>;
-};
-};
-};
-...
--
2.43.0
[PATCH 3/5] dt-bindings: display/msm: merge SM8350 DPU into SC7280
Split of the bindings was artificial and not helping - we end up with
multiple binding files for very similar devices thus increasing the
chances of using different order of reg and clocks entries.
Unify DPU bindings of SC7280 and SM8350, because they are the same.
Signed-off-by: Krzysztof Kozlowski
---
.../bindings/display/msm/qcom,sc7280-dpu.yaml | 1 +
.../bindings/display/msm/qcom,sm8350-dpu.yaml | 120 -
2 files changed, 1 insertion(+), 120 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
index fab7a3b9a20e..3d69a573b450 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml
@@ -17,6 +17,7 @@ properties:
enum:
- qcom,sc7280-dpu
- qcom,sc8280xp-dpu
+ - qcom,sm8350-dpu
reg:
items:
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm8350-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sm8350-dpu.yaml
deleted file mode 100644
index 96ef2d9c3512..
--- a/Documentation/devicetree/bindings/display/msm/qcom,sm8350-dpu.yaml
+++ /dev/null
@@ -1,120 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
-%YAML 1.2
-$id: http://devicetree.org/schemas/display/msm/qcom,sm8350-dpu.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Qualcomm SM8350 Display DPU
-
-maintainers:
- - Robert Foss
-
-$ref: /schemas/display/msm/dpu-common.yaml#
-
-properties:
- compatible:
-const: qcom,sm8350-dpu
-
- reg:
-items:
- - description: Address offset and size for mdp register set
- - description: Address offset and size for vbif register set
-
- reg-names:
-items:
- - const: mdp
- - const: vbif
-
- clocks:
-items:
- - description: Display hf axi clock
- - description: Display sf axi clock
- - description: Display ahb clock
- - description: Display lut clock
- - description: Display core clock
- - description: Display vsync clock
-
- clock-names:
-items:
- - const: bus
- - const: nrt_bus
- - const: iface
- - const: lut
- - const: core
- - const: vsync
-
-unevaluatedProperties: false
-
-examples:
- - |
-#include
-#include
-#include
-#include
-#include
-
-display-controller@ae01000 {
-compatible = "qcom,sm8350-dpu";
-reg = <0x0ae01000 0x8f000>,
- <0x0aeb 0x2008>;
-reg-names = "mdp", "vbif";
-
-clocks = <&gcc GCC_DISP_HF_AXI_CLK>,
- <&gcc GCC_DISP_SF_AXI_CLK>,
- <&dispcc DISP_CC_MDSS_AHB_CLK>,
- <&dispcc DISP_CC_MDSS_MDP_LUT_CLK>,
- <&dispcc DISP_CC_MDSS_MDP_CLK>,
- <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
-clock-names = "bus",
- "nrt_bus",
- "iface",
- "lut",
- "core",
- "vsync";
-
-assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
-assigned-clock-rates = <1920>;
-
-operating-points-v2 = <&mdp_opp_table>;
-power-domains = <&rpmhpd RPMHPD_MMCX>;
-
-interrupt-parent = <&mdss>;
-interrupts = <0>;
-
-ports {
-#address-cells = <1>;
-#size-cells = <0>;
-
-port@0 {
-reg = <0>;
-dpu_intf1_out: endpoint {
-remote-endpoint = <&dsi0_in>;
-};
-};
-};
-
-mdp_opp_table: opp-table {
-compatible = "operating-points-v2";
-
-opp-2 {
-opp-hz = /bits/ 64 <2>;
-required-opps = <&rpmhpd_opp_low_svs>;
-};
-
-opp-3 {
-opp-hz = /bits/ 64 <3>;
-required-opps = <&rpmhpd_opp_svs>;
-};
-
-opp-34500 {
-opp-hz = /bits/ 64 <34500>;
-required-opps = <&rpmhpd_opp_svs_l1>;
-};
-
-opp-46000 {
-opp-hz = /bits/ 64 <46000>;
-required-opps = <&rpmhpd_opp_nom>;
-};
-};
-};
-...
--
2.43.0
[PATCH 2/5] dt-bindings: display/msm: merge SM8250 DPU into SM8150
Split of the bindings was artificial and not helping - we end up with
multiple binding files for very similar devices thus increasing the
chances of using different order of reg and clocks entries.
Unify DPU bindings of SM8150 and SM8250, because they are the same.
Signed-off-by: Krzysztof Kozlowski
---
.../bindings/display/msm/qcom,sm8150-dpu.yaml | 4 +-
.../bindings/display/msm/qcom,sm8250-dpu.yaml | 99 --
2 files changed, 3 insertions(+), 100 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm8150-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sm8150-dpu.yaml
index 13146b3f053c..a88d22f30a60 100644
--- a/Documentation/devicetree/bindings/display/msm/qcom,sm8150-dpu.yaml
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8150-dpu.yaml
@@ -13,7 +13,9 @@ $ref: /schemas/display/msm/dpu-common.yaml#
properties:
compatible:
-const: qcom,sm8150-dpu
+enum:
+ - qcom,sm8150-dpu
+ - qcom,sm8250-dpu
reg:
items:
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm8250-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sm8250-dpu.yaml
deleted file mode 100644
index ffa5047e901f..
--- a/Documentation/devicetree/bindings/display/msm/qcom,sm8250-dpu.yaml
+++ /dev/null
@@ -1,99 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
-%YAML 1.2
-$id: http://devicetree.org/schemas/display/msm/qcom,sm8250-dpu.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Qualcomm SM8250 Display DPU
-
-maintainers:
- - Dmitry Baryshkov
-
-$ref: /schemas/display/msm/dpu-common.yaml#
-
-properties:
- compatible:
-const: qcom,sm8250-dpu
-
- reg:
-items:
- - description: Address offset and size for mdp register set
- - description: Address offset and size for vbif register set
-
- reg-names:
-items:
- - const: mdp
- - const: vbif
-
- clocks:
-items:
- - description: Display ahb clock
- - description: Display hf axi clock
- - description: Display core clock
- - description: Display vsync clock
-
- clock-names:
-items:
- - const: iface
- - const: bus
- - const: core
- - const: vsync
-
-required:
- - compatible
- - reg
- - reg-names
- - clocks
- - clock-names
-
-unevaluatedProperties: false
-
-examples:
- - |
-#include
-#include
-#include
-#include
-#include
-
-display-controller@ae01000 {
-compatible = "qcom,sm8250-dpu";
-reg = <0x0ae01000 0x8f000>,
- <0x0aeb 0x2008>;
-reg-names = "mdp", "vbif";
-
-clocks = <&dispcc DISP_CC_MDSS_AHB_CLK>,
- <&gcc GCC_DISP_HF_AXI_CLK>,
- <&dispcc DISP_CC_MDSS_MDP_CLK>,
- <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
-clock-names = "iface", "bus", "core", "vsync";
-
-assigned-clocks = <&dispcc DISP_CC_MDSS_VSYNC_CLK>;
-assigned-clock-rates = <1920>;
-
-operating-points-v2 = <&mdp_opp_table>;
-power-domains = <&rpmhpd RPMHPD_MMCX>;
-
-interrupt-parent = <&mdss>;
-interrupts = <0>;
-
-ports {
-#address-cells = <1>;
-#size-cells = <0>;
-
-port@0 {
-reg = <0>;
-endpoint {
-remote-endpoint = <&dsi0_in>;
-};
-};
-
-port@1 {
-reg = <1>;
-endpoint {
-remote-endpoint = <&dsi1_in>;
-};
-};
-};
-};
-...
--
2.43.0
[PATCH 0/5] dt-bindings: display/msm: dpu: merge bindings
In 2022 the bindings were split each device per file, but this makes not much sense. Devices are the same in many cases, same clocks, same IO register space. Best regards, Krzysztof --- Krzysztof Kozlowski (5): dt-bindings: display/msm: merge SC8280XP DPU into SC7280 dt-bindings: display/msm: merge SM8250 DPU into SM8150 dt-bindings: display/msm: merge SM8350 DPU into SC7280 dt-bindings: display/msm: merge SM8450 DPU into SC7280 dt-bindings: display/msm: merge SM8550 DPU into SC7280 .../bindings/display/msm/qcom,sc7280-dpu.yaml | 10 +- .../bindings/display/msm/qcom,sc8280xp-dpu.yaml| 122 -- .../bindings/display/msm/qcom,sm8150-dpu.yaml | 4 +- .../bindings/display/msm/qcom,sm8250-dpu.yaml | 99 --- .../bindings/display/msm/qcom,sm8350-dpu.yaml | 120 -- .../bindings/display/msm/qcom,sm8450-dpu.yaml | 139 - .../bindings/display/msm/qcom,sm8550-dpu.yaml | 133 7 files changed, 12 insertions(+), 615 deletions(-) --- base-commit: 77df9e4bb2224d8ffbddec04c333a9d7965dad6c change-id: 20241003-dt-binding-display-msm-merge-df230e26d142 Best regards, -- Krzysztof Kozlowski
Re: [PATCH v3 2/5] dt-bindings: display/msm: Document the DPU for SA8775P
On Tue, Oct 01, 2024 at 12:11:37PM +0530, Mahadevan wrote: > Document the DPU for Qualcomm SA8775P platform. > > Signed-off-by: Mahadevan > --- > .../bindings/display/msm/qcom,sa8775p-dpu.yaml | 122 > + > 1 file changed, 122 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-dpu.yaml > b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-dpu.yaml > new file mode 100644 > index > ..fda88bdbd04214e06255e105eae582ff926d72e9 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/msm/qcom,sa8775p-dpu.yaml > @@ -0,0 +1,122 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/msm/qcom,sa8775p-dpu.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Qualcomm Technologies, Inc. SA8775P Display DPU > + > +maintainers: > + - Mahadevan > + > +$ref: /schemas/display/msm/dpu-common.yaml# > + > +properties: > + compatible: > +const: qcom,sa8775p-dpu > + > + reg: > +items: > + - description: Address offset and size for mdp register set > + - description: Address offset and size for vbif register set > + > + reg-names: > +items: > + - const: mdp > + - const: vbif > + > + clocks: > +items: > + - description: Display hf AXI > + - description: Display AHB > + - description: Display lut > + - description: Display core > + - description: Display vsync > + > + clock-names: > +items: > + - const: bus > + - const: iface > + - const: lut > + - const: core > + - const: vsync This looks the same as sm8650-dpu. Add the compatible there. Best regards, Krzysztof
Re: [PATCH v3 1/5] dt-bindings: display/msm: Document MDSS on SA8775P
On Tue, Oct 01, 2024 at 12:11:36PM +0530, Mahadevan wrote: > +patternProperties: > + "^display-controller@[0-9a-f]+$": > +type: object > +additionalProperties: true > + > +properties: > + compatible: > +const: qcom,sa8775p-dpu > + > + "^displayport-controller@[0-9a-f]+$": > +type: object > +additionalProperties: true > + > +properties: > + compatible: > +items: > + - const: qcom,sa8775p-dp Where is this binding? The schema is incomplete. Best regards, Krzysztof
Re: [PATCH v3 1/5] dt-bindings: display/msm: Document MDSS on SA8775P
On Tue, Oct 01, 2024 at 12:11:36PM +0530, Mahadevan wrote: > Document the MDSS hardware found on the Qualcomm SA8775P platform. > > Signed-off-by: Mahadevan > --- > .../bindings/display/msm/qcom,sa8775p-mdss.yaml| 241 > + > 1 file changed, 241 insertions(+) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 5/5] dt-bindings: display: mediatek: dpi: correct power-domains property
On Wed, Oct 02, 2024 at 11:09:07AM +0800, Macpaul Lin wrote:
>
>
> On 10/2/24 09:51, Rob Herring wrote:
> >
> >
> > External email : Please do not click links or open attachments until you
> > have verified the sender or the content.
> >
> > On Mon, Sep 30, 2024 at 04:38:54PM +0800, Macpaul Lin wrote:
> > > The MediaTek DPI module is typically associated with one of the
> > > following multimedia power domains:
> > > - POWER_DOMAIN_DISPLAY
> > > - POWER_DOMAIN_VDOSYS
> > > - POWER_DOMAIN_MM
> > > The specific power domain used varies depending on the SoC design.
> > >
> > > These power domains are shared by multiple devices within the SoC.
> > > In most cases, these power domains are enabled by other devices.
> > > As a result, the DPI module of legacy SoCs often functions correctly
> > > even without explicit configuration.
> > >
> > > It is recommended to explicitly add the appropriate power domain
> > > property to the DPI node in the device tree. Hence drop the
> > > compatible checking for specific SoCs.
> > >
> > > Fixes: 5474d49b2f79 ("dt-bindings: display: mediatek: dpi: Add power
> > > domains")
> > > Signed-off-by: Macpaul Lin
> > > Signed-off-by: Jitao Shi
> > > Signed-off-by: Pablo Sun
> > > ---
> > > .../display/mediatek/mediatek,dpi.yaml| 24 ---
> > > 1 file changed, 10 insertions(+), 14 deletions(-)
> >
> > You missed Krzysztof's R-by tag.
> >
>
> Oh, I just missed that reply for v3 in the mailbox
> and thought it still need to be reviewed.
> I just found Krzysztof's R-by tag in the mailbox right now.
>
> I'll send an update for this patch set.
> Thanks for the reminder.
Plaese, instead just start using b4...
Best regards,
Krzysztof
Re: [PATCH v3 0/5] Display enablement changes for Qualcomm SA8775P platform
On 01/10/2024 08:41, Mahadevan via B4 Relay wrote: > This series introduces support to enable the Mobile Display Subsystem (MDSS) > and Display Processing Unit (DPU) for the Qualcomm SA8775P target. It > includes the addition of the hardware catalog, compatible string, > relevant device tree changes, and their YAML bindings. > > --- > In this series PATCH 5: "arm64: dts: qcom: sa8775p: add display dt nodes for > MDSS0 and DPU" > depends on the clock enablement change: > https://lore.kernel.org/all/20240816-sa8775p-mm-v3-v1-0-77d53c3c0...@quicinc.com/ > b4 diff fails. b4 mbox + b4 diff -m also fail. Way to make reviewers life more difficult than it should be. I'll move this patchset to the bottom of the queue. Please in the future send patches in standard way, so our tools can handle it easily. Best regards, Krzysztof
Re: [PATCH v17 3/8] dt-bindings: display: bridge: Add Cadence MHDP8501
On 29/09/2024 04:36, Sandor Yu wrote: > Hi Krzysztof， > > Thanks for your comments, > >> >> >> On Tue, Sep 24, 2024 at 03:36:48PM +0800, Sandor Yu wrote: >>> Add bindings for Cadence MHDP8501 DisplayPort/HDMI bridge. >>> >>> Signed-off-by: Sandor Yu >>> Reviewed-by: Krzysztof Kozlowski >> >> Drop > OK, I will remove it in the next version. > >> >>> --- >>> v16->v17: >>> - Add lane-mapping property >> >> That's a significant change. > OK. > >> >>> >>> v9->v16: >>> *No change >>> >>> .../display/bridge/cdns,mhdp8501.yaml | 109 >> ++ >>> 1 file changed, 109 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml >>> b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml >>> new file mode 100644 >>> index 0..3f79f328c7425 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.y >>> +++ aml >> >>> @@ -0,0 +1,109 @@ >>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 >>> +--- >>> +$id: >>> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi >>> >> +cetree.org%2Fschemas%2Fdisplay%2Fbridge%2Fcdns%2Cmhdp8501.yaml%2 >> 3&dat >>> >> +a=05%7C02%7CSandor.yu%40nxp.com%7C40a6bd4ff1cd4d934da008dcdc72 >> 9fd0%7C >>> >> +686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63862763207998660 >> 8%7CUnkno >>> >> +wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h >> aWwi >>> >> +LCJXVCI6Mn0%3D%7C0%7C%7C%7C&sdata=Uw%2FQQ0Qg36Y8Q6wFPC7Zg >> LzLHvOj8GjH1 >>> +k8McgcjrqI%3D&reserved=0 >>> +$schema: >>> +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi >>> >> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&data=05%7C02%7CSandor.y >> u%40n >>> >> +xp.com%7C40a6bd4ff1cd4d934da008dcdc729fd0%7C686ea1d3bc2b4c6fa92 >> cd99c5 >>> >> +c301635%7C0%7C0%7C638627632080031124%7CUnknown%7CTWFpbGZs >> b3d8eyJWIjoi >>> >> +MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0% >> 7C%7C% >>> >> +7C&sdata=IG9Em3AWOwzlpR1Wk0Vw%2FF161YcOxuMHbA4Y%2FzftgYA%3D >> &reserved= >>> +0 >>> + >>> +title: Cadence MHDP8501 DP/HDMI bridge >>> + >>> +maintainers: >>> + - Sandor Yu >>> + >>> +description: >>> + Cadence MHDP8501 DisplayPort/HDMI interface. >>> + >>> +properties: >>> + compatible: >>> +enum: >>> + - fsl,imx8mq-mhdp8501 >>> + >>> + reg: >>> +maxItems: 1 >>> + >>> + clocks: >>> +maxItems: 1 >>> +description: MHDP8501 DP/HDMI APB clock. >>> + >>> + phys: >>> +maxItems: 1 >>> +description: >>> + phandle to the DP/HDMI PHY >>> + >>> + interrupts: >>> +items: >>> + - description: Hotplug cable plugin. >>> + - description: Hotplug cable plugout. >>> + >>> + interrupt-names: >>> +items: >>> + - const: plug_in >>> + - const: plug_out >>> + >>> + lane-mapping: >>> +description: lane mapping for HDMI or DisplayPort interface. >> >> Where is the definition of this property? I do not see any $ref here, so did >> you >> add it to dtschema? > > My apologies, the $ref is missing, will add it in the next version.. And that's different than existing properties, e.g. data-lanes or lane-polarities? There is no description here except copying property name, which is not useful at all. Come with solution matching other bridges and media devices. Best regards, Krzysztof
Re: [PATCH v2] dt-bindings: display: mediatek: split: add clocks count constraint for MT8195
On Fri, Sep 27, 2024 at 01:51:40PM +0800, Moudy Ho wrote:
> From: Moudy Ho
>
> The display node in mt8195.dtsi was triggering a CHECK_DTBS error due
> to an excessively long 'clocks' property:
> display@14f06000: clocks: [[31, 14], [31, 43], [31, 44]] is too long
>
> To resolve this issue, apply the limit by setting 'maxItems: 3' in MT8195
> additional condition.
>
> Fixes: 4ed545e7d100 ("dt-bindings: display: mediatek: disp: split each block
> to individual yaml")
> Signed-off-by: Macpaul Lin
> Signed-off-by: Moudy Ho
Your SoB does not match.
Please run scripts/checkpatch.pl and fix reported warnings. Then please
run and (probably) fix more warnings.
Some warnings can be ignored, especially from --strict run, but the code
here looks like it needs a fix. Feel free to get in touch if the warning
is not clear.
> --
> The purpose of this patch is to separate the corrections for
> MediaTek SPLIT CHECK_DTBS error from the original mailing list
> mentioned below:
> https://lore.kernel.org/all/20240924103156.13119-2-macpaul@mediatek.com/
>
> Changes since v1:
> - Adding functional descriptions and quantity restrictions.
> ---
> .../bindings/display/mediatek/mediatek,split.yaml | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> index e4affc854f3d..bce1b8b866ce 100644
> --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,split.yaml
> @@ -57,6 +57,9 @@ properties:
>clocks:
> items:
>- description: SPLIT Clock
> + - description: HDMI RX Clock
> + - description: HDMI Metadata Clock
> +minItems: 1
>
> required:
>- compatible
> @@ -72,6 +75,9 @@ allOf:
> const: mediatek,mt8195-mdp3-split
>
> then:
> + properties:
> +clocks:
missing minItems
Missing constraints for all the variants.
Best regards,
Krzysztof
Re: [PATCH v3 5/5] dt-bindings: display: mediatek: dpi: correct power-domains property
On Fri, Sep 27, 2024 at 02:50:41PM +0800, Macpaul Lin wrote:
> The MediaTek DPI module is typically associated with one of the
> following multimedia power domains:
> - POWER_DOMAIN_DISPLAY
> - POWER_DOMAIN_VDOSYS
> - POWER_DOMAIN_MM
> The specific power domain used varies depending on the SoC design.
>
> These power domains are shared by multiple devices within the SoC.
> In most cases, these power domains are enabled by other devices.
> As a result, the DPI module of legacy SoCs often functions correctly
> even without explicit configuration.
>
> It is recommended to explicitly add the appropriate power domain
> property to the DPI node in the device tree. Hence drop the
> compatible checking for specific SoCs.
>
> Fixes: 5474d49b2f79 ("dt-bindings: display: mediatek: dpi: Add power domains")
> Signed-off-by: Macpaul Lin
> Signed-off-by: Jitao Shi
> Signed-off-by: Pablo Sun
> ---
> .../display/mediatek/mediatek,dpi.yaml| 24 ---
> 1 file changed, 10 insertions(+), 14 deletions(-)
Reviewed-by: Krzysztof Kozlowski
Best regards,
Krzysztof
Re: [PATCH v3 2/5] dt-bindings: iommu: mediatek: Fix interrupt count constraint for new SoCs
On Fri, Sep 27, 2024 at 02:50:38PM +0800, Macpaul Lin wrote:
> The infra-iommu node in mt8195.dtsi was triggering a CHECK_DTBS error due
> to an excessively long 'interrupts' property. The error message was:
>
> infra-iommu@10315000: interrupts: [[0, 795, 4, 0], [0, 796, 4, 0],
> [0, 797, 4, 0], [0, 798, 4, 0], [0, 799, 4, 0]]
> is too long
>
> To address this issue, update the compatbile matching rule for
> 'interrupts' property. This change allows flexibility in the number
> of interrupts for new SoCs like MT8195.
> The purpose of these 5 interrupts is also added into description.
>
> Fixes: bca28426805d ("dt-bindings: iommu: mediatek: Convert IOMMU to DT
> schema")
> Signed-off-by: Macpaul Lin
> ---
> .../bindings/iommu/mediatek,iommu.yaml| 25 ++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> Changes for v2:
> - commit message: re-formatting and add a description of adding 5 interrupts.
> - add 'description' and 'maxItems: 5' for 'interrupt' property of
>'mt8195-iommu-infra'
> - others keeps 'maxItems: 1'
>
> Changes for v3:
> - Refine the description for 'interrupts' property and fixes the compatible
>matching rules.
> - Refine commit message.
>
> diff --git a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> index ea6b0f5f24de..10e2bb0f0704 100644
> --- a/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> +++ b/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml
> @@ -96,7 +96,13 @@ properties:
> maxItems: 1
>
>interrupts:
> -maxItems: 1
This does not make sense and was not here at v2. Keep constraints at top
level.
This is how variable-length lists are created:
https://elixir.bootlin.com/linux/v6.11-rc6/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L127
Best regards,
Krzysztof
Re: [PATCH v2 2/5] dt-bindings: display/msm: Document the DPU for SA8775P
On 26/09/2024 13:01, Mahadevan wrote: > Document the DPU for Qualcomm SA8775P platform. > > Signed-off-by: Mahadevan > --- > > [v2] > - Use fake DISPCC nodes to avoid clock dependencies in dt-bindings. [Dmitry] > - Update bindings by fixing dt_binding_check tool errors (update includes in > example), > adding proper spacing and indentation in binding example. [Dmitry, Rob] > - Capitalize clock names in description. [Dmitry] > Please start testing patches before you send them. Best regards, Krzysztof
Re: [PATCH v2 1/5] dt-bindings: display/msm: Document MDSS on SA8775P
On 26/09/2024 13:01, Mahadevan wrote: > + > + clocks: > +items: > + - description: Display AHB > + - description: Display hf AXI > + - description: Display core > + > + iommus: > +maxItems: 1 > + > + interconnects: > +maxItems: 3 > + > + interconnect-names: > +maxItems: 3 > + > +patternProperties: > + "^display-controller@[0-9a-f]+$": > +type: object > +properties: > + compatible: > +const: qcom,sa8775p-dpu Which binding did you used as an example? On which kernel was this developed? Best regards, Krzysztof
Re: [PATCH 6/6] dt-bindings: display: samsung,exynos7-decon: add exynos7870 compatible
On 25/09/2024 22:05, Kaustabh Chakraborty wrote: > On 2024-09-25 19:56, Krzysztof Kozlowski wrote: >> On 25/09/2024 21:36, Kaustabh Chakraborty wrote: >>> On 2024-09-25 19:25, Krzysztof Kozlowski wrote: >>>> On 25/09/2024 20:42, Kaustabh Chakraborty wrote: >>>>> On 2024-09-20 12:39, Krzysztof Kozlowski wrote: >>>>>> On 19/09/2024 17:20, Kaustabh Chakraborty wrote: >>>>>>> Add the compatible string of Exynos7870 to the existing list. >>>>>>> >>>>>>> Signed-off-by: Kaustabh Chakraborty >>>>>> >>>>>> ... and the DTS is ? >>>>> >>>>> Didn't quite understand. The patch adds the compatible string >>>>> for Exynos7870 DECON in documentation. There's no DTS involved >>>>> in here, right? >>>> >>>> Provide lore link to the DTS submission. >>> >>> There aren't any DTS submissions *yet* which use the compatible. >>> Is that an issue? >>> >> >> Yeah, users are supposed to be upstream. Not downstream. > > I understand that. I had plans to submit it in the future. > If that's how it's meant to be done, I'll have to revisit this > submission at a later date then. > Partial, asynchronous bringup of a device is fine, so if the basic support is there, I understand that drivers come in different pace. Although I don't understand why DTS for this piece of hardware would come in different pace, considering you cannot test it without DTS. You have there DTS, so it should be sent. But even without the DTS for DECON, the problem is earlier - lack of basic support for this device. There is nothing for this chip. This means it cannot be tested and is trickier to verify. That's not the usual upstreaming way we expect, especially that you did not provide rationale for such way. Best regards, Krzysztof
Re: [PATCH 6/6] dt-bindings: display: samsung,exynos7-decon: add exynos7870 compatible
On 25/09/2024 21:36, Kaustabh Chakraborty wrote: > On 2024-09-25 19:25, Krzysztof Kozlowski wrote: >> On 25/09/2024 20:42, Kaustabh Chakraborty wrote: >>> On 2024-09-20 12:39, Krzysztof Kozlowski wrote: >>>> On 19/09/2024 17:20, Kaustabh Chakraborty wrote: >>>>> Add the compatible string of Exynos7870 to the existing list. >>>>> >>>>> Signed-off-by: Kaustabh Chakraborty >>>> >>>> ... and the DTS is ? >>> >>> Didn't quite understand. The patch adds the compatible string >>> for Exynos7870 DECON in documentation. There's no DTS involved >>> in here, right? >> >> Provide lore link to the DTS submission. > > There aren't any DTS submissions *yet* which use the compatible. > Is that an issue? > Yeah, users are supposed to be upstream. Not downstream. Best regards, Krzysztof
Re: [PATCH 6/6] dt-bindings: display: samsung,exynos7-decon: add exynos7870 compatible
On 25/09/2024 20:42, Kaustabh Chakraborty wrote: > On 2024-09-20 12:39, Krzysztof Kozlowski wrote: >> On 19/09/2024 17:20, Kaustabh Chakraborty wrote: >>> Add the compatible string of Exynos7870 to the existing list. >>> >>> Signed-off-by: Kaustabh Chakraborty >> >> ... and the DTS is ? > > Didn't quite understand. The patch adds the compatible string > for Exynos7870 DECON in documentation. There's no DTS involved > in here, right? Provide lore link to the DTS submission. Best regards, Krzysztof
Re: [PATCH v3 4/5] dt-bindings: display: msm: dp-controller: document SA8775P compatible
On Mon, Sep 23, 2024 at 05:01:49PM +0530, Soutrik Mukhopadhyay wrote: > Add compatible string for the DisplayPort controller found on the > Qualcomm SA8775P platform. > > Signed-off-by: Soutrik Mukhopadhyay > --- > v2: No change > > v3: No change Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v17 3/8] dt-bindings: display: bridge: Add Cadence MHDP8501
On Tue, Sep 24, 2024 at 03:36:48PM +0800, Sandor Yu wrote: > Add bindings for Cadence MHDP8501 DisplayPort/HDMI bridge. > > Signed-off-by: Sandor Yu > Reviewed-by: Krzysztof Kozlowski Drop > --- > v16->v17: > - Add lane-mapping property That's a significant change. > > v9->v16: > *No change > > .../display/bridge/cdns,mhdp8501.yaml | 109 ++ > 1 file changed, 109 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml > > diff --git > a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml > b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml > new file mode 100644 > index 0..3f79f328c7425 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml > @@ -0,0 +1,109 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/cdns,mhdp8501.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Cadence MHDP8501 DP/HDMI bridge > + > +maintainers: > + - Sandor Yu > + > +description: > + Cadence MHDP8501 DisplayPort/HDMI interface. > + > +properties: > + compatible: > +enum: > + - fsl,imx8mq-mhdp8501 > + > + reg: > +maxItems: 1 > + > + clocks: > +maxItems: 1 > +description: MHDP8501 DP/HDMI APB clock. > + > + phys: > +maxItems: 1 > +description: > + phandle to the DP/HDMI PHY > + > + interrupts: > +items: > + - description: Hotplug cable plugin. > + - description: Hotplug cable plugout. > + > + interrupt-names: > +items: > + - const: plug_in > + - const: plug_out > + > + lane-mapping: > +description: lane mapping for HDMI or DisplayPort interface. Where is the definition of this property? I do not see any $ref here, so did you add it to dtschema? Best regards, Krzysztof
Re: [PATCH v3 1/5] dt-bindings: phy: Add eDP PHY compatible for sa8775p
On Mon, Sep 23, 2024 at 05:01:46PM +0530, Soutrik Mukhopadhyay wrote: > Add compatible string for the supported eDP PHY on sa8775p platform. > > Reviewed-by: Krzysztof Kozlowski As explained in reply to Konrad in v2, reviewed-by was given by mistake. Please drop on next submit. > Acked-by: Krzysztof Kozlowski > Signed-off-by: Soutrik Mukhopadhyay > --- > v2: No change > > v3: No change Best regards, Krzysztof
Re: [PATCH 3/3] drm/rockchip: vop: Split rk3288-vop into big and lit
On 23/09/2024 11:01, Jonas Karlman wrote:
>>> 0, 0,
>>> @@ -1245,8 +1253,13 @@ static const struct of_device_id
>>> vop_driver_dt_match[] = {
>>> .data = &rk3066_vop },
>>> { .compatible = "rockchip,rk3188-vop",
>>> .data = &rk3188_vop },
>>> + { .compatible = "rockchip,rk3288-vop-big",
>>> + .data = &rk3288_vop_big },
>>
>> Hm... that's not really needed. Instead of having three compatibles, you
>> could keep "rk3288-vop" as big and then my comment on bindings patch
>> could be ignored (you keep the compatible).
>
> Thanks, I guess that just adding a new compatible for vop-lit should be
> enough.
>
> VOP_BIG: rockchip,rk3288-vop
> VOP_LIT: rockchip,rk3288-vop-lit, rockchip,rk3288-vop
>
> That should ensure backward/forward compatibility with any mix of
> old/new boot-firmware, DTs and kernels.
>
> Will change to use that in v2.
Yes, thanks.
Best regards,
Krzysztof
Re: [PATCH v4 15/27] regulator: add s2dos05 regulator support
On 19/09/2024 16:28, Dzmitry Sankouski wrote: >>> diff --git a/include/linux/regulator/s2dos05.h >>> b/include/linux/regulator/s2dos05.h >>> new file mode 100644 >>> index ..2e89fcbce769 >>> --- /dev/null >>> +++ b/include/linux/regulator/s2dos05.h >>> @@ -0,0 +1,73 @@ >>> +/* SPDX-License-Identifier: GPL-2.0+ */ >> >> Are you sure that here (and other places) you want any newer GPL? This >> is quite odd. Does original code (from which you took 2016 copyrights) >> have this as well? >> > Original code permits redistribution under 2+ license [1]. > Is 2+ preferable over 2 only? > > [1]: > https://github.com/klabit87/twrp_android_samsung_kernel_sdm845/blob/android-8.0/include/linux/regulator/s2dos05.h#L9 For new code we usually suggest 2-only, but your work looks like derivative, so keeping 2+ is fine. Best regards, Krzysztof
Re: [PATCH v3 14/15] dt-bindings: display: vop2: Add rk3576 support
On Fri, Sep 20, 2024 at 04:23:02PM +0800, Andy Yan wrote: > From: Andy Yan > > Add vop found on rk3576, the main difference between rk3576 and the > previous vop is that each VP has its own interrupt line. > > Signed-off-by: Andy Yan > > --- Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 20/26] dt-bindings: allwinner: add H616 DE33 clock binding
On Sat, Sep 21, 2024 at 09:46:09PM +1200, Ryan Walklin wrote: > The Allwinner H616 and variants have a new display engine revision > (DE33). > > Add a clock binding for the DE33. > > Signed-off-by: Ryan Walklin > Acked-by: Conor Dooley > Reviewed-by: Chen-Yu Tsai > > -- Ditto Best regards, Krzysztof
Re: [PATCH v4 19/26] dt-bindings: allwinner: add H616 DE33 bus binding
On Sat, Sep 21, 2024 at 09:46:08PM +1200, Ryan Walklin wrote: > The Allwinner H616 and variants have a new display engine revision > (DE33). > > Add a display engine bus binding for the DE33. > > Signed-off-by: Ryan Walklin > Acked-by: Conor Dooley > Reviewed-by: Chen-Yu Tsai > > -- Please run scripts/checkpatch.pl and fix reported warnings. Then please run and (probably) fix more warnings. Some warnings can be ignored, especially from --strict run, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. Best regards, Krzysztof
Re: [PATCH v4 21/26] dt-bindings: allwinner: add H616 DE33 mixer binding
On Sat, Sep 21, 2024 at 09:46:10PM +1200, Ryan Walklin wrote: > The Allwinner H616 and variants have a new display engine revision > (DE33). > > The mixer configuration registers are significantly different to the DE3 > and DE2 revisions, being split into separate top and display blocks, > therefore a fallback for the mixer compatible is not provided. > > Add a display engine mixer binding for the DE33. > > Signed-off-by: Ryan Walklin > Acked-by: Conor Dooley > Reviewed-by: Chen-Yu Tsai > > -- That's not a delimiter and you would see checkpatch warning (because it is quite confused). Please run scripts/checkpatch.pl and fix reported warnings. Then please run and (probably) fix more warnings. Some warnings can be ignored, especially from --strict run, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear. Best regards, Krzysztof
Re: [PATCH 3/3] drm/rockchip: vop: Split rk3288-vop into big and lit
On 22/09/2024 00:20, Jonas Karlman wrote:
> The Rockchip RK3288 SoC contain two different Visual Output Processor
> (VOP) blocks, VOP_BIG and VOP_LIT. The VOP blocks support different max
> output resolution, 3840x2160 and 2560x1600.
>
> Add support for the compatible used to differentiate between VOP_BIG and
> VOP_LIT, support for the old compatible is kept for compatibility with
> older device tree.
>
> Signed-off-by: Jonas Karlman
> ---
> drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 27 +++--
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> index e2c6ba26f437..978db93cda33 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c
> @@ -762,7 +762,7 @@ static const struct vop_intr rk3288_vop_intr = {
> .clear = VOP_REG(RK3288_INTR_CTRL0, 0xf, 8),
> };
>
> -static const struct vop_data rk3288_vop = {
> +static const struct vop_data rk3288_vop_big = {
> .version = VOP_VERSION(3, 1),
> .feature = VOP_FEATURE_OUTPUT_RGB10,
> .intr = &rk3288_vop_intr,
> @@ -772,14 +772,22 @@ static const struct vop_data rk3288_vop = {
> .win = rk3288_vop_win_data,
> .win_size = ARRAY_SIZE(rk3288_vop_win_data),
> .lut_size = 1024,
> - /*
> - * This is the maximum resolution for the VOPB, the VOPL can only do
> - * 2560x1600, but we can't distinguish them as they have the same
> - * compatible.
> - */
> .max_output = { 3840, 2160 },
> };
>
> +static const struct vop_data rk3288_vop_lit = {
> + .version = VOP_VERSION(3, 1),
> + .feature = VOP_FEATURE_OUTPUT_RGB10,
> + .intr = &rk3288_vop_intr,
> + .common = &rk3288_common,
> + .modeset = &rk3288_modeset,
> + .output = &rk3288_output,
> + .win = rk3288_vop_win_data,
> + .win_size = ARRAY_SIZE(rk3288_vop_win_data),
> + .lut_size = 1024,
> + .max_output = { 2560, 1600 },
> +};
> +
> static const int rk3368_vop_intrs[] = {
> FS_INTR,
> 0, 0,
> @@ -1245,8 +1253,13 @@ static const struct of_device_id vop_driver_dt_match[]
> = {
> .data = &rk3066_vop },
> { .compatible = "rockchip,rk3188-vop",
> .data = &rk3188_vop },
> + { .compatible = "rockchip,rk3288-vop-big",
> + .data = &rk3288_vop_big },
Hm... that's not really needed. Instead of having three compatibles, you
could keep "rk3288-vop" as big and then my comment on bindings patch
could be ignored (you keep the compatible).
Best regards,
Krzysztof
Re: [PATCH 1/3] dt-bindings: display: rockchip-vop: Split rk3288-vop into big and lit
On 22/09/2024 00:20, Jonas Karlman wrote: > The Rockchip RK3288 SoC contain two different Visual Output Processor > (VOP) blocks, VOP_BIG and VOP_LIT. The VOP blocks support different max > output resolution, 3840x2160 and 2560x1600. > > Add compatible to differentiate between the two VOP blocks. > > Signed-off-by: Jonas Karlman > --- > .../display/rockchip/rockchip-vop.yaml| 36 +++ > 1 file changed, 21 insertions(+), 15 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.yaml > b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.yaml > index b339b7e708c6..ce4169b030af 100644 > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.yaml > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop.yaml > @@ -17,21 +17,27 @@ maintainers: > > properties: >compatible: > -enum: > - - rockchip,px30-vop-big > - - rockchip,px30-vop-lit > - - rockchip,rk3036-vop > - - rockchip,rk3066-vop > - - rockchip,rk3126-vop > - - rockchip,rk3188-vop > - - rockchip,rk3228-vop > - - rockchip,rk3288-vop > - - rockchip,rk3328-vop > - - rockchip,rk3366-vop > - - rockchip,rk3368-vop > - - rockchip,rk3399-vop-big > - - rockchip,rk3399-vop-lit > - - rockchip,rv1126-vop > +oneOf: > + - items: > + - enum: > + - rockchip,rk3288-vop-big > + - rockchip,rk3288-vop-lit > + - const: rockchip,rk3288-vop > + - enum: > + - rockchip,px30-vop-big > + - rockchip,px30-vop-lit > + - rockchip,rk3036-vop > + - rockchip,rk3066-vop > + - rockchip,rk3126-vop > + - rockchip,rk3188-vop > + - rockchip,rk3228-vop > + - rockchip,rk3288-vop I think this one should be dropped. You will update all in-kernel users, so it won't be needed here and all other projects should probably follow up. Best regards, Krzysztof
Re: [PATCH 1/5] dt-bindings: display/msm: Document MDSS on SA8775P
On 12/09/2024 09:14, Mahadevan wrote: > > +clocks = <&dispcc0 MDSS_DISP_CC_MDSS_AHB_CLK>, > + <&gcc GCC_DISP_HF_AXI_CLK>, > + <&dispcc0 MDSS_DISP_CC_MDSS_MDP_CLK>; > + > +interrupts = ; > +interrupt-controller; > +#interrupt-cells = <1>; > + > +iommus = <&apps_smmu 0x1000 0x402>; > + > +#address-cells = <2>; > +#size-cells = <2>; > +ranges; > + > +status = "disabled"; Uh no, it cannot be disabled. What would be the point of it? Please reach to your colleagues for some internal review before posting (see also go/upstream in internal systems). Best regards, Krzysztof
Re: [PATCH 2/6] drm/exynos: exynos7_drm_decon: fix suspended condition in decon_commit()
On 19/09/2024 17:11, Kaustabh Chakraborty wrote: > decon_commit() gets called during atomic_enable. At this stage, DECON is > suspended, and thus the function refuses to run. Fix the suspended > condition checking in decon_commit(). > > Signed-off-by: Kaustabh Chakraborty > --- If this is a fix, then you miss fixes tag and cc-stable. However the explanation seems just incomplete. This looked like a intentional code, so you should explain really why original approach was wrong. Best regards, Krzysztof
Re: [PATCH 6/6] dt-bindings: display: samsung,exynos7-decon: add exynos7870 compatible
On 19/09/2024 17:20, Kaustabh Chakraborty wrote: > Add the compatible string of Exynos7870 to the existing list. > > Signed-off-by: Kaustabh Chakraborty ... and the DTS is ? Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 23/27] arm64: dts: qcom: starqltechn: add display PMIC
On 19/09/2024 15:17, Dzmitry Sankouski wrote:
>>> + pmic@60 {
>>> + compatible = "samsung,s2dos05";
>>> + reg = <0x60>;
>>> +
>>> + regulators {
>>> + s2dos05_ldo1: ldo1 {
>>> + regulator-active-discharge = <1>;
>>> + regulator-enable-ramp-delay = <12000>;
>>> + regulator-min-microvolt = <150>;
>>> + regulator-max-microvolt = <200>;
>>> + regulator-name = "s2dos05-ldo1";
>>
>> Useless name. Please use rather names from the schematics, but I guess
>> you might not have them, so maybe downstream has reasonable name?
>
> Unfortunately, downstream uses that same name.
Then "ldo1" is enough.
Best regards,
Krzysztof
Re: [PATCH v4 08/27] mfd: max77693: remove unused declarations
On 19/09/2024 10:40, Dzmitry Sankouski wrote: > чт, 19 сент. 2024 г. в 10:00, Krzysztof Kozlowski : >> >> On 18/09/2024 14:53, Dzmitry Sankouski wrote: >>> пн, 16 сент. 2024 г. в 12:10, Krzysztof Kozlowski : >>>> >>>> On Fri, Sep 13, 2024 at 06:07:51PM +0300, Dzmitry Sankouski wrote: >>>>> Remove `enum max77693_irq_source` declaration because unused. >>>>> >>>>> Signed-off-by: Dzmitry Sankouski >>>>> --- >>>>> include/linux/mfd/max77693-private.h | 11 --- >>>>> 1 file changed, 11 deletions(-) >>>> >>>> Please split your patchset per subsystems. There is no dependency on MFD >>>> bits from your DTS... (if there is, this needs to be fixed anyway) >>> >>> Indeed, my dts has no dependency on this patch. >>> However, my dts has dependency on MAX77705, so AFAIU, >>> I should send this patch separately, while leaving other drivers in same >>> patchset, right? >> >> How DTS could have dependency on MAX77705? It's a clear no go - broken >> patch. And something very weird, almost never happening for new hardware. >> > Oh right, dts only depends on driver bindings, not driver code, so I > can send dts > patches with bindings in separate series, and per subsystem series for new > driver code. This is how you can organize patchsets: https://lore.kernel.org/all/20231121-topic-sm8650-upstream-dt-v3-0-db9d0507f...@linaro.org/ Here is a brief description how to organize the patchset: https://lore.kernel.org/linux-samsung-soc/CADrjBPq_0nUYRABKpskRF_dhHu+4K=duPVZX==0pr+cjsl_...@mail.gmail.com/T/#m2d9130a1342ab201ab49670fa6c858ee3724c83c Best regards, Krzysztof
Re: [PATCH v4 08/27] mfd: max77693: remove unused declarations
On 18/09/2024 14:53, Dzmitry Sankouski wrote: > пн, 16 сент. 2024 г. в 12:10, Krzysztof Kozlowski : >> >> On Fri, Sep 13, 2024 at 06:07:51PM +0300, Dzmitry Sankouski wrote: >>> Remove `enum max77693_irq_source` declaration because unused. >>> >>> Signed-off-by: Dzmitry Sankouski >>> --- >>> include/linux/mfd/max77693-private.h | 11 --- >>> 1 file changed, 11 deletions(-) >> >> Please split your patchset per subsystems. There is no dependency on MFD >> bits from your DTS... (if there is, this needs to be fixed anyway) > > Indeed, my dts has no dependency on this patch. > However, my dts has dependency on MAX77705, so AFAIU, > I should send this patch separately, while leaving other drivers in same > patchset, right? How DTS could have dependency on MAX77705? It's a clear no go - broken patch. And something very weird, almost never happening for new hardware. Best regards, Krzysztof
Re: [PATCH 1/2] dt-bindings: display: panel-simple: Document support for Microchip AC69T88A
On 18/09/2024 05:08, manikanda...@microchip.com wrote: > Hi Krzysztof, > > On 17/09/24 4:07 pm, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the >> content is safe >> >> On 17/09/2024 11:53, Manikandan Muralidharan wrote: >>> Add Microchip AC69T88A 5" LVDS interface (800x480) TFT LCD panel >>> compatible string >>> >>> Signed-off-by: Manikandan Muralidharan >>> --- >>> .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml >>> b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml >>> index b89e39790579..09911b89d140 100644 >>> --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml >>> +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml >>> @@ -200,6 +200,8 @@ properties: >>> - logictechno,lttd800480070-l2rt >>> # Logic Technologies LTTD800480070-L6WH-RT 7” 800x480 TFT >>> Resistive Touch Module >>> - logictechno,lttd800480070-l6wh-rt >>> +# Microchip AC69T88A 5" 800X480 LVDS interface TFT LCD Panel >>> + - microchip,ac69t88a-lvds-panel >> >> Is this device some sort of multi-function? Why "lvds-panel"? What else >> could it be? > This device does not multi-function, I will rephrase and share a v2 Then drop lvds-panel, please. Best regards, Krzysztof
Re: [PATCH 1/2] dt-bindings: display: panel-simple: Document support for Microchip AC69T88A
On 17/09/2024 11:53, Manikandan Muralidharan wrote: > Add Microchip AC69T88A 5" LVDS interface (800x480) TFT LCD panel > compatible string > > Signed-off-by: Manikandan Muralidharan > --- > .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > index b89e39790579..09911b89d140 100644 > --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > @@ -200,6 +200,8 @@ properties: >- logictechno,lttd800480070-l2rt > # Logic Technologies LTTD800480070-L6WH-RT 7” 800x480 TFT Resistive > Touch Module >- logictechno,lttd800480070-l6wh-rt > +# Microchip AC69T88A 5" 800X480 LVDS interface TFT LCD Panel > + - microchip,ac69t88a-lvds-panel Is this device some sort of multi-function? Why "lvds-panel"? What else could it be? Best regards, Krzysztof
Re: [PATCH v2 1/5] dt-bindings: phy: Add eDP PHY compatible for sa8775p
On 17/09/2024 01:26, Konrad Dybcio wrote: > On 16.09.2024 10:33 PM, Dmitry Baryshkov wrote: >> On Mon, Sep 16, 2024 at 05:23:55PM GMT, Krzysztof Kozlowski wrote: >>> On Fri, Sep 13, 2024 at 04:07:51PM +0530, Soutrik Mukhopadhyay wrote: >>>> Add compatible string for the supported eDP PHY on sa8775p platform. >>>> >>>> Signed-off-by: Soutrik Mukhopadhyay >>>> --- >>>> v2: No change >>>> >>> >>> Acked-by: Krzysztof Kozlowski >> >> So, is it reviewed or acked? > > After a thorough review, it has been acked Way too many emails in inbox after short trip... This should be only acked. Best regards, Krzysztof
Re: [PATCH v4 04/27] dt-bindings: panel: add Samsung s6e3ha8
On Fri, Sep 13, 2024 at 06:07:47PM +0300, Dzmitry Sankouski wrote:
> Add binding for the Samsung s6e3ha8 panel found in the Samsung S9.
>
> Signed-off-by: Dzmitry Sankouski
>
> ---
> Changes in v4:
> - change dts example intendation from tabs
> to spaces
> - remove reset-gpios description
> ---
> .../bindings/display/panel/samsung,s6e3ha8.yaml| 75
> ++
> MAINTAINERS| 5 ++
> 2 files changed, 80 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha8.yaml
> b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha8.yaml
> new file mode 100644
> index ..94c812e07571
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/samsung,s6e3ha8.yaml
> @@ -0,0 +1,75 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/panel/samsung,s6e3ha8.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung s6e3ha8 AMOLED DSI panel
> +
> +description: The s6e3ha8 is a 1440x2960 DPI display panel from Samsung Mobile
> + Displays (SMD).
> +
> +maintainers:
> + - Dzmitry Sankouski
> +
> +allOf:
> + - $ref: panel-common.yaml#
> +
> +properties:
> + compatible:
> +const: samsung,s6e3ha8
> +
> + reg:
> +maxItems: 1
> +
> + reset-gpios: true
> +
> + port: true
> +
> + vdd3-supply:
> +description: VDD regulator
> +
> + vci-supply:
> +description: VCI regulator
> +
> + vddr-supply:
> +description: VDDR regulator
> +
> +required:
> + - compatible
> + - reset-gpios
> + - vdd3-supply
> + - vddr-supply
> + - vci-supply
If there is going to be resend, then keep the same order as in
properties: block.
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +#include
> +
> +dsi {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +panel@0 {
> +compatible = "samsung,s6e3ha8";
> +reg = <0>;
> +vci-supply = <&s2dos05_ldo4>;
> +vddr-supply = <&s2dos05_buck1>;
> +vdd3-supply = <&s2dos05_ldo1>;
> +te-gpios = <&tlmm 10 GPIO_ACTIVE_HIGH>;
> +reset-gpios = <&tlmm 6 GPIO_ACTIVE_HIGH>;
> +pinctrl-0 = <&sde_dsi_active &sde_te_active_sleep>;
> +pinctrl-1 = <&sde_dsi_suspend &sde_te_active_sleep>;
> +pinctrl-names = "default", "sleep";
> +
> +port {
> +panel_in: endpoint {
> +remote-endpoint = <&mdss_dsi0_out>;
> +};
> +};
> + };
Indentation is still mixed up.
> +};
> +
> +...
Best regards,
Krzysztof
Re: [PATCH v2 1/5] dt-bindings: phy: Add eDP PHY compatible for sa8775p
On Fri, Sep 13, 2024 at 04:07:51PM +0530, Soutrik Mukhopadhyay wrote: > Add compatible string for the supported eDP PHY on sa8775p platform. > > Signed-off-by: Soutrik Mukhopadhyay > --- > v2: No change > Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 1/5] dt-bindings: phy: Add eDP PHY compatible for sa8775p
On Fri, Sep 13, 2024 at 04:07:51PM +0530, Soutrik Mukhopadhyay wrote: > Add compatible string for the supported eDP PHY on sa8775p platform. > > Signed-off-by: Soutrik Mukhopadhyay > --- > v2: No change > > --- > Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml | 1 + Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 15/27] regulator: add s2dos05 regulator support
On Fri, Sep 13, 2024 at 06:07:58PM +0300, Dzmitry Sankouski wrote:
> S2dos05 has 1 buck and 4 LDO regulators, used for powering
> panel/touchscreen.
>
> Signed-off-by: Dzmitry Sankouski
>
...
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct s2dos05_data {
> + struct regmap *regmap;
> + struct device *dev;
> +};
> +
> +static const struct regulator_ops s2dos05_ops = {
Keep definitions together. This goes down, after all declarations and
macros.
> + .list_voltage = regulator_list_voltage_linear,
> + .map_voltage= regulator_map_voltage_linear,
> + .is_enabled = regulator_is_enabled_regmap,
> + .enable = regulator_enable_regmap,
> + .disable= regulator_disable_regmap,
> + .get_voltage_sel= regulator_get_voltage_sel_regmap,
> + .set_voltage_sel= regulator_set_voltage_sel_regmap,
> + .set_voltage_time_sel = regulator_set_voltage_time_sel,
> + .set_active_discharge = regulator_set_active_discharge_regmap,
> +};
> +
> +#define _BUCK(macro) S2DOS05_BUCK##macro
> +#define _buck_ops(num) s2dos05_ops##num
> +
> +#define _LDO(macro) S2DOS05_LDO##macro
> +#define _REG(ctrl) S2DOS05_REG##ctrl
> +#define _ldo_ops(num)s2dos05_ops##num
> +#define _MASK(macro) S2DOS05_ENABLE_MASK##macro
> +#define _TIME(macro) S2DOS05_ENABLE_TIME##macro
> +
...
> +
> +static struct regulator_desc regulators[S2DOS05_REGULATOR_MAX] = {
This should be const.
> + // name, id, ops, min_uv, uV_step, vsel_reg, enable_reg
> + LDO_DESC("ldo1", _LDO(1), &_ldo_ops(), _LDO(_MIN1),
> + _LDO(_STEP1), _REG(_LDO1_CFG),
> + _REG(_EN), _MASK(_L1), _TIME(_LDO), _REG(_LDO1_CFG)),
> + LDO_DESC("ldo2", _LDO(2), &_ldo_ops(), _LDO(_MIN1),
> + _LDO(_STEP1), _REG(_LDO2_CFG),
> + _REG(_EN), _MASK(_L2), _TIME(_LDO), _REG(_LDO2_CFG)),
> + LDO_DESC("ldo3", _LDO(3), &_ldo_ops(), _LDO(_MIN2),
> + _LDO(_STEP1), _REG(_LDO3_CFG),
> + _REG(_EN), _MASK(_L3), _TIME(_LDO), _REG(_LDO3_CFG)),
> + LDO_DESC("ldo4", _LDO(4), &_ldo_ops(), _LDO(_MIN2),
> + _LDO(_STEP1), _REG(_LDO4_CFG),
> + _REG(_EN), _MASK(_L4), _TIME(_LDO), _REG(_LDO4_CFG)),
> + BUCK_DESC("buck1", _BUCK(1), &_buck_ops(), _BUCK(_MIN1),
> + _BUCK(_STEP1), _REG(_BUCK_VOUT),
> + _REG(_EN), _MASK(_B1), _TIME(_BUCK), _REG(_BUCK_CFG)),
> +};
> +
> +static int s2dos05_pmic_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct sec_pmic_dev *iodev = dev_get_drvdata(pdev->dev.parent);
> + struct of_regulator_match *rdata = NULL;
> + struct s2dos05_data *s2dos05;
> + struct regulator_config config = { };
> + unsigned int rdev_num = ARRAY_SIZE(regulators);
> + int i, ret;
> +
> + s2dos05 = devm_kzalloc(dev, sizeof(struct s2dos05_data),
> + GFP_KERNEL);
sizeof(*)
> + if (!s2dos05)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, s2dos05);
> +
> + rdata = devm_kcalloc(dev, rdev_num, sizeof(*rdata), GFP_KERNEL);
> + if (!rdata)
> + return -ENOMEM;
> +
> + for (i = 0; i < rdev_num; i++)
> + rdata[i].name = regulators[i].name;
> +
> + s2dos05->regmap = iodev->regmap_pmic;
> + s2dos05->dev = dev;
> + if (!dev->of_node)
> + dev->of_node = dev->parent->of_node;
> +
> + for (i = 0; i < rdev_num; i++) {
> + struct regulator_dev *regulator;
> +
> + config.init_data = rdata[i].init_data;
> + config.of_node = rdata[i].of_node;
> + config.dev = dev;
> + config.driver_data = s2dos05;
> + regulator = devm_regulator_register(&pdev->dev,
> + ®ulators[i], &config);
> + if (IS_ERR(regulator)) {
> + ret = PTR_ERR(regulator);
> + dev_err(&pdev->dev, "regulator init failed for %d\n",
> + i);
> + }
> + }
> +
> + return ret;
> +}
> +
> +static const struct platform_device_id s2dos05_pmic_id[] = {
> + { "s2dos05-regulator" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(platform, s2dos05_pmic_id);
> +
> +static struct platform_driver s2dos05_platform_driver = {
> + .driver = {
> + .name = "s2dos05",
> + },
> + .probe = s2dos05_pmic_probe,
> + .id_table = s2dos05_pmic_id,
> +};
> +module_platform_driver(s2dos05_platform_driver);
> +
> +MODULE_AUTHOR("Dzmitry Sankouski ");
> +MODULE_DESCRIPTION("SAMSUNG s2dos05 Regulator Driver");
s/SAMSUNG/Samsung/
Also, your Kconfi
Re: [PATCH v4 22/27] arm64: dts: qcom: starqltechn: add max77705 PMIC
On Fri, Sep 13, 2024 at 06:08:05PM +0300, Dzmitry Sankouski wrote:
> Add support for max77705 MFD device. Supported sub-devices:
> charger, fuelgauge, haptic, led
>
> Signed-off-by: Dzmitry Sankouski
> ---
> .../boot/dts/qcom/sdm845-samsung-starqltechn.dts | 103
> +
> 1 file changed, 103 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> b/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> index a3bd5231569d..865253d8f0c7 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> @@ -18,6 +18,16 @@ / {
> model = "Samsung Galaxy S9 SM-G9600";
> compatible = "samsung,starqltechn", "qcom,sdm845";
>
> + battery: battery {
> + compatible = "simple-battery";
> + constant-charge-current-max-microamp = <215>;
> + charge-full-design-microamp-hours = <300>;
> +
> + over-voltage-threshold-microvolt = <450>;
> + voltage-min-design-microvolt = <340>;
> + voltage-max-design-microvolt = <435>;
> + };
> +
> chosen {
> #address-cells = <2>;
> #size-cells = <2>;
> @@ -90,6 +100,27 @@ key-wink {
> debounce-interval = <15>;
> };
> };
> +
> + vib_regulator: gpio-regulator {
Please use name for all fixed regulators which matches current format
recommendation: 'regulator-[0-9]v[0-9]'
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/regulator/fixed-regulator.yaml?h=v6.11-rc1#n46
> + compatible = "regulator-fixed";
> + regulator-name = "haptic";
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> + regulator-boot-on;
> + enable-active-high;
> + gpio = <&pm8998_gpios 18 GPIO_ACTIVE_HIGH>;
> + };
> +
> + vib_pwm: pwm {
> + #pwm-cells = <2>;
> + compatible = "clk-pwm";
compatible is always the first property. See DTS coding style.
> + clocks = <&gcc GCC_GP1_CLK>;
> + assigned-clock-parents = <&rpmhcc RPMH_CXO_CLK>;
> + assigned-clocks = <&gcc GCC_GP1_CLK_SRC>;
> + pinctrl-0 = <&motor_pwm_default_state>;
> + pinctrl-1 = <&motor_pwm_suspend_state>;
> + pinctrl-names = "default", "suspend";
> + };
> };
>
>
> @@ -385,10 +416,66 @@ &qupv3_id_1 {
> status = "okay";
> };
>
> +&gpi_dma1 {
> + status = "okay";
> +};
> +
> &uart9 {
> status = "okay";
> };
>
> +&i2c14 {
> + status = "okay";
> +
> + pmic@66 {
> + compatible = "maxim,max77705";
> + reg = <0x66>;
> + interrupt-parent = <&pm8998_gpios>;
> + interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
> + pinctrl-0 = <&chg_int_default>;
> + pinctrl-names = "default";
> +
> + leds {
> + compatible = "maxim,max77705-led";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + led@1 {
> + reg = <1>;
> + label = "red:usr1";
> + };
> +
> + led@2 {
> + reg = <2>;
> + label = "green:usr2";
> + };
> +
> + led@3 {
> + reg = <3>;
> + label = "blue:usr3";
> + };
> + };
> +
> + max77705_charger: charger {
> + compatible = "maxim,max77705-charger";
> + monitored-battery = <&battery>;
> + };
> +
> + fuel_gauge {
No underscores in node names. See DTS coding style.
Best regards,
Krzysztof
Re: [PATCH v4 23/27] arm64: dts: qcom: starqltechn: add display PMIC
On Fri, Sep 13, 2024 at 06:08:06PM +0300, Dzmitry Sankouski wrote:
> Add support for s2dos05 display / touchscreen PMIC
>
> Signed-off-by: Dzmitry Sankouski
> ---
> .../boot/dts/qcom/sdm845-samsung-starqltechn.dts | 77
> ++
> 1 file changed, 77 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> b/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> index 865253d8f0c7..5e5684f84ffb 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts
> @@ -39,6 +39,9 @@ framebuffer: framebuffer@9d40 {
> height = <2960>;
> stride = <(1440 * 4)>;
> format = "a8r8g8b8";
> + vci-supply = <&s2dos05_ldo4>;
> + vddr-supply = <&s2dos05_buck1>;
> + vdd3-supply = <&s2dos05_ldo1>;
> };
> };
>
> @@ -101,6 +104,66 @@ key-wink {
> };
> };
>
> + i2c21 {
> + compatible = "i2c-gpio";
> + sda-gpios = <&tlmm 127 GPIO_ACTIVE_HIGH>;
> + scl-gpios = <&tlmm 128 GPIO_ACTIVE_HIGH>;
> + i2c-gpio,delay-us = <2>;
> + pinctrl-0 = <&i2c21_sda_state &i2c21_scl_state>;
> + pinctrl-names = "default";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pmic@60 {
> + compatible = "samsung,s2dos05";
> + reg = <0x60>;
> +
> + regulators {
> + s2dos05_ldo1: ldo1 {
> + regulator-active-discharge = <1>;
> + regulator-enable-ramp-delay = <12000>;
> + regulator-min-microvolt = <150>;
> + regulator-max-microvolt = <200>;
> + regulator-name = "s2dos05-ldo1";
Useless name. Please use rather names from the schematics, but I guess
you might not have them, so maybe downstream has reasonable name?
Best regards,
Krzysztof
Re: [PATCH v4 06/27] dt-bindings: mfd: add samsung,s2dos05
On Fri, Sep 13, 2024 at 06:07:49PM +0300, Dzmitry Sankouski wrote:
> Add samsung,s2dos05 MFD module binding.
>
> Signed-off-by: Dzmitry Sankouski
>
> ---
> Changes in v4:
> - split long(>80) lines
> - fix indentation
> - merge with regulators binding
> - drop pmic suffix
> - drop unused labels in example
> - correct description
> ---
> .../devicetree/bindings/mfd/samsung,s2dos05.yaml | 99
> ++
> MAINTAINERS| 1 +
> 2 files changed, 100 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/samsung,s2dos05.yaml
> b/Documentation/devicetree/bindings/mfd/samsung,s2dos05.yaml
> new file mode 100644
> index ..534434002045
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/samsung,s2dos05.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/samsung,s2dos05.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung S2DOS05 Power Management IC
> +
> +maintainers:
> + - Dzmitry Sankouski
> +
> +description:
> + This is a device tree bindings for S2DOS family of Power Management IC
> (PMIC).
Drop this sentence, not really useful. I know that I put it into other
Samsung PMIC bindings, but let's don't grow this pattern.
> +
> + The S2DOS05 is a companion power management IC for the panel and
> touchscreen
> + in smart phones. Provides voltage regulators and
> + ADC for power/current measurements.
> +
> + Regulator section has 4 LDO and 1 BUCK regulators and also
> + provides ELVDD, ELVSS, AVDD lines.
What are these? Input supplies?
> +
> +properties:
> + compatible:
> +const: samsung,s2dos05
> +
> + reg:
> +maxItems: 1
> +
> + regulators:
> +patternProperties:
> + "^buck1|ldo[1-4]$":
s/buck1/buck/
> +type: object
> +$ref: /schemas/regulator/regulator.yaml#
> +unevaluatedProperties: false
> +
> +required:
> + - regulator-name
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - regulators
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pmic@60 {
> +compatible = "samsung,s2dos05";
> +reg = <0x60>;
> +
> +regulators {
Messed indentation.
Use 4 spaces for example indentation.
> +ldo1 {
> +regulator-name = "s2dos05-ldo1";
Such name is useless, so it's a clear sign you should not require it. If
you keep it in example, then say something useful - see your DTS.
> +regulator-min-microvolt = <150>;
> +regulator-max-microvolt = <200>;
> +regulator-active-discharge = <0x1>;
> +};
...
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 59d027591e34..92135252264a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20465,6 +20465,7 @@ L:linux-samsung-...@vger.kernel.org
> S: Maintained
> B: mailto:linux-samsung-...@vger.kernel.org
> F: Documentation/devicetree/bindings/clock/samsung,s2mps11.yaml
> +F: Documentation/devicetree/bindings/mfd/samsung,s2dos*.yaml
> F: Documentation/devicetree/bindings/mfd/samsung,s2m*.yaml
Maybe just change this pattern to s2*.yaml ?
> F: Documentation/devicetree/bindings/mfd/samsung,s5m*.yaml
> F: Documentation/devicetree/bindings/regulator/samsung,s2m*.yaml
>
> --
> 2.39.2
>
Re: [PATCH v4 08/27] mfd: max77693: remove unused declarations
On Fri, Sep 13, 2024 at 06:07:51PM +0300, Dzmitry Sankouski wrote: > Remove `enum max77693_irq_source` declaration because unused. > > Signed-off-by: Dzmitry Sankouski > --- > include/linux/mfd/max77693-private.h | 11 --- > 1 file changed, 11 deletions(-) Please split your patchset per subsystems. There is no dependency on MFD bits from your DTS... (if there is, this needs to be fixed anyway) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v4 14/27] mfd: sec-core: add s2dos05 support
On Fri, Sep 13, 2024 at 06:07:57PM +0300, Dzmitry Sankouski wrote: > S2dos05 is a panel/touchscreen PMIC, often found in > Samsung phones. We define 2 sub-devices for which drivers will > be added in subsequent patches. > > Signed-off-by: Dzmitry Sankouski > --- > drivers/mfd/sec-core.c | 11 +++ > include/linux/mfd/samsung/core.h | 1 + > 2 files changed, 12 insertions(+) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v1] drm/exynos: gsc: Fix typo in comment
On 09/09/2024 10:06, Shen Lichuan wrote: > Replace 'initailization' with 'initialization' in the comment. > > Signed-off-by: Shen Lichuan > --- > drivers/gpu/drm/exynos/exynos_drm_gsc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gsc.c > b/drivers/gpu/drm/exynos/exynos_drm_gsc.c > index 1b111e2c3347..fc5fc65823c6 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_gsc.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_gsc.c > @@ -1286,7 +1286,7 @@ static int gsc_probe(struct platform_device *pdev) > return ret; > } > > - /* context initailization */ > + /* context initialization */ @vivo.com, please fix all typos given subsystem in one run. One patch or few patches in one patchset. Not one typo per patch. Best regards, Krzysztof
Re: [PATCH v5 1/2] dt-bindings: arm: mediatek: Add MT8186 Ponyta Chromebook
On Mon, Sep 09, 2024 at 10:31:47AM +0800, Jianeng Ceng wrote: > Ponyta is a custom label Chromebook based on MT8186. It is a > self-developed project of Huaqin and has no fixed OEM. > > Signed-off-by: Jianeng Ceng > --- Do not attach (thread) your patchsets to some other threads (unrelated or older versions). This buries them deep in the mailbox and might interfere with applying entire sets. Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
[PATCH 5/5] drm/bridge: ti-dlpc3433: constify regmap_config
Mark local static 'struct regmap_config' as const for safer and more
obvious code.
Signed-off-by: Krzysztof Kozlowski
---
drivers/gpu/drm/bridge/ti-dlpc3433.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-dlpc3433.c
b/drivers/gpu/drm/bridge/ti-dlpc3433.c
index 6b559e071301..a0a1b5dd794e 100644
--- a/drivers/gpu/drm/bridge/ti-dlpc3433.c
+++ b/drivers/gpu/drm/bridge/ti-dlpc3433.c
@@ -94,7 +94,7 @@ static const struct regmap_access_table dlpc_volatile_table =
{
.n_yes_ranges = ARRAY_SIZE(dlpc_volatile_ranges),
};
-static struct regmap_config dlpc_regmap_config = {
+static const struct regmap_config dlpc_regmap_config = {
.reg_bits = 8,
.val_bits = 8,
.max_register = WR_DSI_PORT_EN,
--
2.43.0
[PATCH 4/5] drm/mediatek: dp: constify regmap_config
Mark local static 'struct regmap_config' as const for safer and more
obvious code.
Signed-off-by: Krzysztof Kozlowski
---
drivers/gpu/drm/mediatek/mtk_dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c
b/drivers/gpu/drm/mediatek/mtk_dp.c
index d8796a904eca..f0f6f402994a 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -311,7 +311,7 @@ static const struct mtk_dp_efuse_fmt
mt8195_dp_efuse_fmt[MTK_DP_CAL_MAX] = {
},
};
-static struct regmap_config mtk_dp_regmap_config = {
+static const struct regmap_config mtk_dp_regmap_config = {
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
--
2.43.0
[PATCH 3/5] drm/fsl-dcu: constify regmap_config
Mark local static 'struct regmap_config' as const for safer and more
obvious code.
Signed-off-by: Krzysztof Kozlowski
---
drivers/gpu/drm/fsl-dcu/fsl_tcon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
index 9eb5abaf7d66..49bbd00c77ae 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_tcon.c
@@ -29,7 +29,7 @@ void fsl_tcon_bypass_enable(struct fsl_tcon *tcon)
FSL_TCON_CTRL1_TCON_BYPASS);
}
-static struct regmap_config fsl_tcon_regmap_config = {
+static const struct regmap_config fsl_tcon_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
.val_bits = 32,
--
2.43.0
[PATCH 2/5] drm/meson: constify regmap_config
Mark local static 'struct regmap_config' as const for safer and more
obvious code.
Signed-off-by: Krzysztof Kozlowski
---
drivers/gpu/drm/meson/meson_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/meson/meson_drv.c
b/drivers/gpu/drm/meson/meson_drv.c
index 4bd0baa2a4f5..6c8677d1f562 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -126,7 +126,7 @@ static bool meson_vpu_has_available_connectors(struct
device *dev)
return false;
}
-static struct regmap_config meson_regmap_config = {
+static const struct regmap_config meson_regmap_config = {
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
--
2.43.0
[PATCH 1/5] drm/meson: drop unused staitc dw_hdmi_dwc_write_bits
static inline dw_hdmi_dwc_write_bits() function is not used at all:
drivers/gpu/drm/meson/meson_dw_hdmi.c:276:20: error: unused function
'dw_hdmi_dwc_write_bits' [-Werror,-Wunused-function]
Signed-off-by: Krzysztof Kozlowski
---
drivers/gpu/drm/meson/meson_dw_hdmi.c | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c
b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 5565f529..b75db829b1da 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -272,20 +272,6 @@ static inline void dw_hdmi_g12a_dwc_write(struct
meson_dw_hdmi *dw_hdmi,
writeb(data, dw_hdmi->hdmitx + addr);
}
-/* Helper to change specific bits in controller registers */
-static inline void dw_hdmi_dwc_write_bits(struct meson_dw_hdmi *dw_hdmi,
- unsigned int addr,
- unsigned int mask,
- unsigned int val)
-{
- unsigned int data = dw_hdmi->data->dwc_read(dw_hdmi, addr);
-
- data &= ~mask;
- data |= val;
-
- dw_hdmi->data->dwc_write(dw_hdmi, addr, data);
-}
-
/* Bridge */
/* Setup PHY bandwidth modes */
--
2.43.0
[PATCH 0/5] drm: misc: few simple cleanups
No dependencies, trivial patches. Best regards, Krzysztof --- Krzysztof Kozlowski (5): drm/meson: drop unused staitc dw_hdmi_dwc_write_bits drm/meson: constify regmap_config drm/fsl-dcu: constify regmap_config drm/mediatek: dp: constify regmap_config drm/bridge: ti-dlpc3433: constify regmap_config drivers/gpu/drm/bridge/ti-dlpc3433.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_tcon.c| 2 +- drivers/gpu/drm/mediatek/mtk_dp.c | 2 +- drivers/gpu/drm/meson/meson_drv.c | 2 +- drivers/gpu/drm/meson/meson_dw_hdmi.c | 14 -- 5 files changed, 4 insertions(+), 18 deletions(-) --- base-commit: ad40aff1edffeccc412cde93894196dca7bc739e change-id: 20240908-regmap-config-const-2b6e126feb8b Best regards, -- Krzysztof Kozlowski
Re: [PATCH v6 2/3] dt-bindings: display: rockchip: Add schema for RK3588 HDMI TX Controller
On Fri, Sep 06, 2024 at 04:17:41AM +0300, Cristian Ciocaltea wrote: > Rockchip RK3588 SoC integrates the Synopsys DesignWare HDMI 2.1 > Quad-Pixel (QP) TX controller IP. > > Since this is a new IP block, quite different from those used in the > previous generations of Rockchip SoCs, add a dedicated binding file. > > Signed-off-by: Cristian Ciocaltea > --- > .../rockchip/rockchip,rk3588-dw-hdmi-qp.yaml | 189 > + > 1 file changed, 189 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml > > b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml > new file mode 100644 > index ..37467685621d > --- /dev/null > +++ > b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml ... > + > + power-domains: > +maxItems: 1 > + > + resets: > +minItems: 2 You can drop minItems. > +maxItems: 2 > + > + reset-names: > +items: > + - const: ref > + - const: hdp Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v6 1/2] dt-bindings: display: Add Sharp Memory LCD bindings
On 05/09/2024 22:27, Alex Lanzano wrote: > On Thu, Sep 05, 2024 at 03:23:20PM GMT, Krzysztof Kozlowski wrote: >> On 05/09/2024 14:43, Alex Lanzano wrote: >>> Add device tree bindings for the monochrome Sharp Memory LCD >>> >>> Co-developed-by: Mehdi Djait >>> Signed-off-by: Mehdi Djait >>> Signed-off-by: Alex Lanzano >> >> I don't understand what happened here. Your process of handling patches >> is odd. Tags do not disappear, you had to remove them, right? So where >> is the explanation for this? > > Whoops! My apologies for wasting time. Nothing changed in this patch > I forgot to add in your reviewed-by tag. Tag was there before, so you removed it... Best regards, Krzysztof
Re: [PATCH v6 1/2] dt-bindings: display: Add Sharp Memory LCD bindings
On 05/09/2024 14:43, Alex Lanzano wrote: > Add device tree bindings for the monochrome Sharp Memory LCD > > Co-developed-by: Mehdi Djait > Signed-off-by: Mehdi Djait > Signed-off-by: Alex Lanzano I don't understand what happened here. Your process of handling patches is odd. Tags do not disappear, you had to remove them, right? So where is the explanation for this? Best regards, Krzysztof
Re: [PATCH v2 09/11] dt-bindings: display: vop2: Add rk3576 support
On 04/09/2024 14:02, Andy Yan wrote: > From: Andy Yan > > Add vop found on rk3576. > > Signed-off-by: Andy Yan > > --- > > Changes in v2: > - Add dt bindings > > .../devicetree/bindings/display/rockchip/rockchip-vop2.yaml | 1 + > 1 file changed, 1 insertion(+) > > diff --git > a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > index 2531726af306..23b2371aea46 100644 > --- a/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > +++ b/Documentation/devicetree/bindings/display/rockchip/rockchip-vop2.yaml > @@ -21,6 +21,7 @@ properties: >- rockchip,rk3566-vop >- rockchip,rk3568-vop >- rockchip,rk3588-vop > + - rockchip,rk3576-vop Keep things ordered. Same applies to your other patches. Best regards, Krzysztof
Re: [PATCH] dt-bindings: lcdif: Document the dmas/dma-names properties
On Tue, Sep 03, 2024 at 01:27:29PM -0300, Fabio Estevam wrote: > From: Fabio Estevam > > i.MX28 has an RX DMA channel associated with the LCDIF controller. > > Document the 'dmas' and 'dma-names' properties to fix the following > dt-schema warnings: > > lcdif@8003: 'dma-names', 'dmas' do not match any of the regexes: > 'pinctrl-[0-9]+' > > Signed-off-by: Fabio Estevam > --- > .../bindings/display/fsl,lcdif.yaml | 19 +++ > 1 file changed, 19 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > index 0681fc49aa1b..dd462abd61f8 100644 > --- a/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > +++ b/Documentation/devicetree/bindings/display/fsl,lcdif.yaml > @@ -50,6 +50,14 @@ properties: >- const: disp_axi > minItems: 1 > > + dmas: > +items: > + - description: DMA specifier for the RX DMA channel. > + > + dma-names: > +items: > + - const: rx > + >interrupts: > items: >- description: LCDIF DMA interrupt > @@ -156,6 +164,17 @@ allOf: > interrupts: >maxItems: 1 > > + - if: > + not: > +properties: > + compatible: > +contains: > + enum: > +- fsl,imx28-lcdif > +then: > + properties: > +dmas: false > +dma-names: false Missing blank line. Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v1 2/2] arm64: dts: mediatek: Add MT8186 Ponyta Chromebooks
On Mon, Sep 02, 2024 at 08:55:02PM +0800, Jianeng Ceng wrote: > MT8186 ponyta, known as huaqin custom lable, is a > MT8186 based laptop. It is based on the "corsola" design. > It includes LTE, touchpad combinations. > > Signed-off-by: Jianeng Ceng > --- > arch/arm64/boot/dts/mediatek/Makefile | 2 + > .../mediatek/mt8186-corsola-ponyta-sku0.dts | 24 ++ > .../mediatek/mt8186-corsola-ponyta-sku1.dts | 27 > .../dts/mediatek/mt8186-corsola-ponyta.dtsi | 44 +++ This does not even build... never tested. Best regards, Krzysztof
Re: [PATCH v2 0/2] arm64: dts: mediatek: Add MT8186 Ponyta
On Tue, Sep 03, 2024 at 02:16:01PM +0800, Jianeng Ceng wrote: > This is v2 of the MT8186 Chromebook device tree series. > Where is the changelog? Why do you send the same patch as v2? Best regards, Krzysztof
Re: [PATCH v2 2/2] arm64: dts: mediatek: Add MT8186 Ponyta Chromebooks
On Tue, Sep 03, 2024 at 02:16:03PM +0800, Jianeng Ceng wrote:
> MT8186 ponyta, known as huaqin custom lable, is a
> MT8186 based laptop. It is based on the "corsola" design.
> It includes LTE, touchpad combinations.
>
> Signed-off-by: Jianeng Ceng
> ---
> arch/arm64/boot/dts/mediatek/Makefile | 2 +
> .../mediatek/mt8186-corsola-ponyta-sku0.dts | 24 ++
> .../mediatek/mt8186-corsola-ponyta-sku1.dts | 27
> .../dts/mediatek/mt8186-corsola-ponyta.dtsi | 44 +++
> 4 files changed, 97 insertions(+)
> create mode 100644
> arch/arm64/boot/dts/mediatek/mt8186-corsola-ponyta-sku0.dts
> create mode 100644
> arch/arm64/boot/dts/mediatek/mt8186-corsola-ponyta-sku1.dts
> create mode 100644 arch/arm64/boot/dts/mediatek/mt8186-corsola-ponyta.dtsi
>
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile
> b/arch/arm64/boot/dts/mediatek/Makefile
> index 8fd7b2bb7a15..50b5cf04d3ae 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -58,6 +58,8 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt8183-pumpkin.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-magneton-sku393216.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-magneton-sku393217.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-magneton-sku393218.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-ponyta-sku0.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-ponyta-sku1.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-rusty-sku196608.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-steelix-sku131072.dtb
> dtb-$(CONFIG_ARCH_MEDIATEK) += mt8186-corsola-steelix-sku131073.dtb
> diff --git a/arch/arm64/boot/dts/mediatek/mt8186-corsola-ponyta-sku0.dts
> b/arch/arm64/boot/dts/mediatek/mt8186-corsola-ponyta-sku0.dts
> new file mode 100644
> index ..87e8368189d9
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8186-corsola-ponyta-sku0.dts
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright 2023 Google LLC
> + */
> +
> +/dts-v1/;
> +#include "mt8186-corsola-ponyta.dtsi"
> +
> +/ {
> + model = "Google Ponyta sku0/unprovisioned board";
> + compatible = "google,ponyta-sku0", "google,ponyta-sku2147483647",
> + "google,ponyta", "mediatek,mt8186";
> +};
> +
> +&i2c2 {
> + touchpad@2c {
> + compatible = "hid-over-i2c";
> + reg = <0x2c>;
> + hid-descr-addr = <0x20>;
> + interrupts-extended = <&pio 11 IRQ_TYPE_LEVEL_LOW>;
> + vcc-supply = <&pp3300_s3>;
> + wakeup-source;
> + };
> +};
> diff --git a/arch/arm64/boot/dts/mediatek/mt8186-corsola-ponyta-sku1.dts
> b/arch/arm64/boot/dts/mediatek/mt8186-corsola-ponyta-sku1.dts
> new file mode 100644
> index ..203ee109bbf7
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt8186-corsola-ponyta-sku1.dts
> @@ -0,0 +1,27 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright 2023 Google LLC
> + */
> +
> +/dts-v1/;
> +#include "mt8186-corsola-ponyta.dtsi"
> +
> +/ {
> + model = "Google Ponyta sku1 board";
> + compatible = "google,ponyta-sku1", "google,ponyta", "mediatek,mt8186";
> +};
> +
> +&i2c2 {
> + touchpad@15 {
> + compatible = "elan,ekth3000";
> + reg = <0x15>;
> + interrupt-parent = <&pio>;
> + interrupts = <11 IRQ_TYPE_LEVEL_LOW>;
Be consistent... Look at your other node.
> + vcc-supply = <&pp3300_s3>;
> + wakeup-source;
> + };
> +};
> +
Re: [PATCH v2 1/2] dt-bindings: arm: mediatek: Add MT8186 Ponyta Chromebook
On Tue, Sep 03, 2024 at 02:16:02PM +0800, Jianeng Ceng wrote: > Add an entry for the MT8186 based Ponyta Chromebook (custom lable). > > Signed-off-by: Jianeng Ceng > --- > Documentation/devicetree/bindings/arm/mediatek.yaml | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml > b/Documentation/devicetree/bindings/arm/mediatek.yaml > index 1d4bb50fcd8d..410145976272 100644 > --- a/Documentation/devicetree/bindings/arm/mediatek.yaml > +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml > @@ -257,6 +257,17 @@ properties: >- const: google,steelix-sku393218 >- const: google,steelix >- const: mediatek,mt8186 > + - description: Google Ponyta (Custom lable) lable? label? What is this? > +items: > + - const: google,ponyta-sku0 > + - const: google,ponyta-sku2147483647 maxint? Really? Best regards, Krzysztof
Re: [PATCH] docs: devicetree: Fix typo in lvds.yaml
On Sun, Sep 01, 2024 at 09:30:46PM +0800, Yu-Chun Lin wrote: > Corrected the spelling in the description of LVDS Display Common > Properties. > > Signed-off-by: Yu-Chun Lin > --- > Documentation/devicetree/bindings/display/lvds.yaml | 2 +- Replacing one typo in one place is a churn. Fix several typos. You engaged three people to review this, so three people spent their time on reviewing and answering this trivial stuff. Please use subject prefixes matching the subsystem. You can get them for example with on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters Best regards, Krzysztof
Re: [PATCH v5 1/4] dt-bindings: display: bridge: Add schema for Synopsys DW HDMI QP TX IP
On 31/08/2024 15:58, Heiko Stübner wrote: > > so I guess the fifth interrupt is meant to be the hotplug? > Though I guess this should be specificed in the name-list too. > > From the SoC's manual it looks like the controller is set up from > different modules. > Like AVP is the audio-video-packet-module, there is a Main and CEC Module > as well as a eARC RX controller inside. I'd guess it might be possible > other SoC vendors could leave out specific modules? > > > TL;DR I think those clocks and interrupts are dependent on how the > IP core was synthesized, so for now I'd think we can only guarantee > that they are true for rk3588 and rk3576. > > So I guess they should move to the rockchip-specific part of the binding > until we have more hdmi-qp controllers in the field? Which would leave empty "common" binding. Best regards, Krzysztof
Re: [PATCH v5? 5/6] dt-bindings: display: rockchip: Add schema for RK3588 DW HDMI QP TX machine
On 30/08/2024 17:28, Shimrra Shai wrote: > diff --git > a/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml > > b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml > new file mode 100644 > index 0..e71544ced > --- /dev/null Nope, please carefully read submitting patches. Best regards, Krzysztof
Re: [PATCH v5? 4/6] dt-bindings: soc: rockchip: Document VO0/1 GRF compatible string changes
On 30/08/2024 17:28, Shimrra Shai wrote: > diff --git a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml > b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml > index 78c6d5b64..8fd539125 100644 > --- a/Documentation/devicetree/bindings/soc/rockchip/grf.yaml > +++ b/Documentation/devicetree/bindings/soc/rockchip/grf.yaml > @@ -31,7 +31,8 @@ properties: >- rockchip,rk3588-pcie3-pipe-grf >- rockchip,rk3588-usb-grf >- rockchip,rk3588-usbdpphy-grf > - - rockchip,rk3588-vo-grf > + - rockchip,rk3588-vo0-grf > + - rockchip,rk3588-vo1-grf >- rockchip,rk3588-vop-grf NAK, this does not make any sense. Just random code changes... Best regards, Krzysztof
Re: [PATCH v5? 3/6] dt-bindings: display: bridge: Add schema for Synopsys DW HDMI QP TX IP
On 30/08/2024 17:28, Shimrra Shai wrote: > diff --git > a/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml > b/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml > new file mode 100644 > index 0..141899ba2 What is this? Where is proper message? Why are you sending someone's else work duplicating entire review effort? Best regards, Krzysztof
Re: [PATCH v5 1/4] dt-bindings: display: bridge: Add schema for Synopsys DW HDMI QP TX IP
On Sat, Aug 31, 2024 at 12:55:29AM +0300, Cristian Ciocaltea wrote: > Add dt-binding schema containing the common properties for the Synopsys > DesignWare HDMI QP TX controller. > > Note this is not a full dt-binding specification, but is meant to be > referenced by platform-specific bindings for this IP core. > > Signed-off-by: Cristian Ciocaltea > --- > .../display/bridge/synopsys,dw-hdmi-qp.yaml| 88 > ++ > 1 file changed, 88 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml > b/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml > new file mode 100644 > index ..771f7fba6c50 > --- /dev/null > +++ > b/Documentation/devicetree/bindings/display/bridge/synopsys,dw-hdmi-qp.yaml > @@ -0,0 +1,88 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/synopsys,dw-hdmi-qp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Common Properties for Synopsys DesignWare HDMI QP TX Controller IP > + > +maintainers: > + - Cristian Ciocaltea > + > +description: | > + The Synopsys DesignWare HDMI 2.1 Quad-Pixel (QP) TX Controller IP core > + supports the following features, among others: > + > + * Fixed Rate Link (FRL) > + * Display Stream Compression (DSC) > + * 4K@120Hz and 8K@60Hz video modes > + * Variable Refresh Rate (VRR) including Quick Media Switching (QMS) > + * Fast Vactive (FVA) > + * SCDC I2C DDC access > + * Multi-stream audio > + * Enhanced Audio Return Channel (EARC) > + > + Note this is not a full dt-binding specification, but is meant to be > + referenced by platform-specific bindings for this IP core. > + > +properties: > + reg: > +maxItems: 1 > + > + clocks: > +minItems: 4 > +maxItems: 6 > +items: > + - description: Peripheral/APB bus clock > + - description: EARC RX biphase clock > + - description: Reference clock > + - description: Audio interface clock > +additionalItems: true What is the usefulness of all this? How can you even be sure that each implementation of this core will have exactly these clocks? > + > + clock-names: > +minItems: 4 > +maxItems: 6 > +items: > + - const: pclk > + - const: earc > + - const: ref > + - const: aud > +additionalItems: true > + > + interrupts: > +minItems: 4 > +maxItems: 5 > +items: > + - description: AVP Unit interrupt > + - description: CEC interrupt > + - description: eARC RX interrupt > + - description: Main Unit interrupt If these are real pins, then this seems more possible, but additionalItems does not make me happy. I don't see much value in this schema and I am afraid even enforcing clock and interrupt names won't work for the second or third user. Best regards, Krzysztof
Re: [PATCH v5 3/4] dt-bindings: display: rockchip: Add schema for RK3588 HDMI TX Controller
On Sat, Aug 31, 2024 at 12:55:31AM +0300, Cristian Ciocaltea wrote:
> Rockchip RK3588 SoC integrates the Synopsys DesignWare HDMI 2.1
> Quad-Pixel (QP) TX controller IP.
>
> Since this is a new IP block, quite different from those used in the
> previous generations of Rockchip SoCs, add a dedicated binding file.
>
> Signed-off-by: Cristian Ciocaltea
> ---
> .../rockchip/rockchip,rk3588-dw-hdmi-qp.yaml | 166
> +
> 1 file changed, 166 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml
>
> b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml
> new file mode 100644
> index ..d2919ff6aa23
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml
> @@ -0,0 +1,166 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id:
> http://devicetree.org/schemas/display/rockchip/rockchip,rk3588-dw-hdmi-qp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip DW HDMI QP TX Encoder
> +
> +maintainers:
> + - Cristian Ciocaltea
> +
> +description:
> + Rockchip RK3588 SoC integrates the Synopsys DesignWare HDMI QP TX
> controller
> + IP and a HDMI/eDP TX Combo PHY based on a Samsung IP block.
> +
> +allOf:
> + - $ref: /schemas/display/bridge/synopsys,dw-hdmi-qp.yaml#
> + - $ref: /schemas/sound/dai-common.yaml#
> +
> +properties:
> + compatible:
> +enum:
> + - rockchip,rk3588-dw-hdmi-qp
> +
> + clocks:
> +items:
> + - {}
> + - {}
> + - {}
> + - {}
> + - description: TMDS/FRL link clock
> + - description: Video datapath clock
Please define all clocks.
> +
> + clock-names:
> +items:
> + - {}
> + - {}
> + - {}
> + - {}
> + - enum: [hdp, hclk_vo1]
> + - const: hclk_vo1
> +
> + interrupts:
> +items:
> + - {}
> + - {}
> + - {}
> + - {}
> + - description: HPD interrupt
> +
> + interrupt-names:
> +items:
> + - {}
> + - {}
> + - {}
> + - {}
> + - const: hpd
> +
> + phys:
> +maxItems: 1
> +description: The HDMI/eDP PHY.
> +
> + phy-names:
> +const: hdmi
Drop phy-names, not really useful if it copies the name of the block.
> +
> + power-domains:
> +maxItems: 1
> +
> + resets:
> +minItems: 2
> +maxItems: 2
> +
> + reset-names:
> +items:
> + - const: ref
> + - const: hdp
> +
> + "#sound-dai-cells":
> +const: 0
> +
> + rockchip,grf:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description:
> + Some HDMI QP related data is accessed through SYS GRF regs.
> +
> + rockchip,vo-grf:
> +$ref: /schemas/types.yaml#/definitions/phandle
> +description:
> + Additional HDMI QP related data is accessed through VO GRF regs.
> +
> +required:
> + - compatible
> + - reg
> + - clocks
> + - clock-names
> + - interrupts
> + - interrupt-names
> + - phys
> + - phy-names
> + - ports
> + - resets
> + - reset-names
> + - rockchip,grf
> + - rockchip,vo-grf
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + hdmi@fde8 {
> +compatible = "rockchip,rk3588-dw-hdmi-qp";
> +reg = <0x0 0xfde8 0x0 0x2>;
> +clocks = <&cru PCLK_HDMITX0>,
> + <&cru CLK_HDMITX0_EARC>,
> + <&cru CLK_HDMITX0_REF>,
> + <&cru MCLK_I2S5_8CH_TX>,
> + <&cru CLK_HDMIHDP0>,
> + <&cru HCLK_VO1>;
> +clock-names = "pclk", "earc", "ref", "aud", "hdp", "hclk_vo1";
> +interrupts = ,
> + ,
> + ,
> + ,
> + ;
> +interrupt-names = "avp", "cec", "earc", "main", "hpd";
> +phys = <&hdptxphy_hdmi0>;
> +phy-names = "hdmi";
> +power-domains = <&power RK3588_PD_VO1>;
> +resets = <&cru SRST_HDMITX0_REF>, <&cru SRST_HDMIHDP0>;
> +reset-names = "ref", "hdp";
> +rockchip,grf = <&sys_grf>;
> +rockchip,vo-grf = <&vo1_grf>;
> +#sound-dai-cells = <0>;
> +
> +ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> +reg = <0>;
> +
> +hdmi0_in_vp0: endpoint {
> +remote-endpoint = <&vp0_out_hdmi0>;
Messed indentation.
Best regards,
Krzysztof
Re: [RFC PATCH 3/4] dt-bindings: reserved-memory: add linaro,restricted-heap
On Fri, Aug 30, 2024 at 09:03:50AM +0200, Jens Wiklander wrote:
> From: Olivier Masse
>
> DMABUF reserved memory definition for OP-TEE secure data path feature.
>
> Signed-off-by: Olivier Masse
> Signed-off-by: Jens Wiklander
> ---
> .../linaro,restricted-heap.yaml | 56 +++
> 1 file changed, 56 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/reserved-memory/linaro,restricted-heap.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/reserved-memory/linaro,restricted-heap.yaml
>
> b/Documentation/devicetree/bindings/reserved-memory/linaro,restricted-heap.yaml
> new file mode 100644
> index ..0ab87cf02775
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/reserved-memory/linaro,restricted-heap.yaml
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id:
> http://devicetree.org/schemas/reserved-memory/linaro,restricted-heap.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Linaro Secure DMABUF Heap
> +
> +maintainers:
> + - Olivier Masse
> +
> +description:
> + Linaro OP-TEE firmware needs a reserved memory for the
> + Secure Data Path feature (aka SDP).
> + The purpose is to provide a restricted memory heap which allow
> + the normal world OS (REE) to allocate/free restricted buffers.
> + The TEE is reponsible for protecting the SDP memory buffers.
> + TEE Trusted Application can access restricted memory references
> + provided as parameters (DMABUF file descriptor).
And what is the difference from regular reserved memory? Why it cannot
be used?
> +
> +allOf:
> + - $ref: "reserved-memory.yaml"
It does not look like you tested the bindings, at least after quick
look. Please run (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.
> +
> +properties:
> + compatible:
> +const: linaro,restricted-heap
> +
> + reg:
> +description:
> + Region of memory reserved for OP-TEE SDP feature
> +
> + no-map:
> +$ref: /schemas/types.yaml#/definitions/flag
> +description:
> + Avoid creating a virtual mapping of the region as part of the OS'
> + standard mapping of system memory.
> +
> +unevaluatedProperties: false
This goes after "required:" block.
> +
> +required:
> + - compatible
> + - reg
> + - no-map
> +
> +examples:
> + - |
> + reserved-memory {
> +#address-cells = <2>;
> +#size-cells = <2>;
> +
> +sdp@3e80 {
> + compatible = "linaro,restricted-heap";
> + no-map;
> + reg = <0 0x3E80 0 0x0040>;
lowercase hex
Best regards,
Krzysztof
Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
On 28/08/2024 14:45, Laurent Pinchart wrote: > Hi Krzysztof, > > On Sun, Aug 18, 2024 at 08:48:54PM +0200, Krzysztof Kozlowski wrote: >> On 18/08/2024 19:51, Laurent Pinchart wrote: >>> On Sun, Aug 18, 2024 at 07:44:22PM +0200, Krzysztof Kozlowski wrote: >>>> On 18/08/2024 19:41, Laurent Pinchart wrote: >>>>> On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote: >>>>>> Each variable-length property like interrupts or resets must have fixed >>>>>> constraints on number of items for given variant in binding. The >>>>>> clauses in "if:then:" block should define both limits: upper and lower. >>>>> >>>>> I thought that, when only one of minItems or maxItems was specified, the >>>>> other automatically defaulted to the same value. I'm pretty sure I >>>>> recall Rob asking me to drop one of the two in some bindings. Has the >>>>> rule changes ? Is it documented somewhere ? >>>> >>>> New dtschema changed it and, even if previous behavior is restored, the >>>> size in if:then: always had to be constrained. You could have skipped >>>> one side of limit if it was equal to outer/top-level limit, e.g: >>>> >>>> properties: >>>> clocks: >>>> minItems: 1 >>>> maxItems: 2 >>>> >>>> >>>> if:then:properties: >>>> clocks: >>>> minItems: 2 >>> >>> Where can I find a description of the behaviour of the new dtschema >>> (hopefully with some documentation) ? >> >> No clue, but I feel there is some core concept missing. Your earlier >> statement: >> "I thought that, when only one of minItems or maxItems was specified, the" >> >> was never logically correct for the "if:then", except for the case I >> mentioned above. That's why all schema used as examples had it explicit: >> >> My talk from 2022, page 30: >> https://static.sched.com/hosted_files/osseu2022/bd/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro.pdf?_gl=1*kmzqmt*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1 >> all constraints defined,. >> >> My talk from 2023, page 34: >> https://static.sched.com/hosted_files/eoss2023/a8/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro%20-%20ELCE%202023.pdf?_gl=1*1jgx6d3*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1 >> >> Recently, I started using other example as "useful reference": >> https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132 >> >> That's nothing. All three above reference examples I keep giving are >> already there and repeated in emails all the time. >> >> So aren't you confusing the entire "skip one limit" for top-level >> properties? This patch is not about it all and dtschema did not change. > > There must have been a misunderstanding indeed, I interpreted "New > dtschema changed it" as meaning there were now new rules. Is that > incorrect ? For the binding with a property defined only in top-level properties: no changes, no new rules. For the binding with top-level and if:then:else: dtschema since few months changed interpretation. > > If you don't mind clarifying, what is the current recommendation to > indicate that a property has a fixed number of items ? Which of the > following three options is preferred ? > Answer below assumes we have clocks defined in top-level properties and there is no if:then:else customizing it. > properties: > clocks: > minItems: 2 That's wrong, because items are unconstrained. > > properties: > clocks: > maxItems: 2 This one is preferred. > > properties: > clocks: > minItems: 2 > maxItems: 2 This one is correct, but less preferred. Best regards, Krzysztof
Re: [PATCH v3] dt-bindings: display: st,stm32-ltdc: Document stm32mp25 compatible
On 27/08/2024 16:04, Yannick Fertre wrote: > Add "st,stm32mp25-ltdc" compatible for SOC MP25. This new SOC introduce > new clocks (bus, ref & lvds). Bus clock was separated from lcd clock. > New sources are possible for lcd clock (lvds / ref). > > Signed-off-by: Yannick Fertre > --- > > Changes in v3: Add max/min Items fields. > 'make dt_binding_check' command fails on previous patch, rework fiedls > mas/min items > of properties clocks & clock-names. > > Changes in v2: Rework clock property. > > .../bindings/display/st,stm32-ltdc.yaml | 28 +++ > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/st,stm32-ltdc.yaml > b/Documentation/devicetree/bindings/display/st,stm32-ltdc.yaml > index d6ea4d62a2cf..940127820de3 100644 > --- a/Documentation/devicetree/bindings/display/st,stm32-ltdc.yaml > +++ b/Documentation/devicetree/bindings/display/st,stm32-ltdc.yaml > @@ -12,7 +12,9 @@ maintainers: > > properties: >compatible: > -const: st,stm32-ltdc > +enum: > + - st,stm32-ltdc > + - st,stm32mp25-ltdc > >reg: > maxItems: 1 > @@ -24,12 +26,12 @@ properties: > minItems: 1 > >clocks: > -maxItems: 1 > +minItems: 1 > +maxItems: 4 > >clock-names: > -items: > - - const: lcd > - > +minItems: 1 > +maxItems: 4 Keep the blank line. >resets: > maxItems: 1 > > @@ -51,6 +53,22 @@ required: >- resets >- port > > +allOf: > + - if: > + properties: > +compatible: > + contains: > +enum: > + - st,stm32mp25-ltdc > +then: > + properties: > +clocks: > + minItems: 2 Instead, describe the items. Missing clock-names > +else: > + properties: > +clocks: > + minItems: 1 minItems? Why are you changing existing device? Nothing in commit msg explains this. Best regards, Krzysztof
Re: [PATCH v3 3/4] dt-bindings: display: ti,am65x-dss: Add OLDI properties for AM625 DSS
On 26/08/2024 09:32, Aradhya Bhatia wrote: > Hi Krzysztof, > > > On 7/21/24 21:09, Krzysztof Kozlowski wrote: >> On 16/07/2024 10:42, Aradhya Bhatia wrote: >>> The DSS in AM625 SoC has 2 OLDI TXes. Refer the OLDI schema to add the >>> support for the OLDI TXes. >>> >>> The AM625 DSS VP1 (port@0) can connect and control 2 OLDI TXes, to use >>> them in dual-link or cloned single-link OLDI modes. Add support for an >>> additional endpoint under the port@0 to accurately depict the data flow >>> path. >>> >>> Signed-off-by: Aradhya Bhatia >>> --- >>> .../bindings/display/ti/ti,am65x-dss.yaml | 135 ++ >>> 1 file changed, 135 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml >>> b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml >>> index 399d68986326..249597455d34 100644 >>> --- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml >>> +++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml >>> @@ -91,6 +91,24 @@ properties: >>>For AM625 DSS, the internal DPI output port node from video >>>port 1. >>>For AM62A7 DSS, the port is tied off inside the SoC. >>> +properties: >>> + endpoint@0: >>> +$ref: /schemas/graph.yaml#/properties/endpoint >>> +description: >>> + For AM625 DSS, VP Connection to OLDI0. >>> + For AM65X DSS, OLDI output from the SoC. >>> + >>> + endpoint@1: >>> +$ref: /schemas/graph.yaml#/properties/endpoint >>> +description: >>> + For AM625 DSS, VP Connection to OLDI1. >> >> Eh, that's confusing. Why do you have graph to your children? Isn't this >> entirely pointless? > > I am not sure I fully understand. The same display source video port can > connect up to 2 OLDI TXes - hence 2 endpoints which connect to the OLDI > that were described in the previous patch. The idea has been to > accurately depict the connections of the hardware. > > What am I missing here? You are missing the explanation: why do you need to represent internal parts of a device with graph. Where does this endpoint point? Provide some diagram showing the architecture, because either it is wrong or I do not understand what hardware you want to represent here. > > > side-note: I do realize, as I write this, that it has been quite a while > since you reviewed, and that you may have, rightfully, lost context. > I apologize for that. > >> >>> + >>> +anyOf: >>> + - required: >>> + - endpoint >>> + - required: >>> + - endpoint@0 >>> + - endpoint@1 >>> >>>port@1: >>> $ref: /schemas/graph.yaml#/properties/port >>> @@ -112,6 +130,23 @@ properties: >>>Input memory (from main memory to dispc) bandwidth limit in >>>bytes per second >>> >>> + oldi-txes: >>> +type: object >>> +additionalProperties: true >> >> Why? This looks wrong. > > This, I will admit, was a shot in the dark. The binding check asked me > that I was missing either this or unevaluatedProperties. I tried to make > sense of the two, but with little luck. Eventually, I went with this. > > I could change it to unevaluatedProperties if that is indeed correct. I > could also use some comprehensive resource to understand this, if you > have something to recommend. =) This must be additionalProperties false. See example schema or writing schema. Best regards, Krzysztof
Re: [PATCH v3 2/4] dt-bindings: display: ti: Add schema for AM625 OLDI Transmitter
On 26/08/2024 07:47, Aradhya Bhatia wrote: > Hi Krzysztof, > > Thank you for the reviewing the patches. > > > On 7/21/24 21:06, Krzysztof Kozlowski wrote: >> On 16/07/2024 10:42, Aradhya Bhatia wrote: >>> The OLDI (transmitters) TXes do not have registers of their own, and are >>> dependent on the source video-ports from the DSS to provide >>> configuration data. This hardware doesn't directly sit on the internal >>> bus of the SoC, but does so via the DSS. Hence, the OLDI TXes are >>> supposed to be child nodes under the DSS, and not independent devices. >>> >>> Two of the OLDI TXes can function in tandem to output dual-link OLDI >>> output, or cloned single-link outputs. In these cases, one OLDI will be >>> the primary OLDI, and the other one, a companion. >>> >>> The OLDI functionality is further supported by a system-control module, >>> which contains a few registers to control OLDI IO power and >>> characteristics. >>> >>> Add devicetree binding schema for AM625 OLDI TXes. >>> >>> Signed-off-by: Aradhya Bhatia >>> --- >>> .../bindings/display/ti/ti,am625-oldi.yaml| 153 ++ >>> MAINTAINERS | 1 + >>> 2 files changed, 154 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml >>> >>> diff --git >>> a/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml >>> b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml >>> new file mode 100644 >>> index ..0a96e600bc0b >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/display/ti/ti,am625-oldi.yaml >>> @@ -0,0 +1,153 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/display/ti/ti,am625-oldi.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Texas Instruments AM625 OLDI Transmitter >>> + >>> +maintainers: >>> + - Tomi Valkeinen >>> + - Aradhya Bhatia >>> + >>> +description: | >> >> Do not need '|' unless you need to preserve formatting. > > Okay! > >> >>> + The AM625 TI Keystone OpenLDI transmitter (OLDI TX) supports serialized >>> RGB >>> + pixel data transmission between host and flat panel display over LVDS >>> (Low >>> + Voltage Differential Sampling) interface. The OLDI TX consists of 7-to-1 >>> data >>> + serializers, and 4-data and 1-clock LVDS outputs. It supports the LVDS >>> output >>> + formats "jeida-18", "jeida-24" and "vesa-18", and can accept 24-bit RGB >>> or >>> + padded and un-padded 18-bit RGB bus formats as input. >>> + >>> +properties: >>> + reg: >>> +maxItems: 1 >>> + >> >> How does it even work without compatible? How is this schema selected? >> If by part of your next patch, then this is not a proper split - this >> patch itself is noop. Squash the patches. >> > > Yes, it is supposed to be picked like the next patch does it. I can > squash these both. > >>> + clocks: >>> +maxItems: 1 >>> +description: serial clock input for the OLDI transmitters >>> + >>> + clock-names: >>> +const: s_clk >> >> Drop _clk or name it correctly. > > Alright! > >> >>> + >>> + ti,companion-oldi: >>> +$ref: /schemas/types.yaml#/definitions/phandle >>> +description: >>> + phandle to companion OLDI transmitter. This property is mandatory >>> for the >>> + primarty OLDI TX if the OLDI TXes are expected to work either in >>> dual-lvds >>> + mode or in clone mode. This property should point to the secondary >>> OLDI >>> + TX. >>> + >>> + ti,secondary-oldi: >>> +type: boolean >>> +description: Boolean property to mark an OLDI TX as secondary node. >> >> Why? Lack companion tells it, doesn't it? > > A lack of companion doesn't mean secondary-OLDI automatically, actually. > > There is also a possible configuration where 2 OLDI TXes could be > individually connected to 2 different sources => 2x single Link > configuration. The OLDI TXes would then work independently. You are responding fo
Re: [PATCH v3 1/9] of: property: add of_graph_get_next_port()
On Mon, Aug 26, 2024 at 02:43:23AM +, Kuninori Morimoto wrote:
> We have endpoint base functions
> - of_graph_get_next_device_endpoint()
> - of_graph_get_device_endpoint_count()
> - for_each_of_graph_device_endpoint()
> + if (!prev) {
> + /*
> + * Find "ports" node from parent
> + *
> + * parent {
> + * => ports {
> + * port {...};
> + * };
> + * };
> + */
> + prev = of_get_child_by_name(parent, "ports");
> +
> + /*
> + * Use parent as its ports if it not exist
> + *
> + * => parent {
> + * port {...};
> + * };
> + */
> + if (!prev) {
> + prev = of_node_get(parent);
> +
> + /* check wether it has port node */
> + struct device_node *port __free(device_node) =
> + of_get_child_by_name(prev, "port");
> +
> + if (!port)
> + prev = NULL;
It looks like you leak here "prev".
> + }
> +
> + return prev;
> + }
Best regards,
Krzysztof
[PATCH] dt-bindings: MAINTAINERS: drop stale exynos file pattern
With last TXT binding converted to DT schema, all Samsung Exynos display
bindings are in "samsung" directory, already present in maintainers
entry. Drop old "exynos" directory to fix get_maintainers.pl self-test
warning:
./MAINTAINERS:7539: warning: no file matches F:
Documentation/devicetree/bindings/display/exynos/
Fixes: ad6d17e10306 ("dt-bindings: display: samsung,exynos5-dp: convert to DT
Schema")
Signed-off-by: Krzysztof Kozlowski
---
MAINTAINERS | 1 -
1 file changed, 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 028186bb4e8d..c75918994a53 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7536,7 +7536,6 @@ M:Kyungmin Park
L: dri-devel@lists.freedesktop.org
S: Supported
T: git git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git
-F: Documentation/devicetree/bindings/display/exynos/
F: Documentation/devicetree/bindings/display/samsung/
F: drivers/gpu/drm/exynos/
F: include/uapi/drm/exynos_drm.h
--
2.43.0
Re: [PATCH v2 10/12] dt-bindings: spi: Add rockchip,rk3576-spi compatible
On Fri, Aug 23, 2024 at 12:07:10PM -0400, Detlev Casanova wrote: > It is compatible with the rockchip,rk3066-spi SPI core. Same comments... subject: spi: dt-bindings: Please use subject prefixes matching the subsystem. You can get them for example with on the directory your patch is touching. For bindings, the preferred subjects are explained here: https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 09/12] dt-bindings: watchdog: Add rockchip,rk3576-wdt compatible
On Fri, Aug 23, 2024 at 10:52:36AM -0400, Detlev Casanova wrote: > It is compatible with the other rockchip SoCs. > > Signed-off-by: Detlev Casanova Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 08/12] dt-bindings: gpu: Add rockchip,rk3576-mali compatible
On Fri, Aug 23, 2024 at 10:52:35AM -0400, Detlev Casanova wrote: > Add the rockchip,rk3576-mali in arm,mali-bifrost.yaml This we see from the diff. And from commit subject. You have here plenty of space to explain shortly the hardware, e.g. it's new and not compatible with existing. > > Signed-off-by: Detlev Casanova > --- > Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 1 + Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 07/12] dt-bindings: mmc: Add support for rk3576 eMMC
On Fri, Aug 23, 2024 at 10:52:34AM -0400, Detlev Casanova wrote: > The device is compatible with rk3588, so add an entry for the 2 > compatibles together. > > The rk3576 device has a power-domain that needs to be on for the eMMC to > be used. Add it as a requirement. > > Signed-off-by: Detlev Casanova > --- > .../bindings/mmc/snps,dwcmshc-sdhci.yaml | 32 +-- > 1 file changed, 23 insertions(+), 9 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml > b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml > index 4d3031d9965f3..7d5e388587027 100644 > --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml > +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml > @@ -12,16 +12,29 @@ maintainers: > > allOf: >- $ref: mmc-controller.yaml# > + - if: > + properties: > +compatible: > + contains: > +const: rockchip,rk3576-dwcmshc > +then: > + properties: > +power-domains: > + minItems: 1 Plaese move the allOf: after the required: block. It grows too much with such if:then: and that's not the most important part of binding we need to see first. With above change: Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 04/12] dt-bindings: iio: adc: Add rockchip,rk3576-saradc string
On Fri, Aug 23, 2024 at 10:52:31AM -0400, Detlev Casanova wrote: > Add rockchip,rk3576-saradc compatible string. > The saradc on RK3576 is compatible with the one on RK3588, so they are > used together in an arm of the oneOf. > > Signed-off-by: Detlev Casanova > Reviewed-by: Krzysztof Kozlowski > Acked-by: Heiko Stuebner > --- Why do you keep sending the same patch which was already applied? Best regards, Krzysztof
Re: [PATCH v3 1/3] dt-bindings: display: mediatek: dpi: Add power domains
On Thu, Aug 22, 2024 at 06:46:48AM +, Rohit Agarwal wrote: > Add power domain binding to the mediatek DPI controller > for MT8186. > Also, add power domain binding for other SoCs like > MT6795 and MT8173 that already had power domain property. > > Signed-off-by: Rohit Agarwal > --- > .../bindings/display/mediatek/mediatek,dpi.yaml | 17 + Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v2 1/3] dt-bindings: display: mediatek: dpi: Add power domains
On 21/08/2024 12:00, Rohit Agarwal wrote: >>> +then: >>> + properties: >>> +power-domains: >>> + maxItems: 1 >> This part can be dropped. Just disallow it for other devices. > I was a bit confused here. > > Can we add something like this? > if: > not: > (mt6795, mt8173, mt8186) > then: > properties: > power-domains: false Yes. Look for examples (git grep). Best regards, Krzysztof
Re: [PATCH v2 1/3] dt-bindings: display: mediatek: dpi: Add power domains
On 21/08/2024 11:26, Rohit Agarwal wrote: > Add power domain binding to the mediatek DPI controller > for MT8186. > Also, add power domain binding for other SoCs like > MT6795 and MT8173 that already had power domain property. > > Signed-off-by: Rohit Agarwal > --- > .../display/mediatek/mediatek,dpi.yaml| 19 +++ > 1 file changed, 19 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > index 5ca7679d5427..864b781fdcea 100644 > --- a/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek,dpi.yaml > @@ -62,6 +62,8 @@ properties: >- const: default >- const: sleep > > + power-domains: true Missing maxItems. I don't get why did you change this... > + >port: > $ref: /schemas/graph.yaml#/properties/port > description: > @@ -76,6 +78,23 @@ required: >- clock-names >- port > > +allOf: > + - if: > + properties: > +compatible: > + contains: > +enum: > + - mediatek,mt6795-dpi > + - mediatek,mt8173-dpi > + - mediatek,mt8186-dpi > +then: > + properties: > +power-domains: > + maxItems: 1 This part can be dropped. Just disallow it for other devices. Best regards, Krzysztof
Re: [PATCH 1/3] dt-bindings: display: mediatek: dpi: Add power domains
On 20/08/2024 14:18, Rohit Agarwal wrote: > > On 20/08/24 4:40 PM, Krzysztof Kozlowski wrote: >> On Tue, Aug 20, 2024 at 08:06:57AM +, Rohit Agarwal wrote: >>> Add power domain binding to the mediatek DPI controller. >> Why? Who needs it? Why all devices suddenly have it (IOW, why is it not >> constrained anyhow per variant)? >> > Ok, my intent was to introduce only for this particular variant. > Let me constrain it to this particular compatible. Then add property in top-level properties and in if:then: disallow it for older devices. Best regards, Krzysztof
Re: [PATCH 1/3] dt-bindings: display: mediatek: dpi: Add power domains
On Tue, Aug 20, 2024 at 08:06:57AM +, Rohit Agarwal wrote: > Add power domain binding to the mediatek DPI controller. Why? Who needs it? Why all devices suddenly have it (IOW, why is it not constrained anyhow per variant)? > > Signed-off-by: Rohit Agarwal > --- > .../devicetree/bindings/display/mediatek/mediatek,dpi.yaml | 3 +++ > 1 file changed, 3 insertions(+) Best regards, Krzysztof
Re: [PATCH v2 2/2] mtd: rawnand: nuvoton: add new driver for the Nuvoton MA35 SoC
On Mon, Aug 19, 2024 at 09:20:37AM +, Hui-Ping Chen wrote:
> Nuvoton MA35 SoCs NAND Flash Interface Controller
> supports 2KB, 4KB and 8KB page size, and up to 8-bit,
> 12-bit, and 24-bit hardware ECC calculation circuit
> to protect data communication.
>
> Signed-off-by: Hui-Ping Chen
...
> +static int ma35_nand_probe(struct platform_device *pdev)
> +{
> + struct ma35_nand_info *nand;
> + struct nand_chip *chip;
> + struct mtd_info *mtd;
> + int retval = 0;
> +
> + nand = devm_kzalloc(&pdev->dev, sizeof(*nand), GFP_KERNEL);
> + if (!nand)
> + return -ENOMEM;
> +
> + nand_controller_init(&nand->controller);
> +
> + nand->regs = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(nand->regs))
> + return PTR_ERR(nand->regs);
> +
> + nand->dev = &pdev->dev;
> + chip = &nand->chip;
> + mtd = nand_to_mtd(chip);
> + nand_set_controller_data(chip, nand);
> + nand_set_flash_node(chip, pdev->dev.of_node);
> +
> + mtd->priv = chip;
> + mtd->owner = THIS_MODULE;
> + mtd->dev.parent = &pdev->dev;
> +
> + nand->clk = devm_clk_get(&pdev->dev, "nand_gate");
> + if (IS_ERR(nand->clk))
> + return dev_err_probe(&pdev->dev, PTR_ERR(nand->clk),
> + "failed to find nand clock\n");
> +
> + retval = clk_prepare_enable(nand->clk);
> + if (retval < 0) {
> + dev_err(&pdev->dev, "failed to enable clock\n");
> + retval = -ENXIO;
> + }
> +
> + nand->chip.controller= &nand->controller;
> +
> + chip->legacy.cmdfunc = ma35_nand_command;
> + chip->legacy.waitfunc= ma35_waitfunc;
> + chip->legacy.read_byte = ma35_nand_read_byte;
> + chip->legacy.select_chip = ma35_nand_select_chip;
> + chip->legacy.read_buf= ma35_read_buf_dma;
> + chip->legacy.write_buf = ma35_write_buf_dma;
> + chip->legacy.dev_ready = ma35_nand_devready;
> + chip->legacy.chip_delay = 25; /* us */
> +
> + /* Read OOB data first, then HW read page */
> + chip->ecc.hwctl = ma35_nand_enable_hwecc;
> + chip->ecc.calculate = ma35_nand_calculate_ecc;
> + chip->ecc.correct= ma35_nand_correct_data;
> + chip->ecc.write_page = ma35_nand_write_page_hwecc;
> + chip->ecc.read_page = ma35_nand_read_page_hwecc_oob_first;
> + chip->ecc.read_oob = ma35_nand_read_oob_hwecc;
> + chip->options |= (NAND_NO_SUBPAGE_WRITE | NAND_USES_DMA);
> +
> + ma35_nand_initialize(nand);
> + platform_set_drvdata(pdev, nand);
> +
> + nand->controller.ops = &ma35_nand_controller_ops;
> +
> + nand->irq = platform_get_irq(pdev, 0);
> + if (nand->irq < 0)
> + return dev_err_probe(&pdev->dev, nand->irq,
> + "failed to get platform irq\n");
> +
> + if (request_irq(nand->irq, ma35_nand_irq, IRQF_TRIGGER_HIGH,
> "ma35d1-nand", nand)) {
> + dev_err(&pdev->dev, "Error requesting NAND IRQ\n");
> + return -ENXIO;
> + }
> +
> + retval = nand_scan(chip, 1);
> + if (retval)
> + return retval;
> +
> + if (mtd_device_register(mtd, nand->parts, nand->nr_parts)) {
> + nand_cleanup(chip);
> + devm_kfree(&pdev->dev, nand);
> + return retval;
> + }
> +
> + return retval;
> +}
> +
> +static void ma35_nand_remove(struct platform_device *pdev)
> +{
> + struct ma35_nand_info *nand = platform_get_drvdata(pdev);
> + struct nand_chip *chip = &nand->chip;
> + int ret;
> +
Where do you release IRQ handler?
> + ret = mtd_device_unregister(nand_to_mtd(chip));
> + WARN_ON(ret);
> + nand_cleanup(chip);
> +
> + clk_disable_unprepare(nand->clk);
> +
> + kfree(nand);
NAK, you never tested your code.
> + platform_set_drvdata(pdev, NULL);
Why? Drop.
> +}
> +
> +/* PM Support */
> +#ifdef CONFIG_PM
> +static int ma35_nand_suspend(struct platform_device *pdev, pm_message_t pm)
> +{
> + struct ma35_nand_info *nand = platform_get_drvdata(pdev);
> + unsigned long timeo = jiffies + HZ/2;
> +
> + /* wait DMAC to ready */
> + while (1) {
> + if ((readl(nand->regs + MA35_NFI_REG_DMACTL) & DMA_BUSY) == 0)
> + break;
> + if (time_after(jiffies, timeo))
> + return -ETIMEDOUT;
> + }
> +
> + clk_disable(nand->clk);
> +
> + return 0;
> +}
> +
> +static int ma35_nand_resume(struct platform_device *pdev)
> +{
> + struct ma35_nand_info *nand = platform_get_drvdata(pdev);
> +
> + clk_enable(nand->clk);
> + ma35_nand_hwecc_init(nand);
> + ma35_nand_dmac_init(nand);
> +
> + return 0;
> +}
> +
> +#else
> +#define ma35_nand_suspend NULL
> +#define ma35_nand_resume NULL
> +#endif
> +
> +static const struct of_device_id ma35_nfi_of_match[] = {
> + { .compatible = "nuvoton,ma35d1-nand" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ma35_nfi_of_match);
> +
> +stati
Re: [PATCH v2 1/2] dt-bindings: mtd: nuvoton,ma35d1-nand: add new bindings
On Mon, Aug 19, 2024 at 09:20:36AM +, Hui-Ping Chen wrote: > Add dt-bindings for the Nuvoton MA35 SoC NAND Controller. > > Signed-off-by: Hui-Ping Chen > --- > .../bindings/mtd/nuvoton,ma35d1-nand.yaml | 93 +++ > 1 file changed, 93 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/mtd/nuvoton,ma35d1-nand.yaml > Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
On 18/08/2024 19:51, Laurent Pinchart wrote: > On Sun, Aug 18, 2024 at 07:44:22PM +0200, Krzysztof Kozlowski wrote: >> On 18/08/2024 19:41, Laurent Pinchart wrote: >>> Hi Krzysztof, >>> >>> Thank you for the patch. >>> >>> On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote: >>>> Each variable-length property like interrupts or resets must have fixed >>>> constraints on number of items for given variant in binding. The >>>> clauses in "if:then:" block should define both limits: upper and lower. >>> >>> I thought that, when only one of minItems or maxItems was specified, the >>> other automatically defaulted to the same value. I'm pretty sure I >>> recall Rob asking me to drop one of the two in some bindings. Has the >>> rule changes ? Is it documented somewhere ? >> >> New dtschema changed it and, even if previous behavior is restored, the >> size in if:then: always had to be constrained. You could have skipped >> one side of limit if it was equal to outer/top-level limit, e.g: >> >> properties: >> clocks: >> minItems: 1 >> maxItems: 2 >> >> >> if:then:properties: >> clocks: >> minItems: 2 > > Where can I find a description of the behaviour of the new dtschema > (hopefully with some documentation) ? No clue, but I feel there is some core concept missing. Your earlier statement: "I thought that, when only one of minItems or maxItems was specified, the" was never logically correct for the "if:then", except for the case I mentioned above. That's why all schema used as examples had it explicit: My talk from 2022, page 30: https://static.sched.com/hosted_files/osseu2022/bd/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro.pdf?_gl=1*kmzqmt*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1 all constraints defined,. My talk from 2023, page 34: https://static.sched.com/hosted_files/eoss2023/a8/How%20to%20Get%20Your%20DT%20Schema%20Bindings%20Accepted%20in%20Less%20than%2010%20Iterations%20-%20Krzysztof%20Kozlowski%2C%20Linaro%20-%20ELCE%202023.pdf?_gl=1*1jgx6d3*_gcl_au*MTU2MzQ1MjY0Mi4xNzIxNzE0NDc1 Recently, I started using other example as "useful reference": https://elixir.bootlin.com/linux/v6.8/source/Documentation/devicetree/bindings/ufs/qcom,ufs.yaml#L132 That's nothing. All three above reference examples I keep giving are already there and repeated in emails all the time. So aren't you confusing the entire "skip one limit" for top-level properties? This patch is not about it all and dtschema did not change. Best regards, Krzysztof
Re: [PATCH 1/2] dt-bindings: display: renesas,du: narrow interrupts and resets per variants
On 18/08/2024 19:41, Laurent Pinchart wrote: > Hi Krzysztof, > > Thank you for the patch. > > On Sun, Aug 18, 2024 at 07:30:02PM +0200, Krzysztof Kozlowski wrote: >> Each variable-length property like interrupts or resets must have fixed >> constraints on number of items for given variant in binding. The >> clauses in "if:then:" block should define both limits: upper and lower. > > I thought that, when only one of minItems or maxItems was specified, the > other automatically defaulted to the same value. I'm pretty sure I > recall Rob asking me to drop one of the two in some bindings. Has the > rule changes ? Is it documented somewhere ? New dtschema changed it and, even if previous behavior is restored, the size in if:then: always had to be constrained. You could have skipped one side of limit if it was equal to outer/top-level limit, e.g: properties: clocks: minItems: 1 maxItems: 2 if:then:properties: clocks: minItems: 2 Best regards, Krzysztof
