[PATCH 1/3] video: fbdev: omap2: omapfb: remove __exit annotation

[PATCH 1/3] video: fbdev: omap2: omapfb: remove __exit annotation

[Felipe Balbi]

if we leave __exit annotation, driver can't be unbound
through sysfs.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
index ec2d132..9cbf1ce 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
@@ -2619,7 +2619,7 @@ err0:
 	return r;
 }
 
-static int __exit omapfb_remove(struct platform_device *pdev)
+static int omapfb_remove(struct platform_device *pdev)
 {
 	struct omapfb2_device *fbdev = platform_get_drvdata(pdev);
 
@@ -2636,7 +2636,7 @@ static int __exit omapfb_remove(struct platform_device *pdev)
 
 static struct platform_driver omapfb_driver = {
 	.probe		= omapfb_probe,
-	.remove         = __exit_p(omapfb_remove),
+	.remove         = omapfb_remove,
 	.driver         = {
 		.name   = "omapfb",
 		.owner  = THIS_MODULE,
-- 
2.1.0.GIT

[PATCH 2/3] video: fbdev: omap2: omapfb: add missing MODULE_ALIAS()

[Felipe Balbi]

without MODULE_ALIAS(), omapfb won't get loaded
automatically.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
index 9cbf1ce..b4b9244 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
@@ -2651,6 +2651,7 @@ module_param_named(mirror, def_mirror, bool, 0);
 
 module_platform_driver(omapfb_driver);
 
+MODULE_ALIAS("platform:omapfb");
 MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@nokia.com>");
 MODULE_DESCRIPTION("OMAP2/3 Framebuffer");
 MODULE_LICENSE("GPL v2");
-- 
2.1.0.GIT

[PATCH 3/3] arm: boot: dts: am437x-sk: fix lcd enable pin mux data

[Felipe Balbi]

Caused by a copy & paste error. Note that even with
this bug AM437x SK display still works because GPIO
mux mode is always enabled. It's still wrong to mux
somebody else's pin.

Luckily ball D25 (offset 0x238 - gpio5_8) on AM437x
isn't used for anything.

Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/boot/dts/am437x-sk-evm.dts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
index 859ff3d..681be00 100644
--- a/arch/arm/boot/dts/am437x-sk-evm.dts
+++ b/arch/arm/boot/dts/am437x-sk-evm.dts
@@ -320,8 +320,7 @@
 
 	lcd_pins: lcd_pins {
 		pinctrl-single,pins = <
-			/* GPIO 5_8 to select LCD / HDMI */
-			0x238 (PIN_OUTPUT_PULLUP | MUX_MODE7)
+			0x1c (PIN_OUTPUT_PULLUP | MUX_MODE7) /* gpcm_ad7.gpio1_7 */
 		>;
 	};
 };
-- 
2.1.0.GIT

[PATCH 2/3] video: fbdev: omap2: omapfb: add missing MODULE_ALIAS()

[Felipe Balbi]

On Tue, Oct 14, 2014 at 01:28:54PM -0500, Felipe Balbi wrote:
> without MODULE_ALIAS(), omapfb won't get loaded
> automatically.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>

little note here. This makes omapfb load automatically, but display
still doesn't work with DSS as modules. Backlight is working and pixel
clock is running just fine. Still, nothing on display.

I thought I'd leave that for Tomi to deal with.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141014/59786856/attachment.sig>

[PATCH 1/3] video: fbdev: omap2: omapfb: remove __exit annotation

[Tomi Valkeinen]

Hi,

On 14/10/14 21:28, Felipe Balbi wrote:
> if we leave __exit annotation, driver can't be unbound
> through sysfs.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> index ec2d132..9cbf1ce 100644
> --- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> @@ -2619,7 +2619,7 @@ err0:
>  	return r;
>  }
>  
> -static int __exit omapfb_remove(struct platform_device *pdev)
> +static int omapfb_remove(struct platform_device *pdev)
>  {
>  	struct omapfb2_device *fbdev = platform_get_drvdata(pdev);
>  
> @@ -2636,7 +2636,7 @@ static int __exit omapfb_remove(struct platform_device *pdev)
>  
>  static struct platform_driver omapfb_driver = {
>  	.probe		= omapfb_probe,
> -	.remove         = __exit_p(omapfb_remove),
> +	.remove         = omapfb_remove,
>  	.driver         = {
>  		.name   = "omapfb",
>  		.owner  = THIS_MODULE,

Interesting. I don't know if I'm doing something funny, but without this
patch, I can unbind omapfb, kind of.

"echo omapfb > unbind" goes ok, but remove is obviously not called.
Somehow omapfb device is still unbound from the driver, as I can then
bind it again, causing probe to be called. Which breaks everything.

I would've thought that unbinding is not possible if remove is missing,
but that doesn't seem to be the case. I guess it just means that remove
is not called when the driver & device are unbound.

We have 18 __exit_p()s in omapdss and related drivers. I guess they are
all broken the same way.

Note that omapfb unbind & bind does not work even with this patch, but
results in a crash as some old state is left into omapdss. The same
happens also with unloading and loading omapfb module (but keeping
omapdss module loaded).

So there seems to be more issues around this.

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141015/1805204c/attachment.sig>

[PATCH 2/3] video: fbdev: omap2: omapfb: add missing MODULE_ALIAS()

[Tomi Valkeinen]

On 14/10/14 21:28, Felipe Balbi wrote:
> without MODULE_ALIAS(), omapfb won't get loaded
> automatically.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> index 9cbf1ce..b4b9244 100644
> --- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> @@ -2651,6 +2651,7 @@ module_param_named(mirror, def_mirror, bool, 0);
>  
>  module_platform_driver(omapfb_driver);
>  
> +MODULE_ALIAS("platform:omapfb");
>  MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@nokia.com>");
>  MODULE_DESCRIPTION("OMAP2/3 Framebuffer");
>  MODULE_LICENSE("GPL v2");
> 

Thanks, I've queued this.

Unfortunately it's somewhat based on luck if the automatic loading works
correctly. We can't add more displays after omapfb has been probed, so
all the panel and encoder drivers have to be loaded before omapfb.

We have two workarounds there, which help the situation a bit. First is
that if omapfb finds no displays, it returns EPROBE_DEFER. The second is
that if there are displays, but no main display (display0), then omapfb
returns EPROBE_DEFER.

So even with these workarounds it is possible that drivers for secondary
displays are loaded after omapfb, causing them to be ignored.

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141015/b023c80a/attachment-0001.sig>

[PATCH 3/3] arm: boot: dts: am437x-sk: fix lcd enable pin mux data

[Tomi Valkeinen]

On 14/10/14 21:28, Felipe Balbi wrote:
> Caused by a copy & paste error. Note that even with
> this bug AM437x SK display still works because GPIO
> mux mode is always enabled. It's still wrong to mux
> somebody else's pin.
> 
> Luckily ball D25 (offset 0x238 - gpio5_8) on AM437x
> isn't used for anything.
> 
> Signed-off-by: Felipe Balbi <balbi@ti.com>
> ---
>  arch/arm/boot/dts/am437x-sk-evm.dts | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
> index 859ff3d..681be00 100644
> --- a/arch/arm/boot/dts/am437x-sk-evm.dts
> +++ b/arch/arm/boot/dts/am437x-sk-evm.dts
> @@ -320,8 +320,7 @@
>  
>  	lcd_pins: lcd_pins {
>  		pinctrl-single,pins = <
> -			/* GPIO 5_8 to select LCD / HDMI */
> -			0x238 (PIN_OUTPUT_PULLUP | MUX_MODE7)
> +			0x1c (PIN_OUTPUT_PULLUP | MUX_MODE7) /* gpcm_ad7.gpio1_7 */
>  		>;
>  	};
>  };

I didn't verify the offset, but based on the comments this looks fine to me.

Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141015/6cb913a6/attachment.sig>

[PATCH 2/3] video: fbdev: omap2: omapfb: add missing MODULE_ALIAS()

[Tomi Valkeinen]

On 14/10/14 21:34, Felipe Balbi wrote:
> On Tue, Oct 14, 2014 at 01:28:54PM -0500, Felipe Balbi wrote:
>> without MODULE_ALIAS(), omapfb won't get loaded
>> automatically.
>>
>> Signed-off-by: Felipe Balbi <balbi@ti.com>
> 
> little note here. This makes omapfb load automatically, but display
> still doesn't work with DSS as modules. Backlight is working and pixel
> clock is running just fine. Still, nothing on display.
> 
> I thought I'd leave that for Tomi to deal with.

I presume this is about am437x-sk. I don't have the board, so there's
not too much I can do except send emails.

So the panel works fine if display drivers are built-in?

The compatible string seems to be wrong in the .dts file. It says
"osddisplays,osd057T0559-34ts" which is not the panel used. But that
shouldn't affect this.

What do you mean with "pixel clock is running"?

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141015/ba68b053/attachment-0001.sig>

[PATCH 2/3] video: fbdev: omap2: omapfb: add missing MODULE_ALIAS()

[Felipe Balbi]

HI,

On Wed, Oct 15, 2014 at 03:46:10PM +0300, Tomi Valkeinen wrote:
> On 14/10/14 21:34, Felipe Balbi wrote:
> > On Tue, Oct 14, 2014 at 01:28:54PM -0500, Felipe Balbi wrote:
> >> without MODULE_ALIAS(), omapfb won't get loaded
> >> automatically.
> >>
> >> Signed-off-by: Felipe Balbi <balbi@ti.com>
> > 
> > little note here. This makes omapfb load automatically, but display
> > still doesn't work with DSS as modules. Backlight is working and pixel
> > clock is running just fine. Still, nothing on display.
> > 
> > I thought I'd leave that for Tomi to deal with.
> 
> I presume this is about am437x-sk. I don't have the board, so there's
> not too much I can do except send emails.
> 
> So the panel works fine if display drivers are built-in?

yup.

> The compatible string seems to be wrong in the .dts file. It says
> "osddisplays,osd057T0559-34ts" which is not the panel used. But that
> shouldn't affect this.

I got that from Darren, IIRC. Still, if it's wrong, it's wrong. What
should it be ?

> What do you mean with "pixel clock is running"?

poking at the pixel clock with a scope, I can see it clocking.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141015/ee51cf42/attachment.sig>

[PATCH 2/3] video: fbdev: omap2: omapfb: add missing MODULE_ALIAS()

[Felipe Balbi]

On Wed, Oct 15, 2014 at 03:20:19PM +0300, Tomi Valkeinen wrote:
> On 14/10/14 21:28, Felipe Balbi wrote:
> > without MODULE_ALIAS(), omapfb won't get loaded
> > automatically.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >  drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> > index 9cbf1ce..b4b9244 100644
> > --- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> > +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> > @@ -2651,6 +2651,7 @@ module_param_named(mirror, def_mirror, bool, 0);
> >  
> >  module_platform_driver(omapfb_driver);
> >  
> > +MODULE_ALIAS("platform:omapfb");
> >  MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@nokia.com>");
> >  MODULE_DESCRIPTION("OMAP2/3 Framebuffer");
> >  MODULE_LICENSE("GPL v2");
> > 
> 
> Thanks, I've queued this.
> 
> Unfortunately it's somewhat based on luck if the automatic loading works
> correctly. We can't add more displays after omapfb has been probed, so
> all the panel and encoder drivers have to be loaded before omapfb.
> 
> We have two workarounds there, which help the situation a bit. First is
> that if omapfb finds no displays, it returns EPROBE_DEFER. The second is
> that if there are displays, but no main display (display0), then omapfb
> returns EPROBE_DEFER.
> 
> So even with these workarounds it is possible that drivers for secondary
> displays are loaded after omapfb, causing them to be ignored.

then there is another case to fix, right ? :-)

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141015/1cdee349/attachment.sig>

[PATCH 1/3] video: fbdev: omap2: omapfb: remove __exit annotation

[Felipe Balbi]

Hi,

On Wed, Oct 15, 2014 at 03:13:34PM +0300, Tomi Valkeinen wrote:
> Hi,
> 
> On 14/10/14 21:28, Felipe Balbi wrote:
> > if we leave __exit annotation, driver can't be unbound
> > through sysfs.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >  drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> > index ec2d132..9cbf1ce 100644
> > --- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> > +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
> > @@ -2619,7 +2619,7 @@ err0:
> >  	return r;
> >  }
> >  
> > -static int __exit omapfb_remove(struct platform_device *pdev)
> > +static int omapfb_remove(struct platform_device *pdev)
> >  {
> >  	struct omapfb2_device *fbdev = platform_get_drvdata(pdev);
> >  
> > @@ -2636,7 +2636,7 @@ static int __exit omapfb_remove(struct platform_device *pdev)
> >  
> >  static struct platform_driver omapfb_driver = {
> >  	.probe		= omapfb_probe,
> > -	.remove         = __exit_p(omapfb_remove),
> > +	.remove         = omapfb_remove,
> >  	.driver         = {
> >  		.name   = "omapfb",
> >  		.owner  = THIS_MODULE,
> 
> Interesting. I don't know if I'm doing something funny, but without this
> patch, I can unbind omapfb, kind of.
> 
> "echo omapfb > unbind" goes ok, but remove is obviously not called.

remove isn't called because it won't exist if it's built-in. Look at the
definition of __exit_p()

> Somehow omapfb device is still unbound from the driver, as I can then
> bind it again, causing probe to be called. Which breaks everything.
> 
> I would've thought that unbinding is not possible if remove is missing,
> but that doesn't seem to be the case. I guess it just means that remove
> is not called when the driver & device are unbound.

if no remove it provided on platform_driver structure, platform bus
assumes you have nothing to do on your ->remove(), so you end up leaking
all resources you allocated on ->probe() (unless you *really* don't need
to do anything on ->remove).

> We have 18 __exit_p()s in omapdss and related drivers. I guess they are
> all broken the same way.

yup, I should've grepped.

> Note that omapfb unbind & bind does not work even with this patch, but
> results in a crash as some old state is left into omapdss. The same
> happens also with unloading and loading omapfb module (but keeping
> omapdss module loaded).

It worked fine for me. I unbound and bound omapfb multiple times.

> So there seems to be more issues around this.

quite a few more, I'd say

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141015/2c0f0d39/attachment.sig>

[PATCH 1/3] video: fbdev: omap2: omapfb: remove __exit annotation

[Tomi Valkeinen]

On 15/10/14 17:41, Felipe Balbi wrote:

>> Interesting. I don't know if I'm doing something funny, but without this
>> patch, I can unbind omapfb, kind of.
>>
>> "echo omapfb > unbind" goes ok, but remove is obviously not called.
> 
> remove isn't called because it won't exist if it's built-in. Look at the
> definition of __exit_p()

Yes, that's what I meant with "obviously".

>> Somehow omapfb device is still unbound from the driver, as I can then
>> bind it again, causing probe to be called. Which breaks everything.
>>
>> I would've thought that unbinding is not possible if remove is missing,
>> but that doesn't seem to be the case. I guess it just means that remove
>> is not called when the driver & device are unbound.
> 
> if no remove it provided on platform_driver structure, platform bus
> assumes you have nothing to do on your ->remove(), so you end up leaking
> all resources you allocated on ->probe() (unless you *really* don't need
> to do anything on ->remove).

Yep. That's quite odd, still. grep shows quite many uses of __exit_p(),
and all for remove callback. So, if you have something to release in
remove(), you should set it always, for both module and built-in. And if
you don't have anything to release, you would always just set .release
to NULL.

I mean, what's the use case for __exit_p()? With a quick glance, at
least some of the other users also use __exit_p() the same way omapdss
does (i.e. in the wrong way).

>> We have 18 __exit_p()s in omapdss and related drivers. I guess they are
>> all broken the same way.
> 
> yup, I should've grepped.
> 
>> Note that omapfb unbind & bind does not work even with this patch, but
>> results in a crash as some old state is left into omapdss. The same
>> happens also with unloading and loading omapfb module (but keeping
>> omapdss module loaded).
> 
> It worked fine for me. I unbound and bound omapfb multiple times.

Hmm, ok. Odd, the bug was quite clear and I think it should happen every
time. Well, I was using omap4. If you used AM4xx, that's basically omap3
DSS. Maybe there's a diff there.

>> So there seems to be more issues around this.
> 
> quite a few more, I'd say

Yep, I'll have a look at this.

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141015/93a1841a/attachment.sig>

[PATCH 2/3] video: fbdev: omap2: omapfb: add missing MODULE_ALIAS()

[Tomi Valkeinen]

On 15/10/14 17:38, Felipe Balbi wrote:
> On Wed, Oct 15, 2014 at 03:20:19PM +0300, Tomi Valkeinen wrote:
>> On 14/10/14 21:28, Felipe Balbi wrote:
>>> without MODULE_ALIAS(), omapfb won't get loaded
>>> automatically.
>>>
>>> Signed-off-by: Felipe Balbi <balbi@ti.com>
>>> ---
>>>  drivers/video/fbdev/omap2/omapfb/omapfb-main.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
>>> index 9cbf1ce..b4b9244 100644
>>> --- a/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
>>> +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-main.c
>>> @@ -2651,6 +2651,7 @@ module_param_named(mirror, def_mirror, bool, 0);
>>>  
>>>  module_platform_driver(omapfb_driver);
>>>  
>>> +MODULE_ALIAS("platform:omapfb");
>>>  MODULE_AUTHOR("Tomi Valkeinen <tomi.valkeinen@nokia.com>");
>>>  MODULE_DESCRIPTION("OMAP2/3 Framebuffer");
>>>  MODULE_LICENSE("GPL v2");
>>>
>>
>> Thanks, I've queued this.
>>
>> Unfortunately it's somewhat based on luck if the automatic loading works
>> correctly. We can't add more displays after omapfb has been probed, so
>> all the panel and encoder drivers have to be loaded before omapfb.
>>
>> We have two workarounds there, which help the situation a bit. First is
>> that if omapfb finds no displays, it returns EPROBE_DEFER. The second is
>> that if there are displays, but no main display (display0), then omapfb
>> returns EPROBE_DEFER.
>>
>> So even with these workarounds it is possible that drivers for secondary
>> displays are loaded after omapfb, causing them to be ignored.
> 
> then there is another case to fix, right ? :-)

Yes, but don't hold your breath. The issue's been there for ages and no
good solutions have been found. The same problem is there for omapdrm.

 Tomi


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141015/81112b04/attachment-0001.sig>

[PATCH 1/3] video: fbdev: omap2: omapfb: remove __exit annotation

[Felipe Balbi]

Hi,

On Wed, Oct 15, 2014 at 06:43:40PM +0300, Tomi Valkeinen wrote:
> >> Somehow omapfb device is still unbound from the driver, as I can then
> >> bind it again, causing probe to be called. Which breaks everything.
> >>
> >> I would've thought that unbinding is not possible if remove is missing,
> >> but that doesn't seem to be the case. I guess it just means that remove
> >> is not called when the driver & device are unbound.
> > 
> > if no remove it provided on platform_driver structure, platform bus
> > assumes you have nothing to do on your ->remove(), so you end up leaking
> > all resources you allocated on ->probe() (unless you *really* don't need
> > to do anything on ->remove).
> 
> Yep. That's quite odd, still. grep shows quite many uses of __exit_p(),
> and all for remove callback. So, if you have something to release in
> remove(), you should set it always, for both module and built-in. And if
> you don't have anything to release, you would always just set .release
> to NULL.
> 
> I mean, what's the use case for __exit_p()? With a quick glance, at
> least some of the other users also use __exit_p() the same way omapdss
> does (i.e. in the wrong way).

__exit_p() meant something else a few years back, perhaps those were
left over from some tree-wide cleanups.

> >> We have 18 __exit_p()s in omapdss and related drivers. I guess they are
> >> all broken the same way.
> > 
> > yup, I should've grepped.
> > 
> >> Note that omapfb unbind & bind does not work even with this patch, but
> >> results in a crash as some old state is left into omapdss. The same
> >> happens also with unloading and loading omapfb module (but keeping
> >> omapdss module loaded).
> > 
> > It worked fine for me. I unbound and bound omapfb multiple times.
> 
> Hmm, ok. Odd, the bug was quite clear and I think it should happen every
> time. Well, I was using omap4. If you used AM4xx, that's basically omap3
> DSS. Maybe there's a diff there.

could very well be :-)

> >> So there seems to be more issues around this.
> > 
> > quite a few more, I'd say
> 
> Yep, I'll have a look at this.

alright

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141015/9c26373d/attachment.sig>

[PATCH 3/3] arm: boot: dts: am437x-sk: fix lcd enable pin mux data

[Felipe Balbi]

HI,

On Wed, Oct 15, 2014 at 03:24:20PM +0300, Tomi Valkeinen wrote:
> On 14/10/14 21:28, Felipe Balbi wrote:
> > Caused by a copy & paste error. Note that even with
> > this bug AM437x SK display still works because GPIO
> > mux mode is always enabled. It's still wrong to mux
> > somebody else's pin.
> > 
> > Luckily ball D25 (offset 0x238 - gpio5_8) on AM437x
> > isn't used for anything.
> > 
> > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > ---
> >  arch/arm/boot/dts/am437x-sk-evm.dts | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
> > index 859ff3d..681be00 100644
> > --- a/arch/arm/boot/dts/am437x-sk-evm.dts
> > +++ b/arch/arm/boot/dts/am437x-sk-evm.dts
> > @@ -320,8 +320,7 @@
> >  
> >  	lcd_pins: lcd_pins {
> >  		pinctrl-single,pins = <
> > -			/* GPIO 5_8 to select LCD / HDMI */
> > -			0x238 (PIN_OUTPUT_PULLUP | MUX_MODE7)
> > +			0x1c (PIN_OUTPUT_PULLUP | MUX_MODE7) /* gpcm_ad7.gpio1_7 */
> >  		>;
> >  	};
> >  };
> 
> I didn't verify the offset, but based on the comments this looks fine to me.
> 
> Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Tony, are you taking this patch ? Looks like it has been forgotten and
now it needs to be backported to v3.17 and v3.18 (assuming it's too late
for v3.18).

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141204/b4a5dbe3/attachment.sig>

[PATCH 3/3] arm: boot: dts: am437x-sk: fix lcd enable pin mux data

[Felipe Balbi]

On Thu, Dec 04, 2014 at 08:43:25AM -0600, Felipe Balbi wrote:
> HI,
> 
> On Wed, Oct 15, 2014 at 03:24:20PM +0300, Tomi Valkeinen wrote:
> > On 14/10/14 21:28, Felipe Balbi wrote:
> > > Caused by a copy & paste error. Note that even with
> > > this bug AM437x SK display still works because GPIO
> > > mux mode is always enabled. It's still wrong to mux
> > > somebody else's pin.
> > > 
> > > Luckily ball D25 (offset 0x238 - gpio5_8) on AM437x
> > > isn't used for anything.
> > > 
> > > Signed-off-by: Felipe Balbi <balbi@ti.com>
> > > ---
> > >  arch/arm/boot/dts/am437x-sk-evm.dts | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
> > > index 859ff3d..681be00 100644
> > > --- a/arch/arm/boot/dts/am437x-sk-evm.dts
> > > +++ b/arch/arm/boot/dts/am437x-sk-evm.dts
> > > @@ -320,8 +320,7 @@
> > >  
> > >  	lcd_pins: lcd_pins {
> > >  		pinctrl-single,pins = <
> > > -			/* GPIO 5_8 to select LCD / HDMI */
> > > -			0x238 (PIN_OUTPUT_PULLUP | MUX_MODE7)
> > > +			0x1c (PIN_OUTPUT_PULLUP | MUX_MODE7) /* gpcm_ad7.gpio1_7 */
> > >  		>;
> > >  	};
> > >  };
> > 
> > I didn't verify the offset, but based on the comments this looks fine to me.
> > 
> > Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> 
> Tony, are you taking this patch ? Looks like it has been forgotten and
> now it needs to be backported to v3.17 and v3.18 (assuming it's too late
> for v3.18).

I'll send a new version and Cc stable.

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141204/65c5030a/attachment-0001.sig>

[PATCH] arm: boot: dts: am437x-sk: fix lcd enable pin mux data

[Felipe Balbi]

Caused by a copy & paste error. Note that even with
this bug AM437x SK display still works because GPIO
mux mode is always enabled. It's still wrong to mux
somebody else's pin.

Luckily ball D25 (offset 0x238 - gpio5_8) on AM437x
isn't used for anything.

While at that, also replace a pullup with a pulldown
as that gpio should be normally low, not high.

Cc: <stable@vger.kernel.org> # v3.17+
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Signed-off-by: Felipe Balbi <balbi@ti.com>
---
 arch/arm/boot/dts/am437x-sk-evm.dts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
index f7f77a7..0eceb8a 100644
--- a/arch/arm/boot/dts/am437x-sk-evm.dts
+++ b/arch/arm/boot/dts/am437x-sk-evm.dts
@@ -320,8 +320,7 @@
 
 	lcd_pins: lcd_pins {
 		pinctrl-single,pins = <
-			/* GPIO 5_8 to select LCD / HDMI */
-			0x238 (PIN_OUTPUT_PULLUP | MUX_MODE7)
+			0x1c (PIN_OUTPUT_PULLDOWN | MUX_MODE7) /* gpcm_ad7.gpio1_7 */
 		>;
 	};
 };
