[PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Timothy Pearson]

The mpt2sas driver crashes if the BIOS does not set up at least one
memory I/O resource.  This failure can happen if the device is too
slow to respond during POST and is missed by the BIOS, but Linux
then detects the device later in the boot process.

This patch aborts initialization and prints a warning if no memory I/O
resources are found.

Signed-off-by: Timothy Pearson <tpearson@raptorengineeringinc.com>
Tested-by: Timothy Pearson <tpearson@raptorengineeringinc.com>
---
  drivers/scsi/mpt2sas/mpt2sas_base.c |    9 +++++++++
  1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c 
b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 11248de..15c9504 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -6,6 +6,8 @@
   * Copyright (C) 2007-2014  LSI Corporation
   * Copyright (C) 20013-2014 Avago Technologies
   *  (mailto: MPT-FusionLinux.pdl@avagotech.com)
+ * Copyright (C) 2015 Raptor Engineering
+ *  (mailto: support@araptorengineeringinc.com)
   *
   * This program is free software; you can redistribute it and/or
   * modify it under the terms of the GNU General Public License
@@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER 
*ioc)
  		}
  	}

+	if (ioc->chip == NULL) {
+		printk(MPT2SAS_ERR_FMT "unable to map "
+			"adapter memory (resource not found)!\n", ioc->name);
+		r = -EINVAL;
+		goto out_fail;
+	}
+
  	_base_mask_interrupts(ioc);

  	r = _base_get_ioc_facts(ioc, CAN_SLEEP);
-- 
1.7.9.5

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Timothy Pearson]

On 06/12/2015 05:05 PM, Timothy Pearson wrote:
> The mpt2sas driver crashes if the BIOS does not set up at least one
> memory I/O resource. This failure can happen if the device is too
> slow to respond during POST and is missed by the BIOS, but Linux
> then detects the device later in the boot process.
>
> This patch aborts initialization and prints a warning if no memory I/O
> resources are found.
>
> Signed-off-by: Timothy Pearson <tpearson@raptorengineeringinc.com>
> Tested-by: Timothy Pearson <tpearson@raptorengineeringinc.com>
> ---
> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
> b/drivers/scsi/mpt2sas/mpt2sas_base.c
> index 11248de..15c9504 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
> @@ -6,6 +6,8 @@
> * Copyright (C) 2007-2014 LSI Corporation
> * Copyright (C) 20013-2014 Avago Technologies
> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
> + * Copyright (C) 2015 Raptor Engineering
> + * (mailto: support@araptorengineeringinc.com)
> *
> * This program is free software; you can redistribute it and/or
> * modify it under the terms of the GNU General Public License
> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER
> *ioc)
> }
> }
>
> + if (ioc->chip == NULL) {
> + printk(MPT2SAS_ERR_FMT "unable to map "
> + "adapter memory (resource not found)!\n", ioc->name);
> + r = -EINVAL;
> + goto out_fail;
> + }
> +
> _base_mask_interrupts(ioc);
>
> r = _base_get_ioc_facts(ioc, CAN_SLEEP);

Just following up on this patch as I have not yet received any response.

Thanks!

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Joe Lawrence]

On 06/16/2015 12:28 PM, Timothy Pearson wrote:
> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>> The mpt2sas driver crashes if the BIOS does not set up at least one
>> memory I/O resource. This failure can happen if the device is too
>> slow to respond during POST and is missed by the BIOS, but Linux
>> then detects the device later in the boot process.
>>
>> This patch aborts initialization and prints a warning if no memory I/O
>> resources are found.
>>
>> Signed-off-by: Timothy Pearson <tpearson@raptorengineeringinc.com>
>> Tested-by: Timothy Pearson <tpearson@raptorengineeringinc.com>
>> ---
>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> index 11248de..15c9504 100644
>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> @@ -6,6 +6,8 @@
>> * Copyright (C) 2007-2014 LSI Corporation
>> * Copyright (C) 20013-2014 Avago Technologies
>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
>> + * Copyright (C) 2015 Raptor Engineering
>> + * (mailto: support@araptorengineeringinc.com)
>> *
>> * This program is free software; you can redistribute it and/or
>> * modify it under the terms of the GNU General Public License
>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER
>> *ioc)
>> }
>> }
>>
>> + if (ioc->chip == NULL) {
>> + printk(MPT2SAS_ERR_FMT "unable to map "
>> + "adapter memory (resource not found)!\n", ioc->name);
>> + r = -EINVAL;
>> + goto out_fail;
>> + }
>> +
>> _base_mask_interrupts(ioc);
>>
>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
> 
> Just following up on this patch as I have not yet received any response.
> 
> Thanks!

Hi Tim -- just curious, why was the similar check on ioc->chip just a
few lines above the one added by the patch insufficient?

That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
only sets ioc->chip once.  I wonder if the fix might be simpler if the
existing ioc->chip check relocated entirely to where you put it (maybe
also pulling the entire error text onto one line for easier grepping).

Regards,

-- Joe

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Timothy Pearson]

On 06/16/2015 12:42 PM, Joe Lawrence wrote:
> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>>> The mpt2sas driver crashes if the BIOS does not set up at least one
>>> memory I/O resource. This failure can happen if the device is too
>>> slow to respond during POST and is missed by the BIOS, but Linux
>>> then detects the device later in the boot process.
>>>
>>> This patch aborts initialization and prints a warning if no memory I/O
>>> resources are found.
>>>
>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>> ---
>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
>>> 1 file changed, 9 insertions(+)
>>>
>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>> index 11248de..15c9504 100644
>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>> @@ -6,6 +6,8 @@
>>> * Copyright (C) 2007-2014 LSI Corporation
>>> * Copyright (C) 20013-2014 Avago Technologies
>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
>>> + * Copyright (C) 2015 Raptor Engineering
>>> + * (mailto: support@araptorengineeringinc.com)
>>> *
>>> * This program is free software; you can redistribute it and/or
>>> * modify it under the terms of the GNU General Public License
>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER
>>> *ioc)
>>> }
>>> }
>>>
>>> + if (ioc->chip == NULL) {
>>> + printk(MPT2SAS_ERR_FMT "unable to map "
>>> + "adapter memory (resource not found)!\n", ioc->name);
>>> + r = -EINVAL;
>>> + goto out_fail;
>>> + }
>>> +
>>> _base_mask_interrupts(ioc);
>>>
>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>>
>> Just following up on this patch as I have not yet received any response.
>>
>> Thanks!
>
> Hi Tim -- just curious, why was the similar check on ioc->chip just a
> few lines above the one added by the patch insufficient?
>
> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
> only sets ioc->chip once.  I wonder if the fix might be simpler if the
> existing ioc->chip check relocated entirely to where you put it (maybe
> also pulling the entire error text onto one line for easier grepping).
>
> Regards,
>
> -- Joe

If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if 
the BIOS does not run resource allocation on the mpt2sas device) then 
the check you are referring to is not executed, and the driver attempts 
to perform operations on a null ioc->chip pointer.

I can relocate the check if desired.

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Timothy Pearson]

