* [PATCH v4 0/2] pinctrl: ocelot: Add fixes for ocelot driver
@ 2022-07-12 19:50 Horatiu Vultur
2022-07-12 19:50 ` [PATCH v4 1/2] pinctrl: ocelot: Fix pincfg for lan966x Horatiu Vultur
2022-07-12 19:50 ` [PATCH v4 2/2] pinctrl: ocelot: Fix pincfg Horatiu Vultur
0 siblings, 2 replies; 6+ messages in thread
From: Horatiu Vultur @ 2022-07-12 19:50 UTC (permalink / raw)
To: linux-gpio, linux-kernel
Cc: linus.walleij, alexandre.belloni, kavyasree.kotagiri,
colin.foster, UNGLinuxDriver, maxime.chevallier, michael,
andy.shevchenko, Horatiu Vultur
The patch series fixes 2 issues with pincfg.
- first issue is that on lan966x uses different offsets than sparx5
so it can't use directly the ocelot_confops
- second issue is pincfg stop working when regmap support was added.
v3->v4:
- add missing check for NULL pointer when doing devm_kmemdup
- remove devm_kmemdup for pincfg_data as it can be const
- remove has_schmitt field, because we can use schmitt_bit for checking
if it has support for PIN_CONFIG_INPUT_SCHMITT_ENABLE
- change the declaration of ocelot_pinctrl_create_pincfg
v2->v3:
- reorder ocelot_pincfg_data fields, mandatory fields go first
- move the field pincfg_data inside ocelot_pinctrl
- add back max_register for regmap_config for pincfg.
- make struct ocelot_match_data const and drop the cast.
v1->v2:
- use regmap_get_reg_stride instead of making regmap_config global
- update how ocelot_match_data structs are initialized
Horatiu Vultur (2):
pinctrl: ocelot: Fix pincfg for lan966x
pinctrl: ocelot: Fix pincfg
drivers/pinctrl/pinctrl-ocelot.c | 213 ++++++++++++++++++++-----------
1 file changed, 136 insertions(+), 77 deletions(-)
--
2.33.0
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 1/2] pinctrl: ocelot: Fix pincfg for lan966x
2022-07-12 19:50 [PATCH v4 0/2] pinctrl: ocelot: Add fixes for ocelot driver Horatiu Vultur
@ 2022-07-12 19:50 ` Horatiu Vultur
2022-07-13 9:53 ` Andy Shevchenko
2022-07-12 19:50 ` [PATCH v4 2/2] pinctrl: ocelot: Fix pincfg Horatiu Vultur
1 sibling, 1 reply; 6+ messages in thread
From: Horatiu Vultur @ 2022-07-12 19:50 UTC (permalink / raw)
To: linux-gpio, linux-kernel
Cc: linus.walleij, alexandre.belloni, kavyasree.kotagiri,
colin.foster, UNGLinuxDriver, maxime.chevallier, michael,
andy.shevchenko, Horatiu Vultur
The blamed commit introduce support for lan966x which use the same
pinconf_ops as sparx5. The problem is that pinconf_ops is specific to
sparx5. More precisely the offset of the bits in the pincfg register are
different and also lan966x doesn't have support for
PIN_CONFIG_INPUT_SCHMITT_ENABLE.
Fix this by making pinconf_ops more generic such that it can be also
used by lan966x. This is done by introducing 'ocelot_pincfg_data' which
contains the offset and what is supported for each SOC.
Fixes: 531d6ab36571 ("pinctrl: ocelot: Extend support for lan966x")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
drivers/pinctrl/pinctrl-ocelot.c | 194 ++++++++++++++++++++-----------
1 file changed, 123 insertions(+), 71 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c
index 5f4a8c5c6650..873bba245522 100644
--- a/drivers/pinctrl/pinctrl-ocelot.c
+++ b/drivers/pinctrl/pinctrl-ocelot.c
@@ -29,19 +29,12 @@
#define ocelot_clrsetbits(addr, clear, set) \
writel((readl(addr) & ~(clear)) | (set), (addr))
-/* PINCONFIG bits (sparx5 only) */
enum {
PINCONF_BIAS,
PINCONF_SCHMITT,
PINCONF_DRIVE_STRENGTH,
};
-#define BIAS_PD_BIT BIT(4)
-#define BIAS_PU_BIT BIT(3)
-#define BIAS_BITS (BIAS_PD_BIT|BIAS_PU_BIT)
-#define SCHMITT_BIT BIT(2)
-#define DRIVE_BITS GENMASK(1, 0)
-
/* GPIO standard registers */
#define OCELOT_GPIO_OUT_SET 0x0
#define OCELOT_GPIO_OUT_CLR 0x4
@@ -321,6 +314,13 @@ struct ocelot_pin_caps {
unsigned char a_functions[OCELOT_FUNC_PER_PIN]; /* Additional functions */
};
+struct ocelot_pincfg_data {
+ u8 pd_bit;
+ u8 pu_bit;
+ u8 drive_bits;
+ u8 schmitt_bit;
+};
+
struct ocelot_pinctrl {
struct device *dev;
struct pinctrl_dev *pctl;
@@ -328,10 +328,16 @@ struct ocelot_pinctrl {
struct regmap *map;
struct regmap *pincfg;
struct pinctrl_desc *desc;
+ const struct ocelot_pincfg_data *pincfg_data;
struct ocelot_pmx_func func[FUNC_MAX];
u8 stride;
};
+struct ocelot_match_data {
+ struct pinctrl_desc desc;
+ struct ocelot_pincfg_data pincfg_data;
+};
+
#define LUTON_P(p, f0, f1) \
static struct ocelot_pin_caps luton_pin_##p = { \
.pin = p, \
@@ -1325,6 +1331,7 @@ static int ocelot_hw_get_value(struct ocelot_pinctrl *info,
int ret = -EOPNOTSUPP;
if (info->pincfg) {
+ const struct ocelot_pincfg_data *opd = info->pincfg_data;
u32 regcfg;
ret = regmap_read(info->pincfg, pin, ®cfg);
@@ -1334,15 +1341,15 @@ static int ocelot_hw_get_value(struct ocelot_pinctrl *info,
ret = 0;
switch (reg) {
case PINCONF_BIAS:
- *val = regcfg & BIAS_BITS;
+ *val = regcfg & (opd->pd_bit | opd->pu_bit);
break;
case PINCONF_SCHMITT:
- *val = regcfg & SCHMITT_BIT;
+ *val = regcfg & opd->schmitt_bit;
break;
case PINCONF_DRIVE_STRENGTH:
- *val = regcfg & DRIVE_BITS;
+ *val = regcfg & opd->drive_bits;
break;
default:
@@ -1379,23 +1386,27 @@ static int ocelot_hw_set_value(struct ocelot_pinctrl *info,
int ret = -EOPNOTSUPP;
if (info->pincfg) {
+ const struct ocelot_pincfg_data *opd = info->pincfg_data;
ret = 0;
switch (reg) {
case PINCONF_BIAS:
- ret = ocelot_pincfg_clrsetbits(info, pin, BIAS_BITS,
+ ret = ocelot_pincfg_clrsetbits(info, pin,
+ opd->pd_bit | opd->pu_bit,
val);
break;
case PINCONF_SCHMITT:
- ret = ocelot_pincfg_clrsetbits(info, pin, SCHMITT_BIT,
+ ret = ocelot_pincfg_clrsetbits(info, pin,
+ opd->schmitt_bit,
val);
break;
case PINCONF_DRIVE_STRENGTH:
if (val <= 3)
ret = ocelot_pincfg_clrsetbits(info, pin,
- DRIVE_BITS, val);
+ opd->drive_bits,
+ val);
else
ret = -EINVAL;
break;
@@ -1425,17 +1436,20 @@ static int ocelot_pinconf_get(struct pinctrl_dev *pctldev,
if (param == PIN_CONFIG_BIAS_DISABLE)
val = (val == 0);
else if (param == PIN_CONFIG_BIAS_PULL_DOWN)
- val = (val & BIAS_PD_BIT ? true : false);
+ val = !!(val & info->pincfg_data->pd_bit);
else /* PIN_CONFIG_BIAS_PULL_UP */
- val = (val & BIAS_PU_BIT ? true : false);
+ val = !!(val & info->pincfg_data->pu_bit);
break;
case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
+ if (!info->pincfg_data->schmitt_bit)
+ return -EOPNOTSUPP;
+
err = ocelot_hw_get_value(info, pin, PINCONF_SCHMITT, &val);
if (err)
return err;
- val = (val & SCHMITT_BIT ? true : false);
+ val = !!(val & info->pincfg_data->schmitt_bit);
break;
case PIN_CONFIG_DRIVE_STRENGTH:
@@ -1491,8 +1505,8 @@ static int ocelot_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
case PIN_CONFIG_BIAS_PULL_UP:
case PIN_CONFIG_BIAS_PULL_DOWN:
arg = (param == PIN_CONFIG_BIAS_DISABLE) ? 0 :
- (param == PIN_CONFIG_BIAS_PULL_UP) ? BIAS_PU_BIT :
- BIAS_PD_BIT;
+ (param == PIN_CONFIG_BIAS_PULL_UP) ? info->pincfg_data->pu_bit :
+ info->pincfg_data->pd_bit;
err = ocelot_hw_set_value(info, pin, PINCONF_BIAS, arg);
if (err)
@@ -1501,7 +1515,10 @@ static int ocelot_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin,
break;
case PIN_CONFIG_INPUT_SCHMITT_ENABLE:
- arg = arg ? SCHMITT_BIT : 0;
+ if (!info->pincfg_data->schmitt_bit)
+ return -EOPNOTSUPP;
+
+ arg = arg ? info->pincfg_data->schmitt_bit : 0;
err = ocelot_hw_set_value(info, pin, PINCONF_SCHMITT,
arg);
if (err)
@@ -1562,69 +1579,94 @@ static const struct pinctrl_ops ocelot_pctl_ops = {
.dt_free_map = pinconf_generic_dt_free_map,
};
-static struct pinctrl_desc luton_desc = {
- .name = "luton-pinctrl",
- .pins = luton_pins,
- .npins = ARRAY_SIZE(luton_pins),
- .pctlops = &ocelot_pctl_ops,
- .pmxops = &ocelot_pmx_ops,
- .owner = THIS_MODULE,
+static struct ocelot_match_data luton_desc = {
+ .desc = {
+ .name = "luton-pinctrl",
+ .pins = luton_pins,
+ .npins = ARRAY_SIZE(luton_pins),
+ .pctlops = &ocelot_pctl_ops,
+ .pmxops = &ocelot_pmx_ops,
+ .owner = THIS_MODULE,
+ },
};
-static struct pinctrl_desc serval_desc = {
- .name = "serval-pinctrl",
- .pins = serval_pins,
- .npins = ARRAY_SIZE(serval_pins),
- .pctlops = &ocelot_pctl_ops,
- .pmxops = &ocelot_pmx_ops,
- .owner = THIS_MODULE,
+static struct ocelot_match_data serval_desc = {
+ .desc = {
+ .name = "serval-pinctrl",
+ .pins = serval_pins,
+ .npins = ARRAY_SIZE(serval_pins),
+ .pctlops = &ocelot_pctl_ops,
+ .pmxops = &ocelot_pmx_ops,
+ .owner = THIS_MODULE,
+ },
};
-static struct pinctrl_desc ocelot_desc = {
- .name = "ocelot-pinctrl",
- .pins = ocelot_pins,
- .npins = ARRAY_SIZE(ocelot_pins),
- .pctlops = &ocelot_pctl_ops,
- .pmxops = &ocelot_pmx_ops,
- .owner = THIS_MODULE,
+static struct ocelot_match_data ocelot_desc = {
+ .desc = {
+ .name = "ocelot-pinctrl",
+ .pins = ocelot_pins,
+ .npins = ARRAY_SIZE(ocelot_pins),
+ .pctlops = &ocelot_pctl_ops,
+ .pmxops = &ocelot_pmx_ops,
+ .owner = THIS_MODULE,
+ },
};
-static struct pinctrl_desc jaguar2_desc = {
- .name = "jaguar2-pinctrl",
- .pins = jaguar2_pins,
- .npins = ARRAY_SIZE(jaguar2_pins),
- .pctlops = &ocelot_pctl_ops,
- .pmxops = &ocelot_pmx_ops,
- .owner = THIS_MODULE,
+static struct ocelot_match_data jaguar2_desc = {
+ .desc = {
+ .name = "jaguar2-pinctrl",
+ .pins = jaguar2_pins,
+ .npins = ARRAY_SIZE(jaguar2_pins),
+ .pctlops = &ocelot_pctl_ops,
+ .pmxops = &ocelot_pmx_ops,
+ .owner = THIS_MODULE,
+ },
};
-static struct pinctrl_desc servalt_desc = {
- .name = "servalt-pinctrl",
- .pins = servalt_pins,
- .npins = ARRAY_SIZE(servalt_pins),
- .pctlops = &ocelot_pctl_ops,
- .pmxops = &ocelot_pmx_ops,
- .owner = THIS_MODULE,
+static struct ocelot_match_data servalt_desc = {
+ .desc = {
+ .name = "servalt-pinctrl",
+ .pins = servalt_pins,
+ .npins = ARRAY_SIZE(servalt_pins),
+ .pctlops = &ocelot_pctl_ops,
+ .pmxops = &ocelot_pmx_ops,
+ .owner = THIS_MODULE,
+ },
};
-static struct pinctrl_desc sparx5_desc = {
- .name = "sparx5-pinctrl",
- .pins = sparx5_pins,
- .npins = ARRAY_SIZE(sparx5_pins),
- .pctlops = &ocelot_pctl_ops,
- .pmxops = &ocelot_pmx_ops,
- .confops = &ocelot_confops,
- .owner = THIS_MODULE,
+static struct ocelot_match_data sparx5_desc = {
+ .desc = {
+ .name = "sparx5-pinctrl",
+ .pins = sparx5_pins,
+ .npins = ARRAY_SIZE(sparx5_pins),
+ .pctlops = &ocelot_pctl_ops,
+ .pmxops = &ocelot_pmx_ops,
+ .confops = &ocelot_confops,
+ .owner = THIS_MODULE,
+ },
+ .pincfg_data = {
+ .pd_bit = BIT(4),
+ .pu_bit = BIT(3),
+ .drive_bits = GENMASK(1, 0),
+ .schmitt_bit = BIT(2),
+ },
};
-static struct pinctrl_desc lan966x_desc = {
- .name = "lan966x-pinctrl",
- .pins = lan966x_pins,
- .npins = ARRAY_SIZE(lan966x_pins),
- .pctlops = &ocelot_pctl_ops,
- .pmxops = &lan966x_pmx_ops,
- .confops = &ocelot_confops,
- .owner = THIS_MODULE,
+static struct ocelot_match_data lan966x_desc = {
+ .desc = {
+ .name = "lan966x-pinctrl",
+ .pins = lan966x_pins,
+ .npins = ARRAY_SIZE(lan966x_pins),
+ .pctlops = &ocelot_pctl_ops,
+ .pmxops = &lan966x_pmx_ops,
+ .confops = &ocelot_confops,
+ .owner = THIS_MODULE,
+ },
+ .pincfg_data = {
+ .pd_bit = BIT(3),
+ .pu_bit = BIT(2),
+ .drive_bits = GENMASK(1, 0),
+ },
};
static int ocelot_create_group_func_map(struct device *dev,
@@ -1913,6 +1955,7 @@ static struct regmap *ocelot_pinctrl_create_pincfg(struct platform_device *pdev)
static int ocelot_pinctrl_probe(struct platform_device *pdev)
{
+ const struct ocelot_match_data *data;
struct device *dev = &pdev->dev;
struct ocelot_pinctrl *info;
struct reset_control *reset;
@@ -1929,7 +1972,16 @@ static int ocelot_pinctrl_probe(struct platform_device *pdev)
if (!info)
return -ENOMEM;
- info->desc = (struct pinctrl_desc *)device_get_match_data(dev);
+ data = device_get_match_data(dev);
+ if (!data)
+ return -EINVAL;
+
+ info->desc = devm_kmemdup(dev, &data->desc, sizeof(*info->desc),
+ GFP_KERNEL);
+ if (!info->desc)
+ return -ENOMEM;
+
+ info->pincfg_data = &data->pincfg_data;
reset = devm_reset_control_get_optional_shared(dev, "switch");
if (IS_ERR(reset))
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 2/2] pinctrl: ocelot: Fix pincfg
2022-07-12 19:50 [PATCH v4 0/2] pinctrl: ocelot: Add fixes for ocelot driver Horatiu Vultur
2022-07-12 19:50 ` [PATCH v4 1/2] pinctrl: ocelot: Fix pincfg for lan966x Horatiu Vultur
@ 2022-07-12 19:50 ` Horatiu Vultur
2022-07-13 8:35 ` Colin Foster
2022-07-13 9:54 ` Andy Shevchenko
1 sibling, 2 replies; 6+ messages in thread
From: Horatiu Vultur @ 2022-07-12 19:50 UTC (permalink / raw)
To: linux-gpio, linux-kernel
Cc: linus.walleij, alexandre.belloni, kavyasree.kotagiri,
colin.foster, UNGLinuxDriver, maxime.chevallier, michael,
andy.shevchenko, Horatiu Vultur
The blamed commit changed to use regmaps instead of __iomem. But it
didn't update the register offsets to be at word offset, so it uses byte
offset.
Another issue with the same commit is that it has a limit of 32 registers
which is incorrect. The sparx5 has 64 while lan966x has 77.
Fixes: 076d9e71bcf8 ("pinctrl: ocelot: convert pinctrl to regmap")
Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
drivers/pinctrl/pinctrl-ocelot.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c
index 873bba245522..c5a9f87f0c49 100644
--- a/drivers/pinctrl/pinctrl-ocelot.c
+++ b/drivers/pinctrl/pinctrl-ocelot.c
@@ -1334,7 +1334,9 @@ static int ocelot_hw_get_value(struct ocelot_pinctrl *info,
const struct ocelot_pincfg_data *opd = info->pincfg_data;
u32 regcfg;
- ret = regmap_read(info->pincfg, pin, ®cfg);
+ ret = regmap_read(info->pincfg,
+ pin * regmap_get_reg_stride(info->pincfg),
+ ®cfg);
if (ret)
return ret;
@@ -1366,14 +1368,18 @@ static int ocelot_pincfg_clrsetbits(struct ocelot_pinctrl *info, u32 regaddr,
u32 val;
int ret;
- ret = regmap_read(info->pincfg, regaddr, &val);
+ ret = regmap_read(info->pincfg,
+ regaddr * regmap_get_reg_stride(info->pincfg),
+ &val);
if (ret)
return ret;
val &= ~clrbits;
val |= setbits;
- ret = regmap_write(info->pincfg, regaddr, val);
+ ret = regmap_write(info->pincfg,
+ regaddr * regmap_get_reg_stride(info->pincfg),
+ val);
return ret;
}
@@ -1932,7 +1938,8 @@ static const struct of_device_id ocelot_pinctrl_of_match[] = {
{},
};
-static struct regmap *ocelot_pinctrl_create_pincfg(struct platform_device *pdev)
+static struct regmap *ocelot_pinctrl_create_pincfg(struct platform_device *pdev,
+ const struct ocelot_pinctrl *info)
{
void __iomem *base;
@@ -1940,7 +1947,7 @@ static struct regmap *ocelot_pinctrl_create_pincfg(struct platform_device *pdev)
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
- .max_register = 32,
+ .max_register = info->desc->npins * 4,
.name = "pincfg",
};
@@ -2008,7 +2015,7 @@ static int ocelot_pinctrl_probe(struct platform_device *pdev)
/* Pinconf registers */
if (info->desc->confops) {
- pincfg = ocelot_pinctrl_create_pincfg(pdev);
+ pincfg = ocelot_pinctrl_create_pincfg(pdev, info);
if (IS_ERR(pincfg))
dev_dbg(dev, "Failed to create pincfg regmap\n");
else
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/2] pinctrl: ocelot: Fix pincfg
2022-07-12 19:50 ` [PATCH v4 2/2] pinctrl: ocelot: Fix pincfg Horatiu Vultur
@ 2022-07-13 8:35 ` Colin Foster
2022-07-13 9:54 ` Andy Shevchenko
1 sibling, 0 replies; 6+ messages in thread
From: Colin Foster @ 2022-07-13 8:35 UTC (permalink / raw)
To: Horatiu Vultur
Cc: linux-gpio, linux-kernel, linus.walleij, alexandre.belloni,
kavyasree.kotagiri, UNGLinuxDriver, maxime.chevallier, michael,
andy.shevchenko
On Tue, Jul 12, 2022 at 09:50:43PM +0200, Horatiu Vultur wrote:
> The blamed commit changed to use regmaps instead of __iomem. But it
> didn't update the register offsets to be at word offset, so it uses byte
> offset.
> Another issue with the same commit is that it has a limit of 32 registers
> which is incorrect. The sparx5 has 64 while lan966x has 77.
>
> Fixes: 076d9e71bcf8 ("pinctrl: ocelot: convert pinctrl to regmap")
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
Acked-by: Colin Foster <colin.foster@in-advantage.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/2] pinctrl: ocelot: Fix pincfg for lan966x
2022-07-12 19:50 ` [PATCH v4 1/2] pinctrl: ocelot: Fix pincfg for lan966x Horatiu Vultur
@ 2022-07-13 9:53 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2022-07-13 9:53 UTC (permalink / raw)
To: Horatiu Vultur
Cc: open list:GPIO SUBSYSTEM, Linux Kernel Mailing List,
Linus Walleij, Alexandre Belloni, kavyasree.kotagiri,
Colin Foster, Microchip Linux Driver Support, Maxime Chevallier,
Michael Walle
On Tue, Jul 12, 2022 at 9:46 PM Horatiu Vultur
<horatiu.vultur@microchip.com> wrote:
>
> The blamed commit introduce support for lan966x which use the same
> pinconf_ops as sparx5. The problem is that pinconf_ops is specific to
> sparx5. More precisely the offset of the bits in the pincfg register are
> different and also lan966x doesn't have support for
> PIN_CONFIG_INPUT_SCHMITT_ENABLE.
>
> Fix this by making pinconf_ops more generic such that it can be also
> used by lan966x. This is done by introducing 'ocelot_pincfg_data' which
> contains the offset and what is supported for each SOC.
Looks better, my comments below.
...
> + const struct ocelot_pincfg_data *opd = info->pincfg_data;
This one...
...
> @@ -1425,17 +1436,20 @@ static int ocelot_pinconf_get(struct pinctrl_dev *pctldev,
...can also be applied here...
...
> case PIN_CONFIG_BIAS_PULL_DOWN:
> arg = (param == PIN_CONFIG_BIAS_DISABLE) ? 0 :
> - (param == PIN_CONFIG_BIAS_PULL_UP) ? BIAS_PU_BIT :
> - BIAS_PD_BIT;
> + (param == PIN_CONFIG_BIAS_PULL_UP) ? info->pincfg_data->pu_bit :
> + info->pincfg_data->pd_bit;
...which in particular may help to sort out this indentation mess.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/2] pinctrl: ocelot: Fix pincfg
2022-07-12 19:50 ` [PATCH v4 2/2] pinctrl: ocelot: Fix pincfg Horatiu Vultur
2022-07-13 8:35 ` Colin Foster
@ 2022-07-13 9:54 ` Andy Shevchenko
1 sibling, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2022-07-13 9:54 UTC (permalink / raw)
To: Horatiu Vultur
Cc: open list:GPIO SUBSYSTEM, Linux Kernel Mailing List,
Linus Walleij, Alexandre Belloni, kavyasree.kotagiri,
Colin Foster, Microchip Linux Driver Support, Maxime Chevallier,
Michael Walle
On Tue, Jul 12, 2022 at 9:46 PM Horatiu Vultur
<horatiu.vultur@microchip.com> wrote:
>
> The blamed commit changed to use regmaps instead of __iomem. But it
> didn't update the register offsets to be at word offset, so it uses byte
> offset.
> Another issue with the same commit is that it has a limit of 32 registers
> which is incorrect. The sparx5 has 64 while lan966x has 77.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Fixes: 076d9e71bcf8 ("pinctrl: ocelot: convert pinctrl to regmap")
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
> drivers/pinctrl/pinctrl-ocelot.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pinctrl/pinctrl-ocelot.c b/drivers/pinctrl/pinctrl-ocelot.c
> index 873bba245522..c5a9f87f0c49 100644
> --- a/drivers/pinctrl/pinctrl-ocelot.c
> +++ b/drivers/pinctrl/pinctrl-ocelot.c
> @@ -1334,7 +1334,9 @@ static int ocelot_hw_get_value(struct ocelot_pinctrl *info,
> const struct ocelot_pincfg_data *opd = info->pincfg_data;
> u32 regcfg;
>
> - ret = regmap_read(info->pincfg, pin, ®cfg);
> + ret = regmap_read(info->pincfg,
> + pin * regmap_get_reg_stride(info->pincfg),
> + ®cfg);
> if (ret)
> return ret;
>
> @@ -1366,14 +1368,18 @@ static int ocelot_pincfg_clrsetbits(struct ocelot_pinctrl *info, u32 regaddr,
> u32 val;
> int ret;
>
> - ret = regmap_read(info->pincfg, regaddr, &val);
> + ret = regmap_read(info->pincfg,
> + regaddr * regmap_get_reg_stride(info->pincfg),
> + &val);
> if (ret)
> return ret;
>
> val &= ~clrbits;
> val |= setbits;
>
> - ret = regmap_write(info->pincfg, regaddr, val);
> + ret = regmap_write(info->pincfg,
> + regaddr * regmap_get_reg_stride(info->pincfg),
> + val);
>
> return ret;
> }
> @@ -1932,7 +1938,8 @@ static const struct of_device_id ocelot_pinctrl_of_match[] = {
> {},
> };
>
> -static struct regmap *ocelot_pinctrl_create_pincfg(struct platform_device *pdev)
> +static struct regmap *ocelot_pinctrl_create_pincfg(struct platform_device *pdev,
> + const struct ocelot_pinctrl *info)
> {
> void __iomem *base;
>
> @@ -1940,7 +1947,7 @@ static struct regmap *ocelot_pinctrl_create_pincfg(struct platform_device *pdev)
> .reg_bits = 32,
> .val_bits = 32,
> .reg_stride = 4,
> - .max_register = 32,
> + .max_register = info->desc->npins * 4,
> .name = "pincfg",
> };
>
> @@ -2008,7 +2015,7 @@ static int ocelot_pinctrl_probe(struct platform_device *pdev)
>
> /* Pinconf registers */
> if (info->desc->confops) {
> - pincfg = ocelot_pinctrl_create_pincfg(pdev);
> + pincfg = ocelot_pinctrl_create_pincfg(pdev, info);
> if (IS_ERR(pincfg))
> dev_dbg(dev, "Failed to create pincfg regmap\n");
> else
> --
> 2.33.0
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-13 9:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-12 19:50 [PATCH v4 0/2] pinctrl: ocelot: Add fixes for ocelot driver Horatiu Vultur
2022-07-12 19:50 ` [PATCH v4 1/2] pinctrl: ocelot: Fix pincfg for lan966x Horatiu Vultur
2022-07-13 9:53 ` Andy Shevchenko
2022-07-12 19:50 ` [PATCH v4 2/2] pinctrl: ocelot: Fix pincfg Horatiu Vultur
2022-07-13 8:35 ` Colin Foster
2022-07-13 9:54 ` Andy Shevchenko
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.