[U-Boot] [PATCH v2 0/7] driver model bring-up of sata device on dra72 and dra74 evm

This patch series enables sata driver to adopt driver model. This has been tested on the following evms (logs [1]) by loading kernel and dtb from sata hard-disk. * dra72 evm * dra74 evm
Also pushed a branch for testing [2]
[1] - http://pastebin.ubuntu.com/14865336/ [2] - git://git.ti.com/~mugunthanvnm/ti-u-boot/mugunth-ti-u-boot.git dm-sata-v2
Mugunthan V N (7): arm: omap: sata: move enable sata clocks to enable_basic_clocks() arm: omap: sata: compile out sata init apis when CONFIG_DISK is defined drivers: block: disk-uclass: implement scsi_init() arm: omap-common: sata: prepare driver for DM conversion drivers: block: dwc_ahci: Implement a driver for Synopsys DWC sata device defconfig: dra74_evm: enable disk driver model defconfig: dra72_evm: enable disk driver model
arch/arm/cpu/armv7/omap-common/sata.c | 38 ++++++------------- arch/arm/cpu/armv7/omap5/hw_data.c | 12 ++++++ configs/dra72_evm_defconfig | 2 + configs/dra74_evm_defconfig | 2 + drivers/block/Kconfig | 7 ++++ drivers/block/Makefile | 2 + drivers/block/disk-uclass.c | 39 +++++++++++++++++++ drivers/block/dwc_ahci.c | 71 +++++++++++++++++++++++++++++++++++ include/sata.h | 2 + 9 files changed, 148 insertions(+), 27 deletions(-) create mode 100644 drivers/block/dwc_ahci.c

All the clocks which has to be enabled has to be done in enable_basic_clocks(), so moving enable sata clock to common clocks enable function.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Tom Rini trini@konsulko.com --- arch/arm/cpu/armv7/omap-common/sata.c | 23 ----------------------- arch/arm/cpu/armv7/omap5/hw_data.c | 12 ++++++++++++ 2 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap-common/sata.c b/arch/arm/cpu/armv7/omap-common/sata.c index 2c2d1bc..0c82689 100644 --- a/arch/arm/cpu/armv7/omap-common/sata.c +++ b/arch/arm/cpu/armv7/omap-common/sata.c @@ -37,29 +37,6 @@ int init_sata(int dev) int ret; u32 val;
- u32 const clk_domains_sata[] = { - 0 - }; - - u32 const clk_modules_hw_auto_sata[] = { - (*prcm)->cm_l3init_ocp2scp3_clkctrl, - 0 - }; - - u32 const clk_modules_explicit_en_sata[] = { - (*prcm)->cm_l3init_sata_clkctrl, - 0 - }; - - do_enable_clocks(clk_domains_sata, - clk_modules_hw_auto_sata, - clk_modules_explicit_en_sata, - 0); - - /* Enable optional functional clock for SATA */ - setbits_le32((*prcm)->cm_l3init_sata_clkctrl, - SATA_CLKCTRL_OPTFCLKEN_MASK); - sata_phy.power_reg = (void __iomem *)(*ctrl)->control_phy_power_sata;
/* Power up the PHY */ diff --git a/arch/arm/cpu/armv7/omap5/hw_data.c b/arch/arm/cpu/armv7/omap5/hw_data.c index 7f8c0a4..2e554bd 100644 --- a/arch/arm/cpu/armv7/omap5/hw_data.c +++ b/arch/arm/cpu/armv7/omap5/hw_data.c @@ -460,6 +460,9 @@ void enable_basic_clocks(void) (*prcm)->cm_l4per_gpio6_clkctrl, (*prcm)->cm_l4per_gpio7_clkctrl, (*prcm)->cm_l4per_gpio8_clkctrl, +#ifdef CONFIG_SCSI_AHCI_PLAT + (*prcm)->cm_l3init_ocp2scp3_clkctrl, +#endif 0 };
@@ -478,6 +481,9 @@ void enable_basic_clocks(void) #ifdef CONFIG_TI_QSPI (*prcm)->cm_l4per_qspi_clkctrl, #endif +#ifdef CONFIG_SCSI_AHCI_PLAT + (*prcm)->cm_l3init_sata_clkctrl, +#endif 0 };
@@ -510,6 +516,12 @@ void enable_basic_clocks(void) setbits_le32((*prcm)->cm_l4per_qspi_clkctrl, (1<<24)); #endif
+#ifdef CONFIG_SCSI_AHCI_PLAT + /* Enable optional functional clock for SATA */ + setbits_le32((*prcm)->cm_l3init_sata_clkctrl, + SATA_CLKCTRL_OPTFCLKEN_MASK); +#endif + /* Enable SCRM OPT clocks for PER and CORE dpll */ setbits_le32((*prcm)->cm_wkupaon_scrm_clkctrl, OPTFCLKEN_SCRM_PER_MASK);

On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
All the clocks which has to be enabled has to be done in enable_basic_clocks(), so moving enable sata clock to common clocks enable function.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Tom Rini trini@konsulko.com
arch/arm/cpu/armv7/omap-common/sata.c | 23 ----------------------- arch/arm/cpu/armv7/omap5/hw_data.c | 12 ++++++++++++ 2 files changed, 12 insertions(+), 23 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Compile out sata init/reset apis as this will be implemented in disk-uclass driver to initialize sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com --- arch/arm/cpu/armv7/omap-common/sata.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/arch/arm/cpu/armv7/omap-common/sata.c b/arch/arm/cpu/armv7/omap-common/sata.c index 0c82689..df21ca4 100644 --- a/arch/arm/cpu/armv7/omap-common/sata.c +++ b/arch/arm/cpu/armv7/omap-common/sata.c @@ -32,6 +32,7 @@ struct omap_pipe3 sata_phy = { .dpll_map = dpll_map_sata, };
+#ifndef CONFIG_DISK int init_sata(int dev) { int ret; @@ -68,3 +69,4 @@ void scsi_bus_reset(void) ahci_reset((void __iomem *)DWC_AHSATA_BASE); ahci_init((void __iomem *)DWC_AHSATA_BASE); } +#endif

On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
Compile out sata init/reset apis as this will be implemented in disk-uclass driver to initialize sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com
arch/arm/cpu/armv7/omap-common/sata.c | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com --- drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/drivers/block/disk-uclass.c b/drivers/block/disk-uclass.c index d665b35..4bd7b56 100644 --- a/drivers/block/disk-uclass.c +++ b/drivers/block/disk-uclass.c @@ -7,6 +7,45 @@
#include <common.h> #include <dm.h> +#include <dm/uclass-internal.h> +#include <dm/device-internal.h> +#include <scsi.h> + +int scsi_get_device(int index, struct udevice **devp) +{ + struct udevice *dev; + int ret; + + ret = uclass_find_device(UCLASS_DISK, index, &dev); + if (ret || !dev) { + printf("%d device not found\n", index); + return ret; + } + + ret = device_probe(dev); + if (ret) { + error("device probe error\n"); + return ret; + } + + *devp = dev; + + return ret; +} + +void scsi_init(void) +{ + struct udevice *dev; + int ret; + + ret = scsi_get_device(0, &dev); + if (ret || !dev) { + error("scsi device not found\n"); + return; + } + + scsi_scan(1); +}
UCLASS_DRIVER(disk) = { .id = UCLASS_DISK,

Hi Bin,
On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com
drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
This patch seems odd to me. I would hope that scsi_init() would go away with driver model and it would happen as part of the controller probe. But of course we cannot probe SCSI from UCLASS_DISK. I think the uclass 'disk' is too broad to be useful.
So I am wondering whether the decision to use UCLASS_DISK instead of UCLASS_AHCI was a mistake.
Perhaps instead we should have:
UCLASS_AHCI UCLASS_SCSI UCLASS_MMC etc...
and each of these devices can have a UCLASS_BLK under them (the block device).
Possibly we could even have a dummy UCLASS_DISK device under each of the above, but I'm not sure that is useful.
What do you think?
diff --git a/drivers/block/disk-uclass.c b/drivers/block/disk-uclass.c index d665b35..4bd7b56 100644 --- a/drivers/block/disk-uclass.c +++ b/drivers/block/disk-uclass.c @@ -7,6 +7,45 @@
#include <common.h> #include <dm.h> +#include <dm/uclass-internal.h> +#include <dm/device-internal.h> +#include <scsi.h>
+int scsi_get_device(int index, struct udevice **devp) +{
struct udevice *dev;
int ret;
ret = uclass_find_device(UCLASS_DISK, index, &dev);
if (ret || !dev) {
printf("%d device not found\n", index);
return ret;
}
ret = device_probe(dev);
if (ret) {
error("device probe error\n");
return ret;
}
*devp = dev;
return ret;
+}
+void scsi_init(void) +{
struct udevice *dev;
int ret;
ret = scsi_get_device(0, &dev);
if (ret || !dev) {
error("scsi device not found\n");
return;
}
scsi_scan(1);
+}
UCLASS_DRIVER(disk) = { .id = UCLASS_DISK, -- 2.7.0.75.g3ee1e0f
Regards, Simon

On Sunday 07 February 2016 01:59 AM, Simon Glass wrote:
Hi Bin,
On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com
drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
This patch seems odd to me. I would hope that scsi_init() would go away with driver model and it would happen as part of the controller probe. But of course we cannot probe SCSI from UCLASS_DISK. I think the uclass 'disk' is too broad to be useful.
So I am wondering whether the decision to use UCLASS_DISK instead of UCLASS_AHCI was a mistake.
Perhaps instead we should have:
UCLASS_AHCI UCLASS_SCSI UCLASS_MMC etc...
and each of these devices can have a UCLASS_BLK under them (the block device).
Possibly we could even have a dummy UCLASS_DISK device under each of the above, but I'm not sure that is useful.
What do you think?
Hmmm, this sounds a better approach to me also. So that we can abstract all bulk related activities to UCLASS_BLK. I can do an RFC if you are okay?
Regards Mugunthan V N

Hi Muganthan,
On 8 February 2016 at 04:23, Mugunthan V N mugunthanvnm@ti.com wrote:
On Sunday 07 February 2016 01:59 AM, Simon Glass wrote:
Hi Bin,
On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com
drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
This patch seems odd to me. I would hope that scsi_init() would go away with driver model and it would happen as part of the controller probe. But of course we cannot probe SCSI from UCLASS_DISK. I think the uclass 'disk' is too broad to be useful.
So I am wondering whether the decision to use UCLASS_DISK instead of UCLASS_AHCI was a mistake.
Perhaps instead we should have:
UCLASS_AHCI UCLASS_SCSI UCLASS_MMC etc...
and each of these devices can have a UCLASS_BLK under them (the block device).
Possibly we could even have a dummy UCLASS_DISK device under each of the above, but I'm not sure that is useful.
What do you think?
Hmmm, this sounds a better approach to me also. So that we can abstract all bulk related activities to UCLASS_BLK. I can do an RFC if you are okay?
I have been working on a block uclass and tidying up the partition stuff. I'll see if I can get patches out next week. In the meantime I'd like to get comments from Bin and others about this too.
I suspect we will end up apply what you have (or similar) for this release, but I'd like to figure out the best approach here.
Thinking about it more I suggest this:
UCLASS_AHCI / SCSI / MMC etc. for the controllers each with a UCLASS_DISK child device for each disk attached to the controller each with a (single) UCLASS_BLK child device
Regards Mugunthan V N
Regards, Simon

On Mon, Feb 08, 2016 at 10:45:34AM -0700, Simon Glass wrote:
Hi Muganthan,
On 8 February 2016 at 04:23, Mugunthan V N mugunthanvnm@ti.com wrote:
On Sunday 07 February 2016 01:59 AM, Simon Glass wrote:
Hi Bin,
On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com
drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
This patch seems odd to me. I would hope that scsi_init() would go away with driver model and it would happen as part of the controller probe. But of course we cannot probe SCSI from UCLASS_DISK. I think the uclass 'disk' is too broad to be useful.
So I am wondering whether the decision to use UCLASS_DISK instead of UCLASS_AHCI was a mistake.
Perhaps instead we should have:
UCLASS_AHCI UCLASS_SCSI UCLASS_MMC etc...
and each of these devices can have a UCLASS_BLK under them (the block device).
Possibly we could even have a dummy UCLASS_DISK device under each of the above, but I'm not sure that is useful.
What do you think?
Hmmm, this sounds a better approach to me also. So that we can abstract all bulk related activities to UCLASS_BLK. I can do an RFC if you are okay?
I have been working on a block uclass and tidying up the partition stuff. I'll see if I can get patches out next week. In the meantime I'd like to get comments from Bin and others about this too.
I suspect we will end up apply what you have (or similar) for this release, but I'd like to figure out the best approach here.
OK. I had marked the series deferred this morning, and moved it back to new now. Given the caveat of needing higher level rework after can you review/ack/nack 3 and 5 as much as you feel comfortable with? Thanks!

Hi Simon,
On Sun, Feb 7, 2016 at 4:29 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com
drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
This patch seems odd to me. I would hope that scsi_init() would go away with driver model and it would happen as part of the controller probe. But of course we cannot probe SCSI from UCLASS_DISK. I think the uclass 'disk' is too broad to be useful.
I agree. I raised similar comment before that this just looks a place holder for the SCSI stuff.
So I am wondering whether the decision to use UCLASS_DISK instead of UCLASS_AHCI was a mistake.
Perhaps instead we should have:
UCLASS_AHCI UCLASS_SCSI UCLASS_MMC etc...
I still think UCLASS_AHCI is not a good name. Maybe UCLASS_ATA as we are talking about protocols here (SCSI, MMC).
and each of these devices can have a UCLASS_BLK under them (the block device).
Possibly we could even have a dummy UCLASS_DISK device under each of the above, but I'm not sure that is useful.
What do you think?
[snip]
Regards, Bin

Hi Mugunthan and Bin,
On 14 February 2016 at 20:03, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Feb 7, 2016 at 4:29 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com
drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
This patch seems odd to me. I would hope that scsi_init() would go away with driver model and it would happen as part of the controller probe. But of course we cannot probe SCSI from UCLASS_DISK. I think the uclass 'disk' is too broad to be useful.
I agree. I raised similar comment before that this just looks a place holder for the SCSI stuff.
So I am wondering whether the decision to use UCLASS_DISK instead of UCLASS_AHCI was a mistake.
Perhaps instead we should have:
UCLASS_AHCI UCLASS_SCSI UCLASS_MMC etc...
I still think UCLASS_AHCI is not a good name. Maybe UCLASS_ATA as we are talking about protocols here (SCSI, MMC).
UCLASS_ATA seems confusing. How would we distinguish IDE and SATA?
BTW since your email I have sent a patch to implement block devices.
From what I can tell the above approach will work well. I think our
uclasses should mirror the current IF_TYPE values:
/* Interface types: */ #define IF_TYPE_UNKNOWN 0 #define IF_TYPE_IDE 1 #define IF_TYPE_SCSI 2 #define IF_TYPE_ATAPI 3 #define IF_TYPE_USB 4 #define IF_TYPE_DOC 5 #define IF_TYPE_MMC 6 #define IF_TYPE_SD 7 #define IF_TYPE_SATA 8 #define IF_TYPE_HOST 9
and each of these devices can have a UCLASS_BLK under them (the block device).
Possibly we could even have a dummy UCLASS_DISK device under each of the above, but I'm not sure that is useful.
What do you think?
[snip]
Regards, Bin
Since this patch is presumably destined for the current release and we don't intend to change UCLASS_DISK for that release, I think this patch can go in as is. We can fix it up later. Also note that scsi_get_device() is not the same as scsi_get_dev().
It would be better to use uclass_get_device() though.
Reviewed-by: Simon Glass sjg@chromium.org
Regards, Simon

On Wed, Feb 03, 2016 at 05:29:36PM +0530, Mugunthan V N wrote:
Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Simon Glass sjg@chromium.org
drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/drivers/block/disk-uclass.c b/drivers/block/disk-uclass.c index d665b35..4bd7b56 100644 --- a/drivers/block/disk-uclass.c +++ b/drivers/block/disk-uclass.c @@ -7,6 +7,45 @@
#include <common.h> #include <dm.h> +#include <dm/uclass-internal.h> +#include <dm/device-internal.h> +#include <scsi.h>
+int scsi_get_device(int index, struct udevice **devp) +{
- struct udevice *dev;
- int ret;
- ret = uclass_find_device(UCLASS_DISK, index, &dev);
- if (ret || !dev) {
printf("%d device not found\n", index);
return ret;
- }
- ret = device_probe(dev);
- if (ret) {
error("device probe error\n");
return ret;
- }
- *devp = dev;
- return ret;
+}
+void scsi_init(void) +{
- struct udevice *dev;
- int ret;
- ret = scsi_get_device(0, &dev);
- if (ret || !dev) {
error("scsi device not found\n");
return;
- }
- scsi_scan(1);
+}
UCLASS_DRIVER(disk) = { .id = UCLASS_DISK,
OK, this patch is a problem. Many platforms already define scsi_init() and aren't moved over so now fail to build. Mele_M5 is one of many examples here, thanks!

On Wednesday 24 February 2016 09:50 PM, Tom Rini wrote:
On Wed, Feb 03, 2016 at 05:29:36PM +0530, Mugunthan V N wrote:
Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Simon Glass sjg@chromium.org
drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/drivers/block/disk-uclass.c b/drivers/block/disk-uclass.c index d665b35..4bd7b56 100644 --- a/drivers/block/disk-uclass.c +++ b/drivers/block/disk-uclass.c @@ -7,6 +7,45 @@
#include <common.h> #include <dm.h> +#include <dm/uclass-internal.h> +#include <dm/device-internal.h> +#include <scsi.h>
+int scsi_get_device(int index, struct udevice **devp) +{
- struct udevice *dev;
- int ret;
- ret = uclass_find_device(UCLASS_DISK, index, &dev);
- if (ret || !dev) {
printf("%d device not found\n", index);
return ret;
- }
- ret = device_probe(dev);
- if (ret) {
error("device probe error\n");
return ret;
- }
- *devp = dev;
- return ret;
+}
+void scsi_init(void) +{
- struct udevice *dev;
- int ret;
- ret = scsi_get_device(0, &dev);
- if (ret || !dev) {
error("scsi device not found\n");
return;
- }
- scsi_scan(1);
+}
UCLASS_DRIVER(disk) = { .id = UCLASS_DISK,
OK, this patch is a problem. Many platforms already define scsi_init() and aren't moved over so now fail to build. Mele_M5 is one of many examples here, thanks!
Oops, sorry I didn't run buildman before submitting the patches, will make sure running buildman before submitting patches in future.
Issue is when a platform is converted to DM, by default CONFIG_DISK is selected through Kconfig whether the platform has block device or not, disk_uclass driver is compile which results in build break when the platform has scsi_init already defined and not not converted to DM.
The following diff solves the issue, and CONFIG_DISK has to be selected for platforms which supports disk (sata, ide etc)
Simon, Are you Okay with the patch, so that I can send it as a separate fixup patch.
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 915c1eb..e62bf75 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -1,7 +1,7 @@ config DISK bool "Support disk controllers with driver model" depends on DM - default y if DM + default n if DM help This enables a uclass for disk controllers in U-Boot. Various driver types can use this, such as AHCI/SATA. It does not provide any standard
Regards Mugunthan V N

Hi Mugunthan,
On 25 February 2016 at 02:34, Mugunthan V N mugunthanvnm@ti.com wrote:
On Wednesday 24 February 2016 09:50 PM, Tom Rini wrote:
On Wed, Feb 03, 2016 at 05:29:36PM +0530, Mugunthan V N wrote:
Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Simon Glass sjg@chromium.org
drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/drivers/block/disk-uclass.c b/drivers/block/disk-uclass.c index d665b35..4bd7b56 100644 --- a/drivers/block/disk-uclass.c +++ b/drivers/block/disk-uclass.c @@ -7,6 +7,45 @@
#include <common.h> #include <dm.h> +#include <dm/uclass-internal.h> +#include <dm/device-internal.h> +#include <scsi.h>
+int scsi_get_device(int index, struct udevice **devp) +{
- struct udevice *dev;
- int ret;
- ret = uclass_find_device(UCLASS_DISK, index, &dev);
- if (ret || !dev) {
printf("%d device not found\n", index);
return ret;
- }
- ret = device_probe(dev);
- if (ret) {
error("device probe error\n");
return ret;
- }
- *devp = dev;
- return ret;
+}
+void scsi_init(void) +{
- struct udevice *dev;
- int ret;
- ret = scsi_get_device(0, &dev);
- if (ret || !dev) {
error("scsi device not found\n");
return;
- }
- scsi_scan(1);
+}
UCLASS_DRIVER(disk) = { .id = UCLASS_DISK,
OK, this patch is a problem. Many platforms already define scsi_init() and aren't moved over so now fail to build. Mele_M5 is one of many examples here, thanks!
Oops, sorry I didn't run buildman before submitting the patches, will make sure running buildman before submitting patches in future.
Issue is when a platform is converted to DM, by default CONFIG_DISK is selected through Kconfig whether the platform has block device or not, disk_uclass driver is compile which results in build break when the platform has scsi_init already defined and not not converted to DM.
The following diff solves the issue, and CONFIG_DISK has to be selected for platforms which supports disk (sata, ide etc)
Simon, Are you Okay with the patch, so that I can send it as a separate fixup patch.
It is OK, but please enable CONFIG_DISK in chromebook_link_defconfig. It is the only user at present.
With driver model we actually don't want scsi_init() to be implemented in the end. It's fine for now. The driver-model block-device series will make some changes here down the track. We'll have UCLASS_SATA (or similar) rather than something as generic as UCLASS_DISK.
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 915c1eb..e62bf75 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -1,7 +1,7 @@ config DISK bool "Support disk controllers with driver model" depends on DM
default y if DM
default n if DM help This enables a uclass for disk controllers in U-Boot. Various driver types can use this, such as AHCI/SATA. It does not provide any standard
Regards Mugunthan V N _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Regards, Simon

Hi Tom
On Thursday 25 February 2016 09:00 PM, Simon Glass wrote:
Hi Mugunthan,
On 25 February 2016 at 02:34, Mugunthan V N mugunthanvnm@ti.com wrote:
On Wednesday 24 February 2016 09:50 PM, Tom Rini wrote:
On Wed, Feb 03, 2016 at 05:29:36PM +0530, Mugunthan V N wrote:
Implement scsi_init() api to probe driver model based sata devices.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Simon Glass sjg@chromium.org
drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/drivers/block/disk-uclass.c b/drivers/block/disk-uclass.c index d665b35..4bd7b56 100644 --- a/drivers/block/disk-uclass.c +++ b/drivers/block/disk-uclass.c @@ -7,6 +7,45 @@
#include <common.h> #include <dm.h> +#include <dm/uclass-internal.h> +#include <dm/device-internal.h> +#include <scsi.h>
+int scsi_get_device(int index, struct udevice **devp) +{
- struct udevice *dev;
- int ret;
- ret = uclass_find_device(UCLASS_DISK, index, &dev);
- if (ret || !dev) {
printf("%d device not found\n", index);
return ret;
- }
- ret = device_probe(dev);
- if (ret) {
error("device probe error\n");
return ret;
- }
- *devp = dev;
- return ret;
+}
+void scsi_init(void) +{
- struct udevice *dev;
- int ret;
- ret = scsi_get_device(0, &dev);
- if (ret || !dev) {
error("scsi device not found\n");
return;
- }
- scsi_scan(1);
+}
UCLASS_DRIVER(disk) = { .id = UCLASS_DISK,
OK, this patch is a problem. Many platforms already define scsi_init() and aren't moved over so now fail to build. Mele_M5 is one of many examples here, thanks!
Oops, sorry I didn't run buildman before submitting the patches, will make sure running buildman before submitting patches in future.
Issue is when a platform is converted to DM, by default CONFIG_DISK is selected through Kconfig whether the platform has block device or not, disk_uclass driver is compile which results in build break when the platform has scsi_init already defined and not not converted to DM.
The following diff solves the issue, and CONFIG_DISK has to be selected for platforms which supports disk (sata, ide etc)
Simon, Are you Okay with the patch, so that I can send it as a separate fixup patch.
It is OK, but please enable CONFIG_DISK in chromebook_link_defconfig. It is the only user at present.
With driver model we actually don't want scsi_init() to be implemented in the end. It's fine for now. The driver-model block-device series will make some changes here down the track. We'll have UCLASS_SATA (or similar) rather than something as generic as UCLASS_DISK.
Will send a fixup patch by tomorrow.
Regards Mugunthan V N

Prepare sata driver for DM conversion by abstracting sata phy init to seperate function.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Tom Rini trini@konsulko.com --- arch/arm/cpu/armv7/omap-common/sata.c | 13 +++++++++---- include/sata.h | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/armv7/omap-common/sata.c b/arch/arm/cpu/armv7/omap-common/sata.c index df21ca4..dbdfa1e 100644 --- a/arch/arm/cpu/armv7/omap-common/sata.c +++ b/arch/arm/cpu/armv7/omap-common/sata.c @@ -32,16 +32,21 @@ struct omap_pipe3 sata_phy = { .dpll_map = dpll_map_sata, };
+int enable_sata_phy(void) +{ + sata_phy.power_reg = (void __iomem *)(*ctrl)->control_phy_power_sata; + + /* Power up the PHY */ + return phy_pipe3_power_on(&sata_phy); +} + #ifndef CONFIG_DISK int init_sata(int dev) { int ret; u32 val;
- sata_phy.power_reg = (void __iomem *)(*ctrl)->control_phy_power_sata; - - /* Power up the PHY */ - phy_pipe3_power_on(&sata_phy); + enable_sata_phy();
/* Enable SATA module, No Idle, No Standby */ val = TI_SATA_IDLE_NO | TI_SATA_STANDBY_NO; diff --git a/include/sata.h b/include/sata.h index fa61da8..c2b5360 100644 --- a/include/sata.h +++ b/include/sata.h @@ -16,4 +16,6 @@ int sata_port_status(int dev, int port);
extern block_dev_desc_t sata_dev_desc[];
+int enable_sata_phy(void); + #endif

On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
Prepare sata driver for DM conversion by abstracting sata phy init to seperate function.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Tom Rini trini@konsulko.com
arch/arm/cpu/armv7/omap-common/sata.c | 13 +++++++++---- include/sata.h | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

Implement a sata driver for Synopsys DWC sata device based on U-boot driver model.
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Tom Rini trini@konsulko.com --- drivers/block/Kconfig | 7 +++++ drivers/block/Makefile | 2 ++ drivers/block/dwc_ahci.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+) create mode 100644 drivers/block/dwc_ahci.c
diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig index 990f768..915c1eb 100644 --- a/drivers/block/Kconfig +++ b/drivers/block/Kconfig @@ -7,3 +7,10 @@ config DISK types can use this, such as AHCI/SATA. It does not provide any standard operations at present. The block device interface has not been converted to driver model. + +config DWC_AHCI + bool "Enable Synopsys DWC AHCI driver support" + depends on DISK + help + Enable this driver to support Sata devices through + Synopsys DWC AHCI module. diff --git a/drivers/block/Makefile b/drivers/block/Makefile index 5eb87e0..c8c876d 100644 --- a/drivers/block/Makefile +++ b/drivers/block/Makefile @@ -6,6 +6,8 @@ #
obj-$(CONFIG_DISK) += disk-uclass.o +obj-$(CONFIG_DWC_AHCI) += dwc_ahci.o + obj-$(CONFIG_SCSI_AHCI) += ahci.o obj-$(CONFIG_DWC_AHSATA) += dwc_ahsata.o obj-$(CONFIG_FSL_SATA) += fsl_sata.o diff --git a/drivers/block/dwc_ahci.c b/drivers/block/dwc_ahci.c new file mode 100644 index 0000000..66ab234 --- /dev/null +++ b/drivers/block/dwc_ahci.c @@ -0,0 +1,71 @@ +/* + * DWC SATA platform driver + * + * (C) Copyright 2016 + * Texas Instruments Incorporated, <www.ti.com> + * + * Author: Mugunthan V N mugunthanvnm@ti.com + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <ahci.h> +#include <sata.h> +#include <asm/arch/sata.h> +#include <asm/io.h> + +struct dwc_ahci_priv { + void *base; + void *wrapper_base; +}; + +static int dwc_ahci_ofdata_to_platdata(struct udevice *dev) +{ + struct dwc_ahci_priv *priv = dev_get_priv(dev); + fdt_addr_t addr; + + priv->base = (void *)dev_get_addr(dev); + + addr = dev_get_addr_index(dev, 1); + priv->wrapper_base = (addr != FDT_ADDR_T_NONE) ? (void *)addr : NULL; + + return 0; +} + +static int dwc_ahci_probe(struct udevice *dev) +{ + struct dwc_ahci_priv *priv = dev_get_priv(dev); + int ret; + + if (priv->wrapper_base) { + u32 val = TI_SATA_IDLE_NO | TI_SATA_STANDBY_NO; + + /* Enable SATA module, No Idle, No Standby */ + writel(val, priv->wrapper_base + TI_SATA_SYSCONFIG); + } + + ret = enable_sata_phy(); + if (ret) { + error("Sata phy enable failed\n"); + return ret; + } + + return ahci_init(priv->base); +} + +static const struct udevice_id dwc_ahci_ids[] = { + { .compatible = "snps,dwc-ahci" }, + { } +}; + +U_BOOT_DRIVER(dwc_ahci) = { + .name = "dwc_ahci", + .id = UCLASS_DISK, + .of_match = dwc_ahci_ids, + .ofdata_to_platdata = dwc_ahci_ofdata_to_platdata, + .probe = dwc_ahci_probe, + .priv_auto_alloc_size = sizeof(struct dwc_ahci_priv), + .flags = DM_FLAG_ALLOC_PRIV_DMA, +};

Enable disk driver model for dra74_evm as dwc_ahci supports driver model
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Tom Rini trini@konsulko.com --- configs/dra74_evm_defconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/configs/dra74_evm_defconfig b/configs/dra74_evm_defconfig index 9946261..263527c 100644 --- a/configs/dra74_evm_defconfig +++ b/configs/dra74_evm_defconfig @@ -23,3 +23,5 @@ CONFIG_DM_SPI=y CONFIG_DM_SPI_FLASH=y CONFIG_TIMER=y CONFIG_OMAP_TIMER=y +CONFIG_DISK=y +CONFIG_DWC_AHCI=y

On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
Enable disk driver model for dra74_evm as dwc_ahci supports driver model
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Tom Rini trini@konsulko.com
configs/dra74_evm_defconfig | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

Enable disk driver model for dra72_evm as dwc_ahci supports driver model
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Tom Rini trini@konsulko.com --- configs/dra72_evm_defconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/configs/dra72_evm_defconfig b/configs/dra72_evm_defconfig index 32d1dc1..7ff2a4e 100644 --- a/configs/dra72_evm_defconfig +++ b/configs/dra72_evm_defconfig @@ -24,3 +24,5 @@ CONFIG_DM_SPI=y CONFIG_DM_SPI_FLASH=y CONFIG_TIMER=y CONFIG_OMAP_TIMER=y +CONFIG_DISK=y +CONFIG_DWC_AHCI=y

On 3 February 2016 at 04:59, Mugunthan V N mugunthanvnm@ti.com wrote:
Enable disk driver model for dra72_evm as dwc_ahci supports driver model
Signed-off-by: Mugunthan V N mugunthanvnm@ti.com Reviewed-by: Tom Rini trini@konsulko.com
configs/dra72_evm_defconfig | 2 ++ 1 file changed, 2 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

On Wednesday 03 February 2016 05:29 PM, Mugunthan V N wrote:
This patch series enables sata driver to adopt driver model. This has been tested on the following evms (logs [1]) by loading kernel and dtb from sata hard-disk.
- dra72 evm
- dra74 evm
Also pushed a branch for testing [2]
Forgot to mention the change set. Here are the changes: * Removed sata uclass and adopted to disk uclass.
Regards Mugunthan V N
[1] - http://pastebin.ubuntu.com/14865336/ [2] - git://git.ti.com/~mugunthanvnm/ti-u-boot/mugunth-ti-u-boot.git dm-sata-v2
Mugunthan V N (7): arm: omap: sata: move enable sata clocks to enable_basic_clocks() arm: omap: sata: compile out sata init apis when CONFIG_DISK is defined drivers: block: disk-uclass: implement scsi_init() arm: omap-common: sata: prepare driver for DM conversion drivers: block: dwc_ahci: Implement a driver for Synopsys DWC sata device defconfig: dra74_evm: enable disk driver model defconfig: dra72_evm: enable disk driver model
arch/arm/cpu/armv7/omap-common/sata.c | 38 ++++++------------- arch/arm/cpu/armv7/omap5/hw_data.c | 12 ++++++ configs/dra72_evm_defconfig | 2 + configs/dra74_evm_defconfig | 2 + drivers/block/Kconfig | 7 ++++ drivers/block/Makefile | 2 + drivers/block/disk-uclass.c | 39 +++++++++++++++++++ drivers/block/dwc_ahci.c | 71 +++++++++++++++++++++++++++++++++++ include/sata.h | 2 + 9 files changed, 148 insertions(+), 27 deletions(-) create mode 100644 drivers/block/dwc_ahci.c
participants (4)
-
Bin Meng
-
Mugunthan V N
-
Simon Glass
-
Tom Rini