On Tue, Jun 23, 2015 at 7:05 PM, James Bottomley
<James.Bottomley@xxxxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxx>
>> >>>>>> Tested-by: Timothy Pearson<tpearson@xxxxxxxxxxxxxxxxxxxxxxxx>
>> >>>>>> ---
>> >>>>>> 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@xxxxxxxxxxxxx)
>> >>>>>> + * Copyright (C) 2015 Raptor Engineering
>> >>>>>> + * (mailto: support@xxxxxxxxxxxxxxxxxxxxxxxxx)
>> >>>>>> *
>> >>>>>> * 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@xxxxxxxxxxx>
>> >
>> > -- Joe
>>
>>
>>
>
>
>
--
Regards,
Sreekanth
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
