Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Linus Walleij]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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
> 

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Fabio Estevam]

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.

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Linus Walleij]

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(&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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Mark Brown]

[-- 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 --]

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Linus Walleij]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Linus Walleij]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Linus Walleij]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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
> 

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Linus Walleij]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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
> 

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Linus Walleij]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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
> 

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Linus Walleij]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Linus Walleij]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Christophe Leroy]

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

Re: Boot failure with 5.4-rc5, bisected to 0f0581b24bd0 ("spi: fsl: Convert to use CS GPIO descriptors")

[Linus Walleij]

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