Re: [U-Boot] [PATCH 19/23] x86: spi: Support ValleyView in ICH SPI driver

2015-01-28 Thread Simon Glass 
Hi Bin,

On 27 January 2015 at 23:02, Bin Meng bmeng...@gmail.com wrote:
 Hi Simon,

 On Wed, Jan 28, 2015 at 1:17 PM, Simon Glass s...@chromium.org wrote:
 Hi Bin,

 On 27 January 2015 at 07:00, Bin Meng bmeng...@gmail.com wrote:
 Hi Simon,

 On Tue, Jan 27, 2015 at 9:23 AM, Simon Glass s...@chromium.org wrote:
 The base address is found in a different way and the protection bit is also
 in a different place. Otherwise it is very similar.

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

  drivers/spi/ich.c | 56 
 ---
  drivers/spi/ich.h | 11 ++-
  2 files changed, 47 insertions(+), 20 deletions(-)

 Just a note on this one...for v2 I have done nothing. I think we
 should move it to device tree as you say but it needs some more
 thought, as we have register offsets and device types to think about.
 I will try a few things and test on link also. Once I have this I will
 send a v3. So this patch will have to sit here until that is done,
 hopefully only a few days.


 OK, I think we can just apply this patch for now along with other
 patches in this series. We can revisit this driver when adding DT and
 DM support.

Oh OK we can do it that way. I actually have some partial work to
convert the ICH driver to DM, so I'll go back to that soon.

Applied to u-boot-x86.

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 19/23] x86: spi: Support ValleyView in ICH SPI driver

2015-01-27 Thread Bin Meng 
Hi Simon,

On Tue, Jan 27, 2015 at 9:23 AM, Simon Glass s...@chromium.org wrote:
 The base address is found in a different way and the protection bit is also
 in a different place. Otherwise it is very similar.

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

  drivers/spi/ich.c | 56 
 ---
  drivers/spi/ich.h | 11 ++-
  2 files changed, 47 insertions(+), 20 deletions(-)

 diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
 index fdff158..da85779 100644
 --- a/drivers/spi/ich.c
 +++ b/drivers/spi/ich.c
 @@ -7,6 +7,7 @@
   */

  #include common.h
 +#include errno.h
  #include malloc.h
  #include spi.h
  #include pci.h
 @@ -21,6 +22,7 @@
  struct ich_ctlr {
 pci_dev_t dev;  /* PCI device number */
 int ich_version;/* Controller version, 7 or 9 */
 +   bool use_sbase; /* Use SBASE instead of RCB */
 int ichspi_lock;
 int locked;
 uint8_t *opmenu;
 @@ -145,7 +147,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, 
 unsigned int cs,
  * ICH 7 SPI controller only supports array read command
  * and byte program command for SST flash
  */
 -   if (ctlr.ich_version == 7) {
 +   if (ctlr.ich_version == 7 || ctlr.use_sbase) {
 ich-slave.op_mode_rx = SPI_OPM_RX_AS;
 ich-slave.op_mode_tx = SPI_OPM_TX_BP;
 }

So the Valleyview SPI controller (version 9) has the same behavior
that was seen on Tunnel Creek (version 7). I wonder whether the same
behavior was seen on chromebook_link which is also version 9? Or is
this only because the Minnowmax board has the same SST flash mounted
on board, just like the Crown Bay so that we need set these RX/TX
flags. If SPI controller on chromebook_link also has the simliar
behavior, we should consider set these flags for all Intel SPI
controller variants. This way, the 'use_sbase' variable may not be
necessary (see below for more comments).

 @@ -181,7 +183,8 @@ static int get_ich_version(uint16_t device_id)
 if ((device_id = PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MIN 
  device_id = PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MAX) ||
 (device_id = PCI_DEVICE_ID_INTEL_PANTHERPOINT_LPC_MIN 
 -device_id = PCI_DEVICE_ID_INTEL_PANTHERPOINT_LPC_MAX))
 +device_id = PCI_DEVICE_ID_INTEL_PANTHERPOINT_LPC_MAX) ||
 +   device_id == PCI_DEVICE_ID_INTEL_VALLEYVIEW_LPC)

