Re: [PATCH v2] staging: comedi: ni_mio_common: protect register write overflow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/10/18 21:54, Spencer E. Olson wrote:
Fixes two problems introduced as early as
commit 03aef4b6dc12  ("Staging: comedi: add ni_mio_common code"):
(1) Ensures that the last four bits of NISTC_RTSI_TRIGB_OUT_REG register is
     not unduly overwritten on e-series devices.  On e-series devices, the
     first three of the last four bits are reserved.  The last bit defines
     the output selection of the RGOUT0 pin, otherwise known as
     RTSI_Sub_Selection.  For m-series devices, these last four bits are
     indeed used as the output selection of the RTSI7 pin (and the
     RTSI_Sub_Selection bit for the RGOUT0 pin is moved to the
     RTSI_Trig_Direction register.
(2) Allows all 4 RTSI_BRD lines to be treated as valid sources for RTSI
     lines.

This patch also cleans up the ni_get_rtsi_routing command for readability.

Fixes: 03aef4b6dc12  ("Staging: comedi: add ni_mio_common code")
Signed-off-by: Spencer E. Olson <olsonse@xxxxxxxxx>
---

Changes:
   - [PATCH v2]: Adds missing brace that had been corrected in a later patchset
     but should have been correct here.  This patch is now independent and
     complete.

  .../staging/comedi/drivers/ni_mio_common.c    | 24 +++++++++++++------
  1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c
index 4dee2fc37aed..44fcb3790113 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4980,7 +4980,10 @@ static int ni_valid_rtsi_output_source(struct comedi_device *dev,
  	case NI_RTSI_OUTPUT_G_SRC0:
  	case NI_RTSI_OUTPUT_G_GATE0:
  	case NI_RTSI_OUTPUT_RGOUT0:
-	case NI_RTSI_OUTPUT_RTSI_BRD_0:
+	case NI_RTSI_OUTPUT_RTSI_BRD(0):
+	case NI_RTSI_OUTPUT_RTSI_BRD(1):
+	case NI_RTSI_OUTPUT_RTSI_BRD(2):
+	case NI_RTSI_OUTPUT_RTSI_BRD(3):
  		return 1;
  	case NI_RTSI_OUTPUT_RTSI_OSC:
  		return (devpriv->is_m_series) ? 1 : 0;
@@ -5001,11 +5004,18 @@ static int ni_set_rtsi_routing(struct comedi_device *dev,
  		devpriv->rtsi_trig_a_output_reg |= NISTC_RTSI_TRIG(chan, src);
  		ni_stc_writew(dev, devpriv->rtsi_trig_a_output_reg,
  			      NISTC_RTSI_TRIGA_OUT_REG);
-	} else if (chan < 8) {
+	} else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
  		devpriv->rtsi_trig_b_output_reg &= ~NISTC_RTSI_TRIG_MASK(chan);
  		devpriv->rtsi_trig_b_output_reg |= NISTC_RTSI_TRIG(chan, src);
  		ni_stc_writew(dev, devpriv->rtsi_trig_b_output_reg,
  			      NISTC_RTSI_TRIGB_OUT_REG);
+	} else if (chan != NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
+		/* probably should never reach this, since the
+		 * ni_valid_rtsi_output_source above errors out if chan is too
+		 * high
+		 */
+		dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
+		return -EINVAL;
  	}
  	return 2;
  }
@@ -5021,12 +5031,12 @@ static unsigned int ni_get_rtsi_routing(struct comedi_device *dev,
  	} else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
  		return NISTC_RTSI_TRIG_TO_SRC(chan,
  					      devpriv->rtsi_trig_b_output_reg);
-	} else {
-		if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN)
-			return NI_RTSI_OUTPUT_RTSI_OSC;
-		dev_err(dev->class_dev, "bug! should never get here?\n");
-		return 0;
+	} else if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
+		return NI_RTSI_OUTPUT_RTSI_OSC;
  	}
+
+	dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
+	return -EINVAL;

I'm not really sure what it should return for an invalid channel, since this value gets written to the data[1] of the INSN_CONFIG_GET_ROUTING instruction which doesn't return an error.

Perhaps it would be better if INSN_CONFIG_GET_ROUTING returned an error for an out of range channel, but that's a change for another day.

  }
static int ni_rtsi_insn_config(struct comedi_device *dev,


Reviewed-by: Ian Abbott <abbotti@xxxxxxxxx>

--
-=( Ian Abbott <abbotti@xxxxxxxxx> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux