From: Johan Hovold <johan@kernel.org> To: Kalle Valo <kvalo@kernel.org> Cc: Johan Hovold <johan+linaro@kernel.org>, Bjorn Andersson <andersson@kernel.org>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Andy Gross <agross@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, ath11k@lists.infradead.org Subject: Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings Date: Tue, 21 Mar 2023 09:22:04 +0100 [thread overview] Message-ID: <ZBlpLJfqB1Q7JfQ+@hovoldconsulting.com> (raw) In-Reply-To: <87a607fepa.fsf@kernel.org> On Mon, Mar 20, 2023 at 08:41:21PM +0200, Kalle Valo wrote: > + ath11k list > > Johan Hovold <johan@kernel.org> writes: > > > On Mon, Mar 20, 2023 at 02:22:12PM +0200, Kalle Valo wrote: > >> Johan Hovold <johan+linaro@kernel.org> writes: > >> > >> > Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856 > >> > for which the calibration data variant may need to be described. > >> > > >> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > >> > --- > >> > .../bindings/net/wireless/pci17cb,1103.yaml | 56 +++++++++++++++++++ > >> > 1 file changed, 56 insertions(+) > >> > create mode 100644 > >> > Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml > >> > >> I'm confused (as usual), how does this differ from > >> bindings/net/wireless/qcom,ath11k.yaml? Why we need two .yaml files? > > > > Almost none of bindings/net/wireless/qcom,ath11k.yaml applies to WCN6856 > > when using PCIe (e.g. as most properties are then discoverable). > > > > We could try to encode everything in one file, but that would likely > > just result in a big mess of a schema with conditionals all over. > > Ah, so the current qcom,ath11k.yaml would be only for ath11k AHB devices > and this new file is only for ath11k PCI devices? Right, there would two separate schema files for the two device classes. > But why still the odd > name pci17cb,1103.yaml? It's not really descriptive and I'm for sure > will not remember that pci17cb,1103.yaml is for ath11k :) Yeah, it's not the best name from that perspective, but it follows the current convention of naming the schema files after the first compatible added. That said, we don't have many schemas for PCI devices so perhaps we can establish a new convention for those. Perhaps by replacing the numerical ids with what we'd use if these were platform devices (e.g. 'qcom,wcn6855.yaml'). As long as the DT maintainers are OK with it, I'd also be happy with something like you suggest below: qcom,ath11k-ahb.yaml qcom,ath11k-pci.yaml (or simply not renaming the current file 'qcom,ath11k.yaml') but I have gotten push back on that in the past. > Also it doesn't look good that we have qcom,ath11k-calibration-variant > documented twice now. I'm no DT expert but isn't there any other way? Is > it possible to include other files? For example, if we would have three > files: > > qcom,ath11k.yaml > qcom,ath11k-ahb.yaml > qcom,ath11k-pci.yaml > > Then have the common properties like ath11k-calibration-variant in the > first file and ahb/pci files would include that. That should be possible, but it's not necessarily better as you'd then have to look up two files to see the bindings for either device class (and as far as I can tell there would not be much sharing beyond this single property). Note that the property could just have well have been named 'qcom,calibration-variant' and then it would be shared also with the ath10k set of devices which currently holds another definition of what is essentially the same property ('qcom,ath10k-calibration-variant'). Johan
WARNING: multiple messages have this Message-ID (diff)
From: Johan Hovold <johan@kernel.org> To: Kalle Valo <kvalo@kernel.org> Cc: Johan Hovold <johan+linaro@kernel.org>, Bjorn Andersson <andersson@kernel.org>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Andy Gross <agross@kernel.org>, Konrad Dybcio <konrad.dybcio@linaro.org>, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, devicetree@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, ath11k@lists.infradead.org Subject: Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings Date: Tue, 21 Mar 2023 09:22:04 +0100 [thread overview] Message-ID: <ZBlpLJfqB1Q7JfQ+@hovoldconsulting.com> (raw) In-Reply-To: <87a607fepa.fsf@kernel.org> On Mon, Mar 20, 2023 at 08:41:21PM +0200, Kalle Valo wrote: > + ath11k list > > Johan Hovold <johan@kernel.org> writes: > > > On Mon, Mar 20, 2023 at 02:22:12PM +0200, Kalle Valo wrote: > >> Johan Hovold <johan+linaro@kernel.org> writes: > >> > >> > Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856 > >> > for which the calibration data variant may need to be described. > >> > > >> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org> > >> > --- > >> > .../bindings/net/wireless/pci17cb,1103.yaml | 56 +++++++++++++++++++ > >> > 1 file changed, 56 insertions(+) > >> > create mode 100644 > >> > Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml > >> > >> I'm confused (as usual), how does this differ from > >> bindings/net/wireless/qcom,ath11k.yaml? Why we need two .yaml files? > > > > Almost none of bindings/net/wireless/qcom,ath11k.yaml applies to WCN6856 > > when using PCIe (e.g. as most properties are then discoverable). > > > > We could try to encode everything in one file, but that would likely > > just result in a big mess of a schema with conditionals all over. > > Ah, so the current qcom,ath11k.yaml would be only for ath11k AHB devices > and this new file is only for ath11k PCI devices? Right, there would two separate schema files for the two device classes. > But why still the odd > name pci17cb,1103.yaml? It's not really descriptive and I'm for sure > will not remember that pci17cb,1103.yaml is for ath11k :) Yeah, it's not the best name from that perspective, but it follows the current convention of naming the schema files after the first compatible added. That said, we don't have many schemas for PCI devices so perhaps we can establish a new convention for those. Perhaps by replacing the numerical ids with what we'd use if these were platform devices (e.g. 'qcom,wcn6855.yaml'). As long as the DT maintainers are OK with it, I'd also be happy with something like you suggest below: qcom,ath11k-ahb.yaml qcom,ath11k-pci.yaml (or simply not renaming the current file 'qcom,ath11k.yaml') but I have gotten push back on that in the past. > Also it doesn't look good that we have qcom,ath11k-calibration-variant > documented twice now. I'm no DT expert but isn't there any other way? Is > it possible to include other files? For example, if we would have three > files: > > qcom,ath11k.yaml > qcom,ath11k-ahb.yaml > qcom,ath11k-pci.yaml > > Then have the common properties like ath11k-calibration-variant in the > first file and ahb/pci files would include that. That should be possible, but it's not necessarily better as you'd then have to look up two files to see the bindings for either device class (and as far as I can tell there would not be much sharing beyond this single property). Note that the property could just have well have been named 'qcom,calibration-variant' and then it would be shared also with the ath10k set of devices which currently holds another definition of what is essentially the same property ('qcom,ath10k-calibration-variant'). Johan -- ath11k mailing list ath11k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath11k
next prev parent reply other threads:[~2023-03-21 8:20 UTC|newest] Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-03-20 10:46 [PATCH 0/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold 2023-03-20 10:46 ` [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings Johan Hovold 2023-03-20 12:22 ` Kalle Valo 2023-03-20 12:42 ` Johan Hovold 2023-03-20 18:41 ` Kalle Valo 2023-03-20 18:41 ` Kalle Valo 2023-03-21 8:22 ` Johan Hovold [this message] 2023-03-21 8:22 ` Johan Hovold 2023-03-22 5:59 ` Kalle Valo 2023-03-22 5:59 ` Kalle Valo 2023-03-21 8:14 ` Krzysztof Kozlowski 2023-03-21 8:16 ` Krzysztof Kozlowski 2023-03-21 8:27 ` Johan Hovold 2023-03-21 8:29 ` Krzysztof Kozlowski 2023-03-20 10:46 ` [PATCH 2/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold 2023-03-20 10:59 ` Konrad Dybcio 2023-03-20 12:15 ` Steev Klimaszewski 2023-03-20 10:46 ` [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: " Johan Hovold 2023-03-20 10:50 ` Konrad Dybcio 2023-03-20 10:55 ` Johan Hovold 2023-03-20 10:59 ` Konrad Dybcio 2023-03-20 11:07 ` Johan Hovold 2023-03-20 12:18 ` Kalle Valo 2023-03-20 12:34 ` Johan Hovold
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=ZBlpLJfqB1Q7JfQ+@hovoldconsulting.com \ --to=johan@kernel.org \ --cc=agross@kernel.org \ --cc=andersson@kernel.org \ --cc=ath11k@lists.infradead.org \ --cc=davem@davemloft.net \ --cc=devicetree@vger.kernel.org \ --cc=edumazet@google.com \ --cc=johan+linaro@kernel.org \ --cc=konrad.dybcio@linaro.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=kuba@kernel.org \ --cc=kvalo@kernel.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-wireless@vger.kernel.org \ --cc=netdev@vger.kernel.org \ --cc=pabeni@redhat.com \ --cc=robh+dt@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.