This patch adds suspend and resume support for Tegra pinctrl driver
and registers them to syscore so the pinmux settings are restored
before the devices resume.
Signed-off-by: Sowjanya Komatineni <[email protected]>
---
drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
7 files changed, 84 insertions(+)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index 34596b246578..ceced30d8bd1 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
@@ -20,11 +20,16 @@
#include <linux/pinctrl/pinmux.h>
#include <linux/pinctrl/pinconf.h>
#include <linux/slab.h>
+#include <linux/syscore_ops.h>
#include "../core.h"
#include "../pinctrl-utils.h"
#include "pinctrl-tegra.h"
+#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
+#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
+#define EMMC_DPD_PARKING (0x1fff << 14)
+
static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
{
return readl(pmx->regs[bank] + reg);
@@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
}
}
+
+ if (pmx->soc->has_park_padcfg) {
+ val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
+ val &= ~EMMC_DPD_PARKING;
+ pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
+
+ val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
+ val &= ~EMMC_DPD_PARKING;
+ pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
+ }
+}
+
+int __maybe_unused tegra_pinctrl_suspend(struct device *dev)
+{
+ struct tegra_pmx *pmx = dev_get_drvdata(dev);
+ u32 *backup_regs = pmx->backup_regs;
+ u32 *regs;
+ int i, j;
+
+ for (i = 0; i < pmx->nbanks; i++) {
+ regs = pmx->regs[i];
+ for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
+ *backup_regs++ = readl(regs++);
+ }
+
+ return pinctrl_force_sleep(pmx->pctl);
+}
+
+int __maybe_unused tegra_pinctrl_resume(struct device *dev)
+{
+ struct tegra_pmx *pmx = dev_get_drvdata(dev);
+ u32 *backup_regs = pmx->backup_regs;
+ u32 *regs;
+ int i, j;
+
+ for (i = 0; i < pmx->nbanks; i++) {
+ regs = pmx->regs[i];
+ for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
+ writel(*backup_regs++, regs++);
+ }
+
+ return 0;
}
static bool gpio_node_has_range(const char *compatible)
@@ -645,6 +692,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
int i;
const char **group_pins;
int fn, gn, gfn;
+ unsigned long backup_regs_size = 0;
pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
if (!pmx)
@@ -697,6 +745,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
if (!res)
break;
+ backup_regs_size += resource_size(res);
}
pmx->nbanks = i;
@@ -705,11 +754,24 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
if (!pmx->regs)
return -ENOMEM;
+ pmx->reg_bank_size = devm_kcalloc(&pdev->dev, pmx->nbanks,
+ sizeof(*pmx->reg_bank_size),
+ GFP_KERNEL);
+ if (!pmx->reg_bank_size)
+ return -ENOMEM;
+
+ pmx->backup_regs = devm_kzalloc(&pdev->dev, backup_regs_size,
+ GFP_KERNEL);
+ if (!pmx->backup_regs)
+ return -ENOMEM;
+
for (i = 0; i < pmx->nbanks; i++) {
res = platform_get_resource(pdev, IORESOURCE_MEM, i);
pmx->regs[i] = devm_ioremap_resource(&pdev->dev, res);
if (IS_ERR(pmx->regs[i]))
return PTR_ERR(pmx->regs[i]);
+
+ pmx->reg_bank_size[i] = resource_size(res);
}
pmx->pctl = devm_pinctrl_register(&pdev->dev, &tegra_pinctrl_desc, pmx);
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
index 287702660783..d63e472ee0e1 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.h
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
@@ -17,6 +17,8 @@ struct tegra_pmx {
int nbanks;
void __iomem **regs;
+ size_t *reg_bank_size;
+ u32 *backup_regs;
};
enum tegra_pinconf_param {
@@ -191,8 +193,11 @@ struct tegra_pinctrl_soc_data {
bool hsm_in_mux;
bool schmitt_in_mux;
bool drvtype_in_mux;
+ bool has_park_padcfg;
};
int tegra_pinctrl_probe(struct platform_device *pdev,
const struct tegra_pinctrl_soc_data *soc_data);
+int __maybe_unused tegra_pinctrl_suspend(struct device *dev);
+int __maybe_unused tegra_pinctrl_resume(struct device *dev);
#endif
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra114.c b/drivers/pinctrl/tegra/pinctrl-tegra114.c
index 762151f17a88..06ea8164df9d 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra114.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra114.c
@@ -1841,6 +1841,7 @@ static const struct tegra_pinctrl_soc_data tegra114_pinctrl = {
.hsm_in_mux = false,
.schmitt_in_mux = false,
.drvtype_in_mux = false,
+ .has_park_padcfg = false,
};
static int tegra114_pinctrl_probe(struct platform_device *pdev)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra124.c b/drivers/pinctrl/tegra/pinctrl-tegra124.c
index 930c43758c92..abc8fe92d154 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra124.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra124.c
@@ -2053,6 +2053,7 @@ static const struct tegra_pinctrl_soc_data tegra124_pinctrl = {
.hsm_in_mux = false,
.schmitt_in_mux = false,
.drvtype_in_mux = false,
+ .has_park_padcfg = false,
};
static int tegra124_pinctrl_probe(struct platform_device *pdev)
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra20.c b/drivers/pinctrl/tegra/pinctrl-tegra20.c
index 4b7837e38fb5..993b82cbfba7 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra20.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra20.c
@@ -2223,6 +2223,7 @@ static const struct tegra_pinctrl_soc_data tegra20_pinctrl = {
.hsm_in_mux = false,
.schmitt_in_mux = false,
.drvtype_in_mux = false,
+ .has_park_padcfg = false,
};
static const char *cdev1_parents[] = {
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210.c b/drivers/pinctrl/tegra/pinctrl-tegra210.c
index 0b56ad5c9c1c..10e8a2ec8094 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra210.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra210.c
@@ -1555,6 +1555,7 @@ static const struct tegra_pinctrl_soc_data tegra210_pinctrl = {
.hsm_in_mux = true,
.schmitt_in_mux = true,
.drvtype_in_mux = true,
+ .has_park_padcfg = true,
};
static int tegra210_pinctrl_probe(struct platform_device *pdev)
@@ -1562,6 +1563,17 @@ static int tegra210_pinctrl_probe(struct platform_device *pdev)
return tegra_pinctrl_probe(pdev, &tegra210_pinctrl);
}
+#ifdef CONFIG_PM_SLEEP
+static const struct dev_pm_ops tegra_pinctrl_pm = {
+ .suspend = &tegra_pinctrl_suspend,
+ .resume = &tegra_pinctrl_resume
+};
+
+#define TEGRA_PINCTRL_PM (&tegra_pinctrl_pm)
+#else
+#define TEGRA_PINCTRL_PM NULL
+#endif
+
static const struct of_device_id tegra210_pinctrl_of_match[] = {
{ .compatible = "nvidia,tegra210-pinmux", },
{ },
@@ -1571,6 +1583,7 @@ static struct platform_driver tegra210_pinctrl_driver = {
.driver = {
.name = "tegra210-pinctrl",
.of_match_table = tegra210_pinctrl_of_match,
+ .pm = TEGRA_PINCTRL_PM,
},
.probe = tegra210_pinctrl_probe,
};
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra30.c b/drivers/pinctrl/tegra/pinctrl-tegra30.c
index 610124c3d192..779ee40e5f21 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra30.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra30.c
@@ -2476,6 +2476,7 @@ static const struct tegra_pinctrl_soc_data tegra30_pinctrl = {
.hsm_in_mux = false,
.schmitt_in_mux = false,
.drvtype_in_mux = false,
+ .has_park_padcfg = false,
};
static int tegra30_pinctrl_probe(struct platform_device *pdev)
--
2.7.4
18.06.2019 10:46, Sowjanya Komatineni пишет:
> This patch adds suspend and resume support for Tegra pinctrl driver
> and registers them to syscore so the pinmux settings are restored
> before the devices resume.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
> drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
> 7 files changed, 84 insertions(+)
>
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index 34596b246578..ceced30d8bd1 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> @@ -20,11 +20,16 @@
> #include <linux/pinctrl/pinmux.h>
> #include <linux/pinctrl/pinconf.h>
> #include <linux/slab.h>
> +#include <linux/syscore_ops.h>
>
> #include "../core.h"
> #include "../pinctrl-utils.h"
> #include "pinctrl-tegra.h"
>
> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
> +#define EMMC_DPD_PARKING (0x1fff << 14)
> +
> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
> {
> return readl(pmx->regs[bank] + reg);
> @@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
> }
> }
> +
> + if (pmx->soc->has_park_padcfg) {
> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
> + val &= ~EMMC_DPD_PARKING;
> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
> +
> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
> + val &= ~EMMC_DPD_PARKING;
> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
> + }
> +}
Is there any reason why parked_bit can't be changed to parked_bitmask like I was
asking in a comment to v2?
I suppose that it's more preferable to keep pinctrl-tegra.c platform-agnostic for
consistency when possible, hence adding platform specifics here should be discouraged.
And then the parked_bitmask will also result in a proper hardware description in the code.
18.06.2019 12:22, Dmitry Osipenko пишет:
> 18.06.2019 10:46, Sowjanya Komatineni пишет:
>> This patch adds suspend and resume support for Tegra pinctrl driver
>> and registers them to syscore so the pinmux settings are restored
>> before the devices resume.
>>
>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>> ---
>> drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
>> drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
>> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
>> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
>> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
>> drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
>> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
>> 7 files changed, 84 insertions(+)
>>
>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
>> index 34596b246578..ceced30d8bd1 100644
>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>> @@ -20,11 +20,16 @@
>> #include <linux/pinctrl/pinmux.h>
>> #include <linux/pinctrl/pinconf.h>
>> #include <linux/slab.h>
>> +#include <linux/syscore_ops.h>
>>
>> #include "../core.h"
>> #include "../pinctrl-utils.h"
>> #include "pinctrl-tegra.h"
>>
>> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
>> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
>> +#define EMMC_DPD_PARKING (0x1fff << 14)
>> +
>> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
>> {
>> return readl(pmx->regs[bank] + reg);
>> @@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
>> }
>> }
>> +
>> + if (pmx->soc->has_park_padcfg) {
>> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>> + val &= ~EMMC_DPD_PARKING;
>> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>> +
>> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>> + val &= ~EMMC_DPD_PARKING;
>> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>> + }
>> +}
>
> Is there any reason why parked_bit can't be changed to parked_bitmask like I was
> asking in a comment to v2?
>
> I suppose that it's more preferable to keep pinctrl-tegra.c platform-agnostic for
> consistency when possible, hence adding platform specifics here should be discouraged.
> And then the parked_bitmask will also result in a proper hardware description in the code.
>
I'm now also vaguely recalling that Stephen Warren had some kind of a "code generator"
for the pinctrl drivers. So I guess all those tables were auto-generated initially.
Stephen, maybe you could adjust the generator to take into account the bitmask (of
course if that's a part of the generated code) and then re-gen it all for Sowjanya?
On Tue, Jun 18, 2019 at 12:46:16AM -0700, Sowjanya Komatineni wrote:
> This patch adds suspend and resume support for Tegra pinctrl driver
> and registers them to syscore so the pinmux settings are restored
> before the devices resume.
This no longer uses syscore ops, so you need to reflect that in the
commit message.
>
> Signed-off-by: Sowjanya Komatineni <[email protected]>
> ---
> drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
> drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
> drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
> 7 files changed, 84 insertions(+)
>
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index 34596b246578..ceced30d8bd1 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> @@ -20,11 +20,16 @@
> #include <linux/pinctrl/pinmux.h>
> #include <linux/pinctrl/pinconf.h>
> #include <linux/slab.h>
> +#include <linux/syscore_ops.h>
No longer needed.
>
> #include "../core.h"
> #include "../pinctrl-utils.h"
> #include "pinctrl-tegra.h"
>
> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
> +#define EMMC_DPD_PARKING (0x1fff << 14)
> +
> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
> {
> return readl(pmx->regs[bank] + reg);
> @@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
> }
> }
> +
> + if (pmx->soc->has_park_padcfg) {
> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
> + val &= ~EMMC_DPD_PARKING;
> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
> +
> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
> + val &= ~EMMC_DPD_PARKING;
> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
> + }
> +}
> +
> +int __maybe_unused tegra_pinctrl_suspend(struct device *dev)
> +{
> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
> + u32 *backup_regs = pmx->backup_regs;
> + u32 *regs;
> + int i, j;
Can be unsigned int.
> +
> + for (i = 0; i < pmx->nbanks; i++) {
> + regs = pmx->regs[i];
> + for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
> + *backup_regs++ = readl(regs++);
> + }
> +
> + return pinctrl_force_sleep(pmx->pctl);
> +}
> +
> +int __maybe_unused tegra_pinctrl_resume(struct device *dev)
> +{
> + struct tegra_pmx *pmx = dev_get_drvdata(dev);
> + u32 *backup_regs = pmx->backup_regs;
> + u32 *regs;
> + int i, j;
unsigned
> +
> + for (i = 0; i < pmx->nbanks; i++) {
> + regs = pmx->regs[i];
> + for (j = 0; j < pmx->reg_bank_size[i] / 4; j++)
> + writel(*backup_regs++, regs++);
> + }
> +
> + return 0;
> }
>
> static bool gpio_node_has_range(const char *compatible)
> @@ -645,6 +692,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
> int i;
> const char **group_pins;
> int fn, gn, gfn;
> + unsigned long backup_regs_size = 0;
>
> pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
> if (!pmx)
> @@ -697,6 +745,7 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> if (!res)
> break;
> + backup_regs_size += resource_size(res);
> }
> pmx->nbanks = i;
>
> @@ -705,11 +754,24 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
> if (!pmx->regs)
> return -ENOMEM;
>
> + pmx->reg_bank_size = devm_kcalloc(&pdev->dev, pmx->nbanks,
> + sizeof(*pmx->reg_bank_size),
> + GFP_KERNEL);
> + if (!pmx->reg_bank_size)
> + return -ENOMEM;
> +
> + pmx->backup_regs = devm_kzalloc(&pdev->dev, backup_regs_size,
> + GFP_KERNEL);
> + if (!pmx->backup_regs)
> + return -ENOMEM;
> +
> for (i = 0; i < pmx->nbanks; i++) {
> res = platform_get_resource(pdev, IORESOURCE_MEM, i);
> pmx->regs[i] = devm_ioremap_resource(&pdev->dev, res);
> if (IS_ERR(pmx->regs[i]))
> return PTR_ERR(pmx->regs[i]);
> +
> + pmx->reg_bank_size[i] = resource_size(res);
> }
>
> pmx->pctl = devm_pinctrl_register(&pdev->dev, &tegra_pinctrl_desc, pmx);
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
> index 287702660783..d63e472ee0e1 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.h
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
> @@ -17,6 +17,8 @@ struct tegra_pmx {
>
> int nbanks;
> void __iomem **regs;
> + size_t *reg_bank_size;
> + u32 *backup_regs;
> };
>
> enum tegra_pinconf_param {
> @@ -191,8 +193,11 @@ struct tegra_pinctrl_soc_data {
> bool hsm_in_mux;
> bool schmitt_in_mux;
> bool drvtype_in_mux;
> + bool has_park_padcfg;
> };
>
> int tegra_pinctrl_probe(struct platform_device *pdev,
> const struct tegra_pinctrl_soc_data *soc_data);
> +int __maybe_unused tegra_pinctrl_suspend(struct device *dev);
> +int __maybe_unused tegra_pinctrl_resume(struct device *dev);
> #endif
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra114.c b/drivers/pinctrl/tegra/pinctrl-tegra114.c
> index 762151f17a88..06ea8164df9d 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra114.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra114.c
> @@ -1841,6 +1841,7 @@ static const struct tegra_pinctrl_soc_data tegra114_pinctrl = {
> .hsm_in_mux = false,
> .schmitt_in_mux = false,
> .drvtype_in_mux = false,
> + .has_park_padcfg = false,
> };
>
> static int tegra114_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra124.c b/drivers/pinctrl/tegra/pinctrl-tegra124.c
> index 930c43758c92..abc8fe92d154 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra124.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra124.c
> @@ -2053,6 +2053,7 @@ static const struct tegra_pinctrl_soc_data tegra124_pinctrl = {
> .hsm_in_mux = false,
> .schmitt_in_mux = false,
> .drvtype_in_mux = false,
> + .has_park_padcfg = false,
> };
>
> static int tegra124_pinctrl_probe(struct platform_device *pdev)
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra20.c b/drivers/pinctrl/tegra/pinctrl-tegra20.c
> index 4b7837e38fb5..993b82cbfba7 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra20.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra20.c
> @@ -2223,6 +2223,7 @@ static const struct tegra_pinctrl_soc_data tegra20_pinctrl = {
> .hsm_in_mux = false,
> .schmitt_in_mux = false,
> .drvtype_in_mux = false,
> + .has_park_padcfg = false,
> };
>
> static const char *cdev1_parents[] = {
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210.c b/drivers/pinctrl/tegra/pinctrl-tegra210.c
> index 0b56ad5c9c1c..10e8a2ec8094 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra210.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra210.c
> @@ -1555,6 +1555,7 @@ static const struct tegra_pinctrl_soc_data tegra210_pinctrl = {
> .hsm_in_mux = true,
> .schmitt_in_mux = true,
> .drvtype_in_mux = true,
> + .has_park_padcfg = true,
> };
>
> static int tegra210_pinctrl_probe(struct platform_device *pdev)
> @@ -1562,6 +1563,17 @@ static int tegra210_pinctrl_probe(struct platform_device *pdev)
> return tegra_pinctrl_probe(pdev, &tegra210_pinctrl);
> }
>
> +#ifdef CONFIG_PM_SLEEP
> +static const struct dev_pm_ops tegra_pinctrl_pm = {
> + .suspend = &tegra_pinctrl_suspend,
> + .resume = &tegra_pinctrl_resume
> +};
> +
> +#define TEGRA_PINCTRL_PM (&tegra_pinctrl_pm)
> +#else
> +#define TEGRA_PINCTRL_PM NULL
> +#endif
I think we can simplify this by just dropping the #ifdef. We don't allow
!PM on Tegra anymore and suspend/resume is something that most users
will want to enable. There's very little gain in making the dev_pm_ops
conditional, and keeping them around unconditionally make it simple.
> +
> static const struct of_device_id tegra210_pinctrl_of_match[] = {
> { .compatible = "nvidia,tegra210-pinmux", },
> { },
> @@ -1571,6 +1583,7 @@ static struct platform_driver tegra210_pinctrl_driver = {
> .driver = {
> .name = "tegra210-pinctrl",
> .of_match_table = tegra210_pinctrl_of_match,
> + .pm = TEGRA_PINCTRL_PM,
Please use a single space around '='. No need for arbitrary padding.
Thierry Reding
On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
> 18.06.2019 12:22, Dmitry Osipenko пишет:
>> 18.06.2019 10:46, Sowjanya Komatineni пишет:
>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>> and registers them to syscore so the pinmux settings are restored
>>> before the devices resume.
>>>
>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>> ---
>>> drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
>>> drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
>>> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
>>> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
>>> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
>>> drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
>>> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
>>> 7 files changed, 84 insertions(+)
>>>
>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> index 34596b246578..ceced30d8bd1 100644
>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>> @@ -20,11 +20,16 @@
>>> #include <linux/pinctrl/pinmux.h>
>>> #include <linux/pinctrl/pinconf.h>
>>> #include <linux/slab.h>
>>> +#include <linux/syscore_ops.h>
>>>
>>> #include "../core.h"
>>> #include "../pinctrl-utils.h"
>>> #include "pinctrl-tegra.h"
>>>
>>> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
>>> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
>>> +#define EMMC_DPD_PARKING (0x1fff << 14)
>>> +
>>> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
>>> {
>>> return readl(pmx->regs[bank] + reg);
>>> @@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
>>> }
>>> }
>>> +
>>> + if (pmx->soc->has_park_padcfg) {
>>> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>> + val &= ~EMMC_DPD_PARKING;
>>> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>> +
>>> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>> + val &= ~EMMC_DPD_PARKING;
>>> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>> + }
>>> +}
>>
>> Is there any reason why parked_bit can't be changed to parked_bitmask like I was
>> asking in a comment to v2?
>>
>> I suppose that it's more preferable to keep pinctrl-tegra.c platform-agnostic for
>> consistency when possible, hence adding platform specifics here should be discouraged.
>> And then the parked_bitmask will also result in a proper hardware description in the code.
>>
>
> I'm now also vaguely recalling that Stephen Warren had some kind of a "code generator"
> for the pinctrl drivers. So I guess all those tables were auto-generated initially.
>
> Stephen, maybe you could adjust the generator to take into account the bitmask (of
> course if that's a part of the generated code) and then re-gen it all for Sowjanya?
https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that
generate tegra-pinctrlNNN.c. See soc-to-kernel-pinctrl-driver.py.
IIRC, tegra-pinctrl.c (the core file) isn't auto-generated. Sowjanya is
welcome to send a patch to that repo if the code needs to be updated.
On 6/18/19 8:41 AM, Stephen Warren wrote:
> On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
>> 18.06.2019 12:22, Dmitry Osipenko пишет:
>>> 18.06.2019 10:46, Sowjanya Komatineni пишет:
>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>> and registers them to syscore so the pinmux settings are restored
>>>> before the devices resume.
>>>>
>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>> ---
>>>> drivers/pinctrl/tegra/pinctrl-tegra.c | 62
>>>> ++++++++++++++++++++++++++++++++
>>>> drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
>>>> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
>>>> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
>>>> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
>>>> drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
>>>> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
>>>> 7 files changed, 84 insertions(+)
>>>>
>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> index 34596b246578..ceced30d8bd1 100644
>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>> @@ -20,11 +20,16 @@
>>>> #include <linux/pinctrl/pinmux.h>
>>>> #include <linux/pinctrl/pinconf.h>
>>>> #include <linux/slab.h>
>>>> +#include <linux/syscore_ops.h>
>>>> #include "../core.h"
>>>> #include "../pinctrl-utils.h"
>>>> #include "pinctrl-tegra.h"
>>>> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
>>>> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
>>>> +#define EMMC_DPD_PARKING (0x1fff << 14)
>>>> +
>>>> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32
>>>> reg)
>>>> {
>>>> return readl(pmx->regs[bank] + reg);
>>>> @@ -619,6 +624,48 @@ static void
>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
>>>> }
>>>> }
>>>> +
>>>> + if (pmx->soc->has_park_padcfg) {
>>>> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>> + val &= ~EMMC_DPD_PARKING;
>>>> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>> +
>>>> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>> + val &= ~EMMC_DPD_PARKING;
>>>> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>> + }
>>>> +}
>>>
>>> Is there any reason why parked_bit can't be changed to
>>> parked_bitmask like I was
>>> asking in a comment to v2?
>>>
>>> I suppose that it's more preferable to keep pinctrl-tegra.c
>>> platform-agnostic for
>>> consistency when possible, hence adding platform specifics here
>>> should be discouraged.
>>> And then the parked_bitmask will also result in a proper hardware
>>> description in the code.
>>>
>>
>> I'm now also vaguely recalling that Stephen Warren had some kind of a
>> "code generator"
>> for the pinctrl drivers. So I guess all those tables were
>> auto-generated initially.
>>
>> Stephen, maybe you could adjust the generator to take into account
>> the bitmask (of
>> course if that's a part of the generated code) and then re-gen it all
>> for Sowjanya?
>
> https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that
> generate tegra-pinctrlNNN.c. See soc-to-kernel-pinctrl-driver.py.
> IIRC, tegra-pinctrl.c (the core file) isn't auto-generated. Sowjanya
> is welcome to send a patch to that repo if the code needs to be updated.
Hi Dmitry,
Just want to be clear on my understanding of your request.
"change parked_bit to parked_bitmask" are you requested to change
parked_bit of PINGROUP and DRV_PINGROUP to use bitmask value rather than
bit position inorder to have parked bit configuration for EMMC PADs as
well to happen by masking rather than checking for existence of parked_bit?
Trying to understand the reason/benefit for changing parked_bit to
parked_bitmask.
thanks
Sowjanya
On 6/18/19 9:50 AM, Sowjanya Komatineni wrote:
>
> On 6/18/19 8:41 AM, Stephen Warren wrote:
>> On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
>>> 18.06.2019 12:22, Dmitry Osipenko пишет:
>>>> 18.06.2019 10:46, Sowjanya Komatineni пишет:
>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>>> and registers them to syscore so the pinmux settings are restored
>>>>> before the devices resume.
>>>>>
>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>> ---
>>>>> drivers/pinctrl/tegra/pinctrl-tegra.c | 62
>>>>> ++++++++++++++++++++++++++++++++
>>>>> drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
>>>>> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
>>>>> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
>>>>> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
>>>>> drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
>>>>> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
>>>>> 7 files changed, 84 insertions(+)
>>>>>
>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> index 34596b246578..ceced30d8bd1 100644
>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>> @@ -20,11 +20,16 @@
>>>>> #include <linux/pinctrl/pinmux.h>
>>>>> #include <linux/pinctrl/pinconf.h>
>>>>> #include <linux/slab.h>
>>>>> +#include <linux/syscore_ops.h>
>>>>> #include "../core.h"
>>>>> #include "../pinctrl-utils.h"
>>>>> #include "pinctrl-tegra.h"
>>>>> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
>>>>> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
>>>>> +#define EMMC_DPD_PARKING (0x1fff << 14)
>>>>> +
>>>>> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32
>>>>> reg)
>>>>> {
>>>>> return readl(pmx->regs[bank] + reg);
>>>>> @@ -619,6 +624,48 @@ static void
>>>>> tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
>>>>> }
>>>>> }
>>>>> +
>>>>> + if (pmx->soc->has_park_padcfg) {
>>>>> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>> + val &= ~EMMC_DPD_PARKING;
>>>>> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>> +
>>>>> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>> + val &= ~EMMC_DPD_PARKING;
>>>>> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>> + }
>>>>> +}
>>>>
>>>> Is there any reason why parked_bit can't be changed to
>>>> parked_bitmask like I was
>>>> asking in a comment to v2?
>>>>
>>>> I suppose that it's more preferable to keep pinctrl-tegra.c
>>>> platform-agnostic for
>>>> consistency when possible, hence adding platform specifics here
>>>> should be discouraged.
>>>> And then the parked_bitmask will also result in a proper hardware
>>>> description in the code.
>>>>
>>>
>>> I'm now also vaguely recalling that Stephen Warren had some kind of
>>> a "code generator"
>>> for the pinctrl drivers. So I guess all those tables were
>>> auto-generated initially.
>>>
>>> Stephen, maybe you could adjust the generator to take into account
>>> the bitmask (of
>>> course if that's a part of the generated code) and then re-gen it
>>> all for Sowjanya?
>>
>> https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that
>> generate tegra-pinctrlNNN.c. See soc-to-kernel-pinctrl-driver.py.
>> IIRC, tegra-pinctrl.c (the core file) isn't auto-generated. Sowjanya
>> is welcome to send a patch to that repo if the code needs to be updated.
>
>
> Hi Dmitry,
>
> Just want to be clear on my understanding of your request.
>
> "change parked_bit to parked_bitmask" are you requested to change
> parked_bit of PINGROUP and DRV_PINGROUP to use bitmask value rather
> than bit position inorder to have parked bit configuration for EMMC
> PADs as well to happen by masking rather than checking for existence
> of parked_bit?
>
> Trying to understand the reason/benefit for changing parked_bit to
> parked_bitmask.
Also, Park bits in CFGPAD registers are not common for all CFGPAD
registers. Park bits are available only for EMMC and also those bits are
used for something else on other CFGPAD registers so bitmask can't be
common and this also need an update to DRV_PINGROUP macro args just only
to handle EMMC parked_bitmask. So not sure of the benefit in using
bitmask rather than parked_bit
>
> thanks
>
> Sowjanya
>
18.06.2019 20:34, Sowjanya Komatineni пишет:
>
> On 6/18/19 9:50 AM, Sowjanya Komatineni wrote:
>>
>> On 6/18/19 8:41 AM, Stephen Warren wrote:
>>> On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
>>>> 18.06.2019 12:22, Dmitry Osipenko пишет:
>>>>> 18.06.2019 10:46, Sowjanya Komatineni пишет:
>>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>>>> and registers them to syscore so the pinmux settings are restored
>>>>>> before the devices resume.
>>>>>>
>>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>>> ---
>>>>>> drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
>>>>>> drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
>>>>>> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
>>>>>> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
>>>>>> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
>>>>>> drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
>>>>>> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
>>>>>> 7 files changed, 84 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>> index 34596b246578..ceced30d8bd1 100644
>>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>> @@ -20,11 +20,16 @@
>>>>>> #include <linux/pinctrl/pinmux.h>
>>>>>> #include <linux/pinctrl/pinconf.h>
>>>>>> #include <linux/slab.h>
>>>>>> +#include <linux/syscore_ops.h>
>>>>>> #include "../core.h"
>>>>>> #include "../pinctrl-utils.h"
>>>>>> #include "pinctrl-tegra.h"
>>>>>> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
>>>>>> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
>>>>>> +#define EMMC_DPD_PARKING (0x1fff << 14)
>>>>>> +
>>>>>> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
>>>>>> {
>>>>>> return readl(pmx->regs[bank] + reg);
>>>>>> @@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>>> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
>>>>>> }
>>>>>> }
>>>>>> +
>>>>>> + if (pmx->soc->has_park_padcfg) {
>>>>>> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>> + val &= ~EMMC_DPD_PARKING;
>>>>>> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>> +
>>>>>> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>> + val &= ~EMMC_DPD_PARKING;
>>>>>> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>> + }
>>>>>> +}
>>>>>
>>>>> Is there any reason why parked_bit can't be changed to parked_bitmask like I was
>>>>> asking in a comment to v2?
>>>>>
>>>>> I suppose that it's more preferable to keep pinctrl-tegra.c platform-agnostic for
>>>>> consistency when possible, hence adding platform specifics here should be discouraged.
>>>>> And then the parked_bitmask will also result in a proper hardware description in the code.
>>>>>
>>>>
>>>> I'm now also vaguely recalling that Stephen Warren had some kind of a "code generator"
>>>> for the pinctrl drivers. So I guess all those tables were auto-generated initially.
>>>>
>>>> Stephen, maybe you could adjust the generator to take into account the bitmask (of
>>>> course if that's a part of the generated code) and then re-gen it all for Sowjanya?
>>>
>>> https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that generate
>>> tegra-pinctrlNNN.c. See soc-to-kernel-pinctrl-driver.py. IIRC, tegra-pinctrl.c (the core
>>> file) isn't auto-generated. Sowjanya is welcome to send a patch to that repo if the code
>>> needs to be updated.
>>
>>
>> Hi Dmitry,
>>
>> Just want to be clear on my understanding of your request.
>>
>> "change parked_bit to parked_bitmask" are you requested to change parked_bit of PINGROUP
>> and DRV_PINGROUP to use bitmask value rather than bit position inorder to have parked bit
>> configuration for EMMC PADs as well to happen by masking rather than checking for
>> existence of parked_bit?
>>
>> Trying to understand the reason/benefit for changing parked_bit to parked_bitmask.
> Also, Park bits in CFGPAD registers are not common for all CFGPAD registers. Park bits are
> available only for EMMC and also those bits are used for something else on other CFGPAD
> registers so bitmask can't be common and this also need an update to DRV_PINGROUP macro args
> just only to handle EMMC parked_bitmask. So not sure of the benefit in using bitmask rather
Hi Sowjanya,
The main motivation is to describe hardware properly in the drivers. Why to make a
hacky-looking workaround while you can make things properly? Especially if that doesn't take
much effort.
Stephen, thank you very much for the pointer to the script. Looks like it should be easy to
modify the script accordingly to the required change.
Sowjanya, below is a draft of the change that I'm suggesting. I see this as two separate
patches: first converts drivers to use parked_bitmask, second adds suspend-resume support.
Please note that in the end it's up to you and Tegra/PINCTRL maintainers to decide if this
is a worthwhile change that I'm suggesting. In my opinion it is much better to have a
generic solution rather than to have a special quirk solely for T210.
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
index 34596b246578..4150da74bd44 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
@@ -613,9 +613,9 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
for (i = 0; i < pmx->soc->ngroups; ++i) {
g = &pmx->soc->groups[i];
- if (g->parked_bit >= 0) {
+ if (g->parked_bitmask != -1) {
val = pmx_readl(pmx, g->mux_bank, g->mux_reg);
- val &= ~(1 << g->parked_bit);
+ val &= ~g->parked_bitmask;
pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
}
}
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
index 287702660783..875eb7a1d838 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra.h
+++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
@@ -96,7 +96,7 @@ struct tegra_function {
* @tri_reg: Tri-state register offset.
* @tri_bank: Tri-state register bank.
* @tri_bit: Tri-state register bit.
- * @parked_bit: Parked register bit. -1 if unsupported.
+ * @parked_bitmask: Parked register bitmask. -1 if unsupported.
* @einput_bit: Enable-input register bit.
* @odrain_bit: Open-drain register bit.
* @lock_bit: Lock register bit.
@@ -146,7 +146,7 @@ struct tegra_pingroup {
s32 mux_bit:6;
s32 pupd_bit:6;
s32 tri_bit:6;
- s32 parked_bit:6;
+ s32 parked_bitmask:26;
s32 einput_bit:6;
s32 odrain_bit:6;
s32 lock_bit:6;
diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210.c b/drivers/pinctrl/tegra/pinctrl-tegra210.c
index 0b56ad5c9c1c..d2ba13466e06 100644
--- a/drivers/pinctrl/tegra/pinctrl-tegra210.c
+++ b/drivers/pinctrl/tegra/pinctrl-tegra210.c
@@ -1302,7 +1302,7 @@ static struct tegra_function tegra210_functions[] = {
.lock_bit = 7, \
.ioreset_bit = -1, \
.rcv_sel_bit = PINGROUP_BIT_##e_io_hv(10), \
- .parked_bit = 5, \
+ .parked_bitmask = BIT(5), \
.hsm_bit = PINGROUP_BIT_##hsm(9), \
.schmitt_bit = 12, \
.drvtype_bit = PINGROUP_BIT_##drvtype(13), \
@@ -1320,7 +1320,7 @@ static struct tegra_function tegra210_functions[] = {
}
#define DRV_PINGROUP(pg_name, r, drvdn_b, drvdn_w, drvup_b, drvup_w, \
- slwr_b, slwr_w, slwf_b, slwf_w) \
+ slwr_b, slwr_w, slwf_b, slwf_w, prk_mask) \
{ \
.name = "drive_" #pg_name, \
.pins = drive_##pg_name##_pins, \
@@ -1335,7 +1335,7 @@ static struct tegra_function tegra210_functions[] = {
.rcv_sel_bit = -1, \
.drv_reg = DRV_PINGROUP_REG(r), \
.drv_bank = 0, \
- .parked_bit = -1, \
+ .parked_bitmask = prk_mask, \
.hsm_bit = -1, \
.schmitt_bit = -1, \
.lpmd_bit = -1, \
@@ -1516,31 +1516,31 @@ static const struct tegra_pingroup tegra210_groups[] = {
PINGROUP(pz5, SOC, RSVD1, RSVD2, RSVD3, 0x3290, N, N, N,
-1, -1, -1, -1, -1, -1, -1, -1, -1),
/* pg_name, r, drvdn_b, drvdn_w, drvup_b, drvup_w, slwr_b, slwr_w, slwf_b, slwf_w */
- DRV_PINGROUP(pa6, 0x9c0, 12, 5, 20, 5, -1, -1, -1, -1),
- DRV_PINGROUP(pcc7, 0x9c4, 12, 5, 20, 5, -1, -1, -1, -1),
- DRV_PINGROUP(pe6, 0x9c8, 12, 5, 20, 5, -1, -1, -1, -1),
- DRV_PINGROUP(pe7, 0x9cc, 12, 5, 20, 5, -1, -1, -1, -1),
- DRV_PINGROUP(ph6, 0x9d0, 12, 5, 20, 5, -1, -1, -1, -1),
- DRV_PINGROUP(pk0, 0x9d4, -1, -1, -1, -1, 28, 2, 30, 2),
- DRV_PINGROUP(pk1, 0x9d8, -1, -1, -1, -1, 28, 2, 30, 2),
- DRV_PINGROUP(pk2, 0x9dc, -1, -1, -1, -1, 28, 2, 30, 2),
- DRV_PINGROUP(pk3, 0x9e0, -1, -1, -1, -1, 28, 2, 30, 2),
- DRV_PINGROUP(pk4, 0x9e4, -1, -1, -1, -1, 28, 2, 30, 2),
- DRV_PINGROUP(pk5, 0x9e8, -1, -1, -1, -1, 28, 2, 30, 2),
- DRV_PINGROUP(pk6, 0x9ec, -1, -1, -1, -1, 28, 2, 30, 2),
- DRV_PINGROUP(pk7, 0x9f0, -1, -1, -1, -1, 28, 2, 30, 2),
- DRV_PINGROUP(pl0, 0x9f4, -1, -1, -1, -1, 28, 2, 30, 2),
- DRV_PINGROUP(pl1, 0x9f8, -1, -1, -1, -1, 28, 2, 30, 2),
- DRV_PINGROUP(pz0, 0x9fc, 12, 7, 20, 7, -1, -1, -1, -1),
- DRV_PINGROUP(pz1, 0xa00, 12, 7, 20, 7, -1, -1, -1, -1),
- DRV_PINGROUP(pz2, 0xa04, 12, 7, 20, 7, -1, -1, -1, -1),
- DRV_PINGROUP(pz3, 0xa08, 12, 7, 20, 7, -1, -1, -1, -1),
- DRV_PINGROUP(pz4, 0xa0c, 12, 7, 20, 7, -1, -1, -1, -1),
- DRV_PINGROUP(pz5, 0xa10, 12, 7, 20, 7, -1, -1, -1, -1),
- DRV_PINGROUP(sdmmc1, 0xa98, 12, 7, 20, 7, 28, 2, 30, 2),
- DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2),
- DRV_PINGROUP(sdmmc3, 0xab0, 12, 7, 20, 7, 28, 2, 30, 2),
- DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2),
+ DRV_PINGROUP(pa6, 0x9c0, 12, 5, 20, 5, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(pcc7, 0x9c4, 12, 5, 20, 5, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(pe6, 0x9c8, 12, 5, 20, 5, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(pe7, 0x9cc, 12, 5, 20, 5, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(ph6, 0x9d0, 12, 5, 20, 5, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(pk0, 0x9d4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(pk1, 0x9d8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(pk2, 0x9dc, -1, -1, -1, -1, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(pk3, 0x9e0, -1, -1, -1, -1, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(pk4, 0x9e4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(pk5, 0x9e8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(pk6, 0x9ec, -1, -1, -1, -1, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(pk7, 0x9f0, -1, -1, -1, -1, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(pl0, 0x9f4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(pl1, 0x9f8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(pz0, 0x9fc, 12, 7, 20, 7, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(pz1, 0xa00, 12, 7, 20, 7, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(pz2, 0xa04, 12, 7, 20, 7, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(pz3, 0xa08, 12, 7, 20, 7, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(pz4, 0xa0c, 12, 7, 20, 7, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(pz5, 0xa10, 12, 7, 20, 7, -1, -1, -1, -1, -1),
+ DRV_PINGROUP(sdmmc1, 0xa98, 12, 7, 20, 7, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2, 0x7ffc000),
+ DRV_PINGROUP(sdmmc3, 0xab0, 12, 7, 20, 7, 28, 2, 30, 2, -1),
+ DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2, 0x7ffc000),
};
static const struct tegra_pinctrl_soc_data tegra210_pinctrl = {
On 6/18/19 1:00 PM, Dmitry Osipenko wrote:
> 18.06.2019 20:34, Sowjanya Komatineni пишет:
>> On 6/18/19 9:50 AM, Sowjanya Komatineni wrote:
>>> On 6/18/19 8:41 AM, Stephen Warren wrote:
>>>> On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
>>>>> 18.06.2019 12:22, Dmitry Osipenko пишет:
>>>>>> 18.06.2019 10:46, Sowjanya Komatineni пишет:
>>>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>>>>> and registers them to syscore so the pinmux settings are restored
>>>>>>> before the devices resume.
>>>>>>>
>>>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>>>> ---
>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
>>>>>>> 7 files changed, 84 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> index 34596b246578..ceced30d8bd1 100644
>>>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>> @@ -20,11 +20,16 @@
>>>>>>> #include <linux/pinctrl/pinmux.h>
>>>>>>> #include <linux/pinctrl/pinconf.h>
>>>>>>> #include <linux/slab.h>
>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>> #include "../core.h"
>>>>>>> #include "../pinctrl-utils.h"
>>>>>>> #include "pinctrl-tegra.h"
>>>>>>> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
>>>>>>> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
>>>>>>> +#define EMMC_DPD_PARKING (0x1fff << 14)
>>>>>>> +
>>>>>>> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
>>>>>>> {
>>>>>>> return readl(pmx->regs[bank] + reg);
>>>>>>> @@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>>>> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
>>>>>>> }
>>>>>>> }
>>>>>>> +
>>>>>>> + if (pmx->soc->has_park_padcfg) {
>>>>>>> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>> + val &= ~EMMC_DPD_PARKING;
>>>>>>> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>> +
>>>>>>> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>> + val &= ~EMMC_DPD_PARKING;
>>>>>>> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>> + }
>>>>>>> +}
>>>>>> Is there any reason why parked_bit can't be changed to parked_bitmask like I was
>>>>>> asking in a comment to v2?
>>>>>>
>>>>>> I suppose that it's more preferable to keep pinctrl-tegra.c platform-agnostic for
>>>>>> consistency when possible, hence adding platform specifics here should be discouraged.
>>>>>> And then the parked_bitmask will also result in a proper hardware description in the code.
>>>>>>
>>>>> I'm now also vaguely recalling that Stephen Warren had some kind of a "code generator"
>>>>> for the pinctrl drivers. So I guess all those tables were auto-generated initially.
>>>>>
>>>>> Stephen, maybe you could adjust the generator to take into account the bitmask (of
>>>>> course if that's a part of the generated code) and then re-gen it all for Sowjanya?
>>>> https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that generate
>>>> tegra-pinctrlNNN.c. See soc-to-kernel-pinctrl-driver.py. IIRC, tegra-pinctrl.c (the core
>>>> file) isn't auto-generated. Sowjanya is welcome to send a patch to that repo if the code
>>>> needs to be updated.
>>>
>>> Hi Dmitry,
>>>
>>> Just want to be clear on my understanding of your request.
>>>
>>> "change parked_bit to parked_bitmask" are you requested to change parked_bit of PINGROUP
>>> and DRV_PINGROUP to use bitmask value rather than bit position inorder to have parked bit
>>> configuration for EMMC PADs as well to happen by masking rather than checking for
>>> existence of parked_bit?
>>>
>>> Trying to understand the reason/benefit for changing parked_bit to parked_bitmask.
>> Also, Park bits in CFGPAD registers are not common for all CFGPAD registers. Park bits are
>> available only for EMMC and also those bits are used for something else on other CFGPAD
>> registers so bitmask can't be common and this also need an update to DRV_PINGROUP macro args
>> just only to handle EMMC parked_bitmask. So not sure of the benefit in using bitmask rather
> Hi Sowjanya,
>
> The main motivation is to describe hardware properly in the drivers. Why to make a
> hacky-looking workaround while you can make things properly? Especially if that doesn't take
> much effort.
>
> Stephen, thank you very much for the pointer to the script. Looks like it should be easy to
> modify the script accordingly to the required change.
>
> Sowjanya, below is a draft of the change that I'm suggesting. I see this as two separate
> patches: first converts drivers to use parked_bitmask, second adds suspend-resume support.
>
> Please note that in the end it's up to you and Tegra/PINCTRL maintainers to decide if this
> is a worthwhile change that I'm suggesting. In my opinion it is much better to have a
> generic solution rather than to have a special quirk solely for T210.
OK I can change it. Just thought to find out the reason as I see other
pinmux field also using as bits rather than bitmask.
Got it now. Will update in next version.
>
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index 34596b246578..4150da74bd44 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> @@ -613,9 +613,9 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>
> for (i = 0; i < pmx->soc->ngroups; ++i) {
> g = &pmx->soc->groups[i];
> - if (g->parked_bit >= 0) {
> + if (g->parked_bitmask != -1) {
> val = pmx_readl(pmx, g->mux_bank, g->mux_reg);
> - val &= ~(1 << g->parked_bit);
> + val &= ~g->parked_bitmask;
> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
> }
> }
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
> index 287702660783..875eb7a1d838 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.h
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
> @@ -96,7 +96,7 @@ struct tegra_function {
> * @tri_reg: Tri-state register offset.
> * @tri_bank: Tri-state register bank.
> * @tri_bit: Tri-state register bit.
> - * @parked_bit: Parked register bit. -1 if unsupported.
> + * @parked_bitmask: Parked register bitmask. -1 if unsupported.
> * @einput_bit: Enable-input register bit.
> * @odrain_bit: Open-drain register bit.
> * @lock_bit: Lock register bit.
> @@ -146,7 +146,7 @@ struct tegra_pingroup {
> s32 mux_bit:6;
> s32 pupd_bit:6;
> s32 tri_bit:6;
> - s32 parked_bit:6;
> + s32 parked_bitmask:26;
> s32 einput_bit:6;
> s32 odrain_bit:6;
> s32 lock_bit:6;
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210.c b/drivers/pinctrl/tegra/pinctrl-tegra210.c
> index 0b56ad5c9c1c..d2ba13466e06 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra210.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra210.c
> @@ -1302,7 +1302,7 @@ static struct tegra_function tegra210_functions[] = {
> .lock_bit = 7, \
> .ioreset_bit = -1, \
> .rcv_sel_bit = PINGROUP_BIT_##e_io_hv(10), \
> - .parked_bit = 5, \
> + .parked_bitmask = BIT(5), \
> .hsm_bit = PINGROUP_BIT_##hsm(9), \
> .schmitt_bit = 12, \
> .drvtype_bit = PINGROUP_BIT_##drvtype(13), \
> @@ -1320,7 +1320,7 @@ static struct tegra_function tegra210_functions[] = {
> }
>
> #define DRV_PINGROUP(pg_name, r, drvdn_b, drvdn_w, drvup_b, drvup_w, \
> - slwr_b, slwr_w, slwf_b, slwf_w) \
> + slwr_b, slwr_w, slwf_b, slwf_w, prk_mask) \
> { \
> .name = "drive_" #pg_name, \
> .pins = drive_##pg_name##_pins, \
> @@ -1335,7 +1335,7 @@ static struct tegra_function tegra210_functions[] = {
> .rcv_sel_bit = -1, \
> .drv_reg = DRV_PINGROUP_REG(r), \
> .drv_bank = 0, \
> - .parked_bit = -1, \
> + .parked_bitmask = prk_mask, \
> .hsm_bit = -1, \
> .schmitt_bit = -1, \
> .lpmd_bit = -1, \
> @@ -1516,31 +1516,31 @@ static const struct tegra_pingroup tegra210_groups[] = {
> PINGROUP(pz5, SOC, RSVD1, RSVD2, RSVD3, 0x3290, N, N, N,
> -1, -1, -1, -1, -1, -1, -1, -1, -1),
>
> /* pg_name, r, drvdn_b, drvdn_w, drvup_b, drvup_w, slwr_b, slwr_w, slwf_b, slwf_w */
> - DRV_PINGROUP(pa6, 0x9c0, 12, 5, 20, 5, -1, -1, -1, -1),
> - DRV_PINGROUP(pcc7, 0x9c4, 12, 5, 20, 5, -1, -1, -1, -1),
> - DRV_PINGROUP(pe6, 0x9c8, 12, 5, 20, 5, -1, -1, -1, -1),
> - DRV_PINGROUP(pe7, 0x9cc, 12, 5, 20, 5, -1, -1, -1, -1),
> - DRV_PINGROUP(ph6, 0x9d0, 12, 5, 20, 5, -1, -1, -1, -1),
> - DRV_PINGROUP(pk0, 0x9d4, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk1, 0x9d8, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk2, 0x9dc, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk3, 0x9e0, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk4, 0x9e4, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk5, 0x9e8, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk6, 0x9ec, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk7, 0x9f0, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pl0, 0x9f4, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pl1, 0x9f8, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pz0, 0x9fc, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(pz1, 0xa00, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(pz2, 0xa04, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(pz3, 0xa08, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(pz4, 0xa0c, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(pz5, 0xa10, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(sdmmc1, 0xa98, 12, 7, 20, 7, 28, 2, 30, 2),
> - DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2),
> - DRV_PINGROUP(sdmmc3, 0xab0, 12, 7, 20, 7, 28, 2, 30, 2),
> - DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2),
> + DRV_PINGROUP(pa6, 0x9c0, 12, 5, 20, 5, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pcc7, 0x9c4, 12, 5, 20, 5, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pe6, 0x9c8, 12, 5, 20, 5, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pe7, 0x9cc, 12, 5, 20, 5, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(ph6, 0x9d0, 12, 5, 20, 5, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pk0, 0x9d4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk1, 0x9d8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk2, 0x9dc, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk3, 0x9e0, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk4, 0x9e4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk5, 0x9e8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk6, 0x9ec, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk7, 0x9f0, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pl0, 0x9f4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pl1, 0x9f8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pz0, 0x9fc, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pz1, 0xa00, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pz2, 0xa04, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pz3, 0xa08, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pz4, 0xa0c, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pz5, 0xa10, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(sdmmc1, 0xa98, 12, 7, 20, 7, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2, 0x7ffc000),
> + DRV_PINGROUP(sdmmc3, 0xab0, 12, 7, 20, 7, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2, 0x7ffc000),
> };
>
> static const struct tegra_pinctrl_soc_data tegra210_pinctrl = {
On Tue, Jun 18, 2019 at 11:00:05PM +0300, Dmitry Osipenko wrote:
> 18.06.2019 20:34, Sowjanya Komatineni пишет:
> >
> > On 6/18/19 9:50 AM, Sowjanya Komatineni wrote:
> >>
> >> On 6/18/19 8:41 AM, Stephen Warren wrote:
> >>> On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
> >>>> 18.06.2019 12:22, Dmitry Osipenko пишет:
> >>>>> 18.06.2019 10:46, Sowjanya Komatineni пишет:
> >>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
> >>>>>> and registers them to syscore so the pinmux settings are restored
> >>>>>> before the devices resume.
> >>>>>>
> >>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
> >>>>>> ---
> >>>>>> drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
> >>>>>> drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
> >>>>>> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
> >>>>>> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
> >>>>>> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
> >>>>>> drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
> >>>>>> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
> >>>>>> 7 files changed, 84 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
> >>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
> >>>>>> index 34596b246578..ceced30d8bd1 100644
> >>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> >>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> >>>>>> @@ -20,11 +20,16 @@
> >>>>>> #include <linux/pinctrl/pinmux.h>
> >>>>>> #include <linux/pinctrl/pinconf.h>
> >>>>>> #include <linux/slab.h>
> >>>>>> +#include <linux/syscore_ops.h>
> >>>>>> #include "../core.h"
> >>>>>> #include "../pinctrl-utils.h"
> >>>>>> #include "pinctrl-tegra.h"
> >>>>>> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
> >>>>>> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
> >>>>>> +#define EMMC_DPD_PARKING (0x1fff << 14)
> >>>>>> +
> >>>>>> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
> >>>>>> {
> >>>>>> return readl(pmx->regs[bank] + reg);
> >>>>>> @@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
> >>>>>> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
> >>>>>> }
> >>>>>> }
> >>>>>> +
> >>>>>> + if (pmx->soc->has_park_padcfg) {
> >>>>>> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
> >>>>>> + val &= ~EMMC_DPD_PARKING;
> >>>>>> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
> >>>>>> +
> >>>>>> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
> >>>>>> + val &= ~EMMC_DPD_PARKING;
> >>>>>> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
> >>>>>> + }
> >>>>>> +}
> >>>>>
> >>>>> Is there any reason why parked_bit can't be changed to parked_bitmask like I was
> >>>>> asking in a comment to v2?
> >>>>>
> >>>>> I suppose that it's more preferable to keep pinctrl-tegra.c platform-agnostic for
> >>>>> consistency when possible, hence adding platform specifics here should be discouraged.
> >>>>> And then the parked_bitmask will also result in a proper hardware description in the code.
> >>>>>
> >>>>
> >>>> I'm now also vaguely recalling that Stephen Warren had some kind of a "code generator"
> >>>> for the pinctrl drivers. So I guess all those tables were auto-generated initially.
> >>>>
> >>>> Stephen, maybe you could adjust the generator to take into account the bitmask (of
> >>>> course if that's a part of the generated code) and then re-gen it all for Sowjanya?
> >>>
> >>> https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that generate
> >>> tegra-pinctrlNNN.c. See soc-to-kernel-pinctrl-driver.py. IIRC, tegra-pinctrl.c (the core
> >>> file) isn't auto-generated. Sowjanya is welcome to send a patch to that repo if the code
> >>> needs to be updated.
> >>
> >>
> >> Hi Dmitry,
> >>
> >> Just want to be clear on my understanding of your request.
> >>
> >> "change parked_bit to parked_bitmask" are you requested to change parked_bit of PINGROUP
> >> and DRV_PINGROUP to use bitmask value rather than bit position inorder to have parked bit
> >> configuration for EMMC PADs as well to happen by masking rather than checking for
> >> existence of parked_bit?
> >>
> >> Trying to understand the reason/benefit for changing parked_bit to parked_bitmask.
> > Also, Park bits in CFGPAD registers are not common for all CFGPAD registers. Park bits are
> > available only for EMMC and also those bits are used for something else on other CFGPAD
> > registers so bitmask can't be common and this also need an update to DRV_PINGROUP macro args
> > just only to handle EMMC parked_bitmask. So not sure of the benefit in using bitmask rather
>
> Hi Sowjanya,
>
> The main motivation is to describe hardware properly in the drivers. Why to make a
> hacky-looking workaround while you can make things properly? Especially if that doesn't take
> much effort.
>
> Stephen, thank you very much for the pointer to the script. Looks like it should be easy to
> modify the script accordingly to the required change.
>
> Sowjanya, below is a draft of the change that I'm suggesting. I see this as two separate
> patches: first converts drivers to use parked_bitmask, second adds suspend-resume support.
>
> Please note that in the end it's up to you and Tegra/PINCTRL maintainers to decide if this
> is a worthwhile change that I'm suggesting. In my opinion it is much better to have a
> generic solution rather than to have a special quirk solely for T210.
>
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> index 34596b246578..4150da74bd44 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> @@ -613,9 +613,9 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>
> for (i = 0; i < pmx->soc->ngroups; ++i) {
> g = &pmx->soc->groups[i];
> - if (g->parked_bit >= 0) {
> + if (g->parked_bitmask != -1) {
> val = pmx_readl(pmx, g->mux_bank, g->mux_reg);
> - val &= ~(1 << g->parked_bit);
> + val &= ~g->parked_bitmask;
> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
> }
> }
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
> index 287702660783..875eb7a1d838 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra.h
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
> @@ -96,7 +96,7 @@ struct tegra_function {
> * @tri_reg: Tri-state register offset.
> * @tri_bank: Tri-state register bank.
> * @tri_bit: Tri-state register bit.
> - * @parked_bit: Parked register bit. -1 if unsupported.
> + * @parked_bitmask: Parked register bitmask. -1 if unsupported.
If we're already moving to a bitmask, wouldn't it be easier to just make
0 the case where it is unsupported?
> * @einput_bit: Enable-input register bit.
> * @odrain_bit: Open-drain register bit.
> * @lock_bit: Lock register bit.
> @@ -146,7 +146,7 @@ struct tegra_pingroup {
> s32 mux_bit:6;
> s32 pupd_bit:6;
> s32 tri_bit:6;
> - s32 parked_bit:6;
> + s32 parked_bitmask:26;
If we make parked_bitmask == 0 the case for "unsupported" we could make
this u32 while at it.
> s32 einput_bit:6;
> s32 odrain_bit:6;
> s32 lock_bit:6;
> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210.c b/drivers/pinctrl/tegra/pinctrl-tegra210.c
> index 0b56ad5c9c1c..d2ba13466e06 100644
> --- a/drivers/pinctrl/tegra/pinctrl-tegra210.c
> +++ b/drivers/pinctrl/tegra/pinctrl-tegra210.c
> @@ -1302,7 +1302,7 @@ static struct tegra_function tegra210_functions[] = {
> .lock_bit = 7, \
> .ioreset_bit = -1, \
> .rcv_sel_bit = PINGROUP_BIT_##e_io_hv(10), \
> - .parked_bit = 5, \
> + .parked_bitmask = BIT(5), \
> .hsm_bit = PINGROUP_BIT_##hsm(9), \
> .schmitt_bit = 12, \
> .drvtype_bit = PINGROUP_BIT_##drvtype(13), \
> @@ -1320,7 +1320,7 @@ static struct tegra_function tegra210_functions[] = {
> }
>
> #define DRV_PINGROUP(pg_name, r, drvdn_b, drvdn_w, drvup_b, drvup_w, \
> - slwr_b, slwr_w, slwf_b, slwf_w) \
> + slwr_b, slwr_w, slwf_b, slwf_w, prk_mask) \
> { \
> .name = "drive_" #pg_name, \
> .pins = drive_##pg_name##_pins, \
> @@ -1335,7 +1335,7 @@ static struct tegra_function tegra210_functions[] = {
> .rcv_sel_bit = -1, \
> .drv_reg = DRV_PINGROUP_REG(r), \
> .drv_bank = 0, \
> - .parked_bit = -1, \
> + .parked_bitmask = prk_mask, \
> .hsm_bit = -1, \
> .schmitt_bit = -1, \
> .lpmd_bit = -1, \
> @@ -1516,31 +1516,31 @@ static const struct tegra_pingroup tegra210_groups[] = {
> PINGROUP(pz5, SOC, RSVD1, RSVD2, RSVD3, 0x3290, N, N, N,
> -1, -1, -1, -1, -1, -1, -1, -1, -1),
>
> /* pg_name, r, drvdn_b, drvdn_w, drvup_b, drvup_w, slwr_b, slwr_w, slwf_b, slwf_w */
> - DRV_PINGROUP(pa6, 0x9c0, 12, 5, 20, 5, -1, -1, -1, -1),
> - DRV_PINGROUP(pcc7, 0x9c4, 12, 5, 20, 5, -1, -1, -1, -1),
> - DRV_PINGROUP(pe6, 0x9c8, 12, 5, 20, 5, -1, -1, -1, -1),
> - DRV_PINGROUP(pe7, 0x9cc, 12, 5, 20, 5, -1, -1, -1, -1),
> - DRV_PINGROUP(ph6, 0x9d0, 12, 5, 20, 5, -1, -1, -1, -1),
> - DRV_PINGROUP(pk0, 0x9d4, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk1, 0x9d8, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk2, 0x9dc, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk3, 0x9e0, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk4, 0x9e4, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk5, 0x9e8, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk6, 0x9ec, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pk7, 0x9f0, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pl0, 0x9f4, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pl1, 0x9f8, -1, -1, -1, -1, 28, 2, 30, 2),
> - DRV_PINGROUP(pz0, 0x9fc, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(pz1, 0xa00, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(pz2, 0xa04, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(pz3, 0xa08, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(pz4, 0xa0c, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(pz5, 0xa10, 12, 7, 20, 7, -1, -1, -1, -1),
> - DRV_PINGROUP(sdmmc1, 0xa98, 12, 7, 20, 7, 28, 2, 30, 2),
> - DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2),
> - DRV_PINGROUP(sdmmc3, 0xab0, 12, 7, 20, 7, 28, 2, 30, 2),
> - DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2),
> + DRV_PINGROUP(pa6, 0x9c0, 12, 5, 20, 5, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pcc7, 0x9c4, 12, 5, 20, 5, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pe6, 0x9c8, 12, 5, 20, 5, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pe7, 0x9cc, 12, 5, 20, 5, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(ph6, 0x9d0, 12, 5, 20, 5, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pk0, 0x9d4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk1, 0x9d8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk2, 0x9dc, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk3, 0x9e0, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk4, 0x9e4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk5, 0x9e8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk6, 0x9ec, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pk7, 0x9f0, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pl0, 0x9f4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pl1, 0x9f8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(pz0, 0x9fc, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pz1, 0xa00, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pz2, 0xa04, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pz3, 0xa08, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pz4, 0xa0c, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(pz5, 0xa10, 12, 7, 20, 7, -1, -1, -1, -1, -1),
> + DRV_PINGROUP(sdmmc1, 0xa98, 12, 7, 20, 7, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2, 0x7ffc000),
> + DRV_PINGROUP(sdmmc3, 0xab0, 12, 7, 20, 7, 28, 2, 30, 2, -1),
> + DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2, 0x7ffc000),
Might be worth adding a new DRV_PINGROUP_PARK (or whatever) macro that
takes the additional parameter. that way we could avoid the extra churn.
Thierry
> };
>
> static const struct tegra_pinctrl_soc_data tegra210_pinctrl = {
On Tue, Jun 18, 2019 at 09:41:03AM -0600, Stephen Warren wrote:
> On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
> > 18.06.2019 12:22, Dmitry Osipenko пишет:
> > > 18.06.2019 10:46, Sowjanya Komatineni пишет:
> > > > This patch adds suspend and resume support for Tegra pinctrl driver
> > > > and registers them to syscore so the pinmux settings are restored
> > > > before the devices resume.
> > > >
> > > > Signed-off-by: Sowjanya Komatineni <[email protected]>
> > > > ---
> > > > drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
> > > > drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
> > > > drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
> > > > drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
> > > > drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
> > > > drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
> > > > drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
> > > > 7 files changed, 84 insertions(+)
> > > >
> > > > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> > > > index 34596b246578..ceced30d8bd1 100644
> > > > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> > > > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> > > > @@ -20,11 +20,16 @@
> > > > #include <linux/pinctrl/pinmux.h>
> > > > #include <linux/pinctrl/pinconf.h>
> > > > #include <linux/slab.h>
> > > > +#include <linux/syscore_ops.h>
> > > > #include "../core.h"
> > > > #include "../pinctrl-utils.h"
> > > > #include "pinctrl-tegra.h"
> > > > +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
> > > > +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
> > > > +#define EMMC_DPD_PARKING (0x1fff << 14)
> > > > +
> > > > static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
> > > > {
> > > > return readl(pmx->regs[bank] + reg);
> > > > @@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
> > > > pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
> > > > }
> > > > }
> > > > +
> > > > + if (pmx->soc->has_park_padcfg) {
> > > > + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
> > > > + val &= ~EMMC_DPD_PARKING;
> > > > + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
> > > > +
> > > > + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
> > > > + val &= ~EMMC_DPD_PARKING;
> > > > + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
> > > > + }
> > > > +}
> > >
> > > Is there any reason why parked_bit can't be changed to parked_bitmask like I was
> > > asking in a comment to v2?
> > >
> > > I suppose that it's more preferable to keep pinctrl-tegra.c platform-agnostic for
> > > consistency when possible, hence adding platform specifics here should be discouraged.
> > > And then the parked_bitmask will also result in a proper hardware description in the code.
> > >
> >
> > I'm now also vaguely recalling that Stephen Warren had some kind of a "code generator"
> > for the pinctrl drivers. So I guess all those tables were auto-generated initially.
> >
> > Stephen, maybe you could adjust the generator to take into account the bitmask (of
> > course if that's a part of the generated code) and then re-gen it all for Sowjanya?
>
> https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that
> generate tegra-pinctrlNNN.c. See soc-to-kernel-pinctrl-driver.py. IIRC,
> tegra-pinctrl.c (the core file) isn't auto-generated. Sowjanya is welcome to
> send a patch to that repo if the code needs to be updated.
If we want to do that, we may need to start off by bringing the pinmux
scripts up to date with the latest version of the generated files. There
have been a number of changes in the meantime that cause the scripts to
generate a bit of diff with regards to what's currently upstream. Sounds
like something fairly trivial, though.
Thierry
19.06.2019 11:31, Thierry Reding пишет:
> On Tue, Jun 18, 2019 at 11:00:05PM +0300, Dmitry Osipenko wrote:
>> 18.06.2019 20:34, Sowjanya Komatineni пишет:
>>>
>>> On 6/18/19 9:50 AM, Sowjanya Komatineni wrote:
>>>>
>>>> On 6/18/19 8:41 AM, Stephen Warren wrote:
>>>>> On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
>>>>>> 18.06.2019 12:22, Dmitry Osipenko пишет:
>>>>>>> 18.06.2019 10:46, Sowjanya Komatineni пишет:
>>>>>>>> This patch adds suspend and resume support for Tegra pinctrl driver
>>>>>>>> and registers them to syscore so the pinmux settings are restored
>>>>>>>> before the devices resume.
>>>>>>>>
>>>>>>>> Signed-off-by: Sowjanya Komatineni <[email protected]>
>>>>>>>> ---
>>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
>>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
>>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
>>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
>>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
>>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
>>>>>>>> drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
>>>>>>>> 7 files changed, 84 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>> b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>> index 34596b246578..ceced30d8bd1 100644
>>>>>>>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>>>>>>>> @@ -20,11 +20,16 @@
>>>>>>>> #include <linux/pinctrl/pinmux.h>
>>>>>>>> #include <linux/pinctrl/pinconf.h>
>>>>>>>> #include <linux/slab.h>
>>>>>>>> +#include <linux/syscore_ops.h>
>>>>>>>> #include "../core.h"
>>>>>>>> #include "../pinctrl-utils.h"
>>>>>>>> #include "pinctrl-tegra.h"
>>>>>>>> +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
>>>>>>>> +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
>>>>>>>> +#define EMMC_DPD_PARKING (0x1fff << 14)
>>>>>>>> +
>>>>>>>> static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
>>>>>>>> {
>>>>>>>> return readl(pmx->regs[bank] + reg);
>>>>>>>> @@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>>>>>>> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
>>>>>>>> }
>>>>>>>> }
>>>>>>>> +
>>>>>>>> + if (pmx->soc->has_park_padcfg) {
>>>>>>>> + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>>> + val &= ~EMMC_DPD_PARKING;
>>>>>>>> + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
>>>>>>>> +
>>>>>>>> + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>>> + val &= ~EMMC_DPD_PARKING;
>>>>>>>> + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
>>>>>>>> + }
>>>>>>>> +}
>>>>>>>
>>>>>>> Is there any reason why parked_bit can't be changed to parked_bitmask like I was
>>>>>>> asking in a comment to v2?
>>>>>>>
>>>>>>> I suppose that it's more preferable to keep pinctrl-tegra.c platform-agnostic for
>>>>>>> consistency when possible, hence adding platform specifics here should be discouraged.
>>>>>>> And then the parked_bitmask will also result in a proper hardware description in the code.
>>>>>>>
>>>>>>
>>>>>> I'm now also vaguely recalling that Stephen Warren had some kind of a "code generator"
>>>>>> for the pinctrl drivers. So I guess all those tables were auto-generated initially.
>>>>>>
>>>>>> Stephen, maybe you could adjust the generator to take into account the bitmask (of
>>>>>> course if that's a part of the generated code) and then re-gen it all for Sowjanya?
>>>>>
>>>>> https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that generate
>>>>> tegra-pinctrlNNN.c. See soc-to-kernel-pinctrl-driver.py. IIRC, tegra-pinctrl.c (the core
>>>>> file) isn't auto-generated. Sowjanya is welcome to send a patch to that repo if the code
>>>>> needs to be updated.
>>>>
>>>>
>>>> Hi Dmitry,
>>>>
>>>> Just want to be clear on my understanding of your request.
>>>>
>>>> "change parked_bit to parked_bitmask" are you requested to change parked_bit of PINGROUP
>>>> and DRV_PINGROUP to use bitmask value rather than bit position inorder to have parked bit
>>>> configuration for EMMC PADs as well to happen by masking rather than checking for
>>>> existence of parked_bit?
>>>>
>>>> Trying to understand the reason/benefit for changing parked_bit to parked_bitmask.
>>> Also, Park bits in CFGPAD registers are not common for all CFGPAD registers. Park bits are
>>> available only for EMMC and also those bits are used for something else on other CFGPAD
>>> registers so bitmask can't be common and this also need an update to DRV_PINGROUP macro args
>>> just only to handle EMMC parked_bitmask. So not sure of the benefit in using bitmask rather
>>
>> Hi Sowjanya,
>>
>> The main motivation is to describe hardware properly in the drivers. Why to make a
>> hacky-looking workaround while you can make things properly? Especially if that doesn't take
>> much effort.
>>
>> Stephen, thank you very much for the pointer to the script. Looks like it should be easy to
>> modify the script accordingly to the required change.
>>
>> Sowjanya, below is a draft of the change that I'm suggesting. I see this as two separate
>> patches: first converts drivers to use parked_bitmask, second adds suspend-resume support.
>>
>> Please note that in the end it's up to you and Tegra/PINCTRL maintainers to decide if this
>> is a worthwhile change that I'm suggesting. In my opinion it is much better to have a
>> generic solution rather than to have a special quirk solely for T210.
>>
>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
>> index 34596b246578..4150da74bd44 100644
>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
>> @@ -613,9 +613,9 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
>>
>> for (i = 0; i < pmx->soc->ngroups; ++i) {
>> g = &pmx->soc->groups[i];
>> - if (g->parked_bit >= 0) {
>> + if (g->parked_bitmask != -1) {
>> val = pmx_readl(pmx, g->mux_bank, g->mux_reg);
>> - val &= ~(1 << g->parked_bit);
>> + val &= ~g->parked_bitmask;
>> pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
>> }
>> }
>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.h b/drivers/pinctrl/tegra/pinctrl-tegra.h
>> index 287702660783..875eb7a1d838 100644
>> --- a/drivers/pinctrl/tegra/pinctrl-tegra.h
>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra.h
>> @@ -96,7 +96,7 @@ struct tegra_function {
>> * @tri_reg: Tri-state register offset.
>> * @tri_bank: Tri-state register bank.
>> * @tri_bit: Tri-state register bit.
>> - * @parked_bit: Parked register bit. -1 if unsupported.
>> + * @parked_bitmask: Parked register bitmask. -1 if unsupported.
>
> If we're already moving to a bitmask, wouldn't it be easier to just make
> 0 the case where it is unsupported?
>
>> * @einput_bit: Enable-input register bit.
>> * @odrain_bit: Open-drain register bit.
>> * @lock_bit: Lock register bit.
>> @@ -146,7 +146,7 @@ struct tegra_pingroup {
>> s32 mux_bit:6;
>> s32 pupd_bit:6;
>> s32 tri_bit:6;
>> - s32 parked_bit:6;
>> + s32 parked_bitmask:26;
>
> If we make parked_bitmask == 0 the case for "unsupported" we could make
> this u32 while at it.
>
>> s32 einput_bit:6;
>> s32 odrain_bit:6;
>> s32 lock_bit:6;
>> diff --git a/drivers/pinctrl/tegra/pinctrl-tegra210.c b/drivers/pinctrl/tegra/pinctrl-tegra210.c
>> index 0b56ad5c9c1c..d2ba13466e06 100644
>> --- a/drivers/pinctrl/tegra/pinctrl-tegra210.c
>> +++ b/drivers/pinctrl/tegra/pinctrl-tegra210.c
>> @@ -1302,7 +1302,7 @@ static struct tegra_function tegra210_functions[] = {
>> .lock_bit = 7, \
>> .ioreset_bit = -1, \
>> .rcv_sel_bit = PINGROUP_BIT_##e_io_hv(10), \
>> - .parked_bit = 5, \
>> + .parked_bitmask = BIT(5), \
>> .hsm_bit = PINGROUP_BIT_##hsm(9), \
>> .schmitt_bit = 12, \
>> .drvtype_bit = PINGROUP_BIT_##drvtype(13), \
>> @@ -1320,7 +1320,7 @@ static struct tegra_function tegra210_functions[] = {
>> }
>>
>> #define DRV_PINGROUP(pg_name, r, drvdn_b, drvdn_w, drvup_b, drvup_w, \
>> - slwr_b, slwr_w, slwf_b, slwf_w) \
>> + slwr_b, slwr_w, slwf_b, slwf_w, prk_mask) \
>> { \
>> .name = "drive_" #pg_name, \
>> .pins = drive_##pg_name##_pins, \
>> @@ -1335,7 +1335,7 @@ static struct tegra_function tegra210_functions[] = {
>> .rcv_sel_bit = -1, \
>> .drv_reg = DRV_PINGROUP_REG(r), \
>> .drv_bank = 0, \
>> - .parked_bit = -1, \
>> + .parked_bitmask = prk_mask, \
>> .hsm_bit = -1, \
>> .schmitt_bit = -1, \
>> .lpmd_bit = -1, \
>> @@ -1516,31 +1516,31 @@ static const struct tegra_pingroup tegra210_groups[] = {
>> PINGROUP(pz5, SOC, RSVD1, RSVD2, RSVD3, 0x3290, N, N, N,
>> -1, -1, -1, -1, -1, -1, -1, -1, -1),
>>
>> /* pg_name, r, drvdn_b, drvdn_w, drvup_b, drvup_w, slwr_b, slwr_w, slwf_b, slwf_w */
>> - DRV_PINGROUP(pa6, 0x9c0, 12, 5, 20, 5, -1, -1, -1, -1),
>> - DRV_PINGROUP(pcc7, 0x9c4, 12, 5, 20, 5, -1, -1, -1, -1),
>> - DRV_PINGROUP(pe6, 0x9c8, 12, 5, 20, 5, -1, -1, -1, -1),
>> - DRV_PINGROUP(pe7, 0x9cc, 12, 5, 20, 5, -1, -1, -1, -1),
>> - DRV_PINGROUP(ph6, 0x9d0, 12, 5, 20, 5, -1, -1, -1, -1),
>> - DRV_PINGROUP(pk0, 0x9d4, -1, -1, -1, -1, 28, 2, 30, 2),
>> - DRV_PINGROUP(pk1, 0x9d8, -1, -1, -1, -1, 28, 2, 30, 2),
>> - DRV_PINGROUP(pk2, 0x9dc, -1, -1, -1, -1, 28, 2, 30, 2),
>> - DRV_PINGROUP(pk3, 0x9e0, -1, -1, -1, -1, 28, 2, 30, 2),
>> - DRV_PINGROUP(pk4, 0x9e4, -1, -1, -1, -1, 28, 2, 30, 2),
>> - DRV_PINGROUP(pk5, 0x9e8, -1, -1, -1, -1, 28, 2, 30, 2),
>> - DRV_PINGROUP(pk6, 0x9ec, -1, -1, -1, -1, 28, 2, 30, 2),
>> - DRV_PINGROUP(pk7, 0x9f0, -1, -1, -1, -1, 28, 2, 30, 2),
>> - DRV_PINGROUP(pl0, 0x9f4, -1, -1, -1, -1, 28, 2, 30, 2),
>> - DRV_PINGROUP(pl1, 0x9f8, -1, -1, -1, -1, 28, 2, 30, 2),
>> - DRV_PINGROUP(pz0, 0x9fc, 12, 7, 20, 7, -1, -1, -1, -1),
>> - DRV_PINGROUP(pz1, 0xa00, 12, 7, 20, 7, -1, -1, -1, -1),
>> - DRV_PINGROUP(pz2, 0xa04, 12, 7, 20, 7, -1, -1, -1, -1),
>> - DRV_PINGROUP(pz3, 0xa08, 12, 7, 20, 7, -1, -1, -1, -1),
>> - DRV_PINGROUP(pz4, 0xa0c, 12, 7, 20, 7, -1, -1, -1, -1),
>> - DRV_PINGROUP(pz5, 0xa10, 12, 7, 20, 7, -1, -1, -1, -1),
>> - DRV_PINGROUP(sdmmc1, 0xa98, 12, 7, 20, 7, 28, 2, 30, 2),
>> - DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2),
>> - DRV_PINGROUP(sdmmc3, 0xab0, 12, 7, 20, 7, 28, 2, 30, 2),
>> - DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2),
>> + DRV_PINGROUP(pa6, 0x9c0, 12, 5, 20, 5, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(pcc7, 0x9c4, 12, 5, 20, 5, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(pe6, 0x9c8, 12, 5, 20, 5, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(pe7, 0x9cc, 12, 5, 20, 5, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(ph6, 0x9d0, 12, 5, 20, 5, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(pk0, 0x9d4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(pk1, 0x9d8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(pk2, 0x9dc, -1, -1, -1, -1, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(pk3, 0x9e0, -1, -1, -1, -1, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(pk4, 0x9e4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(pk5, 0x9e8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(pk6, 0x9ec, -1, -1, -1, -1, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(pk7, 0x9f0, -1, -1, -1, -1, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(pl0, 0x9f4, -1, -1, -1, -1, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(pl1, 0x9f8, -1, -1, -1, -1, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(pz0, 0x9fc, 12, 7, 20, 7, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(pz1, 0xa00, 12, 7, 20, 7, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(pz2, 0xa04, 12, 7, 20, 7, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(pz3, 0xa08, 12, 7, 20, 7, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(pz4, 0xa0c, 12, 7, 20, 7, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(pz5, 0xa10, 12, 7, 20, 7, -1, -1, -1, -1, -1),
>> + DRV_PINGROUP(sdmmc1, 0xa98, 12, 7, 20, 7, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(sdmmc2, 0xa9c, 2, 6, 8, 6, 28, 2, 30, 2, 0x7ffc000),
>> + DRV_PINGROUP(sdmmc3, 0xab0, 12, 7, 20, 7, 28, 2, 30, 2, -1),
>> + DRV_PINGROUP(sdmmc4, 0xab4, 2, 6, 8, 6, 28, 2, 30, 2, 0x7ffc000),
>
> Might be worth adding a new DRV_PINGROUP_PARK (or whatever) macro that
> takes the additional parameter. that way we could avoid the extra churn.
Sounds like a very good call! +1
On Wed, Jun 19, 2019 at 10:33:08AM +0200, Thierry Reding wrote:
> On Tue, Jun 18, 2019 at 09:41:03AM -0600, Stephen Warren wrote:
> > On 6/18/19 3:30 AM, Dmitry Osipenko wrote:
> > > 18.06.2019 12:22, Dmitry Osipenko пишет:
> > > > 18.06.2019 10:46, Sowjanya Komatineni пишет:
> > > > > This patch adds suspend and resume support for Tegra pinctrl driver
> > > > > and registers them to syscore so the pinmux settings are restored
> > > > > before the devices resume.
> > > > >
> > > > > Signed-off-by: Sowjanya Komatineni <[email protected]>
> > > > > ---
> > > > > drivers/pinctrl/tegra/pinctrl-tegra.c | 62 ++++++++++++++++++++++++++++++++
> > > > > drivers/pinctrl/tegra/pinctrl-tegra.h | 5 +++
> > > > > drivers/pinctrl/tegra/pinctrl-tegra114.c | 1 +
> > > > > drivers/pinctrl/tegra/pinctrl-tegra124.c | 1 +
> > > > > drivers/pinctrl/tegra/pinctrl-tegra20.c | 1 +
> > > > > drivers/pinctrl/tegra/pinctrl-tegra210.c | 13 +++++++
> > > > > drivers/pinctrl/tegra/pinctrl-tegra30.c | 1 +
> > > > > 7 files changed, 84 insertions(+)
> > > > >
> > > > > diff --git a/drivers/pinctrl/tegra/pinctrl-tegra.c b/drivers/pinctrl/tegra/pinctrl-tegra.c
> > > > > index 34596b246578..ceced30d8bd1 100644
> > > > > --- a/drivers/pinctrl/tegra/pinctrl-tegra.c
> > > > > +++ b/drivers/pinctrl/tegra/pinctrl-tegra.c
> > > > > @@ -20,11 +20,16 @@
> > > > > #include <linux/pinctrl/pinmux.h>
> > > > > #include <linux/pinctrl/pinconf.h>
> > > > > #include <linux/slab.h>
> > > > > +#include <linux/syscore_ops.h>
> > > > > #include "../core.h"
> > > > > #include "../pinctrl-utils.h"
> > > > > #include "pinctrl-tegra.h"
> > > > > +#define EMMC2_PAD_CFGPADCTRL_0 0x1c8
> > > > > +#define EMMC4_PAD_CFGPADCTRL_0 0x1e0
> > > > > +#define EMMC_DPD_PARKING (0x1fff << 14)
> > > > > +
> > > > > static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
> > > > > {
> > > > > return readl(pmx->regs[bank] + reg);
> > > > > @@ -619,6 +624,48 @@ static void tegra_pinctrl_clear_parked_bits(struct tegra_pmx *pmx)
> > > > > pmx_writel(pmx, val, g->mux_bank, g->mux_reg);
> > > > > }
> > > > > }
> > > > > +
> > > > > + if (pmx->soc->has_park_padcfg) {
> > > > > + val = pmx_readl(pmx, 0, EMMC2_PAD_CFGPADCTRL_0);
> > > > > + val &= ~EMMC_DPD_PARKING;
> > > > > + pmx_writel(pmx, val, 0, EMMC2_PAD_CFGPADCTRL_0);
> > > > > +
> > > > > + val = pmx_readl(pmx, 0, EMMC4_PAD_CFGPADCTRL_0);
> > > > > + val &= ~EMMC_DPD_PARKING;
> > > > > + pmx_writel(pmx, val, 0, EMMC4_PAD_CFGPADCTRL_0);
> > > > > + }
> > > > > +}
> > > >
> > > > Is there any reason why parked_bit can't be changed to parked_bitmask like I was
> > > > asking in a comment to v2?
> > > >
> > > > I suppose that it's more preferable to keep pinctrl-tegra.c platform-agnostic for
> > > > consistency when possible, hence adding platform specifics here should be discouraged.
> > > > And then the parked_bitmask will also result in a proper hardware description in the code.
> > > >
> > >
> > > I'm now also vaguely recalling that Stephen Warren had some kind of a "code generator"
> > > for the pinctrl drivers. So I guess all those tables were auto-generated initially.
> > >
> > > Stephen, maybe you could adjust the generator to take into account the bitmask (of
> > > course if that's a part of the generated code) and then re-gen it all for Sowjanya?
> >
> > https://github.com/NVIDIA/tegra-pinmux-scripts holds the scripts that
> > generate tegra-pinctrlNNN.c. See soc-to-kernel-pinctrl-driver.py. IIRC,
> > tegra-pinctrl.c (the core file) isn't auto-generated. Sowjanya is welcome to
> > send a patch to that repo if the code needs to be updated.
>
> If we want to do that, we may need to start off by bringing the pinmux
> scripts up to date with the latest version of the generated files. There
> have been a number of changes in the meantime that cause the scripts to
> generate a bit of diff with regards to what's currently upstream. Sounds
> like something fairly trivial, though.
Something like the below should do the trick.
Thierry
--- >8 ---
From 9a684d2ad3c0e0c7b4dbda5904db1fda3757072b Mon Sep 17 00:00:00 2001
From: Thierry Reding <[email protected]>
Date: Wed, 19 Jun 2019 10:50:57 +0200
Subject: [pinmux scripts PATCH] Update kernel driver template
Some changes in recent years have modified the upstream kernel driver in
some ways that make it incompatible with the current template. Update
the template to take into account changes introduced by the following
commits:
commit e3d2160f12d6aa7a87d9db09d8458b4a3492cd45
Author: Paul Gortmaker <[email protected]>
Date: Mon May 22 16:56:47 2017 -0400
pinctrl: tegra: clean up modular vs. non-modular distinctions
None of the Kconfigs for any of these drivers are tristate,
meaning that they currently are not being built as a module by anyone.
Lets remove the modular code that is essentially orphaned, so that
when reading the drivers there is no doubt they are builtin-only. All
drivers get similar changes, so they are handled in batch.
We remove module.h from code that isn't doing anything modular at
all; if they have __init sections, then replace it with init.h.
A couple drivers have module_exit() code that is essentially orphaned,
and so we remove that.
Quite a few bool drivers (hence non-modular) are converted over to
to builtin_platform_driver().
Since module_platform_driver() uses the same init level priority as
builtin_platform_driver() the init ordering remains unchanged with
this commit.
Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
We also delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.
Cc: Linus Walleij <[email protected]>
Cc: Stephen Warren <[email protected]>
Cc: Thierry Reding <[email protected]>
Cc: Alexandre Courbot <[email protected]>
Cc: Pritesh Raithatha <[email protected]>
Cc: Ashwini Ghuge <[email protected]>
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Paul Gortmaker <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
commit 3c94d2d08a032d911bbe34f2edb24cb63a63644a
Author: Stefan Agner <[email protected]>
Date: Thu Jul 26 17:40:24 2018 +0200
pinctrl: tegra: define GPIO compatible node per SoC
Tegra 2 uses a different GPIO controller which uses "tegra20-gpio" as
compatible string.
Make the compatible string the GPIO node is using a SoC specific
property. This prevents the kernel from registering the GPIO range
twice in case the GPIO range is specified in the device tree.
Fixes: 9462510ce31e ("pinctrl: tegra: Only set the gpio range if needed")
Signed-off-by: Stefan Agner <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
commit 1e0813ee5599932c856bda64a568895ed7a33d3a
Author: Dmitry Osipenko <[email protected]>
Date: Thu Aug 2 14:11:43 2018 +0300
pinctrl: tegra: Move drivers registration to arch_init level
There is a bug in regards to deferred probing within the drivers core
that causes GPIO-driver to suspend after its users. The bug appears if
GPIO-driver probe is getting deferred, which happens after introducing
dependency on PINCTRL-driver for the GPIO-driver by defining "gpio-ranges"
property in device-tree. The bug in the drivers core is old (more than 4
years now) and is well known, unfortunately there is no easy fix for it.
The good news is that we can workaround the deferred probe issue by
changing GPIO / PINCTRL drivers registration order and hence by moving
PINCTRL driver registration to the arch_init level and GPIO to the
subsys_init.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Stefan Agner <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
Note that the last one is something that we probably should fix
correctly by using device links rather than working around it by playing
init level tricks.
Signed-off-by: Thierry Reding <[email protected]>
---
soc-to-kernel-pinctrl-driver.py | 27 +++++++++++----------------
1 file changed, 11 insertions(+), 16 deletions(-)
diff --git a/soc-to-kernel-pinctrl-driver.py b/soc-to-kernel-pinctrl-driver.py
index 65e4c604f1c9..37f34b15db2b 100755
--- a/soc-to-kernel-pinctrl-driver.py
+++ b/soc-to-kernel-pinctrl-driver.py
@@ -41,22 +41,16 @@ if dbg: print(args)
soc = tegra_pmx_soc_parser.load_soc(args.soc)
print('''\
+// SPDX-License-Identifier: GPL-2.0-only
/*
* Pinctrl data for the NVIDIA %s pinmux
*
- * Copyright (c) %s, NVIDIA CORPORATION. All rights reserved.
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms and conditions of the GNU General Public License,
- * version 2, as published by the Free Software Foundation.
+ * Author: %s
*
- * This program is distributed in the hope it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
- * more details.
+ * Copyright (c) %s, NVIDIA CORPORATION. All rights reserved.
*/
-#include <linux/module.h>
+#include <linux/init.h>
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pinctrl/pinctrl.h>
@@ -68,7 +62,7 @@ print('''\
* Most pins affected by the pinmux can also be GPIOs. Define these first.
* These must match how the GPIO driver names/numbers its pins.
*/
-''' % (soc.titlename, soc.kernel_copyright_years), end='')
+''' % (soc.titlename, soc.kernel_author, soc.kernel_copyright_years), end='')
# Do not add any more exceptions here; new SoCs should be formatted correctly
if soc.name == 'tegra30':
@@ -615,6 +609,7 @@ print('''\
static const struct tegra_pinctrl_soc_data %(soc)s_pinctrl = {
.ngpios = NUM_GPIOS,
+ .gpio_compatible = "nvidia,%(soc)s-gpio",
.pins = %(soc)s_pins,
.npins = ARRAY_SIZE(%(soc)s_pins),
.functions = %(soc)s_functions,
@@ -635,7 +630,6 @@ static const struct of_device_id %(soc)s_pinctrl_of_match[] = {
{ .compatible = "nvidia,%(soc)s-pinmux", },
{ },
};
-MODULE_DEVICE_TABLE(of, %(soc)s_pinctrl_of_match);
static struct platform_driver %(soc)s_pinctrl_driver = {
.driver = {
@@ -644,9 +638,10 @@ static struct platform_driver %(soc)s_pinctrl_driver = {
},
.probe = %(soc)s_pinctrl_probe,
};
-module_platform_driver(%(soc)s_pinctrl_driver);
-MODULE_AUTHOR("%(author)s");
-MODULE_DESCRIPTION("NVIDIA %(usoc)s pinctrl driver");
-MODULE_LICENSE("GPL v2");
+static int __init %(soc)s_pinctrl_init(void)
+{
+ return platform_driver_register(&%(soc)s_pinctrl_driver);
+}
+arch_initcall(%(soc)s_pinctrl_init);
''' % socvars, end='')
--
2.21.0