diff mbox series

[RFC,1/6] spi: spi-mem: Tell controller when device is ready for calibration

Message ID 20210311191216.7363-2-p.yadav@ti.com
State Not Applicable
Delegated to: Ambarus Tudor
Headers show
Series spi: Add OSPI PHY calibration support for spi-cadence-quadspi | expand

Commit Message

Pratyush Yadav March 11, 2021, 7:12 p.m. UTC
Some controllers like the Cadence OSPI controller need to perform a
calibration sequence to operate at high clock speeds. This calibration
should happen after the flash is fully initialized otherwise the
calibration might happen in a different SPI mode from the one the flash
is finally set to. Add a hook that can be used to tell the controller
when the flash is ready for calibration. Whether calibration is needed
depends on the controller.

Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
---
 drivers/spi/spi-mem.c       | 12 ++++++++++++
 include/linux/spi/spi-mem.h |  8 ++++++++
 2 files changed, 20 insertions(+)

Comments

Michael Walle March 23, 2021, 11:07 p.m. UTC | #1
Hi Pratyush,

Am 2021-03-11 20:12, schrieb Pratyush Yadav:
> Some controllers like the Cadence OSPI controller need to perform a
> calibration sequence to operate at high clock speeds. This calibration
> should happen after the flash is fully initialized otherwise the
> calibration might happen in a different SPI mode from the one the flash
> is finally set to. Add a hook that can be used to tell the controller
> when the flash is ready for calibration. Whether calibration is needed
> depends on the controller.
> 
> Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> ---
>  drivers/spi/spi-mem.c       | 12 ++++++++++++
>  include/linux/spi/spi-mem.h |  8 ++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> index dc713b0c3c4d..e2f05ad3f4dc 100644
> --- a/drivers/spi/spi-mem.c
> +++ b/drivers/spi/spi-mem.c
> @@ -464,6 +464,18 @@ int spi_mem_adjust_op_size(struct spi_mem *mem,
> struct spi_mem_op *op)
>  }
>  EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
> 
> +int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op)
> +{
> +	struct spi_controller *ctlr = mem->spi->controller;
> +
> +	if (!ctlr->mem_ops || !ctlr->mem_ops->do_calibration)
> +		return -EOPNOTSUPP;
> +
> +	ctlr->mem_ops->do_calibration(mem, op);

Can't a calibration fail?

> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(spi_mem_do_calibration);
> +
>  static ssize_t spi_mem_no_dirmap_read(struct spi_mem_dirmap_desc 
> *desc,
>  				      u64 offs, size_t len, void *buf)
>  {
> diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
> index 2b65c9edc34e..97a2d280f2d0 100644
> --- a/include/linux/spi/spi-mem.h
> +++ b/include/linux/spi/spi-mem.h
> @@ -250,6 +250,12 @@ static inline void *spi_mem_get_drvdata(struct
> spi_mem *mem)
>   *		  the currently mapped area), and the caller of
>   *		  spi_mem_dirmap_write() is responsible for calling it again in
>   *		  this case.
> + * @do_calibration: perform calibration needed for high SPI clock 
> speed
> + *		    operation. Should be called after the SPI memory device has
> + *		    been completely initialized. The op passed should contain
> + *		    a template for the read operation used for the device so
> + *		    the controller can decide what type of calibration is
> + *		    required for this type of read.
>   *
>   * This interface should be implemented by SPI controllers providing 
> an
>   * high-level interface to execute SPI memory operation, which is 
> usually the
> @@ -274,6 +280,7 @@ struct spi_controller_mem_ops {
>  			       u64 offs, size_t len, void *buf);
>  	ssize_t (*dirmap_write)(struct spi_mem_dirmap_desc *desc,
>  				u64 offs, size_t len, const void *buf);
> +	void (*do_calibration)(struct spi_mem *mem, struct spi_mem_op *op);
>  };
> 
>  /**
> @@ -346,6 +353,7 @@ bool spi_mem_dtr_supports_op(struct spi_mem *mem,
>  #endif /* CONFIG_SPI_MEM */
> 
>  int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op 
> *op);
> +int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op 
> *op);
> 
>  bool spi_mem_supports_op(struct spi_mem *mem,
>  			 const struct spi_mem_op *op);
Pratyush Yadav March 24, 2021, 8:08 a.m. UTC | #2
On 24/03/21 12:07AM, Michael Walle wrote:
> Hi Pratyush,
> 
> Am 2021-03-11 20:12, schrieb Pratyush Yadav:
> > Some controllers like the Cadence OSPI controller need to perform a
> > calibration sequence to operate at high clock speeds. This calibration
> > should happen after the flash is fully initialized otherwise the
> > calibration might happen in a different SPI mode from the one the flash
> > is finally set to. Add a hook that can be used to tell the controller
> > when the flash is ready for calibration. Whether calibration is needed
> > depends on the controller.
> > 
> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > ---
> >  drivers/spi/spi-mem.c       | 12 ++++++++++++
> >  include/linux/spi/spi-mem.h |  8 ++++++++
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> > index dc713b0c3c4d..e2f05ad3f4dc 100644
> > --- a/drivers/spi/spi-mem.c
> > +++ b/drivers/spi/spi-mem.c
> > @@ -464,6 +464,18 @@ int spi_mem_adjust_op_size(struct spi_mem *mem,
> > struct spi_mem_op *op)
> >  }
> >  EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
> > 
> > +int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op)
> > +{
> > +	struct spi_controller *ctlr = mem->spi->controller;
> > +
> > +	if (!ctlr->mem_ops || !ctlr->mem_ops->do_calibration)
> > +		return -EOPNOTSUPP;
> > +
> > +	ctlr->mem_ops->do_calibration(mem, op);
> 
> Can't a calibration fail?

It can. If it does, the controller falls back to lower speed transfers. 
There is not much the upper layer can do about this. That's why it is 
not informed whether it succeeded or not.

> 
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(spi_mem_do_calibration);
> > +
> >  static ssize_t spi_mem_no_dirmap_read(struct spi_mem_dirmap_desc *desc,
> >  				      u64 offs, size_t len, void *buf)
> >  {
> > diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
> > index 2b65c9edc34e..97a2d280f2d0 100644
> > --- a/include/linux/spi/spi-mem.h
> > +++ b/include/linux/spi/spi-mem.h
> > @@ -250,6 +250,12 @@ static inline void *spi_mem_get_drvdata(struct
> > spi_mem *mem)
> >   *		  the currently mapped area), and the caller of
> >   *		  spi_mem_dirmap_write() is responsible for calling it again in
> >   *		  this case.
> > + * @do_calibration: perform calibration needed for high SPI clock speed
> > + *		    operation. Should be called after the SPI memory device has
> > + *		    been completely initialized. The op passed should contain
> > + *		    a template for the read operation used for the device so
> > + *		    the controller can decide what type of calibration is
> > + *		    required for this type of read.
> >   *
> >   * This interface should be implemented by SPI controllers providing an
> >   * high-level interface to execute SPI memory operation, which is
> > usually the
> > @@ -274,6 +280,7 @@ struct spi_controller_mem_ops {
> >  			       u64 offs, size_t len, void *buf);
> >  	ssize_t (*dirmap_write)(struct spi_mem_dirmap_desc *desc,
> >  				u64 offs, size_t len, const void *buf);
> > +	void (*do_calibration)(struct spi_mem *mem, struct spi_mem_op *op);
> >  };
> > 
> >  /**
> > @@ -346,6 +353,7 @@ bool spi_mem_dtr_supports_op(struct spi_mem *mem,
> >  #endif /* CONFIG_SPI_MEM */
> > 
> >  int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op);
> > +int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op);
> > 
> >  bool spi_mem_supports_op(struct spi_mem *mem,
> >  			 const struct spi_mem_op *op);
> 
> -- 
> -michael
Michael Walle April 29, 2021, 4:23 p.m. UTC | #3
Hi Pratyush,

