RE: [PATCH] staging: unisys: visorchipset: Use common error handling code in setup_crash_devices_work_queue()

2017-11-03 Thread Kershner, David A 
> -Original Message-
> From: SF Markus Elfring [mailto:elfr...@users.sourceforge.net]
> Sent: Friday, November 3, 2017 3:50 PM
> To: de...@driverdev.osuosl.org; *S-Par-Maintainer
> ; Thompson, Bryan E.
> ; Binder, David Anthony
> ; Kershner, David A
> ; Greg Kroah-Hartman
> ; Sameer Wadgaonkar
> ; Sell, Timothy C
> 
> Cc: LKML ; kernel-janit...@vger.kernel.org
> Subject: [PATCH] staging: unisys: visorchipset: Use common error handling
> code in setup_crash_devices_work_queue()
>
> From: Markus Elfring 
> Date: Fri, 3 Nov 2017 20:37:03 +0100
>
> * Add a jump target so that a specific error message is stored only once
>   at the end of this function implementation.
>
> * Replace four calls of the function "dev_err" by goto statements.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring 

Thanks for the catch!


Acked-by: David Kershner 



> ---
>  drivers/staging/unisys/visorbus/visorchipset.c | 36 
> 
> --
>  1 file changed, 16 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/unisys/visorbus/visorchipset.c
> b/drivers/staging/unisys/visorbus/visorchipset.c
> index fed554a43151..1d54821dd7b6 100644
> --- a/drivers/staging/unisys/visorbus/visorchipset.c
> +++ b/drivers/staging/unisys/visorbus/visorchipset.c
> @@ -1231,11 +1231,9 @@ static void
> setup_crash_devices_work_queue(struct work_struct *work)
>   if (visorchannel_read(chipset_dev->controlvm_channel,
> offsetof(struct visor_controlvm_channel,
>  saved_crash_message_count),
> -   _crash_msg_count, sizeof(u16)) < 0) {
> - dev_err(_dev->acpi_device->dev,
> - "failed to read channel\n");
> - return;
> - }
> +   _crash_msg_count, sizeof(u16)) < 0)
> + goto report_read_failure;
> +
>   if (local_crash_msg_count != CONTROLVM_CRASHMSG_MAX) {
>   dev_err(_dev->acpi_device->dev, "invalid
> count\n");
>   return;
> @@ -1244,30 +1242,24 @@ static void
> setup_crash_devices_work_queue(struct work_struct *work)
>   if (visorchannel_read(chipset_dev->controlvm_channel,
> offsetof(struct visor_controlvm_channel,
>  saved_crash_message_offset),
> -   _crash_msg_offset, sizeof(u32)) < 0) {
> - dev_err(_dev->acpi_device->dev,
> - "failed to read channel\n");
> - return;
> - }
> +   _crash_msg_offset, sizeof(u32)) < 0)
> + goto report_read_failure;
> +
>   /* read create device message for storage bus offset */
>   if (visorchannel_read(chipset_dev->controlvm_channel,
> local_crash_msg_offset,
> _crash_bus_msg,
> -   sizeof(struct controlvm_message)) < 0) {
> - dev_err(_dev->acpi_device->dev,
> - "failed to read channel\n");
> - return;
> - }
> +   sizeof(struct controlvm_message)) < 0)
> + goto report_read_failure;
> +
>   /* read create device message for storage device */
>   if (visorchannel_read(chipset_dev->controlvm_channel,
> local_crash_msg_offset +
> sizeof(struct controlvm_message),
> _crash_dev_msg,
> -   sizeof(struct controlvm_message)) < 0) {
> - dev_err(_dev->acpi_device->dev,
> - "failed to read channel\n");
> - return;
> - }
> +   sizeof(struct controlvm_message)) < 0)
> + goto report_read_failure;
> +
>   /* reuse IOVM create bus message */
>   if (!local_crash_bus_msg.cmd.create_bus.channel_addr) {
>   dev_err(_dev->acpi_device->dev,
> @@ -1282,6 +1274,10 @@ static void
> setup_crash_devices_work_queue(struct work_struct *work)
>   return;
>   }
>   visorbus_device_create(_crash_dev_msg);
> + return;
> +
> +report_read_failure:
> + dev_err(_dev->acpi_device->dev, "failed to read
> channel\n");
>  }
>
>  void visorbus_response(struct visor_device *bus_info, int response,
> --
> 2.15.0



smime.p7s
Description: S/MIME cryptographic signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH char-misc 1/1] Drivers: hv: vmbus: Implement Direct Mode for stimer0

2017-11-03 Thread kbuild test robot 
Hi Michael,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on char-misc/master]

url:
https://github.com/0day-ci/linux/commits/mikelley-exchange-microsoft-com/Drivers-hv-vmbus-Implement-Direct-Mode-for-stimer0/20171103-214519
config: x86_64-randconfig-h0-11040619 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All warnings (new ones prefixed by >>):

   In file included from include/linux/kasan.h:16:0,
from include/linux/slab.h:120,
from include/linux/irq.h:25,
from arch/x86/include/asm/mshyperv.h:7,
from arch/x86/entry/vdso/vdso32/../vclock_gettime.c:20,
from arch/x86/entry/vdso/vdso32/vclock_gettime.c:32:
   arch/x86/include/asm/pgtable.h: In function 'pte_flags_pkey':
>> arch/x86/include/asm/pgtable.h:1199:2: warning: left shift count >= width of 
>> type
 return (pte_flags & _PAGE_PKEY_MASK) >> _PAGE_BIT_PKEY_BIT0;
 ^
>> arch/x86/include/asm/pgtable.h:1199:2: warning: left shift count >= width of 
>> type
>> arch/x86/include/asm/pgtable.h:1199:2: warning: left shift count >= width of 
>> type
>> arch/x86/include/asm/pgtable.h:1199:2: warning: left shift count >= width of 
>> type
   arch/x86/include/asm/pgtable.h:1199:2: warning: right shift count >= width 
of type

vim +1199 arch/x86/include/asm/pgtable.h

33a709b2 Dave Hansen 2016-02-12  1194  
33a709b2 Dave Hansen 2016-02-12  1195  static inline u16 
pte_flags_pkey(unsigned long pte_flags)
33a709b2 Dave Hansen 2016-02-12  1196  {
33a709b2 Dave Hansen 2016-02-12  1197  #ifdef 
CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
33a709b2 Dave Hansen 2016-02-12  1198   /* ifdef to avoid doing 59-bit shift on 
32-bit values */
33a709b2 Dave Hansen 2016-02-12 @1199   return (pte_flags & _PAGE_PKEY_MASK) >> 
_PAGE_BIT_PKEY_BIT0;
33a709b2 Dave Hansen 2016-02-12  1200  #else
33a709b2 Dave Hansen 2016-02-12  1201   return 0;
33a709b2 Dave Hansen 2016-02-12  1202  #endif
33a709b2 Dave Hansen 2016-02-12  1203  }
33a709b2 Dave Hansen 2016-02-12  1204  

:: The code at line 1199 was first introduced by commit
:: 33a709b25a760b91184bb335cf7d7c32b8123013 mm/gup, x86/mm/pkeys: Check 
VMAs and PTEs for protection keys

:: TO: Dave Hansen <dave.han...@linux.intel.com>
:: CC: Ingo Molnar <mi...@kernel.org>

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH char-misc v3 1/1] Drivers: hv: vmbus: Remove x86-isms from arch independent drivers

2017-11-03 Thread mikelley 
From: Michael Kelley 

hv_is_hypercall_page_setup() is used to check if Hyper-V is
initialized, but a 'hypercall page' is an x86 implementation detail
that isn't necessarily present on other architectures. Rename to the
architecture independent hv_is_hyperv_initialized() and add check
that x86_hyper is pointing to Hyper-V.  Use this function instead of
direct references to x86-specific data structures in vmbus_drv.c,
and remove now redundant call in hv_init(). Also remove 'x86' from
the string name passed to cpuhp_setup_state().

Signed-off-by: Michael Kelley 

---
Changed since v1:
* Incorporated x86_hyper check into hv_is_hyperv_initialized() per
  KY Srinivasan's comment that it is still needed
* Removed now redundant call to hv_is_hyperv_initialized() in hv_init()
Changed since v2:
* Fixed block comment style in hv_is_hyperv_initialized()
* Fixed my Reply-To email address

---
 arch/x86/hyperv/hv_init.c   | 21 ++---
 arch/x86/include/asm/mshyperv.h |  4 ++--
 drivers/hv/hv.c |  3 ---
 drivers/hv/vmbus_drv.c  |  5 ++---
 4 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index aeb8edf..e5372c9 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -239,17 +239,24 @@ void hyperv_report_panic(struct pt_regs *regs, long err)
 }
 EXPORT_SYMBOL_GPL(hyperv_report_panic);
 
-bool hv_is_hypercall_page_setup(void)
+bool hv_is_hyperv_initialized(void)
 {
union hv_x64_msr_hypercall_contents hypercall_msr;
 
-   /* Check if the hypercall page is setup */
+   /*
+* Ensure that we're really on Hyper-V, and not a KVM or Xen
+* emulation of Hyper-V
+*/
+   if (x86_hyper != _hyper_ms_hyperv)
+   return false;
+
+   /*
+* Verify that earlier initialization succeeded by checking
+* that the hypercall page is setup
+*/
hypercall_msr.as_uint64 = 0;
rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
 
-   if (!hypercall_msr.enable)
-   return false;
-
-   return true;
+   return hypercall_msr.enable;
 }
-EXPORT_SYMBOL_GPL(hv_is_hypercall_page_setup);
+EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index bd89104..740dc97 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -311,11 +311,11 @@ static inline int hv_cpu_number_to_vp_number(int 
cpu_number)
 void hyperv_setup_mmu_ops(void);
 void hyper_alloc_mmu(void);
 void hyperv_report_panic(struct pt_regs *regs, long err);
-bool hv_is_hypercall_page_setup(void);
+bool hv_is_hyperv_initialized(void);
 void hyperv_cleanup(void);
 #else /* CONFIG_HYPERV */
 static inline void hyperv_init(void) {}
-static inline bool hv_is_hypercall_page_setup(void) { return false; }
+static inline bool hv_is_hyperv_initialized(void) { return false; }
 static inline void hyperv_cleanup(void) {}
 static inline void hyperv_setup_mmu_ops(void) {}
 #endif /* CONFIG_HYPERV */
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 8267439..fe96aab 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -49,9 +49,6 @@ struct hv_context hv_context = {
  */
 int hv_init(void)
 {
-   if (!hv_is_hypercall_page_setup())
-   return -ENOTSUPP;
-
hv_context.cpu_context = alloc_percpu(struct hv_per_cpu_context);
if (!hv_context.cpu_context)
return -ENOMEM;
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 8fe13cd..574668c 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -37,7 +37,6 @@
 #include 
 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -1049,7 +1048,7 @@ static int vmbus_bus_init(void)
 * Initialize the per-cpu interrupt state and
 * connect to the host.
 */
-   ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/hyperv:online",
+   ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "hyperv/vmbus:online",
hv_synic_init, hv_synic_cleanup);
if (ret < 0)
goto err_alloc;
@@ -1711,7 +1710,7 @@ static int __init hv_acpi_init(void)
 {
int ret, t;
 
-   if (x86_hyper != _hyper_ms_hyperv)
+   if (!hv_is_hyperv_initialized())
return -ENODEV;
 
init_completion(_event);
-- 
1.8.3.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: greybus: Convert timers to use timer_setup()

2017-11-03 Thread Kees Cook 
On Fri, Nov 3, 2017 at 2:49 PM, Bryan O'Donoghue
 wrote:
>
>
> On 03/11/17 20:21, Kees Cook wrote:
>>
>> On Mon, Oct 30, 2017 at 5:05 PM, Kees Cook  wrote:
>>>
>>> On Mon, Oct 30, 2017 at 5:01 PM,   wrote:

 There's a separate change to loopback.c an old patch ARAIR that will
 subtract use of the timer from loopback.c so you can skip that bit.
>>>
>>>
>>> Okay, cool. Since the operation.c change is trivial, I'll include it
>>> in the giant tree-wide patch that will (hopefully) land in -rc1.
>>
>>
>> I forgot to ask: will the patch for loopback.c that removes the timers
>> get posted in the next couple days? I just want to make sure the timer
>> conversions don't get blocked behind this.
>
>
> Yep.
>
> I should get that out tomorrow at some stage.

Awesome, thanks very much!

-Kees

-- 
Kees Cook
Pixel Security
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: greybus: Convert timers to use timer_setup()

2017-11-03 Thread Bryan O'Donoghue 



On 03/11/17 20:21, Kees Cook wrote:

On Mon, Oct 30, 2017 at 5:05 PM, Kees Cook  wrote:

On Mon, Oct 30, 2017 at 5:01 PM,   wrote:

There's a separate change to loopback.c an old patch ARAIR that will subtract 
use of the timer from loopback.c so you can skip that bit.


Okay, cool. Since the operation.c change is trivial, I'll include it
in the giant tree-wide patch that will (hopefully) land in -rc1.


I forgot to ask: will the patch for loopback.c that removes the timers
get posted in the next couple days? I just want to make sure the timer
conversions don't get blocked behind this.


Yep.

I should get that out tomorrow at some stage.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: wilc1000: Fix bssid buffer offset in Txq

2017-11-03 Thread Aditya Shankar 
Commit 46949b48568b ("staging: wilc1000: New cfg packet
format in handle_set_wfi_drv_handler") updated the frame
format sent from host to the firmware. The code to update
the bssid offset in the new frame was part of a second
patch in the series which did not make it in and thus
causes connection problems after associating to an AP.

This fix adds the proper offset of the bssid value in the
Tx queue buffer to fix the connection issues.

Fixes: Commit 46949b48568b ("staging: wilc1000: New cfg packet format in 
handle_set_wfi_drv_handler")
Cc: sta...@vger.kernel.org
Signed-off-by: Aditya Shankar 
---
 drivers/staging/wilc1000/wilc_wlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c 
b/drivers/staging/wilc1000/wilc_wlan.c
index 9addef1..f49dfa8 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -714,7 +714,7 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 
*txq_count)
char *bssid = ((struct tx_complete_data 
*)(tqe->priv))->bssid;
 
buffer_offset = ETH_ETHERNET_HDR_OFFSET;
-   memcpy([offset + 4], bssid, 6);
+   memcpy([offset + 8], bssid, 6);
} else {
buffer_offset = HOST_HDR_OFFSET;
}
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: greybus: Convert timers to use timer_setup()

2017-11-03 Thread Kees Cook 
On Mon, Oct 30, 2017 at 5:05 PM, Kees Cook  wrote:
> On Mon, Oct 30, 2017 at 5:01 PM,   wrote:
>> There's a separate change to loopback.c an old patch ARAIR that will 
>> subtract use of the timer from loopback.c so you can skip that bit.
>
> Okay, cool. Since the operation.c change is trivial, I'll include it
> in the giant tree-wide patch that will (hopefully) land in -rc1.

I forgot to ask: will the patch for loopback.c that removes the timers
get posted in the next couple days? I just want to make sure the timer
conversions don't get blocked behind this.

Thanks!

-Kees

-- 
Kees Cook
Pixel Security
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: unisys: visorchipset: Use common error handling code in setup_crash_devices_work_queue()

2017-11-03 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Fri, 3 Nov 2017 20:37:03 +0100

* Add a jump target so that a specific error message is stored only once
  at the end of this function implementation.

* Replace four calls of the function "dev_err" by goto statements.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/staging/unisys/visorbus/visorchipset.c | 36 --
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/unisys/visorbus/visorchipset.c 
b/drivers/staging/unisys/visorbus/visorchipset.c
index fed554a43151..1d54821dd7b6 100644
--- a/drivers/staging/unisys/visorbus/visorchipset.c
+++ b/drivers/staging/unisys/visorbus/visorchipset.c
@@ -1231,11 +1231,9 @@ static void setup_crash_devices_work_queue(struct 
work_struct *work)
if (visorchannel_read(chipset_dev->controlvm_channel,
  offsetof(struct visor_controlvm_channel,
   saved_crash_message_count),
- _crash_msg_count, sizeof(u16)) < 0) {
-   dev_err(_dev->acpi_device->dev,
-   "failed to read channel\n");
-   return;
-   }
+ _crash_msg_count, sizeof(u16)) < 0)
+   goto report_read_failure;
+
if (local_crash_msg_count != CONTROLVM_CRASHMSG_MAX) {
dev_err(_dev->acpi_device->dev, "invalid count\n");
return;
@@ -1244,30 +1242,24 @@ static void setup_crash_devices_work_queue(struct 
work_struct *work)
if (visorchannel_read(chipset_dev->controlvm_channel,
  offsetof(struct visor_controlvm_channel,
   saved_crash_message_offset),
- _crash_msg_offset, sizeof(u32)) < 0) {
-   dev_err(_dev->acpi_device->dev,
-   "failed to read channel\n");
-   return;
-   }
+ _crash_msg_offset, sizeof(u32)) < 0)
+   goto report_read_failure;
+
/* read create device message for storage bus offset */
if (visorchannel_read(chipset_dev->controlvm_channel,
  local_crash_msg_offset,
  _crash_bus_msg,
- sizeof(struct controlvm_message)) < 0) {
-   dev_err(_dev->acpi_device->dev,
-   "failed to read channel\n");
-   return;
-   }
+ sizeof(struct controlvm_message)) < 0)
+   goto report_read_failure;
+
/* read create device message for storage device */
if (visorchannel_read(chipset_dev->controlvm_channel,
  local_crash_msg_offset +
  sizeof(struct controlvm_message),
  _crash_dev_msg,
- sizeof(struct controlvm_message)) < 0) {
-   dev_err(_dev->acpi_device->dev,
-   "failed to read channel\n");
-   return;
-   }
+ sizeof(struct controlvm_message)) < 0)
+   goto report_read_failure;
+
/* reuse IOVM create bus message */
if (!local_crash_bus_msg.cmd.create_bus.channel_addr) {
dev_err(_dev->acpi_device->dev,
@@ -1282,6 +1274,10 @@ static void setup_crash_devices_work_queue(struct 
work_struct *work)
return;
}
visorbus_device_create(_crash_dev_msg);
+   return;
+
+report_read_failure:
+   dev_err(_dev->acpi_device->dev, "failed to read channel\n");
 }
 
 void visorbus_response(struct visor_device *bus_info, int response,
-- 
2.15.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging/rts5208/rtsx: Improve unlocking of a mutex in rtsx_resume()

2017-11-03 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Fri, 3 Nov 2017 20:02:22 +0100

* Add a jump target so that a call of the function "mutex_unlock" is stored
  only twice in this function implementation.

* Replace two calls by goto statements.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/staging/rts5208/rtsx.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rts5208/rtsx.c b/drivers/staging/rts5208/rtsx.c
index 89e2cfe7d1cc..14022a76ecfb 100644
--- a/drivers/staging/rts5208/rtsx.c
+++ b/drivers/staging/rts5208/rtsx.c
@@ -349,9 +349,7 @@ static int rtsx_resume(struct pci_dev *pci)
dev_err(>pci->dev,
"%s: pci_enable_device failed, disabling device\n",
CR_DRIVER_NAME);
-   /* unlock the device pointers */
-   mutex_unlock(>dev_mutex);
-   return -EIO;
+   goto unlock;
}
pci_set_master(pci);
 
@@ -360,11 +358,8 @@ static int rtsx_resume(struct pci_dev *pci)
chip->msi_en = 0;
}
 
-   if (rtsx_acquire_irq(dev) < 0) {
-   /* unlock the device pointers */
-   mutex_unlock(>dev_mutex);
-   return -EIO;
-   }
+   if (rtsx_acquire_irq(dev) < 0)
+   goto unlock;
 
rtsx_write_register(chip, HOST_SLEEP_STATE, 0x03, 0x00);
rtsx_init_chip(chip);
@@ -373,6 +368,10 @@ static int rtsx_resume(struct pci_dev *pci)
mutex_unlock(>dev_mutex);
 
return 0;
+
+unlock:
+   mutex_unlock(>dev_mutex);
+   return -EIO;
 }
 #endif /* CONFIG_PM */
 
-- 
2.15.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: staging: comedi: usbduxfast: Improve unlocking of a mutex in usbduxfast_ai_insn_read()

2017-11-03 Thread SF Markus Elfring 
> FYI, you are responding to someone who is on my blacklist

I am curious if this communication setting will ever be adjusted.


> and I never accept patches from.

The history shows that our collaboration style changed over time.
I got a few update suggestions integrated (also by you) because
other contributors found them good enough to repeat them.

Regards,
Markus
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: comedi: usbduxfast: Improve unlocking of a mutex in usbduxfast_ai_insn_read()

2017-11-03 Thread SF Markus Elfring 
>> @@ -838,6 +834,10 @@ static int usbduxfast_ai_insn_read(struct comedi_device 
>> *dev,
>>   mutex_unlock(>mut);
>>     return insn->n;
> 
> Minor niggle: You could also remove that call to mutex_unlock() by replacing 
> the above three lines with:
> 
> ret = insn->n;
> 
> which will fall through to the 'unlock:' label below.

Thanks for your suggestion.

Such a software refactoring is also possible if a corresponding
consensus could be achieved.
* Can such a change mean that the lock scope will be extended
  for both use cases (successful and failed function execution)?

* How much does this implementation matter for you?

* Would you like to achieve a small reduction of the object code there?

* How do you think about consequences from special communication settings
  by a well-known maintainer for my update suggestions?

Regards,
Markus
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH V3] scsi: storvsc: Allow only one remove lun work item to be issued per lun

2017-11-03 Thread Martin K. Petersen 

Cathy,

> When running multipath on a VM if all available paths go down the
> driver can schedule large amounts of storvsc_remove_lun work items to
> the same lun. In response to the failing paths typically storvsc
> responds by taking host->scan_mutex and issuing a TUR per lun. If
> there has been heavy IO to the failed device all the failed IOs are
> returned from the host. A remove lun work item is issued per failed
> IO. If the outstanding TURs have not been completed in a timely manner
> the scan_mutex is never released or released too late. Consequently
> the many remove lun work items are not completed as scsi_remove_device
> also tries to take host->scan_mutex.  This results in dragging the VM
> down and sometimes completely.
>
> This patch only allows one remove lun to be issued to a particular lun
> while it is an instantiated member of the scsi stack.

Applied to 4.15/scsi-queue.

Next time the change log needs to go after a "---" delimiter.

Thank you!

> Changes since v1:
> Use single threaded workqueue to serialize work in
> storvsc_handle_error [Christoph Hellwig]
>
> Changes since v2:
> Replaced create_singlethread_workqueue with
> alloc_ordered_workqueue [Christoph Hellwig]
>
> Added reviewed by's.

-- 
Martin K. Petersen  Oracle Linux Engineering
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH char-misc 1/1] Drivers: hv: vmbus: Implement Direct Mode for stimer0

2017-11-03 Thread kbuild test robot 
Hi Michael,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on char-misc/master]

url:
https://github.com/0day-ci/linux/commits/mikelley-exchange-microsoft-com/Drivers-hv-vmbus-Implement-Direct-Mode-for-stimer0/20171103-214519
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64 

All errors (new ones prefixed by >>):

>> ERROR: "hv_deallocate_stimer0_irq" [drivers/hv/hv_vmbus.ko] undefined!
>> ERROR: "hv_allocate_stimer0_irq" [drivers/hv/hv_vmbus.ko] undefined!
>> ERROR: "hv_ack_stimer0_interrupt" [drivers/hv/hv_vmbus.ko] undefined!

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: fsl-mc: move bus driver out of staging

2017-11-03 Thread Laurentiu Tudor 
Hi Greg,

On 11/03/2017 05:17 PM, Greg KH wrote:
> On Thu, Aug 31, 2017 at 06:04:30PM +0200, Greg KH wrote:
>> On Mon, Aug 28, 2017 at 01:54:05PM +0300, laurentiu.tu...@nxp.com wrote:
>>> From: Stuart Yoder 
>>>
>>> Move the source files out of staging into their final locations:
>>>-include files in drivers/staging/fsl-mc/include go to include/linux/fsl
>>>-irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
>>>-source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
>>>-README.txt, providing and overview of DPAA goes to
>>> Documentation/dpaa2/overview.txt
>>>
>>> Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
>>> Update dpaa2_eth and dpio staging drivers.
>>>
>>> Signed-off-by: Stuart Yoder 
>>> Signed-off-by: Laurentiu Tudor 
>>> [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
>>> Cc: Thomas Gleixner 
>>> Cc: Jason Cooper 
>>> Cc: Marc Zyngier 
>>
>> This is going to have to wait until I get a chunk of time to do the
>> review.  Probably after 4.13-final is out.
>
> Ok, review comments as I go through the code:

Thanks a lot for taking the time. I'll take care of the comments next week.

---
Best Regards, Laurentiu
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] staging: fsl-mc: move bus driver out of staging

2017-11-03 Thread Greg KH 
On Thu, Aug 31, 2017 at 06:04:30PM +0200, Greg KH wrote:
> On Mon, Aug 28, 2017 at 01:54:05PM +0300, laurentiu.tu...@nxp.com wrote:
> > From: Stuart Yoder 
> > 
> > Move the source files out of staging into their final locations:
> >   -include files in drivers/staging/fsl-mc/include go to include/linux/fsl
> >   -irq-gic-v3-its-fsl-mc-msi.c goes to drivers/irqchip
> >   -source in drivers/staging/fsl-mc/bus goes to drivers/bus/fsl-mc
> >   -README.txt, providing and overview of DPAA goes to
> >Documentation/dpaa2/overview.txt
> > 
> > Update or delete other remaining staging files-- Makefile, Kconfig, TODO.
> > Update dpaa2_eth and dpio staging drivers.
> > 
> > Signed-off-by: Stuart Yoder 
> > Signed-off-by: Laurentiu Tudor 
> > [Laurentiu: rebased, add dpaa2_eth and dpio #include updates]
> > Cc: Thomas Gleixner 
> > Cc: Jason Cooper 
> > Cc: Marc Zyngier 
> 
> This is going to have to wait until I get a chunk of time to do the
> review.  Probably after 4.13-final is out.

Ok, review comments as I go through the code:
mc-sys.c 323 EXPORT_SYMBOL(mc_send_command);

should be EXPORT_SYMBOL_GPL(fsl_mc_send_command); to match up with your
other exports and global namespace, right?

You export a lot of dpcon_* symbols that no one uses, please do not do
that.  Verify that all of them are actually used right now, if not,
remove them.  If you think you are going to use them in the future,
wonderful, add them in then.

Same for your dpaa2_* exported symbols, most are not used from what I
can see.

struct dpaa2_io {
atomic_t refs;

That's a kref, please use it instead of trying to roll your own.

And even for this, your locking is not correct (i.e. you do not have
any), that needs to be fixed so that teardown works correctly.

You have a lot of WARN_ON() calls, that's going to be ignored and should
all not be needed now that the code is debugged and working properly.
Please remove them, or turn them into dev_err() calls with a real if ()
check instead.

You are checking "strings" for the type of device in a lot of places,
like this:
if (strcmp(obj_desc->type, "dprc") == 0) {
why are you not just using the built-in driver model .type field and
comparing that to the different type structures?  It's much easier, and
you don't have to again, "roll your own".  See the USB or Greybus code
for examples of busses that have different "types" of devices on them at
the same time.

Ok, that's enough for now, please work on this, and we can go from
there...

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [REVIEW REQUEST] staging: unisys: review request for visorbus

2017-11-03 Thread gre...@linuxfoundation.org 
On Mon, Oct 02, 2017 at 05:49:52PM +0200, gre...@linuxfoundation.org wrote:
> On Mon, Oct 02, 2017 at 03:41:42PM +, Kershner, David A wrote:
> > Hey Greg, we think the code for visorbus is ready to be moved out
> > of staging, can you review it to see if we have missed anything?
> 
> I think your html email got rejected by the list :(
> 
> > The files include:
> > /drivers/staging/unisys/visorbus/
> > /drivers/staging/unisys/include/visorchannel.h
> > /drivers/staging/unisys/include/visorbus.h
> > 
> > The directories staging/drivers/unisys/visornic, visorhba, and visorinput
> > are not part of the review as well as the file
> > drivers/staging/unisys/include/iochannel.h.
> > 
> > We currently have 5 checkpatch.pl warnings that I know about:
> >  - 3 CHECKs in visorchannel that are due to a MACRO that gets passed a FIELD
> > instead of just a variable
> > - 2 WARNINGS dealing with char * becoming static const
> > 
> >  
> > 
> > Dan Carpenter found some extra parenthesis errors that I will address as
> > well as look through the code to fix, but I would like to ask for the review
> > while we are working on that.
> 
> Sure, I'll look at doing it in a week or so when I catch up with other
> patches in my queue.
> 
> Everyone else is also welcome to do review :)

Well, don't everone jump in at once :(

Anyway, drivers/staging/unisys/visorbus/ looks good, after my one tiny
patch I've just sent you.  Feel free to send a patch moving it to
drivers/visorbus now if you want to.

Then the individual drivers can go into the different subsystem
locations after they are reviewed by the different subsystem
maintainers, but that can't happen until the core moves.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: unisys: don't care about debugfs errors

2017-11-03 Thread gre...@linuxfoundation.org 
A caller should never care about a debugfs error return value, and it
should never abort its normal operation if something "odd" goes on.  Fix
up the unisys init code to not care if the root debugfs directory for
the subsystem is created or not, as no place it is used will matter.

Cc: David Kershner 
Cc: Tim Sell 
Cc: Sameer Wadgaonkar 
Cc: David Binder 
Signed-off-by: Greg Kroah-Hartman 


diff --git a/drivers/staging/unisys/visorbus/visorbus_main.c 
b/drivers/staging/unisys/visorbus/visorbus_main.c
index 2f1e8d36aedf..b604d0cccef1 100644
--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -1217,8 +1217,6 @@ int visorbus_init(void)
int err;
 
visorbus_debugfs_dir = debugfs_create_dir("visorbus", NULL);
-   if (!visorbus_debugfs_dir)
-   return -ENOMEM;
bus_device_info_init(_driverinfo, "clientbus", "visorbus");
err = bus_register(_type);
if (err < 0)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: comedi: usbduxfast: Improve unlocking of a mutex in usbduxfast_ai_insn_read()

2017-11-03 Thread Greg Kroah-Hartman 
On Fri, Nov 03, 2017 at 02:00:18PM +, Ian Abbott wrote:
> On 02/11/17 19:40, SF Markus Elfring wrote:
> > From: Markus Elfring 
> > Date: Thu, 2 Nov 2017 20:30:31 +0100
> > 
> > * Add a jump target so that a call of the function "mutex_unlock" is stored
> >only twice in this function implementation.
> > 
> > * Replace five calls by goto statements.
> > 
> > This issue was detected by using the Coccinelle software.
> > 
> > Signed-off-by: Markus Elfring 
> > ---
> >   drivers/staging/comedi/drivers/usbduxfast.c | 24 
> >   1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/staging/comedi/drivers/usbduxfast.c 
> > b/drivers/staging/comedi/drivers/usbduxfast.c
> > index 608403c7586b..e5884faf7275 100644
> > --- a/drivers/staging/comedi/drivers/usbduxfast.c
> > +++ b/drivers/staging/comedi/drivers/usbduxfast.c
> > @@ -777,8 +777,8 @@ static int usbduxfast_ai_insn_read(struct comedi_device 
> > *dev,
> > if (devpriv->ai_cmd_running) {
> > dev_err(dev->class_dev,
> > "ai_insn_read not possible, async cmd is running\n");
> > -   mutex_unlock(>mut);
> > -   return -EBUSY;
> > +   ret = -EBUSY;
> > +   goto unlock;
> > }
> > /* set command for the first channel */
> > @@ -798,10 +798,8 @@ static int usbduxfast_ai_insn_read(struct 
> > comedi_device *dev,
> > usbduxfast_cmd_data(dev, 6, 0x01, 0x00, rngmask, 0x00);
> > ret = usbduxfast_send_cmd(dev, SENDADCOMMANDS);
> > -   if (ret < 0) {
> > -   mutex_unlock(>mut);
> > -   return ret;
> > -   }
> > +   if (ret < 0)
> > +   goto unlock;
> > for (i = 0; i < PACKETS_TO_IGNORE; i++) {
> > ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, BULKINEP),
> > @@ -809,8 +807,7 @@ static int usbduxfast_ai_insn_read(struct comedi_device 
> > *dev,
> >_length, 1);
> > if (ret < 0) {
> > dev_err(dev->class_dev, "insn timeout, no data\n");
> > -   mutex_unlock(>mut);
> > -   return ret;
> > +   goto unlock;
> > }
> > }
> > @@ -820,14 +817,13 @@ static int usbduxfast_ai_insn_read(struct 
> > comedi_device *dev,
> >_length, 1);
> > if (ret < 0) {
> > dev_err(dev->class_dev, "insn data error: %d\n", ret);
> > -   mutex_unlock(>mut);
> > -   return ret;
> > +   goto unlock;
> > }
> > n = actual_length / sizeof(u16);
> > if ((n % 16) != 0) {
> > dev_err(dev->class_dev, "insn data packet corrupted\n");
> > -   mutex_unlock(>mut);
> > -   return -EINVAL;
> > +   ret = -EINVAL;
> > +   goto unlock;
> > }
> > for (j = chan; (j < n) && (i < insn->n); j = j + 16) {
> > data[i] = ((u16 *)(devpriv->inbuf))[j];
> > @@ -838,6 +834,10 @@ static int usbduxfast_ai_insn_read(struct 
> > comedi_device *dev,
> > mutex_unlock(>mut);
> > return insn->n;
> 
> Minor niggle: You could also remove that call to mutex_unlock() by replacing
> the above three lines with:
> 
>   ret = insn->n;
> 
> which will fall through to the 'unlock:' label below.

FYI, you are responding to someone who is on my blacklist and I never
accept patches from.  I wouldn't waste my time reviewing any patches
from them, sorry.

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: comedi: usbduxsigma: Improve unlocking of a mutex in three functions

2017-11-03 Thread Ian Abbott 

On 02/11/17 20:23, SF Markus Elfring wrote:

From: Markus Elfring 
Date: Thu, 2 Nov 2017 21:16:50 +0100

* Add a jump target so that a call of the function "mutex_unlock" is stored
   only twice in these function implementations.

* Replace seven calls by goto statements.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
  drivers/staging/comedi/drivers/usbduxsigma.c | 48 +++-
  1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c 
b/drivers/staging/comedi/drivers/usbduxsigma.c
index 456e9f13becb..7e8284ed265a 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -672,10 +672,8 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,
devpriv->dux_commands[8] = sysred;
  
  	ret = usbbuxsigma_send_cmd(dev, USBBUXSIGMA_AD_CMD);

-   if (ret < 0) {
-   mutex_unlock(>mut);
-   return ret;
-   }
+   if (ret < 0)
+   goto unlock;
  
  	devpriv->ai_counter = devpriv->ai_timer;
  
@@ -686,8 +684,7 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,

  devpriv->n_ai_urbs, 1);
if (ret < 0) {
devpriv->ai_cmd_running = 0;
-   mutex_unlock(>mut);
-   return ret;
+   goto unlock;
}
s->async->inttrig = NULL;
} else {/* TRIG_INT */
@@ -697,6 +694,10 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,
mutex_unlock(>mut);
  
  	return 0;


You could also remove that call by replacing the above three lines with:

ret = 0;

and falling through to the 'unlock:' label.  (Actually, you don't even 
need the above line, as ret should already be 0 at this point, but I 
think it is easier to understand the code if ret is set to 0 explicitly.)



+
+unlock:
+   mutex_unlock(>mut);
+   return ret;
  }
  
  static int usbduxsigma_ai_insn_read(struct comedi_device *dev,

@@ -714,8 +715,8 @@ static int usbduxsigma_ai_insn_read(struct comedi_device 
*dev,
  
  	mutex_lock(>mut);

if (devpriv->ai_cmd_running) {
-   mutex_unlock(>mut);
-   return -EBUSY;
+   ret = -EBUSY;
+   goto unlock;
}
  
  	create_adc_command(chan, , );

@@ -730,19 +731,15 @@ static int usbduxsigma_ai_insn_read(struct comedi_device 
*dev,
  
  	/* adc commands */

ret = usbbuxsigma_send_cmd(dev, USBDUXSIGMA_SINGLE_AD_CMD);
-   if (ret < 0) {
-   mutex_unlock(>mut);
-   return ret;
-   }
+   if (ret < 0)
+   goto unlock;
  
  	for (i = 0; i < insn->n; i++) {

u32 val;
  
  		ret = usbduxsigma_receive_cmd(dev, USBDUXSIGMA_SINGLE_AD_CMD);

-   if (ret < 0) {
-   mutex_unlock(>mut);
-   return ret;
-   }
+   if (ret < 0)
+   goto unlock;
  
  		/* 32 bits big endian from the A/D converter */

val = be32_to_cpu(get_unaligned((__be32
@@ -753,6 +750,10 @@ static int usbduxsigma_ai_insn_read(struct comedi_device 
*dev,
mutex_unlock(>mut);
  
  	return insn->n;


Similarly, you could replace the above three lines with:

ret = insn->n;

and fall through to the label.


+
+unlock:
+   mutex_unlock(>mut);
+   return ret;
  }
  
  static int usbduxsigma_ao_insn_read(struct comedi_device *dev,

@@ -782,8 +783,8 @@ static int usbduxsigma_ao_insn_write(struct comedi_device 
*dev,
  
  	mutex_lock(>mut);

if (devpriv->ao_cmd_running) {
-   mutex_unlock(>mut);
-   return -EBUSY;
+   ret = -EBUSY;
+   goto unlock;
}
  
  	for (i = 0; i < insn->n; i++) {

@@ -791,15 +792,18 @@ static int usbduxsigma_ao_insn_write(struct comedi_device 
*dev,
devpriv->dux_commands[2] = data[i];  /* value */
devpriv->dux_commands[3] = chan; /* channel number */
ret = usbbuxsigma_send_cmd(dev, USBDUXSIGMA_DA_CMD);
-   if (ret < 0) {
-   mutex_unlock(>mut);
-   return ret;
-   }
+   if (ret < 0)
+   goto unlock;
+
s->readback[chan] = data[i];
}
mutex_unlock(>mut);
  
  	return insn->n;


Ditto.


+
+unlock:
+   mutex_unlock(>mut);
+   return ret;
  }
  
  static int usbduxsigma_ao_inttrig(struct comedi_device *dev,





--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org

Re: [PATCH] staging: comedi: usbduxfast: Improve unlocking of a mutex in usbduxfast_ai_insn_read()

2017-11-03 Thread Ian Abbott 

On 02/11/17 19:40, SF Markus Elfring wrote:

From: Markus Elfring 
Date: Thu, 2 Nov 2017 20:30:31 +0100

* Add a jump target so that a call of the function "mutex_unlock" is stored
   only twice in this function implementation.

* Replace five calls by goto statements.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
  drivers/staging/comedi/drivers/usbduxfast.c | 24 
  1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/comedi/drivers/usbduxfast.c 
b/drivers/staging/comedi/drivers/usbduxfast.c
index 608403c7586b..e5884faf7275 100644
--- a/drivers/staging/comedi/drivers/usbduxfast.c
+++ b/drivers/staging/comedi/drivers/usbduxfast.c
@@ -777,8 +777,8 @@ static int usbduxfast_ai_insn_read(struct comedi_device 
*dev,
if (devpriv->ai_cmd_running) {
dev_err(dev->class_dev,
"ai_insn_read not possible, async cmd is running\n");
-   mutex_unlock(>mut);
-   return -EBUSY;
+   ret = -EBUSY;
+   goto unlock;
}
  
  	/* set command for the first channel */

@@ -798,10 +798,8 @@ static int usbduxfast_ai_insn_read(struct comedi_device 
*dev,
usbduxfast_cmd_data(dev, 6, 0x01, 0x00, rngmask, 0x00);
  
  	ret = usbduxfast_send_cmd(dev, SENDADCOMMANDS);

-   if (ret < 0) {
-   mutex_unlock(>mut);
-   return ret;
-   }
+   if (ret < 0)
+   goto unlock;
  
  	for (i = 0; i < PACKETS_TO_IGNORE; i++) {

ret = usb_bulk_msg(usb, usb_rcvbulkpipe(usb, BULKINEP),
@@ -809,8 +807,7 @@ static int usbduxfast_ai_insn_read(struct comedi_device 
*dev,
   _length, 1);
if (ret < 0) {
dev_err(dev->class_dev, "insn timeout, no data\n");
-   mutex_unlock(>mut);
-   return ret;
+   goto unlock;
}
}
  
@@ -820,14 +817,13 @@ static int usbduxfast_ai_insn_read(struct comedi_device *dev,

   _length, 1);
if (ret < 0) {
dev_err(dev->class_dev, "insn data error: %d\n", ret);
-   mutex_unlock(>mut);
-   return ret;
+   goto unlock;
}
n = actual_length / sizeof(u16);
if ((n % 16) != 0) {
dev_err(dev->class_dev, "insn data packet corrupted\n");
-   mutex_unlock(>mut);
-   return -EINVAL;
+   ret = -EINVAL;
+   goto unlock;
}
for (j = chan; (j < n) && (i < insn->n); j = j + 16) {
data[i] = ((u16 *)(devpriv->inbuf))[j];
@@ -838,6 +834,10 @@ static int usbduxfast_ai_insn_read(struct comedi_device 
*dev,
mutex_unlock(>mut);
  
  	return insn->n;


Minor niggle: You could also remove that call to mutex_unlock() by 
replacing the above three lines with:


ret = insn->n;

which will fall through to the 'unlock:' label below.


+
+unlock:
+   mutex_unlock(>mut);
+   return ret;
  }
  
  static int usbduxfast_upload_firmware(struct comedi_device *dev,





--
-=( Ian Abbott @ MEV Ltd.E-mail:  )=-
-=(  Web: http://www.mev.co.uk/  )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: fbtft: remove redundant initialization of buf

2017-11-03 Thread Colin King 
From: Colin Ian King 

The pointer buf is being set on each iteration of a for-loop and
so the initialization of buf at declaration time is redundant and
can be removed.  Cleans up clang warning:

drivers/staging/fbtft/fb_uc1701.c:130:6: warning: Value stored
to 'buf' during its initialization is never read

Signed-off-by: Colin Ian King 
---
 drivers/staging/fbtft/fb_uc1701.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/fbtft/fb_uc1701.c 
b/drivers/staging/fbtft/fb_uc1701.c
index b78045fe5393..78899a172c7e 100644
--- a/drivers/staging/fbtft/fb_uc1701.c
+++ b/drivers/staging/fbtft/fb_uc1701.c
@@ -127,7 +127,7 @@ static void set_addr_win(struct fbtft_par *par, int xs, int 
ys, int xe, int ye)
 static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
 {
u16 *vmem16 = (u16 *)par->info->screen_buffer;
-   u8 *buf = par->txbuf.buf;
+   u8 *buf;
int x, y, i;
int ret = 0;
 
-- 
2.14.1

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: lustre: Replaces 'uint32_t' with '__u32' and 'uint64_t' with '__u64'.

2017-11-03 Thread Roman Storozhenko 
On Fri, Nov 03, 2017 at 12:46:18PM +0100, Greg Kroah-Hartman wrote:
> On Sun, Oct 29, 2017 at 08:58:39PM +0300, Roman Storozhenko wrote:
> > There are two reasons for that:
> > 1) As Linus Torvalds said we should use kernel types:
> > http://lkml.iu.edu/hypermail//linux/kernel/1506.0/00160.html
> > 
> > 2) There are only few places in the lustre codebase that use such types.
> > In the most cases it uses '__u32' and '__u64'.
> 
> The __ types are only needed for when you cross the user/kernel boundry.
> Otherwise just use the "normal" types of u32 and u64.
> 
> Do the changes you made here all cross that boundry?  If not, please fix
> this up.

Thanks, Greg.

I have checked lustre repository and it seems that changed ".h" files aren't 
used in client code. But I realise that I could be mistaken. That why I want to 
ask lustre guys: am I right?

> 
> thanks,
> 
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] tools/hv: add install target to Makefile

2017-11-03 Thread Vitaly Kuznetsov 
Makefiles usually come with 'install' target included so each distro
doesn't need to implement the procedure from scratch. Add it to tools/hv.

Signed-off-by: Vitaly Kuznetsov 
---
 tools/hv/Makefile | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/tools/hv/Makefile b/tools/hv/Makefile
index 0d1e61b81844..e8f0a0a691d3 100644
--- a/tools/hv/Makefile
+++ b/tools/hv/Makefile
@@ -6,9 +6,30 @@ CFLAGS = $(WARNINGS) -g $(shell getconf LFS_CFLAGS)
 
 CFLAGS += -D__EXPORTED_HEADERS__ -I../../include/uapi -I../../include
 
-all: hv_kvp_daemon hv_vss_daemon hv_fcopy_daemon
+sbindir ?= /usr/sbin
+libexecdir ?= /usr/libexec
+sharedstatedir ?= /var/lib
+
+ALL_PROGRAMS := hv_kvp_daemon hv_vss_daemon hv_fcopy_daemon
+
+ALL_SCRIPTS := hv_get_dhcp_info.sh hv_get_dns_info.sh hv_set_ifconfig.sh
+
+all: $(ALL_PROGRAMS)
+
 %: %.c
$(CC) $(CFLAGS) -o $@ $^
 
 clean:
$(RM) hv_kvp_daemon hv_vss_daemon hv_fcopy_daemon
+
+install: all
+   install -d -m 755 $(DESTDIR)$(sbindir); \
+   install -d -m 755 $(DESTDIR)$(libexecdir)/hypervkvpd; \
+   install -d -m 755 $(DESTDIR)$(sharedstatedir); \
+   for program in $(ALL_PROGRAMS); do \
+   install $$program -m 755 $(DESTDIR)$(sbindir);  \
+   done; \
+   install -m 755 lsvmbus $(DESTDIR)$(sbindir); \
+   for script in $(ALL_SCRIPTS); do \
+   install $$script -m 755 
$(DESTDIR)$(libexecdir)/hypervkvpd/$${script%.sh}; \
+   done
-- 
2.13.6

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: lustre: Replaces 'uint32_t' with '__u32' and 'uint64_t' with '__u64'.

2017-11-03 Thread Greg Kroah-Hartman 
On Sun, Oct 29, 2017 at 08:58:39PM +0300, Roman Storozhenko wrote:
> There are two reasons for that:
> 1) As Linus Torvalds said we should use kernel types:
> http://lkml.iu.edu/hypermail//linux/kernel/1506.0/00160.html
> 
> 2) There are only few places in the lustre codebase that use such types.
> In the most cases it uses '__u32' and '__u64'.

The __ types are only needed for when you cross the user/kernel boundry.
Otherwise just use the "normal" types of u32 and u64.

Do the changes you made here all cross that boundry?  If not, please fix
this up.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 1/2] staging: greybus: remove unused kfifo_ts

2017-11-03 Thread Viresh Kumar 
On 02-11-17, 15:32, Arnd Bergmann wrote:
> As of commit 8e1d6c336d74 ("greybus: loopback: drop bus aggregate
> calculation"), nothing ever reads from kfifo_ts, so there is no
> reason to write to it or even allocate it any more.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/staging/greybus/loopback.c | 27 +++
>  1 file changed, 3 insertions(+), 24 deletions(-)

Reviewed-by: Viresh Kumar 

-- 
viresh
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 2/2] staging: greybus/loopback: use ktime_get() for time intervals

2017-11-03 Thread Bryan O'Donoghue 

On 02/11/17 14:32, Arnd Bergmann wrote:

This driver is the only one using the deprecated timeval_to_ns()
helper. Changing it from do_gettimeofday() to ktime_get() makes
the code more efficient, more robust against concurrent
settimeofday(), more accurate and lets us get rid of that helper
in the future.

Signed-off-by: Arnd Bergmann 
---
  drivers/staging/greybus/loopback.c | 42 --
  1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c 
b/drivers/staging/greybus/loopback.c
index 85046fb16aad..3d92638c424b 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -58,7 +58,7 @@ static struct gb_loopback_device gb_dev;
  struct gb_loopback_async_operation {
struct gb_loopback *gb;
struct gb_operation *operation;
-   struct timeval ts;
+   ktime_t ts;
struct timer_list timer;
struct list_head entry;
struct work_struct work;
@@ -81,7 +81,7 @@ struct gb_loopback {
atomic_t outstanding_operations;
  
  	/* Per connection stats */

-   struct timeval ts;
+   ktime_t ts;
struct gb_loopback_stats latency;
struct gb_loopback_stats throughput;
struct gb_loopback_stats requests_per_second;
@@ -375,14 +375,9 @@ static u64 __gb_loopback_calc_latency(u64 t1, u64 t2)
return NSEC_PER_DAY - t2 + t1;
  }
  
-static u64 gb_loopback_calc_latency(struct timeval *ts, struct timeval *te)

+static u64 gb_loopback_calc_latency(ktime_t ts, ktime_t te)
  {
-   u64 t1, t2;
-
-   t1 = timeval_to_ns(ts);
-   t2 = timeval_to_ns(te);
-
-   return __gb_loopback_calc_latency(t1, t2);
+   return __gb_loopback_calc_latency(ktime_to_ns(ts), ktime_to_ns(te));
  }
  
  static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,

@@ -390,10 +385,10 @@ static int gb_loopback_operation_sync(struct gb_loopback 
*gb, int type,
  void *response, int response_size)
  {
struct gb_operation *operation;
-   struct timeval ts, te;
+   ktime_t ts, te;
int ret;
  
-	do_gettimeofday();

+   ts = ktime_get();
operation = gb_operation_create(gb->connection, type, request_size,
response_size, GFP_KERNEL);
if (!operation)
@@ -421,10 +416,10 @@ static int gb_loopback_operation_sync(struct gb_loopback 
*gb, int type,
}
}
  
-	do_gettimeofday();

+   te = ktime_get();
  
  	/* Calculate the total time the message took */

-   gb->elapsed_nsecs = gb_loopback_calc_latency(, );
+   gb->elapsed_nsecs = gb_loopback_calc_latency(ts, te);
  
  out_put_operation:

gb_operation_put(operation);
@@ -492,10 +487,10 @@ static void gb_loopback_async_operation_callback(struct 
gb_operation *operation)
  {
struct gb_loopback_async_operation *op_async;
struct gb_loopback *gb;
-   struct timeval te;
+   ktime_t te;
bool err = false;
  
-	do_gettimeofday();

+   te = ktime_get();
op_async = gb_loopback_operation_find(operation->id);
if (!op_async)
return;
@@ -512,8 +507,7 @@ static void gb_loopback_async_operation_callback(struct 
gb_operation *operation)
}
  
  	if (!err)

-   gb->elapsed_nsecs = gb_loopback_calc_latency(_async->ts,
-);
+   gb->elapsed_nsecs = gb_loopback_calc_latency(op_async->ts, te);
  
  	if (op_async->pending) {

if (err)
@@ -608,7 +602,7 @@ static int gb_loopback_async_operation(struct gb_loopback 
*gb, int type,
list_add_tail(_async->entry, _dev.list_op_async);
spin_unlock_irqrestore(_dev.lock, flags);
  
-	do_gettimeofday(_async->ts);

+   op_async->ts = ktime_get();
op_async->pending = true;
atomic_inc(>outstanding_operations);
mutex_lock(>mutex);
@@ -842,7 +836,7 @@ static void gb_loopback_reset_stats(struct gb_loopback *gb)
/* Should be initialized at least once per transaction set */
gb->apbridge_latency_ts = 0;
gb->gbphy_latency_ts = 0;
-   memset(>ts, 0, sizeof(struct timeval));
+   gb->ts = ktime_set(0, 0);
  }
  
  static void gb_loopback_update_stats(struct gb_loopback_stats *stats, u32 val)

@@ -925,15 +919,15 @@ static void gb_loopback_calculate_stats(struct 
gb_loopback *gb, bool error)
  {
u64 nlat;
u32 lat;
-   struct timeval te;
+   ktime_t te;
  
  	if (!error) {

gb->requests_completed++;
gb_loopback_calculate_latency_stats(gb);
}
  
-	do_gettimeofday();

-   nlat = gb_loopback_calc_latency(>ts, );
+   te = ktime_get();
+   nlat = gb_loopback_calc_latency(gb->ts, te);
if (nlat >= NSEC_PER_SEC || gb->iteration_count == gb->iteration_max) {
lat =

Re: [PATCH 1/2] staging: greybus: remove unused kfifo_ts

2017-11-03 Thread Bryan O'Donoghue 

On 02/11/17 14:32, Arnd Bergmann wrote:

As of commit 8e1d6c336d74 ("greybus: loopback: drop bus aggregate
calculation"), nothing ever reads from kfifo_ts, so there is no
reason to write to it or even allocate it any more.

Signed-off-by: Arnd Bergmann 
---
  drivers/staging/greybus/loopback.c | 27 +++
  1 file changed, 3 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/greybus/loopback.c 
b/drivers/staging/greybus/loopback.c
index 08e255884206..85046fb16aad 100644
--- a/drivers/staging/greybus/loopback.c
+++ b/drivers/staging/greybus/loopback.c
@@ -72,7 +72,6 @@ struct gb_loopback {
  
  	struct dentry *file;

struct kfifo kfifo_lat;
-   struct kfifo kfifo_ts;
struct mutex mutex;
struct task_struct *task;
struct list_head entry;
@@ -262,7 +261,6 @@ static void gb_loopback_check_attr(struct gb_loopback *gb)
 gb->iteration_max, kfifo_depth);
}
kfifo_reset_out(>kfifo_lat);
-   kfifo_reset_out(>kfifo_ts);
  
  	switch (gb->type) {

case GB_LOOPBACK_TYPE_PING:
@@ -387,13 +385,6 @@ static u64 gb_loopback_calc_latency(struct timeval *ts, 
struct timeval *te)
return __gb_loopback_calc_latency(t1, t2);
  }
  
-static void gb_loopback_push_latency_ts(struct gb_loopback *gb,

-   struct timeval *ts, struct timeval *te)
-{
-   kfifo_in(>kfifo_ts, (unsigned char *)ts, sizeof(*ts));
-   kfifo_in(>kfifo_ts, (unsigned char *)te, sizeof(*te));
-}
-
  static int gb_loopback_operation_sync(struct gb_loopback *gb, int type,
  void *request, int request_size,
  void *response, int response_size)
@@ -433,7 +424,6 @@ static int gb_loopback_operation_sync(struct gb_loopback 
*gb, int type,
do_gettimeofday();
  
  	/* Calculate the total time the message took */

-   gb_loopback_push_latency_ts(gb, , );
gb->elapsed_nsecs = gb_loopback_calc_latency(, );
  
  out_put_operation:

@@ -521,11 +511,9 @@ static void gb_loopback_async_operation_callback(struct 
gb_operation *operation)
err = true;
}
  
-	if (!err) {

-   gb_loopback_push_latency_ts(gb, _async->ts, );
+   if (!err)
gb->elapsed_nsecs = gb_loopback_calc_latency(_async->ts,
 );
-   }
  
  	if (op_async->pending) {

if (err)
@@ -1241,18 +1229,12 @@ static int gb_loopback_probe(struct gb_bundle *bundle,
retval = -ENOMEM;
goto out_conn;
}
-   if (kfifo_alloc(>kfifo_ts, kfifo_depth * sizeof(struct timeval) * 2,
- GFP_KERNEL)) {
-   retval = -ENOMEM;
-   goto out_kfifo0;
-   }
-
/* Fork worker thread */
mutex_init(>mutex);
gb->task = kthread_run(gb_loopback_fn, gb, "gb_loopback");
if (IS_ERR(gb->task)) {
retval = PTR_ERR(gb->task);
-   goto out_kfifo1;
+   goto out_kfifo;
}
  
  	spin_lock_irqsave(_dev.lock, flags);

@@ -1266,9 +1248,7 @@ static int gb_loopback_probe(struct gb_bundle *bundle,
  
  	return 0;
  
-out_kfifo1:

-   kfifo_free(>kfifo_ts);
-out_kfifo0:
+out_kfifo:
kfifo_free(>kfifo_lat);
  out_conn:
device_unregister(dev);
@@ -1302,7 +1282,6 @@ static void gb_loopback_disconnect(struct gb_bundle 
*bundle)
kthread_stop(gb->task);
  
  	kfifo_free(>kfifo_lat);

-   kfifo_free(>kfifo_ts);
gb_connection_latency_tag_disable(gb->connection);
debugfs_remove(gb->file);
  



This looks right to me

Reviewed-by: Bryan O'Donoghue 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging/media/davinci_vpfe: Use common error handling code in vpfe_attach_irq()

2017-11-03 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Fri, 3 Nov 2017 10:45:31 +0100

Add a jump target so that a bit of exception handling can be better reused
at the end of this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c 
b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
index bffe2153b910..80297d2df31d 100644
--- a/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
+++ b/drivers/staging/media/davinci_vpfe/vpfe_mc_capture.c
@@ -309,8 +309,7 @@ static int vpfe_attach_irq(struct vpfe_device *vpfe_dev)
if (ret < 0) {
v4l2_err(_dev->v4l2_dev,
"Error: requesting VINT1 interrupt\n");
-   free_irq(vpfe_dev->ccdc_irq0, vpfe_dev);
-   return ret;
+   goto free_irq;
}
 
ret = request_irq(vpfe_dev->imp_dma_irq, vpfe_imp_dma_isr,
@@ -319,11 +318,14 @@ static int vpfe_attach_irq(struct vpfe_device *vpfe_dev)
v4l2_err(_dev->v4l2_dev,
 "Error: requesting IMP IRQ interrupt\n");
free_irq(vpfe_dev->ccdc_irq1, vpfe_dev);
-   free_irq(vpfe_dev->ccdc_irq0, vpfe_dev);
-   return ret;
+   goto free_irq;
}
 
return 0;
+
+free_irq:
+   free_irq(vpfe_dev->ccdc_irq0, vpfe_dev);
+   return ret;
 }
 
 /*
-- 
2.15.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: iio: ad7746: Improve unlocking of a mutex in ad7746_start_calib()

2017-11-03 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Fri, 3 Nov 2017 09:26:28 +0100

* Add a jump target so that a call of the function "mutex_unlock" is stored
  only twice in this function implementation.

* Replace two calls by goto statements.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/staging/iio/cdc/ad7746.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7746.c b/drivers/staging/iio/cdc/ad7746.c
index a124853a05f0..c4a864725376 100644
--- a/drivers/staging/iio/cdc/ad7746.c
+++ b/drivers/staging/iio/cdc/ad7746.c
@@ -302,23 +302,24 @@ static inline ssize_t ad7746_start_calib(struct device 
*dev,
mutex_lock(>lock);
regval |= chip->config;
ret = i2c_smbus_write_byte_data(chip->client, AD7746_REG_CFG, regval);
-   if (ret < 0) {
-   mutex_unlock(>lock);
-   return ret;
-   }
+   if (ret < 0)
+   goto unlock;
 
do {
msleep(20);
ret = i2c_smbus_read_byte_data(chip->client, AD7746_REG_CFG);
-   if (ret < 0) {
-   mutex_unlock(>lock);
-   return ret;
-   }
+   if (ret < 0)
+   goto unlock;
+
} while ((ret == regval) && timeout--);
 
mutex_unlock(>lock);
 
return len;
+
+unlock:
+   mutex_unlock(>lock);
+   return ret;
 }
 
 static ssize_t ad7746_start_offset_calib(struct device *dev,
-- 
2.15.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: iio: ad7152: Improve unlocking of a mutex in ad7152_start_calib()

2017-11-03 Thread SF Markus Elfring 
From: Markus Elfring 
Date: Fri, 3 Nov 2017 09:00:25 +0100

* Add a jump target so that a call of the function "mutex_unlock" is stored
  only twice in this function implementation.

* Replace two calls by goto statements.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 drivers/staging/iio/cdc/ad7152.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
index 61377ca444de..59d1b35f6a4e 100644
--- a/drivers/staging/iio/cdc/ad7152.c
+++ b/drivers/staging/iio/cdc/ad7152.c
@@ -118,22 +118,23 @@ static inline ssize_t ad7152_start_calib(struct device 
*dev,
 
mutex_lock(>state_lock);
ret = i2c_smbus_write_byte_data(chip->client, AD7152_REG_CFG, regval);
-   if (ret < 0) {
-   mutex_unlock(>state_lock);
-   return ret;
-   }
+   if (ret < 0)
+   goto unlock;
 
do {
mdelay(20);
ret = i2c_smbus_read_byte_data(chip->client, AD7152_REG_CFG);
-   if (ret < 0) {
-   mutex_unlock(>state_lock);
-   return ret;
-   }
+   if (ret < 0)
+   goto unlock;
+
} while ((ret == regval) && timeout--);
 
mutex_unlock(>state_lock);
return len;
+
+unlock:
+   mutex_unlock(>state_lock);
+   return ret;
 }
 
 static ssize_t ad7152_start_offset_calib(struct device *dev,
-- 
2.15.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel