2023-10-30 14:11:31

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 0/3] pinctrl: intel: Add generic platform driver

New Intel platforms one-by-one will be switching to use generic
approach for the pin control description in ACPI, hence this driver.

It depends on the patches in current Linux Next (will be part of
v6.7-rc1) and this series
https://lore.kernel.org/r/[email protected].

Andy Shevchenko (3):
pinctrl: intel: Revert "Unexport intel_pinctrl_probe()"
pinctrl: intel: Add a generic Intel pin control platform driver
pinctrl: intel: Add ACPI ID to the generic platform driver

drivers/pinctrl/intel/Kconfig | 8 +
drivers/pinctrl/intel/Makefile | 1 +
.../pinctrl/intel/pinctrl-intel-platform.c | 224 ++++++++++++++++++
drivers/pinctrl/intel/pinctrl-intel.c | 5 +-
drivers/pinctrl/intel/pinctrl-intel.h | 3 +
5 files changed, 239 insertions(+), 2 deletions(-)
create mode 100644 drivers/pinctrl/intel/pinctrl-intel-platform.c

--
2.40.0.1.gaa8946217a0b


2023-10-30 14:11:44

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 2/3] pinctrl: intel: Add a generic Intel pin control platform driver

New generations of Intel platforms will provide better description
of the pin control devices in the ACPI tables. Hence, we may provide
a generic pin control platform driver to cover all of them.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/intel/Kconfig | 8 +
drivers/pinctrl/intel/Makefile | 1 +
.../pinctrl/intel/pinctrl-intel-platform.c | 223 ++++++++++++++++++
3 files changed, 232 insertions(+)
create mode 100644 drivers/pinctrl/intel/pinctrl-intel-platform.c

diff --git a/drivers/pinctrl/intel/Kconfig b/drivers/pinctrl/intel/Kconfig
index d66f4f6932d8..42a6bc8b7a45 100644
--- a/drivers/pinctrl/intel/Kconfig
+++ b/drivers/pinctrl/intel/Kconfig
@@ -37,6 +37,14 @@ config PINCTRL_INTEL
select GPIOLIB
select GPIOLIB_IRQCHIP

+config PINCTRL_INTEL_PLATFORM
+ tristate "Intel pinctrl and GPIO platform driver"
+ depends on ACPI
+ select PINCTRL_INTEL
+ help
+ This pinctrl driver provides an interface that allows configuring
+ of Intel PCH pins and using them as GPIOs.
+
config PINCTRL_ALDERLAKE
tristate "Intel Alder Lake pinctrl and GPIO driver"
select PINCTRL_INTEL
diff --git a/drivers/pinctrl/intel/Makefile b/drivers/pinctrl/intel/Makefile
index f6d30f2d973a..96c93ed4bd58 100644
--- a/drivers/pinctrl/intel/Makefile
+++ b/drivers/pinctrl/intel/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_PINCTRL_TANGIER) += pinctrl-tangier.o
obj-$(CONFIG_PINCTRL_MERRIFIELD) += pinctrl-merrifield.o
obj-$(CONFIG_PINCTRL_MOOREFIELD) += pinctrl-moorefield.o
obj-$(CONFIG_PINCTRL_INTEL) += pinctrl-intel.o
+obj-$(CONFIG_PINCTRL_INTEL_PLATFORM) += pinctrl-intel-platform.o
obj-$(CONFIG_PINCTRL_ALDERLAKE) += pinctrl-alderlake.o
obj-$(CONFIG_PINCTRL_BROXTON) += pinctrl-broxton.o
obj-$(CONFIG_PINCTRL_CANNONLAKE) += pinctrl-cannonlake.o
diff --git a/drivers/pinctrl/intel/pinctrl-intel-platform.c b/drivers/pinctrl/intel/pinctrl-intel-platform.c
new file mode 100644
index 000000000000..2305d8befdd3
--- /dev/null
+++ b/drivers/pinctrl/intel/pinctrl-intel-platform.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Intel PCH pinctrl/GPIO driver
+ *
+ * Copyright (C) 2021-2023, Intel Corporation
+ * Author: Andy Shevchenko <[email protected]>
+ */
+
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/property.h>
+#include <linux/string_helpers.h>
+
+#include <linux/pinctrl/pinctrl.h>
+
+#include "pinctrl-intel.h"
+
+struct intel_platform_pins {
+ struct pinctrl_pin_desc *pins;
+ size_t npins;
+};
+
+static int intel_platform_pinctrl_prepare_pins(struct device *dev, size_t base,
+ const char *name, u32 size,
+ struct intel_platform_pins *pins)
+{
+ struct pinctrl_pin_desc *descs;
+ char **pin_names;
+ unsigned int i;
+
+ pin_names = devm_kasprintf_strarray(dev, name, size);
+ if (IS_ERR(pin_names))
+ return PTR_ERR(pin_names);
+
+ descs = devm_krealloc_array(dev, pins->pins, base + size, sizeof(*descs), GFP_KERNEL);
+ if (!descs)
+ return -ENOMEM;
+
+ for (i = 0; i < size; i++) {
+ unsigned int pin_number = base + i;
+ char *pin_name = pin_names[i];
+ struct pinctrl_pin_desc *desc;
+
+ /* Unify delimiter for pin name */
+ strreplace(pin_name, '-', '_');
+
+ desc = &descs[pin_number];
+ desc->number = pin_number;
+ desc->name = pin_name;
+ }
+
+ pins->pins = descs;
+ pins->npins = base + size;
+
+ return 0;
+}
+
+static int intel_platform_pinctrl_prepare_group(struct device *dev,
+ struct fwnode_handle *child,
+ struct intel_padgroup *gpp,
+ struct intel_platform_pins *pins)
+{
+ size_t base = pins->npins;
+ const char *name;
+ u32 size;
+ int ret;
+
+ ret = fwnode_property_read_string(child, "intc-gpio-group-name", &name);
+ if (ret)
+ return ret;
+
+ ret = fwnode_property_read_u32(child, "intc-gpio-pad-count", &size);
+ if (ret)
+ return ret;
+
+ ret = intel_platform_pinctrl_prepare_pins(dev, base, name, size, pins);
+ if (ret)
+ return ret;
+
+ gpp->base = base;
+ gpp->size = size;
+ gpp->gpio_base = INTEL_GPIO_BASE_MATCH;
+
+ return 0;
+}
+
+static int intel_platform_pinctrl_prepare_community(struct device *dev,
+ struct intel_community *community,
+ struct intel_platform_pins *pins)
+{
+ struct fwnode_handle *child;
+ struct intel_padgroup *gpps;
+ unsigned int group;
+ size_t ngpps;
+ u32 offset;
+ int ret;
+
+ ret = device_property_read_u32(dev, "intc-gpio-pad-ownership-offset", &offset);
+ if (ret)
+ return ret;
+ community->padown_offset = offset;
+
+ ret = device_property_read_u32(dev, "intc-gpio-pad-configuration-lock-offset", &offset);
+ if (ret)
+ return ret;
+ community->padcfglock_offset = offset;
+
+ ret = device_property_read_u32(dev, "intc-gpio-host-software-pad-ownership-offset", &offset);
+ if (ret)
+ return ret;
+ community->hostown_offset = offset;
+
+ ret = device_property_read_u32(dev, "intc-gpio-gpi-interrupt-status-offset", &offset);
+ if (ret)
+ return ret;
+ community->is_offset = offset;
+
+ ret = device_property_read_u32(dev, "intc-gpio-gpi-interrupt-enable-offset", &offset);
+ if (ret)
+ return ret;
+ community->ie_offset = offset;
+
+ ngpps = device_get_child_node_count(dev);
+ if (ngpps == 0)
+ return -ENODEV;
+
+ gpps = devm_kcalloc(dev, ngpps, sizeof(*gpps), GFP_KERNEL);
+ if (!gpps)
+ return -ENOMEM;
+
+ group = 0;
+ device_for_each_child_node(dev, child) {
+ struct intel_padgroup *gpp = &gpps[group];
+
+ gpp->reg_num = group;
+
+ ret = intel_platform_pinctrl_prepare_group(dev, child, gpp, pins);
+ if (ret)
+ return ret;
+
+ group++;
+ }
+
+ community->ngpps = ngpps;
+ community->gpps = gpps;
+
+ return 0;
+}
+
+static int intel_platform_pinctrl_prepare_soc_data(struct device *dev,
+ struct intel_pinctrl_soc_data *data)
+{
+ struct intel_platform_pins pins = {};
+ struct intel_community *communities;
+ size_t ncommunities;
+ unsigned int i;
+ int ret;
+
+ ncommunities = 1,
+ communities = devm_kcalloc(dev, ncommunities, sizeof(*communities), GFP_KERNEL);
+ if (!communities)
+ return -ENOMEM;
+
+ for (i = 0; i < ncommunities; i++) {
+ struct intel_community *community = &communities[i];
+
+ community->barno = i;
+ community->pin_base = pins.npins;
+
+ ret = intel_platform_pinctrl_prepare_community(dev, community, &pins);
+ if (ret)
+ return ret;
+
+ community->npins = pins.npins - community->pin_base;
+ }
+
+ data->ncommunities = ncommunities;
+ data->communities = communities;
+
+ data->npins = pins.npins;
+ data->pins = pins.pins;
+
+ return 0;
+}
+
+static int intel_platform_pinctrl_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct intel_pinctrl_soc_data *data;
+ int ret;
+
+ data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ ret = intel_platform_pinctrl_prepare_soc_data(dev, data);
+ if (ret)
+ return ret;
+
+ return intel_pinctrl_probe(pdev, data);
+}
+
+static const struct acpi_device_id intel_platform_pinctrl_acpi_match[] = {
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, intel_platform_pinctrl_acpi_match);
+
+static struct platform_driver intel_platform_pinctrl_driver = {
+ .probe = intel_platform_pinctrl_probe,
+ .driver = {
+ .name = "intel-pinctrl",
+ .acpi_match_table = intel_platform_pinctrl_acpi_match,
+ .pm = pm_sleep_ptr(&intel_pinctrl_pm_ops),
+ },
+};
+module_platform_driver(intel_platform_pinctrl_driver);
+
+MODULE_AUTHOR("Andy Shevchenko <[email protected]>");
+MODULE_DESCRIPTION("Intel PCH pinctrl/GPIO driver");
+MODULE_LICENSE("GPL v2");
+MODULE_IMPORT_NS(PINCTRL_INTEL);
--
2.40.0.1.gaa8946217a0b

2023-10-30 14:17:07

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v1 1/3] pinctrl: intel: Revert "Unexport intel_pinctrl_probe()"

In order to prepare for a new coming driver export the original
intel_pinctrl_probe() again.

This reverts commit 0dd519e3784b13befa1cdfeff847a0885b06650f.

Signed-off-by: Andy Shevchenko <[email protected]>
---
drivers/pinctrl/intel/pinctrl-intel.c | 5 +++--
drivers/pinctrl/intel/pinctrl-intel.h | 3 +++
2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/intel/pinctrl-intel.c b/drivers/pinctrl/intel/pinctrl-intel.c
index f1d8f7e0d9b7..f04781915631 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.c
+++ b/drivers/pinctrl/intel/pinctrl-intel.c
@@ -1506,8 +1506,8 @@ static int intel_pinctrl_probe_pwm(struct intel_pinctrl *pctrl,
return PTR_ERR_OR_ZERO(pwm);
}

-static int intel_pinctrl_probe(struct platform_device *pdev,
- const struct intel_pinctrl_soc_data *soc_data)
+int intel_pinctrl_probe(struct platform_device *pdev,
+ const struct intel_pinctrl_soc_data *soc_data)
{
struct device *dev = &pdev->dev;
struct intel_pinctrl *pctrl;
@@ -1625,6 +1625,7 @@ static int intel_pinctrl_probe(struct platform_device *pdev,

return 0;
}
+EXPORT_SYMBOL_NS_GPL(intel_pinctrl_probe, PINCTRL_INTEL);

int intel_pinctrl_probe_by_hid(struct platform_device *pdev)
{
diff --git a/drivers/pinctrl/intel/pinctrl-intel.h b/drivers/pinctrl/intel/pinctrl-intel.h
index e7d911a65584..fde65e18cd14 100644
--- a/drivers/pinctrl/intel/pinctrl-intel.h
+++ b/drivers/pinctrl/intel/pinctrl-intel.h
@@ -252,6 +252,9 @@ struct intel_pinctrl {
int irq;
};

+int intel_pinctrl_probe(struct platform_device *pdev,
+ const struct intel_pinctrl_soc_data *soc_data);
+
int intel_pinctrl_probe_by_hid(struct platform_device *pdev);
int intel_pinctrl_probe_by_uid(struct platform_device *pdev);

--
2.40.0.1.gaa8946217a0b

2023-11-03 05:58:12

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH v1 2/3] pinctrl: intel: Add a generic Intel pin control platform driver

On Mon, Oct 30, 2023 at 04:10:33PM +0200, Andy Shevchenko wrote:
> New generations of Intel platforms will provide better description
> of the pin control devices in the ACPI tables. Hence, we may provide
> a generic pin control platform driver to cover all of them.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/pinctrl/intel/Kconfig | 8 +
> drivers/pinctrl/intel/Makefile | 1 +
> .../pinctrl/intel/pinctrl-intel-platform.c | 223 ++++++++++++++++++
> 3 files changed, 232 insertions(+)
> create mode 100644 drivers/pinctrl/intel/pinctrl-intel-platform.c
>
> diff --git a/drivers/pinctrl/intel/Kconfig b/drivers/pinctrl/intel/Kconfig
> index d66f4f6932d8..42a6bc8b7a45 100644
> --- a/drivers/pinctrl/intel/Kconfig
> +++ b/drivers/pinctrl/intel/Kconfig
> @@ -37,6 +37,14 @@ config PINCTRL_INTEL
> select GPIOLIB
> select GPIOLIB_IRQCHIP
>
> +config PINCTRL_INTEL_PLATFORM
> + tristate "Intel pinctrl and GPIO platform driver"
> + depends on ACPI
> + select PINCTRL_INTEL
> + help
> + This pinctrl driver provides an interface that allows configuring
> + of Intel PCH pins and using them as GPIOs.

Add here some description that explains why this needs to be enabled,
for example for Lunar Lake. Now it is all too generic for distro folks
to understand if this is needed or not.

> +
> config PINCTRL_ALDERLAKE
> tristate "Intel Alder Lake pinctrl and GPIO driver"
> select PINCTRL_INTEL
> diff --git a/drivers/pinctrl/intel/Makefile b/drivers/pinctrl/intel/Makefile
> index f6d30f2d973a..96c93ed4bd58 100644
> --- a/drivers/pinctrl/intel/Makefile
> +++ b/drivers/pinctrl/intel/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_PINCTRL_TANGIER) += pinctrl-tangier.o
> obj-$(CONFIG_PINCTRL_MERRIFIELD) += pinctrl-merrifield.o
> obj-$(CONFIG_PINCTRL_MOOREFIELD) += pinctrl-moorefield.o
> obj-$(CONFIG_PINCTRL_INTEL) += pinctrl-intel.o
> +obj-$(CONFIG_PINCTRL_INTEL_PLATFORM) += pinctrl-intel-platform.o
> obj-$(CONFIG_PINCTRL_ALDERLAKE) += pinctrl-alderlake.o
> obj-$(CONFIG_PINCTRL_BROXTON) += pinctrl-broxton.o
> obj-$(CONFIG_PINCTRL_CANNONLAKE) += pinctrl-cannonlake.o
> diff --git a/drivers/pinctrl/intel/pinctrl-intel-platform.c b/drivers/pinctrl/intel/pinctrl-intel-platform.c
> new file mode 100644
> index 000000000000..2305d8befdd3
> --- /dev/null
> +++ b/drivers/pinctrl/intel/pinctrl-intel-platform.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Intel PCH pinctrl/GPIO driver
> + *
> + * Copyright (C) 2021-2023, Intel Corporation

