On Tue, Jun 14, 2016 at 11:43:56AM -0400, Kamal Dasu wrote:
> On Mon, Jun 13, 2016 at 6:13 AM, Mark Brown <broonie@xxxxxxxxxx> wrote:
> > On Fri, Jun 10, 2016 at 04:06:09PM -0400, Kamal Dasu wrote:
> >> + /* deassert CS on the final byte */
> >> + if (fnb & FNB_BREAK_DESELECT) {
> >> + mspi_cdram = read_cdram_slot(qspi, slot - 1) &
> >> + ~MSPI_CDRAM_CONT_BIT;
> >> + write_cdram_slot(qspi, slot - 1, mspi_cdram);
> >> + }
> > Let the core handle asserting and deasserting chip select.
> Its actually a peripheral CS (PCS) bit in the MSPI block. Peripheral
> chip selects are used to select an external device for serial data
> transfer. PCS[i] corresponds to pin SS[i].
That doesn't appear to address the issue, what you're describing sounds
like the normal function of the chip select?
> >> + switch (command) {
> >> + case SPINOR_OP_READ4_FAST:
> >> + if (!bcm_qspi_is_4_byte_mode(qspi))
> > No, this is not a good idea. You should not be attempting to parse
> > the data stream. If your controller has flash support it should
> > implement the flash interfaces, otherwise it should just pass the data
> > streams through.
> This is only for read command we need to setup both tx and rx at the
> same time for improved performance.
No, to repeat what I said if you want to support accelerated flash reads
implement the standard accelerated flash read operation we have
(spi_flash_read()). This is a fairly common feature and trying to open
code this in individual drivers would at the very least lead to a lot of
duplicated code and is the source of most of the problems in the code.
> >> + spi_message_init(&m);
> >> + m.spi = &spi;
> > I don't know why your driver is creating and trying to do SPI transfers
> > but that is completely inappropriate for a SPI controller. Whatever
> > functionality this is implementing should be in a separate SPI device
> > driver.
> Currently not used. But I did not see a good way to be able to send
> commands, like finding the ID, setting 3/4 byte addressing mode , quad
> mode, on a PM resume(). Are you suggesting I make a driver similar to
> m25p80 ?.
I am suggesting you implement the standard flash interfaces and as a
result directly use m25p80.
> >> +MODULE_DEVICE_TABLE(of, bcm_qspi_of_match);
> > This is adding new DT bindings but there is no documentation,
> > documentation is required for all DT bindings.
> This was part of Patch 1/4 as pointed to by Florian
Please use standard formatting when posting patches.
Attachment:
signature.asc
Description: PGP signature
