Re: [PATCH v2 1/6] dt-bindings: bluetooth: qca: add QCC2072
Krzysztof Kozlowski <[email protected]>
| Newsgroups | org.kernel.vger.linux-bluetooth,org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci,org.kernel.vger.linux-pm |
|---|---|
| Message-ID | <[email protected]> |
On 07/08/2026 11:57, Dmitry Baryshkov wrote: > On Sat, Aug 01, 2026 at 05:49:53PM +0200, Krzysztof Kozlowski wrote: >> On 01/08/2026 17:31, Rahul Samana wrote: >>> >>> >>> On 01-08-2026 20:18, Krzysztof Kozlowski wrote: >>>> On 31/07/2026 17:51, Rahul Samana wrote: >>>>> >>>>> >>>>> On 31-07-2026 20:34, Krzysztof Kozlowski wrote: >>>>>> On 31/07/2026 16:45, Rahul Samana wrote: >>>>>>> >>>>>>> >>>>>>> On 31-07-2026 15:16, Krzysztof Kozlowski wrote: >>>>>>>> On Mon, Jul 27, 2026 at 09:15:01PM +0530, Rahul Samana wrote: >>>>>>>>> QCC2072 can be used on M.2 E-key cards where the card power resources are >>>>>>>>> described by the pcie-m2-e-connector node. In that setup, the M.2 power >>>>>>>>> sequencing provider creates the Bluetooth serdev child after matching the >>>>>>>>> QCC2072 PCI function. >>>>>>>>> >>>>>>>>> Integrated non-M.2 designs need board-specific power resources. Document >>>>>>>>> only the compatible for now and leave those properties to be added with >>>>>>>>> matching driver support. >>>>>>>>> >>>>>>>>> Document the qcom,qcc2072-bt compatible used for QCC2072 Bluetooth >>>>>>>>> controllers connected over UART. >>>>>>>>> >>>>>>>>> Signed-off-by: Rahul Samana <[email protected]> >>>>>>>> >>>>>>>> NAK, exactly same comments as before. >>>>>>>> >>>>>>>> Nothing got improved, although what is weird - original SoB is gone, so >>>>>>>> this is legally dubious work. >>>>>>>> >>>>>>>> Best regards, >>>>>>>> Krzysztof >>>>>>>> >>>>>>> >>>>>>> Hi Krzysztof, >>>>>>> >>>>>>> Thanks for the review. >>>>>>> >>>>>>> For the binding contents, I tried to capture the current scope in the binding >>>>>>> description itself. This series supports QCC2072 only as an M.2 E-key card, >>>>>>> where the card power resources are described by the pcie-m2-e-connector node >>>>>>> and the M.2 pwrseq provider creates the Bluetooth serdev child. >>>>>>> >>>>>>> The binding also says: >>>>>>> >>>>>>> Integrated non-M.2 designs require board-specific power resources. Those >>>>>>> properties, together with a static devicetree example, should be added when >>>>>>> integrated non-M.2 support is added. >>>>>> >>>>>> Bindings must be complete and your driver support is irrelevant here. >>>>>> >>>>>> If you claim this is a PCI device thus you do not need any resources, >>>>>> then the binding is not needed either. PCI devices are enumerable. And >>>>>> to prove it: look at your DTS. Do you see qcom,qcc2072-bt being used? No. >>>>>> >>>>>>> >>>>>>> We do not currently have an integrated non-M.2 QCC2072 design, so I do not >>>>>>> have board-specific regulator supplies to document for that topology. >>>>>>> >>>>>>> Could you please suggest how you would prefer this binding to be handled for >>>>>>> the current M.2-only use case? >>>>>> >>>>>> Drop the binding, you do not need it. >>>>>> >>>>>> Anyway the problem is that more comments were ignored. >>>>>> >>>>> >>>>> Hi Krzysztof, >>>>> >>>>> Just to clarify the reason for adding this binding in v2: >>>>> v1 did not add a binding because this series only targets the M.2 use case. >>>>> >>>>> I added the minimal binding in v2 because checkpatch reported >>>>> qcom,qcc2072-bt as an undocumented compatible, and I interpreted the request >>>>> to fix the checkpatch warnings as requiring this compatible to be documented. >>>>> I also had the earlier feedback in mind, where the indirect >>>>> qcom,qcc2072-bt compatible was pushed back because it was undocumented: >>>>> >>>>> https://lore.kernel.org/all/[email protected]/ >>>>> >>>>> Based on your clarification here, I will drop the binding patch in v3 and >>>>> keep qcom,qcc2072-bt only as the pwrseq-created child compatible for this >>>>> M.2 case. >>>> >>>> My previous statement is also valid, please read entire threads. >>>> >>>> You cannot have undocumented qcom,qcc2072-bt. >>>> >>>> I ask you to drop both, because they are not needed. But feel free to >>>> prove me wrong, see my first paragraph in the previous reply. >>>> >>>> >>> The PCIe M.2 power sequencing driver, pwrseq-pcie-m2.c, uses >>> pwrseq_m2_pci_ids to translate the enumerated PCI function into the >>> Bluetooth compatible used for the generated serdev child. >>> >>> For example, WCN7850 maps PCI ID 17cb:1107 to qcom,wcn7850-bt, then hci_qca >>> matches that compatible to select qca_soc_data_wcn7850. >>> >>> For QCC2072, pwrseq-pcie-m2.c matches PCI ID 17cb:1112 and creates the >>> generated Bluetooth serdev child with compatible "qcom,qcc2072-bt". The >>> hci_qca driver then matches "qcom,qcc2072-bt" and uses qca_soc_data_qcc2072 >>> as the controller-specific data. >> >> The purpose of Devicetree is not to describe Linux internal driver >> matching. Do not use compatibles for that. > > So, if instead of M.2 module the WiFi chip gets soldered to the main > board, the qcom,qcc2072-bt wll be a legitimate compat string? Yes, just like the rest of compatibles in the power sequencing driver (and bindings). > > Also, I understand your concerns about the DT bindings / usage, but > speaking about the driver ;-) > The BT is a serial driver attached to a serial bus. There only two > options to bind a serial driver: OF and ACPI. But maybe we should add > something like serial_device_id and let serials have internal IDs. > It sounded strange in the beginning, but now I think it is a correct way > to go. Driver get away with that limitation, because we had DTS for the existing entries. This is the first one without DTS. Best regards, Krzysztof