Re: [PATCH] staging: lustre: check result of register_shrinker
On Tue, Dec 05, 2017 at 12:50:25AM +, Dilger, Andreas wrote: > On Dec 4, 2017, at 11:42, Aliaksei Karaliou wrote: > > > > On 12/04/2017 11:40 AM, Dan Carpenter wrote: > >> On Sun, Dec 03, 2017 at 07:59:07PM +0300, ak wrote: > >>> Thank you for your extensive comments. > >>> > >>> I've also thought about adding more protection into unregister_shrinker(), > >>> but not sure how to properly organize the patch set, because there will be > >>> three patches: > >>> * change in mm/vmscan that adds protection and sanitizer. > >>> * fixed change for Lustre driver > >>> * there also two explicit usages of shrinker->nr_deferred in drivers - > >>>good idea to fix too. > >>> All patches have different lists of maintainers, and second and third > >>> depend > >>> on first one. And I don't like to send them separately. > >>> So, I'm going to at least prepend this patch with mm/vmscan one. > >> Fix the style for the Lustre patch and resend. Then patch > >> unregister_shrinker(). Then remove the checks. > >> > >> The unregister_shrinker() changes seem like a good idea, but I haven't > >> really looked at it. It might be more involved than it seems. > >> > >> regards, > >> dan carpenter > > Thanks for the comments too. > > I'll send patch with accumulated fixes. > >>> } > >>> diff --git a/drivers/staging/lustre/lustre/obdclass/lu_object.c > >>> b/drivers/staging/lustre/lustre/obdclass/lu_object.c > >>> index b938a3f9d50a..9e0256ca2079 100644 > >>> --- a/drivers/staging/lustre/lustre/obdclass/lu_object.c > >>> +++ b/drivers/staging/lustre/lustre/obdclass/lu_object.c > >>> @@ -1951,7 +1951,7 @@ int lu_global_init(void) > >>>* inode, one for ea. Unfortunately setting this high value results in > >>>* lu_object/inode cache consuming all the memory. > >>>*/ > >>> - register_shrinker(&lu_site_shrinker); > >>> + result = register_shrinker(&lu_site_shrinker); > >> There should be some error handling if the register fails. > > Yeah, I think so, but it seems that it is out of scope of this patch. > > The whole negative branch in the mainline kernel looks broken (IMHO). > > In mainline Lustre's git there is a reworked version of upper function > > obdclass_init(), > > which at least calls lu_global_fini() before exiting `module_init` on > > further failure, > > but yeah, still lacks proper cleanup inside lu_global_initcall(). > > I'll add one more patch in a patch-set so that maintainers may decide what > > to do with that. > > I was looking at the lack of error handling as well, but then I wondered if > the module_init() call returns an error, is module_exit() still called, or > does the module not load at all in the error case? > module_init() is supposed to clean up after itself. module_exit() is only called if init succeeds. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] MAINTAINERS: update Android driver maintainers.
Signed-off-by: Martijn Coenen --- MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index aa71ab52fd76..da8264fc09d4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -859,7 +859,8 @@ F: kernel/configs/android* ANDROID DRIVERS M: Greg Kroah-Hartman M: Arve Hjønnevåg -M: Riley Andrews +M: Todd Kjos +M: Martijn Coenen T: git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git L: de...@driverdev.osuosl.org S: Supported -- 2.15.0.531.g2ccb3012c9-goog ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] MAINTAINERS: update Android driver maintainers.
Add Todd Kjos and myself, remove Riley (who no longer works at Google). Signed-off-by: Martijn Coenen --- Changes in v2: adds commit message. MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index aa71ab52fd76..da8264fc09d4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -859,7 +859,8 @@ F: kernel/configs/android* ANDROID DRIVERS M: Greg Kroah-Hartman M: Arve Hjønnevåg -M: Riley Andrews +M: Todd Kjos +M: Martijn Coenen T: git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git L: de...@driverdev.osuosl.org S: Supported -- 2.15.0.531.g2ccb3012c9-goog ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h
Am 04.12.2017 um 21:05 schrieb Simon Sandström: > On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote: >> >> Hi Simon, hi Dan, >> >> if you both are of the same opinion, for me, it's fine, if we go with two >> functions. >> >> But I don't get the advantage, if we split approx. 10 functions, to get rid >> of enum optionOnOff. >> >> Keep in mind, that if you split the functions, in the interface >> implementation you also need more code: >> >> SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync)); >> >> will have to be converted in something like >> >> if (rx_cfg->enable_sync) >> SET_CHECKED(rf69_set_sync_enbable(dev->spi); >> else >> SET_CHECKED(rf69_set_sync_disable(dev->spi); > > I think that this makes the code very clear. If the config tells us to > enable the sync then we'll enabled it, otherwise we'll disable it. > Hi Simon, I thought about it a while. Yes, you are right, it makes it very clear - but doesn't tell a lot extra. The only thing, you can see extra, is, that there is the option, to turn on and to turn off. You even can't see, whether it is turend on or off. The info, that there is an option of en- or disabling - at least from my side - is visible, too, if there is just one function that takes a bool or optionOnOff as argument. When you 'll redesign every setter, that now deals with optionOnOff in that way, you will introduce something like 50 extra lines of code without bringing any extra functionality to the driver. From my point of view, that's bad: The longer the text, the harder to understand everything entierly, the more chances for bugs and a lot harder to service. So from my point of view such a redesign is nice for optics but bad for further development. I would really prefer a solution, like Marcin Ciupak offered. He is not blowing up the code, but even tries to reduce the calls to READ_REG and WRITE_REG. That increases readbility, too, but also reduces code size and chances for bugs. But regardless of the arguments above, I don't mind. If it's a benefit for you and lot of others in the community, I won't blame you for such a change. If you would add 50 lines of code with new funcitonality (e. g. support the AES feature of the module), I would be a lot happier - and most useres for sure, too. Cheers, Marcus ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h
On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote:
> Keep in mind, that if you split the functions, in the interface
> implementation you also need more code:
>
> SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
>
> will have to be converted in something like
>
> if (rx_cfg->enable_sync)
> SET_CHECKED(rf69_set_sync_enbable(dev->spi);
> else
> SET_CHECKED(rf69_set_sync_disable(dev->spi);
>
Here's what the code looks like right now:
198 /* packet config */
199 /* enable */
200 SET_CHECKED(rf69_set_sync_enable(dev->spi,
rx_cfg->enable_sync));
201 if (rx_cfg->enable_sync == optionOn)
202 {
203 SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
afterSyncInterrupt));
204 }
205 else
206 {
207 SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
always));
208 }
That's for the rx_cfg. We have related but different code for the
tx_cfg. It's strange to me that we can enable sync for rx and not for
tx... How does that work when the setting ends up getting stored in the
same register?
The new code would look like this:
if (rx_cfg->enable_sync) {
ret = rf69_enable_sync(spi);
if (ret)
return ret;
ret = rf69_set_fifo_fill_condition(dev->spi,
afterSyncInterrupt);
if (ret)
return ret;
} else {
ret = rf69_disable_sync(dev->spi);
if (ret)
return ret;
ret = rf69_set_fifo_fill_condition(dev->spi, always);
if (ret)
return ret;
}
It's not the greatest, but it's not the worst... The configuration for
->enable_sync is a bit spread out and it might be nice to move it all to
one function?
I liked Simon's naming scheme and I thought it was clear what the
rf69_set_sync(spi, false) function would do.
Simon, it seems like Marcus and I both are Ok with your style choices.
Do whatever seems best when you implement the code. If it's awkward to
break up the functions then don't.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/5] staging: Introduce NVIDIA Tegra video decoder driver
Hi Hans, On 04.12.2017 17:04, Hans Verkuil wrote: > Hi Dmitry, > > As you already mention in the TODO, this should become a v4l2 codec driver. > > Good existing examples are the coda, qcom/venus and mtk-vcodec drivers. > > One thing that is not clear from this code is if the tegra hardware is a > stateful or stateless codec, i.e. does it keep track of the decoder state > in the hardware, or does the application have to keep track of the state and > provide the state information together with the video data? > > I ask because at the moment only stateful codecs are supported. Work is > ongoing > to support stateless codecs, but we don't support that for now. > It is stateless. Is there anything ready to try out? If yes, could you please give a reference to that work? > Anyway, I'm OK with merging this in staging. Although I think it should go > to staging/media since we want to keep track of it. > Awesome, I'll move driver to staging/media in V5. Thanks! ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h
Am 05.12.2017 um 13:16 schrieb Dan Carpenter:
> On Mon, Dec 04, 2017 at 09:59:02PM +0200, Marcus Wolf wrote:
>> Keep in mind, that if you split the functions, in the interface
>> implementation you also need more code:
>>
>> SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
>>
>> will have to be converted in something like
>>
>> if (rx_cfg->enable_sync)
>> SET_CHECKED(rf69_set_sync_enbable(dev->spi);
>> else
>> SET_CHECKED(rf69_set_sync_disable(dev->spi);
>>
>
> Here's what the code looks like right now:
>
>198 /* packet config */
>199 /* enable */
>200 SET_CHECKED(rf69_set_sync_enable(dev->spi,
> rx_cfg->enable_sync));
>201 if (rx_cfg->enable_sync == optionOn)
>202 {
>203 SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
> afterSyncInterrupt));
>204 }
>205 else
>206 {
>207 SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
> always));
>208 }
>
> That's for the rx_cfg. We have related but different code for the
> tx_cfg. It's strange to me that we can enable sync for rx and not for
> tx... How does that work when the setting ends up getting stored in the
> same register?
>
> The new code would look like this:
>
> if (rx_cfg->enable_sync) {
> ret = rf69_enable_sync(spi);
^
> if (ret)
> return ret;
> ret = rf69_set_fifo_fill_condition(dev->spi,
> afterSyncInterrupt);
> if (ret)
> return ret;
> } else {
> ret = rf69_disable_sync(dev->spi);
> if (ret)
> return ret;
> ret = rf69_set_fifo_fill_condition(dev->spi, always);
> if (ret)
> return ret;
> }
>
> It's not the greatest, but it's not the worst... The configuration for
> ->enable_sync is a bit spread out and it might be nice to move it all to
> one function?
>
> I liked Simon's naming scheme and I thought it was clear what the
> rf69_set_sync(spi, false) function would do.
^
>
> Simon, it seems like Marcus and I both are Ok with your style choices.
> Do whatever seems best when you implement the code. If it's awkward to
> break up the functions then don't.
>
> regards,
> dan carpenter
>
Hi Dan,
now I am a bit confused.
My favourit:
rf69_set_sync_enable(spi, false)
rf69_set_amp_enable(spi, false)
rf69_set_crc_enable(spi, false)
I prefer to keep the enable (or comparable), because it shows, what the
function is doing. For sync, for example, there are several setter:
size, tolerance, values ... AND enable (or comparable).
All functions in rf69.c should start with rf69_get or rf69_set.
Simons proposal:
rf69_set_sync_enable(spi)
rf69_set_sync_disable(spi)
rf69_set_amp_enable(spi)
rf69_set_amp_disable(spi)
rf69_set_crc_enable(spi)
rf69_set_crc_disable(spi)
I don't like that, because it's blowing up the code without bringing
extra feature (see my last mail).
In your code example, you use Simons proposal, but in your explaination
below, you show the original style - although you refer to Simons sheme...
Cheers,
Marcus
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: pi433: Rename enum optionOnOff in rf69_enum.h
On Tue, Dec 05, 2017 at 01:40:02PM +0100, Marcus Wolf wrote: > > It's not the greatest, but it's not the worst... The configuration for > > ->enable_sync is a bit spread out and it might be nice to move it all to > > one function? > > > > I liked Simon's naming scheme and I thought it was clear what the > > rf69_set_sync(spi, false) function would do. > ^ > > Simon's liked splitting it up but he also proposed this alternative: rf69_set_sync_operation(spi, true/false); but I removed the "_operation" because I think it doesn't add anything. > > Simon, it seems like Marcus and I both are Ok with your style choices. > > Do whatever seems best when you implement the code. If it's awkward to > > break up the functions then don't. > > > > regards, > > dan carpenter > > > > Hi Dan, > > now I am a bit confused. > > My favourit: > > rf69_set_sync_enable(spi, false) > rf69_set_amp_enable(spi, false) > rf69_set_crc_enable(spi, false) > > I prefer to keep the enable (or comparable), because it shows, what the > function is doing. For sync, for example, there are several setter: > size, tolerance, values ... AND enable (or comparable). To me it's just weird that "_enable" disables anything. I really prefer just splitting it up. I don't think it will bloat the code. But I'm also fine with: rf69_set_sync(spi, true/false) rf69_set_amp(spi, true/false) rf69_set_crc(spi, true/false) Anyway, I feel like I'll like whatever Simon does. Some of these things, you can't tell how they'll look until the end until you try. Let's wait until we see a patch before we debate any more. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 3/5] staging: Introduce NVIDIA Tegra video decoder driver
On 12/05/17 13:17, Dmitry Osipenko wrote: > Hi Hans, > > On 04.12.2017 17:04, Hans Verkuil wrote: >> Hi Dmitry, >> >> As you already mention in the TODO, this should become a v4l2 codec driver. >> >> Good existing examples are the coda, qcom/venus and mtk-vcodec drivers. >> >> One thing that is not clear from this code is if the tegra hardware is a >> stateful or stateless codec, i.e. does it keep track of the decoder state >> in the hardware, or does the application have to keep track of the state and >> provide the state information together with the video data? >> >> I ask because at the moment only stateful codecs are supported. Work is >> ongoing >> to support stateless codecs, but we don't support that for now. >> > > It is stateless. Is there anything ready to try out? If yes, could you please > give a reference to that work? I rebased my two year old 'requests2' branch to the latest mainline version and gave it the imaginative name 'requests3': https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=requests3 (Note: only compile tested!) This is what ChromeOS has been using (actually they use a slightly older version) and the new version that is currently being developed will be similar, so any work you do on top of this will carry over to the final version without too much effort. At least, that's the intention :-) I've CC-ed Maxime and Giulio as well: they are looking into adding support for the stateless allwinner codec based on this code as well. There may well be opportunities for you to work together, esp. on the userspace side. Note that Rockchip has the same issue, they too have a stateless HW codec. > >> Anyway, I'm OK with merging this in staging. Although I think it should go >> to staging/media since we want to keep track of it. >> > > Awesome, I'll move driver to staging/media in V5. Thanks! Nice, thanks! Hans ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: lustre: Replace 'uint32_t' with 'u32' and 'uint64_t' with 'u64'
On Mon, Dec 04, 2017 at 10:58:23PM +0300, Dan Carpenter wrote: > On Wed, Nov 29, 2017 at 07:46:21PM +0300, Roman Storozhenko wrote: > > There are two reasons for that: > > What I'm asking is there are two reasons for what? Where is the first > part of that paragraph? Hello, Dan! Now I understand what did you mean. I shouldn't have written reason in the subject line and reference to it as 'that' in the body. Thanks for you mentioned that. In the following patches I will avoid such types of mistakes. Regards, Romans Storozhenko > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ccree: Uninitialized return in ssi_ahash_import()
The return value isn't initialized on some success paths.
Fixes: c5f39d07860c ("staging: ccree: fix leak of import() after init()")
Signed-off-by: Dan Carpenter
diff --git a/drivers/staging/ccree/ssi_hash.c b/drivers/staging/ccree/ssi_hash.c
index a2e8a9d32e00..3855c42e61af 100644
--- a/drivers/staging/ccree/ssi_hash.c
+++ b/drivers/staging/ccree/ssi_hash.c
@@ -1823,7 +1823,7 @@ static int ssi_ahash_import(struct ahash_request *req,
const void *in)
struct device *dev = drvdata_to_dev(ctx->drvdata);
struct ahash_req_ctx *state = ahash_request_ctx(req);
u32 tmp;
- int rc;
+ int rc = 0;
memcpy(&tmp, in, sizeof(u32));
if (tmp != CC_EXPORT_MAGIC) {
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[GIT PULL] Staging/IIO driver fixes for 4.15-rc3
The following changes since commit 4fbd8d194f06c8a3fd2af1ce560ddb31f7ec8323: Linux 4.15-rc1 (2017-11-26 16:01:47 -0800) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ tags/staging-4.15-rc3 for you to fetch changes up to e168e9845d956e6d0c057f9ecd28782a1ab4a08a: Merge tag 'iio-fixes-for-4.15a' of git://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio into staging-linus (2017-12-03 16:04:38 +0100) staging and iio driver fixes for 4.15-rc3 Here are a number of small staging and iio driver fixes for reported issues for 4.15-rc3. Nothing major here, the majority is IIO issues, like normal, but there are also some small bugfixes for a few staging drivers as well. Full details are in the shortlog. All of these have been in linux-next for a while with no reported issues. Signed-off-by: Greg Kroah-Hartman Aaro Koskinen (1): staging: octeon-usb: use __delay() instead of cvmx_wait() Andy Shevchenko (1): iio: proximity: sx9500: Assign interrupt from GpioIo() Arnd Bergmann (1): iio: stm32: fix adc/trigger link error Gilad Ben-Yossef (1): staging: ccree: fix leak of import() after init() Greg Kroah-Hartman (1): Merge tag 'iio-fixes-for-4.15a' of git://git.kernel.org/.../jic23/iio into staging-linus Larry Finger (1): staging: rtl8188eu: Fix incorrect response to SIOCGIWESSID Martin Blumenstingl (3): iio: adc: meson-saradc: fix the bit_idx of the adc_en clock iio: adc: meson-saradc: initialize the bandgap correctly on older SoCs iio: adc: meson-saradc: Meson8 and Meson8b do not have REG11 and REG13 Matthew Giassa (1): staging: comedi: ni_atmio: fix license warning. Pan Bian (1): iio: adc: cpcap: fix incorrect validation Peter Meerwald-Stadler (1): iio: health: max30102: Temperature should be in milli Celsius Randy Dunlap (1): iio: fix kernel-doc build errors drivers/iio/adc/cpcap-adc.c| 2 +- drivers/iio/adc/meson_saradc.c | 52 +- drivers/iio/health/max30102.c | 2 +- drivers/iio/industrialio-core.c| 4 +- drivers/iio/proximity/sx9500.c | 9 + drivers/staging/ccree/ssi_hash.c | 9 +++-- drivers/staging/comedi/drivers/ni_atmio.c | 5 +++ drivers/staging/octeon-usb/octeon-hcd.c| 6 +-- drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 14 ++- include/linux/iio/timer/stm32-lptim-trigger.h | 5 ++- 10 files changed, 77 insertions(+), 31 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: pi433: pi433_if.c codestyle brace on previous line
This patch fixes the following checkpatch.pl error:
WARNING: line over 80 characters
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, afterSyncInterrupt));
ERROR: that open brace { should be on the previous line
+ else
+ {
ERROR: else should follow close brace '}'
+ }
+ else
Signed-off-by: Oliver Graute
---
drivers/staging/pi433/pi433_if.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3404cb9..17be4b6 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -198,12 +198,10 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
/* packet config */
/* enable */
SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
- if (rx_cfg->enable_sync == optionOn)
- {
- SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
afterSyncInterrupt));
- }
- else
- {
+ if (rx_cfg->enable_sync == optionOn) {
+ SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
+afterSyncInterrupt));
+ } else {
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
}
if (rx_cfg->enable_length_byte == optionOn) {
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: pi433: codestyle space required
This patch fixes the following checkpatch.pl error:
ERROR: spaces required around that '+=' (ctx:WxV) +
position +=temp;
^
Signed-off-by: Oliver Graute
---
drivers/staging/pi433/pi433_if.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 17be4b6..198f448 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -614,7 +614,7 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
rf69_write_fifo(spi,
&buffer[position],
temp);
- position +=temp;
+ position += temp;
}
else
{ /* msg fits into fifo - take all */
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: pi433: pi433_if.c codestyle fix in IRQ_handler
This patch fixes the following checkpatch.pl errors:
ERROR: that open brace { should be on the previous line
#344: FILE: pi433_if.c:344:
+ if(retval) /* wait was interrupted */
+ {
ERROR: space required before the open parenthesis '('
#344: FILE: pi433_if.c:344:
+ if(retval) /* wait was interrupted */
Signed-off-by: Oliver Graute
---
drivers/staging/pi433/pi433_if.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3404cb9..26576bf 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -341,8 +341,7 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
/* wait for any tx to finish */
dev_dbg(dev->dev,"rx: going to wait for any tx to finish");
retval = wait_event_interruptible(dev->rx_wait_queue, !dev->tx_active);
- if(retval) /* wait was interrupted */
- {
+ if (retval) {
dev->interrupt_rx_allowed = true;
wake_up_interruptible(&dev->tx_wait_queue);
return retval;
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 05/11] staging: pi433: Rename enum modShaping in rf69_enum.h
Renames enum modShaping and its values to get rid of checkpatch.pl
warnings: "Avoid CamelCase: ".
Signed-off-by: Simon Sandström
---
drivers/staging/pi433/pi433_if.c | 2 +-
drivers/staging/pi433/pi433_if.h | 2 +-
drivers/staging/pi433/rf69.c | 21 +++--
drivers/staging/pi433/rf69.h | 2 +-
drivers/staging/pi433/rf69_enum.h | 14 +++---
5 files changed, 21 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 4f6830f369e9..2ae19ac565d1 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -260,7 +260,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct
pi433_tx_cfg *tx_cfg)
SET_CHECKED(rf69_set_modulation (dev->spi, tx_cfg->modulation));
SET_CHECKED(rf69_set_deviation (dev->spi, tx_cfg->dev_frequency));
SET_CHECKED(rf69_set_pa_ramp(dev->spi, tx_cfg->pa_ramp));
- SET_CHECKED(rf69_set_modulation_shaping(dev->spi, tx_cfg->modShaping));
+ SET_CHECKED(rf69_set_modulation_shaping(dev->spi, tx_cfg->mod_shaping));
SET_CHECKED(rf69_set_tx_start_condition(dev->spi,
tx_cfg->tx_start_condition));
/* packet format enable */
diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index 34ff0d4807bd..bcfe29840889 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -63,7 +63,7 @@ struct pi433_tx_cfg {
__u16 bit_rate;
__u32 dev_frequency;
enum modulation modulation;
- enum modShaping modShaping;
+ enum mod_shapingmod_shaping;
enum paRamp pa_ramp;
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index ebb3ddd1a957..b19bca8a0f26 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -109,27 +109,28 @@ enum modulation rf69_get_modulation(struct spi_device
*spi)
}
}
-int rf69_set_modulation_shaping(struct spi_device *spi, enum modShaping
modShaping)
+int rf69_set_modulation_shaping(struct spi_device *spi,
+ enum mod_shaping mod_shaping)
{
#ifdef DEBUG
dev_dbg(&spi->dev, "set: mod shaping");
#endif
if (rf69_get_modulation(spi) == FSK) {
- switch (modShaping) {
- case shapingOff: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_NONE);
- case shaping1_0: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_1_0);
- case shaping0_5: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_0_3);
- case shaping0_3: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_0_5);
+ switch (mod_shaping) {
+ case SHAPING_OFF: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_NONE);
+ case SHAPING_1_0: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_1_0);
+ case SHAPING_0_5: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_0_3);
+ case SHAPING_0_3: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_0_5);
default:
dev_dbg(&spi->dev, "set: illegal input param");
return -EINVAL;
}
} else {
- switch (modShaping) {
- case shapingOff: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_NONE);
- case shapingBR: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_BR);
- case shaping2BR: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_2BR);
+ switch (mod_shaping) {
+ case SHAPING_OFF: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_NONE);
+ case SHAPING_BR: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_BR);
+ case SHAPING_2BR: return WRITE_REG(REG_DATAMODUL,
(READ_REG(REG_DATAMODUL) & ~MASK_DATAMODUL_MODULATION_SHAPE) |
DATAMODUL_MODULATION_SHAPE_2BR);
[PATCH v2 01/11] staging: pi433: Fix indentation in rf69_enum.h
Basically just 's//\t/', to fix checkpatch.pl warnings:
"please, no spaces at the start of a line".
Signed-off-by: Simon Sandström
---
drivers/staging/pi433/rf69_enum.h | 207 +++---
1 file changed, 103 insertions(+), 104 deletions(-)
diff --git a/drivers/staging/pi433/rf69_enum.h
b/drivers/staging/pi433/rf69_enum.h
index 86429aa66ad1..babe597e2ec6 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -19,164 +19,163 @@
#define RF69_ENUM_H
enum optionOnOff {
-optionOff,
-optionOn
+ optionOff,
+ optionOn
};
enum mode {
-mode_sleep,
-standby,
-synthesizer,
-transmit,
-receive
+ mode_sleep,
+ standby,
+ synthesizer,
+ transmit,
+ receive
};
enum dataMode {
-packet,
-continuous,
-continuousNoSync
+ packet,
+ continuous,
+ continuousNoSync
};
enum modulation {
-OOK,
-FSK
+ OOK,
+ FSK
};
enum modShaping {
-shapingOff,
-shaping1_0,
-shaping0_5,
-shaping0_3,
-shapingBR,
-shaping2BR
+ shapingOff,
+ shaping1_0,
+ shaping0_5,
+ shaping0_3,
+ shapingBR,
+ shaping2BR
};
enum paRamp {
-ramp3400,
-ramp2000,
-ramp1000,
-ramp500,
-ramp250,
-ramp125,
-ramp100,
-ramp62,
-ramp50,
-ramp40,
-ramp31,
-ramp25,
-ramp20,
-ramp15,
-ramp12,
-ramp10
+ ramp3400,
+ ramp2000,
+ ramp1000,
+ ramp500,
+ ramp250,
+ ramp125,
+ ramp100,
+ ramp62,
+ ramp50,
+ ramp40,
+ ramp31,
+ ramp25,
+ ramp20,
+ ramp15,
+ ramp12,
+ ramp10
};
enum antennaImpedance {
-fiftyOhm,
-twohundretOhm
+ fiftyOhm,
+ twohundretOhm
};
enum lnaGain {
-automatic,
-max,
-maxMinus6,
-maxMinus12,
-maxMinus24,
-maxMinus36,
-maxMinus48,
-undefined
+ automatic,
+ max,
+ maxMinus6,
+ maxMinus12,
+ maxMinus24,
+ maxMinus36,
+ maxMinus48,
+ undefined
};
enum dccPercent {
-dcc16Percent,
-dcc8Percent,
-dcc4Percent,
-dcc2Percent,
-dcc1Percent,
-dcc0_5Percent,
-dcc0_25Percent,
-dcc0_125Percent
+ dcc16Percent,
+ dcc8Percent,
+ dcc4Percent,
+ dcc2Percent,
+ dcc1Percent,
+ dcc0_5Percent,
+ dcc0_25Percent,
+ dcc0_125Percent
};
enum mantisse {
-mantisse16,
-mantisse20,
-mantisse24
+ mantisse16,
+ mantisse20,
+ mantisse24
};
enum thresholdType {
-fixed,
-peak,
-average
+ fixed,
+ peak,
+ average
};
enum thresholdStep {
-step_0_5db,
-step_1_0db,
-step_1_5db,
-step_2_0db,
-step_3_0db,
-step_4_0db,
-step_5_0db,
-step_6_0db
+ step_0_5db,
+ step_1_0db,
+ step_1_5db,
+ step_2_0db,
+ step_3_0db,
+ step_4_0db,
+ step_5_0db,
+ step_6_0db
};
enum thresholdDecrement {
-dec_every8th,
-dec_every4th,
-dec_every2nd,
-dec_once,
-dec_twice,
-dec_4times,
-dec_8times,
-dec_16times
+ dec_every8th,
+ dec_every4th,
+ dec_every2nd,
+ dec_once,
+ dec_twice,
+ dec_4times,
+ dec_8times,
+ dec_16times
};
enum flag {
-modeSwitchCompleted,
-readyToReceive,
-readyToSend,
-pllLocked,
-rssiExceededThreshold,
-timeout,
-automode,
-syncAddressMatch,
-fifoFull,
-//fifoNotEmpty, collision with next enum; replaced by following enum...
-fifoEmpty,
-fifoLevelBelowThreshold,
-fifoOverrun,
-packetSent,
-payloadReady,
-crcOk,
-batteryLow
+ modeSwitchCompleted,
+ readyToReceive,
+ readyToSend,
+ pllLocked,
+ rssiExceededThreshold,
+ timeout,
+ automode,
+ syncAddressMatch,
+ fifoFull,
+// fifoNotEmpty, collision with next enum; replaced by following enum...
+ fifoEmpty,
+ fifoLevelBelowThreshold,
+ fifoOverrun,
+ packetSent,
+ payloadReady,
+ crcOk,
+ batteryLow
};
enum fifoFillCondition {
-afterSyncInterrupt,
-always
+ afterSyncInterrupt,
+ always
};
enum packetFormat {
-packetLengthFix,
-packetLengthVar
+ packetLengthFix,
+ packetLengthVar
};
enum txStartCondition {
-fifoLevel,
-fifoNotEmpty
+ fifoLevel,
+ fifoNotEmpty
};
enum addressFiltering {
-filteringOff,
-nodeAddress,
-nodeOrBroadcastAddress
+ filteringOff,
+ nodeAddress,
+ nodeOrBroadcastAddress
};
enum dagc {
-normalMode,
-improve,
-improve4LowModulationIndex
+ normalMode,
+ improve,
+ improve4LowModulationIndex
};
-
#endif
--
2.11.0
___
devel mailing list
de...@linux
[PATCH v2 02/11] staging: pi433: Capitalize constant definitions
Fixes checkpatch.pl warnings "Avoid CamelCase ".
Signed-off-by: Simon Sandström
---
drivers/staging/pi433/pi433_if.c | 32 -
drivers/staging/pi433/rf69_registers.h | 44 +-
2 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3404cb9722c9..840a7c7bf19a 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -133,20 +133,20 @@ static irqreturn_t DIO0_irq_handler(int irq, void *dev_id)
{
struct pi433_device *device = dev_id;
- if (device->irq_state[DIO0] == DIO_PacketSent)
+ if (device->irq_state[DIO0] == DIO_PACKET_SENT)
{
device->free_in_fifo = FIFO_SIZE;
dev_dbg(device->dev, "DIO0 irq: Packet sent\n");
wake_up_interruptible(&device->fifo_wait_queue);
}
- else if (device->irq_state[DIO0] == DIO_Rssi_DIO0)
+ else if (device->irq_state[DIO0] == DIO_RSSI_DIO0)
{
dev_dbg(device->dev, "DIO0 irq: RSSI level over threshold\n");
wake_up_interruptible(&device->rx_wait_queue);
}
- else if (device->irq_state[DIO0] == DIO_PayloadReady)
+ else if (device->irq_state[DIO0] == DIO_PAYLOAD_READY)
{
- dev_dbg(device->dev, "DIO0 irq: PayloadReady\n");
+ dev_dbg(device->dev, "DIO0 irq: Payload ready\n");
device->free_in_fifo = 0;
wake_up_interruptible(&device->fifo_wait_queue);
}
@@ -158,11 +158,11 @@ static irqreturn_t DIO1_irq_handler(int irq, void *dev_id)
{
struct pi433_device *device = dev_id;
- if (device->irq_state[DIO1] == DIO_FifoNotEmpty_DIO1)
+ if (device->irq_state[DIO1] == DIO_FIFO_NOT_EMPTY_DIO1)
{
device->free_in_fifo = FIFO_SIZE;
}
- else if (device->irq_state[DIO1] == DIO_FifoLevel)
+ else if (device->irq_state[DIO1] == DIO_FIFO_LEVEL)
{
if (device->rx_active) device->free_in_fifo = FIFO_THRESHOLD -
1;
elsedevice->free_in_fifo = FIFO_SIZE -
FIFO_THRESHOLD - 1;
@@ -309,14 +309,14 @@ pi433_start_rx(struct pi433_device *dev)
if (retval) return retval;
/* setup rssi irq */
- SET_CHECKED(rf69_set_dio_mapping(dev->spi, DIO0, DIO_Rssi_DIO0));
- dev->irq_state[DIO0] = DIO_Rssi_DIO0;
+ SET_CHECKED(rf69_set_dio_mapping(dev->spi, DIO0, DIO_RSSI_DIO0));
+ dev->irq_state[DIO0] = DIO_RSSI_DIO0;
irq_set_irq_type(dev->irq_num[DIO0], IRQ_TYPE_EDGE_RISING);
/* setup fifo level interrupt */
SET_CHECKED(rf69_set_fifo_threshold(dev->spi, FIFO_SIZE -
FIFO_THRESHOLD));
- SET_CHECKED(rf69_set_dio_mapping(dev->spi, DIO1, DIO_FifoLevel));
- dev->irq_state[DIO1] = DIO_FifoLevel;
+ SET_CHECKED(rf69_set_dio_mapping(dev->spi, DIO1, DIO_FIFO_LEVEL));
+ dev->irq_state[DIO1] = DIO_FIFO_LEVEL;
irq_set_irq_type(dev->irq_num[DIO1], IRQ_TYPE_EDGE_RISING);
/* set module to receiving mode */
@@ -378,8 +378,8 @@ pi433_receive(void *data)
}
/* configure payload ready irq */
- SET_CHECKED(rf69_set_dio_mapping(spi, DIO0, DIO_PayloadReady));
- dev->irq_state[DIO0] = DIO_PayloadReady;
+ SET_CHECKED(rf69_set_dio_mapping(spi, DIO0, DIO_PAYLOAD_READY));
+ dev->irq_state[DIO0] = DIO_PAYLOAD_READY;
irq_set_irq_type(dev->irq_num[DIO0], IRQ_TYPE_EDGE_RISING);
/* fixed or unlimited length? */
@@ -590,13 +590,13 @@ pi433_tx_thread(void *data)
rf69_set_tx_cfg(device, &tx_cfg);
/* enable fifo level interrupt */
- SET_CHECKED(rf69_set_dio_mapping(spi, DIO1, DIO_FifoLevel));
- device->irq_state[DIO1] = DIO_FifoLevel;
+ SET_CHECKED(rf69_set_dio_mapping(spi, DIO1, DIO_FIFO_LEVEL));
+ device->irq_state[DIO1] = DIO_FIFO_LEVEL;
irq_set_irq_type(device->irq_num[DIO1], IRQ_TYPE_EDGE_FALLING);
/* enable packet sent interrupt */
- SET_CHECKED(rf69_set_dio_mapping(spi, DIO0, DIO_PacketSent));
- device->irq_state[DIO0] = DIO_PacketSent;
+ SET_CHECKED(rf69_set_dio_mapping(spi, DIO0, DIO_PACKET_SENT));
+ device->irq_state[DIO0] = DIO_PACKET_SENT;
irq_set_irq_type(device->irq_num[DIO0], IRQ_TYPE_EDGE_RISING);
enable_irq(device->irq_num[DIO0]); /* was disabled by rx active
check */
diff --git a/drivers/staging/pi433/rf69_registers.h
b/drivers/staging/pi433/rf69_registers.h
index 6335d42142fe..d23b8b668ef5 100644
--- a/drivers/staging/pi433/rf69_registers.h
+++ b/drivers/staging/pi433/rf69_registers.h
@@ -346,28 +346,28 @@
#define DIO5 5
/* DIO Mapping values (packet mode) */
-#define DIO_ModeReady_DIO4
[PATCH v2 04/11] staging: pi433: Rename enum optionOnOff in rf69_enum.h
Renames the enum optionOnOff and its values optionOn, optionOff to enum
option_on_off and OPTION_ON, OPTION_OFF. Fixes checkpatch.pl warnings:
"Avoid CamelCase: , , ".
Signed-off-by: Simon Sandström
---
drivers/staging/pi433/pi433_if.c | 34 ++---
drivers/staging/pi433/pi433_if.h | 16 +++---
drivers/staging/pi433/rf69.c | 45 ++-
drivers/staging/pi433/rf69.h | 15 -
drivers/staging/pi433/rf69_enum.h | 6 +++---
5 files changed, 63 insertions(+), 53 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index b8efe6a74a2a..4f6830f369e9 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -198,7 +198,7 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct
pi433_rx_cfg *rx_cfg)
/* packet config */
/* enable */
SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
- if (rx_cfg->enable_sync == optionOn)
+ if (rx_cfg->enable_sync == OPTION_ON)
{
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
afterSyncInterrupt));
}
@@ -206,7 +206,7 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct
pi433_rx_cfg *rx_cfg)
{
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
}
- if (rx_cfg->enable_length_byte == optionOn) {
+ if (rx_cfg->enable_length_byte == OPTION_ON) {
ret = rf69_set_packet_format(dev->spi, packetLengthVar);
if (ret < 0)
return ret;
@@ -220,14 +220,14 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct
pi433_rx_cfg *rx_cfg)
/* lengths */
SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
- if (rx_cfg->enable_length_byte == optionOn)
+ if (rx_cfg->enable_length_byte == OPTION_ON)
{
SET_CHECKED(rf69_set_payload_length(dev->spi, 0xff));
}
else if (rx_cfg->fixed_message_length != 0)
{
payload_length = rx_cfg->fixed_message_length;
- if (rx_cfg->enable_length_byte == optionOn) payload_length++;
+ if (rx_cfg->enable_length_byte == OPTION_ON) payload_length++;
if (rx_cfg->enable_address_filtering != filteringOff)
payload_length++;
SET_CHECKED(rf69_set_payload_length(dev->spi, payload_length));
}
@@ -237,7 +237,7 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct
pi433_rx_cfg *rx_cfg)
}
/* values */
- if (rx_cfg->enable_sync == optionOn)
+ if (rx_cfg->enable_sync == OPTION_ON)
{
SET_CHECKED(rf69_set_sync_values(dev->spi,
rx_cfg->sync_pattern));
}
@@ -264,7 +264,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct
pi433_tx_cfg *tx_cfg)
SET_CHECKED(rf69_set_tx_start_condition(dev->spi,
tx_cfg->tx_start_condition));
/* packet format enable */
- if (tx_cfg->enable_preamble == optionOn)
+ if (tx_cfg->enable_preamble == OPTION_ON)
{
SET_CHECKED(rf69_set_preamble_length(dev->spi,
tx_cfg->preamble_length));
}
@@ -273,7 +273,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct
pi433_tx_cfg *tx_cfg)
SET_CHECKED(rf69_set_preamble_length(dev->spi, 0));
}
SET_CHECKED(rf69_set_sync_enable (dev->spi, tx_cfg->enable_sync));
- if (tx_cfg->enable_length_byte == optionOn) {
+ if (tx_cfg->enable_length_byte == OPTION_ON) {
ret = rf69_set_packet_format(dev->spi, packetLengthVar);
if (ret < 0)
return ret;
@@ -285,7 +285,7 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct
pi433_tx_cfg *tx_cfg)
SET_CHECKED(rf69_set_crc_enable (dev->spi, tx_cfg->enable_crc));
/* configure sync, if enabled */
- if (tx_cfg->enable_sync == optionOn) {
+ if (tx_cfg->enable_sync == OPTION_ON) {
SET_CHECKED(rf69_set_sync_size(dev->spi, tx_cfg->sync_length));
SET_CHECKED(rf69_set_sync_values(dev->spi,
tx_cfg->sync_pattern));
}
@@ -400,7 +400,7 @@ pi433_receive(void *data)
}
/* length byte enabled? */
- if (dev->rx_cfg.enable_length_byte == optionOn)
+ if (dev->rx_cfg.enable_length_byte == OPTION_ON)
{
retval = wait_event_interruptible(dev->fifo_wait_queue,
dev->free_in_fifo <
FIFO_SIZE);
@@ -525,11 +525,11 @@ pi433_tx_thread(void *data)
size = tx_cfg.fixed_message_length;
/* increase size, if len byte is requested */
- if (tx_cfg.enable_length_byte == optionOn)
+ if (tx_cfg.enable_length_byte == OPTION_ON)
size++;
/* increase size, if adr byte is requested */
- if (tx_cfg.enable_address_byte == option
[PATCH v2 00/11] Fix indentation and CamelCase issue in staging/pi433
These patches fixes a bunch of code style issues in staging/pi433. The first patch fixes indentation in rf69_enum.h and the rest of the patches fixes CamelCase issues and other minor issues in all of staging/pi433. Changes in v2: - Instead of fixing CamelCase issue in enum data_mode, remove it completely. - Split multiple functions of type "rf69_set_x_enabled(dev, enum option_on_off)" to two functions: "rf69_enable_x(dev)" and "rf69_disable_x(dev)". - Move enum option_on_off to pi433_if.h as it's now only used for ioctl. - Fix issue where GPIOs and device not being deallocated in pi433_probe() if SET_CHECKED fails. - Simon --- Simon Sandström (11): staging: pi433: Fix indentation in rf69_enum.h staging: pi433: Capitalize constant definitions staging: pi433: Rename variable in struct pi433_rx_cfg staging: pi433: Rename enum optionOnOff in rf69_enum.h staging: pi433: Rename enum modShaping in rf69_enum.h staging: pi433: Split rf69_set_crc_enabled into two functions staging: pi433: Split rf69_set_sync_enabled into two functions staging: pi433: Remove enum data_mode staging: pi433: Combine all rf69_set_amplifier_x() staging: pi433: Move enum option_on_off to pi433_if.h staging: pi433: Remove SET_CHECKED usage from pi433_probe drivers/staging/pi433/pi433_if.c | 135 ++--- drivers/staging/pi433/pi433_if.h | 25 ++-- drivers/staging/pi433/rf69.c | 113 +- drivers/staging/pi433/rf69.h | 15 +-- drivers/staging/pi433/rf69_enum.h | 210 - drivers/staging/pi433/rf69_registers.h | 44 +++ 6 files changed, 265 insertions(+), 277 deletions(-) -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 08/11] staging: pi433: Remove enum data_mode
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
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 11/11] staging: pi433: Remove SET_CHECKED usage from pi433_probe
SET_CHECKED returns from the function on failure and in pi433_probe it is necessary to free the GPIOs and the device on failure. Signed-off-by: Simon Sandström --- drivers/staging/pi433/pi433_if.c | 28 +--- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c index 688d0cf00782..55ed45f45998 100644 --- a/drivers/staging/pi433/pi433_if.c +++ b/drivers/staging/pi433/pi433_if.c @@ -1124,13 +1124,27 @@ 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, DATAMODUL_MODE_PACKET)); - SET_CHECKED(rf69_enable_amplifier(spi, MASK_PALEVEL_PA0)); - SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA1)); - SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA2)); - SET_CHECKED(rf69_set_output_power_level (spi, 13)); - SET_CHECKED(rf69_set_antenna_impedance (spi, fiftyOhm)); + retval = rf69_set_mode(spi, standby); + if (retval < 0) + goto minor_failed; + retval = rf69_set_data_mode(spi, DATAMODUL_MODE_PACKET); + if (retval < 0) + goto minor_failed; + retval = rf69_enable_amplifier(spi, MASK_PALEVEL_PA0); + if (retval < 0) + goto minor_failed; + retval = rf69_disable_amplifier(spi, MASK_PALEVEL_PA1); + if (retval < 0) + goto minor_failed; + retval = rf69_disable_amplifier(spi, MASK_PALEVEL_PA2); + if (retval < 0) + goto minor_failed; + retval = rf69_set_output_power_level(spi, 13); + if (retval < 0) + goto minor_failed; + retval = rf69_set_antenna_impedance(spi, fiftyOhm); + if (retval < 0) + goto minor_failed; /* determ minor number */ retval = pi433_get_minor(device); -- 2.11.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 06/11] staging: pi433: Split rf69_set_crc_enabled into two functions
Splits rf69_set_crc_enabled(dev, enabled) into
rf69_enable_crc(dev) and rf69_disable_crc(dev).
Signed-off-by: Simon Sandström
---
drivers/staging/pi433/pi433_if.c | 22 --
drivers/staging/pi433/rf69.c | 18 ++
drivers/staging/pi433/rf69.h | 4 ++--
3 files changed, 28 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 2ae19ac565d1..614eec7dd904 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -216,7 +216,16 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct
pi433_rx_cfg *rx_cfg)
return ret;
}
SET_CHECKED(rf69_set_adressFiltering(dev->spi,
rx_cfg->enable_address_filtering));
- SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
+
+ if (rx_cfg->enable_crc == OPTION_ON) {
+ ret = rf69_enable_crc(dev->spi);
+ if (ret < 0)
+ return ret;
+ } else {
+ ret = rf69_disable_crc(dev->spi);
+ if (ret < 0)
+ return ret;
+ }
/* lengths */
SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
@@ -282,7 +291,16 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct
pi433_tx_cfg *tx_cfg)
if (ret < 0)
return ret;
}
- SET_CHECKED(rf69_set_crc_enable (dev->spi, tx_cfg->enable_crc));
+
+ if (tx_cfg->enable_crc == OPTION_ON) {
+ ret = rf69_enable_crc(dev->spi);
+ if (ret < 0)
+ return ret;
+ } else {
+ ret = rf69_disable_crc(dev->spi);
+ if (ret < 0)
+ return ret;
+ }
/* configure sync, if enabled */
if (tx_cfg->enable_sync == OPTION_ON) {
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index b19bca8a0f26..612d59f61f88 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -822,20 +822,14 @@ int rf69_set_packet_format(struct spi_device *spi, enum
packetFormat packetForma
}
}
-int rf69_set_crc_enable(struct spi_device *spi,
- enum option_on_off option_on_off)
+int rf69_enable_crc(struct spi_device *spi)
{
- #ifdef DEBUG
- dev_dbg(&spi->dev, "set: crc enable");
- #endif
+ return WRITE_REG(REG_PACKETCONFIG1, (READ_REG(REG_PACKETCONFIG1) |
MASK_PACKETCONFIG1_CRC_ON));
+}
- switch (option_on_off) {
- case OPTION_ON: return WRITE_REG(REG_PACKETCONFIG1,
(READ_REG(REG_PACKETCONFIG1) | MASK_PACKETCONFIG1_CRC_ON));
- case OPTION_OFF: return WRITE_REG(REG_PACKETCONFIG1,
(READ_REG(REG_PACKETCONFIG1) & ~MASK_PACKETCONFIG1_CRC_ON));
- default:
- dev_dbg(&spi->dev, "set: illegal input param");
- return -EINVAL;
- }
+int rf69_disable_crc(struct spi_device *spi)
+{
+ return WRITE_REG(REG_PACKETCONFIG1, (READ_REG(REG_PACKETCONFIG1) &
~MASK_PACKETCONFIG1_CRC_ON));
}
int rf69_set_adressFiltering(struct spi_device *spi, enum addressFiltering
addressFiltering)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 1cb6db33d6fe..9428dee97de7 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -66,8 +66,8 @@ int rf69_set_sync_size(struct spi_device *spi, u8 sync_size);
int rf69_set_sync_tolerance(struct spi_device *spi, u8 syncTolerance);
int rf69_set_sync_values(struct spi_device *spi, u8 syncValues[8]);
int rf69_set_packet_format(struct spi_device *spi, enum packetFormat
packetFormat);
-int rf69_set_crc_enable(struct spi_device *spi,
- enum option_on_off option_on_off);
+int rf69_enable_crc(struct spi_device *spi);
+int rf69_disable_crc(struct spi_device *spi);
int rf69_set_adressFiltering(struct spi_device *spi, enum addressFiltering
addressFiltering);
int rf69_set_payload_length(struct spi_device *spi, u8 payloadLength);
u8 rf69_get_payload_length(struct spi_device *spi);
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 10/11] staging: pi433: Move enum option_on_off to pi433_if.h
The enum is now only used for ioctl, so move it pi433_if.h.
Signed-off-by: Simon Sandström
---
drivers/staging/pi433/pi433_if.h | 5 +
drivers/staging/pi433/rf69_enum.h | 5 -
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index bcfe29840889..c8697d144e2b 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -37,6 +37,11 @@
/*---*/
+enum option_on_off {
+ OPTION_OFF,
+ OPTION_ON
+};
+
/* IOCTL structs and commands */
/**
diff --git a/drivers/staging/pi433/rf69_enum.h
b/drivers/staging/pi433/rf69_enum.h
index b0715b4eb6ac..4e64e38ae4ff 100644
--- a/drivers/staging/pi433/rf69_enum.h
+++ b/drivers/staging/pi433/rf69_enum.h
@@ -18,11 +18,6 @@
#ifndef RF69_ENUM_H
#define RF69_ENUM_H
-enum option_on_off {
- OPTION_OFF,
- OPTION_ON
-};
-
enum mode {
mode_sleep,
standby,
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 07/11] staging: pi433: Split rf69_set_sync_enabled into two functions
Splits rf69_set_sync_enabled(dev, enabled) into
rf69_enable_sync(dev) and rf69_disable_sync(dev).
Signed-off-by: Simon Sandström
---
drivers/staging/pi433/pi433_if.c | 21 +++--
drivers/staging/pi433/rf69.c | 18 ++
drivers/staging/pi433/rf69.h | 4 ++--
3 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 614eec7dd904..fb500d062df8 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -197,13 +197,20 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct
pi433_rx_cfg *rx_cfg)
/* packet config */
/* enable */
- SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
if (rx_cfg->enable_sync == OPTION_ON)
{
+ ret = rf69_enable_sync(dev->spi);
+ if (ret < 0)
+ return ret;
+
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
afterSyncInterrupt));
}
else
{
+ ret = rf69_disable_sync(dev->spi);
+ if (ret < 0)
+ return ret;
+
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
}
if (rx_cfg->enable_length_byte == OPTION_ON) {
@@ -281,7 +288,17 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct
pi433_tx_cfg *tx_cfg)
{
SET_CHECKED(rf69_set_preamble_length(dev->spi, 0));
}
- SET_CHECKED(rf69_set_sync_enable (dev->spi, tx_cfg->enable_sync));
+
+ if (tx_cfg->enable_sync == OPTION_ON) {
+ ret = rf69_enable_sync(dev->spi);
+ if (ret < 0)
+ return ret;
+ } else {
+ ret = rf69_disable_sync(dev->spi);
+ if (ret < 0)
+ return ret;
+ }
+
if (tx_cfg->enable_length_byte == OPTION_ON) {
ret = rf69_set_packet_format(dev->spi, packetLengthVar);
if (ret < 0)
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 612d59f61f88..e5b29bed35ef 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -724,20 +724,14 @@ int rf69_set_preamble_length(struct spi_device *spi, u16
preambleLength)
return WRITE_REG(REG_PREAMBLE_LSB, lsb);
}
-int rf69_set_sync_enable(struct spi_device *spi,
-enum option_on_off option_on_off)
+int rf69_enable_sync(struct spi_device *spi)
{
- #ifdef DEBUG
- dev_dbg(&spi->dev, "set: sync enable");
- #endif
+ return WRITE_REG(REG_SYNC_CONFIG, (READ_REG(REG_SYNC_CONFIG) |
MASK_SYNC_CONFIG_SYNC_ON));
+}
- switch (option_on_off) {
- case OPTION_ON: return WRITE_REG(REG_SYNC_CONFIG,
(READ_REG(REG_SYNC_CONFIG) | MASK_SYNC_CONFIG_SYNC_ON));
- case OPTION_OFF: return WRITE_REG(REG_SYNC_CONFIG,
(READ_REG(REG_SYNC_CONFIG) & ~MASK_SYNC_CONFIG_SYNC_ON));
- default:
- dev_dbg(&spi->dev, "set: illegal input param");
- return -EINVAL;
- }
+int rf69_disable_sync(struct spi_device *spi)
+{
+ return WRITE_REG(REG_SYNC_CONFIG, (READ_REG(REG_SYNC_CONFIG) &
~MASK_SYNC_CONFIG_SYNC_ON));
}
int rf69_set_fifo_fill_condition(struct spi_device *spi, enum
fifoFillCondition fifoFillCondition)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 9428dee97de7..177223451c87 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -59,8 +59,8 @@ int rf69_set_rssi_threshold(struct spi_device *spi, u8
threshold);
int rf69_set_rx_start_timeout(struct spi_device *spi, u8 timeout);
int rf69_set_rssi_timeout(struct spi_device *spi, u8 timeout);
int rf69_set_preamble_length(struct spi_device *spi, u16 preambleLength);
-int rf69_set_sync_enable(struct spi_device *spi,
-enum option_on_off option_on_off);
+int rf69_enable_sync(struct spi_device *spi);
+int rf69_disable_sync(struct spi_device *spi);
int rf69_set_fifo_fill_condition(struct spi_device *spi, enum
fifoFillCondition fifoFillCondition);
int rf69_set_sync_size(struct spi_device *spi, u8 sync_size);
int rf69_set_sync_tolerance(struct spi_device *spi, u8 syncTolerance);
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 03/11] staging: pi433: Rename variable in struct pi433_rx_cfg
Renames variable thresholdDecrement in struct pi433_rx_cfg to
threshold_decrement to get rid of checkpatch.pl warning
"Avoid CamelCase: ".
Signed-off-by: Simon Sandström
---
drivers/staging/pi433/pi433_if.c | 2 +-
drivers/staging/pi433/pi433_if.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 840a7c7bf19a..b8efe6a74a2a 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -188,7 +188,7 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct
pi433_rx_cfg *rx_cfg)
SET_CHECKED(rf69_set_modulation (dev->spi, rx_cfg->modulation));
SET_CHECKED(rf69_set_antenna_impedance (dev->spi,
rx_cfg->antenna_impedance));
SET_CHECKED(rf69_set_rssi_threshold (dev->spi,
rx_cfg->rssi_threshold));
- SET_CHECKED(rf69_set_ook_threshold_dec (dev->spi,
rx_cfg->thresholdDecrement));
+ SET_CHECKED(rf69_set_ook_threshold_dec (dev->spi,
rx_cfg->threshold_decrement));
SET_CHECKED(rf69_set_bandwidth (dev->spi,
rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
SET_CHECKED(rf69_set_bandwidth_during_afc(dev->spi,
rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
SET_CHECKED(rf69_set_dagc(dev->spi, rx_cfg->dagc));
diff --git a/drivers/staging/pi433/pi433_if.h b/drivers/staging/pi433/pi433_if.h
index fc842c48c33e..6b31c1584b3a 100644
--- a/drivers/staging/pi433/pi433_if.h
+++ b/drivers/staging/pi433/pi433_if.h
@@ -115,7 +115,7 @@ struct pi433_rx_cfg {
enum modulation modulation;
__u8rssi_threshold;
- enum thresholdDecrement thresholdDecrement;
+ enum thresholdDecrement threshold_decrement;
enum antennaImpedance antenna_impedance;
enum lnaGainlna_gain;
enum mantisse bw_mantisse;/* normal: 0x50 */
--
2.11.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 09/11] staging: pi433: Combine all rf69_set_amplifier_x()
Replaces the functions rf69_set_amplifier_1, _2, _3 with two
functions: rf69_enable_amplifier(dev, amp_mask) and
rf69_disable_amplifier(dev, amp_mask).
Signed-off-by: Simon Sandström
---
drivers/staging/pi433/pi433_if.c | 6 +++---
drivers/staging/pi433/rf69.c | 46
drivers/staging/pi433/rf69.h | 8 ++-
3 files changed, 9 insertions(+), 51 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index 3b4170b9ba94..688d0cf00782 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -1126,9 +1126,9 @@ 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, 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));
+ SET_CHECKED(rf69_enable_amplifier(spi, MASK_PALEVEL_PA0));
+ SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA1));
+ SET_CHECKED(rf69_disable_amplifier(spi, MASK_PALEVEL_PA2));
SET_CHECKED(rf69_set_output_power_level (spi, 13));
SET_CHECKED(rf69_set_antenna_impedance (spi, fiftyOhm));
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index 4c9eb97978ef..c97c65ea8acd 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -262,52 +262,14 @@ int rf69_set_frequency(struct spi_device *spi, u32
frequency)
return 0;
}
-int rf69_set_amplifier_0(struct spi_device *spi,
-enum option_on_off option_on_off)
+int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask)
{
- #ifdef DEBUG
- dev_dbg(&spi->dev, "set: amp #0");
- #endif
-
- switch (option_on_off) {
- case OPTION_ON: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) |
MASK_PALEVEL_PA0));
- case OPTION_OFF: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) &
~MASK_PALEVEL_PA0));
- default:
- dev_dbg(&spi->dev, "set: illegal input param");
- return -EINVAL;
- }
-}
-
-int rf69_set_amplifier_1(struct spi_device *spi,
-enum option_on_off option_on_off)
-{
- #ifdef DEBUG
- dev_dbg(&spi->dev, "set: amp #1");
- #endif
-
- switch (option_on_off) {
- case OPTION_ON: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) |
MASK_PALEVEL_PA1));
- case OPTION_OFF: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) &
~MASK_PALEVEL_PA1));
- default:
- dev_dbg(&spi->dev, "set: illegal input param");
- return -EINVAL;
- }
+ return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) | amplifier_mask));
}
-int rf69_set_amplifier_2(struct spi_device *spi,
-enum option_on_off option_on_off)
+int rf69_disable_amplifier(struct spi_device *spi, u8 amplifier_mask)
{
- #ifdef DEBUG
- dev_dbg(&spi->dev, "set: amp #2");
- #endif
-
- switch (option_on_off) {
- case OPTION_ON: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) |
MASK_PALEVEL_PA2));
- case OPTION_OFF: return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) &
~MASK_PALEVEL_PA2));
- default:
- dev_dbg(&spi->dev, "set: illegal input param");
- return -EINVAL;
- }
+ return WRITE_REG(REG_PALEVEL, (READ_REG(REG_PALEVEL) &
~amplifier_mask));
}
int rf69_set_output_power_level(struct spi_device *spi, u8 powerLevel)
diff --git a/drivers/staging/pi433/rf69.h b/drivers/staging/pi433/rf69.h
index 18296b4502f3..ba25ab6aeae7 100644
--- a/drivers/staging/pi433/rf69.h
+++ b/drivers/staging/pi433/rf69.h
@@ -33,12 +33,8 @@ int rf69_set_modulation_shaping(struct spi_device *spi, enum
mod_shaping mod_sha
int rf69_set_bit_rate(struct spi_device *spi, u16 bitRate);
int rf69_set_deviation(struct spi_device *spi, u32 deviation);
int rf69_set_frequency(struct spi_device *spi, u32 frequency);
-int rf69_set_amplifier_0(struct spi_device *spi,
-enum option_on_off option_on_off);
-int rf69_set_amplifier_1(struct spi_device *spi,
-enum option_on_off option_on_off);
-int rf69_set_amplifier_2(struct spi_device *spi,
-enum option_on_off option_on_off);
+int rf69_enable_amplifier(struct spi_device *spi, u8 amplifier_mask);
+int rf69_disable_amplifier(struct spi_device *spi, u8 amplifier_mask);
int rf69_set_output_power_level(struct spi_device *spi, u8 powerLevel);
int rf69_set_pa_ramp(struct spi_device *spi, enum paRamp paRamp);
int rf69_set_antenna_impedance(struct spi_device *spi, enum antennaImpedance
antennaImpedance);
--
2.11.0
Re: [PATCH v6 2/2] staging: ion: create one device entry per heap
On 12/02/2017 07:53 AM, Greg KH wrote: This is one of the item in the TODO list before been able to unstage ION which is my real need. Why does it matter where in the tree this code is? Don't go adding new things to it that are not needed. Who needs this? What userspace code wants this type of multiple ion devices? Requirements came in from several places to split /dev/ion -> /dev/ion0 and /dev/ion1 so that security policy (i.e. selinux) could be used to protect access to certain heaps. I wanted the ABI to be settled before trying to move out of staging, hence the line in the TODO list about doing the split. thanks, greg k-h Thanks, Laura ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/3] staging: vc04_services: Unsplit user-visible strings
This was found using checkpatch.pl's SPLIT_STRING warning. While joining
these strings makes for long lines, the kernel codebase consistently
does it this way to make user-visible strings easier to grep for.
Signed-off-by: Genki Sky
---
drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 7 +++
drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 3 +--
2 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 4ed3b449f97f..e26895dc052e 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -347,8 +347,7 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
timestamp =
ktime_add_us(dev->capture.kernel_start_ts,
runtime_us);
v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
-"Convert start time %llu and %llu "
-"with offset %llu to %llu\n",
+"Convert start time %llu and %llu with
offset %llu to %llu\n",
ktime_to_ns(dev->capture.kernel_start_ts),
dev->capture.vc_start_timestamp, pts,
ktime_to_ns(timestamp));
@@ -532,8 +531,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned
int count)
vchiq_mmal_port_enable(dev->instance, dev->capture.port, buffer_cb);
if (ret) {
v4l2_err(&dev->v4l2_dev,
- "Failed to enable capture port - error %d. "
- "Disabling camera port again\n", ret);
+ "Failed to enable capture port - error %d. Disabling
camera port again\n",
+ ret);
vchiq_mmal_port_disable(dev->instance,
dev->capture.camera_port);
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 6ea7fb0ea50e..5ddea4f54bf7 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -1360,8 +1360,7 @@ static int port_action_handle(struct vchiq_mmal_instance
*instance,
ret = -rmsg->u.port_action_reply.status;
- pr_debug("%s:result:%d component:0x%x port:%d action:%s(%d)" \
-" connect component:0x%x connect port:%d\n",
+ pr_debug("%s:result:%d component:0x%x port:%d action:%s(%d) connect
component:0x%x connect port:%d\n",
__func__,
ret, port->component->handle, port->handle,
port_action_type_names[action_type],
--
2.15.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/3] staging: vc04_services: Use __func__
This was found using checkpatch.pl's EMBEDDED_FUNCTION_NAME warning.
It is easier to be consistent and always use __func__ instead of having
to remember to update any hardcoded references to the original name.
Signed-off-by: Genki Sky
---
drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 5ddea4f54bf7..6c4d8b4c7cd9 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -618,8 +618,8 @@ static void buffer_to_host_cb(struct vchiq_mmal_instance
*instance,
struct mmal_msg_context *msg_context;
u32 handle;
- pr_debug("buffer_to_host_cb: instance:%p msg:%p msg_len:%d\n",
-instance, msg, msg_len);
+ pr_debug("%s: instance:%p msg:%p msg_len:%d\n",
+__func__, instance, msg, msg_len);
if (msg->u.buffer_from_host.drvbuf.magic == MMAL_MAGIC) {
handle = msg->u.buffer_from_host.drvbuf.client_context;
--
2.15.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/3] staging: vc04_services: Fix small style issues
Hi, This is v3 of a patch series to fix some checkpatch.pl warnings in drivers/staging/vc04_services/bcm2835-camera/* Changes from v2 [2]: - Provide more detailed description in commit messages Changes from v1 [1]: - Split single patch into multiple patches - Remove reference to eudyptula in commit message [1]: https://lkml.kernel.org/r/30cc7b48def470f1bec8d8d255044f4f220531ee.1512368114.git@genki.is [2]: https://lkml.kernel.org/r/cover.1512441073.git@genki.is [ aside: this is for task-10 of http://eudyptula-challenge.org/ ] Genki Sky (3): staging: vc04_services: Join multiline dereferences staging: vc04_services: Unsplit user-visible strings staging: vc04_services: Use __func__ .../vc04_services/bcm2835-camera/bcm2835-camera.c | 23 +- .../vc04_services/bcm2835-camera/mmal-vchiq.c | 7 +++ 2 files changed, 12 insertions(+), 18 deletions(-) -- 2.15.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/3] staging: vc04_services: Join multiline dereferences
This was found using checkpatch.pl's MULTILINE_DEREFERENCE warning.
Putting the dereference onto one line makes them easier to read,
especially when part of a larger expression (in this case, function
arguments and ternary operator), and when the dereferences are short
(as they are here).
Signed-off-by: Genki Sky
---
.../vc04_services/bcm2835-camera/bcm2835-camera.c| 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index d8766b166675..4ed3b449f97f 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -328,11 +328,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
pr_debug("Grab another frame");
vchiq_mmal_port_parameter_set(
instance,
- dev->capture.
- camera_port,
+ dev->capture.camera_port,
MMAL_PARAMETER_CAPTURE,
- &dev->capture.
- frame_count,
+ &dev->capture.frame_count,
sizeof(dev->capture.frame_count));
}
} else {
@@ -368,11 +366,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
"Grab another frame as buffer has
EOS");
vchiq_mmal_port_parameter_set(
instance,
- dev->capture.
- camera_port,
+ dev->capture.camera_port,
MMAL_PARAMETER_CAPTURE,
- &dev->capture.
- frame_count,
+ &dev->capture.frame_count,
sizeof(dev->capture.frame_count));
}
} else {
@@ -1194,8 +1190,8 @@ static int mmal_setup_components(struct bm2835_mmal_dev
*dev,
port->current_buffer.size =
(f->fmt.pix.sizeimage <
(100 << 10))
- ? (100 << 10) : f->fmt.pix.
- sizeimage;
+ ? (100 << 10)
+ : f->fmt.pix.sizeimage;
}
v4l2_dbg(1, bcm2835_v4l2_debug,
&dev->v4l2_dev,
--
2.15.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 2/2] staging: ion: create one device entry per heap
On Tue, Dec 05, 2017 at 03:01:42PM -0800, Laura Abbott wrote: > On 12/02/2017 07:53 AM, Greg KH wrote: > > > This is one of the item in the TODO list before been able to unstage ION > > > which is my real need. > > Why does it matter where in the tree this code is? Don't go adding new > > things to it that are not needed. Who needs this? What userspace code > > wants this type of multiple ion devices? > > > > Requirements came in from several places to split /dev/ion -> /dev/ion0 > and /dev/ion1 so that security policy (i.e. selinux) could be used to > protect access to certain heaps. I wanted the ABI to be settled before > trying to move out of staging, hence the line in the TODO list about > doing the split. Ok, but we should have some way of testing it works, right? :) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: rtl8192u: Fix no spaces around '+'
Added spaces around '+'. Warning found using checkpatch.pl --- drivers/staging/rtl8192u/ieee80211/dot11d.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.c b/drivers/staging/rtl8192u/ieee80211/dot11d.c index 64b13a5..ba284bf 100644 --- a/drivers/staging/rtl8192u/ieee80211/dot11d.c +++ b/drivers/staging/rtl8192u/ieee80211/dot11d.c @@ -11,7 +11,7 @@ void Dot11d_Init(struct ieee80211_device *ieee) pDot11dInfo->State = DOT11D_STATE_NONE; pDot11dInfo->CountryIeLen = 0; - memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER+1); + memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER + 1); memset(pDot11dInfo->MaxTxPwrDbmList, 0xFF, MAX_CHANNEL_NUMBER+1); RESET_CIE_WATCHDOG(ieee); -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: rtl8192u: Fix no spaces around '+'
On Wed, Dec 06, 2017 at 12:35:29PM +0530, Akash Kumar wrote: > Added spaces around '+'. Warning found using checkpatch.pl > --- > drivers/staging/rtl8192u/ieee80211/dot11d.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8192u/ieee80211/dot11d.c > b/drivers/staging/rtl8192u/ieee80211/dot11d.c > index 64b13a5..ba284bf 100644 > --- a/drivers/staging/rtl8192u/ieee80211/dot11d.c > +++ b/drivers/staging/rtl8192u/ieee80211/dot11d.c > @@ -11,7 +11,7 @@ void Dot11d_Init(struct ieee80211_device *ieee) > > pDot11dInfo->State = DOT11D_STATE_NONE; > pDot11dInfo->CountryIeLen = 0; > - memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER+1); > + memset(pDot11dInfo->channel_map, 0, MAX_CHANNEL_NUMBER + 1); > memset(pDot11dInfo->MaxTxPwrDbmList, 0xFF, MAX_CHANNEL_NUMBER+1); > RESET_CIE_WATCHDOG(ieee); > Hi, This is the friendly patch-bot of Greg Kroah-Hartman. You have sent him a patch that has triggered this response. He used to manually respond to these common problems, but in order to save his sanity (he kept writing the same thing over and over, yet to different people), I was created. Hopefully you will not take offence and will fix the problem in your patch and resubmit it so that it can be accepted into the Linux kernel tree. You are receiving this message because of the following common error(s) as indicated below: - Your patch does not have a Signed-off-by: line. Please read the kernel file, Documentation/SubmittingPatches and resend it after adding that line. Note, the line needs to be in the body of the email, before the patch, not at the bottom of the patch or in the email signature. If you wish to discuss this problem further, or you have questions about how to resolve this issue, please feel free to respond to this email and Greg will reply once he has dug out from the pending patches received from other developers. thanks, greg k-h's patch email bot ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
