Re: [PATCH v2 2/2] Add dp PHY dt-bindings
Re: [PATCH v2 2/2] Add dp PHY dt-bindings
On 13/05/2024 16:17, Krzysztof Kozlowski wrote: >>> dp.yaml @@ -0,0 +1,45 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: >>> http://devicetree.org/schemas/display/mediatek/mediatek,phy-dp.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek Display Port Controller + +maintainers: + - Mac shen + - Liankun yang + +description: | + Special settings need to be configured by MediaTek DP based on >>> the actual + hardware situation. For example, when using a certain brand's >>> docking + station for display projection, garbage may appear. Adjusting >>> the specific + ssc value can resolve this issue. + +properties: + status: disabled >>> >>> I think you nicely shocked Rob already. >>> >>> Please reach internally to Mediatek or collaborating companies to get >>> basic training and instructions how to write patches and bindings. >>> >>> Otherwise it is waste of our time. Mediatek is not a small company so >>> there is no excuse in sending such poor quality patches, which would >>> be >>> EASILY spotted by the MOST BASIC review. >>> >>> Best regards, >>> Krzysztof >>> >> >> I used scripts/checkpatch.pl and fix reported error and warnings. >> I am resumbmitting bindings by the MOST BASIC review. > > No, please wait. Who did the basic review of your patch? Who from > Mediatek? Upstream is not a workhorse to use instead of your resources > for the absolute basic stuff... This feels such exploiting. > After reading AngeloGioacchino's response, it looks even worse... This was never tested... So not only exploiting but also feels like wasting our time. Best regards, Krzysztof
Re: [PATCH v2 2/2] Add dp PHY dt-bindings
On 13/05/2024 16:15, LIANKUN YANG (杨连坤) wrote: > On Mon, 2024-05-13 at 08:37 +0200, Krzysztof Kozlowski wrote: >> >> External email : Please do not click links or open attachments until >> you have verified the sender or the content. >> On 10/05/2024 13:04, Liankun Yang wrote: >>> Add dp PHY dt-bindings. >> >> This tells me nothing. Read submitting patches. >> >>> >>> Changeds in v2: >> >> This does goes to changelog, not commit log. >> >>> - Add dp PHY dt-bindings. >>> https://patchwork.kernel.org/project/linux-mediatek/patch/ >>> 20240403040517.3279-1-liankun.y...@mediatek.com/ >>> >>> Signed-off-by: Liankun Yang >> >> This patch looks terrible... there's so many wrong things I don't >> even >> know where to start. >> >> Please use subject prefixes matching the subsystem. You can get them >> for >> example with `git log --oneline -- DIRECTORY_OR_FILE` 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 >> >> Please run scripts/checkpatch.pl and fix reported warnings. Then >> please >> run `scripts/checkpatch.pl --strict` 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. >> >> >>> --- >>> .../display/mediatek/mediatek.phy-dp.yaml | 45 >> +++ >>> 1 file changed, 45 insertions(+) >>> create mode 100644 >> Documentation/devicetree/bindings/display/mediatek/mediatek.phy- >> dp.yaml >> >> Wrong filename >> >>> >>> diff --git >> a/Documentation/devicetree/bindings/display/mediatek/mediatek.phy- >> dp.yaml >> b/Documentation/devicetree/bindings/display/mediatek/mediatek.phy- >> dp.yaml >>> index ..476bc329363f >>> --- /dev/null >>> +++ >> b/Documentation/devicetree/bindings/display/mediatek/mediatek.phy- >> dp.yaml >>> @@ -0,0 +1,45 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: >> http://devicetree.org/schemas/display/mediatek/mediatek,phy-dp.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: MediaTek Display Port Controller >>> + >>> +maintainers: >>> + - Mac shen >>> + - Liankun yang >>> + >>> +description: | >>> + Special settings need to be configured by MediaTek DP based on >> the actual >>> + hardware situation. For example, when using a certain brand's >> docking >>> + station for display projection, garbage may appear. Adjusting >> the specific >>> + ssc value can resolve this issue. >>> + >>> +properties: >>> + status: disabled >> >> I think you nicely shocked Rob already. >> >> Please reach internally to Mediatek or collaborating companies to get >> basic training and instructions how to write patches and bindings. >> >> Otherwise it is waste of our time. Mediatek is not a small company so >> there is no excuse in sending such poor quality patches, which would >> be >> EASILY spotted by the MOST BASIC review. >> >> Best regards, >> Krzysztof >> > > I used scripts/checkpatch.pl and fix reported error and warnings. > I am resumbmitting bindings by the MOST BASIC review. No, please wait. Who did the basic review of your patch? Who from Mediatek? Upstream is not a workhorse to use instead of your resources for the absolute basic stuff... This feels such exploiting. Best regards, Krzysztof
Re: [PATCH v2 2/2] Add dp PHY dt-bindings
Re: [PATCH v2 2/2] Add dp PHY dt-bindings
Re: [PATCH v2 2/2] Add dp PHY dt-bindings
On 10/05/2024 13:04, Liankun Yang wrote: > Add dp PHY dt-bindings. This tells me nothing. Read submitting patches. > > Changeds in v2: This does goes to changelog, not commit log. > - Add dp PHY dt-bindings. > https://patchwork.kernel.org/project/linux-mediatek/patch/ > 20240403040517.3279-1-liankun.y...@mediatek.com/ > > Signed-off-by: Liankun Yang This patch looks terrible... there's so many wrong things I don't even know where to start. Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` 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 Please run scripts/checkpatch.pl and fix reported warnings. Then please run `scripts/checkpatch.pl --strict` 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. > --- > .../display/mediatek/mediatek.phy-dp.yaml | 45 +++ > 1 file changed, 45 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml Wrong filename > > diff --git > a/Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml > b/Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml > index ..476bc329363f > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml > @@ -0,0 +1,45 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/mediatek/mediatek,phy-dp.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek Display Port Controller > + > +maintainers: > + - Mac shen > + - Liankun yang > + > +description: | > + Special settings need to be configured by MediaTek DP based on the actual > + hardware situation. For example, when using a certain brand's docking > + station for display projection, garbage may appear. Adjusting the specific > + ssc value can resolve this issue. > + > +properties: > + status: disabled I think you nicely shocked Rob already. Please reach internally to Mediatek or collaborating companies to get basic training and instructions how to write patches and bindings. Otherwise it is waste of our time. Mediatek is not a small company so there is no excuse in sending such poor quality patches, which would be EASILY spotted by the MOST BASIC review. Best regards, Krzysztof
Re: [PATCH v2 2/2] Add dp PHY dt-bindings
On Fri, May 10, 2024 at 07:04:15PM +0800, Liankun Yang wrote:
> Add dp PHY dt-bindings.
>
> Changeds in v2:
> - Add dp PHY dt-bindings.
> https://patchwork.kernel.org/project/linux-mediatek/patch/
> 20240403040517.3279-1-liankun.y...@mediatek.com/
>
> Signed-off-by: Liankun Yang
> ---
> .../display/mediatek/mediatek.phy-dp.yaml | 45 +++
> 1 file changed, 45 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml
> b/Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml
git refuses to apply your patch because 'new file mode 100644' is
missing. You must have edited the patch or something.
If it did apply, you'd notice it fails testing.
> index ..476bc329363f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml
> @@ -0,0 +1,45 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/mediatek/mediatek,phy-dp.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek Display Port Controller
> +
> +maintainers:
> + - Mac shen
> + - Liankun yang
> +
> +description: |
> + Special settings need to be configured by MediaTek DP based on the actual
> + hardware situation. For example, when using a certain brand's docking
> + station for display projection, garbage may appear. Adjusting the specific
> + ssc value can resolve this issue.
> +
> +properties:
> + status: disabled
> +description: |
> + Since the DP driver has already registered the DP PHY device
> + through mtk_dp_register_phy(), so the status is disabled.
What!? Please show me any other binding that has 'status' in it. Go read
up on how to write bindings and what goes in them.
> +
> + dp-ssc-setting:
> +- ssc-delta-hbr
> +description: Specific values are set based on the actual HW situation.
> +
> +required:
> + - status
> + - dp-ssc-setting
> +
> +examples:
> + - |
> +soc {
> +#address-cells = <2>;
> +#size-cells = <2>;
> +
> +phy-dp@1c60 {
> + status = "disabled";
> + dp-ssc-setting {
> +ssc-delta-hbr = <0x01fe>;
> + }
> +};
> +};
> --
> 2.18.0
>
[PATCH v2 2/2] Add dp PHY dt-bindings
Add dp PHY dt-bindings.
Changeds in v2:
- Add dp PHY dt-bindings.
https://patchwork.kernel.org/project/linux-mediatek/patch/
20240403040517.3279-1-liankun.y...@mediatek.com/
Signed-off-by: Liankun Yang
---
.../display/mediatek/mediatek.phy-dp.yaml | 45 +++
1 file changed, 45 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml
diff --git
a/Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml
b/Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml
index ..476bc329363f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/mediatek/mediatek.phy-dp.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/mediatek/mediatek,phy-dp.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek Display Port Controller
+
+maintainers:
+ - Mac shen
+ - Liankun yang
+
+description: |
+ Special settings need to be configured by MediaTek DP based on the actual
+ hardware situation. For example, when using a certain brand's docking
+ station for display projection, garbage may appear. Adjusting the specific
+ ssc value can resolve this issue.
+
+properties:
+ status: disabled
+description: |
+ Since the DP driver has already registered the DP PHY device
+ through mtk_dp_register_phy(), so the status is disabled.
+
+ dp-ssc-setting:
+- ssc-delta-hbr
+description: Specific values are set based on the actual HW situation.
+
+required:
+ - status
+ - dp-ssc-setting
+
+examples:
+ - |
+soc {
+#address-cells = <2>;
+#size-cells = <2>;
+
+phy-dp@1c60 {
+ status = "disabled";
+ dp-ssc-setting {
+ssc-delta-hbr = <0x01fe>;
+ }
+};
+};
--
2.18.0
