On 18 October 2015 at 20:22, Bin Meng bmeng.cn@gmail.com wrote:

On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass sjg@chromium.org wrote:

...

Add a comment to make it clear to which block the #endif relates.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/rtc/mc146818.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/mc146818.c b/drivers/rtc/mc146818.c index 363ade3..9e94a80 100644 --- a/drivers/rtc/mc146818.c +++ b/drivers/rtc/mc146818.c @@ -192,7 +192,7 @@ static void mc146818_init(void) /* Clear any pending interrupts */ mc146818_read8(RTC_CONFIG_C); } -#endif +#endif /* CONFIG_CMD_DATE */

#ifdef CONFIG_DM_RTC

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Applied to u-boot-x86.

Hi Simon,

On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass sjg@chromium.org wrote:

At present this driver uses bind() to set up the device. The bind() method should not touch the hardware, so move the init code to probe().

I think RTC should be initialized anyway. If moving it to probe, it may not be initialized by U-Boot before jumping to kernel.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/rtc/mc146818.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/mc146818.c b/drivers/rtc/mc146818.c index 9e94a80..da804d5 100644 --- a/drivers/rtc/mc146818.c +++ b/drivers/rtc/mc146818.c @@ -225,7 +225,7 @@ static int rtc_mc146818_write8(struct udevice *dev, unsigned int reg, int val) return 0; }

-static int rtc_mc146818_bind(struct udevice *dev) +static int rtc_mc146818_probe(struct udevice *dev) { mc146818_init();

@@ -249,7 +249,7 @@ U_BOOT_DRIVER(rtc_mc146818) = { .name = "rtc_mc146818", .id = UCLASS_RTC, .of_match = rtc_mc146818_ids,

•   .bind = rtc_mc146818_bind,
  

•   .probe = rtc_mc146818_probe,
    .ops = &rtc_mc146818_ops,
  

};

--

Regards, Bin

Hi Simon,

On Mon, Oct 19, 2015 at 10:26 AM, Simon Glass sjg@chromium.org wrote:

Hi Bin,

On 18 October 2015 at 20:22, Bin Meng bmeng.cn@gmail.com wrote:

...

Hi Simon,

On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass sjg@chromium.org wrote:

...

At present this driver uses bind() to set up the device. The bind() method should not touch the hardware, so move the init code to probe().

I think RTC should be initialized anyway. If moving it to probe, it may not be initialized by U-Boot before jumping to kernel.

That's fine, but the correct way to do this is to probe the device, not put the init code into the bind() method.

Yes, I agree. So maybe we explicitly trigger the probe somewhere in the initialization path?

...

...

Signed-off-by: Simon Glass sjg@chromium.org

drivers/rtc/mc146818.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/mc146818.c b/drivers/rtc/mc146818.c index 9e94a80..da804d5 100644 --- a/drivers/rtc/mc146818.c +++ b/drivers/rtc/mc146818.c @@ -225,7 +225,7 @@ static int rtc_mc146818_write8(struct udevice *dev, unsigned int reg, int val) return 0; }

