The SIUL2 module is now represented as an mfd device. The pinctrl driver
is now an mfd_cell. Therefore, remove its compatible and adjust its
probing in order to get the necessary information from its mfd parent.
Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com>
---
drivers/pinctrl/nxp/pinctrl-s32.h | 1 +
drivers/pinctrl/nxp/pinctrl-s32cc.c | 75 +++++++++++------------------
drivers/pinctrl/nxp/pinctrl-s32g2.c | 23 ++-------
3 files changed, 33 insertions(+), 66 deletions(-)
diff --git a/drivers/pinctrl/nxp/pinctrl-s32.h b/drivers/pinctrl/nxp/pinctrl-s32.h
index add3c77ddfed..829211741050 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32.h
+++ b/drivers/pinctrl/nxp/pinctrl-s32.h
@@ -38,6 +38,7 @@ struct s32_pinctrl_soc_data {
const struct pinctrl_pin_desc *pins;
unsigned int npins;
const struct s32_pin_range *mem_pin_ranges;
+ const struct regmap **regmaps;
unsigned int mem_regions;
};
diff --git a/drivers/pinctrl/nxp/pinctrl-s32cc.c b/drivers/pinctrl/nxp/pinctrl-s32cc.c
index 501eb296c760..709e823b9c7c 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32cc.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32cc.c
@@ -12,6 +12,7 @@
#include <linux/gpio/driver.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/mfd/nxp-siul2.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
@@ -44,12 +45,6 @@ enum s32_write_type {
S32_PINCONF_OVERWRITE,
};
-static struct regmap_config s32_regmap_config = {
- .reg_bits = 32,
- .val_bits = 32,
- .reg_stride = 4,
-};
-
static u32 get_pin_no(u32 pinmux)
{
return (pinmux & S32_PIN_ID_MASK) >> S32_PIN_ID_SHIFT;
@@ -85,14 +80,15 @@ struct s32_pinctrl_context {
unsigned int *pads;
};
-/*
+/**
+ * struct s32_pinctrl - private driver data
* @dev: a pointer back to containing device
* @pctl: a pointer to the pinctrl device structure
* @regions: reserved memory regions with start/end pin
* @info: structure containing information about the pin
* @gpio_configs: Saved configurations for GPIO pins
* @gpiop_configs_lock: lock for the `gpio_configs` list
- * @s32_pinctrl_context: Configuration saved over system sleep
+ * @saved_context: Configuration saved over system sleep
*/
struct s32_pinctrl {
struct device *dev;
@@ -123,14 +119,13 @@ s32_get_region(struct pinctrl_dev *pctldev, unsigned int pin)
return NULL;
}
-static inline int s32_check_pin(struct pinctrl_dev *pctldev,
- unsigned int pin)
+static int s32_check_pin(struct pinctrl_dev *pctldev, unsigned int pin)
{
return s32_get_region(pctldev, pin) ? 0 : -EINVAL;
}
-static inline int s32_regmap_read(struct pinctrl_dev *pctldev,
- unsigned int pin, unsigned int *val)
+static int s32_regmap_read(struct pinctrl_dev *pctldev, unsigned int pin,
+ unsigned int *val)
{
struct s32_pinctrl_mem_region *region;
unsigned int offset;
@@ -145,7 +140,7 @@ static inline int s32_regmap_read(struct pinctrl_dev *pctldev,
return regmap_read(region->map, offset, val);
}
-static inline int s32_regmap_write(struct pinctrl_dev *pctldev,
+static int s32_regmap_write(struct pinctrl_dev *pctldev,
unsigned int pin,
unsigned int val)
{
@@ -163,7 +158,7 @@ static inline int s32_regmap_write(struct pinctrl_dev *pctldev,
}
-static inline int s32_regmap_update(struct pinctrl_dev *pctldev, unsigned int pin,
+static int s32_regmap_update(struct pinctrl_dev *pctldev, unsigned int pin,
unsigned int mask, unsigned int val)
{
struct s32_pinctrl_mem_region *region;
@@ -475,8 +470,8 @@ static int s32_get_slew_regval(int arg)
return -EINVAL;
}
-static inline void s32_pin_set_pull(enum pin_config_param param,
- unsigned int *mask, unsigned int *config)
+static void s32_pin_set_pull(enum pin_config_param param,
+ unsigned int *mask, unsigned int *config)
{
switch (param) {
case PIN_CONFIG_BIAS_DISABLE:
@@ -838,20 +833,21 @@ static int s32_pinctrl_parse_functions(struct device_node *np,
static int s32_pinctrl_probe_dt(struct platform_device *pdev,
struct s32_pinctrl *ipctl)
{
+ struct nxp_siul2_mfd *mfd = dev_get_drvdata(pdev->dev.parent);
struct s32_pinctrl_soc_info *info = ipctl->info;
- struct device_node *np = pdev->dev.of_node;
- struct resource *res;
- struct regmap *map;
- void __iomem *base;
- unsigned int mem_regions = info->soc_data->mem_regions;
+ unsigned int mem_regions;
+ struct device_node *np;
+ u32 nfuncs = 0, i = 0, j;
+ u8 regmap_type;
int ret;
- u32 nfuncs = 0;
- u32 i = 0;
+ np = pdev->dev.parent->of_node;
if (!np)
return -ENODEV;
- if (mem_regions == 0 || mem_regions >= 10000) {
+ /* one MSCR and one IMCR region per SIUL2 module */
+ mem_regions = info->soc_data->mem_regions;
+ if (mem_regions != mfd->num_siul2 * 2) {
dev_err(&pdev->dev, "mem_regions is invalid: %u\n", mem_regions);
return -EINVAL;
}
@@ -861,26 +857,11 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
if (!ipctl->regions)
return -ENOMEM;
+ /* Order is MSCR regions first, then IMCR ones */
for (i = 0; i < mem_regions; i++) {
- base = devm_platform_get_and_ioremap_resource(pdev, i, &res);
- if (IS_ERR(base))
- return PTR_ERR(base);
-
- snprintf(ipctl->regions[i].name,
- sizeof(ipctl->regions[i].name), "map%u", i);
-
- s32_regmap_config.name = ipctl->regions[i].name;
- s32_regmap_config.max_register = resource_size(res) -
- s32_regmap_config.reg_stride;
-
- map = devm_regmap_init_mmio(&pdev->dev, base,
- &s32_regmap_config);
- if (IS_ERR(map)) {
- dev_err(&pdev->dev, "Failed to init regmap[%u]\n", i);
- return PTR_ERR(map);
- }
-
- ipctl->regions[i].map = map;
+ regmap_type = i < mem_regions / 2 ? SIUL2_MSCR : SIUL2_IMCR;
+ j = i % mfd->num_siul2;
+ ipctl->regions[i].map = mfd->siul2[j].regmaps[regmap_type];
ipctl->regions[i].pin_range = &info->soc_data->mem_pin_ranges[i];
}
@@ -918,13 +899,13 @@ static int s32_pinctrl_probe_dt(struct platform_device *pdev,
int s32_pinctrl_probe(struct platform_device *pdev,
const struct s32_pinctrl_soc_data *soc_data)
{
- struct s32_pinctrl *ipctl;
- int ret;
- struct pinctrl_desc *s32_pinctrl_desc;
- struct s32_pinctrl_soc_info *info;
#ifdef CONFIG_PM_SLEEP
struct s32_pinctrl_context *saved_context;
#endif
+ struct pinctrl_desc *s32_pinctrl_desc;
+ struct s32_pinctrl_soc_info *info;
+ struct s32_pinctrl *ipctl;
+ int ret;
if (!soc_data || !soc_data->pins || !soc_data->npins) {
dev_err(&pdev->dev, "wrong pinctrl info\n");
diff --git a/drivers/pinctrl/nxp/pinctrl-s32g2.c b/drivers/pinctrl/nxp/pinctrl-s32g2.c
index 440ff1879424..9c7fe545cc85 100644
--- a/drivers/pinctrl/nxp/pinctrl-s32g2.c
+++ b/drivers/pinctrl/nxp/pinctrl-s32g2.c
@@ -10,6 +10,7 @@
#include <linux/err.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/mfd/nxp-siul2.h>
#include <linux/module.h>
#include <linux/of.h>
#include <linux/platform_device.h>
@@ -713,12 +714,10 @@ static const struct pinctrl_pin_desc s32_pinctrl_pads_siul2[] = {
static const struct s32_pin_range s32_pin_ranges_siul2[] = {
/* MSCR pin ID ranges */
S32_PIN_RANGE(0, 101),
- S32_PIN_RANGE(112, 122),
- S32_PIN_RANGE(144, 190),
+ S32_PIN_RANGE(112, 190),
/* IMCR pin ID ranges */
S32_PIN_RANGE(512, 595),
- S32_PIN_RANGE(631, 909),
- S32_PIN_RANGE(942, 1007),
+ S32_PIN_RANGE(631, 1007),
};
static const struct s32_pinctrl_soc_data s32_pinctrl_data = {
@@ -728,22 +727,9 @@ static const struct s32_pinctrl_soc_data s32_pinctrl_data = {
.mem_regions = ARRAY_SIZE(s32_pin_ranges_siul2),
};
-static const struct of_device_id s32_pinctrl_of_match[] = {
- {
- .compatible = "nxp,s32g2-siul2-pinctrl",
- .data = &s32_pinctrl_data,
- },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, s32_pinctrl_of_match);
-
static int s32g_pinctrl_probe(struct platform_device *pdev)
{
- const struct s32_pinctrl_soc_data *soc_data;
-
- soc_data = of_device_get_match_data(&pdev->dev);
-
- return s32_pinctrl_probe(pdev, soc_data);
+ return s32_pinctrl_probe(pdev, &s32_pinctrl_data);
}
static const struct dev_pm_ops s32g_pinctrl_pm_ops = {
@@ -753,7 +739,6 @@ static const struct dev_pm_ops s32g_pinctrl_pm_ops = {
static struct platform_driver s32g_pinctrl_driver = {
.driver = {
.name = "s32g-siul2-pinctrl",
- .of_match_table = s32_pinctrl_of_match,
.pm = pm_sleep_ptr(&s32g_pinctrl_pm_ops),
.suppress_bind_attrs = true,
},
--
2.45.2On Fri, Nov 01, 2024 at 10:06:10AM +0200, Andrei Stefanescu wrote:
> + /* Order is MSCR regions first, then IMCR ones */
> for (i = 0; i < mem_regions; i++) {
> - base = devm_platform_get_and_ioremap_resource(pdev, i, &res);
> - if (IS_ERR(base))
> - return PTR_ERR(base);
> -
> - snprintf(ipctl->regions[i].name,
> - sizeof(ipctl->regions[i].name), "map%u", i);
> -
> - s32_regmap_config.name = ipctl->regions[i].name;
> - s32_regmap_config.max_register = resource_size(res) -
> - s32_regmap_config.reg_stride;
> -
> - map = devm_regmap_init_mmio(&pdev->dev, base,
> - &s32_regmap_config);
> - if (IS_ERR(map)) {
> - dev_err(&pdev->dev, "Failed to init regmap[%u]\n", i);
> - return PTR_ERR(map);
> - }
> -
> - ipctl->regions[i].map = map;
> + regmap_type = i < mem_regions / 2 ? SIUL2_MSCR : SIUL2_IMCR;
> + j = i % mfd->num_siul2;
> + ipctl->regions[i].map = mfd->siul2[j].regmaps[regmap_type];
> ipctl->regions[i].pin_range = &info->soc_data->mem_pin_ranges[i];
This looks like breaking all the users. Nothing in commit msg about
this: about rationale, impact or backwards compatibility.
Nothing in changelog in cover letter explaining such sudden change in
approach.
Sorry, that's a NAK.
Best regards,
Krzysztof
Hi Krzysztof,
On 02/11/2024 10:51, Krzysztof Kozlowski wrote:
> On Fri, Nov 01, 2024 at 10:06:10AM +0200, Andrei Stefanescu wrote:
>> + /* Order is MSCR regions first, then IMCR ones */
>> for (i = 0; i < mem_regions; i++) {
>> - base = devm_platform_get_and_ioremap_resource(pdev, i, &res);
>> - if (IS_ERR(base))
>> - return PTR_ERR(base);
>> -
>> - snprintf(ipctl->regions[i].name,
>> - sizeof(ipctl->regions[i].name), "map%u", i);
>> -
>> - s32_regmap_config.name = ipctl->regions[i].name;
>> - s32_regmap_config.max_register = resource_size(res) -
>> - s32_regmap_config.reg_stride;
>> -
>> - map = devm_regmap_init_mmio(&pdev->dev, base,
>> - &s32_regmap_config);
>> - if (IS_ERR(map)) {
>> - dev_err(&pdev->dev, "Failed to init regmap[%u]\n", i);
>> - return PTR_ERR(map);
>> - }
>> -
>> - ipctl->regions[i].map = map;
>> + regmap_type = i < mem_regions / 2 ? SIUL2_MSCR : SIUL2_IMCR;
>> + j = i % mfd->num_siul2;
>> + ipctl->regions[i].map = mfd->siul2[j].regmaps[regmap_type];
>> ipctl->regions[i].pin_range = &info->soc_data->mem_pin_ranges[i];
>
> This looks like breaking all the users. Nothing in commit msg about
> this: about rationale, impact or backwards compatibility.
>
> Nothing in changelog in cover letter explaining such sudden change in
> approach.
>
> Sorry, that's a NAK.
I will add a detailed explanation for the change in v6. I changed the existing
implementation because of feedback received in this series and in [0]. The SIUL2
module has the pinctrl&GPIO functionality tightly coupled, which required us to
carve out the registers between GPIO & pinctrl and it resulted in a long and detailed
list.
Also, in the future, we will also have some nvmem cells exported which are part
of SIUL2.
Best regards,
Andrei
[0] - https://lore.kernel.org/linux-gpio/20241002135920.3647322-1-andrei.stefanescu@oss.nxp.com/
>
> Best regards,
> Krzysztof
>
Hi Andrei,
kernel test robot noticed the following build warnings:
[auto build test WARNING on linusw-pinctrl/devel]
[also build test WARNING on linusw-pinctrl/for-next lee-mfd/for-mfd-next shawnguo/for-next linus/master v6.12-rc5 next-20241101]
[cannot apply to lee-mfd/for-mfd-fixes]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andrei-Stefanescu/dt-bindings-mfd-add-support-for-the-NXP-SIUL2-module/20241101-160940
base: https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git devel
patch link: https://lore.kernel.org/r/20241101080614.1070819-5-andrei.stefanescu%40oss.nxp.com
patch subject: [PATCH v5 4/7] pinctrl: s32: convert the driver into an mfd cell
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20241102/202411020514.qOUrieWa-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 639a7ac648f1e50ccd2556e17d401c04f9cce625)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241102/202411020514.qOUrieWa-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202411020514.qOUrieWa-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/pinctrl/nxp/pinctrl-s32cc.c:103: warning: Function parameter or struct member 'gpio_configs_lock' not described in 's32_pinctrl'
>> drivers/pinctrl/nxp/pinctrl-s32cc.c:103: warning: Excess struct member 'gpiop_configs_lock' description in 's32_pinctrl'
Kconfig warnings: (for reference only)
WARNING: unmet direct dependencies detected for MODVERSIONS
Depends on [n]: MODULES [=y] && !COMPILE_TEST [=y]
Selected by [y]:
- RANDSTRUCT_FULL [=y] && (CC_HAS_RANDSTRUCT [=y] || GCC_PLUGINS [=n]) && MODULES [=y]
vim +103 drivers/pinctrl/nxp/pinctrl-s32cc.c
fd84aaa8173d3f Chester Lin 2023-02-20 82
157a51f7e5e81e Andrei Stefanescu 2024-11-01 83 /**
157a51f7e5e81e Andrei Stefanescu 2024-11-01 84 * struct s32_pinctrl - private driver data
fd84aaa8173d3f Chester Lin 2023-02-20 85 * @dev: a pointer back to containing device
fd84aaa8173d3f Chester Lin 2023-02-20 86 * @pctl: a pointer to the pinctrl device structure
fd84aaa8173d3f Chester Lin 2023-02-20 87 * @regions: reserved memory regions with start/end pin
fd84aaa8173d3f Chester Lin 2023-02-20 88 * @info: structure containing information about the pin
fd84aaa8173d3f Chester Lin 2023-02-20 89 * @gpio_configs: Saved configurations for GPIO pins
fd84aaa8173d3f Chester Lin 2023-02-20 90 * @gpiop_configs_lock: lock for the `gpio_configs` list
157a51f7e5e81e Andrei Stefanescu 2024-11-01 91 * @saved_context: Configuration saved over system sleep
fd84aaa8173d3f Chester Lin 2023-02-20 92 */
fd84aaa8173d3f Chester Lin 2023-02-20 93 struct s32_pinctrl {
fd84aaa8173d3f Chester Lin 2023-02-20 94 struct device *dev;
fd84aaa8173d3f Chester Lin 2023-02-20 95 struct pinctrl_dev *pctl;
fd84aaa8173d3f Chester Lin 2023-02-20 96 struct s32_pinctrl_mem_region *regions;
fd84aaa8173d3f Chester Lin 2023-02-20 97 struct s32_pinctrl_soc_info *info;
fd84aaa8173d3f Chester Lin 2023-02-20 98 struct list_head gpio_configs;
fd84aaa8173d3f Chester Lin 2023-02-20 99 spinlock_t gpio_configs_lock;
fd84aaa8173d3f Chester Lin 2023-02-20 100 #ifdef CONFIG_PM_SLEEP
fd84aaa8173d3f Chester Lin 2023-02-20 101 struct s32_pinctrl_context saved_context;
fd84aaa8173d3f Chester Lin 2023-02-20 102 #endif
fd84aaa8173d3f Chester Lin 2023-02-20 @103 };
fd84aaa8173d3f Chester Lin 2023-02-20 104
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Fri, Nov 1, 2024 at 9:06 AM Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> wrote: > The SIUL2 module is now represented as an mfd device. The pinctrl driver > is now an mfd_cell. Therefore, remove its compatible and adjust its > probing in order to get the necessary information from its mfd parent. > > Signed-off-by: Andrei Stefanescu <andrei.stefanescu@oss.nxp.com> Acked-by: Linus Walleij <linus.walleij@linaro.org> I assume this needs to go in with the rest of the patches. Yours, Linus Walleij
© 2016 - 2024 Red Hat, Inc.