Subject: Re: [RESEND PATCH v5 1/2] can: rcar_canfd: Add Renesas R-Car CAN FD driver

From: Oliver Hartkopp <[email-masked]>
Date: Fri, 3 Jun 2016 19:15:29 +0200

On 06/03/2016 07:03 PM, Ulrich Hecht wrote:

> Thanks; I missed that every register is described twice.
>
> Nevertheless, names often vary more or less subtly between your patch
> and the specs, making it very hard to review. Some have letters added,
> some have letters removed, and some are just plain confusing. For
> instance, RCANFD_DCFG_* apparently does not describe, as one might
> think, RSCFDnCFDCmDCFG, but RSCFDnCFDCmFDCFG. These names are, of
> course, completely ridiculous, but inventing a new set makes things
> even worse, IMO.

???

You suggest to use 'completely ridiculous' definitions in favor to 
definitions that have a proper name space RCANFD_ ?

When there is a more readable way that maintains proper readable code 
there's no reason to adopt crappy definitions just because some chip 
designer has no clue how to design proper register names.

When there's some mapping from RSCFDnCFDCmFDCFG to RCANFD_DCFG_* this 
could be mentioned in the comments.

But I'm totally against these blurry upper/lower case letter stuff for 
register definitions.

Regards,
Oliver

>
> At least for the bits, I'd stick with the names given in the
> datasheet. They usually make a modicum of sense, and it makes it way
> easier to search for them. It would also help if the bits were sorted
> consistently.
>
> CU
> Uli
>