* Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")
@ 2019-11-08 12:33 Christophe Leroy
2019-11-08 13:09 ` Linus Walleij
0 siblings, 1 reply; 26+ messages in thread
From: Christophe Leroy @ 2019-11-08 12:33 UTC (permalink / raw)
To: Linus Walleij, linux-spi; +Cc: Mark Brown, Fabio Estevam, linux-gpio
Hi Linus,
With the above mentionned commit, I get a crash on boot:
[ 3.146041] ------------[ cut here ]------------
[ 3.150340] kernel BUG at lib/genalloc.c:516!
[ 3.154649] Oops: Exception in kernel mode, sig: 5 [#1]
[ 3.159821] BE PAGE_SIZE=16K PREEMPT CMPC885
[ 3.164053] CPU: 0 PID: 1 Comm: swapper Not tainted
5.4.0-rc5-s3k-dev-00785-gab26ba2f2775 #2471
[ 3.172630] NIP: c0249054 LR: c0249054 CTR: 00000000
[ 3.177631] REGS: c60e1c50 TRAP: 0700 Not tainted
(5.4.0-rc5-s3k-dev-00785-gab26ba2f2775)
[ 3.185953] MSR: 00021032 <ME,IR,DR,RI> CR: 24002422 XER: 20000000
[ 3.192322]
[ 3.192322] GPR00: c0249054 c60e1d08 c60d4000 c600c200 00009d80
00000000 00000000 00000000
[ 3.192322] GPR08: 00000000 00000000 00009bbf 000affff 24000422
00000000 c0003890 00000000
[ 3.192322] GPR16: 00000000 00000000 00000000 00000000 00000000
00000000 c0800000 0000009e
[ 3.192322] GPR24: c0852778 00000000 00000000 00000000 00000000
c600c200 00009d80 c600c200
[ 3.227053] NIP [c0249054] gen_pool_free_owner+0xfc/0x100
[ 3.232377] LR [c0249054] gen_pool_free_owner+0xfc/0x100
[ 3.237588] Call Trace:
[ 3.240037] [c60e1d08] [c0249054] gen_pool_free_owner+0xfc/0x100
(unreliable)
[ 3.247113] [c60e1d38] [c0299020] cpm_muram_free+0x84/0xf4
[ 3.252517] [c60e1d58] [c030e7a4] fsl_spi_cpm_free+0x94/0x100
[ 3.258198] [c60e1d68] [c030f2d4] of_fsl_spi_probe+0x260/0x3a0
[ 3.263965] [c60e1db8] [c02c5458] platform_drv_probe+0x44/0xa4
[ 3.269754] [c60e1dc8] [c02c35fc] really_probe+0x1ac/0x418
[ 3.275164] [c60e1df8] [c02c407c] device_driver_attach+0x88/0x90
[ 3.281099] [c60e1e18] [c02c4124] __driver_attach+0xa0/0x154
[ 3.286691] [c60e1e38] [c02c165c] bus_for_each_dev+0x64/0xb4
[ 3.292284] [c60e1e68] [c02c2038] bus_add_driver+0xe0/0x218
[ 3.297783] [c60e1e88] [c02c48dc] driver_register+0x84/0x148
[ 3.303394] [c60e1e98] [c06d8d30] do_one_initcall+0x8c/0x1cc
[ 3.308978] [c60e1ef8] [c06d8fac] kernel_init_freeable+0x13c/0x1ec
[ 3.315087] [c60e1f28] [c00038a4] kernel_init+0x14/0x110
[ 3.320342] [c60e1f38] [c000e1cc] ret_from_kernel_thread+0x14/0x1c
[ 3.326401] Instruction dump:
[ 3.329337] 40a2fff4 4192000c 813f0010 913a0000 80010034 81810010
7c0803a6 bb210014
[ 3.336994] 7d808120 38210030 4be1d7a8 4be1d7a5 <0fe00000> 7c0802a6
9421ffe0 bf810010
[ 3.344864] ---[ end trace 2200b36c5d6384b0 ]---
[ 3.349380]
[ 4.320994] note: swapper[1] exited with preempt_count 1
[ 4.326187] Kernel panic - not syncing: Attempted to kill init!
exitcode=0x00000005
[ 4.333602] Rebooting in 180 seconds..
Reverting 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")
solves the issue.
Renaming the gpios item to cs-gpios in the devicetree fixes the crash on
boot but the following warning appears during boot and SPI doesn't work
(can't read board temperature on LM74 chip):
[ 3.145635] ------------[ cut here ]------------
[ 3.150048] WARNING: CPU: 0 PID: 1 at drivers/spi/spi-fsl-spi.c:716
fsl_spi_cs_control+0x64/0x7c
[ 3.158715] CPU: 0 PID: 1 Comm: swapper Not tainted
5.4.0-rc5-s3k-dev-00785-gab26ba2f2775 #2473
[ 3.167296] NIP: c030ee68 LR: c030ee3c CTR: c030ee04
[ 3.172295] REGS: c60e1bd0 TRAP: 0700 Not tainted
(5.4.0-rc5-s3k-dev-00785-gab26ba2f2775)
[ 3.180618] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 24004842 XER: 20000000
[ 3.187246]
[ 3.187246] GPR00: c030fbd4 c60e1c88 c60d4000 c6211c50 00000000
00000000 07de2900 c61f2210
[ 3.187246] GPR08: 00001208 c61f2210 00001200 000affff 24004848
00000000 c0003890 00000000
[ 3.187246] GPR16: 00000000 00000000 00000000 c7fdd478 c06aa8dc
c06b0000 c06aa7f8 c06aa804
[ 3.187246] GPR24: c06aa810 c06aa81c 00000000 c61f2210 c07b72fc
00000000 c6244a40 00000000
[ 3.221966] NIP [c030ee68] fsl_spi_cs_control+0x64/0x7c
[ 3.227126] LR [c030ee3c] fsl_spi_cs_control+0x38/0x7c
[ 3.232167] Call Trace:
[ 3.234616] [c60e1c98] [c030fbd4] fsl_spi_setup+0xc4/0x148
[ 3.240070] [c60e1cb8] [c030c45c] spi_setup+0xd0/0x1c4
[ 3.245129] [c60e1cd8] [c030c5f4] spi_add_device+0xa4/0x190
[ 3.250640] [c60e1cf8] [c030d0ec] spi_register_controller+0x75c/0xb50
[ 3.257009] [c60e1d48] [c030d520] devm_spi_register_controller+0x40/0x98
[ 3.263619] [c60e1d68] [c030f354] of_fsl_spi_probe+0x2e0/0x3a0
[ 3.269388] [c60e1db8] [c02c5458] platform_drv_probe+0x44/0xa4
[ 3.275178] [c60e1dc8] [c02c35fc] really_probe+0x1ac/0x418
[ 3.280587] [c60e1df8] [c02c407c] device_driver_attach+0x88/0x90
[ 3.286523] [c60e1e18] [c02c4124] __driver_attach+0xa0/0x154
[ 3.292115] [c60e1e38] [c02c165c] bus_for_each_dev+0x64/0xb4
[ 3.297708] [c60e1e68] [c02c2038] bus_add_driver+0xe0/0x218
[ 3.303205] [c60e1e88] [c02c48dc] driver_register+0x84/0x148
[ 3.308815] [c60e1e98] [c06d8d30] do_one_initcall+0x8c/0x1cc
[ 3.314400] [c60e1ef8] [c06d8fac] kernel_init_freeable+0x13c/0x1ec
[ 3.320508] [c60e1f28] [c00038a4] kernel_init+0x14/0x110
[ 3.325762] [c60e1f38] [c000e1cc] ret_from_kernel_thread+0x14/0x1c
[ 3.331824] Instruction dump:
[ 3.334759] 4bfffab1 80830018 2f840000 419e0024 80010014 215f0000
7c0803a6 83e1000c
[ 3.342416] 7c631910 54630000 38210010 4bcffa84 <0fe00000> 80010014
83e1000c 7c0803a6
[ 3.350277] ---[ end trace 15d6993a62672e78 ]---
Any idea ?
Thanks,
Christophe
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-08 12:33 Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") Christophe Leroy @ 2019-11-08 13:09 ` Linus Walleij 2019-11-08 13:34 ` Christophe Leroy 0 siblings, 1 reply; 26+ messages in thread From: Linus Walleij @ 2019-11-08 13:09 UTC (permalink / raw) To: Christophe Leroy Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM On Fri, Nov 8, 2019 at 1:33 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > [ 3.227053] NIP [c0249054] gen_pool_free_owner+0xfc/0x100 > [ 3.232377] LR [c0249054] gen_pool_free_owner+0xfc/0x100 > [ 3.237588] Call Trace: > [ 3.240037] [c60e1d08] [c0249054] gen_pool_free_owner+0xfc/0x100 > (unreliable) > [ 3.247113] [c60e1d38] [c0299020] cpm_muram_free+0x84/0xf4 > [ 3.252517] [c60e1d58] [c030e7a4] fsl_spi_cpm_free+0x94/0x100 > [ 3.258198] [c60e1d68] [c030f2d4] of_fsl_spi_probe+0x260/0x3a0 Since fsl_spi_cpm_free() is called from of_fsl_spi_probe() and doesn't even exist on the errorpath of this function I suppose it is some kind inlining of_fsl_spi_probe() which goes on the errorpath and crashes there. I suspect EPROBE_DEFER (-517) to be behind this but it is just a guess. What happens if you add this: diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c index 4b80ace1d137..a26b5e542006 100644 --- a/drivers/spi/spi-fsl-spi.c +++ b/drivers/spi/spi-fsl-spi.c @@ -678,6 +678,7 @@ static struct spi_master * fsl_spi_probe(struct device *dev, return master; err_probe: + dev_info(dev, "bail out with error code %d\n", ret); fsl_spi_cpm_free(mpc8xxx_spi); err_cpm_init: spi_master_put(master); I am assuming that the errorpath inside spi-fsl-cpm.c is crashing, as cpm_muram_free() is called under the wrong conditions, or simply not really working. So fixing the errorpath is a separate problem in itself. By just looking at the code and not understanding any more than that the error path on fsl_spi_cpm_free() should probably equal that of fsl_spi_cpm_init() I guess maybe this could fix it, could you test? diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c index 858f0544289e..54ad0ac121e5 100644 --- a/drivers/spi/spi-fsl-cpm.c +++ b/drivers/spi/spi-fsl-cpm.c @@ -392,7 +392,8 @@ void fsl_spi_cpm_free(struct mpc8xxx_spi *mspi) dma_unmap_single(dev, mspi->dma_dummy_rx, SPI_MRBLR, DMA_FROM_DEVICE); dma_unmap_single(dev, mspi->dma_dummy_tx, PAGE_SIZE, DMA_TO_DEVICE); cpm_muram_free(cpm_muram_offset(mspi->tx_bd)); - cpm_muram_free(cpm_muram_offset(mspi->pram)); + if (!(mspi->flags & SPI_CPM1)) + cpm_muram_free(cpm_muram_offset(mspi->pram)); fsl_spi_free_dummy_rx(); } EXPORT_SYMBOL_GPL(fsl_spi_cpm_free); > Reverting 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") > solves the issue. I hope we can fix the errorpath so we can see what the real problem is with this patch. > Renaming the gpios item to cs-gpios in the devicetree fixes the crash on > boot but the following warning appears during boot and SPI doesn't work > (can't read board temperature on LM74 chip): That's annoying. I hope we can get the code to work though, let's hammer on it a bit and see if we can fix it! Yours, Linus Walleij ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-08 13:09 ` Linus Walleij @ 2019-11-08 13:34 ` Christophe Leroy 2019-11-26 15:01 ` Christophe Leroy 2019-11-26 15:48 ` Linus Walleij 0 siblings, 2 replies; 26+ messages in thread From: Christophe Leroy @ 2019-11-08 13:34 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 08/11/2019 à 14:09, Linus Walleij a écrit : > On Fri, Nov 8, 2019 at 1:33 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > >> [ 3.227053] NIP [c0249054] gen_pool_free_owner+0xfc/0x100 >> [ 3.232377] LR [c0249054] gen_pool_free_owner+0xfc/0x100 >> [ 3.237588] Call Trace: >> [ 3.240037] [c60e1d08] [c0249054] gen_pool_free_owner+0xfc/0x100 >> (unreliable) >> [ 3.247113] [c60e1d38] [c0299020] cpm_muram_free+0x84/0xf4 >> [ 3.252517] [c60e1d58] [c030e7a4] fsl_spi_cpm_free+0x94/0x100 >> [ 3.258198] [c60e1d68] [c030f2d4] of_fsl_spi_probe+0x260/0x3a0 > > Since fsl_spi_cpm_free() is called from of_fsl_spi_probe() and doesn't > even exist on the errorpath of this function I suppose it is some kind > inlining of_fsl_spi_probe() which goes on the errorpath and > crashes there. I suspect EPROBE_DEFER (-517) to be behind this > but it is just a guess. > > What happens if you add this: > > diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c > index 4b80ace1d137..a26b5e542006 100644 > --- a/drivers/spi/spi-fsl-spi.c > +++ b/drivers/spi/spi-fsl-spi.c > @@ -678,6 +678,7 @@ static struct spi_master * fsl_spi_probe(struct device *dev, > return master; > > err_probe: > + dev_info(dev, "bail out with error code %d\n", ret); > fsl_spi_cpm_free(mpc8xxx_spi); > err_cpm_init: > spi_master_put(master); > > I am assuming that the errorpath inside spi-fsl-cpm.c is crashing, > as cpm_muram_free() is called under the wrong conditions, > or simply not really working. > > So fixing the errorpath is a separate problem in itself. > > By just looking at the code and not understanding any more than > that the error path on fsl_spi_cpm_free() should probably > equal that of fsl_spi_cpm_init() I guess maybe this could > fix it, could you test? > > diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c > index 858f0544289e..54ad0ac121e5 100644 > --- a/drivers/spi/spi-fsl-cpm.c > +++ b/drivers/spi/spi-fsl-cpm.c > @@ -392,7 +392,8 @@ void fsl_spi_cpm_free(struct mpc8xxx_spi *mspi) > dma_unmap_single(dev, mspi->dma_dummy_rx, SPI_MRBLR, DMA_FROM_DEVICE); > dma_unmap_single(dev, mspi->dma_dummy_tx, PAGE_SIZE, DMA_TO_DEVICE); > cpm_muram_free(cpm_muram_offset(mspi->tx_bd)); > - cpm_muram_free(cpm_muram_offset(mspi->pram)); > + if (!(mspi->flags & SPI_CPM1)) > + cpm_muram_free(cpm_muram_offset(mspi->pram)); > fsl_spi_free_dummy_rx(); > } > EXPORT_SYMBOL_GPL(fsl_spi_cpm_free); > >> Reverting 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") >> solves the issue. > > I hope we can fix the errorpath so we can see what the real problem > is with this patch. With the two above changes, I get: [ 3.143014] fsl_spi ff000a80.spi: bail out with error code -22 [ 3.148879] ------------[ cut here ]------------ [ 3.153261] remove_proc_entry: removing non-empty directory 'irq/43', leaking at least 'fsl_spi' [ 3.162473] WARNING: CPU: 0 PID: 1 at fs/proc/generic.c:684 remove_proc_entry+0x1a0/0x1c8 [ 3.170324] CPU: 0 PID: 1 Comm: swapper Not tainted 5.4.0-rc5-s3k-dev-00785-gab26ba2f2775-dirty #2475 [ 3.179420] NIP: c017f138 LR: c017f138 CTR: c0047d24 [ 3.184421] REGS: c60e1c20 TRAP: 0700 Not tainted (5.4.0-rc5-s3k-dev-00785-gab26ba2f2775-dirty) [ 3.193259] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 22008222 XER: 00000000 [ 3.199885] [ 3.199885] GPR00: c017f138 c60e1cd8 c60d4000 00000054 00000000 00000000 0000c350 00000010 [ 3.199885] GPR08: c0775030 00000800 00000000 00001032 22000408 00000000 c0003890 00000000 [ 3.199885] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 c0800000 0000009e [ 3.199885] GPR24: c0852778 c6211c50 c61f2210 c07793b8 00000001 c60d3863 c60d08e3 c60d3800 [ 3.234623] NIP [c017f138] remove_proc_entry+0x1a0/0x1c8 [ 3.239861] LR [c017f138] remove_proc_entry+0x1a0/0x1c8 [ 3.244979] Call Trace: [ 3.247435] [c60e1cd8] [c017f138] remove_proc_entry+0x1a0/0x1c8 (unreliable) [ 3.254425] [c60e1d08] [c005d904] unregister_irq_proc+0x5c/0x70 [ 3.260252] [c60e1d28] [c0055978] free_desc+0x4c/0x90 [ 3.265245] [c60e1d48] [c0055be0] irq_free_descs+0x70/0xa8 [ 3.270670] [c60e1d68] [c030f154] of_fsl_spi_probe+0xc0/0x3b4 [ 3.276349] [c60e1db8] [c02c5458] platform_drv_probe+0x44/0xa4 [ 3.282141] [c60e1dc8] [c02c35fc] really_probe+0x1ac/0x418 [ 3.287547] [c60e1df8] [c02c407c] device_driver_attach+0x88/0x90 [ 3.293484] [c60e1e18] [c02c4124] __driver_attach+0xa0/0x154 [ 3.299076] [c60e1e38] [c02c165c] bus_for_each_dev+0x64/0xb4 [ 3.304670] [c60e1e68] [c02c2038] bus_add_driver+0xe0/0x218 [ 3.310166] [c60e1e88] [c02c48dc] driver_register+0x84/0x148 [ 3.315776] [c60e1e98] [c06d8d30] do_one_initcall+0x8c/0x1cc [ 3.321362] [c60e1ef8] [c06d8fac] kernel_init_freeable+0x13c/0x1ec [ 3.327469] [c60e1f28] [c00038a4] kernel_init+0x14/0x110 [ 3.332726] [c60e1f38] [c000e1cc] ret_from_kernel_thread+0x14/0x1c [ 3.338784] Instruction dump: [ 3.341720] 2c030000 4182004c 3863ffb0 3c80c05a 80e3005c 388477d0 3c60c068 7fa6eb78 [ 3.349377] 7fc5f378 38840280 386333e4 4be9e7a1 <0fe00000> 4bffff7c 3c60c06c 7fc4f378 [ 3.357237] ---[ end trace 722649f68cd7c34e ]--- [ 3.362333] fsl_spi: probe of ff000a80.spi failed with error -22 Christophe > >> Renaming the gpios item to cs-gpios in the devicetree fixes the crash on >> boot but the following warning appears during boot and SPI doesn't work >> (can't read board temperature on LM74 chip): > > That's annoying. I hope we can get the code to work though, > let's hammer on it a bit and see if we can fix it! > > Yours, > Linus Walleij > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-08 13:34 ` Christophe Leroy @ 2019-11-26 15:01 ` Christophe Leroy 2019-11-26 15:35 ` Fabio Estevam 2019-11-26 15:48 ` Linus Walleij 1 sibling, 1 reply; 26+ messages in thread From: Christophe Leroy @ 2019-11-26 15:01 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Hi Linus, Le 08/11/2019 à 14:34, Christophe Leroy a écrit : > > > Le 08/11/2019 à 14:09, Linus Walleij a écrit : >> On Fri, Nov 8, 2019 at 1:33 PM Christophe Leroy >> <christophe.leroy@c-s.fr> wrote: >> >>> [ 3.227053] NIP [c0249054] gen_pool_free_owner+0xfc/0x100 >>> [ 3.232377] LR [c0249054] gen_pool_free_owner+0xfc/0x100 >>> [ 3.237588] Call Trace: >>> [ 3.240037] [c60e1d08] [c0249054] gen_pool_free_owner+0xfc/0x100 >>> (unreliable) >>> [ 3.247113] [c60e1d38] [c0299020] cpm_muram_free+0x84/0xf4 >>> [ 3.252517] [c60e1d58] [c030e7a4] fsl_spi_cpm_free+0x94/0x100 >>> [ 3.258198] [c60e1d68] [c030f2d4] of_fsl_spi_probe+0x260/0x3a0 >> >> Since fsl_spi_cpm_free() is called from of_fsl_spi_probe() and doesn't >> even exist on the errorpath of this function I suppose it is some kind >> inlining of_fsl_spi_probe() which goes on the errorpath and >> crashes there. I suspect EPROBE_DEFER (-517) to be behind this >> but it is just a guess. >> >> What happens if you add this: >> >> diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c >> index 4b80ace1d137..a26b5e542006 100644 >> --- a/drivers/spi/spi-fsl-spi.c >> +++ b/drivers/spi/spi-fsl-spi.c >> @@ -678,6 +678,7 @@ static struct spi_master * fsl_spi_probe(struct >> device *dev, >> return master; >> >> err_probe: >> + dev_info(dev, "bail out with error code %d\n", ret); >> fsl_spi_cpm_free(mpc8xxx_spi); >> err_cpm_init: >> spi_master_put(master); >> >> I am assuming that the errorpath inside spi-fsl-cpm.c is crashing, >> as cpm_muram_free() is called under the wrong conditions, >> or simply not really working. >> >> So fixing the errorpath is a separate problem in itself. >> >> By just looking at the code and not understanding any more than >> that the error path on fsl_spi_cpm_free() should probably >> equal that of fsl_spi_cpm_init() I guess maybe this could >> fix it, could you test? >> >> diff --git a/drivers/spi/spi-fsl-cpm.c b/drivers/spi/spi-fsl-cpm.c >> index 858f0544289e..54ad0ac121e5 100644 >> --- a/drivers/spi/spi-fsl-cpm.c >> +++ b/drivers/spi/spi-fsl-cpm.c >> @@ -392,7 +392,8 @@ void fsl_spi_cpm_free(struct mpc8xxx_spi *mspi) >> dma_unmap_single(dev, mspi->dma_dummy_rx, SPI_MRBLR, >> DMA_FROM_DEVICE); >> dma_unmap_single(dev, mspi->dma_dummy_tx, PAGE_SIZE, >> DMA_TO_DEVICE); >> cpm_muram_free(cpm_muram_offset(mspi->tx_bd)); >> - cpm_muram_free(cpm_muram_offset(mspi->pram)); >> + if (!(mspi->flags & SPI_CPM1)) >> + cpm_muram_free(cpm_muram_offset(mspi->pram)); >> fsl_spi_free_dummy_rx(); >> } >> EXPORT_SYMBOL_GPL(fsl_spi_cpm_free); >> >>> Reverting 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") >>> solves the issue. >> >> I hope we can fix the errorpath so we can see what the real problem >> is with this patch. > > With the two above changes, I get: > [ 3.143014] fsl_spi ff000a80.spi: bail out with error code -22 > [ 3.148879] ------------[ cut here ]------------ > [ 3.153261] remove_proc_entry: removing non-empty directory 'irq/43', > leaking at least 'fsl_spi' > [ 3.162473] WARNING: CPU: 0 PID: 1 at fs/proc/generic.c:684 > remove_proc_entry+0x1a0/0x1c8 > [ 3.170324] CPU: 0 PID: 1 Comm: swapper Not tainted > 5.4.0-rc5-s3k-dev-00785-gab26ba2f2775-dirty #2475 > [ 3.179420] NIP: c017f138 LR: c017f138 CTR: c0047d24 > [ 3.184421] REGS: c60e1c20 TRAP: 0700 Not tainted > (5.4.0-rc5-s3k-dev-00785-gab26ba2f2775-dirty) > [ 3.193259] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 22008222 XER: 00000000 > [ 3.199885] > [ 3.199885] GPR00: c017f138 c60e1cd8 c60d4000 00000054 00000000 > 00000000 0000c350 00000010 > [ 3.199885] GPR08: c0775030 00000800 00000000 00001032 22000408 > 00000000 c0003890 00000000 > [ 3.199885] GPR16: 00000000 00000000 00000000 00000000 00000000 > 00000000 c0800000 0000009e > [ 3.199885] GPR24: c0852778 c6211c50 c61f2210 c07793b8 00000001 > c60d3863 c60d08e3 c60d3800 > [ 3.234623] NIP [c017f138] remove_proc_entry+0x1a0/0x1c8 > [ 3.239861] LR [c017f138] remove_proc_entry+0x1a0/0x1c8 > [ 3.244979] Call Trace: > [ 3.247435] [c60e1cd8] [c017f138] remove_proc_entry+0x1a0/0x1c8 > (unreliable) > [ 3.254425] [c60e1d08] [c005d904] unregister_irq_proc+0x5c/0x70 > [ 3.260252] [c60e1d28] [c0055978] free_desc+0x4c/0x90 > [ 3.265245] [c60e1d48] [c0055be0] irq_free_descs+0x70/0xa8 > [ 3.270670] [c60e1d68] [c030f154] of_fsl_spi_probe+0xc0/0x3b4 > [ 3.276349] [c60e1db8] [c02c5458] platform_drv_probe+0x44/0xa4 > [ 3.282141] [c60e1dc8] [c02c35fc] really_probe+0x1ac/0x418 > [ 3.287547] [c60e1df8] [c02c407c] device_driver_attach+0x88/0x90 > [ 3.293484] [c60e1e18] [c02c4124] __driver_attach+0xa0/0x154 > [ 3.299076] [c60e1e38] [c02c165c] bus_for_each_dev+0x64/0xb4 > [ 3.304670] [c60e1e68] [c02c2038] bus_add_driver+0xe0/0x218 > [ 3.310166] [c60e1e88] [c02c48dc] driver_register+0x84/0x148 > [ 3.315776] [c60e1e98] [c06d8d30] do_one_initcall+0x8c/0x1cc > [ 3.321362] [c60e1ef8] [c06d8fac] kernel_init_freeable+0x13c/0x1ec > [ 3.327469] [c60e1f28] [c00038a4] kernel_init+0x14/0x110 > [ 3.332726] [c60e1f38] [c000e1cc] ret_from_kernel_thread+0x14/0x1c > [ 3.338784] Instruction dump: > [ 3.341720] 2c030000 4182004c 3863ffb0 3c80c05a 80e3005c 388477d0 > 3c60c068 7fa6eb78 > [ 3.349377] 7fc5f378 38840280 386333e4 4be9e7a1 <0fe00000> 4bffff7c > 3c60c06c 7fc4f378 > [ 3.357237] ---[ end trace 722649f68cd7c34e ]--- > [ 3.362333] fsl_spi: probe of ff000a80.spi failed with error -22 > > How can we progress on that ? Problem is still present in 5.4 Christophe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-26 15:01 ` Christophe Leroy @ 2019-11-26 15:35 ` Fabio Estevam 2019-11-26 16:16 ` Christophe Leroy 2019-11-26 16:23 ` Mark Brown 0 siblings, 2 replies; 26+ messages in thread From: Fabio Estevam @ 2019-11-26 15:35 UTC (permalink / raw) To: Christophe Leroy Cc: Linus Walleij, linux-spi, Mark Brown, open list:GPIO SUBSYSTEM Hi Christophe, On Tue, Nov 26, 2019 at 12:01 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > How can we progress on that ? Problem is still present in 5.4 Linus has sent the following fix: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20191126&id=c5923243eb3208ea63b5ed7905610039c4ca5201 Does this fix the problem? If it does I think this need to get into stable too. ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-26 15:35 ` Fabio Estevam @ 2019-11-26 16:16 ` Christophe Leroy 2019-11-26 16:23 ` Mark Brown 1 sibling, 0 replies; 26+ messages in thread From: Christophe Leroy @ 2019-11-26 16:16 UTC (permalink / raw) To: Fabio Estevam Cc: Linus Walleij, linux-spi, Mark Brown, open list:GPIO SUBSYSTEM Le 26/11/2019 à 16:35, Fabio Estevam a écrit : > Hi Christophe, > > On Tue, Nov 26, 2019 at 12:01 PM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: > >> How can we progress on that ? Problem is still present in 5.4 > > Linus has sent the following fix: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20191126&id=c5923243eb3208ea63b5ed7905610039c4ca5201 > > Does this fix the problem? This fixes the crash in the error path. But the error path shouldn't be taken, so there is something else to look at. > > If it does I think this need to get into stable too. > Agreed. Christophe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-26 15:35 ` Fabio Estevam 2019-11-26 16:16 ` Christophe Leroy @ 2019-11-26 16:23 ` Mark Brown 1 sibling, 0 replies; 26+ messages in thread From: Mark Brown @ 2019-11-26 16:23 UTC (permalink / raw) To: Fabio Estevam Cc: Christophe Leroy, Linus Walleij, linux-spi, open list:GPIO SUBSYSTEM [-- Attachment #1: Type: text/plain, Size: 623 bytes --] On Tue, Nov 26, 2019 at 12:35:35PM -0300, Fabio Estevam wrote: > On Tue, Nov 26, 2019 at 12:01 PM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: > > How can we progress on that ? Problem is still present in 5.4 > Linus has sent the following fix: > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?h=next-20191126&id=c5923243eb3208ea63b5ed7905610039c4ca5201 > Does this fix the problem? > If it does I think this need to get into stable too. Ah, I hadn't registered that that fix was for something already in Linus' tree rather than new development otherwise I'd have sent it as a fix. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-08 13:34 ` Christophe Leroy 2019-11-26 15:01 ` Christophe Leroy @ 2019-11-26 15:48 ` Linus Walleij 2019-11-26 19:08 ` Christophe Leroy 1 sibling, 1 reply; 26+ messages in thread From: Linus Walleij @ 2019-11-26 15:48 UTC (permalink / raw) To: Christophe Leroy Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Hi Christophe, sorry for forgetting about this :( On Fri, Nov 8, 2019 at 2:34 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > With the two above changes, I get: > [ 3.143014] fsl_spi ff000a80.spi: bail out with error code -22 > [ 3.148879] ------------[ cut here ]------------ > [ 3.153261] remove_proc_entry: removing non-empty directory 'irq/43', > leaking at least 'fsl_spi' > [ 3.162473] WARNING: CPU: 0 PID: 1 at fs/proc/generic.c:684 > remove_proc_entry+0x1a0/0x1c8 So that is another bug again. (Tearing down IRQs is erroneous in some way.) The problem is the first -22 (-EINVAL) Which comes from of_fsl_spi_probe() IIUC. Can you try the following? I'm sorry if gmail mangles the patches, I put a copy here: https://dflund.se/~triad/fsldebug.diff diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c index 114801a32371..3f68bea1c3c0 100644 --- a/drivers/spi/spi-fsl-spi.c +++ b/drivers/spi/spi-fsl-spi.c @@ -617,12 +617,15 @@ static struct spi_master * fsl_spi_probe(struct device *dev, mpc8xxx_spi->type = fsl_spi_get_type(dev); ret = fsl_spi_cpm_init(mpc8xxx_spi); - if (ret) + if (ret) { + dev_err(dev, "fsl_spi_cpm_init() failed\n"); goto err_cpm_init; + } mpc8xxx_spi->reg_base = devm_ioremap_resource(dev, mem); if (IS_ERR(mpc8xxx_spi->reg_base)) { ret = PTR_ERR(mpc8xxx_spi->reg_base); + dev_err(dev, "erroneous ->reg_base\n"); goto err_probe; } @@ -645,8 +648,10 @@ static struct spi_master * fsl_spi_probe(struct device *dev, ret = devm_request_irq(dev, mpc8xxx_spi->irq, fsl_spi_irq, 0, "fsl_spi", mpc8xxx_spi); - if (ret != 0) + if (ret != 0) { + dev_err(dev, "devm_request_irq() failed\n"); goto err_probe; + } reg_base = mpc8xxx_spi->reg_base; @@ -668,8 +673,10 @@ static struct spi_master * fsl_spi_probe(struct device *dev, mpc8xxx_spi_write_reg(®_base->mode, regval); ret = devm_spi_register_master(dev, master); - if (ret < 0) + if (ret < 0) { + dev_err(dev, "devm_spi_register_master() failed\n"); goto err_probe; + } dev_info(dev, "at 0x%p (irq = %d), %s mode\n", reg_base, mpc8xxx_spi->irq, mpc8xxx_spi_strmode(mpc8xxx_spi->flags)); @@ -681,6 +688,7 @@ static struct spi_master * fsl_spi_probe(struct device *dev, err_cpm_init: spi_master_put(master); err: + dev_err(dev, "exiting fsl_spi_probe() with error\n"); return ERR_PTR(ret); } @@ -738,12 +746,14 @@ static int of_fsl_spi_probe(struct platform_device *ofdev) irq = irq_of_parse_and_map(np, 0); if (!irq) { ret = -EINVAL; + dev_err(dev, "irq_of_parse_and_map() failed\n"); goto err; } master = fsl_spi_probe(dev, &mem, irq); if (IS_ERR(master)) { ret = PTR_ERR(master); + dev_err(dev, "fsl_spi_probe() failed\n"); goto err; } Let's drill in and see where this error come from. Yours, Linus Walleij ^ permalink raw reply related [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-26 15:48 ` Linus Walleij @ 2019-11-26 19:08 ` Christophe Leroy 2019-11-26 19:14 ` Christophe Leroy 0 siblings, 1 reply; 26+ messages in thread From: Christophe Leroy @ 2019-11-26 19:08 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 26/11/2019 à 16:48, Linus Walleij a écrit : > Hi Christophe, > > sorry for forgetting about this :( > > On Fri, Nov 8, 2019 at 2:34 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > >> With the two above changes, I get: >> [ 3.143014] fsl_spi ff000a80.spi: bail out with error code -22 >> [ 3.148879] ------------[ cut here ]------------ >> [ 3.153261] remove_proc_entry: removing non-empty directory 'irq/43', >> leaking at least 'fsl_spi' >> [ 3.162473] WARNING: CPU: 0 PID: 1 at fs/proc/generic.c:684 >> remove_proc_entry+0x1a0/0x1c8 > > So that is another bug again. (Tearing down IRQs is erroneous > in some way.) > > The problem is the first -22 (-EINVAL) > > Which comes from of_fsl_spi_probe() IIUC. > > Can you try the following? I'm sorry if gmail mangles the > patches, I put a copy here: > https://dflund.se/~triad/fsldebug.diff I get the following [ 3.159953] fsl_spi ff000a80.spi: devm_spi_register_master() failed [ 3.166154] fsl_spi ff000a80.spi: exiting fsl_spi_probe() with error [ 3.172194] fsl_spi ff000a80.spi: fsl_spi_probe() failed [ 3.177670] ------------[ cut here ]------------ [ 3.181991] remove_proc_entry: removing non-empty directory 'irq/43', leaking at least 'fsl_spi' [ 3.191209] WARNING: CPU: 0 PID: 1 at fs/proc/generic.c:684 remove_proc_entry+0x1a0/0x1c8 [ 3.199139] CPU: 0 PID: 1 Comm: swapper Not tainted 5.4.0-s3k-dev-00898-gd57c00e2472d-dirty #2499 [ 3.207890] NIP: c017eb74 LR: c017eb74 CTR: c0047584 [ 3.212892] REGS: c60e1c20 TRAP: 0700 Not tainted (5.4.0-s3k-dev-00898-gd57c00e2472d-dirty) [ 3.221385] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 22008222 XER: 00000000 [ 3.228011] [ 3.228011] GPR00: c017eb74 c60e1cd8 c60d4000 00000054 00000000 00000000 0000cf08 00000010 [ 3.228011] GPR08: c0779030 00000800 00000000 00001032 22000408 00000000 c0003890 00000000 [ 3.228011] GPR16: 00000000 00000000 00000000 00000000 00000000 00000000 c0800000 0000009e [ 3.228011] GPR24: c0856778 c6211c50 ffffffea c077d3b8 00000001 c60d3863 c60d08e3 c60d3800 [ 3.262659] NIP [c017eb74] remove_proc_entry+0x1a0/0x1c8 [ 3.267905] LR [c017eb74] remove_proc_entry+0x1a0/0x1c8 [ 3.273021] Call Trace: [ 3.275478] [c60e1cd8] [c017eb74] remove_proc_entry+0x1a0/0x1c8 (unreliable) [ 3.282466] [c60e1d08] [c005d17c] unregister_irq_proc+0x5c/0x70 [ 3.288292] [c60e1d28] [c00551f0] free_desc+0x4c/0x90 [ 3.293285] [c60e1d48] [c0055458] irq_free_descs+0x70/0xa8 [ 3.298709] [c60e1d68] [c030eb90] of_fsl_spi_probe+0xc0/0x418 [ 3.304388] [c60e1db8] [c02c4e94] platform_drv_probe+0x44/0xa4 [ 3.310181] [c60e1dc8] [c02c3038] really_probe+0x1ac/0x418 [ 3.315588] [c60e1df8] [c02c3ab8] device_driver_attach+0x88/0x90 [ 3.321524] [c60e1e18] [c02c3b60] __driver_attach+0xa0/0x154 [ 3.327115] [c60e1e38] [c02c1098] bus_for_each_dev+0x64/0xb4 [ 3.332709] [c60e1e68] [c02c1a74] bus_add_driver+0xe0/0x218 [ 3.338222] [c60e1e88] [c02c4318] driver_register+0x84/0x148 [ 3.343815] [c60e1e98] [c06d8d30] do_one_initcall+0x8c/0x1cc [ 3.349401] [c60e1ef8] [c06d8fac] kernel_init_freeable+0x13c/0x1ec [ 3.355509] [c60e1f28] [c00038a4] kernel_init+0x14/0x110 [ 3.360763] [c60e1f38] [c000e1cc] ret_from_kernel_thread+0x14/0x1c [ 3.366825] Instruction dump: [ 3.369760] 2c030000 4182004c 3863ffb0 3c80c05a 80e3005c 388477d0 3c60c068 7fa6eb78 [ 3.377417] 7fc5f378 38840280 386336d4 4be9e5a9 <0fe00000> 4bffff7c 3c60c06c 7fc4f378 [ 3.385277] ---[ end trace 25f4e57540579d18 ]--- [ 3.390369] fsl_spi: probe of ff000a80.spi failed with error -22 Digging a bit further, I see that devm_spi_register_master() fails in spi_register_controler() because ctlr->num_chipselect is 0 Christophe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-26 19:08 ` Christophe Leroy @ 2019-11-26 19:14 ` Christophe Leroy 2019-11-27 8:26 ` Linus Walleij 0 siblings, 1 reply; 26+ messages in thread From: Christophe Leroy @ 2019-11-26 19:14 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 26/11/2019 à 20:08, Christophe Leroy a écrit : > > > Le 26/11/2019 à 16:48, Linus Walleij a écrit : >> Hi Christophe, >> >> sorry for forgetting about this :( >> >> On Fri, Nov 8, 2019 at 2:34 PM Christophe Leroy >> <christophe.leroy@c-s.fr> wrote: >> >>> With the two above changes, I get: >>> [ 3.143014] fsl_spi ff000a80.spi: bail out with error code -22 >>> [ 3.148879] ------------[ cut here ]------------ >>> [ 3.153261] remove_proc_entry: removing non-empty directory 'irq/43', >>> leaking at least 'fsl_spi' >>> [ 3.162473] WARNING: CPU: 0 PID: 1 at fs/proc/generic.c:684 >>> remove_proc_entry+0x1a0/0x1c8 >> >> So that is another bug again. (Tearing down IRQs is erroneous >> in some way.) >> >> The problem is the first -22 (-EINVAL) >> >> Which comes from of_fsl_spi_probe() IIUC. >> >> Can you try the following? I'm sorry if gmail mangles the >> patches, I put a copy here: >> https://dflund.se/~triad/fsldebug.diff > > I get the following > > [ 3.159953] fsl_spi ff000a80.spi: devm_spi_register_master() failed > [ 3.166154] fsl_spi ff000a80.spi: exiting fsl_spi_probe() with error > [ 3.172194] fsl_spi ff000a80.spi: fsl_spi_probe() failed > [ 3.177670] ------------[ cut here ]------------ > [ 3.181991] remove_proc_entry: removing non-empty directory 'irq/43', > leaking at least 'fsl_spi' > [ 3.191209] WARNING: CPU: 0 PID: 1 at fs/proc/generic.c:684 > remove_proc_entry+0x1a0/0x1c8 > [ 3.199139] CPU: 0 PID: 1 Comm: swapper Not tainted > 5.4.0-s3k-dev-00898-gd57c00e2472d-dirty #2499 > [ 3.207890] NIP: c017eb74 LR: c017eb74 CTR: c0047584 > [ 3.212892] REGS: c60e1c20 TRAP: 0700 Not tainted > (5.4.0-s3k-dev-00898-gd57c00e2472d-dirty) > [ 3.221385] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 22008222 XER: 00000000 > [ 3.228011] > [ 3.228011] GPR00: c017eb74 c60e1cd8 c60d4000 00000054 00000000 > 00000000 0000cf08 00000010 > [ 3.228011] GPR08: c0779030 00000800 00000000 00001032 22000408 > 00000000 c0003890 00000000 > [ 3.228011] GPR16: 00000000 00000000 00000000 00000000 00000000 > 00000000 c0800000 0000009e > [ 3.228011] GPR24: c0856778 c6211c50 ffffffea c077d3b8 00000001 > c60d3863 c60d08e3 c60d3800 > [ 3.262659] NIP [c017eb74] remove_proc_entry+0x1a0/0x1c8 > [ 3.267905] LR [c017eb74] remove_proc_entry+0x1a0/0x1c8 > [ 3.273021] Call Trace: > [ 3.275478] [c60e1cd8] [c017eb74] remove_proc_entry+0x1a0/0x1c8 > (unreliable) > [ 3.282466] [c60e1d08] [c005d17c] unregister_irq_proc+0x5c/0x70 > [ 3.288292] [c60e1d28] [c00551f0] free_desc+0x4c/0x90 > [ 3.293285] [c60e1d48] [c0055458] irq_free_descs+0x70/0xa8 > [ 3.298709] [c60e1d68] [c030eb90] of_fsl_spi_probe+0xc0/0x418 > [ 3.304388] [c60e1db8] [c02c4e94] platform_drv_probe+0x44/0xa4 > [ 3.310181] [c60e1dc8] [c02c3038] really_probe+0x1ac/0x418 > [ 3.315588] [c60e1df8] [c02c3ab8] device_driver_attach+0x88/0x90 > [ 3.321524] [c60e1e18] [c02c3b60] __driver_attach+0xa0/0x154 > [ 3.327115] [c60e1e38] [c02c1098] bus_for_each_dev+0x64/0xb4 > [ 3.332709] [c60e1e68] [c02c1a74] bus_add_driver+0xe0/0x218 > [ 3.338222] [c60e1e88] [c02c4318] driver_register+0x84/0x148 > [ 3.343815] [c60e1e98] [c06d8d30] do_one_initcall+0x8c/0x1cc > [ 3.349401] [c60e1ef8] [c06d8fac] kernel_init_freeable+0x13c/0x1ec > [ 3.355509] [c60e1f28] [c00038a4] kernel_init+0x14/0x110 > [ 3.360763] [c60e1f38] [c000e1cc] ret_from_kernel_thread+0x14/0x1c > [ 3.366825] Instruction dump: > [ 3.369760] 2c030000 4182004c 3863ffb0 3c80c05a 80e3005c 388477d0 > 3c60c068 7fa6eb78 > [ 3.377417] 7fc5f378 38840280 386336d4 4be9e5a9 <0fe00000> 4bffff7c > 3c60c06c 7fc4f378 > [ 3.385277] ---[ end trace 25f4e57540579d18 ]--- > [ 3.390369] fsl_spi: probe of ff000a80.spi failed with error -22 > > Digging a bit further, I see that devm_spi_register_master() fails in > spi_register_controler() because ctlr->num_chipselect is 0 Note that I still have the following definition in the device tree: gpios = <&CPM1_PIO_C 4 1 /* SICOFI 1 */ &CPM1_PIO_B 23 1 /* TEMP MCR */ &CPM1_PIO_C 8 1 /* SICOFI 2 */ &CPM1_PIO_C 12 1 /* EEPROM MIAE */ &CPM1_PIO_D 6 1 /* SICOFI 3 */ &CPM1_PIO_B 14 1 /* TEMP MPC885 */ &CPM1_PIO_B 21 1 /* EEPROM CMPC885 */ &FAV_CS_SPI 0 1 /* FAV SPI */ &FAV_CS_SPI 2 1>; /* FAV POSTE FPGA */ I understood from the commit log that it's not needed to change it to cs-gpios. Christophe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-26 19:14 ` Christophe Leroy @ 2019-11-27 8:26 ` Linus Walleij 2019-11-27 8:57 ` Christophe Leroy 2019-11-27 9:07 ` Christophe Leroy 0 siblings, 2 replies; 26+ messages in thread From: Linus Walleij @ 2019-11-27 8:26 UTC (permalink / raw) To: Christophe Leroy Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM On Tue, Nov 26, 2019 at 8:14 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > > Digging a bit further, I see that devm_spi_register_master() fails in > > spi_register_controler() because ctlr->num_chipselect is 0 Aha, I see what the problem is I think. The old code for mpc8xxx had this: ngpios = of_gpio_count(np); ngpios = max(ngpios, 0); if (ngpios == 0 && !spisel_boot) { /* * SPI w/o chip-select line. One SPI device is still permitted * though. */ pdata->max_chipselect = 1; return 0; } (...) master->num_chipselect = pdata->max_chipselect; But the new code in the core has this: nb = gpiod_count(dev, "cs"); ctlr->num_chipselect = max_t(int, nb, ctlr->num_chipselect); So it relied on inspecting the device tree and set this to 1 if it didn't find anything. I will send a patch to test! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 8:26 ` Linus Walleij @ 2019-11-27 8:57 ` Christophe Leroy 2019-11-27 9:07 ` Christophe Leroy 1 sibling, 0 replies; 26+ messages in thread From: Christophe Leroy @ 2019-11-27 8:57 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 27/11/2019 à 09:26, Linus Walleij a écrit : > On Tue, Nov 26, 2019 at 8:14 PM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: > >>> Digging a bit further, I see that devm_spi_register_master() fails in >>> spi_register_controler() because ctlr->num_chipselect is 0 > > Aha, I see what the problem is I think. The old code for mpc8xxx had this: > > ngpios = of_gpio_count(np); > ngpios = max(ngpios, 0); > if (ngpios == 0 && !spisel_boot) { > /* > * SPI w/o chip-select line. One SPI device is still permitted > * though. > */ > pdata->max_chipselect = 1; > return 0; > } > (...) > master->num_chipselect = pdata->max_chipselect; > > But the new code in the core has this: > > nb = gpiod_count(dev, "cs"); > ctlr->num_chipselect = max_t(int, nb, ctlr->num_chipselect); > > So it relied on inspecting the device tree and set this to 1 > if it didn't find anything. > > I will send a patch to test! > I don't think that's the problem. In my device tree I have several gpios defined for the node: spi: spi@a80 { #address-cells = <1>; #size-cells = <0>; cell-index = <0>; compatible = "fsl,spi", "fsl,cpm1-spi"; reg = <0xa80 0x30 0x3d80 0x30>; interrupts = <5>; interrupt-parent = <&CPM_PIC>; mode = "cpu"; gpios = <&CPM1_PIO_C 4 1 /* SICOFI 1 */ &CPM1_PIO_B 23 1 /* TEMP MCR */ &CPM1_PIO_C 8 1 /* SICOFI 2 */ &CPM1_PIO_C 12 1 /* EEPROM MIAE */ &CPM1_PIO_D 6 1 /* SICOFI 3 */ &CPM1_PIO_B 14 1 /* TEMP MPC885 */ &CPM1_PIO_B 21 1 /* EEPROM CMPC885 */ &FAV_CS_SPI 0 1 /* FAV SPI */ &FAV_CS_SPI 2 1>; /* FAV POSTE FPGA */ }; sicofi@0 { compatible = "infineon,sicofi"; spi-max-frequency = <1000000>; reg = <0>; spi-cs-high; spi-cpha; }; lm74@1 { compatible = "ns,lm74"; spi-max-frequency = <1000000>; reg = <1>; spi-cs-high; }; sicofi@2 { compatible = "infineon,sicofi"; spi-max-frequency = <1000000>; reg = <2>; spi-cs-high; spi-cpha; }; eeprom@3 { compatible = "atmel,at25"; spi-max-frequency = <1000000>; reg = <3>; spi-cs-high; at25,byte-len = <1024>; at25,addr-mode = <2>; at25,page-size = <32>; }; sicofi@4 { compatible = "infineon,sicofi"; spi-max-frequency = <1000000>; reg = <4>; spi-cs-high; spi-cpha; }; lm74@5 { compatible = "ns,lm74"; spi-max-frequency = <1000000>; reg = <5>; spi-cs-high; }; eeprom@6 { compatible = "atmel,at25"; spi-max-frequency = <1000000>; reg = <6>; spi-cs-high; at25,byte-len = <1024>; at25,addr-mode = <2>; at25,page-size = <32>; }; iio: csfav@7 { compatible = "iio,ad7923"; spi-max-frequency = <1000000>; reg = <7>; spi-cs-high; spi-cpol; #io-channel-cells = <1>; }; csfavfpga@8 { compatible = "cs,fpga-poste"; spi-max-frequency = <1000000>; reg = <8>; spi-cs-high; }; }; Christophe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 8:26 ` Linus Walleij 2019-11-27 8:57 ` Christophe Leroy @ 2019-11-27 9:07 ` Christophe Leroy 2019-11-27 9:11 ` Linus Walleij 1 sibling, 1 reply; 26+ messages in thread From: Christophe Leroy @ 2019-11-27 9:07 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 27/11/2019 à 09:26, Linus Walleij a écrit : > On Tue, Nov 26, 2019 at 8:14 PM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: > >>> Digging a bit further, I see that devm_spi_register_master() fails in >>> spi_register_controler() because ctlr->num_chipselect is 0 > > Aha, I see what the problem is I think. The old code for mpc8xxx had this: > > ngpios = of_gpio_count(np); > ngpios = max(ngpios, 0); > if (ngpios == 0 && !spisel_boot) { > /* > * SPI w/o chip-select line. One SPI device is still permitted > * though. > */ > pdata->max_chipselect = 1; > return 0; > } > (...) > master->num_chipselect = pdata->max_chipselect; I confirm it can't be that .... here I get ngpios = 9 > > But the new code in the core has this: > > nb = gpiod_count(dev, "cs"); However the above is likely the issue. The property in the DTS is 'gpios' and not 'cs-gpios'. According to commit e3023bf80639 ("gpio: of: Handle the Freescale SPI CS"), it shouldn't be needed to rename it, and that's also what I understand from commit log of 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") > ctlr->num_chipselect = max_t(int, nb, ctlr->num_chipselect); > > So it relied on inspecting the device tree and set this to 1 > if it didn't find anything. But it should find something. Christophe > > I will send a patch to test! Is it worth testing your new patch ? Christophe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 9:07 ` Christophe Leroy @ 2019-11-27 9:11 ` Linus Walleij 2019-11-27 9:34 ` Christophe Leroy 0 siblings, 1 reply; 26+ messages in thread From: Linus Walleij @ 2019-11-27 9:11 UTC (permalink / raw) To: Christophe Leroy Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM On Wed, Nov 27, 2019 at 10:07 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > Le 27/11/2019 à 09:26, Linus Walleij a écrit : > > master->num_chipselect = pdata->max_chipselect; > > I confirm it can't be that .... here I get ngpios = 9 Ah yeah you're right. > > But the new code in the core has this: > > > > nb = gpiod_count(dev, "cs"); > > However the above is likely the issue. The property in the DTS is > 'gpios' and not 'cs-gpios'. According to commit e3023bf80639 ("gpio: of: > Handle the Freescale SPI CS"), it shouldn't be needed to rename it, and > that's also what I understand from commit log of 0f0581b24bd0 ("spi: > fsl: Convert to use CS GPIO descriptors") Yeah I see it now, the gpio_get* does use the extra quirks to find a node just named "gpios" but this gpiod_count doesn't work because that does not loop through the same quirk. I will think of something and send a better patch! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 9:11 ` Linus Walleij @ 2019-11-27 9:34 ` Christophe Leroy 2019-11-27 10:02 ` Linus Walleij 0 siblings, 1 reply; 26+ messages in thread From: Christophe Leroy @ 2019-11-27 9:34 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 27/11/2019 à 10:11, Linus Walleij a écrit : > On Wed, Nov 27, 2019 at 10:07 AM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: >> Le 27/11/2019 à 09:26, Linus Walleij a écrit : > >>> master->num_chipselect = pdata->max_chipselect; >> >> I confirm it can't be that .... here I get ngpios = 9 > > Ah yeah you're right. > >>> But the new code in the core has this: >>> >>> nb = gpiod_count(dev, "cs"); >> >> However the above is likely the issue. The property in the DTS is >> 'gpios' and not 'cs-gpios'. According to commit e3023bf80639 ("gpio: of: >> Handle the Freescale SPI CS"), it shouldn't be needed to rename it, and >> that's also what I understand from commit log of 0f0581b24bd0 ("spi: >> fsl: Convert to use CS GPIO descriptors") > > Yeah I see it now, the gpio_get* does use the extra quirks to find > a node just named "gpios" but this gpiod_count doesn't work > because that does not loop through the same quirk. In the meantime, I have tried changing "gpios" by "cs-gpios" in the device tree, and I get the following warning: [ 3.152279] ------------[ cut here ]------------ [ 3.156654] WARNING: CPU: 0 PID: 1 at drivers/spi/spi-fsl-spi.c:716 fsl_spi_cs_control+0x64/0x7c [ 3.165320] CPU: 0 PID: 1 Comm: swapper Not tainted 5.4.0-s3k-dev-00898-gd57c00e2472d #2507 [ 3.173557] NIP: c030e8c4 LR: c030e898 CTR: c030e860 [ 3.178558] REGS: c60e1bd0 TRAP: 0700 Not tainted (5.4.0-s3k-dev-00898-gd57c00e2472d) [ 3.186536] MSR: 00029032 <EE,ME,IR,DR,RI> CR: 24004842 XER: 20000000 [ 3.193161] [ 3.193161] GPR00: c030f630 c60e1c88 c60d4000 c6211c50 00000000 00000000 07de2900 c61f2210 [ 3.193161] GPR08: 00001208 c61f2210 00001200 000affff 24004848 00000000 c0003890 00000000 [ 3.193161] GPR16: 00000000 00000000 00000000 c7ffd478 c06aabc4 c06b0000 c06aaae0 c06aaaec [ 3.193161] GPR24: c06aaaf8 c06aab04 00000000 c61f2210 c07bb2dc 00000000 c6244a40 00000000 [ 3.227800] NIP [c030e8c4] fsl_spi_cs_control+0x64/0x7c [ 3.232963] LR [c030e898] fsl_spi_cs_control+0x38/0x7c [ 3.238000] Call Trace: [ 3.240450] [c60e1c98] [c030f630] fsl_spi_setup+0xc4/0x148 [ 3.245903] [c60e1cb8] [c030be98] spi_setup+0xd0/0x1c4 [ 3.250960] [c60e1cd8] [c030c030] spi_add_device+0xa4/0x190 [ 3.256472] [c60e1cf8] [c030cb28] spi_register_controller+0x75c/0xb50 [ 3.262840] [c60e1d48] [c030cf5c] devm_spi_register_controller+0x40/0x98 [ 3.269452] [c60e1d68] [c030edb0] of_fsl_spi_probe+0x2e0/0x3a0 [ 3.275220] [c60e1db8] [c02c4e94] platform_drv_probe+0x44/0xa4 [ 3.281010] [c60e1dc8] [c02c3038] really_probe+0x1ac/0x418 [ 3.286419] [c60e1df8] [c02c3ab8] device_driver_attach+0x88/0x90 [ 3.292355] [c60e1e18] [c02c3b60] __driver_attach+0xa0/0x154 [ 3.297947] [c60e1e38] [c02c1098] bus_for_each_dev+0x64/0xb4 [ 3.303540] [c60e1e68] [c02c1a74] bus_add_driver+0xe0/0x218 [ 3.309052] [c60e1e88] [c02c4318] driver_register+0x84/0x148 [ 3.314649] [c60e1e98] [c06d8d30] do_one_initcall+0x8c/0x1cc [ 3.320232] [c60e1ef8] [c06d8fac] kernel_init_freeable+0x13c/0x1ec [ 3.326341] [c60e1f28] [c00038a4] kernel_init+0x14/0x110 [ 3.331595] [c60e1f38] [c000e1cc] ret_from_kernel_thread+0x14/0x1c [ 3.337656] Instruction dump: [ 3.340590] 4bfffab1 80830018 2f840000 419e0024 80010014 215f0000 7c0803a6 83e1000c [ 3.348246] 7c631910 54630000 38210010 4bd00028 <0fe00000> 80010014 83e1000c 7c0803a6 [ 3.356108] ---[ end trace 1b884d70796b2b85 ]--- And SPI doesn't work allthough it seems to get properly registered Christophe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 9:34 ` Christophe Leroy @ 2019-11-27 10:02 ` Linus Walleij 2019-11-27 10:39 ` Christophe Leroy 0 siblings, 1 reply; 26+ messages in thread From: Linus Walleij @ 2019-11-27 10:02 UTC (permalink / raw) To: Christophe Leroy Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM On Wed, Nov 27, 2019 at 10:34 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > In the meantime, I have tried changing "gpios" by "cs-gpios" in the > device tree, and I get the following warning: (...) > [ 3.156654] WARNING: CPU: 0 PID: 1 at drivers/spi/spi-fsl-spi.c:716 > fsl_spi_cs_control+0x64/0x7c That should be this one: if (WARN_ON_ONCE(!pinfo->immr_spi_cs)) return; That happens when spi->cs_gpiod is NULL so the chipselect isn't found and assigned, and the code goes on to check the native CS and find that this isn't available either and issues the warning. That one is a bit puzzling because I know we have this working with other "cs-gpios" consumer drivers working just fine. :/ Yours, Linus Walleij ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 10:02 ` Linus Walleij @ 2019-11-27 10:39 ` Christophe Leroy 2019-11-27 10:55 ` Linus Walleij 0 siblings, 1 reply; 26+ messages in thread From: Christophe Leroy @ 2019-11-27 10:39 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 27/11/2019 à 11:02, Linus Walleij a écrit : > On Wed, Nov 27, 2019 at 10:34 AM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: > >> In the meantime, I have tried changing "gpios" by "cs-gpios" in the >> device tree, and I get the following warning: > (...) >> [ 3.156654] WARNING: CPU: 0 PID: 1 at drivers/spi/spi-fsl-spi.c:716 >> fsl_spi_cs_control+0x64/0x7c > > That should be this one: > > if (WARN_ON_ONCE(!pinfo->immr_spi_cs)) > return; > > That happens when spi->cs_gpiod is NULL so the > chipselect isn't found and assigned, and the code > goes on to check the native CS and find that this isn't > available either and issues the warning. That's in spi_add_device(), it is spi->cs_gpio and not spi->cs_gpiod which is assigned, so spi->cs_gpiod remains NULL. Christophe > > That one is a bit puzzling because I know we have this > working with other "cs-gpios" consumer drivers working > just fine. :/ > > Yours, > Linus Walleij > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 10:39 ` Christophe Leroy @ 2019-11-27 10:55 ` Linus Walleij 2019-11-27 12:04 ` Christophe Leroy 0 siblings, 1 reply; 26+ messages in thread From: Linus Walleij @ 2019-11-27 10:55 UTC (permalink / raw) To: Christophe Leroy Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM On Wed, Nov 27, 2019 at 11:39 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > Le 27/11/2019 à 11:02, Linus Walleij a écrit : > > On Wed, Nov 27, 2019 at 10:34 AM Christophe Leroy > > <christophe.leroy@c-s.fr> wrote: > > > >> In the meantime, I have tried changing "gpios" by "cs-gpios" in the > >> device tree, and I get the following warning: > > (...) > >> [ 3.156654] WARNING: CPU: 0 PID: 1 at drivers/spi/spi-fsl-spi.c:716 > >> fsl_spi_cs_control+0x64/0x7c > > > > That should be this one: > > > > if (WARN_ON_ONCE(!pinfo->immr_spi_cs)) > > return; > > > > That happens when spi->cs_gpiod is NULL so the > > chipselect isn't found and assigned, and the code > > goes on to check the native CS and find that this isn't > > available either and issues the warning. > > That's in spi_add_device(), it is spi->cs_gpio and not spi->cs_gpiod > which is assigned, so spi->cs_gpiod remains NULL. That's weird, because when ->use_gpio_descriptors is set (as for this driver) the core only attempts to look up spi->cs_gpiods and not spi->cs_gpios, and consequently can only assign spi->cd_gpiod and not spi->cs_gpio: if (ctlr->use_gpio_descriptors) { status = spi_get_gpio_descs(ctlr); (...) } else { (....) status = of_spi_get_gpio_numbers(ctlr); } (...) /* Descriptors take precedence */ if (ctlr->cs_gpiods) spi->cs_gpiod = ctlr->cs_gpiods[spi->chip_select]; else if (ctlr->cs_gpios) spi->cs_gpio = ctlr->cs_gpios[spi->chip_select]; So I'm a bit confused... Yours, Linus Walleij ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 10:55 ` Linus Walleij @ 2019-11-27 12:04 ` Christophe Leroy 2019-11-27 13:00 ` Linus Walleij 0 siblings, 1 reply; 26+ messages in thread From: Christophe Leroy @ 2019-11-27 12:04 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 27/11/2019 à 11:55, Linus Walleij a écrit : > On Wed, Nov 27, 2019 at 11:39 AM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: >> Le 27/11/2019 à 11:02, Linus Walleij a écrit : >>> On Wed, Nov 27, 2019 at 10:34 AM Christophe Leroy >>> <christophe.leroy@c-s.fr> wrote: >>> >>>> In the meantime, I have tried changing "gpios" by "cs-gpios" in the >>>> device tree, and I get the following warning: >>> (...) >>>> [ 3.156654] WARNING: CPU: 0 PID: 1 at drivers/spi/spi-fsl-spi.c:716 >>>> fsl_spi_cs_control+0x64/0x7c >>> >>> That should be this one: >>> >>> if (WARN_ON_ONCE(!pinfo->immr_spi_cs)) >>> return; >>> >>> That happens when spi->cs_gpiod is NULL so the >>> chipselect isn't found and assigned, and the code >>> goes on to check the native CS and find that this isn't >>> available either and issues the warning. >> >> That's in spi_add_device(), it is spi->cs_gpio and not spi->cs_gpiod >> which is assigned, so spi->cs_gpiod remains NULL. > > That's weird, because when ->use_gpio_descriptors is set > (as for this driver) the core only attempts to look up > spi->cs_gpiods and not spi->cs_gpios, and consequently > can only assign spi->cd_gpiod and not spi->cs_gpio: That's it. ->use_gpio_descriptors isn't set for the FSL driver: [root@po16098vm linux-powerpc]# git grep use_gpio_descriptors drivers/spi/ drivers/spi/spi-ath79.c: master->use_gpio_descriptors = true; drivers/spi/spi-atmel.c: master->use_gpio_descriptors = true; drivers/spi/spi-bcm2835.c: * as the flag use_gpio_descriptors enforces SPI_CS_HIGH. drivers/spi/spi-bcm2835.c: ctlr->use_gpio_descriptors = true; drivers/spi/spi-cadence.c: master->use_gpio_descriptors = true; drivers/spi/spi-clps711x.c: master->use_gpio_descriptors = true; drivers/spi/spi-davinci.c: master->use_gpio_descriptors = true; drivers/spi/spi-dw.c: master->use_gpio_descriptors = true; drivers/spi/spi-ep93xx.c: master->use_gpio_descriptors = true; drivers/spi/spi-gpio.c: master->use_gpio_descriptors = true; drivers/spi/spi-sh-msiof.c: ctlr->use_gpio_descriptors = true; drivers/spi/spi-tegra114.c: master->use_gpio_descriptors = true; drivers/spi/spi.c: if (ctlr->use_gpio_descriptors) drivers/spi/spi.c: if (ctlr->use_gpio_descriptors) { I have now added it, together with the DTS cs-gpios name change (without your counting patch with crashes), and I get something which is almost working: I get temperature back into sensors, but temperature is 0°C !!! root@vgoip:~# sensors lm74-spi-0-5 Adapter: SPI adapter Temperature processeur: +0.0 C lm74-spi-0-1 Adapter: SPI adapter Temperature MIAE: +0.0 C Looking into dmesg, I see: [ 3.153521] lm74@1 GPIO handle specifies active low - ignored [ 3.178093] lm74@5 GPIO handle specifies active low - ignored Any link with the problem ? Christophe > > if (ctlr->use_gpio_descriptors) { > status = spi_get_gpio_descs(ctlr); > (...) > } else { > (....) > status = of_spi_get_gpio_numbers(ctlr); > } > (...) > /* Descriptors take precedence */ > if (ctlr->cs_gpiods) > spi->cs_gpiod = ctlr->cs_gpiods[spi->chip_select]; > else if (ctlr->cs_gpios) > spi->cs_gpio = ctlr->cs_gpios[spi->chip_select]; > > So I'm a bit confused... > > Yours, > Linus Walleij > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 12:04 ` Christophe Leroy @ 2019-11-27 13:00 ` Linus Walleij 2019-11-27 13:44 ` Christophe Leroy 0 siblings, 1 reply; 26+ messages in thread From: Linus Walleij @ 2019-11-27 13:00 UTC (permalink / raw) To: Christophe Leroy Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM On Wed, Nov 27, 2019 at 1:05 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > Le 27/11/2019 à 11:55, Linus Walleij a écrit : > > That's weird, because when ->use_gpio_descriptors is set > > (as for this driver) the core only attempts to look up > > spi->cs_gpiods and not spi->cs_gpios, and consequently > > can only assign spi->cd_gpiod and not spi->cs_gpio: > > That's it. ->use_gpio_descriptors isn't set for the FSL driver: Oh, my coding mistake. :( And an especially stupid one too. OK I make a separate patch in the series to fix that too. > I have now added it, together with the DTS cs-gpios name change (without > your counting patch with crashes), and I get something which is almost > working: I get temperature back into sensors, but temperature is 0°C !!! OK we almost fixed it I think. It is probably better to test with all three patches (will send out soon) that rely on the gpiolib to do appropriate counting of the gpiod's and so on. > Looking into dmesg, I see: > > [ 3.153521] lm74@1 GPIO handle specifies active low - ignored > [ 3.178093] lm74@5 GPIO handle specifies active low - ignored > > Any link with the problem ? This is because your GPIO handles look like this: gpios = <&CPM1_PIO_C 4 1 /* SICOFI 1 */ &CPM1_PIO_B 23 1 /* TEMP MCR */ &CPM1_PIO_C 8 1 /* SICOFI 2 */ &CPM1_PIO_C 12 1 /* EEPROM MIAE */ &CPM1_PIO_D 6 1 /* SICOFI 3 */ &CPM1_PIO_B 14 1 /* TEMP MPC885 */ &CPM1_PIO_B 21 1 /* EEPROM CMPC885 */ &FAV_CS_SPI 0 1 /* FAV SPI */ &FAV_CS_SPI 2 1>; /* FAV POSTE FPGA */ That "1" at the end of each GPIO phandle means "active low" as can be seen in <dt-bindings/gpio/gpio.h> /* Bit 0 express polarity */ #define GPIO_ACTIVE_HIGH 0 #define GPIO_ACTIVE_LOW 1 But your child nodes look like this: sicofi@0 { compatible = "infineon,sicofi"; spi-max-frequency = <1000000>; reg = <0>; spi-cs-high; spi-cpha; }; lm74@1 { compatible = "ns,lm74"; spi-max-frequency = <1000000>; reg = <1>; spi-cs-high; }; And the spi-cs-high in the child node takes precedence. That's a bit ambigous so that is what the warning is about. Try to remove the "spi-cs-high" bool flag from your nodes, because it seems like the old code was ignoring them. Does that solve the problem? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 13:00 ` Linus Walleij @ 2019-11-27 13:44 ` Christophe Leroy 2019-11-27 13:52 ` Linus Walleij 0 siblings, 1 reply; 26+ messages in thread From: Christophe Leroy @ 2019-11-27 13:44 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 27/11/2019 à 14:00, Linus Walleij a écrit : > On Wed, Nov 27, 2019 at 1:05 PM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: >> Le 27/11/2019 à 11:55, Linus Walleij a écrit : > >>> That's weird, because when ->use_gpio_descriptors is set >>> (as for this driver) the core only attempts to look up >>> spi->cs_gpiods and not spi->cs_gpios, and consequently >>> can only assign spi->cd_gpiod and not spi->cs_gpio: >> >> That's it. ->use_gpio_descriptors isn't set for the FSL driver: > > Oh, my coding mistake. :( > > And an especially stupid one too. OK I make a separate patch > in the series to fix that too. > >> I have now added it, together with the DTS cs-gpios name change (without >> your counting patch with crashes), and I get something which is almost >> working: I get temperature back into sensors, but temperature is 0°C !!! > > OK we almost fixed it I think. It is probably better to test with > all three patches (will send out soon) that rely on the gpiolib > to do appropriate counting of the gpiod's and so on. > >> Looking into dmesg, I see: >> >> [ 3.153521] lm74@1 GPIO handle specifies active low - ignored >> [ 3.178093] lm74@5 GPIO handle specifies active low - ignored >> >> Any link with the problem ? > > This is because your GPIO handles look like this: > > gpios = <&CPM1_PIO_C 4 1 /* SICOFI 1 */ > &CPM1_PIO_B 23 1 /* TEMP MCR */ > &CPM1_PIO_C 8 1 /* SICOFI 2 */ > &CPM1_PIO_C 12 1 /* EEPROM MIAE */ > &CPM1_PIO_D 6 1 /* SICOFI 3 */ > &CPM1_PIO_B 14 1 /* TEMP MPC885 */ > &CPM1_PIO_B 21 1 /* EEPROM CMPC885 */ > &FAV_CS_SPI 0 1 /* FAV SPI */ > &FAV_CS_SPI 2 1>; /* FAV POSTE FPGA */ > > That "1" at the end of each GPIO phandle means "active low" > as can be seen in <dt-bindings/gpio/gpio.h> > > /* Bit 0 express polarity */ > #define GPIO_ACTIVE_HIGH 0 > #define GPIO_ACTIVE_LOW 1 > > But your child nodes look like this: > > sicofi@0 { > compatible = "infineon,sicofi"; > spi-max-frequency = <1000000>; > reg = <0>; > spi-cs-high; > spi-cpha; > }; > > lm74@1 { > compatible = "ns,lm74"; > spi-max-frequency = <1000000>; > reg = <1>; > spi-cs-high; > }; > > And the spi-cs-high in the child node takes precedence. > > That's a bit ambigous so that is what the warning is about. > > Try to remove the "spi-cs-high" bool flag from your nodes, > because it seems like the old code was ignoring them. > > Does that solve the problem? Yes it does. Many thanks. I let you manage the packaging of fixes. Christophe > > Yours, > Linus Walleij > ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 13:44 ` Christophe Leroy @ 2019-11-27 13:52 ` Linus Walleij 2019-11-27 13:54 ` Christophe Leroy 0 siblings, 1 reply; 26+ messages in thread From: Linus Walleij @ 2019-11-27 13:52 UTC (permalink / raw) To: Christophe Leroy Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM On Wed, Nov 27, 2019 at 2:45 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > Le 27/11/2019 à 14:00, Linus Walleij a écrit : > > Try to remove the "spi-cs-high" bool flag from your nodes, > > because it seems like the old code was ignoring them. > > > > Does that solve the problem? > > Yes it does. Many thanks. I let you manage the packaging of fixes. OK I will send a final batch of 3 patches fixing this. Do you have these old device trees deployed so that we also need to make sure that old device trees that have this ambigous syntax will force precendence for the flag on the GPIO line if both are specified for the "fsl,spi" instances? Yours, Linus Walleij ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 13:52 ` Linus Walleij @ 2019-11-27 13:54 ` Christophe Leroy 2019-11-27 13:56 ` Linus Walleij 0 siblings, 1 reply; 26+ messages in thread From: Christophe Leroy @ 2019-11-27 13:54 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 27/11/2019 à 14:52, Linus Walleij a écrit : > On Wed, Nov 27, 2019 at 2:45 PM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: >> Le 27/11/2019 à 14:00, Linus Walleij a écrit : > >>> Try to remove the "spi-cs-high" bool flag from your nodes, >>> because it seems like the old code was ignoring them. >>> >>> Does that solve the problem? >> >> Yes it does. Many thanks. I let you manage the packaging of fixes. > > OK I will send a final batch of 3 patches fixing this. > > Do you have these old device trees deployed so that we > also need to make sure that old device trees that have this > ambigous syntax will force precendence for the flag on the > GPIO line if both are specified for the "fsl,spi" instances? > No, we deliver device trees together with Linux kernel (embedded in an Uboot ITS/ITB image) so no worry on old device trees. Christophe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 13:54 ` Christophe Leroy @ 2019-11-27 13:56 ` Linus Walleij 2019-11-27 14:29 ` Christophe Leroy 0 siblings, 1 reply; 26+ messages in thread From: Linus Walleij @ 2019-11-27 13:56 UTC (permalink / raw) To: Christophe Leroy Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM On Wed, Nov 27, 2019 at 2:54 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > Le 27/11/2019 à 14:52, Linus Walleij a écrit : > > On Wed, Nov 27, 2019 at 2:45 PM Christophe Leroy > > <christophe.leroy@c-s.fr> wrote: > >> Le 27/11/2019 à 14:00, Linus Walleij a écrit : > > > >>> Try to remove the "spi-cs-high" bool flag from your nodes, > >>> because it seems like the old code was ignoring them. > >>> > >>> Does that solve the problem? > >> > >> Yes it does. Many thanks. I let you manage the packaging of fixes. > > > > OK I will send a final batch of 3 patches fixing this. > > > > Do you have these old device trees deployed so that we > > also need to make sure that old device trees that have this > > ambigous syntax will force precendence for the flag on the > > GPIO line if both are specified for the "fsl,spi" instances? > > > > No, we deliver device trees together with Linux kernel (embedded in an > Uboot ITS/ITB image) so no worry on old device trees. OK thanks! I sent three patches, can you apply them on a clean tree and confirm it solves the problem (fingers crossed...) Thanks a lot for helping me fix this! Yours, Linus Walleij ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 13:56 ` Linus Walleij @ 2019-11-27 14:29 ` Christophe Leroy 2019-11-28 8:42 ` Linus Walleij 0 siblings, 1 reply; 26+ messages in thread From: Christophe Leroy @ 2019-11-27 14:29 UTC (permalink / raw) To: Linus Walleij Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM Le 27/11/2019 à 14:56, Linus Walleij a écrit : > On Wed, Nov 27, 2019 at 2:54 PM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: >> Le 27/11/2019 à 14:52, Linus Walleij a écrit : >>> On Wed, Nov 27, 2019 at 2:45 PM Christophe Leroy >>> <christophe.leroy@c-s.fr> wrote: >>>> Le 27/11/2019 à 14:00, Linus Walleij a écrit : >>> >>>>> Try to remove the "spi-cs-high" bool flag from your nodes, >>>>> because it seems like the old code was ignoring them. >>>>> >>>>> Does that solve the problem? >>>> >>>> Yes it does. Many thanks. I let you manage the packaging of fixes. >>> >>> OK I will send a final batch of 3 patches fixing this. >>> >>> Do you have these old device trees deployed so that we >>> also need to make sure that old device trees that have this >>> ambigous syntax will force precendence for the flag on the >>> GPIO line if both are specified for the "fsl,spi" instances? >>> >> >> No, we deliver device trees together with Linux kernel (embedded in an >> Uboot ITS/ITB image) so no worry on old device trees. > > OK thanks! > > I sent three patches, can you apply them on a clean tree > and confirm it solves the problem (fingers crossed...) > > Thanks a lot for helping me fix this! The series is OK if using 'cs-gpios'. With 'gpios' in the DTS, I get: [ 3.154747] fsl_spi ff000a80.spi: cs1 >= max 1 [ 3.159207] spi_master spi0: spi_device register error /soc@ff000000/cpm@9c0/spi@a80/lm74@1 [ 3.167344] spi_master spi0: Failed to create SPI device for /soc@ff000000/cpm@9c0/spi@a80/lm74@1 [ 3.176303] fsl_spi ff000a80.spi: cs2 >= max 1 [ 3.180626] spi_master spi0: spi_device register error /soc@ff000000/cpm@9c0/spi@a80/sicofi_gw@2 [ 3.189329] spi_master spi0: Failed to create SPI device for /soc@ff000000/cpm@9c0/spi@a80/sicofi_gw@2 [ 3.198574] fsl_spi ff000a80.spi: cs3 >= max 1 [ 3.202788] spi_master spi0: spi_device register error /soc@ff000000/cpm@9c0/spi@a80/eeprom@3 [ 3.211364] spi_master spi0: Failed to create SPI device for /soc@ff000000/cpm@9c0/spi@a80/eeprom@3 [ 3.220361] fsl_spi ff000a80.spi: cs4 >= max 1 [ 3.224561] spi_master spi0: spi_device register error /soc@ff000000/cpm@9c0/spi@a80/sicofi@4 [ 3.233137] spi_master spi0: Failed to create SPI device for /soc@ff000000/cpm@9c0/spi@a80/sicofi@4 [ 3.242120] fsl_spi ff000a80.spi: cs5 >= max 1 [ 3.246336] spi_master spi0: spi_device register error /soc@ff000000/cpm@9c0/spi@a80/lm74@5 [ 3.254740] spi_master spi0: Failed to create SPI device for /soc@ff000000/cpm@9c0/spi@a80/lm74@5 [ 3.263552] fsl_spi ff000a80.spi: cs6 >= max 1 [ 3.267764] spi_master spi0: spi_device register error /soc@ff000000/cpm@9c0/spi@a80/eeprom@6 [ 3.276342] spi_master spi0: Failed to create SPI device for /soc@ff000000/cpm@9c0/spi@a80/eeprom@6 [ 3.285328] fsl_spi ff000a80.spi: cs7 >= max 1 [ 3.289667] spi_master spi0: spi_device register error /soc@ff000000/cpm@9c0/spi@a80/csfavgw@7 [ 3.298070] spi_master spi0: Failed to create SPI device for /soc@ff000000/cpm@9c0/spi@a80/csfavgw@7 Christophe ^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") 2019-11-27 14:29 ` Christophe Leroy @ 2019-11-28 8:42 ` Linus Walleij 0 siblings, 0 replies; 26+ messages in thread From: Linus Walleij @ 2019-11-28 8:42 UTC (permalink / raw) To: Christophe Leroy Cc: linux-spi, Mark Brown, Fabio Estevam, open list:GPIO SUBSYSTEM On Wed, Nov 27, 2019 at 3:29 PM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > Le 27/11/2019 à 14:56, Linus Walleij a écrit : > > I sent three patches, can you apply them on a clean tree > > and confirm it solves the problem (fingers crossed...) > > > > Thanks a lot for helping me fix this! > > The series is OK if using 'cs-gpios'. > > With 'gpios' in the DTS, I get: > > [ 3.154747] fsl_spi ff000a80.spi: cs1 >= max 1 > [ 3.159207] spi_master spi0: spi_device register error Ooops I looked over the gpiolib patch and found (I think) my mistake. I sent a v3 patch set that hopefully will make everything work as expected with just gpios = <...>; Yours, Linus Walleij ^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2019-11-28 8:43 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 12:33 Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors") Christophe Leroy
2019-11-08 13:09 ` Linus Walleij
2019-11-08 13:34 ` Christophe Leroy
2019-11-26 15:01 ` Christophe Leroy
2019-11-26 15:35 ` Fabio Estevam
2019-11-26 16:16 ` Christophe Leroy
2019-11-26 16:23 ` Mark Brown
2019-11-26 15:48 ` Linus Walleij
2019-11-26 19:08 ` Christophe Leroy
2019-11-26 19:14 ` Christophe Leroy
2019-11-27 8:26 ` Linus Walleij
2019-11-27 8:57 ` Christophe Leroy
2019-11-27 9:07 ` Christophe Leroy
2019-11-27 9:11 ` Linus Walleij
2019-11-27 9:34 ` Christophe Leroy
2019-11-27 10:02 ` Linus Walleij
2019-11-27 10:39 ` Christophe Leroy
2019-11-27 10:55 ` Linus Walleij
2019-11-27 12:04 ` Christophe Leroy
2019-11-27 13:00 ` Linus Walleij
2019-11-27 13:44 ` Christophe Leroy
2019-11-27 13:52 ` Linus Walleij
2019-11-27 13:54 ` Christophe Leroy
2019-11-27 13:56 ` Linus Walleij
2019-11-27 14:29 ` Christophe Leroy
2019-11-28 8:42 ` Linus Walleij
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).