Hi Nicolas,
On Mon, 2021-08-23 at 16:35 +0200, Nicolas Frattaroli wrote: [...]
I'm not too fond of reimplementing half a reset controller in here. The reset framework does not support synchronized (de)assertion of multiple reset controls, I wonder if this would be useful to add. Without having thought about this too hard, I could imagine this as an extension to the bulk API, or possibly a call to join multiple reset controls into a reset control array.
I agree, the code required for synchronised reset seems to be a good candidate for a generalised solution elsewhere. However, I'm not sure if I'm the right candidate to design this API as my first kernel contribution when the board I'm currently testing this with doesn't even utilise the synchronized reset.
If I come across an opportunity to test synchronised resets, I'll definitely look into refactoring this. I'd also be happy to hear of any other driver which is currently implementing its own synchronised reset.
[...]
What is the reason for the different delays in rockchip_snd_xfer_sync_reset() and rockchip_snd_reset()?
Simply put: I have no idea. This is what downstream does, and it appears to work for me. The git history of the downstream kernel also doesn't tell me anything about why the sync reset is 150 and this one is 1.
I've got no device to test the sync reset with at the moment so I'm wary of playing with the delay value.
Fair points. You could remove the untested synchronized reset support for now; that would allow to get rid of the rockchip,cru device tree property, which is only required to let this driver access the CRU registers behind the clock/reset controller driver's back.
[...]
- if (i2s_tdm->clk_trcm != TRCM_TXRX) {
cru_node = of_parse_phandle(node, "rockchip,cru", 0);i2s_tdm->cru_base = of_iomap(cru_node, 0);This is a bit ugly if there is another driver sitting on the rockchip,cru compatible node. Which reset controller driver is backing the reset controls below?
I'm not sure if I understand the question (I only just today learned that reset controls have drivers) but I think the reset it is using in the Quartz64 dts is backed by rk3568-cru. Let me know if I misunderstood you.
That's the one, thanks.
The devm_reset_control_get() calls below follow the "reset-names" and "resets" device tree properties. Those point (&cru) to a clock-controller node with compatible = "rockchip,rk3568-cru".
The corresponding driver is drivers/clk/rockchip/clk-rk3568.c, so the reset controller is provided by the reset_controller_register() call in drivers/clk/rockchip/softrst.c.
regards Philipp