On 06/16/2015 01:49 PM, Timothy Pearson wrote:
> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
>>>> memory I/O resource. This failure can happen if the device is too
>>>> slow to respond during POST and is missed by the BIOS, but Linux
>>>> then detects the device later in the boot process.
>>>>
>>>> This patch aborts initialization and prints a warning if no memory I/O
>>>> resources are found.
>>>>
>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>> ---
>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
>>>> 1 file changed, 9 insertions(+)
>>>>
>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>> index 11248de..15c9504 100644
>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>> @@ -6,6 +6,8 @@
>>>> * Copyright (C) 2007-2014 LSI Corporation
>>>> * Copyright (C) 20013-2014 Avago Technologies
>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
>>>> + * Copyright (C) 2015 Raptor Engineering
>>>> + * (mailto: support@araptorengineeringinc.com)
>>>> *
>>>> * This program is free software; you can redistribute it and/or
>>>> * modify it under the terms of the GNU General Public License
>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>>>> MPT2SAS_ADAPTER
>>>> *ioc)
>>>> }
>>>> }
>>>>
>>>> + if (ioc->chip == NULL) {
>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
>>>> + "adapter memory (resource not found)!\n", ioc->name);
>>>> + r = -EINVAL;
>>>> + goto out_fail;
>>>> + }
>>>> +
>>>> _base_mask_interrupts(ioc);
>>>>
>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>>>
>>> Just following up on this patch as I have not yet received any response.
>>>
>>> Thanks!
>>
>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
>> few lines above the one added by the patch insufficient?
>>
>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
>> only sets ioc->chip once. I wonder if the fix might be simpler if the
>> existing ioc->chip check relocated entirely to where you put it (maybe
>> also pulling the entire error text onto one line for easier grepping).
>>
>> Regards,
>>
>> -- Joe
>
> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
> the BIOS does not run resource allocation on the mpt2sas device) then
> the check you are referring to is not executed, and the driver attempts
> to perform operations on a null ioc->chip pointer.
>
> I can relocate the check if desired.
>

On looking more closely at the code I think having the two separate 
checks is useful for debugging.  The first error message is triggered if 
the resource exists but Linux cannot map it for some reason, and the 
second error message is triggered if the resource does not exist at all 
(buggy BIOS).

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Joe Lawrence]

On 06/21/2015 02:46 PM, Timothy Pearson wrote:
> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
>>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
>>>>> memory I/O resource. This failure can happen if the device is too
>>>>> slow to respond during POST and is missed by the BIOS, but Linux
>>>>> then detects the device later in the boot process.
>>>>>
>>>>> This patch aborts initialization and prints a warning if no memory I/O
>>>>> resources are found.
>>>>>
>>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>>> ---
>>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
>>>>> 1 file changed, 9 insertions(+)
>>>>>
>>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>> index 11248de..15c9504 100644
>>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>> @@ -6,6 +6,8 @@
>>>>> * Copyright (C) 2007-2014 LSI Corporation
>>>>> * Copyright (C) 20013-2014 Avago Technologies
>>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
>>>>> + * Copyright (C) 2015 Raptor Engineering
>>>>> + * (mailto: support@araptorengineeringinc.com)
>>>>> *
>>>>> * This program is free software; you can redistribute it and/or
>>>>> * modify it under the terms of the GNU General Public License
>>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>>>>> MPT2SAS_ADAPTER
>>>>> *ioc)
>>>>> }
>>>>> }
>>>>>
>>>>> + if (ioc->chip == NULL) {
>>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
>>>>> + "adapter memory (resource not found)!\n", ioc->name);
>>>>> + r = -EINVAL;
>>>>> + goto out_fail;
>>>>> + }
>>>>> +
>>>>> _base_mask_interrupts(ioc);
>>>>>
>>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>>>>
>>>> Just following up on this patch as I have not yet received any
>>>> response.
>>>>
>>>> Thanks!
>>>
>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
>>> few lines above the one added by the patch insufficient?
>>>
>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
>>> existing ioc->chip check relocated entirely to where you put it (maybe
>>> also pulling the entire error text onto one line for easier grepping).
>>>
>>> Regards,
>>>
>>> -- Joe
>>
>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
>> the BIOS does not run resource allocation on the mpt2sas device) then
>> the check you are referring to is not executed, and the driver attempts
>> to perform operations on a null ioc->chip pointer.
>>
>> I can relocate the check if desired.
>>
> 
> On looking more closely at the code I think having the two separate
> checks is useful for debugging.  The first error message is triggered if
> the resource exists but Linux cannot map it for some reason, and the
> second error message is triggered if the resource does not exist at all
> (buggy BIOS).

Fair enough.  The two error messages are unique, so grepping will lead
to the correct check.

Only other nits would be the addition of a copyright (up to Avago I
guess) and an mpt3sas version (these drivers are 99% the same code).

Acked-by: Joe Lawrence <joe.lawrence@stratus.com>

-- Joe

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Sreekanth Reddy]

Hi,

Upstream contributor should not add copyright to this driver code.

Regards,
Sreekanth

On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence <joe.lawrence@stratus.com> wrote:
>
>
> On 06/21/2015 02:46 PM, Timothy Pearson wrote:
>> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
>>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
>>>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>>>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
>>>>>> memory I/O resource. This failure can happen if the device is too
>>>>>> slow to respond during POST and is missed by the BIOS, but Linux
>>>>>> then detects the device later in the boot process.
>>>>>>
>>>>>> This patch aborts initialization and prints a warning if no memory I/O
>>>>>> resources are found.
>>>>>>
>>>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>>>> ---
>>>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
>>>>>> 1 file changed, 9 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>> index 11248de..15c9504 100644
>>>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>> @@ -6,6 +6,8 @@
>>>>>> * Copyright (C) 2007-2014 LSI Corporation
>>>>>> * Copyright (C) 20013-2014 Avago Technologies
>>>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
>>>>>> + * Copyright (C) 2015 Raptor Engineering
>>>>>> + * (mailto: support@araptorengineeringinc.com)
>>>>>> *
>>>>>> * This program is free software; you can redistribute it and/or
>>>>>> * modify it under the terms of the GNU General Public License
>>>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>>>>>> MPT2SAS_ADAPTER
>>>>>> *ioc)
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> + if (ioc->chip == NULL) {
>>>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
>>>>>> + "adapter memory (resource not found)!\n", ioc->name);
>>>>>> + r = -EINVAL;
>>>>>> + goto out_fail;
>>>>>> + }
>>>>>> +
>>>>>> _base_mask_interrupts(ioc);
>>>>>>
>>>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>>>>>
>>>>> Just following up on this patch as I have not yet received any
>>>>> response.
>>>>>
>>>>> Thanks!
>>>>
>>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
>>>> few lines above the one added by the patch insufficient?
>>>>
>>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
>>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
>>>> existing ioc->chip check relocated entirely to where you put it (maybe
>>>> also pulling the entire error text onto one line for easier grepping).
>>>>
>>>> Regards,
>>>>
>>>> -- Joe
>>>
>>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
>>> the BIOS does not run resource allocation on the mpt2sas device) then
>>> the check you are referring to is not executed, and the driver attempts
>>> to perform operations on a null ioc->chip pointer.
>>>
>>> I can relocate the check if desired.
>>>
>>
>> On looking more closely at the code I think having the two separate
>> checks is useful for debugging.  The first error message is triggered if
>> the resource exists but Linux cannot map it for some reason, and the
>> second error message is triggered if the resource does not exist at all
>> (buggy BIOS).
>
> Fair enough.  The two error messages are unique, so grepping will lead
> to the correct check.
>
> Only other nits would be the addition of a copyright (up to Avago I
> guess) and an mpt3sas version (these drivers are 99% the same code).
>
> Acked-by: Joe Lawrence <joe.lawrence@stratus.com>
>
> -- Joe



-- 

Regards,
Sreekanth

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[James Bottomley]

On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
> Hi,
> 
> Upstream contributor should not add copyright to this driver code.

I can't agree with that as a general rule: it depends on the
significance of the contribution.  The somewhat ill defined standard for
this is the contribution must be "an original work of authorship".

I can agree that a trivial bug fix like this is not an original work of
authorship, so in this case adding copyright isn't appropriate.

James

> Regards,
> Sreekanth
> 
> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence <joe.lawrence@stratus.com> wrote:
> >
> >
> > On 06/21/2015 02:46 PM, Timothy Pearson wrote:
> >> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
> >>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
> >>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
> >>>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
> >>>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
> >>>>>> memory I/O resource. This failure can happen if the device is too
> >>>>>> slow to respond during POST and is missed by the BIOS, but Linux
> >>>>>> then detects the device later in the boot process.
> >>>>>>
> >>>>>> This patch aborts initialization and prints a warning if no memory I/O
> >>>>>> resources are found.
> >>>>>>
> >>>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
> >>>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
> >>>>>> ---
> >>>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
> >>>>>> 1 file changed, 9 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>> index 11248de..15c9504 100644
> >>>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>> @@ -6,6 +6,8 @@
> >>>>>> * Copyright (C) 2007-2014 LSI Corporation
> >>>>>> * Copyright (C) 20013-2014 Avago Technologies
> >>>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
> >>>>>> + * Copyright (C) 2015 Raptor Engineering
> >>>>>> + * (mailto: support@araptorengineeringinc.com)
> >>>>>> *
> >>>>>> * This program is free software; you can redistribute it and/or
> >>>>>> * modify it under the terms of the GNU General Public License
> >>>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
> >>>>>> MPT2SAS_ADAPTER
> >>>>>> *ioc)
> >>>>>> }
> >>>>>> }
> >>>>>>
> >>>>>> + if (ioc->chip == NULL) {
> >>>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
> >>>>>> + "adapter memory (resource not found)!\n", ioc->name);
> >>>>>> + r = -EINVAL;
> >>>>>> + goto out_fail;
> >>>>>> + }
> >>>>>> +
> >>>>>> _base_mask_interrupts(ioc);
> >>>>>>
> >>>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
> >>>>>
> >>>>> Just following up on this patch as I have not yet received any
> >>>>> response.
> >>>>>
> >>>>> Thanks!
> >>>>
> >>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
> >>>> few lines above the one added by the patch insufficient?
> >>>>
> >>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
> >>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
> >>>> existing ioc->chip check relocated entirely to where you put it (maybe
> >>>> also pulling the entire error text onto one line for easier grepping).
> >>>>
> >>>> Regards,
> >>>>
> >>>> -- Joe
> >>>
> >>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
> >>> the BIOS does not run resource allocation on the mpt2sas device) then
> >>> the check you are referring to is not executed, and the driver attempts
> >>> to perform operations on a null ioc->chip pointer.
> >>>
> >>> I can relocate the check if desired.
> >>>
> >>
> >> On looking more closely at the code I think having the two separate
> >> checks is useful for debugging.  The first error message is triggered if
> >> the resource exists but Linux cannot map it for some reason, and the
> >> second error message is triggered if the resource does not exist at all
> >> (buggy BIOS).
> >
> > Fair enough.  The two error messages are unique, so grepping will lead
> > to the correct check.
> >
> > Only other nits would be the addition of a copyright (up to Avago I
> > guess) and an mpt3sas version (these drivers are 99% the same code).
> >
> > Acked-by: Joe Lawrence <joe.lawrence@stratus.com>
> >
> > -- Joe
> 
> 
> 

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Sreekanth Reddy]

On Tue, Jun 23, 2015 at 7:05 PM, James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
> On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
>> Hi,
>>
>> Upstream contributor should not add copyright to this driver code.
>
> I can't agree with that as a general rule: it depends on the
> significance of the contribution.  The somewhat ill defined standard for
> this is the contribution must be "an original work of authorship".

Thanks James. I am not aware of this and this is a learning point for
me. As most of the time Avago is the major contributor for this driver
so Initial I thought like that.

Thanks,
Sreekanth

>
> I can agree that a trivial bug fix like this is not an original work of
> authorship, so in this case adding copyright isn't appropriate.
>
> James
>
>> Regards,
>> Sreekanth
>>
>> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence <joe.lawrence@stratus.com> wrote:
>> >
>> >
>> > On 06/21/2015 02:46 PM, Timothy Pearson wrote:
>> >> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
>> >>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>> >>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
>> >>>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>> >>>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
>> >>>>>> memory I/O resource. This failure can happen if the device is too
>> >>>>>> slow to respond during POST and is missed by the BIOS, but Linux
>> >>>>>> then detects the device later in the boot process.
>> >>>>>>
>> >>>>>> This patch aborts initialization and prints a warning if no memory I/O
>> >>>>>> resources are found.
>> >>>>>>
>> >>>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>> >>>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>> >>>>>> ---
>> >>>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
>> >>>>>> 1 file changed, 9 insertions(+)
>> >>>>>>
>> >>>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> >>>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> >>>>>> index 11248de..15c9504 100644
>> >>>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> >>>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> >>>>>> @@ -6,6 +6,8 @@
>> >>>>>> * Copyright (C) 2007-2014 LSI Corporation
>> >>>>>> * Copyright (C) 20013-2014 Avago Technologies
>> >>>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
>> >>>>>> + * Copyright (C) 2015 Raptor Engineering
>> >>>>>> + * (mailto: support@araptorengineeringinc.com)
>> >>>>>> *
>> >>>>>> * This program is free software; you can redistribute it and/or
>> >>>>>> * modify it under the terms of the GNU General Public License
>> >>>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>> >>>>>> MPT2SAS_ADAPTER
>> >>>>>> *ioc)
>> >>>>>> }
>> >>>>>> }
>> >>>>>>
>> >>>>>> + if (ioc->chip == NULL) {
>> >>>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
>> >>>>>> + "adapter memory (resource not found)!\n", ioc->name);
>> >>>>>> + r = -EINVAL;
>> >>>>>> + goto out_fail;
>> >>>>>> + }
>> >>>>>> +
>> >>>>>> _base_mask_interrupts(ioc);
>> >>>>>>
>> >>>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>> >>>>>
>> >>>>> Just following up on this patch as I have not yet received any
>> >>>>> response.
>> >>>>>
>> >>>>> Thanks!
>> >>>>
>> >>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
>> >>>> few lines above the one added by the patch insufficient?
>> >>>>
>> >>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
>> >>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
>> >>>> existing ioc->chip check relocated entirely to where you put it (maybe
>> >>>> also pulling the entire error text onto one line for easier grepping).
>> >>>>
>> >>>> Regards,
>> >>>>
>> >>>> -- Joe
>> >>>
>> >>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
>> >>> the BIOS does not run resource allocation on the mpt2sas device) then
>> >>> the check you are referring to is not executed, and the driver attempts
>> >>> to perform operations on a null ioc->chip pointer.
>> >>>
>> >>> I can relocate the check if desired.
>> >>>
>> >>
>> >> On looking more closely at the code I think having the two separate
>> >> checks is useful for debugging.  The first error message is triggered if
>> >> the resource exists but Linux cannot map it for some reason, and the
>> >> second error message is triggered if the resource does not exist at all
>> >> (buggy BIOS).
>> >
>> > Fair enough.  The two error messages are unique, so grepping will lead
>> > to the correct check.
>> >
>> > Only other nits would be the addition of a copyright (up to Avago I
>> > guess) and an mpt3sas version (these drivers are 99% the same code).
>> >
>> > Acked-by: Joe Lawrence <joe.lawrence@stratus.com>
>> >
>> > -- Joe
>>
>>
>>
>
>
>



-- 

Regards,
Sreekanth

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Timothy Pearson]

