Hi Greg,
Attached is a patch against 2.4.20 (should apply to .21-pre3 and BK-current as
well) that adds 66/100/133MHz PCI-X support to the Compaq Hotplug driver.
Please apply.
Thanks,
Torben Mathiasen
On Wed, Jan 15, 2003 at 10:55:13AM +0100, Torben Mathiasen wrote:
> Hi Greg,
>
> Attached is a patch against 2.4.20 (should apply to .21-pre3 and BK-current as
> well) that adds 66/100/133MHz PCI-X support to the Compaq Hotplug driver.
>
> Please apply.
Looks almost ready. Could you make up a 2.5 version first? I don't
like to have new features in 2.4 before they go into 2.5.
> /* inline functions */
> -
> +extern inline struct slot *find_slot (struct controller *ctrl, u8 device);
Can you just make this a normal function then, with a more private name?
> +/*
> + * get_controller_speed - find the current frequency/mode of controller.
> + *
> + * @ctrl: controller to get frequency/mode for.
> + *
> + * Returns controller speed.
> + *
> + */
> static inline u8 get_controller_speed (struct controller *ctrl)
Thanks for documenting this and get_adapter_speed().
> +static char *get_speed_string (int speed)
> +{
> + switch(speed) {
> + case(PCI_SPEED_33MHz):
> + return "33MHz PCI";
> + case(PCI_SPEED_66MHz):
> + return "66MHz PCI";
> + case(PCI_SPEED_50MHz_PCIX):
> + return "50MHz PCI-X";
> + case(PCI_SPEED_66MHz_PCIX):
> + return "66MHz PCI-X";
> + case(PCI_SPEED_100MHz_PCIX):
> + return "100MHz PCI-X";
> + case(PCI_SPEED_133MHz_PCIX):
> + return "133MHz PCI-X";
> + default:
> + return "UNKNOWN";
> + }
> +}
Ick, why? Just for a debugging message? That /proc file is on the
short list of things to delete :)
> --- linux-2.4.20/drivers/hotplug/pci_hotplug.h Thu Nov 28 17:53:13 2002
> +++ linux-2.4.20-pcix/drivers/hotplug/pci_hotplug.h Mon Jan 6 22:54:47 2003
> @@ -33,9 +33,10 @@
> enum pci_bus_speed {
> PCI_SPEED_33MHz = 0x00,
> PCI_SPEED_66MHz = 0x01,
> - PCI_SPEED_66MHz_PCIX = 0x02,
> - PCI_SPEED_100MHz_PCIX = 0x03,
> - PCI_SPEED_133MHz_PCIX = 0x04,
> + PCI_SPEED_50MHz_PCIX = 0x02,
> + PCI_SPEED_66MHz_PCIX = 0x03,
> + PCI_SPEED_100MHz_PCIX = 0x04,
> + PCI_SPEED_133MHz_PCIX = 0x05,
> PCI_SPEED_66MHz_PCIX_266 = 0x09,
> PCI_SPEED_100MHz_PCIX_266 = 0x0a,
> PCI_SPEED_133MHz_PCIX_266 = 0x0b,
Where are you getting the PCI_SPEED_50MHz_PCIX value from? I took these
values from the Hotplug PCI draft spec. Has 02 been reserved for 50MHz
PCIX and the other values changed?
If it's not in the spec, I'd recommend adding it to the end of the list,
with a big comment about why it's different from the spec values.
thanks,
greg k-h
Sure. I started out doing the patch for 2.5, but hit some hotplug bugs so I
decided to get it working for 2.4 first and then port it to 2.5. I'll get on
that.
A few comments below.
> > +static char *get_speed_string (int speed)
> > +{
> > + switch(speed) {
> > + case(PCI_SPEED_33MHz):
> > + return "33MHz PCI";
> > + case(PCI_SPEED_66MHz):
> > + return "66MHz PCI";
> > + case(PCI_SPEED_50MHz_PCIX):
> > + return "50MHz PCI-X";
> > + case(PCI_SPEED_66MHz_PCIX):
> > + return "66MHz PCI-X";
> > + case(PCI_SPEED_100MHz_PCIX):
> > + return "100MHz PCI-X";
> > + case(PCI_SPEED_133MHz_PCIX):
> > + return "133MHz PCI-X";
> > + default:
> > + return "UNKNOWN";
> > + }
> > +}
>
> Ick, why? Just for a debugging message? That /proc file is on the
> short list of things to delete :)
>
I wanted the user to be able to know exactly which bus speed/mode the driver
switched to in case of a freq/mode change. Besides that it also put the info in
proc as you mention.
> > --- linux-2.4.20/drivers/hotplug/pci_hotplug.h Thu Nov 28 17:53:13 2002
> > +++ linux-2.4.20-pcix/drivers/hotplug/pci_hotplug.h Mon Jan 6 22:54:47 2003
> > @@ -33,9 +33,10 @@
> > enum pci_bus_speed {
> > PCI_SPEED_33MHz = 0x00,
> > PCI_SPEED_66MHz = 0x01,
> > - PCI_SPEED_66MHz_PCIX = 0x02,
> > - PCI_SPEED_100MHz_PCIX = 0x03,
> > - PCI_SPEED_133MHz_PCIX = 0x04,
> > + PCI_SPEED_50MHz_PCIX = 0x02,
> > + PCI_SPEED_66MHz_PCIX = 0x03,
> > + PCI_SPEED_100MHz_PCIX = 0x04,
> > + PCI_SPEED_133MHz_PCIX = 0x05,
> > PCI_SPEED_66MHz_PCIX_266 = 0x09,
> > PCI_SPEED_100MHz_PCIX_266 = 0x0a,
> > PCI_SPEED_133MHz_PCIX_266 = 0x0b,
>
> Where are you getting the PCI_SPEED_50MHz_PCIX value from? I took these
> values from the Hotplug PCI draft spec. Has 02 been reserved for 50MHz
> PCIX and the other values changed?
>
> If it's not in the spec, I'd recommend adding it to the end of the list,
> with a big comment about why it's different from the spec values.
>
Sure, we used to ship a system that only supported 50MHz PCI-X, but I'll have
to get more details on that.
Torben
On Thu, Jan 16, 2003 at 12:47:17PM +0100, Torben Mathiasen wrote:
> Sure. I started out doing the patch for 2.5, but hit some hotplug bugs so I
> decided to get it working for 2.4 first and then port it to 2.5. I'll get on
> that.
Thanks.
> I wanted the user to be able to know exactly which bus speed/mode the driver
> switched to in case of a freq/mode change. Besides that it also put the info in
> proc as you mention.
But that is what the cur_bus_speed and max_bus_speed files in pcihpfs
for the slot are for, right? Isn't this just duplicating that
information?
> Sure, we used to ship a system that only supported 50MHz PCI-X, but I'll have
> to get more details on that.
So 50MHz PCI-X is not in the spec, right? If you all supported it, why
didn't you get it included in the spec? :)
thanks,
greg k-h