RE: [PATCH v5 7/9] platform/x86: serial-multi-instantiate: Add SPI support

Hi,

-----Original Message----- From: Rafael J. Wysocki rafael@kernel.org Sent: 21 January 2022 15:31 To: Stefan Binding sbinding@opensource.cirrus.com Cc: Mark Brown broonie@kernel.org; Rafael J . Wysocki rafael@kernel.org; Len Brown lenb@kernel.org; Hans de Goede hdegoede@redhat.com; Mark Gross markgross@kernel.org; Jaroslav Kysela perex@perex.cz; Takashi Iwai tiwai@suse.com; moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM... <alsa- devel@alsa-project.org>; Linux Kernel Mailing List <linux- kernel@vger.kernel.org>; linux-spi linux-spi@vger.kernel.org; ACPI Devel Maling List linux-acpi@vger.kernel.org; Platform Driver <platform-driver- x86@vger.kernel.org>; patches@opensource.cirrus.com Subject: Re: [PATCH v5 7/9] platform/x86: serial-multi-instantiate: Add SPI support

...

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 5b65d687f046..28f5bbf0f27a 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -991,12 +991,12 @@ config TOPSTAR_LAPTOP If you have a Topstar laptop, say Y or M here.

config SERIAL_MULTI_INSTANTIATE

•   tristate "I2C multi instantiate pseudo device driver"
  

•   depends on I2C && ACPI
  

•   tristate "I2C and SPI multi instantiate pseudo device driver"
  

•   depends on I2C && SPI && ACPI
  

Should this be (I2C || SPI) && ACPI ?

We made it dependent on both I2C and SPI because of how interconnected the serial-multi-instantiate driver is with both SPI and I2C. We felt attempting to make the driver compatible with one without the other would end up very complicated.

...

@@ -146,7 +247,21 @@ static int smi_probe(struct platform_device *pdev)

    platform_set_drvdata(pdev, smi);

•   return smi_i2c_probe(pdev, adev, smi, inst_array);
  

•   switch (node->bus_type) {
  

•   case SMI_I2C:
  

•           return smi_i2c_probe(pdev, adev, smi, node->instances);
  

•   case SMI_SPI:
  

•           return smi_spi_probe(pdev, adev, smi, node->instances);
  

•   case SMI_AUTO_DETECT:
  

•           if (i2c_acpi_client_count(adev) > 0)
  

•                   return smi_i2c_probe(pdev, adev, smi, node->instances);
  

•           else
  

•                   return smi_spi_probe(pdev, adev, smi, node->instances);
  

•   default:
  

•           break;
  

Why is this needed?

This return code is attempting to ensure that we don’t try to guess whether we expect devices to be I2C or SPI - especially with regards to existing devices. We wanted to maintain compatibility with existing devices, which would all be I2C. For the device for which we are adding, the same HID is used by both the same chip for both I2C and SPI, so we also needed a way to support both.

Thanks, Stefan