>From: Heikki Krogerus [mailto:heikki.krogerus@xxxxxxxxxxxxxxx] >Sent: Wednesday, December 09, 2015 3:20 PM >> @@ -171,7 +174,8 @@ static void dw8250_serial_out32(struct uart_port *p, int offset, int value) >> if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR)) >> return; >> dw8250_force_idle(p); >> - writel(value, p->membase + (UART_LCR << p->regshift)); >> + d->serial_out(value, >> + p->membase + (UART_LCR << p->regshift)); >> } >.. The way I see it, this is the only place where you would really need the new private serial_in/out hooks, so why not just check the >iotype here and based on that use writeb/writel/iowrite32be or what ever .. In previous versions (V2) Greg dis-liked using iotype here this is why I added the private serial_in/serial_out >> static void dw8250_setup_port(struct uart_port *p) { >> struct uart_8250_port *up = up_to_u8250p(p); >> + struct dw8250_data *d = p->private_data; >> u32 reg; >> >> /* >> * If the Component Version Register returns zero, we know that >> * ADDITIONAL_FEATURES are not enabled. No need to go any further. >> */ >> - reg = readl(p->membase + DW_UART_UCV); >> + reg = d->serial_in(p->membase + DW_UART_UCV); >> if (!reg) >> return; >> >> dev_dbg(p->dev, "Designware UART version %c.%c%c\n", >> (reg >> 24) & 0xff, (reg >> 16) & 0xff, (reg >> 8) & 0xff); >> >> - reg = readl(p->membase + DW_UART_CPR); >> + reg = d->serial_in(p->membase + DW_UART_CPR); >> if (!reg) >> return; >.. Here you could as well replace the direct readl calls with serial_port_in calls. Again, if you meant here for using iotype it was rejected. >> @@ -390,9 +434,19 @@ static int dw8250_probe(struct platform_device >> *pdev) >> >> err = device_property_read_u32(p->dev, "reg-io-width", &val); >> if (!err && val == 4) { >> - p->iotype = UPIO_MEM32; >> - p->serial_in = dw8250_serial_in32; >> - p->serial_out = dw8250_serial_out32; >> + p->iotype = of_device_is_big_endian(p->dev->of_node) ? >> + UPIO_MEM32BE : UPIO_MEM32; >If this has to be tied to DT, do this check in dw8250_quirks. Thanks , I will move this to dw8250_quirks. >> dw8250_quirks(p, data); >> >> + data->serial_in = _dw8250_serial_in32; >> + data->serial_out = _dw8250_serial_out32; >I don't think I understand what is going on here? You would never actually even use _dw8250_serial_in32be/out32be, no? >Maybe I'm misunderstanding something. This is a default in case where "reg-io-width != 4". What is the case you see that they are not used? (_dw8250_serial_in32be is used from dw8250_setup_port just few lines below) -Noam -- 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