Re: [PATCH] Staging : Add RIFFA PCIe staging driver

2018-12-03 Thread gre...@linuxfoundation.org
On Tue, Dec 04, 2018 at 02:19:38AM +, Cheng Fei Phung wrote:
> This patch adds RIFFA PCIe linux driver for 
> https://github.com/promach/riffa/tree/full_duplex/driver/linux
> 
> This staging driver is modified from this upstream driver at 
> https://github.com/KastnerRG/riffa/tree/master/driver/linux

Please properly wrap your changelog text at 72 columns

> For further details, please refer to 
> https://github.com/KastnerRG/riffa/pull/31

That is not permanent, please provide the details here.

> Signed-off-by: Cheng Fei Phung 
> 
> ---
> Changes in v1:
> - added full-duplex capability

And ignored everything else I asked about?  That's not nice to reviewers
at all.  Please at least comment on things that have been asked about,
otherwise why would anyone want to review this?

Also, is this "v2"?  You didn't say so in your subject line.

Please fix this all up and submit a v3, after at least commenting on the
things asked previously.

Also, you still have this line which prevents me from being able to
accept this patch, as I talked about previously:

> + {PCI_DEVICE(VENDOR_ID1, PCI_ANY_ID)},

Do NOT just bind to all PCI devices from a vendor, that will break other
drivers in the system.

Also, do not use a random MAJOR number that you just picked out of no
where, that too will break working systems and needs to be fixed.  Use
the dynamic allocation method, or better yet, just use a misc device.

thanks,

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


Re: [PATCH] staging: mt7621-spi: drop the broken full-duplex mode

2018-12-03 Thread Chuanhong Guo
Hi!
NeilBrown  于2018年12月4日周二 上午5:55写道:
>
> On Mon, Dec 03 2018, Chuanhong Guo wrote:
>
> > Under MORE_BUF_MODE the controller will always shift one bit out of
> > spi_opcode if (mosi_bit_cnt > 0) && (cmd_bit_cnt == 0) so the full-
> > duplex mode is broken since we can't read anything from MISO during
> > writing spi_opcode.
> > This piece of code also make CS1 unavailable since it forces the
> > broken full-duplex mode to be enabled on CS1.
>
> Hi,
>  How do you know these details of the hardware?  Do you have detailed
>  specs for the hardware or have you experimented with it or are you one
>  of the designers or ?
>  It would be nice to have the source of this information documented.
Those info are provided by John Crispin on IRC #openwrt-devel. Here is
the quoted reply:
---
Nov 26 17:00:15  gch981213[w]: so basically i made cs1 work
for MTK/labs when i built the linkit smart for them. the req-sheet
said that cs1 should be proper duplex spi. however 
Nov 26 17:00:34  1) the core will always send 1 byte before
any transfer, this is the m25p80 command.
Nov 26 17:00:49  2) mode 3 is broken and bit reversed (?)
Nov 26 17:01:01  3) some bit are incorrectly wired in hw for mode2/3
Nov 26 17:01:50  we wrote a test script and test for
[0-0x] on all modes and certain bits are swizzled under certain
conditions and it was not possible to fix this even using a hack.
Nov 26 17:02:06  we then decided to use spi-gpio and i never
removed the errornous code
Nov 26 17:02:38  basically the spi is fecked for anything but
half duplex spi mode0 running a sflash on it
---
And another guy told me that his experiment shows there will be one
extra bit if we set cmd_bit_cnt to 0 and mosi_bit_cnt > 0.
I guess it doesn't matter whether it's an extra "byte" or "bit" since
there are always some data that have to be sent under half-duplex.

Do I need to add "some experiment shows that" at the beginning of my
commit message?
>
> Thanks,
> NeilBrown
>
>
> >
> > Signed-off-by: Chuanhong Guo 
> > ---
> >  drivers/staging/mt7621-spi/spi-mt7621.c | 120 +++-
> >  1 file changed, 15 insertions(+), 105 deletions(-)
> >
> > diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c 
> > b/drivers/staging/mt7621-spi/spi-mt7621.c
> > index d045b5568e0f..8af6f307e176 100644
> > --- a/drivers/staging/mt7621-spi/spi-mt7621.c
> > +++ b/drivers/staging/mt7621-spi/spi-mt7621.c
> > @@ -86,16 +86,13 @@ static inline void mt7621_spi_write(struct mt7621_spi 
> > *rs, u32 reg, u32 val)
> >   iowrite32(val, rs->base + reg);
> >  }
> >
> > -static void mt7621_spi_reset(struct mt7621_spi *rs, int duplex)
> > +static void mt7621_spi_reset(struct mt7621_spi *rs)
> >  {
> >   u32 master = mt7621_spi_read(rs, MT7621_SPI_MASTER);
> >
> >   master |= 7 << 29;
> >   master |= 1 << 2;
> > - if (duplex)
> > - master |= 1 << 10;
> > - else
> > - master &= ~(1 << 10);
> > + master &= ~(1 << 10);
> >
> >   mt7621_spi_write(rs, MT7621_SPI_MASTER, master);
> >   rs->pending_write = 0;
> > @@ -107,7 +104,7 @@ static void mt7621_spi_set_cs(struct spi_device *spi, 
> > int enable)
> >   int cs = spi->chip_select;
> >   u32 polar = 0;
> >
> > - mt7621_spi_reset(rs, cs);
> > + mt7621_spi_reset(rs);
> >   if (enable)
> >   polar = BIT(cs);
> >   mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
> > @@ -261,7 +258,7 @@ static void mt7621_spi_write_half_duplex(struct 
> > mt7621_spi *rs,
> >   rs->pending_write = len;
> >  }
> >
> > -static int mt7621_spi_transfer_half_duplex(struct spi_master *master,
> > +static int mt7621_spi_transfer_one_message(struct spi_master *master,
> >  struct spi_message *m)
> >  {
> >   struct mt7621_spi *rs = spi_master_get_devdata(master);
> > @@ -284,7 +281,16 @@ static int mt7621_spi_transfer_half_duplex(struct 
> > spi_master *master,
> >   mt7621_spi_set_cs(spi, 1);
> >   m->actual_length = 0;
> >   list_for_each_entry(t, >transfers, transfer_list) {
> > - if (t->rx_buf)
> > + if((t->rx_buf) && (t->tx_buf)) {
> > + /* This controller will shift one extra bit out
> > +  * of spi_opcode if (mosi_bit_cnt > 0) &&
> > +  * (cmd_bit_cnt == 0). So the claimed full-duplex
> > +  * support is broken since we have no way to read
> > +  * the MISO value during that bit.
> > +  */
> > + status = -EIO;
> > + goto msg_done;
> > + } else if (t->rx_buf)
> >   mt7621_spi_read_half_duplex(rs, t->len, t->rx_buf);
> >   else if (t->tx_buf)
> >   mt7621_spi_write_half_duplex(rs, t->len, t->tx_buf);
> > @@ -300,102 +306,6 @@ static int mt7621_spi_transfer_half_duplex(struct 
> > spi_master *master,
> >   return 0;
> >  }
> >
> > -static int 

[PATCH] Staging : Add RIFFA PCIe staging driver

2018-12-03 Thread Cheng Fei Phung
This patch adds RIFFA PCIe linux driver for 
https://github.com/promach/riffa/tree/full_duplex/driver/linux

This staging driver is modified from this upstream driver at 
https://github.com/KastnerRG/riffa/tree/master/driver/linux

For further details, please refer to https://github.com/KastnerRG/riffa/pull/31

Signed-off-by: Cheng Fei Phung 

---
Changes in v1:
- added full-duplex capability

 drivers/staging/riffa/Kconfig|5 +
 drivers/staging/riffa/Makefile   |1 +
 drivers/staging/riffa/TODO   |7 +
 drivers/staging/riffa/circ_queue.c   |  188 +++
 drivers/staging/riffa/circ_queue.h   |   96 ++
 drivers/staging/riffa/riffa.c|  152 +++
 drivers/staging/riffa/riffa.h|  121 ++
 drivers/staging/riffa/riffa_driver.c | 1643 ++
 drivers/staging/riffa/riffa_driver.h |  131 ++
 9 files changed, 2344 insertions(+)
 create mode 100644 drivers/staging/riffa/Kconfig
 create mode 100644 drivers/staging/riffa/Makefile
 create mode 100644 drivers/staging/riffa/TODO
 create mode 100644 drivers/staging/riffa/circ_queue.c
 create mode 100644 drivers/staging/riffa/circ_queue.h
 create mode 100644 drivers/staging/riffa/riffa.c
 create mode 100644 drivers/staging/riffa/riffa.h
 create mode 100644 drivers/staging/riffa/riffa_driver.c
 create mode 100644 drivers/staging/riffa/riffa_driver.h

diff --git a/drivers/staging/riffa/Kconfig b/drivers/staging/riffa/Kconfig
new file mode 100644
index ..b8fa2d51fbc0
--- /dev/null
+++ b/drivers/staging/riffa/Kconfig
@@ -0,0 +1,5 @@
+config RIFFA_PCIE
+   tristate "a simple framework for communicating data from a host CPU to 
a FPGA via a PCI Express bus"
+   depends on PCI
+   ---help---
+   Transfers data with full duplex capability using PCIe protocol
diff --git a/drivers/staging/riffa/Makefile b/drivers/staging/riffa/Makefile
new file mode 100644
index ..79ef3b9b8c8f
--- /dev/null
+++ b/drivers/staging/riffa/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_RIFFA) += riffa.o circ_queue.o riffa_driver.o riffa_mod.o
diff --git a/drivers/staging/riffa/TODO b/drivers/staging/riffa/TODO
new file mode 100644
index ..5f13b32a1f91
--- /dev/null
+++ b/drivers/staging/riffa/TODO
@@ -0,0 +1,7 @@
+TODO:
+- optimize the driver code for further speed improvement although it can now 
achieve defined PCIe speed grade
+- solve all the coding style errors from scripts/checkpatch.pl
+- add vendor and device IDs for more supported devices after actual hardware 
testing 
+
+Please send any patches to Greg Kroah-Hartman 
+and Cheng Fei Phung
diff --git a/drivers/staging/riffa/circ_queue.c 
b/drivers/staging/riffa/circ_queue.c
new file mode 100644
index ..fb43ca22e3c0
--- /dev/null
+++ b/drivers/staging/riffa/circ_queue.c
@@ -0,0 +1,188 @@
+// --
+// Copyright (c) 2016, The Regents of the University of California All
+// rights reserved.
+// 
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+// 
+// * Redistributions of source code must retain the above copyright
+//   notice, this list of conditions and the following disclaimer.
+// 
+// * Redistributions in binary form must reproduce the above
+//   copyright notice, this list of conditions and the following
+//   disclaimer in the documentation and/or other materials provided
+//   with the distribution.
+// 
+// * Neither the name of The Regents of the University of California
+//   nor the names of its contributors may be used to endorse or
+//   promote products derived from this software without specific
+//   prior written permission.
+// 
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL REGENTS OF THE
+// UNIVERSITY OF CALIFORNIA BE LIABLE FOR ANY DIRECT, INDIRECT,
+// INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+// BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+// OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+// ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
+// TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
+// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+// DAMAGE.
+// --
+
+/*
+ * Filename: circ_queue.c
+ * Version: 1.0
+ * Description: A lock-free single-producer circular queue implementation 
+ *   modeled after the more elaborate C++ version from Faustino Frechilla at:
+ *   
http://www.codeproject.com/Articles/153898/Yet-another-implementation-of-a-lock-free-circular
+ * Author: 

WAITING FOR YOUR POSITIVE RESPONSE

2018-12-03 Thread ANNA BLAIR
My Dear Friend,

Let me first of all inform you, I got your email address from a mail Directory 
and decided to mail you for a permission to go ahead. I am Mrs. Anna Blair from 
United Kingdom, married to Dr. Anthony R. Blair who worked with Texaco Oil 
Company in Malaysia before he died in a plane crash on his way to a Board 
meeting. My Husband and I were married but without any children. Since his 
death I decided not to re-marry and presently I am 79 Years old. When my late 
husband was Alive he deposited the sum of $11.5M. (Eleven Million Five Hundred 
Thousand U.S. Dollars) with a Bank.

Presently this money is still with the Bank and the management just wrote me as 
the beneficiary to come forward to receive the money or rather Issue a letter 
of authority to somebody to receive it on my behalf. I am presently in a 
hospital where I have been undergoing treatment Cancer of the lungs. I have 
since lost my ability to talk and my doctors have told me that I have only a 
few months to live so I think the best thing to do is to use the money for 
charity purposes.

I want a person who is trustworthy that I will make the beneficiary of my late 
Husband's Fund deposited with the bank so that the person can get the money and 
utilize 70% of this money to fund churches, orphanages and widows around the 
world.

As soon as I receive your reply I shall give you the contact details of the 
Bank. I will also issue you a letter of authority that will prove you as the 
new beneficiary of this fund.Please assure me that you will act accordingly as 
I stated here in and Keep this contact confidential till such a time this funds 
get to your Custody. This is  to ensure that nothing jeopardizes my last wish 
on earth.

I await your urgent reply.

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


Re: [PATCH] binder: fix sparse warnings on locking context

2018-12-03 Thread Luc Van Oostenryck
On Mon, Dec 03, 2018 at 12:24:54PM -0800, Todd Kjos wrote:
> Add __acquire()/__release() annnotations to fix warnings
> in sparse context checking
> 
> There is one case where the warning was due to a lack of
> a "default:" case in a switch statement where a lock was
> being released in each of the cases, so the default
> case was added.
> 
> Signed-off-by: Todd Kjos 
> ---
>  drivers/android/binder.c   | 43 +-
>  drivers/android/binder_alloc.c |  1 +
>  2 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index 9f1000d2a40c7..9f2059d24ae2d 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -751,12 +757,16 @@ _binder_node_unlock(struct binder_node *node, int line)
>  #define binder_node_inner_lock(node) _binder_node_inner_lock(node, __LINE__)
>  static void
>  _binder_node_inner_lock(struct binder_node *node, int line)
> + __acquires(>lock) __acquires(>proc->inner_lock)
>  {
>   binder_debug(BINDER_DEBUG_SPINLOCKS,
>"%s: line=%d\n", __func__, line);
>   spin_lock(>lock);
>   if (node->proc)
>   binder_inner_proc_lock(node->proc);
> + else
> + /* annotation for sparse */
> + __acquire(>proc->inner_lock);

This one is questionnable because:
1) if !node->proc, then '>proc->inner_lock' is not acquired
   since it doesn't even exist.
2) OTOH, the function can't have the annotation 100% right because
   it semantics allows unbalanced locking depending on node->proc
   being null or not.
But I see very well the intent and maybe it's a right solution.
I dunno.

Same for most of the following ones.


Best regards,
-- Luc Van Oostenryck
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: mt7621-spi: drop the broken full-duplex mode

2018-12-03 Thread NeilBrown
On Mon, Dec 03 2018, Chuanhong Guo wrote:

> Under MORE_BUF_MODE the controller will always shift one bit out of
> spi_opcode if (mosi_bit_cnt > 0) && (cmd_bit_cnt == 0) so the full-
> duplex mode is broken since we can't read anything from MISO during
> writing spi_opcode.
> This piece of code also make CS1 unavailable since it forces the
> broken full-duplex mode to be enabled on CS1.

Hi,
 How do you know these details of the hardware?  Do you have detailed
 specs for the hardware or have you experimented with it or are you one
 of the designers or ?
 It would be nice to have the source of this information documented.

Thanks,
NeilBrown


>
> Signed-off-by: Chuanhong Guo 
> ---
>  drivers/staging/mt7621-spi/spi-mt7621.c | 120 +++-
>  1 file changed, 15 insertions(+), 105 deletions(-)
>
> diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c 
> b/drivers/staging/mt7621-spi/spi-mt7621.c
> index d045b5568e0f..8af6f307e176 100644
> --- a/drivers/staging/mt7621-spi/spi-mt7621.c
> +++ b/drivers/staging/mt7621-spi/spi-mt7621.c
> @@ -86,16 +86,13 @@ static inline void mt7621_spi_write(struct mt7621_spi 
> *rs, u32 reg, u32 val)
>   iowrite32(val, rs->base + reg);
>  }
>  
> -static void mt7621_spi_reset(struct mt7621_spi *rs, int duplex)
> +static void mt7621_spi_reset(struct mt7621_spi *rs)
>  {
>   u32 master = mt7621_spi_read(rs, MT7621_SPI_MASTER);
>  
>   master |= 7 << 29;
>   master |= 1 << 2;
> - if (duplex)
> - master |= 1 << 10;
> - else
> - master &= ~(1 << 10);
> + master &= ~(1 << 10);
>  
>   mt7621_spi_write(rs, MT7621_SPI_MASTER, master);
>   rs->pending_write = 0;
> @@ -107,7 +104,7 @@ static void mt7621_spi_set_cs(struct spi_device *spi, int 
> enable)
>   int cs = spi->chip_select;
>   u32 polar = 0;
>  
> - mt7621_spi_reset(rs, cs);
> + mt7621_spi_reset(rs);
>   if (enable)
>   polar = BIT(cs);
>   mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
> @@ -261,7 +258,7 @@ static void mt7621_spi_write_half_duplex(struct 
> mt7621_spi *rs,
>   rs->pending_write = len;
>  }
>  
> -static int mt7621_spi_transfer_half_duplex(struct spi_master *master,
> +static int mt7621_spi_transfer_one_message(struct spi_master *master,
>  struct spi_message *m)
>  {
>   struct mt7621_spi *rs = spi_master_get_devdata(master);
> @@ -284,7 +281,16 @@ static int mt7621_spi_transfer_half_duplex(struct 
> spi_master *master,
>   mt7621_spi_set_cs(spi, 1);
>   m->actual_length = 0;
>   list_for_each_entry(t, >transfers, transfer_list) {
> - if (t->rx_buf)
> + if((t->rx_buf) && (t->tx_buf)) {
> + /* This controller will shift one extra bit out
> +  * of spi_opcode if (mosi_bit_cnt > 0) &&
> +  * (cmd_bit_cnt == 0). So the claimed full-duplex
> +  * support is broken since we have no way to read
> +  * the MISO value during that bit.
> +  */
> + status = -EIO;
> + goto msg_done;
> + } else if (t->rx_buf)
>   mt7621_spi_read_half_duplex(rs, t->len, t->rx_buf);
>   else if (t->tx_buf)
>   mt7621_spi_write_half_duplex(rs, t->len, t->tx_buf);
> @@ -300,102 +306,6 @@ static int mt7621_spi_transfer_half_duplex(struct 
> spi_master *master,
>   return 0;
>  }
>  
> -static int mt7621_spi_transfer_full_duplex(struct spi_master *master,
> -struct spi_message *m)
> -{
> - struct mt7621_spi *rs = spi_master_get_devdata(master);
> - struct spi_device *spi = m->spi;
> - unsigned int speed = spi->max_speed_hz;
> - struct spi_transfer *t = NULL;
> - int status = 0;
> - int i, len = 0;
> - int rx_len = 0;
> - u32 data[9] = { 0 };
> - u32 val = 0;
> -
> - mt7621_spi_wait_till_ready(rs);
> -
> - list_for_each_entry(t, >transfers, transfer_list) {
> - const u8 *buf = t->tx_buf;
> -
> - if (t->rx_buf)
> - rx_len += t->len;
> -
> - if (!buf)
> - continue;
> -
> - if (WARN_ON(len + t->len > 16)) {
> - status = -EIO;
> - goto msg_done;
> - }
> -
> - for (i = 0; i < t->len; i++, len++)
> - data[len / 4] |= buf[i] << (8 * (len & 3));
> - if (speed > t->speed_hz)
> - speed = t->speed_hz;
> - }
> -
> - if (WARN_ON(rx_len > 16)) {
> - status = -EIO;
> - goto msg_done;
> - }
> -
> - if (mt7621_spi_prepare(spi, speed)) {
> - status = -EIO;
> - goto msg_done;
> - }
> -
> - for (i = 0; i < len; i += 4)
> - mt7621_spi_write(rs, MT7621_SPI_DATA0 + i, data[i / 4]);
> -

Re: [PATCH RFCv2 1/4] mm/memory_hotplug: Introduce memory block types

2018-12-03 Thread Wei Yang
[...]
>>>
>>> +   if (type == MEMORY_BLOCK_NONE)
>>> +   return -EINVAL;
>> 
>> No one will pass in this value. Can we omit this check for now?
>
>I could move it to patch nr 2 I guess, but as I introduce
>MEMORY_BLOCK_NONE here it made sense to keep it in here.
>

Yes, this make sense to me now.

>(and I think at least for now it makes sense to not squash patch 1 and
>2, to easier discuss the new user interface/concept introduced in this
>patch).
>
>Thanks!
>
>-- 
>
>Thanks,
>
>David / dhildenb

-- 
Wei Yang
Help you, Help me
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] binder: fix use-after-free due to fdget() optimization

2018-12-03 Thread Todd Kjos
44d8047f1d87a ("binder: use standard functions to allocate fds")
exposed a pre-existing issue in the binder driver.

fdget() is used in ksys_ioctl() as a performance optimization.
One of the rules associated with fdget() is that ksys_close() must
not be called between the fdget() and the fdput(). There is a case
where this requirement is not met in the binder driver (and possibly
other drivers) which results in the reference count dropping to 0
when the device is still in use. This can result in use-after-free
or other issues.

This was observed with the following sequence of events:

Task A and task B are connected via binder; task A has /dev/binder open at
file descriptor number X. Both tasks are single-threaded.

1. task B sends a binder message with a file descriptor array
   (BINDER_TYPE_FDA) containing one file descriptor to task A
2. task A reads the binder message with the translated file
   descriptor number Y
3. task A uses dup2(X, Y) to overwrite file descriptor Y with
   the /dev/binder file
4. task A unmaps the userspace binder memory mapping; the reference
   count on task A's /dev/binder is now 2
5. task A closes file descriptor X; the reference count on task
   A's /dev/binder is now 1
6. task A forks off a child, task C, duplicating the file descriptor
   table; the reference count on task A's /dev/binder is now 2
7. task A invokes the BC_FREE_BUFFER command on file descriptor X
   to release the incoming binder message
8. fdget() in ksys_ioctl() suppresses the reference count increment,
   since the file descriptor table is not shared
9. the BC_FREE_BUFFER handler removes the file descriptor table
   entry for X and decrements the reference count of task A's
   /dev/binder file to 1
10.task C calls close(X), which drops the reference count of
   task A's /dev/binder to 0 and frees it
11.task A continues processing of the ioctl and accesses some
   property of e.g. the binder_proc => KASAN-detectable UAF

Fixed by using get_file() / fput() in binder_ioctl().

Suggested-by: Jann Horn 
Signed-off-by: Todd Kjos 
Acked-by: Martijn Coenen 
---
 drivers/android/binder.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index c525b164d39d2..cbaef3b0d3078 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -4774,6 +4774,13 @@ static long binder_ioctl(struct file *filp, unsigned int 
cmd, unsigned long arg)
unsigned int size = _IOC_SIZE(cmd);
void __user *ubuf = (void __user *)arg;
 
+   /*
+* Need a reference on filp since ksys_close() could
+* be called on binder fd and the fdget() used in
+* ksys_ioctl() might have optimized out the reference.
+*/
+   get_file(filp);
+
/*pr_info("binder_ioctl: %d:%d %x %lx\n",
proc->pid, current->pid, cmd, arg);*/
 
@@ -4885,6 +4892,7 @@ static long binder_ioctl(struct file *filp, unsigned int 
cmd, unsigned long arg)
pr_info("%d:%d ioctl %x %lx returned %d\n", proc->pid, 
current->pid, cmd, arg, ret);
 err_unlocked:
trace_binder_ioctl_done(ret);
+   fput(filp);
return ret;
 }
 
-- 
2.20.0.rc1.387.gf8505762e3-goog

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


[PATCH] binder: fix kerneldoc header for struct binder_buffer

2018-12-03 Thread Todd Kjos
Fix the incomplete kerneldoc header for struct binder_buffer.

Change-Id: If3ca10cf6d90f605a0c078e4cdce28f02a475877
Signed-off-by: Todd Kjos 
---
 drivers/android/binder_alloc.h | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index fb3238c74c8a8..c0aadbbf7f193 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -30,16 +30,16 @@ struct binder_transaction;
  * struct binder_buffer - buffer used for binder transactions
  * @entry:  entry alloc->buffers
  * @rb_node:node for allocated_buffers/free_buffers rb trees
- * @free:   true if buffer is free
- * @allow_user_free:describe the second member of struct blah,
- * @async_transaction:  describe the second member of struct blah,
- * @debug_id:   describe the second member of struct blah,
- * @transaction:describe the second member of struct blah,
- * @target_node:describe the second member of struct blah,
- * @data_size:  describe the second member of struct blah,
- * @offsets_size:   describe the second member of struct blah,
- * @extra_buffers_size: describe the second member of struct blah,
- * @data:i  describe the second member of struct blah,
+ * @free:   %true if buffer is free
+ * @allow_user_free:%true if user is allowed to free buffer
+ * @async_transaction:  %true if buffer is in use for an async txn
+ * @debug_id:   unique ID for debugging
+ * @transaction:pointer to associated struct binder_transaction
+ * @target_node:struct binder_node associated with this buffer
+ * @data_size:  size of @transaction data
+ * @offsets_size:   size of array of offsets
+ * @extra_buffers_size: size of space for other objects (like sg lists)
+ * @data:   pointer to base of buffer space
  *
  * Bookkeeping structure for binder transaction buffers
  */
-- 
2.20.0.rc1.387.gf8505762e3-goog

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


[PATCH] binder: filter out nodes when showing binder procs

2018-12-03 Thread Todd Kjos
When dumping out binder transactions via a debug node,
the output is too verbose if a process has many nodes.
Change the output for transaction dumps to only display
nodes with pending async transactions.

Signed-off-by: Todd Kjos 
---
 drivers/android/binder.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9f2059d24ae2d..c525b164d39d2 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -5432,6 +5432,9 @@ static void print_binder_proc(struct seq_file *m,
for (n = rb_first(>nodes); n != NULL; n = rb_next(n)) {
struct binder_node *node = rb_entry(n, struct binder_node,
rb_node);
+   if (!print_all && !node->has_async_transaction)
+   continue;
+
/*
 * take a temporary reference on the node so it
 * survives and isn't removed from the tree
-- 
2.20.0.rc1.387.gf8505762e3-goog

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


[PATCH] binder: fix sparse warnings on locking context

2018-12-03 Thread Todd Kjos
Add __acquire()/__release() annnotations to fix warnings
in sparse context checking

There is one case where the warning was due to a lack of
a "default:" case in a switch statement where a lock was
being released in each of the cases, so the default
case was added.

Signed-off-by: Todd Kjos 
---
 drivers/android/binder.c   | 43 +-
 drivers/android/binder_alloc.c |  1 +
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index 9f1000d2a40c7..9f2059d24ae2d 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -660,6 +660,7 @@ struct binder_transaction {
 #define binder_proc_lock(proc) _binder_proc_lock(proc, __LINE__)
 static void
 _binder_proc_lock(struct binder_proc *proc, int line)
+   __acquires(>outer_lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -675,6 +676,7 @@ _binder_proc_lock(struct binder_proc *proc, int line)
 #define binder_proc_unlock(_proc) _binder_proc_unlock(_proc, __LINE__)
 static void
 _binder_proc_unlock(struct binder_proc *proc, int line)
+   __releases(>outer_lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -690,6 +692,7 @@ _binder_proc_unlock(struct binder_proc *proc, int line)
 #define binder_inner_proc_lock(proc) _binder_inner_proc_lock(proc, __LINE__)
 static void
 _binder_inner_proc_lock(struct binder_proc *proc, int line)
+   __acquires(>inner_lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -705,6 +708,7 @@ _binder_inner_proc_lock(struct binder_proc *proc, int line)
 #define binder_inner_proc_unlock(proc) _binder_inner_proc_unlock(proc, 
__LINE__)
 static void
 _binder_inner_proc_unlock(struct binder_proc *proc, int line)
+   __releases(>inner_lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -720,6 +724,7 @@ _binder_inner_proc_unlock(struct binder_proc *proc, int 
line)
 #define binder_node_lock(node) _binder_node_lock(node, __LINE__)
 static void
 _binder_node_lock(struct binder_node *node, int line)
+   __acquires(>lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -735,6 +740,7 @@ _binder_node_lock(struct binder_node *node, int line)
 #define binder_node_unlock(node) _binder_node_unlock(node, __LINE__)
 static void
 _binder_node_unlock(struct binder_node *node, int line)
+   __releases(>lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
@@ -751,12 +757,16 @@ _binder_node_unlock(struct binder_node *node, int line)
 #define binder_node_inner_lock(node) _binder_node_inner_lock(node, __LINE__)
 static void
 _binder_node_inner_lock(struct binder_node *node, int line)
+   __acquires(>lock) __acquires(>proc->inner_lock)
 {
binder_debug(BINDER_DEBUG_SPINLOCKS,
 "%s: line=%d\n", __func__, line);
spin_lock(>lock);
if (node->proc)
binder_inner_proc_lock(node->proc);
+   else
+   /* annotation for sparse */
+   __acquire(>proc->inner_lock);
 }
 
 /**
@@ -768,6 +778,7 @@ _binder_node_inner_lock(struct binder_node *node, int line)
 #define binder_node_inner_unlock(node) _binder_node_inner_unlock(node, 
__LINE__)
 static void
 _binder_node_inner_unlock(struct binder_node *node, int line)
+   __releases(>lock) __releases(>proc->inner_lock)
 {
struct binder_proc *proc = node->proc;
 
@@ -775,6 +786,9 @@ _binder_node_inner_unlock(struct binder_node *node, int 
line)
 "%s: line=%d\n", __func__, line);
if (proc)
binder_inner_proc_unlock(proc);
+   else
+   /* annotation for sparse */
+   __release(>proc->inner_lock);
spin_unlock(>lock);
 }
 
@@ -1384,10 +1398,14 @@ static void binder_dec_node_tmpref(struct binder_node 
*node)
binder_node_inner_lock(node);
if (!node->proc)
spin_lock(_dead_nodes_lock);
+   else
+   __acquire(_dead_nodes_lock);
node->tmp_refs--;
BUG_ON(node->tmp_refs < 0);
if (!node->proc)
spin_unlock(_dead_nodes_lock);
+   else
+   __release(_dead_nodes_lock);
/*
 * Call binder_dec_node() to check if all refcounts are 0
 * and cleanup is needed. Calling with strong=0 and internal=1
@@ -1890,18 +1908,22 @@ static struct binder_thread *binder_get_txn_from(
  */
 static struct binder_thread *binder_get_txn_from_and_acq_inner(
struct binder_transaction *t)
+   __acquires(>from->proc->inner_lock)
 {
struct binder_thread *from;
 
from = binder_get_txn_from(t);
-   if (!from)
+   if (!from) {
+   

Re: [PATCH] [repost] Drivers: hv: vmbus: Offload the handling of channels to two workqueues

2018-12-03 Thread Dan Carpenter
On Mon, Dec 03, 2018 at 02:50:19PM +0300, Dan Carpenter wrote:
> The list is still rejecting @microsoft.com patches...  :(
> 
> I mentioned this last time when you guys were complaining that no one
> reads your patches and someone sent me a link to marc.info.  I want to
> help but obviously no one has time to look at patches on marc.info...
> 

Oh... Hm.  Sorry, my bad.  They're making it to the list but somehow
gmail is eating them.  I search my spam folder and they're not there
either.  :(

I don't know what's going on.

regards,
dan carpenter

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


Re: [PATCH] binder: remove BINDER_DEBUG_ENTRY()

2018-12-03 Thread Todd Kjos
On Fri, Nov 30, 2018 at 5:26 PM Yangtao Li  wrote:
>
> We already have the DEFINE_SHOW_ATTRIBUTE.There is no need to define
> such a macro,so remove BINDER_DEBUG_ENTRY.
>
> Signed-off-by: Yangtao Li 

Acked-by: Todd Kjos 

> ---
>  drivers/android/binder.c | 48 ++--
>  1 file changed, 17 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index cb30a524d16d..5496b8e07234 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -94,22 +94,8 @@ static struct dentry *binder_debugfs_dir_entry_root;
>  static struct dentry *binder_debugfs_dir_entry_proc;
>  static atomic_t binder_last_id;
>
> -#define BINDER_DEBUG_ENTRY(name) \
> -static int binder_##name##_open(struct inode *inode, struct file *file) \
> -{ \
> -   return single_open(file, binder_##name##_show, inode->i_private); \
> -} \
> -\
> -static const struct file_operations binder_##name##_fops = { \
> -   .owner = THIS_MODULE, \
> -   .open = binder_##name##_open, \
> -   .read = seq_read, \
> -   .llseek = seq_lseek, \
> -   .release = single_release, \
> -}
> -
> -static int binder_proc_show(struct seq_file *m, void *unused);
> -BINDER_DEBUG_ENTRY(proc);
> +static int proc_show(struct seq_file *m, void *unused);
> +DEFINE_SHOW_ATTRIBUTE(proc);
>
>  /* This is only defined in include/asm-arm/sizes.h */
>  #ifndef SZ_1K
> @@ -4964,7 +4950,7 @@ static int binder_open(struct inode *nodp, struct file 
> *filp)
> proc->debugfs_entry = debugfs_create_file(strbuf, 0444,
> binder_debugfs_dir_entry_proc,
> (void *)(unsigned long)proc->pid,
> -   _proc_fops);
> +   _fops);
> }
>
> return 0;
> @@ -5592,7 +5578,7 @@ static void print_binder_proc_stats(struct seq_file *m,
>  }
>
>
> -static int binder_state_show(struct seq_file *m, void *unused)
> +static int state_show(struct seq_file *m, void *unused)
>  {
> struct binder_proc *proc;
> struct binder_node *node;
> @@ -5631,7 +5617,7 @@ static int binder_state_show(struct seq_file *m, void 
> *unused)
> return 0;
>  }
>
> -static int binder_stats_show(struct seq_file *m, void *unused)
> +static int stats_show(struct seq_file *m, void *unused)
>  {
> struct binder_proc *proc;
>
> @@ -5647,7 +5633,7 @@ static int binder_stats_show(struct seq_file *m, void 
> *unused)
> return 0;
>  }
>
> -static int binder_transactions_show(struct seq_file *m, void *unused)
> +static int transactions_show(struct seq_file *m, void *unused)
>  {
> struct binder_proc *proc;
>
> @@ -5660,7 +5646,7 @@ static int binder_transactions_show(struct seq_file *m, 
> void *unused)
> return 0;
>  }
>
> -static int binder_proc_show(struct seq_file *m, void *unused)
> +static int proc_show(struct seq_file *m, void *unused)
>  {
> struct binder_proc *itr;
> int pid = (unsigned long)m->private;
> @@ -5703,7 +5689,7 @@ static void print_binder_transaction_log_entry(struct 
> seq_file *m,
> "\n" : " (incomplete)\n");
>  }
>
> -static int binder_transaction_log_show(struct seq_file *m, void *unused)
> +static int transaction_log_show(struct seq_file *m, void *unused)
>  {
> struct binder_transaction_log *log = m->private;
> unsigned int log_cur = atomic_read(>cur);
> @@ -5735,10 +5721,10 @@ static const struct file_operations binder_fops = {
> .release = binder_release,
>  };
>
> -BINDER_DEBUG_ENTRY(state);
> -BINDER_DEBUG_ENTRY(stats);
> -BINDER_DEBUG_ENTRY(transactions);
> -BINDER_DEBUG_ENTRY(transaction_log);
> +DEFINE_SHOW_ATTRIBUTE(state);
> +DEFINE_SHOW_ATTRIBUTE(stats);
> +DEFINE_SHOW_ATTRIBUTE(transactions);
> +DEFINE_SHOW_ATTRIBUTE(transaction_log);
>
>  static int __init init_binder_device(const char *name)
>  {
> @@ -5792,27 +5778,27 @@ static int __init binder_init(void)
> 0444,
> binder_debugfs_dir_entry_root,
> NULL,
> -   _state_fops);
> +   _fops);
> debugfs_create_file("stats",
> 0444,
> binder_debugfs_dir_entry_root,
> NULL,
> -   _stats_fops);
> +   _fops);
> debugfs_create_file("transactions",
> 0444,
> binder_debugfs_dir_entry_root,
> NULL,
> -   _transactions_fops);
> +   _fops);
> debugfs_create_file("transaction_log",
> 0444,
> 

Re: [PATCH] staging: iio: ad5933: add binding doc for ad5933

2018-12-03 Thread Jonathan Cameron
On Sun, 2 Dec 2018 14:57:12 -0200
Marcelo Schmitt  wrote:

> Add a devicetree documentation for the ad5933 and ad5934 impedance
> converter, network analyzer.
> 
> Co-Developed-by: Gabriel Capella 

Put this in line below Gabriel's sign off - it makes script
parsing of these easier.

Signed-off-by: Marcelo Schmitt 
Signed-off-by: Gabriel Capella 
Co-Developed-by: Gabriel Capella 

> 
> Signed-off-by: Marcelo Schmitt 
> Signed-off-by: Gabriel Capella 
> ---
>  .../iio/impedance-analyzer/ad5933.txt | 23 +++
>  1 file changed, 23 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt 
> b/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
> new file mode 100644
> index ..d9ae2babf016
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/impedance-analyzer/ad5933.txt
> @@ -0,0 +1,23 @@
> +Analog Devices AD5933/AD5934 Impedance Converter, Network Analyzer
> +
> +https://www.analog.com/media/en/technical-documentation/data-sheets/AD5933.pdf
> +https://www.analog.com/media/en/technical-documentation/data-sheets/AD5934.pdf
> +
> +Required properties:
> + - compatible : should be one of
> + "adi,ad5933"
> + "adi,ad5934"
> + - reg : the I2C address
> + - vdd-supply : supply reference for the device.
> +
> +Optional properties:
> + - vref_mv : default voltage reference.

Regulator framework rather than a simple voltage.

> + - ext_clk_hz : external master clock for the system.

Please use the clocks framework for this rather than rolling our own.
There are fixed clocks to cover the common case, but it may be supplied by
something smarter.

> +
> +Example:
> +
> + impedance-analyzer@0d {
> + compatible = "adi,ad5933";
> + reg = <0x0d>;
> + vdd-supply = <_supply>;
> + };


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


Re: [PATCH] staging: iio: ad5933: replaced kfifo by triggered_buffer

2018-12-03 Thread Jonathan Cameron
On Sun, 2 Dec 2018 16:10:45 -0200
Marcelo Schmitt  wrote:

> On 11/25, Jonathan Cameron wrote:
> > On Thu, 22 Nov 2018 10:53:47 -0200
> > Marcelo Schmitt  wrote:
> >   
> > > Previously, there was an implicit creation of a kfifo which was replaced
> > > by a call to triggered_buffer_setup, which is already implemented in iio
> > > infrastructure.
> > > 
> > > Signed-off-by: Marcelo Schmitt   
> > 
> > I'm a little surprised that this would work without screaming a lot as 
> > it will register an interrupt with no handlers. Do you have this
> > device to test?  It's rapidly heading in the direction of too complex
> > a driver to fix without test hardware.  Also, semantically this change
> > is not sensible as it implies an operating mode which the driver
> > is not using.
> >   
> 
> Thanks for reviewing the patch. I'm not expert at electronics so I'm
> still studying the datasheet to understand how the ad5933 works.
> 
> > There are fundamental questions about how we handle an autotriggered
> > sweep that need answering before this driver can move forwards.
> > It needs some concept of a higher level trigger rather than a per
> > sample one like we typically use in IIO.
> >   
> 
> From what I've read so far it seems that we would need to have two
> operation modes: one for the frequency sweep (that need to be
> discussed yet) and another for the receive stage (in which ad5933 would
> be continuously requested for data through I2C). So my first approach
> would be to build up an abstract trigger that would allow switching
> between these two operation modes. What do you think about that?

Hmm.  I haven't looked at it in much depth yet but based largely on the intro
to the datasheet and what you say here...

Mode 1 : Sweep.
* Controls for peak-to-peak, start, resolution, number of points.

Two ways we could treat this.
* A single buffer with no meta data and rely on start and stop signals
  and careful sync.  This is similar to what we have previously talked
  about doing for impact sensors.

* We could present the current frequency as a channel.  At which point
  this is a simple triple of drive frequency, real and imaginary responses.
  It would need a start control though to get a sweep going. Also potentially
  a control to force a waiting period for the start frequency to sweep
  transition.  There is also the repeat option to deal with.

Mode 2 : Read without changing anything?  This is just hitting the repeat
constantly I guess as I can't immediately see another control for it.
This could also be implemented within the above as a one step sweep with
an indefinite repeat count (magic number such as -1).

Anyhow, just some initial thoughts. I'll think more on this one as
the best answer isn't obvious!

> 
> > The main focus in the short term should be around defining that ABI
> > as it may fundamentally change the structure of the driver.
> > If you want to take this on (and it'll be a big job I think!) then
> > it may be possible to source some hardware to support that effort.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> 
> As a member of the FLOSS group at Universidade de São Paulo I have the
> hardware to test the driver though I didn't figure out all the steps to
> do it yet. I intend to continue development on this driver so I'm really
> thankful for all advise given.

Great!

Jonathan

> 
> Thanks,
> 
> Marcelo
> 
> > > ---
> > >  .../staging/iio/impedance-analyzer/Kconfig|  2 +-
> > >  .../staging/iio/impedance-analyzer/ad5933.c   | 25 ---
> > >  2 files changed, 6 insertions(+), 21 deletions(-)
> > > 
> > > diff --git a/drivers/staging/iio/impedance-analyzer/Kconfig 
> > > b/drivers/staging/iio/impedance-analyzer/Kconfig
> > > index dd97b6bb3fd0..d0af5aa55dc0 100644
> > > --- a/drivers/staging/iio/impedance-analyzer/Kconfig
> > > +++ b/drivers/staging/iio/impedance-analyzer/Kconfig
> > > @@ -7,7 +7,7 @@ config AD5933
> > >   tristate "Analog Devices AD5933, AD5934 driver"
> > >   depends on I2C
> > >   select IIO_BUFFER
> > > - select IIO_KFIFO_BUF
> > > + select IIO_TRIGGERED_BUFFER
> > >   help
> > > Say yes here to build support for Analog Devices Impedance Converter,
> > > Network Analyzer, AD5933/4, provides direct access via sysfs.
> > > diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c 
> > > b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > > index f9bcb8310e21..edb8b540bbf1 100644
> > > --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> > > +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> > > @@ -20,7 +20,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > -#include 
> > > +#include 
> > >  
> > >  /* AD5933/AD5934 Registers */
> > >  #define AD5933_REG_CONTROL_HB0x80/* R/W, 1 byte */
> > > @@ -615,22 +615,6 @@ static const struct iio_buffer_setup_ops 
> > > ad5933_ring_setup_ops = {
> > >   .postdisable = ad5933_ring_postdisable,
> > >  };
> > >  
> > > -static int 

Re: [PATCH v9 04/13] media: staging/imx7: add MIPI CSI-2 receiver subdev for i.MX7

2018-12-03 Thread Sakari Ailus
Hi Rui,

On Thu, Nov 22, 2018 at 03:18:25PM +, Rui Miguel Silva wrote:
> Adds MIPI CSI-2 subdev for i.MX7 to connect with sensors with a MIPI
> CSI-2 interface.
> 
> Signed-off-by: Rui Miguel Silva 
> ---
>  drivers/staging/media/imx/Makefile |1 +
>  drivers/staging/media/imx/imx7-mipi-csis.c | 1135 
>  2 files changed, 1136 insertions(+)
>  create mode 100644 drivers/staging/media/imx/imx7-mipi-csis.c
> 
> diff --git a/drivers/staging/media/imx/Makefile 
> b/drivers/staging/media/imx/Makefile
> index 074f016d3519..d2d909a36239 100644
> --- a/drivers/staging/media/imx/Makefile
> +++ b/drivers/staging/media/imx/Makefile
> @@ -14,3 +14,4 @@ obj-$(CONFIG_VIDEO_IMX_CSI) += imx-media-csi.o
>  obj-$(CONFIG_VIDEO_IMX_CSI) += imx6-mipi-csi2.o
>  
>  obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
> +obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-mipi-csis.o
> diff --git a/drivers/staging/media/imx/imx7-mipi-csis.c 
> b/drivers/staging/media/imx/imx7-mipi-csis.c
> new file mode 100644
> index ..56963d0c2043
> --- /dev/null
> +++ b/drivers/staging/media/imx/imx7-mipi-csis.c
> @@ -0,0 +1,1135 @@
> +// SPDX-License-Identifier: GPL
> +/*
> + * Freescale i.MX7 SoC series MIPI-CSI V3.3 receiver driver
> + *
> + * Copyright (C) 2018 Linaro Ltd
> + * Copyright (C) 2015-2016 Freescale Semiconductor, Inc. All Rights Reserved.
> + * Copyright (C) 2011 - 2013 Samsung Electronics Co., Ltd.
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "imx-media.h"
> +
> +static int debug;
> +module_param(debug, int, 0644);
> +MODULE_PARM_DESC(debug, "Debug level (0-2)");

Could you rely on dynamic debug instead?

> +
> +#define CSIS_DRIVER_NAME "imx7-mipi-csis"
> +#define CSIS_SUBDEV_NAME CSIS_DRIVER_NAME
> +
> +#define CSIS_PAD_SINK0
> +#define CSIS_PAD_SOURCE  1
> +#define CSIS_PADS_NUM2
> +
> +#define MIPI_CSIS_DEF_PIX_WIDTH  640
> +#define MIPI_CSIS_DEF_PIX_HEIGHT 480
> +
> +/* Register map definition */
> +
> +/* CSIS common control */
> +#define MIPI_CSIS_CMN_CTRL   0x04
> +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW BIT(16)
> +#define MIPI_CSIS_CMN_CTRL_INTER_MODEBIT(10)
> +#define MIPI_CSIS_CMN_CTRL_UPDATE_SHADOW_CTRLBIT(2)
> +#define MIPI_CSIS_CMN_CTRL_RESET BIT(1)
> +#define MIPI_CSIS_CMN_CTRL_ENABLEBIT(0)
> +
> +#define MIPI_CSIS_CMN_CTRL_LANE_NR_OFFSET8
> +#define MIPI_CSIS_CMN_CTRL_LANE_NR_MASK  (3 << 8)
> +
> +/* CSIS clock control */
> +#define MIPI_CSIS_CLK_CTRL   0x08
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH3(x)  ((x) << 28)
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH2(x)  ((x) << 24)
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH1(x)  ((x) << 20)
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_TRAIL_CH0(x)  ((x) << 16)
> +#define MIPI_CSIS_CLK_CTRL_CLKGATE_EN_MSK(0xf << 4)
> +#define MIPI_CSIS_CLK_CTRL_WCLK_SRC  BIT(0)
> +
> +/* CSIS Interrupt mask */
> +#define MIPI_CSIS_INTMSK 0x10
> +#define MIPI_CSIS_INTMSK_EVEN_BEFORE BIT(31)
> +#define MIPI_CSIS_INTMSK_EVEN_AFTER  BIT(30)
> +#define MIPI_CSIS_INTMSK_ODD_BEFORE  BIT(29)
> +#define MIPI_CSIS_INTMSK_ODD_AFTER   BIT(28)
> +#define MIPI_CSIS_INTMSK_FRAME_START BIT(24)
> +#define MIPI_CSIS_INTMSK_FRAME_END   BIT(20)
> +#define MIPI_CSIS_INTMSK_ERR_SOT_HS  BIT(16)
> +#define MIPI_CSIS_INTMSK_ERR_LOST_FS BIT(12)
> +#define MIPI_CSIS_INTMSK_ERR_LOST_FE BIT(8)
> +#define MIPI_CSIS_INTMSK_ERR_OVERBIT(4)
> +#define MIPI_CSIS_INTMSK_ERR_WRONG_CFG   BIT(3)
> +#define MIPI_CSIS_INTMSK_ERR_ECC BIT(2)
> +#define MIPI_CSIS_INTMSK_ERR_CRC BIT(1)
> +#define MIPI_CSIS_INTMSK_ERR_UNKNOWN BIT(0)
> +
> +/* CSIS Interrupt source */
> +#define MIPI_CSIS_INTSRC 0x14
> +#define MIPI_CSIS_INTSRC_EVEN_BEFORE BIT(31)
> +#define MIPI_CSIS_INTSRC_EVEN_AFTER  BIT(30)
> +#define MIPI_CSIS_INTSRC_EVENBIT(30)
> +#define MIPI_CSIS_INTSRC_ODD_BEFORE  BIT(29)
> +#define MIPI_CSIS_INTSRC_ODD_AFTER   BIT(28)
> +#define MIPI_CSIS_INTSRC_ODD (0x3 << 28)
> +#define MIPI_CSIS_INTSRC_NON_IMAGE_DATA  (0xf << 28)
> +#define MIPI_CSIS_INTSRC_FRAME_START BIT(24)
> +#define MIPI_CSIS_INTSRC_FRAME_END   BIT(20)
> +#define MIPI_CSIS_INTSRC_ERR_SOT_HS  BIT(16)
> +#define MIPI_CSIS_INTSRC_ERR_LOST_FS BIT(12)
> +#define MIPI_CSIS_INTSRC_ERR_LOST_FE BIT(8)
> +#define MIPI_CSIS_INTSRC_ERR_OVERBIT(4)
> +#define MIPI_CSIS_INTSRC_ERR_WRONG_CFG   BIT(3)
> +#define MIPI_CSIS_INTSRC_ERR_ECC BIT(2)
> +#define MIPI_CSIS_INTSRC_ERR_CRC BIT(1)
> +#define MIPI_CSIS_INTSRC_ERR_UNKNOWN BIT(0)
> +#define MIPI_CSIS_INTSRC_ERRORS  0xf
> +
> +/* D-PHY status control */
> +#define 

Re: [PATCH] [repost] Drivers: hv: vmbus: Offload the handling of channels to two workqueues

2018-12-03 Thread Dan Carpenter
The list is still rejecting @microsoft.com patches...  :(

I mentioned this last time when you guys were complaining that no one
reads your patches and someone sent me a link to marc.info.  I want to
help but obviously no one has time to look at patches on marc.info...

regards,
dan carpenter

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


[PATCH] staging: mt7621-spi: drop support for SPI mode 1/2/3

2018-12-03 Thread Chuanhong Guo
This SPI controller seems to be tested on SPI flash only before mass
production and some bits are swizzled under other SPI modes probably
due to incorrect wiring inside the silicon. Reject all modes except
mode0 because they are broken.

Signed-off-by: Chuanhong Guo 
---
 drivers/staging/mt7621-spi/spi-mt7621.c | 23 ++-
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c 
b/drivers/staging/mt7621-spi/spi-mt7621.c
index 8af6f307e176..fb189e3ce3f2 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -137,19 +137,16 @@ static int mt7621_spi_prepare(struct spi_device *spi, 
unsigned int speed)
reg |= MT7621_LSB_FIRST;
 
reg &= ~(MT7621_CPHA | MT7621_CPOL);
-   switch (spi->mode & (SPI_CPOL | SPI_CPHA)) {
-   case SPI_MODE_0:
-   break;
-   case SPI_MODE_1:
-   reg |= MT7621_CPHA;
-   break;
-   case SPI_MODE_2:
-   reg |= MT7621_CPOL;
-   break;
-   case SPI_MODE_3:
-   reg |= MT7621_CPOL | MT7621_CPHA;
-   break;
-   }
+
+   /* This SPI controller is designed for SPI flash only and
+* some bits are swizzled under other SPI modes due to
+* incorrect wiring inside the silicon. Reject all modes
+* except mode0 because they are broken.
+*/
+
+   if ((spi->mode & (SPI_CPOL | SPI_CPHA)) != SPI_MODE_0)
+   return -EINVAL;
+
mt7621_spi_write(rs, MT7621_SPI_MASTER, reg);
 
return 0;
-- 
2.19.1

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


[PATCH] staging: mt7621-spi: drop the broken full-duplex mode

2018-12-03 Thread Chuanhong Guo
Under MORE_BUF_MODE the controller will always shift one bit out of
spi_opcode if (mosi_bit_cnt > 0) && (cmd_bit_cnt == 0) so the full-
duplex mode is broken since we can't read anything from MISO during
writing spi_opcode.
This piece of code also make CS1 unavailable since it forces the
broken full-duplex mode to be enabled on CS1.

Signed-off-by: Chuanhong Guo 
---
 drivers/staging/mt7621-spi/spi-mt7621.c | 120 +++-
 1 file changed, 15 insertions(+), 105 deletions(-)

diff --git a/drivers/staging/mt7621-spi/spi-mt7621.c 
b/drivers/staging/mt7621-spi/spi-mt7621.c
index d045b5568e0f..8af6f307e176 100644
--- a/drivers/staging/mt7621-spi/spi-mt7621.c
+++ b/drivers/staging/mt7621-spi/spi-mt7621.c
@@ -86,16 +86,13 @@ static inline void mt7621_spi_write(struct mt7621_spi *rs, 
u32 reg, u32 val)
iowrite32(val, rs->base + reg);
 }
 
-static void mt7621_spi_reset(struct mt7621_spi *rs, int duplex)
+static void mt7621_spi_reset(struct mt7621_spi *rs)
 {
u32 master = mt7621_spi_read(rs, MT7621_SPI_MASTER);
 
master |= 7 << 29;
master |= 1 << 2;
-   if (duplex)
-   master |= 1 << 10;
-   else
-   master &= ~(1 << 10);
+   master &= ~(1 << 10);
 
mt7621_spi_write(rs, MT7621_SPI_MASTER, master);
rs->pending_write = 0;
@@ -107,7 +104,7 @@ static void mt7621_spi_set_cs(struct spi_device *spi, int 
enable)
int cs = spi->chip_select;
u32 polar = 0;
 
-   mt7621_spi_reset(rs, cs);
+   mt7621_spi_reset(rs);
if (enable)
polar = BIT(cs);
mt7621_spi_write(rs, MT7621_SPI_POLAR, polar);
@@ -261,7 +258,7 @@ static void mt7621_spi_write_half_duplex(struct mt7621_spi 
*rs,
rs->pending_write = len;
 }
 
-static int mt7621_spi_transfer_half_duplex(struct spi_master *master,
+static int mt7621_spi_transfer_one_message(struct spi_master *master,
   struct spi_message *m)
 {
struct mt7621_spi *rs = spi_master_get_devdata(master);
@@ -284,7 +281,16 @@ static int mt7621_spi_transfer_half_duplex(struct 
spi_master *master,
mt7621_spi_set_cs(spi, 1);
m->actual_length = 0;
list_for_each_entry(t, >transfers, transfer_list) {
-   if (t->rx_buf)
+   if((t->rx_buf) && (t->tx_buf)) {
+   /* This controller will shift one extra bit out
+* of spi_opcode if (mosi_bit_cnt > 0) &&
+* (cmd_bit_cnt == 0). So the claimed full-duplex
+* support is broken since we have no way to read
+* the MISO value during that bit.
+*/
+   status = -EIO;
+   goto msg_done;
+   } else if (t->rx_buf)
mt7621_spi_read_half_duplex(rs, t->len, t->rx_buf);
else if (t->tx_buf)
mt7621_spi_write_half_duplex(rs, t->len, t->tx_buf);
@@ -300,102 +306,6 @@ static int mt7621_spi_transfer_half_duplex(struct 
spi_master *master,
return 0;
 }
 
-static int mt7621_spi_transfer_full_duplex(struct spi_master *master,
-  struct spi_message *m)
-{
-   struct mt7621_spi *rs = spi_master_get_devdata(master);
-   struct spi_device *spi = m->spi;
-   unsigned int speed = spi->max_speed_hz;
-   struct spi_transfer *t = NULL;
-   int status = 0;
-   int i, len = 0;
-   int rx_len = 0;
-   u32 data[9] = { 0 };
-   u32 val = 0;
-
-   mt7621_spi_wait_till_ready(rs);
-
-   list_for_each_entry(t, >transfers, transfer_list) {
-   const u8 *buf = t->tx_buf;
-
-   if (t->rx_buf)
-   rx_len += t->len;
-
-   if (!buf)
-   continue;
-
-   if (WARN_ON(len + t->len > 16)) {
-   status = -EIO;
-   goto msg_done;
-   }
-
-   for (i = 0; i < t->len; i++, len++)
-   data[len / 4] |= buf[i] << (8 * (len & 3));
-   if (speed > t->speed_hz)
-   speed = t->speed_hz;
-   }
-
-   if (WARN_ON(rx_len > 16)) {
-   status = -EIO;
-   goto msg_done;
-   }
-
-   if (mt7621_spi_prepare(spi, speed)) {
-   status = -EIO;
-   goto msg_done;
-   }
-
-   for (i = 0; i < len; i += 4)
-   mt7621_spi_write(rs, MT7621_SPI_DATA0 + i, data[i / 4]);
-
-   val |= len * 8;
-   val |= (rx_len * 8) << 12;
-   mt7621_spi_write(rs, MT7621_SPI_MOREBUF, val);
-
-   mt7621_spi_set_cs(spi, 1);
-
-   val = mt7621_spi_read(rs, MT7621_SPI_TRANS);
-   val |= SPI_CTL_START;
-   mt7621_spi_write(rs, MT7621_SPI_TRANS, val);
-
-   mt7621_spi_wait_till_ready(rs);
-
-   mt7621_spi_set_cs(spi, 0);
-
-   for (i = 0; i < rx_len; i += 4)
-   

Re: [PATCH RFCv2 2/4] mm/memory_hotplug: Replace "bool want_memblock" by "int type"

2018-12-03 Thread David Hildenbrand
On 01.12.18 02:50, Wei Yang wrote:
> On Fri, Nov 30, 2018 at 06:59:20PM +0100, David Hildenbrand wrote:
>> Let's pass a memory block type instead. Pass "MEMORY_BLOCK_NONE" for device
>> memory and for now "MEMORY_BLOCK_UNSPECIFIED" for anything else. No
>> functional change.
> 
> I would suggest to put more words to this.

Sure, makes sense, I'll add more details. Thanks!

> 
> "
> Function arch_add_memory()'s last parameter *want_memblock* is used to
> determin whether it is necessary to create a corresponding memory block
> device. After introducing the memory block type, this patch replaces the
> bool type *want_memblock* with memory block type with following rules
> for now:
> 
>   * Pass "MEMORY_BLOCK_NONE" for device memory
>   * Pass "MEMORY_BLOCK_UNSPECIFIED" for anything else 
> 
> Since this parameter is passed deep to __add_section(), all its
> descendents are effected. Below lists those descendents.
> 
>   arch_add_memory()
> add_pages()
>   __add_pages()
> __add_section()
> 
> "

[...]


-- 

Thanks,

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


Re: [PATCH RFCv2 1/4] mm/memory_hotplug: Introduce memory block types

2018-12-03 Thread David Hildenbrand
On 01.12.18 02:25, Wei Yang wrote:
> On Fri, Nov 30, 2018 at 06:59:19PM +0100, David Hildenbrand wrote:
>> Memory onlining should always be handled by user space, because only user
>> space knows which use cases it wants to satisfy. E.g. memory might be
>> onlined to the MOVABLE zone even if it can never be removed from the
>> system, e.g. to make usage of huge pages more reliable.
>>
>> However to implement such rules (especially default rules in distributions)
>> we need more information about the memory that was added in user space.
>>
>> E.g. on x86 we want to online memory provided by balloon devices (e.g.
>> XEN, Hyper-V) differently (-> will not be unplugged by offlining the whole
>> block) than ordinary DIMMs (-> might eventually be unplugged by offlining
>> the whole block). This might also become relevat for other architectures.
>>
>> Also, udev rules right now check if running on s390x and treat all added
>> memory blocks as standby memory (-> don't online automatically). As soon as
>> we support other memory hotplug mechanism (e.g. virtio-mem) checks would
>> have to get more involved (e.g. also check if under KVM) but eventually
>> also wrong (e.g. if KVM ever supports standby memory we are doomed).
>>
>> I decided to allow to specify the type of memory that is getting added
>> to the system. Let's start with two types, BOOT and UNSPECIFIED to get the
>> basic infrastructure running. We'll introduce and use further types in
>> follow-up patches. For now we classify any hotplugged memory temporarily
>> as as UNSPECIFIED (which will eventually be dropped later on).
>>
>> Cc: Greg Kroah-Hartman 
>> Cc: "Rafael J. Wysocki" 
>> Cc: Andrew Morton 
>> Cc: Ingo Molnar 
>> Cc: Pavel Tatashin 
>> Cc: Stephen Rothwell 
>> Cc: Andrew Banman 
>> Cc: "mike.tra...@hpe.com" 
>> Cc: Oscar Salvador 
>> Cc: Dave Hansen 
>> Cc: Michal Hocko 
>> Cc: Michal Such??nek 
>> Cc: Vitaly Kuznetsov 
>> Cc: Dan Williams 
>> Cc: Pavel Tatashin 
>> Cc: Martin Schwidefsky 
>> Cc: Heiko Carstens 
>> Signed-off-by: David Hildenbrand 
>> ---
>> drivers/base/memory.c  | 38 +++---
>> include/linux/memory.h | 27 +++
>> 2 files changed, 62 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/base/memory.c b/drivers/base/memory.c
>> index 0c290f86ab20..17f2985c07c5 100644
>> --- a/drivers/base/memory.c
>> +++ b/drivers/base/memory.c
>> @@ -381,6 +381,29 @@ static ssize_t show_phys_device(struct device *dev,
>>  return sprintf(buf, "%d\n", mem->phys_device);
>> }
>>
>> +static ssize_t type_show(struct device *dev, struct device_attribute *attr,
>> + char *buf)
>> +{
>> +struct memory_block *mem = to_memory_block(dev);
>> +ssize_t len = 0;
>> +
>> +switch (mem->type) {
>> +case MEMORY_BLOCK_UNSPECIFIED:
>> +len = sprintf(buf, "unspecified\n");
>> +break;
>> +case MEMORY_BLOCK_BOOT:
>> +len = sprintf(buf, "boot\n");
>> +break;
>> +default:
>> +len = sprintf(buf, "ERROR-UNKNOWN-%ld\n",
>> +mem->state);
>> +WARN_ON(1);
>> +break;
>> +}
>> +
>> +return len;
>> +}
>> +
>> #ifdef CONFIG_MEMORY_HOTREMOVE
>> static void print_allowed_zone(char *buf, int nid, unsigned long start_pfn,
>>  unsigned long nr_pages, int online_type,
>> @@ -442,6 +465,7 @@ static DEVICE_ATTR(phys_index, 0444, 
>> show_mem_start_phys_index, NULL);
>> static DEVICE_ATTR(state, 0644, show_mem_state, store_mem_state);
>> static DEVICE_ATTR(phys_device, 0444, show_phys_device, NULL);
>> static DEVICE_ATTR(removable, 0444, show_mem_removable, NULL);
>> +static DEVICE_ATTR_RO(type);
> 
> This is correct, while looks not consistent with other attributes.
> 
> Not that beautiful :-)

I might change the other ones first, too (or keep this one consistent to
the existing ones). Thanks!

> 
>>
>> /*
>>  * Block size attribute stuff
>> @@ -620,6 +644,7 @@ static struct attribute *memory_memblk_attrs[] = {
>>  _attr_state.attr,
>>  _attr_phys_device.attr,
>>  _attr_removable.attr,
>> +_attr_type.attr,
>> #ifdef CONFIG_MEMORY_HOTREMOVE
>>  _attr_valid_zones.attr,
>> #endif
>> @@ -657,13 +682,17 @@ int register_memory(struct memory_block *memory)
>> }
>>
>> static int init_memory_block(struct memory_block **memory,
>> - struct mem_section *section, unsigned long state)
>> + struct mem_section *section, unsigned long state,
>> + int type)
>> {
>>  struct memory_block *mem;
>>  unsigned long start_pfn;
>>  int scn_nr;
>>  int ret = 0;
>>
>> +if (type == MEMORY_BLOCK_NONE)
>> +return -EINVAL;
> 
> No one will pass in this value. Can we omit this check for now?

I could move it to patch nr 2 I guess, but as I introduce
MEMORY_BLOCK_NONE here it made sense to keep it in here.

(and I think at least for now it makes sense to not