<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
On 11.03.2009 13:47 Uhr, Carl-Daniel Hailfinger wrote:
<blockquote cite="mid:49B7B2D1.7000706@gmx.net" type="cite">
<pre wrap="">I don't know the code well, so this is not a real review. Just a nitpick.
On 11.03.2009 13:23, Stefan Reinauer wrote:
</pre>
<blockquote type="cite">
<pre wrap="">+static void smbus_set_subsystem(device_t dev, unsigned vendor, unsigned device)
+{
+ if (!vendor || !device) {
</pre>
</blockquote>
<pre wrap=""><!---->
This case should output a diagnostic message at SPEW or DEBUG level.
</pre>
</blockquote>
I don't think this is a good idea. It's a perfectly fine case that we
don't want to warn about. Plus, the calling function already does a
debug level print in the case that a subsystem function is specified.<br>
<br>
<blockquote cite="mid:49B7B2D1.7000706@gmx.net" type="cite">
<blockquote type="cite">
<pre wrap="">+ pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID,
+ pci_read_config32(dev, 0));
</pre>
</blockquote>
<pre wrap=""><!---->
PCI_VENDOR_ID instead of 0 for consistency.
</pre>
</blockquote>
<br>
Good point.<br>
<br>
<br>
<blockquote cite="mid:49B7B2D1.7000706@gmx.net" type="cite">
<pre wrap="">
</pre>
<blockquote type="cite">
<pre wrap="">+ } else {
+ pci_write_config32(dev, PCI_SUBSYSTEM_VENDOR_ID,
+ ((device & 0xffff) << 16) | (vendor & 0xffff));
+ }
+}
</pre>
</blockquote>
<pre wrap=""><!---->
Same for the other *_set_subsystem. Maybe we even want a generic
function to handle this since the *_set_subsystem functions in the patch
seem to be identical.
</pre>
</blockquote>
Yes, mostly. I was going to look into that as a next step. So far I
only found information from intel that they suggest this behavior, so I
didn't want to make it a global rule.<br>
<br>
<br>
<pre class="moz-signature" cols="72">--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: <a class="moz-txt-link-abbreviated" href="mailto:info@coresystems.de">info@coresystems.de</a> • <a class="moz-txt-link-freetext" href="http://www.coresystems.de/">http://www.coresystems.de/</a>
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
</pre>
</body>
</html>