On 06/23/2015 08:35 AM, James Bottomley wrote:
> On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
>> Hi,
>>
>> Upstream contributor should not add copyright to this driver code.
>
> I can't agree with that as a general rule: it depends on the
> significance of the contribution.  The somewhat ill defined standard for
> this is the contribution must be "an original work of authorship".
>
> I can agree that a trivial bug fix like this is not an original work of
> authorship, so in this case adding copyright isn't appropriate.
>
> James
>
>> Regards,
>> Sreekanth
>>
>> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence<joe.lawrence@stratus.com>  wrote:
>>>
>>>
>>> On 06/21/2015 02:46 PM, Timothy Pearson wrote:
>>>> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
>>>>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>>>>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
>>>>>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>>>>>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
>>>>>>>> memory I/O resource. This failure can happen if the device is too
>>>>>>>> slow to respond during POST and is missed by the BIOS, but Linux
>>>>>>>> then detects the device later in the boot process.
>>>>>>>>
>>>>>>>> This patch aborts initialization and prints a warning if no memory I/O
>>>>>>>> resources are found.
>>>>>>>>
>>>>>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>>>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>>>>>> ---
>>>>>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
>>>>>>>> 1 file changed, 9 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>> index 11248de..15c9504 100644
>>>>>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>> @@ -6,6 +6,8 @@
>>>>>>>> * Copyright (C) 2007-2014 LSI Corporation
>>>>>>>> * Copyright (C) 20013-2014 Avago Technologies
>>>>>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
>>>>>>>> + * Copyright (C) 2015 Raptor Engineering
>>>>>>>> + * (mailto: support@araptorengineeringinc.com)
>>>>>>>> *
>>>>>>>> * This program is free software; you can redistribute it and/or
>>>>>>>> * modify it under the terms of the GNU General Public License
>>>>>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>>>>>>>> MPT2SAS_ADAPTER
>>>>>>>> *ioc)
>>>>>>>> }
>>>>>>>> }
>>>>>>>>
>>>>>>>> + if (ioc->chip == NULL) {
>>>>>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
>>>>>>>> + "adapter memory (resource not found)!\n", ioc->name);
>>>>>>>> + r = -EINVAL;
>>>>>>>> + goto out_fail;
>>>>>>>> + }
>>>>>>>> +
>>>>>>>> _base_mask_interrupts(ioc);
>>>>>>>>
>>>>>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>>>>>>>
>>>>>>> Just following up on this patch as I have not yet received any
>>>>>>> response.
>>>>>>>
>>>>>>> Thanks!
>>>>>>
>>>>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
>>>>>> few lines above the one added by the patch insufficient?
>>>>>>
>>>>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
>>>>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
>>>>>> existing ioc->chip check relocated entirely to where you put it (maybe
>>>>>> also pulling the entire error text onto one line for easier grepping).
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> -- Joe
>>>>>
>>>>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
>>>>> the BIOS does not run resource allocation on the mpt2sas device) then
>>>>> the check you are referring to is not executed, and the driver attempts
>>>>> to perform operations on a null ioc->chip pointer.
>>>>>
>>>>> I can relocate the check if desired.
>>>>>
>>>>
>>>> On looking more closely at the code I think having the two separate
>>>> checks is useful for debugging.  The first error message is triggered if
>>>> the resource exists but Linux cannot map it for some reason, and the
>>>> second error message is triggered if the resource does not exist at all
>>>> (buggy BIOS).
>>>
>>> Fair enough.  The two error messages are unique, so grepping will lead
>>> to the correct check.
>>>
>>> Only other nits would be the addition of a copyright (up to Avago I
>>> guess) and an mpt3sas version (these drivers are 99% the same code).
>>>
>>> Acked-by: Joe Lawrence<joe.lawrence@stratus.com>
>>>
>>> -- Joe
>>
>>
>>
>

Thank you for clarifying the guidelines on copyright lines.  I wasn't 
sure if this was significant enough of a contribution to justify the 
line or not.

Do I need to re-send or can I just state here that the patch can be 
merged without the copyright line?

I don't have any mpt3sas hardware to test with; should I just send in a 
patch anyway without a Tested-by line?

Thanks!

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[James Bottomley]

On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:
> On 06/23/2015 08:35 AM, James Bottomley wrote:
> > On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
> >> Hi,
> >>
> >> Upstream contributor should not add copyright to this driver code.
> >
> > I can't agree with that as a general rule: it depends on the
> > significance of the contribution.  The somewhat ill defined standard for
> > this is the contribution must be "an original work of authorship".
> >
> > I can agree that a trivial bug fix like this is not an original work of
> > authorship, so in this case adding copyright isn't appropriate.
> >
> > James
> >
> >> Regards,
> >> Sreekanth
> >>
> >> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence<joe.lawrence@stratus.com>  wrote:
> >>>
> >>>
> >>> On 06/21/2015 02:46 PM, Timothy Pearson wrote:
> >>>> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
> >>>>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
> >>>>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
> >>>>>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
> >>>>>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
> >>>>>>>> memory I/O resource. This failure can happen if the device is too
> >>>>>>>> slow to respond during POST and is missed by the BIOS, but Linux
> >>>>>>>> then detects the device later in the boot process.
> >>>>>>>>
> >>>>>>>> This patch aborts initialization and prints a warning if no memory I/O
> >>>>>>>> resources are found.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
> >>>>>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
> >>>>>>>> ---
> >>>>>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
> >>>>>>>> 1 file changed, 9 insertions(+)
> >>>>>>>>
> >>>>>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>> index 11248de..15c9504 100644
> >>>>>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>> @@ -6,6 +6,8 @@
> >>>>>>>> * Copyright (C) 2007-2014 LSI Corporation
> >>>>>>>> * Copyright (C) 20013-2014 Avago Technologies
> >>>>>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
> >>>>>>>> + * Copyright (C) 2015 Raptor Engineering
> >>>>>>>> + * (mailto: support@araptorengineeringinc.com)
> >>>>>>>> *
> >>>>>>>> * This program is free software; you can redistribute it and/or
> >>>>>>>> * modify it under the terms of the GNU General Public License
> >>>>>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
> >>>>>>>> MPT2SAS_ADAPTER
> >>>>>>>> *ioc)
> >>>>>>>> }
> >>>>>>>> }
> >>>>>>>>
> >>>>>>>> + if (ioc->chip == NULL) {
> >>>>>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
> >>>>>>>> + "adapter memory (resource not found)!\n", ioc->name);
> >>>>>>>> + r = -EINVAL;
> >>>>>>>> + goto out_fail;
> >>>>>>>> + }
> >>>>>>>> +
> >>>>>>>> _base_mask_interrupts(ioc);
> >>>>>>>>
> >>>>>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
> >>>>>>>
> >>>>>>> Just following up on this patch as I have not yet received any
> >>>>>>> response.
> >>>>>>>
> >>>>>>> Thanks!
> >>>>>>
> >>>>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
> >>>>>> few lines above the one added by the patch insufficient?
> >>>>>>
> >>>>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
> >>>>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
> >>>>>> existing ioc->chip check relocated entirely to where you put it (maybe
> >>>>>> also pulling the entire error text onto one line for easier grepping).
> >>>>>>
> >>>>>> Regards,
> >>>>>>
> >>>>>> -- Joe
> >>>>>
> >>>>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
> >>>>> the BIOS does not run resource allocation on the mpt2sas device) then
> >>>>> the check you are referring to is not executed, and the driver attempts
> >>>>> to perform operations on a null ioc->chip pointer.
> >>>>>
> >>>>> I can relocate the check if desired.
> >>>>>
> >>>>
> >>>> On looking more closely at the code I think having the two separate
> >>>> checks is useful for debugging.  The first error message is triggered if
> >>>> the resource exists but Linux cannot map it for some reason, and the
> >>>> second error message is triggered if the resource does not exist at all
> >>>> (buggy BIOS).
> >>>
> >>> Fair enough.  The two error messages are unique, so grepping will lead
> >>> to the correct check.
> >>>
> >>> Only other nits would be the addition of a copyright (up to Avago I
> >>> guess) and an mpt3sas version (these drivers are 99% the same code).
> >>>
> >>> Acked-by: Joe Lawrence<joe.lawrence@stratus.com>
> >>>
> >>> -- Joe
> >>
> >>
> >>
> >
> 
> Thank you for clarifying the guidelines on copyright lines.  I wasn't 
> sure if this was significant enough of a contribution to justify the 
> line or not.
> 
> Do I need to re-send or can I just state here that the patch can be 
> merged without the copyright line?

It's easier if you send a clean copy rather than risk me botching
something in the hand edit.

> I don't have any mpt3sas hardware to test with; should I just send in a 
> patch anyway without a Tested-by line?

Well, if you haven't actually tested it, adding a tested-by line
wouldn't be correct.

James

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Timothy Pearson]

On 06/23/2015 12:45 PM, James Bottomley wrote:
> On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:
>> On 06/23/2015 08:35 AM, James Bottomley wrote:
>>> On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
>>>> Hi,
>>>>
>>>> Upstream contributor should not add copyright to this driver code.
>>>
>>> I can't agree with that as a general rule: it depends on the
>>> significance of the contribution.  The somewhat ill defined standard for
>>> this is the contribution must be "an original work of authorship".
>>>
>>> I can agree that a trivial bug fix like this is not an original work of
>>> authorship, so in this case adding copyright isn't appropriate.
>>>
>>> James
>>>
>>>> Regards,
>>>> Sreekanth
>>>>
>>>> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence<joe.lawrence@stratus.com>   wrote:
>>>>>
>>>>>
>>>>> On 06/21/2015 02:46 PM, Timothy Pearson wrote:
>>>>>> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
>>>>>>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>>>>>>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
>>>>>>>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>>>>>>>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
>>>>>>>>>> memory I/O resource. This failure can happen if the device is too
>>>>>>>>>> slow to respond during POST and is missed by the BIOS, but Linux
>>>>>>>>>> then detects the device later in the boot process.
>>>>>>>>>>
>>>>>>>>>> This patch aborts initialization and prints a warning if no memory I/O
>>>>>>>>>> resources are found.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>>>>>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>>>>>>>> ---
>>>>>>>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
>>>>>>>>>> 1 file changed, 9 insertions(+)
>>>>>>>>>>
>>>>>>>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>>>> index 11248de..15c9504 100644
>>>>>>>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>>>> @@ -6,6 +6,8 @@
>>>>>>>>>> * Copyright (C) 2007-2014 LSI Corporation
>>>>>>>>>> * Copyright (C) 20013-2014 Avago Technologies
>>>>>>>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
>>>>>>>>>> + * Copyright (C) 2015 Raptor Engineering
>>>>>>>>>> + * (mailto: support@araptorengineeringinc.com)
>>>>>>>>>> *
>>>>>>>>>> * This program is free software; you can redistribute it and/or
>>>>>>>>>> * modify it under the terms of the GNU General Public License
>>>>>>>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>>>>>>>>>> MPT2SAS_ADAPTER
>>>>>>>>>> *ioc)
>>>>>>>>>> }
>>>>>>>>>> }
>>>>>>>>>>
>>>>>>>>>> + if (ioc->chip == NULL) {
>>>>>>>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
>>>>>>>>>> + "adapter memory (resource not found)!\n", ioc->name);
>>>>>>>>>> + r = -EINVAL;
>>>>>>>>>> + goto out_fail;
>>>>>>>>>> + }
>>>>>>>>>> +
>>>>>>>>>> _base_mask_interrupts(ioc);
>>>>>>>>>>
>>>>>>>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>>>>>>>>>
>>>>>>>>> Just following up on this patch as I have not yet received any
>>>>>>>>> response.
>>>>>>>>>
>>>>>>>>> Thanks!
>>>>>>>>
>>>>>>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
>>>>>>>> few lines above the one added by the patch insufficient?
>>>>>>>>
>>>>>>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
>>>>>>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
>>>>>>>> existing ioc->chip check relocated entirely to where you put it (maybe
>>>>>>>> also pulling the entire error text onto one line for easier grepping).
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>>
>>>>>>>> -- Joe
>>>>>>>
>>>>>>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
>>>>>>> the BIOS does not run resource allocation on the mpt2sas device) then
>>>>>>> the check you are referring to is not executed, and the driver attempts
>>>>>>> to perform operations on a null ioc->chip pointer.
>>>>>>>
>>>>>>> I can relocate the check if desired.
>>>>>>>
>>>>>>
>>>>>> On looking more closely at the code I think having the two separate
>>>>>> checks is useful for debugging.  The first error message is triggered if
>>>>>> the resource exists but Linux cannot map it for some reason, and the
>>>>>> second error message is triggered if the resource does not exist at all
>>>>>> (buggy BIOS).
>>>>>
>>>>> Fair enough.  The two error messages are unique, so grepping will lead
>>>>> to the correct check.
>>>>>
>>>>> Only other nits would be the addition of a copyright (up to Avago I
>>>>> guess) and an mpt3sas version (these drivers are 99% the same code).
>>>>>
>>>>> Acked-by: Joe Lawrence<joe.lawrence@stratus.com>
>>>>>
>>>>> -- Joe
>>>>
>>>>
>>>>
>>>
>>
>> Thank you for clarifying the guidelines on copyright lines.  I wasn't
>> sure if this was significant enough of a contribution to justify the
>> line or not.
>>
>> Do I need to re-send or can I just state here that the patch can be
>> merged without the copyright line?
>
> It's easier if you send a clean copy rather than risk me botching
> something in the hand edit.

OK, will do.

>> I don't have any mpt3sas hardware to test with; should I just send in a
>> patch anyway without a Tested-by line?
>
> Well, if you haven't actually tested it, adding a tested-by line
> wouldn't be correct.

Of course not.  My question was, should I send in a patch anyway (with 
appropriate commit message fields) even though it has not been tested?

Thanks!

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[James Bottomley]

On Tue, 2015-06-23 at 12:47 -0500, Timothy Pearson wrote:
> On 06/23/2015 12:45 PM, James Bottomley wrote:
> > On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:
> >> On 06/23/2015 08:35 AM, James Bottomley wrote:
> >>> On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
> >>>> Hi,
> >>>>
> >>>> Upstream contributor should not add copyright to this driver code.
> >>>
> >>> I can't agree with that as a general rule: it depends on the
> >>> significance of the contribution.  The somewhat ill defined standard for
> >>> this is the contribution must be "an original work of authorship".
> >>>
> >>> I can agree that a trivial bug fix like this is not an original work of
> >>> authorship, so in this case adding copyright isn't appropriate.
> >>>
> >>> James
> >>>
> >>>> Regards,
> >>>> Sreekanth
> >>>>
> >>>> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence<joe.lawrence@stratus.com>   wrote:
> >>>>>
> >>>>>
> >>>>> On 06/21/2015 02:46 PM, Timothy Pearson wrote:
> >>>>>> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
> >>>>>>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
> >>>>>>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
> >>>>>>>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
> >>>>>>>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
> >>>>>>>>>> memory I/O resource. This failure can happen if the device is too
> >>>>>>>>>> slow to respond during POST and is missed by the BIOS, but Linux
> >>>>>>>>>> then detects the device later in the boot process.
> >>>>>>>>>>
> >>>>>>>>>> This patch aborts initialization and prints a warning if no memory I/O
> >>>>>>>>>> resources are found.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
> >>>>>>>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
> >>>>>>>>>> ---
> >>>>>>>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
> >>>>>>>>>> 1 file changed, 9 insertions(+)
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>>>> index 11248de..15c9504 100644
> >>>>>>>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>>>> @@ -6,6 +6,8 @@
> >>>>>>>>>> * Copyright (C) 2007-2014 LSI Corporation
> >>>>>>>>>> * Copyright (C) 20013-2014 Avago Technologies
> >>>>>>>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
> >>>>>>>>>> + * Copyright (C) 2015 Raptor Engineering
> >>>>>>>>>> + * (mailto: support@araptorengineeringinc.com)
> >>>>>>>>>> *
> >>>>>>>>>> * This program is free software; you can redistribute it and/or
> >>>>>>>>>> * modify it under the terms of the GNU General Public License
> >>>>>>>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
> >>>>>>>>>> MPT2SAS_ADAPTER
> >>>>>>>>>> *ioc)
> >>>>>>>>>> }
> >>>>>>>>>> }
> >>>>>>>>>>
> >>>>>>>>>> + if (ioc->chip == NULL) {
> >>>>>>>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
> >>>>>>>>>> + "adapter memory (resource not found)!\n", ioc->name);
> >>>>>>>>>> + r = -EINVAL;
> >>>>>>>>>> + goto out_fail;
> >>>>>>>>>> + }
> >>>>>>>>>> +
> >>>>>>>>>> _base_mask_interrupts(ioc);
> >>>>>>>>>>
> >>>>>>>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
> >>>>>>>>>
> >>>>>>>>> Just following up on this patch as I have not yet received any
> >>>>>>>>> response.
> >>>>>>>>>
> >>>>>>>>> Thanks!
> >>>>>>>>
> >>>>>>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
> >>>>>>>> few lines above the one added by the patch insufficient?
> >>>>>>>>
> >>>>>>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
> >>>>>>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
> >>>>>>>> existing ioc->chip check relocated entirely to where you put it (maybe
> >>>>>>>> also pulling the entire error text onto one line for easier grepping).
> >>>>>>>>
> >>>>>>>> Regards,
> >>>>>>>>
> >>>>>>>> -- Joe
> >>>>>>>
> >>>>>>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
> >>>>>>> the BIOS does not run resource allocation on the mpt2sas device) then
> >>>>>>> the check you are referring to is not executed, and the driver attempts
> >>>>>>> to perform operations on a null ioc->chip pointer.
> >>>>>>>
> >>>>>>> I can relocate the check if desired.
> >>>>>>>
> >>>>>>
> >>>>>> On looking more closely at the code I think having the two separate
> >>>>>> checks is useful for debugging.  The first error message is triggered if
> >>>>>> the resource exists but Linux cannot map it for some reason, and the
> >>>>>> second error message is triggered if the resource does not exist at all
> >>>>>> (buggy BIOS).
> >>>>>
> >>>>> Fair enough.  The two error messages are unique, so grepping will lead
> >>>>> to the correct check.
> >>>>>
> >>>>> Only other nits would be the addition of a copyright (up to Avago I
> >>>>> guess) and an mpt3sas version (these drivers are 99% the same code).
> >>>>>
> >>>>> Acked-by: Joe Lawrence<joe.lawrence@stratus.com>
> >>>>>
> >>>>> -- Joe
> >>>>
> >>>>
> >>>>
> >>>
> >>
> >> Thank you for clarifying the guidelines on copyright lines.  I wasn't
> >> sure if this was significant enough of a contribution to justify the
> >> line or not.
> >>
> >> Do I need to re-send or can I just state here that the patch can be
> >> merged without the copyright line?
> >
> > It's easier if you send a clean copy rather than risk me botching
> > something in the hand edit.
> 
> OK, will do.
> 
> >> I don't have any mpt3sas hardware to test with; should I just send in a
> >> patch anyway without a Tested-by line?
> >
> > Well, if you haven't actually tested it, adding a tested-by line
> > wouldn't be correct.
> 
> Of course not.  My question was, should I send in a patch anyway (with 
> appropriate commit message fields) even though it has not been tested?

You mean do you need to retest for a non-material change to a patch
still to have a tested-by on it?  Ideally, yes, but as long as there are
no actual code changes, I don't think it's required.

James

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[Timothy Pearson]

On 06/23/2015 12:56 PM, James Bottomley wrote:
> On Tue, 2015-06-23 at 12:47 -0500, Timothy Pearson wrote:
>> On 06/23/2015 12:45 PM, James Bottomley wrote:
>>> On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:
>>>> On 06/23/2015 08:35 AM, James Bottomley wrote:
>>>>> On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
>>>>>> Hi,
>>>>>>
>>>>>> Upstream contributor should not add copyright to this driver code.
>>>>>
>>>>> I can't agree with that as a general rule: it depends on the
>>>>> significance of the contribution.  The somewhat ill defined standard for
>>>>> this is the contribution must be "an original work of authorship".
>>>>>
>>>>> I can agree that a trivial bug fix like this is not an original work of
>>>>> authorship, so in this case adding copyright isn't appropriate.
>>>>>
>>>>> James
>>>>>
>>>>>> Regards,
>>>>>> Sreekanth
>>>>>>
>>>>>> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence<joe.lawrence@stratus.com>    wrote:
>>>>>>>
>>>>>>>
>>>>>>> On 06/21/2015 02:46 PM, Timothy Pearson wrote:
>>>>>>>> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
>>>>>>>>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
>>>>>>>>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
>>>>>>>>>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
>>>>>>>>>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
>>>>>>>>>>>> memory I/O resource. This failure can happen if the device is too
>>>>>>>>>>>> slow to respond during POST and is missed by the BIOS, but Linux
>>>>>>>>>>>> then detects the device later in the boot process.
>>>>>>>>>>>>
>>>>>>>>>>>> This patch aborts initialization and prints a warning if no memory I/O
>>>>>>>>>>>> resources are found.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>>>>>>>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
>>>>>>>>>>>> 1 file changed, 9 insertions(+)
>>>>>>>>>>>>
>>>>>>>>>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>>>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>>>>>> index 11248de..15c9504 100644
>>>>>>>>>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>>>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>>>>>>>>>>>> @@ -6,6 +6,8 @@
>>>>>>>>>>>> * Copyright (C) 2007-2014 LSI Corporation
>>>>>>>>>>>> * Copyright (C) 20013-2014 Avago Technologies
>>>>>>>>>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
>>>>>>>>>>>> + * Copyright (C) 2015 Raptor Engineering
>>>>>>>>>>>> + * (mailto: support@araptorengineeringinc.com)
>>>>>>>>>>>> *
>>>>>>>>>>>> * This program is free software; you can redistribute it and/or
>>>>>>>>>>>> * modify it under the terms of the GNU General Public License
>>>>>>>>>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
>>>>>>>>>>>> MPT2SAS_ADAPTER
>>>>>>>>>>>> *ioc)
>>>>>>>>>>>> }
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> + if (ioc->chip == NULL) {
>>>>>>>>>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
>>>>>>>>>>>> + "adapter memory (resource not found)!\n", ioc->name);
>>>>>>>>>>>> + r = -EINVAL;
>>>>>>>>>>>> + goto out_fail;
>>>>>>>>>>>> + }
>>>>>>>>>>>> +
>>>>>>>>>>>> _base_mask_interrupts(ioc);
>>>>>>>>>>>>
>>>>>>>>>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>>>>>>>>>>>
>>>>>>>>>>> Just following up on this patch as I have not yet received any
>>>>>>>>>>> response.
>>>>>>>>>>>
>>>>>>>>>>> Thanks!
>>>>>>>>>>
>>>>>>>>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
>>>>>>>>>> few lines above the one added by the patch insufficient?
>>>>>>>>>>
>>>>>>>>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
>>>>>>>>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
>>>>>>>>>> existing ioc->chip check relocated entirely to where you put it (maybe
>>>>>>>>>> also pulling the entire error text onto one line for easier grepping).
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>>
>>>>>>>>>> -- Joe
>>>>>>>>>
>>>>>>>>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
>>>>>>>>> the BIOS does not run resource allocation on the mpt2sas device) then
>>>>>>>>> the check you are referring to is not executed, and the driver attempts
>>>>>>>>> to perform operations on a null ioc->chip pointer.
>>>>>>>>>
>>>>>>>>> I can relocate the check if desired.
>>>>>>>>>
>>>>>>>>
>>>>>>>> On looking more closely at the code I think having the two separate
>>>>>>>> checks is useful for debugging.  The first error message is triggered if
>>>>>>>> the resource exists but Linux cannot map it for some reason, and the
>>>>>>>> second error message is triggered if the resource does not exist at all
>>>>>>>> (buggy BIOS).
>>>>>>>
>>>>>>> Fair enough.  The two error messages are unique, so grepping will lead
>>>>>>> to the correct check.
>>>>>>>
>>>>>>> Only other nits would be the addition of a copyright (up to Avago I
>>>>>>> guess) and an mpt3sas version (these drivers are 99% the same code).
>>>>>>>
>>>>>>> Acked-by: Joe Lawrence<joe.lawrence@stratus.com>
>>>>>>>
>>>>>>> -- Joe
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>> Thank you for clarifying the guidelines on copyright lines.  I wasn't
>>>> sure if this was significant enough of a contribution to justify the
>>>> line or not.
>>>>
>>>> Do I need to re-send or can I just state here that the patch can be
>>>> merged without the copyright line?
>>>
>>> It's easier if you send a clean copy rather than risk me botching
>>> something in the hand edit.
>>
>> OK, will do.
>>
>>>> I don't have any mpt3sas hardware to test with; should I just send in a
>>>> patch anyway without a Tested-by line?
>>>
>>> Well, if you haven't actually tested it, adding a tested-by line
>>> wouldn't be correct.
>>
>> Of course not.  My question was, should I send in a patch anyway (with
>> appropriate commit message fields) even though it has not been tested?
>
> You mean do you need to retest for a non-material change to a patch
> still to have a tested-by on it?  Ideally, yes, but as long as there are
> no actual code changes, I don't think it's required.
>
> James

