Pat Gefre <[email protected]> wrote:
>
> This patch adds rs422 support to the Altix ioc4 serial driver.
>
> +
> +#define PORT_IS_ACTIVE(_x, _y) ((_x->ip_flags & PORT_ACTIVE) \
> + && (_x->ip_port == _y))
> +
- Forgets to parenthesise macro args
- Evaluates args multiple times
- ugleeeee
This:
/*
* Nice comment goes here
*/
static inline int port_is_active(struct ioc4_port *current_port,
struct ioc4_port *my_port)
{
...
}
Is more pleasing, no?
> + if (port && PORT_IS_ACTIVE(port, the_port)) {
And in every case the test for port==NULL can be folded into port_is_active().
> +int ioc4_serial_remove_one(struct ioc4_driver_data *idd)
Should have static scope.
> +{
> + int ii, jj;
> + struct ioc4_control *control;
> + struct uart_port *the_port;
> + struct ioc4_port *port;
> + struct ioc4_soft *soft;
> +
> + control = idd->idd_serial_data;
> +
> + for (ii = 0; ii < IOC4_NUM_SERIAL_PORTS; ii++) {
> + for (jj = UART_PORT_MIN; jj < UART_PORT_COUNT; jj++) {
> + the_port = &control->ic_port[ii].icp_uart_port[jj];
> + if (the_port) {
> + switch (jj) {
> + case UART_PORT_RS422:
> + uart_remove_one_port(&ioc4_uart_rs422,
> + the_port);
> + break;
> + default:
> + case UART_PORT_RS232:
> + uart_remove_one_port(&ioc4_uart_rs232,
> + the_port);
> + break;
> + }
> + }
> + }
> + port = control->ic_port[ii].icp_port;
> + if (!(ii & 1) && port) {
> + pci_free_consistent(port->ip_pdev,
> + TOTAL_RING_BUF_SIZE,
> + (void *)port->ip_cpu_ringbuf,
> + port->ip_dma_ringbuf);
> + kfree(port);
> + }
> + }
Choosing more meaningful identifiers than `ii' and `jj' would help
understandability here.
> + free_irq(control->ic_irq, (void *)soft);
The typecast is unneeded.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
