lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260205-yellow-lion-of-satiation-ffccc9@quoll>
Date: Thu, 5 Feb 2026 12:10:49 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: harshdaniel66356@...il.com
Cc: parthiban.veerasooran@...rochip.com, christian.gromm@...rochip.com, 
	gregkh@...uxfoundation.org, robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org, 
	linux-staging@...ts.linux.dev, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] media: dt-bindings: media: microchip,dim2: Add
 MediaLB DIM2 binding

On Tue, Feb 03, 2026 at 09:43:25AM +0530, harshdaniel66356@...il.com wrote:
> From: Harsh Daniel <harshdaniel66356@...il.com>
> 
> Add device tree binding documentation for the MediaLB DIM2 module found in
> Microchip (formerly SMSC/K2L) IP, used in Freescale i.MX6Q, Renesas
> R-Car Gen2/Gen3 SoCs, and Xilinx FPGAs.

Why? Thre is no user of this... or is there? Then commit msg should
explain that.

 
> This consolidates the previously proposed separate bindings into a single
> document as they share the same hardware block.

How is this relevant to git history? What are you referring to?

> 
> Signed-off-by: Harsh Daniel <harshdaniel66356@...il.com>
> ---

This is v3, so where is the changelog? See submitting patches or just
use b4...

>  .../bindings/media/microchip,dim2.yaml        | 136 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 137 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/microchip,dim2.yaml
> 
> diff --git a/Documentation/devicetree/bindings/media/microchip,dim2.yaml b/Documentation/devicetree/bindings/media/microchip,dim2.yaml
> new file mode 100644
> index 000000000000..c8cbee47ab1e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/microchip,dim2.yaml
> @@ -0,0 +1,136 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/media/microchip,dim2.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip MediaLB DIM2 Controller
> +
> +maintainers:
> +  - Andrey Shvetsov <andrey.shvetsov@....de>
> +
> +description:
> +  MediaLB DIM2 module found in Freescale i.MX6Q, Renesas R-Car Gen2 and
> +  Gen3 SoCs, and Xilinx FPGAs.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,imx6q-mlb150
> +      - renesas,rcar-gen3-mlp
> +      - xlnx,axi4-os62420_3pin-1.00.a
> +      - xlnx,axi4-os62420_6pin-1.00.a
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: MediaLB Interrupt
> +      - description: AHB0 Interrupt
> +
> +  clocks:
> +    minItems: 1
> +    maxItems: 2
> +
> +  clock-names:
> +    minItems: 1
> +    items:
> +      - const: core
> +      - const: pll8_mlb
> +
> +  microchip,clock-frequency:

No, that's not string but -hz... or some other number if justified
correctly.

Anyway, setting clock frequency as astatic is deprecated - it is not
suitable for DT. Use Common Clock Framework to get your clock frequency.

Maybe I miss the context here, but nothing in commit msg explained me
that. You also CANNOT use existing drivers as reason to document poor
API.


> +    $ref: /schemas/types.yaml#/definitions/string
> +    enum:
> +      - 256fs
> +      - 512fs
> +      - 1024fs
> +      - 2048fs
> +      - 3072fs
> +      - 4096fs
> +      - 6144fs
> +      - 8192fs
> +    description:
> +      DIM2 clock speed as a multiple of the frame sync frequency.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - microchip,clock-frequency
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: fsl,imx6q-mlb150
> +    then:
> +      required:
> +        - clocks
> +        - clock-names
> +      properties:
> +        clocks:
> +          minItems: 2
> +        clock-names:
> +          minItems: 2
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,rcar-gen3-mlp
> +    then:
> +      required:
> +        - clocks
> +      properties:
> +        clocks:
> +          maxItems: 1
> +        clock-names: false
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - xlnx,axi4-os62420_3pin-1.00.a
> +              - xlnx,axi4-os62420_6pin-1.00.a
> +    then:
> +      properties:
> +        clocks: false
> +        clock-names: false
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    mlb@...e000 {

Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
If you cannot find a name matching your device, please check in kernel
sources for similar cases or you can grow the spec (via pull request to
DT spec repo).

> +        compatible = "fsl,imx6q-mlb150";
> +        reg = <0x0218e000 0x4000>;
> +        interrupts = <0 150 IRQ_TYPE_LEVEL_HIGH>,
> +                     <0 151 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&clks 1>, <&clks 2>;
> +        clock-names = "core", "pll8_mlb";
> +        microchip,clock-frequency = "2048fs";
> +    };
> +  - |
> +    /* Renesas example */

Drop comment.

> +    mlp@...50000 {
> +        compatible = "renesas,rcar-gen3-mlp";
> +        reg = <0xe6050000 0x1000>;
> +        interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 145 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&cpg 123>;
> +        microchip,clock-frequency = "2048fs";

One or two examples are enough.

> +    };
> +  - |
> +    /* Xilinx example */
> +    dim2@...00000 {
> +        compatible = "xlnx,axi4-os62420_3pin-1.00.a";
> +        reg = <0x43c00000 0x10000>;
> +        interrupts = <0 144 IRQ_TYPE_LEVEL_HIGH>,
> +                     <0 145 IRQ_TYPE_LEVEL_HIGH>;
> +        microchip,clock-frequency = "2048fs";
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5b11839cba9d..f002a1117ef8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17650,6 +17650,7 @@ M:	Christian Gromm <christian.gromm@...rochip.com>
>  S:	Maintained
>  F:	Documentation/ABI/testing/configfs-most
>  F:	Documentation/ABI/testing/sysfs-bus-most
> +F:	Documentation/devicetree/bindings/media/microchip,dim2.yaml

So now I learn that you do it for some existing driver? Why nothing in
commit msg explains that?

Best regards,
Krzysztof


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