linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mfd: intel-lpss: Fix I2C4 not being available on the Microsoft Surface Go & Go 2
@ 2021-12-02 20:13 Hans de Goede
  2021-12-02 20:43 ` Laurent Pinchart
  2021-12-02 21:15 ` Andy Shevchenko
  0 siblings, 2 replies; 5+ messages in thread
From: Hans de Goede @ 2021-12-02 20:13 UTC (permalink / raw)
  To: Lee Jones, Andy Shevchenko, Mika Westerberg
  Cc: Hans de Goede, linux-kernel, Laurent Pinchart, Dan Scally,
	Kate Hsuan, Maximilian Luz

Many DSDTs for Kaby Lake and Kaby Lake Refresh models contain a
_SB.PCI0.GEXP ACPI Device node describing an I2C attached GPIO expander.

This seems to be something which is copy and pasted from the DSDT
from some reference design since this ACPI Device is present even on
models where no such GPIO expander is used at all, such as on the
Microsoft Surface Go & Go 2.

This ACPI Device is a problem because it contains a SystemMemory
OperationRegion which covers the MMIO for the I2C4 I2C controller this
causes the MFD cell for the I2C4 controller to not be instantiated due
to a resource conflict, requiring the use of acpi_enforce_resources=lax
to work around this.

I have done an extensive analysis of all the ACPI tables on the
Microsoft Surface Go and the _SB.PCI0.GEXP ACPI Device's methods are
not used by any code in the ACPI tables, neither are any of them
directly called by any Linux kernel code. This is unsurprising since
running i2cdetect on the I2C4 bus shows that there is no GPIO
expander chip present on these devices at all.

This commit adds a PCI subsystem vendor:device table listing PCI devices
where it is known to be safe to ignore a resource-conflicts with ACPI
declared SystemMemory regions.

This makes the I2C4 bus work out of the box on the Microsoft Surface
Go & Go 2, which is necessary for the cameras on these devices to work.

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Dan Scally <djrscally@gmail.com>
Cc: Kate Hsuan <hpa@redhat.com>
Cc: Maximilian Luz <luzmaximilian@gmail.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/mfd/intel-lpss-pci.c | 12 ++++++++++++
 drivers/mfd/intel-lpss.c     |  1 +
 drivers/mfd/intel-lpss.h     |  1 +
 3 files changed, 14 insertions(+)

diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
index a872b4485eac..593290ff08a6 100644
--- a/drivers/mfd/intel-lpss-pci.c
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -17,6 +17,15 @@
 
 #include "intel-lpss.h"
 
+/* Some DSDTs have an unused GEXP ACPI device conflicting with I2C4 resources */
+static const struct pci_device_id ignore_resource_conflicts_ids[] = {
+	/* Microsoft Surface Go (version 1) I2C4 */
+	{ PCI_DEVICE_SUB(0x8086, 0x9d64, 0x152d, 0x1182), },
+	/* Microsoft Surface Go 2 I2C4 */
+	{ PCI_DEVICE_SUB(0x8086, 0x9d64, 0x152d, 0x1237), },
+	{ }
+};
+
 static int intel_lpss_pci_probe(struct pci_dev *pdev,
 				const struct pci_device_id *id)
 {
@@ -35,6 +44,9 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
 	info->mem = &pdev->resource[0];
 	info->irq = pdev->irq;
 
+	if (pci_match_id(ignore_resource_conflicts_ids, pdev))
+		info->ignore_resource_conflicts = true;
+
 	pdev->d3cold_delay = 0;
 
 	/* Probably it is enough to set this for iDMA capable devices only */
diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
index 0e15afc39f54..cfbee2cfba6b 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -401,6 +401,7 @@ int intel_lpss_probe(struct device *dev,
 		return ret;
 
 	lpss->cell->swnode = info->swnode;
+	lpss->cell->ignore_resource_conflicts = info->ignore_resource_conflicts;
 
 	intel_lpss_init_dev(lpss);
 
diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
index 22dbc4aed793..062ce95b68b9 100644
--- a/drivers/mfd/intel-lpss.h
+++ b/drivers/mfd/intel-lpss.h
@@ -19,6 +19,7 @@ struct software_node;
 
 struct intel_lpss_platform_info {
 	struct resource *mem;
+	bool ignore_resource_conflicts;
 	int irq;
 	unsigned long clk_rate;
 	const char *clk_con_id;
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] mfd: intel-lpss: Fix I2C4 not being available on the Microsoft Surface Go & Go 2
  2021-12-02 20:13 [PATCH] mfd: intel-lpss: Fix I2C4 not being available on the Microsoft Surface Go & Go 2 Hans de Goede
@ 2021-12-02 20:43 ` Laurent Pinchart
  2021-12-03 11:29   ` Hans de Goede
  2021-12-02 21:15 ` Andy Shevchenko
  1 sibling, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2021-12-02 20:43 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Andy Shevchenko, Mika Westerberg, linux-kernel,
	Dan Scally, Kate Hsuan, Maximilian Luz

Hi Hans,

Thank you for the patch.

On Thu, Dec 02, 2021 at 09:13:51PM +0100, Hans de Goede wrote:
> Many DSDTs for Kaby Lake and Kaby Lake Refresh models contain a
> _SB.PCI0.GEXP ACPI Device node describing an I2C attached GPIO expander.
> 
> This seems to be something which is copy and pasted from the DSDT
> from some reference design since this ACPI Device is present even on
> models where no such GPIO expander is used at all, such as on the
> Microsoft Surface Go & Go 2.
> 
> This ACPI Device is a problem because it contains a SystemMemory
> OperationRegion which covers the MMIO for the I2C4 I2C controller this

s/this/that/ (or ". This" ?)

> causes the MFD cell for the I2C4 controller to not be instantiated due
> to a resource conflict, requiring the use of acpi_enforce_resources=lax
> to work around this.
> 
> I have done an extensive analysis of all the ACPI tables on the
> Microsoft Surface Go and the _SB.PCI0.GEXP ACPI Device's methods are
> not used by any code in the ACPI tables, neither are any of them
> directly called by any Linux kernel code. This is unsurprising since
> running i2cdetect on the I2C4 bus shows that there is no GPIO
> expander chip present on these devices at all.
> 
> This commit adds a PCI subsystem vendor:device table listing PCI devices
> where it is known to be safe to ignore a resource-conflicts with ACPI
> declared SystemMemory regions.
> 
> This makes the I2C4 bus work out of the box on the Microsoft Surface
> Go & Go 2, which is necessary for the cameras on these devices to work.

This will make the life of the Go and Go2 users *much* easier.

> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Dan Scally <djrscally@gmail.com>
> Cc: Kate Hsuan <hpa@redhat.com>
> Cc: Maximilian Luz <luzmaximilian@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mfd/intel-lpss-pci.c | 12 ++++++++++++
>  drivers/mfd/intel-lpss.c     |  1 +
>  drivers/mfd/intel-lpss.h     |  1 +
>  3 files changed, 14 insertions(+)
> 
> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> index a872b4485eac..593290ff08a6 100644
> --- a/drivers/mfd/intel-lpss-pci.c
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -17,6 +17,15 @@
>  
>  #include "intel-lpss.h"
>  
> +/* Some DSDTs have an unused GEXP ACPI device conflicting with I2C4 resources */
> +static const struct pci_device_id ignore_resource_conflicts_ids[] = {
> +	/* Microsoft Surface Go (version 1) I2C4 */
> +	{ PCI_DEVICE_SUB(0x8086, 0x9d64, 0x152d, 0x1182), },
> +	/* Microsoft Surface Go 2 I2C4 */
> +	{ PCI_DEVICE_SUB(0x8086, 0x9d64, 0x152d, 0x1237), },
> +	{ }

Reading the commit message, I was going to mention that the match should
probably be machine-dependent, but that seems to be covered by the PCI
subsystem ID.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +};
> +
>  static int intel_lpss_pci_probe(struct pci_dev *pdev,
>  				const struct pci_device_id *id)
>  {
> @@ -35,6 +44,9 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
>  	info->mem = &pdev->resource[0];
>  	info->irq = pdev->irq;
>  
> +	if (pci_match_id(ignore_resource_conflicts_ids, pdev))
> +		info->ignore_resource_conflicts = true;
> +
>  	pdev->d3cold_delay = 0;
>  
>  	/* Probably it is enough to set this for iDMA capable devices only */
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 0e15afc39f54..cfbee2cfba6b 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -401,6 +401,7 @@ int intel_lpss_probe(struct device *dev,
>  		return ret;
>  
>  	lpss->cell->swnode = info->swnode;
> +	lpss->cell->ignore_resource_conflicts = info->ignore_resource_conflicts;
>  
>  	intel_lpss_init_dev(lpss);
>  
> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
> index 22dbc4aed793..062ce95b68b9 100644
> --- a/drivers/mfd/intel-lpss.h
> +++ b/drivers/mfd/intel-lpss.h
> @@ -19,6 +19,7 @@ struct software_node;
>  
>  struct intel_lpss_platform_info {
>  	struct resource *mem;
> +	bool ignore_resource_conflicts;
>  	int irq;
>  	unsigned long clk_rate;
>  	const char *clk_con_id;

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mfd: intel-lpss: Fix I2C4 not being available on the Microsoft Surface Go & Go 2
  2021-12-02 20:13 [PATCH] mfd: intel-lpss: Fix I2C4 not being available on the Microsoft Surface Go & Go 2 Hans de Goede
  2021-12-02 20:43 ` Laurent Pinchart
@ 2021-12-02 21:15 ` Andy Shevchenko
  2021-12-03 11:32   ` Hans de Goede
  1 sibling, 1 reply; 5+ messages in thread
From: Andy Shevchenko @ 2021-12-02 21:15 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Lee Jones, Andy Shevchenko, Mika Westerberg,
	Linux Kernel Mailing List, Laurent Pinchart, Dan Scally,
	Kate Hsuan, Maximilian Luz

On Thu, Dec 2, 2021 at 10:14 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Many DSDTs for Kaby Lake and Kaby Lake Refresh models contain a
> _SB.PCI0.GEXP ACPI Device node describing an I2C attached GPIO expander.

For the record (and probably good to mention it's here) it's a PCA953x
compatible one.

> This seems to be something which is copy and pasted from the DSDT
> from some reference design since this ACPI Device is present even on
> models where no such GPIO expander is used at all, such as on the
> Microsoft Surface Go & Go 2.

Does this come from schematics? Or..? Ah, I see below.

> This ACPI Device is a problem because it contains a SystemMemory
> OperationRegion which covers the MMIO for the I2C4 I2C controller this
> causes the MFD cell for the I2C4 controller to not be instantiated due
> to a resource conflict, requiring the use of acpi_enforce_resources=lax
> to work around this.

Right.

> I have done an extensive analysis of all the ACPI tables on the
> Microsoft Surface Go and the _SB.PCI0.GEXP ACPI Device's methods are
> not used by any code in the ACPI tables, neither are any of them
> directly called by any Linux kernel code. This is unsurprising since
> running i2cdetect on the I2C4 bus shows that there is no GPIO
> expander chip present on these devices at all.

I believe it's an optional component on our reference designs (I saw a
lot of those expanders on our development boards).

> This commit adds a PCI subsystem vendor:device table listing PCI devices
> where it is known to be safe to ignore a resource-conflicts with ACPI

ignore resource conflicts

(no article, no dash) ?

> declared SystemMemory regions.
>
> This makes the I2C4 bus work out of the box on the Microsoft Surface
> Go & Go 2, which is necessary for the cameras on these devices to work.

After addressing comments
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Thanks for the patch!

> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Dan Scally <djrscally@gmail.com>
> Cc: Kate Hsuan <hpa@redhat.com>
> Cc: Maximilian Luz <luzmaximilian@gmail.com>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  drivers/mfd/intel-lpss-pci.c | 12 ++++++++++++
>  drivers/mfd/intel-lpss.c     |  1 +
>  drivers/mfd/intel-lpss.h     |  1 +
>  3 files changed, 14 insertions(+)
>
> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
> index a872b4485eac..593290ff08a6 100644
> --- a/drivers/mfd/intel-lpss-pci.c
> +++ b/drivers/mfd/intel-lpss-pci.c
> @@ -17,6 +17,15 @@
>
>  #include "intel-lpss.h"
>
> +/* Some DSDTs have an unused GEXP ACPI device conflicting with I2C4 resources */
> +static const struct pci_device_id ignore_resource_conflicts_ids[] = {
> +       /* Microsoft Surface Go (version 1) I2C4 */
> +       { PCI_DEVICE_SUB(0x8086, 0x9d64, 0x152d, 0x1182), },
> +       /* Microsoft Surface Go 2 I2C4 */
> +       { PCI_DEVICE_SUB(0x8086, 0x9d64, 0x152d, 0x1237), },

Can we use PCI_VENDOR_ID_INTEL?

> +       { }
> +};
> +
>  static int intel_lpss_pci_probe(struct pci_dev *pdev,
>                                 const struct pci_device_id *id)
>  {
> @@ -35,6 +44,9 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
>         info->mem = &pdev->resource[0];
>         info->irq = pdev->irq;
>
> +       if (pci_match_id(ignore_resource_conflicts_ids, pdev))
> +               info->ignore_resource_conflicts = true;
> +
>         pdev->d3cold_delay = 0;
>
>         /* Probably it is enough to set this for iDMA capable devices only */
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index 0e15afc39f54..cfbee2cfba6b 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -401,6 +401,7 @@ int intel_lpss_probe(struct device *dev,
>                 return ret;
>
>         lpss->cell->swnode = info->swnode;
> +       lpss->cell->ignore_resource_conflicts = info->ignore_resource_conflicts;
>
>         intel_lpss_init_dev(lpss);
>
> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
> index 22dbc4aed793..062ce95b68b9 100644
> --- a/drivers/mfd/intel-lpss.h
> +++ b/drivers/mfd/intel-lpss.h
> @@ -19,6 +19,7 @@ struct software_node;
>
>  struct intel_lpss_platform_info {
>         struct resource *mem;
> +       bool ignore_resource_conflicts;
>         int irq;
>         unsigned long clk_rate;
>         const char *clk_con_id;
> --
> 2.33.1
>


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mfd: intel-lpss: Fix I2C4 not being available on the Microsoft Surface Go & Go 2
  2021-12-02 20:43 ` Laurent Pinchart
@ 2021-12-03 11:29   ` Hans de Goede
  0 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-12-03 11:29 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Lee Jones, Andy Shevchenko, Mika Westerberg, linux-kernel,
	Dan Scally, Kate Hsuan, Maximilian Luz

Hi Laurent,

On 12/2/21 21:43, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Thu, Dec 02, 2021 at 09:13:51PM +0100, Hans de Goede wrote:
>> Many DSDTs for Kaby Lake and Kaby Lake Refresh models contain a
>> _SB.PCI0.GEXP ACPI Device node describing an I2C attached GPIO expander.
>>
>> This seems to be something which is copy and pasted from the DSDT
>> from some reference design since this ACPI Device is present even on
>> models where no such GPIO expander is used at all, such as on the
>> Microsoft Surface Go & Go 2.
>>
>> This ACPI Device is a problem because it contains a SystemMemory
>> OperationRegion which covers the MMIO for the I2C4 I2C controller this
> 
> s/this/that/ (or ". This" ?)

I've gone with ". This" for v2.


> 
>> causes the MFD cell for the I2C4 controller to not be instantiated due
>> to a resource conflict, requiring the use of acpi_enforce_resources=lax
>> to work around this.
>>
>> I have done an extensive analysis of all the ACPI tables on the
>> Microsoft Surface Go and the _SB.PCI0.GEXP ACPI Device's methods are
>> not used by any code in the ACPI tables, neither are any of them
>> directly called by any Linux kernel code. This is unsurprising since
>> running i2cdetect on the I2C4 bus shows that there is no GPIO
>> expander chip present on these devices at all.
>>
>> This commit adds a PCI subsystem vendor:device table listing PCI devices
>> where it is known to be safe to ignore a resource-conflicts with ACPI
>> declared SystemMemory regions.
>>
>> This makes the I2C4 bus work out of the box on the Microsoft Surface
>> Go & Go 2, which is necessary for the cameras on these devices to work.
> 
> This will make the life of the Go and Go2 users *much* easier.
> 
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Dan Scally <djrscally@gmail.com>
>> Cc: Kate Hsuan <hpa@redhat.com>
>> Cc: Maximilian Luz <luzmaximilian@gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/mfd/intel-lpss-pci.c | 12 ++++++++++++
>>  drivers/mfd/intel-lpss.c     |  1 +
>>  drivers/mfd/intel-lpss.h     |  1 +
>>  3 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
>> index a872b4485eac..593290ff08a6 100644
>> --- a/drivers/mfd/intel-lpss-pci.c
>> +++ b/drivers/mfd/intel-lpss-pci.c
>> @@ -17,6 +17,15 @@
>>  
>>  #include "intel-lpss.h"
>>  
>> +/* Some DSDTs have an unused GEXP ACPI device conflicting with I2C4 resources */
>> +static const struct pci_device_id ignore_resource_conflicts_ids[] = {
>> +	/* Microsoft Surface Go (version 1) I2C4 */
>> +	{ PCI_DEVICE_SUB(0x8086, 0x9d64, 0x152d, 0x1182), },
>> +	/* Microsoft Surface Go 2 I2C4 */
>> +	{ PCI_DEVICE_SUB(0x8086, 0x9d64, 0x152d, 0x1237), },
>> +	{ }
> 
> Reading the commit message, I was going to mention that the match should
> probably be machine-dependent, but that seems to be covered by the PCI
> subsystem ID.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks you.

Regards,

Hans


> 
>> +};
>> +
>>  static int intel_lpss_pci_probe(struct pci_dev *pdev,
>>  				const struct pci_device_id *id)
>>  {
>> @@ -35,6 +44,9 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
>>  	info->mem = &pdev->resource[0];
>>  	info->irq = pdev->irq;
>>  
>> +	if (pci_match_id(ignore_resource_conflicts_ids, pdev))
>> +		info->ignore_resource_conflicts = true;
>> +
>>  	pdev->d3cold_delay = 0;
>>  
>>  	/* Probably it is enough to set this for iDMA capable devices only */
>> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
>> index 0e15afc39f54..cfbee2cfba6b 100644
>> --- a/drivers/mfd/intel-lpss.c
>> +++ b/drivers/mfd/intel-lpss.c
>> @@ -401,6 +401,7 @@ int intel_lpss_probe(struct device *dev,
>>  		return ret;
>>  
>>  	lpss->cell->swnode = info->swnode;
>> +	lpss->cell->ignore_resource_conflicts = info->ignore_resource_conflicts;
>>  
>>  	intel_lpss_init_dev(lpss);
>>  
>> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
>> index 22dbc4aed793..062ce95b68b9 100644
>> --- a/drivers/mfd/intel-lpss.h
>> +++ b/drivers/mfd/intel-lpss.h
>> @@ -19,6 +19,7 @@ struct software_node;
>>  
>>  struct intel_lpss_platform_info {
>>  	struct resource *mem;
>> +	bool ignore_resource_conflicts;
>>  	int irq;
>>  	unsigned long clk_rate;
>>  	const char *clk_con_id;
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] mfd: intel-lpss: Fix I2C4 not being available on the Microsoft Surface Go & Go 2
  2021-12-02 21:15 ` Andy Shevchenko
@ 2021-12-03 11:32   ` Hans de Goede
  0 siblings, 0 replies; 5+ messages in thread
From: Hans de Goede @ 2021-12-03 11:32 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Lee Jones, Andy Shevchenko, Mika Westerberg,
	Linux Kernel Mailing List, Laurent Pinchart, Dan Scally,
	Kate Hsuan, Maximilian Luz

Hi,

On 12/2/21 22:15, Andy Shevchenko wrote:
> On Thu, Dec 2, 2021 at 10:14 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Many DSDTs for Kaby Lake and Kaby Lake Refresh models contain a
>> _SB.PCI0.GEXP ACPI Device node describing an I2C attached GPIO expander.
> 
> For the record (and probably good to mention it's here) it's a PCA953x
> compatible one.

Ack, added to the commit msg for v2.

>> This seems to be something which is copy and pasted from the DSDT
>> from some reference design since this ACPI Device is present even on
>> models where no such GPIO expander is used at all, such as on the
>> Microsoft Surface Go & Go 2.
> 
> Does this come from schematics? Or..? Ah, I see below.
> 
>> This ACPI Device is a problem because it contains a SystemMemory
>> OperationRegion which covers the MMIO for the I2C4 I2C controller this
>> causes the MFD cell for the I2C4 controller to not be instantiated due
>> to a resource conflict, requiring the use of acpi_enforce_resources=lax
>> to work around this.
> 
> Right.
> 
>> I have done an extensive analysis of all the ACPI tables on the
>> Microsoft Surface Go and the _SB.PCI0.GEXP ACPI Device's methods are
>> not used by any code in the ACPI tables, neither are any of them
>> directly called by any Linux kernel code. This is unsurprising since
>> running i2cdetect on the I2C4 bus shows that there is no GPIO
>> expander chip present on these devices at all.
> 
> I believe it's an optional component on our reference designs (I saw a
> lot of those expanders on our development boards).
> 
>> This commit adds a PCI subsystem vendor:device table listing PCI devices
>> where it is known to be safe to ignore a resource-conflicts with ACPI
> 
> ignore resource conflicts

Ack, fixed for v2.


> (no article, no dash) ?
> 
>> declared SystemMemory regions.
>>
>> This makes the I2C4 bus work out of the box on the Microsoft Surface
>> Go & Go 2, which is necessary for the cameras on these devices to work.
> 
> After addressing comments
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Thanks for the patch!
> 
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Cc: Dan Scally <djrscally@gmail.com>
>> Cc: Kate Hsuan <hpa@redhat.com>
>> Cc: Maximilian Luz <luzmaximilian@gmail.com>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>>  drivers/mfd/intel-lpss-pci.c | 12 ++++++++++++
>>  drivers/mfd/intel-lpss.c     |  1 +
>>  drivers/mfd/intel-lpss.h     |  1 +
>>  3 files changed, 14 insertions(+)
>>
>> diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
>> index a872b4485eac..593290ff08a6 100644
>> --- a/drivers/mfd/intel-lpss-pci.c
>> +++ b/drivers/mfd/intel-lpss-pci.c
>> @@ -17,6 +17,15 @@
>>
>>  #include "intel-lpss.h"
>>
>> +/* Some DSDTs have an unused GEXP ACPI device conflicting with I2C4 resources */
>> +static const struct pci_device_id ignore_resource_conflicts_ids[] = {
>> +       /* Microsoft Surface Go (version 1) I2C4 */
>> +       { PCI_DEVICE_SUB(0x8086, 0x9d64, 0x152d, 0x1182), },
>> +       /* Microsoft Surface Go 2 I2C4 */
>> +       { PCI_DEVICE_SUB(0x8086, 0x9d64, 0x152d, 0x1237), },
> 
> Can we use PCI_VENDOR_ID_INTEL?

Done for v2.

Thank you for the review.

Regards,

Hans

> 
>> +       { }
>> +};
>> +
>>  static int intel_lpss_pci_probe(struct pci_dev *pdev,
>>                                 const struct pci_device_id *id)
>>  {
>> @@ -35,6 +44,9 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
>>         info->mem = &pdev->resource[0];
>>         info->irq = pdev->irq;
>>
>> +       if (pci_match_id(ignore_resource_conflicts_ids, pdev))
>> +               info->ignore_resource_conflicts = true;
>> +
>>         pdev->d3cold_delay = 0;
>>
>>         /* Probably it is enough to set this for iDMA capable devices only */
>> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
>> index 0e15afc39f54..cfbee2cfba6b 100644
>> --- a/drivers/mfd/intel-lpss.c
>> +++ b/drivers/mfd/intel-lpss.c
>> @@ -401,6 +401,7 @@ int intel_lpss_probe(struct device *dev,
>>                 return ret;
>>
>>         lpss->cell->swnode = info->swnode;
>> +       lpss->cell->ignore_resource_conflicts = info->ignore_resource_conflicts;
>>
>>         intel_lpss_init_dev(lpss);
>>
>> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
>> index 22dbc4aed793..062ce95b68b9 100644
>> --- a/drivers/mfd/intel-lpss.h
>> +++ b/drivers/mfd/intel-lpss.h
>> @@ -19,6 +19,7 @@ struct software_node;
>>
>>  struct intel_lpss_platform_info {
>>         struct resource *mem;
>> +       bool ignore_resource_conflicts;
>>         int irq;
>>         unsigned long clk_rate;
>>         const char *clk_con_id;
>> --
>> 2.33.1
>>
> 
> 


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-12-03 11:32 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-02 20:13 [PATCH] mfd: intel-lpss: Fix I2C4 not being available on the Microsoft Surface Go & Go 2 Hans de Goede
2021-12-02 20:43 ` Laurent Pinchart
2021-12-03 11:29   ` Hans de Goede
2021-12-02 21:15 ` Andy Shevchenko
2021-12-03 11:32   ` Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).