diff mbox

[4/7,media] ir-rx51: add DT support to driver

Message ID 1462634508-24961-5-git-send-email-ivo.g.dimitrov.75@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ivaylo Dimitrov May 7, 2016, 3:21 p.m. UTC
With the upcoming removal of legacy boot, lets add support to one of the
last N900 drivers remaining without it. As the driver still uses omap
dmtimer, add auxdata as well.

Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
---
 .../devicetree/bindings/media/nokia,lirc-rx51         | 19 +++++++++++++++++++
 arch/arm/mach-omap2/pdata-quirks.c                    |  6 +-----
 drivers/media/rc/ir-rx51.c                            | 11 ++++++++++-
 3 files changed, 30 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/media/nokia,lirc-rx51

Comments

Rob Herring May 9, 2016, 8:06 p.m. UTC | #1
On Sat, May 07, 2016 at 06:21:45PM +0300, Ivaylo Dimitrov wrote:
> With the upcoming removal of legacy boot, lets add support to one of the
> last N900 drivers remaining without it. As the driver still uses omap
> dmtimer, add auxdata as well.
> 
> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
> ---
>  .../devicetree/bindings/media/nokia,lirc-rx51         | 19 +++++++++++++++++++
>  arch/arm/mach-omap2/pdata-quirks.c                    |  6 +-----
>  drivers/media/rc/ir-rx51.c                            | 11 ++++++++++-
>  3 files changed, 30 insertions(+), 6 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/media/nokia,lirc-rx51
> 
> diff --git a/Documentation/devicetree/bindings/media/nokia,lirc-rx51 b/Documentation/devicetree/bindings/media/nokia,lirc-rx51
> new file mode 100644
> index 0000000..5b3081e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/media/nokia,lirc-rx51
> @@ -0,0 +1,19 @@
> +Device-Tree bindings for LIRC TX driver for Nokia N900(RX51)
> +
> +Required properties:
> +	- compatible: should be "nokia,lirc-rx51".

lirc is a Linux term. Also, nokia,rx51-... would be conventional 
ordering.

Is this anything more than a PWM LED?

> +	- pwms: specifies PWM used for IR signal transmission.
> +
> +Example node:
> +
> +	pwm9: dmtimer-pwm@9 {
> +		compatible = "ti,omap-dmtimer-pwm";
> +		ti,timers = <&timer9>;
> +		#pwm-cells = <3>;
> +	};
> +
> +	ir: lirc-rx51 {
> +		compatible = "nokia,lirc-rx51";
> +
> +		pwms = <&pwm9 0 26316 0>; /* 38000 Hz */
> +	};
Ivaylo Dimitrov May 9, 2016, 8:53 p.m. UTC | #2
Hi,

On  9.05.2016 23:06, Rob Herring wrote:
> On Sat, May 07, 2016 at 06:21:45PM +0300, Ivaylo Dimitrov wrote:
>> With the upcoming removal of legacy boot, lets add support to one of the
>> last N900 drivers remaining without it. As the driver still uses omap
>> dmtimer, add auxdata as well.
>>
>> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>> ---
>>   .../devicetree/bindings/media/nokia,lirc-rx51         | 19 +++++++++++++++++++
>>   arch/arm/mach-omap2/pdata-quirks.c                    |  6 +-----
>>   drivers/media/rc/ir-rx51.c                            | 11 ++++++++++-
>>   3 files changed, 30 insertions(+), 6 deletions(-)
>>   create mode 100644 Documentation/devicetree/bindings/media/nokia,lirc-rx51
>>
>> diff --git a/Documentation/devicetree/bindings/media/nokia,lirc-rx51 b/Documentation/devicetree/bindings/media/nokia,lirc-rx51
>> new file mode 100644
>> index 0000000..5b3081e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/media/nokia,lirc-rx51
>> @@ -0,0 +1,19 @@
>> +Device-Tree bindings for LIRC TX driver for Nokia N900(RX51)
>> +
>> +Required properties:
>> +	- compatible: should be "nokia,lirc-rx51".
>
> lirc is a Linux term. Also, nokia,rx51-... would be conventional
> ordering.
>

I used the driver name ("lirc_rx51") to not bring confusion. Also, it 
registers itself through lirc_register_driver() call, so having lirc in 
its name somehow makes sense.

I am not very good in inventing names, the best compatible I can think 
of is "nokia,rx51-ir". Is that ok?

> Is this anything more than a PWM LED?
>

It is an IR LED connected through a driver to McSPI2_SIMO pin of OMAP3, 
which pin can be configured as PWM or GPIO(there are other 
configurations, but they don't make sense). In theory it could be used 
for various things (like uni-directional serial TX, or stuff like that), 
but in practice it allows N900 to be act as an IR remote controller. I 
guess that fits in "nothing more than a PWM LED", more or less.

>> +	- pwms: specifies PWM used for IR signal transmission.
>> +
>> +Example node:
>> +
>> +	pwm9: dmtimer-pwm@9 {
>> +		compatible = "ti,omap-dmtimer-pwm";
>> +		ti,timers = <&timer9>;
>> +		#pwm-cells = <3>;
>> +	};
>> +
>> +	ir: lirc-rx51 {
>> +		compatible = "nokia,lirc-rx51";
>> +
>> +		pwms = <&pwm9 0 26316 0>; /* 38000 Hz */
>> +	};

Thanks,
Ivo
Rob Herring May 9, 2016, 9:07 p.m. UTC | #3
On Mon, May 9, 2016 at 3:53 PM, Ivaylo Dimitrov
<ivo.g.dimitrov.75@gmail.com> wrote:
> Hi,
>
> On  9.05.2016 23:06, Rob Herring wrote:
>>
>> On Sat, May 07, 2016 at 06:21:45PM +0300, Ivaylo Dimitrov wrote:
>>>
>>> With the upcoming removal of legacy boot, lets add support to one of the
>>> last N900 drivers remaining without it. As the driver still uses omap
>>> dmtimer, add auxdata as well.
>>>
>>> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>>> ---
>>>   .../devicetree/bindings/media/nokia,lirc-rx51         | 19
>>> +++++++++++++++++++
>>>   arch/arm/mach-omap2/pdata-quirks.c                    |  6 +-----
>>>   drivers/media/rc/ir-rx51.c                            | 11 ++++++++++-
>>>   3 files changed, 30 insertions(+), 6 deletions(-)
>>>   create mode 100644
>>> Documentation/devicetree/bindings/media/nokia,lirc-rx51
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/nokia,lirc-rx51
>>> b/Documentation/devicetree/bindings/media/nokia,lirc-rx51
>>> new file mode 100644
>>> index 0000000..5b3081e
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/media/nokia,lirc-rx51
>>> @@ -0,0 +1,19 @@
>>> +Device-Tree bindings for LIRC TX driver for Nokia N900(RX51)
>>> +
>>> +Required properties:
>>> +       - compatible: should be "nokia,lirc-rx51".
>>
>>
>> lirc is a Linux term. Also, nokia,rx51-... would be conventional
>> ordering.
>>
>
> I used the driver name ("lirc_rx51") to not bring confusion. Also, it
> registers itself through lirc_register_driver() call, so having lirc in its
> name somehow makes sense.
>
> I am not very good in inventing names, the best compatible I can think of is
> "nokia,rx51-ir". Is that ok?

Sure, but...

>> Is this anything more than a PWM LED?
>>
>
> It is an IR LED connected through a driver to McSPI2_SIMO pin of OMAP3,
> which pin can be configured as PWM or GPIO(there are other configurations,
> but they don't make sense). In theory it could be used for various things
> (like uni-directional serial TX, or stuff like that), but in practice it
> allows N900 to be act as an IR remote controller. I guess that fits in
> "nothing more than a PWM LED", more or less.

There's already a pwm-led binding that can be used. Though there may
be missing consumer IR to LED subsystem support in the kernel. You
could list both compatibles, use the rx51 IR driver now, and then move
to pwm-led driver in the future.

Rob
Ivaylo Dimitrov May 9, 2016, 10:06 p.m. UTC | #4
On 10.05.2016 00:07, Rob Herring wrote:
> On Mon, May 9, 2016 at 3:53 PM, Ivaylo Dimitrov
> <ivo.g.dimitrov.75@gmail.com> wrote:
>> Hi,
>>
>> On  9.05.2016 23:06, Rob Herring wrote:
>>>
>>> On Sat, May 07, 2016 at 06:21:45PM +0300, Ivaylo Dimitrov wrote:
>>>>
>>>> With the upcoming removal of legacy boot, lets add support to one of the
>>>> last N900 drivers remaining without it. As the driver still uses omap
>>>> dmtimer, add auxdata as well.
>>>>
>>>> Signed-off-by: Ivaylo Dimitrov <ivo.g.dimitrov.75@gmail.com>
>>>> ---
>>>>    .../devicetree/bindings/media/nokia,lirc-rx51         | 19
>>>> +++++++++++++++++++
>>>>    arch/arm/mach-omap2/pdata-quirks.c                    |  6 +-----
>>>>    drivers/media/rc/ir-rx51.c                            | 11 ++++++++++-
>>>>    3 files changed, 30 insertions(+), 6 deletions(-)
>>>>    create mode 100644
>>>> Documentation/devicetree/bindings/media/nokia,lirc-rx51
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/nokia,lirc-rx51
>>>> b/Documentation/devicetree/bindings/media/nokia,lirc-rx51
>>>> new file mode 100644
>>>> index 0000000..5b3081e
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/media/nokia,lirc-rx51
>>>> @@ -0,0 +1,19 @@
>>>> +Device-Tree bindings for LIRC TX driver for Nokia N900(RX51)
>>>> +
>>>> +Required properties:
>>>> +       - compatible: should be "nokia,lirc-rx51".
>>>
>>>
>>> lirc is a Linux term. Also, nokia,rx51-... would be conventional
>>> ordering.
>>>
>>
>> I used the driver name ("lirc_rx51") to not bring confusion. Also, it
>> registers itself through lirc_register_driver() call, so having lirc in its
>> name somehow makes sense.
>>
>> I am not very good in inventing names, the best compatible I can think of is
>> "nokia,rx51-ir". Is that ok?
>
> Sure, but...
>
>>> Is this anything more than a PWM LED?
>>>
>>
>> It is an IR LED connected through a driver to McSPI2_SIMO pin of OMAP3,
>> which pin can be configured as PWM or GPIO(there are other configurations,
>> but they don't make sense). In theory it could be used for various things
>> (like uni-directional serial TX, or stuff like that), but in practice it
>> allows N900 to be act as an IR remote controller. I guess that fits in
>> "nothing more than a PWM LED", more or less.
>
> There's already a pwm-led binding that can be used. Though there may
> be missing consumer IR to LED subsystem support in the kernel. You
> could list both compatibles, use the rx51 IR driver now, and then move
> to pwm-led driver in the future.
>

I looked at the pwm-leds (and related) code and couldn't see how it will 
do the job of controlling IR TX LED as a remote controller. So yeah, it 
seems CIR support in LEDS subsystem is non existent.

Could you elaborate on "list both compatibles", I don't really 
understand what I should do.

Ivo
Sebastian Reichel May 10, 2016, 2:18 a.m. UTC | #5
Hi,

On Mon, May 09, 2016 at 04:07:35PM -0500, Rob Herring wrote:
> There's already a pwm-led binding that can be used. Though there
> may be missing consumer IR to LED subsystem support in the kernel.
> You could list both compatibles, use the rx51 IR driver now, and
> then move to pwm-led driver in the future.

Well from a purely HW point of view it's a PWM connected led. The
usage is completely different though. Usually PWM is used to control
the LED's brightness via the duty cycle (basic concept: enabling led
only 50% of time reduces brightness to 50%).

In the IR led's case the aim is generating a specific serial pattern
instead. For this task it uses a dmtimer in PWM mode and a second
one to reconfigure the pwm timer.

I don't know about a good name, but rx51 should be replaced with
n900 in the compatible string. So maybe "nokia,n900-infrared-diode".

-- Sebastian
Rob Herring May 11, 2016, 2:14 p.m. UTC | #6
On Tue, May 10, 2016 at 04:18:27AM +0200, Sebastian Reichel wrote:
> Hi,
> 
> On Mon, May 09, 2016 at 04:07:35PM -0500, Rob Herring wrote:
> > There's already a pwm-led binding that can be used. Though there
> > may be missing consumer IR to LED subsystem support in the kernel.
> > You could list both compatibles, use the rx51 IR driver now, and
> > then move to pwm-led driver in the future.
> 
> Well from a purely HW point of view it's a PWM connected led. The
> usage is completely different though. Usually PWM is used to control
> the LED's brightness via the duty cycle (basic concept: enabling led
> only 50% of time reduces brightness to 50%).
> 
> In the IR led's case the aim is generating a specific serial pattern
> instead. For this task it uses a dmtimer in PWM mode and a second
> one to reconfigure the pwm timer.

In that case, it will probably never be a generic driver.

> I don't know about a good name, but rx51 should be replaced with
> n900 in the compatible string. So maybe "nokia,n900-infrared-diode".

That's fine, but the shorter '-ir' was too.

Rob
Ivaylo Dimitrov May 13, 2016, 6:15 a.m. UTC | #7
Hi,

On 11.05.2016 17:14, Rob Herring wrote:
> On Tue, May 10, 2016 at 04:18:27AM +0200, Sebastian Reichel wrote:
>> Hi,
>>
>> On Mon, May 09, 2016 at 04:07:35PM -0500, Rob Herring wrote:
>>> There's already a pwm-led binding that can be used. Though there
>>> may be missing consumer IR to LED subsystem support in the kernel.
>>> You could list both compatibles, use the rx51 IR driver now, and
>>> then move to pwm-led driver in the future.
>>
>> Well from a purely HW point of view it's a PWM connected led. The
>> usage is completely different though. Usually PWM is used to control
>> the LED's brightness via the duty cycle (basic concept: enabling led
>> only 50% of time reduces brightness to 50%).
>>
>> In the IR led's case the aim is generating a specific serial pattern
>> instead. For this task it uses a dmtimer in PWM mode and a second
>> one to reconfigure the pwm timer.
>
> In that case, it will probably never be a generic driver.
>
>> I don't know about a good name, but rx51 should be replaced with
>> n900 in the compatible string. So maybe "nokia,n900-infrared-diode".
>
> That's fine, but the shorter '-ir' was too.
>

I prefer the shorter "nokia,n900-ir", will resend the series with it 
used, unless someone has concerns about it.

Ivo
Sebastian Reichel May 13, 2016, 2:01 p.m. UTC | #8
Hi,

On Fri, May 13, 2016 at 09:15:38AM +0300, Ivaylo Dimitrov wrote:
> I prefer the shorter "nokia,n900-ir", will resend the series with
> it used, unless someone has concerns about it.

Sounds fine to me.

-- Sebastian
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/media/nokia,lirc-rx51 b/Documentation/devicetree/bindings/media/nokia,lirc-rx51
new file mode 100644
index 0000000..5b3081e
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/nokia,lirc-rx51
@@ -0,0 +1,19 @@ 
+Device-Tree bindings for LIRC TX driver for Nokia N900(RX51)
+
+Required properties:
+	- compatible: should be "nokia,lirc-rx51".
+	- pwms: specifies PWM used for IR signal transmission.
+
+Example node:
+
+	pwm9: dmtimer-pwm@9 {
+		compatible = "ti,omap-dmtimer-pwm";
+		ti,timers = <&timer9>;
+		#pwm-cells = <3>;
+	};
+
+	ir: lirc-rx51 {
+		compatible = "nokia,lirc-rx51";
+
+		pwms = <&pwm9 0 26316 0>; /* 38000 Hz */
+	};
diff --git a/arch/arm/mach-omap2/pdata-quirks.c b/arch/arm/mach-omap2/pdata-quirks.c
index af65781..c15ccac 100644
--- a/arch/arm/mach-omap2/pdata-quirks.c
+++ b/arch/arm/mach-omap2/pdata-quirks.c
@@ -273,8 +273,6 @@  static struct platform_device omap3_rom_rng_device = {
 	},
 };
 
-static struct platform_device rx51_lirc_device;
-
 static void __init nokia_n900_legacy_init(void)
 {
 	hsmmc2_internal_input_clk();
@@ -293,10 +291,7 @@  static void __init nokia_n900_legacy_init(void)
 
 		pr_info("RX-51: Registering OMAP3 HWRNG device\n");
 		platform_device_register(&omap3_rom_rng_device);
-
 	}
-
-	platform_device_register(&rx51_lirc_device);
 }
 
 static void __init omap3_tao3530_legacy_init(void)
@@ -534,6 +529,7 @@  static struct of_dev_auxdata omap_auxdata_lookup[] __initdata = {
 		       &omap3_iommu_pdata),
 	OF_DEV_AUXDATA("ti,omap3-hsmmc", 0x4809c000, "4809c000.mmc", &mmc_pdata[0]),
 	OF_DEV_AUXDATA("ti,omap3-hsmmc", 0x480b4000, "480b4000.mmc", &mmc_pdata[1]),
+	OF_DEV_AUXDATA("nokia,lirc-rx51", 0, "lirc-rx51", &rx51_lirc_data),
 	/* Only on am3517 */
 	OF_DEV_AUXDATA("ti,davinci_mdio", 0x5c030000, "davinci_mdio.0", NULL),
 	OF_DEV_AUXDATA("ti,am3517-emac", 0x5c000000, "davinci_emac.0",
diff --git a/drivers/media/rc/ir-rx51.c b/drivers/media/rc/ir-rx51.c
index 5096ef3..7a329d8 100644
--- a/drivers/media/rc/ir-rx51.c
+++ b/drivers/media/rc/ir-rx51.c
@@ -21,6 +21,7 @@ 
 #include <linux/sched.h>
 #include <linux/wait.h>
 #include <linux/pwm.h>
+#include <linux/of.h>
 
 #include <media/lirc.h>
 #include <media/lirc_dev.h>
@@ -478,6 +479,14 @@  static int lirc_rx51_remove(struct platform_device *dev)
 	return lirc_unregister_driver(lirc_rx51_driver.minor);
 }
 
+static const struct of_device_id lirc_rx51_match[] = {
+	{
+		.compatible = "nokia,lirc-rx51",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, lirc_rx51_match);
+
 struct platform_driver lirc_rx51_platform_driver = {
 	.probe		= lirc_rx51_probe,
 	.remove		= lirc_rx51_remove,
@@ -485,7 +494,7 @@  struct platform_driver lirc_rx51_platform_driver = {
 	.resume		= lirc_rx51_resume,
 	.driver		= {
 		.name	= DRIVER_NAME,
-		.owner	= THIS_MODULE,
+		.of_match_table = of_match_ptr(lirc_rx51_match),
 	},
 };
 module_platform_driver(lirc_rx51_platform_driver);