* [U-Boot] [PATCH] arm: socfpga: fix up a questionable macro for SDMMC
@ 2015-12-01 23:20 dinguyen at opensource.altera.com
2015-12-01 23:30 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: dinguyen at opensource.altera.com @ 2015-12-01 23:20 UTC (permalink / raw)
To: u-boot
From: Dinh Nguyen <dinguyen@opensource.altera.com>
Not sure what made this macro questionable, but edit the macro to be similar
to what is used in Linux.
Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
---
arch/arm/mach-socfpga/include/mach/system_manager.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/mach-socfpga/include/mach/system_manager.h b/arch/arm/mach-socfpga/include/mach/system_manager.h
index 618c92f..c0566b0 100644
--- a/arch/arm/mach-socfpga/include/mach/system_manager.h
+++ b/arch/arm/mach-socfpga/include/mach/system_manager.h
@@ -201,9 +201,8 @@ struct socfpga_system_manager {
#define SYSMGR_FPGAINTF_NAND (1 << 4)
#define SYSMGR_FPGAINTF_SDMMC (1 << 5)
-/* FIXME: This is questionable macro. */
#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
- ((((drvsel) << 0) & 0x7) | (((smplsel) << 3) & 0x38))
+ ((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
/* EMAC Group Bit definitions */
#define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII 0x0
--
2.6.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] arm: socfpga: fix up a questionable macro for SDMMC
2015-12-01 23:20 [U-Boot] [PATCH] arm: socfpga: fix up a questionable macro for SDMMC dinguyen at opensource.altera.com
@ 2015-12-01 23:30 ` Marek Vasut
2015-12-01 23:33 ` Dinh Nguyen
0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2015-12-01 23:30 UTC (permalink / raw)
To: u-boot
On Wednesday, December 02, 2015 at 12:20:47 AM, dinguyen at opensource.altera.com
wrote:
> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>
> Not sure what made this macro questionable, but edit the macro to be
> similar to what is used in Linux.
It should most likely be dissolved and moved into socfpga_dw_mmc.c , since
it's used only once in there. What do you think ?
> Signed-off-by: Dinh Nguyen <dinguyen@opensource.altera.com>
> ---
> arch/arm/mach-socfpga/include/mach/system_manager.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-socfpga/include/mach/system_manager.h
> b/arch/arm/mach-socfpga/include/mach/system_manager.h index
> 618c92f..c0566b0 100644
> --- a/arch/arm/mach-socfpga/include/mach/system_manager.h
> +++ b/arch/arm/mach-socfpga/include/mach/system_manager.h
> @@ -201,9 +201,8 @@ struct socfpga_system_manager {
> #define SYSMGR_FPGAINTF_NAND (1 << 4)
> #define SYSMGR_FPGAINTF_SDMMC (1 << 5)
>
> -/* FIXME: This is questionable macro. */
> #define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
> - ((((drvsel) << 0) & 0x7) | (((smplsel) << 3) & 0x38))
> + ((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
>
> /* EMAC Group Bit definitions */
> #define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII 0x0
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] arm: socfpga: fix up a questionable macro for SDMMC
2015-12-01 23:30 ` Marek Vasut
@ 2015-12-01 23:33 ` Dinh Nguyen
2015-12-02 0:20 ` Marek Vasut
2015-12-03 18:42 ` Pavel Machek
0 siblings, 2 replies; 5+ messages in thread
From: Dinh Nguyen @ 2015-12-01 23:33 UTC (permalink / raw)
To: u-boot
On 12/01/2015 05:30 PM, Marek Vasut wrote:
> On Wednesday, December 02, 2015 at 12:20:47 AM, dinguyen at opensource.altera.com
> wrote:
>> From: Dinh Nguyen <dinguyen@opensource.altera.com>
>>
>> Not sure what made this macro questionable, but edit the macro to be
>> similar to what is used in Linux.
>
> It should most likely be dissolved and moved into socfpga_dw_mmc.c , since
> it's used only once in there. What do you think ?
>
Hmm...if you move this to socfpga_dw_mmc.c then the #if defined(A10)
would have to follow as the bit for smplsel is at shift 4 instead of 3.
So if you had this for in system_manager.h, then socfpga_dw_mmc.c
doesn't have to change.
--- a/arch/arm/mach-socfpga/include/mach/system_manager.h
+++ b/arch/arm/mach-socfpga/include/mach/system_manager.h
@@ -201,8 +201,13 @@ struct socfpga_system_manager {
#define SYSMGR_FPGAINTF_NAND (1 << 4)
#define SYSMGR_FPGAINTF_SDMMC (1 << 5)
+#if defined(SOCFPGA_GEN5)
#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
+#else /* Arria10 */
+#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
+ ((((smplsel) & 0x7) << 4) | (((drvsel) & 0x7) << 0))
+#endif
---
Dinh
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] arm: socfpga: fix up a questionable macro for SDMMC
2015-12-01 23:33 ` Dinh Nguyen
@ 2015-12-02 0:20 ` Marek Vasut
2015-12-03 18:42 ` Pavel Machek
1 sibling, 0 replies; 5+ messages in thread
From: Marek Vasut @ 2015-12-02 0:20 UTC (permalink / raw)
To: u-boot
On Wednesday, December 02, 2015 at 12:33:02 AM, Dinh Nguyen wrote:
> On 12/01/2015 05:30 PM, Marek Vasut wrote:
> > On Wednesday, December 02, 2015 at 12:20:47 AM,
> > dinguyen at opensource.altera.com
> >
> > wrote:
> >> From: Dinh Nguyen <dinguyen@opensource.altera.com>
> >>
> >> Not sure what made this macro questionable, but edit the macro to be
> >> similar to what is used in Linux.
> >
> > It should most likely be dissolved and moved into socfpga_dw_mmc.c ,
> > since it's used only once in there. What do you think ?
>
> Hmm...if you move this to socfpga_dw_mmc.c then the #if defined(A10)
> would have to follow as the bit for smplsel is at shift 4 instead of 3.
>
> So if you had this for in system_manager.h, then socfpga_dw_mmc.c
> doesn't have to change.
>
> --- a/arch/arm/mach-socfpga/include/mach/system_manager.h
> +++ b/arch/arm/mach-socfpga/include/mach/system_manager.h
> @@ -201,8 +201,13 @@ struct socfpga_system_manager {
> #define SYSMGR_FPGAINTF_NAND (1 << 4)
> #define SYSMGR_FPGAINTF_SDMMC (1 << 5)
>
> +#if defined(SOCFPGA_GEN5)
> #define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
> ((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
> +#else /* Arria10 */
> +#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
> + ((((smplsel) & 0x7) << 4) | (((drvsel) & 0x7) << 0))
> +#endif
Then define the offsets in system_manager.h and then just do
writel((smplsel << OFFSET1) | (drvsel << offset2), ...);
in the MMC driver. That would work without ifdeffery, right ?
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] arm: socfpga: fix up a questionable macro for SDMMC
2015-12-01 23:33 ` Dinh Nguyen
2015-12-02 0:20 ` Marek Vasut
@ 2015-12-03 18:42 ` Pavel Machek
1 sibling, 0 replies; 5+ messages in thread
From: Pavel Machek @ 2015-12-03 18:42 UTC (permalink / raw)
To: u-boot
On Tue 2015-12-01 17:33:02, Dinh Nguyen wrote:
> On 12/01/2015 05:30 PM, Marek Vasut wrote:
> > On Wednesday, December 02, 2015 at 12:20:47 AM, dinguyen at opensource.altera.com
> > wrote:
> >> From: Dinh Nguyen <dinguyen@opensource.altera.com>
> >>
> >> Not sure what made this macro questionable, but edit the macro to be
> >> similar to what is used in Linux.
> >
> > It should most likely be dissolved and moved into socfpga_dw_mmc.c , since
> > it's used only once in there. What do you think ?
> >
>
> Hmm...if you move this to socfpga_dw_mmc.c then the #if defined(A10)
> would have to follow as the bit for smplsel is at shift 4 instead of 3.
>
> So if you had this for in system_manager.h, then socfpga_dw_mmc.c
> doesn't have to change.
>
> --- a/arch/arm/mach-socfpga/include/mach/system_manager.h
> +++ b/arch/arm/mach-socfpga/include/mach/system_manager.h
> @@ -201,8 +201,13 @@ struct socfpga_system_manager {
> #define SYSMGR_FPGAINTF_NAND (1 << 4)
> #define SYSMGR_FPGAINTF_SDMMC (1 << 5)
>
> +#if defined(SOCFPGA_GEN5)
> #define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
> ((((smplsel) & 0x7) << 3) | (((drvsel) & 0x7) << 0))
> +#else /* Arria10 */
> +#define SYSMGR_SDMMC_CTRL_SET(smplsel, drvsel) \
> + ((((smplsel) & 0x7) << 4) | (((drvsel) & 0x7) << 0))
> +#endif
Also... can we get a rid of << 0 here?
Is it ever valid to pass values >= 8 to this macro?
Thanks,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-12-03 18:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-12-01 23:20 [U-Boot] [PATCH] arm: socfpga: fix up a questionable macro for SDMMC dinguyen at opensource.altera.com
2015-12-01 23:30 ` Marek Vasut
2015-12-01 23:33 ` Dinh Nguyen
2015-12-02 0:20 ` Marek Vasut
2015-12-03 18:42 ` Pavel Machek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox