linux-kernel - Re: [PATCH 1/2] dt-bindings: mmc: mtk-sd: Add eMMC for AN7581

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: <phnwf7vkincyeua3jhboi5pod7uwkigwc2ibpygrg7gezeh36d@gpx6krsqfbqe>
Date: Wed, 15 Jan 2025 09:51:16 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: Chaotian Jing <chaotian.jing@...iatek.com>, 
	Ulf Hansson <ulf.hansson@...aro.org>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Matthias Brugger <matthias.bgg@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Wenbin Mei <wenbin.mei@...iatek.com>, linux-mmc@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org, upstream@...oha.com
Subject: Re: [PATCH 1/2] dt-bindings: mmc: mtk-sd: Add eMMC for AN7581

On Wed, Jan 15, 2025 at 08:29:50AM +0100, Christian Marangi wrote:
> Document eMMC compatible for AN7581. This eMMC controller doesn't have
> regulator exposed to the system and have a single clock only for source
> clock and only default pintctrl.
> 
> Rework the schema to permit these new requirements and make supply
> optional only for airoha,an7581-mmc compatible.
> 
> Also provide an example for airoha,an7581-mmc.
> 
> Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
> ---
> This depends on patch merged in clk-next 
> bfe257f9780d8f77045a7da6ec959ee0659d2f98
> 
>  .../devicetree/bindings/mmc/mtk-sd.yaml       | 64 +++++++++++++++++--
>  1 file changed, 58 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> index f86ebd81f5a5..6dad5455b369 100644
> --- a/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> +++ b/Documentation/devicetree/bindings/mmc/mtk-sd.yaml
> @@ -27,6 +27,7 @@ properties:
>            - mediatek,mt8183-mmc
>            - mediatek,mt8196-mmc
>            - mediatek,mt8516-mmc
> +          - airoha,an7581-mmc

Please keep things ordered. The list is nicely ordered so far.

>        - items:
>            - const: mediatek,mt7623-mmc
>            - const: mediatek,mt2701-mmc
> @@ -48,11 +49,11 @@ properties:
>    clocks:
>      description:
>        Should contain phandle for the clock feeding the MMC controller.
> -    minItems: 2
> +    minItems: 1
>      maxItems: 7
>  
>    clock-names:
> -    minItems: 2
> +    minItems: 1
>      maxItems: 7
>  
>    interrupts:
> @@ -72,7 +73,7 @@ properties:
>        Should at least contain default and state_uhs. To support SDIO in-band wakeup, dat1 pin
>        will be switched between GPIO mode and SDIO DAT1 mode, state_eint is mandatory in this
>        scenario.
> -    minItems: 2
> +    minItems: 1
>      items:
>        - const: default
>        - const: state_uhs
> @@ -170,9 +171,6 @@ required:
>    - clock-names
>    - pinctrl-names
>    - pinctrl-0
> -  - pinctrl-1
> -  - vmmc-supply
> -  - vqmmc-supply
>  
>  allOf:
>    - $ref: mmc-controller.yaml#
> @@ -335,6 +333,40 @@ allOf:
>              - const: axi_cg
>              - const: ahb_cg
>  
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: airoha,an7581-mmc
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: source clock
> +
> +        clock-names:
> +          items:
> +            - const: source
> +
> +        pinctrl-names:
> +          items:
> +            - const: default
> +    else:

No else, you are now duplicating other "if:then" cases. There is already
such for "reg" so it does not really make it more readable. Just be sure
each variant has a strict, upper and lower, constraints.


> +      properties:
> +        clocks:
> +          minItems: 2
> +
> +        clock-names:
> +          minItems: 2
> +
> +        pinctrl-names:
> +          minItems: 2
> +
> +      required:
> +        - pinctrl-1
> +        - vmmc-supply
> +        - vqmmc-supply

These should go to each variant's if:then: or separate if:then: covering
all existing variants for the required fields.

> +
>  unevaluatedProperties: false
>  
>  examples:
> @@ -389,5 +421,25 @@ examples:
>          vqmmc-supply = <&mt6397_vgp3_reg>;
>          mmc-pwrseq = <&wifi_pwrseq>;
>      };
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/en7523-clk.h>
> +    mmc@...0e000 {
> +        compatible = "airoha,an7581-mmc";
> +        reg = <0x1fa0e000 0x1000>,
> +              <0x1fa0c000 0x60>;
> +        clocks = <&scuclk EN7581_CLK_EMMC>;
> +        clock-names = "source";
> +        interrupts = <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>;
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&mmc_pins>;
> +        bus-width = <4>;
> +        max-frequency = <52000000>;
> +        disable-wp;
> +        cap-mmc-highspeed;
> +        non-removable;
> +

Drop blank line.


Best regards,
Krzysztof