Hi Ken,
On 24.03.2017 05:11, Ken Ma wrote:
<snip>
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@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@e0000 {
+ compatible = "marvell,armada-3700-ahci";
+ reg = <0xe0000 0x2000>;
+ interrupts = <GIC_SPI 27 IRQ_TYPE_LEVEL_HIGH>;
+ status = "disabled";
+ };
};
gic: interrupt-controller@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.
[Ken] Because scsi is a bus, for example, if there are 2 scsi buses,
each bus has some scsi device controllers connected as below.
Scsi ID 0 Scsi ID 1 Scsi ID 2 Scsi ID 3
HDD0 HDD1 tape0 cd-rom0
|| || || ||
===============================================================
SCSI BUS1
HDD2 HDD3 tape1 cd-rom2
|| || || ||
===============================================================
SCSI BUS2
As far as I understand, you are looking at this from the external point
of view (SCSI devices connected to the board). But the DT describes
the hardware / interfaces from the CPU / SoC point of view. The
SCSI bus topology can change, depending on which and how the "user"
connects the multiple SCSI devices to the different controllers.
This is definitely not what we can describe in the DT for the
board. Here only the view of the internal controllers / interfaces
is described (UART controller at 0x..., SPI controller at 0x..,
AHCI controller at 0x..., etc).
Then in my opinion, since now scsi has its own class id and its
compatible string, then the scsi device controllers dts node should be
above the scsi node.
As mentioned before, we are not "free" to insert "virtual" controllers
in the DT. Only real hardware interfaces can be described.
If we keep existing DT layout and keep "marvell,armada-3700-ahci"’s
uclass id as UCLASS_AHCI(there are no scsi nodes but only ahci nodes),
then scsi_scan() can not find a3700’s sata at all since there are no
UCLASS_SCSI devices;
I've attached the small patch that I've send you a few weeks ago.
Didn't this work at all? IIRC, then the devices connected to the
SATA ports cuold be detected this way.
If we keep existing DT layout and set scsi devices’ uclass id to be
UCLASS_SCSI, how can we know that hdd0 and hdd1 are in scsi bus1 but
hdd2 and hdd3 are in scsi bus2? For each scsi bus, its max id should be
4; but now how to set each scsi device’ scsi id?
Not sure if I understand you correctly here. Could you please
describe the problem that you are facing, using an example? That
would be clearer, at least to me.
So I think we should move scsi devices “level up” on the scsio bus.
Might be that I misunderstand you, but I think you are still
viewing this scenario from the external point of view and not
the internal one (as mentioned before). This is not, what the
DT is supposed to describe though.
Thanks,
Stefan
>From 0c071f4815ad76ed9eb64e4261066856062ba115 Mon Sep 17 00:00:00 2001
From: Stefan Roese <s...@denx.de>
Date: Fri, 13 Jan 2017 12:53:20 +0100
Subject: [PATCH] ahci: Add DM based support for the Marvell MVEBU SATA
controller
This patch adds support for AHCI compatible SATA controller available
on the Marvell MVEBU SoCs. This replaces the old-style SATA implementation
present in the arch/arm/mach-mvebu directory.
Signed-off-by: Stefan Roese <s...@denx.de>
---
drivers/block/Kconfig | 8 ++++++
drivers/block/Makefile | 1 +
drivers/block/sata_mvebu.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 80 insertions(+)
create mode 100644 drivers/block/sata_mvebu.c
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 88e66e2377..dd4ee3f082 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -48,4 +48,12 @@ config SATA_CEVA
ZynqMP. Support up to 2 external devices. Complient with SATA 3.1 and
AHCI 1.3 specifications with hot-plug detect feature.
+config SATA_MVEBU
+ bool "Marvell MVEBU SATA (AHCI) controller"
+ depends on AHCI
+ depends on DM_SCSI
+ help
+ This option enables support for the AHCI compatible SATA controller
+ available on the Marvell MVEBU based SoCs.
+
endmenu
diff --git a/drivers/block/Makefile b/drivers/block/Makefile
index a72feecd54..22cd53e40e 100644
--- a/drivers/block/Makefile
+++ b/drivers/block/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_PATA_BFIN) += pata_bfin.o
obj-$(CONFIG_SATA_CEVA) += sata_ceva.o
obj-$(CONFIG_SATA_DWC) += sata_dwc.o
obj-$(CONFIG_SATA_MV) += sata_mv.o
+obj-$(CONFIG_SATA_MVEBU) += sata_mvebu.o
obj-$(CONFIG_SATA_SIL3114) += sata_sil3114.o
obj-$(CONFIG_SATA_SIL) += sata_sil.o
obj-$(CONFIG_IDE_SIL680) += sil680.o
diff --git a/drivers/block/sata_mvebu.c b/drivers/block/sata_mvebu.c
new file mode 100644
index 0000000000..bbf7f37aa4
--- /dev/null
+++ b/drivers/block/sata_mvebu.c
@@ -0,0 +1,71 @@
+/*
+ * Copyright (C) 2016 Stefan Roese <s...@denx.de>
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <ahci.h>
+#include <dm.h>
+#include <scsi.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/*
+ * Dummy implementation that can be overwritten by a board
+ * specific function
+ */
+__weak int board_ahci_enable(void)
+{
+ return 0;
+}
+
+#ifdef CONFIG_ARMADA_8K
+/* CP110 has different AHCI port addresses */
+void __iomem *ahci_port_base(void __iomem *base, u32 port)
+{
+ return base + 0x10000 + (port * 0x10000);
+}
+#endif
+
+static int mvebu_ahci_probe(struct udevice *dev)
+{
+ /*
+ * Board specific SATA / AHCI enable code, e.g. enable the
+ * AHCI power or deassert reset
+ */
+ board_ahci_enable();
+
+ return 0;
+}
+
+static int mvebu_ahci_ofdata_to_platdata(struct udevice *dev)
+{
+ struct scsi_platdata *plat = dev_get_platdata(dev);
+
+ plat->base = dev_get_addr(dev);
+ if (plat->base == FDT_ADDR_T_NONE)
+ return -EINVAL;
+
+ /* Hardcode number for MVEBU SATA controller */
+ /* test-only: these following values need to get checked... */
+ plat->max_lun = 1;
+ plat->max_id = 2;
+
+ return 0;
+}
+
+static const struct udevice_id mvebu_ahci_ids[] = {
+ { .compatible = "marvell,armada-3700-ahci" },
+ { .compatible = "marvell,armada-8k-ahci" },
+ { }
+};
+
+U_BOOT_DRIVER(ahci_mvebu_blk) = {
+ .name = "ahci_mvebu",
+ .id = UCLASS_SCSI,
+ .of_match = mvebu_ahci_ids,
+ .probe = mvebu_ahci_probe,
+ .ofdata_to_platdata = mvebu_ahci_ofdata_to_platdata,
+ .platdata_auto_alloc_size = sizeof(struct scsi_platdata),
+};
--
2.12.1
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
