[PATCH] fix prom revision checks in SN kernel

[PATCH] fix prom revision checks in SN kernel

[Aaron Young]

The following patch fixes a few spots in the SN kernel
that check a fixed prom revision number to determine prom
feature support. These checks are only valid on shub1 systems.
They are invalid on shub2 systems which have a different prom
with different revision numbers.

Signed-off-by: Aaron Young <ayoung@sgi.com>


Index: linux/arch/ia64/sn/kernel/setup.c
=================================--- linux.orig/arch/ia64/sn/kernel/setup.c	2006-04-11 00:33:26.000000000 -0500
+++ linux/arch/ia64/sn/kernel/setup.c	2006-06-26 14:33:33.674551229 -0500
@@ -458,7 +458,7 @@
 	 * support here so we don't have to listen to failed keyboard probe
 	 * messages.
 	 */
-	if (version <= 0x0209 && acpi_kbd_controller_present) {
+	if (is_shub1() && version <= 0x0209 && acpi_kbd_controller_present) {
 		printk(KERN_INFO "Disabling legacy keyboard support as prom "
 		       "is too old and doesn't provide FADT\n");
 		acpi_kbd_controller_present = 0;
Index: linux/arch/ia64/sn/pci/tioca_provider.c
=================================--- linux.orig/arch/ia64/sn/pci/tioca_provider.c	2006-04-11 00:33:26.000000000 -0500
+++ linux/arch/ia64/sn/pci/tioca_provider.c	2006-06-26 14:52:26.166211993 -0500
@@ -589,7 +589,7 @@
 
 	/* sanity check prom rev */
 
-	if (sn_sal_rev() < 0x0406) {
+	if (is_shub1() && sn_sal_rev() < 0x0406) {
 		printk
 		    (KERN_ERR "%s:  SGI prom rev 4.06 or greater required "
 		     "for tioca support\n", __FUNCTION__);
Index: linux/drivers/pci/hotplug/sgi_hotplug.c
=================================--- linux.orig/drivers/pci/hotplug/sgi_hotplug.c	2006-04-11 00:33:26.000000000 -0500
+++ linux/drivers/pci/hotplug/sgi_hotplug.c	2006-06-26 14:53:15.693480748 -0500
@@ -553,7 +553,7 @@
 	int rc;
 	int registered = 0;
 
-	if (sn_sal_rev() < SGI_HOTPLUG_PROM_REV) {
+	if (is_shub1() && sn_sal_rev() < SGI_HOTPLUG_PROM_REV) {
 		printk(KERN_ERR "%s: PROM version must be greater than 4.30\n",
 		       __FUNCTION__);
 		return -EPERM;

RE: [PATCH] fix prom revision checks in SN kernel

[Luck, Tony]

+	if (is_shub1() && version <= 0x0209 && acpi_kbd_controller_present) {
 ...
+	if (is_shub1() && sn_sal_rev() < 0x0406) {
 ...
+	if (is_shub1() && sn_sal_rev() < SGI_HOTPLUG_PROM_REV) {


Is it time to come up with a more elegant way of doing this?  One
place you have a define for the special revision, others you hard-wire
the hex numbers.  Possibly at some point in the future you might
have some dividing line in prom revisions for shub2 systems, so
if you continue in this path the tests will just get uglier.

Maybe some inline functions that hide things:

    if (sn_sal_supports_fadt() && acpi_kbd_controller_present) {

    if (sn_sal_supports_tioca()) {

    if (sn_sal_support_hotplug()) {

-Tony

Re: [PATCH] fix prom revision checks in SN kernel

[Aaron Young]

 Yeah, I didn't go for any clean up. We *shouldn't* be adding
 any more explicit prom rev. checks to the code because we
 now have an interface to check for prom "features" - see
 sn_prom_feature_available().

 Unfortunately these checks were added before the interface
 was available and therefore the checks have to stay.

 I can still do some clean up if preferred.

 -Aaron

> 
> +	if (is_shub1() && version <= 0x0209 && acpi_kbd_controller_present) {
>  ...
> +	if (is_shub1() && sn_sal_rev() < 0x0406) {
>  ...
> +	if (is_shub1() && sn_sal_rev() < SGI_HOTPLUG_PROM_REV) {
> 
> 
> Is it time to come up with a more elegant way of doing this?  One
> place you have a define for the special revision, others you hard-wire
> the hex numbers.  Possibly at some point in the future you might
> have some dividing line in prom revisions for shub2 systems, so
> if you continue in this path the tests will just get uglier.
> 
> Maybe some inline functions that hide things:
> 
>     if (sn_sal_supports_fadt() && acpi_kbd_controller_present) {
> 
>     if (sn_sal_supports_tioca()) {
> 
>     if (sn_sal_support_hotplug()) {
> 
> -Tony
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

RE: [PATCH] fix prom revision checks in SN kernel

[Luck, Tony]

> Yeah, I didn't go for any clean up. We *shouldn't* be adding
> any more explicit prom rev. checks to the code because we
> now have an interface to check for prom "features" - see
> sn_prom_feature_available().

Recollections of that in the hind-brain prompted me to think
that there should be a better way.

> Unfortunately these checks were added before the interface
> was available and therefore the checks have to stay.

Makes sense.

> I can still do some clean up if preferred.

No, 3-places doesn't really justify invention of a generic
method ... and since you've already got a better method that
will prevent this from spreading elsewhere, I'll take it
as-is.

-Tony

Re: [PATCH] fix prom revision checks in SN kernel

[Aaron Young]

> 
> > Yeah, I didn't go for any clean up. We *shouldn't* be adding
> > any more explicit prom rev. checks to the code because we
> > now have an interface to check for prom "features" - see
> > sn_prom_feature_available().
> 
> Recollections of that in the hind-brain prompted me to think
> that there should be a better way.
> 
> > Unfortunately these checks were added before the interface
> > was available and therefore the checks have to stay.
> 
> Makes sense.
> 
> > I can still do some clean up if preferred.
> 
> No, 3-places doesn't really justify invention of a generic
> method ... and since you've already got a better method that
> will prevent this from spreading elsewhere, I'll take it
> as-is.
> 

 Tony, hold up. I was just informed there is a patch in-play which
 will conflict with this one:

http://marc.theaimsgroup.com/?l=linux-ia64&m\x113944733900687&w=2

 It removes the prom rev. check from
 drivers/pci/hotplug/sgi_hotplug.c replacing it with
 a sn_prom_feature_available() call. It's obviously
 the preferrable code.

 Should I resubmit without a change to drivers/pci/hotplug/sgi_hotplug.c?

 Thanks,

 -Aaron

RE: [PATCH] fix prom revision checks in SN kernel

[Luck, Tony]

> Tony, hold up. I was just informed there is a patch in-play which
> will conflict with this one:
>
> http://marc.theaimsgroup.com/?l=linux-ia64&m\x113944733900687&w=2
>
> It removes the prom rev. check from
> drivers/pci/hotplug/sgi_hotplug.c replacing it with
> a sn_prom_feature_available() call. It's obviously
> the preferrable code.

That's not in my queue (he said in a slight panic) ... my recollection
of the "tollhouse" patches was that there was an ia64 bit (that I've
already sent to Linus), and some pci stuff that Prarit was going to
push to Greg.

> Should I resubmit without a change to drivers/pci/hotplug/sgi_hotplug.c?

Yes, please re-spin without this hunk (I'm feeling lazy today).

-Tony

Re: [PATCH] fix prom revision checks in SN kernel

[Prarit Bhargava]

 ...
+	if (is_shub1() && sn_sal_rev() < SGI_HOTPLUG_PROM_REV) {

... asm/sn/sn_feature_sets.h, line 33 ...

P.
>   

Re: [PATCH] fix prom revision checks in SN kernel

[Prarit Bhargava]

>
> Index: linux/drivers/pci/hotplug/sgi_hotplug.c
> =================================> --- linux.orig/drivers/pci/hotplug/sgi_hotplug.c	2006-04-11 00:33:26.000000000 -0500
> +++ linux/drivers/pci/hotplug/sgi_hotplug.c	2006-06-26 14:53:15.693480748 -0500
> @@ -553,7 +553,7 @@
>  	int rc;
>  	int registered = 0;
>  
> -	if (sn_sal_rev() < SGI_HOTPLUG_PROM_REV) {
> +	if (is_shub1() && sn_sal_rev() < SGI_HOTPLUG_PROM_REV) {
>  		printk(KERN_ERR "%s: PROM version must be greater than 4.30\n",
>  		       __FUNCTION__);
>  		return -EPERM;
> -
This should probably go to pcihpd-discuss ... But, I recall changing 
this to use feature sets at some point.  I'm surprised it is still in 
the code.

Greg, I cannot recall where you queue your patches -- would you mind 
fwd'ing a link?

Thanks,

P.
>   

Re: [PATCH] fix prom revision checks in SN kernel

[Prarit Bhargava]

>
>
> Greg, I cannot recall where you queue your patches -- would you mind 
> fwd'ing a link?
>
Oops.  Sorry everyone -- this looks like it was resolved on linux-ia64 
(I just checked the marc archive)...

Sorry 'bout that,

P.

> Thanks,
>
> P.
>>   
>