| Message ID | 20231031120307.1600689-3-quic_mdalam@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add QPIC SPI NAND driver support | expand |
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>;
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
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.
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 --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>;