On 20/05/2022 20:46, Sibi Sankar wrote: >>> + memory-region: >>> + maxItems: 2 >>> + description: Phandle reference to the reserved-memory for the MBA region followed >>> + by the modem region. >>> + >>> + firmware-name: >>> + $ref: /schemas/types.yaml#/definitions/string-array >>> + maxItems: 2 >> >> Instead of maxItems can this be >> >> items: >> - description: Name of MBA firmware >> - description: Name of modem firmware >> >> so that we know the order? Same for 'memory-region' above. > > ack > >> >>> + description: >>> + The name of the MBA and modem firmware to be loaded for this remote processor. >>> + >>> + qcom,halt-regs: >>> + $ref: /schemas/types.yaml#/definitions/phandle-array >> >> Should this have maxItems: 1? Or that's implicit from description? > > It's implicit! I am not aware of such implicit rule in schema. maxItems are always required. If this is maxItems:1 it is not even an array. > >> >>> + description: >>> + Phandle reference to a syscon representing TCSR followed by the >>> + four offsets within syscon for q6, modem, nc and vq6 halt registers. >>> + >>> + qcom,ext-regs: >>> + $ref: /schemas/types.yaml#/definitions/phandle-array >> >> Should this have min/maxItems: 2? > > ack You should also define the items. This applies to all such fields. Check the examples of syscon consumers. > >> >>> + description: >>> + Two phandle references to syscons representing TCSR_REG and TCSR register >>> + space followed by the two offsets within the syscon to force_clk_en/rscc_disable >>> + and axim1_clk_off/crypto_clk_off registers respectively. >>> + >>> + qcom,qaccept-regs: >>> + $ref: /schemas/types.yaml#/definitions/phandle-array >>> + description: >>> + Phandle reference to a syscon representing TCSR followed by the >>> + three offsets within syscon for mdm, cx and axi qaccept registers. >>> + >>> + qcom,qmp: >>> + $ref: /schemas/types.yaml#/definitions/phandle >>> + description: Reference to the AOSS side-channel message RAM. >>> + >>> + qcom,smem-states: >>> + $ref: /schemas/types.yaml#/definitions/phandle-array >>> + description: States used by the AP to signal the Hexagon core >>> + items: >>> + - description: Stop the modem >> >> This one did items for a phandle array so I think we should follow the >> same above. > > ack > >> >>> + >>> + qcom,smem-state-names: >>> + description: The names of the state bits used for SMP2P output >>> + const: stop >>> + >>> + glink-edge: >>> + $ref: qcom,glink-edge.yaml# >>> + description: >>> + Qualcomm G-Link subnode which represents communication edge, channels >>> + and devices related to the DSP. >> [..] >>> + power-domain-names = "cx", "mss"; >>> + >>> + memory-region = <&mba_mem>, <&mpss_mem>; >>> + >>> + qcom,qmp = <&aoss_qmp>; >>> + >>> + qcom,smem-states = <&modem_smp2p_out 0>; >>> + qcom,smem-state-names = "stop"; >>> + >>> + resets = <&aoss_reset AOSS_CC_MSS_RESTART>, >>> + <&pdc_reset PDC_MODEM_SYNC_RESET>; >>> + reset-names = "mss_restart", "pdc_reset"; >>> + >>> + qcom,halt-regs = <&tcsr_mutex 0x23000 0x25000 0x28000 0x33000>; >>> + qcom,ext-regs = <&tcsr 0x10000 0x10004 &tcsr_mutex 0x26004 0x26008>; >> >> Because it's two items I'd expect: >> >> <&tcsr 0x10000 0x10004>, <&tcsr_mutex 0x26004 0x26008>; > > I guess both the ways work since the driver uses > of_parse_phandle_with_fixed_args. But only one is correct... Best regards, Krzysztof
