[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20160603.113902.296916236989519473.davem@davemloft.net>
Date: Fri, 03 Jun 2016 11:39:02 -0700 (PDT)
From: David Miller <davem@...emloft.net>
To: socketcan@...tkopp.net
Cc: ulrich.hecht@...il.com, ramesh.shanmugasundaram@...renesas.com,
mkl@...gutronix.de, wg@...ndegger.com, linux-can@...r.kernel.org,
netdev@...r.kernel.org, Chris.Paterson2@...esas.com,
horms@...ge.net.au, magnus.damm@...il.com,
linux-renesas-soc@...r.kernel.org
Subject: Re: [RESEND PATCH v5 1/2] can: rcar_canfd: Add Renesas R-Car CAN
FD driver
From: Oliver Hartkopp <socketcan@...tkopp.net>
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.
I agree with Oliver, these StuDlyCaPS names used in the spec should
not be used in the driver, they are completely unreadable.
Powered by blists - more mailing lists