Re: [PATCH v2] powerpc: kernel: remove useless code which related with 'max_cpus'

[Chen Gang <gang.chen@asianux.com>]

On 07/24/2013 09:16 AM, Michael Ellerman wrote:
> On Wed, Jul 24, 2013 at 08:28:07AM +0800, Chen Gang wrote:
>> > On 07/23/2013 09:44 PM, Michael Ellerman wrote:
>>> > > On Mon, Jul 22, 2013 at 12:21:16PM +0530, Srivatsa S. Bhat wrote:
>>>> > >> On 07/22/2013 12:10 PM, Chen Gang wrote:
>>>>> > >>> Since not need 'max_cpus' after the related commit, the related code
>>>>> > >>> are useless too, need be removed.
>>>>> > >>>
>>>>> > >>> The related commit:
>>>>> > >>>
>>>>> > >>>   c1aa687 powerpc: Clean up obsolete code relating to decrementer and timebase
>>>>> > >>>
>>>>> > >>> The related warning:
>>>>> > >>>
>>>>> > >>>   arch/powerpc/kernel/smp.c:323:43: warning: parameter ‘max_cpus’ set but not used [-Wunused-but-set-parameter]
>>>>> > >>>
>>>>> > >>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>>>> > >>
>>>> > >> This version looks good.
>>> > > 
>>> > > Agreed.
>>> > > 
>>> > > A good follow up patch, or actually series of patches, would be to
>>> > > change the prototype of smp_ops->probe() to return void, and fix all the
>>> > > implementations to no longer return anything.
>>> > > 
>> > 
>> > Hmm... normally, a function need have a return value, it will make it
>> > more extensible (especially, it is an API which need be implemented in
>> > various sub modules).
> A function doesn't need a return value, and if it needs one in future then
> we'll add it then. We don't carry code around "just in case".
> 

But for API (also include the internal API), at least, better to always
provide the return value which can indicate failure by negative number
(if succeed can return the meanness value, e.g. the number of cpus).


>> > Even though the return value may be useless, now, if the performance is
>> > not quite important in our case, I still suggest to have it (especially
>> > each various original implementation already has it).
> It's dead code, it should be removed.

For API, if not cause the real world issue, better to keep compatible
(especially, the return value still can indicate failure by negative
number).


Thanks.
-- 
Chen Gang