Re: [PATCH v3] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap

[Alexey Skidanov]

On 01/31/2018 03:00 PM, Greg KH wrote:

On Wed, Jan 31, 2018 at 02:03:42PM +0200, Alexey Skidanov wrote:

Any driver may access shared buffers, created by ion, using dma_buf_vmap and
dma_buf_vunmap dma-buf API that maps/unmaps previosuly allocated buffers into
the kernel virtual address space. The implementation of these API is missing in
the current ion implementation.

Signed-off-by: Alexey Skidanov 
---


No review from any other Intel developers? :(

Will add.


Anyway, what in-tree driver needs access to these functions?
I'm not sure that there are the in-tree drivers using these functions 
and ion as buffer exporter because they are not implemented in ion :) 
But there are some in-tree drivers using these APIs (gpu drivers) with 
other buffer exporters.


And are you sure that you don't need to do any "real" logic in the
vmap/vunmap calls?  That feels like there would be some reference
counting problems here.
dma_buf_start_cpu_access is called before the call to dma_buf_vmap. It 
actually increments the reference count. dma_buf_end_cpu_access is 
called after the dma_buf_vunmap. It actually decrements the reference 
count.>>

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index f480885..4f1dc7f 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -327,6 +327,17 @@ static void ion_dma_buf_kunmap(struct dma_buf *dmabuf, 
unsigned long offset,
  {
  }
  
+static void *ion_dma_buf_vmap(struct dma_buf *dmabuf)

+{
+   struct ion_buffer *buffer = dmabuf->priv;
+
+   return buffer->vaddr;


Just call ion_dma_buf_kmap(dmabuf, 0)?

Sure.


Again, please get this reviewed by someone else in Intel first.  Don't
ignore the resources you have, to do so would be foolish :)

Sure. Will do.


thanks,

greg k-h



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

[PATCH] Staging: fwserial: remove unnecessary parenthesis

[Quytelda Kahja]

Removed parenthesis causing a coding style warning.

Signed-off-by: Quytelda Kahja 
---
 drivers/staging/fwserial/dma_fifo.c | 2 +-
 drivers/staging/fwserial/fwserial.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/fwserial/dma_fifo.c 
b/drivers/staging/fwserial/dma_fifo.c
index 8b23a553fd4a..ead71f93f3c9 100644
--- a/drivers/staging/fwserial/dma_fifo.c
+++ b/drivers/staging/fwserial/dma_fifo.c
@@ -259,7 +259,7 @@ int dma_fifo_out_complete(struct dma_fifo *fifo, struct 
dma_pending *complete)
if (list_empty(>pending) && fifo->open == 0)
return -EINVAL;
 
-   if (FAIL(fifo, list_empty(>pending) != (fifo->open == 0),
+   if (FAIL(fifo, list_empty(>pending) != fifo->open == 0,
 "pending list disagrees with open count:%d",
 fifo->open))
return -ENXIO;
diff --git a/drivers/staging/fwserial/fwserial.c 
b/drivers/staging/fwserial/fwserial.c
index bba7e9c888b3..99cdefc873e5 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -1296,7 +1296,7 @@ static void fwtty_set_termios(struct tty_struct *tty, 
struct ktermios *old)
spin_lock_bh(>lock);
baud = set_termios(port, tty);
 
-   if ((baud == 0) && (old->c_cflag & CBAUD)) {
+   if (baud == 0 && (old->c_cflag & CBAUD)) {
port->mctrl &= ~(TIOCM_DTR | TIOCM_RTS);
} else if ((baud != 0) && !(old->c_cflag & CBAUD)) {
if (C_CRTSCTS(tty) || !tty_throttled(tty))
-- 
2.16.1

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

[PATCH resend 3/3] staging: android: ion: Nuke ion_page_pool_init

[Yisheng Xie]

ion_page_pool.c now is used to apply pool APIs for system heap, which do
not need do any initial at device_initcall. Therefore ion_page_pool_init
can be nuked.

Signed-off-by: Yisheng Xie 
---
 drivers/staging/android/ion/ion_page_pool.c | 7 ---
 1 file changed, 7 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c 
b/drivers/staging/android/ion/ion_page_pool.c
index f2318c9..3c1eba5 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -15,7 +15,6 @@
  */
 
 #include 
-#include 
 #include 
 #include 
 
@@ -167,9 +166,3 @@ void ion_page_pool_destroy(struct ion_page_pool *pool)
 {
kfree(pool);
 }
-
-static int __init ion_page_pool_init(void)
-{
-   return 0;
-}
-device_initcall(ion_page_pool_init);
-- 
1.7.12.4

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

[PATCH resend 1/3] staging: android: ion: Remove unused declaration ion_buffer_fault_user_mappings

[Yisheng Xie]

ion_buffer_fault_user_mappings's definition has been removed and not be
used anymore, just remove its useless declaration.

Signed-off-by: Yisheng Xie 
---
 drivers/staging/android/ion/ion.h | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/staging/android/ion/ion.h 
b/drivers/staging/android/ion/ion.h
index f5f9cd6..2160c35 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -201,15 +201,6 @@ struct ion_heap {
 bool ion_buffer_cached(struct ion_buffer *buffer);
 
 /**
- * ion_buffer_fault_user_mappings - fault in user mappings of this buffer
- * @buffer:buffer
- *
- * indicates whether userspace mappings of this buffer will be faulted
- * in, this can affect how buffers are allocated from the heap.
- */
-bool ion_buffer_fault_user_mappings(struct ion_buffer *buffer);
-
-/**
  * ion_device_add_heap - adds a heap to the ion device
  * @heap:  the heap to add
  */
-- 
1.7.12.4

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

[PATCH resend 2/3] staging: android: ion: Remove unused include files for ion_page_pool.c

[Yisheng Xie]

After rewrite of ion_page_pool, some of its include file is no need
anymore, just remove it.

Signed-off-by: Yisheng Xie 
---
 drivers/staging/android/ion/ion_page_pool.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c 
b/drivers/staging/android/ion/ion_page_pool.c
index 817849d..f2318c9 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -14,10 +14,6 @@
  *
  */
 
-#include 
-#include 
-#include 
-#include 
 #include 
 #include 
 #include 
-- 
1.7.12.4

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

Re: [PATCH] staging: android: ion: minor cleanup for ion_page_pool

[Yisheng Xie]

Hi Greg,

On 2018/1/31 19:32, Greg KH wrote:
> On Wed, Jan 31, 2018 at 05:50:52PM +0800, Yisheng Xie wrote:
>> ion_page_pool.c now is used to apply pool APIs for system heap, which do
>> not need do any initial at device_initcall.
>>
>> Meanwhile, this patch also remove some useless include files.
> 
> Please only do one thing per patch, otherwise if one of the different
> things you do has a problem, all of the changes have been reverted.
> 
> Yeah, it's a pain at times, but that's the way kernel development is,
> sorry.

Sorry, I should always remember this.

> 
> Please break this up into two patches and resend.  I will be glad to
> queue them up after the merge window ends when 4.16-rc1 is released.

I will sent the next version ASAP.

Thanks
Yisheng
> 
> thanks,
> 
> greg k-h
> 
> .
> 

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

Re: [RFC PATCH 1/2] hv_netvsc: Split netvsc_revoke_buf() and netvsc_teardown_gpadl()

[Stephen Hemminger]

On Wed, 31 Jan 2018 12:16:49 +0100
Mohammed Gamal  wrote:

> On Tue, 2018-01-30 at 11:29 -0800, Stephen Hemminger wrote:
> > On Tue, 23 Jan 2018 10:34:04 +0100
> > Mohammed Gamal  wrote:
> >   
> > > Split each of the functions into two for each of send/recv buffers
> > > 
> > > Signed-off-by: Mohammed Gamal   
> > 
> > Splitting these functions is not necessary  
> 
> How so? We need to send each message independently, and hence the split
> (see cover letter). Is there another way?

This is all that is needed.


Subject: [PATCH] hv_netvsc: work around for gpadl teardown on older windows
 server

On WS2012 the host ignores messages after vmbus channel is closed.
Workaround this by doing what Windows does and send the teardown
before close on older versions of NVSP protocol.

Reported-by: Mohammed Gamal 
Fixes: 0cf737808ae7 ("hv_netvsc: netvsc_teardown_gpadl() split")
Signed-off-by: Stephen Hemminger 
---
 drivers/net/hyperv/netvsc.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/hyperv/netvsc.c b/drivers/net/hyperv/netvsc.c
index 17e529af79dc..1a3df0eff42f 100644
--- a/drivers/net/hyperv/netvsc.c
+++ b/drivers/net/hyperv/netvsc.c
@@ -574,10 +574,17 @@ void netvsc_device_remove(struct hv_device *device)
 */
netdev_dbg(ndev, "net device safe to remove\n");
 
+   /* Workaround for older versions of Windows require that
+* buffer be revoked before channel is disabled
+*/
+   if (net_device->nvsp_version < NVSP_PROTOCOL_VERSION_4)
+   netvsc_teardown_gpadl(device, net_device);
+
/* Now, we can close the channel safely */
vmbus_close(device->channel);
 
-   netvsc_teardown_gpadl(device, net_device);
+   if (net_device->nvsp_version >= NVSP_PROTOCOL_VERSION_4)
+   netvsc_teardown_gpadl(device, net_device);
 
/* And dissassociate NAPI context from device */
for (i = 0; i < net_device->num_chn; i++)
-- 
2.15.1

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

RE: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking a channel for I/O requests

[Long Li]

> Subject: RE: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking a
> channel for I/O requests
> 
> > From: Long Li
> > Sent: Wednesday, January 31, 2018 12:23 PM
> > To: Michael Kelley (EOSG) ; KY
> > Srinivasan ; Stephen Hemminger
> > ; martin.peter...@oracle.com;
> > de...@linuxdriverproject.org; linux-ker...@vger.kernel.org;
> > linux-s...@vger.kernel.org; James E . J . Bottomley
> > 
> > Subject: RE: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking
> > a channel for I/O requests
> >
> > > Subject: RE: [PATCH 1/1] scsi: storvsc: Spread interrupts when
> > > picking a channel for I/O requests
> > >
> > > Updated/corrected two email addresses ...
> > >
> > > > -Original Message-
> > > > From: Michael Kelley (EOSG)
> > > > Sent: Wednesday, January 24, 2018 2:14 PM
> > > > To: KY Srinivasan ; Stephen Hemminger
> > > > ; martin.peter...@oracle.com;
> > > > lo...@microsoft.com; jbottom...@odin.com;
> > > > de...@linuxdriverproject.org; linux-ker...@vger.kernel.org;
> > > > linux-s...@vger.kernel.org
> > > > Cc: Michael Kelley (EOSG) 
> > > > Subject: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking
> > > > a channel for I/O requests
> > > >
> > > > Update the algorithm in storvsc_do_io to look for a channel
> > > > starting with the current CPU + 1 and wrap around (within the
> > > > current NUMA node). This spreads VMbus interrupts more evenly
> > > > across CPUs. Previous code always started with first CPU in the
> > > > current NUMA node, skewing the interrupt load to that CPU.
> > > >
> > > > Signed-off-by: Michael Kelley 

Reviewed-by: Long Li 

> > > > ---
> > > >  drivers/scsi/storvsc_drv.c | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/scsi/storvsc_drv.c
> > > > b/drivers/scsi/storvsc_drv.c index e07907d..f3264c4 100644
> > > > --- a/drivers/scsi/storvsc_drv.c
> > > > +++ b/drivers/scsi/storvsc_drv.c
> > > > @@ -1310,7 +1310,8 @@ static int storvsc_do_io(struct hv_device
> *device,
> > > >  */
> > > > cpumask_and(_mask, _device-
> alloced_cpus,
> > > >
> > > cpumask_of_node(cpu_to_node(q_num)));
> > > > -   for_each_cpu(tgt_cpu, _mask) {
> > > > +   for_each_cpu_wrap(tgt_cpu, _mask,
> > > > +   outgoing_channel->target_cpu + 
> > > > 1) {
> >
> > Does it work when target_cpu is the last CPU on the system?
> >
> > Otherwise, looking good.
> 
> Yes, it works.  for_each_cpu_wrap() correctly wraps in the case where the
> 3rd parameter ('start') is one past the end of the mask.  Arguably, we
> shouldn't rely on that, and should do the wrap to 0 before calling
> for_each_cpu_wrap().
> 
> >
> > > > if (tgt_cpu != 
> > > > outgoing_channel->target_cpu)
> > > {
> > > > outgoing_channel =
> > > > stor_device->stor_chns[tgt_cpu];
> > > > --
> > > > 1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking a channel for I/O requests

[Michael Kelley (EOSG)]

> From: Long Li
> Sent: Wednesday, January 31, 2018 12:23 PM
> To: Michael Kelley (EOSG) ; KY Srinivasan
> ; Stephen Hemminger ;
> martin.peter...@oracle.com; de...@linuxdriverproject.org; 
> linux-ker...@vger.kernel.org;
> linux-s...@vger.kernel.org; James E . J . Bottomley 
> Subject: RE: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking a 
> channel for I/O
> requests
> 
> > Subject: RE: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking a
> > channel for I/O requests
> >
> > Updated/corrected two email addresses ...
> >
> > > -Original Message-
> > > From: Michael Kelley (EOSG)
> > > Sent: Wednesday, January 24, 2018 2:14 PM
> > > To: KY Srinivasan ; Stephen Hemminger
> > > ; martin.peter...@oracle.com;
> > > lo...@microsoft.com; jbottom...@odin.com;
> > > de...@linuxdriverproject.org; linux-ker...@vger.kernel.org;
> > > linux-s...@vger.kernel.org
> > > Cc: Michael Kelley (EOSG) 
> > > Subject: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking a
> > > channel for I/O requests
> > >
> > > Update the algorithm in storvsc_do_io to look for a channel starting
> > > with the current CPU + 1 and wrap around (within the current NUMA
> > > node). This spreads VMbus interrupts more evenly across CPUs. Previous
> > > code always started with first CPU in the current NUMA node, skewing
> > > the interrupt load to that CPU.
> > >
> > > Signed-off-by: Michael Kelley 
> > > ---
> > >  drivers/scsi/storvsc_drv.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > > index e07907d..f3264c4 100644
> > > --- a/drivers/scsi/storvsc_drv.c
> > > +++ b/drivers/scsi/storvsc_drv.c
> > > @@ -1310,7 +1310,8 @@ static int storvsc_do_io(struct hv_device *device,
> > >*/
> > >   cpumask_and(_mask, _device-
> > >alloced_cpus,
> > >
> > cpumask_of_node(cpu_to_node(q_num)));
> > > - for_each_cpu(tgt_cpu, _mask) {
> > > + for_each_cpu_wrap(tgt_cpu, _mask,
> > > + outgoing_channel->target_cpu + 1) {
> 
> Does it work when target_cpu is the last CPU on the system?
> 
> Otherwise, looking good.

Yes, it works.  for_each_cpu_wrap() correctly wraps in the case where
the 3rd parameter ('start') is one past the end of the mask.  Arguably,
we shouldn't rely on that, and should do the wrap to 0 before calling
for_each_cpu_wrap().

> 
> > >   if (tgt_cpu != outgoing_channel->target_cpu)
> > {
> > >   outgoing_channel =
> > >   stor_device->stor_chns[tgt_cpu];
> > > --
> > > 1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking a channel for I/O requests

[Long Li]

> Subject: RE: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking a
> channel for I/O requests
> 
> Updated/corrected two email addresses ...
> 
> > -Original Message-
> > From: Michael Kelley (EOSG)
> > Sent: Wednesday, January 24, 2018 2:14 PM
> > To: KY Srinivasan ; Stephen Hemminger
> > ; martin.peter...@oracle.com;
> > lo...@microsoft.com; jbottom...@odin.com;
> > de...@linuxdriverproject.org; linux-ker...@vger.kernel.org;
> > linux-s...@vger.kernel.org
> > Cc: Michael Kelley (EOSG) 
> > Subject: [PATCH 1/1] scsi: storvsc: Spread interrupts when picking a
> > channel for I/O requests
> >
> > Update the algorithm in storvsc_do_io to look for a channel starting
> > with the current CPU + 1 and wrap around (within the current NUMA
> > node). This spreads VMbus interrupts more evenly across CPUs. Previous
> > code always started with first CPU in the current NUMA node, skewing
> > the interrupt load to that CPU.
> >
> > Signed-off-by: Michael Kelley 
> > ---
> >  drivers/scsi/storvsc_drv.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> > index e07907d..f3264c4 100644
> > --- a/drivers/scsi/storvsc_drv.c
> > +++ b/drivers/scsi/storvsc_drv.c
> > @@ -1310,7 +1310,8 @@ static int storvsc_do_io(struct hv_device *device,
> >  */
> > cpumask_and(_mask, _device-
> >alloced_cpus,
> >
> cpumask_of_node(cpu_to_node(q_num)));
> > -   for_each_cpu(tgt_cpu, _mask) {
> > +   for_each_cpu_wrap(tgt_cpu, _mask,
> > +   outgoing_channel->target_cpu + 1) {

Does it work when target_cpu is the last CPU on the system?

Otherwise, looking good.

> > if (tgt_cpu != outgoing_channel->target_cpu)
> {
> > outgoing_channel =
> > stor_device->stor_chns[tgt_cpu];
> > --
> > 1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] binder: check for binder_thread allocation failure in binder_poll()

[Todd Kjos]

Looks good to me.

On Tue, Jan 30, 2018 at 11:11 PM, Eric Biggers  wrote:
> From: Eric Biggers 
>
> If the kzalloc() in binder_get_thread() fails, binder_poll()
> dereferences the resulting NULL pointer.
>
> Fix it by returning POLLERR if the memory allocation failed.
>
> This bug was found by syzkaller using fault injection.
>
> Reported-by: syzbot 
> Fixes: 457b9a6f09f0 ("Staging: android: add binder driver")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Eric Biggers 
> ---
>  drivers/android/binder.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index d21040c5d343f..326ca8ea9ebcf 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -4391,6 +4391,8 @@ static __poll_t binder_poll(struct file *filp,
> bool wait_for_proc_work;
>
> thread = binder_get_thread(proc);
> +   if (!thread)
> +   return POLLERR;
>
> binder_inner_proc_lock(thread->proc);
> thread->looper |= BINDER_LOOPER_STATE_POLL;
> --
> 2.16.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/1] scsi: storvsc: Increase cmd_per_lun for higher speed devices

[KY Srinivasan]

> -Original Message-
> From: Michael Kelley (EOSG)
> Sent: Wednesday, January 24, 2018 2:50 PM
> To: KY Srinivasan ; Stephen Hemminger
> ; martin.peter...@oracle.com; Long Li
> ; j...@linux.vnet.ibm.com;
> de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; linux-
> s...@vger.kernel.org
> Cc: Michael Kelley (EOSG) 
> Subject: [PATCH 1/1] scsi: storvsc: Increase cmd_per_lun for higher speed
> devices
> 
> Increase cmd_per_lun to allow more I/Os in progress per device,
> particularly for NVMe's.  The Hyper-V host side can handle the
> higher count with no issues.
> 
> Signed-off-by: Michael Kelley 

Reviewed-by: K. Y. Srinivasan 
Acked-by: K. Y. Srinivasan 

Thanks,

K. Y
> ---
>  drivers/scsi/storvsc_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
> index f3264c4..6205107 100644
> --- a/drivers/scsi/storvsc_drv.c
> +++ b/drivers/scsi/storvsc_drv.c
> @@ -1657,7 +1657,7 @@ static int storvsc_queuecommand(struct Scsi_Host
> *host, struct scsi_cmnd *scmnd)
>   .eh_timed_out = storvsc_eh_timed_out,
>   .slave_alloc =  storvsc_device_alloc,
>   .slave_configure =  storvsc_device_configure,
> - .cmd_per_lun =  255,
> + .cmd_per_lun =  2048,
>   .this_id =  -1,
>   .use_clustering =   ENABLE_CLUSTERING,
>   /* Make sure we dont get a sg segment crosses a page boundary */
> --
> 1.8.3.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/6] staging: unisys: visorinput: Clean up Makefile includes

[David Kershner]

The driver no longer needs to include drivers/staging/unisys/include, so we
can get rid of it.

Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
---
 drivers/staging/unisys/visorinput/Makefile | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/unisys/visorinput/Makefile 
b/drivers/staging/unisys/visorinput/Makefile
index beedca7..6e4bfa0 100644
--- a/drivers/staging/unisys/visorinput/Makefile
+++ b/drivers/staging/unisys/visorinput/Makefile
@@ -4,4 +4,3 @@
 
 obj-$(CONFIG_UNISYS_VISORINPUT)+= visorinput.o
 
-ccflags-y += -Idrivers/staging/unisys/include
-- 
git-series 0.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 5/6] staging: unisys: visorinput: Fix spacing after open paranthesis

[David Kershner]

Checkpatch was giving errors about an open parenthesis being the last thing
on a line. This patch cleans up some names and removes the checkpatch
warnings.

Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
---
 drivers/staging/unisys/visorinput/visorinput.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/unisys/visorinput/visorinput.c 
b/drivers/staging/unisys/visorinput/visorinput.c
index 634b16b..aae3fe5 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -91,7 +91,7 @@ struct visor_input_channel_data {
};
 } __packed;
 
-enum visorinput_device_type {
+enum visorinput_dev_type {
visorinput_keyboard,
visorinput_mouse,
 };
@@ -397,16 +397,15 @@ static struct input_dev *setup_client_mouse(void 
*devdata, unsigned int xres,
return visorinput_dev;
 }
 
-static struct visorinput_devdata *devdata_create(
-   struct visor_device *dev,
-   enum visorinput_device_type devtype)
+static struct visorinput_devdata *devdata_create(struct visor_device *dev,
+enum visorinput_dev_type dtype)
 {
struct visorinput_devdata *devdata = NULL;
unsigned int extra_bytes = 0;
unsigned int size, xres, yres, err;
struct visor_input_channel_data data;
 
-   if (devtype == visorinput_keyboard)
+   if (dtype == visorinput_keyboard)
/* allocate room for devdata->keycode_table, filled in below */
extra_bytes = KEYCODE_TABLE_BYTES * 2;
devdata = kzalloc(sizeof(*devdata) + extra_bytes, GFP_KERNEL);
@@ -429,7 +428,7 @@ static struct visorinput_devdata *devdata_create(
 * so we need to create whatever input nodes are necessary to
 * deliver our inputs to the guest OS.
 */
-   switch (devtype) {
+   switch (dtype) {
case visorinput_keyboard:
devdata->keycode_table_bytes = extra_bytes;
memcpy(devdata->keycode_table, visorkbd_keycode,
@@ -497,17 +496,17 @@ static struct visorinput_devdata *devdata_create(
 static int visorinput_probe(struct visor_device *dev)
 {
const guid_t *guid;
-   enum visorinput_device_type devtype;
+   enum visorinput_dev_type dtype;
 
guid = visorchannel_get_guid(dev->visorchannel);
if (guid_equal(guid, _mouse_channel_guid))
-   devtype = visorinput_mouse;
+   dtype = visorinput_mouse;
else if (guid_equal(guid, _keyboard_channel_guid))
-   devtype = visorinput_keyboard;
+   dtype = visorinput_keyboard;
else
return -ENODEV;
visorbus_disable_channel_interrupts(dev);
-   if (!devdata_create(dev, devtype))
+   if (!devdata_create(dev, dtype))
return -ENOMEM;
return 0;
 }
-- 
git-series 0.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/6] staging: unisys: visorinput: Updates to visorinput

[David Kershner]

Miscellaneous patches to the visorinput driver.

David Kershner (5):
  staging: unisys: visorinput: Clean up Makefile includes
  staging: unisys: visorinput: remove duplicate comments
  staging: unisys: visorinput: combine ultrainputreport.h with visorinput.c
  staging: unisys: visorinput: Fix spacing after open paranthesis
  staging: unisys: visorinput: use the full 80 characters of the screen

Tim Sell (1):
  staging: unisys: visorinput: remove need for 'depends on FB'

 drivers/staging/unisys/visorinput/Kconfig|   2 +-
 drivers/staging/unisys/visorinput/Makefile   |   1 +-
 drivers/staging/unisys/visorinput/ultrainputreport.h |  57 +
 drivers/staging/unisys/visorinput/visorinput.c   | 169 +++-
 4 files changed, 113 insertions(+), 116 deletions(-)
 delete mode 100644 drivers/staging/unisys/visorinput/ultrainputreport.h

base-commit: 3384e01179eff2b2fe91ba7bcad98ee2be5f87f7
-- 
git-series 0.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/6] staging: unisys: visorinput: remove need for 'depends on FB'

[David Kershner]

From: Tim Sell 

Previously, we used a hack to determine the max x,y resolution of the
visor virtual mouse: we just looked at the resolution of the
first-registered framebuffer device, using the currently-valid assumption
that in a Unisys s-Par guest environment the video will be provided by an
efifb framebuffer device.

This hack has been removed, by instead determining the default mouse
resolution by looking at fields within the visor mouse channel memory,
mouse.x_res and mouse.y_res.  If these fields are 0, a default resolution
of 1024x768 is assumed.

Signed-off-by: Tim Sell 
Signed-off-by: David Kershner 
---
 drivers/staging/unisys/visorinput/Kconfig  |  2 +-
 drivers/staging/unisys/visorinput/visorinput.c | 43 +--
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/unisys/visorinput/Kconfig 
b/drivers/staging/unisys/visorinput/Kconfig
index 655cd62..a3817e0 100644
--- a/drivers/staging/unisys/visorinput/Kconfig
+++ b/drivers/staging/unisys/visorinput/Kconfig
@@ -4,7 +4,7 @@
 
 config UNISYS_VISORINPUT
tristate "Unisys visorinput driver"
-   depends on UNISYSSPAR && UNISYS_VISORBUS && FB && INPUT
+   depends on UNISYSSPAR && UNISYS_VISORBUS && INPUT
---help---
The Unisys s-Par visorinput driver provides a virtualized system
console (keyboard and mouse) that is accessible through the
diff --git a/drivers/staging/unisys/visorinput/visorinput.c 
b/drivers/staging/unisys/visorinput/visorinput.c
index d8048e4..9973040 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -32,10 +32,24 @@
  0x81, 0xc3, 0x61, 0xab, 0xcd, 0xbd, 0xbd, 0x87)
 #define VISOR_MOUSE_CHANNEL_GUID_STR "addf07d4-94a9-46e2-81c3-61abcdbdbd87"
 
-#define PIXELS_ACROSS_DEFAULT 800
-#define PIXELS_DOWN_DEFAULT   600
+#define PIXELS_ACROSS_DEFAULT 1024
+#define PIXELS_DOWN_DEFAULT   768
 #define KEYCODE_TABLE_BYTES   256
 
+/* header of keyboard/mouse channels */
+struct visor_input_channel_data {
+   u32 n_input_reports;
+   union {
+   struct {
+   u16 x_res;
+   u16 y_res;
+   } mouse;
+   struct {
+   u32 flags;
+   } keyboard;
+   };
+} __packed;
+
 enum visorinput_device_type {
visorinput_keyboard,
visorinput_mouse,
@@ -306,10 +320,9 @@ static struct input_dev *setup_client_keyboard(void 
*devdata,
return visorinput_dev;
 }
 
-static struct input_dev *setup_client_mouse(void *devdata)
+static struct input_dev *setup_client_mouse(void *devdata, unsigned int xres,
+   unsigned int yres)
 {
-   int xres, yres;
-   struct fb_info *fb0;
struct input_dev *visorinput_dev = input_allocate_device();
 
if (!visorinput_dev)
@@ -327,14 +340,10 @@ static struct input_dev *setup_client_mouse(void *devdata)
set_bit(BTN_RIGHT, visorinput_dev->keybit);
set_bit(BTN_MIDDLE, visorinput_dev->keybit);
 
-   if (registered_fb[0]) {
-   fb0 = registered_fb[0];
-   xres = fb0->var.xres_virtual;
-   yres = fb0->var.yres_virtual;
-   } else {
+   if (xres == 0)
xres = PIXELS_ACROSS_DEFAULT;
+   if (yres == 0)
yres = PIXELS_DOWN_DEFAULT;
-   }
input_set_abs_params(visorinput_dev, ABS_X, 0, xres, 0, 0);
input_set_abs_params(visorinput_dev, ABS_Y, 0, yres, 0, 0);
 
@@ -353,6 +362,8 @@ static struct visorinput_devdata *devdata_create(
 {
struct visorinput_devdata *devdata = NULL;
unsigned int extra_bytes = 0;
+   unsigned int size, xres, yres, err;
+   struct visor_input_channel_data data;
 
if (devtype == visorinput_keyboard)
/* allocate room for devdata->keycode_table, filled in below */
@@ -390,7 +401,15 @@ static struct visorinput_devdata *devdata_create(
goto cleanups_register;
break;
case visorinput_mouse:
-   devdata->visorinput_dev = setup_client_mouse(devdata);
+   size = sizeof(struct visor_input_channel_data);
+   err = visorbus_read_channel(dev, sizeof(struct channel_header),
+   , size);
+   if (err)
+   goto cleanups_register;
+   xres = data.mouse.x_res;
+   yres = data.mouse.y_res;
+   devdata->visorinput_dev = setup_client_mouse(devdata, xres,
+yres);
if (!devdata->visorinput_dev)
goto cleanups_register;
break;
-- 
git-series 0.9.1
___
devel mailing list
de...@linuxdriverproject.org

[PATCH 3/6] staging: unisys: visorinput: remove duplicate comments

[David Kershner]

Comments were based on individual entries, if we group the entries, we can
get rid of duplicate comments.

Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
---
 drivers/staging/unisys/visorinput/ultrainputreport.h | 16 +
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/unisys/visorinput/ultrainputreport.h 
b/drivers/staging/unisys/visorinput/ultrainputreport.h
index 67dac43..5eafa6c 100644
--- a/drivers/staging/unisys/visorinput/ultrainputreport.h
+++ b/drivers/staging/unisys/visorinput/ultrainputreport.h
@@ -12,35 +12,33 @@
 /* These defines identify mouse and keyboard activity which is specified by the
  * firmware to the host using the cmsimpleinput protocol.  @ingroup coretypes
  */
-   /* only motion; arg1=x, arg2=y */
+/* only motion; arg1=x, arg2=y */
 #define INPUTACTION_XY_MOTION 1
+
 /* arg1: 1=left,2=center,3=right */
 #define INPUTACTION_MOUSE_BUTTON_DOWN 2
-/* arg1: 1=left,2=center,3=right */
 #define INPUTACTION_MOUSE_BUTTON_UP 3
-/* arg1: 1=left,2=center,3=right */
 #define INPUTACTION_MOUSE_BUTTON_CLICK 4
-/* arg1: 1=left,2=center 3=right */
 #define INPUTACTION_MOUSE_BUTTON_DCLICK 5
-/* arg1: wheel rotation away from user */
+
+/* arg1: wheel rotation away from/toward user */
 #define INPUTACTION_WHEEL_ROTATE_AWAY 6
-/* arg1: wheel rotation toward user */
 #define INPUTACTION_WHEEL_ROTATE_TOWARD 7
+
 /* arg1: scancode, as follows: If arg1 <= 0xff, it's a 1-byte scancode and arg1
  *  is that scancode. If arg1 > 0xff, it's a 2-byte scanecode, with the 1st
  *  byte in the low 8 bits, and the 2nd byte in the high 8 bits.
  *  E.g., the right ALT key would appear as x'38e0'.
  */
 #define INPUTACTION_KEY_DOWN 64
-/* arg1: scancode (in same format as inputaction_keyDown) */
 #define INPUTACTION_KEY_UP 65
+#define INPUTACTION_KEY_DOWN_UP 67
+
 /* arg1: scancode (in same format as inputaction_keyDown); MUST refer to one of
  *  the locking keys, like capslock, numlock, or scrolllock.
  * arg2: 1 iff locking key should be in the LOCKED position (e.g., light is ON)
  */
 #define INPUTACTION_SET_LOCKING_KEY_STATE 66
-/* arg1: scancode (in same format as inputaction_keyDown */
-#define INPUTACTION_KEY_DOWN_UP 67
 
 struct visor_inputactivity {
u16 action;
-- 
git-series 0.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 6/6] staging: unisys: visorinput: use the full 80 characters of the screen

[David Kershner]

Several of the comments in the code were not using the full 80 characters
of the screen. This patch combines the lines to make full use of the
screen.

Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
---
 drivers/staging/unisys/visorinput/visorinput.c | 64 ---
 1 file changed, 30 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/unisys/visorinput/visorinput.c 
b/drivers/staging/unisys/visorinput/visorinput.c
index aae3fe5..9693fb5 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -97,9 +97,9 @@ enum visorinput_dev_type {
 };
 
 /*
- * This is the private data that we store for each device.
- * A pointer to this struct is maintained via
- * dev_get_drvdata() / dev_set_drvdata() for each struct device.
+ * This is the private data that we store for each device. A pointer to this
+ * struct is maintained via dev_get_drvdata() / dev_set_drvdata() for each
+ * struct device.
  */
 struct visorinput_devdata {
struct visor_device *dev;
@@ -270,10 +270,9 @@ static int visorinput_open(struct input_dev 
*visorinput_dev)
dev_dbg(_dev->dev, "%s opened\n", __func__);
 
/*
-* If we're not paused, really enable interrupts.
-* Regardless of whether we are paused, set a flag indicating
-* interrupts should be enabled so when we resume, interrupts
-* will really be enabled.
+* If we're not paused, really enable interrupts. Regardless of whether
+* we are paused, set a flag indicating interrupts should be enabled so
+* when we resume, interrupts will really be enabled.
 */
mutex_lock(>lock_visor_dev);
devdata->interrupts_enabled = true;
@@ -299,10 +298,9 @@ static void visorinput_close(struct input_dev 
*visorinput_dev)
dev_dbg(_dev->dev, "%s closed\n", __func__);
 
/*
-* If we're not paused, really disable interrupts.
-* Regardless of whether we are paused, set a flag indicating
-* interrupts should be disabled so when we resume we will
-* not re-enable them.
+* If we're not paused, really disable interrupts. Regardless of
+* whether we are paused, set a flag indicating interrupts should be
+* disabled so when we resume we will not re-enable them.
 */
mutex_lock(>lock_visor_dev);
devdata->interrupts_enabled = false;
@@ -315,9 +313,9 @@ static void visorinput_close(struct input_dev 
*visorinput_dev)
 }
 
 /*
- * setup_client_keyboard() initializes and returns a Linux input node that
- * we can use to deliver keyboard inputs to Linux.  We of course do this when
- * we see keyboard inputs coming in on a keyboard channel.
+ * setup_client_keyboard() initializes and returns a Linux input node that we
+ * can use to deliver keyboard inputs to Linux.  We of course do this when we
+ * see keyboard inputs coming in on a keyboard channel.
  */
 static struct input_dev *setup_client_keyboard(void *devdata,
   unsigned char *keycode_table)
@@ -424,9 +422,9 @@ static struct visorinput_devdata *devdata_create(struct 
visor_device *dev,
devdata->paused = true;
 
/*
-* This is an input device in a client guest partition,
-* so we need to create whatever input nodes are necessary to
-* deliver our inputs to the guest OS.
+* This is an input device in a client guest partition, so we need to
+* create whatever input nodes are necessary to deliver our inputs to
+* the guest OS.
 */
switch (dtype) {
case visorinput_keyboard:
@@ -463,10 +461,9 @@ static struct visorinput_devdata *devdata_create(struct 
visor_device *dev,
 
/*
 * Device struct is completely set up now, with the exception of
-* visorinput_dev being registered.
-* We need to unlock before we register the device, because this
-* can cause an on-stack call of visorinput_open(), which would
-* deadlock if we had the lock.
+* visorinput_dev being registered. We need to unlock before we
+* register the device, because this can cause an on-stack call of
+* visorinput_open(), which would deadlock if we had the lock.
 */
if (input_register_device(devdata->visorinput_dev)) {
input_free_device(devdata->visorinput_dev);
@@ -475,9 +472,9 @@ static struct visorinput_devdata *devdata_create(struct 
visor_device *dev,
 
mutex_lock(>lock_visor_dev);
/*
-* Establish calls to visorinput_channel_interrupt() if that is
-* the desired state that we've kept track of in interrupts_enabled
-* while the device was being created.
+* Establish calls to visorinput_channel_interrupt() if that is the
+* desired state that we've kept track of in interrupts_enabled while
+* 

[PATCH 4/6] staging: unisys: visorinput: combine ultrainputreport.h with visorinput.c

[David Kershner]

The file ultrainputreport.h was just being used by visorinput.c. Move the
definitions into visorinput.c and get rid of the file.

Signed-off-by: David Kershner 
Reviewed-by: Tim Sell 
---
 drivers/staging/unisys/visorinput/ultrainputreport.h | 55 +-
 drivers/staging/unisys/visorinput/visorinput.c   | 43 +-
 2 files changed, 42 insertions(+), 56 deletions(-)
 delete mode 100644 drivers/staging/unisys/visorinput/ultrainputreport.h

diff --git a/drivers/staging/unisys/visorinput/ultrainputreport.h 
b/drivers/staging/unisys/visorinput/ultrainputreport.h
deleted file mode 100644
index 5eafa6c..000
--- a/drivers/staging/unisys/visorinput/ultrainputreport.h
+++ /dev/null
@@ -1,55 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 2010 - 2015 UNISYS CORPORATION
- * All rights reserved.
- */
-
-#ifndef __SPAR_ULTRAINPUTREPORT_H__
-#define __SPAR_ULTRAINPUTREPORT_H__
-
-#include 
-
-/* These defines identify mouse and keyboard activity which is specified by the
- * firmware to the host using the cmsimpleinput protocol.  @ingroup coretypes
- */
-/* only motion; arg1=x, arg2=y */
-#define INPUTACTION_XY_MOTION 1
-
-/* arg1: 1=left,2=center,3=right */
-#define INPUTACTION_MOUSE_BUTTON_DOWN 2
-#define INPUTACTION_MOUSE_BUTTON_UP 3
-#define INPUTACTION_MOUSE_BUTTON_CLICK 4
-#define INPUTACTION_MOUSE_BUTTON_DCLICK 5
-
-/* arg1: wheel rotation away from/toward user */
-#define INPUTACTION_WHEEL_ROTATE_AWAY 6
-#define INPUTACTION_WHEEL_ROTATE_TOWARD 7
-
-/* arg1: scancode, as follows: If arg1 <= 0xff, it's a 1-byte scancode and arg1
- *  is that scancode. If arg1 > 0xff, it's a 2-byte scanecode, with the 1st
- *  byte in the low 8 bits, and the 2nd byte in the high 8 bits.
- *  E.g., the right ALT key would appear as x'38e0'.
- */
-#define INPUTACTION_KEY_DOWN 64
-#define INPUTACTION_KEY_UP 65
-#define INPUTACTION_KEY_DOWN_UP 67
-
-/* arg1: scancode (in same format as inputaction_keyDown); MUST refer to one of
- *  the locking keys, like capslock, numlock, or scrolllock.
- * arg2: 1 iff locking key should be in the LOCKED position (e.g., light is ON)
- */
-#define INPUTACTION_SET_LOCKING_KEY_STATE 66
-
-struct visor_inputactivity {
-   u16 action;
-   u16 arg1;
-   u16 arg2;
-   u16 arg3;
-} __packed;
-
-struct visor_inputreport {
-   u64 seq_no;
-   struct visor_inputactivity activity;
-} __packed;
-
-#endif
diff --git a/drivers/staging/unisys/visorinput/visorinput.c 
b/drivers/staging/unisys/visorinput/visorinput.c
index 9973040..634b16b 100644
--- a/drivers/staging/unisys/visorinput/visorinput.c
+++ b/drivers/staging/unisys/visorinput/visorinput.c
@@ -18,7 +18,36 @@
 #include 
 #include 
 
-#include "ultrainputreport.h"
+/* These defines identify mouse and keyboard activity which is specified by the
+ * firmware to the host using the cmsimpleinput protocol.  @ingroup coretypes
+ */
+/* only motion; arg1=x, arg2=y */
+#define INPUTACTION_XY_MOTION 1
+
+/* arg1: 1=left,2=center,3=right */
+#define INPUTACTION_MOUSE_BUTTON_DOWN 2
+#define INPUTACTION_MOUSE_BUTTON_UP 3
+#define INPUTACTION_MOUSE_BUTTON_CLICK 4
+#define INPUTACTION_MOUSE_BUTTON_DCLICK 5
+
+/* arg1: wheel rotation away from/toward user */
+#define INPUTACTION_WHEEL_ROTATE_AWAY 6
+#define INPUTACTION_WHEEL_ROTATE_TOWARD 7
+
+/* arg1: scancode, as follows: If arg1 <= 0xff, it's a 1-byte scancode and arg1
+ *  is that scancode. If arg1 > 0xff, it's a 2-byte scanecode, with the 1st
+ *  byte in the low 8 bits, and the 2nd byte in the high 8 bits.
+ *  E.g., the right ALT key would appear as x'38e0'.
+ */
+#define INPUTACTION_KEY_DOWN 64
+#define INPUTACTION_KEY_UP 65
+#define INPUTACTION_KEY_DOWN_UP 67
+
+/* arg1: scancode (in same format as inputaction_keyDown); MUST refer to one of
+ *  the locking keys, like capslock, numlock, or scrolllock.
+ * arg2: 1 iff locking key should be in the LOCKED position (e.g., light is ON)
+ */
+#define INPUTACTION_SET_LOCKING_KEY_STATE 66
 
 /* Keyboard channel {c73416d0-b0b8-44af-b304-9d2ae99f1b3d} */
 #define VISOR_KEYBOARD_CHANNEL_GUID \
@@ -36,6 +65,18 @@
 #define PIXELS_DOWN_DEFAULT   768
 #define KEYCODE_TABLE_BYTES   256
 
+struct visor_inputactivity {
+   u16 action;
+   u16 arg1;
+   u16 arg2;
+   u16 arg3;
+} __packed;
+
+struct visor_inputreport {
+   u64 seq_no;
+   struct visor_inputactivity activity;
+} __packed;
+
 /* header of keyboard/mouse channels */
 struct visor_input_channel_data {
u32 n_input_reports;
-- 
git-series 0.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[bug report] staging: Add rtl8723bs sdio wifi driver

[Dan Carpenter]

Hello Hans de Goede,

The patch 554c0a3abf21: "staging: Add rtl8723bs sdio wifi driver"
from Mar 29, 2017, leads to the following static checker warning:

drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c:1374 
_PHY_PathADDAOn8723B()
warn: both sides of ternary the same: '29360148' 0x01c00014 0x01c00014

drivers/staging/rtl8723bs/hal/HalPhyRf_8723B.c
  1360  static void _PHY_PathADDAOn8723B(
  1361  struct adapter *padapter,
  1362  u32 *ADDAReg,
  1363  bool isPathAOn,
  1364  bool is2T
  1365  )
  1366  {
  1367  u32 pathOn;
  1368  u32 i;
  1369  struct hal_com_data *pHalData = GET_HAL_DATA(padapter);
  1370  PDM_ODM_T pDM_Odm = >odmpriv;
  1371  
  1372  ODM_RT_TRACE(pDM_Odm, ODM_COMP_CALIBRATION, ODM_DBG_LOUD, 
("ADDA ON.\n"));
  1373  
  1374  pathOn = isPathAOn ? 0x01c00014 : 0x01c00014;
 ^^^
  1375  if (false == is2T) {
  1376  pathOn = 0x01c00014;
 ^^
  1377  PHY_SetBBReg(pDM_Odm->Adapter, ADDAReg[0], bMaskDWord, 
0x01c00014);
   
^^
These are all four the same.  We're really determined that it must be
0x01c00014.  :P  Do you have any idea what was intended?

  1378  } else {
  1379  PHY_SetBBReg(pDM_Odm->Adapter, ADDAReg[0], bMaskDWord, 
pathOn);
  1380  }
  1381  
  1382  for (i = 1 ; i < IQK_ADDA_REG_NUM ; i++) {
  1383  PHY_SetBBReg(pDM_Odm->Adapter, ADDAReg[i], bMaskDWord, 
pathOn);
  1384  }
  1385  
  1386  }

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

Re: kernel BUG at drivers/android/binder_alloc.c:LINE!

[Dan Carpenter]

On Wed, Jan 31, 2018 at 01:00:35PM +0100, Dmitry Vyukov wrote:
> On Wed, Jan 31, 2018 at 9:08 AM, Dan Carpenter  
> wrote:
> > On Tue, Jan 30, 2018 at 11:59:47PM -0800, Eric Biggers wrote:
> >> On Fri, Dec 01, 2017 at 04:22:00PM -0800, syzbot wrote:
> >> > syzkaller has found reproducer for the following crash on
> >> > 3c1c4ddffb58b9e10b3365764fe59546130b3f32
> >> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
> >> > compiler: gcc (GCC) 7.1.1 20170620
> >> > .config is attached
> >> > Raw console output is attached.
> >> > C reproducer is attached
> >> > syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> >> > for information about syzkaller reproducers
> >> >
> >> >
> >> > binder: 3087:3087 ERROR: BC_REGISTER_LOOPER called without request
> >> > binder: 3087:3087 ioctl c0306201 2000dfd0 returned -14
> >> > [ cut here ]
> >> > kernel BUG at drivers/android/binder_alloc.c:750!
> >> > invalid opcode:  [#1] SMP KASAN
> >> > Dumping ftrace buffer:
> >> >(ftrace buffer empty)
> >> > Modules linked in:
> >> > CPU: 0 PID: 1404 Comm: kworker/0:2 Not tainted 4.15.0-rc1+ #203
> >> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> >> > Google 01/01/2011
> >> > Workqueue: events binder_deferred_func
> >> > task: 09b739b6 task.stack: c1d4442c
> >> > RIP: 0010:binder_alloc_deferred_release+0x146/0xa40
> >> > drivers/android/binder_alloc.c:750
> >> > RSP: 0018:8801d2b16fd8 EFLAGS: 00010293
> >> > RAX: 8801d2b08080 RBX: 8801d7829300 RCX: 8403b856
> >> > RDX:  RSI: 0004 RDI: 8801d7829330
> >> > RBP: 8801d2b17138 R08: 8403b7d9 R09: 10e53001
> >> > R10: 8801d2b16fc8 R11: 87489d60 R12: 
> >> > R13: dc00 R14: 8801d2b17110 R15: 8801d7829310
> >> > FS:  () GS:8801db40() 
> >> > knlGS:
> >> > CS:  0010 DS:  ES:  CR0: 80050033
> >> > CR2: 006d00a8 CR3: 05e25000 CR4: 001406f0
> >> > DR0:  DR1:  DR2: 
> >> > DR3:  DR6: fffe0ff0 DR7: 0400
> >> > Call Trace:
> >> >  binder_free_proc drivers/android/binder.c:4200 [inline]
> >> >  binder_proc_dec_tmpref+0x2f3/0x420 drivers/android/binder.c:1833
> >> >  binder_deferred_release drivers/android/binder.c:4858 [inline]
> >> >  binder_deferred_func+0xe22/0x12f0 drivers/android/binder.c:4893
> >> >  process_one_work+0xbfd/0x1be0 kernel/workqueue.c:2112
> >> >  worker_thread+0x223/0x1990 kernel/workqueue.c:2246
> >> >  kthread+0x37a/0x440 kernel/kthread.c:238
> >> >  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:441
> >> > Code: e8 00 40 6c fd 49 8d 7f 20 49 8d 5f f0 48 89 fa 48 c1 ea 03 42 80 
> >> > 3c
> >> > 2a 00 0f 85 84 07 00 00 49 83 7f 20 00 74 a9 e8 da 3f 6c fd <0f> 0b 48 
> >> > 8b 9d
> >> > e8 fe ff ff 44 89 a5 bc fe ff ff e8 c5 3f 6c fd
> >> > RIP: binder_alloc_deferred_release+0x146/0xa40
> >> > drivers/android/binder_alloc.c:750 RSP: 8801d2b16fd8
> >> > ---[ end trace 616e085d0dbf3c21 ]---
> >> > Kernel panic - not syncing: Fatal exception
> >> > Dumping ftrace buffer:
> >> >(ftrace buffer empty)
> >> > Kernel Offset: disabled
> >> > Rebooting in 86400 seconds..
> >>
> >> This crash stopped occurring in v4.15-rc3, and I verified that it was 
> >> commit
> >> fb2c445277e which fixed it (thanks Martijn!).  So, telling syzbot so that 
> >> it can
> >> start reporting similar crashes again:
> >>
> >> #syz fix: ANDROID: binder: fix transaction leak.
> >
> > Could you change the syz fix format to include the hash in case we want
> > to look it up?
> 
> Hi Dan,
> 
> There is an unpleasant issue related to this. Some email clients
> (notably gmail) reflow text emails to 72/80 columns and this
> transformation is irreversible. So if we add hash, this increases
> chances of commit title (what is actually used by syzbot) being
> corrupted.
> The Reported-by tags allow precise matching of bugs to reports and
> back (and that's what I think now is the preferred way to doing the
> association as it does not require somebody to go back to the report
> email and attach the tag).
> Or I can suggest to provide commit hash on a separate line.
> The reflow thing is nasty, but hit us periodically. The problem that
> should not be there at all, but I don't see a good solution for it.
> Obviously requiring developers to follow some formal escaping rules
> for titles is not going to work either.

Ah, that's fine then, I didn't realize.  Eric obviously did include the
hash in this email so that's great.

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

Re: [PATCH v3] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap

[Greg KH]

On Wed, Jan 31, 2018 at 02:03:42PM +0200, Alexey Skidanov wrote:
> Any driver may access shared buffers, created by ion, using dma_buf_vmap and
> dma_buf_vunmap dma-buf API that maps/unmaps previosuly allocated buffers into
> the kernel virtual address space. The implementation of these API is missing 
> in
> the current ion implementation.
> 
> Signed-off-by: Alexey Skidanov 
> ---

No review from any other Intel developers? :(

Anyway, what in-tree driver needs access to these functions?

And are you sure that you don't need to do any "real" logic in the
vmap/vunmap calls?  That feels like there would be some reference
counting problems here.

>  drivers/staging/android/ion/ion.c | 13 +
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/staging/android/ion/ion.c 
> b/drivers/staging/android/ion/ion.c
> index f480885..4f1dc7f 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -327,6 +327,17 @@ static void ion_dma_buf_kunmap(struct dma_buf *dmabuf, 
> unsigned long offset,
>  {
>  }
>  
> +static void *ion_dma_buf_vmap(struct dma_buf *dmabuf)
> +{
> + struct ion_buffer *buffer = dmabuf->priv;
> +
> + return buffer->vaddr;

Just call ion_dma_buf_kmap(dmabuf, 0)?

Again, please get this reviewed by someone else in Intel first.  Don't
ignore the resources you have, to do so would be foolish :)

thanks,

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

[PATCH v3] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap

[Alexey Skidanov]

Any driver may access shared buffers, created by ion, using dma_buf_vmap and
dma_buf_vunmap dma-buf API that maps/unmaps previosuly allocated buffers into
the kernel virtual address space. The implementation of these API is missing in
the current ion implementation.

Signed-off-by: Alexey Skidanov 
---
 drivers/staging/android/ion/ion.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index f480885..4f1dc7f 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -327,6 +327,17 @@ static void ion_dma_buf_kunmap(struct dma_buf *dmabuf, 
unsigned long offset,
 {
 }
 
+static void *ion_dma_buf_vmap(struct dma_buf *dmabuf)
+{
+   struct ion_buffer *buffer = dmabuf->priv;
+
+   return buffer->vaddr;
+}
+
+static void ion_dma_buf_vunmap(struct dma_buf *dmabuf, void *vaddr)
+{
+}
+
 static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf,
enum dma_data_direction direction)
 {
@@ -388,6 +399,8 @@ static const struct dma_buf_ops dma_buf_ops = {
.unmap_atomic = ion_dma_buf_kunmap,
.map = ion_dma_buf_kmap,
.unmap = ion_dma_buf_kunmap,
+   .vmap = ion_dma_buf_vmap,
+   .vunmap = ion_dma_buf_vunmap
 };
 
 int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
-- 
2.7.4

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

Re: [PATCH] staging: android: ion: minor cleanup for ion_page_pool

[Greg KH]

On Wed, Jan 31, 2018 at 05:50:52PM +0800, Yisheng Xie wrote:
> ion_page_pool.c now is used to apply pool APIs for system heap, which do
> not need do any initial at device_initcall.
> 
> Meanwhile, this patch also remove some useless include files.

Please only do one thing per patch, otherwise if one of the different
things you do has a problem, all of the changes have been reverted.

Yeah, it's a pain at times, but that's the way kernel development is,
sorry.

Please break this up into two patches and resend.  I will be glad to
queue them up after the merge window ends when 4.16-rc1 is released.

thanks,

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

Re: [RFC PATCH 1/2] hv_netvsc: Split netvsc_revoke_buf() and netvsc_teardown_gpadl()

[Mohammed Gamal]

On Tue, 2018-01-30 at 11:29 -0800, Stephen Hemminger wrote:
> On Tue, 23 Jan 2018 10:34:04 +0100
> Mohammed Gamal  wrote:
> 
> > Split each of the functions into two for each of send/recv buffers
> > 
> > Signed-off-by: Mohammed Gamal 
> 
> Splitting these functions is not necessary

How so? We need to send each message independently, and hence the split
(see cover letter). Is there another way?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 05/10] Staging: rtl8192e: kconfig: Remove empty help text

[Dan Carpenter]

On Wed, Jan 31, 2018 at 10:52:08AM +0100, Ulf Magnusson wrote:
> On Wed, Jan 31, 2018 at 9:46 AM, Dan Carpenter  
> wrote:
> > On Wed, Jan 31, 2018 at 09:41:24AM +0100, Ulf Magnusson wrote:
> >> On Wed, Jan 31, 2018 at 9:06 AM, Dan Carpenter  
> >> wrote:
> >> > On Tue, Jan 30, 2018 at 08:05:27PM +0100, Ulf Magnusson wrote:
> >> >> In preparation for adding a warning ("kconfig: Warn if help text is
> >> >> blank"): https://lkml.org/lkml/2018/1/30/516
> >> >>
> >> >
> >> > Please don't start the boddy of the email in the middle of a sentence.
> >> >
> >> > It looks like this:
> >> >
> >> > https://marc.info/?l=linux-driver-devel=151733925532413=2
> >> >
> >> > Notice how the subject is up above the From, Date, and Message-id email
> >> > headers?  It's totally disconnected from the body of the email.
> >> >
> >> > regards,
> >> > dan carpenter
> >>
> >> The 'this is in...' was implied. I'll try to be less of a sloppy bastard.
> >>
> >
> > I do feel that patch descriptions should say what the patch does as well
> > as why.  Right now the subject says what but I normally read either the
> > body or the subject but not both.
> >
> > regards,
> > dan carpenter
> >
> 
> I fleshed out the individual commit messages and rolled a new
> patchset, which adds the warning at the end:
> https://lkml.org/lkml/2018/1/31/137

Thanks!

regards,
dan carpenter

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

[PATCH] staging: android: ion: minor cleanup for ion_page_pool

[Yisheng Xie]

ion_page_pool.c now is used to apply pool APIs for system heap, which do
not need do any initial at device_initcall.

Meanwhile, this patch also remove some useless include files.

Signed-off-by: Yisheng Xie 
---
 drivers/staging/android/ion/ion_page_pool.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/drivers/staging/android/ion/ion_page_pool.c 
b/drivers/staging/android/ion/ion_page_pool.c
index 817849d..3c1eba5 100644
--- a/drivers/staging/android/ion/ion_page_pool.c
+++ b/drivers/staging/android/ion/ion_page_pool.c
@@ -14,12 +14,7 @@
  *
  */
 
-#include 
-#include 
-#include 
-#include 
 #include 
-#include 
 #include 
 #include 
 
@@ -171,9 +166,3 @@ void ion_page_pool_destroy(struct ion_page_pool *pool)
 {
kfree(pool);
 }
-
-static int __init ion_page_pool_init(void)
-{
-   return 0;
-}
-device_initcall(ion_page_pool_init);
-- 
1.7.12.4

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

Re: [PATCH 05/10] Staging: rtl8192e: kconfig: Remove empty help text

[Ulf Magnusson]

On Wed, Jan 31, 2018 at 9:46 AM, Dan Carpenter  wrote:
> On Wed, Jan 31, 2018 at 09:41:24AM +0100, Ulf Magnusson wrote:
>> On Wed, Jan 31, 2018 at 9:06 AM, Dan Carpenter  
>> wrote:
>> > On Tue, Jan 30, 2018 at 08:05:27PM +0100, Ulf Magnusson wrote:
>> >> In preparation for adding a warning ("kconfig: Warn if help text is
>> >> blank"): https://lkml.org/lkml/2018/1/30/516
>> >>
>> >
>> > Please don't start the boddy of the email in the middle of a sentence.
>> >
>> > It looks like this:
>> >
>> > https://marc.info/?l=linux-driver-devel=151733925532413=2
>> >
>> > Notice how the subject is up above the From, Date, and Message-id email
>> > headers?  It's totally disconnected from the body of the email.
>> >
>> > regards,
>> > dan carpenter
>>
>> The 'this is in...' was implied. I'll try to be less of a sloppy bastard.
>>
>
> I do feel that patch descriptions should say what the patch does as well
> as why.  Right now the subject says what but I normally read either the
> body or the subject but not both.
>
> regards,
> dan carpenter
>

I fleshed out the individual commit messages and rolled a new
patchset, which adds the warning at the end:
https://lkml.org/lkml/2018/1/31/137

I wonder why that one doesn't display with proper threading on LKML.
It does show the entire thread once you click on one of the later
messages.

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

[PATCH 05/11] Staging: rtl8192e: kconfig: Remove blank help text

[Ulf Magnusson]

Blank help texts are probably either a typo, a Kconfig misunderstanding,
or some kind of half-committing to adding a help text (in which case a
TODO comment would be clearer, if the help text really can't be added
right away).

Best to remove them, IMO.

Signed-off-by: Ulf Magnusson 
---
 drivers/staging/rtl8192e/rtl8192e/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/Kconfig 
b/drivers/staging/rtl8192e/rtl8192e/Kconfig
index 282e293da18f..7ac42a590e21 100644
--- a/drivers/staging/rtl8192e/rtl8192e/Kconfig
+++ b/drivers/staging/rtl8192e/rtl8192e/Kconfig
@@ -6,4 +6,3 @@ config RTL8192E
select WEXT_PRIV
select CRYPTO
select FW_LOADER
-   ---help---
-- 
2.14.1

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

[PATCH 04/11] Staging: rtl8192u: kconfig: Remove blank help text

[Ulf Magnusson]

Blank help texts are probably either a typo, a Kconfig misunderstanding,
or some kind of half-committing to adding a help text (in which case a
TODO comment would be clearer, if the help text really can't be added
right away).

Best to remove them, IMO.

Signed-off-by: Ulf Magnusson 
---
 drivers/staging/rtl8192u/Kconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/staging/rtl8192u/Kconfig b/drivers/staging/rtl8192u/Kconfig
index 3ee9d0d00fb6..97df6507a485 100644
--- a/drivers/staging/rtl8192u/Kconfig
+++ b/drivers/staging/rtl8192u/Kconfig
@@ -5,4 +5,3 @@ config RTL8192U
select WIRELESS_EXT
select WEXT_PRIV
select CRYPTO
-   ---help---
-- 
2.14.1

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

Re: [PATCH 05/10] Staging: rtl8192e: kconfig: Remove empty help text

[Dan Carpenter]

On Wed, Jan 31, 2018 at 09:41:24AM +0100, Ulf Magnusson wrote:
> On Wed, Jan 31, 2018 at 9:06 AM, Dan Carpenter  
> wrote:
> > On Tue, Jan 30, 2018 at 08:05:27PM +0100, Ulf Magnusson wrote:
> >> In preparation for adding a warning ("kconfig: Warn if help text is
> >> blank"): https://lkml.org/lkml/2018/1/30/516
> >>
> >
> > Please don't start the boddy of the email in the middle of a sentence.
> >
> > It looks like this:
> >
> > https://marc.info/?l=linux-driver-devel=151733925532413=2
> >
> > Notice how the subject is up above the From, Date, and Message-id email
> > headers?  It's totally disconnected from the body of the email.
> >
> > regards,
> > dan carpenter
> 
> The 'this is in...' was implied. I'll try to be less of a sloppy bastard.
> 

I do feel that patch descriptions should say what the patch does as well
as why.  Right now the subject says what but I normally read either the
body or the subject but not both.

regards,
dan carpenter

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

Re: [PATCH] staging: rtlwifi: add identifier names to function definition arguments

[Dan Carpenter]

On Sun, Jan 28, 2018 at 12:41:25PM -0600, Larry Finger wrote:
> On 01/28/2018 08:05 AM, Erik Liodden wrote:
> > Add identifier names to function definition arguments to comply with
> > the kernel coding style and the naming convention in the rest of the
> > file.
> > 
> > Issues found by checkpatch.
> > 
> > Signed-off-by: Erik Liodden 
> > ---
> 
> As far as I am concerned, this patch is merely churning the source. Those
> prototypes correctly describe the calling sequence to the compiler, which is
> all that is necessary. Checkpatch is a moving specification. These were NOT
> ERRORS/WARNINGS/CHECKS when this code was first introduced. Why should it be
> made to conform to it now?

Because it's still in staging...  The checkpatch rule is new but it's
sort of been an unwritten rule for almost two decades.

regards,
dan carpenter

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

Re: [PATCH 05/10] Staging: rtl8192e: kconfig: Remove empty help text

[Ulf Magnusson]

On Wed, Jan 31, 2018 at 9:06 AM, Dan Carpenter  wrote:
> On Tue, Jan 30, 2018 at 08:05:27PM +0100, Ulf Magnusson wrote:
>> In preparation for adding a warning ("kconfig: Warn if help text is
>> blank"): https://lkml.org/lkml/2018/1/30/516
>>
>
> Please don't start the boddy of the email in the middle of a sentence.
>
> It looks like this:
>
> https://marc.info/?l=linux-driver-devel=151733925532413=2
>
> Notice how the subject is up above the From, Date, and Message-id email
> headers?  It's totally disconnected from the body of the email.
>
> regards,
> dan carpenter

The 'this is in...' was implied. I'll try to be less of a sloppy bastard.

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

Re: [PATCH] Staging: speakup: Fixed a multiple assignment issue

[Greg KH]

On Wed, Jan 31, 2018 at 07:19:05PM +1300, Shannon Booth wrote:
> Fixed a coding style issue.
> 
> Signed-off-by: Shannon Booth 
> ---
>  drivers/staging/speakup/main.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index cf12590..b721f41 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -261,9 +261,12 @@ static unsigned char get_attributes(struct vc_data *vc, 
> u16 *pos)
>  
>  static void speakup_date(struct vc_data *vc)
>  {
> - spk_x = spk_cx = vc->vc_x;
> - spk_y = spk_cy = vc->vc_y;
> - spk_pos = spk_cp = vc->vc_pos;
> + spk_x =  vc->vc_x;
> + spk_cx = vc->vc_x;
> + spk_y = vc->vc_y;
> + spk_cy = vc->vc_y;
> + spk_pos =  vc->vc_pos;
> + spk_cp = vc->vc_pos;

The original code looks much better than this, don't you agree?

Coding style is just a guideline, no need to change things that don't
make sense.

thanks,

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

Re: [PATCH v2] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap

[Greg KH]

On Wed, Jan 31, 2018 at 01:44:47AM +0200, alexey wrote:
> 
> 
> On 01/31/2018 12:15 AM, Greg KH wrote:
> > On Tue, Jan 30, 2018 at 10:39:13PM +0200, Alexey Skidanov wrote:
> > > dma_buf_vmap and dma_buf_vunmap allow drivers to access buffers, created 
> > > by ion.
> > But why would anyone ever want to do that?  What is wrong with the
> > existing interfaces that drivers use to access buffers created by ion?
> Any driver, sharing the buffers, created by ion, through dma-buf, may get
> back the
> sgtable describing the buffer for device DMA  and may call dma_buf_vmap to
> get back
> the kernel virtual address of the buffer to get access to it. Currently, the
> second
> option is missing. Actually, the buffer already mapped by ion
> implementation.

Very odd formatting :(

Anyway, as Dan said, this needs to be put in the changelog text itself.

I know Intel has documentation and training for how to submit patches to
the kernel properly.  Please use that resource and get your patch
submission reviewed by someone from the Linux kernel team at Intel
before resending this.  Don't force the community to help you learn this
when you have people whose job it is to help out with this at your
company :)

thanks,

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

Re: kernel BUG at drivers/android/binder_alloc.c:LINE!

[Dan Carpenter]

On Tue, Jan 30, 2018 at 11:59:47PM -0800, Eric Biggers wrote:
> On Fri, Dec 01, 2017 at 04:22:00PM -0800, syzbot wrote:
> > syzkaller has found reproducer for the following crash on
> > 3c1c4ddffb58b9e10b3365764fe59546130b3f32
> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
> > compiler: gcc (GCC) 7.1.1 20170620
> > .config is attached
> > Raw console output is attached.
> > C reproducer is attached
> > syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> > for information about syzkaller reproducers
> > 
> > 
> > binder: 3087:3087 ERROR: BC_REGISTER_LOOPER called without request
> > binder: 3087:3087 ioctl c0306201 2000dfd0 returned -14
> > [ cut here ]
> > kernel BUG at drivers/android/binder_alloc.c:750!
> > invalid opcode:  [#1] SMP KASAN
> > Dumping ftrace buffer:
> >(ftrace buffer empty)
> > Modules linked in:
> > CPU: 0 PID: 1404 Comm: kworker/0:2 Not tainted 4.15.0-rc1+ #203
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > Google 01/01/2011
> > Workqueue: events binder_deferred_func
> > task: 09b739b6 task.stack: c1d4442c
> > RIP: 0010:binder_alloc_deferred_release+0x146/0xa40
> > drivers/android/binder_alloc.c:750
> > RSP: 0018:8801d2b16fd8 EFLAGS: 00010293
> > RAX: 8801d2b08080 RBX: 8801d7829300 RCX: 8403b856
> > RDX:  RSI: 0004 RDI: 8801d7829330
> > RBP: 8801d2b17138 R08: 8403b7d9 R09: 10e53001
> > R10: 8801d2b16fc8 R11: 87489d60 R12: 
> > R13: dc00 R14: 8801d2b17110 R15: 8801d7829310
> > FS:  () GS:8801db40() knlGS:
> > CS:  0010 DS:  ES:  CR0: 80050033
> > CR2: 006d00a8 CR3: 05e25000 CR4: 001406f0
> > DR0:  DR1:  DR2: 
> > DR3:  DR6: fffe0ff0 DR7: 0400
> > Call Trace:
> >  binder_free_proc drivers/android/binder.c:4200 [inline]
> >  binder_proc_dec_tmpref+0x2f3/0x420 drivers/android/binder.c:1833
> >  binder_deferred_release drivers/android/binder.c:4858 [inline]
> >  binder_deferred_func+0xe22/0x12f0 drivers/android/binder.c:4893
> >  process_one_work+0xbfd/0x1be0 kernel/workqueue.c:2112
> >  worker_thread+0x223/0x1990 kernel/workqueue.c:2246
> >  kthread+0x37a/0x440 kernel/kthread.c:238
> >  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:441
> > Code: e8 00 40 6c fd 49 8d 7f 20 49 8d 5f f0 48 89 fa 48 c1 ea 03 42 80 3c
> > 2a 00 0f 85 84 07 00 00 49 83 7f 20 00 74 a9 e8 da 3f 6c fd <0f> 0b 48 8b 9d
> > e8 fe ff ff 44 89 a5 bc fe ff ff e8 c5 3f 6c fd
> > RIP: binder_alloc_deferred_release+0x146/0xa40
> > drivers/android/binder_alloc.c:750 RSP: 8801d2b16fd8
> > ---[ end trace 616e085d0dbf3c21 ]---
> > Kernel panic - not syncing: Fatal exception
> > Dumping ftrace buffer:
> >(ftrace buffer empty)
> > Kernel Offset: disabled
> > Rebooting in 86400 seconds..
> 
> This crash stopped occurring in v4.15-rc3, and I verified that it was commit
> fb2c445277e which fixed it (thanks Martijn!).  So, telling syzbot so that it 
> can
> start reporting similar crashes again:
> 
> #syz fix: ANDROID: binder: fix transaction leak.

Could you change the syz fix format to include the hash in case we want
to look it up?

regards,
dan carpenter

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

Re: [PATCH 05/10] Staging: rtl8192e: kconfig: Remove empty help text

[Dan Carpenter]

On Tue, Jan 30, 2018 at 08:05:27PM +0100, Ulf Magnusson wrote:
> In preparation for adding a warning ("kconfig: Warn if help text is
> blank"): https://lkml.org/lkml/2018/1/30/516
> 

Please don't start the boddy of the email in the middle of a sentence.

It looks like this:

https://marc.info/?l=linux-driver-devel=151733925532413=2

Notice how the subject is up above the From, Date, and Message-id email
headers?  It's totally disconnected from the body of the email.

regards,
dan carpenter



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

Re: kernel BUG at drivers/android/binder_alloc.c:LINE!

[Eric Biggers]

On Fri, Dec 01, 2017 at 04:22:00PM -0800, syzbot wrote:
> syzkaller has found reproducer for the following crash on
> 3c1c4ddffb58b9e10b3365764fe59546130b3f32
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached
> Raw console output is attached.
> C reproducer is attached
> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> for information about syzkaller reproducers
> 
> 
> binder: 3087:3087 ERROR: BC_REGISTER_LOOPER called without request
> binder: 3087:3087 ioctl c0306201 2000dfd0 returned -14
> [ cut here ]
> kernel BUG at drivers/android/binder_alloc.c:750!
> invalid opcode:  [#1] SMP KASAN
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 1404 Comm: kworker/0:2 Not tainted 4.15.0-rc1+ #203
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Workqueue: events binder_deferred_func
> task: 09b739b6 task.stack: c1d4442c
> RIP: 0010:binder_alloc_deferred_release+0x146/0xa40
> drivers/android/binder_alloc.c:750
> RSP: 0018:8801d2b16fd8 EFLAGS: 00010293
> RAX: 8801d2b08080 RBX: 8801d7829300 RCX: 8403b856
> RDX:  RSI: 0004 RDI: 8801d7829330
> RBP: 8801d2b17138 R08: 8403b7d9 R09: 10e53001
> R10: 8801d2b16fc8 R11: 87489d60 R12: 
> R13: dc00 R14: 8801d2b17110 R15: 8801d7829310
> FS:  () GS:8801db40() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 006d00a8 CR3: 05e25000 CR4: 001406f0
> DR0:  DR1:  DR2: 
> DR3:  DR6: fffe0ff0 DR7: 0400
> Call Trace:
>  binder_free_proc drivers/android/binder.c:4200 [inline]
>  binder_proc_dec_tmpref+0x2f3/0x420 drivers/android/binder.c:1833
>  binder_deferred_release drivers/android/binder.c:4858 [inline]
>  binder_deferred_func+0xe22/0x12f0 drivers/android/binder.c:4893
>  process_one_work+0xbfd/0x1be0 kernel/workqueue.c:2112
>  worker_thread+0x223/0x1990 kernel/workqueue.c:2246
>  kthread+0x37a/0x440 kernel/kthread.c:238
>  ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:441
> Code: e8 00 40 6c fd 49 8d 7f 20 49 8d 5f f0 48 89 fa 48 c1 ea 03 42 80 3c
> 2a 00 0f 85 84 07 00 00 49 83 7f 20 00 74 a9 e8 da 3f 6c fd <0f> 0b 48 8b 9d
> e8 fe ff ff 44 89 a5 bc fe ff ff e8 c5 3f 6c fd
> RIP: binder_alloc_deferred_release+0x146/0xa40
> drivers/android/binder_alloc.c:750 RSP: 8801d2b16fd8
> ---[ end trace 616e085d0dbf3c21 ]---
> Kernel panic - not syncing: Fatal exception
> Dumping ftrace buffer:
>(ftrace buffer empty)
> Kernel Offset: disabled
> Rebooting in 86400 seconds..

This crash stopped occurring in v4.15-rc3, and I verified that it was commit
fb2c445277e which fixed it (thanks Martijn!).  So, telling syzbot so that it can
start reporting similar crashes again:

#syz fix: ANDROID: binder: fix transaction leak.

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

Re: [PATCH v2] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap

[Dan Carpenter]

The point is that you have to send a v3 patch which includes that
information.  And please CC all the android folk as well.

regards,
dan carpenter

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