* Re: efifb: patch to allow userspace to unbind efi framebuffer driver from VGA device
From: Bruno Prémont @ 2013-08-19 5:51 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <52119D6D.5070209@castus.tv>
Hi Jonathan,
On Sun, 18 Aug 2013 21:22:05 -0700 Jonathan Campbell wrote:
> This patch allows userspace to direct efifb to let go of the VGA device
> and unregister it's framebuffer. As far as I can tell, the Linux kernel
> framebuffer console knows to let go when efifb unregisters it's
> framebuffer. The problem I'm trying to solve is that I need efifb so the
> kernel can show it's status on-screen during boot, but then I need efifb
> to step aside and let a better driver load and take the VGA device later
> during boot up.
>
> The custom Linux distribution I've made for myself uses a userspace
> program to "boot" secondary VGA devices and load both fbdev and drm/kms
> drivers to bring video online. When I wrote this, I needed the ability
> for efifb to let go so that it could load bochsfb to better use the VGA
> output of VirtualBox and bochs. Without it, my console is stuck at
> whatever video mode the UEFI BIOS happened to leave it at and the more
> specialized driver cannot acquire the resources it needs to do it's work.
>
> The patch adds a sysfs device file "bind_fb" to the
> /sys/bus/platform/devices/efifb.0 filesystem tree. Writing "1" causes it
> to re-register the framebuffer, "0" to unregister. Checks are in place
> to not register the framebuffer if already registered, etc.
That is the wrong approach.
On fbdev subsystem there is infrastructure for replacing generic
firmware drivers with specialized drivers (as is called by KMS
drivers).
Have a look at remove_conflicting_framebuffers() and its use by KMS
drivers (i915, radeon, nouveau, mgag200, cirrus).
Unloading efifb should be able to happen automatically when
loading/probing new driver, without userspace help (so it can work when
both are built-in).
Bruno
> I realize it's not perfect and I wanted to know what I could do to
> improve it and eventually make it to mainline.
>
> On a related issue, when will efifb make use of the Graphics Output
> Protocol through UEFI to offer modesetting? Is that possible? If no
> specialized drivers are available it'd be nice to at least have basic
> modesetting as needed.
^ permalink raw reply
* Re: efifb: patch to allow userspace to unbind efi framebuffer driver from VGA device
From: Jonathan Campbell @ 2013-08-19 6:34 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <52119D6D.5070209@castus.tv>
Perfect!
I removed the request_mem_region from my bochsfb driver and coded it to
allocate an aperture struct, then register_framebuffer(). It kicks efifb
off just as you said. Thanks!
Jonathan Campbell
> Hi Jonathan,
>
> On Sun, 18 Aug 2013 21:22:05 -0700 Jonathan Campbell wrote:
>> This patch allows userspace to direct efifb to let go of the VGA device
>> and unregister it's framebuffer. As far as I can tell, the Linux kernel
>> framebuffer console knows to let go when efifb unregisters it's
>> framebuffer. The problem I'm trying to solve is that I need efifb so the
>> kernel can show it's status on-screen during boot, but then I need efifb
>> to step aside and let a better driver load and take the VGA device later
>> during boot up.
>>
>> The custom Linux distribution I've made for myself uses a userspace
>> program to "boot" secondary VGA devices and load both fbdev and drm/kms
>> drivers to bring video online. When I wrote this, I needed the ability
>> for efifb to let go so that it could load bochsfb to better use the VGA
>> output of VirtualBox and bochs. Without it, my console is stuck at
>> whatever video mode the UEFI BIOS happened to leave it at and the more
>> specialized driver cannot acquire the resources it needs to do it's work.
>>
>> The patch adds a sysfs device file "bind_fb" to the
>> /sys/bus/platform/devices/efifb.0 filesystem tree. Writing "1" causes it
>> to re-register the framebuffer, "0" to unregister. Checks are in place
>> to not register the framebuffer if already registered, etc.
> That is the wrong approach.
>
> On fbdev subsystem there is infrastructure for replacing generic
> firmware drivers with specialized drivers (as is called by KMS
> drivers).
>
> Have a look at remove_conflicting_framebuffers() and its use by KMS
> drivers (i915, radeon, nouveau, mgag200, cirrus).
>
> Unloading efifb should be able to happen automatically when
> loading/probing new driver, without userspace help (so it can work when
> both are built-in).
>
> Bruno
>
>
>> I realize it's not perfect and I wanted to know what I could do to
>> improve it and eventually make it to mainline.
>>
>> On a related issue, when will efifb make use of the Graphics Output
>> Protocol through UEFI to offer modesetting? Is that possible? If no
>> specialized drivers are available it'd be nice to at least have basic
>> modesetting as needed.
^ permalink raw reply
* Bochs VBE & Virtualbox framebuffer driver
From: Jonathan Campbell @ 2013-08-19 8:24 UTC (permalink / raw)
To: linux-fbdev
This is an experimental framebuffer driver for VirtualBox and Bochs VBE
displays. It started off as a proof of concept and grew into the code it
is now. When testing Linux in Bochs or VirtualBox this allows greater
control over the display and bit depth, though no h/w acceleration.
What do you think? Would this be something that could make it into the
mainline kernel?
http://www.hackipedia.org/Projects/bochsfb/tvbox-v2.2-bochsfb-rev-00000019-src.tar.xz
^ permalink raw reply
* [PATCH 0/7] replace devm_request_and_ioremap by devm_ioremap_resource
From: Julia Lawall @ 2013-08-19 11:20 UTC (permalink / raw)
To: linux-arm-kernel
devm_request_and_ioremap has been replaced by devm_ioremap_resource, which
gives more informative error return code information. This patch series
removes the remaining uses of devm_request_and_ioremap.
This patch series was generated using the semantic patch
scripts/coccinelle/api/devm_ioremap_resource.cocci from the Linux kernel
source tree. Manual modifications have been made to move associated calls
to platform_get_resource closer to the resulting call to
devm_ioremap_resource and to remove the associated error handling code.
^ permalink raw reply
* [PATCH 6/7] video: xilinxfb: replace devm_request_and_ioremap by devm_ioremap_resource
From: Julia Lawall @ 2013-08-19 11:20 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard
Cc: kernel-janitors, Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1376911241-27720-1-git-send-email-Julia.Lawall@lip6.fr>
From: Julia Lawall <Julia.Lawall@lip6.fr>
Use devm_ioremap_resource instead of devm_request_and_ioremap.
This was done using the semantic patch
scripts/coccinelle/api/devm_ioremap_resource.cocci
The initialization of drvdata->regs_phys was manually moved lower, to take
advantage of the NULL test on res performed by devm_ioremap_resource.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
drivers/video/xilinxfb.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index 6629b29..84c664e 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -259,12 +259,12 @@ static int xilinxfb_assign(struct platform_device *pdev,
struct resource *res;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- drvdata->regs_phys = res->start;
- drvdata->regs = devm_request_and_ioremap(&pdev->dev, res);
- if (!drvdata->regs) {
- rc = -EADDRNOTAVAIL;
+ drvdata->regs = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(drvdata->regs)) {
+ rc = PTR_ERR(drvdata->regs);
goto err_region;
}
+ drvdata->regs_phys = res->start;
}
/* Allocate the framebuffer memory */
^ permalink raw reply related
* Re: [PATCH] Release efifb's colormap in efifb_destroy()
From: Peter Jones @ 2013-08-19 14:02 UTC (permalink / raw)
To: David Herrmann
Cc: Catalin Marinas, Alexandra N. Kossovsky,
linux-fbdev@vger.kernel.org, linux-kernel, Tomi Valkeinen,
Jean-Christophe Plagniol-Villard
In-Reply-To: <CANq1E4Sf7o2ct3np0F11k0B=03UL0P=qZOUCUBM2fKuRDrHXbw@mail.gmail.com>
On Fri, Aug 16, 2013 at 03:51:34PM +0200, David Herrmann wrote:
> Hi
>
> On Thu, Jul 25, 2013 at 5:48 PM, Peter Jones <pjones@redhat.com> wrote:
> > This was found by Alexandra Kossovsky, who noted this traceback from
> > kmemleak:
> >
> >> unreferenced object 0xffff880216fcfe00 (size 512):
> >> comm "swapper/0", pid 1, jiffies 4294895429 (age 1415.320s)
> >> hex dump (first 32 bytes):
> >> 00 00 00 00 00 00 00 00 aa aa aa aa aa aa aa aa ................
> >> 55 55 55 55 55 55 55 55 ff ff ff ff ff ff ff ff UUUUUUUU........
> >> backtrace:
> >> [<ffffffff813e415c>] kmemleak_alloc+0x21/0x3e
> >> [<ffffffff8111c17f>]
> >> kmemleak_alloc_recursive.constprop.57+0x16/0x18
> >> [<ffffffff8111e63b>] __kmalloc+0xf9/0x144
> >> [<ffffffff8123d9cf>] fb_alloc_cmap_gfp+0x47/0xe1
> >> [<ffffffff8123da77>] fb_alloc_cmap+0xe/0x10
> >> [<ffffffff81aff40a>] efifb_probe+0x3e9/0x48f
> >> [<ffffffff812c566f>] platform_drv_probe+0x34/0x5e
> >> [<ffffffff812c3e6d>] driver_probe_device+0x98/0x1b4
> >> [<ffffffff812c3fd7>] __driver_attach+0x4e/0x6f
> >> [<ffffffff812c25bf>] bus_for_each_dev+0x57/0x8a
> >> [<ffffffff812c3984>] driver_attach+0x19/0x1b
> >> [<ffffffff812c362b>] bus_add_driver+0xde/0x201
> >> [<ffffffff812c453f>] driver_register+0x8c/0x110
> >> [<ffffffff812c510d>] platform_driver_register+0x41/0x43
> >> [<ffffffff812c5127>] platform_driver_probe+0x18/0x8a
> >> [<ffffffff81aff002>] efifb_init+0x276/0x295
>
> (CC'ing fbdev maintainers)
>
> Your signed-off-by is missing. Apart from that:
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Indeed it is. With that in mind:
Signed-off-by: Peter Jones <pjones@redhat.com>
>
> Regards
> David
>
> > ---
> > drivers/video/efifb.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
> > index 390b61b..1f3eab3 100644
> > --- a/drivers/video/efifb.c
> > +++ b/drivers/video/efifb.c
> > @@ -289,6 +289,7 @@ static void efifb_destroy(struct fb_info *info)
> > if (request_mem_succeeded)
> > release_mem_region(info->apertures->ranges[0].base,
> > info->apertures->ranges[0].size);
> > + fb_dealloc_cmap(&info->cmap);
> > framebuffer_release(info);
> > }
> >
> > --
> > 1.8.3.1
> >
> > --
--
Peter
^ permalink raw reply
* Re: [PATCH] drivers: video: fbcmap: add signed type cast for comparation
From: Chen Gang @ 2013-08-20 2:24 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <51ECECA2.1020006@asianux.com>
On 08/16/2013 09:56 PM, David Herrmann wrote:
> Hi
>
> On Mon, Jul 22, 2013 at 10:26 AM, Chen Gang <gang.chen@asianux.com> wrote:
>> According to fb_set_cmap() in this file, knows about it is necessary to
>> check 'start' whether less than zero or not, so need add related type
>> cast for its comparing.
>>
>> The related warning:
>>
>> drivers/video/fbcmap.c:288:2: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
>>
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> ---
>> drivers/video/fbcmap.c | 2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c
>> index 5c3960d..44c88e3 100644
>> --- a/drivers/video/fbcmap.c
>> +++ b/drivers/video/fbcmap.c
>> @@ -285,7 +285,7 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info)
>> rc = -ENODEV;
>> goto out;
>> }
>> - if (cmap->start < 0 || (!info->fbops->fb_setcolreg &&
>> + if ((int)cmap->start < 0 || (!info->fbops->fb_setcolreg &&
>> !info->fbops->fb_setcmap)) {
>
> What's the reason to do this, anyway? fb_set_cmap() does the same but
> actually casts to a signed integer. So I think we can remove this
> check here.
>
OK, that sounds reasonable to me, and now my mail client is OK, I will
send patch v2 for it.
:-)
> Regards
> David
>
>> rc = -EINVAL;
>> goto out1;
>> --
>> 1.7.7.6
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
>
--
Chen Gang
^ permalink raw reply
* [PATCH v2] drivers: video: fbcmap: remove the redundency and incorrect checkings
From: Chen Gang @ 2013-08-20 2:33 UTC (permalink / raw)
To: linux-fbdev
fb_set_cmap() already checks the parameters, so need remove the
redundancy checking.
This redundancy checking is also incorrect, the related warning:
drivers/video/fbcmap.c:288:2: warning: comparison of unsigned expression < 0 is always false [-Wtype-limits]
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
drivers/video/fbcmap.c | 7 +------
1 files changed, 1 insertions(+), 6 deletions(-)
diff --git a/drivers/video/fbcmap.c b/drivers/video/fbcmap.c
index 5c3960d..f89245b 100644
--- a/drivers/video/fbcmap.c
+++ b/drivers/video/fbcmap.c
@@ -285,13 +285,8 @@ int fb_set_user_cmap(struct fb_cmap_user *cmap, struct fb_info *info)
rc = -ENODEV;
goto out;
}
- if (cmap->start < 0 || (!info->fbops->fb_setcolreg &&
- !info->fbops->fb_setcmap)) {
- rc = -EINVAL;
- goto out1;
- }
+
rc = fb_set_cmap(&umap, info);
-out1:
unlock_fb_info(info);
out:
fb_dealloc_cmap(&umap);
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Felipe Balbi @ 2013-08-20 12:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5211ACE9.3040302@ti.com>
[-- Attachment #1: Type: text/plain, Size: 849 bytes --]
Hi,
On Mon, Aug 19, 2013 at 10:58:09AM +0530, Kishon Vijay Abraham I wrote:
> >> So maybe let's stop solving an already solved problem and just state that
> >> you need to explicitly assign device ID to use this framework?
> >
> > Felipe,
> > Can we have it the way I had in my v10 patch series till we find a better way?
> > I think this *non-dt* stuff shouldn't be blocking as most of the users are dt only?
I don't have a lot of things against it, but preventing driver authors
to use PLATFORM_DEVID_AUTO just to use the framework is likely going to
piss some people off.
Perhaps we can start with this approach and fix things later ? At least
it ungates all the PHY drivers which are depending on this framework
(quite a few already). If everybody agrees with this approach, I'd be ok
with it too.
cheers
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
From: Konrad Rzeszutek Wilk @ 2013-08-20 19:22 UTC (permalink / raw)
To: Inki Dae
Cc: linux-fbdev, linaro-kernel, dri-devel, kyungmin.park,
myungjoo.ham, linux-arm-kernel, linux-media
In-Reply-To: <1376385576-9039-2-git-send-email-inki.dae@samsung.com>
On Tue, Aug 13, 2013 at 06:19:35PM +0900, Inki Dae wrote:
> This patch adds a buffer synchronization framework based on DMA BUF[1]
> and and based on ww-mutexes[2] for lock mechanism.
>
> The purpose of this framework is to provide not only buffer access control
> to CPU and DMA but also easy-to-use interfaces for device drivers and
> user application. This framework can be used for all dma devices using
> system memory as dma buffer, especially for most ARM based SoCs.
>
> Changelog v6:
> - Fix sync lock to multiple reads.
> - Add select system call support.
> . Wake up poll_wait when a dmabuf is unlocked.
> - Remove unnecessary the use of mutex lock.
> - Add private backend ops callbacks.
> . This ops has one callback for device drivers to clean up their
> sync object resource when the sync object is freed. For this,
> device drivers should implement the free callback properly.
> - Update document file.
>
> Changelog v5:
> - Rmove a dependence on reservation_object: the reservation_object is used
> to hook up to ttm and dma-buf for easy sharing of reservations across
> devices. However, the dmabuf sync can be used for all dma devices; v4l2
> and drm based drivers, so doesn't need the reservation_object anymore.
> With regared to this, it adds 'void *sync' to dma_buf structure.
> - All patches are rebased on mainline, Linux v3.10.
>
> Changelog v4:
> - Add user side interface for buffer synchronization mechanism and update
> descriptions related to the user side interface.
>
> Changelog v3:
> - remove cache operation relevant codes and update document file.
>
> Changelog v2:
> - use atomic_add_unless to avoid potential bug.
> - add a macro for checking valid access type.
> - code clean.
>
> The mechanism of this framework has the following steps,
> 1. Register dmabufs to a sync object - A task gets a new sync object and
> can add one or more dmabufs that the task wants to access.
> This registering should be performed when a device context or an event
> context such as a page flip event is created or before CPU accesses a shared
> buffer.
>
> dma_buf_sync_get(a sync object, a dmabuf);
>
> 2. Lock a sync object - A task tries to lock all dmabufs added in its own
> sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid dead
> lock issue and for race condition between CPU and CPU, CPU and DMA, and DMA
> and DMA. Taking a lock means that others cannot access all locked dmabufs
> until the task that locked the corresponding dmabufs, unlocks all the locked
> dmabufs.
> This locking should be performed before DMA or CPU accesses these dmabufs.
>
> dma_buf_sync_lock(a sync object);
>
> 3. Unlock a sync object - The task unlocks all dmabufs added in its own sync
> object. The unlock means that the DMA or CPU accesses to the dmabufs have
> been completed so that others may access them.
> This unlocking should be performed after DMA or CPU has completed accesses
> to the dmabufs.
>
> dma_buf_sync_unlock(a sync object);
>
> 4. Unregister one or all dmabufs from a sync object - A task unregisters
> the given dmabufs from the sync object. This means that the task dosen't
> want to lock the dmabufs.
> The unregistering should be performed after DMA or CPU has completed
> accesses to the dmabufs or when dma_buf_sync_lock() is failed.
>
> dma_buf_sync_put(a sync object, a dmabuf);
> dma_buf_sync_put_all(a sync object);
>
> The described steps may be summarized as:
> get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
>
> This framework includes the following two features.
> 1. read (shared) and write (exclusive) locks - A task is required to declare
> the access type when the task tries to register a dmabuf;
> READ, WRITE, READ DMA, or WRITE DMA.
>
> The below is example codes,
> struct dmabuf_sync *sync;
>
> sync = dmabuf_sync_init(...);
> ...
>
> dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
> ...
>
> And the below can be used as access types:
> DMA_BUF_ACCESS_R - CPU will access a buffer for read.
> DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
> DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
> DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or
> write.
>
> 2. Mandatory resource releasing - a task cannot hold a lock indefinitely.
> A task may never try to unlock a buffer after taking a lock to the buffer.
> In this case, a timer handler to the corresponding sync object is called
> in five (default) seconds and then the timed-out buffer is unlocked by work
> queue handler to avoid lockups and to enforce resources of the buffer.
>
> The below is how to use interfaces for device driver:
> 1. Allocate and Initialize a sync object:
> static void xxx_dmabuf_sync_free(void *priv)
> {
> struct xxx_context *ctx = priv;
>
> if (!ctx)
> return;
>
> ctx->sync = NULL;
> }
> ...
>
> static struct dmabuf_sync_priv_ops driver_specific_ops = {
> .free = xxx_dmabuf_sync_free,
> };
> ...
>
> struct dmabuf_sync *sync;
>
> sync = dmabuf_sync_init("test sync", &driver_specific_ops, ctx);
> ...
>
> 2. Add a dmabuf to the sync object when setting up dma buffer relevant
> registers:
> dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
> ...
>
> 3. Lock all dmabufs of the sync object before DMA or CPU accesses
> the dmabufs:
> dmabuf_sync_lock(sync);
> ...
>
> 4. Now CPU or DMA can access all dmabufs locked in step 3.
>
> 5. Unlock all dmabufs added in a sync object after DMA or CPU access
> to these dmabufs is completed:
> dmabuf_sync_unlock(sync);
>
> And call the following functions to release all resources,
> dmabuf_sync_put_all(sync);
> dmabuf_sync_fini(sync);
>
> You can refer to actual example codes:
> "drm/exynos: add dmabuf sync support for g2d driver" and
> "drm/exynos: add dmabuf sync support for kms framework" from
> https://git.kernel.org/cgit/linux/kernel/git/daeinki/
> drm-exynos.git/log/?h=dmabuf-sync
>
> And this framework includes fcntl system call[3] as interfaces exported
> to user. As you know, user sees a buffer object as a dma-buf file descriptor.
> So fcntl() call with the file descriptor means to lock some buffer region being
> managed by the dma-buf object.
>
> The below is how to use interfaces for user application:
>
> fcntl system call:
>
> struct flock filelock;
>
> 1. Lock a dma buf:
> filelock.l_type = F_WRLCK or F_RDLCK;
>
> /* lock entire region to the dma buf. */
> filelock.lwhence = SEEK_CUR;
> filelock.l_start = 0;
> filelock.l_len = 0;
>
> fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> ...
> CPU access to the dma buf
>
> 2. Unlock a dma buf:
> filelock.l_type = F_UNLCK;
>
> fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
>
> close(dmabuf fd) call would also unlock the dma buf. And for more
> detail, please refer to [3]
>
> select system call:
>
> fd_set wdfs or rdfs;
>
> FD_ZERO(&wdfs or &rdfs);
> FD_SET(fd, &wdfs or &rdfs);
>
> select(fd + 1, &rdfs, NULL, NULL, NULL);
> or
> select(fd + 1, NULL, &wdfs, NULL, NULL);
>
> Every time select system call is called, a caller will wait for
> the completion of DMA or CPU access to a shared buffer if there
> is someone accessing the shared buffer; locked the shared buffer.
> However, if no anyone then select system call will be returned
> at once.
>
> References:
> [1] http://lwn.net/Articles/470339/
> [2] https://patchwork.kernel.org/patch/2625361/
> [3] http://linux.die.net/man/2/fcntl
>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> Documentation/dma-buf-sync.txt | 285 +++++++++++++++++
> drivers/base/Kconfig | 7 +
> drivers/base/Makefile | 1 +
> drivers/base/dma-buf.c | 4 +
> drivers/base/dmabuf-sync.c | 678 ++++++++++++++++++++++++++++++++++++++++
> include/linux/dma-buf.h | 16 +
> include/linux/dmabuf-sync.h | 190 +++++++++++
> 7 files changed, 1181 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/dma-buf-sync.txt
> create mode 100644 drivers/base/dmabuf-sync.c
> create mode 100644 include/linux/dmabuf-sync.h
>
> diff --git a/Documentation/dma-buf-sync.txt b/Documentation/dma-buf-sync.txt
> new file mode 100644
> index 0000000..8023d06
> --- /dev/null
> +++ b/Documentation/dma-buf-sync.txt
> @@ -0,0 +1,285 @@
> + DMA Buffer Synchronization Framework
> + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +
> + Inki Dae
> + <inki dot dae at samsung dot com>
> + <daeinki at gmail dot com>
> +
> +This document is a guide for device-driver writers describing the DMA buffer
> +synchronization API. This document also describes how to use the API to
> +use buffer synchronization mechanism between DMA and DMA, CPU and DMA, and
> +CPU and CPU.
> +
> +The DMA Buffer synchronization API provides buffer synchronization mechanism;
> +i.e., buffer access control to CPU and DMA, and easy-to-use interfaces for
> +device drivers and user application. And this API can be used for all dma
> +devices using system memory as dma buffer, especially for most ARM based SoCs.
> +
> +
> +Motivation
> +----------
> +
> +Buffer synchronization issue between DMA and DMA:
> + Sharing a buffer, a device cannot be aware of when the other device
> + will access the shared buffer: a device may access a buffer containing
> + wrong data if the device accesses the shared buffer while another
> + device is still accessing the shared buffer.
> + Therefore, a user process should have waited for the completion of DMA
> + access by another device before a device tries to access the shared
> + buffer.
> +
> +Buffer synchronization issue between CPU and DMA:
> + A user process should consider that when having to send a buffer, filled
> + by CPU, to a device driver for the device driver to access the buffer as
> + a input buffer while CPU and DMA are sharing the buffer.
> + This means that the user process needs to understand how the device
> + driver is worked. Hence, the conventional mechanism not only makes
> + user application complicated but also incurs performance overhead.
> +
> +Buffer synchronization issue between CPU and CPU:
> + In case that two processes share one buffer; shared with DMA also,
> + they may need some mechanism to allow process B to access the shared
> + buffer after the completion of CPU access by process A.
> + Therefore, process B should have waited for the completion of CPU access
> + by process A using the mechanism before trying to access the shared
> + buffer.
> +
> +What is the best way to solve these buffer synchronization issues?
> + We may need a common object that a device driver and a user process
> + notify the common object of when they try to access a shared buffer.
> + That way we could decide when we have to allow or not to allow for CPU
> + or DMA to access the shared buffer through the common object.
> + If so, what could become the common object? Right, that's a dma-buf[1].
> + Now we have already been using the dma-buf to share one buffer with
> + other drivers.
> +
> +
> +Basic concept
> +-------------
> +
> +The mechanism of this framework has the following steps,
> + 1. Register dmabufs to a sync object - A task gets a new sync object and
> + can add one or more dmabufs that the task wants to access.
> + This registering should be performed when a device context or an event
> + context such as a page flip event is created or before CPU accesses a shared
> + buffer.
> +
> + dma_buf_sync_get(a sync object, a dmabuf);
> +
> + 2. Lock a sync object - A task tries to lock all dmabufs added in its own
> + sync object. Basically, the lock mechanism uses ww-mutexes[2] to avoid dead
> + lock issue and for race condition between CPU and CPU, CPU and DMA, and DMA
> + and DMA. Taking a lock means that others cannot access all locked dmabufs
> + until the task that locked the corresponding dmabufs, unlocks all the locked
> + dmabufs.
> + This locking should be performed before DMA or CPU accesses these dmabufs.
> +
> + dma_buf_sync_lock(a sync object);
> +
> + 3. Unlock a sync object - The task unlocks all dmabufs added in its own sync
> + object. The unlock means that the DMA or CPU accesses to the dmabufs have
> + been completed so that others may access them.
> + This unlocking should be performed after DMA or CPU has completed accesses
> + to the dmabufs.
> +
> + dma_buf_sync_unlock(a sync object);
> +
> + 4. Unregister one or all dmabufs from a sync object - A task unregisters
> + the given dmabufs from the sync object. This means that the task dosen't
> + want to lock the dmabufs.
> + The unregistering should be performed after DMA or CPU has completed
> + accesses to the dmabufs or when dma_buf_sync_lock() is failed.
> +
> + dma_buf_sync_put(a sync object, a dmabuf);
> + dma_buf_sync_put_all(a sync object);
> +
> + The described steps may be summarized as:
> + get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
> +
> +This framework includes the following two features.
> + 1. read (shared) and write (exclusive) locks - A task is required to declare
> + the access type when the task tries to register a dmabuf;
> + READ, WRITE, READ DMA, or WRITE DMA.
> +
> + The below is example codes,
> + struct dmabuf_sync *sync;
> +
> + sync = dmabuf_sync_init(NULL, "test sync");
> +
> + dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
> + ...
> +
> + 2. Mandatory resource releasing - a task cannot hold a lock indefinitely.
> + A task may never try to unlock a buffer after taking a lock to the buffer.
> + In this case, a timer handler to the corresponding sync object is called
> + in five (default) seconds and then the timed-out buffer is unlocked by work
> + queue handler to avoid lockups and to enforce resources of the buffer.
> +
> +
> +Access types
> +------------
> +
> +DMA_BUF_ACCESS_R - CPU will access a buffer for read.
> +DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
> +DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
> +DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or write.
> +
> +
> +Generic user interfaces
> +-----------------------
> +
> +And this framework includes fcntl system call[3] as interfaces exported
> +to user. As you know, user sees a buffer object as a dma-buf file descriptor.
> +So fcntl() call with the file descriptor means to lock some buffer region being
> +managed by the dma-buf object.
> +
> +
> +API set
> +-------
> +
> +bool is_dmabuf_sync_supported(void)
> + - Check if dmabuf sync is supported or not.
> +
> +struct dmabuf_sync *dmabuf_sync_init(const char *name,
> + struct dmabuf_sync_priv_ops *ops,
> + void priv*)
> + - Allocate and initialize a new sync object. The caller can get a new
> + sync object for buffer synchronization. ops is used for device driver
> + to clean up its own sync object. For this, each device driver should
> + implement a free callback. priv is used for device driver to get its
> + device context when free callback is called.
> +
> +void dmabuf_sync_fini(struct dmabuf_sync *sync)
> + - Release all resources to the sync object.
> +
> +int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
> + unsigned int type)
> + - Get dmabuf sync object. Internally, this function allocates
> + a dmabuf_sync object and adds a given dmabuf to it, and also takes
> + a reference to the dmabuf. The caller can tie up multiple dmabufs
> + into one sync object by calling this function several times.
> +
> +void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
> + - Put dmabuf sync object to a given dmabuf. Internally, this function
> + removes a given dmabuf from a sync object and remove the sync object.
> + At this time, the dmabuf is putted.
> +
> +void dmabuf_sync_put_all(struct dmabuf_sync *sync)
> + - Put dmabuf sync object to dmabufs. Internally, this function removes
> + all dmabufs from a sync object and remove the sync object.
> + At this time, all dmabufs are putted.
> +
> +int dmabuf_sync_lock(struct dmabuf_sync *sync)
> + - Lock all dmabufs added in a sync object. The caller should call this
> + function prior to CPU or DMA access to the dmabufs so that others can
> + not access the dmabufs. Internally, this function avoids dead lock
> + issue with ww-mutexes.
> +
> +int dmabuf_sync_single_lock(struct dma_buf *dmabuf)
> + - Lock a dmabuf. The caller should call this
> + function prior to CPU or DMA access to the dmabuf so that others can
> + not access the dmabuf.
> +
> +int dmabuf_sync_unlock(struct dmabuf_sync *sync)
> + - Unlock all dmabufs added in a sync object. The caller should call
> + this function after CPU or DMA access to the dmabufs is completed so
> + that others can access the dmabufs.
> +
> +void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
> + - Unlock a dmabuf. The caller should call this function after CPU or
> + DMA access to the dmabuf is completed so that others can access
> + the dmabuf.
> +
> +
> +Tutorial for device driver
> +--------------------------
> +
> +1. Allocate and Initialize a sync object:
> + static void xxx_dmabuf_sync_free(void *priv)
> + {
> + struct xxx_context *ctx = priv;
> +
> + if (!ctx)
> + return;
> +
> + ctx->sync = NULL;
> + }
> + ...
> +
> + static struct dmabuf_sync_priv_ops driver_specific_ops = {
> + .free = xxx_dmabuf_sync_free,
> + };
> + ...
> +
> + struct dmabuf_sync *sync;
> +
> + sync = dmabuf_sync_init("test sync", &driver_specific_ops, ctx);
> + ...
> +
> +2. Add a dmabuf to the sync object when setting up dma buffer relevant registers:
> + dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
> + ...
> +
> +3. Lock all dmabufs of the sync object before DMA or CPU accesses the dmabufs:
> + dmabuf_sync_lock(sync);
> + ...
> +
> +4. Now CPU or DMA can access all dmabufs locked in step 3.
> +
> +5. Unlock all dmabufs added in a sync object after DMA or CPU access to these
> + dmabufs is completed:
> + dmabuf_sync_unlock(sync);
> +
> + And call the following functions to release all resources,
> + dmabuf_sync_put_all(sync);
> + dmabuf_sync_fini(sync);
> +
> +
> +Tutorial for user application
> +-----------------------------
> +fcntl system call:
> +
> + struct flock filelock;
> +
> +1. Lock a dma buf:
> + filelock.l_type = F_WRLCK or F_RDLCK;
> +
> + /* lock entire region to the dma buf. */
> + filelock.lwhence = SEEK_CUR;
> + filelock.l_start = 0;
> + filelock.l_len = 0;
> +
> + fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> + ...
> + CPU access to the dma buf
> +
> +2. Unlock a dma buf:
> + filelock.l_type = F_UNLCK;
> +
> + fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> +
> + close(dmabuf fd) call would also unlock the dma buf. And for more
> + detail, please refer to [3]
> +
> +
> +select system call:
> +
> + fd_set wdfs or rdfs;
> +
> + FD_ZERO(&wdfs or &rdfs);
> + FD_SET(fd, &wdfs or &rdfs);
> +
> + select(fd + 1, &rdfs, NULL, NULL, NULL);
> + or
> + select(fd + 1, NULL, &wdfs, NULL, NULL);
> +
> + Every time select system call is called, a caller will wait for
> + the completion of DMA or CPU access to a shared buffer if there is
> + someone accessing the shared buffer; locked the shared buffer.
> + However, if no anyone then select system call will be returned
> + at once.
> +
> +References:
> +[1] http://lwn.net/Articles/470339/
> +[2] https://patchwork.kernel.org/patch/2625361/
> +[3] http://linux.die.net/man/2/fcntl
> diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> index 5daa259..35e1518 100644
> --- a/drivers/base/Kconfig
> +++ b/drivers/base/Kconfig
> @@ -200,6 +200,13 @@ config DMA_SHARED_BUFFER
> APIs extension; the file's descriptor can then be passed on to other
> driver.
>
> +config DMABUF_SYNC
> + bool "DMABUF Synchronization Framework"
> + depends on DMA_SHARED_BUFFER
> + help
> + This option enables dmabuf sync framework for buffer synchronization between
> + DMA and DMA, CPU and DMA, and CPU and CPU.
> +
> config CMA
> bool "Contiguous Memory Allocator"
> depends on HAVE_DMA_CONTIGUOUS && HAVE_MEMBLOCK
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 48029aa..e06a5d7 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -11,6 +11,7 @@ obj-y += power/
> obj-$(CONFIG_HAS_DMA) += dma-mapping.o
> obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
> obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o reservation.o
> +obj-$(CONFIG_DMABUF_SYNC) += dmabuf-sync.o
> obj-$(CONFIG_ISA) += isa.o
> obj-$(CONFIG_FW_LOADER) += firmware_class.o
> obj-$(CONFIG_NUMA) += node.o
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index 6687ba7..4aca57a 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -29,6 +29,7 @@
> #include <linux/export.h>
> #include <linux/debugfs.h>
> #include <linux/seq_file.h>
> +#include <linux/dmabuf-sync.h>
>
> static inline int is_dma_buf_file(struct file *);
>
> @@ -56,6 +57,8 @@ static int dma_buf_release(struct inode *inode, struct file *file)
> list_del(&dmabuf->list_node);
> mutex_unlock(&db_list.lock);
>
> + dmabuf_sync_reservation_fini(dmabuf);
> +
> kfree(dmabuf);
> return 0;
> }
> @@ -134,6 +137,7 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,
>
> file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, flags);
>
> + dmabuf_sync_reservation_init(dmabuf);
> dmabuf->file = file;
>
> mutex_init(&dmabuf->lock);
> diff --git a/drivers/base/dmabuf-sync.c b/drivers/base/dmabuf-sync.c
> new file mode 100644
> index 0000000..fbe711c
> --- /dev/null
> +++ b/drivers/base/dmabuf-sync.c
> @@ -0,0 +1,678 @@
> +/*
> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
> + * Authors:
> + * Inki Dae <inki.dae@samsung.com>
> + *
> + * 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.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/debugfs.h>
> +#include <linux/uaccess.h>
> +
> +#include <linux/dmabuf-sync.h>
> +
> +#define MAX_SYNC_TIMEOUT 5 /* Second. */
> +
> +int dmabuf_sync_enabled = 1;
> +
> +MODULE_PARM_DESC(enabled, "Check if dmabuf sync is supported or not");
> +module_param_named(enabled, dmabuf_sync_enabled, int, 0444);
> +
> +DEFINE_WW_CLASS(dmabuf_sync_ww_class);
> +EXPORT_SYMBOL(dmabuf_sync_ww_class);
> +
> +static void dmabuf_sync_timeout_worker(struct work_struct *work)
> +{
> + struct dmabuf_sync *sync = container_of(work, struct dmabuf_sync, work);
> + struct dmabuf_sync_object *sobj;
> +
> + mutex_lock(&sync->lock);
> +
> + list_for_each_entry(sobj, &sync->syncs, head) {
You are using the 'sobj->robj' quite a lot. Why not just use a temp structure:
struct dmabuf_sync_reservation *rsvp = sobj->robj;
and use that in this function. It would make it easier to read I think.
> + BUG_ON(!sobj->robj);
> +
> + mutex_lock(&sobj->robj->lock);
> +
> + printk(KERN_WARNING "%s: timeout = 0x%x [type = %d:%d, " \
> + "refcnt = %d, locked = %d]\n",
> + sync->name, (u32)sobj->dmabuf,
> + sobj->robj->accessed_type,
> + sobj->access_type,
> + atomic_read(&sobj->robj->shared_cnt),
> + sobj->robj->locked);
pr_warn_ratelimited?
> +
> + /* unlock only valid sync object. */
> + if (!sobj->robj->locked) {
> + mutex_unlock(&sobj->robj->lock);
> + continue;
> + }
> +
> + if (sobj->robj->polled) {
> + sobj->robj->poll_event = true;
> + sobj->robj->polled = false;
> + wake_up_interruptible(&sobj->robj->poll_wait);
> + }
> +
> + if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
> + mutex_unlock(&sobj->robj->lock);
> + continue;
> + }
> +
> + mutex_unlock(&sobj->robj->lock);
> +
> + ww_mutex_unlock(&sobj->robj->sync_lock);
> +
> + mutex_lock(&sobj->robj->lock);
> + sobj->robj->locked = false;
> +
> + if (sobj->access_type & DMA_BUF_ACCESS_R)
> + printk(KERN_WARNING "%s: r-unlocked = 0x%x\n",
> + sync->name, (u32)sobj->dmabuf);
> + else
> + printk(KERN_WARNING "%s: w-unlocked = 0x%x\n",
> + sync->name, (u32)sobj->dmabuf);
How about using 'pr_warn'? And in it have:
sobj->access_type & DMA_BUF_ACCESS_R ? "r-" : "w-",
and just have one printk.
Why the (u32) casting? Don't you want %p ?
> +
> + mutex_unlock(&sobj->robj->lock);
> + }
> +
> + sync->status = 0;
> + mutex_unlock(&sync->lock);
> +
> + dmabuf_sync_put_all(sync);
> + dmabuf_sync_fini(sync);
> +}
> +
> +static void dmabuf_sync_lock_timeout(unsigned long arg)
> +{
> + struct dmabuf_sync *sync = (struct dmabuf_sync *)arg;
> +
> + schedule_work(&sync->work);
> +}
> +
> +static int dmabuf_sync_lock_objs(struct dmabuf_sync *sync,
> + struct ww_acquire_ctx *ctx)
> +{
> + struct dmabuf_sync_object *contended_sobj = NULL;
> + struct dmabuf_sync_object *res_sobj = NULL;
> + struct dmabuf_sync_object *sobj = NULL;
> + int ret;
> +
> + if (ctx)
> + ww_acquire_init(ctx, &dmabuf_sync_ww_class);
> +
> +retry:
> + list_for_each_entry(sobj, &sync->syncs, head) {
> + if (WARN_ON(!sobj->robj))
> + continue;
> +
> + mutex_lock(&sobj->robj->lock);
> +
> + /* Don't lock in case of read and read. */
> + if (sobj->robj->accessed_type & DMA_BUF_ACCESS_R &&
> + sobj->access_type & DMA_BUF_ACCESS_R) {
> + atomic_inc(&sobj->robj->shared_cnt);
> + mutex_unlock(&sobj->robj->lock);
> + continue;
> + }
> +
> + if (sobj = res_sobj) {
> + res_sobj = NULL;
> + mutex_unlock(&sobj->robj->lock);
> + continue;
> + }
> +
> + mutex_unlock(&sobj->robj->lock);
> +
> + ret = ww_mutex_lock(&sobj->robj->sync_lock, ctx);
> + if (ret < 0) {
> + contended_sobj = sobj;
> +
> + if (ret = -EDEADLK)
> + printk(KERN_WARNING"%s: deadlock = 0x%x\n",
> + sync->name, (u32)sobj->dmabuf);
Again, why (u32) and not %p?
> + goto err;
This looks odd. You jump to err, which jumps back to 'retry'. Won't this
cause an infinite loop? Perhaps you need to add a retry counter to only
do this up to five times or so and then give up?
> + }
> +
> + mutex_lock(&sobj->robj->lock);
> + sobj->robj->locked = true;
> +
> + mutex_unlock(&sobj->robj->lock);
> + }
> +
> + if (ctx)
> + ww_acquire_done(ctx);
> +
> + init_timer(&sync->timer);
> +
> + sync->timer.data = (unsigned long)sync;
> + sync->timer.function = dmabuf_sync_lock_timeout;
> + sync->timer.expires = jiffies + (HZ * MAX_SYNC_TIMEOUT);
> +
> + add_timer(&sync->timer);
> +
> + return 0;
> +
> +err:
> + list_for_each_entry_continue_reverse(sobj, &sync->syncs, head) {
> + mutex_lock(&sobj->robj->lock);
> +
> + /* Don't need to unlock in case of read and read. */
> + if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
> + mutex_unlock(&sobj->robj->lock);
> + continue;
> + }
> +
> + ww_mutex_unlock(&sobj->robj->sync_lock);
> + sobj->robj->locked = false;
> +
> + mutex_unlock(&sobj->robj->lock);
> + }
> +
> + if (res_sobj) {
> + mutex_lock(&res_sobj->robj->lock);
> +
> + if (!atomic_add_unless(&res_sobj->robj->shared_cnt, -1, 1)) {
> + ww_mutex_unlock(&res_sobj->robj->sync_lock);
> + res_sobj->robj->locked = false;
> + }
> +
> + mutex_unlock(&res_sobj->robj->lock);
> + }
> +
> + if (ret = -EDEADLK) {
> + ww_mutex_lock_slow(&contended_sobj->robj->sync_lock, ctx);
> + res_sobj = contended_sobj;
> +
> + goto retry;
> + }
> +
> + if (ctx)
> + ww_acquire_fini(ctx);
> +
> + return ret;
> +}
> +
> +static void dmabuf_sync_unlock_objs(struct dmabuf_sync *sync,
> + struct ww_acquire_ctx *ctx)
> +{
> + struct dmabuf_sync_object *sobj;
> +
> + if (list_empty(&sync->syncs))
> + return;
> +
> + mutex_lock(&sync->lock);
> +
> + list_for_each_entry(sobj, &sync->syncs, head) {
> + mutex_lock(&sobj->robj->lock);
> +
> + if (sobj->robj->polled) {
> + sobj->robj->poll_event = true;
> + sobj->robj->polled = false;
> + wake_up_interruptible(&sobj->robj->poll_wait);
> + }
> +
> + if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
> + mutex_unlock(&sobj->robj->lock);
> + continue;
> + }
> +
> + mutex_unlock(&sobj->robj->lock);
> +
> + ww_mutex_unlock(&sobj->robj->sync_lock);
> +
> + mutex_lock(&sobj->robj->lock);
> + sobj->robj->locked = false;
> + mutex_unlock(&sobj->robj->lock);
> + }
> +
> + mutex_unlock(&sync->lock);
> +
> + if (ctx)
> + ww_acquire_fini(ctx);
> +
> + del_timer(&sync->timer);
> +}
> +
> +/**
> + * is_dmabuf_sync_supported - Check if dmabuf sync is supported or not.
> + */
> +bool is_dmabuf_sync_supported(void)
> +{
> + return dmabuf_sync_enabled = 1;
> +}
> +EXPORT_SYMBOL(is_dmabuf_sync_supported);
_GPL ?
I would also prefix it with 'dmabuf_is_sync_supported' just to make
all of the libraries call start with 'dmabuf'
> +
> +/**
> + * dmabuf_sync_init - Allocate and initialize a dmabuf sync.
> + *
> + * @priv: A device private data.
> + * @name: A sync object name.
> + *
> + * This function should be called when a device context or an event
> + * context such as a page flip event is created. And the created
> + * dmabuf_sync object should be set to the context.
> + * The caller can get a new sync object for buffer synchronization
> + * through this function.
> + */
> +struct dmabuf_sync *dmabuf_sync_init(const char *name,
> + struct dmabuf_sync_priv_ops *ops,
> + void *priv)
> +{
> + struct dmabuf_sync *sync;
> +
> + sync = kzalloc(sizeof(*sync), GFP_KERNEL);
> + if (!sync)
> + return ERR_PTR(-ENOMEM);
> +
> + strncpy(sync->name, name, ARRAY_SIZE(sync->name) - 1);
> +
That is odd usage of an ARRAY_SIZE, but I can see how you can use it.
I would say you should just do a #define for the 64 line and use that
instead.
> + sync->ops = ops;
> + sync->priv = priv;
> + INIT_LIST_HEAD(&sync->syncs);
> + mutex_init(&sync->lock);
> + INIT_WORK(&sync->work, dmabuf_sync_timeout_worker);
> +
> + return sync;
> +}
> +EXPORT_SYMBOL(dmabuf_sync_init);
_GPL ?
> +
> +/**
> + * dmabuf_sync_fini - Release a given dmabuf sync.
> + *
> + * @sync: An object to dmabuf_sync structure.
> + *
> + * This function should be called if some operation is failed after
> + * dmabuf_sync_init call to release relevant resources, and after
> + * dmabuf_sync_unlock function is called.
> + */
> +void dmabuf_sync_fini(struct dmabuf_sync *sync)
> +{
> + if (WARN_ON(!sync))
> + return;
> +
> + if (sync->ops && sync->ops->free)
> + sync->ops->free(sync->priv);
> +
No need to cancel the sync->work in case that is still
running?
> + kfree(sync);
> +}
> +EXPORT_SYMBOL(dmabuf_sync_fini);
_GPL ?
> +
> +/*
> + * dmabuf_sync_get_obj - Add a given object to syncs list.
sync's list I think?
> + *
> + * @sync: An object to dmabuf_sync structure.
> + * @dmabuf: An object to dma_buf structure.
> + * @type: A access type to a dma buf.
> + * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
> + * others for read access. On the other hand, the DMA_BUF_ACCESS_W
> + * means that this dmabuf couldn't be accessed by others but would be
> + * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can be
> + * combined.
Should this be an enum?
> + *
> + * This function creates and initializes a new dmabuf sync object and it adds
> + * the dmabuf sync object to syncs list to track and manage all dmabufs.
> + */
> +static int dmabuf_sync_get_obj(struct dmabuf_sync *sync, struct dma_buf *dmabuf,
> + unsigned int type)
enum for 'type'?
> +{
> + struct dmabuf_sync_object *sobj;
> +
> + if (!dmabuf->sync) {
> + WARN_ON(1);
> + return -EFAULT;
> + }
> +
> + if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type))
> + return -EINVAL;
> +
> + if ((type & DMA_BUF_ACCESS_RW) = DMA_BUF_ACCESS_RW)
> + type &= ~DMA_BUF_ACCESS_R;
Ah, that is why you are not using an enum.
> +
> + sobj = kzalloc(sizeof(*sobj), GFP_KERNEL);
> + if (!sobj) {
> + WARN_ON(1);
I think you can skip that WARN_ON. Handling an -ENOMEM should be
something fairly easy to handle by the calleer.
> + return -ENOMEM;
> + }
> +
> + get_dma_buf(dmabuf);
> +
> + sobj->dmabuf = dmabuf;
> + sobj->robj = dmabuf->sync;
> + sobj->access_type = type;
> +
> + mutex_lock(&sync->lock);
> + list_add_tail(&sobj->head, &sync->syncs);
> + mutex_unlock(&sync->lock);
> +
> + return 0;
> +}
> +
> +/*
> + * dmabuf_sync_put_obj - Release a given sync object.
> + *
> + * @sync: An object to dmabuf_sync structure.
> + *
> + * This function should be called if some operation is failed after
s/is//
> + * dmabuf_sync_get_obj call to release a given sync object.
> + */
> +static void dmabuf_sync_put_obj(struct dmabuf_sync *sync,
> + struct dma_buf *dmabuf)
> +{
> + struct dmabuf_sync_object *sobj;
> +
> + mutex_lock(&sync->lock);
> +
> + list_for_each_entry(sobj, &sync->syncs, head) {
> + if (sobj->dmabuf != dmabuf)
> + continue;
> +
> + dma_buf_put(sobj->dmabuf);
> +
> + list_del_init(&sobj->head);
> + kfree(sobj);
> + break;
> + }
> +
> + if (list_empty(&sync->syncs))
> + sync->status = 0;
> +
> + mutex_unlock(&sync->lock);
> +}
> +
> +/*
> + * dmabuf_sync_put_objs - Release all sync objects of dmabuf_sync.
> + *
> + * @sync: An object to dmabuf_sync structure.
> + *
> + * This function should be called if some operation is failed after
s/is//
> + * dmabuf_sync_get_obj call to release all sync objects.
> + */
> +static void dmabuf_sync_put_objs(struct dmabuf_sync *sync)
> +{
> + struct dmabuf_sync_object *sobj, *next;
> +
> + mutex_lock(&sync->lock);
> +
> + list_for_each_entry_safe(sobj, next, &sync->syncs, head) {
> + dma_buf_put(sobj->dmabuf);
> +
> + list_del_init(&sobj->head);
> + kfree(sobj);
> + }
> +
> + mutex_unlock(&sync->lock);
> +
> + sync->status = 0;
> +}
> +
> +/**
> + * dmabuf_sync_lock - lock all dmabufs added to syncs list.
> + *
> + * @sync: An object to dmabuf_sync structure.
> + *
> + * The caller should call this function prior to CPU or DMA access to
> + * the dmabufs so that others can not access the dmabufs.
> + * Internally, this function avoids dead lock issue with ww-mutex.
> + */
> +int dmabuf_sync_lock(struct dmabuf_sync *sync)
> +{
> + int ret;
> +
> + if (!sync) {
> + WARN_ON(1);
> + return -EFAULT;
> + }
> +
> + if (list_empty(&sync->syncs))
> + return -EINVAL;
> +
> + if (sync->status != DMABUF_SYNC_GOT)
> + return -EINVAL;
> +
> + ret = dmabuf_sync_lock_objs(sync, &sync->ctx);
> + if (ret < 0) {
> + WARN_ON(1);
Perhaps also include the ret value in the WARN?
> + return ret;
> + }
> +
> + sync->status = DMABUF_SYNC_LOCKED;
> +
> + return ret;
> +}
> +EXPORT_SYMBOL(dmabuf_sync_lock);
I think you know what I am going to say.
> +
> +/**
> + * dmabuf_sync_unlock - unlock all objects added to syncs list.
> + *
> + * @sync: An object to dmabuf_sync structure.
> + *
> + * The caller should call this function after CPU or DMA access to
> + * the dmabufs is completed so that others can access the dmabufs.
> + */
> +int dmabuf_sync_unlock(struct dmabuf_sync *sync)
> +{
> + if (!sync) {
> + WARN_ON(1);
> + return -EFAULT;
> + }
> +
> + /* If current dmabuf sync object wasn't reserved then just return. */
> + if (sync->status != DMABUF_SYNC_LOCKED)
> + return -EAGAIN;
> +
> + dmabuf_sync_unlock_objs(sync, &sync->ctx);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(dmabuf_sync_unlock);
> +
> +/**
> + * dmabuf_sync_single_lock - lock a dma buf.
> + *
> + * @dmabuf: A dma buf object that tries to lock.
> + * @type: A access type to a dma buf.
> + * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
> + * others for read access. On the other hand, the DMA_BUF_ACCESS_W
> + * means that this dmabuf couldn't be accessed by others but would be
> + * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can
> + * be combined with other.
> + * @wait: Indicate whether caller is blocked or not.
> + * true means that caller will be blocked, and false means that this
> + * function will return -EAGAIN if this caller can't take the lock
> + * right now.
> + *
> + * The caller should call this function prior to CPU or DMA access to the dmabuf
> + * so that others cannot access the dmabuf.
> + */
> +int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
> + bool wait)
> +{
> + struct dmabuf_sync_reservation *robj;
> +
> + if (!dmabuf->sync) {
> + WARN_ON(1);
> + return -EFAULT;
> + }
> +
> + if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type)) {
> + WARN_ON(1);
> + return -EINVAL;
> + }
> +
> + get_dma_buf(dmabuf);
> + robj = dmabuf->sync;
> +
> + mutex_lock(&robj->lock);
> +
> + /* Don't lock in case of read and read. */
> + if (robj->accessed_type & DMA_BUF_ACCESS_R && type & DMA_BUF_ACCESS_R) {
> + atomic_inc(&robj->shared_cnt);
> + mutex_unlock(&robj->lock);
> + return 0;
> + }
> +
> + /*
> + * In case of F_SETLK, just return -EAGAIN if this dmabuf has already
> + * been locked.
> + */
> + if (!wait && robj->locked) {
> + mutex_unlock(&robj->lock);
> + dma_buf_put(dmabuf);
> + return -EAGAIN;
> + }
> +
> + mutex_unlock(&robj->lock);
> +
> + mutex_lock(&robj->sync_lock.base);
> +
> + mutex_lock(&robj->lock);
> + robj->locked = true;
> + mutex_unlock(&robj->lock);
Are you missing an mutex_unlock on &robj->sync_lock.base?
Oh wait, that is the purpose of this code. You might want
to put a nice comment right above that and say: "Unlocked
by dmabuf_sync_single_unlock"
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(dmabuf_sync_single_lock);
> +
> +/**
> + * dmabuf_sync_single_unlock - unlock a dma buf.
> + *
> + * @dmabuf: A dma buf object that tries to unlock.
> + *
> + * The caller should call this function after CPU or DMA access to
> + * the dmabuf is completed so that others can access the dmabuf.
> + */
> +void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
> +{
> + struct dmabuf_sync_reservation *robj;
> +
> + if (!dmabuf->sync) {
> + WARN_ON(1);
> + return;
> + }
> +
> + robj = dmabuf->sync;
> +
> + mutex_lock(&robj->lock);
> +
> + if (robj->polled) {
> + robj->poll_event = true;
> + robj->polled = false;
> + wake_up_interruptible(&robj->poll_wait);
> + }
> +
> + if (atomic_add_unless(&robj->shared_cnt, -1 , 1)) {
> + mutex_unlock(&robj->lock);
> + dma_buf_put(dmabuf);
> + return;
> + }
> +
> + mutex_unlock(&robj->lock);
> +
> + mutex_unlock(&robj->sync_lock.base);
> +
> + mutex_lock(&robj->lock);
> + robj->locked = false;
> + mutex_unlock(&robj->lock);
> +
> + dma_buf_put(dmabuf);
> +
> + return;
> +}
> +EXPORT_SYMBOL(dmabuf_sync_single_unlock);
> +
> +/**
> + * dmabuf_sync_get - Get dmabuf sync object.
> + *
> + * @sync: An object to dmabuf_sync structure.
> + * @sync_buf: A dmabuf object to be synchronized with others.
> + * @type: A access type to a dma buf.
> + * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
> + * others for read access. On the other hand, the DMA_BUF_ACCESS_W
> + * means that this dmabuf couldn't be accessed by others but would be
> + * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can
> + * be combined with other.
> + *
> + * This function should be called after dmabuf_sync_init function is called.
> + * The caller can tie up multiple dmabufs into one sync object by calling this
> + * function several times. Internally, this function allocates
> + * a dmabuf_sync_object and adds a given dmabuf to it, and also takes
> + * a reference to a dmabuf.
> + */
> +int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf, unsigned int type)
> +{
> + int ret;
> +
> + if (!sync || !sync_buf) {
> + WARN_ON(1);
> + return -EFAULT;
> + }
> +
> + ret = dmabuf_sync_get_obj(sync, sync_buf, type);
> + if (ret < 0) {
> + WARN_ON(1);
> + return ret;
> + }
> +
> + sync->status = DMABUF_SYNC_GOT;
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(dmabuf_sync_get);
> +
> +/**
> + * dmabuf_sync_put - Put dmabuf sync object to a given dmabuf.
> + *
> + * @sync: An object to dmabuf_sync structure.
> + * @dmabuf: An dmabuf object.
> + *
> + * This function should be called if some operation is failed after
> + * dmabuf_sync_get function is called to release the dmabuf, or
> + * dmabuf_sync_unlock function is called. Internally, this function
> + * removes a given dmabuf from a sync object and remove the sync object.
> + * At this time, the dmabuf is putted.
> + */
> +void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
> +{
> + if (!sync || !dmabuf) {
> + WARN_ON(1);
> + return;
> + }
> +
> + if (list_empty(&sync->syncs))
> + return;
> +
> + dmabuf_sync_put_obj(sync, dmabuf);
> +}
> +EXPORT_SYMBOL(dmabuf_sync_put);
> +
> +/**
> + * dmabuf_sync_put_all - Put dmabuf sync object to dmabufs.
> + *
> + * @sync: An object to dmabuf_sync structure.
> + *
> + * This function should be called if some operation is failed after
> + * dmabuf_sync_get function is called to release all sync objects, or
> + * dmabuf_sync_unlock function is called. Internally, this function
> + * removes dmabufs from a sync object and remove the sync object.
> + * At this time, all dmabufs are putted.
> + */
> +void dmabuf_sync_put_all(struct dmabuf_sync *sync)
> +{
> + if (!sync) {
> + WARN_ON(1);
> + return;
> + }
> +
> + if (list_empty(&sync->syncs))
> + return;
> +
> + dmabuf_sync_put_objs(sync);
> +}
> +EXPORT_SYMBOL(dmabuf_sync_put_all);
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index dfac5ed..0109673 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -115,6 +115,7 @@ struct dma_buf_ops {
> * @exp_name: name of the exporter; useful for debugging.
> * @list_node: node for dma_buf accounting and debugging.
> * @priv: exporter specific private data for this buffer object.
> + * @sync: sync object linked to this dma-buf
> */
> struct dma_buf {
> size_t size;
> @@ -128,6 +129,7 @@ struct dma_buf {
> const char *exp_name;
> struct list_head list_node;
> void *priv;
> + void *sync;
> };
>
> /**
> @@ -148,6 +150,20 @@ struct dma_buf_attachment {
> void *priv;
> };
>
> +#define DMA_BUF_ACCESS_R 0x1
> +#define DMA_BUF_ACCESS_W 0x2
> +#define DMA_BUF_ACCESS_DMA 0x4
> +#define DMA_BUF_ACCESS_RW (DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_W)
> +#define DMA_BUF_ACCESS_DMA_R (DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_DMA)
> +#define DMA_BUF_ACCESS_DMA_W (DMA_BUF_ACCESS_W | DMA_BUF_ACCESS_DMA)
> +#define DMA_BUF_ACCESS_DMA_RW (DMA_BUF_ACCESS_DMA_R | DMA_BUF_ACCESS_DMA_W)
> +#define IS_VALID_DMA_BUF_ACCESS_TYPE(t) (t = DMA_BUF_ACCESS_R || \
> + t = DMA_BUF_ACCESS_W || \
> + t = DMA_BUF_ACCESS_DMA_R || \
> + t = DMA_BUF_ACCESS_DMA_W || \
> + t = DMA_BUF_ACCESS_RW || \
> + t = DMA_BUF_ACCESS_DMA_RW)
> +
> /**
> * get_dma_buf - convenience wrapper for get_file.
> * @dmabuf: [in] pointer to dma_buf
> diff --git a/include/linux/dmabuf-sync.h b/include/linux/dmabuf-sync.h
> new file mode 100644
> index 0000000..9a3afc4
> --- /dev/null
> +++ b/include/linux/dmabuf-sync.h
> @@ -0,0 +1,190 @@
> +/*
> + * Copyright (C) 2013 Samsung Electronics Co.Ltd
> + * Authors:
> + * Inki Dae <inki.dae@samsung.com>
> + *
> + * 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.
> + *
> + */
> +
> +#include <linux/mutex.h>
> +#include <linux/sched.h>
> +#include <linux/dma-buf.h>
> +
> +enum dmabuf_sync_status {
> + DMABUF_SYNC_GOT = 1,
> + DMABUF_SYNC_LOCKED,
> +};
> +
No comment about this structure?
> +struct dmabuf_sync_reservation {
> + struct ww_mutex sync_lock;
> + struct mutex lock;
> + wait_queue_head_t poll_wait;
> + unsigned int poll_event;
> + unsigned int polled;
> + atomic_t shared_cnt;
> + unsigned int accessed_type;
> + unsigned int locked;
> +};
> +
> +/*
> + * A structure for dmabuf_sync_object.
> + *
> + * @head: A list head to be added to syncs list.
> + * @robj: A reservation_object object.
> + * @dma_buf: A dma_buf object.
> + * @access_type: Indicate how a current task tries to access
> + * a given buffer.
Huh? What values are expected then? Is there some #define or enum
for that?
> + */
> +struct dmabuf_sync_object {
> + struct list_head head;
> + struct dmabuf_sync_reservation *robj;
> + struct dma_buf *dmabuf;
> + unsigned int access_type;
> +};
> +
> +struct dmabuf_sync_priv_ops {
> + void (*free)(void *priv);
> +};
> +
> +/*
> + * A structure for dmabuf_sync.
> + *
> + * @syncs: A list head to sync object and this is global to system.
> + * @list: A list entry used as committed list node
> + * @lock: A mutex lock to current sync object.
You should say for which specific operations this mutex is needed.
For everything? Or just for list operations.
> + * @ctx: A current context for ww mutex.
> + * @work: A work struct to release resources at timeout.
> + * @priv: A private data.
> + * @name: A string to dmabuf sync owner.
> + * @timer: A timer list to avoid lockup and release resources.
> + * @status: Indicate current status (DMABUF_SYNC_GOT or DMABUF_SYNC_LOCKED).
> + */
> +struct dmabuf_sync {
> + struct list_head syncs;
> + struct list_head list;
> + struct mutex lock;
> + struct ww_acquire_ctx ctx;
> + struct work_struct work;
> + void *priv;
> + struct dmabuf_sync_priv_ops *ops;
> + char name[64];
Perhaps a #define for the size?
> + struct timer_list timer;
> + unsigned int status;
> +};
> +
> +#ifdef CONFIG_DMABUF_SYNC
> +
> +extern struct ww_class dmabuf_sync_ww_class;
> +
> +static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf)
> +{
> + struct dmabuf_sync_reservation *obj;
> +
> + obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> + if (!obj)
> + return;
> +
> + dmabuf->sync = obj;
> +
> + ww_mutex_init(&obj->sync_lock, &dmabuf_sync_ww_class);
> +
> + mutex_init(&obj->lock);
> + atomic_set(&obj->shared_cnt, 1);
> +
> + init_waitqueue_head(&obj->poll_wait);
> +}
> +
> +static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf)
> +{
> + struct dmabuf_sync_reservation *obj;
> +
> + if (!dmabuf->sync)
> + return;
> +
> + obj = dmabuf->sync;
> +
> + ww_mutex_destroy(&obj->sync_lock);
> +
> + kfree(obj);
> +}
> +
> +extern bool is_dmabuf_sync_supported(void);
> +
> +extern struct dmabuf_sync *dmabuf_sync_init(const char *name,
> + struct dmabuf_sync_priv_ops *ops,
> + void *priv);
> +
> +extern void dmabuf_sync_fini(struct dmabuf_sync *sync);
> +
> +extern int dmabuf_sync_lock(struct dmabuf_sync *sync);
> +
> +extern int dmabuf_sync_unlock(struct dmabuf_sync *sync);
> +
> +int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
> + bool wait);
> +
> +void dmabuf_sync_single_unlock(struct dma_buf *dmabuf);
> +
> +extern int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
> + unsigned int type);
> +
> +extern void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf);
> +
> +extern void dmabuf_sync_put_all(struct dmabuf_sync *sync);
> +
> +#else
> +
> +static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf) { }
> +
> +static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf) { }
> +
> +static inline bool is_dmabuf_sync_supported(void) { return false; }
> +
> +static inline struct dmabuf_sync *dmabuf_sync_init(const char *name,
> + struct dmabuf_sync_priv_ops *ops,
> + void *priv)
> +{
> + return ERR_PTR(0);
> +}
> +
> +static inline void dmabuf_sync_fini(struct dmabuf_sync *sync) { }
> +
> +static inline int dmabuf_sync_lock(struct dmabuf_sync *sync)
> +{
> + return 0;
> +}
> +
> +static inline int dmabuf_sync_unlock(struct dmabuf_sync *sync)
> +{
> + return 0;
> +}
> +
> +static inline int dmabuf_sync_single_lock(struct dma_buf *dmabuf,
> + unsigned int type,
> + bool wait)
> +{
> + return 0;
> +}
> +
> +static inline void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
> +{
> + return;
> +}
> +
> +static inline int dmabuf_sync_get(struct dmabuf_sync *sync,
> + void *sync_buf,
> + unsigned int type)
> +{
> + return 0;
> +}
> +
> +static inline void dmabuf_sync_put(struct dmabuf_sync *sync,
> + struct dma_buf *dmabuf) { }
> +
> +static inline void dmabuf_sync_put_all(struct dmabuf_sync *sync) { }
> +
> +#endif
> --
> 1.7.5.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v11 0/8] PHY framework
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
Added a generic PHY framework that provides a set of APIs for the PHY drivers
to create/destroy a PHY and APIs for the PHY users to obtain a reference to
the PHY with or without using phandle.
This framework will be of use only to devices that uses external PHY (PHY
functionality is not embedded within the controller).
The intention of creating this framework is to bring the phy drivers spread
all over the Linux kernel to drivers/phy to increase code re-use and to
increase code maintainability.
Comments to make PHY as bus wasn't done because PHY devices can be part of
other bus and making a same device attached to multiple bus leads to bad
design.
If the PHY driver has to send notification on connect/disconnect, the PHY
driver should make use of the extcon framework. Using this susbsystem
to use extcon framwork will have to be analysed.
You can find this patch series @
git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git testing
I'll create a new branch *next* once this patch series is finalized. All the
PHY driver development that depends on PHY framework can be based on this
branch.
Did USB enumeration testing in panda and beagle after applying [1]
[1] -> https://lkml.org/lkml/2013/7/26/88
Changes from v10:
* fixed a mistake in devm_of_phy_provider_register macro which was carried over from earlier version.
* used ida_simple_get for obtaining the id.
Changes from v9:
* Fixed Greg's concern on having *find PHY by string* and changed it to Tomasz
pseudo code.
* move omap-usb2 phy and twl4030-usb phy to drivers/phy
* made all the dependent drivers select GENERIC_PHY instead of having depends
on
* Made PHY core to assign the id's (so changed the phy_create API).
* Adapted twl4030-usb to the new design.
Changes from v8:
* Added phy_set_drvdata and phy_get_drvdata in phy.h.
* Changed phy_create API not to take void *priv. private data should now be set
using phy_set_drvdata now.
Changes from v7:
* Fixed Documentation
* Added to_phy, of_phy_provider_register and devm_of_phy_provider_register
* modified runtime_pm usage in phy_init, phy_exit, phy_power_on and
phy_power_off. Now phy_power_on will enable the clocks and phy_power_off will
disable the clocks.
* pm_runtime_no_callbacks() is added so that pm_runtime_get_sync doesn't fail
* modified other patches to adhere to the changes in the PHY framework
* removed usb: phy: twl4030: twl4030 shouldn't be subsys_initcall as it will
be merged separately.
* reference counting has been added to protect phy ops when the PHY is shared
by multiple consumers.
Changes from v6
* corrected few typos in Documentation
* Changed PHY Subsystem to *bool* in Kconfig (to avoid compilation errors when
PHY Subsystem is kept as module and the dependent modules are built-in)
* Added if pm_runtime_enabled check before runtime pm calls.
Changes from v5:
* removed the new sysfs entries as it dint have any new information other than
what is already there in /sys/devices/...
* removed a bunch of APIs added to get the PHY and now only phy_get and
devm_phy_get are used.
* Added new APIs to register/unregister the PHY provider. This is needed for
dt boot case.
* Enabled pm runtime and incorporated the comments given by Alan Stern in a
different patch series by Gautam.
* Removed the *phy_bind* API. Now the phy binding information should be passed
using the platform data to the controller devices.
* Fixed a few typos.
Changes from v4:
* removed of_phy_get_with_args/devm_of_phy_get_with_args. Now the *phy providers*
should use their custom implementation of of_xlate or use of_phy_xlate to get
*phy instance* from *phy providers*.
* Added of_phy_xlate to be used by *phy providers* if it provides only one PHY.
* changed phy_core from having subsys_initcall to module_init.
* other minor fixes.
Changes from v3:
* Changed the return value of PHY APIs to ENOSYS
* Added APIs of_phy_get_with_args/devm_of_phy_get_with_args to support getting
PHYs if the same IP implements multiple PHYs.
* modified phy_bind API so that the binding information can now be _updated_.
In effect of this removed the binding information added in board files and
added only in usb-musb.c. If a particular board uses a different phy binding,
it can update it in board file after usb_musb_init().
* Added Documentation/devicetree/bindings/phy/phy-bindings.txt for dt binding
information.
Changes from v2:
* removed phy_descriptor structure completely so changed the APIs which were
taking phy_descriptor as parameters
* Added 2 more APIs *of_phy_get_byname* and *devm_of_phy_get_byname* to be used
by PHY user drivers which has *phy* and *phy-names* binding in the dt data
* Fixed a few typos
* Removed phy_list and we now use class_dev_iter_init, class_dev_iter_next and
class_dev_iter_exit for traversing through the phy list. (Note we still need
phy_bind list and phy_bind_mutex).
* Changed the sysfs entry name from *bind* to *phy_bind*.
Changes from v1:
* Added Documentation for the PHY framework
* Added few more APIs mostly w.r.t devres
* Modified omap-usb2 and twl4030 to make use of the new framework
Kishon Vijay Abraham I (8):
drivers: phy: add generic PHY framework
usb: phy: omap-usb2: use the new generic PHY framework
usb: phy: twl4030: use the new generic PHY framework
arm: omap3: twl: add phy consumer data in twl4030_usb_data
ARM: dts: omap: update usb_otg_hs data
usb: musb: omap2430: use the new generic PHY framework
usb: phy: omap-usb2: remove *set_suspend* callback from omap-usb2
usb: phy: twl4030-usb: remove *set_suspend* and *phy_init* ops
.../devicetree/bindings/phy/phy-bindings.txt | 66 ++
Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +
Documentation/devicetree/bindings/usb/usb-phy.txt | 6 +
Documentation/phy.txt | 166 +++++
MAINTAINERS | 8 +
arch/arm/boot/dts/omap3-beagle-xm.dts | 2 +
arch/arm/boot/dts/omap3-evm.dts | 2 +
arch/arm/boot/dts/omap3-overo.dtsi | 2 +
arch/arm/boot/dts/omap4.dtsi | 3 +
arch/arm/boot/dts/twl4030.dtsi | 1 +
arch/arm/mach-omap2/twl-common.c | 11 +
drivers/Kconfig | 2 +
drivers/Makefile | 2 +
drivers/phy/Kconfig | 41 ++
drivers/phy/Makefile | 7 +
drivers/phy/phy-core.c | 698 ++++++++++++++++++++
drivers/{usb => }/phy/phy-omap-usb2.c | 60 +-
drivers/{usb => }/phy/phy-twl4030-usb.c | 69 +-
drivers/usb/musb/Kconfig | 1 +
drivers/usb/musb/musb_core.h | 2 +
drivers/usb/musb/omap2430.c | 26 +-
drivers/usb/phy/Kconfig | 19 -
drivers/usb/phy/Makefile | 2 -
include/linux/i2c/twl.h | 2 +
include/linux/phy/phy.h | 270 ++++++++
25 files changed, 1397 insertions(+), 76 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
create mode 100644 Documentation/phy.txt
create mode 100644 drivers/phy/Kconfig
create mode 100644 drivers/phy/Makefile
create mode 100644 drivers/phy/phy-core.c
rename drivers/{usb => }/phy/phy-omap-usb2.c (88%)
rename drivers/{usb => }/phy/phy-twl4030-usb.c (94%)
create mode 100644 include/linux/phy/phy.h
--
1.7.10.4
^ permalink raw reply
* [PATCH v11 1/8] drivers: phy: add generic PHY framework
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
The PHY framework provides a set of APIs for the PHY drivers to
create/destroy a PHY and APIs for the PHY users to obtain a reference to the
PHY with or without using phandle. For dt-boot, the PHY drivers should
also register *PHY provider* with the framework.
PHY drivers should create the PHY by passing id and ops like init, exit,
power_on and power_off. This framework is also pm runtime enabled.
The documentation for the generic PHY framework is added in
Documentation/phy.txt and the documentation for dt binding can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt
Cc: Tomasz Figa <t.figa@samsung.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Tested-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
.../devicetree/bindings/phy/phy-bindings.txt | 66 ++
Documentation/phy.txt | 166 +++++
MAINTAINERS | 8 +
drivers/Kconfig | 2 +
drivers/Makefile | 2 +
drivers/phy/Kconfig | 18 +
drivers/phy/Makefile | 5 +
drivers/phy/phy-core.c | 698 ++++++++++++++++++++
include/linux/phy/phy.h | 270 ++++++++
9 files changed, 1235 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/phy-bindings.txt
create mode 100644 Documentation/phy.txt
create mode 100644 drivers/phy/Kconfig
create mode 100644 drivers/phy/Makefile
create mode 100644 drivers/phy/phy-core.c
create mode 100644 include/linux/phy/phy.h
diff --git a/Documentation/devicetree/bindings/phy/phy-bindings.txt b/Documentation/devicetree/bindings/phy/phy-bindings.txt
new file mode 100644
index 0000000..8ae844f
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-bindings.txt
@@ -0,0 +1,66 @@
+This document explains only the device tree data binding. For general
+information about PHY subsystem refer to Documentation/phy.txt
+
+PHY device node
+=======+
+Required Properties:
+#phy-cells: Number of cells in a PHY specifier; The meaning of all those
+ cells is defined by the binding for the phy node. The PHY
+ provider can use the values in cells to find the appropriate
+ PHY.
+
+For example:
+
+phys: phy {
+ compatible = "xxx";
+ reg = <...>;
+ .
+ .
+ #phy-cells = <1>;
+ .
+ .
+};
+
+That node describes an IP block (PHY provider) that implements 2 different PHYs.
+In order to differentiate between these 2 PHYs, an additonal specifier should be
+given while trying to get a reference to it.
+
+PHY user node
+======+
+Required Properties:
+phys : the phandle for the PHY device (used by the PHY subsystem)
+phy-names : the names of the PHY corresponding to the PHYs present in the
+ *phys* phandle
+
+Example 1:
+usb1: usb_otg_ss@xxx {
+ compatible = "xxx";
+ reg = <xxx>;
+ .
+ .
+ phys = <&usb2_phy>, <&usb3_phy>;
+ phy-names = "usb2phy", "usb3phy";
+ .
+ .
+};
+
+This node represents a controller that uses two PHYs, one for usb2 and one for
+usb3.
+
+Example 2:
+usb2: usb_otg_ss@xxx {
+ compatible = "xxx";
+ reg = <xxx>;
+ .
+ .
+ phys = <&phys 1>;
+ phy-names = "usbphy";
+ .
+ .
+};
+
+This node represents a controller that uses one of the PHYs of the PHY provider
+device defined previously. Note that the phy handle has an additional specifier
+"1" to differentiate between the two PHYs.
diff --git a/Documentation/phy.txt b/Documentation/phy.txt
new file mode 100644
index 0000000..0103e4b
--- /dev/null
+++ b/Documentation/phy.txt
@@ -0,0 +1,166 @@
+ PHY SUBSYSTEM
+ Kishon Vijay Abraham I <kishon@ti.com>
+
+This document explains the Generic PHY Framework along with the APIs provided,
+and how-to-use.
+
+1. Introduction
+
+*PHY* is the abbreviation for physical layer. It is used to connect a device
+to the physical medium e.g., the USB controller has a PHY to provide functions
+such as serialization, de-serialization, encoding, decoding and is responsible
+for obtaining the required data transmission rate. Note that some USB
+controllers have PHY functionality embedded into it and others use an external
+PHY. Other peripherals that use PHY include Wireless LAN, Ethernet,
+SATA etc.
+
+The intention of creating this framework is to bring the PHY drivers spread
+all over the Linux kernel to drivers/phy to increase code re-use and for
+better code maintainability.
+
+This framework will be of use only to devices that use external PHY (PHY
+functionality is not embedded within the controller).
+
+2. Registering/Unregistering the PHY provider
+
+PHY provider refers to an entity that implements one or more PHY instances.
+For the simple case where the PHY provider implements only a single instance of
+the PHY, the framework provides its own implementation of of_xlate in
+of_phy_simple_xlate. If the PHY provider implements multiple instances, it
+should provide its own implementation of of_xlate. of_xlate is used only for
+dt boot case.
+
+#define of_phy_provider_register(dev, xlate) \
+ __of_phy_provider_register((dev), THIS_MODULE, (xlate))
+
+#define devm_of_phy_provider_register(dev, xlate) \
+ __devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
+
+of_phy_provider_register and devm_of_phy_provider_register macros can be used to
+register the phy_provider and it takes device and of_xlate as
+arguments. For the dt boot case, all PHY providers should use one of the above
+2 macros to register the PHY provider.
+
+void devm_of_phy_provider_unregister(struct device *dev,
+ struct phy_provider *phy_provider);
+void of_phy_provider_unregister(struct phy_provider *phy_provider);
+
+devm_of_phy_provider_unregister and of_phy_provider_unregister can be used to
+unregister the PHY.
+
+3. Creating the PHY
+
+The PHY driver should create the PHY in order for other peripheral controllers
+to make use of it. The PHY framework provides 2 APIs to create the PHY.
+
+struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
+ struct phy_init_data *init_data);
+struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops,
+ struct phy_init_data *init_data);
+
+The PHY drivers can use one of the above 2 APIs to create the PHY by passing
+the device pointer, phy ops and init_data.
+phy_ops is a set of function pointers for performing PHY operations such as
+init, exit, power_on and power_off. *init_data* is mandatory to get a reference
+to the PHY in the case of non-dt boot. See section *Board File Initialization*
+on how init_data should be used.
+
+Inorder to dereference the private data (in phy_ops), the phy provider driver
+can use phy_set_drvdata() after creating the PHY and use phy_get_drvdata() in
+phy_ops to get back the private data.
+
+4. Getting a reference to the PHY
+
+Before the controller can make use of the PHY, it has to get a reference to
+it. This framework provides the following APIs to get a reference to the PHY.
+
+struct phy *phy_get(struct device *dev, const char *string);
+struct phy *devm_phy_get(struct device *dev, const char *string);
+
+phy_get and devm_phy_get can be used to get the PHY. In the case of dt boot,
+the string arguments should contain the phy name as given in the dt data and
+in the case of non-dt boot, it should contain the label of the PHY.
+The only difference between the two APIs is that devm_phy_get associates the
+device with the PHY using devres on successful PHY get. On driver detach,
+release function is invoked on the the devres data and devres data is freed.
+
+5. Releasing a reference to the PHY
+
+When the controller no longer needs the PHY, it has to release the reference
+to the PHY it has obtained using the APIs mentioned in the above section. The
+PHY framework provides 2 APIs to release a reference to the PHY.
+
+void phy_put(struct phy *phy);
+void devm_phy_put(struct device *dev, struct phy *phy);
+
+Both these APIs are used to release a reference to the PHY and devm_phy_put
+destroys the devres associated with this PHY.
+
+6. Destroying the PHY
+
+When the driver that created the PHY is unloaded, it should destroy the PHY it
+created using one of the following 2 APIs.
+
+void phy_destroy(struct phy *phy);
+void devm_phy_destroy(struct device *dev, struct phy *phy);
+
+Both these APIs destroy the PHY and devm_phy_destroy destroys the devres
+associated with this PHY.
+
+7. PM Runtime
+
+This subsystem is pm runtime enabled. So while creating the PHY,
+pm_runtime_enable of the phy device created by this subsystem is called and
+while destroying the PHY, pm_runtime_disable is called. Note that the phy
+device created by this subsystem will be a child of the device that calls
+phy_create (PHY provider device).
+
+So pm_runtime_get_sync of the phy_device created by this subsystem will invoke
+pm_runtime_get_sync of PHY provider device because of parent-child relationship.
+It should also be noted that phy_power_on and phy_power_off performs
+phy_pm_runtime_get_sync and phy_pm_runtime_put respectively.
+There are exported APIs like phy_pm_runtime_get, phy_pm_runtime_get_sync,
+phy_pm_runtime_put, phy_pm_runtime_put_sync, phy_pm_runtime_allow and
+phy_pm_runtime_forbid for performing PM operations.
+
+8. Board File Initialization
+
+Certain board file initialization is necessary in order to get a reference
+to the PHY in the case of non-dt boot.
+Say we have a single device that implements 3 PHYs that of USB, SATA and PCIe,
+then in the board file the following initialization should be done.
+
+struct phy_consumer consumers[] = {
+ PHY_CONSUMER("dwc3.0", "usb"),
+ PHY_CONSUMER("pcie.0", "pcie"),
+ PHY_CONSUMER("sata.0", "sata"),
+};
+PHY_CONSUMER takes 2 parameters, first is the device name of the controller
+(PHY consumer) and second is the port name.
+
+struct phy_init_data init_data = {
+ .consumers = consumers,
+ .num_consumers = ARRAY_SIZE(consumers),
+};
+
+static const struct platform_device pipe3_phy_dev = {
+ .name = "pipe3-phy",
+ .id = -1,
+ .dev = {
+ .platform_data = {
+ .init_data = &init_data,
+ },
+ },
+};
+
+then, while doing phy_create, the PHY driver should pass this init_data
+ phy_create(dev, ops, pdata->init_data);
+
+and the controller driver (phy consumer) should pass the port name along with
+the device to get a reference to the PHY
+ phy_get(dev, "pcie");
+
+9. DeviceTree Binding
+
+The documentation for PHY dt binding can be found @
+Documentation/devicetree/bindings/phy/phy-bindings.txt
diff --git a/MAINTAINERS b/MAINTAINERS
index 229c66b..5438e23 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3599,6 +3599,14 @@ S: Maintained
F: include/asm-generic
F: include/uapi/asm-generic
+GENERIC PHY FRAMEWORK
+M: Kishon Vijay Abraham I <kishon@ti.com>
+L: linux-kernel@vger.kernel.org
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git
+S: Supported
+F: drivers/phy/
+F: include/linux/phy/
+
GENERIC UIO DRIVER FOR PCI DEVICES
M: "Michael S. Tsirkin" <mst@redhat.com>
L: kvm@vger.kernel.org
diff --git a/drivers/Kconfig b/drivers/Kconfig
index aa43b91..8f45144 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -166,4 +166,6 @@ source "drivers/reset/Kconfig"
source "drivers/fmc/Kconfig"
+source "drivers/phy/Kconfig"
+
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index ab93de8..687da89 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -8,6 +8,8 @@
obj-y += irqchip/
obj-y += bus/
+obj-$(CONFIG_GENERIC_PHY) += phy/
+
# GPIO must come after pinctrl as gpios may need to mux pins etc
obj-y += pinctrl/
obj-y += gpio/
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
new file mode 100644
index 0000000..349bef2
--- /dev/null
+++ b/drivers/phy/Kconfig
@@ -0,0 +1,18 @@
+#
+# PHY
+#
+
+menu "PHY Subsystem"
+
+config GENERIC_PHY
+ tristate "PHY Core"
+ help
+ Generic PHY support.
+
+ This framework is designed to provide a generic interface for PHY
+ devices present in the kernel. This layer will have the generic
+ API by which phy drivers can create PHY using the phy framework and
+ phy users can obtain reference to the PHY. All the users of this
+ framework should select this config.
+
+endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
new file mode 100644
index 0000000..9e9560f
--- /dev/null
+++ b/drivers/phy/Makefile
@@ -0,0 +1,5 @@
+#
+# Makefile for the phy drivers.
+#
+
+obj-$(CONFIG_GENERIC_PHY) += phy-core.o
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
new file mode 100644
index 0000000..03cf8fb
--- /dev/null
+++ b/drivers/phy/phy-core.c
@@ -0,0 +1,698 @@
+/*
+ * phy-core.c -- Generic Phy framework.
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Author: Kishon Vijay Abraham I <kishon@ti.com>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/export.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/of.h>
+#include <linux/phy/phy.h>
+#include <linux/idr.h>
+#include <linux/pm_runtime.h>
+
+static struct class *phy_class;
+static DEFINE_MUTEX(phy_provider_mutex);
+static LIST_HEAD(phy_provider_list);
+static DEFINE_IDA(phy_ida);
+
+static void devm_phy_release(struct device *dev, void *res)
+{
+ struct phy *phy = *(struct phy **)res;
+
+ phy_put(phy);
+}
+
+static void devm_phy_provider_release(struct device *dev, void *res)
+{
+ struct phy_provider *phy_provider = *(struct phy_provider **)res;
+
+ of_phy_provider_unregister(phy_provider);
+}
+
+static void devm_phy_consume(struct device *dev, void *res)
+{
+ struct phy *phy = *(struct phy **)res;
+
+ phy_destroy(phy);
+}
+
+static int devm_phy_match(struct device *dev, void *res, void *match_data)
+{
+ return res = match_data;
+}
+
+static struct phy *phy_lookup(struct device *device, const char *port)
+{
+ unsigned int count;
+ struct phy *phy;
+ struct device *dev;
+ struct phy_consumer *consumers;
+ struct class_dev_iter iter;
+
+ class_dev_iter_init(&iter, phy_class, NULL, NULL);
+ while ((dev = class_dev_iter_next(&iter))) {
+ phy = to_phy(dev);
+ count = phy->init_data->num_consumers;
+ consumers = phy->init_data->consumers;
+ while (count--) {
+ if (!strcmp(consumers->dev_name, dev_name(device)) &&
+ !strcmp(consumers->port, port)) {
+ class_dev_iter_exit(&iter);
+ return phy;
+ }
+ consumers++;
+ }
+ }
+
+ class_dev_iter_exit(&iter);
+ return ERR_PTR(-ENODEV);
+}
+
+static struct phy_provider *of_phy_provider_lookup(struct device_node *node)
+{
+ struct phy_provider *phy_provider;
+
+ list_for_each_entry(phy_provider, &phy_provider_list, list) {
+ if (phy_provider->dev->of_node = node)
+ return phy_provider;
+ }
+
+ return ERR_PTR(-EPROBE_DEFER);
+}
+
+int phy_pm_runtime_get(struct phy *phy)
+{
+ if (!pm_runtime_enabled(&phy->dev))
+ return -ENOTSUPP;
+
+ return pm_runtime_get(&phy->dev);
+}
+EXPORT_SYMBOL_GPL(phy_pm_runtime_get);
+
+int phy_pm_runtime_get_sync(struct phy *phy)
+{
+ if (!pm_runtime_enabled(&phy->dev))
+ return -ENOTSUPP;
+
+ return pm_runtime_get_sync(&phy->dev);
+}
+EXPORT_SYMBOL_GPL(phy_pm_runtime_get_sync);
+
+int phy_pm_runtime_put(struct phy *phy)
+{
+ if (!pm_runtime_enabled(&phy->dev))
+ return -ENOTSUPP;
+
+ return pm_runtime_put(&phy->dev);
+}
+EXPORT_SYMBOL_GPL(phy_pm_runtime_put);
+
+int phy_pm_runtime_put_sync(struct phy *phy)
+{
+ if (!pm_runtime_enabled(&phy->dev))
+ return -ENOTSUPP;
+
+ return pm_runtime_put_sync(&phy->dev);
+}
+EXPORT_SYMBOL_GPL(phy_pm_runtime_put_sync);
+
+void phy_pm_runtime_allow(struct phy *phy)
+{
+ if (!pm_runtime_enabled(&phy->dev))
+ return;
+
+ pm_runtime_allow(&phy->dev);
+}
+EXPORT_SYMBOL_GPL(phy_pm_runtime_allow);
+
+void phy_pm_runtime_forbid(struct phy *phy)
+{
+ if (!pm_runtime_enabled(&phy->dev))
+ return;
+
+ pm_runtime_forbid(&phy->dev);
+}
+EXPORT_SYMBOL_GPL(phy_pm_runtime_forbid);
+
+int phy_init(struct phy *phy)
+{
+ int ret;
+
+ ret = phy_pm_runtime_get_sync(phy);
+ if (ret < 0 && ret != -ENOTSUPP)
+ return ret;
+
+ mutex_lock(&phy->mutex);
+ if (phy->init_count++ = 0 && phy->ops->init) {
+ ret = phy->ops->init(phy);
+ if (ret < 0) {
+ dev_err(&phy->dev, "phy init failed --> %d\n", ret);
+ goto out;
+ }
+ }
+
+out:
+ mutex_unlock(&phy->mutex);
+ phy_pm_runtime_put(phy);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_init);
+
+int phy_exit(struct phy *phy)
+{
+ int ret;
+
+ ret = phy_pm_runtime_get_sync(phy);
+ if (ret < 0 && ret != -ENOTSUPP)
+ return ret;
+
+ mutex_lock(&phy->mutex);
+ if (--phy->init_count = 0 && phy->ops->exit) {
+ ret = phy->ops->exit(phy);
+ if (ret < 0) {
+ dev_err(&phy->dev, "phy exit failed --> %d\n", ret);
+ goto out;
+ }
+ }
+
+out:
+ mutex_unlock(&phy->mutex);
+ phy_pm_runtime_put(phy);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_exit);
+
+int phy_power_on(struct phy *phy)
+{
+ int ret = -ENOTSUPP;
+
+ ret = phy_pm_runtime_get_sync(phy);
+ if (ret < 0 && ret != -ENOTSUPP)
+ return ret;
+
+ mutex_lock(&phy->mutex);
+ if (phy->power_count++ = 0 && phy->ops->power_on) {
+ ret = phy->ops->power_on(phy);
+ if (ret < 0) {
+ dev_err(&phy->dev, "phy poweron failed --> %d\n", ret);
+ goto out;
+ }
+ }
+
+out:
+ mutex_unlock(&phy->mutex);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_power_on);
+
+int phy_power_off(struct phy *phy)
+{
+ int ret = -ENOTSUPP;
+
+ mutex_lock(&phy->mutex);
+ if (--phy->power_count = 0 && phy->ops->power_off) {
+ ret = phy->ops->power_off(phy);
+ if (ret < 0) {
+ dev_err(&phy->dev, "phy poweroff failed --> %d\n", ret);
+ goto out;
+ }
+ }
+
+out:
+ mutex_unlock(&phy->mutex);
+ phy_pm_runtime_put(phy);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(phy_power_off);
+
+/**
+ * of_phy_get() - lookup and obtain a reference to a phy by phandle
+ * @dev: device that requests this phy
+ * @index: the index of the phy
+ *
+ * Returns the phy associated with the given phandle value,
+ * after getting a refcount to it or -ENODEV if there is no such phy or
+ * -EPROBE_DEFER if there is a phandle to the phy, but the device is
+ * not yet loaded. This function uses of_xlate call back function provided
+ * while registering the phy_provider to find the phy instance.
+ */
+static struct phy *of_phy_get(struct device *dev, int index)
+{
+ int ret;
+ struct phy_provider *phy_provider;
+ struct phy *phy = NULL;
+ struct of_phandle_args args;
+
+ ret = of_parse_phandle_with_args(dev->of_node, "phys", "#phy-cells",
+ index, &args);
+ if (ret) {
+ dev_dbg(dev, "failed to get phy in %s node\n",
+ dev->of_node->full_name);
+ return ERR_PTR(-ENODEV);
+ }
+
+ mutex_lock(&phy_provider_mutex);
+ phy_provider = of_phy_provider_lookup(args.np);
+ if (IS_ERR(phy_provider) || !try_module_get(phy_provider->owner)) {
+ phy = ERR_PTR(-EPROBE_DEFER);
+ goto err0;
+ }
+
+ phy = phy_provider->of_xlate(phy_provider->dev, &args);
+ module_put(phy_provider->owner);
+
+err0:
+ mutex_unlock(&phy_provider_mutex);
+ of_node_put(args.np);
+
+ return phy;
+}
+
+/**
+ * phy_put() - release the PHY
+ * @phy: the phy returned by phy_get()
+ *
+ * Releases a refcount the caller received from phy_get().
+ */
+void phy_put(struct phy *phy)
+{
+ if (IS_ERR(phy))
+ return;
+
+ module_put(phy->ops->owner);
+ put_device(&phy->dev);
+}
+EXPORT_SYMBOL_GPL(phy_put);
+
+/**
+ * devm_phy_put() - release the PHY
+ * @dev: device that wants to release this phy
+ * @phy: the phy returned by devm_phy_get()
+ *
+ * destroys the devres associated with this phy and invokes phy_put
+ * to release the phy.
+ */
+void devm_phy_put(struct device *dev, struct phy *phy)
+{
+ int r;
+
+ r = devres_destroy(dev, devm_phy_release, devm_phy_match, phy);
+ dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
+}
+EXPORT_SYMBOL_GPL(devm_phy_put);
+
+/**
+ * of_phy_simple_xlate() - returns the phy instance from phy provider
+ * @dev: the PHY provider device
+ * @args: of_phandle_args (not used here)
+ *
+ * Intended to be used by phy provider for the common case where #phy-cells is
+ * 0. For other cases where #phy-cells is greater than '0', the phy provider
+ * should provide a custom of_xlate function that reads the *args* and returns
+ * the appropriate phy.
+ */
+struct phy *of_phy_simple_xlate(struct device *dev, struct of_phandle_args
+ *args)
+{
+ struct phy *phy;
+ struct class_dev_iter iter;
+ struct device_node *node = dev->of_node;
+
+ class_dev_iter_init(&iter, phy_class, NULL, NULL);
+ while ((dev = class_dev_iter_next(&iter))) {
+ phy = to_phy(dev);
+ if (node != phy->dev.of_node)
+ continue;
+
+ class_dev_iter_exit(&iter);
+ return phy;
+ }
+
+ class_dev_iter_exit(&iter);
+ return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
+
+/**
+ * phy_get() - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @string: the phy name as given in the dt data or the name of the controller
+ * port for non-dt case
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy. The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *phy_get(struct device *dev, const char *string)
+{
+ int index = 0;
+ struct phy *phy = NULL;
+
+ if (string = NULL) {
+ dev_WARN(dev, "missing string\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (dev->of_node) {
+ index = of_property_match_string(dev->of_node, "phy-names",
+ string);
+ phy = of_phy_get(dev, index);
+ if (IS_ERR(phy)) {
+ dev_err(dev, "unable to find phy\n");
+ return phy;
+ }
+ } else {
+ phy = phy_lookup(dev, string);
+ if (IS_ERR(phy)) {
+ dev_err(dev, "unable to find phy\n");
+ return phy;
+ }
+ }
+
+ if (!try_module_get(phy->ops->owner))
+ return ERR_PTR(-EPROBE_DEFER);
+
+ get_device(&phy->dev);
+
+ return phy;
+}
+EXPORT_SYMBOL_GPL(phy_get);
+
+/**
+ * devm_phy_get() - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @string: the phy name as given in the dt data or phy device name
+ * for non-dt case
+ *
+ * Gets the phy using phy_get(), and associates a device with it using
+ * devres. On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.
+ */
+struct phy *devm_phy_get(struct device *dev, const char *string)
+{
+ struct phy **ptr, *phy;
+
+ ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ phy = phy_get(dev, string);
+ if (!IS_ERR(phy)) {
+ *ptr = phy;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return phy;
+}
+EXPORT_SYMBOL_GPL(devm_phy_get);
+
+/**
+ * phy_create() - create a new phy
+ * @dev: device that is creating the new phy
+ * @ops: function pointers for performing phy operations
+ * @init_data: contains the list of PHY consumers or NULL
+ *
+ * Called to create a phy using phy framework.
+ */
+struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
+ struct phy_init_data *init_data)
+{
+ int ret;
+ int id;
+ struct phy *phy;
+
+ if (!dev) {
+ dev_WARN(dev, "no device provided for PHY\n");
+ ret = -EINVAL;
+ goto err0;
+ }
+
+ phy = kzalloc(sizeof(*phy), GFP_KERNEL);
+ if (!phy) {
+ ret = -ENOMEM;
+ goto err0;
+ }
+
+ id = ida_simple_get(&phy_ida, 0, 0, GFP_KERNEL);
+ if (id < 0) {
+ dev_err(dev, "unable to get id\n");
+ ret = id;
+ goto err0;
+ }
+
+ device_initialize(&phy->dev);
+ mutex_init(&phy->mutex);
+
+ phy->dev.class = phy_class;
+ phy->dev.parent = dev;
+ phy->dev.of_node = dev->of_node;
+ phy->id = id;
+ phy->ops = ops;
+ phy->init_data = init_data;
+
+ ret = dev_set_name(&phy->dev, "phy-%s.%d", dev_name(dev), id);
+ if (ret)
+ goto err1;
+
+ ret = device_add(&phy->dev);
+ if (ret)
+ goto err1;
+
+ if (pm_runtime_enabled(dev)) {
+ pm_runtime_enable(&phy->dev);
+ pm_runtime_no_callbacks(&phy->dev);
+ }
+
+ return phy;
+
+err1:
+ ida_remove(&phy_ida, phy->id);
+ put_device(&phy->dev);
+ kfree(phy);
+
+err0:
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(phy_create);
+
+/**
+ * devm_phy_create() - create a new phy
+ * @dev: device that is creating the new phy
+ * @ops: function pointers for performing phy operations
+ * @init_data: contains the list of PHY consumers or NULL
+ *
+ * Creates a new PHY device adding it to the PHY class.
+ * While at that, it also associates the device with the phy using devres.
+ * On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.
+ */
+struct phy *devm_phy_create(struct device *dev, const struct phy_ops *ops,
+ struct phy_init_data *init_data)
+{
+ struct phy **ptr, *phy;
+
+ ptr = devres_alloc(devm_phy_consume, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ phy = phy_create(dev, ops, init_data);
+ if (!IS_ERR(phy)) {
+ *ptr = phy;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return phy;
+}
+EXPORT_SYMBOL_GPL(devm_phy_create);
+
+/**
+ * phy_destroy() - destroy the phy
+ * @phy: the phy to be destroyed
+ *
+ * Called to destroy the phy.
+ */
+void phy_destroy(struct phy *phy)
+{
+ pm_runtime_disable(&phy->dev);
+ device_unregister(&phy->dev);
+}
+EXPORT_SYMBOL_GPL(phy_destroy);
+
+/**
+ * devm_phy_destroy() - destroy the PHY
+ * @dev: device that wants to release this phy
+ * @phy: the phy returned by devm_phy_get()
+ *
+ * destroys the devres associated with this phy and invokes phy_destroy
+ * to destroy the phy.
+ */
+void devm_phy_destroy(struct device *dev, struct phy *phy)
+{
+ int r;
+
+ r = devres_destroy(dev, devm_phy_consume, devm_phy_match, phy);
+ dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
+}
+EXPORT_SYMBOL_GPL(devm_phy_destroy);
+
+/**
+ * __of_phy_provider_register() - create/register phy provider with the framework
+ * @dev: struct device of the phy provider
+ * @owner: the module owner containing of_xlate
+ * @of_xlate: function pointer to obtain phy instance from phy provider
+ *
+ * Creates struct phy_provider from dev and of_xlate function pointer.
+ * This is used in the case of dt boot for finding the phy instance from
+ * phy provider.
+ */
+struct phy_provider *__of_phy_provider_register(struct device *dev,
+ struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args))
+{
+ struct phy_provider *phy_provider;
+
+ phy_provider = kzalloc(sizeof(*phy_provider), GFP_KERNEL);
+ if (!phy_provider)
+ return ERR_PTR(-ENOMEM);
+
+ phy_provider->dev = dev;
+ phy_provider->owner = owner;
+ phy_provider->of_xlate = of_xlate;
+
+ mutex_lock(&phy_provider_mutex);
+ list_add_tail(&phy_provider->list, &phy_provider_list);
+ mutex_unlock(&phy_provider_mutex);
+
+ return phy_provider;
+}
+EXPORT_SYMBOL_GPL(__of_phy_provider_register);
+
+/**
+ * __devm_of_phy_provider_register() - create/register phy provider with the
+ * framework
+ * @dev: struct device of the phy provider
+ * @owner: the module owner containing of_xlate
+ * @of_xlate: function pointer to obtain phy instance from phy provider
+ *
+ * Creates struct phy_provider from dev and of_xlate function pointer.
+ * This is used in the case of dt boot for finding the phy instance from
+ * phy provider. While at that, it also associates the device with the
+ * phy provider using devres. On driver detach, release function is invoked
+ * on the devres data, then, devres data is freed.
+ */
+struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
+ struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args))
+{
+ struct phy_provider **ptr, *phy_provider;
+
+ ptr = devres_alloc(devm_phy_provider_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ phy_provider = __of_phy_provider_register(dev, owner, of_xlate);
+ if (!IS_ERR(phy_provider)) {
+ *ptr = phy_provider;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return phy_provider;
+}
+EXPORT_SYMBOL_GPL(__devm_of_phy_provider_register);
+
+/**
+ * of_phy_provider_unregister() - unregister phy provider from the framework
+ * @phy_provider: phy provider returned by of_phy_provider_register()
+ *
+ * Removes the phy_provider created using of_phy_provider_register().
+ */
+void of_phy_provider_unregister(struct phy_provider *phy_provider)
+{
+ if (IS_ERR(phy_provider))
+ return;
+
+ mutex_lock(&phy_provider_mutex);
+ list_del(&phy_provider->list);
+ kfree(phy_provider);
+ mutex_unlock(&phy_provider_mutex);
+}
+EXPORT_SYMBOL_GPL(of_phy_provider_unregister);
+
+/**
+ * devm_of_phy_provider_unregister() - remove phy provider from the framework
+ * @dev: struct device of the phy provider
+ *
+ * destroys the devres associated with this phy provider and invokes
+ * of_phy_provider_unregister to unregister the phy provider.
+ */
+void devm_of_phy_provider_unregister(struct device *dev,
+ struct phy_provider *phy_provider) {
+ int r;
+
+ r = devres_destroy(dev, devm_phy_provider_release, devm_phy_match,
+ phy_provider);
+ dev_WARN_ONCE(dev, r, "couldn't find PHY provider device resource\n");
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_provider_unregister);
+
+/**
+ * phy_release() - release the phy
+ * @dev: the dev member within phy
+ *
+ * When the last reference to the device is removed, it is called
+ * from the embedded kobject as release method.
+ */
+static void phy_release(struct device *dev)
+{
+ struct phy *phy;
+
+ phy = to_phy(dev);
+ dev_vdbg(dev, "releasing '%s'\n", dev_name(dev));
+ ida_remove(&phy_ida, phy->id);
+ kfree(phy);
+}
+
+static int __init phy_core_init(void)
+{
+ phy_class = class_create(THIS_MODULE, "phy");
+ if (IS_ERR(phy_class)) {
+ pr_err("failed to create phy class --> %ld\n",
+ PTR_ERR(phy_class));
+ return PTR_ERR(phy_class);
+ }
+
+ phy_class->dev_release = phy_release;
+
+ return 0;
+}
+module_init(phy_core_init);
+
+static void __exit phy_core_exit(void)
+{
+ class_destroy(phy_class);
+}
+module_exit(phy_core_exit);
+
+MODULE_DESCRIPTION("Generic PHY Framework");
+MODULE_AUTHOR("Kishon Vijay Abraham I <kishon@ti.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
new file mode 100644
index 0000000..ca9114d
--- /dev/null
+++ b/include/linux/phy/phy.h
@@ -0,0 +1,270 @@
+/*
+ * phy.h -- generic phy header file
+ *
+ * Copyright (C) 2013 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Author: Kishon Vijay Abraham I <kishon@ti.com>
+ *
+ * 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.
+ */
+
+#ifndef __DRIVERS_PHY_H
+#define __DRIVERS_PHY_H
+
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/device.h>
+#include <linux/pm_runtime.h>
+
+struct phy;
+
+/**
+ * struct phy_ops - set of function pointers for performing phy operations
+ * @init: operation to be performed for initializing phy
+ * @exit: operation to be performed while exiting
+ * @power_on: powering on the phy
+ * @power_off: powering off the phy
+ * @owner: the module owner containing the ops
+ */
+struct phy_ops {
+ int (*init)(struct phy *phy);
+ int (*exit)(struct phy *phy);
+ int (*power_on)(struct phy *phy);
+ int (*power_off)(struct phy *phy);
+ struct module *owner;
+};
+
+/**
+ * struct phy - represents the phy device
+ * @dev: phy device
+ * @id: id of the phy device
+ * @ops: function pointers for performing phy operations
+ * @init_data: list of PHY consumers (non-dt only)
+ * @mutex: mutex to protect phy_ops
+ * @init_count: used to protect when the PHY is used by multiple consumers
+ * @power_count: used to protect when the PHY is used by multiple consumers
+ */
+struct phy {
+ struct device dev;
+ int id;
+ const struct phy_ops *ops;
+ struct phy_init_data *init_data;
+ struct mutex mutex;
+ int init_count;
+ int power_count;
+};
+
+/**
+ * struct phy_provider - represents the phy provider
+ * @dev: phy provider device
+ * @owner: the module owner having of_xlate
+ * @of_xlate: function pointer to obtain phy instance from phy pointer
+ * @list: to maintain a linked list of PHY providers
+ */
+struct phy_provider {
+ struct device *dev;
+ struct module *owner;
+ struct list_head list;
+ struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args);
+};
+
+/**
+ * struct phy_consumer - represents the phy consumer
+ * @dev_name: the device name of the controller that will use this PHY device
+ * @port: name given to the consumer port
+ */
+struct phy_consumer {
+ const char *dev_name;
+ const char *port;
+};
+
+/**
+ * struct phy_init_data - contains the list of PHY consumers
+ * @num_consumers: number of consumers for this PHY device
+ * @consumers: list of PHY consumers
+ */
+struct phy_init_data {
+ unsigned int num_consumers;
+ struct phy_consumer *consumers;
+};
+
+#define PHY_CONSUMER(_dev_name, _port) \
+{ \
+ .dev_name = _dev_name, \
+ .port = _port, \
+}
+
+#define to_phy(dev) (container_of((dev), struct phy, dev))
+
+#define of_phy_provider_register(dev, xlate) \
+ __of_phy_provider_register((dev), THIS_MODULE, (xlate))
+
+#define devm_of_phy_provider_register(dev, xlate) \
+ __devm_of_phy_provider_register((dev), THIS_MODULE, (xlate))
+
+static inline void phy_set_drvdata(struct phy *phy, void *data)
+{
+ dev_set_drvdata(&phy->dev, data);
+}
+
+static inline void *phy_get_drvdata(struct phy *phy)
+{
+ return dev_get_drvdata(&phy->dev);
+}
+
+#if IS_ENABLED(CONFIG_GENERIC_PHY)
+extern int phy_pm_runtime_get(struct phy *phy);
+extern int phy_pm_runtime_get_sync(struct phy *phy);
+extern int phy_pm_runtime_put(struct phy *phy);
+extern int phy_pm_runtime_put_sync(struct phy *phy);
+extern void phy_pm_runtime_allow(struct phy *phy);
+extern void phy_pm_runtime_forbid(struct phy *phy);
+extern int phy_init(struct phy *phy);
+extern int phy_exit(struct phy *phy);
+extern int phy_power_on(struct phy *phy);
+extern int phy_power_off(struct phy *phy);
+extern struct phy *phy_get(struct device *dev, const char *string);
+extern struct phy *devm_phy_get(struct device *dev, const char *string);
+extern void phy_put(struct phy *phy);
+extern void devm_phy_put(struct device *dev, struct phy *phy);
+extern struct phy *of_phy_simple_xlate(struct device *dev,
+ struct of_phandle_args *args);
+extern struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
+ struct phy_init_data *init_data);
+extern struct phy *devm_phy_create(struct device *dev,
+ const struct phy_ops *ops, struct phy_init_data *init_data);
+extern void phy_destroy(struct phy *phy);
+extern void devm_phy_destroy(struct device *dev, struct phy *phy);
+extern struct phy_provider *__of_phy_provider_register(struct device *dev,
+ struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args));
+extern struct phy_provider *__devm_of_phy_provider_register(struct device *dev,
+ struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args));
+extern void of_phy_provider_unregister(struct phy_provider *phy_provider);
+extern void devm_of_phy_provider_unregister(struct device *dev,
+ struct phy_provider *phy_provider);
+#else
+static inline int phy_pm_runtime_get(struct phy *phy)
+{
+ return -ENOSYS;
+}
+
+static inline int phy_pm_runtime_get_sync(struct phy *phy)
+{
+ return -ENOSYS;
+}
+
+static inline int phy_pm_runtime_put(struct phy *phy)
+{
+ return -ENOSYS;
+}
+
+static inline int phy_pm_runtime_put_sync(struct phy *phy)
+{
+ return -ENOSYS;
+}
+
+static inline void phy_pm_runtime_allow(struct phy *phy)
+{
+ return;
+}
+
+static inline void phy_pm_runtime_forbid(struct phy *phy)
+{
+ return;
+}
+
+static inline int phy_init(struct phy *phy)
+{
+ return -ENOSYS;
+}
+
+static inline int phy_exit(struct phy *phy)
+{
+ return -ENOSYS;
+}
+
+static inline int phy_power_on(struct phy *phy)
+{
+ return -ENOSYS;
+}
+
+static inline int phy_power_off(struct phy *phy)
+{
+ return -ENOSYS;
+}
+
+static inline struct phy *phy_get(struct device *dev, const char *string)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline struct phy *devm_phy_get(struct device *dev, const char *string)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline void phy_put(struct phy *phy)
+{
+}
+
+static inline void devm_phy_put(struct device *dev, struct phy *phy)
+{
+}
+
+static inline struct phy *of_phy_simple_xlate(struct device *dev,
+ struct of_phandle_args *args)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline struct phy *phy_create(struct device *dev,
+ const struct phy_ops *ops, struct phy_init_data *init_data)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline struct phy *devm_phy_create(struct device *dev,
+ const struct phy_ops *ops, struct phy_init_data *init_data)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline void phy_destroy(struct phy *phy)
+{
+}
+
+static inline void devm_phy_destroy(struct device *dev, struct phy *phy)
+{
+}
+
+static inline struct phy_provider *__of_phy_provider_register(
+ struct device *dev, struct module *owner, struct phy * (*of_xlate)(
+ struct device *dev, struct of_phandle_args *args))
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline struct phy_provider *__devm_of_phy_provider_register(struct device
+ *dev, struct module *owner, struct phy * (*of_xlate)(struct device *dev,
+ struct of_phandle_args *args))
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline void of_phy_provider_unregister(struct phy_provider *phy_provider)
+{
+}
+
+static inline void devm_of_phy_provider_unregister(struct device *dev,
+ struct phy_provider *phy_provider)
+{
+}
+#endif
+
+#endif /* __DRIVERS_PHY_H */
--
1.7.10.4
^ permalink raw reply related
* [PATCH v11 2/8] usb: phy: omap-usb2: use the new generic PHY framework
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Used the generic PHY framework API to create the PHY. Now the power off and
power on are done in omap_usb_power_off and omap_usb_power_on respectively.
The omap-usb2 driver is also moved to driver/phy.
However using the old USB PHY library cannot be completely removed
because OTG is intertwined with PHY and moving to the new framework
will break OTG. Once we have a separate OTG state machine, we
can get rid of the USB PHY library.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
drivers/phy/Kconfig | 12 +++++++++
drivers/phy/Makefile | 1 +
drivers/{usb => }/phy/phy-omap-usb2.c | 45 ++++++++++++++++++++++++++++++---
drivers/usb/phy/Kconfig | 10 --------
drivers/usb/phy/Makefile | 1 -
5 files changed, 54 insertions(+), 15 deletions(-)
rename drivers/{usb => }/phy/phy-omap-usb2.c (88%)
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 349bef2..38c3477 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -15,4 +15,16 @@ config GENERIC_PHY
phy users can obtain reference to the PHY. All the users of this
framework should select this config.
+config OMAP_USB2
+ tristate "OMAP USB2 PHY Driver"
+ depends on ARCH_OMAP2PLUS
+ select GENERIC_PHY
+ select USB_PHY
+ select OMAP_CONTROL_USB
+ help
+ Enable this to support the transceiver that is part of SOC. This
+ driver takes care of all the PHY functionality apart from comparator.
+ The USB OTG controller communicates with the comparator using this
+ driver.
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 9e9560f..ed5b088 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,3 +3,4 @@
#
obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
similarity index 88%
rename from drivers/usb/phy/phy-omap-usb2.c
rename to drivers/phy/phy-omap-usb2.c
index 844ab68..25e0f3c 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -28,6 +28,7 @@
#include <linux/pm_runtime.h>
#include <linux/delay.h>
#include <linux/usb/omap_control_usb.h>
+#include <linux/phy/phy.h>
/**
* omap_usb2_set_comparator - links the comparator present in the sytem with
@@ -119,10 +120,36 @@ static int omap_usb2_suspend(struct usb_phy *x, int suspend)
return 0;
}
+static int omap_usb_power_off(struct phy *x)
+{
+ struct omap_usb *phy = phy_get_drvdata(x);
+
+ omap_control_usb_phy_power(phy->control_dev, 0);
+
+ return 0;
+}
+
+static int omap_usb_power_on(struct phy *x)
+{
+ struct omap_usb *phy = phy_get_drvdata(x);
+
+ omap_control_usb_phy_power(phy->control_dev, 1);
+
+ return 0;
+}
+
+static struct phy_ops ops = {
+ .power_on = omap_usb_power_on,
+ .power_off = omap_usb_power_off,
+ .owner = THIS_MODULE,
+};
+
static int omap_usb2_probe(struct platform_device *pdev)
{
struct omap_usb *phy;
+ struct phy *generic_phy;
struct usb_otg *otg;
+ struct phy_provider *phy_provider;
phy = devm_kzalloc(&pdev->dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -144,6 +171,11 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy->phy.otg = otg;
phy->phy.type = USB_PHY_TYPE_USB2;
+ phy_provider = devm_of_phy_provider_register(phy->dev,
+ of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
phy->control_dev = omap_get_control_dev();
if (IS_ERR(phy->control_dev)) {
dev_dbg(&pdev->dev, "Failed to get control device\n");
@@ -159,6 +191,15 @@ static int omap_usb2_probe(struct platform_device *pdev)
otg->start_srp = omap_usb_start_srp;
otg->phy = &phy->phy;
+ platform_set_drvdata(pdev, phy);
+ pm_runtime_enable(phy->dev);
+
+ generic_phy = devm_phy_create(phy->dev, &ops, NULL);
+ if (IS_ERR(generic_phy))
+ return PTR_ERR(generic_phy);
+
+ phy_set_drvdata(generic_phy, phy);
+
phy->wkupclk = devm_clk_get(phy->dev, "usb_phy_cm_clk32k");
if (IS_ERR(phy->wkupclk)) {
dev_err(&pdev->dev, "unable to get usb_phy_cm_clk32k\n");
@@ -174,10 +215,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
usb_add_phy_dev(&phy->phy);
- platform_set_drvdata(pdev, phy);
-
- pm_runtime_enable(phy->dev);
-
return 0;
}
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 3622fff..7813238 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -72,16 +72,6 @@ config OMAP_CONTROL_USB
power on the USB2 PHY is present in OMAP4 and OMAP5. OMAP5 has an
additional register to power on USB3 PHY.
-config OMAP_USB2
- tristate "OMAP USB2 PHY Driver"
- depends on ARCH_OMAP2PLUS
- select OMAP_CONTROL_USB
- help
- Enable this to support the transceiver that is part of SOC. This
- driver takes care of all the PHY functionality apart from comparator.
- The USB OTG controller communicates with the comparator using this
- driver.
-
config OMAP_USB3
tristate "OMAP USB3 PHY Driver"
select OMAP_CONTROL_USB
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 070eca3..56d2b03 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -16,7 +16,6 @@ obj-$(CONFIG_ISP1301_OMAP) += phy-isp1301-omap.o
obj-$(CONFIG_MV_U3D_PHY) += phy-mv-u3d-usb.o
obj-$(CONFIG_NOP_USB_XCEIV) += phy-nop.o
obj-$(CONFIG_OMAP_CONTROL_USB) += phy-omap-control.o
-obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
obj-$(CONFIG_OMAP_USB3) += phy-omap-usb3.o
obj-$(CONFIG_SAMSUNG_USBPHY) += phy-samsung-usb.o
obj-$(CONFIG_SAMSUNG_USB2PHY) += phy-samsung-usb2.o
--
1.7.10.4
^ permalink raw reply related
* [PATCH v11 3/8] usb: phy: twl4030: use the new generic PHY framework
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Used the generic PHY framework API to create the PHY. For powering on
and powering off the PHY, power_on and power_off ops are used. Once the
MUSB OMAP glue is adapted to the new framework, the suspend and resume
ops of usb phy library will be removed. Also twl4030-usb driver is moved
to drivers/phy/.
However using the old usb phy library cannot be completely removed
because otg is intertwined with phy and moving to the new
framework completely will break otg. Once we have a separate otg state machine,
we can get rid of the usb phy library.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
drivers/phy/Kconfig | 11 ++++++
drivers/phy/Makefile | 1 +
drivers/{usb => }/phy/phy-twl4030-usb.c | 56 +++++++++++++++++++++++++++++--
drivers/usb/phy/Kconfig | 9 -----
drivers/usb/phy/Makefile | 1 -
include/linux/i2c/twl.h | 2 ++
6 files changed, 67 insertions(+), 13 deletions(-)
rename drivers/{usb => }/phy/phy-twl4030-usb.c (95%)
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index 38c3477..ac239ac 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -27,4 +27,15 @@ config OMAP_USB2
The USB OTG controller communicates with the comparator using this
driver.
+config TWL4030_USB
+ tristate "TWL4030 USB Transceiver Driver"
+ depends on TWL4030_CORE && REGULATOR_TWL4030 && USB_MUSB_OMAP2PLUS
+ select GENERIC_PHY
+ select USB_PHY
+ help
+ Enable this to support the USB OTG transceiver on TWL4030
+ family chips (including the TWL5030 and TPS659x0 devices).
+ This transceiver supports high and full speed devices plus,
+ in host mode, low speed.
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index ed5b088..0dd8a98 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -4,3 +4,4 @@
obj-$(CONFIG_GENERIC_PHY) += phy-core.o
obj-$(CONFIG_OMAP_USB2) += phy-omap-usb2.o
+obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
diff --git a/drivers/usb/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
similarity index 95%
rename from drivers/usb/phy/phy-twl4030-usb.c
rename to drivers/phy/phy-twl4030-usb.c
index 8f78d2d..494f107 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -33,6 +33,7 @@
#include <linux/io.h>
#include <linux/delay.h>
#include <linux/usb/otg.h>
+#include <linux/phy/phy.h>
#include <linux/usb/musb-omap.h>
#include <linux/usb/ulpi.h>
#include <linux/i2c/twl.h>
@@ -431,6 +432,14 @@ static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
dev_dbg(twl->dev, "%s\n", __func__);
}
+static int twl4030_phy_power_off(struct phy *phy)
+{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
+ twl4030_phy_suspend(twl, 0);
+ return 0;
+}
+
static void __twl4030_phy_resume(struct twl4030_usb *twl)
{
twl4030_phy_power(twl, 1);
@@ -459,6 +468,14 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
}
}
+static int twl4030_phy_power_on(struct phy *phy)
+{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
+ twl4030_phy_resume(twl);
+ return 0;
+}
+
static int twl4030_usb_ldo_init(struct twl4030_usb *twl)
{
/* Enable writing to power configuration registers */
@@ -602,13 +619,22 @@ static int twl4030_usb_phy_init(struct usb_phy *phy)
status = twl4030_usb_linkstat(twl);
twl->linkstat = status;
- if (status = OMAP_MUSB_ID_GROUND || status = OMAP_MUSB_VBUS_VALID)
+ if (status = OMAP_MUSB_ID_GROUND || status = OMAP_MUSB_VBUS_VALID) {
omap_musb_mailbox(twl->linkstat);
+ twl4030_phy_resume(twl);
+ }
sysfs_notify(&twl->dev->kobj, NULL, "vbus");
return 0;
}
+static int twl4030_phy_init(struct phy *phy)
+{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
+ return twl4030_usb_phy_init(&twl->phy);
+}
+
static int twl4030_set_suspend(struct usb_phy *x, int suspend)
{
struct twl4030_usb *twl = phy_to_twl(x);
@@ -646,13 +672,23 @@ static int twl4030_set_host(struct usb_otg *otg, struct usb_bus *host)
return 0;
}
+static const struct phy_ops ops = {
+ .init = twl4030_phy_init,
+ .power_on = twl4030_phy_power_on,
+ .power_off = twl4030_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
static int twl4030_usb_probe(struct platform_device *pdev)
{
struct twl4030_usb_data *pdata = pdev->dev.platform_data;
struct twl4030_usb *twl;
+ struct phy *phy;
int status, err;
struct usb_otg *otg;
struct device_node *np = pdev->dev.of_node;
+ struct phy_provider *phy_provider;
+ struct phy_init_data *init_data = NULL;
twl = devm_kzalloc(&pdev->dev, sizeof *twl, GFP_KERNEL);
if (!twl)
@@ -661,9 +697,10 @@ static int twl4030_usb_probe(struct platform_device *pdev)
if (np)
of_property_read_u32(np, "usb_mode",
(enum twl4030_usb_mode *)&twl->usb_mode);
- else if (pdata)
+ else if (pdata) {
twl->usb_mode = pdata->usb_mode;
- else {
+ init_data = pdata->init_data;
+ } else {
dev_err(&pdev->dev, "twl4030 initialized without pdata\n");
return -EINVAL;
}
@@ -689,6 +726,19 @@ static int twl4030_usb_probe(struct platform_device *pdev)
otg->set_host = twl4030_set_host;
otg->set_peripheral = twl4030_set_peripheral;
+ phy_provider = devm_of_phy_provider_register(twl->dev,
+ of_phy_simple_xlate);
+ if (IS_ERR(phy_provider))
+ return PTR_ERR(phy_provider);
+
+ phy = devm_phy_create(twl->dev, &ops, init_data);
+ if (IS_ERR(phy)) {
+ dev_dbg(&pdev->dev, "Failed to create PHY\n");
+ return PTR_ERR(phy);
+ }
+
+ phy_set_drvdata(phy, twl);
+
/* init spinlock for workqueue */
spin_lock_init(&twl->lock);
diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig
index 7813238..1e05b1d 100644
--- a/drivers/usb/phy/Kconfig
+++ b/drivers/usb/phy/Kconfig
@@ -102,15 +102,6 @@ config SAMSUNG_USB3PHY
Enable this to support Samsung USB 3.0 (Super Speed) phy controller
for samsung SoCs.
-config TWL4030_USB
- tristate "TWL4030 USB Transceiver Driver"
- depends on TWL4030_CORE && REGULATOR_TWL4030 && USB_MUSB_OMAP2PLUS
- help
- Enable this to support the USB OTG transceiver on TWL4030
- family chips (including the TWL5030 and TPS659x0 devices).
- This transceiver supports high and full speed devices plus,
- in host mode, low speed.
-
config TWL6030_USB
tristate "TWL6030 USB Transceiver Driver"
depends on TWL4030_CORE && OMAP_USB2 && USB_MUSB_OMAP2PLUS
diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile
index 56d2b03..7bcc9ed0 100644
--- a/drivers/usb/phy/Makefile
+++ b/drivers/usb/phy/Makefile
@@ -20,7 +20,6 @@ obj-$(CONFIG_OMAP_USB3) += phy-omap-usb3.o
obj-$(CONFIG_SAMSUNG_USBPHY) += phy-samsung-usb.o
obj-$(CONFIG_SAMSUNG_USB2PHY) += phy-samsung-usb2.o
obj-$(CONFIG_SAMSUNG_USB3PHY) += phy-samsung-usb3.o
-obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
obj-$(CONFIG_TWL6030_USB) += phy-twl6030-usb.o
obj-$(CONFIG_USB_EHCI_TEGRA) += phy-tegra-usb.o
obj-$(CONFIG_USB_GPIO_VBUS) += phy-gpio-vbus-usb.o
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index 81cbbdb..673a3ce 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -26,6 +26,7 @@
#define __TWL_H_
#include <linux/types.h>
+#include <linux/phy/phy.h>
#include <linux/input/matrix_keypad.h>
/*
@@ -615,6 +616,7 @@ enum twl4030_usb_mode {
struct twl4030_usb_data {
enum twl4030_usb_mode usb_mode;
unsigned long features;
+ struct phy_init_data *init_data;
int (*phy_init)(struct device *dev);
int (*phy_exit)(struct device *dev);
--
1.7.10.4
^ permalink raw reply related
* [PATCH v11 4/8] arm: omap3: twl: add phy consumer data in twl4030_usb_data
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
The PHY framework uses the phy consumer data populated in platform data in the
case of non-dt boot to return the reference to the PHY when the controller
(PHY consumer) requests for it. So populated the phy consumer data in the platform
data of twl usb.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
---
arch/arm/mach-omap2/twl-common.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm/mach-omap2/twl-common.c b/arch/arm/mach-omap2/twl-common.c
index c05898f..b0d54da 100644
--- a/arch/arm/mach-omap2/twl-common.c
+++ b/arch/arm/mach-omap2/twl-common.c
@@ -24,6 +24,7 @@
#include <linux/i2c/twl.h>
#include <linux/gpio.h>
#include <linux/string.h>
+#include <linux/phy/phy.h>
#include <linux/regulator/machine.h>
#include <linux/regulator/fixed.h>
@@ -90,8 +91,18 @@ void __init omap_pmic_late_init(void)
}
#if defined(CONFIG_ARCH_OMAP3)
+struct phy_consumer consumers[] = {
+ PHY_CONSUMER("musb-hdrc.0", "usb"),
+};
+
+struct phy_init_data init_data = {
+ .consumers = consumers,
+ .num_consumers = ARRAY_SIZE(consumers),
+};
+
static struct twl4030_usb_data omap3_usb_pdata = {
.usb_mode = T2_USB_MODE_ULPI,
+ .init_data = &init_data,
};
static int omap3_batt_table[] = {
--
1.7.10.4
^ permalink raw reply related
* [PATCH v11 5/8] ARM: dts: omap: update usb_otg_hs data
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Updated the usb_otg_hs dt data to include the *phy* and *phy-names*
binding in order for the driver to use the new generic PHY framework.
Also updated the Documentation to include the binding information.
The PHY binding information can be found at
Documentation/devicetree/bindings/phy/phy-bindings.txt
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Acked-by: Tony Lindgren <tony@atomide.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
Documentation/devicetree/bindings/usb/omap-usb.txt | 5 +++++
Documentation/devicetree/bindings/usb/usb-phy.txt | 6 ++++++
arch/arm/boot/dts/omap3-beagle-xm.dts | 2 ++
arch/arm/boot/dts/omap3-evm.dts | 2 ++
arch/arm/boot/dts/omap3-overo.dtsi | 2 ++
arch/arm/boot/dts/omap4.dtsi | 3 +++
arch/arm/boot/dts/twl4030.dtsi | 1 +
7 files changed, 21 insertions(+)
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt b/Documentation/devicetree/bindings/usb/omap-usb.txt
index 57e71f6..825790d 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -19,6 +19,9 @@ OMAP MUSB GLUE
- power : Should be "50". This signifies the controller can supply up to
100mA when operating in host mode.
- usb-phy : the phandle for the PHY device
+ - phys : the phandle for the PHY device (used by generic PHY framework)
+ - phy-names : the names of the PHY corresponding to the PHYs present in the
+ *phy* phandle.
Optional properties:
- ctrl-module : phandle of the control module this glue uses to write to
@@ -33,6 +36,8 @@ usb_otg_hs: usb_otg_hs@4a0ab000 {
num-eps = <16>;
ram-bits = <12>;
ctrl-module = <&omap_control_usb>;
+ phys = <&usb2_phy>;
+ phy-names = "usb2-phy";
};
Board specific device node entry
diff --git a/Documentation/devicetree/bindings/usb/usb-phy.txt b/Documentation/devicetree/bindings/usb/usb-phy.txt
index 61496f5..c0245c8 100644
--- a/Documentation/devicetree/bindings/usb/usb-phy.txt
+++ b/Documentation/devicetree/bindings/usb/usb-phy.txt
@@ -5,6 +5,8 @@ OMAP USB2 PHY
Required properties:
- compatible: Should be "ti,omap-usb2"
- reg : Address and length of the register set for the device.
+ - #phy-cells: determine the number of cells that should be given in the
+ phandle while referencing this phy.
Optional properties:
- ctrl-module : phandle of the control module used by PHY driver to power on
@@ -16,6 +18,7 @@ usb2phy@4a0ad080 {
compatible = "ti,omap-usb2";
reg = <0x4a0ad080 0x58>;
ctrl-module = <&omap_control_usb>;
+ #phy-cells = <0>;
};
OMAP USB3 PHY
@@ -25,6 +28,8 @@ Required properties:
- reg : Address and length of the register set for the device.
- reg-names: The names of the register addresses corresponding to the registers
filled in "reg".
+ - #phy-cells: determine the number of cells that should be given in the
+ phandle while referencing this phy.
Optional properties:
- ctrl-module : phandle of the control module used by PHY driver to power on
@@ -39,4 +44,5 @@ usb3phy@4a084400 {
<0x4a084c00 0x40>;
reg-names = "phy_rx", "phy_tx", "pll_ctrl";
ctrl-module = <&omap_control_usb>;
+ #phy-cells = <0>;
};
diff --git a/arch/arm/boot/dts/omap3-beagle-xm.dts b/arch/arm/boot/dts/omap3-beagle-xm.dts
index afdb164..533b2da 100644
--- a/arch/arm/boot/dts/omap3-beagle-xm.dts
+++ b/arch/arm/boot/dts/omap3-beagle-xm.dts
@@ -144,6 +144,8 @@
&usb_otg_hs {
interface-type = <0>;
usb-phy = <&usb2_phy>;
+ phys = <&usb2_phy>;
+ phy-names = "usb2-phy";
mode = <3>;
power = <50>;
};
diff --git a/arch/arm/boot/dts/omap3-evm.dts b/arch/arm/boot/dts/omap3-evm.dts
index 7d4329d..4134dd0 100644
--- a/arch/arm/boot/dts/omap3-evm.dts
+++ b/arch/arm/boot/dts/omap3-evm.dts
@@ -70,6 +70,8 @@
&usb_otg_hs {
interface-type = <0>;
usb-phy = <&usb2_phy>;
+ phys = <&usb2_phy>;
+ phy-names = "usb2-phy";
mode = <3>;
power = <50>;
};
diff --git a/arch/arm/boot/dts/omap3-overo.dtsi b/arch/arm/boot/dts/omap3-overo.dtsi
index 8f1abec..a461d2f 100644
--- a/arch/arm/boot/dts/omap3-overo.dtsi
+++ b/arch/arm/boot/dts/omap3-overo.dtsi
@@ -76,6 +76,8 @@
&usb_otg_hs {
interface-type = <0>;
usb-phy = <&usb2_phy>;
+ phys = <&usb2_phy>;
+ phy-names = "usb2-phy";
mode = <3>;
power = <50>;
};
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 22d9f2b..1e8e2fe 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -520,6 +520,7 @@
compatible = "ti,omap-usb2";
reg = <0x4a0ad080 0x58>;
ctrl-module = <&omap_control_usb>;
+ #phy-cells = <0>;
};
};
@@ -658,6 +659,8 @@
interrupt-names = "mc", "dma";
ti,hwmods = "usb_otg_hs";
usb-phy = <&usb2_phy>;
+ phys = <&usb2_phy>;
+ phy-names = "usb2-phy";
multipoint = <1>;
num-eps = <16>;
ram-bits = <12>;
diff --git a/arch/arm/boot/dts/twl4030.dtsi b/arch/arm/boot/dts/twl4030.dtsi
index ae6a17a..5aba238 100644
--- a/arch/arm/boot/dts/twl4030.dtsi
+++ b/arch/arm/boot/dts/twl4030.dtsi
@@ -86,6 +86,7 @@
usb1v8-supply = <&vusb1v8>;
usb3v1-supply = <&vusb3v1>;
usb_mode = <1>;
+ #phy-cells = <0>;
};
twl_pwm: pwm {
--
1.7.10.4
^ permalink raw reply related
* [PATCH v11 6/8] usb: musb: omap2430: use the new generic PHY framework
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Use the generic PHY framework API to get the PHY. The usb_phy_set_resume
and usb_phy_set_suspend is replaced with power_on and
power_off to align with the new PHY framework.
musb->xceiv can't be removed as of now because musb core uses xceiv.state and
xceiv.otg. Once there is a separate state machine to handle otg, these can be
moved out of xceiv and then we can start using the generic PHY framework.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
Acked-by: Felipe Balbi <balbi@ti.com>
---
drivers/usb/musb/Kconfig | 1 +
drivers/usb/musb/musb_core.h | 2 ++
drivers/usb/musb/omap2430.c | 26 ++++++++++++++++++++------
3 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 797e3fd..25e2e57 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -76,6 +76,7 @@ config USB_MUSB_TUSB6010
config USB_MUSB_OMAP2PLUS
tristate "OMAP2430 and onwards"
depends on ARCH_OMAP2PLUS
+ select GENERIC_PHY
config USB_MUSB_AM35X
tristate "AM35x"
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 7d341c3..6e567bd 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -46,6 +46,7 @@
#include <linux/usb.h>
#include <linux/usb/otg.h>
#include <linux/usb/musb.h>
+#include <linux/phy/phy.h>
struct musb;
struct musb_hw_ep;
@@ -346,6 +347,7 @@ struct musb {
u16 int_tx;
struct usb_phy *xceiv;
+ struct phy *phy;
int nIrq;
unsigned irq_wake:1;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index f44e8b5..063773a 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -348,11 +348,21 @@ static int omap2430_musb_init(struct musb *musb)
* up through ULPI. TWL4030-family PMICs include one,
* which needs a driver, drivers aren't always needed.
*/
- if (dev->parent->of_node)
+ if (dev->parent->of_node) {
+ musb->phy = devm_phy_get(dev->parent, "usb2-phy");
+
+ /* We can't totally remove musb->xceiv as of now because
+ * musb core uses xceiv.state and xceiv.otg. Once we have
+ * a separate state machine to handle otg, these can be moved
+ * out of xceiv and then we can start using the generic PHY
+ * framework
+ */
musb->xceiv = devm_usb_get_phy_by_phandle(dev->parent,
"usb-phy", 0);
- else
+ } else {
musb->xceiv = devm_usb_get_phy_dev(dev, 0);
+ musb->phy = devm_phy_get(dev, "usb");
+ }
if (IS_ERR(musb->xceiv)) {
status = PTR_ERR(musb->xceiv);
@@ -364,6 +374,10 @@ static int omap2430_musb_init(struct musb *musb)
return -EPROBE_DEFER;
}
+ if (IS_ERR(musb->phy)) {
+ pr_err("HS USB OTG: no PHY configured\n");
+ return PTR_ERR(musb->phy);
+ }
musb->isr = omap2430_musb_interrupt;
status = pm_runtime_get_sync(dev);
@@ -397,7 +411,7 @@ static int omap2430_musb_init(struct musb *musb)
if (glue->status != OMAP_MUSB_UNKNOWN)
omap_musb_set_mailbox(glue);
- usb_phy_init(musb->xceiv);
+ phy_init(musb->phy);
pm_runtime_put_noidle(musb->controller);
return 0;
@@ -460,6 +474,7 @@ static int omap2430_musb_exit(struct musb *musb)
del_timer_sync(&musb_idle_timer);
omap2430_low_level_exit(musb);
+ phy_exit(musb->phy);
return 0;
}
@@ -638,7 +653,7 @@ static int omap2430_runtime_suspend(struct device *dev)
OTG_INTERFSEL);
omap2430_low_level_exit(musb);
- usb_phy_set_suspend(musb->xceiv, 1);
+ phy_power_off(musb->phy);
}
return 0;
@@ -653,8 +668,7 @@ static int omap2430_runtime_resume(struct device *dev)
omap2430_low_level_init(musb);
musb_writel(musb->mregs, OTG_INTERFSEL,
musb->context.otg_interfsel);
-
- usb_phy_set_suspend(musb->xceiv, 0);
+ phy_power_on(musb->phy);
}
return 0;
--
1.7.10.4
^ permalink raw reply related
* [PATCH v11 7/8] usb: phy: omap-usb2: remove *set_suspend* callback from omap-usb2
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Now that omap-usb2 is adapted to the new generic PHY framework,
*set_suspend* ops can be removed from omap-usb2 driver.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
drivers/phy/phy-omap-usb2.c | 25 -------------------------
1 file changed, 25 deletions(-)
diff --git a/drivers/phy/phy-omap-usb2.c b/drivers/phy/phy-omap-usb2.c
index 25e0f3c..dec3fab 100644
--- a/drivers/phy/phy-omap-usb2.c
+++ b/drivers/phy/phy-omap-usb2.c
@@ -97,29 +97,6 @@ static int omap_usb_set_peripheral(struct usb_otg *otg,
return 0;
}
-static int omap_usb2_suspend(struct usb_phy *x, int suspend)
-{
- u32 ret;
- struct omap_usb *phy = phy_to_omapusb(x);
-
- if (suspend && !phy->is_suspended) {
- omap_control_usb_phy_power(phy->control_dev, 0);
- pm_runtime_put_sync(phy->dev);
- phy->is_suspended = 1;
- } else if (!suspend && phy->is_suspended) {
- ret = pm_runtime_get_sync(phy->dev);
- if (ret < 0) {
- dev_err(phy->dev, "get_sync failed with err %d\n",
- ret);
- return ret;
- }
- omap_control_usb_phy_power(phy->control_dev, 1);
- phy->is_suspended = 0;
- }
-
- return 0;
-}
-
static int omap_usb_power_off(struct phy *x)
{
struct omap_usb *phy = phy_get_drvdata(x);
@@ -167,7 +144,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy->phy.dev = phy->dev;
phy->phy.label = "omap-usb2";
- phy->phy.set_suspend = omap_usb2_suspend;
phy->phy.otg = otg;
phy->phy.type = USB_PHY_TYPE_USB2;
@@ -182,7 +158,6 @@ static int omap_usb2_probe(struct platform_device *pdev)
return -ENODEV;
}
- phy->is_suspended = 1;
omap_control_usb_phy_power(phy->control_dev, 0);
otg->set_host = omap_usb_set_host;
--
1.7.10.4
^ permalink raw reply related
* [PATCH v11 8/8] usb: phy: twl4030-usb: remove *set_suspend* and *phy_init* ops
From: Kishon Vijay Abraham I @ 2013-08-21 5:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1377063973-22044-1-git-send-email-kishon@ti.com>
Now that twl4030-usb is adapted to the new generic PHY framework,
*set_suspend* and *phy_init* ops can be removed from twl4030-usb driver.
Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
Acked-by: Felipe Balbi <balbi@ti.com>
Reviewed-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
drivers/phy/phy-twl4030-usb.c | 57 ++++++++++-------------------------------
1 file changed, 13 insertions(+), 44 deletions(-)
diff --git a/drivers/phy/phy-twl4030-usb.c b/drivers/phy/phy-twl4030-usb.c
index 494f107..c437211 100644
--- a/drivers/phy/phy-twl4030-usb.c
+++ b/drivers/phy/phy-twl4030-usb.c
@@ -422,25 +422,20 @@ static void twl4030_phy_power(struct twl4030_usb *twl, int on)
}
}
-static void twl4030_phy_suspend(struct twl4030_usb *twl, int controller_off)
+static int twl4030_phy_power_off(struct phy *phy)
{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
if (twl->asleep)
- return;
+ return 0;
twl4030_phy_power(twl, 0);
twl->asleep = 1;
dev_dbg(twl->dev, "%s\n", __func__);
-}
-
-static int twl4030_phy_power_off(struct phy *phy)
-{
- struct twl4030_usb *twl = phy_get_drvdata(phy);
-
- twl4030_phy_suspend(twl, 0);
return 0;
}
-static void __twl4030_phy_resume(struct twl4030_usb *twl)
+static void __twl4030_phy_power_on(struct twl4030_usb *twl)
{
twl4030_phy_power(twl, 1);
twl4030_i2c_access(twl, 1);
@@ -449,11 +444,13 @@ static void __twl4030_phy_resume(struct twl4030_usb *twl)
twl4030_i2c_access(twl, 0);
}
-static void twl4030_phy_resume(struct twl4030_usb *twl)
+static int twl4030_phy_power_on(struct phy *phy)
{
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
+
if (!twl->asleep)
- return;
- __twl4030_phy_resume(twl);
+ return 0;
+ __twl4030_phy_power_on(twl);
twl->asleep = 0;
dev_dbg(twl->dev, "%s\n", __func__);
@@ -466,13 +463,6 @@ static void twl4030_phy_resume(struct twl4030_usb *twl)
cancel_delayed_work(&twl->id_workaround_work);
schedule_delayed_work(&twl->id_workaround_work, HZ);
}
-}
-
-static int twl4030_phy_power_on(struct phy *phy)
-{
- struct twl4030_usb *twl = phy_get_drvdata(phy);
-
- twl4030_phy_resume(twl);
return 0;
}
@@ -604,9 +594,9 @@ static void twl4030_id_workaround_work(struct work_struct *work)
}
}
-static int twl4030_usb_phy_init(struct usb_phy *phy)
+static int twl4030_phy_init(struct phy *phy)
{
- struct twl4030_usb *twl = phy_to_twl(phy);
+ struct twl4030_usb *twl = phy_get_drvdata(phy);
enum omap_musb_vbus_id_status status;
/*
@@ -621,32 +611,13 @@ static int twl4030_usb_phy_init(struct usb_phy *phy)
if (status = OMAP_MUSB_ID_GROUND || status = OMAP_MUSB_VBUS_VALID) {
omap_musb_mailbox(twl->linkstat);
- twl4030_phy_resume(twl);
+ twl4030_phy_power_on(phy);
}
sysfs_notify(&twl->dev->kobj, NULL, "vbus");
return 0;
}
-static int twl4030_phy_init(struct phy *phy)
-{
- struct twl4030_usb *twl = phy_get_drvdata(phy);
-
- return twl4030_usb_phy_init(&twl->phy);
-}
-
-static int twl4030_set_suspend(struct usb_phy *x, int suspend)
-{
- struct twl4030_usb *twl = phy_to_twl(x);
-
- if (suspend)
- twl4030_phy_suspend(twl, 1);
- else
- twl4030_phy_resume(twl);
-
- return 0;
-}
-
static int twl4030_set_peripheral(struct usb_otg *otg,
struct usb_gadget *gadget)
{
@@ -719,8 +690,6 @@ static int twl4030_usb_probe(struct platform_device *pdev)
twl->phy.label = "twl4030";
twl->phy.otg = otg;
twl->phy.type = USB_PHY_TYPE_USB2;
- twl->phy.set_suspend = twl4030_set_suspend;
- twl->phy.init = twl4030_usb_phy_init;
otg->phy = &twl->phy;
otg->set_host = twl4030_set_host;
--
1.7.10.4
^ permalink raw reply related
* RE: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
From: Inki Dae @ 2013-08-21 8:40 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1376385576-9039-2-git-send-email-inki.dae@samsung.com>
Thanks for the review,
Inki Dae
> -----Original Message-----
> From: linux-fbdev-owner@vger.kernel.org [mailto:linux-fbdev-
> owner@vger.kernel.org] On Behalf Of Konrad Rzeszutek Wilk
> Sent: Wednesday, August 21, 2013 4:22 AM
> To: Inki Dae
> Cc: dri-devel@lists.freedesktop.org; linux-fbdev@vger.kernel.org; linux-
> arm-kernel@lists.infradead.org; linux-media@vger.kernel.org; linaro-
> kernel@lists.linaro.org; kyungmin.park@samsung.com;
> myungjoo.ham@samsung.com
> Subject: Re: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer
> synchronization framework
>
> On Tue, Aug 13, 2013 at 06:19:35PM +0900, Inki Dae wrote:
> > This patch adds a buffer synchronization framework based on DMA BUF[1]
> > and and based on ww-mutexes[2] for lock mechanism.
> >
> > The purpose of this framework is to provide not only buffer access
> control
> > to CPU and DMA but also easy-to-use interfaces for device drivers and
> > user application. This framework can be used for all dma devices using
> > system memory as dma buffer, especially for most ARM based SoCs.
> >
> > Changelog v6:
> > - Fix sync lock to multiple reads.
> > - Add select system call support.
> > . Wake up poll_wait when a dmabuf is unlocked.
> > - Remove unnecessary the use of mutex lock.
> > - Add private backend ops callbacks.
> > . This ops has one callback for device drivers to clean up their
> > sync object resource when the sync object is freed. For this,
> > device drivers should implement the free callback properly.
> > - Update document file.
> >
> > Changelog v5:
> > - Rmove a dependence on reservation_object: the reservation_object is
> used
> > to hook up to ttm and dma-buf for easy sharing of reservations across
> > devices. However, the dmabuf sync can be used for all dma devices;
> v4l2
> > and drm based drivers, so doesn't need the reservation_object anymore.
> > With regared to this, it adds 'void *sync' to dma_buf structure.
> > - All patches are rebased on mainline, Linux v3.10.
> >
> > Changelog v4:
> > - Add user side interface for buffer synchronization mechanism and
> update
> > descriptions related to the user side interface.
> >
> > Changelog v3:
> > - remove cache operation relevant codes and update document file.
> >
> > Changelog v2:
> > - use atomic_add_unless to avoid potential bug.
> > - add a macro for checking valid access type.
> > - code clean.
> >
> > The mechanism of this framework has the following steps,
> > 1. Register dmabufs to a sync object - A task gets a new sync object
> and
> > can add one or more dmabufs that the task wants to access.
> > This registering should be performed when a device context or an
> event
> > context such as a page flip event is created or before CPU accesses
a
> shared
> > buffer.
> >
> > dma_buf_sync_get(a sync object, a dmabuf);
> >
> > 2. Lock a sync object - A task tries to lock all dmabufs added in
its
> own
> > sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid
> dead
> > lock issue and for race condition between CPU and CPU, CPU and DMA,
> and DMA
> > and DMA. Taking a lock means that others cannot access all locked
> dmabufs
> > until the task that locked the corresponding dmabufs, unlocks all
the
> locked
> > dmabufs.
> > This locking should be performed before DMA or CPU accesses these
> dmabufs.
> >
> > dma_buf_sync_lock(a sync object);
> >
> > 3. Unlock a sync object - The task unlocks all dmabufs added in its
> own sync
> > object. The unlock means that the DMA or CPU accesses to the dmabufs
> have
> > been completed so that others may access them.
> > This unlocking should be performed after DMA or CPU has completed
> accesses
> > to the dmabufs.
> >
> > dma_buf_sync_unlock(a sync object);
> >
> > 4. Unregister one or all dmabufs from a sync object - A task
> unregisters
> > the given dmabufs from the sync object. This means that the task
> dosen't
> > want to lock the dmabufs.
> > The unregistering should be performed after DMA or CPU has completed
> > accesses to the dmabufs or when dma_buf_sync_lock() is failed.
> >
> > dma_buf_sync_put(a sync object, a dmabuf);
> > dma_buf_sync_put_all(a sync object);
> >
> > The described steps may be summarized as:
> > get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
> >
> > This framework includes the following two features.
> > 1. read (shared) and write (exclusive) locks - A task is required to
> declare
> > the access type when the task tries to register a dmabuf;
> > READ, WRITE, READ DMA, or WRITE DMA.
> >
> > The below is example codes,
> > struct dmabuf_sync *sync;
> >
> > sync = dmabuf_sync_init(...);
> > ...
> >
> > dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
> > ...
> >
> > And the below can be used as access types:
> > DMA_BUF_ACCESS_R - CPU will access a buffer for read.
> > DMA_BUF_ACCESS_W - CPU will access a buffer for read or
> write.
> > DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
> > DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or
> > write.
> >
> > 2. Mandatory resource releasing - a task cannot hold a lock
> indefinitely.
> > A task may never try to unlock a buffer after taking a lock to the
> buffer.
> > In this case, a timer handler to the corresponding sync object is
> called
> > in five (default) seconds and then the timed-out buffer is unlocked
> by work
> > queue handler to avoid lockups and to enforce resources of the
buffer.
> >
> > The below is how to use interfaces for device driver:
> > 1. Allocate and Initialize a sync object:
> > static void xxx_dmabuf_sync_free(void *priv)
> > {
> > struct xxx_context *ctx = priv;
> >
> > if (!ctx)
> > return;
> >
> > ctx->sync = NULL;
> > }
> > ...
> >
> > static struct dmabuf_sync_priv_ops driver_specific_ops = {
> > .free = xxx_dmabuf_sync_free,
> > };
> > ...
> >
> > struct dmabuf_sync *sync;
> >
> > sync = dmabuf_sync_init("test sync", &driver_specific_ops,
> ctx);
> > ...
> >
> > 2. Add a dmabuf to the sync object when setting up dma buffer
> relevant
> > registers:
> > dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
> > ...
> >
> > 3. Lock all dmabufs of the sync object before DMA or CPU accesses
> > the dmabufs:
> > dmabuf_sync_lock(sync);
> > ...
> >
> > 4. Now CPU or DMA can access all dmabufs locked in step 3.
> >
> > 5. Unlock all dmabufs added in a sync object after DMA or CPU
> access
> > to these dmabufs is completed:
> > dmabuf_sync_unlock(sync);
> >
> > And call the following functions to release all resources,
> > dmabuf_sync_put_all(sync);
> > dmabuf_sync_fini(sync);
> >
> > You can refer to actual example codes:
> > "drm/exynos: add dmabuf sync support for g2d driver" and
> > "drm/exynos: add dmabuf sync support for kms framework" from
> > https://git.kernel.org/cgit/linux/kernel/git/daeinki/
> > drm-exynos.git/log/?h=dmabuf-sync
> >
> > And this framework includes fcntl system call[3] as interfaces exported
> > to user. As you know, user sees a buffer object as a dma-buf file
> descriptor.
> > So fcntl() call with the file descriptor means to lock some buffer
> region being
> > managed by the dma-buf object.
> >
> > The below is how to use interfaces for user application:
> >
> > fcntl system call:
> >
> > struct flock filelock;
> >
> > 1. Lock a dma buf:
> > filelock.l_type = F_WRLCK or F_RDLCK;
> >
> > /* lock entire region to the dma buf. */
> > filelock.lwhence = SEEK_CUR;
> > filelock.l_start = 0;
> > filelock.l_len = 0;
> >
> > fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> > ...
> > CPU access to the dma buf
> >
> > 2. Unlock a dma buf:
> > filelock.l_type = F_UNLCK;
> >
> > fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> >
> > close(dmabuf fd) call would also unlock the dma buf. And for
> more
> > detail, please refer to [3]
> >
> > select system call:
> >
> > fd_set wdfs or rdfs;
> >
> > FD_ZERO(&wdfs or &rdfs);
> > FD_SET(fd, &wdfs or &rdfs);
> >
> > select(fd + 1, &rdfs, NULL, NULL, NULL);
> > or
> > select(fd + 1, NULL, &wdfs, NULL, NULL);
> >
> > Every time select system call is called, a caller will wait for
> > the completion of DMA or CPU access to a shared buffer if there
> > is someone accessing the shared buffer; locked the shared buffer.
> > However, if no anyone then select system call will be returned
> > at once.
> >
> > References:
> > [1] http://lwn.net/Articles/470339/
> > [2] https://patchwork.kernel.org/patch/2625361/
> > [3] http://linux.die.net/man/2/fcntl
> >
> > Signed-off-by: Inki Dae <inki.dae@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> > Documentation/dma-buf-sync.txt | 285 +++++++++++++++++
> > drivers/base/Kconfig | 7 +
> > drivers/base/Makefile | 1 +
> > drivers/base/dma-buf.c | 4 +
> > drivers/base/dmabuf-sync.c | 678
> ++++++++++++++++++++++++++++++++++++++++
> > include/linux/dma-buf.h | 16 +
> > include/linux/dmabuf-sync.h | 190 +++++++++++
> > 7 files changed, 1181 insertions(+), 0 deletions(-)
> > create mode 100644 Documentation/dma-buf-sync.txt
> > create mode 100644 drivers/base/dmabuf-sync.c
> > create mode 100644 include/linux/dmabuf-sync.h
> >
> > diff --git a/Documentation/dma-buf-sync.txt b/Documentation/dma-buf-
> sync.txt
> > new file mode 100644
> > index 0000000..8023d06
> > --- /dev/null
> > +++ b/Documentation/dma-buf-sync.txt
> > @@ -0,0 +1,285 @@
> > + DMA Buffer Synchronization Framework
> > + ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > + Inki Dae
> > + <inki dot dae at samsung dot com>
> > + <daeinki at gmail dot com>
> > +
> > +This document is a guide for device-driver writers describing the DMA
> buffer
> > +synchronization API. This document also describes how to use the API to
> > +use buffer synchronization mechanism between DMA and DMA, CPU and DMA,
> and
> > +CPU and CPU.
> > +
> > +The DMA Buffer synchronization API provides buffer synchronization
> mechanism;
> > +i.e., buffer access control to CPU and DMA, and easy-to-use interfaces
> for
> > +device drivers and user application. And this API can be used for all
> dma
> > +devices using system memory as dma buffer, especially for most ARM
> based SoCs.
> > +
> > +
> > +Motivation
> > +----------
> > +
> > +Buffer synchronization issue between DMA and DMA:
> > + Sharing a buffer, a device cannot be aware of when the other device
> > + will access the shared buffer: a device may access a buffer
> containing
> > + wrong data if the device accesses the shared buffer while another
> > + device is still accessing the shared buffer.
> > + Therefore, a user process should have waited for the completion of
> DMA
> > + access by another device before a device tries to access the shared
> > + buffer.
> > +
> > +Buffer synchronization issue between CPU and DMA:
> > + A user process should consider that when having to send a buffer,
> filled
> > + by CPU, to a device driver for the device driver to access the
> buffer as
> > + a input buffer while CPU and DMA are sharing the buffer.
> > + This means that the user process needs to understand how the device
> > + driver is worked. Hence, the conventional mechanism not only makes
> > + user application complicated but also incurs performance overhead.
> > +
> > +Buffer synchronization issue between CPU and CPU:
> > + In case that two processes share one buffer; shared with DMA also,
> > + they may need some mechanism to allow process B to access the
> shared
> > + buffer after the completion of CPU access by process A.
> > + Therefore, process B should have waited for the completion of CPU
> access
> > + by process A using the mechanism before trying to access the shared
> > + buffer.
> > +
> > +What is the best way to solve these buffer synchronization issues?
> > + We may need a common object that a device driver and a user process
> > + notify the common object of when they try to access a shared buffer.
> > + That way we could decide when we have to allow or not to allow for
> CPU
> > + or DMA to access the shared buffer through the common object.
> > + If so, what could become the common object? Right, that's a dma-
> buf[1].
> > + Now we have already been using the dma-buf to share one buffer with
> > + other drivers.
> > +
> > +
> > +Basic concept
> > +-------------
> > +
> > +The mechanism of this framework has the following steps,
> > + 1. Register dmabufs to a sync object - A task gets a new sync
object
> and
> > + can add one or more dmabufs that the task wants to access.
> > + This registering should be performed when a device context or an
> event
> > + context such as a page flip event is created or before CPU accesses
> a shared
> > + buffer.
> > +
> > + dma_buf_sync_get(a sync object, a dmabuf);
> > +
> > + 2. Lock a sync object - A task tries to lock all dmabufs added in
> its own
> > + sync object. Basically, the lock mechanism uses ww-mutexes[2] to
> avoid dead
> > + lock issue and for race condition between CPU and CPU, CPU and DMA,
> and DMA
> > + and DMA. Taking a lock means that others cannot access all locked
> dmabufs
> > + until the task that locked the corresponding dmabufs, unlocks all
> the locked
> > + dmabufs.
> > + This locking should be performed before DMA or CPU accesses these
> dmabufs.
> > +
> > + dma_buf_sync_lock(a sync object);
> > +
> > + 3. Unlock a sync object - The task unlocks all dmabufs added in its
> own sync
> > + object. The unlock means that the DMA or CPU accesses to the
dmabufs
> have
> > + been completed so that others may access them.
> > + This unlocking should be performed after DMA or CPU has completed
> accesses
> > + to the dmabufs.
> > +
> > + dma_buf_sync_unlock(a sync object);
> > +
> > + 4. Unregister one or all dmabufs from a sync object - A task
> unregisters
> > + the given dmabufs from the sync object. This means that the task
> dosen't
> > + want to lock the dmabufs.
> > + The unregistering should be performed after DMA or CPU has
completed
> > + accesses to the dmabufs or when dma_buf_sync_lock() is failed.
> > +
> > + dma_buf_sync_put(a sync object, a dmabuf);
> > + dma_buf_sync_put_all(a sync object);
> > +
> > + The described steps may be summarized as:
> > + get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
> > +
> > +This framework includes the following two features.
> > + 1. read (shared) and write (exclusive) locks - A task is required
to
> declare
> > + the access type when the task tries to register a dmabuf;
> > + READ, WRITE, READ DMA, or WRITE DMA.
> > +
> > + The below is example codes,
> > + struct dmabuf_sync *sync;
> > +
> > + sync = dmabuf_sync_init(NULL, "test sync");
> > +
> > + dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
> > + ...
> > +
> > + 2. Mandatory resource releasing - a task cannot hold a lock
> indefinitely.
> > + A task may never try to unlock a buffer after taking a lock to the
> buffer.
> > + In this case, a timer handler to the corresponding sync object is
> called
> > + in five (default) seconds and then the timed-out buffer is unlocked
> by work
> > + queue handler to avoid lockups and to enforce resources of the
> buffer.
> > +
> > +
> > +Access types
> > +------------
> > +
> > +DMA_BUF_ACCESS_R - CPU will access a buffer for read.
> > +DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
> > +DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
> > +DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or write.
> > +
> > +
> > +Generic user interfaces
> > +-----------------------
> > +
> > +And this framework includes fcntl system call[3] as interfaces exported
> > +to user. As you know, user sees a buffer object as a dma-buf file
> descriptor.
> > +So fcntl() call with the file descriptor means to lock some buffer
> region being
> > +managed by the dma-buf object.
> > +
> > +
> > +API set
> > +-------
> > +
> > +bool is_dmabuf_sync_supported(void)
> > + - Check if dmabuf sync is supported or not.
> > +
> > +struct dmabuf_sync *dmabuf_sync_init(const char *name,
> > + struct dmabuf_sync_priv_ops *ops,
> > + void priv*)
> > + - Allocate and initialize a new sync object. The caller can get a
> new
> > + sync object for buffer synchronization. ops is used for device
> driver
> > + to clean up its own sync object. For this, each device driver
> should
> > + implement a free callback. priv is used for device driver to get
> its
> > + device context when free callback is called.
> > +
> > +void dmabuf_sync_fini(struct dmabuf_sync *sync)
> > + - Release all resources to the sync object.
> > +
> > +int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
> > + unsigned int type)
> > + - Get dmabuf sync object. Internally, this function allocates
> > + a dmabuf_sync object and adds a given dmabuf to it, and also takes
> > + a reference to the dmabuf. The caller can tie up multiple dmabufs
> > + into one sync object by calling this function several times.
> > +
> > +void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
> > + - Put dmabuf sync object to a given dmabuf. Internally, this
> function
> > + removes a given dmabuf from a sync object and remove the sync
> object.
> > + At this time, the dmabuf is putted.
> > +
> > +void dmabuf_sync_put_all(struct dmabuf_sync *sync)
> > + - Put dmabuf sync object to dmabufs. Internally, this function
> removes
> > + all dmabufs from a sync object and remove the sync object.
> > + At this time, all dmabufs are putted.
> > +
> > +int dmabuf_sync_lock(struct dmabuf_sync *sync)
> > + - Lock all dmabufs added in a sync object. The caller should call
> this
> > + function prior to CPU or DMA access to the dmabufs so that others
> can
> > + not access the dmabufs. Internally, this function avoids dead lock
> > + issue with ww-mutexes.
> > +
> > +int dmabuf_sync_single_lock(struct dma_buf *dmabuf)
> > + - Lock a dmabuf. The caller should call this
> > + function prior to CPU or DMA access to the dmabuf so that others
> can
> > + not access the dmabuf.
> > +
> > +int dmabuf_sync_unlock(struct dmabuf_sync *sync)
> > + - Unlock all dmabufs added in a sync object. The caller should call
> > + this function after CPU or DMA access to the dmabufs is completed
> so
> > + that others can access the dmabufs.
> > +
> > +void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
> > + - Unlock a dmabuf. The caller should call this function after CPU
> or
> > + DMA access to the dmabuf is completed so that others can access
> > + the dmabuf.
> > +
> > +
> > +Tutorial for device driver
> > +--------------------------
> > +
> > +1. Allocate and Initialize a sync object:
> > + static void xxx_dmabuf_sync_free(void *priv)
> > + {
> > + struct xxx_context *ctx = priv;
> > +
> > + if (!ctx)
> > + return;
> > +
> > + ctx->sync = NULL;
> > + }
> > + ...
> > +
> > + static struct dmabuf_sync_priv_ops driver_specific_ops = {
> > + .free = xxx_dmabuf_sync_free,
> > + };
> > + ...
> > +
> > + struct dmabuf_sync *sync;
> > +
> > + sync = dmabuf_sync_init("test sync", &driver_specific_ops, ctx);
> > + ...
> > +
> > +2. Add a dmabuf to the sync object when setting up dma buffer relevant
> registers:
> > + dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
> > + ...
> > +
> > +3. Lock all dmabufs of the sync object before DMA or CPU accesses the
> dmabufs:
> > + dmabuf_sync_lock(sync);
> > + ...
> > +
> > +4. Now CPU or DMA can access all dmabufs locked in step 3.
> > +
> > +5. Unlock all dmabufs added in a sync object after DMA or CPU access to
> these
> > + dmabufs is completed:
> > + dmabuf_sync_unlock(sync);
> > +
> > + And call the following functions to release all resources,
> > + dmabuf_sync_put_all(sync);
> > + dmabuf_sync_fini(sync);
> > +
> > +
> > +Tutorial for user application
> > +-----------------------------
> > +fcntl system call:
> > +
> > + struct flock filelock;
> > +
> > +1. Lock a dma buf:
> > + filelock.l_type = F_WRLCK or F_RDLCK;
> > +
> > + /* lock entire region to the dma buf. */
> > + filelock.lwhence = SEEK_CUR;
> > + filelock.l_start = 0;
> > + filelock.l_len = 0;
> > +
> > + fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> > + ...
> > + CPU access to the dma buf
> > +
> > +2. Unlock a dma buf:
> > + filelock.l_type = F_UNLCK;
> > +
> > + fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
> > +
> > + close(dmabuf fd) call would also unlock the dma buf. And for more
> > + detail, please refer to [3]
> > +
> > +
> > +select system call:
> > +
> > + fd_set wdfs or rdfs;
> > +
> > + FD_ZERO(&wdfs or &rdfs);
> > + FD_SET(fd, &wdfs or &rdfs);
> > +
> > + select(fd + 1, &rdfs, NULL, NULL, NULL);
> > + or
> > + select(fd + 1, NULL, &wdfs, NULL, NULL);
> > +
> > + Every time select system call is called, a caller will wait for
> > + the completion of DMA or CPU access to a shared buffer if there is
> > + someone accessing the shared buffer; locked the shared buffer.
> > + However, if no anyone then select system call will be returned
> > + at once.
> > +
> > +References:
> > +[1] http://lwn.net/Articles/470339/
> > +[2] https://patchwork.kernel.org/patch/2625361/
> > +[3] http://linux.die.net/man/2/fcntl
> > diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
> > index 5daa259..35e1518 100644
> > --- a/drivers/base/Kconfig
> > +++ b/drivers/base/Kconfig
> > @@ -200,6 +200,13 @@ config DMA_SHARED_BUFFER
> > APIs extension; the file's descriptor can then be passed on to
> other
> > driver.
> >
> > +config DMABUF_SYNC
> > + bool "DMABUF Synchronization Framework"
> > + depends on DMA_SHARED_BUFFER
> > + help
> > + This option enables dmabuf sync framework for buffer
> synchronization between
> > + DMA and DMA, CPU and DMA, and CPU and CPU.
> > +
> > config CMA
> > bool "Contiguous Memory Allocator"
> > depends on HAVE_DMA_CONTIGUOUS && HAVE_MEMBLOCK
> > diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> > index 48029aa..e06a5d7 100644
> > --- a/drivers/base/Makefile
> > +++ b/drivers/base/Makefile
> > @@ -11,6 +11,7 @@ obj-y += power/
> > obj-$(CONFIG_HAS_DMA) += dma-mapping.o
> > obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
> > obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o reservation.o
> > +obj-$(CONFIG_DMABUF_SYNC) += dmabuf-sync.o
> > obj-$(CONFIG_ISA) += isa.o
> > obj-$(CONFIG_FW_LOADER) += firmware_class.o
> > obj-$(CONFIG_NUMA) += node.o
> > diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> > index 6687ba7..4aca57a 100644
> > --- a/drivers/base/dma-buf.c
> > +++ b/drivers/base/dma-buf.c
> > @@ -29,6 +29,7 @@
> > #include <linux/export.h>
> > #include <linux/debugfs.h>
> > #include <linux/seq_file.h>
> > +#include <linux/dmabuf-sync.h>
> >
> > static inline int is_dma_buf_file(struct file *);
> >
> > @@ -56,6 +57,8 @@ static int dma_buf_release(struct inode *inode, struct
> file *file)
> > list_del(&dmabuf->list_node);
> > mutex_unlock(&db_list.lock);
> >
> > + dmabuf_sync_reservation_fini(dmabuf);
> > +
> > kfree(dmabuf);
> > return 0;
> > }
> > @@ -134,6 +137,7 @@ struct dma_buf *dma_buf_export_named(void *priv,
> const struct dma_buf_ops *ops,
> >
> > file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, flags);
> >
> > + dmabuf_sync_reservation_init(dmabuf);
> > dmabuf->file = file;
> >
> > mutex_init(&dmabuf->lock);
> > diff --git a/drivers/base/dmabuf-sync.c b/drivers/base/dmabuf-sync.c
> > new file mode 100644
> > index 0000000..fbe711c
> > --- /dev/null
> > +++ b/drivers/base/dmabuf-sync.c
> > @@ -0,0 +1,678 @@
> > +/*
> > + * Copyright (C) 2013 Samsung Electronics Co.Ltd
> > + * Authors:
> > + * Inki Dae <inki.dae@samsung.com>
> > + *
> > + * 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.
> > + *
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +#include <linux/debugfs.h>
> > +#include <linux/uaccess.h>
> > +
> > +#include <linux/dmabuf-sync.h>
> > +
> > +#define MAX_SYNC_TIMEOUT 5 /* Second. */
> > +
> > +int dmabuf_sync_enabled = 1;
> > +
> > +MODULE_PARM_DESC(enabled, "Check if dmabuf sync is supported or not");
> > +module_param_named(enabled, dmabuf_sync_enabled, int, 0444);
> > +
> > +DEFINE_WW_CLASS(dmabuf_sync_ww_class);
> > +EXPORT_SYMBOL(dmabuf_sync_ww_class);
> > +
> > +static void dmabuf_sync_timeout_worker(struct work_struct *work)
> > +{
> > + struct dmabuf_sync *sync = container_of(work, struct dmabuf_sync,
> work);
> > + struct dmabuf_sync_object *sobj;
> > +
> > + mutex_lock(&sync->lock);
> > +
> > + list_for_each_entry(sobj, &sync->syncs, head) {
>
> You are using the 'sobj->robj' quite a lot. Why not just use a temp
> structure:
>
> struct dmabuf_sync_reservation *rsvp = sobj->robj;
>
> and use that in this function. It would make it easier to read I think.
Ok, will use the temp structure.
>
>
> > + BUG_ON(!sobj->robj);
> > +
> > + mutex_lock(&sobj->robj->lock);
> > +
> > + printk(KERN_WARNING "%s: timeout = 0x%x [type = %d:%d, " \
> > + "refcnt = %d, locked = %d]\n",
> > + sync->name, (u32)sobj->dmabuf,
> > + sobj->robj->accessed_type,
> > + sobj->access_type,
> > +
atomic_read(&sobj->robj->shared_cnt),
> > + sobj->robj->locked);
>
> pr_warn_ratelimited?
Will use pr_warn because the timeout worker handler isn't called so
frequently so the printk storm wouldn't be caused
>
> > +
> > + /* unlock only valid sync object. */
> > + if (!sobj->robj->locked) {
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + if (sobj->robj->polled) {
> > + sobj->robj->poll_event = true;
> > + sobj->robj->polled = false;
> > + wake_up_interruptible(&sobj->robj->poll_wait);
> > + }
> > +
> > + if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > +
> > + ww_mutex_unlock(&sobj->robj->sync_lock);
> > +
> > + mutex_lock(&sobj->robj->lock);
> > + sobj->robj->locked = false;
> > +
> > + if (sobj->access_type & DMA_BUF_ACCESS_R)
> > + printk(KERN_WARNING "%s: r-unlocked = 0x%x\n",
> > + sync->name, (u32)sobj->dmabuf);
> > + else
> > + printk(KERN_WARNING "%s: w-unlocked = 0x%x\n",
> > + sync->name, (u32)sobj->dmabuf);
>
> How about using 'pr_warn'? And in it have:
Ok, will use it.
>
> sobj->access_type & DMA_BUF_ACCESS_R ? "r-" : "w-",
>
> and just have one printk.
>
> Why the (u32) casting? Don't you want %p ?
Right, I should had used %p instead. Will remove the casting and use %p
instead.
>
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > + }
> > +
> > + sync->status = 0;
> > + mutex_unlock(&sync->lock);
> > +
> > + dmabuf_sync_put_all(sync);
> > + dmabuf_sync_fini(sync);
> > +}
> > +
> > +static void dmabuf_sync_lock_timeout(unsigned long arg)
> > +{
> > + struct dmabuf_sync *sync = (struct dmabuf_sync *)arg;
> > +
> > + schedule_work(&sync->work);
> > +}
> > +
> > +static int dmabuf_sync_lock_objs(struct dmabuf_sync *sync,
> > + struct ww_acquire_ctx *ctx)
> > +{
> > + struct dmabuf_sync_object *contended_sobj = NULL;
> > + struct dmabuf_sync_object *res_sobj = NULL;
> > + struct dmabuf_sync_object *sobj = NULL;
> > + int ret;
> > +
> > + if (ctx)
> > + ww_acquire_init(ctx, &dmabuf_sync_ww_class);
> > +
> > +retry:
> > + list_for_each_entry(sobj, &sync->syncs, head) {
> > + if (WARN_ON(!sobj->robj))
> > + continue;
> > +
> > + mutex_lock(&sobj->robj->lock);
> > +
> > + /* Don't lock in case of read and read. */
> > + if (sobj->robj->accessed_type & DMA_BUF_ACCESS_R &&
> > + sobj->access_type & DMA_BUF_ACCESS_R) {
> > + atomic_inc(&sobj->robj->shared_cnt);
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + if (sobj = res_sobj) {
> > + res_sobj = NULL;
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > +
> > + ret = ww_mutex_lock(&sobj->robj->sync_lock, ctx);
> > + if (ret < 0) {
> > + contended_sobj = sobj;
> > +
> > + if (ret = -EDEADLK)
> > + printk(KERN_WARNING"%s: deadlock = 0x%x\n",
> > + sync->name, (u32)sobj->dmabuf);
>
> Again, why (u32) and not %p?
>
> > + goto err;
>
> This looks odd. You jump to err, which jumps back to 'retry'. Won't this
> cause an infinite loop? Perhaps you need to add a retry counter to only
> do this up to five times or so and then give up?
It jumps to err only if ww_mutex_lock returns -EDEADLK. This means that the
lock trying to a given sync object caused dead lock. So all robjs already
locked should be unlocked, and retried to take lock again going to err. So I
think the infinite loop isn't caused.
>
> > + }
> > +
> > + mutex_lock(&sobj->robj->lock);
> > + sobj->robj->locked = true;
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > + }
> > +
> > + if (ctx)
> > + ww_acquire_done(ctx);
> > +
> > + init_timer(&sync->timer);
> > +
> > + sync->timer.data = (unsigned long)sync;
> > + sync->timer.function = dmabuf_sync_lock_timeout;
> > + sync->timer.expires = jiffies + (HZ * MAX_SYNC_TIMEOUT);
> > +
> > + add_timer(&sync->timer);
> > +
> > + return 0;
> > +
> > +err:
> > + list_for_each_entry_continue_reverse(sobj, &sync->syncs, head) {
> > + mutex_lock(&sobj->robj->lock);
> > +
> > + /* Don't need to unlock in case of read and read. */
> > + if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + ww_mutex_unlock(&sobj->robj->sync_lock);
> > + sobj->robj->locked = false;
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > + }
> > +
> > + if (res_sobj) {
> > + mutex_lock(&res_sobj->robj->lock);
> > +
> > + if (!atomic_add_unless(&res_sobj->robj->shared_cnt, -1, 1))
> {
> > + ww_mutex_unlock(&res_sobj->robj->sync_lock);
> > + res_sobj->robj->locked = false;
> > + }
> > +
> > + mutex_unlock(&res_sobj->robj->lock);
> > + }
> > +
> > + if (ret = -EDEADLK) {
> > + ww_mutex_lock_slow(&contended_sobj->robj->sync_lock, ctx);
> > + res_sobj = contended_sobj;
> > +
> > + goto retry;
> > + }
> > +
> > + if (ctx)
> > + ww_acquire_fini(ctx);
> > +
> > + return ret;
> > +}
> > +
> > +static void dmabuf_sync_unlock_objs(struct dmabuf_sync *sync,
> > + struct ww_acquire_ctx *ctx)
> > +{
> > + struct dmabuf_sync_object *sobj;
> > +
> > + if (list_empty(&sync->syncs))
> > + return;
> > +
> > + mutex_lock(&sync->lock);
> > +
> > + list_for_each_entry(sobj, &sync->syncs, head) {
> > + mutex_lock(&sobj->robj->lock);
> > +
> > + if (sobj->robj->polled) {
> > + sobj->robj->poll_event = true;
> > + sobj->robj->polled = false;
> > + wake_up_interruptible(&sobj->robj->poll_wait);
> > + }
> > +
> > + if (atomic_add_unless(&sobj->robj->shared_cnt, -1, 1)) {
> > + mutex_unlock(&sobj->robj->lock);
> > + continue;
> > + }
> > +
> > + mutex_unlock(&sobj->robj->lock);
> > +
> > + ww_mutex_unlock(&sobj->robj->sync_lock);
> > +
> > + mutex_lock(&sobj->robj->lock);
> > + sobj->robj->locked = false;
> > + mutex_unlock(&sobj->robj->lock);
> > + }
> > +
> > + mutex_unlock(&sync->lock);
> > +
> > + if (ctx)
> > + ww_acquire_fini(ctx);
> > +
> > + del_timer(&sync->timer);
> > +}
> > +
> > +/**
> > + * is_dmabuf_sync_supported - Check if dmabuf sync is supported or not.
> > + */
> > +bool is_dmabuf_sync_supported(void)
> > +{
> > + return dmabuf_sync_enabled = 1;
> > +}
> > +EXPORT_SYMBOL(is_dmabuf_sync_supported);
>
> _GPL ?
>
> I would also prefix it with 'dmabuf_is_sync_supported' just to make
> all of the libraries call start with 'dmabuf'
>
Seems better. Will change it to dmabuf_is_sync_supported, and use
EXPORT_SYMBOL_GPL.
> > +
> > +/**
> > + * dmabuf_sync_init - Allocate and initialize a dmabuf sync.
> > + *
> > + * @priv: A device private data.
> > + * @name: A sync object name.
> > + *
> > + * This function should be called when a device context or an event
> > + * context such as a page flip event is created. And the created
> > + * dmabuf_sync object should be set to the context.
> > + * The caller can get a new sync object for buffer synchronization
> > + * through this function.
> > + */
> > +struct dmabuf_sync *dmabuf_sync_init(const char *name,
> > + struct dmabuf_sync_priv_ops *ops,
> > + void *priv)
> > +{
> > + struct dmabuf_sync *sync;
> > +
> > + sync = kzalloc(sizeof(*sync), GFP_KERNEL);
> > + if (!sync)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + strncpy(sync->name, name, ARRAY_SIZE(sync->name) - 1);
> > +
>
> That is odd usage of an ARRAY_SIZE, but I can see how you can use it.
> I would say you should just do a #define for the 64 line and use that
> instead.
>
Ok, will use the macro instead.
> > + sync->ops = ops;
> > + sync->priv = priv;
> > + INIT_LIST_HEAD(&sync->syncs);
> > + mutex_init(&sync->lock);
> > + INIT_WORK(&sync->work, dmabuf_sync_timeout_worker);
> > +
> > + return sync;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_init);
>
> _GPL ?
Sure.
> > +
> > +/**
> > + * dmabuf_sync_fini - Release a given dmabuf sync.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * This function should be called if some operation is failed after
> > + * dmabuf_sync_init call to release relevant resources, and after
> > + * dmabuf_sync_unlock function is called.
> > + */
> > +void dmabuf_sync_fini(struct dmabuf_sync *sync)
> > +{
> > + if (WARN_ON(!sync))
> > + return;
> > +
> > + if (sync->ops && sync->ops->free)
> > + sync->ops->free(sync->priv);
> > +
>
> No need to cancel the sync->work in case that is still
> running?
Right, the locks to all buffers should be canceled if dmabuf_sync_fini was
called without unlock call.
>
> > + kfree(sync);
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_fini);
>
> _GPL ?
> > +
> > +/*
> > + * dmabuf_sync_get_obj - Add a given object to syncs list.
>
> sync's list I think?
>
Ok, seems better.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + * @dmabuf: An object to dma_buf structure.
> > + * @type: A access type to a dma buf.
> > + * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
> > + * others for read access. On the other hand, the DMA_BUF_ACCESS_W
> > + * means that this dmabuf couldn't be accessed by others but would be
> > + * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA
> can be
> > + * combined.
>
> Should this be an enum?
> > + *
> > + * This function creates and initializes a new dmabuf sync object and
> it adds
> > + * the dmabuf sync object to syncs list to track and manage all
dmabufs.
> > + */
> > +static int dmabuf_sync_get_obj(struct dmabuf_sync *sync, struct dma_buf
> *dmabuf,
> > + unsigned int type)
>
> enum for 'type'?
> > +{
> > + struct dmabuf_sync_object *sobj;
> > +
> > + if (!dmabuf->sync) {
> > + WARN_ON(1);
> > + return -EFAULT;
> > + }
> > +
> > + if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type))
> > + return -EINVAL;
> > +
> > + if ((type & DMA_BUF_ACCESS_RW) = DMA_BUF_ACCESS_RW)
> > + type &= ~DMA_BUF_ACCESS_R;
>
> Ah, that is why you are not using an enum.
>
> > +
> > + sobj = kzalloc(sizeof(*sobj), GFP_KERNEL);
> > + if (!sobj) {
> > + WARN_ON(1);
>
> I think you can skip that WARN_ON. Handling an -ENOMEM should be
> something fairly easy to handle by the calleer.
>
Ok, will remove it.
> > + return -ENOMEM;
> > + }
> > +
> > + get_dma_buf(dmabuf);
> > +
> > + sobj->dmabuf = dmabuf;
> > + sobj->robj = dmabuf->sync;
> > + sobj->access_type = type;
> > +
> > + mutex_lock(&sync->lock);
> > + list_add_tail(&sobj->head, &sync->syncs);
> > + mutex_unlock(&sync->lock);
> > +
> > + return 0;
> > +}
> > +
> > +/*
> > + * dmabuf_sync_put_obj - Release a given sync object.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * This function should be called if some operation is failed after
>
> s/is//
Sure.
> > + * dmabuf_sync_get_obj call to release a given sync object.
> > + */
> > +static void dmabuf_sync_put_obj(struct dmabuf_sync *sync,
> > + struct dma_buf *dmabuf)
> > +{
> > + struct dmabuf_sync_object *sobj;
> > +
> > + mutex_lock(&sync->lock);
> > +
> > + list_for_each_entry(sobj, &sync->syncs, head) {
> > + if (sobj->dmabuf != dmabuf)
> > + continue;
> > +
> > + dma_buf_put(sobj->dmabuf);
> > +
> > + list_del_init(&sobj->head);
> > + kfree(sobj);
> > + break;
> > + }
> > +
> > + if (list_empty(&sync->syncs))
> > + sync->status = 0;
> > +
> > + mutex_unlock(&sync->lock);
> > +}
> > +
> > +/*
> > + * dmabuf_sync_put_objs - Release all sync objects of dmabuf_sync.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * This function should be called if some operation is failed after
>
> s/is//
Sure.
>
> > + * dmabuf_sync_get_obj call to release all sync objects.
> > + */
> > +static void dmabuf_sync_put_objs(struct dmabuf_sync *sync)
> > +{
> > + struct dmabuf_sync_object *sobj, *next;
> > +
> > + mutex_lock(&sync->lock);
> > +
> > + list_for_each_entry_safe(sobj, next, &sync->syncs, head) {
> > + dma_buf_put(sobj->dmabuf);
> > +
> > + list_del_init(&sobj->head);
> > + kfree(sobj);
> > + }
> > +
> > + mutex_unlock(&sync->lock);
> > +
> > + sync->status = 0;
> > +}
> > +
> > +/**
> > + * dmabuf_sync_lock - lock all dmabufs added to syncs list.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * The caller should call this function prior to CPU or DMA access to
> > + * the dmabufs so that others can not access the dmabufs.
> > + * Internally, this function avoids dead lock issue with ww-mutex.
> > + */
> > +int dmabuf_sync_lock(struct dmabuf_sync *sync)
> > +{
> > + int ret;
> > +
> > + if (!sync) {
> > + WARN_ON(1);
> > + return -EFAULT;
> > + }
> > +
> > + if (list_empty(&sync->syncs))
> > + return -EINVAL;
> > +
> > + if (sync->status != DMABUF_SYNC_GOT)
> > + return -EINVAL;
> > +
> > + ret = dmabuf_sync_lock_objs(sync, &sync->ctx);
> > + if (ret < 0) {
> > + WARN_ON(1);
>
> Perhaps also include the ret value in the WARN?
>
> > + return ret;
> > + }
> > +
> > + sync->status = DMABUF_SYNC_LOCKED;
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_lock);
>
> I think you know what I am going to say.
> > +
> > +/**
> > + * dmabuf_sync_unlock - unlock all objects added to syncs list.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * The caller should call this function after CPU or DMA access to
> > + * the dmabufs is completed so that others can access the dmabufs.
> > + */
> > +int dmabuf_sync_unlock(struct dmabuf_sync *sync)
> > +{
> > + if (!sync) {
> > + WARN_ON(1);
> > + return -EFAULT;
> > + }
> > +
> > + /* If current dmabuf sync object wasn't reserved then just return.
> */
> > + if (sync->status != DMABUF_SYNC_LOCKED)
> > + return -EAGAIN;
> > +
> > + dmabuf_sync_unlock_objs(sync, &sync->ctx);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_unlock);
> > +
> > +/**
> > + * dmabuf_sync_single_lock - lock a dma buf.
> > + *
> > + * @dmabuf: A dma buf object that tries to lock.
> > + * @type: A access type to a dma buf.
> > + * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
> > + * others for read access. On the other hand, the DMA_BUF_ACCESS_W
> > + * means that this dmabuf couldn't be accessed by others but would be
> > + * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA
> can
> > + * be combined with other.
> > + * @wait: Indicate whether caller is blocked or not.
> > + * true means that caller will be blocked, and false means that this
> > + * function will return -EAGAIN if this caller can't take the lock
> > + * right now.
> > + *
> > + * The caller should call this function prior to CPU or DMA access to
> the dmabuf
> > + * so that others cannot access the dmabuf.
> > + */
> > +int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
> > + bool wait)
> > +{
> > + struct dmabuf_sync_reservation *robj;
> > +
> > + if (!dmabuf->sync) {
> > + WARN_ON(1);
> > + return -EFAULT;
> > + }
> > +
> > + if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type)) {
> > + WARN_ON(1);
> > + return -EINVAL;
> > + }
> > +
> > + get_dma_buf(dmabuf);
> > + robj = dmabuf->sync;
> > +
> > + mutex_lock(&robj->lock);
> > +
> > + /* Don't lock in case of read and read. */
> > + if (robj->accessed_type & DMA_BUF_ACCESS_R && type &
> DMA_BUF_ACCESS_R) {
> > + atomic_inc(&robj->shared_cnt);
> > + mutex_unlock(&robj->lock);
> > + return 0;
> > + }
> > +
> > + /*
> > + * In case of F_SETLK, just return -EAGAIN if this dmabuf has
> already
> > + * been locked.
> > + */
> > + if (!wait && robj->locked) {
> > + mutex_unlock(&robj->lock);
> > + dma_buf_put(dmabuf);
> > + return -EAGAIN;
> > + }
> > +
> > + mutex_unlock(&robj->lock);
> > +
> > + mutex_lock(&robj->sync_lock.base);
> > +
> > + mutex_lock(&robj->lock);
> > + robj->locked = true;
> > + mutex_unlock(&robj->lock);
>
> Are you missing an mutex_unlock on &robj->sync_lock.base?
> Oh wait, that is the purpose of this code. You might want
> to put a nice comment right above that and say: "Unlocked
> by dmabuf_sync_single_unlock"
Will add the comment.
>
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_single_lock);
> > +
> > +/**
> > + * dmabuf_sync_single_unlock - unlock a dma buf.
> > + *
> > + * @dmabuf: A dma buf object that tries to unlock.
> > + *
> > + * The caller should call this function after CPU or DMA access to
> > + * the dmabuf is completed so that others can access the dmabuf.
> > + */
> > +void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
> > +{
> > + struct dmabuf_sync_reservation *robj;
> > +
> > + if (!dmabuf->sync) {
> > + WARN_ON(1);
> > + return;
> > + }
> > +
> > + robj = dmabuf->sync;
> > +
> > + mutex_lock(&robj->lock);
> > +
> > + if (robj->polled) {
> > + robj->poll_event = true;
> > + robj->polled = false;
> > + wake_up_interruptible(&robj->poll_wait);
> > + }
> > +
> > + if (atomic_add_unless(&robj->shared_cnt, -1 , 1)) {
> > + mutex_unlock(&robj->lock);
> > + dma_buf_put(dmabuf);
> > + return;
> > + }
> > +
> > + mutex_unlock(&robj->lock);
> > +
> > + mutex_unlock(&robj->sync_lock.base);
> > +
> > + mutex_lock(&robj->lock);
> > + robj->locked = false;
> > + mutex_unlock(&robj->lock);
> > +
> > + dma_buf_put(dmabuf);
> > +
> > + return;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_single_unlock);
> > +
> > +/**
> > + * dmabuf_sync_get - Get dmabuf sync object.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + * @sync_buf: A dmabuf object to be synchronized with others.
> > + * @type: A access type to a dma buf.
> > + * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
> > + * others for read access. On the other hand, the DMA_BUF_ACCESS_W
> > + * means that this dmabuf couldn't be accessed by others but would be
> > + * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA
> can
> > + * be combined with other.
> > + *
> > + * This function should be called after dmabuf_sync_init function is
> called.
> > + * The caller can tie up multiple dmabufs into one sync object by
> calling this
> > + * function several times. Internally, this function allocates
> > + * a dmabuf_sync_object and adds a given dmabuf to it, and also takes
> > + * a reference to a dmabuf.
> > + */
> > +int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf, unsigned
> int type)
> > +{
> > + int ret;
> > +
> > + if (!sync || !sync_buf) {
> > + WARN_ON(1);
> > + return -EFAULT;
> > + }
> > +
> > + ret = dmabuf_sync_get_obj(sync, sync_buf, type);
> > + if (ret < 0) {
> > + WARN_ON(1);
> > + return ret;
> > + }
> > +
> > + sync->status = DMABUF_SYNC_GOT;
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_get);
> > +
> > +/**
> > + * dmabuf_sync_put - Put dmabuf sync object to a given dmabuf.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + * @dmabuf: An dmabuf object.
> > + *
> > + * This function should be called if some operation is failed after
> > + * dmabuf_sync_get function is called to release the dmabuf, or
> > + * dmabuf_sync_unlock function is called. Internally, this function
> > + * removes a given dmabuf from a sync object and remove the sync
object.
> > + * At this time, the dmabuf is putted.
> > + */
> > +void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
> > +{
> > + if (!sync || !dmabuf) {
> > + WARN_ON(1);
> > + return;
> > + }
> > +
> > + if (list_empty(&sync->syncs))
> > + return;
> > +
> > + dmabuf_sync_put_obj(sync, dmabuf);
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_put);
> > +
> > +/**
> > + * dmabuf_sync_put_all - Put dmabuf sync object to dmabufs.
> > + *
> > + * @sync: An object to dmabuf_sync structure.
> > + *
> > + * This function should be called if some operation is failed after
> > + * dmabuf_sync_get function is called to release all sync objects, or
> > + * dmabuf_sync_unlock function is called. Internally, this function
> > + * removes dmabufs from a sync object and remove the sync object.
> > + * At this time, all dmabufs are putted.
> > + */
> > +void dmabuf_sync_put_all(struct dmabuf_sync *sync)
> > +{
> > + if (!sync) {
> > + WARN_ON(1);
> > + return;
> > + }
> > +
> > + if (list_empty(&sync->syncs))
> > + return;
> > +
> > + dmabuf_sync_put_objs(sync);
> > +}
> > +EXPORT_SYMBOL(dmabuf_sync_put_all);
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index dfac5ed..0109673 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -115,6 +115,7 @@ struct dma_buf_ops {
> > * @exp_name: name of the exporter; useful for debugging.
> > * @list_node: node for dma_buf accounting and debugging.
> > * @priv: exporter specific private data for this buffer object.
> > + * @sync: sync object linked to this dma-buf
> > */
> > struct dma_buf {
> > size_t size;
> > @@ -128,6 +129,7 @@ struct dma_buf {
> > const char *exp_name;
> > struct list_head list_node;
> > void *priv;
> > + void *sync;
> > };
> >
> > /**
> > @@ -148,6 +150,20 @@ struct dma_buf_attachment {
> > void *priv;
> > };
> >
> > +#define DMA_BUF_ACCESS_R 0x1
> > +#define DMA_BUF_ACCESS_W 0x2
> > +#define DMA_BUF_ACCESS_DMA 0x4
> > +#define DMA_BUF_ACCESS_RW (DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_W)
> > +#define DMA_BUF_ACCESS_DMA_R (DMA_BUF_ACCESS_R |
DMA_BUF_ACCESS_DMA)
> > +#define DMA_BUF_ACCESS_DMA_W (DMA_BUF_ACCESS_W |
DMA_BUF_ACCESS_DMA)
> > +#define DMA_BUF_ACCESS_DMA_RW (DMA_BUF_ACCESS_DMA_R |
> DMA_BUF_ACCESS_DMA_W)
> > +#define IS_VALID_DMA_BUF_ACCESS_TYPE(t) (t = DMA_BUF_ACCESS_R || \
> > + t = DMA_BUF_ACCESS_W || \
> > + t = DMA_BUF_ACCESS_DMA_R || \
> > + t = DMA_BUF_ACCESS_DMA_W || \
> > + t = DMA_BUF_ACCESS_RW || \
> > + t = DMA_BUF_ACCESS_DMA_RW)
> > +
> > /**
> > * get_dma_buf - convenience wrapper for get_file.
> > * @dmabuf: [in] pointer to dma_buf
> > diff --git a/include/linux/dmabuf-sync.h b/include/linux/dmabuf-sync.h
> > new file mode 100644
> > index 0000000..9a3afc4
> > --- /dev/null
> > +++ b/include/linux/dmabuf-sync.h
> > @@ -0,0 +1,190 @@
> > +/*
> > + * Copyright (C) 2013 Samsung Electronics Co.Ltd
> > + * Authors:
> > + * Inki Dae <inki.dae@samsung.com>
> > + *
> > + * 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.
> > + *
> > + */
> > +
> > +#include <linux/mutex.h>
> > +#include <linux/sched.h>
> > +#include <linux/dma-buf.h>
> > +
> > +enum dmabuf_sync_status {
> > + DMABUF_SYNC_GOT = 1,
> > + DMABUF_SYNC_LOCKED,
> > +};
> > +
>
> No comment about this structure?
Will add comments.
>
> > +struct dmabuf_sync_reservation {
> > + struct ww_mutex sync_lock;
> > + struct mutex lock;
> > + wait_queue_head_t poll_wait;
> > + unsigned int poll_event;
> > + unsigned int polled;
> > + atomic_t shared_cnt;
> > + unsigned int accessed_type;
> > + unsigned int locked;
> > +};
> > +
> > +/*
> > + * A structure for dmabuf_sync_object.
> > + *
> > + * @head: A list head to be added to syncs list.
> > + * @robj: A reservation_object object.
> > + * @dma_buf: A dma_buf object.
> > + * @access_type: Indicate how a current task tries to access
> > + * a given buffer.
>
> Huh? What values are expected then? Is there some #define or enum
> for that?
>
Right, there are definitions for that. Will add more comments.
> > + */
> > +struct dmabuf_sync_object {
> > + struct list_head head;
> > + struct dmabuf_sync_reservation *robj;
> > + struct dma_buf *dmabuf;
> > + unsigned int access_type;
> > +};
> > +
> > +struct dmabuf_sync_priv_ops {
> > + void (*free)(void *priv);
> > +};
> > +
> > +/*
> > + * A structure for dmabuf_sync.
> > + *
> > + * @syncs: A list head to sync object and this is global to system.
> > + * @list: A list entry used as committed list node
> > + * @lock: A mutex lock to current sync object.
>
> You should say for which specific operations this mutex is needed.
> For everything? Or just for list operations.
Ok, will add more comments.
>
> > + * @ctx: A current context for ww mutex.
> > + * @work: A work struct to release resources at timeout.
> > + * @priv: A private data.
> > + * @name: A string to dmabuf sync owner.
> > + * @timer: A timer list to avoid lockup and release resources.
> > + * @status: Indicate current status (DMABUF_SYNC_GOT or
> DMABUF_SYNC_LOCKED).
> > + */
> > +struct dmabuf_sync {
> > + struct list_head syncs;
> > + struct list_head list;
> > + struct mutex lock;
> > + struct ww_acquire_ctx ctx;
> > + struct work_struct work;
> > + void *priv;
> > + struct dmabuf_sync_priv_ops *ops;
> > + char name[64];
>
> Perhaps a #define for the size?
Ok, will use macro instead.
>
> > + struct timer_list timer;
> > + unsigned int status;
> > +};
> > +
> > +#ifdef CONFIG_DMABUF_SYNC
> > +
> > +extern struct ww_class dmabuf_sync_ww_class;
> > +
> > +static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf)
> > +{
> > + struct dmabuf_sync_reservation *obj;
> > +
> > + obj = kzalloc(sizeof(*obj), GFP_KERNEL);
> > + if (!obj)
> > + return;
> > +
> > + dmabuf->sync = obj;
> > +
> > + ww_mutex_init(&obj->sync_lock, &dmabuf_sync_ww_class);
> > +
> > + mutex_init(&obj->lock);
> > + atomic_set(&obj->shared_cnt, 1);
> > +
> > + init_waitqueue_head(&obj->poll_wait);
> > +}
> > +
> > +static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf)
> > +{
> > + struct dmabuf_sync_reservation *obj;
> > +
> > + if (!dmabuf->sync)
> > + return;
> > +
> > + obj = dmabuf->sync;
> > +
> > + ww_mutex_destroy(&obj->sync_lock);
> > +
> > + kfree(obj);
> > +}
> > +
> > +extern bool is_dmabuf_sync_supported(void);
> > +
> > +extern struct dmabuf_sync *dmabuf_sync_init(const char *name,
> > + struct dmabuf_sync_priv_ops *ops,
> > + void *priv);
> > +
> > +extern void dmabuf_sync_fini(struct dmabuf_sync *sync);
> > +
> > +extern int dmabuf_sync_lock(struct dmabuf_sync *sync);
> > +
> > +extern int dmabuf_sync_unlock(struct dmabuf_sync *sync);
> > +
> > +int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
> > + bool wait);
> > +
> > +void dmabuf_sync_single_unlock(struct dma_buf *dmabuf);
> > +
> > +extern int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
> > + unsigned int type);
> > +
> > +extern void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf
> *dmabuf);
> > +
> > +extern void dmabuf_sync_put_all(struct dmabuf_sync *sync);
> > +
> > +#else
> > +
> > +static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf)
> { }
> > +
> > +static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf)
> { }
> > +
> > +static inline bool is_dmabuf_sync_supported(void) { return false; }
> > +
> > +static inline struct dmabuf_sync *dmabuf_sync_init(const char *name,
> > + struct dmabuf_sync_priv_ops *ops,
> > + void *priv)
> > +{
> > + return ERR_PTR(0);
> > +}
> > +
> > +static inline void dmabuf_sync_fini(struct dmabuf_sync *sync) { }
> > +
> > +static inline int dmabuf_sync_lock(struct dmabuf_sync *sync)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline int dmabuf_sync_unlock(struct dmabuf_sync *sync)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline int dmabuf_sync_single_lock(struct dma_buf *dmabuf,
> > + unsigned int type,
> > + bool wait)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
> > +{
> > + return;
> > +}
> > +
> > +static inline int dmabuf_sync_get(struct dmabuf_sync *sync,
> > + void *sync_buf,
> > + unsigned int type)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline void dmabuf_sync_put(struct dmabuf_sync *sync,
> > + struct dma_buf *dmabuf) { }
> > +
> > +static inline void dmabuf_sync_put_all(struct dmabuf_sync *sync) { }
> > +
> > +#endif
> > --
> > 1.7.5.4
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] fbmem: move EXPORT_SYMBOL annotation next to symbol declarations
From: Daniel Mack @ 2013-08-21 9:20 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1376575975-13215-1-git-send-email-zonque@gmail.com>
On 15.08.2013 16:12, Daniel Mack wrote:
> Just a cosmetic thing to bring that file in line with others in the
> tree.
Jean-Christophe, in case you're fine with that patch, could you queue it
up for the next merge window?
Thanks,
Daniel
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
> drivers/video/fbmem.c | 29 +++++++++++++----------------
> 1 file changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
> index 36e1fe2..dacaf74 100644
> --- a/drivers/video/fbmem.c
> +++ b/drivers/video/fbmem.c
> @@ -43,8 +43,12 @@
> #define FBPIXMAPSIZE (1024 * 8)
>
> static DEFINE_MUTEX(registration_lock);
> +
> struct fb_info *registered_fb[FB_MAX] __read_mostly;
> +EXPORT_SYMBOL(registered_fb);
> +
> int num_registered_fb __read_mostly;
> +EXPORT_SYMBOL(num_registered_fb);
>
> static struct fb_info *get_fb_info(unsigned int idx)
> {
> @@ -182,6 +186,7 @@ char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size
>
> return addr;
> }
> +EXPORT_SYMBOL(fb_get_buffer_offset);
>
> #ifdef CONFIG_LOGO
>
> @@ -669,6 +674,7 @@ int fb_show_logo(struct fb_info *info, int rotate)
> int fb_prepare_logo(struct fb_info *info, int rotate) { return 0; }
> int fb_show_logo(struct fb_info *info, int rotate) { return 0; }
> #endif /* CONFIG_LOGO */
> +EXPORT_SYMBOL(fb_show_logo);
>
> static void *fb_seq_start(struct seq_file *m, loff_t *pos)
> {
> @@ -909,6 +915,7 @@ fb_pan_display(struct fb_info *info, struct fb_var_screeninfo *var)
> info->var.vmode &= ~FB_VMODE_YWRAP;
> return 0;
> }
> +EXPORT_SYMBOL(fb_pan_display);
>
> static int fb_check_caps(struct fb_info *info, struct fb_var_screeninfo *var,
> u32 activate)
> @@ -1042,6 +1049,7 @@ fb_set_var(struct fb_info *info, struct fb_var_screeninfo *var)
> done:
> return ret;
> }
> +EXPORT_SYMBOL(fb_set_var);
>
> int
> fb_blank(struct fb_info *info, int blank)
> @@ -1073,6 +1081,7 @@ fb_blank(struct fb_info *info, int blank)
>
> return ret;
> }
> +EXPORT_SYMBOL(fb_blank);
>
> static long do_fb_ioctl(struct fb_info *info, unsigned int cmd,
> unsigned long arg)
> @@ -1745,6 +1754,7 @@ register_framebuffer(struct fb_info *fb_info)
>
> return ret;
> }
> +EXPORT_SYMBOL(register_framebuffer);
>
> /**
> * unregister_framebuffer - releases a frame buffer device
> @@ -1773,6 +1783,7 @@ unregister_framebuffer(struct fb_info *fb_info)
>
> return ret;
> }
> +EXPORT_SYMBOL(unregister_framebuffer);
>
> /**
> * fb_set_suspend - low level driver signals suspend
> @@ -1796,6 +1807,7 @@ void fb_set_suspend(struct fb_info *info, int state)
> fb_notifier_call_chain(FB_EVENT_RESUME, &event);
> }
> }
> +EXPORT_SYMBOL(fb_set_suspend);
>
> /**
> * fbmem_init - init frame buffer subsystem
> @@ -1912,6 +1924,7 @@ int fb_get_options(const char *name, char **option)
>
> return retval;
> }
> +EXPORT_SYMBOL(fb_get_options);
>
> #ifndef MODULE
> /**
> @@ -1959,20 +1972,4 @@ static int __init video_setup(char *options)
> __setup("video=", video_setup);
> #endif
>
> - /*
> - * Visible symbols for modules
> - */
> -
> -EXPORT_SYMBOL(register_framebuffer);
> -EXPORT_SYMBOL(unregister_framebuffer);
> -EXPORT_SYMBOL(num_registered_fb);
> -EXPORT_SYMBOL(registered_fb);
> -EXPORT_SYMBOL(fb_show_logo);
> -EXPORT_SYMBOL(fb_set_var);
> -EXPORT_SYMBOL(fb_blank);
> -EXPORT_SYMBOL(fb_pan_display);
> -EXPORT_SYMBOL(fb_get_buffer_offset);
> -EXPORT_SYMBOL(fb_set_suspend);
> -EXPORT_SYMBOL(fb_get_options);
> -
> MODULE_LICENSE("GPL");
>
^ permalink raw reply
* [PATCH v7 0/2] Introduce buffer synchronization framework
From: Inki Dae @ 2013-08-21 10:33 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-arm-kernel, linux-media,
linaro-kernel
Cc: maarten.lankhorst, sumit.semwal, kyungmin.park, myungjoo.ham,
Inki Dae
Hi all,
This patch set introduces a buffer synchronization framework based
on DMA BUF[1] and based on ww-mutexes[2] for lock mechanism, and
has been rebased on linux-3.11-rc6.
The purpose of this framework is to provide not only buffer access
control to CPU and CPU, and CPU and DMA, and DMA and DMA but also
easy-to-use interfaces for device drivers and user application.
In addtion, this patch set suggests a way for enhancing performance.
Changelog v7:
Fix things pointed out by Konrad Rzeszutek Wilk,
- Use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL.
- Make sure to unlock and unreference all dmabuf objects
when dmabuf_sync_fini() is called.
- Add more comments.
- Code cleanups.
Changelog v6:
- Fix sync lock to multiple reads.
- Add select system call support.
. Wake up poll_wait when a dmabuf is unlocked.
- Remove unnecessary the use of mutex lock.
- Add private backend ops callbacks.
. This ops has one callback for device drivers to clean up their
sync object resource when the sync object is freed. For this,
device drivers should implement the free callback properly.
- Update document file.
Changelog v5:
- Rmove a dependence on reservation_object: the reservation_object is used
to hook up to ttm and dma-buf for easy sharing of reservations across
devices. However, the dmabuf sync can be used for all dma devices; v4l2
and drm based drivers, so doesn't need the reservation_object anymore.
With regared to this, it adds 'void *sync' to dma_buf structure.
- All patches are rebased on mainline, Linux v3.10.
Changelog v4:
- Add user side interface for buffer synchronization mechanism and update
descriptions related to the user side interface.
Changelog v3:
- remove cache operation relevant codes and update document file.
Changelog v2:
- use atomic_add_unless to avoid potential bug.
- add a macro for checking valid access type.
- code clean.
For generic user mode interface, we have used fcntl and select system
call[3]. As you know, user application sees a buffer object as a dma-buf
file descriptor. So fcntl() call with the file descriptor means to lock
some buffer region being managed by the dma-buf object. And select() call
means to wait for the completion of CPU or DMA access to the dma-buf
without locking. For more detail, you can refer to the dma-buf-sync.txt
in Documentation/
There are some cases we should use this buffer synchronization framework.
One of which is to primarily enhance GPU rendering performance on Tizen
platform in case of 3d app with compositing mode that 3d app draws
something in off-screen buffer, and Web app.
In case of 3d app with compositing mode which is not a full screen mode,
the app calls glFlush to submit 3d commands to GPU driver instead of
glFinish for more performance. The reason we call glFlush is that glFinish
blocks caller's task until the execution of the 2d commands is completed.
Thus, that makes GPU and CPU more idle. As result, 3d rendering performance
with glFinish is quite lower than glFlush. However, the use of glFlush has
one issue that the a buffer shared with GPU could be broken when CPU
accesses the buffer at once after glFlush because CPU cannot be aware of
the completion of GPU access to the buffer. Of course, the app can be aware
of that time using eglWaitGL but this function is valid only in case of the
same process.
The below summarizes how app's window is displayed on Tizen platform:
1. X client requests a window buffer to Xorg.
2. X client draws something in the window buffer using CPU.
3. X client requests SWAP to Xorg.
4. Xorg notifies a damage event to Composite Manager.
5. Composite Manager gets the window buffer (front buffer) through
DRI2GetBuffers.
6. Composite Manager composes the window buffer and its own back buffer
using GPU. At this time, eglSwapBuffers is called: internally, 3d
commands are flushed to gpu driver.
7. Composite Manager requests SWAP to Xorg.
8. Xorg performs drm page flip. At this time, the window buffer is
displayed on screen.
Web app based on HTML5 also has the same issue. Web browser and its web app
are different process. The web app draws something in its own pixmap buffer,
and then the web browser gets a window buffer from Xorg, and then composites
the pixmap buffer with the window buffer. And finally, page flip.
Thus, in such cases, a shared buffer could be broken as one process draws
something in pixmap buffer using CPU, when other process composites the
pixmap buffer with window buffer using GPU without any locking mechanism.
That is why we need user land locking interface, fcntl system call.
And last one is a deferred page flip issue. This issue is that a window
buffer rendered can be displayed on screen in about 32ms in worst case:
assume that the gpu rendering is completed within 16ms.
That can be incurred when compositing a pixmap buffer with a window buffer
using GPU and when vsync is just started. At this time, Xorg waits for
a vblank event to get a window buffer so 3d rendering will be delayed
up to about 16ms. As a result, the window buffer would be displayed in
about two vsyncs (about 32ms) and in turn, that would show slow
responsiveness.
For this, we could enhance the responsiveness with locking
mechanism: skipping one vblank wait. I guess in the similar reason,
Android, Chrome OS, and other platforms are using their own locking
mechanisms; Android sync driver, KDS, and DMA fence.
The below shows the deferred page flip issue in worst case,
|------------ <- vsync signal
|<------ DRI2GetBuffers
|
|
|
|------------ <- vsync signal
|<------ Request gpu rendering
time |
|
|<------ Request page flip (deferred)
|------------ <- vsync signal
|<------ Displayed on screen
|
|
|
|------------ <- vsync signal
Thanks,
Inki Dae
References:
[1] http://lwn.net/Articles/470339/
[2] https://patchwork.kernel.org/patch/2625361/
[3] http://linux.die.net/man/2/fcntl
Inki Dae (2):
dmabuf-sync: Add a buffer synchronization framework
dma-buf: Add user interfaces for dmabuf sync support
Documentation/dma-buf-sync.txt | 286 ++++++++++++++++
drivers/base/Kconfig | 7 +
drivers/base/Makefile | 1 +
drivers/base/dma-buf.c | 85 +++++
drivers/base/dmabuf-sync.c | 706 ++++++++++++++++++++++++++++++++++++++++
include/linux/dma-buf.h | 16 +
include/linux/dmabuf-sync.h | 236 ++++++++++++++
7 files changed, 1337 insertions(+), 0 deletions(-)
create mode 100644 Documentation/dma-buf-sync.txt
create mode 100644 drivers/base/dmabuf-sync.c
create mode 100644 include/linux/dmabuf-sync.h
--
1.7.5.4
^ permalink raw reply
* [PATCH v7 1/2] dmabuf-sync: Add a buffer synchronization framework
From: Inki Dae @ 2013-08-21 10:33 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-arm-kernel, linux-media,
linaro-kernel
Cc: maarten.lankhorst, sumit.semwal, kyungmin.park, myungjoo.ham,
Inki Dae
In-Reply-To: <1377081215-5948-1-git-send-email-inki.dae@samsung.com>
This patch adds a buffer synchronization framework based on DMA BUF[1]
and and based on ww-mutexes[2] for lock mechanism, and has been rebased
on linux-3.11-rc6.
The purpose of this framework is to provide not only buffer access control
to CPU and DMA but also easy-to-use interfaces for device drivers and
user application. This framework can be used for all dma devices using
system memory as dma buffer, especially for most ARM based SoCs.
Changelog v7:
Fix things pointed out by Konrad Rzeszutek Wilk,
- Use EXPORT_SYMBOL_GPL instead of EXPORT_SYMBOL.
- Make sure to unlock and unreference all dmabuf objects
when dmabuf_sync_fini() is called.
- Add more comments.
- Code cleanups.
Changelog v6:
- Fix sync lock to multiple reads.
- Add select system call support.
. Wake up poll_wait when a dmabuf is unlocked.
- Remove unnecessary the use of mutex lock.
- Add private backend ops callbacks.
. This ops has one callback for device drivers to clean up their
sync object resource when the sync object is freed. For this,
device drivers should implement the free callback properly.
- Update document file.
Changelog v5:
- Rmove a dependence on reservation_object: the reservation_object is used
to hook up to ttm and dma-buf for easy sharing of reservations across
devices. However, the dmabuf sync can be used for all dma devices; v4l2
and drm based drivers, so doesn't need the reservation_object anymore.
With regared to this, it adds 'void *sync' to dma_buf structure.
- All patches are rebased on mainline, Linux v3.10.
Changelog v4:
- Add user side interface for buffer synchronization mechanism and update
descriptions related to the user side interface.
Changelog v3:
- remove cache operation relevant codes and update document file.
Changelog v2:
- use atomic_add_unless to avoid potential bug.
- add a macro for checking valid access type.
- code clean.
The mechanism of this framework has the following steps,
1. Register dmabufs to a sync object - A task gets a new sync object and
can add one or more dmabufs that the task wants to access.
This registering should be performed when a device context or an event
context such as a page flip event is created or before CPU accesses a shared
buffer.
dma_buf_sync_get(a sync object, a dmabuf);
2. Lock a sync object - A task tries to lock all dmabufs added in its own
sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid dead
lock issue and for race condition between CPU and CPU, CPU and DMA, and DMA
and DMA. Taking a lock means that others cannot access all locked dmabufs
until the task that locked the corresponding dmabufs, unlocks all the locked
dmabufs.
This locking should be performed before DMA or CPU accesses these dmabufs.
dma_buf_sync_lock(a sync object);
3. Unlock a sync object - The task unlocks all dmabufs added in its own sync
object. The unlock means that the DMA or CPU accesses to the dmabufs have
been completed so that others may access them.
This unlocking should be performed after DMA or CPU has completed accesses
to the dmabufs.
dma_buf_sync_unlock(a sync object);
4. Unregister one or all dmabufs from a sync object - A task unregisters
the given dmabufs from the sync object. This means that the task dosen't
want to lock the dmabufs.
The unregistering should be performed after DMA or CPU has completed
accesses to the dmabufs or when dma_buf_sync_lock() is failed.
dma_buf_sync_put(a sync object, a dmabuf);
dma_buf_sync_put_all(a sync object);
The described steps may be summarized as:
get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
This framework includes the following two features.
1. read (shared) and write (exclusive) locks - A task is required to declare
the access type when the task tries to register a dmabuf;
READ, WRITE, READ DMA, or WRITE DMA.
The below is example codes,
struct dmabuf_sync *sync;
sync = dmabuf_sync_init(...);
...
dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
...
And the below can be used as access types:
DMA_BUF_ACCESS_R - CPU will access a buffer for read.
DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or
write.
2. Mandatory resource releasing - a task cannot hold a lock indefinitely.
A task may never try to unlock a buffer after taking a lock to the buffer.
In this case, a timer handler to the corresponding sync object is called
in five (default) seconds and then the timed-out buffer is unlocked by work
queue handler to avoid lockups and to enforce resources of the buffer.
The below is how to use interfaces for device driver:
1. Allocate and Initialize a sync object:
static void xxx_dmabuf_sync_free(void *priv)
{
struct xxx_context *ctx = priv;
if (!ctx)
return;
ctx->sync = NULL;
}
...
static struct dmabuf_sync_priv_ops driver_specific_ops = {
.free = xxx_dmabuf_sync_free,
};
...
struct dmabuf_sync *sync;
sync = dmabuf_sync_init("test sync", &driver_specific_ops, ctx);
...
2. Add a dmabuf to the sync object when setting up dma buffer relevant
registers:
dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
...
3. Lock all dmabufs of the sync object before DMA or CPU accesses
the dmabufs:
dmabuf_sync_lock(sync);
...
4. Now CPU or DMA can access all dmabufs locked in step 3.
5. Unlock all dmabufs added in a sync object after DMA or CPU access
to these dmabufs is completed:
dmabuf_sync_unlock(sync);
And call the following functions to release all resources,
dmabuf_sync_put_all(sync);
dmabuf_sync_fini(sync);
You can refer to actual example codes:
"drm/exynos: add dmabuf sync support for g2d driver" and
"drm/exynos: add dmabuf sync support for kms framework" from
https://git.kernel.org/cgit/linux/kernel/git/daeinki/
drm-exynos.git/log/?h=dmabuf-sync
And this framework includes fcntl[3] and select system call as interfaces
exported to user. As you know, user sees a buffer object as a dma-buf file
descriptor. fcntl() call with the file descriptor means to lock some buffer
region being managed by the dma-buf object. And select() call with the file
descriptor means to poll the completion event of CPU or DMA access to
the dma-buf.
The below is how to use interfaces for user application:
fcntl system call:
struct flock filelock;
1. Lock a dma buf:
filelock.l_type = F_WRLCK or F_RDLCK;
/* lock entire region to the dma buf. */
filelock.lwhence = SEEK_CUR;
filelock.l_start = 0;
filelock.l_len = 0;
fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
...
CPU access to the dma buf
2. Unlock a dma buf:
filelock.l_type = F_UNLCK;
fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
close(dmabuf fd) call would also unlock the dma buf. And for more
detail, please refer to [3]
select system call:
fd_set wdfs or rdfs;
FD_ZERO(&wdfs or &rdfs);
FD_SET(fd, &wdfs or &rdfs);
select(fd + 1, &rdfs, NULL, NULL, NULL);
or
select(fd + 1, NULL, &wdfs, NULL, NULL);
Every time select system call is called, a caller will wait for
the completion of DMA or CPU access to a shared buffer if there
is someone accessing the shared buffer. If no anyone then select
system call will be returned at once.
References:
[1] http://lwn.net/Articles/470339/
[2] https://patchwork.kernel.org/patch/2625361/
[3] http://linux.die.net/man/2/fcntl
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Documentation/dma-buf-sync.txt | 286 ++++++++++++++++
drivers/base/Kconfig | 7 +
drivers/base/Makefile | 1 +
drivers/base/dma-buf.c | 4 +
drivers/base/dmabuf-sync.c | 706 ++++++++++++++++++++++++++++++++++++++++
include/linux/dma-buf.h | 16 +
include/linux/dmabuf-sync.h | 236 ++++++++++++++
7 files changed, 1256 insertions(+), 0 deletions(-)
create mode 100644 Documentation/dma-buf-sync.txt
create mode 100644 drivers/base/dmabuf-sync.c
create mode 100644 include/linux/dmabuf-sync.h
diff --git a/Documentation/dma-buf-sync.txt b/Documentation/dma-buf-sync.txt
new file mode 100644
index 0000000..5945c8a
--- /dev/null
+++ b/Documentation/dma-buf-sync.txt
@@ -0,0 +1,286 @@
+ DMA Buffer Synchronization Framework
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+ Inki Dae
+ <inki dot dae at samsung dot com>
+ <daeinki at gmail dot com>
+
+This document is a guide for device-driver writers describing the DMA buffer
+synchronization API. This document also describes how to use the API to
+use buffer synchronization mechanism between DMA and DMA, CPU and DMA, and
+CPU and CPU.
+
+The DMA Buffer synchronization API provides buffer synchronization mechanism;
+i.e., buffer access control to CPU and DMA, and easy-to-use interfaces for
+device drivers and user application. And this API can be used for all dma
+devices using system memory as dma buffer, especially for most ARM based SoCs.
+
+
+Motivation
+----------
+
+Buffer synchronization issue between DMA and DMA:
+ Sharing a buffer, a device cannot be aware of when the other device
+ will access the shared buffer: a device may access a buffer containing
+ wrong data if the device accesses the shared buffer while another
+ device is still accessing the shared buffer.
+ Therefore, a user process should have waited for the completion of DMA
+ access by another device before a device tries to access the shared
+ buffer.
+
+Buffer synchronization issue between CPU and DMA:
+ A user process should consider that when having to send a buffer, filled
+ by CPU, to a device driver for the device driver to access the buffer as
+ a input buffer while CPU and DMA are sharing the buffer.
+ This means that the user process needs to understand how the device
+ driver is worked. Hence, the conventional mechanism not only makes
+ user application complicated but also incurs performance overhead.
+
+Buffer synchronization issue between CPU and CPU:
+ In case that two processes share one buffer; shared with DMA also,
+ they may need some mechanism to allow process B to access the shared
+ buffer after the completion of CPU access by process A.
+ Therefore, process B should have waited for the completion of CPU access
+ by process A using the mechanism before trying to access the shared
+ buffer.
+
+What is the best way to solve these buffer synchronization issues?
+ We may need a common object that a device driver and a user process
+ notify the common object of when they try to access a shared buffer.
+ That way we could decide when we have to allow or not to allow for CPU
+ or DMA to access the shared buffer through the common object.
+ If so, what could become the common object? Right, that's a dma-buf[1].
+ Now we have already been using the dma-buf to share one buffer with
+ other drivers.
+
+
+Basic concept
+-------------
+
+The mechanism of this framework has the following steps,
+ 1. Register dmabufs to a sync object - A task gets a new sync object and
+ can add one or more dmabufs that the task wants to access.
+ This registering should be performed when a device context or an event
+ context such as a page flip event is created or before CPU accesses a shared
+ buffer.
+
+ dma_buf_sync_get(a sync object, a dmabuf);
+
+ 2. Lock a sync object - A task tries to lock all dmabufs added in its own
+ sync object. Basically, the lock mechanism uses ww-mutexes[2] to avoid dead
+ lock issue and for race condition between CPU and CPU, CPU and DMA, and DMA
+ and DMA. Taking a lock means that others cannot access all locked dmabufs
+ until the task that locked the corresponding dmabufs, unlocks all the locked
+ dmabufs.
+ This locking should be performed before DMA or CPU accesses these dmabufs.
+
+ dma_buf_sync_lock(a sync object);
+
+ 3. Unlock a sync object - The task unlocks all dmabufs added in its own sync
+ object. The unlock means that the DMA or CPU accesses to the dmabufs have
+ been completed so that others may access them.
+ This unlocking should be performed after DMA or CPU has completed accesses
+ to the dmabufs.
+
+ dma_buf_sync_unlock(a sync object);
+
+ 4. Unregister one or all dmabufs from a sync object - A task unregisters
+ the given dmabufs from the sync object. This means that the task dosen't
+ want to lock the dmabufs.
+ The unregistering should be performed after DMA or CPU has completed
+ accesses to the dmabufs or when dma_buf_sync_lock() is failed.
+
+ dma_buf_sync_put(a sync object, a dmabuf);
+ dma_buf_sync_put_all(a sync object);
+
+ The described steps may be summarized as:
+ get -> lock -> CPU or DMA access to a buffer/s -> unlock -> put
+
+This framework includes the following two features.
+ 1. read (shared) and write (exclusive) locks - A task is required to declare
+ the access type when the task tries to register a dmabuf;
+ READ, WRITE, READ DMA, or WRITE DMA.
+
+ The below is example codes,
+ struct dmabuf_sync *sync;
+
+ sync = dmabuf_sync_init(NULL, "test sync");
+
+ dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
+ ...
+
+ 2. Mandatory resource releasing - a task cannot hold a lock indefinitely.
+ A task may never try to unlock a buffer after taking a lock to the buffer.
+ In this case, a timer handler to the corresponding sync object is called
+ in five (default) seconds and then the timed-out buffer is unlocked by work
+ queue handler to avoid lockups and to enforce resources of the buffer.
+
+
+Access types
+------------
+
+DMA_BUF_ACCESS_R - CPU will access a buffer for read.
+DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
+DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
+DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or write.
+
+
+Generic user interfaces
+-----------------------
+
+And this framework includes fcntl[3] and select system calls as interfaces
+exported to user. As you know, user sees a buffer object as a dma-buf file
+descriptor. fcntl() call with the file descriptor means to lock some buffer
+region being managed by the dma-buf object. And select call with the file
+descriptor means to poll the completion event of CPU or DMA access to
+the dma-buf.
+
+
+API set
+-------
+
+bool is_dmabuf_sync_supported(void)
+ - Check if dmabuf sync is supported or not.
+
+struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void priv*)
+ - Allocate and initialize a new sync object. The caller can get a new
+ sync object for buffer synchronization. ops is used for device driver
+ to clean up its own sync object. For this, each device driver should
+ implement a free callback. priv is used for device driver to get its
+ device context when free callback is called.
+
+void dmabuf_sync_fini(struct dmabuf_sync *sync)
+ - Release all resources to the sync object.
+
+int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
+ unsigned int type)
+ - Get dmabuf sync object. Internally, this function allocates
+ a dmabuf_sync object and adds a given dmabuf to it, and also takes
+ a reference to the dmabuf. The caller can tie up multiple dmabufs
+ into one sync object by calling this function several times.
+
+void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
+ - Put dmabuf sync object to a given dmabuf. Internally, this function
+ removes a given dmabuf from a sync object and remove the sync object.
+ At this time, the dmabuf is putted.
+
+void dmabuf_sync_put_all(struct dmabuf_sync *sync)
+ - Put dmabuf sync object to dmabufs. Internally, this function removes
+ all dmabufs from a sync object and remove the sync object.
+ At this time, all dmabufs are putted.
+
+int dmabuf_sync_lock(struct dmabuf_sync *sync)
+ - Lock all dmabufs added in a sync object. The caller should call this
+ function prior to CPU or DMA access to the dmabufs so that others can
+ not access the dmabufs. Internally, this function avoids dead lock
+ issue with ww-mutexes.
+
+int dmabuf_sync_single_lock(struct dma_buf *dmabuf)
+ - Lock a dmabuf. The caller should call this
+ function prior to CPU or DMA access to the dmabuf so that others can
+ not access the dmabuf.
+
+int dmabuf_sync_unlock(struct dmabuf_sync *sync)
+ - Unlock all dmabufs added in a sync object. The caller should call
+ this function after CPU or DMA access to the dmabufs is completed so
+ that others can access the dmabufs.
+
+void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
+ - Unlock a dmabuf. The caller should call this function after CPU or
+ DMA access to the dmabuf is completed so that others can access
+ the dmabuf.
+
+
+Tutorial for device driver
+--------------------------
+
+1. Allocate and Initialize a sync object:
+ static void xxx_dmabuf_sync_free(void *priv)
+ {
+ struct xxx_context *ctx = priv;
+
+ if (!ctx)
+ return;
+
+ ctx->sync = NULL;
+ }
+ ...
+
+ static struct dmabuf_sync_priv_ops driver_specific_ops = {
+ .free = xxx_dmabuf_sync_free,
+ };
+ ...
+
+ struct dmabuf_sync *sync;
+
+ sync = dmabuf_sync_init("test sync", &driver_specific_ops, ctx);
+ ...
+
+2. Add a dmabuf to the sync object when setting up dma buffer relevant registers:
+ dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_READ);
+ ...
+
+3. Lock all dmabufs of the sync object before DMA or CPU accesses the dmabufs:
+ dmabuf_sync_lock(sync);
+ ...
+
+4. Now CPU or DMA can access all dmabufs locked in step 3.
+
+5. Unlock all dmabufs added in a sync object after DMA or CPU access to these
+ dmabufs is completed:
+ dmabuf_sync_unlock(sync);
+
+ And call the following functions to release all resources,
+ dmabuf_sync_put_all(sync);
+ dmabuf_sync_fini(sync);
+
+
+Tutorial for user application
+-----------------------------
+fcntl system call:
+
+ struct flock filelock;
+
+1. Lock a dma buf:
+ filelock.l_type = F_WRLCK or F_RDLCK;
+
+ /* lock entire region to the dma buf. */
+ filelock.lwhence = SEEK_CUR;
+ filelock.l_start = 0;
+ filelock.l_len = 0;
+
+ fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
+ ...
+ CPU access to the dma buf
+
+2. Unlock a dma buf:
+ filelock.l_type = F_UNLCK;
+
+ fcntl(dmabuf fd, F_SETLKW or F_SETLK, &filelock);
+
+ close(dmabuf fd) call would also unlock the dma buf. And for more
+ detail, please refer to [3]
+
+
+select system call:
+
+ fd_set wdfs or rdfs;
+
+ FD_ZERO(&wdfs or &rdfs);
+ FD_SET(fd, &wdfs or &rdfs);
+
+ select(fd + 1, &rdfs, NULL, NULL, NULL);
+ or
+ select(fd + 1, NULL, &wdfs, NULL, NULL);
+
+ Every time select system call is called, a caller will wait for
+ the completion of DMA or CPU access to a shared buffer if there
+ is someone accessing the shared buffer. If no anyone then select
+ system call will be returned at once.
+
+References:
+[1] http://lwn.net/Articles/470339/
+[2] https://patchwork.kernel.org/patch/2625361/
+[3] http://linux.die.net/man/2/fcntl
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 5daa259..35e1518 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -200,6 +200,13 @@ config DMA_SHARED_BUFFER
APIs extension; the file's descriptor can then be passed on to other
driver.
+config DMABUF_SYNC
+ bool "DMABUF Synchronization Framework"
+ depends on DMA_SHARED_BUFFER
+ help
+ This option enables dmabuf sync framework for buffer synchronization between
+ DMA and DMA, CPU and DMA, and CPU and CPU.
+
config CMA
bool "Contiguous Memory Allocator"
depends on HAVE_DMA_CONTIGUOUS && HAVE_MEMBLOCK
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 48029aa..e06a5d7 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -11,6 +11,7 @@ obj-y += power/
obj-$(CONFIG_HAS_DMA) += dma-mapping.o
obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
obj-$(CONFIG_DMA_SHARED_BUFFER) += dma-buf.o reservation.o
+obj-$(CONFIG_DMABUF_SYNC) += dmabuf-sync.o
obj-$(CONFIG_ISA) += isa.o
obj-$(CONFIG_FW_LOADER) += firmware_class.o
obj-$(CONFIG_NUMA) += node.o
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 6687ba7..4aca57a 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -29,6 +29,7 @@
#include <linux/export.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/dmabuf-sync.h>
static inline int is_dma_buf_file(struct file *);
@@ -56,6 +57,8 @@ static int dma_buf_release(struct inode *inode, struct file *file)
list_del(&dmabuf->list_node);
mutex_unlock(&db_list.lock);
+ dmabuf_sync_reservation_fini(dmabuf);
+
kfree(dmabuf);
return 0;
}
@@ -134,6 +137,7 @@ struct dma_buf *dma_buf_export_named(void *priv, const struct dma_buf_ops *ops,
file = anon_inode_getfile("dmabuf", &dma_buf_fops, dmabuf, flags);
+ dmabuf_sync_reservation_init(dmabuf);
dmabuf->file = file;
mutex_init(&dmabuf->lock);
diff --git a/drivers/base/dmabuf-sync.c b/drivers/base/dmabuf-sync.c
new file mode 100644
index 0000000..5d69aef
--- /dev/null
+++ b/drivers/base/dmabuf-sync.c
@@ -0,0 +1,706 @@
+/*
+ * Copyright (C) 2013 Samsung Electronics Co.Ltd
+ * Authors:
+ * Inki Dae <inki.dae@samsung.com>
+ *
+ * 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.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/debugfs.h>
+#include <linux/uaccess.h>
+
+#include <linux/dmabuf-sync.h>
+
+#define MAX_SYNC_TIMEOUT 5 /* Second. */
+
+int dmabuf_sync_enabled = 1;
+
+MODULE_PARM_DESC(enabled, "Check if dmabuf sync is supported or not");
+module_param_named(enabled, dmabuf_sync_enabled, int, 0444);
+
+DEFINE_WW_CLASS(dmabuf_sync_ww_class);
+
+static void dmabuf_sync_timeout_worker(struct work_struct *work)
+{
+ struct dmabuf_sync *sync = container_of(work, struct dmabuf_sync, work);
+ struct dmabuf_sync_object *sobj;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ struct dmabuf_sync_reservation *rsvp = sobj->robj;
+
+ BUG_ON(!rsvp);
+
+ mutex_lock(&rsvp->lock);
+
+ pr_warn("%s: timeout = 0x%p [type = %d:%d, " \
+ "refcnt = %d, locked = %d]\n",
+ sync->name, sobj->dmabuf,
+ rsvp->accessed_type,
+ sobj->access_type,
+ atomic_read(&rsvp->shared_cnt),
+ rsvp->locked);
+
+ /* unlock only valid sync object. */
+ if (!rsvp->locked) {
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ if (rsvp->polled) {
+ rsvp->poll_event = true;
+ rsvp->polled = false;
+ wake_up_interruptible(&rsvp->poll_wait);
+ }
+
+ if (atomic_add_unless(&rsvp->shared_cnt, -1, 1)) {
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ mutex_unlock(&rsvp->lock);
+
+ ww_mutex_unlock(&rsvp->sync_lock);
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = false;
+
+ if (sobj->access_type & DMA_BUF_ACCESS_R)
+ pr_warn("%s: r-unlocked = 0x%p\n",
+ sync->name, sobj->dmabuf);
+ else
+ pr_warn("%s: w-unlocked = 0x%p\n",
+ sync->name, sobj->dmabuf);
+
+ mutex_unlock(&rsvp->lock);
+ }
+
+ sync->status = 0;
+ mutex_unlock(&sync->lock);
+
+ dmabuf_sync_put_all(sync);
+ dmabuf_sync_fini(sync);
+}
+
+static void dmabuf_sync_lock_timeout(unsigned long arg)
+{
+ struct dmabuf_sync *sync = (struct dmabuf_sync *)arg;
+
+ schedule_work(&sync->work);
+}
+
+static int dmabuf_sync_lock_objs(struct dmabuf_sync *sync,
+ struct ww_acquire_ctx *ctx)
+{
+ struct dmabuf_sync_object *contended_sobj = NULL;
+ struct dmabuf_sync_object *res_sobj = NULL;
+ struct dmabuf_sync_object *sobj = NULL;
+ int ret;
+
+ if (ctx)
+ ww_acquire_init(ctx, &dmabuf_sync_ww_class);
+
+retry:
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ struct dmabuf_sync_reservation *rsvp = sobj->robj;
+
+ if (WARN_ON(!rsvp))
+ continue;
+
+ mutex_lock(&rsvp->lock);
+
+ /* Don't lock in case of read and read. */
+ if (rsvp->accessed_type & DMA_BUF_ACCESS_R &&
+ sobj->access_type & DMA_BUF_ACCESS_R) {
+ atomic_inc(&rsvp->shared_cnt);
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ if (sobj = res_sobj) {
+ res_sobj = NULL;
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ mutex_unlock(&rsvp->lock);
+
+ ret = ww_mutex_lock(&rsvp->sync_lock, ctx);
+ if (ret < 0) {
+ contended_sobj = sobj;
+
+ if (ret = -EDEADLK)
+ pr_warn("%s: deadlock = 0x%p\n",
+ sync->name, sobj->dmabuf);
+ goto err;
+ }
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = true;
+
+ mutex_unlock(&rsvp->lock);
+ }
+
+ if (ctx)
+ ww_acquire_done(ctx);
+
+ init_timer(&sync->timer);
+
+ sync->timer.data = (unsigned long)sync;
+ sync->timer.function = dmabuf_sync_lock_timeout;
+ sync->timer.expires = jiffies + (HZ * MAX_SYNC_TIMEOUT);
+
+ add_timer(&sync->timer);
+
+ return 0;
+
+err:
+ list_for_each_entry_continue_reverse(sobj, &sync->syncs, head) {
+ struct dmabuf_sync_reservation *rsvp = sobj->robj;
+
+ mutex_lock(&rsvp->lock);
+
+ /* Don't need to unlock in case of read and read. */
+ if (atomic_add_unless(&rsvp->shared_cnt, -1, 1)) {
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ mutex_unlock(&rsvp->lock);
+
+ ww_mutex_unlock(&rsvp->sync_lock);
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = false;
+ mutex_unlock(&rsvp->lock);
+ }
+
+ if (res_sobj) {
+ struct dmabuf_sync_reservation *rsvp = res_sobj->robj;
+
+ mutex_lock(&rsvp->lock);
+
+ if (!atomic_add_unless(&rsvp->shared_cnt, -1, 1)) {
+ mutex_unlock(&rsvp->lock);
+
+ ww_mutex_unlock(&rsvp->sync_lock);
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = false;
+ }
+
+ mutex_unlock(&rsvp->lock);
+ }
+
+ if (ret = -EDEADLK) {
+ ww_mutex_lock_slow(&contended_sobj->robj->sync_lock, ctx);
+ res_sobj = contended_sobj;
+
+ goto retry;
+ }
+
+ if (ctx)
+ ww_acquire_fini(ctx);
+
+ return ret;
+}
+
+static void dmabuf_sync_unlock_objs(struct dmabuf_sync *sync,
+ struct ww_acquire_ctx *ctx)
+{
+ struct dmabuf_sync_object *sobj;
+
+ if (list_empty(&sync->syncs))
+ return;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ struct dmabuf_sync_reservation *rsvp = sobj->robj;
+
+ mutex_lock(&rsvp->lock);
+
+ if (rsvp->polled) {
+ rsvp->poll_event = true;
+ rsvp->polled = false;
+ wake_up_interruptible(&rsvp->poll_wait);
+ }
+
+ if (atomic_add_unless(&rsvp->shared_cnt, -1, 1)) {
+ mutex_unlock(&rsvp->lock);
+ continue;
+ }
+
+ mutex_unlock(&rsvp->lock);
+
+ ww_mutex_unlock(&rsvp->sync_lock);
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = false;
+ mutex_unlock(&rsvp->lock);
+ }
+
+ mutex_unlock(&sync->lock);
+
+ if (ctx)
+ ww_acquire_fini(ctx);
+
+ del_timer(&sync->timer);
+}
+
+/**
+ * dmabuf_sync_is_supported - Check if dmabuf sync is supported or not.
+ */
+bool dmabuf_sync_is_supported(void)
+{
+ return dmabuf_sync_enabled = 1;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_is_supported);
+
+/**
+ * dmabuf_sync_init - Allocate and initialize a dmabuf sync.
+ *
+ * @priv: A device private data.
+ * @name: A sync object name.
+ *
+ * This function should be called when a device context or an event
+ * context such as a page flip event is created. And the created
+ * dmabuf_sync object should be set to the context.
+ * The caller can get a new sync object for buffer synchronization
+ * through this function.
+ */
+struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void *priv)
+{
+ struct dmabuf_sync *sync;
+
+ sync = kzalloc(sizeof(*sync), GFP_KERNEL);
+ if (!sync)
+ return ERR_PTR(-ENOMEM);
+
+ strncpy(sync->name, name, DMABUF_SYNC_NAME_SIZE);
+
+ sync->ops = ops;
+ sync->priv = priv;
+ INIT_LIST_HEAD(&sync->syncs);
+ mutex_init(&sync->lock);
+ INIT_WORK(&sync->work, dmabuf_sync_timeout_worker);
+
+ return sync;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_init);
+
+/**
+ * dmabuf_sync_fini - Release a given dmabuf sync.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_init call to release relevant resources, and after
+ * dmabuf_sync_unlock function is called.
+ */
+void dmabuf_sync_fini(struct dmabuf_sync *sync)
+{
+ struct dmabuf_sync_object *sobj;
+
+ if (WARN_ON(!sync))
+ return;
+
+ if (list_empty(&sync->syncs))
+ goto free_sync;
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ struct dmabuf_sync_reservation *rsvp = sobj->robj;
+
+ mutex_lock(&rsvp->lock);
+
+ if (rsvp->locked) {
+ mutex_unlock(&rsvp->lock);
+ ww_mutex_unlock(&rsvp->sync_lock);
+
+ mutex_lock(&rsvp->lock);
+ rsvp->locked = false;
+ }
+
+ mutex_unlock(&rsvp->lock);
+ }
+
+ /*
+ * If !list_empty(&sync->syncs) then it means that dmabuf_sync_put()
+ * or dmabuf_sync_put_all() was never called. So unreference all
+ * dmabuf objects added to sync->syncs, and remove them from the syncs.
+ */
+ dmabuf_sync_put_all(sync);
+
+free_sync:
+ if (sync->ops && sync->ops->free)
+ sync->ops->free(sync->priv);
+
+ kfree(sync);
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_fini);
+
+/*
+ * dmabuf_sync_get_obj - Add a given object to sync's list.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ * @dmabuf: An object to dma_buf structure.
+ * @type: A access type to a dma buf.
+ * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
+ * others for read access. On the other hand, the DMA_BUF_ACCESS_W
+ * means that this dmabuf couldn't be accessed by others but would be
+ * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can be
+ * combined.
+ *
+ * This function creates and initializes a new dmabuf sync object and it adds
+ * the dmabuf sync object to syncs list to track and manage all dmabufs.
+ */
+static int dmabuf_sync_get_obj(struct dmabuf_sync *sync, struct dma_buf *dmabuf,
+ unsigned int type)
+{
+ struct dmabuf_sync_object *sobj;
+
+ if (!dmabuf->sync)
+ return -EFAULT;
+
+ if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type))
+ return -EINVAL;
+
+ if ((type & DMA_BUF_ACCESS_RW) = DMA_BUF_ACCESS_RW)
+ type &= ~DMA_BUF_ACCESS_R;
+
+ sobj = kzalloc(sizeof(*sobj), GFP_KERNEL);
+ if (!sobj)
+ return -ENOMEM;
+
+ get_dma_buf(dmabuf);
+
+ sobj->dmabuf = dmabuf;
+ sobj->robj = dmabuf->sync;
+ sobj->access_type = type;
+
+ mutex_lock(&sync->lock);
+ list_add_tail(&sobj->head, &sync->syncs);
+ mutex_unlock(&sync->lock);
+
+ return 0;
+}
+
+/*
+ * dmabuf_sync_put_obj - Release a given sync object.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation failed after
+ * dmabuf_sync_get_obj call to release a given sync object.
+ */
+static void dmabuf_sync_put_obj(struct dmabuf_sync *sync,
+ struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_object *sobj;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry(sobj, &sync->syncs, head) {
+ if (sobj->dmabuf != dmabuf)
+ continue;
+
+ dma_buf_put(sobj->dmabuf);
+
+ list_del_init(&sobj->head);
+ kfree(sobj);
+ break;
+ }
+
+ if (list_empty(&sync->syncs))
+ sync->status = 0;
+
+ mutex_unlock(&sync->lock);
+}
+
+/*
+ * dmabuf_sync_put_objs - Release all sync objects of dmabuf_sync.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation failed after
+ * dmabuf_sync_get_obj call to release all sync objects.
+ */
+static void dmabuf_sync_put_objs(struct dmabuf_sync *sync)
+{
+ struct dmabuf_sync_object *sobj, *next;
+
+ mutex_lock(&sync->lock);
+
+ list_for_each_entry_safe(sobj, next, &sync->syncs, head) {
+ dma_buf_put(sobj->dmabuf);
+
+ list_del_init(&sobj->head);
+ kfree(sobj);
+ }
+
+ mutex_unlock(&sync->lock);
+
+ sync->status = 0;
+}
+
+/**
+ * dmabuf_sync_lock - lock all dmabufs added to syncs list.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * The caller should call this function prior to CPU or DMA access to
+ * the dmabufs so that others can not access the dmabufs.
+ * Internally, this function avoids dead lock issue with ww-mutex.
+ */
+int dmabuf_sync_lock(struct dmabuf_sync *sync)
+{
+ int ret;
+
+ if (!sync)
+ return -EFAULT;
+
+ if (list_empty(&sync->syncs))
+ return -EINVAL;
+
+ if (sync->status != DMABUF_SYNC_GOT)
+ return -EINVAL;
+
+ ret = dmabuf_sync_lock_objs(sync, &sync->ctx);
+ if (ret < 0)
+ return ret;
+
+ sync->status = DMABUF_SYNC_LOCKED;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_lock);
+
+/**
+ * dmabuf_sync_unlock - unlock all objects added to syncs list.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * The caller should call this function after CPU or DMA access to
+ * the dmabufs is completed so that others can access the dmabufs.
+ */
+int dmabuf_sync_unlock(struct dmabuf_sync *sync)
+{
+ if (!sync)
+ return -EFAULT;
+
+ /* If current dmabuf sync object wasn't reserved then just return. */
+ if (sync->status != DMABUF_SYNC_LOCKED)
+ return -EAGAIN;
+
+ dmabuf_sync_unlock_objs(sync, &sync->ctx);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_unlock);
+
+/**
+ * dmabuf_sync_single_lock - lock a dma buf.
+ *
+ * @dmabuf: A dma buf object that tries to lock.
+ * @type: A access type to a dma buf.
+ * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
+ * others for read access. On the other hand, the DMA_BUF_ACCESS_W
+ * means that this dmabuf couldn't be accessed by others but would be
+ * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can
+ * be combined with other.
+ * @wait: Indicate whether caller is blocked or not.
+ * true means that caller will be blocked, and false means that this
+ * function will return -EAGAIN if this caller can't take the lock
+ * right now.
+ *
+ * The caller should call this function prior to CPU or DMA access to the dmabuf
+ * so that others cannot access the dmabuf.
+ */
+int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
+ bool wait)
+{
+ struct dmabuf_sync_reservation *robj;
+
+ if (!dmabuf->sync)
+ return -EFAULT;
+
+ if (!IS_VALID_DMA_BUF_ACCESS_TYPE(type))
+ return -EINVAL;
+
+ get_dma_buf(dmabuf);
+ robj = dmabuf->sync;
+
+ mutex_lock(&robj->lock);
+
+ /* Don't lock in case of read and read. */
+ if (robj->accessed_type & DMA_BUF_ACCESS_R && type & DMA_BUF_ACCESS_R) {
+ atomic_inc(&robj->shared_cnt);
+ mutex_unlock(&robj->lock);
+ return 0;
+ }
+
+ /*
+ * In case of F_SETLK, just return -EAGAIN if this dmabuf has already
+ * been locked.
+ */
+ if (!wait && robj->locked) {
+ mutex_unlock(&robj->lock);
+ dma_buf_put(dmabuf);
+ return -EAGAIN;
+ }
+
+ mutex_unlock(&robj->lock);
+
+ /* Unlocked by dmabuf_sync_single_unlock or dmabuf_sync_unlock. */
+ mutex_lock(&robj->sync_lock.base);
+
+ mutex_lock(&robj->lock);
+ robj->locked = true;
+ mutex_unlock(&robj->lock);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_single_lock);
+
+/**
+ * dmabuf_sync_single_unlock - unlock a dma buf.
+ *
+ * @dmabuf: A dma buf object that tries to unlock.
+ *
+ * The caller should call this function after CPU or DMA access to
+ * the dmabuf is completed so that others can access the dmabuf.
+ */
+void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_reservation *robj;
+
+ if (!dmabuf->sync) {
+ WARN_ON(1);
+ return;
+ }
+
+ robj = dmabuf->sync;
+
+ mutex_lock(&robj->lock);
+
+ if (robj->polled) {
+ robj->poll_event = true;
+ robj->polled = false;
+ wake_up_interruptible(&robj->poll_wait);
+ }
+
+ if (atomic_add_unless(&robj->shared_cnt, -1 , 1)) {
+ mutex_unlock(&robj->lock);
+ dma_buf_put(dmabuf);
+ return;
+ }
+
+ mutex_unlock(&robj->lock);
+
+ mutex_unlock(&robj->sync_lock.base);
+
+ mutex_lock(&robj->lock);
+ robj->locked = false;
+ mutex_unlock(&robj->lock);
+
+ dma_buf_put(dmabuf);
+
+ return;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_single_unlock);
+
+/**
+ * dmabuf_sync_get - Get dmabuf sync object.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ * @sync_buf: A dmabuf object to be synchronized with others.
+ * @type: A access type to a dma buf.
+ * The DMA_BUF_ACCESS_R means that this dmabuf could be accessed by
+ * others for read access. On the other hand, the DMA_BUF_ACCESS_W
+ * means that this dmabuf couldn't be accessed by others but would be
+ * accessed by caller's dma exclusively. And the DMA_BUF_ACCESS_DMA can
+ * be combined with other.
+ *
+ * This function should be called after dmabuf_sync_init function is called.
+ * The caller can tie up multiple dmabufs into one sync object by calling this
+ * function several times. Internally, this function allocates
+ * a dmabuf_sync_object and adds a given dmabuf to it, and also takes
+ * a reference to a dmabuf.
+ */
+int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf, unsigned int type)
+{
+ int ret;
+
+ if (!sync || !sync_buf)
+ return -EFAULT;
+
+ ret = dmabuf_sync_get_obj(sync, sync_buf, type);
+ if (ret < 0)
+ return ret;
+
+ sync->status = DMABUF_SYNC_GOT;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_get);
+
+/**
+ * dmabuf_sync_put - Put dmabuf sync object to a given dmabuf.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ * @dmabuf: An dmabuf object.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_get function is called to release the dmabuf, or
+ * dmabuf_sync_unlock function is called. Internally, this function
+ * removes a given dmabuf from a sync object and remove the sync object.
+ * At this time, the dmabuf is putted.
+ */
+void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf)
+{
+ if (!sync || !dmabuf) {
+ WARN_ON(1);
+ return;
+ }
+
+ if (list_empty(&sync->syncs))
+ return;
+
+ dmabuf_sync_put_obj(sync, dmabuf);
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_put);
+
+/**
+ * dmabuf_sync_put_all - Put dmabuf sync object to dmabufs.
+ *
+ * @sync: An object to dmabuf_sync structure.
+ *
+ * This function should be called if some operation is failed after
+ * dmabuf_sync_get function is called to release all sync objects, or
+ * dmabuf_sync_unlock function is called. Internally, this function
+ * removes dmabufs from a sync object and remove the sync object.
+ * At this time, all dmabufs are putted.
+ */
+void dmabuf_sync_put_all(struct dmabuf_sync *sync)
+{
+ if (!sync) {
+ WARN_ON(1);
+ return;
+ }
+
+ if (list_empty(&sync->syncs))
+ return;
+
+ dmabuf_sync_put_objs(sync);
+}
+EXPORT_SYMBOL_GPL(dmabuf_sync_put_all);
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index dfac5ed..0109673 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -115,6 +115,7 @@ struct dma_buf_ops {
* @exp_name: name of the exporter; useful for debugging.
* @list_node: node for dma_buf accounting and debugging.
* @priv: exporter specific private data for this buffer object.
+ * @sync: sync object linked to this dma-buf
*/
struct dma_buf {
size_t size;
@@ -128,6 +129,7 @@ struct dma_buf {
const char *exp_name;
struct list_head list_node;
void *priv;
+ void *sync;
};
/**
@@ -148,6 +150,20 @@ struct dma_buf_attachment {
void *priv;
};
+#define DMA_BUF_ACCESS_R 0x1
+#define DMA_BUF_ACCESS_W 0x2
+#define DMA_BUF_ACCESS_DMA 0x4
+#define DMA_BUF_ACCESS_RW (DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_W)
+#define DMA_BUF_ACCESS_DMA_R (DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_DMA)
+#define DMA_BUF_ACCESS_DMA_W (DMA_BUF_ACCESS_W | DMA_BUF_ACCESS_DMA)
+#define DMA_BUF_ACCESS_DMA_RW (DMA_BUF_ACCESS_DMA_R | DMA_BUF_ACCESS_DMA_W)
+#define IS_VALID_DMA_BUF_ACCESS_TYPE(t) (t = DMA_BUF_ACCESS_R || \
+ t = DMA_BUF_ACCESS_W || \
+ t = DMA_BUF_ACCESS_DMA_R || \
+ t = DMA_BUF_ACCESS_DMA_W || \
+ t = DMA_BUF_ACCESS_RW || \
+ t = DMA_BUF_ACCESS_DMA_RW)
+
/**
* get_dma_buf - convenience wrapper for get_file.
* @dmabuf: [in] pointer to dma_buf
diff --git a/include/linux/dmabuf-sync.h b/include/linux/dmabuf-sync.h
new file mode 100644
index 0000000..91c9111
--- /dev/null
+++ b/include/linux/dmabuf-sync.h
@@ -0,0 +1,236 @@
+/*
+ * Copyright (C) 2013 Samsung Electronics Co.Ltd
+ * Authors:
+ * Inki Dae <inki.dae@samsung.com>
+ *
+ * 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.
+ *
+ */
+
+#include <linux/ww_mutex.h>
+#include <linux/sched.h>
+#include <linux/dma-buf.h>
+
+#define DMABUF_SYNC_NAME_SIZE 64
+
+/*
+ * Status to a dmabuf_sync object.
+ *
+ * @DMABUF_SYNC_GOT: Indicate that one more dmabuf objects have been added
+ * to a sync's list.
+ * @DMABUF_SYNC_LOCKED: Indicate that all dmabuf objects in a sync's list
+ * have been locked.
+ */
+enum dmabuf_sync_status {
+ DMABUF_SYNC_GOT = 1,
+ DMABUF_SYNC_LOCKED,
+};
+
+/*
+ * A structure for dmabuf_sync_reservation.
+ *
+ * @sync_lock: This provides read or write lock to a dmabuf.
+ * Except in the below cases, a task will be blocked if the task
+ * tries to lock a dmabuf for CPU or DMA access when other task
+ * already locked the dmabuf.
+ *
+ * Before After
+ * --------------------------
+ * CPU read CPU read
+ * CPU read DMA read
+ * DMA read CPU read
+ * DMA read DMA read
+ *
+ * @lock: Protecting a dmabuf_sync_reservation object.
+ * @poll_wait: A wait queue object to poll a dmabuf object.
+ * @poll_event: Indicate whether a dmabuf object - being polled -
+ * was unlocked or not. If true, a blocked task will be out
+ * of select system call.
+ * @poll: Indicate whether the polling to a dmabuf object was requested
+ * or not by userspace.
+ * @shared_cnt: Shared count to a dmabuf object.
+ * @accessed_type: Indicate how and who a dmabuf object was accessed by.
+ * One of the below types could be set.
+ * DMA_BUF_ACCESS_R -> CPU access for read.
+ * DMA_BUF_ACCRSS_W -> CPU access for write.
+ * DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_DMA -> DMA access for read.
+ * DMA_BUF_ACCESS_W | DMA_BUF_ACCESS_DMA -> DMA access for write.
+ * @locked: Indicate whether a dmabuf object has been locked or not.
+ *
+ */
+struct dmabuf_sync_reservation {
+ struct ww_mutex sync_lock;
+ struct mutex lock;
+ wait_queue_head_t poll_wait;
+ unsigned int poll_event;
+ unsigned int polled;
+ atomic_t shared_cnt;
+ unsigned int accessed_type;
+ unsigned int locked;
+};
+
+/*
+ * A structure for dmabuf_sync_object.
+ *
+ * @head: A list head to be added to syncs list.
+ * @robj: A reservation_object object.
+ * @dma_buf: A dma_buf object.
+ * @access_type: Indicate how a current task tries to access
+ * a given buffer, and one of the below types could be set.
+ * DMA_BUF_ACCESS_R -> CPU access for read.
+ * DMA_BUF_ACCRSS_W -> CPU access for write.
+ * DMA_BUF_ACCESS_R | DMA_BUF_ACCESS_DMA -> DMA access for read.
+ * DMA_BUF_ACCESS_W | DMA_BUF_ACCESS_DMA -> DMA access for write.
+ */
+struct dmabuf_sync_object {
+ struct list_head head;
+ struct dmabuf_sync_reservation *robj;
+ struct dma_buf *dmabuf;
+ unsigned int access_type;
+};
+
+struct dmabuf_sync_priv_ops {
+ void (*free)(void *priv);
+};
+
+/*
+ * A structure for dmabuf_sync.
+ *
+ * @syncs: A list head to sync object and this is global to system.
+ * @list: A list entry used as committed list node
+ * @lock: Protecting a dmabuf_sync object.
+ * @ctx: A current context for ww mutex.
+ * @work: A work struct to release resources at timeout.
+ * @priv: A private data.
+ * @name: A string to dmabuf sync owner.
+ * @timer: A timer list to avoid lockup and release resources.
+ * @status: Indicate current status (DMABUF_SYNC_GOT or DMABUF_SYNC_LOCKED).
+ */
+struct dmabuf_sync {
+ struct list_head syncs;
+ struct list_head list;
+ struct mutex lock;
+ struct ww_acquire_ctx ctx;
+ struct work_struct work;
+ void *priv;
+ struct dmabuf_sync_priv_ops *ops;
+ char name[DMABUF_SYNC_NAME_SIZE];
+ struct timer_list timer;
+ unsigned int status;
+};
+
+#ifdef CONFIG_DMABUF_SYNC
+
+extern struct ww_class dmabuf_sync_ww_class;
+
+static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_reservation *obj;
+
+ obj = kzalloc(sizeof(*obj), GFP_KERNEL);
+ if (!obj)
+ return;
+
+ dmabuf->sync = obj;
+
+ ww_mutex_init(&obj->sync_lock, &dmabuf_sync_ww_class);
+
+ mutex_init(&obj->lock);
+ atomic_set(&obj->shared_cnt, 1);
+
+ init_waitqueue_head(&obj->poll_wait);
+}
+
+static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf)
+{
+ struct dmabuf_sync_reservation *obj;
+
+ if (!dmabuf->sync)
+ return;
+
+ obj = dmabuf->sync;
+
+ ww_mutex_destroy(&obj->sync_lock);
+
+ kfree(obj);
+}
+
+extern bool dmabuf_sync_is_supported(void);
+
+extern struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void *priv);
+
+extern void dmabuf_sync_fini(struct dmabuf_sync *sync);
+
+extern int dmabuf_sync_lock(struct dmabuf_sync *sync);
+
+extern int dmabuf_sync_unlock(struct dmabuf_sync *sync);
+
+int dmabuf_sync_single_lock(struct dma_buf *dmabuf, unsigned int type,
+ bool wait);
+
+void dmabuf_sync_single_unlock(struct dma_buf *dmabuf);
+
+extern int dmabuf_sync_get(struct dmabuf_sync *sync, void *sync_buf,
+ unsigned int type);
+
+extern void dmabuf_sync_put(struct dmabuf_sync *sync, struct dma_buf *dmabuf);
+
+extern void dmabuf_sync_put_all(struct dmabuf_sync *sync);
+
+#else
+
+static inline void dmabuf_sync_reservation_init(struct dma_buf *dmabuf) { }
+
+static inline void dmabuf_sync_reservation_fini(struct dma_buf *dmabuf) { }
+
+static inline bool dmabuf_sync_is_supported(void) { return false; }
+
+static inline struct dmabuf_sync *dmabuf_sync_init(const char *name,
+ struct dmabuf_sync_priv_ops *ops,
+ void *priv)
+{
+ return ERR_PTR(0);
+}
+
+static inline void dmabuf_sync_fini(struct dmabuf_sync *sync) { }
+
+static inline int dmabuf_sync_lock(struct dmabuf_sync *sync)
+{
+ return 0;
+}
+
+static inline int dmabuf_sync_unlock(struct dmabuf_sync *sync)
+{
+ return 0;
+}
+
+static inline int dmabuf_sync_single_lock(struct dma_buf *dmabuf,
+ unsigned int type,
+ bool wait)
+{
+ return 0;
+}
+
+static inline void dmabuf_sync_single_unlock(struct dma_buf *dmabuf)
+{
+ return;
+}
+
+static inline int dmabuf_sync_get(struct dmabuf_sync *sync,
+ void *sync_buf,
+ unsigned int type)
+{
+ return 0;
+}
+
+static inline void dmabuf_sync_put(struct dmabuf_sync *sync,
+ struct dma_buf *dmabuf) { }
+
+static inline void dmabuf_sync_put_all(struct dmabuf_sync *sync) { }
+
+#endif
--
1.7.5.4
^ permalink raw reply related
* [PATCH v2 2/2] dma-buf: Add user interfaces for dmabuf sync support
From: Inki Dae @ 2013-08-21 10:33 UTC (permalink / raw)
To: dri-devel, linux-fbdev, linux-arm-kernel, linux-media,
linaro-kernel
Cc: maarten.lankhorst, sumit.semwal, kyungmin.park, myungjoo.ham,
Inki Dae
In-Reply-To: <1377081215-5948-1-git-send-email-inki.dae@samsung.com>
This patch adds lock and poll callbacks to dma buf file operations,
and these callbacks will be called by fcntl and select system calls.
fcntl and select system calls can be used to wait for the completion
of DMA or CPU access to a shared dmabuf. The difference of them is
fcntl system call takes a lock after the completion but select system
call doesn't. So in case of fcntl system call, it's useful when a task
wants to access a shared dmabuf without any broken. On the other hand,
it's useful when a task wants to just wait for the completion.
Changelog v2:
- Add select system call support.
. The purpose of this feature is to wait for the completion of DMA or
CPU access to a dmabuf without that caller locks the dmabuf again
after the completion.
That is useful when caller wants to be aware of the completion of
DMA access to the dmabuf, and the caller doesn't use intefaces for
the DMA device driver.
Signed-off-by: Inki Dae <inki.dae@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/base/dma-buf.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 81 insertions(+), 0 deletions(-)
diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 4aca57a..f16a396 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -29,6 +29,7 @@
#include <linux/export.h>
#include <linux/debugfs.h>
#include <linux/seq_file.h>
+#include <linux/poll.h>
#include <linux/dmabuf-sync.h>
static inline int is_dma_buf_file(struct file *);
@@ -80,9 +81,89 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
return dmabuf->ops->mmap(dmabuf, vma);
}
+static unsigned int dma_buf_poll(struct file *filp,
+ struct poll_table_struct *poll)
+{
+ struct dma_buf *dmabuf;
+ struct dmabuf_sync_reservation *robj;
+ int ret = 0;
+
+ if (!is_dma_buf_file(filp))
+ return POLLERR;
+
+ dmabuf = filp->private_data;
+ if (!dmabuf || !dmabuf->sync)
+ return POLLERR;
+
+ robj = dmabuf->sync;
+
+ mutex_lock(&robj->lock);
+
+ robj->polled = true;
+
+ /*
+ * CPU or DMA access to this buffer has been completed, and
+ * the blocked task has been waked up. Return poll event
+ * so that the task can get out of select().
+ */
+ if (robj->poll_event) {
+ robj->poll_event = false;
+ mutex_unlock(&robj->lock);
+ return POLLIN | POLLOUT;
+ }
+
+ /*
+ * There is no anyone accessing this buffer so just return.
+ */
+ if (!robj->locked) {
+ mutex_unlock(&robj->lock);
+ return POLLIN | POLLOUT;
+ }
+
+ poll_wait(filp, &robj->poll_wait, poll);
+
+ mutex_unlock(&robj->lock);
+
+ return ret;
+}
+
+static int dma_buf_lock(struct file *file, int cmd, struct file_lock *fl)
+{
+ struct dma_buf *dmabuf;
+ unsigned int type;
+ bool wait = false;
+
+ if (!is_dma_buf_file(file))
+ return -EINVAL;
+
+ dmabuf = file->private_data;
+
+ if ((fl->fl_type & F_UNLCK) = F_UNLCK) {
+ dmabuf_sync_single_unlock(dmabuf);
+ return 0;
+ }
+
+ /* convert flock type to dmabuf sync type. */
+ if ((fl->fl_type & F_WRLCK) = F_WRLCK)
+ type = DMA_BUF_ACCESS_W;
+ else if ((fl->fl_type & F_RDLCK) = F_RDLCK)
+ type = DMA_BUF_ACCESS_R;
+ else
+ return -EINVAL;
+
+ if (fl->fl_flags & FL_SLEEP)
+ wait = true;
+
+ /* TODO. the locking to certain region should also be considered. */
+
+ return dmabuf_sync_single_lock(dmabuf, type, wait);
+}
+
static const struct file_operations dma_buf_fops = {
.release = dma_buf_release,
.mmap = dma_buf_mmap_internal,
+ .poll = dma_buf_poll,
+ .lock = dma_buf_lock,
};
/*
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH v2 2/2] dma-buf: Add user interfaces for dmabuf sync support
From: David Herrmann @ 2013-08-21 13:17 UTC (permalink / raw)
To: Inki Dae
Cc: dri-devel@lists.freedesktop.org, linux-fbdev@vger.kernel.org,
linux-arm-kernel, linux-media, linaro-kernel, Maarten Lankhorst,
Sumit Semwal, kyungmin.park, myungjoo.ham
In-Reply-To: <1377081215-5948-3-git-send-email-inki.dae@samsung.com>
Hi
On Wed, Aug 21, 2013 at 12:33 PM, Inki Dae <inki.dae@samsung.com> wrote:
> This patch adds lock and poll callbacks to dma buf file operations,
> and these callbacks will be called by fcntl and select system calls.
>
> fcntl and select system calls can be used to wait for the completion
> of DMA or CPU access to a shared dmabuf. The difference of them is
> fcntl system call takes a lock after the completion but select system
> call doesn't. So in case of fcntl system call, it's useful when a task
> wants to access a shared dmabuf without any broken. On the other hand,
> it's useful when a task wants to just wait for the completion.
1)
So how is that supposed to work in user-space? I don't want to block
on a buffer, but get notified once I can lock it. So I do:
select(..dmabuf..)
Once it is finished, I want to use it:
flock(..dmabuf..)
However, how can I guarantee the flock will not block? Some other
process might have locked it in between. So I do a non-blocking
flock() and if it fails I wait again? Looks ugly and un-predictable.
2)
What do I do if some user-space program holds a lock and dead-locks?
3)
How do we do modesetting in atomic-context in the kernel? There is no
way to lock the object. But this is required for panic-handlers and
more importantly the kdb debugging hooks.
Ok, I can live with that being racy, but would still be nice to be considered.
4)
Why do we need locks? Aren't fences enough? That is, in which
situation is a lock really needed?
If we assume we have two writers A and B (DMA, CPU, GPU, whatever) and
they have no synchronization on their own. What do we win by
synchronizing their writes? Ok, yeah, we end up with either A or B and
not a mixture of both. But if we cannot predict whether we get A or B,
I don't know why we care at all? It's random, so a mixture would be
fine, too, wouldn't it?
So if user-space doesn't have any synchronization on its own, I don't
see why we need an implicit sync on a dma-buf. Could you describe a
more elaborate use-case?
I think the problems we need to fix are read/write syncs. So we have a
write that issues the DMA+write plus a fence and passes the buf plus
fence to the reader. The reader waits for the fence and then issues
the read plus fence. It passes the fence back to the writer. The
writer waits for the fence again and then issues the next write if
required.
This has the following advantages:
- fences are _guaranteed_ to finish in a given time period. Locks, on
the other hand, might never be freed (of the holder dead-locks, for
instance)
- you avoid any stalls. That is, if a writer releases a buffer and
immediately locks it again, the reader side might stall if it didn't
lock it in exactly the given window. You have no control to guarantee
the reader ever gets access. You would need a synchronization in
user-space between the writer and reader to guarantee that. This makes
the whole lock useles, doesn't it?
Cheers
David
> Changelog v2:
> - Add select system call support.
> . The purpose of this feature is to wait for the completion of DMA or
> CPU access to a dmabuf without that caller locks the dmabuf again
> after the completion.
> That is useful when caller wants to be aware of the completion of
> DMA access to the dmabuf, and the caller doesn't use intefaces for
> the DMA device driver.
>
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
> drivers/base/dma-buf.c | 81 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 81 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
> index 4aca57a..f16a396 100644
> --- a/drivers/base/dma-buf.c
> +++ b/drivers/base/dma-buf.c
> @@ -29,6 +29,7 @@
> #include <linux/export.h>
> #include <linux/debugfs.h>
> #include <linux/seq_file.h>
> +#include <linux/poll.h>
> #include <linux/dmabuf-sync.h>
>
> static inline int is_dma_buf_file(struct file *);
> @@ -80,9 +81,89 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma)
> return dmabuf->ops->mmap(dmabuf, vma);
> }
>
> +static unsigned int dma_buf_poll(struct file *filp,
> + struct poll_table_struct *poll)
> +{
> + struct dma_buf *dmabuf;
> + struct dmabuf_sync_reservation *robj;
> + int ret = 0;
> +
> + if (!is_dma_buf_file(filp))
> + return POLLERR;
> +
> + dmabuf = filp->private_data;
> + if (!dmabuf || !dmabuf->sync)
> + return POLLERR;
> +
> + robj = dmabuf->sync;
> +
> + mutex_lock(&robj->lock);
> +
> + robj->polled = true;
> +
> + /*
> + * CPU or DMA access to this buffer has been completed, and
> + * the blocked task has been waked up. Return poll event
> + * so that the task can get out of select().
> + */
> + if (robj->poll_event) {
> + robj->poll_event = false;
> + mutex_unlock(&robj->lock);
> + return POLLIN | POLLOUT;
> + }
> +
> + /*
> + * There is no anyone accessing this buffer so just return.
> + */
> + if (!robj->locked) {
> + mutex_unlock(&robj->lock);
> + return POLLIN | POLLOUT;
> + }
> +
> + poll_wait(filp, &robj->poll_wait, poll);
> +
> + mutex_unlock(&robj->lock);
> +
> + return ret;
> +}
> +
> +static int dma_buf_lock(struct file *file, int cmd, struct file_lock *fl)
> +{
> + struct dma_buf *dmabuf;
> + unsigned int type;
> + bool wait = false;
> +
> + if (!is_dma_buf_file(file))
> + return -EINVAL;
> +
> + dmabuf = file->private_data;
> +
> + if ((fl->fl_type & F_UNLCK) = F_UNLCK) {
> + dmabuf_sync_single_unlock(dmabuf);
> + return 0;
> + }
> +
> + /* convert flock type to dmabuf sync type. */
> + if ((fl->fl_type & F_WRLCK) = F_WRLCK)
> + type = DMA_BUF_ACCESS_W;
> + else if ((fl->fl_type & F_RDLCK) = F_RDLCK)
> + type = DMA_BUF_ACCESS_R;
> + else
> + return -EINVAL;
> +
> + if (fl->fl_flags & FL_SLEEP)
> + wait = true;
> +
> + /* TODO. the locking to certain region should also be considered. */
> +
> + return dmabuf_sync_single_lock(dmabuf, type, wait);
> +}
> +
> static const struct file_operations dma_buf_fops = {
> .release = dma_buf_release,
> .mmap = dma_buf_mmap_internal,
> + .poll = dma_buf_poll,
> + .lock = dma_buf_lock,
> };
>
> /*
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox