[PATCH v5 1/9] dt-bindings: display: bindings for starfive,JH7110 display pipeline

[Krzysztof Kozlowski]

On Wed, Nov 20, 2024 at 02:18:40PM +0800, keith zhao wrote:
> - Added bindings to support the display subsystem on the JH7110 SoC.

Please do not use "This commit/patch/change" (implied) and past tense,
but imperative mood. See longer explanation here:
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> 
> - Included the DC8200 display controller and Inno HDMI controller.
> 
> - Created innosilicon,inno-hdmi.yaml schema containing common properties
>   for the Inno DesignWare HDMI TX controller.
>   This isn't a full device tree binding specification,
>   but is intended to be referenced by platform-specific bindings
>   for the IP core.
> 
> Signed-off-by: keith zhao <keith.zhao at starfivetech.com>
> ---
>  .../display/bridge/innosilicon,inno-hdmi.yaml |  45 +++++
>  .../display/rockchip/rockchip,inno-hdmi.yaml  |  27 +--
>  .../starfive/starfive,jh7110-dc8200.yaml      | 176 ++++++++++++++++++
>  .../starfive/starfive,jh7110-inno-hdmi.yaml   |  91 +++++++++
>  .../soc/starfive/starfive,jh7110-syscon.yaml  |   1 +

I do not see how you addressed my feedback. I asked you to split the
patch. Where is the split?

Your answer to my request to split was "Background is ...", yeah, so
what? You are not going to split?


>  MAINTAINERS                                   |   8 +
>  6 files changed, 323 insertions(+), 25 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml
>  create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-dc8200.yaml
>  create mode 100644 Documentation/devicetree/bindings/display/starfive/starfive,jh7110-inno-hdmi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml b/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.yaml
> new file mode 100644
> index 000000000000..f2543aebc312
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/innosilicon,inno-hdmi.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/bridge/innosilicon,inno-hdmi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common Properties for Innosilicon HDMI TX IP
> +
> +maintainers:
> +  - keith zhao <keith.zhao at starfivetech.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.



> +  Innosilicon HDMI TX is an HDMI transmission device integrated into the zap SoC.

Keep and extend.

> +  This document specifies the device tree properties for the INNO HDMI IP core.

Keep... but what is INNO HDMI IP core? Another name? Different block?


> +  It is intended to be referenced by platform-specific device tree bindings,
> +  which will determine the necessity of each property.

Not much improved here. Last two sentences are almost useless. Again -
document the hardware. Drop this sentence.

I already complained about this. I already complained that you keep
ignoring my comments and this does not improve much.

Since you did not implement my main feedback I am going to NAK the rest
except one more thing:

...

> +
> +properties:
> +  compatible:
> +    const: "starfive,jh7110-inno-hdmi"

I could not be more specific in my previous feedback. Improvements? No.
Stop wasting our time.

NAK.

Best regards,
Krzysztof