On 6.12.2024 10:56 AM, Raj Kumar Bhagat wrote:
> On 10/19/2024 1:59 AM, Konrad Dybcio wrote:
>> On 15.10.2024 8:26 PM, Raj Kumar Bhagat wrote:
>>> From: Balamurugan S <quic_bselvara@xxxxxxxxxxx>
>>>
>>> Add Initial Ath12k AHB driver support for IPQ5332. IPQ5332 is AHB
>>> based IEEE802.11be 2 GHz 2x2 WiFi device.
>>>
>>> Tested-on: IPQ5332 hw1.0 AHB WLAN.WBE.1.3.1-00130-QCAHKSWPL_SILICONZ-1
>>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.1.1-00210-QCAHKSWPL_SILICONZ-1
>>>
>>> Signed-off-by: Balamurugan S <quic_bselvara@xxxxxxxxxxx>
>>> Co-developed-by: P Praneesh <quic_ppranees@xxxxxxxxxxx>
>>> Signed-off-by: P Praneesh <quic_ppranees@xxxxxxxxxxx>
>>> Co-developed-by: Raj Kumar Bhagat <quic_rajkbhag@xxxxxxxxxxx>
>>> Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@xxxxxxxxxxx>
>>> ---
>>
>> [...]
>>
>>> +enum ext_irq_num {
>>> + host2wbm_desc_feed = 16,
>>> + host2reo_re_injection,
>>
>> Why?
>>
>
> This enum is used as a IRQ number for Ath12k AHB. Based on this enum
> we can get the IRQ name from irq_name[]. This helps to request the original
> IRQ number from the DT.
> It is starting from 16 becasue, in irq_name[], the name for ext IRQ starts
> from 16 index.
[...]
>
>>> + irq_grp->irqs[num_irq++] =
>>> + reo2host_destination_ring1 - j;
>>> + }
>>> +
>>> + if (ab->hw_params->ring_mask->rx_err[i] & BIT(j))
>>> + irq_grp->irqs[num_irq++] = reo2host_exception;
>>> +
>>> + if (ab->hw_params->ring_mask->rx_wbm_rel[i] & BIT(j))
>>> + irq_grp->irqs[num_irq++] = wbm2host_rx_release;
>>> +
>>> + if (ab->hw_params->ring_mask->reo_status[i] & BIT(j))
>>> + irq_grp->irqs[num_irq++] = reo2host_status;
>>> +
>>> + if (ab->hw_params->ring_mask->rx_mon_dest[i] & BIT(j))
>>> + irq_grp->irqs[num_irq++] =
>>> + rxdma2host_monitor_destination_mac1;
>>> + }
>>> +
>>> + irq_grp->num_irq = num_irq;
>>> +
>>> + for (j = 0; j < irq_grp->num_irq; j++) {
>>> + irq_idx = irq_grp->irqs[j];
>>> +
>>> + irq = platform_get_irq_byname(ab->pdev,
>>> + irq_name[irq_idx]);
>>> + ab->irq_num[irq_idx] = irq;
>>> + irq_set_status_flags(irq, IRQ_NOAUTOEN | IRQ_DISABLE_UNLAZY);
>>> + ret = request_irq(irq, ath12k_ahb_ext_interrupt_handler,
>>> + IRQF_TRIGGER_RISING,
>>> + irq_name[irq_idx], irq_grp);
>>> + if (ret) {
>>> + ath12k_err(ab, "failed request_irq for %d\n",
>>> + irq);
>>> + }
>>> + }
>>
>> Instead of doing all this magic, can we request the IRQs manually, as we
>> have interrupt-names in dt?
>>
>
> I'm not sure if I fully understood this comment.
> If we manually request IRQs using their names from the DT, we won't be able to
> group the IRQs. Grouping the IRQs is one of our main objectives here. Additionally,
> we are not using all the IRQ names defined in the DT, so the logic in this function
> is crucial for grouping and requesting the IRQs according to the ring-mask.
Surely you can name these "foo_bar_ring%d" in DT and use the OF APIs
[...]
>>
>>> + /* Set fixed_mem_region to true for platforms that support fixed memory
>>> + * reservation from DT. If memory is reserved from DT for FW, ath12k driver
>>> + * need not to allocate memory.
>>> + */
>>> + if (!of_property_read_u32(ab->dev->of_node, "memory-region", &addr)) {
>>> + set_bit(ATH12K_FLAG_FIXED_MEM_REGION, &ab->dev_flags);
>>> + mem_node = of_find_node_by_name(NULL, "mlo_global_mem_0");
>>
>> This is not mentioned or documented anywhere.
>>
>
> In next version, will document the below info:
>
> "If the platform supports fixed memory, then it should define/reserve
> MLO global memory in DT to support Multi Link Operation.
> If MLO global memory is not reserved in fixed memory mode, then
> MLO cannot be supported."
You should also explain what Multi Link Operation means
Konrad