-static int rtc_mc146818_bind(struct udevice *dev) +static int rtc_mc146818_probe(struct udevice *dev) { mc146818_init();

@@ -249,7 +249,7 @@ U_BOOT_DRIVER(rtc_mc146818) = { .name = "rtc_mc146818", .id = UCLASS_RTC, .of_match = rtc_mc146818_ids,

•   .bind = rtc_mc146818_bind,
  

•   .probe = rtc_mc146818_probe,
    .ops = &rtc_mc146818_ops,
  

};

--

Regards, Bin

On 18 October 2015 at 20:23, Bin Meng bmeng.cn@gmail.com wrote:

On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass sjg@chromium.org wrote:

...

The current check is incorrect and will fail when any non-zero byte is read. Fix it.

Signed-off-by: Simon Glass sjg@chromium.org

drivers/rtc/rtc-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-uclass.c b/drivers/rtc/rtc-uclass.c index fe74c69..300e9b3 100644 --- a/drivers/rtc/rtc-uclass.c +++ b/drivers/rtc/rtc-uclass.c @@ -68,7 +68,7 @@ int rtc_read32(struct udevice *dev, unsigned int reg, u32 *valuep)

    for (i = 0; i < sizeof(value); i++) {
            ret = rtc_read8(dev, reg + i);

•           if (ret)
  

•           if (ret < 0)
                    return ret;
            value |= ret << (i << 3);
    }
  

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Applied to u-boot-x86.

On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass sjg@chromium.org wrote:

For consistency, use 'ret' to handle a return value.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/cpu/ivybridge/sdram.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/x86/cpu/ivybridge/sdram.c b/arch/x86/cpu/ivybridge/sdram.c index fc66a3c..26e2e5b 100644 --- a/arch/x86/cpu/ivybridge/sdram.c +++ b/arch/x86/cpu/ivybridge/sdram.c @@ -93,11 +93,11 @@ static int read_seed_from_cmos(struct pei_data *pei_data) { u16 c1, c2, checksum, seed_checksum; struct udevice *dev;

•   int rcode = 0;
  

•   int ret = 0;
  
  

•   rcode = uclass_get_device(UCLASS_RTC, 0, &dev);
  

•   if (rcode) {
  

•           debug("Cannot find RTC: err=%d\n", rcode);
  

•   ret = uclass_get_device(UCLASS_RTC, 0, &dev);
  

•   if (ret) {
  

•           debug("Cannot find RTC: err=%d\n", ret);
            return -ENODEV;
    }
  
  

@@ -170,11 +170,11 @@ static int write_seeds_to_cmos(struct pei_data *pei_data) { u16 c1, c2, checksum; struct udevice *dev;

•   int rcode = 0;
  

•   int ret = 0;
  
  

•   rcode = uclass_get_device(UCLASS_RTC, 0, &dev);
  

•   if (rcode) {
  

•           debug("Cannot find RTC: err=%d\n", rcode);
  

•   ret = uclass_get_device(UCLASS_RTC, 0, &dev);
  

•   if (ret) {
  

•           debug("Cannot find RTC: err=%d\n", ret);
            return -ENODEV;
    }
  
  

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi Simon,

On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass sjg@chromium.org wrote:

Use this option instead of a private CONFIG_CACHE_MRC_BIN option.

The CONFIG_CACHE_MRC_BIN option seems to be used to program the MTRR for the mrc.bin text range to make it run faster. It is nothing related to the MRC cache data that the CONFIG_ENABLE_MRC_CACHE option is about.

Signed-off-by: Simon Glass sjg@chromium.org

arch/x86/cpu/ivybridge/Kconfig | 6 ------ arch/x86/cpu/ivybridge/car.S | 4 ++-- 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/x86/cpu/ivybridge/Kconfig b/arch/x86/cpu/ivybridge/Kconfig index 0e249a4..c3f324e 100644 --- a/arch/x86/cpu/ivybridge/Kconfig +++ b/arch/x86/cpu/ivybridge/Kconfig @@ -8,12 +8,10 @@

config NORTHBRIDGE_INTEL_SANDYBRIDGE bool

•   select CACHE_MRC_BIN
    select CPU_INTEL_MODEL_206AX
  
  

config NORTHBRIDGE_INTEL_IVYBRIDGE bool

•   select CACHE_MRC_BIN
    select CPU_INTEL_MODEL_306AX
  
  

if NORTHBRIDGE_INTEL_SANDYBRIDGE @@ -136,8 +134,4 @@ config SOCKET_SPECIFIC_OPTIONS # dummy select SSE select CACHE_AS_RAM

-config CACHE_MRC_BIN

•   bool
  

•   default n
  

endif diff --git a/arch/x86/cpu/ivybridge/car.S b/arch/x86/cpu/ivybridge/car.S index 407e451..770ef17 100644 --- a/arch/x86/cpu/ivybridge/car.S +++ b/arch/x86/cpu/ivybridge/car.S @@ -145,7 +145,7 @@ clear_mtrrs: wrmsr

    post_code(POST_CAR_ROM_CACHE)

-#ifdef CONFIG_CACHE_MRC_BIN +#ifdef CONFIG_ENABLE_MRC_CACHE /* Enable caching for ram init code to run faster */ movl $MTRR_PHYS_BASE_MSR(2), %ecx movl $(CACHE_MRC_BASE | MTRR_TYPE_WRPROT), %eax @@ -200,7 +200,7 @@ car_uninit: andl $~1, %eax wrmsr

-#ifdef CONFIG_CACHE_MRC_BIN +#ifdef CONFIG_ENABLE_MRC_CACHE /* Clear the MTRR that was used to cache MRC */ xorl %eax, %eax xorl %edx, %edx --

Regards, Bin

Hi Bin,

On 18 October 2015 at 20:23, Bin Meng bmeng.cn@gmail.com wrote:

Hi Simon,

On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass sjg@chromium.org wrote:

...

At present a missing $ causes this code to hang when using the MRC cache/ Fix it.

Signed-off-by: Simon Glass sjg@chromium.org

Reviewed-by: Bin Meng bmeng.cn@gmail.com

...

---

arch/x86/cpu/ivybridge/car.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/cpu/ivybridge/car.S b/arch/x86/cpu/ivybridge/car.S index 770ef17..b75c2a5 100644 --- a/arch/x86/cpu/ivybridge/car.S +++ b/arch/x86/cpu/ivybridge/car.S @@ -188,7 +188,7 @@ car_uninit: wrmsr

    /* Disable the no-eviction run state */

•   movl    NOEVICTMOD_MSR, %ecx
  

•   movl    $NOEVICTMOD_MSR, %ecx
  

I am wondering why compiler does not complain this.

I suppose it just uses the value at that address.

...

    rdmsr
    andl    $~2, %eax
    wrmsr

--

Regards, Simon

On Mon, Oct 19, 2015 at 5:55 AM, Simon Glass sjg@chromium.org wrote:

From: Bin Meng bmeng.cn@gmail.com

This works correctly now, so enable it.

Signed-off-by: Bin Meng bmeng.cn@gmail.com Dropped malloc() and adjusted commit message: Signed-off-by: Simon Glass sjg@chromium.org

---

arch/x86/cpu/ivybridge/sdram.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/arch/x86/cpu/ivybridge/sdram.c b/arch/x86/cpu/ivybridge/sdram.c index d9b3dfc..4372a5c 100644 --- a/arch/x86/cpu/ivybridge/sdram.c +++ b/arch/x86/cpu/ivybridge/sdram.c @@ -158,14 +158,8 @@ static int prepare_mrc_cache(struct pei_data *pei_data) if (!mrc_cache) return -ENOENT;

•   /*
  

•    * TODO(sjg@chromium.org): Skip this for now as it causes boot
  

•    * problems
  

•    */
  

•   if (0) {
  

•           pei_data->mrc_input = mrc_cache->data;
  

•           pei_data->mrc_input_len = mrc_cache->data_size;
  

•   }
  

•   pei_data->mrc_input = mrc_cache->data;
  

•   pei_data->mrc_input_len = mrc_cache->data_size;
    debug("%s: at %p, size %x checksum %04x\n", __func__,
          pei_data->mrc_input, pei_data->mrc_input_len,
          mrc_cache->checksum);
  

--

Reviewed-by: Bin Meng bmeng.cn@gmail.com