pinctrl: tegra: remove redundant data table fields
Subsystems (2)
PATCH 1/4 pinctrl: tegra: remove redundant data table fields
From: Stephen Warren <[email protected]>
Any SoC which supports the einput, odrain, lock, ioreset, or rcv_sel options has the relevant HW register fields in the same register as the mux function selection. Similarly, the drvtype option is always in the drive register, if it is supported at all. Hence, we don't need to have struct *_reg fields in the pin group table to define which register and bank to use for those options. Delete this to save space in the driver's data tables.
However, many of those options are not supported on all SoCs, or not supported on some pingroups. We need a way to detect when they are supported. Previously, this was indicated by setting the struct *_reg field to -1. With the struct *_reg fields removed, we use the struct *_bit fields for this purpose instead. The struct *_bit fields need to be expanded from 5 to 6 bits in order to store a value outside the valid HW bit range of 0..31.
Even without removing the struct *_reg fields, we still need to add code to validate the struct *_bit fields, since some struct *_bit fields were already being set to -1, without an option-specific struct *_reg field to "guard" them. In other words, before this change, the pinmux driver might allow some unsupported options to be written to HW.
--- drivers/pinctrl/pinctrl-tegra.c | 26 +++++++++---------- drivers/pinctrl/pinctrl-tegra.h | 42 +++++++++++------------------- drivers/pinctrl/pinctrl-tegra114.c | 53 ++++++++++++++------------------------ drivers/pinctrl/pinctrl-tegra124.c | 53 ++++++++++++++------------------------ drivers/pinctrl/pinctrl-tegra20.c | 24 ++++------------- drivers/pinctrl/pinctrl-tegra30.c | 48 ++++++++++++++-------------------- 6 files changed, 93 insertions(+), 153 deletions(-)
drivers/pinctrl/pinctrl-tegra.c +13 -13
@@ -336,32 +336,32 @@ static int tegra_pinconf_reg(struct tegra_pmx *pmx, *width =1 ;break ;case TEGRA_PINCONF_PARAM_ENABLE_INPUT : - *bank = g->einput_bank; - *reg = g->einput_reg; + *bank = g->mux_bank; + *reg = g->mux_reg; *bit = g->einput_bit; *width =1 ;break ;case TEGRA_PINCONF_PARAM_OPEN_DRAIN : - *bank = g->odrain_bank; - *reg = g->odrain_reg; + *bank = g->mux_bank; + *reg = g->mux_reg; *bit = g->odrain_bit; *width =1 ;break ;case TEGRA_PINCONF_PARAM_LOCK : - *bank = g->lock_bank; - *reg = g->lock_reg; + *bank = g->mux_bank; + *reg = g->mux_reg; *bit = g->lock_bit; *width =1 ;break ;case TEGRA_PINCONF_PARAM_IORESET : - *bank = g->ioreset_bank; - *reg = g->ioreset_reg; + *bank = g->mux_bank; + *reg = g->mux_reg; *bit = g->ioreset_bit; *width =1 ;break ;case TEGRA_PINCONF_PARAM_RCV_SEL : - *bank = g->rcv_sel_bank; - *reg = g->rcv_sel_reg; + *bank = g->mux_bank; + *reg = g->mux_reg; *bit = g->rcv_sel_bit; *width =1 ;break ;
@@ -408,8 +408,8 @@ static int tegra_pinconf_reg(struct tegra_pmx *pmx, *width = g->slwr_width;break ;case TEGRA_PINCONF_PARAM_DRIVE_TYPE : - *bank = g->drvtype_bank; - *reg = g->drvtype_reg; + *bank = g->drv_bank; + *reg = g->drv_reg; *bit = g->drvtype_bit; *width =2 ;break ;
@@ -418,7 +418,7 @@ static int tegra_pinconf_reg(struct tegra_pmx *pmx,return -ENOTSUPP ; } -if (*reg <0 ) { +if (*reg <0 || *bit >31 ) {if (report_err) dev_err(pmx->dev, "Config param %04x not supported on group %s\n",
drivers/pinctrl/pinctrl-tegra.h +15 -27
53 lines not on this page · raw message
drivers/pinctrl/pinctrl-tegra114.c +20 -33
86 lines not on this page · raw message
drivers/pinctrl/pinctrl-tegra124.c +20 -33
86 lines not on this page · raw message
drivers/pinctrl/pinctrl-tegra20.c +5 -19
@@ -1995,13 +1995,12 @@ static struct tegra_function tegra20_functions[] = { .tri_reg = ((tri_r) - TRISTATE_REG_A), \ .tri_bank = 0, \ .tri_bit = tri_b, \ - .einput_reg = -1, \ - .odrain_reg = -1, \ - .lock_reg = -1, \ - .ioreset_reg = -1, \ - .rcv_sel_reg = -1, \ + .einput_bit = -1, \ + .odrain_bit = -1, \ + .lock_bit = -1, \ + .ioreset_bit = -1, \ + .rcv_sel_bit = -1, \ .drv_reg = -1, \ - .drvtype_reg = -1, \ }/* Pin groups with only pull up and pull down control */
@@ -2014,14 +2013,7 @@ static struct tegra_function tegra20_functions[] = { .pupd_reg = ((pupd_r) - PULLUPDOWN_REG_A), \ .pupd_bank = 2, \ .pupd_bit = pupd_b, \ - .tri_reg = -1, \ - .einput_reg = -1, \ - .odrain_reg = -1, \ - .lock_reg = -1, \ - .ioreset_reg = -1, \ - .rcv_sel_reg = -1, \ .drv_reg = -1, \ - .drvtype_reg = -1, \ }/* Pin groups for drive strength registers (configurable version) */
@@ -2035,11 +2027,6 @@ static struct tegra_function tegra20_functions[] = { .mux_reg = -1, \ .pupd_reg = -1, \ .tri_reg = -1, \ - .einput_reg = -1, \ - .odrain_reg = -1, \ - .lock_reg = -1, \ - .ioreset_reg = -1, \ - .rcv_sel_reg = -1, \ .drv_reg = ((r) - PINGROUP_REG_A), \ .drv_bank = 3, \ .hsm_bit = hsm_b, \
@@ -2053,7 +2040,6 @@ static struct tegra_function tegra20_functions[] = { .slwr_width = slwr_w, \ .slwf_bit = slwf_b, \ .slwf_width = slwf_w, \ - .drvtype_reg = -1, \ }/* Pin groups for drive strength registers (simple version) */
drivers/pinctrl/pinctrl-tegra30.c +20 -28
81 lines not on this page · raw message
Signature · 1 line
1.8.1.5
A message that is not in the archive (sent off-list or not yet received): 1397511222-28533-1-git-send-email-swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org
PATCH 2/4 pinctrl: tegra: remove fsafe from data tables
From: Stephen Warren <[email protected]>
The fsafe value in the pingroup data tables is only used to implement tegra_pinctrl_disable(). The only reason this function is called is when dynamically switching between pinmux states, i.e. when disabling the old state before programming the new state. It's simpler to have the new target state define the expected value of each pin (and all current DTs do that). This also gives more flexibility, since it allows individual boards explicit control over the "inactive" mux function for each pin, rather than requiring it to be an SoC-specific value. Assuming this, we can get rid of the fsafe value from the driver completely, thus saving some more space in the driver tables.
While re-writing the content of tegra124_pingroups[], fix the indentation to use a TAB instead of spaces.
--- drivers/pinctrl/pinctrl-tegra.c | 6 - drivers/pinctrl/pinctrl-tegra.h | 1 - drivers/pinctrl/pinctrl-tegra114.c | 359 +++++++++++++------------- drivers/pinctrl/pinctrl-tegra124.c | 467 +++++++++++++++++----------------- drivers/pinctrl/pinctrl-tegra20.c | 219 ++++++++-------- drivers/pinctrl/pinctrl-tegra30.c | 503 ++++++++++++++++++------------------- 6 files changed, 772 insertions(+), 783 deletions(-)
drivers/pinctrl/pinctrl-tegra.c +0 -6
@@ -295,17 +295,11 @@ static void tegra_pinctrl_disable(struct pinctrl_dev *pctldev, {struct tegra_pmx *pmx =pinctrl_dev_get_drvdata (pctldev);const struct tegra_pingroup *g; -u32 val; g = &pmx->soc->groups[group];if (WARN_ON (g->mux_reg <0 ))return ; - - val =pmx_readl (pmx, g->mux_bank, g->mux_reg); - val &= ~(0x3 << g->mux_bit); - val |= g->func_safe << g->mux_bit; -pmx_writel (pmx, val, g->mux_bank, g->mux_reg); }static const struct pinmux_ops tegra_pinmux_ops = {
drivers/pinctrl/pinctrl-tegra.h +0 -1
@@ -133,7 +133,6 @@ struct tegra_pingroup {const unsigned *pins;unsigned npins;unsigned funcs[4 ]; -unsigned func_safe;s16 mux_reg;s16 pupd_reg;s16 tri_reg;
drivers/pinctrl/pinctrl-tegra114.c +179 -180
377 lines not on this page · raw message
drivers/pinctrl/pinctrl-tegra124.c +233 -234
487 lines not on this page · raw message
drivers/pinctrl/pinctrl-tegra20.c +109 -110
237 lines not on this page · raw message
drivers/pinctrl/pinctrl-tegra30.c +251 -252
521 lines not on this page · raw message
Signature · 1 line
1.8.1.5
PATCH 2/4 pinctrl: tegra: remove fsafe from data tables
Quoted 13 lines · Stephen Warren
From: Stephen Warren <[email protected]>
The fsafe value in the pingroup data tables is only used to implement
tegra_pinctrl_disable(). The only reason this function is called is when dynamically switching between pinmux states, i.e. when disabling the old state before programming the new state. It's simpler to have the new target state define the expected value of each pin (and all current DTs do that). This also gives more flexibility, since it allows individual boards explicit control over the "inactive" mux function for each pin, rather than requiring it to be an SoC-specific value. Assuming this, we can get rid of the fsafe value from the driver completely, thus saving some more space in the driver tables.
Quoted 3 lines
While re-writing the content of tegra124_pingroups[], fix the indentation to use a TAB instead of spaces.
Looks good to me.
PATCH 2/4 pinctrl: tegra: remove fsafe from data tables
In reply to Stephen WarrenQuoted 13 lines · Stephen Warren
From: Stephen Warren <[email protected]>
The fsafe value in the pingroup data tables is only used to implement
tegra_pinctrl_disable(). The only reason this function is called is when dynamically switching between pinmux states, i.e. when disabling the old state before programming the new state. It's simpler to have the new target state define the expected value of each pin (and all current DTs do that). This also gives more flexibility, since it allows individual boards explicit control over the "inactive" mux function for each pin, rather than requiring it to be an SoC-specific value. Assuming this, we can get rid of the fsafe value from the driver completely, thus saving some more space in the driver tables.
Quoted 4 lines
While re-writing the content of tegra124_pingroups[], fix the indentation to use a TAB instead of spaces.
Signed-off-by Stephen Warren <[email protected]>
Patch applied with Laxman's ACK.
Yours,
Linus Walleij
PATCH 3/4 pinctrl: tegra: reduce size of data table fields
In reply to Stephen WarrenFrom: Stephen Warren <[email protected]>
The range of npins and function ID values is small enough to fit into a u8. Use this type rather than unsigned to shrink the pinmux data tables.
--- drivers/pinctrl/pinctrl-tegra.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/pinctrl/pinctrl-tegra.h +2 -2
@@ -131,8 +131,8 @@ struct tegra_function {struct tegra_pingroup {const char *name;const unsigned *pins; -unsigned npins; -unsigned funcs[4 ]; +u8 npins; +u8 funcs[4 ];s16 mux_reg;s16 pupd_reg;s16 tri_reg;
Signature · 1 line
1.8.1.5
PATCH 3/4 pinctrl: tegra: reduce size of data table fields
Quoted 5 lines · Stephen Warren
From: Stephen Warren <[email protected]>
The range of npins and function ID values is small enough to fit into a u8. Use this type rather than unsigned to shrink the pinmux data tables.
For 3 and 4, also
PATCH 3/4 pinctrl: tegra: reduce size of data table fields
In reply to Stephen WarrenQuoted 6 lines · Stephen Warren
From: Stephen Warren <[email protected]>
The range of npins and function ID values is small enough to fit into a u8. Use this type rather than unsigned to shrink the pinmux data tables.
Signed-off-by Stephen Warren <[email protected]>
Applied with Laxman's ACK.
Yours,
Linus Walleij
PATCH 4/4 pinctrl: tegra: print better error messages
In reply to Stephen WarrenFrom: Stephen Warren <[email protected]>
When an attempt is made to configure an unsupported option on a pin, print the DT property name of that option, so it's easier to debug what the problem is.
--- drivers/pinctrl/pinctrl-tegra.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
drivers/pinctrl/pinctrl-tegra.c +14 -3
@@ -413,10 +413,21 @@ static int tegra_pinconf_reg(struct tegra_pmx *pmx, }if (*reg <0 || *bit >31 ) { -if (report_err) +if (report_err) { +const char *prop ="unknown" ; +int i; + +for (i =0 ; i <ARRAY_SIZE (cfg_params); i++) { +if (cfg_params[i].param == param) { + prop = cfg_params[i].property; +break ; + } + } +dev_err (pmx->dev, -"Config param %04x not supported on group %s\n" , - param, g->name); +"Config param %04x (%s) not supported on group %s\n" , + param, prop, g->name); + }return -ENOTSUPP ; }
Signature · 1 line
1.8.1.5
PATCH 4/4 pinctrl: tegra: print better error messages
Quoted 7 lines · Stephen Warren
From: Stephen Warren <[email protected]>
When an attempt is made to configure an unsupported option on a pin, print the DT property name of that option, so it's easier to debug what the problem is.
Signed-off-by Stephen Warren <[email protected]>
Applied with Laxman's ACK.
Yours,
Linus Walleij
PATCH 1/4 pinctrl: tegra: remove redundant data table fields
In reply to Stephen WarrenQuoted 3 lines · Stephen Warren
From: Stephen Warren <[email protected]>
u32 drvtype_bit:5;
Seems this is also need for 6.
PATCH 1/4 pinctrl: tegra: remove redundant data table fields
Quoted 6 lines · Laxman Dewangan
Quoted 3 lines · Stephen Warren
From: Stephen Warren <[email protected]>
u32 drvtype_bit:5;Seems this is also need for 6.
Good catch, thanks.
I'll posted a V2 for this patch, although I won't repost patches 2..4 to avoid re-spamming the list with the very large patch 2. Linus, I hope that's OK.