Re: Aw: Re: [PATCH v1 0/4] add syscon requirement for mt7988

On Wed, Jul 10, 2024 at 02:50:42PM +0200, AngeloGioacchino Del Regno wrote:

Il 10/07/24 13:34, Frank Wunderlich ha scritto:

quoted

Hi

quoted

Gesendet: Mittwoch, 10. Juli 2024 um 12:45 Uhr
Von: "AngeloGioacchino Del Regno" [off-list ref]
Betreff: Re: [PATCH v1 0/4] add syscon requirement for mt7988

Il 09/07/24 12:13, Frank Wunderlich ha scritto:

quoted

From: Frank Wunderlich <redacted>

Some nodes require the syscon fallback at least in u-boot when using
OF_UPSTREAM.

This is because uboot driver uses syscon_node_to_regmap in mtk_eth.c for
"mediatek,toprgu", "mediatek,xfi_pll" and reset pointing to watchdog-node.

I wonder what's the major blocker here to modify the u-boot driver to take
the upstream devicetree as-is, instead of using syscon_node_to_regmap?

in uboot there is no driver for all syscon and to handle parallel 
access this is done with the syscon fallback.

The syscon uclass is a small driver which is generic and only 
handle the regmap in global context.

In theory it could be possible that regmap is aquired twice when 
used from 2+ other drivers...to prevent this without
adding the syscon fallback each syscon needs a dedicated driver 
like in linux which does only syscon stuff (code
duplication at its best :) ).

of course i can use regmap_init_mem in the uboot ethernet driver

https://elixir.bootlin.com/u-boot/latest/source/drivers/core/regmap.c#L242

like it's done once for syscon-uclass.

but i will cause issues when a second device tries to access this 
regmap. So it was be much easier (for me) to add this
fallback and not writing 3 device-drivers in uboot doing the 
exactly same as syscon.

if you have a better idea how to handle it, let me know :)

I see. The problem is that, from your description, it looks like u-boot
uses that as a kind of workaround for concurrent access to MMIO...

...looks like a good topic to discuss in the u-boot mailing lists.

Definitely, the TOPRGU and the XFI PLL are not system controllers, so the actual
"syscon" definition would be wrong for these, that's it.

While I'd prefer "syscon" never existed in the first place, I don't care 
too much if it gets added here or not. U-boot's reasoning for wanting it 
isn't really much better or worse than Linux's. Though if u-boot has 
multiple drivers using it, seems like an abstraction is missing if Linux 
doesn't need that.

Rob