[PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Jane Wan]

Make FSL eSPI properties configurable through device tree.  The
configurable parameters include CSnBEF and CSnAFT in ESPI_SPMODEn
registers (n=0,1,2,3), and whether to send all received data to user.

The FSL eSPI driver hardcodes CSnBEF and CSnAFT to 0.  Some device
needs to set them to different values.

When user sends n_tx bytes and receives n_rx bytes on FSL eSPI
interface, the FSL eSPI driver sends (n_tx + n_rx) bytes on MOSI.
Simultaniously, there are (n_tx + n_rx) received on MISO.  The FSL
eSPI driver only passes the last n_rx bytes to user device driver.
The other n_tx bytes received from the slave device are dropped.  
Some device has issue with this mechanism.  The device driver
requires to know all bytes that the slave device puts on MISO.

Part of this fix is based on a previous patch that was not included,
see http://www.mail-archive.com/spi-devel-general@lists.sourceforge.net/msg08658.html

Test done:
- Kernel version 3.8.13
- Three devices attached to FSL eSPI interface with chip select 0-2.
- The device at cs0 and cs1 work with original FSL eSPI driver.
- The device driver at cs2 has issue with original FSL eSPI driver.
  It requires to receive all data that the slave device puts on MISO.
  It also requires CS2BEF in SPI_SPMODE2 register to be set to 1.
- After set proper values in the device tree (as example below), all
  three  devices work fine with the FSL eSPI driver.
        spi@110000 {
                redpine@2 {
                        #address-cells = <1>;
                        #size-cells = <1>;
                        compatible = "spidev";
                        reg = <2>;
                        spi-max-frequency = <10000000>; /* input clock */
                        fsl,csbef = <1>; /* CS assertiion time in bits before frame start */
                        fsl,csaft = <1>; /* CS assertiion time in bits after frame end */
                        fsl,spi-raw-rxdata-to-user = <1>; /* send all rx data to user */
                };
        };


Jane Wan (1):
  Configure fsl espi CSBEF, CSAFT, and whether to send all received
    data to user

 drivers/spi/spi-fsl-espi.c       |   68 ++++++++++++++++++++++++++++++++++----
 1 files changed, 61 insertions(+), 7 deletions(-)

-- 
1.7.9.5

[PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Jane Wan]

Make FSL eSPI CSnBEF and CSnAFT in ESPI_SPMODEn registers (n=0,1,2,3)
configurable through device tree.  FSL eSPI driver hardcodes them to 0.
Some device requires different value.

Allow FSL eSPI driver configurable whether to send all received data
form slave devices to user.  When user wants to send n_tx bytes and
receives n_rx bytes, FSL eSPI driver sends (n_tx + n_rx) bytes on MOSI.
For the received (n_tx + n_rx) bytes from MISO, current FSL eSPI driver
drops the first n_tx bytes, only passes the last n_rx bytes to user.
Some device driver has problem with this.  It requires to know all bytes
that the slave device puts on MISO.

Part of this fix is based on a previous patch:
http://www.mail-archive.com/spi-devel-general@lists.sourceforge.net/msg08658.html

Signed-off-by: Jane Wan <Jane.Wan@gainspeed.com>
---
 drivers/spi/spi-fsl-espi.c       |   68 ++++++++++++++++++++++++++++++++++----
 1 files changed, 61 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c
index 0622165..660039c 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -45,6 +45,9 @@ struct fsl_espi_transfer {
 	int status;
 };
 
+/* whether to send all rx data to user per chip select */
+static u8 *spi_raw_rxdata_to_user;
+
 /* eSPI Controller mode register definitions */
 #define SPMODE_ENABLE		(1 << 31)
 #define SPMODE_LOOP		(1 << 30)
