[PATCH] spi/acpi: fix incorrect ACPI parent check

[PATCH] spi/acpi: fix incorrect ACPI parent check

[Ard Biesheuvel]

The ACPI device object parsing code for SPI slaves enumerates the
entire ACPI namespace to look for devices that refer to the master
in question via the 'resource_source' field in the 'SPISerialBus'
resource. If that field does not refer to a valid ACPI device or
if it refers to the wrong SPI master, we should disregard the
device.

Current, the valid device check is wrong, since it gets the
polarity of 'status' wrong. This could cause issues if the
'resource_source' field is bogus but parent_handle happens to
refer to the correct master (which is not entirely imaginary
since this code runs in a loop)

So test for ACPI_FAILURE() instead, to make the code more
self explanatory.

Fixes: 4c3c59544f33 ("spi/acpi: enumerate all SPI slaves in the namespace")
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: andy.shevchenko@gmail.com
Cc: masahisa.kojima@linaro.org
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: linux-acpi@vger.kernel.org
Cc: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/spi/spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c8adcc97f3ef..50d230b33c42 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1859,7 +1859,7 @@ static int acpi_spi_add_resource(struct acpi_resource *ares, void *data)
 						 sb->resource_source.string_ptr,
 						 &parent_handle);
 
-			if (!status ||
+			if (ACPI_FAILURE(status) ||
 			    ACPI_HANDLE(ctlr->dev.parent) != parent_handle)
 				return -ENODEV;
 
-- 
2.20.1

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Mika Westerberg]

On Wed, Jun 19, 2019 at 11:52:54AM +0200, Ard Biesheuvel wrote:
> The ACPI device object parsing code for SPI slaves enumerates the
> entire ACPI namespace to look for devices that refer to the master
> in question via the 'resource_source' field in the 'SPISerialBus'
> resource. If that field does not refer to a valid ACPI device or
> if it refers to the wrong SPI master, we should disregard the
> device.
> 
> Current, the valid device check is wrong, since it gets the
> polarity of 'status' wrong. This could cause issues if the
> 'resource_source' field is bogus but parent_handle happens to
> refer to the correct master (which is not entirely imaginary
> since this code runs in a loop)
> 
> So test for ACPI_FAILURE() instead, to make the code more
> self explanatory.
> 
> Fixes: 4c3c59544f33 ("spi/acpi: enumerate all SPI slaves in the namespace")
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Jarkko Nikula]

On 6/19/19 1:16 PM, Mika Westerberg wrote:
> On Wed, Jun 19, 2019 at 11:52:54AM +0200, Ard Biesheuvel wrote:
>> The ACPI device object parsing code for SPI slaves enumerates the
>> entire ACPI namespace to look for devices that refer to the master
>> in question via the 'resource_source' field in the 'SPISerialBus'
>> resource. If that field does not refer to a valid ACPI device or
>> if it refers to the wrong SPI master, we should disregard the
>> device.
>>
>> Current, the valid device check is wrong, since it gets the
>> polarity of 'status' wrong. This could cause issues if the
>> 'resource_source' field is bogus but parent_handle happens to
>> refer to the correct master (which is not entirely imaginary
>> since this code runs in a loop)
>>
>> So test for ACPI_FAILURE() instead, to make the code more
>> self explanatory.
>>
>> Fixes: 4c3c59544f33 ("spi/acpi: enumerate all SPI slaves in the namespace")
>> Reported-by: kbuild test robot <lkp@intel.com>
>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> 
I hit yesterday a regression caused by 4c3c59544f33. I've a custom ACPI 
tables defining I2C gpio expanders (gpio-pca953x.c as INT3491) and a 
spidev test device (SPT0001).

Both stopped enumerating after 4c3c59544f33. With this fix spidev device 
enumerates but still get confused with I2C GPIO expanders (INT3491):

