From 7996c5f5ec7a20b3f6b8fae93fcf3cb8f1c01743 Mon Sep 17 00:00:00 2001 From: Michael Walle Date: Sat, 26 Feb 2022 21:45:03 +0100 Subject: pinctrl: microchip-sgpio: lock RMW access Protect any RMW access to the registers by a spinlock. Fixes: 7e5ea974e61c ("pinctrl: pinctrl-microchip-sgpio: Add pinctrl driver for Microsemi Serial GPIO") Signed-off-by: Michael Walle Link: https://lore.kernel.org/r/20220226204507.2511633-2-michael@walle.cc Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-microchip-sgpio.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'drivers/pinctrl/pinctrl-microchip-sgpio.c') diff --git a/drivers/pinctrl/pinctrl-microchip-sgpio.c b/drivers/pinctrl/pinctrl-microchip-sgpio.c index 8e081c90bdb2..642f1d7ec243 100644 --- a/drivers/pinctrl/pinctrl-microchip-sgpio.c +++ b/drivers/pinctrl/pinctrl-microchip-sgpio.c @@ -19,6 +19,7 @@ #include #include #include +#include #include "core.h" #include "pinconf.h" @@ -116,6 +117,7 @@ struct sgpio_priv { u32 clock; struct regmap *regs; const struct sgpio_properties *properties; + spinlock_t lock; }; struct sgpio_port_addr { @@ -228,6 +230,7 @@ static void sgpio_output_set(struct sgpio_priv *priv, int value) { unsigned int bit = SGPIO_SRC_BITS * addr->bit; + unsigned long flags; u32 clr, set; switch (priv->properties->arch) { @@ -246,7 +249,10 @@ static void sgpio_output_set(struct sgpio_priv *priv, default: return; } + + spin_lock_irqsave(&priv->lock, flags); sgpio_clrsetbits(priv, REG_PORT_CONFIG, addr->port, clr, set); + spin_unlock_irqrestore(&priv->lock, flags); } static int sgpio_output_get(struct sgpio_priv *priv, @@ -574,10 +580,13 @@ static void microchip_sgpio_irq_settype(struct irq_data *data, struct sgpio_bank *bank = gpiochip_get_data(chip); unsigned int gpio = irqd_to_hwirq(data); struct sgpio_port_addr addr; + unsigned long flags; u32 ena; sgpio_pin_to_addr(bank->priv, gpio, &addr); + spin_lock_irqsave(&bank->priv->lock, flags); + /* Disable interrupt while changing type */ ena = sgpio_readl(bank->priv, REG_INT_ENABLE, addr.bit); sgpio_writel(bank->priv, ena & ~BIT(addr.port), REG_INT_ENABLE, addr.bit); @@ -594,6 +603,8 @@ static void microchip_sgpio_irq_settype(struct irq_data *data, /* Possibly re-enable interrupts */ sgpio_writel(bank->priv, ena, REG_INT_ENABLE, addr.bit); + + spin_unlock_irqrestore(&bank->priv->lock, flags); } static void microchip_sgpio_irq_setreg(struct irq_data *data, @@ -604,13 +615,16 @@ static void microchip_sgpio_irq_setreg(struct irq_data *data, struct sgpio_bank *bank = gpiochip_get_data(chip); unsigned int gpio = irqd_to_hwirq(data); struct sgpio_port_addr addr; + unsigned long flags; sgpio_pin_to_addr(bank->priv, gpio, &addr); + spin_lock_irqsave(&bank->priv->lock, flags); if (clear) sgpio_clrsetbits(bank->priv, reg, addr.bit, BIT(addr.port), 0); else sgpio_clrsetbits(bank->priv, reg, addr.bit, 0, BIT(addr.port)); + spin_unlock_irqrestore(&bank->priv->lock, flags); } static void microchip_sgpio_irq_mask(struct irq_data *data) @@ -832,6 +846,7 @@ static int microchip_sgpio_probe(struct platform_device *pdev) return -ENOMEM; priv->dev = dev; + spin_lock_init(&priv->lock); reset = devm_reset_control_get_optional_shared(&pdev->dev, "switch"); if (IS_ERR(reset)) -- cgit v1.2.3 From cd8c48b4efcfb2e71ef8a5d2208a0db5002b78bf Mon Sep 17 00:00:00 2001 From: Michael Walle Date: Sat, 26 Feb 2022 21:45:04 +0100 Subject: pinctrl: microchip-sgpio: don't do RMW for interrupt ack register The interrupt ack register has the usual "write one to clear" semantics. No read-modify-write is required here. This is also a preparation patch to change the sgpio_clrsetbits() to use regmap_update_bits() which don't write the value if it is not changed. Signed-off-by: Michael Walle Link: https://lore.kernel.org/r/20220226204507.2511633-3-michael@walle.cc Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-microchip-sgpio.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'drivers/pinctrl/pinctrl-microchip-sgpio.c') diff --git a/drivers/pinctrl/pinctrl-microchip-sgpio.c b/drivers/pinctrl/pinctrl-microchip-sgpio.c index 642f1d7ec243..ce62bbb80dc7 100644 --- a/drivers/pinctrl/pinctrl-microchip-sgpio.c +++ b/drivers/pinctrl/pinctrl-microchip-sgpio.c @@ -639,7 +639,14 @@ static void microchip_sgpio_irq_unmask(struct irq_data *data) static void microchip_sgpio_irq_ack(struct irq_data *data) { - microchip_sgpio_irq_setreg(data, REG_INT_ACK, false); + struct gpio_chip *chip = irq_data_get_irq_chip_data(data); + struct sgpio_bank *bank = gpiochip_get_data(chip); + unsigned int gpio = irqd_to_hwirq(data); + struct sgpio_port_addr addr; + + sgpio_pin_to_addr(bank->priv, gpio, &addr); + + sgpio_writel(bank->priv, BIT(addr.port), REG_INT_ACK, addr.bit); } static int microchip_sgpio_irq_set_type(struct irq_data *data, unsigned int type) -- cgit v1.2.3 From 0e68328edb3227cd00681671843c3d4113083456 Mon Sep 17 00:00:00 2001 From: Michael Walle Date: Sat, 26 Feb 2022 21:45:05 +0100 Subject: pinctrl: microchip-sgpio: use regmap_update_bits() Convert sgpio_clrsetbits() to use regmap_update_bits() and drop the spinlocks because regmap already takes care of the locking. Signed-off-by: Michael Walle Link: https://lore.kernel.org/r/20220226204507.2511633-4-michael@walle.cc Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-microchip-sgpio.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) (limited to 'drivers/pinctrl/pinctrl-microchip-sgpio.c') diff --git a/drivers/pinctrl/pinctrl-microchip-sgpio.c b/drivers/pinctrl/pinctrl-microchip-sgpio.c index ce62bbb80dc7..c5d0663148ac 100644 --- a/drivers/pinctrl/pinctrl-microchip-sgpio.c +++ b/drivers/pinctrl/pinctrl-microchip-sgpio.c @@ -167,12 +167,11 @@ static void sgpio_writel(struct sgpio_priv *priv, static inline void sgpio_clrsetbits(struct sgpio_priv *priv, u32 rno, u32 off, u32 clear, u32 set) { - u32 val = sgpio_readl(priv, rno, off); - - val &= ~clear; - val |= set; + u32 addr = sgpio_get_addr(priv, rno, off); + int ret; - sgpio_writel(priv, val, rno, off); + ret = regmap_update_bits(priv->regs, addr, clear | set, set); + WARN_ONCE(ret, "error updating sgpio reg %d\n", ret); } static inline void sgpio_configure_bitstream(struct sgpio_priv *priv) @@ -230,7 +229,6 @@ static void sgpio_output_set(struct sgpio_priv *priv, int value) { unsigned int bit = SGPIO_SRC_BITS * addr->bit; - unsigned long flags; u32 clr, set; switch (priv->properties->arch) { @@ -250,9 +248,7 @@ static void sgpio_output_set(struct sgpio_priv *priv, return; } - spin_lock_irqsave(&priv->lock, flags); sgpio_clrsetbits(priv, REG_PORT_CONFIG, addr->port, clr, set); - spin_unlock_irqrestore(&priv->lock, flags); } static int sgpio_output_get(struct sgpio_priv *priv, @@ -615,16 +611,13 @@ static void microchip_sgpio_irq_setreg(struct irq_data *data, struct sgpio_bank *bank = gpiochip_get_data(chip); unsigned int gpio = irqd_to_hwirq(data); struct sgpio_port_addr addr; - unsigned long flags; sgpio_pin_to_addr(bank->priv, gpio, &addr); - spin_lock_irqsave(&bank->priv->lock, flags); if (clear) sgpio_clrsetbits(bank->priv, reg, addr.bit, BIT(addr.port), 0); else sgpio_clrsetbits(bank->priv, reg, addr.bit, 0, BIT(addr.port)); - spin_unlock_irqrestore(&bank->priv->lock, flags); } static void microchip_sgpio_irq_mask(struct irq_data *data) -- cgit v1.2.3 From 2560c681999c9d33eb32b208de6a607622867c95 Mon Sep 17 00:00:00 2001 From: Michael Walle Date: Sat, 26 Feb 2022 21:45:06 +0100 Subject: pinctrl: microchip-sgpio: return error in spgio_output_set() Make sgpio_output_set() return an error value. Don't just ignore the return value of any regmap access but propagate it to our callers. Even if the accesses never fail, this is a preparation patch to add single shot mode where we need to poll a bit and thus we might get -ETIMEDOUT. Signed-off-by: Michael Walle Link: https://lore.kernel.org/r/20220226204507.2511633-5-michael@walle.cc Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-microchip-sgpio.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'drivers/pinctrl/pinctrl-microchip-sgpio.c') diff --git a/drivers/pinctrl/pinctrl-microchip-sgpio.c b/drivers/pinctrl/pinctrl-microchip-sgpio.c index c5d0663148ac..53e4e6699c48 100644 --- a/drivers/pinctrl/pinctrl-microchip-sgpio.c +++ b/drivers/pinctrl/pinctrl-microchip-sgpio.c @@ -224,9 +224,9 @@ static inline void sgpio_configure_clock(struct sgpio_priv *priv, u32 clkfrq) sgpio_clrsetbits(priv, REG_SIO_CLOCK, 0, clr, set); } -static void sgpio_output_set(struct sgpio_priv *priv, - struct sgpio_port_addr *addr, - int value) +static int sgpio_output_set(struct sgpio_priv *priv, + struct sgpio_port_addr *addr, + int value) { unsigned int bit = SGPIO_SRC_BITS * addr->bit; u32 clr, set; @@ -245,10 +245,12 @@ static void sgpio_output_set(struct sgpio_priv *priv, set = FIELD_PREP(SGPIO_SPARX5_BIT_SOURCE, value << bit); break; default: - return; + return -EINVAL; } sgpio_clrsetbits(priv, REG_PORT_CONFIG, addr->port, clr, set); + + return 0; } static int sgpio_output_get(struct sgpio_priv *priv, @@ -334,7 +336,7 @@ static int sgpio_pinconf_set(struct pinctrl_dev *pctldev, unsigned int pin, case PIN_CONFIG_OUTPUT: if (bank->is_input) return -EINVAL; - sgpio_output_set(priv, &addr, arg); + err = sgpio_output_set(priv, &addr, arg); break; default: @@ -474,9 +476,7 @@ static int microchip_sgpio_direction_output(struct gpio_chip *gc, sgpio_pin_to_addr(priv, gpio, &addr); - sgpio_output_set(priv, &addr, value); - - return 0; + return sgpio_output_set(priv, &addr, value); } static int microchip_sgpio_get_direction(struct gpio_chip *gc, unsigned int gpio) -- cgit v1.2.3 From 1f3c85c22db4aecee4080d42fccc336a1bbc5da7 Mon Sep 17 00:00:00 2001 From: Michael Walle Date: Sat, 26 Feb 2022 21:45:07 +0100 Subject: pinctrl: microchip-sgpio: wait until output is actually set Right now, when a gpio value is set, the actual hardware pin gets set asynchronously. When linux write the output register, it takes some time until it is actually propagated to the output shift registers. If that output port is connected to an I2C mux for example, the linux driver assumes the I2C bus is already switched although it is not. Fortunately, there is a single shot mode with a feedback: you can trigger the single shot and the hardware will clear that bit once it has finished the clocking and strobed the load signal of the shift registers. This can take a considerable amount of time though. Measuremens have shown that it takes up to a whole burst cycle gap which is about 50ms on the largest setting. Therefore, we have to mark the output bank as sleepable. To avoid unnecessary waiting, just trigger the single shot if the value was actually changed. Signed-off-by: Michael Walle Link: https://lore.kernel.org/r/20220226204507.2511633-6-michael@walle.cc Signed-off-by: Linus Walleij --- drivers/pinctrl/pinctrl-microchip-sgpio.c | 69 ++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) (limited to 'drivers/pinctrl/pinctrl-microchip-sgpio.c') diff --git a/drivers/pinctrl/pinctrl-microchip-sgpio.c b/drivers/pinctrl/pinctrl-microchip-sgpio.c index 53e4e6699c48..bbfe4394c557 100644 --- a/drivers/pinctrl/pinctrl-microchip-sgpio.c +++ b/drivers/pinctrl/pinctrl-microchip-sgpio.c @@ -64,11 +64,13 @@ struct sgpio_properties { #define SGPIO_LUTON_BIT_SOURCE GENMASK(11, 0) #define SGPIO_OCELOT_AUTO_REPEAT BIT(10) +#define SGPIO_OCELOT_SINGLE_SHOT BIT(11) #define SGPIO_OCELOT_PORT_WIDTH GENMASK(8, 7) #define SGPIO_OCELOT_CLK_FREQ GENMASK(19, 8) #define SGPIO_OCELOT_BIT_SOURCE GENMASK(23, 12) #define SGPIO_SPARX5_AUTO_REPEAT BIT(6) +#define SGPIO_SPARX5_SINGLE_SHOT BIT(7) #define SGPIO_SPARX5_PORT_WIDTH GENMASK(4, 3) #define SGPIO_SPARX5_CLK_FREQ GENMASK(19, 8) #define SGPIO_SPARX5_BIT_SOURCE GENMASK(23, 12) @@ -118,6 +120,8 @@ struct sgpio_priv { struct regmap *regs; const struct sgpio_properties *properties; spinlock_t lock; + /* protects the config register and single shot mode */ + struct mutex poll_lock; }; struct sgpio_port_addr { @@ -224,12 +228,64 @@ static inline void sgpio_configure_clock(struct sgpio_priv *priv, u32 clkfrq) sgpio_clrsetbits(priv, REG_SIO_CLOCK, 0, clr, set); } +static int sgpio_single_shot(struct sgpio_priv *priv) +{ + u32 addr = sgpio_get_addr(priv, REG_SIO_CONFIG, 0); + int ret, ret2; + u32 ctrl; + unsigned int single_shot; + unsigned int auto_repeat; + + switch (priv->properties->arch) { + case SGPIO_ARCH_LUTON: + /* not supported for now */ + return 0; + case SGPIO_ARCH_OCELOT: + single_shot = SGPIO_OCELOT_SINGLE_SHOT; + auto_repeat = SGPIO_OCELOT_AUTO_REPEAT; + break; + case SGPIO_ARCH_SPARX5: + single_shot = SGPIO_SPARX5_SINGLE_SHOT; + auto_repeat = SGPIO_SPARX5_AUTO_REPEAT; + break; + default: + return -EINVAL; + } + + /* + * Trigger immediate burst. This only works when auto repeat is turned + * off. Otherwise, the single shot bit will never be cleared by the + * hardware. Measurements showed that an update might take as long as + * the burst gap. On a LAN9668 this is about 50ms for the largest + * setting. + * After the manual burst, reenable the auto repeat mode again. + */ + mutex_lock(&priv->poll_lock); + ret = regmap_update_bits(priv->regs, addr, single_shot | auto_repeat, + single_shot); + if (ret) + goto out; + + ret = regmap_read_poll_timeout(priv->regs, addr, ctrl, + !(ctrl & single_shot), 100, 60000); + + /* reenable auto repeat mode even if there was an error */ + ret2 = regmap_update_bits(priv->regs, addr, auto_repeat, auto_repeat); +out: + mutex_unlock(&priv->poll_lock); + + return ret ?: ret2; +} + static int sgpio_output_set(struct sgpio_priv *priv, struct sgpio_port_addr *addr, int value) { unsigned int bit = SGPIO_SRC_BITS * addr->bit; + u32 reg = sgpio_get_addr(priv, REG_PORT_CONFIG, addr->port); + bool changed; u32 clr, set; + int ret; switch (priv->properties->arch) { case SGPIO_ARCH_LUTON: @@ -248,7 +304,16 @@ static int sgpio_output_set(struct sgpio_priv *priv, return -EINVAL; } - sgpio_clrsetbits(priv, REG_PORT_CONFIG, addr->port, clr, set); + ret = regmap_update_bits_check(priv->regs, reg, clr | set, set, + &changed); + if (ret) + return ret; + + if (changed) { + ret = sgpio_single_shot(priv); + if (ret) + return ret; + } return 0; } @@ -787,6 +852,7 @@ static int microchip_sgpio_register_bank(struct device *dev, gc->of_gpio_n_cells = 3; gc->base = -1; gc->ngpio = ngpios; + gc->can_sleep = !bank->is_input; if (bank->is_input && priv->properties->flags & SGPIO_FLAGS_HAS_IRQ) { int irq = fwnode_irq_get(fwnode, 0); @@ -847,6 +913,7 @@ static int microchip_sgpio_probe(struct platform_device *pdev) priv->dev = dev; spin_lock_init(&priv->lock); + mutex_init(&priv->poll_lock); reset = devm_reset_control_get_optional_shared(&pdev->dev, "switch"); if (IS_ERR(reset)) -- cgit v1.2.3