Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
From: Ian Campbell @ 2013-12-04 11:23 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Konrad Rzeszutek Wilk, xen-devel, linux-kernel, boris.ostrovsky,
	david.vrabel, leosilva, ashley, peterhuewe, mail, tpmdd, tpmdd,
	dmitry.torokhov, bhelgaas, plagnioj, tomi.valkeinen, tpmdd-devel,
	linux-input, netdev, linux-pci, linux-fbdev
In-Reply-To: <alpine.DEB.2.02.1312041116230.7093@kaball.uk.xensource.com>

On Wed, 2013-12-04 at 11:18 +0000, Stefano Stabellini wrote:
> On Wed, 4 Dec 2013, Ian Campbell wrote:
> > On Wed, 2013-12-04 at 11:05 +0000, Stefano Stabellini wrote:
> > > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > > On Wed, 2013-12-04 at 10:51 +0000, Stefano Stabellini wrote:
> > > > > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > > > > > +bool xen_has_pv_devices(void)
> > > > > > > +{
> > > > > > > +	if (!xen_domain())
> > > > > > > +		return false;
> > > > > > > +
> > > > > > > +	if (xen_hvm_domain()) {
> > > > > > > +		/* User requested no unplug, so no PV drivers. */
> > > > > > > +		if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > > > > > > +			return false;
> > > > > > 
> > > > > > I think you need
> > > > > > 		if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> > > > > > 			return true;
> > > > > > don't you?
> > > > > 
> > > > > XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
> > > > > even if it didn't respond properly to the unplug protocol.
> > > > > The corresponding parameter is called "unnecessary" because if you pass
> > > > > it to the kernel you mean that it is unnecessary to unplug the emulated
> > > > > devices but you can use the pv devices anyway.
> > > > > 
> > > > > So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
> > > > 
> > > > Oh, we will eventually fall through to the return true, so it does
> > > > actually work out OK.
> > > > 
> > > > I'd still be in favour of handling each option explicitly, for clarity.
> > > > Which means checking for XEN_UNPLUG_UNNECESSARY.
> > > 
> > > I think is wrong to check for any xen_emul_unpug options in this function.
> > > The xen_emul_unpug options should be used to set the right value of
> > > xen_platform_pci_unplug. (See my other reply.)
> > 
> > Whichever one we check we should still be checking explicitly for the
> > "unnecessary" case, for clarity if nothing else.
> 
> Sure, that is OK for me.
> In that case should we check for the full list of possible options?

We probably should. That probably means an extra
xen_has_pv_{disk,nic}_devices() which is the existing one plus the
specific checks?

> 
>             ide-disks -- unplug primary master IDE devices
> 			aux-ide-disks -- unplug non-primary-master IDE devices
> 			nics -- unplug network devices
> 			all -- unplug all emulated devices (NICs and IDE disks)
> 			unnecessary -- unplugging emulated devices is
> 				unnecessary even if the host did not respond to
> 				the unplug protocol
> 			never -- do not unplug even if version check succeeds



^ permalink raw reply

* Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
From: Stefano Stabellini @ 2013-12-04 11:18 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, xen-devel,
	linux-kernel, boris.ostrovsky, david.vrabel, leosilva, ashley,
	peterhuewe, mail, tpmdd, tpmdd, dmitry.torokhov, bhelgaas,
	plagnioj, tomi.valkeinen, tpmdd-devel, linux-input, netdev,
	linux-pci, linux-fbdev
In-Reply-To: <1386155291.17466.19.camel@kazak.uk.xensource.com>

On Wed, 4 Dec 2013, Ian Campbell wrote:
> On Wed, 2013-12-04 at 11:05 +0000, Stefano Stabellini wrote:
> > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > On Wed, 2013-12-04 at 10:51 +0000, Stefano Stabellini wrote:
> > > > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > > > > +bool xen_has_pv_devices(void)
> > > > > > +{
> > > > > > +	if (!xen_domain())
> > > > > > +		return false;
> > > > > > +
> > > > > > +	if (xen_hvm_domain()) {
> > > > > > +		/* User requested no unplug, so no PV drivers. */
> > > > > > +		if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > > > > > +			return false;
> > > > > 
> > > > > I think you need
> > > > > 		if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> > > > > 			return true;
> > > > > don't you?
> > > > 
> > > > XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
> > > > even if it didn't respond properly to the unplug protocol.
> > > > The corresponding parameter is called "unnecessary" because if you pass
> > > > it to the kernel you mean that it is unnecessary to unplug the emulated
> > > > devices but you can use the pv devices anyway.
> > > > 
> > > > So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
> > > 
> > > Oh, we will eventually fall through to the return true, so it does
> > > actually work out OK.
> > > 
> > > I'd still be in favour of handling each option explicitly, for clarity.
> > > Which means checking for XEN_UNPLUG_UNNECESSARY.
> > 
> > I think is wrong to check for any xen_emul_unpug options in this function.
> > The xen_emul_unpug options should be used to set the right value of
> > xen_platform_pci_unplug. (See my other reply.)
> 
> Whichever one we check we should still be checking explicitly for the
> "unnecessary" case, for clarity if nothing else.

Sure, that is OK for me.
In that case should we check for the full list of possible options?

            ide-disks -- unplug primary master IDE devices
			aux-ide-disks -- unplug non-primary-master IDE devices
			nics -- unplug network devices
			all -- unplug all emulated devices (NICs and IDE disks)
			unnecessary -- unplugging emulated devices is
				unnecessary even if the host did not respond to
				the unplug protocol
			never -- do not unplug even if version check succeeds

^ permalink raw reply

* Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
From: Ian Campbell @ 2013-12-04 11:08 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Konrad Rzeszutek Wilk, xen-devel, linux-kernel, boris.ostrovsky,
	david.vrabel, leosilva, ashley, peterhuewe, mail, tpmdd, tpmdd,
	dmitry.torokhov, bhelgaas, plagnioj, tomi.valkeinen, tpmdd-devel,
	linux-input, netdev, linux-pci, linux-fbdev
In-Reply-To: <alpine.DEB.2.02.1312041104110.7093@kaball.uk.xensource.com>

On Wed, 2013-12-04 at 11:05 +0000, Stefano Stabellini wrote:
> On Wed, 4 Dec 2013, Ian Campbell wrote:
> > On Wed, 2013-12-04 at 10:51 +0000, Stefano Stabellini wrote:
> > > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > > > +bool xen_has_pv_devices(void)
> > > > > +{
> > > > > +	if (!xen_domain())
> > > > > +		return false;
> > > > > +
> > > > > +	if (xen_hvm_domain()) {
> > > > > +		/* User requested no unplug, so no PV drivers. */
> > > > > +		if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > > > > +			return false;
> > > > 
> > > > I think you need
> > > > 		if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> > > > 			return true;
> > > > don't you?
> > > 
> > > XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
> > > even if it didn't respond properly to the unplug protocol.
> > > The corresponding parameter is called "unnecessary" because if you pass
> > > it to the kernel you mean that it is unnecessary to unplug the emulated
> > > devices but you can use the pv devices anyway.
> > > 
> > > So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
> > 
> > Oh, we will eventually fall through to the return true, so it does
> > actually work out OK.
> > 
> > I'd still be in favour of handling each option explicitly, for clarity.
> > Which means checking for XEN_UNPLUG_UNNECESSARY.
> 
> I think is wrong to check for any xen_emul_unpug options in this function.
> The xen_emul_unpug options should be used to set the right value of
> xen_platform_pci_unplug. (See my other reply.)

Whichever one we check we should still be checking explicitly for the
"unnecessary" case, for clarity if nothing else.

TBH I think the split between xen_emul_unplug and
xen_platform_pci_unplug is a bit artificial. There should be one value
which is static to platform-pci-unplug.c and accessor functions should
be provided for other code to use. Open coding all those accesses to
xen_platform_pci_unplug in every driver is just too error prone.

Ian.


^ permalink raw reply

* Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
From: Stefano Stabellini @ 2013-12-04 11:05 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, xen-devel,
	linux-kernel, boris.ostrovsky, david.vrabel, leosilva, ashley,
	peterhuewe, mail, tpmdd, tpmdd, dmitry.torokhov, bhelgaas,
	plagnioj, tomi.valkeinen, tpmdd-devel, linux-input, netdev,
	linux-pci, linux-fbdev
In-Reply-To: <1386154783.17466.14.camel@kazak.uk.xensource.com>

On Wed, 4 Dec 2013, Ian Campbell wrote:
> On Wed, 2013-12-04 at 10:51 +0000, Stefano Stabellini wrote:
> > On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > > +bool xen_has_pv_devices(void)
> > > > +{
> > > > +	if (!xen_domain())
> > > > +		return false;
> > > > +
> > > > +	if (xen_hvm_domain()) {
> > > > +		/* User requested no unplug, so no PV drivers. */
> > > > +		if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > > > +			return false;
> > > 
> > > I think you need
> > > 		if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> > > 			return true;
> > > don't you?
> > 
> > XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
> > even if it didn't respond properly to the unplug protocol.
> > The corresponding parameter is called "unnecessary" because if you pass
> > it to the kernel you mean that it is unnecessary to unplug the emulated
> > devices but you can use the pv devices anyway.
> > 
> > So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.
> 
> Oh, we will eventually fall through to the return true, so it does
> actually work out OK.
> 
> I'd still be in favour of handling each option explicitly, for clarity.
> Which means checking for XEN_UNPLUG_UNNECESSARY.

I think is wrong to check for any xen_emul_unpug options in this function.
The xen_emul_unpug options should be used to set the right value of
xen_platform_pci_unplug. (See my other reply.)


> > > > +		/* And user has xen_platform_pci=0 set in guest config as
> > > > +		 * driver did not modify the value. */
> > > > +		if (!xen_platform_pci_unplug)
> > > > +			return false;
> 
> I assume this check doesn't trigger if unnecessary has been specified?
 
right

^ permalink raw reply

* Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
From: Ian Campbell @ 2013-12-04 10:59 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: Konrad Rzeszutek Wilk, xen-devel, linux-kernel, boris.ostrovsky,
	david.vrabel, leosilva, ashley, peterhuewe, mail, tpmdd, tpmdd,
	dmitry.torokhov, bhelgaas, plagnioj, tomi.valkeinen, tpmdd-devel,
	linux-input, netdev, linux-pci, linux-fbdev
In-Reply-To: <alpine.DEB.2.02.1312041049310.7093@kaball.uk.xensource.com>

On Wed, 2013-12-04 at 10:51 +0000, Stefano Stabellini wrote:
> On Wed, 4 Dec 2013, Ian Campbell wrote:
> > > +bool xen_has_pv_devices(void)
> > > +{
> > > +	if (!xen_domain())
> > > +		return false;
> > > +
> > > +	if (xen_hvm_domain()) {
> > > +		/* User requested no unplug, so no PV drivers. */
> > > +		if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > > +			return false;
> > 
> > I think you need
> > 		if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> > 			return true;
> > don't you?
> 
> XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
> even if it didn't respond properly to the unplug protocol.
> The corresponding parameter is called "unnecessary" because if you pass
> it to the kernel you mean that it is unnecessary to unplug the emulated
> devices but you can use the pv devices anyway.
> 
> So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.

Oh, we will eventually fall through to the return true, so it does
actually work out OK.

I'd still be in favour of handling each option explicitly, for clarity.
Which means checking for XEN_UNPLUG_UNNECESSARY.

> > > +		/* And user has xen_platform_pci=0 set in guest config as
> > > +		 * driver did not modify the value. */
> > > +		if (!xen_platform_pci_unplug)
> > > +			return false;

I assume this check doesn't trigger if unnecessary has been specified?

> > > +	}
> > > +	return true;
> > > +}
> > > +EXPORT_SYMBOL_GPL(xen_has_pv_devices);
> > > +
> > >  void xen_unplug_emulated_devices(void)
> > >  {
> > >  	int r;
> > 
> > 



^ permalink raw reply

* Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
From: Stefano Stabellini @ 2013-12-04 10:51 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Konrad Rzeszutek Wilk, stefano.stabellini, xen-devel,
	linux-kernel, boris.ostrovsky, david.vrabel, leosilva, ashley,
	peterhuewe, mail, tpmdd, tpmdd, dmitry.torokhov, bhelgaas,
	plagnioj, tomi.valkeinen, tpmdd-devel, linux-input, netdev,
	linux-pci, linux-fbdev
In-Reply-To: <1386149848.13256.86.camel@kazak.uk.xensource.com>

On Wed, 4 Dec 2013, Ian Campbell wrote:
> > +bool xen_has_pv_devices(void)
> > +{
> > +	if (!xen_domain())
> > +		return false;
> > +
> > +	if (xen_hvm_domain()) {
> > +		/* User requested no unplug, so no PV drivers. */
> > +		if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> > +			return false;
> 
> I think you need
> 		if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
> 			return true;
> don't you?

XEN_UNPLUG_UNNECESSARY was introduced to enable the platform PCI device
even if it didn't respond properly to the unplug protocol.
The corresponding parameter is called "unnecessary" because if you pass
it to the kernel you mean that it is unnecessary to unplug the emulated
devices but you can use the pv devices anyway.

So no, we shouldn't check for XEN_UNPLUG_UNNECESSARY here.



> > +		/* And user has xen_platform_pci=0 set in guest config as
> > +		 * driver did not modify the value. */
> > +		if (!xen_platform_pci_unplug)
> > +			return false;
> > +	}
> > +	return true;
> > +}
> > +EXPORT_SYMBOL_GPL(xen_has_pv_devices);
> > +
> >  void xen_unplug_emulated_devices(void)
> >  {
> >  	int r;
> 
> 

^ permalink raw reply

* Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
From: Stefano Stabellini @ 2013-12-04 10:48 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: stefano.stabellini, ian.campbell, xen-devel, linux-kernel,
	boris.ostrovsky, david.vrabel, leosilva, ashley, peterhuewe, mail,
	tpmdd, tpmdd, dmitry.torokhov, bhelgaas, plagnioj, tomi.valkeinen,
	tpmdd-devel, linux-input, netdev, linux-pci, linux-fbdev
In-Reply-To: <1386105246-14337-1-git-send-email-konrad.wilk@oracle.com>

On Tue, 3 Dec 2013, Konrad Rzeszutek Wilk wrote:
> diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
> index 0a78524..087dfeb 100644
> --- a/arch/x86/xen/platform-pci-unplug.c
> +++ b/arch/x86/xen/platform-pci-unplug.c
> @@ -69,6 +69,24 @@ static int check_platform_magic(void)
>  	return 0;
>  }
>  
> +bool xen_has_pv_devices(void)
> +{
> +	if (!xen_domain())
> +		return false;
> +
> +	if (xen_hvm_domain()) {
> +		/* User requested no unplug, so no PV drivers. */
> +		if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> +			return false;

Considering that if (xen_emul_unplug & XEN_UNPLUG_NEVER) we never set
xen_platform_pci_unplug, this check is redundant.


> +		/* And user has xen_platform_pci=0 set in guest config as
> +		 * driver did not modify the value. */
> +		if (!xen_platform_pci_unplug)
> +			return false;
> +	}
> +	return true;
> +}
> +EXPORT_SYMBOL_GPL(xen_has_pv_devices);
> +
>  void xen_unplug_emulated_devices(void)
>  {
>  	int r;

^ permalink raw reply

* Re: [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
From: Ian Campbell @ 2013-12-04  9:37 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: stefano.stabellini, xen-devel, linux-kernel, boris.ostrovsky,
	david.vrabel, leosilva, ashley, peterhuewe, mail, tpmdd, tpmdd,
	dmitry.torokhov, bhelgaas, plagnioj, tomi.valkeinen, tpmdd-devel,
	linux-input, netdev, linux-pci, linux-fbdev
In-Reply-To: <1386105246-14337-1-git-send-email-konrad.wilk@oracle.com>

On Tue, 2013-12-03 at 16:14 -0500, Konrad Rzeszutek Wilk wrote:
> The user has the option of disabling the platform driver:
> 00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
> 
> which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
> and allow the PV drivers to take over. If the user wishes
> to disable that they can set:
> 
>   xen_platform_pci=0
>   (in the guest config file)
> 
> or
>   xen_emul_unplug=never
>   (on the Linux command line)
> 
> except it does not work properly. The PV drivers still try to
> load and since the Xen platform driver is not run - and it
> has not initialized the grant tables, most of the PV drivers
> stumble upon:
> 
> input: Xen Virtual Keyboard as /devices/virtual/input/input5
> input: Xen Virtual Pointer as /devices/virtual/input/input6M
> ------------[ cut here ]------------
> kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
> invalid opcode: 0000 [#1] SMP
> Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
> CPU: 6 PID: 1389 Comm: modprobe Not tainted 3.13.0-rc1upstream-00021-ga6c892b-dirty #1
> Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
> RIP: 0010:[<ffffffff813ddc40>]  [<ffffffff813ddc40>] get_free_entries+0x2e0/0x300
> Call Trace:
>  [<ffffffff8150d9a3>] ? evdev_connect+0x1e3/0x240
>  [<ffffffff813ddd0e>] gnttab_grant_foreign_access+0x2e/0x70
>  [<ffffffffa0010081>] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
>  [<ffffffffa0010a12>] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
>  [<ffffffff813e5757>] xenbus_dev_probe+0x77/0x130
>  [<ffffffff813e7217>] xenbus_frontend_dev_probe+0x47/0x50
>  [<ffffffff8145e9a9>] driver_probe_device+0x89/0x230
>  [<ffffffff8145ebeb>] __driver_attach+0x9b/0xa0
>  [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
>  [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
>  [<ffffffff8145cf1c>] bus_for_each_dev+0x8c/0xb0
>  [<ffffffff8145e7d9>] driver_attach+0x19/0x20
>  [<ffffffff8145e260>] bus_add_driver+0x1a0/0x220
>  [<ffffffff8145f1ff>] driver_register+0x5f/0xf0
>  [<ffffffff813e55c5>] xenbus_register_driver_common+0x15/0x20
>  [<ffffffff813e76b3>] xenbus_register_frontend+0x23/0x40
>  [<ffffffffa0015000>] ? 0xffffffffa0014fff
>  [<ffffffffa001502b>] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
>  [<ffffffff81002049>] do_one_initcall+0x49/0x170
> 
> .. snip..
> 
> which is hardly nice. This patch fixes this by having each
> PV driver check for:
>  - if running in PV, then it is fine to execute (as that is their
>    native environment).
>  - if running in HVM, check if user wanted 'xen_emul_unplug=never',
>    in which case bail out and don't load PV drivers.
>  - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
>    does not exist, then bail out and not load PV drivers.
> 
> P.S.
> Ian Campbell suggested getting rid of 'xen_platform_pci_unplug'
> but unfortunatly the xen-blkfront driver is using it, so we
> cannot do it.

It might still be nice to expose a suitable semantic interface (i.e.
some relevant predicate) rather than the raw value for blkfront to use.
But that can be a future thing I think.

> Reported-by: Sander Eikelenboom <linux@eikelenboom.it
> Reported-by: Anthony PERARD <anthony.perard@citrix.com>
> Reported-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> ---
>  arch/x86/xen/platform-pci-unplug.c         | 18 ++++++++++++++++++
>  drivers/block/xen-blkfront.c               |  2 +-
>  drivers/char/tpm/xen-tpmfront.c            |  4 ++++
>  drivers/input/misc/xen-kbdfront.c          |  4 ++++
>  drivers/net/xen-netfront.c                 |  2 +-
>  drivers/pci/xen-pcifront.c                 |  4 ++++
>  drivers/video/xen-fbfront.c                |  4 ++++
>  drivers/xen/xenbus/xenbus_probe_frontend.c |  2 +-
>  include/xen/platform_pci.h                 | 13 ++++++++++++-
>  9 files changed, 49 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
> index 0a78524..087dfeb 100644
> --- a/arch/x86/xen/platform-pci-unplug.c
> +++ b/arch/x86/xen/platform-pci-unplug.c
> @@ -69,6 +69,24 @@ static int check_platform_magic(void)
>  	return 0;
>  }
>  
> +bool xen_has_pv_devices(void)
> +{
> +	if (!xen_domain())
> +		return false;
> +
> +	if (xen_hvm_domain()) {
> +		/* User requested no unplug, so no PV drivers. */
> +		if (xen_emul_unplug & XEN_UNPLUG_NEVER)
> +			return false;

I think you need
		if (xen_emul_unpug & XEN_UNPLUG_UNNECESSARY)
			return true;
don't you?

> +		/* And user has xen_platform_pci=0 set in guest config as
> +		 * driver did not modify the value. */
> +		if (!xen_platform_pci_unplug)
> +			return false;
> +	}
> +	return true;
> +}
> +EXPORT_SYMBOL_GPL(xen_has_pv_devices);
> +
>  void xen_unplug_emulated_devices(void)
>  {
>  	int r;



^ permalink raw reply

* Re: [patch] video: vt8500: fix error handling in probe()
From: Tomi Valkeinen @ 2013-12-04  8:51 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131202081118.GB3852@elgon.mountain>

[-- Attachment #1: Type: text/plain, Size: 2667 bytes --]

On 2013-12-02 10:11, Dan Carpenter wrote:
> We shouldn't kfree(fbi) because that was allocated with devm_kzalloc().
> There were several error paths which returned directly instead of
> releasing resources.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> 
> diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
> index b30e5a439d1f..a8f2b280f796 100644
> --- a/drivers/video/vt8500lcdfb.c
> +++ b/drivers/video/vt8500lcdfb.c
> @@ -293,8 +293,7 @@ static int vt8500lcd_probe(struct platform_device *pdev)
>  			+ sizeof(u32) * 16, GFP_KERNEL);
>  	if (!fbi) {
>  		dev_err(&pdev->dev, "Failed to initialize framebuffer device\n");
> -		ret = -ENOMEM;
> -		goto failed;
> +		return -ENOMEM;
>  	}
>  
>  	strcpy(fbi->fb.fix.id, "VT8500 LCD");
> @@ -327,15 +326,13 @@ static int vt8500lcd_probe(struct platform_device *pdev)
>  	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	if (res == NULL) {
>  		dev_err(&pdev->dev, "no I/O memory resource defined\n");
> -		ret = -ENODEV;
> -		goto failed_fbi;
> +		return -ENODEV;
>  	}
>  
>  	res = request_mem_region(res->start, resource_size(res), "vt8500lcd");
>  	if (res == NULL) {
>  		dev_err(&pdev->dev, "failed to request I/O memory\n");
> -		ret = -EBUSY;
> -		goto failed_fbi;
> +		return -EBUSY;
>  	}
>  
>  	fbi->regbase = ioremap(res->start, resource_size(res));
> @@ -346,17 +343,19 @@ static int vt8500lcd_probe(struct platform_device *pdev)
>  	}
>  
>  	disp_timing = of_get_display_timings(pdev->dev.of_node);
> -	if (!disp_timing)
> -		return -EINVAL;
> +	if (!disp_timing) {
> +		ret = -EINVAL;
> +		goto failed_free_io;
> +	}
>  
>  	ret = of_get_fb_videomode(pdev->dev.of_node, &of_mode,
>  							OF_USE_NATIVE_MODE);
>  	if (ret)
> -		return ret;
> +		goto failed_free_io;
>  
>  	ret = of_property_read_u32(pdev->dev.of_node, "bits-per-pixel", &bpp);
>  	if (ret)
> -		return ret;
> +		goto failed_free_io;
>  
>  	/* try allocating the framebuffer */
>  	fb_mem_len = of_mode.xres * of_mode.yres * 2 * (bpp / 8);
> @@ -364,7 +363,8 @@ static int vt8500lcd_probe(struct platform_device *pdev)
>  				GFP_KERNEL);
>  	if (!fb_mem_virt) {
>  		pr_err("%s: Failed to allocate framebuffer\n", __func__);
> -		return -ENOMEM;
> +		ret = -ENOMEM;
> +		goto failed_free_io;
>  	}
>  
>  	fbi->fb.fix.smem_start	= fb_mem_phys;
> @@ -447,9 +447,6 @@ failed_free_io:
>  	iounmap(fbi->regbase);
>  failed_free_res:
>  	release_mem_region(res->start, resource_size(res));
> -failed_fbi:
> -	kfree(fbi);
> -failed:
>  	return ret;
>  }

Thanks, queued for 3.13 fbdev fixes.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH] atmel_lcdfb: fix module autoload
From: Tomi Valkeinen @ 2013-12-04  8:49 UTC (permalink / raw)
  To: Nicolas Ferre, Johan Hovold, linux-fbdev,
	Jean-Christophe PLAGNIOL-VILLARD
  Cc: linux-kernel
In-Reply-To: <529CD0B5.600@atmel.com>

[-- Attachment #1: Type: text/plain, Size: 1008 bytes --]

On 2013-12-02 20:25, Nicolas Ferre wrote:
> On 22/10/2013 18:36, Johan Hovold :
>> Add missing module device table which is needed for module autoloading.
>>
>> Signed-off-by: Johan Hovold <jhovold@gmail.com>
> 
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> Jean-Christophe, Tomi,
> 
> Can you please take this patch?
> 
> Best regards,
> 
> 
>> ---
>>   drivers/video/atmel_lcdfb.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
>> index 088511a..67b339c 100644
>> --- a/drivers/video/atmel_lcdfb.c
>> +++ b/drivers/video/atmel_lcdfb.c
>> @@ -94,6 +94,7 @@ static const struct platform_device_id
>> atmel_lcdfb_devtypes[] = {
>>           /* terminator */
>>       }
>>   };
>> +MODULE_DEVICE_TABLE(platform, atmel_lcdfb_devtypes);
>>
>>   static struct atmel_lcdfb_config *
>>   atmel_lcdfb_get_config(struct platform_device *pdev)
>>


Thanks, queued for 3.13 fbdev fixes.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [Xen-devel] [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
From: Matthew Daley @ 2013-12-04  0:03 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Stefano Stabellini, Ian Campbell, xen-devel, linux-kernel,
	boris.ostrovsky, david.vrabel, leosilva, ashley, peterhuewe, mail,
	tpmdd, tpmdd, dmitry.torokhov, bhelgaas, plagnioj, tomi.valkeinen,
	tpmdd-devel, linux-input, netdev, linux-pci, linux-fbdev
In-Reply-To: <1386105246-14337-1-git-send-email-konrad.wilk@oracle.com>

On Wed, Dec 4, 2013 at 10:14 AM, Konrad Rzeszutek Wilk
<konrad.wilk@oracle.com> wrote:
> The user has the option of disabling the platform driver:
> 00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)
>
> which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
> and allow the PV drivers to take over. If the user wishes
> to disable that they can set:
>
>   xen_platform_pci=0
>   (in the guest config file)
>
> or
>   xen_emul_unplug=never
>   (on the Linux command line)
>
> except it does not work properly. The PV drivers still try to
> load and since the Xen platform driver is not run - and it
> has not initialized the grant tables, most of the PV drivers
> stumble upon:
>
> input: Xen Virtual Keyboard as /devices/virtual/input/input5
> input: Xen Virtual Pointer as /devices/virtual/input/input6M
> ------------[ cut here ]------------
> kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
> invalid opcode: 0000 [#1] SMP
> Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
> CPU: 6 PID: 1389 Comm: modprobe Not tainted 3.13.0-rc1upstream-00021-ga6c892b-dirty #1
> Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
> RIP: 0010:[<ffffffff813ddc40>]  [<ffffffff813ddc40>] get_free_entries+0x2e0/0x300
> Call Trace:
>  [<ffffffff8150d9a3>] ? evdev_connect+0x1e3/0x240
>  [<ffffffff813ddd0e>] gnttab_grant_foreign_access+0x2e/0x70
>  [<ffffffffa0010081>] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
>  [<ffffffffa0010a12>] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
>  [<ffffffff813e5757>] xenbus_dev_probe+0x77/0x130
>  [<ffffffff813e7217>] xenbus_frontend_dev_probe+0x47/0x50
>  [<ffffffff8145e9a9>] driver_probe_device+0x89/0x230
>  [<ffffffff8145ebeb>] __driver_attach+0x9b/0xa0
>  [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
>  [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
>  [<ffffffff8145cf1c>] bus_for_each_dev+0x8c/0xb0
>  [<ffffffff8145e7d9>] driver_attach+0x19/0x20
>  [<ffffffff8145e260>] bus_add_driver+0x1a0/0x220
>  [<ffffffff8145f1ff>] driver_register+0x5f/0xf0
>  [<ffffffff813e55c5>] xenbus_register_driver_common+0x15/0x20
>  [<ffffffff813e76b3>] xenbus_register_frontend+0x23/0x40
>  [<ffffffffa0015000>] ? 0xffffffffa0014fff
>  [<ffffffffa001502b>] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
>  [<ffffffff81002049>] do_one_initcall+0x49/0x170
>
> .. snip..
>
> which is hardly nice. This patch fixes this by having each
> PV driver check for:
>  - if running in PV, then it is fine to execute (as that is their
>    native environment).
>  - if running in HVM, check if user wanted 'xen_emul_unplug=never',
>    in which case bail out and don't load PV drivers.
>  - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
>    does not exist, then bail out and not load PV drivers.
>
> P.S.
> Ian Campbell suggested getting rid of 'xen_platform_pci_unplug'
> but unfortunatly the xen-blkfront driver is using it, so we
> cannot do it.
>
> Reported-by: Sander Eikelenboom <linux@eikelenboom.it
> Reported-by: Anthony PERARD <anthony.perard@citrix.com>
> Reported-by: Fabio Fantoni <fabio.fantoni@m2r.biz>

I think you forgot your Signed-off-by line.

- Matthew

^ permalink raw reply

* [PATCH] xen/pvhvm: If xen_platform_pci=0 is set don't blow up.
From: Konrad Rzeszutek Wilk @ 2013-12-03 21:14 UTC (permalink / raw)
  To: stefano.stabellini, ian.campbell, xen-devel, linux-kernel,
	boris.ostrovsky, david.vrabel, leosilva, ashley, peterhuewe, mail,
	tpmdd, tpmdd, dmitry.torokhov, bhelgaas, plagnioj, tomi.valkeinen,
	tpmdd-devel, linux-input, netdev, linux-pci, linux-fbdev
  Cc: Konrad Rzeszutek Wilk

The user has the option of disabling the platform driver:
00:02.0 Unassigned class [ff80]: XenSource, Inc. Xen Platform Device (rev 01)

which is used to unplug the emulated drivers (IDE, Realtek 8169, etc)
and allow the PV drivers to take over. If the user wishes
to disable that they can set:

  xen_platform_pci=0
  (in the guest config file)

or
  xen_emul_unplug=never
  (on the Linux command line)

except it does not work properly. The PV drivers still try to
load and since the Xen platform driver is not run - and it
has not initialized the grant tables, most of the PV drivers
stumble upon:

input: Xen Virtual Keyboard as /devices/virtual/input/input5
input: Xen Virtual Pointer as /devices/virtual/input/input6M
------------[ cut here ]------------
kernel BUG at /home/konrad/ssd/konrad/linux/drivers/xen/grant-table.c:1206!
invalid opcode: 0000 [#1] SMP
Modules linked in: xen_kbdfront(+) xenfs xen_privcmd
CPU: 6 PID: 1389 Comm: modprobe Not tainted 3.13.0-rc1upstream-00021-ga6c892b-dirty #1
Hardware name: Xen HVM domU, BIOS 4.4-unstable 11/26/2013
RIP: 0010:[<ffffffff813ddc40>]  [<ffffffff813ddc40>] get_free_entries+0x2e0/0x300
Call Trace:
 [<ffffffff8150d9a3>] ? evdev_connect+0x1e3/0x240
 [<ffffffff813ddd0e>] gnttab_grant_foreign_access+0x2e/0x70
 [<ffffffffa0010081>] xenkbd_connect_backend+0x41/0x290 [xen_kbdfront]
 [<ffffffffa0010a12>] xenkbd_probe+0x2f2/0x324 [xen_kbdfront]
 [<ffffffff813e5757>] xenbus_dev_probe+0x77/0x130
 [<ffffffff813e7217>] xenbus_frontend_dev_probe+0x47/0x50
 [<ffffffff8145e9a9>] driver_probe_device+0x89/0x230
 [<ffffffff8145ebeb>] __driver_attach+0x9b/0xa0
 [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
 [<ffffffff8145eb50>] ? driver_probe_device+0x230/0x230
 [<ffffffff8145cf1c>] bus_for_each_dev+0x8c/0xb0
 [<ffffffff8145e7d9>] driver_attach+0x19/0x20
 [<ffffffff8145e260>] bus_add_driver+0x1a0/0x220
 [<ffffffff8145f1ff>] driver_register+0x5f/0xf0
 [<ffffffff813e55c5>] xenbus_register_driver_common+0x15/0x20
 [<ffffffff813e76b3>] xenbus_register_frontend+0x23/0x40
 [<ffffffffa0015000>] ? 0xffffffffa0014fff
 [<ffffffffa001502b>] xenkbd_init+0x2b/0x1000 [xen_kbdfront]
 [<ffffffff81002049>] do_one_initcall+0x49/0x170

.. snip..

which is hardly nice. This patch fixes this by having each
PV driver check for:
 - if running in PV, then it is fine to execute (as that is their
   native environment).
 - if running in HVM, check if user wanted 'xen_emul_unplug=never',
   in which case bail out and don't load PV drivers.
 - if running in HVM, and if PCI device 5853:0001 (xen_platform_pci)
   does not exist, then bail out and not load PV drivers.

P.S.
Ian Campbell suggested getting rid of 'xen_platform_pci_unplug'
but unfortunatly the xen-blkfront driver is using it, so we
cannot do it.

Reported-by: Sander Eikelenboom <linux@eikelenboom.it
Reported-by: Anthony PERARD <anthony.perard@citrix.com>
Reported-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
---
 arch/x86/xen/platform-pci-unplug.c         | 18 ++++++++++++++++++
 drivers/block/xen-blkfront.c               |  2 +-
 drivers/char/tpm/xen-tpmfront.c            |  4 ++++
 drivers/input/misc/xen-kbdfront.c          |  4 ++++
 drivers/net/xen-netfront.c                 |  2 +-
 drivers/pci/xen-pcifront.c                 |  4 ++++
 drivers/video/xen-fbfront.c                |  4 ++++
 drivers/xen/xenbus/xenbus_probe_frontend.c |  2 +-
 include/xen/platform_pci.h                 | 13 ++++++++++++-
 9 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/arch/x86/xen/platform-pci-unplug.c b/arch/x86/xen/platform-pci-unplug.c
index 0a78524..087dfeb 100644
--- a/arch/x86/xen/platform-pci-unplug.c
+++ b/arch/x86/xen/platform-pci-unplug.c
@@ -69,6 +69,24 @@ static int check_platform_magic(void)
 	return 0;
 }
 
+bool xen_has_pv_devices(void)
+{
+	if (!xen_domain())
+		return false;
+
+	if (xen_hvm_domain()) {
+		/* User requested no unplug, so no PV drivers. */
+		if (xen_emul_unplug & XEN_UNPLUG_NEVER)
+			return false;
+		/* And user has xen_platform_pci=0 set in guest config as
+		 * driver did not modify the value. */
+		if (!xen_platform_pci_unplug)
+			return false;
+	}
+	return true;
+}
+EXPORT_SYMBOL_GPL(xen_has_pv_devices);
+
 void xen_unplug_emulated_devices(void)
 {
 	int r;
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 432db1b..9616b81 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -2074,7 +2074,7 @@ static int __init xlblk_init(void)
 	if (!xen_domain())
 		return -ENODEV;
 
-	if (xen_hvm_domain() && !xen_platform_pci_unplug)
+	if (!xen_has_pv_devices())
 		return -ENODEV;
 
 	if (register_blkdev(XENVBD_MAJOR, DEV_NAME)) {
diff --git a/drivers/char/tpm/xen-tpmfront.c b/drivers/char/tpm/xen-tpmfront.c
index c8ff4df..62e7d38 100644
--- a/drivers/char/tpm/xen-tpmfront.c
+++ b/drivers/char/tpm/xen-tpmfront.c
@@ -17,6 +17,7 @@
 #include <xen/xenbus.h>
 #include <xen/page.h>
 #include "tpm.h"
+#include <xen/platform_pci.h>
 
 struct tpm_private {
 	struct tpm_chip *chip;
@@ -421,6 +422,9 @@ static int __init xen_tpmfront_init(void)
 	if (!xen_domain())
 		return -ENODEV;
 
+	if (!xen_has_pv_devices())
+		return -ENODEV;
+
 	return xenbus_register_frontend(&tpmfront_driver);
 }
 module_init(xen_tpmfront_init);
diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
index e21c181..fbfdc10 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -29,6 +29,7 @@
 #include <xen/interface/io/fbif.h>
 #include <xen/interface/io/kbdif.h>
 #include <xen/xenbus.h>
+#include <xen/platform_pci.h>
 
 struct xenkbd_info {
 	struct input_dev *kbd;
@@ -380,6 +381,9 @@ static int __init xenkbd_init(void)
 	if (xen_initial_domain())
 		return -ENODEV;
 
+	if (!xen_has_pv_devices())
+		return -ENODEV;
+
 	return xenbus_register_frontend(&xenkbd_driver);
 }
 
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index e59acb1..d4b52e9 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -2115,7 +2115,7 @@ static int __init netif_init(void)
 	if (!xen_domain())
 		return -ENODEV;
 
-	if (xen_hvm_domain() && !xen_platform_pci_unplug)
+	if (!xen_has_pv_devices())
 		return -ENODEV;
 
 	pr_info("Initialising Xen virtual ethernet driver\n");
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index f7197a7..eae7cd9 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -20,6 +20,7 @@
 #include <linux/workqueue.h>
 #include <linux/bitops.h>
 #include <linux/time.h>
+#include <xen/platform_pci.h>
 
 #include <asm/xen/swiotlb-xen.h>
 #define INVALID_GRANT_REF (0)
@@ -1138,6 +1139,9 @@ static int __init pcifront_init(void)
 	if (!xen_pv_domain() || xen_initial_domain())
 		return -ENODEV;
 
+	if (!xen_has_pv_devices())
+		return -ENODEV;
+
 	pci_frontend_registrar(1 /* enable */);
 
 	return xenbus_register_frontend(&xenpci_driver);
diff --git a/drivers/video/xen-fbfront.c b/drivers/video/xen-fbfront.c
index cd005c2..4b2d3ab 100644
--- a/drivers/video/xen-fbfront.c
+++ b/drivers/video/xen-fbfront.c
@@ -35,6 +35,7 @@
 #include <xen/interface/io/fbif.h>
 #include <xen/interface/io/protocols.h>
 #include <xen/xenbus.h>
+#include <xen/platform_pci.h>
 
 struct xenfb_info {
 	unsigned char		*fb;
@@ -699,6 +700,9 @@ static int __init xenfb_init(void)
 	if (xen_initial_domain())
 		return -ENODEV;
 
+	if (!xen_has_pv_devices())
+		return -ENODEV;
+
 	return xenbus_register_frontend(&xenfb_driver);
 }
 
diff --git a/drivers/xen/xenbus/xenbus_probe_frontend.c b/drivers/xen/xenbus/xenbus_probe_frontend.c
index 129bf84..cb385c1 100644
--- a/drivers/xen/xenbus/xenbus_probe_frontend.c
+++ b/drivers/xen/xenbus/xenbus_probe_frontend.c
@@ -496,7 +496,7 @@ subsys_initcall(xenbus_probe_frontend_init);
 #ifndef MODULE
 static int __init boot_wait_for_devices(void)
 {
-	if (xen_hvm_domain() && !xen_platform_pci_unplug)
+	if (!xen_has_pv_devices())
 		return -ENODEV;
 
 	ready_to_wait_for_devices = 1;
diff --git a/include/xen/platform_pci.h b/include/xen/platform_pci.h
index 438c256..87bc59c 100644
--- a/include/xen/platform_pci.h
+++ b/include/xen/platform_pci.h
@@ -47,5 +47,16 @@ static inline int xen_must_unplug_disks(void) {
 }
 
 extern int xen_platform_pci_unplug;
-
+#if defined(CONFIG_XEN_PVHVM)
+extern bool xen_has_pv_devices(void);
+#else
+static inline bool xen_has_pv_devices(void)
+{
+#if defined(CONFIG_XEN)
+	return true;
+#else
+	return false;
+#endif
+}
+#endif
 #endif /* _XEN_PLATFORM_PCI_H */
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH 32/39] video: remove DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-12-02 23:28 UTC (permalink / raw)
  To: 'Greg Kroah-Hartman'
  Cc: linux-kernel, 'Jingoo Han', 'Tomi Valkeinen',
	linux-fbdev
In-Reply-To: <001501ceefb1$69c96820$3d5c3860$%han@samsung.com>

Don't use DEFINE_PCI_DEVICE_TABLE macro, because this macro
is not preferred.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/hyperv_fb.c |    2 +-
 drivers/video/i740fb.c    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/hyperv_fb.c b/drivers/video/hyperv_fb.c
index 130708f..8254ed7 100644
--- a/drivers/video/hyperv_fb.c
+++ b/drivers/video/hyperv_fb.c
@@ -800,7 +800,7 @@ static int hvfb_remove(struct hv_device *hdev)
 }
 
 
-static DEFINE_PCI_DEVICE_TABLE(pci_stub_id_table) = {
+static const struct pci_device_id pci_stub_id_table[] = {
 	{
 		.vendor      = PCI_VENDOR_ID_MICROSOFT,
 		.device      = PCI_DEVICE_ID_HYPERV_VIDEO,
diff --git a/drivers/video/i740fb.c b/drivers/video/i740fb.c
index ca7c9df..a2b4204 100644
--- a/drivers/video/i740fb.c
+++ b/drivers/video/i740fb.c
@@ -1260,7 +1260,7 @@ fail:
 #define I740_ID_PCI 0x00d1
 #define I740_ID_AGP 0x7800
 
-static DEFINE_PCI_DEVICE_TABLE(i740fb_id_table) = {
+static const struct pci_device_id i740fb_id_table[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, I740_ID_PCI) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, I740_ID_AGP) },
 	{ 0 }
-- 
1.7.10.4



^ permalink raw reply related

* Re: [PATCH] atmel_lcdfb: fix module autoload
From: Nicolas Ferre @ 2013-12-02 18:25 UTC (permalink / raw)
  To: Johan Hovold, linux-fbdev, Jean-Christophe PLAGNIOL-VILLARD,
	Tomi Valkeinen
  Cc: linux-kernel
In-Reply-To: <1382459817-26990-1-git-send-email-jhovold@gmail.com>

On 22/10/2013 18:36, Johan Hovold :
> Add missing module device table which is needed for module autoloading.
>
> Signed-off-by: Johan Hovold <jhovold@gmail.com>

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

Jean-Christophe, Tomi,

Can you please take this patch?

Best regards,


> ---
>   drivers/video/atmel_lcdfb.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/video/atmel_lcdfb.c b/drivers/video/atmel_lcdfb.c
> index 088511a..67b339c 100644
> --- a/drivers/video/atmel_lcdfb.c
> +++ b/drivers/video/atmel_lcdfb.c
> @@ -94,6 +94,7 @@ static const struct platform_device_id atmel_lcdfb_devtypes[] = {
>   		/* terminator */
>   	}
>   };
> +MODULE_DEVICE_TABLE(platform, atmel_lcdfb_devtypes);
>
>   static struct atmel_lcdfb_config *
>   atmel_lcdfb_get_config(struct platform_device *pdev)
>


-- 
Nicolas Ferre

^ permalink raw reply

* Re: kernel mode splash screen
From: Cliff Brake @ 2013-12-02 18:19 UTC (permalink / raw)
  To: linux-fbdev

On Tue, Aug 6, 2013 at 12:15 PM, Cliff Brake <cliff.brake@gmail.com> wrote:
> Hello,
>
> We are working on an embedded system where we need immediate feedback
> on the screen in response to power button events, etc.  We also need
> to be able to display the splash screen whenever kernel is running.
> User space splash screens get killed too early on shutdown, etc.
>
> Does anyone know of a kernel mode splash screen that will:
>
> 1) display an image
> 2) display status messages
> 3) display progress bar (nice to have, but not required)
> 4) can be accessed from kernel and user space
> 5) when enabled, will disable writes from user space
> 6) FB based

I'm currently porting
http://git.yoctoproject.org/cgit/cgit.cgi/psplash/ to the linux
kernel.  So far, its going fairly well.  Psplash is fairly clean and
minimal.  Two questions:

1) is there kernel function to write pixels to the framebuffer?  I've
been looking at fb_fillrect()

2) what is the format of the color parameter in the following data
structure (gets passed to fb_fillrect():

struct fb_fillrect {
__u32 dx; /* screen-relative */
__u32 dy;
__u32 width;
__u32 height;
__u32 color;
__u32 rop;
};

3) We are using the omap2 frame buffer driver.  By default its
configured to provide 3 frame-buffers.  I'm not sure what this means,
but I thinking that perhaps we can use one for the splash screen, and
one for the normal frame buffer used by userspace?  The issue is I
want to disable user space from drawing to the framebuffer instantly,
and switch to the splash screen when the user presses the power switch
(instant feedback).  What is the purpose of multiple frame buffers,
and is this a reasonable use of them?

Thanks,
Cliff


-- 
========http://bec-systems.com

^ permalink raw reply

* [patch] video: vt8500: fix error handling in probe()
From: Dan Carpenter @ 2013-12-02  8:11 UTC (permalink / raw)
  To: linux-arm-kernel

We shouldn't kfree(fbi) because that was allocated with devm_kzalloc().
There were several error paths which returned directly instead of
releasing resources.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/video/vt8500lcdfb.c b/drivers/video/vt8500lcdfb.c
index b30e5a439d1f..a8f2b280f796 100644
--- a/drivers/video/vt8500lcdfb.c
+++ b/drivers/video/vt8500lcdfb.c
@@ -293,8 +293,7 @@ static int vt8500lcd_probe(struct platform_device *pdev)
 			+ sizeof(u32) * 16, GFP_KERNEL);
 	if (!fbi) {
 		dev_err(&pdev->dev, "Failed to initialize framebuffer device\n");
-		ret = -ENOMEM;
-		goto failed;
+		return -ENOMEM;
 	}
 
 	strcpy(fbi->fb.fix.id, "VT8500 LCD");
@@ -327,15 +326,13 @@ static int vt8500lcd_probe(struct platform_device *pdev)
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (res = NULL) {
 		dev_err(&pdev->dev, "no I/O memory resource defined\n");
-		ret = -ENODEV;
-		goto failed_fbi;
+		return -ENODEV;
 	}
 
 	res = request_mem_region(res->start, resource_size(res), "vt8500lcd");
 	if (res = NULL) {
 		dev_err(&pdev->dev, "failed to request I/O memory\n");
-		ret = -EBUSY;
-		goto failed_fbi;
+		return -EBUSY;
 	}
 
 	fbi->regbase = ioremap(res->start, resource_size(res));
@@ -346,17 +343,19 @@ static int vt8500lcd_probe(struct platform_device *pdev)
 	}
 
 	disp_timing = of_get_display_timings(pdev->dev.of_node);
-	if (!disp_timing)
-		return -EINVAL;
+	if (!disp_timing) {
+		ret = -EINVAL;
+		goto failed_free_io;
+	}
 
 	ret = of_get_fb_videomode(pdev->dev.of_node, &of_mode,
 							OF_USE_NATIVE_MODE);
 	if (ret)
-		return ret;
+		goto failed_free_io;
 
 	ret = of_property_read_u32(pdev->dev.of_node, "bits-per-pixel", &bpp);
 	if (ret)
-		return ret;
+		goto failed_free_io;
 
 	/* try allocating the framebuffer */
 	fb_mem_len = of_mode.xres * of_mode.yres * 2 * (bpp / 8);
@@ -364,7 +363,8 @@ static int vt8500lcd_probe(struct platform_device *pdev)
 				GFP_KERNEL);
 	if (!fb_mem_virt) {
 		pr_err("%s: Failed to allocate framebuffer\n", __func__);
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto failed_free_io;
 	}
 
 	fbi->fb.fix.smem_start	= fb_mem_phys;
@@ -447,9 +447,6 @@ failed_free_io:
 	iounmap(fbi->regbase);
 failed_free_res:
 	release_mem_region(res->start, resource_size(res));
-failed_fbi:
-	kfree(fbi);
-failed:
 	return ret;
 }
 

^ permalink raw reply related

* [PATCH RESEND] drivers: video: metronomefb: avoid out-of-bounds array access
From: Michal Nazarewicz @ 2013-11-29 16:51 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: linux-fbdev, linux-kernel

From: Michal Nazarewicz <mina86@mina86.com>

load_waveform function checks whether padding bytes in stuff2a
and stuff2b are all zero, but does so by treating those arrays
as a single longer array.  Since the structure is packed, and
the size sum matches, it all works, but creates some confusion
in the code.

This commit changes the stuff2a and stuff2b arrays into pad1 and
pad2 fields such that they cover the same bytes as the arrays
covered, and changes the check in the load_waveform function so
that the fields are read instead of iterating over an arary.

It also renames the other “stuff” fields to “ignore*” fields to
give them more semantic meaning.

Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
 drivers/video/metronomefb.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/video/metronomefb.c b/drivers/video/metronomefb.c
index 195cc2d..4f36a2b 100644
--- a/drivers/video/metronomefb.c
+++ b/drivers/video/metronomefb.c
@@ -126,7 +126,7 @@ static struct fb_var_screeninfo metronomefb_var = {
 
 /* the waveform structure that is coming from userspace firmware */
 struct waveform_hdr {
-	u8 stuff[32];
+	u8 ignore1[32];
 
 	u8 wmta[3];
 	u8 fvsn;
@@ -134,13 +134,14 @@ struct waveform_hdr {
 	u8 luts;
 	u8 mc;
 	u8 trc;
-	u8 stuff3;
+	u8 ignore2;
 
 	u8 endb;
 	u8 swtb;
-	u8 stuff2a[2];
+	u32 pad1; /* u16 halfof(pad1) */
 
-	u8 stuff2b[3];
+	/* u16 halfof(pad1) */
+	u8 pad2;
 	u8 wfm_cs;
 } __attribute__ ((packed));
 
@@ -210,11 +211,9 @@ static int load_waveform(u8 *mem, size_t size, int m, int t,
 	}
 	wfm_hdr->mc += 1;
 	wfm_hdr->trc += 1;
-	for (i = 0; i < 5; i++) {
-		if (*(wfm_hdr->stuff2a + i) != 0) {
-			dev_err(dev, "Error: unexpected value in padding\n");
-			return -EINVAL;
-		}
+	if (wfm_hdr->pad1 || wfm_hdr->pad2) {
+		dev_err(dev, "Error: unexpected value in padding\n");
+		return -EINVAL;
 	}
 
 	/* calculating trn. trn is something used to index into
-- 
1.8.4.1


^ permalink raw reply related

* [PATCH v4] video: add OpenCores VGA/LCD framebuffer driver
From: Stefan Kristiansson @ 2013-11-29 15:45 UTC (permalink / raw)
  To: linux-kernel, linux-fbdev; +Cc: tomi.valkeinen, plagnioj, Stefan Kristiansson

This adds support for the VGA/LCD core available from OpenCores:
http://opencores.org/project,vga_lcd

The driver have been tested together with both OpenRISC and
ARM (socfpga) processors.

Signed-off-by: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>
---
Changes in v2:
- Add Microblaze as an example user and fix a typo in Xilinx Zynq

Changes in v3:
- Use devm_kzalloc instead of kzalloc
- Remove superflous MODULE #ifdef

Changes in v4:
- Remove 'default n' in Kconfig
- Simplify ioremap/request_mem_region by using devm_ioremap_resource
- Remove release_mem_region
---
 drivers/video/Kconfig  |  16 ++
 drivers/video/Makefile |   1 +
 drivers/video/ocfb.c   | 454 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 471 insertions(+)
 create mode 100644 drivers/video/ocfb.c

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 84b685f..8e41a1e 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -979,6 +979,22 @@ config FB_PVR2
 	  (<file:drivers/video/pvr2fb.c>). Please see the file
 	  <file:Documentation/fb/pvr2fb.txt>.
 
+config FB_OPENCORES
+	tristate "OpenCores VGA/LCD core 2.0 framebuffer support"
+	depends on FB
+	select FB_CFB_FILLRECT
+	select FB_CFB_COPYAREA
+	select FB_CFB_IMAGEBLIT
+	help
+	  This enables support for the OpenCores VGA/LCD core.
+
+	  The OpenCores VGA/LCD core is typically used together with
+	  softcore CPUs (e.g. OpenRISC or Microblaze) or hard processor
+	  systems (e.g. Altera socfpga or Xilinx Zynq) on FPGAs.
+
+	  The source code and specification for the core is available at
+	  <http://opencores.org/project,vga_lcd>
+
 config FB_S1D13XXX
 	tristate "Epson S1D13XXX framebuffer support"
 	depends on FB
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index e8bae8d..ae17ddf 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -150,6 +150,7 @@ obj-$(CONFIG_FB_NUC900)           += nuc900fb.o
 obj-$(CONFIG_FB_JZ4740)		  += jz4740_fb.o
 obj-$(CONFIG_FB_PUV3_UNIGFX)      += fb-puv3.o
 obj-$(CONFIG_FB_HYPERV)		  += hyperv_fb.o
+obj-$(CONFIG_FB_OPENCORES)	  += ocfb.o
 
 # Platform or fallback drivers go here
 obj-$(CONFIG_FB_UVESA)            += uvesafb.o
diff --git a/drivers/video/ocfb.c b/drivers/video/ocfb.c
new file mode 100644
index 0000000..51e9795
--- /dev/null
+++ b/drivers/video/ocfb.c
@@ -0,0 +1,454 @@
+/*
+ * OpenCores VGA/LCD 2.0 core frame buffer driver
+ *
+ * Copyright (C) 2013 Stefan Kristiansson, stefan.kristiansson@saunalahti.fi
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/delay.h>
+#include <linux/mm.h>
+#include <linux/dma-mapping.h>
+#include <linux/fb.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+
+/* OCFB register defines */
+#define OCFB_CTRL	0x000
+#define OCFB_STAT	0x004
+#define OCFB_HTIM	0x008
+#define OCFB_VTIM	0x00c
+#define OCFB_HVLEN	0x010
+#define OCFB_VBARA	0x014
+#define OCFB_PALETTE	0x800
+
+#define OCFB_CTRL_VEN	0x00000001 /* Video Enable */
+#define OCFB_CTRL_HIE	0x00000002 /* HSync Interrupt Enable */
+#define OCFB_CTRL_PC	0x00000800 /* 8-bit Pseudo Color Enable*/
+#define OCFB_CTRL_CD8	0x00000000 /* Color Depth 8 */
+#define OCFB_CTRL_CD16	0x00000200 /* Color Depth 16 */
+#define OCFB_CTRL_CD24	0x00000400 /* Color Depth 24 */
+#define OCFB_CTRL_CD32	0x00000600 /* Color Depth 32 */
+#define OCFB_CTRL_VBL1	0x00000000 /* Burst Length 1 */
+#define OCFB_CTRL_VBL2	0x00000080 /* Burst Length 2 */
+#define OCFB_CTRL_VBL4	0x00000100 /* Burst Length 4 */
+#define OCFB_CTRL_VBL8	0x00000180 /* Burst Length 8 */
+
+#define PALETTE_SIZE	256
+
+#define OCFB_NAME	"OC VGA/LCD"
+
+static char *mode_option;
+
+static const struct fb_videomode default_mode = {
+	/* 640x480 @ 60 Hz, 31.5 kHz hsync */
+	NULL, 60, 640, 480, 39721, 40, 24, 32, 11, 96, 2,
+	0, FB_VMODE_NONINTERLACED
+};
+
+struct ocfb_dev {
+	struct fb_info info;
+	void __iomem *regs;
+	/* flag indicating whether the regs are little endian accessed */
+	int little_endian;
+	/* Physical and virtual addresses of framebuffer */
+	phys_addr_t fb_phys;
+	void __iomem *fb_virt;
+	u32 pseudo_palette[PALETTE_SIZE];
+};
+
+struct ocfb_par {
+	void __iomem *pal_adr;
+};
+
+static struct ocfb_par ocfb_par_priv;
+
+static struct fb_var_screeninfo ocfb_var;
+static struct fb_fix_screeninfo ocfb_fix;
+
+#ifndef MODULE
+static int __init ocfb_setup(char *options)
+{
+	char *curr_opt;
+
+	if (!options || !*options)
+		return 0;
+
+	while ((curr_opt = strsep(&options, ",")) != NULL) {
+		if (!*curr_opt)
+			continue;
+		mode_option = curr_opt;
+	}
+
+	return 0;
+}
+#endif
+
+static inline u32 ocfb_readreg(struct ocfb_dev *fbdev, loff_t offset)
+{
+	if (fbdev->little_endian)
+		return ioread32(fbdev->regs + offset);
+	else
+		return ioread32be(fbdev->regs + offset);
+}
+
+static void ocfb_writereg(struct ocfb_dev *fbdev, loff_t offset, u32 data)
+{
+	if (fbdev->little_endian)
+		iowrite32(data, fbdev->regs + offset);
+	else
+		iowrite32be(data, fbdev->regs + offset);
+}
+
+static int ocfb_setupfb(struct ocfb_dev *fbdev)
+{
+	unsigned long bpp_config;
+	struct fb_var_screeninfo *var = &fbdev->info.var;
+	struct device *dev = fbdev->info.device;
+	u32 hlen;
+	u32 vlen;
+
+	/* Disable display */
+	ocfb_writereg(fbdev, OCFB_CTRL, 0);
+
+	/* Register framebuffer address */
+	fbdev->little_endian = 0;
+	ocfb_writereg(fbdev, OCFB_VBARA, fbdev->fb_phys);
+
+	/* Detect endianess */
+	if (ocfb_readreg(fbdev, OCFB_VBARA) != fbdev->fb_phys) {
+		fbdev->little_endian = 1;
+		ocfb_writereg(fbdev, OCFB_VBARA, fbdev->fb_phys);
+	}
+
+	/* Horizontal timings */
+	ocfb_writereg(fbdev, OCFB_HTIM, (var->hsync_len - 1) << 24 |
+		      (var->right_margin - 1) << 16 | (var->xres - 1));
+
+	/* Vertical timings */
+	ocfb_writereg(fbdev, OCFB_VTIM, (var->vsync_len - 1) << 24 |
+		      (var->lower_margin - 1) << 16 | (var->yres - 1));
+
+	/* Total length of frame */
+	hlen = var->left_margin + var->right_margin + var->hsync_len +
+		var->xres;
+
+	vlen = var->upper_margin + var->lower_margin + var->vsync_len +
+		var->yres;
+
+	ocfb_writereg(fbdev, OCFB_HVLEN, (hlen - 1) << 16 | (vlen - 1));
+
+	bpp_config = OCFB_CTRL_CD8;
+	switch (var->bits_per_pixel) {
+	case 8:
+		if (!var->grayscale)
+			bpp_config |= OCFB_CTRL_PC;  /* enable palette */
+		break;
+
+	case 16:
+		bpp_config |= OCFB_CTRL_CD16;
+		break;
+
+	case 24:
+		bpp_config |= OCFB_CTRL_CD24;
+		break;
+
+	case 32:
+		bpp_config |= OCFB_CTRL_CD32;
+		break;
+
+	default:
+		dev_err(dev, "no bpp specified\n");
+		break;
+	}
+
+	/* maximum (8) VBL (video memory burst length) */
+	bpp_config |= OCFB_CTRL_VBL8;
+
+	/* Enable output */
+	ocfb_writereg(fbdev, OCFB_CTRL, (OCFB_CTRL_VEN | bpp_config));
+
+	return 0;
+}
+
+static int ocfb_setcolreg(unsigned regno, unsigned red, unsigned green,
+			  unsigned blue, unsigned transp,
+			  struct fb_info *info)
+{
+	struct ocfb_par *par = (struct ocfb_par *)info->par;
+	u32 color;
+
+	if (regno >= info->cmap.len) {
+		dev_err(info->device, "regno >= cmap.len\n");
+		return 1;
+	}
+
+	if (info->var.grayscale) {
+		/* grayscale = 0.30*R + 0.59*G + 0.11*B */
+		red = green = blue = (red * 77 + green * 151 + blue * 28) >> 8;
+	}
+
+	red >>= (16 - info->var.red.length);
+	green >>= (16 - info->var.green.length);
+	blue >>= (16 - info->var.blue.length);
+	transp >>= (16 - info->var.transp.length);
+
+	if (info->var.bits_per_pixel = 8 && !info->var.grayscale) {
+		regno <<= 2;
+		color = (red << 16) | (green << 8) | blue;
+		ocfb_writereg(par->pal_adr, regno, color);
+	} else {
+		((u32 *)(info->pseudo_palette))[regno] +			(red << info->var.red.offset) |
+			(green << info->var.green.offset) |
+			(blue << info->var.blue.offset) |
+			(transp << info->var.transp.offset);
+	}
+
+	return 0;
+}
+
+static int ocfb_init_fix(struct ocfb_dev *fbdev)
+{
+	struct fb_var_screeninfo *var = &fbdev->info.var;
+	struct fb_fix_screeninfo *fix = &fbdev->info.fix;
+
+	strcpy(fix->id, OCFB_NAME);
+
+	fix->line_length = var->xres * var->bits_per_pixel/8;
+	fix->smem_len = fix->line_length * var->yres;
+	fix->type = FB_TYPE_PACKED_PIXELS;
+
+	if (var->bits_per_pixel = 8 && !var->grayscale)
+		fix->visual = FB_VISUAL_PSEUDOCOLOR;
+	else
+		fix->visual = FB_VISUAL_TRUECOLOR;
+
+	return 0;
+}
+
+static int ocfb_init_var(struct ocfb_dev *fbdev)
+{
+	struct fb_var_screeninfo *var = &fbdev->info.var;
+
+	var->accel_flags = FB_ACCEL_NONE;
+	var->activate = FB_ACTIVATE_NOW;
+	var->xres_virtual = var->xres;
+	var->yres_virtual = var->yres;
+
+	switch (var->bits_per_pixel) {
+	case 8:
+		var->transp.offset = 0;
+		var->transp.length = 0;
+		var->red.offset = 0;
+		var->red.length = 8;
+		var->green.offset = 0;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		break;
+
+	case 16:
+		var->transp.offset = 0;
+		var->transp.length = 0;
+		var->red.offset = 11;
+		var->red.length = 5;
+		var->green.offset = 5;
+		var->green.length = 6;
+		var->blue.offset = 0;
+		var->blue.length  = 5;
+		break;
+
+	case 24:
+		var->transp.offset = 0;
+		var->transp.length = 0;
+		var->red.offset = 16;
+		var->red.length = 8;
+		var->green.offset = 8;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		break;
+
+	case 32:
+		var->transp.offset = 24;
+		var->transp.length = 8;
+		var->red.offset = 16;
+		var->red.length = 8;
+		var->green.offset = 8;
+		var->green.length = 8;
+		var->blue.offset = 0;
+		var->blue.length = 8;
+		break;
+	}
+
+	return 0;
+}
+
+static struct fb_ops ocfb_ops = {
+	.owner		= THIS_MODULE,
+	.fb_setcolreg	= ocfb_setcolreg,
+	.fb_fillrect	= cfb_fillrect,
+	.fb_copyarea	= cfb_copyarea,
+	.fb_imageblit	= cfb_imageblit,
+};
+
+static int ocfb_probe(struct platform_device *pdev)
+{
+	int ret = 0;
+	struct ocfb_dev *fbdev;
+	struct ocfb_par *par = &ocfb_par_priv;
+	struct resource *res;
+	int fbsize;
+
+	fbdev = devm_kzalloc(&pdev->dev, sizeof(*fbdev), GFP_KERNEL);
+	if (!fbdev)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, fbdev);
+
+	fbdev->info.fbops = &ocfb_ops;
+	fbdev->info.var = ocfb_var;
+	fbdev->info.fix = ocfb_fix;
+	fbdev->info.device = &pdev->dev;
+	fbdev->info.par = par;
+
+	/* Video mode setup */
+	if (!fb_find_mode(&fbdev->info.var, &fbdev->info, mode_option,
+			  NULL, 0, &default_mode, 16)) {
+		dev_err(&pdev->dev, "No valid video modes found\n");
+		return -EINVAL;
+	}
+	ocfb_init_var(fbdev);
+	ocfb_init_fix(fbdev);
+
+	/* Request I/O resource */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(&pdev->dev, "I/O resource request failed\n");
+		return -ENXIO;
+	}
+	res->flags &= ~IORESOURCE_CACHEABLE;
+	fbdev->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(fbdev->regs))
+		return PTR_ERR(fbdev->regs);
+
+	par->pal_adr = fbdev->regs + OCFB_PALETTE;
+
+	/* Allocate framebuffer memory */
+	fbsize = fbdev->info.fix.smem_len;
+	fbdev->fb_virt = dma_alloc_coherent(&pdev->dev, PAGE_ALIGN(fbsize),
+					    &fbdev->fb_phys, GFP_KERNEL);
+	if (!fbdev->fb_virt) {
+		dev_err(&pdev->dev,
+			"Frame buffer memory allocation failed\n");
+		return -ENOMEM;
+	}
+	fbdev->info.fix.smem_start = fbdev->fb_phys;
+	fbdev->info.screen_base = (void __iomem *)fbdev->fb_virt;
+	fbdev->info.pseudo_palette = fbdev->pseudo_palette;
+
+	/* Clear framebuffer */
+	memset_io((void __iomem *)fbdev->fb_virt, 0, fbsize);
+
+	/* Setup and enable the framebuffer */
+	ocfb_setupfb(fbdev);
+
+	if (fbdev->little_endian)
+		fbdev->info.flags |= FBINFO_FOREIGN_ENDIAN;
+
+	/* Allocate color map */
+	ret = fb_alloc_cmap(&fbdev->info.cmap, PALETTE_SIZE, 0);
+	if (ret) {
+		dev_err(&pdev->dev, "Color map allocation failed\n");
+		goto err_dma_free;
+	}
+
+	/* Register framebuffer */
+	ret = register_framebuffer(&fbdev->info);
+	if (ret) {
+		dev_err(&pdev->dev, "Framebuffer registration failed\n");
+		goto err_dealloc_cmap;
+	}
+
+	return 0;
+
+err_dealloc_cmap:
+	fb_dealloc_cmap(&fbdev->info.cmap);
+
+err_dma_free:
+	dma_free_coherent(&pdev->dev, PAGE_ALIGN(fbsize), fbdev->fb_virt,
+			  fbdev->fb_phys);
+
+	return ret;
+}
+
+static int ocfb_remove(struct platform_device *pdev)
+{
+	struct ocfb_dev *fbdev = platform_get_drvdata(pdev);
+
+	unregister_framebuffer(&fbdev->info);
+	fb_dealloc_cmap(&fbdev->info.cmap);
+	dma_free_coherent(&pdev->dev, PAGE_ALIGN(fbdev->info.fix.smem_len),
+			  fbdev->fb_virt, fbdev->fb_phys);
+
+	/* Disable display */
+	ocfb_writereg(fbdev, OCFB_CTRL, 0);
+
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static struct of_device_id ocfb_match[] = {
+	{ .compatible = "opencores,ocfb", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ocfb_match);
+
+static struct platform_driver ocfb_driver = {
+	.probe  = ocfb_probe,
+	.remove	= ocfb_remove,
+	.driver = {
+		.name = "ocfb_fb",
+		.of_match_table = ocfb_match,
+	}
+};
+
+/*
+ * Init and exit routines
+ */
+static int __init ocfb_init(void)
+{
+#ifndef MODULE
+	char *option = NULL;
+
+	if (fb_get_options("ocfb", &option))
+		return -ENODEV;
+	ocfb_setup(option);
+#endif
+	return platform_driver_register(&ocfb_driver);
+}
+
+static void __exit ocfb_exit(void)
+{
+	platform_driver_unregister(&ocfb_driver);
+}
+
+module_init(ocfb_init);
+module_exit(ocfb_exit);
+
+MODULE_AUTHOR("Stefan Kristiansson <stefan.kristiansson@saunalahti.fi>");
+MODULE_DESCRIPTION("OpenCores VGA/LCD 2.0 frame buffer driver");
+MODULE_LICENSE("GPL v2");
+module_param(mode_option, charp, 0);
+MODULE_PARM_DESC(mode_option, "Video mode ('<xres>x<yres>[-<bpp>][@refresh]')");
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCH 01/28] video: arkfb: use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  4:39 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <002901ceebe7$c3eec160$4bcc4420$%han@samsung.com>

On Thursday, November 28, 2013 12:13 PM, Jingoo Han
> 
> This macro is used to create a struct pci_device_id array.
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>

Please, ignore these patches.
According to the Greg Kroah-Hartman, 

"Yeah, and it's a horrid macro that deserves to be removed, please don't
use it in more places.

Actually, if you could just remove it, that would be best, sorry, I'm
not going to take these patches."

So, I will send the patch to remove 'DEFINE_PCI_DEVICE_TABLE' instead.
Sorry for annoying. :-)

Best regards,
Jingoo Han

> ---
>  drivers/video/arkfb.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/arkfb.c b/drivers/video/arkfb.c
> index a6b29bd..cc9f8a2 100644
> --- a/drivers/video/arkfb.c
> +++ b/drivers/video/arkfb.c
> @@ -1183,7 +1183,7 @@ fail:
> 
>  /* List of boards that we are trying to support */
> 
> -static struct pci_device_id ark_devices[] = {
> +static DEFINE_PCI_DEVICE_TABLE(ark_devices) = {
>  	{PCI_DEVICE(0xEDD8, 0xA099)},
>  	{0, 0, 0, 0, 0, 0, 0}
>  };
> --
> 1.7.10.4



^ permalink raw reply

* [PATCH 28/28] video: vt8623fb: use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  3:35 UTC (permalink / raw)
  To: linux-fbdev

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/vt8623fb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/vt8623fb.c b/drivers/video/vt8623fb.c
index 8bc6e09..f210d0f 100644
--- a/drivers/video/vt8623fb.c
+++ b/drivers/video/vt8623fb.c
@@ -907,7 +907,7 @@ fail:
 
 /* List of boards that we are trying to support */
 
-static struct pci_device_id vt8623_devices[] = {
+static DEFINE_PCI_DEVICE_TABLE(vt8623_devices) = {
 	{PCI_DEVICE(PCI_VENDOR_ID_VIA, 0x3122)},
 	{0, 0, 0, 0, 0, 0, 0}
 };
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 27/28] video: tridentfb: use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  3:34 UTC (permalink / raw)
  To: linux-fbdev

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/tridentfb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/tridentfb.c b/drivers/video/tridentfb.c
index 7ed9a22..ba7dc99 100644
--- a/drivers/video/tridentfb.c
+++ b/drivers/video/tridentfb.c
@@ -1559,7 +1559,7 @@ static void trident_pci_remove(struct pci_dev *dev)
 }
 
 /* List of boards that we are trying to support */
-static struct pci_device_id trident_devices[] = {
+static DEFINE_PCI_DEVICE_TABLE(trident_devices) = {
 	{PCI_VENDOR_ID_TRIDENT,	BLADE3D, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
 	{PCI_VENDOR_ID_TRIDENT,	CYBERBLADEi7, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
 	{PCI_VENDOR_ID_TRIDENT,	CYBERBLADEi7D, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 26/28] video: tgafb: use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  3:33 UTC (permalink / raw)
  To: linux-fbdev

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/tgafb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/tgafb.c b/drivers/video/tgafb.c
index f28674f..09f662f 100644
--- a/drivers/video/tgafb.c
+++ b/drivers/video/tgafb.c
@@ -96,7 +96,7 @@ static struct fb_ops tgafb_ops = {
 static int tgafb_pci_register(struct pci_dev *, const struct pci_device_id *);
 static void tgafb_pci_unregister(struct pci_dev *);
 
-static struct pci_device_id const tgafb_pci_table[] = {
+static DEFINE_PCI_DEVICE_TABLE(tgafb_pci_table) = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_DEC, PCI_DEVICE_ID_DEC_TGA) },
 	{ }
 };
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 25/28] video: tdfxfb: use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  3:32 UTC (permalink / raw)
  To: linux-fbdev

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/tdfxfb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/tdfxfb.c b/drivers/video/tdfxfb.c
index f761fe3..61369ce 100644
--- a/drivers/video/tdfxfb.c
+++ b/drivers/video/tdfxfb.c
@@ -138,7 +138,7 @@ static struct fb_var_screeninfo tdfx_var = {
 static int tdfxfb_probe(struct pci_dev *pdev, const struct pci_device_id *id);
 static void tdfxfb_remove(struct pci_dev *pdev);
 
-static struct pci_device_id tdfxfb_id_table[] = {
+static DEFINE_PCI_DEVICE_TABLE(tdfxfb_id_table) = {
 	{ PCI_VENDOR_ID_3DFX, PCI_DEVICE_ID_3DFX_BANSHEE,
 	  PCI_ANY_ID, PCI_ANY_ID, PCI_BASE_CLASS_DISPLAY << 16,
 	  0xff0000, 0 },
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 24/28] video: sstfb: use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  3:32 UTC (permalink / raw)
  To: linux-fbdev

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/sstfb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/sstfb.c b/drivers/video/sstfb.c
index f0cb279..f132691 100644
--- a/drivers/video/sstfb.c
+++ b/drivers/video/sstfb.c
@@ -1477,7 +1477,7 @@ static void sstfb_remove(struct pci_dev *pdev)
 }
 
 
-static const struct pci_device_id sstfb_id_tbl[] = {
+static DEFINE_PCI_DEVICE_TABLE(sstfb_id_tbl) = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_3DFX, PCI_DEVICE_ID_3DFX_VOODOO ),
 		.driver_data = ID_VOODOO1, },
 	{ PCI_DEVICE(PCI_VENDOR_ID_3DFX, PCI_DEVICE_ID_3DFX_VOODOO2),
-- 
1.7.10.4



^ permalink raw reply related

* [PATCH 23/28] video: sisfb: use DEFINE_PCI_DEVICE_TABLE macro
From: Jingoo Han @ 2013-11-28  3:31 UTC (permalink / raw)
  To: linux-fbdev

This macro is used to create a struct pci_device_id array.

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/video/sis/sis_main.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/sis/sis_main.h b/drivers/video/sis/sis_main.h
index 32e23c2..91dd282 100644
--- a/drivers/video/sis/sis_main.h
+++ b/drivers/video/sis/sis_main.h
@@ -113,7 +113,7 @@ static struct sisfb_chip_info {
 	{ XGI_40,     SIS_315_VGA, 1, HW_CURSOR_AREA_SIZE_315 * 4, SIS_CRT2_WENABLE_315, "XGI V3XT/V5/V8" },
 };
 
-static struct pci_device_id sisfb_pci_table[] = {
+static DEFINE_PCI_DEVICE_TABLE(sisfb_pci_table) = {
 #ifdef CONFIG_FB_SIS_300
 	{ PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_300,     PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
 	{ PCI_VENDOR_ID_SI, PCI_DEVICE_ID_SI_540_VGA, PCI_ANY_ID, PCI_ANY_ID, 0, 0, 1},
-- 
1.7.10.4



^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox