Re: [PATCH] staging:rtl8188eu:hal Fix wrong comparison to False

2017-09-21 Thread Dan Carpenter 
On Thu, Sep 21, 2017 at 11:09:55AM +0530, Janani Sankara Babu wrote:
> This patch solves the warning "Using comparison to false is error prone"

Out of curiosity, which tool complains about that?

regards,
dan carpenter

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

[PATCH] staging: vc04_services: remove BCM2835_VCHIQ_SUPPORT_MEMDUMP

2017-09-21 Thread Dan Carpenter 
BCM2835_VCHIQ_SUPPORT_MEMDUMP lets you look through any user memory.
That's too big of an information leak from a security perspective.  The
debugging dumps need to be more specific to this driver.

Signed-off-by: Dan Carpenter 
---
It's possible I have misunderstood.  Untested.

diff --git a/drivers/staging/vc04_services/Kconfig 
b/drivers/staging/vc04_services/Kconfig
index 9e2763663ab8..f5aaf7d629f0 100644
--- a/drivers/staging/vc04_services/Kconfig
+++ b/drivers/staging/vc04_services/Kconfig
@@ -19,18 +19,6 @@ config BCM2835_VCHIQ
Defaults to Y when the Broadcom Videocore services
are included in the build, N otherwise.
 
-if BCM2835_VCHIQ
-
-config BCM2835_VCHIQ_SUPPORT_MEMDUMP
-   bool "Support dumping memory contents to debug log"
-   help
-   BCM2835 VCHIQ supports the ability to dump the
-   contents of memory to the debug log.  This
-   is typically only needed by diagnostic tools used
-   to debug issues with VideoCore.
-
-endif
-
 source "drivers/staging/vc04_services/bcm2835-audio/Kconfig"
 
 source "drivers/staging/vc04_services/bcm2835-camera/Kconfig"
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 314ffac50bb8..d23152bb1379 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
@@ -195,11 +195,6 @@ static const char *const ioctl_names[] = {
 vchiq_static_assert(ARRAY_SIZE(ioctl_names) ==
(VCHIQ_IOC_MAX + 1));
 
-#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
-static void
-dump_phys_mem(void *virt_addr, u32 num_bytes);
-#endif
-
 /
 *
 *   add_completion
@@ -1161,20 +1156,6 @@ vchiq_ioctl(struct file *file, unsigned int cmd, 
unsigned long arg)
args.handle, args.option, args.value);
} break;
 
-#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
-   case VCHIQ_IOC_DUMP_PHYS_MEM: {
-   VCHIQ_DUMP_MEM_T  args;
-
-   if (copy_from_user
-(&args, (const void __user *)arg,
- sizeof(args)) != 0) {
-   ret = -EFAULT;
-   break;
-   }
-   dump_phys_mem(args.virt_addr, args.num_bytes);
-   } break;
-#endif
-
case VCHIQ_IOC_LIB_VERSION: {
unsigned int lib_version = (unsigned int)arg;
 
@@ -1654,42 +1635,6 @@ vchiq_compat_ioctl_get_config(struct file *file,
return vchiq_ioctl(file, VCHIQ_IOC_GET_CONFIG, (unsigned long)args);
 }
 
-#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
-
-struct vchiq_dump_mem32 {
-   compat_uptr_t virt_addr;
-   u32 num_bytes;
-};
-
-#define VCHIQ_IOC_DUMP_PHYS_MEM32 \
-   _IOW(VCHIQ_IOC_MAGIC, 15, struct vchiq_dump_mem32)
-
-static long
-vchiq_compat_ioctl_dump_phys_mem(struct file *file,
-unsigned int cmd,
-unsigned long arg)
-{
-   VCHIQ_DUMP_MEM_T *args;
-   struct vchiq_dump_mem32 args32;
-
-   args = compat_alloc_user_space(sizeof(*args));
-   if (!args)
-   return -EFAULT;
-
-   if (copy_from_user(&args32,
-  (struct vchiq_dump_mem32 *)arg,
-  sizeof(args32)))
-   return -EFAULT;
-
-   if (put_user(compat_ptr(args32.virt_addr), &args->virt_addr) ||
-   put_user(args32.num_bytes, &args->num_bytes))
-   return -EFAULT;
-
-   return vchiq_ioctl(file, VCHIQ_IOC_DUMP_PHYS_MEM, (unsigned long)args);
-}
-
-#endif
-
 static long
 vchiq_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
 {
@@ -1707,10 +1652,6 @@ vchiq_compat_ioctl(struct file *file, unsigned int cmd, 
unsigned long arg)
return vchiq_compat_ioctl_dequeue_message(file, cmd, arg);
case VCHIQ_IOC_GET_CONFIG32:
return vchiq_compat_ioctl_get_config(file, cmd, arg);
-#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
-   case VCHIQ_IOC_DUMP_PHYS_MEM32:
-   return vchiq_compat_ioctl_dump_phys_mem(file, cmd, arg);
-#endif
default:
return vchiq_ioctl(file, cmd, arg);
}
@@ -2050,98 +1991,6 @@ vchiq_dump_platform_service_state(void *dump_context, 
VCHIQ_SERVICE_T *service)
 
 /
 *
-*   dump_user_mem
-*
-***/
-
-#if defined(CONFIG_BCM2835_VCHIQ_SUPPORT_MEMDUMP)
-
-static void
-dump_phys_mem(void *virt_addr, u32 num_bytes)
-{
-   intrc;
-   u8*end_virt_addr = virt_addr + num_bytes;
-   intnum_pages;
-   intoffset;
-   intend_offs

Re: [PATCH] staging:rtl8188eu Remove unnecessary {} braces in

2017-09-21 Thread Dan Carpenter 
On Thu, Sep 21, 2017 at 12:18:04PM +0530, Janani Sankara Babu wrote:
> --- a/drivers/staging/rtl8188eu/hal/phy.c
> +++ b/drivers/staging/rtl8188eu/hal/phy.c
> @@ -728,9 +728,9 @@ static void patha_fill_iqk(struct adapter *adapt, bool 
> iqkok, s32 result[][8],
>   u32 oldval_0, x, tx0_a, reg;
>   s32 y, tx0_c;
>  
> - if (final_candidate == 0xFF) {
> + if (final_candidate == 0xFF)
>   return;
> - } else if (iqkok) {
> + else if (iqkok) {

No.  These ones stay.  Your change would introduce a new checkpatch.pl
warning if you ran it against the patched file.  The rule here is that
if one side of the if else has curly braces then both sides get them.

regards,
dan carpenter

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

Re: [PATCH] android: binder: fix type mismatch warning

2017-09-21 Thread Greg Kroah-Hartman 
On Wed, Sep 20, 2017 at 01:37:45PM +, Arnd Bergmann wrote:
> On Wed, Sep 20, 2017 at 12:24 PM, Martijn Coenen  wrote:
> > On Wed, Sep 20, 2017 at 11:58 AM, Arnd Bergmann  wrote:
> >>
> >> - Since you say there are existing users of recent 32-bit Android
> >>   including Oreo, I also think that removing support for the v7 ABI
> >>   is no longer an option. I only made that suggestion under the
> >>   assumption that there is no user space requiring that. Linux
> >>   has no real way of "deprecating" an existing ABI, either it is
> >>   currently used and has to stay around, or it is not used at all
> >>   and can be removed without anybody noticing.
> >
> > I don't know of any Android devices shipping with 4.14 - we don't even
> > have a common tree for 4.14 (4.9 is the latest). So I don't think
> > we're hurting anyone in the Android ecosystem if we remove v7 from
> > 4.14. From what I know, it's extremely rare for an Android device to
> > change their kernel version after a device ships, but even if someone
> > hypothetically did update their Android device to use 4.14, they can
> > pretty easily switch the build-time config option. I understand that
> > this is against Linux' philosophy around not breaking userspace, I
> > just want to point out that I think from an Android POV, removing v7
> > from 4.14 is not an issue. I'm not sure if that is good enough.
> 
> I'm not really worried about shipping Android products, for those
> there is no big problem using the compile-time option as they build
> everything together.
> 
> The case that gets interesting is a any kind of user that wants to
> run an Android application on a regular Linux box without
> using virtual machines or emulation, e.g. a an app developer,
> or a user that wants to run some Android app on their desktop
> using anbox.
> 
> This obviously requires enabling the module, but it should not
> require enabling an option to support one version of the user
> space while breaking another version.

To be fair, lots of people, including myself, have always said, "never
run binder on a system that is not a 'real' Android system" for a
variety of good reasons, including security issues.

Now around the 4.10 or so kernel release, most of those issues were
resolved, and with 4.14, all of those have been taken care of (that I
know of), so this should be allowed.  But given that no one has done it
in the past (or should have), I don't know how hard you want to support
"older" situations at all.

> >> If we end up doing a runtime switch between the ABIs instead,
> >> I see two ways of doing that:
> >>
> >> The easiest implementation would make it a module parameter:
> >> Since there is only one instance of the binder in any given
> >> system, and presumably all processes talking to it have to use
> >> the same ABI, this should be sufficient. The main downside is
> >> that it prevents us from having incompatible versions per
> >> namespace if we ever get a containerized version of binder.
> >
> > Namespace support for binder is currently being investigated, but I'm
> > not sure we'd ever have a system where we want to mix v7 and v8. There
> > really is no good reason to use v7 anymore (even on 32-bit mahcines),
> > we just should have removed it from our userspace a lot earlier.
> 
> The only possible reason for v7 is backwards compatibility. I agree
> we don't need a single machine (or container) to support a mix of
> v7 and v8 applications, but I can see someone using a v7 based
> chroot user space today to keep running that in a container in the
> future while using another container for an updated image.
> 
> This is clearly a corner case that we could decide to ignore, it
> just feels like a bit of a hack.

I think it's a corner case that no one will actually run into at all.

Remember, it's only libbinder that is touching the binder device node,
not random applications, so it's not a matter of an application in a
container, but rather, a whole "system" there.  And running whole
"systems" in different containers is not really viable from what I can
tell (but I could be wrong.)

> >> The correct way to do it would be to unify the two versions of
> >> the ABI so they can be used interchangeably: any 32-bit
> >> process would start out with the v7 interface and a 64-bit
> >> process would start out with the v8 interface
> >
> > This wouldn't work with the current implementation - if a 32-bit and
> > 64-bit process communicate, then userspace would make wrong
> > assumptions about the sizes of transactions. Or is this what you're
> > proposing to fix?
> 
> The scenario I had in mind is like this:
> 
> - Any 64-bit process would use the v8 ABI all the time. When the binder
>   device has been opened by a 64-bit process already, this forces
>   all other processes opening it to also use v8.
> 
> - An existing 32-bit process would keep using the v7 ABI, but as long
>   as the the v7 interface is in use, no other process may use the v8 ABI.
>   This wou

Re: [PATCH 02/16] hyper-v: trace vmbus_on_message()

2017-09-21 Thread Vitaly Kuznetsov 
Steven Rostedt  writes:

> On Wed, 20 Sep 2017 19:21:53 +0200
> Vitaly Kuznetsov  wrote:
>
>> diff --git a/drivers/hv/hv_trace.h b/drivers/hv/hv_trace.h
>> index 9a29ef55477d..72911dfc9682 100644
>> --- a/drivers/hv/hv_trace.h
>> +++ b/drivers/hv/hv_trace.h
>> @@ -14,6 +14,14 @@ TRACE_EVENT(vmbus_on_msg_dpc,
>>  TP_printk("message %u received", __entry->msgtype)
>>  );
>>  
>> +TRACE_EVENT(vmbus_on_message,
>> +TP_PROTO(const struct vmbus_channel_message_header *hdr),
>> +TP_ARGS(hdr),
>> +TP_STRUCT__entry(__field(unsigned int, msgtype)),
>> +TP_fast_assign(__entry->msgtype = hdr->msgtype),
>> +TP_printk("processing message %u", __entry->msgtype)
>> +);
>
> Whenever you have two trace events with everything the same but the
> TP_printk(), you can save a little space by using DEFINE_EVENT_PRINT()
>
> DECLARE_EVENT_CLASS(vmbus_hdr_msg,
>   TP_PROTO(const struct vmbus_channel_message_header *hdr),
>   TP_ARGS(hdr),
>   TP_STRUCT__entry(__field(unsigned int, msgtype),
>   TP_fast_assign(__entry->msg = hdr->msgtype;),
>   TP_printk("msgtype=%d", __entry->msgtype)
> );
>
> DEFINE_EVENT_PRINT(vmbus_hdr_msg, vmbus_on_msg_dpc,
>   TP_PROTO(const struct vmbus_channel_message_header *hdr),
>   TP_ARGS(hdr),
>   TP_printk("message %u received", __entry->msgtype));
>
> DEFINE_EVENT_PRINT(vmbus_hdr_msg, vmbus_on_message,
>   TP_PROTO(const struct vmbus_channel_message_header *hdr),
>   TP_ARGS(hdr),
>   TP_printk("processing message %u", __entry->msgtype));
>
> This will use the same functions required to save and record the
> message but will use a different function to output it to the trace.

Oh, thanks! This seems to be useful for
vmbus_on_msg_dpc/vmbus_on_message only as all the rest needs to parse
different structures. Will use it in v2.

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

Re: [PATCH v2 26/31] staging/comedi/das16: Make timer initialization unconditional

2017-09-21 Thread Ian Abbott 

On 21/09/17 00:27, Kees Cook wrote:

With timer initialization made unconditional, there is no reason to
make del_timer_sync() calls conditionally, there by removing the test
of the .data field.

Cc: Ian Abbott 
Cc: H Hartley Sweeten 
Cc: Greg Kroah-Hartman 
Cc: de...@driverdev.osuosl.org
Signed-off-by: Kees Cook 
---
  drivers/staging/comedi/drivers/das16.c | 10 --
  1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16.c 
b/drivers/staging/comedi/drivers/das16.c
index 5d157951f63f..2b2a446af3f5 100644
--- a/drivers/staging/comedi/drivers/das16.c
+++ b/drivers/staging/comedi/drivers/das16.c
@@ -934,6 +934,9 @@ static void das16_alloc_dma(struct comedi_device *dev, 
unsigned int dma_chan)
  {
struct das16_private_struct *devpriv = dev->private;
  
+	setup_timer(&devpriv->timer, das16_timer_interrupt,

+   (unsigned long)dev);
+
/* only DMA channels 3 and 1 are valid */
if (!(dma_chan == 1 || dma_chan == 3))
return;
@@ -941,10 +944,6 @@ static void das16_alloc_dma(struct comedi_device *dev, 
unsigned int dma_chan)
/* DMA uses two buffers */
devpriv->dma = comedi_isadma_alloc(dev, 2, dma_chan, dma_chan,
   DAS16_DMA_SIZE, COMEDI_ISADMA_READ);
-   if (devpriv->dma) {
-   setup_timer(&devpriv->timer, das16_timer_interrupt,
-   (unsigned long)dev);
-   }
  }
  
  static void das16_free_dma(struct comedi_device *dev)

@@ -952,8 +951,7 @@ static void das16_free_dma(struct comedi_device *dev)
struct das16_private_struct *devpriv = dev->private;
  
  	if (devpriv) {

-   if (devpriv->timer.data)
-   del_timer_sync(&devpriv->timer);
+   del_timer_sync(&devpriv->timer);
comedi_isadma_free(devpriv->dma);
}
  }



That's fine.

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] staging:rtl8188eu Remove unnecessary {} braces in

2017-09-21 Thread Joe Perches 
On Thu, 2017-09-21 at 10:15 +0300, Dan Carpenter wrote:
> On Thu, Sep 21, 2017 at 12:18:04PM +0530, Janani Sankara Babu wrote:
> > --- a/drivers/staging/rtl8188eu/hal/phy.c
> > +++ b/drivers/staging/rtl8188eu/hal/phy.c
> > @@ -728,9 +728,9 @@ static void patha_fill_iqk(struct adapter *adapt, bool 
> > iqkok, s32 result[][8],
> > u32 oldval_0, x, tx0_a, reg;
> > s32 y, tx0_c;
> >  
> > -   if (final_candidate == 0xFF) {
> > +   if (final_candidate == 0xFF)
> > return;
> > -   } else if (iqkok) {
> > +   else if (iqkok) {
> 
> No.  These ones stay.  Your change would introduce a new checkpatch.pl
> warning if you ran it against the patched file.  The rule here is that
> if one side of the if else has curly braces then both sides get them.

And the else could be removed

if (final_candidate == 0xff)
return;

if (iqkok) {
[etc...]

and the code should probably be

if (final_candidate == 0xff)
return;

if (!iqkok)
return;

[unindented etc...]

or combine the first 2 tests

if (final_candidate == 0xff || !iqkok)
return;

[unindented etc...]

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

[PATCH] staging: ccree: Convert to platform_{get,set}_drvdata()

2017-09-21 Thread sunil . m 
From: Suniel Mahesh 

Platform devices are expected to use wrapper functions,
platform_{get,set}_drvdata() with platform_device as argument,
for getting and setting the driver data. dev_{get,set}_drvdata()
are using &plat_dev->dev.
For wrapper functions we can directly pass a struct platform_device.

dev_set_drvdata() is redundant and therefore removed. The driver core
clears the driver data to NULL after device_release or on probe failure.

Signed-off-by: Suniel Mahesh 
---
Note:
- Patch was tested and built(ARCH=arm) on next-20170921.
  No build issues reported, however it was not tested on
  real hardware.
---
 drivers/staging/ccree/ssi_driver.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index 6d16220..53b4a8c 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -236,7 +236,7 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
rc = -ENOMEM;
goto post_drvdata_err;
}
-   dev_set_drvdata(&plat_dev->dev, new_drvdata);
+   platform_set_drvdata(plat_dev, new_drvdata);
new_drvdata->plat_dev = plat_dev;
 
new_drvdata->clk = of_clk_get(np, 0);
@@ -415,7 +415,6 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
cc_clk_off(new_drvdata);
 post_drvdata_err:
SSI_LOG_ERR("ccree init error occurred!\n");
-   dev_set_drvdata(&plat_dev->dev, NULL);
return rc;
 }
 
@@ -429,7 +428,7 @@ void fini_cc_regs(struct ssi_drvdata *drvdata)
 static void cleanup_cc_resources(struct platform_device *plat_dev)
 {
struct ssi_drvdata *drvdata =
-   (struct ssi_drvdata *)dev_get_drvdata(&plat_dev->dev);
+   (struct ssi_drvdata *)platform_get_drvdata(plat_dev);
 
ssi_aead_free(drvdata);
ssi_hash_free(drvdata);
@@ -445,7 +444,6 @@ static void cleanup_cc_resources(struct platform_device 
*plat_dev)
 #endif
fini_cc_regs(drvdata);
cc_clk_off(drvdata);
-   dev_set_drvdata(&plat_dev->dev, NULL);
 }
 
 int cc_clk_on(struct ssi_drvdata *drvdata)
-- 
1.9.1

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

[PATCH] media: staging: greybus: Release memory obtained by kasprintf

2017-09-21 Thread Arvind Yadav 
Free memory region, if gb_lights_channel_config is not successful.

Signed-off-by: Arvind Yadav 
---
 drivers/staging/greybus/light.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index 3f4148c..b00d47c 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -984,7 +984,7 @@ static int gb_lights_channel_config(struct gb_light *light,
 
ret = channel_attr_groups_set(channel, cdev);
if (ret < 0)
-   return ret;
+   goto err;
 
gb_lights_led_operations_set(channel, cdev);
 
@@ -994,15 +994,18 @@ static int gb_lights_channel_config(struct gb_light 
*light,
 * configurations.
 */
if (!is_channel_flash(channel))
-   return ret;
+   goto err;
 
light->has_flash = true;
 
ret = gb_lights_channel_flash_config(channel);
if (ret < 0)
-   return ret;
+   goto err;
 
return ret;
+err:
+   kfree(cdev->name);
+   return ret;
 }
 
 static int gb_lights_light_config(struct gb_lights *glights, u8 id)
-- 
1.9.1

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

Re: [PATCH] staging:vme Fix use BIT macro

2017-09-21 Thread Martyn Welch 
On 21 September 2017 at 06:52, Janani Sankara Babu  wrote:
> This patch is created to solve the following warning shown by the checkpatch
> script Warning: Replace all occurences of (1<
> Signed-off-by: Janani Sankara Babu 
> ---
>  drivers/staging/vme/devices/vme_pio2.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/vme/devices/vme_pio2.h 
> b/drivers/staging/vme/devices/vme_pio2.h
> index ac4a4ba..e9c3cf6 100644
> --- a/drivers/staging/vme/devices/vme_pio2.h
> +++ b/drivers/staging/vme/devices/vme_pio2.h
> @@ -179,7 +179,7 @@
> PIO2_REGS_CTRL_WRD1 };
>
>  #define PIO2_CNTR_SC_DEV0  0
> -#define PIO2_CNTR_SC_DEV1  (1 << 6)
> +#define PIO2_CNTR_SC_DEV1  BIT(6)

Sorry, these changes just don't make sense given the defines below.

>  #define PIO2_CNTR_SC_DEV2  (2 << 6)
>  #define PIO2_CNTR_SC_RDBACK(3 << 6)
>
> @@ -188,12 +188,12 @@
> PIO2_CNTR_SC_DEV1, PIO2_CNTR_SC_DEV2 
> };
>
>  #define PIO2_CNTR_RW_LATCH 0
> -#define PIO2_CNTR_RW_LSB   (1 << 4)
> +#define PIO2_CNTR_RW_LSB   BIT(4)
>  #define PIO2_CNTR_RW_MSB   (2 << 4)
>  #define PIO2_CNTR_RW_BOTH  (3 << 4)
>
>  #define PIO2_CNTR_MODE00
> -#define PIO2_CNTR_MODE1(1 << 1)
> +#define PIO2_CNTR_MODE1BIT(1)
>  #define PIO2_CNTR_MODE2(2 << 1)
>  #define PIO2_CNTR_MODE3(3 << 1)
>  #define PIO2_CNTR_MODE4(4 << 1)
> --
> 1.9.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2 04/11] usb: xhci: Add Intel cherrytrail extended cap / otg phy mux handling

2017-09-21 Thread Hans de Goede 

Hi,

On 19-09-17 14:40, Mathias Nyman wrote:

Hi,

sorry about the long delay

On 07.09.2017 18:49, Hans de Goede wrote:

Hi,

On 07-09-17 15:14, Mathias Nyman wrote:

On 05.09.2017 19:42, Hans de Goede wrote:

The Intel cherrytrail xhci controller has an extended cap mmio-range
which contains registers to control the muxing to the xhci (host mode)
or the dwc3 (device mode) and vbus-detection for the otg usb-phy.

Having a mux driver included in the xhci code (or under drivers/usb/host)
is not desirable. So this commit adds a simple handler for this extended
capability, which creates a platform device with the caps mmio region as
resource, this allows us to write a separate platform mux driver for the
mux.


I think it would be better to have one place where we add handlers for
vendor specific extended capabilities.

Something like xhci-vendor-ext-caps.c, or just xhci-ext-caps.c as
there's a xhci-ext-caps.h header already

We could walk through the capability list once and add the needed handlers.
Something like:

+int xhci_ext_cap_init(void __iomem *base)


This will need to take a struct xhci_hcd *xhci param instead
as some of the ext_cap handling (including the cht mux code)
will need access to this.



yes, sample code added in second patch for reference/testing.



So I see 2 options here (without making this function PCI specific)
1) Add an u32 product_id field to struct xhci_hcd; or
2) Use a quirk flag as my current code is doing.

I'm fine with doing this either way, please let me know your preference.


Lets go with the quirk for now, I'll sort that out later



Can you do a "git format-patch" of that and send it to me? If you
can give me that + your preference for how to check if we're
dealing with a cht xhci hcd in xhci_ext_cap_init I can do a v3
with your suggestions applied.


Ended up modifying xhci_find_next_ext_cap() using id = 0 for
the next capability in list. Patch attached,

Second patch is just for reference how to use it.


Thank you for the patches, I'm working on prepping a v3 of
this series which includes and uses the first patch.

Regards,

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

Re: [PATCH v2 03/11] mux: core: Add usb.h header with MUX_USB_* and and MUX_TYPEC_* state constants

2017-09-21 Thread Hans de Goede 

Hi,

On 10-09-17 23:36, Peter Rosin wrote:

On 2017-09-08 19:07, Hans de Goede wrote:

Hi,

On 08-09-17 17:47, Peter Rosin wrote:

On 2017-09-05 18:42, Hans de Goede wrote:

Add MUX_USB_* and MUX_TYPEC_* state constant defines, which can be used by
USB device/host, resp. Type-C polarity/role/altmode mux drivers and
consumers to ensure that they agree on the meaning of the
mux_control_select() state argument.

Signed-off-by: Hans de Goede 
---


*snip*


+/*
+ * Mux state values for Type-C polarity/role/altmode muxes.
+ *
+ * MUX_TYPEC_POLARITY_INV may be or-ed together with any other mux-state as
+ * inverted-polarity (Type-C plugged in upside down) can happen with any
+ * other mux-state.
+ */
+#define MUX_TYPEC_POLARITY_INV BIT(0)   /* Polarity inverted bit */
+#define MUX_TYPEC_DEVICE   (0 << 1) /* USB device mode */
+#define MUX_TYPEC_HOST (1 << 1) /* USB host mode */
+#define MUX_TYPEC_HOST_AND_DP_SRC  (2 << 1) /* USB host + 2 lanes DP src */
+#define MUX_TYPEC_DP_SRC   (3 << 1) /* 4 lanes Display Port src */
+#define MUX_TYPEC_STATES   (4 << 1)


But USB Type-C muxes need not support just these states If I read it right?
USB Type-C seems to be usable for a variety of protocols and the above list
seems pretty much like a special case for this mux (and perhaps a set of
other similar muxes). But when someone with a USB Type-C mux for different
protocols shows up, that person will probably be frustrated by these
defines, no? Or is there something I don't see that limits USB-C to DP?


In general almost all hardware is limited to the above (+ analog audio over
the 2 Sideband use pins, but I expect that to have a separate mux).

You're right, theoretically there might be other cases, e.g. there is a spec
for HDMI over Type-C (wishful thinking from the HDMI group, no one uses this),
but:

1) I expect most muxes to implement the above set, that is what all
hardware out there supports (well that or less).

2) We can always add extra defines here, that means that a Type-C mux may
not implement all states and return -EINVAL when asked for something it
does not implement, which I understand is a bit weird from a mux subsys
pov. But that can be the case anyways because even though the mux supports
these options, the board it is used on does no necessarily have to support
these options, e.g. there may be only 2 lanes of DP hooked up to the mux
(or no DP at all, but then I would them to expect a different mux).

So the Type-C Port Manager already needs to be passed some platform
data describing which features the board has and keep that in mind
when negotiation with the dongle attached to the Type-C port, so if
we do get boards which do HDMI and no DP, then the TCPM would simply
never use the MUX_TYPEC_HOST_AND_DP_SRC and MUX_TYPEC_DP_SRC states.


Ok, I googled "usb type c mux" and came up with HD3SS460 from Texas as
the first hit.

http://www.ti.com/lit/ds/symlink/hd3ss460.pdf

That one has three control pins, but two of them (AMSEL and EN) are
tri-state. So 18 states in theory. However, if EN is low everything is
HighZ, so that collapses 6 states into 1, and 2 other states are reserved.
Still 11 states, which is two more than what you have implemented for
PI3USB30532. If we ignore polarity switching, it's only a one state diff.
However, when I try to make sense of the states for the HD3SS460, I don't
see anything that selects USB device or host. And I don't really see why
a Type C mux has to know that; in my head the mux should just route the
signals. And then when I look in your PI3USB30532 driver I don't seen any
such difference either. Along the same lines, the Type C mux does not
know/care if DP is source or sink. Or?

How about:

#define MUX_TYPEC_POLARITY_INV  BIT(0)   /* Polarity inverted bit */
#define MUX_TYPEC_USB   (0 << 1) /* USB only mode */
#define MUX_TYPEC_USB_AND_DP(1 << 1) /* USB host + 2 lanes DP */
#define MUX_TYPEC_DP(2 << 1) /* 4 lanes Display Port */
#define MUX_TYPEC_STATES(3 << 1)



Sure that works for me, I will switch over to this for v3 of the patch-set.

One note though, compared to my list, this changes DEVICE / HOST to just a 
single
_USB entry. That works fine for my purpose, but typically USB host and device
controllers are 2 separate blocks with a mux in between them. Now most
current hardware have that mux in the SoC and then an external mux to
mux the USB3 lines and optionally also DP lines to the Type-C connector
and the above table does is correct (as the Type-C mux only has 1 USB
state not separate host / device states as my proposal was). But my
reason for having separate DEVICE / HOST states (and treating those
identical in the pi3usb30532 driver) was to future proof things a bit.


I'm not sure what 2 states the HS3SS460 have in addition to the above, but
the way I read the spec those to are variations on the MUX_TYPEC_USB_AND_DP
state

[PATCH v2] staging: ccree: Convert to platform_{get,set}_drvdata()

2017-09-21 Thread sunil . m 
From: Suniel Mahesh 

Platform devices are expected to use wrapper functions,
platform_{get,set}_drvdata() with platform_device as argument,
for getting and setting the driver data. dev_{get,set}_drvdata()
are using &plat_dev->dev.
For wrapper functions we can directly pass a struct platform_device.

dev_set_drvdata() is redundant and therefore removed. The driver core
clears the driver data to NULL after device_release or on probe failure.

Signed-off-by: Suniel Mahesh 
---
Changes for v2:
- Rebased on top of staging-testing.
---
Note:
- Patch was tested and built(ARCH=arm) on next-20170921.
  No build issues reported, however it was not tested on
  real hardware.
---
 drivers/staging/ccree/ssi_driver.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/ccree/ssi_driver.c 
b/drivers/staging/ccree/ssi_driver.c
index 6d16220..53b4a8c 100644
--- a/drivers/staging/ccree/ssi_driver.c
+++ b/drivers/staging/ccree/ssi_driver.c
@@ -236,7 +236,7 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
rc = -ENOMEM;
goto post_drvdata_err;
}
-   dev_set_drvdata(&plat_dev->dev, new_drvdata);
+   platform_set_drvdata(plat_dev, new_drvdata);
new_drvdata->plat_dev = plat_dev;
 
