[PATCH 1/1] vmbus: make sysfs names consistent with PCI

2016-10-31 Thread kys
From: Stephen Hemminger 

In commit 9a56e5d6a0ba ("Drivers: hv: make VMBus bus ids persistent")
the name of vmbus devices in sysfs changed to be (in 4.9-rc1):
  /sys/bus/vmbus/vmbus-6aebe374-9ba0-11e6-933c-00259086b36b

The prefix ("vmbus-") is redundant and differs from how PCI is
represented in sysfs. Therefore simplify to:
  /sys/bus/vmbus/6aebe374-9ba0-11e6-933c-00259086b36b

Please merge this before 4.9 is released and the old format
has to live forever.

Signed-off-by: Stephen Hemminger 
Signed-off-by: K. Y. Srinivasan 
---
 drivers/hv/vmbus_drv.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index a259e18..0276d2e 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -961,7 +961,7 @@ int vmbus_device_register(struct hv_device 
*child_device_obj)
 {
int ret = 0;
 
-   dev_set_name(_device_obj->device, "vmbus-%pUl",
+   dev_set_name(_device_obj->device, "%pUl",
 child_device_obj->channel->offermsg.offer.if_instance.b);
 
child_device_obj->device.bus = _bus;
-- 
1.7.4.1

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


Re: [PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()

2016-10-31 Thread Matt Ranostay
On Mon, Oct 31, 2016 at 10:04 AM, Eva Rachel Retuya  wrote:
> The name passed to devm_regulator_get() should match the name of the
> supply as specified in the device datasheet. This makes it clear what
> power supply is being referred to in case of presence of other
> regulators.
>
> Currently, the supply name specified on the affected devices is 'vcc'.
> Use lowercase version of the datasheet name to specify the supply
> voltage.
>

Aren't we possibly breaking current device tree definitions that
people may have? We should still check the old name after the new
datasheet name in my opinion.

> Suggested-by: Lars-Peter Clausen 
> Signed-off-by: Eva Rachel Retuya 
> ---
>  drivers/staging/iio/adc/ad7192.c| 2 +-
>  drivers/staging/iio/adc/ad7780.c| 2 +-
>  drivers/staging/iio/frequency/ad9832.c  | 2 +-
>  drivers/staging/iio/frequency/ad9834.c  | 2 +-
>  drivers/staging/iio/impedance-analyzer/ad5933.c | 2 +-
>  5 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/iio/adc/ad7192.c 
> b/drivers/staging/iio/adc/ad7192.c
> index bfa12ce..41fb32d 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -633,7 +633,7 @@ static int ad7192_probe(struct spi_device *spi)
>
> st = iio_priv(indio_dev);
>
> -   st->reg = devm_regulator_get(>dev, "vcc");
> +   st->reg = devm_regulator_get(>dev, "avdd");
> if (!IS_ERR(st->reg)) {
> ret = regulator_enable(st->reg);
> if (ret)
> diff --git a/drivers/staging/iio/adc/ad7780.c 
> b/drivers/staging/iio/adc/ad7780.c
> index c9a0c2a..a88236e 100644
> --- a/drivers/staging/iio/adc/ad7780.c
> +++ b/drivers/staging/iio/adc/ad7780.c
> @@ -173,7 +173,7 @@ static int ad7780_probe(struct spi_device *spi)
>
> ad_sd_init(>sd, indio_dev, spi, _sigma_delta_info);
>
> -   st->reg = devm_regulator_get(>dev, "vcc");
> +   st->reg = devm_regulator_get(>dev, "avdd");
> if (!IS_ERR(st->reg)) {
> ret = regulator_enable(st->reg);
> if (ret)
> diff --git a/drivers/staging/iio/frequency/ad9832.c 
> b/drivers/staging/iio/frequency/ad9832.c
> index 358400b..744c8ee 100644
> --- a/drivers/staging/iio/frequency/ad9832.c
> +++ b/drivers/staging/iio/frequency/ad9832.c
> @@ -212,7 +212,7 @@ static int ad9832_probe(struct spi_device *spi)
> return -ENODEV;
> }
>
> -   reg = devm_regulator_get(>dev, "vcc");
> +   reg = devm_regulator_get(>dev, "avdd");
> if (!IS_ERR(reg)) {
> ret = regulator_enable(reg);
> if (ret)
> diff --git a/drivers/staging/iio/frequency/ad9834.c 
> b/drivers/staging/iio/frequency/ad9834.c
> index 6366216..ca3cea6 100644
> --- a/drivers/staging/iio/frequency/ad9834.c
> +++ b/drivers/staging/iio/frequency/ad9834.c
> @@ -329,7 +329,7 @@ static int ad9834_probe(struct spi_device *spi)
> return -ENODEV;
> }
>
> -   reg = devm_regulator_get(>dev, "vcc");
> +   reg = devm_regulator_get(>dev, "avdd");
> if (!IS_ERR(reg)) {
> ret = regulator_enable(reg);
> if (ret)
> diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c 
> b/drivers/staging/iio/impedance-analyzer/ad5933.c
> index 5eecf1c..62f61bc 100644
> --- a/drivers/staging/iio/impedance-analyzer/ad5933.c
> +++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
> @@ -723,7 +723,7 @@ static int ad5933_probe(struct i2c_client *client,
> if (!pdata)
> pdata = _default_pdata;
>
> -   st->reg = devm_regulator_get(>dev, "vcc");
> +   st->reg = devm_regulator_get(>dev, "vdd");
> if (!IS_ERR(st->reg)) {
> ret = regulator_enable(st->reg);
> if (ret)
> --
> 2.7.4
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: sm750fb: Replace pr_*() with dev_*().

2016-10-31 Thread Elise Lennion
dev_*() functions print identifying information about the struct device
and should be used instead of pr_*() whenever possible.

Signed-off-by: Elise Lennion 
---
 drivers/staging/sm750fb/sm750.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 0663ec0..5c153d6 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -947,13 +947,13 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, 
char *src)
g_hwcursor = 3;
 
if (!src || !*src) {
-   pr_warn("no specific g_option.\n");
+   dev_warn(_dev->pdev->dev, "no specific g_option.\n");
goto NO_PARAM;
}
 
while ((opt = strsep(, ":")) != NULL && *opt != 0) {
-   pr_info("opt=%s\n", opt);
-   pr_info("src=%s\n", src);
+   dev_info(_dev->pdev->dev, "opt=%s\n", opt);
+   dev_info(_dev->pdev->dev, "src=%s\n", src);
 
if (!strncmp(opt, "swap", strlen("swap")))
swap = 1;
@@ -974,12 +974,12 @@ static void sm750fb_setup(struct sm750_dev *sm750_dev, 
char *src)
else {
if (!g_fbmode[0]) {
g_fbmode[0] = opt;
-   pr_info("find fbmode0 : %s\n", g_fbmode[0]);
+   dev_info(_dev->pdev->dev, "find fbmode0 : 
%s\n", g_fbmode[0]);
} else if (!g_fbmode[1]) {
g_fbmode[1] = opt;
-   pr_info("find fbmode1 : %s\n", g_fbmode[1]);
+   dev_info(_dev->pdev->dev, "find fbmode1 : 
%s\n", g_fbmode[1]);
} else {
-   pr_warn("How many view you wann set?\n");
+   dev_warn(_dev->pdev->dev, "How many view 
you wann set?\n");
}
}
}
-- 
2.7.4

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


[PATCH v2] staging: vc04_services: setup DMA and coherent mask

2016-10-31 Thread Michael Zoran
VCHI messages between the CPU and firmware use 32-bit
bus addresses. Explicitly set the DMA mask and coherent
on all platforms.

Signed-off-by: Michael Zoran 
---
 .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c   | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index a5afcc5..ba77fd8 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -97,6 +97,15 @@ int vchiq_platform_init(struct platform_device *pdev, 
VCHIQ_STATE_T *state)
int slot_mem_size, frag_mem_size;
int err, irq, i;
 
+   /*
+* VCHI messages between the CPU and firmware use
+* 32-bit bus addresses.
+*/
+   err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+
+   if (err < 0)
+   return err;
+
(void)of_property_read_u32(dev->of_node, "cache-line-size",
   _cache_line_size);
g_fragments_size = 2 * g_cache_line_size;
-- 
2.10.1

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


Re: [PATCH] staging: vc04_services: setup DMA and coherent mask

2016-10-31 Thread Michael Zoran
On Mon, 2016-10-31 at 12:53 -0700, Michael Zoran wrote:
> On Mon, 2016-10-31 at 11:40 -0700, Michael Zoran wrote:
> > On Mon, 2016-10-31 at 11:36 -0700, Eric Anholt wrote:
> > > Michael Zoran  writes:
> > > 
> > > > Setting the DMA mask is optional on 32 bit but
> > > > is mandatory on 64 bit.  Set the DMA mask and coherent
> > > > to force all DMA to be in the 32 bit address space.
> > > > 
> > > > This is considered a "good practice" and most drivers
> > > > already do this.
> > > > 
> > > > Signed-off-by: Michael Zoran 
> > > > ---
> > > >  .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> > > > |
> > > > 10 ++
> > > >  1 file changed, 10 insertions(+)
> > > > 
> > > > diff --git
> > > > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_
> > > > ar
> > > > m.
> > > > c
> > > > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_
> > > > ar
> > > > m.
> > > > c
> > > > index a5afcc5..6fa2b5a 100644
> > > > ---
> > > > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_
> > > > ar
> > > > m.
> > > > c
> > > > +++
> > > > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_
> > > > ar
> > > > m.
> > > > c
> > > > @@ -97,6 +97,16 @@ int vchiq_platform_init(struct
> > > > platform_device
> > > > *pdev, VCHIQ_STATE_T *state)
> > > >     int slot_mem_size, frag_mem_size;
> > > >     int err, irq, i;
> > > >  
> > > > +   /*
> > > > +    * Setting the DMA mask is necessary in the 64 bit
> > > > environment.
> > > > +    * It isn't necessary in a 32 bit environment but is
> > > > considered
> > > > +    * a good practice.
> > > > +    */
> > > > +   err = dma_set_mask_and_coherent(dev,
> > > > DMA_BIT_MASK(32));
> > > 
> > > I think a better comment here would be simply:
> > > 
> > > /* VCHI messages between the CPU and firmware use 32-bit bus
> > > addresses. */
> > > 
> > > explaining why the value is chosen (once you know that the 32 bit
> > > restriction exists, reporting it is obviously needed).  I'm
> > > curious,
> > > though: what failed when you didn't set it?
> > > 
> > 
> > The comment is easy to change.
> > 
> > I don't have the log available ATM, but if I remember the DMA API's
> > bugcheck the first time that are used.  
> > 
> > I think this was a policy decision or something because the
> > information
> > should be available in the dma-ranges.
> > 
> > If it's important, I can setup a test again without the change and
> > e-
> > mail the logs.
> > 
> > If you look at the DWC2 driver you will see that it also sets this
> > mask.
> 
> OK, I'm begging to understand this.  It appears the architecture
> specific paths are very different.
> 
> In arm the mask and coherent is set to DMA_BIT_MASK(32) in mm/dma-
> mapping.c the first time the dma APIs are used.  On arm64, it appears
> this variable is uninitialized and will contain random crude.
> 
> Like I said, I don't know if this is a policy decision or if it just
> slipped through the cracks.
> 
Actually, I'm getting confused here.   If I need to prove this is
needed, is their anybody I can send e-mail to that has a deep
understanding of the two different architecture paths.   Perhaps they
can explain exactly why arm64 is not defaulting to 32 bit DMA.

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


Re: [PATCH] staging: vc04_services: setup DMA and coherent mask

2016-10-31 Thread Michael Zoran
On Mon, 2016-10-31 at 11:40 -0700, Michael Zoran wrote:
> On Mon, 2016-10-31 at 11:36 -0700, Eric Anholt wrote:
> > Michael Zoran  writes:
> > 
> > > Setting the DMA mask is optional on 32 bit but
> > > is mandatory on 64 bit.  Set the DMA mask and coherent
> > > to force all DMA to be in the 32 bit address space.
> > > 
> > > This is considered a "good practice" and most drivers
> > > already do this.
> > > 
> > > Signed-off-by: Michael Zoran 
> > > ---
> > >  .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c |
> > > 10 ++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git
> > > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_ar
> > > m.
> > > c
> > > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_ar
> > > m.
> > > c
> > > index a5afcc5..6fa2b5a 100644
> > > ---
> > > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_ar
> > > m.
> > > c
> > > +++
> > > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_ar
> > > m.
> > > c
> > > @@ -97,6 +97,16 @@ int vchiq_platform_init(struct platform_device
> > > *pdev, VCHIQ_STATE_T *state)
> > >   int slot_mem_size, frag_mem_size;
> > >   int err, irq, i;
> > >  
> > > + /*
> > > +  * Setting the DMA mask is necessary in the 64 bit
> > > environment.
> > > +  * It isn't necessary in a 32 bit environment but is
> > > considered
> > > +  * a good practice.
> > > +  */
> > > + err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> > 
> > I think a better comment here would be simply:
> > 
> > /* VCHI messages between the CPU and firmware use 32-bit bus
> > addresses. */
> > 
> > explaining why the value is chosen (once you know that the 32 bit
> > restriction exists, reporting it is obviously needed).  I'm
> > curious,
> > though: what failed when you didn't set it?
> > 
> 
> The comment is easy to change.
> 
> I don't have the log available ATM, but if I remember the DMA API's
> bugcheck the first time that are used.  
> 
> I think this was a policy decision or something because the
> information
> should be available in the dma-ranges.
> 
> If it's important, I can setup a test again without the change and e-
> mail the logs.
> 
> If you look at the DWC2 driver you will see that it also sets this
> mask.

OK, I'm begging to understand this.  It appears the architecture
specific paths are very different.

In arm the mask and coherent is set to DMA_BIT_MASK(32) in mm/dma-
mapping.c the first time the dma APIs are used.  On arm64, it appears
this variable is uninitialized and will contain random crude.

Like I said, I don't know if this is a policy decision or if it just
slipped through the cracks.

arch/arm/mm/dma-mapping.c(Note the call to get_coherent_dma_mask(dev))

static u64 get_coherent_dma_mask(struct device *dev)
{
u64 mask = (u64)DMA_BIT_MASK(32);

if (dev) {
mask = dev->coherent_dma_mask;

/*
 * Sanity check the DMA mask - it must be non-zero, and
 * must be able to be satisfied by a DMA allocation.
 */
if (mask == 0) {
dev_warn(dev, "coherent DMA mask is unset\n");
return 0;
}

if (!__dma_supported(dev, mask, true))
return 0;
}

return mask;
}

static void *__dma_alloc(struct device *dev, size_t size, dma_addr_t
*handle,
 gfp_t gfp, pgprot_t prot, bool is_coherent,
 unsigned long attrs, const void *caller)
{
u64 mask = get_coherent_dma_mask(dev);
struct page *page = NULL;
void *addr;
bool allowblock, cma;
struct arm_dma_buffer *buf;
struct arm_dma_alloc_args args = {
.dev = dev,
.size = PAGE_ALIGN(size),
.gfp = gfp,
.prot = prot,
.caller = caller,
.want_vaddr = ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) ==
0),
.coherent_flag = is_coherent ? COHERENT : NORMAL,
};

arch/arm64/include/asm/dma-mapping.h

/* do not use this function in a driver */
static inline bool is_device_dma_coherent(struct device *dev)
{
if (!dev)
return false;
return dev->archdata.dma_coherent;
}

arch/arm64/mm/dma-mapping.c(Note no call to get_coherent_dma_mask)

static void *__dma_alloc(struct device *dev, size_t size,
 dma_addr_t *dma_handle, gfp_t flags,
 unsigned long attrs)
{
struct page *page;
void *ptr, *coherent_ptr;
bool coherent = is_device_dma_coherent(dev);
pgprot_t prot = __get_dma_pgprot(attrs, PAGE_KERNEL, false);

size = PAGE_ALIGN(size);

if (!coherent && !gfpflags_allow_blocking(flags)) {
struct page *page = NULL;
void *addr = 

Re: [PATCH 2/2] mm: remove get_user_pages_locked()

2016-10-31 Thread Lorenzo Stoakes
On Mon, Oct 31, 2016 at 06:55:33PM +0100, Paolo Bonzini wrote:
> > 2. There is currently only one caller of get_user_pages_locked() in
> >mm/frame_vector.c which seems to suggest this function isn't widely
> >used/known.
>
> Or not widely necessary. :)

Well, quite :)
>
> > 3. This change results in all slow-path get_user_pages*() functions having 
> > the
> >ability to use VM_FAULT_RETRY logic rather than 'defaulting' to using
> >get_user_pages() that doesn't let you do this even if you wanted to.
>
> This is only true if someone does the work though.  From a quick look
> at your series, the following files can be trivially changed to use
> get_user_pages_unlocked:
>
> - drivers/gpu/drm/via/via_dmablit.c
> - drivers/platform/goldfish/goldfish_pipe.c
> - drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> - drivers/rapidio/devices/rio_mport_cdev.c
> - drivers/virt/fsl_hypervisor.c
>

Ah indeed, I rather glossed through the callers and noticed that at least some
held locks long enough or were called with a lock held sufficient that I wasn't
sure it could be released.

> Also, videobuf_dma_init_user could be changed to use retry by adding a
> *locked argument to videobuf_dma_init_user_locked, prototype patch
> after my signature.
>

Very nice!

> Everything else is probably best kept using get_user_pages.
>
> > 4. It's somewhat confusing/redundant having both get_user_pages_locked() and
> >get_user_pages() functions which both require mmap_sem to be held (i.e. 
> > both
> >are 'locked' versions.)
> >
> >> If all callers were changed, then sure removing the _locked suffix would
> >> be a good idea.
> >
> > It seems many callers cannot release mmap_sem so VM_FAULT_RETRY logic 
> > couldn't
> > happen anyway in this cases and in these cases we couldn't change the 
> > caller.
>
> But then get_user_pages_locked remains a less-common case, so perhaps
> it's a good thing to give it a longer name!

My (somewhat minor) concern was that there would be confusion due to the
existence of the triumvirate of g_u_p()/g_u_p_unlocked()/g_u_p_locked(), however
the comments do a decent enough job of explaining the situation.

>
> > Overall, an alternative here might be to remove get_user_pages() and update
> > get_user_pages_locked() to add a 'vmas' parameter, however doing this 
> > renders
> > get_user_pages_unlocked() asymmetric as it would lack an vmas parameter 
> > (adding
> > one there would make no sense as VM_FAULT_RETRY logic invalidates VMAs) 
> > though
> > perhaps not such a big issue as it makes sense as to why this is the case.
>
> Adding the 'vmas' parameter to get_user_pages_locked would make little
> sense.  Since VM_FAULT_RETRY invalidates it and g_u_p_locked can and
> does retry, it would generally not be useful.

I meant only in the case where we'd remove get_user_pages() and instead be left
with get_user_pages_[un]locked() only, meaning we'd have to add some means of
retrieving vmas if locked was set NULL, of course in cases where locked is not
NULL it makes no sense to add it.

>
> So I think we have the right API now:
>
> - do not have lock?  Use get_user_pages_unlocked, get retry for free,
> no need to handle  mmap_sem and the locked argument; cannot get back vmas.
>
> - have and cannot drop lock?  User get_user_pages, no need to pass NULL
> for the locked argument; can get back vmas.
>
> - have but can drop lock (rare case)?  Use get_user_pages_locked,
> cannot get back vams.

Yeah I think this is sane as it is actually, this patch set was a lot less
convincing of a cleanup than prior ones and overall it seems we are better off
with the existing API.

I wonder whether a better patch series to come out of this would be to find
cases where the lock could be dropped (i.e. the ones you mention above) and to
switch to using get_user_pages_[un]locked() where it makes sense to.

I am happy to look into these cases (though of course I must leave your
suggested patch here to you :)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: vc04_services: setup DMA and coherent mask

2016-10-31 Thread Michael Zoran
On Mon, 2016-10-31 at 11:36 -0700, Eric Anholt wrote:
> Michael Zoran  writes:
> 
> > Setting the DMA mask is optional on 32 bit but
> > is mandatory on 64 bit.  Set the DMA mask and coherent
> > to force all DMA to be in the 32 bit address space.
> > 
> > This is considered a "good practice" and most drivers
> > already do this.
> > 
> > Signed-off-by: Michael Zoran 
> > ---
> >  .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c |
> > 10 ++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git
> > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.
> > c
> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.
> > c
> > index a5afcc5..6fa2b5a 100644
> > ---
> > a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.
> > c
> > +++
> > b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.
> > c
> > @@ -97,6 +97,16 @@ int vchiq_platform_init(struct platform_device
> > *pdev, VCHIQ_STATE_T *state)
> >     int slot_mem_size, frag_mem_size;
> >     int err, irq, i;
> >  
> > +   /*
> > +    * Setting the DMA mask is necessary in the 64 bit
> > environment.
> > +    * It isn't necessary in a 32 bit environment but is
> > considered
> > +    * a good practice.
> > +    */
> > +   err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> 
> I think a better comment here would be simply:
> 
> /* VCHI messages between the CPU and firmware use 32-bit bus
> addresses. */
> 
> explaining why the value is chosen (once you know that the 32 bit
> restriction exists, reporting it is obviously needed).  I'm curious,
> though: what failed when you didn't set it?
> 

The comment is easy to change.

I don't have the log available ATM, but if I remember the DMA API's
bugcheck the first time that are used.  

I think this was a policy decision or something because the information
should be available in the dma-ranges.

If it's important, I can setup a test again without the change and e-
mail the logs.

If you look at the DWC2 driver you will see that it also sets this
mask.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: vc04_services: setup DMA and coherent mask

2016-10-31 Thread Eric Anholt
Michael Zoran  writes:

> Setting the DMA mask is optional on 32 bit but
> is mandatory on 64 bit.  Set the DMA mask and coherent
> to force all DMA to be in the 32 bit address space.
>
> This is considered a "good practice" and most drivers
> already do this.
>
> Signed-off-by: Michael Zoran 
> ---
>  .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 10 
> ++
>  1 file changed, 10 insertions(+)
>
> diff --git 
> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index a5afcc5..6fa2b5a 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -97,6 +97,16 @@ int vchiq_platform_init(struct platform_device *pdev, 
> VCHIQ_STATE_T *state)
>   int slot_mem_size, frag_mem_size;
>   int err, irq, i;
>  
> + /*
> +  * Setting the DMA mask is necessary in the 64 bit environment.
> +  * It isn't necessary in a 32 bit environment but is considered
> +  * a good practice.
> +  */
> + err = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));

I think a better comment here would be simply:

/* VCHI messages between the CPU and firmware use 32-bit bus addresses. */

explaining why the value is chosen (once you know that the 32 bit
restriction exists, reporting it is obviously needed).  I'm curious,
though: what failed when you didn't set it?

> +
> + if (err < 0)
> + return err;
> +
>   (void)of_property_read_u32(dev->of_node, "cache-line-size",
>  _cache_line_size);
>   g_fragments_size = 2 * g_cache_line_size;
> -- 
> 2.10.1


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


Re: [PATCH] staging: vc04_services: call sg_init_table to init scatterlist

2016-10-31 Thread Eric Anholt
Michael Zoran  writes:

> Call the sg_init_table function to correctly initialze
> the DMA scatterlist.  This function is required to completely
> initialize the list and is mandatory if DMA debugging is
> enabled in the build configuration.
>
> One of the purposes of sg_init_table is to set
> the magic "cookie" on each list element and ensure
> the chain end is marked.
>
> Signed-off-by: Michael Zoran 
> ---
>  drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git 
> a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> index 6fa2b5a..21b26e5 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
> @@ -464,6 +464,12 @@ create_pagelist(char __user *buf, size_t count, unsigned 
> short type,
>   pagelist->type = type;
>   pagelist->offset = offset;
>  
> + /*
> +  * Initialize the scatterlist so that the magic cookie
> +  *  is filled if debugging is enabled
> +  */
> + sg_init_table(scatterlist, num_pages);
> + /* Now set the pages for each scatterlist */

I feel like the comments don't add much, but either way:

Acked-by: Eric Anholt 

>   for (i = 0; i < num_pages; i++)
>   sg_set_page(scatterlist + i, pages[i], PAGE_SIZE, 0);
>  
> -- 
> 2.10.1


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


Re: [PATCH] PCI: hv: Make unnecessarily global IRQ masking functions static

2016-10-31 Thread Bjorn Helgaas
On Mon, Oct 31, 2016 at 12:04:09PM +0100, Tobias Klauser wrote:
> Make hv_irq_mask and hv_irq_unmask static as they are only used in
> pci-hyperv.c
> 
> This fixes a sparse warning.
> 
> Signed-off-by: Tobias Klauser 

Applied with KY's ack to pci/host-hv for v4.10, thanks!

> ---
>  drivers/pci/host/pci-hyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 763ff8745828..06c98695c06c 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -755,7 +755,7 @@ static int hv_set_affinity(struct irq_data *data, const 
> struct cpumask *dest,
>   return parent->chip->irq_set_affinity(parent, dest, force);
>  }
>  
> -void hv_irq_mask(struct irq_data *data)
> +static void hv_irq_mask(struct irq_data *data)
>  {
>   pci_msi_mask_irq(data);
>  }
> @@ -770,7 +770,7 @@ void hv_irq_mask(struct irq_data *data)
>   * is built out of this PCI bus's instance GUID and the function
>   * number of the device.
>   */
> -void hv_irq_unmask(struct irq_data *data)
> +static void hv_irq_unmask(struct irq_data *data)
>  {
>   struct msi_desc *msi_desc = irq_data_get_msi_desc(data);
>   struct irq_cfg *cfg = irqd_cfg(data);
> -- 
> 2.9.0
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/2] mm: remove get_user_pages_locked()

2016-10-31 Thread Paolo Bonzini


On 31/10/2016 14:48, Lorenzo Stoakes wrote:
> On Mon, Oct 31, 2016 at 12:45:36PM +0100, Paolo Bonzini wrote:
>>
>>
>> On 31/10/2016 11:02, Lorenzo Stoakes wrote:
>>> - *
>>> - * get_user_pages should be phased out in favor of
>>> - * get_user_pages_locked|unlocked or get_user_pages_fast. Nothing
>>> - * should use get_user_pages because it cannot pass
>>> - * FAULT_FLAG_ALLOW_RETRY to handle_mm_fault.
>>
>> This comment should be preserved in some way.  In addition, removing
> 
> Could you clarify what you think should be retained?
> 
> The comment seems to me to be largely rendered redundant by this change -
> get_user_pages() now offers identical behaviour, and of course the latter part
> of the comment ('because it cannot pass FAULT_FLAG_ALLOW_RETRY') is rendered
> incorrect by this change.
> 
> It could be replaced with a recommendation to make use of VM_FAULT_RETRY logic
> if possible.

Yes, exactly.  locked == NULL should be avoided whenever mmap_sem can 
be dropped, and the comment indirectly said so.  Now most of those cases
actually are those where you can just call get_user_pages_unlocked.

>> get_user_pages_locked() makes it harder (compared to a simple "git grep
>> -w") to identify callers that lack allow-retry functionality).  So I'm
>> not sure about the benefits of these patches.
> 
> That's a fair point, and certainly this cleanup series is less obviously 
> helpful
> than previous ones.
> 
> However, there are a few points in its favour:
> 
> 1. get_user_pages_remote() was the mirror of get_user_pages() prior to adding 
> an
>int *locked parameter to the former (necessary to allow for the unexport of
>__get_user_pages_unlocked()), differing only in task/mm being specified and
>FOLL_REMOTE being set. This patch series keeps these functions 
> 'synchronised'
>in this respect.
> 
> 2. There is currently only one caller of get_user_pages_locked() in
>mm/frame_vector.c which seems to suggest this function isn't widely
>used/known.

Or not widely necessary. :)

> 3. This change results in all slow-path get_user_pages*() functions having the
>ability to use VM_FAULT_RETRY logic rather than 'defaulting' to using
>get_user_pages() that doesn't let you do this even if you wanted to.

This is only true if someone does the work though.  From a quick look 
at your series, the following files can be trivially changed to use 
get_user_pages_unlocked:

- drivers/gpu/drm/via/via_dmablit.c
- drivers/platform/goldfish/goldfish_pipe.c
- drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
- drivers/rapidio/devices/rio_mport_cdev.c
- drivers/virt/fsl_hypervisor.c

Also, videobuf_dma_init_user could be changed to use retry by adding a 
*locked argument to videobuf_dma_init_user_locked, prototype patch 
after my signature.

Everything else is probably best kept using get_user_pages.

> 4. It's somewhat confusing/redundant having both get_user_pages_locked() and
>get_user_pages() functions which both require mmap_sem to be held (i.e. 
> both
>are 'locked' versions.)
> 
>> If all callers were changed, then sure removing the _locked suffix would
>> be a good idea.
> 
> It seems many callers cannot release mmap_sem so VM_FAULT_RETRY logic couldn't
> happen anyway in this cases and in these cases we couldn't change the caller.

But then get_user_pages_locked remains a less-common case, so perhaps 
it's a good thing to give it a longer name!

> Overall, an alternative here might be to remove get_user_pages() and update
> get_user_pages_locked() to add a 'vmas' parameter, however doing this renders
> get_user_pages_unlocked() asymmetric as it would lack an vmas parameter 
> (adding
> one there would make no sense as VM_FAULT_RETRY logic invalidates VMAs) though
> perhaps not such a big issue as it makes sense as to why this is the case.

Adding the 'vmas' parameter to get_user_pages_locked would make little 
sense.  Since VM_FAULT_RETRY invalidates it and g_u_p_locked can and 
does retry, it would generally not be useful.

So I think we have the right API now:

- do not have lock?  Use get_user_pages_unlocked, get retry for free,
no need to handle  mmap_sem and the locked argument; cannot get back vmas.

- have and cannot drop lock?  User get_user_pages, no need to pass NULL 
for the locked argument; can get back vmas.

- have but can drop lock (rare case)?  Use get_user_pages_locked, 
cannot get back vams.

Paolo

> get_user_pages_unlocked() definitely seems to be a useful helper and therefore
> makes sense to retain.

> Of course another alternative is to leave things be :)
> 

diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c 
b/drivers/media/v4l2-core/videobuf-dma-sg.c
index 1db0af6c7f94..dae4eb8e9d5b 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -152,7 +152,8 @@ static void videobuf_dma_init(struct videobuf_dmabuf *dma)
 }
 
 static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
-

Re: [PATCH 2/3] staging: most: hdm-usb: do h/w specific synchronization at configuration time

2016-10-31 Thread Dan Carpenter
Yes yes.  I wasn't commenting on the patch, which seems fine.

regards,
dan carpenter

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


[PATCH 4/6] staging: iio: ad7192: rename regulator 'reg' to 'avdd'

2016-10-31 Thread Eva Rachel Retuya
Rename regulator 'reg' to 'avdd' so as to be clear what regulator it
stands for specifically. Also, update the goto label accordingly.

Signed-off-by: Eva Rachel Retuya 
---
 drivers/staging/iio/adc/ad7192.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 3f8dc66..1fb68c0 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -152,7 +152,7 @@
  */
 
 struct ad7192_state {
-   struct regulator*reg;
+   struct regulator*avdd;
struct regulator*dvdd;
u16 int_vref_mv;
u32 mclk;
@@ -634,11 +634,11 @@ static int ad7192_probe(struct spi_device *spi)
 
st = iio_priv(indio_dev);
 
-   st->reg = devm_regulator_get(>dev, "avdd");
-   if (IS_ERR(st->reg))
-   return PTR_ERR(st->reg);
+   st->avdd = devm_regulator_get(>dev, "avdd");
+   if (IS_ERR(st->avdd))
+   return PTR_ERR(st->avdd);
 
-   ret = regulator_enable(st->reg);
+   ret = regulator_enable(st->avdd);
if (ret) {
dev_err(>dev, "Failed to enable specified AVdd supply\n");
return ret;
@@ -647,16 +647,16 @@ static int ad7192_probe(struct spi_device *spi)
st->dvdd = devm_regulator_get(>dev, "dvdd");
if (IS_ERR(st->dvdd)) {
ret = PTR_ERR(st->dvdd);
-   goto error_disable_reg;
+   goto error_disable_avdd;
}
 
ret = regulator_enable(st->dvdd);
if (ret) {
dev_err(>dev, "Failed to enable specified DVdd supply\n");
-   goto error_disable_reg;
+   goto error_disable_avdd;
}
 
-   voltage_uv = regulator_get_voltage(st->reg);
+   voltage_uv = regulator_get_voltage(st->avdd);
 
if (pdata->vref_mv)
st->int_vref_mv = pdata->vref_mv;
@@ -706,8 +706,8 @@ static int ad7192_probe(struct spi_device *spi)
ad_sd_cleanup_buffer_and_trigger(indio_dev);
 error_disable_dvdd:
regulator_disable(st->dvdd);
-error_disable_reg:
-   regulator_disable(st->reg);
+error_disable_avdd:
+   regulator_disable(st->avdd);
 
return ret;
 }
@@ -721,7 +721,7 @@ static int ad7192_remove(struct spi_device *spi)
ad_sd_cleanup_buffer_and_trigger(indio_dev);
 
regulator_disable(st->dvdd);
-   regulator_disable(st->reg);
+   regulator_disable(st->avdd);
 
return 0;
 }
-- 
2.7.4

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


[PATCH 6/6] staging: iio: ad9832: clean-up regulator 'reg'

2016-10-31 Thread Eva Rachel Retuya
Rename regulator 'reg' to 'avdd' so as to be clear what regulator it
stands for specifically. Additionally, get rid of local variable 'reg'
and use direct assignment instead. Update also the goto label pertaining
to the avdd regulator during disable.

Signed-off-by: Eva Rachel Retuya 
---
 drivers/staging/iio/frequency/ad9832.c | 20 +---
 drivers/staging/iio/frequency/ad9832.h |  4 ++--
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c 
b/drivers/staging/iio/frequency/ad9832.c
index 6a5ab02..639047f 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -204,7 +204,6 @@ static int ad9832_probe(struct spi_device *spi)
struct ad9832_platform_data *pdata = dev_get_platdata(>dev);
struct iio_dev *indio_dev;
struct ad9832_state *st;
-   struct regulator *reg;
int ret;
 
if (!pdata) {
@@ -212,11 +211,11 @@ static int ad9832_probe(struct spi_device *spi)
return -ENODEV;
}
 
-   reg = devm_regulator_get(>dev, "avdd");
-   if (IS_ERR(reg))
-   return PTR_ERR(reg);
+   st->avdd = devm_regulator_get(>dev, "avdd");
+   if (IS_ERR(st->avdd))
+   return PTR_ERR(st->avdd);
 
-   ret = regulator_enable(reg);
+   ret = regulator_enable(st->avdd);
if (ret) {
dev_err(>dev, "Failed to enable specified AVDD supply\n");
return ret;
@@ -225,13 +224,13 @@ static int ad9832_probe(struct spi_device *spi)
st->dvdd = devm_regulator_get(>dev, "dvdd");
if (IS_ERR(st->dvdd)) {
ret = PTR_ERR(st->dvdd);
-   goto error_disable_reg;
+   goto error_disable_avdd;
}
 
ret = regulator_enable(st->dvdd);
if (ret) {
dev_err(>dev, "Failed to enable specified DVDD supply\n");
-   goto error_disable_reg;
+   goto error_disable_avdd;
}
 
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
@@ -241,7 +240,6 @@ static int ad9832_probe(struct spi_device *spi)
}
spi_set_drvdata(spi, indio_dev);
st = iio_priv(indio_dev);
-   st->reg = reg;
st->mclk = pdata->mclk;
st->spi = spi;
 
@@ -327,8 +325,8 @@ static int ad9832_probe(struct spi_device *spi)
 
 error_disable_dvdd:
regulator_disable(st->dvdd);
-error_disable_reg:
-   regulator_disable(reg);
+error_disable_avdd:
+   regulator_disable(st->avdd);
 
return ret;
 }
@@ -340,7 +338,7 @@ static int ad9832_remove(struct spi_device *spi)
 
iio_device_unregister(indio_dev);
regulator_disable(st->dvdd);
-   regulator_disable(st->reg);
+   regulator_disable(st->avdd);
 
return 0;
 }
diff --git a/drivers/staging/iio/frequency/ad9832.h 
b/drivers/staging/iio/frequency/ad9832.h
index eb0e7f2..1b08b04 100644
--- a/drivers/staging/iio/frequency/ad9832.h
+++ b/drivers/staging/iio/frequency/ad9832.h
@@ -58,7 +58,7 @@
 /**
  * struct ad9832_state - driver instance specific data
  * @spi:   spi_device
- * @reg:   supply regulator
+ * @avdd:  supply regulator for the analog section
  * @dvdd:  supply regulator for the digital section
  * @mclk:  external master clock
  * @ctrl_fp:   cached frequency/phase control word
@@ -77,7 +77,7 @@
 
 struct ad9832_state {
struct spi_device   *spi;
-   struct regulator*reg;
+   struct regulator*avdd;
struct regulator*dvdd;
unsigned long   mclk;
unsigned short  ctrl_fp;
-- 
2.7.4

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


[PATCH 5/6] staging: iio: ad9832: add DVDD regulator

2016-10-31 Thread Eva Rachel Retuya
The AD9832/AD9835 is supplied with two power sources: AVDD as analog
supply voltage and DVDD as digital supply voltage.

Attempt to fetch and enable the regulator 'dvdd'. Bail out if any error
occurs.

Suggested-by: Lars-Peter Clausen 
Signed-off-by: Eva Rachel Retuya 
---
 drivers/staging/iio/frequency/ad9832.c | 33 -
 drivers/staging/iio/frequency/ad9832.h |  2 ++
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/iio/frequency/ad9832.c 
b/drivers/staging/iio/frequency/ad9832.c
index 7d8dc6c..6a5ab02 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -222,10 +222,22 @@ static int ad9832_probe(struct spi_device *spi)
return ret;
}
 
+   st->dvdd = devm_regulator_get(>dev, "dvdd");
+   if (IS_ERR(st->dvdd)) {
+   ret = PTR_ERR(st->dvdd);
+   goto error_disable_reg;
+   }
+
+   ret = regulator_enable(st->dvdd);
+   if (ret) {
+   dev_err(>dev, "Failed to enable specified DVDD supply\n");
+   goto error_disable_reg;
+   }
+
indio_dev = devm_iio_device_alloc(>dev, sizeof(*st));
if (!indio_dev) {
ret = -ENOMEM;
-   goto error_disable_reg;
+   goto error_disable_dvdd;
}
spi_set_drvdata(spi, indio_dev);
st = iio_priv(indio_dev);
@@ -280,39 +292,41 @@ static int ad9832_probe(struct spi_device *spi)
ret = spi_sync(st->spi, >msg);
if (ret) {
dev_err(>dev, "device init failed\n");
-   goto error_disable_reg;
+   goto error_disable_dvdd;
}
 
ret = ad9832_write_frequency(st, AD9832_FREQ0HM, pdata->freq0);
if (ret)
-   goto error_disable_reg;
+   goto error_disable_dvdd;
 
ret = ad9832_write_frequency(st, AD9832_FREQ1HM, pdata->freq1);
if (ret)
-   goto error_disable_reg;
+   goto error_disable_dvdd;
 
ret = ad9832_write_phase(st, AD9832_PHASE0H, pdata->phase0);
if (ret)
-   goto error_disable_reg;
+   goto error_disable_dvdd;
 
ret = ad9832_write_phase(st, AD9832_PHASE1H, pdata->phase1);
if (ret)
-   goto error_disable_reg;
+   goto error_disable_dvdd;
 
ret = ad9832_write_phase(st, AD9832_PHASE2H, pdata->phase2);
if (ret)
-   goto error_disable_reg;
+   goto error_disable_dvdd;
 
ret = ad9832_write_phase(st, AD9832_PHASE3H, pdata->phase3);
if (ret)
-   goto error_disable_reg;
+   goto error_disable_dvdd;
 
ret = iio_device_register(indio_dev);
if (ret)
-   goto error_disable_reg;
+   goto error_disable_dvdd;
 
return 0;
 
+error_disable_dvdd:
+   regulator_disable(st->dvdd);
 error_disable_reg:
regulator_disable(reg);
 
@@ -325,6 +339,7 @@ static int ad9832_remove(struct spi_device *spi)
struct ad9832_state *st = iio_priv(indio_dev);
 
iio_device_unregister(indio_dev);
+   regulator_disable(st->dvdd);
regulator_disable(st->reg);
 
return 0;
diff --git a/drivers/staging/iio/frequency/ad9832.h 
b/drivers/staging/iio/frequency/ad9832.h
index d32323b..eb0e7f2 100644
--- a/drivers/staging/iio/frequency/ad9832.h
+++ b/drivers/staging/iio/frequency/ad9832.h
@@ -59,6 +59,7 @@
  * struct ad9832_state - driver instance specific data
  * @spi:   spi_device
  * @reg:   supply regulator
+ * @dvdd:  supply regulator for the digital section
  * @mclk:  external master clock
  * @ctrl_fp:   cached frequency/phase control word
  * @ctrl_ss:   cached sync/selsrc control word
@@ -77,6 +78,7 @@
 struct ad9832_state {
struct spi_device   *spi;
struct regulator*reg;
+   struct regulator*dvdd;
unsigned long   mclk;
unsigned short  ctrl_fp;
unsigned short  ctrl_ss;
-- 
2.7.4

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


[PATCH 0/6] staging: iio: regulator clean-up

2016-10-31 Thread Eva Rachel Retuya
Rework regulator handling: 
* use supply names found on the datasheet
* fix regulator usage
* declare digital supplies previously missing

Semantic patch used to detect affected drivers:
@r1@
expression reg;
position p;
@@

reg = \(devm_regulator_get@p\|regulator_get@p\)(...);
if (IS_ERR(reg)) {
...
PTR_ERR(reg)
...
}

@@
position p != r1.p;
@@

* \(devm_regulator_get@p\|regulator_get@p\)(...)

Eva Rachel Retuya (6):
  staging: iio: set proper supply name to devm_regulator_get()
  staging: iio: rework regulator handling
  staging: iio: ad7192: add DVdd regulator
  staging: iio: ad7192: rename regulator 'reg' to 'avdd'
  staging: iio: ad9832: add DVDD regulator
  staging: iio: ad9832: clean-up regulator 'reg'

 drivers/staging/iio/adc/ad7192.c| 43 +--
 drivers/staging/iio/adc/ad7780.c| 22 +-
 drivers/staging/iio/frequency/ad9832.c  | 56 +++--
 drivers/staging/iio/frequency/ad9832.h  |  6 ++-
 drivers/staging/iio/frequency/ad9834.c  | 19 +
 drivers/staging/iio/impedance-analyzer/ad5933.c | 21 +-
 6 files changed, 101 insertions(+), 66 deletions(-)

-- 
2.7.4

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


[PATCH 2/6] staging: iio: rework regulator handling

2016-10-31 Thread Eva Rachel Retuya
Currently, the affected drivers ignore all errors from regulator_get().
The way it is now, it also breaks probe deferral (EPROBE_DEFER). The
correct behavior is to propagate the error to the upper layers so they
can handle it accordingly.

Rework the regulator handling so that it matches the standard behavior.
If the specific design uses a static always-on regulator and does not
explicitly specify it, regulator_get() will return the dummy regulator.

The following semantic patch was used to apply the change:
@r1@
expression reg, dev, en, volt;
@@

reg = \(devm_regulator_get\|regulator_get\)(dev, ...);
if (
- !
   IS_ERR(reg))
+   return PTR_ERR(reg);
(
- { en = regulator_enable(reg);
-   if (en) return en; }
+
+   en = regulator_enable(reg);
+   if (en) {
+   dev_err(dev, "Failed to enable specified supply\n");
+   return en; }
|
+
- { en = regulator_enable(reg);
-   if (en) return en;
-   volt = regulator_get_voltage(reg); }
+   en = regulator_enable(reg);
+   if (en) {
+   dev_err(dev, "Failed to enable specified supply\n");
+   return en;
+   }
+   volt = regulator_get_voltage(reg);
)

@r2@
expression arg;
@@

-   if (!IS_ERR(arg)) regulator_disable(arg);
+   regulator_disable(arg);

Hand-edit the debugging prints with the supply name to become more
specific.

Suggested-by: Lars-Peter Clausen 
Signed-off-by: Eva Rachel Retuya 
---
 drivers/staging/iio/adc/ad7192.c| 18 +-
 drivers/staging/iio/adc/ad7780.c| 18 +-
 drivers/staging/iio/frequency/ad9832.c  | 17 +
 drivers/staging/iio/frequency/ad9834.c  | 17 +
 drivers/staging/iio/impedance-analyzer/ad5933.c | 19 ++-
 5 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 41fb32d..29e32b7 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -634,13 +634,15 @@ static int ad7192_probe(struct spi_device *spi)
st = iio_priv(indio_dev);
 
st->reg = devm_regulator_get(>dev, "avdd");
-   if (!IS_ERR(st->reg)) {
-   ret = regulator_enable(st->reg);
-   if (ret)
-   return ret;
+   if (IS_ERR(st->reg))
+   return PTR_ERR(st->reg);
 
-   voltage_uv = regulator_get_voltage(st->reg);
+   ret = regulator_enable(st->reg);
+   if (ret) {
+   dev_err(>dev, "Failed to enable specified AVdd supply\n");
+   return ret;
}
+   voltage_uv = regulator_get_voltage(st->reg);
 
if (pdata->vref_mv)
st->int_vref_mv = pdata->vref_mv;
@@ -689,8 +691,7 @@ static int ad7192_probe(struct spi_device *spi)
 error_remove_trigger:
ad_sd_cleanup_buffer_and_trigger(indio_dev);
 error_disable_reg:
-   if (!IS_ERR(st->reg))
-   regulator_disable(st->reg);
+   regulator_disable(st->reg);
 
return ret;
 }
@@ -703,8 +704,7 @@ static int ad7192_remove(struct spi_device *spi)
iio_device_unregister(indio_dev);
ad_sd_cleanup_buffer_and_trigger(indio_dev);
 
-   if (!IS_ERR(st->reg))
-   regulator_disable(st->reg);
+   regulator_disable(st->reg);
 
return 0;
 }
diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
index a88236e..e149600 100644
--- a/drivers/staging/iio/adc/ad7780.c
+++ b/drivers/staging/iio/adc/ad7780.c
@@ -174,13 +174,15 @@ static int ad7780_probe(struct spi_device *spi)
ad_sd_init(>sd, indio_dev, spi, _sigma_delta_info);
 
st->reg = devm_regulator_get(>dev, "avdd");
-   if (!IS_ERR(st->reg)) {
-   ret = regulator_enable(st->reg);
-   if (ret)
-   return ret;
+   if (IS_ERR(st->reg))
+   return PTR_ERR(st->reg);
 
-   voltage_uv = regulator_get_voltage(st->reg);
+   ret = regulator_enable(st->reg);
+   if (ret) {
+   dev_err(>dev, "Failed to enable specified AVdd supply\n");
+   return ret;
}
+   voltage_uv = regulator_get_voltage(st->reg);
 
st->chip_info =
_chip_info_tbl[spi_get_device_id(spi)->driver_data];
@@ -222,8 +224,7 @@ static int ad7780_probe(struct spi_device *spi)
 error_cleanup_buffer_and_trigger:
ad_sd_cleanup_buffer_and_trigger(indio_dev);
 error_disable_reg:
-   if (!IS_ERR(st->reg))
-   regulator_disable(st->reg);
+   regulator_disable(st->reg);
 
return ret;
 }
@@ -236,8 +237,7 @@ static int ad7780_remove(struct spi_device *spi)
iio_device_unregister(indio_dev);
ad_sd_cleanup_buffer_and_trigger(indio_dev);
 
-   if (!IS_ERR(st->reg))
-   regulator_disable(st->reg);
+   regulator_disable(st->reg);
 

[PATCH 3/6] staging: iio: ad7192: add DVdd regulator

2016-10-31 Thread Eva Rachel Retuya
The AD7190/AD7192/AD7193/AD7195 is supplied with two power sources:
AVdd as analog supply voltage and DVdd as digital supply voltage.

Attempt to fetch and enable the regulator 'dvdd'. Bail out if any error
occurs.

Suggested-by: Lars-Peter Clausen 
Signed-off-by: Eva Rachel Retuya 
---
 drivers/staging/iio/adc/ad7192.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index 29e32b7..3f8dc66 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -153,6 +153,7 @@
 
 struct ad7192_state {
struct regulator*reg;
+   struct regulator*dvdd;
u16 int_vref_mv;
u32 mclk;
u32 f_order;
@@ -642,6 +643,19 @@ static int ad7192_probe(struct spi_device *spi)
dev_err(>dev, "Failed to enable specified AVdd supply\n");
return ret;
}
+
+   st->dvdd = devm_regulator_get(>dev, "dvdd");
+   if (IS_ERR(st->dvdd)) {
+   ret = PTR_ERR(st->dvdd);
+   goto error_disable_reg;
+   }
+
+   ret = regulator_enable(st->dvdd);
+   if (ret) {
+   dev_err(>dev, "Failed to enable specified DVdd supply\n");
+   goto error_disable_reg;
+   }
+
voltage_uv = regulator_get_voltage(st->reg);
 
if (pdata->vref_mv)
@@ -677,7 +691,7 @@ static int ad7192_probe(struct spi_device *spi)
 
ret = ad_sd_setup_buffer_and_trigger(indio_dev);
if (ret)
-   goto error_disable_reg;
+   goto error_disable_dvdd;
 
ret = ad7192_setup(st, pdata);
if (ret)
@@ -690,6 +704,8 @@ static int ad7192_probe(struct spi_device *spi)
 
 error_remove_trigger:
ad_sd_cleanup_buffer_and_trigger(indio_dev);
+error_disable_dvdd:
+   regulator_disable(st->dvdd);
 error_disable_reg:
regulator_disable(st->reg);
 
@@ -704,6 +720,7 @@ static int ad7192_remove(struct spi_device *spi)
iio_device_unregister(indio_dev);
ad_sd_cleanup_buffer_and_trigger(indio_dev);
 
+   regulator_disable(st->dvdd);
regulator_disable(st->reg);
 
return 0;
-- 
2.7.4

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


[PATCH 1/6] staging: iio: set proper supply name to devm_regulator_get()

2016-10-31 Thread Eva Rachel Retuya
The name passed to devm_regulator_get() should match the name of the
supply as specified in the device datasheet. This makes it clear what
power supply is being referred to in case of presence of other
regulators.

Currently, the supply name specified on the affected devices is 'vcc'.
Use lowercase version of the datasheet name to specify the supply
voltage.

Suggested-by: Lars-Peter Clausen 
Signed-off-by: Eva Rachel Retuya 
---
 drivers/staging/iio/adc/ad7192.c| 2 +-
 drivers/staging/iio/adc/ad7780.c| 2 +-
 drivers/staging/iio/frequency/ad9832.c  | 2 +-
 drivers/staging/iio/frequency/ad9834.c  | 2 +-
 drivers/staging/iio/impedance-analyzer/ad5933.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index bfa12ce..41fb32d 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -633,7 +633,7 @@ static int ad7192_probe(struct spi_device *spi)
 
st = iio_priv(indio_dev);
 
-   st->reg = devm_regulator_get(>dev, "vcc");
+   st->reg = devm_regulator_get(>dev, "avdd");
if (!IS_ERR(st->reg)) {
ret = regulator_enable(st->reg);
if (ret)
diff --git a/drivers/staging/iio/adc/ad7780.c b/drivers/staging/iio/adc/ad7780.c
index c9a0c2a..a88236e 100644
--- a/drivers/staging/iio/adc/ad7780.c
+++ b/drivers/staging/iio/adc/ad7780.c
@@ -173,7 +173,7 @@ static int ad7780_probe(struct spi_device *spi)
 
ad_sd_init(>sd, indio_dev, spi, _sigma_delta_info);
 
-   st->reg = devm_regulator_get(>dev, "vcc");
+   st->reg = devm_regulator_get(>dev, "avdd");
if (!IS_ERR(st->reg)) {
ret = regulator_enable(st->reg);
if (ret)
diff --git a/drivers/staging/iio/frequency/ad9832.c 
b/drivers/staging/iio/frequency/ad9832.c
index 358400b..744c8ee 100644
--- a/drivers/staging/iio/frequency/ad9832.c
+++ b/drivers/staging/iio/frequency/ad9832.c
@@ -212,7 +212,7 @@ static int ad9832_probe(struct spi_device *spi)
return -ENODEV;
}
 
-   reg = devm_regulator_get(>dev, "vcc");
+   reg = devm_regulator_get(>dev, "avdd");
if (!IS_ERR(reg)) {
ret = regulator_enable(reg);
if (ret)
diff --git a/drivers/staging/iio/frequency/ad9834.c 
b/drivers/staging/iio/frequency/ad9834.c
index 6366216..ca3cea6 100644
--- a/drivers/staging/iio/frequency/ad9834.c
+++ b/drivers/staging/iio/frequency/ad9834.c
@@ -329,7 +329,7 @@ static int ad9834_probe(struct spi_device *spi)
return -ENODEV;
}
 
-   reg = devm_regulator_get(>dev, "vcc");
+   reg = devm_regulator_get(>dev, "avdd");
if (!IS_ERR(reg)) {
ret = regulator_enable(reg);
if (ret)
diff --git a/drivers/staging/iio/impedance-analyzer/ad5933.c 
b/drivers/staging/iio/impedance-analyzer/ad5933.c
index 5eecf1c..62f61bc 100644
--- a/drivers/staging/iio/impedance-analyzer/ad5933.c
+++ b/drivers/staging/iio/impedance-analyzer/ad5933.c
@@ -723,7 +723,7 @@ static int ad5933_probe(struct i2c_client *client,
if (!pdata)
pdata = _default_pdata;
 
-   st->reg = devm_regulator_get(>dev, "vcc");
+   st->reg = devm_regulator_get(>dev, "vdd");
if (!IS_ERR(st->reg)) {
ret = regulator_enable(st->reg);
if (ret)
-- 
2.7.4

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


RE: [PATCH] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg()

2016-10-31 Thread KY Srinivasan


> -Original Message-
> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> Sent: Monday, October 31, 2016 3:05 AM
> To: KY Srinivasan 
> Cc: de...@linuxdriverproject.org; Van De Ven, Arjan
> ; linux-ker...@vger.kernel.org; Haiyang Zhang
> 
> Subject: Re: [PATCH] Drivers: hv: vmbus: Raise retry/wait limits in
> vmbus_post_msg()
> 
> KY Srinivasan  writes:
> 
> >> -Original Message-
> >> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> >> Sent: Wednesday, October 26, 2016 4:12 AM
> >> To: de...@linuxdriverproject.org
> >> Cc: linux-ker...@vger.kernel.org; KY Srinivasan ;
> >> Haiyang Zhang 
> >> Subject: [PATCH] Drivers: hv: vmbus: Raise retry/wait limits in
> >> vmbus_post_msg()
> >>
> >> DoS protection conditions were altered in WS2016 and now it's easy to get
> >> -EAGAIN returned from vmbus_post_msg() (e.g. when we try changing
> MTU
> >> on a
> >> netvsc device in a loop). All vmbus_post_msg() callers don't retry the
> >> operation and we usually end up with a non-functional device or crash.
> >>
> >> While host's DoS protection conditions are unknown to me my tests show
> >> that
> >> it can take up to 46 attempts to send a message after changing udelay() to
> >> mdelay() and caping msec at '256', this means we can wait up to 10
> seconds
> >> before the message is sent so we need to use msleep() instead. Almost all
> >> vmbus_post_msg() callers are ready to sleep but there is one special case:
> >> vmbus_initiate_unload() which can be called from interrupt/NMI context
> >> and
> >> we can't sleep there. I'm also not sure about the lonely
> >> vmbus_send_tl_connect_request() which has no in-tree users but its
> >> external
> >> users are most likely waiting for the host to reply so sleeping there is
> >> also appropriate.
> >
> > Vitaly,
> >
> > One of the reasons why the delay was in microseconds was to make sure
> that the boot time
> > was not adversely affected by the delay we had in setting up the channel.
> The change to microsecond
> > delay and other changes in this code reduced the time it took to initialize
> netvsc from
> > 200 milliseconds to about 12 milliseconds. This is important for us as we 
> > look
> at achieving sub-second
> > boot times.
> > The situation you are trying to address are test cases where you are hitting
> the host with
> > requests that triggers hosts DOS prevention code. Perhaps we could have a
> hybrid approach: we
> > retain microsecond wait until we hit a threshold and then we use
> millisecond delays. This way, the normal boot
> > path is still fast while we can handle some of the other cases where the 
> > host
> DOS prevention code kicks in.
> >
> 
> Ok,
> 
> I actually tested boot time with my patch and didn't see a difference
> (so I guess our first attempt to send messages usually succeeds) but if
> we're concearned about less-than-a-second boot time we'd rather keep the
> microseonds delay for first several attempts. I'll do v2.

Thank you.

K. Y
> 
> Thanks,
> 
> 
> --
>   Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH] PCI: hv: Make unnecessarily global IRQ masking functions static

2016-10-31 Thread KY Srinivasan


> -Original Message-
> From: Tobias Klauser [mailto:tklau...@distanz.ch]
> Sent: Monday, October 31, 2016 4:04 AM
> To: KY Srinivasan ; Haiyang Zhang
> 
> Cc: Bjorn Helgaas ; de...@linuxdriverproject.org;
> linux-...@vger.kernel.org
> Subject: [PATCH] PCI: hv: Make unnecessarily global IRQ masking functions
> static
> 
> Make hv_irq_mask and hv_irq_unmask static as they are only used in
> pci-hyperv.c
> 
> This fixes a sparse warning.
> 
> Signed-off-by: Tobias Klauser 

Thanks.

Acked-by: K. Y. Srinivasan 

> ---
>  drivers/pci/host/pci-hyperv.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
> index 763ff8745828..06c98695c06c 100644
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -755,7 +755,7 @@ static int hv_set_affinity(struct irq_data *data, const
> struct cpumask *dest,
>   return parent->chip->irq_set_affinity(parent, dest, force);
>  }
> 
> -void hv_irq_mask(struct irq_data *data)
> +static void hv_irq_mask(struct irq_data *data)
>  {
>   pci_msi_mask_irq(data);
>  }
> @@ -770,7 +770,7 @@ void hv_irq_mask(struct irq_data *data)
>   * is built out of this PCI bus's instance GUID and the function
>   * number of the device.
>   */
> -void hv_irq_unmask(struct irq_data *data)
> +static void hv_irq_unmask(struct irq_data *data)
>  {
>   struct msi_desc *msi_desc = irq_data_get_msi_desc(data);
>   struct irq_cfg *cfg = irqd_cfg(data);
> --
> 2.9.0
> 

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


RE: [PATCH 12/15] vmbus: make sysfs names consistent with PCI

2016-10-31 Thread KY Srinivasan


> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, October 31, 2016 5:44 AM
> To: KY Srinivasan 
> Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> jasow...@redhat.com; leann.ogasaw...@canonical.com; Stephen
> Hemminger 
> Subject: Re: [PATCH 12/15] vmbus: make sysfs names consistent with PCI
> 
> On Mon, Oct 31, 2016 at 12:41:47AM -0700, k...@exchange.microsoft.com
> wrote:
> > From: Stephen Hemminger 
> >
> > In commit 9a56e5d6a0ba ("Drivers: hv: make VMBus bus ids persistent")
> > the name of vmbus devices in sysfs changed to be (in 4.9-rc1):
> >   /sys/bus/vmbus/vmbus-6aebe374-9ba0-11e6-933c-00259086b36b
> >
> > The prefix ("vmbus-") is redundant and differs from how PCI is
> > represented in sysfs. Therefore simplify to:
> >   /sys/bus/vmbus/6aebe374-9ba0-11e6-933c-00259086b36b
> >
> > Please merge this before 4.9 is released and the old format
> > has to live forever.
> 
> Why have you included this in a patchset that will not get merged until
> 4.10-rc1???
> 
> You want this in 4.9, right?
> 
> confused,

Sorry about that; I will send this separately.

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


RE: [PATCH 00/15] Drivers: hv: Some miscellaneous fixes and enhancements

2016-10-31 Thread KY Srinivasan


> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, October 31, 2016 5:46 AM
> To: KY Srinivasan 
> Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com;
> jasow...@redhat.com; leann.ogasaw...@canonical.com; Stephen
> Hemminger 
> Subject: Re: [PATCH 00/15] Drivers: hv: Some miscellaneous fixes and
> enhancements
> 
> On Mon, Oct 31, 2016 at 12:41:08AM -0700, k...@exchange.microsoft.com
> wrote:
> > From: K. Y. Srinivasan 
> >
> > Some miscellaneous fixes and enhancements.
> 
> With this patch series applied I get the following build error:
>   ERROR: "vmbus_setevent" [drivers/net/hyperv/hv_netvsc.ko]
> undefined!
> 
> I don't have the time right now to bisect down to which patch is broken,
> please fix it up and resend them all after resolving this.

Thanks Greg. It built clean for me; I will fix this and resend.

Regards,

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


[PATCH 7/7] staging: vchiq_arm: change order during module probe

2016-10-31 Thread Stefan Wahren
The current order during module probe is prone to race conditions:

* debugfs entries, sysfs entries, platform code

So fix this by swapping the steps debugfs entries and platform code.
As a benefit this saves us a clean up step in the error path.

Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |   18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index a6bb177..a279acd 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -2813,16 +2813,15 @@ static int vchiq_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, fw);
 
-   /* create debugfs entries */
-   err = vchiq_debugfs_init();
+   err = vchiq_platform_init(pdev, _state);
if (err != 0)
-   goto failed_debugfs_init;
+   goto failed_platform_init;
 
err = alloc_chrdev_region(_devid, VCHIQ_MINOR, 1, DEVICE_NAME);
if (err != 0) {
vchiq_log_error(vchiq_arm_log_level,
"Unable to allocate device number");
-   goto failed_alloc_chrdev;
+   goto failed_platform_init;
}
cdev_init(_cdev, _fops);
vchiq_cdev.owner = THIS_MODULE;
@@ -2845,9 +2844,10 @@ static int vchiq_probe(struct platform_device *pdev)
if (IS_ERR(ptr_err))
goto failed_device_create;
 
-   err = vchiq_platform_init(pdev, _state);
+   /* create debugfs entries */
+   err = vchiq_debugfs_init();
if (err != 0)
-   goto failed_platform_init;
+   goto failed_debugfs_init;
 
vchiq_log_info(vchiq_arm_log_level,
"vchiq: initialised - version %d (min %d), device %d.%d",
@@ -2856,7 +2856,7 @@ static int vchiq_probe(struct platform_device *pdev)
 
return 0;
 
-failed_platform_init:
+failed_debugfs_init:
device_destroy(vchiq_class, vchiq_devid);
 failed_device_create:
class_destroy(vchiq_class);
@@ -2865,9 +2865,7 @@ static int vchiq_probe(struct platform_device *pdev)
err = PTR_ERR(ptr_err);
 failed_cdev_add:
unregister_chrdev_region(vchiq_devid, 1);
-failed_alloc_chrdev:
-   vchiq_debugfs_deinit();
-failed_debugfs_init:
+failed_platform_init:
vchiq_log_warning(vchiq_arm_log_level, "could not load vchiq");
return err;
 }
-- 
1.7.9.5

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


[PATCH 1/7] staging: vchiq_core: fix service dereference in unlock_service

2016-10-31 Thread Stefan Wahren
The service state is dereferenced before BUG_ON and outside of the
spin lock. So in order to avoid possible NULL pointer dereferences or
races move the whole scope at a safer place.

This issue has been found by Cppcheck.

Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c |3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index f3e1000..2e0e8ef 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -296,12 +296,13 @@ static const char *msg_type_str(unsigned int msg_type)
 void
 unlock_service(VCHIQ_SERVICE_T *service)
 {
-   VCHIQ_STATE_T *state = service->state;
spin_lock(_spinlock);
BUG_ON(!service || (service->ref_count == 0));
if (service && service->ref_count) {
service->ref_count--;
if (!service->ref_count) {
+   VCHIQ_STATE_T *state = service->state;
+
BUG_ON(service->srvstate != VCHIQ_SRVSTATE_FREE);
state->services[service->localport] = NULL;
} else
-- 
1.7.9.5

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


[PATCH 4/7] staging: vchiq_arm: remove hardcoded buffer length

2016-10-31 Thread Stefan Wahren
We better use sizeof instead of hardcoding buffer length multiple
times. This make it easier to increase the buffer in the future.
In order to keep below 80 chars limit make the variable name shorter.

Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index b20d217..ba124c6 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -2025,20 +2025,20 @@ static void close_delivered(USER_SERVICE_T 
*user_service)
 output_timeout_error(VCHIQ_STATE_T *state)
 {
VCHIQ_ARM_STATE_T *arm_state = vchiq_platform_get_arm_state(state);
-   char service_err[50] = "";
+   char err[50] = "";
int vc_use_count = arm_state->videocore_use_count;
int active_services = state->unused_service;
int i;
 
if (!arm_state->videocore_use_count) {
-   snprintf(service_err, 50, " Videocore usecount is 0");
+   snprintf(err, sizeof(err), " Videocore usecount is 0");
goto output_msg;
}
for (i = 0; i < active_services; i++) {
VCHIQ_SERVICE_T *service_ptr = state->services[i];
if (service_ptr && service_ptr->service_use_count &&
(service_ptr->srvstate != VCHIQ_SRVSTATE_FREE)) {
-   snprintf(service_err, 50, " %c%c%c%c(%d) service has "
+   snprintf(err, sizeof(err), " %c%c%c%c(%d) service has "
"use count %d%s", VCHIQ_FOURCC_AS_4CHARS(
service_ptr->base.fourcc),
 service_ptr->client_id,
@@ -2052,7 +2052,7 @@ static void close_delivered(USER_SERVICE_T *user_service)
 output_msg:
vchiq_log_error(vchiq_susp_log_level,
"timed out waiting for vc suspend (%d).%s",
-arm_state->autosuspend_override, service_err);
+arm_state->autosuspend_override, err);
 
 }
 
-- 
1.7.9.5

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


[PATCH 5/7] staging: vchiq_arm: handle error case of get_user_pages

2016-10-31 Thread Stefan Wahren
It's possible that get_user_pages() could fail. So evaluate its
return code and handle this error case properly.

This issue has been found by Cppcheck.

Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index ba124c6..be890f9 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -1478,6 +1478,12 @@ static void close_delivered(USER_SERVICE_T *user_service)
prev_idx = -1;
page = NULL;
 
+   if (rc < 0) {
+   vchiq_log_error(vchiq_arm_log_level,
+   "Failed to get user pages: %d\n", rc);
+   goto out;
+   }
+
while (offset < end_offset) {
 
int page_offset = offset % PAGE_SIZE;
@@ -1501,6 +1507,8 @@ static void close_delivered(USER_SERVICE_T *user_service)
 
offset += 16;
}
+
+out:
if (page != NULL)
kunmap(page);
 
-- 
1.7.9.5

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


[PATCH 2/7] staging: vchiq_core: fix format strings in vchiq_dump_service_state

2016-10-31 Thread Stefan Wahren
The member localport and remoteport are unsigned. So fix the format
string accordingly.

The issue has been found by Cppcheck.

Signed-off-by: Stefam Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_core.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
index 2e0e8ef..2139c94 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_core.c
@@ -3687,7 +3687,7 @@ static const char *msg_type_str(unsigned int msg_type)
char buf[80];
int len;
 
-   len = snprintf(buf, sizeof(buf), "Service %d: %s (ref %u)",
+   len = snprintf(buf, sizeof(buf), "Service %u: %s (ref %u)",
service->localport, srvstate_names[service->srvstate],
service->ref_count - 1); /*Don't include the lock just taken*/
 
@@ -3699,7 +3699,7 @@ static const char *msg_type_str(unsigned int msg_type)
int tx_pending, rx_pending;
if (service->remoteport != VCHIQ_PORT_FREE) {
int len2 = snprintf(remoteport, sizeof(remoteport),
-   "%d", service->remoteport);
+   "%u", service->remoteport);
if (service->public_fourcc != VCHIQ_FOURCC_INVALID)
snprintf(remoteport + len2,
sizeof(remoteport) - len2,
-- 
1.7.9.5

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


[PATCH 6/7] staging: vchiq_arm: remove debugfs entries on module unload

2016-10-31 Thread Stefan Wahren
This removes the debugfs entries on module unload and fix one
of the many kernel oops after loading the module again.

Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index be890f9..a6bb177 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -2874,6 +2874,7 @@ static int vchiq_probe(struct platform_device *pdev)
 
 static int vchiq_remove(struct platform_device *pdev)
 {
+   vchiq_debugfs_deinit();
device_destroy(vchiq_class, vchiq_devid);
class_destroy(vchiq_class);
cdev_del(_cdev);
-- 
1.7.9.5

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


[PATCH 3/7] staging: vchiq_arm: add missing of_node_put

2016-10-31 Thread Stefan Wahren
After device_node usage the refcount must be decremented with
of_node_put().

Signed-off-by: Stefan Wahren 
---
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
index 4e0401e..b20d217 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -2799,6 +2799,7 @@ static int vchiq_probe(struct platform_device *pdev)
}
 
fw = rpi_firmware_get(fw_node);
+   of_node_put(fw_node);
if (!fw)
return -EPROBE_DEFER;
 
-- 
1.7.9.5

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


Greybus Future

2016-10-31 Thread Alex Elder
The Greybus kernel code, developed as part of Google's Project Ara,
is in the upstream Linux kernel tree (under drivers/staging).  The
cancellation of that project makes the future for Greybus a bit less
certain.  There is interest among the core developers of Greybus
(and others) to do what we can to preserve the value of the Greybus
implementation for use beyond its original target platform.  For
that to have any chance of success, we need to establish some
organization, and basically form a community of developers and
interested parties.

I'm proposing below a few things, which I'll implement this week
unless I hear people express strong opposition (or clearly better
suggestions).  If you have comments, please say something.

Git repositories.  Public git repositories related to Project Ara
are all hosted here:
https://github.com/projectara
At this time I see no reason to move away from this, but it would
not surprise me if we decided to host the code and documentation
somewhere else at some future date.

Mailing List.  Because the Greybus kernel code sits in the staging
tree, mail about it should go to the driver-devel mailing list
(de...@driverdev.osuosl.org).  I know that some people prefer better
mail filters rather than more mailing lists, but I think Greybus
discussion warrants a separate list.  Its code spans multiple code
bases, and I think we're going to need some dedicated and ongoing
strategy and design discussions.  So I'd like to create a Greybus
mailing list as a home for all Greybus-related discussion, including
patch traffic.  My suggestion is to create a majordomo list:
grey...@kernel.org

Patchwork.  In the past I have found Patchwork to be a very useful
tool in managing incoming patches.  I'd like to set up a patchwork
instance for Greybus, monitoring the greybus mailing list, again
at kernel.org.

Wiki page.  If we want a Wiki page, we could set one up at
kernel.org.  Thoughts?

Web home page.  I have secured the "greybus.org" domain, so if there
is any value in populating that page it could serve as the home page
for the project.  It currently has no content.

Are there any other important resources I've missed?  Once we get
a list going (or not) I've got a few other things to talk about.

Thanks.

-Alex

PS  I have addressed this message to a set of interested people
whose e-mail addresses I knew; the set is definitely incomplete.
This will form the initial membership of the mailing list.  If
you do not want to be on that list, or if you know others I
should include, please send me contact information by private
message.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: Greybus Future

2016-10-31 Thread Laurent Pinchart
Hi Alex,

On Monday 31 Oct 2016 08:50:53 Alex Elder wrote:
> The Greybus kernel code, developed as part of Google's Project Ara,
> is in the upstream Linux kernel tree (under drivers/staging).  The
> cancellation of that project makes the future for Greybus a bit less
> certain.  There is interest among the core developers of Greybus
> (and others) to do what we can to preserve the value of the Greybus
> implementation for use beyond its original target platform.  For
> that to have any chance of success, we need to establish some
> organization, and basically form a community of developers and
> interested parties.
> 
> I'm proposing below a few things, which I'll implement this week
> unless I hear people express strong opposition (or clearly better
> suggestions).  If you have comments, please say something.
> 
> Git repositories.  Public git repositories related to Project Ara
> are all hosted here:
> https://github.com/projectara
> At this time I see no reason to move away from this, but it would
> not surprise me if we decided to host the code and documentation
> somewhere else at some future date.
> 
> Mailing List.  Because the Greybus kernel code sits in the staging
> tree, mail about it should go to the driver-devel mailing list
> (de...@driverdev.osuosl.org).  I know that some people prefer better
> mail filters rather than more mailing lists, but I think Greybus
> discussion warrants a separate list.  Its code spans multiple code
> bases, and I think we're going to need some dedicated and ongoing
> strategy and design discussions.  So I'd like to create a Greybus
> mailing list as a home for all Greybus-related discussion, including
> patch traffic.  My suggestion is to create a majordomo list:
> grey...@kernel.org
> 
> Patchwork.  In the past I have found Patchwork to be a very useful
> tool in managing incoming patches.  I'd like to set up a patchwork
> instance for Greybus, monitoring the greybus mailing list, again
> at kernel.org.

I definitely second that as it doesn't involve Gerrit ;-)

> Wiki page.  If we want a Wiki page, we could set one up at
> kernel.org.  Thoughts?
> 
> Web home page.  I have secured the "greybus.org" domain, so if there
> is any value in populating that page it could serve as the home page
> for the project.  It currently has no content.
> 
> Are there any other important resources I've missed?  Once we get
> a list going (or not) I've got a few other things to talk about.
> 
> Thanks.
> 
>   -Alex
> 
> PS  I have addressed this message to a set of interested people
> whose e-mail addresses I knew; the set is definitely incomplete.
> This will form the initial membership of the mailing list.  If
> you do not want to be on that list, or if you know others I
> should include, please send me contact information by private
> message.

-- 
Regards,

Laurent Pinchart

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


Re: [PATCH 2/2] mm: remove get_user_pages_locked()

2016-10-31 Thread Lorenzo Stoakes
On Mon, Oct 31, 2016 at 12:45:36PM +0100, Paolo Bonzini wrote:
>
>
> On 31/10/2016 11:02, Lorenzo Stoakes wrote:
> > - *
> > - * get_user_pages should be phased out in favor of
> > - * get_user_pages_locked|unlocked or get_user_pages_fast. Nothing
> > - * should use get_user_pages because it cannot pass
> > - * FAULT_FLAG_ALLOW_RETRY to handle_mm_fault.
>
> This comment should be preserved in some way.  In addition, removing

Could you clarify what you think should be retained?

The comment seems to me to be largely rendered redundant by this change -
get_user_pages() now offers identical behaviour, and of course the latter part
of the comment ('because it cannot pass FAULT_FLAG_ALLOW_RETRY') is rendered
incorrect by this change.

It could be replaced with a recommendation to make use of VM_FAULT_RETRY logic
if possible. Note that the line above retains the reference to
'get_user_pages_fast()' - 'See also get_user_pages_fast, for performance
critical applications.'

One important thing to note here is this is a comment on get_user_pages_remote()
for which it was already incorrect syntactically (I'm guessing once
get_user_pages_remote() was added it 'stole' get_user_pages()'s comment :)

> get_user_pages_locked() makes it harder (compared to a simple "git grep
> -w") to identify callers that lack allow-retry functionality).  So I'm
> not sure about the benefits of these patches.
>

That's a fair point, and certainly this cleanup series is less obviously helpful
than previous ones.

However, there are a few points in its favour:

1. get_user_pages_remote() was the mirror of get_user_pages() prior to adding an
   int *locked parameter to the former (necessary to allow for the unexport of
   __get_user_pages_unlocked()), differing only in task/mm being specified and
   FOLL_REMOTE being set. This patch series keeps these functions 'synchronised'
   in this respect.

2. There is currently only one caller of get_user_pages_locked() in
   mm/frame_vector.c which seems to suggest this function isn't widely
   used/known.

3. This change results in all slow-path get_user_pages*() functions having the
   ability to use VM_FAULT_RETRY logic rather than 'defaulting' to using
   get_user_pages() that doesn't let you do this even if you wanted to.

4. It's somewhat confusing/redundant having both get_user_pages_locked() and
   get_user_pages() functions which both require mmap_sem to be held (i.e. both
   are 'locked' versions.)

> If all callers were changed, then sure removing the _locked suffix would
> be a good idea.

It seems many callers cannot release mmap_sem so VM_FAULT_RETRY logic couldn't
happen anyway in this cases and in these cases we couldn't change the caller.


Overall, an alternative here might be to remove get_user_pages() and update
get_user_pages_locked() to add a 'vmas' parameter, however doing this renders
get_user_pages_unlocked() asymmetric as it would lack an vmas parameter (adding
one there would make no sense as VM_FAULT_RETRY logic invalidates VMAs) though
perhaps not such a big issue as it makes sense as to why this is the case.

get_user_pages_unlocked() definitely seems to be a useful helper and therefore
makes sense to retain.

Of course another alternative is to leave things be :)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 00/15] Drivers: hv: Some miscellaneous fixes and enhancements

2016-10-31 Thread Greg KH
On Mon, Oct 31, 2016 at 12:41:08AM -0700, k...@exchange.microsoft.com wrote:
> From: K. Y. Srinivasan 
> 
> Some miscellaneous fixes and enhancements.

With this patch series applied I get the following build error:
ERROR: "vmbus_setevent" [drivers/net/hyperv/hv_netvsc.ko] undefined!

I don't have the time right now to bisect down to which patch is broken,
please fix it up and resend them all after resolving this.

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


Re: [PATCH 12/15] vmbus: make sysfs names consistent with PCI

2016-10-31 Thread Greg KH
On Mon, Oct 31, 2016 at 12:41:47AM -0700, k...@exchange.microsoft.com wrote:
> From: Stephen Hemminger 
> 
> In commit 9a56e5d6a0ba ("Drivers: hv: make VMBus bus ids persistent")
> the name of vmbus devices in sysfs changed to be (in 4.9-rc1):
>   /sys/bus/vmbus/vmbus-6aebe374-9ba0-11e6-933c-00259086b36b
> 
> The prefix ("vmbus-") is redundant and differs from how PCI is
> represented in sysfs. Therefore simplify to:
>   /sys/bus/vmbus/6aebe374-9ba0-11e6-933c-00259086b36b
> 
> Please merge this before 4.9 is released and the old format
> has to live forever.

Why have you included this in a patchset that will not get merged until
4.10-rc1???

You want this in 4.9, right?

confused,

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


Re: [PATCH 2/2] mm: remove get_user_pages_locked()

2016-10-31 Thread Paolo Bonzini


On 31/10/2016 11:02, Lorenzo Stoakes wrote:
> - *
> - * get_user_pages should be phased out in favor of
> - * get_user_pages_locked|unlocked or get_user_pages_fast. Nothing
> - * should use get_user_pages because it cannot pass
> - * FAULT_FLAG_ALLOW_RETRY to handle_mm_fault.

This comment should be preserved in some way.  In addition, removing
get_user_pages_locked() makes it harder (compared to a simple "git grep
-w") to identify callers that lack allow-retry functionality).  So I'm
not sure about the benefits of these patches.

If all callers were changed, then sure removing the _locked suffix would
be a good idea.

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


[PATCH v4] staging : rtl8712: Free memory when kmalloc fails

2016-10-31 Thread Souptick Joarder
There are few functions where we need to free previously allocated
memory when kmalloc fails. Else it may lead to memory leakage. In
_init_cmd_priv() and _r8712_init_xmit_priv(), in few places we are
not freeing previously allocated memory when kmalloc fails.

Signed-off-by: Souptick joarder 
---
v2:
  - Make the commit message more descriptive
v3:
  - Modified the patch description by adding space in required places
v4:
  - patch v3 was broken due to hand-edit in patch file. Corrected it.

 drivers/staging/rtl8712/rtl871x_cmd.c  | 5 -
 drivers/staging/rtl8712/rtl871x_xmit.c | 5 -
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c 
b/drivers/staging/rtl8712/rtl871x_cmd.c
index b7ee5e6..04638f1 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.c
+++ b/drivers/staging/rtl8712/rtl871x_cmd.c
@@ -72,8 +72,11 @@ static sint _init_cmd_priv(struct cmd_priv *pcmdpriv)
((addr_t)(pcmdpriv->cmd_allocated_buf) &
(CMDBUFF_ALIGN_SZ - 1));
pcmdpriv->rsp_allocated_buf = kmalloc(MAX_RSPSZ + 4, GFP_ATOMIC);
-   if (!pcmdpriv->rsp_allocated_buf)
+   if (!pcmdpriv->rsp_allocated_buf) {
+   kfree(pcmdpriv->cmd_allocated_buf);
+   pcmdpriv->cmd_allocated_buf = NULL;
return _FAIL;
+   }
pcmdpriv->rsp_buf = pcmdpriv->rsp_allocated_buf  +  4 -
((addr_t)(pcmdpriv->rsp_allocated_buf) & 3);
pcmdpriv->cmd_issued_cnt = 0;
diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c 
b/drivers/staging/rtl8712/rtl871x_xmit.c
index be38364..484d2f2 100644
--- a/drivers/staging/rtl8712/rtl871x_xmit.c
+++ b/drivers/staging/rtl8712/rtl871x_xmit.c
@@ -128,8 +128,11 @@ sint _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
_init_queue(>pending_xmitbuf_queue);
pxmitpriv->pallocated_xmitbuf = kmalloc(NR_XMITBUFF * sizeof(struct 
xmit_buf) + 4,
GFP_ATOMIC);
-   if (!pxmitpriv->pallocated_xmitbuf)
+   if (!pxmitpriv->pallocated_xmitbuf) {
+   kfree(pxmitpriv->pallocated_frame_buf);
+   pxmitpriv->pallocated_frame_buf = NULL;
return _FAIL;
+   }
pxmitpriv->pxmitbuf = pxmitpriv->pallocated_xmitbuf + 4 -
  ((addr_t)(pxmitpriv->pallocated_xmitbuf) & 3);
pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
-- 
1.9.1

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


[PATCH] PCI: hv: Make unnecessarily global IRQ masking functions static

2016-10-31 Thread Tobias Klauser
Make hv_irq_mask and hv_irq_unmask static as they are only used in
pci-hyperv.c

This fixes a sparse warning.

Signed-off-by: Tobias Klauser 
---
 drivers/pci/host/pci-hyperv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 763ff8745828..06c98695c06c 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -755,7 +755,7 @@ static int hv_set_affinity(struct irq_data *data, const 
struct cpumask *dest,
return parent->chip->irq_set_affinity(parent, dest, force);
 }
 
-void hv_irq_mask(struct irq_data *data)
+static void hv_irq_mask(struct irq_data *data)
 {
pci_msi_mask_irq(data);
 }
@@ -770,7 +770,7 @@ void hv_irq_mask(struct irq_data *data)
  * is built out of this PCI bus's instance GUID and the function
  * number of the device.
  */
-void hv_irq_unmask(struct irq_data *data)
+static void hv_irq_unmask(struct irq_data *data)
 {
struct msi_desc *msi_desc = irq_data_get_msi_desc(data);
struct irq_cfg *cfg = irqd_cfg(data);
-- 
2.9.0


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


Re: [PATCH] staging: comedi: ni_tio: fix warnings of uninitialized variables

2016-10-31 Thread Ian Abbott

On 30/10/16 15:29, Ted Chen wrote:

Fix the following warnings by initializing these variables
to zero and add error check to return early when the check
returns an error.

drivers/staging/comedi/drivers/ni_tio.c: In function ‘ni_tio_set_sync_mode’:
drivers/staging/comedi/drivers/ni_tio.c:492:28: warning: ‘ps’ may be used
uninitialized in this function [-Wuninitialized]
drivers/staging/comedi/drivers/ni_tio.c: In function ‘ni_tio_insn_config’:
drivers/staging/comedi/drivers/ni_tio.c:820:2: warning: ‘temp64’ may be used
uninitialized in this function [-Wuninitialized]
drivers/staging/comedi/drivers/ni_tio.c:811:6: note: ‘temp64’ was declared her

Signed-off-by: Ted Chen 
---
 drivers/staging/comedi/drivers/ni_tio.c |   16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)


This patch (Message-Id: 
<1477841360-3949-1-git-send-email-tedc.37z...@gmail.com>) should really 
have been tagged as "[PATCH v2]" and should have included a brief 
description (after the "---" line) of changes since the first patch, 
which I guess would be something along the lines of:


v2: In function 'ni_tio_set_sync_mode', also initialize 'clk_src' and 
return early if 'ni_tio_generic_clock_src_select' returns an error.


(I don't think you need to resend the patch for that, as I'm sure Greg 
can figure it out.)


Apart from that, the patch looks fine, thanks!

Reviewed-by: Ian Abbott 

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


Re: [PATCH v3] staging : rtl8712: Free memory when kmalloc fails

2016-10-31 Thread Souptick Joarder
H Greg,

On Mon, Oct 31, 2016 at 3:37 PM, Greg KH  wrote:
> On Mon, Oct 31, 2016 at 02:32:39PM +0530, Souptick Joarder wrote:
>> There are few functions where we need to free previously allocated
>> memory when kmalloc fails. Else it may lead to memory leakage. In
>> _init_cmd_priv() and _r8712_init_xmit_priv(), in few places we are
>> not freeing previously allocated memory when kmalloc fails.
>>
>> Signed-off-by: Souptick joarder 
>> ---
>> v2:
>>   - Make the commit message more descriptive
>> v3:
>>   - Modified the patch description by adding space in required places
>
> Why did you resend the broken v3 patch?

I am getting confused here. Do I need to update the change log again to v4?
The last patch which I send today is the latest patch file from my side.
I tried to apply it locally and it's working fine.

I am not clear what is the mistake I am doing ?
It will be helpful if you explain a bit that I will not repeat the
same mistake again.
>
> This is getting really annoying, what would you do in my situation?...

I am extremely sorry for causing this unnecessary overhead.
It was not intentional.  Sorry once again.

As this is really getting worse, Shall I drop this patch , rewrite
this patch and summit it as a completely new patch ?

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


Re: [PATCH v3] staging : rtl8712: Free memory when kmalloc fails

2016-10-31 Thread Greg KH
On Mon, Oct 31, 2016 at 02:32:39PM +0530, Souptick Joarder wrote:
> There are few functions where we need to free previously allocated
> memory when kmalloc fails. Else it may lead to memory leakage. In
> _init_cmd_priv() and _r8712_init_xmit_priv(), in few places we are
> not freeing previously allocated memory when kmalloc fails.
> 
> Signed-off-by: Souptick joarder 
> ---
> v2:
>   - Make the commit message more descriptive
> v3:
>   - Modified the patch description by adding space in required places

Why did you resend the broken v3 patch?

This is getting really annoying, what would you do in my situation?...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg()

2016-10-31 Thread Van De Ven, Arjan
> Ok,
> 
> I actually tested boot time with my patch and didn't see a difference
> (so I guess our first attempt to send messages usually succeeds) but if
> we're concearned about less-than-a-second boot time we'd rather keep the
> microseonds delay for first several attempts. I'll do v2.

of course we care about less-than-a-second boot time..
it's a virtual machine.. there's no reason the kernel should ever take more 
than 100 msec total
(including all drivers)

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


Re: [PATCH] Drivers: hv: vmbus: Raise retry/wait limits in vmbus_post_msg()

2016-10-31 Thread Vitaly Kuznetsov
KY Srinivasan  writes:

>> -Original Message-
>> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
>> Sent: Wednesday, October 26, 2016 4:12 AM
>> To: de...@linuxdriverproject.org
>> Cc: linux-ker...@vger.kernel.org; KY Srinivasan ;
>> Haiyang Zhang 
>> Subject: [PATCH] Drivers: hv: vmbus: Raise retry/wait limits in
>> vmbus_post_msg()
>> 
>> DoS protection conditions were altered in WS2016 and now it's easy to get
>> -EAGAIN returned from vmbus_post_msg() (e.g. when we try changing MTU
>> on a
>> netvsc device in a loop). All vmbus_post_msg() callers don't retry the
>> operation and we usually end up with a non-functional device or crash.
>> 
>> While host's DoS protection conditions are unknown to me my tests show
>> that
>> it can take up to 46 attempts to send a message after changing udelay() to
>> mdelay() and caping msec at '256', this means we can wait up to 10 seconds
>> before the message is sent so we need to use msleep() instead. Almost all
>> vmbus_post_msg() callers are ready to sleep but there is one special case:
>> vmbus_initiate_unload() which can be called from interrupt/NMI context
>> and
>> we can't sleep there. I'm also not sure about the lonely
>> vmbus_send_tl_connect_request() which has no in-tree users but its
>> external
>> users are most likely waiting for the host to reply so sleeping there is
>> also appropriate.
>
> Vitaly,
>
> One of the reasons why the delay was in microseconds was to make sure that 
> the boot time
> was not adversely affected by the delay we had in setting up the channel. The 
> change to microsecond
> delay and other changes in this code reduced the time it took to initialize 
> netvsc from 
> 200 milliseconds to about 12 milliseconds. This is important for us as we 
> look at achieving sub-second
> boot times.
> The situation you are trying to address are test cases where you are hitting 
> the host with
> requests that triggers hosts DOS prevention code. Perhaps we could have a 
> hybrid approach: we
> retain microsecond wait until we hit a threshold and then we use millisecond 
> delays. This way, the normal boot
> path is still fast while we can handle some of the other cases where the host 
> DOS prevention code kicks in.
>

Ok,

I actually tested boot time with my patch and didn't see a difference
(so I guess our first attempt to send messages usually succeeds) but if
we're concearned about less-than-a-second boot time we'd rather keep the
microseonds delay for first several attempts. I'll do v2.

Thanks,


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


[PATCH 1/2] mm: add locked parameter to get_user_pages()

2016-10-31 Thread Lorenzo Stoakes
This patch adds an int *locked parameter to get_user_pages() to allow
VM_FAULT_RETRY faulting behaviour similar to get_user_pages_[un]locked().

It additionally clears the way for get_user_pages_locked() to be removed as its
sole remaining useful characteristic was to allow for VM_FAULT_RETRY behaviour
when faulting in pages.

It should not introduce any functional changes, however it does allow for
subsequent changes to get_user_pages() callers to take advantage of
VM_FAULT_RETRY.

Signed-off-by: Lorenzo Stoakes 
---
 arch/cris/arch-v32/drivers/cryptocop.c| 2 ++
 arch/ia64/kernel/err_inject.c | 2 +-
 arch/x86/mm/mpx.c | 2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   | 2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c   | 2 +-
 drivers/gpu/drm/via/via_dmablit.c | 2 +-
 drivers/infiniband/core/umem.c| 2 +-
 drivers/infiniband/hw/mthca/mthca_memfree.c   | 3 ++-
 drivers/infiniband/hw/qib/qib_user_pages.c| 2 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c  | 2 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c | 2 +-
 drivers/misc/mic/scif/scif_rma.c  | 1 +
 drivers/misc/sgi-gru/grufault.c   | 3 ++-
 drivers/platform/goldfish/goldfish_pipe.c | 2 +-
 drivers/rapidio/devices/rio_mport_cdev.c  | 2 +-
 .../staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c| 3 ++-
 drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
 drivers/virt/fsl_hypervisor.c | 2 +-
 include/linux/mm.h| 2 +-
 mm/gup.c  | 8 
 mm/ksm.c  | 3 ++-
 mm/mempolicy.c| 2 +-
 mm/nommu.c| 4 ++--
 virt/kvm/kvm_main.c   | 4 ++--
 24 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/arch/cris/arch-v32/drivers/cryptocop.c 
b/arch/cris/arch-v32/drivers/cryptocop.c
index 0068fd4..7444b45 100644
--- a/arch/cris/arch-v32/drivers/cryptocop.c
+++ b/arch/cris/arch-v32/drivers/cryptocop.c
@@ -2723,6 +2723,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
 noinpages,
 0,  /* read access only for in data */
 inpages,
+NULL,
 NULL);
 
if (err < 0) {
@@ -2737,6 +2738,7 @@ static int cryptocop_ioctl_process(struct inode *inode, 
struct file *filp, unsig
 nooutpages,
 FOLL_WRITE, /* write access for out data */
 outpages,
+NULL,
 NULL);
up_read(>mm->mmap_sem);
if (err < 0) {
diff --git a/arch/ia64/kernel/err_inject.c b/arch/ia64/kernel/err_inject.c
index 5ed0ea9..99f3fa2 100644
--- a/arch/ia64/kernel/err_inject.c
+++ b/arch/ia64/kernel/err_inject.c
@@ -142,7 +142,7 @@ store_virtual_to_phys(struct device *dev, struct 
device_attribute *attr,
u64 virt_addr=simple_strtoull(buf, NULL, 16);
int ret;
 
-   ret = get_user_pages(virt_addr, 1, FOLL_WRITE, NULL, NULL);
+   ret = get_user_pages(virt_addr, 1, FOLL_WRITE, NULL, NULL, NULL);
if (ret<=0) {
 #ifdef ERR_INJ_DEBUG
printk("Virtual address %lx is not existing.\n",virt_addr);
diff --git a/arch/x86/mm/mpx.c b/arch/x86/mm/mpx.c
index e4f8009..b74a7b2 100644
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -546,7 +546,7 @@ static int mpx_resolve_fault(long __user *addr, int write)
int nr_pages = 1;
 
gup_ret = get_user_pages((unsigned long)addr, nr_pages,
-   write ? FOLL_WRITE : 0, NULL, NULL);
+   write ? FOLL_WRITE : 0, NULL, NULL, NULL);
/*
 * get_user_pages() returns number of pages gotten.
 * 0 means we failed to fault in and get anything,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index dcaf691..3d9a195 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -584,7 +584,7 @@ int amdgpu_ttm_tt_get_user_pages(struct ttm_tt *ttm, struct 
page **pages)
list_add(, >guptasks);
spin_unlock(>guptasklock);
 
-   r = get_user_pages(userptr, 

[PATCH 2/2] mm: remove get_user_pages_locked()

2016-10-31 Thread Lorenzo Stoakes
get_user_pages() now has an int *locked parameter which renders
get_user_pages_locked() redundant, so remove it.

This patch should not introduce any functional changes.

Signed-off-by: Lorenzo Stoakes 
---
 include/linux/mm.h |  2 --
 mm/frame_vector.c  |  4 ++--
 mm/gup.c   | 56 +++---
 mm/nommu.c |  8 
 4 files changed, 22 insertions(+), 48 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 396984e..4db3147 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1278,8 +1278,6 @@ long get_user_pages_remote(struct task_struct *tsk, 
struct mm_struct *mm,
 long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
struct vm_area_struct **vmas, int *locked);
-long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-   unsigned int gup_flags, struct page **pages, int *locked);
 long __get_user_pages_unlocked(struct task_struct *tsk, struct mm_struct *mm,
   unsigned long start, unsigned long nr_pages,
   struct page **pages, unsigned int gup_flags);
diff --git a/mm/frame_vector.c b/mm/frame_vector.c
index db77dcb..c5ce1b1 100644
--- a/mm/frame_vector.c
+++ b/mm/frame_vector.c
@@ -55,8 +55,8 @@ int get_vaddr_frames(unsigned long start, unsigned int 
nr_frames,
if (!(vma->vm_flags & (VM_IO | VM_PFNMAP))) {
vec->got_ref = true;
vec->is_pfns = false;
-   ret = get_user_pages_locked(start, nr_frames,
-   gup_flags, (struct page **)(vec->ptrs), );
+   ret = get_user_pages(start, nr_frames,
+   gup_flags, (struct page **)(vec->ptrs), NULL, );
goto out;
}
 
diff --git a/mm/gup.c b/mm/gup.c
index 6b5539e..6cec693 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -826,37 +826,6 @@ static __always_inline long __get_user_pages_locked(struct 
task_struct *tsk,
 }
 
 /*
- * We can leverage the VM_FAULT_RETRY functionality in the page fault
- * paths better by using either get_user_pages_locked() or
- * get_user_pages_unlocked().
- *
- * get_user_pages_locked() is suitable to replace the form:
- *
- *  down_read(>mmap_sem);
- *  do_something()
- *  get_user_pages(tsk, mm, ..., pages, NULL, NULL);
- *  up_read(>mmap_sem);
- *
- *  to:
- *
- *  int locked = 1;
- *  down_read(>mmap_sem);
- *  do_something()
- *  get_user_pages_locked(tsk, mm, ..., pages, );
- *  if (locked)
- *  up_read(>mmap_sem);
- */
-long get_user_pages_locked(unsigned long start, unsigned long nr_pages,
-  unsigned int gup_flags, struct page **pages,
-  int *locked)
-{
-   return __get_user_pages_locked(current, current->mm, start, nr_pages,
-  pages, NULL, locked, true,
-  gup_flags | FOLL_TOUCH);
-}
-EXPORT_SYMBOL(get_user_pages_locked);
-
-/*
  * Same as get_user_pages_unlocked(, FOLL_TOUCH) but it allows to
  * pass additional gup_flags as last parameter (like FOLL_HWPOISON).
  *
@@ -954,11 +923,6 @@ EXPORT_SYMBOL(get_user_pages_unlocked);
  * use the correct cache flushing APIs.
  *
  * See also get_user_pages_fast, for performance critical applications.
- *
- * get_user_pages should be phased out in favor of
- * get_user_pages_locked|unlocked or get_user_pages_fast. Nothing
- * should use get_user_pages because it cannot pass
- * FAULT_FLAG_ALLOW_RETRY to handle_mm_fault.
  */
 long get_user_pages_remote(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, unsigned long nr_pages,
@@ -976,6 +940,26 @@ EXPORT_SYMBOL(get_user_pages_remote);
  * less-flexible calling convention where we assume that the task
  * and mm being operated on are the current task's.  We also
  * obviously don't pass FOLL_REMOTE in here.
+ *
+ * We can leverage the VM_FAULT_RETRY functionality in the page fault
+ * paths better by using either get_user_pages()'s locked parameter or
+ * get_user_pages_unlocked().
+ *
+ * get_user_pages()'s locked parameter is suitable to replace the form:
+ *
+ *  down_read(>mmap_sem);
+ *  do_something()
+ *  get_user_pages(tsk, mm, ..., pages, NULL, NULL);
+ *  up_read(>mmap_sem);
+ *
+ *  to:
+ *
+ *  int locked = 1;
+ *  down_read(>mmap_sem);
+ *  do_something()
+ *  get_user_pages(tsk, mm, ..., pages, NULL, );
+ *  if (locked)
+ *  up_read(>mmap_sem);
  */
 long get_user_pages(unsigned long start, unsigned long nr_pages,
unsigned int gup_flags, struct page **pages,
diff --git a/mm/nommu.c b/mm/nommu.c
index 82aaa33..3d38d40 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -168,14 +168,6 @@ long get_user_pages(unsigned long start, unsigned long 
nr_pages,
 }
 

[PATCH 0/2] mm: remove get_user_pages_locked()

2016-10-31 Thread Lorenzo Stoakes
by adding an int *locked parameter to get_user_pages() callers to this function
can now utilise VM_FAULT_RETRY functionality.

Taken in conjunction with the patch series adding the same parameter to
get_user_pages_remote() this means all slow-path get_user_pages*() functions
will now have the ability to utilise VM_FAULT_RETRY.

Additionally get_user_pages() and get_user_pages_remote() previously mirrored
one another in functionality differing only in the ability to specify task/mm,
this patch series reinstates this relationship.

This patch series should not introduce any functional changes.

Lorenzo Stoakes (2):
  mm: add locked parameter to get_user_pages()
  mm: remove get_user_pages_locked()

 arch/cris/arch-v32/drivers/cryptocop.c |  2 +
 arch/ia64/kernel/err_inject.c  |  2 +-
 arch/x86/mm/mpx.c  |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c|  2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c|  2 +-
 drivers/gpu/drm/via/via_dmablit.c  |  2 +-
 drivers/infiniband/core/umem.c |  2 +-
 drivers/infiniband/hw/mthca/mthca_memfree.c|  3 +-
 drivers/infiniband/hw/qib/qib_user_pages.c |  2 +-
 drivers/infiniband/hw/usnic/usnic_uiom.c   |  2 +-
 drivers/media/v4l2-core/videobuf-dma-sg.c  |  2 +-
 drivers/misc/mic/scif/scif_rma.c   |  1 +
 drivers/misc/sgi-gru/grufault.c|  3 +-
 drivers/platform/goldfish/goldfish_pipe.c  |  2 +-
 drivers/rapidio/devices/rio_mport_cdev.c   |  2 +-
 .../interface/vchiq_arm/vchiq_2835_arm.c   |  3 +-
 .../vc04_services/interface/vchiq_arm/vchiq_arm.c  |  3 +-
 drivers/virt/fsl_hypervisor.c  |  2 +-
 include/linux/mm.h |  4 +-
 mm/frame_vector.c  |  4 +-
 mm/gup.c   | 62 --
 mm/ksm.c   |  3 +-
 mm/mempolicy.c |  2 +-
 mm/nommu.c | 10 +---
 virt/kvm/kvm_main.c|  4 +-
 25 files changed, 55 insertions(+), 73 deletions(-)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3] staging : rtl8712: Free memory when kmalloc fails

2016-10-31 Thread Greg KH
On Mon, Oct 31, 2016 at 01:08:07PM +0530, Souptick Joarder wrote:
> On Sun, Oct 30, 2016 at 8:41 PM, Greg KH  wrote:
> > On Fri, Oct 28, 2016 at 10:37:53AM +0530, Souptick Joarder wrote:
> >> There are few functions where we need to free previously allocated memory
> >> when kmalloc fails. Else it may lead to memory leakage. In  
> >> _init_cmd_priv()
> >> and _r8712_init_xmit_priv(), in few places we are not freeing previously
> >> allocated memory when kmalloc fails.
> >>
> >> Signed-off-by: Souptick joarder 
> >> ---
> >> v2:
> >>   - Make the commit message more descriptive
> >> v3:
> >>   - Modified the patch description by adding space in required places
> >>
> >>  drivers/staging/rtl8712/rtl871x_cmd.c  | 4 +++-
> >>  drivers/staging/rtl8712/rtl871x_xmit.c | 4 +++-
> >>  2 files changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c 
> >> b/drivers/staging/rtl8712/rtl871x_cmd.c
> >> index b7ee5e6..1f284ae 100644
> >> --- a/drivers/staging/rtl8712/rtl871x_cmd.c
> >> +++ b/drivers/staging/rtl8712/rtl871x_cmd.c
> >> @@ -72,8 +72,10 @@ static sint _init_cmd_priv(struct cmd_priv *pcmdpriv)
> >>   ((addr_t)(pcmdpriv->cmd_allocated_buf) &
> >>   (CMDBUFF_ALIGN_SZ - 1));
> >>   pcmdpriv->rsp_allocated_buf = kmalloc(MAX_RSPSZ + 4, GFP_ATOMIC);
> >> - if (!pcmdpriv->rsp_allocated_buf)
> >> + if (!pcmdpriv->rsp_allocated_buf) {
> >> + kfree(pcmdpriv->cmd_allocated_buf);
> >> + pcmdpriv->cmd_allocated_buf = NULL;
> >>   return _FAIL;
> >> + }
> >>   pcmdpriv->rsp_buf = pcmdpriv->rsp_allocated_buf  +  4 -
> >>   ((addr_t)(pcmdpriv->rsp_allocated_buf) & 3);
> >>   pcmdpriv->cmd_issued_cnt = 0;
> >> diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c 
> >> b/drivers/staging/rtl8712/rtl871x_xmit.c
> >> index be38364..f335161 100644
> >> --- a/drivers/staging/rtl8712/rtl871x_xmit.c
> >> +++ b/drivers/staging/rtl8712/rtl871x_xmit.c
> >> @@ -128,8 +128,10 @@ sint _r8712_init_xmit_priv(struct xmit_priv 
> >> *pxmitpriv,
> >>   _init_queue(>pending_xmitbuf_queue);
> >>   pxmitpriv->pallocated_xmitbuf = kmalloc(NR_XMITBUFF * sizeof(struct 
> >> xmit_buf) + 4,
> >>   GFP_ATOMIC);
> >> - if (!pxmitpriv->pallocated_xmitbuf)
> >> + if (!pxmitpriv->pallocated_xmitbuf) {
> >> + kfree(pxmitpriv->pallocated_frame_buf);
> >> + pxmitpriv->pallocated_frame_buf = NULL;
> >>   return _FAIL;
> >> + }
> >>   pxmitpriv->pxmitbuf = pxmitpriv->pallocated_xmitbuf + 4 -
> >> ((addr_t)(pxmitpriv->pallocated_xmitbuf) & 3);
> >>   pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
> >> --
> >> 1.9.1
> >
> > This patch is totally corrupted, how did you generate it?
> 
> I edited the patch file.
> 
> > You can't hand-edit a patch file, unless you know how to do it...
> 
> I am not aware of it. Is there any guidelines to hand-edit a patch file?

If you don't know how to do that, never do it.  You created something
that no one can use, try it yourself to see.

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


[PATCH v3] staging : rtl8712: Free memory when kmalloc fails

2016-10-31 Thread Souptick Joarder
There are few functions where we need to free previously allocated
memory when kmalloc fails. Else it may lead to memory leakage. In
_init_cmd_priv() and _r8712_init_xmit_priv(), in few places we are
not freeing previously allocated memory when kmalloc fails.

Signed-off-by: Souptick joarder 
---
v2:
  - Make the commit message more descriptive
v3:
  - Modified the patch description by adding space in required places

 drivers/staging/rtl8712/rtl871x_cmd.c  | 5 -
 drivers/staging/rtl8712/rtl871x_xmit.c | 5 -
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c 
b/drivers/staging/rtl8712/rtl871x_cmd.c
index b7ee5e6..04638f1 100644
--- a/drivers/staging/rtl8712/rtl871x_cmd.c
+++ b/drivers/staging/rtl8712/rtl871x_cmd.c
@@ -72,8 +72,11 @@ static sint _init_cmd_priv(struct cmd_priv *pcmdpriv)
((addr_t)(pcmdpriv->cmd_allocated_buf) &
(CMDBUFF_ALIGN_SZ - 1));
pcmdpriv->rsp_allocated_buf = kmalloc(MAX_RSPSZ + 4, GFP_ATOMIC);
-   if (!pcmdpriv->rsp_allocated_buf)
+   if (!pcmdpriv->rsp_allocated_buf) {
+   kfree(pcmdpriv->cmd_allocated_buf);
+   pcmdpriv->cmd_allocated_buf = NULL;
return _FAIL;
+   }
pcmdpriv->rsp_buf = pcmdpriv->rsp_allocated_buf  +  4 -
((addr_t)(pcmdpriv->rsp_allocated_buf) & 3);
pcmdpriv->cmd_issued_cnt = 0;
diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c 
b/drivers/staging/rtl8712/rtl871x_xmit.c
index be38364..484d2f2 100644
--- a/drivers/staging/rtl8712/rtl871x_xmit.c
+++ b/drivers/staging/rtl8712/rtl871x_xmit.c
@@ -128,8 +128,11 @@ sint _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
_init_queue(>pending_xmitbuf_queue);
pxmitpriv->pallocated_xmitbuf = kmalloc(NR_XMITBUFF * sizeof(struct 
xmit_buf) + 4,
GFP_ATOMIC);
-   if (!pxmitpriv->pallocated_xmitbuf)
+   if (!pxmitpriv->pallocated_xmitbuf) {
+   kfree(pxmitpriv->pallocated_frame_buf);
+   pxmitpriv->pallocated_frame_buf = NULL;
return _FAIL;
+   }
pxmitpriv->pxmitbuf = pxmitpriv->pallocated_xmitbuf + 4 -
  ((addr_t)(pxmitpriv->pallocated_xmitbuf) & 3);
pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
-- 
1.9.1

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


[PATCH] Staging:emxx_udc:emxx_udc: Compression of lines for immediate return

2016-10-31 Thread Nadim Almas
This patch compresses two lines into a single line
if immediate return statement is found. Remove variable data as
it is no longer needed.

It is done using script Coccinelle. And coccinelle uses the following
semantic patch for this compression function

@@
local idexpression ret;
expression e;
@@

-ret =
+return
 e;
-return ret;

Signed-off-by: Nadim Almas 
---
 drivers/staging/emxx_udc/emxx_udc.c   | 4 +---
 
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c 
b/drivers/staging/emxx_udc/emxx_udc.c
index abe2aaf..31f4206 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -2979,9 +2979,7 @@ static int nbu2ss_gad_get_frame(struct usb_gadget 
*pgadget)
if (data == 0)
return -EINVAL;
 
-   data = _nbu2ss_readl(>p_regs->USB_ADDRESS) & FRAME;
-
-   return data;
+   return _nbu2ss_readl(>p_regs->USB_ADDRESS) & FRAME;
 }
 
 
2.7.4

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


[PATCH] staging: vc04_services: add vchiq_pagelist_info structure

2016-10-31 Thread Michael Zoran
The current dma_map_sg based implementation for bulk messages
computes many offsets into a single allocation multiple times in
both the create and free code paths.  This is inefficient,
error prone and in fact still has a few lingering issues
with arm64.

This change replaces a small portion of that inplementation with
new code that uses a new struct vchiq_pagelist_info to store the
needed information rather then complex offset calculations.

This improved implementation should be more efficient and easier
to understand and maintain.

Tests Run(Both Pass):
vchiq_test -p 1
vchiq_test -f 10

Signed-off-by: Michael Zoran 
---
 .../interface/vchiq_arm/vchiq_2835_arm.c   | 223 +++--
 1 file changed, 113 insertions(+), 110 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
index 12938f2..a297d89 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_2835_arm.c
@@ -62,6 +62,18 @@ typedef struct vchiq_2835_state_struct {
VCHIQ_ARM_STATE_T arm_state;
 } VCHIQ_2835_ARM_STATE_T;
 
+struct vchiq_pagelist_info {
+   PAGELIST_T *pagelist;
+   size_t pagelist_buffer_size;
+   dma_addr_t dma_addr;
+   enum dma_data_direction dma_dir;
+   unsigned int num_pages;
+   unsigned int pages_need_release;
+   struct page **pages;
+   struct scatterlist *scatterlist;
+   unsigned int scatterlist_mapped;
+};
+
 static void __iomem *g_regs;
 static unsigned int g_cache_line_size = sizeof(CACHE_LINE_SIZE);
 static unsigned int g_fragments_size;
@@ -77,13 +89,13 @@ static DEFINE_SEMAPHORE(g_free_fragments_mutex);
 static irqreturn_t
 vchiq_doorbell_irq(int irq, void *dev_id);
 
-static int
+static struct vchiq_pagelist_info *
 create_pagelist(char __user *buf, size_t count, unsigned short type,
-   struct task_struct *task, PAGELIST_T **ppagelist,
-   dma_addr_t *dma_addr);
+   struct task_struct *task);
 
 static void
-free_pagelist(dma_addr_t dma_addr, PAGELIST_T *pagelist, int actual);
+free_pagelist(struct vchiq_pagelist_info *pagelistinfo,
+ int actual);
 
 int vchiq_platform_init(struct platform_device *pdev, VCHIQ_STATE_T *state)
 {
@@ -226,29 +238,27 @@ VCHIQ_STATUS_T
 vchiq_prepare_bulk_data(VCHIQ_BULK_T *bulk, VCHI_MEM_HANDLE_T memhandle,
void *offset, int size, int dir)
 {
-   PAGELIST_T *pagelist;
-   int ret;
-   dma_addr_t dma_addr;
+   struct vchiq_pagelist_info *pagelistinfo;
 
WARN_ON(memhandle != VCHI_MEM_HANDLE_INVALID);
 
-   ret = create_pagelist((char __user *)offset, size,
-   (dir == VCHIQ_BULK_RECEIVE)
-   ? PAGELIST_READ
-   : PAGELIST_WRITE,
-   current,
-   ,
-   _addr);
+   pagelistinfo = create_pagelist((char __user *)offset, size,
+  (dir == VCHIQ_BULK_RECEIVE)
+  ? PAGELIST_READ
+  : PAGELIST_WRITE,
+  current);
 
-   if (ret != 0)
+   if (!pagelistinfo)
return VCHIQ_ERROR;
 
bulk->handle = memhandle;
-   bulk->data = (void *)(unsigned long)dma_addr;
+   bulk->data = (void *)(unsigned long)pagelistinfo->dma_addr;
 
-   /* Store the pagelist address in remote_data, which isn't used by the
-  slave. */
-   bulk->remote_data = pagelist;
+   /*
+* Store the pagelistinfo address in remote_data,
+* which isn't used by the slave.
+*/
+   bulk->remote_data = pagelistinfo;
 
return VCHIQ_SUCCESS;
 }
@@ -257,8 +267,8 @@ void
 vchiq_complete_bulk(VCHIQ_BULK_T *bulk)
 {
if (bulk && bulk->remote_data && bulk->actual)
-   free_pagelist((dma_addr_t)(unsigned long)bulk->data,
- (PAGELIST_T *)bulk->remote_data, bulk->actual);
+   free_pagelist((struct vchiq_pagelist_info *)bulk->remote_data,
+ bulk->actual);
 }
 
 void
@@ -346,6 +356,25 @@ vchiq_doorbell_irq(int irq, void *dev_id)
return ret;
 }
 
+static void
+cleaup_pagelistinfo(struct vchiq_pagelist_info *pagelistinfo)
+{
+   if (pagelistinfo->scatterlist_mapped) {
+   dma_unmap_sg(g_dev, pagelistinfo->scatterlist,
+pagelistinfo->num_pages, pagelistinfo->dma_dir);
+   }
+
+   if (pagelistinfo->pages_need_release) {
+   unsigned int i;
+
+   for (i = 0; i < pagelistinfo->num_pages; i++)
+   put_page(pagelistinfo->pages[i]);
+   }
+
+   dma_free_coherent(g_dev, pagelistinfo->pagelist_buffer_size,
+ pagelistinfo->pagelist, 

Re: [PATCH v3] staging: iio: cdc: ad7746: add additional config defines

2016-10-31 Thread Eva Rachel Retuya
On Mon, Oct 31, 2016 at 03:49:01PM +0800, Eva Rachel Retuya wrote:
> On Sun, Oct 30, 2016 at 06:49:00PM +, Jonathan Cameron wrote:
> > On 30/10/16 17:46, Lars-Peter Clausen wrote:
> > > On 10/30/2016 06:41 PM, Jonathan Cameron wrote:
> > >> On 28/10/16 09:26, Eva Rachel Retuya wrote:
> > >>> Introduce defines for shifting and mask under the config register for
> > >>> better readability. Also, introduce helper variables for index
> > >>> calculation.
> > >>>
> > >>> Signed-off-by: Eva Rachel Retuya 
> > >> Looks good to me.
> > >>
> > >> Lars could you sanity check this one as well?
> > > 
> > > Acked-by: Lars-Peter Clausen 
> > There was a bit of fun applying this.  Eva, can you check I didn't mess it
> > up?
> 
> It looks fine to me. Thanks!
> 

Have to retract that statement, I rebased and went through the merge
conflict. Extra whitespace can be seen after the statement in line 644:
*val = ad7746_cap_filter_rate_table[idx][0];

What to do?
Eva

> > 
> > Applied to the togreg branch of iio.git.  Initially pushed out as testing
> > for the autobuilders to play with it. (and this time I remembered to 
> > actually
> > do the push!)
> > 
> > thanks,
> > 
> > Jonathan
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> > > the body of a message to majord...@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3] staging: iio: cdc: ad7746: add additional config defines

2016-10-31 Thread Eva Rachel Retuya
On Sun, Oct 30, 2016 at 06:49:00PM +, Jonathan Cameron wrote:
> On 30/10/16 17:46, Lars-Peter Clausen wrote:
> > On 10/30/2016 06:41 PM, Jonathan Cameron wrote:
> >> On 28/10/16 09:26, Eva Rachel Retuya wrote:
> >>> Introduce defines for shifting and mask under the config register for
> >>> better readability. Also, introduce helper variables for index
> >>> calculation.
> >>>
> >>> Signed-off-by: Eva Rachel Retuya 
> >> Looks good to me.
> >>
> >> Lars could you sanity check this one as well?
> > 
> > Acked-by: Lars-Peter Clausen 
> There was a bit of fun applying this.  Eva, can you check I didn't mess it
> up?

It looks fine to me. Thanks!

> 
> Applied to the togreg branch of iio.git.  Initially pushed out as testing
> for the autobuilders to play with it. (and this time I remembered to actually
> do the push!)
> 
> thanks,
> 
> Jonathan
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v3] staging : rtl8712: Free memory when kmalloc fails

2016-10-31 Thread Souptick Joarder
On Sun, Oct 30, 2016 at 8:41 PM, Greg KH  wrote:
> On Fri, Oct 28, 2016 at 10:37:53AM +0530, Souptick Joarder wrote:
>> There are few functions where we need to free previously allocated memory
>> when kmalloc fails. Else it may lead to memory leakage. In  _init_cmd_priv()
>> and _r8712_init_xmit_priv(), in few places we are not freeing previously
>> allocated memory when kmalloc fails.
>>
>> Signed-off-by: Souptick joarder 
>> ---
>> v2:
>>   - Make the commit message more descriptive
>> v3:
>>   - Modified the patch description by adding space in required places
>>
>>  drivers/staging/rtl8712/rtl871x_cmd.c  | 4 +++-
>>  drivers/staging/rtl8712/rtl871x_xmit.c | 4 +++-
>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8712/rtl871x_cmd.c 
>> b/drivers/staging/rtl8712/rtl871x_cmd.c
>> index b7ee5e6..1f284ae 100644
>> --- a/drivers/staging/rtl8712/rtl871x_cmd.c
>> +++ b/drivers/staging/rtl8712/rtl871x_cmd.c
>> @@ -72,8 +72,10 @@ static sint _init_cmd_priv(struct cmd_priv *pcmdpriv)
>>   ((addr_t)(pcmdpriv->cmd_allocated_buf) &
>>   (CMDBUFF_ALIGN_SZ - 1));
>>   pcmdpriv->rsp_allocated_buf = kmalloc(MAX_RSPSZ + 4, GFP_ATOMIC);
>> - if (!pcmdpriv->rsp_allocated_buf)
>> + if (!pcmdpriv->rsp_allocated_buf) {
>> + kfree(pcmdpriv->cmd_allocated_buf);
>> + pcmdpriv->cmd_allocated_buf = NULL;
>>   return _FAIL;
>> + }
>>   pcmdpriv->rsp_buf = pcmdpriv->rsp_allocated_buf  +  4 -
>>   ((addr_t)(pcmdpriv->rsp_allocated_buf) & 3);
>>   pcmdpriv->cmd_issued_cnt = 0;
>> diff --git a/drivers/staging/rtl8712/rtl871x_xmit.c 
>> b/drivers/staging/rtl8712/rtl871x_xmit.c
>> index be38364..f335161 100644
>> --- a/drivers/staging/rtl8712/rtl871x_xmit.c
>> +++ b/drivers/staging/rtl8712/rtl871x_xmit.c
>> @@ -128,8 +128,10 @@ sint _r8712_init_xmit_priv(struct xmit_priv *pxmitpriv,
>>   _init_queue(>pending_xmitbuf_queue);
>>   pxmitpriv->pallocated_xmitbuf = kmalloc(NR_XMITBUFF * sizeof(struct 
>> xmit_buf) + 4,
>>   GFP_ATOMIC);
>> - if (!pxmitpriv->pallocated_xmitbuf)
>> + if (!pxmitpriv->pallocated_xmitbuf) {
>> + kfree(pxmitpriv->pallocated_frame_buf);
>> + pxmitpriv->pallocated_frame_buf = NULL;
>>   return _FAIL;
>> + }
>>   pxmitpriv->pxmitbuf = pxmitpriv->pallocated_xmitbuf + 4 -
>> ((addr_t)(pxmitpriv->pallocated_xmitbuf) & 3);
>>   pxmitbuf = (struct xmit_buf *)pxmitpriv->pxmitbuf;
>> --
>> 1.9.1
>
> This patch is totally corrupted, how did you generate it?

I edited the patch file.

> You can't hand-edit a patch file, unless you know how to do it...

I am not aware of it. Is there any guidelines to hand-edit a patch file?
>
> Please fix up and resend in a format I can apply it in.

I will fix and send it to you.
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 01/10] staging: iio: tsl2583: add of_match table for device tree support

2016-10-31 Thread Rob Herring
On Fri, Oct 28, 2016 at 06:00:12AM -0400, Brian Masney wrote:
> Add device tree support for the tsl2583 IIO driver with no custom
> properties.
> 
> Signed-off-by: Brian Masney 
> ---
>  .../devicetree/bindings/iio/light/tsl2583.txt  | 26 
> ++
>  drivers/staging/iio/light/tsl2583.c| 13 +++
>  2 files changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/tsl2583.txt

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