| Message ID | 20240716105245.49549-2-rayyan.ansari@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Convert {a,i}pq8064 SATA AHCI controller bindings to dtschema | expand |
On Tue, Jul 16, 2024 at 11:45:59AM +0100, Rayyan Ansari wrote: > Correct the clock-names in the AHCI SATA controller node to adhere to > the bindings. > > Signed-off-by: Rayyan Ansari <rayyan.ansari@linaro.org> Hello Rayyan, This patch is 1/3, so first in the series. A patch that is first in the series usually has no other dependencies. (Unless referencing another series in the cover-letter.) So is this a fix that can be sent out separately and picked up the QCOM maintainers / ARM DT maintainers directly, or does this patch actually depend on patches 2-3 ? If the former, I suggest that you send out patch 1/3 as a standalone fix, since it does not need to be blocked by unrelated DT binding conversion. If the latter, perhaps reorder the patches and improve the commit log for this patch. Kind regards, Niklas > --- > arch/arm/boot/dts/qcom/qcom-apq8064.dtsi | 4 ++-- > arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi > index 81cf387e1817..277bde958d0e 100644 > --- a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi > +++ b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi > @@ -889,9 +889,9 @@ sata0: sata@29000000 { > <&gcc SATA_PMALIVE_CLK>; > clock-names = "slave_iface", > "iface", > - "bus", > + "core", > "rxoob", > - "core_pmalive"; > + "pmalive"; > > assigned-clocks = <&gcc SATA_RXOOB_CLK>, > <&gcc SATA_PMALIVE_CLK>; > diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi > index da0fd75f4711..dd974eb4065f 100644 > --- a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi > +++ b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi > @@ -1292,7 +1292,7 @@ sata: sata@29000000 { > <&gcc SATA_A_CLK>, > <&gcc SATA_RXOOB_CLK>, > <&gcc SATA_PMALIVE_CLK>; > - clock-names = "slave_face", "iface", "core", > + clock-names = "slave_iface", "iface", "core", > "rxoob", "pmalive"; > > assigned-clocks = <&gcc SATA_RXOOB_CLK>, <&gcc SATA_PMALIVE_CLK>; > -- > 2.45.2 >
On Wed Jul 17, 2024 at 9:45 AM BST, Niklas Cassel wrote: > On Tue, Jul 16, 2024 at 11:45:59AM +0100, Rayyan Ansari wrote: > > Correct the clock-names in the AHCI SATA controller node to adhere to > > the bindings. > > > > Signed-off-by: Rayyan Ansari <rayyan.ansari@linaro.org> > > Hello Rayyan, > > This patch is 1/3, so first in the series. > A patch that is first in the series usually has no other dependencies. > (Unless referencing another series in the cover-letter.) > > So is this a fix that can be sent out separately and picked up the > QCOM maintainers / ARM DT maintainers directly, or does this patch > actually depend on patches 2-3 ? Hi Niklas, Yes, this patch does not depend on the following two patches, I just thought that sending this as a series would make sense given that patches 2-3 would surface this error (as we can run dtbs_check against yaml bindings but not text bindings). > If the former, I suggest that you send out patch 1/3 as a standalone > fix, since it does not need to be blocked by unrelated DT binding > conversion. Ah okay - for v2 I'll send patch 1 on its own, and then patch 2 & 3 as a series. > If the latter, perhaps reorder the patches and improve the commit log > for this patch. > > > Kind regards, > Niklas Thanks, Rayyan > > --- > > arch/arm/boot/dts/qcom/qcom-apq8064.dtsi | 4 ++-- > > arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 2 +- > > 2 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi > > index 81cf387e1817..277bde958d0e 100644 > > --- a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi > > +++ b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi > > @@ -889,9 +889,9 @@ sata0: sata@29000000 { > > <&gcc SATA_PMALIVE_CLK>; > > clock-names = "slave_iface", > > "iface", > > - "bus", > > + "core", > > "rxoob", > > - "core_pmalive"; > > + "pmalive"; > > > > assigned-clocks = <&gcc SATA_RXOOB_CLK>, > > <&gcc SATA_PMALIVE_CLK>; > > diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi > > index da0fd75f4711..dd974eb4065f 100644 > > --- a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi > > +++ b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi > > @@ -1292,7 +1292,7 @@ sata: sata@29000000 { > > <&gcc SATA_A_CLK>, > > <&gcc SATA_RXOOB_CLK>, > > <&gcc SATA_PMALIVE_CLK>; > > - clock-names = "slave_face", "iface", "core", > > + clock-names = "slave_iface", "iface", "core", > > "rxoob", "pmalive"; > > > > assigned-clocks = <&gcc SATA_RXOOB_CLK>, <&gcc SATA_PMALIVE_CLK>; > > -- > > 2.45.2 > >
On Wed, Jul 17, 2024 at 10:05:09AM +0100, Rayyan Ansari wrote: > > Hi Niklas, > > Yes, this patch does not depend on the following two patches, I just > thought that sending this as a series would make sense given that > patches 2-3 would surface this error (as we can run dtbs_check against > yaml bindings but not text bindings). Usually, DT maintainers prefer for DT bindings to go via subsystem trees (in this case libata). I guess DT maintainers could have picked the whole series, as they do occasionally, but they seem to want to avoid this as much as possible. In this case, considering that the DTS change (patch 1/3) is a strict fix, I think that it should be merged ASAP (target 6.11 instead of 6.12). We will queue the DT binding changes for 6.12. When also taking into consideration that the DT bindings and DTS changes have different trees, splitting the series was probably the right move. Kind regards, Niklas
diff --git a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi index 81cf387e1817..277bde958d0e 100644 --- a/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi +++ b/arch/arm/boot/dts/qcom/qcom-apq8064.dtsi @@ -889,9 +889,9 @@ sata0: sata@29000000 { <&gcc SATA_PMALIVE_CLK>; clock-names = "slave_iface", "iface", - "bus", + "core", "rxoob", - "core_pmalive"; + "pmalive"; assigned-clocks = <&gcc SATA_RXOOB_CLK>, <&gcc SATA_PMALIVE_CLK>; diff --git a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi index da0fd75f4711..dd974eb4065f 100644 --- a/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi +++ b/arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi @@ -1292,7 +1292,7 @@ sata: sata@29000000 { <&gcc SATA_A_CLK>, <&gcc SATA_RXOOB_CLK>, <&gcc SATA_PMALIVE_CLK>; - clock-names = "slave_face", "iface", "core", + clock-names = "slave_iface", "iface", "core", "rxoob", "pmalive"; assigned-clocks = <&gcc SATA_RXOOB_CLK>, <&gcc SATA_PMALIVE_CLK>;
Correct the clock-names in the AHCI SATA controller node to adhere to the bindings. Signed-off-by: Rayyan Ansari <rayyan.ansari@linaro.org> --- arch/arm/boot/dts/qcom/qcom-apq8064.dtsi | 4 ++-- arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)