Re: [U-Boot] [PATCH] dwmmc: make driver usable for non-exynos platforms
Hi Alexey, On 11/29/2013 04:41 PM, Alexey Brodkin wrote: Hi Jaehoon, On Fri, 2013-11-29 at 12:46 +0900, Jaehoon Chung wrote: Hi, Alexey, I didn't know exactly which version is used this register. I known this register is used only the exynos series. I have checked the IP version 2.70a, but CLKSEL(0x9c) wasn't used at general register map. If I missed the something, let me know.(It's published the upper IP version than 2.70a) The main point of my patch was to remove #include asm/arch/dwmmc.h from dwmmc.c. Oh, Sorry. I didn't recognize this point. I think it's pretty clear that if any particular device driver is implemented not in your board folder but in common drivers folder it is sort of expected to be independent from architecture/platform, right? Right. This point is correct. If other soc is used the dw-mmc controller, then this header file can't refer. And for simplicity I copy-pasted all defines from arch/arm/include/asm/arch-exynos/dwmmc.h to common include/dwmmc.h. I think good that just removed the #include asm/arch/dwmmc.h. Instead, Exnyos specific code can move into dw-mmc-exynos.c with callback function. (I didn't know whcih Soc is used with MPSCTRL register.) Which was a bit too recklessly from my side - I should have spend so more time and come up with more accurate solution which I will do now with v2 re-spin. As for DWMCI_CLKSEL. I checked the same version of databook for DesignWare Mobile Storage and do see there a register with offset = 0x9c. This is: DSCADDRU (For 64-bit Address Configuration Only) Name: Current Host Descriptor Address Upper Register I may understand that your current DW MMC is configured to use 32-bit addresses, right? Right, i used 32-bit address. Because of this you selected this offset 0x9c and experience no problems. But at some point you may get HW configured to use 64-bit addressing and you'll need to use another offset for Exynos-specific things. I understand that as SW engineer you have not much things to do here but would be good if you communicate this to HW engineers. If other soc is used 0x9c offset as other register? 0x9c is used the CLKSEL at exynos. we need to consider other soc. Then it's good that exynos specific code will move into dw-mmc-exynos.c. isn't? And indeed this one could be left untouched in asm/arch-exynos/dwmmc.h. Also I may see following defines aren't used anywhere in existing (today's master branch) U-Boot sources: #define DWMCI_SHIFT_0 0x0 #define DWMCI_SHIFT_1 0x1 #define DWMCI_SHIFT_2 0x2 #define DWMCI_SHIFT_3 0x3 So these might be dumped at all. This shift define is exynos specific define. But i will think this need or not Best Regards, Jaehoon Chung Regards, Alexey ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dwmmc: make driver usable for non-exynos platforms
On Fri, 2013-11-29 at 18:57 +0900, Jaehoon Chung wrote: I think good that just removed the #include asm/arch/dwmmc.h. Instead, Exnyos specific code can move into dw-mmc-exynos.c with callback function. (I didn't know whcih Soc is used with MPSCTRL register.) Because of this you selected this offset 0x9c and experience no problems. But at some point you may get HW configured to use 64-bit addressing and you'll need to use another offset for Exynos-specific things. I understand that as SW engineer you have not much things to do here but would be good if you communicate this to HW engineers. If other soc is used 0x9c offset as other register? 0x9c is used the CLKSEL at exynos. we need to consider other soc. Then it's good that exynos specific code will move into dw-mmc-exynos.c. isn't? Ok, so please look at my re-spin I already sent (you were on CC-list). There I only mask problematic bits of code - so driver works again for me (I'm on other architecture BTW - not ARM). But if you're willing to clean-up dwmmc code from Exynos-specific parts I'll be more than happy. Well it doesn't stop us from application of my v2 that will be a temporary fix until clean-up. -Alexey ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dwmmc: make driver usable for non-exynos platforms
On 11/29/2013 07:02 PM, Alexey Brodkin wrote: On Fri, 2013-11-29 at 18:57 +0900, Jaehoon Chung wrote: I think good that just removed the #include asm/arch/dwmmc.h. Instead, Exnyos specific code can move into dw-mmc-exynos.c with callback function. (I didn't know whcih Soc is used with MPSCTRL register.) Because of this you selected this offset 0x9c and experience no problems. But at some point you may get HW configured to use 64-bit addressing and you'll need to use another offset for Exynos-specific things. I understand that as SW engineer you have not much things to do here but would be good if you communicate this to HW engineers. If other soc is used 0x9c offset as other register? 0x9c is used the CLKSEL at exynos. we need to consider other soc. Then it's good that exynos specific code will move into dw-mmc-exynos.c. isn't? Ok, so please look at my re-spin I already sent (you were on CC-list). There I only mask problematic bits of code - so driver works again for me (I'm on other architecture BTW - not ARM). But if you're willing to clean-up dwmmc code from Exynos-specific parts I'll be more than happy. I will work to clean-up dwmmc code from exynos specific code. And will send patch. Thanks for reporting this point. Best Regards, Jaehoon Chung Well it doesn't stop us from application of my v2 that will be a temporary fix until clean-up. -Alexey ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] dwmmc: make driver usable for non-exynos platforms
There's no point in having of dwmmc.h per architecure - we're talking about device driver (DesignWare MMC controller) which is expected to work with any CPU it is attached to. So defines used for DW MMC configuration should be in its own header which is avaialble for everybody. Signed-off-by: Alexey Brodkin abrod...@synopsys.com Cc: Mischa Jonker mjon...@synopsys.com Cc: Alexey Brodkin abrod...@synopsys.com Cc: Jaehoon Chung jh80.ch...@samsung.com Cc: Andy Fleming aflem...@gmail.com Cc: Alim Akhtar alim.akh...@samsung.com Cc: Rajeshwari Shinde rajeshwar...@samsung.com Cc: Simon Glass s...@chromium.org Cc: Pantelis Antoniou pa...@antoniou-consulting.com --- arch/arm/include/asm/arch-exynos/dwmmc.h | 22 -- drivers/mmc/dw_mmc.c | 1 - include/dwmmc.h | 22 ++ 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h b/arch/arm/include/asm/arch-exynos/dwmmc.h index d1c5d4f..da66e25 100644 --- a/arch/arm/include/asm/arch-exynos/dwmmc.h +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h @@ -5,28 +5,6 @@ * SPDX-License-Identifier:GPL-2.0+ */ -#define DWMCI_CLKSEL 0x09C -#define DWMCI_SHIFT_0 0x0 -#define DWMCI_SHIFT_1 0x1 -#define DWMCI_SHIFT_2 0x2 -#define DWMCI_SHIFT_3 0x3 -#define DWMCI_SET_SAMPLE_CLK(x)(x) -#define DWMCI_SET_DRV_CLK(x) ((x) 16) -#define DWMCI_SET_DIV_RATIO(x) ((x) 24) - -#define EMMCP_MPSBEGIN00x1200 -#define EMMCP_SEND00x1204 -#define EMMCP_CTRL00x120C - -#define MPSCTRL_SECURE_READ_BIT(0x17) -#define MPSCTRL_SECURE_WRITE_BIT (0x16) -#define MPSCTRL_NON_SECURE_READ_BIT(0x15) -#define MPSCTRL_NON_SECURE_WRITE_BIT (0x14) -#define MPSCTRL_USE_FUSE_KEY (0x13) -#define MPSCTRL_ECB_MODE (0x12) -#define MPSCTRL_ENCRYPTION (0x11) -#define MPSCTRL_VALID (0x10) - #ifdef CONFIG_OF_CONTROL int exynos_dwmmc_init(const void *blob); #endif diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 1e0f72b..2638c3f 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -11,7 +11,6 @@ #include mmc.h #include dwmmc.h #include asm-generic/errno.h -#include asm/arch/dwmmc.h #define PAGE_SIZE 4096 diff --git a/include/dwmmc.h b/include/dwmmc.h index 6c91143..8ea1092 100644 --- a/include/dwmmc.h +++ b/include/dwmmc.h @@ -123,6 +123,28 @@ #define DWMCI_BMOD_IDMAC_FB(1 1) #define DWMCI_BMOD_IDMAC_EN(1 7) +#define DWMCI_CLKSEL 0x09C +#define DWMCI_SHIFT_0 0x0 +#define DWMCI_SHIFT_1 0x1 +#define DWMCI_SHIFT_2 0x2 +#define DWMCI_SHIFT_3 0x3 +#define DWMCI_SET_SAMPLE_CLK(x)(x) +#define DWMCI_SET_DRV_CLK(x) ((x) 16) +#define DWMCI_SET_DIV_RATIO(x) ((x) 24) + +#define EMMCP_MPSBEGIN00x1200 +#define EMMCP_SEND00x1204 +#define EMMCP_CTRL00x120C + +#define MPSCTRL_SECURE_READ_BIT(0x17) +#define MPSCTRL_SECURE_WRITE_BIT (0x16) +#define MPSCTRL_NON_SECURE_READ_BIT(0x15) +#define MPSCTRL_NON_SECURE_WRITE_BIT (0x14) +#define MPSCTRL_USE_FUSE_KEY (0x13) +#define MPSCTRL_ECB_MODE (0x12) +#define MPSCTRL_ENCRYPTION (0x11) +#define MPSCTRL_VALID (0x10) + /* quirks */ #define DWMCI_QUIRK_DISABLE_SMU(1 0) -- 1.8.4.2 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dwmmc: make driver usable for non-exynos platforms
Hi, Alexey, I didn't know exactly which version is used this register. I known this register is used only the exynos series. I have checked the IP version 2.70a, but CLKSEL(0x9c) wasn't used at general register map. If I missed the something, let me know.(It's published the upper IP version than 2.70a) Best Regards, Jaehoon Chung On 11/28/2013 10:52 PM, Alexey Brodkin wrote: There's no point in having of dwmmc.h per architecure - we're talking about device driver (DesignWare MMC controller) which is expected to work with any CPU it is attached to. So defines used for DW MMC configuration should be in its own header which is avaialble for everybody. Signed-off-by: Alexey Brodkin abrod...@synopsys.com Cc: Mischa Jonker mjon...@synopsys.com Cc: Alexey Brodkin abrod...@synopsys.com Cc: Jaehoon Chung jh80.ch...@samsung.com Cc: Andy Fleming aflem...@gmail.com Cc: Alim Akhtar alim.akh...@samsung.com Cc: Rajeshwari Shinde rajeshwar...@samsung.com Cc: Simon Glass s...@chromium.org Cc: Pantelis Antoniou pa...@antoniou-consulting.com --- arch/arm/include/asm/arch-exynos/dwmmc.h | 22 -- drivers/mmc/dw_mmc.c | 1 - include/dwmmc.h | 22 ++ 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/arch/arm/include/asm/arch-exynos/dwmmc.h b/arch/arm/include/asm/arch-exynos/dwmmc.h index d1c5d4f..da66e25 100644 --- a/arch/arm/include/asm/arch-exynos/dwmmc.h +++ b/arch/arm/include/asm/arch-exynos/dwmmc.h @@ -5,28 +5,6 @@ * SPDX-License-Identifier: GPL-2.0+ */ -#define DWMCI_CLKSEL 0x09C -#define DWMCI_SHIFT_00x0 -#define DWMCI_SHIFT_10x1 -#define DWMCI_SHIFT_20x2 -#define DWMCI_SHIFT_30x3 -#define DWMCI_SET_SAMPLE_CLK(x) (x) -#define DWMCI_SET_DRV_CLK(x) ((x) 16) -#define DWMCI_SET_DIV_RATIO(x) ((x) 24) - -#define EMMCP_MPSBEGIN0 0x1200 -#define EMMCP_SEND0 0x1204 -#define EMMCP_CTRL0 0x120C - -#define MPSCTRL_SECURE_READ_BIT (0x17) -#define MPSCTRL_SECURE_WRITE_BIT (0x16) -#define MPSCTRL_NON_SECURE_READ_BIT (0x15) -#define MPSCTRL_NON_SECURE_WRITE_BIT (0x14) -#define MPSCTRL_USE_FUSE_KEY (0x13) -#define MPSCTRL_ECB_MODE (0x12) -#define MPSCTRL_ENCRYPTION (0x11) -#define MPSCTRL_VALID(0x10) - #ifdef CONFIG_OF_CONTROL int exynos_dwmmc_init(const void *blob); #endif diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 1e0f72b..2638c3f 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -11,7 +11,6 @@ #include mmc.h #include dwmmc.h #include asm-generic/errno.h -#include asm/arch/dwmmc.h #define PAGE_SIZE 4096 diff --git a/include/dwmmc.h b/include/dwmmc.h index 6c91143..8ea1092 100644 --- a/include/dwmmc.h +++ b/include/dwmmc.h @@ -123,6 +123,28 @@ #define DWMCI_BMOD_IDMAC_FB (1 1) #define DWMCI_BMOD_IDMAC_EN (1 7) +#define DWMCI_CLKSEL 0x09C +#define DWMCI_SHIFT_00x0 +#define DWMCI_SHIFT_10x1 +#define DWMCI_SHIFT_20x2 +#define DWMCI_SHIFT_30x3 +#define DWMCI_SET_SAMPLE_CLK(x) (x) +#define DWMCI_SET_DRV_CLK(x) ((x) 16) +#define DWMCI_SET_DIV_RATIO(x) ((x) 24) + +#define EMMCP_MPSBEGIN0 0x1200 +#define EMMCP_SEND0 0x1204 +#define EMMCP_CTRL0 0x120C + +#define MPSCTRL_SECURE_READ_BIT (0x17) +#define MPSCTRL_SECURE_WRITE_BIT (0x16) +#define MPSCTRL_NON_SECURE_READ_BIT (0x15) +#define MPSCTRL_NON_SECURE_WRITE_BIT (0x14) +#define MPSCTRL_USE_FUSE_KEY (0x13) +#define MPSCTRL_ECB_MODE (0x12) +#define MPSCTRL_ENCRYPTION (0x11) +#define MPSCTRL_VALID(0x10) + /* quirks */ #define DWMCI_QUIRK_DISABLE_SMU (1 0) ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] dwmmc: make driver usable for non-exynos platforms
Hi Jaehoon, On Fri, 2013-11-29 at 12:46 +0900, Jaehoon Chung wrote: Hi, Alexey, I didn't know exactly which version is used this register. I known this register is used only the exynos series. I have checked the IP version 2.70a, but CLKSEL(0x9c) wasn't used at general register map. If I missed the something, let me know.(It's published the upper IP version than 2.70a) The main point of my patch was to remove #include asm/arch/dwmmc.h from dwmmc.c. I think it's pretty clear that if any particular device driver is implemented not in your board folder but in common drivers folder it is sort of expected to be independent from architecture/platform, right? And for simplicity I copy-pasted all defines from arch/arm/include/asm/arch-exynos/dwmmc.h to common include/dwmmc.h. Which was a bit too recklessly from my side - I should have spend so more time and come up with more accurate solution which I will do now with v2 re-spin. As for DWMCI_CLKSEL. I checked the same version of databook for DesignWare Mobile Storage and do see there a register with offset = 0x9c. This is: DSCADDRU (For 64-bit Address Configuration Only) Name: Current Host Descriptor Address Upper Register I may understand that your current DW MMC is configured to use 32-bit addresses, right? Because of this you selected this offset 0x9c and experience no problems. But at some point you may get HW configured to use 64-bit addressing and you'll need to use another offset for Exynos-specific things. I understand that as SW engineer you have not much things to do here but would be good if you communicate this to HW engineers. And indeed this one could be left untouched in asm/arch-exynos/dwmmc.h. Also I may see following defines aren't used anywhere in existing (today's master branch) U-Boot sources: #define DWMCI_SHIFT_0 0x0 #define DWMCI_SHIFT_1 0x1 #define DWMCI_SHIFT_2 0x2 #define DWMCI_SHIFT_3 0x3 So these might be dumped at all. Regards, Alexey ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
