Hi!
NeilBrown <neil@xxxxxxxxxx> 于2018年12月4日周二 上午5:55写道：
>
> On Mon, Dec 03 2018, Chuanhong Guo wrote:
>
> > Under MORE_BUF_MODE the controller will always shift one bit out of
> > spi_opcode if (mosi_bit_cnt > 0) && (cmd_bit_cnt == 0) so the full-
> > duplex mode is broken since we can't read anything from MISO during
> > writing spi_opcode.
> > This piece of code also make CS1 unavailable since it forces the
> > broken full-duplex mode to be enabled on CS1.
>
> Hi,
> How do you know these details of the hardware? Do you have detailed
> specs for the hardware or have you experimented with it or are you one
> of the designers or ....?
> It would be nice to have the source of this information documented.
Those info are provided by John Crispin on IRC #openwrt-devel. Here is
the quoted reply:
---
Nov 26 17:00:15 <blogic> gch981213[w]: so basically i made cs1 work
for MTK/labs when i built the linkit smart for them. the req-sheet
said that cs1 should be proper duplex spi. however ....
Nov 26 17:00:34 <blogic> 1) the core will always send 1 byte before
any transfer, this is the m25p80 command.
Nov 26 17:00:49 <blogic> 2) mode 3 is broken and bit reversed (?)
Nov 26 17:01:01 <blogic> 3) some bit are incorrectly wired in hw for mode2/3
Nov 26 17:01:50 <blogic> we wrote a test script and test for
[0-0xffff] on all modes and certain bits are swizzled under certain
conditions and it was not possible to fix this even using a hack.
Nov 26 17:02:06 <blogic> we then decided to use spi-gpio and i never
removed the errornous code
Nov 26 17:02:38 <blogic> basically the spi is fecked for anything but
half duplex spi mode0 running a sflash on it
---
And another guy told me that his experiment shows there will be one
extra bit if we set cmd_bit_cnt to 0 and mosi_bit_cnt > 0.
I guess it doesn't matter whether it's an extra "byte" or "bit" since
there are always some data that have to be sent under half-duplex.
Do I need to add "some experiment shows that" at the beginning of my
commit message?
>
> Thanks,
> NeilBrown
>
>
> >
> > Signed-off-by: Chuanhong Guo <gch981213@xxxxxxxxx>
> > ---
> > drivers/staging/mt7621-spi/spi-mt7621.c | 120 +++---------------------
> > 1 file changed, 15 insertions(+), 105 deletions(-)
> >
> > diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c b/drivers/staging/mt7621-spi/spi-mt7621.c
> > index d045b5568e0f..8af6f307e176 100644
> > --- a/drivers/staging/mt7621-spi/spi-mt7621.c
> > +++ b/drivers/staging/mt7621-spi/spi-mt7621.c
> > @@ -86,16 +86,13 @@ static inline void mt7621_spi_write(struct mt7621_spi *rs, u32 reg, u32 val)
> > iowrite32(val, rs->base + reg);
> > }
> >
> > -static void mt7621_spi_reset(struct mt7621_spi *rs, int duplex)
> > +static void mt7621_spi_reset(struct mt7621_spi *rs)
> > {
> > u32 master = mt7621_spi_read(rs, MT7621_SPI_MASTER);
> >
> > master |= 7 << 29;
> > master |= 1 << 2;
> > - if (duplex)
> > - master |= 1 << 10;
> > - else
> > - master &= ~(1 << 10);
> > + master &= ~(1 << 10);
> >
> > mt7621_spi_write(rs, MT7621_SPI_MASTER, master);
> > rs->pending_write = 0;
> > @@ -107,7 +104,7 @@ static void mt7621_spi_set_cs(struct spi_device *spi, int enable)
> > int cs = spi->chip_select;
> > u32 polar = 0;
> >
> > - mt7621_spi_reset(rs, cs);
> > + mt7621_spi_reset(rs);
> > if (enable)
> > polar = BIT(cs);
> > mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
> > @@ -261,7 +258,7 @@ static void mt7621_spi_write_half_duplex(struct mt7621_spi *rs,
> > rs->pending_write = len;
> > }
> >
> > -static int mt7621_spi_transfer_half_duplex(struct spi_master *master,
> > +static int mt7621_spi_transfer_one_message(struct spi_master *master,
> > struct spi_message *m)
> > {
> > struct mt7621_spi *rs = spi_master_get_devdata(master);
> > @@ -284,7 +281,16 @@ static int mt7621_spi_transfer_half_duplex(struct spi_master *master,
> > mt7621_spi_set_cs(spi, 1);
> > m->actual_length = 0;
> > list_for_each_entry(t, &m->transfers, transfer_list) {
> > - if (t->rx_buf)
> > + if((t->rx_buf) && (t->tx_buf)) {
> > + /* This controller will shift one extra bit out
> > + * of spi_opcode if (mosi_bit_cnt > 0) &&
> > + * (cmd_bit_cnt == 0). So the claimed full-duplex
> > + * support is broken since we have no way to read
> > + * the MISO value during that bit.
> > + */
> > + status = -EIO;
> > + goto msg_done;
> > + } else if (t->rx_buf)
> > mt7621_spi_read_half_duplex(rs, t->len, t->rx_buf);
> > else if (t->tx_buf)
> > mt7621_spi_write_half_duplex(rs, t->len, t->tx_buf);
> > @@ -300,102 +306,6 @@ static int mt7621_spi_transfer_half_duplex(struct spi_master *master,
> > return 0;
> > }
> >
> > -static int mt7621_spi_transfer_full_duplex(struct spi_master *master,
> > - struct spi_message *m)
> > -{
> > - struct mt7621_spi *rs = spi_master_get_devdata(master);
> > - struct spi_device *spi = m->spi;
> > - unsigned int speed = spi->max_speed_hz;
> > - struct spi_transfer *t = NULL;
> > - int status = 0;
> > - int i, len = 0;
> > - int rx_len = 0;
> > - u32 data[9] = { 0 };
> > - u32 val = 0;
> > -
> > - mt7621_spi_wait_till_ready(rs);
> > -
> > - list_for_each_entry(t, &m->transfers, transfer_list) {
> > - const u8 *buf = t->tx_buf;
> > -
> > - if (t->rx_buf)
> > - rx_len += t->len;
> > -
> > - if (!buf)
> > - continue;
> > -
> > - if (WARN_ON(len + t->len > 16)) {
> > - status = -EIO;
> > - goto msg_done;
> > - }
> > -
> > - for (i = 0; i < t->len; i++, len++)
> > - data[len / 4] |= buf[i] << (8 * (len & 3));
> > - if (speed > t->speed_hz)
> > - speed = t->speed_hz;
> > - }
> > -
> > - if (WARN_ON(rx_len > 16)) {
> > - status = -EIO;
> > - goto msg_done;
> > - }
> > -
> > - if (mt7621_spi_prepare(spi, speed)) {
> > - status = -EIO;
> > - goto msg_done;
> > - }
> > -
> > - for (i = 0; i < len; i += 4)
> > - mt7621_spi_write(rs, MT7621_SPI_DATA0 + i, data[i / 4]);
> > -
> > - val |= len * 8;
> > - val |= (rx_len * 8) << 12;
> > - mt7621_spi_write(rs, MT7621_SPI_MOREBUF, val);
> > -
> > - mt7621_spi_set_cs(spi, 1);
> > -
> > - val = mt7621_spi_read(rs, MT7621_SPI_TRANS);
> > - val |= SPI_CTL_START;
> > - mt7621_spi_write(rs, MT7621_SPI_TRANS, val);
> > -
> > - mt7621_spi_wait_till_ready(rs);
> > -
> > - mt7621_spi_set_cs(spi, 0);
> > -
> > - for (i = 0; i < rx_len; i += 4)
> > - data[i / 4] = mt7621_spi_read(rs, MT7621_SPI_DATA4 + i);
> > -
> > - m->actual_length = rx_len;
> > -
> > - len = 0;
> > - list_for_each_entry(t, &m->transfers, transfer_list) {
> > - u8 *buf = t->rx_buf;
> > -
> > - if (!buf)
> > - continue;
> > -
> > - for (i = 0; i < t->len; i++, len++)
> > - buf[i] = data[len / 4] >> (8 * (len & 3));
> > - }
> > -
> > -msg_done:
> > - m->status = status;
> > - spi_finalize_current_message(master);
> > -
> > - return 0;
> > -}
> > -
> > -static int mt7621_spi_transfer_one_message(struct spi_master *master,
> > - struct spi_message *m)
> > -{
> > - struct spi_device *spi = m->spi;
> > - int cs = spi->chip_select;
> > -
> > - if (cs)
> > - return mt7621_spi_transfer_full_duplex(master, m);
> > - return mt7621_spi_transfer_half_duplex(master, m);
> > -}
> > -
> > static int mt7621_spi_setup(struct spi_device *spi)
> > {
> > struct mt7621_spi *rs = spidev_to_mt7621_spi(spi);
> > @@ -478,7 +388,7 @@ static int mt7621_spi_probe(struct platform_device *pdev)
> >
> > device_reset(&pdev->dev);
> >
> > - mt7621_spi_reset(rs, 0);
> > + mt7621_spi_reset(rs);
> >
> > return spi_register_master(master);
> > }
> > --
> > 2.19.1
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