I've had a look at the LS1028A FlexSPI calibration feature. The
reference manual is very sparse on details, though. What you need to
do there is to program a special read command sequence (the whole
controller is made of these lookup table entries, where you can
have a short sequence of operations for read/write/program and so
on). Therefore, for data learning you'll take the read operation
and insert a LEARN op in between and read a specific data pattern.
Then the hardware will automatically figure out the correct sample
phase for the read data pins.

Unfortunately, it does not mention how often you have to do it. It
might be the case that is has to be calibrated more than once.

I'm just mentioning this so it won't be lost. If needed, it can
be added later.

Am 2021-03-24 09:08, schrieb Pratyush Yadav:
> On 24/03/21 12:07AM, Michael Walle wrote:
>> Am 2021-03-11 20:12, schrieb Pratyush Yadav:
>> > Some controllers like the Cadence OSPI controller need to perform a
>> > calibration sequence to operate at high clock speeds. This calibration
>> > should happen after the flash is fully initialized otherwise the
>> > calibration might happen in a different SPI mode from the one the flash
>> > is finally set to. Add a hook that can be used to tell the controller
>> > when the flash is ready for calibration. Whether calibration is needed
>> > depends on the controller.
>> >
>> > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
>> > ---
>> >  drivers/spi/spi-mem.c       | 12 ++++++++++++
>> >  include/linux/spi/spi-mem.h |  8 ++++++++
>> >  2 files changed, 20 insertions(+)
>> >
>> > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
>> > index dc713b0c3c4d..e2f05ad3f4dc 100644
>> > --- a/drivers/spi/spi-mem.c
>> > +++ b/drivers/spi/spi-mem.c
>> > @@ -464,6 +464,18 @@ int spi_mem_adjust_op_size(struct spi_mem *mem,
>> > struct spi_mem_op *op)
>> >  }
>> >  EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
>> >
>> > +int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op)
>> > +{
>> > +	struct spi_controller *ctlr = mem->spi->controller;
>> > +
>> > +	if (!ctlr->mem_ops || !ctlr->mem_ops->do_calibration)
>> > +		return -EOPNOTSUPP;
>> > +
>> > +	ctlr->mem_ops->do_calibration(mem, op);
>> 
>> Can't a calibration fail?
> 
> It can. If it does, the controller falls back to lower speed transfers.
> There is not much the upper layer can do about this. That's why it is
> not informed whether it succeeded or not.

Ok, if needed, that should be an easy change.

op is there to decide if we need a calibration at all, correct?
What if there are different factors, like frequency? For example
on the LS1028A its just a matter of the SCK frequency. It seems
that this parameter is tailored to the OPHY.

-michael

>> > +	return 0;
>> > +}
>> > +EXPORT_SYMBOL_GPL(spi_mem_do_calibration);
>> > +
>> >  static ssize_t spi_mem_no_dirmap_read(struct spi_mem_dirmap_desc *desc,
>> >  				      u64 offs, size_t len, void *buf)
>> >  {
>> > diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
>> > index 2b65c9edc34e..97a2d280f2d0 100644
>> > --- a/include/linux/spi/spi-mem.h
>> > +++ b/include/linux/spi/spi-mem.h
>> > @@ -250,6 +250,12 @@ static inline void *spi_mem_get_drvdata(struct
>> > spi_mem *mem)
>> >   *		  the currently mapped area), and the caller of
>> >   *		  spi_mem_dirmap_write() is responsible for calling it again in
>> >   *		  this case.
>> > + * @do_calibration: perform calibration needed for high SPI clock speed
>> > + *		    operation. Should be called after the SPI memory device has
>> > + *		    been completely initialized. The op passed should contain
>> > + *		    a template for the read operation used for the device so
>> > + *		    the controller can decide what type of calibration is
>> > + *		    required for this type of read.
>> >   *
>> >   * This interface should be implemented by SPI controllers providing an
>> >   * high-level interface to execute SPI memory operation, which is
>> > usually the
>> > @@ -274,6 +280,7 @@ struct spi_controller_mem_ops {
>> >  			       u64 offs, size_t len, void *buf);
>> >  	ssize_t (*dirmap_write)(struct spi_mem_dirmap_desc *desc,
>> >  				u64 offs, size_t len, const void *buf);
>> > +	void (*do_calibration)(struct spi_mem *mem, struct spi_mem_op *op);
>> >  };
>> >
>> >  /**
>> > @@ -346,6 +353,7 @@ bool spi_mem_dtr_supports_op(struct spi_mem *mem,
>> >  #endif /* CONFIG_SPI_MEM */
>> >
>> >  int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op);
>> > +int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op);
>> >
>> >  bool spi_mem_supports_op(struct spi_mem *mem,
>> >  			 const struct spi_mem_op *op);
>>
Pratyush Yadav April 29, 2021, 6:41 p.m. UTC | #4
On 29/04/21 06:23PM, Michael Walle wrote:
> Hi Pratyush,
> 
> I've had a look at the LS1028A FlexSPI calibration feature. The
> reference manual is very sparse on details, though. What you need to
> do there is to program a special read command sequence (the whole
> controller is made of these lookup table entries, where you can
> have a short sequence of operations for read/write/program and so
> on). Therefore, for data learning you'll take the read operation
> and insert a LEARN op in between and read a specific data pattern.
> Then the hardware will automatically figure out the correct sample
> phase for the read data pins.
> 
> Unfortunately, it does not mention how often you have to do it. It
> might be the case that is has to be calibrated more than once.

I haven't read the datasheet, I wonder how long this calibration takes. 
If it is too long then the overhead might not even be worth the extra 
read throughput. Especially when using a file system on top which 
generally don't do very large reads in one go.

Anyway, when the do_calibration() is called the controller can save the 
calibration op and use it later as needed. It knows when an exec_op() 
will result in a read since it has access to the whole op.

> 
> I'm just mentioning this so it won't be lost. If needed, it can
> be added later.
> 
> Am 2021-03-24 09:08, schrieb Pratyush Yadav:
> > On 24/03/21 12:07AM, Michael Walle wrote:
> > > Am 2021-03-11 20:12, schrieb Pratyush Yadav:
> > > > Some controllers like the Cadence OSPI controller need to perform a
> > > > calibration sequence to operate at high clock speeds. This calibration
> > > > should happen after the flash is fully initialized otherwise the
> > > > calibration might happen in a different SPI mode from the one the flash
> > > > is finally set to. Add a hook that can be used to tell the controller
> > > > when the flash is ready for calibration. Whether calibration is needed
> > > > depends on the controller.
> > > >
> > > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
> > > > ---
> > > >  drivers/spi/spi-mem.c       | 12 ++++++++++++
> > > >  include/linux/spi/spi-mem.h |  8 ++++++++
> > > >  2 files changed, 20 insertions(+)
> > > >
> > > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> > > > index dc713b0c3c4d..e2f05ad3f4dc 100644
> > > > --- a/drivers/spi/spi-mem.c
> > > > +++ b/drivers/spi/spi-mem.c
> > > > @@ -464,6 +464,18 @@ int spi_mem_adjust_op_size(struct spi_mem *mem,
> > > > struct spi_mem_op *op)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
> > > >
> > > > +int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op)
> > > > +{
> > > > +	struct spi_controller *ctlr = mem->spi->controller;
> > > > +
> > > > +	if (!ctlr->mem_ops || !ctlr->mem_ops->do_calibration)
> > > > +		return -EOPNOTSUPP;
> > > > +
> > > > +	ctlr->mem_ops->do_calibration(mem, op);
> > > 
> > > Can't a calibration fail?
> > 
> > It can. If it does, the controller falls back to lower speed transfers.
> > There is not much the upper layer can do about this. That's why it is
> > not informed whether it succeeded or not.
> 
> Ok, if needed, that should be an easy change.
> 
> op is there to decide if we need a calibration at all, correct?

Yes. It can also be used to choose which calibration algorithm to use. 
For example on the Cadence controller, there are different algorithms 
for 8S and 8D operations.

> What if there are different factors, like frequency? For example
> on the LS1028A its just a matter of the SCK frequency. It seems
> that this parameter is tailored to the OPHY.

As of now there is no way in SPI MEM to tell the controller the expected 
speed of the operation. AFAIK most controllers get the speed via device 
tree. So in the current case, the controller already knows the speed it 
should run at, and can decide if calibration is needed or not.

But if operation speed is eventually added to SPI MEM, I would assume it 
would be part of struct spi_mem_op. The op passed in would have this 
information filled, and the controller can use that information to 
decide if it needs to perform the calibration or not.

I am all for making this API flexible, but with very few controllers 
supporting this feature in the wild, it is difficult to predict all the 
information that might be needed. In the current state, I think the API 
provides a fair bit of information to the controller about how a read 
operation would look like.

> 
> -michael
> 
> > > > +	return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(spi_mem_do_calibration);
> > > > +
> > > >  static ssize_t spi_mem_no_dirmap_read(struct spi_mem_dirmap_desc *desc,
> > > >  				      u64 offs, size_t len, void *buf)
> > > >  {
> > > > diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
> > > > index 2b65c9edc34e..97a2d280f2d0 100644
> > > > --- a/include/linux/spi/spi-mem.h
> > > > +++ b/include/linux/spi/spi-mem.h
> > > > @@ -250,6 +250,12 @@ static inline void *spi_mem_get_drvdata(struct
> > > > spi_mem *mem)
> > > >   *		  the currently mapped area), and the caller of
> > > >   *		  spi_mem_dirmap_write() is responsible for calling it again in
> > > >   *		  this case.
> > > > + * @do_calibration: perform calibration needed for high SPI clock speed
> > > > + *		    operation. Should be called after the SPI memory device has
> > > > + *		    been completely initialized. The op passed should contain
> > > > + *		    a template for the read operation used for the device so
> > > > + *		    the controller can decide what type of calibration is
> > > > + *		    required for this type of read.
> > > >   *
> > > >   * This interface should be implemented by SPI controllers providing an
> > > >   * high-level interface to execute SPI memory operation, which is
> > > > usually the
> > > > @@ -274,6 +280,7 @@ struct spi_controller_mem_ops {
> > > >  			       u64 offs, size_t len, void *buf);
> > > >  	ssize_t (*dirmap_write)(struct spi_mem_dirmap_desc *desc,
> > > >  				u64 offs, size_t len, const void *buf);
> > > > +	void (*do_calibration)(struct spi_mem *mem, struct spi_mem_op *op);
> > > >  };
> > > >
> > > >  /**
> > > > @@ -346,6 +353,7 @@ bool spi_mem_dtr_supports_op(struct spi_mem *mem,
> > > >  #endif /* CONFIG_SPI_MEM */
> > > >
> > > >  int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op);
> > > > +int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op);
> > > >
> > > >  bool spi_mem_supports_op(struct spi_mem *mem,
> > > >  			 const struct spi_mem_op *op);
> > > 
>
Michael Walle April 29, 2021, 10:46 p.m. UTC | #5
Am 2021-04-29 20:41, schrieb Pratyush Yadav:
> On 29/04/21 06:23PM, Michael Walle wrote:
>> I've had a look at the LS1028A FlexSPI calibration feature. The
>> reference manual is very sparse on details, though. What you need to
>> do there is to program a special read command sequence (the whole
>> controller is made of these lookup table entries, where you can
>> have a short sequence of operations for read/write/program and so
>> on). Therefore, for data learning you'll take the read operation
>> and insert a LEARN op in between and read a specific data pattern.
>> Then the hardware will automatically figure out the correct sample
>> phase for the read data pins.
>> 
>> Unfortunately, it does not mention how often you have to do it. It
>> might be the case that is has to be calibrated more than once.
> 
> I haven't read the datasheet, I wonder how long this calibration takes.
> If it is too long then the overhead might not even be worth the extra
> read throughput. Especially when using a file system on top which
> generally don't do very large reads in one go.

I was just thinking of compensating a possible temperature drift. You
wouldn't have to do it on every read.

There is a second mode, where it is actually done on every read. But
that will be used where the flash supports a read preamble, where
dummy bytes after the read opcode are replaced by a calibration pattern.
If the pattern has the same length as the dummy bytes there is no 
penalty.
IIRC the controller just supports a pattern of max 32 bits.

Oh I forgot to mention, this doesn't need to be repeated. I guess
the hardware already captures all possible phases (there are only
16) and compares each one with a predefined pattern.

> Anyway, when the do_calibration() is called the controller can save the
> calibration op and use it later as needed. It knows when an exec_op()
> will result in a read since it has access to the whole op.
> 
>> 
>> I'm just mentioning this so it won't be lost. If needed, it can
>> be added later.
>> 
>> Am 2021-03-24 09:08, schrieb Pratyush Yadav:
>> > On 24/03/21 12:07AM, Michael Walle wrote:
>> > > Am 2021-03-11 20:12, schrieb Pratyush Yadav:
>> > > > Some controllers like the Cadence OSPI controller need to perform a
>> > > > calibration sequence to operate at high clock speeds. This calibration
>> > > > should happen after the flash is fully initialized otherwise the
>> > > > calibration might happen in a different SPI mode from the one the flash
>> > > > is finally set to. Add a hook that can be used to tell the controller
>> > > > when the flash is ready for calibration. Whether calibration is needed
>> > > > depends on the controller.
>> > > >
>> > > > Signed-off-by: Pratyush Yadav <p.yadav@ti.com>
>> > > > ---
>> > > >  drivers/spi/spi-mem.c       | 12 ++++++++++++
>> > > >  include/linux/spi/spi-mem.h |  8 ++++++++
>> > > >  2 files changed, 20 insertions(+)
>> > > >
>> > > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
>> > > > index dc713b0c3c4d..e2f05ad3f4dc 100644
>> > > > --- a/drivers/spi/spi-mem.c
>> > > > +++ b/drivers/spi/spi-mem.c
>> > > > @@ -464,6 +464,18 @@ int spi_mem_adjust_op_size(struct spi_mem *mem,
>> > > > struct spi_mem_op *op)
>> > > >  }
>> > > >  EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
>> > > >
>> > > > +int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op)
>> > > > +{
>> > > > +	struct spi_controller *ctlr = mem->spi->controller;
>> > > > +
>> > > > +	if (!ctlr->mem_ops || !ctlr->mem_ops->do_calibration)
>> > > > +		return -EOPNOTSUPP;
>> > > > +
>> > > > +	ctlr->mem_ops->do_calibration(mem, op);
>> > >
>> > > Can't a calibration fail?
>> >
>> > It can. If it does, the controller falls back to lower speed transfers.
>> > There is not much the upper layer can do about this. That's why it is
>> > not informed whether it succeeded or not.
>> 
>> Ok, if needed, that should be an easy change.
>> 
>> op is there to decide if we need a calibration at all, correct?
> 
> Yes. It can also be used to choose which calibration algorithm to use.
> For example on the Cadence controller, there are different algorithms
> for 8S and 8D operations.
> 
>> What if there are different factors, like frequency? For example
>> on the LS1028A its just a matter of the SCK frequency. It seems
>> that this parameter is tailored to the OPHY.
> 
> As of now there is no way in SPI MEM to tell the controller the 
> expected
> speed of the operation. AFAIK most controllers get the speed via device
> tree. So in the current case, the controller already knows the speed it
> should run at, and can decide if calibration is needed or not.
> 
> But if operation speed is eventually added to SPI MEM, I would assume 
> it
> would be part of struct spi_mem_op. The op passed in would have this
> information filled, and the controller can use that information to
> decide if it needs to perform the calibration or not.
> 
> I am all for making this API flexible, but with very few controllers
> supporting this feature in the wild, it is difficult to predict all the
> information that might be needed. In the current state, I think the API
> provides a fair bit of information to the controller about how a read
> operation would look like.

Sure, and its also quite hard to review without any other hardware
which supports that ;) I was thinking about letting the spi driver
call into spi-mem to retrieve the information it needs instead of
having that second argument. Anyway, if we need any changes in the
future this isn't set in stone.

