On 2024/8/10 17:44, Sai Krishna Gajula [off-list ref] wrote:

quoted

-----Original Message-----
From: Jacobe Zang <redacted>
Sent: Saturday, August 10, 2024 9:22 AM
To: robh@kernel.org; krzk+dt@kernel.org; heiko@sntech.de;
kvalo@kernel.org; davem@davemloft.net; edumazet@google.com;
kuba@kernel.org; pabeni@redhat.com; conor+dt@kernel.org;
arend.vanspriel@broadcom.com
Cc: efectn@protonmail.com; dsimic@manjaro.org; jagan@edgeble.ai;
devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
rockchip@lists.infradead.org; linux-kernel@vger.kernel.org;
arend@broadcom.com; linux-wireless@vger.kernel.org;
netdev@vger.kernel.org; megi@xff.cz; duoming@zju.edu.cn;
bhelgaas@google.com; minipli@grsecurity.net; brcm80211@lists.linux.dev;
brcm80211-dev-list.pdl@broadcom.com; nick@khadas.com; Jacobe Zang
[off-list ref]
Subject:  [PATCH v9 4/5] wifi: brcmfmac: Add optional lpo clock
enable support

WiFi modules often require 32kHz clock to function. Add support to enable
the clock to PCIe driver and move "brcm,bcm4329-fmac" check to the top of
brcmf_of_probe. Change function prototypes from void to int and add
appropriate errno's for return
WiFi modules often require 32kHz clock to function. Add support to enable
the clock to PCIe driver and move "brcm,bcm4329-fmac" check to the top of
brcmf_of_probe. Change function prototypes from void to int and add
appropriate errno's for return values that will be send to bus when error
occurred.

Co-developed-by: Ondrej Jirman <megi@xff.cz>
Signed-off-by: Ondrej Jirman <megi@xff.cz>
Co-developed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Jacobe Zang <redacted>
---
  .../broadcom/brcm80211/brcmfmac/bcmsdh.c      |  4 +-
  .../broadcom/brcm80211/brcmfmac/common.c      |  3 +-
  .../wireless/broadcom/brcm80211/brcmfmac/of.c | 53 +++++++++++--------
.../wireless/broadcom/brcm80211/brcmfmac/of.h |  9 ++--
  .../broadcom/brcm80211/brcmfmac/pcie.c        |  3 ++
  .../broadcom/brcm80211/brcmfmac/sdio.c        | 24 ++++++---
  .../broadcom/brcm80211/brcmfmac/usb.c         |  3 ++
  7 files changed, 63 insertions(+), 36 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c

b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 13391c2d82aae..b2ede4e579c5c 100644

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c

@@ -947,8 +947,8 @@ int brcmf_sdiod_probe(struct brcmf_sdio_dev

*sdiodev)

  	/* try to attach to the target device */
  	sdiodev->bus = brcmf_sdio_probe(sdiodev);
-	if (!sdiodev->bus) {
-		ret = -ENODEV;
+	if (IS_ERR(sdiodev->bus)) {
+		ret = PTR_ERR(sdiodev->bus);
  		goto out;
  	}
  	brcmf_sdiod_host_fixup(sdiodev->func2->card->host);

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c

b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index b24faae35873d..58d50918dd177 100644

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c

@@ -561,7 +561,8 @@ struct brcmf_mp_device

*brcmf_get_module_param(struct device *dev,
  	if (!found) {
  		/* No platform data for this device, try OF and DMI data */
  		brcmf_dmi_probe(settings, chip, chiprev);
-		brcmf_of_probe(dev, bus_type, settings);
+		if (brcmf_of_probe(dev, bus_type, settings) == -
EPROBE_DEFER)
+			return ERR_PTR(-EPROBE_DEFER);
  		brcmf_acpi_probe(dev, bus_type, settings);
  	}
  	return settings;

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c

b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
index e406e11481a62..f19dc7355e0e8 100644

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/of.c

@@ -6,6 +6,7 @@
  #include <linux/of.h>
  #include <linux/of_irq.h>
  #include <linux/of_net.h>
+#include <linux/clk.h>

  #include <defs.h>
  #include "debug.h"

@@ -65,17 +66,21 @@ static int brcmf_of_get_country_codes(struct device

*dev,
  	return 0;
  }

-void brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
-		    struct brcmf_mp_device *settings)
+int brcmf_of_probe(struct device *dev, enum brcmf_bus_type bus_type,
+		   struct brcmf_mp_device *settings)
  {
  	struct brcmfmac_sdio_pd *sdio = &settings->bus.sdio;
  	struct device_node *root, *np = dev->of_node;
+	struct clk *clk;
  	const char *prop;

Small nit, please check if reverse x-mas tree order need to be follow here.

quoted

  	int irq;
  	int err;
  	u32 irqf;

It can be seen from this line that there should be no need to follow the reverse x-mas tree order. Because it is a struct variable, so place with other struct ones.

quoted

  	u32 val;

+	if (!np || !of_device_is_compatible(np, "brcm,bcm4329-fmac"))
+		return 0;
+
  	/* Apple ARM64 platforms have their own idea of board type, passed
in
  	 * via the device tree. They also have an antenna SKU parameter
  	 */

@@ -105,7 +110,7 @@ void brcmf_of_probe(struct device *dev, enum

brcmf_bus_type bus_type,
  		board_type = devm_kstrdup(dev, tmp, GFP_KERNEL);
  		if (!board_type) {
  			of_node_put(root);
-			return;
+			return 0;
  		}
  		strreplace(board_type, '/', '-');
  		settings->board_type = board_type;

@@ -113,33 +118,39 @@ void brcmf_of_probe(struct device *dev, enum

brcmf_bus_type bus_type,
  		of_node_put(root);
  	}

-	if (!np || !of_device_is_compatible(np, "brcm,bcm4329-fmac"))
-		return;
-
  	err = brcmf_of_get_country_codes(dev, settings);
  	if (err)
  		brcmf_err("failed to get OF country code map (err=%d)\n",
err);

  	of_get_mac_address(np, settings->mac);

-	if (bus_type != BRCMF_BUSTYPE_SDIO)
-		return;
+	if (bus_type == BRCMF_BUSTYPE_SDIO) {
+		if (of_property_read_u32(np, "brcm,drive-strength", &val) ==
0)
+			sdio->drive_strength = val;

-	if (of_property_read_u32(np, "brcm,drive-strength", &val) == 0)
-		sdio->drive_strength = val;
+		/* make sure there are interrupts defined in the node */
+		if (!of_property_present(np, "interrupts"))
+			return 0;

-	/* make sure there are interrupts defined in the node */
-	if (!of_property_present(np, "interrupts"))
-		return;
+		irq = irq_of_parse_and_map(np, 0);
+		if (!irq) {
+			brcmf_err("interrupt could not be mapped\n");
+			return 0;
+		}
+		irqf = irqd_get_trigger_type(irq_get_irq_data(irq));
+
+		sdio->oob_irq_supported = true;
+		sdio->oob_irq_nr = irq;
+		sdio->oob_irq_flags = irqf;
+	}

-	irq = irq_of_parse_and_map(np, 0);
-	if (!irq) {
-		brcmf_err("interrupt could not be mapped\n");
-		return;
+	clk = devm_clk_get_optional_enabled(dev, "lpo");
+	if (!IS_ERR_OR_NULL(clk)) {
+		brcmf_dbg(INFO, "enabling 32kHz clock\n");
+		return clk_set_rate(clk, 32768);
+	} else {
+		return PTR_ERR_OR_ZERO(clk);
  	}
-	irqf = irqd_get_trigger_type(irq_get_irq_data(irq));

-	sdio->oob_irq_supported = true;
-	sdio->oob_irq_nr = irq;
-	sdio->oob_irq_flags = irqf;
+	return 0;

....
....

quoted

  /* Detach and free everything */

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c

b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
index 9a105e6debe1f..f7db46ae44906 100644

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c

@@ -1272,6 +1272,9 @@ static int brcmf_usb_probe_cb(struct

brcmf_usbdev_info *devinfo,
  		ret = -ENOMEM;
  		goto fail;
  	}
+	ret = PTR_ERR_OR_ZERO(devinfo->settings);
+	if (ret < 0)
+		goto fail;

  	if (!brcmf_usb_dlneeded(devinfo)) {
  		ret = brcmf_alloc(devinfo->dev, devinfo->settings);
--
2.34.1

Reviewed-by:  Sai Krishna <redacted>

--
Best Regards
Jacobe