[U-Boot] [PATCH 1/2][v2] powerpc: eSPI and eSPI controller support

[Kumar Gala]

On Apr 21, 2011, at 4:25 PM, Mike Frysinger wrote:

> On Thu, Apr 21, 2011 at 2:04 AM, Shaohui Xie wrote:
>> +struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
>> +               unsigned int max_hz, unsigned int mode)
>> +{
>> +       ccsr_espi_t *espi = (void *)(CONFIG_SYS_MPC85xx_ESPI_ADDR);
> 
> you support just one bus ?  that's no fun ;).

Until our SoC guys actually build a device with more than one, we'll keep with the one.
> 
>> +       /* Enable eSPI interface */
>> +       out_be32(&espi->mode, ESPI_MODE_RXTHR(3)
>> +                       | ESPI_MODE_TXTHR(4) | ESPI_MODE_EN);
>> +
>> +       out_be32(&espi->event, 0xffffffff); /* Clear all eSPI events */
>> +       out_be32(&espi->mask, 0x00000000); /* Mask  all eSPI interrupts */
>> +
>> +       /* Init CS mode interface */
>> +       for (i = 0; i < ESPI_MAX_CS_NUM; i++)
>> +               out_be32(&espi->csmode[i], ESPI_CSMODE_INIT_VAL);
>> +
>> +       out_be32(&espi->csmode[cs], in_be32(&espi->csmode[cs]) &
>> +               ~(ESPI_CSMODE_PM(0xF) | ESPI_CSMODE_DIV16
>> +               | ESPI_CSMODE_CI_INACTIVEHIGH | ESPI_CSMODE_CP_BEGIN_EDGCLK
>> +               | ESPI_CSMODE_REV_MSB_FIRST | ESPI_CSMODE_LEN(0xF)));
>> ........
> 
> usually spi_setup_slave() is to get the slave client ready to be used.
> it shouldnt be programming any actual hardware registers.  that is
> the point of the spi_claim_bus() and spi_release_bus() steps ... use
> the information in the slave state to setup the hardware for that
> slave and then break it down in the release step.
> 
>> +static u8 cmd_buf[16];
>> +static size_t cmd_len;
>> +static size_t data_len;
>> +
>> +int spi_xfer(struct spi_slave *slave, unsigned int bitlen, const void *data_out,
>> +               void *data_in, unsigned long flags)
>> +{
>> +       ccsr_espi_t *espi = (void *)(CONFIG_SYS_MPC85xx_ESPI_ADDR);
>> +       unsigned int tmpdout, tmpdin, event;
>> +       const void *dout = NULL;
>> +       void *din = NULL;
>> +       unsigned int len;
>> +       int numBlks;
>> +       int num_bytes;
>> +       unsigned char *ch;
>> +       unsigned char *buffer;
>> +       size_t buf_len;
>> +
>> +       switch (flags) {
>> +       case SPI_XFER_BEGIN:
>> +               cmd_len = bitlen / 8;
>> +               memcpy(cmd_buf, data_out, cmd_len);
>> +               return 0;
> 
> interesting solution to the problem, but i'd think it might make more
> sense to have this state live in the slave data rather than in the
> global bus.
> 
>> +void spi_cs_activate(struct spi_slave *slave)
>> +{
>> +       ccsr_espi_t *espi = (void *)(CONFIG_SYS_MPC85xx_ESPI_ADDR);
>> +       unsigned int com = 0;
>> +
>> +       com &= ~(ESPI_COM_CS(0x3) | ESPI_COM_TRANLEN(0xFFFF));
>> +       com |= ESPI_COM_CS(slave->cs);
>> +       com |= ESPI_COM_TRANLEN(data_len - 1);
>> +       out_be32(&espi->com, com);
>> +
>> +       return;
>> +}
> 
> that return is useless and can be punted

- k