| Message ID | 1439963714-2561-1-git-send-email-clsee@altera.com |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
On Wednesday, August 19, 2015 at 07:55:14 AM, Chin Liang See wrote: > Remove hard-coded SDMMC timing parameter drvsel and smplsel. > This setting now will come from SDMMC calibration > > Signed-off-by: Chin Liang See <clsee@altera.com> > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > Cc: Pavel Machek <pavel@denx.de> > Cc: Marek Vasut <marex@denx.de> > Cc: Wolfgang Denk <wd@denx.de> > Cc: Stefan Roese <sr@denx.de> > Cc: Tom Rini <trini@konsulko.com> > --- > include/configs/socfpga_common.h | 2 -- > 1 files changed, 0 insertions(+), 2 deletions(-) > > diff --git a/include/configs/socfpga_common.h > b/include/configs/socfpga_common.h index 5ca45a9..1ca795c 100644 > --- a/include/configs/socfpga_common.h > +++ b/include/configs/socfpga_common.h > @@ -155,8 +155,6 @@ > #define CONFIG_DWMMC > #define CONFIG_SOCFPGA_DWMMC > #define CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH 1024 I believe this is pulled from DT nowaways, so feel free to send a separate patch to remove this from here and README.socfpga. > -#define CONFIG_SOCFPGA_DWMMC_DRVSEL 3 > -#define CONFIG_SOCFPGA_DWMMC_SMPSEL 0 Certainly, I agree, this horribleness should go away. > /* FIXME */ > /* using smaller max blk cnt to avoid flooding the limited stack we have > */ #define CONFIG_SYS_MMC_MAX_BLK_COUNT 256 /* FIXME -- SPL only? */ Now that you're digging in the SD/MMC stuff -- is this still relevant at all? I don't think so, so you should be able to remove this as well (again, in a separate patch and with a sufficient amount of testing). Best regards, Marek Vasut
Hi, On Wed, 2015-08-19 at 09:37 +0200, marex@denx.de wrote: > On Wednesday, August 19, 2015 at 07:55:14 AM, Chin Liang See wrote: > > Remove hard-coded SDMMC timing parameter drvsel and smplsel. > > This setting now will come from SDMMC calibration > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > Cc: Pavel Machek <pavel@denx.de> > > Cc: Marek Vasut <marex@denx.de> > > Cc: Wolfgang Denk <wd@denx.de> > > Cc: Stefan Roese <sr@denx.de> > > Cc: Tom Rini <trini@konsulko.com> > > --- > > include/configs/socfpga_common.h | 2 -- > > 1 files changed, 0 insertions(+), 2 deletions(-) > > > > diff --git a/include/configs/socfpga_common.h > > b/include/configs/socfpga_common.h index 5ca45a9..1ca795c 100644 > > --- a/include/configs/socfpga_common.h > > +++ b/include/configs/socfpga_common.h > > @@ -155,8 +155,6 @@ > > #define CONFIG_DWMMC > > #define CONFIG_SOCFPGA_DWMMC > > #define CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH 1024 > > I believe this is pulled from DT nowaways, so feel free to send a separate > patch to remove this from here and README.socfpga. > > > -#define CONFIG_SOCFPGA_DWMMC_DRVSEL 3 > > -#define CONFIG_SOCFPGA_DWMMC_SMPSEL 0 > > Certainly, I agree, this horribleness should go away. > > > /* FIXME */ > > /* using smaller max blk cnt to avoid flooding the limited stack we have > > */ #define CONFIG_SYS_MMC_MAX_BLK_COUNT 256 /* FIXME -- SPL only? */ > > Now that you're digging in the SD/MMC stuff -- is this still relevant at all? > I don't think so, so you should be able to remove this as well (again, in a > separate patch and with a sufficient amount of testing). > Ok, let me take a look into this. Thanks Chin Liang > Best regards, > Marek Vasut
On Wednesday, August 19, 2015 at 10:22:21 AM, Chin Liang See wrote: > Hi, Hi, > On Wed, 2015-08-19 at 09:37 +0200, marex@denx.de wrote: > > On Wednesday, August 19, 2015 at 07:55:14 AM, Chin Liang See wrote: > > > Remove hard-coded SDMMC timing parameter drvsel and smplsel. > > > This setting now will come from SDMMC calibration > > > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > Cc: Pavel Machek <pavel@denx.de> > > > Cc: Marek Vasut <marex@denx.de> > > > Cc: Wolfgang Denk <wd@denx.de> > > > Cc: Stefan Roese <sr@denx.de> > > > Cc: Tom Rini <trini@konsulko.com> > > > --- > > > > > > include/configs/socfpga_common.h | 2 -- > > > 1 files changed, 0 insertions(+), 2 deletions(-) > > > > > > diff --git a/include/configs/socfpga_common.h > > > b/include/configs/socfpga_common.h index 5ca45a9..1ca795c 100644 > > > --- a/include/configs/socfpga_common.h > > > +++ b/include/configs/socfpga_common.h > > > @@ -155,8 +155,6 @@ > > > > > > #define CONFIG_DWMMC > > > #define CONFIG_SOCFPGA_DWMMC > > > #define CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH 1024 > > > > I believe this is pulled from DT nowaways, so feel free to send a > > separate patch to remove this from here and README.socfpga. > > > > > -#define CONFIG_SOCFPGA_DWMMC_DRVSEL 3 > > > -#define CONFIG_SOCFPGA_DWMMC_SMPSEL 0 > > > > Certainly, I agree, this horribleness should go away. > > > > > /* FIXME */ > > > /* using smaller max blk cnt to avoid flooding the limited stack we > > > have > > > > > > */ #define CONFIG_SYS_MMC_MAX_BLK_COUNT 256 /* FIXME -- SPL only? */ > > > > Now that you're digging in the SD/MMC stuff -- is this still relevant at > > all? I don't think so, so you should be able to remove this as well > > (again, in a separate patch and with a sufficient amount of testing). > > Ok, let me take a look into this. > Thanks Cool, thanks! Best regards, Marek Vasut
Hi Marek, On Wed, 2015-08-19 at 03:22 -0500, Chin Liang See wrote: > Hi, > > On Wed, 2015-08-19 at 09:37 +0200, marex@denx.de wrote: > > On Wednesday, August 19, 2015 at 07:55:14 AM, Chin Liang See wrote: > > > Remove hard-coded SDMMC timing parameter drvsel and smplsel. > > > This setting now will come from SDMMC calibration > > > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > Cc: Pavel Machek <pavel@denx.de> > > > Cc: Marek Vasut <marex@denx.de> > > > Cc: Wolfgang Denk <wd@denx.de> > > > Cc: Stefan Roese <sr@denx.de> > > > Cc: Tom Rini <trini@konsulko.com> > > > --- > > > include/configs/socfpga_common.h | 2 -- > > > 1 files changed, 0 insertions(+), 2 deletions(-) > > > > > > diff --git a/include/configs/socfpga_common.h > > > b/include/configs/socfpga_common.h index 5ca45a9..1ca795c 100644 > > > --- a/include/configs/socfpga_common.h > > > +++ b/include/configs/socfpga_common.h > > > @@ -155,8 +155,6 @@ > > > #define CONFIG_DWMMC > > > #define CONFIG_SOCFPGA_DWMMC > > > #define CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH 1024 > > > > I believe this is pulled from DT nowaways, so feel free to send a separate > > patch to remove this from here and README.socfpga. > > > > > -#define CONFIG_SOCFPGA_DWMMC_DRVSEL 3 > > > -#define CONFIG_SOCFPGA_DWMMC_SMPSEL 0 > > > > Certainly, I agree, this horribleness should go away. > > > > > /* FIXME */ > > > /* using smaller max blk cnt to avoid flooding the limited stack we have > > > */ #define CONFIG_SYS_MMC_MAX_BLK_COUNT 256 /* FIXME -- SPL only? */ > > > > Now that you're digging in the SD/MMC stuff -- is this still relevant at all? > > I don't think so, so you should be able to remove this as well (again, in a > > separate patch and with a sufficient amount of testing). > > > > Ok, let me take a look into this. I further checked and we still need this. But I believe all these info can be moved to dts. Let mark this as future enhancement. Thanks Chin Liang > Thanks > > Chin Liang > > > Best regards, > > Marek Vasut >
On Thursday, August 20, 2015 at 07:15:25 AM, Chin Liang See wrote: > Hi Marek, Hi, > On Wed, 2015-08-19 at 03:22 -0500, Chin Liang See wrote: > > Hi, > > > > On Wed, 2015-08-19 at 09:37 +0200, marex@denx.de wrote: > > > On Wednesday, August 19, 2015 at 07:55:14 AM, Chin Liang See wrote: > > > > Remove hard-coded SDMMC timing parameter drvsel and smplsel. > > > > This setting now will come from SDMMC calibration > > > > > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > Cc: Pavel Machek <pavel@denx.de> > > > > Cc: Marek Vasut <marex@denx.de> > > > > Cc: Wolfgang Denk <wd@denx.de> > > > > Cc: Stefan Roese <sr@denx.de> > > > > Cc: Tom Rini <trini@konsulko.com> > > > > --- > > > > > > > > include/configs/socfpga_common.h | 2 -- > > > > 1 files changed, 0 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/include/configs/socfpga_common.h > > > > b/include/configs/socfpga_common.h index 5ca45a9..1ca795c 100644 > > > > --- a/include/configs/socfpga_common.h > > > > +++ b/include/configs/socfpga_common.h > > > > @@ -155,8 +155,6 @@ > > > > > > > > #define CONFIG_DWMMC > > > > #define CONFIG_SOCFPGA_DWMMC > > > > #define CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH 1024 > > > > > > I believe this is pulled from DT nowaways, so feel free to send a > > > separate patch to remove this from here and README.socfpga. > > > > > > > -#define CONFIG_SOCFPGA_DWMMC_DRVSEL 3 > > > > -#define CONFIG_SOCFPGA_DWMMC_SMPSEL 0 > > > > > > Certainly, I agree, this horribleness should go away. > > > > > > > /* FIXME */ > > > > /* using smaller max blk cnt to avoid flooding the limited stack we > > > > have > > > > > > > > */ #define CONFIG_SYS_MMC_MAX_BLK_COUNT 256 /* FIXME -- SPL only? */ > > > > > > Now that you're digging in the SD/MMC stuff -- is this still relevant > > > at all? I don't think so, so you should be able to remove this as well > > > (again, in a separate patch and with a sufficient amount of testing). > > > > Ok, let me take a look into this. > > I further checked and we still need this. Why ? > But I believe all these info can be moved to dts. Is this a property (limitation) of the hardware ? > Let mark this as future enhancement. This is a current FIXME ;-) [...] Best regards, Marek Vasut
On Thu, 2015-08-20 at 07:27 +0200, marex@denx.de wrote: > On Thursday, August 20, 2015 at 07:15:25 AM, Chin Liang See wrote: > > Hi Marek, > > Hi, > > > On Wed, 2015-08-19 at 03:22 -0500, Chin Liang See wrote: > > > Hi, > > > > > > On Wed, 2015-08-19 at 09:37 +0200, marex@denx.de wrote: > > > > On Wednesday, August 19, 2015 at 07:55:14 AM, Chin Liang See wrote: > > > > > Remove hard-coded SDMMC timing parameter drvsel and smplsel. > > > > > This setting now will come from SDMMC calibration > > > > > > > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > > Cc: Pavel Machek <pavel@denx.de> > > > > > Cc: Marek Vasut <marex@denx.de> > > > > > Cc: Wolfgang Denk <wd@denx.de> > > > > > Cc: Stefan Roese <sr@denx.de> > > > > > Cc: Tom Rini <trini@konsulko.com> > > > > > --- > > > > > > > > > > include/configs/socfpga_common.h | 2 -- > > > > > 1 files changed, 0 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/include/configs/socfpga_common.h > > > > > b/include/configs/socfpga_common.h index 5ca45a9..1ca795c 100644 > > > > > --- a/include/configs/socfpga_common.h > > > > > +++ b/include/configs/socfpga_common.h > > > > > @@ -155,8 +155,6 @@ > > > > > > > > > > #define CONFIG_DWMMC > > > > > #define CONFIG_SOCFPGA_DWMMC > > > > > #define CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH 1024 > > > > > > > > I believe this is pulled from DT nowaways, so feel free to send a > > > > separate patch to remove this from here and README.socfpga. > > > > > > > > > -#define CONFIG_SOCFPGA_DWMMC_DRVSEL 3 > > > > > -#define CONFIG_SOCFPGA_DWMMC_SMPSEL 0 > > > > > > > > Certainly, I agree, this horribleness should go away. > > > > > > > > > /* FIXME */ > > > > > /* using smaller max blk cnt to avoid flooding the limited stack we > > > > > have > > > > > > > > > > */ #define CONFIG_SYS_MMC_MAX_BLK_COUNT 256 /* FIXME -- SPL > only? */ > > > > > > > > Now that you're digging in the SD/MMC stuff -- is this still relevant > > > > at all? I don't think so, so you should be able to remove this as well > > > > (again, in a separate patch and with a sufficient amount of testing). > > > > > > Ok, let me take a look into this. > > > > I further checked and we still need this. > > Why ? > > But I believe all these info can be moved to dts. > > Is this a property (limitation) of the hardware ? > > > Let mark this as future enhancement. > > This is a current FIXME ;-) > Actually I tested after remove this. It failed with fatload a 10MB file. I need to troubleshoot to understand further on this. Hope this FIXME won't be long :) haha Chin Liang > [...] > > Best regards, > Marek Vasut
On Thursday, August 20, 2015 at 08:58:23 AM, Chin Liang See wrote: > On Thu, 2015-08-20 at 07:27 +0200, marex@denx.de wrote: > > On Thursday, August 20, 2015 at 07:15:25 AM, Chin Liang See wrote: > > > Hi Marek, > > > > Hi, > > > > > On Wed, 2015-08-19 at 03:22 -0500, Chin Liang See wrote: > > > > Hi, > > > > > > > > On Wed, 2015-08-19 at 09:37 +0200, marex@denx.de wrote: > > > > > On Wednesday, August 19, 2015 at 07:55:14 AM, Chin Liang See wrote: > > > > > > Remove hard-coded SDMMC timing parameter drvsel and smplsel. > > > > > > This setting now will come from SDMMC calibration > > > > > > > > > > > > Signed-off-by: Chin Liang See <clsee@altera.com> > > > > > > Cc: Dinh Nguyen <dinguyen@opensource.altera.com> > > > > > > Cc: Pavel Machek <pavel@denx.de> > > > > > > Cc: Marek Vasut <marex@denx.de> > > > > > > Cc: Wolfgang Denk <wd@denx.de> > > > > > > Cc: Stefan Roese <sr@denx.de> > > > > > > Cc: Tom Rini <trini@konsulko.com> > > > > > > --- > > > > > > > > > > > > include/configs/socfpga_common.h | 2 -- > > > > > > 1 files changed, 0 insertions(+), 2 deletions(-) > > > > > > > > > > > > diff --git a/include/configs/socfpga_common.h > > > > > > b/include/configs/socfpga_common.h index 5ca45a9..1ca795c 100644 > > > > > > --- a/include/configs/socfpga_common.h > > > > > > +++ b/include/configs/socfpga_common.h > > > > > > @@ -155,8 +155,6 @@ > > > > > > > > > > > > #define CONFIG_DWMMC > > > > > > #define CONFIG_SOCFPGA_DWMMC > > > > > > #define CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH 1024 > > > > > > > > > > I believe this is pulled from DT nowaways, so feel free to send a > > > > > separate patch to remove this from here and README.socfpga. > > > > > > > > > > > -#define CONFIG_SOCFPGA_DWMMC_DRVSEL 3 > > > > > > -#define CONFIG_SOCFPGA_DWMMC_SMPSEL 0 > > > > > > > > > > Certainly, I agree, this horribleness should go away. > > > > > > > > > > > /* FIXME */ > > > > > > /* using smaller max blk cnt to avoid flooding the limited stack > > > > > > we have > > > > > > > > > > > > */ #define CONFIG_SYS_MMC_MAX_BLK_COUNT 256 /* FIXME -- SPL > > > > only? */ > > > > > > > Now that you're digging in the SD/MMC stuff -- is this still > > > > > relevant at all? I don't think so, so you should be able to remove > > > > > this as well (again, in a separate patch and with a sufficient > > > > > amount of testing). > > > > > > > > Ok, let me take a look into this. > > > > > > I further checked and we still need this. > > > > Why ? > > > > > But I believe all these info can be moved to dts. > > > > Is this a property (limitation) of the hardware ? > > > > > Let mark this as future enhancement. > > > > This is a current FIXME ;-) > > Actually I tested after remove this. It failed with fatload a 10MB file. > I need to troubleshoot to understand further on this. Hope this FIXME > won't be long :) haha I see. I'd like to know what the problem is, whether this is silicon limitation, stack limitation (shouldn't be the case) or something else.
diff --git a/include/configs/socfpga_common.h b/include/configs/socfpga_common.h index 5ca45a9..1ca795c 100644 --- a/include/configs/socfpga_common.h +++ b/include/configs/socfpga_common.h @@ -155,8 +155,6 @@ #define CONFIG_DWMMC #define CONFIG_SOCFPGA_DWMMC #define CONFIG_SOCFPGA_DWMMC_FIFO_DEPTH 1024 -#define CONFIG_SOCFPGA_DWMMC_DRVSEL 3 -#define CONFIG_SOCFPGA_DWMMC_SMPSEL 0 /* FIXME */ /* using smaller max blk cnt to avoid flooding the limited stack we have */ #define CONFIG_SYS_MMC_MAX_BLK_COUNT 256 /* FIXME -- SPL only? */
Remove hard-coded SDMMC timing parameter drvsel and smplsel. This setting now will come from SDMMC calibration Signed-off-by: Chin Liang See <clsee@altera.com> Cc: Dinh Nguyen <dinguyen@opensource.altera.com> Cc: Pavel Machek <pavel@denx.de> Cc: Marek Vasut <marex@denx.de> Cc: Wolfgang Denk <wd@denx.de> Cc: Stefan Roese <sr@denx.de> Cc: Tom Rini <trini@konsulko.com> --- include/configs/socfpga_common.h | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)