Re: [DO NOT MERGE v5 12/37] dt-bindings: pci: pci-sh7751: Add SH7751 PCI

Geert Uytterhoeven
2023-12-05T14:22:05Z

Hi Sato-san,

On Tue, Dec 5, 2023 at 10:46 AM Yoshinori Sato
<ysato@users.sourceforge.jp> wrote:
> Renesas SH7751 PCI Controller json-schema.
>
> Signed-off-by: Yoshinori Sato <ysato@users.sourceforge.jp>

Thanks for your patch!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/renesas,sh7751-pci.yaml
> @@ -0,0 +1,128 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pci/renesas,sh7751-pci.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas SH7751 PCI Host controller
> +
> +maintainers:
> + - Yoshinori Sato <ysato@users.sourceforge.jp>
> +
> +allOf:
> + - $ref: /schemas/pci/pci-bus.yaml#
> +
> +properties:
> + compatible:
> + items:
> + - enum:
> + - renesas,sh7751-pci
> +
> + reg:
> + minItems: 2
> + maxItems: 2

Please add "reg-names", as there is more than one entry.
If that is not sufficient to document what each entry means, please add
"description"s, too.

> + renesas,memory:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + PCI BMDMA src/dst memory area.

Isn't that the purpose of the "dma-ranges" property?

> +
> + renesas,bcr1:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + SH7751 PCIC PCIBCR1 value. This value makes add the value of BSC's BCR1.

What does this mean?

> +
> + renesas,mcrmask:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + SH7751 PCIC PCIMCR value. This value makes clear bit in the value of BSC's MCR.

What does this mean?

> +
> + renesas,intm:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + SH7751 PCIC PCIINTM value.
> +
> + renesas,aintm:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + SH7751 PCIC PCIIANTM value.
> +
> + renesas,lsr:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + SH7751 PCIC PCILSR0 and PCILSR1 values.
> + First word is PCILSR0, Second word is PCILSR1.
> +
> + renesas,lar:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + SH7751 PCIC PCILSA0 and PCILAR1 values.
> + First word is PCILAR0, Second word is PCILAR1.
> +
> + renesas,dmabt:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + SH7751 PCIC PCIDMABT value.
> +
> + renesas,pintm:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + SH7751 PCIC PCIPINTM value.
> +
> + renesas,config:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + SH7751 PCIC PCICONFIG values array. Register Number and value pair list.

Several of these properties look like pure hardware programming.
Can these values be derived from other (standard) DT properties?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds