RE: [PATCH V1 18/26] spi: tegra114: add support for hw based cs

[Sowjanya Komatineni]

> 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(>transfers);
> > list_for_each_entry(xfer, >transfers, transfer_list) {
> > u32 cmd1;
> >  
> > reinit_completion(>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.

Re: [PATCH V1 18/26] spi: tegra114: add support for hw based cs

[Mark Brown]

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(>transfers);
>   list_for_each_entry(xfer, >transfers, transfer_list) {
>   u32 cmd1;
>  
>   reinit_completion(>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.


signature.asc
Description: PGP signature

[PATCH V1 18/26] spi: tegra114: add support for hw based cs

[Sowjanya Komatineni]

This patch adds support for HW based CS control.

Tegra SPI controller supports both HW and SW based CS control
transfers.

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.

Signed-off-by: Sowjanya Komatineni 
---
 drivers/spi/spi-tegra114.c | 71 +-
 1 file changed, 64 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-tegra114.c b/drivers/spi/spi-tegra114.c
index 68fee474580b..86c34f02d13a 100644
--- a/drivers/spi/spi-tegra114.c
+++ b/drivers/spi/spi-tegra114.c
@@ -167,6 +167,10 @@ struct tegra_spi_soc_data {
bool has_intr_mask_reg;
 };
 
+struct tegra_spi_client_data {
+   bool is_hw_based_cs;
+};
+
 struct tegra_spi_data {
struct device   *dev;
struct spi_master   *master;
@@ -193,6 +197,7 @@ struct tegra_spi_data {
unsigneddma_buf_size;
unsignedmax_buf_size;
boolis_curr_dma_xfer;
+   booluse_hw_based_cs;
 
struct completion   rx_dma_complete;
struct completion   tx_dma_complete;
@@ -723,9 +728,11 @@ static void tegra_spi_deinit_dma_param(struct 
tegra_spi_data *tspi,
 }
 
 static u32 tegra_spi_setup_transfer_one(struct spi_device *spi,
-   struct spi_transfer *t, bool is_first_of_msg)
+   struct spi_transfer *t, bool is_first_of_msg,
+   bool is_single_xfer)
 {
struct tegra_spi_data *tspi = spi_master_get_devdata(spi->master);
+   struct tegra_spi_client_data *cdata = spi->controller_data;
u32 speed = t->speed_hz;
u8 bits_per_word = t->bits_per_word;
u32 command1;
@@ -776,11 +783,19 @@ static u32 tegra_spi_setup_transfer_one(struct spi_device 
*spi,
} else
tegra_spi_writel(tspi, command1, SPI_COMMAND1);
 
-   command1 |= SPI_CS_SW_HW;
-   if (spi->mode & SPI_CS_HIGH)
-   command1 |= SPI_CS_SW_VAL;
-   else
-   command1 &= ~SPI_CS_SW_VAL;
+   tspi->use_hw_based_cs = false;
+   if (cdata && cdata->is_hw_based_cs && is_single_xfer)
+   tspi->use_hw_based_cs = true;
+
+   if (!tspi->use_hw_based_cs) {
+   command1 |= SPI_CS_SW_HW;
+   if (spi->mode & SPI_CS_HIGH)
+   command1 |= SPI_CS_SW_VAL;
+   else
+   command1 &= ~SPI_CS_SW_VAL;
+   } else {
+   command1 &= ~(SPI_CS_SW_HW | SPI_CS_SW_VAL);
+   }
 
tegra_spi_writel(tspi, 0, SPI_COMMAND2);
} else {
@@ -838,9 +853,41 @@ static int tegra_spi_start_transfer_one(struct spi_device 
*spi,
return ret;
 }
 
+static struct tegra_spi_client_data
+   *tegra_spi_parse_cdata_dt(struct spi_device *spi)
+{
+   struct tegra_spi_client_data *cdata;
+   struct device_node *slave_np;
+
+   slave_np = spi->dev.of_node;
+   if (!slave_np) {
+   dev_dbg(>dev, "device node not found\n");
+   return NULL;
+   }
+
+   cdata = kzalloc(sizeof(*cdata), GFP_KERNEL);
+   if (!cdata)
+   return NULL;
+
+   if (of_property_read_bool(slave_np, "nvidia,enable-hw-based-cs"))
+   cdata->is_hw_based_cs = true;
+
+   return cdata;
+}
+
+static void tegra_spi_cleanup(struct spi_device *spi)
+{
+   struct tegra_spi_client_data *cdata = spi->controller_data;
+
+   spi->controller_data = NULL;
+   if (spi->dev.of_node)
+   kfree(cdata);
+}
+
 static int tegra_spi_setup(struct spi_device *spi)
 {
struct tegra_spi_data *tspi = spi_master_get_devdata(spi->master);
+   struct tegra_spi_client_data *cdata = spi->controller_data;
u32 val;
unsigned long flags;
int ret;
@@ -851,9 +898,15 @@ static int tegra_spi_setup(struct spi_device *spi)
spi->mode & SPI_CPHA ? "" : "~",
spi->max_speed_hz);
 
+   if (!cdata) {
+   cdata = tegra_spi_parse_cdata_dt(spi);
+   spi->controller_data = cdata;
+   }
+
ret = pm_runtime_get_sync(tspi->dev);
if (ret < 0) {
dev_err(tspi->dev, "pm runtime failed, e = %d\n", ret);
+   tegra_spi_cleanup(spi);
return ret;
}
 
@@ -909,18 +962,21 @@ static int tegra_spi_transfer_one_message(struct 
spi_master *master,
struct tegra_spi_data *tspi = spi_master_get_devdata(master);
struct spi_transfer *xfer;
struct spi_device *spi = msg->spi;
+   int single_xfer;