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@xxxxxxxxx>
> ---
> 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@1fa0e000 {
> + 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
