From 91ff6028b87b03d130b8f77c7c1b6dca05c800c6 Mon Sep 17 00:00:00 2001 From: Rian Hunter Date: Wed, 29 May 2024 20:18:11 -0400 Subject: [PATCH 1/3] Add bounds to center_offset The center_offset parameter doesn't work for values larger than 63 and it definitely does not work for negative values (negative values would cause overflow into the other bitfields of the MO_AGC_SYNC_TIP3). Bound the parameter similar to how we add bounds to the level parameter. --- README.md | 2 +- cxadc.c | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 12eed8c..d3352e9 100644 --- a/README.md +++ b/README.md @@ -374,7 +374,7 @@ extra cooling required above 40mhz). This value is ONLY used to compute the sample rates entered for the tenxfsc parameters other than 0, 1, 2. -### `center_offset` (0 to 255, default 2) +### `center_offset` (0 to 63, default 2) This option allows you to manually adjust DC centre offset or the centring of the RF signal you wish to capture. diff --git a/cxadc.c b/cxadc.c index 3d45dae..24e9d30 100644 --- a/cxadc.c +++ b/cxadc.c @@ -671,6 +671,10 @@ static int cxadc_char_open(struct inode *inode, struct file *file) ctd->level = 31; /* control gain also bit 16 */ cx_write(MO_AGC_GAIN_ADJ4, (ctd->sixdb<<23)|(0<<22)|(0<<21)|(ctd->level<<16)|(0xff<<8)|(0x0<<0)); + if (ctd->center_offset < 0) + ctd->center_offset = 0; + if (ctd->center_offset > 63) + ctd->center_offset = 63; cx_write(MO_AGC_SYNC_TIP3, (0x1e48<<16)|(0xff<<8)|(ctd->center_offset)); if (ctd->tenxfsc < 10) { @@ -804,6 +808,10 @@ static ssize_t cxadc_char_read(struct file *file, char __user *tgt, if (ctd->level > 31) ctd->level = 31; cx_write(MO_AGC_GAIN_ADJ4, (ctd->sixdb<<23)|(0<<22)|(0<<21)|(ctd->level<<16)|(0xff<<8)|(0x0<<0)); + if (ctd->center_offset < 0) + ctd->center_offset = 0; + if (ctd->center_offset > 63) + ctd->center_offset = 63; cx_write(MO_AGC_SYNC_TIP3, (0x1e48<<16)|(0xff<<8)|(ctd->center_offset)); if (count) { @@ -1153,6 +1161,11 @@ static int cxadc_probe(struct pci_dev *pci_dev, if (ctd->level > 31) ctd->level = 31; + if (ctd->center_offset < 0) + ctd->center_offset = 0; + if (ctd->center_offset > 63) + ctd->center_offset = 63; + cx_write(MO_AGC_BACK_VBI, (0<<27)|(0<<26)|(1<<25)|(0x100<<16)|(0xfff<<0)); /* control gain also bit 16 */ cx_write(MO_AGC_GAIN_ADJ4, (ctd->sixdb<<23)|(0<<22)|(0<<21)|(ctd->level<<16)|(0xff<<8)|(0x0<<0)); @@ -1405,6 +1418,11 @@ static int cxadc_resume(struct pci_dev *pci_dev) if (ctd->level > 31) ctd->level = 31; + if (ctd->center_offset < 0) + ctd->center_offset = 0; + if (ctd->center_offset > 63) + ctd->center_offset = 63; + cx_write(MO_AGC_BACK_VBI, (0<<27)|(0<<26)|(1<<25)|(0x100<<16)|(0xfff<<0)); /* control gain also bit 16 */ cx_write(MO_AGC_GAIN_ADJ4, (ctd->sixdb<<23)|(0<<22)|(0<<21)|(ctd->level<<16)|(0xff<<8)|(0x0<<0)); From f7a848b80d5aab19c60317bd1e0b34e381d76e77 Mon Sep 17 00:00:00 2001 From: Rian Hunter Date: Wed, 29 May 2024 20:20:57 -0400 Subject: [PATCH 2/3] Update default center_offset value to 0 Also make sure the default value listed documented in the README.md is consistent with what is in the source code. --- README.md | 2 +- cxadc.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d3352e9..134ca2f 100644 --- a/README.md +++ b/README.md @@ -374,8 +374,8 @@ extra cooling required above 40mhz). This value is ONLY used to compute the sample rates entered for the tenxfsc parameters other than 0, 1, 2. -### `center_offset` (0 to 63, default 2) +### `center_offset` (0 to 63, default 0) This option allows you to manually adjust DC centre offset or the centring of the RF signal you wish to capture. diff --git a/cxadc.c b/cxadc.c index 24e9d30..26b6c1d 100644 --- a/cxadc.c +++ b/cxadc.c @@ -47,7 +47,7 @@ #define default_tenxfsc 0 #define default_sixdb 1 #define default_crystal 28636363 -#define default_center_offset 8 +#define default_center_offset 0 #define cx_read(reg) readl(ctd->mmio + ((reg) >> 2)) #define cx_write(reg, value) writel((value), ctd->mmio + ((reg) >> 2)) From a96c313ef28a95cf182463ea2cb17c7bba1015cc Mon Sep 17 00:00:00 2001 From: Rian Hunter Date: Wed, 29 May 2024 20:28:53 -0400 Subject: [PATCH 3/3] Log message when parameter was out of bound We also factor out bounding routine to minimize redundancy. --- cxadc.c | 65 +++++++++++++++++++++++++++------------------------------ 1 file changed, 31 insertions(+), 34 deletions(-) diff --git a/cxadc.c b/cxadc.c index 26b6c1d..d56cd90 100644 --- a/cxadc.c +++ b/cxadc.c @@ -56,6 +56,8 @@ dev_err(&ctd->pci->dev, fmt, ##__VA_ARGS__) #define cx_info(fmt, ...) \ dev_info(&ctd->pci->dev, fmt, ##__VA_ARGS__) +#define cx_notice(fmt, ...) \ + dev_notice(&ctd->pci->dev, fmt, ##__VA_ARGS__) /* 64 Mbytes VBI DMA BUFF */ #define VBI_DMA_BUFF_SIZE (1024*1024*64) @@ -627,6 +629,31 @@ static int make_risc_instructions(struct cxadc *ctd) return 0; } +static void bound_parameters(struct cxadc *ctd) +{ + if (ctd->level < 0) { + cx_notice("Negative 'level' value: %d, setting to 0\n", + ctd->level); + ctd->level = 0; + } + if (ctd->level > 31) { + cx_notice("Excessively large 'level' value: %d, setting to 31\n", + ctd->level); + ctd->level = 31; + } + + if (ctd->center_offset < 0) { + cx_notice("Negative 'center_offset' value: %d, setting to 0\n", + ctd->center_offset); + ctd->center_offset = 0; + } + if (ctd->center_offset > 63) { + cx_notice("Excessively large 'center_offset' value: %d, setting to 63\n", + ctd->level); + ctd->center_offset = 63; + } +} + static int cxadc_char_open(struct inode *inode, struct file *file) { int minor = iminor(inode); @@ -665,16 +692,9 @@ static int cxadc_char_open(struct inode *inode, struct file *file) /* re-set the level, clock speed, and bit size */ - if (ctd->level < 0) - ctd->level = 0; - if (ctd->level > 31) - ctd->level = 31; + bound_parameters(ctd); /* control gain also bit 16 */ cx_write(MO_AGC_GAIN_ADJ4, (ctd->sixdb<<23)|(0<<22)|(0<<21)|(ctd->level<<16)|(0xff<<8)|(0x0<<0)); - if (ctd->center_offset < 0) - ctd->center_offset = 0; - if (ctd->center_offset > 63) - ctd->center_offset = 63; cx_write(MO_AGC_SYNC_TIP3, (0x1e48<<16)|(0xff<<8)|(ctd->center_offset)); if (ctd->tenxfsc < 10) { @@ -803,15 +823,8 @@ static ssize_t cxadc_char_read(struct file *file, char __user *tgt, * adding code to allow level change during read, have tested, works with CAV capture * script i have been working on */ - if (ctd->level < 0) - ctd->level = 0; - if (ctd->level > 31) - ctd->level = 31; + bound_parameters(ctd); cx_write(MO_AGC_GAIN_ADJ4, (ctd->sixdb<<23)|(0<<22)|(0<<21)|(ctd->level<<16)|(0xff<<8)|(0x0<<0)); - if (ctd->center_offset < 0) - ctd->center_offset = 0; - if (ctd->center_offset > 63) - ctd->center_offset = 63; cx_write(MO_AGC_SYNC_TIP3, (0x1e48<<16)|(0xff<<8)|(ctd->center_offset)); if (count) { @@ -1156,15 +1169,7 @@ static int cxadc_probe(struct pci_dev *pci_dev, /* set vbi agc */ cx_write(MO_AGC_SYNC_SLICER, 0x0); - if (ctd->level < 0) - ctd->level = 0; - if (ctd->level > 31) - ctd->level = 31; - - if (ctd->center_offset < 0) - ctd->center_offset = 0; - if (ctd->center_offset > 63) - ctd->center_offset = 63; + bound_parameters(ctd); cx_write(MO_AGC_BACK_VBI, (0<<27)|(0<<26)|(1<<25)|(0x100<<16)|(0xfff<<0)); /* control gain also bit 16 */ @@ -1413,15 +1418,7 @@ static int cxadc_resume(struct pci_dev *pci_dev) /* set vbi agc */ cx_write(MO_AGC_SYNC_SLICER, 0x0); - if (ctd->level < 0) - ctd->level = 0; - if (ctd->level > 31) - ctd->level = 31; - - if (ctd->center_offset < 0) - ctd->center_offset = 0; - if (ctd->center_offset > 63) - ctd->center_offset = 63; + bound_parameters(ctd); cx_write(MO_AGC_BACK_VBI, (0<<27)|(0<<26)|(1<<25)|(0x100<<16)|(0xfff<<0)); /* control gain also bit 16 */