Coding-style-only modifications to remove every warning saying:
WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
Compiled against revision "next-20160327".
(checkpatch.pl was updated to treat "UNSPECIFIED_INT" warnings
as of commit a1ce18e4f941d20 )
Signed-off-by: Dominique van den Broeck <[email protected]>
---
drivers/staging/fwserial/dma_fifo.c | 8 +++----
drivers/staging/fwserial/dma_fifo.h | 16 +++++++-------
drivers/staging/fwserial/fwserial.c | 38 +++++++++++++++++----------------
drivers/staging/fwserial/fwserial.h | 42 ++++++++++++++++++-------------------
4 files changed, 53 insertions(+), 51 deletions(-)
diff --git a/drivers/staging/fwserial/dma_fifo.c b/drivers/staging/fwserial/dma_fifo.c
index 4cd3ed3..8b23a55 100644
--- a/drivers/staging/fwserial/dma_fifo.c
+++ b/drivers/staging/fwserial/dma_fifo.c
@@ -35,7 +35,7 @@
/*
* private helper fn to determine if check is in open interval (lo,hi)
*/
-static bool addr_check(unsigned check, unsigned lo, unsigned hi)
+static bool addr_check(unsigned int check, unsigned int lo, unsigned int hi)
{
return check - (lo + 1) < (hi - 1) - lo;
}
@@ -64,7 +64,7 @@ void dma_fifo_init(struct dma_fifo *fifo)
* The 'apparent' size will be rounded up to next greater aligned size.
* Returns 0 if no error, otherwise an error code
*/
-int dma_fifo_alloc(struct dma_fifo *fifo, int size, unsigned align,
+int dma_fifo_alloc(struct dma_fifo *fifo, int size, unsigned int align,
int tx_limit, int open_limit, gfp_t gfp_mask)
{
int capacity;
@@ -190,7 +190,7 @@ int dma_fifo_in(struct dma_fifo *fifo, const void *src, int n)
*/
int dma_fifo_out_pend(struct dma_fifo *fifo, struct dma_pending *pended)
{
- unsigned len, n, ofs, l, limit;
+ unsigned int len, n, ofs, l, limit;
if (!fifo->data)
return -ENOENT;
@@ -210,7 +210,7 @@ int dma_fifo_out_pend(struct dma_fifo *fifo, struct dma_pending *pended)
n = len;
ofs = fifo->out % fifo->capacity;
l = fifo->capacity - ofs;
- limit = min_t(unsigned, l, fifo->tx_limit);
+ limit = min_t(unsigned int, l, fifo->tx_limit);
if (n > limit) {
n = limit;
fifo->out += limit;
diff --git a/drivers/staging/fwserial/dma_fifo.h b/drivers/staging/fwserial/dma_fifo.h
index 4109882..37a91c6 100644
--- a/drivers/staging/fwserial/dma_fifo.h
+++ b/drivers/staging/fwserial/dma_fifo.h
@@ -45,9 +45,9 @@
#define DMA_FIFO_GUARD 3 /* # of cache lines to reserve for the guard area */
struct dma_fifo {
- unsigned in;
- unsigned out; /* updated when dma is pended */
- unsigned done; /* updated upon dma completion */
+ unsigned int in;
+ unsigned int out; /* updated when dma is pended */
+ unsigned int done; /* updated upon dma completion */
struct {
unsigned corrupt:1;
};
@@ -55,7 +55,7 @@ struct dma_fifo {
int guard; /* ofs of guard area */
int capacity; /* size + reserved */
int avail; /* # of unused bytes in fifo */
- unsigned align; /* must be power of 2 */
+ unsigned int align; /* must be power of 2 */
int tx_limit; /* max # of bytes per dma transaction */
int open_limit; /* max # of outstanding allowed */
int open; /* # of outstanding dma transactions */
@@ -66,9 +66,9 @@ struct dma_fifo {
struct dma_pending {
struct list_head link;
void *data;
- unsigned len;
- unsigned next;
- unsigned out;
+ unsigned int len;
+ unsigned int next;
+ unsigned int out;
};
static inline void dp_mark_completed(struct dma_pending *dp)
@@ -82,7 +82,7 @@ static inline bool dp_is_completed(struct dma_pending *dp)
}
void dma_fifo_init(struct dma_fifo *fifo);
-int dma_fifo_alloc(struct dma_fifo *fifo, int size, unsigned align,
+int dma_fifo_alloc(struct dma_fifo *fifo, int size, unsigned int align,
int tx_limit, int open_limit, gfp_t gfp_mask);
void dma_fifo_free(struct dma_fifo *fifo);
void dma_fifo_reset(struct dma_fifo *fifo);
diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index 9b23b5c..66e54e7 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -132,7 +132,7 @@ static struct fwtty_peer *__fwserial_peer_by_node_id(struct fw_card *card,
#ifdef FWTTY_PROFILING
-static void fwtty_profile_fifo(struct fwtty_port *port, unsigned *stat)
+static void fwtty_profile_fifo(struct fwtty_port *port, unsigned int *stat)
{
spin_lock_bh(&port->lock);
fwtty_profile_data(stat, dma_fifo_avail(&port->tx_fifo));
@@ -143,7 +143,7 @@ static void fwtty_dump_profile(struct seq_file *m, struct stats *stats)
{
/* for each stat, print sum of 0 to 2^k, then individually */
int k = 4;
- unsigned sum;
+ unsigned int sum;
int j;
char t[10];
@@ -303,9 +303,10 @@ static void fwtty_restart_tx(struct fwtty_port *port)
* Note: in loopback, the port->lock is being held. Only use functions that
* don't attempt to reclaim the port->lock.
*/
-static void fwtty_update_port_status(struct fwtty_port *port, unsigned status)
+static void fwtty_update_port_status(struct fwtty_port *port,
+ unsigned int status)
{
- unsigned delta;
+ unsigned int delta;
struct tty_struct *tty;
/* simulated LSR/MSR status from remote */
@@ -396,9 +397,9 @@ static void fwtty_update_port_status(struct fwtty_port *port, unsigned status)
*
* Note: caller must be holding port lock
*/
-static unsigned __fwtty_port_line_status(struct fwtty_port *port)
+static unsigned int __fwtty_port_line_status(struct fwtty_port *port)
{
- unsigned status = 0;
+ unsigned int status = 0;
/* TODO: add module param to tie RNG to DTR as well */
@@ -424,7 +425,7 @@ static int __fwtty_write_port_status(struct fwtty_port *port)
{
struct fwtty_peer *peer;
int err = -ENOENT;
- unsigned status = __fwtty_port_line_status(port);
+ unsigned int status = __fwtty_port_line_status(port);
rcu_read_lock();
peer = rcu_dereference(port->peer);
@@ -454,7 +455,7 @@ static int fwtty_write_port_status(struct fwtty_port *port)
static void fwtty_throttle_port(struct fwtty_port *port)
{
struct tty_struct *tty;
- unsigned old;
+ unsigned int old;
tty = tty_port_tty_get(&port->port);
if (!tty)
@@ -540,7 +541,7 @@ static void fwtty_emit_breaks(struct work_struct *work)
static int fwtty_rx(struct fwtty_port *port, unsigned char *data, size_t len)
{
int c, n = len;
- unsigned lsr;
+ unsigned int lsr;
int err = 0;
fwtty_dbg(port, "%d\n", n);
@@ -635,7 +636,7 @@ static void fwtty_port_handler(struct fw_card *card,
if (addr != port->rx_handler.offset || len != 4) {
rcode = RCODE_ADDRESS_ERROR;
} else {
- fwtty_update_port_status(port, *(unsigned *)data);
+ fwtty_update_port_status(port, *(unsigned int *)data);
rcode = RCODE_COMPLETE;
}
break;
@@ -828,7 +829,7 @@ static void fwtty_write_xchar(struct fwtty_port *port, char ch)
rcu_read_unlock();
}
-static struct fwtty_port *fwtty_port_get(unsigned index)
+static struct fwtty_port *fwtty_port_get(unsigned int index)
{
struct fwtty_port *port;
@@ -934,9 +935,9 @@ static int fwtty_port_carrier_raised(struct tty_port *tty_port)
return rc;
}
-static unsigned set_termios(struct fwtty_port *port, struct tty_struct *tty)
+static unsigned int set_termios(struct fwtty_port *port, struct tty_struct *tty)
{
- unsigned baud, frame;
+ unsigned int baud, frame;
baud = tty_termios_baud_rate(&tty->termios);
tty_termios_encode_baud_rate(&tty->termios, baud, baud);
@@ -988,7 +989,7 @@ static int fwtty_port_activate(struct tty_port *tty_port,
struct tty_struct *tty)
{
struct fwtty_port *port = to_port(tty_port, port);
- unsigned baud;
+ unsigned int baud;
int err;
set_bit(TTY_IO_ERROR, &tty->flags);
@@ -1264,7 +1265,7 @@ static int set_serial_info(struct fwtty_port *port,
return 0;
}
-static int fwtty_ioctl(struct tty_struct *tty, unsigned cmd,
+static int fwtty_ioctl(struct tty_struct *tty, unsigned int cmd,
unsigned long arg)
{
struct fwtty_port *port = tty->driver_data;
@@ -1297,7 +1298,7 @@ static int fwtty_ioctl(struct tty_struct *tty, unsigned cmd,
static void fwtty_set_termios(struct tty_struct *tty, struct ktermios *old)
{
struct fwtty_port *port = tty->driver_data;
- unsigned baud;
+ unsigned int baud;
spin_lock_bh(&port->lock);
baud = set_termios(port, tty);
@@ -1369,7 +1370,7 @@ static int fwtty_break_ctl(struct tty_struct *tty, int state)
static int fwtty_tiocmget(struct tty_struct *tty)
{
struct fwtty_port *port = tty->driver_data;
- unsigned tiocm;
+ unsigned int tiocm;
spin_lock_bh(&port->lock);
tiocm = (port->mctrl & MCTRL_MASK) | (port->mstatus & ~MCTRL_MASK);
@@ -1380,7 +1381,8 @@ static int fwtty_tiocmget(struct tty_struct *tty)
return tiocm;
}
-static int fwtty_tiocmset(struct tty_struct *tty, unsigned set, unsigned clear)
+static int fwtty_tiocmset(struct tty_struct *tty,
+ unsigned int set, unsigned int clear)
{
struct fwtty_port *port = tty->driver_data;
diff --git a/drivers/staging/fwserial/fwserial.h b/drivers/staging/fwserial/fwserial.h
index 6fa9365..30b2481 100644
--- a/drivers/staging/fwserial/fwserial.h
+++ b/drivers/staging/fwserial/fwserial.h
@@ -22,7 +22,7 @@
#ifdef FWTTY_PROFILING
#define DISTRIBUTION_MAX_SIZE 8192
#define DISTRIBUTION_MAX_INDEX (ilog2(DISTRIBUTION_MAX_SIZE) + 1)
-static inline void fwtty_profile_data(unsigned stat[], unsigned val)
+static inline void fwtty_profile_data(unsigned int stat[], unsigned int val)
{
int n = (val) ? min(ilog2(val) + 1, DISTRIBUTION_MAX_INDEX) : 0;
++stat[n];
@@ -78,7 +78,7 @@ struct fwtty_peer {
u64 guid;
int generation;
int node_id;
- unsigned speed;
+ unsigned int speed;
int max_payload;
u64 mgmt_addr;
@@ -160,17 +160,17 @@ struct fwserial_mgmt_pkt {
#define VIRT_CABLE_PLUG_TIMEOUT (60 * HZ)
struct stats {
- unsigned xchars;
- unsigned dropped;
- unsigned tx_stall;
- unsigned fifo_errs;
- unsigned sent;
- unsigned lost;
- unsigned throttled;
- unsigned reads[DISTRIBUTION_MAX_INDEX + 1];
- unsigned writes[DISTRIBUTION_MAX_INDEX + 1];
- unsigned txns[DISTRIBUTION_MAX_INDEX + 1];
- unsigned unthrottle[DISTRIBUTION_MAX_INDEX + 1];
+ unsigned int xchars;
+ unsigned int dropped;
+ unsigned int tx_stall;
+ unsigned int fifo_errs;
+ unsigned int sent;
+ unsigned int lost;
+ unsigned int throttled;
+ unsigned int reads[DISTRIBUTION_MAX_INDEX + 1];
+ unsigned int writes[DISTRIBUTION_MAX_INDEX + 1];
+ unsigned int txns[DISTRIBUTION_MAX_INDEX + 1];
+ unsigned int unthrottle[DISTRIBUTION_MAX_INDEX + 1];
};
struct fwconsole_ops {
@@ -237,7 +237,7 @@ struct fwconsole_ops {
struct fwtty_port {
struct tty_port port;
struct device *device;
- unsigned index;
+ unsigned int index;
struct fw_serial *serial;
struct fw_address_handler rx_handler;
@@ -246,21 +246,21 @@ struct fwtty_port {
wait_queue_head_t wait_tx;
struct delayed_work emit_breaks;
- unsigned cps;
+ unsigned int cps;
unsigned long break_last;
struct work_struct hangup;
- unsigned mstatus;
+ unsigned int mstatus;
spinlock_t lock;
- unsigned mctrl;
+ unsigned int mctrl;
struct delayed_work drain;
struct dma_fifo tx_fifo;
int max_payload;
- unsigned status_mask;
- unsigned ignore_mask;
- unsigned break_ctl:1,
+ unsigned int status_mask;
+ unsigned int ignore_mask;
+ unsigned int break_ctl:1,
write_only:1,
overrun:1,
loopback:1;
@@ -349,7 +349,7 @@ extern struct tty_driver *fwtty_driver;
* being used for isochronous traffic)
* 2) isochronous arbitration always wins.
*/
-static inline int link_speed_to_max_payload(unsigned speed)
+static inline int link_speed_to_max_payload(unsigned int speed)
{
/* Max async payload is 4096 - see IEEE 1394-2008 tables 6-4, 16-18 */
return min(512 << speed, 4096);
--
2.4.3
Removing two "!= NULL" from fwserial.c as suggested by checkpatch.pl.
Note that the associated expression "port->port.console" is a 1-bit-field
that is already assumed as an implicit boolean (that is: without comparison)
Signed-off-by: Dominique van den Broeck <[email protected]>
---
drivers/staging/fwserial/fwserial.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index 66e54e7..4dd5304 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -1701,7 +1701,7 @@ static void fwserial_virt_plug_complete(struct fwtty_peer *peer,
dma_fifo_change_tx_limit(&port->tx_fifo, port->max_payload);
spin_unlock_bh(&peer->port->lock);
- if (port->port.console && port->fwcon_ops->notify != NULL)
+ if (port->port.console && port->fwcon_ops->notify)
(*port->fwcon_ops->notify)(FWCON_NOTIFY_ATTACH, port->con_data);
fwtty_info(&peer->unit, "peer (guid:%016llx) connected on %s\n",
@@ -1808,7 +1808,7 @@ static void fwserial_release_port(struct fwtty_port *port, bool reset)
RCU_INIT_POINTER(port->peer, NULL);
spin_unlock_bh(&port->lock);
- if (port->port.console && port->fwcon_ops->notify != NULL)
+ if (port->port.console && port->fwcon_ops->notify)
(*port->fwcon_ops->notify)(FWCON_NOTIFY_DETACH, port->con_data);
}
--
2.4.3
Fixing a lone row exceeding 80 columns so the only remaining warnings
emitted by checkpatch.pl are missing comments on spinlocks and memory
barriers.
Signed-off-by: Dominique van den Broeck <[email protected]>
---
drivers/staging/fwserial/fwserial.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
index 4dd5304..c5f73ef 100644
--- a/drivers/staging/fwserial/fwserial.c
+++ b/drivers/staging/fwserial/fwserial.c
@@ -1343,9 +1343,11 @@ static int fwtty_break_ctl(struct tty_struct *tty, int state)
if (state == -1) {
set_bit(STOP_TX, &port->flags);
- ret = wait_event_interruptible_timeout(port->wait_tx,
- !test_bit(IN_TX, &port->flags),
- 10);
+ ret =
+ wait_event_interruptible_timeout(port->wait_tx,
+ !test_bit(IN_TX, &port->flags),
+ 10);
+
if (ret == 0 || ret == -ERESTARTSYS) {
clear_bit(STOP_TX, &port->flags);
fwtty_restart_tx(port);
--
2.4.3
On Tue, 2016-03-29 at 19:14 +0200, Dominique van den Broeck wrote:
> Fixing a lone row exceeding 80 columns so the only remaining warnings
> emitted by checkpatch.pl are missing comments on spinlocks and memory
> barriers.
[]
> diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
[]
> @@ -1343,9 +1343,11 @@ static int fwtty_break_ctl(struct tty_struct *tty, int state)
> ?
> ? if (state == -1) {
> ? set_bit(STOP_TX, &port->flags);
> - ret = wait_event_interruptible_timeout(port->wait_tx,
> - ???????!test_bit(IN_TX, &port->flags),
> - ???????10);
> + ret =
> + wait_event_interruptible_timeout(port->wait_tx,
> + ?!test_bit(IN_TX, &port->flags),
> + ?10);
Does this really look better to you?
Long identifiers like "wait_event_interruptible_timeout"
(32 chars) make
using 80 columns a bit silly.
Please remember checkpatch is a stupid script and that
not every warning it emits is dicta.
> Does this really look better to you?
>
> Long identifiers like "wait_event_interruptible_timeout"
> (32 chars) make
> using 80 columns a bit silly.
>
> Please remember checkpatch is a stupid script and that
> not every warning it emits is dicta.
Actually, not much and as a matter of fact, I hesitated before sending
this particular patch. That's also why I submitted it as the last one
of them all and why it fixes only a single row : so we can eventually
reject it easily.
I'm not a checkpatch.pl nazi neither and anyway, even the "Development
Process" document specify that it's not to be considered as a strict
rule if it makes the things worse.
But I found it useful anyway because this set brings back all the
warnings to only two categories (comments over spinlocks and memory
barriers) and also because this kind of minor corrections could still
be appreciated by people that have better things to do... :-)
Cheers.
On 03/29/2016 10:14 AM, Dominique van den Broeck wrote:
> Coding-style-only modifications to remove every warning saying:
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
>
> Compiled against revision "next-20160327".
>
> (checkpatch.pl was updated to treat "UNSPECIFIED_INT" warnings
> as of commit a1ce18e4f941d20 )
Please change $subject to "staging: fwserial: Convert unsigned to unsigned int"
With that,
Reviewed-by: Peter Hurley <[email protected]>
PS - For v2 of this patch, be sure to change the subject prefix to [PATCH v2 ...],
and note any changes in the diffstat area of the patch itself; eg.,
Signed-off-by: Dominique van den Broeck <[email protected]>
---
v2: change subject per Peter Hurley's comments
drivers/staging/fwserial/dma_fifo.c | 8 +++----
drivers/staging/fwserial/dma_fifo.h | 16 +++++++-------
drivers/staging/fwserial/fwserial.c | 38 +++++++++++++++++----------------
drivers/staging/fwserial/fwserial.h | 42 ++++++++++++++++++-------------------
4 files changed, 53 insertions(+), 51 deletions(-)
On 03/29/2016 10:14 AM, Dominique van den Broeck wrote:
> Removing two "!= NULL" from fwserial.c as suggested by checkpatch.pl.
> Note that the associated expression "port->port.console" is a 1-bit-field
> that is already assumed as an implicit boolean (that is: without comparison)
Similarly, please change $subject to "staging: fwserial: Simplify NULL ptr check"
With that,
Reviewed-by: Peter Hurley <[email protected]>
Hi Dominique,
On 03/29/2016 10:14 AM, Dominique van den Broeck wrote:
> Fixing a lone row exceeding 80 columns so the only remaining warnings
> emitted by checkpatch.pl are missing comments on spinlocks and memory
> barriers.
>
> Signed-off-by: Dominique van den Broeck <[email protected]>
> ---
> drivers/staging/fwserial/fwserial.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/fwserial/fwserial.c b/drivers/staging/fwserial/fwserial.c
> index 4dd5304..c5f73ef 100644
> --- a/drivers/staging/fwserial/fwserial.c
> +++ b/drivers/staging/fwserial/fwserial.c
> @@ -1343,9 +1343,11 @@ static int fwtty_break_ctl(struct tty_struct *tty, int state)
>
> if (state == -1) {
> set_bit(STOP_TX, &port->flags);
> - ret = wait_event_interruptible_timeout(port->wait_tx,
> - !test_bit(IN_TX, &port->flags),
> - 10);
> + ret =
> + wait_event_interruptible_timeout(port->wait_tx,
> + !test_bit(IN_TX, &port->flags),
> + 10);
> +
I don't see a > 80-col line here?
And even if I did, this change would be super-ugly.
The preferred way to reduce this is to fold it into a helper function, like
if (state == -1 && fwtty_wait_tx_complete(port))
return -EINTR;
Regards,
Peter Hurley
> if (ret == 0 || ret == -ERESTARTSYS) {
> clear_bit(STOP_TX, &port->flags);
> fwtty_restart_tx(port);
>
Hello Peter,
Thanks a lot for your review and kind advice !
> I don't see a > 80-col line here?
In fact, it was not even a 80-col issue but a mis-aligned parenthesis
one. Realign the rows in this state would make them exceed the 80th
column.
I tend to agree with the fact that the way it currently is remains the
best one.
> And even if I did, this change would be super-ugly.
> The preferred way to reduce this is to fold it into a helper
> function
Actually, before I resend my patches, I have two or three small
questions:
1) My v1 patches already made it to staging and linux-next trees.
Should I resend them anyway ?
2) Would it be helpful to people if I write a function the way you
specified it or would it be better to let it as is ?
3) If we don't, and then discard the last patch, shall I number « n/2 »
or « n/3 » anyway ?
Forgive me if these questions are lame, I still have only a few
experience of the kernel tree. Documentation/SubmittingPatches states
that no one should be expected to refer to a previous set of patches,
so I suppose this would be « 1/2 » and « 2/2 » but I prefer being OK
about this from the beginning.
Thanks for caring.
On 04/01/2016 04:20 PM, Dominique van den Broeck wrote:
> Hello Peter,
> Thanks a lot for your review and kind advice !
>
>> I don't see a > 80-col line here?
>
> In fact, it was not even a 80-col issue but a mis-aligned parenthesis
> one. Realign the rows in this state would make them exceed the 80th
> column.
Ah, ok. Wasn't clear from the commit message.
> I tend to agree with the fact that the way it currently is remains the
> best one.
Ok.
>> And even if I did, this change would be super-ugly.
>> The preferred way to reduce this is to fold it into a helper
>> function
>
> Actually, before I resend my patches, I have two or three small
> questions:
>
> 1) My v1 patches already made it to staging and linux-next trees.
> Should I resend them anyway ?
No, I didn't know they were already in staging-next.
Nevermind then :)
> 2) Would it be helpful to people if I write a function the way you
> specified it or would it be better to let it as is ?
As is, please.
> 3) If we don't, and then discard the last patch, shall I number « n/2 »
> or « n/3 » anyway ?
n/a now.
> Forgive me if these questions are lame, I still have only a few
> experience of the kernel tree.
Your questions are not lame; no need to apologize.
> Documentation/SubmittingPatches states
> that no one should be expected to refer to a previous set of patches,
> so I suppose this would be « 1/2 » and « 2/2 » but I prefer being OK
> about this from the beginning.
If you would have sent the patches, yes, they would have been 1/2 and 2/2.
What I do there is send the v2 series in-reply-to the original 1/2 patch.
> Thanks for caring.
Regards,
Peter Hurley