2011-02-02 04:31:28

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On Mon, Jan 24, 2011 at 01:58:13PM +0800, Thomas Chou wrote:
> From: Walter Goossens <[email protected]>
>
> Signed-off-by: Walter Goossens <[email protected]>
> Signed-off-by: Thomas Chou <[email protected]>
> ---
> v2 use const and add compat version.
>
> drivers/input/serio/altera_ps2.c | 11 +++++++++--
> 1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/serio/altera_ps2.c b/drivers/input/serio/altera_ps2.c
> index 7998560..63325ee 100644
> --- a/drivers/input/serio/altera_ps2.c
> +++ b/drivers/input/serio/altera_ps2.c
> @@ -19,6 +19,7 @@
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
>
> #define DRV_NAME "altera_ps2"
>
> @@ -173,6 +174,12 @@ static int __devexit altera_ps2_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct of_device_id altera_ps2_match[] = {
> + { .compatible = "altr,ps2-1.0", },

I thought I had seen 'altera' instead of an abbreviation being used in
a previous patch. I don't care much whether 'altr' or 'altera' is
used, but I'd like to know that there is consensus from the Altera
users so that all the drivers use the same prefix.

> + {},
> +}
> +MODULE_DEVICE_TABLE(of, altera_ps2_match);
> +
> /*
> * Our device driver structure
> */
> @@ -182,6 +189,7 @@ static struct platform_driver altera_ps2_driver = {
> .driver = {
> .name = DRV_NAME,
> .owner = THIS_MODULE,
> + .of_match_table = altera_ps2_match,

At the moment, this patch can only be applied against my
devicetree/next branch since there is a #ifdef CONFIG_OF around
of_match_table in mainline, and the patch to remove it is in my next
branch. I'm okay with taking it through my tree if Dmitry agrees.

g.

> },
> };
>
> @@ -189,13 +197,12 @@ static int __init altera_ps2_init(void)
> {
> return platform_driver_register(&altera_ps2_driver);
> }
> +module_init(altera_ps2_init);
>
> static void __exit altera_ps2_exit(void)
> {
> platform_driver_unregister(&altera_ps2_driver);
> }
> -
> -module_init(altera_ps2_init);
> module_exit(altera_ps2_exit);
>
> MODULE_DESCRIPTION("Altera University Program PS2 controller driver");
> --
> 1.7.3.4
>


2011-02-02 04:37:21

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On Tue, Feb 1, 2011 at 9:31 PM, Grant Likely <[email protected]> wrote:
> On Mon, Jan 24, 2011 at 01:58:13PM +0800, Thomas Chou wrote:
>> From: Walter Goossens <[email protected]>
>>
>> Signed-off-by: Walter Goossens <[email protected]>
>> Signed-off-by: Thomas Chou <[email protected]>
>> ---
>> v2 use const and add compat version.
>>
>> drivers/input/serio/altera_ps2.c | 11 +++++++++--
>> 1 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/serio/altera_ps2.c b/drivers/input/serio/altera_ps2.c
>> index 7998560..63325ee 100644
>> --- a/drivers/input/serio/altera_ps2.c
>> +++ b/drivers/input/serio/altera_ps2.c
>> @@ -19,6 +19,7 @@
>> #include <linux/platform_device.h>
>> #include <linux/io.h>
>> #include <linux/slab.h>
>> +#include <linux/of.h>
>>
>> #define DRV_NAME "altera_ps2"
>>
>> @@ -173,6 +174,12 @@ static int __devexit altera_ps2_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static const struct of_device_id altera_ps2_match[] = {
>> + { .compatible = "altr,ps2-1.0", },
>
> I thought I had seen 'altera' instead of an abbreviation being used in
> a previous patch. I don't care much whether 'altr' or 'altera' is
> used, but I'd like to know that there is consensus from the Altera
> users so that all the drivers use the same prefix.

Oh, and this patch should add documentation to
Documentation/devicetree/bindings/spi specifying exactly what device
this compatible string represents and what properties are expected to
be in the node (if any) beyond the standard reg, interrupts, etc. It
doesn't need to be huge and detailed, but it at least needs to show
that the string is in use. This comment goes for the other patches
that add bindings too.

g.

2011-02-02 04:45:12

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On Tue, Feb 01, 2011 at 09:31:21PM -0700, Grant Likely wrote:
> On Mon, Jan 24, 2011 at 01:58:13PM +0800, Thomas Chou wrote:
> > @@ -182,6 +189,7 @@ static struct platform_driver altera_ps2_driver = {
> > .driver = {
> > .name = DRV_NAME,
> > .owner = THIS_MODULE,
> > + .of_match_table = altera_ps2_match,
>
> At the moment, this patch can only be applied against my
> devicetree/next branch since there is a #ifdef CONFIG_OF around
> of_match_table in mainline, and the patch to remove it is in my next
> branch. I'm okay with taking it through my tree if Dmitry agrees.
>

Actually I was going to ask you if you could do just that.

Thanks.

--
Dmitry

2011-02-02 11:48:27

by Thomas Chou

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On 02/02/2011 12:31 PM, Grant Likely wrote:
>> +static const struct of_device_id altera_ps2_match[] = {
>> + { .compatible = "altr,ps2-1.0", },
>
> I thought I had seen 'altera' instead of an abbreviation being used in
> a previous patch. I don't care much whether 'altr' or 'altera' is
> used, but I'd like to know that there is consensus from the Altera
> users so that all the drivers use the same prefix.
>

We had discussed on nios2-dev mailing list, and decided to use 'altr' as
Walter suggested that it saves space.

About the documentation on dts binding, shall we have a single
altera.txt to describe all Altera related binding, and opencores.txt to
describe all OpenCores binding? Or separate file for each core in its
driver class?

- Thomas

2011-02-02 12:11:12

by Tobias Klauser

[permalink] [raw]
Subject: Re: [Nios2-dev] [PATCH v2] altera_ps2: Add devicetree support

On Wed, 2 Feb 2011 19:48:58 +0800 Thomas Chou <[email protected]> wrote:
> On 02/02/2011 12:31 PM, Grant Likely wrote:
> >> +static const struct of_device_id altera_ps2_match[] = {
> >> + { .compatible = "altr,ps2-1.0", },
> >
> > I thought I had seen 'altera' instead of an abbreviation being used in
> > a previous patch. I don't care much whether 'altr' or 'altera' is
> > used, but I'd like to know that there is consensus from the Altera
> > users so that all the drivers use the same prefix.
> >
>
> We had discussed on nios2-dev mailing list, and decided to use 'altr' as
> Walter suggested that it saves space.
>
> About the documentation on dts binding, shall we have a single
> altera.txt to describe all Altera related binding, and opencores.txt to
> describe all OpenCores binding? Or separate file for each core in its
> driver class?

As I will submit the device tree support for the altera_uart driver
(and the altera_jtaguart driver sometime in the future) I was asking
myself the same question. I'd vote for the altera.txt solution, as we
could use that for documenting the uart drivers, other Altera component
specific drivers and also the Nios2 arch specific parameters in the
future.

Cheers
Tobias

2011-02-02 15:38:10

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On Wed, Feb 02, 2011 at 07:48:58PM +0800, Thomas Chou wrote:
> On 02/02/2011 12:31 PM, Grant Likely wrote:
> >>+static const struct of_device_id altera_ps2_match[] = {
> >>+ { .compatible = "altr,ps2-1.0", },
> >
> >I thought I had seen 'altera' instead of an abbreviation being used in
> >a previous patch. I don't care much whether 'altr' or 'altera' is
> >used, but I'd like to know that there is consensus from the Altera
> >users so that all the drivers use the same prefix.
> >
>
> We had discussed on nios2-dev mailing list, and decided to use
> 'altr' as Walter suggested that it saves space.
>
> About the documentation on dts binding, shall we have a single
> altera.txt to describe all Altera related binding, and opencores.txt
> to describe all OpenCores binding? Or separate file for each core in
> its driver class?

Since I'm reorganizing the binding documentation to reflect subsystems
(bindings/spi, bindings/i2c, bindings/powerpc, etc) I'd prefer to see
a separate file for each type of core.

Eventually, I'd like to define a record format for documenting
bindings that can generate searchable and crosslinked output, but I've
not spent any time looking at that seriously yet.

g.

>
> - Thomas

2011-02-02 15:40:05

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On Wed, Feb 02, 2011 at 07:48:58PM +0800, Thomas Chou wrote:
> On 02/02/2011 12:31 PM, Grant Likely wrote:
> >>+static const struct of_device_id altera_ps2_match[] = {
> >>+ { .compatible = "altr,ps2-1.0", },
> >
> >I thought I had seen 'altera' instead of an abbreviation being used in
> >a previous patch. I don't care much whether 'altr' or 'altera' is
> >used, but I'd like to know that there is consensus from the Altera
> >users so that all the drivers use the same prefix.
> >
>
> We had discussed on nios2-dev mailing list, and decided to use
> 'altr' as Walter suggested that it saves space.

Is altr the stock ticker symbol? The convention is to either use the
stock ticker in all uppercase (although the uppercase bit hasn't been
consistently applied), or to use the full name in lowercase.

g.

2011-02-02 23:32:08

by Thomas Chou

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On 02/02/2011 11:38 PM, Grant Likely wrote:
> On Wed, Feb 02, 2011 at 07:48:58PM +0800, Thomas Chou wrote:
>> On 02/02/2011 12:31 PM, Grant Likely wrote:
>>>> +static const struct of_device_id altera_ps2_match[] = {
>>>> + { .compatible = "altr,ps2-1.0", },
>>>
>>> I thought I had seen 'altera' instead of an abbreviation being used in
>>> a previous patch. I don't care much whether 'altr' or 'altera' is
>>> used, but I'd like to know that there is consensus from the Altera
>>> users so that all the drivers use the same prefix.
>>>
>>
>> We had discussed on nios2-dev mailing list, and decided to use
>> 'altr' as Walter suggested that it saves space.
>>
>> About the documentation on dts binding, shall we have a single
>> altera.txt to describe all Altera related binding, and opencores.txt
>> to describe all OpenCores binding? Or separate file for each core in
>> its driver class?
>
> Since I'm reorganizing the binding documentation to reflect subsystems
> (bindings/spi, bindings/i2c, bindings/powerpc, etc) I'd prefer to see
> a separate file for each type of core.

OK. We will use separate files. I will send an updated patch to include
the dts binding.

As for nios2 cpu related binding, we shall use binding/nios2 dir. Right?

- Thomas

>
> Eventually, I'd like to define a record format for documenting
> bindings that can generate searchable and crosslinked output, but I've
> not spent any time looking at that seriously yet.
>
> g.
>
>>
>> - Thomas
>

2011-02-02 23:35:03

by Thomas Chou

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On 02/02/2011 11:39 PM, Grant Likely wrote:
> On Wed, Feb 02, 2011 at 07:48:58PM +0800, Thomas Chou wrote:
>> On 02/02/2011 12:31 PM, Grant Likely wrote:
>>>> +static const struct of_device_id altera_ps2_match[] = {
>>>> + { .compatible = "altr,ps2-1.0", },
>>>
>>> I thought I had seen 'altera' instead of an abbreviation being used in
>>> a previous patch. I don't care much whether 'altr' or 'altera' is
>>> used, but I'd like to know that there is consensus from the Altera
>>> users so that all the drivers use the same prefix.
>>>
>>
>> We had discussed on nios2-dev mailing list, and decided to use
>> 'altr' as Walter suggested that it saves space.
>
> Is altr the stock ticker symbol? The convention is to either use the
> stock ticker in all uppercase (although the uppercase bit hasn't been
> consistently applied), or to use the full name in lowercase.

Yes, it is the stock ticker symbol. Then we shall follow the convention,
and change it to uppercase.

- Thomas

>
> g.
>
>

2011-02-03 03:04:28

by Thomas Chou

[permalink] [raw]
Subject: [PATCH v3] altera_ps2: Add devicetree support

Signed-off-by: Walter Goossens <[email protected]>
Signed-off-by: Thomas Chou <[email protected]>
---
v2 use const and add compat version.
v3 change compatible vendor to ALTR.
add dts binding doc.

.../devicetree/bindings/serio/altera_ps2.txt | 4 ++++
drivers/input/serio/altera_ps2.c | 11 +++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serio/altera_ps2.txt

diff --git a/Documentation/devicetree/bindings/serio/altera_ps2.txt b/Documentation/devicetree/bindings/serio/altera_ps2.txt
new file mode 100644
index 0000000..4d9eecc
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/altera_ps2.txt
@@ -0,0 +1,4 @@
+Altera UP PS/2 controller
+
+Required properties:
+- compatible : should be "ALTR,ps2-1.0".
diff --git a/drivers/input/serio/altera_ps2.c b/drivers/input/serio/altera_ps2.c
index 7998560..d94b4a3 100644
--- a/drivers/input/serio/altera_ps2.c
+++ b/drivers/input/serio/altera_ps2.c
@@ -19,6 +19,7 @@
#include <linux/platform_device.h>
#include <linux/io.h>
#include <linux/slab.h>
+#include <linux/of.h>

#define DRV_NAME "altera_ps2"

@@ -173,6 +174,12 @@ static int __devexit altera_ps2_remove(struct platform_device *pdev)
return 0;
}

+static const struct of_device_id altera_ps2_match[] = {
+ { .compatible = "ALTR,ps2-1.0", },
+ {},
+}
+MODULE_DEVICE_TABLE(of, altera_ps2_match);
+
/*
* Our device driver structure
*/
@@ -182,6 +189,7 @@ static struct platform_driver altera_ps2_driver = {
.driver = {
.name = DRV_NAME,
.owner = THIS_MODULE,
+ .of_match_table = altera_ps2_match,
},
};

@@ -189,13 +197,12 @@ static int __init altera_ps2_init(void)
{
return platform_driver_register(&altera_ps2_driver);
}
+module_init(altera_ps2_init);

static void __exit altera_ps2_exit(void)
{
platform_driver_unregister(&altera_ps2_driver);
}
-
-module_init(altera_ps2_init);
module_exit(altera_ps2_exit);

MODULE_DESCRIPTION("Altera University Program PS2 controller driver");
--
1.7.3.5

2011-02-03 22:54:19

by Mitch Bradley

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On 2/3/2011 12:27 PM, Walter Goossens wrote:
> On 2/2/11 4:39 PM, Grant Likely wrote:
>> On Wed, Feb 02, 2011 at 07:48:58PM +0800, Thomas Chou wrote:
>>> On 02/02/2011 12:31 PM, Grant Likely wrote:
>>>>> +static const struct of_device_id altera_ps2_match[] = {
>>>>> + { .compatible = "altr,ps2-1.0", },
>>>> I thought I had seen 'altera' instead of an abbreviation being used in
>>>> a previous patch. I don't care much whether 'altr' or 'altera' is
>>>> used, but I'd like to know that there is consensus from the Altera
>>>> users so that all the drivers use the same prefix.
>>>>
>>> We had discussed on nios2-dev mailing list, and decided to use
>>> 'altr' as Walter suggested that it saves space.
>> Is altr the stock ticker symbol? The convention is to either use the
>> stock ticker in all uppercase (although the uppercase bit hasn't been
>> consistently applied), or to use the full name in lowercase.
>>
>> g.
>>
>>
> Risking my limbs here by breaking in this late in the discussion... (I
> wasn't able to reply earlier) but where does it state it needs to be
> uppercase? I found a bunch of microblaze code which seems to use the
> lowercase xlnx and freescale seems happy with fsl. Unless I'm missing
> something obvious here I guess ALTR would actually be the first to use
> uppercase.
> The only reference to uppercase I found in the ePAPR docs was chapter
> 1.6 that talks about uppercase hex-characters as an OUI.
> Not that I terribly mind either way, but I want to double-check before
> we go ahead and change all altera-related devicetree stuff to uppercase.


The relevant text in IEEE 1275-1994 is in the description of the "name"
property in Annex A. If a node name begins with a sequence of from one
to five uppercase letters followed by a comma, that means a stock symbol
on some exchange whose names do not conflict with NYSE or NASDAQ.

A lower-case prefix is okay, but it does not necessarily mean that it is
a ticker symbol. So in some sense, a lower case prefix provides less
protection against collisions than an upper case prefix, which comes
from an externally-arbitrated name space. Case is explicitly
significant in node names.

In practice, the important thing is that names must not conflict. Name
collisions haven't been much of a problem so far.

>
> Greetz
> Walter
>
>
> _______________________________________________
> devicetree-discuss mailing list
> [email protected]
> https://lists.ozlabs.org/listinfo/devicetree-discuss
>

2011-02-03 23:02:36

by Walter Goossens

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On 2/3/11 11:53 PM, Mitch Bradley wrote:
> On 2/3/2011 12:27 PM, Walter Goossens wrote:
>> On 2/2/11 4:39 PM, Grant Likely wrote:
>>> On Wed, Feb 02, 2011 at 07:48:58PM +0800, Thomas Chou wrote:
>>>> On 02/02/2011 12:31 PM, Grant Likely wrote:
>>>>>> +static const struct of_device_id altera_ps2_match[] = {
>>>>>> + { .compatible = "altr,ps2-1.0", },
>>>>> I thought I had seen 'altera' instead of an abbreviation being
>>>>> used in
>>>>> a previous patch. I don't care much whether 'altr' or 'altera' is
>>>>> used, but I'd like to know that there is consensus from the Altera
>>>>> users so that all the drivers use the same prefix.
>>>>>
>>>> We had discussed on nios2-dev mailing list, and decided to use
>>>> 'altr' as Walter suggested that it saves space.
>>> Is altr the stock ticker symbol? The convention is to either use the
>>> stock ticker in all uppercase (although the uppercase bit hasn't been
>>> consistently applied), or to use the full name in lowercase.
>>>
>>> g.
>>>
>>>
>> Risking my limbs here by breaking in this late in the discussion... (I
>> wasn't able to reply earlier) but where does it state it needs to be
>> uppercase? I found a bunch of microblaze code which seems to use the
>> lowercase xlnx and freescale seems happy with fsl. Unless I'm missing
>> something obvious here I guess ALTR would actually be the first to use
>> uppercase.
>> The only reference to uppercase I found in the ePAPR docs was chapter
>> 1.6 that talks about uppercase hex-characters as an OUI.
>> Not that I terribly mind either way, but I want to double-check before
>> we go ahead and change all altera-related devicetree stuff to uppercase.
>
>
> The relevant text in IEEE 1275-1994 is in the description of the
> "name" property in Annex A. If a node name begins with a sequence of
> from one to five uppercase letters followed by a comma, that means a
> stock symbol on some exchange whose names do not conflict with NYSE or
> NASDAQ.
>
> A lower-case prefix is okay, but it does not necessarily mean that it
> is a ticker symbol. So in some sense, a lower case prefix provides
> less protection against collisions than an upper case prefix, which
> comes from an externally-arbitrated name space. Case is explicitly
> significant in node names.
>
> In practice, the important thing is that names must not conflict.
> Name collisions haven't been much of a problem so far.
>
Excellent!
Must have missed that one. Good reason. We'll change 'm!

Thanks
Walter
>>
>> Greetz
>> Walter
>>
>>
>> _______________________________________________
>> devicetree-discuss mailing list
>> [email protected]
>> https://lists.ozlabs.org/listinfo/devicetree-discuss
>>
>

2011-02-03 23:04:47

by Walter Goossens

[permalink] [raw]
Subject: Re: [PATCH v2] altera_ps2: Add devicetree support

On 2/2/11 4:39 PM, Grant Likely wrote:
> On Wed, Feb 02, 2011 at 07:48:58PM +0800, Thomas Chou wrote:
>> On 02/02/2011 12:31 PM, Grant Likely wrote:
>>>> +static const struct of_device_id altera_ps2_match[] = {
>>>> + { .compatible = "altr,ps2-1.0", },
>>> I thought I had seen 'altera' instead of an abbreviation being used in
>>> a previous patch. I don't care much whether 'altr' or 'altera' is
>>> used, but I'd like to know that there is consensus from the Altera
>>> users so that all the drivers use the same prefix.
>>>
>> We had discussed on nios2-dev mailing list, and decided to use
>> 'altr' as Walter suggested that it saves space.
> Is altr the stock ticker symbol? The convention is to either use the
> stock ticker in all uppercase (although the uppercase bit hasn't been
> consistently applied), or to use the full name in lowercase.
>
> g.
>
>
Risking my limbs here by breaking in this late in the discussion... (I
wasn't able to reply earlier) but where does it state it needs to be
uppercase? I found a bunch of microblaze code which seems to use the
lowercase xlnx and freescale seems happy with fsl. Unless I'm missing
something obvious here I guess ALTR would actually be the first to use
uppercase.
The only reference to uppercase I found in the ePAPR docs was chapter
1.6 that talks about uppercase hex-characters as an OUI.
Not that I terribly mind either way, but I want to double-check before
we go ahead and change all altera-related devicetree stuff to uppercase.

Greetz
Walter

2011-02-12 09:26:11

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v3] altera_ps2: Add devicetree support

On Thu, Feb 03, 2011 at 11:05:04AM +0800, Thomas Chou wrote:
> Signed-off-by: Walter Goossens <[email protected]>
> Signed-off-by: Thomas Chou <[email protected]>
> ---
> v2 use const and add compat version.
> v3 change compatible vendor to ALTR.
> add dts binding doc.
>
> .../devicetree/bindings/serio/altera_ps2.txt | 4 ++++
> drivers/input/serio/altera_ps2.c | 11 +++++++++--
> 2 files changed, 13 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/serio/altera_ps2.txt
>
> diff --git a/Documentation/devicetree/bindings/serio/altera_ps2.txt b/Documentation/devicetree/bindings/serio/altera_ps2.txt
> new file mode 100644
> index 0000000..4d9eecc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serio/altera_ps2.txt
> @@ -0,0 +1,4 @@
> +Altera UP PS/2 controller
> +
> +Required properties:
> +- compatible : should be "ALTR,ps2-1.0".
> diff --git a/drivers/input/serio/altera_ps2.c b/drivers/input/serio/altera_ps2.c
> index 7998560..d94b4a3 100644
> --- a/drivers/input/serio/altera_ps2.c
> +++ b/drivers/input/serio/altera_ps2.c
> @@ -19,6 +19,7 @@
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
>
> #define DRV_NAME "altera_ps2"
>
> @@ -173,6 +174,12 @@ static int __devexit altera_ps2_remove(struct platform_device *pdev)
> return 0;
> }
>
> +static const struct of_device_id altera_ps2_match[] = {
> + { .compatible = "ALTR,ps2-1.0", },
> + {},
> +}
> +MODULE_DEVICE_TABLE(of, altera_ps2_match);
> +

Should be:

#ifdef CONFIG_OF
static const struct of_device_id altera_ps2_match[] = {
{ .compatible = "ALTR,ps2-1.0", },
{},
}
MODULE_DEVICE_TABLE(of, altera_ps2_match);
#else /* CONFIG_OF */
#define altera_ps2_match NULL
#endif /* CONFIG_OF */

Otherwise the driver will be advertising that it provides device tree
support when CONFIG_OF is disabled.

Otherwise, looks good to me. Feel free to add my acked-by line.

g.

> /*
> * Our device driver structure
> */
> @@ -182,6 +189,7 @@ static struct platform_driver altera_ps2_driver = {
> .driver = {
> .name = DRV_NAME,
> .owner = THIS_MODULE,
> + .of_match_table = altera_ps2_match,
> },
> };
>
> @@ -189,13 +197,12 @@ static int __init altera_ps2_init(void)
> {
> return platform_driver_register(&altera_ps2_driver);
> }
> +module_init(altera_ps2_init);
>
> static void __exit altera_ps2_exit(void)
> {
> platform_driver_unregister(&altera_ps2_driver);
> }
> -
> -module_init(altera_ps2_init);
> module_exit(altera_ps2_exit);
>
> MODULE_DESCRIPTION("Altera University Program PS2 controller driver");
> --
> 1.7.3.5
>

2011-02-12 13:22:33

by Thomas Chou

[permalink] [raw]
Subject: Re: [PATCH v3] altera_ps2: Add devicetree support

On 02/12/2011 05:26 PM, Grant Likely wrote:
> On Thu, Feb 03, 2011 at 11:05:04AM +0800, Thomas Chou wrote:
>> Signed-off-by: Walter Goossens<[email protected]>
>> Signed-off-by: Thomas Chou<[email protected]>
>> ---
>> v2 use const and add compat version.
>> v3 change compatible vendor to ALTR.
>> add dts binding doc.
>>
>> .../devicetree/bindings/serio/altera_ps2.txt | 4 ++++
>> drivers/input/serio/altera_ps2.c | 11 +++++++++--
>> 2 files changed, 13 insertions(+), 2 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/serio/altera_ps2.txt
>>
>> diff --git a/Documentation/devicetree/bindings/serio/altera_ps2.txt b/Documentation/devicetree/bindings/serio/altera_ps2.txt
>> new file mode 100644
>> index 0000000..4d9eecc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serio/altera_ps2.txt
>> @@ -0,0 +1,4 @@
>> +Altera UP PS/2 controller
>> +
>> +Required properties:
>> +- compatible : should be "ALTR,ps2-1.0".
>> diff --git a/drivers/input/serio/altera_ps2.c b/drivers/input/serio/altera_ps2.c
>> index 7998560..d94b4a3 100644
>> --- a/drivers/input/serio/altera_ps2.c
>> +++ b/drivers/input/serio/altera_ps2.c
>> @@ -19,6 +19,7 @@
>> #include<linux/platform_device.h>
>> #include<linux/io.h>
>> #include<linux/slab.h>
>> +#include<linux/of.h>
>>
>> #define DRV_NAME "altera_ps2"
>>
>> @@ -173,6 +174,12 @@ static int __devexit altera_ps2_remove(struct platform_device *pdev)
>> return 0;
>> }
>>
>> +static const struct of_device_id altera_ps2_match[] = {
>> + { .compatible = "ALTR,ps2-1.0", },
>> + {},
>> +}
>> +MODULE_DEVICE_TABLE(of, altera_ps2_match);
>> +
>
> Should be:
>
> #ifdef CONFIG_OF
> static const struct of_device_id altera_ps2_match[] = {
> { .compatible = "ALTR,ps2-1.0", },
> {},
> }
> MODULE_DEVICE_TABLE(of, altera_ps2_match);
> #else /* CONFIG_OF */
> #define altera_ps2_match NULL
> #endif /* CONFIG_OF */
>
> Otherwise the driver will be advertising that it provides device tree
> support when CONFIG_OF is disabled.

Thanks. Will update all related drivers.

- Thomas

>
> Otherwise, looks good to me. Feel free to add my acked-by line.
>
> g.
>
>> /*
>> * Our device driver structure
>> */
>> @@ -182,6 +189,7 @@ static struct platform_driver altera_ps2_driver = {
>> .driver = {
>> .name = DRV_NAME,
>> .owner = THIS_MODULE,
>> + .of_match_table = altera_ps2_match,
>> },
>> };
>>
>> @@ -189,13 +197,12 @@ static int __init altera_ps2_init(void)
>> {
>> return platform_driver_register(&altera_ps2_driver);
>> }
>> +module_init(altera_ps2_init);
>>
>> static void __exit altera_ps2_exit(void)
>> {
>> platform_driver_unregister(&altera_ps2_driver);
>> }
>> -
>> -module_init(altera_ps2_init);
>> module_exit(altera_ps2_exit);
>>
>> MODULE_DESCRIPTION("Altera University Program PS2 controller driver");
>> --
>> 1.7.3.5
>>
>

2011-02-14 02:04:00

by Thomas Chou

[permalink] [raw]
Subject: [PATCH v4] altera_ps2: Add devicetree support

From: Walter Goossens <[email protected]>

Signed-off-by: Walter Goossens <[email protected]>
Signed-off-by: Thomas Chou <[email protected]>
Acked-by: Grant Likely <[email protected]>
---
v2 use const and add compat version.
v3 change compatible vendor to ALTR.
add dts binding doc.
v4 condition module device table export for of.

Grant, please take it through your tree as Dmitry agreed.

.../devicetree/bindings/serio/altera_ps2.txt | 4 ++++
drivers/input/serio/altera_ps2.c | 15 +++++++++++++--
2 files changed, 17 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serio/altera_ps2.txt

diff --git a/Documentation/devicetree/bindings/serio/altera_ps2.txt b/Documentation/devicetree/bindings/serio/altera_ps2.txt
new file mode 100644
index 0000000..4d9eecc
--- /dev/null
+++ b/Documentation/devicetree/bindings/serio/altera_ps2.txt
@@ -0,0 +1,4 @@
+Altera UP PS/2 controller
+
+Required properties:
+- compatible : should be "ALTR,ps2-1.0".
diff --git a/drivers/input/serio/altera_ps2.c b/drivers/input/serio/altera_ps2.c
index 7998560..3fee88b 100644
--- a/drivers/input/serio/altera_ps2.c
+++ b/drivers/input/serio/altera_ps2.c
@@ -19,6 +19,7 @@
#include <linux/platform_device.h>
#include <linux/io.h>
#include <linux/slab.h>
+#include <linux/of.h>

#define DRV_NAME "altera_ps2"

@@ -173,6 +174,16 @@ static int __devexit altera_ps2_remove(struct platform_device *pdev)
return 0;
}

+#ifdef CONFIG_OF
+static const struct of_device_id altera_ps2_match[] = {
+ { .compatible = "ALTR,ps2-1.0", },
+ {},
+}
+MODULE_DEVICE_TABLE(of, altera_ps2_match);
+#else /* CONFIG_OF */
+#define altera_ps2_match NULL
+#endif /* CONFIG_OF */
+
/*
* Our device driver structure
*/
@@ -182,6 +193,7 @@ static struct platform_driver altera_ps2_driver = {
.driver = {
.name = DRV_NAME,
.owner = THIS_MODULE,
+ .of_match_table = altera_ps2_match,
},
};

@@ -189,13 +201,12 @@ static int __init altera_ps2_init(void)
{
return platform_driver_register(&altera_ps2_driver);
}
+module_init(altera_ps2_init);

static void __exit altera_ps2_exit(void)
{
platform_driver_unregister(&altera_ps2_driver);
}
-
-module_init(altera_ps2_init);
module_exit(altera_ps2_exit);

MODULE_DESCRIPTION("Altera University Program PS2 controller driver");
--
1.7.4

2011-02-14 02:21:11

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v4] altera_ps2: Add devicetree support

On Mon, Feb 14, 2011 at 10:06:42AM +0800, Thomas Chou wrote:
> From: Walter Goossens <[email protected]>
>
> Signed-off-by: Walter Goossens <[email protected]>
> Signed-off-by: Thomas Chou <[email protected]>
> Acked-by: Grant Likely <[email protected]>

Acked-by: Dmitry Torokhov <[email protected]>

> ---
> v2 use const and add compat version.
> v3 change compatible vendor to ALTR.
> add dts binding doc.
> v4 condition module device table export for of.
>
> Grant, please take it through your tree as Dmitry agreed.

*nod* Please.

>
> .../devicetree/bindings/serio/altera_ps2.txt | 4 ++++
> drivers/input/serio/altera_ps2.c | 15 +++++++++++++--
> 2 files changed, 17 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/serio/altera_ps2.txt
>
> diff --git a/Documentation/devicetree/bindings/serio/altera_ps2.txt b/Documentation/devicetree/bindings/serio/altera_ps2.txt
> new file mode 100644
> index 0000000..4d9eecc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serio/altera_ps2.txt
> @@ -0,0 +1,4 @@
> +Altera UP PS/2 controller
> +
> +Required properties:
> +- compatible : should be "ALTR,ps2-1.0".
> diff --git a/drivers/input/serio/altera_ps2.c b/drivers/input/serio/altera_ps2.c
> index 7998560..3fee88b 100644
> --- a/drivers/input/serio/altera_ps2.c
> +++ b/drivers/input/serio/altera_ps2.c
> @@ -19,6 +19,7 @@
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
>
> #define DRV_NAME "altera_ps2"
>
> @@ -173,6 +174,16 @@ static int __devexit altera_ps2_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id altera_ps2_match[] = {
> + { .compatible = "ALTR,ps2-1.0", },
> + {},
> +}
> +MODULE_DEVICE_TABLE(of, altera_ps2_match);
> +#else /* CONFIG_OF */
> +#define altera_ps2_match NULL
> +#endif /* CONFIG_OF */
> +
> /*
> * Our device driver structure
> */
> @@ -182,6 +193,7 @@ static struct platform_driver altera_ps2_driver = {
> .driver = {
> .name = DRV_NAME,
> .owner = THIS_MODULE,
> + .of_match_table = altera_ps2_match,
> },
> };
>
> @@ -189,13 +201,12 @@ static int __init altera_ps2_init(void)
> {
> return platform_driver_register(&altera_ps2_driver);
> }
> +module_init(altera_ps2_init);
>
> static void __exit altera_ps2_exit(void)
> {
> platform_driver_unregister(&altera_ps2_driver);
> }
> -
> -module_init(altera_ps2_init);
> module_exit(altera_ps2_exit);
>
> MODULE_DESCRIPTION("Altera University Program PS2 controller driver");
> --
> 1.7.4
>

--
Dmitry

2011-02-16 04:40:22

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v4] altera_ps2: Add devicetree support

On Mon, Feb 14, 2011 at 10:06:42AM +0800, Thomas Chou wrote:
> From: Walter Goossens <[email protected]>
>
> Signed-off-by: Walter Goossens <[email protected]>
> Signed-off-by: Thomas Chou <[email protected]>
> Acked-by: Grant Likely <[email protected]>

Merged, thanks,
g.

> ---
> v2 use const and add compat version.
> v3 change compatible vendor to ALTR.
> add dts binding doc.
> v4 condition module device table export for of.
>
> Grant, please take it through your tree as Dmitry agreed.
>
> .../devicetree/bindings/serio/altera_ps2.txt | 4 ++++
> drivers/input/serio/altera_ps2.c | 15 +++++++++++++--
> 2 files changed, 17 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/serio/altera_ps2.txt
>
> diff --git a/Documentation/devicetree/bindings/serio/altera_ps2.txt b/Documentation/devicetree/bindings/serio/altera_ps2.txt
> new file mode 100644
> index 0000000..4d9eecc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serio/altera_ps2.txt
> @@ -0,0 +1,4 @@
> +Altera UP PS/2 controller
> +
> +Required properties:
> +- compatible : should be "ALTR,ps2-1.0".
> diff --git a/drivers/input/serio/altera_ps2.c b/drivers/input/serio/altera_ps2.c
> index 7998560..3fee88b 100644
> --- a/drivers/input/serio/altera_ps2.c
> +++ b/drivers/input/serio/altera_ps2.c
> @@ -19,6 +19,7 @@
> #include <linux/platform_device.h>
> #include <linux/io.h>
> #include <linux/slab.h>
> +#include <linux/of.h>
>
> #define DRV_NAME "altera_ps2"
>
> @@ -173,6 +174,16 @@ static int __devexit altera_ps2_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id altera_ps2_match[] = {
> + { .compatible = "ALTR,ps2-1.0", },
> + {},
> +}
> +MODULE_DEVICE_TABLE(of, altera_ps2_match);
> +#else /* CONFIG_OF */
> +#define altera_ps2_match NULL
> +#endif /* CONFIG_OF */
> +
> /*
> * Our device driver structure
> */
> @@ -182,6 +193,7 @@ static struct platform_driver altera_ps2_driver = {
> .driver = {
> .name = DRV_NAME,
> .owner = THIS_MODULE,
> + .of_match_table = altera_ps2_match,
> },
> };
>
> @@ -189,13 +201,12 @@ static int __init altera_ps2_init(void)
> {
> return platform_driver_register(&altera_ps2_driver);
> }
> +module_init(altera_ps2_init);
>
> static void __exit altera_ps2_exit(void)
> {
> platform_driver_unregister(&altera_ps2_driver);
> }
> -
> -module_init(altera_ps2_init);
> module_exit(altera_ps2_exit);
>
> MODULE_DESCRIPTION("Altera University Program PS2 controller driver");
> --
> 1.7.4
>