* [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
@ 2012-08-07 13:37 ` Vaibhav Hiremath
0 siblings, 0 replies; 14+ messages in thread
From: Vaibhav Hiremath @ 2012-08-07 13:37 UTC (permalink / raw)
To: linux-omap
Cc: linux-arm-kernel, Vaibhav Hiremath, Tony Lindgren, Paul Walmsley,
Benoit Cousson, Grant Likely
If the module requires only one clocksource to function, then
driver can very well call clk_get() api with "con_id = NULL",
clk = clk_get(dev, NULL);
And platform code should respect this and return valid clk handle.
That means, now the clk_get() api would rely on dev_id, and platform
code should either have clk node with matching dev_id (with con_id=NULL)
or return dummy clk node.
With DT support, we can fix the dev_id for particular module
using "struct of_dev_auxdata" to satisfy above clk framework requirement.
In case of AM33XX, we required this for the DCAN module, so this
patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
This patch is boot tested on BeagleBone platform, and checked for
clk_get() return value in d_can module driver.
arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 6f93a20..c9b7903 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
{ }
};
+/*
+ * Lookup table for attaching a specific name and platform_data pointer to
+ * devices as they get created by of_platform_populate(). Ideally this table
+ * would not exist, but the current clock implementation depends on some devices
+ * having a specific name OR to satisfy NULL con_id requirement from driver.
+ */
+static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
+ OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
+ OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
+ { },
+};
+
static void __init omap_generic_init(void)
{
omap_sdrc_init(NULL, NULL);
- of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
+ if (of_machine_is_compatible("ti,am33xx"))
+ of_platform_populate(NULL, omap_dt_match_table,
+ am33xx_auxdata_lookup, NULL);
+ else
+ of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
}
#ifdef CONFIG_SOC_OMAP2420
--
1.7.0.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
@ 2012-08-07 13:37 ` Vaibhav Hiremath
0 siblings, 0 replies; 14+ messages in thread
From: Vaibhav Hiremath @ 2012-08-07 13:37 UTC (permalink / raw)
To: linux-arm-kernel
If the module requires only one clocksource to function, then
driver can very well call clk_get() api with "con_id = NULL",
clk = clk_get(dev, NULL);
And platform code should respect this and return valid clk handle.
That means, now the clk_get() api would rely on dev_id, and platform
code should either have clk node with matching dev_id (with con_id=NULL)
or return dummy clk node.
With DT support, we can fix the dev_id for particular module
using "struct of_dev_auxdata" to satisfy above clk framework requirement.
In case of AM33XX, we required this for the DCAN module, so this
patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Benoit Cousson <b-cousson@ti.com>
Cc: Grant Likely <grant.likely@secretlab.ca>
---
This patch is boot tested on BeagleBone platform, and checked for
clk_get() return value in d_can module driver.
arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
1 files changed, 17 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
index 6f93a20..c9b7903 100644
--- a/arch/arm/mach-omap2/board-generic.c
+++ b/arch/arm/mach-omap2/board-generic.c
@@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
{ }
};
+/*
+ * Lookup table for attaching a specific name and platform_data pointer to
+ * devices as they get created by of_platform_populate(). Ideally this table
+ * would not exist, but the current clock implementation depends on some devices
+ * having a specific name OR to satisfy NULL con_id requirement from driver.
+ */
+static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
+ OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
+ OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
+ { },
+};
+
static void __init omap_generic_init(void)
{
omap_sdrc_init(NULL, NULL);
- of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
+ if (of_machine_is_compatible("ti,am33xx"))
+ of_platform_populate(NULL, omap_dt_match_table,
+ am33xx_auxdata_lookup, NULL);
+ else
+ of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
}
#ifdef CONFIG_SOC_OMAP2420
--
1.7.0.4
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
2012-08-07 13:37 ` Vaibhav Hiremath
@ 2012-08-07 15:19 ` Rob Herring
-1 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2012-08-07 15:19 UTC (permalink / raw)
To: Vaibhav Hiremath
Cc: linux-omap, Paul Walmsley, Benoit Cousson, Tony Lindgren,
Grant Likely, linux-arm-kernel
On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
> If the module requires only one clocksource to function, then
> driver can very well call clk_get() api with "con_id = NULL",
>
> clk = clk_get(dev, NULL);
>
> And platform code should respect this and return valid clk handle.
> That means, now the clk_get() api would rely on dev_id, and platform
> code should either have clk node with matching dev_id (with con_id=NULL)
> or return dummy clk node.
>
> With DT support, we can fix the dev_id for particular module
> using "struct of_dev_auxdata" to satisfy above clk framework requirement.
>
> In case of AM33XX, we required this for the DCAN module, so this
> patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
>
> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> ---
> This patch is boot tested on BeagleBone platform, and checked for
> clk_get() return value in d_can module driver.
>
> arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
> 1 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> index 6f93a20..c9b7903 100644
> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
> { }
> };
>
> +/*
> + * Lookup table for attaching a specific name and platform_data pointer to
> + * devices as they get created by of_platform_populate(). Ideally this table
> + * would not exist, but the current clock implementation depends on some devices
> + * having a specific name OR to satisfy NULL con_id requirement from driver.
> + */
> +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
> + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
> + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
> + { },
> +};
Adding an additional clkdev lookup accomplishes the same thing and is a
cleaner solution.
Rob
> +
> static void __init omap_generic_init(void)
> {
> omap_sdrc_init(NULL, NULL);
>
> - of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
> + if (of_machine_is_compatible("ti,am33xx"))
> + of_platform_populate(NULL, omap_dt_match_table,
> + am33xx_auxdata_lookup, NULL);
> + else
> + of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
> }
>
> #ifdef CONFIG_SOC_OMAP2420
> --
> 1.7.0.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
@ 2012-08-07 15:19 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2012-08-07 15:19 UTC (permalink / raw)
To: linux-arm-kernel
On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
> If the module requires only one clocksource to function, then
> driver can very well call clk_get() api with "con_id = NULL",
>
> clk = clk_get(dev, NULL);
>
> And platform code should respect this and return valid clk handle.
> That means, now the clk_get() api would rely on dev_id, and platform
> code should either have clk node with matching dev_id (with con_id=NULL)
> or return dummy clk node.
>
> With DT support, we can fix the dev_id for particular module
> using "struct of_dev_auxdata" to satisfy above clk framework requirement.
>
> In case of AM33XX, we required this for the DCAN module, so this
> patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
>
> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Benoit Cousson <b-cousson@ti.com>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> ---
> This patch is boot tested on BeagleBone platform, and checked for
> clk_get() return value in d_can module driver.
>
> arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
> 1 files changed, 17 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> index 6f93a20..c9b7903 100644
> --- a/arch/arm/mach-omap2/board-generic.c
> +++ b/arch/arm/mach-omap2/board-generic.c
> @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
> { }
> };
>
> +/*
> + * Lookup table for attaching a specific name and platform_data pointer to
> + * devices as they get created by of_platform_populate(). Ideally this table
> + * would not exist, but the current clock implementation depends on some devices
> + * having a specific name OR to satisfy NULL con_id requirement from driver.
> + */
> +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
> + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
> + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
> + { },
> +};
Adding an additional clkdev lookup accomplishes the same thing and is a
cleaner solution.
Rob
> +
> static void __init omap_generic_init(void)
> {
> omap_sdrc_init(NULL, NULL);
>
> - of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
> + if (of_machine_is_compatible("ti,am33xx"))
> + of_platform_populate(NULL, omap_dt_match_table,
> + am33xx_auxdata_lookup, NULL);
> + else
> + of_platform_populate(NULL, omap_dt_match_table, NULL, NULL);
> }
>
> #ifdef CONFIG_SOC_OMAP2420
> --
> 1.7.0.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
2012-08-07 15:19 ` Rob Herring
@ 2012-08-07 15:53 ` Hiremath, Vaibhav
-1 siblings, 0 replies; 14+ messages in thread
From: Hiremath, Vaibhav @ 2012-08-07 15:53 UTC (permalink / raw)
To: Rob Herring
Cc: linux-omap, Paul Walmsley, Cousson, Benoit, Tony Lindgren,
Grant Likely, linux-arm-kernel
On Tue, Aug 07, 2012 at 20:49:48, Rob Herring wrote:
> On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
> > If the module requires only one clocksource to function, then
> > driver can very well call clk_get() api with "con_id = NULL",
> >
> > clk = clk_get(dev, NULL);
> >
> > And platform code should respect this and return valid clk handle.
> > That means, now the clk_get() api would rely on dev_id, and platform
> > code should either have clk node with matching dev_id (with con_id=NULL)
> > or return dummy clk node.
> >
> > With DT support, we can fix the dev_id for particular module
> > using "struct of_dev_auxdata" to satisfy above clk framework requirement.
> >
> > In case of AM33XX, we required this for the DCAN module, so this
> > patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
> >
> > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Benoit Cousson <b-cousson@ti.com>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > ---
> > This patch is boot tested on BeagleBone platform, and checked for
> > clk_get() return value in d_can module driver.
> >
> > arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
> > 1 files changed, 17 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> > index 6f93a20..c9b7903 100644
> > --- a/arch/arm/mach-omap2/board-generic.c
> > +++ b/arch/arm/mach-omap2/board-generic.c
> > @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
> > { }
> > };
> >
> > +/*
> > + * Lookup table for attaching a specific name and platform_data pointer to
> > + * devices as they get created by of_platform_populate(). Ideally this table
> > + * would not exist, but the current clock implementation depends on some devices
> > + * having a specific name OR to satisfy NULL con_id requirement from driver.
> > + */
> > +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
> > + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
> > + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
> > + { },
> > +};
>
> Adding an additional clkdev lookup accomplishes the same thing and is a
> cleaner solution.
>
That is also required and I have submitted patch for the same -
http://www.spinics.net/lists/arm-kernel/msg187998.html
With DT support, I am getting dev_id as -
- Without "reg" property: d_can.16 and d_can.17,
(I believe it changes dynamically here)
- With "reg" property: 481cc000.d_can and 481d0000.d_can
Which is not so intuitive, I would like to see them as per Spec/TRM, so
doesn't this patch make sense?
Similar thing has already been done for other platforms too.
Thanks,
Vaibhav
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
@ 2012-08-07 15:53 ` Hiremath, Vaibhav
0 siblings, 0 replies; 14+ messages in thread
From: Hiremath, Vaibhav @ 2012-08-07 15:53 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Aug 07, 2012 at 20:49:48, Rob Herring wrote:
> On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
> > If the module requires only one clocksource to function, then
> > driver can very well call clk_get() api with "con_id = NULL",
> >
> > clk = clk_get(dev, NULL);
> >
> > And platform code should respect this and return valid clk handle.
> > That means, now the clk_get() api would rely on dev_id, and platform
> > code should either have clk node with matching dev_id (with con_id=NULL)
> > or return dummy clk node.
> >
> > With DT support, we can fix the dev_id for particular module
> > using "struct of_dev_auxdata" to satisfy above clk framework requirement.
> >
> > In case of AM33XX, we required this for the DCAN module, so this
> > patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
> >
> > Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Paul Walmsley <paul@pwsan.com>
> > Cc: Benoit Cousson <b-cousson@ti.com>
> > Cc: Grant Likely <grant.likely@secretlab.ca>
> > ---
> > This patch is boot tested on BeagleBone platform, and checked for
> > clk_get() return value in d_can module driver.
> >
> > arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
> > 1 files changed, 17 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> > index 6f93a20..c9b7903 100644
> > --- a/arch/arm/mach-omap2/board-generic.c
> > +++ b/arch/arm/mach-omap2/board-generic.c
> > @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
> > { }
> > };
> >
> > +/*
> > + * Lookup table for attaching a specific name and platform_data pointer to
> > + * devices as they get created by of_platform_populate(). Ideally this table
> > + * would not exist, but the current clock implementation depends on some devices
> > + * having a specific name OR to satisfy NULL con_id requirement from driver.
> > + */
> > +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
> > + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
> > + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
> > + { },
> > +};
>
> Adding an additional clkdev lookup accomplishes the same thing and is a
> cleaner solution.
>
That is also required and I have submitted patch for the same -
http://www.spinics.net/lists/arm-kernel/msg187998.html
With DT support, I am getting dev_id as -
- Without "reg" property: d_can.16 and d_can.17,
(I believe it changes dynamically here)
- With "reg" property: 481cc000.d_can and 481d0000.d_can
Which is not so intuitive, I would like to see them as per Spec/TRM, so
doesn't this patch make sense?
Similar thing has already been done for other platforms too.
Thanks,
Vaibhav
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
2012-08-07 15:53 ` Hiremath, Vaibhav
@ 2012-08-07 17:42 ` Koen Kooi
-1 siblings, 0 replies; 14+ messages in thread
From: Koen Kooi @ 2012-08-07 17:42 UTC (permalink / raw)
To: Hiremath, Vaibhav
Cc: Rob Herring, linux-omap, Paul Walmsley, Cousson, Benoit,
Tony Lindgren, Grant Likely, linux-arm-kernel
Op 7 aug. 2012, om 17:53 heeft "Hiremath, Vaibhav" <hvaibhav@ti.com> het volgende geschreven:
> On Tue, Aug 07, 2012 at 20:49:48, Rob Herring wrote:
>> On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
>>> If the module requires only one clocksource to function, then
>>> driver can very well call clk_get() api with "con_id = NULL",
>>>
>>> clk = clk_get(dev, NULL);
>>>
>>> And platform code should respect this and return valid clk handle.
>>> That means, now the clk_get() api would rely on dev_id, and platform
>>> code should either have clk node with matching dev_id (with con_id=NULL)
>>> or return dummy clk node.
>>>
>>> With DT support, we can fix the dev_id for particular module
>>> using "struct of_dev_auxdata" to satisfy above clk framework requirement.
>>>
>>> In case of AM33XX, we required this for the DCAN module, so this
>>> patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
>>>
>>> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
>>> Cc: Tony Lindgren <tony@atomide.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>> ---
>>> This patch is boot tested on BeagleBone platform, and checked for
>>> clk_get() return value in d_can module driver.
>>>
>>> arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
>>> 1 files changed, 17 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
>>> index 6f93a20..c9b7903 100644
>>> --- a/arch/arm/mach-omap2/board-generic.c
>>> +++ b/arch/arm/mach-omap2/board-generic.c
>>> @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
>>> { }
>>> };
>>>
>>> +/*
>>> + * Lookup table for attaching a specific name and platform_data pointer to
>>> + * devices as they get created by of_platform_populate(). Ideally this table
>>> + * would not exist, but the current clock implementation depends on some devices
>>> + * having a specific name OR to satisfy NULL con_id requirement from driver.
>>> + */
>>> +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
>>> + { },
>>> +};
>>
>> Adding an additional clkdev lookup accomplishes the same thing and is a
>> cleaner solution.
>>
>
> That is also required and I have submitted patch for the same -
>
> http://www.spinics.net/lists/arm-kernel/msg187998.html
>
>
> With DT support, I am getting dev_id as -
>
> - Without "reg" property: d_can.16 and d_can.17,
> (I believe it changes dynamically here)
>
> - With "reg" property: 481cc000.d_can and 481d0000.d_can
>
> Which is not so intuitive, I would like to see them as per Spec/TRM, so
> doesn't this patch make sense?
>
> Similar thing has already been done for other platforms too.
Davinci-mdio has a similar problem
regards,
Koen
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
@ 2012-08-07 17:42 ` Koen Kooi
0 siblings, 0 replies; 14+ messages in thread
From: Koen Kooi @ 2012-08-07 17:42 UTC (permalink / raw)
To: linux-arm-kernel
Op 7 aug. 2012, om 17:53 heeft "Hiremath, Vaibhav" <hvaibhav@ti.com> het volgende geschreven:
> On Tue, Aug 07, 2012 at 20:49:48, Rob Herring wrote:
>> On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
>>> If the module requires only one clocksource to function, then
>>> driver can very well call clk_get() api with "con_id = NULL",
>>>
>>> clk = clk_get(dev, NULL);
>>>
>>> And platform code should respect this and return valid clk handle.
>>> That means, now the clk_get() api would rely on dev_id, and platform
>>> code should either have clk node with matching dev_id (with con_id=NULL)
>>> or return dummy clk node.
>>>
>>> With DT support, we can fix the dev_id for particular module
>>> using "struct of_dev_auxdata" to satisfy above clk framework requirement.
>>>
>>> In case of AM33XX, we required this for the DCAN module, so this
>>> patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
>>>
>>> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
>>> Cc: Tony Lindgren <tony@atomide.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>> ---
>>> This patch is boot tested on BeagleBone platform, and checked for
>>> clk_get() return value in d_can module driver.
>>>
>>> arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
>>> 1 files changed, 17 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
>>> index 6f93a20..c9b7903 100644
>>> --- a/arch/arm/mach-omap2/board-generic.c
>>> +++ b/arch/arm/mach-omap2/board-generic.c
>>> @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
>>> { }
>>> };
>>>
>>> +/*
>>> + * Lookup table for attaching a specific name and platform_data pointer to
>>> + * devices as they get created by of_platform_populate(). Ideally this table
>>> + * would not exist, but the current clock implementation depends on some devices
>>> + * having a specific name OR to satisfy NULL con_id requirement from driver.
>>> + */
>>> +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
>>> + { },
>>> +};
>>
>> Adding an additional clkdev lookup accomplishes the same thing and is a
>> cleaner solution.
>>
>
> That is also required and I have submitted patch for the same -
>
> http://www.spinics.net/lists/arm-kernel/msg187998.html
>
>
> With DT support, I am getting dev_id as -
>
> - Without "reg" property: d_can.16 and d_can.17,
> (I believe it changes dynamically here)
>
> - With "reg" property: 481cc000.d_can and 481d0000.d_can
>
> Which is not so intuitive, I would like to see them as per Spec/TRM, so
> doesn't this patch make sense?
>
> Similar thing has already been done for other platforms too.
Davinci-mdio has a similar problem
regards,
Koen
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
2012-08-07 15:53 ` Hiremath, Vaibhav
@ 2012-08-08 2:08 ` Rob Herring
-1 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2012-08-08 2:08 UTC (permalink / raw)
To: Hiremath, Vaibhav
Cc: linux-omap, Paul Walmsley, Cousson, Benoit, Tony Lindgren,
Grant Likely, linux-arm-kernel
On 08/07/2012 10:53 AM, Hiremath, Vaibhav wrote:
> On Tue, Aug 07, 2012 at 20:49:48, Rob Herring wrote:
>> On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
>>> If the module requires only one clocksource to function, then
>>> driver can very well call clk_get() api with "con_id = NULL",
>>>
>>> clk = clk_get(dev, NULL);
>>>
>>> And platform code should respect this and return valid clk handle.
>>> That means, now the clk_get() api would rely on dev_id, and platform
>>> code should either have clk node with matching dev_id (with con_id=NULL)
>>> or return dummy clk node.
>>>
>>> With DT support, we can fix the dev_id for particular module
>>> using "struct of_dev_auxdata" to satisfy above clk framework requirement.
>>>
>>> In case of AM33XX, we required this for the DCAN module, so this
>>> patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
>>>
>>> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
>>> Cc: Tony Lindgren <tony@atomide.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>> ---
>>> This patch is boot tested on BeagleBone platform, and checked for
>>> clk_get() return value in d_can module driver.
>>>
>>> arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
>>> 1 files changed, 17 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
>>> index 6f93a20..c9b7903 100644
>>> --- a/arch/arm/mach-omap2/board-generic.c
>>> +++ b/arch/arm/mach-omap2/board-generic.c
>>> @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
>>> { }
>>> };
>>>
>>> +/*
>>> + * Lookup table for attaching a specific name and platform_data pointer to
>>> + * devices as they get created by of_platform_populate(). Ideally this table
>>> + * would not exist, but the current clock implementation depends on some devices
>>> + * having a specific name OR to satisfy NULL con_id requirement from driver.
>>> + */
>>> +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
>>> + { },
>>> +};
>>
>> Adding an additional clkdev lookup accomplishes the same thing and is a
>> cleaner solution.
>>
>
> That is also required and I have submitted patch for the same -
>
> http://www.spinics.net/lists/arm-kernel/msg187998.html
That only adds the non-DT name. What I'm saying is you can have 2 lookup
names for the same clock.
>
> With DT support, I am getting dev_id as -
>
> - Without "reg" property: d_can.16 and d_can.17,
> (I believe it changes dynamically here)
>
> - With "reg" property: 481cc000.d_can and 481d0000.d_can
>
> Which is not so intuitive, I would like to see them as per Spec/TRM, so
> doesn't this patch make sense?
The spec doesn't have a base address for the module? This name is going
to get used in sysfs anyway, and the old name should be going away.
> Similar thing has already been done for other platforms too.
Only ones I haven't reviewed.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
@ 2012-08-08 2:08 ` Rob Herring
0 siblings, 0 replies; 14+ messages in thread
From: Rob Herring @ 2012-08-08 2:08 UTC (permalink / raw)
To: linux-arm-kernel
On 08/07/2012 10:53 AM, Hiremath, Vaibhav wrote:
> On Tue, Aug 07, 2012 at 20:49:48, Rob Herring wrote:
>> On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
>>> If the module requires only one clocksource to function, then
>>> driver can very well call clk_get() api with "con_id = NULL",
>>>
>>> clk = clk_get(dev, NULL);
>>>
>>> And platform code should respect this and return valid clk handle.
>>> That means, now the clk_get() api would rely on dev_id, and platform
>>> code should either have clk node with matching dev_id (with con_id=NULL)
>>> or return dummy clk node.
>>>
>>> With DT support, we can fix the dev_id for particular module
>>> using "struct of_dev_auxdata" to satisfy above clk framework requirement.
>>>
>>> In case of AM33XX, we required this for the DCAN module, so this
>>> patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
>>>
>>> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
>>> Cc: Tony Lindgren <tony@atomide.com>
>>> Cc: Paul Walmsley <paul@pwsan.com>
>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>> ---
>>> This patch is boot tested on BeagleBone platform, and checked for
>>> clk_get() return value in d_can module driver.
>>>
>>> arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
>>> 1 files changed, 17 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
>>> index 6f93a20..c9b7903 100644
>>> --- a/arch/arm/mach-omap2/board-generic.c
>>> +++ b/arch/arm/mach-omap2/board-generic.c
>>> @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
>>> { }
>>> };
>>>
>>> +/*
>>> + * Lookup table for attaching a specific name and platform_data pointer to
>>> + * devices as they get created by of_platform_populate(). Ideally this table
>>> + * would not exist, but the current clock implementation depends on some devices
>>> + * having a specific name OR to satisfy NULL con_id requirement from driver.
>>> + */
>>> +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
>>> + { },
>>> +};
>>
>> Adding an additional clkdev lookup accomplishes the same thing and is a
>> cleaner solution.
>>
>
> That is also required and I have submitted patch for the same -
>
> http://www.spinics.net/lists/arm-kernel/msg187998.html
That only adds the non-DT name. What I'm saying is you can have 2 lookup
names for the same clock.
>
> With DT support, I am getting dev_id as -
>
> - Without "reg" property: d_can.16 and d_can.17,
> (I believe it changes dynamically here)
>
> - With "reg" property: 481cc000.d_can and 481d0000.d_can
>
> Which is not so intuitive, I would like to see them as per Spec/TRM, so
> doesn't this patch make sense?
The spec doesn't have a base address for the module? This name is going
to get used in sysfs anyway, and the old name should be going away.
> Similar thing has already been done for other platforms too.
Only ones I haven't reviewed.
Rob
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
2012-08-08 2:08 ` Rob Herring
@ 2012-08-08 11:43 ` Hiremath, Vaibhav
-1 siblings, 0 replies; 14+ messages in thread
From: Hiremath, Vaibhav @ 2012-08-08 11:43 UTC (permalink / raw)
To: Rob Herring
Cc: linux-omap, Paul Walmsley, Cousson, Benoit, Tony Lindgren,
Grant Likely, linux-arm-kernel
On Wed, Aug 08, 2012 at 07:38:01, Rob Herring wrote:
> On 08/07/2012 10:53 AM, Hiremath, Vaibhav wrote:
> > On Tue, Aug 07, 2012 at 20:49:48, Rob Herring wrote:
> >> On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
> >>> If the module requires only one clocksource to function, then
> >>> driver can very well call clk_get() api with "con_id = NULL",
> >>>
> >>> clk = clk_get(dev, NULL);
> >>>
> >>> And platform code should respect this and return valid clk handle.
> >>> That means, now the clk_get() api would rely on dev_id, and platform
> >>> code should either have clk node with matching dev_id (with con_id=NULL)
> >>> or return dummy clk node.
> >>>
> >>> With DT support, we can fix the dev_id for particular module
> >>> using "struct of_dev_auxdata" to satisfy above clk framework requirement.
> >>>
> >>> In case of AM33XX, we required this for the DCAN module, so this
> >>> patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
> >>>
> >>> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> >>> Cc: Tony Lindgren <tony@atomide.com>
> >>> Cc: Paul Walmsley <paul@pwsan.com>
> >>> Cc: Benoit Cousson <b-cousson@ti.com>
> >>> Cc: Grant Likely <grant.likely@secretlab.ca>
> >>> ---
> >>> This patch is boot tested on BeagleBone platform, and checked for
> >>> clk_get() return value in d_can module driver.
> >>>
> >>> arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
> >>> 1 files changed, 17 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> >>> index 6f93a20..c9b7903 100644
> >>> --- a/arch/arm/mach-omap2/board-generic.c
> >>> +++ b/arch/arm/mach-omap2/board-generic.c
> >>> @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
> >>> { }
> >>> };
> >>>
> >>> +/*
> >>> + * Lookup table for attaching a specific name and platform_data pointer to
> >>> + * devices as they get created by of_platform_populate(). Ideally this table
> >>> + * would not exist, but the current clock implementation depends on some devices
> >>> + * having a specific name OR to satisfy NULL con_id requirement from driver.
> >>> + */
> >>> +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
> >>> + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
> >>> + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
> >>> + { },
> >>> +};
> >>
> >> Adding an additional clkdev lookup accomplishes the same thing and is a
> >> cleaner solution.
> >>
> >
> > That is also required and I have submitted patch for the same -
> >
> > http://www.spinics.net/lists/arm-kernel/msg187998.html
>
> That only adds the non-DT name. What I'm saying is you can have 2 lookup
> names for the same clock.
>
> >
> > With DT support, I am getting dev_id as -
> >
> > - Without "reg" property: d_can.16 and d_can.17,
> > (I believe it changes dynamically here)
> >
> > - With "reg" property: 481cc000.d_can and 481d0000.d_can
> >
> > Which is not so intuitive, I would like to see them as per Spec/TRM, so
> > doesn't this patch make sense?
>
> The spec doesn't have a base address for the module? This name is going
> to get used in sysfs anyway, and the old name should be going away.
>
This is certainly going to change user interface change, and I thought we
should not change the user interface irrespective of whether it is DT or
non-DT.
Few points, which I do care about, are,
1. request_irq():
For the driver if I am using dev_name(dev) as for the 3rd argument, which
will now different with DT support - brings change in user interface.
Non-DT:
#cat /proc/interrupts
CPU0
52: 0 INTC xxx0
55: 0 INTC xxx1
DT:
#cat /proc/interrupts
CPU0
52: 0 INTC 481cc000.xxx
55: 0 INTC 481d0000.xxx
2. SYSFS:
For example,
Without auxdata:
/sys/devices/481cc000.xxx/
/sys/devices/481d0000.xxx/
With auxdata:
/sys/devices/xxx.0/
/sys/devices/xxx.1/
From user perspective, I still would not want to change the existing
naming convention.
3. Certainly I don't want user to go back to spec everytime to find out
which address is mapped to which instance of module.
Also, in addition to that, in case of SoC's like AM33xx (for that matter,
any embedded SoC) we have different domains and modules are placed in
different domains as per use-cases.
For example, in this case, can0 is places in wakeup domain and can1 is
placed in per-domain. Some boards might use only one instance or some
board might use both of them.
I still would like to keep instance number wherever user interface comes
into picture and populate base address as an additional info to user.
I may be missing something, may be I am not able to see the bigger picture
here.
Grant/Benoit, May be you can conform and put your opinion on this.
Thanks,
Vaibhav
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
@ 2012-08-08 11:43 ` Hiremath, Vaibhav
0 siblings, 0 replies; 14+ messages in thread
From: Hiremath, Vaibhav @ 2012-08-08 11:43 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Aug 08, 2012 at 07:38:01, Rob Herring wrote:
> On 08/07/2012 10:53 AM, Hiremath, Vaibhav wrote:
> > On Tue, Aug 07, 2012 at 20:49:48, Rob Herring wrote:
> >> On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
> >>> If the module requires only one clocksource to function, then
> >>> driver can very well call clk_get() api with "con_id = NULL",
> >>>
> >>> clk = clk_get(dev, NULL);
> >>>
> >>> And platform code should respect this and return valid clk handle.
> >>> That means, now the clk_get() api would rely on dev_id, and platform
> >>> code should either have clk node with matching dev_id (with con_id=NULL)
> >>> or return dummy clk node.
> >>>
> >>> With DT support, we can fix the dev_id for particular module
> >>> using "struct of_dev_auxdata" to satisfy above clk framework requirement.
> >>>
> >>> In case of AM33XX, we required this for the DCAN module, so this
> >>> patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
> >>>
> >>> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
> >>> Cc: Tony Lindgren <tony@atomide.com>
> >>> Cc: Paul Walmsley <paul@pwsan.com>
> >>> Cc: Benoit Cousson <b-cousson@ti.com>
> >>> Cc: Grant Likely <grant.likely@secretlab.ca>
> >>> ---
> >>> This patch is boot tested on BeagleBone platform, and checked for
> >>> clk_get() return value in d_can module driver.
> >>>
> >>> arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
> >>> 1 files changed, 17 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
> >>> index 6f93a20..c9b7903 100644
> >>> --- a/arch/arm/mach-omap2/board-generic.c
> >>> +++ b/arch/arm/mach-omap2/board-generic.c
> >>> @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
> >>> { }
> >>> };
> >>>
> >>> +/*
> >>> + * Lookup table for attaching a specific name and platform_data pointer to
> >>> + * devices as they get created by of_platform_populate(). Ideally this table
> >>> + * would not exist, but the current clock implementation depends on some devices
> >>> + * having a specific name OR to satisfy NULL con_id requirement from driver.
> >>> + */
> >>> +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
> >>> + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
> >>> + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
> >>> + { },
> >>> +};
> >>
> >> Adding an additional clkdev lookup accomplishes the same thing and is a
> >> cleaner solution.
> >>
> >
> > That is also required and I have submitted patch for the same -
> >
> > http://www.spinics.net/lists/arm-kernel/msg187998.html
>
> That only adds the non-DT name. What I'm saying is you can have 2 lookup
> names for the same clock.
>
> >
> > With DT support, I am getting dev_id as -
> >
> > - Without "reg" property: d_can.16 and d_can.17,
> > (I believe it changes dynamically here)
> >
> > - With "reg" property: 481cc000.d_can and 481d0000.d_can
> >
> > Which is not so intuitive, I would like to see them as per Spec/TRM, so
> > doesn't this patch make sense?
>
> The spec doesn't have a base address for the module? This name is going
> to get used in sysfs anyway, and the old name should be going away.
>
This is certainly going to change user interface change, and I thought we
should not change the user interface irrespective of whether it is DT or
non-DT.
Few points, which I do care about, are,
1. request_irq():
For the driver if I am using dev_name(dev) as for the 3rd argument, which
will now different with DT support - brings change in user interface.
Non-DT:
#cat /proc/interrupts
CPU0
52: 0 INTC xxx0
55: 0 INTC xxx1
DT:
#cat /proc/interrupts
CPU0
52: 0 INTC 481cc000.xxx
55: 0 INTC 481d0000.xxx
2. SYSFS:
For example,
Without auxdata:
/sys/devices/481cc000.xxx/
/sys/devices/481d0000.xxx/
With auxdata:
/sys/devices/xxx.0/
/sys/devices/xxx.1/
From user perspective, I still would not want to change the existing
naming convention.
3. Certainly I don't want user to go back to spec everytime to find out
which address is mapped to which instance of module.
Also, in addition to that, in case of SoC's like AM33xx (for that matter,
any embedded SoC) we have different domains and modules are placed in
different domains as per use-cases.
For example, in this case, can0 is places in wakeup domain and can1 is
placed in per-domain. Some boards might use only one instance or some
board might use both of them.
I still would like to keep instance number wherever user interface comes
into picture and populate base address as an additional info to user.
I may be missing something, may be I am not able to see the bigger picture
here.
Grant/Benoit, May be you can conform and put your opinion on this.
Thanks,
Vaibhav
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
2012-08-08 11:43 ` Hiremath, Vaibhav
@ 2012-08-15 9:21 ` Vaibhav Hiremath
-1 siblings, 0 replies; 14+ messages in thread
From: Vaibhav Hiremath @ 2012-08-15 9:21 UTC (permalink / raw)
To: Rob Herring
Cc: linux-omap, Paul Walmsley, Cousson, Benoit, Tony Lindgren,
Grant Likely, linux-arm-kernel
On 8/8/2012 5:13 PM, Hiremath, Vaibhav wrote:
> On Wed, Aug 08, 2012 at 07:38:01, Rob Herring wrote:
>> On 08/07/2012 10:53 AM, Hiremath, Vaibhav wrote:
>>> On Tue, Aug 07, 2012 at 20:49:48, Rob Herring wrote:
>>>> On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
>>>>> If the module requires only one clocksource to function, then
>>>>> driver can very well call clk_get() api with "con_id = NULL",
>>>>>
>>>>> clk = clk_get(dev, NULL);
>>>>>
>>>>> And platform code should respect this and return valid clk handle.
>>>>> That means, now the clk_get() api would rely on dev_id, and platform
>>>>> code should either have clk node with matching dev_id (with con_id=NULL)
>>>>> or return dummy clk node.
>>>>>
>>>>> With DT support, we can fix the dev_id for particular module
>>>>> using "struct of_dev_auxdata" to satisfy above clk framework requirement.
>>>>>
>>>>> In case of AM33XX, we required this for the DCAN module, so this
>>>>> patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
>>>>>
>>>>> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
>>>>> Cc: Tony Lindgren <tony@atomide.com>
>>>>> Cc: Paul Walmsley <paul@pwsan.com>
>>>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>>>> ---
>>>>> This patch is boot tested on BeagleBone platform, and checked for
>>>>> clk_get() return value in d_can module driver.
>>>>>
>>>>> arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
>>>>> 1 files changed, 17 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
>>>>> index 6f93a20..c9b7903 100644
>>>>> --- a/arch/arm/mach-omap2/board-generic.c
>>>>> +++ b/arch/arm/mach-omap2/board-generic.c
>>>>> @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
>>>>> { }
>>>>> };
>>>>>
>>>>> +/*
>>>>> + * Lookup table for attaching a specific name and platform_data pointer to
>>>>> + * devices as they get created by of_platform_populate(). Ideally this table
>>>>> + * would not exist, but the current clock implementation depends on some devices
>>>>> + * having a specific name OR to satisfy NULL con_id requirement from driver.
>>>>> + */
>>>>> +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
>>>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
>>>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
>>>>> + { },
>>>>> +};
>>>>
>>>> Adding an additional clkdev lookup accomplishes the same thing and is a
>>>> cleaner solution.
>>>>
>>>
>>> That is also required and I have submitted patch for the same -
>>>
>>> http://www.spinics.net/lists/arm-kernel/msg187998.html
>>
>> That only adds the non-DT name. What I'm saying is you can have 2 lookup
>> names for the same clock.
>>
>>>
>>> With DT support, I am getting dev_id as -
>>>
>>> - Without "reg" property: d_can.16 and d_can.17,
>>> (I believe it changes dynamically here)
>>>
>>> - With "reg" property: 481cc000.d_can and 481d0000.d_can
>>>
>>> Which is not so intuitive, I would like to see them as per Spec/TRM, so
>>> doesn't this patch make sense?
>>
>> The spec doesn't have a base address for the module? This name is going
>> to get used in sysfs anyway, and the old name should be going away.
>>
>
> This is certainly going to change user interface change, and I thought we
> should not change the user interface irrespective of whether it is DT or
> non-DT.
> Few points, which I do care about, are,
>
> 1. request_irq():
> For the driver if I am using dev_name(dev) as for the 3rd argument, which
> will now different with DT support - brings change in user interface.
>
> Non-DT:
> #cat /proc/interrupts
> CPU0
> 52: 0 INTC xxx0
> 55: 0 INTC xxx1
>
> DT:
> #cat /proc/interrupts
> CPU0
> 52: 0 INTC 481cc000.xxx
> 55: 0 INTC 481d0000.xxx
>
>
> 2. SYSFS:
> For example,
> Without auxdata:
> /sys/devices/481cc000.xxx/
> /sys/devices/481d0000.xxx/
>
> With auxdata:
> /sys/devices/xxx.0/
> /sys/devices/xxx.1/
>
> From user perspective, I still would not want to change the existing
> naming convention.
>
> 3. Certainly I don't want user to go back to spec everytime to find out
> which address is mapped to which instance of module.
> Also, in addition to that, in case of SoC's like AM33xx (for that matter,
> any embedded SoC) we have different domains and modules are placed in
> different domains as per use-cases.
> For example, in this case, can0 is places in wakeup domain and can1 is
> placed in per-domain. Some boards might use only one instance or some
> board might use both of them.
>
> I still would like to keep instance number wherever user interface comes
> into picture and populate base address as an additional info to user.
>
>
> I may be missing something, may be I am not able to see the bigger picture
> here.
>
> Grant/Benoit, May be you can conform and put your opinion on this.
>
Rob,
I think we do not have any conclusion on this, so here should I assume
that I should just simply add another clk node entry in clock-tree, like
below -
CLK("481cc000.d_can", NULL, &dcan0_fck, CK_AM33XX),
CLK("481d0000.d_can", NULL, &dcan1_fck, CK_AM33XX),
Thanks,
Vaibhav
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module
@ 2012-08-15 9:21 ` Vaibhav Hiremath
0 siblings, 0 replies; 14+ messages in thread
From: Vaibhav Hiremath @ 2012-08-15 9:21 UTC (permalink / raw)
To: linux-arm-kernel
On 8/8/2012 5:13 PM, Hiremath, Vaibhav wrote:
> On Wed, Aug 08, 2012 at 07:38:01, Rob Herring wrote:
>> On 08/07/2012 10:53 AM, Hiremath, Vaibhav wrote:
>>> On Tue, Aug 07, 2012 at 20:49:48, Rob Herring wrote:
>>>> On 08/07/2012 08:37 AM, Vaibhav Hiremath wrote:
>>>>> If the module requires only one clocksource to function, then
>>>>> driver can very well call clk_get() api with "con_id = NULL",
>>>>>
>>>>> clk = clk_get(dev, NULL);
>>>>>
>>>>> And platform code should respect this and return valid clk handle.
>>>>> That means, now the clk_get() api would rely on dev_id, and platform
>>>>> code should either have clk node with matching dev_id (with con_id=NULL)
>>>>> or return dummy clk node.
>>>>>
>>>>> With DT support, we can fix the dev_id for particular module
>>>>> using "struct of_dev_auxdata" to satisfy above clk framework requirement.
>>>>>
>>>>> In case of AM33XX, we required this for the DCAN module, so this
>>>>> patch adds "of_dev_auxdata" for both DCAN_0/1 instances.
>>>>>
>>>>> Signed-off-by: Vaibhav Hiremath <hvaibhav@ti.com>
>>>>> Cc: Tony Lindgren <tony@atomide.com>
>>>>> Cc: Paul Walmsley <paul@pwsan.com>
>>>>> Cc: Benoit Cousson <b-cousson@ti.com>
>>>>> Cc: Grant Likely <grant.likely@secretlab.ca>
>>>>> ---
>>>>> This patch is boot tested on BeagleBone platform, and checked for
>>>>> clk_get() return value in d_can module driver.
>>>>>
>>>>> arch/arm/mach-omap2/board-generic.c | 18 +++++++++++++++++-
>>>>> 1 files changed, 17 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/mach-omap2/board-generic.c b/arch/arm/mach-omap2/board-generic.c
>>>>> index 6f93a20..c9b7903 100644
>>>>> --- a/arch/arm/mach-omap2/board-generic.c
>>>>> +++ b/arch/arm/mach-omap2/board-generic.c
>>>>> @@ -37,11 +37,27 @@ static struct of_device_id omap_dt_match_table[] __initdata = {
>>>>> { }
>>>>> };
>>>>>
>>>>> +/*
>>>>> + * Lookup table for attaching a specific name and platform_data pointer to
>>>>> + * devices as they get created by of_platform_populate(). Ideally this table
>>>>> + * would not exist, but the current clock implementation depends on some devices
>>>>> + * having a specific name OR to satisfy NULL con_id requirement from driver.
>>>>> + */
>>>>> +static const struct of_dev_auxdata am33xx_auxdata_lookup[] __initconst = {
>>>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481cc000, "d_can.0", NULL),
>>>>> + OF_DEV_AUXDATA("bosch,d_can", 0x481d0000, "d_can.1", NULL),
>>>>> + { },
>>>>> +};
>>>>
>>>> Adding an additional clkdev lookup accomplishes the same thing and is a
>>>> cleaner solution.
>>>>
>>>
>>> That is also required and I have submitted patch for the same -
>>>
>>> http://www.spinics.net/lists/arm-kernel/msg187998.html
>>
>> That only adds the non-DT name. What I'm saying is you can have 2 lookup
>> names for the same clock.
>>
>>>
>>> With DT support, I am getting dev_id as -
>>>
>>> - Without "reg" property: d_can.16 and d_can.17,
>>> (I believe it changes dynamically here)
>>>
>>> - With "reg" property: 481cc000.d_can and 481d0000.d_can
>>>
>>> Which is not so intuitive, I would like to see them as per Spec/TRM, so
>>> doesn't this patch make sense?
>>
>> The spec doesn't have a base address for the module? This name is going
>> to get used in sysfs anyway, and the old name should be going away.
>>
>
> This is certainly going to change user interface change, and I thought we
> should not change the user interface irrespective of whether it is DT or
> non-DT.
> Few points, which I do care about, are,
>
> 1. request_irq():
> For the driver if I am using dev_name(dev) as for the 3rd argument, which
> will now different with DT support - brings change in user interface.
>
> Non-DT:
> #cat /proc/interrupts
> CPU0
> 52: 0 INTC xxx0
> 55: 0 INTC xxx1
>
> DT:
> #cat /proc/interrupts
> CPU0
> 52: 0 INTC 481cc000.xxx
> 55: 0 INTC 481d0000.xxx
>
>
> 2. SYSFS:
> For example,
> Without auxdata:
> /sys/devices/481cc000.xxx/
> /sys/devices/481d0000.xxx/
>
> With auxdata:
> /sys/devices/xxx.0/
> /sys/devices/xxx.1/
>
> From user perspective, I still would not want to change the existing
> naming convention.
>
> 3. Certainly I don't want user to go back to spec everytime to find out
> which address is mapped to which instance of module.
> Also, in addition to that, in case of SoC's like AM33xx (for that matter,
> any embedded SoC) we have different domains and modules are placed in
> different domains as per use-cases.
> For example, in this case, can0 is places in wakeup domain and can1 is
> placed in per-domain. Some boards might use only one instance or some
> board might use both of them.
>
> I still would like to keep instance number wherever user interface comes
> into picture and populate base address as an additional info to user.
>
>
> I may be missing something, may be I am not able to see the bigger picture
> here.
>
> Grant/Benoit, May be you can conform and put your opinion on this.
>
Rob,
I think we do not have any conclusion on this, so here should I assume
that I should just simply add another clk node entry in clock-tree, like
below -
CLK("481cc000.d_can", NULL, &dcan0_fck, CK_AM33XX),
CLK("481d0000.d_can", NULL, &dcan1_fck, CK_AM33XX),
Thanks,
Vaibhav
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-08-15 9:21 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-07 13:37 [PATCH] ARM: AM33XX: board-generic: Add of_dev_auxdata to fix dev_id for CAN module Vaibhav Hiremath
2012-08-07 13:37 ` Vaibhav Hiremath
2012-08-07 15:19 ` Rob Herring
2012-08-07 15:19 ` Rob Herring
2012-08-07 15:53 ` Hiremath, Vaibhav
2012-08-07 15:53 ` Hiremath, Vaibhav
2012-08-07 17:42 ` Koen Kooi
2012-08-07 17:42 ` Koen Kooi
2012-08-08 2:08 ` Rob Herring
2012-08-08 2:08 ` Rob Herring
2012-08-08 11:43 ` Hiremath, Vaibhav
2012-08-08 11:43 ` Hiremath, Vaibhav
2012-08-15 9:21 ` Vaibhav Hiremath
2012-08-15 9:21 ` Vaibhav Hiremath
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.