2013-03-12 12:01:49

by Danny Huang

[permalink] [raw]
Subject: [PATCH v2] ARM: tegra: expose chip ID and revision

Expose Tegra chip ID and revision in /sys/devices/soc for user mode
usage

Signed-off-by: Danny Huang <[email protected]>
---
arch/arm/mach-tegra/Kconfig | 3 +++
arch/arm/mach-tegra/common.c | 23 +++++++++++++++++++++++
2 files changed, 26 insertions(+)

diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index d1c4893..b7abcfa 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -17,6 +17,7 @@ config ARCH_TEGRA_2x_SOC
select PINCTRL_TEGRA20
select PL310_ERRATA_727915 if CACHE_L2X0
select PL310_ERRATA_769419 if CACHE_L2X0
+ select SOC_BUS
select USB_ARCH_HAS_EHCI if USB_SUPPORT
select USB_ULPI if USB
select USB_ULPI_VIEWPORT if USB_SUPPORT
@@ -36,6 +37,7 @@ config ARCH_TEGRA_3x_SOC
select PINCTRL
select PINCTRL_TEGRA30
select PL310_ERRATA_769419 if CACHE_L2X0
+ select SOC_BUS
select USB_ARCH_HAS_EHCI if USB_SUPPORT
select USB_ULPI if USB
select USB_ULPI_VIEWPORT if USB_SUPPORT
@@ -51,6 +53,7 @@ config ARCH_TEGRA_114_SOC
select CPU_V7
select PINCTRL
select PINCTRL_TEGRA114
+ select SOC_BUS
help
Support for NVIDIA Tegra T114 processor family, based on the
ARM CortexA15MP CPU
diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index f0315c9..9547c59 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -23,6 +23,8 @@
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/irqchip.h>
+#include <linux/slab.h>
+#include <linux/sys_soc.h>
#include <linux/clk/tegra.h>

#include <asm/hardware/cache-l2x0.h>
@@ -94,6 +96,26 @@ static void __init tegra_init_cache(void)

}

