[lkml]   [2017]   [Dec]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 5/6] staging: pi433: Rename enum dataMode in rf69_enum.h

Am 04.12.2017 um 12:24 schrieb Dan Carpenter:
> On Sun, Dec 03, 2017 at 04:17:25PM +0100, Simon Sandström wrote:
>> Renames enum dataMode and its values packet, continuous, continuousNoSync
>> to enum data_mode and PACKET, CONTINUOUS, CONTINUOUS_NO_SYNC. Fixes
>> warnings: "Avoid CamelCase: <dataMode>, <continuousNoSync>".
> These names are too generic. Delete them. Use DATAMODUL_MODE_PACKET
> and friends directly.
> int rf69_set_data_mode(struct spi_device *spi, u8 val)
> {
> }
> Only DATAMODUL_MODE_PACKET is ever used. There is no need to validate
> the parameters.
> regards,
> dan carpenter

Hi Dan, hi Simon,

like I wrote a few days ago to Marcin Ciupak, I see two disadvantages in
doing so.

If you want to go that way, you - as far as I believe - need to alter
the values in rf69_enum.h, so they carry the corresponding values from
rf69_reg.h. To avoid confusion, you will need to remove the values from
But then you have to keep track of two files (enum.h and reg.h), if you
want to further develop register access stuff. I would prefer to keep
all chip/register related values at the same place.

Second there might be the idea of supporting different chips in the
future (I already thought about).
Then it might be, that DATAMODUL_MODE_PACKET might need an other value.
Therefore, I introduced the "double layer" - enums as labels for the
user space and defines, containing the values, for the register access.

For closer details, pls. see my long answer to Marcin.

I am not sure, whether simplification of the code like proposed is more
important, then the disadvatages, I mentioned.



 \ /
  Last update: 2017-12-04 20:13    [W:0.110 / U:0.012 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site