by Rob Herring (Arm)

> On Tue, Mar 26, 2019 at 10:56:39PM -0700, Sowjanya Komatineni wrote:
>
> > Tegra SPI driver default uses SW CS control for transfers and HW CS
> > control can be enabled through SPI client device node DT property
> > nvidia,enable-hw-based-cs and is used only for single transfers.
>
> Why have a property - if this feature works and is useful why would a system ever not want to use it?

> > + single_xfer = list_is_singular(&msg->transfers);
> > list_for_each_entry(xfer, &msg->transfers, transfer_list) {
> > u32 cmd1;
> >
> > reinit_completion(&tspi->xfer_completion);
> >
> > - cmd1 = tegra_spi_setup_transfer_one(spi, xfer, is_first_msg);
> > + cmd1 = tegra_spi_setup_transfer_one(spi, xfer, is_first_msg,
> > + single_xfer);
>
> It's not sufficient to check if there's only one transfer, you also need to make sure that the driver isn't using cs_change to use non-standard chip select handling.

Yes we can avoid DT and force to use HW CS incase of all single transfers without cs_change. Will update in next version.

2019-04-02 04:53:33

by Mark Brown

[permalink] [raw]

Subject: Re: [PATCH V1 19/26] DT bindings: spi: add spi client device properties

On Mon, Apr 01, 2019 at 05:59:57PM +0000, Sowjanya Komatineni wrote:

Please fix your mail client to word wrap within paragraphs at something
substantially less than 80 columns. Doing this makes your messages much
easier to read and reply to.

> > > +spi-client device controller properties:
> > > +- nvidia,cs-setup-clk-count: CS setup timing parameter.
> > > +- nvidia,cs-hold-clk-count: CS hold timing parameter.
> > > +- nvidia,cs-inactive-cycles: CS inactive delay in terms of clock
> > > +between
> > > + transfers.

> > Why are these being done as nVidia specific properties rather than
> > generic ones and why are these being configured in DT rather than by
> > the client driver? If the devices have particular timing
> > requirements for chip select presumably that's going to apply no
> > matter what controller or system they're used with so it seems best
> > to configure this in the client driver and have an API that any
> > controller can implement.

> These are implemented thru DT as Tegra SPI is master and master controls the timing.
> Some SPI slaves have specific requirements of certain CS setup/hold time and inactive cycles which SPI master should meet when driving during transfer and Tegra SPI controller supports tuning these parameters.

This doesn't address the issue at all, obviously the timings for the SPI
bus are going to be impelemented by the controller but that's not the
issue.

Attachments:

(No filename) (1.42 kB)
signature.asc (499.00 B)
Download all attachments

2019-04-02 04:54:32

by Mark Brown

[permalink] [raw]

Subject: Re: [PATCH V1 24/26] spi: tegra114: de-assert CS before SPI mode is reset to its default

On Mon, Apr 01, 2019 at 06:07:45PM +0000, Sowjanya Komatineni wrote:

> I see you have applied some patches in V1 series so should I re-send
> again those as well along with feedback changes in next version or
> just only the patches that are not applied.

Please don't resend already applied patches.

Attachments:

(No filename) (309.00 B)
signature.asc (499.00 B)
Download all attachments

2019-04-02 20:30:48

by Sowjanya Komatineni

[permalink] [raw]

Subject: RE: [PATCH V1 21/26] DT bindings: spi: add tx/rx clock delay SPI client properties

> On Tue, Mar 26, 2019 at 10:56:42PM -0700, Sowjanya Komatineni wrote:
> > This patch adds Tegra SPI master tx and rx clock delay properties.
> >
> > TX/RX clock delays may vary depending on the platform design trace
> > lengths for each client on the Tegra SPI bus. These properties helps
> > to tune the clock delays.
> >
> > Signed-off-by: Sowjanya Komatineni <[email protected]>
> > ---
> > .../devicetree/bindings/spi/nvidia,tegra114-spi.txt | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
>
> Just combine this with patch 19.
>
> > diff --git
> > a/Documentation/devicetree/bindings/spi/nvidia,tegra114-spi.txt
> > b/Documentation/devicetree/bindings/spi/nvidia,tegra114-spi.txt
> > index 6167c5234b64..2b84b7b726ce 100644
> > --- a/Documentation/devicetree/bindings/spi/nvidia,tegra114-spi.txt
> > +++ b/Documentation/devicetree/bindings/spi/nvidia,tegra114-spi.txt
> > @@ -29,6 +29,12 @@ spi-client device controller properties:
> > - nvidia,cs-hold-clk-count: CS hold timing parameter.
> > - nvidia,cs-inactive-cycles: CS inactive delay in terms of clock between
> > transfers.
> > +- nvidia,tx-clk-tap-delay: Delays the clock going out to the external
> > +device
> > + with this tap value. This property is used to tune the outgoing
> > +data from
> > + Tegra SPI master with respect to outgoing Tegra SPI master clock.
> > +- nvidia,rx-clk-tap-delay: Delays the clock coming in from the
> > +external device
> > + with this tap value. This property is used to adjust the Tegra SPI
> > +master
> > + clock with respect to the data from the SPI slave device.
>
> Are there units? What's the range of values.

TX/RX Clock delays are tap counts and there is internal tap-to-tap delay.
Will update to specify tap to delay correlation along with supported tap values range.

As per feedback from mark, will move CS timing to API implementation as they are not
Tegra SPI specific and applicable in general.

>
> >
> > Example:
> >
> > @@ -45,4 +51,14 @@ spi@7000d600 {
> > reset-names = "spi";
> > dmas = <&apbdma 16>, <&apbdma 16>;
> > dma-names = "rx", "tx";
> > +
> > + <spi-client>@<bus_num> {
> > + ...
> > + ...
> > + nvidia,cs-setup-clk-count = <10>;
> > + nvidia,cs-hold-clk-count = <10>;
> > + nvidia,rx-clk-tap-delay = <0>;
> > + nvidia,tx-clk-tap-delay = <16>;
> > + ...
> > + };
> > };
> > --
> > 2.7.4
> >