Re: Re: [RFCv4 06/11] misc: Introduce Nokia CMT driver
On 16.12.2013 20:34, Sebastian Reichel wrote:
Hi,
On Mon, Dec 16, 2013 at 02:31:53PM +0100, Linus Walleij wrote:
I am very reluctant in letting device trees specify exports of GPIOs
to userspace, not so much because it's Linux-specific but for
the fact that people are doing things in userspace that should not
be done in userspace.
Last time it was proposed I asked to the specific usecase,
exactly why userspace needed this handle on a physical
GPIO line, and why it can't use another userspace interface
(example: leds, keys etc.)
There are a couple of lines ("cmt_apeslpx", "cmt_rst_rq", "cmt_en",
"cmt_rst", "cmt_bsi"), which are handled by ofono to do the correct
power sequence for the modem. The relevant ofono code is here:
https://git.kernel.org/cgit/network/ofono/ofono.git/tree/plugins/nokia-gpio.c
In MeeGo etc. they have a little board specific init script, which
exports the gpio lines and setups some symlinks. IMHO at least the
board specific stuff should be handled by the kernel, thus I added
this code to the driver. I guess you prefer to move the power
sequencing completly to the kernel?
Don't forget there is not only ofono, but rtcom-call-ui and all the
telephony stack in Maemo 5 :). However, power sequencing and control is
specific not only to the modem model, but to the firmware version the
modem is running as well (afaik). IMO you can't simply move the modem
power/reset/sleep control to the kernel and hope for the best, I am not
sure there is enough documentation (if any) for this to be done
reliably, esp on n900 with its BB5 modem. The point is that those gpios
are used not only for the initial power-up, but for control of the modem
state and reset (if needed) during normal usage. The APE reset line is
an example of stuff that can't be moved to the kernel without providing
some interface/feedback to/from the userspace IMO - what if she is
dialing 112 at the moment the modem decides it is too hot and wants a
device reset (or whatever reason there could be for a modem to request a
device reset)? The same goes for the APE sleep request line
(cmt_apeslpx) - based on what should the kernel decide whether to put
the modem in sleep?
Sure, exporting gpios to userspace might not be the best way to achieve
the required functionality, but every way could be argued if it is the
best. And for sure labeling a modem "LED" won't make it such.
Regards,
Ivo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [RFCv4 06/11] misc: Introduce Nokia CMT driver
On 16.12.2013 20:34, Sebastian Reichel wrote: Hi, On Mon, Dec 16, 2013 at 02:31:53PM +0100, Linus Walleij wrote: I am very reluctant in letting device trees specify exports of GPIOs to userspace, not so much because it's Linux-specific but for the fact that people are doing things in userspace that should not be done in userspace. Last time it was proposed I asked to the specific usecase, exactly why userspace needed this handle on a physical GPIO line, and why it can't use another userspace interface (example: leds, keys etc.) There are a couple of lines (cmt_apeslpx, cmt_rst_rq, cmt_en, cmt_rst, cmt_bsi), which are handled by ofono to do the correct power sequence for the modem. The relevant ofono code is here: https://git.kernel.org/cgit/network/ofono/ofono.git/tree/plugins/nokia-gpio.c In MeeGo etc. they have a little board specific init script, which exports the gpio lines and setups some symlinks. IMHO at least the board specific stuff should be handled by the kernel, thus I added this code to the driver. I guess you prefer to move the power sequencing completly to the kernel? Don't forget there is not only ofono, but rtcom-call-ui and all the telephony stack in Maemo 5 :). However, power sequencing and control is specific not only to the modem model, but to the firmware version the modem is running as well (afaik). IMO you can't simply move the modem power/reset/sleep control to the kernel and hope for the best, I am not sure there is enough documentation (if any) for this to be done reliably, esp on n900 with its BB5 modem. The point is that those gpios are used not only for the initial power-up, but for control of the modem state and reset (if needed) during normal usage. The APE reset line is an example of stuff that can't be moved to the kernel without providing some interface/feedback to/from the userspace IMO - what if she is dialing 112 at the moment the modem decides it is too hot and wants a device reset (or whatever reason there could be for a modem to request a device reset)? The same goes for the APE sleep request line (cmt_apeslpx) - based on what should the kernel decide whether to put the modem in sleep? Sure, exporting gpios to userspace might not be the best way to achieve the required functionality, but every way could be argued if it is the best. And for sure labeling a modem LED won't make it such. Regards, Ivo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 15/16] wl1251: Add sysfs file address for setting permanent mac address
On 12.12.2013 21:55, Ben Hutchings wrote: On Wed, 2013-12-11 at 16:53 -0600, Dan Williams wrote: On Wed, 2013-12-11 at 22:15 +, Ben Hutchings wrote: On Wed, 2013-12-11 at 23:35 +0200, Ivajlo Dimitrov wrote: On 11.12.2013 23:17, Ben Hutchings wrote: I think that's an even worse idea. This is not firmware and it already exists in separate storage. I think that rx51_init_wl1251() in arch/arm/mach-omap2/board-rx51-peripherals.c should either copy the MAC address out of NVRAM, or if it's too early to do that, then schedule a function to run later and only then set up wl1251 platform data. Ben. And how will that work with DT when board files will be in the history? 1. Boot loader reads the MAC address from NVRAM and puts it in the DT node. or 2. NVRAM reading is done by a tiny driver that is loaded based on the platform name and updates the DT node in memory. (But I don't know how wl1251 should decide to defer probing if it's probed before that other driver.) I'm uncomfortable with it too, and yes the permanent MAC should really be known before the interface is even registered, but... Imagine if the MAC address for your ethernet device was stored in /etc/my-mac-addr.txt, except that /etc was a read-only protected partition from a very small SSD. That's essentially the N900; it's stored in a file in a normal ext2/ext3 (?) filesystem on a partition of the internal flash. Oh, from my quick look I got the impression that this 'CAL' was stored directly in a specific flash partition. It seems like overkill to write a small driver that duplicates the ext3 + MTD drivers just to read the MAC. [...] I don't see that anything would need to be duplicated. But reading files from the kernel is really ugly, and deciding when to read the file would be another problem (as you noted). Ben Yes, CAL is stored in /dev/mtd1, a dedicated partition in the NAND flash. But reading the data stored in it from the kernel is not a trivial task, as it uses its own pseudo-fs format etc. Not impossible (I RE-ed libcal a while ago [0], so there is a code that could be reused, calvaria works too afaik), but still doesn't sound like a very good idea. [0] https://gitorious.org/community-ssu/libcal/source/d4c5fd9293ddb693c9b032b4c084cd0343b3ea26: Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 15/16] wl1251: Add sysfs file address for setting permanent mac address
On 12.12.2013 21:55, Ben Hutchings wrote: On Wed, 2013-12-11 at 16:53 -0600, Dan Williams wrote: On Wed, 2013-12-11 at 22:15 +, Ben Hutchings wrote: On Wed, 2013-12-11 at 23:35 +0200, Ivajlo Dimitrov wrote: On 11.12.2013 23:17, Ben Hutchings wrote: I think that's an even worse idea. This is not firmware and it already exists in separate storage. I think that rx51_init_wl1251() in arch/arm/mach-omap2/board-rx51-peripherals.c should either copy the MAC address out of NVRAM, or if it's too early to do that, then schedule a function to run later and only then set up wl1251 platform data. Ben. And how will that work with DT when board files will be in the history? 1. Boot loader reads the MAC address from NVRAM and puts it in the DT node. or 2. NVRAM reading is done by a tiny driver that is loaded based on the platform name and updates the DT node in memory. (But I don't know how wl1251 should decide to defer probing if it's probed before that other driver.) I'm uncomfortable with it too, and yes the permanent MAC should really be known before the interface is even registered, but... Imagine if the MAC address for your ethernet device was stored in /etc/my-mac-addr.txt, except that /etc was a read-only protected partition from a very small SSD. That's essentially the N900; it's stored in a file in a normal ext2/ext3 (?) filesystem on a partition of the internal flash. Oh, from my quick look I got the impression that this 'CAL' was stored directly in a specific flash partition. It seems like overkill to write a small driver that duplicates the ext3 + MTD drivers just to read the MAC. [...] I don't see that anything would need to be duplicated. But reading files from the kernel is really ugly, and deciding when to read the file would be another problem (as you noted). Ben Yes, CAL is stored in /dev/mtd1, a dedicated partition in the NAND flash. But reading the data stored in it from the kernel is not a trivial task, as it uses its own pseudo-fs format etc. Not impossible (I RE-ed libcal a while ago [0], so there is a code that could be reused, calvaria works too afaik), but still doesn't sound like a very good idea. [0] https://gitorious.org/community-ssu/libcal/source/d4c5fd9293ddb693c9b032b4c084cd0343b3ea26: Ivo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 15/16] wl1251: Add sysfs file address for setting permanent mac address
On 12.12.2013 00:15, Ben Hutchings wrote: 2. NVRAM reading is done by a tiny driver that is loaded based on the platform name and updates the DT node in memory. (But I don't know how wl1251 should decide to defer probing if it's probed before that other driver.) Ben. Maybe a "hwmac" property with a value of XX:XX:XX:XX:XX:XX (or some other invalid value) set in DT could be used to tell the driver to defer until that value changes to something sane? Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2 15/16] wl1251: Add sysfs file address for setting permanent mac address
On 11.12.2013 23:17, Ben Hutchings wrote: I think that's an even worse idea. This is not firmware and it already exists in separate storage. I think that rx51_init_wl1251() in arch/arm/mach-omap2/board-rx51-peripherals.c should either copy the MAC address out of NVRAM, or if it's too early to do that, then schedule a function to run later and only then set up wl1251 platform data. Ben. And how will that work with DT when board files will be in the history? Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2 15/16] wl1251: Add sysfs file address for setting permanent mac address
On 11.12.2013 23:17, Ben Hutchings wrote: I think that's an even worse idea. This is not firmware and it already exists in separate storage. I think that rx51_init_wl1251() in arch/arm/mach-omap2/board-rx51-peripherals.c should either copy the MAC address out of NVRAM, or if it's too early to do that, then schedule a function to run later and only then set up wl1251 platform data. Ben. And how will that work with DT when board files will be in the history? Ivo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 15/16] wl1251: Add sysfs file address for setting permanent mac address
On 12.12.2013 00:15, Ben Hutchings wrote: 2. NVRAM reading is done by a tiny driver that is loaded based on the platform name and updates the DT node in memory. (But I don't know how wl1251 should decide to defer probing if it's probed before that other driver.) Ben. Maybe a hwmac property with a value of XX:XX:XX:XX:XX:XX (or some other invalid value) set in DT could be used to tell the driver to defer until that value changes to something sane? Ivo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Use vm_iomap_memory for mmap-ing instead of remap_pfn_range
On 08.12.2013 01:49, Steven Luo wrote: This patch causes problems with DSP codecs on OMAP3 devices running Android -- specifically, when the decoder is cleaning up after itself, munmap() of the mapped area fails, leading to a memory leak which eventually crashes the system. As far as I can tell, the code with this patch applied reduces to (ignoring checks and such) remap_pfn_range(vma, vma->vm_start, (pdata->phys_mempool_base >> PAGE_SHIFT) + vma->vm_pgoff, vma->vm_end - vma->vm_start, vma->vm_page_prot); whereas the original was - status = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff, -vma->vm_end - vma->vm_start, -vma->vm_page_prot); We're subtracting (pdata->phys_mempool_base >> PAGE_SHIFT) from vma->vm_pgoff before calling vm_iomap_memory() to address the issue -- if that's satisfactory to everyone involved, I can submit the following patch. Hi, I can pick your changes and re-send the original patch with them incorporated if there are no objections. Are you fine with that? Regards, Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Use vm_iomap_memory for mmap-ing instead of remap_pfn_range
On 08.12.2013 01:49, Steven Luo wrote: This patch causes problems with DSP codecs on OMAP3 devices running Android -- specifically, when the decoder is cleaning up after itself, munmap() of the mapped area fails, leading to a memory leak which eventually crashes the system. As far as I can tell, the code with this patch applied reduces to (ignoring checks and such) remap_pfn_range(vma, vma-vm_start, (pdata-phys_mempool_base PAGE_SHIFT) + vma-vm_pgoff, vma-vm_end - vma-vm_start, vma-vm_page_prot); whereas the original was - status = remap_pfn_range(vma, vma-vm_start, vma-vm_pgoff, -vma-vm_end - vma-vm_start, -vma-vm_page_prot); We're subtracting (pdata-phys_mempool_base PAGE_SHIFT) from vma-vm_pgoff before calling vm_iomap_memory() to address the issue -- if that's satisfactory to everyone involved, I can submit the following patch. Hi, I can pick your changes and re-send the original patch with them incorporated if there are no objections. Are you fine with that? Regards, Ivo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Use vm_iomap_memory for mmap-ing instead of remap_pfn_range
On 08.12.2013 01:49, Steven Luo wrote:
This patch causes problems with DSP codecs on OMAP3 devices running
Android -- specifically, when the decoder is cleaning up after itself,
munmap() of the mapped area fails, leading to a memory leak which
eventually crashes the system.
As far as I can tell, the code with this patch applied reduces to
(ignoring checks and such)
remap_pfn_range(vma, vma->vm_start,
(pdata->phys_mempool_base >> PAGE_SHIFT) + vma->vm_pgoff,
vma->vm_end - vma->vm_start,
vma->vm_page_prot);
whereas the original was
- status = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
-vma->vm_end - vma->vm_start,
-vma->vm_page_prot);
We're subtracting (pdata->phys_mempool_base >> PAGE_SHIFT) from
vma->vm_pgoff before calling vm_iomap_memory() to address the issue --
if that's satisfactory to everyone involved, I can submit the following
patch.
-Steven Luo
(please cc, not subscribed)
From: Steven Luo
Date: Sat, 7 Dec 2013 02:11:20 -0800
Subject: [PATCH] tidspbridge: fix last patch to map same region of physical
memory as before
Commit 559c71fe5dc3 ("Staging: TIDSPBRIDGE: Use vm_iomap_memory for
mmap-ing instead of remap_pfn_range") had the effect of inadvertently
shifting the start of the physical memory area mapped by
pdata->phys_mempool_base. Correct this by subtracting that shift before
calling vm_iomap_memory() and adding it back afterwards.
Reported-by: Dheeraj CVR
Signed-off-by: Steven Luo
---
drivers/staging/tidspbridge/rmgr/drv_interface.c | 29 +++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/tidspbridge/rmgr/drv_interface.c
b/drivers/staging/tidspbridge/rmgr/drv_interface.c
index 83cc3a5..d7f7d04 100644
--- a/drivers/staging/tidspbridge/rmgr/drv_interface.c
+++ b/drivers/staging/tidspbridge/rmgr/drv_interface.c
@@ -258,6 +258,9 @@ err:
/* This function maps kernel space memory to user space memory. */
static int bridge_mmap(struct file *filp, struct vm_area_struct *vma)
{
+ unsigned long base_pgoff;
+ int status;
+
struct omap_dsp_platform_data *pdata =
omap_dspbridge_dev->dev.platform_data;
@@ -269,9 +272,29 @@ static int bridge_mmap(struct file *filp, struct
vm_area_struct *vma)
vma->vm_start, vma->vm_end, vma->vm_page_prot,
vma->vm_flags);
- return vm_iomap_memory(vma,
- pdata->phys_mempool_base,
- pdata->phys_mempool_size);
+ /*
+* vm_iomap_memory() expects vma->vm_pgoff to be expressed as an offset
+* from the start of the physical memory pool, but we're called with
+* a pfn (physical page number) stored there instead.
+*
+* To avoid duplicating lots of tricky overflow checking logic,
+* temporarily convert vma->vm_pgoff to the offset vm_iomap_memory()
+* expects, but restore the original value once the mapping has been
+* created.
+*/
+ base_pgoff = pdata->phys_mempool_base >> PAGE_SHIFT;
+ if (vma->vm_pgoff < base_pgoff)
+ return -EINVAL;
+ vma->vm_pgoff -= base_pgoff;
+
+ status = vm_iomap_memory(vma,
+pdata->phys_mempool_base,
+pdata->phys_mempool_size);
+
+ /* Restore the original value of vma->vm_pgoff */
+ vma->vm_pgoff += base_pgoff;
+
+ return status;
}
static const struct file_operations bridge_fops = {
Tested on Nokia N900 with Maemo 5 and Harmattan codec nodes
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Use vm_iomap_memory for mmap-ing instead of remap_pfn_range
On 08.12.2013 01:49, Steven Luo wrote:
This patch causes problems with DSP codecs on OMAP3 devices running
Android -- specifically, when the decoder is cleaning up after itself,
munmap() of the mapped area fails, leading to a memory leak which
eventually crashes the system.
As far as I can tell, the code with this patch applied reduces to
(ignoring checks and such)
remap_pfn_range(vma, vma-vm_start,
(pdata-phys_mempool_base PAGE_SHIFT) + vma-vm_pgoff,
vma-vm_end - vma-vm_start,
vma-vm_page_prot);
whereas the original was
- status = remap_pfn_range(vma, vma-vm_start, vma-vm_pgoff,
-vma-vm_end - vma-vm_start,
-vma-vm_page_prot);
We're subtracting (pdata-phys_mempool_base PAGE_SHIFT) from
vma-vm_pgoff before calling vm_iomap_memory() to address the issue --
if that's satisfactory to everyone involved, I can submit the following
patch.
-Steven Luo
(please cc, not subscribed)
From: Steven Luo ste...@steven676.net
Date: Sat, 7 Dec 2013 02:11:20 -0800
Subject: [PATCH] tidspbridge: fix last patch to map same region of physical
memory as before
Commit 559c71fe5dc3 (Staging: TIDSPBRIDGE: Use vm_iomap_memory for
mmap-ing instead of remap_pfn_range) had the effect of inadvertently
shifting the start of the physical memory area mapped by
pdata-phys_mempool_base. Correct this by subtracting that shift before
calling vm_iomap_memory() and adding it back afterwards.
Reported-by: Dheeraj CVR cvr.dhee...@gmail.com
Signed-off-by: Steven Luo ste...@steven676.net
---
drivers/staging/tidspbridge/rmgr/drv_interface.c | 29 +++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/tidspbridge/rmgr/drv_interface.c
b/drivers/staging/tidspbridge/rmgr/drv_interface.c
index 83cc3a5..d7f7d04 100644
--- a/drivers/staging/tidspbridge/rmgr/drv_interface.c
+++ b/drivers/staging/tidspbridge/rmgr/drv_interface.c
@@ -258,6 +258,9 @@ err:
/* This function maps kernel space memory to user space memory. */
static int bridge_mmap(struct file *filp, struct vm_area_struct *vma)
{
+ unsigned long base_pgoff;
+ int status;
+
struct omap_dsp_platform_data *pdata =
omap_dspbridge_dev-dev.platform_data;
@@ -269,9 +272,29 @@ static int bridge_mmap(struct file *filp, struct
vm_area_struct *vma)
vma-vm_start, vma-vm_end, vma-vm_page_prot,
vma-vm_flags);
- return vm_iomap_memory(vma,
- pdata-phys_mempool_base,
- pdata-phys_mempool_size);
+ /*
+* vm_iomap_memory() expects vma-vm_pgoff to be expressed as an offset
+* from the start of the physical memory pool, but we're called with
+* a pfn (physical page number) stored there instead.
+*
+* To avoid duplicating lots of tricky overflow checking logic,
+* temporarily convert vma-vm_pgoff to the offset vm_iomap_memory()
+* expects, but restore the original value once the mapping has been
+* created.
+*/
+ base_pgoff = pdata-phys_mempool_base PAGE_SHIFT;
+ if (vma-vm_pgoff base_pgoff)
+ return -EINVAL;
+ vma-vm_pgoff -= base_pgoff;
+
+ status = vm_iomap_memory(vma,
+pdata-phys_mempool_base,
+pdata-phys_mempool_size);
+
+ /* Restore the original value of vma-vm_pgoff */
+ vma-vm_pgoff += base_pgoff;
+
+ return status;
}
static const struct file_operations bridge_fops = {
Tested on Nokia N900 with Maemo 5 and Harmattan codec nodes
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
On 06.12.2013 17:10, gre...@linuxfoundation.org wrote: On Fri, Dec 06, 2013 at 08:05:38AM +0200, Ivajlo Dimitrov wrote: Hi Greg, On 01.12.2013 19:07, Ivaylo DImitrov wrote: From: Ivaylo Dimitrov Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't need to be exported. It can also be made way simpler by using sscanf. Signed-off-by: Ivaylo Dimitrov --- drivers/staging/tidspbridge/Makefile |2 +- drivers/staging/tidspbridge/gen/uuidutil.c | 85 .../tidspbridge/include/dspbridge/uuidutil.h | 18 drivers/staging/tidspbridge/rmgr/dbdcd.c | 42 +- 4 files changed, 39 insertions(+), 108 deletions(-) delete mode 100644 drivers/staging/tidspbridge/gen/uuidutil.c I guess the initial mail somehow didn't make it through your spam filter: https://lkml.org/lkml/2013/12/1/70 It did, but I thought that people asked for it to be changed in the thread afterwards, so I was expecting an updated version from you. Care to fix things up and resend it? thanks, greg k-h Sure, the change I was asked for is trivial, but I didn't get the reason why it is needed. Neither there is a reply to my follow-up comment [0]. Sorry, I am pretty much new on LKML and could miss things that are supposed to be clear from the start, but my impression is that when someone says "it is better", he/she should explain why it is better or at least what is wrong with the patch he/she wants to be changed. However, I don't want to enter some arguing loop, so if you think I should change the code as per Joe's comment, just confirm it and I'll do it. Thanks, Ivo [0] https://lkml.org/lkml/2013/12/1/113 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
On 06.12.2013 17:10, gre...@linuxfoundation.org wrote: On Fri, Dec 06, 2013 at 08:05:38AM +0200, Ivajlo Dimitrov wrote: Hi Greg, On 01.12.2013 19:07, Ivaylo DImitrov wrote: From: Ivaylo Dimitrov freemangor...@abv.bg Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't need to be exported. It can also be made way simpler by using sscanf. Signed-off-by: Ivaylo Dimitrov freemangor...@abv.bg --- drivers/staging/tidspbridge/Makefile |2 +- drivers/staging/tidspbridge/gen/uuidutil.c | 85 .../tidspbridge/include/dspbridge/uuidutil.h | 18 drivers/staging/tidspbridge/rmgr/dbdcd.c | 42 +- 4 files changed, 39 insertions(+), 108 deletions(-) delete mode 100644 drivers/staging/tidspbridge/gen/uuidutil.c I guess the initial mail somehow didn't make it through your spam filter: https://lkml.org/lkml/2013/12/1/70 It did, but I thought that people asked for it to be changed in the thread afterwards, so I was expecting an updated version from you. Care to fix things up and resend it? thanks, greg k-h Sure, the change I was asked for is trivial, but I didn't get the reason why it is needed. Neither there is a reply to my follow-up comment [0]. Sorry, I am pretty much new on LKML and could miss things that are supposed to be clear from the start, but my impression is that when someone says it is better, he/she should explain why it is better or at least what is wrong with the patch he/she wants to be changed. However, I don't want to enter some arguing loop, so if you think I should change the code as per Joe's comment, just confirm it and I'll do it. Thanks, Ivo [0] https://lkml.org/lkml/2013/12/1/113 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: OMAPFB: CMA allocation failures
On 05.12.2013 13:25, Tomi Valkeinen wrote: How about the patch below? If I'm not mistaken (and I might) it reserves separate memory area for omapfb, which is not used by CMA. If it works, it should be extended to get the parameters via kernel cmdline, and use that alloc only if the user requests it. YAY!!! That one seems to fix the issue. Though I had to revert 7faa92339bbb1e6b9a80983b206642517327eb75 (well, I hacked check_horiz_timing_omap3 to always return 0). Otherwise I have "omapdss DISPC error: horizontal timing too tight" error when try to play anything above 320x240 or so. I'll look at the issue. Regards, Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: OMAPFB: CMA allocation failures
On 05.12.2013 13:25, Tomi Valkeinen wrote: How about the patch below? If I'm not mistaken (and I might) it reserves separate memory area for omapfb, which is not used by CMA. If it works, it should be extended to get the parameters via kernel cmdline, and use that alloc only if the user requests it. YAY!!! That one seems to fix the issue. Though I had to revert 7faa92339bbb1e6b9a80983b206642517327eb75 (well, I hacked check_horiz_timing_omap3 to always return 0). Otherwise I have omapdss DISPC error: horizontal timing too tight error when try to play anything above 320x240 or so. I'll look at the issue. Regards, Ivo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
My other patch (the one that fixes the security issue) was already applied, albeit it was sent after this one, see https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/commit/?h=staging-linus=559c71fe5dc3bf2ecc55afb336145db7f0abf810 , that is why I think there is some problem with the mail itself. Sure I will wait :) regards, Ivo On 06.12.2013 09:32, Dan Carpenter wrote: On Fri, Dec 06, 2013 at 08:05:38AM +0200, Ivajlo Dimitrov wrote: Hi Greg, On 01.12.2013 19:07, Ivaylo DImitrov wrote: From: Ivaylo Dimitrov Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't need to be exported. It can also be made way simpler by using sscanf. Signed-off-by: Ivaylo Dimitrov --- drivers/staging/tidspbridge/Makefile |2 +- drivers/staging/tidspbridge/gen/uuidutil.c | 85 .../tidspbridge/include/dspbridge/uuidutil.h | 18 drivers/staging/tidspbridge/rmgr/dbdcd.c | 42 +- 4 files changed, 39 insertions(+), 108 deletions(-) delete mode 100644 drivers/staging/tidspbridge/gen/uuidutil.c I guess the initial mail somehow didn't make it through your spam filter: https://lkml.org/lkml/2013/12/1/70 It's too early to start resending. Wait for another week at least. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
Hi Greg, On 01.12.2013 19:07, Ivaylo DImitrov wrote: From: Ivaylo Dimitrov Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't need to be exported. It can also be made way simpler by using sscanf. Signed-off-by: Ivaylo Dimitrov --- drivers/staging/tidspbridge/Makefile |2 +- drivers/staging/tidspbridge/gen/uuidutil.c | 85 .../tidspbridge/include/dspbridge/uuidutil.h | 18 drivers/staging/tidspbridge/rmgr/dbdcd.c | 42 +- 4 files changed, 39 insertions(+), 108 deletions(-) delete mode 100644 drivers/staging/tidspbridge/gen/uuidutil.c I guess the initial mail somehow didn't make it through your spam filter: https://lkml.org/lkml/2013/12/1/70 Regards, Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
Hi Greg, On 01.12.2013 19:07, Ivaylo DImitrov wrote: From: Ivaylo Dimitrov freemangor...@abv.bg Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't need to be exported. It can also be made way simpler by using sscanf. Signed-off-by: Ivaylo Dimitrov freemangor...@abv.bg --- drivers/staging/tidspbridge/Makefile |2 +- drivers/staging/tidspbridge/gen/uuidutil.c | 85 .../tidspbridge/include/dspbridge/uuidutil.h | 18 drivers/staging/tidspbridge/rmgr/dbdcd.c | 42 +- 4 files changed, 39 insertions(+), 108 deletions(-) delete mode 100644 drivers/staging/tidspbridge/gen/uuidutil.c I guess the initial mail somehow didn't make it through your spam filter: https://lkml.org/lkml/2013/12/1/70 Regards, Ivo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
My other patch (the one that fixes the security issue) was already applied, albeit it was sent after this one, see https://git.kernel.org/cgit/linux/kernel/git/gregkh/staging.git/commit/?h=staging-linusid=559c71fe5dc3bf2ecc55afb336145db7f0abf810 , that is why I think there is some problem with the mail itself. Sure I will wait :) regards, Ivo On 06.12.2013 09:32, Dan Carpenter wrote: On Fri, Dec 06, 2013 at 08:05:38AM +0200, Ivajlo Dimitrov wrote: Hi Greg, On 01.12.2013 19:07, Ivaylo DImitrov wrote: From: Ivaylo Dimitrov freemangor...@abv.bg Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't need to be exported. It can also be made way simpler by using sscanf. Signed-off-by: Ivaylo Dimitrov freemangor...@abv.bg --- drivers/staging/tidspbridge/Makefile |2 +- drivers/staging/tidspbridge/gen/uuidutil.c | 85 .../tidspbridge/include/dspbridge/uuidutil.h | 18 drivers/staging/tidspbridge/rmgr/dbdcd.c | 42 +- 4 files changed, 39 insertions(+), 108 deletions(-) delete mode 100644 drivers/staging/tidspbridge/gen/uuidutil.c I guess the initial mail somehow didn't make it through your spam filter: https://lkml.org/lkml/2013/12/1/70 It's too early to start resending. Wait for another week at least. regards, dan carpenter -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
On 01.12.2013 21:06, Joe Perches wrote:
On Sun, 2013-12-01 at 19:07 +0200, Ivaylo DImitrov wrote:
From: Ivaylo Dimitrov
Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't
need to be exported. It can also be made way simpler by using sscanf.
[]
diff --git a/drivers/staging/tidspbridge/rmgr/dbdcd.c
b/drivers/staging/tidspbridge/rmgr/dbdcd.c
[]
@@ -74,6 +74,40 @@ static int get_dep_lib_info(struct dcd_manager *hdcd_mgr,
enum nldr_phase phase);
/*
+ * dcd_uuid_from_string
+ * Purpose:
+ * Converts an ANSI string to a dsp_uuid.
+ * Parameters:
+ * sz_uuid:Pointer to a string that represents a dsp_uuid object.
+ * uuid_obj: Pointer to a dsp_uuid object.
+ * Returns:
+ * Requires:
+ * uuid_obj & sz_uuid are non-NULL values.
+ * Ensures:
+ * Details:
+ * We assume the string representation of a UUID has the following format:
+ * "12345678_1234_1234_1234_123456789abc".
+ */
+static void dcd_uuid_from_string(char *sz_uuid, struct dsp_uuid *uuid_obj)
+{
+ char c;
+ u64 t;
+
+ /*
+* sscanf implementation cannot deal with hh format modifier
+* if the converted value doesn't fit in u32. So, convert the
+* last six bytes to u64 and memcpy what is needed
+*/
+ sscanf(sz_uuid, "%8x%c%4hx%c%4hx%c%2hhx%2hhx%c%llx",
+ _obj->data1, , _obj->data2, ,
+ _obj->data3, , _obj->data4,
+ _obj->data5, , );
+
+ t = cpu_to_be64(t);
+ memcpy(_obj->data6[0], ((char*)) + 2, 6);
+}
It'd probably be better to return true or false on
successful conversion, use a temporary struct dsp_uuid,
check the sscanf return is 10 and only copy to uuid_obj
on success.
Something like:
static bool dcd_uuid_from_string(char *sz_uuid, struct dsp_uuid *uuid_obj)
{
char c;
u64 t;
struct dsp_uuid tmp;
/*
* sscanf implementation cannot deal with hh format modifier
* if the converted value doesn't fit in u32. So, convert the
* last six bytes to u64 and memcpy what is needed
*/
if (sscanf(sz_uuid, "%8x%c%4hx%c%4hx%c%2hhx%2hhx%c%llx",
, , , ,
, , ,
, , ) != 10)
return false;
t = cpu_to_be64(t);
memcpy([0], ((char*)) + 2, 6);
*uuid_obj = tmp;
return true;
}
If there is to be a return value from that function, it is better to be
int (-EINVAL/0), IMO. And I am not sure that makes much of a sense, as
(afaik) uuids are read from baseimage.dof and codec nodes, if those are
broken I suspect wrong uuids will be our least problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Staging: tidspbridge: make mmap root-only so it is not a security problem
On 01.12.2013 14:27, Dan Carpenter wrote:
On Sun, Dec 01, 2013 at 01:10:06PM +0100, Pavel Machek wrote:
diff --git a/drivers/staging/tidspbridge/rmgr/drv_interface.c
b/drivers/staging/tidspbridge/rmgr/drv_interface.c
index 1aa4a3f..a8e86cf 100644
--- a/drivers/staging/tidspbridge/rmgr/drv_interface.c
+++ b/drivers/staging/tidspbridge/rmgr/drv_interface.c
@@ -258,7 +258,17 @@ err:
/* This function maps kernel space memory to user space memory. */
static int bridge_mmap(struct file *filp, struct vm_area_struct *vma)
{
- u32 status;
+ int status;
+ struct omap_dsp_platform_data *pdata =
+ omap_dspbridge_dev->dev.platform_data;
+ unsigned long start = vma->vm_pgoff << PAGE_SHIFT;
+
+ if (start < pdata->phys_mempool_base)
+ return -EINVAL;
+
+ if (vma->vm_end - vma->vm_start + (start - pdata->phys_mempool_base)
+ > pdata->phys_mempool_size)
This test is vulnerable to integer overflows if you pick a very high
value for start. Consider using the vm_iomap_memory() helper function
instead of calling remap_pfn_range() directly. Commit 7314e613d5ff
('Fix a few incorrectly checked [io_]remap_pfn_range() calls') has an
example of how the conversion works.
regards,
dan carpenter
Dan,
If that one looks fine, I'll send a correctly formatted patch.
Pavel - feel free you to do it, I don't want to "steal" your bug :) .
Patch tested with Maemo5 on n900:
diff --git a/drivers/staging/tidspbridge/rmgr/drv_interface.c
b/drivers/staging/tidspbridge/rmgr/drv_interface.c
index 1aa4a3f..2d500ea 100644
--- a/drivers/staging/tidspbridge/rmgr/drv_interface.c
+++ b/drivers/staging/tidspbridge/rmgr/drv_interface.c
@@ -258,8 +258,9 @@ err:
/* This function maps kernel space memory to user space memory. */
static int bridge_mmap(struct file *filp, struct vm_area_struct *vma)
{
-u32 status;
-
+struct omap_dsp_platform_data *pdata =
+omap_dspbridge_dev->dev.platform_data;
+
/* VM_IO | VM_DONTEXPAND | VM_DONTDUMP are set by remap_pfn_range() */
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
@@ -268,13 +269,9 @@ static int bridge_mmap(struct file *filp, struct
vm_area_struct *vma)
vma->vm_start, vma->vm_end, vma->vm_page_prot,
vma->vm_flags);
-status = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
- vma->vm_end - vma->vm_start,
- vma->vm_page_prot);
-if (status != 0)
-status = -EAGAIN;
-
-return status;
+return vm_iomap_memory(vma,
+ pdata->phys_mempool_base,
+ pdata->phys_mempool_size);
}
static const struct file_operations bridge_fops = {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] Staging: tidspbridge: make mmap root-only so it is not a security problem
On 01.12.2013 14:27, Dan Carpenter wrote:
On Sun, Dec 01, 2013 at 01:10:06PM +0100, Pavel Machek wrote:
diff --git a/drivers/staging/tidspbridge/rmgr/drv_interface.c
b/drivers/staging/tidspbridge/rmgr/drv_interface.c
index 1aa4a3f..a8e86cf 100644
--- a/drivers/staging/tidspbridge/rmgr/drv_interface.c
+++ b/drivers/staging/tidspbridge/rmgr/drv_interface.c
@@ -258,7 +258,17 @@ err:
/* This function maps kernel space memory to user space memory. */
static int bridge_mmap(struct file *filp, struct vm_area_struct *vma)
{
- u32 status;
+ int status;
+ struct omap_dsp_platform_data *pdata =
+ omap_dspbridge_dev-dev.platform_data;
+ unsigned long start = vma-vm_pgoff PAGE_SHIFT;
+
+ if (start pdata-phys_mempool_base)
+ return -EINVAL;
+
+ if (vma-vm_end - vma-vm_start + (start - pdata-phys_mempool_base)
+pdata-phys_mempool_size)
This test is vulnerable to integer overflows if you pick a very high
value for start. Consider using the vm_iomap_memory() helper function
instead of calling remap_pfn_range() directly. Commit 7314e613d5ff
('Fix a few incorrectly checked [io_]remap_pfn_range() calls') has an
example of how the conversion works.
regards,
dan carpenter
Dan,
If that one looks fine, I'll send a correctly formatted patch.
Pavel - feel free you to do it, I don't want to steal your bug :) .
Patch tested with Maemo5 on n900:
diff --git a/drivers/staging/tidspbridge/rmgr/drv_interface.c
b/drivers/staging/tidspbridge/rmgr/drv_interface.c
index 1aa4a3f..2d500ea 100644
--- a/drivers/staging/tidspbridge/rmgr/drv_interface.c
+++ b/drivers/staging/tidspbridge/rmgr/drv_interface.c
@@ -258,8 +258,9 @@ err:
/* This function maps kernel space memory to user space memory. */
static int bridge_mmap(struct file *filp, struct vm_area_struct *vma)
{
-u32 status;
-
+struct omap_dsp_platform_data *pdata =
+omap_dspbridge_dev-dev.platform_data;
+
/* VM_IO | VM_DONTEXPAND | VM_DONTDUMP are set by remap_pfn_range() */
vma-vm_page_prot = pgprot_noncached(vma-vm_page_prot);
@@ -268,13 +269,9 @@ static int bridge_mmap(struct file *filp, struct
vm_area_struct *vma)
vma-vm_start, vma-vm_end, vma-vm_page_prot,
vma-vm_flags);
-status = remap_pfn_range(vma, vma-vm_start, vma-vm_pgoff,
- vma-vm_end - vma-vm_start,
- vma-vm_page_prot);
-if (status != 0)
-status = -EAGAIN;
-
-return status;
+return vm_iomap_memory(vma,
+ pdata-phys_mempool_base,
+ pdata-phys_mempool_size);
}
static const struct file_operations bridge_fops = {
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Staging: TIDSPBRIDGE: Remove UUID helper
On 01.12.2013 21:06, Joe Perches wrote:
On Sun, 2013-12-01 at 19:07 +0200, Ivaylo DImitrov wrote:
From: Ivaylo Dimitrov freemangor...@abv.bg
Custom uuid helper function is needed only in rmgr/dbdcd.c and doesn't
need to be exported. It can also be made way simpler by using sscanf.
[]
diff --git a/drivers/staging/tidspbridge/rmgr/dbdcd.c
b/drivers/staging/tidspbridge/rmgr/dbdcd.c
[]
@@ -74,6 +74,40 @@ static int get_dep_lib_info(struct dcd_manager *hdcd_mgr,
enum nldr_phase phase);
/*
+ * dcd_uuid_from_string
+ * Purpose:
+ * Converts an ANSI string to a dsp_uuid.
+ * Parameters:
+ * sz_uuid:Pointer to a string that represents a dsp_uuid object.
+ * uuid_obj: Pointer to a dsp_uuid object.
+ * Returns:
+ * Requires:
+ * uuid_obj sz_uuid are non-NULL values.
+ * Ensures:
+ * Details:
+ * We assume the string representation of a UUID has the following format:
+ * 12345678_1234_1234_1234_123456789abc.
+ */
+static void dcd_uuid_from_string(char *sz_uuid, struct dsp_uuid *uuid_obj)
+{
+ char c;
+ u64 t;
+
+ /*
+* sscanf implementation cannot deal with hh format modifier
+* if the converted value doesn't fit in u32. So, convert the
+* last six bytes to u64 and memcpy what is needed
+*/
+ sscanf(sz_uuid, %8x%c%4hx%c%4hx%c%2hhx%2hhx%c%llx,
+ uuid_obj-data1, c, uuid_obj-data2, c,
+ uuid_obj-data3, c, uuid_obj-data4,
+ uuid_obj-data5, c, t);
+
+ t = cpu_to_be64(t);
+ memcpy(uuid_obj-data6[0], ((char*)t) + 2, 6);
+}
It'd probably be better to return true or false on
successful conversion, use a temporary struct dsp_uuid,
check the sscanf return is 10 and only copy to uuid_obj
on success.
Something like:
static bool dcd_uuid_from_string(char *sz_uuid, struct dsp_uuid *uuid_obj)
{
char c;
u64 t;
struct dsp_uuid tmp;
/*
* sscanf implementation cannot deal with hh format modifier
* if the converted value doesn't fit in u32. So, convert the
* last six bytes to u64 and memcpy what is needed
*/
if (sscanf(sz_uuid, %8x%c%4hx%c%4hx%c%2hhx%2hhx%c%llx,
tmp.data1, c, tmp.data2, c,
tmp.data3, c, tmp.data4,
tmp.data5, c, t) != 10)
return false;
t = cpu_to_be64(t);
memcpy(tmp.data6[0], ((char*)t) + 2, 6);
*uuid_obj = tmp;
return true;
}
If there is to be a return value from that function, it is better to be
int (-EINVAL/0), IMO. And I am not sure that makes much of a sense, as
(afaik) uuids are read from baseimage.dof and codec nodes, if those are
broken I suspect wrong uuids will be our least problem.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: OMAPFB: CMA allocation failures
Ping? - Original Message - From: "Ивайло Димитров" To: "Tomi Valkeinen" Cc: ; ; ; ; ; ; Sent: Tuesday, November 05, 2013 9:55 PM Subject: Re: OMAPFB: CMA allocation failures > Оригинално писмо >От: Tomi Valkeinen >Относно: Re: OMAPFB: CMA allocation failures >До: Ивайло Димитров >Изпратено на: Сряда, 2013, Октомври 30 14:19:32 EET > >I really dislike the idea of adding the omap vram allocator back. Then >again, if the CMA doesn't work, something has to be done. > If I got Minchan Kim's explanation correctly, CMA simply can't be used for allocation of framebuffer memory, because it is unreliable. >Pre-allocating is possible, but that won't work if there's any need to >re-allocating the framebuffers. Except if the omapfb would retain and >manage the pre-allocated buffers, but that would just be more or less >the old vram allocator again. > >So, as I see it, the best option would be to have the standard dma_alloc >functions get the memory for omapfb from a private pool, which is not >used for anything else. > >I wonder if that's possible already? It sounds quite trivial to me. dma_alloc functions use either CMA or (iirc) get_pages_exact if CMA is disabled. Both of those fail easily. AFAIK there are several implementations with similar functionality, like CMEM and ION but (correct me if I am wrong) neither of them is upstreamed. In the current kernel I don't see anything that can be used for the purpose of reliable allocation of big chunks of contiguous memory. So, something should be done, but honestly, I can't think of anything but bringing VRAM allocator back. Not that I like the idea of bringing back ~700 lines of code, but I see no other option if omapfb driver is to be actually useful. Regards, Ivo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Staging: tidspbridge: disable driver
Hi, (re-sending in plain text, sorry for the noise) commit https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=930ba4a374b96560ef9fde2145cdc454a164ddcc disables tidspbridge driver for the reasons it has a security bug and there is no active maintainer. I was following the development of that driver for a bit and my impression was that the reason it is still in staging is this http://www.spinics.net/lists/linux-omap/msg84115.html . And as dmtimer driver is on its way to be merged upstream(or was it already merged?) I was planning to work on tidspbridge driver to fix it and send the patches. I also have a couple of another small patches ready to be send. However, could you elaborate on the "security bug" so I can try to fix it and send the patch? Also, what needs to be done for the tidspdriver to get out of staging as it seems that what I though initially is incorrect. Regards, Ivo PS: my further mails will be from this account on gmal as it seems LKML does not like my abv.bg account (freemangordon at abv dot bg) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Staging: tidspbridge: disable driver
Hi, (re-sending in plain text, sorry for the noise) commit https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=930ba4a374b96560ef9fde2145cdc454a164ddcc disables tidspbridge driver for the reasons it has a security bug and there is no active maintainer. I was following the development of that driver for a bit and my impression was that the reason it is still in staging is this http://www.spinics.net/lists/linux-omap/msg84115.html . And as dmtimer driver is on its way to be merged upstream(or was it already merged?) I was planning to work on tidspbridge driver to fix it and send the patches. I also have a couple of another small patches ready to be send. However, could you elaborate on the security bug so I can try to fix it and send the patch? Also, what needs to be done for the tidspdriver to get out of staging as it seems that what I though initially is incorrect. Regards, Ivo PS: my further mails will be from this account on gmal as it seems LKML does not like my abv.bg account (freemangordon at abv dot bg) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: OMAPFB: CMA allocation failures
Ping? - Original Message - From: Ивайло Димитров freemangor...@abv.bg To: Tomi Valkeinen tomi.valkei...@ti.com Cc: minc...@kernel.org; pa...@ucw.cz; s...@debian.org; pali.ro...@gmail.com; pc+n...@asdf.org; linux-kernel@vger.kernel.org; linux...@kvack.org Sent: Tuesday, November 05, 2013 9:55 PM Subject: Re: OMAPFB: CMA allocation failures Оригинално писмо От: Tomi Valkeinen Относно: Re: OMAPFB: CMA allocation failures До: Ивайло Димитров Изпратено на: Сряда, 2013, Октомври 30 14:19:32 EET I really dislike the idea of adding the omap vram allocator back. Then again, if the CMA doesn't work, something has to be done. If I got Minchan Kim's explanation correctly, CMA simply can't be used for allocation of framebuffer memory, because it is unreliable. Pre-allocating is possible, but that won't work if there's any need to re-allocating the framebuffers. Except if the omapfb would retain and manage the pre-allocated buffers, but that would just be more or less the old vram allocator again. So, as I see it, the best option would be to have the standard dma_alloc functions get the memory for omapfb from a private pool, which is not used for anything else. I wonder if that's possible already? It sounds quite trivial to me. dma_alloc functions use either CMA or (iirc) get_pages_exact if CMA is disabled. Both of those fail easily. AFAIK there are several implementations with similar functionality, like CMEM and ION but (correct me if I am wrong) neither of them is upstreamed. In the current kernel I don't see anything that can be used for the purpose of reliable allocation of big chunks of contiguous memory. So, something should be done, but honestly, I can't think of anything but bringing VRAM allocator back. Not that I like the idea of bringing back ~700 lines of code, but I see no other option if omapfb driver is to be actually useful. Regards, Ivo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
