diff mbox series

[RFC,2/5] arm64: dts: qcom: ipq9574: Add ecc engine support

Message ID 20231031120307.1600689-3-quic_mdalam@quicinc.com
State New
Headers
Series Add QPIC SPI NAND driver support |

Commit Message

Md Sadre Alam Oct. 31, 2023, 12:03 p.m. UTC 
  Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
---
 arch/arm64/boot/dts/qcom/ipq9574.dtsi | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Konrad Dybcio Oct. 31, 2023, 3:23 p.m. UTC | #1 
On 31.10.2023 13:03, Md Sadre Alam wrote:
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
> ---
Hello,

you're missing:

- dt-bindings (make dtbs_check is unhappy)
- a commit message
- Co-developed-by for Sricharan


status should read "okay" instead, but in this case it's unnecessary
as you're defining the node and lack of the status property also means
that device is enabled

however

this ECC engine seems to be a part of the NAND controller, so it's
unlikely that the DT maintainers will agree for it to have a separate
node

Konrad
>  arch/arm64/boot/dts/qcom/ipq9574.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 5f83ee42a719..b44acb1fac74 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> @@ -336,6 +336,11 @@ sdhc_1: mmc@7804000 {
>  			status = "disabled";
>  		};
>  
> +		bch: qpic_ecc {
> +			compatible = "qcom,ipq9574-ecc";
> +			status = "ok";
> +		}
> +
>  		blsp_dma: dma-controller@7884000 {
>  			compatible = "qcom,bam-v1.7.0";
>  			reg = <0x07884000 0x2b000>;
Krzysztof Kozlowski Oct. 31, 2023, 5:12 p.m. UTC | #2 
On 31/10/2023 13:03, Md Sadre Alam wrote:
> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
> Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/ipq9574.dtsi | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> index 5f83ee42a719..b44acb1fac74 100644
> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
> @@ -336,6 +336,11 @@ sdhc_1: mmc@7804000 {
>  			status = "disabled";
>  		};
>  
> +		bch: qpic_ecc {
> +			compatible = "qcom,ipq9574-ecc";

NAK. There are so many wrong things with it... Let's start with missing
checkpatch. Then with unndeeded label. Then with not allowed underscores
in node names.
Finally:
> +			status = "ok";

Drop.



Best regards,
Krzysztof
Md Sadre Alam Nov. 3, 2023, 11:26 a.m. UTC | #3 
On 10/31/2023 8:53 PM, Konrad Dybcio wrote:
> On 31.10.2023 13:03, Md Sadre Alam wrote:
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
>> ---
> Hello,
> 
> you're missing:
> 
> - dt-bindings (make dtbs_check is unhappy)
> - a commit message
> - Co-developed-by for Sricharan

> 
> status should read "okay" instead, but in this case it's unnecessary
> as you're defining the node and lack of the status property also means
> that device is enabled
> 
> however
> 
> this ECC engine seems to be a part of the NAND controller, so it's
> unlikely that the DT maintainers will agree for it to have a separate
> node
> 


Will drop this patch as this was NAK-ed
QPIC controller has the ecc pipelined so will keep the ecc support
inlined in both raw nand and serial nand driver.

Regards
Alam.
Md Sadre Alam Nov. 3, 2023, 12:09 p.m. UTC | #4 
On 10/31/2023 10:42 PM, Krzysztof Kozlowski wrote:
> On 31/10/2023 13:03, Md Sadre Alam wrote:
>> Signed-off-by: Md Sadre Alam <quic_mdalam@quicinc.com>
>> Signed-off-by: Sricharan R <quic_srichara@quicinc.com>
>> ---
>>   arch/arm64/boot/dts/qcom/ipq9574.dtsi | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> index 5f83ee42a719..b44acb1fac74 100644
>> --- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> +++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
>> @@ -336,6 +336,11 @@ sdhc_1: mmc@7804000 {
>>   			status = "disabled";
>>   		};
>>   
>> +		bch: qpic_ecc {
>> +			compatible = "qcom,ipq9574-ecc";
> 
> NAK. There are so many wrong things with it... Let's start with missing
> checkpatch. Then with unndeeded label. Then with not allowed underscores
> in node names.
> Finally:
>> +			status = "ok";
> 
> Drop.
> 
Ok
> 
> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/ipq9574.dtsi b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
index 5f83ee42a719..b44acb1fac74 100644
--- a/arch/arm64/boot/dts/qcom/ipq9574.dtsi
+++ b/arch/arm64/boot/dts/qcom/ipq9574.dtsi
@@ -336,6 +336,11 @@  sdhc_1: mmc@7804000 {
 			status = "disabled";
 		};
 
+		bch: qpic_ecc {
+			compatible = "qcom,ipq9574-ecc";
+			status = "ok";
+		}
+
 		blsp_dma: dma-controller@7884000 {
 			compatible = "qcom,bam-v1.7.0";
 			reg = <0x07884000 0x2b000>;