Re: [PATCH 4/4] bus: add BCM6362 on-chip WLAN SHIM bridge driver

[Philipp Zabel]

On Fr, 2026-05-29 at 02:06 +0200, Alessio Ferri wrote:
> Add the bridge driver that brings up the BCM6362 on-chip WLAN SHIM
> and then populates a brcm,bus-axi child whose backplane is
> enumerated by drivers/bcma/host_soc.c.
> 
> Add myself as MANTAINER for this shim.
>
[...]
> 
> Assisted-by: Claude:claude-4.8-opus
> Signed-off-by: Alessio Ferri <alessio.ferri@xxxxxxxxxxx>
> ---
>  MAINTAINERS                     |   7 ++
>  drivers/bus/Kconfig             |  13 +++
>  drivers/bus/Makefile            |   1 +
>  drivers/bus/bcm6362-wlan-shim.c | 252 ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 273 insertions(+)
> 
[...]
> diff --git a/drivers/bus/bcm6362-wlan-shim.c b/drivers/bus/bcm6362-wlan-shim.c
> new file mode 100644
> index 000000000000..a2de03cf8ff7
> --- /dev/null
> +++ b/drivers/bus/bcm6362-wlan-shim.c
> @@ -0,0 +1,252 @@
[...]
> +static int bcm6362_wlan_bringup(struct bcm6362_wlan *priv)
> +{
> +	int ret;
> +
> +	dev_info(priv->dev, "bring-up: start\n");

Too verbose.

> +
> +	ret = clk_prepare_enable(priv->clk);
> +	if (ret) {
> +		dev_err(priv->dev, "clk_prepare_enable failed: %d\n", ret);
> +		return ret;
> +	}
> +	dev_info(priv->dev, "bring-up: clock enabled, rate=%lu Hz\n",
> +		 clk_get_rate(priv->clk));
> +	mdelay(10);

Why is this a delay instead of a sleep? Why is it there at all?
Prefer using msleep() or fsleep() if possible.

> +
> +	/* Reset toggle (brcm,bcm6345-reset hides the active-low softResetB
> +	 * encoding, so assert/deassert read naturally here).
> +	 */
> +	reset_control_assert(priv->rst_shim_ubus);
> +	reset_control_assert(priv->rst_shim);
> +	mdelay(1);
> +	reset_control_deassert(priv->rst_shim_ubus);
> +	reset_control_deassert(priv->rst_shim);

Switch deassert order for consistency with teardown, if possible.
Or use reset_control_bulk_(de)assert() API.

[...]

> +static int bcm6362_wlan_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct of_dev_auxdata auxdata[2];
> +	struct bcm6362_wlan *priv;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +	priv->dev = dev;
> +
> +	priv->shim = devm_platform_ioremap_resource_byname(pdev, "shim");
> +	if (IS_ERR(priv->shim))
> +		return PTR_ERR(priv->shim);
> +
> +	priv->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(priv->clk))
> +		return PTR_ERR(priv->clk);
> +
> +	priv->rst_shim = devm_reset_control_get_exclusive(dev, "shim");
> +	if (IS_ERR(priv->rst_shim))
> +		return PTR_ERR(priv->rst_shim);
> +
> +	priv->rst_shim_ubus = devm_reset_control_get_exclusive(dev,
> +							       "shim-ubus");
> +	if (IS_ERR(priv->rst_shim_ubus))
> +		return PTR_ERR(priv->rst_shim_ubus);

Consider using devm_reset_control_bulk_get_exclusive().

regards
Philipp