That's 2023

> + * Author: Andy Shevchenko <[email protected]>
> + */
> +
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm.h>
> +#include <linux/property.h>
> +#include <linux/string_helpers.h>
> +
> +#include <linux/pinctrl/pinctrl.h>
> +
> +#include "pinctrl-intel.h"
> +
> +struct intel_platform_pins {
> + struct pinctrl_pin_desc *pins;
> + size_t npins;
> +};
> +
> +static int intel_platform_pinctrl_prepare_pins(struct device *dev, size_t base,
> + const char *name, u32 size,
> + struct intel_platform_pins *pins)
> +{
> + struct pinctrl_pin_desc *descs;
> + char **pin_names;
> + unsigned int i;
> +
> + pin_names = devm_kasprintf_strarray(dev, name, size);
> + if (IS_ERR(pin_names))
> + return PTR_ERR(pin_names);
> +
> + descs = devm_krealloc_array(dev, pins->pins, base + size, sizeof(*descs), GFP_KERNEL);
> + if (!descs)
> + return -ENOMEM;
> +
> + for (i = 0; i < size; i++) {
> + unsigned int pin_number = base + i;
> + char *pin_name = pin_names[i];
> + struct pinctrl_pin_desc *desc;
> +
> + /* Unify delimiter for pin name */
> + strreplace(pin_name, '-', '_');
> +
> + desc = &descs[pin_number];
> + desc->number = pin_number;
> + desc->name = pin_name;
> + }
> +
> + pins->pins = descs;
> + pins->npins = base + size;
> +
> + return 0;
> +}
> +
> +static int intel_platform_pinctrl_prepare_group(struct device *dev,
> + struct fwnode_handle *child,
> + struct intel_padgroup *gpp,
> + struct intel_platform_pins *pins)
> +{
> + size_t base = pins->npins;
> + const char *name;
> + u32 size;
> + int ret;
> +
> + ret = fwnode_property_read_string(child, "intc-gpio-group-name", &name);
> + if (ret)
> + return ret;
> +
> + ret = fwnode_property_read_u32(child, "intc-gpio-pad-count", &size);
> + if (ret)
> + return ret;
> +
> + ret = intel_platform_pinctrl_prepare_pins(dev, base, name, size, pins);
> + if (ret)
> + return ret;
> +
> + gpp->base = base;
> + gpp->size = size;
> + gpp->gpio_base = INTEL_GPIO_BASE_MATCH;
> +
> + return 0;
> +}
> +
> +static int intel_platform_pinctrl_prepare_community(struct device *dev,
> + struct intel_community *community,
> + struct intel_platform_pins *pins)
> +{
> + struct fwnode_handle *child;
> + struct intel_padgroup *gpps;
> + unsigned int group;
> + size_t ngpps;
> + u32 offset;
> + int ret;
> +
> + ret = device_property_read_u32(dev, "intc-gpio-pad-ownership-offset", &offset);
> + if (ret)
> + return ret;
> + community->padown_offset = offset;
> +
> + ret = device_property_read_u32(dev, "intc-gpio-pad-configuration-lock-offset", &offset);
> + if (ret)
> + return ret;
> + community->padcfglock_offset = offset;
> +
> + ret = device_property_read_u32(dev, "intc-gpio-host-software-pad-ownership-offset", &offset);
> + if (ret)
> + return ret;
> + community->hostown_offset = offset;
> +
> + ret = device_property_read_u32(dev, "intc-gpio-gpi-interrupt-status-offset", &offset);
> + if (ret)
> + return ret;
> + community->is_offset = offset;
> +
> + ret = device_property_read_u32(dev, "intc-gpio-gpi-interrupt-enable-offset", &offset);
> + if (ret)
> + return ret;
> + community->ie_offset = offset;
> +
> + ngpps = device_get_child_node_count(dev);
> + if (ngpps == 0)

if (!nggps)

> + return -ENODEV;
> +
> + gpps = devm_kcalloc(dev, ngpps, sizeof(*gpps), GFP_KERNEL);
> + if (!gpps)
> + return -ENOMEM;
> +
> + group = 0;
> + device_for_each_child_node(dev, child) {
> + struct intel_padgroup *gpp = &gpps[group];
> +
> + gpp->reg_num = group;
> +
> + ret = intel_platform_pinctrl_prepare_group(dev, child, gpp, pins);
> + if (ret)
> + return ret;
> +
> + group++;
> + }
> +
> + community->ngpps = ngpps;
> + community->gpps = gpps;
> +
> + return 0;
> +}
> +
> +static int intel_platform_pinctrl_prepare_soc_data(struct device *dev,
> + struct intel_pinctrl_soc_data *data)
> +{
> + struct intel_platform_pins pins = {};
> + struct intel_community *communities;
> + size_t ncommunities;
> + unsigned int i;
> + int ret;
> +
> + ncommunities = 1,

Why this is 1? Can't we have more communities?

> + communities = devm_kcalloc(dev, ncommunities, sizeof(*communities), GFP_KERNEL);
> + if (!communities)
> + return -ENOMEM;
> +
> + for (i = 0; i < ncommunities; i++) {
> + struct intel_community *community = &communities[i];
> +
> + community->barno = i;
> + community->pin_base = pins.npins;
> +
> + ret = intel_platform_pinctrl_prepare_community(dev, community, &pins);
> + if (ret)
> + return ret;
> +
> + community->npins = pins.npins - community->pin_base;
> + }
> +
> + data->ncommunities = ncommunities;
> + data->communities = communities;
> +
> + data->npins = pins.npins;
> + data->pins = pins.pins;
> +
> + return 0;
> +}
> +
> +static int intel_platform_pinctrl_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct intel_pinctrl_soc_data *data;


