| Message ID | 1369891191-28274-3-git-send-email-rajeshwari.s@samsung.com |
|---|---|
| State | New |
| Headers | show |
Hi, On 30-05-2013 10:49, Rajeshwari Shinde wrote: > For devices that need some time to react after a spi transaction > finishes, add the ability to set a delay. > > Implement this as a delay on the first/next transaction to avoid > any delay in the fairly common case where a SPI transaction is > followed by other processing. > > Based on: > "[U-Boot] [PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode" Please use the below format for referring previous commits ex: "<PREVIOUS_COMMIT_HEAD>" (sha1: PREVIOUS_COMMIT_ID) in this case "spi: exynos: Support SPI_PREAMBLE mode" (sha1: e4eaef8910df805d511b1cb9b2caafa7d2827fdc) > > Signed-off-by: Simon Glass <sjg@chromium.org> > Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> > --- > Changes in V2: > - None. > Changes in V3: > - Rebased on "[PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode" > drivers/spi/exynos_spi.c | 19 +++++++++++++++++++ > 1 files changed, 19 insertions(+), 0 deletions(-) > > diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c > index 01378d0..03cf503 100644 > --- a/drivers/spi/exynos_spi.c > +++ b/drivers/spi/exynos_spi.c > @@ -38,6 +38,7 @@ struct spi_bus { > struct exynos_spi *regs; > int inited; /* 1 if this bus is ready for use */ > int node; > + uint deactivate_delay_us; /* Delay to wait after deactivate */ > }; > > /* A list of spi buses that we know about */ > @@ -52,6 +53,8 @@ struct exynos_spi_slave { > enum periph_id periph_id; /* Peripheral ID for this device */ > unsigned int fifo_size; > int skip_preamble; > + struct spi_bus *bus; /* Pointer to our SPI bus info */ > + ulong last_transaction_us; /* Time of last transaction end */ > }; > > static struct spi_bus *spi_get_bus(unsigned dev_index) > @@ -97,6 +100,7 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, unsigned int cs, > } > > bus = &spi_bus[busnum]; > + spi_slave->bus = bus; > spi_slave->regs = bus->regs; > spi_slave->mode = mode; > spi_slave->periph_id = bus->periph_id; > @@ -107,6 +111,7 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, unsigned int cs, > spi_slave->fifo_size = 256; > > spi_slave->skip_preamble = 0; > + spi_slave->last_transaction_us = timer_get_us(); > > spi_slave->freq = bus->frequency; > if (max_hz) > @@ -371,9 +376,21 @@ void spi_cs_activate(struct spi_slave *slave) > { > struct exynos_spi_slave *spi_slave = to_exynos_spi(slave); > > + /* If it's too soon to do another transaction, wait */ > + if (spi_slave->bus->deactivate_delay_us && > + spi_slave->last_transaction_us) { > + ulong delay_us; /* The delay completed so far */ > + delay_us = timer_get_us() - spi_slave->last_transaction_us; > + if (delay_us < spi_slave->bus->deactivate_delay_us) > + udelay(spi_slave->bus->deactivate_delay_us - delay_us); > + } Add one space > clrbits_le32(&spi_slave->regs->cs_reg, SPI_SLAVE_SIG_INACT); > debug("Activate CS, bus %d\n", spi_slave->slave.bus); > spi_slave->skip_preamble = spi_slave->mode & SPI_PREAMBLE; > + > + /* Remember time of this transaction so we can honour the bus delay */ > + if (spi_slave->bus->deactivate_delay_us) > + spi_slave->last_transaction_us = timer_get_us(); > } > > /** > @@ -423,6 +440,8 @@ static int spi_get_config(const void *blob, int node, struct spi_bus *bus) > /* Use 500KHz as a suitable default */ > bus->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", > 500000); > + bus->deactivate_delay_us = fdtdec_get_int(blob, node, > + "spi-deactivate-delay", 0); > > return 0; > } > Was this tested, usually what is the max deactivate delay? In this case your are passing through dts is it? Thanks, Jagan.
Hi Jagan, On Wed, Jun 12, 2013 at 12:51 PM, Jagan Teki <jagannadh.teki@gmail.com>wrote: > Hi, > > > On 30-05-2013 10:49, Rajeshwari Shinde wrote: > >> For devices that need some time to react after a spi transaction >> finishes, add the ability to set a delay. >> >> Implement this as a delay on the first/next transaction to avoid >> any delay in the fairly common case where a SPI transaction is >> followed by other processing. >> >> Based on: >> "[U-Boot] [PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode" >> > > Please use the below format for referring previous commits > ex: > "<PREVIOUS_COMMIT_HEAD>" > (sha1: PREVIOUS_COMMIT_ID) > > in this case > "spi: exynos: Support SPI_PREAMBLE mode" > (sha1: e4eaef8910df805d511b1cb9b2caaf**a7d2827fdc) I hope you don't mind me answering part of this... > > > >> Signed-off-by: Simon Glass <sjg@chromium.org> >> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> >> --- >> Changes in V2: >> - None. >> Changes in V3: >> - Rebased on "[PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode" >> drivers/spi/exynos_spi.c | 19 +++++++++++++++++++ >> 1 files changed, 19 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c >> index 01378d0..03cf503 100644 >> --- a/drivers/spi/exynos_spi.c >> +++ b/drivers/spi/exynos_spi.c >> @@ -38,6 +38,7 @@ struct spi_bus { >> struct exynos_spi *regs; >> int inited; /* 1 if this bus is ready for use */ >> int node; >> + uint deactivate_delay_us; /* Delay to wait after deactivate >> */ >> }; >> >> /* A list of spi buses that we know about */ >> @@ -52,6 +53,8 @@ struct exynos_spi_slave { >> enum periph_id periph_id; /* Peripheral ID for this device >> */ >> unsigned int fifo_size; >> int skip_preamble; >> + struct spi_bus *bus; /* Pointer to our SPI bus info */ >> + ulong last_transaction_us; /* Time of last transaction end */ >> }; >> >> static struct spi_bus *spi_get_bus(unsigned dev_index) >> @@ -97,6 +100,7 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, >> unsigned int cs, >> } >> >> bus = &spi_bus[busnum]; >> + spi_slave->bus = bus; >> spi_slave->regs = bus->regs; >> spi_slave->mode = mode; >> spi_slave->periph_id = bus->periph_id; >> @@ -107,6 +111,7 @@ struct spi_slave *spi_setup_slave(unsigned int >> busnum, unsigned int cs, >> spi_slave->fifo_size = 256; >> >> spi_slave->skip_preamble = 0; >> + spi_slave->last_transaction_us = timer_get_us(); >> >> spi_slave->freq = bus->frequency; >> if (max_hz) >> @@ -371,9 +376,21 @@ void spi_cs_activate(struct spi_slave *slave) >> { >> struct exynos_spi_slave *spi_slave = to_exynos_spi(slave); >> >> + /* If it's too soon to do another transaction, wait */ >> + if (spi_slave->bus->deactivate_**delay_us && >> + spi_slave->last_transaction_**us) { >> + ulong delay_us; /* The delay completed so far */ >> + delay_us = timer_get_us() - spi_slave->last_transaction_* >> *us; >> + if (delay_us < spi_slave->bus->deactivate_**delay_us) >> + udelay(spi_slave->bus->**deactivate_delay_us - >> delay_us); >> + } >> > Add one space > > > clrbits_le32(&spi_slave->regs-**>cs_reg, SPI_SLAVE_SIG_INACT); >> debug("Activate CS, bus %d\n", spi_slave->slave.bus); >> spi_slave->skip_preamble = spi_slave->mode & SPI_PREAMBLE; >> + >> + /* Remember time of this transaction so we can honour the bus >> delay */ >> + if (spi_slave->bus->deactivate_**delay_us) >> + spi_slave->last_transaction_us = timer_get_us(); >> } >> >> /** >> @@ -423,6 +440,8 @@ static int spi_get_config(const void *blob, int node, >> struct spi_bus *bus) >> /* Use 500KHz as a suitable default */ >> bus->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", >> 500000); >> + bus->deactivate_delay_us = fdtdec_get_int(blob, node, >> + "spi-deactivate-delay", 0); >> >> return 0; >> } >> >> > Was this tested, usually what is the max deactivate delay? > This was tested with 50us. > In this case your are passing through dts is it? > Yes that's right. Regards, Simon > > Thanks, > Jagan. > > >
On Thu, Jun 13, 2013 at 3:36 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Jagan, > > On Wed, Jun 12, 2013 at 12:51 PM, Jagan Teki <jagannadh.teki@gmail.com> > wrote: >> >> Hi, >> >> >> On 30-05-2013 10:49, Rajeshwari Shinde wrote: >>> >>> For devices that need some time to react after a spi transaction >>> finishes, add the ability to set a delay. >>> >>> Implement this as a delay on the first/next transaction to avoid >>> any delay in the fairly common case where a SPI transaction is >>> followed by other processing. >>> >>> Based on: >>> "[U-Boot] [PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode" >> >> >> Please use the below format for referring previous commits >> ex: >> "<PREVIOUS_COMMIT_HEAD>" >> (sha1: PREVIOUS_COMMIT_ID) >> >> in this case >> "spi: exynos: Support SPI_PREAMBLE mode" >> (sha1: e4eaef8910df805d511b1cb9b2caafa7d2827fdc) > > > I hope you don't mind me answering part of this... > >> >> >> >>> >>> Signed-off-by: Simon Glass <sjg@chromium.org> >>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> >>> --- >>> Changes in V2: >>> - None. >>> Changes in V3: >>> - Rebased on "[PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode" >>> drivers/spi/exynos_spi.c | 19 +++++++++++++++++++ >>> 1 files changed, 19 insertions(+), 0 deletions(-) >>> >>> diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c >>> index 01378d0..03cf503 100644 >>> --- a/drivers/spi/exynos_spi.c >>> +++ b/drivers/spi/exynos_spi.c >>> @@ -38,6 +38,7 @@ struct spi_bus { >>> struct exynos_spi *regs; >>> int inited; /* 1 if this bus is ready for use */ >>> int node; >>> + uint deactivate_delay_us; /* Delay to wait after deactivate >>> */ >>> }; >>> >>> /* A list of spi buses that we know about */ >>> @@ -52,6 +53,8 @@ struct exynos_spi_slave { >>> enum periph_id periph_id; /* Peripheral ID for this device >>> */ >>> unsigned int fifo_size; >>> int skip_preamble; >>> + struct spi_bus *bus; /* Pointer to our SPI bus info */ >>> + ulong last_transaction_us; /* Time of last transaction end >>> */ >>> }; >>> >>> static struct spi_bus *spi_get_bus(unsigned dev_index) >>> @@ -97,6 +100,7 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, >>> unsigned int cs, >>> } >>> >>> bus = &spi_bus[busnum]; >>> + spi_slave->bus = bus; >>> spi_slave->regs = bus->regs; >>> spi_slave->mode = mode; >>> spi_slave->periph_id = bus->periph_id; >>> @@ -107,6 +111,7 @@ struct spi_slave *spi_setup_slave(unsigned int >>> busnum, unsigned int cs, >>> spi_slave->fifo_size = 256; >>> >>> spi_slave->skip_preamble = 0; >>> + spi_slave->last_transaction_us = timer_get_us(); >>> >>> spi_slave->freq = bus->frequency; >>> if (max_hz) >>> @@ -371,9 +376,21 @@ void spi_cs_activate(struct spi_slave *slave) >>> { >>> struct exynos_spi_slave *spi_slave = to_exynos_spi(slave); >>> >>> + /* If it's too soon to do another transaction, wait */ >>> + if (spi_slave->bus->deactivate_delay_us && >>> + spi_slave->last_transaction_us) { >>> + ulong delay_us; /* The delay completed so far */ >>> + delay_us = timer_get_us() - >>> spi_slave->last_transaction_us; >>> + if (delay_us < spi_slave->bus->deactivate_delay_us) >>> + udelay(spi_slave->bus->deactivate_delay_us - >>> delay_us); >>> + } >> >> Add one space >> >> >>> clrbits_le32(&spi_slave->regs->cs_reg, SPI_SLAVE_SIG_INACT); >>> debug("Activate CS, bus %d\n", spi_slave->slave.bus); >>> spi_slave->skip_preamble = spi_slave->mode & SPI_PREAMBLE; >>> + >>> + /* Remember time of this transaction so we can honour the bus >>> delay */ >>> + if (spi_slave->bus->deactivate_delay_us) >>> + spi_slave->last_transaction_us = timer_get_us(); >>> } >>> >>> /** >>> @@ -423,6 +440,8 @@ static int spi_get_config(const void *blob, int node, >>> struct spi_bus *bus) >>> /* Use 500KHz as a suitable default */ >>> bus->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", >>> 500000); >>> + bus->deactivate_delay_us = fdtdec_get_int(blob, node, >>> + "spi-deactivate-delay", 0); >>> >>> return 0; >>> } >>> >> >> Was this tested, usually what is the max deactivate delay? > > > This was tested with 50us. > >> >> In this case your are passing through dts is it? > > > Yes that's right. > > Regards, > Simon > >> >> >> Thanks, >> Jagan. >> >> > Can you please send the next version patches. -- Thanks, Jagan.
HI Rajeshwari, Can you please send the next version patch w.r.t to current tree. and also fix the comments on last version. Planning to apply on tree, feel free to ask if you have any concerns. -- Thanks, Jagan. On Thu, Jun 13, 2013 at 8:47 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: > On Thu, Jun 13, 2013 at 3:36 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Jagan, >> >> On Wed, Jun 12, 2013 at 12:51 PM, Jagan Teki <jagannadh.teki@gmail.com> >> wrote: >>> >>> Hi, >>> >>> >>> On 30-05-2013 10:49, Rajeshwari Shinde wrote: >>>> >>>> For devices that need some time to react after a spi transaction >>>> finishes, add the ability to set a delay. >>>> >>>> Implement this as a delay on the first/next transaction to avoid >>>> any delay in the fairly common case where a SPI transaction is >>>> followed by other processing. >>>> >>>> Based on: >>>> "[U-Boot] [PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode" >>> >>> >>> Please use the below format for referring previous commits >>> ex: >>> "<PREVIOUS_COMMIT_HEAD>" >>> (sha1: PREVIOUS_COMMIT_ID) >>> >>> in this case >>> "spi: exynos: Support SPI_PREAMBLE mode" >>> (sha1: e4eaef8910df805d511b1cb9b2caafa7d2827fdc) >> >> >> I hope you don't mind me answering part of this... >> >>> >>> >>> >>>> >>>> Signed-off-by: Simon Glass <sjg@chromium.org> >>>> Signed-off-by: Rajeshwari Shinde <rajeshwari.s@samsung.com> >>>> --- >>>> Changes in V2: >>>> - None. >>>> Changes in V3: >>>> - Rebased on "[PATCH 0/2 V5] spi: Enable SPI_PREAMBLE Mode" >>>> drivers/spi/exynos_spi.c | 19 +++++++++++++++++++ >>>> 1 files changed, 19 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c >>>> index 01378d0..03cf503 100644 >>>> --- a/drivers/spi/exynos_spi.c >>>> +++ b/drivers/spi/exynos_spi.c >>>> @@ -38,6 +38,7 @@ struct spi_bus { >>>> struct exynos_spi *regs; >>>> int inited; /* 1 if this bus is ready for use */ >>>> int node; >>>> + uint deactivate_delay_us; /* Delay to wait after deactivate >>>> */ >>>> }; >>>> >>>> /* A list of spi buses that we know about */ >>>> @@ -52,6 +53,8 @@ struct exynos_spi_slave { >>>> enum periph_id periph_id; /* Peripheral ID for this device >>>> */ >>>> unsigned int fifo_size; >>>> int skip_preamble; >>>> + struct spi_bus *bus; /* Pointer to our SPI bus info */ >>>> + ulong last_transaction_us; /* Time of last transaction end >>>> */ >>>> }; >>>> >>>> static struct spi_bus *spi_get_bus(unsigned dev_index) >>>> @@ -97,6 +100,7 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, >>>> unsigned int cs, >>>> } >>>> >>>> bus = &spi_bus[busnum]; >>>> + spi_slave->bus = bus; >>>> spi_slave->regs = bus->regs; >>>> spi_slave->mode = mode; >>>> spi_slave->periph_id = bus->periph_id; >>>> @@ -107,6 +111,7 @@ struct spi_slave *spi_setup_slave(unsigned int >>>> busnum, unsigned int cs, >>>> spi_slave->fifo_size = 256; >>>> >>>> spi_slave->skip_preamble = 0; >>>> + spi_slave->last_transaction_us = timer_get_us(); >>>> >>>> spi_slave->freq = bus->frequency; >>>> if (max_hz) >>>> @@ -371,9 +376,21 @@ void spi_cs_activate(struct spi_slave *slave) >>>> { >>>> struct exynos_spi_slave *spi_slave = to_exynos_spi(slave); >>>> >>>> + /* If it's too soon to do another transaction, wait */ >>>> + if (spi_slave->bus->deactivate_delay_us && >>>> + spi_slave->last_transaction_us) { >>>> + ulong delay_us; /* The delay completed so far */ >>>> + delay_us = timer_get_us() - >>>> spi_slave->last_transaction_us; >>>> + if (delay_us < spi_slave->bus->deactivate_delay_us) >>>> + udelay(spi_slave->bus->deactivate_delay_us - >>>> delay_us); >>>> + } >>> >>> Add one space >>> >>> >>>> clrbits_le32(&spi_slave->regs->cs_reg, SPI_SLAVE_SIG_INACT); >>>> debug("Activate CS, bus %d\n", spi_slave->slave.bus); >>>> spi_slave->skip_preamble = spi_slave->mode & SPI_PREAMBLE; >>>> + >>>> + /* Remember time of this transaction so we can honour the bus >>>> delay */ >>>> + if (spi_slave->bus->deactivate_delay_us) >>>> + spi_slave->last_transaction_us = timer_get_us(); >>>> } >>>> >>>> /** >>>> @@ -423,6 +440,8 @@ static int spi_get_config(const void *blob, int node, >>>> struct spi_bus *bus) >>>> /* Use 500KHz as a suitable default */ >>>> bus->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", >>>> 500000); >>>> + bus->deactivate_delay_us = fdtdec_get_int(blob, node, >>>> + "spi-deactivate-delay", 0); >>>> >>>> return 0; >>>> } >>>> >>> >>> Was this tested, usually what is the max deactivate delay? >> >> >> This was tested with 50us. >> >>> >>> In this case your are passing through dts is it? >> >> >> Yes that's right. >> >> Regards, >> Simon >> >>> >>> >>> Thanks, >>> Jagan. >>> >>> >> > > Can you please send the next version patches. > > -- > Thanks, > Jagan.
Any update on this!
diff --git a/drivers/spi/exynos_spi.c b/drivers/spi/exynos_spi.c index 01378d0..03cf503 100644 --- a/drivers/spi/exynos_spi.c +++ b/drivers/spi/exynos_spi.c @@ -38,6 +38,7 @@ struct spi_bus { struct exynos_spi *regs; int inited; /* 1 if this bus is ready for use */ int node; + uint deactivate_delay_us; /* Delay to wait after deactivate */ }; /* A list of spi buses that we know about */ @@ -52,6 +53,8 @@ struct exynos_spi_slave { enum periph_id periph_id; /* Peripheral ID for this device */ unsigned int fifo_size; int skip_preamble; + struct spi_bus *bus; /* Pointer to our SPI bus info */ + ulong last_transaction_us; /* Time of last transaction end */ }; static struct spi_bus *spi_get_bus(unsigned dev_index) @@ -97,6 +100,7 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, unsigned int cs, } bus = &spi_bus[busnum]; + spi_slave->bus = bus; spi_slave->regs = bus->regs; spi_slave->mode = mode; spi_slave->periph_id = bus->periph_id; @@ -107,6 +111,7 @@ struct spi_slave *spi_setup_slave(unsigned int busnum, unsigned int cs, spi_slave->fifo_size = 256; spi_slave->skip_preamble = 0; + spi_slave->last_transaction_us = timer_get_us(); spi_slave->freq = bus->frequency; if (max_hz) @@ -371,9 +376,21 @@ void spi_cs_activate(struct spi_slave *slave) { struct exynos_spi_slave *spi_slave = to_exynos_spi(slave); + /* If it's too soon to do another transaction, wait */ + if (spi_slave->bus->deactivate_delay_us && + spi_slave->last_transaction_us) { + ulong delay_us; /* The delay completed so far */ + delay_us = timer_get_us() - spi_slave->last_transaction_us; + if (delay_us < spi_slave->bus->deactivate_delay_us) + udelay(spi_slave->bus->deactivate_delay_us - delay_us); + } clrbits_le32(&spi_slave->regs->cs_reg, SPI_SLAVE_SIG_INACT); debug("Activate CS, bus %d\n", spi_slave->slave.bus); spi_slave->skip_preamble = spi_slave->mode & SPI_PREAMBLE; + + /* Remember time of this transaction so we can honour the bus delay */ + if (spi_slave->bus->deactivate_delay_us) + spi_slave->last_transaction_us = timer_get_us(); } /** @@ -423,6 +440,8 @@ static int spi_get_config(const void *blob, int node, struct spi_bus *bus) /* Use 500KHz as a suitable default */ bus->frequency = fdtdec_get_int(blob, node, "spi-max-frequency", 500000); + bus->deactivate_delay_us = fdtdec_get_int(blob, node, + "spi-deactivate-delay", 0); return 0; }