re: staging: drm/imx: add i.MX IPUv3 base driver
Hello Sascha Hauer,
The patch aecfbdb1803b: "staging: drm/imx: add i.MX IPUv3 base
driver" from Sep 21, 2012, leads to the following
static checker warning:
"drivers/staging/imx-drm/ipu-v3/ipu-common.c:110
ipu_ch_param_write_field()
warn: buffer overflow 'base->word[word]->data' 5 <= 5"
drivers/staging/imx-drm/ipu-v3/ipu-common.c
92 void ipu_ch_param_write_field(struct ipu_ch_param __iomem *base, u32
wbs, u32 v)
93 {
94 u32 bit = (wbs >> 8) % 160;
95 u32 size = wbs & 0xff;
96 u32 word = (wbs >> 8) / 160;
97 u32 i = bit / 32;
^
i is between 0 and 4.
98 u32 ofs = bit % 32;
99 u32 mask = (1 << size) - 1;
100 u32 val;
101
102 pr_debug("%s %d %d %d\n", __func__, word, bit , size);
103
104 val = readl(&base->word[word].data[i]);
105 val &= ~(mask << ofs);
106 val |= v << ofs;
107 writel(val, &base->word[word].data[i]);
108
109 if ((bit + size - 1) / 32 > i) {
110 val = readl(&base->word[word].data[i + 1]);
^
The "+ 1" could put use one step beyond the end of the array.
111 val &= ~(mask >> (ofs ? (32 - ofs) : 0));
112 val |= v >> (ofs ? (32 - ofs) : 0);
113 writel(val, &base->word[word].data[i + 1]);
114 }
115 }
See also:
drivers/staging/imx-drm/ipu-v3/ipu-common.c:134
ipu_ch_param_read_field()
warn: buffer overflow 'base->word[word]->data' 5 <= 5
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
re: staging: drm/imx: add i.MX IPUv3 base driver
Hello Sascha Hauer,
This is a semi-automatic email about new static checker warnings.
The patch aecfbdb1803b: "staging: drm/imx: add i.MX IPUv3 base
driver" from Sep 21, 2012, leads to the following Smatch complaint:
drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c:164 ipu_dmfc_setup_channel()
warn: variable dereferenced before check 'dmfc' (see line 157)
drivers/staging/imx-drm/ipu-v3/ipu-dmfc.c
156 {
157 struct ipu_dmfc_priv *priv = dmfc->priv;
^^
Dereference.
158 u32 val, field;
159
160 dev_dbg(priv->dev,
161 "dmfc: using %d slots starting from segment %d
for IPU channel %d\n",
162 slots, segment, dmfc->data->ipu_channel);
163
164 if (!dmfc)
^
Check.
165 return -EINVAL;
166
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] mfd: rtsx: add card reader rtl8402
On Fri, 01 Nov 2013, micky_ch...@realsil.com.cn wrote:
> From: Micky Ching
>
> Add card reader rtl8042, rtl8402 is much like rtl8411, so just add it to
> rtl8411.c
>
> Signed-off-by: Micky Ching
> ---
> drivers/mfd/rtl8411.c | 62
>
> drivers/mfd/rtsx_pcr.c |5
> drivers/mfd/rtsx_pcr.h |1 +
> 3 files changed, 68 insertions(+)
Sorry Micky, but I'm not accepting this.
This patch adds 62 lines of duplicated code, with only a few
variations. Please find a way to generify the driver. Also I think we
should consider doing the voltage stuff using the Regulator Framework.
> diff --git a/drivers/mfd/rtl8411.c b/drivers/mfd/rtl8411.c
> index 5280135..9d37dd6 100644
> --- a/drivers/mfd/rtl8411.c
> +++ b/drivers/mfd/rtl8411.c
> @@ -498,3 +498,65 @@ void rtl8411b_init_params(struct rtsx_pcr *pcr)
> rtl8411b_qfn64_ms_pull_ctl_disable_tbl;
> }
> }
> +
> +static int rtl8402_switch_output_voltage(struct rtsx_pcr *pcr, u8 voltage)
> +{
> + u8 mask, val;
> + int err;
> +
> + mask = (BPP_REG_TUNED18 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_MASK;
> + if (voltage == OUTPUT_3V3) {
> + err = rtsx_pci_write_register(pcr,
> + SD30_DRIVE_SEL, 0x07, pcr->sd30_drive_sel_3v3);
> + if (err < 0)
> + return err;
> + val = (BPP_ASIC_3V3 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_3V3;
> + } else if (voltage == OUTPUT_1V8) {
> + err = rtsx_pci_write_register(pcr,
> + SD30_DRIVE_SEL, 0x07, pcr->sd30_drive_sel_1v8);
> + if (err < 0)
> + return err;
> + val = (BPP_ASIC_2V0 << BPP_TUNED18_SHIFT_8402) | BPP_PAD_1V8;
> + } else {
> + return -EINVAL;
> + }
> +
> + return rtsx_pci_write_register(pcr, LDO_CTL, mask, val);
> +}
> +
> +static const struct pcr_ops rtl8402_pcr_ops = {
> + .fetch_vendor_settings = rtl8411_fetch_vendor_settings,
> + .extra_init_hw = rtl8411_extra_init_hw,
> + .optimize_phy = NULL,
> + .turn_on_led = rtl8411_turn_on_led,
> + .turn_off_led = rtl8411_turn_off_led,
> + .enable_auto_blink = rtl8411_enable_auto_blink,
> + .disable_auto_blink = rtl8411_disable_auto_blink,
> + .card_power_on = rtl8411_card_power_on,
> + .card_power_off = rtl8411_card_power_off,
> + .switch_output_voltage = rtl8402_switch_output_voltage,
> + .cd_deglitch = rtl8411_cd_deglitch,
> + .conv_clk_and_div_n = rtl8411_conv_clk_and_div_n,
> + .force_power_down = rtl8411_force_power_down,
> +};
> +
> +void rtl8402_init_params(struct rtsx_pcr *pcr)
> +{
> + pcr->extra_caps = EXTRA_CAPS_SD_SDR50 | EXTRA_CAPS_SD_SDR104;
> + pcr->num_slots = 2;
> + pcr->ops = &rtl8402_pcr_ops;
> +
> + pcr->flags = 0;
> + pcr->card_drive_sel = RTL8411_CARD_DRIVE_DEFAULT;
> + pcr->sd30_drive_sel_1v8 = DRIVER_TYPE_B;
> + pcr->sd30_drive_sel_3v3 = DRIVER_TYPE_D;
> + pcr->aspm_en = ASPM_L1_EN;
> + pcr->tx_initial_phase = SET_CLOCK_PHASE(23, 7, 14);
> + pcr->rx_initial_phase = SET_CLOCK_PHASE(4, 3, 10);
> +
> + pcr->ic_version = rtl8411_get_ic_version(pcr);
> + pcr->sd_pull_ctl_enable_tbl = rtl8411_sd_pull_ctl_enable_tbl;
> + pcr->sd_pull_ctl_disable_tbl = rtl8411_sd_pull_ctl_disable_tbl;
> + pcr->ms_pull_ctl_enable_tbl = rtl8411_ms_pull_ctl_enable_tbl;
> + pcr->ms_pull_ctl_disable_tbl = rtl8411_ms_pull_ctl_disable_tbl;
> +}
> diff --git a/drivers/mfd/rtsx_pcr.c b/drivers/mfd/rtsx_pcr.c
> index 11e20af..93fabc7 100644
> --- a/drivers/mfd/rtsx_pcr.c
> +++ b/drivers/mfd/rtsx_pcr.c
> @@ -56,6 +56,7 @@ static DEFINE_PCI_DEVICE_TABLE(rtsx_pci_ids) = {
> { PCI_DEVICE(0x10EC, 0x5289), PCI_CLASS_OTHERS << 16, 0xFF },
> { PCI_DEVICE(0x10EC, 0x5227), PCI_CLASS_OTHERS << 16, 0xFF },
> { PCI_DEVICE(0x10EC, 0x5249), PCI_CLASS_OTHERS << 16, 0xFF },
> + { PCI_DEVICE(0x10EC, 0x5286), PCI_CLASS_OTHERS << 16, 0xFF },
> { PCI_DEVICE(0x10EC, 0x5287), PCI_CLASS_OTHERS << 16, 0xFF },
> { 0, }
> };
> @@ -1046,6 +1047,10 @@ static int rtsx_pci_init_chip(struct rtsx_pcr *pcr)
> rts5229_init_params(pcr);
> break;
>
> + case 0x5286:
> + rtl8402_init_params(pcr);
> + break;
> +
> case 0x5289:
> rtl8411_init_params(pcr);
> break;
> diff --git a/drivers/mfd/rtsx_pcr.h b/drivers/mfd/rtsx_pcr.h
> index 947e79b..8cac8db 100644
> --- a/drivers/mfd/rtsx_pcr.h
> +++ b/drivers/mfd/rtsx_pcr.h
> @@ -29,6 +29,7 @@
>
> void rts5209_init_params(struct rtsx_pcr *pcr);
> void rts5229_init_params(struct rtsx_pcr *pcr);
> +void rtl8402_init_params(struct rtsx_pcr *pcr);
> void rtl8411_init_params(struct rtsx_pcr *pcr);
> void rts5227_init_params(struct rtsx_pcr *pcr);
> void rts5249_init_params(struct rtsx_pcr *pcr);
--
Lee Jones
Linaro STMicroelectronics Landing Te
[PATCH 2/2] VME: Rename vme_slot_get to avoid confusion with reference counting
Traditionally the "get" functions increment the reference count of the
object that is returned, which does not happen with vme_slot_get. The
function vme_slot_get returns the physical VME slot associated with a
particular struct vme_dev. Rename vme_slot_num to avoid any confusion.
Signed-off-by: Martyn Welch
---
Documentation/vme_api.txt |2 +-
drivers/staging/vme/devices/vme_user.c |2 +-
drivers/vme/vme.c |6 +++---
include/linux/vme.h|2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/vme_api.txt b/Documentation/vme_api.txt
index 126a1f1..ffe6e22 100644
--- a/Documentation/vme_api.txt
+++ b/Documentation/vme_api.txt
@@ -393,7 +393,7 @@ Slot Detection
This function returns the slot ID of the provided bridge.
- int vme_slot_get(struct vme_dev *dev);
+ int vme_slot_num(struct vme_dev *dev);
Bus Detection
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index caee906..bc4f862 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -666,7 +666,7 @@ static int vme_user_match(struct vme_dev *vdev)
int i;
int cur_bus = vme_bus_num(vdev);
- int cur_slot = vme_slot_get(vdev);
+ int cur_slot = vme_slot_num(vdev);
for (i = 0; i < bus_num; i++)
if ((cur_bus == bus[i]) && (cur_slot == vdev->num))
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 8df5e4e..96562c9 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -1274,7 +1274,7 @@ void vme_lm_free(struct vme_resource *resource)
}
EXPORT_SYMBOL(vme_lm_free);
-int vme_slot_get(struct vme_dev *vdev)
+int vme_slot_num(struct vme_dev *vdev)
{
struct vme_bridge *bridge;
@@ -1285,13 +1285,13 @@ int vme_slot_get(struct vme_dev *vdev)
}
if (bridge->slot_get == NULL) {
- printk(KERN_WARNING "vme_slot_get not supported\n");
+ printk(KERN_WARNING "vme_slot_num not supported\n");
return -EINVAL;
}
return bridge->slot_get(bridge);
}
-EXPORT_SYMBOL(vme_slot_get);
+EXPORT_SYMBOL(vme_slot_num);
int vme_bus_num(struct vme_dev *vdev)
{
diff --git a/include/linux/vme.h b/include/linux/vme.h
index 4eb42c8..8cd6f19 100644
--- a/include/linux/vme.h
+++ b/include/linux/vme.h
@@ -164,7 +164,7 @@ int vme_lm_attach(struct vme_resource *, int, void
(*callback)(int));
int vme_lm_detach(struct vme_resource *, int);
void vme_lm_free(struct vme_resource *);
-int vme_slot_get(struct vme_dev *);
+int vme_slot_num(struct vme_dev *);
int vme_bus_num(struct vme_dev *);
int vme_register_driver(struct vme_driver *, unsigned int);
--
1.7.0.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] VME: Provide access to VME bus enumeration and fix vme_user match function
The match function for vme_user is completely wrong. It will blindly bind
against the first VME slot on each bus (at this point that would be just the
first bus as the driver can only handle one bus).
The original intention (before some major subsystem changes) was that the
driver bind against the slot to which the bridge was attached in the VME
system and to the bus(es) provided via the "bus" module parameter.
To do this cleanly (i.e. without poking arround in the subsystems internal
stuctures) a functionality has been added to provide access to the bus
enumeration.
Signed-off-by: Martyn Welch
---
Documentation/vme_api.txt | 10 ++
drivers/staging/vme/devices/vme_user.c | 13 ++---
drivers/vme/vme.c | 13 +
include/linux/vme.h|1 +
4 files changed, 34 insertions(+), 3 deletions(-)
diff --git a/Documentation/vme_api.txt b/Documentation/vme_api.txt
index 856efa3..126a1f1 100644
--- a/Documentation/vme_api.txt
+++ b/Documentation/vme_api.txt
@@ -394,3 +394,13 @@ Slot Detection
This function returns the slot ID of the provided bridge.
int vme_slot_get(struct vme_dev *dev);
+
+
+Bus Detection
+=
+
+This function returns the bus ID of the provided bridge.
+
+ int vme_bus_num(struct vme_dev *dev);
+
+
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index daec155..caee906 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -663,9 +663,16 @@ err_nocard:
static int vme_user_match(struct vme_dev *vdev)
{
- if (vdev->num >= VME_USER_BUS_MAX)
- return 0;
- return 1;
+ int i;
+
+ int cur_bus = vme_bus_num(vdev);
+ int cur_slot = vme_slot_get(vdev);
+
+ for (i = 0; i < bus_num; i++)
+ if ((cur_bus == bus[i]) && (cur_slot == vdev->num))
+ return 1;
+
+ return 0;
}
/*
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index f6856b4..8df5e4e 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -1293,6 +1293,19 @@ int vme_slot_get(struct vme_dev *vdev)
}
EXPORT_SYMBOL(vme_slot_get);
+int vme_bus_num(struct vme_dev *vdev)
+{
+ struct vme_bridge *bridge;
+
+ bridge = vdev->bridge;
+ if (bridge == NULL) {
+ pr_err("Can't find VME bus\n");
+ return -EINVAL;
+ }
+
+ return bridge->num;
+}
+EXPORT_SYMBOL(vme_bus_num);
/* - Bridge Registration --- */
diff --git a/include/linux/vme.h b/include/linux/vme.h
index c9d65bf..4eb42c8 100644
--- a/include/linux/vme.h
+++ b/include/linux/vme.h
@@ -165,6 +165,7 @@ int vme_lm_detach(struct vme_resource *, int);
void vme_lm_free(struct vme_resource *);
int vme_slot_get(struct vme_dev *);
+int vme_bus_num(struct vme_dev *);
int vme_register_driver(struct vme_driver *, unsigned int);
void vme_unregister_driver(struct vme_driver *);
--
1.7.0.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] VME: Provide access to VME bus enumeration and fix vme_user match function
My appologies - the script I had to send patches had got a bit broken and I'd
completely forgotten...
Martyn
On 08/11/13 11:55, y wrote:
> From: Martyn Welch
>
> The match function for vme_user is completely wrong. It will blindly bind
> against the first VME slot on each bus (at this point that would be just the
> first bus as the driver can only handle one bus).
>
> The original intention (before some major subsystem changes) was that the
> driver bind against the slot to which the bridge was attached in the VME
> system and to the bus(es) provided via the "bus" module parameter.
>
> To do this cleanly (i.e. without poking arround in the subsystems internal
> stuctures) a functionality has been added to provide access to the bus
> enumeration.
>
> Signed-off-by: Martyn Welch
> ---
> Documentation/vme_api.txt | 10 ++
> drivers/staging/vme/devices/vme_user.c | 13 ++---
> drivers/vme/vme.c | 13 +
> include/linux/vme.h|1 +
> 4 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/vme_api.txt b/Documentation/vme_api.txt
> index 856efa3..126a1f1 100644
> --- a/Documentation/vme_api.txt
> +++ b/Documentation/vme_api.txt
> @@ -394,3 +394,13 @@ Slot Detection
> This function returns the slot ID of the provided bridge.
>
> int vme_slot_get(struct vme_dev *dev);
> +
> +
> +Bus Detection
> +=
> +
> +This function returns the bus ID of the provided bridge.
> +
> + int vme_bus_num(struct vme_dev *dev);
> +
> +
> diff --git a/drivers/staging/vme/devices/vme_user.c
> b/drivers/staging/vme/devices/vme_user.c
> index daec155..caee906 100644
> --- a/drivers/staging/vme/devices/vme_user.c
> +++ b/drivers/staging/vme/devices/vme_user.c
> @@ -663,9 +663,16 @@ err_nocard:
>
> static int vme_user_match(struct vme_dev *vdev)
> {
> - if (vdev->num >= VME_USER_BUS_MAX)
> - return 0;
> - return 1;
> + int i;
> +
> + int cur_bus = vme_bus_num(vdev);
> + int cur_slot = vme_slot_get(vdev);
> +
> + for (i = 0; i < bus_num; i++)
> + if ((cur_bus == bus[i]) && (cur_slot == vdev->num))
> + return 1;
> +
> + return 0;
> }
>
> /*
> diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
> index f6856b4..8df5e4e 100644
> --- a/drivers/vme/vme.c
> +++ b/drivers/vme/vme.c
> @@ -1293,6 +1293,19 @@ int vme_slot_get(struct vme_dev *vdev)
> }
> EXPORT_SYMBOL(vme_slot_get);
>
> +int vme_bus_num(struct vme_dev *vdev)
> +{
> + struct vme_bridge *bridge;
> +
> + bridge = vdev->bridge;
> + if (bridge == NULL) {
> + pr_err("Can't find VME bus\n");
> + return -EINVAL;
> + }
> +
> + return bridge->num;
> +}
> +EXPORT_SYMBOL(vme_bus_num);
>
> /* - Bridge Registration ---
> */
>
> diff --git a/include/linux/vme.h b/include/linux/vme.h
> index c9d65bf..4eb42c8 100644
> --- a/include/linux/vme.h
> +++ b/include/linux/vme.h
> @@ -165,6 +165,7 @@ int vme_lm_detach(struct vme_resource *, int);
> void vme_lm_free(struct vme_resource *);
>
> int vme_slot_get(struct vme_dev *);
> +int vme_bus_num(struct vme_dev *);
>
> int vme_register_driver(struct vme_driver *, unsigned int);
> void vme_unregister_driver(struct vme_driver *);
>
--
Martyn Welch (Lead Software Engineer) | Registered in England and Wales
GE Intelligent Platforms | (3828642) at 100 Barbirolli Square
T +44(0)1327322748 | Manchester, M2 3AB
E martyn.we...@ge.com | VAT:GB 927559189
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] VME: Rename vme_slot_get to avoid confusion with reference counting
Traditionally the "get" functions increment the reference count of the
object that is returned, which does not happen with vme_slot_get. The
function vme_slot_get returns the physical VME slot associated with a
particular struct vme_dev. Rename vme_slot_num to avoid any confusion.
Signed-off-by: Martyn Welch
---
Documentation/vme_api.txt |2 +-
drivers/staging/vme/devices/vme_user.c |2 +-
drivers/vme/vme.c |6 +++---
include/linux/vme.h|2 +-
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/vme_api.txt b/Documentation/vme_api.txt
index 126a1f1..ffe6e22 100644
--- a/Documentation/vme_api.txt
+++ b/Documentation/vme_api.txt
@@ -393,7 +393,7 @@ Slot Detection
This function returns the slot ID of the provided bridge.
- int vme_slot_get(struct vme_dev *dev);
+ int vme_slot_num(struct vme_dev *dev);
Bus Detection
diff --git a/drivers/staging/vme/devices/vme_user.c
b/drivers/staging/vme/devices/vme_user.c
index caee906..bc4f862 100644
--- a/drivers/staging/vme/devices/vme_user.c
+++ b/drivers/staging/vme/devices/vme_user.c
@@ -666,7 +666,7 @@ static int vme_user_match(struct vme_dev *vdev)
int i;
int cur_bus = vme_bus_num(vdev);
- int cur_slot = vme_slot_get(vdev);
+ int cur_slot = vme_slot_num(vdev);
for (i = 0; i < bus_num; i++)
if ((cur_bus == bus[i]) && (cur_slot == vdev->num))
diff --git a/drivers/vme/vme.c b/drivers/vme/vme.c
index 8df5e4e..96562c9 100644
--- a/drivers/vme/vme.c
+++ b/drivers/vme/vme.c
@@ -1274,7 +1274,7 @@ void vme_lm_free(struct vme_resource *resource)
}
EXPORT_SYMBOL(vme_lm_free);
-int vme_slot_get(struct vme_dev *vdev)
+int vme_slot_num(struct vme_dev *vdev)
{
struct vme_bridge *bridge;
@@ -1285,13 +1285,13 @@ int vme_slot_get(struct vme_dev *vdev)
}
if (bridge->slot_get == NULL) {
- printk(KERN_WARNING "vme_slot_get not supported\n");
+ printk(KERN_WARNING "vme_slot_num not supported\n");
return -EINVAL;
}
return bridge->slot_get(bridge);
}
-EXPORT_SYMBOL(vme_slot_get);
+EXPORT_SYMBOL(vme_slot_num);
int vme_bus_num(struct vme_dev *vdev)
{
diff --git a/include/linux/vme.h b/include/linux/vme.h
index 4eb42c8..8cd6f19 100644
--- a/include/linux/vme.h
+++ b/include/linux/vme.h
@@ -164,7 +164,7 @@ int vme_lm_attach(struct vme_resource *, int, void
(*callback)(int));
int vme_lm_detach(struct vme_resource *, int);
void vme_lm_free(struct vme_resource *);
-int vme_slot_get(struct vme_dev *);
+int vme_slot_num(struct vme_dev *);
int vme_bus_num(struct vme_dev *);
int vme_register_driver(struct vme_driver *, unsigned int);
--
1.7.0.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Treat as Urgent and Read Carefully.
Good Day, My name is Mr. Luis Elmer, i am the credit officer in Banco Santander Plc, London, UK. I have a business proposal of $28,800,000, for you from my bank. There were no beneficiaries stated concerning these funds which means no one would ever come forward to claim it. I want us to work together so as to have the sum transferred out of my bank into your account. Please endeavor to observe utmost discretion in all matters concerning this issue. Once the funds have been transferred to your bank account, we shall then share in the ratio of 50% for me, 50% for you. Your own part is to find safe bank account where the 28million will be transferred safely. Should you be interested please treat this business with utmost and get back to me with the following: 1 Full names, 2 Private phone number 3 Current residential address Regards, Mr. Luis Elmer. Credit officer, Banco Santander Plc, London, UK ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Treat as Urgent and Read Carefully.
Good Day, My name is Mr. Luis Elmer, i am the credit officer in Banco Santander Plc, London, UK. I have a business proposal of $28,800,000, for you from my bank. There were no beneficiaries stated concerning these funds which means no one would ever come forward to claim it. I want us to work together so as to have the sum transferred out of my bank into your account. Please endeavor to observe utmost discretion in all matters concerning this issue. Once the funds have been transferred to your bank account, we shall then share in the ratio of 50% for me, 50% for you. Your own part is to find safe bank account where the 28million will be transferred safely. Should you be interested please treat this business with utmost and get back to me with the following: 1 Full names, 2 Private phone number 3 Current residential address Regards, Mr. Luis Elmer. Credit officer, Banco Santander Plc, London, UK ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 04/22] staging: comedi: use attach_lock semaphore during attach and detach
Acquire the `attach_lock` semaphore in the `struct comedi_device` while
modifying the `attached` flag. This is a "write" acquire. Note that
the main mutex in the `struct comedi_device` is also held at this time.
Tasks wishing to check the device is attached will need to either
acquire the main mutex, or "read" acquire the `attach_lock` semaphore,
or both in that order.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 8f02bf6..c398422 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -133,10 +133,12 @@ static void cleanup_device(struct comedi_device *dev)
void comedi_device_detach(struct comedi_device *dev)
{
+ down_write(&dev->attach_lock);
dev->attached = false;
if (dev->driver)
dev->driver->detach(dev);
cleanup_device(dev);
+ up_write(&dev->attach_lock);
}
static int poll_invalid(struct comedi_device *dev, struct comedi_subdevice *s)
@@ -355,8 +357,9 @@ static int comedi_device_postconfig(struct comedi_device
*dev)
ret = __comedi_device_postconfig(dev);
if (ret < 0)
return ret;
- smp_wmb();
+ down_write(&dev->attach_lock);
dev->attached = true;
+ up_write(&dev->attach_lock);
return 0;
}
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 00/22] staging: comedi: protect operations from device removal
Calls to `comedi_auto_unconfig()` from low-level comedi drivers result in the `struct comedi_device` being freed along with any comedi data buffers. That's bad news for the 'read' and 'write' file operations that currently have no protection from this (they don't use the main mutex in the `struct comedi_device` for performance reasons). It's also bad news if the comedi data buffers have been mmapped. The functions in the "kcomedilib" module also need some protection. 01) staging: comedi: remove unused command callback support 02) staging: comedi: add a couple of #includes to comedidev.h 03) staging: comedi: add rw_semaphore to protect against device detachment 04) staging: comedi: use attach_lock semaphore during attach and detach 05) staging: comedi: cleanup_device() -> comedi_device_detach_cleanup() 06) staging: comedi: wake up async waiters when become non-busy 07) staging: comedi: cancel commands before detaching device 08) staging: comedi: add detachment counter for validity checks 09) staging: comedi: protect against detach during write operation 10) staging: comedi: protect against detach during read operation 11) staging: comedi: add a kref to comedi device 12) staging: comedi: add comedi_dev_get_from_minor() 13) staging: comedi: increment reference while file open 14) staging: comedi: use refcount in comedi_driver_unregister() 15) staging: comedi: use refcount while reading /proc/comedi 16) staging: comedi: use refcount in sysfs attribute handlers 17) staging: comedi: kcomedilib: increment reference while device in use 18) staging: comedi: kcomedilib: protect against device detachment 19) staging: comedi: use file->private_data in file operations 20) staging: comedi: remove comedi_dev_from_minor() 21) staging: comedi: make determination of read or write subdevice safer 22) staging: comedi: protect buffer from being freed while mmapped drivers/staging/comedi/comedi_buf.c| 99 -- drivers/staging/comedi/comedi_fops.c | 358 + drivers/staging/comedi/comedi_internal.h | 4 + drivers/staging/comedi/comedidev.h | 28 +- drivers/staging/comedi/drivers.c | 14 +- drivers/staging/comedi/drivers/mite.c | 2 +- .../staging/comedi/kcomedilib/kcomedilib_main.c| 74 +++-- drivers/staging/comedi/proc.c | 6 +- 8 files changed, 382 insertions(+), 203 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 11/22] staging: comedi: add a kref to comedi device
Add a `struct kref refcount` member to `struct comedi_device` to allow
safe destruction of the comedi device. Only free the comedi device via
the 'release' callback `kref_put()`. Currently, nothing calls
`kref_put()`, so the safe destruction is ineffective, but this will be
addressed by later patches.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 22 +++---
drivers/staging/comedi/comedidev.h | 2 ++
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index fa8da20..403324c 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -89,12 +89,29 @@ static struct cdev comedi_cdev;
static void comedi_device_init(struct comedi_device *dev)
{
+ kref_init(&dev->refcount);
spin_lock_init(&dev->spinlock);
mutex_init(&dev->mutex);
init_rwsem(&dev->attach_lock);
dev->minor = -1;
}
+static void comedi_dev_kref_release(struct kref *kref)
+{
+ struct comedi_device *dev =
+ container_of(kref, struct comedi_device, refcount);
+
+ mutex_destroy(&dev->mutex);
+ kfree(dev);
+}
+
+int comedi_dev_put(struct comedi_device *dev)
+{
+ if (dev)
+ return kref_put(&dev->refcount, comedi_dev_kref_release);
+ return 1;
+}
+
static void comedi_device_cleanup(struct comedi_device *dev)
{
struct module *driver_module = NULL;
@@ -112,7 +129,6 @@ static void comedi_device_cleanup(struct comedi_device *dev)
dev->use_count--;
}
mutex_unlock(&dev->mutex);
- mutex_destroy(&dev->mutex);
}
static bool comedi_clear_board_dev(struct comedi_device *dev)
@@ -148,7 +164,7 @@ static void comedi_free_board_dev(struct comedi_device *dev)
MKDEV(COMEDI_MAJOR, dev->minor));
}
comedi_device_cleanup(dev);
- kfree(dev);
+ comedi_dev_put(dev);
}
}
@@ -2494,7 +2510,7 @@ struct comedi_device *comedi_alloc_board_minor(struct
device *hardware_device)
if (i == COMEDI_NUM_BOARD_MINORS) {
mutex_unlock(&dev->mutex);
comedi_device_cleanup(dev);
- kfree(dev);
+ comedi_dev_put(dev);
pr_err("comedi: error: ran out of minor numbers for board
device files.\n");
return ERR_PTR(-EBUSY);
}
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index 05cc8db..08652df 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -23,6 +23,7 @@
#include
#include
#include
+#include
#include "comedi.h"
@@ -187,6 +188,7 @@ struct comedi_device {
spinlock_t spinlock;
struct mutex mutex;
struct rw_semaphore attach_lock;
+ struct kref refcount;
int n_subdevices;
struct comedi_subdevice *subdevices;
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 07/22] staging: comedi: cancel commands before detaching device
The comedi core module's handling of the `COMEDI_DEVCONFIG` ioctl will
not allow a device to be detached if it is busy. However, comedi
devices can also be auto-detached due to a removal of a hardware device.
One of the things we should do in that case is cancel any asynchronous
commands that are running. Add a new function
`comedi_device_cancel_all()` to do that and call it from
`comedi_device_detach()`.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 15 +++
drivers/staging/comedi/comedi_internal.h | 1 +
drivers/staging/comedi/drivers.c | 1 +
3 files changed, 17 insertions(+)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 9c85f01..eafa18e 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -584,6 +584,21 @@ static int do_cancel(struct comedi_device *dev, struct
comedi_subdevice *s)
return ret;
}
+void comedi_device_cancel_all(struct comedi_device *dev)
+{
+ struct comedi_subdevice *s;
+ int i;
+
+ if (!dev->attached)
+ return;
+
+ for (i = 0; i < dev->n_subdevices; i++) {
+ s = &dev->subdevices[i];
+ if (s->async)
+ do_cancel(dev, s);
+ }
+}
+
static int is_device_busy(struct comedi_device *dev)
{
struct comedi_subdevice *s;
diff --git a/drivers/staging/comedi/comedi_internal.h
b/drivers/staging/comedi/comedi_internal.h
index fda1a7b..151693b 100644
--- a/drivers/staging/comedi/comedi_internal.h
+++ b/drivers/staging/comedi/comedi_internal.h
@@ -17,6 +17,7 @@ int comedi_buf_alloc(struct comedi_device *dev, struct
comedi_subdevice *s,
unsigned long new_size);
void comedi_buf_reset(struct comedi_async *async);
unsigned int comedi_buf_write_n_allocated(struct comedi_async *async);
+void comedi_device_cancel_all(struct comedi_device *dev);
extern unsigned int comedi_default_buf_size_kb;
extern unsigned int comedi_default_buf_maxsize_kb;
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 1f44f29..4f72773 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -133,6 +133,7 @@ static void comedi_device_detach_cleanup(struct
comedi_device *dev)
void comedi_device_detach(struct comedi_device *dev)
{
+ comedi_device_cancel_all(dev);
down_write(&dev->attach_lock);
dev->attached = false;
if (dev->driver)
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 08/22] staging: comedi: add detachment counter for validity checks
Add a member `detach_count` to `struct comedi_device` that is
incremented every time the device gets detached. This will be used in
some validity checks in the 'read' and 'write' file operations to make
sure the attachment remains valid.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedidev.h | 1 +
drivers/staging/comedi/drivers.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index 70de355..05cc8db 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -173,6 +173,7 @@ struct comedi_device {
struct device *class_dev;
int minor;
+ unsigned int detach_count;
/* hw_dev is passed to dma_alloc_coherent when allocating async buffers
* for subdevices that have async_dma_dir set to something other than
* DMA_NONE */
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 4f72773..15b1ab8 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -136,6 +136,7 @@ void comedi_device_detach(struct comedi_device *dev)
comedi_device_cancel_all(dev);
down_write(&dev->attach_lock);
dev->attached = false;
+ dev->detach_count++;
if (dev->driver)
dev->driver->detach(dev);
comedi_device_detach_cleanup(dev);
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 09/22] staging: comedi: protect against detach during write operation
The 'write' file operation for comedi devices does not use the main
mutex in the `struct comedi_device` to avoid contention with some ioctls
that may take a while to complete. Use the `attach_lock` semaphore to
protect against detachment while the 'write' operation is in progress.
This is a `struct rw_semaphore` and we read-lock it to protect against
device detachment.
Note that `comedi_device_cancel_all()` is called during device
detachment, which cancels any ongoing asynchronous commands. This will
wake up any blocked writers which will then release the `attach_lock`
semaphore and complete the 'write' operation early.
The only time the 'write' file operation does use the main mutex is at
the end of the command when it has to call `do_become_nonbusy()` to mark
the subdevice as no longer busy handling an asynchronous command. To
avoid deadlock, it has to remove the task from the wait queue and
release the `attach_lock` semaphore before acquiring the main mutex. It
then needs to confirm that the device is still attached. Unfortunately,
we do not yet protect against a dynamically allocated `struct
comedi_device` being deleted during the operation. This will be
addressed by a later patch.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 61 ++--
1 file changed, 52 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index eafa18e..1ae2bdf 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2023,38 +2023,77 @@ static ssize_t comedi_write(struct file *file, const
char __user *buf,
DECLARE_WAITQUEUE(wait, current);
const unsigned minor = iminor(file_inode(file));
struct comedi_device *dev = comedi_dev_from_minor(minor);
+ bool on_wait_queue = false;
+ bool attach_locked;
+ unsigned int old_detach_count;
if (!dev)
return -ENODEV;
+ /* Protect against device detachment during operation. */
+ down_read(&dev->attach_lock);
+ attach_locked = true;
+ old_detach_count = dev->detach_count;
+
if (!dev->attached) {
DPRINTK("no driver configured on comedi%i\n", dev->minor);
- return -ENODEV;
+ retval = -ENODEV;
+ goto out;
}
s = comedi_write_subdevice(dev, minor);
- if (!s || !s->async)
- return -EIO;
+ if (!s || !s->async) {
+ retval = -EIO;
+ goto out;
+ }
async = s->async;
if (!s->busy || !nbytes)
- return 0;
- if (s->busy != file)
- return -EACCES;
+ goto out;
+ if (s->busy != file) {
+ retval = -EACCES;
+ goto out;
+ }
add_wait_queue(&async->wait_head, &wait);
+ on_wait_queue = true;
while (nbytes > 0 && !retval) {
set_current_state(TASK_INTERRUPTIBLE);
if (!comedi_is_subdevice_running(s)) {
if (count == 0) {
- mutex_lock(&dev->mutex);
+ struct comedi_subdevice *new_s;
+
if (comedi_is_subdevice_in_error(s))
retval = -EPIPE;
else
retval = 0;
- do_become_nonbusy(dev, s);
+ /*
+* To avoid deadlock, cannot acquire dev->mutex
+* while dev->attach_lock is held. Need to
+* remove task from the async wait queue before
+* releasing dev->attach_lock, as it might not
+* be valid afterwards.
+*/
+ remove_wait_queue(&async->wait_head, &wait);
+ on_wait_queue = false;
+ up_read(&dev->attach_lock);
+ attach_locked = false;
+ mutex_lock(&dev->mutex);
+ /*
+* Become non-busy unless things have changed
+* behind our back. Checking dev->detach_count
+* is unchanged ought to be sufficient (unless
+* there have been 2**32 detaches in the
+* meantime!), but check the subdevice pointer
+* as well just in case.
+*/
+ new_s = comedi_write_subdevice(dev, minor);
+ if (dev->attached &&
+ old_detach_count == dev->d
[PATCH 13/22] staging: comedi: increment reference while file open
In the 'open' file operation handler `comedi_open()` in "comedi_fops.c",
call `comedi_dev_get_from_minor()` instead of `comedi_dev_from_minor()`
to get the pointer to the `struct comedi_device`. This increments the
reference to prevent it being freed. Call `comedi_dev_put()` to
decrement the reference on failure, and also call it from the 'release'
file operation handler `comedi_close()`.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 28
1 file changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 5e5ddbd..0ff6913 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2348,7 +2348,8 @@ out:
static int comedi_open(struct inode *inode, struct file *file)
{
const unsigned minor = iminor(inode);
- struct comedi_device *dev = comedi_dev_from_minor(minor);
+ struct comedi_device *dev = comedi_dev_get_from_minor(minor);
+ int rc;
if (!dev) {
DPRINTK("invalid minor number\n");
@@ -2373,8 +2374,8 @@ static int comedi_open(struct inode *inode, struct file
*file)
goto ok;
if (!capable(CAP_NET_ADMIN) && dev->in_request_module) {
DPRINTK("in request module\n");
- mutex_unlock(&dev->mutex);
- return -ENODEV;
+ rc = -ENODEV;
+ goto out;
}
if (capable(CAP_NET_ADMIN) && dev->in_request_module)
goto ok;
@@ -2391,8 +2392,8 @@ static int comedi_open(struct inode *inode, struct file
*file)
if (!dev->attached && !capable(CAP_NET_ADMIN)) {
DPRINTK("not attached and not CAP_NET_ADMIN\n");
- mutex_unlock(&dev->mutex);
- return -ENODEV;
+ rc = -ENODEV;
+ goto out;
}
ok:
__module_get(THIS_MODULE);
@@ -2400,26 +2401,28 @@ ok:
if (dev->attached) {
if (!try_module_get(dev->driver->module)) {
module_put(THIS_MODULE);
- mutex_unlock(&dev->mutex);
- return -ENOSYS;
+ rc = -ENOSYS;
+ goto out;
}
}
if (dev->attached && dev->use_count == 0 && dev->open) {
- int rc = dev->open(dev);
+ rc = dev->open(dev);
if (rc < 0) {
module_put(dev->driver->module);
module_put(THIS_MODULE);
- mutex_unlock(&dev->mutex);
- return rc;
+ goto out;
}
}
dev->use_count++;
+ rc = 0;
+out:
mutex_unlock(&dev->mutex);
-
- return 0;
+ if (rc)
+ comedi_dev_put(dev);
+ return rc;
}
static int comedi_fasync(int fd, struct file *file, int on)
@@ -2465,6 +2468,7 @@ static int comedi_close(struct inode *inode, struct file
*file)
dev->use_count--;
mutex_unlock(&dev->mutex);
+ comedi_dev_put(dev);
return 0;
}
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 16/22] staging: comedi: use refcount in sysfs attribute handlers
Call `comedi_dev_get_from_minor()` instead of `comedi_dev_from_minor()` in the sysfs attribute handler functions to increment the reference of the `struct comedi_device` during the operation. Call `comedi_dev_put()` to decrement the reference afterwards. Signed-off-by: Ian Abbott --- drivers/staging/comedi/comedi_fops.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index 0ff6913..37400e8 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -328,7 +328,7 @@ static ssize_t max_read_buffer_kb_show(struct device *csdev, struct comedi_subdevice *s; unsigned int size = 0; - dev = comedi_dev_from_minor(minor); + dev = comedi_dev_get_from_minor(minor); if (!dev) return -ENODEV; @@ -338,6 +338,7 @@ static ssize_t max_read_buffer_kb_show(struct device *csdev, size = s->async->max_bufsize / 1024; mutex_unlock(&dev->mutex); + comedi_dev_put(dev); return snprintf(buf, PAGE_SIZE, "%i\n", size); } @@ -358,7 +359,7 @@ static ssize_t max_read_buffer_kb_store(struct device *csdev, return -EINVAL; size *= 1024; - dev = comedi_dev_from_minor(minor); + dev = comedi_dev_get_from_minor(minor); if (!dev) return -ENODEV; @@ -370,6 +371,7 @@ static ssize_t max_read_buffer_kb_store(struct device *csdev, err = -EINVAL; mutex_unlock(&dev->mutex); + comedi_dev_put(dev); return err ? err : count; } static DEVICE_ATTR_RW(max_read_buffer_kb); @@ -382,7 +384,7 @@ static ssize_t read_buffer_kb_show(struct device *csdev, struct comedi_subdevice *s; unsigned int size = 0; - dev = comedi_dev_from_minor(minor); + dev = comedi_dev_get_from_minor(minor); if (!dev) return -ENODEV; @@ -392,6 +394,7 @@ static ssize_t read_buffer_kb_show(struct device *csdev, size = s->async->prealloc_bufsz / 1024; mutex_unlock(&dev->mutex); + comedi_dev_put(dev); return snprintf(buf, PAGE_SIZE, "%i\n", size); } @@ -412,7 +415,7 @@ static ssize_t read_buffer_kb_store(struct device *csdev, return -EINVAL; size *= 1024; - dev = comedi_dev_from_minor(minor); + dev = comedi_dev_get_from_minor(minor); if (!dev) return -ENODEV; @@ -424,6 +427,7 @@ static ssize_t read_buffer_kb_store(struct device *csdev, err = -EINVAL; mutex_unlock(&dev->mutex); + comedi_dev_put(dev); return err ? err : count; } static DEVICE_ATTR_RW(read_buffer_kb); @@ -437,7 +441,7 @@ static ssize_t max_write_buffer_kb_show(struct device *csdev, struct comedi_subdevice *s; unsigned int size = 0; - dev = comedi_dev_from_minor(minor); + dev = comedi_dev_get_from_minor(minor); if (!dev) return -ENODEV; @@ -447,6 +451,7 @@ static ssize_t max_write_buffer_kb_show(struct device *csdev, size = s->async->max_bufsize / 1024; mutex_unlock(&dev->mutex); + comedi_dev_put(dev); return snprintf(buf, PAGE_SIZE, "%i\n", size); } @@ -467,7 +472,7 @@ static ssize_t max_write_buffer_kb_store(struct device *csdev, return -EINVAL; size *= 1024; - dev = comedi_dev_from_minor(minor); + dev = comedi_dev_get_from_minor(minor); if (!dev) return -ENODEV; @@ -479,6 +484,7 @@ static ssize_t max_write_buffer_kb_store(struct device *csdev, err = -EINVAL; mutex_unlock(&dev->mutex); + comedi_dev_put(dev); return err ? err : count; } static DEVICE_ATTR_RW(max_write_buffer_kb); @@ -491,7 +497,7 @@ static ssize_t write_buffer_kb_show(struct device *csdev, struct comedi_subdevice *s; unsigned int size = 0; - dev = comedi_dev_from_minor(minor); + dev = comedi_dev_get_from_minor(minor); if (!dev) return -ENODEV; @@ -501,6 +507,7 @@ static ssize_t write_buffer_kb_show(struct device *csdev, size = s->async->prealloc_bufsz / 1024; mutex_unlock(&dev->mutex); + comedi_dev_put(dev); return snprintf(buf, PAGE_SIZE, "%i\n", size); } @@ -521,7 +528,7 @@ static ssize_t write_buffer_kb_store(struct device *csdev, return -EINVAL; size *= 1024; - dev = comedi_dev_from_minor(minor); + dev = comedi_dev_get_from_minor(minor); if (!dev) return -ENODEV; @@ -533,6 +540,7 @@ static ssize_t write_buffer_kb_store(struct device *csdev, err = -EINVAL; mutex_unlock(&dev->mutex); + comedi_dev_put(dev); return err ? err : count; } static DEVICE_ATTR_RW(write_buffer_kb); -- 1.8.4.2 ___
[PATCH 12/22] staging: comedi: add comedi_dev_get_from_minor()
Add function `struct comedi_device *comedi_dev_get_from_minor(unsigned
minor)`. This behaves like the existing `comedi_dev_from_minor()`
except that it also increments the `struct kref refcount` member (via
new helper function `comedi_dev_get()`) to prevent it being freed. If
it returns a valid pointer, the caller is responsible for calling
`comedi_dev_put()` to decrement the reference count.
Export `comedi_dev_get_from_minor()` and `comedi_dev_put()` as they will
be used by the "kcomedilib" module in addition to the "comedi" module
itself.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 42
drivers/staging/comedi/comedidev.h | 2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 403324c..5e5ddbd 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -111,6 +111,14 @@ int comedi_dev_put(struct comedi_device *dev)
return kref_put(&dev->refcount, comedi_dev_kref_release);
return 1;
}
+EXPORT_SYMBOL_GPL(comedi_dev_put);
+
+static struct comedi_device *comedi_dev_get(struct comedi_device *dev)
+{
+ if (dev)
+ kref_get(&dev->refcount);
+ return dev;
+}
static void comedi_device_cleanup(struct comedi_device *dev)
{
@@ -209,6 +217,40 @@ struct comedi_device *comedi_dev_from_minor(unsigned minor)
}
EXPORT_SYMBOL_GPL(comedi_dev_from_minor);
+static struct comedi_device *comedi_dev_get_from_board_minor(unsigned minor)
+{
+ struct comedi_device *dev;
+
+ BUG_ON(minor >= COMEDI_NUM_BOARD_MINORS);
+ mutex_lock(&comedi_board_minor_table_lock);
+ dev = comedi_dev_get(comedi_board_minor_table[minor]);
+ mutex_unlock(&comedi_board_minor_table_lock);
+ return dev;
+}
+
+static struct comedi_device *comedi_dev_get_from_subdevice_minor(unsigned
minor)
+{
+ struct comedi_device *dev;
+ struct comedi_subdevice *s;
+ unsigned int i = minor - COMEDI_NUM_BOARD_MINORS;
+
+ BUG_ON(i >= COMEDI_NUM_SUBDEVICE_MINORS);
+ mutex_lock(&comedi_subdevice_minor_table_lock);
+ s = comedi_subdevice_minor_table[i];
+ dev = comedi_dev_get(s ? s->device : NULL);
+ mutex_unlock(&comedi_subdevice_minor_table_lock);
+ return dev;
+}
+
+struct comedi_device *comedi_dev_get_from_minor(unsigned minor)
+{
+ if (minor < COMEDI_NUM_BOARD_MINORS)
+ return comedi_dev_get_from_board_minor(minor);
+ else
+ return comedi_dev_get_from_subdevice_minor(minor);
+}
+EXPORT_SYMBOL_GPL(comedi_dev_get_from_minor);
+
static struct comedi_subdevice *
comedi_read_subdevice(const struct comedi_device *dev, unsigned int minor)
{
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index 08652df..a9b1468 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -236,6 +236,8 @@ static const unsigned COMEDI_SUBDEVICE_MINOR_SHIFT = 4;
static const unsigned COMEDI_SUBDEVICE_MINOR_OFFSET = 1;
struct comedi_device *comedi_dev_from_minor(unsigned minor);
+struct comedi_device *comedi_dev_get_from_minor(unsigned minor);
+int comedi_dev_put(struct comedi_device *dev);
void init_polling(void);
void cleanup_polling(void);
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 15/22] staging: comedi: use refcount while reading /proc/comedi
In the seq_file 'show' handler for "/proc/comedi" - `comedi_read()` in
"comedi/proc.c", call `comedi_dev_get_from_minor()` instead of
`comedi_dev_from_minor()` to increment the reference counter for the
`struct comedi_device` while it is being examined. Call
`comedi_dev_put()` to decrement the reference afterwards. Also acquire
the `attach_lock` rwsem while checking whether the device is attached.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/proc.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/proc.c b/drivers/staging/comedi/proc.c
index ade0003..da6bc58 100644
--- a/drivers/staging/comedi/proc.c
+++ b/drivers/staging/comedi/proc.c
@@ -41,16 +41,20 @@ static int comedi_read(struct seq_file *m, void *v)
"driver_name, board_name, n_subdevices");
for (i = 0; i < COMEDI_NUM_BOARD_MINORS; i++) {
- struct comedi_device *dev = comedi_dev_from_minor(i);
+ struct comedi_device *dev = comedi_dev_get_from_minor(i);
+
if (!dev)
continue;
+ down_read(&dev->attach_lock);
if (dev->attached) {
devices_q = 1;
seq_printf(m, "%2d: %-20s %-20s %4d\n",
i, dev->driver->driver_name,
dev->board_name, dev->n_subdevices);
}
+ up_read(&dev->attach_lock);
+ comedi_dev_put(dev);
}
if (!devices_q)
seq_puts(m, "no devices\n");
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 14/22] staging: comedi: use refcount in comedi_driver_unregister()
Change `comedi_driver_unregister()` to call
`comedi_dev_get_from_minor()` instead of `comedi_dev_from_minor()` when
finding devices using the driver. This increments the reference count
to prevent the device being removed while it is being checked to see if
it is attached to the driver. Call `comedi_dev_put()` to decrement the
reference afterwards.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 15b1ab8..a5d03b9 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -662,7 +662,7 @@ void comedi_driver_unregister(struct comedi_driver *driver)
/* check for devices using this driver */
for (i = 0; i < COMEDI_NUM_BOARD_MINORS; i++) {
- struct comedi_device *dev = comedi_dev_from_minor(i);
+ struct comedi_device *dev = comedi_dev_get_from_minor(i);
if (!dev)
continue;
@@ -676,6 +676,7 @@ void comedi_driver_unregister(struct comedi_driver *driver)
comedi_device_detach(dev);
}
mutex_unlock(&dev->mutex);
+ comedi_dev_put(dev);
}
}
EXPORT_SYMBOL_GPL(comedi_driver_unregister);
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 10/22] staging: comedi: protect against detach during read operation
The 'read' file operation for comedi devices does not use the main mutex
in the `struct comedi_device` to avoid contention with some ioctls that
may take a while to complete. Use the `attach_lock` semaphore to
protect against detachment while the 'read' operation is in progress.
This is a `struct rw_semaphore` and we read-lock it to protect against
device detachment.
Note that `comedi_device_cancel_all()` is called during device
detachment, which cancels any ongoing asynchronous commands. This will
wake up any blocked readers which will then release the `attach_lock`
semaphore and complete the 'read' operation early.
The only time the 'read' file operation does use the main mutex is at
the end of the command when it has to call `do_become_nonbusy()` to mark
the subdevice as no longer busy handling an asynchronous command. To
avoid deadlock, it has to remove the task from the wait queue and
release the `attach_lock` semaphore before acquiring the main mutex. It
then needs to confirm the device is still attached. Unfortunately, we
do not yet protect against a dynamically allocated `struct
comedi_device` being deleted during the operation. This will be
addressed by a later patch.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 62
1 file changed, 48 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 1ae2bdf..fa8da20 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2162,24 +2162,37 @@ static ssize_t comedi_read(struct file *file, char
__user *buf, size_t nbytes,
DECLARE_WAITQUEUE(wait, current);
const unsigned minor = iminor(file_inode(file));
struct comedi_device *dev = comedi_dev_from_minor(minor);
+ unsigned int old_detach_count;
+ bool become_nonbusy = false;
+ bool attach_locked;
if (!dev)
return -ENODEV;
+ /* Protect against device detachment during operation. */
+ down_read(&dev->attach_lock);
+ attach_locked = true;
+ old_detach_count = dev->detach_count;
+
if (!dev->attached) {
DPRINTK("no driver configured on comedi%i\n", dev->minor);
- return -ENODEV;
+ retval = -ENODEV;
+ goto out;
}
s = comedi_read_subdevice(dev, minor);
- if (!s || !s->async)
- return -EIO;
+ if (!s || !s->async) {
+ retval = -EIO;
+ goto out;
+ }
async = s->async;
if (!s->busy || !nbytes)
- return 0;
- if (s->busy != file)
- return -EACCES;
+ goto out;
+ if (s->busy != file) {
+ retval = -EACCES;
+ goto out;
+ }
add_wait_queue(&async->wait_head, &wait);
while (nbytes > 0 && !retval) {
@@ -2197,13 +2210,11 @@ static ssize_t comedi_read(struct file *file, char
__user *buf, size_t nbytes,
if (n == 0) {
if (!comedi_is_subdevice_running(s)) {
- mutex_lock(&dev->mutex);
- do_become_nonbusy(dev, s);
if (comedi_is_subdevice_in_error(s))
retval = -EPIPE;
else
retval = 0;
- mutex_unlock(&dev->mutex);
+ become_nonbusy = true;
break;
}
if (file->f_flags & O_NONBLOCK) {
@@ -2241,14 +2252,37 @@ static ssize_t comedi_read(struct file *file, char
__user *buf, size_t nbytes,
buf += n;
break; /* makes device work like a pipe */
}
- if (comedi_is_subdevice_idle(s)) {
+ remove_wait_queue(&async->wait_head, &wait);
+ set_current_state(TASK_RUNNING);
+ if (become_nonbusy || comedi_is_subdevice_idle(s)) {
+ struct comedi_subdevice *new_s;
+
+ /*
+* To avoid deadlock, cannot acquire dev->mutex
+* while dev->attach_lock is held.
+*/
+ up_read(&dev->attach_lock);
+ attach_locked = false;
mutex_lock(&dev->mutex);
- if (async->buf_read_count - async->buf_write_count == 0)
- do_become_nonbusy(dev, s);
+ /*
+* Check device hasn't become detached behind our back.
+* Checking dev->detach_count is unchanged ought to be
+* sufficient (unless there have been 2**32 detaches in the
+* meantime!), but check the subdevice pointer as well just in
+* case.
+*/
+ new_s = comedi_read_subdevice(dev,
[PATCH 19/22] staging: comedi: use file->private_data in file operations
Since the `struct comedi_device` should now be protected from being
freed while an open file object is using it, use the `private_data`
member of the `struct file` to point to it. Set it in `comedi_open()`
and use it in the other file operation handlers instead of calling
`comedi_dev_from_minor()` and checking the result.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 38
1 file changed, 8 insertions(+), 30 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 37400e8..08aa93a 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1828,12 +1828,9 @@ static long comedi_unlocked_ioctl(struct file *file,
unsigned int cmd,
unsigned long arg)
{
const unsigned minor = iminor(file_inode(file));
- struct comedi_device *dev = comedi_dev_from_minor(minor);
+ struct comedi_device *dev = file->private_data;
int rc;
- if (!dev)
- return -ENODEV;
-
mutex_lock(&dev->mutex);
/* Device config is special, because it must work on
@@ -1964,7 +1961,7 @@ static struct vm_operations_struct comedi_vm_ops = {
static int comedi_mmap(struct file *file, struct vm_area_struct *vma)
{
const unsigned minor = iminor(file_inode(file));
- struct comedi_device *dev = comedi_dev_from_minor(minor);
+ struct comedi_device *dev = file->private_data;
struct comedi_subdevice *s;
struct comedi_async *async;
unsigned long start = vma->vm_start;
@@ -1973,9 +1970,6 @@ static int comedi_mmap(struct file *file, struct
vm_area_struct *vma)
int i;
int retval;
- if (!dev)
- return -ENODEV;
-
mutex_lock(&dev->mutex);
if (!dev->attached) {
@@ -2043,12 +2037,9 @@ static unsigned int comedi_poll(struct file *file,
poll_table *wait)
{
unsigned int mask = 0;
const unsigned minor = iminor(file_inode(file));
- struct comedi_device *dev = comedi_dev_from_minor(minor);
+ struct comedi_device *dev = file->private_data;
struct comedi_subdevice *s;
- if (!dev)
- return -ENODEV;
-
mutex_lock(&dev->mutex);
if (!dev->attached) {
@@ -2088,14 +2079,11 @@ static ssize_t comedi_write(struct file *file, const
char __user *buf,
int n, m, count = 0, retval = 0;
DECLARE_WAITQUEUE(wait, current);
const unsigned minor = iminor(file_inode(file));
- struct comedi_device *dev = comedi_dev_from_minor(minor);
+ struct comedi_device *dev = file->private_data;
bool on_wait_queue = false;
bool attach_locked;
unsigned int old_detach_count;
- if (!dev)
- return -ENODEV;
-
/* Protect against device detachment during operation. */
down_read(&dev->attach_lock);
attach_locked = true;
@@ -2227,14 +2215,11 @@ static ssize_t comedi_read(struct file *file, char
__user *buf, size_t nbytes,
int n, m, count = 0, retval = 0;
DECLARE_WAITQUEUE(wait, current);
const unsigned minor = iminor(file_inode(file));
- struct comedi_device *dev = comedi_dev_from_minor(minor);
+ struct comedi_device *dev = file->private_data;
unsigned int old_detach_count;
bool become_nonbusy = false;
bool attach_locked;
- if (!dev)
- return -ENODEV;
-
/* Protect against device detachment during operation. */
down_read(&dev->attach_lock);
attach_locked = true;
@@ -2424,6 +2409,7 @@ ok:
}
dev->use_count++;
+ file->private_data = dev;
rc = 0;
out:
@@ -2435,25 +2421,17 @@ out:
static int comedi_fasync(int fd, struct file *file, int on)
{
- const unsigned minor = iminor(file_inode(file));
- struct comedi_device *dev = comedi_dev_from_minor(minor);
-
- if (!dev)
- return -ENODEV;
+ struct comedi_device *dev = file->private_data;
return fasync_helper(fd, file, on, &dev->async_queue);
}
static int comedi_close(struct inode *inode, struct file *file)
{
- const unsigned minor = iminor(inode);
- struct comedi_device *dev = comedi_dev_from_minor(minor);
+ struct comedi_device *dev = file->private_data;
struct comedi_subdevice *s = NULL;
int i;
- if (!dev)
- return -ENODEV;
-
mutex_lock(&dev->mutex);
if (dev->subdevices) {
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 02/22] staging: comedi: add a couple of #includes to comedidev.h
Two structures defined in "comedidev.h" have an element of type `spinlock_t`, so add `#include ` to declare it. One structure has an element of type `struct mutex` so add `#include ` to declare it. Signed-off-by: Ian Abbott --- drivers/staging/comedi/comedidev.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/staging/comedi/comedidev.h b/drivers/staging/comedi/comedidev.h index 38a4eeb..e243247 100644 --- a/drivers/staging/comedi/comedidev.h +++ b/drivers/staging/comedi/comedidev.h @@ -20,6 +20,8 @@ #define _COMEDIDEV_H #include +#include +#include #include "comedi.h" -- 1.8.4.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 06/22] staging: comedi: wake up async waiters when become non-busy
Wake up all waiters on the comedi subdevice's async wait queue whenever
the subdevice is marked "non-busy". This happens when an asynchronous
command is cancelled or when a command is terminated and all data has
been read or written. Note: use `wake_up_interruptible_all()` as we
only use interruptible waits.
Remove the call to `wake_up_interruptible()` from `do_cancel_ioctl()` as
it will call `wake_up_interruptible_all()` indirectly via `do_cancel()`
and `do_become_nonbusy()`.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index a1705ed..9c85f01 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -563,12 +563,13 @@ static void do_become_nonbusy(struct comedi_device *dev,
async->inttrig = NULL;
kfree(async->cmd.chanlist);
async->cmd.chanlist = NULL;
+ s->busy = NULL;
+ wake_up_interruptible_all(&s->async->wait_head);
} else {
dev_err(dev->class_dev,
"BUG: (?) do_become_nonbusy called with async=NULL\n");
+ s->busy = NULL;
}
-
- s->busy = NULL;
}
static int do_cancel(struct comedi_device *dev, struct comedi_subdevice *s)
@@ -1700,7 +1701,6 @@ static int do_cancel_ioctl(struct comedi_device *dev,
unsigned int arg,
return -EBUSY;
ret = do_cancel(dev, s);
- wake_up_interruptible(&s->async->wait_head);
return ret;
}
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 18/22] staging: comedi: kcomedilib: protect against device detachment
The functions in "kcomedilib" need to prevent the comedi device being
detached during their operation. This can be done by acquiring either
the main mutex or the "attach lock" semaphore in the `struct
comedi_device`. Use the attach lock when merely checking whether the
device is attached. Use the mutex when processing a comedi instruction.
Also, don't bother trying to manipulate the module use count of
low-level comedi driver in `comedi_open()` and `comedi_close()`. If the
device gets detached while it is "open", we wouldn't be able to
decrement the module use count anyway.
Signed-off-by: Ian Abbott
---
.../staging/comedi/kcomedilib/kcomedilib_main.c| 57 +-
1 file changed, 35 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/comedi/kcomedilib/kcomedilib_main.c
b/drivers/staging/comedi/kcomedilib/kcomedilib_main.c
index b3a06f2..7dc5a18 100644
--- a/drivers/staging/comedi/kcomedilib/kcomedilib_main.c
+++ b/drivers/staging/comedi/kcomedilib/kcomedilib_main.c
@@ -50,10 +50,12 @@ struct comedi_device *comedi_open(const char *filename)
if (!dev)
return NULL;
- if (dev->attached && try_module_get(dev->driver->module))
+ down_read(&dev->attach_lock);
+ if (dev->attached)
retval = dev;
else
retval = NULL;
+ up_read(&dev->attach_lock);
if (retval == NULL)
comedi_dev_put(dev);
@@ -62,13 +64,9 @@ struct comedi_device *comedi_open(const char *filename)
}
EXPORT_SYMBOL_GPL(comedi_open);
-int comedi_close(struct comedi_device *d)
+int comedi_close(struct comedi_device *dev)
{
- struct comedi_device *dev = (struct comedi_device *)d;
-
- module_put(dev->driver->module);
comedi_dev_put(dev);
-
return 0;
}
EXPORT_SYMBOL_GPL(comedi_close);
@@ -78,7 +76,14 @@ static int comedi_do_insn(struct comedi_device *dev,
unsigned int *data)
{
struct comedi_subdevice *s;
- int ret = 0;
+ int ret;
+
+ mutex_lock(&dev->mutex);
+
+ if (!dev->attached) {
+ ret = -EINVAL;
+ goto error;
+ }
/* a subdevice instruction */
if (insn->subdev >= dev->n_subdevices) {
@@ -125,6 +130,7 @@ static int comedi_do_insn(struct comedi_device *dev,
s->busy = NULL;
error:
+ mutex_unlock(&dev->mutex);
return ret;
}
@@ -174,9 +180,6 @@ int comedi_dio_bitfield2(struct comedi_device *dev,
unsigned int subdev,
unsigned int shift;
int ret;
- if (subdev >= dev->n_subdevices)
- return -EINVAL;
-
base_channel = CR_CHAN(base_channel);
n_chan = comedi_get_n_channels(dev, subdev);
if (base_channel >= n_chan)
@@ -216,23 +219,33 @@ int comedi_find_subdevice_by_type(struct comedi_device
*dev, int type,
unsigned int subd)
{
struct comedi_subdevice *s;
-
- if (subd > dev->n_subdevices)
- return -ENODEV;
-
- for (; subd < dev->n_subdevices; subd++) {
- s = &dev->subdevices[subd];
- if (s->type == type)
- return subd;
- }
- return -1;
+ int ret = -ENODEV;
+
+ down_read(&dev->attach_lock);
+ if (dev->attached)
+ for (; subd < dev->n_subdevices; subd++) {
+ s = &dev->subdevices[subd];
+ if (s->type == type) {
+ ret = subd;
+ break;
+ }
+ }
+ up_read(&dev->attach_lock);
+ return ret;
}
EXPORT_SYMBOL_GPL(comedi_find_subdevice_by_type);
int comedi_get_n_channels(struct comedi_device *dev, unsigned int subdevice)
{
- struct comedi_subdevice *s = &dev->subdevices[subdevice];
+ int n;
+
+ down_read(&dev->attach_lock);
+ if (!dev->attached || subdevice >= dev->n_subdevices)
+ n = 0;
+ else
+ n = dev->subdevices[subdevice].n_chan;
+ up_read(&dev->attach_lock);
- return s->n_chan;
+ return n;
}
EXPORT_SYMBOL_GPL(comedi_get_n_channels);
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 05/22] staging: comedi: cleanup_device() -> comedi_device_detach_cleanup()
Rename the local function `cleanup_device()` to
`comedi_device_detach_cleanup()`. It is only called from the
`comedi_device_detach()` function and that is called from
`comedi_device_cleanup()` and other places. The more specific function
name seems less confusing.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index c398422..1f44f29 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -95,7 +95,7 @@ int comedi_alloc_subdevices(struct comedi_device *dev, int
num_subdevices)
}
EXPORT_SYMBOL_GPL(comedi_alloc_subdevices);
-static void cleanup_device(struct comedi_device *dev)
+static void comedi_device_detach_cleanup(struct comedi_device *dev)
{
int i;
struct comedi_subdevice *s;
@@ -137,7 +137,7 @@ void comedi_device_detach(struct comedi_device *dev)
dev->attached = false;
if (dev->driver)
dev->driver->detach(dev);
- cleanup_device(dev);
+ comedi_device_detach_cleanup(dev);
up_write(&dev->attach_lock);
}
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 22/22] staging: comedi: protect buffer from being freed while mmapped
If a comedi device is automatically detached by `comedi_auto_unconfig()`
any data buffers associated with subdevices that support asynchronous
commands will be freed. If the buffer is mmapped at the time, bad
things are likely to happen! Prevent this by moving some of the buffer
details from `struct comedi_async` into a new, dynamically allocated,
and kref-counted `struct comedi_buf_map`. This holds a list of pages, a
reference count, and enough information to free the pages. The new
member `buf_map` of `struct comedi_async` points to a `struct
comedi_buf_map` when the buffer size is non-zero.
Provide a new helper function `comedi_buf_is_mapped()` to check whether
an a buffer is mmapped. If it is mmapped, the buffer is not allowed to
be resized and the device is not allowed to be manually detached by the
`COMEDI_DEVCONFIG` ioctl. Provide helper functions
`comedi_buf_map_get()` and `comedi_buf_map_put()` to manipulate the
reference count of the `struct comedi_buf_map`, which will be freed
along with its contents via the 'release' callback of the `kref_put()`
call. The reference count is manipulated by the vma operations and the
mmap file operation.
Now, when the comedi device is automatically detached, the buffer will
be effectively freed by calling `comedi_buf_alloc()` with a new buffer
size of 0. That calls local function `__comedi_buf_free()` which calls
`comedi_buf_map_put()` on the `buf_map` member to free it. It won't
actually be freed until the final 'put'.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_buf.c | 99 ++--
drivers/staging/comedi/comedi_fops.c | 38 ++--
drivers/staging/comedi/comedi_internal.h | 3 +
drivers/staging/comedi/comedidev.h | 14 +++--
drivers/staging/comedi/drivers/mite.c| 2 +-
5 files changed, 99 insertions(+), 57 deletions(-)
diff --git a/drivers/staging/comedi/comedi_buf.c
b/drivers/staging/comedi/comedi_buf.c
index 4e26bd7..924fce9 100644
--- a/drivers/staging/comedi/comedi_buf.c
+++ b/drivers/staging/comedi/comedi_buf.c
@@ -16,6 +16,7 @@
*/
#include
+#include
#include "comedidev.h"
#include "comedi_internal.h"
@@ -26,31 +27,21 @@
#define COMEDI_PAGE_PROTECTION PAGE_KERNEL
#endif
-static void __comedi_buf_free(struct comedi_device *dev,
- struct comedi_subdevice *s,
- unsigned n_pages)
+static void comedi_buf_map_kref_release(struct kref *kref)
{
- struct comedi_async *async = s->async;
+ struct comedi_buf_map *bm =
+ container_of(kref, struct comedi_buf_map, refcount);
struct comedi_buf_page *buf;
- unsigned i;
-
- if (async->prealloc_buf) {
- vunmap(async->prealloc_buf);
- async->prealloc_buf = NULL;
- async->prealloc_bufsz = 0;
- }
+ unsigned int i;
- if (!async->buf_page_list)
- return;
-
- for (i = 0; i < n_pages; ++i) {
- buf = &async->buf_page_list[i];
- if (buf->virt_addr) {
+ if (bm->page_list) {
+ for (i = 0; i < bm->n_pages; i++) {
+ buf = &bm->page_list[i];
clear_bit(PG_reserved,
&(virt_to_page(buf->virt_addr)->flags));
- if (s->async_dma_dir != DMA_NONE) {
+ if (bm->dma_dir != DMA_NONE) {
#ifdef CONFIG_HAS_DMA
- dma_free_coherent(dev->hw_dev,
+ dma_free_coherent(bm->dma_hw_dev,
PAGE_SIZE,
buf->virt_addr,
buf->dma_addr);
@@ -59,10 +50,26 @@ static void __comedi_buf_free(struct comedi_device *dev,
free_page((unsigned long)buf->virt_addr);
}
}
+ vfree(bm->page_list);
}
- vfree(async->buf_page_list);
- async->buf_page_list = NULL;
- async->n_buf_pages = 0;
+ if (bm->dma_dir != DMA_NONE)
+ put_device(bm->dma_hw_dev);
+ kfree(bm);
+}
+
+static void __comedi_buf_free(struct comedi_device *dev,
+ struct comedi_subdevice *s)
+{
+ struct comedi_async *async = s->async;
+
+ if (async->prealloc_buf) {
+ vunmap(async->prealloc_buf);
+ async->prealloc_buf = NULL;
+ async->prealloc_bufsz = 0;
+ }
+
+ comedi_buf_map_put(async->buf_map);
+ async->buf_map = NULL;
}
static void __comedi_buf_alloc(struct comedi_device *dev,
@@ -71,6 +78,7 @@ static void __comedi_buf_alloc(struct comedi_device *dev,
{
struct comedi_async *async = s->async;
struct page **pages = NULL;
+ struct comedi_buf_map *bm;
struct comedi_buf_page *buf;
unsigned i;
[PATCH 21/22] staging: comedi: make determination of read or write subdevice safer
`comedi_read_subdevice()` and `comedi_write_subdevice()` respectively
determine the read and write subdevice to use for a comedi device,
depending on a minor device number passed in. The comedi device has a
main "board" minor device number and may also have dynamically assigned,
subdevice-specific minor device numbers, in a range of numbers shared by
all comedi devices. If the minor device number is within the range of
subdevice-specific minor device numbers, both functions call
`comedi_subdevice_from_minor()` to determine what subdevice is
associated with the minor device number (if any) and then check the
subdevice belongs to the comedi device. Since the subdevice might
belong to a different comedi device, the check is not protected against
the subdevice being freed. Perform the check in
`comedi_subdevice_from_minor()` instead, where it is protected against
the subdevice being freed. Make it return `NULL` if the subdevice does
not belong to the device.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 16 +++-
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index d569484..8cb9d95 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -177,7 +177,7 @@ static void comedi_free_board_dev(struct comedi_device *dev)
}
static struct comedi_subdevice
-*comedi_subdevice_from_minor(unsigned minor)
+*comedi_subdevice_from_minor(const struct comedi_device *dev, unsigned minor)
{
struct comedi_subdevice *s;
unsigned int i = minor - COMEDI_NUM_BOARD_MINORS;
@@ -185,6 +185,8 @@ static struct comedi_subdevice
BUG_ON(i >= COMEDI_NUM_SUBDEVICE_MINORS);
mutex_lock(&comedi_subdevice_minor_table_lock);
s = comedi_subdevice_minor_table[i];
+ if (s && s->device != dev)
+ s = NULL;
mutex_unlock(&comedi_subdevice_minor_table_lock);
return s;
}
@@ -229,10 +231,8 @@ comedi_read_subdevice(const struct comedi_device *dev,
unsigned int minor)
struct comedi_subdevice *s;
if (minor >= COMEDI_NUM_BOARD_MINORS) {
- s = comedi_subdevice_from_minor(minor);
- if (!s || s->device != dev)
- return NULL;
- if (s->subdev_flags & SDF_CMD_READ)
+ s = comedi_subdevice_from_minor(dev, minor);
+ if (s == NULL || (s->subdev_flags & SDF_CMD_READ))
return s;
}
return dev->read_subdev;
@@ -244,10 +244,8 @@ comedi_write_subdevice(const struct comedi_device *dev,
unsigned int minor)
struct comedi_subdevice *s;
if (minor >= COMEDI_NUM_BOARD_MINORS) {
- s = comedi_subdevice_from_minor(minor);
- if (!s || s->device != dev)
- return NULL;
- if (s->subdev_flags & SDF_CMD_WRITE)
+ s = comedi_subdevice_from_minor(dev, minor);
+ if (s == NULL || (s->subdev_flags & SDF_CMD_WRITE))
return s;
}
return dev->write_subdev;
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 20/22] staging: comedi: remove comedi_dev_from_minor()
The `comedi_dev_from_minor()` function is no longer used, so remove it.
Calls to it have either been replaced by calls to
`comedi_dev_get_from_minor()` or by using the `private_data` member of
the open file object.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 28
drivers/staging/comedi/comedidev.h | 1 -
2 files changed, 29 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 08aa93a..d569484 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -189,34 +189,6 @@ static struct comedi_subdevice
return s;
}
-static struct comedi_device *comedi_dev_from_board_minor(unsigned minor)
-{
- struct comedi_device *dev;
-
- BUG_ON(minor >= COMEDI_NUM_BOARD_MINORS);
- mutex_lock(&comedi_board_minor_table_lock);
- dev = comedi_board_minor_table[minor];
- mutex_unlock(&comedi_board_minor_table_lock);
- return dev;
-}
-
-static struct comedi_device *comedi_dev_from_subdevice_minor(unsigned minor)
-{
- struct comedi_subdevice *s;
-
- s = comedi_subdevice_from_minor(minor);
- return s ? s->device : NULL;
-}
-
-struct comedi_device *comedi_dev_from_minor(unsigned minor)
-{
- if (minor < COMEDI_NUM_BOARD_MINORS)
- return comedi_dev_from_board_minor(minor);
- else
- return comedi_dev_from_subdevice_minor(minor);
-}
-EXPORT_SYMBOL_GPL(comedi_dev_from_minor);
-
static struct comedi_device *comedi_dev_get_from_board_minor(unsigned minor)
{
struct comedi_device *dev;
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index a9b1468..4e7cd14 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -235,7 +235,6 @@ enum comedi_minor_bits {
static const unsigned COMEDI_SUBDEVICE_MINOR_SHIFT = 4;
static const unsigned COMEDI_SUBDEVICE_MINOR_OFFSET = 1;
-struct comedi_device *comedi_dev_from_minor(unsigned minor);
struct comedi_device *comedi_dev_get_from_minor(unsigned minor);
int comedi_dev_put(struct comedi_device *dev);
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 17/22] staging: comedi: kcomedilib: increment reference while device in use
Low-level comedi drivers that use the "kcomedilib" module (currently only
the "comedi_bond" driver) call `comedi_open()` to "open" another comedi
device (not as a file) and `comedi_close()` to "close" it. (Note: these
are the functions exported by the "kcomedilib" module, not the
identically named, statically linked functions in the core "comedi"
module.)
In `comedi_open()`, call `comedi_dev_get_from_minor()` instead of
`comedi_dev_from_minor()` to get the pointer to the `struct
comedi_device` being "opened". This increments its reference count to
prevent it being freed. Call `comedi_dev_put()` if `comedi_open()`
returns `NULL`, and also call it from `comedi_close()`. This decrements
the reference count.
Note that although we now protect against the `struct comedi_device`
being freed, we do not yet protect against it being "detached" while it
is being used. This will be addressed by a later patch.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/kcomedilib/kcomedilib_main.c | 19 ---
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/kcomedilib/kcomedilib_main.c
b/drivers/staging/comedi/kcomedilib/kcomedilib_main.c
index cd60677..b3a06f2 100644
--- a/drivers/staging/comedi/kcomedilib/kcomedilib_main.c
+++ b/drivers/staging/comedi/kcomedilib/kcomedilib_main.c
@@ -35,7 +35,7 @@ MODULE_LICENSE("GPL");
struct comedi_device *comedi_open(const char *filename)
{
- struct comedi_device *dev;
+ struct comedi_device *dev, *retval = NULL;
unsigned int minor;
if (strncmp(filename, "/dev/comedi", 11) != 0)
@@ -46,15 +46,19 @@ struct comedi_device *comedi_open(const char *filename)
if (minor >= COMEDI_NUM_BOARD_MINORS)
return NULL;
- dev = comedi_dev_from_minor(minor);
-
- if (!dev || !dev->attached)
+ dev = comedi_dev_get_from_minor(minor);
+ if (!dev)
return NULL;
- if (!try_module_get(dev->driver->module))
- return NULL;
+ if (dev->attached && try_module_get(dev->driver->module))
+ retval = dev;
+ else
+ retval = NULL;
+
+ if (retval == NULL)
+ comedi_dev_put(dev);
- return dev;
+ return retval;
}
EXPORT_SYMBOL_GPL(comedi_open);
@@ -63,6 +67,7 @@ int comedi_close(struct comedi_device *d)
struct comedi_device *dev = (struct comedi_device *)d;
module_put(dev->driver->module);
+ comedi_dev_put(dev);
return 0;
}
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 03/22] staging: comedi: add rw_semaphore to protect against device detachment
The 'read' and 'write' file operations on comedi devices do not use the
main mutex in the `struct comedi_device` to avoid contention with ioctls
that may take a while to complete. However, it is necessary to protect
against the device being detached while the operation is in progress.
Add member `struct rw_semaphore attach_lock` to `struct comedi_device`
for this purpose and initialize it on creation.
The actual locking and unlocking will be implemented by subsequent
patches. Tasks that are attaching or detaching comedi devices will
write-acquire the new semaphore whilst also holding the main mutex in
the `struct comedi_device`. Tasks that wish to protect against the
comedi device being detached need to acquire either the main mutex, or
read-acquire the new semaphore, or both in that order.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 1 +
drivers/staging/comedi/comedidev.h | 2 ++
2 files changed, 3 insertions(+)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 0d85654..a1705ed 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -91,6 +91,7 @@ static void comedi_device_init(struct comedi_device *dev)
{
spin_lock_init(&dev->spinlock);
mutex_init(&dev->mutex);
+ init_rwsem(&dev->attach_lock);
dev->minor = -1;
}
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index e243247..70de355 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
#include "comedi.h"
@@ -184,6 +185,7 @@ struct comedi_device {
bool ioenabled:1;
spinlock_t spinlock;
struct mutex mutex;
+ struct rw_semaphore attach_lock;
int n_subdevices;
struct comedi_subdevice *subdevices;
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 01/22] staging: comedi: remove unused command callback support
The 'kcomedilib' module used to provide functions to allow asynchronous
comedi commands to be set up from another kernel module, but now
commands can only be set up by ioctls from user space via the core
comedi module. Since support for commands initiated from kernel space
has been dropped, the `cb_func` and `cb_arg` members of `struct
comedi_async` are never set (although the `cb_mask` member is still used
to mask comedi events). The `SRF_USER` bit of the comedi subdevice
runflags is no longer needed to distinguish commands from user and
kernel space since they only come from user space.
Don't bother setting or testing the `SRF_USER` flag, and get rid of it,
along with the `cb_func` and `cb_arg` members.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 21 +++--
drivers/staging/comedi/comedidev.h | 4
2 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index f3d59e2..0d85654 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1469,8 +1469,7 @@ static int do_cmd_ioctl(struct comedi_device *dev,
if (async->cmd.flags & TRIG_WAKE_EOS)
async->cb_mask |= COMEDI_CB_EOS;
- comedi_set_subdevice_runflags(s, SRF_USER | SRF_ERROR | SRF_RUNNING,
- SRF_USER | SRF_RUNNING);
+ comedi_set_subdevice_runflags(s, SRF_ERROR | SRF_RUNNING, SRF_RUNNING);
/* set s->busy _after_ setting SRF_RUNNING flag to avoid race with
* comedi_read() or comedi_write() */
@@ -1700,8 +1699,7 @@ static int do_cancel_ioctl(struct comedi_device *dev,
unsigned int arg,
return -EBUSY;
ret = do_cancel(dev, s);
- if (comedi_get_subdevice_runflags(s) & SRF_USER)
- wake_up_interruptible(&s->async->wait_head);
+ wake_up_interruptible(&s->async->wait_head);
return ret;
}
@@ -2368,16 +2366,11 @@ void comedi_event(struct comedi_device *dev, struct
comedi_subdevice *s)
}
if (async->cb_mask & s->async->events) {
- if (comedi_get_subdevice_runflags(s) & SRF_USER) {
- wake_up_interruptible(&async->wait_head);
- if (s->subdev_flags & SDF_CMD_READ)
- kill_fasync(&dev->async_queue, SIGIO, POLL_IN);
- if (s->subdev_flags & SDF_CMD_WRITE)
- kill_fasync(&dev->async_queue, SIGIO, POLL_OUT);
- } else {
- if (async->cb_func)
- async->cb_func(s->async->events, async->cb_arg);
- }
+ wake_up_interruptible(&async->wait_head);
+ if (s->subdev_flags & SDF_CMD_READ)
+ kill_fasync(&dev->async_queue, SIGIO, POLL_IN);
+ if (s->subdev_flags & SDF_CMD_WRITE)
+ kill_fasync(&dev->async_queue, SIGIO, POLL_OUT);
}
s->async->events = 0;
}
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index 143be80..38a4eeb 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -141,10 +141,7 @@ struct comedi_async {
wait_queue_head_t wait_head;
- /* callback stuff */
unsigned int cb_mask;
- int (*cb_func) (unsigned int flags, void *);
- void *cb_arg;
int (*inttrig) (struct comedi_device *dev, struct comedi_subdevice *s,
unsigned int x);
@@ -240,7 +237,6 @@ void stop_polling(struct comedi_device *);
/* subdevice runflags */
enum subdevice_runflags {
- SRF_USER = 0x0001,
SRF_RT = 0x0002,
/* indicates an COMEDI_CB_ERROR event has occurred since the last
* command was started */
--
1.8.4.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
