Re: [PATCH v2 06/11] staging/android: turn fence_info into a __u64 pointer
Op 02-02-16 om 21:28 schreef Gustavo Padovan:
> Hi Maarten,
>
> 2016-02-02 Maarten Lankhorst :
>
>> Op 02-02-16 om 14:23 schreef Gustavo Padovan:
>>> From: Gustavo Padovan
>>>
>>> Making fence_info a pointer enables us to extend the struct in the future
>>> without breaking the ABI.
>>>
>>> v2: use type __u64 for fence_info
>>> Signed-off-by: Gustavo Padovan
>>> ---
>>> drivers/staging/android/sync.c | 2 +-
>>> drivers/staging/android/uapi/sync.h | 2 +-
>>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
>>> index f7530f0..03b1214 100644
>>> --- a/drivers/staging/android/sync.c
>>> +++ b/drivers/staging/android/sync.c
>>> @@ -525,7 +525,7 @@ static long sync_file_ioctl_fence_info(struct sync_file
>>> *sync_file,
>>> if (info->status >= 0)
>>> info->status = !info->status;
>>>
>>> - len = sizeof(struct sync_file_info);
>>> + len = sizeof(struct sync_file_info) - sizeof(__u64);
>>>
>>> for (i = 0; i < sync_file->num_fences; ++i) {
>>> struct fence *fence = sync_file->cbs[i].fence;
>>> diff --git a/drivers/staging/android/uapi/sync.h
>>> b/drivers/staging/android/uapi/sync.h
>>> index ed281fc..8e2ed32 100644
>>> --- a/drivers/staging/android/uapi/sync.h
>>> +++ b/drivers/staging/android/uapi/sync.h
>>> @@ -54,7 +54,7 @@ struct sync_file_info {
>>> charname[32];
>>> __s32 status;
>>>
>>> - __u8fence_info[0];
>>> + __u64 fence_info;
>>>
>> With the information from the commit I would expect fence_info would be a
>> true pointer.
> I missed that. It is fixed for v3 now.
>
>> so if (sync_file_info->num_fences == 0) not copying fence_info, but set all
>> members including this one.
>> else for_all_fences() copy_to_user(fence_info struct)
>>
>> Otherwise you still can't extend the struct without breaking the abi.
> all sync_files have at least 1 fence so we are left to the else case,
> coupy all fences to the user. So we always have variable length at the
> end of the struct. But I still don't see how this will prevent us from
> breaking the API.
Ok this was with fence_info not being a pointer. :)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] misc: Move panel driver out of staging
On Wed, Feb 3, 2016 at 10:19 AM, Joe Perches wrote: > On Wed, 2016-02-03 at 08:15 +0100, Ksenija Stanojevic wrote: >> Move panel driver from drivers/staging/panel to drivers/misc. > [] >> diff --git a/MAINTAINERS b/MAINTAINERS > [] >> @@ -10393,8 +10393,10 @@ F: drivers/staging/nvec/ >> >> STAGING - PARALLEL LCD/KEYPAD PANEL DRIVER > > Apologies for not noticing this before. > > Please remove the "STAGING - " from the section name > and relocate the entire section to the appropriate > place alphabetically in the MAINTAINER file. It's my fault for not noticing, I'll fix it and resend patch. Thanks, Ksenija ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5] misc: Move panel driver out of staging
Move panel driver from drivers/staging/panel to drivers/misc.
Signed-off-by: Ksenija Stanojevic
Acked-by: Willy Tarreau
---
Changes in v5:
- relocate the entire section to the appropriate place alphabetically
in the MAINTAINER file.
Changes in v4:
- modify driver location in MAINTAINERS file
Changes in v3:
- modify driver location in MAINTAINERS file
Changes in v2:
- modify MAINTAINERS file
- move lcd-panel-cgram.txt to Documentation/
.../misc-devices}/lcd-panel-cgram.txt | 0
MAINTAINERS| 12 +-
drivers/misc/Kconfig | 278 +
drivers/misc/Makefile | 1 +
drivers/{staging/panel => misc}/panel.c| 0
drivers/staging/Kconfig| 2 -
drivers/staging/Makefile | 1 -
drivers/staging/panel/Kconfig | 278 -
drivers/staging/panel/Makefile | 1 -
drivers/staging/panel/TODO | 8 -
10 files changed, 286 insertions(+), 295 deletions(-)
rename {drivers/staging/panel =>
Documentation/misc-devices}/lcd-panel-cgram.txt (100%)
rename drivers/{staging/panel => misc}/panel.c (100%)
delete mode 100644 drivers/staging/panel/Kconfig
delete mode 100644 drivers/staging/panel/Makefile
delete mode 100644 drivers/staging/panel/TODO
diff --git a/drivers/staging/panel/lcd-panel-cgram.txt
b/Documentation/misc-devices/lcd-panel-cgram.txt
similarity index 100%
rename from drivers/staging/panel/lcd-panel-cgram.txt
rename to Documentation/misc-devices/lcd-panel-cgram.txt
diff --git a/MAINTAINERS b/MAINTAINERS
index 369f6ae..7c39a48 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8168,6 +8168,13 @@ S: Maintained
F: Documentation/mn10300/
F: arch/mn10300/
+PARALLEL LCD/KEYPAD PANEL DRIVER
+M: Willy Tarreau
+M: Ksenija Stanojevic
+S: Odd Fixes
+F: Documentation/misc-devices/lcd-panel-cgram.txt
+F: drivers/misc/panel.c
+
PARALLEL PORT SUBSYSTEM
M: Sudip Mukherjee
M: Sudip Mukherjee
@@ -10391,11 +10398,6 @@ L: linux-te...@vger.kernel.org
S: Maintained
F: drivers/staging/nvec/
-STAGING - PARALLEL LCD/KEYPAD PANEL DRIVER
-M: Willy Tarreau
-S: Odd Fixes
-F: drivers/staging/panel/
-
STAGING - REALTEK RTL8712U DRIVERS
M: Larry Finger
M: Florian Schilhabel .
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 054fc10..f0ba782 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -525,6 +525,284 @@ config VEXPRESS_SYSCFG
ARM Ltd. Versatile Express uses specialised platform configuration
bus. System Configuration interface is one of the possible means
of generating transactions on this bus.
+config PANEL
+ tristate "Parallel port LCD/Keypad Panel support"
+ depends on PARPORT
+ ---help---
+ Say Y here if you have an HD44780 or KS-0074 LCD connected to your
+ parallel port. This driver also features 4 and 6-key keypads. The LCD
+ is accessible through the /dev/lcd char device (10, 156), and the
+ keypad through /dev/keypad (10, 185). Both require misc device to be
+ enabled. This code can either be compiled as a module, or linked into
+ the kernel and started at boot. If you don't understand what all this
+ is about, say N.
+
+config PANEL_PARPORT
+ int "Default parallel port number (0=LPT1)"
+ depends on PANEL
+ range 0 255
+ default "0"
+ ---help---
+ This is the index of the parallel port the panel is connected to. One
+ driver instance only supports one parallel port, so if your keypad
+ and LCD are connected to two separate ports, you have to start two
+ modules with different arguments. Numbering starts with '0' for LPT1,
+ and so on.
+
+config PANEL_PROFILE
+ int "Default panel profile (0-5, 0=custom)"
+ depends on PANEL
+ range 0 5
+ default "5"
+ ---help---
+ To ease configuration, the driver supports different configuration
+ profiles for past and recent wirings. These profiles can also be
+ used to define an approximative configuration, completed by a few
+ other options. Here are the profiles :
+
+ 0 = custom (see further)
+ 1 = 2x16 parallel LCD, old keypad
+ 2 = 2x16 serial LCD (KS-0074), new keypad
+ 3 = 2x16 parallel LCD (Hantronix), no keypad
+ 4 = 2x16 parallel LCD (Nexcom NSA1045) with Nexcom's keypad
+ 5 = 2x40 parallel LCD (old one), with old keypad
+
+ Custom configurations allow you to define how your display is
+ wired to the parallel port, and how it works. This is only intended
+ for experts.
+
+config PANEL_KEYPAD
+ depends on PANEL && PA
Re: [PATCHv2] staging: android: ion: use the manged version of DMA memory allocation
On 03/02/16 06:49, Gujulan Elango, Hari Prasath (H.) wrote:
From: Hari Prasath Gujulan Elango
Use the managed version of the dma_alloc_coherent() i.e. the
dmam_alloc_coherent() & accordingly cleanup the error handling
part.Also,remove the references to dma_free_coherent
That last aspect looks a bit off to me - the heaps don't seem to be
something that exist for the lifetime of the ION "device", given that
these are specific runtime alloc and free calls, rather than the probe
and remove routines. I don't know if CMA heaps are among those which ION
creates and destroys frequently (enough that it apparently kicks off a
whole background thread to manage the freeing), but this looks like a
recipe for leaks. If the free call doesn't actually free the buffer,
it's going to remain hanging around consuming precious CMA area until
the ION device itself is torn down, which is likely never.
I wouldn't say it's necessarily inappropriate to use managed DMA
resources here to cover unexpected failure cases for the ION device
itself (I see the comment in ion_device_remove()), but that means still
using dmam_free_coherent() when naturally releasing allocations for
other reasons (i.e. both cases here). Think Java finalisers, rather than
C++ destructors.
Robin.
Signed-off-by: Hari Prasath Gujulan Elango
---
v2:kbuild test robot reported warnings on ununsed
variables.Those warnings are fixed.
---
drivers/staging/android/ion/ion_cma_heap.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index a3446da..8cd720b 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -61,7 +61,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
if (!info)
return ION_CMA_ALLOCATE_FAILED;
- info->cpu_addr = dma_alloc_coherent(dev, len, &(info->handle),
+ info->cpu_addr = dmam_alloc_coherent(dev, len, &(info->handle),
GFP_HIGHUSER | __GFP_ZERO);
if (!info->cpu_addr) {
@@ -71,7 +71,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
info->table = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!info->table)
- goto free_mem;
+ goto err;
if (dma_get_sgtable(dev, info->table, info->cpu_addr, info->handle,
len))
@@ -83,8 +83,6 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
free_table:
kfree(info->table);
-free_mem:
- dma_free_coherent(dev, len, info->cpu_addr, info->handle);
err:
kfree(info);
return ION_CMA_ALLOCATE_FAILED;
@@ -92,13 +90,8 @@ err:
static void ion_cma_free(struct ion_buffer *buffer)
{
- struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
- struct device *dev = cma_heap->dev;
struct ion_cma_buffer_info *info = buffer->priv_virt;
- dev_dbg(dev, "Release buffer %p\n", buffer);
- /* release memory */
- dma_free_coherent(dev, buffer->size, info->cpu_addr, info->handle);
/* release sg table */
sg_free_table(info->table);
kfree(info->table);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4] misc: Move panel driver out of staging
On Wed, 2016-02-03 at 08:15 +0100, Ksenija Stanojevic wrote: > Move panel driver from drivers/staging/panel to drivers/misc. [] > diff --git a/MAINTAINERS b/MAINTAINERS [] > @@ -10393,8 +10393,10 @@ F: drivers/staging/nvec/ > > STAGING - PARALLEL LCD/KEYPAD PANEL DRIVER Apologies for not noticing this before. Please remove the "STAGING - " from the section name and relocate the entire section to the appropriate place alphabetically in the MAINTAINER file. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net-next] hv_netvsc: Increase delay for RNDIS_STATUS_NETWORK_CHANGE
Haiyang Zhang writes:
> We simulates a link down period for RNDIS_STATUS_NETWORK_CHANGE message to
> trigger DHCP renew. User daemons may need multiple seconds to trigger the
> link down event. (e.g. ifplugd: 5sec, network-manager: 4sec.) So update
> this link down period to 10 sec to properly trigger DHCP renew.
>
I probably don't follow: why do we need sucha a delay? If (with real
hardware) you plug network cable out and in one second you plug it in
you'll get DHCP renewed, right?
When I introduced RNDIS_STATUS_NETWORK_CHANGE handling by emulating a
pair of up/down events I put 2 second delay to make link_watch happy (as
we only handle 1 event per second there) but 10 seconds sounds to much
to me.
> Signed-off-by: Haiyang Zhang
> ---
> drivers/net/hyperv/netvsc_drv.c | 10 --
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
> index 1d3a665..6f23973 100644
> --- a/drivers/net/hyperv/netvsc_drv.c
> +++ b/drivers/net/hyperv/netvsc_drv.c
> @@ -43,6 +43,8 @@
>
> #define RING_SIZE_MIN 64
> #define LINKCHANGE_INT (2 * HZ)
> +/* Extra delay for RNDIS_STATUS_NETWORK_CHANGE: */
> +#define LINKCHANGE_DELAY (8 * HZ)
> static int ring_size = 128;
> module_param(ring_size, int, S_IRUGO);
> MODULE_PARM_DESC(ring_size, "Ring buffer size (# of pages)");
> @@ -964,6 +966,7 @@ static void netvsc_link_change(struct work_struct *w)
> return;
> }
> ndev_ctx->last_reconfig = jiffies;
> + delay = LINKCHANGE_INT;
>
> spin_lock_irqsave(&ndev_ctx->lock, flags);
> if (!list_empty(&ndev_ctx->reconfig_events)) {
> @@ -1009,8 +1012,11 @@ static void netvsc_link_change(struct work_struct *w)
> netif_tx_stop_all_queues(net);
> event->event = RNDIS_STATUS_MEDIA_CONNECT;
> spin_lock_irqsave(&ndev_ctx->lock, flags);
> - list_add_tail(&event->list, &ndev_ctx->reconfig_events);
> + list_add(&event->list, &ndev_ctx->reconfig_events);
Why? Adding to tail was here to not screw the order of events...
> spin_unlock_irqrestore(&ndev_ctx->lock, flags);
> +
> + ndev_ctx->last_reconfig += LINKCHANGE_DELAY;
> + delay = LINKCHANGE_INT + LINKCHANGE_DELAY;
> reschedule = true;
> }
> break;
> @@ -1025,7 +1031,7 @@ static void netvsc_link_change(struct work_struct *w)
>* second, handle next reconfig event in 2 seconds.
>*/
> if (reschedule)
> - schedule_delayed_work(&ndev_ctx->dwork, LINKCHANGE_INT);
> + schedule_delayed_work(&ndev_ctx->dwork, delay);
> }
>
> static void netvsc_free_netdev(struct net_device *netdev)
--
Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: coding style cleanups for staging/most/mostcore driver
On Tue, Feb 02, 2016 at 06:57:46AM +0530, Bijosh Thykkoottathil wrote: > Gentle reminder. Wait for two more weeks before reminding us. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 05/11] staging/android: remove len field from struct sync_fence_info
From: Gustavo Padovan
After removing driver_data struct sync_fence_info has now a fixed size,
thus it doesn't need any field to tell its size, it is already known.
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sync.c | 4 +---
drivers/staging/android/uapi/sync.h | 2 --
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index d527878..2ab0c20 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -486,8 +486,6 @@ static int sync_fill_fence_info(struct fence *fence, void
*data, int size)
if (size < sizeof(*info))
return -ENOMEM;
- info->len = sizeof(*info);
-
strlcpy(info->obj_name, fence->ops->get_timeline_name(fence),
sizeof(info->obj_name));
strlcpy(info->driver_name, fence->ops->get_driver_name(fence),
@@ -498,7 +496,7 @@ static int sync_fill_fence_info(struct fence *fence, void
*data, int size)
info->status = 0;
info->timestamp_ns = ktime_to_ns(fence->timestamp);
- return info->len;
+ return sizeof(*info);
}
static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
diff --git a/drivers/staging/android/uapi/sync.h
b/drivers/staging/android/uapi/sync.h
index 4d8cf00..a0cf357 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/drivers/staging/android/uapi/sync.h
@@ -28,14 +28,12 @@ struct sync_merge_data {
/**
* struct sync_fence_info - detailed fence information
- * @len: length of sync_fence_info
* @obj_name: name of parent sync_timeline
* @driver_name: name of driver implementing the parent
* @status:status of the fence 0:active 1:signaled <0:error
* @timestamp_ns: timestamp of status change in nanoseconds
*/
struct sync_fence_info {
- __u32 len;
charobj_name[32];
chardriver_name[32];
__s32 status;
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 01/11] staging/android: remove SYNC_WAIT ioctl
From: Gustavo Padovan
This ioctl is replicating the work of poll() syscall so let's take the
opportunity that this is still on staging tree and remove the duplication
and force new users to use the poll() standard interface.
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sync.c | 52
drivers/staging/android/sync.h | 13 -
drivers/staging/android/trace/sync.h | 44 --
drivers/staging/android/uapi/sync.h | 7 -
4 files changed, 116 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 5fa4779..381209a 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -300,44 +300,6 @@ struct sync_file *sync_file_merge(const char *name,
}
EXPORT_SYMBOL(sync_file_merge);
-int sync_file_wait(struct sync_file *sync_file, long timeout)
-{
- long ret;
- int i;
-
- if (timeout < 0)
- timeout = MAX_SCHEDULE_TIMEOUT;
- else
- timeout = msecs_to_jiffies(timeout);
-
- trace_sync_wait(sync_file, 1);
- for (i = 0; i < sync_file->num_fences; ++i)
- trace_fence(sync_file->cbs[i].fence);
- ret = wait_event_interruptible_timeout(sync_file->wq,
- atomic_read(&sync_file->status)
<= 0,
- timeout);
- trace_sync_wait(sync_file, 0);
-
- if (ret < 0) {
- return ret;
- } else if (ret == 0) {
- if (timeout) {
- pr_info("sync_file timeout on [%p] after %dms\n",
- sync_file, jiffies_to_msecs(timeout));
- sync_dump();
- }
- return -ETIME;
- }
-
- ret = atomic_read(&sync_file->status);
- if (ret) {
- pr_info("sync_file error %ld on [%p]\n", ret, sync_file);
- sync_dump();
- }
- return ret;
-}
-EXPORT_SYMBOL(sync_file_wait);
-
static const char *android_fence_get_driver_name(struct fence *fence)
{
struct sync_timeline *parent = fence_parent(fence);
@@ -478,17 +440,6 @@ static unsigned int sync_file_poll(struct file *file,
poll_table *wait)
return 0;
}
-static long sync_file_ioctl_wait(struct sync_file *sync_file,
- unsigned long arg)
-{
- __s32 value;
-
- if (copy_from_user(&value, (void __user *)arg, sizeof(value)))
- return -EFAULT;
-
- return sync_file_wait(sync_file, value);
-}
-
static long sync_file_ioctl_merge(struct sync_file *sync_file,
unsigned long arg)
{
@@ -629,9 +580,6 @@ static long sync_file_ioctl(struct file *file, unsigned int
cmd,
struct sync_file *sync_file = file->private_data;
switch (cmd) {
- case SYNC_IOC_WAIT:
- return sync_file_ioctl_wait(sync_file, arg);
-
case SYNC_IOC_MERGE:
return sync_file_ioctl_merge(sync_file, arg);
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index 8980b55..af1af99 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -18,7 +18,6 @@
#include
#include
#include
-#include
#include
#include "uapi/sync.h"
@@ -230,18 +229,6 @@ void sync_file_put(struct sync_file *sync_file);
*/
void sync_file_install(struct sync_file *sync_file, int fd);
-/**
- * sync_file_wait() - wait on sync file
- * @sync_file: file to wait on
- * @tiemout: timeout in ms
- *
- * Wait for @sync_file to be signaled or have an error. Waits indefinitely
- * if @timeout < 0.
- *
- * Returns 0 if fence signaled, > 0 if it is still active and <0 on error
- */
-int sync_file_wait(struct sync_file *sync_file, long timeout);
-
#ifdef CONFIG_DEBUG_FS
void sync_timeline_debug_add(struct sync_timeline *obj);
diff --git a/drivers/staging/android/trace/sync.h
b/drivers/staging/android/trace/sync.h
index 87c60e9..a0f80f4 100644
--- a/drivers/staging/android/trace/sync.h
+++ b/drivers/staging/android/trace/sync.h
@@ -32,50 +32,6 @@ TRACE_EVENT(sync_timeline,
TP_printk("name=%s value=%s", __get_str(name), __entry->value)
);
-TRACE_EVENT(sync_wait,
- TP_PROTO(struct sync_file *sync_file, int begin),
-
- TP_ARGS(sync_file, begin),
-
- TP_STRUCT__entry(
- __string(name, sync_file->name)
- __field(s32, status)
- __field(u32, begin)
- ),
-
- TP_fast_assign(
- __assign_str(name, sync_file->name);
- __entry->status = atomic_read(&sync_file->status);
- __entry->begin = begin;
- ),
-
- TP_printk("%s name=%s state=%d", __entry->begin ? "begin" : "end",
- __get_str(name), __entry->status)
-);
-
-TRACE_EVENT(fence,
- TP_PROTO(struct
[PATCH v3 06/11] staging/android: turn fence_info into a __u64 pointer
From: Gustavo Padovan
Turn sync_fence_info into __u64 type enable us to extend the struct in the
future without breaking the ABI.
v2: use type __u64 for fence_info
v3: fix commit message to reflect the v2 change
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sync.c | 2 +-
drivers/staging/android/uapi/sync.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 2ab0c20..8425457 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -525,7 +525,7 @@ static long sync_file_ioctl_fence_info(struct sync_file
*sync_file,
if (info->status >= 0)
info->status = !info->status;
- len = sizeof(struct sync_file_info);
+ len = sizeof(struct sync_file_info) - sizeof(__u64);
for (i = 0; i < sync_file->num_fences; ++i) {
struct fence *fence = sync_file->cbs[i].fence;
diff --git a/drivers/staging/android/uapi/sync.h
b/drivers/staging/android/uapi/sync.h
index a0cf357..e649953 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/drivers/staging/android/uapi/sync.h
@@ -54,7 +54,7 @@ struct sync_file_info {
charname[32];
__s32 status;
- __u8sync_fence_info[0];
+ __u64 sync_fence_info;
};
#define SYNC_IOC_MAGIC '>'
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 09/11] staging/android: rename SYNC_IOC_FENCE_INFO
From: Gustavo Padovan
We don't use the 'fence' name to refer to sync_file anymore. So rename it
to SYNC_IOC_FILE_INFO.
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sync.c | 2 +-
drivers/staging/android/uapi/sync.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index d6cf89f..36a6e62 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -569,7 +569,7 @@ static long sync_file_ioctl(struct file *file, unsigned int
cmd,
case SYNC_IOC_MERGE:
return sync_file_ioctl_merge(sync_file, arg);
- case SYNC_IOC_FENCE_INFO:
+ case SYNC_IOC_FILE_INFO:
return sync_file_ioctl_fence_info(sync_file, arg);
default:
diff --git a/drivers/staging/android/uapi/sync.h
b/drivers/staging/android/uapi/sync.h
index 4e1d38b..6b687a8 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/drivers/staging/android/uapi/sync.h
@@ -80,6 +80,6 @@ struct sync_file_info {
* pt_info is a buffer containing sync_pt_infos for every sync_pt in the fence.
* To iterate over the sync_pt_infos, use the sync_pt_info.len field.
*/
-#define SYNC_IOC_FENCE_INFO_IOWR(SYNC_IOC_MAGIC, 2, struct sync_file_info)
+#define SYNC_IOC_FILE_INFO _IOWR(SYNC_IOC_MAGIC, 2, struct sync_file_info)
#endif /* _UAPI_LINUX_SYNC_H */
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 03/11] staging/android: rename sync_file_info_data to sync_file_info
From: Gustavo Padovan
info_data is a bit redundant, let's keep it as only sync_file_info. It is
also smaller.
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sync.c | 26 +-
drivers/staging/android/uapi/sync.h | 9 -
2 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index f6ffb10..85a0e87 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -525,7 +525,7 @@ static int sync_fill_fence_info(struct fence *fence, void
*data, int size)
static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
unsigned long arg)
{
- struct sync_file_info_data *data;
+ struct sync_file_info *info;
__u32 size;
__u32 len = 0;
int ret, i;
@@ -533,27 +533,27 @@ static long sync_file_ioctl_fence_info(struct sync_file
*sync_file,
if (copy_from_user(&size, (void __user *)arg, sizeof(size)))
return -EFAULT;
- if (size < sizeof(struct sync_file_info_data))
+ if (size < sizeof(struct sync_file_info))
return -EINVAL;
if (size > 4096)
size = 4096;
- data = kzalloc(size, GFP_KERNEL);
- if (!data)
+ info = kzalloc(size, GFP_KERNEL);
+ if (!info)
return -ENOMEM;
- strlcpy(data->name, sync_file->name, sizeof(data->name));
- data->status = atomic_read(&sync_file->status);
- if (data->status >= 0)
- data->status = !data->status;
+ strlcpy(info->name, sync_file->name, sizeof(info->name));
+ info->status = atomic_read(&sync_file->status);
+ if (info->status >= 0)
+ info->status = !info->status;
- len = sizeof(struct sync_file_info_data);
+ len = sizeof(struct sync_file_info);
for (i = 0; i < sync_file->num_fences; ++i) {
struct fence *fence = sync_file->cbs[i].fence;
- ret = sync_fill_fence_info(fence, (u8 *)data + len, size - len);
+ ret = sync_fill_fence_info(fence, (u8 *)info + len, size - len);
if (ret < 0)
goto out;
@@ -561,15 +561,15 @@ static long sync_file_ioctl_fence_info(struct sync_file
*sync_file,
len += ret;
}
- data->len = len;
+ info->len = len;
- if (copy_to_user((void __user *)arg, data, len))
+ if (copy_to_user((void __user *)arg, info, len))
ret = -EFAULT;
else
ret = 0;
out:
- kfree(data);
+ kfree(info);
return ret;
}
diff --git a/drivers/staging/android/uapi/sync.h
b/drivers/staging/android/uapi/sync.h
index 9a0c3cd..fcc0b3c 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/drivers/staging/android/uapi/sync.h
@@ -46,15 +46,15 @@ struct sync_fence_info {
};
/**
- * struct sync_file_info_data - data returned from fence info ioctl
+ * struct sync_file_info - data returned from fence info ioctl
* @len: ioctl caller writes the size of the buffer its passing in.
- * ioctl returns length of sync_file_info_data returned to
+ * ioctl returns length of sync_file_info returned to
* userspace including pt_info.
* @name: name of fence
* @status:status of fence. 1: signaled 0:active <0:error
* @sync_fence_info: array of sync_fence_info for every fence in the sync_file
*/
-struct sync_file_info_data {
+struct sync_file_info {
__u32 len;
charname[32];
__s32 status;
@@ -84,7 +84,6 @@ struct sync_file_info_data {
* pt_info is a buffer containing sync_pt_infos for every sync_pt in the fence.
* To iterate over the sync_pt_infos, use the sync_pt_info.len field.
*/
-#define SYNC_IOC_FENCE_INFO_IOWR(SYNC_IOC_MAGIC, 2,\
- struct sync_file_info_data)
+#define SYNC_IOC_FENCE_INFO_IOWR(SYNC_IOC_MAGIC, 2, struct sync_file_info)
#endif /* _UAPI_LINUX_SYNC_H */
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 08/11] staging/android: make info->len return only size of sync_fence_info array
From: Gustavo Padovan
The len member of struct sync_file_info was returning the size of the whole
buffer (struct sync_file_info + sync_fence_infos at the of it). This commit
change it to return only the size of the array of sync_fence_infos.
It also moves len to be right before the sync_fences_info field.
v2: fix check for name field (Maarten)
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sync.c | 17 -
drivers/staging/android/uapi/sync.h | 7 +++
2 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index e301b55..d6cf89f 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -502,14 +502,20 @@ static int sync_fill_fence_info(struct fence *fence, void
*data, int size)
static long sync_file_ioctl_fence_info(struct sync_file *sync_file,
unsigned long arg)
{
- struct sync_file_info *info;
+ struct sync_file_info in, *info;
__u32 size;
- __u32 len = 0;
+ __u32 b_len, len = 0;
int ret, i;
- if (copy_from_user(&size, (void __user *)arg, sizeof(size)))
+ if (copy_from_user(&in, (void __user *)arg, sizeof(*info)))
return -EFAULT;
+ if (in.status || in.num_fences || in.sync_fence_info ||
+ strcmp(in.name, "\0"))
+ return -EFAULT;
+
+ size = in.len;
+
if (size < sizeof(struct sync_file_info))
return -EINVAL;
@@ -527,8 +533,9 @@ static long sync_file_ioctl_fence_info(struct sync_file
*sync_file,
info->num_fences = sync_file->num_fences;
- len = sizeof(struct sync_file_info) - sizeof(__u64);
+ b_len = sizeof(struct sync_file_info) - sizeof(__u64);
+ len = b_len;
for (i = 0; i < sync_file->num_fences; ++i) {
struct fence *fence = sync_file->cbs[i].fence;
@@ -540,7 +547,7 @@ static long sync_file_ioctl_fence_info(struct sync_file
*sync_file,
len += ret;
}
- info->len = len;
+ info->len = len - b_len;
if (copy_to_user((void __user *)arg, info, len))
ret = -EFAULT;
diff --git a/drivers/staging/android/uapi/sync.h
b/drivers/staging/android/uapi/sync.h
index fc7fbcf..4e1d38b 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/drivers/staging/android/uapi/sync.h
@@ -42,19 +42,18 @@ struct sync_fence_info {
/**
* struct sync_file_info - data returned from fence info ioctl
- * @len: ioctl caller writes the size of the buffer its passing in.
- * ioctl returns length of sync_file_info returned to
- * userspace including pt_info.
* @name: name of fence
* @status:status of fence. 1: signaled 0:active <0:error
* @num_fences number of fences in the sync_file
+ * @len: ioctl caller writes the size of the buffer its passing in.
+ * ioctl returns length of all fence_infos summed.
* @sync_fence_info: array of sync_fence_info for every fence in the sync_file
*/
struct sync_file_info {
- __u32 len;
charname[32];
__s32 status;
__u32 num_fences;
+ __u32 len;
__u64 sync_fence_info;
};
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 07/11] staging/android: add num_fences field to struct sync_file_info
From: Gustavo Padovan
Inform userspace how many fences are in the sync_fence_info field.
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sync.c | 2 ++
drivers/staging/android/uapi/sync.h | 2 ++
2 files changed, 4 insertions(+)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 8425457..e301b55 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -525,6 +525,8 @@ static long sync_file_ioctl_fence_info(struct sync_file
*sync_file,
if (info->status >= 0)
info->status = !info->status;
+ info->num_fences = sync_file->num_fences;
+
len = sizeof(struct sync_file_info) - sizeof(__u64);
for (i = 0; i < sync_file->num_fences; ++i) {
diff --git a/drivers/staging/android/uapi/sync.h
b/drivers/staging/android/uapi/sync.h
index e649953..fc7fbcf 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/drivers/staging/android/uapi/sync.h
@@ -47,12 +47,14 @@ struct sync_fence_info {
* userspace including pt_info.
* @name: name of fence
* @status:status of fence. 1: signaled 0:active <0:error
+ * @num_fences number of fences in the sync_file
* @sync_fence_info: array of sync_fence_info for every fence in the sync_file
*/
struct sync_file_info {
__u32 len;
charname[32];
__s32 status;
+ __u32 num_fences;
__u64 sync_fence_info;
};
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 02/11] staging/android: rename sync_pt_info to sync_fence_info
From: Gustavo Padovan
As struct sync_pt doesn't exist anymore it is a good idea remove any
reference to it in the sync_framework. sync_pts were replaced directly by
fences and here we rename it to sync_fence_info to let the fence namespace
clean.
v2: rename fence_info to sync_fence_info (Maarten)
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sync.c | 10 +-
drivers/staging/android/uapi/sync.h | 12 ++--
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 381209a..f6ffb10 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -490,15 +490,15 @@ err_put_fd:
return err;
}
-static int sync_fill_pt_info(struct fence *fence, void *data, int size)
+static int sync_fill_fence_info(struct fence *fence, void *data, int size)
{
- struct sync_pt_info *info = data;
+ struct sync_fence_info *info = data;
int ret;
- if (size < sizeof(struct sync_pt_info))
+ if (size < sizeof(*info))
return -ENOMEM;
- info->len = sizeof(struct sync_pt_info);
+ info->len = sizeof(*info);
if (fence->ops->fill_driver_data) {
ret = fence->ops->fill_driver_data(fence, info->driver_data,
@@ -553,7 +553,7 @@ static long sync_file_ioctl_fence_info(struct sync_file
*sync_file,
for (i = 0; i < sync_file->num_fences; ++i) {
struct fence *fence = sync_file->cbs[i].fence;
- ret = sync_fill_pt_info(fence, (u8 *)data + len, size - len);
+ ret = sync_fill_fence_info(fence, (u8 *)data + len, size - len);
if (ret < 0)
goto out;
diff --git a/drivers/staging/android/uapi/sync.h
b/drivers/staging/android/uapi/sync.h
index 6c88c80..9a0c3cd 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/drivers/staging/android/uapi/sync.h
@@ -27,15 +27,15 @@ struct sync_merge_data {
};
/**
- * struct sync_pt_info - detailed sync_pt information
- * @len: length of sync_pt_info including any driver_data
+ * struct sync_fence_info - detailed fence information
+ * @len: length of sync_fence_info including any driver_data
* @obj_name: name of parent sync_timeline
* @driver_name: name of driver implementing the parent
- * @status:status of the sync_pt 0:active 1:signaled <0:error
+ * @status:status of the fence 0:active 1:signaled <0:error
* @timestamp_ns: timestamp of status change in nanoseconds
* @driver_data: any driver dependent data
*/
-struct sync_pt_info {
+struct sync_fence_info {
__u32 len;
charobj_name[32];
chardriver_name[32];
@@ -52,14 +52,14 @@ struct sync_pt_info {
* userspace including pt_info.
* @name: name of fence
* @status:status of fence. 1: signaled 0:active <0:error
- * @pt_info: a sync_pt_info struct for every sync_pt in the fence
+ * @sync_fence_info: array of sync_fence_info for every fence in the sync_file
*/
struct sync_file_info_data {
__u32 len;
charname[32];
__s32 status;
- __u8pt_info[0];
+ __u8sync_fence_info[0];
};
#define SYNC_IOC_MAGIC '>'
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 00/11] android sync framework: clean up IOCTLs and ABI
From: Gustavo Padovan Hi, This patch series clean up IOCTLs and abi of sync framework and it is a follow up on the clean up series I've sent on Jan 21: http://thread.gmane.org/gmane.comp.video.dri.devel/145509 The main changes here are: * remove of SYNC_WAIT ioctl, poll() should be used instead. * rename some structs and macros to better reflect the new internal names. * clean up and improve ABI on SYNC_IOC_FILE_INFO * add flags filed to all ABI structs * make sync_file_info->len return only the size of the array of fence_infos v2: - Check flags passed to the kernel (Maarten) - Rework len to report only the size of fence_infos array (Emil) - Fix fence_info type. Use __u64 pointer type (Emil, Maarten) v3: - Rename fence_info to sync_fence_info (Maarten) - Fix check for info.name (Maarten) - Update commit description of patch 06 (Emil, Maarten) Please review! Thanks. Gustavo Padovan (11): staging/android: remove SYNC_WAIT ioctl staging/android: rename sync_pt_info to sync_fence_info staging/android: rename sync_file_info_data to sync_file_info staging/android: remove driver_data from struct sync_fence_info staging/android: remove len field from struct sync_fence_info staging/android: turn fence_info into a __u64 pointer staging/android: add num_fences field to struct sync_file_info staging/android: make info->len return only size of sync_fence_info array staging/android: rename SYNC_IOC_FENCE_INFO staging/android: add flags member to sync ioctl structs staging/android: remove redundant comments on sync_merge_data drivers/staging/android/sw_sync.c| 14 drivers/staging/android/sync.c | 129 +-- drivers/staging/android/sync.h | 20 -- drivers/staging/android/trace/sync.h | 44 drivers/staging/android/uapi/sync.h | 50 ++ 5 files changed, 56 insertions(+), 201 deletions(-) -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 04/11] staging/android: remove driver_data from struct sync_fence_info
From: Gustavo Padovan
It is unclear in what situations driver_data should be used thus better do
not upstream it for now. If a need arises in the future a discussion can
be started to re-add it.
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sw_sync.c | 14 --
drivers/staging/android/sync.c | 21 -
drivers/staging/android/sync.h | 7 ---
drivers/staging/android/uapi/sync.h | 5 +
4 files changed, 1 insertion(+), 46 deletions(-)
diff --git a/drivers/staging/android/sw_sync.c
b/drivers/staging/android/sw_sync.c
index 3bee959..af39ff5 100644
--- a/drivers/staging/android/sw_sync.c
+++ b/drivers/staging/android/sw_sync.c
@@ -47,19 +47,6 @@ static int sw_sync_fence_has_signaled(struct fence *fence)
return (pt->value > obj->value) ? 0 : 1;
}
-static int sw_sync_fill_driver_data(struct fence *fence,
- void *data, int size)
-{
- struct sw_sync_pt *pt = (struct sw_sync_pt *)fence;
-
- if (size < sizeof(pt->value))
- return -ENOMEM;
-
- memcpy(data, &pt->value, sizeof(pt->value));
-
- return sizeof(pt->value);
-}
-
static void sw_sync_timeline_value_str(struct sync_timeline *sync_timeline,
char *str, int size)
{
@@ -78,7 +65,6 @@ static void sw_sync_fence_value_str(struct fence *fence, char
*str, int size)
static struct sync_timeline_ops sw_sync_timeline_ops = {
.driver_name = "sw_sync",
.has_signaled = sw_sync_fence_has_signaled,
- .fill_driver_data = sw_sync_fill_driver_data,
.timeline_value_str = sw_sync_timeline_value_str,
.fence_value_str = sw_sync_fence_value_str,
};
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 85a0e87..d527878 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -351,16 +351,6 @@ static bool android_fence_enable_signaling(struct fence
*fence)
return true;
}
-static int android_fence_fill_driver_data(struct fence *fence,
- void *data, int size)
-{
- struct sync_timeline *parent = fence_parent(fence);
-
- if (!parent->ops->fill_driver_data)
- return 0;
- return parent->ops->fill_driver_data(fence, data, size);
-}
-
static void android_fence_value_str(struct fence *fence,
char *str, int size)
{
@@ -394,7 +384,6 @@ static const struct fence_ops android_fence_ops = {
.signaled = android_fence_signaled,
.wait = fence_default_wait,
.release = android_fence_release,
- .fill_driver_data = android_fence_fill_driver_data,
.fence_value_str = android_fence_value_str,
.timeline_value_str = android_fence_timeline_value_str,
};
@@ -493,22 +482,12 @@ err_put_fd:
static int sync_fill_fence_info(struct fence *fence, void *data, int size)
{
struct sync_fence_info *info = data;
- int ret;
if (size < sizeof(*info))
return -ENOMEM;
info->len = sizeof(*info);
- if (fence->ops->fill_driver_data) {
- ret = fence->ops->fill_driver_data(fence, info->driver_data,
- size - sizeof(*info));
- if (ret < 0)
- return ret;
-
- info->len += ret;
- }
-
strlcpy(info->obj_name, fence->ops->get_timeline_name(fence),
sizeof(info->obj_name));
strlcpy(info->driver_name, fence->ops->get_driver_name(fence),
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index af1af99..d2a1734 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -32,10 +32,6 @@ struct sync_file;
* 1 if pt has signaled
* 0 if pt has not signaled
* <0 on error
- * @fill_driver_data: write implementation specific driver data to data.
- * should return an error if there is not enough room
- * as specified by size. This information is returned
- * to userspace by SYNC_IOC_FENCE_INFO.
* @timeline_value_str: fill str with the value of the sync_timeline's counter
* @fence_value_str: fill str with the value of the fence
*/
@@ -46,9 +42,6 @@ struct sync_timeline_ops {
int (*has_signaled)(struct fence *fence);
/* optional */
- int (*fill_driver_data)(struct fence *fence, void *data, int size);
-
- /* optional */
void (*timeline_value_str)(struct sync_timeline *timeline, char *str,
int size);
diff --git a/drivers/staging/android/uapi/sync.h
b/drivers/staging/android/uapi/sync.h
index fcc0b3c..4d8cf00 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/drivers/staging/android/uapi/sync.h
@@ -28,12 +28,11 @@ struct sync_merge_data
[PATCH v3 10/11] staging/android: add flags member to sync ioctl structs
From: Gustavo Padovan
Play safe and add flags member to all structs. So we don't need to
break API or create new IOCTL in the future if new features that requires
flags arises.
v2: check if flags are valid (zero, in this case)
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/sync.c | 7 ++-
drivers/staging/android/uapi/sync.h | 6 ++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index 36a6e62..876a57b 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -445,6 +445,11 @@ static long sync_file_ioctl_merge(struct sync_file
*sync_file,
goto err_put_fd;
}
+ if (data.flags) {
+ err = -EFAULT;
+ goto err_put_fd;
+ }
+
fence2 = sync_file_fdget(data.fd2);
if (!fence2) {
err = -ENOENT;
@@ -510,7 +515,7 @@ static long sync_file_ioctl_fence_info(struct sync_file
*sync_file,
if (copy_from_user(&in, (void __user *)arg, sizeof(*info)))
return -EFAULT;
- if (in.status || in.num_fences || in.sync_fence_info ||
+ if (in.status || in.flags || in.num_fences || in.sync_fence_info ||
strcmp(in.name, "\0"))
return -EFAULT;
diff --git a/drivers/staging/android/uapi/sync.h
b/drivers/staging/android/uapi/sync.h
index 6b687a8..48c6d45 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/drivers/staging/android/uapi/sync.h
@@ -19,11 +19,13 @@
* @fd2: file descriptor of second fence
* @name: name of new fence
* @fence: returns the fd of the new fence to userspace
+ * @flags: merge_data flags
*/
struct sync_merge_data {
__s32 fd2; /* fd of second fence */
charname[32]; /* name of new fence */
__s32 fence; /* fd on newly created fence */
+ __u32 flags;
};
/**
@@ -31,12 +33,14 @@ struct sync_merge_data {
* @obj_name: name of parent sync_timeline
* @driver_name: name of driver implementing the parent
* @status:status of the fence 0:active 1:signaled <0:error
+ * @flags: fence_info flags
* @timestamp_ns: timestamp of status change in nanoseconds
*/
struct sync_fence_info {
charobj_name[32];
chardriver_name[32];
__s32 status;
+ __u32 flags;
__u64 timestamp_ns;
};
@@ -44,6 +48,7 @@ struct sync_fence_info {
* struct sync_file_info - data returned from fence info ioctl
* @name: name of fence
* @status:status of fence. 1: signaled 0:active <0:error
+ * @flags: sync_file_info flags
* @num_fences number of fences in the sync_file
* @len: ioctl caller writes the size of the buffer its passing in.
* ioctl returns length of all fence_infos summed.
@@ -52,6 +57,7 @@ struct sync_fence_info {
struct sync_file_info {
charname[32];
__s32 status;
+ __u32 flags;
__u32 num_fences;
__u32 len;
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 11/11] staging/android: remove redundant comments on sync_merge_data
From: Gustavo Padovan
struct sync_merge_data already have documentation on top of the
struct definition. No need to duplicate it.
Signed-off-by: Gustavo Padovan
---
drivers/staging/android/uapi/sync.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/android/uapi/sync.h
b/drivers/staging/android/uapi/sync.h
index 48c6d45..f56a6c2 100644
--- a/drivers/staging/android/uapi/sync.h
+++ b/drivers/staging/android/uapi/sync.h
@@ -22,9 +22,9 @@
* @flags: merge_data flags
*/
struct sync_merge_data {
- __s32 fd2; /* fd of second fence */
- charname[32]; /* name of new fence */
- __s32 fence; /* fd on newly created fence */
+ __s32 fd2;
+ charname[32];
+ __s32 fence;
__u32 flags;
};
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 06/11] staging/android: turn fence_info into a __u64 pointer
Op 03-02-16 om 14:25 schreef Gustavo Padovan:
> From: Gustavo Padovan
>
> Turn sync_fence_info into __u64 type enable us to extend the struct in the
> future without breaking the ABI.
>
> v2: use type __u64 for fence_info
>
> v3: fix commit message to reflect the v2 change
>
> Signed-off-by: Gustavo Padovan
> ---
> drivers/staging/android/sync.c | 2 +-
> drivers/staging/android/uapi/sync.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
> index 2ab0c20..8425457 100644
> --- a/drivers/staging/android/sync.c
> +++ b/drivers/staging/android/sync.c
> @@ -525,7 +525,7 @@ static long sync_file_ioctl_fence_info(struct sync_file
> *sync_file,
> if (info->status >= 0)
> info->status = !info->status;
>
> - len = sizeof(struct sync_file_info);
> + len = sizeof(struct sync_file_info) - sizeof(__u64);
>
> for (i = 0; i < sync_file->num_fences; ++i) {
> struct fence *fence = sync_file->cbs[i].fence;
> diff --git a/drivers/staging/android/uapi/sync.h
> b/drivers/staging/android/uapi/sync.h
> index a0cf357..e649953 100644
> --- a/drivers/staging/android/uapi/sync.h
> +++ b/drivers/staging/android/uapi/sync.h
> @@ -54,7 +54,7 @@ struct sync_file_info {
> charname[32];
> __s32 status;
>
> - __u8sync_fence_info[0];
> + __u64 sync_fence_info;
> };
>
> #define SYNC_IOC_MAGIC '>'
This still doesn't do what you expect it to.
I think this is what you want is for userspace to do:
struct sync_file_info info;
info.flags = info.num_fences = 0;
ioctl(fd, SYNC_IOC_FENCE_INFO, &info);
if (info.num_fences) {
info.sync_fence_info = (uintptr)kcalloc(info.num_fences, sizeof(struct
sync_fence_info));
ioctl(fd, SYNC_IOC_FENCE_INFO, &info);
}
Maybe userspace could preallocate the max in advance and set num_fences higher,
kernel would do something like:
num_fences = min(info.num_fences, sync->num_fences);
struct sync_fence_info array[num_fences];
info.num_fences = sync->num_fences;
if (num_fences &&
copy_to_user((void * __user)(unsigned long)info.sync_fence_info, array,
num_fences * sizeof(array)))
return -EFAULT;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH net-next] hv_netvsc: Increase delay for RNDIS_STATUS_NETWORK_CHANGE
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Wednesday, February 3, 2016 8:06 AM > To: Haiyang Zhang > Cc: da...@davemloft.net; net...@vger.kernel.org; KY Srinivasan > ; o...@aepfle.de; linux-ker...@vger.kernel.org; > driverdev-devel@linuxdriverproject.org > Subject: Re: [PATCH net-next] hv_netvsc: Increase delay for > RNDIS_STATUS_NETWORK_CHANGE > > Haiyang Zhang writes: > > > We simulates a link down period for RNDIS_STATUS_NETWORK_CHANGE > message to > > trigger DHCP renew. User daemons may need multiple seconds to trigger > the > > link down event. (e.g. ifplugd: 5sec, network-manager: 4sec.) So update > > this link down period to 10 sec to properly trigger DHCP renew. > > > > I probably don't follow: why do we need sucha a delay? If (with real > hardware) you plug network cable out and in one second you plug it in > you'll get DHCP renewed, right? > > When I introduced RNDIS_STATUS_NETWORK_CHANGE handling by > emulating a > pair of up/down events I put 2 second delay to make link_watch happy (as > we only handle 1 event per second there) but 10 seconds sounds to much > to me. In the test on Hyper-V, our software on host side wants to trigger DHCP renew by sending only a RNDIS_STATUS_NETWORK_CHANGE message to a guest without physically unplug the cable. But, this message didn't trigger DHCP renew within 2 seconds. The VM is Centos 7.1 using Networkmanager, which needs 4 seconds after link down to renew IP. Some daemon, like ifplugd, needs 5 sec to renew. That's why we increase the simulated link down time for RNDIS_STATUS_NETWORK_CHANGE message. > > @@ -1009,8 +1012,11 @@ static void netvsc_link_change(struct > work_struct *w) > > netif_tx_stop_all_queues(net); > > event->event = RNDIS_STATUS_MEDIA_CONNECT; > > spin_lock_irqsave(&ndev_ctx->lock, flags); > > - list_add_tail(&event->list, &ndev_ctx- > >reconfig_events); > > + list_add(&event->list, &ndev_ctx->reconfig_events); > > Why? Adding to tail was here to not screw the order of events... The RNDIS_STATUS_MEDIA_CONNECT message triggers two "half" events -- link down & up. After "link down", we want the paired "link up" to be the immediate next event. Since the function picks the next event from the list head, so it should be inserted to list head. Otherwise, the possible existing events in the list will be processed in the middle of these two "half events" -- link down & up. Thanks, - Haiyang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net-next] hv_netvsc: Increase delay for RNDIS_STATUS_NETWORK_CHANGE
Haiyang Zhang writes: >> -Original Message- >> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] >> Sent: Wednesday, February 3, 2016 8:06 AM >> To: Haiyang Zhang >> Cc: da...@davemloft.net; net...@vger.kernel.org; KY Srinivasan >> ; o...@aepfle.de; linux-ker...@vger.kernel.org; >> driverdev-devel@linuxdriverproject.org >> Subject: Re: [PATCH net-next] hv_netvsc: Increase delay for >> RNDIS_STATUS_NETWORK_CHANGE >> >> Haiyang Zhang writes: >> >> > We simulates a link down period for RNDIS_STATUS_NETWORK_CHANGE >> message to >> > trigger DHCP renew. User daemons may need multiple seconds to trigger >> the >> > link down event. (e.g. ifplugd: 5sec, network-manager: 4sec.) So update >> > this link down period to 10 sec to properly trigger DHCP renew. >> > >> >> I probably don't follow: why do we need sucha a delay? If (with real >> hardware) you plug network cable out and in one second you plug it in >> you'll get DHCP renewed, right? >> >> When I introduced RNDIS_STATUS_NETWORK_CHANGE handling by >> emulating a >> pair of up/down events I put 2 second delay to make link_watch happy (as >> we only handle 1 event per second there) but 10 seconds sounds to much >> to me. > > In the test on Hyper-V, our software on host side wants to trigger DHCP > renew by sending only a RNDIS_STATUS_NETWORK_CHANGE message to > a guest without physically unplug the cable. But, this message didn't trigger > DHCP renew within 2 seconds. The VM is Centos 7.1 using Networkmanager, > which needs 4 seconds after link down to renew IP. Some daemon, like > ifplugd, needs 5 sec to renew. That's why we increase the simulated link > down time for RNDIS_STATUS_NETWORK_CHANGE message. Yes, I understand the motivation but sorry if I was unclear with my question. I meant to say that with physical network adapters it's possible to trigger same two events by plugging your cable out and then plugging it back in and it is certailnly doable in less than 10 seconds. NetworkManager or whoever is supposed to handle these events and we don't really care how fast -- I think that 4 or 5 seconds mentioned above is just an observation. > >> > @@ -1009,8 +1012,11 @@ static void netvsc_link_change(struct >> work_struct *w) >> >netif_tx_stop_all_queues(net); >> >event->event = RNDIS_STATUS_MEDIA_CONNECT; >> >spin_lock_irqsave(&ndev_ctx->lock, flags); >> > - list_add_tail(&event->list, &ndev_ctx- >> >reconfig_events); >> > + list_add(&event->list, &ndev_ctx->reconfig_events); >> >> Why? Adding to tail was here to not screw the order of events... > > The RNDIS_STATUS_MEDIA_CONNECT message triggers two "half" events -- > link down & up. After "link down", we want the paired "link up" to be the > immediate next event. Since the function picks the next event from the list > head, so it should be inserted to list head. Otherwise, the possible existing > events in the list will be processed in the middle of these two "half events" > -- link down & up. Ah, yes, you're probably right. -- Vitaly ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH net-next] hv_netvsc: Increase delay for RNDIS_STATUS_NETWORK_CHANGE
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Wednesday, February 3, 2016 11:06 AM > To: Haiyang Zhang > Cc: da...@davemloft.net; net...@vger.kernel.org; KY Srinivasan > ; o...@aepfle.de; linux-ker...@vger.kernel.org; > driverdev-devel@linuxdriverproject.org > Subject: Re: [PATCH net-next] hv_netvsc: Increase delay for > RNDIS_STATUS_NETWORK_CHANGE > > Haiyang Zhang writes: > > >> -Original Message- > >> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > >> Sent: Wednesday, February 3, 2016 8:06 AM > >> To: Haiyang Zhang > >> Cc: da...@davemloft.net; net...@vger.kernel.org; KY Srinivasan > >> ; o...@aepfle.de; linux-ker...@vger.kernel.org; > >> driverdev-devel@linuxdriverproject.org > >> Subject: Re: [PATCH net-next] hv_netvsc: Increase delay for > >> RNDIS_STATUS_NETWORK_CHANGE > >> > >> Haiyang Zhang writes: > >> > >> > We simulates a link down period for > RNDIS_STATUS_NETWORK_CHANGE > >> message to > >> > trigger DHCP renew. User daemons may need multiple seconds to > trigger > >> the > >> > link down event. (e.g. ifplugd: 5sec, network-manager: 4sec.) So update > >> > this link down period to 10 sec to properly trigger DHCP renew. > >> > > >> > >> I probably don't follow: why do we need sucha a delay? If (with real > >> hardware) you plug network cable out and in one second you plug it in > >> you'll get DHCP renewed, right? > >> > >> When I introduced RNDIS_STATUS_NETWORK_CHANGE handling by > >> emulating a > >> pair of up/down events I put 2 second delay to make link_watch happy > (as > >> we only handle 1 event per second there) but 10 seconds sounds to much > >> to me. > > > > In the test on Hyper-V, our software on host side wants to trigger DHCP > > renew by sending only a RNDIS_STATUS_NETWORK_CHANGE message to > > a guest without physically unplug the cable. But, this message didn't > > trigger > > DHCP renew within 2 seconds. The VM is Centos 7.1 using > Networkmanager, > > which needs 4 seconds after link down to renew IP. Some daemon, like > > ifplugd, needs 5 sec to renew. That's why we increase the simulated link > > down time for RNDIS_STATUS_NETWORK_CHANGE message. > > Yes, I understand the motivation but sorry if I was unclear with my > question. I meant to say that with physical network adapters it's > possible to trigger same two events by plugging your cable out and then > plugging it back in and it is certailnly doable in less than 10 > seconds. NetworkManager or whoever is supposed to handle these events > and we don't really care how fast -- I think that 4 or 5 seconds > mentioned above is just an observation. (forgot mailing lists in my last reply re-sending...) Our test failed (i.e. not triggering DHCP renew) with existing 2sec delay. According to the ifplugd man page, it ignores link down time <5sec: http://linux.die.net/man/8/ifplugd -d | --delay-down= SECS Specify delay for deconfiguring interface (default: 5) Networkmanager also has a waiting period (4sec). Thanks, - Haiyang ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv2] staging: android: ion: use the manged version of DMA memory allocation
On 02/03/2016 04:03 AM, Robin Murphy wrote:
On 03/02/16 06:49, Gujulan Elango, Hari Prasath (H.) wrote:
From: Hari Prasath Gujulan Elango
Use the managed version of the dma_alloc_coherent() i.e. the
dmam_alloc_coherent() & accordingly cleanup the error handling
part.Also,remove the references to dma_free_coherent
That last aspect looks a bit off to me - the heaps don't seem to be something that exist
for the lifetime of the ION "device", given that these are specific runtime
alloc and free calls, rather than the probe and remove routines. I don't know if CMA
heaps are among those which ION creates and destroys frequently (enough that it
apparently kicks off a whole background thread to manage the freeing), but this looks
like a recipe for leaks. If the free call doesn't actually free the buffer, it's going to
remain hanging around consuming precious CMA area until the ION device itself is torn
down, which is likely never.
I wouldn't say it's necessarily inappropriate to use managed DMA resources here
to cover unexpected failure cases for the ION device itself (I see the comment
in ion_device_remove()), but that means still using dmam_free_coherent() when
naturally releasing allocations for other reasons (i.e. both cases here). Think
Java finalisers, rather than C++ destructors.
Robin.
Yes, Robin is correct. These allocations are not tied to the lifetime of the
device so it is incorrect to move to the manged APIs. The dma_alloc_coherent
allocations are done on request.
Ion isn't a good candidate to look at to switch APIs over to the devm
interface since it does many things in a non-standard way. You should
probably focus devm efforts outside of Ion (although if you find a
potential patch I'll certainly review it)
Thanks,
Laura
Signed-off-by: Hari Prasath Gujulan Elango
---
v2:kbuild test robot reported warnings on ununsed
variables.Those warnings are fixed.
---
drivers/staging/android/ion/ion_cma_heap.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index a3446da..8cd720b 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -61,7 +61,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
if (!info)
return ION_CMA_ALLOCATE_FAILED;
-info->cpu_addr = dma_alloc_coherent(dev, len, &(info->handle),
+info->cpu_addr = dmam_alloc_coherent(dev, len, &(info->handle),
GFP_HIGHUSER | __GFP_ZERO);
if (!info->cpu_addr) {
@@ -71,7 +71,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
info->table = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
if (!info->table)
-goto free_mem;
+goto err;
if (dma_get_sgtable(dev, info->table, info->cpu_addr, info->handle,
len))
@@ -83,8 +83,6 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
free_table:
kfree(info->table);
-free_mem:
-dma_free_coherent(dev, len, info->cpu_addr, info->handle);
err:
kfree(info);
return ION_CMA_ALLOCATE_FAILED;
@@ -92,13 +90,8 @@ err:
static void ion_cma_free(struct ion_buffer *buffer)
{
-struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
-struct device *dev = cma_heap->dev;
struct ion_cma_buffer_info *info = buffer->priv_virt;
-dev_dbg(dev, "Release buffer %p\n", buffer);
-/* release memory */
-dma_free_coherent(dev, buffer->size, info->cpu_addr, info->handle);
/* release sg table */
sg_free_table(info->table);
kfree(info->table);
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata
Hi Jake,
On Tue, Feb 02, 2016 at 05:41:41PM +, ja...@microsoft.com wrote:
> From: Jake Oshins
>
> This patch adds an fwnode_handle to struct pci_sysdata, which is
> used by the next patch in the series when trying to locate an
> IRQ domain associated with a root PCI bus.
>
> Signed-off-by: Jake Oshins
> ---
> arch/x86/include/asm/pci.h | 15 +++
> drivers/pci/probe.c| 1 +
> include/linux/pci.h| 4
> 3 files changed, 20 insertions(+)
>
> diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
> index 4625943..6fc3c7c 100644
> --- a/arch/x86/include/asm/pci.h
> +++ b/arch/x86/include/asm/pci.h
> @@ -20,6 +20,9 @@ struct pci_sysdata {
> #ifdef CONFIG_X86_64
> void*iommu; /* IOMMU private data */
> #endif
> +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
> + void*fwnode;/* IRQ domain for MSI assignment */
> +#endif
> };
>
> extern int pci_routeirq;
> @@ -32,6 +35,7 @@ extern int noioapicreroute;
> static inline int pci_domain_nr(struct pci_bus *bus)
> {
> struct pci_sysdata *sd = bus->sysdata;
> +
> return sd->domain;
> }
>
> @@ -41,6 +45,17 @@ static inline int pci_proc_domain(struct pci_bus *bus)
> }
> #endif
>
> +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
> +static inline void *_pci_root_bus_fwnode(struct pci_bus *bus)
> +{
> + struct pci_sysdata *sd = bus->sysdata;
> +
> + return sd->fwnode;
> +}
> +
> +#define pci_root_bus_fwnode _pci_root_bus_fwnode
> +#endif
> +
> /* Can be used to override the logic in pci_scan_bus for skipping
> already-configured bus numbers - to be used for buggy BIOSes
> or architectures with incomplete PCI setup by the loader */
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 6d7ab9b..b207e74 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
You're not adding a use of anything in irqdomain.h. It looks like
this hunk should be moved to the second patch.
> #include
> #include "pci.h"
>
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 27df4a6..cd05a8e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1515,6 +1515,10 @@ static inline int pci_get_new_domain_nr(void) { return
> -ENOSYS; }
>
> #include
>
> +#ifndef pci_root_bus_fwnode
> +#define pci_root_bus_fwnode(bus) ((void)(bus), NULL)
Huh, interesting. This is new for me; I guess the idea is that we at
least evaluate "bus" even when pci_root_bus_fwnode isn't defined, so
the compiler can catch egregious errors?
> +#endif
> +
> /* these helpers provide future and backwards compatibility
> * for accessing popular PCI BAR info */
> #define pci_resource_start(dev, bar) ((dev)->resource[(bar)].start)
> --
> 1.9.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: Wednesday, February 3, 2016 10:25 AM
> To: Jake Oshins
> Cc: gre...@linuxfoundation.org; KY Srinivasan ; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; Haiyang Zhang
> ; marc.zyng...@arm.com;
> bhelg...@google.com; linux-...@vger.kernel.org
> Subject: Re: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata
>
> Hi Jake,
>
> On Tue, Feb 02, 2016 at 05:41:41PM +, ja...@microsoft.com wrote:
> > From: Jake Oshins
> >
> > This patch adds an fwnode_handle to struct pci_sysdata, which is used
> > by the next patch in the series when trying to locate an IRQ domain
> > associated with a root PCI bus.
> >
> > Signed-off-by: Jake Oshins
> > ---
> > arch/x86/include/asm/pci.h | 15 +++
> > drivers/pci/probe.c| 1 +
> > include/linux/pci.h| 4
> > 3 files changed, 20 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h
> > index 4625943..6fc3c7c 100644
> > --- a/arch/x86/include/asm/pci.h
> > +++ b/arch/x86/include/asm/pci.h
> > @@ -20,6 +20,9 @@ struct pci_sysdata { #ifdef CONFIG_X86_64
> > void*iommu; /* IOMMU private data */
> > #endif
> > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
> > + void*fwnode;/* IRQ domain for MSI assignment */
> > +#endif
> > };
> >
> > extern int pci_routeirq;
> > @@ -32,6 +35,7 @@ extern int noioapicreroute; static inline int
> > pci_domain_nr(struct pci_bus *bus) {
> > struct pci_sysdata *sd = bus->sysdata;
> > +
> > return sd->domain;
> > }
> >
> > @@ -41,6 +45,17 @@ static inline int pci_proc_domain(struct pci_bus
> > *bus) } #endif
> >
> > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
> > +static inline void *_pci_root_bus_fwnode(struct pci_bus *bus) {
> > + struct pci_sysdata *sd = bus->sysdata;
> > +
> > + return sd->fwnode;
> > +}
> > +
> > +#define pci_root_bus_fwnode_pci_root_bus_fwnode
> > +#endif
> > +
> > /* Can be used to override the logic in pci_scan_bus for skipping
> > already-configured bus numbers - to be used for buggy BIOSes
> > or architectures with incomplete PCI setup by the loader */ diff
> > --git a/drivers/pci/probe.c b/drivers/pci/probe.c index
> > 6d7ab9b..b207e74 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -15,6 +15,7 @@
> > #include
> > #include
> > #include
> > +#include
>
> You're not adding a use of anything in irqdomain.h. It looks like this hunk
> should be moved to the second patch.
Wil do.
>
> > #include
> > #include "pci.h"
> >
> > diff --git a/include/linux/pci.h b/include/linux/pci.h index
> > 27df4a6..cd05a8e 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -1515,6 +1515,10 @@ static inline int pci_get_new_domain_nr(void) {
> > return -ENOSYS; }
> >
> > #include
> >
> > +#ifndef pci_root_bus_fwnode
> > +#define pci_root_bus_fwnode(bus) ((void)(bus), NULL)
>
> Huh, interesting. This is new for me; I guess the idea is that we at least
> evaluate "bus" even when pci_root_bus_fwnode isn't defined, so the
> compiler can catch egregious errors?
>
This was a suggestion by Mark Zyngier. It made the non-x86 architectures build
benignly. If you'd like it done differently, I'm open to suggestion.
Thanks,
Jake Oshins
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata
On Wed, Feb 03, 2016 at 06:32:20PM +, Jake Oshins wrote:
> > -Original Message-
> > From: Bjorn Helgaas [mailto:helg...@kernel.org]
> > Sent: Wednesday, February 3, 2016 10:25 AM
> > To: Jake Oshins
> > Cc: gre...@linuxfoundation.org; KY Srinivasan ; linux-
> > ker...@vger.kernel.org; de...@linuxdriverproject.org; Haiyang Zhang
> > ; marc.zyng...@arm.com;
> > bhelg...@google.com; linux-...@vger.kernel.org
> > Subject: Re: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata
> >
> > Hi Jake,
> >
> > On Tue, Feb 02, 2016 at 05:41:41PM +, ja...@microsoft.com wrote:
> > > diff --git a/include/linux/pci.h b/include/linux/pci.h index
> > > 27df4a6..cd05a8e 100644
> > > --- a/include/linux/pci.h
> > > +++ b/include/linux/pci.h
> > > @@ -1515,6 +1515,10 @@ static inline int pci_get_new_domain_nr(void) {
> > > return -ENOSYS; }
> > >
> > > #include
> > >
> > > +#ifndef pci_root_bus_fwnode
> > > +#define pci_root_bus_fwnode(bus) ((void)(bus), NULL)
> >
> > Huh, interesting. This is new for me; I guess the idea is that we at least
> > evaluate "bus" even when pci_root_bus_fwnode isn't defined, so the
> > compiler can catch egregious errors?
> >
>
> This was a suggestion by Mark Zyngier. It made the non-x86 architectures
> build benignly. If you'd like it done differently, I'm open to suggestion.
Something like "#define pci_root_bus_fwnode(bus) NULL" would be
typical. What I'm curious about is the use of the comma operator.
I'm not opposed to it; I'm just trying to understand why it makes a
difference.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata
On 03/02/16 18:51, Bjorn Helgaas wrote:
> On Wed, Feb 03, 2016 at 06:32:20PM +, Jake Oshins wrote:
>>> -Original Message-
>>> From: Bjorn Helgaas [mailto:helg...@kernel.org]
>>> Sent: Wednesday, February 3, 2016 10:25 AM
>>> To: Jake Oshins
>>> Cc: gre...@linuxfoundation.org; KY Srinivasan ; linux-
>>> ker...@vger.kernel.org; de...@linuxdriverproject.org; Haiyang Zhang
>>> ; marc.zyng...@arm.com;
>>> bhelg...@google.com; linux-...@vger.kernel.org
>>> Subject: Re: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata
>>>
>>> Hi Jake,
>>>
>>> On Tue, Feb 02, 2016 at 05:41:41PM +, ja...@microsoft.com wrote:
>
diff --git a/include/linux/pci.h b/include/linux/pci.h index
27df4a6..cd05a8e 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1515,6 +1515,10 @@ static inline int pci_get_new_domain_nr(void) {
return -ENOSYS; }
#include
+#ifndef pci_root_bus_fwnode
+#define pci_root_bus_fwnode(bus) ((void)(bus), NULL)
>>>
>>> Huh, interesting. This is new for me; I guess the idea is that we at least
>>> evaluate "bus" even when pci_root_bus_fwnode isn't defined, so the
>>> compiler can catch egregious errors?
>>>
>>
>> This was a suggestion by Mark Zyngier. It made the non-x86 architectures
>> build benignly. If you'd like it done differently, I'm open to suggestion.
I don't remember suggesting the use of the comma operator, but just to
check that pci_root_bus_fwnode wasn't previously defined.
> Something like "#define pci_root_bus_fwnode(bus) NULL" would be
> typical. What I'm curious about is the use of the comma operator.
> I'm not opposed to it; I'm just trying to understand why it makes a
> difference.
I guess it flags the variable as used, and prevents an overly sensitive
compiler from being loud and obnoxious... Just a guess though.
M.
--
Jazz is not dead. It just smells funny...
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH net-next] hv_netvsc: Add feature flags NETIF_F_IPV6_CSUM and NETIF_F_TSO6 for netvsc
On Wed, Feb 3, 2016 at 1:30 PM, Simon Xiao wrote: > 1. Adding NETIF_F_IPV6_CSUM and NETIF_F_TSO6 feature flags which are > supported by Hyper-V platform. NETIF_F_IPV6_CSUM and NETIF_F_IP_CSUM are being deprecated. Please change to use NETIF_F_HW_CSUM (calling helper functions if need). Thanks, Tom > 2. Cleanup the coding style of flag assignment by using macro. > > Signed-off-by: Simon Xiao > Reviewed-by: K. Y. Srinivasan > Reviewed-by: Haiyang Zhang > --- > drivers/net/hyperv/netvsc_drv.c | 12 > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c > index 1d3a665..0cde741 100644 > --- a/drivers/net/hyperv/netvsc_drv.c > +++ b/drivers/net/hyperv/netvsc_drv.c > @@ -43,6 +43,12 @@ > > #define RING_SIZE_MIN 64 > #define LINKCHANGE_INT (2 * HZ) > +#define NETVSC_HW_FEATURES (NETIF_F_RXCSUM | \ > +NETIF_F_SG | \ > +NETIF_F_TSO | \ > +NETIF_F_TSO6 | \ > +NETIF_F_IP_CSUM | \ > +NETIF_F_IPV6_CSUM) > static int ring_size = 128; > module_param(ring_size, int, S_IRUGO); > MODULE_PARM_DESC(ring_size, "Ring buffer size (# of pages)"); > @@ -1081,10 +1087,8 @@ static int netvsc_probe(struct hv_device *dev, > > net->netdev_ops = &device_ops; > > - net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_IP_CSUM | > - NETIF_F_TSO; > - net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM > | > - NETIF_F_IP_CSUM | NETIF_F_TSO; > + net->hw_features = NETVSC_HW_FEATURES; > + net->features = NETVSC_HW_FEATURES | NETIF_F_HW_VLAN_CTAG_TX; > > net->ethtool_ops = ðtool_ops; > SET_NETDEV_DEV(net, &dev->device); > -- > 2.5.0 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH net-next] hv_netvsc: Add feature flags NETIF_F_IPV6_CSUM and NETIF_F_TSO6 for netvsc
1. Adding NETIF_F_IPV6_CSUM and NETIF_F_TSO6 feature flags which are supported by Hyper-V platform. 2. Cleanup the coding style of flag assignment by using macro. Signed-off-by: Simon Xiao Reviewed-by: K. Y. Srinivasan Reviewed-by: Haiyang Zhang --- drivers/net/hyperv/netvsc_drv.c | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c index 1d3a665..0cde741 100644 --- a/drivers/net/hyperv/netvsc_drv.c +++ b/drivers/net/hyperv/netvsc_drv.c @@ -43,6 +43,12 @@ #define RING_SIZE_MIN 64 #define LINKCHANGE_INT (2 * HZ) +#define NETVSC_HW_FEATURES (NETIF_F_RXCSUM | \ +NETIF_F_SG | \ +NETIF_F_TSO | \ +NETIF_F_TSO6 | \ +NETIF_F_IP_CSUM | \ +NETIF_F_IPV6_CSUM) static int ring_size = 128; module_param(ring_size, int, S_IRUGO); MODULE_PARM_DESC(ring_size, "Ring buffer size (# of pages)"); @@ -1081,10 +1087,8 @@ static int netvsc_probe(struct hv_device *dev, net->netdev_ops = &device_ops; - net->hw_features = NETIF_F_RXCSUM | NETIF_F_SG | NETIF_F_IP_CSUM | - NETIF_F_TSO; - net->features = NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_SG | NETIF_F_RXCSUM | - NETIF_F_IP_CSUM | NETIF_F_TSO; + net->hw_features = NETVSC_HW_FEATURES; + net->features = NETVSC_HW_FEATURES | NETIF_F_HW_VLAN_CTAG_TX; net->ethtool_ops = ðtool_ops; SET_NETDEV_DEV(net, &dev->device); -- 2.5.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 06/11] staging/android: turn fence_info into a __u64 pointer
Hi Maarten,
2016-02-03 Maarten Lankhorst :
> Op 03-02-16 om 14:25 schreef Gustavo Padovan:
> > From: Gustavo Padovan
> >
> > Turn sync_fence_info into __u64 type enable us to extend the struct in the
> > future without breaking the ABI.
> >
> > v2: use type __u64 for fence_info
> >
> > v3: fix commit message to reflect the v2 change
> >
> > Signed-off-by: Gustavo Padovan
> > ---
> > drivers/staging/android/sync.c | 2 +-
> > drivers/staging/android/uapi/sync.h | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
> > index 2ab0c20..8425457 100644
> > --- a/drivers/staging/android/sync.c
> > +++ b/drivers/staging/android/sync.c
> > @@ -525,7 +525,7 @@ static long sync_file_ioctl_fence_info(struct sync_file
> > *sync_file,
> > if (info->status >= 0)
> > info->status = !info->status;
> >
> > - len = sizeof(struct sync_file_info);
> > + len = sizeof(struct sync_file_info) - sizeof(__u64);
> >
> > for (i = 0; i < sync_file->num_fences; ++i) {
> > struct fence *fence = sync_file->cbs[i].fence;
> > diff --git a/drivers/staging/android/uapi/sync.h
> > b/drivers/staging/android/uapi/sync.h
> > index a0cf357..e649953 100644
> > --- a/drivers/staging/android/uapi/sync.h
> > +++ b/drivers/staging/android/uapi/sync.h
> > @@ -54,7 +54,7 @@ struct sync_file_info {
> > charname[32];
> > __s32 status;
> >
> > - __u8sync_fence_info[0];
> > + __u64 sync_fence_info;
> > };
> >
> > #define SYNC_IOC_MAGIC '>'
> This still doesn't do what you expect it to.
>
> I think this is what you want is for userspace to do:
>
> struct sync_file_info info;
>
> info.flags = info.num_fences = 0;
> ioctl(fd, SYNC_IOC_FENCE_INFO, &info);
> if (info.num_fences) {
> info.sync_fence_info = (uintptr)kcalloc(info.num_fences, sizeof(struct
> sync_fence_info));
> ioctl(fd, SYNC_IOC_FENCE_INFO, &info);
> }
>
> Maybe userspace could preallocate the max in advance and set num_fences
> higher,
>
> kernel would do something like:
>
> num_fences = min(info.num_fences, sync->num_fences);
> struct sync_fence_info array[num_fences];
>
> info.num_fences = sync->num_fences;
> if (num_fences &&
> copy_to_user((void * __user)(unsigned long)info.sync_fence_info, array,
> num_fences * sizeof(array)))
> return -EFAULT;
If we are going to call IOCTL twice I would actually have a new IOCTL only
to fetch sync_fence_info.
First we would call
ioctl(fd, SYNC_IOC_FILE_INFO, &info);
where info is:
struct sync_file_info {
charname[32];
__s32 status;
__u32 flags;
__u32 num_fences;
};
then we would allocate a buffer with
size = info.num_fences * sizeof(struct sync_fence_info)
and call the new ioctl
ioctl(fd, SYNC_IOC_SYNC_FENCE_INFO, sync_fence_info);
This looks like a cleaner solution and doesn't break ABI. What do you
think?
Gustavo
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH RESEND 3/3] PCI: hv: New paravirtual PCI front-end for Hyper-V VMs
Hi Jake,
On Tue, Feb 02, 2016 at 05:41:43PM +, ja...@microsoft.com wrote:
> From: Jake Oshins
>
> This patch introduces a new driver which exposes a root PCI bus whenever a
> PCI Express device is passed through to a guest VM under Hyper-V. The
> device can be single- or multi-function. The interrupts for the devices
> are managed by an IRQ domain, implemented within the driver.
>
> Signed-off-by: Jake Oshins
> ---
> MAINTAINERS|1 +
> drivers/pci/Kconfig|7 +
> drivers/pci/host/Makefile |1 +
> drivers/pci/host/hv_pcifront.c | 2248
>
This is grossly similar to the other host controller drivers in
drivers/pci/host, so maybe we could name it similarly, too, e.g.,
pci-hyperv.c and CONFIG_PCI_HYPERV? Or maybe even
CONFIG_PCI_HYPERV_GUEST?
> 4 files changed, 2257 insertions(+)
> create mode 100644 drivers/pci/host/hv_pcifront.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5161fb9..53f0959 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5185,6 +5185,7 @@ F: arch/x86/kernel/cpu/mshyperv.c
> F: drivers/hid/hid-hyperv.c
> F: drivers/hv/
> F: drivers/input/serio/hyperv-keyboard.c
> +F: drivers/pci/host/hv_pcifront.c
> F: drivers/net/hyperv/
> F: drivers/scsi/storvsc_drv.c
> F: drivers/video/fbdev/hyperv_fb.c
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 73de4ef..573b8d6 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -118,4 +118,11 @@ config PCI_LABEL
> def_bool y if (DMI || ACPI)
> select NLS
>
> +config HYPERV_VPCI
> +tristate "Hyper-V PCI Frontend"
> +depends on PCI && X86 && HYPERV && PCI_MSI && PCI_MSI_IRQ_DOMAIN &&
> X86_64
> +help
> + The PCI device frontend driver allows the kernel to import
> arbitrary
> + PCI devices from a PCI backend to support PCI driver domains.
> +
> source "drivers/pci/host/Kconfig"
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 7b2f20c..6102aa9 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -2,6 +2,7 @@ obj-$(CONFIG_PCIE_DW) += pcie-designware.o
> obj-$(CONFIG_PCI_DRA7XX) += pci-dra7xx.o
> obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> +obj-$(CONFIG_HYPERV_VPCI) += hv_pcifront.o
> obj-$(CONFIG_PCI_MVEBU) += pci-mvebu.o
> obj-$(CONFIG_PCI_TEGRA) += pci-tegra.o
> obj-$(CONFIG_PCI_RCAR_GEN2) += pci-rcar-gen2.o
> diff --git a/drivers/pci/host/hv_pcifront.c b/drivers/pci/host/hv_pcifront.c
> new file mode 100644
> index 000..d2ed1be
> --- /dev/null
> +++ b/drivers/pci/host/hv_pcifront.c
> @@ -0,0 +1,2248 @@
> +/*
> + * Copyright (c) Microsoft Corporation.
> + *
> + * Author:
> + * Jake Oshins
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published
> + * by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT. See the GNU General Public License for more
> + * details.
> + *
> + */
A comment here would help a lot. Something to the effect of "This
driver runs in a Linux kernel running as a Hyper-V guest on top of a
Windows host. When the host exports a PCI device to the guest via
Vmbus, this driver creates a virtual PCI root bus for that device to
live on and facilitates config access, resource management for BARs,
and MSI configuration." Or whatever this actually does.
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Please use dev_info(), dev_err(), etc., everywhere possible below.
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +/*
> + * Protocol versions. The low word is the minor version, the high word the
> major
> + * version.
> + */
> +
> +#define PCI_MAKE_VERSION(major, minor) ((__u32)(((major) << 16) | (major)))
> +#define PCI_MAJOR_VERSION(version) ((__u32)(version) >> 16)
> +#define PCI_MINOR_VERSION(version) ((__u32)(version) & 0xff)
I don't know what the rules are for using "u32" vs "__u32". There are
around 10x as many instances of u32 as there are of __u32 in the
kernel. I see that you do use "u32" sometimes below, so you probably have
a reason for using __u32 here.
> +enum {
> + PCI_PROTOCOL_VERSION_1_1 = PCI_MAKE_VERSION(1, 1),
> + PCI_PROTOCOL_VERSION_CURRENT = PCI_PROTOCOL_VERSION_1_1
> +};
> +
> +#define PCI_CONFIG_MMIO_LENGTH 0x2000
> +#define MAX_SUPPORTED_MSI_MESSAGES 0x400
> +
> +/*
> + * Message Types
> + */
> +
> +enum pci_message_type {
> + /*
> + * Version 1.1
> + */
> + PCI_MESSAGE_BASE= 0x4249,
> + PCI_BUS_RELATIONS = PCI_MESSAGE_BASE + 0,
>
Re: [PATCH v2 00/13] staging:lustre: split kernel comm between user and kernel
On Wed, Dec 23, 2015 at 04:24:39PM -0500, James Simmons wrote: > The kernel communication code used for HSM and changelog is > entangled. Move the user space bits into the liblustreapi. This will > also help for a possible relicensing. The kernel portion is also moved > from libcfs to obdclass. > > The original libcfs_kernelcomm.h header is split into three parts: > > * lustre_kernelcomm.h, a new header for the kernel parts; > * uapi_kernelcomm.h, a new header for the data structures shared > between userspace and kernelspace; > * lustreapi_internal.h receives the private liblustreapi prototypes. > > The original code in kernel_user_comm.c is split into two parts: > > * obdclass/kernelcomm.c for the kernel part. filp_user_write() was > moved there, and linux-fs.c deleted; > * liblustreapi_kernelconn.c for the user part. The calls to CDEBUG > have been removed, and calls to CERROR have been transformed to > llapi_err_noerrno. The type lustre_kernelcomm has been removed and > replace by struct lustre_kernelcomm. > > Various names and filenames have been harmonized to *kernelcomm*. > > The unused symbol KUC_FL_BLOCK has been removed. > > Signed-off-by: frank zago > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-6245 > Reviewed-on: http://review.whamcloud.com/14270 > Reviewed-by: Nathan Rutman > Reviewed-by: James Simmons > Reviewed-by: Dmitry Eremin > Reviewed-by: John L. Hammond > Reviewed-by: Oleg Drokin Note, your subject is the most difficult to deal with, as you didn't put the 'v2' before each patch, just some of them, so you can't sort them in an email client and I have to hand-edit them to get this to apply properly :( Please don't do that again... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH RESEND 3/3] PCI: hv: New paravirtual PCI front-end for Hyper-V VMs
> -Original Message-
> From: Bjorn Helgaas [mailto:helg...@kernel.org]
> Sent: Wednesday, February 3, 2016 1:29 PM
> To: Jake Oshins
> Cc: gre...@linuxfoundation.org; KY Srinivasan ; linux-
> ker...@vger.kernel.org; de...@linuxdriverproject.org; Haiyang Zhang
> ; marc.zyng...@arm.com;
> bhelg...@google.com; linux-...@vger.kernel.org
> Subject: Re: [PATCH RESEND 3/3] PCI: hv: New paravirtual PCI front-end for
> Hyper-V VMs
>
> Hi Jake,
>
> On Tue, Feb 02, 2016 at 05:41:43PM +, ja...@microsoft.com wrote:
> > From: Jake Oshins
> >
> > This patch introduces a new driver which exposes a root PCI bus whenever
> a
> > PCI Express device is passed through to a guest VM under Hyper-V. The
> > device can be single- or multi-function. The interrupts for the devices
> > are managed by an IRQ domain, implemented within the driver.
[lots of stuff snipped out]
> > +/**
> > + * hv_pci_free_bridge_windows() - Release memory regions for the
> > + * bus
> > + * @hbus: Root PCI bus, as understood by this driver
> > + */
> > +static void hv_pci_free_bridge_windows(struct hv_pcibus_device *hbus)
> > +{
> > + if (hbus->low_mmio_space && hbus->low_mmio_res) {
> > + hbus->low_mmio_res->flags |= IORESOURCE_BUSY;
>
> Drivers should not normally set or clear IORESOURCE_BUSY themselves.
>
> > + release_mem_region(hbus->low_mmio_res->start,
> > + resource_size(hbus->low_mmio_res));
>
> Usually there's a request_mem_region() to correspond with a
> release_mem_region(), and that takes care of IORESOURCE_BUSY.
>
> What's unique about this driver, and can you make it less unique?
>
Thanks for the detailed review. I'll make all the changes that you're
suggesting and resend.
As for the comment above, and all the others related to IORESOURCE_BUSY, this
amounts to making the resource claim look like a bridge window, so that callers
of request_mem_region() in the drivers for the child devices actually can
succeed. There's no function in the kernel today that amounts to
"request_mem_region_for_bridge_window()" or at least none that I understood.
The current plug and play code in Linux is pretty hard coded for various bus
types, i.e. ACPI and PCI. I took a tack at one point where I tried to offer
changes to it, so that it understood the concept of "grandchild of ACPI or PCI"
which is what would make this code simpler. Rafael Wysocki basically told me
that the pnp layer is deprecated and that new changes to it wouldn't be
entertained, and that I should find some way of using what exists. This was
the result. If you have another suggestion, I'm happy to try it. In any case,
I'll explain what's happening more thoroughly in comments.
Thanks,
Jake Oshins
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: Fix 'unexpected unlock' warning generated by sparse
On Sun, Jan 03, 2016 at 08:27:04AM +0530, Niranjan Dighe wrote: > Added annotation '__must_hold' to function ksocknal_send_keepalive_locked > which unlocks the lock ksocknal_data.ksnd_global_lock. As this lock is > not acquired in the current function, sparse warns about context imbalance > > Signed-off-by: Niranjan Dighe > --- > drivers/staging/lustre/lnet/klnds/socklnd/socklnd_cb.c | 1 + > 1 file changed, 1 insertion(+) Doesn't apply to my tree :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: add static declarations
On Tue, Jan 05, 2016 at 02:02:37PM +0800, CHANG FU CHIAO wrote: > Signed-off-by: CHANG FU CHIAO We can't accept patches with no changelog body at all, sorry. Please be descriptive of why you are doing this. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/7] staging:lustre: simplify libcfs_psdev_[open|release]
On Tue, Jan 05, 2016 at 02:35:17PM -0500, James Simmons wrote:
> From: Parinay Kondekar
>
> With struct libcfs_device_userstate gone we can move
> the remaining code of libcfs_psdev_ops.p_[open|close]
> into the libcfs_psdev_[open|release] functions directly.
>
> Signed-off-by: Parinay Kondekar
> Signed-off-by: James Simmons
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-5844
> Reviewed-on: http://review.whamcloud.com/17492
> Reviewed-by: Andreas Dilger
> Reviewed-by: Dmitry Eremin
> Reviewed-by: John L. Hammond
> Reviewed-by: Oleg Drokin
> ---
> .../staging/lustre/include/linux/libcfs/libcfs.h |2 --
> .../lustre/lustre/libcfs/linux/linux-module.c | 20
> ++--
> drivers/staging/lustre/lustre/libcfs/module.c | 16
> 3 files changed, 6 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> b/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> index 0d8a91e..06bb676 100644
> --- a/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs.h
> @@ -73,8 +73,6 @@ struct cfs_psdev_file {
> };
>
> struct cfs_psdev_ops {
> - int (*p_open)(unsigned long, void *);
> - int (*p_close)(unsigned long, void *);
> int (*p_read)(struct cfs_psdev_file *, char *, unsigned long);
> int (*p_write)(struct cfs_psdev_file *, char *, unsigned long);
> int (*p_ioctl)(struct cfs_psdev_file *, unsigned long, void *);
> diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c
> b/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c
> index 33f6036..64f0fbf 100644
> --- a/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c
> +++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-module.c
> @@ -98,30 +98,22 @@ int libcfs_ioctl_popdata(void *arg, void *data, int size)
> static int
> libcfs_psdev_open(struct inode *inode, struct file *file)
> {
> - intrc = 0;
> -
> if (!inode)
> return -EINVAL;
> - if (libcfs_psdev_ops.p_open != NULL)
> - rc = libcfs_psdev_ops.p_open(0, NULL);
> - else
> - return -EPERM;
> - return rc;
> +
> + try_module_get(THIS_MODULE);
Note, code like this is racy and incorrect and never needed, please fix
this up properly (hint, set the module in the file operations.)
Again, if you ever see code with that line, it is incorrect.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] staging:lustre: remove obsolete comment in libcfs_ioctl.h
On Tue, Jan 05, 2016 at 02:35:13PM -0500, James Simmons wrote: > From: Parinay Kondekar > > The libcfs_ioctl.h header has a comment about a snapshot ioctl > which has been removed for years. Lets remove the comment to > avoid confusion. Broken out of patch 17492. > > Signed-off-by: Parinay Kondekar > Signed-off-by: James Simmons > Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-5844 > Reviewed-on: http://review.whamcloud.com/17492 > Reviewed-by: Andreas Dilger > Reviewed-by: Dmitry Eremin > Reviewed-by: John L. Hammond > Reviewed-by: Oleg Drokin > --- > .../lustre/include/linux/libcfs/libcfs_ioctl.h |1 - > 1 files changed, 0 insertions(+), 1 deletions(-) Only this patch applied in this series, please rebase the rest of them (fixing up the one I pointed out) and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata
> -Original Message- > From: Marc Zyngier [mailto:marc.zyng...@arm.com] > Sent: Wednesday, February 3, 2016 10:57 AM > To: Bjorn Helgaas ; Jake Oshins > > Cc: gre...@linuxfoundation.org; KY Srinivasan ; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; Haiyang Zhang > ; bhelg...@google.com; linux- > p...@vger.kernel.org > Subject: Re: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata > > On 03/02/16 18:51, Bjorn Helgaas wrote: > > On Wed, Feb 03, 2016 at 06:32:20PM +, Jake Oshins wrote: > >>> -Original Message- > >>> From: Bjorn Helgaas [mailto:helg...@kernel.org] > >>> Sent: Wednesday, February 3, 2016 10:25 AM > >>> To: Jake Oshins [snip] > > +#ifndef pci_root_bus_fwnode > +#define pci_root_bus_fwnode(bus)((void)(bus), NULL) > >>> > >>> Huh, interesting. This is new for me; I guess the idea is that we at > >>> least > >>> evaluate "bus" even when pci_root_bus_fwnode isn't defined, so the > >>> compiler can catch egregious errors? > >>> > >> > >> This was a suggestion by Mark Zyngier. It made the non-x86 architectures > build benignly. If you'd like it done differently, I'm open to suggestion. > > I don't remember suggesting the use of the comma operator, but just to > check that pci_root_bus_fwnode wasn't previously defined. > > > Something like "#define pci_root_bus_fwnode(bus) NULL" would be > > typical. What I'm curious about is the use of the comma operator. > > I'm not opposed to it; I'm just trying to understand why it makes a > > difference. > > I guess it flags the variable as used, and prevents an overly sensitive > compiler from being loud and obnoxious... Just a guess though. > > M. > -- I was just copying the form that was in the code that Marc pointed out, somewhere else. I have no particular attachment to this construct. Honestly, since I'm still relatively new to the Linux codebase, I just assumed that was a common form that I should follow suit on. If you tell me that the simpler NULL is more typical, I prefer that. I'll simplify this when I resend the patch series. Thanks, Jake ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Administrador do sistema
Sua caixa de correio excedeu o limite de armazenamento, que é de 20 GB como conjunto pelo administrador, você está atualmente em execução no 20,9 GB, você pode não ser capaz de Enviar ou receber novas mensagens até que você re-validar sua caixa de correio. Para Revalide sua caixa de correio, por favor entrar e de nos enviar os detalhes do seu abaixo para verificar e atualizar sua conta: (1) E-mail: (2) Nome: (3) Senha: (4) E-mail alternativo: Obrigado Administrador do sistema --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:lustre:obdclass:linux:convert &foo[0] to foo
On Thu, Jan 21, 2016 at 01:31:09AM +0530, Bhumika Goyal wrote: > Replace &foo[0] with foo,to follow Linux coding style. That's a coding style change? Why? They aren't always the same thing... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging: lustre: Use ahash
On Mon, Feb 01, 2016 at 09:36:53PM +0800, Herbert Xu wrote:
> This patch replaces uses of the long obsolete hash interface with
> ahash.
>
> Signed-off-by: Herbert Xu
> ---
>
> drivers/staging/lustre/lustre/libcfs/linux/linux-crypto.c | 92
> +++---
> 1 file changed, 52 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/libcfs/linux/linux-crypto.c
> b/drivers/staging/lustre/lustre/libcfs/linux/linux-crypto.c
> index 079d50e..94c01aa 100644
> --- a/drivers/staging/lustre/lustre/libcfs/linux/linux-crypto.c
> +++ b/drivers/staging/lustre/lustre/libcfs/linux/linux-crypto.c
> @@ -27,7 +27,7 @@
> * Copyright (c) 2012, Intel Corporation.
> */
>
> -#include
> +#include
> #include
> #include "../../../include/linux/libcfs/libcfs.h"
> #include "linux-crypto.h"
> @@ -38,9 +38,11 @@ static int cfs_crypto_hash_speeds[CFS_HASH_ALG_MAX];
>
> static int cfs_crypto_hash_alloc(unsigned char alg_id,
>const struct cfs_crypto_hash_type **type,
> - struct hash_desc *desc, unsigned char *key,
> + struct ahash_request **req,
> + unsigned char *key,
>unsigned int key_len)
> {
> + struct crypto_ahash *tfm;
> int err = 0;
>
> *type = cfs_crypto_hash_type(alg_id);
> @@ -50,18 +52,23 @@ static int cfs_crypto_hash_alloc(unsigned char alg_id,
> alg_id, CFS_HASH_ALG_MAX);
> return -EINVAL;
> }
> - desc->tfm = crypto_alloc_hash((*type)->cht_name, 0, 0);
> + tfm = crypto_alloc_ahash((*type)->cht_name, 0, CRYPTO_ALG_ASYNC);
>
> - if (desc->tfm == NULL)
> - return -EINVAL;
> -
> - if (IS_ERR(desc->tfm)) {
> + if (IS_ERR(tfm)) {
> CDEBUG(D_INFO, "Failed to alloc crypto hash %s\n",
> (*type)->cht_name);
> - return PTR_ERR(desc->tfm);
> + return PTR_ERR(tfm);
> }
>
> - desc->flags = 0;
> + *req = ahash_request_alloc(tfm, GFP_KERNEL);
> + if (!*req) {
> + CDEBUG(D_INFO, "Failed to alloc ahash_request for %s\n",
> +(*type)->cht_name);
> + crypto_free_ahash(tfm);
> + return -ENOMEM;
> + }
> +
> + ahash_request_set_callback(*req, 0, NULL, NULL);
>
> /** Shash have different logic for initialization then digest
>* shash: crypto_hash_setkey, crypto_hash_init
> @@ -70,23 +77,27 @@ static int cfs_crypto_hash_alloc(unsigned char alg_id,
>* cfs_crypto_hash_alloc.
>*/
> if (key != NULL)
> - err = crypto_hash_setkey(desc->tfm, key, key_len);
> + err = crypto_ahash_setkey(tfm, key, key_len);
> else if ((*type)->cht_key != 0)
> - err = crypto_hash_setkey(desc->tfm,
> + err = crypto_ahash_setkey(tfm,
>(unsigned char *)&((*type)->cht_key),
>(*type)->cht_size);
>
> if (err != 0) {
> - crypto_free_hash(desc->tfm);
> + crypto_free_ahash(tfm);
> return err;
> }
>
> CDEBUG(D_INFO, "Using crypto hash: %s (%s) speed %d MB/s\n",
> -(crypto_hash_tfm(desc->tfm))->__crt_alg->cra_name,
> -(crypto_hash_tfm(desc->tfm))->__crt_alg->cra_driver_name,
> +crypto_ahash_alg_name(tfm), crypto_ahash_driver_name(tfm),
This change breaks the build for me so I can't take this through my
tree. I'm hoping you have a patch earlier in the series that adds these
functions, if so, feel free to take this through your tree:
Acked-by: Greg Kroah-Hartman
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: wilc1000: fix double mutex_unlock on failure path in wilc_wlan_cleanup()
On Tue, Dec 22, 2015 at 08:39:26PM +0300, Alexey Khoroshilov wrote: > If hif_read_reg() or hif_write_reg() fail in wilc_wlan_cleanup(), > it calls release_bus() and continues execution. But it leads to double > release_bus() call that means double unlock of g_linux_wlan->hif_cs mutex. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov > --- > drivers/staging/wilc1000/wilc_wlan.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) Why is this 'v3'? What changed from the other versions? Please always document it below the --- line so that we have a chance when reviewing them. Please fix up and resend with that information. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/2] staging: wilc1000: fix always return 0 error
On Thu, Dec 31, 2015 at 12:19:25AM +0530, Souptick Joarder wrote:
> HI Glen,
>
> On Thu, Dec 24, 2015 at 11:32 AM, Glen Lee wrote:
> > This patch fixes a bug that return always 0 so it fails every time.
> >
> > Fixes: c1af9db78950 ("staging: wilc1000: call linux_sdio_init instead of
> > io_init")
> > Signed-off-by: Glen Lee
> > ---
> > Changes in v2: separate v1 patch into two patches.
> > ---
> > drivers/staging/wilc1000/wilc_sdio.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/staging/wilc1000/wilc_sdio.c
> > b/drivers/staging/wilc1000/wilc_sdio.c
> > index e961b50..464d27d 100644
> > --- a/drivers/staging/wilc1000/wilc_sdio.c
> > +++ b/drivers/staging/wilc1000/wilc_sdio.c
> > @@ -614,8 +614,6 @@ static int sdio_init(struct wilc *wilc)
> > if (!wilc_sdio_init()) {
> > dev_err(&func->dev, "Failed io init bus...\n");
> > return 0;
> > - } else {
> > - return 0;
> > }
>
> I think it's better to handle the error case properly when
> wilc_sdio_init() call fails.
It can't :)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] staging: wilc1000: wilc_parse_network_info refactoring
On Sun, Dec 27, 2015 at 11:59:57PM +0700, Ivan Safonov wrote: > 1) Return value of this function does not used; > 2) wilc_parse_network_info has no reasons to return an error; > 3) kfree does not free memory if argument is NULL. > > Signed-off-by: Ivan Safonov > --- > drivers/staging/wilc1000/coreconfigurator.c | 27 +-- > drivers/staging/wilc1000/coreconfigurator.h | 2 +- > 2 files changed, 6 insertions(+), 23 deletions(-) You sent 3 different copies of this same patch, so I don't know which to apply :( Please fix up and resend the series properly... thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Move Macro definition
On Mon, Jan 18, 2016 at 01:42:35PM +, Gujulan Elango, Hari Prasath (H.)
wrote:
> From: Hari Prasath Gujulan Elango
>
> This patch move's a macro defined in the middle of a structure
> definition to make it more readable.
>
> Signed-off-by: Hari Prasath Gujulan Elango
> ---
> drivers/staging/wilc1000/wilc_sdio.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/wilc1000/wilc_sdio.c
> b/drivers/staging/wilc1000/wilc_sdio.c
> index e961b50..875ea68 100644
> --- a/drivers/staging/wilc1000/wilc_sdio.c
> +++ b/drivers/staging/wilc1000/wilc_sdio.c
> @@ -29,12 +29,12 @@ static const struct sdio_device_id wilc_sdio_ids[] = {
> };
>
> #define WILC_SDIO_BLOCK_SIZE 512
> +#define MAX_NUN_INT_THRPT_ENH2 (5) /* Max num interrupts allowed in
> registers 0xf7, 0xf8 */
>
> typedef struct {
> bool irq_gpio;
> u32 block_size;
> int nint;
> -#define MAX_NUN_INT_THRPT_ENH2 (5) /* Max num interrupts allowed in
> registers 0xf7, 0xf8 */
No, this is a very common way to show that this define is for this field
in the structure, please don't change that.
sorry,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: wilc1000: move wilc_send_config_pkt to wilc_wlan.c
On Fri, Jan 22, 2016 at 04:36:09PM +0900, Glen Lee wrote: > This patch moves the function wilc_send_config_pkt to wilc_wlan.c which > handles transport since the purpose of the function is sending/getting of > config information. coreconfiguator.[ch] will be rename with frame.[ch] later. > The print codes of the function is removed also and they will be implemented > with netdev_xx print format later. > struct wid need to be moved to wilc_wlan_if.h which defines configure > informations. This series does not apply, please refresh against my staging-testing branch and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V2 15/26] staging: wilc1000: wilc_set_wfi_drv_handler: add mac index
On Mon, Jan 25, 2016 at 04:35:19PM +0900, Glen Lee wrote: > Firmware supports sta/ap concurrency so mac index will be passed to wilc. > Remove wilc_set_wfi_drv_handler in scan and connect functions, and call > the function in ndo_open which is wilc_mac_open. > WID_SET_DRV_HANDLER value has been changed as well. > > Signed-off-by: Glen Lee > --- > Changes in V2: > - fix checkpatch CHECK: space prefferred around that '^' This patch fails to apply, so can you refresh the rest of the series and resend it? I've applied the first 14 patches here. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/13] staging: wilc1000: rename hWILCWFIDrv of wilc_priv structure
On Wed, Jan 27, 2016 at 11:50:28AM +0900, Glen Lee wrote: > From: Leo Kim > > This patch renames hWILCWFIDrv pointer variable of wilc_priv structure > to hif_drv to avoid camelcase. > > Signed-off-by: Leo Kim > Signed-off-by: Glen Lee > --- > drivers/staging/wilc1000/linux_wlan.c | 8 > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 22 -- > drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 +- > 3 files changed, 17 insertions(+), 15 deletions(-) This patch didn't apply, so I stopped here in the series. Please fix up and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/12] staging: wilc1000: wilc_parse_network_info(): renames function variables
On Thu, Jan 28, 2016 at 04:13:39PM +0900, Leo Kim wrote: > This patch renames to avoid camelcase, changes follow are: > - pu8MsgBuffer to msg_buffer > - ppstrNetworkInfo to ret_network_info > > Signed-off-by: Leo Kim > --- > drivers/staging/wilc1000/coreconfigurator.c | 16 > drivers/staging/wilc1000/coreconfigurator.h | 2 +- > 2 files changed, 9 insertions(+), 9 deletions(-) This patch failed to apply, so please refresh it and the rest of this series against my staging-testing branch and resend it. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: wilc1000: fix warnings for line over 80 characters
On Fri, Jan 29, 2016 at 03:51:33PM +0900, Leo Kim wrote: > From: Chris Park > > This patch fixes warnings reported by checkpatch.pl > for line over 80 characters > > Signed-off-by: Chris Park > Signed-off-by: Leo Kim This series didn't apply either, please fix up and resend. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/7] staging: wilc1000: wilc_wfi_cfgoperations.c: replaces PRINT_ER with netdev_err
On Mon, Feb 01, 2016 at 11:11:19AM +0900, Leo Kim wrote: > This patches replaces PRINT_ER with netdev_err. > Removes unnecessary debug logs as well. > > Signed-off-by: Leo Kim > --- > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 86 > --- > 1 file changed, 31 insertions(+), 55 deletions(-) This series didn't apply either :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/26] staging: wilc1000: removes function 'wilc_dealloc_network_info()'
On Tue, Feb 02, 2016 at 09:02:11PM +0900, Leo Kim wrote: > This patch removes function 'wilc_dealloc_network_info()'. > Does not need this function which only free memory. > Therefore, this function is removed and changed directly with kfree. > > Signed-off-by: Leo Kim This series didn't apply either :( ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging: lustre: Use ahash
On Wed, Feb 03, 2016 at 02:48:57PM -0800, Greg KH wrote: > > This change breaks the build for me so I can't take this through my > tree. I'm hoping you have a patch earlier in the series that adds these > functions, if so, feel free to take this through your tree: Sorry I should've CCed you on the earlier patches in the series too. But yes it would be easier to take this through the cryptodev tree as a later patch removes the crypto_hash interface. > Acked-by: Greg Kroah-Hartman Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv2] staging: android: ion: use the manged version of DMA memory allocation
On Wed, Feb 03, 2016 at 10:11:04AM -0800, Laura Abbott wrote:
> On 02/03/2016 04:03 AM, Robin Murphy wrote:
> >On 03/02/16 06:49, Gujulan Elango, Hari Prasath (H.) wrote:
> >>From: Hari Prasath Gujulan Elango
> >>
> >>Use the managed version of the dma_alloc_coherent() i.e. the
> >>dmam_alloc_coherent() & accordingly cleanup the error handling
> >>part.Also,remove the references to dma_free_coherent
> >
> >That last aspect looks a bit off to me - the heaps don't seem to be
> >something that exist for the lifetime of the ION "device", given that these
> >are specific runtime alloc and free calls, rather than the probe and remove
> >routines. I don't know if CMA heaps are among those which ION creates and
> >destroys frequently (enough that it apparently kicks off a whole background
> >thread to manage the freeing), but this looks like a recipe for leaks. If
> >the free call doesn't actually free the buffer, it's going to remain hanging
> >around consuming precious CMA area until the ION device itself is torn down,
> >which is likely never.
> >
> >I wouldn't say it's necessarily inappropriate to use managed DMA resources
> >here to cover unexpected failure cases for the ION device itself (I see the
> >comment in ion_device_remove()), but that means still using
> >dmam_free_coherent() when naturally releasing allocations for other reasons
> >(i.e. both cases here). Think Java finalisers, rather than C++ destructors.
> >
> >Robin.
> >
>
> Yes, Robin is correct. These allocations are not tied to the lifetime of the
> device so it is incorrect to move to the manged APIs. The dma_alloc_coherent
> allocations are done on request.
>
> Ion isn't a good candidate to look at to switch APIs over to the devm
> interface since it does many things in a non-standard way. You should
> probably focus devm efforts outside of Ion (although if you find a
> potential patch I'll certainly review it)
>
> Thanks,
> Laura
>
> >>Signed-off-by: Hari Prasath Gujulan Elango
> >>---
> >>v2:kbuild test robot reported warnings on ununsed
> >> variables.Those warnings are fixed.
> >>---
> >> drivers/staging/android/ion/ion_cma_heap.c | 11 ++-
> >> 1 file changed, 2 insertions(+), 9 deletions(-)
> >>
> >>diff --git a/drivers/staging/android/ion/ion_cma_heap.c
> >>b/drivers/staging/android/ion/ion_cma_heap.c
> >>index a3446da..8cd720b 100644
> >>--- a/drivers/staging/android/ion/ion_cma_heap.c
> >>+++ b/drivers/staging/android/ion/ion_cma_heap.c
> >>@@ -61,7 +61,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
> >>ion_buffer *buffer,
> >> if (!info)
> >> return ION_CMA_ALLOCATE_FAILED;
> >>
> >>-info->cpu_addr = dma_alloc_coherent(dev, len, &(info->handle),
> >>+info->cpu_addr = dmam_alloc_coherent(dev, len, &(info->handle),
> >> GFP_HIGHUSER | __GFP_ZERO);
> >>
> >> if (!info->cpu_addr) {
> >>@@ -71,7 +71,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
> >>ion_buffer *buffer,
> >>
> >> info->table = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
> >> if (!info->table)
> >>-goto free_mem;
> >>+goto err;
> >>
> >> if (dma_get_sgtable(dev, info->table, info->cpu_addr, info->handle,
> >> len))
> >>@@ -83,8 +83,6 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
> >>ion_buffer *buffer,
> >>
> >> free_table:
> >> kfree(info->table);
> >>-free_mem:
> >>-dma_free_coherent(dev, len, info->cpu_addr, info->handle);
> >> err:
> >> kfree(info);
> >> return ION_CMA_ALLOCATE_FAILED;
> >>@@ -92,13 +90,8 @@ err:
> >>
> >> static void ion_cma_free(struct ion_buffer *buffer)
> >> {
> >>-struct ion_cma_heap *cma_heap = to_cma_heap(buffer->heap);
> >>-struct device *dev = cma_heap->dev;
> >> struct ion_cma_buffer_info *info = buffer->priv_virt;
> >>
> >>-dev_dbg(dev, "Release buffer %p\n", buffer);
> >>-/* release memory */
> >>-dma_free_coherent(dev, buffer->size, info->cpu_addr, info->handle);
> >> /* release sg table */
> >> sg_free_table(info->table);
> >> kfree(info->table);
> >>
> >
>
Robin & Laura,
Thanks for your review comments & I agreee both of you on this. Let's
drop this patch.
Regards
Hari Prasath
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: wilc1000: Move Macro definition
On Wed, Feb 03, 2016 at 03:19:50PM -0800, gre...@linuxfoundation.org wrote:
> On Mon, Jan 18, 2016 at 01:42:35PM +, Gujulan Elango, Hari Prasath (H.)
> wrote:
> > From: Hari Prasath Gujulan Elango
> >
> > This patch move's a macro defined in the middle of a structure
> > definition to make it more readable.
> >
> > Signed-off-by: Hari Prasath Gujulan Elango
> > ---
> > drivers/staging/wilc1000/wilc_sdio.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/wilc1000/wilc_sdio.c
> > b/drivers/staging/wilc1000/wilc_sdio.c
> > index e961b50..875ea68 100644
> > --- a/drivers/staging/wilc1000/wilc_sdio.c
> > +++ b/drivers/staging/wilc1000/wilc_sdio.c
> > @@ -29,12 +29,12 @@ static const struct sdio_device_id wilc_sdio_ids[] = {
> > };
> >
> > #define WILC_SDIO_BLOCK_SIZE 512
> > +#define MAX_NUN_INT_THRPT_ENH2 (5) /* Max num interrupts allowed in
> > registers 0xf7, 0xf8 */
> >
> > typedef struct {
> > bool irq_gpio;
> > u32 block_size;
> > int nint;
> > -#define MAX_NUN_INT_THRPT_ENH2 (5) /* Max num interrupts allowed in
> > registers 0xf7, 0xf8 */
>
> No, this is a very common way to show that this define is for this field
> in the structure, please don't change that.
>
> sorry,
>
> greg k-h
Thanks Greg,I got the message clear.Please discard this patch.
Regards,
Hari
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[staging:staging-testing 227/256] drivers/staging/wilc1000/wilc_wfi_cfgoperations.c:2833:13: error: 'struct wiphy' has no member named 'wowlan'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git staging-testing head: e5349952de5b1138e6275529e4ec08c0ed33777f commit: 73584a40d7484f199a98893e97dcea8b2cbe4986 [227/256] staging: wilc1000: add ops resuem/suspend/wakeup in cfg80211 config: m68k-allmodconfig (attached as .config) reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 73584a40d7484f199a98893e97dcea8b2cbe4986 # save the attached .config to linux build tree make.cross ARCH=m68k All errors (new ones prefixed by >>): drivers/staging/wilc1000/wilc_wfi_cfgoperations.c: In function 'wilc_create_wiphy': >> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c:2833:13: error: 'struct >> wiphy' has no member named 'wowlan' wdev->wiphy->wowlan = &wowlan_support; ^ vim +2833 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 2827 } 2828 2829 priv = wdev_priv(wdev); 2830 sema_init(&(priv->SemHandleUpdateStats), 1); 2831 priv->wdev = wdev; 2832 wdev->wiphy->max_scan_ssids = MAX_NUM_PROBED_SSID; > 2833 wdev->wiphy->wowlan = &wowlan_support; 2834 wdev->wiphy->max_num_pmkids = WILC_MAX_NUM_PMKIDS; 2835 PRINT_INFO(CFG80211_DBG, "Max number of PMKIDs = %d\n", wdev->wiphy->max_num_pmkids); 2836 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
