Re: [Linux-ATM-General] [PATCH v2 1/1] solos-pci: Fix regression introduced by newest firmware

[MIke W. <we...@cs...>]

I definitely concur with keeping patch fingerprints as small as 
possible.   And  since state change is (hopefully) a rare event,  the 
impact of one instance stuff like this is clearly minimal,   but enough 
of this stuff wastes memory.   I guess we should just be thankful the 
original programmer did not feel like writing

if (strcmp(state_str, "man we are up, rocking and rolling and kicking 
a$$ now"))



 
Philip Prindeville wrote:
> I'm not especially worried about it...  Gcc has a lot of __builtin 
> functions for memcpy, memcmp, strcmp, etc. and usually does a fairly 
> efficient job of it...  I've not had to check lately, but I wouldn't 
> be surprised if it even does a string unroll for short strings.  The 
> test could be changed to:
>
> if (state_str[0] != 'S' || strcmp(&state_str[1], "howtime"))
>
>
> and I wouldn't especially mind, but I did want to keep the amount of 
> fingerprints down to a minimum.
>
> Yes, I'm also old and grouchy... I remember having to write in BAL-360 
> and manage my own stack-frame... The PDP-11 was a breath of fresh air...
>
> But anyway, like I said... wanted to keep patches to "low touch", 
> especially since a lot of folks will be back-porting these fixes to 
> 2.6.32 or even 2.6.27...
>
>
> On 3/20/11 7:11 PM, MIke Westall wrote:
>> I'll be the first to admit that (1) I'm a grouchy old guy who (2) 
>> first programmed on an IBM 1620 and then on a 360  Model 30 with 32K 
>> ram.  But why oh why do we need to use 9 byte string constants with 
>> per character compare to decide if the device is up or not :-(
>>
>>
>>
>>
>> Philip Prindeville wrote:
>>> The newest FPGA firmware on the Solos processors correctly signals 
>>> carrier transitions, bitrate, etc.
>>>
>>> The driver previously ignored these messages, and the physical state 
>>> was always ATM_PHY_SIG_UNKNOWN.
>>>
>>> Now that the board reports its state, we expose a bug whereby the 
>>> transition from UNKNOWN to LOST causes us to release all VC's.
>>>
>>> We don't delete any VC's, but instead just send an indication of 
>>> carrier change.
>>>
>>> Signed-off-by: Philip A Prindeville <ph...@re...>
>>> ---
>>>
>>> --- a/drivers/atm/solos-pci.c    2011-03-20 15:27:40.000000000 -0600
>>> +++ b/drivers/atm/solos-pci.c    2011-03-20 16:32:11.000000000 -0600
>>> @@ -382,8 +382,10 @@ static int process_status(struct solos_c
>>>
>>>       /* Anything but 'Showtime' is down */
>>>       if (strcmp(state_str, "Showtime")) {
>>>           atm_dev_signal_change(card->atmdev[port], ATM_PHY_SIG_LOST);
>>> +#if 0
>>>           atm_dev_release_vccs(card->atmdev[port]);
>>> +#endif
>>>           dev_info(&card->dev->dev, "Port %d: %s\n", port, state_str);
>>>         return 0;
>>>     }
>>>
>>>
>
>
>