Re: [PATCH v2] staging: vboxvideo: Remove unnecessary parentheses
On Tue, Oct 23, 2018 at 02:43:04PM -0300, Shayenne da Luz Moura wrote: > Remove unneeded parentheses around the arguments of ||. This reduces > clutter and code behave in the same way. > Change suggested by checkpatch.pl. > > vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].x2 < > crtc->x' > > Signed-off-by: Shayenne da Luz Moura > Reviewed-by: Hans de Goede > --- > Changes in v2: > - Make the commit message more clearer. This patch does not apply to the latest kernel tree at all :( Please fix up and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: speakup: clean up few indentation issues
From: Colin Ian King
Trivial fix to clean up indentation issues across the driver
Signed-off-by: Colin Ian King
---
drivers/staging/speakup/kobjects.c | 2 +-
drivers/staging/speakup/speakup_decpc.c | 6 +++---
drivers/staging/speakup/speakup_keypc.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/speakup/kobjects.c
b/drivers/staging/speakup/kobjects.c
index 08f11cc17371..2e36d872662c 100644
--- a/drivers/staging/speakup/kobjects.c
+++ b/drivers/staging/speakup/kobjects.c
@@ -545,7 +545,7 @@ ssize_t spk_var_show(struct kobject *kobj, struct
kobj_attribute *attr,
int rv = 0;
struct st_var_header *param;
struct var_t *var;
- char *cp1;
+ char *cp1;
char *cp;
char ch;
unsigned long flags;
diff --git a/drivers/staging/speakup/speakup_decpc.c
b/drivers/staging/speakup/speakup_decpc.c
index 6649309e0342..459ee0c0bd57 100644
--- a/drivers/staging/speakup/speakup_decpc.c
+++ b/drivers/staging/speakup/speakup_decpc.c
@@ -302,12 +302,12 @@ static void synth_flush(struct spk_synth *synth)
while (dt_ctrl(CTRL_flush)) {
if (--timeout == 0)
break;
-udelay(50);
+ udelay(50);
}
for (timeout = 0; timeout < 10; timeout++) {
if (dt_waitbit(STAT_dma_ready))
break;
-udelay(50);
+ udelay(50);
}
outb_p(DMA_sync, speakup_info.port_tts + 4);
outb_p(0, speakup_info.port_tts + 4);
@@ -315,7 +315,7 @@ udelay(50);
for (timeout = 0; timeout < 10; timeout++) {
if (!(dt_getstatus() & STAT_flushing))
break;
-udelay(50);
+ udelay(50);
}
dma_state = dt_getstatus() & STAT_dma_state;
dma_state ^= STAT_dma_state;
diff --git a/drivers/staging/speakup/speakup_keypc.c
b/drivers/staging/speakup/speakup_keypc.c
index 3901734982a4..b788272da4f9 100644
--- a/drivers/staging/speakup/speakup_keypc.c
+++ b/drivers/staging/speakup/speakup_keypc.c
@@ -177,7 +177,7 @@ static void do_catch_up(struct spk_synth *synth)
jiffy_delta = spk_get_var(JIFFY);
delay_time = spk_get_var(DELAY);
full_time = spk_get_var(FULL);
-spin_lock_irqsave(&speakup_info.spinlock, flags);
+ spin_lock_irqsave(&speakup_info.spinlock, flags);
jiffy_delta_val = jiffy_delta->u.n.value;
spin_unlock_irqrestore(&speakup_info.spinlock, flags);
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6656: clean up few indentation issues
From: Colin Ian King
Trivial fix to clean up indentation issues
Signed-off-by: Colin Ian King
---
drivers/staging/vt6656/firmware.c | 2 +-
drivers/staging/vt6656/main_usb.c | 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/vt6656/firmware.c
b/drivers/staging/vt6656/firmware.c
index 38521c338917..205347cff7c4 100644
--- a/drivers/staging/vt6656/firmware.c
+++ b/drivers/staging/vt6656/firmware.c
@@ -42,7 +42,7 @@ int vnt_download_firmware(struct vnt_private *priv)
if (rc) {
dev_err(dev, "firmware file %s request failed (%d)\n",
FIRMWARE_NAME, rc);
- goto out;
+ goto out;
}
buffer = kmalloc(FIRMWARE_CHUNK_SIZE, GFP_KERNEL);
diff --git a/drivers/staging/vt6656/main_usb.c
b/drivers/staging/vt6656/main_usb.c
index ccafcc2c87ac..b613a1d113bb 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -245,10 +245,10 @@ static int vnt_init_registers(struct vnt_private *priv)
} else {
priv->tx_antenna_mode = ANT_B;
- if (priv->tx_rx_ant_inv)
- priv->rx_antenna_mode = ANT_A;
- else
- priv->rx_antenna_mode = ANT_B;
+ if (priv->tx_rx_ant_inv)
+ priv->rx_antenna_mode = ANT_A;
+ else
+ priv->rx_antenna_mode = ANT_B;
}
}
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: xgifb: clean an indentation issue
From: Colin Ian King
Trivial fix to clean up an indentation issue
Signed-off-by: Colin Ian King
---
drivers/staging/xgifb/XGI_main_26.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c
b/drivers/staging/xgifb/XGI_main_26.c
index eca0b50f0df6..217c6bb82c0d 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -923,8 +923,9 @@ static int XGIfb_do_set_var(struct fb_var_screeninfo *var,
int isactive,
if (!htotal || !vtotal) {
pr_debug("Invalid 'var' information\n");
return -EINVAL;
- } pr_debug("var->pixclock=%d, htotal=%d, vtotal=%d\n",
- var->pixclock, htotal, vtotal);
+ }
+ pr_debug("var->pixclock=%d, htotal=%d, vtotal=%d\n",
+var->pixclock, htotal, vtotal);
if (var->pixclock) {
drate = 10 / var->pixclock;
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Transaction
Dear Sir, My Name is Ms Maria Lucas ; Manager Of BOA Bank Abidjan Ci ; On the course of 2013/2014 Financial Annual Report, A Surplus Profit of Eleven Million Nine Hundred and Fifty Thousand Us Dollars , [ $ 11,950,000.00 ] Was Discovered and Placed in a SUSPENSE ACCOUNT Without Any Beneficiary. As an Employee Of the Bank , I can NOT be Directly Connected to the Fund For Security Reasons, That is why I am contacting You for us to Work Together as Partners to Receive the Said Fund in Your Account For INVESTMENT in Your Country. The percentage Ratio is thus: 40% for you , 60 % for me and my colleagues . Note: There are Practically No Risks Involved in this Transaction , It is 100% Risk Free and Shall Be Legally Bonded, All You Need to do is to Stand as the BENEFICIARY to the Deposit For a Proper Wire to Your Account .If you Find this Proposal Suitable For you, Kindly Reply For Full Details and Procedures . Best regards, Ms. Maria Lucas ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: speakup: clean up few indentation issues
Colin King, le mar. 30 oct. 2018 11:09:59 +, a ecrit:
> From: Colin Ian King
>
> Trivial fix to clean up indentation issues across the driver
>
> Signed-off-by: Colin Ian King
Reviewed-by: Samuel Thibault
> ---
> drivers/staging/speakup/kobjects.c | 2 +-
> drivers/staging/speakup/speakup_decpc.c | 6 +++---
> drivers/staging/speakup/speakup_keypc.c | 2 +-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/speakup/kobjects.c
> b/drivers/staging/speakup/kobjects.c
> index 08f11cc17371..2e36d872662c 100644
> --- a/drivers/staging/speakup/kobjects.c
> +++ b/drivers/staging/speakup/kobjects.c
> @@ -545,7 +545,7 @@ ssize_t spk_var_show(struct kobject *kobj, struct
> kobj_attribute *attr,
> int rv = 0;
> struct st_var_header *param;
> struct var_t *var;
> - char *cp1;
> + char *cp1;
> char *cp;
> char ch;
> unsigned long flags;
> diff --git a/drivers/staging/speakup/speakup_decpc.c
> b/drivers/staging/speakup/speakup_decpc.c
> index 6649309e0342..459ee0c0bd57 100644
> --- a/drivers/staging/speakup/speakup_decpc.c
> +++ b/drivers/staging/speakup/speakup_decpc.c
> @@ -302,12 +302,12 @@ static void synth_flush(struct spk_synth *synth)
> while (dt_ctrl(CTRL_flush)) {
> if (--timeout == 0)
> break;
> -udelay(50);
> + udelay(50);
> }
> for (timeout = 0; timeout < 10; timeout++) {
> if (dt_waitbit(STAT_dma_ready))
> break;
> -udelay(50);
> + udelay(50);
> }
> outb_p(DMA_sync, speakup_info.port_tts + 4);
> outb_p(0, speakup_info.port_tts + 4);
> @@ -315,7 +315,7 @@ udelay(50);
> for (timeout = 0; timeout < 10; timeout++) {
> if (!(dt_getstatus() & STAT_flushing))
> break;
> -udelay(50);
> + udelay(50);
> }
> dma_state = dt_getstatus() & STAT_dma_state;
> dma_state ^= STAT_dma_state;
> diff --git a/drivers/staging/speakup/speakup_keypc.c
> b/drivers/staging/speakup/speakup_keypc.c
> index 3901734982a4..b788272da4f9 100644
> --- a/drivers/staging/speakup/speakup_keypc.c
> +++ b/drivers/staging/speakup/speakup_keypc.c
> @@ -177,7 +177,7 @@ static void do_catch_up(struct spk_synth *synth)
> jiffy_delta = spk_get_var(JIFFY);
> delay_time = spk_get_var(DELAY);
> full_time = spk_get_var(FULL);
> -spin_lock_irqsave(&speakup_info.spinlock, flags);
> + spin_lock_irqsave(&speakup_info.spinlock, flags);
> jiffy_delta_val = jiffy_delta->u.n.value;
> spin_unlock_irqrestore(&speakup_info.spinlock, flags);
>
> --
> 2.19.1
>
--
Samuel
je sens venir la fonte 14 pour le rapport
-+- #ens-mim -+-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: change do_insn*_ioctl to allow more samples
On 25/10/18 02:36, Spencer E. Olson wrote:
Changes do_insn*_ioctl functions to allow for data lengths for each
comedi_insn of up to 2^16. This patch also changes these functions to only
allocate as much memory as is necessary for each comedi_insn, rather than
allocating a fixed-sized scratch space.
In testing some user-space code for the new INSN_DEVICE_CONFIG_GET_ROUTES
facility with some newer hardware, I discovered that do_insn_ioctl and
do_insnlist_ioctl limited the amount of data that can be passed into the
kernel for insn's to a length of 256. For some newer hardware, the number
of routes can be greater than 1000. Working around the old limits (256)
would complicate the user-space/kernel interaction.
The new upper limit is reasonable with current memory available and does
not otherwise impact the memory footprint for any current or otherwise
typical configuration.
Signed-off-by: Spencer E. Olson
---
drivers/staging/comedi/comedi_fops.c | 37 +---
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index c1c6b2b4ab91..a163ec2df872 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1500,7 +1500,7 @@ static int parse_insn(struct comedi_device *dev, struct
comedi_insn *insn,
*data (for reads) to insns[].data pointers
*/
/* arbitrary limits */
-#define MAX_SAMPLES 256
+#define MAX_SAMPLES 65536
static int do_insnlist_ioctl(struct comedi_device *dev,
struct comedi_insnlist __user *arg, void *file)
{
@@ -1513,12 +1513,6 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
if (copy_from_user(&insnlist, arg, sizeof(insnlist)))
return -EFAULT;
- data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
- if (!data) {
- ret = -ENOMEM;
- goto error;
- }
-
insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL);
if (!insns) {
ret = -ENOMEM;
@@ -1539,6 +1533,14 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
ret = -EINVAL;
goto error;
}
+
+ data = kmalloc_array(insns[i].n, sizeof(unsigned int),
+GFP_KERNEL);
+ if (!data) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
if (insns[i].insn & INSN_MASK_WRITE) {
if (copy_from_user(data, insns[i].data,
insns[i].n * sizeof(unsigned int))) {
@@ -1560,6 +1562,11 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
goto error;
}
}
+
+ /* free up the single-instruction data memory */
+ kfree(data);
+ data = NULL;
+
if (need_resched())
schedule();
}
@@ -1594,20 +1601,20 @@ static int do_insn_ioctl(struct comedi_device *dev,
unsigned int *data = NULL;
int ret = 0;
- data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
- if (!data) {
- ret = -ENOMEM;
- goto error;
- }
-
if (copy_from_user(&insn, arg, sizeof(insn))) {
- ret = -EFAULT;
- goto error;
+ return -EFAULT;
}
/* This is where the behavior of insn and insnlist deviate. */
if (insn.n > MAX_SAMPLES)
insn.n = MAX_SAMPLES;
+
+ data = kmalloc_array(insn.n, sizeof(unsigned int), GFP_KERNEL);
+ if (!data) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
if (insn.insn & INSN_MASK_WRITE) {
if (copy_from_user(data,
insn.data,
There are still some broken drivers that do not check insn->n and assume
the data has some minimum length, so we will need to make the data array
some minimum length (say 16 * sizeof(unsigned int)) until those drivers
have been fixed.
For example (there might be other places) ...
In cb_pcidas64.c:
* ao_winsn() (the insn_write handler for the AO subdevice) assumes that
insn->n is 1 and reads data[0], but insn->n could be 0.
* eeprom_read_insn() (the insn_read handler for the serial EEPROM) also
assumes insn->n is 1 and accesses data[0].
* ai_config_master_clock() (the handler for INSN_CONFIG_TIMER_1) assumes
insn->n is at least 5 but nothing checks it. (It isn't checked by
check_insn_config_length() in comedi_fops.c currently.)
In cb_pcidda.c:
* cb_pcidda_ao_insn_write() assumes insn->n is 1 and that data[0] is valid.
In ni_labpc_common.c:
* labpc_ao_insn_write() assumes insn->n is 1 and that data[0] is valid.
In ni_mio_common.c:
* ni_calib_insn_read(), ni_eeprom_insn_re
[PATCH 1/3] staging: comedi: cb_pcidas64: Use insn->n in AO insn_write handler
The `insn_write` handler for the AO subdevice (`ao_winsn()` currently
ignores `insn->n` (the number of samples to write) and assumes a single
sample is to be written. But `insn->n` could be 0, meaning no samples
should be written, in which case `data[0]` is invalid.
Follow the usual Comedi guidelines and change `ao_winsn()` to write the
specified number of samples. This fixes the assumption that `data[0]`
is valid.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/cb_pcidas64.c | 32
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c
b/drivers/staging/comedi/drivers/cb_pcidas64.c
index 631a703b345d..44e5aaf8bae5 100644
--- a/drivers/staging/comedi/drivers/cb_pcidas64.c
+++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
@@ -3097,8 +3097,10 @@ static int ao_winsn(struct comedi_device *dev, struct
comedi_subdevice *s,
{
const struct pcidas64_board *board = dev->board_ptr;
struct pcidas64_private *devpriv = dev->private;
- int chan = CR_CHAN(insn->chanspec);
- int range = CR_RANGE(insn->chanspec);
+ unsigned int chan = CR_CHAN(insn->chanspec);
+ unsigned int range = CR_RANGE(insn->chanspec);
+ unsigned int val = s->readback[chan];
+ unsigned int i;
/* do some initializing */
writew(0, devpriv->main_iobase + DAC_CONTROL0_REG);
@@ -3108,20 +3110,24 @@ static int ao_winsn(struct comedi_device *dev, struct
comedi_subdevice *s,
writew(devpriv->dac_control1_bits,
devpriv->main_iobase + DAC_CONTROL1_REG);
- /* write to channel */
- if (board->layout == LAYOUT_4020) {
- writew(data[0] & 0xff,
- devpriv->main_iobase + dac_lsb_4020_reg(chan));
- writew((data[0] >> 8) & 0xf,
- devpriv->main_iobase + dac_msb_4020_reg(chan));
- } else {
- writew(data[0], devpriv->main_iobase + dac_convert_reg(chan));
+ for (i = 0; i < insn->n; i++) {
+ /* write to channel */
+ val = data[i];
+ if (board->layout == LAYOUT_4020) {
+ writew(val & 0xff,
+ devpriv->main_iobase + dac_lsb_4020_reg(chan));
+ writew((val >> 8) & 0xf,
+ devpriv->main_iobase + dac_msb_4020_reg(chan));
+ } else {
+ writew(val,
+ devpriv->main_iobase + dac_convert_reg(chan));
+ }
}
- /* remember output value */
- s->readback[chan] = data[0];
+ /* remember last output value */
+ s->readback[chan] = val;
- return 1;
+ return insn->n;
}
static void set_dac_control0_reg(struct comedi_device *dev,
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] staging: comedi: cb_pcidas64: Fix insn->n assumptions
Fix some assumptions about the length of Comedi instructions in the "cb_pcidas64" driver. 1) staging: comedi: cb_pcidas64: Use insn->n in AO insn_write handler 2) staging: comedi: cb_pcidas64: Use insn->n in EEPROM insn_read handler 3) staging: comedi: Check length of INSN_CONFIG_TIMER_1 instruction drivers/staging/comedi/comedi_fops.c | 1 + drivers/staging/comedi/drivers/cb_pcidas64.c | 44 ++-- 2 files changed, 30 insertions(+), 15 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: comedi: Check length of INSN_CONFIG_TIMER_1 instruction
The contents of the Comedi configuration instruction `INSN_CONFIG_TIMER_1` instruction are not very well defined, but the one driver that uses it (the "cb_pcidas64" driver for the PCI-DAS4020/12 card) assumes its `insn->n` is 5. Add a check in `check_insn_config_length()` to verify that `insn->n` is correct for this configuration instruction. Signed-off-by: Ian Abbott --- drivers/staging/comedi/comedi_fops.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index c1c6b2b4ab91..ceb6ba5dd57c 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -1209,6 +1209,7 @@ static int check_insn_config_length(struct comedi_insn *insn, break; case INSN_CONFIG_PWM_OUTPUT: case INSN_CONFIG_ANALOG_TRIG: + case INSN_CONFIG_TIMER_1: if (insn->n == 5) return 0; break; -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: comedi: cb_pcidas64: Use insn->n in EEPROM insn_read handler
The `insn_read` handler for the EEPROM subdevice (`eeprom_insn_read()`)
currently ignores `insn->n` (the number of samples to be read) and
assumes a single sample is to be read. But `insn->n` could be 0,
meaning no samples should be read, in which case `data[0]` ought not to
be written. (The comedi core at least ensures that `data[0]` exists,
but we should not rely on that.)
Follow the usual Comedi guidelines and interpret `insn->n` as the number
of samples to be read, but only read the EEPROM location once and make
`insn->n` copies, as we don't expect the contents of the EEPROM location
to change between readings.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/cb_pcidas64.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c
b/drivers/staging/comedi/drivers/cb_pcidas64.c
index 44e5aaf8bae5..e1774e09a320 100644
--- a/drivers/staging/comedi/drivers/cb_pcidas64.c
+++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
@@ -3768,9 +3768,17 @@ static int eeprom_read_insn(struct comedi_device *dev,
struct comedi_subdevice *s,
struct comedi_insn *insn, unsigned int *data)
{
- data[0] = read_eeprom(dev, CR_CHAN(insn->chanspec));
+ unsigned int val;
+ unsigned int i;
- return 1;
+ if (insn->n) {
+ /* No point reading the same EEPROM location more than once. */
+ val = read_eeprom(dev, CR_CHAN(insn->chanspec));
+ for (i = 0; i < insn->n; i++)
+ data[i] = val;
+ }
+
+ return insn->n;
}
/* Allocate and initialize the subdevice structures. */
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: cb_pcidda: Use insn->n in AO insn_write handler
The `insn_write` handler for the AO subdevice (`cb_pcidda_ao_insn_write()`) currently ignores `insn->n` (the number of samples to write) and assumes a single sample is to be written. But `insn->n` could be 0, meaning no samples should be written, in which case `data[0]` is invalid. Follow the usual Comedi guidelines and change `cb_pcidda_ao_insn_write()` to write the specified number of samples. This fixes the assumption that `data[0]` is valid. Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/cb_pcidda.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/cb_pcidda.c b/drivers/staging/comedi/drivers/cb_pcidda.c index 807a31e75883..1d09dd265ab7 100644 --- a/drivers/staging/comedi/drivers/cb_pcidda.c +++ b/drivers/staging/comedi/drivers/cb_pcidda.c @@ -291,6 +291,7 @@ static int cb_pcidda_ao_insn_write(struct comedi_device *dev, unsigned int channel = CR_CHAN(insn->chanspec); unsigned int range = CR_RANGE(insn->chanspec); unsigned int ctrl; + unsigned int i; if (range != devpriv->ao_range[channel]) cb_pcidda_calibrate(dev, channel, range); @@ -317,7 +318,8 @@ static int cb_pcidda_ao_insn_write(struct comedi_device *dev, outw(ctrl, devpriv->daqio + CB_DDA_DA_CTRL_REG); - outw(data[0], devpriv->daqio + CB_DDA_DA_DATA_REG(channel)); + for (i = 0; i < insn->n; i++) + outw(data[i], devpriv->daqio + CB_DDA_DA_DATA_REG(channel)); return insn->n; } -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 2/3] staging: comedi: cb_pcidas64: Use insn->n in EEPROM insn_read handler
On Tuesday, October 30, 2018 7:17 AM, Ian Abbott wrote:
> The `insn_read` handler for the EEPROM subdevice (`eeprom_insn_read()`)
> currently
> ignores `insn->n` (the number of samples to be read) and assumes a single
> sample is
> to be read. But `insn->n` could be 0, meaning no samples should be read, in
> which
> case `data[0]` ought not to be written. (The comedi core at least ensures
> that
> `data[0]` exists, but we should not rely on that.)
>
> Follow the usual Comedi guidelines and interpret `insn->n` as the number of
> samples
> to be read, but only read the EEPROM location once and make `insn->n` copies,
> as we
> don't expect the contents of the EEPROM location to change between readings.
>
> Signed-off-by: Ian Abbott
> ---
> drivers/staging/comedi/drivers/cb_pcidas64.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c
> b/drivers/staging/comedi/drivers/cb_pcidas64.c
> index 44e5aaf8bae5..e1774e09a320 100644
> --- a/drivers/staging/comedi/drivers/cb_pcidas64.c
> +++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
> @@ -3768,9 +3768,17 @@ static int eeprom_read_insn(struct comedi_device *dev,
> struct comedi_subdevice *s,
> struct comedi_insn *insn, unsigned int *data) {
> - data[0] = read_eeprom(dev, CR_CHAN(insn->chanspec));
> + unsigned int val;
> + unsigned int i;
>
> - return 1;
> + if (insn->n) {
> + /* No point reading the same EEPROM location more than once. */
> + val = read_eeprom(dev, CR_CHAN(insn->chanspec));
> + for (i = 0; i < insn->n; i++)
> + data[i] = val;
> + }
> +
> + return insn->n;
> }
Hi Ian,
I realize it's not the "normal" Comedi use, but with the EEPROM I would think
that if the user
wanted to read more than one sample they would expect to read a block of data
from the
EEPROM.
Maybe the EEPROM read should be something like:
+ if (insn->n) {
+ unsigned int address = CR_CHAN(insn->chanspec);
+ for (i = 0; i < insn->n; i++) {
+ val = read_eeprom(dev, address);
+ data[i] = val;
+ address++;
+ if (address >= s->n_chan)
+ address = 0;
+ }
+ }
Regards,
Hartley
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/6] staging:iio:ad2s90: Add scale info and improve error handling
On Sun, Oct 28, 2018 at 1:52 PM Jonathan Cameron wrote: > > On Fri, 26 Oct 2018 22:59:59 -0300 > Matheus Tavares wrote: > > > This patch set adds scale info to ad2s90's single channel, improve > > error handling in it's functions and fix a possible race condition > > issue. > > > > The goal with this patch set is to address the points discussed in the > > mailing list in an effort to move ad2s90.c out of staging. > Thanks, > > A good series in general. A few suggested improvements. > If I haven't commented on a patch, usually it means I'm happy with it > and will pick it up with the rest of the series. > > Jonathan > Thanks for the review, Jonathan. We will address the necessary changes in v3! Matheus > > > > Changes in v2: > > - Added my S-o-B in patch 5. > > > > Matheus Tavares (5): > > staging:iio:ad2s90: Make read_raw return spi_read's error code > > staging:iio:ad2s90: Make probe handle spi_setup failure > > staging:iio:ad2s90: Remove always overwritten assignment > > staging:iio:ad2s90: Move device registration to the end of probe > > staging:iio:ad2s90: Check channel type at read_raw > > > > Victor Colombo (1): > > staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and > > read_raw > > > > drivers/staging/iio/resolver/ad2s90.c | 55 ++- > > 1 file changed, 37 insertions(+), 18 deletions(-) > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] staging: comedi: cb_pcidas64: Use insn->n in EEPROM insn_read handler
On 30/10/18 15:34, Hartley Sweeten wrote:
On Tuesday, October 30, 2018 7:17 AM, Ian Abbott wrote:
The `insn_read` handler for the EEPROM subdevice (`eeprom_insn_read()`)
currently
ignores `insn->n` (the number of samples to be read) and assumes a single
sample is
to be read. But `insn->n` could be 0, meaning no samples should be read, in
which
case `data[0]` ought not to be written. (The comedi core at least ensures that
`data[0]` exists, but we should not rely on that.)
Follow the usual Comedi guidelines and interpret `insn->n` as the number of
samples
to be read, but only read the EEPROM location once and make `insn->n` copies,
as we
don't expect the contents of the EEPROM location to change between readings.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/cb_pcidas64.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c
b/drivers/staging/comedi/drivers/cb_pcidas64.c
index 44e5aaf8bae5..e1774e09a320 100644
--- a/drivers/staging/comedi/drivers/cb_pcidas64.c
+++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
@@ -3768,9 +3768,17 @@ static int eeprom_read_insn(struct comedi_device *dev,
struct comedi_subdevice *s,
struct comedi_insn *insn, unsigned int *data) {
- data[0] = read_eeprom(dev, CR_CHAN(insn->chanspec));
+ unsigned int val;
+ unsigned int i;
- return 1;
+ if (insn->n) {
+ /* No point reading the same EEPROM location more than once. */
+ val = read_eeprom(dev, CR_CHAN(insn->chanspec));
+ for (i = 0; i < insn->n; i++)
+ data[i] = val;
+ }
+
+ return insn->n;
}
Hi Ian,
I realize it's not the "normal" Comedi use, but with the EEPROM I would think
that if the user
wanted to read more than one sample they would expect to read a block of data
from the
EEPROM.
Maybe the EEPROM read should be something like:
+ if (insn->n) {
+ unsigned int address = CR_CHAN(insn->chanspec);
+ for (i = 0; i < insn->n; i++) {
+ val = read_eeprom(dev, address);
+ data[i] = val;
+ address++;
+ if (address >= s->n_chan)
+ address = 0;
+ }
+ }
Regards,
Hartley
I'd rather keep it consistent and use the "normal" Comedi meaning. We
already have some insn_write handlers for EEPROM that just write the
final sample from the data (data[insn->n - 1]) to the location specified
by insn->chanspec (see labpc_eeprom_insn_write() in
.../comedi/drivers/ni_labpc_common.c), so it would be inconsistent with
that.
--
-=( Ian Abbott || 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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: ni_labpc_common: Use insn->n in AO insn_write handler
The `insn_write` handler for the AO subdevice (`labpc_ao_insn_write()`)
currently ignores `insn->n` (the number of samples to write) and assumes
a single sample is to be written. But `insn->n` could be 0, meaning no
samples should be written, in which case `data[0]` is invalid.
Follow the usual Comedi guidelines and change `labpc_ao_insn_write()` to
write the specified number of samples. This fixes the assumption that
`data[0]` is valid.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/ni_labpc_common.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_labpc_common.c
b/drivers/staging/comedi/drivers/ni_labpc_common.c
index 7fa2d39562db..406952f5521d 100644
--- a/drivers/staging/comedi/drivers/ni_labpc_common.c
+++ b/drivers/staging/comedi/drivers/ni_labpc_common.c
@@ -906,7 +906,9 @@ static int labpc_ao_insn_write(struct comedi_device *dev,
{
const struct labpc_boardinfo *board = dev->board_ptr;
struct labpc_private *devpriv = dev->private;
- int channel, range;
+ unsigned int channel;
+ unsigned int range;
+ unsigned int i;
unsigned long flags;
channel = CR_CHAN(insn->chanspec);
@@ -932,9 +934,10 @@ static int labpc_ao_insn_write(struct comedi_device *dev,
devpriv->write_byte(dev, devpriv->cmd6, CMD6_REG);
}
/* send data */
- labpc_ao_write(dev, s, channel, data[0]);
+ for (i = 0; i < insn->n; i++)
+ labpc_ao_write(dev, s, channel, data[i]);
- return 1;
+ return insn->n;
}
/* lowlevel write to eeprom/dac */
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool
Hi,
> On Sun, 28 Oct 2018, Himanshu Jha wrote:
>
> > On Sun, Oct 28, 2018 at 09:47:15AM +0100, Julia Lawall wrote:
> > > > The "possible alignement issues" in CHECK report is difficult to figure
> > > > out by just doing a glance analysis. :)
> > > >
> > > > Linus also suggested to use bool as the base type i.e., `bool x:1` but
> > > > again sizeof(_Bool) is implementation defined ranging from 1-4 bytes.
> > >
> > > If bool x:1 has the size of bool, then wouldn't int x:1 have the size of
> > > int? But my little experiments suggest that the size is the smallest that
> > > fits the requested bits and alignment chosen by the compiler, regardless
> > > of
> > > the type.
> >
> > Yes, correct!
> > And we can't use sizeof on bitfields *directly*, nor reference it using a
> > pointer.
> >
> > It can be applied only when these bitfields are wrapped in a structure.
> >
> > Testing:
> >
> > #include
> > #include
> >
> > struct S {
> > bool a:1;
> > bool b:1;
> > bool c:1;
> > bool d:1;
> > };
> >
> > int main(void)
> > {
> > printf("%zu\n", sizeof(struct S));
> > }
> >
> > Output: 1
> >
> > If I change all bool to unsigned int, output is: *4*.
> >
> > So, conclusion is compiler doesn't squeeze the size less than
> > native size of the datatype i.e., if we changed all members to
> > unsigned int:1,
> > total width = 4 bits
> > padding = 4 bits
> >
> > Therefore, total size should have been = 1 byte!
> > But since sizeof(unsigned int) == 4, it can't be squeezed to
> > less than it.
>
> This conclusion does not seem to be correct, if you try the following
> program. I get 4 for everything, meaning that the four unsigned int bits
> are getting squeezed into one byte when it is convenient.
>
> #include
> #include
>
> struct S1 {
> bool a:1;
> bool b:1;
> bool c:1;
> bool d:1;
> char a1;
> char a2;
> char a3;
> };
>
> struct S2 {
> unsigned int a:1;
> unsigned int b:1;
> unsigned int c:1;
> unsigned int d:1;
> char a1;
> char a2;
> char a3;
> };
>
> int main(void)
> {
> printf("%zu\n", sizeof(struct S1));
> printf("%zu\n", sizeof(struct S2));
> printf("%zu\n", sizeof(unsigned int));
> }
>
> > Well, int x:1 can either have 0..1 or -1..0 range due implementation
> > defined behavior as I said in the previous reply.
> >
> > If you really want to consider negative values, then make it explicit
> > using `signed int x:1` which make range guaranteed to be -1..0
>
> The code wants booleans, not negative values.
>
> julia
Thank you all for the discussion!
However, I think I do not understand the conclusion.
It means that the best way is to use only boolean instead of use unsigned
int with bitfield? I mean specifically in the case of my patch, where there
are some boolean variables are mixed with other variables types.
Best,
Shayenne
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool
On Tue, 30 Oct 2018, Shayenne Moura wrote:
> Hi,
>
> > On Sun, 28 Oct 2018, Himanshu Jha wrote:
> >
> > > On Sun, Oct 28, 2018 at 09:47:15AM +0100, Julia Lawall wrote:
> > > > > The "possible alignement issues" in CHECK report is difficult to
> > > > > figure
> > > > > out by just doing a glance analysis. :)
> > > > >
> > > > > Linus also suggested to use bool as the base type i.e., `bool x:1` but
> > > > > again sizeof(_Bool) is implementation defined ranging from 1-4 bytes.
> > > >
> > > > If bool x:1 has the size of bool, then wouldn't int x:1 have the size of
> > > > int? But my little experiments suggest that the size is the smallest
> > > > that
> > > > fits the requested bits and alignment chosen by the compiler,
> > > > regardless of
> > > > the type.
> > >
> > > Yes, correct!
> > > And we can't use sizeof on bitfields *directly*, nor reference it using a
> > > pointer.
> > >
> > > It can be applied only when these bitfields are wrapped in a structure.
> > >
> > > Testing:
> > >
> > > #include
> > > #include
> > >
> > > struct S {
> > > bool a:1;
> > > bool b:1;
> > > bool c:1;
> > > bool d:1;
> > > };
> > >
> > > int main(void)
> > > {
> > > printf("%zu\n", sizeof(struct S));
> > > }
> > >
> > > Output: 1
> > >
> > > If I change all bool to unsigned int, output is: *4*.
> > >
> > > So, conclusion is compiler doesn't squeeze the size less than
> > > native size of the datatype i.e., if we changed all members to
> > > unsigned int:1,
> > > total width = 4 bits
> > > padding = 4 bits
> > >
> > > Therefore, total size should have been = 1 byte!
> > > But since sizeof(unsigned int) == 4, it can't be squeezed to
> > > less than it.
> >
> > This conclusion does not seem to be correct, if you try the following
> > program. I get 4 for everything, meaning that the four unsigned int bits
> > are getting squeezed into one byte when it is convenient.
> >
> > #include
> > #include
> >
> > struct S1 {
> > bool a:1;
> > bool b:1;
> > bool c:1;
> > bool d:1;
> > char a1;
> > char a2;
> > char a3;
> > };
> >
> > struct S2 {
> > unsigned int a:1;
> > unsigned int b:1;
> > unsigned int c:1;
> > unsigned int d:1;
> > char a1;
> > char a2;
> > char a3;
> > };
> >
> > int main(void)
> > {
> > printf("%zu\n", sizeof(struct S1));
> > printf("%zu\n", sizeof(struct S2));
> > printf("%zu\n", sizeof(unsigned int));
> > }
> >
> > > Well, int x:1 can either have 0..1 or -1..0 range due implementation
> > > defined behavior as I said in the previous reply.
> > >
> > > If you really want to consider negative values, then make it explicit
> > > using `signed int x:1` which make range guaranteed to be -1..0
> >
> > The code wants booleans, not negative values.
> >
> > julia
>
> Thank you all for the discussion!
>
> However, I think I do not understand the conclusion.
>
> It means that the best way is to use only boolean instead of use unsigned
> int with bitfield? I mean specifically in the case of my patch, where there
> are some boolean variables are mixed with other variables types.
To my recollection, your code had a bool with larger types on either side.
In that case, I think bool is fine. The compiler it likely to align those
larger typed values such that the field with the bool type will get more
than one byte no matter what type you use. If there are several fields
with very small types adjacent, there might be some benefit to thinking
about what the type should be.
julia
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] [RESEND PATCH 2/2] staging: vboxvideo: Use unsigned int instead bool
On 10/30, Julia Lawall wrote:
>
> On Tue, 30 Oct 2018, Shayenne Moura wrote:
>
> > Hi,
> >
> > > On Sun, 28 Oct 2018, Himanshu Jha wrote:
> > >
> > > > On Sun, Oct 28, 2018 at 09:47:15AM +0100, Julia Lawall wrote:
> > > > > > The "possible alignement issues" in CHECK report is difficult to
> > > > > > figure
> > > > > > out by just doing a glance analysis. :)
> > > > > >
> > > > > > Linus also suggested to use bool as the base type i.e., `bool x:1`
> > > > > > but
> > > > > > again sizeof(_Bool) is implementation defined ranging from 1-4
> > > > > > bytes.
> > > > >
> > > > > If bool x:1 has the size of bool, then wouldn't int x:1 have the size
> > > > > of
> > > > > int? But my little experiments suggest that the size is the smallest
> > > > > that
> > > > > fits the requested bits and alignment chosen by the compiler,
> > > > > regardless of
> > > > > the type.
> > > >
> > > > Yes, correct!
> > > > And we can't use sizeof on bitfields *directly*, nor reference it using
> > > > a
> > > > pointer.
> > > >
> > > > It can be applied only when these bitfields are wrapped in a structure.
> > > >
> > > > Testing:
> > > >
> > > > #include
> > > > #include
> > > >
> > > > struct S {
> > > > bool a:1;
> > > > bool b:1;
> > > > bool c:1;
> > > > bool d:1;
> > > > };
> > > >
> > > > int main(void)
> > > > {
> > > > printf("%zu\n", sizeof(struct S));
> > > > }
> > > >
> > > > Output: 1
> > > >
> > > > If I change all bool to unsigned int, output is: *4*.
> > > >
> > > > So, conclusion is compiler doesn't squeeze the size less than
> > > > native size of the datatype i.e., if we changed all members to
> > > > unsigned int:1,
> > > > total width = 4 bits
> > > > padding = 4 bits
> > > >
> > > > Therefore, total size should have been = 1 byte!
> > > > But since sizeof(unsigned int) == 4, it can't be squeezed to
> > > > less than it.
> > >
> > > This conclusion does not seem to be correct, if you try the following
> > > program. I get 4 for everything, meaning that the four unsigned int bits
> > > are getting squeezed into one byte when it is convenient.
> > >
> > > #include
> > > #include
> > >
> > > struct S1 {
> > > bool a:1;
> > > bool b:1;
> > > bool c:1;
> > > bool d:1;
> > > char a1;
> > > char a2;
> > > char a3;
> > > };
> > >
> > > struct S2 {
> > > unsigned int a:1;
> > > unsigned int b:1;
> > > unsigned int c:1;
> > > unsigned int d:1;
> > > char a1;
> > > char a2;
> > > char a3;
> > > };
> > >
> > > int main(void)
> > > {
> > > printf("%zu\n", sizeof(struct S1));
> > > printf("%zu\n", sizeof(struct S2));
> > > printf("%zu\n", sizeof(unsigned int));
> > > }
> > >
> > > > Well, int x:1 can either have 0..1 or -1..0 range due implementation
> > > > defined behavior as I said in the previous reply.
> > > >
> > > > If you really want to consider negative values, then make it explicit
> > > > using `signed int x:1` which make range guaranteed to be -1..0
> > >
> > > The code wants booleans, not negative values.
> > >
> > > julia
> >
> > Thank you all for the discussion!
> >
> > However, I think I do not understand the conclusion.
> >
> > It means that the best way is to use only boolean instead of use unsigned
> > int with bitfield? I mean specifically in the case of my patch, where there
> > are some boolean variables are mixed with other variables types.
>
> To my recollection, your code had a bool with larger types on either side.
> In that case, I think bool is fine. The compiler it likely to align those
> larger typed values such that the field with the bool type will get more
> than one byte no matter what type you use. If there are several fields
> with very small types adjacent, there might be some benefit to thinking
> about what the type should be.
>
> julia
Got it! Thank you!
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: change do_insn*_ioctl to allow more samples
On Tue, Oct 30, 2018 at 6:21 AM Ian Abbott wrote:
>
> On 25/10/18 02:36, Spencer E. Olson wrote:
> > Changes do_insn*_ioctl functions to allow for data lengths for each
> > comedi_insn of up to 2^16. This patch also changes these functions to only
> > allocate as much memory as is necessary for each comedi_insn, rather than
> > allocating a fixed-sized scratch space.
> >
> > In testing some user-space code for the new INSN_DEVICE_CONFIG_GET_ROUTES
> > facility with some newer hardware, I discovered that do_insn_ioctl and
> > do_insnlist_ioctl limited the amount of data that can be passed into the
> > kernel for insn's to a length of 256. For some newer hardware, the number
> > of routes can be greater than 1000. Working around the old limits (256)
> > would complicate the user-space/kernel interaction.
> >
> > The new upper limit is reasonable with current memory available and does
> > not otherwise impact the memory footprint for any current or otherwise
> > typical configuration.
> >
> > Signed-off-by: Spencer E. Olson
> > ---
> > drivers/staging/comedi/comedi_fops.c | 37 +---
> > 1 file changed, 22 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/staging/comedi/comedi_fops.c
> > b/drivers/staging/comedi/comedi_fops.c
> > index c1c6b2b4ab91..a163ec2df872 100644
> > --- a/drivers/staging/comedi/comedi_fops.c
> > +++ b/drivers/staging/comedi/comedi_fops.c
> > @@ -1500,7 +1500,7 @@ static int parse_insn(struct comedi_device *dev,
> > struct comedi_insn *insn,
> >* data (for reads) to insns[].data pointers
> >*/
> > /* arbitrary limits */
> > -#define MAX_SAMPLES 256
> > +#define MAX_SAMPLES 65536
> > static int do_insnlist_ioctl(struct comedi_device *dev,
> >struct comedi_insnlist __user *arg, void *file)
> > {
> > @@ -1513,12 +1513,6 @@ static int do_insnlist_ioctl(struct comedi_device
> > *dev,
> > if (copy_from_user(&insnlist, arg, sizeof(insnlist)))
> > return -EFAULT;
> >
> > - data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
> > - if (!data) {
> > - ret = -ENOMEM;
> > - goto error;
> > - }
> > -
> > insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL);
> > if (!insns) {
> > ret = -ENOMEM;
> > @@ -1539,6 +1533,14 @@ static int do_insnlist_ioctl(struct comedi_device
> > *dev,
> > ret = -EINVAL;
> > goto error;
> > }
> > +
> > + data = kmalloc_array(insns[i].n, sizeof(unsigned int),
> > + GFP_KERNEL);
> > + if (!data) {
> > + ret = -ENOMEM;
> > + goto error;
> > + }
> > +
> > if (insns[i].insn & INSN_MASK_WRITE) {
> > if (copy_from_user(data, insns[i].data,
> > insns[i].n * sizeof(unsigned
> > int))) {
> > @@ -1560,6 +1562,11 @@ static int do_insnlist_ioctl(struct comedi_device
> > *dev,
> > goto error;
> > }
> > }
> > +
> > + /* free up the single-instruction data memory */
> > + kfree(data);
> > + data = NULL;
> > +
> > if (need_resched())
> > schedule();
> > }
> > @@ -1594,20 +1601,20 @@ static int do_insn_ioctl(struct comedi_device *dev,
> > unsigned int *data = NULL;
> > int ret = 0;
> >
> > - data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
> > - if (!data) {
> > - ret = -ENOMEM;
> > - goto error;
> > - }
> > -
> > if (copy_from_user(&insn, arg, sizeof(insn))) {
> > - ret = -EFAULT;
> > - goto error;
> > + return -EFAULT;
> > }
> >
> > /* This is where the behavior of insn and insnlist deviate. */
> > if (insn.n > MAX_SAMPLES)
> > insn.n = MAX_SAMPLES;
> > +
> > + data = kmalloc_array(insn.n, sizeof(unsigned int), GFP_KERNEL);
> > + if (!data) {
> > + ret = -ENOMEM;
> > + goto error;
> > + }
> > +
> > if (insn.insn & INSN_MASK_WRITE) {
> > if (copy_from_user(data,
> > insn.data,
> >
>
> There are still some broken drivers that do not check insn->n and assume
> the data has some minimum length, so we will need to make the data array
> some minimum length (say 16 * sizeof(unsigned int)) until those drivers
> have been fixed.
>
I don't think that would be much of a problem, since 16*sizeof(uint)
is pretty negligible. How certain are you that we would not have to
make that higher, as the previous default was a length of
256*sizeof(uint)? It'll take me a day or two to resubmit (have other
things pressing on my time).
> For example (there might be other places) ...
>
> In cb_pcidas64.c:
>
> *
Re: [PATCH v2] staging: vboxvideo: Remove unnecessary parentheses
On 10/30, Greg Kroah-Hartman wrote: > On Tue, Oct 23, 2018 at 02:43:04PM -0300, Shayenne da Luz Moura wrote: > > Remove unneeded parentheses around the arguments of ||. This reduces > > clutter and code behave in the same way. > > Change suggested by checkpatch.pl. > > > > vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].x2 < > > crtc->x' > > > > Signed-off-by: Shayenne da Luz Moura > > Reviewed-by: Hans de Goede > > --- > > Changes in v2: > > - Make the commit message more clearer. > > This patch does not apply to the latest kernel tree at all :( > > Please fix up and resend. > > thanks, > > greg k-h Hi Greg! Sorry, I do not know what branch are expected to be used. I used the drm-misc-next to create this patch and I could not find any merge problem. Could you please tell me the details? Thanks, Shayenne ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] Re: [PATCH v2] staging: vboxvideo: Remove unnecessary parentheses
On Tue, Oct 30, 2018 at 08:17:57PM -0300, Shayenne Moura wrote: On 10/30, Greg Kroah-Hartman wrote: On Tue, Oct 23, 2018 at 02:43:04PM -0300, Shayenne da Luz Moura wrote: > Remove unneeded parentheses around the arguments of ||. This reduces > clutter and code behave in the same way. > Change suggested by checkpatch.pl. > > vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].x2 < > crtc->x' > > Signed-off-by: Shayenne da Luz Moura > Reviewed-by: Hans de Goede > --- > Changes in v2: > - Make the commit message more clearer. This patch does not apply to the latest kernel tree at all :( Please fix up and resend. thanks, greg k-h Hi Greg! Sorry, I do not know what branch are expected to be used. I used the drm-misc-next to create this patch and I could not find any merge problem. Could you please tell me the details? Hi Shayenne, There's an easy trick for this: the kernel tree has a script that can help you find both the relevant maintainers, and their development tree for each file in the kernel. Simply run: ./scripts/get_maintainer.pl --scm -f [filename] For example, in this case, you'd run: $ ./scripts/get_maintainer.pl --scm -f drivers/staging/vboxvideo/vbox_main.c Greg Kroah-Hartman (supporter:STAGING SUBSYSTEM,commit_signer:10/10=100%) Hans de Goede (commit_signer:7/10=70%,authored:6/10=60%,added_lines:31/37=84%,removed_lines:153/159=96%) Fabio Rafael da Rosa (commit_signer:1/10=10%,authored:1/10=10%) Alexander Kapshuk (commit_signer:1/10=10%,authored:1/10=10%) Nicholas Mc Guire (commit_signer:1/10=10%) Thomas Zimmermann (authored:1/10=10%,added_lines:2/37=5%) Daniel Junho (authored:1/10=10%,added_lines:2/37=5%) de...@driverdev.osuosl.org (open list:STAGING SUBSYSTEM) linux-ker...@vger.kernel.org (open list) git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git The list of people and mailing lists are the ones that should be the recipients of your patch, and the last line is the git tree on which the patch should be based on. -- Thanks, Sasha ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [Outreachy kernel] Re: [PATCH v2] staging: vboxvideo: Remove unnecessary parentheses
On Tue, 30 Oct 2018, Shayenne Moura wrote: > On 10/30, Greg Kroah-Hartman wrote: > > On Tue, Oct 23, 2018 at 02:43:04PM -0300, Shayenne da Luz Moura wrote: > > > Remove unneeded parentheses around the arguments of ||. This reduces > > > clutter and code behave in the same way. > > > Change suggested by checkpatch.pl. > > > > > > vbox_main.c:119: CHECK: Unnecessary parentheses around 'rects[i].x2 < > > > crtc->x' > > > > > > Signed-off-by: Shayenne da Luz Moura > > > Reviewed-by: Hans de Goede > > > --- > > > Changes in v2: > > > - Make the commit message more clearer. > > > > This patch does not apply to the latest kernel tree at all :( > > > > Please fix up and resend. > > > > thanks, > > > > greg k-h > > Hi Greg! > > Sorry, I do not know what branch are expected to be used. > I used the drm-misc-next to create this patch and I could not > find any merge problem. > > Could you please tell me the details? For staging drivers you should use the staging tree as indicated in the outreachy tutorial. julia > > Thanks, > > Shayenne > > -- > You received this message because you are subscribed to the Google Groups > "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to outreachy-kernel+unsubscr...@googlegroups.com. > To post to this group, send email to outreachy-ker...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/20181030231757.24ooubt25oykmgkt%40smtp.gmail.com. > For more options, visit https://groups.google.com/d/optout. > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