Change the ordering of the above:

struct intel_pinctrl_soc_data *data;
struct device *dev = &pdev->dev;

> + int ret;
> +
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + ret = intel_platform_pinctrl_prepare_soc_data(dev, data);
> + if (ret)
> + return ret;
> +
> + return intel_pinctrl_probe(pdev, data);
> +}
> +
> +static const struct acpi_device_id intel_platform_pinctrl_acpi_match[] = {
> + { }

And add the _CID here in this patch as I commented in the last patch.

> +};
> +MODULE_DEVICE_TABLE(acpi, intel_platform_pinctrl_acpi_match);
> +
> +static struct platform_driver intel_platform_pinctrl_driver = {
> + .probe = intel_platform_pinctrl_probe,
> + .driver = {
> + .name = "intel-pinctrl",
> + .acpi_match_table = intel_platform_pinctrl_acpi_match,
> + .pm = pm_sleep_ptr(&intel_pinctrl_pm_ops),
> + },
> +};
> +module_platform_driver(intel_platform_pinctrl_driver);
> +
> +MODULE_AUTHOR("Andy Shevchenko <[email protected]>");
> +MODULE_DESCRIPTION("Intel PCH pinctrl/GPIO driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_IMPORT_NS(PINCTRL_INTEL);
> --
> 2.40.0.1.gaa8946217a0b