* Re: conflict of hyperv_fb and the generic video driver?
From: David Herrmann @ 2013-08-16 19:11 UTC (permalink / raw)
To: Haiyang Zhang
Cc: linux-fbdev@vger.kernel.org,
driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <511c70f647a8481d90e8660773512efc@DFM-DB3MBX15-06.exchange.corp.microsoft.com>
Hi
(hint: your mail header seems to drop Reference-to/Reply-to headers
and breaks thread-info)
On Fri, Aug 16, 2013 at 8:45 PM, Haiyang Zhang <haiyangz@microsoft.com> wrote:
>
>
>> -----Original Message-----
>> From: David Herrmann [mailto:dh.herrmann@gmail.com]
>> Sent: Friday, August 16, 2013 9:46 AM
>> To: Haiyang Zhang
>> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
>> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
>> foundation.org; KY Srinivasan
>> Subject: Re: source code file for the generic vga driver?
>>
>> Hi
>>
>> On Mon, Aug 5, 2013 at 9:12 PM, Haiyang Zhang <haiyangz@microsoft.com>
>> wrote:
>> > Hi folks,
>> >
>> > I'm working on an issue of HyperV synthetic frame buffer driver, which
>> > seems to have a conflict with the generic vga driver (not the vesa
>> > driver). I hope to read and trace into the source code for the generic vga
>> driver...
>> >
>> > Can anyone point me to the source code file for the generic vga driver in
>> the kernel tree?
>>
>> Everything lives in ./drivers/video/. The drivers you're probably interested in
>> are "vesafb.c" or "vga16fb.c". There is also the "vgacon" driver
>> in ./drivers/video/console/vgacon.c. I am not sure which one you are talking
>> about.
>>
>> You might also want to have a look at the x86 sysfb infrastructure which isn't
>> merged, yet:
>> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86/fb
>> It provides proper platform-devices so drivers no longer conflict on the
>> vga/vesa/efi.. framebuffer resources. It's x86 only as all the relevant drivers
>> only work on x86.
>>
>> If you give some more information on what you are trying to do, I can point
>> you to the relevant resources. My guess is that you want to have a look at
>> remove_conflicting_framebuffers() in ./drivers/video/fbmem.c.
>
> Thank you for the detailed reply!
>
> I'm looking at a problem of Hyper-V synthetic fb driver (hyperv_fb) which seems
> to have conflict with some kind of generic video driver. I'm not sure which driver
> is this.
>
> On Suse, the vesafb is removed automatically by remove_conflicting_framebuffers()
> when hyperv_fb is loaded. We don't have any problem here.
>
> On some other Distros, like RHEL, CentOS, Ubuntu, the generic driver seems not
> to be vesafb -- I can't see any /dev/fb* there. And, the generic video driver seems not be
> removed when hyperv_fb is loaded. This generic video driver is not vesafb or vga16fb or
> vgacon, because it supports x-window GUI, and it's still here after I un-configured vesafb
> and vga16fb.
>
> Could point out what is the generic video driver used by RHEL, Ubuntu by default? And,
> how to let it exit automatically when our FB driver (hyperv_fb) is loaded?
I have no idea what RHEL or Ubuntu use, sorry. But your description
sounds odd. The kernel has 3 different places that could use VGA
resources:
- fbdev drivers (/dev/fbX or /sys/class/graphics)
- DRM drivers (/dev/dri/cardX or /sys/class/drm)
- vgacon
Could you check whether these directories are empty?
(/sys/class/graphics/ and /sys/class/drm/ on a running machine)
If these are empty, this doesn't look like a kernel thing. Are you
sure it's a kernel driver that accesses your VGA resources? The
xserver used to have some pretty huge vga/vesa/.. user-space drivers.
Could you tell me whether the linux-console actually works? That is,
do you get some console output if xserver is not running?
Cheers
David
^ permalink raw reply
* RE: [RFC 1/1] video: da8xx-fb: adding dt support
From: Etheridge, Darren @ 2013-08-16 20:11 UTC (permalink / raw)
To: Prabhakar Lad
Cc: devicetree@vger.kernel.org, Valkeinen, Tomi,
plagnioj@jcrosoft.com, linux-fbdev@vger.kernel.org,
Mohammed, Afzal
In-Reply-To: <CA+V-a8uSd7cypG=zTPyfkdq_xSdxHeGpoRu497U8C-3ZUsfu5w@mail.gmail.com>
From: Prabhakar Lad [mailto:prabhakar.csengg@gmail.com]
> Sent: Thursday, August 08, 2013 11:07 PM
> Subject: Re: [RFC 1/1] video: da8xx-fb: adding dt support
>
> Hi Darren,
>
> Thanks for the patch, below are few nits!
>
Prabhakar,
Thanks for reviewing, somehow I missed your reply so sorry about late response.
> On Fri, Aug 9, 2013 at 1:45 AM, Darren Etheridge <detheridge@ti.com>
> wrote:
> > Enhancing driver to enable probe triggered by a corresponding dt entry.
> >
<snip>
> > +
> > + cfg = devm_kzalloc(&dev->dev, sizeof(struct fb_videomode),
> GFP_KERNEL);
> > + if (!cfg) {
> > + dev_err(&dev->dev, "memory allocation failed\n");
>
> devm_* api will print the message if allocation has failed, please remove the
> above err statement and also similarly below.
Will fix.
<snip>
> > + lcdc_info = devm_kzalloc(&dev->dev,
> > + sizeof(struct fb_videomode),
> > + GFP_KERNEL);
> > + if (!lcdc_info) {
> > + dev_err(&dev->dev, "memory allocation
> > + failed\n");
> ditto
Will fix
<snip>
> > @@ -1390,7 +1439,7 @@ static int fb_probe(struct platform_device
> *device)
> > par->dev = &device->dev;
> > par->lcdc_clk = tmp_lcdc_clk;
> > par->lcdc_clk_rate = clk_get_rate(par->lcdc_clk);
> > - if (fb_pdata->panel_power_ctrl) {
> > + if (fb_pdata && fb_pdata->panel_power_ctrl) {
> > par->panel_power_ctrl = fb_pdata->panel_power_ctrl;
> > par->panel_power_ctrl(1);
>
> What happens to this data in DT case ?
>
Good find, right now we don't do any power control in the DT case - this is something that needs to be addressed.
>
> > }
> > @@ -1638,6 +1687,17 @@ static int fb_resume(struct platform_device
> > *dev) #define fb_resume NULL #endif
> >
> > +static const struct of_device_id da8xx_fb_of_match[] = {
> > + /*
> > + * this driver supports version 1 and version 2 of the
> > + * Texas Instruments lcd controller (lcdc) hardware block
> > + */
> > + {.compatible = "ti,da830-lcdc", },
> > + {.compatible = "ti,am3352-lcdc", },
> > + {},
> > +};
> > +MODULE_DEVICE_TABLE(of, da8xx_fb_of_match);
> > +
> you can bound this structure and the macro in IS_ENABLED(CONFIG_OF)
Ok will look at that.
Thanks,
Darren
^ permalink raw reply
* RE: conflict of hyperv_fb and the generic video driver?
From: Haiyang Zhang @ 2013-08-16 20:27 UTC (permalink / raw)
To: David Herrmann
Cc: linux-fbdev@vger.kernel.org,
driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <CANq1E4SsnQfRNpgcNW12t26MeQvVVS9p136dGTirPaJZKxXo_Q@mail.gmail.com>
> -----Original Message-----
> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> Sent: Friday, August 16, 2013 3:11 PM
> To: Haiyang Zhang
> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> foundation.org; KY Srinivasan
> Subject: Re: conflict of hyperv_fb and the generic video driver?
>
> Hi
>
> (hint: your mail header seems to drop Reference-to/Reply-to headers
> and breaks thread-info)
>
> On Fri, Aug 16, 2013 at 8:45 PM, Haiyang Zhang <haiyangz@microsoft.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> >> Sent: Friday, August 16, 2013 9:46 AM
> >> To: Haiyang Zhang
> >> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
> >> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> >> foundation.org; KY Srinivasan
> >> Subject: Re: source code file for the generic vga driver?
> >>
> >> Hi
> >>
> >> On Mon, Aug 5, 2013 at 9:12 PM, Haiyang Zhang
> <haiyangz@microsoft.com>
> >> wrote:
> >> > Hi folks,
> >> >
> >> > I'm working on an issue of HyperV synthetic frame buffer driver, which
> >> > seems to have a conflict with the generic vga driver (not the vesa
> >> > driver). I hope to read and trace into the source code for the generic vga
> >> driver...
> >> >
> >> > Can anyone point me to the source code file for the generic vga driver in
> >> the kernel tree?
> >>
> >> Everything lives in ./drivers/video/. The drivers you're probably interested
> in
> >> are "vesafb.c" or "vga16fb.c". There is also the "vgacon" driver
> >> in ./drivers/video/console/vgacon.c. I am not sure which one you are
> talking
> >> about.
> >>
> >> You might also want to have a look at the x86 sysfb infrastructure which
> isn't
> >> merged, yet:
> >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86/fb
> >> It provides proper platform-devices so drivers no longer conflict on the
> >> vga/vesa/efi.. framebuffer resources. It's x86 only as all the relevant
> drivers
> >> only work on x86.
> >>
> >> If you give some more information on what you are trying to do, I can
> point
> >> you to the relevant resources. My guess is that you want to have a look at
> >> remove_conflicting_framebuffers() in ./drivers/video/fbmem.c.
> >
> > Thank you for the detailed reply!
> >
> > I'm looking at a problem of Hyper-V synthetic fb driver (hyperv_fb) which
> seems
> > to have conflict with some kind of generic video driver. I'm not sure which
> driver
> > is this.
> >
> > On Suse, the vesafb is removed automatically by
> remove_conflicting_framebuffers()
> > when hyperv_fb is loaded. We don't have any problem here.
> >
> > On some other Distros, like RHEL, CentOS, Ubuntu, the generic driver
> seems not
> > to be vesafb -- I can't see any /dev/fb* there. And, the generic video driver
> seems not be
> > removed when hyperv_fb is loaded. This generic video driver is not vesafb
> or vga16fb or
> > vgacon, because it supports x-window GUI, and it's still here after I un-
> configured vesafb
> > and vga16fb.
> >
> > Could point out what is the generic video driver used by RHEL, Ubuntu by
> default? And,
> > how to let it exit automatically when our FB driver (hyperv_fb) is loaded?
>
> I have no idea what RHEL or Ubuntu use, sorry. But your description
> sounds odd. The kernel has 3 different places that could use VGA
> resources:
> - fbdev drivers (/dev/fbX or /sys/class/graphics)
> - DRM drivers (/dev/dri/cardX or /sys/class/drm)
> - vgacon
> Could you check whether these directories are empty?
> (/sys/class/graphics/ and /sys/class/drm/ on a running machine)
>
> If these are empty, this doesn't look like a kernel thing. Are you
> sure it's a kernel driver that accesses your VGA resources? The
> xserver used to have some pretty huge vga/vesa/.. user-space drivers.
>
> Could you tell me whether the linux-console actually works? That is,
> do you get some console output if xserver is not running?
To find out the default driver, I manually removed my hyperv_fb driver.
The vesafb is unconfigured:
# CONFIG_FB_BOOT_VESA_SUPPORT is not set
# CONFIG_FB_UVESA is not set
# CONFIG_FB_VESA is not set
But I saw VESA VBE in the x log. Seems it's the default driver:
"/var/log/Xorg.0.log":
[ 12.340] (II) VESA(0): Primary V_BIOS segment is: 0xc000
[ 12.341] (II) VESA(0): VESA BIOS detected
[ 12.341] (II) VESA(0): VESA VBE Version 2.0
[ 12.341] (II) VESA(0): VESA VBE Total Mem: 4096 kB
[ 12.341] (II) VESA(0): VESA VBE OEM: IBM SVGA BIOS, (C) 1993 International Business Machines
[ 12.341] (II) VESA(0): VESA VBE OEM Software Rev: 0.0
[ 12.365] (II) VESA(0): Creating default Display subsection in Screen section
"Default Screen Section" for depth/fbbpp 24/32
[ 12.365] (=) VESA(0): Depth 24, (--) framebuffer bpp 32
[ 12.365] (=) VESA(0): RGB weight 888
[ 12.365] (=) VESA(0): Default visual is TrueColor
[ 12.365] (=) VESA(0): Using gamma correction (1.0, 1.0, 1.0)
There is no /dev/fb*, /dev/dri/, /sys/class/drm
I see /sys/class/graphics/fbcon is here. But console output is not working.
Seems that the VESA VBE is causing conflict with my driver... Is there
any way to disable VESA VBE driver?
Thanks,
- Haiyang
^ permalink raw reply
* Re: [PATCH v2] efifb: prevent null dereferences by removing unused array indices from dmi_list
From: David Herrmann @ 2013-08-16 20:36 UTC (permalink / raw)
To: James Bates
Cc: Peter Jones, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
linux-fbdev@vger.kernel.org, linux-kernel, H. Peter Anvin
In-Reply-To: <1376684395.6529.5.camel@hermes>
Hi
On Fri, Aug 16, 2013 at 10:19 PM, James Bates <james.h.bates@gmail.com> wrote:
> On Fri, 2013-08-16 at 15:37 +0200, David Herrmann wrote:
>
> Hi
> (CC'ing hpa)
>
> Yepp, this patch looks good:
> Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
>
> However, I'd like to see some code to prevent this from happening
> again. It isn't really obvious that removing an entry will result in a
> NULL-deref. I am not the maintainer of this code, but I'd really like
> to see a "(dmi_list[i].optname && !strcmp())" check in efifb_setup().
> Otherwise we _will_ run into this again.
>
> Side note: this code got moved to arch/x86/kernel/sysfb_efi.c in the
> x86 tree. I am adding hpa here so he will remember this once Linus
> gets a merge conflict (iff the sysfb changes get merged through the
> x86 tree).
>
> Thanks
> David
>
> Sure thing, here is the patch again, with the extra check in efifb_setup()
> (it turns out one can simply
> switch around the order of the checks: implicitly initialized dmi_list items
> will have base = 0):
You can put such comments below the "---" in your patch so they will
not show up in the commit-message (between commit message and the
diffstat).
> Full patch v2:
>
> the dmi_list array is initialized using gnu designated initializers, and
> therefore
> contains fewer explicitly defined entries as there are elements in it. This
> is because the enum above with M_blabla constants contains more items than
> the designated
> initializer. Those elements not explicitly initialized are implicitly set to
> 0.
>
> Now efifb_setup(), L.322 & L.323, loops through all these array elements,
> and performs
> a strcmp o a field (optname) in each item. For non explicitly initialized
> elements this
> will be a null pointer:
>
> for (i = 0; i < M_UNKNOWN; i++) {
> if (!strcmp(this_opt, dmi_list[i].optname)
> &&
>
> On my macbook6,1 the predefined values are for some reason incorrect, and
> most parameters
> are preset correctly by my efi bootloader (elilo). but stride/line_length is
> not detected
> correctly and so I wish to set it explicitly using a
> "videoïifb:stride:2048" command-line
> argument. Because of the above null dereference, an exception (presumably)
> occurs before
> the parsing code (L.333) is ever reached. I say presumably since the mac
> hangs on boot
> without a console, and I can therefore not see any output.
>
> By removing the unused values from the enum, and thus preventing implicitly
> initialized items
> in the dmi_list array, the null dereference does not occur, my customer
> command-line arg is
> parsed correctly, and my console displays correctly.
>
> This patch removes the unused enum values, and also guards against any
> future implicit
> initializing by inverting the check order in the if statement, and checking
> first whether
> dmi_list[i].base is null.
>
> Signed-off-by: James Bates <james.h.bates@yahoo.com>
> ---
> drivers/video/efifb.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
> index 50fe668..161757b 100644
>
> --- a/drivers/video/efifb.c
> +++ b/drivers/video/efifb.c
> @@ -50,12 +50,9 @@ enum {
> M_MINI_3_1, /* Mac Mini, 3,1th gen */
> M_MINI_4_1, /* Mac Mini, 4,1th gen */
> M_MB, /* MacBook */
> - M_MB_2, /* MacBook, 2nd rev. */
> - M_MB_3, /* MacBook, 3rd rev. */
> M_MB_5_1, /* MacBook, 5th rev. */
> M_MB_6_1, /* MacBook, 6th rev. */
> M_MB_7_1, /* MacBook, 7th rev. */
> - M_MB_SR, /* MacBook, 2nd gen, (Santa Rosa) */
> M_MBA, /* MacBook Air */
> M_MBA_3, /* Macbook Air, 3rd rev */
> M_MBP, /* MacBook Pro */
> @@ -323,8 +320,8 @@ static int __init efifb_setup(char *options)
> if (!*this_opt) continue;
>
>
> for (i = 0; i < M_UNKNOWN; i++) {
> - if (!strcmp(this_opt, dmi_list[i].optname) &&
> - dmi_list[i].base != 0) {
> + if (dmi_list[i].base != 0 &&
From a quick glance, M_MBA_3 has "base = 0" but is a valid entry. Hm,
I think we can be sure that there will never be a framebuffer at phys
0, so yeah, I am fine with this.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Regards
David
> + !strcmp(this_opt, dmi_list[i].optname)) {
> screen_info.lfb_base = dmi_list[i].base;
> screen_info.lfb_linelength = dmi_list[i].stride;
> screen_info.lfb_width = dmi_list[i].width;
> --
>
^ permalink raw reply
* Re: conflict of hyperv_fb and the generic video driver?
From: David Herrmann @ 2013-08-16 20:40 UTC (permalink / raw)
To: Haiyang Zhang
Cc: linux-fbdev@vger.kernel.org,
driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <c4a33a56e2844e5b8bda5bcfc2535aa4@DFM-DB3MBX15-06.exchange.corp.microsoft.com>
Hi
On Fri, Aug 16, 2013 at 10:27 PM, Haiyang Zhang <haiyangz@microsoft.com> wrote:
>
>
>> -----Original Message-----
>> From: David Herrmann [mailto:dh.herrmann@gmail.com]
>> Sent: Friday, August 16, 2013 3:11 PM
>> To: Haiyang Zhang
>> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
>> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
>> foundation.org; KY Srinivasan
>> Subject: Re: conflict of hyperv_fb and the generic video driver?
>>
>> Hi
>>
>> (hint: your mail header seems to drop Reference-to/Reply-to headers
>> and breaks thread-info)
>>
>> On Fri, Aug 16, 2013 at 8:45 PM, Haiyang Zhang <haiyangz@microsoft.com>
>> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: David Herrmann [mailto:dh.herrmann@gmail.com]
>> >> Sent: Friday, August 16, 2013 9:46 AM
>> >> To: Haiyang Zhang
>> >> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
>> >> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
>> >> foundation.org; KY Srinivasan
>> >> Subject: Re: source code file for the generic vga driver?
>> >>
>> >> Hi
>> >>
>> >> On Mon, Aug 5, 2013 at 9:12 PM, Haiyang Zhang
>> <haiyangz@microsoft.com>
>> >> wrote:
>> >> > Hi folks,
>> >> >
>> >> > I'm working on an issue of HyperV synthetic frame buffer driver, which
>> >> > seems to have a conflict with the generic vga driver (not the vesa
>> >> > driver). I hope to read and trace into the source code for the generic vga
>> >> driver...
>> >> >
>> >> > Can anyone point me to the source code file for the generic vga driver in
>> >> the kernel tree?
>> >>
>> >> Everything lives in ./drivers/video/. The drivers you're probably interested
>> in
>> >> are "vesafb.c" or "vga16fb.c". There is also the "vgacon" driver
>> >> in ./drivers/video/console/vgacon.c. I am not sure which one you are
>> talking
>> >> about.
>> >>
>> >> You might also want to have a look at the x86 sysfb infrastructure which
>> isn't
>> >> merged, yet:
>> >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86/fb
>> >> It provides proper platform-devices so drivers no longer conflict on the
>> >> vga/vesa/efi.. framebuffer resources. It's x86 only as all the relevant
>> drivers
>> >> only work on x86.
>> >>
>> >> If you give some more information on what you are trying to do, I can
>> point
>> >> you to the relevant resources. My guess is that you want to have a look at
>> >> remove_conflicting_framebuffers() in ./drivers/video/fbmem.c.
>> >
>> > Thank you for the detailed reply!
>> >
>> > I'm looking at a problem of Hyper-V synthetic fb driver (hyperv_fb) which
>> seems
>> > to have conflict with some kind of generic video driver. I'm not sure which
>> driver
>> > is this.
>> >
>> > On Suse, the vesafb is removed automatically by
>> remove_conflicting_framebuffers()
>> > when hyperv_fb is loaded. We don't have any problem here.
>> >
>> > On some other Distros, like RHEL, CentOS, Ubuntu, the generic driver
>> seems not
>> > to be vesafb -- I can't see any /dev/fb* there. And, the generic video driver
>> seems not be
>> > removed when hyperv_fb is loaded. This generic video driver is not vesafb
>> or vga16fb or
>> > vgacon, because it supports x-window GUI, and it's still here after I un-
>> configured vesafb
>> > and vga16fb.
>> >
>> > Could point out what is the generic video driver used by RHEL, Ubuntu by
>> default? And,
>> > how to let it exit automatically when our FB driver (hyperv_fb) is loaded?
>>
>> I have no idea what RHEL or Ubuntu use, sorry. But your description
>> sounds odd. The kernel has 3 different places that could use VGA
>> resources:
>> - fbdev drivers (/dev/fbX or /sys/class/graphics)
>> - DRM drivers (/dev/dri/cardX or /sys/class/drm)
>> - vgacon
>> Could you check whether these directories are empty?
>> (/sys/class/graphics/ and /sys/class/drm/ on a running machine)
>>
>> If these are empty, this doesn't look like a kernel thing. Are you
>> sure it's a kernel driver that accesses your VGA resources? The
>> xserver used to have some pretty huge vga/vesa/.. user-space drivers.
>>
>> Could you tell me whether the linux-console actually works? That is,
>> do you get some console output if xserver is not running?
>
> To find out the default driver, I manually removed my hyperv_fb driver.
> The vesafb is unconfigured:
> # CONFIG_FB_BOOT_VESA_SUPPORT is not set
> # CONFIG_FB_UVESA is not set
> # CONFIG_FB_VESA is not set
>
> But I saw VESA VBE in the x log. Seems it's the default driver:
> "/var/log/Xorg.0.log":
> [ 12.340] (II) VESA(0): Primary V_BIOS segment is: 0xc000
> [ 12.341] (II) VESA(0): VESA BIOS detected
> [ 12.341] (II) VESA(0): VESA VBE Version 2.0
> [ 12.341] (II) VESA(0): VESA VBE Total Mem: 4096 kB
> [ 12.341] (II) VESA(0): VESA VBE OEM: IBM SVGA BIOS, (C) 1993 International Business Machines
> [ 12.341] (II) VESA(0): VESA VBE OEM Software Rev: 0.0
> [ 12.365] (II) VESA(0): Creating default Display subsection in Screen section
> "Default Screen Section" for depth/fbbpp 24/32
> [ 12.365] (=) VESA(0): Depth 24, (--) framebuffer bpp 32
> [ 12.365] (=) VESA(0): RGB weight 888
> [ 12.365] (=) VESA(0): Default visual is TrueColor
> [ 12.365] (=) VESA(0): Using gamma correction (1.0, 1.0, 1.0)
>
> There is no /dev/fb*, /dev/dri/, /sys/class/drm
> I see /sys/class/graphics/fbcon is here. But console output is not working.
>
> Seems that the VESA VBE is causing conflict with my driver... Is there
> any way to disable VESA VBE driver?
This is the Xorg vesa driver. There is no way to detect that from the
kernel (at least no sane way). Just uninstall the vesa driver, no one
uses that these days. At least I see no reason why you would use it.
Probably named xf86-video-vesa. Or make sure your hyperv fbdev driver
is loaded before xorg starts and then load xf86-video-fbdev over
xf86-video-vesa.
Regards
David
^ permalink raw reply
* RE: conflict of hyperv_fb and the generic video driver?
From: Haiyang Zhang @ 2013-08-16 23:04 UTC (permalink / raw)
To: David Herrmann
Cc: linux-fbdev@vger.kernel.org,
driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <CANq1E4S9kwDj_J0tu8pgXOVJhvokEzCFNvs2VTQfxA21GHgfGQ@mail.gmail.com>
> -----Original Message-----
> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> Sent: Friday, August 16, 2013 4:40 PM
> To: Haiyang Zhang
> Cc: linux-fbdev@vger.kernel.org; driverdev-devel@linuxdriverproject.org;
> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> foundation.org; KY Srinivasan
> Subject: Re: conflict of hyperv_fb and the generic video driver?
>
> Hi
>
> On Fri, Aug 16, 2013 at 10:27 PM, Haiyang Zhang <haiyangz@microsoft.com>
> wrote:
> >
> >
> >> -----Original Message-----
> >> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> >> Sent: Friday, August 16, 2013 3:11 PM
> >> To: Haiyang Zhang
> >> Cc: linux-fbdev@vger.kernel.org;
> >> driverdev-devel@linuxdriverproject.org;
> >> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> >> foundation.org; KY Srinivasan
> >> Subject: Re: conflict of hyperv_fb and the generic video driver?
> >>
> >> Hi
> >>
> >> (hint: your mail header seems to drop Reference-to/Reply-to headers
> >> and breaks thread-info)
> >>
> >> On Fri, Aug 16, 2013 at 8:45 PM, Haiyang Zhang
> >> <haiyangz@microsoft.com>
> >> wrote:
> >> >
> >> >
> >> >> -----Original Message-----
> >> >> From: David Herrmann [mailto:dh.herrmann@gmail.com]
> >> >> Sent: Friday, August 16, 2013 9:46 AM
> >> >> To: Haiyang Zhang
> >> >> Cc: linux-fbdev@vger.kernel.org;
> >> >> driverdev-devel@linuxdriverproject.org;
> >> >> Tomi Valkeinen; Jean-Christophe Plagniol-Villard; akpm@linux-
> >> >> foundation.org; KY Srinivasan
> >> >> Subject: Re: source code file for the generic vga driver?
> >> >>
> >> >> Hi
> >> >>
> >> >> On Mon, Aug 5, 2013 at 9:12 PM, Haiyang Zhang
> >> <haiyangz@microsoft.com>
> >> >> wrote:
> >> >> > Hi folks,
> >> >> >
> >> >> > I'm working on an issue of HyperV synthetic frame buffer driver,
> >> >> > which seems to have a conflict with the generic vga driver (not
> >> >> > the vesa driver). I hope to read and trace into the source code
> >> >> > for the generic vga
> >> >> driver...
> >> >> >
> >> >> > Can anyone point me to the source code file for the generic vga
> >> >> > driver in
> >> >> the kernel tree?
> >> >>
> >> >> Everything lives in ./drivers/video/. The drivers you're probably
> >> >> interested
> >> in
> >> >> are "vesafb.c" or "vga16fb.c". There is also the "vgacon" driver
> >> >> in ./drivers/video/console/vgacon.c. I am not sure which one you
> >> >> are
> >> talking
> >> >> about.
> >> >>
> >> >> You might also want to have a look at the x86 sysfb infrastructure
> >> >> which
> >> isn't
> >> >> merged, yet:
> >> >> http://git.kernel.org/cgit/linux/kernel/git/tip/tip.git/log/?h=x86
> >> >> /fb It provides proper platform-devices so drivers no longer
> >> >> conflict on the vga/vesa/efi.. framebuffer resources. It's x86
> >> >> only as all the relevant
> >> drivers
> >> >> only work on x86.
> >> >>
> >> >> If you give some more information on what you are trying to do, I
> >> >> can
> >> point
> >> >> you to the relevant resources. My guess is that you want to have a
> >> >> look at
> >> >> remove_conflicting_framebuffers() in ./drivers/video/fbmem.c.
> >> >
> >> > Thank you for the detailed reply!
> >> >
> >> > I'm looking at a problem of Hyper-V synthetic fb driver (hyperv_fb)
> >> > which
> >> seems
> >> > to have conflict with some kind of generic video driver. I'm not
> >> > sure which
> >> driver
> >> > is this.
> >> >
> >> > On Suse, the vesafb is removed automatically by
> >> remove_conflicting_framebuffers()
> >> > when hyperv_fb is loaded. We don't have any problem here.
> >> >
> >> > On some other Distros, like RHEL, CentOS, Ubuntu, the generic
> >> > driver
> >> seems not
> >> > to be vesafb -- I can't see any /dev/fb* there. And, the generic
> >> > video driver
> >> seems not be
> >> > removed when hyperv_fb is loaded. This generic video driver is not
> >> > vesafb
> >> or vga16fb or
> >> > vgacon, because it supports x-window GUI, and it's still here after
> >> > I un-
> >> configured vesafb
> >> > and vga16fb.
> >> >
> >> > Could point out what is the generic video driver used by RHEL,
> >> > Ubuntu by
> >> default? And,
> >> > how to let it exit automatically when our FB driver (hyperv_fb) is loaded?
> >>
> >> I have no idea what RHEL or Ubuntu use, sorry. But your description
> >> sounds odd. The kernel has 3 different places that could use VGA
> >> resources:
> >> - fbdev drivers (/dev/fbX or /sys/class/graphics)
> >> - DRM drivers (/dev/dri/cardX or /sys/class/drm)
> >> - vgacon
> >> Could you check whether these directories are empty?
> >> (/sys/class/graphics/ and /sys/class/drm/ on a running machine)
> >>
> >> If these are empty, this doesn't look like a kernel thing. Are you
> >> sure it's a kernel driver that accesses your VGA resources? The
> >> xserver used to have some pretty huge vga/vesa/.. user-space drivers.
> >>
> >> Could you tell me whether the linux-console actually works? That is,
> >> do you get some console output if xserver is not running?
> >
> > To find out the default driver, I manually removed my hyperv_fb driver.
> > The vesafb is unconfigured:
> > # CONFIG_FB_BOOT_VESA_SUPPORT is not set # CONFIG_FB_UVESA is
> not set
> > # CONFIG_FB_VESA is not set
> >
> > But I saw VESA VBE in the x log. Seems it's the default driver:
> > "/var/log/Xorg.0.log":
> > [ 12.340] (II) VESA(0): Primary V_BIOS segment is: 0xc000
> > [ 12.341] (II) VESA(0): VESA BIOS detected
> > [ 12.341] (II) VESA(0): VESA VBE Version 2.0
> > [ 12.341] (II) VESA(0): VESA VBE Total Mem: 4096 kB
> > [ 12.341] (II) VESA(0): VESA VBE OEM: IBM SVGA BIOS, (C) 1993
> International Business Machines
> > [ 12.341] (II) VESA(0): VESA VBE OEM Software Rev: 0.0
> > [ 12.365] (II) VESA(0): Creating default Display subsection in Screen
> section
> > "Default Screen Section" for depth/fbbpp 24/32
> > [ 12.365] (=) VESA(0): Depth 24, (--) framebuffer bpp 32
> > [ 12.365] (=) VESA(0): RGB weight 888
> > [ 12.365] (=) VESA(0): Default visual is TrueColor
> > [ 12.365] (=) VESA(0): Using gamma correction (1.0, 1.0, 1.0)
> >
> > There is no /dev/fb*, /dev/dri/, /sys/class/drm I see
> > /sys/class/graphics/fbcon is here. But console output is not working.
> >
> > Seems that the VESA VBE is causing conflict with my driver... Is there
> > any way to disable VESA VBE driver?
>
> This is the Xorg vesa driver. There is no way to detect that from the kernel (at
> least no sane way). Just uninstall the vesa driver, no one uses that these days.
> At least I see no reason why you would use it.
> Probably named xf86-video-vesa. Or make sure your hyperv fbdev driver is
> loaded before xorg starts and then load xf86-video-fbdev over xf86-video-
> vesa.
Removing the xorg vesa driver has solved the conflict.
Also, I found adding a xorg.conf which specifies fbdev can solve the problem too:
[root@localhost ~]# cat /etc/X11/xorg.conf
Section "Device"
Identifier "HYPER-V Framebuffer"
Driver "fbdev"
EndSection
Thank you SO MUCH for the help!!
- Haiyang
^ permalink raw reply
* Re: conflict of hyperv_fb and the generic video driver?
From: David Herrmann @ 2013-08-16 23:11 UTC (permalink / raw)
To: Haiyang Zhang
Cc: linux-fbdev@vger.kernel.org,
driverdev-devel@linuxdriverproject.org, Tomi Valkeinen,
akpm@linux-foundation.org, Jean-Christophe Plagniol-Villard
In-Reply-To: <34df20beb0de41f4b52ec4a2177e8f3e@DFM-DB3MBX15-06.exchange.corp.microsoft.com>
Hi
On Sat, Aug 17, 2013 at 1:04 AM, Haiyang Zhang <haiyangz@microsoft.com> wrote:
>> > But I saw VESA VBE in the x log. Seems it's the default driver:
>> > "/var/log/Xorg.0.log":
>> > [ 12.340] (II) VESA(0): Primary V_BIOS segment is: 0xc000
>> > [ 12.341] (II) VESA(0): VESA BIOS detected
>> > [ 12.341] (II) VESA(0): VESA VBE Version 2.0
>> > [ 12.341] (II) VESA(0): VESA VBE Total Mem: 4096 kB
>> > [ 12.341] (II) VESA(0): VESA VBE OEM: IBM SVGA BIOS, (C) 1993
>> International Business Machines
>> > [ 12.341] (II) VESA(0): VESA VBE OEM Software Rev: 0.0
>> > [ 12.365] (II) VESA(0): Creating default Display subsection in Screen
>> section
>> > "Default Screen Section" for depth/fbbpp 24/32
>> > [ 12.365] (=) VESA(0): Depth 24, (--) framebuffer bpp 32
>> > [ 12.365] (=) VESA(0): RGB weight 888
>> > [ 12.365] (=) VESA(0): Default visual is TrueColor
>> > [ 12.365] (=) VESA(0): Using gamma correction (1.0, 1.0, 1.0)
>> >
>> > There is no /dev/fb*, /dev/dri/, /sys/class/drm I see
>> > /sys/class/graphics/fbcon is here. But console output is not working.
>> >
>> > Seems that the VESA VBE is causing conflict with my driver... Is there
>> > any way to disable VESA VBE driver?
>>
>> This is the Xorg vesa driver. There is no way to detect that from the kernel (at
>> least no sane way). Just uninstall the vesa driver, no one uses that these days.
>> At least I see no reason why you would use it.
>> Probably named xf86-video-vesa. Or make sure your hyperv fbdev driver is
>> loaded before xorg starts and then load xf86-video-fbdev over xf86-video-
>> vesa.
>
> Removing the xorg vesa driver has solved the conflict.
>
> Also, I found adding a xorg.conf which specifies fbdev can solve the problem too:
> [root@localhost ~]# cat /etc/X11/xorg.conf
> Section "Device"
> Identifier "HYPER-V Framebuffer"
> Driver "fbdev"
> EndSection
>
> Thank you SO MUCH for the help!!
You're welcome. Good to hear it's solved now.
Cheers
David
^ permalink raw reply
* efifb: patch to allow userspace to unbind efi framebuffer driver from VGA device
From: Jonathan Campbell @ 2013-08-19 4:22 UTC (permalink / raw)
To: linux-fbdev
[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]
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.
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.
[-- Attachment #2: efifb-bind-ctl.patch --]
[-- Type: text/plain, Size: 5950 bytes --]
--- a/drivers/video/efifb.c 2013-08-15 05:59:42.000000000 +0000
+++ b/drivers/video/efifb.c 2013-08-17 23:56:56.929260269 +0000
@@ -18,7 +18,8 @@
static bool request_mem_succeeded = false;
-static struct pci_dev *default_vga;
+static struct pci_dev *default_vga = NULL;
+static struct fb_info *efifb_info = NULL;
static struct fb_var_screeninfo efifb_defined = {
.activate = FB_ACTIVATE_NOW,
@@ -86,7 +87,7 @@
int width;
int height;
int flags;
-} dmi_list[] __initdata = {
+} dmi_list[] = {
[M_I17] = { "i17", 0x80010000, 1472 * 4, 1440, 900, OVERRIDE_NONE },
[M_I20] = { "i20", 0x80010000, 1728 * 4, 1680, 1050, OVERRIDE_NONE }, /* guess */
[M_I20_SR] = { "imac7", 0x40010000, 1728 * 4, 1680, 1050, OVERRIDE_NONE },
@@ -127,7 +128,7 @@
DMI_MATCH(DMI_PRODUCT_NAME, name) }, \
&dmi_list[enumid] }
-static const struct dmi_system_id dmi_system_table[] __initconst = {
+static const struct dmi_system_id dmi_system_table[] = {
EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "iMac4,1", M_I17),
/* At least one of these two will be right; maybe both? */
EFIFB_DMI_SYSTEM_ID("Apple Computer, Inc.", "iMac5,1", M_I20),
@@ -288,6 +289,7 @@
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);
}
@@ -312,7 +314,7 @@
default_vga = pdev;
}
-static int __init efifb_setup(char *options)
+static int efifb_setup(char *options)
{
char *this_opt;
int i;
@@ -369,9 +371,8 @@
return 0;
}
-static int __init efifb_probe(struct platform_device *dev)
+static int efifb_probe(struct platform_device *dev)
{
- struct fb_info *info;
int err;
unsigned int size_vmode;
unsigned int size_remap;
@@ -438,25 +439,25 @@
efifb_fix.smem_start);
}
- info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
- if (!info) {
+ efifb_info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
+ if (!efifb_info) {
printk(KERN_ERR "efifb: cannot allocate framebuffer\n");
err = -ENOMEM;
goto err_release_mem;
}
- info->pseudo_palette = info->par;
- info->par = NULL;
+ efifb_info->pseudo_palette = efifb_info->par;
+ efifb_info->par = NULL;
- info->apertures = alloc_apertures(1);
- if (!info->apertures) {
+ efifb_info->apertures = alloc_apertures(1);
+ if (!efifb_info->apertures) {
err = -ENOMEM;
goto err_release_fb;
}
- info->apertures->ranges[0].base = efifb_fix.smem_start;
- info->apertures->ranges[0].size = size_remap;
+ efifb_info->apertures->ranges[0].base = efifb_fix.smem_start;
+ efifb_info->apertures->ranges[0].size = size_remap;
- info->screen_base = ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
- if (!info->screen_base) {
+ efifb_info->screen_base = ioremap_wc(efifb_fix.smem_start, efifb_fix.smem_len);
+ if (!efifb_info->screen_base) {
printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
"0x%x @ 0x%lx\n",
efifb_fix.smem_len, efifb_fix.smem_start);
@@ -466,7 +467,7 @@
printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, "
"using %dk, total %dk\n",
- efifb_fix.smem_start, info->screen_base,
+ efifb_fix.smem_start, efifb_info->screen_base,
size_remap/1024, size_total/1024);
printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
efifb_defined.xres, efifb_defined.yres,
@@ -509,29 +510,29 @@
efifb_fix.ypanstep = 0;
efifb_fix.ywrapstep = 0;
- info->fbops = &efifb_ops;
- info->var = efifb_defined;
- info->fix = efifb_fix;
- info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
+ efifb_info->fbops = &efifb_ops;
+ efifb_info->var = efifb_defined;
+ efifb_info->fix = efifb_fix;
+ efifb_info->flags = FBINFO_FLAG_DEFAULT | FBINFO_MISC_FIRMWARE;
- if ((err = fb_alloc_cmap(&info->cmap, 256, 0)) < 0) {
+ if ((err = fb_alloc_cmap(&efifb_info->cmap, 256, 0)) < 0) {
printk(KERN_ERR "efifb: cannot allocate colormap\n");
goto err_unmap;
}
- if ((err = register_framebuffer(info)) < 0) {
+ if ((err = register_framebuffer(efifb_info)) < 0) {
printk(KERN_ERR "efifb: cannot register framebuffer\n");
goto err_fb_dealoc;
}
printk(KERN_INFO "fb%d: %s frame buffer device\n",
- info->node, info->fix.id);
+ efifb_info->node, efifb_info->fix.id);
return 0;
err_fb_dealoc:
- fb_dealloc_cmap(&info->cmap);
+ fb_dealloc_cmap(&efifb_info->cmap);
err_unmap:
- iounmap(info->screen_base);
+ iounmap(efifb_info->screen_base);
err_release_fb:
- framebuffer_release(info);
+ framebuffer_release(efifb_info);
err_release_mem:
if (request_mem_succeeded)
release_mem_region(efifb_fix.smem_start, size_total);
@@ -548,6 +549,38 @@
.name = "efifb",
};
+/* use a sysfs file "trace_running" to start/stop tracing */
+static ssize_t efifb_bind_fb_attr_show(struct device *dev,struct device_attribute *attr,char *buf)
+{
+ return sprintf(buf, "%u\n", efifb_info != NULL ? 1 : 0);
+}
+
+static ssize_t efifb_bind_fb_attr_store(struct device *dev,struct device_attribute *attr,const char *buf,size_t n)
+{
+ unsigned int value;
+ int ret = 0;
+
+ if (sscanf(buf, "%u", &value) != 1)
+ return -EINVAL;
+ if (value > 1)
+ return -EINVAL;
+
+ if ((efifb_info != NULL ? 1 : 0) != value) {
+ if (value)
+ ret = efifb_probe(&efifb_device);
+ else {
+ ret = unregister_framebuffer(efifb_info);
+ if (ret >= 0) efifb_info = NULL;
+ }
+ }
+
+ return (ret < 0 ? ret : n);
+}
+
+static const struct device_attribute efifb_bind_fb_attr =
+ __ATTR(bind_fb, 0644, efifb_bind_fb_attr_show, efifb_bind_fb_attr_store);
+
+
static int __init efifb_init(void)
{
int ret;
@@ -575,6 +608,11 @@
if (ret)
return ret;
+ /* create "bind_fb" sysfs file */
+ if (device_create_file(&efifb_device.dev,&efifb_bind_fb_attr))
+ printk(KERN_WARNING
+ "efifb: cannot create bind_fb attribute\n");
+
/*
* This is not just an optimization. We will interfere
* with a real driver if we get reprobed, so don't allow
^ permalink raw reply
* Re: [PATCH 01/15] drivers: phy: add generic PHY framework
From: Kishon Vijay Abraham I @ 2013-08-19 5:40 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <520B9C9E.8010002@ti.com>
Felipe,
ping..
On Wednesday 14 August 2013 08:35 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On Wednesday 14 August 2013 04:34 AM, Tomasz Figa wrote:
>> On Wednesday 14 of August 2013 00:19:28 Sylwester Nawrocki wrote:
>>> W dniu 2013-08-13 14:05, Kishon Vijay Abraham I pisze:
>>>> On Tuesday 13 August 2013 05:07 PM, Tomasz Figa wrote:
>>>>> On Tuesday 13 of August 2013 16:14:44 Kishon Vijay Abraham I wrote:
>>>>>> On Wednesday 31 July 2013 11:45 AM, Felipe Balbi wrote:
>>>>>>> On Wed, Jul 31, 2013 at 11:14:32AM +0530, Kishon Vijay Abraham I
>> wrote:
>>>>>>>>>>>>> IMHO we need a lookup method for PHYs, just like for clocks,
>>>>>>>>>>>>> regulators, PWMs or even i2c busses because there are complex
>>>>>>>>>>>>> cases
>>>>>>>>>>>>> when passing just a name using platform data will not work. I
>>>>>>>>>>>>> would
>>>>>>>>>>>>> second what Stephen said [1] and define a structure doing
>>>>>>>>>>>>> things
>>>>>>>>>>>>> in a
>>>>>>>>>>>>> DT-like way.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Example;
>>>>>>>>>>>>>
>>>>>>>>>>>>> [platform code]
>>>>>>>>>>>>>
>>>>>>>>>>>>> static const struct phy_lookup my_phy_lookup[] = {
>>>>>>>>>>>>>
>>>>>>>>>>>>> PHY_LOOKUP("s3c-hsotg.0", "otg", "samsung-usbphy.1",
>>>>>>>>>>>>> "phy.2"),
>>>>>>>>>>>>
>>>>>>>>>>>> The only problem here is that if *PLATFORM_DEVID_AUTO* is used
>>>>>>>>>>>> while
>>>>>>>>>>>> creating the device, the ids in the device name would change
>>>>>>>>>>>> and
>>>>>>>>>>>> PHY_LOOKUP wont be useful.
>>>>>>>>>>>
>>>>>>>>>>> I don't think this is a problem. All the existing lookup
>>>>>>>>>>> methods
>>>>>>>>>>> already
>>>>>>>>>>> use ID to identify devices (see regulators, clkdev, PWMs, i2c,
>>>>>>>>>>> ...). You
>>>>>>>>>>> can simply add a requirement that the ID must be assigned
>>>>>>>>>>> manually,
>>>>>>>>>>> without using PLATFORM_DEVID_AUTO to use PHY lookup.
>>>>>>>>>>
>>>>>>>>>> And I'm saying that this idea, of using a specific name and id,
>>>>>>>>>> is
>>>>>>>>>> frought with fragility and will break in the future in various
>>>>>>>>>> ways
>>>>>>>>>> when
>>>>>>>>>> devices get added to systems, making these strings constantly
>>>>>>>>>> have
>>>>>>>>>> to be
>>>>>>>>>> kept up to date with different board configurations.
>>>>>>>>>>
>>>>>>>>>> People, NEVER, hardcode something like an id. The fact that
>>>>>>>>>> this
>>>>>>>>>> happens today with the clock code, doesn't make it right, it
>>>>>>>>>> makes
>>>>>>>>>> the
>>>>>>>>>> clock code wrong. Others have already said that this is wrong
>>>>>>>>>> there
>>>>>>>>>> as
>>>>>>>>>> well, as systems change and dynamic ids get used more and more.
>>>>>>>>>>
>>>>>>>>>> Let's not repeat the same mistakes of the past just because we
>>>>>>>>>> refuse to
>>>>>>>>>> learn from them...
>>>>>>>>>>
>>>>>>>>>> So again, the "find a phy by a string" functions should be
>>>>>>>>>> removed,
>>>>>>>>>> the
>>>>>>>>>> device id should be automatically created by the phy core just
>>>>>>>>>> to
>>>>>>>>>> make
>>>>>>>>>> things unique in sysfs, and no driver code should _ever_ be
>>>>>>>>>> reliant
>>>>>>>>>> on
>>>>>>>>>> the number that is being created, and the pointer to the phy
>>>>>>>>>> structure
>>>>>>>>>> should be used everywhere instead.
>>>>>>>>>>
>>>>>>>>>> With those types of changes, I will consider merging this
>>>>>>>>>> subsystem,
>>>>>>>>>> but
>>>>>>>>>> without them, sorry, I will not.
>>>>>>>>>
>>>>>>>>> I'll agree with Greg here, the very fact that we see people
>>>>>>>>> trying to
>>>>>>>>> add a requirement of *NOT* using PLATFORM_DEVID_AUTO already
>>>>>>>>> points
>>>>>>>>> to a big problem in the framework.
>>>>>>>>>
>>>>>>>>> The fact is that if we don't allow PLATFORM_DEVID_AUTO we will
>>>>>>>>> end up
>>>>>>>>> adding similar infrastructure to the driver themselves to make
>>>>>>>>> sure
>>>>>>>>> we
>>>>>>>>> don't end up with duplicate names in sysfs in case we have
>>>>>>>>> multiple
>>>>>>>>> instances of the same IP in the SoC (or several of the same PCIe
>>>>>>>>> card).
>>>>>>>>> I really don't want to go back to that.
>>>>>>>>
>>>>>>>> If we are using PLATFORM_DEVID_AUTO, then I dont see any way we
>>>>>>>> can
>>>>>>>> give the correct binding information to the PHY framework. I think
>>>>>>>> we
>>>>>>>> can drop having this non-dt support in PHY framework? I see only
>>>>>>>> one
>>>>>>>> platform (OMAP3) going to be needing this non-dt support and we
>>>>>>>> can
>>>>>>>> use the USB PHY library for it.>
>>>>>>>
>>>>>>> you shouldn't drop support for non-DT platform, in any case we
>>>>>>> lived
>>>>>>> without DT (and still do) for years. Gotta find a better way ;-)
>>>>>>
>>>>>> hmm..
>>>>>>
>>>>>> how about passing the device names of PHY in platform data of the
>>>>>> controller? It should be deterministic as the PHY framework assigns
>>>>>> its
>>>>>> own id and we *don't* want to add any requirement that the ID must
>>>>>> be
>>>>>> assigned manually without using PLATFORM_DEVID_AUTO. We can get rid
>>>>>> of
>>>>>> *phy_init_data* in the v10 patch series.
>>>
>>> OK, so the PHY device name would have a fixed part, passed as
>>> platform data of the controller and a variable part appended
>>> by the PHY core, depending on the number of registered PHYs ?
>>>
>>> Then same PHY names would be passed as the PHY provider driver's
>>> platform data ?
>>>
>>> Then if there are 2 instances of the above (same names in platform
>>> data) how would be determined which PHY controller is linked to
>>> which PHY supplier ?
>>>
>>> I guess you want each device instance to have different PHY device
>>> names already in platform data ? That might work. We probably will
>>> be focused mostly on DT anyway. It seem without DT we are trying
>>> to find some layer that would allow us to couple relevant devices
>>> and overcome driver core inconvenience that it provides to means
>>> to identify specific devices in advance. :) Your proposal sounds
>>> reasonable, however I might be missing some details or corner cases.
>>>
>>>>> What about slightly altering the concept of v9 to pass a pointer to
>>>>> struct device instead of device name inside phy_init_data?
>>>
>>> As Felipe said, we don't want to pass pointers in platform_data
>>> to/from random subsystems. We pass data, passing pointers would
>>> be a total mess IMHO.
>>
>> Well, this is a total mess anyway... I don't really get the point of using
>> PLATFORM_DEVID_AUTO. The only thing that comes to my mind is that you can
>> use it if you don't care about the ID and so it can be assigned
>> automatically.
>>
>> However my understanding of the device ID is that it was supposed to
>> provide a way to identify multiple instances of identical devices in a
>> reliable way, to solve problems like the one we are trying to solve
>> here...
>>
>> 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?
>
> Thanks
> Kishon
>
^ permalink raw reply
* 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
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