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

[Abbarapu, Venkatesh]

[AMD Official Use Only - AMD Internal Distribution Only]

Hi Prasanth,

> -----Original Message-----
> From: Prasanth Mantena <p-mantena at ti.com>
> Sent: Friday, July 11, 2025 2:00 AM
> To: Simek, Michal <michal.simek at amd.com>
> Cc: Abbarapu, Venkatesh <venkatesh.abbarapu at amd.com>; u-boot at lists.denx.de;
> tudor.ambarus at linaro.org; j-humphreys at ti.com; marex at denx.de;
> jagan at amarulasolutions.com; vigneshr at ti.com; u-kumar1 at ti.com;
> trini at konsulko.com; seanga2 at gmail.com; caleb.connolly at linaro.org;
> sjg at chromium.org; william.zhang at broadcom.com; stefan_b at posteo.net;
> quentin.schulz at cherry.de; Takahiro.Kuwano at infineon.com; git (AMD-Xilinx)
> <git at amd.com>; Ashok Reddy Soma <ashok.reddy.soma at amd.com>
> Subject: Re: [PATCH] spi: cadence_qspi: Set tshsl_ns to at least one sclk_ns
>
> 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 at amd.com>
> > > Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu at 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

If the calculated tshsl_ns value (after potentially subtracting the inherent delay) falls below sclk_ns, this forces it up to sclk_ns. This ensures that even if your initial request or the controller's base delay logic could result in a shorter actual delay, the minimum 1 SCLK idle time is always guaranteed by overriding it. Ensures CS# remains asserted long enough for the slave device to properly latch data. Avoids signal integrity issues due to too short hold times.

Thanks
Venkatesh
>
>
> > >           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