diff mbox series

[v2,1/4] arm64: dts: qcom: x1e80100: Add PCIe lane equalization preset properties

Message ID 20241212-preset_v2-v2-1-210430fbcd8a@oss.qualcomm.com (mailing list archive)
State Superseded
Headers show
Series PCI: dwc: Add support for configuring lane equalization presets | expand

Commit Message

Krishna Chaitanya Chundru Dec. 12, 2024, 10:32 a.m. UTC
From: Krishna chaitanya chundru <quic_krichai@quicinc.com>

Add PCIe lane equalization preset properties for 8 GT/s and 16 GT/s data
rates used in lane equalization procedure.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/x1e80100.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Krzysztof Kozlowski Dec. 12, 2024, 12:25 p.m. UTC | #1
On 12/12/2024 11:32, Krishna Chaitanya Chundru wrote:
> From: Krishna chaitanya chundru <quic_krichai@quicinc.com>
> 
> Add PCIe lane equalization preset properties for 8 GT/s and 16 GT/s data
> rates used in lane equalization procedure.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> index a36076e3c56b..6a2074297030 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
> @@ -2993,6 +2993,10 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>  			phys = <&pcie6a_phy>;
>  			phy-names = "pciephy";
>  
> +			eq-presets-8gts = /bits/ 16 <0x5555 0x5555>;
> +
> +			eq-presets-16gts = /bits/ 8 <0x55 0x55>;
NAK for two reasons (stated many times during review):
1. There is no way driver code can depend on DTS, unless you fix
something serious but nothing is explained about that serious fix in
commit msg.

2. There are no such properties. It does not look like you tested the
DTS against bindings. Please run `make dtbs_check W=1` (see
Documentation/devicetree/bindings/writing-schema.rst or
https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
for instructions).

Best regards,
Krzysztof
Krishna Chaitanya Chundru Dec. 12, 2024, 12:32 p.m. UTC | #2
On 12/12/2024 5:55 PM, Krzysztof Kozlowski wrote:
> On 12/12/2024 11:32, Krishna Chaitanya Chundru wrote:
>> From: Krishna chaitanya chundru <quic_krichai@quicinc.com>
>>
>> Add PCIe lane equalization preset properties for 8 GT/s and 16 GT/s data
>> rates used in lane equalization procedure.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>> ---
>>   arch/arm64/boot/dts/qcom/x1e80100.dtsi | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> index a36076e3c56b..6a2074297030 100644
>> --- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
>> @@ -2993,6 +2993,10 @@ &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
>>   			phys = <&pcie6a_phy>;
>>   			phy-names = "pciephy";
>>   
>> +			eq-presets-8gts = /bits/ 16 <0x5555 0x5555>;
>> +
>> +			eq-presets-16gts = /bits/ 8 <0x55 0x55>;
> NAK for two reasons (stated many times during review):
> 1. There is no way driver code can depend on DTS, unless you fix
> something serious but nothing is explained about that serious fix in
> commit msg.
> 
Please ignore this series as the main patch was missing in this series,
I will resend this after adding missing patch shortly, currently facing
issues with git config on my system. If you look in to v1 patch there is
driver patch which is missing in this series. I will fix it next series.
> 2. There are no such properties. It does not look like you tested the
> DTS against bindings. Please run `make dtbs_check W=1` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).
The property is added recently in to the dtschema in github repo, I
added the pull request details in the cover letter, I will add the
github link as part of the comment section for this patch in next
series.

- Krishna Chaitanya
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Dec. 12, 2024, 12:37 p.m. UTC | #3
On 12/12/2024 13:32, Krishna Chaitanya Chundru wrote:
>> 2. There are no such properties. It does not look like you tested the
>> DTS against bindings. Please run `make dtbs_check W=1` (see
>> Documentation/devicetree/bindings/writing-schema.rst or
>> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
>> for instructions).
> The property is added recently in to the dtschema in github repo, I
> added the pull request details in the cover letter, I will add the
> github link as part of the comment section for this patch in next
> series.
> 
Mention in the commit msg or cover letter where are the bindings.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/x1e80100.dtsi b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
index a36076e3c56b..6a2074297030 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e80100.dtsi
@@ -2993,6 +2993,10 @@  &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 			phys = <&pcie6a_phy>;
 			phy-names = "pciephy";
 
+			eq-presets-8gts = /bits/ 16 <0x5555 0x5555>;
+
+			eq-presets-16gts = /bits/ 8 <0x55 0x55>;
+
 			status = "disabled";
 		};
 
@@ -3115,6 +3119,8 @@  &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 			phys = <&pcie5_phy>;
 			phy-names = "pciephy";
 
+			eq-presets-8gts = /bits/ 16 <0x5555 0x5555>;
+
 			status = "disabled";
 		};
 
@@ -3235,6 +3241,8 @@  &mc_virt SLAVE_EBI1 QCOM_ICC_TAG_ALWAYS>,
 			phys = <&pcie4_phy>;
 			phy-names = "pciephy";
 
+			eq-presets-8gts = /bits/ 16 <0x5555 0x5555>;
+
 			status = "disabled";
 
 			pcie4_port0: pcie@0 {