Re: [PATCH v7 4/7] can: provide a separate bittiming_const parameter to bittiming functions

[Oliver Hartkopp <socketcan@hartkopp.net>]

On 28.02.2014 11:46, Marc Kleine-Budde wrote:
> On 02/28/2014 11:26 AM, Oliver Hartkopp wrote:


>>  {
>> -	struct can_priv *priv = netdev_priv(dev);
>> -	int err;
>> +	int err = -EINVAL;
> 
> Keep err uninitialized and make an explicit else err = -EINVAL.... see
> below:
> 


>> +	if (!bt->tq) {
>> +		/* non-expert mode: Determine bit-timing parameters */
>> +		if (bt->bitrate)
> 
> Why not:
> 	if (!bt->tq && bt->bitrate) {
> 
>> +			err = can_calc_bittiming(dev, bt, btc);
>> +	} else {
> 
> 	} else if (bt->tq && !bt->bitrate) {
> 
>> +		/*
>> +		 * expert mode: Check bit-timing parameters and
>> +		 * calculate proper brp and a human readable bitrate
>> +		 */
>> +		if (!bt->bitrate)
>> +			err = can_fixup_bittiming(dev, bt, btc);
>>  	}
> 	} else {
> 		err = -EINVAL;
> 	}

I don't have a strong preference on my implementation.

But IMHO your suggested changes only add additional code and additional
comparisons in the if statement.

I don't see a benefit in your suggestions :-(

Regards,
Oliver