On 23/10/2024 12:28, Raj Kumar Bhagat wrote: > On 10/23/2024 12:29 PM, Krzysztof Kozlowski wrote: >> On 23/10/2024 08:53, Raj Kumar Bhagat wrote: >>> On 10/23/2024 12:17 PM, Krzysztof Kozlowski wrote: >>>> On 23/10/2024 08:45, Raj Kumar Bhagat wrote: >>>>> On 10/23/2024 12:05 PM, Krzysztof Kozlowski wrote: >>>>>> On 23/10/2024 08:03, Raj Kumar Bhagat wrote: >>>>>>> The current device-tree bindings for the Ath12K module list many >>>>>>> WCN7850-specific properties as required. However, these properties are >>>>>>> not applicable to other Ath12K devices. >>>>>>> >>>>>>> Hence, remove WCN7850-specific properties from the required section, >>>>>>> retaining only generic properties valid across all Ath12K devices. >>>>>>> WCN7850-specific properties will remain required based on the device's >>>>>>> compatible enum. >>>>>> Just not true. These apply to all devices described in this binding. >>>>>> >>>>>> NAK. >>>>>> >>>>>> Don't send patches for your downstream stuff. >>>>> This is not for downstream. This series is the per-requisite for ath12k >>>>> MLO support in upstream. >>>>> >>>>> In the subsequent patch [2/6] we are adding new device (QCN9274) in this >>>>> binding that do not require the WCN7850 specific properties. >>>>> >>>>> This is a refactoring patch for the next patch [2/6]. >>>> It's just wrong. Not true. At this point of patch there are no other >>>> devices. Don't refactor uselessly introducing incorrect hardware >>> Ok then, If we squash this patch with the next patch [2/6], that actually adding >>> the new device, then this patch changes are valid right? >> Yes, except I asked to have separate binding for devices with different >> interface (WSI). You add unrelated devices to same binding, growing it >> into something tricky to manage. Your second patch misses if:then >> disallwing all this WSI stuff for existing device... and then you should >> notice there is absolutely *nothing* in common. >> > > I understand your point about having separate bindings if there are no common > properties. However, the title and description of this binding indicate that it > is intended for Qualcomm ath12k wireless devices with a PCI bus. Given this, the > QCN9274 seems to fit within the same binding. Feel free to fix it. Or add common schema used by multiple bindings. > > Additionally, there will likely be more properties added in the future that could > be common. For example, the “qcom,ath12k-calibration-variant” property (which the You are supposed to add them now, not later. See writing bindings. They are supposed to be complete. > ath12k host currently doesn’t support reading and using, hence we are not adding it > now) could be a common property. What is "host"? Either the device has this property or not. Whether host supports something does not really matter, right? You have hardware property or you have it *not*. > > If you still recommend creating a separate binding for the QCN9274, we are open to > working on that. Best regards, Krzysztof
