From: Milton Miller <hidden> Date: 2008-07-10 21:14:28
After the following patch to mark the isa region busy and applying a few
patches[1], I was able to kexec boot into an all-yes-config kernel from
linux-next, with the following KCONFIG_ALLCONFIG file:
# CONFIG_KALLSYMS_EXTRA_PASS is not set
# CONFIG_CRASH_DUMP is not set
CONFIG_PPC_EARLY_DEBUG_RTAS_CONSOLE=y
# CONFIG_NSC_FIR is not set
# CONFIG_SMC_IRCC_FIR is not set
# CONFIG_ALI_FIR is not set
# CONFIG_TCG_NSC is not set
# CONFIG_TCG_ATMEL is not set
# CONFIG_SENSORS_F71805F is not set
# CONFIG_SENSORS_F71882FG is not set
# CONFIG_SENSORS_F75375S is not set
# CONFIG_SENSORS_IT87 is not set
# CONFIG_SENSORS_PC87360 is not set
# CONFIG_SENSORS_PC87427 is not set
# CONFIG_SENSORS_DME1737 is not set
# CONFIG_SENSORS_SMSC47M1 is not set
# CONFIG_SENSORS_SMSC47B397 is not set
# CONFIG_SENSORS_VT1211 is not set
# CONFIG_SENSORS_W83627HF is not set
# CONFIG_SENSORS_W83627EHF is not set
While the first two might not be required, and the third is just
selecting the right platform, it would be nice to get the drivers
to play as well as the rest of the kernel.
The drivers all are for super-io chips, either irda/FIR drivers or hwmon,
and poke at isa ports without checking request_region.
It would be great if these 17 super-io drivers would check for resource
availability before poking at io ports.
[not that I expect this to merge as it, but maybe insert-resource or something]
[1] 3 section layout patches and the MTD DISKONCHIP patch,
see linuxppc-dev for full list
From: Milton Miller <hidden> Date: 2008-07-10 21:15:01
Such a hardcoded address can cause a checkstop or machine check if
the driver is in the kernel but the address is not acknowledged.
Both drivers allow an address to be specified as either a module
parameter or config option. Any future powerpc board should either
use one of these methods or find the address in the device tree.
Signed-off-by: Milton Miller <redacted>
---
There is no powerpc defconfig that sets either CONFIG_DOC200{0,1*} nor
CONFIG_MTD_NAND_DISKONCHIP in next-20080710.
From: Milton Miller <hidden> Date: 2008-07-10 21:16:19
This is the first step to moving the interrupt vectors to .text.head.
Rather than creating more repetition, factor out creating a function
descriptor, from declaring it to be global in a section.
The order of .globl and the symbol definitions is changed but is
a don't care.
Signed-off-by: Milton Miller <redacted>
---
I left the section in the macros, and added a new macro. I didn't
want to hunt down other uses of _GLOBAL to audit the section, and _KPROBE
needs to set the section.
From: Milton Miller <hidden> Date: 2008-07-10 21:17:44
We rely on careful placement of the start of the kernel, and use org from
the start of our image to line up the exception handlers.
When building a large kernel, the linker will insert stubs to call from one
toc to the next, breaking our careful placement. In addition, kexec does
not (currently) look at the entry address specified in the header but assumes
that the begining of the load segment is the entry point.
Move the first 0x8000 bytes to .text.head so it will be linked first.
Signed-off-by: Milton Miller <redacted>
---
@@ -614,7 +616,7 @@ unrecov_user_slb:*r3issavedinpaca->slb_r3*Weassumewearen't going to take any exceptions during this procedure.*/-_GLOBAL(slb_miss_realmode)+_HEAD_GLOBAL(slb_miss_realmode)mflrr10stwr9,PACA_EXSLB+EX_CCR(r13)/*saveCRinexc.frame*/
From: Milton Miller <hidden> Date: 2008-07-10 21:20:26
numa_enforce_memory_limit trys to be smart and only trim the memory it
finds to lmb_end_of_DRAM when a memory limit is set. However, the
early boot code may also limit memory when iommu=off is specified. When
this happens, the page allocator is told of pages not in the linear mapping
and gets a fatal DSI.
Signed-off-by: Milton Miller <redacted>
--
Previously this patch tried to check if iommu=off was specified, but it was
requested that the check just be removed.
http://patchwork.ozlabs.org/linuxppc/patch?id=11774
From: Milton Miller <hidden> Date: 2008-07-10 21:22:12
Replace a left over % 2 to be a check for the primary thread and
update the comments.
Note: We assume the secondary threads are not spinning, but they are
actually spinning if we start via kexec. This means that we call rtas to
spin up the secondary threads. If we get a bad return code from we skip
bringing the cpu online, leaving the thread spinning on the paca. We don't
handle this case in kexec, and will leave the thread wild during another
kexec.
Signed-off-by: Milton Miller <redacted>
---
I added a debug print and manually verified the correct cpumask was
generated on pSeries. I ran across the identical code in cell by
the grep of System.map.
@@ -208,11 +208,7 @@ void __init smp_init_cell(void)/* Mark threads which are still spinning in hold loops. */if(cpu_has_feature(CPU_FTR_SMT)){for_each_present_cpu(i){-if(i%2==0)-/*-*Even-numberedlogicalcpuscorrespondto-*primarythreads.-*/+if(cpu_thread_in_core(i)==0)cpu_set(i,of_spin_map);}}else{
@@ -191,8 +191,7 @@ static void __devinit smp_pSeries_kick_cpu(int nr)staticintsmp_pSeries_cpu_bootable(unsignedintnr){/* Special case - we inhibit secondary thread startup-*duringbootiftheuserrequestsit.Odd-numbered-*cpusareassumedtobesecondarythreads.+*duringbootiftheuserrequestsit.*/if(system_state<SYSTEM_RUNNING&&cpu_has_feature(CPU_FTR_SMT)&&
@@ -229,11 +228,7 @@ static void __init smp_init_pseries(void)/* Mark threads which are still spinning in hold loops. */if(cpu_has_feature(CPU_FTR_SMT)){for_each_present_cpu(i){-if(i%2==0)-/*-*Even-numberedlogicalcpuscorrespondto-*primarythreads.-*/+if(cpu_thread_in_core(i)==0)cpu_set(i,of_spin_map);}}else{
From: Milton Miller <hidden> Date: 2008-07-10 21:23:37
Because we copy the image using the kernels virtual memory system, we
require kexec user space to avoid overwriting the static kernel image, tce
tables, and the hash table. Kexec userspace gets this information by
reading propertys the kernel adds, but does not filter these propertys when
starting the next kernel.
When the second kernel trys to add its propertys, the export via
/proc/device-tree is hidden by the pre-existing but stale value from the
flat tree. Kexec userspace reads the old property, allocates the new kernel
at the old kernel's end, and gets rejected by the check
Search and remove these stale properties before adding the new values.
Signed-off-by: Milton Miller <redacted>
---
While there is an override flag to tell kexec a minimum memory to use, that
is a crude workaround and this should be applied to stable.
@@ -312,11 +312,24 @@ static struct property kernel_end_prop = {staticvoid__initexport_htab_values(void){structdevice_node*node;+structproperty*prop;node=of_find_node_by_path("/chosen");if(!node)return;+/* remove any stale propertys so ours can be found */+prop=of_find_property(node,kernel_end_prop.name,NULL);+if(prop)+prom_remove_property(node,prop);+prop=of_find_property(node,htab_base_prop.name,NULL);+if(prop)+prom_remove_property(node,prop);+prop=of_find_property(node,htab_size_prop.name,NULL);+if(prop)+prom_remove_property(node,prop);++/* information needed by userspace when using default_machine_kexec */kernel_end=__pa(_end);prom_add_property(node,&kernel_end_prop);
From: Milton Miller <hidden> Date: 2008-07-10 21:24:58
These functions should have been static, and inspection shows they
are no longer used. (We used to parse mem= but we now defer that
to early_param).
Signed-off-by: Milton Miller <redacted>
---
Should we just remove them instead?
@@ -489,6 +489,7 @@ static int __init prom_setprop(phandle node, const char *nodename,returncall_prom("interpret",1,1,(u32)(unsignedlong)cmd);}+#if 0/* We can't use the standard versions because of RELOC headaches. */#define isxdigit(c) (('0' <= (c) && (c) <= '9') \||('a'<=(c)&&(c)<='f')\
From: Hans de Goede <hidden> Date: 2008-07-10 21:26:23
Milton Miller wrote:
After the following patch to mark the isa region busy and applying a few
patches[1], I was able to kexec boot into an all-yes-config kernel from
linux-next, with the following KCONFIG_ALLCONFIG file:
# CONFIG_KALLSYMS_EXTRA_PASS is not set
# CONFIG_CRASH_DUMP is not set
CONFIG_PPC_EARLY_DEBUG_RTAS_CONSOLE=y
# CONFIG_NSC_FIR is not set
# CONFIG_SMC_IRCC_FIR is not set
# CONFIG_ALI_FIR is not set
# CONFIG_TCG_NSC is not set
# CONFIG_TCG_ATMEL is not set
# CONFIG_SENSORS_F71805F is not set
# CONFIG_SENSORS_F71882FG is not set
# CONFIG_SENSORS_F75375S is not set
# CONFIG_SENSORS_IT87 is not set
# CONFIG_SENSORS_PC87360 is not set
# CONFIG_SENSORS_PC87427 is not set
# CONFIG_SENSORS_DME1737 is not set
# CONFIG_SENSORS_SMSC47M1 is not set
# CONFIG_SENSORS_SMSC47B397 is not set
# CONFIG_SENSORS_VT1211 is not set
# CONFIG_SENSORS_W83627HF is not set
# CONFIG_SENSORS_W83627EHF is not set
While the first two might not be required, and the third is just
selecting the right platform, it would be nice to get the drivers
to play as well as the rest of the kernel.
The drivers all are for super-io chips, either irda/FIR drivers or hwmon,
and poke at isa ports without checking request_region.
Erm,
The superio sensor drivers only poke the superio chip registers without request
region during the probe phase, iow they try to detect the chip, using a widely
document and standardized (part of isa pnp AFAIK) procedure on standardized ports.
Let me try to explain a bit about superio chips, they have 2 superio control
registers (an index and data register) with which things like a manufacturer
and device id can be read, besides these id registers they also have a set of
registers with config for different logical devices. Once the id is matched,
the driver knows which logical device config to read, reads a (different) isa
base address + range from the logical device config, and then does a
request_region on the region actually used by the logical device.
The superio control registers are thus a sort of pci configuration space if you
want, doing a request_region on these is not such a good idea, as multiple
drivers (for different logical devices within the superio device) may use
these, so trying to gain exclusive access will lead to troubles.
I hope with this info about the problem space, that you maybe have a suggestion
on howto fix this?
Regards,
Hans
From: Milton Miller <hidden> Date: 2008-07-10 21:29:15
kcalloc is supposed to be called with the count as its first argument and
the element size as the second.
Signed-off-by: Milton Miller <redacted>
---
Both arguments are size_t so does not affect correctness. This callsite is
during module_init and therefore not performance critical. Another patch
will optimize the case when the count is variable but the size is fixed.
From: Milton Miller <hidden> Date: 2008-07-10 22:10:12
On Jul 10, 2008, at 4:33 PM, Hans de Goede wrote:
Milton Miller wrote:
quoted
After the following patch to mark the isa region busy and applying a
few
patches[1], I was able to kexec boot into an all-yes-config kernel
from linux-next, with the following KCONFIG_ALLCONFIG file:
...
quoted
While the first two might not be required, and the third is just
selecting the right platform, it would be nice to get the drivers
to play as well as the rest of the kernel.
The drivers all are for super-io chips, either irda/FIR drivers or
hwmon,
and poke at isa ports without checking request_region.
Erm,
The superio sensor drivers only poke the superio chip registers
without request region during the probe phase, iow they try to detect
the chip, using a widely document and standardized (part of isa pnp
AFAIK) procedure on standardized ports.
Let me try to explain a bit about superio chips, they have 2 superio
control registers (an index and data register) with which things like
a manufacturer and device id can be read, besides these id registers
they also have a set of registers with config for different logical
devices. Once the id is matched, the driver knows which logical device
config to read, reads a (different) isa base address + range from the
logical device config, and then does a request_region on the region
actually used by the logical device.
The superio control registers are thus a sort of pci configuration
space if you want, doing a request_region on these is not such a good
idea, as multiple drivers (for different logical devices within the
superio device) may use these, so trying to gain exclusive access will
lead to troubles.
I hope with this info about the problem space, that you maybe have a
suggestion on howto fix this?
My point is that they are probing without even knowing that a such a IO
range exists.
These are the only drivers left in the tree that still do this. (At
least that are not
blocked on a powerpc allyesconfig for 64-bit, !CONFIG_VIRT_TO_BUS).
One could make a superio driver, and create sub-devices for the IR,
I2C, floppy, parallel, etc
nodes.
I would even accept a check_region (horrors!), it would impove the
situation.
But most other drivers do this by request_region, probe, then release
the region afterwards.
Besides, one could argue the superio region should be requested while
probing, to prevent other cpus from poking at the super io chip at the
same time. Think of it as missing locking.
milton
From: Jean Delvare <hidden> Date: 2008-07-11 06:53:19
Hi Hans, hi Milton,
On Thu, 10 Jul 2008 16:51:33 -0500, Milton Miller wrote:
On Jul 10, 2008, at 4:33 PM, Hans de Goede wrote:
quoted
Milton Miller wrote:
quoted
After the following patch to mark the isa region busy and applying a
few
patches[1], I was able to kexec boot into an all-yes-config kernel
from linux-next, with the following KCONFIG_ALLCONFIG file:
...
quoted
quoted
While the first two might not be required, and the third is just
selecting the right platform, it would be nice to get the drivers
to play as well as the rest of the kernel.
The drivers all are for super-io chips, either irda/FIR drivers or
hwmon,
and poke at isa ports without checking request_region.
Erm,
The superio sensor drivers only poke the superio chip registers
without request region during the probe phase, iow they try to detect
the chip, using a widely document and standardized (part of isa pnp
AFAIK) procedure on standardized ports.
Let me try to explain a bit about superio chips, they have 2 superio
control registers (an index and data register) with which things like
a manufacturer and device id can be read, besides these id registers
they also have a set of registers with config for different logical
devices. Once the id is matched, the driver knows which logical device
config to read, reads a (different) isa base address + range from the
logical device config, and then does a request_region on the region
actually used by the logical device.
The superio control registers are thus a sort of pci configuration
space if you want, doing a request_region on these is not such a good
idea, as multiple drivers (for different logical devices within the
superio device) may use these, so trying to gain exclusive access will
lead to troubles.
I hope with this info about the problem space, that you maybe have a
suggestion on howto fix this?
My point is that they are probing without even knowing that a such a IO
range exists.
These are the only drivers left in the tree that still do this. (At
least that are not
blocked on a powerpc allyesconfig for 64-bit, !CONFIG_VIRT_TO_BUS).
I guess we could make all these drivers depend on X86, I suspect that
these chips are only used in PCs?
One could make a superio driver, and create sub-devices for the IR,
I2C, floppy, parallel, etc
nodes.
There have been proposals to do this, and this would indeed be a very
good idea, but unfortunately nobody took the time to implement this
properly, push it upstream and volunteer to maintain it. The problem is
that you don't need just a "driver", but a new subsystem, that needs to
be designed and maintained.
I would even accept a check_region (horrors!), it would impove the
situation.
But most other drivers do this by request_region, probe, then release
the region afterwards.
I agree that the hwmon drivers should do this, as long as no superio
subsystem exist. This should solve the problem at hand and might even
help spot bad drivers which use the configuration space for longer than
initialization time.
Besides, one could argue the superio region should be requested while
probing, to prevent other cpus from poking at the super io chip at the
same time. Think of it as missing locking.
Agreed.
Milton, care to submit a patch fixing the affected hwmon drivers?
Thanks,
--
Jean Delvare
From: Hans de Goede <hidden> Date: 2008-07-11 07:20:27
Jean Delvare wrote:
Hi Hans, hi Milton,
<snip>
quoted
One could make a superio driver, and create sub-devices for the IR,
I2C, floppy, parallel, etc
nodes.
There have been proposals to do this, and this would indeed be a very
good idea, but unfortunately nobody took the time to implement this
properly, push it upstream and volunteer to maintain it. The problem is
that you don't need just a "driver", but a new subsystem, that needs to
be designed and maintained.
Well, I believe there have been some lightweight superio locking coordinator
patches been floating around on the lm_sensors list, and I have reviewed them
and then a new version was done with my issues fixed.
I kinda liked the proposed solution there, it was quite simple, moved all the
generic superio stuff into generic superio code, and added locking for super io
access from multiple drivers, what ever happened to those patches?
If were to start using those, we could actually do a request region and then
never release it, as things should be.
Regards,
Hans
From: Jean Delvare <hidden> Date: 2008-07-11 07:37:19
On Fri, 11 Jul 2008 09:27:26 +0200, Hans de Goede wrote:
Jean Delvare wrote:
quoted
Hi Hans, hi Milton,
<snip>
quoted
quoted
One could make a superio driver, and create sub-devices for the IR,
I2C, floppy, parallel, etc
nodes.
There have been proposals to do this, and this would indeed be a very
good idea, but unfortunately nobody took the time to implement this
properly, push it upstream and volunteer to maintain it. The problem is
that you don't need just a "driver", but a new subsystem, that needs to
be designed and maintained.
Well, I believe there have been some lightweight superio locking coordinator
patches been floating around on the lm_sensors list, and I have reviewed them
and then a new version was done with my issues fixed.
I kinda liked the proposed solution there, it was quite simple, moved all the
generic superio stuff into generic superio code, and added locking for super io
access from multiple drivers, what ever happened to those patches?
As far as I know, nothing, and this is the problem. Somebody needs to
step up and call him/herself the maintainer of the new code, and push
it upstream and convert all the drivers (hwmon, watchdog, parallel
port...) to make use of it. And I am not the one to do this, I am busy
enough as is with i2c and hwmon.
If were to start using those, we could actually do a request region and then
never release it, as things should be.
Yes, if we have a superio access coordinator, it can request the region
and not release it. But as long as we don't have that, I agree with
Milton that the individual drivers should temporarily request the
Super-I/O region before accessing it.
--
Jean Delvare
From: Hans de Goede <hidden> Date: 2008-07-13 06:50:29
Jean Delvare wrote:
On Fri, 11 Jul 2008 09:27:26 +0200, Hans de Goede wrote:
quoted
Jean Delvare wrote:
quoted
Hi Hans, hi Milton,
<snip>
quoted
quoted
One could make a superio driver, and create sub-devices for the IR,
I2C, floppy, parallel, etc
nodes.
There have been proposals to do this, and this would indeed be a very
good idea, but unfortunately nobody took the time to implement this
properly, push it upstream and volunteer to maintain it. The problem is
that you don't need just a "driver", but a new subsystem, that needs to
be designed and maintained.
Well, I believe there have been some lightweight superio locking coordinator
patches been floating around on the lm_sensors list, and I have reviewed them
and then a new version was done with my issues fixed.
I kinda liked the proposed solution there, it was quite simple, moved all the
generic superio stuff into generic superio code, and added locking for super io
access from multiple drivers, what ever happened to those patches?
As far as I know, nothing, and this is the problem. Somebody needs to
step up and call him/herself the maintainer of the new code, and push
it upstream and convert all the drivers (hwmon, watchdog, parallel
port...) to make use of it. And I am not the one to do this, I am busy
enough as is with i2c and hwmon.
Ok, I've mailed Jim Cromie, the author of the superio access patches
from end 2007 and told him we're still interested in this and asked him
if he is willing to drive this forward.
Regards,
Hans
From: David Hubbard <hidden> Date: 2008-07-13 21:11:36
Hi Hans, Milton, Jean,
On Sun, Jul 13, 2008 at 12:31 AM, Hans de Goede [off-list ref] wrote:
Jean Delvare wrote:
quoted
On Fri, 11 Jul 2008 09:27:26 +0200, Hans de Goede wrote:
quoted
Jean Delvare wrote:
quoted
Hi Hans, hi Milton,
<snip>
quoted
quoted
One could make a superio driver, and create sub-devices for the IR,
I2C, floppy, parallel, etc
nodes.
There have been proposals to do this, and this would indeed be a very
good idea, but unfortunately nobody took the time to implement this
properly, push it upstream and volunteer to maintain it. The problem is
that you don't need just a "driver", but a new subsystem, that needs to
be designed and maintained.
Well, I believe there have been some lightweight superio locking coordinator
patches been floating around on the lm_sensors list, and I have reviewed them
and then a new version was done with my issues fixed.
I kinda liked the proposed solution there, it was quite simple, moved all the
generic superio stuff into generic superio code, and added locking for super io
access from multiple drivers, what ever happened to those patches?
As far as I know, nothing, and this is the problem. Somebody needs to
step up and call him/herself the maintainer of the new code, and push
it upstream and convert all the drivers (hwmon, watchdog, parallel
port...) to make use of it. And I am not the one to do this, I am busy
enough as is with i2c and hwmon.
Ok, I've mailed Jim Cromie, the author of the superio access patches
from end 2007 and told him we're still interested in this and asked him
if he is willing to drive this forward.
I propose writing a subsystem driver. (Is that properly called "The
SuperIO Bus Driver"?) If no one thinks it's a really bad idea I will
put together some code and submit it for review, and maintain it.
Some hwmon chips have odd / unique probe sequences. IMHO this is where
the design needs to be inspected. One of those is the w83627ehf, which
Jean and Hans are familiar enough with to check my work.
Thoughts?
David
From: Hans de Goede <hidden> Date: 2008-07-13 21:15:17
David Hubbard wrote:
Hi Hans, Milton, Jean,
On Sun, Jul 13, 2008 at 12:31 AM, Hans de Goede [off-list ref] wrote:
quoted
Jean Delvare wrote:
quoted
On Fri, 11 Jul 2008 09:27:26 +0200, Hans de Goede wrote:
quoted
Jean Delvare wrote:
quoted
Hi Hans, hi Milton,
<snip>
quoted
quoted
One could make a superio driver, and create sub-devices for the IR,
I2C, floppy, parallel, etc
nodes.
There have been proposals to do this, and this would indeed be a very
good idea, but unfortunately nobody took the time to implement this
properly, push it upstream and volunteer to maintain it. The problem is
that you don't need just a "driver", but a new subsystem, that needs to
be designed and maintained.
Well, I believe there have been some lightweight superio locking coordinator
patches been floating around on the lm_sensors list, and I have reviewed them
and then a new version was done with my issues fixed.
I kinda liked the proposed solution there, it was quite simple, moved all the
generic superio stuff into generic superio code, and added locking for super io
access from multiple drivers, what ever happened to those patches?
As far as I know, nothing, and this is the problem. Somebody needs to
step up and call him/herself the maintainer of the new code, and push
it upstream and convert all the drivers (hwmon, watchdog, parallel
port...) to make use of it. And I am not the one to do this, I am busy
enough as is with i2c and hwmon.
Ok, I've mailed Jim Cromie, the author of the superio access patches
from end 2007 and told him we're still interested in this and asked him
if he is willing to drive this forward.
I propose writing a subsystem driver. (Is that properly called "The
SuperIO Bus Driver"?) If no one thinks it's a really bad idea I will
put together some code and submit it for review, and maintain it.
Some hwmon chips have odd / unique probe sequences. IMHO this is where
the design needs to be inspected. One of those is the w83627ehf, which
Jean and Hans are familiar enough with to check my work.
Thoughts?
I'm afraid that making this a "bus" will be a bit overkill. Jim's patches are
quite simple and seem todo the job.
Also keep in mind that most users will be platform devices which just want to
use the superio registers to find out the baseaddress of their logical device,
a whole bus seems overkill for this, would the hwmon driver then need to be a
superio_driver (as well as an platform_driver) or can the bus be queried / used
without having to be a bustype-driver?
Regards,
Hans
From: David Hubbard <hidden> Date: 2008-07-13 21:27:11
Hi Hans,
quoted
I propose writing a subsystem driver. (Is that properly called "The
SuperIO Bus Driver"?) If no one thinks it's a really bad idea I will
put together some code and submit it for review, and maintain it.
Some hwmon chips have odd / unique probe sequences. IMHO this is where
the design needs to be inspected. One of those is the w83627ehf, which
Jean and Hans are familiar enough with to check my work.
Thoughts?
I'm afraid that making this a "bus" will be a bit overkill. Jim's patches
are quite simple and seem todo the job.
Also keep in mind that most users will be platform devices which just want
to use the superio registers to find out the baseaddress of their logical
device, a whole bus seems overkill for this, would the hwmon driver then
need to be a superio_driver (as well as an platform_driver) or can the bus
be queried / used
without having to be a bustype-driver?
I think that's a valid point. I am willing to keep it small, but I
would prefer to follow the pattern set in other subsystems. It may be
my lack of experience in designing a subsystem showing here! What are
some alternative ways to implement it?
In other words, Jim's patches are a good start, but maybe I am
misunderstanding them. I see it as the superio-locks module, a driver
that other drivers would depend on / auto-load. Is that something
other subsystems also do?
Regards,
David
From: Jean Delvare <hidden> Date: 2008-07-14 07:59:38
On Sun, 13 Jul 2008 15:26:56 -0600, David Hubbard wrote:
Hi Hans,
quoted
quoted
I propose writing a subsystem driver. (Is that properly called "The
SuperIO Bus Driver"?) If no one thinks it's a really bad idea I will
put together some code and submit it for review, and maintain it.
Some hwmon chips have odd / unique probe sequences. IMHO this is where
the design needs to be inspected. One of those is the w83627ehf, which
Jean and Hans are familiar enough with to check my work.
Thoughts?
I'm afraid that making this a "bus" will be a bit overkill. Jim's patches
are quite simple and seem todo the job.
Also keep in mind that most users will be platform devices which just want
to use the superio registers to find out the baseaddress of their logical
device, a whole bus seems overkill for this, would the hwmon driver then
need to be a superio_driver (as well as an platform_driver) or can the bus
be queried / used
without having to be a bustype-driver?
I think that's a valid point. I am willing to keep it small, but I
would prefer to follow the pattern set in other subsystems. It may be
my lack of experience in designing a subsystem showing here! What are
some alternative ways to implement it?
In other words, Jim's patches are a good start, but maybe I am
misunderstanding them. I see it as the superio-locks module, a driver
that other drivers would depend on / auto-load. Is that something
other subsystems also do?
Well, there are two approaches to the problem. The first approach
(which I think Jim took in his patches? I don't really remember) is to
simply solve the problem of concurrent I/O access to the Super-I/O
configuration ports (typically 0x2e/0x2f or 0x4e/0x4f). That would be a
simple driver requesting the ports in question and exporting an API for
other drivers to access them in a safe way.
The second approach is to make it a whole subsystem, as David is
suggesting. The Super-I/O driver would then not only request the I/O
ports, it would also identify which Super-I/O is there, and it would
create devices (in the Linux device driver model sense of the term) for
every logical device we are interested in (amongst which the hwmon
ones.) The hwmon drivers would have to be converted from platform
drivers to superio drivers.
Each approach has its advantages. The first one is rather simple and
also very generic in nature. It could be reused for other purposes. The
second one offers automatic loading of hwmon drivers, which would be
nice to have.
There's probably a middle way which would keep the simplicity of the
first approach while still allowing for driver auto-loading, without
changing the bus type of all drivers. It would probably take some
research though.
Me, I don't really care which path we choose, given that I am not the
one who will take care of it. All I have to say is that this has been
discussed several times over the past 2 years and nothing came out of
it (as far as the mainline kernel is concerned, at least) so whatever
is done now, what really matters is that it makes it into the kernel
quickly. We have some drivers missing features because of this (for
example real-time reading of VID pin values.)
This should also not prevent us from fixing the drivers now so that
they temporarily request the Super-I/O ports they are using, as Milton
suggested. Not only we don't know when we will have something better to
offer, but it might also help us find conflicts between drivers, so
that we know which drivers should make use of the future superio
driver. This could also reveal conflicts with PNP BIOS reservations,
etc. Milton, will you write a patch?
--
Jean Delvare
From: Milton Miller <hidden> Date: 2008-07-14 17:08:34
On Jul 14, 2008, at 2:59 AM, Jean Delvare wrote:
On Sun, 13 Jul 2008 15:26:56 -0600, David Hubbard wrote:
quoted
Hi Hans,
quoted
quoted
I propose writing a subsystem driver. (Is that properly called "The
SuperIO Bus Driver"?) If no one thinks it's a really bad idea I will
put together some code and submit it for review, and maintain it.
Some hwmon chips have odd / unique probe sequences. IMHO this is
where
the design needs to be inspected. One of those is the w83627ehf,
which
Jean and Hans are familiar enough with to check my work.
Thoughts?
I'm afraid that making this a "bus" will be a bit overkill. Jim's
patches
are quite simple and seem todo the job.
Also keep in mind that most users will be platform devices which
just want
to use the superio registers to find out the baseaddress of their
logical
device, a whole bus seems overkill for this, would the hwmon driver
then
need to be a superio_driver (as well as an platform_driver) or can
the bus
be queried / used
without having to be a bustype-driver?
I think that's a valid point. I am willing to keep it small, but I
would prefer to follow the pattern set in other subsystems. It may be
my lack of experience in designing a subsystem showing here! What are
some alternative ways to implement it?
In other words, Jim's patches are a good start, but maybe I am
misunderstanding them. I see it as the superio-locks module, a driver
that other drivers would depend on / auto-load. Is that something
other subsystems also do?
Well, there are two approaches to the problem. The first approach
(which I think Jim took in his patches? I don't really remember) is to
simply solve the problem of concurrent I/O access to the Super-I/O
configuration ports (typically 0x2e/0x2f or 0x4e/0x4f). That would be a
simple driver requesting the ports in question and exporting an API for
other drivers to access them in a safe way.
The second approach is to make it a whole subsystem, as David is
suggesting. The Super-I/O driver would then not only request the I/O
ports, it would also identify which Super-I/O is there, and it would
create devices (in the Linux device driver model sense of the term) for
every logical device we are interested in (amongst which the hwmon
ones.) The hwmon drivers would have to be converted from platform
drivers to superio drivers.
Each approach has its advantages. The first one is rather simple and
also very generic in nature. It could be reused for other purposes. The
second one offers automatic loading of hwmon drivers, which would be
nice to have.
There's probably a middle way which would keep the simplicity of the
first approach while still allowing for driver auto-loading, without
changing the bus type of all drivers. It would probably take some
research though.
I haven't done the research, but it might be keep superio as
a platform driver, and keep the clients as platform drivers. Only
have the superio driver probe and discover the subcomponent
addresses and then create the platform devices as children
instead of having each driver create its own platform device.
(This all assumes they are all platform devices in sysfs, I have
not looked).
This is all because in the platform bus the bus driver does not
discover the addresses but relies on drivers or platform setup code.
Me, I don't really care which path we choose, given that I am not the
one who will take care of it. All I have to say is that this has been
discussed several times over the past 2 years and nothing came out of
it (as far as the mainline kernel is concerned, at least) so whatever
is done now, what really matters is that it makes it into the kernel
quickly. We have some drivers missing features because of this (for
example real-time reading of VID pin values.)
This should also not prevent us from fixing the drivers now so that
they temporarily request the Super-I/O ports they are using, as Milton
suggested. Not only we don't know when we will have something better to
offer, but it might also help us find conflicts between drivers, so
that we know which drivers should make use of the future superio
driver. This could also reveal conflicts with PNP BIOS reservations,
etc. Milton, will you write a patch?
Well, that is the second time you asked me, so I guess I should respond.
While I it is possible for me to write this patch, my schedule and
priority list predict it would not be before the merge window closes.
In
fact, I'm not sure when it would come out. It might be argued it could
go in early -rc, but I would fear somebody's chip will not be detected
with the additional check. For example, the port may reserved as mother
board resources or something. Also, I have no hardware to test any
proposed patch, so it would be compile check only.
milton
From: Hans de Goede <hidden> Date: 2008-07-14 17:23:09
Milton Miller wrote:
On Jul 14, 2008, at 2:59 AM, Jean Delvare wrote:
quoted
On Sun, 13 Jul 2008 15:26:56 -0600, David Hubbard wrote:
quoted
Hi Hans,
quoted
quoted
I propose writing a subsystem driver. (Is that properly called "The
SuperIO Bus Driver"?) If no one thinks it's a really bad idea I will
put together some code and submit it for review, and maintain it.
Some hwmon chips have odd / unique probe sequences. IMHO this is
where
the design needs to be inspected. One of those is the w83627ehf,
which
Jean and Hans are familiar enough with to check my work.
Thoughts?
I'm afraid that making this a "bus" will be a bit overkill. Jim's
patches
are quite simple and seem todo the job.
Also keep in mind that most users will be platform devices which
just want
to use the superio registers to find out the baseaddress of their
logical
device, a whole bus seems overkill for this, would the hwmon driver
then
need to be a superio_driver (as well as an platform_driver) or can
the bus
be queried / used
without having to be a bustype-driver?
I think that's a valid point. I am willing to keep it small, but I
would prefer to follow the pattern set in other subsystems. It may be
my lack of experience in designing a subsystem showing here! What are
some alternative ways to implement it?
In other words, Jim's patches are a good start, but maybe I am
misunderstanding them. I see it as the superio-locks module, a driver
that other drivers would depend on / auto-load. Is that something
other subsystems also do?
Well, there are two approaches to the problem. The first approach
(which I think Jim took in his patches? I don't really remember) is to
simply solve the problem of concurrent I/O access to the Super-I/O
configuration ports (typically 0x2e/0x2f or 0x4e/0x4f). That would be a
simple driver requesting the ports in question and exporting an API for
other drivers to access them in a safe way.
The second approach is to make it a whole subsystem, as David is
suggesting. The Super-I/O driver would then not only request the I/O
ports, it would also identify which Super-I/O is there, and it would
create devices (in the Linux device driver model sense of the term) for
every logical device we are interested in (amongst which the hwmon
ones.) The hwmon drivers would have to be converted from platform
drivers to superio drivers.
Each approach has its advantages. The first one is rather simple and
also very generic in nature. It could be reused for other purposes. The
second one offers automatic loading of hwmon drivers, which would be
nice to have.
There's probably a middle way which would keep the simplicity of the
first approach while still allowing for driver auto-loading, without
changing the bus type of all drivers. It would probably take some
research though.
I haven't done the research, but it might be keep superio as
a platform driver, and keep the clients as platform drivers. Only
have the superio driver probe and discover the subcomponent
addresses and then create the platform devices as children
instead of having each driver create its own platform device.
(This all assumes they are all platform devices in sysfs, I have
not looked).
This is all because in the platform bus the bus driver does not
discover the addresses but relies on drivers or platform setup code.
This sounds like a good plan, rather then add a new bus type add a superio
platform driver which does superio probing and registering of platform devices
for discovered logical devices.
This superio platform driver then needs to also export some functions of those
few logical devices which need access to the superio registers for more then
just finding out their own base address.
I guess that it then would be best to load this superio driver by default on
most systems.
How does this all mix and match with isapnp, it feels to me we're doing
somewhat the same as isapnp here.
Regards,
Hans
From: David Hubbard <hidden> Date: 2008-07-14 17:55:20
Hi Hans,
On Mon, Jul 14, 2008 at 11:30 AM, Hans de Goede [off-list ref] wrote:
Milton Miller wrote:
quoted
I haven't done the research, but it might be keep superio as
a platform driver, and keep the clients as platform drivers. Only
have the superio driver probe and discover the subcomponent
addresses and then create the platform devices as children
instead of having each driver create its own platform device.
(This all assumes they are all platform devices in sysfs, I have
not looked).
This is all because in the platform bus the bus driver does not
discover the addresses but relies on drivers or platform setup code.
This sounds like a good plan, rather then add a new bus type add a superio
platform driver which does superio probing and registering of platform devices
for discovered logical devices.
This superio platform driver then needs to also export some functions of those
few logical devices which need access to the superio registers for more then
just finding out their own base address.
I guess that it then would be best to load this superio driver by default on
most systems.
How does this all mix and match with isapnp, it feels to me we're doing
somewhat the same as isapnp here.
Is there any way to use lspci and start at the LPC bridge, then find
the SuperIO chip's IO address? What about ACPI tables? Perhaps probing
logic could look for an LPC bridge before probing certain IO addresses
even if the addresses are not in the LPC bridge config.
A superio platform driver is a good way to go -- it fits with the way
the platform bus does things. Also, Jim's patches are almost there
already.
Thanks,
David
From: Jean Delvare <hidden> Date: 2008-07-15 08:28:29
Hi Milton,
On Mon, 14 Jul 2008 12:09:03 -0500, Milton Miller wrote:
On Jul 14, 2008, at 2:59 AM, Jean Delvare wrote:
quoted
Well, there are two approaches to the problem. The first approach
(which I think Jim took in his patches? I don't really remember) is to
simply solve the problem of concurrent I/O access to the Super-I/O
configuration ports (typically 0x2e/0x2f or 0x4e/0x4f). That would be a
simple driver requesting the ports in question and exporting an API for
other drivers to access them in a safe way.
The second approach is to make it a whole subsystem, as David is
suggesting. The Super-I/O driver would then not only request the I/O
ports, it would also identify which Super-I/O is there, and it would
create devices (in the Linux device driver model sense of the term) for
every logical device we are interested in (amongst which the hwmon
ones.) The hwmon drivers would have to be converted from platform
drivers to superio drivers.
Each approach has its advantages. The first one is rather simple and
also very generic in nature. It could be reused for other purposes. The
second one offers automatic loading of hwmon drivers, which would be
nice to have.
There's probably a middle way which would keep the simplicity of the
first approach while still allowing for driver auto-loading, without
changing the bus type of all drivers. It would probably take some
research though.
I haven't done the research, but it might be keep superio as
a platform driver, and keep the clients as platform drivers. Only
have the superio driver probe and discover the subcomponent
addresses and then create the platform devices as children
instead of having each driver create its own platform device.
(This all assumes they are all platform devices in sysfs, I have
not looked).
This is all because in the platform bus the bus driver does not
discover the addresses but relies on drivers or platform setup code.
That's more or less what I had in mind, yes.
quoted
(...) Milton, will you write a patch?
Well, that is the second time you asked me, so I guess I should respond.
While I it is possible for me to write this patch, my schedule and
priority list predict it would not be before the merge window closes.
In fact, I'm not sure when it would come out. It might be argued it
could go in early -rc, but I would fear somebody's chip will not be detected
with the additional check. For example, the port may reserved as mother
board resources or something.
I really don't see this as something for kernel 2.6.27, it's too late
already. It doesn't fix any actual problem anyway (none that can be
fixed by not loading drivers you don't need, at least.) That would be
for 2.6.28, so we have plenty of time to test the changes and ensure
they do not break anything. As you are the one who reported the issue
as something that was bothering you personally, I expected you to also
spend some time trying to address it.
(...) Also, I have no hardware to test any
proposed patch, so it would be compile check only.
If you write the patches and post them to the lm-sensors list, I am
certain that you will find several volunteers for testing. Me, I can
offer testing of the it87, f71805f and w83627ehf drivers.
Thanks,
--
Jean Delvare
From: Jean Delvare <hidden> Date: 2008-07-15 08:36:31
On Mon, 14 Jul 2008 11:55:01 -0600, David Hubbard wrote:
Hi Hans,
On Mon, Jul 14, 2008 at 11:30 AM, Hans de Goede [off-list ref] wrote:
quoted
Milton Miller wrote:
quoted
I haven't done the research, but it might be keep superio as
a platform driver, and keep the clients as platform drivers. Only
have the superio driver probe and discover the subcomponent
addresses and then create the platform devices as children
instead of having each driver create its own platform device.
(This all assumes they are all platform devices in sysfs, I have
not looked).
This is all because in the platform bus the bus driver does not
discover the addresses but relies on drivers or platform setup code.
This sounds like a good plan, rather then add a new bus type add a superio
platform driver which does superio probing and registering of platform devices
for discovered logical devices.
This superio platform driver then needs to also export some functions of those
few logical devices which need access to the superio registers for more then
just finding out their own base address.
I guess that it then would be best to load this superio driver by default on
most systems.
How does this all mix and match with isapnp, it feels to me we're doing
somewhat the same as isapnp here.
Is there any way to use lspci and start at the LPC bridge, then find
the SuperIO chip's IO address? What about ACPI tables? Perhaps probing
logic could look for an LPC bridge before probing certain IO addresses
even if the addresses are not in the LPC bridge config.
I always assumed that there was no way to know in advance if a
Super-I/O (LPC) chip was present or not, let alone the exact model of
the chip. The I/O addresses are decoded by the Super-I/O chip itself,
and in general it has no relation to PCI. And I've never seen ports
0x2e/0x2f nor 0x4e/0x4f listed in /proc/ioports.
But of course if there is a way to know, we should use it. Avoiding
random access to I/O ports, even if they are relatively standard in
this case, is always good.
A superio platform driver is a good way to go -- it fits with the way
the platform bus does things. Also, Jim's patches are almost there
already.
From: David Hubbard <hidden> Date: 2008-07-15 15:31:49
Hi Jean,
On Tue, Jul 15, 2008 at 2:36 AM, Jean Delvare [off-list ref] wrote:
quoted
Is there any way to use lspci and start at the LPC bridge, then find
the SuperIO chip's IO address? What about ACPI tables? Perhaps probing
logic could look for an LPC bridge before probing certain IO addresses
even if the addresses are not in the LPC bridge config.
I always assumed that there was no way to know in advance if a
Super-I/O (LPC) chip was present or not, let alone the exact model of
the chip. The I/O addresses are decoded by the Super-I/O chip itself,
and in general it has no relation to PCI. And I've never seen ports
0x2e/0x2f nor 0x4e/0x4f listed in /proc/ioports.
But of course if there is a way to know, we should use it. Avoiding
random access to I/O ports, even if they are relatively standard in
this case, is always good.
I looked at my lspci output and did a little researching, and I think
the only thing we can deduce is that there is an LPC bridge, so
looking for a SuperIO is a good idea. If there is no LPC bridge
listed, I can't say whether probing the ports is a good idea or not.
David
From: Jean Delvare <hidden> Date: 2008-07-16 07:46:22
Hi David,
On Tue, 15 Jul 2008 09:31:29 -0600, David Hubbard wrote:
On Tue, Jul 15, 2008 at 2:36 AM, Jean Delvare [off-list ref] wrote:
quoted
I always assumed that there was no way to know in advance if a
Super-I/O (LPC) chip was present or not, let alone the exact model of
the chip. The I/O addresses are decoded by the Super-I/O chip itself,
and in general it has no relation to PCI. And I've never seen ports
0x2e/0x2f nor 0x4e/0x4f listed in /proc/ioports.
But of course if there is a way to know, we should use it. Avoiding
random access to I/O ports, even if they are relatively standard in
this case, is always good.
I looked at my lspci output and did a little researching, and I think
the only thing we can deduce is that there is an LPC bridge, so
looking for a SuperIO is a good idea. If there is no LPC bridge
listed, I can't say whether probing the ports is a good idea or not.
Machines I have here have an "ISA bridge" PCI device. Some of them have
"LPC" in their name, but not all, and anyway the kernel only knows the
device ID, not its user-friendly name:
VIA:
00:11.0 ISA bridge: VIA Technologies, Inc. VT8237 ISA bridge [KT600/K8T800/K8T890 South]
Intel:
00:01.0 ISA bridge: Intel Corporation 82371AB/EB/MB PIIX4 ISA (rev 01)
00:1f.0 ISA bridge: Intel Corporation 82801CAM ISA Bridge (LPC) (rev 01)
00:1f.0 ISA bridge: Intel Corporation 82801EB/ER (ICH5/ICH5R) LPC Interface Bridge (rev 02)
nVidia:
00:01.0 ISA bridge: nVidia Corporation nForce2 ISA Bridge (rev a4)
One of these machines has a LPC bridge but no (detected) Super-I/O chip.
The VIA and nVidia machines do not have "LPC" in their bridge name, but
they do have a Super-I/O chip, while the first Intel machine doesn't.
As a conclusion, there is no clear relationship between the presence of
an ISA or LPC bridge and the presence of a Super-I/O chip. All we can
say is that apparently all PC systems have an ISA bridge. So indeed we
can probably make the probes conditional upon the presence of an "ISA
bridge" PCI device. That's very easy to test. In practice it might be
equivalent to making the driver x86-only.
--
Jean Delvare
From: Rene Herman <hidden> Date: 2008-07-16 08:08:00
On 16-07-08 09:46, Jean Delvare wrote:
As a conclusion, there is no clear relationship between the presence
of an ISA or LPC bridge and the presence of a Super-I/O chip. All we
can say is that apparently all PC systems have an ISA bridge. So
indeed we can probably make the probes conditional upon the presence
of an "ISA bridge" PCI device. That's very easy to test. In practice
it might be equivalent to making the driver x86-only.
And foregoing non (pre-) PCI. Admittedly, that might not be much of an
issue and I don't know if "Super-I/O" is maybe even defined such as to
make it a NON issue but I do for example remember my old 386 chipset
providing for extended serial rates which I thought was way cool back
then. If that's Super-I/O (or something that should/could be covered by
the infrastructure at least) then PCI wouldn't be helpful.
Rene.