* [PATCH 0/3] serial: 8250_aspeed_vuart: fix duplicate __release_region() on unbind @ 2021-05-10 1:42 Zev Weiss 2021-05-10 1:42 ` [PATCH 1/3] serial: 8250_aspeed_vuart: factor out aspeed_vuart_{read, write}b() helper functions Zev Weiss ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Zev Weiss @ 2021-05-10 1:42 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: openbmc, Zev Weiss, Jiri Slaby, Joel Stanley, Andrew Jeffery, Johan Hovold, linux-serial, linux-arm-kernel, linux-aspeed, linux-kernel Hello, This series addresses a bug reported by Andrew Jeffery [0] wherein the aspeed-vuart driver triggers a "Trying to free nonexistent resource" warning from __release_region() when unbound from a device. The first two patches provide some preparatory refactoring and cleanups; the actual fix is in the final patch. Joel, note that this series has a dependency on the "generalized DT properties" series [1] (specifically the second patch), which has been merged upstream but is not yet in the OpenBMC dev-5.10 tree. [0] https://github.com/openbmc/linux/issues/203 [1] https://lore.kernel.org/openbmc/20210412034712.16778-1-zev@bewilderbeest.net/ Zev Weiss (3): serial: 8250_aspeed_vuart: factor out aspeed_vuart_{read,write}b() helper functions serial: 8250_aspeed_vuart: initialize vuart->port in aspeed_vuart_probe() serial: 8250_aspeed_vuart: use UPF_IOREMAP to set up register mapping drivers/tty/serial/8250/8250_aspeed_vuart.c | 50 +++++++++++---------- 1 file changed, 27 insertions(+), 23 deletions(-) -- 2.31.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] serial: 8250_aspeed_vuart: factor out aspeed_vuart_{read, write}b() helper functions 2021-05-10 1:42 [PATCH 0/3] serial: 8250_aspeed_vuart: fix duplicate __release_region() on unbind Zev Weiss @ 2021-05-10 1:42 ` Zev Weiss 2021-05-13 1:21 ` Andrew Jeffery 2021-05-10 1:42 ` [PATCH 2/3] serial: 8250_aspeed_vuart: initialize vuart->port in aspeed_vuart_probe() Zev Weiss 2021-05-10 1:42 ` [PATCH 3/3] serial: 8250_aspeed_vuart: use UPF_IOREMAP to set up register mapping Zev Weiss 2 siblings, 1 reply; 8+ messages in thread From: Zev Weiss @ 2021-05-10 1:42 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: openbmc, Zev Weiss, Jiri Slaby, Joel Stanley, Andrew Jeffery, Johan Hovold, linux-serial, linux-arm-kernel, linux-aspeed, linux-kernel This is a small prepatory step for changing the way this driver does its I/O accesses. Signed-off-by: Zev Weiss <zev@bewilderbeest.net> --- drivers/tty/serial/8250/8250_aspeed_vuart.c | 38 +++++++++++++-------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c index 61550f24a2d3..9e8b2e8e32b6 100644 --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c @@ -64,14 +64,24 @@ static const int unthrottle_timeout = HZ/10; * different system (though most of them use 3f8/4). */ +static inline u8 aspeed_vuart_readb(struct aspeed_vuart *vuart, u8 reg) +{ + return readb(vuart->regs + reg); +} + +static inline void aspeed_vuart_writeb(struct aspeed_vuart *vuart, u8 val, u8 reg) +{ + writeb(val, vuart->regs + reg); +} + static ssize_t lpc_address_show(struct device *dev, struct device_attribute *attr, char *buf) { struct aspeed_vuart *vuart = dev_get_drvdata(dev); u16 addr; - addr = (readb(vuart->regs + ASPEED_VUART_ADDRH) << 8) | - (readb(vuart->regs + ASPEED_VUART_ADDRL)); + addr = (aspeed_vuart_readb(vuart, ASPEED_VUART_ADDRH) << 8) | + (aspeed_vuart_readb(vuart, ASPEED_VUART_ADDRL)); return snprintf(buf, PAGE_SIZE - 1, "0x%x\n", addr); } @@ -81,8 +91,8 @@ static int aspeed_vuart_set_lpc_address(struct aspeed_vuart *vuart, u32 addr) if (addr > U16_MAX) return -EINVAL; - writeb(addr >> 8, vuart->regs + ASPEED_VUART_ADDRH); - writeb(addr >> 0, vuart->regs + ASPEED_VUART_ADDRL); + aspeed_vuart_writeb(vuart, addr >> 8, ASPEED_VUART_ADDRH); + aspeed_vuart_writeb(vuart, addr >> 0, ASPEED_VUART_ADDRL); return 0; } @@ -111,7 +121,7 @@ static ssize_t sirq_show(struct device *dev, struct aspeed_vuart *vuart = dev_get_drvdata(dev); u8 reg; - reg = readb(vuart->regs + ASPEED_VUART_GCRB); + reg = aspeed_vuart_readb(vuart, ASPEED_VUART_GCRB); reg &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK; reg >>= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT; @@ -128,10 +138,10 @@ static int aspeed_vuart_set_sirq(struct aspeed_vuart *vuart, u32 sirq) sirq <<= ASPEED_VUART_GCRB_HOST_SIRQ_SHIFT; sirq &= ASPEED_VUART_GCRB_HOST_SIRQ_MASK; - reg = readb(vuart->regs + ASPEED_VUART_GCRB); + reg = aspeed_vuart_readb(vuart, ASPEED_VUART_GCRB); reg &= ~ASPEED_VUART_GCRB_HOST_SIRQ_MASK; reg |= sirq; - writeb(reg, vuart->regs + ASPEED_VUART_GCRB); + aspeed_vuart_writeb(vuart, reg, ASPEED_VUART_GCRB); return 0; } @@ -159,7 +169,7 @@ static ssize_t sirq_polarity_show(struct device *dev, struct aspeed_vuart *vuart = dev_get_drvdata(dev); u8 reg; - reg = readb(vuart->regs + ASPEED_VUART_GCRA); + reg = aspeed_vuart_readb(vuart, ASPEED_VUART_GCRA); reg &= ASPEED_VUART_GCRA_HOST_SIRQ_POLARITY; return snprintf(buf, PAGE_SIZE - 1, "%u\n", reg ? 1 : 0); @@ -168,14 +178,14 @@ static ssize_t sirq_polarity_show(struct device *dev, static void aspeed_vuart_set_sirq_polarity(struct aspeed_vuart *vuart, bool polarity) { - u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA); + u8 reg = aspeed_vuart_readb(vuart, ASPEED_VUART_GCRA); if (polarity) reg |= ASPEED_VUART_GCRA_HOST_SIRQ_POLARITY; else reg &= ~ASPEED_VUART_GCRA_HOST_SIRQ_POLARITY; - writeb(reg, vuart->regs + ASPEED_VUART_GCRA); + aspeed_vuart_writeb(vuart, reg, ASPEED_VUART_GCRA); } static ssize_t sirq_polarity_store(struct device *dev, @@ -210,14 +220,14 @@ static const struct attribute_group aspeed_vuart_attr_group = { static void aspeed_vuart_set_enabled(struct aspeed_vuart *vuart, bool enabled) { - u8 reg = readb(vuart->regs + ASPEED_VUART_GCRA); + u8 reg = aspeed_vuart_readb(vuart, ASPEED_VUART_GCRA); if (enabled) reg |= ASPEED_VUART_GCRA_VUART_EN; else reg &= ~ASPEED_VUART_GCRA_VUART_EN; - writeb(reg, vuart->regs + ASPEED_VUART_GCRA); + aspeed_vuart_writeb(vuart, reg, ASPEED_VUART_GCRA); } static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart, @@ -225,7 +235,7 @@ static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart, { u8 reg; - reg = readb(vuart->regs + ASPEED_VUART_GCRA); + reg = aspeed_vuart_readb(vuart, ASPEED_VUART_GCRA); /* If the DISABLE_HOST_TX_DISCARD bit is set, discard is disabled */ if (!discard) @@ -233,7 +243,7 @@ static void aspeed_vuart_set_host_tx_discard(struct aspeed_vuart *vuart, else reg &= ~ASPEED_VUART_GCRA_DISABLE_HOST_TX_DISCARD; - writeb(reg, vuart->regs + ASPEED_VUART_GCRA); + aspeed_vuart_writeb(vuart, reg, ASPEED_VUART_GCRA); } static int aspeed_vuart_startup(struct uart_port *uart_port) -- 2.31.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] serial: 8250_aspeed_vuart: factor out aspeed_vuart_{read, write}b() helper functions 2021-05-10 1:42 ` [PATCH 1/3] serial: 8250_aspeed_vuart: factor out aspeed_vuart_{read, write}b() helper functions Zev Weiss @ 2021-05-13 1:21 ` Andrew Jeffery 0 siblings, 0 replies; 8+ messages in thread From: Andrew Jeffery @ 2021-05-13 1:21 UTC (permalink / raw) To: Zev Weiss, Greg Kroah-Hartman Cc: openbmc, Jiri Slaby, Joel Stanley, Johan Hovold, linux-serial, linux-arm-kernel, linux-aspeed, linux-kernel On Mon, 10 May 2021, at 11:12, Zev Weiss wrote: > This is a small prepatory step for changing the way this driver does > its I/O accesses. > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] serial: 8250_aspeed_vuart: initialize vuart->port in aspeed_vuart_probe() 2021-05-10 1:42 [PATCH 0/3] serial: 8250_aspeed_vuart: fix duplicate __release_region() on unbind Zev Weiss 2021-05-10 1:42 ` [PATCH 1/3] serial: 8250_aspeed_vuart: factor out aspeed_vuart_{read, write}b() helper functions Zev Weiss @ 2021-05-10 1:42 ` Zev Weiss 2021-05-13 1:34 ` Andrew Jeffery 2021-05-10 1:42 ` [PATCH 3/3] serial: 8250_aspeed_vuart: use UPF_IOREMAP to set up register mapping Zev Weiss 2 siblings, 1 reply; 8+ messages in thread From: Zev Weiss @ 2021-05-10 1:42 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: openbmc, Zev Weiss, Jiri Slaby, Joel Stanley, Andrew Jeffery, Johan Hovold, linux-serial, linux-arm-kernel, linux-aspeed, linux-kernel Previously this had only been initialized if we hit the throttling path in aspeed_vuart_handle_irq(); moving it to the probe function is a slight consistency improvement and avoids redundant reinitialization in the interrupt handler. It also serves as preparation for converting the driver's I/O accesses to use port->port.membase instead of its own vuart->regs. Signed-off-by: Zev Weiss <zev@bewilderbeest.net> --- drivers/tty/serial/8250/8250_aspeed_vuart.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c index 9e8b2e8e32b6..249164dc397b 100644 --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c @@ -349,11 +349,9 @@ static int aspeed_vuart_handle_irq(struct uart_port *port) struct aspeed_vuart *vuart = port->private_data; __aspeed_vuart_set_throttle(up, true); - if (!timer_pending(&vuart->unthrottle_timer)) { - vuart->port = up; + if (!timer_pending(&vuart->unthrottle_timer)) mod_timer(&vuart->unthrottle_timer, jiffies + unthrottle_timeout); - } } else { count = min(space, 256); @@ -511,6 +509,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev) goto err_clk_disable; vuart->line = rc; + vuart->port = serial8250_get_port(vuart->line); rc = of_parse_phandle_with_fixed_args( np, "aspeed,sirq-polarity-sense", 2, 0, -- 2.31.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] serial: 8250_aspeed_vuart: initialize vuart->port in aspeed_vuart_probe() 2021-05-10 1:42 ` [PATCH 2/3] serial: 8250_aspeed_vuart: initialize vuart->port in aspeed_vuart_probe() Zev Weiss @ 2021-05-13 1:34 ` Andrew Jeffery 2021-05-13 19:25 ` Zev Weiss 0 siblings, 1 reply; 8+ messages in thread From: Andrew Jeffery @ 2021-05-13 1:34 UTC (permalink / raw) To: Zev Weiss, Greg Kroah-Hartman, Jeremy Kerr Cc: openbmc, Jiri Slaby, Joel Stanley, Johan Hovold, linux-serial, linux-arm-kernel, linux-aspeed, linux-kernel On Mon, 10 May 2021, at 11:12, Zev Weiss wrote: > Previously this had only been initialized if we hit the throttling path > in aspeed_vuart_handle_irq(); moving it to the probe function is a > slight consistency improvement and avoids redundant reinitialization in > the interrupt handler. It also serves as preparation for converting the > driver's I/O accesses to use port->port.membase instead of its own > vuart->regs. > > Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > --- > drivers/tty/serial/8250/8250_aspeed_vuart.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c > b/drivers/tty/serial/8250/8250_aspeed_vuart.c > index 9e8b2e8e32b6..249164dc397b 100644 > --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c > +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c > @@ -349,11 +349,9 @@ static int aspeed_vuart_handle_irq(struct > uart_port *port) > struct aspeed_vuart *vuart = port->private_data; > __aspeed_vuart_set_throttle(up, true); > > - if (!timer_pending(&vuart->unthrottle_timer)) { > - vuart->port = up; > + if (!timer_pending(&vuart->unthrottle_timer)) > mod_timer(&vuart->unthrottle_timer, > jiffies + unthrottle_timeout); > - } > > } else { > count = min(space, 256); > @@ -511,6 +509,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev) > goto err_clk_disable; > > vuart->line = rc; > + vuart->port = serial8250_get_port(vuart->line); The documentation of serial8250_get_port() is somewhat concerning wrt the use: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/8250/8250_core.c?h=v5.13-rc1#n399 However, given the existing behaviour it shouldn't be problematic? Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] serial: 8250_aspeed_vuart: initialize vuart->port in aspeed_vuart_probe() 2021-05-13 1:34 ` Andrew Jeffery @ 2021-05-13 19:25 ` Zev Weiss 2021-05-14 1:58 ` Andrew Jeffery 0 siblings, 1 reply; 8+ messages in thread From: Zev Weiss @ 2021-05-13 19:25 UTC (permalink / raw) To: Andrew Jeffery Cc: Greg Kroah-Hartman, Jeremy Kerr, openbmc, Jiri Slaby, Joel Stanley, Johan Hovold, linux-serial, linux-arm-kernel, linux-aspeed, linux-kernel On Wed, May 12, 2021 at 08:34:06PM CDT, Andrew Jeffery wrote: > > >On Mon, 10 May 2021, at 11:12, Zev Weiss wrote: >> Previously this had only been initialized if we hit the throttling path >> in aspeed_vuart_handle_irq(); moving it to the probe function is a >> slight consistency improvement and avoids redundant reinitialization in >> the interrupt handler. It also serves as preparation for converting the >> driver's I/O accesses to use port->port.membase instead of its own >> vuart->regs. >> >> Signed-off-by: Zev Weiss <zev@bewilderbeest.net> >> --- >> drivers/tty/serial/8250/8250_aspeed_vuart.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c >> b/drivers/tty/serial/8250/8250_aspeed_vuart.c >> index 9e8b2e8e32b6..249164dc397b 100644 >> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c >> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c >> @@ -349,11 +349,9 @@ static int aspeed_vuart_handle_irq(struct >> uart_port *port) >> struct aspeed_vuart *vuart = port->private_data; >> __aspeed_vuart_set_throttle(up, true); >> >> - if (!timer_pending(&vuart->unthrottle_timer)) { >> - vuart->port = up; >> + if (!timer_pending(&vuart->unthrottle_timer)) >> mod_timer(&vuart->unthrottle_timer, >> jiffies + unthrottle_timeout); >> - } >> >> } else { >> count = min(space, 256); >> @@ -511,6 +509,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev) >> goto err_clk_disable; >> >> vuart->line = rc; >> + vuart->port = serial8250_get_port(vuart->line); > >The documentation of serial8250_get_port() is somewhat concerning wrt >the use: > >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/8250/8250_core.c?h=v5.13-rc1#n399 Hmm, good point -- though despite that comment it looks like there is some existing code using it outside of suspend/resume callbacks (in 8250_pci.c and 8250_pnp.c). I'm not certain if those would necessarily be considered good precedent to follow for this, but I don't see any obvious better way of getting hold of the corresponding uart_8250_port (or its port.membase). I did receive a notification that Greg had added this series to his tty-testing branch; not sure if that means he thinks it's OK or if it just kind of slipped by unnoticed though. > >However, given the existing behaviour it shouldn't be problematic? > "existing behaviour" referring to what here? Thanks, Zev _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] serial: 8250_aspeed_vuart: initialize vuart->port in aspeed_vuart_probe() 2021-05-13 19:25 ` Zev Weiss @ 2021-05-14 1:58 ` Andrew Jeffery 0 siblings, 0 replies; 8+ messages in thread From: Andrew Jeffery @ 2021-05-14 1:58 UTC (permalink / raw) To: Zev Weiss Cc: Greg Kroah-Hartman, Jeremy Kerr, openbmc, Jiri Slaby, Joel Stanley, Johan Hovold, linux-serial, linux-arm-kernel, linux-aspeed, linux-kernel On Fri, 14 May 2021, at 04:55, Zev Weiss wrote: > On Wed, May 12, 2021 at 08:34:06PM CDT, Andrew Jeffery wrote: > > > > > >On Mon, 10 May 2021, at 11:12, Zev Weiss wrote: > >> Previously this had only been initialized if we hit the throttling path > >> in aspeed_vuart_handle_irq(); moving it to the probe function is a > >> slight consistency improvement and avoids redundant reinitialization in > >> the interrupt handler. It also serves as preparation for converting the > >> driver's I/O accesses to use port->port.membase instead of its own > >> vuart->regs. > >> > >> Signed-off-by: Zev Weiss <zev@bewilderbeest.net> > >> --- > >> drivers/tty/serial/8250/8250_aspeed_vuart.c | 5 ++--- > >> 1 file changed, 2 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c > >> b/drivers/tty/serial/8250/8250_aspeed_vuart.c > >> index 9e8b2e8e32b6..249164dc397b 100644 > >> --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c > >> +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c > >> @@ -349,11 +349,9 @@ static int aspeed_vuart_handle_irq(struct > >> uart_port *port) > >> struct aspeed_vuart *vuart = port->private_data; > >> __aspeed_vuart_set_throttle(up, true); > >> > >> - if (!timer_pending(&vuart->unthrottle_timer)) { > >> - vuart->port = up; > >> + if (!timer_pending(&vuart->unthrottle_timer)) > >> mod_timer(&vuart->unthrottle_timer, > >> jiffies + unthrottle_timeout); > >> - } > >> > >> } else { > >> count = min(space, 256); > >> @@ -511,6 +509,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev) > >> goto err_clk_disable; > >> > >> vuart->line = rc; > >> + vuart->port = serial8250_get_port(vuart->line); > > > >The documentation of serial8250_get_port() is somewhat concerning wrt > >the use: > > > >https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tty/serial/8250/8250_core.c?h=v5.13-rc1#n399 > > Hmm, good point -- though despite that comment it looks like there is > some existing code using it outside of suspend/resume callbacks (in > 8250_pci.c and 8250_pnp.c). I'm not certain if those would necessarily > be considered good precedent to follow for this, but I don't see any > obvious better way of getting hold of the corresponding uart_8250_port > (or its port.membase). > > I did receive a notification that Greg had added this series to his > tty-testing branch; not sure if that means he thinks it's OK or if it > just kind of slipped by unnoticed though. Yeah, I just highlighted it in case anyone else wanted to weigh in. Essentially I'm just deferring to Greg. If he's picked them up, great! > > > > >However, given the existing behaviour it shouldn't be problematic? > > > > "existing behaviour" referring to what here? Well, we were poking at the registers through vuart->regs anyway. So I don't think what you've done is any less correct. Andrew _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] serial: 8250_aspeed_vuart: use UPF_IOREMAP to set up register mapping 2021-05-10 1:42 [PATCH 0/3] serial: 8250_aspeed_vuart: fix duplicate __release_region() on unbind Zev Weiss 2021-05-10 1:42 ` [PATCH 1/3] serial: 8250_aspeed_vuart: factor out aspeed_vuart_{read, write}b() helper functions Zev Weiss 2021-05-10 1:42 ` [PATCH 2/3] serial: 8250_aspeed_vuart: initialize vuart->port in aspeed_vuart_probe() Zev Weiss @ 2021-05-10 1:42 ` Zev Weiss 2 siblings, 0 replies; 8+ messages in thread From: Zev Weiss @ 2021-05-10 1:42 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: openbmc, Zev Weiss, Andrew Jeffery, Jiri Slaby, Joel Stanley, Johan Hovold, linux-serial, linux-arm-kernel, linux-aspeed, linux-kernel Previously this driver's use of devm_ioremap_resource() led to duplicated calls to __release_region() when unbinding it (one from serial8250_release_std_resource() and one from devres_release_all()), the second of which resulted in a warning message: # echo 1e787000.serial > /sys/bus/platform/drivers/aspeed-vuart/unbind [33091.774200] Trying to free nonexistent resource <000000001e787000-000000001e78703f> With this change the driver uses the generic serial8250 code's UPF_IOREMAP to take care of the register mapping automatically instead of doing its own devm_ioremap_resource(), thus avoiding the duplicate __release_region() on unbind. In doing this we eliminate vuart->regs, since it merely duplicates vuart->port->port.membase, which we now use for our I/O accesses. Signed-off-by: Zev Weiss <zev@bewilderbeest.net> Reported-by: Andrew Jeffery <andrew@aj.id.au> --- drivers/tty/serial/8250/8250_aspeed_vuart.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/drivers/tty/serial/8250/8250_aspeed_vuart.c b/drivers/tty/serial/8250/8250_aspeed_vuart.c index 249164dc397b..2bf1d8582d9a 100644 --- a/drivers/tty/serial/8250/8250_aspeed_vuart.c +++ b/drivers/tty/serial/8250/8250_aspeed_vuart.c @@ -34,7 +34,6 @@ struct aspeed_vuart { struct device *dev; - void __iomem *regs; struct clk *clk; int line; struct timer_list unthrottle_timer; @@ -66,12 +65,12 @@ static const int unthrottle_timeout = HZ/10; static inline u8 aspeed_vuart_readb(struct aspeed_vuart *vuart, u8 reg) { - return readb(vuart->regs + reg); + return readb(vuart->port->port.membase + reg); } static inline void aspeed_vuart_writeb(struct aspeed_vuart *vuart, u8 val, u8 reg) { - writeb(val, vuart->regs + reg); + writeb(val, vuart->port->port.membase + reg); } static ssize_t lpc_address_show(struct device *dev, @@ -429,13 +428,9 @@ static int aspeed_vuart_probe(struct platform_device *pdev) timer_setup(&vuart->unthrottle_timer, aspeed_vuart_unthrottle_exp, 0); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - vuart->regs = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(vuart->regs)) - return PTR_ERR(vuart->regs); memset(&port, 0, sizeof(port)); port.port.private_data = vuart; - port.port.membase = vuart->regs; port.port.mapbase = res->start; port.port.mapsize = resource_size(res); port.port.startup = aspeed_vuart_startup; @@ -492,7 +487,7 @@ static int aspeed_vuart_probe(struct platform_device *pdev) port.port.iotype = UPIO_MEM; port.port.type = PORT_16550A; port.port.uartclk = clk; - port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF + port.port.flags = UPF_SHARE_IRQ | UPF_BOOT_AUTOCONF | UPF_IOREMAP | UPF_FIXED_PORT | UPF_FIXED_TYPE | UPF_NO_THRE_TEST; if (of_property_read_bool(np, "no-loopback-test")) -- 2.31.1 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-05-14 2:01 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-10 1:42 [PATCH 0/3] serial: 8250_aspeed_vuart: fix duplicate __release_region() on unbind Zev Weiss 2021-05-10 1:42 ` [PATCH 1/3] serial: 8250_aspeed_vuart: factor out aspeed_vuart_{read, write}b() helper functions Zev Weiss 2021-05-13 1:21 ` Andrew Jeffery 2021-05-10 1:42 ` [PATCH 2/3] serial: 8250_aspeed_vuart: initialize vuart->port in aspeed_vuart_probe() Zev Weiss 2021-05-13 1:34 ` Andrew Jeffery 2021-05-13 19:25 ` Zev Weiss 2021-05-14 1:58 ` Andrew Jeffery 2021-05-10 1:42 ` [PATCH 3/3] serial: 8250_aspeed_vuart: use UPF_IOREMAP to set up register mapping Zev Weiss
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).