[    5.629874][    T1] pxa2xx-spi pxa2xx-spi.3: registered master spi3
[    5.644447][    T1] pxa2xx-spi pxa2xx-spi.5: registered master spi5
[    5.653930][    T1] spi spi-SPT0001:00: in setup: DMA burst size set to 8
[    5.661300][    T1] spi spi-SPT0001:00: setup mode 0, 8 bits/w, 
1000000 Hz max --> 0
[    5.671360][    T1] spidev spi-SPT0001:00: do not use this driver in 
production systems!
[    5.682325][    T1] pxa2xx-spi pxa2xx-spi.5: registered child 
spi-SPT0001:00
[    5.690240][    T1] spi spi-PRP0001:00: in setup: DMA burst size set to 8
[    5.697492][    T1] spi spi-PRP0001:00: setup mode 0, 8 bits/w, 
20000000 Hz max --> 0
[    5.706928][    T1] pxa2xx-spi pxa2xx-spi.5: registered child 
spi-PRP0001:00
[    5.715754][    T1] pxa2xx-spi pxa2xx-spi.5: cs104 >= max 4
[    5.721688][    T1] spi_master spi5: failed to add SPI device 
INT3491:00 from ACPI
[    5.730648][    T1] pxa2xx-spi pxa2xx-spi.5: cs104 >= max 4
[    5.736657][    T1] spi_master spi5: failed to add SPI device 
INT3491:01 from ACPI
[    5.745617][    T1] pxa2xx-spi pxa2xx-spi.5: cs104 >= max 4
[    5.751546][    T1] spi_master spi5: failed to add SPI device 
INT3491:02 from ACPI
[    5.760628][    T1] pxa2xx-spi pxa2xx-spi.5: cs104 >= max 4
[    5.766549][    T1] spi_master spi5: failed to add SPI device 
INT3491:03 from ACPI
[    5.777160][    T1] pxa2xx-spi pxa2xx-spi.5: cs104 >= max 4
[    5.783087][    T1] spi_master spi5: failed to add SPI device 
BCM2E95:00 from ACPI
[    5.797008][    T1] pxa2xx-spi pxa2xx-spi.6: registered master spi6

Ok log with commit 4c3c59544f33 reverted:

[    5.633116][    T1] pxa2xx-spi pxa2xx-spi.3: registered master spi3
[    5.647701][    T1] pxa2xx-spi pxa2xx-spi.5: registered master spi5
[    5.655668][    T1] spi spi-SPT0001:00: in setup: DMA burst size set to 8
[    5.663066][    T1] spi spi-SPT0001:00: setup mode 0, 8 bits/w, 
1000000 Hz max --> 0
[    5.672758][    T1] pxa2xx-spi pxa2xx-spi.5: registered child 
spi-SPT0001:00
[    5.680602][    T1] spi spi-PRP0001:00: in setup: DMA burst size set to 8
[    5.687820][    T1] spi spi-PRP0001:00: setup mode 0, 8 bits/w, 
20000000 Hz max --> 0
[    5.697366][    T1] pxa2xx-spi pxa2xx-spi.5: registered child 
spi-PRP0001:00
[    5.709064][    T1] pxa2xx-spi pxa2xx-spi.6: registered master spi6
[   11.021760][   T84] spidev spi-SPT0001:00: do not use this driver in 
production systems!

-- 
Jarkko

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Ard Biesheuvel]

On Wed, 19 Jun 2019 at 13:58, Jarkko Nikula
<jarkko.nikula@linux.intel.com> wrote:
>
> On 6/19/19 1:16 PM, Mika Westerberg wrote:
> > On Wed, Jun 19, 2019 at 11:52:54AM +0200, Ard Biesheuvel wrote:
> >> The ACPI device object parsing code for SPI slaves enumerates the
> >> entire ACPI namespace to look for devices that refer to the master
> >> in question via the 'resource_source' field in the 'SPISerialBus'
> >> resource. If that field does not refer to a valid ACPI device or
> >> if it refers to the wrong SPI master, we should disregard the
> >> device.
> >>
> >> Current, the valid device check is wrong, since it gets the
> >> polarity of 'status' wrong. This could cause issues if the
> >> 'resource_source' field is bogus but parent_handle happens to
> >> refer to the correct master (which is not entirely imaginary
> >> since this code runs in a loop)
> >>
> >> So test for ACPI_FAILURE() instead, to make the code more
> >> self explanatory.
> >>
> >> Fixes: 4c3c59544f33 ("spi/acpi: enumerate all SPI slaves in the namespace")
> >> Reported-by: kbuild test robot <lkp@intel.com>
> >> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> >
> > Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >
> I hit yesterday a regression caused by 4c3c59544f33. I've a custom ACPI
> tables defining I2C gpio expanders (gpio-pca953x.c as INT3491) and a
> spidev test device (SPT0001).
>
> Both stopped enumerating after 4c3c59544f33. With this fix spidev device
> enumerates but still get confused with I2C GPIO expanders (INT3491):
>

