[PATCH v2] slimbus: qcom-ngd-ctrl: fix ngd register base address

[PATCH v2] slimbus: qcom-ngd-ctrl: fix ngd register base address

[Federico Amedeo Izzo via B4 Relay]

From: Federico Amedeo Izzo <federico@izzo.pro>

The slimbus controller register base was calculated using the
controller ID and the register size, producing wrong results for ID > 1.

The expression reduces to ngd->base = ctrl->base for controller with ID = 1;
but for ID > 1 the other factors produce a wrong base address.
This bug caused a crash while trying to bring-up the secondary
slimbus controller (ID = 3) on oneplus-enchilada with sdm845.

In practice, the correct base address for each slimbus controller can be
obtained from the device-tree, so there is no need for further
calculations.

This was validated by reading the version register from the WCN3990 slim
interface on oneplus-enchilada.

Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
Signed-off-by: Federico Amedeo Izzo <federico@izzo.pro>
---
Changes in v2:
- add missing Fixes and S-o-b tags
- Link to v1: https://patch.msgid.link/20260709-qcom-ngd-register-base-fix-v1-1-5f7ab48f9209@izzo.pro
---
 drivers/slimbus/qcom-ngd-ctrl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index 80877e951849..91932f8d021b 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -1547,8 +1547,7 @@ static int of_qcom_slim_ngd_register(struct device *parent,
 			kfree(ngd);
 			return ret;
 		}
-		ngd->base = ctrl->base + ngd->id * data->offset +
-					(ngd->id - 1) * data->size;
+		ngd->base = ctrl->base + data->offset;
 
 		return 0;
 	}

---
base-commit: b9810cd75b9fb56a3425d391cba3f608502bd474
change-id: 20260709-qcom-ngd-register-base-fix-a59713a184b8

Best regards,
--  
Federico Amedeo Izzo <federico@izzo.pro>

Re: [PATCH v2] slimbus: qcom-ngd-ctrl: fix ngd register base address

[Konrad Dybcio]

On 7/10/26 12:28 PM, Federico Amedeo Izzo via B4 Relay wrote:
> From: Federico Amedeo Izzo <federico@izzo.pro>
> 
> The slimbus controller register base was calculated using the
> controller ID and the register size, producing wrong results for ID > 1.
> 
> The expression reduces to ngd->base = ctrl->base for controller with ID = 1;
> but for ID > 1 the other factors produce a wrong base address.
> This bug caused a crash while trying to bring-up the secondary
> slimbus controller (ID = 3) on oneplus-enchilada with sdm845.

SDM845 has 2 slimbus controllers:

AUD_SLIM @ 0x171c0000 (the currently described node in sdm845.dtsi)
QCA_SLIM @ 0x17240000 (the one you're trying to enable)

Each one of these controllers has 2 NGDs (indexed 1 and 2), located
at an offset of +0x1000 from the base of the controller and spaced
0x1000 apart from each other (i.e. the current calculation is right)

You do however need to pass the right ID to the QMI message, so the
bug is in this line:

req.instance = (ctrl->ngd->id >> 1);

This shouldn't be the same ID.

Downstream solves this using a `cell-index` property, which
represents the ordinal of the instance of a hw block, but that's
highly discouraged in dt-bindings nowadays. Instead, the recommended
way would be to introduce a struct like:

{ .base = 0x171c0000, .index = 1 },
{ .base = 0x17240000, .index = 3 },

However that would further need a sdm845-specific compatible to bind
the data to this specific SoC. I think we can introduce one without any
regressions (because the other slimbus controller is not yet described)

Konrad