Re: [PATCH v2] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap
On 01/31/2018 12:15 AM, Greg KH wrote:
On Tue, Jan 30, 2018 at 10:39:13PM +0200, Alexey Skidanov wrote:
dma_buf_vmap and dma_buf_vunmap allow drivers to access buffers, created by ion.
But why would anyone ever want to do that? What is wrong with the
existing interfaces that drivers use to access buffers created by ion?
Any driver, sharing the buffers, created by ion, through dma-buf, may
get back the
sgtable describing the buffer for device DMA and may call dma_buf_vmap
to get back
the kernel virtual address of the buffer to get access to it. Currently,
the second
option is missing. Actually, the buffer already mapped by ion
implementation.
What code uses this new interface?
This is not the new interface. this is the missing implementation of the
last two ops:
struct dma_buf_ops {
int (* attach) (struct dma_buf *, struct device *, struct
dma_buf_attachment *);
void (* detach) (struct dma_buf *, struct dma_buf_attachment *);
struct sg_table * (* map_dma_buf) (struct dma_buf_attachment *, enum
dma_data_direction);
void (* unmap_dma_buf) (struct dma_buf_attachment *,struct sg_table
*, enum dma_data_direction);
void (* release) (struct dma_buf *);
int (* begin_cpu_access) (struct dma_buf *, enum dma_data_direction);
int (* end_cpu_access) (struct dma_buf *, enum dma_data_direction);
void *(* map_atomic) (struct dma_buf *, unsigned long);
void (* unmap_atomic) (struct dma_buf *, unsigned long, void *);
void *(* map) (struct dma_buf *, unsigned long);
void (* unmap) (struct dma_buf *, unsigned long, void *);
int (* mmap) (struct dma_buf *, struct vm_area_struct *vma);
void *(* vmap) (struct dma_buf *);
void (* vunmap) (struct dma_buf *, void *vaddr);
};
We need a bit more information please.
thanks,
greg k-h
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: Fix compilation warning
Label unlock is defined but not used
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 5fa5363..080ff1c 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -341,17 +341,14 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf
*dmabuf,
{
struct ion_buffer *buffer = dmabuf->priv;
struct ion_dma_buf_attachment *a;
- int ret = 0;
mutex_lock(&buffer->lock);
list_for_each_entry(a, &buffer->attachments, list) {
dma_sync_sg_for_cpu(a->dev, a->table->sgl, a->table->nents,
direction);
}
-
-unlock:
mutex_unlock(&buffer->lock);
- return ret;
+ return 0;
}
static int ion_dma_buf_end_cpu_access(struct dma_buf *dmabuf,
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Fix compilation warning
On 09/08/2018 09:42 PM, Dan Carpenter wrote: > On Sat, Sep 08, 2018 at 07:33:40PM +0300, Alexey Skidanov wrote: >> Label unlock is defined but not used >> >> Signed-off-by: Alexey Skidanov > > There is no Fixes tag. I don't understand how the compile warning was > added in the first place? The code in linux-next from Friday doesn't > match. Sorry, you are correct. It's my fault. Thanks. > > What the heck is going on? > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: Add per-heap counters
The heap statistics have been removed and currently even basics statistics
are missing.
This patch creates per heap debugfs directory /sys/kernel/debug/
and adds two counters - the number of allocated buffers and number of
allocated bytes.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.c | 40 +++
drivers/staging/android/ion/ion.h | 3 ++-
2 files changed, 34 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..62335b9 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -95,6 +95,9 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
goto err1;
}
+ heap->num_of_buffers++;
+ heap->num_of_alloc_bytes += len;
+
INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
mutex_lock(&dev->buffer_lock);
@@ -528,6 +531,8 @@ void ion_device_add_heap(struct ion_heap *heap)
{
struct ion_device *dev = internal_dev;
int ret;
+ struct dentry *heap_root;
+ char debug_name[64];
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
@@ -546,6 +551,33 @@ void ion_device_add_heap(struct ion_heap *heap)
}
heap->dev = dev;
+ heap->num_of_buffers = 0;
+ heap->num_of_alloc_bytes = 0;
+
+ /* Create heap root directory. If creation is failed, we may continue */
+ heap_root = debugfs_create_dir(heap->name, dev->debug_root);
+ if (!IS_ERR_OR_NULL(heap_root)) {
+ debugfs_create_u64("num_of_buffers",
+ 0444, heap_root,
+ &heap->num_of_buffers);
+ debugfs_create_u64("num_of_alloc_bytes",
+ 0444,
+ heap_root,
+ &heap->num_of_alloc_bytes);
+
+ if (heap->shrinker.count_objects &&
+ heap->shrinker.scan_objects) {
+ snprintf(debug_name, 64, "%s_shrink", heap->name);
+ debugfs_create_file(debug_name,
+ 0644,
+ heap_root,
+ heap,
+ &debug_shrink_fops);
+ }
+ } else {
+ pr_err("%s: Failed to create heap dir in debugfs\n", __func__);
+ }
+
down_write(&dev->lock);
heap->id = heap_id++;
/*
@@ -555,14 +587,6 @@ void ion_device_add_heap(struct ion_heap *heap)
plist_node_init(&heap->node, -heap->id);
plist_add(&heap->node, &dev->heaps);
- if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
- char debug_name[64];
-
- snprintf(debug_name, 64, "%s_shrink", heap->name);
- debugfs_create_file(debug_name, 0644, dev->debug_root,
- heap, &debug_shrink_fops);
- }
-
dev->heap_cnt++;
up_write(&dev->lock);
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 16cbd38..8f1326a 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -179,9 +179,10 @@ struct ion_heap {
spinlock_t free_lock;
wait_queue_head_t waitqueue;
struct task_struct *task;
-
int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
void *unused);
+ u64 num_of_buffers;
+ u64 num_of_alloc_bytes;
};
/**
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add per-heap counters
On 09/10/2018 12:36 PM, Dan Carpenter wrote:
> On Sun, Sep 09, 2018 at 01:44:31AM +0300, Alexey Skidanov wrote:
>> The heap statistics have been removed and currently even basics statistics
>> are missing.
>
> Remind me why did we remove them? What was the git hash?
15c6098cfec5 ("staging: android: ion: Remove ion_handle and ion_client")
According to Laura, "The goal was to rip out the racy handle/buffer mess
and the associated debugfs there"
>
> regards,
> dan carpenter
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add per-heap counters
On 09/10/2018 11:27 AM, Greg KH wrote:
> On Sun, Sep 09, 2018 at 01:44:31AM +0300, Alexey Skidanov wrote:
>> The heap statistics have been removed and currently even basics statistics
>> are missing.
>>
>> This patch creates per heap debugfs directory /sys/kernel/debug/
>> and adds two counters - the number of allocated buffers and number of
>> allocated bytes.
>
> Why? What can we do with this information?
Are you in doubt that we need these two specific counters or debugfs at all?
>
>>
>> Signed-off-by: Alexey Skidanov
>> ---
>> drivers/staging/android/ion/ion.c | 40
>> +++
>> drivers/staging/android/ion/ion.h | 3 ++-
>> 2 files changed, 34 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 9907332..62335b9 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -95,6 +95,9 @@ static struct ion_buffer *ion_buffer_create(struct
>> ion_heap *heap,
>> goto err1;
>> }
>>
>> +heap->num_of_buffers++;
>> +heap->num_of_alloc_bytes += len;
>> +
>> INIT_LIST_HEAD(&buffer->attachments);
>> mutex_init(&buffer->lock);
>> mutex_lock(&dev->buffer_lock);
>> @@ -528,6 +531,8 @@ void ion_device_add_heap(struct ion_heap *heap)
>> {
>> struct ion_device *dev = internal_dev;
>> int ret;
>> +struct dentry *heap_root;
>> +char debug_name[64];
>>
>> if (!heap->ops->allocate || !heap->ops->free)
>> pr_err("%s: can not add heap with invalid ops struct.\n",
>> @@ -546,6 +551,33 @@ void ion_device_add_heap(struct ion_heap *heap)
>> }
>>
>> heap->dev = dev;
>> +heap->num_of_buffers = 0;
>> +heap->num_of_alloc_bytes = 0;
>> +
>> +/* Create heap root directory. If creation is failed, we may continue */
>> +heap_root = debugfs_create_dir(heap->name, dev->debug_root);
>> +if (!IS_ERR_OR_NULL(heap_root)) {
>
> Close, but no, never check the return value here. Just keep on moving,
> and call further debugfs calls. No need even for a comment here.
>
>> +debugfs_create_u64("num_of_buffers",
>> + 0444, heap_root,
>> + &heap->num_of_buffers);
>> +debugfs_create_u64("num_of_alloc_bytes",
>> + 0444,
>> + heap_root,
>> + &heap->num_of_alloc_bytes);
>> +
>> +if (heap->shrinker.count_objects &&
>> +heap->shrinker.scan_objects) {
>> +snprintf(debug_name, 64, "%s_shrink", heap->name);
>> +debugfs_create_file(debug_name,
>> +0644,
>> +heap_root,
>> +heap,
>> +&debug_shrink_fops);
>> +}
>> +} else {
>> +pr_err("%s: Failed to create heap dir in debugfs\n", __func__);
>
> No need to print anything.
>
>> +}
>> +
>> down_write(&dev->lock);
>> heap->id = heap_id++;
>> /*
>> @@ -555,14 +587,6 @@ void ion_device_add_heap(struct ion_heap *heap)
>> plist_node_init(&heap->node, -heap->id);
>> plist_add(&heap->node, &dev->heaps);
>>
>> -if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
>> -char debug_name[64];
>> -
>> -snprintf(debug_name, 64, "%s_shrink", heap->name);
>> -debugfs_create_file(debug_name, 0644, dev->debug_root,
>> -heap, &debug_shrink_fops);
>> -}
>
> You just moved the location of these debugfs files, right? What
> userspace tool just broke when you did that? :)
>
> thanks,
>
> greg k-h
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add per-heap counters
On 09/10/2018 05:21 PM, Greg KH wrote:
> On Mon, Sep 10, 2018 at 01:46:13PM +0300, Alexey Skidanov wrote:
>>
>>
>> On 09/10/2018 11:27 AM, Greg KH wrote:
>>> On Sun, Sep 09, 2018 at 01:44:31AM +0300, Alexey Skidanov wrote:
>>>> The heap statistics have been removed and currently even basics statistics
>>>> are missing.
>>>>
>>>> This patch creates per heap debugfs directory /sys/kernel/debug/
>>>> and adds two counters - the number of allocated buffers and number of
>>>> allocated bytes.
>>>
>>> Why? What can we do with this information?
>> Are you in doubt that we need these two specific counters or debugfs at all?
>
> These specific counters. What are you going to do with them?
I would like to see:
- heap watermark, in order to profile memory usage
- current heap status, in order to catch some memory leaks
Probably, we may add more.
>
> And you also moved debugfs files (you didn't comment on that comment),
> which will probably break tools that expect the files to be in one
> place, right?
Right. I assumed that for staging modules we still can change user
interface. Otherwise, how to explain this change 15c6098cfec5 ("staging:
android: ion: Remove ion_handle and ion_client")?
How did you test this patch?
I use self-tests
https://www.kernel.org/doc/readme/tools-testing-selftests-android-ion-README
and other proprietary code to check sharing buffers between several
kernel modules.
>
> thanks,
>
> greg k-h
>
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add per-heap counters
On 09/10/2018 06:57 PM, Greg KH wrote:
> On Mon, Sep 10, 2018 at 06:51:18PM +0300, Alexey Skidanov wrote:
>> On 09/10/2018 05:21 PM, Greg KH wrote:
>>> On Mon, Sep 10, 2018 at 01:46:13PM +0300, Alexey Skidanov wrote:
>>>> On 09/10/2018 11:27 AM, Greg KH wrote:
>>> And you also moved debugfs files (you didn't comment on that comment),
>>> which will probably break tools that expect the files to be in one
>>> place, right?
>> Right. I assumed that for staging modules we still can change user
>> interface. Otherwise, how to explain this change 15c6098cfec5 ("staging:
>> android: ion: Remove ion_handle and ion_client")?
>
> Yes, we can change them, but I asked if there were existing scripts that
> would break with this change, which isn't good. I know there are a
> number of android debug/crash tools/scripts that dig in debugfs to get
> lots of information, so breaking them is usually not nice :)
So, maybe we can address this question to Android guys? Who may help
with this issue?
>
> thanks,
>
> greg k-h
>
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add per-heap counters
On 09/10/2018 11:21 PM, Laura Abbott wrote:
> On 09/10/2018 03:00 AM, Alexey Skidanov wrote:
>>
>>
>> On 09/10/2018 12:36 PM, Dan Carpenter wrote:
>>> On Sun, Sep 09, 2018 at 01:44:31AM +0300, Alexey Skidanov wrote:
>>>> The heap statistics have been removed and currently even basics
>>>> statistics
>>>> are missing.
>>>
>>> Remind me why did we remove them? What was the git hash?
>> 15c6098cfec5 ("staging: android: ion: Remove ion_handle and ion_client")
>> According to Laura, "The goal was to rip out the racy handle/buffer mess
>> and the associated debugfs there"
>
> Yes, the debugfs was removed with that commit because the underlying
> handle/client were being removed and the associated debugfs
> tended to race with tear down. There was also a question about what
> information should go in Ion vs. dma_buf since Ion is now basically
> a wrapper around dma_buf.
>
> I don't disagree that we want some kind of debugfs counters but
> I'd like to see some description of what problems we can solve
> with this that aren't also solved by the debugfs with dma_buf.
The dma_buf debugfs provides very detailed information about allocated
buffers. However the heap related statistics are missing. I would
suggest to start with at least heap watermark, in order to profile heap
memory usage.
In addition, it might be good to correlate dma_buf debugfs and ion
debugfs. For example, to get per heap buffers list.
>
> Thanks,
> Laura
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: android: ion: Add per-heap counters
Heap statistics have been removed and currently even basics statistics
are missing.
This patch creates per heap debugfs directory /sys/kernel/debug/
and adds the following counters:
- the number of allocated buffers;
- the number of allocated bytes;
- the number of allocated bytes watermark.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.c | 50 ---
drivers/staging/android/ion/ion.h | 6 ++---
2 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..dad7cc9 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -95,6 +95,11 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
goto err1;
}
+ heap->num_of_buffers++;
+ heap->num_of_alloc_bytes += len;
+ if (heap->num_of_alloc_bytes > heap->alloc_bytes_wm)
+ heap->alloc_bytes_wm = heap->num_of_alloc_bytes;
+
INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
mutex_lock(&dev->buffer_lock);
@@ -117,6 +122,9 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
}
buffer->heap->ops->free(buffer);
+ buffer->heap->num_of_buffers--;
+ buffer->heap->num_of_alloc_bytes -= buffer->size;
+
kfree(buffer);
}
@@ -528,6 +536,8 @@ void ion_device_add_heap(struct ion_heap *heap)
{
struct ion_device *dev = internal_dev;
int ret;
+ struct dentry *heap_root;
+ char debug_name[64];
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
@@ -546,6 +556,38 @@ void ion_device_add_heap(struct ion_heap *heap)
}
heap->dev = dev;
+ heap->num_of_buffers = 0;
+ heap->num_of_alloc_bytes = 0;
+ heap->alloc_bytes_wm = 0;
+
+ /* Create heap root directory. If creation is failed, we may continue */
+ heap_root = debugfs_create_dir(heap->name, dev->debug_root);
+ if (!IS_ERR_OR_NULL(heap_root)) {
+ debugfs_create_u64("num_of_buffers",
+ 0444, heap_root,
+ &heap->num_of_buffers);
+ debugfs_create_u64("num_of_alloc_bytes",
+ 0444,
+ heap_root,
+ &heap->num_of_alloc_bytes);
+ debugfs_create_u64("alloc_bytes_wm",
+ 0444,
+ heap_root,
+ &heap->alloc_bytes_wm);
+
+ if (heap->shrinker.count_objects &&
+ heap->shrinker.scan_objects) {
+ snprintf(debug_name, 64, "%s_shrink", heap->name);
+ debugfs_create_file(debug_name,
+ 0644,
+ heap_root,
+ heap,
+ &debug_shrink_fops);
+ }
+ } else {
+ pr_err("%s: Failed to create heap dir in debugfs\n", __func__);
+ }
+
down_write(&dev->lock);
heap->id = heap_id++;
/*
@@ -555,14 +597,6 @@ void ion_device_add_heap(struct ion_heap *heap)
plist_node_init(&heap->node, -heap->id);
plist_add(&heap->node, &dev->heaps);
- if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
- char debug_name[64];
-
- snprintf(debug_name, 64, "%s_shrink", heap->name);
- debugfs_create_file(debug_name, 0644, dev->debug_root,
- heap, &debug_shrink_fops);
- }
-
dev->heap_cnt++;
up_write(&dev->lock);
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 16cbd38..bea84b6 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -179,9 +179,9 @@ struct ion_heap {
spinlock_t free_lock;
wait_queue_head_t waitqueue;
struct task_struct *task;
-
- int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
- void *unused);
+ u64 num_of_buffers;
+ u64 num_of_alloc_bytes;
+ u64 alloc_bytes_wm;
};
/**
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: android: ion: Add per-heap counters
On 09/11/2018 11:59 AM, Greg KH wrote:
> On Tue, Sep 11, 2018 at 11:50:19AM +0300, Dan Carpenter wrote:
>> On Tue, Sep 11, 2018 at 11:17:10AM +0300, Alexey Skidanov wrote:
>>> @@ -546,6 +556,38 @@ void ion_device_add_heap(struct ion_heap *heap)
>>> }
>>>
>>> heap->dev = dev;
>>> + heap->num_of_buffers = 0;
>>> + heap->num_of_alloc_bytes = 0;
>>> + heap->alloc_bytes_wm = 0;
>>> +
>>> + /* Create heap root directory. If creation is failed, we may continue */
>>> + heap_root = debugfs_create_dir(heap->name, dev->debug_root);
>>> + if (!IS_ERR_OR_NULL(heap_root)) {
>>
>> Just remove this check. If debugfs_create_dir() fails, it doesn't
>> cause any problems here.
If debugfs_create_dir fails, the heap_root will be either NULL (and thus
the underlying files will be created under /sys/kernel/debug) or -EXXX
(and thus we probably crash the Kernel). Am I wrong?
>
> I asked for that to be done last time :(
What exactly?
>
> {sigh}
>
> greg k-h
>
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: android: ion: Add per-heap counters
On 09/11/2018 12:15 PM, Dan Carpenter wrote:
> On Tue, Sep 11, 2018 at 12:11:23PM +0300, Alexey Skidanov wrote:
>>
>>
>> On 09/11/2018 11:59 AM, Greg KH wrote:
>>> On Tue, Sep 11, 2018 at 11:50:19AM +0300, Dan Carpenter wrote:
>>>> On Tue, Sep 11, 2018 at 11:17:10AM +0300, Alexey Skidanov wrote:
>>>>> @@ -546,6 +556,38 @@ void ion_device_add_heap(struct ion_heap *heap)
>>>>> }
>>>>>
>>>>> heap->dev = dev;
>>>>> + heap->num_of_buffers = 0;
>>>>> + heap->num_of_alloc_bytes = 0;
>>>>> + heap->alloc_bytes_wm = 0;
>>>>> +
>>>>> + /* Create heap root directory. If creation is failed, we may continue */
>>>>> + heap_root = debugfs_create_dir(heap->name, dev->debug_root);
>>>>> + if (!IS_ERR_OR_NULL(heap_root)) {
>>>>
>>>> Just remove this check. If debugfs_create_dir() fails, it doesn't
>>>> cause any problems here.
>> If debugfs_create_dir fails, the heap_root will be either NULL (and thus
>> the underlying files will be created under /sys/kernel/debug) or -EXXX
>> (and thus we probably crash the Kernel). Am I wrong?
>
> Yes. You're wrong. It's fine.
>
> It's designed to normally work without error handling. Some drivers
> dereference "heap_root" so they would need to check, but if you're not
> dereferencing it yourself, then debugfs checks for NULL. (It doesn't
> check for error pointer because in that situation debugfs functions
> are all no-ops).
>
> regards,
> dan carpenter
>
Agree with you regarding the error case. But in case of NULL - the
underlying files will be created in the wrong path. So probably it makes
sense to check it for NULL only?
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: android: ion: Add per-heap counters
On 09/11/2018 12:31 PM, Greg KH wrote:
> On Tue, Sep 11, 2018 at 12:11:23PM +0300, Alexey Skidanov wrote:
>>
>>
>> On 09/11/2018 11:59 AM, Greg KH wrote:
>>> On Tue, Sep 11, 2018 at 11:50:19AM +0300, Dan Carpenter wrote:
>>>> On Tue, Sep 11, 2018 at 11:17:10AM +0300, Alexey Skidanov wrote:
>>>>> @@ -546,6 +556,38 @@ void ion_device_add_heap(struct ion_heap *heap)
>>>>> }
>>>>>
>>>>> heap->dev = dev;
>>>>> + heap->num_of_buffers = 0;
>>>>> + heap->num_of_alloc_bytes = 0;
>>>>> + heap->alloc_bytes_wm = 0;
>>>>> +
>>>>> + /* Create heap root directory. If creation is failed, we may continue */
>>>>> + heap_root = debugfs_create_dir(heap->name, dev->debug_root);
>>>>> + if (!IS_ERR_OR_NULL(heap_root)) {
>>>>
>>>> Just remove this check. If debugfs_create_dir() fails, it doesn't
>>>> cause any problems here.
>> If debugfs_create_dir fails, the heap_root will be either NULL (and thus
>> the underlying files will be created under /sys/kernel/debug) or -EXXX
>> (and thus we probably crash the Kernel). Am I wrong?
>
> If the result is NULL, then something bad is wrong with debugfs, so any
> future calls will also fail, no need to worry.
>
> If -ENODEV returns, then you can pass that into a future debugfs call
> just fine as well, all is good.
>
>>> I asked for that to be done last time :(
>> What exactly?
>
> I asked that you not check the return value of the debugfs call, it's
> explicitly designed to not need to do this. Don't do extra work for no
> reason :)
Ok, got it ... Will fix.
>
> thanks,
>
> greg k-h
>
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: android: ion: Add per-heap counters
Heap statistics have been removed and currently even basics statistics
are missing.
This patch creates per heap debugfs directory /sys/kernel/debug/
and adds the following counters:
- the number of allocated buffers;
- the number of allocated bytes;
- the number of allocated bytes watermark.
Signed-off-by: Alexey Skidanov
---
v3:
Removed debugfs_create_dir() return value checking
drivers/staging/android/ion/ion.c | 46 ---
drivers/staging/android/ion/ion.h | 6 ++---
2 files changed, 41 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..ba4c6e6 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -95,6 +95,11 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
goto err1;
}
+ heap->num_of_buffers++;
+ heap->num_of_alloc_bytes += len;
+ if (heap->num_of_alloc_bytes > heap->alloc_bytes_wm)
+ heap->alloc_bytes_wm = heap->num_of_alloc_bytes;
+
INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
mutex_lock(&dev->buffer_lock);
@@ -117,6 +122,9 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
}
buffer->heap->ops->free(buffer);
+ buffer->heap->num_of_buffers--;
+ buffer->heap->num_of_alloc_bytes -= buffer->size;
+
kfree(buffer);
}
@@ -528,6 +536,8 @@ void ion_device_add_heap(struct ion_heap *heap)
{
struct ion_device *dev = internal_dev;
int ret;
+ struct dentry *heap_root;
+ char debug_name[64];
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
@@ -546,6 +556,34 @@ void ion_device_add_heap(struct ion_heap *heap)
}
heap->dev = dev;
+ heap->num_of_buffers = 0;
+ heap->num_of_alloc_bytes = 0;
+ heap->alloc_bytes_wm = 0;
+
+ /* Create heap root directory */
+ heap_root = debugfs_create_dir(heap->name, dev->debug_root);
+ debugfs_create_u64("num_of_buffers",
+ 0444, heap_root,
+ &heap->num_of_buffers);
+ debugfs_create_u64("num_of_alloc_bytes",
+ 0444,
+ heap_root,
+ &heap->num_of_alloc_bytes);
+ debugfs_create_u64("alloc_bytes_wm",
+ 0444,
+ heap_root,
+ &heap->alloc_bytes_wm);
+
+ if (heap->shrinker.count_objects &&
+ heap->shrinker.scan_objects) {
+ snprintf(debug_name, 64, "%s_shrink", heap->name);
+ debugfs_create_file(debug_name,
+ 0644,
+ heap_root,
+ heap,
+ &debug_shrink_fops);
+ }
+
down_write(&dev->lock);
heap->id = heap_id++;
/*
@@ -555,14 +593,6 @@ void ion_device_add_heap(struct ion_heap *heap)
plist_node_init(&heap->node, -heap->id);
plist_add(&heap->node, &dev->heaps);
- if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
- char debug_name[64];
-
- snprintf(debug_name, 64, "%s_shrink", heap->name);
- debugfs_create_file(debug_name, 0644, dev->debug_root,
- heap, &debug_shrink_fops);
- }
-
dev->heap_cnt++;
up_write(&dev->lock);
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 16cbd38..bea84b6 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -179,9 +179,9 @@ struct ion_heap {
spinlock_t free_lock;
wait_queue_head_t waitqueue;
struct task_struct *task;
-
- int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
- void *unused);
+ u64 num_of_buffers;
+ u64 num_of_alloc_bytes;
+ u64 alloc_bytes_wm;
};
/**
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: android: ion: Add per-heap counters
On 09/21/2018 05:40 PM, Laura Abbott wrote:
> On 09/11/2018 04:29 AM, Alexey Skidanov wrote:
>> Heap statistics have been removed and currently even basics statistics
>> are missing.
>>
>> This patch creates per heap debugfs directory
>> /sys/kernel/debug/
>> and adds the following counters:
>> - the number of allocated buffers;
>> - the number of allocated bytes;
>> - the number of allocated bytes watermark.
>>
>> Signed-off-by: Alexey Skidanov
>> ---
>>
>> v3:
>> Removed debugfs_create_dir() return value checking
>>
>> drivers/staging/android/ion/ion.c | 46
>> ---
>> drivers/staging/android/ion/ion.h | 6 ++---
>> 2 files changed, 41 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 9907332..ba4c6e6 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -95,6 +95,11 @@ static struct ion_buffer *ion_buffer_create(struct
>> ion_heap *heap,
>> goto err1;
>> }
>> + heap->num_of_buffers++;
>> + heap->num_of_alloc_bytes += len;
>> + if (heap->num_of_alloc_bytes > heap->alloc_bytes_wm)
>> + heap->alloc_bytes_wm = heap->num_of_alloc_bytes;
>> +
>> INIT_LIST_HEAD(&buffer->attachments);
>> mutex_init(&buffer->lock);
>> mutex_lock(&dev->buffer_lock);
>> @@ -117,6 +122,9 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
>> buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
>> }
>> buffer->heap->ops->free(buffer);
>> + buffer->heap->num_of_buffers--;
>> + buffer->heap->num_of_alloc_bytes -= buffer->size;
>> +
>> kfree(buffer);
>> }
>> @@ -528,6 +536,8 @@ void ion_device_add_heap(struct ion_heap *heap)
>> {
>> struct ion_device *dev = internal_dev;
>> int ret;
>> + struct dentry *heap_root;
>> + char debug_name[64];
>> if (!heap->ops->allocate || !heap->ops->free)
>> pr_err("%s: can not add heap with invalid ops struct.\n",
>> @@ -546,6 +556,34 @@ void ion_device_add_heap(struct ion_heap *heap)
>> }
>> heap->dev = dev;
>> + heap->num_of_buffers = 0;
>> + heap->num_of_alloc_bytes = 0;
>> + heap->alloc_bytes_wm = 0;
>> +
>> + /* Create heap root directory */
>> + heap_root = debugfs_create_dir(heap->name, dev->debug_root);
>> + debugfs_create_u64("num_of_buffers",
>> + 0444, heap_root,
>> + &heap->num_of_buffers);
>> + debugfs_create_u64("num_of_alloc_bytes",
>> + 0444,
>> + heap_root,
>> + &heap->num_of_alloc_bytes);
>> + debugfs_create_u64("alloc_bytes_wm",
>> + 0444,
>> + heap_root,
>> + &heap->alloc_bytes_wm);
>> +
>> + if (heap->shrinker.count_objects &&
>> + heap->shrinker.scan_objects) {
>> + snprintf(debug_name, 64, "%s_shrink", heap->name);
>> + debugfs_create_file(debug_name,
>> + 0644,
>> + heap_root,
>> + heap,
>> + &debug_shrink_fops);
>> + }
>> +
>> down_write(&dev->lock);
>> heap->id = heap_id++;
>> /*
>> @@ -555,14 +593,6 @@ void ion_device_add_heap(struct ion_heap *heap)
>> plist_node_init(&heap->node, -heap->id);
>> plist_add(&heap->node, &dev->heaps);
>> - if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
>> - char debug_name[64];
>> -
>> - snprintf(debug_name, 64, "%s_shrink", heap->name);
>> - debugfs_create_file(debug_name, 0644, dev->debug_root,
>> - heap, &debug_shrink_fops);
>> - }
>> -
>> dev->heap_cnt++;
>> up_write(&dev->lock);
>> }
>> diff --git a/drivers/staging/android/ion/ion.h
>> b/drivers/staging/android/ion/ion.h
>> index 16cbd38..bea84b6 100644
>> --- a/drivers/staging/android/ion/ion.h
>> +++ b/drivers/staging/android/ion/ion.h
>> @@ -179,9 +179,9 @@ struct ion_heap {
>> spinlock_t free_lock;
>> wait_queue_head_t waitqueue;
>> struct task_struct *task;
>> -
>> - int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
>> - void *unused);
>> + u64 num_of_buffers;
>> + u64 num_of_alloc_bytes;
>> + u64 alloc_bytes_wm;
>
> What lock is protecting these?
You are correct - I missed it. Will add.
>
>> };
>> /**
>>
>
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: axis-fifo: add error handling of class_create()
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/axis-fifo/axis-fifo.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/axis-fifo/axis-fifo.c
b/drivers/staging/axis-fifo/axis-fifo.c
index abeee0ecc122..63c8efd1b8db 100644
--- a/drivers/staging/axis-fifo/axis-fifo.c
+++ b/drivers/staging/axis-fifo/axis-fifo.c
@@ -1089,6 +1089,8 @@ static int __init axis_fifo_init(void)
pr_info("axis-fifo driver loaded with parameters read_timeout = %i,
write_timeout = %i\n",
read_timeout, write_timeout);
axis_fifo_driver_class = class_create(THIS_MODULE, DRIVER_NAME);
+ if (IS_ERR(axis_fifo_driver_class))
+ return PTR_ERR(axis_fifo_driver_class);
return platform_driver_register(&axis_fifo_driver);
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4] staging: android: ion: Add per-heap counters
Heap statistics have been removed and currently even basics statistics
are missing.
This patch creates per heap debugfs directory /sys/kernel/debug/
and adds the following counters:
- the number of allocated buffers;
- the number of allocated bytes;
- the number of allocated bytes watermark.
Signed-off-by: Alexey Skidanov
---
v3:
Removed debugfs_create_dir() return value checking
v4:
Added spinlock to protect heap statistics
drivers/staging/android/ion/ion.c | 50 ---
drivers/staging/android/ion/ion.h | 10 +++-
2 files changed, 51 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..6fd8979 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -95,6 +95,13 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
goto err1;
}
+ spin_lock(&heap->stat_lock);
+ heap->num_of_buffers++;
+ heap->num_of_alloc_bytes += len;
+ if (heap->num_of_alloc_bytes > heap->alloc_bytes_wm)
+ heap->alloc_bytes_wm = heap->num_of_alloc_bytes;
+ spin_unlock(&heap->stat_lock);
+
INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
mutex_lock(&dev->buffer_lock);
@@ -117,6 +124,11 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
}
buffer->heap->ops->free(buffer);
+ spin_lock(&buffer->heap->stat_lock);
+ buffer->heap->num_of_buffers--;
+ buffer->heap->num_of_alloc_bytes -= buffer->size;
+ spin_unlock(&buffer->heap->stat_lock);
+
kfree(buffer);
}
@@ -528,12 +540,15 @@ void ion_device_add_heap(struct ion_heap *heap)
{
struct ion_device *dev = internal_dev;
int ret;
+ struct dentry *heap_root;
+ char debug_name[64];
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
__func__);
spin_lock_init(&heap->free_lock);
+ spin_lock_init(&heap->stat_lock);
heap->free_list_size = 0;
if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
@@ -546,6 +561,33 @@ void ion_device_add_heap(struct ion_heap *heap)
}
heap->dev = dev;
+ heap->num_of_buffers = 0;
+ heap->num_of_alloc_bytes = 0;
+ heap->alloc_bytes_wm = 0;
+
+ heap_root = debugfs_create_dir(heap->name, dev->debug_root);
+ debugfs_create_u64("num_of_buffers",
+ 0444, heap_root,
+ &heap->num_of_buffers);
+ debugfs_create_u64("num_of_alloc_bytes",
+ 0444,
+ heap_root,
+ &heap->num_of_alloc_bytes);
+ debugfs_create_u64("alloc_bytes_wm",
+ 0444,
+ heap_root,
+ &heap->alloc_bytes_wm);
+
+ if (heap->shrinker.count_objects &&
+ heap->shrinker.scan_objects) {
+ snprintf(debug_name, 64, "%s_shrink", heap->name);
+ debugfs_create_file(debug_name,
+ 0644,
+ heap_root,
+ heap,
+ &debug_shrink_fops);
+ }
+
down_write(&dev->lock);
heap->id = heap_id++;
/*
@@ -555,14 +597,6 @@ void ion_device_add_heap(struct ion_heap *heap)
plist_node_init(&heap->node, -heap->id);
plist_add(&heap->node, &dev->heaps);
- if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
- char debug_name[64];
-
- snprintf(debug_name, 64, "%s_shrink", heap->name);
- debugfs_create_file(debug_name, 0644, dev->debug_root,
- heap, &debug_shrink_fops);
- }
-
dev->heap_cnt++;
up_write(&dev->lock);
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 16cbd38..f487127 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -159,6 +159,9 @@ struct ion_heap_ops {
* @task: task struct of deferred free thread
* @debug_show:called when heap debug file is read to add any
* heap specific debug info to output
+ * @num_of_buffers the number of currently allocated buffers
+ * @num_of_alloc_bytes the number of allocated bytes
+ * @alloc_
staging: android: ion: aligned allocation support
Hi, Sometimes HW requires memory buffer to be aligned in order to be used properly. Of course, we may overcome the lack of aligned allocation support, but we may easily add it because CMA and gen_pool (used by several heaps) already support it. Does someone have an objection to add it? Thanks, Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: android: ion: aligned allocation support
On 10/03/2018 09:07 PM, Laura Abbott wrote: > On 10/02/2018 07:27 AM, Alexey Skidanov wrote: >> Hi, >> >> Sometimes HW requires memory buffer to be aligned in order to be used >> properly. Of course, we may overcome the lack of aligned allocation >> support, but we may easily add it because CMA and gen_pool (used by >> several heaps) already support it. >> >> Does someone have an objection to add it? >> >> Thanks, >> Alexey >> > > The alignment option was removed from the allocation API before > because the most common heap (system heap) didn't support it > and it was causing more confusion. We've already mangled the > ABI once so I really don't want to break it again. I'm not > opposed to adding alignment support for the CMA via the allocation > flags. Currently, the flags member is used to define the way the buffer will be mapped - cached or uncached. So,if I understand you correct, we need to add ION_FLAG_ALIGNED flag and to share 32 bit field between flags and flags specific data (alignment value) ? I'm probably going to remove the carveout and chunk heap because > nobody has stepped up to figure out how to tie allocation of those > to device tree or another method. > > Thanks, > Laura Thanks, Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: staging: android: ion: aligned allocation support
> -Original Message- > From: Laura Abbott [mailto:labb...@redhat.com] > Sent: Monday, October 08, 2018 21:26 > To: Skidanov, Alexey ; > de...@driverdev.osuosl.org > Cc: Sumit Semwal > Subject: Re: staging: android: ion: aligned allocation support > > On 10/03/2018 01:03 PM, Alexey Skidanov wrote: > > > > > > On 10/03/2018 09:07 PM, Laura Abbott wrote: > >> On 10/02/2018 07:27 AM, Alexey Skidanov wrote: > >>> Hi, > >>> > >>> Sometimes HW requires memory buffer to be aligned in order to be used > >>> properly. Of course, we may overcome the lack of aligned allocation > >>> support, but we may easily add it because CMA and gen_pool (used by > >>> several heaps) already support it. > >>> > >>> Does someone have an objection to add it? > >>> > >>> Thanks, > >>> Alexey > >>> > >> > >> The alignment option was removed from the allocation API before > >> because the most common heap (system heap) didn't support it > >> and it was causing more confusion. We've already mangled the > >> ABI once so I really don't want to break it again. I'm not > >> opposed to adding alignment support for the CMA via the allocation > >> flags. > > Currently, the flags member is used to define the way the buffer will be > > mapped - cached or uncached. So,if I understand you correct, we need to > > add ION_FLAG_ALIGNED flag and to share 32 bit field between flags and > > flags specific data (alignment value) ? > > > > Yes, that's what I was thinking. Excellent. I will prepare the patch. > > > I'm probably going to remove the carveout and chunk heap because > >> nobody has stepped up to figure out how to tie allocation of those > >> to device tree or another method. > >> > >> Thanks, > >> Laura > > > > Thanks, > > Alexey > > Thanks, Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5] staging: android: ion: Add per-heap counters
Heap statistics have been removed and currently even basics statistics
are missing.
This patch creates per heap debugfs directory /sys/kernel/debug/
and adds the following counters:
- the number of allocated buffers;
- the number of allocated bytes;
- the number of allocated bytes watermark.
Signed-off-by: Alexey Skidanov
---
v3:
Removed debugfs_create_dir() return value checking
v4:
Added spinlock to protect heap statistics
v5:
Rebased on staging-next
drivers/staging/android/ion/ion.c | 50 ---
drivers/staging/android/ion/ion.h | 9 +++
2 files changed, 51 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..6fd8979 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -95,6 +95,13 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap
*heap,
goto err1;
}
+ spin_lock(&heap->stat_lock);
+ heap->num_of_buffers++;
+ heap->num_of_alloc_bytes += len;
+ if (heap->num_of_alloc_bytes > heap->alloc_bytes_wm)
+ heap->alloc_bytes_wm = heap->num_of_alloc_bytes;
+ spin_unlock(&heap->stat_lock);
+
INIT_LIST_HEAD(&buffer->attachments);
mutex_init(&buffer->lock);
mutex_lock(&dev->buffer_lock);
@@ -117,6 +124,11 @@ void ion_buffer_destroy(struct ion_buffer *buffer)
buffer->heap->ops->unmap_kernel(buffer->heap, buffer);
}
buffer->heap->ops->free(buffer);
+ spin_lock(&buffer->heap->stat_lock);
+ buffer->heap->num_of_buffers--;
+ buffer->heap->num_of_alloc_bytes -= buffer->size;
+ spin_unlock(&buffer->heap->stat_lock);
+
kfree(buffer);
}
@@ -528,12 +540,15 @@ void ion_device_add_heap(struct ion_heap *heap)
{
struct ion_device *dev = internal_dev;
int ret;
+ struct dentry *heap_root;
+ char debug_name[64];
if (!heap->ops->allocate || !heap->ops->free)
pr_err("%s: can not add heap with invalid ops struct.\n",
__func__);
spin_lock_init(&heap->free_lock);
+ spin_lock_init(&heap->stat_lock);
heap->free_list_size = 0;
if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
@@ -546,6 +561,33 @@ void ion_device_add_heap(struct ion_heap *heap)
}
heap->dev = dev;
+ heap->num_of_buffers = 0;
+ heap->num_of_alloc_bytes = 0;
+ heap->alloc_bytes_wm = 0;
+
+ heap_root = debugfs_create_dir(heap->name, dev->debug_root);
+ debugfs_create_u64("num_of_buffers",
+ 0444, heap_root,
+ &heap->num_of_buffers);
+ debugfs_create_u64("num_of_alloc_bytes",
+ 0444,
+ heap_root,
+ &heap->num_of_alloc_bytes);
+ debugfs_create_u64("alloc_bytes_wm",
+ 0444,
+ heap_root,
+ &heap->alloc_bytes_wm);
+
+ if (heap->shrinker.count_objects &&
+ heap->shrinker.scan_objects) {
+ snprintf(debug_name, 64, "%s_shrink", heap->name);
+ debugfs_create_file(debug_name,
+ 0644,
+ heap_root,
+ heap,
+ &debug_shrink_fops);
+ }
+
down_write(&dev->lock);
heap->id = heap_id++;
/*
@@ -555,14 +597,6 @@ void ion_device_add_heap(struct ion_heap *heap)
plist_node_init(&heap->node, -heap->id);
plist_add(&heap->node, &dev->heaps);
- if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
- char debug_name[64];
-
- snprintf(debug_name, 64, "%s_shrink", heap->name);
- debugfs_create_file(debug_name, 0644, dev->debug_root,
- heap, &debug_shrink_fops);
- }
-
dev->heap_cnt++;
up_write(&dev->lock);
}
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index c006fc1..47b594c 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -157,6 +157,9 @@ struct ion_heap_ops {
* @lock: protects the free list
* @waitqueue: queue to wait on from deferred free thread
* @task: task struct of deferred free thread
+ * @num_of_buffers the number of currently allocated buffers
+ * @num_of_alloc_bytes the number of allocated bytes
[PATCH] staging: android: ion: Fixed uninitialized heap name access
The heap name might be uninitialized and access might crash the
kernel.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c
b/drivers/staging/android/ion/ion.c
index 9907332..55bca92d 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -459,8 +459,11 @@ int ion_query_heaps(struct ion_heap_query *query)
max_cnt = query->cnt;
plist_for_each_entry(heap, &dev->heaps, node) {
- strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
- hdata.name[sizeof(hdata.name) - 1] = '\0';
+ if (heap->name) {
+ strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
+ hdata.name[sizeof(hdata.name) - 1] = '\0';
+ }
+
hdata.type = heap->type;
hdata.heap_id = heap->id;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Fixed uninitialized heap name access
On 10/22/18 17:32, Laura Abbott wrote:
> On 10/22/2018 07:02 AM, Alexey Skidanov wrote:
>> The heap name might be uninitialized and access might crash the
>> kernel.
>>
>
> The heap name should never be null so this seems like this is being
> fixed in the wrong place. Can you explain more how you are hitting
> this issue?
Sure. Carve out heap name is uninitialized. There is the next patch
fixing it. But to be on the safe side, I have added the check.
Thanks,
Alexey
>
> Thanks,
> Laura
>
>> Signed-off-by: Alexey Skidanov
>> ---
>> drivers/staging/android/ion/ion.c | 7 +--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 9907332..55bca92d 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -459,8 +459,11 @@ int ion_query_heaps(struct ion_heap_query *query)
>> max_cnt = query->cnt;
>> plist_for_each_entry(heap, &dev->heaps, node) {
>> - strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
>> - hdata.name[sizeof(hdata.name) - 1] = '\0';
>> + if (heap->name) {
>> + strncpy(hdata.name, heap->name, MAX_HEAP_NAME);
>> + hdata.name[sizeof(hdata.name) - 1] = '\0';
>> + }
>> +
>> hdata.type = heap->type;
>> hdata.heap_id = heap->id;
>>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Add carve out heap name initialization
Heap name is mundatory. Signed-off-by: Alexey Skidanov --- drivers/staging/android/ion/ion_carveout_heap.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/android/ion/ion_carveout_heap.c b/drivers/staging/android/ion/ion_carveout_heap.c index e129237..e89b464 100644 --- a/drivers/staging/android/ion/ion_carveout_heap.c +++ b/drivers/staging/android/ion/ion_carveout_heap.c @@ -131,6 +131,7 @@ struct ion_heap *ion_carveout_heap_create(struct ion_platform_heap *heap_data) gen_pool_add(carveout_heap->pool, carveout_heap->base, heap_data->size, -1); carveout_heap->heap.ops = &carveout_heap_ops; + carveout_heap->heap.name = heap_data->name; carveout_heap->heap.type = ION_HEAP_TYPE_CARVEOUT; carveout_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] staging: android: ion: Fixed uninitialized heap name access
> -Original Message- > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > Sent: Tuesday, October 23, 2018 08:33 > To: Skidanov, Alexey > Cc: Laura Abbott ; gre...@linuxfoundation.org; > de...@driverdev.osuosl.org > Subject: Re: [PATCH] staging: android: ion: Fixed uninitialized heap name > access > > On Mon, Oct 22, 2018 at 05:47:08PM +0300, Alexey Skidanov wrote: > > > > > > On 10/22/18 17:32, Laura Abbott wrote: > > > On 10/22/2018 07:02 AM, Alexey Skidanov wrote: > > >> The heap name might be uninitialized and access might crash the > > >> kernel. > > >> > > > > > > The heap name should never be null so this seems like this is being > > > fixed in the wrong place. Can you explain more how you are hitting > > > this issue? > > Sure. Carve out heap name is uninitialized. There is the next patch > > fixing it. But to be on the safe side, I have added the check. > > > > You keep saying uninitialized but you mean NULL. I meant the uninitialized name, not the pointer. > > ion_carveout_heap_create() is never called so far as I can see so this > isn't an issue in real life. It feels like it would be detected right ion_carveout_heap_create() is the only way to create this kind of heap. You are correct that currently it's never called - it's designed to be called by board specific code and in the meanwhile there is no standard way to do it. > away when that code was used. We should just apply your follow on > patch instead. > > regards, > dan carpenter Thanks, Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Add carve out heap name initialization
On 11/8/18 9:15 PM, Laura Abbott wrote: > On 10/22/18 2:15 PM, Alexey Skidanov wrote: >> Heap name is mundatory. >> > > I'm wary of this and the other change because it misses the > broader problem of dealing with the carveout heaps. > I still want to remove the carveout and chunk heap. I get > that it's being used for out of tree work but at this point > the focus needs to be on moving Ion out of staging and > if we can't get an end-to-end solution for carveout/chunk > heaps to be allocated in tree this needs to be removed. > > Thanks, > Laura There are several options I would suggest: 1. The heaps may be initialized by parsing some kernel parameter, defining the contiguous chunks 2. Some functions may be exported by ION to initialize the heaps 3. CONFIG_XXX options (just like it's in CMA) Thanks, Alexey > >> Signed-off-by: Alexey Skidanov >> --- >> drivers/staging/android/ion/ion_carveout_heap.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/staging/android/ion/ion_carveout_heap.c >> b/drivers/staging/android/ion/ion_carveout_heap.c >> index e129237..e89b464 100644 >> --- a/drivers/staging/android/ion/ion_carveout_heap.c >> +++ b/drivers/staging/android/ion/ion_carveout_heap.c >> @@ -131,6 +131,7 @@ struct ion_heap *ion_carveout_heap_create(struct >> ion_platform_heap *heap_data) >> gen_pool_add(carveout_heap->pool, carveout_heap->base, >> heap_data->size, >> -1); >> carveout_heap->heap.ops = &carveout_heap_ops; >> + carveout_heap->heap.name = heap_data->name; >> carveout_heap->heap.type = ION_HEAP_TYPE_CARVEOUT; >> carveout_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE; >> > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Add carve out heap name initialization
On 11/8/18 9:41 PM, Laura Abbott wrote: > On 11/8/18 11:34 AM, Alexey Skidanov wrote: >> >> >> On 11/8/18 9:15 PM, Laura Abbott wrote: >>> On 10/22/18 2:15 PM, Alexey Skidanov wrote: >>>> Heap name is mundatory. >>>> >>> >>> I'm wary of this and the other change because it misses the >>> broader problem of dealing with the carveout heaps. >>> I still want to remove the carveout and chunk heap. I get >>> that it's being used for out of tree work but at this point >>> the focus needs to be on moving Ion out of staging and >>> if we can't get an end-to-end solution for carveout/chunk >>> heaps to be allocated in tree this needs to be removed. >>> >>> Thanks, >>> Laura >> There are several options I would suggest: >> 1. The heaps may be initialized by parsing some kernel parameter, >> defining the contiguous chunks >> 2. Some functions may be exported by ION to initialize the heaps >> 3. CONFIG_XXX options (just like it's in CMA) >> > > We've had lots of suggestions but nobody has actually stepped > up to submit patches to make this work. If you'd like to submit > patches that would be great. > Yes, sure, I will submit in the coming weeks along with some other suggestions I have. Thanks, Alexey > Thanks, > Laura > >> Thanks, >> Alexey >> >>> >>>> Signed-off-by: Alexey Skidanov >>>> --- >>>> drivers/staging/android/ion/ion_carveout_heap.c | 1 + >>>> 1 file changed, 1 insertion(+) >>>> >>>> diff --git a/drivers/staging/android/ion/ion_carveout_heap.c >>>> b/drivers/staging/android/ion/ion_carveout_heap.c >>>> index e129237..e89b464 100644 >>>> --- a/drivers/staging/android/ion/ion_carveout_heap.c >>>> +++ b/drivers/staging/android/ion/ion_carveout_heap.c >>>> @@ -131,6 +131,7 @@ struct ion_heap *ion_carveout_heap_create(struct >>>> ion_platform_heap *heap_data) >>>> gen_pool_add(carveout_heap->pool, carveout_heap->base, >>>> heap_data->size, >>>> -1); >>>> carveout_heap->heap.ops = &carveout_heap_ops; >>>> + carveout_heap->heap.name = heap_data->name; >>>> carveout_heap->heap.type = ION_HEAP_TYPE_CARVEOUT; >>>> carveout_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE; >>>> >>> > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: Add chunk heap initialization
Create chunk heap of specified size and base address by adding
"ion_chunk_heap=size@start" kernel boot parameter.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion_chunk_heap.c | 40
1 file changed, 40 insertions(+)
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 159d72f..67573aa4 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
}
chunk_heap->base = heap_data->base;
chunk_heap->size = heap_data->size;
+ chunk_heap->heap.name = heap_data->name;
chunk_heap->allocated = 0;
gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_data->size, -1);
@@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
return ERR_PTR(ret);
}
+static u64 base;
+static u64 size;
+
+static int __init setup_heap(char *param)
+{
+ char *p, *pp;
+
+ size = memparse(param, &p);
+ if (param == p)
+ return -EINVAL;
+
+ if (*p == '@')
+ base = memparse(p + 1, &pp);
+ else
+ return -EINVAL;
+
+ if (p == pp)
+ return -EINVAL;
+
+ return 0;
+}
+
+__setup("ion_chunk_heap=", setup_heap);
+
+static int ion_add_chunk_heap(void)
+{
+ struct ion_heap *heap;
+ struct ion_platform_heap plat_heap = {.base = base,
+ .size = size,
+ .name = "chunk_heap",
+ .priv = (void *)PAGE_SIZE};
+ heap = ion_chunk_heap_create(&plat_heap);
+ if (heap)
+ ion_device_add_heap(heap);
+
+ return 0;
+}
+device_initcall(ion_add_chunk_heap);
+
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/25/18 10:51 PM, Laura Abbott wrote:
> On 11/11/18 11:29 AM, Alexey Skidanov wrote:
>> Create chunk heap of specified size and base address by adding
>> "ion_chunk_heap=size@start" kernel boot parameter.
>>
>> Signed-off-by: Alexey Skidanov
>> ---
>> drivers/staging/android/ion/ion_chunk_heap.c | 40
>>
>> 1 file changed, 40 insertions(+)
>>
>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
>> b/drivers/staging/android/ion/ion_chunk_heap.c
>> index 159d72f..67573aa4 100644
>> --- a/drivers/staging/android/ion/ion_chunk_heap.c
>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
>> @@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
>> ion_platform_heap *heap_data)
>> }
>> chunk_heap->base = heap_data->base;
>> chunk_heap->size = heap_data->size;
>> + chunk_heap->heap.name = heap_data->name;
>> chunk_heap->allocated = 0;
>> gen_pool_add(chunk_heap->pool, chunk_heap->base,
>> heap_data->size, -1);
>> @@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
>> ion_platform_heap *heap_data)
>> return ERR_PTR(ret);
>> }
>> +static u64 base;
>> +static u64 size;
>> +
>> +static int __init setup_heap(char *param)
>> +{
>> + char *p, *pp;
>> +
>> + size = memparse(param, &p);
>> + if (param == p)
>> + return -EINVAL;
>> +
>> + if (*p == '@')
>> + base = memparse(p + 1, &pp);
>> + else
>> + return -EINVAL;
>> +
>> + if (p == pp)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> +__setup("ion_chunk_heap=", setup_heap);
>> +
>> +static int ion_add_chunk_heap(void)
>> +{
>> + struct ion_heap *heap;
>> + struct ion_platform_heap plat_heap = {.base = base,
>> + .size = size,
>> + .name = "chunk_heap",
>> + .priv = (void *)PAGE_SIZE};
>> + heap = ion_chunk_heap_create(&plat_heap);
>> + if (heap)
>> + ion_device_add_heap(heap);
>> +
>> + return 0;
>> +}
>> +device_initcall(ion_add_chunk_heap);
>> +
>>
>
> This solves a problem but not enough of the problem.
>
> We need to be able to support more than one chunk/carveout
> heap.
This is easy to support.
This also assumes that the memory has already been
> reserved/placed and that you know the base and size to
> pass on the command line. Part of the issue with the carveout
> heaps is that we need a way to tell the kernel to reserve
> the memory early enough and then get that information to
> Ion. Hard coding memory locations tends to be buggy from
> my past experience with Ion.
memmap= kernel option marks the memory region(s) as reserved (Zone
Allocator doesn't use this memory region(s)). So the heap(s) may manage
this memory region(s).
>
> If you'd like to see about coming up with a complete solution,
> feel free to resubmit but I'm still strongly considering
> removing these heaps.
>
I will add the multiple heaps support and resubmit the patch
> Thanks,
> Laura
Thanks,
Alexey
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/25/18 11:40 PM, Laura Abbott wrote:
> On 11/25/18 1:22 PM, Alexey Skidanov wrote:
>>
>>
>> On 11/25/18 10:51 PM, Laura Abbott wrote:
>>> On 11/11/18 11:29 AM, Alexey Skidanov wrote:
>>>> Create chunk heap of specified size and base address by adding
>>>> "ion_chunk_heap=size@start" kernel boot parameter.
>>>>
>>>> Signed-off-by: Alexey Skidanov
>>>> ---
>>>> drivers/staging/android/ion/ion_chunk_heap.c | 40
>>>>
>>>> 1 file changed, 40 insertions(+)
>>>>
>>>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
>>>> b/drivers/staging/android/ion/ion_chunk_heap.c
>>>> index 159d72f..67573aa4 100644
>>>> --- a/drivers/staging/android/ion/ion_chunk_heap.c
>>>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
>>>> @@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>> ion_platform_heap *heap_data)
>>>> }
>>>> chunk_heap->base = heap_data->base;
>>>> chunk_heap->size = heap_data->size;
>>>> + chunk_heap->heap.name = heap_data->name;
>>>> chunk_heap->allocated = 0;
>>>> gen_pool_add(chunk_heap->pool, chunk_heap->base,
>>>> heap_data->size, -1);
>>>> @@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>> ion_platform_heap *heap_data)
>>>> return ERR_PTR(ret);
>>>> }
>>>> +static u64 base;
>>>> +static u64 size;
>>>> +
>>>> +static int __init setup_heap(char *param)
>>>> +{
>>>> + char *p, *pp;
>>>> +
>>>> + size = memparse(param, &p);
>>>> + if (param == p)
>>>> + return -EINVAL;
>>>> +
>>>> + if (*p == '@')
>>>> + base = memparse(p + 1, &pp);
>>>> + else
>>>> + return -EINVAL;
>>>> +
>>>> + if (p == pp)
>>>> + return -EINVAL;
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +__setup("ion_chunk_heap=", setup_heap);
>>>> +
>>>> +static int ion_add_chunk_heap(void)
>>>> +{
>>>> + struct ion_heap *heap;
>>>> + struct ion_platform_heap plat_heap = {.base = base,
>>>> + .size = size,
>>>> + .name = "chunk_heap",
>>>> + .priv = (void *)PAGE_SIZE};
>>>> + heap = ion_chunk_heap_create(&plat_heap);
>>>> + if (heap)
>>>> + ion_device_add_heap(heap);
>>>> +
>>>> + return 0;
>>>> +}
>>>> +device_initcall(ion_add_chunk_heap);
>>>> +
>>>>
>>>
>>> This solves a problem but not enough of the problem.
>>>
>>> We need to be able to support more than one chunk/carveout
>>> heap.
>> This is easy to support.
>> This also assumes that the memory has already been
>>> reserved/placed and that you know the base and size to
>>> pass on the command line. Part of the issue with the carveout
>>> heaps is that we need a way to tell the kernel to reserve
>>> the memory early enough and then get that information to
>>> Ion. Hard coding memory locations tends to be buggy from
>>> my past experience with Ion.
>> memmap= kernel option marks the memory region(s) as reserved (Zone
>> Allocator doesn't use this memory region(s)). So the heap(s) may manage
>> this memory region(s).
>
> memmap= is x86 only. I really don't like using the command line for
> specifying the base/size as it seems likely to conflict with platforms
> that rely on devicetree for reserving memory regions.
>
> Thanks,
> Laura
>
I see ... So probably the better way is the one similar to this
https://elixir.bootlin.com/linux/latest/source/kernel/dma/contiguous.c#L245
?
Thanks,
Alexey
>>>
>>> If you'd like to see about coming up with a complete solution,
>>> feel free to resubmit but I'm still strongly considering
>>> removing these heaps.
>>>
>> I will add the multiple heaps support and resubmit the patch
>>> Thanks,
>>> Laura
>> Thanks,
>> Alexey
>>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/26/18 6:39 PM, Laura Abbott wrote:
> On 11/25/18 2:02 PM, Alexey Skidanov wrote:
>>
>>
>> On 11/25/18 11:40 PM, Laura Abbott wrote:
>>> On 11/25/18 1:22 PM, Alexey Skidanov wrote:
>>>>
>>>>
>>>> On 11/25/18 10:51 PM, Laura Abbott wrote:
>>>>> On 11/11/18 11:29 AM, Alexey Skidanov wrote:
>>>>>> Create chunk heap of specified size and base address by adding
>>>>>> "ion_chunk_heap=size@start" kernel boot parameter.
>>>>>>
>>>>>> Signed-off-by: Alexey Skidanov
>>>>>> ---
>>>>>> drivers/staging/android/ion/ion_chunk_heap.c | 40
>>>>>>
>>>>>> 1 file changed, 40 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>> b/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>> index 159d72f..67573aa4 100644
>>>>>> --- a/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>> @@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>>>> ion_platform_heap *heap_data)
>>>>>> }
>>>>>> chunk_heap->base = heap_data->base;
>>>>>> chunk_heap->size = heap_data->size;
>>>>>> + chunk_heap->heap.name = heap_data->name;
>>>>>> chunk_heap->allocated = 0;
>>>>>> gen_pool_add(chunk_heap->pool, chunk_heap->base,
>>>>>> heap_data->size, -1);
>>>>>> @@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>>>> ion_platform_heap *heap_data)
>>>>>> return ERR_PTR(ret);
>>>>>> }
>>>>>> +static u64 base;
>>>>>> +static u64 size;
>>>>>> +
>>>>>> +static int __init setup_heap(char *param)
>>>>>> +{
>>>>>> + char *p, *pp;
>>>>>> +
>>>>>> + size = memparse(param, &p);
>>>>>> + if (param == p)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + if (*p == '@')
>>>>>> + base = memparse(p + 1, &pp);
>>>>>> + else
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + if (p == pp)
>>>>>> + return -EINVAL;
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> +__setup("ion_chunk_heap=", setup_heap);
>>>>>> +
>>>>>> +static int ion_add_chunk_heap(void)
>>>>>> +{
>>>>>> + struct ion_heap *heap;
>>>>>> + struct ion_platform_heap plat_heap = {.base = base,
>>>>>> + .size = size,
>>>>>> + .name = "chunk_heap",
>>>>>> + .priv = (void *)PAGE_SIZE};
>>>>>> + heap = ion_chunk_heap_create(&plat_heap);
>>>>>> + if (heap)
>>>>>> + ion_device_add_heap(heap);
>>>>>> +
>>>>>> + return 0;
>>>>>> +}
>>>>>> +device_initcall(ion_add_chunk_heap);
>>>>>> +
>>>>>>
>>>>>
>>>>> This solves a problem but not enough of the problem.
>>>>>
>>>>> We need to be able to support more than one chunk/carveout
>>>>> heap.
>>>> This is easy to support.
>>>> This also assumes that the memory has already been
>>>>> reserved/placed and that you know the base and size to
>>>>> pass on the command line. Part of the issue with the carveout
>>>>> heaps is that we need a way to tell the kernel to reserve
>>>>> the memory early enough and then get that information to
>>>>> Ion. Hard coding memory locations tends to be buggy from
>>>>> my past experience with Ion.
>>>> memmap= kernel option marks the memory region(s) as reserved (Zone
>>>> Allocator doesn't use this memory region(s)). So the heap(s) may manage
>>>> this memory region(s).
>>>
>>> memmap= is x86 only. I rea
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/27/18 9:20 PM, Laura Abbott wrote:
> On 11/26/18 10:43 AM, Alexey Skidanov wrote:
>>
>>
>> On 11/26/18 6:39 PM, Laura Abbott wrote:
>>> On 11/25/18 2:02 PM, Alexey Skidanov wrote:
>>>>
>>>>
>>>> On 11/25/18 11:40 PM, Laura Abbott wrote:
>>>>> On 11/25/18 1:22 PM, Alexey Skidanov wrote:
>>>>>>
>>>>>>
>>>>>> On 11/25/18 10:51 PM, Laura Abbott wrote:
>>>>>>> On 11/11/18 11:29 AM, Alexey Skidanov wrote:
>>>>>>>> Create chunk heap of specified size and base address by adding
>>>>>>>> "ion_chunk_heap=size@start" kernel boot parameter.
>>>>>>>>
>>>>>>>> Signed-off-by: Alexey Skidanov
>>>>>>>> ---
>>>>>>>> drivers/staging/android/ion/ion_chunk_heap.c | 40
>>>>>>>>
>>>>>>>> 1 file changed, 40 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>> b/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>> index 159d72f..67573aa4 100644
>>>>>>>> --- a/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>> @@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>>>>>> ion_platform_heap *heap_data)
>>>>>>>> }
>>>>>>>> chunk_heap->base = heap_data->base;
>>>>>>>> chunk_heap->size = heap_data->size;
>>>>>>>> + chunk_heap->heap.name = heap_data->name;
>>>>>>>> chunk_heap->allocated = 0;
>>>>>>>> gen_pool_add(chunk_heap->pool, chunk_heap->base,
>>>>>>>> heap_data->size, -1);
>>>>>>>> @@ -151,3 +152,42 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>>>>>> ion_platform_heap *heap_data)
>>>>>>>> return ERR_PTR(ret);
>>>>>>>> }
>>>>>>>> +static u64 base;
>>>>>>>> +static u64 size;
>>>>>>>> +
>>>>>>>> +static int __init setup_heap(char *param)
>>>>>>>> +{
>>>>>>>> + char *p, *pp;
>>>>>>>> +
>>>>>>>> + size = memparse(param, &p);
>>>>>>>> + if (param == p)
>>>>>>>> + return -EINVAL;
>>>>>>>> +
>>>>>>>> + if (*p == '@')
>>>>>>>> + base = memparse(p + 1, &pp);
>>>>>>>> + else
>>>>>>>> + return -EINVAL;
>>>>>>>> +
>>>>>>>> + if (p == pp)
>>>>>>>> + return -EINVAL;
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +
>>>>>>>> +__setup("ion_chunk_heap=", setup_heap);
>>>>>>>> +
>>>>>>>> +static int ion_add_chunk_heap(void)
>>>>>>>> +{
>>>>>>>> + struct ion_heap *heap;
>>>>>>>> + struct ion_platform_heap plat_heap = {.base = base,
>>>>>>>> + .size = size,
>>>>>>>> + .name = "chunk_heap",
>>>>>>>> + .priv = (void *)PAGE_SIZE};
>>>>>>>> + heap = ion_chunk_heap_create(&plat_heap);
>>>>>>>> + if (heap)
>>>>>>>> + ion_device_add_heap(heap);
>>>>>>>> +
>>>>>>>> + return 0;
>>>>>>>> +}
>>>>>>>> +device_initcall(ion_add_chunk_heap);
>>>>>>>> +
>>>>>>>>
>>>>>>>
>>>>>>> This solves a problem but not enough of the problem.
>>>>>>>
>>>>>>> We need to be able to support more than one chunk/carveout
>>>>>>> heap.
>>>>>> This is easy
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/29/18 3:30 AM, Laura Abbott wrote:
> On 11/27/18 12:07 PM, Alexey Skidanov wrote:
>>
>>
>> On 11/27/18 9:20 PM, Laura Abbott wrote:
>>> On 11/26/18 10:43 AM, Alexey Skidanov wrote:
>>>>
>>>>
>>>> On 11/26/18 6:39 PM, Laura Abbott wrote:
>>>>> On 11/25/18 2:02 PM, Alexey Skidanov wrote:
>>>>>>
>>>>>>
>>>>>> On 11/25/18 11:40 PM, Laura Abbott wrote:
>>>>>>> On 11/25/18 1:22 PM, Alexey Skidanov wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 11/25/18 10:51 PM, Laura Abbott wrote:
>>>>>>>>> On 11/11/18 11:29 AM, Alexey Skidanov wrote:
>>>>>>>>>> Create chunk heap of specified size and base address by adding
>>>>>>>>>> "ion_chunk_heap=size@start" kernel boot parameter.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Alexey Skidanov
>>>>>>>>>> ---
>>>>>>>>>> drivers/staging/android/ion/ion_chunk_heap.c | 40
>>>>>>>>>>
>>>>>>>>>> 1 file changed, 40 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>>>> b/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>>>> index 159d72f..67573aa4 100644
>>>>>>>>>> --- a/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>>>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>>>> @@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>>>>>>>> ion_platform_heap *heap_data)
>>>>>>>>>> }
>>>>>>>>>> chunk_heap->base = heap_data->base;
>>>>>>>>>> chunk_heap->size = heap_data->size;
>>>>>>>>>> + chunk_heap->heap.name = heap_data->name;
>>>>>>>>>> chunk_heap->allocated = 0;
>>>>>>>>>> gen_pool_add(chunk_heap->pool, chunk_heap->base,
>>>>>>>>>> heap_data->size, -1);
>>>>>>>>>> @@ -151,3 +152,42 @@ struct ion_heap
>>>>>>>>>> *ion_chunk_heap_create(struct
>>>>>>>>>> ion_platform_heap *heap_data)
>>>>>>>>>> return ERR_PTR(ret);
>>>>>>>>>> }
>>>>>>>>>> +static u64 base;
>>>>>>>>>> +static u64 size;
>>>>>>>>>> +
>>>>>>>>>> +static int __init setup_heap(char *param)
>>>>>>>>>> +{
>>>>>>>>>> + char *p, *pp;
>>>>>>>>>> +
>>>>>>>>>> + size = memparse(param, &p);
>>>>>>>>>> + if (param == p)
>>>>>>>>>> + return -EINVAL;
>>>>>>>>>> +
>>>>>>>>>> + if (*p == '@')
>>>>>>>>>> + base = memparse(p + 1, &pp);
>>>>>>>>>> + else
>>>>>>>>>> + return -EINVAL;
>>>>>>>>>> +
>>>>>>>>>> + if (p == pp)
>>>>>>>>>> + return -EINVAL;
>>>>>>>>>> +
>>>>>>>>>> + return 0;
>>>>>>>>>> +}
>>>>>>>>>> +
>>>>>>>>>> +__setup("ion_chunk_heap=", setup_heap);
>>>>>>>>>> +
>>>>>>>>>> +static int ion_add_chunk_heap(void)
>>>>>>>>>> +{
>>>>>>>>>> + struct ion_heap *heap;
>>>>>>>>>> + struct ion_platform_heap plat_heap = {.base = base,
>>>>>>>>>> + .size = size,
>>>>>>>>>> + .name = "chunk_heap",
>>>>>>>>>> + .priv = (void *)PAGE_SIZE};
>>>>>>>>&
Re: [PATCH] staging: android: ion: Add chunk heap initialization
On 11/29/18 8:25 AM, Alexey Skidanov wrote:
>
>
> On 11/29/18 3:30 AM, Laura Abbott wrote:
>> On 11/27/18 12:07 PM, Alexey Skidanov wrote:
>>>
>>>
>>> On 11/27/18 9:20 PM, Laura Abbott wrote:
>>>> On 11/26/18 10:43 AM, Alexey Skidanov wrote:
>>>>>
>>>>>
>>>>> On 11/26/18 6:39 PM, Laura Abbott wrote:
>>>>>> On 11/25/18 2:02 PM, Alexey Skidanov wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 11/25/18 11:40 PM, Laura Abbott wrote:
>>>>>>>> On 11/25/18 1:22 PM, Alexey Skidanov wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 11/25/18 10:51 PM, Laura Abbott wrote:
>>>>>>>>>> On 11/11/18 11:29 AM, Alexey Skidanov wrote:
>>>>>>>>>>> Create chunk heap of specified size and base address by adding
>>>>>>>>>>> "ion_chunk_heap=size@start" kernel boot parameter.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Alexey Skidanov
>>>>>>>>>>> ---
>>>>>>>>>>> drivers/staging/android/ion/ion_chunk_heap.c | 40
>>>>>>>>>>>
>>>>>>>>>>> 1 file changed, 40 insertions(+)
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>>>>> b/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>>>>> index 159d72f..67573aa4 100644
>>>>>>>>>>> --- a/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>>>>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
>>>>>>>>>>> @@ -135,6 +135,7 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>>>>>>>>> ion_platform_heap *heap_data)
>>>>>>>>>>> }
>>>>>>>>>>> chunk_heap->base = heap_data->base;
>>>>>>>>>>> chunk_heap->size = heap_data->size;
>>>>>>>>>>> + chunk_heap->heap.name = heap_data->name;
>>>>>>>>>>> chunk_heap->allocated = 0;
>>>>>>>>>>> gen_pool_add(chunk_heap->pool, chunk_heap->base,
>>>>>>>>>>> heap_data->size, -1);
>>>>>>>>>>> @@ -151,3 +152,42 @@ struct ion_heap
>>>>>>>>>>> *ion_chunk_heap_create(struct
>>>>>>>>>>> ion_platform_heap *heap_data)
>>>>>>>>>>> return ERR_PTR(ret);
>>>>>>>>>>> }
>>>>>>>>>>> +static u64 base;
>>>>>>>>>>> +static u64 size;
>>>>>>>>>>> +
>>>>>>>>>>> +static int __init setup_heap(char *param)
>>>>>>>>>>> +{
>>>>>>>>>>> + char *p, *pp;
>>>>>>>>>>> +
>>>>>>>>>>> + size = memparse(param, &p);
>>>>>>>>>>> + if (param == p)
>>>>>>>>>>> + return -EINVAL;
>>>>>>>>>>> +
>>>>>>>>>>> + if (*p == '@')
>>>>>>>>>>> + base = memparse(p + 1, &pp);
>>>>>>>>>>> + else
>>>>>>>>>>> + return -EINVAL;
>>>>>>>>>>> +
>>>>>>>>>>> + if (p == pp)
>>>>>>>>>>> + return -EINVAL;
>>>>>>>>>>> +
>>>>>>>>>>> + return 0;
>>>>>>>>>>> +}
>>>>>>>>>>> +
>>>>>>>>>>> +__setup("ion_chunk_heap=", setup_heap);
>>>>>>>>>>> +
>>>>>>>>>>> +static int ion_add_chunk_heap(void)
>>>>>>>>>>> +{
>>>>>>>>>>> + struct ion_heap *heap;
>>>>>>>>>>> + struct ion
Re: [PATCH v3] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap
On 12/16/18 7:20 AM, Liam Mark wrote: > On Tue, 6 Feb 2018, Alexey Skidanov wrote: > >> >> >> On 02/07/2018 01:56 AM, Laura Abbott wrote: >>> On 01/31/2018 10:10 PM, Alexey Skidanov wrote: >>>> >>>> On 01/31/2018 03:00 PM, Greg KH wrote: >>>>> On Wed, Jan 31, 2018 at 02:03:42PM +0200, Alexey Skidanov wrote: >>>>>> Any driver may access shared buffers, created by ion, using >>>>>> dma_buf_vmap and >>>>>> dma_buf_vunmap dma-buf API that maps/unmaps previosuly allocated >>>>>> buffers into >>>>>> the kernel virtual address space. The implementation of these API is >>>>>> missing in >>>>>> the current ion implementation. >>>>>> >>>>>> Signed-off-by: Alexey Skidanov >>>>>> --- >>>>> >>>>> No review from any other Intel developers? :( >>>> Will add. >>>>> >>>>> Anyway, what in-tree driver needs access to these functions? >>>> I'm not sure that there are the in-tree drivers using these functions >>>> and ion as> buffer exporter because they are not implemented in ion :) >>>> But there are some in-tre> drivers using these APIs (gpu drivers) with >>>> other buffer exporters. >>> >>> It's still not clear why you need to implement these APIs. >> How the importing kernel module may access the content of the buffer? :) >> With the current ion implementation it's only possible by dma_buf_kmap, >> mapping one page at a time. For pretty large buffers, it might have some >> performance impact. >> (Probably, the page by page mapping is the only way to access large >> buffers on 32 bit systems, where the vmalloc range is very small. By the >> way, the current ion dma_map_kmap doesn't really map only 1 page at a >> time - it uses the result of vmap() that might fail on 32 bit systems.) >> >>> Are you planning to use Ion with GPU drivers? I'm especially >>> interested in this if you have a non-Android use case. >> Yes, my use case is the non-Android one. But not with GPU drivers. >>> >>> Thanks, >>> Laura >> >> Thanks, >> Alexey > > I was wondering if we could re-open the discussion on adding support to > ION for dma_buf_vmap. > It seems like the patch was not taken as the reviewers wanted more > evidence of an upstream use case. > > Here would be my upstream usage argument for including dma_buf_vmap > support in ION. > > Currently all calls to ion_dma_buf_begin_cpu_access result in the creation > of a kernel mapping for the buffer, unfortunately the resulting call to > alloc_vmap_area can be quite expensive and this has caused a performance > regression for certain clients when they have moved to the new version of > ION. > > The kernel mapping is not actually needed in ion_dma_buf_begin_cpu_access, > and generally isn't needed by clients. So if we remove the creation of the > kernel mapping in ion_dma_buf_begin_cpu_access and only create it when > needed we can speed up the calls to ion_dma_buf_begin_cpu_access. > > An additional benefit of removing the creation of kernel mappings from > ion_dma_buf_begin_cpu_access is that it makes the ION code more secure. > Currently a malicious client could call the DMA_BUF_IOCTL_SYNC IOCTL with > flags DMA_BUF_SYNC_END multiple times to cause the ION buffer kmap_cnt to > go negative which could lead to undesired behavior. > > One disadvantage of the above change is that a kernel mapping is not > already created when a client calls dma_buf_kmap. So the following > dma_buf_kmap contract can't be satisfied. > > /** > * dma_buf_kmap - Map a page of the buffer object into kernel address > space. The > * same restrictions as for kmap and friends apply. > * @dmabuf:[in]buffer to map page from. > * @page_num: [in]page in PAGE_SIZE units to map. > * > * This call must always succeed, any necessary preparations that might > fail > * need to be done in begin_cpu_access. > */ > > But hopefully we can work around this by moving clients to dma_buf_vmap. I think the problem is with the contract. We can't ensure that the call is always succeeds regardless the implementation - any mapping might fail. Probably this is why *all* clients of dma_buf_kmap() check the return value (so it's safe to return NULL in case of failure). I would suggest to fix the contract and to keep the dma_buf_kmap() support in ION. > > Based on discussions at LPC here is what was proposed: > - #1 Add support to ION for dma_buf_vmap and dma_buf_vunmap > - #2 Move any existing ION clients over from using dma_buf_kmap to > dma_buf_vmap > - #3 Deprecate support in ION for dma_buf_kmap? > - #4 Make the above performance optimization to > ion_dma_buf_begin_cpu_access to remove the creation of a kernel mapping. > > Thoughts? > > Liam > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > Thanks, Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: Add chunk heaps instantiation
Chunk heap instantiation should be supported for device tree platforms and
non device tree platforms. For device tree platforms, it's a platform
specific code responsibility to retrieve the heap configuration data
and to call the appropriate API for heap creation. For non device tree
platforms, there is no defined way to create the heaps.
This patch provides the way of chunk heaps creation using
"ion_chunk_heap=name:size@start" kernel boot parameter.
Link:
http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128495.html
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion_chunk_heap.c | 96 +---
include/linux/ion.h | 18 ++
2 files changed, 105 insertions(+), 9 deletions(-)
create mode 100644 include/linux/ion.h
diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
b/drivers/staging/android/ion/ion_chunk_heap.c
index 102c093..1a8e3ad 100644
--- a/drivers/staging/android/ion/ion_chunk_heap.c
+++ b/drivers/staging/android/ion/ion_chunk_heap.c
@@ -21,8 +21,12 @@
#include
#include
#include
+#include
#include "ion.h"
+static struct ion_chunk_heap_cfg chunk_heap_cfg[MAX_NUM_OF_CHUNK_HEAPS];
+static unsigned int num_of_req_chunk_heaps;
+
struct ion_chunk_heap {
struct ion_heap heap;
struct gen_pool *pool;
@@ -117,15 +121,15 @@ static struct ion_heap_ops chunk_heap_ops = {
.unmap_kernel = ion_heap_unmap_kernel,
};
-struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap *heap_data)
+static struct ion_heap *ion_chunk_heap_create(struct ion_chunk_heap_cfg
*heap_cfg)
{
struct ion_chunk_heap *chunk_heap;
int ret;
struct page *page;
size_t size;
- page = pfn_to_page(PFN_DOWN(heap_data->base));
- size = heap_data->size;
+ page = pfn_to_page(PFN_DOWN(heap_cfg->base));
+ size = heap_cfg->size;
ret = ion_heap_pages_zero(page, size, pgprot_writecombine(PAGE_KERNEL));
if (ret)
@@ -135,23 +139,27 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
if (!chunk_heap)
return ERR_PTR(-ENOMEM);
- chunk_heap->chunk_size = (unsigned long)heap_data->priv;
+ chunk_heap->chunk_size = heap_cfg->chunk_size;
chunk_heap->pool = gen_pool_create(get_order(chunk_heap->chunk_size) +
PAGE_SHIFT, -1);
if (!chunk_heap->pool) {
ret = -ENOMEM;
goto error_gen_pool_create;
}
- chunk_heap->base = heap_data->base;
- chunk_heap->size = heap_data->size;
+ chunk_heap->base = heap_cfg->base;
+ chunk_heap->size = heap_cfg->size;
+ chunk_heap->heap.name = heap_cfg->heap_name;
chunk_heap->allocated = 0;
- gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_data->size, -1);
+ gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_cfg->size, -1);
chunk_heap->heap.ops = &chunk_heap_ops;
chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK;
chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE;
- pr_debug("%s: base %pa size %zu\n", __func__,
-&chunk_heap->base, heap_data->size);
+
+ pr_info("%s: name %s base %pa size %zu\n", __func__,
+ heap_cfg->heap_name,
+ &heap_cfg->base,
+ heap_cfg->size);
return &chunk_heap->heap;
@@ -160,3 +168,73 @@ struct ion_heap *ion_chunk_heap_create(struct
ion_platform_heap *heap_data)
return ERR_PTR(ret);
}
+static int __init setup_heap(char *param)
+{
+ char *at_sign, *coma, *colon;
+ size_t size_to_copy;
+ struct ion_chunk_heap_cfg *cfg;
+
+ do {
+ cfg = &chunk_heap_cfg[num_of_req_chunk_heaps];
+
+ /* heap name */
+ colon = strchr(param, ':');
+ if (!colon)
+ return -EINVAL;
+
+ size_to_copy = min_t(size_t, MAX_CHUNK_HEAP_NAME_SIZE - 1,
+(colon - param));
+ strncpy(cfg->heap_name, param, size_to_copy);
+ cfg->heap_name[size_to_copy] = '\0';
+
+ /* heap size */
+ cfg->size = memparse((colon + 1), &at_sign);
+ if ((colon + 1) == at_sign)
+ return -EINVAL;
+
+ /* heap base addr */
+ if (*at_sign == '@')
+ cfg->base = memparse(at_sign + 1, &coma);
+ else
+ return -EINVAL;
+
+ if (at_sign == coma)
+ return -EINVAL;
+
+ /* Chunk size */
+ cfg->chunk_size = PAGE_SIZE;
+
+
Re: [PATCH v3] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap
On 12/17/18 20:42, Liam Mark wrote: > On Sun, 16 Dec 2018, Alexey Skidanov wrote: > >> >> >> On 12/16/18 7:20 AM, Liam Mark wrote: >>> On Tue, 6 Feb 2018, Alexey Skidanov wrote: >>> >>>> >>>> >>>> On 02/07/2018 01:56 AM, Laura Abbott wrote: >>>>> On 01/31/2018 10:10 PM, Alexey Skidanov wrote: >>>>>> >>>>>> On 01/31/2018 03:00 PM, Greg KH wrote: >>>>>>> On Wed, Jan 31, 2018 at 02:03:42PM +0200, Alexey Skidanov wrote: >>>>>>>> Any driver may access shared buffers, created by ion, using >>>>>>>> dma_buf_vmap and >>>>>>>> dma_buf_vunmap dma-buf API that maps/unmaps previosuly allocated >>>>>>>> buffers into >>>>>>>> the kernel virtual address space. The implementation of these API is >>>>>>>> missing in >>>>>>>> the current ion implementation. >>>>>>>> >>>>>>>> Signed-off-by: Alexey Skidanov >>>>>>>> --- >>>>>>> >>>>>>> No review from any other Intel developers? :( >>>>>> Will add. >>>>>>> >>>>>>> Anyway, what in-tree driver needs access to these functions? >>>>>> I'm not sure that there are the in-tree drivers using these functions >>>>>> and ion as> buffer exporter because they are not implemented in ion :) >>>>>> But there are some in-tre> drivers using these APIs (gpu drivers) with >>>>>> other buffer exporters. >>>>> >>>>> It's still not clear why you need to implement these APIs. >>>> How the importing kernel module may access the content of the buffer? :) >>>> With the current ion implementation it's only possible by dma_buf_kmap, >>>> mapping one page at a time. For pretty large buffers, it might have some >>>> performance impact. >>>> (Probably, the page by page mapping is the only way to access large >>>> buffers on 32 bit systems, where the vmalloc range is very small. By the >>>> way, the current ion dma_map_kmap doesn't really map only 1 page at a >>>> time - it uses the result of vmap() that might fail on 32 bit systems.) >>>> >>>>> Are you planning to use Ion with GPU drivers? I'm especially >>>>> interested in this if you have a non-Android use case. >>>> Yes, my use case is the non-Android one. But not with GPU drivers. >>>>> >>>>> Thanks, >>>>> Laura >>>> >>>> Thanks, >>>> Alexey >>> >>> I was wondering if we could re-open the discussion on adding support to >>> ION for dma_buf_vmap. >>> It seems like the patch was not taken as the reviewers wanted more >>> evidence of an upstream use case. >>> >>> Here would be my upstream usage argument for including dma_buf_vmap >>> support in ION. >>> >>> Currently all calls to ion_dma_buf_begin_cpu_access result in the creation >>> of a kernel mapping for the buffer, unfortunately the resulting call to >>> alloc_vmap_area can be quite expensive and this has caused a performance >>> regression for certain clients when they have moved to the new version of >>> ION. >>> >>> The kernel mapping is not actually needed in ion_dma_buf_begin_cpu_access, >>> and generally isn't needed by clients. So if we remove the creation of the >>> kernel mapping in ion_dma_buf_begin_cpu_access and only create it when >>> needed we can speed up the calls to ion_dma_buf_begin_cpu_access. >>> >>> An additional benefit of removing the creation of kernel mappings from >>> ion_dma_buf_begin_cpu_access is that it makes the ION code more secure. >>> Currently a malicious client could call the DMA_BUF_IOCTL_SYNC IOCTL with >>> flags DMA_BUF_SYNC_END multiple times to cause the ION buffer kmap_cnt to >>> go negative which could lead to undesired behavior. >>> >>> One disadvantage of the above change is that a kernel mapping is not >>> already created when a client calls dma_buf_kmap. So the following >>> dma_buf_kmap contract can't be satisfied. >>> >>> /** >>> * dma_buf_kmap - Map a page of the buffer object into kernel address >>> space. The >>> * same restrictions as for kmap and friends apply. >>> * @d
Re: [PATCH] staging: android: ion: Add chunk heaps instantiation
On 12/20/18 10:36 PM, Laura Abbott wrote:
> On 12/16/18 2:46 AM, Alexey Skidanov wrote:
>> Chunk heap instantiation should be supported for device tree platforms
>> and
>> non device tree platforms. For device tree platforms, it's a platform
>> specific code responsibility to retrieve the heap configuration data
>> and to call the appropriate API for heap creation. For non device tree
>> platforms, there is no defined way to create the heaps.
>>
>> This patch provides the way of chunk heaps creation using
>> "ion_chunk_heap=name:size@start" kernel boot parameter.
>>
>
> I've been thinking about this and I think it works but
> I'm still kind of torn about not having devicetree bindings.
> It doesn't _preclude_ devicetree bindings but I'd hate to
> merge this and then end up with something incompatible.
> I do want to support non-Android use cases too.
Sorry, what do you mean by non-Android cases?
>
> I'm also curious about the value of this heap with just PAGE_SIZE.
> The original purpose of the chunk heap was a carveout where
> you could easily get allocations large than PAGE_SIZE to
> reduce TLB pressure. Do you have another use case for the
> chunk heap?
It need to be fixed ... Obviously ... The minimum allocation size should
be parametrized
>
> Sumit, do you have any thoughts?
>
> Thanks,
> Laura
>
>> Link:
>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128495.html
>>
>> Signed-off-by: Alexey Skidanov
>> ---
>> drivers/staging/android/ion/ion_chunk_heap.c | 96
>> +---
>> include/linux/ion.h | 18 ++
>> 2 files changed, 105 insertions(+), 9 deletions(-)
>> create mode 100644 include/linux/ion.h
>>
>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
>> b/drivers/staging/android/ion/ion_chunk_heap.c
>> index 102c093..1a8e3ad 100644
>> --- a/drivers/staging/android/ion/ion_chunk_heap.c
>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
>> @@ -21,8 +21,12 @@
>> #include
>> #include
>> #include
>> +#include
>> #include "ion.h"
>> +static struct ion_chunk_heap_cfg
>> chunk_heap_cfg[MAX_NUM_OF_CHUNK_HEAPS];
>> +static unsigned int num_of_req_chunk_heaps;
>> +
>> struct ion_chunk_heap {
>> struct ion_heap heap;
>> struct gen_pool *pool;
>> @@ -117,15 +121,15 @@ static struct ion_heap_ops chunk_heap_ops = {
>> .unmap_kernel = ion_heap_unmap_kernel,
>> };
>> -struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap
>> *heap_data)
>> +static struct ion_heap *ion_chunk_heap_create(struct
>> ion_chunk_heap_cfg *heap_cfg)
>> {
>> struct ion_chunk_heap *chunk_heap;
>> int ret;
>> struct page *page;
>> size_t size;
>> - page = pfn_to_page(PFN_DOWN(heap_data->base));
>> - size = heap_data->size;
>> + page = pfn_to_page(PFN_DOWN(heap_cfg->base));
>> + size = heap_cfg->size;
>> ret = ion_heap_pages_zero(page, size,
>> pgprot_writecombine(PAGE_KERNEL));
>> if (ret)
>> @@ -135,23 +139,27 @@ struct ion_heap *ion_chunk_heap_create(struct
>> ion_platform_heap *heap_data)
>> if (!chunk_heap)
>> return ERR_PTR(-ENOMEM);
>> - chunk_heap->chunk_size = (unsigned long)heap_data->priv;
>> + chunk_heap->chunk_size = heap_cfg->chunk_size;
>> chunk_heap->pool =
>> gen_pool_create(get_order(chunk_heap->chunk_size) +
>> PAGE_SHIFT, -1);
>> if (!chunk_heap->pool) {
>> ret = -ENOMEM;
>> goto error_gen_pool_create;
>> }
>> - chunk_heap->base = heap_data->base;
>> - chunk_heap->size = heap_data->size;
>> + chunk_heap->base = heap_cfg->base;
>> + chunk_heap->size = heap_cfg->size;
>> + chunk_heap->heap.name = heap_cfg->heap_name;
>> chunk_heap->allocated = 0;
>> - gen_pool_add(chunk_heap->pool, chunk_heap->base,
>> heap_data->size, -1);
>> + gen_pool_add(chunk_heap->pool, chunk_heap->base, heap_cfg->size,
>> -1);
>> chunk_heap->heap.ops = &chunk_heap_ops;
>> chunk_heap->heap.type = ION_HEAP_TYPE_CHUNK;
>> chunk_heap->heap.flags = ION_HEAP_FLAG_DEFER_FREE;
>> - pr_debug("%s: base %pa size %zu\n", __func__,
>> - &chunk_heap->base, heap_data->size);
>>
Re: [PATCH] staging: android: ion: Add chunk heaps instantiation
On 1/3/19 12:37 AM, Laura Abbott wrote:
> On 12/20/18 1:29 PM, Alexey Skidanov wrote:
>>
>>
>> On 12/20/18 10:36 PM, Laura Abbott wrote:
>>> On 12/16/18 2:46 AM, Alexey Skidanov wrote:
>>>> Chunk heap instantiation should be supported for device tree platforms
>>>> and
>>>> non device tree platforms. For device tree platforms, it's a platform
>>>> specific code responsibility to retrieve the heap configuration data
>>>> and to call the appropriate API for heap creation. For non device tree
>>>> platforms, there is no defined way to create the heaps.
>>>>
>>>> This patch provides the way of chunk heaps creation using
>>>> "ion_chunk_heap=name:size@start" kernel boot parameter.
>>>>
>>>
>>> I've been thinking about this and I think it works but
>>> I'm still kind of torn about not having devicetree bindings.
>>> It doesn't _preclude_ devicetree bindings but I'd hate to
>>> merge this and then end up with something incompatible.
>>> I do want to support non-Android use cases too.
>> Sorry, what do you mean by non-Android cases?
>
> Any user of Ion that isn't tied to Android. This includes other
> userspace frameworks as well as non-devicetree targets.
>
Sorry, don't follow you ... I tested the patch on Ubuntu machine - so
the non-Android and non-devicetree case is obviously supported :)
>>>
>>> I'm also curious about the value of this heap with just PAGE_SIZE.
>>> The original purpose of the chunk heap was a carveout where
>>> you could easily get allocations large than PAGE_SIZE to
>>> reduce TLB pressure. Do you have another use case for the
>>> chunk heap?
>> It need to be fixed ... Obviously ... The minimum allocation size should
>> be parametrized
>>>
>>> Sumit, do you have any thoughts?
>>>
>>> Thanks,
>>> Laura
>>>
>>>> Link:
>>>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-November/128495.html
>>>>
>>>>
>>>> Signed-off-by: Alexey Skidanov
>>>> ---
>>>> drivers/staging/android/ion/ion_chunk_heap.c | 96
>>>> +---
>>>> include/linux/ion.h | 18 ++
>>>> 2 files changed, 105 insertions(+), 9 deletions(-)
>>>> create mode 100644 include/linux/ion.h
>>>>
>>>> diff --git a/drivers/staging/android/ion/ion_chunk_heap.c
>>>> b/drivers/staging/android/ion/ion_chunk_heap.c
>>>> index 102c093..1a8e3ad 100644
>>>> --- a/drivers/staging/android/ion/ion_chunk_heap.c
>>>> +++ b/drivers/staging/android/ion/ion_chunk_heap.c
>>>> @@ -21,8 +21,12 @@
>>>> #include
>>>> #include
>>>> #include
>>>> +#include
>>>> #include "ion.h"
>>>> +static struct ion_chunk_heap_cfg
>>>> chunk_heap_cfg[MAX_NUM_OF_CHUNK_HEAPS];
>>>> +static unsigned int num_of_req_chunk_heaps;
>>>> +
>>>> struct ion_chunk_heap {
>>>> struct ion_heap heap;
>>>> struct gen_pool *pool;
>>>> @@ -117,15 +121,15 @@ static struct ion_heap_ops chunk_heap_ops = {
>>>> .unmap_kernel = ion_heap_unmap_kernel,
>>>> };
>>>> -struct ion_heap *ion_chunk_heap_create(struct ion_platform_heap
>>>> *heap_data)
>>>> +static struct ion_heap *ion_chunk_heap_create(struct
>>>> ion_chunk_heap_cfg *heap_cfg)
>>>> {
>>>> struct ion_chunk_heap *chunk_heap;
>>>> int ret;
>>>> struct page *page;
>>>> size_t size;
>>>> - page = pfn_to_page(PFN_DOWN(heap_data->base));
>>>> - size = heap_data->size;
>>>> + page = pfn_to_page(PFN_DOWN(heap_cfg->base));
>>>> + size = heap_cfg->size;
>>>> ret = ion_heap_pages_zero(page, size,
>>>> pgprot_writecombine(PAGE_KERNEL));
>>>> if (ret)
>>>> @@ -135,23 +139,27 @@ struct ion_heap *ion_chunk_heap_create(struct
>>>> ion_platform_heap *heap_data)
>>>> if (!chunk_heap)
>>>> return ERR_PTR(-ENOMEM);
>>>> - chunk_heap->chunk_size = (unsigned long)heap_data->priv;
>>>> + chunk_heap->chunk_size = heap_cfg->chunk_size;
>>>> chunk_heap->p
RE: [PATCH] staging: android: ion: Add chunk heaps instantiation
> -Original Message- > From: Laura Abbott [mailto:labb...@redhat.com] > Sent: Friday, January 04, 2019 03:58 > To: Skidanov, Alexey ; gre...@linuxfoundation.org; > Sumit > Semwal > Cc: de...@driverdev.osuosl.org > Subject: Re: [PATCH] staging: android: ion: Add chunk heaps instantiation > > On 1/2/19 10:28 PM, Alexey Skidanov wrote: > > > > > > On 1/3/19 12:37 AM, Laura Abbott wrote: > >> On 12/20/18 1:29 PM, Alexey Skidanov wrote: > >>> > >>> > >>> On 12/20/18 10:36 PM, Laura Abbott wrote: > >>>> On 12/16/18 2:46 AM, Alexey Skidanov wrote: > >>>>> Chunk heap instantiation should be supported for device tree platforms > >>>>> and > >>>>> non device tree platforms. For device tree platforms, it's a platform > >>>>> specific code responsibility to retrieve the heap configuration data > >>>>> and to call the appropriate API for heap creation. For non device tree > >>>>> platforms, there is no defined way to create the heaps. > >>>>> > >>>>> This patch provides the way of chunk heaps creation using > >>>>> "ion_chunk_heap=name:size@start" kernel boot parameter. > >>>>> > >>>> > >>>> I've been thinking about this and I think it works but > >>>> I'm still kind of torn about not having devicetree bindings. > >>>> It doesn't _preclude_ devicetree bindings but I'd hate to > >>>> merge this and then end up with something incompatible. > >>>> I do want to support non-Android use cases too. > >>> Sorry, what do you mean by non-Android cases? > >> > >> Any user of Ion that isn't tied to Android. This includes other > >> userspace frameworks as well as non-devicetree targets. > >> > > Sorry, don't follow you ... I tested the patch on Ubuntu machine - so > > the non-Android and non-devicetree case is obviously supported :) > > This patch _doesn't_ provide the required support for devicetree > bindings and Android use case. I'm trying to balance wanting > to support this use case with knowing that the Android use > case is still unsolved. > > Laura Correct, but it does provide the way to instantiate the heaps by the parameters (no matter how we get them). Probably I'm wrong but it's difficult to think about something more generic. Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v3] staging: android: ion: Add implementation of dma_buf_vmap and dma_buf_vunmap
> -Original Message- > From: Liam Mark [mailto:lm...@codeaurora.org] > Sent: Friday, January 04, 2019 19:42 > To: Skidanov, Alexey > Cc: Laura Abbott ; Greg KH ; > de...@driverdev.osuosl.org; tk...@android.com; r...@android.com; linux- > ker...@vger.kernel.org; m...@android.com; sumit.sem...@linaro.org > Subject: Re: [PATCH v3] staging: android: ion: Add implementation of > dma_buf_vmap and > dma_buf_vunmap > > On Tue, 18 Dec 2018, Alexey Skidanov wrote: > > > >>> I was wondering if we could re-open the discussion on adding support to > > >>> ION for dma_buf_vmap. > > >>> It seems like the patch was not taken as the reviewers wanted more > > >>> evidence of an upstream use case. > > >>> > > >>> Here would be my upstream usage argument for including dma_buf_vmap > > >>> support in ION. > > >>> > > >>> Currently all calls to ion_dma_buf_begin_cpu_access result in the > > >>> creation > > >>> of a kernel mapping for the buffer, unfortunately the resulting call to > > >>> alloc_vmap_area can be quite expensive and this has caused a performance > > >>> regression for certain clients when they have moved to the new version > > >>> of > > >>> ION. > > >>> > > >>> The kernel mapping is not actually needed in > > >>> ion_dma_buf_begin_cpu_access, > > >>> and generally isn't needed by clients. So if we remove the creation of > > >>> the > > >>> kernel mapping in ion_dma_buf_begin_cpu_access and only create it when > > >>> needed we can speed up the calls to ion_dma_buf_begin_cpu_access. > > >>> > > >>> An additional benefit of removing the creation of kernel mappings from > > >>> ion_dma_buf_begin_cpu_access is that it makes the ION code more secure. > > >>> Currently a malicious client could call the DMA_BUF_IOCTL_SYNC IOCTL > > >>> with > > >>> flags DMA_BUF_SYNC_END multiple times to cause the ION buffer kmap_cnt > > >>> to > > >>> go negative which could lead to undesired behavior. > > >>> > > >>> One disadvantage of the above change is that a kernel mapping is not > > >>> already created when a client calls dma_buf_kmap. So the following > > >>> dma_buf_kmap contract can't be satisfied. > > >>> > > >>> /** > > >>> * dma_buf_kmap - Map a page of the buffer object into kernel address > > >>> space. The > > >>> * same restrictions as for kmap and friends apply. > > >>> * @dmabuf: [in]buffer to map page from. > > >>> * @page_num:[in]page in PAGE_SIZE units to map. > > >>> * > > >>> * This call must always succeed, any necessary preparations that might > > >>> fail > > >>> * need to be done in begin_cpu_access. > > >>> */ > > >>> > > >>> But hopefully we can work around this by moving clients to dma_buf_vmap. > > >> I think the problem is with the contract. We can't ensure that the call > > >> is always succeeds regardless the implementation - any mapping might > > >> fail. Probably this is why *all* clients of dma_buf_kmap() check the > > >> return value (so it's safe to return NULL in case of failure). > > >> > > > > > > I think currently the call to dma_buf_kmap will always succeed since the > > > DMA-Buf contract requires that the client first successfully call > > > dma_buf_begin_cpu_access(), and if dma_buf_begin_cpu_access() succeeds > > > then dma_buf_kmap will succeed. > > > > > >> I would suggest to fix the contract and to keep the dma_buf_kmap() > > >> support in ION. > > > > > > I will leave it to the DMA-Buf maintainers as to whether they want to > > > change their contract. > > > > > > Liam > > > > > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > > > a Linux Foundation Collaborative Project > > > > > > > Ok. We need the list of the clients using the ION in the mainline tree. > > > > Looks to me like the only functions which might be calling > dma_buf_kmap/dma_buf_kunmap on ION buffers are > tegra_bo_kmap/tegra_bo_kunmap, I assume Tegra is used in some Android > automotive products. > > Looks like these functions could be moved over to using > dma_buf_vmap/dma_buf_vunmap but it wouldn't be very clean and would add a > performance hit. > > Liam > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project I'm a little bit confused. Why making the buffer accessible by CPU (mapping the buffer) and making the content of the buffer valid (coherent) are so tightly coupled in DMA-BUF? Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: cma heap: Limit size of allocated buffer
In ion_cma_heap, the allocated buffer is represented by a single
struct scatterlist instance. The length field of this struct is
32 bit, hence the maximal size of requested buffer should be
less than 4GB.
The len paramer of the allocation function is 64 bit (on 64 bit systems).
Hence the requested size might be greater than 4GB and in this case
the field length of the struct scatterlist is initialized incorrectly.
To fix this, we check that requested size may fit into
the field length of the struct scatterlist
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.h | 5 +
drivers/staging/android/ion/ion_cma_heap.c | 3 +++
2 files changed, 8 insertions(+)
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index e291299..9dd7e20 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -21,6 +21,11 @@
#include "../uapi/ion.h"
+#define MAX_SCATTERLIST_LEN ({\
+ typeof(((struct scatterlist *)0)->length) v;\
+ v = -1;\
+ })
+
/**
* struct ion_buffer - metadata for a particular buffer
* @node: node in the ion_device buffers tree
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index bf65e67..d069719 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -36,6 +36,9 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
unsigned long align = get_order(size);
int ret;
+ if (size > MAX_SCATTERLIST_LEN)
+ return -EINVAL;
+
if (align > CONFIG_CMA_ALIGNMENT)
align = CONFIG_CMA_ALIGNMENT;
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ion: cma heap: Limit size of allocated buffer
On 8/26/19 11:36 AM, Laura Abbott wrote:
> On 8/23/19 10:28 PM, Alexey Skidanov wrote:
>> In ion_cma_heap, the allocated buffer is represented by a single
>> struct scatterlist instance. The length field of this struct is
>> 32 bit, hence the maximal size of requested buffer should be
>> less than 4GB.
>>
>> The len paramer of the allocation function is 64 bit (on 64 bit systems).
>> Hence the requested size might be greater than 4GB and in this case
>> the field length of the struct scatterlist is initialized incorrectly.
>>
>> To fix this, we check that requested size may fit into
>> the field length of the struct scatterlist
>>
>
> Is this a real issue that's actually possible to hit? Allocating
> more than a 4GB region of CMA seems ill advised and likely to
> throw off all the accounting.
>
Yes. Not sure why it seems ill advised - most of the buffers are small or
middle size ones, but sometimes really huge one is requested.
>> Signed-off-by: Alexey Skidanov
>> ---
>> drivers/staging/android/ion/ion.h | 5 +
>> drivers/staging/android/ion/ion_cma_heap.c | 3 +++
>> 2 files changed, 8 insertions(+)
>>
>> diff --git a/drivers/staging/android/ion/ion.h
>> b/drivers/staging/android/ion/ion.h
>> index e291299..9dd7e20 100644
>> --- a/drivers/staging/android/ion/ion.h
>> +++ b/drivers/staging/android/ion/ion.h
>> @@ -21,6 +21,11 @@
>> #include "../uapi/ion.h"
>> +#define MAX_SCATTERLIST_LEN ({\
>> + typeof(((struct scatterlist *)0)->length) v;\
>> + v = -1;\
>> + })
>> +
>> /**
>> * struct ion_buffer - metadata for a particular buffer
>> * @node: node in the ion_device buffers tree
>> diff --git a/drivers/staging/android/ion/ion_cma_heap.c
>> b/drivers/staging/android/ion/ion_cma_heap.c
>> index bf65e67..d069719 100644
>> --- a/drivers/staging/android/ion/ion_cma_heap.c
>> +++ b/drivers/staging/android/ion/ion_cma_heap.c
>> @@ -36,6 +36,9 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
>> ion_buffer *buffer,
>> unsigned long align = get_order(size);
>> int ret;
>> + if (size > MAX_SCATTERLIST_LEN)
>> + return -EINVAL;
>> +
>> if (align > CONFIG_CMA_ALIGNMENT)
>> align = CONFIG_CMA_ALIGNMENT;
>>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: dgnc: implement proper error handling in dgnc_start()
dgnc_start() ignores errors in class_create() and device_create()
and it does not deallocate resources if dgnc_tty_preinit() fails.
The patch implements proper error handling.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/dgnc/dgnc_driver.c | 28
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/dgnc/dgnc_driver.c
b/drivers/staging/dgnc/dgnc_driver.c
index ba98ff348112..f610ae1f3a9f 100644
--- a/drivers/staging/dgnc/dgnc_driver.c
+++ b/drivers/staging/dgnc/dgnc_driver.c
@@ -238,6 +238,7 @@ static int dgnc_start(void)
{
int rc = 0;
unsigned long flags;
+ struct device *dev;
/* make sure that the globals are init'd before we do anything else */
dgnc_init_globals();
@@ -257,9 +258,20 @@ static int dgnc_start(void)
dgnc_Major = rc;
dgnc_class = class_create(THIS_MODULE, "dgnc_mgmt");
- device_create(dgnc_class, NULL,
- MKDEV(dgnc_Major, 0),
- NULL, "dgnc_mgmt");
+ if (IS_ERR(dgnc_class)) {
+ rc = PTR_ERR(dgnc_class);
+ pr_err(DRVSTR ": Can't create dgnc_mgmt class (%d)\n", rc);
+ goto failed_class;
+ }
+
+ dev = device_create(dgnc_class, NULL,
+ MKDEV(dgnc_Major, 0),
+ NULL, "dgnc_mgmt");
+ if (IS_ERR(dev)) {
+ rc = PTR_ERR(dev);
+ pr_err(DRVSTR ": Can't create device (%d)\n", rc);
+ goto failed_device;
+ }
/*
* Init any global tty stuff.
@@ -268,7 +280,7 @@ static int dgnc_start(void)
if (rc < 0) {
pr_err(DRVSTR ": tty preinit - not enough memory (%d)\n", rc);
- return rc;
+ goto failed_tty;
}
/* Start the poller */
@@ -282,6 +294,14 @@ static int dgnc_start(void)
add_timer(&dgnc_poll_timer);
+ return 0;
+
+failed_tty:
+ device_destroy(dgnc_class, MKDEV(dgnc_Major, 0));
+failed_device:
+ class_destroy(dgnc_class);
+failed_class:
+ unregister_chrdev(dgnc_Major, "dgnc");
return rc;
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723au: fix sparse warning
drivers/staging/rtl8723au/core/rtw_xmit.c:2375 warning: symbol
'rtl8723a_EfusePgPacketRead' was not declared. Should it be static?
Function 'rtw_ack_tx_done23a' seems to be unused in current staging
code.
Signed-off-by: Alexey Tulia
---
drivers/staging/rtl8723au/core/rtw_xmit.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/drivers/staging/rtl8723au/core/rtw_xmit.c
b/drivers/staging/rtl8723au/core/rtw_xmit.c
index 7a5e6bf..1c82dff 100644
--- a/drivers/staging/rtl8723au/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723au/core/rtw_xmit.c
@@ -2372,12 +2372,3 @@ int rtw_ack_tx_wait23a(struct xmit_priv *pxmitpriv, u32
timeout_ms)
return rtw_sctx_wait23a(pack_tx_ops);
}
-void rtw_ack_tx_done23a(struct xmit_priv *pxmitpriv, int status)
-{
- struct submit_ctx *pack_tx_ops = &pxmitpriv->ack_tx_ops;
-
- if (pxmitpriv->ack_tx)
- rtw23a_sctx_done_err(&pack_tx_ops, status);
- else
- DBG_8723A("%s ack_tx not set\n", __func__);
-}
--
2.1.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: rtl8723au: fix sparse warning
drivers/staging/rtl8723au/core/rtw_xmit.c:2375 warning: symbol
'rtw_ack_tx_done23a' was not declared. Should it be static?
Function 'rtw_ack_tx_done23a' seems to be unused in current staging
code.
Signed-off-by: Alexey Tulia
---
drivers/staging/rtl8723au/core/rtw_xmit.c | 9 -
1 file changed, 9 deletions(-)
diff --git a/drivers/staging/rtl8723au/core/rtw_xmit.c
b/drivers/staging/rtl8723au/core/rtw_xmit.c
index 7a5e6bf..1c82dff 100644
--- a/drivers/staging/rtl8723au/core/rtw_xmit.c
+++ b/drivers/staging/rtl8723au/core/rtw_xmit.c
@@ -2372,12 +2372,3 @@ int rtw_ack_tx_wait23a(struct xmit_priv *pxmitpriv, u32
timeout_ms)
return rtw_sctx_wait23a(pack_tx_ops);
}
-void rtw_ack_tx_done23a(struct xmit_priv *pxmitpriv, int status)
-{
- struct submit_ctx *pack_tx_ops = &pxmitpriv->ack_tx_ops;
-
- if (pxmitpriv->ack_tx)
- rtw23a_sctx_done_err(&pack_tx_ops, status);
- else
- DBG_8723A("%s ack_tx not set\n", __func__);
-}
--
2.1.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: lustre: do not ignore try_module_get() fail in obd_class_open()
obd_class_open() ignores error code of try_module_get(),
while it can lead to race with module unload.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/lustre/lustre/obdclass/linux/linux-module.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
index 66ceab20c743..bb4bc72ddac7 100644
--- a/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
+++ b/drivers/staging/lustre/lustre/obdclass/linux/linux-module.c
@@ -168,8 +168,7 @@ EXPORT_SYMBOL(obd_ioctl_popdata);
/* opening /dev/obd */
static int obd_class_open(struct inode *inode, struct file *file)
{
- try_module_get(THIS_MODULE);
- return 0;
+ return try_module_get(THIS_MODULE);
}
/* closing /dev/obd */
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
user provided data for ion buffer
Hi, I would like to add 64 bit user data to ion allocated buffer. This data may be assigned during buffer allocation (passed as one of the ioctl parameters) and may be queried by fd representing the buffer or pfn. Looks like may be useful in many use cases. Please, let me know what do you think. Thanks, Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
ION doesn't set the cache attributes according to ION_FLAG_CACHED buffer flag
Hi, Mapping the buffer to user space, ION failed to set the cache attributes according to ION_FLAG_CACHED flag on x86. When the reserved memory (reserved by memmap= kernel boot option) or part of it is mapped to the user space, the user space memory mapping is always *uncachable*. ION uses remap_pfn_range to create the mapping. On x86 arch, the actual cache attributes may be different from the requested ones due to the PAT implementation. When we map the PFNs one by one to some VMA, the cache attributes are set by lookup_memtype, completely ignoring the cache attributes of VMA. For example: - if the requested memory region is managed by the OS (has struct page object), the actual cache attribute is set based on struct page cache attributes saved in its flag member - if the memory region is not managed by the OS (there are no struct page object), the PAT internal RBT of memory types is scanned. If RBT defines the memory type for the requested PFN, this memory type is used, otherwise the PFN is mapped as uncachable. Is it x86 specific issue? I have implemented and tested the fix for this issue, but it looks like it should be submitted to the PAT. Thanks, Alexey ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: android: ion: Clean unused debug_show memeber of the heap object
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.h | 5 -
drivers/staging/android/ion/ion_system_heap.c | 24
2 files changed, 29 deletions(-)
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 876197b..0afa9cd 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -158,8 +158,6 @@ struct ion_heap_ops {
* @lock: protects the free list
* @waitqueue: queue to wait on from deferred free thread
* @task: task struct of deferred free thread
- * @debug_show:called when heap debug file is read to add any
- * heap specific debug info to output
*
* Represents a pool of memory from which buffers can be made. In some
* systems the only heap is regular system memory allocated via vmalloc.
@@ -180,9 +178,6 @@ struct ion_heap {
spinlock_t free_lock;
wait_queue_head_t waitqueue;
struct task_struct *task;
-
- int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
- void *unused);
};
/**
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index b5c3195..d0d0490 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -213,29 +213,6 @@ static struct ion_heap_ops system_heap_ops = {
.shrink = ion_system_heap_shrink,
};
-static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file
*s,
- void *unused)
-{
- struct ion_system_heap *sys_heap = container_of(heap,
- struct ion_system_heap,
- heap);
- int i;
- struct ion_page_pool *pool;
-
- for (i = 0; i < NUM_ORDERS; i++) {
- pool = sys_heap->pools[i];
-
- seq_printf(s, "%d order %u highmem pages %lu total\n",
- pool->high_count, pool->order,
- (PAGE_SIZE << pool->order) * pool->high_count);
- seq_printf(s, "%d order %u lowmem pages %lu total\n",
- pool->low_count, pool->order,
- (PAGE_SIZE << pool->order) * pool->low_count);
- }
-
- return 0;
-}
-
static void ion_system_heap_destroy_pools(struct ion_page_pool **pools)
{
int i;
@@ -282,7 +259,6 @@ static struct ion_heap *__ion_system_heap_create(void)
if (ion_system_heap_create_pools(heap->pools))
goto free_heap;
- heap->heap.debug_show = ion_system_heap_debug_show;
return &heap->heap;
free_heap:
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: android: ion: Clean unused debug_show memeber of the heap object
ION had supported heap debug info under /sys/kernel/debug/ion/.
This support have been removed but some leftovers (dead code) still exist.
This patch removes the existing dead code.
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.h | 5 -
drivers/staging/android/ion/ion_system_heap.c | 24
2 files changed, 29 deletions(-)
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 876197b..0afa9cd 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -158,8 +158,6 @@ struct ion_heap_ops {
* @lock: protects the free list
* @waitqueue: queue to wait on from deferred free thread
* @task: task struct of deferred free thread
- * @debug_show:called when heap debug file is read to add any
- * heap specific debug info to output
*
* Represents a pool of memory from which buffers can be made. In some
* systems the only heap is regular system memory allocated via vmalloc.
@@ -180,9 +178,6 @@ struct ion_heap {
spinlock_t free_lock;
wait_queue_head_t waitqueue;
struct task_struct *task;
-
- int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
- void *unused);
};
/**
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index b5c3195..d0d0490 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -213,29 +213,6 @@ static struct ion_heap_ops system_heap_ops = {
.shrink = ion_system_heap_shrink,
};
-static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file
*s,
- void *unused)
-{
- struct ion_system_heap *sys_heap = container_of(heap,
- struct ion_system_heap,
- heap);
- int i;
- struct ion_page_pool *pool;
-
- for (i = 0; i < NUM_ORDERS; i++) {
- pool = sys_heap->pools[i];
-
- seq_printf(s, "%d order %u highmem pages %lu total\n",
- pool->high_count, pool->order,
- (PAGE_SIZE << pool->order) * pool->high_count);
- seq_printf(s, "%d order %u lowmem pages %lu total\n",
- pool->low_count, pool->order,
- (PAGE_SIZE << pool->order) * pool->low_count);
- }
-
- return 0;
-}
-
static void ion_system_heap_destroy_pools(struct ion_page_pool **pools)
{
int i;
@@ -282,7 +259,6 @@ static struct ion_heap *__ion_system_heap_create(void)
if (ion_system_heap_create_pools(heap->pools))
goto free_heap;
- heap->heap.debug_show = ion_system_heap_debug_show;
return &heap->heap;
free_heap:
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: android: ion: Clean unused debug_show memeber of the heap object
On 09/04/2018 09:23 PM, Laura Abbott wrote:
> On 08/26/2018 01:08 PM, Alexey Skidanov wrote:
>> ION had supported heap debug info under
>> /sys/kernel/debug/ion/.
>> This support have been removed but some leftovers (dead code) still
>> exist.
>>
>> This patch removes the existing dead code.
>>
>
> This was actually an unintended side effect. The goal was to rip out
> the racy handle/buffer mess and the associated debugfs there. I do
> think the extra heap_show for system heap is useful but I'd rather
> see it reimplemented with a full debugfs vs. trying to just tack it
> on.
>
I would start with some basic, per heap, statistics - # of buffers,
total allocated space, etc ...
> I'd like to see the exact commit where this was orphaned (15c6098cfec5
> ("staging: android: ion: Remove ion_handle and ion_client")) specified
> somewhere in the commit text so if you add that, you can add
Yes, sure, I will add it
>
> Acked-by: Laura Abbott
>
>> Signed-off-by: Alexey Skidanov
>> ---
>> drivers/staging/android/ion/ion.h | 5 -
>> drivers/staging/android/ion/ion_system_heap.c | 24
>>
>> 2 files changed, 29 deletions(-)
>>
>> diff --git a/drivers/staging/android/ion/ion.h
>> b/drivers/staging/android/ion/ion.h
>> index 876197b..0afa9cd 100644
>> --- a/drivers/staging/android/ion/ion.h
>> +++ b/drivers/staging/android/ion/ion.h
>> @@ -158,8 +158,6 @@ struct ion_heap_ops {
>> * @lock: protects the free list
>> * @waitqueue: queue to wait on from deferred free thread
>> * @task: task struct of deferred free thread
>> - * @debug_show: called when heap debug file is read to add any
>> - * heap specific debug info to output
>> *
>> * Represents a pool of memory from which buffers can be made. In some
>> * systems the only heap is regular system memory allocated via
>> vmalloc.
>> @@ -180,9 +178,6 @@ struct ion_heap {
>> spinlock_t free_lock;
>> wait_queue_head_t waitqueue;
>> struct task_struct *task;
>> -
>> - int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
>> - void *unused);
>> };
>> /**
>> diff --git a/drivers/staging/android/ion/ion_system_heap.c
>> b/drivers/staging/android/ion/ion_system_heap.c
>> index b5c3195..d0d0490 100644
>> --- a/drivers/staging/android/ion/ion_system_heap.c
>> +++ b/drivers/staging/android/ion/ion_system_heap.c
>> @@ -213,29 +213,6 @@ static struct ion_heap_ops system_heap_ops = {
>> .shrink = ion_system_heap_shrink,
>> };
>> -static int ion_system_heap_debug_show(struct ion_heap *heap, struct
>> seq_file *s,
>> - void *unused)
>> -{
>> - struct ion_system_heap *sys_heap = container_of(heap,
>> - struct ion_system_heap,
>> - heap);
>> - int i;
>> - struct ion_page_pool *pool;
>> -
>> - for (i = 0; i < NUM_ORDERS; i++) {
>> - pool = sys_heap->pools[i];
>> -
>> - seq_printf(s, "%d order %u highmem pages %lu total\n",
>> - pool->high_count, pool->order,
>> - (PAGE_SIZE << pool->order) * pool->high_count);
>> - seq_printf(s, "%d order %u lowmem pages %lu total\n",
>> - pool->low_count, pool->order,
>> - (PAGE_SIZE << pool->order) * pool->low_count);
>> - }
>> -
>> - return 0;
>> -}
>> -
>> static void ion_system_heap_destroy_pools(struct ion_page_pool **pools)
>> {
>> int i;
>> @@ -282,7 +259,6 @@ static struct ion_heap
>> *__ion_system_heap_create(void)
>> if (ion_system_heap_create_pools(heap->pools))
>> goto free_heap;
>> - heap->heap.debug_show = ion_system_heap_debug_show;
>> return &heap->heap;
>> free_heap:
>>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: android: ion: Clean unused debug_show memeber of the heap object
ION had supported heap debug info under /sys/kernel/debug/ion/.
This support have been removed but some leftovers (dead code) still exist.
This patch removes the existing dead code.
Fixes: 15c6098cfec5 ("staging: android: ion: Remove ion_handle and ion_client")
Signed-off-by: Alexey Skidanov
---
drivers/staging/android/ion/ion.h | 5 -
drivers/staging/android/ion/ion_system_heap.c | 24
2 files changed, 29 deletions(-)
diff --git a/drivers/staging/android/ion/ion.h
b/drivers/staging/android/ion/ion.h
index 876197b..0afa9cd 100644
--- a/drivers/staging/android/ion/ion.h
+++ b/drivers/staging/android/ion/ion.h
@@ -158,8 +158,6 @@ struct ion_heap_ops {
* @lock: protects the free list
* @waitqueue: queue to wait on from deferred free thread
* @task: task struct of deferred free thread
- * @debug_show:called when heap debug file is read to add any
- * heap specific debug info to output
*
* Represents a pool of memory from which buffers can be made. In some
* systems the only heap is regular system memory allocated via vmalloc.
@@ -180,9 +178,6 @@ struct ion_heap {
spinlock_t free_lock;
wait_queue_head_t waitqueue;
struct task_struct *task;
-
- int (*debug_show)(struct ion_heap *heap, struct seq_file *s,
- void *unused);
};
/**
diff --git a/drivers/staging/android/ion/ion_system_heap.c
b/drivers/staging/android/ion/ion_system_heap.c
index b5c3195..d0d0490 100644
--- a/drivers/staging/android/ion/ion_system_heap.c
+++ b/drivers/staging/android/ion/ion_system_heap.c
@@ -213,29 +213,6 @@ static struct ion_heap_ops system_heap_ops = {
.shrink = ion_system_heap_shrink,
};
-static int ion_system_heap_debug_show(struct ion_heap *heap, struct seq_file
*s,
- void *unused)
-{
- struct ion_system_heap *sys_heap = container_of(heap,
- struct ion_system_heap,
- heap);
- int i;
- struct ion_page_pool *pool;
-
- for (i = 0; i < NUM_ORDERS; i++) {
- pool = sys_heap->pools[i];
-
- seq_printf(s, "%d order %u highmem pages %lu total\n",
- pool->high_count, pool->order,
- (PAGE_SIZE << pool->order) * pool->high_count);
- seq_printf(s, "%d order %u lowmem pages %lu total\n",
- pool->low_count, pool->order,
- (PAGE_SIZE << pool->order) * pool->low_count);
- }
-
- return 0;
-}
-
static void ion_system_heap_destroy_pools(struct ion_page_pool **pools)
{
int i;
@@ -282,7 +259,6 @@ static struct ion_heap *__ion_system_heap_create(void)
if (ion_system_heap_create_pools(heap->pools))
goto free_heap;
- heap->heap.debug_show = ion_system_heap_debug_show;
return &heap->heap;
free_heap:
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[media] s5p-cec: strange clk enabling pattern
The s5p-cec driver has a few places that touch hdmicec clk:
static int s5p_cec_probe(struct platform_device *pdev)
{
...
cec->clk = devm_clk_get(dev, "hdmicec");
if (IS_ERR(cec->clk))
return PTR_ERR(cec->clk);
...
}
static int __maybe_unused s5p_cec_runtime_suspend(struct device *dev)
{
struct s5p_cec_dev *cec = dev_get_drvdata(dev);
clk_disable_unprepare(cec->clk);
return 0;
}
static int __maybe_unused s5p_cec_runtime_resume(struct device *dev)
{
struct s5p_cec_dev *cec = dev_get_drvdata(dev);
int ret;
ret = clk_prepare_enable(cec->clk);
if (ret < 0)
return ret;
return 0;
}
Is it ok to enable/disable clock in rusume/suspend only?
Or have I missed anything?
--
Thank you,
Alexey Khoroshilov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 02/39] proc: introduce proc_create_seq{,_data}
On Thu, Apr 19, 2018 at 02:41:03PM +0200, Christoph Hellwig wrote:
> Variants of proc_create{,_data} that directly take a struct seq_operations
> argument and drastically reduces the boilerplate code in the callers.
> +static int proc_seq_open(struct inode *inode, struct file *file)
> +{
> + struct proc_dir_entry *de = PDE(inode);
> +
> + return seq_open(file, de->seq_ops);
> +}
> +
> +static const struct file_operations proc_seq_fops = {
> + .open = proc_seq_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release= seq_release,
> +};
> +
> +struct proc_dir_entry *proc_create_seq_data(const char *name, umode_t mode,
> + struct proc_dir_entry *parent, const struct seq_operations *ops,
> + void *data)
> +{
> + struct proc_dir_entry *p;
> +
> + p = proc_create_data(name, mode, parent, &proc_seq_fops, data);
> + if (p)
> + p->seq_ops = ops;
> + return p;
> +}
Should be oopsable.
Once proc_create_data() returns, entry is live, ->open can be called.
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -44,6 +44,7 @@ struct proc_dir_entry {
> struct completion *pde_unload_completion;
> const struct inode_operations *proc_iops;
> const struct file_operations *proc_fops;
> + const struct seq_operations *seq_ops;
> void *data;
> unsigned int low_ino;
> nlink_t nlink;
"struct proc_dir_entry is 192/128 bytes now.
If someone knows how to pad array to certain size without union
please tell.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 14/39] proc: introduce proc_create_net_single
On Thu, Apr 19, 2018 at 02:41:15PM +0200, Christoph Hellwig wrote:
> Variant of proc_create_data that directly take a seq_file show
> +struct proc_dir_entry *proc_create_net_single(const char *name, umode_t mode,
> + struct proc_dir_entry *parent,
> + int (*show)(struct seq_file *, void *), void *data)
> +{
> + struct proc_dir_entry *p;
> +
> + p = proc_create_data(name, mode, parent, &proc_net_single_fops, data);
> + if (p)
> + p->single_show = show;
> + return p;
> +}
Ditto, should be oopsable.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 03/39] proc: introduce proc_create_seq_private
On Thu, Apr 19, 2018 at 02:41:04PM +0200, Christoph Hellwig wrote:
> Variant of proc_create_data that directly take a struct seq_operations
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -45,6 +45,7 @@ struct proc_dir_entry {
> const struct inode_operations *proc_iops;
> const struct file_operations *proc_fops;
> const struct seq_operations *seq_ops;
> + size_t state_size;
"unsigned int" please.
Where have you seen 4GB priv states?
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: simplify procfs code for seq_file instances
> git://git.infradead.org/users/hch/misc.git proc_create I want to ask if it is time to start using poorman function overloading with _b_c_e(). There are millions of allocation functions for example, all slightly difference, and people will add more. Seeing /proc interfaces doubled like this is painful. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: simplify procfs code for seq_file instances
On Tue, Apr 24, 2018 at 06:06:53PM +0200, Christoph Hellwig wrote: > On Tue, Apr 24, 2018 at 08:19:16AM -0700, Andrew Morton wrote: > > > > I want to ask if it is time to start using poorman function overloading > > > > with _b_c_e(). There are millions of allocation functions for example, > > > > all slightly difference, and people will add more. Seeing /proc > > > > interfaces > > > > doubled like this is painful. > > > > > > Function overloading is totally unacceptable. > > > > > > And I very much disagree with a tradeoff that keeps 5000 lines of > > > code vs a few new helpers. > > > > OK, the curiosity and suspense are killing me. What the heck is > > "function overloading with _b_c_e()"? > > The way I understood Alexey was to use have a proc_create macro > that can take different ops types. Although the short cut for > __builtin_types_compatible_p would be _b_t_c or similar, so maybe > I misunderstood him. That's correct. I also think that several dozens kmalloc signatures are a problem. And there will be more with pmalloc* stuff and more 2D/3D array checked allocations and who knows what. And I want to add typed kmalloc! ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: simplify procfs code for seq_file instances V2
On Wed, Apr 25, 2018 at 05:47:47PM +0200, Christoph Hellwig wrote:
> Changes since V1:
> - open code proc_create_data to avoid setting not fully initialized
>entries live
> - use unsigned int for state_size
Need this to maintain sizeof(struct proc_dir_entry):
Otherwise ACK fs/proc/ part.
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 6d171485c45b..a318ae5b36b4 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -48,8 +48,8 @@ struct proc_dir_entry {
const struct seq_operations *seq_ops;
int (*single_show)(struct seq_file *, void *);
};
- unsigned int state_size;
void *data;
+ unsigned int state_size;
unsigned int low_ino;
nlink_t nlink;
kuid_t uid;
@@ -62,9 +62,9 @@ struct proc_dir_entry {
umode_t mode;
u8 namelen;
#ifdef CONFIG_64BIT
-#define SIZEOF_PDE_INLINE_NAME (192-139)
+#define SIZEOF_PDE_INLINE_NAME (192-155)
#else
-#define SIZEOF_PDE_INLINE_NAME (128-87)
+#define SIZEOF_PDE_INLINE_NAME (128-95)
#endif
char inline_name[SIZEOF_PDE_INLINE_NAME];
} __randomize_layout;
diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
index baf1994289ce..7d94fa005b0d 100644
--- a/fs/proc/proc_net.c
+++ b/fs/proc/proc_net.c
@@ -40,7 +40,7 @@ static struct net *get_proc_net(const struct inode *inode)
static int seq_open_net(struct inode *inode, struct file *file)
{
- size_t state_size = PDE(inode)->state_size;
+ unsigned int state_size = PDE(inode)->state_size;
struct seq_net_private *p;
struct net *net;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: simplify procfs code for seq_file instances V2
On Sun, May 06, 2018 at 06:45:31PM +0100, Al Viro wrote:
> On Sun, May 06, 2018 at 08:19:49PM +0300, Alexey Dobriyan wrote:
> > @@ -62,9 +62,9 @@ struct proc_dir_entry {
> > umode_t mode;
> > u8 namelen;
> > #ifdef CONFIG_64BIT
> > -#define SIZEOF_PDE_INLINE_NAME (192-139)
> > +#define SIZEOF_PDE_INLINE_NAME (192-155)
> > #else
> > -#define SIZEOF_PDE_INLINE_NAME (128-87)
> > +#define SIZEOF_PDE_INLINE_NAME (128-95)
>
> > #endif
> > char inline_name[SIZEOF_PDE_INLINE_NAME];
> > } __randomize_layout;
>
> *UGH*
I agree. Maintaining these numbers is a pain point.
Who knew people were going to start adding fields right away.
> Both to the original state and that kind of "adjustments".
> Incidentally, with __bugger_layout in there these expressions
> are simply wrong.
Struct randomization is exempt from maintaining sizeof as they are already
screwing cachelines and everything. But if patch works with lockdep and
randomization -- even better.
> If nothing else, I would suggest turning the last one into
> char inline_name[];
> in hope that layout won't get... randomized that much and
> used
>
> #ifdef CONFIG_64BIT
> #define PDE_SIZE 192
> #else
> #define PDE_SIZE 128
> #endif
>
> union __proc_dir_entry {
> char pad[PDE_SIZE];
> struct proc_dir_entry real;
> };
>
> #define SIZEOF_PDE_INLINE_NAME (PDE_SIZE - offsetof(struct proc_dir_entry,
> inline_name))
>
> for constants, adjusted sizeof and sizeof_field when creating
> proc_dir_entry_cache and turned proc_root into
>
> union __proc_dir_entry __proc_root = { .real = {
> .low_ino= PROC_ROOT_INO,
> .namelen= 5,
> .mode = S_IFDIR | S_IRUGO | S_IXUGO,
> .nlink = 2,
> .refcnt = REFCOUNT_INIT(1),
> .proc_iops = &proc_root_inode_operations,
> .proc_fops = &proc_root_operations,
> .parent = &__proc_root.real,
> .subdir = RB_ROOT,
> .name = __proc_root.real.inline_name,
> .inline_name= "/proc",
> }};
>
> #define proc_root __proc_root.real
> (or actually used __proc_root.real in all of a 6 places where it remains).
>
> > - size_t state_size = PDE(inode)->state_size;
> > + unsigned int state_size = PDE(inode)->state_size;
>
>
> You and your "size_t is evil" crusade...
[nods]
unsigned long flags; /* error bits */
unsigned long i_state;
unsigned long s_blocksize;
unsigned long s_flags;
unsigned long s_iflags; /* internal SB_I_* flags */
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [lustre-devel] [PATCH] staging: lustre: delete the filesystem from the tree.
> 4 июня 2018 г., в 6:54, NeilBrown написал(а): > > On Sun, Jun 03 2018, Dilger, Andreas wrote: > >> On Jun 1, 2018, at 17:19, NeilBrown wrote: >>> >>> On Fri, Jun 01 2018, Doug Oucharek wrote: >>> Would it makes sense to land LNet and LNDs on their own first? Get the networking house in order first before layering on the file system? >>> >>> I'd like to turn that question on it's head: >>> Do we need LNet and LNDs? What value do they provide? >>> (this is a genuine question, not being sarcastic). >>> >>> It is a while since I tried to understand LNet, and then it was a >>> fairly superficial look, but I think it is an abstraction layer >>> that provides packet-based send/receive with some numa-awareness >>> and routing functionality. It sits over sockets (TCP) and IB and >>> provides a uniform interface. >> >> LNet is originally based on a high-performance networking stack called >> Portals (v3, http://www.cs.sandia.gov/Portals/), with additions for LNet >> routing to allow cross-network bridging. >> >> A critical part of LNet is that it is for RDMA and not packet-based >> messages. Everything in Lustre is structured around RDMA. Of course, >> RDMA is not possible with TCP To be clear. Soft IB (aka Soft RoCE) driver is part of OFED stack from 4.8(or 4.9). So RDMA API now is possible with TCP networks. Alex ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [lustre-devel] [PATCH] staging: lustre: delete the filesystem from the tree.
> 4 июня 2018 г., в 7:15, Andreas Dilger написал(а): > > On Jun 3, 2018, at 9:59 PM, Alexey Lyashkov wrote: >> >>> On Sun, Jun 03 2018, Dilger, Andreas wrote: >>> >>>> LNet is originally based on a high-performance networking stack called >>>> Portals (v3, http://www.cs.sandia.gov/Portals/), with additions for LNet >>>> routing to allow cross-network bridging. >>>> >>>> A critical part of LNet is that it is for RDMA and not packet-based >>>> messages. Everything in Lustre is structured around RDMA. Of course, >>>> RDMA is not possible with TCP >> >> To be clear. Soft IB (aka Soft RoCE) driver is part of OFED stack from 4.8 >> (or 4.9). So RDMA API now is possible with TCP networks. > > Well, strictly speaking RoCE still isn't possible with TCP networks. RoCE v1 > is an Ethernet layer protocol (not IP based), while RoCE v2 is UDP/IP based. > > Andreas, I talk about RXE driver in OFED stack. It have works over TCP networks. It likely to be RoCE v2 compatible as UDP tunnel use and exist for while. Alex ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: dgap: remove unneeded status variables
dgap_driver_start and dgap_Major_Control_Registered are used
to keep status of initialization of the driver as a whole and its "Major
Control".
But the code that checks them is executed once on module init/unload.
That makes no sense in these variables as far as their values are predictable
at any time.
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/dgap/dgap_driver.c | 105 -
1 file changed, 46 insertions(+), 59 deletions(-)
diff --git a/drivers/staging/dgap/dgap_driver.c
b/drivers/staging/dgap/dgap_driver.c
index 089d017fc291..d7f1e999aaa4 100644
--- a/drivers/staging/dgap/dgap_driver.c
+++ b/drivers/staging/dgap/dgap_driver.c
@@ -117,9 +117,6 @@ int dgap_poll_tick = 20;/* Poll
interval - 20 ms */
/*
* Static vars.
*/
-static int dgap_Major_Control_Registered = FALSE;
-static uintdgap_driver_start = FALSE;
-
static struct class * dgap_class;
/*
@@ -283,65 +280,57 @@ static int dgap_start(void)
int rc = 0;
unsigned long flags;
- if (dgap_driver_start == FALSE) {
-
- dgap_driver_start = TRUE;
+ /* make sure that the globals are init'd before we do anything else */
+ dgap_init_globals();
- /* make sure that the globals are init'd before we do anything
else */
- dgap_init_globals();
+ dgap_NumBoards = 0;
- dgap_NumBoards = 0;
+ APR(("For the tools package or updated drivers please visit
http://www.digi.com\n";));
- APR(("For the tools package or updated drivers please visit
http://www.digi.com\n";));
-
- /*
-* Register our base character device into the kernel.
-* This allows the download daemon to connect to the downld
device
-* before any of the boards are init'ed.
-*/
- if (!dgap_Major_Control_Registered) {
- /*
-* Register management/dpa devices
-*/
- rc = register_chrdev(DIGI_DGAP_MAJOR, "dgap",
&DgapBoardFops);
- if (rc < 0) {
- APR(("Can't register dgap driver device
(%d)\n", rc));
- return (rc);
- }
+ /*
+* Register our base character device into the kernel.
+* This allows the download daemon to connect to the downld device
+* before any of the boards are init'ed.
+*/
- dgap_class = class_create(THIS_MODULE, "dgap_mgmt");
- device_create(dgap_class, NULL,
- MKDEV(DIGI_DGAP_MAJOR, 0),
- NULL, "dgap_mgmt");
- device_create(dgap_class, NULL,
- MKDEV(DIGI_DGAP_MAJOR, 1),
- NULL, "dgap_downld");
- dgap_Major_Control_Registered = TRUE;
- }
+ /*
+* Register management/dpa devices
+*/
+ rc = register_chrdev(DIGI_DGAP_MAJOR, "dgap", &DgapBoardFops);
+ if (rc < 0) {
+ APR(("Can't register dgap driver device (%d)\n", rc));
+ return (rc);
+ }
- /*
-* Init any global tty stuff.
-*/
- rc = dgap_tty_preinit();
+ dgap_class = class_create(THIS_MODULE, "dgap_mgmt");
+ device_create(dgap_class, NULL,
+ MKDEV(DIGI_DGAP_MAJOR, 0),
+ NULL, "dgap_mgmt");
+ device_create(dgap_class, NULL,
+ MKDEV(DIGI_DGAP_MAJOR, 1),
+ NULL, "dgap_downld");
- if (rc < 0) {
- APR(("tty preinit - not enough memory (%d)\n", rc));
- return(rc);
- }
+ /*
+* Init any global tty stuff.
+*/
+ rc = dgap_tty_preinit();
+ if (rc < 0) {
+ APR(("tty preinit - not enough memory (%d)\n", rc));
+ return(rc);
+ }
- /* Start the poller */
- DGAP_LOCK(dgap_poll_lock, flags);
- init_timer(&dgap_poll_timer);
- dgap_poll_timer.function = dgap_poll_handler;
- dgap_poll_timer.data = 0;
- dgap_poll_time = jiffies + dgap_jiffies_from_ms(dgap_poll_tick);
- dgap_poll_timer.expires = dgap_poll_time;
- DGAP_UNLOCK(dgap_poll_lock, flags);
+ /* Start the poller */
+ DGAP_LOCK(dgap_poll_lock, flags);
+ init_timer(&dgap_poll_timer);
+ dgap_poll_timer.function = dgap_poll_handler;
+ dgap_poll_timer.data = 0;
+
[PATCH 2/3] staging: dgap: implement proper error handling in dgap_start()
dgap_start() ignored errors in class_create() and device_create().
The patch implements proper error handling.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/dgap/dgap_driver.c | 42 +++---
1 file changed, 35 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/dgap/dgap_driver.c
b/drivers/staging/dgap/dgap_driver.c
index d7f1e999aaa4..136c879b560c 100644
--- a/drivers/staging/dgap/dgap_driver.c
+++ b/drivers/staging/dgap/dgap_driver.c
@@ -279,6 +279,7 @@ static int dgap_start(void)
{
int rc = 0;
unsigned long flags;
+ struct device *device;
/* make sure that the globals are init'd before we do anything else */
dgap_init_globals();
@@ -303,12 +304,29 @@ static int dgap_start(void)
}
dgap_class = class_create(THIS_MODULE, "dgap_mgmt");
- device_create(dgap_class, NULL,
- MKDEV(DIGI_DGAP_MAJOR, 0),
- NULL, "dgap_mgmt");
- device_create(dgap_class, NULL,
- MKDEV(DIGI_DGAP_MAJOR, 1),
- NULL, "dgap_downld");
+ if (IS_ERR(dgap_class)) {
+ rc = PTR_ERR(dgap_class);
+ APR(("Can't create dgap_mgmt class (%d)\n", rc));
+ goto failed_class;
+ }
+
+ device = device_create(dgap_class, NULL,
+ MKDEV(DIGI_DGAP_MAJOR, 0),
+ NULL, "dgap_mgmt");
+ if (IS_ERR(device)) {
+ rc = PTR_ERR(device);
+ APR(("Can't create device (%d)\n", rc));
+ goto failed_device0;
+ }
+
+ device = device_create(dgap_class, NULL,
+ MKDEV(DIGI_DGAP_MAJOR, 1),
+ NULL, "dgap_downld");
+ if (IS_ERR(device)) {
+ rc = PTR_ERR(device);
+ APR(("Can't create device (%d)\n", rc));
+ goto failed_device1;
+ }
/*
* Init any global tty stuff.
@@ -316,7 +334,7 @@ static int dgap_start(void)
rc = dgap_tty_preinit();
if (rc < 0) {
APR(("tty preinit - not enough memory (%d)\n", rc));
- return(rc);
+ goto failed_tty;
}
/* Start the poller */
@@ -333,6 +351,16 @@ static int dgap_start(void)
dgap_driver_state = DRIVER_NEED_CONFIG_LOAD;
return (rc);
+
+failed_tty:
+ device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 1));
+failed_device1:
+ device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 0));
+failed_device0:
+ class_destroy(dgap_class);
+failed_class:
+ unregister_chrdev(DIGI_DGAP_MAJOR, "dgap");
+ return (rc);
}
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: dgap: fix error handling in dgap_init_module()
No need to call pci_unregister_driver() if pci_register_driver() failed.
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/dgap/dgap_driver.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/dgap/dgap_driver.c
b/drivers/staging/dgap/dgap_driver.c
index 136c879b560c..cc6933db6feb 100644
--- a/drivers/staging/dgap/dgap_driver.c
+++ b/drivers/staging/dgap/dgap_driver.c
@@ -254,18 +254,10 @@ int dgap_init_module(void)
/*
* If something went wrong in the scan, bail out of driver.
*/
- if (rc < 0) {
- /* Only unregister the pci driver if it was actually
registered. */
- if (dgap_NumBoards)
- pci_unregister_driver(&dgap_driver);
- else
- printk("WARNING: dgap driver load failed. No DGAP
boards found.\n");
-
+ if (rc < 0)
dgap_cleanup_module();
- }
- else {
+ else
dgap_create_driver_sysfiles(&dgap_driver);
- }
DPR_INIT(("Finished init_module. Returning %d\n", rc));
return (rc);
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: dgap: remove unneeded status variables
dgap_driver_start and dgap_Major_Control_Registered are used
to keep status of initialization of the driver as a whole and its "Major
Control".
But the code that checks them is executed once on module init/unload.
That makes no sense in these variables as far as their values are predictable
at any time.
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/dgap/dgap.c | 97 -
1 file changed, 42 insertions(+), 55 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index cbce457..5271856 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -254,9 +254,6 @@ static int dgap_poll_tick = 20; /* Poll interval - 20
ms */
/*
* Static vars.
*/
-static int dgap_Major_Control_Registered = FALSE;
-static uint dgap_driver_start = FALSE;
-
static struct class *dgap_class;
static struct board_t *dgap_BoardsByMajor[256];
@@ -561,61 +558,54 @@ static int dgap_start(void)
int rc = 0;
unsigned long flags;
- if (dgap_driver_start == FALSE) {
+ /*
+* make sure that the globals are
+* init'd before we do anything else
+*/
+ dgap_init_globals();
- dgap_driver_start = TRUE;
+ dgap_NumBoards = 0;
- /*
-* make sure that the globals are
-* init'd before we do anything else
-*/
- dgap_init_globals();
+ pr_info("For the tools package please visit http://www.digi.com\n";);
- dgap_NumBoards = 0;
+ /*
+* Register our base character device into the kernel.
+* This allows the download daemon to connect to the downld device
+* before any of the boards are init'ed.
+*/
- pr_info("For the tools package please visit
http://www.digi.com\n";);
+ /*
+* Register management/dpa devices
+*/
+ rc = register_chrdev(DIGI_DGAP_MAJOR, "dgap", &DgapBoardFops);
+ if (rc < 0)
+ return rc;
- /*
-* Register our base character device into the kernel.
-* This allows the download daemon to connect to the downld
device
-* before any of the boards are init'ed.
-*/
- if (!dgap_Major_Control_Registered) {
- /*
-* Register management/dpa devices
-*/
- rc = register_chrdev(DIGI_DGAP_MAJOR, "dgap",
&DgapBoardFops);
- if (rc < 0)
- return rc;
-
- dgap_class = class_create(THIS_MODULE, "dgap_mgmt");
- device_create(dgap_class, NULL,
- MKDEV(DIGI_DGAP_MAJOR, 0),
- NULL, "dgap_mgmt");
- dgap_Major_Control_Registered = TRUE;
- }
+ dgap_class = class_create(THIS_MODULE, "dgap_mgmt");
+ device_create(dgap_class, NULL,
+ MKDEV(DIGI_DGAP_MAJOR, 0),
+ NULL, "dgap_mgmt");
- /*
-* Init any global tty stuff.
-*/
- rc = dgap_tty_preinit();
+ /*
+* Init any global tty stuff.
+*/
+ rc = dgap_tty_preinit();
- if (rc < 0)
- return rc;
+ if (rc < 0)
+ return rc;
- /* Start the poller */
- DGAP_LOCK(dgap_poll_lock, flags);
- init_timer(&dgap_poll_timer);
- dgap_poll_timer.function = dgap_poll_handler;
- dgap_poll_timer.data = 0;
- dgap_poll_time = jiffies + dgap_jiffies_from_ms(dgap_poll_tick);
- dgap_poll_timer.expires = dgap_poll_time;
- DGAP_UNLOCK(dgap_poll_lock, flags);
+ /* Start the poller */
+ DGAP_LOCK(dgap_poll_lock, flags);
+ init_timer(&dgap_poll_timer);
+ dgap_poll_timer.function = dgap_poll_handler;
+ dgap_poll_timer.data = 0;
+ dgap_poll_time = jiffies + dgap_jiffies_from_ms(dgap_poll_tick);
+ dgap_poll_timer.expires = dgap_poll_time;
+ DGAP_UNLOCK(dgap_poll_lock, flags);
- add_timer(&dgap_poll_timer);
+ add_timer(&dgap_poll_timer);
- dgap_driver_state = DRIVER_NEED_CONFIG_LOAD;
- }
+ dgap_driver_state = DRIVER_NEED_CONFIG_LOAD;
return rc;
}
@@ -677,13 +667,10 @@ void dgap_cleanup_module(void)
dgap_remove_driver_sysfiles(&dgap_driver);
-
- if (dgap_Major_Control_Registered) {
- device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 0));
- device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 1));
- cla
[PATCH 3/3] staging: dgap: fix error handling in dgap_init_module()
No need to call pci_unregister_driver() if pci_register_driver() failed.
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/dgap/dgap.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index b4157d7..510e8b3 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -535,18 +535,13 @@ int dgap_init_module(void)
* If something went wrong in the scan, bail out of driver.
*/
if (rc < 0) {
- /* Only unregister the pci driver if it was actually
registered. */
- if (dgap_NumBoards)
- pci_unregister_driver(&dgap_driver);
- else
- printk("WARNING: dgap driver load failed. No DGAP
boards found.\n");
-
dgap_cleanup_module();
- } else {
- dgap_create_driver_sysfiles(&dgap_driver);
- dgap_driver_state = DRIVER_READY;
+ return rc;
}
+ dgap_create_driver_sysfiles(&dgap_driver);
+ dgap_driver_state = DRIVER_READY;
+
return rc;
}
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: dgap: implement proper error handling in dgap_start()
dgap_start() ignored errors in class_create() and device_create().
The patch implements proper error handling.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/dgap/dgap.c | 22 --
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index 5271856..b4157d7 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -557,6 +557,7 @@ static int dgap_start(void)
{
int rc = 0;
unsigned long flags;
+ struct device *device;
/*
* make sure that the globals are
@@ -582,9 +583,18 @@ static int dgap_start(void)
return rc;
dgap_class = class_create(THIS_MODULE, "dgap_mgmt");
- device_create(dgap_class, NULL,
+ if (IS_ERR(dgap_class)) {
+ rc = PTR_ERR(dgap_class);
+ goto failed_class;
+ }
+
+ device = device_create(dgap_class, NULL,
MKDEV(DIGI_DGAP_MAJOR, 0),
NULL, "dgap_mgmt");
+ if (IS_ERR(device)) {
+ rc = PTR_ERR(device);
+ goto failed_device;
+ }
/*
* Init any global tty stuff.
@@ -592,7 +602,7 @@ static int dgap_start(void)
rc = dgap_tty_preinit();
if (rc < 0)
- return rc;
+ goto failed_tty;
/* Start the poller */
DGAP_LOCK(dgap_poll_lock, flags);
@@ -608,6 +618,14 @@ static int dgap_start(void)
dgap_driver_state = DRIVER_NEED_CONFIG_LOAD;
return rc;
+
+failed_tty:
+ device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 0));
+failed_device:
+ class_destroy(dgap_class);
+failed_class:
+ unregister_chrdev(DIGI_DGAP_MAJOR, "dgap");
+ return rc;
}
/*
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: dgap: remove unneeded status variables
dgap_driver_start and dgap_Major_Control_Registered are used
to keep status of initialization of the driver as a whole and its "Major
Control".
But the code that checks them is executed once on module init/unload.
That makes no sense in these variables as far as their values are predictable
at any time.
Also "dgap_downld" device was removed, while
device_destroy(MKDEV(DIGI_DGAP_MAJOR, 1)) is still in dgap_cleanup_module().
The patch removes it by the way.
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/dgap/dgap.c | 81 ++---
1 file changed, 33 insertions(+), 48 deletions(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index d00283a..ad5afbc 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -254,9 +254,6 @@ static int dgap_poll_tick = 20; /* Poll interval - 20
ms */
/*
* Static vars.
*/
-static int dgap_Major_Control_Registered = FALSE;
-static uint dgap_driver_start = FALSE;
-
static struct class *dgap_class;
static struct board_t *dgap_BoardsByMajor[256];
@@ -551,52 +548,44 @@ static int dgap_start(void)
int rc = 0;
unsigned long flags;
- if (dgap_driver_start == FALSE) {
+ /*
+* make sure that the globals are
+* init'd before we do anything else
+*/
+ dgap_init_globals();
- dgap_driver_start = TRUE;
+ dgap_NumBoards = 0;
- /*
-* make sure that the globals are
-* init'd before we do anything else
-*/
- dgap_init_globals();
+ pr_info("For the tools package please visit http://www.digi.com\n";);
- dgap_NumBoards = 0;
+ /*
+* Register our base character device into the kernel.
+*/
- pr_info("For the tools package please visit
http://www.digi.com\n";);
+ /*
+* Register management/dpa devices
+*/
+ rc = register_chrdev(DIGI_DGAP_MAJOR, "dgap", &DgapBoardFops);
+ if (rc < 0)
+ return rc;
- /*
-* Register our base character device into the kernel.
-*/
- if (!dgap_Major_Control_Registered) {
- /*
-* Register management/dpa devices
-*/
- rc = register_chrdev(DIGI_DGAP_MAJOR, "dgap",
-&DgapBoardFops);
- if (rc < 0)
- return rc;
-
- dgap_class = class_create(THIS_MODULE, "dgap_mgmt");
- device_create(dgap_class, NULL,
- MKDEV(DIGI_DGAP_MAJOR, 0),
- NULL, "dgap_mgmt");
- dgap_Major_Control_Registered = TRUE;
- }
+ dgap_class = class_create(THIS_MODULE, "dgap_mgmt");
+ device_create(dgap_class, NULL,
+ MKDEV(DIGI_DGAP_MAJOR, 0),
+ NULL, "dgap_mgmt");
- /* Start the poller */
- DGAP_LOCK(dgap_poll_lock, flags);
- init_timer(&dgap_poll_timer);
- dgap_poll_timer.function = dgap_poll_handler;
- dgap_poll_timer.data = 0;
- dgap_poll_time = jiffies + dgap_jiffies_from_ms(dgap_poll_tick);
- dgap_poll_timer.expires = dgap_poll_time;
- DGAP_UNLOCK(dgap_poll_lock, flags);
+ /* Start the poller */
+ DGAP_LOCK(dgap_poll_lock, flags);
+ init_timer(&dgap_poll_timer);
+ dgap_poll_timer.function = dgap_poll_handler;
+ dgap_poll_timer.data = 0;
+ dgap_poll_time = jiffies + dgap_jiffies_from_ms(dgap_poll_tick);
+ dgap_poll_timer.expires = dgap_poll_time;
+ DGAP_UNLOCK(dgap_poll_lock, flags);
- add_timer(&dgap_poll_timer);
+ add_timer(&dgap_poll_timer);
- dgap_driver_state = DRIVER_NEED_CONFIG_LOAD;
- }
+ dgap_driver_state = DRIVER_NEED_CONFIG_LOAD;
return rc;
}
@@ -658,13 +647,9 @@ static void dgap_cleanup_module(void)
dgap_remove_driver_sysfiles(&dgap_driver);
-
- if (dgap_Major_Control_Registered) {
- device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 0));
- device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 1));
- class_destroy(dgap_class);
- unregister_chrdev(DIGI_DGAP_MAJOR, "dgap");
- }
+ device_destroy(dgap_class, MKDEV(DIGI_DGAP_MAJOR, 0));
+ class_destroy(dgap_class);
+ unregister_chrdev(DIGI_DGAP_MAJOR, "dgap");
kfree(dgap_config_buf);
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: dgap: implement proper error handling in dgap_start()
dgap_start() ignored errors in class_create() and device_create().
The patch implements proper error handling.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/dgap/dgap.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/dgap/dgap.c b/drivers/staging/dgap/dgap.c
index ad5afbc..bfafe7e 100644
--- a/drivers/staging/dgap/dgap.c
+++ b/drivers/staging/dgap/dgap.c
@@ -547,6 +547,7 @@ static int dgap_start(void)
{
int rc = 0;
unsigned long flags;
+ struct device *device;
/*
* make sure that the globals are
@@ -570,9 +571,18 @@ static int dgap_start(void)
return rc;
dgap_class = class_create(THIS_MODULE, "dgap_mgmt");
- device_create(dgap_class, NULL,
+ if (IS_ERR(dgap_class)) {
+ rc = PTR_ERR(dgap_class);
+ goto failed_class;
+ }
+
+ device = device_create(dgap_class, NULL,
MKDEV(DIGI_DGAP_MAJOR, 0),
NULL, "dgap_mgmt");
+ if (IS_ERR(device)) {
+ rc = PTR_ERR(device);
+ goto failed_device;
+ }
/* Start the poller */
DGAP_LOCK(dgap_poll_lock, flags);
@@ -588,6 +598,12 @@ static int dgap_start(void)
dgap_driver_state = DRIVER_NEED_CONFIG_LOAD;
return rc;
+
+failed_device:
+ class_destroy(dgap_class);
+failed_class:
+ unregister_chrdev(DIGI_DGAP_MAJOR, "dgap");
+ return rc;
}
/*
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: iio: ad7152: Fix deadlock in ad7152_write_raw_samp_freq()
ad7152_write_raw_samp_freq() is called by ad7152_write_raw() with
chip->state_lock held. So, there is unavoidable deadlock when
ad7152_write_raw_samp_freq() locks the mutex itself.
The patch removes unneeded locking.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/iio/cdc/ad7152.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/iio/cdc/ad7152.c b/drivers/staging/iio/cdc/ad7152.c
index dc6ecd824365..ff10d1f0a7e4 100644
--- a/drivers/staging/iio/cdc/ad7152.c
+++ b/drivers/staging/iio/cdc/ad7152.c
@@ -231,16 +231,12 @@ static int ad7152_write_raw_samp_freq(struct device *dev,
int val)
if (i >= ARRAY_SIZE(ad7152_filter_rate_table))
i = ARRAY_SIZE(ad7152_filter_rate_table) - 1;
- mutex_lock(&chip->state_lock);
ret = i2c_smbus_write_byte_data(chip->client,
AD7152_REG_CFG2, AD7152_CFG2_OSR(i));
- if (ret < 0) {
- mutex_unlock(&chip->state_lock);
+ if (ret < 0)
return ret;
- }
chip->filter_rate_setup = i;
- mutex_unlock(&chip->state_lock);
return ret;
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [media] lirc_imon: do not leave imon_probe() with mutex held
Commit af8a819a2513 ("[media] lirc_imon: simplify error handling code")
lost mutex_unlock(&context->ctx_lock), so imon_probe() exits with
the context->ctx_lock mutex acquired.
The patch adds mutex_unlock(&context->ctx_lock) back.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
Fixes: af8a819a2513 ("[media] lirc_imon: simplify error handling code")
---
drivers/staging/media/lirc/lirc_imon.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/media/lirc/lirc_imon.c
b/drivers/staging/media/lirc/lirc_imon.c
index 534b8103ae80..ff1926ca1f96 100644
--- a/drivers/staging/media/lirc/lirc_imon.c
+++ b/drivers/staging/media/lirc/lirc_imon.c
@@ -885,12 +885,14 @@ static int imon_probe(struct usb_interface *interface,
vendor, product, ifnum, usbdev->bus->busnum, usbdev->devnum);
/* Everything went fine. Just unlock and return retval (with is 0) */
+ mutex_unlock(&context->ctx_lock);
goto driver_unlock;
unregister_lirc:
lirc_unregister_driver(driver->minor);
free_tx_urb:
+ mutex_unlock(&context->ctx_lock);
usb_free_urb(tx_urb);
free_rx_urb:
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: remove an extra space
drivers/staging/rtl8188eu/core/rtw_wlan_util.c:1377 check_assoc_AP() warn:
inconsistent indenting
Signed-off-by: Alexey Tulia
---
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 59b4432..aba090c 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -1374,7 +1374,7 @@ unsigned char check_assoc_AP(u8 *pframe, uint len)
epigram_vendor_flag = 1;
if (ralink_vendor_flag) {
DBG_88E("link to Tenda W311R AP\n");
-return HT_IOT_PEER_TENDA;
+ return HT_IOT_PEER_TENDA;
} else {
DBG_88E("Capture EPIGRAM_OUI\n");
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6656: fix definitions of DEVICE_FLAGS_* flags
test_bit and set_bit take the bit number to operate on, rather than a
mask. This patch fixes the DEVICE_FLAGS_* definitions so that they represent
the bit index in priv->flags as opposed to the mask returned by the
BIT macro.
Signed-off-by: Alexey Tulia
---
drivers/staging/vt6656/device.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vt6656/device.h b/drivers/staging/vt6656/device.h
index dec36f2..cfdd727 100644
--- a/drivers/staging/vt6656/device.h
+++ b/drivers/staging/vt6656/device.h
@@ -262,8 +262,8 @@ enum {
};
/* flags for options */
-#define DEVICE_FLAGS_UNPLUGBIT(0)
-#define DEVICE_FLAGS_DISCONNECTED BIT(1)
+#define DEVICE_FLAGS_UNPLUG0
+#define DEVICE_FLAGS_DISCONNECTED 1
struct vnt_private {
/* mac80211 */
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wilc1000: fix double mutex_unlock on failure path in wilc_wlan_cleanup()
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.
The patch adds return in case of failure.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/wilc1000/wilc_wlan.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/wilc1000/wilc_wlan.c
b/drivers/staging/wilc1000/wilc_wlan.c
index c02665747705..cd7f52a51173 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -1703,12 +1703,14 @@ void wilc_wlan_cleanup(struct net_device *dev)
if (!ret) {
PRINT_ER("Error while reading reg\n");
release_bus(RELEASE_ALLOW_SLEEP);
+ return;
}
PRINT_ER("Writing ABORT reg\n");
ret = p->hif_func.hif_write_reg(WILC_GP_REG_0, (reg | ABORT_INT));
if (!ret) {
PRINT_ER("Error while writing reg\n");
release_bus(RELEASE_ALLOW_SLEEP);
+ return;
}
release_bus(RELEASE_ALLOW_SLEEP);
/**
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wilc1000: fix double mutex_unlock on failure path in wilc_wlan_cleanup()
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(-)
diff --git a/drivers/staging/wilc1000/wilc_wlan.c
b/drivers/staging/wilc1000/wilc_wlan.c
index a73e99f..4b7c8e9 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -1459,15 +1459,16 @@ void wilc_wlan_cleanup(struct net_device *dev)
ret = p->hif_func.hif_read_reg(wilc, WILC_GP_REG_0, ®);
if (!ret) {
PRINT_ER("Error while reading reg\n");
- release_bus(wilc, RELEASE_ALLOW_SLEEP);
+ goto _unlock;
}
PRINT_ER("Writing ABORT reg\n");
ret = p->hif_func.hif_write_reg(wilc, WILC_GP_REG_0,
(reg | ABORT_INT));
if (!ret) {
PRINT_ER("Error while writing reg\n");
- release_bus(wilc, RELEASE_ALLOW_SLEEP);
+ goto _unlock;
}
+_unlock:
release_bus(wilc, RELEASE_ALLOW_SLEEP);
p->hif_func.hif_deinit(NULL);
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] staging: wilc1000: fix double mutex_unlock on failure path in wilc_wlan_cleanup()
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(-)
diff --git a/drivers/staging/wilc1000/wilc_wlan.c
b/drivers/staging/wilc1000/wilc_wlan.c
index 83af51b..b8c4a63 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -1381,15 +1381,16 @@ void wilc_wlan_cleanup(struct net_device *dev)
ret = wilc->hif_func->hif_read_reg(wilc, WILC_GP_REG_0, ®);
if (!ret) {
PRINT_ER("Error while reading reg\n");
- release_bus(wilc, RELEASE_ALLOW_SLEEP);
+ goto unlock;
}
PRINT_ER("Writing ABORT reg\n");
ret = wilc->hif_func->hif_write_reg(wilc, WILC_GP_REG_0,
(reg | ABORT_INT));
if (!ret) {
PRINT_ER("Error while writing reg\n");
- release_bus(wilc, RELEASE_ALLOW_SLEEP);
+ goto unlock;
}
+unlock:
release_bus(wilc, RELEASE_ALLOW_SLEEP);
wilc->hif_func->hif_deinit(NULL);
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] added fbtft ssd1305 controller support
Signed-off-by: Alexey Mednyy
---
drivers/staging/fbtft/Kconfig | 6 +
drivers/staging/fbtft/Makefile | 1 +
drivers/staging/fbtft/fb_ssd1305.c | 271 +
3 files changed, 278 insertions(+)
create mode 100644 drivers/staging/fbtft/fb_ssd1305.c
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 883ff5b..0bfc776 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -117,6 +117,12 @@ config FB_TFT_SSD1289
help
Framebuffer support for SSD1289
+config FB_TFT_SSD1305
+tristate "FB driver for the SSD1305 OLED Controller"
+depends on FB_TFT
+help
+ Framebuffer support for SSD1305
+
config FB_TFT_SSD1306
tristate "FB driver for the SSD1306 OLED Controller"
depends on FB_TFT
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 4f9071d..1ddc764 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_FB_TFT_RA8875) += fb_ra8875.o
obj-$(CONFIG_FB_TFT_S6D02A1) += fb_s6d02a1.o
obj-$(CONFIG_FB_TFT_S6D1121) += fb_s6d1121.o
obj-$(CONFIG_FB_TFT_SSD1289) += fb_ssd1289.o
+obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1305.o
obj-$(CONFIG_FB_TFT_SSD1306) += fb_ssd1306.o
obj-$(CONFIG_FB_TFT_SSD1331) += fb_ssd1331.o
obj-$(CONFIG_FB_TFT_SSD1351) += fb_ssd1351.o
diff --git a/drivers/staging/fbtft/fb_ssd1305.c
b/drivers/staging/fbtft/fb_ssd1305.c
new file mode 100644
index 000..2f65fb8
--- /dev/null
+++ b/drivers/staging/fbtft/fb_ssd1305.c
@@ -0,0 +1,271 @@
+/*
+ * FB driver for the SSD1305 OLED Controller
+ *
+ * based on SSD1306 driver by Noralf Tronnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "fbtft.h"
+
+#define DRVNAME"fb_ssd1305"
+#define WIDTH 128
+#define HEIGHT 64
+
+/*
+ *
+ *
+ * write_reg() caveat:
+ *
+ *This doesn't work because D/C has to be LOW for both values:
+ * write_reg(par, val1, val2);
+ *
+ *Do it like this:
+ * write_reg(par, val1);
+ * write_reg(par, val2);
+*/
+
+/* Init sequence taken from the Adafruit SSD1306 Arduino library */
+static int init_display(struct fbtft_par *par)
+{
+ par->fbtftops.reset(par);
+
+ if (par->gamma.curves[0] == 0) {
+ mutex_lock(&par->gamma.lock);
+ if (par->info->var.yres == 64)
+ par->gamma.curves[0] = 0xCF;
+ else
+ par->gamma.curves[0] = 0x8F;
+ mutex_unlock(&par->gamma.lock);
+ }
+
+ /* Set Display OFF */
+ write_reg(par, 0xAE);
+
+ /* Set Display Clock Divide Ratio/ Oscillator Frequency */
+ write_reg(par, 0xD5);
+ write_reg(par, 0x80);
+
+ /* Set Multiplex Ratio */
+ write_reg(par, 0xA8);
+ if (par->info->var.yres == 64)
+ write_reg(par, 0x3F);
+ else
+ write_reg(par, 0x1F);
+
+ /* Set Display Offset */
+ write_reg(par, 0xD3);
+ write_reg(par, 0x0);
+
+ /* Set Display Start Line */
+ write_reg(par, 0x40 | 0x0);
+
+ /* Charge Pump Setting */
+ write_reg(par, 0x8D);
+ /* A[2] = 1b, Enable charge pump during display on */
+ write_reg(par, 0x14);
+
+ /* Set Memory Addressing Mode */
+ write_reg(par, 0x20);
+ /* Vertical addressing mode */
+ write_reg(par, 0x01);
+
+ /*Set Segment Re-map */
+ /* column address 127 is mapped to SEG0 */
+ write_reg(par, 0xA0 | ((par->info->var.rotate == 180) ? 0x0 : 0x1));
+
+ /* Set COM Output Scan Direction */
+ /* remapped mode. Scan from COM[N-1] to COM0 */
+ write_reg(par, ((par->info->var.rotate == 180) ? 0xC8 : 0xC0));
+
+ /* Set COM Pins Hardware Configuration */
+ write_reg(par, 0xDA);
+ if (par->info->var.yres == 64)
+ /* A[4]=1b, Alternative COM pin configuration */
+ write_reg(par, 0x12);
+ else
+ /* A[4]=0b, Sequential COM pin configuration */
+ write_reg(par, 0x02);
+
+ /* Set Pre-charge Period */
+ write_reg(par, 0xD9);
+ write_reg(par, 0xF1);
+
+ /* Set VCOMH Deselect Level */
+ write_reg(par, 0xDB);
+ /* according to th
[PATCH] add fbtft ssd1325 controller support
Signed-off-by: Alexey Mednyy
---
drivers/staging/fbtft/Kconfig | 6 +
drivers/staging/fbtft/Makefile | 1 +
drivers/staging/fbtft/fb_ssd1325.c | 266 +
3 files changed, 273 insertions(+)
create mode 100644 drivers/staging/fbtft/fb_ssd1325.c
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 0bfc776..6f5e824 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -129,6 +129,12 @@ config FB_TFT_SSD1306
help
Framebuffer support for SSD1306
+config FB_TFT_SSD1325
+tristate "FB driver for the SSD1325 OLED Controller"
+depends on FB_TFT
+help
+ Framebuffer support for SSD1305
+
config FB_TFT_SSD1331
tristate "FB driver for the SSD1331 LCD Controller"
depends on FB_TFT
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 1ddc764..2725ea9 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_FB_TFT_S6D1121) += fb_s6d1121.o
obj-$(CONFIG_FB_TFT_SSD1289) += fb_ssd1289.o
obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1305.o
obj-$(CONFIG_FB_TFT_SSD1306) += fb_ssd1306.o
+obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1325.o
obj-$(CONFIG_FB_TFT_SSD1331) += fb_ssd1331.o
obj-$(CONFIG_FB_TFT_SSD1351) += fb_ssd1351.o
obj-$(CONFIG_FB_TFT_ST7735R) += fb_st7735r.o
diff --git a/drivers/staging/fbtft/fb_ssd1325.c
b/drivers/staging/fbtft/fb_ssd1325.c
new file mode 100644
index 000..cdb96be
--- /dev/null
+++ b/drivers/staging/fbtft/fb_ssd1325.c
@@ -0,0 +1,266 @@
+/*
+ * FB driver for the SSD1325 OLED Controller
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "fbtft.h"
+
+#define DRVNAME"fb_ssd1325"
+#define WIDTH 128
+#define HEIGHT 64
+
+#define GAMMA_NUM 1
+#define GAMMA_LEN 15
+#define DEFAULT_GAMMA "7 1 1 1 1 2 2 3 3 4 4 5 5 6 6"
+
+/*
+ * write_reg() caveat:
+ *
+ *This doesn't work because D/C has to be LOW for both values:
+ * write_reg(par, val1, val2);
+ *
+ *Do it like this:
+ * write_reg(par, val1);
+ * write_reg(par, val2);
+ */
+
+/* Init sequence taken from the Adafruit SSD1306 Arduino library */
+static int init_display(struct fbtft_par *par)
+{
+ par->fbtftops.reset(par);
+
+ gpio_set_value(par->gpio.cs, 0);
+
+ write_reg(par, 0xb3);
+ write_reg(par, 0xf0);
+ write_reg(par, 0xae);
+ write_reg(par, 0xa1);
+ write_reg(par, 0x00);
+ write_reg(par, 0xa8);
+ write_reg(par, 0x3f);
+ write_reg(par, 0xa0);
+ write_reg(par, 0x45);
+ write_reg(par, 0xa2);
+ write_reg(par, 0x40);
+ write_reg(par, 0x75);
+ write_reg(par, 0x00);
+ write_reg(par, 0x3f);
+ write_reg(par, 0x15);
+ write_reg(par, 0x00);
+ write_reg(par, 0x7f);
+ write_reg(par, 0xa4);
+ write_reg(par, 0xaf);
+
+ return 0;
+}
+
+static uint8_t rgb565_to_g16(uint16_t pixel)
+{
+ uint16_t b = pixel & 0x1f;
+ uint16_t g = (pixel & (0x3f << 5)) >> 5;
+ uint16_t r = (pixel & (0x1f << (5 + 6))) >> (5 + 6);
+
+ pixel = (299 * r + 587 * g + 114 * b) / 195;
+ if (pixel > 255)
+ pixel = 255;
+ return (uint8_t) pixel / 16;
+}
+
+/* Check if all necessary GPIOS defined */
+static int verify_gpios(struct fbtft_par *par)
+{
+ int i;
+
+ dev_dbg(par->info->device, "%s()\n", __func__);
+
+ if (par->gpio.wr < 0) {
+ dev_err(par->info->device,
+ "Missing info about 'wr' (aka E) gpio. Aborting.\n");
+ return -EINVAL;
+ }
+ for (i = 0; i < 8; ++i) {
+ if (par->gpio.db[i] < 0) {
+ dev_err(par->info->device,
+ "Missing info about 'db[%i]' gpio. Aborting.\n",
+ i);
+ return -EINVAL;
+ }
+ }
+ if (par->gpio.dc < 0) {
+ dev_err(par->info->device,
+ "Missing info about 'dc' gpio. Aborting.\n");
+ return -EINVAL;
+ }
+ if (par-&g
Re: [PATCH] added fbtft ssd1305 controller support
On 01/26/2016 05:00 PM, Dan Carpenter wrote:
The subject should be:
[PATCH] Staging: fbtft: added ssd1305 controller support
On Tue, Jan 26, 2016 at 04:07:24PM +0300, Alexey Mednyy wrote:
Signed-off-by: Alexey Mednyy
---
drivers/staging/fbtft/Kconfig | 6 +
drivers/staging/fbtft/Makefile | 1 +
drivers/staging/fbtft/fb_ssd1305.c | 271 +
3 files changed, 278 insertions(+)
create mode 100644 drivers/staging/fbtft/fb_ssd1305.c
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 883ff5b..0bfc776 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -117,6 +117,12 @@ config FB_TFT_SSD1289
help
Framebuffer support for SSD1289
+config FB_TFT_SSD1305
+tristate "FB driver for the SSD1305 OLED Controller"
+depends on FB_TFT
+help
+ Framebuffer support for SSD1305
Maybe say who the vendor is or where this is used or something.
Checkpatch is supposed to complain about this kind of stuff. :(
I just followed other descriptions above and below mine, every other
fbtft drivers have this short description. Do I really need longer
description?
+
config FB_TFT_SSD1306
tristate "FB driver for the SSD1306 OLED Controller"
depends on FB_TFT
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 4f9071d..1ddc764 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_FB_TFT_RA8875) += fb_ra8875.o
obj-$(CONFIG_FB_TFT_S6D02A1) += fb_s6d02a1.o
obj-$(CONFIG_FB_TFT_S6D1121) += fb_s6d1121.o
obj-$(CONFIG_FB_TFT_SSD1289) += fb_ssd1289.o
+obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1305.o
obj-$(CONFIG_FB_TFT_SSD1306) += fb_ssd1306.o
obj-$(CONFIG_FB_TFT_SSD1331) += fb_ssd1331.o
obj-$(CONFIG_FB_TFT_SSD1351) += fb_ssd1351.o
diff --git a/drivers/staging/fbtft/fb_ssd1305.c
b/drivers/staging/fbtft/fb_ssd1305.c
new file mode 100644
index 000..2f65fb8
--- /dev/null
+++ b/drivers/staging/fbtft/fb_ssd1305.c
@@ -0,0 +1,271 @@
+/*
+ * FB driver for the SSD1305 OLED Controller
+ *
+ * based on SSD1306 driver by Noralf Tronnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "fbtft.h"
+
+#define DRVNAME"fb_ssd1305"
+#define WIDTH 128
+#define HEIGHT 64
WIDTH and HEIGHT don't add anything. Just say ".width = 128,".
+
+/*
+ *
+ *
+ * write_reg() caveat:
+ *
+ *This doesn't work because D/C has to be LOW for both values:
+ * write_reg(par, val1, val2);
+ *
+ *Do it like this:
+ * write_reg(par, val1);
+ * write_reg(par, val2);
Clean up this comment a bit.
+*/
+
+/* Init sequence taken from the Adafruit SSD1306 Arduino library */
+static int init_display(struct fbtft_par *par)
+{
+ par->fbtftops.reset(par);
+
+ if (par->gamma.curves[0] == 0) {
+ mutex_lock(&par->gamma.lock);
+ if (par->info->var.yres == 64)
+ par->gamma.curves[0] = 0xCF;
+ else
+ par->gamma.curves[0] = 0x8F;
+ mutex_unlock(&par->gamma.lock);
+ }
+
+ /* Set Display OFF */
+ write_reg(par, 0xAE);
+
+ /* Set Display Clock Divide Ratio/ Oscillator Frequency */
+ write_reg(par, 0xD5);
+ write_reg(par, 0x80);
+
+ /* Set Multiplex Ratio */
+ write_reg(par, 0xA8);
+ if (par->info->var.yres == 64)
+ write_reg(par, 0x3F);
+ else
+ write_reg(par, 0x1F);
+
+ /* Set Display Offset */
+ write_reg(par, 0xD3);
+ write_reg(par, 0x0);
+
+ /* Set Display Start Line */
+ write_reg(par, 0x40 | 0x0);
+
+ /* Charge Pump Setting */
+ write_reg(par, 0x8D);
+ /* A[2] = 1b, Enable charge pump during display on */
+ write_reg(par, 0x14);
+
+ /* Set Memory Addressing Mode */
+ write_reg(par, 0x20);
+ /* Vertical addressing mode */
+ write_reg(par, 0x01);
+
+ /*Set Segment Re-map */
+ /* column address 127 is mapped to SEG0 */
+ write_reg(par, 0xA0 | ((par->info->var.rotate == 180) ? 0x0 : 0x1));
+
+ /* Set COM Output Scan Direction */
+ /* remapped mode. Scan from COM[N-1] to COM0 */
+
Re: [PATCH] add fbtft ssd1325 controller support
On 01/26/2016 05:09 PM, Dan Carpenter wrote:
Same stuff. Run checkpatch.pl.
+ gpio_set_value(par->gpio.dc, 1);
+ /* Write data */
+
+ ret =
+ par->fbtftops.write(par, par->txbuf.buf,
+ par->info->var.xres * par->info->var.yres / 2);
Clean that line.
+static struct fbtft_display display = {
+ .regwidth = 8,
+ .width = WIDTH,
+ .height = HEIGHT,
+ .txbuflen = WIDTH * HEIGHT / 2,
The other driver didn't have a txbuflen. What does that do? (I have
barely looked at fbtft before so I don't know).
fbtft core creates 16bpp framebuffer, and default
txbuflen = display->width * display->height * bpp / 8;
so setting txbuflen saves some space in my case, I have 4bpp display.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] added fbtft ssd1305 controller support
Signed-off-by: Alexey Mednyy
---
drivers/staging/fbtft/Kconfig | 6 ++
drivers/staging/fbtft/Makefile | 1 +
drivers/staging/fbtft/fb_ssd1305.c | 216 +
3 files changed, 223 insertions(+)
create mode 100644 drivers/staging/fbtft/fb_ssd1305.c
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 883ff5b..0bfc776 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -117,6 +117,12 @@ config FB_TFT_SSD1289
help
Framebuffer support for SSD1289
+config FB_TFT_SSD1305
+tristate "FB driver for the SSD1305 OLED Controller"
+depends on FB_TFT
+help
+ Framebuffer support for SSD1305
+
config FB_TFT_SSD1306
tristate "FB driver for the SSD1306 OLED Controller"
depends on FB_TFT
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 4f9071d..1ddc764 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_FB_TFT_RA8875) += fb_ra8875.o
obj-$(CONFIG_FB_TFT_S6D02A1) += fb_s6d02a1.o
obj-$(CONFIG_FB_TFT_S6D1121) += fb_s6d1121.o
obj-$(CONFIG_FB_TFT_SSD1289) += fb_ssd1289.o
+obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1305.o
obj-$(CONFIG_FB_TFT_SSD1306) += fb_ssd1306.o
obj-$(CONFIG_FB_TFT_SSD1331) += fb_ssd1331.o
obj-$(CONFIG_FB_TFT_SSD1351) += fb_ssd1351.o
diff --git a/drivers/staging/fbtft/fb_ssd1305.c
b/drivers/staging/fbtft/fb_ssd1305.c
new file mode 100644
index 000..4b38c3f
--- /dev/null
+++ b/drivers/staging/fbtft/fb_ssd1305.c
@@ -0,0 +1,216 @@
+/*
+ * FB driver for the SSD1305 OLED Controller
+ *
+ * based on SSD1306 driver by Noralf Tronnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "fbtft.h"
+
+#define DRVNAME"fb_ssd1305"
+
+#define WIDTH 128
+#define HEIGHT 64
+
+/*
+ * write_reg() caveat:
+ *
+ *This doesn't work because D/C has to be LOW for both values:
+ * write_reg(par, val1, val2);
+ *
+ *Do it like this:
+ * write_reg(par, val1);
+ * write_reg(par, val2);
+ */
+
+/* Init sequence taken from the Adafruit SSD1306 Arduino library */
+static int init_display(struct fbtft_par *par)
+{
+ par->fbtftops.reset(par);
+
+ if (par->gamma.curves[0] == 0) {
+ mutex_lock(&par->gamma.lock);
+ if (par->info->var.yres == 64)
+ par->gamma.curves[0] = 0xCF;
+ else
+ par->gamma.curves[0] = 0x8F;
+ mutex_unlock(&par->gamma.lock);
+ }
+
+ /* Set Display OFF */
+ write_reg(par, 0xAE);
+
+ /* Set Display Clock Divide Ratio/ Oscillator Frequency */
+ write_reg(par, 0xD5);
+ write_reg(par, 0x80);
+
+ /* Set Multiplex Ratio */
+ write_reg(par, 0xA8);
+ if (par->info->var.yres == 64)
+ write_reg(par, 0x3F);
+ else
+ write_reg(par, 0x1F);
+
+ /* Set Display Offset */
+ write_reg(par, 0xD3);
+ write_reg(par, 0x0);
+
+ /* Set Display Start Line */
+ write_reg(par, 0x40 | 0x0);
+
+ /* Charge Pump Setting */
+ write_reg(par, 0x8D);
+ /* A[2] = 1b, Enable charge pump during display on */
+ write_reg(par, 0x14);
+
+ /* Set Memory Addressing Mode */
+ write_reg(par, 0x20);
+ /* Vertical addressing mode */
+ write_reg(par, 0x01);
+
+ /*
+* Set Segment Re-map
+* column address 127 is mapped to SEG0
+*/
+ write_reg(par, 0xA0 | ((par->info->var.rotate == 180) ? 0x0 : 0x1));
+
+ /*
+* Set COM Output Scan Direction
+* remapped mode. Scan from COM[N-1] to COM0
+*/
+ write_reg(par, ((par->info->var.rotate == 180) ? 0xC8 : 0xC0));
+
+ /* Set COM Pins Hardware Configuration */
+ write_reg(par, 0xDA);
+ if (par->info->var.yres == 64) {
+ /* A[4]=1b, Alternative COM pin configuration */
+ write_reg(par, 0x12);
+ } else {
+ /* A[4]=0b, Sequential COM pin configuration */
+ write_reg(par, 0x02);
+ }
+
+ /* Set Pre-charge Period */
+ write_reg(par, 0xD9);
+ write_reg(par, 0xF1);
+
+ /*
+* Entire Display ON
+
[PATCH v2] add fbtft ssd1325 controller support
Signed-off-by: Alexey Mednyy
---
drivers/staging/fbtft/Kconfig | 6 ++
drivers/staging/fbtft/Makefile | 1 +
drivers/staging/fbtft/fb_ssd1325.c | 205 +
3 files changed, 212 insertions(+)
create mode 100644 drivers/staging/fbtft/fb_ssd1325.c
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 0bfc776..6f5e824 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -129,6 +129,12 @@ config FB_TFT_SSD1306
help
Framebuffer support for SSD1306
+config FB_TFT_SSD1325
+tristate "FB driver for the SSD1325 OLED Controller"
+depends on FB_TFT
+help
+ Framebuffer support for SSD1305
+
config FB_TFT_SSD1331
tristate "FB driver for the SSD1331 LCD Controller"
depends on FB_TFT
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 1ddc764..2725ea9 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_FB_TFT_S6D1121) += fb_s6d1121.o
obj-$(CONFIG_FB_TFT_SSD1289) += fb_ssd1289.o
obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1305.o
obj-$(CONFIG_FB_TFT_SSD1306) += fb_ssd1306.o
+obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1325.o
obj-$(CONFIG_FB_TFT_SSD1331) += fb_ssd1331.o
obj-$(CONFIG_FB_TFT_SSD1351) += fb_ssd1351.o
obj-$(CONFIG_FB_TFT_ST7735R) += fb_st7735r.o
diff --git a/drivers/staging/fbtft/fb_ssd1325.c
b/drivers/staging/fbtft/fb_ssd1325.c
new file mode 100644
index 000..15078bf
--- /dev/null
+++ b/drivers/staging/fbtft/fb_ssd1325.c
@@ -0,0 +1,205 @@
+/*
+ * FB driver for the SSD1325 OLED Controller
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "fbtft.h"
+
+#define DRVNAME"fb_ssd1325"
+
+#define WIDTH 128
+#define HEIGHT 64
+#define GAMMA_NUM 1
+#define GAMMA_LEN 15
+#define DEFAULT_GAMMA "7 1 1 1 1 2 2 3 3 4 4 5 5 6 6"
+
+/*
+ * write_reg() caveat:
+ *
+ *This doesn't work because D/C has to be LOW for both values:
+ * write_reg(par, val1, val2);
+ *
+ *Do it like this:
+ * write_reg(par, val1);
+ * write_reg(par, val2);
+ */
+
+/* Init sequence taken from the Adafruit SSD1306 Arduino library */
+static int init_display(struct fbtft_par *par)
+{
+ par->fbtftops.reset(par);
+
+ gpio_set_value(par->gpio.cs, 0);
+
+ write_reg(par, 0xb3);
+ write_reg(par, 0xf0);
+ write_reg(par, 0xae);
+ write_reg(par, 0xa1);
+ write_reg(par, 0x00);
+ write_reg(par, 0xa8);
+ write_reg(par, 0x3f);
+ write_reg(par, 0xa0);
+ write_reg(par, 0x45);
+ write_reg(par, 0xa2);
+ write_reg(par, 0x40);
+ write_reg(par, 0x75);
+ write_reg(par, 0x00);
+ write_reg(par, 0x3f);
+ write_reg(par, 0x15);
+ write_reg(par, 0x00);
+ write_reg(par, 0x7f);
+ write_reg(par, 0xa4);
+ write_reg(par, 0xaf);
+
+ return 0;
+}
+
+static uint8_t rgb565_to_g16(u16 pixel)
+{
+ u16 b = pixel & 0x1f;
+ u16 g = (pixel & (0x3f << 5)) >> 5;
+ u16 r = (pixel & (0x1f << (5 + 6))) >> (5 + 6);
+
+ pixel = (299 * r + 587 * g + 114 * b) / 195;
+ if (pixel > 255)
+ pixel = 255;
+ return (uint8_t)pixel / 16;
+}
+
+static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
+{
+ fbtft_par_dbg(DEBUG_SET_ADDR_WIN, par,
+ "%s(xs=%d, ys=%d, xe=%d, ye=%d)\n", __func__, xs, ys, xe,
+ ye);
+
+ write_reg(par, 0x75);
+ write_reg(par, 0x00);
+ write_reg(par, 0x3f);
+ write_reg(par, 0x15);
+ write_reg(par, 0x00);
+ write_reg(par, 0x7f);
+}
+
+static int blank(struct fbtft_par *par, bool on)
+{
+ fbtft_par_dbg(DEBUG_BLANK, par, "%s(blank=%s)\n",
+ __func__, on ? "true" : "false");
+
+ if (on)
+ write_reg(par, 0xAE);
+ else
+ write_reg(par, 0xAF);
+ return 0;
+}
+
+/*
+ * Grayscale Lookup Table
+ * GS1 - GS15
+ * The "Gamma curve" contains the relative values between the entries
+ * in the Lookup table.
+ *
+ * 0 = Setting of GS1 < Setting of GS2 < Setting of GS3.<
+ * Setting of GS14 < Setting of GS15
+ */
+static int set_g
[PATCH v3] Staging: fbtft: add ssd1325 controller support
g of GS1 < Setting of GS2 < Setting of GS3.<
+ * Setting of GS14 < Setting of GS15
+ */
+static int set_gamma(struct fbtft_par *par, unsigned long *curves)
+{
+ int i;
+
+ fbtft_par_dbg(DEBUG_INIT_DISPLAY, par, "%s()\n", __func__);
+
+ for (i = 0; i < GAMMA_LEN; i++) {
+ if (i > 0 && curves[i] < 1) {
+ dev_err(par->info->device,
+ "Illegal value in Grayscale Lookup Table at
index %d.\n"
+ "Must be greater than 0\n", i);
+ return -EINVAL;
+ }
+ if (curves[i] > 7) {
+ dev_err(par->info->device,
+ "Illegal value(s) in Grayscale Lookup Table.\n"
+ "At index=%d, the accumulated value has
exceeded 7\n",
+ i);
+ return -EINVAL;
+ }
+ }
+ write_reg(par, 0xB8);
+ for (i = 0; i < 8; i++)
+ write_reg(par, (curves[i] & 0xFF));
+ return 0;
+}
+
+static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
+{
+ u16 *vmem16 = (u16 *)par->info->screen_buffer;
+ u8 *buf = par->txbuf.buf;
+ u8 n1;
+ u8 n2;
+ int y, x;
+ int ret;
+
+ for (x = 0; x < par->info->var.xres; x++) {
+ if (x % 2)
+ continue;
+ for (y = 0; y < par->info->var.yres; y++) {
+ n1 = rgb565_to_g16(vmem16[y * par->info->var.xres + x]);
+ n2 = rgb565_to_g16(vmem16
+ [y * par->info->var.xres + x + 1]);
+ *buf = (n1 << 4) | n2;
+ buf++;
+ }
+ }
+
+ gpio_set_value(par->gpio.dc, 1);
+
+ /* Write data */
+ ret = par->fbtftops.write(par, par->txbuf.buf,
+ par->info->var.xres * par->info->var.yres / 2);
+ if (ret < 0)
+ dev_err(par->info->device,
+ "%s: write failed and returned: %d\n", __func__, ret);
+
+ return ret;
+}
+
+static struct fbtft_display display = {
+ .regwidth = 8,
+ .width = WIDTH,
+ .height = HEIGHT,
+ .txbuflen = WIDTH * HEIGHT / 2,
+ .gamma_num = GAMMA_NUM,
+ .gamma_len = GAMMA_LEN,
+ .gamma = DEFAULT_GAMMA,
+ .fbtftops = {
+ .write_vmem = write_vmem,
+ .init_display = init_display,
+ .set_addr_win = set_addr_win,
+ .blank = blank,
+ .set_gamma = set_gamma,
+ },
+};
+
+FBTFT_REGISTER_DRIVER(DRVNAME, "solomon,ssd1325", &display);
+
+MODULE_ALIAS("spi:" DRVNAME);
+MODULE_ALIAS("platform:" DRVNAME);
+MODULE_ALIAS("spi:ssd1325");
+MODULE_ALIAS("platform:ssd1325");
+
+MODULE_DESCRIPTION("SSD1325 OLED Driver");
+MODULE_AUTHOR("Alexey Mednyy");
+MODULE_LICENSE("GPL");
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3] Staging: fbtft: add ssd1305 controller support
par, 0xD9);
+ write_reg(par, 0xF1);
+
+ /*
+* Entire Display ON
+* Resume to RAM content display. Output follows RAM content
+*/
+ write_reg(par, 0xA4);
+
+ /*
+* Set Normal Display
+* 0 in RAM: OFF in display panel
+* 1 in RAM: ON in display panel
+*/
+ write_reg(par, 0xA6);
+
+ /* Set Display ON */
+ write_reg(par, 0xAF);
+
+ return 0;
+}
+
+static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
+{
+ /* Set Lower Column Start Address for Page Addressing Mode */
+ write_reg(par, 0x00 | ((par->info->var.rotate == 180) ? 0x0 : 0x4));
+ /* Set Higher Column Start Address for Page Addressing Mode */
+ write_reg(par, 0x10 | 0x0);
+ /* Set Display Start Line */
+ write_reg(par, 0x40 | 0x0);
+}
+
+static int blank(struct fbtft_par *par, bool on)
+{
+ if (on)
+ write_reg(par, 0xAE);
+ else
+ write_reg(par, 0xAF);
+ return 0;
+}
+
+/* Gamma is used to control Contrast */
+static int set_gamma(struct fbtft_par *par, unsigned long *curves)
+{
+ curves[0] &= 0xFF;
+ /* Set Contrast Control for BANK0 */
+ write_reg(par, 0x81);
+ write_reg(par, curves[0]);
+
+ return 0;
+}
+
+static int write_vmem(struct fbtft_par *par, size_t offset, size_t len)
+{
+ u16 *vmem16 = (u16 *)par->info->screen_buffer;
+ u8 *buf = par->txbuf.buf;
+ int x, y, i;
+ int ret;
+
+ for (x = 0; x < par->info->var.xres; x++) {
+ for (y = 0; y < par->info->var.yres / 8; y++) {
+ *buf = 0x00;
+ for (i = 0; i < 8; i++)
+ *buf |= (vmem16[(y * 8 + i) *
+ par->info->var.xres + x] ?
+1 : 0) << i;
+ buf++;
+ }
+ }
+
+ /* Write data */
+ gpio_set_value(par->gpio.dc, 1);
+ ret = par->fbtftops.write(par, par->txbuf.buf,
+ par->info->var.xres * par->info->var.yres /
+ 8);
+ if (ret < 0)
+ dev_err(par->info->device, "write failed and returned: %d\n",
+ ret);
+ return ret;
+}
+
+static struct fbtft_display display = {
+ .regwidth = 8,
+ .width = WIDTH,
+ .height = HEIGHT,
+ .txbuflen = WIDTH * HEIGHT / 8,
+ .gamma_num = 1,
+ .gamma_len = 1,
+ .gamma = "00",
+ .fbtftops = {
+ .write_vmem = write_vmem,
+ .init_display = init_display,
+ .set_addr_win = set_addr_win,
+ .blank = blank,
+ .set_gamma = set_gamma,
+ },
+};
+
+FBTFT_REGISTER_DRIVER(DRVNAME, "solomon,ssd1305", &display);
+
+MODULE_ALIAS("spi:" DRVNAME);
+MODULE_ALIAS("platform:" DRVNAME);
+MODULE_ALIAS("spi:ssd1305");
+MODULE_ALIAS("platform:ssd1305");
+
+MODULE_DESCRIPTION("SSD1305 OLED Driver");
+MODULE_AUTHOR("Alexey Mednyy");
+MODULE_LICENSE("GPL");
--
2.5.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] add fbtft ssd1325 controller support
On 01/27/2016 02:36 AM, Greg KH wrote: > On Tue, Jan 26, 2016 at 07:31:16PM +0300, Alexey Mednyy wrote: >> Signed-off-by: Alexey Mednyy > Why? Sorry, that was required by some other project I sent patches to. > > I can't take patches without any changelog entries, sorry, we need some > more information here about what this is, why it's being submitted, etc. > > thanks, > > greg k-h -- _____ Best regards, Mednyy Alexey. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4] Staging: fbtft: add ssd1325 controller support
That patch adds support for SSD1325 controller.
That is 4bpp grayscale OLED display controller present in several
displays eq: Winstar WEX012864
Signed-off-by: Alexey Mednyy
---
drivers/staging/fbtft/Kconfig | 6 ++
drivers/staging/fbtft/Makefile | 1 +
drivers/staging/fbtft/fb_ssd1325.c | 205 +
3 files changed, 212 insertions(+)
create mode 100644 drivers/staging/fbtft/fb_ssd1325.c
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 0bfc776..6f5e824 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -129,6 +129,12 @@ config FB_TFT_SSD1306
help
Framebuffer support for SSD1306
+config FB_TFT_SSD1325
+tristate "FB driver for the SSD1325 OLED Controller"
+depends on FB_TFT
+help
+ Framebuffer support for SSD1305
+
config FB_TFT_SSD1331
tristate "FB driver for the SSD1331 LCD Controller"
depends on FB_TFT
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 1ddc764..2725ea9 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_FB_TFT_S6D1121) += fb_s6d1121.o
obj-$(CONFIG_FB_TFT_SSD1289) += fb_ssd1289.o
obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1305.o
obj-$(CONFIG_FB_TFT_SSD1306) += fb_ssd1306.o
+obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1325.o
obj-$(CONFIG_FB_TFT_SSD1331) += fb_ssd1331.o
obj-$(CONFIG_FB_TFT_SSD1351) += fb_ssd1351.o
obj-$(CONFIG_FB_TFT_ST7735R) += fb_st7735r.o
diff --git a/drivers/staging/fbtft/fb_ssd1325.c
b/drivers/staging/fbtft/fb_ssd1325.c
new file mode 100644
index 000..15078bf
--- /dev/null
+++ b/drivers/staging/fbtft/fb_ssd1325.c
@@ -0,0 +1,205 @@
+/*
+ * FB driver for the SSD1325 OLED Controller
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "fbtft.h"
+
+#define DRVNAME"fb_ssd1325"
+
+#define WIDTH 128
+#define HEIGHT 64
+#define GAMMA_NUM 1
+#define GAMMA_LEN 15
+#define DEFAULT_GAMMA "7 1 1 1 1 2 2 3 3 4 4 5 5 6 6"
+
+/*
+ * write_reg() caveat:
+ *
+ *This doesn't work because D/C has to be LOW for both values:
+ * write_reg(par, val1, val2);
+ *
+ *Do it like this:
+ * write_reg(par, val1);
+ * write_reg(par, val2);
+ */
+
+/* Init sequence taken from the Adafruit SSD1306 Arduino library */
+static int init_display(struct fbtft_par *par)
+{
+ par->fbtftops.reset(par);
+
+ gpio_set_value(par->gpio.cs, 0);
+
+ write_reg(par, 0xb3);
+ write_reg(par, 0xf0);
+ write_reg(par, 0xae);
+ write_reg(par, 0xa1);
+ write_reg(par, 0x00);
+ write_reg(par, 0xa8);
+ write_reg(par, 0x3f);
+ write_reg(par, 0xa0);
+ write_reg(par, 0x45);
+ write_reg(par, 0xa2);
+ write_reg(par, 0x40);
+ write_reg(par, 0x75);
+ write_reg(par, 0x00);
+ write_reg(par, 0x3f);
+ write_reg(par, 0x15);
+ write_reg(par, 0x00);
+ write_reg(par, 0x7f);
+ write_reg(par, 0xa4);
+ write_reg(par, 0xaf);
+
+ return 0;
+}
+
+static uint8_t rgb565_to_g16(u16 pixel)
+{
+ u16 b = pixel & 0x1f;
+ u16 g = (pixel & (0x3f << 5)) >> 5;
+ u16 r = (pixel & (0x1f << (5 + 6))) >> (5 + 6);
+
+ pixel = (299 * r + 587 * g + 114 * b) / 195;
+ if (pixel > 255)
+ pixel = 255;
+ return (uint8_t)pixel / 16;
+}
+
+static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
+{
+ fbtft_par_dbg(DEBUG_SET_ADDR_WIN, par,
+ "%s(xs=%d, ys=%d, xe=%d, ye=%d)\n", __func__, xs, ys, xe,
+ ye);
+
+ write_reg(par, 0x75);
+ write_reg(par, 0x00);
+ write_reg(par, 0x3f);
+ write_reg(par, 0x15);
+ write_reg(par, 0x00);
+ write_reg(par, 0x7f);
+}
+
+static int blank(struct fbtft_par *par, bool on)
+{
+ fbtft_par_dbg(DEBUG_BLANK, par, "%s(blank=%s)\n",
+ __func__, on ? "true" : "false");
+
+ if (on)
+ write_reg(par, 0xAE);
+ else
+ write_reg(par, 0xAF);
+ return 0;
+}
+
+/*
+ * Grayscale Lookup Table
+ * GS1 - GS15
+ * The "Gamma curve" contains the relative values between the entries
+ * in the Lo
[PATCH v4] Staging: fbtft: add ssd1305 controller support
That patch adds support for SSD1305 controller.
That is monochrome OLED display controller present in several
displays eq: Winstar WEX012864
Signed-off-by: Alexey Mednyy
---
drivers/staging/fbtft/Kconfig | 6 ++
drivers/staging/fbtft/Makefile | 1 +
drivers/staging/fbtft/fb_ssd1305.c | 216 +
3 files changed, 223 insertions(+)
create mode 100644 drivers/staging/fbtft/fb_ssd1305.c
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 883ff5b..0bfc776 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -117,6 +117,12 @@ config FB_TFT_SSD1289
help
Framebuffer support for SSD1289
+config FB_TFT_SSD1305
+tristate "FB driver for the SSD1305 OLED Controller"
+depends on FB_TFT
+help
+ Framebuffer support for SSD1305
+
config FB_TFT_SSD1306
tristate "FB driver for the SSD1306 OLED Controller"
depends on FB_TFT
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index 4f9071d..1ddc764 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_FB_TFT_RA8875) += fb_ra8875.o
obj-$(CONFIG_FB_TFT_S6D02A1) += fb_s6d02a1.o
obj-$(CONFIG_FB_TFT_S6D1121) += fb_s6d1121.o
obj-$(CONFIG_FB_TFT_SSD1289) += fb_ssd1289.o
+obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1305.o
obj-$(CONFIG_FB_TFT_SSD1306) += fb_ssd1306.o
obj-$(CONFIG_FB_TFT_SSD1331) += fb_ssd1331.o
obj-$(CONFIG_FB_TFT_SSD1351) += fb_ssd1351.o
diff --git a/drivers/staging/fbtft/fb_ssd1305.c
b/drivers/staging/fbtft/fb_ssd1305.c
new file mode 100644
index 000..4b38c3f
--- /dev/null
+++ b/drivers/staging/fbtft/fb_ssd1305.c
@@ -0,0 +1,216 @@
+/*
+ * FB driver for the SSD1305 OLED Controller
+ *
+ * based on SSD1306 driver by Noralf Tronnes
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * 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. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "fbtft.h"
+
+#define DRVNAME"fb_ssd1305"
+
+#define WIDTH 128
+#define HEIGHT 64
+
+/*
+ * write_reg() caveat:
+ *
+ *This doesn't work because D/C has to be LOW for both values:
+ * write_reg(par, val1, val2);
+ *
+ *Do it like this:
+ * write_reg(par, val1);
+ * write_reg(par, val2);
+ */
+
+/* Init sequence taken from the Adafruit SSD1306 Arduino library */
+static int init_display(struct fbtft_par *par)
+{
+ par->fbtftops.reset(par);
+
+ if (par->gamma.curves[0] == 0) {
+ mutex_lock(&par->gamma.lock);
+ if (par->info->var.yres == 64)
+ par->gamma.curves[0] = 0xCF;
+ else
+ par->gamma.curves[0] = 0x8F;
+ mutex_unlock(&par->gamma.lock);
+ }
+
+ /* Set Display OFF */
+ write_reg(par, 0xAE);
+
+ /* Set Display Clock Divide Ratio/ Oscillator Frequency */
+ write_reg(par, 0xD5);
+ write_reg(par, 0x80);
+
+ /* Set Multiplex Ratio */
+ write_reg(par, 0xA8);
+ if (par->info->var.yres == 64)
+ write_reg(par, 0x3F);
+ else
+ write_reg(par, 0x1F);
+
+ /* Set Display Offset */
+ write_reg(par, 0xD3);
+ write_reg(par, 0x0);
+
+ /* Set Display Start Line */
+ write_reg(par, 0x40 | 0x0);
+
+ /* Charge Pump Setting */
+ write_reg(par, 0x8D);
+ /* A[2] = 1b, Enable charge pump during display on */
+ write_reg(par, 0x14);
+
+ /* Set Memory Addressing Mode */
+ write_reg(par, 0x20);
+ /* Vertical addressing mode */
+ write_reg(par, 0x01);
+
+ /*
+* Set Segment Re-map
+* column address 127 is mapped to SEG0
+*/
+ write_reg(par, 0xA0 | ((par->info->var.rotate == 180) ? 0x0 : 0x1));
+
+ /*
+* Set COM Output Scan Direction
+* remapped mode. Scan from COM[N-1] to COM0
+*/
+ write_reg(par, ((par->info->var.rotate == 180) ? 0xC8 : 0xC0));
+
+ /* Set COM Pins Hardware Configuration */
+ write_reg(par, 0xDA);
+ if (par->info->var.yres == 64) {
+ /* A[4]=1b, Alternative COM pin configuration */
+ write_reg(par, 0x12);
+ } else {
+ /* A[4]=0b, Sequential COM pin configuration */
+ write_reg(par, 0x02);
+
[PATCH] staging: vt6656: don't return zero on failure path in vt6656_probe()
If ieee80211_alloc_hw() fails in vt6656_probe(), it breaks off
initialization, but returns zero.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/vt6656/main_usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/vt6656/main_usb.c
b/drivers/staging/vt6656/main_usb.c
index 71adc1f61838..ab3ab84cb0a7 100644
--- a/drivers/staging/vt6656/main_usb.c
+++ b/drivers/staging/vt6656/main_usb.c
@@ -963,6 +963,7 @@ vt6656_probe(struct usb_interface *intf, const struct
usb_device_id *id)
hw = ieee80211_alloc_hw(sizeof(struct vnt_private), &vnt_mac_ops);
if (!hw) {
dev_err(&udev->dev, "could not register ieee80211_hw\n");
+ rc = -ENOMEM;
goto err_nomem;
}
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ozwpan: implement error handling in ozwpan_init()
Errors are correctly handled in oz_cdev_register() and oz_protocol_init(),
but then they are ignored in ozwpan_init().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/ozwpan/ozmain.c | 14 --
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/ozwpan/ozmain.c b/drivers/staging/ozwpan/ozmain.c
index 7d6ef4cadf1a..74ef34815b98 100644
--- a/drivers/staging/ozwpan/ozmain.c
+++ b/drivers/staging/ozwpan/ozmain.c
@@ -34,11 +34,21 @@ MODULE_PARM_DESC(g_net_dev, "The device(s) to bind to; "
*/
static int __init ozwpan_init(void)
{
- oz_cdev_register();
- oz_protocol_init(g_net_dev);
+ int err;
+
+ err = oz_cdev_register();
+ if (err)
+ return err;
+ err = oz_protocol_init(g_net_dev);
+ if (err)
+ goto err_protocol;
oz_app_enable(OZ_APPID_USB, 1);
oz_apps_init();
return 0;
+
+err_protocol:
+ oz_cdev_deregister();
+ return err;
}
/*
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: r8188eu: Add _enter_critical_mutex() error handling
_enter_critical_mutex() is a simple call to mutex_lock_interruptible(),
but there is no error handling code for it.
The patch removes wrapper _enter_critical_mutex() and
adds error handling for mutex_lock_interruptible().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 5 +++--
drivers/staging/rtl8188eu/include/osdep_service.h | 9 -
drivers/staging/rtl8188eu/os_dep/os_intfs.c | 3 ++-
drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 5 -
4 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index 935b48eef8b1..4e9312f0e902 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -271,7 +271,8 @@ static s32 dump_mgntframe_and_wait_ack(struct adapter
*padapter,
if (padapter->bSurpriseRemoved || padapter->bDriverStopped)
return -1;
- _enter_critical_mutex(&pxmitpriv->ack_tx_mutex, NULL);
+ if (!mutex_lock_interruptible(&pxmitpriv->ack_tx_mutex))
+ return _FAIL;
pxmitpriv->ack_tx = true;
pmgntframe->ack_report = 1;
@@ -282,7 +283,7 @@ static s32 dump_mgntframe_and_wait_ack(struct adapter
*padapter,
pxmitpriv->ack_tx = false;
mutex_unlock(&pxmitpriv->ack_tx_mutex);
-return ret;
+ return ret;
}
static int update_hidden_ssid(u8 *ies, u32 ies_len, u8 hidden_ssid_mode)
diff --git a/drivers/staging/rtl8188eu/include/osdep_service.h
b/drivers/staging/rtl8188eu/include/osdep_service.h
index cf9ca685eb77..96505a6dbe2c 100644
--- a/drivers/staging/rtl8188eu/include/osdep_service.h
+++ b/drivers/staging/rtl8188eu/include/osdep_service.h
@@ -67,15 +67,6 @@ static inline struct list_head *get_list_head(struct __queue
*queue)
return &(queue->queue);
}
-static inline int _enter_critical_mutex(struct mutex *pmutex,
- unsigned long *pirqL)
-{
- int ret;
-
- ret = mutex_lock_interruptible(pmutex);
- return ret;
-}
-
static inline int rtw_netif_queue_stopped(struct net_device *pnetdev)
{
return netif_tx_queue_stopped(netdev_get_tx_queue(pnetdev, 0)) &&
diff --git a/drivers/staging/rtl8188eu/os_dep/os_intfs.c
b/drivers/staging/rtl8188eu/os_dep/os_intfs.c
index 2361bce480c3..9a3425a7110d 100644
--- a/drivers/staging/rtl8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8188eu/os_dep/os_intfs.c
@@ -1053,7 +1053,8 @@ static int netdev_open(struct net_device *pnetdev)
int ret;
struct adapter *padapter = (struct adapter *)rtw_netdev_priv(pnetdev);
- _enter_critical_mutex(&padapter->hw_init_mutex, NULL);
+ if (mutex_lock_interruptible(&padapter->hw_init_mutex))
+ return -ERESTARTSYS;
ret = _netdev_open(pnetdev);
mutex_unlock(&padapter->hw_init_mutex);
return ret;
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
index 7e599bc5b2d3..0fea338d7313 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
@@ -249,7 +249,10 @@ static int usbctrl_vendorreq(struct adapter *adapt, u8
request, u16 value, u16 i
goto exit;
}
- _enter_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL);
+ if (mutex_lock_interruptible(&dvobjpriv->usb_vendor_req_mutex)) {
+ status = -ERESTARTSYS;
+ goto exit;
+ }
/* Acquire IO memory for vendorreq */
pIo_buf = dvobjpriv->usb_vendor_req_buf;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: r8188eu: Add _enter_critical_mutex() error handling
_enter_critical_mutex() is a simple call to mutex_lock_interruptible(),
but there is no error handling code for it.
The patch removes wrapper _enter_critical_mutex() and
adds error handling for mutex_lock_interruptible().
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 5 +++--
drivers/staging/rtl8188eu/include/osdep_service.h | 9 -
drivers/staging/rtl8188eu/os_dep/os_intfs.c | 3 ++-
drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 5 -
4 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index 935b48eef8b1..4e9312f0e902 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -271,7 +271,8 @@ static s32 dump_mgntframe_and_wait_ack(struct adapter
*padapter,
if (padapter->bSurpriseRemoved || padapter->bDriverStopped)
return -1;
- _enter_critical_mutex(&pxmitpriv->ack_tx_mutex, NULL);
+ if (mutex_lock_interruptible(&pxmitpriv->ack_tx_mutex))
+ return _FAIL;
pxmitpriv->ack_tx = true;
pmgntframe->ack_report = 1;
@@ -282,7 +283,7 @@ static s32 dump_mgntframe_and_wait_ack(struct adapter
*padapter,
pxmitpriv->ack_tx = false;
mutex_unlock(&pxmitpriv->ack_tx_mutex);
-return ret;
+ return ret;
}
static int update_hidden_ssid(u8 *ies, u32 ies_len, u8 hidden_ssid_mode)
diff --git a/drivers/staging/rtl8188eu/include/osdep_service.h
b/drivers/staging/rtl8188eu/include/osdep_service.h
index cf9ca685eb77..96505a6dbe2c 100644
--- a/drivers/staging/rtl8188eu/include/osdep_service.h
+++ b/drivers/staging/rtl8188eu/include/osdep_service.h
@@ -67,15 +67,6 @@ static inline struct list_head *get_list_head(struct __queue
*queue)
return &(queue->queue);
}
-static inline int _enter_critical_mutex(struct mutex *pmutex,
- unsigned long *pirqL)
-{
- int ret;
-
- ret = mutex_lock_interruptible(pmutex);
- return ret;
-}
-
static inline int rtw_netif_queue_stopped(struct net_device *pnetdev)
{
return netif_tx_queue_stopped(netdev_get_tx_queue(pnetdev, 0)) &&
diff --git a/drivers/staging/rtl8188eu/os_dep/os_intfs.c
b/drivers/staging/rtl8188eu/os_dep/os_intfs.c
index 2361bce480c3..9a3425a7110d 100644
--- a/drivers/staging/rtl8188eu/os_dep/os_intfs.c
+++ b/drivers/staging/rtl8188eu/os_dep/os_intfs.c
@@ -1053,7 +1053,8 @@ static int netdev_open(struct net_device *pnetdev)
int ret;
struct adapter *padapter = (struct adapter *)rtw_netdev_priv(pnetdev);
- _enter_critical_mutex(&padapter->hw_init_mutex, NULL);
+ if (mutex_lock_interruptible(&padapter->hw_init_mutex))
+ return -ERESTARTSYS;
ret = _netdev_open(pnetdev);
mutex_unlock(&padapter->hw_init_mutex);
return ret;
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
index 7e599bc5b2d3..0fea338d7313 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
@@ -249,7 +249,10 @@ static int usbctrl_vendorreq(struct adapter *adapt, u8
request, u16 value, u16 i
goto exit;
}
- _enter_critical_mutex(&dvobjpriv->usb_vendor_req_mutex, NULL);
+ if (mutex_lock_interruptible(&dvobjpriv->usb_vendor_req_mutex)) {
+ status = -ERESTARTSYS;
+ goto exit;
+ }
/* Acquire IO memory for vendorreq */
pIo_buf = dvobjpriv->usb_vendor_req_buf;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[BUG] IB/hfi1: might sleep under spinlock in hfi1_ioctl()
Hello, hfi1_ioctl() contains many calls to might sleep functions with dd->hfi1_snoop.snoop_lock spinlock held (for example, access_ok, copy_from_user, kzalloc(GFP_KERNEL), etc.). Should dd->hfi1_snoop.snoop_lock be acquired just before updating state? Found by Linux Driver Verification project (linuxtesting.org). -- Alexey Khoroshilov Linux Verification Center, ISPRAS web: http://linuxtesting.org ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: gdm7240: alloc_mux_rx() does not need GFP_ATOMIC
As far as alloc_mux_rx() is called from probe() only
there is no need in GFP_ATOMIC here.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/gdm724x/gdm_mux.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/gdm724x/gdm_mux.c
b/drivers/staging/gdm724x/gdm_mux.c
index 5b1ef40..5221cf9 100644
--- a/drivers/staging/gdm724x/gdm_mux.c
+++ b/drivers/staging/gdm724x/gdm_mux.c
@@ -96,12 +96,12 @@ static struct mux_rx *alloc_mux_rx(void)
{
struct mux_rx *r = NULL;
- r = kzalloc(sizeof(struct mux_rx), GFP_ATOMIC);
+ r = kzalloc(sizeof(struct mux_rx), GFP_KERNEL);
if (!r)
return NULL;
- r->urb = usb_alloc_urb(0, GFP_ATOMIC);
- r->buf = kmalloc(MUX_RX_MAX_SIZE, GFP_ATOMIC);
+ r->urb = usb_alloc_urb(0, GFP_KERNEL);
+ r->buf = kmalloc(MUX_RX_MAX_SIZE, GFP_KERNEL);
if (!r->urb || !r->buf) {
usb_free_urb(r->urb);
kfree(r->buf);
--
1.8.1.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: gdm7240: fix memory leak on failure path
init_usb() may fail after some of mux_rxes already allocated. So we need to release them on the failure path. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov --- drivers/staging/gdm724x/gdm_mux.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/gdm724x/gdm_mux.c b/drivers/staging/gdm724x/gdm_mux.c index 5221cf9..69813c7 100644 --- a/drivers/staging/gdm724x/gdm_mux.c +++ b/drivers/staging/gdm724x/gdm_mux.c @@ -541,7 +541,7 @@ static int gdm_mux_probe(struct usb_interface *intf, const struct usb_device_id ret = init_usb(mux_dev); if (ret) - goto err_free_tty; + goto err_free_usb; tty_dev->priv_dev = (void *)mux_dev; tty_dev->send_func = gdm_mux_send; @@ -565,8 +565,8 @@ static int gdm_mux_probe(struct usb_interface *intf, const struct usb_device_id err_unregister_tty: unregister_lte_tty_device(tty_dev); +err_free_usb: release_usb(mux_dev); -err_free_tty: kfree(tty_dev); err_free_mux: kfree(mux_dev); -- 1.8.1.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: gdm724x: fix leak at failure path in gdm_usb_probe()
Error handling code in gdm_usb_probe() deallocates all resources,
but calls usb_get_dev(usbdev) and returns error code after that.
The patch fixes it and, by the way, several other issues:
- no need to use GFP_ATOMIC in probe();
- return -ENODEV instead of -1;
- kmalloc+memset -> kzalloc
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/gdm724x/gdm_usb.c | 40 +--
1 file changed, 17 insertions(+), 23 deletions(-)
diff --git a/drivers/staging/gdm724x/gdm_usb.c
b/drivers/staging/gdm724x/gdm_usb.c
index 781134af69d1..33458a583142 100644
--- a/drivers/staging/gdm724x/gdm_usb.c
+++ b/drivers/staging/gdm724x/gdm_usb.c
@@ -830,24 +830,19 @@ static int gdm_usb_probe(struct usb_interface *intf,
const struct usb_device_id
if (bInterfaceNumber > NETWORK_INTERFACE) {
pr_info("not a network device\n");
- return -1;
+ return -ENODEV;
}
- phy_dev = kmalloc(sizeof(struct phy_dev), GFP_ATOMIC);
- if (!phy_dev) {
- ret = -ENOMEM;
- goto out;
- }
+ phy_dev = kzalloc(sizeof(struct phy_dev), GFP_KERNEL);
+ if (!phy_dev)
+ return -ENOMEM;
- udev = kmalloc(sizeof(struct lte_udev), GFP_ATOMIC);
+ udev = kzalloc(sizeof(struct lte_udev), GFP_KERNEL);
if (!udev) {
ret = -ENOMEM;
- goto out;
+ goto err_udev;
}
- memset(phy_dev, 0, sizeof(struct phy_dev));
- memset(udev, 0, sizeof(struct lte_udev));
-
phy_dev->priv_dev = (void *)udev;
phy_dev->send_hci_func = gdm_usb_hci_send;
phy_dev->send_sdu_func = gdm_usb_sdu_send;
@@ -858,7 +853,7 @@ static int gdm_usb_probe(struct usb_interface *intf, const
struct usb_device_id
ret = init_usb(udev);
if (ret < 0) {
pr_err("init_usb func failed\n");
- goto out;
+ goto err_init_usb;
}
udev->intf = intf;
@@ -875,23 +870,22 @@ static int gdm_usb_probe(struct usb_interface *intf,
const struct usb_device_id
ret = request_mac_address(udev);
if (ret < 0) {
pr_err("request Mac address failed\n");
- goto out;
+ goto err_mac_address;
}
start_rx_proc(phy_dev);
-out:
-
- if (ret < 0) {
- kfree(phy_dev);
- if (udev) {
- release_usb(udev);
- kfree(udev);
- }
- }
-
usb_get_dev(usbdev);
usb_set_intfdata(intf, phy_dev);
+ return 0;
+
+err_mac_address:
+ release_usb(udev);
+err_init_usb:
+ kfree(udev);
+err_udev:
+ kfree(phy_dev);
+
return ret;
}
--
1.8.1.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [media] go7007-loader: fix usb_dev leak
There is usb_get_dev() in go7007_loader_probe(),
but there is no usb_put_dev() anywhere.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/media/go7007/go7007-loader.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/media/go7007/go7007-loader.c
b/drivers/staging/media/go7007/go7007-loader.c
index f846ad5819dc..2b15952e4c18 100644
--- a/drivers/staging/media/go7007/go7007-loader.c
+++ b/drivers/staging/media/go7007/go7007-loader.c
@@ -60,7 +60,7 @@ static int go7007_loader_probe(struct usb_interface
*interface,
if (usbdev->descriptor.bNumConfigurations != 1) {
dev_err(&interface->dev, "can't handle multiple config\n");
- return -ENODEV;
+ goto failed2;
}
vendor = le16_to_cpu(usbdev->descriptor.idVendor);
@@ -109,6 +109,7 @@ static int go7007_loader_probe(struct usb_interface
*interface,
return 0;
failed2:
+ usb_put_dev(usbdev);
dev_err(&interface->dev, "probe failed\n");
return -ENODEV;
}
@@ -116,6 +117,7 @@ failed2:
static void go7007_loader_disconnect(struct usb_interface *interface)
{
dev_info(&interface->dev, "disconnect\n");
+ usb_put_dev(interface_to_usbdev(interface));
usb_set_intfdata(interface, NULL);
}
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] [media] as102: fix leaks at failure paths in as102_usb_probe()
Failure handling is incomplete in as102_usb_probe(). The patch implements proper resource deallocations. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov --- drivers/staging/media/as102/as102_usb_drv.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/staging/media/as102/as102_usb_drv.c b/drivers/staging/media/as102/as102_usb_drv.c index 9f275f020150..c1c6152d1ab4 100644 --- a/drivers/staging/media/as102/as102_usb_drv.c +++ b/drivers/staging/media/as102/as102_usb_drv.c @@ -419,15 +419,22 @@ static int as102_usb_probe(struct usb_interface *intf, /* request buffer allocation for streaming */ ret = as102_alloc_usb_stream_buffer(as102_dev); if (ret != 0) - goto failed; + goto failed_stream; /* register dvb layer */ ret = as102_dvb_register(as102_dev); + if (ret != 0) + goto failed_dvb; LEAVE(); return ret; +failed_dvb: + as102_free_usb_stream_buffer(as102_dev); +failed_stream: + usb_deregister_dev(intf, &as102_usb_class_driver); failed: + usb_put_dev(as102_dev->bus_adap.usb_dev); usb_set_intfdata(intf, NULL); kfree(as102_dev); return ret; -- 1.8.3.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: wlan-ng: fix leaks on failure paths in prism2sta_probe_usb()
There are leaks of resources allocated by wlan_setup() and usb_dev refcnt
on failure paths in prism2sta_probe_usb().
The patch adds appropriate deallocations and removes invalid code
from hfa384x_corereset() failure handling.
unregister_wlandev() is wrong because it is not registered yet.
hfa384x_destroy() is just noop in init state.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov
---
drivers/staging/wlan-ng/prism2usb.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/wlan-ng/prism2usb.c
b/drivers/staging/wlan-ng/prism2usb.c
index b401974fb282..4739c14d8359 100644
--- a/drivers/staging/wlan-ng/prism2usb.c
+++ b/drivers/staging/wlan-ng/prism2usb.c
@@ -140,11 +140,9 @@ static int prism2sta_probe_usb(struct usb_interface
*interface,
prism2_reset_holdtime,
prism2_reset_settletime, 0);
if (result != 0) {
- unregister_wlandev(wlandev);
- hfa384x_destroy(hw);
result = -EIO;
dev_err(&interface->dev, "hfa384x_corereset()
failed.\n");
- goto failed;
+ goto failed_reset;
}
}
@@ -159,11 +157,15 @@ static int prism2sta_probe_usb(struct usb_interface
*interface,
if (register_wlandev(wlandev) != 0) {
dev_err(&interface->dev, "register_wlandev() failed.\n");
result = -EIO;
- goto failed;
+ goto failed_register;
}
goto done;
+failed_register:
+ usb_put_dev(dev);
+failed_reset:
+ wlan_unsetup(wlandev);
failed:
kfree(wlandev);
kfree(hw);
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
