| Message ID | d8cb86b2717e0fc084b6651be54ee6d96dbc603a.1436174801.git.nsekhar@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Sekhar, On 07/06/2015 05:47 AM, Sekhar Nori wrote: > omap_device infrastructure has a suspend_noirq hook which > runtime suspends all devices late in the suspend cycle (see > _od_suspend_noirq() in arch/arm/mach-omap2/omap_device.c) Why is omap2 the only arch/SoC that does this; ie., call the runtime callbacks after the system pm callbacks? Whatever positive it brings, it's a mess at the driver level. For example, this driver has to hook prepare()/complete() so it can set local state so that it can detect when the runtime suspend is being called during system suspend. Regards, Peter Hurley > This leads to a NULL pointer exception in 8250_omap driver > since by the time omap8250_runtime_suspend() is called, 8250_dma > driver has already set rxchan to NULL via serial8250_release_dma(). > > Make an explicit check to see if rxchan is NULL in > runtime_{suspend|resume} hooks to fix this. > > Signed-off-by: Sekhar Nori <nsekhar@ti.com> > --- > Previous version: http://www.spinics.net/lists/linux-omap/msg119459.html > No change in this version except rebased to v4.2-rc1 > > drivers/tty/serial/8250/8250_omap.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c > index d75a66c72750..20c5b9c4c288 100644 > --- a/drivers/tty/serial/8250/8250_omap.c > +++ b/drivers/tty/serial/8250/8250_omap.c > @@ -1285,7 +1285,7 @@ static int omap8250_runtime_suspend(struct device *dev) > return -EBUSY; > } > > - if (up->dma) > + if (up->dma && up->dma->rxchan) > omap_8250_rx_dma(up, UART_IIR_RX_TIMEOUT); > > priv->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; > @@ -1310,7 +1310,7 @@ static int omap8250_runtime_resume(struct device *dev) > if (loss_cntx) > omap8250_restore_regs(up); > > - if (up->dma) > + if (up->dma && up->dma->rxchan) > omap_8250_rx_dma(up, 0); > > priv->latency = priv->calc_latency; > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Peter, On Wednesday 08 July 2015 07:34 PM, Peter Hurley wrote: > Hi Sekhar, > > On 07/06/2015 05:47 AM, Sekhar Nori wrote: >> omap_device infrastructure has a suspend_noirq hook which >> runtime suspends all devices late in the suspend cycle (see >> _od_suspend_noirq() in arch/arm/mach-omap2/omap_device.c) > > Why is omap2 the only arch/SoC that does this; ie., call the runtime > callbacks after the system pm callbacks? Even I am not sure why this needs to be done. I _think_ it was done to make sure all IPs are idled even if driver did not implement system sleep ops, but I could be wrong. Cc: Kevin Hilman since he added this support. In any case, I think this patch is okay, as the additional NULL pointer check is still valid. Thanks, Sekhar > > Whatever positive it brings, it's a mess at the driver level. > For example, this driver has to hook prepare()/complete() so it can > set local state so that it can detect when the runtime suspend > is being called during system suspend. > > Regards, > Peter Hurley > > >> This leads to a NULL pointer exception in 8250_omap driver >> since by the time omap8250_runtime_suspend() is called, 8250_dma >> driver has already set rxchan to NULL via serial8250_release_dma(). >> >> Make an explicit check to see if rxchan is NULL in >> runtime_{suspend|resume} hooks to fix this. >> >> Signed-off-by: Sekhar Nori <nsekhar@ti.com> >> --- >> Previous version: http://www.spinics.net/lists/linux-omap/msg119459.html >> No change in this version except rebased to v4.2-rc1 >> >> drivers/tty/serial/8250/8250_omap.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c >> index d75a66c72750..20c5b9c4c288 100644 >> --- a/drivers/tty/serial/8250/8250_omap.c >> +++ b/drivers/tty/serial/8250/8250_omap.c >> @@ -1285,7 +1285,7 @@ static int omap8250_runtime_suspend(struct device *dev) >> return -EBUSY; >> } >> >> - if (up->dma) >> + if (up->dma && up->dma->rxchan) >> omap_8250_rx_dma(up, UART_IIR_RX_TIMEOUT); >> >> priv->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; >> @@ -1310,7 +1310,7 @@ static int omap8250_runtime_resume(struct device *dev) >> if (loss_cntx) >> omap8250_restore_regs(up); >> >> - if (up->dma) >> + if (up->dma && up->dma->rxchan) >> omap_8250_rx_dma(up, 0); >> >> priv->latency = priv->calc_latency; >> > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c index d75a66c72750..20c5b9c4c288 100644 --- a/drivers/tty/serial/8250/8250_omap.c +++ b/drivers/tty/serial/8250/8250_omap.c @@ -1285,7 +1285,7 @@ static int omap8250_runtime_suspend(struct device *dev) return -EBUSY; } - if (up->dma) + if (up->dma && up->dma->rxchan) omap_8250_rx_dma(up, UART_IIR_RX_TIMEOUT); priv->latency = PM_QOS_CPU_DMA_LAT_DEFAULT_VALUE; @@ -1310,7 +1310,7 @@ static int omap8250_runtime_resume(struct device *dev) if (loss_cntx) omap8250_restore_regs(up); - if (up->dma) + if (up->dma && up->dma->rxchan) omap_8250_rx_dma(up, 0); priv->latency = priv->calc_latency;
omap_device infrastructure has a suspend_noirq hook which runtime suspends all devices late in the suspend cycle (see _od_suspend_noirq() in arch/arm/mach-omap2/omap_device.c) This leads to a NULL pointer exception in 8250_omap driver since by the time omap8250_runtime_suspend() is called, 8250_dma driver has already set rxchan to NULL via serial8250_release_dma(). Make an explicit check to see if rxchan is NULL in runtime_{suspend|resume} hooks to fix this. Signed-off-by: Sekhar Nori <nsekhar@ti.com> --- Previous version: http://www.spinics.net/lists/linux-omap/msg119459.html No change in this version except rebased to v4.2-rc1 drivers/tty/serial/8250/8250_omap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)