Anilkumar Kolli <akolli@xxxxxxxxxxxxxx> writes:
> Host DDR memory (contiguous 45 MB in mode-0 or 15 MB in mode-2)
> is reserved through DT entries for firmware usage. Send the base
> address and size from DT entries.
>
> If DT entry is available, PCI devices work with
> fixed_mem_region else host allocates multiple segments.
>
> IPQ8074 on HK10 board supports multiple PCI devices.
> IPQ8074 + QCN9074 is tested with this patch.
>
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.4.0.1-01838-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Anilkumar Kolli <akolli@xxxxxxxxxxxxxx>
> ---
> V2:
> - Use of_ API to read from dt node (Rob)
>
> drivers/net/wireless/ath/ath11k/mhi.c | 42 ++++++++++++++++++--
> drivers/net/wireless/ath/ath11k/pci.c | 10 ++++-
> drivers/net/wireless/ath/ath11k/qmi.c | 75 +++++++++++++++++++++++++++++++----
> drivers/net/wireless/ath/ath11k/qmi.h | 1 +
> 4 files changed, 116 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
> index 26c7ae242db6..0ea6ab7edeb9 100644
> --- a/drivers/net/wireless/ath/ath11k/mhi.c
> +++ b/drivers/net/wireless/ath/ath11k/mhi.c
> @@ -3,6 +3,7 @@
>
> #include <linux/msi.h>
> #include <linux/pci.h>
> +#include <linux/of.h>
>
> #include "core.h"
> #include "debug.h"
> @@ -316,7 +317,11 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
> struct ath11k_base *ab = ab_pci->ab;
> struct mhi_controller *mhi_ctrl;
> struct mhi_controller_config *ath11k_mhi_config;
> - int ret;
> + struct device_node *np;
> + int ret, len, sw, aw;
> + u32 *reg, *reg_end;
> + unsigned long start, size;
> + bool no_dt_entry = 0;
Use only true or false with bools.
> mhi_ctrl = mhi_alloc_controller();
> if (!mhi_ctrl)
> @@ -339,8 +344,39 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
> return ret;
> }
>
> - mhi_ctrl->iova_start = 0;
> - mhi_ctrl->iova_stop = 0xffffffff;
> + np = of_find_node_by_type(NULL, "memory");
> + if (!np) {
> + no_dt_entry = 1;
true
> + goto no_dt_entry;
> + }
> +
> + aw = of_n_addr_cells(np);
> + sw = of_n_size_cells(np);
> +
> + reg = (unsigned int *)of_get_property(np, "reg", &len);
Always try to avoid casts. of_get_property() returns 'const void *' so
you are casting out const here, which is a bug. You don't need to do
casting with void pointers so remove it here.
But is reg property really unsigned int? And not u32 or dma_addr_t, for
example?
> + if (!reg) {
> + no_dt_entry = 1;
true
> + goto no_dt_entry;
> + }
> +
> + reg_end = reg + len / (aw * sw);
> +
> + do {
> + start = of_read_number(reg, aw);
> + reg += aw;
> + size = of_read_number(reg, sw);
of_read_number() takes 'const __be32 *cell' but reg is 'u32 *'?
> + reg += sw;
> + } while (reg < reg_end);
> +
> +no_dt_entry:
> + if (no_dt_entry) {
> + mhi_ctrl->iova_start = 0;
> + mhi_ctrl->iova_stop = 0xFFFFFFFF;
> + } else {
> + mhi_ctrl->iova_start = (dma_addr_t)(start + 0x1000000);
> + mhi_ctrl->iova_stop = (dma_addr_t)(start + size);
I don't like casts, they hide bugs like the const issue above. Is there
any way to do this without casts?
> @@ -1245,6 +1246,13 @@ static int ath11k_pci_probe(struct pci_dev *pdev,
> pci_set_drvdata(pdev, ab);
> spin_lock_init(&ab_pci->window_lock);
>
> + /* Set fixed_mem_region to true for platforms support reserved memory
> + * from DT. If memory is reserved from DT for FW, ath11k driver need not
> + * allocate memory.
> + */
> + if (!of_property_read_u32(ab->dev->of_node, "memory-region", &addr))
> + ab->bus_params.fixed_mem_region = true;
ab->bus_params is supposed to be const to catch this, but clearly it's
not. The idea of bus_params is to have static bus level configuration,
please instead use dev_flags or something else.
> static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
> {
> - int i, idx;
> + struct device *dev = ab->dev;
> + struct device_node *hremote_node = NULL;
> + phandle hremote_phandle;
> + int i, idx, len, sw, aw, host_ddr_sz;
> + u32 *reg, *reg_end;
> + u64 start, size;
>
> for (i = 0, idx = 0; i < ab->qmi.mem_seg_count; i++) {
> switch (ab->qmi.target_mem[i].type) {
> + case HOST_DDR_REGION_TYPE:
> + if (of_property_read_u32(dev->of_node, "memory-region",
> + &hremote_phandle)) {
> + ath11k_dbg(ab, ATH11K_DBG_QMI,
> + "qmi fail to get hremote phandle\n");
> + return 0;
> + }
> +
> + hremote_node = of_find_node_by_phandle(hremote_phandle);
> + if (!hremote_node) {
> + ath11k_dbg(ab, ATH11K_DBG_QMI,
> + "qmi fail to get hremote_node\n");
> + return 0;
> + }
> +
> + aw = of_n_addr_cells(hremote_node);
> + sw = of_n_size_cells(hremote_node);
> +
> + reg = (unsigned int *)of_get_property(hremote_node, "reg", &len);
No need to cast. And I guess you are again removing the const?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
