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

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

[Danny Huang]

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

Signed-off-by: Danny Huang <dahuang@nvidia.com>
---
 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

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

[Russell King - ARM Linux]

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.

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

[Arnd Bergmann]

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

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

[Stephen Warren]

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.

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

[Stephen Warren]

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?

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

[Stephen Warren]

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?

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

[Arnd Bergmann]

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

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

[Stephen Warren]

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().