| Message ID | 20230423023056.27929-1-qianfanguijin@163.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drivers: spi: sunxi: Enable irq after the initialization is done | expand |
On Sun, Apr 23, 2023 at 10:30:56AM +0800, qianfanguijin@163.com wrote: > > The kernel error messages is as follows: > > [ 1.362449] sun6i-spi 1c06000.spi: Failed to request RX DMA channel > [ 1.369654] 8<--- cut here --- > [ 1.372716] Unable to handle kernel paging request at virtual address fffffffc > [ 1.379928] pgd = (ptrval) > [ 1.382632] [fffffffc] *pgd=6bef6861, *pte=00000000, *ppte=00000000 > [ 1.388907] Internal error: Oops: 37 [#1] SMP ARM > ... > [ 1.784024] [<c0159c54>] (swake_up_locked.part.0) from [<c0159d9c>] (complete+0x30/0x40) Please think hard before including complete backtraces in upstream reports, they are very large and contain almost no useful information relative to their size so often obscure the relevant content in your message. If part of the backtrace is usefully illustrative (it often is for search engines if nothing else) then it's usually better to pull out the relevant sections. > ret = devm_request_irq(&pdev->dev, irq, sun4i_spi_handler, > - 0, "sun4i-spi", sspi); > + IRQF_NO_AUTOEN, "sun4i-spi", sspi); > if (ret) { > dev_err(&pdev->dev, "Cannot request IRQ\n"); > goto err_free_master; > @@ -506,6 +506,8 @@ static int sun4i_spi_probe(struct platform_device *pdev) > goto err_pm_disable; > } > > + enable_irq(irq); > + The usual approach would be to move the requesting of the interrupt later. Why do this instead?
在 2023/4/24 19:46, Mark Brown 写道: > On Sun, Apr 23, 2023 at 10:30:56AM +0800, qianfanguijin@163.com wrote: > >> The kernel error messages is as follows: >> >> [ 1.362449] sun6i-spi 1c06000.spi: Failed to request RX DMA channel >> [ 1.369654] 8<--- cut here --- >> [ 1.372716] Unable to handle kernel paging request at virtual address fffffffc >> [ 1.379928] pgd = (ptrval) >> [ 1.382632] [fffffffc] *pgd=6bef6861, *pte=00000000, *ppte=00000000 >> [ 1.388907] Internal error: Oops: 37 [#1] SMP ARM >> ... >> [ 1.784024] [<c0159c54>] (swake_up_locked.part.0) from [<c0159d9c>] (complete+0x30/0x40) > Please think hard before including complete backtraces in upstream > reports, they are very large and contain almost no useful information > relative to their size so often obscure the relevant content in your > message. If part of the backtrace is usefully illustrative (it often is > for search engines if nothing else) then it's usually better to pull out > the relevant sections. Thanks and I will drop the backtrace messages. > >> ret = devm_request_irq(&pdev->dev, irq, sun4i_spi_handler, >> - 0, "sun4i-spi", sspi); >> + IRQF_NO_AUTOEN, "sun4i-spi", sspi); >> if (ret) { >> dev_err(&pdev->dev, "Cannot request IRQ\n"); >> goto err_free_master; >> @@ -506,6 +506,8 @@ static int sun4i_spi_probe(struct platform_device *pdev) >> goto err_pm_disable; >> } >> >> + enable_irq(irq); >> + > The usual approach would be to move the requesting of the interrupt > later. Why do this instead? Nothing special, this way does not change the goto logic.
On Tue, Apr 25, 2023 at 08:22:16AM +0800, qianfan wrote: > 在 2023/4/24 19:46, Mark Brown 写道: > > On Sun, Apr 23, 2023 at 10:30:56AM +0800, qianfanguijin@163.com wrote: > > > + enable_irq(irq); > > The usual approach would be to move the requesting of the interrupt > > later. Why do this instead? > Nothing special, this way does not change the goto logic. It would be better to restructure the code, enable_irq() is a fairly specialist function and should only be used if it is really needed - it's a big warning sign to see it used.
diff --git a/drivers/spi/spi-sun4i.c b/drivers/spi/spi-sun4i.c index 1fdfc6e6691d..2f797b903692 100644 --- a/drivers/spi/spi-sun4i.c +++ b/drivers/spi/spi-sun4i.c @@ -452,7 +452,7 @@ static int sun4i_spi_probe(struct platform_device *pdev) } ret = devm_request_irq(&pdev->dev, irq, sun4i_spi_handler, - 0, "sun4i-spi", sspi); + IRQF_NO_AUTOEN, "sun4i-spi", sspi); if (ret) { dev_err(&pdev->dev, "Cannot request IRQ\n"); goto err_free_master; @@ -506,6 +506,8 @@ static int sun4i_spi_probe(struct platform_device *pdev) goto err_pm_disable; } + enable_irq(irq); + return 0; err_pm_disable: diff --git a/drivers/spi/spi-sun6i.c b/drivers/spi/spi-sun6i.c index 1151d8592656..0eec2d0e312b 100644 --- a/drivers/spi/spi-sun6i.c +++ b/drivers/spi/spi-sun6i.c @@ -583,7 +583,7 @@ static int sun6i_spi_probe(struct platform_device *pdev) } ret = devm_request_irq(&pdev->dev, irq, sun6i_spi_handler, - 0, "sun6i-spi", sspi); + IRQF_NO_AUTOEN, "sun6i-spi", sspi); if (ret) { dev_err(&pdev->dev, "Cannot request IRQ\n"); goto err_free_master; @@ -675,6 +675,8 @@ static int sun6i_spi_probe(struct platform_device *pdev) goto err_pm_disable; } + enable_irq(irq); + return 0; err_pm_disable: