* [PATCH 1/3] ARM: omap_device: handle first time activation of console device
2011-11-16 11:02 [PATCH 0/3] OMAP serial device tree support Rajendra Nayak
@ 2011-11-16 11:02 ` Rajendra Nayak
2011-11-16 14:50 ` Rob Herring
2011-11-16 15:01 ` Cousson, Benoit
2011-11-16 11:02 ` [PATCH 2/3] omap-serial: Add minimal device tree support Rajendra Nayak
2011-11-16 11:02 ` [PATCH 3/3] ARM: omap: pass minimal SoC/board data for UART from dt Rajendra Nayak
2 siblings, 2 replies; 17+ messages in thread
From: Rajendra Nayak @ 2011-11-16 11:02 UTC (permalink / raw)
To: linux-arm-kernel
console device on OMAP is never reset or idled by hwmod post
initial setup, early during boot, for obvious reasons not to
break early debug prints thrown on console.
This leaves the console device enabled at boot and the first activation
of it using hwmod needs to be handled in such a way that a disable is
called followed by an enable of the hwmod, the subsequent activations
can then use the default activation technique.
To handle this add a new binding to identify a hwmod which is used as
the console device.
This patch is based on the what is done in serial.c for non-DT builds.
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
.../devicetree/bindings/arm/omap/omap.txt | 1 +
arch/arm/plat-omap/omap_device.c | 33 +++++++++++++++++++-
2 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
index dbdab40..46ffd41 100644
--- a/Documentation/devicetree/bindings/arm/omap/omap.txt
+++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
@@ -21,6 +21,7 @@ Required properties:
Optional properties:
- ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
during suspend.
+- ti,console_hwmod: boolean, identifies the hwmod used as console device
Example:
diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
index e8d9869..2b2d068 100644
--- a/arch/arm/plat-omap/omap_device.c
+++ b/arch/arm/plat-omap/omap_device.c
@@ -88,6 +88,7 @@
#include <linux/pm_runtime.h>
#include <linux/of.h>
#include <linux/notifier.h>
+#include <linux/console.h>
#include <plat/omap_device.h>
#include <plat/omap_hwmod.h>
@@ -114,6 +115,32 @@ static struct omap_device_pm_latency omap_default_latency[] = {
}
};
+static int omap_console_hwmod_enable(struct omap_device *od)
+{
+ console_lock();
+ /*
+ * For early console we prevented hwmod reset and idle
+ * So before we enable the uart clocks idle the console
+ * uart clocks, then enable back the console uart hwmod.
+ */
+ omap_hwmod_idle(od->hwmods[0]);
+ omap_hwmod_enable(od->hwmods[0]);
+ console_unlock();
+ /*
+ * Restore the default activate/deactivate funcs,
+ * since now we have set the hwmod state machine right
+ * with the idle/enable for console uart
+ */
+ od->pm_lats = omap_default_latency;
+ return 0;
+}
+
+static struct omap_device_pm_latency omap_console_latency[] = {
+ {
+ .activate_func = omap_console_hwmod_enable,
+ },
+};
+
/* Private functions */
/**
@@ -342,6 +369,7 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
struct omap_hwmod **hwmods;
struct omap_device *od;
struct omap_hwmod *oh;
+ struct omap_device_pm_latency *pm_lat = NULL;
struct device_node *node = pdev->dev.of_node;
const char *oh_name;
int oh_cnt, i, ret = 0;
@@ -370,7 +398,10 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
hwmods[i] = oh;
}
- od = omap_device_alloc(pdev, hwmods, oh_cnt, NULL, 0);
+ if (of_find_property(node, "ti,console_hwmod", NULL))
+ pm_lat = omap_console_latency;
+
+ od = omap_device_alloc(pdev, hwmods, oh_cnt, pm_lat, 0);
if (!od) {
dev_err(&pdev->dev, "Cannot allocate omap_device for :%s\n",
oh_name);
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 1/3] ARM: omap_device: handle first time activation of console device
2011-11-16 11:02 ` [PATCH 1/3] ARM: omap_device: handle first time activation of console device Rajendra Nayak
@ 2011-11-16 14:50 ` Rob Herring
2011-11-16 15:14 ` Cousson, Benoit
2011-11-17 7:31 ` Rajendra Nayak
2011-11-16 15:01 ` Cousson, Benoit
1 sibling, 2 replies; 17+ messages in thread
From: Rob Herring @ 2011-11-16 14:50 UTC (permalink / raw)
To: linux-arm-kernel
On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
> console device on OMAP is never reset or idled by hwmod post
> initial setup, early during boot, for obvious reasons not to
> break early debug prints thrown on console.
> This leaves the console device enabled at boot and the first activation
> of it using hwmod needs to be handled in such a way that a disable is
> called followed by an enable of the hwmod, the subsequent activations
> can then use the default activation technique.
>
> To handle this add a new binding to identify a hwmod which is used as
> the console device.
>
> This patch is based on the what is done in serial.c for non-DT builds.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
> .../devicetree/bindings/arm/omap/omap.txt | 1 +
> arch/arm/plat-omap/omap_device.c | 33 +++++++++++++++++++-
> 2 files changed, 33 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
> index dbdab40..46ffd41 100644
> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> @@ -21,6 +21,7 @@ Required properties:
> Optional properties:
> - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
> during suspend.
> +- ti,console_hwmod: boolean, identifies the hwmod used as console device
>
This doesn't seem right. Which console is not a h/w property. Why can't
you use aliases like other platforms are doing?
Also, it's not clear in the documentation where this (and
ti,no_idle_on_suspend) should go in the DT. Both seem like they should
be kernel cmdline params.
Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] ARM: omap_device: handle first time activation of console device
2011-11-16 14:50 ` Rob Herring
@ 2011-11-16 15:14 ` Cousson, Benoit
2011-11-16 15:41 ` Rob Herring
2011-11-17 7:31 ` Rajendra Nayak
1 sibling, 1 reply; 17+ messages in thread
From: Cousson, Benoit @ 2011-11-16 15:14 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rob,
On 11/16/2011 3:50 PM, Rob Herring wrote:
> On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
>> console device on OMAP is never reset or idled by hwmod post
>> initial setup, early during boot, for obvious reasons not to
>> break early debug prints thrown on console.
>> This leaves the console device enabled at boot and the first activation
>> of it using hwmod needs to be handled in such a way that a disable is
>> called followed by an enable of the hwmod, the subsequent activations
>> can then use the default activation technique.
>>
>> To handle this add a new binding to identify a hwmod which is used as
>> the console device.
>>
>> This patch is based on the what is done in serial.c for non-DT builds.
>>
>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>> ---
>> .../devicetree/bindings/arm/omap/omap.txt | 1 +
>> arch/arm/plat-omap/omap_device.c | 33 +++++++++++++++++++-
>> 2 files changed, 33 insertions(+), 1 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
>> index dbdab40..46ffd41 100644
>> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
>> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
>> @@ -21,6 +21,7 @@ Required properties:
>> Optional properties:
>> - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
>> during suspend.
>> +- ti,console_hwmod: boolean, identifies the hwmod used as console device
>>
>
> This doesn't seem right. Which console is not a h/w property. Why can't
> you use aliases like other platforms are doing?
>
> Also, it's not clear in the documentation where this (and
> ti,no_idle_on_suspend) should go in the DT. Both seem like they should
> be kernel cmdline params.
That raise the question about using DT to pass linux specific parameter.
We already discuss that on the mailing list, but never get a clear answer.
It seems that DT is already used to provide some OS specific information
using the "linux," prefix for example.
There are clearly a couple of parameters that can be provided by the
bootloader, but that does not reflect a HW property.
So what is the guideline for that, should we stick to cmdline params for
that?
Quoting Grant's documentation, runtime configuration is supported:
"Runtime configuration
In most cases, a DT will be the sole method of communicating data from
firmware to the kernel, so also gets used to pass in runtime and
configuration data like the kernel parameters string and the location of
an initrd image.
Most of this data is contained in the /chosen node, and when booting
Linux it will look something like this:
chosen {
bootargs = "console=ttyS0,115200 loglevel=8";
initrd-start = <0xc8000000>;
initrd-end = <0xc8200000>;
};
The bootargs property contains the kernel arguments, and the initrd-*
properties define the address and size of an initrd blob. The chosen
node may also optionally contain an arbitrary number of additional
properties for platform-specific configuration data."
Does that mean that this is supported only if the bootloader does not
support cmdline?
Thanks,
Benoit
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] ARM: omap_device: handle first time activation of console device
2011-11-16 15:14 ` Cousson, Benoit
@ 2011-11-16 15:41 ` Rob Herring
2011-11-16 18:18 ` Cousson, Benoit
0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2011-11-16 15:41 UTC (permalink / raw)
To: linux-arm-kernel
Benoit,
On 11/16/2011 09:14 AM, Cousson, Benoit wrote:
> Hi Rob,
>
> On 11/16/2011 3:50 PM, Rob Herring wrote:
>> On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
>>> console device on OMAP is never reset or idled by hwmod post
>>> initial setup, early during boot, for obvious reasons not to
>>> break early debug prints thrown on console.
>>> This leaves the console device enabled at boot and the first activation
>>> of it using hwmod needs to be handled in such a way that a disable is
>>> called followed by an enable of the hwmod, the subsequent activations
>>> can then use the default activation technique.
>>>
>>> To handle this add a new binding to identify a hwmod which is used as
>>> the console device.
>>>
>>> This patch is based on the what is done in serial.c for non-DT builds.
>>>
>>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>>> ---
>>> .../devicetree/bindings/arm/omap/omap.txt | 1 +
>>> arch/arm/plat-omap/omap_device.c | 33
>>> +++++++++++++++++++-
>>> 2 files changed, 33 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt
>>> b/Documentation/devicetree/bindings/arm/omap/omap.txt
>>> index dbdab40..46ffd41 100644
>>> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
>>> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
>>> @@ -21,6 +21,7 @@ Required properties:
>>> Optional properties:
>>> - ti,no_idle_on_suspend: When present, it prevents the PM to idle
>>> the module
>>> during suspend.
>>> +- ti,console_hwmod: boolean, identifies the hwmod used as console
>>> device
>>>
>>
>> This doesn't seem right. Which console is not a h/w property. Why can't
>> you use aliases like other platforms are doing?
>>
>> Also, it's not clear in the documentation where this (and
>> ti,no_idle_on_suspend) should go in the DT. Both seem like they should
>> be kernel cmdline params.
>
> That raise the question about using DT to pass linux specific parameter.
> We already discuss that on the mailing list, but never get a clear answer.
> It seems that DT is already used to provide some OS specific information
> using the "linux," prefix for example.
True, but "linux," properties will always face resistance and scrutiny.
> There are clearly a couple of parameters that can be provided by the
> bootloader, but that does not reflect a HW property.
>
> So what is the guideline for that, should we stick to cmdline params for
> that?
>
I would say that if the setting does not depend on something in the DT,
then the cmdline is the right place. If you have a property that
references a phandle, then it can't be on the cmdline. For console
selection, there is already a defined way to select it with
console=blah. And there is an agreed way to define indexes in the DT
with aliases.
How were these properties set without DT?
> Quoting Grant's documentation, runtime configuration is supported:
>
> "Runtime configuration
>
> In most cases, a DT will be the sole method of communicating data from
> firmware to the kernel, so also gets used to pass in runtime and
> configuration data like the kernel parameters string and the location of
> an initrd image.
>
> Most of this data is contained in the /chosen node, and when booting
> Linux it will look something like this:
>
> chosen {
> bootargs = "console=ttyS0,115200 loglevel=8";
> initrd-start = <0xc8000000>;
> initrd-end = <0xc8200000>;
> };
>
> The bootargs property contains the kernel arguments, and the initrd-*
> properties define the address and size of an initrd blob. The chosen
> node may also optionally contain an arbitrary number of additional
> properties for platform-specific configuration data."
>
>
> Does that mean that this is supported only if the bootloader does not
> support cmdline?
No. I think it means chosen can be extended. However, how many other
chosen properties are defined? Not very many. Clearly, it's not a place
for adding whatever random property you want. chosen is really the boot
interface between the bootloader and kernel as it is ATAG type data.
Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] ARM: omap_device: handle first time activation of console device
2011-11-16 15:41 ` Rob Herring
@ 2011-11-16 18:18 ` Cousson, Benoit
0 siblings, 0 replies; 17+ messages in thread
From: Cousson, Benoit @ 2011-11-16 18:18 UTC (permalink / raw)
To: linux-arm-kernel
On 11/16/2011 4:41 PM, Rob Herring wrote:
> Benoit,
>
> On 11/16/2011 09:14 AM, Cousson, Benoit wrote:
>> Hi Rob,
>>
>> On 11/16/2011 3:50 PM, Rob Herring wrote:
>>> On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
>>>> console device on OMAP is never reset or idled by hwmod post
>>>> initial setup, early during boot, for obvious reasons not to
>>>> break early debug prints thrown on console.
>>>> This leaves the console device enabled at boot and the first activation
>>>> of it using hwmod needs to be handled in such a way that a disable is
>>>> called followed by an enable of the hwmod, the subsequent activations
>>>> can then use the default activation technique.
>>>>
>>>> To handle this add a new binding to identify a hwmod which is used as
>>>> the console device.
>>>>
>>>> This patch is based on the what is done in serial.c for non-DT builds.
>>>>
>>>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>>>> ---
>>>> .../devicetree/bindings/arm/omap/omap.txt | 1 +
>>>> arch/arm/plat-omap/omap_device.c | 33
>>>> +++++++++++++++++++-
>>>> 2 files changed, 33 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt
>>>> b/Documentation/devicetree/bindings/arm/omap/omap.txt
>>>> index dbdab40..46ffd41 100644
>>>> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
>>>> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
>>>> @@ -21,6 +21,7 @@ Required properties:
>>>> Optional properties:
>>>> - ti,no_idle_on_suspend: When present, it prevents the PM to idle
>>>> the module
>>>> during suspend.
>>>> +- ti,console_hwmod: boolean, identifies the hwmod used as console
>>>> device
>>>>
>>>
>>> This doesn't seem right. Which console is not a h/w property. Why can't
>>> you use aliases like other platforms are doing?
>>>
>>> Also, it's not clear in the documentation where this (and
>>> ti,no_idle_on_suspend) should go in the DT. Both seem like they should
>>> be kernel cmdline params.
>>
>> That raise the question about using DT to pass linux specific parameter.
>> We already discuss that on the mailing list, but never get a clear answer.
>> It seems that DT is already used to provide some OS specific information
>> using the "linux," prefix for example.
>
> True, but "linux," properties will always face resistance and scrutiny.
OK, that's good to know. So we should avoid abusing that kind of prefix.
>> There are clearly a couple of parameters that can be provided by the
>> bootloader, but that does not reflect a HW property.
>>
>> So what is the guideline for that, should we stick to cmdline params for
>> that?
>>
>
> I would say that if the setting does not depend on something in the DT,
> then the cmdline is the right place. If you have a property that
> references a phandle, then it can't be on the cmdline. For console
> selection, there is already a defined way to select it with
> console=blah. And there is an agreed way to define indexes in the DT
> with aliases.
OK, I saw that in some DT adapted serial driver, we can retrieve the
index from the string "serial<x>" and thus can get the console from the
ttyS number.
> How were these properties set without DT?
From the board file most of the time. Or using some dirty hacks here
and there :-)
>> Quoting Grant's documentation, runtime configuration is supported:
>>
>> "Runtime configuration
>>
>> In most cases, a DT will be the sole method of communicating data from
>> firmware to the kernel, so also gets used to pass in runtime and
>> configuration data like the kernel parameters string and the location of
>> an initrd image.
>>
>> Most of this data is contained in the /chosen node, and when booting
>> Linux it will look something like this:
>>
>> chosen {
>> bootargs = "console=ttyS0,115200 loglevel=8";
>> initrd-start =<0xc8000000>;
>> initrd-end =<0xc8200000>;
>> };
>>
>> The bootargs property contains the kernel arguments, and the initrd-*
>> properties define the address and size of an initrd blob. The chosen
>> node may also optionally contain an arbitrary number of additional
>> properties for platform-specific configuration data."
>>
>>
>> Does that mean that this is supported only if the bootloader does not
>> support cmdline?
>
> No. I think it means chosen can be extended. However, how many other
> chosen properties are defined? Not very many. Clearly, it's not a place
> for adding whatever random property you want. chosen is really the boot
> interface between the bootloader and kernel as it is ATAG type data.
OK, thanks for the clarification.
Regards,
Benoit
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] ARM: omap_device: handle first time activation of console device
2011-11-16 14:50 ` Rob Herring
2011-11-16 15:14 ` Cousson, Benoit
@ 2011-11-17 7:31 ` Rajendra Nayak
1 sibling, 0 replies; 17+ messages in thread
From: Rajendra Nayak @ 2011-11-17 7:31 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rob,
On Wednesday 16 November 2011 08:20 PM, Rob Herring wrote:
[]...
>>
>> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
>> index dbdab40..46ffd41 100644
>> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
>> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
>> @@ -21,6 +21,7 @@ Required properties:
>> Optional properties:
>> - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
>> during suspend.
>> +- ti,console_hwmod: boolean, identifies the hwmod used as console device
>>
>
> This doesn't seem right. Which console is not a h/w property. Why can't
> you use aliases like other platforms are doing?
After thinking of this some more, I figured its more of a hwmod problem
to be solved, than with being able to identify which console.
Will post a separate patch to fix it up in hmwod and drop this from the
DT series.
Thanks for the review.
regards,
Rajendra
>
> Also, it's not clear in the documentation where this (and
> ti,no_idle_on_suspend) should go in the DT. Both seem like they should
> be kernel cmdline params.
>
> Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] ARM: omap_device: handle first time activation of console device
2011-11-16 11:02 ` [PATCH 1/3] ARM: omap_device: handle first time activation of console device Rajendra Nayak
2011-11-16 14:50 ` Rob Herring
@ 2011-11-16 15:01 ` Cousson, Benoit
2011-11-17 7:19 ` Rajendra Nayak
1 sibling, 1 reply; 17+ messages in thread
From: Cousson, Benoit @ 2011-11-16 15:01 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rajendra,
On 11/16/2011 12:02 PM, Rajendra Nayak wrote:
> console device on OMAP is never reset or idled by hwmod post
> initial setup, early during boot, for obvious reasons not to
> break early debug prints thrown on console.
> This leaves the console device enabled at boot and the first activation
> of it using hwmod needs to be handled in such a way that a disable is
> called followed by an enable of the hwmod, the subsequent activations
> can then use the default activation technique.
>
> To handle this add a new binding to identify a hwmod which is used as
> the console device.
>
> This patch is based on the what is done in serial.c for non-DT builds.
>
> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
> ---
> .../devicetree/bindings/arm/omap/omap.txt | 1 +
> arch/arm/plat-omap/omap_device.c | 33 +++++++++++++++++++-
> 2 files changed, 33 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/arm/omap/omap.txt b/Documentation/devicetree/bindings/arm/omap/omap.txt
> index dbdab40..46ffd41 100644
> --- a/Documentation/devicetree/bindings/arm/omap/omap.txt
> +++ b/Documentation/devicetree/bindings/arm/omap/omap.txt
> @@ -21,6 +21,7 @@ Required properties:
> Optional properties:
> - ti,no_idle_on_suspend: When present, it prevents the PM to idle the module
> during suspend.
> +- ti,console_hwmod: boolean, identifies the hwmod used as console device
This is not directly hwmod related,
> Example:
> diff --git a/arch/arm/plat-omap/omap_device.c b/arch/arm/plat-omap/omap_device.c
> index e8d9869..2b2d068 100644
> --- a/arch/arm/plat-omap/omap_device.c
> +++ b/arch/arm/plat-omap/omap_device.c
> @@ -88,6 +88,7 @@
> #include<linux/pm_runtime.h>
> #include<linux/of.h>
> #include<linux/notifier.h>
> +#include<linux/console.h>
>
> #include<plat/omap_device.h>
> #include<plat/omap_hwmod.h>
> @@ -114,6 +115,32 @@ static struct omap_device_pm_latency omap_default_latency[] = {
> }
> };
>
> +static int omap_console_hwmod_enable(struct omap_device *od)
> +{
> + console_lock();
> + /*
> + * For early console we prevented hwmod reset and idle
A period is missing. Or maybe it should a comma with not capital letter.
> + * So before we enable the uart clocks idle the console
> + * uart clocks, then enable back the console uart hwmod.
> + */
> + omap_hwmod_idle(od->hwmods[0]);
> + omap_hwmod_enable(od->hwmods[0]);
Why do we have to idle -> enable? The module is already enabled, isn't it?
The comment is not super clear for me :-)
Is the goal is to reset the IP?
> + console_unlock();
> + /*
> + * Restore the default activate/deactivate funcs,
> + * since now we have set the hwmod state machine right
> + * with the idle/enable for console uart
> + */
> + od->pm_lats = omap_default_latency;
I do not understand that part to?
This is not the default, but instead a specific one only for the serial driver.
It looks to me that you are overwriting the default one with a specific one.
> + return 0;
> +}
> +
> +static struct omap_device_pm_latency omap_console_latency[] = {
> + {
> + .activate_func = omap_console_hwmod_enable,
> + },
> +};
> +
> /* Private functions */
>
> /**
> @@ -342,6 +369,7 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
> struct omap_hwmod **hwmods;
> struct omap_device *od;
> struct omap_hwmod *oh;
> + struct omap_device_pm_latency *pm_lat = NULL;
> struct device_node *node = pdev->dev.of_node;
> const char *oh_name;
> int oh_cnt, i, ret = 0;
> @@ -370,7 +398,10 @@ static int omap_device_build_from_dt(struct platform_device *pdev)
> hwmods[i] = oh;
> }
>
> - od = omap_device_alloc(pdev, hwmods, oh_cnt, NULL, 0);
> + if (of_find_property(node, "ti,console_hwmod", NULL))
> + pm_lat = omap_console_latency;
That's looks like a hack to me to add that specific serial hooks into the core code.
But I'm not sure to have any better idea, since the issue is the lack of function pointer in DT :-)
> + od = omap_device_alloc(pdev, hwmods, oh_cnt, pm_lat, 0);
It should not be 0 but the size of the array here: ARRAY_SIZE(omap_console_latency).
I'm wondering how it can work with zero since the code is doing that in omap_device_alloc:
od->pm_lats = kmemdup(pm_lats,
sizeof(struct omap_device_pm_latency) * pm_lats_cnt,
GFP_KERNEL);
Regards,
Benoit
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] ARM: omap_device: handle first time activation of console device
2011-11-16 15:01 ` Cousson, Benoit
@ 2011-11-17 7:19 ` Rajendra Nayak
2011-11-17 9:52 ` Cousson, Benoit
0 siblings, 1 reply; 17+ messages in thread
From: Rajendra Nayak @ 2011-11-17 7:19 UTC (permalink / raw)
To: linux-arm-kernel
[...]
>> +static int omap_console_hwmod_enable(struct omap_device *od)
>> +{
>> + console_lock();
>> + /*
>> + * For early console we prevented hwmod reset and idle
>
> A period is missing. Or maybe it should a comma with not capital letter.
>
>> + * So before we enable the uart clocks idle the console
>> + * uart clocks, then enable back the console uart hwmod.
>> + */
>> + omap_hwmod_idle(od->hwmods[0]);
>> + omap_hwmod_enable(od->hwmods[0]);
>
> Why do we have to idle -> enable? The module is already enabled, isn't it?
> The comment is not super clear for me :-)
> Is the goal is to reset the IP?
Yes, now that I read it, it does sound confusing. I should have at-least
read it once before I picked it from serial.c
But anyway, here's what the problem is.
I feel its partly to do with the inability of hwmod to handle some
modules which are left enabled post a setup, due to the
'HWMOD_INIT_NO_IDLE' flag set.
Such modules end up with a hwmod state as '_HWMOD_STATE_ENABLED' post
a setup. Now when a driver for such devices/modules tries to enable the
module the first time, hwmod throws up a big WARN stating the hwmod is
already in an enabled state.
[uart used as console is one such module, which cannot be idled post a
setup by hwmod]
If hwmod could be made in some way intelligent enough to know that the
module is in enabled state because of the 'HWMOD_INIT_NO_IDLE' itself,
a lot of this hackery might not be needed at all.
Simplest way to do it could be to just add another intermediate state,
something like '_HWMOD_STATE_ENABLED_AT_INIT'.
I will post a patch for this, see if you like it being handled that way.
The other part of the problem is also with the inability to hook up
'custom' omap_device_pm_latency for omap devices created from DT.
Maybe a lot of such cases which need custom activate/deactivate
functions might have to be handled in some way in the framework
itself like the one above.
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] ARM: omap_device: handle first time activation of console device
2011-11-17 7:19 ` Rajendra Nayak
@ 2011-11-17 9:52 ` Cousson, Benoit
2011-11-17 10:16 ` Rajendra Nayak
0 siblings, 1 reply; 17+ messages in thread
From: Cousson, Benoit @ 2011-11-17 9:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi Rajendra,
On 11/17/2011 8:19 AM, Rajendra Nayak wrote:
> [...]
>>> +static int omap_console_hwmod_enable(struct omap_device *od)
>>> +{
>>> + console_lock();
>>> + /*
>>> + * For early console we prevented hwmod reset and idle
>>
>> A period is missing. Or maybe it should a comma with not capital letter.
>>
>>> + * So before we enable the uart clocks idle the console
>>> + * uart clocks, then enable back the console uart hwmod.
>>> + */
>>> + omap_hwmod_idle(od->hwmods[0]);
>>> + omap_hwmod_enable(od->hwmods[0]);
>>
>> Why do we have to idle -> enable? The module is already enabled, isn't
>> it?
>> The comment is not super clear for me :-)
>> Is the goal is to reset the IP?
>
> Yes, now that I read it, it does sound confusing. I should have at-least
> read it once before I picked it from serial.c
>
> But anyway, here's what the problem is.
>
> I feel its partly to do with the inability of hwmod to handle some
> modules which are left enabled post a setup, due to the
> 'HWMOD_INIT_NO_IDLE' flag set.
> Such modules end up with a hwmod state as '_HWMOD_STATE_ENABLED' post
> a setup. Now when a driver for such devices/modules tries to enable the
> module the first time, hwmod throws up a big WARN stating the hwmod is
> already in an enabled state.
OK, now, that makes sense :-)
We have hwmod in ENABLE state whereas the omap_device is still in IDLE
or even DISABLE.
> [uart used as console is one such module, which cannot be idled post a
> setup by hwmod]
>
> If hwmod could be made in some way intelligent enough to know that the
> module is in enabled state because of the 'HWMOD_INIT_NO_IDLE' itself,
> a lot of this hackery might not be needed at all.
Fully agree, the fmwk should handle that.
> Simplest way to do it could be to just add another intermediate state,
> something like '_HWMOD_STATE_ENABLED_AT_INIT'.
> I will post a patch for this, see if you like it being handled that way.
That seems to be good. I'm just wondering if we need to introduce a new
state for that or use a dedicated flag.
My concern is just that we will have two flavors of HWMOD_STATE_ENABLED
that we will have to check in various places in the hwmod core code.
Maybe that's not such a big deal. Go ahead, and we will see how it looks
like.
> The other part of the problem is also with the inability to hook up
> 'custom' omap_device_pm_latency for omap devices created from DT.
> Maybe a lot of such cases which need custom activate/deactivate
> functions might have to be handled in some way in the framework
> itself like the one above.
For the moment, it looks like only the serial is requiring such custom
stuff, but anyway, we should have a mechanism to allow that...
Thanks,
Benoit
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/3] ARM: omap_device: handle first time activation of console device
2011-11-17 9:52 ` Cousson, Benoit
@ 2011-11-17 10:16 ` Rajendra Nayak
0 siblings, 0 replies; 17+ messages in thread
From: Rajendra Nayak @ 2011-11-17 10:16 UTC (permalink / raw)
To: linux-arm-kernel
[]...
>>>
>>> Why do we have to idle -> enable? The module is already enabled, isn't
>>> it?
>>> The comment is not super clear for me :-)
>>> Is the goal is to reset the IP?
>>
>> Yes, now that I read it, it does sound confusing. I should have at-least
>> read it once before I picked it from serial.c
>>
>> But anyway, here's what the problem is.
>>
>> I feel its partly to do with the inability of hwmod to handle some
>> modules which are left enabled post a setup, due to the
>> 'HWMOD_INIT_NO_IDLE' flag set.
>> Such modules end up with a hwmod state as '_HWMOD_STATE_ENABLED' post
>> a setup. Now when a driver for such devices/modules tries to enable the
>> module the first time, hwmod throws up a big WARN stating the hwmod is
>> already in an enabled state.
>
> OK, now, that makes sense :-)
> We have hwmod in ENABLE state whereas the omap_device is still in IDLE
> or even DISABLE.
Right.
>
>> [uart used as console is one such module, which cannot be idled post a
>> setup by hwmod]
>>
>> If hwmod could be made in some way intelligent enough to know that the
>> module is in enabled state because of the 'HWMOD_INIT_NO_IDLE' itself,
>> a lot of this hackery might not be needed at all.
>
> Fully agree, the fmwk should handle that.
>
>> Simplest way to do it could be to just add another intermediate state,
>> something like '_HWMOD_STATE_ENABLED_AT_INIT'.
>> I will post a patch for this, see if you like it being handled that way.
>
> That seems to be good. I'm just wondering if we need to introduce a new
> state for that or use a dedicated flag.
> My concern is just that we will have two flavors of HWMOD_STATE_ENABLED
> that we will have to check in various places in the hwmod core code.
>
> Maybe that's not such a big deal. Go ahead, and we will see how it looks
> like.
I initially thought I could do that using the existing flag itself, but
the key is that its needed only the first time a enable is done on the
hwmod. The rest of the state handling remains the same.
I just posted the patch, so that should make it more clear.
>
>> The other part of the problem is also with the inability to hook up
>> 'custom' omap_device_pm_latency for omap devices created from DT.
>> Maybe a lot of such cases which need custom activate/deactivate
>> functions might have to be handled in some way in the framework
>> itself like the one above.
>
> For the moment, it looks like only the serial is requiring such custom
yes.
> stuff, but anyway, we should have a mechanism to allow that...
>
> Thanks,
> Benoit
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] omap-serial: Add minimal device tree support
2011-11-16 11:02 [PATCH 0/3] OMAP serial device tree support Rajendra Nayak
2011-11-16 11:02 ` [PATCH 1/3] ARM: omap_device: handle first time activation of console device Rajendra Nayak
@ 2011-11-16 11:02 ` Rajendra Nayak
2011-11-16 14:59 ` Rob Herring
2011-11-16 11:02 ` [PATCH 3/3] ARM: omap: pass minimal SoC/board data for UART from dt Rajendra Nayak
2 siblings, 1 reply; 17+ messages in thread
From: Rajendra Nayak @ 2011-11-16 11:02 UTC (permalink / raw)
To: linux-arm-kernel
Adapt the driver to device tree and pass minimal platform
data from device tree needed for console boot.
No power management features will be suppported for now
since it requires more tweaks around OCP settings
to toggle forceidle/noidle/smaridle bits and handling
remote wakeup and dynamic muxing.
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
.../devicetree/bindings/serial/omap_serial.txt | 9 +++++
drivers/tty/serial/omap-serial.c | 37 +++++++++++++++++++-
2 files changed, 45 insertions(+), 1 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/omap_serial.txt
diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
new file mode 100644
index 0000000..bf6d631
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
@@ -0,0 +1,9 @@
+OMAP UART controller
+
+Required properties:
+- compatible : should be "ti,omap-uart"
+- ti,hwmods : Must be "uart<n>", n being the instance number (1-based)
+
+Optional properties:
+- clock-frequency : frequency of the clock input to the UART
+- ti,console_hwmod : boolean, UART used as debug console
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index e1c8527..e3419c6 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -38,6 +38,7 @@
#include <linux/serial_core.h>
#include <linux/irq.h>
#include <linux/pm_runtime.h>
+#include <linux/of.h>
#include <plat/dma.h>
#include <plat/dmtimer.h>
@@ -1322,6 +1323,22 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
return;
}
+static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
+{
+ struct omap_uart_port_info *omap_up_info;
+
+ omap_up_info = devm_kzalloc(dev, sizeof(*omap_up_info), GFP_KERNEL);
+ if (!omap_up_info)
+ return NULL; /* out of memory */
+
+ of_property_read_u32(dev->of_node, "clock-frequency",
+ &(*omap_up_info).uartclk);
+
+ return omap_up_info;
+}
+
+static atomic_t omap_uart_next_id = ATOMIC_INIT(0);
+
static int serial_omap_probe(struct platform_device *pdev)
{
struct uart_omap_port *up;
@@ -1329,6 +1346,11 @@ static int serial_omap_probe(struct platform_device *pdev)
struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
int ret = -ENOSPC;
+ if (pdev->dev.of_node) {
+ omap_up_info = of_get_uart_port_info(&pdev->dev);
+ pdev->id = atomic_inc_return(&omap_uart_next_id) - 1;
+ }
+
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!mem) {
dev_err(&pdev->dev, "no mem resource?\n");
@@ -1523,7 +1545,7 @@ static int serial_omap_runtime_suspend(struct device *dev)
if (!up)
return -EINVAL;
- if (!pdata->enable_wakeup)
+ if (!pdata || !pdata->enable_wakeup)
return 0;
if (pdata->get_context_loss_count)
@@ -1582,12 +1604,25 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = {
serial_omap_runtime_resume, NULL)
};
+#if defined(CONFIG_OF)
+static const struct of_device_id omap_serial_of_match[] = {
+ {
+ .compatible = "ti,omap-uart",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, omap_serial_of_match);
+#else
+#define omap_serial_of_match NULL
+#endif
+
static struct platform_driver serial_omap_driver = {
.probe = serial_omap_probe,
.remove = serial_omap_remove,
.driver = {
.name = DRIVER_NAME,
.pm = &serial_omap_dev_pm_ops,
+ .of_match_table = omap_serial_of_match,
},
};
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/3] omap-serial: Add minimal device tree support
2011-11-16 11:02 ` [PATCH 2/3] omap-serial: Add minimal device tree support Rajendra Nayak
@ 2011-11-16 14:59 ` Rob Herring
2011-11-17 8:39 ` Rajendra Nayak
0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2011-11-16 14:59 UTC (permalink / raw)
To: linux-arm-kernel
On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
> Adapt the driver to device tree and pass minimal platform
> data from device tree needed for console boot.
> No power management features will be suppported for now
> since it requires more tweaks around OCP settings
> to toggle forceidle/noidle/smaridle bits and handling
> remote wakeup and dynamic muxing.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
> .../devicetree/bindings/serial/omap_serial.txt | 9 +++++
> drivers/tty/serial/omap-serial.c | 37 +++++++++++++++++++-
> 2 files changed, 45 insertions(+), 1 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/serial/omap_serial.txt
>
> diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
> new file mode 100644
> index 0000000..bf6d631
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
> @@ -0,0 +1,9 @@
> +OMAP UART controller
> +
> +Required properties:
> +- compatible : should be "ti,omap-uart"
This seems too generic. There are no h/w differences in the uart since
the 1st OMAP1 device?
> +- ti,hwmods : Must be "uart<n>", n being the instance number (1-based)
> +
> +Optional properties:
> +- clock-frequency : frequency of the clock input to the UART
> +- ti,console_hwmod : boolean, UART used as debug console
> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
> index e1c8527..e3419c6 100644
> --- a/drivers/tty/serial/omap-serial.c
> +++ b/drivers/tty/serial/omap-serial.c
> @@ -38,6 +38,7 @@
> #include <linux/serial_core.h>
> #include <linux/irq.h>
> #include <linux/pm_runtime.h>
> +#include <linux/of.h>
>
> #include <plat/dma.h>
> #include <plat/dmtimer.h>
> @@ -1322,6 +1323,22 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
> return;
> }
>
> +static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
> +{
> + struct omap_uart_port_info *omap_up_info;
> +
> + omap_up_info = devm_kzalloc(dev, sizeof(*omap_up_info), GFP_KERNEL);
> + if (!omap_up_info)
> + return NULL; /* out of memory */
> +
> + of_property_read_u32(dev->of_node, "clock-frequency",
> + &(*omap_up_info).uartclk);
&omap_up_info->uartclk
You want 0 to be the default freq?
> +
> + return omap_up_info;
> +}
> +
> +static atomic_t omap_uart_next_id = ATOMIC_INIT(0);
> +
> static int serial_omap_probe(struct platform_device *pdev)
> {
> struct uart_omap_port *up;
> @@ -1329,6 +1346,11 @@ static int serial_omap_probe(struct platform_device *pdev)
> struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
> int ret = -ENOSPC;
>
> + if (pdev->dev.of_node) {
> + omap_up_info = of_get_uart_port_info(&pdev->dev);
> + pdev->id = atomic_inc_return(&omap_uart_next_id) - 1;
I don't think a driver changing this value is a good idea. Look at other
serial drivers like iMX for how they use aliases.
> + }
> +
> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> if (!mem) {
> dev_err(&pdev->dev, "no mem resource?\n");
> @@ -1523,7 +1545,7 @@ static int serial_omap_runtime_suspend(struct device *dev)
> if (!up)
> return -EINVAL;
>
> - if (!pdata->enable_wakeup)
> + if (!pdata || !pdata->enable_wakeup)
> return 0;
>
> if (pdata->get_context_loss_count)
> @@ -1582,12 +1604,25 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = {
> serial_omap_runtime_resume, NULL)
> };
>
> +#if defined(CONFIG_OF)
> +static const struct of_device_id omap_serial_of_match[] = {
> + {
> + .compatible = "ti,omap-uart",
> + },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, omap_serial_of_match);
> +#else
> +#define omap_serial_of_match NULL
> +#endif
> +
> static struct platform_driver serial_omap_driver = {
> .probe = serial_omap_probe,
> .remove = serial_omap_remove,
> .driver = {
> .name = DRIVER_NAME,
> .pm = &serial_omap_dev_pm_ops,
> + .of_match_table = omap_serial_of_match,
Use of_match_ptr and get rid of the #else
Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 2/3] omap-serial: Add minimal device tree support
2011-11-16 14:59 ` Rob Herring
@ 2011-11-17 8:39 ` Rajendra Nayak
0 siblings, 0 replies; 17+ messages in thread
From: Rajendra Nayak @ 2011-11-17 8:39 UTC (permalink / raw)
To: linux-arm-kernel
On Wednesday 16 November 2011 08:29 PM, Rob Herring wrote:
> On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
>> Adapt the driver to device tree and pass minimal platform
>> data from device tree needed for console boot.
>> No power management features will be suppported for now
>> since it requires more tweaks around OCP settings
>> to toggle forceidle/noidle/smaridle bits and handling
>> remote wakeup and dynamic muxing.
>>
>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>> ---
>> .../devicetree/bindings/serial/omap_serial.txt | 9 +++++
>> drivers/tty/serial/omap-serial.c | 37 +++++++++++++++++++-
>> 2 files changed, 45 insertions(+), 1 deletions(-)
>> create mode 100644 Documentation/devicetree/bindings/serial/omap_serial.txt
>>
>> diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
>> new file mode 100644
>> index 0000000..bf6d631
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
>> @@ -0,0 +1,9 @@
>> +OMAP UART controller
>> +
>> +Required properties:
>> +- compatible : should be "ti,omap-uart"
>
> This seems too generic. There are no h/w differences in the uart since
> the 1st OMAP1 device?
This driver supports only OMAP2+ devices, and there doesn't seem to be
really much difference. But I might have to re-look, in case I need to
handle some silicon errata.
>
>> +- ti,hwmods : Must be "uart<n>", n being the instance number (1-based)
>> +
>> +Optional properties:
>> +- clock-frequency : frequency of the clock input to the UART
>> +- ti,console_hwmod : boolean, UART used as debug console
>> diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
>> index e1c8527..e3419c6 100644
>> --- a/drivers/tty/serial/omap-serial.c
>> +++ b/drivers/tty/serial/omap-serial.c
>> @@ -38,6 +38,7 @@
>> #include<linux/serial_core.h>
>> #include<linux/irq.h>
>> #include<linux/pm_runtime.h>
>> +#include<linux/of.h>
>>
>> #include<plat/dma.h>
>> #include<plat/dmtimer.h>
>> @@ -1322,6 +1323,22 @@ static void uart_tx_dma_callback(int lch, u16 ch_status, void *data)
>> return;
>> }
>>
>> +static struct omap_uart_port_info *of_get_uart_port_info(struct device *dev)
>> +{
>> + struct omap_uart_port_info *omap_up_info;
>> +
>> + omap_up_info = devm_kzalloc(dev, sizeof(*omap_up_info), GFP_KERNEL);
>> + if (!omap_up_info)
>> + return NULL; /* out of memory */
>> +
>> + of_property_read_u32(dev->of_node, "clock-frequency",
>> + &(*omap_up_info).uartclk);
>
> &omap_up_info->uartclk
>
> You want 0 to be the default freq?
good point, I need to handle this better.
>
>> +
>> + return omap_up_info;
>> +}
>> +
>> +static atomic_t omap_uart_next_id = ATOMIC_INIT(0);
>> +
>> static int serial_omap_probe(struct platform_device *pdev)
>> {
>> struct uart_omap_port *up;
>> @@ -1329,6 +1346,11 @@ static int serial_omap_probe(struct platform_device *pdev)
>> struct omap_uart_port_info *omap_up_info = pdev->dev.platform_data;
>> int ret = -ENOSPC;
>>
>> + if (pdev->dev.of_node) {
>> + omap_up_info = of_get_uart_port_info(&pdev->dev);
>> + pdev->id = atomic_inc_return(&omap_uart_next_id) - 1;
>
> I don't think a driver changing this value is a good idea. Look at other
> serial drivers like iMX for how they use aliases.
Well, I did lookup, but maybe the wrong one. I looked up the msm serial
which added DT support not too long ago and does the exact same thing.
>
>> + }
>> +
>> mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> if (!mem) {
>> dev_err(&pdev->dev, "no mem resource?\n");
>> @@ -1523,7 +1545,7 @@ static int serial_omap_runtime_suspend(struct device *dev)
>> if (!up)
>> return -EINVAL;
>>
>> - if (!pdata->enable_wakeup)
>> + if (!pdata || !pdata->enable_wakeup)
>> return 0;
>>
>> if (pdata->get_context_loss_count)
>> @@ -1582,12 +1604,25 @@ static const struct dev_pm_ops serial_omap_dev_pm_ops = {
>> serial_omap_runtime_resume, NULL)
>> };
>>
>> +#if defined(CONFIG_OF)
>> +static const struct of_device_id omap_serial_of_match[] = {
>> + {
>> + .compatible = "ti,omap-uart",
>> + },
>> + {},
>> +};
>> +MODULE_DEVICE_TABLE(of, omap_serial_of_match);
>> +#else
>> +#define omap_serial_of_match NULL
>> +#endif
>> +
>> static struct platform_driver serial_omap_driver = {
>> .probe = serial_omap_probe,
>> .remove = serial_omap_remove,
>> .driver = {
>> .name = DRIVER_NAME,
>> .pm =&serial_omap_dev_pm_ops,
>> + .of_match_table = omap_serial_of_match,
>
> Use of_match_ptr and get rid of the #else
Ok, thanks.
Rajendra
>
> Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] ARM: omap: pass minimal SoC/board data for UART from dt
2011-11-16 11:02 [PATCH 0/3] OMAP serial device tree support Rajendra Nayak
2011-11-16 11:02 ` [PATCH 1/3] ARM: omap_device: handle first time activation of console device Rajendra Nayak
2011-11-16 11:02 ` [PATCH 2/3] omap-serial: Add minimal device tree support Rajendra Nayak
@ 2011-11-16 11:02 ` Rajendra Nayak
2011-11-17 1:04 ` Rob Herring
2 siblings, 1 reply; 17+ messages in thread
From: Rajendra Nayak @ 2011-11-16 11:02 UTC (permalink / raw)
To: linux-arm-kernel
Pass minimal data needed for console boot, from dt, for
OMAP4 panda/sdp and OMAP3 beagle boards, and get rid of the
static initialization from generic board file.
Signed-off-by: Rajendra Nayak <rnayak@ti.com>
---
arch/arm/boot/dts/omap3-beagle.dts | 17 +++++++++++++++++
arch/arm/boot/dts/omap3.dtsi | 27 +++++++++++++++++++++++++++
arch/arm/boot/dts/omap4-panda.dts | 17 +++++++++++++++++
arch/arm/boot/dts/omap4-sdp.dts | 17 +++++++++++++++++
arch/arm/boot/dts/omap4.dtsi | 24 ++++++++++++++++++++++++
arch/arm/mach-omap2/board-generic.c | 1 -
6 files changed, 102 insertions(+), 1 deletions(-)
diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
index 9486be6..4c8f11e 100644
--- a/arch/arm/boot/dts/omap3-beagle.dts
+++ b/arch/arm/boot/dts/omap3-beagle.dts
@@ -27,3 +27,20 @@
reg = <0x80000000 0x20000000>; /* 512 MB */
};
};
+
+&uart1 {
+ clock-frequency = <48000000>;
+};
+
+&uart2 {
+ clock-frequency = <48000000>;
+};
+
+&uart3 {
+ ti,console_hwmod;
+ clock-frequency = <48000000>;
+};
+
+&uart4 {
+ clock-frequency = <48000000>;
+};
diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
index d202bb5..ea591c5 100644
--- a/arch/arm/boot/dts/omap3.dtsi
+++ b/arch/arm/boot/dts/omap3.dtsi
@@ -13,6 +13,13 @@
/ {
compatible = "ti,omap3430", "ti,omap3";
+ aliases {
+ uart1 = &uart1;
+ uart2 = &uart2;
+ uart3 = &uart3;
+ uart4 = &uart4;
+ };
+
cpus {
cpu at 0 {
compatible = "arm,cortex-a8";
@@ -59,5 +66,25 @@
interrupt-controller;
#interrupt-cells = <1>;
};
+
+ uart1: uart at 1 {
+ compatible = "ti,omap-uart";
+ ti,hwmods = "uart1";
+ };
+
+ uart2: uart at 2 {
+ compatible = "ti,omap-uart";
+ ti,hwmods = "uart2";
+ };
+
+ uart3: uart at 3 {
+ compatible = "ti,omap-uart";
+ ti,hwmods = "uart3";
+ };
+
+ uart4: uart at 4 {
+ compatible = "ti,omap-uart";
+ ti,hwmods = "uart4";
+ };
};
};
diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
index c702657..aa65449 100644
--- a/arch/arm/boot/dts/omap4-panda.dts
+++ b/arch/arm/boot/dts/omap4-panda.dts
@@ -27,3 +27,20 @@
reg = <0x80000000 0x40000000>; /* 1 GB */
};
};
+
+&uart1 {
+ clock-frequency = <48000000>;
+};
+
+&uart2 {
+ clock-frequency = <48000000>;
+};
+
+&uart3 {
+ ti,console_hwmod;
+ clock-frequency = <48000000>;
+};
+
+&uart4 {
+ clock-frequency = <48000000>;
+};
diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
index 066e28c..524f5bf 100644
--- a/arch/arm/boot/dts/omap4-sdp.dts
+++ b/arch/arm/boot/dts/omap4-sdp.dts
@@ -27,3 +27,20 @@
reg = <0x80000000 0x40000000>; /* 1 GB */
};
};
+
+&uart1 {
+ clock-frequency = <48000000>;
+};
+
+&uart2 {
+ clock-frequency = <48000000>;
+};
+
+&uart3 {
+ ti,console_hwmod;
+ clock-frequency = <48000000>;
+};
+
+&uart4 {
+ clock-frequency = <48000000>;
+};
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 4c61c82..2378d1e 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -21,6 +21,10 @@
interrupt-parent = <&gic>;
aliases {
+ uart1 = &uart1;
+ uart2 = &uart2;
+ uart3 = &uart3;
+ uart4 = &uart4;
};
cpus {
@@ -99,5 +103,25 @@
reg = <0x48241000 0x1000>,
<0x48240100 0x0100>;
};
+
+ uart1: uart at 1 {
+ compatible = "ti,omap-uart";
+ ti,hwmods = "uart1";
+ };
+
+ uart2: uart at 2 {
+ compatible = "ti,omap-uart";
+ ti,hwmods = "uart2";
+ };
+
+ uart3: uart at 3 {
+ compatible = "ti,omap-uart";
+ ti,hwmods = "uart3";
+ };
+
+ uart4: uart at 4 {
+ compatible = "ti,omap-uart";
+ ti,hwmods = "uart4";
+ };
};
};
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index fb55fa3..bb72809 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -70,7 +70,6 @@ static void __init omap_generic_init(void)
if (node)
irq_domain_add_simple(node, 0);
- omap_serial_init();
omap_sdrc_init(NULL, NULL);
of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
--
1.7.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/3] ARM: omap: pass minimal SoC/board data for UART from dt
2011-11-16 11:02 ` [PATCH 3/3] ARM: omap: pass minimal SoC/board data for UART from dt Rajendra Nayak
@ 2011-11-17 1:04 ` Rob Herring
2011-11-17 8:42 ` Rajendra Nayak
0 siblings, 1 reply; 17+ messages in thread
From: Rob Herring @ 2011-11-17 1:04 UTC (permalink / raw)
To: linux-arm-kernel
On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
> Pass minimal data needed for console boot, from dt, for
> OMAP4 panda/sdp and OMAP3 beagle boards, and get rid of the
> static initialization from generic board file.
>
> Signed-off-by: Rajendra Nayak <rnayak@ti.com>
> ---
> arch/arm/boot/dts/omap3-beagle.dts | 17 +++++++++++++++++
> arch/arm/boot/dts/omap3.dtsi | 27 +++++++++++++++++++++++++++
> arch/arm/boot/dts/omap4-panda.dts | 17 +++++++++++++++++
> arch/arm/boot/dts/omap4-sdp.dts | 17 +++++++++++++++++
> arch/arm/boot/dts/omap4.dtsi | 24 ++++++++++++++++++++++++
> arch/arm/mach-omap2/board-generic.c | 1 -
> 6 files changed, 102 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
> index 9486be6..4c8f11e 100644
> --- a/arch/arm/boot/dts/omap3-beagle.dts
> +++ b/arch/arm/boot/dts/omap3-beagle.dts
> @@ -27,3 +27,20 @@
> reg = <0x80000000 0x20000000>; /* 512 MB */
> };
> };
> +
> +&uart1 {
> + clock-frequency = <48000000>;
> +};
> +
> +&uart2 {
> + clock-frequency = <48000000>;
> +};
> +
> +&uart3 {
> + ti,console_hwmod;
> + clock-frequency = <48000000>;
> +};
> +
> +&uart4 {
> + clock-frequency = <48000000>;
> +};
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index d202bb5..ea591c5 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -13,6 +13,13 @@
> / {
> compatible = "ti,omap3430", "ti,omap3";
>
> + aliases {
> + uart1 = &uart1;
> + uart2 = &uart2;
> + uart3 = &uart3;
> + uart4 = &uart4;
> + };
> +
> cpus {
> cpu at 0 {
> compatible = "arm,cortex-a8";
> @@ -59,5 +66,25 @@
> interrupt-controller;
> #interrupt-cells = <1>;
> };
> +
> + uart1: uart at 1 {
Use the generic name serial and the address: uart1: serial at 1234abcd
> + compatible = "ti,omap-uart";
> + ti,hwmods = "uart1";
> + };
> +
> + uart2: uart at 2 {
> + compatible = "ti,omap-uart";
> + ti,hwmods = "uart2";
> + };
> +
> + uart3: uart at 3 {
> + compatible = "ti,omap-uart";
> + ti,hwmods = "uart3";
> + };
> +
> + uart4: uart at 4 {
> + compatible = "ti,omap-uart";
> + ti,hwmods = "uart4";
> + };
> };
> };
> diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
> index c702657..aa65449 100644
> --- a/arch/arm/boot/dts/omap4-panda.dts
> +++ b/arch/arm/boot/dts/omap4-panda.dts
> @@ -27,3 +27,20 @@
> reg = <0x80000000 0x40000000>; /* 1 GB */
> };
> };
> +
> +&uart1 {
> + clock-frequency = <48000000>;
> +};
> +
> +&uart2 {
> + clock-frequency = <48000000>;
> +};
> +
> +&uart3 {
> + ti,console_hwmod;
> + clock-frequency = <48000000>;
> +};
> +
> +&uart4 {
> + clock-frequency = <48000000>;
> +};
> diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
> index 066e28c..524f5bf 100644
> --- a/arch/arm/boot/dts/omap4-sdp.dts
> +++ b/arch/arm/boot/dts/omap4-sdp.dts
> @@ -27,3 +27,20 @@
> reg = <0x80000000 0x40000000>; /* 1 GB */
> };
> };
> +
> +&uart1 {
> + clock-frequency = <48000000>;
> +};
> +
> +&uart2 {
> + clock-frequency = <48000000>;
> +};
> +
> +&uart3 {
> + ti,console_hwmod;
> + clock-frequency = <48000000>;
> +};
> +
> +&uart4 {
> + clock-frequency = <48000000>;
It doesn't seem that this frequency ever varies and is likely to be
replaced with clock bindings, so maybe just put it in the dtsi files.
Rob
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 3/3] ARM: omap: pass minimal SoC/board data for UART from dt
2011-11-17 1:04 ` Rob Herring
@ 2011-11-17 8:42 ` Rajendra Nayak
0 siblings, 0 replies; 17+ messages in thread
From: Rajendra Nayak @ 2011-11-17 8:42 UTC (permalink / raw)
To: linux-arm-kernel
On Thursday 17 November 2011 06:34 AM, Rob Herring wrote:
> On 11/16/2011 05:02 AM, Rajendra Nayak wrote:
>> Pass minimal data needed for console boot, from dt, for
>> OMAP4 panda/sdp and OMAP3 beagle boards, and get rid of the
>> static initialization from generic board file.
>>
>> Signed-off-by: Rajendra Nayak<rnayak@ti.com>
>> ---
>> arch/arm/boot/dts/omap3-beagle.dts | 17 +++++++++++++++++
>> arch/arm/boot/dts/omap3.dtsi | 27 +++++++++++++++++++++++++++
>> arch/arm/boot/dts/omap4-panda.dts | 17 +++++++++++++++++
>> arch/arm/boot/dts/omap4-sdp.dts | 17 +++++++++++++++++
>> arch/arm/boot/dts/omap4.dtsi | 24 ++++++++++++++++++++++++
>> arch/arm/mach-omap2/board-generic.c | 1 -
>> 6 files changed, 102 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
>> index 9486be6..4c8f11e 100644
>> --- a/arch/arm/boot/dts/omap3-beagle.dts
>> +++ b/arch/arm/boot/dts/omap3-beagle.dts
>> @@ -27,3 +27,20 @@
>> reg =<0x80000000 0x20000000>; /* 512 MB */
>> };
>> };
>> +
>> +&uart1 {
>> + clock-frequency =<48000000>;
>> +};
>> +
>> +&uart2 {
>> + clock-frequency =<48000000>;
>> +};
>> +
>> +&uart3 {
>> + ti,console_hwmod;
>> + clock-frequency =<48000000>;
>> +};
>> +
>> +&uart4 {
>> + clock-frequency =<48000000>;
>> +};
>> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
>> index d202bb5..ea591c5 100644
>> --- a/arch/arm/boot/dts/omap3.dtsi
>> +++ b/arch/arm/boot/dts/omap3.dtsi
>> @@ -13,6 +13,13 @@
>> / {
>> compatible = "ti,omap3430", "ti,omap3";
>>
>> + aliases {
>> + uart1 =&uart1;
>> + uart2 =&uart2;
>> + uart3 =&uart3;
>> + uart4 =&uart4;
>> + };
>> +
>> cpus {
>> cpu at 0 {
>> compatible = "arm,cortex-a8";
>> @@ -59,5 +66,25 @@
>> interrupt-controller;
>> #interrupt-cells =<1>;
>> };
>> +
>> + uart1: uart at 1 {
>
> Use the generic name serial and the address: uart1: serial at 1234abcd
ok.
>
>> + compatible = "ti,omap-uart";
>> + ti,hwmods = "uart1";
>> + };
>> +
>> + uart2: uart at 2 {
>> + compatible = "ti,omap-uart";
>> + ti,hwmods = "uart2";
>> + };
>> +
>> + uart3: uart at 3 {
>> + compatible = "ti,omap-uart";
>> + ti,hwmods = "uart3";
>> + };
>> +
>> + uart4: uart at 4 {
>> + compatible = "ti,omap-uart";
>> + ti,hwmods = "uart4";
>> + };
>> };
>> };
>> diff --git a/arch/arm/boot/dts/omap4-panda.dts b/arch/arm/boot/dts/omap4-panda.dts
>> index c702657..aa65449 100644
>> --- a/arch/arm/boot/dts/omap4-panda.dts
>> +++ b/arch/arm/boot/dts/omap4-panda.dts
>> @@ -27,3 +27,20 @@
>> reg =<0x80000000 0x40000000>; /* 1 GB */
>> };
>> };
>> +
>> +&uart1 {
>> + clock-frequency =<48000000>;
>> +};
>> +
>> +&uart2 {
>> + clock-frequency =<48000000>;
>> +};
>> +
>> +&uart3 {
>> + ti,console_hwmod;
>> + clock-frequency =<48000000>;
>> +};
>> +
>> +&uart4 {
>> + clock-frequency =<48000000>;
>> +};
>> diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts
>> index 066e28c..524f5bf 100644
>> --- a/arch/arm/boot/dts/omap4-sdp.dts
>> +++ b/arch/arm/boot/dts/omap4-sdp.dts
>> @@ -27,3 +27,20 @@
>> reg =<0x80000000 0x40000000>; /* 1 GB */
>> };
>> };
>> +
>> +&uart1 {
>> + clock-frequency =<48000000>;
>> +};
>> +
>> +&uart2 {
>> + clock-frequency =<48000000>;
>> +};
>> +
>> +&uart3 {
>> + ti,console_hwmod;
>> + clock-frequency =<48000000>;
>> +};
>> +
>> +&uart4 {
>> + clock-frequency =<48000000>;
>
> It doesn't seem that this frequency ever varies and is likely to be
> replaced with clock bindings, so maybe just put it in the dtsi files.
sure, will do.
regards,
Rajendra
>
> Rob
>
^ permalink raw reply [flat|nested] 17+ messages in thread