Re: [PATCH] spi: cadence_qspi: Set tshsl_ns to at least one sclk_ns

[Prasanth Mantena <p-mantena@ti.com>]

On 15:07, Michal Simek wrote:
> 
> 
> On 7/2/25 08:57, Venkatesh Yadav Abbarapu wrote:
> > tshsl_ns is the clock delay for chip select deassert. This is the delay in
> > master reference clocks for the length that the master mode chip select
> > outputs are de-asserted between transactions.
> > 
> > The minimum delay is always SCLK period to ensure the chip select is never
> > re-asserted within one SCLK period.
> > 
> > That is why tshsl_ns delay should be at least one sclk_ns value. If it is
> > less than sclk_ns, set it equal to sclk_ns.
> > 
> > Signed-off-by: Ashok Reddy Soma <ashok.reddy.soma@amd.com>
> > Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
> > ---
> >   drivers/spi/cadence_qspi_apb.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
> > index 65fb2d8f9fb..4696c09f754 100644
> > --- a/drivers/spi/cadence_qspi_apb.c
> > +++ b/drivers/spi/cadence_qspi_apb.c
> > @@ -303,6 +303,10 @@ void cadence_qspi_apb_delay(void *reg_base,
> >   		tshsl_ns -= sclk_ns + ref_clk_ns;
> >   	if (tchsh_ns >= sclk_ns + 3 * ref_clk_ns)
> >   		tchsh_ns -= sclk_ns + 3 * ref_clk_ns;
> > +
> > +	if (tshsl_ns < sclk_ns)
> > +		tshsl_ns = sclk_ns;
> > +

Hi Venkatesh,

Just referring to the Controller datasheet, I found this in the
register map for the tshsl delay register field.

Ref : 2.3.4. Device Delay Register
"
The minimum delay for chip select to be de-asserted (CSDA=0)
is:
1 sclk_out + 1 ref_clk to ensure the chip select is never re-
asserted within an sclk_out period.

"
if this delay configured is exactly equals to sclk_out, and if sclk_out
roundsup exactly to ref_clk without a headroom, then the next
transaction would start right after next spike, unless if the controller
adds any delay apart from whats configured in this register.
May be thats why it is having that one ref_clk tick extra added acc to reg map.
Please help me understand this.

Thanks,
Prasanth


> >   	tshsl = DIV_ROUND_UP(tshsl_ns, ref_clk_ns);
> >   	tchsh = DIV_ROUND_UP(tchsh_ns, ref_clk_ns);
> >   	tslch = DIV_ROUND_UP(tslch_ns, ref_clk_ns);
> 
> Applied.
> M