[PATCH] staging: media: davinci_vpfe: add error handling on kmalloc failure

[Ji-Hun Kim]

There is no failure checking on the param value which will be allocated
memory by kmalloc. Add a null pointer checking statement. Then goto error:
and return -ENOMEM error code when kmalloc is failed.

Signed-off-by: Ji-Hun Kim 
---
 drivers/staging/media/davinci_vpfe/dm365_ipipe.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c 
b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
index 6a3434c..55a922c 100644
--- a/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
+++ b/drivers/staging/media/davinci_vpfe/dm365_ipipe.c
@@ -1280,6 +1280,10 @@ static int ipipe_s_config(struct v4l2_subdev *sd, struct 
vpfe_ipipe_config *cfg)
 
params = kmalloc(sizeof(struct ipipe_module_params),
 GFP_KERNEL);
+   if (!params) {
+   rval = -ENOMEM;
+   goto error;
+   }
to = (void *)params + module_if->param_offset;
size = module_if->param_size;
 
@@ -1323,6 +1327,10 @@ static int ipipe_g_config(struct v4l2_subdev *sd, struct 
vpfe_ipipe_config *cfg)
 
params =  kmalloc(sizeof(struct ipipe_module_params),
GFP_KERNEL);
+   if (!params) {
+   rval = -ENOMEM;
+   goto error;
+   }
from = (void *)params + module_if->param_offset;
size = module_if->param_size;
 
-- 
1.9.1

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

Re: [PATCH] storvsc: Set up correct queue depth values for IDE devices

[Stephen Hemminger]

On Thu, 15 Mar 2018 16:52:23 -0700
Long Li  wrote:

> From: Long Li 
> 
> Unlike SCSI and FC, we don't use multiple channels for IDE. So set queue depth
> correctly for IDE.
> 
> Also set the correct cmd_per_lun for all devices.
> 
> Signed-off-by: Long Li 

Looks correct.
Acked-by: Stephen Hemminger 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 7/7] staging:iio:ade7854: Add proper error handling condition

[Rodrigo Siqueira]

