[PATCH v2] staging: pi433: fix race condition in pi433_ioctl
In the PI433_IOC_WR_TX_CFG case in pi433_ioctl, instance->tx_cfg is
modified via
copy_from_user(>tx_cfg, argp, sizeof(struct pi433_tx_cfg)))
without any kind of synchronization. In the case where two threads
would execute this same command concurrently the tx_cfg field might
enter in an inconsistent state.
Additionally: if ioctl(PI433_IOC_WR_TX_CFG) and write() execute
concurrently the tx config might be modified while it is being
copied to the fifo, resulting in potential data corruption.
Fix: Get instance->tx_cfg_lock before modifying tx config in the
PI433_IOC_WR_TX_CFG case in pi433_ioctl.
Also, do not copy data directly from user space to instance->tx_cfg.
Instead use a temporary buffer allowing future checks for correctness
of copied data.
Signed-off-by: Hugo Lefeuvre
---
Changes in v2:
- Use device->tx_fifo_lock instead of introducing a new lock in
instance.
- Do not copy data directly from user space to instance->tx_cfg,
instead use a temporary buffer allowing future checks for
correctness of copied data.
---
drivers/staging/pi433/pi433_if.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index b061f77dda41..3ec1ed01d04b 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -880,6 +880,7 @@ pi433_ioctl(struct file *filp, unsigned int cmd, unsigned
long arg)
int retval = 0;
struct pi433_instance *instance;
struct pi433_device *device;
+ struct pi433_tx_cfg tx_cfg_buffer;
void __user *argp = (void __user *)arg;
/* Check type and command number */
@@ -902,9 +903,15 @@ pi433_ioctl(struct file *filp, unsigned int cmd, unsigned
long arg)
return -EFAULT;
break;
case PI433_IOC_WR_TX_CFG:
- if (copy_from_user(>tx_cfg, argp,
- sizeof(struct pi433_tx_cfg)))
+ /* do not modify tx config while it is being copied to fifo */
+ mutex_lock(>tx_fifo_lock);
+ if (copy_from_user(_cfg_buffer, argp,
+ sizeof(struct pi433_tx_cfg))) {
+ mutex_unlock(>tx_fifo_lock);
return -EFAULT;
+ }
+ memcpy(>tx_cfg, _cfg_buffer, sizeof(struct
pi433_tx_cfg));
+ mutex_unlock(>tx_fifo_lock);
break;
case PI433_IOC_RD_RX_CFG:
if (copy_to_user(argp, >rx_cfg,
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: pi433: add mutex fixing concurrency issues.
> > case PI433_IOC_WR_TX_CFG: > > if (copy_from_user(>tx_cfg, argp, > >sizeof(struct pi433_tx_cfg))) > > return -EFAULT; > > break; > > Btw, it looks so wrong to me that we copy partial data to > >tx_cfg... I'd really prefer copying it to a tmp buffer and > then verifying it's corrent then memcpy()ing it to >tx_cfg. AFAIK this piece of code is not supposed to check passed tx config. These checks are realised later by rf69_set_tx_cfg (called by pi433_tx_thread) after the config has been written to the fifo by pi433_write. What kind of checks do you want to perform exactly ? But, right, I prefer the idea of the temporary buffer too, and seeing the rest of kernel code it seems to be the usual way to go. Regards, Hugo -- Hugo Lefeuvre (hle)|www.owl.eu.com 4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: pi433: add mutex fixing race condition when accessing tx_cfg
> We read the data from the user here and then we write it to the fifo > in pi433_write(). We should be using the device->tx_fifo_lock so that > we don't copy over the data at the same time we're writing it to the > fifo. Oh right, that makes the bug even worse. In this case we don't even need to introduce a new lock, using device->tx_fifo_lock should be fine. I'll update the patch. -- Hugo Lefeuvre (hle)|www.owl.eu.com 4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net 0/3] hv_netvsc: notification and namespace fixes
From: Stephen Hemminger Date: Mon, 11 Jun 2018 12:44:53 -0700 > This set of patches addresses two set of fixes. First it backs out > the common callback model which was merged in net-next without > completing all the review feedback or getting maintainer approval. > > Then it fixes the transparent VF management code to handle network > namespaces. Series applied. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] hv: add driver_override support
Add support for overriding the default driver for a VMBus device
in the same way that it can be done for PCI devices. This patch
adds the /sys/bus/vmbus/devices/.../driver_override file
and the logic for matching.
This is used by driverctl tool to do driver override.
https://gitlab.com/driverctl/driverctl
When using devices with DPDK it is useful to be able to use
this tool instead of manual bind/unbind.
Signed-off-by: Stephen Hemminger
---
Resend of patch, that seems to have gotten lost.
Documentation/ABI/testing/sysfs-bus-vmbus | 6 ++
drivers/hv/vmbus_drv.c| 115 ++
include/linux/hyperv.h| 1 +
3 files changed, 103 insertions(+), 19 deletions(-)
create mode 100644 Documentation/ABI/testing/sysfs-bus-vmbus
diff --git a/Documentation/ABI/testing/sysfs-bus-vmbus
b/Documentation/ABI/testing/sysfs-bus-vmbus
new file mode 100644
index ..4f57b8f8b767
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-vmbus
@@ -0,0 +1,6 @@
+What: /sys/bus/vmbus/devices/.../driver_override
+Date: April 2018
+Contact: Stephen Hemminger
+Description:
+ This file allows the driver for a device to be specified which
+ will override standard static and dynamic ID matching.
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index b10fe26c4891..b5e3b3dc1bbd 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -482,6 +482,54 @@ static ssize_t device_show(struct device *dev,
}
static DEVICE_ATTR_RO(device);
+static ssize_t driver_override_store(struct device *dev,
+struct device_attribute *attr,
+const char *buf, size_t count)
+{
+ struct hv_device *hv_dev = device_to_hv_device(dev);
+ char *driver_override, *old, *cp;
+
+ /* We need to keep extra room for a newline */
+ if (count >= (PAGE_SIZE - 1))
+ return -EINVAL;
+
+ driver_override = kstrndup(buf, count, GFP_KERNEL);
+ if (!driver_override)
+ return -ENOMEM;
+
+ cp = strchr(driver_override, '\n');
+ if (cp)
+ *cp = '\0';
+
+ device_lock(dev);
+ old = hv_dev->driver_override;
+ if (strlen(driver_override)) {
+ hv_dev->driver_override = driver_override;
+ } else {
+ kfree(driver_override);
+ hv_dev->driver_override = NULL;
+ }
+ device_unlock(dev);
+
+ kfree(old);
+
+ return count;
+}
+
+static ssize_t driver_override_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hv_device *hv_dev = device_to_hv_device(dev);
+ ssize_t len;
+
+ device_lock(dev);
+ len = snprintf(buf, PAGE_SIZE, "%s\n", hv_dev->driver_override);
+ device_unlock(dev);
+
+ return len;
+}
+static DEVICE_ATTR_RW(driver_override);
+
/* Set up per device attributes in /sys/bus/vmbus/devices/ */
static struct attribute *vmbus_dev_attrs[] = {
_attr_id.attr,
@@ -509,6 +557,7 @@ static struct attribute *vmbus_dev_attrs[] = {
_attr_channel_vp_mapping.attr,
_attr_vendor.attr,
_attr_device.attr,
+ _attr_driver_override.attr,
NULL,
};
ATTRIBUTE_GROUPS(vmbus_dev);
@@ -544,17 +593,26 @@ static inline bool is_null_guid(const uuid_le *guid)
return true;
}
-/*
- * Return a matching hv_vmbus_device_id pointer.
- * If there is no match, return NULL.
- */
-static const struct hv_vmbus_device_id *hv_vmbus_get_id(struct hv_driver *drv,
- const uuid_le *guid)
+static const struct hv_vmbus_device_id *
+hv_vmbus_dev_match(const struct hv_vmbus_device_id *id, const uuid_le *guid)
+
+{
+ if (id == NULL)
+ return NULL; /* empty device table */
+
+ for (; !is_null_guid(>guid); id++)
+ if (!uuid_le_cmp(id->guid, *guid))
+ return id;
+
+ return NULL;
+}
+
+static const struct hv_vmbus_device_id *
+hv_vmbus_dynid_match(struct hv_driver *drv, const uuid_le *guid)
{
const struct hv_vmbus_device_id *id = NULL;
struct vmbus_dynid *dynid;
- /* Look at the dynamic ids first, before the static ones */
spin_lock(>dynids.lock);
list_for_each_entry(dynid, >dynids.list, node) {
if (!uuid_le_cmp(dynid->id.guid, *guid)) {
@@ -564,18 +622,37 @@ static const struct hv_vmbus_device_id
*hv_vmbus_get_id(struct hv_driver *drv,
}
spin_unlock(>dynids.lock);
- if (id)
- return id;
+ return id;
+}
- id = drv->id_table;
- if (id == NULL)
- return NULL; /* empty device table */
+static const struct hv_vmbus_device_id vmbus_device_null = {
+ .guid = NULL_UUID_LE,
+};
- for (; !is_null_guid(>guid); id++)
- if (!uuid_le_cmp(id->guid,
Re: [PATCH 1/3] staging: hwmon: Fix SPACING errors
On Tue, Jun 12, 2018 at 08:56:58PM +0200, Jakob Albert wrote:
> Fix SPACING errors in drivers/hwmon/nct7904.c reported by checkpatch.pl
>
Subject for all patches in this series should be
hwmon: (nct7904)
There is no need to reference the file name in the patch description.
It is obvious from both Subject and the patch itself.
linux-hw...@vger.kernel.org needs to be in Cc:.
As Greg already mentioned, this series has nothing to do with staging.
Thanks,
Guenter
> Signed-off-by: Lorenz Kaestle
> Signed-off-by: Jakob Albert
> ---
> drivers/hwmon/nct7904.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hwmon/nct7904.c b/drivers/hwmon/nct7904.c
> index 95a68ab..7de2421 100644
> --- a/drivers/hwmon/nct7904.c
> +++ b/drivers/hwmon/nct7904.c
> @@ -159,7 +159,7 @@ static int nct7904_read_fan(struct device *dev, u32 attr,
> int channel,
> unsigned int cnt, rpm;
> int ret;
>
> - switch(attr) {
> + switch (attr) {
> case hwmon_fan_input:
> ret = nct7904_read_reg16(data, BANK_0,
>FANIN1_HV_REG + channel * 2);
> @@ -200,7 +200,7 @@ static int nct7904_read_in(struct device *dev, u32 attr,
> int channel,
>
> index = nct7904_chan_to_index[channel];
>
> - switch(attr) {
> + switch (attr) {
> case hwmon_in_input:
> ret = nct7904_read_reg16(data, BANK_0,
>VSEN1_HV_REG + index * 2);
> @@ -236,7 +236,7 @@ static int nct7904_read_temp(struct device *dev, u32
> attr, int channel,
> struct nct7904_data *data = dev_get_drvdata(dev);
> int ret, temp;
>
> - switch(attr) {
> + switch (attr) {
> case hwmon_temp_input:
> if (channel == 0)
> ret = nct7904_read_reg16(data, BANK_0, LTD_HV_REG);
> @@ -276,7 +276,7 @@ static int nct7904_read_pwm(struct device *dev, u32 attr,
> int channel,
> struct nct7904_data *data = dev_get_drvdata(dev);
> int ret;
>
> - switch(attr) {
> + switch (attr) {
> case hwmon_pwm_input:
> ret = nct7904_read_reg(data, BANK_3, FANCTL1_OUT_REG + channel);
> if (ret < 0)
> @@ -301,7 +301,7 @@ static int nct7904_write_pwm(struct device *dev, u32
> attr, int channel,
> struct nct7904_data *data = dev_get_drvdata(dev);
> int ret;
>
> - switch(attr) {
> + switch (attr) {
> case hwmon_pwm_input:
> if (val < 0 || val > 255)
> return -EINVAL;
> @@ -322,7 +322,7 @@ static int nct7904_write_pwm(struct device *dev, u32
> attr, int channel,
>
> static umode_t nct7904_pwm_is_visible(const void *_data, u32 attr, int
> channel)
> {
> - switch(attr) {
> + switch (attr) {
> case hwmon_pwm_input:
> case hwmon_pwm_enable:
> return S_IRUGO | S_IWUSR;
> --
> 2.7.4
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] dt-bindings: document gpio-mt7621 bindings
On Sat, Jun 02, 2018 at 09:30:10AM +0200, Sergio Paracuellos wrote:
> Add a devicetree binding documentation for the mt7621 driver.
Bindings are for h/w, not a driver.
> Signed-off-by: Sergio Paracuellos
> Reviewed-by: NeilBrown
Space ^
> ---
> .../bindings/gpio/mediatek,mt7621-gpio.txt | 68
> ++
> 1 file changed, 68 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/gpio/mediatek,mt7621-gpio.txt
>
> diff --git a/Documentation/devicetree/bindings/gpio/mediatek,mt7621-gpio.txt
> b/Documentation/devicetree/bindings/gpio/mediatek,mt7621-gpio.txt
> new file mode 100644
> index 000..30d8a02
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpio/mediatek,mt7621-gpio.txt
> @@ -0,0 +1,68 @@
> +Mediatek SoC GPIO controller bindings
> +
> +The IP core used inside these SoCs has 3 banks of 32 GPIOs each.
> +The registers of all the banks are interwoven inside one single IO range.
> +We load one GPIO controller instance per bank. To make this possible
> +we support 2 types of nodes. The parent node defines the memory I/O range and
> +has 3 children each describing a single bank. Also the GPIO controller can
> receive
> +interrupts on any of the GPIOs, either edge or level. It then interrupts the
> CPU
> +using GIC INT12.
> +
> +Required properties for the top level node:
> +- compatible:
> + - "mediatek,mt7621-gpio" for Mediatek controllers
> +- reg : Physical base address and length of the controller's registers
> +- interrupt-parent : phandle of the parent interrupt controller.
> +- interrupts : Interrupt specifier for the controllers interrupt.
> +- interrupt-controller : Mark the device node as an interrupt controller.
> +- #interrupt-cells : Should be 2. The first cell defines the interrupt
> number.
> + The second cell bits[3:0] is used to specify trigger type as follows:
> + - 1 = low-to-high edge triggered.
> + - 2 = high-to-low edge triggered.
> + - 4 = active high level-sensitive.
> + - 8 = active low level-sensitive.
Just refer to the common definition.
> +
> +
> +Required properties for the GPIO bank node:
> +- compatible:
> + - "mediatek,mt7621-gpio-bank" for Mediatek banks
> +- #gpio-cells : Should be two. The first cell is the GPIO pin number and the
So interrupt numbers and gpio numbers are different? 0-95 and 3x 0-31
That doesn't seem ideal.
> + second cell specifies GPIO flags, as defined in .
> + Only the GPIO_ACTIVE_HIGH and GPIO_ACTIVE_LOW flags are supported.
> +- gpio-controller : Marks the device node as a GPIO controller.
> +- reg : The id of the bank that the node describes.
I'd prefer to not have banks defined in DT. Do you have a variable
number or resources that are per bank? If not, then you don't need them.
> +
> +Example:
> + gpio@600 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + compatible = "mediatek,mt7621-gpio";
> + reg = <0x600 0x100>;
> +
> + interrupt-parent = <>;
> + interrupts = ;
> + interrupt-controller;
> + #interrupt-cells = <2>;
> +
> + gpio0: bank@0 {
> + reg = <0>;
> + compatible = "mediatek,mt7621-gpio-bank";
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + gpio1: bank@1 {
> + reg = <1>;
> + compatible = "mediatek,mt7621-gpio-bank";
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> +
> + gpio2: bank@2 {
> + reg = <2>;
> + compatible = "mediatek,mt7621-gpio-bank";
> + gpio-controller;
> + #gpio-cells = <2>;
> + };
> + };
> --
> 2.7.4
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: hwmon: Fix SPACING errors
On Tue, Jun 12, 2018 at 08:56:58PM +0200, Jakob Albert wrote: > Fix SPACING errors in drivers/hwmon/nct7904.c reported by checkpatch.pl > > Signed-off-by: Lorenz Kaestle > Signed-off-by: Jakob Albert > --- > drivers/hwmon/nct7904.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) Your subject is "odd", this is not a staging driver :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: lustre: Change return type to vm_fault_t
On Tue, Jun 12, 2018 at 2:37 AM, Greg KH wrote: > On Tue, Jun 12, 2018 at 02:30:27AM +0530, Souptick Joarder wrote: >> > > >> > > If no further comment, we would like to get this patch in 4.18-rc-X. >> > >> > Why? Is it a regression fix? That's all that is allowed after -rc1. >> >> No, this is not regression fix. We need to get this into 4.18-rc-1. But >> mostly it can't make into linus tree in rc-1 :) > > Why does it _have_ to get into 4.18-rc1? My tree is long-closed and > Linus already has all of my patches in his tree for the staging section > of the kernel. > >> > And have you tried applying it to Linus's current tree? :) >> >> Last tested on 4.17-rc-6 and it worked fine. Let me verify in current tree. > > Try it, you might be surprised :) Yes, got the surprise :) Sorry for making noise, I will drop this patch as it is no more valid in current Linus's tree. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4] staging: comedi: Improved readability of function comedi_nsamples_left.
Changes since v3:
a) Reverted u64 to unsigned long long and u32 to unsigned int.
b) Added patch versioning.
c) Changed type of scans_left to unsigned long long to avoid cast.
d) Clarified and updated changelog.
>8---8<
Improve readability of comedi_nsamples_left:
a) Reduce nesting by using more return statements.
b) Declare variables scans_left and samples_left at start of function.
c) Change type of scans_Left to unsigned long long to avoid cast.
Signed-off-by: Chris Opperman
---
drivers/staging/comedi/drivers.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 9d73347..57dd63d 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -473,21 +473,21 @@ unsigned int comedi_nsamples_left(struct comedi_subdevice
*s,
{
struct comedi_async *async = s->async;
struct comedi_cmd *cmd = >cmd;
+ unsigned long long scans_left;
+ unsigned long long samples_left;
- if (cmd->stop_src == TRIG_COUNT) {
- unsigned int scans_left = __comedi_nscans_left(s,
cmd->stop_arg);
- unsigned int scan_pos =
- comedi_bytes_to_samples(s, async->scan_progress);
- unsigned long long samples_left = 0;
-
- if (scans_left) {
- samples_left = ((unsigned long long)scans_left *
- cmd->scan_end_arg) - scan_pos;
- }
+ if (cmd->stop_src != TRIG_COUNT)
+ return nsamples;
- if (samples_left < nsamples)
- nsamples = samples_left;
- }
+ scans_left = __comedi_nscans_left(s, cmd->stop_arg);
+ if (!scans_left)
+ return 0;
+
+ samples_left = scans_left * cmd->scan_end_arg -
+ comedi_bytes_to_samples(s, async->scan_progress);
+
+ if (samples_left < nsamples)
+ return samples_left;
return nsamples;
}
EXPORT_SYMBOL_GPL(comedi_nsamples_left);
--
2.1.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: hwmon: Fix SPACING errors
Fix SPACING errors in drivers/hwmon/nct7904.c reported by checkpatch.pl
Signed-off-by: Lorenz Kaestle
Signed-off-by: Jakob Albert
---
drivers/hwmon/nct7904.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/hwmon/nct7904.c b/drivers/hwmon/nct7904.c
index 95a68ab..7de2421 100644
--- a/drivers/hwmon/nct7904.c
+++ b/drivers/hwmon/nct7904.c
@@ -159,7 +159,7 @@ static int nct7904_read_fan(struct device *dev, u32 attr,
int channel,
unsigned int cnt, rpm;
int ret;
- switch(attr) {
+ switch (attr) {
case hwmon_fan_input:
ret = nct7904_read_reg16(data, BANK_0,
FANIN1_HV_REG + channel * 2);
@@ -200,7 +200,7 @@ static int nct7904_read_in(struct device *dev, u32 attr,
int channel,
index = nct7904_chan_to_index[channel];
- switch(attr) {
+ switch (attr) {
case hwmon_in_input:
ret = nct7904_read_reg16(data, BANK_0,
VSEN1_HV_REG + index * 2);
@@ -236,7 +236,7 @@ static int nct7904_read_temp(struct device *dev, u32 attr,
int channel,
struct nct7904_data *data = dev_get_drvdata(dev);
int ret, temp;
- switch(attr) {
+ switch (attr) {
case hwmon_temp_input:
if (channel == 0)
ret = nct7904_read_reg16(data, BANK_0, LTD_HV_REG);
@@ -276,7 +276,7 @@ static int nct7904_read_pwm(struct device *dev, u32 attr,
int channel,
struct nct7904_data *data = dev_get_drvdata(dev);
int ret;
- switch(attr) {
+ switch (attr) {
case hwmon_pwm_input:
ret = nct7904_read_reg(data, BANK_3, FANCTL1_OUT_REG + channel);
if (ret < 0)
@@ -301,7 +301,7 @@ static int nct7904_write_pwm(struct device *dev, u32 attr,
int channel,
struct nct7904_data *data = dev_get_drvdata(dev);
int ret;
- switch(attr) {
+ switch (attr) {
case hwmon_pwm_input:
if (val < 0 || val > 255)
return -EINVAL;
@@ -322,7 +322,7 @@ static int nct7904_write_pwm(struct device *dev, u32 attr,
int channel,
static umode_t nct7904_pwm_is_visible(const void *_data, u32 attr, int channel)
{
- switch(attr) {
+ switch (attr) {
case hwmon_pwm_input:
case hwmon_pwm_enable:
return S_IRUGO | S_IWUSR;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: hwmon: Fix UNSPECIFIED_INT warning
Fix UNSPECIFIED_INT warning in drivers/hwmon/nct7904.c reported by
checkpatch.pl
Signed-off-by: Lorenz Kaestle
Signed-off-by: Jakob Albert
---
drivers/hwmon/nct7904.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/nct7904.c b/drivers/hwmon/nct7904.c
index 657ce88..7815ddf 100644
--- a/drivers/hwmon/nct7904.c
+++ b/drivers/hwmon/nct7904.c
@@ -77,7 +77,7 @@ struct nct7904_data {
};
/* Access functions */
-static int nct7904_bank_lock(struct nct7904_data *data, unsigned bank)
+static int nct7904_bank_lock(struct nct7904_data *data, unsigned int bank)
{
int ret;
@@ -99,7 +99,7 @@ static inline void nct7904_bank_release(struct nct7904_data
*data)
/* Read 1-byte register. Returns unsigned reg or -ERRNO on error. */
static int nct7904_read_reg(struct nct7904_data *data,
- unsigned bank, unsigned reg)
+ unsigned int bank, unsigned int reg)
{
struct i2c_client *client = data->client;
int ret;
@@ -117,7 +117,7 @@ static int nct7904_read_reg(struct nct7904_data *data,
* -ERRNO on error.
*/
static int nct7904_read_reg16(struct nct7904_data *data,
- unsigned bank, unsigned reg)
+ unsigned int bank, unsigned int reg)
{
struct i2c_client *client = data->client;
int ret, hi;
@@ -139,7 +139,7 @@ static int nct7904_read_reg16(struct nct7904_data *data,
/* Write 1-byte register. Returns 0 or -ERRNO on error. */
static int nct7904_write_reg(struct nct7904_data *data,
-unsigned bank, unsigned reg, u8 val)
+unsigned int bank, unsigned int reg, u8 val)
{
struct i2c_client *client = data->client;
int ret;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] staging: hwmon: Fix style issues
This set of patches fixes style errors in drivers/hwmon/nct7904.c reported by checkpatch.pl. This adapts the code to the coding style. Jakob Albert (3): staging: hwmon: Fix SPACING errors staging: hwmon: Fix CODE_INDENT error staging: hwmon: Fix UNSPECIFIED_INT warning drivers/hwmon/nct7904.c | 68 - 1 file changed, 34 insertions(+), 34 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: hwmon: Fix CODE_INDENT error
Fix CODE_INDENT error in drivers/hwmon/nct7904.c reported by checkpatch.pl
Signed-off-by: Lorenz Kaestle
Signed-off-by: Jakob Albert
---
drivers/hwmon/nct7904.c | 48
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/hwmon/nct7904.c b/drivers/hwmon/nct7904.c
index 7de2421..657ce88 100644
--- a/drivers/hwmon/nct7904.c
+++ b/drivers/hwmon/nct7904.c
@@ -431,15 +431,15 @@ static const struct hwmon_channel_info nct7904_in = {
};
static const u32 nct7904_fan_config[] = {
-HWMON_F_INPUT,
-HWMON_F_INPUT,
-HWMON_F_INPUT,
-HWMON_F_INPUT,
-HWMON_F_INPUT,
-HWMON_F_INPUT,
-HWMON_F_INPUT,
-HWMON_F_INPUT,
- 0
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ HWMON_F_INPUT,
+ 0
};
static const struct hwmon_channel_info nct7904_fan = {
@@ -448,11 +448,11 @@ static const struct hwmon_channel_info nct7904_fan = {
};
static const u32 nct7904_pwm_config[] = {
-HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
-HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
-HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
-HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
- 0
+ HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
+ HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
+ HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
+ HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
+ 0
};
static const struct hwmon_channel_info nct7904_pwm = {
@@ -461,16 +461,16 @@ static const struct hwmon_channel_info nct7904_pwm = {
};
static const u32 nct7904_temp_config[] = {
-HWMON_T_INPUT,
-HWMON_T_INPUT,
-HWMON_T_INPUT,
-HWMON_T_INPUT,
-HWMON_T_INPUT,
-HWMON_T_INPUT,
-HWMON_T_INPUT,
-HWMON_T_INPUT,
-HWMON_T_INPUT,
- 0
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ HWMON_T_INPUT,
+ 0
};
static const struct hwmon_channel_info nct7904_temp = {
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] Staging:rtl8192e Replace function names by using __func__ identifier
On Fri, Jun 8, 2018 at 7:32 AM, Janani Sankara Babu wrote:
> This patch is created to solve the warning shown by checkpatch script
> Prefer using '"%s...", __func__' to using ', this function's name,
> in a string
Have you even tried to compile it?
NAK
>
> Signed-off-by: Janani Sankara Babu
> ---
> drivers/staging/rtl8192e/rtl819x_BAProc.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl819x_BAProc.c
> b/drivers/staging/rtl8192e/rtl819x_BAProc.c
> index c466a5e7..3c7ba33 100644
> --- a/drivers/staging/rtl8192e/rtl819x_BAProc.c
> +++ b/drivers/staging/rtl8192e/rtl819x_BAProc.c
> @@ -127,7 +127,7 @@ static struct sk_buff *rtllib_ADDBA(struct rtllib_device
> *ieee, u8 *Dst,
> }
>
> #ifdef VERBOSE_DEBUG
> - print_hex_dump_bytes("rtllib_ADDBA(): ", DUMP_PREFIX_NONE, skb->data,
> + print_hex_dump_bytes("%s(): ", __func__, DUMP_PREFIX_NONE, skb->data,
> skb->len);
> #endif
> return skb;
> @@ -178,7 +178,7 @@ static struct sk_buff *rtllib_DELBA(struct rtllib_device
> *ieee, u8 *dst,
> tag += 2;
>
> #ifdef VERBOSE_DEBUG
> - print_hex_dump_bytes("rtllib_DELBA(): ", DUMP_PREFIX_NONE, skb->data,
> + print_hex_dump_bytes("%s(): ", __func__, DUMP_PREFIX_NONE, skb->data,
> skb->len);
> #endif
> return skb;
> @@ -243,7 +243,7 @@ int rtllib_rx_ADDBAReq(struct rtllib_device *ieee, struct
> sk_buff *skb)
> }
>
> #ifdef VERBOSE_DEBUG
> - print_hex_dump_bytes("rtllib_rx_ADDBAReq(): ", DUMP_PREFIX_NONE,
> + print_hex_dump_bytes("%s(): ", __func__, DUMP_PREFIX_NONE,
> skb->data, skb->len);
> #endif
>
> @@ -441,7 +441,7 @@ int rtllib_rx_DELBA(struct rtllib_device *ieee, struct
> sk_buff *skb)
> }
>
> #ifdef VERBOSE_DEBUG
> - print_hex_dump_bytes("rtllib_rx_DELBA(): ", DUMP_PREFIX_NONE,
> skb->data,
> + print_hex_dump_bytes("%s():", __func__, DUMP_PREFIX_NONE, skb->data,
> skb->len);
> #endif
> delba = (struct rtllib_hdr_3addr *)skb->data;
> --
> 1.9.1
>
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: Improved readability of function comedi_nsamples_left.
Hi Dan, Thank you for the feedback, I'll update V4 of the patch accordingly and resend. I'll soon get a hang of the workflow! :) Best Regards, Chris Opperman ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging:iio:accel:adis16203: sign extend function rather code duplication
Use sign_extend32 kernel function instead of code duplication.
This function is also safe for 16 bits.
Signed-off-by: Karim Eshapa
---
drivers/staging/iio/accel/adis16203.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16203.c
b/drivers/staging/iio/accel/adis16203.c
index b3e4571340ab..5cc96c8086b5 100644
--- a/drivers/staging/iio/accel/adis16203.c
+++ b/drivers/staging/iio/accel/adis16203.c
@@ -168,7 +168,6 @@ static int adis16203_read_raw(struct iio_dev *indio_dev,
{
struct adis *st = iio_priv(indio_dev);
int ret;
- int bits;
u8 addr;
s16 val16;
@@ -202,14 +201,11 @@ static int adis16203_read_raw(struct iio_dev *indio_dev,
*val = 25000 / -470 - 1278; /* 25 C = 1278 */
return IIO_VAL_INT;
case IIO_CHAN_INFO_CALIBBIAS:
- bits = 14;
addr = adis16203_addresses[chan->scan_index];
ret = adis_read_reg_16(st, addr, );
if (ret)
return ret;
- val16 &= (1 << bits) - 1;
- val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
- *val = val16;
+ *val = sign_extend32(val16, 13);
return IIO_VAL_INT;
default:
return -EINVAL;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging:iio:accel:adis16240: sign extend function avoiding code duplication
Use sign_extend32 kernel function instead of code duplication,
Safe also for 16 bit. and remove declaration of bits variable not needed.
Signed-off-by: Karim Eshapa
>On Tue, 2018-06-12 at 01:38 +0200, Karim Eshapa wrote:
>> Use sign_extend32 kernel function instead of code duplication.
>> Safe also for 16 bit.
>
>Perhaps remove the bits declaration and assignments
>and just use 9 directly.
>
>> diff --git a/drivers/staging/iio/accel/adis16240.c
>> b/drivers/staging/iio/accel/adis16240.c
>
>> @@ -292,9 +292,7 @@ static int adis16240_read_raw(struct iio_dev *indio_dev,
>> ret = adis_read_reg_16(st, addr, );
>> if (ret)
>> return ret;
>> - val16 &= (1 << bits) - 1;
>> - val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
>> - *val = val16;
>> + *val = sign_extend32(val16, bits - 1);
>> return IIO_VAL_INT;
>> case IIO_CHAN_INFO_PEAK:
>> bits = 10;
>> @@ -302,9 +300,7 @@ static int adis16240_read_raw(struct iio_dev *indio_dev,
>> ret = adis_read_reg_16(st, addr, );
>> if (ret)
>> return ret;
>> - val16 &= (1 << bits) - 1;
>> - val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
>> - *val = val16;
>> + *val = sign_extend32(val16, bits - 1);
>> return IIO_VAL_INT;
>> }
>> return -EINVAL;
---
drivers/staging/iio/accel/adis16240.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/iio/accel/adis16240.c
b/drivers/staging/iio/accel/adis16240.c
index fff6d99089cc..24e525f1ef25 100644
--- a/drivers/staging/iio/accel/adis16240.c
+++ b/drivers/staging/iio/accel/adis16240.c
@@ -250,7 +250,6 @@ static int adis16240_read_raw(struct iio_dev *indio_dev,
{
struct adis *st = iio_priv(indio_dev);
int ret;
- int bits;
u8 addr;
s16 val16;
@@ -287,24 +286,18 @@ static int adis16240_read_raw(struct iio_dev *indio_dev,
*val = 25000 / 244 - 0x133; /* 25 C = 0x133 */
return IIO_VAL_INT;
case IIO_CHAN_INFO_CALIBBIAS:
- bits = 10;
addr = adis16240_addresses[chan->scan_index][0];
ret = adis_read_reg_16(st, addr, );
if (ret)
return ret;
- val16 &= (1 << bits) - 1;
- val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
- *val = val16;
+ *val = sign_extend32(val16, 9);
return IIO_VAL_INT;
case IIO_CHAN_INFO_PEAK:
- bits = 10;
addr = adis16240_addresses[chan->scan_index][1];
ret = adis_read_reg_16(st, addr, );
if (ret)
return ret;
- val16 &= (1 << bits) - 1;
- val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
- *val = val16;
+ *val = sign_extend32(val16, 9);
return IIO_VAL_INT;
}
return -EINVAL;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 2/4] resource: Use list_head to link sibling resource
This looks wrong. After a list iterator, the index variable points to a
dummy structure.
julia
url:
https://github.com/0day-ci/linux/commits/Baoquan-He/resource-Use-list_head-to-link-sibling-resource/20180612-113600
:: branch date: 7 hours ago
:: commit date: 7 hours ago
>> kernel/resource.c:265:17-20: ERROR: invalid reference to the index variable
>> of the iterator on line 253
#
https://github.com/0day-ci/linux/commit/e906f15906750a86913ba2b1f08bad99129d3dfc
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout e906f15906750a86913ba2b1f08bad99129d3dfc
vim +265 kernel/resource.c
^1da177e4 Linus Torvalds 2005-04-16 247
5eeec0ec9 Yinghai Lu 2009-12-22 248 static void
__release_child_resources(struct resource *r)
5eeec0ec9 Yinghai Lu 2009-12-22 249 {
e906f1590 Baoquan He 2018-06-12 250struct resource *tmp, *next;
5eeec0ec9 Yinghai Lu 2009-12-22 251resource_size_t size;
5eeec0ec9 Yinghai Lu 2009-12-22 252
e906f1590 Baoquan He 2018-06-12 @253list_for_each_entry_safe(tmp,
next, >child, sibling) {
5eeec0ec9 Yinghai Lu 2009-12-22 254tmp->parent = NULL;
e906f1590 Baoquan He 2018-06-12 255
INIT_LIST_HEAD(>sibling);
5eeec0ec9 Yinghai Lu 2009-12-22 256
__release_child_resources(tmp);
5eeec0ec9 Yinghai Lu 2009-12-22 257
5eeec0ec9 Yinghai Lu 2009-12-22 258printk(KERN_DEBUG
"release child resource %pR\n", tmp);
5eeec0ec9 Yinghai Lu 2009-12-22 259/* need to restore
size, and keep flags */
5eeec0ec9 Yinghai Lu 2009-12-22 260size =
resource_size(tmp);
5eeec0ec9 Yinghai Lu 2009-12-22 261tmp->start = 0;
5eeec0ec9 Yinghai Lu 2009-12-22 262tmp->end = size - 1;
5eeec0ec9 Yinghai Lu 2009-12-22 263}
e906f1590 Baoquan He 2018-06-12 264
e906f1590 Baoquan He 2018-06-12 @265INIT_LIST_HEAD(>child);
5eeec0ec9 Yinghai Lu 2009-12-22 266 }
5eeec0ec9 Yinghai Lu 2009-12-22 267
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net 2/3] hv_netvsc: fix network namespace issues with VF support
On Tue, 12 Jun 2018 12:51:28 +0300
Dan Carpenter wrote:
> On Mon, Jun 11, 2018 at 12:44:55PM -0700, Stephen Hemminger wrote:
> > When finding the parent netvsc device, the search needs to be across
> > all netvsc device instances (independent of network namespace).
> >
> > Find parent device of VF using upper_dev_get routine which
> > searches only adjacent list.
> >
> > Fixes: e8ff40d4bff1 ("hv_netvsc: improve VF device matching")
> > Signed-off-by: Stephen Hemminger
> >
> > netns aware byref
>
> What? Presumably that wasn't supposed to be part of the commit message.
That was leftover from earlier commit message
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
On Tue, Jun 12, 2018 at 5:20 PM, Andy Shevchenko
wrote:
> On Tue, Jun 12, 2018 at 12:38 PM, Baoquan He wrote:
>> On 06/12/18 at 11:29am, Andy Shevchenko wrote:
>>> On Tue, Jun 12, 2018 at 6:28 AM, Baoquan He wrote:
>
>>> > +{
>>>
>>> > + for (pp = >child; (p = *pp) != NULL; pp = >sibling) {
>>> > + if (p->end < res->start)
>>> > + continue;
>>> > + if (res->end < p->start)
>>> > + break;
>>>
>>> > + if (p->start < res->start || p->end > res->end)
>>> > + return -1; /* not completely contained */
>>>
>>> Usually we are expecting real eeror codes.
>>
>> Hmm, I just copied it from arch/powerpc/kernel/pci-common.c. The
>> function interface expects an integer returned value, not sure what a
>> real error codes look like, could you give more hints? Will change
>> accordingly.
>
> I briefly looked at the code and error codes we have, so, my proposal
> is one of the following
> - use -ECANCELED (not the best choice for first occurrence here,
> though I can't find better)
Actually -ENOTSUPP might suit the first case (although the actual
would be something like -EOVERLAP, which we don't have)
> - use positive integers (or enum), like
> #define RES_REPARENTED 0
> #define RES_OVERLAPPED 1
> #define RES_NOCONFLICT 2
>
>
>>> > + if (firstpp == NULL)
>>> > + firstpp = pp;
>>> > + }
>>>
>>> > + if (firstpp == NULL)
>>> > + return -1; /* didn't find any conflicting entries? */
>>>
>>> Ditto.
>
> Ditto.
>
>>>
>>> > +}
>>> > +EXPORT_SYMBOL(reparent_resources);
>
> --
> With Best Regards,
> Andy Shevchenko
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
On Tue, Jun 12, 2018 at 12:38 PM, Baoquan He wrote:
> On 06/12/18 at 11:29am, Andy Shevchenko wrote:
>> On Tue, Jun 12, 2018 at 6:28 AM, Baoquan He wrote:
>> > +{
>>
>> > + for (pp = >child; (p = *pp) != NULL; pp = >sibling) {
>> > + if (p->end < res->start)
>> > + continue;
>> > + if (res->end < p->start)
>> > + break;
>>
>> > + if (p->start < res->start || p->end > res->end)
>> > + return -1; /* not completely contained */
>>
>> Usually we are expecting real eeror codes.
>
> Hmm, I just copied it from arch/powerpc/kernel/pci-common.c. The
> function interface expects an integer returned value, not sure what a
> real error codes look like, could you give more hints? Will change
> accordingly.
I briefly looked at the code and error codes we have, so, my proposal
is one of the following
- use -ECANCELED (not the best choice for first occurrence here,
though I can't find better)
- use positive integers (or enum), like
#define RES_REPARENTED 0
#define RES_OVERLAPPED 1
#define RES_NOCONFLICT 2
>> > + if (firstpp == NULL)
>> > + firstpp = pp;
>> > + }
>>
>> > + if (firstpp == NULL)
>> > + return -1; /* didn't find any conflicting entries? */
>>
>> Ditto.
Ditto.
>>
>> > +}
>> > +EXPORT_SYMBOL(reparent_resources);
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Fix type of argument in ieee80211_is_action()
On Sun, Jun 10, 2018 at 12:40:17PM +0530, Nishad Kamdar wrote: > Type used is unsigned char but expected is restricted __le16. > Warning reported by sparse. Part of eudyptula challenge. > The code is buggy, but this fix just papers over it. We discussed this code earlier. https://lkml.org/lkml/2018/6/5/304 regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rts5208: add check on NULL before dereference
On Sat, Jun 09, 2018 at 10:34:43PM +0300, Andy Shevchenko wrote:
> On Sat, Jun 9, 2018 at 7:58 PM, wrote:
> > On 2018-06-09 12:38, Anton Vasilyev wrote:
> >>
> >> If rtsx_probe fails to allocate dev->chip, then NULL pointer
> >> dereference occurs at rtsx_release_resources().
> >>
> >> Patch adds checks chip on NULL before its dereference at
> >> rtsx_release_resources and passing with dereference inside
> >> rtsx_release_chip.
> >>
> >> Found by Linux Driver Verification project (linuxtesting.org).
>
> > I think you should bail out if dev->chip is null rather than adding
> > conditiinals.
>
> I'm wondering if it's false positive. At which circumstances that may happen?
>
Here's how the code looks like in rtsx_probe().
972 /* We come here if there are any problems */
973 errout:
974 dev_err(>dev, "%s failed\n", __func__);
975 release_everything(dev);
976
977 return err;
978 }
Do everything error handling is error prone because you're trying to
free some things which haven't been allocated. It's also more
complicated so it leads to leaks.
The correct way to do error handling is to have a series of labels which
undo one thing at a time. The labels should be named properly so that
you can tell what the goto does such as "goto err_release_foo;". That
way you never have to worry about freeing things which haven't been
allocated. As you read the code, you just have to track the most
recently allocated resource and verify that the goto does what is
expected so you avoid leaks.
In this example:
857 dev->chip = kzalloc(sizeof(*dev->chip), GFP_KERNEL);
858 if (!dev->chip) {
859 err = -ENOMEM;
860 goto errout;
861 }
862
863 spin_lock_init(>reg_lock);
864 mutex_init(>dev_mutex);
865 init_completion(>cmnd_ready);
866 init_completion(>control_exit);
867 init_completion(>polling_exit);
868 init_completion(>notify);
869 init_completion(>scanning_done);
870 init_waitqueue_head(>delay_wait);
If the kzalloc() fails, then we call release_everything() with
"dev->chip" NULL. But we'll actually crash before we hit the NULL
dereference because we didn't do the init_completion(>cmnd_ready);
So this patch doesn't really fix anything because do everything error
handling is a hopeless approach.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: pi433: add mutex fixing concurrency issues.
On Sat, Jun 09, 2018 at 11:48:42AM -0400, Hugo Lefeuvre wrote: > case PI433_IOC_WR_TX_CFG: > if (copy_from_user(>tx_cfg, argp, >sizeof(struct pi433_tx_cfg))) > return -EFAULT; > break; Btw, it looks so wrong to me that we copy partial data to >tx_cfg... I'd really prefer copying it to a tmp buffer and then verifying it's corrent then memcpy()ing it to >tx_cfg. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gdm724x: add error handling for nlmsg_put
On Mon, Jun 11, 2018 at 01:09:24PM +0800, Zhouyang Jia wrote: > When nlmsg_put fails, the lack of error-handling code may > cause unexpected results. > > This patch adds error-handling code after calling nlmsg_put. > > Signed-off-by: Zhouyang Jia > --- > drivers/staging/gdm724x/netlink_k.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/staging/gdm724x/netlink_k.c > b/drivers/staging/gdm724x/netlink_k.c > index abe2425..4623140 100644 > --- a/drivers/staging/gdm724x/netlink_k.c > +++ b/drivers/staging/gdm724x/netlink_k.c > @@ -119,6 +119,9 @@ int netlink_send(struct sock *sock, int group, u16 type, > void *msg, int len) > seq++; > > nlh = nlmsg_put(skb, 0, seq, type, len, 0); > + if (!nlh) > + return -EMSGSIZE; This can't fail. We just allocated skb on the line before and we allocated enough space for "len". Also if we *did* hit a bug here then we would need to do some more cleanup instead of a direct return. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: add error handling for vmap
On Tue, Jun 12, 2018 at 11:25:35AM +0800, Zhouyang Jia wrote: > When vmap fails, the lack of error-handling code may > cause unexpected results. > > This patch adds error-handling code after calling vmap. > Again, this is not error handling, this is just an error message. This error condition is handled in the caller. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8712: add error handling for register_netdev
On Tue, Jun 12, 2018 at 11:18:43AM +0800, Zhouyang Jia wrote: > When register_netdev fails, the lack of error-handling code may > cause unexpected results. > > This patch adds error-handling code after calling register_netdev. > It doesn't add any error handling. It just adds an error message. The error handling does look sort of messed up, but this isn't a useful patch. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 6/8] staging: mt7621-gpio: avoid one level indentation in interrupt handler
There is no need to check for 'pending' before loop over the
interrupts using 'for_each_set_bit' if nothing is set the
return values will be the same so just avoid this check avoiding
also one level intentation and improving readability.
Signed-off-by: Sergio Paracuellos
---
drivers/staging/mt7621-gpio/gpio-mt7621.c | 12 +---
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 96dee10..698a95d 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -93,14 +93,12 @@ mediatek_gpio_irq_handler(int irq, void *data)
pending = mtk_gpio_r32(rg, GPIO_REG_STAT);
- if (pending) {
- for_each_set_bit(bit, , MTK_BANK_WIDTH) {
- u32 map = irq_find_mapping(gc->irq.domain, bit);
+ for_each_set_bit(bit, , MTK_BANK_WIDTH) {
+ u32 map = irq_find_mapping(gc->irq.domain, bit);
- generic_handle_irq(map);
- mtk_gpio_w32(rg, GPIO_REG_STAT, BIT(bit));
- ret |= IRQ_HANDLED;
- }
+ generic_handle_irq(map);
+ mtk_gpio_w32(rg, GPIO_REG_STAT, BIT(bit));
+ ret |= IRQ_HANDLED;
}
return ret;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 8/8] staging: mt7621-gpio: avoid long line in a comment
Checkpatch script is complaining about a comment line
which exceeds 80 characteres. Just silence it.
Signed-off-by: Sergio Paracuellos
---
drivers/staging/mt7621-gpio/gpio-mt7621.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 63fb5a1..9a4a12f 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -246,8 +246,9 @@ mediatek_gpio_bank_probe(struct platform_device *pdev,
struct device_node *bank)
if (gpio->gpio_irq) {
/*
-* Manually request the irq here instead of passing a
flow-handler
-* to gpiochip_set_chained_irqchip, because the irq is shared.
+* Manually request the irq here instead of passing
+* a flow-handler to gpiochip_set_chained_irqchip,
+* because the irq is shared.
*/
ret = devm_request_irq(>dev, gpio->gpio_irq,
mediatek_gpio_irq_handler, IRQF_SHARED,
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 5/8] staging: mt7621-gpio: avoid to set up irqs if not defined in dts
If there is no interrupt defined in the dts 'irq_of_parse_and_map'
returns 0 and we should't set up interrupts for each gpio chip in
that case.
Signed-off-by: Sergio Paracuellos
---
drivers/staging/mt7621-gpio/gpio-mt7621.c | 42 ---
1 file changed, 22 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 1318003..96dee10 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -237,30 +237,32 @@ mediatek_gpio_bank_probe(struct platform_device *pdev,
struct device_node *bank)
return ret;
}
- /*
-* Manually request the irq here instead of passing a flow-handler
-* to gpiochip_set_chained_irqchip, because the irq is shared.
-*/
- ret = devm_request_irq(>dev, gpio->gpio_irq,
- mediatek_gpio_irq_handler, IRQF_SHARED,
- "mt7621", >chip);
+ if (gpio->gpio_irq) {
+ /*
+* Manually request the irq here instead of passing a
flow-handler
+* to gpiochip_set_chained_irqchip, because the irq is shared.
+*/
+ ret = devm_request_irq(>dev, gpio->gpio_irq,
+ mediatek_gpio_irq_handler, IRQF_SHARED,
+ "mt7621", >chip);
+
+ if (ret) {
+ dev_err(>dev, "Error requesting IRQ %d: %d\n",
+ gpio->gpio_irq, ret);
+ return ret;
+ }
- if (ret) {
- dev_err(>dev, "Error requesting IRQ %d: %d\n",
- gpio->gpio_irq, ret);
- return ret;
- }
+ ret = gpiochip_irqchip_add(>chip, _gpio_irq_chip,
+ 0, handle_simple_irq, IRQ_TYPE_NONE);
+ if (ret) {
+ dev_err(>dev, "failed to add gpiochip_irqchip\n");
+ return ret;
+ }
- ret = gpiochip_irqchip_add(>chip, _gpio_irq_chip,
- 0, handle_simple_irq, IRQ_TYPE_NONE);
- if (ret) {
- dev_err(>dev, "failed to add gpiochip_irqchip\n");
- return ret;
+ gpiochip_set_chained_irqchip(>chip, _gpio_irq_chip,
+gpio->gpio_irq, NULL);
}
- gpiochip_set_chained_irqchip(>chip, _gpio_irq_chip,
-gpio->gpio_irq, NULL);
-
/* set polarity to low for all gpios */
mtk_gpio_w32(rg, GPIO_REG_POL, 0);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 7/8] staging: mt7621-gpio: set different names for each gpio_chip and irq_chip
Currently the driver defines 3 gpiochips, one for each bank.
/sys/class/gpio/gpiochip416/label:1e000600.gpio
/sys/class/gpio/gpiochip448/label:1e000600.gpio
/sys/class/gpio/gpiochip480/label:1e000600.gpio
Unfortunately they all have the same label
Interrupts from /proc/interrupt show the same name which is
confusing:
/proc/interrupts:
17: 0 0 0 0 MIPS GIC 19 mt7621, mt7621, mt7621
which is the interrupt from the GPIO controller.
It is a little weird that all three banks are named "mt7621"
here. We also have:
26: 0 0 0 0 GPIO 18 reset
which is the interrupt from GPIO which provides the "reset"
button. I suspect that if I had interrupts form two different
banks they would both be called "GPIO" which would be a little
confusing.
In order to unify all of this set different names for each chip
Use a 'bank-based' name instead the same for all: 'mt7621-bank[0-2]'.
Create a new 'mediatek_gpio_bank_name' function which return the
name depending on the bank number. This function is allways called
with a valid index 0, 1 or 2.
Signed-off-by: Sergio Paracuellos
---
drivers/staging/mt7621-gpio/gpio-mt7621.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 698a95d..63fb5a1 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -190,13 +190,21 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int
type)
}
static struct irq_chip mediatek_gpio_irq_chip = {
- .name = "GPIO",
.irq_unmask = mediatek_gpio_irq_unmask,
.irq_mask = mediatek_gpio_irq_mask,
.irq_mask_ack = mediatek_gpio_irq_mask,
.irq_set_type = mediatek_gpio_irq_type,
};
+static inline const char * const mediatek_gpio_bank_name(int bank)
+{
+ static const char * const bank_names[] = {
+ "mt7621-bank0", "mt7621-bank1", "mt7621-bank2",
+ };
+
+ return bank_names[bank];
+}
+
static int
mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node
*bank)
{
@@ -215,6 +223,7 @@ mediatek_gpio_bank_probe(struct platform_device *pdev,
struct device_node *bank)
spin_lock_init(>lock);
rg->chip.of_node = bank;
rg->bank = be32_to_cpu(*id);
+ rg->chip.label = mediatek_gpio_bank_name(rg->bank);
dat = gpio->gpio_membase + GPIO_REG_DATA + (rg->bank * GPIO_BANK_WIDE);
set = gpio->gpio_membase + GPIO_REG_DSET + (rg->bank * GPIO_BANK_WIDE);
@@ -242,7 +251,7 @@ mediatek_gpio_bank_probe(struct platform_device *pdev,
struct device_node *bank)
*/
ret = devm_request_irq(>dev, gpio->gpio_irq,
mediatek_gpio_irq_handler, IRQF_SHARED,
- "mt7621", >chip);
+ rg->chip.label, >chip);
if (ret) {
dev_err(>dev, "Error requesting IRQ %d: %d\n",
@@ -250,6 +259,7 @@ mediatek_gpio_bank_probe(struct platform_device *pdev,
struct device_node *bank)
return ret;
}
+ mediatek_gpio_irq_chip.name = rg->chip.label;
ret = gpiochip_irqchip_add(>chip, _gpio_irq_chip,
0, handle_simple_irq, IRQ_TYPE_NONE);
if (ret) {
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/8] staging: mt7621-gpio: avoid check for NULL in 'to_mediatek_gpio' calls
Function 'to_mediatek_gpio' cannot return NULL, so this NULL
checkings are pointless.
Signed-off-by: Sergio Paracuellos
---
drivers/staging/mt7621-gpio/gpio-mt7621.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 853817a..1318003 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -115,9 +115,6 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
unsigned long flags;
u32 rise, fall, high, low;
- if (!rg)
- return;
-
spin_lock_irqsave(>lock, flags);
rise = mtk_gpio_r32(rg, GPIO_REG_REDGE);
fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE);
@@ -139,9 +136,6 @@ mediatek_gpio_irq_mask(struct irq_data *d)
unsigned long flags;
u32 rise, fall, high, low;
- if (!rg)
- return;
-
spin_lock_irqsave(>lock, flags);
rise = mtk_gpio_r32(rg, GPIO_REG_REDGE);
fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE);
@@ -162,9 +156,6 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int
type)
int pin = d->hwirq;
u32 mask = BIT(pin);
- if (!rg)
- return -1;
-
if (type == IRQ_TYPE_PROBE) {
if ((rg->rising | rg->falling |
rg->hlevel | rg->llevel) & mask)
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/8] staging: mt7621-gpio: remove no more necessary PIN_MASK macro
PIN_MASK macro was being used because of the fact we were only using one interrupt controller for all of the gpio chips. This has been changed to use one per gpio chip and each has 32 irqs. Because of this this macro is not needed anymore. Use BIT macro instead. Signed-off-by: Sergio Paracuellos --- drivers/staging/mt7621-gpio/gpio-mt7621.c | 19 +-- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c index d2a7512..a8893e8 100644 --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c @@ -15,7 +15,6 @@ #define MTK_BANK_CNT 3 #define MTK_BANK_WIDTH 32 -#define PIN_MASK(nr) (1UL << ((nr % MTK_BANK_WIDTH))) #define GPIO_BANK_WIDE 0x04 #define GPIO_REG_CTRL 0x00 @@ -133,10 +132,10 @@ mediatek_gpio_irq_unmask(struct irq_data *d) fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE); high = mtk_gpio_r32(rg, GPIO_REG_HLVL); low = mtk_gpio_r32(rg, GPIO_REG_LLVL); - mtk_gpio_w32(rg, GPIO_REG_REDGE, rise | (PIN_MASK(pin) & rg->rising)); - mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall | (PIN_MASK(pin) & rg->falling)); - mtk_gpio_w32(rg, GPIO_REG_HLVL, high | (PIN_MASK(pin) & rg->hlevel)); - mtk_gpio_w32(rg, GPIO_REG_LLVL, low | (PIN_MASK(pin) & rg->llevel)); + mtk_gpio_w32(rg, GPIO_REG_REDGE, rise | (BIT(pin) & rg->rising)); + mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall | (BIT(pin) & rg->falling)); + mtk_gpio_w32(rg, GPIO_REG_HLVL, high | (BIT(pin) & rg->hlevel)); + mtk_gpio_w32(rg, GPIO_REG_LLVL, low | (BIT(pin) & rg->llevel)); spin_unlock_irqrestore(>lock, flags); } @@ -157,10 +156,10 @@ mediatek_gpio_irq_mask(struct irq_data *d) fall = mtk_gpio_r32(rg, GPIO_REG_FEDGE); high = mtk_gpio_r32(rg, GPIO_REG_HLVL); low = mtk_gpio_r32(rg, GPIO_REG_LLVL); - mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall & ~PIN_MASK(pin)); - mtk_gpio_w32(rg, GPIO_REG_REDGE, rise & ~PIN_MASK(pin)); - mtk_gpio_w32(rg, GPIO_REG_HLVL, high & ~PIN_MASK(pin)); - mtk_gpio_w32(rg, GPIO_REG_LLVL, low & ~PIN_MASK(pin)); + mtk_gpio_w32(rg, GPIO_REG_FEDGE, fall & ~BIT(pin)); + mtk_gpio_w32(rg, GPIO_REG_REDGE, rise & ~BIT(pin)); + mtk_gpio_w32(rg, GPIO_REG_HLVL, high & ~BIT(pin)); + mtk_gpio_w32(rg, GPIO_REG_LLVL, low & ~BIT(pin)); spin_unlock_irqrestore(>lock, flags); } @@ -170,7 +169,7 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int type) struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct mtk_gc *rg = to_mediatek_gpio(gc); int pin = d->hwirq; - u32 mask = PIN_MASK(pin); + u32 mask = BIT(pin); if (!rg) return -1; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/8] staging: mt7621-gpio: update kerneldoc for state containers
Update kernel doc for mtk_data and also remove no needed
documentation for mtk_gc which is clear enough to don't
need it.
Signed-off-by: Sergio Paracuellos
---
drivers/staging/mt7621-gpio/gpio-mt7621.c | 15 +++
1 file changed, 3 insertions(+), 12 deletions(-)
diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index a8893e8..5f7d524c 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -29,16 +29,6 @@
#define GPIO_REG_STAT 0x90
#define GPIO_REG_EDGE 0xA0
-/**
- * struct mtk_gc - data for a single gpio-chip
- * @chip: gpio chip instance
- * @lock: spinlock to protect the chip
- * @bank: gpio bank number for the chip
- * @rising: mask for rising irqs
- * @falling: mask for falling irqs
- * @hlevel: mask for high level irqs
- * @llevel: mask for low level irqs
- */
struct mtk_gc {
struct gpio_chip chip;
spinlock_t lock;
@@ -51,8 +41,9 @@ struct mtk_gc {
/**
* struct mtk_data - state container for
- * data of the platform driver. It is a
- * single irq-chip but 3 separate gpio-chip.
+ * data of the platform driver. It is 3
+ * separate gpio-chip each one with its
+ * own irq_chip.
* @dev: device instance
* @gpio_membase: memory base address
* @gpio_irq: irq number from the device tree
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/8] staging: mt7621-gpio: align indentation for all defines
There was two remaining defines which weren't properly aligned with the rest. Align them improving readability. Signed-off-by: Sergio Paracuellos --- drivers/staging/mt7621-gpio/gpio-mt7621.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c b/drivers/staging/mt7621-gpio/gpio-mt7621.c index 5f7d524c..853817a 100644 --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c @@ -13,8 +13,8 @@ #include #include -#define MTK_BANK_CNT 3 -#define MTK_BANK_WIDTH 32 +#define MTK_BANK_CNT 3 +#define MTK_BANK_WIDTH 32 #define GPIO_BANK_WIDE 0x04 #define GPIO_REG_CTRL 0x00 -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/7] staging: mt7621-gpio: last minor cleanups
Now that all seems to be working these are the last minor cleanups in order to make a second try to get this out of staging. Sergio Paracuellos (7): staging: mt7621-gpio: remove no more necessary PIN_MASK macro staging: mt7621-gpio: update kerneldoc for state containers staging: mtk7621-gpio: align indentation for all defines staging: mt7621-gpio: avoid check for NULL in 'to_mediatek_gpio' calls staging: mt7621-gpio: avoid to set up irqs if not defined in dts staging: mt7621-gpio: avoid one level indentation in interrupt handler staging: mt7621-gpio: set different names for each gpio_chip and irq_chip drivers/staging/mt7621-gpio/gpio-mt7621.c | 113 ++ 1 file changed, 52 insertions(+), 61 deletions(-) -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:iio:accel:adis16240: sign extend function avoiding code duplication
On Tue, 12 Jun 2018 02:57:48 +0200
Karim Eshapa wrote:
> Use sign_extend32 kernel function instead of code duplication.
> Safe also for 16 bit.
>
> Signed-off-by: Karim Eshapa
Please resend as a fresh patch marked at V2 with a change log below the ---
so we can easily see what has changed.
Thanks,
Jonathan
> ---
> drivers/staging/iio/accel/adis16240.c | 11 ++-
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/iio/accel/adis16240.c
> b/drivers/staging/iio/accel/adis16240.c
> index fff6d99089cc..24e525f1ef25 100644
> --- a/drivers/staging/iio/accel/adis16240.c
> +++ b/drivers/staging/iio/accel/adis16240.c
> @@ -250,7 +250,6 @@ static int adis16240_read_raw(struct iio_dev *indio_dev,
> {
> struct adis *st = iio_priv(indio_dev);
> int ret;
> - int bits;
> u8 addr;
> s16 val16;
>
> @@ -287,24 +286,18 @@ static int adis16240_read_raw(struct iio_dev *indio_dev,
> *val = 25000 / 244 - 0x133; /* 25 C = 0x133 */
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_CALIBBIAS:
> - bits = 10;
> addr = adis16240_addresses[chan->scan_index][0];
> ret = adis_read_reg_16(st, addr, );
> if (ret)
> return ret;
> - val16 &= (1 << bits) - 1;
> - val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
> - *val = val16;
> + *val = sign_extend32(val16, 9);
> return IIO_VAL_INT;
> case IIO_CHAN_INFO_PEAK:
> - bits = 10;
> addr = adis16240_addresses[chan->scan_index][1];
> ret = adis_read_reg_16(st, addr, );
> if (ret)
> return ret;
> - val16 &= (1 << bits) - 1;
> - val16 = (s16)(val16 << (16 - bits)) >> (16 - bits);
> - *val = val16;
> + *val = sign_extend32(val16, 9);
> return IIO_VAL_INT;
> }
> return -EINVAL;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net 2/3] hv_netvsc: fix network namespace issues with VF support
On Mon, Jun 11, 2018 at 12:44:55PM -0700, Stephen Hemminger wrote:
> When finding the parent netvsc device, the search needs to be across
> all netvsc device instances (independent of network namespace).
>
> Find parent device of VF using upper_dev_get routine which
> searches only adjacent list.
>
> Fixes: e8ff40d4bff1 ("hv_netvsc: improve VF device matching")
> Signed-off-by: Stephen Hemminger
>
> netns aware byref
What? Presumably that wasn't supposed to be part of the commit message.
> ---
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
On 06/12/18 at 11:29am, Andy Shevchenko wrote:
> On Tue, Jun 12, 2018 at 6:28 AM, Baoquan He wrote:
> > reparent_resources() is duplicated in arch/microblaze/pci/pci-common.c
> > and arch/powerpc/kernel/pci-common.c, so move it to kernel/resource.c
> > so that it's shared. Later its code also need be updated using list_head
> > to replace singly linked list.
>
> While this is a good deduplication of the code, some requirements for
> public functions would be good to satisfy.
>
> > +/*
> > + * Reparent resource children of pr that conflict with res
> > + * under res, and make res replace those children.
> > + */
>
> kernel doc format, though...
Will rewrite it, thanks.
>
> > +static int reparent_resources(struct resource *parent,
> > +struct resource *res)
>
> ...is it really public with static keyword?!
>
>
>
> > +{
>
> > + for (pp = >child; (p = *pp) != NULL; pp = >sibling) {
> > + if (p->end < res->start)
> > + continue;
> > + if (res->end < p->start)
> > + break;
>
> > + if (p->start < res->start || p->end > res->end)
> > + return -1; /* not completely contained */
>
> Usually we are expecting real eeror codes.
>
> > + if (firstpp == NULL)
> > + firstpp = pp;
> > + }
>
> > + if (firstpp == NULL)
> > + return -1; /* didn't find any conflicting entries? */
>
> Ditto.
>
> > +}
> > +EXPORT_SYMBOL(reparent_resources);
>
> --
> With Best Regards,
> Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
On 06/12/18 at 11:29am, Andy Shevchenko wrote:
> On Tue, Jun 12, 2018 at 6:28 AM, Baoquan He wrote:
> > reparent_resources() is duplicated in arch/microblaze/pci/pci-common.c
> > and arch/powerpc/kernel/pci-common.c, so move it to kernel/resource.c
> > so that it's shared. Later its code also need be updated using list_head
> > to replace singly linked list.
>
> While this is a good deduplication of the code, some requirements for
> public functions would be good to satisfy.
>
> > +/*
> > + * Reparent resource children of pr that conflict with res
> > + * under res, and make res replace those children.
> > + */
>
> kernel doc format, though...
>
> > +static int reparent_resources(struct resource *parent,
> > +struct resource *res)
>
> ...is it really public with static keyword?!
Thanks for looking into this. This is a code bug, I copied and changed,
but forgot merging the changing to local commit. And the error reported
by test robot in patch 2 was changed too locally, forgot merging it to
patch. Will repost to address this.
>
>
>
> > +{
>
> > + for (pp = >child; (p = *pp) != NULL; pp = >sibling) {
> > + if (p->end < res->start)
> > + continue;
> > + if (res->end < p->start)
> > + break;
>
> > + if (p->start < res->start || p->end > res->end)
> > + return -1; /* not completely contained */
>
> Usually we are expecting real eeror codes.
Hmm, I just copied it from arch/powerpc/kernel/pci-common.c. The
function interface expects an integer returned value, not sure what a
real error codes look like, could you give more hints? Will change
accordingly.
>
> > + if (firstpp == NULL)
> > + firstpp = pp;
> > + }
>
> > + if (firstpp == NULL)
> > + return -1; /* didn't find any conflicting entries? */
>
> Ditto.
>
> > +}
> > +EXPORT_SYMBOL(reparent_resources);
>
> --
> With Best Regards,
> Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 2/2] media: staging/imx: fill vb2_v4l2_buffer sequence entry
On 12/06/18 10:41, Philipp Zabel wrote: > Hi, > > On Mon, 2018-06-11 at 22:48 +0200, Peter Seiderer wrote: >> Hello *, >> >> On Fri, 16 Mar 2018 10:05:44 -0700, Steve Longerbeam >> wrote: >> >>> Reviewed-by: Steve Longerbeam >> >> Ping? Anybody taking this one? > > The patches have been marked not applicable in patchwork [1][2]. > They do apply on media_tree master. > > [1] https://patchwork.linuxtv.org/patch/47946/ > [2] https://patchwork.linuxtv.org/patch/47947/ > > regards > Philipp > I changed the state to NEW and delegated it to me, so the next time I'm processing patches I'll handled them. I suspect the state was changed by mistake by someone (quite possibly me!). Regards, Hans ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: Improved readability of function comedi_nsamples_left.
You haven't been sending the v2 and v3 patches in the right way.
Take a look through the email archive and see how this is normally done.
Also google it:
https://www.google.com/search?q=how+to+send+a+v2+patch
On Sun, Jun 10, 2018 at 12:30:01PM +0200, chris wrote:
> Hi Greg,
> I've added changelog text to the patch below. Appreciate your
> feedback!
> Regards,
> Chris Opperman
>
> >8--8<
>
> Improved the readability of comedi_nsamples_left:
> a) Reduced nesting by using more return calls.
nit: return isn't a function so it can't be called.
> b) Separated variable declarations from code.
It was separate in the original code too.
> c) Using kernel integer types.
The original types were fine/better.
The rules are that we don't like uint32_t and the reason for not liking
it is that we looked through the kernel code and standardized on what
was most common...
Generally u32 and friends mean that the hardware or network protocol
specifies the number of bits. That's especially true for s32. If you
see code using s32 when it's not tied to a hardware spec then that's a
sign that the code is written by an insane person and it's a tricky
situation to deal with.
Some other reasons to use u32 is because it's shorter to type than
unsigned int and because of line lengths... I've sometimes done that.
But it's perhaps lazy.
>
> Signed-off-by: Chris Opperman
> ---
> drivers/staging/comedi/drivers.c | 29 ++---
> 1 file changed, 14 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers.c
> b/drivers/staging/comedi/drivers.c
> index 9d73347..3207ae2 100644
> --- a/drivers/staging/comedi/drivers.c
> +++ b/drivers/staging/comedi/drivers.c
> @@ -468,26 +468,25 @@ EXPORT_SYMBOL_GPL(comedi_nscans_left);
> * Returns the number of samples remaining to complete the command, or the
> * specified expected number of samples (@nsamples), whichever is fewer.
> */
> -unsigned int comedi_nsamples_left(struct comedi_subdevice *s,
> - unsigned int nsamples)
> +u32 comedi_nsamples_left(struct comedi_subdevice *s, u32 nsamples)
> {
> struct comedi_async *async = s->async;
> struct comedi_cmd *cmd = >cmd;
> + u32 scans_left;
I would make scans_left an unsigned long long so that you can remove
the cast when you do the multiply.
> + u64 samples_left;
Nothing wrong with unsigned long long.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 2/2] media: staging/imx: fill vb2_v4l2_buffer sequence entry
Hi, On Mon, 2018-06-11 at 22:48 +0200, Peter Seiderer wrote: > Hello *, > > On Fri, 16 Mar 2018 10:05:44 -0700, Steve Longerbeam > wrote: > > > Reviewed-by: Steve Longerbeam > > Ping? Anybody taking this one? The patches have been marked not applicable in patchwork [1][2]. They do apply on media_tree master. [1] https://patchwork.linuxtv.org/patch/47946/ [2] https://patchwork.linuxtv.org/patch/47947/ regards Philipp ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: rtl8192u: remove redundant variables
Looks good thanks. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 1/4] resource: Move reparent_resources() to kernel/resource.c and make it public
On Tue, Jun 12, 2018 at 6:28 AM, Baoquan He wrote:
> reparent_resources() is duplicated in arch/microblaze/pci/pci-common.c
> and arch/powerpc/kernel/pci-common.c, so move it to kernel/resource.c
> so that it's shared. Later its code also need be updated using list_head
> to replace singly linked list.
While this is a good deduplication of the code, some requirements for
public functions would be good to satisfy.
> +/*
> + * Reparent resource children of pr that conflict with res
> + * under res, and make res replace those children.
> + */
kernel doc format, though...
> +static int reparent_resources(struct resource *parent,
> +struct resource *res)
...is it really public with static keyword?!
> +{
> + for (pp = >child; (p = *pp) != NULL; pp = >sibling) {
> + if (p->end < res->start)
> + continue;
> + if (res->end < p->start)
> + break;
> + if (p->start < res->start || p->end > res->end)
> + return -1; /* not completely contained */
Usually we are expecting real eeror codes.
> + if (firstpp == NULL)
> + firstpp = pp;
> + }
> + if (firstpp == NULL)
> + return -1; /* didn't find any conflicting entries? */
Ditto.
> +}
> +EXPORT_SYMBOL(reparent_resources);
--
With Best Regards,
Andy Shevchenko
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Reply Me Now It's Very Urgent
Reply Me Now It's Very Urgent Hello My Good Friend,I am a banker in Bank Atlantic Ivory Coast . I want to transfer an abandoned sum of 7.2millions USD to your account. 50% will be for you. No risk involved. Contact me immediately for more details becuase this is a deal at stake. Mrs Celina Joseph. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 0/8] staging: mt7621-gpio: last cleanups
On Tue, Jun 12, 2018 at 12:33:42PM +1000, NeilBrown wrote:
> On Mon, Jun 11 2018, Sergio Paracuellos wrote:
>
> > After submiting this driver to try to get mainlined and get
> > out of staging some new cleanups seems to be necessary.
> > According to this main of Linus Walleij:
> >
> > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-June/121742.html
> >
> > this series tries to fix all of the issues in order to send
> > v2 and give it a new try. Because I don't have to hardware to
> > test my changes I send new cleanups first in staging to make
> > easier to NeilBrown test it and get a feedback about them.
> >
> > Changes in v3:
> > - PATCH 7: refactor irq_type to make better code.
> > - Add PATCH 8 avoiding the use of custom domain and requesting
> > manually a 'IRQF_SHARED'. It should be working now??
>
> Yes, it is working now - thanks.
> With this series, the driver again works for all the tests I can
> perform - except that some names aren't unique, as I've mentioned
> separately.
Awesome. Thanks for testing a review! Now that al is working it is
time for small cleanups.
>
> Looking over the new code:
>
> - I don't think we need PIN_MASK() any more. We needed that
> when we had 1 irq_chip which handled 96 irqs. Now we have
> 3 irq_chips with 32 irqs each.
>
> - documentation for 'struct mtk_data' says it is a single
> irqchip, but I don't think it is any more - there is one
> per gpio chip.
> Related: doco for 'struct mtk_gc' contains data for both
>the gpio_chip and the irq_chip. I don't know if that
>needs to be spelled out.
>
> - In
> if (pending) {
> for_each_set_bit(bit, , MTK_BANK_WIDTH) {
> I wouldn't bother with the "if (pending)".
> If pending is zero, then find_each_set_bit() won't find anything.
> It is at most a minor optimization.
> This is a personal preference and if you like it that way, leave it.
> Though if you are keen to optimize, then instead of calling
> mtk_gpio_w32(...BIT(bit)) for every found bit, just call
> mtk_gpio_w32(... pending) once at the top.
>
> - to_mediatek_gpio() cannot return NULL, so testing "if (!rg)" in
> several places is pointless.
>
> - If the dts file doesn't specify an irq, the irq_of_parse_and_map()
> will return -1 (I think). This might deserve a warning and probably
> shouldn't cause the probe to fail, but it should cause
> mediatek_gpio_bank_probe to avoid trying to set up interrupts.
>
>
> Nothing serious, but some might be worth fixing.
Thanks for pointing out these in this mail also.
Hopefully I'll resend a new cleanups series in reply to this
mail.
>
> Thanks a lot,
> NeilBrown
Best regards,
Sergio Paracuellos
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/7] staging: mt7621-gpio: last cleanups
On Tue, Jun 12, 2018 at 12:01:38PM +1000, NeilBrown wrote: > On Mon, Jun 11 2018, Sergio Paracuellos wrote: > > > On Mon, Jun 11, 2018 at 06:33:44PM +1000, NeilBrown wrote: > >> On Mon, Jun 11 2018, Sergio Paracuellos wrote: > >> > >> > After submiting this driver to try to get mainlined and get > >> > out of staging some new cleanups seems to be necessary. > >> > According to this main of Linus Walleij: > >> > > >> > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-June/121742.html > >> > > >> > this series tries to fix all of the issues in order to send > >> > v2 and give it a new try. Because I don't have to hardware to > >> > test my changes I send new cleanups first in staging to make > >> > easier to NeilBrown test it and get a feedback about them. > >> > > >> > Changes in v2: > >> > - Patch where GPIOLIB_IRQCHIP was used avoiding > >> > the use of a custom irq domain has been dropped to > >> > be sure after this changes all is working properly. > >> > (This was PATCH 7 in previous series) > >> > - PATCH 1: > >> > * avoid introducing new macros and use 'bank' > >> >field of mtk_gc with register offset. > >> > * Make correct use of bgpio_init passing new > >> >void __iomem pointers instead of use the > >> >macros. > >> > - Previous series PATCH 8 now is PATCH 7. Avoid the > >> > use of a switch-case statement which was wrong and > >> > distinc if we have RISSING AND FALLING EDGE interrupt > >> > or HIGH LOW level ones. This last two are exclusive and > >> > cannot be generated at the same time. > >> > > >> > Also, I think is we finally avoid to use a new irq_domain the > >> > need for the new functions introduced for request and release > >> > resources dissapears. I was diving down the other drivers code > >> > and I see that these two only are used in drivers which use its > >> > own irq_domain. Correct me if I am wrong, please. > >> > > >> > Hope this helps. > >> > > >> > Thanks in advance. > >> > >> Thanks a lot. > >> This series appears to work, though I sent a separate comment on one > >> piece of code. > > > > Thanks for testing, review and feedback. > > > > I have resent a complete v3 of this series taking into account > > your comments there. > > > >> However the gpio are numbers > >> 480-511 > >> 448-479 > >> 416-447 > >> > >> instead of > >> 0-31 > >> 32-63 > >> 64-95 > >> > >> which would be more normal. > >> Maybe when you resubmit I'll raid it with Linus Walleij and see if he > >> can explain why I can't have 0-95. > > > > This change is because the chip.base property changed to be -1 for > > dynamic enumeration of the gpio's. In this mail there is some explanation > > about it: > > > > http://lists.infradead.org/pipermail/linux-rpi-kernel/2014-October/001045.html > > Interesting - thanks for the link. > > It seems that the goal is to focus more on names than on numbers, and > that probably makes sense. > It means that we need to make sure that the names are useful. > > Currently the driver defines 3 gpiochips, one for each bank. > > # grep . /sys/class/gpio/gpiochip4*/label > /sys/class/gpio/gpiochip416/label:1e000600.gpio > /sys/class/gpio/gpiochip448/label:1e000600.gpio > /sys/class/gpio/gpiochip480/label:1e000600.gpio > > Unfortunately they all have the same label :-( > It would be good if there was a way to add 0, 1, and 2 to the labels, or > something like that. There might be a way to set that labels from dts. I'll check other drivers code to change those. > > In /proc/interrupts we now have: > > 17: 0 0 0 0 MIPS GIC 19 mt7621, > mt7621, mt7621 > > which is the interrupt from the GPIO controller. > It is a little weird that all three banks are named "mt7621" here. This is the name which is being set from devm_request_irq to mark line as shared. Maybe set mt7621-bank[0-2] is more usefull. > We also have: > > 26: 0 0 0 0 GPIO 18 reset This is the name from the one from the static initialization of the gpio_chip where callbacks are defined. Let's see what can I do to change that also. > > which is the interrupt from GPIO which provides the "reset" button. > I suspect that if I had interrupts form two different banks they would > both be called "GPIO" which would be a little confusing. > We could declare three different 'struct irq_chip' with three different > names, but that would be ugly. Hopefully there is a better way. > > Thanks, > NeilBrown Best regards, Sergio Paracuellos ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: speakup: refactor synths array to use a list
Gregory Nowak, le lun. 11 juin 2018 16:51:22 -0700, a ecrit: > On Tue, Jun 12, 2018 at 12:57:03AM +0200, Samuel Thibault wrote: > > Anybody up for testing please? > > > > If people want to see speakup get mainlined instead of staging, please > > help. > > If I understand right, this patch changes how synthesizers are loaded > and unloaded through /sys/accessibility/speakup/synth, correct? The load/unload is about the module itself, i.e. modprobe speakup_bns ; modprobe speakup_soft, switch between them, then rmmod speakup_bns ; speakup_soft or the converse (to exercise both orders). Thanks! Samuel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: add error handling for try_module_get
On Tue, Jun 12, 2018 at 12:49:26PM +0800, Zhouyang Jia wrote: > When try_module_get fails, the lack of error-handling code may > cause unexpected results. > > This patch adds error-handling code after calling try_module_get. > > Signed-off-by: Zhouyang Jia > --- > drivers/staging/lustre/lnet/klnds/socklnd/socklnd.c | 5 - This patch does not apply to Linus's tree. Always be sure to work against linux-next to catch things like this. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
