| Message ID | 20210215121713.57687-21-marcan@marcan.st (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Apple M1 SoC platform bring-up | expand |
On Mon, Feb 15, 2021 at 09:17:08PM +0900, Hector Martin wrote: > This picks up the non-posted I/O mode needed for Apple platforms to > work properly. > > This removes the request/release functions, which are no longer > necessary, since devm_ioremap_resource takes care of that already. Most > other drivers already do it this way, anyway. > > Also fix a bug checking the return value, which should use IS_ERR(). No, no, no. We never, never combine fixing bugs with some rework. However devm_ioremap() returns NULL so where is the error? Did you test your patches on existing platforms? If not, please mark all of them as RFT on next submission, so Greg does not pick them too fast. Best regards, Krzysztof
On 16/02/2021 03.51, Krzysztof Kozlowski wrote: >> Also fix a bug checking the return value, which should use IS_ERR(). > > No, no, no. We never, never combine fixing bugs with some rework. > However devm_ioremap() returns NULL so where is the error? Sorry, this was a commit message mistake. The code is correct and so is the patch: just the NULL check is correct for the previous variant and IS_ERR is correct for devm_ioremap_resource. I confused myself while writing the commit message after the fact. > Did you test your patches on existing platforms? If not, please mark all > of them as RFT on next submission, so Greg does not pick them too fast. I unfortunately don't have any Exynos devices where I could test the code (I have a couple but no serial connections, and I have no idea if mailine would run on them). I'll mark v3 as RFT.
On Thu, Feb 18, 2021 at 11:01:21PM +0900, Hector Martin wrote: > On 16/02/2021 03.51, Krzysztof Kozlowski wrote: > > > Also fix a bug checking the return value, which should use IS_ERR(). > > > > No, no, no. We never, never combine fixing bugs with some rework. > > However devm_ioremap() returns NULL so where is the error? > > Sorry, this was a commit message mistake. The code is correct and so is the > patch: just the NULL check is correct for the previous variant and IS_ERR is > correct for devm_ioremap_resource. I confused myself while writing the > commit message after the fact. > > > Did you test your patches on existing platforms? If not, please mark all > > of them as RFT on next submission, so Greg does not pick them too fast. > > I unfortunately don't have any Exynos devices where I could test the code (I > have a couple but no serial connections, and I have no idea if mailine would > run on them). I'll mark v3 as RFT. If you have one of Odroid boards with Exynos, then you can nicely test Exynos. Others - depends, on board. Anyway I can test them for you. I just want to be sure that Greg waits for this testing. Best regards, Krzysztof
On 2021-02-20 19:13, Krzysztof Kozlowski wrote: > On Thu, Feb 18, 2021 at 11:01:21PM +0900, Hector Martin wrote: >> On 16/02/2021 03.51, Krzysztof Kozlowski wrote: >> > > Also fix a bug checking the return value, which should use IS_ERR(). >> > >> > No, no, no. We never, never combine fixing bugs with some rework. >> > However devm_ioremap() returns NULL so where is the error? >> >> Sorry, this was a commit message mistake. The code is correct and so >> is the >> patch: just the NULL check is correct for the previous variant and >> IS_ERR is >> correct for devm_ioremap_resource. I confused myself while writing the >> commit message after the fact. >> >> > Did you test your patches on existing platforms? If not, please mark all >> > of them as RFT on next submission, so Greg does not pick them too fast. >> >> I unfortunately don't have any Exynos devices where I could test the >> code (I >> have a couple but no serial connections, and I have no idea if mailine >> would >> run on them). I'll mark v3 as RFT. > > If you have one of Odroid boards with Exynos, then you can nicely test > Exynos. Others - depends, on board. > Anyway I can test them for you. I just want to be sure that Greg waits > for this testing. Worse case, QEMU has some Exynos4210 emulation that is usable. Thanks, M.
On 21/02/2021 04.17, Marc Zyngier wrote: > On 2021-02-20 19:13, Krzysztof Kozlowski wrote: >> On Thu, Feb 18, 2021 at 11:01:21PM +0900, Hector Martin wrote: >>> On 16/02/2021 03.51, Krzysztof Kozlowski wrote: >>>>> Also fix a bug checking the return value, which should use IS_ERR(). >>>> >>>> No, no, no. We never, never combine fixing bugs with some rework. >>>> However devm_ioremap() returns NULL so where is the error? >>> >>> Sorry, this was a commit message mistake. The code is correct and so >>> is the >>> patch: just the NULL check is correct for the previous variant and >>> IS_ERR is >>> correct for devm_ioremap_resource. I confused myself while writing the >>> commit message after the fact. >>> >>>> Did you test your patches on existing platforms? If not, please mark all >>>> of them as RFT on next submission, so Greg does not pick them too fast. >>> >>> I unfortunately don't have any Exynos devices where I could test the >>> code (I >>> have a couple but no serial connections, and I have no idea if mailine >>> would >>> run on them). I'll mark v3 as RFT. >> >> If you have one of Odroid boards with Exynos, then you can nicely test >> Exynos. Others - depends, on board. >> Anyway I can test them for you. I just want to be sure that Greg waits >> for this testing. > > Worse case, QEMU has some Exynos4210 emulation that is usable. That's a good point; better than nothing, certainly. Does anyone have a known good example of booting an exynos kernel under qemu? I tried building a plain 5.11 arm exynos_defconfig and booting it, but without much luck: $ qemu-system-arm -kernel arch/arm/boot/zImage -append "console=ttySAC0,115200n8 debug" -dtb arch/arm/boot/dts/exynos4210-universal_c210.dtb -nographic -serial mon:stdio -M smdkc210 -smp 2 (I also tried without the -dtb option, in case qemu provides something usable) Of course I'll still mark v3 as RFT, I just thought I might as well try qemu.
On Sun, 21 Feb 2021 14:38:16 +0000, Hector Martin <marcan@marcan.st> wrote: > > On 21/02/2021 04.17, Marc Zyngier wrote: > > On 2021-02-20 19:13, Krzysztof Kozlowski wrote: > >> On Thu, Feb 18, 2021 at 11:01:21PM +0900, Hector Martin wrote: > >>> On 16/02/2021 03.51, Krzysztof Kozlowski wrote: > >>>>> Also fix a bug checking the return value, which should use IS_ERR(). > >>>> > >>>> No, no, no. We never, never combine fixing bugs with some rework. > >>>> However devm_ioremap() returns NULL so where is the error? > >>> > >>> Sorry, this was a commit message mistake. The code is correct and so > >>> is the > >>> patch: just the NULL check is correct for the previous variant and > >>> IS_ERR is > >>> correct for devm_ioremap_resource. I confused myself while writing the > >>> commit message after the fact. > >>> > >>>> Did you test your patches on existing platforms? If not, please mark all > >>>> of them as RFT on next submission, so Greg does not pick them too fast. > >>> > >>> I unfortunately don't have any Exynos devices where I could test the > >>> code (I > >>> have a couple but no serial connections, and I have no idea if mailine > >>> would > >>> run on them). I'll mark v3 as RFT. > >> > >> If you have one of Odroid boards with Exynos, then you can nicely test > >> Exynos. Others - depends, on board. > >> Anyway I can test them for you. I just want to be sure that Greg waits > >> for this testing. > > > > Worse case, QEMU has some Exynos4210 emulation that is usable. > > That's a good point; better than nothing, certainly. > > Does anyone have a known good example of booting an exynos kernel > under qemu? I tried building a plain 5.11 arm exynos_defconfig and > booting it, but without much luck: > > $ qemu-system-arm -kernel arch/arm/boot/zImage -append > "console=ttySAC0,115200n8 debug" -dtb > arch/arm/boot/dts/exynos4210-universal_c210.dtb -nographic -serial > mon:stdio -M smdkc210 -smp 2 Here's what I've been using last time I had to muck with the 4210 stuff: <quote> qemu-system-arm \ -kernel arch/arm/boot/zImage -M smdkc210 \ -append "console=ttySAC0,115200n8 earlycon=smh root=/dev/mmcblk0p2 rootwait" \ -nographic -semihosting -smp 2 \ -dtb arch/arm/boot/dts/exynos4210-smdkv310.dtb \ -drive if=sd,driver=null-co -drive if=sd,driver=null-co \ -drive if=sd,file=../vminstall/bullseye32/MsiKFRxxujYIkiKT.img,format=raw </quote> where the last line points to a standard Debian image created separately. M.
On 21/02/2021 23.59, Marc Zyngier wrote: > Here's what I've been using last time I had to muck with the 4210 > stuff: > > <quote> > qemu-system-arm \ > -kernel arch/arm/boot/zImage -M smdkc210 \ > -append "console=ttySAC0,115200n8 earlycon=smh root=/dev/mmcblk0p2 rootwait" \ > -nographic -semihosting -smp 2 \ > -dtb arch/arm/boot/dts/exynos4210-smdkv310.dtb \ > -drive if=sd,driver=null-co -drive if=sd,driver=null-co \ > -drive if=sd,file=../vminstall/bullseye32/MsiKFRxxujYIkiKT.img,format=raw > </quote> > > where the last line points to a standard Debian image created > separately. Hah, exynos4210-smdkv310.dtb is what did it. And here I was thinking something with "c210" in the name would be more likely to work with qemu machine "smdkc210"... :-)
diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c index 821cd0e4f870..619bc4864e2a 100644 --- a/drivers/tty/serial/samsung_tty.c +++ b/drivers/tty/serial/samsung_tty.c @@ -1572,26 +1572,11 @@ static const char *s3c24xx_serial_type(struct uart_port *port) } } -#define MAP_SIZE (0x100) - -static void s3c24xx_serial_release_port(struct uart_port *port) -{ - release_mem_region(port->mapbase, MAP_SIZE); -} - -static int s3c24xx_serial_request_port(struct uart_port *port) -{ - const char *name = s3c24xx_serial_portname(port); - - return request_mem_region(port->mapbase, MAP_SIZE, name) ? 0 : -EBUSY; -} - static void s3c24xx_serial_config_port(struct uart_port *port, int flags) { struct s3c24xx_uart_info *info = s3c24xx_port_to_info(port); - if (flags & UART_CONFIG_TYPE && - s3c24xx_serial_request_port(port) == 0) + if (flags & UART_CONFIG_TYPE) port->type = info->port_type; } @@ -1644,8 +1629,6 @@ static struct uart_ops s3c24xx_serial_ops = { .shutdown = s3c24xx_serial_shutdown, .set_termios = s3c24xx_serial_set_termios, .type = s3c24xx_serial_type, - .release_port = s3c24xx_serial_release_port, - .request_port = s3c24xx_serial_request_port, .config_port = s3c24xx_serial_config_port, .verify_port = s3c24xx_serial_verify_port, #if defined(CONFIG_SERIAL_SAMSUNG_CONSOLE) && defined(CONFIG_CONSOLE_POLL) @@ -1667,8 +1650,6 @@ static const struct uart_ops s3c64xx_serial_ops = { .shutdown = s3c64xx_serial_shutdown, .set_termios = s3c24xx_serial_set_termios, .type = s3c24xx_serial_type, - .release_port = s3c24xx_serial_release_port, - .request_port = s3c24xx_serial_request_port, .config_port = s3c24xx_serial_config_port, .verify_port = s3c24xx_serial_verify_port, #if defined(CONFIG_SERIAL_SAMSUNG_CONSOLE) && defined(CONFIG_CONSOLE_POLL) @@ -1926,8 +1907,8 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport, dev_dbg(port->dev, "resource %pR)\n", res); - port->membase = devm_ioremap(port->dev, res->start, resource_size(res)); - if (!port->membase) { + port->membase = devm_ioremap_resource(port->dev, res); + if (IS_ERR(port->membase)) { dev_err(port->dev, "failed to remap controller address\n"); return -EBUSY; }
This picks up the non-posted I/O mode needed for Apple platforms to work properly. This removes the request/release functions, which are no longer necessary, since devm_ioremap_resource takes care of that already. Most other drivers already do it this way, anyway. Also fix a bug checking the return value, which should use IS_ERR(). Signed-off-by: Hector Martin <marcan@marcan.st> --- drivers/tty/serial/samsung_tty.c | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-)