Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754526AbdLFJLk (ORCPT ); Wed, 6 Dec 2017 04:11:40 -0500 Received: from dd39320.kasserver.com ([85.13.155.146]:35436 "EHLO dd39320.kasserver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754288AbdLFJLf (ORCPT ); Wed, 6 Dec 2017 04:11:35 -0500 Subject: Re: [PATCH v2 08/11] staging: pi433: Remove enum data_mode To: =?UTF-8?Q?Simon_Sandstr=c3=b6m?= , gregkh@linuxfoundation.org Cc: linux@Wolf-Entwicklungen.de, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org References: <20171205220849.5486-1-simon@nikanor.nu> <20171205220849.5486-9-simon@nikanor.nu> From: Marcus Wolf Message-ID: <2b04702d-65f0-ab53-d28f-e20e79b6c1c1@smarthome-wolf.de> Date: Wed, 6 Dec 2017 11:11:27 +0200 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171205220849.5486-9-simon@nikanor.nu> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: de-DE Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4031 Lines: 105 Am 06.12.2017 um 00:08 schrieb Simon Sandström: > Call rf69_set_data_mode with DATAMODUL_MODE value directly. > > Signed-off-by: Simon Sandström > --- > drivers/staging/pi433/pi433_if.c | 2 +- > drivers/staging/pi433/rf69.c | 15 ++------------- > drivers/staging/pi433/rf69.h | 2 +- > drivers/staging/pi433/rf69_enum.h | 6 ------ > 4 files changed, 4 insertions(+), 21 deletions(-) > > diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c > index fb500d062df8..3b4170b9ba94 100644 > --- a/drivers/staging/pi433/pi433_if.c > +++ b/drivers/staging/pi433/pi433_if.c > @@ -1125,7 +1125,7 @@ static int pi433_probe(struct spi_device *spi) > > /* setup the radio module */ > SET_CHECKED(rf69_set_mode (spi, standby)); > - SET_CHECKED(rf69_set_data_mode (spi, packet)); > + SET_CHECKED(rf69_set_data_mode (spi, DATAMODUL_MODE_PACKET)); > SET_CHECKED(rf69_set_amplifier_0 (spi, OPTION_ON)); > SET_CHECKED(rf69_set_amplifier_1 (spi, OPTION_OFF)); > SET_CHECKED(rf69_set_amplifier_2 (spi, OPTION_OFF)); > diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c > index e5b29bed35ef..4c9eb97978ef 100644 > --- a/drivers/staging/pi433/rf69.c > +++ b/drivers/staging/pi433/rf69.c > @@ -61,20 +61,9 @@ int rf69_set_mode(struct spi_device *spi, enum mode mode) > > } > > -int rf69_set_data_mode(struct spi_device *spi, enum dataMode dataMode) > +int rf69_set_data_mode(struct spi_device *spi, u8 data_mode) > { > - #ifdef DEBUG > - dev_dbg(&spi->dev, "set: data mode"); > - #endif > - > - switch (dataMode) { > - case packet: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODE) | DATAMODUL_MODE_PACKET); > - case continuous: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODE) | DATAMODUL_MODE_CONTINUOUS); > - case continuousNoSync: return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODE) | DATAMODUL_MODE_CONTINUOUS_NOSYNC); > - default: > - dev_dbg(&spi->dev, "set: illegal input param"); > - return -EINVAL; > - } > + return WRITE_REG(REG_DATAMODUL, (READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODE) | data_mode); > } > > int rf69_set_modulation(struct spi_device *spi, enum modulation modulation) > diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h > index 177223451c87..18296b4502f3 100644 > --- a/drivers/staging/pi433/rf69.h > +++ b/drivers/staging/pi433/rf69.h > @@ -26,7 +26,7 @@ > #define FIFO_THRESHOLD 15 /* in byte */ > > int rf69_set_mode(struct spi_device *spi, enum mode mode); > -int rf69_set_data_mode(struct spi_device *spi, enum dataMode dataMode); > +int rf69_set_data_mode(struct spi_device *spi, u8 data_mode); > int rf69_set_modulation(struct spi_device *spi, enum modulation modulation); > enum modulation rf69_get_modulation(struct spi_device *spi); > int rf69_set_modulation_shaping(struct spi_device *spi, enum mod_shaping mod_shaping); > diff --git a/drivers/staging/pi433/rf69_enum.h b/drivers/staging/pi433/rf69_enum.h > index 97f615effca3..b0715b4eb6ac 100644 > --- a/drivers/staging/pi433/rf69_enum.h > +++ b/drivers/staging/pi433/rf69_enum.h > @@ -31,12 +31,6 @@ enum mode { > receive > }; > > -enum dataMode { > - packet, > - continuous, > - continuousNoSync > -}; > - > enum modulation { > OOK, > FSK > Hi Simon, this change is "closing a door". The rf69 is able to work in an advanced packet mode or in a simple continuous mode. The driver so far is using the advanced packet mode, only. But if it will support continuous mode some day, it will be necessary to configure this. The open source projects fhem and domoticz already asked for such a change, since for their architecture, they'll need a pi433 working in continuous mode. But so far I am not planning to implement such a functionality. Since the rule for kernel development seems to be, not to care about future, most probably you patch is fine, anyway. Marcus