generic check_legacy_ioport
PATCH 1/1 generic check_legacy_ioport
Reject access to legacy ioports unless an isa node exists check_legacy_ioport makes only sense on PREP, CHRP and pSeries. They may have an isa node with PS/2, parport, floppy and serial ports.
ipmi calls check_legacy_ioport.
Only cell_defconfig has ipmi enabled.
cell returns -ENODEV per default.
Conclusion: nothing uses ipmi.
--- arch/powerpc/kernel/setup-common.c | 8 +++++++- arch/powerpc/platforms/cell/setup.c | 10 ---------- arch/powerpc/platforms/celleb/setup.c | 10 ---------- arch/powerpc/platforms/iseries/setup.c | 10 ---------- arch/powerpc/platforms/pasemi/setup.c | 7 ------- arch/powerpc/platforms/powermac/setup.c | 10 ---------- arch/powerpc/platforms/pseries/setup.c | 4 ++++ 7 files changed, 11 insertions(+), 48 deletions(-)
compile tested.
arch/powerpc/kernel/setup-common.c +7 -1
@@ -498,8 +498,14 @@ void probe_machine(void)int check_legacy_ioport (unsigned long base_port) { -if (ppc_md.check_legacy_ioport ==NULL ) +struct device_node *np; +if (ppc_md.check_legacy_ioport ==NULL ) { + np =of_find_node_by_type (NULL ,"isa" ); +if (np ==NULL ) +return -ENODEV ; +of_node_put (np);return 0 ; + }return ppc_md.check_legacy_ioport (base_port); }EXPORT_SYMBOL (check_legacy_ioport);
arch/powerpc/platforms/cell/setup.c +0 -10
@@ -190,15 +190,6 @@ static int __init cell_probe(void)return 1 ; } -/* -* Cell has no legacy IO; anything calling this function has to -* fail or bad things will happen -*/ -static int cell_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -define_machine (cell) { .name = "Cell", .probe = cell_probe,
@@ -211,7 +202,6 @@ define_machine(cell) { .get_rtc_time = rtas_get_rtc_time, .set_rtc_time = rtas_set_rtc_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = cell_check_legacy_ioport, .progress = cell_progress, .init_IRQ = cell_init_irq, .pci_setup_phb = rtas_setup_phb,
arch/powerpc/platforms/celleb/setup.c +0 -10
@@ -128,15 +128,6 @@ static int __init celleb_probe(void)return 1 ; } -/* -* Cell has no legacy IO; anything calling this function has to -* fail or bad things will happen -*/ -static int celleb_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -#ifdef CONFIG_KEXEC static void celleb_kexec_cpu_down (int crash,int secondary) {
@@ -173,7 +164,6 @@ define_machine(celleb) { .get_rtc_time = beat_get_rtc_time, .set_rtc_time = beat_set_rtc_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = celleb_check_legacy_ioport, .progress = celleb_progress, .power_save = beat_power_save, .nvram_size = beat_nvram_get_size,
arch/powerpc/platforms/iseries/setup.c +0 -10
@@ -628,15 +628,6 @@ static void iseries_iounmap(volatile voi { } -/* -* iSeries has no legacy IO, anything calling this function has to -* fail or bad things will happen -*/ -static int iseries_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static int __init iseries_probe (void ) {unsigned long root =of_get_flat_dt_root ();
@@ -667,7 +658,6 @@ define_machine(iseries) { .calibrate_decr = generic_calibrate_decr, .progress = iSeries_progress, .probe = iseries_probe, - .check_legacy_ioport = iseries_check_legacy_ioport, .ioremap = iseries_ioremap, .iounmap = iseries_iounmap,/* XXX Implement enable_pmcs for iSeries */
arch/powerpc/platforms/pasemi/setup.c +0 -7
@@ -101,12 +101,6 @@ void __init pas_setup_arch(void)pasemi_idle_init (); } -/* No legacy IO on our parts */ -static int pas_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static __init void pas_init_IRQ (void ) {struct device_node *np;
@@ -237,7 +231,6 @@ define_machine(pas) { .restart = pas_restart, .get_boot_time = pas_get_boot_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = pas_check_legacy_ioport, .progress = pas_progress, .machine_check_exception = pas_machine_check_handler, .pci_irq_fixup = pas_pci_irq_fixup,
arch/powerpc/platforms/powermac/setup.c +0 -10
@@ -616,15 +616,6 @@ static void __init pmac_init_early(void)#endif } -/* -* pmac has no legacy IO, anything calling this function has to -* fail or bad things will happen -*/ -static int pmac_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static int __init pmac_declare_of_platform_devices (void ) {struct device_node *np;
@@ -736,7 +727,6 @@ define_machine(powermac) { .get_rtc_time = pmac_get_rtc_time, .calibrate_decr = pmac_calibrate_decr, .feature_call = pmac_do_feature_call, - .check_legacy_ioport = pmac_check_legacy_ioport, .progress = udbg_progress, #ifdef CONFIG_PPC64 .pci_probe_mode = pmac_pci_probe_mode,
arch/powerpc/platforms/pseries/setup.c +4 -0
@@ -356,6 +356,10 @@ static int pSeries_check_legacy_ioport(ureturn -ENODEV ;of_node_put (np);break ; +default : +printk ("%s rejected access to port %u\n" , __FUNCTION__, baseport); +WARN_ON (baseport); +return -ENODEV ; }return 0 ; }
Quoted 7 lines · Olaf Hering
=20
=A0intcheck_legacy_ioport(unsigned long base_port)=A0{
-=A0=A0=A0=A0=A0=A0=A0if (ppc_md.check_legacy_ioport =3D=3D NULL)+=A0=A0=A0=A0=A0=A0=A0struct device_node *np; +=A0=A0=A0=A0=A0=A0=A0if (ppc_md.check_legacy_ioport =3D=3D NULL) { +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0np =3D of_find_node_by_type=
(NULL, "isa");
Quoted 2 lines
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (np =3D=3D NULL) +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0ret=
urn -ENODEV;
Quoted 5 lines
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0of_node_put(np); =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0return 0; +=A0=A0=A0=A0=A0=A0=A0}
=A0=A0=A0=A0=A0=A0=A0=A0returnppc_md.check_legacy_ioport(base_port); =A0}
I could be wrong, but I think I've seen fake 'isa' bus nodes in the device = tree
for machines that don't actually have isa. I probably saw this on very early cell blades (not the ones currently shipping), but perhaps others have made the same mistake.
How about simply defining a new common function like
int generic_deny_legacy_ioport(unsigned long base_port)
{
return -EINVAL;
}so that not every platform has to define their own but can either set ppc_md.check_legacy_ioport to NULL, to generic_deny_legacy_ioport or their own function if they do something fancy?
Arnd <><
Quoted 4 lines
I could be wrong, but I think I've seen fake 'isa' bus nodes in the device tree for machines that don't actually have isa. I probably saw this on very early cell blades (not the ones currently shipping), but perhaps others have made the same mistake.
JS2x SLOF pretends the LPC bus is ISA (although the two are not completely compatible!) because otherwise Linux wouldn't find the serial ports. I suppose I could have fixed the Linux code instead but it was hard enough to understand already, and really hard to change (since that would require testing on all previously supported platforms).
Quoted 10 lines
How about simply defining a new common function like
int
generic_deny_legacy_ioport(unsigned long base_port){ return -EINVAL; }so that not every platform has to define their own but can either set ppc_md.check_legacy_ioport to NULL, to
generic_deny_legacy_ioportor their own function if they do something fancy?
I'd rather deny everything legacy x86 by default, and only allow a few ranges on the few platforms that really want that.
Segher
PATCH 1/1 generic check_legacy_ioport
In reply to Olaf Heringcheck_legacy_ioport makes only sense on PREP, CHRP and pSeries. They may have an isa node with PS/2, parport, floppy and serial ports. Cell has IPMI, check for that too.
Remove the check_legacy_ioport call from ppc_md, its not needed anymore. Hardware capabilities come from the device-tree.
--- arch/powerpc/kernel/setup-common.c | 42 ++++++++++++++++++++++++++++++-- arch/powerpc/platforms/cell/setup.c | 10 ------- arch/powerpc/platforms/celleb/setup.c | 10 ------- arch/powerpc/platforms/iseries/setup.c | 10 ------- arch/powerpc/platforms/pasemi/setup.c | 7 ----- arch/powerpc/platforms/powermac/setup.c | 10 ------- arch/powerpc/platforms/pseries/setup.c | 27 -------------------- include/asm-powerpc/io.h | 7 ++++- include/asm-powerpc/machdep.h | 3 -- 9 files changed, 46 insertions(+), 80 deletions(-)
arch/powerpc/kernel/setup-common.c +40 -2
@@ -498,9 +498,47 @@ void probe_machine(void)int check_legacy_ioport (unsigned long base_port) { -if (ppc_md.check_legacy_ioport ==NULL ) +struct device_node *np; + +switch (base_port) { +#ifdef CONFIG_SERIO_I8042 +case I8042_DATA_REG : + np =of_find_node_by_type (NULL ,"8042" ); +if (np ==NULL ) +return -ENODEV ; +of_node_put (np); +return 0 ; +#endif +#ifdef CONFIG_BLK_DEV_FD +case FDC_BASE :/* FDC1 */ + np =of_find_node_by_type (NULL ,"fdc" ); +if (np ==NULL ) +return -ENODEV ; +of_node_put (np); +return 0 ; +#endif +#ifdef CONFIG_IPMI_HANDLER +/* IPMI */ +case 0xca2 : +case 0xca9 : +case 0xe4 : + np =of_find_node_by_type (NULL ,"ipmi" ); +if (np ==NULL ) +return -ENODEV ; +of_node_put (np);return 0 ; -return ppc_md.check_legacy_ioport (base_port); +#endif +#ifdef CONFIG_PPC_PREP +case _PIDXR: +case _PNPWRP: +case PNPBIOS_BASE : +/* implement me for PReP */ +#endif +default : +printk (KERN_DEBUG "%s rejected access to port %u\n" , __FUNCTION__, base_port); +WARN_ON (base_port); + } +return -ENODEV ; }EXPORT_SYMBOL (check_legacy_ioport);
arch/powerpc/platforms/cell/setup.c +0 -10
@@ -190,15 +190,6 @@ static int __init cell_probe(void)return 1 ; } -/* -* Cell has no legacy IO; anything calling this function has to -* fail or bad things will happen -*/ -static int cell_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -define_machine (cell) { .name = "Cell", .probe = cell_probe,
@@ -211,7 +202,6 @@ define_machine(cell) { .get_rtc_time = rtas_get_rtc_time, .set_rtc_time = rtas_set_rtc_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = cell_check_legacy_ioport, .progress = cell_progress, .init_IRQ = cell_init_irq, .pci_setup_phb = rtas_setup_phb,
arch/powerpc/platforms/celleb/setup.c +0 -10
@@ -128,15 +128,6 @@ static int __init celleb_probe(void)return 1 ; } -/* -* Cell has no legacy IO; anything calling this function has to -* fail or bad things will happen -*/ -static int celleb_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -#ifdef CONFIG_KEXEC static void celleb_kexec_cpu_down (int crash,int secondary) {
@@ -173,7 +164,6 @@ define_machine(celleb) { .get_rtc_time = beat_get_rtc_time, .set_rtc_time = beat_set_rtc_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = celleb_check_legacy_ioport, .progress = celleb_progress, .power_save = beat_power_save, .nvram_size = beat_nvram_get_size,
arch/powerpc/platforms/iseries/setup.c +0 -10
@@ -628,15 +628,6 @@ static void iseries_iounmap(volatile voi { } -/* -* iSeries has no legacy IO, anything calling this function has to -* fail or bad things will happen -*/ -static int iseries_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static int __init iseries_probe (void ) {unsigned long root =of_get_flat_dt_root ();
@@ -667,7 +658,6 @@ define_machine(iseries) { .calibrate_decr = generic_calibrate_decr, .progress = iSeries_progress, .probe = iseries_probe, - .check_legacy_ioport = iseries_check_legacy_ioport, .ioremap = iseries_ioremap, .iounmap = iseries_iounmap,/* XXX Implement enable_pmcs for iSeries */
arch/powerpc/platforms/pasemi/setup.c +0 -7
@@ -101,12 +101,6 @@ void __init pas_setup_arch(void)pasemi_idle_init (); } -/* No legacy IO on our parts */ -static int pas_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static __init void pas_init_IRQ (void ) {struct device_node *np;
@@ -237,7 +231,6 @@ define_machine(pas) { .restart = pas_restart, .get_boot_time = pas_get_boot_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = pas_check_legacy_ioport, .progress = pas_progress, .machine_check_exception = pas_machine_check_handler, .pci_irq_fixup = pas_pci_irq_fixup,
arch/powerpc/platforms/powermac/setup.c +0 -10
@@ -616,15 +616,6 @@ static void __init pmac_init_early(void)#endif } -/* -* pmac has no legacy IO, anything calling this function has to -* fail or bad things will happen -*/ -static int pmac_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static int __init pmac_declare_of_platform_devices (void ) {struct device_node *np;
@@ -736,7 +727,6 @@ define_machine(powermac) { .get_rtc_time = pmac_get_rtc_time, .calibrate_decr = pmac_calibrate_decr, .feature_call = pmac_do_feature_call, - .check_legacy_ioport = pmac_check_legacy_ioport, .progress = udbg_progress, #ifdef CONFIG_PPC64 .pci_probe_mode = pmac_pci_probe_mode,
arch/powerpc/platforms/pseries/setup.c +0 -27
@@ -334,32 +334,6 @@ static void __init pSeries_init_early(voDBG (" <- pSeries_init_early()\n" ); } - -static int pSeries_check_legacy_ioport (unsigned int baseport) -{ -struct device_node *np; - -#define I8042_DATA_REG 0x60 -#define FDC_BASE 0x3f0 - - -switch (baseport) { -case I8042_DATA_REG : - np =of_find_node_by_type (NULL ,"8042" ); -if (np ==NULL ) -return -ENODEV ; -of_node_put (np); -break ; -case FDC_BASE : - np =of_find_node_by_type (NULL ,"fdc" ); -if (np ==NULL ) -return -ENODEV ; -of_node_put (np); -break ; - } -return 0 ; -} -/* * Called very early, MMU is off, device-tree isn't unflattened */
@@ -532,7 +506,6 @@ define_machine(pseries) { .set_rtc_time = rtas_set_rtc_time, .calibrate_decr = generic_calibrate_decr, .progress = rtas_progress, - .check_legacy_ioport = pSeries_check_legacy_ioport, .system_reset_exception = pSeries_system_reset_exception, .machine_check_exception = pSeries_machine_check_exception, };
include/asm-powerpc/io.h +6 -1
@@ -11,7 +11,12 @@/* Check of existence of legacy devices */ extern int check_legacy_ioport (unsigned long base_port); -#define PNPBIOS_BASE 0xf000/* only relevant for PReP */ +#define I8042_DATA_REG 0x60 +#define FDC_BASE 0x3f0 +/* only relevant for PReP */ +#define _PIDXR 0x279 +#define _PNPWRP 0xa79 +#define PNPBIOS_BASE 0xf000#include <linux/compiler.h> #include <asm/page.h>
include/asm-powerpc/machdep.h +0 -3
@@ -153,9 +153,6 @@ struct machdep_calls {*/ long (*feature_call)(unsigned int feature, ...); -/* Check availability of legacy devices like i8042 */ -int (*check_legacy_ioport)(unsigned int baseport); -/* Get legacy PCI/IDE interrupt mapping */ int (*pci_get_legacy_ide_irq)(struct pci_dev *dev,int channel);
Quoted 6 lines · Olaf Hering
check_legacy_ioportmakes only sense on PREP, CHRP and pSeries. They may have an isa node with PS/2, parport, floppy and serial ports. Cell has IPMI, check for that too.Remove the
check_legacy_ioportcall fromppc_md, its not needed anymore. Hardware capabilities come from the device-tree.
..
Quoted 30 lines
Index:
b/arch/powerpc/kernel/setup-common.c
===================================================================arch/powerpc/kernel/setup-common.c +21
-1@@ -498,9 +498,47 @@ void probe_machine(void)int check_legacy_ioport (unsigned long base_port) { -if (ppc_md.check_legacy_ioport ==NULL ) +struct device_node *np; + +switch (base_port) { +#ifdef CONFIG_SERIO_I8042 +case I8042_DATA_REG : + np =of_find_node_by_type (NULL ,"8042" ); +if (np ==NULL ) +return -ENODEV ; +of_node_put (np); +return 0 ; +#endif +#ifdef CONFIG_BLK_DEV_FD +case FDC_BASE :/* FDC1 */ + np =of_find_node_by_type (NULL ,"fdc" ); +if (np ==NULL ) +return -ENODEV ; +of_node_put (np); +return 0 ; +#endif +#ifdef CONFIG_IPMI_HANDLER +/* IPMI */
arch/powerpc/kernel/setup-common.c void probe_machine(void) · line 530
Quoted 10 lines
arch/powerpc/kernel/setup-common.c +8
-1+case 0xca2 : +case 0xca9 : +case 0xe4 : + np =of_find_node_by_type (NULL ,"ipmi" ); +if (np ==NULL ) +return -ENODEV ; +of_node_put (np);return 0 ; -return ppc_md.check_legacy_ioport (base_port); +#endif
These are all ifdef'd for the driver being built-in. Also,
they check that a given type of device exists, but not that the device is at the expected address.
Can we write someting generic instead that finds the isa bus, then walks children looking for a direct child that has a matching reg to the requested port?
If this doesn't catch the cell IPMI case then another approach is to have a table of (port, expected device type) and loop rather than the case statement.
milton
Quoted 1 line · Milton Miller
These are all ifdef'd for the driver being built-in.
Yes, silly Kbuild.
Quoted 3 lines
Can we write someting generic instead that finds the isa bus, then walks children looking for a direct child that has a matching reg to the requested port?
Why bother?
Nothing else has device_type xyz in the device-tree, my patch handles all cases. The code can be updated when some new board comes along with different requirements.
PATCH 1/1 generic check_legacy_ioport
In reply to Olaf Heringcheck_legacy_ioport makes only sense on PREP, CHRP and pSeries. They may have an isa node with PS/2, parport, floppy and serial ports. Cell has IPMI, check for that too.
Remove the check_legacy_ioport call from ppc_md, its not needed anymore. Hardware capabilities come from the device-tree.
--- arch/powerpc/kernel/setup-common.c | 35 ++++++++++++++++++++++++++++++-- arch/powerpc/platforms/cell/setup.c | 10 --------- arch/powerpc/platforms/celleb/setup.c | 10 --------- arch/powerpc/platforms/iseries/setup.c | 10 --------- arch/powerpc/platforms/pasemi/setup.c | 7 ------ arch/powerpc/platforms/powermac/setup.c | 10 --------- arch/powerpc/platforms/pseries/setup.c | 27 ------------------------ include/asm-powerpc/io.h | 7 +++++- include/asm-powerpc/machdep.h | 3 -- 9 files changed, 39 insertions(+), 80 deletions(-)
arch/powerpc/kernel/setup-common.c +33 -2
@@ -498,9 +498,40 @@ void probe_machine(void)int check_legacy_ioport (unsigned long base_port) { -if (ppc_md.check_legacy_ioport ==NULL ) +struct device_node *np =NULL ; + +switch (base_port) { +#if defined(CONFIG_SERIO_I8042 ) || defined(CONFIG_SERIO_I8042_MODULE ) +case I8042_DATA_REG : + np =of_find_node_by_type (NULL ,"8042" ); +break ; +#endif +#if defined(CONFIG_BLK_DEV_FD ) || defined(CONFIG_BLK_DEV_FD_MODULE ) +case FDC_BASE :/* FDC1 */ + np =of_find_node_by_type (NULL ,"fdc" ); +break ; +#endif +#if defined(CONFIG_IPMI_HANDLER ) || defined(CONFIG_IPMI_HANDLER_MODULE ) +case 0xca2 : +case 0xca9 : +case 0xe4 : + np =of_find_node_by_type (NULL ,"ipmi" ); +break ; +#endif +#ifdef CONFIG_PPC_PREP +case _PIDXR: +case _PNPWRP: +case PNPBIOS_BASE : +/* implement me for PReP */ +#endif +default : +break ; + } +if (np) { +of_node_put (np);return 0 ; -return ppc_md.check_legacy_ioport (base_port); + } +return -ENODEV ; }EXPORT_SYMBOL (check_legacy_ioport);
arch/powerpc/platforms/cell/setup.c +0 -10
@@ -190,15 +190,6 @@ static int __init cell_probe(void)return 1 ; } -/* -* Cell has no legacy IO; anything calling this function has to -* fail or bad things will happen -*/ -static int cell_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -define_machine (cell) { .name = "Cell", .probe = cell_probe,
@@ -211,7 +202,6 @@ define_machine(cell) { .get_rtc_time = rtas_get_rtc_time, .set_rtc_time = rtas_set_rtc_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = cell_check_legacy_ioport, .progress = cell_progress, .init_IRQ = cell_init_irq, .pci_setup_phb = rtas_setup_phb,
arch/powerpc/platforms/celleb/setup.c +0 -10
@@ -128,15 +128,6 @@ static int __init celleb_probe(void)return 1 ; } -/* -* Cell has no legacy IO; anything calling this function has to -* fail or bad things will happen -*/ -static int celleb_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -#ifdef CONFIG_KEXEC static void celleb_kexec_cpu_down (int crash,int secondary) {
@@ -173,7 +164,6 @@ define_machine(celleb) { .get_rtc_time = beat_get_rtc_time, .set_rtc_time = beat_set_rtc_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = celleb_check_legacy_ioport, .progress = celleb_progress, .power_save = beat_power_save, .nvram_size = beat_nvram_get_size,
arch/powerpc/platforms/iseries/setup.c +0 -10
@@ -628,15 +628,6 @@ static void iseries_iounmap(volatile voi { } -/* -* iSeries has no legacy IO, anything calling this function has to -* fail or bad things will happen -*/ -static int iseries_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static int __init iseries_probe (void ) {unsigned long root =of_get_flat_dt_root ();
@@ -667,7 +658,6 @@ define_machine(iseries) { .calibrate_decr = generic_calibrate_decr, .progress = iSeries_progress, .probe = iseries_probe, - .check_legacy_ioport = iseries_check_legacy_ioport, .ioremap = iseries_ioremap, .iounmap = iseries_iounmap,/* XXX Implement enable_pmcs for iSeries */
arch/powerpc/platforms/pasemi/setup.c +0 -7
@@ -101,12 +101,6 @@ void __init pas_setup_arch(void)pasemi_idle_init (); } -/* No legacy IO on our parts */ -static int pas_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static __init void pas_init_IRQ (void ) {struct device_node *np;
@@ -237,7 +231,6 @@ define_machine(pas) { .restart = pas_restart, .get_boot_time = pas_get_boot_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = pas_check_legacy_ioport, .progress = pas_progress, .machine_check_exception = pas_machine_check_handler, .pci_irq_fixup = pas_pci_irq_fixup,
arch/powerpc/platforms/powermac/setup.c +0 -10
@@ -616,15 +616,6 @@ static void __init pmac_init_early(void)#endif } -/* -* pmac has no legacy IO, anything calling this function has to -* fail or bad things will happen -*/ -static int pmac_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static int __init pmac_declare_of_platform_devices (void ) {struct device_node *np;
@@ -736,7 +727,6 @@ define_machine(powermac) { .get_rtc_time = pmac_get_rtc_time, .calibrate_decr = pmac_calibrate_decr, .feature_call = pmac_do_feature_call, - .check_legacy_ioport = pmac_check_legacy_ioport, .progress = udbg_progress, #ifdef CONFIG_PPC64 .pci_probe_mode = pmac_pci_probe_mode,
arch/powerpc/platforms/pseries/setup.c +0 -27
@@ -334,32 +334,6 @@ static void __init pSeries_init_early(voDBG (" <- pSeries_init_early()\n" ); } - -static int pSeries_check_legacy_ioport (unsigned int baseport) -{ -struct device_node *np; - -#define I8042_DATA_REG 0x60 -#define FDC_BASE 0x3f0 - - -switch (baseport) { -case I8042_DATA_REG : - np =of_find_node_by_type (NULL ,"8042" ); -if (np ==NULL ) -return -ENODEV ; -of_node_put (np); -break ; -case FDC_BASE : - np =of_find_node_by_type (NULL ,"fdc" ); -if (np ==NULL ) -return -ENODEV ; -of_node_put (np); -break ; - } -return 0 ; -} -/* * Called very early, MMU is off, device-tree isn't unflattened */
@@ -532,7 +506,6 @@ define_machine(pseries) { .set_rtc_time = rtas_set_rtc_time, .calibrate_decr = generic_calibrate_decr, .progress = rtas_progress, - .check_legacy_ioport = pSeries_check_legacy_ioport, .system_reset_exception = pSeries_system_reset_exception, .machine_check_exception = pSeries_machine_check_exception, };
include/asm-powerpc/io.h +6 -1
@@ -11,7 +11,12 @@/* Check of existence of legacy devices */ extern int check_legacy_ioport (unsigned long base_port); -#define PNPBIOS_BASE 0xf000/* only relevant for PReP */ +#define I8042_DATA_REG 0x60 +#define FDC_BASE 0x3f0 +/* only relevant for PReP */ +#define _PIDXR 0x279 +#define _PNPWRP 0xa79 +#define PNPBIOS_BASE 0xf000#include <linux/compiler.h> #include <asm/page.h>
include/asm-powerpc/machdep.h +0 -3
@@ -153,9 +153,6 @@ struct machdep_calls {*/ long (*feature_call)(unsigned int feature, ...); -/* Check availability of legacy devices like i8042 */ -int (*check_legacy_ioport)(unsigned int baseport); -/* Get legacy PCI/IDE interrupt mapping */ int (*pci_get_legacy_ide_irq)(struct pci_dev *dev,int channel);
Quoted 8 lines · Olaf Hering
check_legacy_ioportmakes only sense on PREP, CHRP and pSeries. They may have an isa node with PS/2, parport, floppy and serial ports. Cell has IPMI, check for that too.Remove the
check_legacy_ioportcall fromppc_md, its not needed anymore. Hardware capabilities come from the device-tree.Signed-off-by Olaf Hering <[email protected]>
Ok, I like the aproach, but why the heck those ifdef's ? Can't you just remove them ?
Also, doing ifdef on xxxx_MODULE is always BAD because that means the module can't be built afterward.
arch/powerpc/kernel/setup-common.c void probe_machine(void) · line 532
Quoted 3 lines
arch/powerpc/kernel/setup-common.c +2
-0+if (np) { +of_node_put (np);return 0 ;
And also check now if the parent is "isa". Just to be sure. If a platform won't match that, then it will need it's own check_legacy_ioport which is fine with me.
To be totally correct, we should -also- check if the port number fits in the the actual "reg" property but I'm not sure I can be bothered :-) It's legacy stuff after all...
Cheers,
Ben.
Quoted 2 lines · Benjamin Herrenschmidt
Ok, I like the aproach, but why the heck those ifdef's ? Can't you just remove them ?
Just to keep it small for .configs without floppy, PS/2 etc. I can remove it.
Quoted 7 lines
arch/powerpc/kernel/setup-common.c void probe_machine(void) · line 532
Quoted 3 lines
arch/powerpc/kernel/setup-common.c +2
-0+if (np) { +of_node_put (np);return 0 ;And also check now if the parent is "isa". Just to be sure. If a platform won't match that, then it will need it's own
check_legacy_ioportwhich is fine with me.
Do you think a device_type fdc, i8042 or ipmi will appear outside an isa node?
Quoted 2 lines
To be totally correct, we should -also- check if the port number fits in the the actual "reg" property but I'm not sure I can be bothered :-)
Why all this complexity? Its there mainly to match a class of boards, not to match a specific device configuration.
Thats how it may look finally, currently only compile tested.
int check_legacy_ioport(unsigned long base_port)
{
struct device_node *parent, *np = NULL;
int ret = -ENODEV;
switch(base_port) {
case I8042_DATA_REG:
np = of_find_node_by_type(NULL, "8042");
break;
case FDC_BASE: /* FDC1 */
np = of_find_node_by_type(NULL, "fdc");
break;
case 0xca2:
case 0xca9:
case 0xe4:
np = of_find_node_by_type(NULL, "ipmi");
break;
#ifdef CONFIG_PPC_PREP
case _PIDXR:
case _PNPWRP:
case PNPBIOS_BASE:
/* implement me */
#endif
default:
break;
}
if (np) {
parent = of_get_parent(np);
if (parent) {
ret = strcmp(parent->type, "isa");
of_node_put(parent);
}
of_node_put(np);
}
return ret;
}Quoted 6 lines · Olaf Hering
Thats how it may look finally, currently only compile tested.
int
check_legacy_ioport(unsigned long base_port){ struct device_node *parent, *np = NULL; int ret = -ENODEV;
...
Quoted 4 lines
if (np) {
parent = of_get_parent(np);
if (parent) {
ret = strcmp(parent->type, "isa");This calculates the return as 0 / anything, vs 0 / -ENODEV.
Quoted 6 lines
of_node_put(parent);
}
of_node_put(np);
}
return ret;
}Quoted 6 lines · Olaf Hering
Quoted 3 lines
And also check now if the parent is "isa". Just to be sure. If a platform won't match that, then it will need it's own
check_legacy_ioportwhich is fine with me.Do you think a
device_typefdc, i8042 or ipmi will appear outside an isa node?
Does anyone know where those ipmi devices appear in the device-tree?
Quoted 5 lines · Olaf Hering
Quoted 3 lines
Do you think a
device_typefdc, i8042 or ipmi will appear outside an isa node?Does anyone know where those ipmi devices appear in the device-tree?
There are actually _no_ ipmi devices that we expect to appear here. the reason why the check is in the ipmi driver is in order not to crash on powerpc machines that load the ipmi driver but have no ipmi nodes in the device tree.
The init function of the ipmi module will scan all buses (PCI, ACPI, OF,
...) until it finds any devices using regular probes. If it doesn't,
it will poke at "well-known" io-ports that are used by convention on legacy i386 machines.
I don't think there are any powerpc machines where it can find something there, but we decided to leave the code architecture independent in case there ever are, and just to add the check_legacy_ioport call in there.
Arnd <><
Quoted 11 lines · Arnd Bergmann
Quoted 5 lines · Olaf Hering
Quoted 3 lines
Do you think a
device_typefdc, i8042 or ipmi will appear outside an isa node?Does anyone know where those ipmi devices appear in the device-tree?
There are actually _no_ ipmi devices that we expect to appear here. the reason why the check is in the ipmi driver is in order not to crash on powerpc machines that load the ipmi driver but have no ipmi nodes in the device tree.
To be totally correct, there might well be, but we shouldn't care.
That is, if a device XXX (let's say XXX is ipmi but it could be i8042 or whatever else) exist in the device-tree outside of the ISA bus, then it needs it's own platform or of_platform device to be probed and shouldn't make the check_legacy_io_port() go true.
There are various cases (especially in embedded world) where "standard" components have been puts in ASICs but at funky/different addresses, and they shouldn't be hit by the legacy probe.
Thus, the default implementation of check_legacy_ioport() should only match things that are under an "isa" bus. Anything else needs to be handled either by a platform specific check_legacy_ioport() override via ppc_md (if it's really some legacy stuff and under a fucked up device-tree) or via driver specific alternate probing method (like an of_platform_device).
Ben.
Quoted 5 lines
Quoted 4 lines
Quoted 2 lines
Do you think a
device_typefdc, i8042 or ipmi will appear outside an isa node?Does anyone know where those ipmi devices appear in the device-tree?
Quoted 8 lines
There are actually _no_ ipmi devices that we expect to appear here. the reason why the check is in the ipmi driver is in order not to crash on powerpc machines that load the ipmi driver but have no ipmi nodes in the device tree.
I don't think there are any powerpc machines where it can find something there, but we decided to leave the code architecture independent in case there ever are, and just to add the
check_legacy_ioportcall in there.
SLOF/JS21 (at least some versions of it) have an "ipmi" node on the "isa" bus. And the kernel ipmi driver actually works on it, too (no idea about the currently proposed scanning though -- but the principle is correct at least).
Segher
Quoted 10 lines · Segher Boessenkool
Quoted 5 lines
I don't think there are any powerpc machines where it can find=20 something
there, but we decided to leave the code architecture independent in case there ever are, and just to add thecheck_legacy_ioportcall in=20 there.=20
SLOF/JS21 (at least some versions of it) have an "ipmi" node on the "isa" bus. =A0And the kernel ipmi driver actually works on it, too (no idea about the currently proposed scanning though -- but the principle is correct at least).
It won'd be found by the of_platform_driver part of ipmi, since devices on the ISA bus do not get added to the linux device tree.
If the ipmi node on the ISA bus is located at the standardized legacy I/O port range, it should get found by the later probing, if check_legacy_ioport allows it.
We might still want to have the driver use a proper of_device, which would require the maple platform code to add this device during probing.
Arnd <><
Quoted 8 lines
Quoted 4 lines
SLOF/JS21 (at least some versions of it) have an "ipmi" node on the "isa" bus. =A0And the kernel ipmi driver actually works on it, too (no idea about the currently proposed scanning though -- but the principle is correct at least).
It won'd be found by the
of_platform_driverpart of ipmi, since devices on the ISA bus do not get added to the linux device tree.
Drat. Why not?
Quoted 3 lines
If the ipmi node on the ISA bus is located at the standardized legacy I/O port range, it should get found by the later probing, if
check_legacy_ioportallows it.
"Standardised" hahaha. No, it is at some other
address.
Quoted 3 lines
We might still want to have the driver use a proper
of_device, which would require the maple platform code to add this device during probing.
That sounds like the only option then. Does
this have to be done per platform though? It
sounds perfectly safe to do it in more generic
code.
Segher
Quoted 5 lines · Segher Boessenkool
Quoted 3 lines
It won'd be found by the
of_platform_driverpart of ipmi, since devices on the ISA bus do not get added to the linux device tree.=20
Drat. =A0Why not?
We have a number of buses on which devices get automatically added as of_devices into the Linux driver infrastructures. Which buses are probed depends on the platform, it may include e.g. "soc", "plb5" and "siliconbackplane", but no platform currently scans the ISA bus.
Quoted 8 lines
Quoted 3 lines
We might still want to have the driver use a proper
of_device, which would require the maple platform code to add this device during probing.=20
That sounds like the only option then. =A0Does
this have to be done per platform though? =A0It
sounds perfectly safe to do it in more generic
code.
I think the right solution would be to add automatic probing of all ISA devices into the platform independent code. Besides making IPMI work on JS21, it would result in the legacy serial ports to show up in the right place in sysfs.
I am however worried about what other side-effects this change could have on platforms that have a broken ISA device in their device trees.
Arnd <><
Quoted 10 lines · Olaf Hering
Quoted 6 lines · Olaf Hering
Quoted 3 lines
And also check now if the parent is "isa". Just to be sure. If a platform won't match that, then it will need it's own
check_legacy_ioportwhich is fine with me.Do you think a
device_typefdc, i8042 or ipmi will appear outside an isa node?Does anyone know where those ipmi devices appear in the device-tree?
Somebody with an up-to-date qs21 device-tree at hand ?
Ben.
Quoted 3 lines
Do you think a
device_typefdc, i8042 or ipmi will appear outside an isa node?
Well, if they do, I don't want that code to recognize them... I'm pretty sure for example we have IPMI on QS21 blades and it's not on an ISA bus and shouldn't match the legacy IO ports for example.
Quoted 27 lines
Quoted 2 lines
To be totally correct, we should -also- check if the port number fits in the the actual "reg" property but I'm not sure I can be bothered :-)
Why all this complexity? Its there mainly to match a class of boards, not to match a specific device configuration.
Thats how it may look finally, currently only compile tested.
int
check_legacy_ioport(unsigned long base_port){ struct device_node *parent, *np = NULL; int ret = -ENODEV; switch(base_port) { case I8042_DATA_REG: np = of_find_node_by_type(NULL, "8042"); break; case FDC_BASE: /* FDC1 */ np = of_find_node_by_type(NULL, "fdc"); break; case 0xca2: case 0xca9: case 0xe4: np = of_find_node_by_type(NULL, "ipmi"); break; #ifdef CONFIG_PPC_PREP case _PIDXR:
Quoted 8 lines
case _PNPWRP:
case PNPBIOS_BASE:
/* implement me */
#endif
default:
break;
}
if (np) {Why not just
if (np == NULL) return -ENODEV;
And save some tabs ? :-)
Quoted 9 lines
parent = of_get_parent(np);
if (parent) {
ret = strcmp(parent->type, "isa");
of_node_put(parent);
}
of_node_put(np);
}
return ret;
}Looks good except maybe
ret = strcmp(parent->type, "isa");
should probably be
ret = strcmp(parent->type, "isa") ? -ENODEV : 0;
Cheers,
Ben.
PATCH 1/1 generic check_legacy_ioport
In reply to Olaf Heringcheck_legacy_ioport makes only sense on PREP, CHRP and pSeries. They may have an isa node with PS/2, parport, floppy and serial ports.
Remove the check_legacy_ioport call from ppc_md, its not needed anymore. Hardware capabilities come from the device-tree.
--- arch/powerpc/kernel/setup-common.c | 34 +++++++++++++++++++++++++++++--- arch/powerpc/platforms/cell/setup.c | 10 --------- arch/powerpc/platforms/celleb/setup.c | 10 --------- arch/powerpc/platforms/iseries/setup.c | 10 --------- arch/powerpc/platforms/pasemi/setup.c | 7 ------ arch/powerpc/platforms/powermac/setup.c | 10 --------- arch/powerpc/platforms/pseries/setup.c | 27 ------------------------- include/asm-powerpc/io.h | 7 +++++- include/asm-powerpc/machdep.h | 3 -- 9 files changed, 37 insertions(+), 81 deletions(-)
arch/powerpc/kernel/setup-common.c +31 -3
@@ -496,11 +496,39 @@ void probe_machine(void)printk (KERN_INFO "Using %s machine description\n" , ppc_md.name); } +/* Match a class of boards, not a specific device configuration. */ int check_legacy_ioport (unsigned long base_port) { -if (ppc_md.check_legacy_ioport ==NULL ) -return 0 ; -return ppc_md.check_legacy_ioport (base_port); +struct device_node *parent, *np =NULL ; +int ret = -ENODEV ; + +switch (base_port) { +case I8042_DATA_REG : + np =of_find_node_by_type (NULL ,"8042" ); +break ; +case FDC_BASE :/* FDC1 */ + np =of_find_node_by_type (NULL ,"fdc" ); +break ; +#ifdef CONFIG_PPC_PREP +case _PIDXR: +case _PNPWRP: +case PNPBIOS_BASE : +/* implement me */ +#endif +default : +/* ipmi is supposed to fail here */ +break ; + } +if (!np) +return ret; + parent =of_get_parent (np); +if (parent) { +if (strcmp (parent->type,"isa" ) ==0 ) + ret =0 ; +of_node_put (parent); + } +of_node_put (np); +return ret; }EXPORT_SYMBOL (check_legacy_ioport);
arch/powerpc/platforms/cell/setup.c +0 -10
@@ -190,15 +190,6 @@ static int __init cell_probe(void)return 1 ; } -/* -* Cell has no legacy IO; anything calling this function has to -* fail or bad things will happen -*/ -static int cell_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -define_machine (cell) { .name = "Cell", .probe = cell_probe,
@@ -211,7 +202,6 @@ define_machine(cell) { .get_rtc_time = rtas_get_rtc_time, .set_rtc_time = rtas_set_rtc_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = cell_check_legacy_ioport, .progress = cell_progress, .init_IRQ = cell_init_irq, .pci_setup_phb = rtas_setup_phb,
arch/powerpc/platforms/celleb/setup.c +0 -10
@@ -128,15 +128,6 @@ static int __init celleb_probe(void)return 1 ; } -/* -* Cell has no legacy IO; anything calling this function has to -* fail or bad things will happen -*/ -static int celleb_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -#ifdef CONFIG_KEXEC static void celleb_kexec_cpu_down (int crash,int secondary) {
@@ -173,7 +164,6 @@ define_machine(celleb) { .get_rtc_time = beat_get_rtc_time, .set_rtc_time = beat_set_rtc_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = celleb_check_legacy_ioport, .progress = celleb_progress, .power_save = beat_power_save, .nvram_size = beat_nvram_get_size,
arch/powerpc/platforms/iseries/setup.c +0 -10
@@ -628,15 +628,6 @@ static void iseries_iounmap(volatile voi { } -/* -* iSeries has no legacy IO, anything calling this function has to -* fail or bad things will happen -*/ -static int iseries_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static int __init iseries_probe (void ) {unsigned long root =of_get_flat_dt_root ();
@@ -667,7 +658,6 @@ define_machine(iseries) { .calibrate_decr = generic_calibrate_decr, .progress = iSeries_progress, .probe = iseries_probe, - .check_legacy_ioport = iseries_check_legacy_ioport, .ioremap = iseries_ioremap, .iounmap = iseries_iounmap,/* XXX Implement enable_pmcs for iSeries */
arch/powerpc/platforms/pasemi/setup.c +0 -7
@@ -101,12 +101,6 @@ void __init pas_setup_arch(void)pasemi_idle_init (); } -/* No legacy IO on our parts */ -static int pas_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static __init void pas_init_IRQ (void ) {struct device_node *np;
@@ -237,7 +231,6 @@ define_machine(pas) { .restart = pas_restart, .get_boot_time = pas_get_boot_time, .calibrate_decr = generic_calibrate_decr, - .check_legacy_ioport = pas_check_legacy_ioport, .progress = pas_progress, .machine_check_exception = pas_machine_check_handler, .pci_irq_fixup = pas_pci_irq_fixup,
arch/powerpc/platforms/powermac/setup.c +0 -10
@@ -616,15 +616,6 @@ static void __init pmac_init_early(void)#endif } -/* -* pmac has no legacy IO, anything calling this function has to -* fail or bad things will happen -*/ -static int pmac_check_legacy_ioport (unsigned int baseport) -{ -return -ENODEV ; -} -static int __init pmac_declare_of_platform_devices (void ) {struct device_node *np;
@@ -736,7 +727,6 @@ define_machine(powermac) { .get_rtc_time = pmac_get_rtc_time, .calibrate_decr = pmac_calibrate_decr, .feature_call = pmac_do_feature_call, - .check_legacy_ioport = pmac_check_legacy_ioport, .progress = udbg_progress, #ifdef CONFIG_PPC64 .pci_probe_mode = pmac_pci_probe_mode,
arch/powerpc/platforms/pseries/setup.c +0 -27
@@ -334,32 +334,6 @@ static void __init pSeries_init_early(voDBG (" <- pSeries_init_early()\n" ); } - -static int pSeries_check_legacy_ioport (unsigned int baseport) -{ -struct device_node *np; - -#define I8042_DATA_REG 0x60 -#define FDC_BASE 0x3f0 - - -switch (baseport) { -case I8042_DATA_REG : - np =of_find_node_by_type (NULL ,"8042" ); -if (np ==NULL ) -return -ENODEV ; -of_node_put (np); -break ; -case FDC_BASE : - np =of_find_node_by_type (NULL ,"fdc" ); -if (np ==NULL ) -return -ENODEV ; -of_node_put (np); -break ; - } -return 0 ; -} -/* * Called very early, MMU is off, device-tree isn't unflattened */
@@ -532,7 +506,6 @@ define_machine(pseries) { .set_rtc_time = rtas_set_rtc_time, .calibrate_decr = generic_calibrate_decr, .progress = rtas_progress, - .check_legacy_ioport = pSeries_check_legacy_ioport, .system_reset_exception = pSeries_system_reset_exception, .machine_check_exception = pSeries_machine_check_exception, };
include/asm-powerpc/io.h +6 -1
@@ -11,7 +11,12 @@/* Check of existence of legacy devices */ extern int check_legacy_ioport (unsigned long base_port); -#define PNPBIOS_BASE 0xf000/* only relevant for PReP */ +#define I8042_DATA_REG 0x60 +#define FDC_BASE 0x3f0 +/* only relevant for PReP */ +#define _PIDXR 0x279 +#define _PNPWRP 0xa79 +#define PNPBIOS_BASE 0xf000#include <linux/compiler.h> #include <asm/page.h>
include/asm-powerpc/machdep.h +0 -3
@@ -153,9 +153,6 @@ struct machdep_calls {*/ long (*feature_call)(unsigned int feature, ...); -/* Check availability of legacy devices like i8042 */ -int (*check_legacy_ioport)(unsigned int baseport); -/* Get legacy PCI/IDE interrupt mapping */ int (*pci_get_legacy_ide_irq)(struct pci_dev *dev,int channel);