[PATCH] Implement check_legacy_ioport() on PS3

It doesn't like it when we poke at legacy I/O ports like the i8042. We
should probably change the _default_ to return -ENODEV, and let any
strange platform which wants to allow unfettered access provide its own
function for that.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>

--- linux-2.6.20.ppc64/arch/powerpc/platforms/ps3/setup.c.orig	2007-03-30 00:45:59.000000000 +0100
+++ linux-2.6.20.ppc64/arch/powerpc/platforms/ps3/setup.c	2007-03-30 00:48:29.000000000 +0100

@@ -230,6 +230,11 @@ static void ps3_machine_kexec(struct kim
 }
 #endif
 
+static int ps3_check_legacy_ioport(unsigned int baseport)
+{
+        return -ENODEV;
+}
+
 define_machine(ps3) {
 	.name				= "PS3",
 	.probe				= ps3_probe,

@@ -240,6 +245,7 @@ define_machine(ps3) {
 	.set_rtc_time			= ps3_set_rtc_time,
 	.get_rtc_time			= ps3_get_rtc_time,
 	.calibrate_decr			= ps3_calibrate_decr,
+	.check_legacy_ioport		= ps3_check_legacy_ioport,
 	.progress			= ps3_progress,
 	.restart			= ps3_restart,
 	.power_off			= ps3_power_off,

-- 
dwmw2

On Thu, 2007-03-29 at 20:45 -0500, Olof Johansson wrote:

I ran into the very same thing earlier myself and had exactly the same
idea. However, it turned out then that it would have meant adding more
cases of ioport being allowed than it would have removed the opposite. It
might have changed since then. Either way it's annoying. 

Perhaps we could set it to (void *)-1 to always return -ENODEV?

-- 
dwmw2

On Thu, 2007-03-29 at 20:45 -0500, Olof Johansson wrote:

On Fri, Mar 30, 2007 at 01:02:52AM +0100, David Woodhouse wrote:

quoted

It doesn't like it when we poke at legacy I/O ports like the i8042. We
should probably change the _default_ to return -ENODEV, and let any
strange platform which wants to allow unfettered access provide its own
function for that.

I ran into the very same thing earlier myself and had exactly the same
idea. However, it turned out then that it would have meant adding more
cases of ioport being allowed than it would have removed the opposite. It
might have changed since then. Either way it's annoying.

No I just looked at it too. We have 19 platforms, 7 define
check_legacy_ioport(), 6 of those return ENODEV, one does something
useful. Which leaves 12 platforms that get the default "return 0"
behaviour.

I still don't like it though, having to opt _out_ of random io port
poking is not nice.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person

On Thu, 2007-03-29 at 20:45 -0500, Olof Johansson wrote:

On Fri, Mar 30, 2007 at 01:02:52AM +0100, David Woodhouse wrote:

quoted

It doesn't like it when we poke at legacy I/O ports like the i8042. We
should probably change the _default_ to return -ENODEV, and let any
strange platform which wants to allow unfettered access provide its own
function for that.

I ran into the very same thing earlier myself and had exactly the same
idea. However, it turned out then that it would have meant adding more
cases of ioport being allowed than it would have removed the opposite. It
might have changed since then. Either way it's annoying.

I was thinking that we could have one single helper shared by most
platforms who allow that legacy ISA crap which checks the device-tree
for known ISA devices. That would cover PREP, CHRP and pSeries at
least...

Ben.

On Fri, Mar 30, 2007 at 01:02:52AM +0100, David Woodhouse wrote:

It doesn't like it when we poke at legacy I/O ports like the i8042. We
should probably change the _default_ to return -ENODEV, and let any
strange platform which wants to allow unfettered access provide its own
function for that.

I ran into the very same thing earlier myself and had exactly the same
idea. However, it turned out then that it would have meant adding more
cases of ioport being allowed than it would have removed the opposite. It
might have changed since then. Either way it's annoying.


-Olof

On Fri, 2007-03-30 at 11:43 +1000, Michael Ellerman wrote:

On Thu, 2007-03-29 at 20:45 -0500, Olof Johansson wrote:

quoted

On Fri, Mar 30, 2007 at 01:02:52AM +0100, David Woodhouse wrote:

quoted

It doesn't like it when we poke at legacy I/O ports like the i8042. We
should probably change the _default_ to return -ENODEV, and let any
strange platform which wants to allow unfettered access provide its own
function for that.

I ran into the very same thing earlier myself and had exactly the same
idea. However, it turned out then that it would have meant adding more
cases of ioport being allowed than it would have removed the opposite. It
might have changed since then. Either way it's annoying.

No I just looked at it too. We have 19 platforms, 7 define
check_legacy_ioport(), 6 of those return ENODEV, one does something
useful. Which leaves 12 platforms that get the default "return 0"
behaviour.

I still don't like it though, having to opt _out_ of random io port
poking is not nice.

Well, we need to do what I suggested I think, that is have a default
helper that checks the device-tree. Only platforms that do have legacy
ISA crap -and- don't have them properly in the device-tree would then
need to provide their own.

Ben.

On Fri, 2007-03-30 at 11:43 +1000, Michael Ellerman wrote:

No I just looked at it too. We have 19 platforms, 7 define
check_legacy_ioport(), 6 of those return ENODEV, one does something
useful. Which leaves 12 platforms that get the default "return 0"
behaviour.

I still don't like it though, having to opt _out_ of random io port
poking is not nice. 

Of those 12 platforms which get the default behaviour, how many of them
_want_ it? And how many, like PS3, are buggy?

-- 
dwmw2

I was thinking that we could have one single helper shared by most
platforms who allow that legacy ISA crap which checks the device-tree
for known ISA devices. That would cover PREP, CHRP and pSeries at
least...

Make the helper check every device node under the ISA
node and translate the "reg" in there.  If the requested
legacy I/O region is a subset of the union of all those,
allow the access, otherwise not.  This should cover all
cases (well at least those with a valid device tree, heh).


Segher

David Woodhouse wrote:

It doesn't like it when we poke at legacy I/O ports like the i8042. We
should probably change the _default_ to return -ENODEV, and let any
strange platform which wants to allow unfettered access provide its own
function for that.

I added this one to ps3-linux.git until support for a platform default
routine is made.

-Geoff