On Sun, Nov 18, 2018 at 08:45:06AM +0000, Gil Beniamini wrote:
> Sorry I can only use Microsoft Office, and so I created an attachment,
> hopefully in acceptible format (I am not sure about who toname as "signed-off"),
> See attached files
Your signoff should be from yourself so what you've got is fine and now
the tab/spaces thing is sorted which is good but the format is still not
a normal kernel patch format - you basically want what git format-patch
would generate if your commit log looks like other commit logs in git.
> + /* The following while() which copies tx-data into tx-fifo, must not be interrupted,
> + ** interruption might lead to chip tx-fifo become empty before end of the transaction data,
> + ** and cause chip-select release at mid transaction, which means SPI' master-slave protocol violation!
> + */
Please wrap lines at less than 80 columns.
> + local_irq_save(flags);
> while (max--) {
> /* Set the tx word if the transfer's original "tx" is not null */
> if (dws->tx_end - dws->len) {
> @@ -194,6 +200,7 @@ static void dw_writer(struct dw_spi *dws
> dw_write_io_reg(dws, DW_SPI_DR, txw);
> dws->tx += dws->n_bytes;
> }
> + local_irq_restore(flags);
That's going to be a pretty long interrupts disabled period added
unconditionally which is a bit rude for other users. As Trent said the
normal thing with hardware with half baked chip select control like this
is to put the chip select into GPIO mode (or use a separate GPIO if
there's not enough pinmux flexibility). If we do decide to go down this
road we should ensure that those users don't have to pay the cost of the
interrupt disabled period and only disable the interrupts when using the
native chip select.
You'll probably also run into trouble with RT usage as you can't
actually disable interrupts there.
Attachment:
signature.asc
Description: PGP signature
