Correct ranges in Armada38x dts

[bsz_semihalf.com]

Summary

Subject: [PATCH 03/34]

Ranges property of 'soc' node used two-cell addresses which resulted in
casting errors as simplebus resource allocation works with 32-bit u_long
variables. FDT ranges were simplified.

Obtained from: Semihalf
Sponsored by: Stormshield
Submitted by: Michal Stanek <[e-mail]>

[imp]

In the previous review I suggested moving these. Disregard. If we're using non-standard FDTs, we need to keep them here.

I do wonder, though why the second change. simplebus will attach to any bus that has the compatible string... Which has caused some issues for me in the past...

[andrew]

Why is this not part of D4210?

[andrew]

Why is this needed? i.e. Why are you not fixing the code?

[mst_semihalf.com]

In D4212#94393, @andrew wrote:

Why is this needed? i.e. Why are you not fixing the code?

Actually I looked into fdt_immr_addr() and found no reason to require the compatible string to be exactly "simple-bus" so I will modify that function and leave the original compatible string in FDT.
As for the 'ranges' type mismatch I think it is best to leave the change in FDT as I see no possible simple fix in the code. It would require changing the types of arguments in bus_alloc_resource() interface to avoid truncation of addresses.

[• ian]

In D4212#95141, @mst_semihalf.com wrote:

In D4212#94393, @andrew wrote:

Why is this needed? i.e. Why are you not fixing the code?

Actually I looked into fdt_immr_addr() and found no reason to require the compatible string to be exactly "simple-bus" so I will modify that function and leave the original compatible string in FDT.
As for the 'ranges' type mismatch I think it is best to leave the change in FDT as I see no possible simple fix in the code. It would require changing the types of arguments in bus_alloc_resource() interface to avoid truncation of addresses.

I have tried several times to get rid of fdt_immr_addr() and run into roadblocks, but it really needs to be eliminated (and its functionality needs to be moved into arm/mv in some better way because it's mv-specific). I don't remember what problems I ran into, it was a long time ago.

If rman's 32-bit limitation is a part of the problem, that should be getting better soon... D4544 has changes to expand rman types from u_long to uintmax_t. It will take a while for that to get tested and committed, but we should revisit this issue once that's done.

[andrew]

In D4212#95141, @mst_semihalf.com wrote:

In D4212#94393, @andrew wrote:

Why is this needed? i.e. Why are you not fixing the code?

Actually I looked into fdt_immr_addr() and found no reason to require the compatible string to be exactly "simple-bus" so I will modify that function and leave the original compatible string in FDT.

Do you still need to call fdt_immr_addr? No other SoCs need this and would be easy to restrict to the older Marvell SoCs.

[mst_semihalf.com]

In D4212#95771, @andrew wrote:

In D4212#95141, @mst_semihalf.com wrote:

In D4212#94393, @andrew wrote:

Why is this needed? i.e. Why are you not fixing the code?

Actually I looked into fdt_immr_addr() and found no reason to require the compatible string to be exactly "simple-bus" so I will modify that function and leave the original compatible string in FDT.

Do you still need to call fdt_immr_addr? No other SoCs need this and would be easy to restrict to the older Marvell SoCs.

Yes, fdt_immr_addr() initializes fdt_immr_pa and fdt_immr_va which are needed in some parts of code common for all Marvell SoCs including Armada38x. Only Marvell code uses this function, except for sys/mips/nlm/xlp_machdep.c where it seems to have been added by mistake.

[andrew]

In D4212#96059, @mst_semihalf.com wrote:

In D4212#95771, @andrew wrote:

Do you still need to call fdt_immr_addr? No other SoCs need this and would be easy to restrict to the older Marvell SoCs.

Yes, fdt_immr_addr() initializes fdt_immr_pa and fdt_immr_va which are needed in some parts of code common for all Marvell SoCs including Armada38x. Only Marvell code uses this function, except for sys/mips/nlm/xlp_machdep.c where it seems to have been added by mistake.

Is this common code called on Armada 38x? The only place that I can see is in platform_devmap_init, and I think this could be replaced with a local function to find the pa & size.

[bsz_semihalf.com]

fdt_immr_addr modified instead of changing compatible.

[mst_semihalf.com]

In D4212#96118, @andrew wrote:

In D4212#96059, @mst_semihalf.com wrote:

In D4212#95771, @andrew wrote:

Do you still need to call fdt_immr_addr? No other SoCs need this and would be easy to restrict to the older Marvell SoCs.

Yes, fdt_immr_addr() initializes fdt_immr_pa and fdt_immr_va which are needed in some parts of code common for all Marvell SoCs including Armada38x. Only Marvell code uses this function, except for sys/mips/nlm/xlp_machdep.c where it seems to have been added by mistake.

Is this common code called on Armada 38x? The only place that I can see is in platform_devmap_init, and I think this could be replaced with a local function to find the pa & size.

It is also required in decode_win_pcie_setup() which gets called on A38X as well.

[bsz_semihalf.com]

I decided to seperate DTS change and correcting strict compatibility requirement, as there are more calls we are affected by.

[bsz_semihalf.com]

@andrew, the commit was reduced. Necessity of remaining change was explained by Michal above. Please let me know if it's acceptable for you.