To avoid bigger and bigger test logic, should we consider using DT to
pass such information?

 return 9;

 return 0;
 @@ -204,14 +207,14 @@ static int ich9_can_do_33mhz(pci_dev_t dev)
 return speed == 1;
  }

 -static int ich_find_spi_controller(pci_dev_t *devp, int *ich_versionp)
 +static int ich_find_spi_controller(struct ich_ctlr *ich)
  {
 int last_bus = pci_last_busno();
 int bus;

 if (last_bus == -1) {
 debug(No PCI busses?\n);
 -   return -1;
 +   return -ENODEV;
 }

 for (bus = 0; bus = last_bus; bus++) {
 @@ -225,24 +228,33 @@ static int ich_find_spi_controller(pci_dev_t *devp, int 
 *ich_versionp)
 device_id = ids  16;

 if (vendor_id == PCI_VENDOR_ID_INTEL) {
 -   *devp = dev;
 -   *ich_versionp = get_ich_version(device_id);
 -   return 0;
 +   ich-dev = dev;
 +   ich-ich_version = get_ich_version(device_id);
 +   if (device_id == PCI_DEVICE_ID_INTEL_VALLEYVIEW_LPC)
 +   ich-use_sbase = true;
 +   return ich-ich_version == 0 ? -ENODEV : 0;
 }
 }

 debug(ICH SPI: No ICH found.\n);
 -   return -1;
 +   return -ENODEV;
  }

  static int ich_init_controller(struct ich_ctlr *ctlr)
  {
 uint8_t *rcrb; /* Root Complex Register Block */
 uint32_t rcba; /* Root Complex Base Address */
 +   uint32_t sbase_addr;
 +   uint8_t *sbase;

 pci_read_config_dword(ctlr-dev, 0xf0, rcba);
 /* Bits 31-14 are the base address, 13-1 are reserved, 0 is enable. */
 rcrb = (uint8_t *)(rcba  0xc000);
 +
 +   /* SBASE is similar */
 +   pci_read_config_dword(ctlr-dev, 0x54, sbase_addr);
 +   sbase = (uint8_t *)(sbase_addr  0xfe00);
 +

Again, DT can be used to determine the base address.

 if (ctlr-ich_version == 7) {
 struct ich7_spi_regs *ich7_spi;

 @@ -262,7 +274,10 @@ static int ich_init_controller(struct ich_ctlr *ctlr)
 } else if (ctlr-ich_version == 9) {
 struct ich9_spi_regs *ich9_spi;

 -   ich9_spi = (struct ich9_spi_regs *)(rcrb + 0x3800);
 +   if (ctlr-use_sbase)
 +   ich9_spi = (struct ich9_spi_regs *)sbase;
 +   else
 +   ich9_spi = (struct

Re: [U-Boot] [PATCH 19/23] x86: spi: Support ValleyView in ICH SPI driver

2015-01-27 Thread Simon Glass 
Hi Bin,

On 27 January 2015 at 07:00, Bin Meng bmeng...@gmail.com wrote:
 Hi Simon,

 On Tue, Jan 27, 2015 at 9:23 AM, Simon Glass s...@chromium.org wrote:
 The base address is found in a different way and the protection bit is also
 in a different place. Otherwise it is very similar.

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

  drivers/spi/ich.c | 56 
 ---
  drivers/spi/ich.h | 11 ++-
  2 files changed, 47 insertions(+), 20 deletions(-)

Just a note on this one...for v2 I have done nothing. I think we
should move it to device tree as you say but it needs some more
thought, as we have register offsets and device types to think about.
I will try a few things and test on link also. Once I have this I will
send a v3. So this patch will have to sit here until that is done,
hopefully only a few days.

Thanks for the quick reviews!

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 19/23] x86: spi: Support ValleyView in ICH SPI driver

2015-01-27 Thread Bin Meng 
Hi Simon,

On Wed, Jan 28, 2015 at 1:17 PM, Simon Glass s...@chromium.org wrote:
 Hi Bin,

 On 27 January 2015 at 07:00, Bin Meng bmeng...@gmail.com wrote:
 Hi Simon,

 On Tue, Jan 27, 2015 at 9:23 AM, Simon Glass s...@chromium.org wrote:
 The base address is found in a different way and the protection bit is also
 in a different place. Otherwise it is very similar.

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

  drivers/spi/ich.c | 56 
 ---
  drivers/spi/ich.h | 11 ++-
  2 files changed, 47 insertions(+), 20 deletions(-)

 Just a note on this one...for v2 I have done nothing. I think we
 should move it to device tree as you say but it needs some more
 thought, as we have register offsets and device types to think about.
 I will try a few things and test on link also. Once I have this I will
 send a v3. So this patch will have to sit here until that is done,
 hopefully only a few days.


OK, I think we can just apply this patch for now along with other
patches in this series. We can revisit this driver when adding DT and
DM support.

 Thanks for the quick reviews!


You are welcome!

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH 19/23] x86: spi: Support ValleyView in ICH SPI driver

2015-01-26 Thread Simon Glass 
The base address is found in a different way and the protection bit is also
in a different place. Otherwise it is very similar.

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

 drivers/spi/ich.c | 56 ---
 drivers/spi/ich.h | 11 ++-
 2 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
index fdff158..da85779 100644
--- a/drivers/spi/ich.c
+++ b/drivers/spi/ich.c
@@ -7,6 +7,7 @@
  */
 
 #include common.h
+#include errno.h
 #include malloc.h
 #include spi.h
 #include pci.h
@@ -21,6 +22,7 @@
 struct ich_ctlr {
pci_dev_t dev;  /* PCI device number */
int ich_version;/* Controller version, 7 or 9 */
+   bool use_sbase; /* Use SBASE instead of RCB */
int ichspi_lock;
int locked;
uint8_t *opmenu;
@@ -145,7 +147,7 @@ struct spi_slave *spi_setup_slave(unsigned int bus, 
unsigned int cs,
 * ICH 7 SPI controller only supports array read command
 * and byte program command for SST flash
 */
-   if (ctlr.ich_version == 7) {
+   if (ctlr.ich_version == 7 || ctlr.use_sbase) {
ich-slave.op_mode_rx = SPI_OPM_RX_AS;
ich-slave.op_mode_tx = SPI_OPM_TX_BP;
}
@@ -181,7 +183,8 @@ static int get_ich_version(uint16_t device_id)
if ((device_id = PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MIN 
 device_id = PCI_DEVICE_ID_INTEL_COUGARPOINT_LPC_MAX) ||
(device_id = PCI_DEVICE_ID_INTEL_PANTHERPOINT_LPC_MIN 
-device_id = PCI_DEVICE_ID_INTEL_PANTHERPOINT_LPC_MAX))
+device_id = PCI_DEVICE_ID_INTEL_PANTHERPOINT_LPC_MAX) ||
+   device_id == PCI_DEVICE_ID_INTEL_VALLEYVIEW_LPC)
return 9;
 
return 0;
@@ -204,14 +207,14 @@ static int ich9_can_do_33mhz(pci_dev_t dev)
return speed == 1;
 }
 
-static int ich_find_spi_controller(pci_dev_t *devp, int *ich_versionp)
+static int ich_find_spi_controller(struct ich_ctlr *ich)
 {
int last_bus = pci_last_busno();
int bus;
 
if (last_bus == -1) {
debug(No PCI busses?\n);
-   return -1;
+   return -ENODEV;
}
 
for (bus = 0; bus = last_bus; bus++) {
@@ -225,24 +228,33 @@ static int ich_find_spi_controller(pci_dev_t *devp, int 
*ich_versionp)
device_id = ids  16;
 
if (vendor_id == PCI_VENDOR_ID_INTEL) {
-   *devp = dev;
-   *ich_versionp = get_ich_version(device_id);
-   return 0;
+   ich-dev = dev;
+   ich-ich_version = get_ich_version(device_id);
+   if (device_id == PCI_DEVICE_ID_INTEL_VALLEYVIEW_LPC)
+   ich-use_sbase = true;
+   return ich-ich_version == 0 ? -ENODEV : 0;
}
}
 
debug(ICH SPI: No ICH found.\n);
-   return -1;
+   return -ENODEV;
 }
 
 static int ich_init_controller(struct ich_ctlr *ctlr)
 {
uint8_t *rcrb; /* Root Complex Register Block */
uint32_t rcba; /* Root Complex Base Address */
+   uint32_t sbase_addr;
+   uint8_t *sbase;
 
pci_read_config_dword(ctlr-dev, 0xf0, rcba);
/* Bits 31-14 are the base address, 13-1 are reserved, 0 is enable. */
rcrb = (uint8_t *)(rcba  0xc000);
+
+   /* SBASE is similar */
+   pci_read_config_dword(ctlr-dev, 0x54, sbase_addr);
+   sbase = (uint8_t *)(sbase_addr  0xfe00);
+
if (ctlr-ich_version == 7) {
struct ich7_spi_regs *ich7_spi;
 
@@ -262,7 +274,10 @@ static int ich_init_controller(struct ich_ctlr *ctlr)
} else if (ctlr-ich_version == 9) {
struct ich9_spi_regs *ich9_spi;
 
-   ich9_spi = (struct ich9_spi_regs *)(rcrb + 0x3800);
+   if (ctlr-use_sbase)
+   ich9_spi = (struct ich9_spi_regs *)sbase;
+   else
+   ich9_spi = (struct ich9_spi_regs *)(rcrb + 0x3800);
ctlr-ichspi_lock = ich_readw(ich9_spi-hsfs)  HSFS_FLOCKDN;
ctlr-opmenu = ich9_spi-opmenu;
ctlr-menubytes = sizeof(ich9_spi-opmenu);
@@ -282,12 +297,13 @@ static int ich_init_controller(struct ich_ctlr *ctlr)
  ctlr-ich_version);
return -1;
}
-   debug(ICH SPI: Version %d detected\n, ctlr-ich_version);
 
/* Work out the maximum speed we can support */
ctlr-max_speed = 2000;
if (ctlr-ich_version == 9  ich9_can_do_33mhz(ctlr-dev))
ctlr-max_speed = 3300;
+   debug(ICH SPI: Version %d detected at %p, speed %ld\n,
+ ctlr-ich_version, ctlr-base, ctlr-max_speed);
 
ich_set_bbar(ctlr, 0);
 
@@ -298,7 +314,7 @@ void spi_init(void)
 {
uint8_t bios_cntl;
 
-   if