[U-Boot] [PATCH 7/7] scsi: dts: a3700: add scsi node

[Stefan Roese]

Hi Ken,

On 23.03.2017 10:29, make at marvell.com wrote:
> From: Ken Ma <make at marvell.com>
>
> - Add scsi node which acts as a bus for scsi devices, armada3700 has
>   only 1 scsi interface, so max-id is 1, and the logic unit number is
>   also 1 for armada3700;
> - Since a3700's scsi is sas(serial attached scsi) which is compatible
>   for sata and sata hard disk is a sas device, so move sata node to be
>   under scsi node.
>
> Signed-off-by: Ken Ma <make at marvell.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Stefan Roese <sr at denx.de>
> Cc: Michal Simek <michal.simek at xilinx.com>
> Reviewed-on: http://vgitil04.il.marvell.com:8080/35303
> Tested-by: iSoC Platform CI <ykjenk at marvell.com>
> Reviewed-by: Kostya Porotchkin <kostap at marvell.com>
> Reviewed-by: Omri Itach <omrii at marvell.com>
> ---
>  arch/arm/dts/armada-3720-db.dts |  4 ++++
>  arch/arm/dts/armada-37xx.dtsi   | 16 ++++++++++++----
>  2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/dts/armada-3720-db.dts b/arch/arm/dts/armada-3720-db.dts
> index 85761af..9fc60f6 100644
> --- a/arch/arm/dts/armada-3720-db.dts
> +++ b/arch/arm/dts/armada-3720-db.dts
> @@ -89,6 +89,10 @@
>  	status = "okay";
>  };
>
> +&scsi {
> +	status = "okay";
> +};
> +
>  /* CON3 */
>  &sata {
>  	status = "okay";
> diff --git a/arch/arm/dts/armada-37xx.dtsi b/arch/arm/dts/armada-37xx.dtsi
> index 062f2a6..de5d3a1 100644
> --- a/arch/arm/dts/armada-37xx.dtsi
> +++ b/arch/arm/dts/armada-37xx.dtsi
> @@ -149,11 +149,19 @@
>  				status = "disabled";
>  			};
>
> -			sata: sata at e0000 {
> -				compatible = "marvell,armada-3700-ahci";
> -				reg = <0xe0000 0x2000>;
> -				interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
> +			scsi: scsi {
> +				compatible = "marvell,mvebu-scsi";
> +				#address-cells = <1>;
> +				#size-cells = <1>;
> +				max-id = <1>;
> +				max-lun = <1>;
>  				status = "disabled";
> +				sata: sata at e0000 {
> +					compatible = "marvell,armada-3700-ahci";
> +					reg = <0xe0000 0x2000>;
> +					interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
> +					status = "disabled";
> +				};
>  			};
>
>  			gic: interrupt-controller at 1d00000 {
>

I see that you introduce a "scsi" DT node and move the SATA controller
one "level up". I'm not sure if such a change is acceptable as we try
to re-use the DT from Linux. Or thinking more about this, I'm pretty
sure that such a change is not acceptable in general.

Can't you use the existing DT layout and use the
"marvell,armada-3700-ahci" (and other perhaps?) compatible property
instead for driver probing? Not sure how to handle the "max-id" and
"max-lun" properties though. We definitely can't just add some ad-hoc
properties here in U-Boot which have no chance for Linux upstream
acceptance.

Thanks,
Stefan