| Message ID | 20200430220619.3169-9-ansuelsmth@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Multiple fixes in PCIe qcom driver | expand |
On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote: > It is now supported the editing of Tx De-Emphasis, Tx Swing and > Rx equalization params on ipq8064. Document this new optional params. > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > --- > .../devicetree/bindings/pci/qcom,pcie.txt | 36 +++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > index 6efcef040741..8cc5aea8a1da 100644 > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > @@ -254,6 +254,42 @@ > - "perst-gpios" PCIe endpoint reset signal line > - "wake-gpios" PCIe endpoint wake signal line > > +- qcom,tx-deemph-gen1: > + Usage: optional (available for ipq/apq8064) > + Value type: <u32> > + Definition: Gen1 De-emphasis value. > + For ipq806x should be set to 24. Unless these need to be tuned per board, then the compatible string for ipq806x should imply all these settings. > + > +- qcom,tx-deemph-gen2-3p5db: > + Usage: optional (available for ipq/apq8064) > + Value type: <u32> > + Definition: Gen2 (3.5db) De-emphasis value. > + For ipq806x should be set to 24. > + > +- qcom,tx-deemph-gen2-6db: > + Usage: optional (available for ipq/apq8064) > + Value type: <u32> > + Definition: Gen2 (6db) De-emphasis value. > + For ipq806x should be set to 34. > + > +- qcom,tx-swing-full: > + Usage: optional (available for ipq/apq8064) > + Value type: <u32> > + Definition: TX launch amplitude swing_full value. > + For ipq806x should be set to 120. > + > +- qcom,tx-swing-low: > + Usage: optional (available for ipq/apq8064) > + Value type: <u32> > + Definition: TX launch amplitude swing_low value. > + For ipq806x should be set to 120. > + > +- qcom,rx0-eq: > + Usage: optional (available for ipq/apq8064) > + Value type: <u32> > + Definition: RX0 equalization value. > + For ipq806x should be set to 4. > + > * Example for ipq/apq8064 > pcie@1b500000 { > compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "snps,dw-pcie"; > -- > 2.25.1 >
> On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote: > > It is now supported the editing of Tx De-Emphasis, Tx Swing and > > Rx equalization params on ipq8064. Document this new optional params. > > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > > --- > > .../devicetree/bindings/pci/qcom,pcie.txt | 36 +++++++++++++++++++ > > 1 file changed, 36 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > > index 6efcef040741..8cc5aea8a1da 100644 > > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > > @@ -254,6 +254,42 @@ > > - "perst-gpios" PCIe endpoint reset signal line > > - "wake-gpios" PCIe endpoint wake signal line > > > > +- qcom,tx-deemph-gen1: > > + Usage: optional (available for ipq/apq8064) > > + Value type: <u32> > > + Definition: Gen1 De-emphasis value. > > + For ipq806x should be set to 24. > > Unless these need to be tuned per board, then the compatible string for > ipq806x should imply all these settings. > It was requested by v2 to make this settings tunable. These don't change are all the same for every ipq806x SoC. The original implementation had this value hardcoded for ipq806x. Should I restore this and drop this patch? Anyway thanks for the review. > > + > > +- qcom,tx-deemph-gen2-3p5db: > > + Usage: optional (available for ipq/apq8064) > > + Value type: <u32> > > + Definition: Gen2 (3.5db) De-emphasis value. > > + For ipq806x should be set to 24. > > + > > +- qcom,tx-deemph-gen2-6db: > > + Usage: optional (available for ipq/apq8064) > > + Value type: <u32> > > + Definition: Gen2 (6db) De-emphasis value. > > + For ipq806x should be set to 34. > > + > > +- qcom,tx-swing-full: > > + Usage: optional (available for ipq/apq8064) > > + Value type: <u32> > > + Definition: TX launch amplitude swing_full value. > > + For ipq806x should be set to 120. > > + > > +- qcom,tx-swing-low: > > + Usage: optional (available for ipq/apq8064) > > + Value type: <u32> > > + Definition: TX launch amplitude swing_low value. > > + For ipq806x should be set to 120. > > + > > +- qcom,rx0-eq: > > + Usage: optional (available for ipq/apq8064) > > + Value type: <u32> > > + Definition: RX0 equalization value. > > + For ipq806x should be set to 4. > > + > > * Example for ipq/apq8064 > > pcie@1b500000 { > > compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", > "snps,dw-pcie"; > > -- > > 2.25.1 > >
On Thu, May 07, 2020 at 09:34:35PM +0200, ansuelsmth@gmail.com wrote: > > On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote: > > > It is now supported the editing of Tx De-Emphasis, Tx Swing and > > > Rx equalization params on ipq8064. Document this new optional params. > > > > > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > > > --- > > > .../devicetree/bindings/pci/qcom,pcie.txt | 36 +++++++++++++++++++ > > > 1 file changed, 36 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > > b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > > > index 6efcef040741..8cc5aea8a1da 100644 > > > --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > > > +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > > > @@ -254,6 +254,42 @@ > > > - "perst-gpios" PCIe endpoint reset signal line > > > - "wake-gpios" PCIe endpoint wake signal line > > > > > > +- qcom,tx-deemph-gen1: > > > + Usage: optional (available for ipq/apq8064) > > > + Value type: <u32> > > > + Definition: Gen1 De-emphasis value. > > > + For ipq806x should be set to 24. > > > > Unless these need to be tuned per board, then the compatible string for > > ipq806x should imply all these settings. > > > > It was requested by v2 to make this settings tunable. These don't change are > all the same for every ipq806x SoC. The original implementation had this > value hardcoded for ipq806x. Should I restore this and drop this patch? Yes, please. Rob
On 5/12/20 6:45 PM, Rob Herring wrote: > On Thu, May 07, 2020 at 09:34:35PM +0200, ansuelsmth@gmail.com wrote: >>> On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote: >>>> It is now supported the editing of Tx De-Emphasis, Tx Swing and >>>> Rx equalization params on ipq8064. Document this new optional params. >>>> >>>> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> >>>> --- >>>> .../devicetree/bindings/pci/qcom,pcie.txt | 36 +++++++++++++++++++ >>>> 1 file changed, 36 insertions(+) >>>> >>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt >>> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt >>>> index 6efcef040741..8cc5aea8a1da 100644 >>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt >>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt >>>> @@ -254,6 +254,42 @@ >>>> - "perst-gpios" PCIe endpoint reset signal line >>>> - "wake-gpios" PCIe endpoint wake signal line >>>> >>>> +- qcom,tx-deemph-gen1: >>>> + Usage: optional (available for ipq/apq8064) >>>> + Value type: <u32> >>>> + Definition: Gen1 De-emphasis value. >>>> + For ipq806x should be set to 24. >>> >>> Unless these need to be tuned per board, then the compatible string for >>> ipq806x should imply all these settings. >>> >> >> It was requested by v2 to make this settings tunable. These don't change are >> all the same for every ipq806x SoC. The original implementation had this >> value hardcoded for ipq806x. Should I restore this and drop this patch? > > Yes, please. I still think that the values for tx deemph and tx swing should be tunable. But I can live with them in the driver if they not break support for apq8064. The default values in the registers for apq8064 and ipq806x are: default your change TX_DEEMPH_GEN1 21 24 TX_DEEMPH_GEN2_3_5DB 21 24 TX_DEEMPH_GEN2_6DB 32 34 TX_SWING_FULL 121 120 TX_SWING_LOW 121 120 So until now (without your change) apq8064 worked with default values. > > Rob >
> On 5/12/20 6:45 PM, Rob Herring wrote: > > On Thu, May 07, 2020 at 09:34:35PM +0200, ansuelsmth@gmail.com > wrote: > >>> On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote: > >>>> It is now supported the editing of Tx De-Emphasis, Tx Swing and > >>>> Rx equalization params on ipq8064. Document this new optional > params. > >>>> > >>>> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> > >>>> --- > >>>> .../devicetree/bindings/pci/qcom,pcie.txt | 36 > +++++++++++++++++++ > >>>> 1 file changed, 36 insertions(+) > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > >>> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > >>>> index 6efcef040741..8cc5aea8a1da 100644 > >>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt > >>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt > >>>> @@ -254,6 +254,42 @@ > >>>> - "perst-gpios" PCIe endpoint reset signal line > >>>> - "wake-gpios" PCIe endpoint wake signal line > >>>> > >>>> +- qcom,tx-deemph-gen1: > >>>> + Usage: optional (available for ipq/apq8064) > >>>> + Value type: <u32> > >>>> + Definition: Gen1 De-emphasis value. > >>>> + For ipq806x should be set to 24. > >>> > >>> Unless these need to be tuned per board, then the compatible string > for > >>> ipq806x should imply all these settings. > >>> > >> > >> It was requested by v2 to make this settings tunable. These don't change > are > >> all the same for every ipq806x SoC. The original implementation had this > >> value hardcoded for ipq806x. Should I restore this and drop this patch? > > > > Yes, please. > > I still think that the values for tx deemph and tx swing should be > tunable. But I can live with them in the driver if they not break > support for apq8064. > > The default values in the registers for apq8064 and ipq806x are: > > default your change > TX_DEEMPH_GEN1 21 24 > TX_DEEMPH_GEN2_3_5DB 21 24 > TX_DEEMPH_GEN2_6DB 32 34 > > TX_SWING_FULL 121 120 > TX_SWING_LOW 121 120 > > So until now (without your change) apq8064 worked with default values. > I will limit this to ipq8064(-v2) if this could be a problem. > > > > Rob > > > > -- > regards, > Stan
Hi, On 5/13/20 3:56 PM, ansuelsmth@gmail.com wrote: >> On 5/12/20 6:45 PM, Rob Herring wrote: >>> On Thu, May 07, 2020 at 09:34:35PM +0200, ansuelsmth@gmail.com >> wrote: >>>>> On Fri, May 01, 2020 at 12:06:15AM +0200, Ansuel Smith wrote: >>>>>> It is now supported the editing of Tx De-Emphasis, Tx Swing and >>>>>> Rx equalization params on ipq8064. Document this new optional >> params. >>>>>> >>>>>> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> >>>>>> --- >>>>>> .../devicetree/bindings/pci/qcom,pcie.txt | 36 >> +++++++++++++++++++ >>>>>> 1 file changed, 36 insertions(+) >>>>>> >>>>>> diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt >>>>> b/Documentation/devicetree/bindings/pci/qcom,pcie.txt >>>>>> index 6efcef040741..8cc5aea8a1da 100644 >>>>>> --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt >>>>>> +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt >>>>>> @@ -254,6 +254,42 @@ >>>>>> - "perst-gpios" PCIe endpoint reset signal line >>>>>> - "wake-gpios" PCIe endpoint wake signal line >>>>>> >>>>>> +- qcom,tx-deemph-gen1: >>>>>> + Usage: optional (available for ipq/apq8064) >>>>>> + Value type: <u32> >>>>>> + Definition: Gen1 De-emphasis value. >>>>>> + For ipq806x should be set to 24. >>>>> >>>>> Unless these need to be tuned per board, then the compatible string >> for >>>>> ipq806x should imply all these settings. >>>>> >>>> >>>> It was requested by v2 to make this settings tunable. These don't change >> are >>>> all the same for every ipq806x SoC. The original implementation had this >>>> value hardcoded for ipq806x. Should I restore this and drop this patch? >>> >>> Yes, please. >> >> I still think that the values for tx deemph and tx swing should be >> tunable. But I can live with them in the driver if they not break >> support for apq8064. >> >> The default values in the registers for apq8064 and ipq806x are: >> >> default your change >> TX_DEEMPH_GEN1 21 24 >> TX_DEEMPH_GEN2_3_5DB 21 24 >> TX_DEEMPH_GEN2_6DB 32 34 >> >> TX_SWING_FULL 121 120 >> TX_SWING_LOW 121 120 >> >> So until now (without your change) apq8064 worked with default values. >> > > I will limit this to ipq8064(-v2) if this could be a problem. I guess you can do it that way, but if new board appear in the future with slightly different parameters (for example deemph_gen1 = 23 and so on) do we need to add another compatible for that? At the end we will have compatibles per board but not per SoC. :(
diff --git a/Documentation/devicetree/bindings/pci/qcom,pcie.txt b/Documentation/devicetree/bindings/pci/qcom,pcie.txt index 6efcef040741..8cc5aea8a1da 100644 --- a/Documentation/devicetree/bindings/pci/qcom,pcie.txt +++ b/Documentation/devicetree/bindings/pci/qcom,pcie.txt @@ -254,6 +254,42 @@ - "perst-gpios" PCIe endpoint reset signal line - "wake-gpios" PCIe endpoint wake signal line +- qcom,tx-deemph-gen1: + Usage: optional (available for ipq/apq8064) + Value type: <u32> + Definition: Gen1 De-emphasis value. + For ipq806x should be set to 24. + +- qcom,tx-deemph-gen2-3p5db: + Usage: optional (available for ipq/apq8064) + Value type: <u32> + Definition: Gen2 (3.5db) De-emphasis value. + For ipq806x should be set to 24. + +- qcom,tx-deemph-gen2-6db: + Usage: optional (available for ipq/apq8064) + Value type: <u32> + Definition: Gen2 (6db) De-emphasis value. + For ipq806x should be set to 34. + +- qcom,tx-swing-full: + Usage: optional (available for ipq/apq8064) + Value type: <u32> + Definition: TX launch amplitude swing_full value. + For ipq806x should be set to 120. + +- qcom,tx-swing-low: + Usage: optional (available for ipq/apq8064) + Value type: <u32> + Definition: TX launch amplitude swing_low value. + For ipq806x should be set to 120. + +- qcom,rx0-eq: + Usage: optional (available for ipq/apq8064) + Value type: <u32> + Definition: RX0 equalization value. + For ipq806x should be set to 4. + * Example for ipq/apq8064 pcie@1b500000 { compatible = "qcom,pcie-apq8064", "qcom,pcie-ipq8064", "snps,dw-pcie";
It is now supported the editing of Tx De-Emphasis, Tx Swing and Rx equalization params on ipq8064. Document this new optional params. Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com> --- .../devicetree/bindings/pci/qcom,pcie.txt | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+)