I guess I'm being somewhat confusing today, my apologies for that.  I 
understand there is no real need to re-test when removing the copyright 
line for the mpt2sas driver patch.

My question is related to your earlier comment about the mpt3sas driver, 
which apparently uses mostly the same codebase as the mpt2sas driver. 
Specifically, when referring to the mpt3sas driver (which I cannot 
directly test), should I send in an untested patch for the mpt3sas 
driver, which would be based on the tested / working patch for the 
mpt2sas driver?

Thanks!

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com

Re: [PATCH] mpt2sas: Abort initialization if no memory I/O resources, detected

[James Bottomley]

On Tue, 2015-06-23 at 12:59 -0500, Timothy Pearson wrote:
> On 06/23/2015 12:56 PM, James Bottomley wrote:
> > On Tue, 2015-06-23 at 12:47 -0500, Timothy Pearson wrote:
> >> On 06/23/2015 12:45 PM, James Bottomley wrote:
> >>> On Tue, 2015-06-23 at 12:33 -0500, Timothy Pearson wrote:
> >>>> On 06/23/2015 08:35 AM, James Bottomley wrote:
> >>>>> On Tue, 2015-06-23 at 18:06 +0530, Sreekanth Reddy wrote:
> >>>>>> Hi,
> >>>>>>
> >>>>>> Upstream contributor should not add copyright to this driver code.
> >>>>>
> >>>>> I can't agree with that as a general rule: it depends on the
> >>>>> significance of the contribution.  The somewhat ill defined standard for
> >>>>> this is the contribution must be "an original work of authorship".
> >>>>>
> >>>>> I can agree that a trivial bug fix like this is not an original work of
> >>>>> authorship, so in this case adding copyright isn't appropriate.
> >>>>>
> >>>>> James
> >>>>>
> >>>>>> Regards,
> >>>>>> Sreekanth
> >>>>>>
> >>>>>> On Tue, Jun 23, 2015 at 9:24 AM, Joe Lawrence<joe.lawrence@stratus.com>    wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> On 06/21/2015 02:46 PM, Timothy Pearson wrote:
> >>>>>>>> On 06/16/2015 01:49 PM, Timothy Pearson wrote:
> >>>>>>>>> On 06/16/2015 12:42 PM, Joe Lawrence wrote:
> >>>>>>>>>> On 06/16/2015 12:28 PM, Timothy Pearson wrote:
> >>>>>>>>>>> On 06/12/2015 05:05 PM, Timothy Pearson wrote:
> >>>>>>>>>>>> The mpt2sas driver crashes if the BIOS does not set up at least one
> >>>>>>>>>>>> memory I/O resource. This failure can happen if the device is too
> >>>>>>>>>>>> slow to respond during POST and is missed by the BIOS, but Linux
> >>>>>>>>>>>> then detects the device later in the boot process.
> >>>>>>>>>>>>
> >>>>>>>>>>>> This patch aborts initialization and prints a warning if no memory I/O
> >>>>>>>>>>>> resources are found.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
> >>>>>>>>>>>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
> >>>>>>>>>>>> ---
> >>>>>>>>>>>> drivers/scsi/mpt2sas/mpt2sas_base.c | 9 +++++++++
> >>>>>>>>>>>> 1 file changed, 9 insertions(+)
> >>>>>>>>>>>>
> >>>>>>>>>>>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>>>>>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>>>>>> index 11248de..15c9504 100644
> >>>>>>>>>>>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>>>>>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
> >>>>>>>>>>>> @@ -6,6 +6,8 @@
> >>>>>>>>>>>> * Copyright (C) 2007-2014 LSI Corporation
> >>>>>>>>>>>> * Copyright (C) 20013-2014 Avago Technologies
> >>>>>>>>>>>> * (mailto: MPT-FusionLinux.pdl@avagotech.com)
> >>>>>>>>>>>> + * Copyright (C) 2015 Raptor Engineering
> >>>>>>>>>>>> + * (mailto: support@araptorengineeringinc.com)
> >>>>>>>>>>>> *
> >>>>>>>>>>>> * This program is free software; you can redistribute it and/or
> >>>>>>>>>>>> * modify it under the terms of the GNU General Public License
> >>>>>>>>>>>> @@ -1582,6 +1584,13 @@ mpt2sas_base_map_resources(struct
> >>>>>>>>>>>> MPT2SAS_ADAPTER
> >>>>>>>>>>>> *ioc)
> >>>>>>>>>>>> }
> >>>>>>>>>>>> }
> >>>>>>>>>>>>
> >>>>>>>>>>>> + if (ioc->chip == NULL) {
> >>>>>>>>>>>> + printk(MPT2SAS_ERR_FMT "unable to map "
> >>>>>>>>>>>> + "adapter memory (resource not found)!\n", ioc->name);
> >>>>>>>>>>>> + r = -EINVAL;
> >>>>>>>>>>>> + goto out_fail;
> >>>>>>>>>>>> + }
> >>>>>>>>>>>> +
> >>>>>>>>>>>> _base_mask_interrupts(ioc);
> >>>>>>>>>>>>
> >>>>>>>>>>>> r = _base_get_ioc_facts(ioc, CAN_SLEEP);
> >>>>>>>>>>>
> >>>>>>>>>>> Just following up on this patch as I have not yet received any
> >>>>>>>>>>> response.
> >>>>>>>>>>>
> >>>>>>>>>>> Thanks!
> >>>>>>>>>>
> >>>>>>>>>> Hi Tim -- just curious, why was the similar check on ioc->chip just a
> >>>>>>>>>> few lines above the one added by the patch insufficient?
> >>>>>>>>>>
> >>>>>>>>>> That loop block sets memap_sz when it finds an IORESOURCE_MEM so that it
> >>>>>>>>>> only sets ioc->chip once. I wonder if the fix might be simpler if the
> >>>>>>>>>> existing ioc->chip check relocated entirely to where you put it (maybe
> >>>>>>>>>> also pulling the entire error text onto one line for easier grepping).
> >>>>>>>>>>
> >>>>>>>>>> Regards,
> >>>>>>>>>>
> >>>>>>>>>> -- Joe
> >>>>>>>>>
> >>>>>>>>> If there are no IORESOURCE_MEM resources allocated by the BIOS (i.e. if
> >>>>>>>>> the BIOS does not run resource allocation on the mpt2sas device) then
> >>>>>>>>> the check you are referring to is not executed, and the driver attempts
> >>>>>>>>> to perform operations on a null ioc->chip pointer.
> >>>>>>>>>
> >>>>>>>>> I can relocate the check if desired.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> On looking more closely at the code I think having the two separate
> >>>>>>>> checks is useful for debugging.  The first error message is triggered if
> >>>>>>>> the resource exists but Linux cannot map it for some reason, and the
> >>>>>>>> second error message is triggered if the resource does not exist at all
> >>>>>>>> (buggy BIOS).
> >>>>>>>
> >>>>>>> Fair enough.  The two error messages are unique, so grepping will lead
> >>>>>>> to the correct check.
> >>>>>>>
> >>>>>>> Only other nits would be the addition of a copyright (up to Avago I
> >>>>>>> guess) and an mpt3sas version (these drivers are 99% the same code).
> >>>>>>>
> >>>>>>> Acked-by: Joe Lawrence<joe.lawrence@stratus.com>
> >>>>>>>
> >>>>>>> -- Joe
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>
> >>>> Thank you for clarifying the guidelines on copyright lines.  I wasn't
> >>>> sure if this was significant enough of a contribution to justify the
> >>>> line or not.
> >>>>
> >>>> Do I need to re-send or can I just state here that the patch can be
> >>>> merged without the copyright line?
> >>>
> >>> It's easier if you send a clean copy rather than risk me botching
> >>> something in the hand edit.
> >>
> >> OK, will do.
> >>
> >>>> I don't have any mpt3sas hardware to test with; should I just send in a
> >>>> patch anyway without a Tested-by line?
> >>>
> >>> Well, if you haven't actually tested it, adding a tested-by line
> >>> wouldn't be correct.
> >>
> >> Of course not.  My question was, should I send in a patch anyway (with
> >> appropriate commit message fields) even though it has not been tested?
> >
> > You mean do you need to retest for a non-material change to a patch
> > still to have a tested-by on it?  Ideally, yes, but as long as there are
> > no actual code changes, I don't think it's required.
> >
> > James
> 
> I guess I'm being somewhat confusing today, my apologies for that.  I 
> understand there is no real need to re-test when removing the copyright 
> line for the mpt2sas driver patch.
> 
> My question is related to your earlier comment about the mpt3sas driver, 
> which apparently uses mostly the same codebase as the mpt2sas driver. 
> Specifically, when referring to the mpt3sas driver (which I cannot 
> directly test), should I send in an untested patch for the mpt3sas 
> driver, which would be based on the tested / working patch for the 
> mpt2sas driver?

