Re: [PATCH 07/18] spi: qup: Fix transaction done signaling
Hi Andy, On 6/15/2017 1:21 AM, Andy Gross wrote: > On Wed, Jun 14, 2017 at 12:43:43PM +0530, Sricharan R wrote: >> Hi Varada, >> >> On 6/14/2017 11:22 AM, Varadarajan Narayanan wrote: >>> Wait to signal done until we get all of the interrupts we are expecting >>> to get for a transaction. If we don't wait for the input done flag, we >>> can be inbetween transactions when the done flag comes in and this can >>> mess up the next transaction. >>> >>> Signed-off-by: Andy Gross>>> Signed-off-by: Varadarajan Narayanan >>> --- >>> drivers/spi/spi-qup.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c >>> index 2124815..7c22ee4 100644 >>> --- a/drivers/spi/spi-qup.c >>> +++ b/drivers/spi/spi-qup.c >>> @@ -465,7 +465,8 @@ static irqreturn_t spi_qup_qup_irq(int irq, void >>> *dev_id) >>> controller->xfer = xfer; >>> spin_unlock_irqrestore(>lock, flags); >>> >>> - if (controller->rx_bytes == xfer->len || error) >>> + if ((controller->rx_bytes == xfer->len && >>> + (opflags & QUP_OP_MAX_INPUT_DONE_FLAG)) || error) >> >> Not sure why we need this additional check, because having read all the >> bytes implies transfer complete (or) why not just check only for >> QUP_OP_MAX_INPUT_DONE_FLAG ? > > So you can receive an interrupt for the last data without it having also > signalled the INPUT_DONE. That means you'd have one more IRQ come in and if > you > don't wait for that, you could start up the next transaction and have an irq > come in that screws up that transaction. > > It might be sufficient to just wait for the INPUT_DONE_FLAG. That cannot be > signalled unless the rx_bytes == xfer->len. > Right, that should simply it little bit. Regards, Sricharan -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 07/18] spi: qup: Fix transaction done signaling
Hi Andy, On 6/15/2017 1:21 AM, Andy Gross wrote: > On Wed, Jun 14, 2017 at 12:43:43PM +0530, Sricharan R wrote: >> Hi Varada, >> >> On 6/14/2017 11:22 AM, Varadarajan Narayanan wrote: >>> Wait to signal done until we get all of the interrupts we are expecting >>> to get for a transaction. If we don't wait for the input done flag, we >>> can be inbetween transactions when the done flag comes in and this can >>> mess up the next transaction. >>> >>> Signed-off-by: Andy Gross >>> Signed-off-by: Varadarajan Narayanan >>> --- >>> drivers/spi/spi-qup.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c >>> index 2124815..7c22ee4 100644 >>> --- a/drivers/spi/spi-qup.c >>> +++ b/drivers/spi/spi-qup.c >>> @@ -465,7 +465,8 @@ static irqreturn_t spi_qup_qup_irq(int irq, void >>> *dev_id) >>> controller->xfer = xfer; >>> spin_unlock_irqrestore(>lock, flags); >>> >>> - if (controller->rx_bytes == xfer->len || error) >>> + if ((controller->rx_bytes == xfer->len && >>> + (opflags & QUP_OP_MAX_INPUT_DONE_FLAG)) || error) >> >> Not sure why we need this additional check, because having read all the >> bytes implies transfer complete (or) why not just check only for >> QUP_OP_MAX_INPUT_DONE_FLAG ? > > So you can receive an interrupt for the last data without it having also > signalled the INPUT_DONE. That means you'd have one more IRQ come in and if > you > don't wait for that, you could start up the next transaction and have an irq > come in that screws up that transaction. > > It might be sufficient to just wait for the INPUT_DONE_FLAG. That cannot be > signalled unless the rx_bytes == xfer->len. > Right, that should simply it little bit. Regards, Sricharan -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 07/18] spi: qup: Fix transaction done signaling
On Wed, Jun 14, 2017 at 12:43:43PM +0530, Sricharan R wrote: > Hi Varada, > > On 6/14/2017 11:22 AM, Varadarajan Narayanan wrote: > > Wait to signal done until we get all of the interrupts we are expecting > > to get for a transaction. If we don't wait for the input done flag, we > > can be inbetween transactions when the done flag comes in and this can > > mess up the next transaction. > > > > Signed-off-by: Andy Gross> > Signed-off-by: Varadarajan Narayanan > > --- > > drivers/spi/spi-qup.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c > > index 2124815..7c22ee4 100644 > > --- a/drivers/spi/spi-qup.c > > +++ b/drivers/spi/spi-qup.c > > @@ -465,7 +465,8 @@ static irqreturn_t spi_qup_qup_irq(int irq, void > > *dev_id) > > controller->xfer = xfer; > > spin_unlock_irqrestore(>lock, flags); > > > > - if (controller->rx_bytes == xfer->len || error) > > + if ((controller->rx_bytes == xfer->len && > > + (opflags & QUP_OP_MAX_INPUT_DONE_FLAG)) || error) > > Not sure why we need this additional check, because having read all the > bytes implies transfer complete (or) why not just check only for > QUP_OP_MAX_INPUT_DONE_FLAG ? So you can receive an interrupt for the last data without it having also signalled the INPUT_DONE. That means you'd have one more IRQ come in and if you don't wait for that, you could start up the next transaction and have an irq come in that screws up that transaction. It might be sufficient to just wait for the INPUT_DONE_FLAG. That cannot be signalled unless the rx_bytes == xfer->len. Regards, Andy
Re: [PATCH 07/18] spi: qup: Fix transaction done signaling
On Wed, Jun 14, 2017 at 12:43:43PM +0530, Sricharan R wrote: > Hi Varada, > > On 6/14/2017 11:22 AM, Varadarajan Narayanan wrote: > > Wait to signal done until we get all of the interrupts we are expecting > > to get for a transaction. If we don't wait for the input done flag, we > > can be inbetween transactions when the done flag comes in and this can > > mess up the next transaction. > > > > Signed-off-by: Andy Gross > > Signed-off-by: Varadarajan Narayanan > > --- > > drivers/spi/spi-qup.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c > > index 2124815..7c22ee4 100644 > > --- a/drivers/spi/spi-qup.c > > +++ b/drivers/spi/spi-qup.c > > @@ -465,7 +465,8 @@ static irqreturn_t spi_qup_qup_irq(int irq, void > > *dev_id) > > controller->xfer = xfer; > > spin_unlock_irqrestore(>lock, flags); > > > > - if (controller->rx_bytes == xfer->len || error) > > + if ((controller->rx_bytes == xfer->len && > > + (opflags & QUP_OP_MAX_INPUT_DONE_FLAG)) || error) > > Not sure why we need this additional check, because having read all the > bytes implies transfer complete (or) why not just check only for > QUP_OP_MAX_INPUT_DONE_FLAG ? So you can receive an interrupt for the last data without it having also signalled the INPUT_DONE. That means you'd have one more IRQ come in and if you don't wait for that, you could start up the next transaction and have an irq come in that screws up that transaction. It might be sufficient to just wait for the INPUT_DONE_FLAG. That cannot be signalled unless the rx_bytes == xfer->len. Regards, Andy
Re: [PATCH 07/18] spi: qup: Fix transaction done signaling
Hi Varada, On 6/14/2017 11:22 AM, Varadarajan Narayanan wrote: > Wait to signal done until we get all of the interrupts we are expecting > to get for a transaction. If we don't wait for the input done flag, we > can be inbetween transactions when the done flag comes in and this can > mess up the next transaction. > > Signed-off-by: Andy Gross> Signed-off-by: Varadarajan Narayanan > --- > drivers/spi/spi-qup.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c > index 2124815..7c22ee4 100644 > --- a/drivers/spi/spi-qup.c > +++ b/drivers/spi/spi-qup.c > @@ -465,7 +465,8 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id) > controller->xfer = xfer; > spin_unlock_irqrestore(>lock, flags); > > - if (controller->rx_bytes == xfer->len || error) > + if ((controller->rx_bytes == xfer->len && > + (opflags & QUP_OP_MAX_INPUT_DONE_FLAG)) || error) Not sure why we need this additional check, because having read all the bytes implies transfer complete (or) why not just check only for QUP_OP_MAX_INPUT_DONE_FLAG ? Regards, Sricharan -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 07/18] spi: qup: Fix transaction done signaling
Hi Varada, On 6/14/2017 11:22 AM, Varadarajan Narayanan wrote: > Wait to signal done until we get all of the interrupts we are expecting > to get for a transaction. If we don't wait for the input done flag, we > can be inbetween transactions when the done flag comes in and this can > mess up the next transaction. > > Signed-off-by: Andy Gross > Signed-off-by: Varadarajan Narayanan > --- > drivers/spi/spi-qup.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c > index 2124815..7c22ee4 100644 > --- a/drivers/spi/spi-qup.c > +++ b/drivers/spi/spi-qup.c > @@ -465,7 +465,8 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id) > controller->xfer = xfer; > spin_unlock_irqrestore(>lock, flags); > > - if (controller->rx_bytes == xfer->len || error) > + if ((controller->rx_bytes == xfer->len && > + (opflags & QUP_OP_MAX_INPUT_DONE_FLAG)) || error) Not sure why we need this additional check, because having read all the bytes implies transfer complete (or) why not just check only for QUP_OP_MAX_INPUT_DONE_FLAG ? Regards, Sricharan -- "QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 07/18] spi: qup: Fix transaction done signaling
Wait to signal done until we get all of the interrupts we are expecting to get for a transaction. If we don't wait for the input done flag, we can be inbetween transactions when the done flag comes in and this can mess up the next transaction. Signed-off-by: Andy GrossSigned-off-by: Varadarajan Narayanan --- drivers/spi/spi-qup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c index 2124815..7c22ee4 100644 --- a/drivers/spi/spi-qup.c +++ b/drivers/spi/spi-qup.c @@ -465,7 +465,8 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id) controller->xfer = xfer; spin_unlock_irqrestore(>lock, flags); - if (controller->rx_bytes == xfer->len || error) + if ((controller->rx_bytes == xfer->len && + (opflags & QUP_OP_MAX_INPUT_DONE_FLAG)) || error) complete(>done); return IRQ_HANDLED; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH 07/18] spi: qup: Fix transaction done signaling
Wait to signal done until we get all of the interrupts we are expecting to get for a transaction. If we don't wait for the input done flag, we can be inbetween transactions when the done flag comes in and this can mess up the next transaction. Signed-off-by: Andy Gross Signed-off-by: Varadarajan Narayanan --- drivers/spi/spi-qup.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-qup.c b/drivers/spi/spi-qup.c index 2124815..7c22ee4 100644 --- a/drivers/spi/spi-qup.c +++ b/drivers/spi/spi-qup.c @@ -465,7 +465,8 @@ static irqreturn_t spi_qup_qup_irq(int irq, void *dev_id) controller->xfer = xfer; spin_unlock_irqrestore(>lock, flags); - if (controller->rx_bytes == xfer->len || error) + if ((controller->rx_bytes == xfer->len && + (opflags & QUP_OP_MAX_INPUT_DONE_FLAG)) || error) complete(>done); return IRQ_HANDLED; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation