Framework for exporting System-on-Chip information via sysfs
PATCH 1/3 Framework for exporting System-on-Chip information via sysfs
--- drivers/base/Kconfig | 10 +++++ drivers/base/Makefile | 1 + drivers/base/soc.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sys_soc.h | 45 +++++++++++++++++++++ 4 files changed, 154 insertions(+), 0 deletions(-) create mode 100644 drivers/base/soc.c create mode 100644 include/linux/sys_soc.h
drivers/base/Kconfig +10 -0
@@ -168,4 +168,14 @@ config SYS_HYPERVISOR bool default n +config ARCH_NO_SYSDEV_OPS + bool + ---help--- + To be selected by architectures that don't use sysdev class or + sysdev driver power management (suspend/resume) and shutdown + operations. + +config SYS_SOC + bool + endmenu
drivers/base/Makefile +1 -0
@@ -18,6 +18,7 @@ ifeq ($(CONFIG_SYSFS),y) obj-$(CONFIG_MODULES) += module.o endif obj-$(CONFIG_SYS_HYPERVISOR) += hypervisor.o +obj-$(CONFIG_SYS_SOC) += soc.o ccflags-$(CONFIG_DEBUG_DRIVER) := -DDEBUG
drivers/base/soc.c new file +98 -0
98 lines not on this page · raw message
include/linux/sys_soc.h new file +45 -0
45 lines not on this page · raw message
Signature · 1 line
1.7.4.1
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
--- arch/arm/mach-ux500/Kconfig | 1 + arch/arm/mach-ux500/id.c | 115 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 0 deletions(-)
arch/arm/mach-ux500/Kconfig +1 -0
@@ -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.
arch/arm/mach-ux500/id.c +115 -0
130 lines not on this page · raw message
Signature · 1 line
1.7.4.1
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 8 lines · Lee Jones
Signed-off-by Lee Jones<[email protected]>--- arch/arm/mach-ux500/Kconfig | 1 + arch/arm/mach-ux500/id.c | 115 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 0 deletions(-)arch/arm/mach-ux500/Kconfig +0
-0
<snip>
Quoted 17 lines
arch/arm/mach-ux500/Kconfig +17
-0+ +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;
arch/arm/mach-ux500/Kconfig
Quoted 10 lines
arch/arm/mach-ux500/Kconfig +10
-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.
arch/arm/mach-ux500/Kconfig
Quoted 11 lines
+ out:
arch/arm/mach-ux500/Kconfig +10
-0+ 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.
arch/arm/mach-ux500/Kconfig
Quoted 6 lines
arch/arm/mach-ux500/Kconfig +6
-0+ + soc_device_unregister(&soc_parent); +} +module_exit(ux500_soc_sysfs_exit); + +#endif
Thanks,
Saravana
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 22 lines · Saravana Kannan
Quoted 22 lines
arch/arm/mach-ux500/id.c +22
-0+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++]);
Quoted 8 lines
arch/arm/mach-ux500/id.c void __init ux500_map_io(void) · line 205
Quoted 4 lines
arch/arm/mach-ux500/id.c +4
-0+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:
Quoted 3 lines · Arnd Bergmann
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:
Quoted 4 lines
arch/arm/mach-ux500/id.c void __init ux500_map_io(void) · line 184
Quoted 4 lines
arch/arm/mach-ux500/id.c +4
-0+ .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:
Quoted 1 line
Quoted 1 line
+
__ATTR(process, S_IRUGO, ux500_get_process, NULL),
Which is subsequently added by the platform.
Kind regards,
Lee
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 5 lines · Lee Jones
Quoted 3 lines · Saravana Kannan
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.
Quoted 11 lines
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:
Quoted 3 lines · Arnd Bergmann
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
Quoted 16 lines
Hence all of these are the _standard_ attributes:
Quoted 4 lines
arch/arm/mach-ux500/id.c void __init ux500_map_io(void) · line 184
Quoted 4 lines
arch/arm/mach-ux500/id.c +4
-0+ .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
_nonstandard_one:Quoted 1 line
Quoted 1 line
+
__ATTR(process, S_IRUGO, ux500_get_process, NULL),Which is subsequently added by the platform.
Kind regards,
Lee
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 14 lines · Saravana Kannan
Quoted 13 lines · Lee Jones
Quoted 3 lines · Saravana Kannan
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:
Quoted 8 lines
Quoted 3 lines
Quoted 3 lines
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
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 15 lines · On Wednesday 13 July 2011 22:32:26 Greg KH wrote:
Quoted 14 lines · Saravana Kannan
Quoted 10 lines · Lee Jones
On 12/07/11 17:29, Saravana Kannan wrote:
I initially had it all as part ofsoc_device_register, but Arnd told me to remove it in this patch-set.See here:
Quoted 3 lines · Arnd Bergmann
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,
Quoted 8 lines
Quoted 2 lines
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
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 3 lines
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
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 6 lines · Lee Jones
Quoted 3 lines
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.
Quoted 2 lines
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.
Quoted 2 lines
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
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 10 lines · Arnd Bergmann
Quoted 6 lines · Lee Jones
Quoted 3 lines
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:
Quoted 8 lines
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.
Quoted 6 lines
Quoted 2 lines
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.
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
In reply to Arnd BergmannQuoted 16 lines · Arnd Bergmann
Quoted 15 lines · On Wednesday 13 July 2011 22:32:26 Greg KH wrote:
Quoted 14 lines · Saravana Kannan
Quoted 10 lines · Lee Jones
On 12/07/11 17:29, Saravana Kannan wrote:
I initially had it all as part ofsoc_device_register, but Arnd told me to remove it in this patch-set.See here:
Quoted 3 lines · Arnd Bergmann
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,
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
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
Quoted 19 lines
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
Signature · 2 lines
Sent by an employee of the Qualcomm Innovation Center, Inc. The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 30 lines · Saravana Kannan
Quoted 18 lines · Arnd Bergmann
Quoted 17 lines · On Wednesday 13 July 2011 22:32:26 Greg KH wrote:
Quoted 16 lines · Saravana Kannan
Quoted 5 lines · Lee Jones
On 12/07/11 17:29, Saravana Kannan wrote:
I initially had it all as part ofsoc_device_register, but Arnd told me
to remove it in this patch-set.Quoted 7 lines
See here:
Quoted 4 lines · Arnd Bergmann
For the nonstandard attributes, I would recommend having the individual
drivers calldevice_create_file, in order to discourage the use of device specific attribute names.Arnd,
Quoted 8 lines
Quoted 8 lines
Quoted 2 lines
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,
Quoted 8 lines
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 todevice_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
Quoted 20 lines
Quoted 19 lines
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
Signature · 5 lines
Lee Jones Linaro ST-Ericsson Landing Team Lead M: +44 77 88 633 515 Linaro.org ? Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
Arnd Bergmann
NAKPATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
In reply to Lee JonesQuoted 1 line · Lee Jones
Signed-off-by Lee Jones <[email protected]>
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
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 10 lines · Arnd Bergmann
Quoted 1 line · Lee Jones
Signed-off-by Lee Jones <[email protected]>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
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 14 lines · Lee Jones
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 ...
Quoted 8 lines
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
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 46 lines · Arnd Bergmann
Quoted 14 lines · Lee Jones
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 ...
Quoted 8 lines
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_devicesthat 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.cand 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/...
Quoted 3 lines
Basically, if you have a soc node, I would expect
/sys/devices/platformto 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?
PATCH 2/3 mach-ux500: export System-on-Chip information via sysfs
Quoted 4 lines · Lee Jones
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
PATCH 3/3 Add documenation for new sysfs devices/soc functionallity
In reply to Lee Jones--- Documentation/ABI/testing/sysfs-devices-soc | 16 ++++++++++++++++ 1 files changed, 16 insertions(+), 0 deletions(-) create mode 100644 Documentation/ABI/testing/sysfs-devices-soc
Documentation/ABI/testing/sysfs-devices-soc new file +16 -0
@@ -0,0 +1,16 @@ +What: /sys/devices/soc +Date: April 2011 +contact: Lee Jones <[email protected]> +Description: + The /sys/devices/soc directory contains a sub-directory for each + System-on-Chip (SoC) device on a running platform. Information + regarding each SoC can be obtained by reading sysfs files. This + functionallity is only available if implemented by the platform. + This directory contains two kinds of attributes: + - Common attributes: + * machine: the name of the machine (e.g. Ux500). + * family: the family name of the SoC (e.g. DB8500). + - SoC-specific attributes: + * SoC vendor declared, such as serial and rev numbers. +Users: + Any platform dependent user-space applications or profiling tools.
Signature · 1 line
1.7.4.1
PATCH 1/3 Framework for exporting System-on-Chip information via sysfs
In reply to Lee JonesHi Lee,
Quoted 9 lines · Lee Jones
Signed-off-by Lee Jones <[email protected]>--- drivers/base/Kconfig | 10 +++++ drivers/base/Makefile | 1 + drivers/base/soc.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sys_soc.h | 45 +++++++++++++++++++++ 4 files changed, 154 insertions(+), 0 deletions(-) create mode 100644 drivers/base/soc.c create mode 100644 include/linux/sys_soc.h
I guess you should Cc Greg Kroah-Hartman <[email protected]> on this series.
baruch
[snip]
Signature · 3 lines
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -PATCH 1/3 Framework for exporting System-on-Chip information via sysfs
Quoted 13 lines · Baruch Siach
Hi Lee,
Quoted 9 lines · Lee Jones
Signed-off-by Lee Jones <[email protected]>--- drivers/base/Kconfig | 10 +++++ drivers/base/Makefile | 1 + drivers/base/soc.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sys_soc.h | 45 +++++++++++++++++++++ 4 files changed, 154 insertions(+), 0 deletions(-) create mode 100644 drivers/base/soc.c create mode 100644 include/linux/sys_soc.h
Quoted 1 line
I guess you should Cc Greg Kroah-Hartman <[email protected]> on this series.
Only if the submitter wants it applied, obviously, they don't :)
Why do we have things like the MAINTAINERS file and scripts/get_maintainer.pl if no one uses them...
greg k-h
PATCH 1/3 Framework for exporting System-on-Chip information via sysfs
Quoted 14 lines · Greg KH
Quoted 13 lines · Baruch Siach
Hi Lee,
Quoted 9 lines · Lee Jones
Signed-off-by Lee Jones <[email protected]>--- drivers/base/Kconfig | 10 +++++ drivers/base/Makefile | 1 + drivers/base/soc.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sys_soc.h | 45 +++++++++++++++++++++ 4 files changed, 154 insertions(+), 0 deletions(-) create mode 100644 drivers/base/soc.c create mode 100644 include/linux/sys_soc.h
Quoted 1 line
Quoted 1 line
I guess you should Cc Greg Kroah-Hartman <[email protected]> on this series.
Yes, of course. Thanks Baruch.
Quoted 4 lines
Only if the submitter wants it applied, obviously, they don't :)
Why do we have things like the
MAINTAINERSfile andscripts/get_maintainer.plif no one uses them...
Doh! Sorry Greg. I'm playing around with my new send-patches.sh and I've neglected to add functionality to apply bespoke To, CC and BCC fields on sending. I'll make the amendments to the script for next time.
PATCH 1/3 Framework for exporting System-on-Chip information via sysfs
Quoted 19 lines · Lee Jones
Quoted 14 lines · Greg KH
Quoted 13 lines · Baruch Siach
Hi Lee,
Quoted 9 lines · Lee Jones
Signed-off-by Lee Jones <[email protected]>--- drivers/base/Kconfig | 10 +++++ drivers/base/Makefile | 1 + drivers/base/soc.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sys_soc.h | 45 +++++++++++++++++++++ 4 files changed, 154 insertions(+), 0 deletions(-) create mode 100644 drivers/base/soc.c create mode 100644 include/linux/sys_soc.hQuoted 1 line
Quoted 1 line
I guess you should Cc Greg Kroah-Hartman <[email protected]> on this series.
Yes, of course. Thanks Baruch.
Quoted 8 lines
Quoted 4 lines
Only if the submitter wants it applied, obviously, they don't :)
Why do we have things like the
MAINTAINERSfile andscripts/get_maintainer.plif no one uses them...Doh! Sorry Greg. I'm playing around with my new
send-patches.shand I've neglected to add functionality to apply bespoke To, CC and BCC fields on sending. I'll make the amendments to the script for next time.
Do you really need another 'send-patches.sh' type script? What's wrong with the built in ones from git or quilt that requires a custom one?
greg k-h
PATCH 1/3 Framework for exporting System-on-Chip information via sysfs
Quoted 25 lines · Greg KH
Quoted 24 lines · Lee Jones
Quoted 14 lines · Greg KH
Quoted 13 lines · Baruch Siach
Hi Lee,
Quoted 9 lines · Lee Jones
Signed-off-by Lee Jones <[email protected]>--- drivers/base/Kconfig | 10 +++++ drivers/base/Makefile | 1 + drivers/base/soc.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sys_soc.h | 45 +++++++++++++++++++++ 4 files changed, 154 insertions(+), 0 deletions(-) create mode 100644 drivers/base/soc.c create mode 100644 include/linux/sys_soc.hQuoted 1 line
Quoted 1 line
I guess you should Cc Greg Kroah-Hartman <[email protected]> on this series.
Yes, of course. Thanks Baruch.
Quoted 4 lines
Only if the submitter wants it applied, obviously, they don't :)
Why do we have things like the
MAINTAINERSfile andscripts/get_maintainer.plif no one uses them...
Quoted 6 lines
Quoted 3 lines
Doh! Sorry Greg. I'm playing around with my new
send-patches.shand I've neglected to add functionality to apply bespoke To, CC and BCC fields on sending. I'll make the amendments to the script for next time.Do you really need another '
send-patches.sh' type script? What's wrong with the built in ones from git or quilt that requires a custom one?
Yes, I really am that lazy. :)
Firstly, I don't send enough patches upstream to enable me to learn then retain the knowledge to quickly type out a git send-email command. If I write a script just once and keep it centrally located, it will inevitably save me time in the long-term. Also, when I draft patches I may do so from a different email address depending on the hat I'm wearing at any given moment. Supplying an account flag "--can" or "--lin" is somewhat easier than typing out different credentials each time. "--smtp-server", "--smtp-server-port", "--smtp-encryption", "--smtp-user", "--smtp-pass" and "--from" all differ from account to account.
With this script I dump the patch-set into a directory and issue "send-patches.sh --lin --arm <patchdir>" and off goes the patch-set to the Arm ML. However, as Baruch and yourself kindly noticed, I omitted functionality to add the maintainer and interested party email address - I'll fix that now.
Sorry for any inconvenience this may have caused.
Kind regards,
Lee
PATCH 1/3 Framework for exporting System-on-Chip information via sysfs
In reply to Lee JonesQuoted 6 lines · Lee Jones
Signed-off-by Lee Jones <[email protected]>--- drivers/base/Kconfig | 10 +++++ drivers/base/Makefile | 1 + drivers/base/soc.c | 98 +++++++++++++++++++++++++++++++++++++++++++++++ include/linux/sys_soc.h | 45 +++++++++++++++++++++
After looking at the patch again, I do have some important comments after all. I had only looked at the documentation you posted, not at the actual patch.
Quoted 5 lines
drivers/base/Kconfig +0
-0
drivers/base/Kconfig config SYS_HYPERVISOR · line 176
Quoted 9 lines
drivers/base/Kconfig +6
-0@@ -168,4 +168,14 @@ config SYS_HYPERVISOR bool default n +config ARCH_NO_SYSDEV_OPS + bool + ---help--- + To be selected by architectures that don't use sysdev class or + sysdev driver power management (suspend/resume) and shutdown + operations.
You don't seem to be using this anywhere. What is this for?
Quoted 17 lines
drivers/base/soc.c new file +11
-0@@ -0,0 +1,98 @@ +/* +* Copyright (C) ST-Ericsson SA 2011 +* +* Author: Lee Jones <[email protected]> for ST-Ericsson. +* License terms: GNU General Public License (GPL), version 2 +*/ + +#include <linux/sysfs.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/stat.h>
drivers/base/soc.c line 21
Quoted 10 lines
drivers/base/soc.c new file +10
-0+#include <linux/slab.h> +#include <linux/sys_soc.h> + +struct device_attribute soc_attrs[] = { +__ATTR (machine,S_IRUGO ,NULL ,NULL ), +__ATTR (family,S_IRUGO ,NULL ,NULL ), +__ATTR (soc_id,S_IRUGO ,NULL ,NULL ), +__ATTR (revision,S_IRUGO ,NULL ,NULL ), + __ATTR_NULL, +};
This method does not work. You only set the function pointers when you call soc_device_register, but they will end up overwriting the existing function pointers when you have multiple callers of that function.
It would be better to define a 'struct soc_device' derived from 'struct device' that holds a pointer to the actual strings (or operations, if you insist) and provide a single soc_info_get() function that is used for all the attributes.
drivers/base/soc.c line 24
Quoted 2 lines
drivers/base/soc.c new file +2
-0+const char *soc_names[] = {"1" ,"2" ,"3" ,"4" ,"5" ,"6" ,"7" ,"8" }; +static int soc_count =0 ;
This is a bit silly and artificially limits the number of SoC devices. It's much simpler to just use dev_set_name().
drivers/base/soc.c line 31
Quoted 6 lines
drivers/base/soc.c new file +5
-0+static struct device soc_grandparent = { + .init_name ="soc" , +}; +static int soc_device_create_files (struct device *soc); +static void soc_device_remove_files (struct device *soc);
No forward declarations for static functions please.
drivers/base/soc.c line 66
Quoted 9 lines
drivers/base/soc.c new file +9
-0+int __init soc_device_register (struct device *soc_parent, +struct soc_callback_functions *soc_callbacks) +{ +int ret; + + soc_attrs[MACHINE ].show = soc_callbacks->get_machine_fn; + soc_attrs[FAMILY ].show = soc_callbacks->get_family_fn; + soc_attrs[SOCID ].show = soc_callbacks->get_soc_id_fn; + soc_attrs[REVISION ].show = soc_callbacks->get_revision_fn;
It's better to allocate the device here, so you don't have to do it in each caller.
drivers/base/soc.c line 84
Quoted 10 lines
drivers/base/soc.c new file +10
-0+ soc_parent->init_name = soc_names[soc_count]; + soc_parent->parent = &soc_grandparent; + + ret =device_register (soc_parent); +if (ret) +return ret; + +soc_device_create_files (soc_parent); + + soc_count++;
This needs some locking to ensure that you don't try to register two devices with the same number.
drivers/base/soc.c line 98
Quoted 11 lines
drivers/base/soc.c new file +11
-0+ +void __exit soc_device_unregister(struct device *soc_parent) +{ +/* Unregister and free SoC from sysfs */ +soc_device_remove_files (soc_parent); +device_unregister (soc_parent); + +if (--soc_count ==0 ) +/* Unregister top-level SoC device '/sys/devices/soc' */ +device_unregister (&soc_grandparent); +}
The exit function does not make any sense if you cannot build the soc support itself as a module, which in turn makes no sense at all.
drivers/base/soc.c
Quoted 7 lines
drivers/base/soc.c new file +7
-0+ +#define MAX_SOCS 8 +#define MACHINE 0 +#define FAMILY 1 +#define SOCID 2 +#define REVISION 3 +
I think these defines are used nowhere by the individual drivers, so they need not be in the header file. More importantly, the names are not put in a proper namespace, so they can easily collide with other macros or enums.
Arnd