All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] pinctrl: ingenic: Fix regmap on X series SoCs
@ 2022-03-17  0:07 Aidan MacDonald
  2022-03-17  0:25 ` Paul Cercueil
  2022-03-24 19:07 ` Linus Walleij
  0 siblings, 2 replies; 3+ messages in thread
From: Aidan MacDonald @ 2022-03-17  0:07 UTC (permalink / raw)
  To: paul, linus.walleij; +Cc: linux-mips, linux-gpio, linux-kernel

The X series Ingenic SoCs have a shadow GPIO group which is at a higher
offset than the other groups, and is used for all GPIO configuration.
The regmap did not take this offset into account and set max_register
too low, so the regmap API blocked writes to the shadow group, which
made the pinctrl driver unable to configure any pins.

Fix this by adding regmap access tables to the chip info. The way that
max_register was computed was also off by one, since max_register is an
inclusive bound, not an exclusive bound; this has been fixed.

Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
---
v1 -> v2: use regmap_access_table
v2 -> v3: compute max_register instead of putting it in chip_info
v3 -> v4: explain the fix to the max_register calculation

 drivers/pinctrl/pinctrl-ingenic.c | 46 ++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-ingenic.c b/drivers/pinctrl/pinctrl-ingenic.c
index 2712f51eb238..fa6becca1788 100644
--- a/drivers/pinctrl/pinctrl-ingenic.c
+++ b/drivers/pinctrl/pinctrl-ingenic.c
@@ -119,6 +119,8 @@ struct ingenic_chip_info {
 	unsigned int num_functions;
 
 	const u32 *pull_ups, *pull_downs;
+
+	const struct regmap_access_table *access_table;
 };
 
 struct ingenic_pinctrl {
@@ -2179,6 +2181,17 @@ static const struct function_desc x1000_functions[] = {
 	{ "mac", x1000_mac_groups, ARRAY_SIZE(x1000_mac_groups), },
 };
 
+static const struct regmap_range x1000_access_ranges[] = {
+	regmap_reg_range(0x000, 0x400 - 4),
+	regmap_reg_range(0x700, 0x800 - 4),
+};
+
+/* shared with X1500 */
+static const struct regmap_access_table x1000_access_table = {
+	.yes_ranges = x1000_access_ranges,
+	.n_yes_ranges = ARRAY_SIZE(x1000_access_ranges),
+};
+
 static const struct ingenic_chip_info x1000_chip_info = {
 	.num_chips = 4,
 	.reg_offset = 0x100,
@@ -2189,6 +2202,7 @@ static const struct ingenic_chip_info x1000_chip_info = {
 	.num_functions = ARRAY_SIZE(x1000_functions),
 	.pull_ups = x1000_pull_ups,
 	.pull_downs = x1000_pull_downs,
+	.access_table = &x1000_access_table,
 };
 
 static int x1500_uart0_data_pins[] = { 0x4a, 0x4b, };
@@ -2300,6 +2314,7 @@ static const struct ingenic_chip_info x1500_chip_info = {
 	.num_functions = ARRAY_SIZE(x1500_functions),
 	.pull_ups = x1000_pull_ups,
 	.pull_downs = x1000_pull_downs,
+	.access_table = &x1000_access_table,
 };
 
 static const u32 x1830_pull_ups[4] = {
@@ -2506,6 +2521,16 @@ static const struct function_desc x1830_functions[] = {
 	{ "mac", x1830_mac_groups, ARRAY_SIZE(x1830_mac_groups), },
 };
 
+static const struct regmap_range x1830_access_ranges[] = {
+	regmap_reg_range(0x0000, 0x4000 - 4),
+	regmap_reg_range(0x7000, 0x8000 - 4),
+};
+
+static const struct regmap_access_table x1830_access_table = {
+	.yes_ranges = x1830_access_ranges,
+	.n_yes_ranges = ARRAY_SIZE(x1830_access_ranges),
+};
+
 static const struct ingenic_chip_info x1830_chip_info = {
 	.num_chips = 4,
 	.reg_offset = 0x1000,
@@ -2516,6 +2541,7 @@ static const struct ingenic_chip_info x1830_chip_info = {
 	.num_functions = ARRAY_SIZE(x1830_functions),
 	.pull_ups = x1830_pull_ups,
 	.pull_downs = x1830_pull_downs,
+	.access_table = &x1830_access_table,
 };
 
 static const u32 x2000_pull_ups[5] = {
@@ -2969,6 +2995,17 @@ static const struct function_desc x2000_functions[] = {
 	{ "otg", x2000_otg_groups, ARRAY_SIZE(x2000_otg_groups), },
 };
 
+static const struct regmap_range x2000_access_ranges[] = {
+	regmap_reg_range(0x000, 0x500 - 4),
+	regmap_reg_range(0x700, 0x800 - 4),
+};
+
+/* shared with X2100 */
+static const struct regmap_access_table x2000_access_table = {
+	.yes_ranges = x2000_access_ranges,
+	.n_yes_ranges = ARRAY_SIZE(x2000_access_ranges),
+};
+
 static const struct ingenic_chip_info x2000_chip_info = {
 	.num_chips = 5,
 	.reg_offset = 0x100,
@@ -2979,6 +3016,7 @@ static const struct ingenic_chip_info x2000_chip_info = {
 	.num_functions = ARRAY_SIZE(x2000_functions),
 	.pull_ups = x2000_pull_ups,
 	.pull_downs = x2000_pull_downs,
+	.access_table = &x2000_access_table,
 };
 
 static const u32 x2100_pull_ups[5] = {
@@ -3189,6 +3227,7 @@ static const struct ingenic_chip_info x2100_chip_info = {
 	.num_functions = ARRAY_SIZE(x2100_functions),
 	.pull_ups = x2100_pull_ups,
 	.pull_downs = x2100_pull_downs,
+	.access_table = &x2000_access_table,
 };
 
 static u32 ingenic_gpio_read_reg(struct ingenic_gpio_chip *jzgc, u8 reg)
@@ -4168,7 +4207,12 @@ static int __init ingenic_pinctrl_probe(struct platform_device *pdev)
 		return PTR_ERR(base);
 
 	regmap_config = ingenic_pinctrl_regmap_config;
-	regmap_config.max_register = chip_info->num_chips * chip_info->reg_offset;
+	if (chip_info->access_table) {
+		regmap_config.rd_table = chip_info->access_table;
+		regmap_config.wr_table = chip_info->access_table;
+	} else {
+		regmap_config.max_register = chip_info->num_chips * chip_info->reg_offset - 4;
+	}
 
 	jzpc->map = devm_regmap_init_mmio(dev, base, &regmap_config);
 	if (IS_ERR(jzpc->map)) {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] pinctrl: ingenic: Fix regmap on X series SoCs
  2022-03-17  0:07 [PATCH v4] pinctrl: ingenic: Fix regmap on X series SoCs Aidan MacDonald
@ 2022-03-17  0:25 ` Paul Cercueil
  2022-03-24 19:07 ` Linus Walleij
  1 sibling, 0 replies; 3+ messages in thread
From: Paul Cercueil @ 2022-03-17  0:25 UTC (permalink / raw)
  To: Aidan MacDonald; +Cc: linus.walleij, linux-mips, linux-gpio, linux-kernel

Hi,

Le jeu., mars 17 2022 at 00:07:40 +0000, Aidan MacDonald 
<aidanmacdonald.0x0@gmail.com> a écrit :
> The X series Ingenic SoCs have a shadow GPIO group which is at a 
> higher
> offset than the other groups, and is used for all GPIO configuration.
> The regmap did not take this offset into account and set max_register
> too low, so the regmap API blocked writes to the shadow group, which
> made the pinctrl driver unable to configure any pins.
> 
> Fix this by adding regmap access tables to the chip info. The way that
> max_register was computed was also off by one, since max_register is 
> an
> inclusive bound, not an exclusive bound; this has been fixed.
> 
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>

Fixes: 6626a76ef857 ("pinctrl: ingenic: Add .max_register in 
regmap_config")
Cc: stable@vger.kernel.org
Reviewed-by: Paul Cercueil <paul@crapouillou.net>

Cheers,
-Paul

> ---
> v1 -> v2: use regmap_access_table
> v2 -> v3: compute max_register instead of putting it in chip_info
> v3 -> v4: explain the fix to the max_register calculation
> 
>  drivers/pinctrl/pinctrl-ingenic.c | 46 
> ++++++++++++++++++++++++++++++-
>  1 file changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-ingenic.c 
> b/drivers/pinctrl/pinctrl-ingenic.c
> index 2712f51eb238..fa6becca1788 100644
> --- a/drivers/pinctrl/pinctrl-ingenic.c
> +++ b/drivers/pinctrl/pinctrl-ingenic.c
> @@ -119,6 +119,8 @@ struct ingenic_chip_info {
>  	unsigned int num_functions;
> 
>  	const u32 *pull_ups, *pull_downs;
> +
> +	const struct regmap_access_table *access_table;
>  };
> 
>  struct ingenic_pinctrl {
> @@ -2179,6 +2181,17 @@ static const struct function_desc 
> x1000_functions[] = {
>  	{ "mac", x1000_mac_groups, ARRAY_SIZE(x1000_mac_groups), },
>  };
> 
> +static const struct regmap_range x1000_access_ranges[] = {
> +	regmap_reg_range(0x000, 0x400 - 4),
> +	regmap_reg_range(0x700, 0x800 - 4),
> +};
> +
> +/* shared with X1500 */
> +static const struct regmap_access_table x1000_access_table = {
> +	.yes_ranges = x1000_access_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(x1000_access_ranges),
> +};
> +
>  static const struct ingenic_chip_info x1000_chip_info = {
>  	.num_chips = 4,
>  	.reg_offset = 0x100,
> @@ -2189,6 +2202,7 @@ static const struct ingenic_chip_info 
> x1000_chip_info = {
>  	.num_functions = ARRAY_SIZE(x1000_functions),
>  	.pull_ups = x1000_pull_ups,
>  	.pull_downs = x1000_pull_downs,
> +	.access_table = &x1000_access_table,
>  };
> 
>  static int x1500_uart0_data_pins[] = { 0x4a, 0x4b, };
> @@ -2300,6 +2314,7 @@ static const struct ingenic_chip_info 
> x1500_chip_info = {
>  	.num_functions = ARRAY_SIZE(x1500_functions),
>  	.pull_ups = x1000_pull_ups,
>  	.pull_downs = x1000_pull_downs,
> +	.access_table = &x1000_access_table,
>  };
> 
>  static const u32 x1830_pull_ups[4] = {
> @@ -2506,6 +2521,16 @@ static const struct function_desc 
> x1830_functions[] = {
>  	{ "mac", x1830_mac_groups, ARRAY_SIZE(x1830_mac_groups), },
>  };
> 
> +static const struct regmap_range x1830_access_ranges[] = {
> +	regmap_reg_range(0x0000, 0x4000 - 4),
> +	regmap_reg_range(0x7000, 0x8000 - 4),
> +};
> +
> +static const struct regmap_access_table x1830_access_table = {
> +	.yes_ranges = x1830_access_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(x1830_access_ranges),
> +};
> +
>  static const struct ingenic_chip_info x1830_chip_info = {
>  	.num_chips = 4,
>  	.reg_offset = 0x1000,
> @@ -2516,6 +2541,7 @@ static const struct ingenic_chip_info 
> x1830_chip_info = {
>  	.num_functions = ARRAY_SIZE(x1830_functions),
>  	.pull_ups = x1830_pull_ups,
>  	.pull_downs = x1830_pull_downs,
> +	.access_table = &x1830_access_table,
>  };
> 
>  static const u32 x2000_pull_ups[5] = {
> @@ -2969,6 +2995,17 @@ static const struct function_desc 
> x2000_functions[] = {
>  	{ "otg", x2000_otg_groups, ARRAY_SIZE(x2000_otg_groups), },
>  };
> 
> +static const struct regmap_range x2000_access_ranges[] = {
> +	regmap_reg_range(0x000, 0x500 - 4),
> +	regmap_reg_range(0x700, 0x800 - 4),
> +};
> +
> +/* shared with X2100 */
> +static const struct regmap_access_table x2000_access_table = {
> +	.yes_ranges = x2000_access_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(x2000_access_ranges),
> +};
> +
>  static const struct ingenic_chip_info x2000_chip_info = {
>  	.num_chips = 5,
>  	.reg_offset = 0x100,
> @@ -2979,6 +3016,7 @@ static const struct ingenic_chip_info 
> x2000_chip_info = {
>  	.num_functions = ARRAY_SIZE(x2000_functions),
>  	.pull_ups = x2000_pull_ups,
>  	.pull_downs = x2000_pull_downs,
> +	.access_table = &x2000_access_table,
>  };
> 
>  static const u32 x2100_pull_ups[5] = {
> @@ -3189,6 +3227,7 @@ static const struct ingenic_chip_info 
> x2100_chip_info = {
>  	.num_functions = ARRAY_SIZE(x2100_functions),
>  	.pull_ups = x2100_pull_ups,
>  	.pull_downs = x2100_pull_downs,
> +	.access_table = &x2000_access_table,
>  };
> 
>  static u32 ingenic_gpio_read_reg(struct ingenic_gpio_chip *jzgc, u8 
> reg)
> @@ -4168,7 +4207,12 @@ static int __init ingenic_pinctrl_probe(struct 
> platform_device *pdev)
>  		return PTR_ERR(base);
> 
>  	regmap_config = ingenic_pinctrl_regmap_config;
> -	regmap_config.max_register = chip_info->num_chips * 
> chip_info->reg_offset;
> +	if (chip_info->access_table) {
> +		regmap_config.rd_table = chip_info->access_table;
> +		regmap_config.wr_table = chip_info->access_table;
> +	} else {
> +		regmap_config.max_register = chip_info->num_chips * 
> chip_info->reg_offset - 4;
> +	}
> 
>  	jzpc->map = devm_regmap_init_mmio(dev, base, &regmap_config);
>  	if (IS_ERR(jzpc->map)) {
> --
> 2.34.1
> 



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] pinctrl: ingenic: Fix regmap on X series SoCs
  2022-03-17  0:07 [PATCH v4] pinctrl: ingenic: Fix regmap on X series SoCs Aidan MacDonald
  2022-03-17  0:25 ` Paul Cercueil
@ 2022-03-24 19:07 ` Linus Walleij
  1 sibling, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2022-03-24 19:07 UTC (permalink / raw)
  To: Aidan MacDonald; +Cc: paul, linux-mips, linux-gpio, linux-kernel

On Thu, Mar 17, 2022 at 1:07 AM Aidan MacDonald
<aidanmacdonald.0x0@gmail.com> wrote:

> The X series Ingenic SoCs have a shadow GPIO group which is at a higher
> offset than the other groups, and is used for all GPIO configuration.
> The regmap did not take this offset into account and set max_register
> too low, so the regmap API blocked writes to the shadow group, which
> made the pinctrl driver unable to configure any pins.
>
> Fix this by adding regmap access tables to the chip info. The way that
> max_register was computed was also off by one, since max_register is an
> inclusive bound, not an exclusive bound; this has been fixed.
>
> Signed-off-by: Aidan MacDonald <aidanmacdonald.0x0@gmail.com>
> ---
> v1 -> v2: use regmap_access_table
> v2 -> v3: compute max_register instead of putting it in chip_info
> v3 -> v4: explain the fix to the max_register calculation

Patch applied!

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-03-24 19:07 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-17  0:07 [PATCH v4] pinctrl: ingenic: Fix regmap on X series SoCs Aidan MacDonald
2022-03-17  0:25 ` Paul Cercueil
2022-03-24 19:07 ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.