Nokia N900: Broken lirc ir-rx51 driver

PATCH Pali Rohár 4 participants 8 messages Jan 2, 2016 – May 2, 2016

Subsystems (2)

Media (V4L/DVB) RC-core / lirc framework

lore

Share & tools

lore

Pali Rohár

author

2016-01-02 14:45 UTC #0 lore raw

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:

drivers/media/rc/Kconfig +1 -1

index b6e1311..f70d4c7 100644
@@ -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.

drivers/media/rc/ir-rx51.c +3 -3

index b1e19a2..be29bd0 100644
@@ -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;

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.

Signature · 2 lines

Pali Rohár
[email protected]

Tony Lindgren

2016-01-02 17:06 UTC #1 lore raw

Hi,

drivers/media/rc/ir-rx51.c line 30

Quoted 11 lines · Pali Rohár

drivers/media/rc/ir-rx51.c +2 -2

@@ -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.

Quoted 12 lines

drivers/media/rc/ir-rx51.c +1 -1

 #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;

Quoted 4 lines

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.

Quoted 3 lines

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

Pali Rohár

author

2016-01-05 10:18 UTC #2 lore raw

Quoted 11 lines · On Saturday 02 January 2016 09:06:57 Tony Lindgren wrote:

Hi,

Quoted 8 lines · Pali Rohár

drivers/media/rc/ir-rx51.c +1 -0

@@ -25,9 +25,9 @@
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/wait.h>
+#include <linux/clk.h>

Quoted 7 lines

drivers/media/rc/ir-rx51.c line 30
Quoted 3 lines
drivers/media/rc/ir-rx51.c +1 -2
-#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?

Quoted 12 lines

drivers/media/rc/ir-rx51.c +1 -1

 #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;

Quoted 9 lines

Quoted 4 lines
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?

Quoted 11 lines

Quoted 3 lines
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

Signature · 2 lines

Pali Rohár
[email protected]

Tony Lindgren

2016-01-06 01:12 UTC #3 lore raw

Quoted 9 lines · Pali Rohár

Quoted 5 lines · 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

Pali Rohár

author

2016-04-22 13:49 UTC #4 lore raw

Quoted 11 lines · On Tuesday 05 January 2016 17:12:50 Tony Lindgren wrote:

Quoted 9 lines · Pali Rohár
Quoted 5 lines · 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?

Quoted 7 lines

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?

Signature · 2 lines

Pali Rohár
[email protected]

Tony Lindgren

2016-04-22 22:21 UTC #5 lore raw

Quoted 12 lines · Pali Rohár

Quoted 11 lines · On Tuesday 05 January 2016 17:12:50 Tony Lindgren wrote:
Quoted 9 lines · Pali Rohár
Quoted 5 lines · 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?

Quoted 6 lines

Quoted 3 lines
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

Pavel Machek

2016-05-02 07:06 UTC #6 lore raw

In reply to Pali Rohár

Quoted 16 lines · Pali Rohár

Quoted 15 lines · On Tuesday 05 January 2016 17:12:50 Tony Lindgren wrote:
Quoted 9 lines · Pali Rohár
Quoted 5 lines · 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.

Quoted 6 lines

Quoted 3 lines
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 <[email protected]>
To: [email protected]
Cc: [email protected], Aaro Koskinen <[email protected]>, Ivaylo Dimitrov
<[email protected]>, Sebastian Reichel <[email protected]>, Pavel Machel
<[email protected]>, Timo Kokkonen <[email protected]>, [email protected], Mauro
Carvalho Chehab <[email protected]>, Neil Armstrong <[email protected]>
Subject: [PATCH 0/2] Fix ir-rx51 by using PWM pdata
X-Mailer: git-send-email 2.8.1

Best regards,
Pavel

Signature · 2 lines

(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Ivaylo Dimitrov

2016-05-02 08:39 UTC #7 lore raw

Hi,

Quoted 17 lines · Pavel Machek

Quoted 16 lines · Pali Rohár
Quoted 15 lines · On Tuesday 05 January 2016 17:12:50 Tony Lindgren wrote:
Quoted 9 lines · Pali Rohár
Quoted 5 lines · 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.

Quoted 9 lines

Quoted 6 lines
Quoted 3 lines
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