+void __init tegra_soc_device_init(void)
+{
+ struct soc_device *soc_dev;
+ struct soc_device_attribute *soc_dev_attr;
+
+ soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
+ if (!soc_dev_attr)
+ return;
+
+ soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%d", tegra_chip_id);
+ soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d", tegra_revision);
+ soc_dev_attr->family = kasprintf(GFP_KERNEL, "Tegra");
+
+ soc_dev = soc_device_register(soc_dev_attr);
+ if (IS_ERR_OR_NULL(soc_dev))
+ kfree(soc_dev_attr);
+
+ return;
+}
+
void __init tegra_init_early(void)
{
tegra_cpu_reset_handler_init();
@@ -108,4 +130,5 @@ void __init tegra_init_early(void)
void __init tegra_init_late(void)
{
tegra_powergate_debugfs_init();
+ tegra_soc_device_init();
}
--
1.8.1.5


2013-03-12 12:19:50

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: expose chip ID and revision

On Tue, Mar 12, 2013 at 08:01:07PM +0800, Danny Huang wrote:
> + if (IS_ERR_OR_NULL(soc_dev))

Sigh. NAK. Fur cryin out loud! Use the IS_ERR() version.

And you don't need the following "return;" statement - that's already
implied at the closing brace of a function.

2013-03-12 12:59:26

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: expose chip ID and revision

On Tuesday 12 March 2013, Danny Huang wrote:
>
> +void __init tegra_soc_device_init(void)
> +{
> + struct soc_device *soc_dev;
> + struct soc_device_attribute *soc_dev_attr;
> +
> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
> + if (!soc_dev_attr)
> + return;
> +
> + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%d", tegra_chip_id);
> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d", tegra_revision);
> + soc_dev_attr->family = kasprintf(GFP_KERNEL, "Tegra");
> +
> + soc_dev = soc_device_register(soc_dev_attr);
> + if (IS_ERR_OR_NULL(soc_dev))
> + kfree(soc_dev_attr);
> +
> + return;

You are dropping the soc_dev on the floor here by just returning.

The idea of the soc node is to have all on-soc components be children
of that node, so you should instead pass it into of_platform_populate
as the parent device.

Arnd

2013-03-12 17:59:02

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: expose chip ID and revision

On 03/12/2013 06:01 AM, Danny Huang wrote:
> Expose Tegra chip ID and revision in /sys/devices/soc for user mode
> usage

> diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
> index d1c4893..b7abcfa 100644
> --- a/arch/arm/mach-tegra/Kconfig
> +++ b/arch/arm/mach-tegra/Kconfig
> @@ -17,6 +17,7 @@ config ARCH_TEGRA_2x_SOC
> select PINCTRL_TEGRA20
> select PL310_ERRATA_727915 if CACHE_L2X0
> select PL310_ERRATA_769419 if CACHE_L2X0
> + select SOC_BUS
> select USB_ARCH_HAS_EHCI if USB_SUPPORT
> select USB_ULPI if USB
> select USB_ULPI_VIEWPORT if USB_SUPPORT
> @@ -36,6 +37,7 @@ config ARCH_TEGRA_3x_SOC
> select PINCTRL
> select PINCTRL_TEGRA30
> select PL310_ERRATA_769419 if CACHE_L2X0
> + select SOC_BUS
> select USB_ARCH_HAS_EHCI if USB_SUPPORT
> select USB_ULPI if USB
> select USB_ULPI_VIEWPORT if USB_SUPPORT
> @@ -51,6 +53,7 @@ config ARCH_TEGRA_114_SOC
> select CPU_V7
> select PINCTRL
> select PINCTRL_TEGRA114
> + select SOC_BUS
> help

I assume we'll want this feature for all Tegra SoCs. Can't we select it
from ARCH_TEGRA, rather than separately for each chip? CONFIG_ARCH_TEGRA
is defined in arch/arm/Kconfig.

2013-03-12 18:01:31

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: expose chip ID and revision

On 03/12/2013 06:01 AM, Danny Huang wrote:
> Expose Tegra chip ID and revision in /sys/devices/soc for user mode
> usage

> diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c

> +void __init tegra_soc_device_init(void)

Oh, and since this is exposing data that's initialized by Tegra's
fuse.c, wouldn't it make sense to put this function into fuse.c at the
end of its initialization function?

2013-03-12 18:04:35

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: expose chip ID and revision

On 03/12/2013 06:59 AM, Arnd Bergmann wrote:
> On Tuesday 12 March 2013, Danny Huang wrote:
>>
>> +void __init tegra_soc_device_init(void)
>> +{
>> + struct soc_device *soc_dev;
>> + struct soc_device_attribute *soc_dev_attr;
>> +
>> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
>> + if (!soc_dev_attr)
>> + return;
>> +
>> + soc_dev_attr->soc_id = kasprintf(GFP_KERNEL, "%d", tegra_chip_id);
>> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%d", tegra_revision);
>> + soc_dev_attr->family = kasprintf(GFP_KERNEL, "Tegra");
>> +
>> + soc_dev = soc_device_register(soc_dev_attr);
>> + if (IS_ERR_OR_NULL(soc_dev))
>> + kfree(soc_dev_attr);
>> +
>> + return;
>
> You are dropping the soc_dev on the floor here by just returning.
>
> The idea of the soc node is to have all on-soc components be children
> of that node, so you should instead pass it into of_platform_populate
> as the parent device.

Tegra DTs don't have a separate node for on-soc vs. off-soc components.
Wouldn't passing soc_dev into of_platform_populate() make everything a
child of this soc_dev; is that what we want?

2013-03-12 19:11:11

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: expose chip ID and revision

On Tuesday 12 March 2013, Stephen Warren wrote:
> > You are dropping the soc_dev on the floor here by just returning.
> >
> > The idea of the soc node is to have all on-soc components be children
> > of that node, so you should instead pass it into of_platform_populate
> > as the parent device.
>
> Tegra DTs don't have a separate node for on-soc vs. off-soc components.
> Wouldn't passing soc_dev into of_platform_populate() make everything a
> child of this soc_dev; is that what we want?

Yes, we had long discussions about this when the soc infrastructure was
merged. Right now, everything is a child of /sys/devices/platform/,
basically saying that all devices are random stuff that cannot be
probed. Moving it to /sys/devices/soc0 would not make the hierarchy
any deeper but show much clearer which devices are part of the
soc, and which ones are added as anonymous platform devices by code
that does not use DT based probing. Ideally the second category is
empty.

Arnd

2013-03-12 19:45:21

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v2] ARM: tegra: expose chip ID and revision

On 03/12/2013 01:10 PM, Arnd Bergmann wrote:
> On Tuesday 12 March 2013, Stephen Warren wrote:
>>> You are dropping the soc_dev on the floor here by just returning.
>>>
>>> The idea of the soc node is to have all on-soc components be children
>>> of that node, so you should instead pass it into of_platform_populate
>>> as the parent device.
>>
>> Tegra DTs don't have a separate node for on-soc vs. off-soc components.
>> Wouldn't passing soc_dev into of_platform_populate() make everything a
>> child of this soc_dev; is that what we want?
>
> Yes, we had long discussions about this when the soc infrastructure was
> merged. Right now, everything is a child of /sys/devices/platform/,
> basically saying that all devices are random stuff that cannot be
> probed. Moving it to /sys/devices/soc0 would not make the hierarchy
> any deeper but show much clearer which devices are part of the
> soc, and which ones are added as anonymous platform devices by code
> that does not use DT based probing. Ideally the second category is
> empty.

OK, that makes sense.

Danny, in that case, the initialization of this SoC object should
definitely happen inside tegra_init_fuse(), which is called from
tegra_init_early(), so that mach-tegra/tegra.c:tegra_dt_init() can call
a function in fuse.c to retrieve that SoC object in order to pass it
into of_platform_populate().