On Sat, Dec 02, 2017 at 02:21:32PM +0200, Marcus Wolf wrote:
> To increase the readability of the register accesses, the abstraction of the
> helpers was increased from simple read and write to set bit, reset bit and
> read modify write bit. In addition - according to the proposal from Walter
> Harms from 20.07.2017 - instead of marcros inline functions were used.
>
> As a bonus, with this refactoring a lot of lines were shortened a lot. So
> some of them now undershoot 80 chars, thus reducing the total number of
> complaints of checkPatch.pl in rf69.c.
> ---
> drivers/staging/pi433/rf69.c | 347 ++++++++++++++++++++++--------------------
> 1 file changed, 185 insertions(+), 162 deletions(-)
No signed-off-by line. Always use scripts/checkpatch.pl so you don't
get grumpy emails from maintainers telling you to use
scripts/checkpatch.pl.
>
> diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
> index 0df084e..f6d0b82 100644
> --- a/drivers/staging/pi433/rf69.c
> +++ b/drivers/staging/pi433/rf69.c
> @@ -30,13 +30,37 @@
> #include "rf69.h"
> #include "rf69_registers.h"
>
> #define F_OSC 32000000 /* in Hz */
> #define FIFO_SIZE 66 /* in byte */
>
> /*-------------------------------------------------------------------------*/
>
> -#define READ_REG(x) rf69_read_reg (spi, x)
> -#define WRITE_REG(x, y) rf69_write_reg(spi, x, y)
> +inline static int setBit(struct spi_device *spi, u8 reg, u8 mask)
We have setbit functions, perhaps name this a bit differently as it is
doing something else? And no interCaps please.
> +{
What kind of crazy git options do you have that it creates so much
context for the diff?
> + u8 tmpVal;
> +
> + tmpVal = rf69_read_reg (spi, reg);
> + tmpVal = tmpVal | mask;
> + return rf69_write_reg(spi, reg, tmpVal);
> +}
> +
> +inline static int rstBit(struct spi_device *spi, u8 reg, u8 mask)
rstBit()? What does that mean?
> +{
> + u8 tmpVal;
> +
> + tmpVal = rf69_read_reg (spi, reg);
> + tmpVal = tmpVal & ~mask;
> + return rf69_write_reg(spi, reg, tmpVal);
> +}
> +
> +inline static int rmw(struct spi_device *spi, u8 reg, u8 mask, u8 value)
what does rmw() mean? Spell it out so no one has to try to guess :)
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