You can send untested patches, yes.  The maintainer will review them and
decide on inclusion.

James

[PATCH v2] mpt2sas: Abort initialization if no memory I/O resources detected

[Timothy Pearson]

The mpt2sas driver crashes if the BIOS does not set up at least one
memory I/O resource.  This failure can happen if the device is too
slow to respond during POST and is missed by the BIOS, but Linux
then detects the device later in the boot process.

Signed-off-by: Timothy Pearson <tpearson@raptorengineeringinc.com>
Tested-by: Timothy Pearson <tpearson@raptorengineeringinc.com>
---
 drivers/scsi/mpt2sas/mpt2sas_base.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
index 11248de..b70fa5a 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_base.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
@@ -1582,6 +1582,13 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER *ioc)
 		}
 	}
 
+	if (ioc->chip == NULL) {
+		printk(MPT2SAS_ERR_FMT "unable to map "
+			"adapter memory (resource not found)!\n", ioc->name);
+		r = -EINVAL;
+		goto out_fail;
+	}
+
 	_base_mask_interrupts(ioc);
 
 	r = _base_get_ioc_facts(ioc, CAN_SLEEP);
-- 
1.7.9.5

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com 

Re: [PATCH v2] mpt2sas: Abort initialization if no memory I/O resources detected

[Timothy Pearson]

On 06/23/2015 02:28 PM, Timothy Pearson wrote:
> The mpt2sas driver crashes if the BIOS does not set up at least one
> memory I/O resource.  This failure can happen if the device is too
> slow to respond during POST and is missed by the BIOS, but Linux
> then detects the device later in the boot process.
>
> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
> ---
>   drivers/scsi/mpt2sas/mpt2sas_base.c |    7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c b/drivers/scsi/mpt2sas/mpt2sas_base.c
> index 11248de..b70fa5a 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
> @@ -1582,6 +1582,13 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER *ioc)
>   		}
>   	}
>
> +	if (ioc->chip == NULL) {
> +		printk(MPT2SAS_ERR_FMT "unable to map "
> +			"adapter memory (resource not found)!\n", ioc->name);
> +		r = -EINVAL;
> +		goto out_fail;
> +	}
> +
>   	_base_mask_interrupts(ioc);
>
>   	r = _base_get_ioc_facts(ioc, CAN_SLEEP);

Just wanted to follow up on this patch since I haven't received a 
response.  As far as I know the (trivial) concerns from the first 
version should be addressed in this version.

Thanks!

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com

Re: [PATCH v2] mpt2sas: Abort initialization if no memory I/O resources detected

[Sreekanth Reddy]

Hi Timothy,

Thanks for the patch.

I feel there is some code redundancy after including this patch, I
mean to say is twice we are checking for (ioc->chip == NULL). So I am
working on to reduce this redundancy. Soon I will post a new patch.

Thanks,
Sreekanth

On Sun, Jun 28, 2015 at 7:00 AM, Timothy Pearson
<tpearson@raptorengineeringinc.com> wrote:
> On 06/23/2015 02:28 PM, Timothy Pearson wrote:
>>
>> The mpt2sas driver crashes if the BIOS does not set up at least one
>> memory I/O resource.  This failure can happen if the device is too
>> slow to respond during POST and is missed by the BIOS, but Linux
>> then detects the device later in the boot process.
>>
>> Signed-off-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>> Tested-by: Timothy Pearson<tpearson@raptorengineeringinc.com>
>> ---
>>   drivers/scsi/mpt2sas/mpt2sas_base.c |    7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> index 11248de..b70fa5a 100644
>> --- a/drivers/scsi/mpt2sas/mpt2sas_base.c
>> +++ b/drivers/scsi/mpt2sas/mpt2sas_base.c
>> @@ -1582,6 +1582,13 @@ mpt2sas_base_map_resources(struct MPT2SAS_ADAPTER
>> *ioc)
>>                 }
>>         }
>>
>> +       if (ioc->chip == NULL) {
>> +               printk(MPT2SAS_ERR_FMT "unable to map "
>> +                       "adapter memory (resource not found)!\n",
>> ioc->name);
>> +               r = -EINVAL;
>> +               goto out_fail;
>> +       }
>> +
>>         _base_mask_interrupts(ioc);
>>
>>         r = _base_get_ioc_facts(ioc, CAN_SLEEP);
>
>
> Just wanted to follow up on this patch since I haven't received a response.
> As far as I know the (trivial) concerns from the first version should be
> addressed in this version.
>
> Thanks!
>
> --
> Timothy Pearson
> Raptor Engineering
> +1 (415) 727-8645
> http://www.raptorengineeringinc.com



-- 

Regards,
Sreekanth

[PATCH] mpt3sas: Abort initialization if no memory I/O resources detected

[Timothy Pearson]

The mpt3sas driver crashes if the BIOS does not set up at least one
memory I/O resource.  This failure can happen if the device is too
slow to respond during POST and is missed by the BIOS, but Linux
then detects the device later in the boot process.

Signed-off-by: Timothy Pearson <tpearson@raptorengineeringinc.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 14a781b..4460d48 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1865,6 +1865,13 @@ mpt3sas_base_map_resources(struct MPT3SAS_ADAPTER *ioc)
 		}
 	}
 
+	if (ioc->chip == NULL) {
+		printk(MPT3SAS_FMT "unable to map "
+			"adapter memory (resource not found)!\n", ioc->name);
+		r = -EINVAL;
+		goto out_fail;
+	}
+
 	_base_mask_interrupts(ioc);
 
 	r = _base_get_ioc_facts(ioc, CAN_SLEEP);
-- 
1.7.9.5

-- 
Timothy Pearson
Raptor Engineering
+1 (415) 727-8645
http://www.raptorengineeringinc.com