drivers/mfd/intel-lpss-pci.c | 36 ++++++++++++++++++++++++++++-------- drivers/mfd/intel-lpss.c | 9 ++++++++- drivers/mfd/intel-lpss.h | 5 ++++- 3 files changed, 40 insertions(+), 10 deletions(-)
Dell XPS 9530 (2023) uses spi-pxa2xx with clock-divider enabled from intel-lpss with the ratio of 1:32767 (Dell's firmware bug). This caps SPI controller's speed at very low value of 3051Hz, which makes the interface practically unusable. Since either spi-pxa2xx or intel-lpss should have clock divider enabled, not both, and SPI controller can have higher speed than requested by the device, it is preffered to disable intel-lpss clock-divider, and let SPI controller handle it. Fixing this issue directly in Dell firmware by setting lpss divider to 1:1 would've been the ideal solution, but is unlikely to ever happen. Particular driver already implements customized solution for handling buggy ACPI tables for select Microsoft devices. This patch series converts it to a more generic quirk table, and adds a new quirk QUIRK_CLOCK_DIVIDER_UNITY which forces clock divider to be set to 1:1. In the future, devices with similar bug (if any) can be easily added to the same pci id lookup table. Aleksandrs Vinarskis (2): mfd: intel-lpss: Switch to generalized quirk table mfd: intel-lpss: Introduce QUIRK_CLOCK_DIVIDER_UNITY for XPS 9530 drivers/mfd/intel-lpss-pci.c | 36 ++++++++++++++++++++++++++++-------- drivers/mfd/intel-lpss.c | 9 ++++++++- drivers/mfd/intel-lpss.h | 5 ++++- 3 files changed, 40 insertions(+), 10 deletions(-) -- 2.40.1
Thanks to Andy Shevchenko for the review and suggestions. Fixed accordingly. --- Dell XPS 9530 (2023) uses spi-pxa2xx with clock-divider enabled from intel-lpss with the ratio of 1:32767 (Dell's firmware bug). This caps SPI controller's speed at very low value of 3051Hz, which makes the interface practically unusable. Since either spi-pxa2xx or intel-lpss should have clock divider enabled, not both, and SPI controller can have higher speed than requested by the device, it is preffered to disable intel-lpss clock-divider, and let SPI controller handle it. Fixing this issue directly in Dell firmware by setting lpss divider to 1:1 would've been the ideal solution, but is unlikely to ever happen. Particular driver already implements customized solution for handling buggy ACPI tables for select Microsoft devices. This patch series converts it to a more generic quirk table, and adds a new quirk QUIRK_CLOCK_DIVIDER_UNITY which forces clock divider to be set to 1:1. In the future, devices with similar bug (if any) can be easily added to the same pci id lookup table. Aleksandrs Vinarskis (2): mfd: intel-lpss: Switch to generalized quirk table mfd: intel-lpss: Introduce QUIRK_CLOCK_DIVIDER_UNITY for XPS 9530 drivers/mfd/intel-lpss-pci.c | 25 ++++++++++++++++--------- drivers/mfd/intel-lpss.c | 9 ++++++++- drivers/mfd/intel-lpss.h | 14 +++++++++++++- 3 files changed, 37 insertions(+), 11 deletions(-) -- 2.40.1
Introduce generic quirk table, and port existing walkaround for select
Microsoft devices to it. This is a preparation for
QUIRK_CLOCK_DIVIDER_UNITY.
Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
---
V1 -> V2: Removed unnecessary intermidiate data structure
drivers/mfd/intel-lpss-pci.c | 22 +++++++++++++---------
drivers/mfd/intel-lpss.c | 2 +-
drivers/mfd/intel-lpss.h | 9 ++++++++-
3 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
index 4621d3950b8f..f8454d802677 100644
--- a/drivers/mfd/intel-lpss-pci.c
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -23,13 +23,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(PCI_VENDOR_ID_INTEL, 0x9d64, 0x152d, 0x1182), },
- /* Microsoft Surface Go 2 I2C4 */
- { PCI_DEVICE_SUB(PCI_VENDOR_ID_INTEL, 0x9d64, 0x152d, 0x1237), },
- { }
+static const struct pci_device_id quirk_ids[] = {
+ { /* Microsoft Surface Go (version 1) I2C4 */
+ PCI_DEVICE_SUB(PCI_VENDOR_ID_INTEL, 0x9d64, 0x152d, 0x1182),
+ .driver_data = QUIRK_IGNORE_RESOURCE_CONFLICTS,
+ }, { /* Microsoft Surface Go 2 I2C4 */
+ PCI_DEVICE_SUB(PCI_VENDOR_ID_INTEL, 0x9d64, 0x152d, 0x1237),
+ .driver_data = QUIRK_IGNORE_RESOURCE_CONFLICTS,
+ },
+ {}
};
static int intel_lpss_pci_probe(struct pci_dev *pdev,
@@ -37,6 +39,7 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
{
const struct intel_lpss_platform_info *data = (void *)id->driver_data;
struct intel_lpss_platform_info *info;
+ const struct pci_device_id *quirk_pci_info;
int ret;
ret = pcim_enable_device(pdev);
@@ -55,8 +58,9 @@ static int intel_lpss_pci_probe(struct pci_dev *pdev,
info->mem = pci_resource_n(pdev, 0);
info->irq = pci_irq_vector(pdev, 0);
- if (pci_match_id(ignore_resource_conflicts_ids, pdev))
- info->ignore_resource_conflicts = true;
+ quirk_pci_info = pci_match_id(quirk_ids, pdev);
+ if (quirk_pci_info)
+ info->quirks = quirk_pci_info->driver_data;
pdev->d3cold_delay = 0;
diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
index eff423f7dd28..aafa0da5f8db 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -412,7 +412,7 @@ int intel_lpss_probe(struct device *dev,
return ret;
lpss->cell->swnode = info->swnode;
- lpss->cell->ignore_resource_conflicts = info->ignore_resource_conflicts;
+ lpss->cell->ignore_resource_conflicts = info->quirks & QUIRK_IGNORE_RESOURCE_CONFLICTS;
intel_lpss_init_dev(lpss);
diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
index c1d72b117ed5..8a1ffd4f3546 100644
--- a/drivers/mfd/intel-lpss.h
+++ b/drivers/mfd/intel-lpss.h
@@ -11,16 +11,23 @@
#ifndef __MFD_INTEL_LPSS_H
#define __MFD_INTEL_LPSS_H
+#include <linux/bits.h>
#include <linux/pm.h>
+/*
+ * Some DSDTs have an unused GEXP ACPI device conflicting with I2C4 resources
+ * Set to ignore resource conflicts with ACPI declared SystemMemory regions
+ */
+#define QUIRK_IGNORE_RESOURCE_CONFLICTS BIT(0)
+
struct device;
struct resource;
struct software_node;
struct intel_lpss_platform_info {
struct resource *mem;
- bool ignore_resource_conflicts;
int irq;
+ unsigned int quirks;
unsigned long clk_rate;
const char *clk_con_id;
const struct software_node *swnode;
--
2.40.1On Wed, Dec 20, 2023 at 09:56:20PM +0100, Aleksandrs Vinarskis wrote:
> Introduce generic quirk table, and port existing walkaround for select
> Microsoft devices to it. This is a preparation for
> QUIRK_CLOCK_DIVIDER_UNITY.
Thank you for the update!
Some nit-picks below, after addressing them feel free to add
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
...
> - { }
> + {}
No need to change here.
Also I'm not sure about "}, {" style. Lee as the maintainer can clarify
on this.
...
> const struct intel_lpss_platform_info *data = (void *)id->driver_data;
> struct intel_lpss_platform_info *info;
> + const struct pci_device_id *quirk_pci_info;
> int ret;
I would preserve reversed xmas tree order:
const struct intel_lpss_platform_info *data = (void *)id->driver_data;
const struct pci_device_id *quirk_pci_info;
struct intel_lpss_platform_info *info;
int ret;
...
> + quirk_pci_info = pci_match_id(quirk_ids, pdev);
> + if (quirk_pci_info)
> + info->quirks = quirk_pci_info->driver_data;
Can you confirm that there is no compiler warning if you compile for 32-bit?
...
> +/*
> + * Some DSDTs have an unused GEXP ACPI device conflicting with I2C4 resources
> + * Set to ignore resource conflicts with ACPI declared SystemMemory regions
Please, mind the periods at the ends of sentences.
> + */
...
> + unsigned int quirks;
Depending on the above (32-bit compilation) this might be converted to
unsigned long.
--
With Best Regards,
Andy Shevchenko
On Thu, Dec 21, 2023 at 02:41:28PM +0200, Andy Shevchenko wrote:
> On Wed, Dec 20, 2023 at 09:56:20PM +0100, Aleksandrs Vinarskis wrote:
...
> Also I'm not sure about "}, {" style. Lee as the maintainer can clarify
> on this.
JFYI:
$ git grep '^[[:space:]]\+}, {$' -- drivers/mfd/ | wc
250 750 8008
$ git grep '^[[:space:]]\+},$' -- drivers/mfd/ | wc
1898 3796 55230
--
With Best Regards,
Andy Shevchenko
Some devices (eg. Dell XPS 9530, 2023) due to a firmware bug have a
misconfigured clock divider, which should've been 1:1. This introduces
quirk which conditionally re-configures the clock divider to 1:1.
Signed-off-by: Aleksandrs Vinarskis <alex.vinarskis@gmail.com>
---
V1 -> V2: Documented the quirk
drivers/mfd/intel-lpss-pci.c | 3 +++
drivers/mfd/intel-lpss.c | 7 +++++++
drivers/mfd/intel-lpss.h | 5 +++++
3 files changed, 15 insertions(+)
diff --git a/drivers/mfd/intel-lpss-pci.c b/drivers/mfd/intel-lpss-pci.c
index f8454d802677..951e28ca5e42 100644
--- a/drivers/mfd/intel-lpss-pci.c
+++ b/drivers/mfd/intel-lpss-pci.c
@@ -30,6 +30,9 @@ static const struct pci_device_id quirk_ids[] = {
}, { /* Microsoft Surface Go 2 I2C4 */
PCI_DEVICE_SUB(PCI_VENDOR_ID_INTEL, 0x9d64, 0x152d, 0x1237),
.driver_data = QUIRK_IGNORE_RESOURCE_CONFLICTS,
+ }, { /* Dell XPS 9530 (2023) */
+ PCI_DEVICE_SUB(PCI_VENDOR_ID_INTEL, 0x51fb, 0x1028, 0x0beb),
+ .driver_data = QUIRK_CLOCK_DIVIDER_UNITY,
},
{}
};
diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
index aafa0da5f8db..2a9018112dfc 100644
--- a/drivers/mfd/intel-lpss.c
+++ b/drivers/mfd/intel-lpss.c
@@ -300,6 +300,7 @@ static int intel_lpss_register_clock_divider(struct intel_lpss *lpss,
{
char name[32];
struct clk *tmp = *clk;
+ int ret;
snprintf(name, sizeof(name), "%s-enable", devname);
tmp = clk_register_gate(NULL, name, __clk_get_name(tmp), 0,
@@ -316,6 +317,12 @@ static int intel_lpss_register_clock_divider(struct intel_lpss *lpss,
return PTR_ERR(tmp);
*clk = tmp;
+ if (lpss->info->quirks & QUIRK_CLOCK_DIVIDER_UNITY) {
+ ret = clk_set_rate(tmp, lpss->info->clk_rate);
+ if (ret)
+ return ret;
+ }
+
snprintf(name, sizeof(name), "%s-update", devname);
tmp = clk_register_gate(NULL, name, __clk_get_name(tmp),
CLK_SET_RATE_PARENT, lpss->priv, 31, 0, NULL);
diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
index 8a1ffd4f3546..8c032177611e 100644
--- a/drivers/mfd/intel-lpss.h
+++ b/drivers/mfd/intel-lpss.h
@@ -19,6 +19,11 @@
* Set to ignore resource conflicts with ACPI declared SystemMemory regions
*/
#define QUIRK_IGNORE_RESOURCE_CONFLICTS BIT(0)
+/*
+ * Some devices have misconfigured clock divider due to a firmware bug
+ * Set this to force the clock divider to 1:1 ratio
+ */
+#define QUIRK_CLOCK_DIVIDER_UNITY BIT(1)
struct device;
struct resource;
--
2.40.1On Wed, Dec 20, 2023 at 09:56:21PM +0100, Aleksandrs Vinarskis wrote: > Some devices (eg. Dell XPS 9530, 2023) due to a firmware bug have a > misconfigured clock divider, which should've been 1:1. This introduces > quirk which conditionally re-configures the clock divider to 1:1. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> ... > + * Some devices have misconfigured clock divider due to a firmware bug > + * Set this to force the clock divider to 1:1 ratio Same comment: Use periods. -- With Best Regards, Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.