Could you share the decomplied D/SSDT please?

> [    5.629874][    T1] pxa2xx-spi pxa2xx-spi.3: registered master spi3
> [    5.644447][    T1] pxa2xx-spi pxa2xx-spi.5: registered master spi5
> [    5.653930][    T1] spi spi-SPT0001:00: in setup: DMA burst size set to 8
> [    5.661300][    T1] spi spi-SPT0001:00: setup mode 0, 8 bits/w,
> 1000000 Hz max --> 0
> [    5.671360][    T1] spidev spi-SPT0001:00: do not use this driver in
> production systems!
> [    5.682325][    T1] pxa2xx-spi pxa2xx-spi.5: registered child
> spi-SPT0001:00
> [    5.690240][    T1] spi spi-PRP0001:00: in setup: DMA burst size set to 8
> [    5.697492][    T1] spi spi-PRP0001:00: setup mode 0, 8 bits/w,
> 20000000 Hz max --> 0
> [    5.706928][    T1] pxa2xx-spi pxa2xx-spi.5: registered child
> spi-PRP0001:00
> [    5.715754][    T1] pxa2xx-spi pxa2xx-spi.5: cs104 >= max 4
> [    5.721688][    T1] spi_master spi5: failed to add SPI device
> INT3491:00 from ACPI
> [    5.730648][    T1] pxa2xx-spi pxa2xx-spi.5: cs104 >= max 4
> [    5.736657][    T1] spi_master spi5: failed to add SPI device
> INT3491:01 from ACPI
> [    5.745617][    T1] pxa2xx-spi pxa2xx-spi.5: cs104 >= max 4
> [    5.751546][    T1] spi_master spi5: failed to add SPI device
> INT3491:02 from ACPI
> [    5.760628][    T1] pxa2xx-spi pxa2xx-spi.5: cs104 >= max 4
> [    5.766549][    T1] spi_master spi5: failed to add SPI device
> INT3491:03 from ACPI
> [    5.777160][    T1] pxa2xx-spi pxa2xx-spi.5: cs104 >= max 4
> [    5.783087][    T1] spi_master spi5: failed to add SPI device
> BCM2E95:00 from ACPI
> [    5.797008][    T1] pxa2xx-spi pxa2xx-spi.6: registered master spi6
>
> Ok log with commit 4c3c59544f33 reverted:
>
> [    5.633116][    T1] pxa2xx-spi pxa2xx-spi.3: registered master spi3
> [    5.647701][    T1] pxa2xx-spi pxa2xx-spi.5: registered master spi5
> [    5.655668][    T1] spi spi-SPT0001:00: in setup: DMA burst size set to 8
> [    5.663066][    T1] spi spi-SPT0001:00: setup mode 0, 8 bits/w,
> 1000000 Hz max --> 0
> [    5.672758][    T1] pxa2xx-spi pxa2xx-spi.5: registered child
> spi-SPT0001:00
> [    5.680602][    T1] spi spi-PRP0001:00: in setup: DMA burst size set to 8
> [    5.687820][    T1] spi spi-PRP0001:00: setup mode 0, 8 bits/w,
> 20000000 Hz max --> 0
> [    5.697366][    T1] pxa2xx-spi pxa2xx-spi.5: registered child
> spi-PRP0001:00
> [    5.709064][    T1] pxa2xx-spi pxa2xx-spi.6: registered master spi6
> [   11.021760][   T84] spidev spi-SPT0001:00: do not use this driver in
> production systems!
>
> --
> Jarkko

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Jarkko Nikula]

On 6/19/19 2:59 PM, Ard Biesheuvel wrote:
> On Wed, 19 Jun 2019 at 13:58, Jarkko Nikula
> <jarkko.nikula@linux.intel.com> wrote:
>>
>> On 6/19/19 1:16 PM, Mika Westerberg wrote:
>>> On Wed, Jun 19, 2019 at 11:52:54AM +0200, Ard Biesheuvel wrote:
>>>> The ACPI device object parsing code for SPI slaves enumerates the
>>>> entire ACPI namespace to look for devices that refer to the master
>>>> in question via the 'resource_source' field in the 'SPISerialBus'
>>>> resource. If that field does not refer to a valid ACPI device or
>>>> if it refers to the wrong SPI master, we should disregard the
>>>> device.
>>>>
>>>> Current, the valid device check is wrong, since it gets the
>>>> polarity of 'status' wrong. This could cause issues if the
>>>> 'resource_source' field is bogus but parent_handle happens to
>>>> refer to the correct master (which is not entirely imaginary
>>>> since this code runs in a loop)
>>>>
>>>> So test for ACPI_FAILURE() instead, to make the code more
>>>> self explanatory.
>>>>
>>>> Fixes: 4c3c59544f33 ("spi/acpi: enumerate all SPI slaves in the namespace")
>>>> Reported-by: kbuild test robot <lkp@intel.com>
>>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>>>> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
>>>
>>> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>>>
>> I hit yesterday a regression caused by 4c3c59544f33. I've a custom ACPI
>> tables defining I2C gpio expanders (gpio-pca953x.c as INT3491) and a
>> spidev test device (SPT0001).
>>
>> Both stopped enumerating after 4c3c59544f33. With this fix spidev device
>> enumerates but still get confused with I2C GPIO expanders (INT3491):
>>
> 
> Could you share the decomplied D/SSDT please?
> 
It's Intel Edison with tables from Mika's sample ACPI tables. The 
interesting parts here are these two:

https://github.com/westeri/meta-acpi/blob/master/recipes-bsp/acpi-tables/samples/edison/spidev.asl

https://github.com/westeri/meta-acpi/blob/master/recipes-bsp/acpi-tables/samples/edison/gpioexp.asli

The full tables are of course larger but I think those two above are 
relevant here. I build SSDT from arduino-all.asl below which includes 
bunch of other files and with above spidev.asl.

https://github.com/westeri/meta-acpi/blob/master/recipes-bsp/acpi-tables/samples/edison/arduino-all.asl

Let me know if you need full dump.

-- 
Jarkko

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Ard Biesheuvel]

On Wed, 19 Jun 2019 at 15:21, Jarkko Nikula
<jarkko.nikula@linux.intel.com> wrote:
>
> On 6/19/19 2:59 PM, Ard Biesheuvel wrote:
> > On Wed, 19 Jun 2019 at 13:58, Jarkko Nikula
> > <jarkko.nikula@linux.intel.com> wrote:
> >>
> >> On 6/19/19 1:16 PM, Mika Westerberg wrote:
> >>> On Wed, Jun 19, 2019 at 11:52:54AM +0200, Ard Biesheuvel wrote:
> >>>> The ACPI device object parsing code for SPI slaves enumerates the
> >>>> entire ACPI namespace to look for devices that refer to the master
> >>>> in question via the 'resource_source' field in the 'SPISerialBus'
> >>>> resource. If that field does not refer to a valid ACPI device or
> >>>> if it refers to the wrong SPI master, we should disregard the
> >>>> device.
> >>>>
> >>>> Current, the valid device check is wrong, since it gets the
> >>>> polarity of 'status' wrong. This could cause issues if the
> >>>> 'resource_source' field is bogus but parent_handle happens to
> >>>> refer to the correct master (which is not entirely imaginary
> >>>> since this code runs in a loop)
> >>>>
> >>>> So test for ACPI_FAILURE() instead, to make the code more
> >>>> self explanatory.
> >>>>
> >>>> Fixes: 4c3c59544f33 ("spi/acpi: enumerate all SPI slaves in the namespace")
> >>>> Reported-by: kbuild test robot <lkp@intel.com>
> >>>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >>>> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> >>>
> >>> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >>>
> >> I hit yesterday a regression caused by 4c3c59544f33. I've a custom ACPI
> >> tables defining I2C gpio expanders (gpio-pca953x.c as INT3491) and a
> >> spidev test device (SPT0001).
> >>
> >> Both stopped enumerating after 4c3c59544f33. With this fix spidev device
> >> enumerates but still get confused with I2C GPIO expanders (INT3491):
> >>
> >
> > Could you share the decomplied D/SSDT please?
> >
> It's Intel Edison with tables from Mika's sample ACPI tables. The
> interesting parts here are these two:
>
> https://github.com/westeri/meta-acpi/blob/master/recipes-bsp/acpi-tables/samples/edison/spidev.asl
>
> https://github.com/westeri/meta-acpi/blob/master/recipes-bsp/acpi-tables/samples/edison/gpioexp.asli
>
> The full tables are of course larger but I think those two above are
> relevant here. I build SSDT from arduino-all.asl below which includes
> bunch of other files and with above spidev.asl.
>
> https://github.com/westeri/meta-acpi/blob/master/recipes-bsp/acpi-tables/samples/edison/arduino-all.asl
>
> Let me know if you need full dump.
>

So can you explain how exactly the I2C GPIO expander is failing? I
struggle to understand how the SPI slave probing could be related to
that.

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Jarkko Nikula]

Hi

On 6/19/19 4:58 PM, Ard Biesheuvel wrote:
> So can you explain how exactly the I2C GPIO expander is failing? I
> struggle to understand how the SPI slave probing could be related to
> that.
> 
They don't show up in /sys/kernel/debug/gpio, are not present in 
/sys/bus/i2c/devices/ but SPI core instead tries add them with a bogus 
Chip Select number:

[    5.727699][    T1] pxa2xx-spi pxa2xx-spi.5: cs56 >= max 4
[    5.733545][    T1] spi_master spi5: failed to add SPI device 
INT3491:00 from ACPI

-- 
Jarkko

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Mika Westerberg]

On Wed, Jun 19, 2019 at 05:17:59PM +0300, Jarkko Nikula wrote:
> Hi
> 
> On 6/19/19 4:58 PM, Ard Biesheuvel wrote:
> > So can you explain how exactly the I2C GPIO expander is failing? I
> > struggle to understand how the SPI slave probing could be related to
> > that.
> > 
> They don't show up in /sys/kernel/debug/gpio, are not present in
> /sys/bus/i2c/devices/ but SPI core instead tries add them with a bogus Chip
> Select number:
> 
> [    5.727699][    T1] pxa2xx-spi pxa2xx-spi.5: cs56 >= max 4
> [    5.733545][    T1] spi_master spi5: failed to add SPI device INT3491:00
> from ACPI

Just a guess but looking at the 4c3c59544f33 acpi_register_spi_device()
does not seem to zero fill the whole struct acpi_spi_lookup structure so
when it is supposed to bail out when SPI slave was not found:

	if (!lookup.max_speed_hz)
 		return AE_OK

it instead continues to register SPI slave because lookup.max_speed_hz
may contain whatever garbage there is in the stack at that address.

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Ard Biesheuvel]

On Wed, 19 Jun 2019 at 16:43, Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:
>
> On Wed, Jun 19, 2019 at 05:17:59PM +0300, Jarkko Nikula wrote:
> > Hi
> >
> > On 6/19/19 4:58 PM, Ard Biesheuvel wrote:
> > > So can you explain how exactly the I2C GPIO expander is failing? I
> > > struggle to understand how the SPI slave probing could be related to
> > > that.
> > >
> > They don't show up in /sys/kernel/debug/gpio, are not present in
> > /sys/bus/i2c/devices/ but SPI core instead tries add them with a bogus Chip
> > Select number:
> >
> > [    5.727699][    T1] pxa2xx-spi pxa2xx-spi.5: cs56 >= max 4
> > [    5.733545][    T1] spi_master spi5: failed to add SPI device INT3491:00
> > from ACPI
>
> Just a guess but looking at the 4c3c59544f33 acpi_register_spi_device()
> does not seem to zero fill the whole struct acpi_spi_lookup structure so
> when it is supposed to bail out when SPI slave was not found:
>
>         if (!lookup.max_speed_hz)
>                 return AE_OK
>
> it instead continues to register SPI slave because lookup.max_speed_hz
> may contain whatever garbage there is in the stack at that address.

Good point.

Jarkko, does this help?

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 50d230b33c42..d072efdd65ba 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1914,6 +1914,7 @@ static acpi_status
acpi_register_spi_device(struct spi_controller *ctlr,
                return AE_OK;

        lookup.ctlr             = ctlr;
+       lookup.max_speed_hz     = 0;
        lookup.mode             = 0;
        lookup.bits_per_word    = 0;
        lookup.irq              = -1;

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Mika Westerberg]

