[PATCH] Implement check_legacy_ioport() on PS3

[PATCH] Implement check_legacy_ioport() on PS3

[David Woodhouse]

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

Re: [PATCH] Implement check_legacy_ioport() on PS3

[Olof Johansson]

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

Re: [PATCH] Implement check_legacy_ioport() on PS3

[David Woodhouse]

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

Re: [Cbe-oss-dev] [PATCH] Implement check_legacy_ioport() on PS3

[Michael Ellerman]

[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]

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:
> > 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [Cbe-oss-dev] [PATCH] Implement check_legacy_ioport() on PS3

[Benjamin Herrenschmidt]

On Fri, 2007-03-30 at 11:43 +1000, Michael Ellerman wrote:
> 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:
> > > 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.

Re: [Cbe-oss-dev] [PATCH] Implement check_legacy_ioport() on PS3

[David Woodhouse]

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

Re: [Cbe-oss-dev] [PATCH] Implement check_legacy_ioport() on PS3

[Benjamin Herrenschmidt]

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:
> > 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.

Re: [Cbe-oss-dev] [PATCH] Implement check_legacy_ioport() on PS3

[Segher Boessenkool]

> 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

Re: [Cbe-oss-dev] [PATCH] Implement check_legacy_ioport() on PS3

[Geoff Levand]

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