| Message ID | 1450848915-9772-1-git-send-email-rmani@qti.qualcomm.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Wed, Dec 23, 2015 at 11:05:15AM +0530, Raja Mani wrote: > Add device tree binding documentation details for wifi block present > in Qualcomm IPQ4019 SoC into qcom,ath10k.txt. > > Signed-off-by: Raja Mani <rmani@qti.qualcomm.com> > --- > .../bindings/net/wireless/qcom,ath10k.txt | 87 ++++++++++++++++++++-- > 1 file changed, 82 insertions(+), 5 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > index edefc26..ffd0742 100644 > --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > @@ -1,17 +1,42 @@ > * Qualcomm Atheros ath10k wireless devices > > -For ath10k devices the calibration data can be provided through Device > -Tree. The node is a child node of the PCI controller. So it is now not a PCI device? > - > Required properties: > --compatible : Should be "qcom,ath10k" > +- compatible: Should be one of the following: > + * "qcom,ath10k" > + * "qcom,ipq4019-wifi" One is a standalone PCI device and one is embedded block in an SOC? These should be more separated as all these new properties would only apply in the latter case. > > Optional properties: > +- reg: Address and length of the register set for the device. > +- core-id: Core number associated to the device. This needs a better explanation. > +- resets: Must contain an entry for each entry in reset-names. > + See ../reset/reseti.txt for details. > +- reset-names: Must include the list of following reset names, > + "wifi_cpu_init" > + "wifi_radio_srif" > + "wifi_radio_warm" > + "wifi_radio_cold" > + "wifi_core_warm" > + "wifi_core_cold" > +- clocks: List of clock specifiers, must contain an entry for each required > + entry in clock-names. > +- clock-names: Should contain the clock names "wifi_wcss_cmd", "wifi_wcss_ref", > + "wifi_wcss_rtc". > +- interrupts: List of interrupt lines. Must contain an entry > + for each entry in the interrupt-names property. > +- interrupt-names: Must include the entries for MSI interrupt > + names ("msi0" to "msi15") and legacy interrupt > + name ("legacy"), > +- qca,msi_addr: MSI interrupt address. > +- qca,msi_base: Base value to add before writing MSI data into > + MSI address register. Why don't the standard MSI properties work? > - qcom,ath10k-calibration-data : calibration data as an array, the > length can vary between hw versions > +- status: Either "disabled" or "ok". > + No need to document status here. Rob
On Wednesday 30 December 2015 10:05 PM, Rob Herring wrote: > On Wed, Dec 23, 2015 at 11:05:15AM +0530, Raja Mani wrote: >> Add device tree binding documentation details for wifi block present >> in Qualcomm IPQ4019 SoC into qcom,ath10k.txt. >> >> Signed-off-by: Raja Mani <rmani@qti.qualcomm.com> >> --- >> .../bindings/net/wireless/qcom,ath10k.txt | 87 ++++++++++++++++++++-- >> 1 file changed, 82 insertions(+), 5 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> index edefc26..ffd0742 100644 >> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> @@ -1,17 +1,42 @@ >> * Qualcomm Atheros ath10k wireless devices >> >> -For ath10k devices the calibration data can be provided through Device >> -Tree. The node is a child node of the PCI controller. > > So it is now not a PCI device? Right now, ath10k wireless driver has support for PCI devices. There is a plan to extend ath10k driver to support wifi devices which are connected over AHB as well (enumeration will happen via device tree node). Qualcomm IPQ4019 SoC has two inbuilt wifi block which are connected over AHB bus (not connected over PCI bus) and also has one external PCI slot where we can connect standalone wifi PCI devices. In future, ath10k would support both PCI and AHB. For PCI based devices, only calibration data is supplied via device tree. For AHB based devices (ie, ipq4019), all wifi properties are supplied via device tree (including irq, reg addr, cal data,etc). > >> - >> Required properties: >> --compatible : Should be "qcom,ath10k" >> +- compatible: Should be one of the following: >> + * "qcom,ath10k" >> + * "qcom,ipq4019-wifi" > > One is a standalone PCI device and one is embedded block in an SOC? Yes, it's possible to have this combination (one in SoC + one in standalone PCI device in the same platform). > These should be more separated as all these new properties would only > apply in the latter case. Sorry, i didn't get this point. I mentioned it under optional properties. Whichever properties applies to particular wifi, only those parameters are needed. For example, for PCI based devices, only calibration data is needed, for AHB based devices most the properties are needed. Is it fine if add below text some thing like this ? "only "qcom,ath10k-calibration-data" is applicable for PCI based devices, rest of the members are applicable only for AHB based devices" Correct me if i am wrong. > >> >> Optional properties: >> +- reg: Address and length of the register set for the device. >> +- core-id: Core number associated to the device. > > This needs a better explanation. Sure, Let me add below text in next version. "core-id is numeric number associated to the wifi block. For example, 0 means first block, 1 means second wifi block,etc." > >> +- resets: Must contain an entry for each entry in reset-names. >> + See ../reset/reseti.txt for details. >> +- reset-names: Must include the list of following reset names, >> + "wifi_cpu_init" >> + "wifi_radio_srif" >> + "wifi_radio_warm" >> + "wifi_radio_cold" >> + "wifi_core_warm" >> + "wifi_core_cold" >> +- clocks: List of clock specifiers, must contain an entry for each required >> + entry in clock-names. >> +- clock-names: Should contain the clock names "wifi_wcss_cmd", "wifi_wcss_ref", >> + "wifi_wcss_rtc". >> +- interrupts: List of interrupt lines. Must contain an entry >> + for each entry in the interrupt-names property. >> +- interrupt-names: Must include the entries for MSI interrupt >> + names ("msi0" to "msi15") and legacy interrupt >> + name ("legacy"), >> +- qca,msi_addr: MSI interrupt address. >> +- qca,msi_base: Base value to add before writing MSI data into >> + MSI address register. > > Why don't the standard MSI properties work? Standard msi controller mapping is not needed. Basically, msi property defined above just goes to the firmware runs in wifi block. Meaning, Driver will read it and push it to firmware. That's all. > >> - qcom,ath10k-calibration-data : calibration data as an array, the >> length can vary between hw versions >> +- status: Either "disabled" or "ok". >> + > > No need to document status here. Sure, will remove it in next version. > > Rob > Thanks for the review. -- Raja
On Wed, Dec 30, 2015 at 11:41 PM, Raja Mani <rmani@qti.qualcomm.com> wrote: > > On Wednesday 30 December 2015 10:05 PM, Rob Herring wrote: >> >> On Wed, Dec 23, 2015 at 11:05:15AM +0530, Raja Mani wrote: >>> >>> Add device tree binding documentation details for wifi block present >>> in Qualcomm IPQ4019 SoC into qcom,ath10k.txt. >>> >>> Signed-off-by: Raja Mani <rmani@qti.qualcomm.com> >>> --- >>> .../bindings/net/wireless/qcom,ath10k.txt | 87 >>> ++++++++++++++++++++-- >>> 1 file changed, 82 insertions(+), 5 deletions(-) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >>> b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >>> index edefc26..ffd0742 100644 >>> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >>> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >>> @@ -1,17 +1,42 @@ >>> * Qualcomm Atheros ath10k wireless devices >>> >>> -For ath10k devices the calibration data can be provided through Device >>> -Tree. The node is a child node of the PCI controller. >> >> >> So it is now not a PCI device? > > > Right now, ath10k wireless driver has support for PCI devices. There is > a plan to extend ath10k driver to support wifi devices which are connected > over AHB as well (enumeration will happen via device tree node). > > Qualcomm IPQ4019 SoC has two inbuilt wifi block which are connected over AHB > bus (not connected over PCI bus) and also has one external PCI > slot where we can connect standalone wifi PCI devices. > > In future, ath10k would support both PCI and AHB. For PCI based > devices, only calibration data is supplied via device tree. For AHB > based devices (ie, ipq4019), all wifi properties are supplied > via device tree (including irq, reg addr, cal data,etc). Put this information in the binding. It is not clear and you seem to be removing PCI information. >>> - >>> Required properties: >>> --compatible : Should be "qcom,ath10k" >>> +- compatible: Should be one of the following: >>> + * "qcom,ath10k" >>> + * "qcom,ipq4019-wifi" >> >> >> One is a standalone PCI device and one is embedded block in an SOC? > > > Yes, it's possible to have this combination (one in SoC + one in > standalone PCI device in the same platform). > >> These should be more separated as all these new properties would only >> apply in the latter case. > > > Sorry, i didn't get this point. I mentioned it under optional > properties. Whichever properties applies to particular wifi, only > those parameters are needed. For example, for PCI based devices, > only calibration data is needed, for AHB based devices most the > properties are needed. > > Is it fine if add below text some thing like this ? > > "only "qcom,ath10k-calibration-data" is applicable for PCI based > devices, rest of the members are applicable only for AHB based > devices" Yes, but you need to explain in terms of compatible strings. Which compatible strings correspond to PCI devices. >>> Optional properties: >>> +- reg: Address and length of the register set for the device. >>> +- core-id: Core number associated to the device. >> >> >> This needs a better explanation. > > > Sure, Let me add below text in next version. > > "core-id is numeric number associated to the wifi block. > For example, 0 means first block, 1 means second wifi block,etc." Are the blocks the same? If not, then use different compatible strings. Otherwise doesn't the reg property identify which block is which? Or some other property? Different freq bands? We generally don't allow indexes like this in the DT, so I need to understand how you use this. Rob
On Wednesday 13 January 2016 07:53 AM, Rob Herring wrote: > On Wed, Dec 30, 2015 at 11:41 PM, Raja Mani <rmani@qti.qualcomm.com> wrote: >> >> On Wednesday 30 December 2015 10:05 PM, Rob Herring wrote: >>> >>> On Wed, Dec 23, 2015 at 11:05:15AM +0530, Raja Mani wrote: >>>> >>>> Add device tree binding documentation details for wifi block present >>>> in Qualcomm IPQ4019 SoC into qcom,ath10k.txt. >>>> >>>> Signed-off-by: Raja Mani <rmani@qti.qualcomm.com> >>>> --- >>>> .../bindings/net/wireless/qcom,ath10k.txt | 87 >>>> ++++++++++++++++++++-- >>>> 1 file changed, 82 insertions(+), 5 deletions(-) >>>> >>>> diff --git >>>> a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >>>> b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >>>> index edefc26..ffd0742 100644 >>>> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >>>> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >>>> @@ -1,17 +1,42 @@ >>>> * Qualcomm Atheros ath10k wireless devices >>>> >>>> -For ath10k devices the calibration data can be provided through Device >>>> -Tree. The node is a child node of the PCI controller. >>> >>> >>> So it is now not a PCI device? >> >> >> Right now, ath10k wireless driver has support for PCI devices. There is >> a plan to extend ath10k driver to support wifi devices which are connected >> over AHB as well (enumeration will happen via device tree node). >> >> Qualcomm IPQ4019 SoC has two inbuilt wifi block which are connected over AHB >> bus (not connected over PCI bus) and also has one external PCI >> slot where we can connect standalone wifi PCI devices. >> >> In future, ath10k would support both PCI and AHB. For PCI based >> devices, only calibration data is supplied via device tree. For AHB >> based devices (ie, ipq4019), all wifi properties are supplied >> via device tree (including irq, reg addr, cal data,etc). > > Put this information in the binding. It is not clear and you seem to > be removing PCI information. Okay, I'll fix it in next version. > >>>> - >>>> Required properties: >>>> --compatible : Should be "qcom,ath10k" >>>> +- compatible: Should be one of the following: >>>> + * "qcom,ath10k" >>>> + * "qcom,ipq4019-wifi" >>> >>> >>> One is a standalone PCI device and one is embedded block in an SOC? >> >> >> Yes, it's possible to have this combination (one in SoC + one in >> standalone PCI device in the same platform). >> >>> These should be more separated as all these new properties would only >>> apply in the latter case. >> >> >> Sorry, i didn't get this point. I mentioned it under optional >> properties. Whichever properties applies to particular wifi, only >> those parameters are needed. For example, for PCI based devices, >> only calibration data is needed, for AHB based devices most the >> properties are needed. >> >> Is it fine if add below text some thing like this ? >> >> "only "qcom,ath10k-calibration-data" is applicable for PCI based >> devices, rest of the members are applicable only for AHB based >> devices" > > Yes, but you need to explain in terms of compatible strings. Which > compatible strings correspond to PCI devices. Sure, i'll add this detail. > >>>> Optional properties: >>>> +- reg: Address and length of the register set for the device. >>>> +- core-id: Core number associated to the device. >>> >>> >>> This needs a better explanation. >> >> >> Sure, Let me add below text in next version. >> >> "core-id is numeric number associated to the wifi block. >> For example, 0 means first block, 1 means second wifi block,etc." > > Are the blocks the same? If not, then use different compatible > strings. Otherwise doesn't the reg property identify which block is > which? Or some other property? Different freq bands? We generally > don't allow indexes like this in the DT, so I need to understand how > you use this. > > Rob > I understand the confusion and the limitation, i'll remove core-id in next version and manage this in driver itself. Could you please consider v3 of this patch for further review? I hope, i addressed all your review comments. -- Raja
diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt index edefc26..ffd0742 100644 --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt @@ -1,17 +1,42 @@ * Qualcomm Atheros ath10k wireless devices -For ath10k devices the calibration data can be provided through Device -Tree. The node is a child node of the PCI controller. - Required properties: --compatible : Should be "qcom,ath10k" +- compatible: Should be one of the following: + * "qcom,ath10k" + * "qcom,ipq4019-wifi" Optional properties: +- reg: Address and length of the register set for the device. +- core-id: Core number associated to the device. +- resets: Must contain an entry for each entry in reset-names. + See ../reset/reseti.txt for details. +- reset-names: Must include the list of following reset names, + "wifi_cpu_init" + "wifi_radio_srif" + "wifi_radio_warm" + "wifi_radio_cold" + "wifi_core_warm" + "wifi_core_cold" +- clocks: List of clock specifiers, must contain an entry for each required + entry in clock-names. +- clock-names: Should contain the clock names "wifi_wcss_cmd", "wifi_wcss_ref", + "wifi_wcss_rtc". +- interrupts: List of interrupt lines. Must contain an entry + for each entry in the interrupt-names property. +- interrupt-names: Must include the entries for MSI interrupt + names ("msi0" to "msi15") and legacy interrupt + name ("legacy"), +- qca,msi_addr: MSI interrupt address. +- qca,msi_base: Base value to add before writing MSI data into + MSI address register. - qcom,ath10k-calibration-data : calibration data as an array, the length can vary between hw versions +- status: Either "disabled" or "ok". + +Example (to supply the calibration data alone): -Example: +In this example, the node is defined as child node of the PCI controller. pci { pcie@0 { @@ -28,3 +53,55 @@ pci { }; }; }; + +Example (to supply ipq4019 SoC wifi block details): + +wifi0: wifi@a000000 { + compatible = "qcom,ipq4019-wifi"; + reg = <0xa000000 0x200000>; + core-id = <0x0>; + resets = <&gcc WIFI0_CPU_INIT_RESET>, + <&gcc WIFI0_RADIO_SRIF_RESET>, + <&gcc WIFI0_RADIO_WARM_RESET>, + <&gcc WIFI0_RADIO_COLD_RESET>, + <&gcc WIFI0_CORE_WARM_RESET>, + <&gcc WIFI0_CORE_COLD_RESET>; + reset-names = "wifi_cpu_init", + "wifi_radio_srif", + "wifi_radio_warm", + "wifi_radio_cold", + "wifi_core_warm", + "wifi_core_cold"; + clocks = <&gcc GCC_WCSS2G_CLK>, + <&gcc GCC_WCSS2G_REF_CLK>, + <&gcc GCC_WCSS2G_RTC_CLK>; + clock-names = "wifi_wcss_cmd", + "wifi_wcss_ref", + "wifi_wcss_rtc"; + interrupts = <0 0x20 0x1>, + <0 0x21 0x1>, + <0 0x22 0x1>, + <0 0x23 0x1>, + <0 0x24 0x1>, + <0 0x25 0x1>, + <0 0x26 0x1>, + <0 0x27 0x1>, + <0 0x28 0x1>, + <0 0x29 0x1>, + <0 0x2a 0x1>, + <0 0x2b 0x1>, + <0 0x2c 0x1>, + <0 0x2d 0x1>, + <0 0x2e 0x1>, + <0 0x2f 0x1>, + <0 0xa8 0x0>; + interrupt-names = "msi0", "msi1", "msi2", "msi3", + "msi4", "msi5", "msi6", "msi7", + "msi8", "msi9", "msi10", "msi11", + "msi12", "msi13", "msi14", "msi15", + "legacy"; + status = "ok"; + qca,msi_addr = <0x0b006040>; + qca,msi_base = <0x40>; + qcom,ath10k-calibration-data = [ 01 02 03 ... ]; +};
Add device tree binding documentation details for wifi block present in Qualcomm IPQ4019 SoC into qcom,ath10k.txt. Signed-off-by: Raja Mani <rmani@qti.qualcomm.com> --- .../bindings/net/wireless/qcom,ath10k.txt | 87 ++++++++++++++++++++-- 1 file changed, 82 insertions(+), 5 deletions(-)