Enable setting the dma tag at the nexus level

[mw_semihalf.com]

Summary

Allow to set the dma tag for nexus in the platform init code,
so that all busses and devices would be able to inherit it.
This change is useful e.g. for setting coherent dma tag for
the platforms with hardware IO cache coherency.

[• ian]

I don't think this change is right. There are a lot of simplebus instances in the system, and they don't all need their own private copies of essentially the same tag.

I think the right way to do this is to add to arm/nexus.c, wrapped in #ifdef FDT,

DEVMETHOD(bus_get_dma_tag, nexus_get_dma_tag),

and nexus would do the property check and tag creation in its attach() routine, also wrapped in #ifdef FDT. Hmm, and nexus will have to grow a softc to take care of all that.

Another possibility would be to do it in ofwbus, child of nexus and parent of almost everything else. It's the "almost" part that makes me think nexus is the better place to do it, if it's truly a platform-wide property.

[• ian]

More basic question: Where is it documented that a soc node can have a dma-coherent property?

[mkm_semihalf.com]

I decided to create dma tag in ofwbus and moved 'dma-coherent' property to '/' node in DTS.
All changes related to dma tag creation are now done only if FDT is defined.

The 'dma-coherent' property is not documented on FreeBSD.
On Linux it is used by arm/arm64 code and can be placed in various device and bus nodes (e.g. amba bus in artpec6.dtsi).

[• ian]

In D11202#233164, @mkm_semihalf.com wrote:

I decided to create dma tag in ofwbus and moved 'dma-coherent' property to '/' node in DTS.
All changes related to dma tag creation are now done only if FDT is defined.

The 'dma-coherent' property is not documented on FreeBSD.
On Linux it is used by arm/arm64 code and can be placed in various device and bus nodes (e.g. amba bus in artpec6.dtsi).

That was kind of my point: it's not documented, period. You just made it up, taking us another step further away from supporting standard DTS sources for marvell on freebsd.

How does the standard DTS for marvell systems handle this? How does linux handle it?

If this is truly a platform-wide attribute, not a per-bus attribute, then nexus is where the tag should be.

If standard DTS and linux don't describe this with a property in the dts source, but rather just let the platform code handle it somehow, then we should do something similar rather than creating even more non-standard dts for ourselves.

[mw_semihalf.com]

Hi @ian

In D11202#233180, @ian wrote:

In D11202#233164, @mkm_semihalf.com wrote:

I decided to create dma tag in ofwbus and moved 'dma-coherent' property to '/' node in DTS.
All changes related to dma tag creation are now done only if FDT is defined.

The 'dma-coherent' property is not documented on FreeBSD.
On Linux it is used by arm/arm64 code and can be placed in various device and bus nodes (e.g. amba bus in artpec6.dtsi).

That was kind of my point: it's not documented, period. You just made it up, taking us another step further away from supporting standard DTS sources for marvell on freebsd.

How does the standard DTS for marvell systems handle this? How does linux handle it?

If this is truly a platform-wide attribute, not a per-bus attribute, then nexus is where the tag should be.

If standard DTS and linux don't describe this with a property in the dts source, but rather just let the platform code handle it somehow, then we should do something similar rather than creating even more non-standard dts for ourselves.

In linux all 'magic' happens in arch/arm/mach-mvebu/coherency.c, which is:

• set coherent dma ops
• runtime PL310 DT node fixup (add "arm,io-coherent") property, due to CONFIG_SMP limitation (not relevant for us)

We tested and unfortunately it seems there is no way to add new property to the DT in FreeBSD (only modify existing one, but the value must be not bigger than original), hence another way should be made up.

I would really like to be able to use some mechanism (e.g. in platform_late_init). So basing your expertise, do you think it is possible to force settings of parent dma tag (nexus, ofwbus, simple-bus - whatever you prefer), so that all children (devices) can inherit this when creating theirs?

[• ian]

In D11202#233184, @mw_semihalf.com wrote:

Hi @ian
...
In linux all 'magic' happens in arch/arm/mach-mvebu/coherency.c, which is:

• set coherent dma ops
• runtime PL310 DT node fixup (add "arm,io-coherent") property, due to CONFIG_SMP limitation (not relevant for us)

We tested and unfortunately it seems there is no way to add new property to the DT in FreeBSD (only modify existing one, but the value must be not bigger than original), hence another way should be made up.

I would really like to be able to use some mechanism (e.g. in platform_late_init). So basing your expertise, do you think it is possible to force settings of parent dma tag (nexus, ofwbus, simple-bus - whatever you prefer), so that all children (devices) can inherit this when creating theirs?

Yes, doing it in platform_late_init is just the sort of thing I was imagining, if it is a property of the entire system. Let me put together a little proposal-patch to see if my vague idea is practical.

[mw_semihalf.com]

In D11202#233185, @ian wrote:

In D11202#233184, @mw_semihalf.com wrote:

Hi @ian
...
In linux all 'magic' happens in arch/arm/mach-mvebu/coherency.c, which is:

• set coherent dma ops
• runtime PL310 DT node fixup (add "arm,io-coherent") property, due to CONFIG_SMP limitation (not relevant for us)

We tested and unfortunately it seems there is no way to add new property to the DT in FreeBSD (only modify existing one, but the value must be not bigger than original), hence another way should be made up.

I would really like to be able to use some mechanism (e.g. in platform_late_init). So basing your expertise, do you think it is possible to force settings of parent dma tag (nexus, ofwbus, simple-bus - whatever you prefer), so that all children (devices) can inherit this when creating theirs?

Yes, doing it in platform_late_init is just the sort of thing I was imagining, if it is a property of the entire system. Let me put together a little proposal-patch to see if my vague idea is practical.

Ok, looking forward to your proposal then.

Thanks,
Marcin

[• ian]

Proposing a different solution, which can only be done by commandeering the revision (the Phab authors apparently consider this a feature, not a bug).

The revised patch very likely needs some changes, so please just commandeer the revision back to your own ownership as needed.

[• ian]

An alternate proposal, which handles the root tag at the nexus level, with platform-level code to set up the tag when needed. The code that checks the FDT data probably needs some changes to check for the right thing, I just guessed at that part.

I hate that I had to make a whole new nexusvar.h file to prototype one function, but I couldn't find any existing private header file that made sense, and I didn't want to put it in something that gets installed in /usr/include/machine

[mw_semihalf.com]

@ian thank - I'll test and let know.

[mw_semihalf.com]

Hi @ian we cleaned the code a bit, solution seems nice, but... bus_dma_tag_create() won't work in platform_late_init stage, because it uses malloc :/ I have some hacks in mind how to overcome it, e.g. create the default dma tag in nexus code only when nexus_set_dma_tag() is called. The caller would only pass the desired flag and trigger tag creation inside nexus. What do you think?

[• ian]

In D11202#233463, @mw_semihalf.com wrote:

Hi @ian we cleaned the code a bit, solution seems nice, but... bus_dma_tag_create() won't work in platform_late_init stage, because it uses malloc :/ I have some hacks in mind how to overcome it, e.g. create the default dma tag in nexus code only when nexus_set_dma_tag() is called. The caller would only pass the desired flag and trigger tag creation inside nexus. What do you think?

Oh. Hmm, simple fix, add a "void *dummy" arg to ​mv_busdma_tag_init(), and instead of calling it from platform_late_init(), add:

SYSINIT(​mv_dmatag_init, SI_SUB_DRIVERS, SI_ORDER_ANY, ​mv_busdma_tag_init, NULL);

And I guess mv_busdma_tag_init() will need its guts wrapped in an #ifdef SOC_MV_ARMADA38X

SI_SUB_DRIVERS will make it run immediately before the device_add_child(root, nexus) which kicks off device configuration.

[mw_semihalf.com]

In D11202#233493, @ian wrote:

In D11202#233463, @mw_semihalf.com wrote:

Hi @ian we cleaned the code a bit, solution seems nice, but... bus_dma_tag_create() won't work in platform_late_init stage, because it uses malloc :/ I have some hacks in mind how to overcome it, e.g. create the default dma tag in nexus code only when nexus_set_dma_tag() is called. The caller would only pass the desired flag and trigger tag creation inside nexus. What do you think?

Oh. Hmm, simple fix, add a "void *dummy" arg to ​mv_busdma_tag_init(), and instead of calling it from platform_late_init(), add:

SYSINIT(​mv_dmatag_init, SI_SUB_DRIVERS, SI_ORDER_ANY, ​mv_busdma_tag_init, NULL);

Thanks, will try that.

And I guess mv_busdma_tag_init() will need its guts wrapped in an #ifdef SOC_MV_ARMADA38X

SI_SUB_DRIVERS will make it run immediately before the device_add_child(root, nexus) which kicks off device configuration.

Indeed device_add_child(root, nexus) is SI_SUB_CONFIGURE, which appears to be after SI_SUB_DRIVERS, however it's a bit misleading comment next to the latter's enum definition (/* Let Drivers initialize */). I'll check it and update patches if this works.

[• ian]

In D11202#233505, @mw_semihalf.com wrote:

In D11202#233493, @ian wrote:

In D11202#233463, @mw_semihalf.com wrote:

[...]

Indeed device_add_child(root, nexus) is SI_SUB_CONFIGURE, which appears to be after SI_SUB_DRIVERS, however it's a bit misleading comment next to the latter's enum definition (/* Let Drivers initialize */). I'll check it and update patches if this works.

Actually, the comment is correct. SI_SUB_DRIVERS is for *drivers* to init. SI_SUB_CONFIGURE is for *devices* to init. You can see by the existing use of SI_SUB_DRIVERS that some people have confused the two things. In your case, you are initializing the nexus driver -- getting it prepared to be attached as the nexus device during the CONFIGURE pass.

[mw_semihalf.com]

Need to take over the revision for the patch update.

[mw_semihalf.com]

Extract @ian 's changes around nexus and modify the commit log.