| Message ID | 1538653364-1239-2-git-send-email-bperumal@codeaurora.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath10k: Add support to configure BB timing for external FEM | expand |
On Thu, Oct 04, 2018 at 05:12:43PM +0530, Bhagavathi Perumal S wrote: > This adds new dt entry ext-fem-name, it is used by ath10k driver > to select correct timing parameters and configure it in target wifi hardware. > The Front End Module(FEM) normally includes tx power amplifier(PA) and > rx low noise amplifier(LNA). The default timing parameters like tx end to > PA off timing values were fine tuned for internal FEM used in reference > design. And these timing values can not be same if ODM modifies hardware > design with different external FEM. This DT entry helps to choose correct > timing values in driver if different external FEM hardware used. 'dt-bindings: net: ath10k: ...' for the subject please. > > Signed-off-by: Bhagavathi Perumal S <bperumal@codeaurora.org> > --- > .../bindings/net/wireless/qcom,ath10k.txt | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > index 7fd4e8c..fbaf309 100644 > --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > @@ -56,6 +56,7 @@ Optional properties: > the length can vary between hw versions. > - <supply-name>-supply: handle to the regulator device tree node > optional "supply-name" is "vdd-0.8-cx-mx". > +- ext-fem-name: name of external front end module used. What are valid names? What if the OS doesn't recognize the name? Perhaps this should be a compatible string for the particular module instead? Then it could cover any differences, not just the FEM. > > Example (to supply the calibration data alone): > > @@ -150,3 +151,24 @@ wifi@18000000 { > <0 141 0 /* CE11 */ >; > vdd-0.8-cx-mx-supply = <&pm8998_l5>; > }; > + > +Example (to supply the external front end module name): > + > +In this example, the front end module is defined as a property of the ath10k > +device node. Really need a whole new example for 1 property? > + > +pci { > + pcie@0 { > + reg = <0 0 0 0 0>; > + #interrupt-cells = <1>; > + #size-cells = <2>; > + #address-cells = <3>; > + device_type = "pci"; > + > + ath10k@0,0 { wifi@0,0 > + reg = <0 0 0 0 0>; > + device_type = "pci"; > + ext-fem-name = "microsemi-lx5586"; > + }; > + }; > +}; > -- > 1.9.1 >
On 2018-10-16 00:54, Rob Herring wrote: > On Thu, Oct 04, 2018 at 05:12:43PM +0530, Bhagavathi Perumal S wrote: >> This adds new dt entry ext-fem-name, it is used by ath10k driver >> to select correct timing parameters and configure it in target wifi >> hardware. >> The Front End Module(FEM) normally includes tx power amplifier(PA) and >> rx low noise amplifier(LNA). The default timing parameters like tx end >> to >> PA off timing values were fine tuned for internal FEM used in >> reference >> design. And these timing values can not be same if ODM modifies >> hardware >> design with different external FEM. This DT entry helps to choose >> correct >> timing values in driver if different external FEM hardware used. > > 'dt-bindings: net: ath10k: ...' for the subject please. Sure, I will change and send v2 patch. > >> >> Signed-off-by: Bhagavathi Perumal S <bperumal@codeaurora.org> >> --- >> .../bindings/net/wireless/qcom,ath10k.txt | 22 >> ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> >> diff --git >> a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> index 7fd4e8c..fbaf309 100644 >> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> @@ -56,6 +56,7 @@ Optional properties: >> the length can vary between hw versions. >> - <supply-name>-supply: handle to the regulator device tree node >> optional "supply-name" is "vdd-0.8-cx-mx". >> +- ext-fem-name: name of external front end module used. > > What are valid names? What if the OS doesn't recognize the name? > Perhaps Some valid FEM devices are "microsemi-lx5586", "sky85703-11" and "sky85803" etc. Currently driver accepts only "microsemi-lx5586", Since this FEM requires different timing settings, And it can be extended for other FEM devices in future. If OS doesn't recognize the name, it uses predefined default timing settings. > this should be a compatible string for the particular module instead? > Then it could cover any differences, not just the FEM. > No, it is specific to FEM device. >> >> Example (to supply the calibration data alone): >> >> @@ -150,3 +151,24 @@ wifi@18000000 { >> <0 141 0 /* CE11 */ >; >> vdd-0.8-cx-mx-supply = <&pm8998_l5>; >> }; >> + >> +Example (to supply the external front end module name): >> + >> +In this example, the front end module is defined as a property of the >> ath10k >> +device node. > > Really need a whole new example for 1 property? Will add this property in existing example. > >> + >> +pci { >> + pcie@0 { >> + reg = <0 0 0 0 0>; >> + #interrupt-cells = <1>; >> + #size-cells = <2>; >> + #address-cells = <3>; >> + device_type = "pci"; >> + >> + ath10k@0,0 { > > wifi@0,0 Added in ath10k@0,0 to make consistent with existing property "qcom,ath10k-calibration-data" below, " pci { pcie@0 { reg = <0 0 0 0 0>; #interrupt-cells = <1>; #size-cells = <2>; #address-cells = <3>; device_type = "pci"; ath10k@0,0 { reg = <0 0 0 0 0>; device_type = "pci"; qcom,ath10k-calibration-data = [ 01 02 03 ... ]; }; }; }; " If wifi@0,0 is preferable, then I will add new entry "wifi@0,0" and add ext-fem-name into it. > >> + reg = <0 0 0 0 0>; >> + device_type = "pci"; >> + ext-fem-name = "microsemi-lx5586"; >> + }; >> + }; >> +}; >> -- >> 1.9.1 >> Thanks, Sorry for the delayed response, Bhagavathi Perumal S.
On Tue, Oct 30, 2018 at 3:41 AM <bperumal@codeaurora.org> wrote: > > On 2018-10-16 00:54, Rob Herring wrote: > > On Thu, Oct 04, 2018 at 05:12:43PM +0530, Bhagavathi Perumal S wrote: > >> This adds new dt entry ext-fem-name, it is used by ath10k driver > >> to select correct timing parameters and configure it in target wifi > >> hardware. > >> The Front End Module(FEM) normally includes tx power amplifier(PA) and > >> rx low noise amplifier(LNA). The default timing parameters like tx end > >> to > >> PA off timing values were fine tuned for internal FEM used in > >> reference > >> design. And these timing values can not be same if ODM modifies > >> hardware > >> design with different external FEM. This DT entry helps to choose > >> correct > >> timing values in driver if different external FEM hardware used. > > > > 'dt-bindings: net: ath10k: ...' for the subject please. > Sure, I will change and send v2 patch. > > > > >> > >> Signed-off-by: Bhagavathi Perumal S <bperumal@codeaurora.org> > >> --- > >> .../bindings/net/wireless/qcom,ath10k.txt | 22 > >> ++++++++++++++++++++++ > >> 1 file changed, 22 insertions(+) > >> > >> diff --git > >> a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > >> b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > >> index 7fd4e8c..fbaf309 100644 > >> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > >> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt > >> @@ -56,6 +56,7 @@ Optional properties: > >> the length can vary between hw versions. > >> - <supply-name>-supply: handle to the regulator device tree node > >> optional "supply-name" is "vdd-0.8-cx-mx". > >> +- ext-fem-name: name of external front end module used. > > > > What are valid names? What if the OS doesn't recognize the name? > > Perhaps > Some valid FEM devices are "microsemi-lx5586", "sky85703-11" and > "sky85803" etc. > Currently driver accepts only "microsemi-lx5586", Since this FEM > requires different timing settings, Assuming you keep this, then you need to enumerate what are valid values in the binding. Otherwise, 'ext-fem-name = "rob"' is valid. > And it can be extended for other FEM devices in future. > If OS doesn't recognize the name, it uses predefined default timing > settings. > > > this should be a compatible string for the particular module instead? > > Then it could cover any differences, not just the FEM. > > > No, it is specific to FEM device. You will be better off having either a specific compatible string or using VID/PID as this is a PCI device. Then you can handle other differences you may find between h/w versions without a DT change. > >> > >> Example (to supply the calibration data alone): > >> > >> @@ -150,3 +151,24 @@ wifi@18000000 { > >> <0 141 0 /* CE11 */ >; > >> vdd-0.8-cx-mx-supply = <&pm8998_l5>; > >> }; > >> + > >> +Example (to supply the external front end module name): > >> + > >> +In this example, the front end module is defined as a property of the > >> ath10k > >> +device node. > > > > Really need a whole new example for 1 property? > Will add this property in existing example. > > > > >> + > >> +pci { > >> + pcie@0 { > >> + reg = <0 0 0 0 0>; > >> + #interrupt-cells = <1>; > >> + #size-cells = <2>; > >> + #address-cells = <3>; > >> + device_type = "pci"; > >> + > >> + ath10k@0,0 { > > > > wifi@0,0 > Added in ath10k@0,0 to make consistent with existing property > "qcom,ath10k-calibration-data" below, > " I don't see how that matters. > pci { > pcie@0 { > reg = <0 0 0 0 0>; > #interrupt-cells = <1>; > #size-cells = <2>; > #address-cells = <3>; > device_type = "pci"; > > ath10k@0,0 { > reg = <0 0 0 0 0>; > device_type = "pci"; > qcom,ath10k-calibration-data = [ 01 02 03 ... ]; > }; > }; > }; > " > > If wifi@0,0 is preferable, then I will add new entry "wifi@0,0" and add > ext-fem-name into it. Node names are supposed to reflect the class of device, not the model. See the DT spec for a list. > > > > >> + reg = <0 0 0 0 0>; > >> + device_type = "pci"; Also, this is wrong. Only PCI bridges should have this property. dtc will give you a warning on this. > >> + ext-fem-name = "microsemi-lx5586"; > >> + }; > >> + }; > >> +}; > >> -- > >> 1.9.1 > >> > > Thanks, Sorry for the delayed response, > Bhagavathi Perumal S.
On 2018-10-30 21:02, Rob Herring wrote: > On Tue, Oct 30, 2018 at 3:41 AM <bperumal@codeaurora.org> wrote: >> >> On 2018-10-16 00:54, Rob Herring wrote: >> > On Thu, Oct 04, 2018 at 05:12:43PM +0530, Bhagavathi Perumal S wrote: >> >> This adds new dt entry ext-fem-name, it is used by ath10k driver >> >> to select correct timing parameters and configure it in target wifi >> >> hardware. >> >> The Front End Module(FEM) normally includes tx power amplifier(PA) and >> >> rx low noise amplifier(LNA). The default timing parameters like tx end >> >> to >> >> PA off timing values were fine tuned for internal FEM used in >> >> reference >> >> design. And these timing values can not be same if ODM modifies >> >> hardware >> >> design with different external FEM. This DT entry helps to choose >> >> correct >> >> timing values in driver if different external FEM hardware used. >> > >> > 'dt-bindings: net: ath10k: ...' for the subject please. >> Sure, I will change and send v2 patch. >> >> > >> >> >> >> Signed-off-by: Bhagavathi Perumal S <bperumal@codeaurora.org> >> >> --- >> >> .../bindings/net/wireless/qcom,ath10k.txt | 22 >> >> ++++++++++++++++++++++ >> >> 1 file changed, 22 insertions(+) >> >> >> >> diff --git >> >> a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> >> b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> >> index 7fd4e8c..fbaf309 100644 >> >> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> >> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt >> >> @@ -56,6 +56,7 @@ Optional properties: >> >> the length can vary between hw versions. >> >> - <supply-name>-supply: handle to the regulator device tree node >> >> optional "supply-name" is "vdd-0.8-cx-mx". >> >> +- ext-fem-name: name of external front end module used. >> > >> > What are valid names? What if the OS doesn't recognize the name? >> > Perhaps >> Some valid FEM devices are "microsemi-lx5586", "sky85703-11" and >> "sky85803" etc. >> Currently driver accepts only "microsemi-lx5586", Since this FEM >> requires different timing settings, > > Assuming you keep this, then you need to enumerate what are valid > values in the binding. Otherwise, 'ext-fem-name = "rob"' is valid. Sure, Will enumerate valid values in v2 patch. > >> And it can be extended for other FEM devices in future. >> If OS doesn't recognize the name, it uses predefined default timing >> settings. >> >> > this should be a compatible string for the particular module instead? >> > Then it could cover any differences, not just the FEM. >> > >> No, it is specific to FEM device. > > You will be better off having either a specific compatible string or > using VID/PID as this is a PCI device. Then you can handle other > differences you may find between h/w versions without a DT change. > FEM is a part of wifi device, and not a separate PCI device. Different external FEM can be used in a same wifi device, so it might not be feasible to define a compatible string to cover any other differences. IMO we can go with the property name instead of defining compatible string specific to every FEM device. >> >> >> >> Example (to supply the calibration data alone): >> >> >> >> @@ -150,3 +151,24 @@ wifi@18000000 { >> >> <0 141 0 /* CE11 */ >; >> >> vdd-0.8-cx-mx-supply = <&pm8998_l5>; >> >> }; >> >> + >> >> +Example (to supply the external front end module name): >> >> + >> >> +In this example, the front end module is defined as a property of the >> >> ath10k >> >> +device node. >> > >> > Really need a whole new example for 1 property? >> Will add this property in existing example. >> >> > >> >> + >> >> +pci { >> >> + pcie@0 { >> >> + reg = <0 0 0 0 0>; >> >> + #interrupt-cells = <1>; >> >> + #size-cells = <2>; >> >> + #address-cells = <3>; >> >> + device_type = "pci"; >> >> + >> >> + ath10k@0,0 { >> > >> > wifi@0,0 >> Added in ath10k@0,0 to make consistent with existing property >> "qcom,ath10k-calibration-data" below, >> " > > I don't see how that matters. > >> pci { >> pcie@0 { >> reg = <0 0 0 0 0>; >> #interrupt-cells = <1>; >> #size-cells = <2>; >> #address-cells = <3>; >> device_type = "pci"; >> >> ath10k@0,0 { >> reg = <0 0 0 0 0>; >> device_type = "pci"; >> qcom,ath10k-calibration-data = [ 01 02 03 ... >> ]; >> }; >> }; >> }; >> " >> >> If wifi@0,0 is preferable, then I will add new entry "wifi@0,0" and >> add >> ext-fem-name into it. > > Node names are supposed to reflect the class of device, not the model. > See the DT spec for a list. Will change accordingly. > >> >> > >> >> + reg = <0 0 0 0 0>; >> >> + device_type = "pci"; > > Also, this is wrong. Only PCI bridges should have this property. dtc > will give you a warning on this. Will remove it in v2 patch. > >> >> + ext-fem-name = "microsemi-lx5586"; >> >> + }; >> >> + }; >> >> +}; >> >> -- >> >> 1.9.1 >> >> >> >> Thanks, Sorry for the delayed response, >> Bhagavathi Perumal S.
diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt index 7fd4e8c..fbaf309 100644 --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt @@ -56,6 +56,7 @@ Optional properties: the length can vary between hw versions. - <supply-name>-supply: handle to the regulator device tree node optional "supply-name" is "vdd-0.8-cx-mx". +- ext-fem-name: name of external front end module used. Example (to supply the calibration data alone): @@ -150,3 +151,24 @@ wifi@18000000 { <0 141 0 /* CE11 */ >; vdd-0.8-cx-mx-supply = <&pm8998_l5>; }; + +Example (to supply the external front end module name): + +In this example, the front end module is defined as a property of the ath10k +device node. + +pci { + pcie@0 { + reg = <0 0 0 0 0>; + #interrupt-cells = <1>; + #size-cells = <2>; + #address-cells = <3>; + device_type = "pci"; + + ath10k@0,0 { + reg = <0 0 0 0 0>; + device_type = "pci"; + ext-fem-name = "microsemi-lx5586"; + }; + }; +};
This adds new dt entry ext-fem-name, it is used by ath10k driver to select correct timing parameters and configure it in target wifi hardware. The Front End Module(FEM) normally includes tx power amplifier(PA) and rx low noise amplifier(LNA). The default timing parameters like tx end to PA off timing values were fine tuned for internal FEM used in reference design. And these timing values can not be same if ODM modifies hardware design with different external FEM. This DT entry helps to choose correct timing values in driver if different external FEM hardware used. Signed-off-by: Bhagavathi Perumal S <bperumal@codeaurora.org> --- .../bindings/net/wireless/qcom,ath10k.txt | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)