new_drvdata->clk = of_clk_get(np, 0);
@@ -415,7 +415,6 @@ static int init_cc_resources(struct platform_device 
*plat_dev)
cc_clk_off(new_drvdata);
 post_drvdata_err:
SSI_LOG_ERR("ccree init error occurred!\n");
-   dev_set_drvdata(&plat_dev->dev, NULL);
return rc;
 }
 
@@ -429,7 +428,7 @@ void fini_cc_regs(struct ssi_drvdata *drvdata)
 static void cleanup_cc_resources(struct platform_device *plat_dev)
 {
struct ssi_drvdata *drvdata =
-   (struct ssi_drvdata *)dev_get_drvdata(&plat_dev->dev);
+   (struct ssi_drvdata *)platform_get_drvdata(plat_dev);
 
ssi_aead_free(drvdata);
ssi_hash_free(drvdata);
@@ -445,7 +444,6 @@ static void cleanup_cc_resources(struct platform_device 
*plat_dev)
 #endif
fini_cc_regs(drvdata);
cc_clk_off(drvdata);
-   dev_set_drvdata(&plat_dev->dev, NULL);
 }
 
 int cc_clk_on(struct ssi_drvdata *drvdata)
-- 
1.9.1

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

Re: [PATCH] media: staging: greybus: Release memory obtained by kasprintf