-- 
2.1.0.GIT

[PATCH] arm: boot: dts: am437x-sk: fix lcd enable pin mux data

[Tony Lindgren]

* Felipe Balbi <balbi@ti.com> [141204 07:12]:
> Caused by a copy & paste error. Note that even with
> this bug AM437x SK display still works because GPIO
> mux mode is always enabled. It's still wrong to mux
> somebody else's pin.
> 
> Luckily ball D25 (offset 0x238 - gpio5_8) on AM437x
> isn't used for anything.
> 
> While at that, also replace a pullup with a pulldown
> as that gpio should be normally low, not high.
> 
> Cc: <stable@vger.kernel.org> # v3.17+
> Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Signed-off-by: Felipe Balbi <balbi@ti.com>

Applying into omap-for-v3.19/fixes thanks.

Tony

> ---
>  arch/arm/boot/dts/am437x-sk-evm.dts | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/am437x-sk-evm.dts b/arch/arm/boot/dts/am437x-sk-evm.dts
> index f7f77a7..0eceb8a 100644
> --- a/arch/arm/boot/dts/am437x-sk-evm.dts
> +++ b/arch/arm/boot/dts/am437x-sk-evm.dts
> @@ -320,8 +320,7 @@
>  
>  	lcd_pins: lcd_pins {
>  		pinctrl-single,pins = <
> -			/* GPIO 5_8 to select LCD / HDMI */
> -			0x238 (PIN_OUTPUT_PULLUP | MUX_MODE7)
> +			0x1c (PIN_OUTPUT_PULLDOWN | MUX_MODE7) /* gpcm_ad7.gpio1_7 */
>  		>;
>  	};
>  };
> -- 
> 2.1.0.GIT
>