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@xxxxxxxxxxxxxxxx> > --- > .../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@xxxxxxxxxxxxxxxx> > + > +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