2017-09-21 Thread Dan Carpenter 
On Thu, Sep 21, 2017 at 05:05:27PM +0530, Arvind Yadav wrote:
> Free memory region, if gb_lights_channel_config is not successful.
> 

The question I have is do we free this on module unload?  I don't see
that we do.  I feel like we should do a free after calling
__gb_lights_led_unregister().  But that's awkward because we call
__gb_lights_led_unregister() when this function fails so it would end
up being a double free.

> Signed-off-by: Arvind Yadav 
> ---
>  drivers/staging/greybus/light.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
> index 3f4148c..b00d47c 100644
> --- a/drivers/staging/greybus/light.c
> +++ b/drivers/staging/greybus/light.c
> @@ -984,7 +984,7 @@ static int gb_lights_channel_config(struct gb_light 
> *light,
>  
>   ret = channel_attr_groups_set(channel, cdev);
>   if (ret < 0)
> - return ret;
> + goto err;
>  
>   gb_lights_led_operations_set(channel, cdev);
>  
> @@ -994,15 +994,18 @@ static int gb_lights_channel_config(struct gb_light 
> *light,
>* configurations.
>*/
>   if (!is_channel_flash(channel))
> - return ret;
> + goto err;

"ret" is zero here.  This is actually a success return.  It would be
cleaner to just write "return 0;".  Anyway, this patch introduces a use
after free so that doesn't work.

Also it's better to choose a label name which says what the label does
so in this case it would be "goto err_free_name" or "goto err_cdev_name"
or whatever, but something to indicate that it's to do with freeing
the cdev->name.  Just "err" is too ambiguous.

>  
>   light->has_flash = true;
>  
>   ret = gb_lights_channel_flash_config(channel);
>   if (ret < 0)
> - return ret;
> + goto err;
>  
>   return ret;
^^
Here as well, change this from "return ret;" to "return 0;".

regards,
dan carpenter

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

Re: [PATCH] media: staging: greybus: Release memory obtained by kasprintf

2017-09-21 Thread Rui Miguel Silva 
Hi,
On Thu, Sep 21, 2017 at 03:59:18PM +0300, Dan Carpenter wrote:
> On Thu, Sep 21, 2017 at 05:05:27PM +0530, Arvind Yadav wrote:
> > Free memory region, if gb_lights_channel_config is not successful.

Arvind, thanks for patch and good catch.
But please look at the subject of other patches applied to this
file and try to stick with the labels, staging: greybus: light:

> > 
> 
> The question I have is do we free this on module unload?  I don't see
> that we do.  I feel like we should do a free after calling
> __gb_lights_led_unregister().  But that's awkward because we call
> __gb_lights_led_unregister() when this function fails so it would end
> up being a double free.

Yes Dan, You are correct, this should be free in
__gb_lights_led_unregister(), and not here.

> 
> > Signed-off-by: Arvind Yadav 
> > ---
> >  drivers/staging/greybus/light.c | 9 ++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/greybus/light.c 
> > b/drivers/staging/greybus/light.c
> > index 3f4148c..b00d47c 100644
> > --- a/drivers/staging/greybus/light.c
> > +++ b/drivers/staging/greybus/light.c
> > @@ -984,7 +984,7 @@ static int gb_lights_channel_config(struct gb_light 
> > *light,
> >  
> > ret = channel_attr_groups_set(channel, cdev);
> > if (ret < 0)
> > -   return ret;
> > +   goto err;
> >  
> > gb_lights_led_operations_set(channel, cdev);
> >  
> > @@ -994,15 +994,18 @@ static int gb_lights_channel_config(struct gb_light 
> > *light,
> >  * configurations.
> >  */
> > if (!is_channel_flash(channel))
> > -   return ret;
> > +   goto err;
> 
> "ret" is zero here.  This is actually a success return.  It would be
> cleaner to just write "return 0;".  Anyway, this patch introduces a use
> after free so that doesn't work.
> 
> Also it's better to choose a label name which says what the label does
> so in this case it would be "goto err_free_name" or "goto err_cdev_name"
> or whatever, but something to indicate that it's to do with freeing
> the cdev->name.  Just "err" is too ambiguous.
> 
> >  
> > light->has_flash = true;
> >  
> > ret = gb_lights_channel_flash_config(channel);
> > if (ret < 0)
> > -   return ret;
> > +   goto err;
> >  
> > return ret;
> ^^
> Here as well, change this from "return ret;" to "return 0;".

It should be return 0; from the start, you are right, but that
would be a complete different change than the actual fix that now
goes far away from this code.

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

Re: [PATCH v2 1/2] include: linux: sysfs: Add __ATTR_NAMED macro

2017-09-21 Thread Jonathan Cameron 
On Mon, 18 Sep 2017 16:19:07 +0530
Himanshi Jain  wrote:

> On Thu, Sep 14, 2017 at 2:20 AM, Jonathan Cameron
>  wrote:
> >
> >
> > On 13 September 2017 12:23:31 GMT-07:00, Lars-Peter Clausen 
> >  wrote:  
> >>On 09/13/2017 08:58 PM, Greg KH wrote:  
> >>> On Wed, Sep 13, 2017 at 06:03:10PM +0100, Jonathan Cameron wrote:  
>  On Wed, 13 Sep 2017 14:14:07 +0530
>  Himanshi Jain  wrote:
>   
> > Add __ATTR_NAMED macro similar to __ATTR but taking name as a
> > string instead of implicit conversion of argument to string using
> > the macro _stringify(_name).
> >
> > Signed-off-by: Himanshi Jain 
> > ---
> >  include/linux/sysfs.h | 7 +++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> > index aa02c32..20321cf 100644
> > --- a/include/linux/sysfs.h
> > +++ b/include/linux/sysfs.h
> > @@ -104,6 +104,13 @@ struct attribute_group {
> >.store  = _store,   \
> >  }
> >
> > +#define __ATTR_NAMED(_name, _mode, _show, _store) {
> >\  
> 
>  I'm not sure about the naming here.  The normal __ATTR macro is also
>  'named'.  Maybe something as awful as
> 
>  __ATTR_STRING_NAME ?
> 
>  Greg what do you think?  
> >>>
> >>> ick ick ick.
> >>>  
>  This is all to allow us to have names with operators in them without
>  checkpatch complaining about them... A worthwhile aim just to stop
>  more people wasting time trying to 'fix' those cases by adding  
> >>spaces.  
> >>>
> >>> Yeah, but this really seems "heavy" for just a crazy sysfs name in a
> >>> macro.  Adding a whole new "core" define for that is a hard sell...
> >>>
> >>> I also want to get rid of the "generic" __ATTR type macros, and force
> >>> people to use the proper _RW and friends instead.  I don't want to  
> >>add  
> >>> another new one that people will start to use that I later have to
> >>> change...
> >>>
> >>> So no, I don't like this, how about just changing your macros  
> >>instead?  
> >>> No one else has this problem :)  
> >>
> >>Nobody else realized they have this problem yet. E.g. there are a few
> >>users
> >>of __ATTR in block/genhd.c that have the same issue and are likely to
> >>generate the same false positives from static checkers.  
> >
> > For IIO there is the option of moving these over to the core generated 
> > available callbacks, but
> > that won't work in every case and is a more major change.  I need to shift 
> > a few more drivers
> > over to the available callbacks and see how well it works out.  Might find 
> > time to do one in a
> >  gap between interesting talks this afternoon...  
> 
> Can I help you in this? It is about exploring options as far as I can
> make out, although can't really understand what options are those for
> now.

You are welcome to have a go.  However the relevant code isn't all that
well tested so I'd want to actually get enough of the setup to run to see
if it works (which is trickier).  I doubt anyone has ready access
to this hardware.

Anyhow, to see what I mean see : 

https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/tree/drivers/iio/dac/dpot-dac.c

In particular dpot_dac_read_avail() and 

static const struct iio_chan_spec dpot_dac_iio_channel = {
.type = IIO_VOLTAGE,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
| BIT(IIO_CHAN_INFO_SCALE),
.info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW),
.output = 1,
.indexed = 1,
};

What this does is allow the core to create *_available attributes
(sometimes this is required by in kernel consumers of the channels as they
need to know the range of the channel for example).

The intent ultimately is to move drivers over to this interface and hopefully
get rid of the majority of remaining hand specified attrs.

It's always been well down the todo list though as it is easy to get wrong
in a given driver and we need to get more emulation in place to allow
testing of the various drivers. There are a couple of ways of doing such
emulation (i2c devices can use the i2c-stub framework, or we can do full blown
qemu emulation) - both are interesting diversions.

Also I promised to write the ABI docs (see the original patches introducing
it for some docs) but haven't actually done so yet which makes it tricky
to tell people to start using this stuff even in new drivers!

Anyhow, whilst you are welcome to look at this it might be a high risk choice
for where to get started during the outreachy applications period.

Still nothing wrong with being brave, but perhaps discuss with Alison, Daniel
or Julia.

Thanks,

Jonathan

> 
> Or do you want me to put comments to not to fix this checkpatch
> warning as you suggested earlier?
> 
> >
> > If I am feeling really keen I might write this missing docs I promised a 
> > while back on that stu

Re: [Outreachy kernel] Re: [PATCH v2 1/2] include: linux: sysfs: Add __ATTR_NAMED macro

2017-09-21 Thread Julia Lawall 


On Thu, 21 Sep 2017, Jonathan Cameron wrote:

> On Mon, 18 Sep 2017 16:19:07 +0530
> Himanshi Jain  wrote:
>
> > On Thu, Sep 14, 2017 at 2:20 AM, Jonathan Cameron
> >  wrote:
> > >
> > >
> > > On 13 September 2017 12:23:31 GMT-07:00, Lars-Peter Clausen 
> > >  wrote:
> > >>On 09/13/2017 08:58 PM, Greg KH wrote:
> > >>> On Wed, Sep 13, 2017 at 06:03:10PM +0100, Jonathan Cameron wrote:
> >  On Wed, 13 Sep 2017 14:14:07 +0530
> >  Himanshi Jain  wrote:
> > 
> > > Add __ATTR_NAMED macro similar to __ATTR but taking name as a
> > > string instead of implicit conversion of argument to string using
> > > the macro _stringify(_name).
> > >
> > > Signed-off-by: Himanshi Jain 
> > > ---
> > >  include/linux/sysfs.h | 7 +++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> > > index aa02c32..20321cf 100644
> > > --- a/include/linux/sysfs.h
> > > +++ b/include/linux/sysfs.h
> > > @@ -104,6 +104,13 @@ struct attribute_group {
> > >.store  = _store,   \
> > >  }
> > >
> > > +#define __ATTR_NAMED(_name, _mode, _show, _store) {  
> > >  \
> > 
> >  I'm not sure about the naming here.  The normal __ATTR macro is also
> >  'named'.  Maybe something as awful as
> > 
> >  __ATTR_STRING_NAME ?
> > 
> >  Greg what do you think?
> > >>>
> > >>> ick ick ick.
> > >>>
> >  This is all to allow us to have names with operators in them without
> >  checkpatch complaining about them... A worthwhile aim just to stop
> >  more people wasting time trying to 'fix' those cases by adding
> > >>spaces.
> > >>>
> > >>> Yeah, but this really seems "heavy" for just a crazy sysfs name in a
> > >>> macro.  Adding a whole new "core" define for that is a hard sell...
> > >>>
> > >>> I also want to get rid of the "generic" __ATTR type macros, and force
> > >>> people to use the proper _RW and friends instead.  I don't want to
> > >>add
> > >>> another new one that people will start to use that I later have to
> > >>> change...
> > >>>
> > >>> So no, I don't like this, how about just changing your macros
> > >>instead?
> > >>> No one else has this problem :)
> > >>
> > >>Nobody else realized they have this problem yet. E.g. there are a few
> > >>users
> > >>of __ATTR in block/genhd.c that have the same issue and are likely to
> > >>generate the same false positives from static checkers.
> > >
> > > For IIO there is the option of moving these over to the core generated 
> > > available callbacks, but
> > > that won't work in every case and is a more major change.  I need to 
> > > shift a few more drivers
> > > over to the available callbacks and see how well it works out.  Might 
> > > find time to do one in a
> > >  gap between interesting talks this afternoon...
> >
> > Can I help you in this? It is about exploring options as far as I can
> > make out, although can't really understand what options are those for
> > now.
>
> You are welcome to have a go.  However the relevant code isn't all that
> well tested so I'd want to actually get enough of the setup to run to see
> if it works (which is trickier).  I doubt anyone has ready access
> to this hardware.
>
> Anyhow, to see what I mean see :
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/tree/drivers/iio/dac/dpot-dac.c
>
> In particular dpot_dac_read_avail() and
>
> static const struct iio_chan_spec dpot_dac_iio_channel = {
>   .type = IIO_VOLTAGE,
>   .info_mask_separate = BIT(IIO_CHAN_INFO_RAW)
>   | BIT(IIO_CHAN_INFO_SCALE),
>   .info_mask_separate_available = BIT(IIO_CHAN_INFO_RAW),
>   .output = 1,
>   .indexed = 1,
> };
>
> What this does is allow the core to create *_available attributes
> (sometimes this is required by in kernel consumers of the channels as they
> need to know the range of the channel for example).
>
> The intent ultimately is to move drivers over to this interface and hopefully
> get rid of the majority of remaining hand specified attrs.
>
> It's always been well down the todo list though as it is easy to get wrong
> in a given driver and we need to get more emulation in place to allow
> testing of the various drivers. There are a couple of ways of doing such
> emulation (i2c devices can use the i2c-stub framework, or we can do full blown
> qemu emulation) - both are interesting diversions.
>
> Also I promised to write the ABI docs (see the original patches introducing
> it for some docs) but haven't actually done so yet which makes it tricky
> to tell people to start using this stuff even in new drivers!
>
> Anyhow, whilst you are welcome to look at this it might be a high risk choice
> for where to get started during the outreachy applications period.
>
> Still nothing wrong with being brave, but perhaps discuss with Alison, Daniel
> or Julia.

Himans

[PATCH] staging: vc04_services: Remove typedef struct

2017-09-21 Thread Harsha Sharma 
Remove typedef from struct as linux-kernel coding style tends to
avoid using typedefs

Done using following coccinelle semantic patch

@r1@
type T;
@@

typedef struct { ... } T;

@script:python c1@
T2;
T << r1.T;
@@
if T[-2:] =="_t" or T[-2:] == "_T":
coccinelle.T2 = T[:-2];
else:
coccinelle.T2 = T;

print T, coccinelle.T2

@r2@
type r1.T;
identifier c1.T2;
@@
-typedef
struct
+ T2
{ ... }
-T
;

@r3@
type r1.T;
identifier c1.T2;
@@
-T
+struct T2

Signed-off-by: Harsha Sharma 
---
 .../vc04_services/interface/vchiq_arm/vchiq_shim.c | 44 +++---
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c 
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
index 8af95fc..adb602d 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
@@ -41,14 +41,14 @@
 
 #define vchiq_status_to_vchi(status) ((int32_t)status)
 
-typedef struct {
+struct SHIM_SERVICE {
VCHIQ_SERVICE_HANDLE_T handle;
 
VCHIU_QUEUE_T queue;
 
VCHI_CALLBACK_T callback;
void *callback_param;
-} SHIM_SERVICE_T;
+};
 
 /* --
  * return pointer to the mphi message driver function table
@@ -99,7 +99,7 @@ int32_t vchi_msg_peek(VCHI_SERVICE_HANDLE_T handle,
uint32_t *msg_size,
VCHI_FLAGS_T flags)
 {
-   SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
+   struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
VCHIQ_HEADER_T *header;
 
WARN_ON((flags != VCHI_FLAGS_NONE) &&
@@ -131,7 +131,7 @@ int32_t vchi_msg_peek(VCHI_SERVICE_HANDLE_T handle,
  ***/
 int32_t vchi_msg_remove(VCHI_SERVICE_HANDLE_T handle)
 {
-   SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
+   struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
VCHIQ_HEADER_T *header;
 
header = vchiu_queue_pop(&service->queue);
@@ -163,7 +163,7 @@ int32_t vchi_msg_queue(VCHI_SERVICE_HANDLE_T handle,
void *context,
uint32_t data_size)
 {
-   SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
+   struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
VCHIQ_STATUS_T status;
 
while (1) {
@@ -262,7 +262,7 @@ int32_t vchi_bulk_queue_receive(VCHI_SERVICE_HANDLE_T 
handle,
VCHI_FLAGS_T flags,
void *bulk_handle)
 {
-   SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
+   struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
VCHIQ_BULK_MODE_T mode;
VCHIQ_STATUS_T status;
 
@@ -322,7 +322,7 @@ int32_t vchi_bulk_queue_transmit(VCHI_SERVICE_HANDLE_T 
handle,
VCHI_FLAGS_T flags,
void *bulk_handle)
 {
-   SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
+   struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
VCHIQ_BULK_MODE_T mode;
VCHIQ_STATUS_T status;
 
@@ -384,7 +384,7 @@ int32_t vchi_msg_dequeue(VCHI_SERVICE_HANDLE_T handle,
uint32_t *actual_msg_size,
VCHI_FLAGS_T flags)
 {
-   SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
+   struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
VCHIQ_HEADER_T *header;
 
WARN_ON((flags != VCHI_FLAGS_NONE) &&
@@ -458,7 +458,7 @@ int32_t vchi_msg_hold(VCHI_SERVICE_HANDLE_T handle,
VCHI_FLAGS_T flags,
VCHI_HELD_MSG_T *message_handle)
 {
-   SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
+   struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
VCHIQ_HEADER_T *header;
 
WARN_ON((flags != VCHI_FLAGS_NONE) &&
@@ -579,8 +579,8 @@ int32_t vchi_disconnect(VCHI_INSTANCE_T instance_handle)
 static VCHIQ_STATUS_T shim_callback(VCHIQ_REASON_T reason,
VCHIQ_HEADER_T *header, VCHIQ_SERVICE_HANDLE_T handle, void *bulk_user)
 {
-   SHIM_SERVICE_T *service =
-   (SHIM_SERVICE_T *)VCHIQ_GET_SERVICE_USERDATA(handle);
+   struct SHIM_SERVICE *service =
+   (struct SHIM_SERVICE *)VCHIQ_GET_SERVICE_USERDATA(handle);
 
if (!service->callback)
goto release;
@@ -637,10 +637,10 @@ static VCHIQ_STATUS_T shim_callback(VCHIQ_REASON_T reason,
return VCHIQ_SUCCESS;
 }
 
-static SHIM_SERVICE_T *service_alloc(VCHIQ_INSTANCE_T instance,
+static struct SHIM_SERVICE *service_alloc(VCHIQ_INSTANCE_T instance,
SERVICE_CREATION_T *setup)
 {
-   SHIM_SERVICE_T *service = kzalloc(sizeof(SHIM_SERVICE_T), GFP_KERNEL);
+   struct SHIM_SERVICE *service = kzalloc(sizeof(struct SHIM_SERVICE), 
GFP_KERNEL);
 
(void)instance;
 
@@ -657,7 +657,7 @@ static SHIM_SERVICE_T *service_alloc(VCHIQ_INSTANCE_T 
instance,
return service;
 }
 
-static void service_free(SHIM_SERVICE_T *service)
+static void service_free(struct SHIM_SERVICE *serv

Re: [Outreachy kernel] [PATCH] staging: vc04_services: Remove typedef struct

2017-09-21 Thread Julia Lawall 


On Fri, 22 Sep 2017, Harsha Sharma wrote:

> Remove typedef from struct as linux-kernel coding style tends to
> avoid using typedefs
>
> Done using following coccinelle semantic patch
>
> @r1@
> type T;
> @@
>
> typedef struct { ... } T;
>
> @script:python c1@
> T2;
> T << r1.T;
> @@
> if T[-2:] =="_t" or T[-2:] == "_T":
> coccinelle.T2 = T[:-2];
> else:
> coccinelle.T2 = T;
>
> print T, coccinelle.T2

Could you also convert the structure name to lowercase?

julia

>
> @r2@
> type r1.T;
> identifier c1.T2;
> @@
> -typedef
> struct
> + T2
> { ... }
> -T
> ;
>
> @r3@
> type r1.T;
> identifier c1.T2;
> @@
> -T
> +struct T2
>
> Signed-off-by: Harsha Sharma 
> ---
>  .../vc04_services/interface/vchiq_arm/vchiq_shim.c | 44 
> +++---
>  1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c 
> b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
> index 8af95fc..adb602d 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_shim.c
> @@ -41,14 +41,14 @@
>
>  #define vchiq_status_to_vchi(status) ((int32_t)status)
>
> -typedef struct {
> +struct SHIM_SERVICE {
>   VCHIQ_SERVICE_HANDLE_T handle;
>
>   VCHIU_QUEUE_T queue;
>
>   VCHI_CALLBACK_T callback;
>   void *callback_param;
> -} SHIM_SERVICE_T;
> +};
>
>  /* --
>   * return pointer to the mphi message driver function table
> @@ -99,7 +99,7 @@ int32_t vchi_msg_peek(VCHI_SERVICE_HANDLE_T handle,
>   uint32_t *msg_size,
>   VCHI_FLAGS_T flags)
>  {
> - SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
> + struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
>   VCHIQ_HEADER_T *header;
>
>   WARN_ON((flags != VCHI_FLAGS_NONE) &&
> @@ -131,7 +131,7 @@ int32_t vchi_msg_peek(VCHI_SERVICE_HANDLE_T handle,
>   ***/
>  int32_t vchi_msg_remove(VCHI_SERVICE_HANDLE_T handle)
>  {
> - SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
> + struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
>   VCHIQ_HEADER_T *header;
>
>   header = vchiu_queue_pop(&service->queue);
> @@ -163,7 +163,7 @@ int32_t vchi_msg_queue(VCHI_SERVICE_HANDLE_T handle,
>   void *context,
>   uint32_t data_size)
>  {
> - SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
> + struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
>   VCHIQ_STATUS_T status;
>
>   while (1) {
> @@ -262,7 +262,7 @@ int32_t vchi_bulk_queue_receive(VCHI_SERVICE_HANDLE_T 
> handle,
>   VCHI_FLAGS_T flags,
>   void *bulk_handle)
>  {
> - SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
> + struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
>   VCHIQ_BULK_MODE_T mode;
>   VCHIQ_STATUS_T status;
>
> @@ -322,7 +322,7 @@ int32_t vchi_bulk_queue_transmit(VCHI_SERVICE_HANDLE_T 
> handle,
>   VCHI_FLAGS_T flags,
>   void *bulk_handle)
>  {
> - SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
> + struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
>   VCHIQ_BULK_MODE_T mode;
>   VCHIQ_STATUS_T status;
>
> @@ -384,7 +384,7 @@ int32_t vchi_msg_dequeue(VCHI_SERVICE_HANDLE_T handle,
>   uint32_t *actual_msg_size,
>   VCHI_FLAGS_T flags)
>  {
> - SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
> + struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
>   VCHIQ_HEADER_T *header;
>
>   WARN_ON((flags != VCHI_FLAGS_NONE) &&
> @@ -458,7 +458,7 @@ int32_t vchi_msg_hold(VCHI_SERVICE_HANDLE_T handle,
>   VCHI_FLAGS_T flags,
>   VCHI_HELD_MSG_T *message_handle)
>  {
> - SHIM_SERVICE_T *service = (SHIM_SERVICE_T *)handle;
> + struct SHIM_SERVICE *service = (struct SHIM_SERVICE *)handle;
>   VCHIQ_HEADER_T *header;
>
>   WARN_ON((flags != VCHI_FLAGS_NONE) &&
> @@ -579,8 +579,8 @@ int32_t vchi_disconnect(VCHI_INSTANCE_T instance_handle)
>  static VCHIQ_STATUS_T shim_callback(VCHIQ_REASON_T reason,
>   VCHIQ_HEADER_T *header, VCHIQ_SERVICE_HANDLE_T handle, void *bulk_user)
>  {
> - SHIM_SERVICE_T *service =
> - (SHIM_SERVICE_T *)VCHIQ_GET_SERVICE_USERDATA(handle);
> + struct SHIM_SERVICE *service =
> + (struct SHIM_SERVICE *)VCHIQ_GET_SERVICE_USERDATA(handle);
>
>   if (!service->callback)
>   goto release;
> @@ -637,10 +637,10 @@ static VCHIQ_STATUS_T shim_callback(VCHIQ_REASON_T 
> reason,
>   return VCHIQ_SUCCESS;
>  }
>
> -static SHIM_SERVICE_T *service_alloc(VCHIQ_INSTANCE_T instance,
> +static struct SHIM_SERVICE *service_alloc(VCHIQ_INSTANCE_T instance,
>   SERVICE_CREATION_T *setup)
>  {
> - SHIM_SERVICE_T *service = kzalloc(sizeof(SHIM_SERVICE_T), GFP_KERNEL);
> + struct SHIM_SERVICE *service = kzalloc(sizeof(struc

[PATCH] staging: xgifb: make const array static to shink object code size

2017-09-21 Thread Colin King 
From: Colin Ian King 

Don't populate const array LCDARefreshIndex on the stack, instead
make it static.  Makes the object code smaller by 340 bytes:

Before:
   textdata bss dec hex filename
  84949   12336   0   97285   17c05 drivers/staging/xgifb/vb_setmode.o

After:
   textdata bss dec hex filename
  84506   12432   0   96938   17aaa drivers/staging/xgifb/vb_setmode.o

(gcc version 7.2.0 x86_64)

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

diff --git a/drivers/staging/xgifb/vb_setmode.c 
b/drivers/staging/xgifb/vb_setmode.c
index d55ffa61bc40..a6cd0a1e8c05 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -5046,7 +5046,7 @@ unsigned short XGI_GetRatePtrCRT2(struct 
xgi_hw_device_info *pXGIHWDE,
  unsigned short ModeIdIndex,
  struct vb_device_info *pVBInfo)
 {
-   const u8 LCDARefreshIndex[] = {
+   static const u8 LCDARefreshIndex[] = {
0x00, 0x00, 0x03, 0x01, 0x01, 0x01, 0x01, 0x00 };
 
unsigned short RefreshRateTableIndex, i, index, temp;
-- 
2.14.1

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

[PATCH 0/2] Drivers: hv: vmbus: Add additional per-channel sysfs info

2017-09-21 Thread kys 
From: "K. Y. Srinivasan" 

Add additional per-channel sysfs information to help debug performance
issues.

Greg, please apply this patch-set to 4.15-rc1.

Stephen Hemminger (2):
  vmbus: add per-channel sysfs info
  Drivers: hv: vmbus: Expose per-channel interrupts and events counters

 Documentation/ABI/stable/sysfs-bus-vmbus |  70 +++
 drivers/hv/channel_mgmt.c|  10 +-
 drivers/hv/connection.c  |   2 +
 drivers/hv/hyperv_vmbus.h|   2 +
 drivers/hv/vmbus_drv.c   | 203 +--
 include/linux/hyperv.h   |  10 ++
 6 files changed, 284 insertions(+), 13 deletions(-)

-- 
2.14.1

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

[PATCH 1/2] vmbus: add per-channel sysfs info

2017-09-21 Thread kys 
From: Stephen Hemminger 

This extends existing vmbus related sysfs structure to provide per-channel
state information. This is useful when diagnosing issues with multiple
queues in networking and storage.

The existing sysfs only displayed information about the primary
channel. The one place it reported multiple channels was the
channel_vp_mapping file which violated the sysfs convention
of one value per file.

Signed-off-by: Stephen Hemminger 
Signed-off-by: K. Y. Srinivasan 
---
 Documentation/ABI/stable/sysfs-bus-vmbus |  56 ++
 drivers/hv/channel_mgmt.c|  10 +-
 drivers/hv/hyperv_vmbus.h|   2 +
 drivers/hv/vmbus_drv.c   | 185 +--
 include/linux/hyperv.h   |   6 +
 5 files changed, 246 insertions(+), 13 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus 
b/Documentation/ABI/stable/sysfs-bus-vmbus
index 5d0125f7bcaf..0ebd8a1537a0 100644
--- a/Documentation/ABI/stable/sysfs-bus-vmbus
+++ b/Documentation/ABI/stable/sysfs-bus-vmbus
@@ -41,3 +41,59 @@ KernelVersion:   4.5
 Contact:   K. Y. Srinivasan 
 Description:   The 16 bit vendor ID of the device
 Users: tools/hv/lsvmbus and user level RDMA libraries
+
+What:  /sys/bus/vmbus/devices/vmbus_*/channels/relid/cpu
+Date:  September. 2017
+KernelVersion: 4.14
+Contact:   Stephen Hemminger 
+Description:   VCPU (sub)channel is affinitized to
+Users: tools/hv/lsvmbus and other debuggig tools
+
+What:  /sys/bus/vmbus/devices/vmbus_*/channels/relid/cpu
+Date:  September. 2017
+KernelVersion: 4.14
+Contact:   Stephen Hemminger 
+Description:   VCPU (sub)channel is affinitized to
+Users: tools/hv/lsvmbus and other debuggig tools
+
+What:  /sys/bus/vmbus/devices/vmbus_*/channels/relid/in_mask
+Date:  September. 2017
+KernelVersion: 4.14
+Contact:   Stephen Hemminger 
+Description:   Inbound channel signaling state
+Users: Debuggig tools
+
+What:  /sys/bus/vmbus/devices/vmbus_*/channels/relid/latency
+Date:  September. 2017
+KernelVersion: 4.14
+Contact:   Stephen Hemminger 
+Description:   Channel signaling latency
+Users: Debuggig tools
+
+What:  /sys/bus/vmbus/devices/vmbus_*/channels/relid/out_mask
+Date:  September. 2017
+KernelVersion: 4.14
+Contact:   Stephen Hemminger 
+Description:   Outbound channel signaling state
+Users: Debuggig tools
+
+What:  /sys/bus/vmbus/devices/vmbus_*/channels/relid/pending
+Date:  September. 2017
+KernelVersion: 4.14
+Contact:   Stephen Hemminger 
+Description:   Channel interrupt pending state
+Users: Debuggig tools
+
+What:  /sys/bus/vmbus/devices/vmbus_*/channels/relid/read_avail
+Date:  September. 2017
+KernelVersion: 4.14
+Contact:   Stephen Hemminger 
+Description:   Bytes availabble to read
+Users: Debuggig tools
+
+What:  /sys/bus/vmbus/devices/vmbus_*/channels/relid/write_avail
+Date:  September. 2017
+KernelVersion: 4.14
+Contact:   Stephen Hemminger 
+Description:   Bytes availabble to write
+Users: Debuggig tools
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 624d815745e4..747887038992 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -350,7 +350,7 @@ static void free_channel(struct vmbus_channel *channel)
 {
tasklet_kill(&channel->callback_event);
 
-   kfree_rcu(channel, rcu);
+   kobject_put(&channel->kobj);
 }
 
 static void percpu_channel_enq(void *arg)
@@ -513,6 +513,14 @@ static void vmbus_process_offer(struct vmbus_channel 
*newchannel)
newchannel->state = CHANNEL_OPEN_STATE;
 
if (!fnew) {
+   struct hv_device *dev
+   = newchannel->primary_channel->device_obj;
+
+   if (vmbus_add_channel_kobj(dev, newchannel)) {
+   atomic_dec(&vmbus_connection.offer_in_progress);
+   goto err_free_chan;
+   }
+
if (channel->sc_creation_callback != NULL)
channel->sc_creation_callback(newchannel);
return;
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 1b6a5e0dfa75..bfa2d0af4e1d 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -384,6 +384,8 @@ struct hv_device *vmbus_device_create(const uuid_le *type,
 
 int vmbus_device_register(struct hv_device *child_device_obj);
 void vmbus_device_unregister(struct hv_device *device_obj);
+int vmbus_add_channel_kobj(struct hv_device *device_obj,
+  struct vmbus_channel *channel);
 
 struct vmbus_channel *relid2channel(u32 relid);
 
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 43160a2eafe0..c126d3937414 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -107,28 +107,30 @@ static void print_alias_na

[PATCH 2/2] Drivers: hv: vmbus: Expose per-channel interrupts and events counters

2017-09-21 Thread kys 
From: Stephen Hemminger 

When investigating performance, it is useful to be able to look at
the number of host and guest events per-channel. This is equivalent
to per-device interrupt statistics.

Signed-off-by: Stephen Hemminger 
Signed-off-by: K. Y. Srinivasan 
---
 Documentation/ABI/stable/sysfs-bus-vmbus | 14 ++
 drivers/hv/connection.c  |  2 ++
 drivers/hv/vmbus_drv.c   | 18 ++
 include/linux/hyperv.h   |  4 
 4 files changed, 38 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-bus-vmbus 
b/Documentation/ABI/stable/sysfs-bus-vmbus
index 0ebd8a1537a0..d4eca1717adb 100644
--- a/Documentation/ABI/stable/sysfs-bus-vmbus
+++ b/Documentation/ABI/stable/sysfs-bus-vmbus
@@ -97,3 +97,17 @@ KernelVersion:   4.14
 Contact:   Stephen Hemminger 
 Description:   Bytes availabble to write
 Users: Debuggig tools
+
+What:  /sys/bus/vmbus/devices/vmbus_*/channels/relid/events
+Date:  September. 2017
+KernelVersion: 4.14
+Contact:   Stephen Hemminger 
+Description:   Number of times we have signaled the host
+Users: Debuggig tools
+
+What:  /sys/bus/vmbus/devices/vmbus_*/channels/relid/interrupts
+Date:  September. 2017
+KernelVersion: 4.14
+Contact:   Stephen Hemminger 
+Description:   Number of times we have taken an interrupt (incoming)
+Users: Debuggig tools
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 59c11ff90d12..2bb0d03c5212 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -406,6 +406,8 @@ void vmbus_set_event(struct vmbus_channel *channel)
if (!channel->is_dedicated_interrupt)
vmbus_send_interrupt(child_relid);
 
+   ++channel->sig_events;
+
hv_do_hypercall(HVCALL_SIGNAL_EVENT, channel->sig_event, NULL);
 }
 EXPORT_SYMBOL_GPL(vmbus_set_event);
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index c126d3937414..88c4f5e98fc9 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -945,6 +945,8 @@ static void vmbus_chan_sched(struct hv_per_cpu_context 
*hv_cpu)
if (channel->rescind)
continue;
 
+   ++channel->interrupts;
+
switch (channel->callback_mode) {
case HV_CALL_ISR:
vmbus_channel_isr(channel);
@@ -1238,6 +1240,20 @@ static ssize_t channel_latency_show(const struct 
vmbus_channel *channel,
 }
 VMBUS_CHAN_ATTR(latency, S_IRUGO, channel_latency_show, NULL);
 
+static ssize_t channel_interrupts_show(const struct vmbus_channel *channel, 
char *buf)
+{
+   return sprintf(buf, "%u\n", channel->interrupts);
+}
+VMBUS_CHAN_ATTR(interrupts, S_IRUGO, channel_interrupts_show, NULL);
+
+static ssize_t channel_events_show(const struct vmbus_channel *channel, char 
*buf)
+{
+   return sprintf(buf, "%u\n", channel->sig_events);
+}
+VMBUS_CHAN_ATTR(events, S_IRUGO, channel_events_show, NULL);
+
+
+
 static struct attribute *vmbus_chan_attrs[] = {
&chan_attr_out_mask.attr,
&chan_attr_in_mask.attr,
@@ -1246,6 +1262,8 @@ static struct attribute *vmbus_chan_attrs[] = {
&chan_attr_cpu.attr,
&chan_attr_pending.attr,
&chan_attr_latency.attr,
+   &chan_attr_interrupts.attr,
+   &chan_attr_events.attr,
NULL
 };
 
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 91c88e0e771e..d7c85a475c46 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -731,6 +731,10 @@ struct vmbus_channel {
 
struct vmbus_close_msg close_msg;
 
+   /* Statistics */
+   unsigned intinterrupts; /* Host to Guest interrupts */
+   unsigned intsig_events; /* Guest to Host events */
+
/* Channel callback's invoked in softirq context */
struct tasklet_struct callback_event;
void (*onchannel_callback)(void *context);
-- 
2.14.1

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

[PATCH] staging: ccree: else is not generally useful after a break or return

2017-09-21 Thread sunil . m 
From: Suniel Mahesh 

Fixes checkpatch warnings:

WARNING: else is not generally useful after a break or return

Signed-off-by: Suniel Mahesh 
---
Note:
- Patch was tested and built(ARCH=arm) on next-20170921.
  No build issues reported.
---
 drivers/staging/ccree/ssi_request_mgr.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/ccree/ssi_request_mgr.c 
b/drivers/staging/ccree/ssi_request_mgr.c
index daa5432..25eecbb 100644
--- a/drivers/staging/ccree/ssi_request_mgr.c
+++ b/drivers/staging/ccree/ssi_request_mgr.c
@@ -387,10 +387,9 @@ int send_request(
 */
wait_for_completion(&ssi_req->seq_compl);
return 0;
-   } else {
-   /* Operation still in process */
-   return -EINPROGRESS;
}
+   /* Operation still in process */
+   return -EINPROGRESS;
 }
 
 /*!
-- 
1.9.1

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

[PATCH v2] staging: greybus: light: Release memory obtained by kasprintf

2017-09-21 Thread Arvind Yadav 
 - Free memory region, if gb_lights_channel_config is not successful.
 - No need to add check for gb_lights_channel_flash_config().

Signed-off-by: Arvind Yadav 
---
changes in v2:
- Subject line changed.
- add kfree in __gb_lights_led_unregister().
- No need to check return value of gb_lights_channel_flash_config().

 drivers/staging/greybus/light.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/greybus/light.c b/drivers/staging/greybus/light.c
index 3f4148c..bc1f8d2 100644
--- a/drivers/staging/greybus/light.c
+++ b/drivers/staging/greybus/light.c
@@ -926,6 +926,8 @@ static void __gb_lights_led_unregister(struct gb_channel 
*channel)
 
led_classdev_unregister(cdev);
channel->led = NULL;
+   kfree(cdev->name);
+   cdev->name = NULL;
 }
 
 static void gb_lights_channel_unregister(struct gb_channel *channel)
@@ -998,11 +1000,8 @@ static int gb_lights_channel_config(struct gb_light 
*light,
 
light->has_flash = true;
 
-   ret = gb_lights_channel_flash_config(channel);
-   if (ret < 0)
-   return ret;
+   return gb_lights_channel_flash_config(channel);
 
-   return ret;
 }
 
 static int gb_lights_light_config(struct gb_lights *glights, u8 id)
-- 
1.9.1

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

[PATCH] [media] atomisp2: remove cast from memory allocation

2017-09-21 Thread Aishwarya Pant 
Patch removes the following warning issued was coccicheck:
WARNING: casting value returned by memory allocation function to (char *) is
useless.

Signed-off-by: Aishwarya Pant 
---
 drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git 
a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c 
b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
index 6358216..53a7891 100644
--- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
+++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css_firmware.c
@@ -145,8 +145,8 @@ sh_css_load_blob_info(const char *fw, const struct 
ia_css_fw_info *bi, struct ia
size_t configstruct_size = sizeof(struct 
ia_css_config_memory_offsets);
size_t statestruct_size = sizeof(struct 
ia_css_state_memory_offsets);
 
-   char *parambuf = (char *)kmalloc(paramstruct_size + 
configstruct_size + statestruct_size,
-   GFP_KERNEL);
+   char *parambuf = kmalloc(paramstruct_size + configstruct_size + 
statestruct_size,
+GFP_KERNEL);
if (parambuf == NULL)
return IA_CSS_ERR_CANNOT_ALLOCATE_MEMORY;
 
-- 
2.7.4

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

Re: [PATCH v2] [media] staging: atomisp: use clock framework for camera clocks

2017-09-21 Thread Sakari Ailus 
Hi Pierre-Louis,

On Wed, Sep 20, 2017 at 03:53:58PM -0500, Pierre-Louis Bossart wrote:
> The Atom ISP driver initializes and configures PMC clocks which are
> already handled by the clock framework.
> 
> Remove all legacy vlv2_platform_clock stuff and move to the clk API to
> avoid conflicts, e.g. with audio machine drivers enabling the MCLK for
> external codecs
> 
> Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2")
> Tested-by: Carlo Caione 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Pierre-Louis Bossart 

I've applied the patch with small changes, there were other patches
changing the deleted files.

The tree is here:

https://git.linuxtv.org/sailus/media_tree.git/log/?h=atomisp>

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ai...@iki.fi
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 0/2] Drivers: hv: Miscellaneous fixes

2017-09-21 Thread kys 
From: "K. Y. Srinivasan" 

Miscellaneous fixes. Greg, please apply these to 4.14-final.

Dexuan Cui (1):
  vmbus: don't acquire the mutex in vmbus_hvsock_device_unregister()

Olaf Hering (1):
  Drivers: hv: fcopy: restore correct transfer length

 drivers/hv/channel_mgmt.c | 4 
 drivers/hv/hv_fcopy.c | 4 
 2 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.14.1

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

[PATCH 1/2] vmbus: don't acquire the mutex in vmbus_hvsock_device_unregister()

2017-09-21 Thread kys 
From: Dexuan Cui 

Due to commit 54a66265d675 ("Drivers: hv: vmbus: Fix rescind handling"),
we need this patch to resolve the below deadlock:

after we get the mutex in vmbus_hvsock_device_unregister() and call
vmbus_device_unregister() -> device_unregister() -> ... -> device_release()
-> vmbus_device_release(), we'll get a deadlock, because
vmbus_device_release() tries to get the same mutex.

Signed-off-by: Dexuan Cui 
Cc: K. Y. Srinivasan 
Cc: Haiyang Zhang 
Cc: Stephen Hemminger 
Signed-off-by: K. Y. Srinivasan 
Cc: sta...@vger.kernel.org (4.13 and above)
---
 drivers/hv/channel_mgmt.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 968af173c4c1..624d815745e4 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -945,14 +945,10 @@ static void vmbus_onoffer_rescind(struct 
vmbus_channel_message_header *hdr)
 
 void vmbus_hvsock_device_unregister(struct vmbus_channel *channel)
 {
-   mutex_lock(&vmbus_connection.channel_mutex);
-
BUG_ON(!is_hvsock_channel(channel));
 
channel->rescind = true;
vmbus_device_unregister(channel->device_obj);
-
-   mutex_unlock(&vmbus_connection.channel_mutex);
 }
 EXPORT_SYMBOL_GPL(vmbus_hvsock_device_unregister);
 
-- 
2.14.1

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

[PATCH 2/2] Drivers: hv: fcopy: restore correct transfer length

2017-09-21 Thread kys 
From: Olaf Hering 

Till recently the expected length of bytes read by the
daemon did depend on the context. It was either hv_start_fcopy or
hv_do_fcopy. The daemon had a buffer size of two pages, which was much
larger than needed.

Now the expected length of bytes read by the
daemon changed slightly. For START_FILE_COPY it is still the size of
hv_start_fcopy.  But for WRITE_TO_FILE and the other operations it is as
large as the buffer that arrived via vmbus. In case of WRITE_TO_FILE
that is slightly larger than a struct hv_do_fcopy. Since the buffer in
the daemon was still larger everything was fine.

Currently, the daemon reads only what is actually needed.
The new buffer layout is as large as a struct hv_do_fcopy, for the
WRITE_TO_FILE operation. Since the kernel expects a slightly larger
size, hvt_op_read will return -EINVAL because the daemon will read
slightly less than expected. Address this by restoring the expected
buffer size in case of WRITE_TO_FILE.

Fixes: 'c7e490fc23eb ("Drivers: hv: fcopy: convert to hv_utils_transport")'
Fixes: '3f2baa8a7d2e ("Tools: hv: update buffer handling in hv_fcopy_daemon")'

Signed-off-by: Olaf Hering 
Signed-off-by: K. Y. Srinivasan 
Cc: sta...@vger.kernel.org
---
 drivers/hv/hv_fcopy.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/hv/hv_fcopy.c b/drivers/hv/hv_fcopy.c
index daa75bd41f86..2364281d8593 100644
--- a/drivers/hv/hv_fcopy.c
+++ b/drivers/hv/hv_fcopy.c
@@ -170,6 +170,10 @@ static void fcopy_send_data(struct work_struct *dummy)
out_src = smsg_out;
break;
 
+   case WRITE_TO_FILE:
+   out_src = fcopy_transaction.fcopy_msg;
+   out_len = sizeof(struct hv_do_fcopy);
+   break;
default:
out_src = fcopy_transaction.fcopy_msg;
out_len = fcopy_transaction.recv_len;
-- 
2.14.1

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