On 03/15, Dan Carpenter wrote:
> On Wed, Mar 14, 2018 at 03:12:18PM -0300, Rodrigo Siqueira wrote:
> > There is some improper error handling for IRQ and device register.  This
> > patch adds a proper verification. The IRQ correction was extracted from
> > John Syne patches.
> > 
> > Signed-off-by: Rodrigo Siqueira 
> > Signed-off-by: John Syne 
> > ---
> >  drivers/staging/iio/meter/ade7854.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/meter/ade7854.c 
> > b/drivers/staging/iio/meter/ade7854.c
> > index 09fd8c067738..49cbe365e43d 100644
> > --- a/drivers/staging/iio/meter/ade7854.c
> > +++ b/drivers/staging/iio/meter/ade7854.c
> > @@ -436,7 +436,7 @@ static int ade7854_initial_setup(struct iio_dev 
> > *indio_dev)
> >  
> > /* Disable IRQ */
> > ret = ade7854_set_irq(dev, false);
> > -   if (ret) {
> > +   if (ret < 0) {
> > dev_err(dev, "disable irq failed");
> > goto err_ret;
> > }
> 
> Why is the original wrong?  It seems fine.

Hi,

If you look at "drivers/staging/iio/meter/ade7854-(i2c|spi).c", you will
find a line like this: " st->write_reg = ade7854_(i2c|spi)_write_reg;".
Than, if you go through the "ade7854_i2c_write_reg" (focus only on i2c
for while) you will see "i2c_master_send(st->i2c, st->tx, count)",
which can returns a positive number that does not necessary means an
error. If you find the same for the spi case, you will end up in the
function "spi_sync_transfer" that only return 0 for success or negative
for failure. So, handling the negative case better fit the i2c and spi case.

Thanks

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

Re: [PATCH 1/7] staging:iio:ade7854: Rework I2C write function

[Rodrigo Siqueira]

Hi,

I will fixes all of these things here and in the other patches and send a
v2.

Thanks for the review.

On 03/15, Dan Carpenter wrote:
> On Wed, Mar 14, 2018 at 03:10:18PM -0300, Rodrigo Siqueira wrote:
> > The write operation using I2C has many code duplications and four
> > different interfaces per data size. This patch introduces a single
> > function that centralizes the main tasks.
> > 
> > The central function inserted by this patch can easily replace all the
> > four functions related to the data size. However, this patch does not
> > remove any code signature for keeping the meter module work and make
> > easier to review this patch.
> > 
> > Signed-off-by: Rodrigo Siqueira 
> > ---
> >  drivers/staging/iio/meter/ade7854-i2c.c | 89 
> > +++--
> >  drivers/staging/iio/meter/ade7854.h |  7 +++
> >  2 files changed, 58 insertions(+), 38 deletions(-)
> > 
> > diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> > b/drivers/staging/iio/meter/ade7854-i2c.c
> > index 317e4f0d8176..03133a05eae4 100644
> > --- a/drivers/staging/iio/meter/ade7854-i2c.c
> > +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> > @@ -15,41 +15,74 @@
> >  #include 
> >  #include "ade7854.h"
> >  
> > -static int ade7854_i2c_write_reg_8(struct device *dev,
> > -  u16 reg_address,
> > -  u8 val)
> > +static int ade7854_i2c_write_reg(struct device *dev,
> > +u16 reg_address,
> > +u32 val,
> > +enum data_size type)
> 
> 
> The data size should just be the number of bytes and not an enum.
> 1 means 1 byte / 8 bits.
> 2 means 2 bytes / 16 bits.
> 3 means 3 bytes / 24 bits.
> etc.
> 
> >  {
> > int ret;
> > +   int count;
> > struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > struct ade7854_state *st = iio_priv(indio_dev);
> >  
> > mutex_lock(>buf_lock);
> > st->tx[0] = (reg_address >> 8) & 0xFF;
> > st->tx[1] = reg_address & 0xFF;
> > -   st->tx[2] = val;
> >  
> > -   ret = i2c_master_send(st->i2c, st->tx, 3);
> > +   switch (type) {
> > +   case DATA_SIZE_8_BITS:
> > +   st->tx[2] = val & 0xFF;
> > +   count = 3;
> > +   break;
> > +   case DATA_SIZE_16_BITS:
> > +   st->tx[2] = (val >> 8) & 0xFF;
> > +   st->tx[3] = val & 0xFF;
> > +   count = 4;
> > +   break;
> > +   case DATA_SIZE_24_BITS:
> > +   st->tx[2] = (val >> 16) & 0xFF;
> > +   st->tx[3] = (val >> 8) & 0xFF;
> > +   st->tx[4] = val & 0xFF;
> > +   count = 5;
> > +   break;
> > +   case DATA_SIZE_32_BITS:
> > +   st->tx[2] = (val >> 24) & 0xFF;
> > +   st->tx[3] = (val >> 16) & 0xFF;
> > +   st->tx[4] = (val >> 8) & 0xFF;
> > +   st->tx[5] = val & 0xFF;
> > +   count = 6;
> > +   break;
> > +   default:
> > +   ret = -EINVAL;
> > +   goto error_i2c_write_unlock;
> > +   }
> > +
> > +   ret = i2c_master_send(st->i2c, st->tx, count);
> > +
> > +error_i2c_write_unlock:
> 
> These labels are sort of long.  And what does the "i2c_write" really
> mean?  It should be obvious that we're not jumping to a different
> function.
> 
> Just "unlock:" is OK as a label name.
> 
> > mutex_unlock(>buf_lock);
> >  
> > return ret;
> >  }
> >  
> > +static int ade7854_i2c_write_reg_8(struct device *dev,
> > +  u16 reg_address,
> > +  u8 val)
> > +{
> > +   int ret;
> > +
> > +   ret = ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS);
> > +
> > +   return ret;
> > +}
> 
> Just do it like this:
> 
> static int ade7854_i2c_write_reg_8(struct device *dev, u16 reg_address, u8 
> val)
> {
>   return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS);
> }
> 
> 
> 
> > +
> >  static int ade7854_i2c_write_reg_16(struct device *dev,
> > u16 reg_address,
> > u16 val)
> >  {
> > int ret;
> > -   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > -   struct ade7854_state *st = iio_priv(indio_dev);
> >  
> > -   mutex_lock(>buf_lock);
> > -   st->tx[0] = (reg_address >> 8) & 0xFF;
> > -   st->tx[1] = reg_address & 0xFF;
> > -   st->tx[2] = (val >> 8) & 0xFF;
> > -   st->tx[3] = val & 0xFF;
> > -
> > -   ret = i2c_master_send(st->i2c, st->tx, 4);
> > -   mutex_unlock(>buf_lock);
> > +   ret = ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS);
> >  
> > return ret;
> 
> Again:
> 
>   return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS);
> 
> 
> 
> >  }
> > @@ -59,18 +92,8 @@ static int ade7854_i2c_write_reg_24(struct device *dev,
> > u32 val)
> >  {
> > int ret;
> > -   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> > -   struct ade7854_state *st = iio_priv(indio_dev);
> >  
> > -  

Re: [PATCH v6 0/6] staging: Introduce DPAA2 Ethernet Switch driver

[Andrew Lunn]

On Thu, Mar 15, 2018 at 01:56:42PM +0300, Dan Carpenter wrote:
> On Thu, Mar 15, 2018 at 12:44:37AM +0100, Andrew Lunn wrote:
> > On Wed, Mar 14, 2018 at 10:55:52AM -0500, Razvan Stefanescu wrote:
> > > This patchset introduces the Ethernet Switch Driver for Freescale/NXP SoCs
> > > with DPAA2 (DataPath Acceleration Architecture v2). The driver manages
> > > switch objects discovered on the fsl-mc bus. A description of the driver
> > > can be found in the associated README file.
> > 
> > Hi Greg
> > 
> > This code has much better quality than the usual stuff in staging. I
> > see no reason not to merge it. 
> 
> Yeah.  It seems pretty decent.  Stuart, Laurentiu, care to comment?
> 
> Meanwhile, netdev and DaveM aren't even on the CC list and they're the
> ones to ultimately decide.

The patches are for staging, so it is GregKH who decides at this
point, not really DaveM.

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

[PATCH] storvsc: Set up correct queue depth values for IDE devices

[Long Li]

From: Long Li 

Unlike SCSI and FC, we don't use multiple channels for IDE. So set queue depth
correctly for IDE.

Also set the correct cmd_per_lun for all devices.

Signed-off-by: Long Li 
---
 drivers/scsi/storvsc_drv.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c
index 8c51d628b52e..fba170640e9c 100644
--- a/drivers/scsi/storvsc_drv.c
+++ b/drivers/scsi/storvsc_drv.c
@@ -1722,15 +1722,19 @@ static int storvsc_probe(struct hv_device *device,
max_targets = STORVSC_MAX_TARGETS;
max_channels = STORVSC_MAX_CHANNELS;
/*
-* On Windows8 and above, we support sub-channels for storage.
+* On Windows8 and above, we support sub-channels for storage
+* on SCSI and FC controllers.
 * The number of sub-channels offerred is based on the number of
 * VCPUs in the guest.
 */
-   max_sub_channels = (num_cpus / storvsc_vcpus_per_sub_channel);
+   if (!dev_is_ide)
+   max_sub_channels =
+   num_cpus / storvsc_vcpus_per_sub_channel;
}
 
scsi_driver.can_queue = (max_outstanding_req_per_channel *
 (max_sub_channels + 1));
+   scsi_driver.cmd_per_lun = scsi_driver.can_queue;
 
host = scsi_host_alloc(_driver,
   sizeof(struct hv_host_device));
-- 
2.14.1

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

Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1

[NeilBrown]

On Thu, Mar 15 2018, John Crispin wrote:

> On 15/03/18 21:12, NeilBrown wrote:
>> On Thu, Mar 15 2018, John Crispin wrote:
>>
>>> On 15/03/18 11:48, Dan Carpenter wrote:
 This all seems fine.  Generally the requirements for staging are that it
 has a TODO, someone to work on it, and it doesn't break the build.  But
 some of the patches don't have commit message and those are required and
 some of the commit messages are just the changes you have made not don't
 describe the actual code...

 John Crispin's email is j...@phrozen.org.

 regards,
 dan carpenter

>>> Hi All,
>>>
>>> looks like i was CC'ed on the openwrt addr, which no longer exists. This
>>> series makes no sense. None of the stuff posted is anywhere near ready
>>> to be upstreamed.
>>>
>>> * we dont need a dedicated pinctrl driver, pinctrl-single will work fine
>>> on these SoCs
>>> * the DMA/sdhci driver is a hacked up version of the SDK driver.
>>> * drivers/net/ethernet/mediatek/* works on mt7623 and is easily portable
>>> to mt7621, same goes for the gsw driver.
>> Hi John,
>>   I think it makes sense in that, with the patches, the hardware works, and
>>   without the patches (at least the first) you cannot even build with
>>   CONFIG_SOC_MT7621=y as pcibios_map_irq() is undefined.  Having
>>   working code is a great starting point for further development.
>>   It certainly isn't ready for upstream, which is why it is heading for
>>   drivers/staging.  This is explicitly for code that isn't yet ready.
>>   By putting the code there it should be safe from bit-rot, and can be
>>   worked on by multiple people.  It gets increased visibility so people
>>   can say how bad it is (as you have done - thanks).  This feed back is a
>>   valuable part of improving the code and getting it out of staging.
>>
>>   I'll add notes to various TODO files based on your comments.  If you
>>   have anything else to add, it would be most welcome.  Thank you for
>>   making these patches available in the first place, so that my hardware
>>   can work!
>>
>> Thanks,
>> NeilBrown
>
> Hi Neil,
>
> I understand your reasoning, however ...
> only the pcie driver is worth merging. all other drivers are already 
> inside the kernel for mt7623 and can be easily adapted to work on 
> mt7621. having duplicate drivers is a certain no-go.
> cleaning up the pci driver is a matter of a few days work. merging a 
> shitty pci driver just to postpone doing the 3-5 days work involved to 
> polish it seems a rally bad trade-off.
> i strongly oppose having any of this code merged into the kernel, even 
> if it is only the staging area.

Hi John,
 I don't understand why you would want to deny people easy access to code
 which makes their hardware work.  I'm sure that isn't your intention,
 but it could well be the effect.  If you think you can make it work in
 3-5 days, then please go right ahead.  I will happily test anything you
 submit.  I suspect the most likely outcome, however, is that I'll end
 up doing all the work, and I'm sure it will take a lot more than 3-5
 days and will involve a lot of learning.  I'll be happy if I can get
 all of this back out of staging in 6 months. That is an extra 6 months
 (at least) that people will be able to use a mainline kernel on their
 gnubee.

 With respect to your suggestion that "having duplicate drivers is a
 certain no-go", I don't think that is correct.  As an approximate
 counter-point, I'm in the process of cleaning up the
 drivers/staging/lustre filesystem with the hope of eventually moving it
 out of staging.  It had duplicate wait_event() code, duplicate PRNG,
 duplicate workqueues, duplicate resizeable hashtable, duplicate tracing
 infrastructure, and more that I haven't had a chance to look closely at
 yet.  This duplication certainly kept it out of linux/fs/, but has no
 bearing on whether it belong in linux/drivers/staging/.

Thanks,
NeilBrown


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

[PATCH] staging: lustre: o2iblnd: Enable Multiple OPA Endpoints between Nodes

[Doug Oucharek]

OPA driver optimizations are based on the MPI model where it is
expected to have multiple endpoints between two given nodes. To
enable this optimization for Lustre, we need to make it possible,
via an LND-specific tuneable, to create multiple endpoints and to
balance the traffic over them.

Both sides of a connection must have this patch for it to work.
Only the active side of the connection (usually the client)
needs to have the new tuneable set > 1.

Signed-off-by: Doug Oucharek 
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-8943
Reviewed-on: https://review.whamcloud.com/25168
Reviewed-by: Amir Shehata 
Reviewed-by: Dmitry Eremin 
Reviewed-by: James Simmons 
Reviewed-by: Oleg Drokin 
Signed-off-by: Doug Oucharek 
---
.../lustre/include/uapi/linux/lnet/lnet-dlc.h  |  3 ++-
.../staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h| 17 ---
.../staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 25 +++---
.../lustre/lnet/klnds/o2iblnd/o2iblnd_modparams.c  |  9 
4 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/lustre/include/uapi/linux/lnet/lnet-dlc.h 
b/drivers/staging/lustre/include/uapi/linux/lnet/lnet-dlc.h
index e45d828..c1619f4 100644
--- a/drivers/staging/lustre/include/uapi/linux/lnet/lnet-dlc.h
+++ b/drivers/staging/lustre/include/uapi/linux/lnet/lnet-dlc.h
@@ -53,7 +53,8 @@ struct lnet_ioctl_config_o2iblnd_tunables {
__u32 lnd_fmr_pool_size;
__u32 lnd_fmr_flush_trigger;
__u32 lnd_fmr_cache;
-   __u32 pad;
+   __u16 lnd_conns_per_peer;
+   __u16 pad;
};

struct lnet_ioctl_config_lnd_tunables {
diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h 
b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
index b18911d..af5f573 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.h
@@ -568,6 +568,8 @@ struct kib_peer {
lnet_nid_t   ibp_nid; /* who's on the other end(s) */
struct lnet_ni  *ibp_ni; /* LNet interface */
struct list_head ibp_conns;   /* all active connections */
+   struct kib_conn *ibp_next_conn;  /* next connection to send on for
+   round robin */
struct list_head ibp_tx_queue;/* msgs waiting for a conn */
__u64ibp_incarnation; /* incarnation of peer */
/* when (in jiffies) I was last alive */
@@ -581,7 +583,7 @@ struct kib_peer {
/* current active connection attempts */
unsigned short  ibp_connecting;
/* reconnect this peer later */
-   unsigned short  ibp_reconnecting:1;
+   unsigned char   ibp_reconnecting;
/* counter of how many times we triggered a conn race */
unsigned char   ibp_races;
/* # consecutive reconnection attempts to this peer */
@@ -744,10 +746,19 @@ struct kib_peer {
static inline struct kib_conn *
kiblnd_get_conn_locked(struct kib_peer *peer)
{
+   struct list_head *next;
+
LASSERT(!list_empty(>ibp_conns));

-   /* just return the first connection */
-   return list_entry(peer->ibp_conns.next, struct kib_conn, ibc_list);
+   /* Advance to next connection, be sure to skip the head node */
+   if (!peer->ibp_next_conn ||
+   peer->ibp_next_conn->ibc_list.next == >ibp_conns)
+   next = peer->ibp_conns.next;
+   else
+   next = peer->ibp_next_conn->ibc_list.next;
+   peer->ibp_next_conn = list_entry(next, struct kib_conn, ibc_list);
+
+   return peer->ibp_next_conn;
}

static inline int
diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c 
b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
index 6690a6c..3cb1f720 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
@@ -1250,7 +1250,6 @@ static int kiblnd_resolve_addr(struct rdma_cm_id *cmid,

LASSERT(net);
LASSERT(peer->ibp_connecting > 0);
-   LASSERT(!peer->ibp_reconnecting);

cmid = kiblnd_rdma_create_id(kiblnd_cm_callback, peer, RDMA_PS_TCP,
 IB_QPT_RC);
@@ -1332,7 +1331,7 @@ static int kiblnd_resolve_addr(struct rdma_cm_id *cmid,

LASSERT(!peer->ibp_accepting && !peer->ibp_connecting &&
list_empty(>ibp_conns));
-   peer->ibp_reconnecting = 0;
+   peer->ibp_reconnecting--;

if (!kiblnd_peer_active(peer)) {
list_splice_init(>ibp_tx_queue, );
@@ -1365,6 +1364,8 @@ static int kiblnd_resolve_addr(struct rdma_cm_id *cmid,
rwlock_t *g_lock = _data.kib_global_lock;
unsigned long flags;
int rc;
+   inti;
+   struct lnet_ioctl_config_o2iblnd_tunables *tunables;

Re: [PATCH v2] Staging: iio: adis16209: Move adis16209 driver out of staging

[Shreeya Patel]

On 16 March 2018 00:31:53 GMT+05:30, Shreeya Patel 
 wrote:
>On Sat, 2018-03-10 at 15:57 +, Jonathan Cameron wrote:
>
>Hi Jonathan,
>
>> On Sat, 10 Mar 2018 15:50:23 +0530
>> Shreeya Patel  wrote:
>> 
>> > 
>> > Move the adis16209 driver out of staging directory and merge to the
>> > mainline IIO subsystem.
>> > 
>> > Signed-off-by: Shreeya Patel 
>> As this has a clear dependency on the previous patch, please put them
>> in the same series for the next version.  That way I won't miss one!
>> 
>> This also doesn't actually seem to have all the patches in place.
>> The sign extend one is definitely missing for some reason.
>> 
>> One question on the ABI choice of X for the rotation axis.
>> I think the logical choice is actually Z but would like to know what
>> you and others think.
>> 
>> All existing users of IIO_ROT (outside staging) have been
>> magnetometer
>> where we don't have an axis, but rather a magnetic reference frame or
>> a quaternion output which includes all the axes.
>> 
>> Jonathan
>> 
>> > 
>> > ---
>> > 
>> > Changes in v2
>> >   -Re-send the patch after having some cleanups in the
>> > file included in this patch.
>> > 
>> >  drivers/iio/accel/Kconfig |  12 ++
>> >  drivers/iio/accel/Makefile|   1 +
>> >  drivers/iio/accel/adis16209.c | 329
>> > ++
>> >  drivers/staging/iio/accel/Kconfig |  12 --
>> >  drivers/staging/iio/accel/Makefile|   1 -
>> >  drivers/staging/iio/accel/adis16209.c | 329 --
>> > 
>> >  6 files changed, 342 insertions(+), 342 deletions(-)
>> >  create mode 100644 drivers/iio/accel/adis16209.c
>> >  delete mode 100644 drivers/staging/iio/accel/adis16209.c
>> > 
>> > diff --git a/drivers/iio/accel/Kconfig b/drivers/iio/accel/Kconfig
>> > index c6d9517..f95f43c 100644
>> > --- a/drivers/iio/accel/Kconfig
>> > +++ b/drivers/iio/accel/Kconfig
>> > @@ -5,6 +5,18 @@
>> >  
>> >  menu "Accelerometers"
>> >  
>> > +config ADIS16209
>> > +tristate "Analog Devices ADIS16209 Dual-Axis Digital
>> > Inclinometer and Accelerometer"
>> > +depends on SPI
>> > +select IIO_ADIS_LIB
>> > +select IIO_ADIS_LIB_BUFFER if IIO_BUFFER
>> > +help
>> > +  Say Y here to build support for Analog Devices adis16209
>> > dual-axis digital inclinometer
>> > +  and accelerometer.
>> > +
>> > +  To compile this driver as a module, say M here: the
>> > module will be
>> > +  called adis16209.
>> > +
>> >  config ADXL345
>> >    tristate
>> >  
>> > diff --git a/drivers/iio/accel/Makefile
>> > b/drivers/iio/accel/Makefile
>> > index 368aedb..40861b9 100644
>> > --- a/drivers/iio/accel/Makefile
>> > +++ b/drivers/iio/accel/Makefile
>> > @@ -4,6 +4,7 @@
>> >  #
>> >  
>> >  # When adding new entries keep the list in alphabetical order
>> > +obj-$(CONFIG_ADIS16209) += adis16209.o
>> >  obj-$(CONFIG_ADXL345) += adxl345_core.o
>> >  obj-$(CONFIG_ADXL345_I2C) += adxl345_i2c.o
>> >  obj-$(CONFIG_ADXL345_SPI) += adxl345_spi.o
>> > diff --git a/drivers/iio/accel/adis16209.c
>> > b/drivers/iio/accel/adis16209.c
>> > new file mode 100644
>> > index 000..ed2e89f
>> > --- /dev/null
>> > +++ b/drivers/iio/accel/adis16209.c
>> > @@ -0,0 +1,329 @@
>> > +/*
>> > + * ADIS16209 Dual-Axis Digital Inclinometer and Accelerometer
>> > + *
>> > + * Copyright 2010 Analog Devices Inc.
>> > + *
>> > + * Licensed under the GPL-2 or later.
>> > + */
>> > +
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +
>> > +#include 
>> > +#include 
>> > +#include 
>> > +#include 
>> > +
>> > +#define ADIS16209_STARTUP_DELAY_MS220
>> > +#define ADIS16209_FLASH_CNT_REG   0x00
>> > +
>> > +/* Data Output Register Definitions */
>> > +#define ADIS16209_SUPPLY_OUT_REG  0x02
>> > +#define ADIS16209_XACCL_OUT_REG   0x04
>> > +#define ADIS16209_YACCL_OUT_REG   0x06
>> > +/* Output, auxiliary ADC input */
>> > +#define ADIS16209_AUX_ADC_REG 0x08
>> > +/* Output, temperature */
>> > +#define ADIS16209_TEMP_OUT_REG0x0A
>> > +/* Output, +/- 90 degrees X-axis inclination */
>> > +#define ADIS16209_XINCL_OUT_REG   0x0C
>> > +#define ADIS16209_YINCL_OUT_REG   0x0E
>> > +/* Output, +/-180 vertical rotational position */
>> > +#define ADIS16209_ROT_OUT_REG 0x10
>> > +
>> > +/*
>> > + * Calibration Register Definitions.
>> > + * Acceleration, inclination or rotation offset null.
>> > + */
>> > +#define ADIS16209_XACCL_NULL_REG  0x12
>> > +#define ADIS16209_YACCL_NULL_REG  0x14
>> > +#define ADIS16209_XINCL_NULL_REG  0x16
>> > +#define ADIS16209_YINCL_NULL_REG  0x18
>> > +#define ADIS16209_ROT_NULL_REG0x1A
>> > +
>> > +/* Alarm Register Definitions */
>> > +#define ADIS16209_ALM_MAG1_REG0x20
>> > 

Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1

[John Crispin]

On 15/03/18 21:12, NeilBrown wrote:

On Thu, Mar 15 2018, John Crispin wrote:


On 15/03/18 11:48, Dan Carpenter wrote:

This all seems fine.  Generally the requirements for staging are that it
has a TODO, someone to work on it, and it doesn't break the build.  But
some of the patches don't have commit message and those are required and
some of the commit messages are just the changes you have made not don't
describe the actual code...

John Crispin's email is j...@phrozen.org.

regards,
dan carpenter


Hi All,

looks like i was CC'ed on the openwrt addr, which no longer exists. This
series makes no sense. None of the stuff posted is anywhere near ready
to be upstreamed.

* we dont need a dedicated pinctrl driver, pinctrl-single will work fine
on these SoCs
* the DMA/sdhci driver is a hacked up version of the SDK driver.
* drivers/net/ethernet/mediatek/* works on mt7623 and is easily portable
to mt7621, same goes for the gsw driver.

Hi John,
  I think it makes sense in that, with the patches, the hardware works, and
  without the patches (at least the first) you cannot even build with
  CONFIG_SOC_MT7621=y as pcibios_map_irq() is undefined.  Having
  working code is a great starting point for further development.
  It certainly isn't ready for upstream, which is why it is heading for
  drivers/staging.  This is explicitly for code that isn't yet ready.
  By putting the code there it should be safe from bit-rot, and can be
  worked on by multiple people.  It gets increased visibility so people
  can say how bad it is (as you have done - thanks).  This feed back is a
  valuable part of improving the code and getting it out of staging.

  I'll add notes to various TODO files based on your comments.  If you
  have anything else to add, it would be most welcome.  Thank you for
  making these patches available in the first place, so that my hardware
  can work!

Thanks,
NeilBrown


Hi Neil,

I understand your reasoning, however ...
only the pcie driver is worth merging. all other drivers are already 
inside the kernel for mt7623 and can be easily adapted to work on 
mt7621. having duplicate drivers is a certain no-go.
cleaning up the pci driver is a matter of a few days work. merging a 
shitty pci driver just to postpone doing the 3-5 days work involved to 
polish it seems a rally bad trade-off.
i strongly oppose having any of this code merged into the kernel, even 
if it is only the staging area.


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

Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1

[NeilBrown]

On Thu, Mar 15 2018, John Crispin wrote:

> On 15/03/18 11:48, Dan Carpenter wrote:
>> This all seems fine.  Generally the requirements for staging are that it
>> has a TODO, someone to work on it, and it doesn't break the build.  But
>> some of the patches don't have commit message and those are required and
>> some of the commit messages are just the changes you have made not don't
>> describe the actual code...
>>
>> John Crispin's email is j...@phrozen.org.
>>
>> regards,
>> dan carpenter
>>
> Hi All,
>
> looks like i was CC'ed on the openwrt addr, which no longer exists. This 
> series makes no sense. None of the stuff posted is anywhere near ready 
> to be upstreamed.
>
> * we dont need a dedicated pinctrl driver, pinctrl-single will work fine 
> on these SoCs
> * the DMA/sdhci driver is a hacked up version of the SDK driver.
> * drivers/net/ethernet/mediatek/* works on mt7623 and is easily portable 
> to mt7621, same goes for the gsw driver.

Hi John,
 I think it makes sense in that, with the patches, the hardware works, and
 without the patches (at least the first) you cannot even build with
 CONFIG_SOC_MT7621=y as pcibios_map_irq() is undefined.  Having
 working code is a great starting point for further development.
 It certainly isn't ready for upstream, which is why it is heading for
 drivers/staging.  This is explicitly for code that isn't yet ready.
 By putting the code there it should be safe from bit-rot, and can be
 worked on by multiple people.  It gets increased visibility so people
 can say how bad it is (as you have done - thanks).  This feed back is a
 valuable part of improving the code and getting it out of staging.

 I'll add notes to various TODO files based on your comments.  If you
 have anything else to add, it would be most welcome.  Thank you for
 making these patches available in the first place, so that my hardware
 can work!

Thanks,
NeilBrown


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

Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1

[NeilBrown]

On Thu, Mar 15 2018, Dan Carpenter wrote:

> On Thu, Mar 15, 2018 at 10:04:33PM +1100, NeilBrown wrote:
>> On Thu, Mar 15 2018, Dan Carpenter wrote:
>> 
>> > This all seems fine.  Generally the requirements for staging are that it
>> > has a TODO, someone to work on it, and it doesn't break the build.  But
>> > some of the patches don't have commit message and those are required and
>> > some of the commit messages are just the changes you have made not don't
>> > describe the actual code...
>> 
>> Thanks for having a look.
>> It seems odd to require detailed commit messages, when we don't require
>> the same level of quality in the code.
>> Naturally when the driver is moved out of staging a properly detailed
>> commit message should be added, but is that needed on the way in to
>> staging?  At this stage I don't know much more than is already there.
>> After I've cleaned up the code I probably will.
>> 
>> For patch 01/13 you asked "what kind of device this is".  The subject
>> line makes it clear that it is a "pcie driver".  What extra detail did
>> you want?  Would it be sufficient to just copy the subject line so that
>> it appears twice in the commit message?
>> 
>
> Ah...  Sorry.  It's literally a pcie driver.  For some reason I thought
> it was a device that ran over pcie.
>
> We don't require a detailed changelog, but you have to put something...
> Probably just restating the subject and adding that it's for the gnubee1
> is fine.

I'll resend sometime next week with more words.  However could you
please clarify a couple of things for me?

1/ Why do you (sometimes) call the commit message a "change log".  When
  I see the term "change log" in the context of a patch, my first
  thought is that it it means a log of changes that have been made to
  the patch - typically through the review cycle.  But that isn't what
  you mean.  This has confused me a couple of times.

2/ Why don't you consider the first line of the commit message to be
   part of the commit message?  Why is duplication required?
   (You said "some of the patches don't have commit message[s]",
   which isn't true, though some of the messages are only one line).

Maybe the requirements on the commit message (including this
duplication) could be included in the "Staging trees" section of
Documentation/process/2.process.rst.
That file only lists the TODO and "doesn't break the build"
requirements.  It doesn't metion the "someone to work on it" requirement.
That might seem obvious, but it doesn't hurt to be explicit.

Thanks,
NeilBrown


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

Re: [PATCH] Staging: comedi: drivers: ni_atmio.c: fixed multi-line derefernce issue

[Pratik Jain]

Resending the email because it was sent only to Greg.

Context:
In my previous patch, I had removed an extra newline
at the end of the code.

My Reply:
It was unintentional, but does it violate any coding or
other standard?




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

[PATCH v2] drivers:staging:android:ashmem: Changing return type from int to loff_t

[Rohit Kumar]

Changing return type from int to loff_t. Actual return type of the
function (vfs_llseek) is loff_t (long long). Here due to implicit
converion from long long to int, result will be implementation defined.

Signed-off-by: Rohit Kumar 
---
 drivers/staging/android/ashmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 86580b6..a1a0025 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -321,7 +321,7 @@ static ssize_t ashmem_read_iter(struct kiocb *iocb, struct 
iov_iter *iter)
 static loff_t ashmem_llseek(struct file *file, loff_t offset, int origin)
 {
struct ashmem_area *asma = file->private_data;
-   int ret;
+   loff_t ret;
 
mutex_lock(_mutex);
 
-- 
2.7.4

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

[no subject]

[Rohit Kumar]

de...@driverdev.osuosl.org
Bcc: 
Subject: [PATCH v2] drivers:staging:android:ashmem: Changing return type from
 int to loff_t
Reply-To: 

Changing return type from int to loff_t. Actual return type of the
function (vfs_llseek) is loff_t (long long). Here due to implicit
converion from long long to int, result will be implementation defined.

Signed-off-by: Rohit Kumar 
---
 drivers/staging/android/ashmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/android/ashmem.c b/drivers/staging/android/ashmem.c
index 86580b6..a1a0025 100644
--- a/drivers/staging/android/ashmem.c
+++ b/drivers/staging/android/ashmem.c
@@ -321,7 +321,7 @@ static ssize_t ashmem_read_iter(struct kiocb *iocb, struct 
iov_iter *iter)
 static loff_t ashmem_llseek(struct file *file, loff_t offset, int origin)
 {
struct ashmem_area *asma = file->private_data;
-   int ret;
+   loff_t ret;
 
mutex_lock(_mutex);
 
-- 
2.7.4

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

[PATCH v4 2/2] media: staging/imx: fill vb2_v4l2_buffer sequence entry

[Peter Seiderer]

- enables gstreamer v4l2src lost frame detection, e.g:

  0:00:08.685185668  348  0x54f520 WARN  v4l2src 
gstv4l2src.c:970:gst_v4l2src_create: lost frames detected: count = 
141 - ts: 0:00:08.330177332

- fixes v4l2-compliance test failure:

  Streaming ioctls:
  test read/write: OK (Not Supported)
  Video Capture:
  Buffer: 0 Sequence: 0 Field: None Timestamp: 92.991450s
  Buffer: 1 Sequence: 0 Field: None Timestamp: 93.008135s
  fail: v4l2-test-buffers.cpp(294): (int)g_sequence() < 
seq.last_seq + 1
  fail: v4l2-test-buffers.cpp(707): buf.check(q, last_seq)

Signed-off-by: Peter Seiderer 
---
Changes in v2:
  - fill vb2_v4l2_buffer sequence entry in imx-ic-prpencvf too
(suggested by Steve Longerbeam)

Changes in v3:
  - add changelog (suggested by Greg Kroah-Hartman, Fabio Estevam
and Dan Carpenter) and patch history
  - use u32 (instead of __u32) (suggested by Dan Carpenter)
  - let sequence counter start with zero, keeping v4l2-compliance
testing happy (needs additional setting of field to a valid
value, patch will follow soon)

Changes in v4:
  - add v4l2-compliance test failure to changelog
  - reorder frame_sequence increment and assignement to
avoid -1 as start value (suggeted by Steve Longerbeam)
---
 drivers/staging/media/imx/imx-ic-prpencvf.c | 4 
 drivers/staging/media/imx/imx-media-csi.c   | 4 
 2 files changed, 8 insertions(+)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c 
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index ffeb017c73b2..28f41caba05d 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -103,6 +103,7 @@ struct prp_priv {
int nfb4eof_irq;
 
int stream_count;
+   u32 frame_sequence; /* frame sequence counter */
bool last_eof;  /* waiting for last EOF at stream off */
bool nfb4eof;/* NFB4EOF encountered during streaming */
struct completion last_eof_comp;
@@ -211,12 +212,14 @@ static void prp_vb2_buf_done(struct prp_priv *priv, 
struct ipuv3_channel *ch)
done = priv->active_vb2_buf[priv->ipu_buf_num];
if (done) {
done->vbuf.field = vdev->fmt.fmt.pix.field;
+   done->vbuf.sequence = priv->frame_sequence;
vb = >vbuf.vb2_buf;
vb->timestamp = ktime_get_ns();
vb2_buffer_done(vb, priv->nfb4eof ?
VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
}
 
+   priv->frame_sequence++;
priv->nfb4eof = false;
 
/* get next queued buffer */
@@ -638,6 +641,7 @@ static int prp_start(struct prp_priv *priv)
 
/* init EOF completion waitq */
init_completion(>last_eof_comp);
+   priv->frame_sequence = 0;
priv->last_eof = false;
priv->nfb4eof = false;
 
diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 5f69117b5811..3f2ce05848f3 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -111,6 +111,7 @@ struct csi_priv {
struct v4l2_ctrl_handler ctrl_hdlr;
 
int stream_count; /* streaming counter */
+   u32 frame_sequence; /* frame sequence counter */
bool last_eof;   /* waiting for last EOF at stream off */
bool nfb4eof;/* NFB4EOF encountered during streaming */
struct completion last_eof_comp;
@@ -237,12 +238,14 @@ static void csi_vb2_buf_done(struct csi_priv *priv)
done = priv->active_vb2_buf[priv->ipu_buf_num];
if (done) {
done->vbuf.field = vdev->fmt.fmt.pix.field;
+   done->vbuf.sequence = priv->frame_sequence;
vb = >vbuf.vb2_buf;
vb->timestamp = ktime_get_ns();
vb2_buffer_done(vb, priv->nfb4eof ?
VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE);
}
 
+   priv->frame_sequence++;
priv->nfb4eof = false;
 
/* get next queued buffer */
@@ -544,6 +547,7 @@ static int csi_idmac_start(struct csi_priv *priv)
 
/* init EOF completion waitq */
init_completion(>last_eof_comp);
+   priv->frame_sequence = 0;
priv->last_eof = false;
priv->nfb4eof = false;
 
-- 
2.16.2

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

[PATCH v4 1/2] media: staging/imx: fill vb2_v4l2_buffer field entry

[Peter Seiderer]

- fixes gstreamer v4l2src warning:

  0:00:00.716640334  349  0x164f720 WARN  v4l2bufferpool 
gstv4l2bufferpool.c:1195:gst_v4l2_buffer_pool_dqbuf: Driver 
should never set v4l2_buffer.field to ANY

- fixes v4l2-compliance test failure:

  Streaming ioctls:
  test read/write: OK (Not Supported)
  Video Capture:
  Buffer: 0 Sequence: 0 Field: Any Timestamp: 58.383658s
  fail: v4l2-test-buffers.cpp(297): g_field() == V4L2_FIELD_ANY

Signed-off-by: Peter Seiderer 
---
Changes in v4:
  - new patch (put first because patch is needed to advance with
the v4l2-compliance test), thanks to Philipp Zabel
 for suggested solution for the right
field value source
---
 drivers/staging/media/imx/imx-ic-prpencvf.c | 1 +
 drivers/staging/media/imx/imx-media-csi.c   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/staging/media/imx/imx-ic-prpencvf.c 
b/drivers/staging/media/imx/imx-ic-prpencvf.c
index ae453fd422f0..ffeb017c73b2 100644
--- a/drivers/staging/media/imx/imx-ic-prpencvf.c
+++ b/drivers/staging/media/imx/imx-ic-prpencvf.c
@@ -210,6 +210,7 @@ static void prp_vb2_buf_done(struct prp_priv *priv, struct 
ipuv3_channel *ch)
 
done = priv->active_vb2_buf[priv->ipu_buf_num];
if (done) {
+   done->vbuf.field = vdev->fmt.fmt.pix.field;
vb = >vbuf.vb2_buf;
vb->timestamp = ktime_get_ns();
vb2_buffer_done(vb, priv->nfb4eof ?
diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index 5a195f80a24d..5f69117b5811 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -236,6 +236,7 @@ static void csi_vb2_buf_done(struct csi_priv *priv)
 
done = priv->active_vb2_buf[priv->ipu_buf_num];
if (done) {
+   done->vbuf.field = vdev->fmt.fmt.pix.field;
vb = >vbuf.vb2_buf;
vb->timestamp = ktime_get_ns();
vb2_buffer_done(vb, priv->nfb4eof ?
-- 
2.16.2

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

[PATCH v2 4/4] staging: ks7010: remove unnecessary brackets in single statement block

[Sergio Paracuellos]

This patch fix the following checkpatch warning:
braces {} are not necessary for single statement blocks

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/ks7010/ks_hostif.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/ks_hostif.c 
b/drivers/staging/ks7010/ks_hostif.c
index a5ff033..d644a1d 100644
--- a/drivers/staging/ks7010/ks_hostif.c
+++ b/drivers/staging/ks7010/ks_hostif.c
@@ -1297,9 +1297,8 @@ static __le16 ks_wlan_cap(struct ks_wlan_private *priv)
 {
u16 capability = 0x;
 
-   if (priv->reg.preamble == SHORT_PREAMBLE) {
+   if (priv->reg.preamble == SHORT_PREAMBLE)
capability |= WLAN_CAPABILITY_SHORT_PREAMBLE;
-   }
 
capability &= ~(WLAN_CAPABILITY_PBCC);  /* pbcc not support */
 
-- 
2.7.4

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

[PATCH v2 0/4] staging: ks7010: some cleanups and replaces

[Sergio Paracuellos]

The following patch series includes some cleanups of useless traces as well
as some replaces in order to use preferred macros for debugging and others.

v2:
* Review deleted DPRINTK traces
* Minor checkpatch warning cleanup 

Sergio Paracuellos (4):
  staging: ks7010: remove useless DPRINTK traces
  staging: ks7010: replace DPRINTK traces in favour of netdev_*
  staging: ks7010: replace KS_WLAN_DEBUG with DEBUG preprocessor
directive
  staging: ks7010: remove unnecessary brackets in single statement block

 drivers/staging/ks7010/ks7010_sdio.c | 151 ++
 drivers/staging/ks7010/ks_hostif.c   | 288 ++-
 drivers/staging/ks7010/ks_wlan.h |  10 --
 drivers/staging/ks7010/ks_wlan_net.c |  90 +++
 4 files changed, 145 insertions(+), 394 deletions(-)

-- 
2.7.4

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

[PATCH v2 1/4] staging: ks7010: remove useless DPRINTK traces

[Sergio Paracuellos]

This commit removes some useless traces in some source files

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/ks7010/ks7010_sdio.c |  58 --
 drivers/staging/ks7010/ks_hostif.c   | 147 +++
 drivers/staging/ks7010/ks_wlan_net.c |  56 +
 3 files changed, 10 insertions(+), 251 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 7de78d1..beb689b 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -112,8 +112,6 @@ static void ks_wlan_hw_sleep_doze_request(struct 
ks_wlan_private *priv)
 {
int ret;
 
-   DPRINTK(4, "\n");
-
/* clear request */
atomic_set(>sleepstatus.doze_request, 0);
 
@@ -123,11 +121,8 @@ static void ks_wlan_hw_sleep_doze_request(struct 
ks_wlan_private *priv)
DPRINTK(1, " error : GCR_B\n");
goto set_sleep_mode;
}
-   DPRINTK(3, "sleep_mode=SLP_SLEEP\n");
atomic_set(>sleepstatus.status, 1);
priv->last_doze = jiffies;
-   } else {
-   DPRINTK(1, "sleep_mode=%d\n", priv->sleep_mode);
}
 
 set_sleep_mode:
@@ -138,8 +133,6 @@ static void ks_wlan_hw_sleep_wakeup_request(struct 
ks_wlan_private *priv)
 {
int ret;
 
-   DPRINTK(4, "\n");
-
/* clear request */
atomic_set(>sleepstatus.wakeup_request, 0);
 
@@ -149,12 +142,9 @@ static void ks_wlan_hw_sleep_wakeup_request(struct 
ks_wlan_private *priv)
DPRINTK(1, " error : WAKEUP\n");
goto set_sleep_mode;
}
-   DPRINTK(4, "wake up : WAKEUP\n");
atomic_set(>sleepstatus.status, 0);
priv->last_wakeup = jiffies;
++priv->wakeup_count;
-   } else {
-   DPRINTK(1, "sleep_mode=%d\n", priv->sleep_mode);
}
 
 set_sleep_mode:
@@ -165,19 +155,13 @@ void ks_wlan_hw_wakeup_request(struct ks_wlan_private 
*priv)
 {
int ret;
 
-   DPRINTK(4, "\n");
if (atomic_read(>psstatus.status) == PS_SNOOZE) {
ret = ks7010_sdio_writeb(priv, WAKEUP, WAKEUP_REQ);
if (ret)
DPRINTK(1, " error : WAKEUP\n");
-   else
-   DPRINTK(4, "wake up : WAKEUP\n");
 
priv->last_wakeup = jiffies;
++priv->wakeup_count;
-   } else {
-   DPRINTK(1, "psstatus=%d\n",
-   atomic_read(>psstatus.status));
}
 }
 
@@ -228,7 +212,6 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private 
*priv)
goto queue_delayed_work;
}
atomic_set(>psstatus.status, PS_SNOOZE);
-   DPRINTK(3, "psstatus.status=PS_SNOOZE\n");
 
return;
 
@@ -288,7 +271,6 @@ static int write_to_device(struct ks_wlan_private *priv, 
unsigned char *buffer,
 
hdr = (struct hostif_hdr *)buffer;
 
-   DPRINTK(4, "size=%d\n", hdr->size);
if (le16_to_cpu(hdr->event) < HIF_DATA_REQ ||
le16_to_cpu(hdr->event) > HIF_REQ_MAX) {
DPRINTK(1, "unknown event=%04X\n", hdr->event);
@@ -315,7 +297,6 @@ static void tx_device_task(struct ks_wlan_private *priv)
struct tx_device_buffer *sp;
int ret;
 
-   DPRINTK(4, "\n");
if (cnt_txqbody(priv) <= 0 ||
atomic_read(>psstatus.status) == PS_SNOOZE)
return;
@@ -358,7 +339,6 @@ int ks_wlan_hw_tx(struct ks_wlan_private *priv, void *p, 
unsigned long size,
priv->hostt.buff[priv->hostt.qtail] = le16_to_cpu(hdr->event);
priv->hostt.qtail = (priv->hostt.qtail + 1) % SME_EVENT_BUFF_SIZE;
 
-   DPRINTK(4, "event=%04X\n", hdr->event);
spin_lock(>tx_dev.tx_dev_lock);
result = enqueue_txdev(priv, p, size, complete_handler, skb);
spin_unlock(>tx_dev.tx_dev_lock);
@@ -374,8 +354,6 @@ static void rx_event_task(unsigned long dev)
struct ks_wlan_private *priv = (struct ks_wlan_private *)dev;
struct rx_device_buffer *rp;
 
-   DPRINTK(4, "\n");
-
if (cnt_rxqbody(priv) > 0 && priv->dev_state >= DEVICE_STATE_BOOT) {
rp = >rx_dev.rx_dev_buff[priv->rx_dev.qhead];
hostif_receive(priv, rp->data, rp->size);
@@ -393,8 +371,6 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, 
uint16_t size)
struct hostif_hdr *hdr;
unsigned short event = 0;
 
-   DPRINTK(4, "\n");
-
/* receive data */
if (cnt_rxqbody(priv) >= (RX_DEVICE_BUFF_SIZE - 1)) {
DPRINTK(1, "rx buffer overflow\n");
@@ -450,8 +426,6 @@ static void ks7010_rw_function(struct work_struct *work)
 
priv = container_of(work, struct ks_wlan_private, rw_dwork.work);
 
-   DPRINTK(4, "\n");
-
/* wait after DOZE */
if (time_after(priv->last_doze + ((30 * HZ) / 

[PATCH v2 2/4] staging: ks7010: replace DPRINTK traces in favour of netdev_*

[Sergio Paracuellos]

This commit removes custom defined DPRINTK macro and replaces all the
associated debug and other traces for preferred ones netdev_*.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/ks7010/ks7010_sdio.c |  84 ++---
 drivers/staging/ks7010/ks_hostif.c   | 138 ++-
 drivers/staging/ks7010/ks_wlan.h |  10 ---
 drivers/staging/ks7010/ks_wlan_net.c |  34 -
 4 files changed, 130 insertions(+), 136 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index beb689b..50a7a15 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -118,7 +118,7 @@ static void ks_wlan_hw_sleep_doze_request(struct 
ks_wlan_private *priv)
if (atomic_read(>sleepstatus.status) == 0) {
ret = ks7010_sdio_writeb(priv, GCR_B, GCR_B_DOZE);
if (ret) {
-   DPRINTK(1, " error : GCR_B\n");
+   netdev_err(priv->net_dev, " error : GCR_B\n");
goto set_sleep_mode;
}
atomic_set(>sleepstatus.status, 1);
@@ -139,7 +139,7 @@ static void ks_wlan_hw_sleep_wakeup_request(struct 
ks_wlan_private *priv)
if (atomic_read(>sleepstatus.status) == 1) {
ret = ks7010_sdio_writeb(priv, WAKEUP, WAKEUP_REQ);
if (ret) {
-   DPRINTK(1, " error : WAKEUP\n");
+   netdev_err(priv->net_dev, " error : WAKEUP\n");
goto set_sleep_mode;
}
atomic_set(>sleepstatus.status, 0);
@@ -158,7 +158,7 @@ void ks_wlan_hw_wakeup_request(struct ks_wlan_private *priv)
if (atomic_read(>psstatus.status) == PS_SNOOZE) {
ret = ks7010_sdio_writeb(priv, WAKEUP, WAKEUP_REQ);
if (ret)
-   DPRINTK(1, " error : WAKEUP\n");
+   netdev_err(priv->net_dev, " error : WAKEUP\n");
 
priv->last_wakeup = jiffies;
++priv->wakeup_count;
@@ -185,11 +185,11 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private 
*priv)
if (atomic_read(>psstatus.status) == PS_SNOOZE)
return;
 
-   DPRINTK(5, 
"\npsstatus.status=%d\npsstatus.confirm_wait=%d\npsstatus.snooze_guard=%d\ncnt_txqbody=%d\n",
-   atomic_read(>psstatus.status),
-   atomic_read(>psstatus.confirm_wait),
-   atomic_read(>psstatus.snooze_guard),
-   cnt_txqbody(priv));
+   netdev_dbg(priv->net_dev, 
"\npsstatus.status=%d\npsstatus.confirm_wait=%d\npsstatus.snooze_guard=%d\ncnt_txqbody=%d\n",
+  atomic_read(>psstatus.status),
+  atomic_read(>psstatus.confirm_wait),
+  atomic_read(>psstatus.snooze_guard),
+  cnt_txqbody(priv));
 
if (atomic_read(>psstatus.confirm_wait) ||
atomic_read(>psstatus.snooze_guard) ||
@@ -200,7 +200,7 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private 
*priv)
 
ret = ks7010_sdio_readb(priv, INT_PENDING, );
if (ret) {
-   DPRINTK(1, " error : INT_PENDING\n");
+   netdev_err(priv->net_dev, " error : INT_PENDING\n");
goto queue_delayed_work;
}
if (byte)
@@ -208,7 +208,7 @@ static void _ks_wlan_hw_power_save(struct ks_wlan_private 
*priv)
 
ret = ks7010_sdio_writeb(priv, GCR_B, GCR_B_DOZE);
if (ret) {
-   DPRINTK(1, " error : GCR_B\n");
+   netdev_err(priv->net_dev, " error : GCR_B\n");
goto queue_delayed_work;
}
atomic_set(>psstatus.status, PS_SNOOZE);
@@ -240,7 +240,7 @@ static int enqueue_txdev(struct ks_wlan_private *priv, 
unsigned char *p,
}
 
if ((TX_DEVICE_BUFF_SIZE - 1) <= cnt_txqbody(priv)) {
-   DPRINTK(1, "tx buffer overflow\n");
+   netdev_err(priv->net_dev, "tx buffer overflow\n");
ret = -EOVERFLOW;
goto err_complete;
}
@@ -273,19 +273,19 @@ static int write_to_device(struct ks_wlan_private *priv, 
unsigned char *buffer,
 
if (le16_to_cpu(hdr->event) < HIF_DATA_REQ ||
le16_to_cpu(hdr->event) > HIF_REQ_MAX) {
-   DPRINTK(1, "unknown event=%04X\n", hdr->event);
+   netdev_err(priv->net_dev, "unknown event=%04X\n", hdr->event);
return 0;
}
 
ret = ks7010_sdio_write(priv, DATA_WINDOW, buffer, size);
if (ret) {
-   DPRINTK(1, " write error : retval=%d\n", ret);
+   netdev_err(priv->net_dev, " write error : retval=%d\n", ret);
return ret;
}
 
ret = ks7010_sdio_writeb(priv, WRITE_STATUS, REG_STATUS_BUSY);
if (ret) {
-   DPRINTK(1, " error : WRITE_STATUS\n");
+   netdev_err(priv->net_dev, " error : 

[PATCH v2 3/4] staging: ks7010: replace KS_WLAN_DEBUG with DEBUG preprocessor directive

[Sergio Paracuellos]

This commit replaces custom KS_WLAN_DEBUG which is not being used anymore
in favour of DEBUG which is the one included when debugging is enabled.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/ks7010/ks7010_sdio.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c 
b/drivers/staging/ks7010/ks7010_sdio.c
index 50a7a15..b8f55a1 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -385,11 +385,10 @@ static void ks_wlan_hw_rx(struct ks_wlan_private *priv, 
uint16_t size)
 
/* length check */
if (size > 2046 || size == 0) {
-#ifdef KS_WLAN_DEBUG
-   if (KS_WLAN_DEBUG > 5)
-   print_hex_dump_bytes("INVALID DATA dump: ",
-DUMP_PREFIX_OFFSET,
-rx_buffer->data, 32);
+#ifdef DEBUG
+   print_hex_dump_bytes("INVALID DATA dump: ",
+DUMP_PREFIX_OFFSET,
+rx_buffer->data, 32);
 #endif
ret = ks7010_sdio_writeb(priv, READ_STATUS, REG_STATUS_IDLE);
if (ret)
-- 
2.7.4

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

[PATCH] Staging: comedi: drivers: ni_atmio.c: fixed multi-line derefernce issue

[Pratik Jain]

Fixed coding style issue.

Signed-off-by: Pratik Jain 
---
 drivers/staging/comedi/drivers/ni_atmio.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_atmio.c 
b/drivers/staging/comedi/drivers/ni_atmio.c
index b9e9ab548c4b..4e27a2959b64 100644
--- a/drivers/staging/comedi/drivers/ni_atmio.c
+++ b/drivers/staging/comedi/drivers/ni_atmio.c
@@ -226,8 +226,8 @@ static int ni_isapnp_find_board(struct pnp_dev **dev)
for (i = 0; i < ARRAY_SIZE(ni_boards); i++) {
isapnp_dev = pnp_find_dev(NULL,
  ISAPNP_VENDOR('N', 'I', 'C'),
- ISAPNP_FUNCTION(ni_boards[i].
- isapnp_id), NULL);
+ 
ISAPNP_FUNCTION(ni_boards[i].isapnp_id),
+ NULL);
 
if (!isapnp_dev || !isapnp_dev->card)
continue;
-- 
2.16.2

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

RE: [PATCH v4 1/2] PCI: hv: Serialize the present and eject work items

[Dexuan Cui]

> From: Dexuan Cui
> > From: Lorenzo Pieralisi 
> > I need to know either what commit you are fixing (ie Fixes: tag - which
> > is preferrable) or you tell me which kernel versions we are targeting
> > for the stable backport.
> > Lorenzo
> 
> Sorry.  Here I was hesitant to add a "Fixes:" because the bug was there the 
> first
> day
> when the driver was introduced.
> 
> Please use
> Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft
> Hyper-V VMs")
> or
> Cc:  # v4.6+

BTW, the bug here is a race condtion which couldn't be easily hit in the past,
probably because most of the time only one PCI device was only added into the
VM once. But now it's becoming typical that a VM can have 4 GPU devices so we
start to notice this bug. With 7 Mellanox VFs assigned to a VM, we can easily
reproduce the bug by "hot-remove and hot-add VFs" test:

general protection fault:  [#1] SMP
...
Workqueue: events hv_eject_device_work [pci_hyperv]
task: 8800ed5e5400 ti: 8800ee674000 task.ti: 8800ee674000
RIP: 0010:[]  ... hv_eject_device_work+0xbe/0x160 [pci_hyperv]
...
Call Trace:
 [] ? __schedule+0x3b6/0xa30
 [] process_one_work+0x165/0x480
 [] worker_thread+0x4b/0x4c0
 [] ? process_one_work+0x480/0x480
 [] kthread+0xe5/0x100
 [] ? kthread_create_on_node+0x1e0/0x1e0
 [] ret_from_fork+0x3f/0x70
 [] ? kthread_create_on_node+0x1e0/0x1e0
Code: ...
RIP  [] hv_eject_device_work+0xbe/0x160 [pci_hyperv]
...
BUG: unable to handle kernel paging request at ffd8
IP: [] kthread_data+0x10/0x20
PGD 1e0d067 PUD 1e0f067 PMD 0
Oops:  [#2] SMP
 
Thanks,
-- Dexuan
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption

[Sridhar Pitchai]

Apologies for not aligning my reply, I just realized after looking into
the mail archive in LKML.  I have aligned the replay now.


On 3/15/18, 10:56 AM, "Sridhar Pitchai"  wrote:

Hi Lorenzo,
Answering the question inline.
Kindly let me know if it clarifies. I will send out another patch after we 
agree on the clarification.

Thanks
Sridhar Pitchai

-Original Message-
From: Lorenzo Pieralisi 
Sent: Thursday, March 15, 2018 5:05 AM
To: Sridhar Pitchai 
Cc: Bjorn Helgaas ; Jake Oshins ; 
Haiyang Zhang ; Stephen Hemminger 
; Dexuan Cui ; KY Srinivasan 
; Michael Kelley (EOSG) ; 
de...@linuxdriverproject.org; linux-...@vger.kernel.org; 
linux-ker...@vger.kernel.org
Subject: Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption

On Thu, Mar 15, 2018 at 12:03:07AM +, Sridhar Pitchai wrote:
Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even

"Whenever a PCI bus is added"
Sridhar>> yes

with that when a first device is added to the bus, it overrides bus domain
ID with the device serial number. Sometime this can result in BUS ID not

Define "Sometime".

Sridhar>> HyperV when it creates a PCI bus it guarantees it provide a unique ID 
for it.
But, that unique BUS ID is replaced with device serial number. 0 is a valid
device serial number, and if there exists a PCI bus with domain ID 0 (Gen 1
version of hyperV VM have this for para virtual devices), this will result in
PCI bus id not being unique.


being unique. In this case, when PCI_BUS and a device to bus is added, the
first device overwrites the bus domain ID to the device serial number,
which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI

s/exsist/exist

Sridhar>> yes

bus addition fails. This patch make sure when a device is added to a bus,
it never updated the bus domain ID.

s/updated/updates
Sridhar >> yes

Since we have the transparent SRIOV mode now, the short VF device name
is no longer needed.

I still do not understand what this means and how it is related to the
patch below, it may be clear to you, it is not to me, at all.

Sridhar >> the patch below, was introduced to make the device name small, by 
taking only
16bits of the serial number. Since we are not going to have the serial number
updated to the BUS id, this has to be removed.

Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")

Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
Sridhr >> yes

I asked you an explicit question. Commit above was added for a reason
I assume. This patch implies that kernel has been broken since v4.11
which is almost a year ago and nobody every noticed ? Or there are
systems where commit above is _necessary_ and this patch would break
them ?

I want a detailed explanation that highlights *why* it is safe to apply
this patch and send it to stable kernels, commit log above won't do.

Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by 
the child
device when it is added. This cannot produce a unique domain ID all the time.
Here in the bug, we see the collision between the serial number and already
existing PCI bus. The cleaner way is never touch the domain ID provided by
hyperV during the PCI bus creation. As long as hyperV make sure it provides a
unique domain ID for the PCI for a VM it will not break, and HyperV will
guarantees that the domain for the PCI bus for a given VM will be always unique.
The original patch was also intending to have a unique domain ID for the PCI
bus, by taking the serial number of the device, but it is not sufficient, when
the device serial number is number which is the domain ID of the existing PCI
bus.  With the current kernel we can repro this issue by adding a device with a
serial number matching the existing PCI bus domain id. (in this case that
happens to be zero).


Thanks,
Lorenzo

Cc: sta...@vger.kernel.org
Signed-off-by: Sridhar Pitchai 
---
Changes in v3:
* fix the commit comment. [KY Srinivasan, Michael Kelley]
---
  drivers/pci/host/pci-hyperv.c | 11 ---
  1 file changed, 11 deletions(-)
diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 2faf38e..ac67e56 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1518,17 +1518,6 @@ static struct hv_pci_dev *new_pcichild_device(struct 
hv_pcibus_device *hbus,
get_pcichild(hpdev, hv_pcidev_ref_childlist);
spin_lock_irqsave(>device_list_lock, flags);
  
-   /*
-* When a device is being added to the bus, we set the PCI domain
-* number to be the device serial number, which is non-zero and
-* unique on the same VM.  The serial numbers start with 1, and
-* increase by 1 for each 

[PATCH] staging: android: ion: Update wording in drivers/staging/android/ion/Kconfig

[Phillip Potter]

Changes the usage of the word 'Chose' to 'Choose' in the ION
Memory Manager Kconfig.

Signed-off-by: Phillip Potter 
---
--- a/drivers/staging/android/ion/Kconfig
+++ b/drivers/staging/android/ion/Kconfig
@@ -4,7 +4,7 @@ menuconfig ION
select GENERIC_ALLOCATOR
select DMA_SHARED_BUFFER
---help---
- Chose this option to enable the ION Memory Manager,
+ Choose this option to enable the ION Memory Manager,
  used by Android to efficiently allocate buffers
  from userspace that can be shared between drivers.
  If you're not using Android its probably safe to
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption

[Sridhar Pitchai]

Hi Lorenzo,
Answering the question inline.
Kindly let me know if it clarifies. I will send out another patch after we 
agree on the clarification.

Thanks
Sridhar Pitchai

-Original Message-
From: Lorenzo Pieralisi  
Sent: Thursday, March 15, 2018 5:05 AM
To: Sridhar Pitchai 
Cc: Bjorn Helgaas ; Jake Oshins ; 
Haiyang Zhang ; Stephen Hemminger 
; Dexuan Cui ; KY Srinivasan 
; Michael Kelley (EOSG) ; 
de...@linuxdriverproject.org; linux-...@vger.kernel.org; 
linux-ker...@vger.kernel.org
Subject: Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption

On Thu, Mar 15, 2018 at 12:03:07AM +, Sridhar Pitchai wrote:
> Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even

"Whenever a PCI bus is added"
Sridhar>> yes

> with that when a first device is added to the bus, it overrides bus domain
> ID with the device serial number. Sometime this can result in BUS ID not

Define "Sometime".

Sridhar>> HyperV when it creates a PCI bus it guarantees it provide a unique ID 
for it. But, that unique BUS ID is replaced with device serial number. 0 is a 
valid device serial number, and if there exists a PCI bus with domain ID 0 (Gen 
1 version of hyperV VM have this for para virtual devices), this will result in 
PCI bus id not being unique. 

> being unique. In this case, when PCI_BUS and a device to bus is added, the
> first device overwrites the bus domain ID to the device serial number,
> which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI

s/exsist/exist

Sridhar>> yes

> bus addition fails. This patch make sure when a device is added to a bus,
> it never updated the bus domain ID. 

s/updated/updates
Sridhar >> yes

> Since we have the transparent SRIOV mode now, the short VF device name
> is no longer needed.

I still do not understand what this means and how it is related to the
patch below, it may be clear to you, it is not to me, at all.

Sridhar >> the patch below, was introduced to make the device name small, by 
taking only 16bits of the serial number. Since we are not going to have the 
serial number updated to the BUS id, this has to be removed.

> Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")

Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")
Sridhr >> yes

I asked you an explicit question. Commit above was added for a reason
I assume. This patch implies that kernel has been broken since v4.11
which is almost a year ago and nobody every noticed ? Or there are
systems where commit above is _necessary_ and this patch would break
them ?

I want a detailed explanation that highlights *why* it is safe to apply
this patch and send it to stable kernels, commit log above won't do.

Sridhar>> HyperV provides a unique domain ID for PCI BUS. But it is modified by 
the child device when it is added. This cannot produce a unique domain ID all 
the time. Here in the bug, we see the collision between the serial number and 
already existing PCI bus. The cleaner way is never touch the domain ID provided 
by hyperV during the PCI bus creation. As long as hyperV make sure it provides 
a unique domain ID for the PCI for a VM it will not break, and HyperV will 
guarantees that the domain for the PCI bus for a given VM will be always unique.
The original patch was also intending to have a unique domain ID for the PCI 
bus, by taking the serial number of the device, but it is not sufficient, when 
the device serial number is number which is the domain ID of the existing PCI 
bus.
With the current kernel we can repro this issue by adding a device with a 
serial number matching the existing PCI bus domain id. (in this case that 
happens to be zero).

Thanks,
Lorenzo

> Cc: sta...@vger.kernel.org
> Signed-off-by: Sridhar Pitchai 
> ---
> 
> Changes in v3:
> * fix the commit comment. [KY Srinivasan, Michael Kelley]
> ---
>  drivers/pci/host/pci-hyperv.c | 11 ---
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 2faf38e..ac67e56 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1518,17 +1518,6 @@ static struct hv_pci_dev *new_pcichild_device(struct 
> hv_pcibus_device *hbus,
>   get_pcichild(hpdev, hv_pcidev_ref_childlist);
>   spin_lock_irqsave(>device_list_lock, flags);
>  
> - /*
> -  * When a device is being added to the bus, we set the PCI domain
> -  * number to be the device serial number, which is non-zero and
> -  * unique on the same VM.  The serial numbers start with 1, and
> -  * increase by 1 for each device.  So device names including this
> -  * can have shorter names than based on the bus instance UUID.
> -  * Only the first device 

RE: [PATCH v4 1/2] PCI: hv: Serialize the present and eject work items

[Dexuan Cui]

> From: Lorenzo Pieralisi 
> Sent: Thursday, March 15, 2018 10:02
> On Thu, Mar 15, 2018 at 02:20:53PM +, Dexuan Cui wrote:
> > When we hot-remove the device, we first receive a PCI_EJECT message and
> > then receive a PCI_BUS_RELATIONS message with bus_rel->device_count ==
> 0.
> >
> > The first message is offloaded to   (), and the second
> > is offloaded to pci_devices_present_work(). Both the paths can be running
> > list_del(>list_entry), causing general protection fault, because
> > system_wq can run them concurrently.
> >
> > The patch eliminates the race condition.
> >
> > Cc: sta...@vger.kernel.org
> 
> I need to know either what commit you are fixing (ie Fixes: tag - which
> is preferrable) or you tell me which kernel versions we are targeting
> for the stable backport.
> 
> Thanks,
> Lorenzo

Sorry.  Here I was hesitant to add a "Fixes:" because the bug was there the 
first day
when the driver was introduced.

Please use
Fixes: 4daace0d8ce8 ("PCI: hv: Add paravirtual PCI front-end for Microsoft 
Hyper-V VMs")
or 
Cc:  # v4.6+

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

Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send

[Stephen Hemminger]

On Thu, 15 Mar 2018 17:24:13 +0100
Mohammed Gamal  wrote:

> On Wed, 2018-03-14 at 10:22 +0100, Mohammed Gamal wrote:
> > On Tue, 2018-03-13 at 12:35 -0700, Stephen Hemminger wrote:  
> > > On Tue, 13 Mar 2018 20:06:50 +0100
> > > Mohammed Gamal  wrote:
> > >   
> > > > Dring high network traffic changes to network interface
> > > > parameters
> > > > such as number of channels or MTU can cause a kernel panic with a
> > > > NULL
> > > > pointer dereference. This is due to netvsc_device_remove() being
> > > > called and deallocating the channel ring buffers, which can then
> > > > be
> > > > accessed by netvsc_send_pkt() before they're allocated on calling
> > > > netvsc_device_add()
> > > > 
> > > > The patch fixes this problem by checking the channel state and
> > > > returning
> > > > ENODEV if not yet opened. We also move the call to
> > > > hv_ringbuf_avail_percent()
> > > > which may access the uninitialized ring buffer.
> > > > 
> > > > Signed-off-by: Mohammed Gamal 
> > > > ---
> > > >  drivers/net/hyperv/netvsc.c | 5 +++--
> > > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/hyperv/netvsc.c
> > > > b/drivers/net/hyperv/netvsc.c
> > > > index 0265d70..44a8358 100644
> > > > --- a/drivers/net/hyperv/netvsc.c
> > > > +++ b/drivers/net/hyperv/netvsc.c
> > > > @@ -757,7 +757,7 @@ static inline int netvsc_send_pkt(
> > > >     struct netdev_queue *txq = netdev_get_tx_queue(ndev,
> > > > packet->q_idx);
> > > >     u64 req_id;
> > > >     int ret;
> > > > -   u32 ring_avail = hv_ringbuf_avail_percent(_channel-  
> > > > > outbound);  
> > > > 
> > > > +   u32 ring_avail;
> > > >  
> > > >     nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
> > > >     if (skb)
> > > > @@ -773,7 +773,7 @@ static inline int netvsc_send_pkt(
> > > >  
> > > >     req_id = (ulong)skb;
> > > >  
> > > > -   if (out_channel->rescind)
> > > > +   if (out_channel->rescind || out_channel->state !=
> > > > CHANNEL_OPENED_STATE)
> > > >     return -ENODEV;
> > > >  
> > > >     if (packet->page_buf_cnt) {
> > > > @@ -791,6 +791,7 @@ static inline int netvsc_send_pkt(
> > > >        VMBUS_DATA_PACKET_FLAG_CO
> > > > MP
> > > > LETION_REQUESTED);
> > > >     }
> > > >  
> > > > +   ring_avail = hv_ringbuf_avail_percent(_channel-  
> > > > > outbound);  
> > > > 
> > > >     if (ret == 0) {
> > > >     atomic_inc_return(>queue_sends);
> > > >    
> > > 
> > > Thanks for your patch. Yes there are races with the current update
> > > logic. The root cause goes higher up in the flow; the send queues
> > > should
> > > be stopped before netvsc_device_remove is called. Solving it where
> > > you tried
> > > to is racy and not going to work reliably.
> > > 
> > > Network patches should go to net...@vger.kernel.org
> > > 
> > > You can't move the ring_avail check until after the
> > > vmbus_sendpacket
> > > because
> > > that will break the flow control logic.
> > >   
> > 
> > Why? I don't see ring_avail being used before that point.  
> 
> Ah, stupid me. vmbus_sendpacket() will write to the ring buffer and
> that means that ring_avail value will be different than the expected.
> 
> >   
> > > Instead, you should just move the avail_read check until just after
> > > the existing rescind
> > > check.
> > > 
> > > Also, you shouldn't need to check for OPENED_STATE, just rescind is
> > > enough.  
> > 
> > That rarely mitigated the race. channel->rescind flag is set on vmbus
> > exit - called on module unload - and when a rescind offer is received
> > from the host, which AFAICT doesn't happen on every call to
> > netvsc_device_remove, so it's quite possible that the ringbuffer is
> > accessed before it's allocated again on channel open and hence the
> > check for OPENED_STAT - which is only set after all vmbus data is
> > initialized.
> >   
> 
> Perhaps I haven't been clear enough. The NULL pointer dereference
> happens in the call to hv_ringbuf_avail_percent() which is used to
> calculate ring_avail. 
> 
> So we need to stop the queues before calling it if the channel's ring
> buffers haven't been allocated yet, but OTOH we should only stop the
> queues based upon the value of ring_avail, so this leads into a chicken
> and egg situation. 
> 
> Is my observation here correct? Please correct me if I am wrong,
> Stephen.

This is a far more drastic work of the shutdown logic which I am still working
on. The current netvsc driver is not doing a good job of handling concurrent
send/receives during ring changes.


From a22da18b41ad5024340dddcc989d918987836f6d Mon Sep 17 00:00:00 2001
From: Stephen Hemminger 
Date: Tue, 6 Feb 2018 15:05:19 -0800
Subject: [PATCH] hv_netvsc: common detach logic

Make common function for detaching internals of device
during changes to MTU and RSS. Make sure no more packets
are 

Re: [PATCH] Staging: comedi: drivers: ni_atmio.c: fixed multi-line derefernce issue

[Greg KH]

On Thu, Mar 15, 2018 at 11:00:40PM +0530, Pratik Jain wrote:
> Fixed coding style issue.
> 
> Signed-off-by: Pratik Jain 
> ---
>  drivers/staging/comedi/drivers/ni_atmio.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/comedi/drivers/ni_atmio.c 
> b/drivers/staging/comedi/drivers/ni_atmio.c
> index b9e9ab548c4b..e82fbe987dd8 100644
> --- a/drivers/staging/comedi/drivers/ni_atmio.c
> +++ b/drivers/staging/comedi/drivers/ni_atmio.c
> @@ -226,8 +226,8 @@ static int ni_isapnp_find_board(struct pnp_dev **dev)
>   for (i = 0; i < ARRAY_SIZE(ni_boards); i++) {
>   isapnp_dev = pnp_find_dev(NULL,
> ISAPNP_VENDOR('N', 'I', 'C'),
> -   ISAPNP_FUNCTION(ni_boards[i].
> -   isapnp_id), NULL);
> +   
> ISAPNP_FUNCTION(ni_boards[i].isapnp_id),
> +   NULL);
>  
>   if (!isapnp_dev || !isapnp_dev->card)
>   continue;
> @@ -356,4 +356,3 @@ module_comedi_driver(ni_atmio_driver);
>  MODULE_AUTHOR("Comedi http://www.comedi.org;);
>  MODULE_DESCRIPTION("Comedi low-level driver");
>  MODULE_LICENSE("GPL");
> -

Why did you also delete this last line?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] Staging: comedi: drivers: ni_atmio.c: fixed multi-line derefernce issue

[Pratik Jain]

Fixed coding style issue.

Signed-off-by: Pratik Jain 
---
 drivers/staging/comedi/drivers/ni_atmio.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_atmio.c 
b/drivers/staging/comedi/drivers/ni_atmio.c
index b9e9ab548c4b..e82fbe987dd8 100644
--- a/drivers/staging/comedi/drivers/ni_atmio.c
+++ b/drivers/staging/comedi/drivers/ni_atmio.c
@@ -226,8 +226,8 @@ static int ni_isapnp_find_board(struct pnp_dev **dev)
for (i = 0; i < ARRAY_SIZE(ni_boards); i++) {
isapnp_dev = pnp_find_dev(NULL,
  ISAPNP_VENDOR('N', 'I', 'C'),
- ISAPNP_FUNCTION(ni_boards[i].
- isapnp_id), NULL);
+ 
ISAPNP_FUNCTION(ni_boards[i].isapnp_id),
+ NULL);
 
if (!isapnp_dev || !isapnp_dev->card)
continue;
@@ -356,4 +356,3 @@ module_comedi_driver(ni_atmio_driver);
 MODULE_AUTHOR("Comedi http://www.comedi.org;);
 MODULE_DESCRIPTION("Comedi low-level driver");
 MODULE_LICENSE("GPL");
-
-- 
2.16.2

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

[PATCH] staging:mt29f_spinand: MT29F2G failing as only 16 bits used for addressing.

[palle.christensen]

From: Palle Christensen 

For NAND flash chips with more than 1Gbit (e.g. MT29F2G) more than 16 bits
are necessary to address the correct page.

Signed-off-by: Palle Christensen 
---
 drivers/staging/mt29f_spinand/mt29f_spinand.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/mt29f_spinand/mt29f_spinand.c 
b/drivers/staging/mt29f_spinand/mt29f_spinand.c
index 264ad36..d20284b 100644
--- a/drivers/staging/mt29f_spinand/mt29f_spinand.c
+++ b/drivers/staging/mt29f_spinand/mt29f_spinand.c
@@ -316,6 +316,7 @@ static int spinand_read_page_to_cache(struct spi_device 
*spi_nand, u16 page_id)
row = page_id;
cmd.cmd = CMD_READ;
cmd.n_addr = 3;
+   cmd.addr[0] = (u8)((row & 0xff) >> 16);
cmd.addr[1] = (u8)((row & 0xff00) >> 8);
cmd.addr[2] = (u8)(row & 0x00ff);
 
@@ -464,6 +465,7 @@ static int spinand_program_execute(struct spi_device 
*spi_nand, u16 page_id)
row = page_id;
cmd.cmd = CMD_PROG_PAGE_EXC;
cmd.n_addr = 3;
+   cmd.addr[0] = (u8)((row & 0xff) >> 16);
cmd.addr[1] = (u8)((row & 0xff00) >> 8);
cmd.addr[2] = (u8)(row & 0x00ff);
 
@@ -579,6 +581,7 @@ static int spinand_erase_block_erase(struct spi_device 
*spi_nand, u16 block_id)
row = block_id;
cmd.cmd = CMD_ERASE_BLK;
cmd.n_addr = 3;
+   cmd.addr[0] = (u8)((row & 0xff) >> 16);
cmd.addr[1] = (u8)((row & 0xff00) >> 8);
cmd.addr[2] = (u8)(row & 0x00ff);
 
-- 
1.9.1

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

Re: [PATCH v4 1/2] PCI: hv: Serialize the present and eject work items

[Lorenzo Pieralisi]

On Thu, Mar 15, 2018 at 02:20:53PM +, Dexuan Cui wrote:
> When we hot-remove the device, we first receive a PCI_EJECT message and
> then receive a PCI_BUS_RELATIONS message with bus_rel->device_count == 0.
> 
> The first message is offloaded to hv_eject_device_work(), and the second
> is offloaded to pci_devices_present_work(). Both the paths can be running
> list_del(>list_entry), causing general protection fault, because
> system_wq can run them concurrently.
> 
> The patch eliminates the race condition.
> 
> Tested-by: Adrian Suhov 
> Tested-by: Chris Valean 
> Signed-off-by: Dexuan Cui 
> Reviewed-by: Michael Kelley 
> Acked-by: Haiyang Zhang 
> Cc: sta...@vger.kernel.org

I need to know either what commit you are fixing (ie Fixes: tag - which
is preferrable) or you tell me which kernel versions we are targeting
for the stable backport.

Thanks,
Lorenzo

> Cc: Vitaly Kuznetsov 
> Cc: Jack Morgenstein 
> Cc: Stephen Hemminger 
> Cc: K. Y. Srinivasan 
> ---
>  drivers/pci/host/pci-hyperv.c | 17 ++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 2faf38eab785..5e67dff779a7 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -461,6 +461,8 @@ struct hv_pcibus_device {
>   struct retarget_msi_interrupt retarget_msi_interrupt_params;
>  
>   spinlock_t retarget_msi_interrupt_lock;
> +
> + struct workqueue_struct *wq;
>  };
>  
>  /*
> @@ -1770,7 +1772,7 @@ static void hv_pci_devices_present(struct 
> hv_pcibus_device *hbus,
>   spin_unlock_irqrestore(>device_list_lock, flags);
>  
>   get_hvpcibus(hbus);
> - schedule_work(_wrk->wrk);
> + queue_work(hbus->wq, _wrk->wrk);
>  }
>  
>  /**
> @@ -1848,7 +1850,7 @@ static void hv_pci_eject_device(struct hv_pci_dev 
> *hpdev)
>   get_pcichild(hpdev, hv_pcidev_ref_pnp);
>   INIT_WORK(>wrk, hv_eject_device_work);
>   get_hvpcibus(hpdev->hbus);
> - schedule_work(>wrk);
> + queue_work(hpdev->hbus->wq, >wrk);
>  }
>  
>  /**
> @@ -2463,11 +2465,17 @@ static int hv_pci_probe(struct hv_device *hdev,
>   spin_lock_init(>retarget_msi_interrupt_lock);
>   sema_init(>enum_sem, 1);
>   init_completion(>remove_event);
> + hbus->wq = alloc_ordered_workqueue("hv_pci_%x", 0,
> +hbus->sysdata.domain);
> + if (!hbus->wq) {
> + ret = -ENOMEM;
> + goto free_bus;
> + }
>  
>   ret = vmbus_open(hdev->channel, pci_ring_size, pci_ring_size, NULL, 0,
>hv_pci_onchannelcallback, hbus);
>   if (ret)
> - goto free_bus;
> + goto destroy_wq;
>  
>   hv_set_drvdata(hdev, hbus);
>  
> @@ -2536,6 +2544,8 @@ static int hv_pci_probe(struct hv_device *hdev,
>   hv_free_config_window(hbus);
>  close:
>   vmbus_close(hdev->channel);
> +destroy_wq:
> + destroy_workqueue(hbus->wq);
>  free_bus:
>   free_page((unsigned long)hbus);
>   return ret;
> @@ -2615,6 +2625,7 @@ static int hv_pci_remove(struct hv_device *hdev)
>   irq_domain_free_fwnode(hbus->sysdata.fwnode);
>   put_hvpcibus(hbus);
>   wait_for_completion(>remove_event);
> + destroy_workqueue(hbus->wq);
>   free_page((unsigned long)hbus);
>   return 0;
>  }
> -- 
> 2.7.4
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] hv_netvsc: Make sure out channel is fully opened on send

[Mohammed Gamal]

On Wed, 2018-03-14 at 10:22 +0100, Mohammed Gamal wrote:
> On Tue, 2018-03-13 at 12:35 -0700, Stephen Hemminger wrote:
> > On Tue, 13 Mar 2018 20:06:50 +0100
> > Mohammed Gamal  wrote:
> > 
> > > Dring high network traffic changes to network interface
> > > parameters
> > > such as number of channels or MTU can cause a kernel panic with a
> > > NULL
> > > pointer dereference. This is due to netvsc_device_remove() being
> > > called and deallocating the channel ring buffers, which can then
> > > be
> > > accessed by netvsc_send_pkt() before they're allocated on calling
> > > netvsc_device_add()
> > > 
> > > The patch fixes this problem by checking the channel state and
> > > returning
> > > ENODEV if not yet opened. We also move the call to
> > > hv_ringbuf_avail_percent()
> > > which may access the uninitialized ring buffer.
> > > 
> > > Signed-off-by: Mohammed Gamal 
> > > ---
> > >  drivers/net/hyperv/netvsc.c | 5 +++--
> > >  1 file changed, 3 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/net/hyperv/netvsc.c
> > > b/drivers/net/hyperv/netvsc.c
> > > index 0265d70..44a8358 100644
> > > --- a/drivers/net/hyperv/netvsc.c
> > > +++ b/drivers/net/hyperv/netvsc.c
> > > @@ -757,7 +757,7 @@ static inline int netvsc_send_pkt(
> > >   struct netdev_queue *txq = netdev_get_tx_queue(ndev,
> > > packet->q_idx);
> > >   u64 req_id;
> > >   int ret;
> > > - u32 ring_avail = hv_ringbuf_avail_percent(_channel-
> > > > outbound);
> > > 
> > > + u32 ring_avail;
> > >  
> > >   nvmsg.hdr.msg_type = NVSP_MSG1_TYPE_SEND_RNDIS_PKT;
> > >   if (skb)
> > > @@ -773,7 +773,7 @@ static inline int netvsc_send_pkt(
> > >  
> > >   req_id = (ulong)skb;
> > >  
> > > - if (out_channel->rescind)
> > > + if (out_channel->rescind || out_channel->state !=
> > > CHANNEL_OPENED_STATE)
> > >   return -ENODEV;
> > >  
> > >   if (packet->page_buf_cnt) {
> > > @@ -791,6 +791,7 @@ static inline int netvsc_send_pkt(
> > >      VMBUS_DATA_PACKET_FLAG_CO
> > > MP
> > > LETION_REQUESTED);
> > >   }
> > >  
> > > + ring_avail = hv_ringbuf_avail_percent(_channel-
> > > > outbound);
> > > 
> > >   if (ret == 0) {
> > >   atomic_inc_return(>queue_sends);
> > >  
> > 
> > Thanks for your patch. Yes there are races with the current update
> > logic. The root cause goes higher up in the flow; the send queues
> > should
> > be stopped before netvsc_device_remove is called. Solving it where
> > you tried
> > to is racy and not going to work reliably.
> > 
> > Network patches should go to net...@vger.kernel.org
> > 
> > You can't move the ring_avail check until after the
> > vmbus_sendpacket
> > because
> > that will break the flow control logic.
> > 
> 
> Why? I don't see ring_avail being used before that point.

Ah, stupid me. vmbus_sendpacket() will write to the ring buffer and
that means that ring_avail value will be different than the expected.

> 
> > Instead, you should just move the avail_read check until just after
> > the existing rescind
> > check.
> > 
> > Also, you shouldn't need to check for OPENED_STATE, just rescind is
> > enough.
> 
> That rarely mitigated the race. channel->rescind flag is set on vmbus
> exit - called on module unload - and when a rescind offer is received
> from the host, which AFAICT doesn't happen on every call to
> netvsc_device_remove, so it's quite possible that the ringbuffer is
> accessed before it's allocated again on channel open and hence the
> check for OPENED_STAT - which is only set after all vmbus data is
> initialized.
> 

Perhaps I haven't been clear enough. The NULL pointer dereference
happens in the call to hv_ringbuf_avail_percent() which is used to
calculate ring_avail. 

So we need to stop the queues before calling it if the channel's ring
buffers haven't been allocated yet, but OTOH we should only stop the
queues based upon the value of ring_avail, so this leads into a chicken
and egg situation. 

Is my observation here correct? Please correct me if I am wrong,
Stephen.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v6 0/6] staging: Introduce DPAA2 Ethernet Switch driver

[Dan Carpenter]

On Thu, Mar 15, 2018 at 12:44:37AM +0100, Andrew Lunn wrote:
> On Wed, Mar 14, 2018 at 10:55:52AM -0500, Razvan Stefanescu wrote:
> > This patchset introduces the Ethernet Switch Driver for Freescale/NXP SoCs
> > with DPAA2 (DataPath Acceleration Architecture v2). The driver manages
> > switch objects discovered on the fsl-mc bus. A description of the driver
> > can be found in the associated README file.
> 
> Hi Greg
> 
> This code has much better quality than the usual stuff in staging. I
> see no reason not to merge it. 

Yeah.  It seems pretty decent.  Stuart, Laurentiu, care to comment?

Meanwhile, netdev and DaveM aren't even on the CC list and they're the
ones to ultimately decide.

regards,
dan carpenter

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

[PATCH v4 4/4] PCI: hv: Only queue a new work in hv_pci_devices_present() if necessary

[Dexuan Cui]

If there is a pending work, we just need to add the new dr into
the dr_list.

Signed-off-by: Dexuan Cui 
Reviewed-by: Michael Kelley 
Acked-by: Haiyang Zhang 
Cc: Vitaly Kuznetsov 
Cc: Jack Morgenstein 
Cc: Stephen Hemminger 
Cc: K. Y. Srinivasan 
---
 drivers/pci/host/pci-hyperv.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 0dc2ecdbbe45..50cdefe3f6d3 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1789,6 +1789,7 @@ static void hv_pci_devices_present(struct 
hv_pcibus_device *hbus,
struct hv_dr_state *dr;
struct hv_dr_work *dr_wrk;
unsigned long flags;
+   bool pending_dr;
 
dr_wrk = kzalloc(sizeof(*dr_wrk), GFP_NOWAIT);
if (!dr_wrk)
@@ -1812,11 +1813,21 @@ static void hv_pci_devices_present(struct 
hv_pcibus_device *hbus,
}
 
spin_lock_irqsave(>device_list_lock, flags);
+   /*
+* If pending_dr is true, we have already queued a work,
+* which will see the new dr. Otherwise, we need to
+* queue a new work.
+*/
+   pending_dr = !list_empty(>dr_list);
list_add_tail(>list_entry, >dr_list);
spin_unlock_irqrestore(>device_list_lock, flags);
 
-   get_hvpcibus(hbus);
-   queue_work(hbus->wq, _wrk->wrk);
+   if (pending_dr) {
+   kfree(dr_wrk);
+   } else {
+   get_hvpcibus(hbus);
+   queue_work(hbus->wq, _wrk->wrk);
+   }
 }
 
 /**
-- 
2.7.4

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

[PATCH v4 3/4] PCI: hv: Remove hbus->enum_sem

[Dexuan Cui]

Since we serialize the present/eject work items now, we don't need the
semaphore any more.

Signed-off-by: Dexuan Cui 
Reviewed-by: Michael Kelley 
Acked-by: Haiyang Zhang 
Cc: Vitaly Kuznetsov 
Cc: Jack Morgenstein 
Cc: Stephen Hemminger 
Cc: K. Y. Srinivasan 
---
 drivers/pci/host/pci-hyperv.c | 17 ++---
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 1d2e1cb617f4..0dc2ecdbbe45 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -447,7 +447,6 @@ struct hv_pcibus_device {
spinlock_t device_list_lock;/* Protect lists below */
void __iomem *cfg_addr;
 
-   struct semaphore enum_sem;
struct list_head resources_for_children;
 
struct list_head children;
@@ -1648,12 +1647,8 @@ static struct hv_pci_dev *get_pcichild_wslot(struct 
hv_pcibus_device *hbus,
  * It must also treat the omission of a previously observed device as
  * notification that the device no longer exists.
  *
- * Note that this function is a work item, and it may not be
- * invoked in the order that it was queued.  Back to back
- * updates of the list of present devices may involve queuing
- * multiple work items, and this one may run before ones that
- * were sent later. As such, this function only does something
- * if is the last one in the queue.
+ * Note that this function is serialized with hv_eject_device_work(),
+ * because both are pushed to the ordered workqueue hbus->wq.
  */
 static void pci_devices_present_work(struct work_struct *work)
 {
@@ -1674,11 +1669,6 @@ static void pci_devices_present_work(struct work_struct 
*work)
 
INIT_LIST_HEAD();
 
-   if (down_interruptible(>enum_sem)) {
-   put_hvpcibus(hbus);
-   return;
-   }
-
/* Pull this off the queue and process it if it was the last one. */
spin_lock_irqsave(>device_list_lock, flags);
while (!list_empty(>dr_list)) {
@@ -1695,7 +1685,6 @@ static void pci_devices_present_work(struct work_struct 
*work)
spin_unlock_irqrestore(>device_list_lock, flags);
 
if (!dr) {
-   up(>enum_sem);
put_hvpcibus(hbus);
return;
}
@@ -1782,7 +1771,6 @@ static void pci_devices_present_work(struct work_struct 
*work)
break;
}
 
-   up(>enum_sem);
put_hvpcibus(hbus);
kfree(dr);
 }
@@ -2516,7 +2504,6 @@ static int hv_pci_probe(struct hv_device *hdev,
spin_lock_init(>config_lock);
spin_lock_init(>device_list_lock);
spin_lock_init(>retarget_msi_interrupt_lock);
-   sema_init(>enum_sem, 1);
init_completion(>remove_event);
hbus->wq = alloc_ordered_workqueue("hv_pci_%x", 0,
   hbus->sysdata.domain);
-- 
2.7.4

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

[PATCH v4 2/4] PCI: hv: Remove the bogus test in hv_eject_device_work()

[Dexuan Cui]

When we're in the function, hpdev->state must be hv_pcichild_ejecting:
see hv_pci_eject_device().

Signed-off-by: Dexuan Cui 
Reviewed-by: Michael Kelley 
Acked-by: Haiyang Zhang 
Cc: Vitaly Kuznetsov 
Cc: Jack Morgenstein 
Cc: Stephen Hemminger 
Cc: K. Y. Srinivasan 
---
 drivers/pci/host/pci-hyperv.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 28fce2be0a5f..1d2e1cb617f4 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -1854,10 +1854,7 @@ static void hv_eject_device_work(struct work_struct 
*work)
 
hpdev = container_of(work, struct hv_pci_dev, wrk);
 
-   if (hpdev->state != hv_pcichild_ejecting) {
-   put_pcichild(hpdev, hv_pcidev_ref_pnp);
-   return;
-   }
+   WARN_ON(hpdev->state != hv_pcichild_ejecting);
 
/*
 * Ejection can come before or after the PCI bus has been set up, so
-- 
2.7.4

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

[PATCH v4 1/4] PCI: hv: Fix a comment typo in _hv_pcifront_read_config()

[Dexuan Cui]

No functional change.

Fixes: bdd74440d9e8 ("PCI: hv: Add explicit barriers to config space access")
Signed-off-by: Dexuan Cui 
Acked-by: Haiyang Zhang 
Cc: Vitaly Kuznetsov 
Cc: Stephen Hemminger 
Cc: K. Y. Srinivasan 
---
 drivers/pci/host/pci-hyperv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index acb1941423a9..28fce2be0a5f 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -657,7 +657,7 @@ static void _hv_pcifront_read_config(struct hv_pci_dev 
*hpdev, int where,
break;
}
/*
-* Make sure the write was done before we release the spinlock
+* Make sure the read was done before we release the spinlock
 * allowing consecutive reads/writes.
 */
mb();
-- 
2.7.4

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

[PATCH v4 0/4] PCI: hv: Add 4 cosmetic patches

[Dexuan Cui]

The 4 patches should go in v4.16.

There is no code change since v3:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1628496.html

I just split the original 6 patches to 2 series, and this is series 2.

I fixed the order of Signed-off-by/Tested-by/Cc, etc., and fixed the
other format issues in changelog (I hope I have fixed all of them).

Dexuan Cui (4):
  PCI: hv: Fix a comment typo in _hv_pcifront_read_config()
  PCI: hv: Remove the bogus test in hv_eject_device_work()
  PCI: hv: Remove hbus->enum_sem
  PCI: hv: Only queue a new work in hv_pci_devices_present() if
necessary

 drivers/pci/host/pci-hyperv.c | 39 +--
 1 file changed, 17 insertions(+), 22 deletions(-)

-- 
2.7.4

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

[PATCH v4 2/2] PCI: hv: Fix 2 hang issues in hv_compose_msi_msg()

[Dexuan Cui]

1. With the patch "x86/vector/msi: Switch to global reservation mode",
the recent v4.15 and newer kernels always hang for 1-vCPU Hyper-V VM
with SR-IOV. This is because when we reach hv_compose_msi_msg() by
request_irq() -> request_threaded_irq() ->__setup_irq()->irq_startup()
-> __irq_startup() -> irq_domain_activate_irq() -> ... ->
msi_domain_activate() -> ... -> hv_compose_msi_msg(), local irq is
disabled in __setup_irq().

Note: when we reach hv_compose_msi_msg() by another code path:
pci_enable_msix_range() -> ... -> irq_domain_activate_irq() -> ... ->
hv_compose_msi_msg(), local irq is not disabled.

hv_compose_msi_msg() depends on an interrupt from the host.
With interrupts disabled, a UP VM always hangs in the busy loop in
the function, because the interrupt callback hv_pci_onchannelcallback()
can not be called.

We can do nothing but work it around by polling the channel. This
is ugly, but we don't have any other choice.

2. If the host is ejecting the VF device before we reach
hv_compose_msi_msg(), in a UP VM, we can hang in hv_compose_msi_msg()
forever, because at this time the host doesn't respond to the
CREATE_INTERRUPT request. This issue exists the first day the
pci-hyperv driver appears in the kernel.

Luckily, this can also by worked around by polling the channel
for the PCI_EJECT message and hpdev->state, and by checking the
PCI vendor ID.

Note: actually the above 2 issues also happen to a SMP VM, if
"hbus->hdev->channel->target_cpu == smp_processor_id()" is true.

Fixes: 4900be83602b ("x86/vector/msi: Switch to global reservation mode")
Tested-by: Adrian Suhov 
Tested-by: Chris Valean 
Signed-off-by: Dexuan Cui 
Reviewed-by: Michael Kelley 
Acked-by: Haiyang Zhang 
Cc: sta...@vger.kernel.org
Cc: Stephen Hemminger 
Cc: K. Y. Srinivasan 
Cc: Vitaly Kuznetsov 
Cc: Jack Morgenstein 
---
 drivers/pci/host/pci-hyperv.c | 58 ++-
 1 file changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 5e67dff779a7..acb1941423a9 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -522,6 +522,8 @@ struct hv_pci_compl {
s32 completion_status;
 };
 
+static void hv_pci_onchannelcallback(void *context);
+
 /**
  * hv_pci_generic_compl() - Invoked for a completion packet
  * @context:   Set up by the sender of the packet.
@@ -666,6 +668,31 @@ static void _hv_pcifront_read_config(struct hv_pci_dev 
*hpdev, int where,
}
 }
 
+static u16 hv_pcifront_get_vendor_id(struct hv_pci_dev *hpdev)
+{
+   u16 ret;
+   unsigned long flags;
+   void __iomem *addr = hpdev->hbus->cfg_addr + CFG_PAGE_OFFSET +
+PCI_VENDOR_ID;
+
+   spin_lock_irqsave(>hbus->config_lock, flags);
+
+   /* Choose the function to be read. (See comment above) */
+   writel(hpdev->desc.win_slot.slot, hpdev->hbus->cfg_addr);
+   /* Make sure the function was chosen before we start reading. */
+   mb();
+   /* Read from that function's config space. */
+   ret = readw(addr);
+   /*
+* mb() is not required here, because the spin_unlock_irqrestore()
+* is a barrier.
+*/
+
+   spin_unlock_irqrestore(>hbus->config_lock, flags);
+
+   return ret;
+}
+
 /**
  * _hv_pcifront_write_config() - Internal PCI config write
  * @hpdev: The PCI driver's representation of the device
@@ -1108,8 +1135,37 @@ static void hv_compose_msi_msg(struct irq_data *data, 
struct msi_msg *msg)
 * Since this function is called with IRQ locks held, can't
 * do normal wait for completion; instead poll.
 */
-   while (!try_wait_for_completion(_pkt.host_event))
+   while (!try_wait_for_completion(_pkt.host_event)) {
+   /* 0x means an invalid PCI VENDOR ID. */
+   if (hv_pcifront_get_vendor_id(hpdev) == 0x) {
+   dev_err_once(>hdev->device,
+"the device has gone\n");
+   goto free_int_desc;
+   }
+
+   /*
+* When the higher level interrupt code calls us with
+* interrupt disabled, we must poll the channel by calling
+* the channel callback directly when channel->target_cpu is
+* the current CPU. When the higher level interrupt code
+* calls us with interrupt enabled, let's add the
+* local_bh_disable()/enable() to avoid race.
+*/
+   local_bh_disable();
+
+   if (hbus->hdev->channel->target_cpu == smp_processor_id())
+   hv_pci_onchannelcallback(hbus);
+
+   local_bh_enable();
+
+   if 

[PATCH v4 1/2] PCI: hv: Serialize the present and eject work items

[Dexuan Cui]

When we hot-remove the device, we first receive a PCI_EJECT message and
then receive a PCI_BUS_RELATIONS message with bus_rel->device_count == 0.

The first message is offloaded to hv_eject_device_work(), and the second
is offloaded to pci_devices_present_work(). Both the paths can be running
list_del(>list_entry), causing general protection fault, because
system_wq can run them concurrently.

The patch eliminates the race condition.

Tested-by: Adrian Suhov 
Tested-by: Chris Valean 
Signed-off-by: Dexuan Cui 
Reviewed-by: Michael Kelley 
Acked-by: Haiyang Zhang 
Cc: sta...@vger.kernel.org
Cc: Vitaly Kuznetsov 
Cc: Jack Morgenstein 
Cc: Stephen Hemminger 
Cc: K. Y. Srinivasan 
---
 drivers/pci/host/pci-hyperv.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 2faf38eab785..5e67dff779a7 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -461,6 +461,8 @@ struct hv_pcibus_device {
struct retarget_msi_interrupt retarget_msi_interrupt_params;
 
spinlock_t retarget_msi_interrupt_lock;
+
+   struct workqueue_struct *wq;
 };
 
 /*
@@ -1770,7 +1772,7 @@ static void hv_pci_devices_present(struct 
hv_pcibus_device *hbus,
spin_unlock_irqrestore(>device_list_lock, flags);
 
get_hvpcibus(hbus);
-   schedule_work(_wrk->wrk);
+   queue_work(hbus->wq, _wrk->wrk);
 }
 
 /**
@@ -1848,7 +1850,7 @@ static void hv_pci_eject_device(struct hv_pci_dev *hpdev)
get_pcichild(hpdev, hv_pcidev_ref_pnp);
INIT_WORK(>wrk, hv_eject_device_work);
get_hvpcibus(hpdev->hbus);
-   schedule_work(>wrk);
+   queue_work(hpdev->hbus->wq, >wrk);
 }
 
 /**
@@ -2463,11 +2465,17 @@ static int hv_pci_probe(struct hv_device *hdev,
spin_lock_init(>retarget_msi_interrupt_lock);
sema_init(>enum_sem, 1);
init_completion(>remove_event);
+   hbus->wq = alloc_ordered_workqueue("hv_pci_%x", 0,
+  hbus->sysdata.domain);
+   if (!hbus->wq) {
+   ret = -ENOMEM;
+   goto free_bus;
+   }
 
ret = vmbus_open(hdev->channel, pci_ring_size, pci_ring_size, NULL, 0,
 hv_pci_onchannelcallback, hbus);
if (ret)
-   goto free_bus;
+   goto destroy_wq;
 
hv_set_drvdata(hdev, hbus);
 
@@ -2536,6 +2544,8 @@ static int hv_pci_probe(struct hv_device *hdev,
hv_free_config_window(hbus);
 close:
vmbus_close(hdev->channel);
+destroy_wq:
+   destroy_workqueue(hbus->wq);
 free_bus:
free_page((unsigned long)hbus);
return ret;
@@ -2615,6 +2625,7 @@ static int hv_pci_remove(struct hv_device *hdev)
irq_domain_free_fwnode(hbus->sysdata.fwnode);
put_hvpcibus(hbus);
wait_for_completion(>remove_event);
+   destroy_workqueue(hbus->wq);
free_page((unsigned long)hbus);
return 0;
 }
-- 
2.7.4

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

[PATCH v4 0/2] PCI: hv: Fix some real issues

[Dexuan Cui]

The 2 patches should go in v4.16.

They should go in old stable kernels too, especially this one should go in
v4.15:
  PCI: hv: Fix 2 hang issues in hv_compose_msi_msg()
Otherwise, a UP v4.15 VM can 100% hang when we pass through a VF
to the VM that runs on Hyper-V.

There is no code change since v3:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1628496.html

I just split the original 6 patches to 2 series, and this is series 1.

I fixed the order of Signed-off-by/Tested-by/Cc, etc., and added a Fixes:
tag, and fixed the other format issues in changelog. I hope I didn't omit
anything this time. :-)

Dexuan Cui (2):
  PCI: hv: Serialize the present and eject work items
  PCI: hv: Fix 2 hang issues in hv_compose_msi_msg()

 drivers/pci/host/pci-hyperv.c | 75 ---
 1 file changed, 71 insertions(+), 4 deletions(-)

-- 
2.7.4

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

Re: [PATCH 7/7] staging:iio:ade7854: Add proper error handling condition

[Dan Carpenter]

On Wed, Mar 14, 2018 at 03:12:18PM -0300, Rodrigo Siqueira wrote:
> There is some improper error handling for IRQ and device register.  This
> patch adds a proper verification. The IRQ correction was extracted from
> John Syne patches.
> 
> Signed-off-by: Rodrigo Siqueira 
> Signed-off-by: John Syne 
> ---
>  drivers/staging/iio/meter/ade7854.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854.c 
> b/drivers/staging/iio/meter/ade7854.c
> index 09fd8c067738..49cbe365e43d 100644
> --- a/drivers/staging/iio/meter/ade7854.c
> +++ b/drivers/staging/iio/meter/ade7854.c
> @@ -436,7 +436,7 @@ static int ade7854_initial_setup(struct iio_dev 
> *indio_dev)
>  
>   /* Disable IRQ */
>   ret = ade7854_set_irq(dev, false);
> - if (ret) {
> + if (ret < 0) {
>   dev_err(dev, "disable irq failed");
>   goto err_ret;
>   }

Why is the original wrong?  It seems fine.

regards,
dan carpenter


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

Re: [PATCH 1/7] staging:iio:ade7854: Rework I2C write function

[Dan Carpenter]

On Wed, Mar 14, 2018 at 03:10:18PM -0300, Rodrigo Siqueira wrote:
> The write operation using I2C has many code duplications and four
> different interfaces per data size. This patch introduces a single
> function that centralizes the main tasks.
> 
> The central function inserted by this patch can easily replace all the
> four functions related to the data size. However, this patch does not
> remove any code signature for keeping the meter module work and make
> easier to review this patch.
> 
> Signed-off-by: Rodrigo Siqueira 
> ---
>  drivers/staging/iio/meter/ade7854-i2c.c | 89 
> +++--
>  drivers/staging/iio/meter/ade7854.h |  7 +++
>  2 files changed, 58 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c 
> b/drivers/staging/iio/meter/ade7854-i2c.c
> index 317e4f0d8176..03133a05eae4 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -15,41 +15,74 @@
>  #include 
>  #include "ade7854.h"
>  
> -static int ade7854_i2c_write_reg_8(struct device *dev,
> -u16 reg_address,
> -u8 val)
> +static int ade7854_i2c_write_reg(struct device *dev,
> +  u16 reg_address,
> +  u32 val,
> +  enum data_size type)


The data size should just be the number of bytes and not an enum.
1 means 1 byte / 8 bits.
2 means 2 bytes / 16 bits.
3 means 3 bytes / 24 bits.
etc.

>  {
>   int ret;
> + int count;
>   struct iio_dev *indio_dev = dev_to_iio_dev(dev);
>   struct ade7854_state *st = iio_priv(indio_dev);
>  
>   mutex_lock(>buf_lock);
>   st->tx[0] = (reg_address >> 8) & 0xFF;
>   st->tx[1] = reg_address & 0xFF;
> - st->tx[2] = val;
>  
> - ret = i2c_master_send(st->i2c, st->tx, 3);
> + switch (type) {
> + case DATA_SIZE_8_BITS:
> + st->tx[2] = val & 0xFF;
> + count = 3;
> + break;
> + case DATA_SIZE_16_BITS:
> + st->tx[2] = (val >> 8) & 0xFF;
> + st->tx[3] = val & 0xFF;
> + count = 4;
> + break;
> + case DATA_SIZE_24_BITS:
> + st->tx[2] = (val >> 16) & 0xFF;
> + st->tx[3] = (val >> 8) & 0xFF;
> + st->tx[4] = val & 0xFF;
> + count = 5;
> + break;
> + case DATA_SIZE_32_BITS:
> + st->tx[2] = (val >> 24) & 0xFF;
> + st->tx[3] = (val >> 16) & 0xFF;
> + st->tx[4] = (val >> 8) & 0xFF;
> + st->tx[5] = val & 0xFF;
> + count = 6;
> + break;
> + default:
> + ret = -EINVAL;
> + goto error_i2c_write_unlock;
> + }
> +
> + ret = i2c_master_send(st->i2c, st->tx, count);
> +
> +error_i2c_write_unlock:

These labels are sort of long.  And what does the "i2c_write" really
mean?  It should be obvious that we're not jumping to a different
function.

Just "unlock:" is OK as a label name.

>   mutex_unlock(>buf_lock);
>  
>   return ret;
>  }
>  
> +static int ade7854_i2c_write_reg_8(struct device *dev,
> +u16 reg_address,
> +u8 val)
> +{
> + int ret;
> +
> + ret = ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS);
> +
> + return ret;
> +}

Just do it like this:

static int ade7854_i2c_write_reg_8(struct device *dev, u16 reg_address, u8 val)
{
return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_8_BITS);
}



> +
>  static int ade7854_i2c_write_reg_16(struct device *dev,
>   u16 reg_address,
>   u16 val)
>  {
>   int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
>  
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> - st->tx[2] = (val >> 8) & 0xFF;
> - st->tx[3] = val & 0xFF;
> -
> - ret = i2c_master_send(st->i2c, st->tx, 4);
> - mutex_unlock(>buf_lock);
> + ret = ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS);
>  
>   return ret;

Again:

return ade7854_i2c_write_reg(dev, reg_address, val, DATA_SIZE_16_BITS);



>  }
> @@ -59,18 +92,8 @@ static int ade7854_i2c_write_reg_24(struct device *dev,
>   u32 val)
>  {
>   int ret;
> - struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> - struct ade7854_state *st = iio_priv(indio_dev);
>  
> - mutex_lock(>buf_lock);
> - st->tx[0] = (reg_address >> 8) & 0xFF;
> - st->tx[1] = reg_address & 0xFF;
> - st->tx[2] = (val >> 16) & 0xFF;
> - st->tx[3] = (val >> 8) & 0xFF;
> - st->tx[4] = val & 0xFF;
> -
> - ret = i2c_master_send(st->i2c, st->tx, 5);
> - 

Re: [PATCH v3]PCI: hv: fix PCI-BUS domainID corruption

[Lorenzo Pieralisi]

On Thu, Mar 15, 2018 at 12:03:07AM +, Sridhar Pitchai wrote:
> Whenever PCI bus is added, HyperV guarantees the BUS id is unique. Even

"Whenever a PCI bus is added"

> with that when a first device is added to the bus, it overrides bus domain
> ID with the device serial number. Sometime this can result in BUS ID not

Define "Sometime".

> being unique. In this case, when PCI_BUS and a device to bus is added, the
> first device overwrites the bus domain ID to the device serial number,
> which is 0. Since there exsist a PCI bus with domain ID 0 already the PCI

s/exsist/exist

> bus addition fails. This patch make sure when a device is added to a bus,
> it never updated the bus domain ID. 

s/updated/updates

> Since we have the transparent SRIOV mode now, the short VF device name
> is no longer needed.

I still do not understand what this means and how it is related to the
patch below, it may be clear to you, it is not to me, at all.

> Fixes: 4a9b0933bdfc("PCI:hv:Use device serial number as PCI domain")

Fixes: 4a9b0933bdfc ("PCI: hv: Use device serial number as PCI domain")

I asked you an explicit question. Commit above was added for a reason
I assume. This patch implies that kernel has been broken since v4.11
which is almost a year ago and nobody every noticed ? Or there are
systems where commit above is _necessary_ and this patch would break
them ?

I want a detailed explanation that highlights *why* it is safe to apply
this patch and send it to stable kernels, commit log above won't do.

Thanks,
Lorenzo

> Cc: sta...@vger.kernel.org
> Signed-off-by: Sridhar Pitchai 
> ---
> 
> Changes in v3:
> * fix the commit comment. [KY Srinivasan, Michael Kelley]
> ---
>  drivers/pci/host/pci-hyperv.c | 11 ---
>  1 file changed, 11 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 2faf38e..ac67e56 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -1518,17 +1518,6 @@ static struct hv_pci_dev *new_pcichild_device(struct 
> hv_pcibus_device *hbus,
>   get_pcichild(hpdev, hv_pcidev_ref_childlist);
>   spin_lock_irqsave(>device_list_lock, flags);
>  
> - /*
> -  * When a device is being added to the bus, we set the PCI domain
> -  * number to be the device serial number, which is non-zero and
> -  * unique on the same VM.  The serial numbers start with 1, and
> -  * increase by 1 for each device.  So device names including this
> -  * can have shorter names than based on the bus instance UUID.
> -  * Only the first device serial number is used for domain, so the
> -  * domain number will not change after the first device is added.
> -  */
> - if (list_empty(>children))
> - hbus->sysdata.domain = desc->ser;
>   list_add_tail(>list_entry, >children);
>   spin_unlock_irqrestore(>device_list_lock, flags);
>   return hpdev;
> -- 
> 2.7.4 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1

[John Crispin]

On 15/03/18 11:48, Dan Carpenter wrote:

This all seems fine.  Generally the requirements for staging are that it
has a TODO, someone to work on it, and it doesn't break the build.  But
some of the patches don't have commit message and those are required and
some of the commit messages are just the changes you have made not don't
describe the actual code...

John Crispin's email is j...@phrozen.org.

regards,
dan carpenter


Hi All,

looks like i was CC'ed on the openwrt addr, which no longer exists. This 
series makes no sense. None of the stuff posted is anywhere near ready 
to be upstreamed.


* we dont need a dedicated pinctrl driver, pinctrl-single will work fine 
on these SoCs

* the DMA/sdhci driver is a hacked up version of the SDK driver.
* drivers/net/ethernet/mediatek/* works on mt7623 and is easily portable 
to mt7621, same goes for the gsw driver.


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

Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1

[Dan Carpenter]

On Thu, Mar 15, 2018 at 12:07:26PM +0100, John Crispin wrote:
>  None of the stuff posted is anywhere near ready to be
> upstreamed.

Yeah.  This is staging so we accept any quality...

The rest of the information is very useful.  Thanks!

regards,
dan carpenter

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

Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1

[Dan Carpenter]

On Thu, Mar 15, 2018 at 10:04:33PM +1100, NeilBrown wrote:
> On Thu, Mar 15 2018, Dan Carpenter wrote:
> 
> > This all seems fine.  Generally the requirements for staging are that it
> > has a TODO, someone to work on it, and it doesn't break the build.  But
> > some of the patches don't have commit message and those are required and
> > some of the commit messages are just the changes you have made not don't
> > describe the actual code...
> 
> Thanks for having a look.
> It seems odd to require detailed commit messages, when we don't require
> the same level of quality in the code.
> Naturally when the driver is moved out of staging a properly detailed
> commit message should be added, but is that needed on the way in to
> staging?  At this stage I don't know much more than is already there.
> After I've cleaned up the code I probably will.
> 
> For patch 01/13 you asked "what kind of device this is".  The subject
> line makes it clear that it is a "pcie driver".  What extra detail did
> you want?  Would it be sufficient to just copy the subject line so that
> it appears twice in the commit message?
> 

Ah...  Sorry.  It's literally a pcie driver.  For some reason I thought
it was a device that ran over pcie.

We don't require a detailed changelog, but you have to put something...
Probably just restating the subject and adding that it's for the gnubee1
is fine.

regards,
dan carpenter

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

Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1

[NeilBrown]

On Thu, Mar 15 2018, Dan Carpenter wrote:

> This all seems fine.  Generally the requirements for staging are that it
> has a TODO, someone to work on it, and it doesn't break the build.  But
> some of the patches don't have commit message and those are required and
> some of the commit messages are just the changes you have made not don't
> describe the actual code...

Thanks for having a look.
It seems odd to require detailed commit messages, when we don't require
the same level of quality in the code.
Naturally when the driver is moved out of staging a properly detailed
commit message should be added, but is that needed on the way in to
staging?  At this stage I don't know much more than is already there.
After I've cleaned up the code I probably will.

For patch 01/13 you asked "what kind of device this is".  The subject
line makes it clear that it is a "pcie driver".  What extra detail did
you want?  Would it be sufficient to just copy the subject line so that
it appears twice in the commit message?

>
> John Crispin's email is j...@phrozen.org.

Thanks.

NeilBrown

>
> regards,
> dan carpenter


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

Re: [PATCH 00/13] staging: add drivers to support Mediatek mt7621 in gnubee-pc1

[Dan Carpenter]

This all seems fine.  Generally the requirements for staging are that it
has a TODO, someone to work on it, and it doesn't break the build.  But
some of the patches don't have commit message and those are required and
some of the commit messages are just the changes you have made not don't
describe the actual code...

John Crispin's email is j...@phrozen.org.

regards,
dan carpenter

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

Re: [PATCH 01/13] staging: mt7621-pci: MIPS/ralink: add MT7621 pcie driver

[Dan Carpenter]

These patches need better changelogs.  Some of them don't have a
changelog at all.  Like just say this what kind of device this is or
something?

regards,
dan carpenter


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

[PATCH] staging: comedi: ni_mio_common: ack ai fifo error interrupts.

[Ian Abbott]

From: Frank Mori Hess 

Ack ai fifo error interrupts in interrupt handler to clear interrupt
after fifo overflow.  It should prevent lock-ups after the ai fifo
overflows.

Cc:  # v4.2+
Signed-off-by: Frank Mori Hess 
Signed-off-by: Ian Abbott 
---
I have not tested this patch on hardware. -- Frank Mori Hess

Applies cleanly to v4.2+, but needs backporting for earlier stable
kernels. -- Ian Abbott.
---
 drivers/staging/comedi/drivers/ni_mio_common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
b/drivers/staging/comedi/drivers/ni_mio_common.c
index d6eb55b41814..e40a2c0a9543 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -1275,6 +1275,8 @@ static void ack_a_interrupt(struct comedi_device *dev, 
unsigned short a_status)
ack |= NISTC_INTA_ACK_AI_START;
if (a_status & NISTC_AI_STATUS1_STOP)
ack |= NISTC_INTA_ACK_AI_STOP;
+   if (a_status & NISTC_AI_STATUS1_OVER)
+   ack |= NISTC_INTA_ACK_AI_ERR;
if (ack)
ni_stc_writew(dev, ack, NISTC_INTA_ACK_REG);
 }
-- 
2.16.1

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

[PATCH] staging: lustre: o2iblnd: Fix FastReg map/unmap for MLX5

[Doug Oucharek]

The FastReg support in ko2iblnd was not unmapping pool items
causing the items to leak.  In addition, the mapping code
is not growing the pool like we do with FMR.

This patch makes sure we are unmapping FastReg pool elements
when we are done with them.  It also makes sure the pool
will grow when we depleat the pool.

Signed-off-by: Doug Oucharek 
Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-9472
Reviewed-on: https://review.whamcloud.com/27015
Reviewed-by: Andrew Perepechko 
Reviewed-by: Dmitry Eremin 
Reviewed-by: James Simmons 
Reviewed-by: Oleg Drokin 
Signed-off-by: Doug Oucharek 
---
drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c|  2 +-
drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c | 12 
2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c 
b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
index 7ae2955..355c816 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd.c
@@ -1702,7 +1702,7 @@ int kiblnd_fmr_pool_map(struct kib_fmr_poolset *fps, 
struct kib_tx *tx,
return 0;
}
spin_unlock(>fps_lock);
-   rc = -EBUSY;
+   rc = -EAGAIN;
}

spin_lock(>fps_lock);
diff --git a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c 
b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
index 6690a6c..6ed779ef0 100644
--- a/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
+++ b/drivers/staging/lustre/lnet/klnds/o2iblnd/o2iblnd_cb.c
@@ -48,7 +48,7 @@ static int kiblnd_init_rdma(struct kib_conn *conn, struct 
kib_tx *tx, int type,
__u64 dstcookie);
static void kiblnd_queue_tx_locked(struct kib_tx *tx, struct kib_conn *conn);
static void kiblnd_queue_tx(struct kib_tx *tx, struct kib_conn *conn);
-static void kiblnd_unmap_tx(struct lnet_ni *ni, struct kib_tx *tx);
+static void kiblnd_unmap_tx(struct kib_tx *tx);
static void kiblnd_check_sends_locked(struct kib_conn *conn);

static void
@@ -66,7 +66,7 @@ static int kiblnd_init_rdma(struct kib_conn *conn, struct 
kib_tx *tx, int type,
LASSERT(!tx->tx_waiting); /* mustn't be awaiting peer 
response */
LASSERT(tx->tx_pool);

-   kiblnd_unmap_tx(ni, tx);
+   kiblnd_unmap_tx(tx);

/* tx may have up to 2 lnet msgs to finalise */
lntmsg[0] = tx->tx_lntmsg[0]; tx->tx_lntmsg[0] = NULL;
@@ -591,13 +591,9 @@ static int kiblnd_init_rdma(struct kib_conn *conn, struct 
kib_tx *tx, int type,
return 0;
}

-static void kiblnd_unmap_tx(struct lnet_ni *ni, struct kib_tx *tx)
+static void kiblnd_unmap_tx(struct kib_tx *tx)
{
-   struct kib_net *net = ni->ni_data;
-
-   LASSERT(net);
-
-   if (net->ibn_fmr_ps)
+   if (tx->fmr.fmr_pfmr || tx->fmr.fmr_frd)
kiblnd_fmr_pool_unmap(>fmr, tx->tx_status);

if (tx->tx_nfrags) {
-- 
1.8.3.1

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

Re: [PATCH v2 1/3] staging:iio:meter: Replaces IIO_DEV_ATTR_CH_OFF by IIO_DEVICE_ATTR

[John Syne]

Hi Jonathan,

I have been looking at the IIO ABI docs and if I understand correctly, the idea 
is to use consistent naming conventions? So for example, looking at the ADE7854 
datasheet, the naming matching the ADE7854 registers would be as follows:

{direction}_{type}_{index}_{modifier}_{info_mask}

AIGAIN  -   In_current_a_gain
AVGAIN  -   in_voltage_a_gain
BIGAIN  -   in_current_b_gain
BVGAIN  -   in_voltage_b_gain
—
How do we represent the rms and offset
AIRMSOS -   in_current_a_rmsoffset
AVRMSOS -   in_voltage_a_rmsoffset
—
Here I don’t understand how to represent both the phase and the 
active/reactive/apparent power components. Do we combine the phase and 
quadrature part like this
AVAGAIN -   in_power_a_gain /* apparent 
power */
—
AWGAIN  -   in_power_ai_gain/* 
active power */
—
AVARGAIN-   in_power_aq_gain/* 
reactive power */
—
Now here it becomes more complicated. Not sure how this gets handled. 
AFWATTOS-   in_power_a_active/fundamental/offset
—
AWATTHR -   in_energy_ai_accumulation
—
AVARHR  -   in_energy_aq_accumulation
—
IPEAK   -   in_current_peak
—

I’ll leave it there, because there are some even more complicated register 
naming issues.

Regards,
John





> On Mar 10, 2018, at 7:10 AM, Jonathan Cameron  wrote:
> 
> On Thu, 8 Mar 2018 21:37:33 -0300
> Rodrigo Siqueira  wrote:
> 
>> On 03/07, Jonathan Cameron wrote:
>>> On Tue, 6 Mar 2018 21:43:47 -0300
>>> Rodrigo Siqueira  wrote:
>>> 
 The macro IIO_DEV_ATTR_CH_OFF is a wrapper for IIO_DEVICE_ATTR, with a
 tiny change in the name definition. This extra macro does not improve
 the readability and also creates some checkpatch errors.
 
 This patch fixes the checkpatch.pl errors:
 
 staging/iio/meter/ade7753.c:391: ERROR: Use 4 digit octal (0777) not
 decimal permissions
 staging/iio/meter/ade7753.c:395: ERROR: Use 4 digit octal (0777) not
 decimal permissions
 staging/iio/meter/ade7759.c:331: ERROR: Use 4 digit octal (0777) not
 decimal permissions
 staging/iio/meter/ade7759.c:335: ERROR: Use 4 digit octal (0777) not
 decimal permissions
 
 Signed-off-by: Rodrigo Siqueira   
>>> 
>>> Hmm. I wondered a bit about this one. It's a correct patch in of
>>> itself but the interface in question doesn't even vaguely conform
>>> to any of defined IIO ABI.  Anyhow, it's still and improvement so
>>> I'll take it.  
>> 
>> I am not sure if I understood the comment about the ABI. The meter
>> interface is wrong because it uses things like IIO_DEVICE_ATTR? It
>> should use iio_info together with *write_raw and *read_raw. Right? Is it
>> the ABI problem that you refer?
> The ABI is about the userspace interface of IIO.  It is defined
> in Documentation/ABI/testing/sysfs-bus-iio*
> So this documents the naming of sysfs attributes and (more or less)
> describes a consistent interface to userspace across lots of different
> types of devices.
> 
> A lot of these older drivers in staging involve a good deal of ABI that
> was not reviewed or discussed.  That is one of the biggest reasons we
> didn't take them out of staging in the first place.
> 
> In order for generic userspace programs to have any idea what to do
> with these devices this all needs to be fixed.
> 
> There may well be cases where we need to expand the existing ABI to
> cover new things.   That's fine, but it has to be done with full
> review of the relevant documentation patches.
> 
> Incidentally if you want an easy driver to work on moving out of staging
> then first thing to do is to compare what it shows to userspace with these
> docs.  If it's totally different then you have a big job on your hands
> as often ABI can take a lot of discussion and a long time to establish
> a consensus.
> 
> Jonathan
> 
> 
>> 
>> Thanks :)
>> 
>>> Applied to the togreg branch of iio.git and pushed out as testing
>>> for the autobuilders to play with it.
>>> 
>>> I also added the removal of the header define which is no
>>> longer used.
>>> 
>>> Please note, following discussions with Michael, I am going to send
>>> an email announcing an intent to drop these meter drivers next
>>> cycle unless someone can provide testing for any attempt to
>>> move them out of staging.  I'm still taking patches on the basis
>>> that 'might' happen - but I wouldn't focus on these until we
>>> have some certainty on whether they will be around long term!
>>> 
>>> Jonathan
>>> 
 ---
 drivers/staging/iio/meter/ade7753.c | 18 ++
 drivers/staging/iio/meter/ade7759.c | 18 ++
 2 files changed, 20 insertions(+), 16 deletions(-)
 
 diff --git a/drivers/staging/iio/meter/ade7753.c