Alexander Stein <alexander.stein@xxxxxxxxxxxxxxx> writes:
> Would you consider current converting into YAML format?
You mean converting Documentation/devicetree/bindings/soc/fsl/qe.txt and
Documentation/devicetree/bindings/soc/fsl/qe/*.txt into YAML?
I can consider that. I haven't done something like that before, but I
assume it might include some additional work other than trivially format
conversion. So I would prefer to do that after this patch, if that is
ok.
> Am Donnerstag, 30. Mai 2024, 16:22:54 CEST schrieb Esben Haabendal:
>> The LS1021A contains a QUICC Engine Block, so add a node to device
>> tree describing that.
>>
>> Signed-off-by: Esben Haabendal <esben@xxxxxxxxxx>
>> ---
>> arch/arm/boot/dts/nxp/ls/ls1021a.dtsi | 51 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 51 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/nxp/ls/ls1021a.dtsi b/arch/arm/boot/dts/nxp/ls/ls1021a.dtsi
>> index e86998ca77d6..ff7be69acdd5 100644
>> --- a/arch/arm/boot/dts/nxp/ls/ls1021a.dtsi
>> +++ b/arch/arm/boot/dts/nxp/ls/ls1021a.dtsi
>> @@ -460,6 +460,57 @@ gpio3: gpio@2330000 {
>> #interrupt-cells = <2>;
>> };
>>
>> + uqe: uqe@2400000 {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + device_type = "qe";
>> + compatible = "fsl,qe", "simple-bus";
>> + ranges = <0x0 0x0 0x2400000 0x40000>;
>> + reg = <0x0 0x2400000 0x0 0x480>;
>
> Properties please in this order:
> * compatible
> * reg
> * #address-cells
> * #size-cells
> * ranges
> * device_type
Fixing.
>> + brg-frequency = <150000000>;
>> + bus-frequency = <300000000>;
>
> Mh, aren't these values depending on your actual RCW configuration?
Yes, you are right. The QE bus-frequency comes from platform_clk which
is controlled by various bits in RCW and sys_ref_clk.
So I guess it should be possible to derive bus-frequency from sysclk
clock-frequency attribute and RCW. But fsl,qe bus-frequency is a
required property...
Max bus-frequency for LS1021A is 300 MHz. But it should be possible to
set it lower, although I suspect that many/most/everyone is running it
at 300 MHz.
>> + fsl,qe-num-riscs = <1>;
>> + fsl,qe-num-snums = <28>;
>
> Current bindings defines:
>> fsl,qe-snums: This property has to be specified as '/bits/ 8' value,
>> defining the array of serial number (SNUM) values for the virtual
>> threads.
>
> So '/bits/ 8' is missing.
Ok, so you want me to add an array for fs,qe-snums attribute?
None of the existing fsl,qe devices has a fsl,qe-snums.
And qe_snums_init() has a fallback, so I don't think it is correct to
specify fsl,qe-snums to be a required property in the bindings. It
should be listed as optional.
>> + qeic: qeic@80 {
>> + compatible = "fsl,qe-ic";
>> + reg = <0x80 0x80>;
>> + #address-cells = <0>;
>> + interrupt-controller;
>> + #interrupt-cells = <1>;
>> + interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH
>> + GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
>> + };
>> +
>> + ucc@2000 {
>> + cell-index = <1>;
>> + reg = <0x2000 0x200>;
>> + interrupts = <32>;
>> + interrupt-parent = <&qeic>;
>
> Move cell-index to last position.
Done.
>> + };
>> +
>> + ucc@2200 {
>> + cell-index = <3>;
>> + reg = <0x2200 0x200>;
>> + interrupts = <34>;
>> + interrupt-parent = <&qeic>;
>
> Same here.
Done.
>> + };
>> +
>> + muram@10000 {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + compatible = "fsl,qe-muram", "fsl,cpm-muram";
>> + ranges = <0x0 0x10000 0x6000>;
>
> Node address but no 'reg' property? I have no idea if this is okay.
> Also compatible (and possibly reg) first.
It is done in the same way for all existing fsl,qe-muram devices. So if
it is not okay, a tree-wide fixup would be in place.
/Esben