@@ -398,12 +401,19 @@ static void fsl_espi_rw_trans(struct spi_message *m,
 
 		espi_trans->n_tx = n_tx;
 		espi_trans->n_rx = trans_len;
-		espi_trans->len = trans_len + n_tx;
+		if (spi_raw_rxdata_to_user[m->spi->chip_select])
+			espi_trans->len = n_tx;
+		else
+			espi_trans->len = trans_len + n_tx;
 		espi_trans->tx_buf = local_buf;
 		espi_trans->rx_buf = local_buf + n_tx;
 		fsl_espi_do_trans(m, espi_trans);
 
-		memcpy(rx_buf + pos, espi_trans->rx_buf + n_tx, trans_len);
+		if (spi_raw_rxdata_to_user[m->spi->chip_select])
+			memcpy(rx_buf + pos, espi_trans->rx_buf, trans_len);
+		else
+			memcpy(rx_buf + pos, espi_trans->rx_buf + n_tx,
+				trans_len);
 
 		if (loop > 0)
 			espi_trans->actual_length += espi_trans->len - n_tx;
@@ -433,7 +443,10 @@ static void fsl_espi_do_one_msg(struct spi_message *m)
 
 	espi_trans.n_tx = n_tx;
 	espi_trans.n_rx = n_rx;
-	espi_trans.len = n_tx + n_rx;
+	if (spi_raw_rxdata_to_user[m->spi->chip_select])
+		espi_trans.len = n_tx;
+	else
+		espi_trans.len = n_tx + n_rx;
 	espi_trans.actual_length = 0;
 	espi_trans.status = 0;
 
@@ -579,6 +592,7 @@ static irqreturn_t fsl_espi_irq(s32 irq, void *context_data)
 static void fsl_espi_remove(struct mpc8xxx_spi *mspi)
 {
 	iounmap(mspi->reg_base);
+	kfree(spi_raw_rxdata_to_user);
 }
 
 static struct spi_master * fsl_espi_probe(struct device *dev,
@@ -588,8 +602,10 @@ static struct spi_master * fsl_espi_probe(struct device *dev,
 	struct spi_master *master;
 	struct mpc8xxx_spi *mpc8xxx_spi;
 	struct fsl_espi_reg *reg_base;
-	u32 regval;
-	int i, ret = 0;
+	struct device_node *nc;
+	const __be32 *prop;
+	u32 regval, csmode;
+	int i, len, ret = 0;
 
 	master = spi_alloc_master(dev, sizeof(struct mpc8xxx_spi));
 	if (!master) {
@@ -635,9 +651,45 @@ static struct spi_master * fsl_espi_probe(struct device *dev,
 	mpc8xxx_spi_write_reg(&reg_base->command, 0);
 	mpc8xxx_spi_write_reg(&reg_base->event, 0xffffffff);
 
+	spi_raw_rxdata_to_user = kzalloc(pdata->max_chipselect, GFP_KERNEL);
+	if (!spi_raw_rxdata_to_user) {
+		ret = -ENOMEM;
+		goto err_spi_raw_rxdata_to_user;
+	}
+
 	/* Init eSPI CS mode register */
-	for (i = 0; i < pdata->max_chipselect; i++)
-		mpc8xxx_spi_write_reg(&reg_base->csmode[i], CSMODE_INIT_VAL);
+	for_each_available_child_of_node(master->dev.of_node, nc) {
+		/* get chip select */
+		prop = of_get_property(nc, "reg", &len);
+		if (!prop || len < sizeof(*prop))
+			continue;
+		i = be32_to_cpup(prop);
+		if (i < 0 || i >= pdata->max_chipselect)
+			continue;
+
+		csmode = CSMODE_INIT_VAL;
+		/* check if CSBEF is set in device tree */
+		prop = of_get_property(nc, "fsl,csbef", &len);
+		if (prop && len >= sizeof(*prop)) {
+			csmode &= ~(CSMODE_BEF(0xf));
+			csmode |= CSMODE_BEF(be32_to_cpup(prop));
+		}
+		/* check if CSAFT is set in device tree */
+		prop = of_get_property(nc, "fsl,csaft", &len);
+		if (prop && len >= sizeof(*prop)) {
+			csmode &= ~(CSMODE_AFT(0xf));
+			csmode |= CSMODE_AFT(be32_to_cpup(prop));
+		}
+		mpc8xxx_spi_write_reg(&reg_base->csmode[i], csmode);
+
+		/* check if set to send all received data to user */
+		prop = of_get_property(nc, "fsl,spi-raw-rxdata-to-user", &len);
+		if (prop && len >= sizeof(*prop))
+			spi_raw_rxdata_to_user[i] = be32_to_cpup(prop);
+
+		dev_info(dev, "cs=%d, init_csmode=0x%x, spi_raw_rxdata_to_user=%d\n",
+			 i, csmode, spi_raw_rxdata_to_user[i]);
+	}
 
 	/* Enable SPI interface */
 	regval = pdata->initial_spmode | SPMODE_INIT_VAL | SPMODE_ENABLE;
@@ -653,6 +705,8 @@ static struct spi_master * fsl_espi_probe(struct device *dev,
 	return master;
 
 unreg_master:
+	kfree(spi_raw_rxdata_to_user);
+err_spi_raw_rxdata_to_user:
 	free_irq(mpc8xxx_spi->irq, mpc8xxx_spi);
 free_irq:
 	iounmap(mpc8xxx_spi->reg_base);
-- 
1.7.9.5

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1597 bytes --]

On Sat, Apr 12, 2014 at 11:48:36AM -0700, Jane Wan wrote:
> Make FSL eSPI CSnBEF and CSnAFT in ESPI_SPMODEn registers (n=0,1,2,3)
> configurable through device tree.  FSL eSPI driver hardcodes them to 0.
> Some device requires different value.

What do these do?

> Allow FSL eSPI driver configurable whether to send all received data
> form slave devices to user.  When user wants to send n_tx bytes and
> receives n_rx bytes, FSL eSPI driver sends (n_tx + n_rx) bytes on MOSI.
> For the received (n_tx + n_rx) bytes from MISO, current FSL eSPI driver
> drops the first n_tx bytes, only passes the last n_rx bytes to user.
> Some device driver has problem with this.  It requires to know all bytes
> that the slave device puts on MISO.

This sounds like a separate patch to the first one, the described
behaviour is definitely buggy and any correctly implemented Linux driver
that does bidirectional I/O will have trouble with it.  It should be
split out from the new DT bindings which are a new feature.

> ---
>  drivers/spi/spi-fsl-espi.c       |   68 ++++++++++++++++++++++++++++++++++----
>  1 files changed, 61 insertions(+), 7 deletions(-)

All DT binding changes need to be documented in the binding document.

> +/* whether to send all rx data to user per chip select */
> +static u8 *spi_raw_rxdata_to_user;
> +

No, any data needs to be part of the driver data structure not a global
variable.

> +		if (spi_raw_rxdata_to_user[m->spi->chip_select])
> +			espi_trans->len = n_tx;
> +		else
> +			espi_trans->len = trans_len + n_tx;

Why is there even an option for the buggy behaviour?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

RE: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Jane Wan]

On Mon, Apr 14, 2014 at 09:51:56PM +0100, Mark Brown wrote:
> On Sat, Apr 12, 2014 at 11:48:36AM -0700, Jane Wan wrote:
> > Make FSL eSPI CSnBEF and CSnAFT in ESPI_SPMODEn registers (n=0,1,2,3)
> > configurable through device tree.  FSL eSPI driver hardcodes them to 0.
> > Some device requires different value.
> 
> What do these do?

CSnBEF is the chip select setup time.  It's the delay in bits from the activation 
of chip select pin to the first clock for data frame.

CSnAFT is the chip select hold time.  It's the delay in bits from the last clock 
for data frame to the deactivation of chip select pin.

> > Allow FSL eSPI driver configurable whether to send all received data
> > form slave devices to user.  When user wants to send n_tx bytes and
> > receives n_rx bytes, FSL eSPI driver sends (n_tx + n_rx) bytes on MOSI.
> > For the received (n_tx + n_rx) bytes from MISO, current FSL eSPI
> > driver drops the first n_tx bytes, only passes the last n_rx bytes to user.
> > Some device driver has problem with this.  It requires to know all
> > bytes that the slave device puts on MISO.
> 
> This sounds like a separate patch to the first one, the described behaviour is
> definitely buggy and any correctly implemented Linux driver that does
> bidirectional I/O will have trouble with it.  It should be split out from the new DT
> bindings which are a new feature.
> 
> > ---
> >  drivers/spi/spi-fsl-espi.c       |   68 ++++++++++++++++++++++++++++++++++---
> -
> >  1 files changed, 61 insertions(+), 7 deletions(-)
> 
> All DT binding changes need to be documented in the binding document.

The binding document is updated.  The new changes (for CSnBEF and CSnAFT only)  will 
look like the following.  If this is ok, I'll send it as a separate patch for further discussion.

---
 Documentation/devicetree/bindings/spi/fsl-spi.txt |    6 ++++
 drivers/spi/spi-fsl-espi.c                        |   34 ++++++++++++++++++---
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/fsl-spi.txt b/Documentation/devicetree/bindings/spi/fsl-spi.txt
index b032dd7..2c7fd41 100644
--- a/Documentation/devicetree/bindings/spi/fsl-spi.txt
+++ b/Documentation/devicetree/bindings/spi/fsl-spi.txt
@@ -42,6 +42,10 @@ Required properties:
 - interrupts : should contain eSPI interrupt, the device has one interrupt.
 - fsl,espi-num-chipselects : the number of the chipselect signals.
 
+Optional properties:
+- fsl,csbef: chip select assertion time in bits before frame starts
+- fsl,csaft: chip select negation time in bits after frame ends
+
 Example:
 	spi@110000 {
 		#address-cells = <1>;
@@ -51,4 +55,6 @@ Example:
 		interrupts = <53 0x2>;
 		interrupt-parent = <&mpic>;
 		fsl,espi-num-chipselects = <4>;
+		fsl,csbef = <1>;
+		fsl,csaft = <1>;
 	};
diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c index 428dc7a..7ff9463 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -590,8 +590,10 @@ static struct spi_master * fsl_espi_probe(struct device *dev,
 	struct spi_master *master;
 	struct mpc8xxx_spi *mpc8xxx_spi;
 	struct fsl_espi_reg *reg_base;
-	u32 regval;
-	int i, ret = 0;
+	struct device_node *nc;
+	const __be32 *prop;
+	u32 regval, csmode;
+	int i, len, ret = 0;
 
 	master = spi_alloc_master(dev, sizeof(struct mpc8xxx_spi));
 	if (!master) {
@@ -638,8 +640,32 @@ static struct spi_master * fsl_espi_probe(struct device *dev,
 	mpc8xxx_spi_write_reg(&reg_base->event, 0xffffffff);
 
 	/* Init eSPI CS mode register */
-	for (i = 0; i < pdata->max_chipselect; i++)
-		mpc8xxx_spi_write_reg(&reg_base->csmode[i], CSMODE_INIT_VAL);
+	for_each_available_child_of_node(master->dev.of_node, nc) {
+		/* get chip select */
+		prop = of_get_property(nc, "reg", &len);
+		if (!prop || len < sizeof(*prop))
+			continue;
+		i = be32_to_cpup(prop);
+		if (i < 0 || i >= pdata->max_chipselect)
+			continue;
+
+		csmode = CSMODE_INIT_VAL;
+		/* check if CSBEF is set in device tree */
+		prop = of_get_property(nc, "fsl,csbef", &len);
+		if (prop && len >= sizeof(*prop)) {
+			csmode &= ~(CSMODE_BEF(0xf));
+			csmode |= CSMODE_BEF(be32_to_cpup(prop));
+		}
+		/* check if CSAFT is set in device tree */
+		prop = of_get_property(nc, "fsl,csaft", &len);
+		if (prop && len >= sizeof(*prop)) {
+			csmode &= ~(CSMODE_AFT(0xf));
+			csmode |= CSMODE_AFT(be32_to_cpup(prop));
+		}
+		mpc8xxx_spi_write_reg(&reg_base->csmode[i], csmode);
+
+		dev_info(dev, "cs=%d, init_csmode=0x%x\n", i, csmode);
+	}
 
 	/* Enable SPI interface */
 	regval = pdata->initial_spmode | SPMODE_INIT_VAL | SPMODE_ENABLE;
--
1.7.9.5

> > +/* whether to send all rx data to user per chip select */ static u8
> > +*spi_raw_rxdata_to_user;
> > +
> 
> No, any data needs to be part of the driver data structure not a global variable.

I will look into this.

> > +		if (spi_raw_rxdata_to_user[m->spi->chip_select])
> > +			espi_trans->len = n_tx;
> > +		else
> > +			espi_trans->len = trans_len + n_tx;
> 
> Why is there even an option for the buggy behaviour?

We have three devices attached to the FSL eSPI interface, with chip select (CS) 
0-2. The device driver for the device at CS #2 requires to know all the data that 
the slave device put on MISO.  But the device drivers for the other two devices 
(at CS #0 and #1) work with the existing FSL eSPI driver.  The device at CS #0 is 
Micron n25q512a compatible.

We make the FSL eSPI driver configurable through device tree.  If we make a fix 
without the DT option, the fix will break other device drivers working with the 
existing FSL eSPI driver.

Could this still be considered as a solution?  If this is ok, I can send it as a separate 
patch.  Otherwise, we will look if this driver can be modified without DT option.

Regards,
Jane

RE: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Jane Wan]

On Mon, Apr 14, 2014 at 09:51:56PM +0100, Mark Brown wrote:
> On Sat, Apr 12, 2014 at 11:48:36AM -0700, Jane Wan wrote:
> > Make FSL eSPI CSnBEF and CSnAFT in ESPI_SPMODEn registers (n=0,1,2,3)
> > configurable through device tree.  FSL eSPI driver hardcodes them to 0.
> > Some device requires different value.
> 
> What do these do?

CSnBEF is the chip select setup time.  It's the delay in bits from the activation 
of chip select pin to the first clock for data frame.

CSnAFT is the chip select hold time.  It's the delay in bits from the last clock 
for data frame to the deactivation of chip select pin.

> > Allow FSL eSPI driver configurable whether to send all received data
> > form slave devices to user.  When user wants to send n_tx bytes and
> > receives n_rx bytes, FSL eSPI driver sends (n_tx + n_rx) bytes on MOSI.
> > For the received (n_tx + n_rx) bytes from MISO, current FSL eSPI
> > driver drops the first n_tx bytes, only passes the last n_rx bytes to user.
> > Some device driver has problem with this.  It requires to know all
> > bytes that the slave device puts on MISO.
> 
> This sounds like a separate patch to the first one, the described behaviour is
> definitely buggy and any correctly implemented Linux driver that does
> bidirectional I/O will have trouble with it.  It should be split out from the new DT
> bindings which are a new feature.
> 
> > ---
> >  drivers/spi/spi-fsl-espi.c       |   68 ++++++++++++++++++++++++++++++++++---
> -
> >  1 files changed, 61 insertions(+), 7 deletions(-)
> 
> All DT binding changes need to be documented in the binding document.

The binding document is updated.  The new changes (for CSnBEF and CSnAFT only)  will 
look like the following.  If this is ok, I'll send it as a separate patch for further discussion.

---
 Documentation/devicetree/bindings/spi/fsl-spi.txt |    6 ++++
 drivers/spi/spi-fsl-espi.c                        |   34 ++++++++++++++++++---
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/fsl-spi.txt b/Documentation/devicetree/bindings/spi/fsl-spi.txt
index b032dd7..2c7fd41 100644
--- a/Documentation/devicetree/bindings/spi/fsl-spi.txt
+++ b/Documentation/devicetree/bindings/spi/fsl-spi.txt
@@ -42,6 +42,10 @@ Required properties:
 - interrupts : should contain eSPI interrupt, the device has one interrupt.
 - fsl,espi-num-chipselects : the number of the chipselect signals.
 
+Optional properties:
+- fsl,csbef: chip select assertion time in bits before frame starts
+- fsl,csaft: chip select negation time in bits after frame ends
+
 Example:
 	spi@110000 {
 		#address-cells = <1>;
@@ -51,4 +55,6 @@ Example:
 		interrupts = <53 0x2>;
 		interrupt-parent = <&mpic>;
 		fsl,espi-num-chipselects = <4>;
+		fsl,csbef = <1>;
+		fsl,csaft = <1>;
 	};
diff --git a/drivers/spi/spi-fsl-espi.c b/drivers/spi/spi-fsl-espi.c index 428dc7a..7ff9463 100644
--- a/drivers/spi/spi-fsl-espi.c
+++ b/drivers/spi/spi-fsl-espi.c
@@ -590,8 +590,10 @@ static struct spi_master * fsl_espi_probe(struct device *dev,
 	struct spi_master *master;
 	struct mpc8xxx_spi *mpc8xxx_spi;
 	struct fsl_espi_reg *reg_base;
-	u32 regval;
-	int i, ret = 0;
+	struct device_node *nc;
+	const __be32 *prop;
+	u32 regval, csmode;
+	int i, len, ret = 0;
 
 	master = spi_alloc_master(dev, sizeof(struct mpc8xxx_spi));
 	if (!master) {
@@ -638,8 +640,32 @@ static struct spi_master * fsl_espi_probe(struct device *dev,
 	mpc8xxx_spi_write_reg(&reg_base->event, 0xffffffff);
 
 	/* Init eSPI CS mode register */
-	for (i = 0; i < pdata->max_chipselect; i++)
-		mpc8xxx_spi_write_reg(&reg_base->csmode[i], CSMODE_INIT_VAL);
+	for_each_available_child_of_node(master->dev.of_node, nc) {
+		/* get chip select */
+		prop = of_get_property(nc, "reg", &len);
+		if (!prop || len < sizeof(*prop))
+			continue;
+		i = be32_to_cpup(prop);
+		if (i < 0 || i >= pdata->max_chipselect)
+			continue;
+
+		csmode = CSMODE_INIT_VAL;
+		/* check if CSBEF is set in device tree */
+		prop = of_get_property(nc, "fsl,csbef", &len);
+		if (prop && len >= sizeof(*prop)) {
+			csmode &= ~(CSMODE_BEF(0xf));
+			csmode |= CSMODE_BEF(be32_to_cpup(prop));
+		}
+		/* check if CSAFT is set in device tree */
+		prop = of_get_property(nc, "fsl,csaft", &len);
+		if (prop && len >= sizeof(*prop)) {
+			csmode &= ~(CSMODE_AFT(0xf));
+			csmode |= CSMODE_AFT(be32_to_cpup(prop));
+		}
+		mpc8xxx_spi_write_reg(&reg_base->csmode[i], csmode);
+
+		dev_info(dev, "cs=%d, init_csmode=0x%x\n", i, csmode);
+	}
 
 	/* Enable SPI interface */
 	regval = pdata->initial_spmode | SPMODE_INIT_VAL | SPMODE_ENABLE;
--
1.7.9.5

> > +/* whether to send all rx data to user per chip select */ static u8
> > +*spi_raw_rxdata_to_user;
> > +
> 
> No, any data needs to be part of the driver data structure not a global variable.

I will look into this.

> > +		if (spi_raw_rxdata_to_user[m->spi->chip_select])
> > +			espi_trans->len = n_tx;
> > +		else
> > +			espi_trans->len = trans_len + n_tx;
> 
> Why is there even an option for the buggy behaviour?

We have three devices attached to the FSL eSPI interface, with chip select (CS) 
0-2. The device driver for the device at CS #2 requires to know all the data that 
the slave device put on MISO.  But the device drivers for the other two devices 
(at CS #0 and #1) work with the existing FSL eSPI driver.  The device at CS #0 is 
Micron n25q512a compatible.

We make the FSL eSPI driver configurable through device tree.  If we make a fix 
without the DT option, the fix will break other device drivers working with the 
existing FSL eSPI driver.

Could this still be considered as a solution?  If this is ok, I can send it as a separate 
patch.  Otherwise, we will look if this driver can be modified without DT option.

Regards,
Jane
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1318 bytes --]

On Wed, Apr 16, 2014 at 04:39:47PM +0000, Jane Wan wrote:
> On Mon, Apr 14, 2014 at 09:51:56PM +0100, Mark Brown wrote:

> > > +		if (spi_raw_rxdata_to_user[m->spi->chip_select])
> > > +			espi_trans->len = n_tx;
> > > +		else
> > > +			espi_trans->len = trans_len + n_tx;

> > Why is there even an option for the buggy behaviour?

> We have three devices attached to the FSL eSPI interface, with chip select (CS) 
> 0-2. The device driver for the device at CS #2 requires to know all the data that 
> the slave device put on MISO.  But the device drivers for the other two devices 
> (at CS #0 and #1) work with the existing FSL eSPI driver.  The device at CS #0 is 
> Micron n25q512a compatible.

> We make the FSL eSPI driver configurable through device tree.  If we make a fix 
> without the DT option, the fix will break other device drivers working with the 
> existing FSL eSPI driver.

> Could this still be considered as a solution?  If this is ok, I can send it as a separate 
> patch.  Otherwise, we will look if this driver can be modified without DT option.

No, this is completely unaceptable.  The drivers relying on the buggy
behaviour are broken and must be fixed.  The whole point of DT is to
describe the hardware, not to allow the OS to implement workarounds for
itself.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1318 bytes --]

On Wed, Apr 16, 2014 at 04:39:47PM +0000, Jane Wan wrote:
> On Mon, Apr 14, 2014 at 09:51:56PM +0100, Mark Brown wrote:

> > > +		if (spi_raw_rxdata_to_user[m->spi->chip_select])
> > > +			espi_trans->len = n_tx;
> > > +		else
> > > +			espi_trans->len = trans_len + n_tx;

> > Why is there even an option for the buggy behaviour?

> We have three devices attached to the FSL eSPI interface, with chip select (CS) 
> 0-2. The device driver for the device at CS #2 requires to know all the data that 
> the slave device put on MISO.  But the device drivers for the other two devices 
> (at CS #0 and #1) work with the existing FSL eSPI driver.  The device at CS #0 is 
> Micron n25q512a compatible.

> We make the FSL eSPI driver configurable through device tree.  If we make a fix 
> without the DT option, the fix will break other device drivers working with the 
> existing FSL eSPI driver.

> Could this still be considered as a solution?  If this is ok, I can send it as a separate 
> patch.  Otherwise, we will look if this driver can be modified without DT option.

No, this is completely unaceptable.  The drivers relying on the buggy
behaviour are broken and must be fixed.  The whole point of DT is to
describe the hardware, not to allow the OS to implement workarounds for
itself.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

On Sat, Apr 12, 2014 at 11:48:35AM -0700, Jane Wan wrote:
> Make FSL eSPI properties configurable through device tree.  The
> configurable parameters include CSnBEF and CSnAFT in ESPI_SPMODEn
> registers (n=0,1,2,3), and whether to send all received data to user.

Don't send cover letters for individual patches, include anything that's
needed in the patch changelog or after the ---.

> Test done:
> - Kernel version 3.8.13

We're developing v3.15 now, you need to submit against current code.

Please also be sure to CC the mailing list on patches, the sourceforge
list has been unused for quite some time now.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Insop Song]

On Mon, Apr 14, 2014 at 09:51:51PM +0100, Mark Brown wrote:
> On Sat, Apr 12, 2014 at 11:48:35AM -0700, Jane Wan wrote:
> > Make FSL eSPI properties configurable through device tree.  The
> > configurable parameters include CSnBEF and CSnAFT in ESPI_SPMODEn
> > registers (n=0,1,2,3), and whether to send all received data to user.
> 
> Don't send cover letters for individual patches, include anything that's
> needed in the patch changelog or after the ---.
> 
> > Test done:
> > - Kernel version 3.8.13
> 
> We're developing v3.15 now, you need to submit against current code.
> 

We will generate a new patch.

> Please also be sure to CC the mailing list on patches, the sourceforge
> list has been unused for quite some time now.

We use scripts/get_maintainer.pl to get the mailing addresses.
We will double check the mailing list from MAINTAINER file.

Regards,

Insop

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Insop Song]

On Mon, Apr 14, 2014 at 09:51:51PM +0100, Mark Brown wrote:
> On Sat, Apr 12, 2014 at 11:48:35AM -0700, Jane Wan wrote:
> > Make FSL eSPI properties configurable through device tree.  The
> > configurable parameters include CSnBEF and CSnAFT in ESPI_SPMODEn
> > registers (n=0,1,2,3), and whether to send all received data to user.
> 
> Don't send cover letters for individual patches, include anything that's
> needed in the patch changelog or after the ---.
> 
> > Test done:
> > - Kernel version 3.8.13
> 
> We're developing v3.15 now, you need to submit against current code.
> 

We will generate a new patch.

> Please also be sure to CC the mailing list on patches, the sourceforge
> list has been unused for quite some time now.

We use scripts/get_maintainer.pl to get the mailing addresses.
We will double check the mailing list from MAINTAINER file.

Regards,

Insop

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

On Mon, Apr 14, 2014 at 02:38:53PM -0700, Insop Song wrote:
> On Mon, Apr 14, 2014 at 09:51:51PM +0100, Mark Brown wrote:

> > Please also be sure to CC the mailing list on patches, the sourceforge
> > list has been unused for quite some time now.

> We use scripts/get_maintainer.pl to get the mailing addresses.
> We will double check the mailing list from MAINTAINER file.

This will be part of not using a current kernel - if you're using a v3.8
kernel to check then that's over a year out of date.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

On Mon, Apr 14, 2014 at 02:38:53PM -0700, Insop Song wrote:
> On Mon, Apr 14, 2014 at 09:51:51PM +0100, Mark Brown wrote:

> > Please also be sure to CC the mailing list on patches, the sourceforge
> > list has been unused for quite some time now.

> We use scripts/get_maintainer.pl to get the mailing addresses.
> We will double check the mailing list from MAINTAINER file.

This will be part of not using a current kernel - if you're using a v3.8
kernel to check then that's over a year out of date.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Insop Song]

On Tue, Apr 15, 2014 at 12:49:49AM +0100, Mark Brown wrote:
> On Mon, Apr 14, 2014 at 02:38:53PM -0700, Insop Song wrote:
> > On Mon, Apr 14, 2014 at 09:51:51PM +0100, Mark Brown wrote:
> 
> > > Please also be sure to CC the mailing list on patches, the sourceforge
> > > list has been unused for quite some time now.
> 
> > We use scripts/get_maintainer.pl to get the mailing addresses.
> > We will double check the mailing list from MAINTAINER file.
> 
> This will be part of not using a current kernel - if you're using a v3.8
> kernel to check then that's over a year out of date.

Mark,

If you were asking that the issue is due to the fact that the kernel we are using is
out of date, then here is our response.

Though, we didn't generate the patch with the latest kernel, we HAVE checked the
latest kernel and the *bugginess* of the freescale espi driver was the same as
the kernel that we are using.

Using the other thread, we will response to your other concerns.

Regards,

Insop

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Insop Song]

On Tue, Apr 15, 2014 at 12:49:49AM +0100, Mark Brown wrote:
> On Mon, Apr 14, 2014 at 02:38:53PM -0700, Insop Song wrote:
> > On Mon, Apr 14, 2014 at 09:51:51PM +0100, Mark Brown wrote:
> 
> > > Please also be sure to CC the mailing list on patches, the sourceforge
> > > list has been unused for quite some time now.
> 
> > We use scripts/get_maintainer.pl to get the mailing addresses.
> > We will double check the mailing list from MAINTAINER file.
> 
> This will be part of not using a current kernel - if you're using a v3.8
> kernel to check then that's over a year out of date.

Mark,

If you were asking that the issue is due to the fact that the kernel we are using is
out of date, then here is our response.

Though, we didn't generate the patch with the latest kernel, we HAVE checked the
latest kernel and the *bugginess* of the freescale espi driver was the same as
the kernel that we are using.

Using the other thread, we will response to your other concerns.

Regards,

Insop
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Insop Song]

On Tue, Apr 15, 2014 at 12:49:49AM +0100, Mark Brown wrote:
> On Mon, Apr 14, 2014 at 02:38:53PM -0700, Insop Song wrote:
> > On Mon, Apr 14, 2014 at 09:51:51PM +0100, Mark Brown wrote:
> 
> > > Please also be sure to CC the mailing list on patches, the sourceforge
> > > list has been unused for quite some time now.
> 
> > We use scripts/get_maintainer.pl to get the mailing addresses.
> > We will double check the mailing list from MAINTAINER file.
> 
> This will be part of not using a current kernel - if you're using a v3.8
> kernel to check then that's over a year out of date.

I think I sent too soon, you meant for the get_maintainer.pl script.

Regards,

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Insop Song]

On Tue, Apr 15, 2014 at 12:49:49AM +0100, Mark Brown wrote:
> On Mon, Apr 14, 2014 at 02:38:53PM -0700, Insop Song wrote:
> > On Mon, Apr 14, 2014 at 09:51:51PM +0100, Mark Brown wrote:
> 
> > > Please also be sure to CC the mailing list on patches, the sourceforge
> > > list has been unused for quite some time now.
> 
> > We use scripts/get_maintainer.pl to get the mailing addresses.
> > We will double check the mailing list from MAINTAINER file.
> 
> This will be part of not using a current kernel - if you're using a v3.8
> kernel to check then that's over a year out of date.

I think I sent too soon, you meant for the get_maintainer.pl script.

Regards,

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

On Mon, Apr 14, 2014 at 05:00:59PM -0700, Insop Song wrote:
> On Tue, Apr 15, 2014 at 12:49:49AM +0100, Mark Brown wrote:

> > > We use scripts/get_maintainer.pl to get the mailing addresses.
> > > We will double check the mailing list from MAINTAINER file.

> > This will be part of not using a current kernel - if you're using a v3.8
> > kernel to check then that's over a year out of date.

> I think I sent too soon, you meant for the get_maintainer.pl script.

Yup, and for the MAINTAINERS file which it looks things up in - your
copies will be out of date if you're running them from the v3.8 kernel.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Configure FSL eSPI CSBEF, CSAFT, and whether to send all received data to user

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

On Mon, Apr 14, 2014 at 05:00:59PM -0700, Insop Song wrote:
> On Tue, Apr 15, 2014 at 12:49:49AM +0100, Mark Brown wrote:

> > > We use scripts/get_maintainer.pl to get the mailing addresses.
> > > We will double check the mailing list from MAINTAINER file.

> > This will be part of not using a current kernel - if you're using a v3.8
> > kernel to check then that's over a year out of date.

> I think I sent too soon, you meant for the get_maintainer.pl script.

Yup, and for the MAINTAINERS file which it looks things up in - your
copies will be out of date if you're running them from the v3.8 kernel.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]