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

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>,
	linux-spi@vger.kernel.org, Mark Brown <broonie@kernel.org>,
	kbuild test robot <lkp@intel.com>,
	Dan Carpenter <dan.carpenter@oracle.com>,
	Andy Shevchenko <andy.shevchenko@gmail.com>,
	Masahisa Kojima <masahisa.kojima@linaro.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>,
	Lukas Wunner <lukas@wunner.de>
Subject: Re: [PATCH] spi/acpi: fix incorrect ACPI parent check
Date: Wed, 19 Jun 2019 15:58:38 +0200	[thread overview]
Message-ID: <CAKv+Gu9fafXNrAYAgUuqMPVjZm2bWJmzg_aPz9WD1skemQme8A@mail.gmail.com> (raw)
In-Reply-To: <c96afe71-c7bd-d30f-ef37-0c6eeb726f67@linux.intel.com>

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.