diff mbox

Nokia N900: Broken lirc ir-rx51 driver

Message ID 201601021545.35201@pali (mailing list archive)
State New, archived
Headers show

Commit Message

Pali Rohár Jan. 2, 2016, 2:45 p.m. UTC 
Hello,

due to this commit (ARM: OMAP2+: Disable code that currently does not
work with multiplaform)

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/media/rc/Kconfig?id=a62a6e98c370ccca37d353a5f763b532411a4c14

lirc driver for Nokia N900 (ir-rx51) cannot be enabled via make
menuconfig. It is because Nokia N900 support cannot be compiled without
ARCH_MULTIPLATFORM, but Nokia N900 lirc driver (IR_RX51) cannot be
compiled when ARCH_MULTIPLATFORM is enabled.

Because ir-rx51 driver is just for Nokia N900 it is nonsense to have
such condition because nobody can use ir-rx51 driver... It is even not
possible to enable compilation for it...

Here is simple patch which enable compilation for Nokia N900 and fix
compile errors:


So Tony, you are author of that commit (a62a6e98c3) which broke ir-rx51
module for Nokia N900. Do you know how to fix this driver for upstream
kernel? It would be great to have driver working and not to have it in
this dead state...

Also platform data for this driver are only in legacy board code.
Support in DTS is missing, so driver (after fixing above problem) cannot
be used on DT booted kernel.

Comments

Tony Lindgren Jan. 2, 2016, 5:06 p.m. UTC | #1 
Hi,

* Pali Rohár <pali.rohar@gmail.com> [160102 06:46]:
> --- a/drivers/media/rc/ir-rx51.c
> +++ b/drivers/media/rc/ir-rx51.c
> @@ -25,9 +25,9 @@
>  #include <linux/platform_device.h>
>  #include <linux/sched.h>
>  #include <linux/wait.h>
> +#include <linux/clk.h>
>  
> -#include <plat/dmtimer.h>
> -#include <plat/clock.h>
> +#include "../../../arch/arm/plat-omap/include/plat/dmtimer.h"

Well we don't want to export the dmtimer functions to drivers..But
we now have the PWM driver that can be already used for most of the
ir-rx51.c.

>  #include <media/lirc.h>
>  #include <media/lirc_dev.h>
> @@ -208,7 +208,7 @@ static int lirc_rx51_init_port(struct lirc_rx51 *lirc_rx51)
>  	}
>  
>  	clk_fclk = omap_dm_timer_get_fclk(lirc_rx51->pwm_timer);
> -	lirc_rx51->fclk_khz = clk_fclk->rate / 1000;
> +	lirc_rx51->fclk_khz = clk_get_rate(clk_fclk) / 1000;
>  
>  	return 0;
>  
> 
> So Tony, you are author of that commit (a62a6e98c3) which broke ir-rx51
> module for Nokia N900. Do you know how to fix this driver for upstream
> kernel? It would be great to have driver working and not to have it in
> this dead state...

Yup please take a look at thread "[PATCH 0/3] pwm: omap: Add PWM support
using dual-mode timers". Chances are we still need to set up the dmtimer
code to provide also irqchip functions. That way ir-rx51.c can just do
request_irq on the selected dmtimer for interrupts.

> Also platform data for this driver are only in legacy board code.
> Support in DTS is missing, so driver (after fixing above problem) cannot
> be used on DT booted kernel.

Yeah those parts should be already doable with the PWM timer code AFAIK.

Regards,

Tony


--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár Jan. 5, 2016, 10:18 a.m. UTC | #2 
On Saturday 02 January 2016 09:06:57 Tony Lindgren wrote:
> Hi,
> 
> * Pali Rohár <pali.rohar@gmail.com> [160102 06:46]:
> > --- a/drivers/media/rc/ir-rx51.c
> > +++ b/drivers/media/rc/ir-rx51.c
> > @@ -25,9 +25,9 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/sched.h>
> >  #include <linux/wait.h>
> > +#include <linux/clk.h>
> >  
> > -#include <plat/dmtimer.h>
> > -#include <plat/clock.h>
> > +#include "../../../arch/arm/plat-omap/include/plat/dmtimer.h"
> 
> Well we don't want to export the dmtimer functions to drivers..But
> we now have the PWM driver that can be already used for most of the
> ir-rx51.c.

Ok. Is PWM driver included in mainline kernel?

> >  #include <media/lirc.h>
> >  #include <media/lirc_dev.h>
> > @@ -208,7 +208,7 @@ static int lirc_rx51_init_port(struct lirc_rx51 *lirc_rx51)
> >  	}
> >  
> >  	clk_fclk = omap_dm_timer_get_fclk(lirc_rx51->pwm_timer);
> > -	lirc_rx51->fclk_khz = clk_fclk->rate / 1000;
> > +	lirc_rx51->fclk_khz = clk_get_rate(clk_fclk) / 1000;
> >  
> >  	return 0;
> >  
> > 
> > So Tony, you are author of that commit (a62a6e98c3) which broke ir-rx51
> > module for Nokia N900. Do you know how to fix this driver for upstream
> > kernel? It would be great to have driver working and not to have it in
> > this dead state...
> 
> Yup please take a look at thread "[PATCH 0/3] pwm: omap: Add PWM support
> using dual-mode timers". Chances are we still need to set up the dmtimer
> code to provide also irqchip functions. That way ir-rx51.c can just do
> request_irq on the selected dmtimer for interrupts.

No I see that patch from that thread uses dmtimer.h from plat-omap. So
it is really OK?

> > Also platform data for this driver are only in legacy board code.
> > Support in DTS is missing, so driver (after fixing above problem) cannot
> > be used on DT booted kernel.
> 
> Yeah those parts should be already doable with the PWM timer code AFAIK.
> 
> Regards,
> 
> Tony
> 
>
Tony Lindgren Jan. 6, 2016, 1:12 a.m. UTC | #3 
* Pali Rohár <pali.rohar@gmail.com> [160105 02:19]:
> On Saturday 02 January 2016 09:06:57 Tony Lindgren wrote:
> >
> > Yup please take a look at thread "[PATCH 0/3] pwm: omap: Add PWM support
> > using dual-mode timers". Chances are we still need to set up the dmtimer
> > code to provide also irqchip functions. That way ir-rx51.c can just do
> > request_irq on the selected dmtimer for interrupts.
> 
> No I see that patch from that thread uses dmtimer.h from plat-omap. So
> it is really OK?

It's using the header to populate the platform data in mach-omap2 so
that's fine. But we do not want to directly expose the dmtimer functions
to device drivers as they are not Linux generic at this point.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pali Rohár April 22, 2016, 1:49 p.m. UTC | #4 
On Tuesday 05 January 2016 17:12:50 Tony Lindgren wrote:
> * Pali Rohár <pali.rohar@gmail.com> [160105 02:19]:
> > On Saturday 02 January 2016 09:06:57 Tony Lindgren wrote:
> > >
> > > Yup please take a look at thread "[PATCH 0/3] pwm: omap: Add PWM support
> > > using dual-mode timers". Chances are we still need to set up the dmtimer
> > > code to provide also irqchip functions. That way ir-rx51.c can just do
> > > request_irq on the selected dmtimer for interrupts.
> > 
> > No I see that patch from that thread uses dmtimer.h from plat-omap. So
> > it is really OK?
> 
> It's using the header to populate the platform data in mach-omap2 so
> that's fine. But we do not want to directly expose the dmtimer functions
> to device drivers as they are not Linux generic at this point.
> 
> Regards,
> 
> Tony

Hi Tony! Is there any progress for ir-rx51 driver? Months ago we were
waiting for some omap pwm patches... What is current state?
Tony Lindgren April 22, 2016, 10:21 p.m. UTC | #5 
* Pali Rohár <pali.rohar@gmail.com> [160422 06:50]:
> On Tuesday 05 January 2016 17:12:50 Tony Lindgren wrote:
> > * Pali Rohár <pali.rohar@gmail.com> [160105 02:19]:
> > > On Saturday 02 January 2016 09:06:57 Tony Lindgren wrote:
> > > >
> > > > Yup please take a look at thread "[PATCH 0/3] pwm: omap: Add PWM support
> > > > using dual-mode timers". Chances are we still need to set up the dmtimer
> > > > code to provide also irqchip functions. That way ir-rx51.c can just do
> > > > request_irq on the selected dmtimer for interrupts.
> > > 
> > > No I see that patch from that thread uses dmtimer.h from plat-omap. So
> > > it is really OK?
> > 
> > It's using the header to populate the platform data in mach-omap2 so
> > that's fine. But we do not want to directly expose the dmtimer functions
> > to device drivers as they are not Linux generic at this point.
>
> Hi Tony! Is there any progress for ir-rx51 driver? Months ago we were
> waiting for some omap pwm patches... What is current state?

Sorry it will be slow going for this one.. I think we should initially
just make it use include/linux/platform_data/pwm_omap_dmtimer.h and
pdata-quirks.c. It seems we need to add few new functions there.

Then once it's working again, we can continue to make it use hrtimer
for one of the gptimers. Then after that it should be easy to change
it to use the PWM framework with the pwm_omap_dmtimer.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Pavel Machek May 2, 2016, 7:06 a.m. UTC | #6 
On Fri 2016-04-22 15:49:29, Pali Rohár wrote:
> On Tuesday 05 January 2016 17:12:50 Tony Lindgren wrote:
> > * Pali Rohár <pali.rohar@gmail.com> [160105 02:19]:
> > > On Saturday 02 January 2016 09:06:57 Tony Lindgren wrote:
> > > >
> > > > Yup please take a look at thread "[PATCH 0/3] pwm: omap: Add PWM support
> > > > using dual-mode timers". Chances are we still need to set up the dmtimer
> > > > code to provide also irqchip functions. That way ir-rx51.c can just do
> > > > request_irq on the selected dmtimer for interrupts.
> > > 
> > > No I see that patch from that thread uses dmtimer.h from plat-omap. So
> > > it is really OK?
> > 
> > It's using the header to populate the platform data in mach-omap2 so
> > that's fine. But we do not want to directly expose the dmtimer functions
> > to device drivers as they are not Linux generic at this point.
> > 
> > Regards,
> > 
> > Tony
> 
> Hi Tony! Is there any progress for ir-rx51 driver? Months ago we were
> waiting for some omap pwm patches... What is current state?

Tony now posted patch series... it would be good to test them:

Date: Tue, 26 Apr 2016 16:51:47 -0700
From: Tony Lindgren <tony@atomide.com>
To: linux-omap@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org, Aaro Koskinen
<aaro.koskinen@iki.fi>, Ivaylo Dimitrov
        <ivo.g.dimitrov.75@gmail.com>, Sebastian Reichel
	<sre@kernel.org>, Pavel Machel
	        <pavel@ucw.cz>, Timo Kokkonen
		<timo.t.kokkonen@iki.fi>, linux-media@vger.kernel.org,
		Mauro
		        Carvalho Chehab <mchehab@osg.samsung.com>,
			Neil Armstrong <narmstrong@baylibre.com>
			Subject: [PATCH 0/2] Fix ir-rx51 by using PWM
			pdata
			X-Mailer: git-send-email 2.8.1


Best regards,
									Pavel
Ivaylo Dimitrov May 2, 2016, 8:39 a.m. UTC | #7 
Hi,

On  2.05.2016 10:06, Pavel Machek wrote:
> On Fri 2016-04-22 15:49:29, Pali Rohár wrote:
>> On Tuesday 05 January 2016 17:12:50 Tony Lindgren wrote:
>>> * Pali Rohár <pali.rohar@gmail.com> [160105 02:19]:
>>>> On Saturday 02 January 2016 09:06:57 Tony Lindgren wrote:
>>>>>
>>>>> Yup please take a look at thread "[PATCH 0/3] pwm: omap: Add PWM support
>>>>> using dual-mode timers". Chances are we still need to set up the dmtimer
>>>>> code to provide also irqchip functions. That way ir-rx51.c can just do
>>>>> request_irq on the selected dmtimer for interrupts.
>>>>
>>>> No I see that patch from that thread uses dmtimer.h from plat-omap. So
>>>> it is really OK?
>>>
>>> It's using the header to populate the platform data in mach-omap2 so
>>> that's fine. But we do not want to directly expose the dmtimer functions
>>> to device drivers as they are not Linux generic at this point.
>>>
>>> Regards,
>>>
>>> Tony
>>
>> Hi Tony! Is there any progress for ir-rx51 driver? Months ago we were
>> waiting for some omap pwm patches... What is current state?
>
> Tony now posted patch series... it would be good to test them:
>

Hmm, I guess you've missed 
http://www.spinics.net/lists/linux-omap/msg128465.html

Regards,
Ivo
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/media/rc/Kconfig b/drivers/media/rc/Kconfig
index b6e1311..f70d4c7 100644
--- a/drivers/media/rc/Kconfig
+++ b/drivers/media/rc/Kconfig
@@ -335,7 +335,7 @@  config IR_TTUSBIR
 
 config IR_RX51
 	tristate "Nokia N900 IR transmitter diode"
-	depends on OMAP_DM_TIMER && ARCH_OMAP2PLUS && LIRC && !ARCH_MULTIPLATFORM
+	depends on OMAP_DM_TIMER && ARCH_OMAP2PLUS && LIRC
 	---help---
 	   Say Y or M here if you want to enable support for the IR
 	   transmitter diode built in the Nokia N900 (RX51) device.
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index b1e19a2..be29bd0 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -25,9 +25,9 @@ 
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/wait.h>
+#include <linux/clk.h>
 
-#include <plat/dmtimer.h>
-#include <plat/clock.h>
+#include "../../../arch/arm/plat-omap/include/plat/dmtimer.h"
 
 #include <media/lirc.h>
 #include <media/lirc_dev.h>
@@ -208,7 +208,7 @@  static int lirc_rx51_init_port(struct lirc_rx51 *lirc_rx51)
 	}
 
 	clk_fclk = omap_dm_timer_get_fclk(lirc_rx51->pwm_timer);
-	lirc_rx51->fclk_khz = clk_fclk->rate / 1000;
+	lirc_rx51->fclk_khz = clk_get_rate(clk_fclk) / 1000;
 
 	return 0;