Krzysztof Kozlowski <krzk@xxxxxxxxxx> writes: > On 20/09/2024 08:45, Kalle Valo wrote: > >> Krzysztof Kozlowski <krzk@xxxxxxxxxx> writes: >> >>> On 19/09/2024 09:48, Kalle Valo wrote: >>>> Krzysztof Kozlowski <krzk@xxxxxxxxxx> writes: >>>> >>>>> On 14/08/2024 10:23, Bartosz Golaszewski wrote: >>>>>> From: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> >>>>>> >>>>>> Describe the inputs from the PMU of the ath11k module on WCN6855. >>>>>> >>>>>> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx> >>>>>> --- >>>>>> v1 -> v2: >>>>>> - update the example >>>>> >>>>> I don't understand why this patch is no being picked up. The code >>>>> correct represents the piece of hardware. The supplies should be >>>>> required, because this one particular device - the one described in this >>>>> binding - cannot work without them. >>>> >>>> I have already explained the situation. With supplies changed to >>>> optional I'm happy take the patch. >>> >>> You did not provide any relevant argument to this case. Your concerns >>> described quite different case and are no applicable to DT based platforms. >> >> Ok, I'll try to explain my concerns one more time. I'll try to be >> thorough so will be a longer mail. >> >> In ath11k we have board files, it's basically board/product specific >> calibration data which is combined with the calibration data from chip's >> OTP. Choosing the correct board file is essential as otherwise the >> performance can be bad or the device doesn't work at all. >> >> The board files are stored in board-2.bin file in /lib/firmware. ath11k >> chooses the correct board file based on the information provided by the >> ath11k firmware and then transfers the board file to firmware. From >> board-2.bin the correct board file is search based on strings like this: >> >> bus=pci,vendor=17cb,device=1103,subsystem-vendor=105b,subsystem-device=e0ca,qmi-chip-id=2,qmi-board-id=255 >> bus=pci,vendor=17cb,device=1103,subsystem-vendor=105b,subsystem-device=e0ca,qmi-chip-id=2,qmi-board-id=255,variant=HO_BNM >> >> But the firmware does not always provide unique enough information for >> choosing the correct board file and that's why we added the variant >> property (the second example above). This variant property gives us the >> means to name the board files uniquely and not have any conflicts. In >> x86 systems we retrieve it from SMBIOS and in DT systems using >> qcom,ath11k-calibration-variant property. >> >> If WCN6855 supplies are marked as required, it means that we cannot use >> qcom,ath11k-calibration-variant DT property anymore with WCN6855 M.2 >> boards. So if we have devices which don't provide unique information > > No, it does not mean that. > >> then for those devices it's impossible to automatically to choose the >> correct board file. > > Anyway, only this device must be fully described, because you cannot > have pci17cb,1103 without these supplies. It's just electrically not > possible, according to our investigation. Yeah, you have been telling that all along. But on the contrary I have WCN6855 (pci17cb,1103) M.2 board which I installed to my NUC and they haven't needed any supplies (unless BIOS does something automatically). Also I have QCA6390 and WCN7850 M.2 boards, both which you claim needs the supplies, and they just work out-of-box as well. So I have a hard time trusting your spec and believing it's the ultimate authority. To me if reality and spec doesn't match, reality wins. >> So based on this, to me the correct solution here is to make the >> supplies optional so that qcom,ath11k-calibration-variant DT property >> can continue to be used with WCN6855 M.2 boards. > > WCN6855 can still do whatever they want. They are not restricted, not > limited. pci17cb,1103 must provide suppplies, because pci17cb,1103 > cannot work without them. Claiming that WCN6885 can still do whatever they want is confusing to me because WCN6855 is pci17cb,1103, there are no other ids. See ath11k/pci.c: #define WCN6855_DEVICE_ID 0x1103 { PCI_VDEVICE(QCOM, WCN6855_DEVICE_ID) }, But this discussion is going circles and honestly is waste of time. I don't think the patch is right but I'll apply it anyway, let's deal with the problems if/when they come up. -- https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches