diff mbox

[2/3] mach-ux500: export System-on-Chip information via sysfs

Message ID 1310476090-9807-2-git-send-email-lee.jones@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Lee Jones July 12, 2011, 1:08 p.m. UTC
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/mach-ux500/Kconfig |    1 +
 arch/arm/mach-ux500/id.c    |  115 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+), 0 deletions(-)

Comments

Saravana Kannan July 12, 2011, 4:29 p.m. UTC | #1
On 07/12/2011 06:08 AM, Lee Jones wrote:
> Signed-off-by: Lee Jones<lee.jones@linaro.org>
> ---
>   arch/arm/mach-ux500/Kconfig |    1 +
>   arch/arm/mach-ux500/id.c    |  115 +++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 116 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-ux500/Kconfig b/arch/arm/mach-ux500/Kconfig
> index 4210cb4..4d2f2c2 100644

<snip>

> +
> +struct soc_callback_functions soc_callbacks = {
> +	.get_machine_fn  = ux500_get_machine,
> +	.get_family_fn   = ux500_get_family,
> +	.get_soc_id_fn   = ux500_get_soc_id,
> +	.get_revision_fn = ux500_get_revision,
> +};
> +
> +struct device_attribute ux500_soc_attrs[] = {
> +	__ATTR(process,  S_IRUGO, ux500_get_process,  NULL),
> +	__ATTR_NULL,
> +};
> +
> +static int __init ux500_soc_sysfs_init(void)
> +{
> +	int ret;
> +	int i = 0;
> +	ret = soc_device_register(&soc_parent,
> +				&soc_callbacks);
> +	if (ret>= 0) {
> +		while (ux500_soc_attrs[i].attr.name != NULL) {
> +			ret = device_create_file(&soc_parent,
> +						&ux500_soc_attrs[i++]);
> +			if (ret)
> +				goto out;
> +		}
> +	}

Can you please make this code as part of soc_device_register? Otherwise, 
every SoC that wants to add its own set of attributes will have to 
repeat this code.

> + out:
> +	return ret;
> +}
> +module_init(ux500_soc_sysfs_init);
> +
> +static void __exit ux500_soc_sysfs_exit(void)
> +{
> +	int i = 0;
> +
> +	while (ux500_soc_attrs[i].attr.name != NULL)
> +		device_remove_file(&soc_parent,&ux500_soc_attrs[i++]);

That would also pull this in and make sure the ordering is always correct.

> +
> +	soc_device_unregister(&soc_parent);
> +}
> +module_exit(ux500_soc_sysfs_exit);
> +
> +#endif

Thanks,
Saravana
Arnd Bergmann July 12, 2011, 4:47 p.m. UTC | #2
On Tuesday 12 July 2011, Lee Jones wrote:
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

Just like last time:

   NAK

You still have all the SoC devices as top-level platform devices.
Make them children of the SoC device you create here, and we can
apply this patch.

	Arnd
Lee Jones July 13, 2011, 7:55 a.m. UTC | #3
On 12/07/11 17:29, Saravana Kannan wrote:
>> +struct soc_callback_functions soc_callbacks = {
>> +    .get_machine_fn  = ux500_get_machine,
>> +    .get_family_fn   = ux500_get_family,
>> +    .get_soc_id_fn   = ux500_get_soc_id,
>> +    .get_revision_fn = ux500_get_revision,
>> +};
>> +
>> +struct device_attribute ux500_soc_attrs[] = {
>> +    __ATTR(process,  S_IRUGO, ux500_get_process,  NULL),
>> +    __ATTR_NULL,
>> +};
>> +
>> +static int __init ux500_soc_sysfs_init(void)
>> +{
>> +    int ret;
>> +    int i = 0;
>> +    ret = soc_device_register(&soc_parent,
>> +                &soc_callbacks);
>> +    if (ret>= 0) {
>> +        while (ux500_soc_attrs[i].attr.name != NULL) {
>> +            ret = device_create_file(&soc_parent,
>> +                        &ux500_soc_attrs[i++]);
>> +            if (ret)
>> +                goto out;
>> +        }
>> +    }
> 
> Can you please make this code as part of soc_device_register? Otherwise,
> every SoC that wants to add its own set of attributes will have to
> repeat this code.

That's the idea.

I initially had it all as part of soc_device_register, but Arnd told me
to remove it in this patch-set.

See here:

On 17/04/11 19:36, Arnd Bergmann wrote:
> For the nonstandard attributes, I would recommend having the individual
> drivers call device_create_file, in order to discourage the use of 
> device specific attribute names.

Hence all of these are the _standard_ attributes:

>> +    .get_machine_fn  = ux500_get_machine,
>> +    .get_family_fn   = ux500_get_family,
>> +    .get_soc_id_fn   = ux500_get_soc_id,
>> +    .get_revision_fn = ux500_get_revision,

And this is the _non standard_ one:
>> +    __ATTR(process,  S_IRUGO, ux500_get_process,  NULL),

Which is subsequently added by the platform.

Kind regards,
Lee
Lee Jones July 13, 2011, 8:10 a.m. UTC | #4
On 12/07/11 17:47, Arnd Bergmann wrote:
> On Tuesday 12 July 2011, Lee Jones wrote:
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> 
> Just like last time:
> 
>    NAK
> 
> You still have all the SoC devices as top-level platform devices.
> Make them children of the SoC device you create here, and we can
> apply this patch.

Okay, obviously I'm missing something. I am trying to adhere to your
advice, but there must be some communication break-down somewhere down
the line. Let me attempt to explain what's going on in my head.

You keep asking for the SoC devices to be children of the parent SoC
device and as far as I'm concerned they are. Devices appear like this in
sysfs:

/sys/devices/soc/1   /* 1st SoC */
/sys/devices/soc/2   /* 2nd SoC */
/sys/devices/soc/3   /* 3rd SoC */

etc ...

Surely the parent which you speak of is "soc" and each SoC is
represented by "1|2|3|..."? Under each directory with a digit naming
convention appears that SoC's attributes, namely: "family", "machine",
"process", "revision" and "soc_id".

If this is incorrect, would you be kind enough to tell me why its
incorrect and how you would like it changed/fixed please?

Kind regards,
Lee
Arnd Bergmann July 13, 2011, 1:42 p.m. UTC | #5
On Wednesday 13 July 2011, Lee Jones wrote:
> Okay, obviously I'm missing something. I am trying to adhere to your
> advice, but there must be some communication break-down somewhere down
> the line. Let me attempt to explain what's going on in my head.
> 
> You keep asking for the SoC devices to be children of the parent SoC
> device and as far as I'm concerned they are. Devices appear like this in
> sysfs:
> 
> /sys/devices/soc/1   /* 1st SoC */
> /sys/devices/soc/2   /* 2nd SoC */
> /sys/devices/soc/3   /* 3rd SoC */
> 
> etc ...
> 
> Surely the parent which you speak of is "soc" and each SoC is
> represented by "1|2|3|..."? Under each directory with a digit naming
> convention appears that SoC's attributes, namely: "family", "machine",
> "process", "revision" and "soc_id".
> 
> If this is incorrect, would you be kind enough to tell me why its
> incorrect and how you would like it changed/fixed please?
> 

I'm not talking of the root soc devices but the platform_devices that
are part of the soc, i.e. interrupt controller, i2c bus, mmc host,
etc.

Basically all the devices that are currently statically declared
in arch/arm/mach-ux500/board-mop500.c and are missing a parent pointer.

What I'm asking you to do is fix those device declarations to have
their .dev.parent pointer point to the correct soc device. If there
are nested buses, they should ideally also be represented as devices
so you end up with something like

/sys/devices/soc/1/amba/uart1
                       /uart2
                       /i2c/bus1
                           /bus2/tc35892
                                /lp5521
                       /spi/foo
                           /bar
                  /dma
                /2/amba/...

Basically, if you have a soc node, I would expect /sys/devices/platform to
become empty, and all devices properly parented to where they are actually
connected.

	Arnd
Lee Jones July 13, 2011, 2:31 p.m. UTC | #6
On 13/07/11 14:42, Arnd Bergmann wrote:
> On Wednesday 13 July 2011, Lee Jones wrote:
>> Okay, obviously I'm missing something. I am trying to adhere to your
>> advice, but there must be some communication break-down somewhere down
>> the line. Let me attempt to explain what's going on in my head.
>>
>> You keep asking for the SoC devices to be children of the parent SoC
>> device and as far as I'm concerned they are. Devices appear like this in
>> sysfs:
>>
>> /sys/devices/soc/1   /* 1st SoC */
>> /sys/devices/soc/2   /* 2nd SoC */
>> /sys/devices/soc/3   /* 3rd SoC */
>>
>> etc ...
>>
>> Surely the parent which you speak of is "soc" and each SoC is
>> represented by "1|2|3|..."? Under each directory with a digit naming
>> convention appears that SoC's attributes, namely: "family", "machine",
>> "process", "revision" and "soc_id".
>>
>> If this is incorrect, would you be kind enough to tell me why its
>> incorrect and how you would like it changed/fixed please?
>>
> 
> I'm not talking of the root soc devices but the platform_devices that
> are part of the soc, i.e. interrupt controller, i2c bus, mmc host,
> etc.
> 
> Basically all the devices that are currently statically declared
> in arch/arm/mach-ux500/board-mop500.c and are missing a parent pointer.
> 
> What I'm asking you to do is fix those device declarations to have
> their .dev.parent pointer point to the correct soc device. If there
> are nested buses, they should ideally also be represented as devices
> so you end up with something like
> 
> /sys/devices/soc/1/amba/uart1
>                        /uart2
>                        /i2c/bus1
>                            /bus2/tc35892
>                                 /lp5521
>                        /spi/foo
>                            /bar
>                   /dma
>                 /2/amba/...
> 
> Basically, if you have a soc node, I would expect /sys/devices/platform to
> become empty, and all devices properly parented to where they are actually
> connected.


Ahhhh, well why didn't you say so? I've been racking my brains about
this for ages.:)

Does this have to be coded and submitted with this patch-set? Is there
any way we can accept this one and I'll take this reorganisation of
platform drivers as an action to be completed when I have some more
spare cycles?
Arnd Bergmann July 13, 2011, 4:20 p.m. UTC | #7
On Wednesday 13 July 2011, Lee Jones wrote:
> Does this have to be coded and submitted with this patch-set? Is there
> any way we can accept this one and I'll take this reorganisation of
> platform drivers as an action to be completed when I have some more
> spare cycles?

I prefer to remain strict on this one. I don't mind the first
patch getting in now, but I wouldn't want to set a precedent
on incorrect usage of the soc nodes. Note that doing this right
will become a lot easier once the platform has been converted
into device tree probing, because that implies a hierarchical
view of the devices by nature.

	Arnd
Saravana Kannan July 13, 2011, 4:40 p.m. UTC | #8
On 07/13/2011 12:55 AM, Lee Jones wrote:
> On 12/07/11 17:29, Saravana Kannan wrote:
>> Can you please make this code as part of soc_device_register? Otherwise,
>> every SoC that wants to add its own set of attributes will have to
>> repeat this code.
>
> That's the idea.
>
> I initially had it all as part of soc_device_register, but Arnd told me
> to remove it in this patch-set.
>
> See here:
>
> On 17/04/11 19:36, Arnd Bergmann wrote:
>> For the nonstandard attributes, I would recommend having the individual
>> drivers call device_create_file, in order to discourage the use of
>> device specific attribute names.

Arnd,

I don't think forcing individual driver to call device_create_file() is 
really going to be much of a discouragement. If someone thinks they need 
to export some info, they are going to do it and end up with repeated 
code. The only real discouragement in my opinion would be code review.

Can we please pull the creation of extra attributes into the common code?

Thanks,
Saravana

>
> Hence all of these are the _standard_ attributes:
>
>>> +    .get_machine_fn  = ux500_get_machine,
>>> +    .get_family_fn   = ux500_get_family,
>>> +    .get_soc_id_fn   = ux500_get_soc_id,
>>> +    .get_revision_fn = ux500_get_revision,
>
> And this is the _non standard_ one:
>>> +    __ATTR(process,  S_IRUGO, ux500_get_process,  NULL),
>
> Which is subsequently added by the platform.
>
> Kind regards,
> Lee
>
Greg Kroah-Hartman July 13, 2011, 8:32 p.m. UTC | #9
On Wed, Jul 13, 2011 at 09:40:35AM -0700, Saravana Kannan wrote:
> On 07/13/2011 12:55 AM, Lee Jones wrote:
> >On 12/07/11 17:29, Saravana Kannan wrote:
> >>Can you please make this code as part of soc_device_register? Otherwise,
> >>every SoC that wants to add its own set of attributes will have to
> >>repeat this code.
> >
> >That's the idea.
> >
> >I initially had it all as part of soc_device_register, but Arnd told me
> >to remove it in this patch-set.
> >
> >See here:
> >
> >On 17/04/11 19:36, Arnd Bergmann wrote:
> >>For the nonstandard attributes, I would recommend having the individual
> >>drivers call device_create_file, in order to discourage the use of
> >>device specific attribute names.
> 
> Arnd,
> 
> I don't think forcing individual driver to call device_create_file()
> is really going to be much of a discouragement.

Yes it is as calling that is broken and wrong and userspace will race
with the kernel and bad things will happen on the wrong phase of the
moon.

But again, I never saw the original series to explain why...

greg k-h
Arnd Bergmann July 13, 2011, 8:51 p.m. UTC | #10
On Wednesday 13 July 2011 22:32:26 Greg KH wrote:
> On Wed, Jul 13, 2011 at 09:40:35AM -0700, Saravana Kannan wrote:
> > On 07/13/2011 12:55 AM, Lee Jones wrote:
> > >On 12/07/11 17:29, Saravana Kannan wrote:
> > >I initially had it all as part of soc_device_register, but Arnd told me
> > >to remove it in this patch-set.
> > >
> > >See here:
> > >
> > >On 17/04/11 19:36, Arnd Bergmann wrote:
> > >>For the nonstandard attributes, I would recommend having the individual
> > >>drivers call device_create_file, in order to discourage the use of
> > >>device specific attribute names.
> > 
> > Arnd,
> > 
> > I don't think forcing individual driver to call device_create_file()
> > is really going to be much of a discouragement.
> 
> Yes it is as calling that is broken and wrong and userspace will race
> with the kernel and bad things will happen on the wrong phase of the
> moon.
> 
> But again, I never saw the original series to explain why...

This is about attributes to the main device that acts as a parent for all
devices on the SoC, so there is no user space at the time.

IIRC, the original patch was documenting a set of standard attributes and
letting the SoC specific driver pass a set of static attributes in the
hope that they do whatever is documented.

In one of my review comments, I asked this to be changed to pass a data
structure with the actual contents of those attributes and to make
the implementation common to the soc independent code, in order to
reduce the amount of code needed in each soc implementation, to enforce
consistent behavior, and to make it more obvious (and painful) when an
soc implementation adds a nonstandard attribute.

It seems the final outcome was to have a data structure of function
pointers to get the attribute contents, which is less nice but still
acceptable IMHO.

	Arnd
Lee Jones July 14, 2011, 6:42 a.m. UTC | #11
> It seems the final outcome was to have a data structure of function
> pointers to get the attribute contents, which is less nice but still
> acceptable IMHO.

I did think this was a little neater than passing strings all over the
place. I'm pleased you think this patch is now acceptable. Would you
mind re-enforcing your approval with a Signed-off-by please?

I will endeavor to look at the other patch and attempt to reorganise the
Platform Drivers in sysfs sometime soon after this one has been accepted.

Kind regards,
Lee
Arnd Bergmann July 14, 2011, 12:58 p.m. UTC | #12
On Thursday 14 July 2011, Lee Jones wrote:
> > It seems the final outcome was to have a data structure of function
> > pointers to get the attribute contents, which is less nice but still
> > acceptable IMHO.
> 
> I did think this was a little neater than passing strings all over the
> place.

To give some background on why I think it is not:

The contents of the files are all static, so you can generate them
at compile time from an init function that is then discarded. When you
build a kernel for many different SoCs, you only need the pointers
at run-time, while your approach means that the kernel image will
have to keep all the functions for every SoC that is built in.

Also, in many cases, the strings themselves will be static and not
taken from a specific register.

> I'm pleased you think this patch is now acceptable. Would you
> mind re-enforcing your approval with a Signed-off-by please?

A Signed-off-by is not appropriate because I was not involved in sending
the patch but in reviewing it. I can give you a 'Reviewed-by', which
is the appropriate reply in this case.

> I will endeavor to look at the other patch and attempt to reorganise the
> Platform Drivers in sysfs sometime soon after this one has been accepted.

Ok.

	Arnd
Lee Jones July 14, 2011, 1:04 p.m. UTC | #13
On 14/07/11 13:58, Arnd Bergmann wrote:
> On Thursday 14 July 2011, Lee Jones wrote:
>>> It seems the final outcome was to have a data structure of function
>>> pointers to get the attribute contents, which is less nice but still
>>> acceptable IMHO.
>>
>> I did think this was a little neater than passing strings all over the
>> place.
> 
> To give some background on why I think it is not:
> 
> The contents of the files are all static, so you can generate them
> at compile time from an init function that is then discarded. When you
> build a kernel for many different SoCs, you only need the pointers
> at run-time, while your approach means that the kernel image will
> have to keep all the functions for every SoC that is built in.
> 
> Also, in many cases, the strings themselves will be static and not
> taken from a specific register.

I see your point.

>> I'm pleased you think this patch is now acceptable. Would you
>> mind re-enforcing your approval with a Signed-off-by please?
> 
> A Signed-off-by is not appropriate because I was not involved in sending
> the patch but in reviewing it. I can give you a 'Reviewed-by', which
> is the appropriate reply in this case.

That'll do, thanks.
Saravana Kannan July 14, 2011, 5:25 p.m. UTC | #14
On 07/13/2011 01:51 PM, Arnd Bergmann wrote:
> On Wednesday 13 July 2011 22:32:26 Greg KH wrote:
>> On Wed, Jul 13, 2011 at 09:40:35AM -0700, Saravana Kannan wrote:
>>> On 07/13/2011 12:55 AM, Lee Jones wrote:
>>>> On 12/07/11 17:29, Saravana Kannan wrote:
>>>> I initially had it all as part of soc_device_register, but Arnd told me
>>>> to remove it in this patch-set.
>>>>
>>>> See here:
>>>>
>>>> On 17/04/11 19:36, Arnd Bergmann wrote:
>>>>> For the nonstandard attributes, I would recommend having the individual
>>>>> drivers call device_create_file, in order to discourage the use of
>>>>> device specific attribute names.
>>>
>>> Arnd,
>>>
>>> I don't think forcing individual driver to call device_create_file()
>>> is really going to be much of a discouragement.
>>
>> Yes it is as calling that is broken and wrong and userspace will race
>> with the kernel and bad things will happen on the wrong phase of the
>> moon.
>>
>> But again, I never saw the original series to explain why...

Greg,

Your wording was a bit confusing for me (sorry), but it appears that you 
are disagreeing with me. Assuming that to be the case, how will moving a 
couple of lines of code (calls to device_create_file()) from a caller 
function A into caller function B make any functional difference if the 
order of the calls are maintained? The calls to device_create_file() 
will still be done as part of the same call flow/execution flow. I'm 
just asking for code refactoring to avoid repeating the code multiple times.

Thanks,
Saravana

> This is about attributes to the main device that acts as a parent for all
> devices on the SoC, so there is no user space at the time.
>
> IIRC, the original patch was documenting a set of standard attributes and
> letting the SoC specific driver pass a set of static attributes in the
> hope that they do whatever is documented.
>
> In one of my review comments, I asked this to be changed to pass a data
> structure with the actual contents of those attributes and to make
> the implementation common to the soc independent code, in order to
> reduce the amount of code needed in each soc implementation, to enforce
> consistent behavior, and to make it more obvious (and painful) when an
> soc implementation adds a nonstandard attribute.
>
> It seems the final outcome was to have a data structure of function
> pointers to get the attribute contents, which is less nice but still
> acceptable IMHO.
>
> 	Arnd
Lee Jones July 15, 2011, 6:27 a.m. UTC | #15
On 14/07/11 18:25, Saravana Kannan wrote:
> On 07/13/2011 01:51 PM, Arnd Bergmann wrote:
>> On Wednesday 13 July 2011 22:32:26 Greg KH wrote:
>>> On Wed, Jul 13, 2011 at 09:40:35AM -0700, Saravana Kannan wrote:
>>>> On 07/13/2011 12:55 AM, Lee Jones wrote:
>>>>> On 12/07/11 17:29, Saravana Kannan wrote:
>>>>> I initially had it all as part of soc_device_register, but Arnd
>>>>> told me
>>>>> to remove it in this patch-set.
>>>>>
>>>>> See here:
>>>>>
>>>>> On 17/04/11 19:36, Arnd Bergmann wrote:
>>>>>> For the nonstandard attributes, I would recommend having the
>>>>>> individual
>>>>>> drivers call device_create_file, in order to discourage the use of
>>>>>> device specific attribute names.
>>>>
>>>> Arnd,
>>>>
>>>> I don't think forcing individual driver to call device_create_file()
>>>> is really going to be much of a discouragement.
>>>
>>> Yes it is as calling that is broken and wrong and userspace will race
>>> with the kernel and bad things will happen on the wrong phase of the
>>> moon.
>>>
>>> But again, I never saw the original series to explain why...
> 
> Greg,
> 
> Your wording was a bit confusing for me (sorry), but it appears that you
> are disagreeing with me. Assuming that to be the case, how will moving a
> couple of lines of code (calls to device_create_file()) from a caller
> function A into caller function B make any functional difference if the
> order of the calls are maintained? The calls to device_create_file()
> will still be done as part of the same call flow/execution flow. I'm
> just asking for code refactoring to avoid repeating the code multiple
> times.

I believe it is Arnd that's disagreeing with you. For his full
description as to why he thought it was a bad idea to allow users to
_easily_ add nodes, see my first attempts at this patch.

I initially had this:

+struct device_attribute soc_one_attrs[] = {
+	__ATTR(machine,  S_IRUGO, ux500_get_machine,  NULL),
+	__ATTR(family,   S_IRUGO, ux500_get_family,   NULL),
+	__ATTR(soc_id,   S_IRUGO, ux500_get_soc_id,   NULL),
+	__ATTR(revision, S_IRUGO, ux500_get_revision, NULL),
+	__ATTR(process,  S_IRUGO, ux500_get_process,  NULL),
+	__ATTR_NULL,
+};
+
+struct device_attribute *each_soc_attrs[] = {
+	soc_one_attrs,
+	NULL,
+};
+
+static int __init ux500_soc_sysfs_init(void)
+{
+	return soc_device_register(&parent_soc, each_soc_attrs
+                                  ARRAY_SIZE(each_soc_attrs));
+}

Which I loved. It was neat and easy to read. However, Arnd felt that it
would be far too easy for SoC vendors/hackers to add nodes willy-nilly,
which he wanted to prevent. Hence the request for change.

Kind regards,
Lee

>> This is about attributes to the main device that acts as a parent for all
>> devices on the SoC, so there is no user space at the time.
>>
>> IIRC, the original patch was documenting a set of standard attributes and
>> letting the SoC specific driver pass a set of static attributes in the
>> hope that they do whatever is documented.
>>
>> In one of my review comments, I asked this to be changed to pass a data
>> structure with the actual contents of those attributes and to make
>> the implementation common to the soc independent code, in order to
>> reduce the amount of code needed in each soc implementation, to enforce
>> consistent behavior, and to make it more obvious (and painful) when an
>> soc implementation adds a nonstandard attribute.
>>
>> It seems the final outcome was to have a data structure of function
>> pointers to get the attribute contents, which is less nice but still
>> acceptable IMHO.
>>
>>     Arnd
>
diff mbox

Patch

diff --git a/arch/arm/mach-ux500/Kconfig b/arch/arm/mach-ux500/Kconfig
index 4210cb4..4d2f2c2 100644
--- a/arch/arm/mach-ux500/Kconfig
+++ b/arch/arm/mach-ux500/Kconfig
@@ -26,6 +26,7 @@  config MACH_U8500
 	bool "U8500 Development platform"
 	depends on UX500_SOC_DB8500
 	select TPS6105X
+	select SYS_SOC
 	help
 	  Include support for the mop500 development platform.
 
diff --git a/arch/arm/mach-ux500/id.c b/arch/arm/mach-ux500/id.c
index d35122e..5156438 100644
--- a/arch/arm/mach-ux500/id.c
+++ b/arch/arm/mach-ux500/id.c
@@ -2,12 +2,16 @@ 
  * Copyright (C) ST-Ericsson SA 2010
  *
  * Author: Rabin Vincent <rabin.vincent@stericsson.com> for ST-Ericsson
+ * Author: Lee Jones <lee.jones@linaro.org> for ST-Ericsson
  * License terms: GNU General Public License (GPL) version 2
  */
 
 #include <linux/kernel.h>
 #include <linux/init.h>
 #include <linux/io.h>
+#include <linux/sys_soc.h>
+#include <linux/slab.h>
+#include <linux/stat.h>
 
 #include <asm/cputype.h>
 #include <asm/tlbflush.h>
@@ -105,3 +109,114 @@  void __init ux500_map_io(void)
 
 	ux500_print_soc_info(asicid);
 }
+
+#ifdef CONFIG_SYS_SOC
+#define U8500_BB_UID_BASE (U8500_BACKUPRAM1_BASE + 0xFC0)
+#define U8500_BB_UID_LENGTH 5
+
+static struct device soc_parent;
+
+static ssize_t ux500_get_family(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	return sprintf(buf, "Ux500\n");
+}
+
+static ssize_t ux500_get_machine(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	return sprintf(buf, "DB%4x\n", dbx500_partnumber());
+}
+
+static ssize_t ux500_get_soc_id(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	void __iomem *uid_base;
+	int i;
+	ssize_t sz = 0;
+
+	if (dbx500_partnumber() == 0x8500) {
+		uid_base = __io_address(U8500_BB_UID_BASE);
+		for (i = 0; i < U8500_BB_UID_LENGTH; i++)
+			sz += sprintf(buf + sz, "%08x",
+				readl(uid_base + i * sizeof(u32)));
+		sz += sprintf(buf + sz, "\n");
+	} else {
+		/* Don't know where it is located for U5500 */
+		sz = sprintf(buf, "N/A\n");
+	}
+
+	return sz;
+}
+
+static ssize_t ux500_get_revision(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	unsigned int rev = dbx500_revision();
+
+	if (rev == 0x01)
+		return sprintf(buf, "%s\n", "ED");
+	else if (rev >= 0xA0)
+		return sprintf(buf, "%d.%d\n" ,
+			(rev >> 4) - 0xA + 1, rev & 0xf);
+
+	return sprintf(buf, "%s", "Unknown\n");
+}
+
+static ssize_t ux500_get_process(struct device *dev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	if (dbx500_id.process == 0x00)
+		return sprintf(buf, "Standard\n");
+
+	return sprintf(buf, "%02xnm\n", dbx500_id.process);
+}
+
+struct soc_callback_functions soc_callbacks = {
+	.get_machine_fn  = ux500_get_machine,
+	.get_family_fn   = ux500_get_family,
+	.get_soc_id_fn   = ux500_get_soc_id,
+	.get_revision_fn = ux500_get_revision,
+};
+
+struct device_attribute ux500_soc_attrs[] = {
+	__ATTR(process,  S_IRUGO, ux500_get_process,  NULL),
+	__ATTR_NULL,
+};
+
+static int __init ux500_soc_sysfs_init(void)
+{
+	int ret;
+	int i = 0;
+	ret = soc_device_register(&soc_parent,
+				  &soc_callbacks);
+	if (ret >= 0) {
+		while (ux500_soc_attrs[i].attr.name != NULL) {
+			ret = device_create_file(&soc_parent,
+						&ux500_soc_attrs[i++]);
+			if (ret)
+				goto out;
+		}
+	}
+ out:
+	return ret;
+}
+module_init(ux500_soc_sysfs_init);
+
+static void __exit ux500_soc_sysfs_exit(void)
+{
+	int i = 0;
+
+	while (ux500_soc_attrs[i].attr.name != NULL)
+		device_remove_file(&soc_parent, &ux500_soc_attrs[i++]);
+
+	soc_device_unregister(&soc_parent);
+}
+module_exit(ux500_soc_sysfs_exit);
+
+#endif