On Thu, Jun 20, 2019 at 12:33:41PM +0200, Ard Biesheuvel wrote:
> Jarkko, does this help?
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 50d230b33c42..d072efdd65ba 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1914,6 +1914,7 @@ static acpi_status
> acpi_register_spi_device(struct spi_controller *ctlr,
>                 return AE_OK;
> 
>         lookup.ctlr             = ctlr;
> +       lookup.max_speed_hz     = 0;
>         lookup.mode             = 0;
>         lookup.bits_per_word    = 0;
>         lookup.irq              = -1;

IMHO it's better to do:

	memset(&lookup, 0, sizeof(lookup));
	lookup.ctlr = ctlr;
	lookup.irq = -1;

this also initializes chip_select and possibly other fields that get
added to the lookup structure later.

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 362 bytes --]

On Thu, Jun 20, 2019 at 01:41:28PM +0300, Mika Westerberg wrote:
> On Thu, Jun 20, 2019 at 12:33:41PM +0200, Ard Biesheuvel wrote:

> IMHO it's better to do:

> 	memset(&lookup, 0, sizeof(lookup));
> 	lookup.ctlr = ctlr;
> 	lookup.irq = -1;

> this also initializes chip_select and possibly other fields that get
> added to the lookup structure later.

I agree.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Ard Biesheuvel]

On Thu, 20 Jun 2019 at 13:19, Mark Brown <broonie@kernel.org> wrote:
>
> On Thu, Jun 20, 2019 at 01:41:28PM +0300, Mika Westerberg wrote:
> > On Thu, Jun 20, 2019 at 12:33:41PM +0200, Ard Biesheuvel wrote:
>
> > IMHO it's better to do:
>
> >       memset(&lookup, 0, sizeof(lookup));
> >       lookup.ctlr = ctlr;
> >       lookup.irq = -1;
>
> > this also initializes chip_select and possibly other fields that get
> > added to the lookup structure later.
>
> I agree.

Sure, I will spin it like that once Jarkko confirms that this fixes his problem.

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Jarkko Nikula]

On 6/20/19 1:33 PM, Ard Biesheuvel wrote:
> Jarkko, does this help?
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index 50d230b33c42..d072efdd65ba 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -1914,6 +1914,7 @@ static acpi_status
> acpi_register_spi_device(struct spi_controller *ctlr,
>                  return AE_OK;
> 
>          lookup.ctlr             = ctlr;
> +       lookup.max_speed_hz     = 0;
>          lookup.mode             = 0;
>          lookup.bits_per_word    = 0;
>          lookup.irq              = -1;
> 
Yes it does.

I guess you have some cleanups or changes on top of your b5e3cf410b48 
("spi/acpi: fix incorrect ACPI parent check") since for me change go 
around lines @@ -1981,6 +1981,7 @@ ?

-- 
Jarkko

Re: [PATCH] spi/acpi: fix incorrect ACPI parent check

[Ard Biesheuvel]

On Thu, 20 Jun 2019 at 14:21, Jarkko Nikula
<jarkko.nikula@linux.intel.com> wrote:
>
> On 6/20/19 1:33 PM, Ard Biesheuvel wrote:
> > Jarkko, does this help?
> >
> > diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> > index 50d230b33c42..d072efdd65ba 100644
> > --- a/drivers/spi/spi.c
> > +++ b/drivers/spi/spi.c
> > @@ -1914,6 +1914,7 @@ static acpi_status
> > acpi_register_spi_device(struct spi_controller *ctlr,
> >                  return AE_OK;
> >
> >          lookup.ctlr             = ctlr;
> > +       lookup.max_speed_hz     = 0;
> >          lookup.mode             = 0;
> >          lookup.bits_per_word    = 0;
> >          lookup.irq              = -1;
> >
> Yes it does.
>
> I guess you have some cleanups or changes on top of your b5e3cf410b48
> ("spi/acpi: fix incorrect ACPI parent check") since for me change go
> around lines @@ -1981,6 +1981,7 @@ ?
>

Thanks,

This is my own tree with just my own 2 patches, not what's in
broonie's tree for spi-next