diff mbox series

[v1,2/3] net: wireless: brcmfmac: Add optional 32k clock enable support

Message ID 20240620020015.4021696-3-jacobe.zang@wesion.com (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series Add AP6275P wireless support | expand

Commit Message

Jacobe Zang June 20, 2024, 2 a.m. UTC
WiFi modules often require 32kHz clock to function. Add support to
enable the clock to PCIe driver.

Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c    | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Ondřej Jirman June 20, 2024, 6:22 a.m. UTC | #1
Hi,

On Thu, Jun 20, 2024 at 10:00:14AM GMT, Jacobe Zang wrote:
> WiFi modules often require 32kHz clock to function. Add support to
> enable the clock to PCIe driver.

There's also this approach https://lore.kernel.org/lkml/20240117160748.37682-1-brgl@bgdev.pl/

> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
> ---
>  .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c    | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index 06698a714b523..f241e1757d7e3 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -3,6 +3,7 @@
>   * Copyright (c) 2014 Broadcom Corporation
>   */
>  
> +#include <linux/clk.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/firmware.h>
> @@ -2411,6 +2412,7 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	struct brcmf_pciedev *pcie_bus_dev;
>  	struct brcmf_core *core;
>  	struct brcmf_bus *bus;
> +	struct clk *clk;
>  
>  	if (!id) {
>  		id = pci_match_id(brcmf_pcie_devid_table, pdev);
> @@ -2422,6 +2424,14 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	brcmf_dbg(PCIE, "Enter %x:%x\n", pdev->vendor, pdev->device);
>  
> +	clk = devm_clk_get_optional_enabled(&pdev->dev, "32k");

If you do this, you need to update the DT bindings. Current ones don't have 32k
clock.

> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +	if (clk) {
> +		dev_info(&pdev->dev, "enabling 32kHz clock\n");

The driver has its own debug logging infrastructure (brcmf_dbg).

Kind regards,
	o.

> +		clk_set_rate(clk, 32768);
> +	}
> +
>  	ret = -ENOMEM;
>  	devinfo = kzalloc(sizeof(*devinfo), GFP_KERNEL);
>  	if (devinfo == NULL)
> -- 
> 2.34.1
>
Krzysztof Kozlowski June 20, 2024, 6:54 a.m. UTC | #2
On 20/06/2024 04:00, Jacobe Zang wrote:
> WiFi modules often require 32kHz clock to function. Add support to
> enable the clock to PCIe driver.
> 
> Signed-off-by: Jacobe Zang <jacobe.zang@wesion.com>
> ---
>  .../net/wireless/broadcom/brcm80211/brcmfmac/pcie.c    | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> index 06698a714b523..f241e1757d7e3 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
> @@ -3,6 +3,7 @@
>   * Copyright (c) 2014 Broadcom Corporation
>   */
>  
> +#include <linux/clk.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/firmware.h>
> @@ -2411,6 +2412,7 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	struct brcmf_pciedev *pcie_bus_dev;
>  	struct brcmf_core *core;
>  	struct brcmf_bus *bus;
> +	struct clk *clk;
>  
>  	if (!id) {
>  		id = pci_match_id(brcmf_pcie_devid_table, pdev);
> @@ -2422,6 +2424,14 @@ brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	brcmf_dbg(PCIE, "Enter %x:%x\n", pdev->vendor, pdev->device);
>  
> +	clk = devm_clk_get_optional_enabled(&pdev->dev, "32k");

Where is the binding for this?

> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +	if (clk) {
> +		dev_info(&pdev->dev, "enabling 32kHz clock\n");

Drop



Best regards,
Krzysztof
Jacobe Zang June 21, 2024, 1:45 a.m. UTC | #3
Hello,

> Where is the binding for this?
I seperate the binding in this patch " [PATCH v1 1/3] arm64: dts: rockchip: Add AP6275P wireless support to Khadas Edge 2 ", the specific code is 
+                       clocks = <&hym8563>;
+                       clock-names = "32k";

Should I combine these two patch to one?

---
Best Regards
Jacobe
Arend van Spriel June 21, 2024, 4:59 a.m. UTC | #4
On June 21, 2024 3:45:51 AM Jacobe Zang <jacobe.zang@wesion.com> wrote:

> Hello,
>
>> Where is the binding for this?
> I seperate the binding in this patch " [PATCH v1 1/3] arm64: dts: rockchip: 
> Add AP6275P wireless support to Khadas Edge 2 ", the specific code is
> +                       clocks = <&hym8563>;
> +                       clock-names = "32k";
>
> Should I combine these two patch to one?

That's not the binding. That is the device tree specification which must 
adhere to the binding specification under Documentation/device 
tree/bindings/net/wireless.

Regards,
Arend
Krzysztof Kozlowski June 21, 2024, 6:03 a.m. UTC | #5
On 21/06/2024 03:45, Jacobe Zang wrote:
> Hello,
> 
>> Where is the binding for this?
> I seperate the binding in this patch " [PATCH v1 1/3] arm64: dts: rockchip: Add AP6275P wireless support to Khadas Edge 2 ", the specific code is 
> +                       clocks = <&hym8563>;
> +                       clock-names = "32k";
> 
> Should I combine these two patch to one?

That's DTS, not binding. I ask about Devicetree binding.

Best regards,
Krzysztof
Jacobe Zang June 21, 2024, 7:45 a.m. UTC | #6
> That's DTS, not binding. I ask about Devicetree binding.

Ok... I have grep in all dts files and can't find wifi node which is under pcie node has clock. So should I add an example in the yaml file? 

Just like:

diff --git a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
index e564f20d8f415..2d9f8de447659 100644
--- a/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/brcm,bcm4329-fmac.yaml
@@ -153,3 +153,19 @@ examples:
         brcm,ccode-map = "JP-JP-78", "US-Q2-86";
       };
     };
+  - |
+    pcie@0,0 {
+                 reg = <0x400000 0 0 0 0>;
+                 #address-cells = <3>;
+                 #size-cells = <2>;
+                 ranges;
+                 device_type = "pci";
+                 bus-range = <0x40 0x4f>;
+
+                 wifi: wifi@0,0 {
+                         compatible = "pci14e4,449d";
+                         reg = <0x410000 0 0 0 0>;
+                         clocks = <&hym8563>;
+                         clock-names = "32k";
+                 };
+         };

---
Best Regards
Jacobe
Krzysztof Kozlowski June 21, 2024, 10:06 a.m. UTC | #7
On 21/06/2024 09:45, Jacobe Zang wrote:
>> That's DTS, not binding. I ask about Devicetree binding.
> 
> Ok... I have grep in all dts files and can't find wifi node which is under pcie node has clock. So should I add an example in the yaml file? 

No, that's example, not binding.

Validate your DTS with dtbs_check.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
index 06698a714b523..f241e1757d7e3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/pcie.c
@@ -3,6 +3,7 @@ 
  * Copyright (c) 2014 Broadcom Corporation
  */
 
+#include <linux/clk.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/firmware.h>
@@ -2411,6 +2412,7 @@  brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	struct brcmf_pciedev *pcie_bus_dev;
 	struct brcmf_core *core;
 	struct brcmf_bus *bus;
+	struct clk *clk;
 
 	if (!id) {
 		id = pci_match_id(brcmf_pcie_devid_table, pdev);
@@ -2422,6 +2424,14 @@  brcmf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	brcmf_dbg(PCIE, "Enter %x:%x\n", pdev->vendor, pdev->device);
 
+	clk = devm_clk_get_optional_enabled(&pdev->dev, "32k");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+	if (clk) {
+		dev_info(&pdev->dev, "enabling 32kHz clock\n");
+		clk_set_rate(clk, 32768);
+	}
+
 	ret = -ENOMEM;
 	devinfo = kzalloc(sizeof(*devinfo), GFP_KERNEL);
 	if (devinfo == NULL)