-michael
diff mbox series

Patch

diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
index dc713b0c3c4d..e2f05ad3f4dc 100644
--- a/drivers/spi/spi-mem.c
+++ b/drivers/spi/spi-mem.c
@@ -464,6 +464,18 @@  int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 }
 EXPORT_SYMBOL_GPL(spi_mem_adjust_op_size);
 
+int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op)
+{
+	struct spi_controller *ctlr = mem->spi->controller;
+
+	if (!ctlr->mem_ops || !ctlr->mem_ops->do_calibration)
+		return -EOPNOTSUPP;
+
+	ctlr->mem_ops->do_calibration(mem, op);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(spi_mem_do_calibration);
+
 static ssize_t spi_mem_no_dirmap_read(struct spi_mem_dirmap_desc *desc,
 				      u64 offs, size_t len, void *buf)
 {
diff --git a/include/linux/spi/spi-mem.h b/include/linux/spi/spi-mem.h
index 2b65c9edc34e..97a2d280f2d0 100644
--- a/include/linux/spi/spi-mem.h
+++ b/include/linux/spi/spi-mem.h
@@ -250,6 +250,12 @@  static inline void *spi_mem_get_drvdata(struct spi_mem *mem)
  *		  the currently mapped area), and the caller of
  *		  spi_mem_dirmap_write() is responsible for calling it again in
  *		  this case.
+ * @do_calibration: perform calibration needed for high SPI clock speed
+ *		    operation. Should be called after the SPI memory device has
+ *		    been completely initialized. The op passed should contain
+ *		    a template for the read operation used for the device so
+ *		    the controller can decide what type of calibration is
+ *		    required for this type of read.
  *
  * This interface should be implemented by SPI controllers providing an
  * high-level interface to execute SPI memory operation, which is usually the
@@ -274,6 +280,7 @@  struct spi_controller_mem_ops {
 			       u64 offs, size_t len, void *buf);
 	ssize_t (*dirmap_write)(struct spi_mem_dirmap_desc *desc,
 				u64 offs, size_t len, const void *buf);
+	void (*do_calibration)(struct spi_mem *mem, struct spi_mem_op *op);
 };
 
 /**
@@ -346,6 +353,7 @@  bool spi_mem_dtr_supports_op(struct spi_mem *mem,
 #endif /* CONFIG_SPI_MEM */
 
 int spi_mem_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op);
+int spi_mem_do_calibration(struct spi_mem *mem, struct spi_mem_op *op);
 
 bool spi_mem_supports_op(struct spi_mem *mem,
 			 const struct spi_mem_op *op);