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.gere...@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
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
