diff mbox series

[v3,08/11] devicetree: bindings: pci: document PARF params bindings

Message ID 20200430220619.3169-9-ansuelsmth@gmail.com (mailing list archive)
State Superseded
Headers show
Series Multiple fixes in PCIe qcom driver | expand

Commit Message

Christian Marangi April 30, 2020, 10:06 p.m. UTC
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(+)

Comments

Rob Herring (Arm) May 7, 2020, 6:10 p.m. UTC | #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.

> +
> +- 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
>
Christian Marangi May 7, 2020, 7:34 p.m. UTC | #2
> 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
> >
Rob Herring (Arm) May 12, 2020, 3:45 p.m. UTC | #3
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
Stanimir Varbanov May 13, 2020, 11:43 a.m. UTC | #4
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
>
Christian Marangi May 13, 2020, 12:56 p.m. UTC | #5
> 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
Stanimir Varbanov May 20, 2020, 10:01 a.m. UTC | #6
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 mbox series

Patch

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";