Hi,
attached patch changes out/in combinations to pci_read/write_byte in sis630 chipset enable.
-- Alex
On Sat, Sep 08, 2007 at 04:49:46PM +0200, Alex Beregszaszi wrote:
attached patch changes out/in combinations to pci_read/write_byte in sis630 chipset enable.
Why?
Did you test the patch on hardware? Was the code broken before and works after the patch? If not, why change it?
Uwe.
I don't think this should be done unless someone can test. It looks right to me, but ... well ... hardware, you know :-)
ron
On Sat, Sep 08, 2007 at 07:56:32PM +0200, Uwe Hermann wrote:
On Sat, Sep 08, 2007 at 04:49:46PM +0200, Alex Beregszaszi wrote:
attached patch changes out/in combinations to pci_read/write_byte in sis630 chipset enable.
Why?
There is abstraction available - so it should be used everywhere.
Did you test the patch on hardware?
On Sat, Sep 08, 2007 at 10:59:39AM -0700, ron minnich wrote:
I don't think this should be done unless someone can test.
I agree with the patch - I want the change, but I also agree it would be nice to have it tested.
We could argue that we decided to go through with the patch in order to improve the code and until someone has a problem with the code not working there is no real problem.
//Peter
On Sat, Sep 08, 2007 at 08:41:18PM +0200, Peter Stuge wrote:
On Sat, Sep 08, 2007 at 07:56:32PM +0200, Uwe Hermann wrote:
On Sat, Sep 08, 2007 at 04:49:46PM +0200, Alex Beregszaszi wrote:
attached patch changes out/in combinations to pci_read/write_byte in sis630 chipset enable.
Why?
There is abstraction available - so it should be used everywhere.
Ah, yes. I missed the fact that those functions do the same as the replaced code.
Did you test the patch on hardware?
On Sat, Sep 08, 2007 at 10:59:39AM -0700, ron minnich wrote:
I don't think this should be done unless someone can test.
I agree with the patch - I want the change, but I also agree it would be nice to have it tested.
We could argue that we decided to go through with the patch in order to improve the code and until someone has a problem with the code not working there is no real problem.
I'm not too eager to find out the hard way. This is low-level enough that I think there _might_ be unintended/unnoticed consequences.
Anybody with such a chipset willing to test the patch?
Uwe.
On Sun, 2007-09-09 at 22:11 +0200, Uwe Hermann wrote:
On Sat, Sep 08, 2007 at 08:41:18PM +0200, Peter Stuge wrote:
On Sat, Sep 08, 2007 at 07:56:32PM +0200, Uwe Hermann wrote:
On Sat, Sep 08, 2007 at 04:49:46PM +0200, Alex Beregszaszi wrote:
attached patch changes out/in combinations to pci_read/write_byte in sis630 chipset enable.
Why?
There is abstraction available - so it should be used everywhere.
Ah, yes. I missed the fact that those functions do the same as the replaced code.
Did you test the patch on hardware?
On Sat, Sep 08, 2007 at 10:59:39AM -0700, ron minnich wrote:
I don't think this should be done unless someone can test.
I agree with the patch - I want the change, but I also agree it would be nice to have it tested.
We could argue that we decided to go through with the patch in order to improve the code and until someone has a problem with the code not working there is no real problem.
I'm not too eager to find out the hard way. This is low-level enough that I think there _might_ be unintended/unnoticed consequences.
Anybody with such a chipset willing to test the patch?
It was too long ago for me to remember why I was doing something like that. My guess is that the pci_dev passed to the function is the northbridge but you have to program the southbridge or the LPC or it was not linked with pci_util then.
Ollie
* Li-Ta Lo ollie@lanl.gov [070910 19:24]:
I'm not too eager to find out the hard way. This is low-level enough that I think there _might_ be unintended/unnoticed consequences.
Anybody with such a chipset willing to test the patch?
It was too long ago for me to remember why I was doing something like that. My guess is that the pci_dev passed to the function is the northbridge but you have to program the southbridge or the LPC or it was not linked with pci_util then.
Not linking against libpci was indeed the issue that caused this. I think we should check the code in like that.
Stefan
On Tue, Sep 11, 2007 at 11:53:58AM +0200, Stefan Reinauer wrote:
- Li-Ta Lo ollie@lanl.gov [070910 19:24]:
I'm not too eager to find out the hard way. This is low-level enough that I think there _might_ be unintended/unnoticed consequences.
Anybody with such a chipset willing to test the patch?
It was too long ago for me to remember why I was doing something like that. My guess is that the pci_dev passed to the function is the northbridge but you have to program the southbridge or the LPC or it was not linked with pci_util then.
Not linking against libpci was indeed the issue that caused this. I think we should check the code in like that.
OK then, r2770.
It would still be nice if somebody could test this on hardware, though.
Uwe.
-
Alex Beregszaszi -
Li-Ta Lo -
Peter Stuge -
ron minnich -
Stefan Reinauer -
Uwe Hermann