Re: [PATCH] 8250: add a UPIO_DWAPB32 for 32 bit accesses (v2)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 01, 2010 at 08:10:44AM +0000, Jamie Iles wrote:
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -454,21 +454,40 @@ static void tsi_serial_out(struct uart_port *p, int offset, int value)
>  		writeb(value, p->membase + offset);
>  }
>  
> +/* Save the LCR value so it can be re-written when a Busy Detect IRQ occurs. */
> +static inline void dwapb_save_out_value(struct uart_port *p, int offset,
> +					int value)
> +{
> +	struct uart_8250_port *up =
> +		container_of(p, struct uart_8250_port, port);

container_of, when the original code did a simple cast:

>  static void dwapb_serial_out(struct uart_port *p, int offset, int value)
>  {
>  	int save_offset = offset;
>  	offset = map_8250_out_reg(p, offset) << p->regshift;
> -	/* Save the LCR value so it can be re-written when a
> -	 * Busy Detect interrupt occurs. */
> -	if (save_offset == UART_LCR) {
> -		struct uart_8250_port *up = (struct uart_8250_port *)p;
> -		up->lcr = value;
> -	}
> +	dwapb_save_out_value(p, save_offset, value);

Because of that, are you sure this is correct now?  You might just be
getting lucky due to the location of the pointer within the structure,
but then the old code was wrong.

Either way, I don't feel comfortable with this, do you?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux