* [PATCH] Staging: pi433: fix some warnings detected using sparse
@ 2017-07-28 12:56 Elia Geretto
2017-07-28 14:17 ` Dan Carpenter
2017-07-29 13:36 ` kbuild test robot
0 siblings, 2 replies; 4+ messages in thread
From: Elia Geretto @ 2017-07-28 12:56 UTC (permalink / raw)
To: Greg Kroah-Hartman, devel, linux-kernel; +Cc: Elia Geretto
This patch corrects some visibility issues regarding some functions and
solves a warning related to a non-matching union. After this patch,
sparse produces only one other warning regarding a bitwise operator;
however, this behaviour seems to be intended.
Signed-off-by: Elia Geretto <elia.f.geretto@gmail.com>
---
drivers/staging/pi433/pi433_if.c | 17 +++++++++++------
drivers/staging/pi433/rf69.c | 4 +++-
2 files changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
index d9328ce5ec1d..f8219a53ce60 100644
--- a/drivers/staging/pi433/pi433_if.c
+++ b/drivers/staging/pi433/pi433_if.c
@@ -208,7 +208,10 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
{
SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
}
- SET_CHECKED(rf69_set_packet_format (dev->spi, rx_cfg->enable_length_byte));
+ if (rx_cfg->enable_length_byte == optionOn)
+ SET_CHECKED(rf69_set_packet_format(dev->spi, packetLengthVar));
+ else
+ SET_CHECKED(rf69_set_packet_format(dev->spi, packetLengthFix));
SET_CHECKED(rf69_set_adressFiltering(dev->spi, rx_cfg->enable_address_filtering));
SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
@@ -264,8 +267,11 @@ rf69_set_tx_cfg(struct pi433_device *dev, struct pi433_tx_cfg *tx_cfg)
{
SET_CHECKED(rf69_set_preamble_length(dev->spi, 0));
}
+ if (tx_cfg->enable_length_byte == optionOn)
+ SET_CHECKED(rf69_set_packet_format(dev->spi, packetLengthVar));
+ else
+ SET_CHECKED(rf69_set_packet_format(dev->spi, packetLengthFix));
SET_CHECKED(rf69_set_sync_enable (dev->spi, tx_cfg->enable_sync));
- SET_CHECKED(rf69_set_packet_format(dev->spi, tx_cfg->enable_length_byte));
SET_CHECKED(rf69_set_crc_enable (dev->spi, tx_cfg->enable_crc));
/* configure sync, if enabled */
@@ -313,7 +319,7 @@ pi433_start_rx(struct pi433_device *dev)
/*-------------------------------------------------------------------------*/
-int
+static int
pi433_receive(void *data)
{
struct pi433_device *dev = data;
@@ -463,7 +469,7 @@ pi433_receive(void *data)
return bytes_total;
}
-int
+static int
pi433_tx_thread(void *data)
{
struct pi433_device *device = data;
@@ -1152,8 +1158,7 @@ static int pi433_probe(struct spi_device *spi)
device->tx_task_struct = kthread_run(pi433_tx_thread,
device,
"pi433_tx_task");
- if (device->tx_task_struct < 0)
- {
+ if (IS_ERR(device->tx_task_struct)) {
dev_dbg(device->dev, "start of send thread failed");
goto send_thread_failed;
}
diff --git a/drivers/staging/pi433/rf69.c b/drivers/staging/pi433/rf69.c
index e391ce777bc7..52521fa18c9b 100644
--- a/drivers/staging/pi433/rf69.c
+++ b/drivers/staging/pi433/rf69.c
@@ -433,7 +433,9 @@ int rf69_set_dc_cut_off_frequency_during_afc(struct spi_device *spi, enum dccPer
return rf69_set_dc_cut_off_frequency_intern(spi, REG_AFCBW, dccPercent);
}
-int rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg, enum mantisse mantisse, u8 exponent)
+static int
+rf69_set_bandwidth_intern(struct spi_device *spi, u8 reg,
+ enum mantisse mantisse, u8 exponent)
{
u8 newValue;
--
2.13.3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Staging: pi433: fix some warnings detected using sparse
2017-07-28 12:56 [PATCH] Staging: pi433: fix some warnings detected using sparse Elia Geretto
@ 2017-07-28 14:17 ` Dan Carpenter
2017-07-29 8:30 ` Elia Geretto
2017-07-29 13:36 ` kbuild test robot
1 sibling, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2017-07-28 14:17 UTC (permalink / raw)
To: Elia Geretto; +Cc: Greg Kroah-Hartman, devel, linux-kernel
On Fri, Jul 28, 2017 at 02:56:26PM +0200, Elia Geretto wrote:
> This patch corrects some visibility issues regarding some functions and
> solves a warning related to a non-matching union. After this patch,
> sparse produces only one other warning regarding a bitwise operator;
> however, this behaviour seems to be intended.
I can't understand this changelog at all.... :/ What are we fixing
exactly? It seems like we're fixing something about bitwise
operators... I guess let me check the Sparse warnings... Here they are
from the latest linux-next:
drivers/staging/pi433/pi433_if.c:211:9: warning: mixing different enum types
drivers/staging/pi433/pi433_if.c:211:9: int enum optionOnOff versus
drivers/staging/pi433/pi433_if.c:211:9: int enum packetFormat
drivers/staging/pi433/pi433_if.c:211:9: warning: mixing different enum types
drivers/staging/pi433/pi433_if.c:211:9: int enum optionOnOff versus
drivers/staging/pi433/pi433_if.c:211:9: int enum packetFormat
drivers/staging/pi433/pi433_if.c:268:9: warning: mixing different enum types
drivers/staging/pi433/pi433_if.c:268:9: int enum optionOnOff versus
drivers/staging/pi433/pi433_if.c:268:9: int enum packetFormat
drivers/staging/pi433/pi433_if.c:268:9: warning: mixing different enum types
drivers/staging/pi433/pi433_if.c:268:9: int enum optionOnOff versus
drivers/staging/pi433/pi433_if.c:268:9: int enum packetFormat
drivers/staging/pi433/pi433_if.c:317:1: warning: symbol 'pi433_receive' was not declared. Should it be static?
drivers/staging/pi433/pi433_if.c:467:1: warning: symbol 'pi433_tx_thread' was not declared. Should it be static?
drivers/staging/pi433/pi433_if.c:1155:36: error: incompatible types for operation (<)
drivers/staging/pi433/pi433_if.c:1155:36: left side has type struct task_struct *tx_task_struct
drivers/staging/pi433/pi433_if.c:1155:36: right side has type int
drivers/staging/pi433/rf69.c:206:17: warning: dubious: x & !y
drivers/staging/pi433/rf69.c:436:5: warning: symbol 'rf69_set_bandwidth_intern' was not declared. Should it be static?
Each type of fix should be sent as a separate fix with a better
changelog. People have already done the "static" fixes and IS_ERR()
fixes, so don't worry about those. But I don't think anyway has fixed
the enum issues so resend that. Also the bitwise thing is a real bug,
but there is already a fix for that, it just hasn't been merged yet.
>
> Signed-off-by: Elia Geretto <elia.f.geretto@gmail.com>
> ---
> drivers/staging/pi433/pi433_if.c | 17 +++++++++++------
> drivers/staging/pi433/rf69.c | 4 +++-
> 2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/pi433/pi433_if.c b/drivers/staging/pi433/pi433_if.c
> index d9328ce5ec1d..f8219a53ce60 100644
> --- a/drivers/staging/pi433/pi433_if.c
> +++ b/drivers/staging/pi433/pi433_if.c
> @@ -208,7 +208,10 @@ rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
> {
> SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
> }
> - SET_CHECKED(rf69_set_packet_format (dev->spi, rx_cfg->enable_length_byte));
> + if (rx_cfg->enable_length_byte == optionOn)
> + SET_CHECKED(rf69_set_packet_format(dev->spi, packetLengthVar));
> + else
> + SET_CHECKED(rf69_set_packet_format(dev->spi, packetLengthFix));
The SET_CHECKED() macro is total garbage. It has a hidden return and
it calls the rf69_set_packet_format() twice on error it expands to:
if (rf69_set_packet_format(dev->spi, rx_cfg->enable_length_byte)) < 0)
return rf69_set_packet_format(dev->spi, rx_cfg->enable_length_byte);
Mega turbo barf! Kill it with fire!
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Staging: pi433: fix some warnings detected using sparse
2017-07-28 14:17 ` Dan Carpenter
@ 2017-07-29 8:30 ` Elia Geretto
0 siblings, 0 replies; 4+ messages in thread
From: Elia Geretto @ 2017-07-29 8:30 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Greg Kroah-Hartman, devel, linux-kernel
On Fri, 2017-07-28 at 17:17 +0300, Dan Carpenter wrote:
> On Fri, Jul 28, 2017 at 02:56:26PM +0200, Elia Geretto wrote:
> > This patch corrects some visibility issues regarding some functions
> > and
> > solves a warning related to a non-matching union. After this patch,
> > sparse produces only one other warning regarding a bitwise
> > operator;
> > however, this behaviour seems to be intended.
>
> I can't understand this changelog at all.... :/ What are we fixing
> exactly? It seems like we're fixing something about bitwise
> operators... I guess let me check the Sparse warnings... Here they
> are
> from the latest linux-next:
>
> drivers/staging/pi433/pi433_if.c:211:9: warning: mixing different
> enum types
> drivers/staging/pi433/pi433_if.c:211:9: int enum
> optionOnOff versus
> drivers/staging/pi433/pi433_if.c:211:9: int enum packetFormat
> drivers/staging/pi433/pi433_if.c:211:9: warning: mixing different
> enum types
> drivers/staging/pi433/pi433_if.c:211:9: int enum
> optionOnOff versus
> drivers/staging/pi433/pi433_if.c:211:9: int enum packetFormat
> drivers/staging/pi433/pi433_if.c:268:9: warning: mixing different
> enum types
> drivers/staging/pi433/pi433_if.c:268:9: int enum
> optionOnOff versus
> drivers/staging/pi433/pi433_if.c:268:9: int enum packetFormat
> drivers/staging/pi433/pi433_if.c:268:9: warning: mixing different
> enum types
> drivers/staging/pi433/pi433_if.c:268:9: int enum
> optionOnOff versus
> drivers/staging/pi433/pi433_if.c:268:9: int enum packetFormat
> drivers/staging/pi433/pi433_if.c:317:1: warning: symbol
> 'pi433_receive' was not declared. Should it be static?
> drivers/staging/pi433/pi433_if.c:467:1: warning: symbol
> 'pi433_tx_thread' was not declared. Should it be static?
> drivers/staging/pi433/pi433_if.c:1155:36: error: incompatible types
> for operation (<)
> drivers/staging/pi433/pi433_if.c:1155:36: left side has type
> struct task_struct *tx_task_struct
> drivers/staging/pi433/pi433_if.c:1155:36: right side has type int
> drivers/staging/pi433/rf69.c:206:17: warning: dubious: x & !y
> drivers/staging/pi433/rf69.c:436:5: warning: symbol
> 'rf69_set_bandwidth_intern' was not declared. Should it be static?
>
> Each type of fix should be sent as a separate fix with a better
> changelog. People have already done the "static" fixes and IS_ERR()
> fixes, so don't worry about those. But I don't think anyway has
> fixed
> the enum issues so resend that. Also the bitwise thing is a real
> bug,
> but there is already a fix for that, it just hasn't been merged yet.
>
> >
> > Signed-off-by: Elia Geretto <elia.f.geretto@gmail.com>
> > ---
> > drivers/staging/pi433/pi433_if.c | 17 +++++++++++------
> > drivers/staging/pi433/rf69.c | 4 +++-
> > 2 files changed, 14 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/pi433/pi433_if.c
> > b/drivers/staging/pi433/pi433_if.c
> > index d9328ce5ec1d..f8219a53ce60 100644
> > --- a/drivers/staging/pi433/pi433_if.c
> > +++ b/drivers/staging/pi433/pi433_if.c
> > @@ -208,7 +208,10 @@ rf69_set_rx_cfg(struct pi433_device *dev,
> > struct pi433_rx_cfg *rx_cfg)
> > {
> > SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi,
> > always));
> > }
> > - SET_CHECKED(rf69_set_packet_format (dev->spi, rx_cfg-
> > >enable_length_byte));
> > + if (rx_cfg->enable_length_byte == optionOn)
> > + SET_CHECKED(rf69_set_packet_format(dev->spi,
> > packetLengthVar));
> > + else
> > + SET_CHECKED(rf69_set_packet_format(dev->spi,
> > packetLengthFix));
>
> The SET_CHECKED() macro is total garbage. It has a hidden return and
> it calls the rf69_set_packet_format() twice on error it expands to:
>
> if (rf69_set_packet_format(dev->spi, rx_cfg-
> >enable_length_byte)) < 0)
> return rf69_set_packet_format(dev->spi, rx_cfg-
> >enable_length_byte);
>
> Mega turbo barf! Kill it with fire!
>
> regards,
> dan carpenter
>
I will resend a separate patch containing the enum work; I apologize
for the unclear changelog, I am still trying to understand how much in
detail I should go. Next time I will be more precise.
Regards,
Elia Geretto
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Staging: pi433: fix some warnings detected using sparse
2017-07-28 12:56 [PATCH] Staging: pi433: fix some warnings detected using sparse Elia Geretto
2017-07-28 14:17 ` Dan Carpenter
@ 2017-07-29 13:36 ` kbuild test robot
1 sibling, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2017-07-29 13:36 UTC (permalink / raw)
To: Elia Geretto
Cc: kbuild-all, Greg Kroah-Hartman, devel, linux-kernel, Elia Geretto
[-- Attachment #1: Type: text/plain, Size: 4169 bytes --]
Hi Elia,
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on next-20170728]
[cannot apply to v4.13-rc2]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Elia-Geretto/Staging-pi433-fix-some-warnings-detected-using-sparse/20170729-205713
config: blackfin-allyesconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin
All errors (new ones prefixed by >>):
drivers/staging//pi433/pi433_if.c: In function 'rf69_set_rx_cfg':
>> drivers/staging//pi433/pi433_if.c:213:2: error: 'else' without a previous 'if'
else
^~~~
drivers/staging//pi433/pi433_if.c: In function 'rf69_set_tx_cfg':
drivers/staging//pi433/pi433_if.c:272:2: error: 'else' without a previous 'if'
else
^~~~
vim +213 drivers/staging//pi433/pi433_if.c
181
182 static int
183 rf69_set_rx_cfg(struct pi433_device *dev, struct pi433_rx_cfg *rx_cfg)
184 {
185 int payload_length;
186
187 /* receiver config */
188 SET_CHECKED(rf69_set_frequency (dev->spi, rx_cfg->frequency));
189 SET_CHECKED(rf69_set_bit_rate (dev->spi, rx_cfg->bit_rate));
190 SET_CHECKED(rf69_set_modulation (dev->spi, rx_cfg->modulation));
191 SET_CHECKED(rf69_set_antenna_impedance (dev->spi, rx_cfg->antenna_impedance));
192 SET_CHECKED(rf69_set_rssi_threshold (dev->spi, rx_cfg->rssi_threshold));
193 SET_CHECKED(rf69_set_ook_threshold_dec (dev->spi, rx_cfg->thresholdDecrement));
194 SET_CHECKED(rf69_set_bandwidth (dev->spi, rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
195 SET_CHECKED(rf69_set_bandwidth_during_afc(dev->spi, rx_cfg->bw_mantisse, rx_cfg->bw_exponent));
196 SET_CHECKED(rf69_set_dagc (dev->spi, rx_cfg->dagc));
197
198 dev->rx_bytes_to_drop = rx_cfg->bytes_to_drop;
199
200 /* packet config */
201 /* enable */
202 SET_CHECKED(rf69_set_sync_enable(dev->spi, rx_cfg->enable_sync));
203 if (rx_cfg->enable_sync == optionOn)
204 {
205 SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, afterSyncInterrupt));
206 }
207 else
208 {
209 SET_CHECKED(rf69_set_fifo_fill_condition(dev->spi, always));
210 }
211 if (rx_cfg->enable_length_byte == optionOn)
212 SET_CHECKED(rf69_set_packet_format(dev->spi, packetLengthVar));
> 213 else
214 SET_CHECKED(rf69_set_packet_format(dev->spi, packetLengthFix));
215 SET_CHECKED(rf69_set_adressFiltering(dev->spi, rx_cfg->enable_address_filtering));
216 SET_CHECKED(rf69_set_crc_enable (dev->spi, rx_cfg->enable_crc));
217
218 /* lengths */
219 SET_CHECKED(rf69_set_sync_size(dev->spi, rx_cfg->sync_length));
220 if (rx_cfg->enable_length_byte == optionOn)
221 {
222 SET_CHECKED(rf69_set_payload_length(dev->spi, 0xff));
223 }
224 else if (rx_cfg->fixed_message_length != 0)
225 {
226 payload_length = rx_cfg->fixed_message_length;
227 if (rx_cfg->enable_length_byte == optionOn) payload_length++;
228 if (rx_cfg->enable_address_filtering != filteringOff) payload_length++;
229 SET_CHECKED(rf69_set_payload_length(dev->spi, payload_length));
230 }
231 else
232 {
233 SET_CHECKED(rf69_set_payload_length(dev->spi, 0));
234 }
235
236 /* values */
237 if (rx_cfg->enable_sync == optionOn)
238 {
239 SET_CHECKED(rf69_set_sync_values(dev->spi, rx_cfg->sync_pattern));
240 }
241 if (rx_cfg->enable_address_filtering != filteringOff)
242 {
243 SET_CHECKED(rf69_set_node_address (dev->spi, rx_cfg->node_address));
244 SET_CHECKED(rf69_set_broadcast_address(dev->spi, rx_cfg->broadcast_address));
245 }
246
247 return 0;
248 }
249
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45670 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-07-29 13:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-28 12:56 [PATCH] Staging: pi433: fix some warnings detected using sparse Elia Geretto
2017-07-28 14:17 ` Dan Carpenter
2017-07-29 8:30 ` Elia Geretto
2017-07-29 13:36 ` kbuild test robot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.