[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <0c9ea547-eaa5-4446-9d6c-e6fcc289ec4a@linaro.org>
Date: Tue, 16 Jan 2024 21:43:41 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Dharma Balasubiramani <dharma.b@...rochip.com>,
conor.dooley@...rochip.com, sam@...nborg.org, bbrezillon@...nel.org,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org, tzimmermann@...e.de,
airlied@...il.com, daniel@...ll.ch, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
nicolas.ferre@...rochip.com, alexandre.belloni@...tlin.com,
claudiu.beznea@...on.dev, dri-devel@...ts.freedesktop.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-kernel@...r.kernel.org, lee@...nel.org, thierry.reding@...il.com,
u.kleine-koenig@...gutronix.de, linux-pwm@...r.kernel.org
Cc: linux4microchip@...rochip.com
Subject: Re: [PATCH v2 3/3] dt-bindings: mfd: atmel,hlcdc: Convert to DT
schema format
On 16/01/2024 12:38, Dharma Balasubiramani wrote:
> Convert the atmel,hlcdc binding to DT schema format.
>
> Adjust the clock-names property to clarify that the LCD controller expects
> one of these clocks (either sys_clk or lvds_pll_clk to be present but not
> both) along with the slow_clk and periph_clk. This alignment with the actual
> hardware requirements will enable accurate device tree configuration for
> systems using the HLCDC IP.
>
> Signed-off-by: Dharma Balasubiramani <dharma.b@...rochip.com>
> ---
> changelog
> v1 -> v2
> - Remove the explicit copyrights.
> - Modify title (not include words like binding/driver).
> - Modify description actually describing the hardware and not the driver.
> - Add details of lvds_pll addition in commit message.
> - Ref endpoint and not endpoint-base.
> - Fix coding style.
>
> Note: Renaming hlcdc-display-controller, hlcdc-pwm to generic names throws
> errors from the existing DTS files.
> ...
> /home/dharma/Mainline/linux/arch/arm/boot/dts/microchip/at91sam9n12ek.dtb:
> hlcdc@...38000: 'hlcdc-display-controller' does not match any of the
> regexes: 'pinctrl-[0-9]+'
> ---
> .../devicetree/bindings/mfd/atmel,hlcdc.yaml | 105 ++++++++++++++++++
> .../devicetree/bindings/mfd/atmel-hlcdc.txt | 56 ----------
> 2 files changed, 105 insertions(+), 56 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
> delete mode 100644 Documentation/devicetree/bindings/mfd/atmel-hlcdc.txt
>
> diff --git a/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
> new file mode 100644
> index 000000000000..f624b60b76fb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/atmel,hlcdc.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/mfd/atmel,hlcdc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Atmel's HLCD Controller
> +
> +maintainers:
> + - Nicolas Ferre <nicolas.ferre@...rochip.com>
> + - Alexandre Belloni <alexandre.belloni@...tlin.com>
> + - Claudiu Beznea <claudiu.beznea@...on.dev>
> +
> +description: |
> + The Atmel HLCDC (HLCD Controller) IP available on Atmel SoCs exposes two
> + subdevices
> + # a PWM chip:
> + # a Display Controller:
This is a friendly reminder during the review process.
It seems my or other reviewer's previous comments were not fully
addressed. Maybe the feedback got lost between the quotes, maybe you
just forgot to apply it. Please go back to the previous discussion and
either implement all requested changes or keep discussing them.
Thank you.
> +
> +properties:
> + compatible:
> + enum:
> + - atmel,at91sam9n12-hlcdc
> + - atmel,at91sam9x5-hlcdc
> + - atmel,sama5d2-hlcdc
> + - atmel,sama5d3-hlcdc
> + - atmel,sama5d4-hlcdc
> + - microchip,sam9x60-hlcdc
> + - microchip,sam9x75-xlcdc
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 3
> +
> + clock-names:
> + anyOf:
> + - items:
> + - enum:
> + - sys_clk
> + - lvds_pll_clk
> + - contains:
> + const: periph_clk
> + - contains:
> + const: slow_clk
NAK. You just ignored entire review.
Best regards,
Krzysztof
Powered by blists - more mailing lists