| Message ID | 20221225-mtk-spi-fixes-v1-0-bb6c14c232f8@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spi: mediatek: Enable irq before the spi registration | expand |
On Sun, Dec 25, 2022 at 09:37:12AM +0100, Ricardo Ribalda wrote: > If the irq is enabled after the spi si registered, there can be a race > with the initialization of the devices on the spi bus. > > Eg: > mtk-spi 1100a000.spi: spi-mem transfer timeout > spi-nor: probe of spi0.0 failed with error -110 > Unable to handle kernel NULL pointer dereference at virtual address > 0000000000000010 > ... > Call trace: > mtk_spi_can_dma+0x0/0x2c > > Fixes: c6f7874687f7 ("spi: mediatek: Enable irq when pdata is ready") > Reported-by: Daniel Golle <daniel@makrotopia.org> > Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> > --- > spi: mediatek: Fix init order (again) > > Hi Mark > > Here is a fixup of the previous patch. Daniel, can you confirm that it > works on your hardware? Yes, this fixes the issue and SPI now works fine on MT7986 with the patch applied. Tested-by: Daniel Golle <daniel@makrotopia.org> > > Thanks and sorry for annoyance. > > To: Mark Brown <broonie@kernel.org> > To: Matthias Brugger <matthias.bgg@gmail.com> > To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > Cc: linux-spi@vger.kernel.org > Cc: linux-arm-kernel@lists.infradead.org > Cc: linux-mediatek@lists.infradead.org > Cc: linux-kernel@vger.kernel.org > Cc: Daniel Golle <daniel@makrotopia.org> > --- > drivers/spi/spi-mt65xx.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c > index 6de8360e5c2a..9eab6c20dbc5 100644 > --- a/drivers/spi/spi-mt65xx.c > +++ b/drivers/spi/spi-mt65xx.c > @@ -1253,6 +1253,11 @@ static int mtk_spi_probe(struct platform_device *pdev) > dev_notice(dev, "SPI dma_set_mask(%d) failed, ret:%d\n", > addr_bits, ret); > > + ret = devm_request_irq(dev, irq, mtk_spi_interrupt, > + IRQF_TRIGGER_NONE, dev_name(dev), master); > + if (ret) > + return dev_err_probe(dev, ret, "failed to register irq\n"); > + > pm_runtime_enable(dev); > > ret = devm_spi_register_master(dev, master); > @@ -1261,13 +1266,6 @@ static int mtk_spi_probe(struct platform_device *pdev) > return dev_err_probe(dev, ret, "failed to register master\n"); > } > > - ret = devm_request_irq(dev, irq, mtk_spi_interrupt, > - IRQF_TRIGGER_NONE, dev_name(dev), master); > - if (ret) { > - pm_runtime_disable(dev); > - return dev_err_probe(dev, ret, "failed to register irq\n"); > - } > - > return 0; > } > > > --- > base-commit: 45b3cd900bf8d1a3cd7cf48361df8c09ae5b4009 > change-id: 20221225-mtk-spi-fixes-99c863a6fdf1 > > Best regards, > -- > Ricardo Ribalda <ribalda@chromium.org>
On Sun, 25 Dec 2022 09:37:12 +0100, Ricardo Ribalda wrote: > If the irq is enabled after the spi si registered, there can be a race > with the initialization of the devices on the spi bus. > > Eg: > mtk-spi 1100a000.spi: spi-mem transfer timeout > spi-nor: probe of spi0.0 failed with error -110 > Unable to handle kernel NULL pointer dereference at virtual address > 0000000000000010 > ... > Call trace: > mtk_spi_can_dma+0x0/0x2c > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: mediatek: Enable irq before the spi registration commit: b24cded8c065d7cef8690b2c7b82b828cce57708 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
On Sun, 25 Dec 2022 09:37:12 +0100, Ricardo Ribalda wrote: > If the irq is enabled after the spi si registered, there can be a race > with the initialization of the devices on the spi bus. > > Eg: > mtk-spi 1100a000.spi: spi-mem transfer timeout > spi-nor: probe of spi0.0 failed with error -110 > Unable to handle kernel NULL pointer dereference at virtual address > 0000000000000010 > ... > Call trace: > mtk_spi_can_dma+0x0/0x2c > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next Thanks! [1/1] spi: mediatek: Enable irq before the spi registration commit: b24cded8c065d7cef8690b2c7b82b828cce57708 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
Hi Mark! On 27.12.22 14:43, Mark Brown wrote: > On Sun, 25 Dec 2022 09:37:12 +0100, Ricardo Ribalda wrote: >> If the irq is enabled after the spi si registered, there can be a race >> with the initialization of the devices on the spi bus. >> >> Eg: >> mtk-spi 1100a000.spi: spi-mem transfer timeout >> spi-nor: probe of spi0.0 failed with error -110 >> Unable to handle kernel NULL pointer dereference at virtual address >> 0000000000000010 >> ... >> Call trace: >> mtk_spi_can_dma+0x0/0x2c >> >> [...] > > Applied to > > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next > > Thanks! > > [1/1] spi: mediatek: Enable irq before the spi registration > commit: b24cded8c065d7cef8690b2c7b82b828cce57708 > > All being well this means that it will be integrated into the linux-next > tree (usually sometime in the next 24 hours) and sent to Linus during > the next merge window (or sooner if it is a bug fix), however if > problems are discovered then the patch may be dropped or reverted. > [...] Quick question: why did you queue this for the next merge window? This change *afaics* is fixing a reported regression (a kernel oops) introduced this cycle: https://lore.kernel.org/lkml/Y6dL2ZWgd1BD6kew@makrotopia.org/ Or have I missed or confused something? Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr That page also explains what to do if mails like this annoy you. #regzbot poke
On Wed, Jan 04, 2023 at 03:08:51PM +0100, Thorsten Leemhuis wrote: > On 27.12.22 14:43, Mark Brown wrote: > > the next merge window (or sooner if it is a bug fix), however if > > problems are discovered then the patch may be dropped or reverted. > Quick question: why did you queue this for the next merge window? This > change *afaics* is fixing a reported regression (a kernel oops) > introduced this cycle: > https://lore.kernel.org/lkml/Y6dL2ZWgd1BD6kew@makrotopia.org/ > Or have I missed or confused something? You've missed something, it's queued for 6.2.
diff --git a/drivers/spi/spi-mt65xx.c b/drivers/spi/spi-mt65xx.c index 6de8360e5c2a..9eab6c20dbc5 100644 --- a/drivers/spi/spi-mt65xx.c +++ b/drivers/spi/spi-mt65xx.c @@ -1253,6 +1253,11 @@ static int mtk_spi_probe(struct platform_device *pdev) dev_notice(dev, "SPI dma_set_mask(%d) failed, ret:%d\n", addr_bits, ret); + ret = devm_request_irq(dev, irq, mtk_spi_interrupt, + IRQF_TRIGGER_NONE, dev_name(dev), master); + if (ret) + return dev_err_probe(dev, ret, "failed to register irq\n"); + pm_runtime_enable(dev); ret = devm_spi_register_master(dev, master); @@ -1261,13 +1266,6 @@ static int mtk_spi_probe(struct platform_device *pdev) return dev_err_probe(dev, ret, "failed to register master\n"); } - ret = devm_request_irq(dev, irq, mtk_spi_interrupt, - IRQF_TRIGGER_NONE, dev_name(dev), master); - if (ret) { - pm_runtime_disable(dev); - return dev_err_probe(dev, ret, "failed to register irq\n"); - } - return 0; }
If the irq is enabled after the spi si registered, there can be a race with the initialization of the devices on the spi bus. Eg: mtk-spi 1100a000.spi: spi-mem transfer timeout spi-nor: probe of spi0.0 failed with error -110 Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010 ... Call trace: mtk_spi_can_dma+0x0/0x2c Fixes: c6f7874687f7 ("spi: mediatek: Enable irq when pdata is ready") Reported-by: Daniel Golle <daniel@makrotopia.org> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> --- spi: mediatek: Fix init order (again) Hi Mark Here is a fixup of the previous patch. Daniel, can you confirm that it works on your hardware? Thanks and sorry for annoyance. To: Mark Brown <broonie@kernel.org> To: Matthias Brugger <matthias.bgg@gmail.com> To: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Cc: linux-spi@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Cc: linux-mediatek@lists.infradead.org Cc: linux-kernel@vger.kernel.org Cc: Daniel Golle <daniel@makrotopia.org> --- drivers/spi/spi-mt65xx.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) --- base-commit: 45b3cd900bf8d1a3cd7cf48361df8c09ae5b4009 change-id: 20221225-mtk-spi-fixes-99c863a6fdf1 Best regards,