mbox series

[0/3] Convert {a,i}pq8064 SATA AHCI controller bindings to dtschema

Message ID 20240716105245.49549-1-rayyan.ansari@linaro.org
Headers show
Series Convert {a,i}pq8064 SATA AHCI controller bindings to dtschema | expand

Message

Rayyan Ansari July 16, 2024, 10:45 a.m. UTC
Hello,
The following patches convert the old text bindings to dtschema by using
ahci-platform.yaml, whilst also fixing the device trees to actually
adhere to this schema.

Thanks,
Rayyan

Rayyan Ansari (3):
  ARM: dts: qcom: {a,i}pq8064: correct clock-names in sata node
  dt-bindings: ata: qcom,ipq806x-ahci: use dtschema
  dt-bindings: ata: qcom,apq8064-ahci: add to dtschema

 .../bindings/ata/ahci-platform.yaml           | 42 +++++++++++++++-
 .../devicetree/bindings/ata/qcom-sata.txt     | 48 -------------------
 arch/arm/boot/dts/qcom/qcom-apq8064.dtsi      |  4 +-
 arch/arm/boot/dts/qcom/qcom-ipq8064.dtsi      |  2 +-
 4 files changed, 43 insertions(+), 53 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/ata/qcom-sata.txt

Comments

Niklas Cassel July 17, 2024, 8:45 a.m. UTC | #1
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
>
Rayyan Ansari July 17, 2024, 9:05 a.m. UTC | #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
> >
Niklas Cassel July 17, 2024, 1:11 p.m. UTC | #3
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