From e87daf0bd83cb82576940d0a6e0e48b01d18c939 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 12 Jun 2020 17:50:06 +0300 Subject: pinctrl: baytrail: Drop no-op ACPI_PTR() call Since we dependent on ACPI, there is no need to use ACPI_PTR() which is a no-op in this case. Signed-off-by: Andy Shevchenko Acked-by: Mika Westerberg --- drivers/pinctrl/intel/pinctrl-baytrail.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/pinctrl/intel/pinctrl-baytrail.c') diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c index 0ff7c55173da..e3ceb3dfeabe 100644 --- a/drivers/pinctrl/intel/pinctrl-baytrail.c +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c @@ -1757,9 +1757,8 @@ static struct platform_driver byt_gpio_driver = { .driver = { .name = "byt_gpio", .pm = &byt_gpio_pm_ops, + .acpi_match_table = byt_gpio_acpi_match, .suppress_bind_attrs = true, - - .acpi_match_table = ACPI_PTR(byt_gpio_acpi_match), }, }; -- cgit v1.2.3 From 156abe2961601d60a8c2a60c6dc8dd6ce7adcdaf Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sat, 6 Jun 2020 11:31:50 +0200 Subject: pinctrl: baytrail: Fix pin being driven low for a while on gpiod_get(..., GPIOD_OUT_HIGH) The pins on the Bay Trail SoC have separate input-buffer and output-buffer enable bits and a read of the level bit of the value register will always return the value from the input-buffer. The BIOS of a device may configure a pin in output-only mode, only enabling the output buffer, and write 1 to the level bit to drive the pin high. This 1 written to the level bit will be stored inside the data-latch of the output buffer. But a subsequent read of the value register will return 0 for the level bit because the input-buffer is disabled. This causes a read-modify-write as done by byt_gpio_set_direction() to write 0 to the level bit, driving the pin low! Before this commit byt_gpio_direction_output() relied on pinctrl_gpio_direction_output() to set the direction, followed by a call to byt_gpio_set() to apply the selected value. This causes the pin to go low between the pinctrl_gpio_direction_output() and byt_gpio_set() calls. Change byt_gpio_direction_output() to directly make the register modifications itself instead. Replacing the 2 subsequent writes to the value register with a single write. Note that the pinctrl code does not keep track internally of the direction, so not going through pinctrl_gpio_direction_output() is not an issue. This issue was noticed on a Trekstor SurfTab Twin 10.1. When the panel is already on at boot (no external monitor connected), then the i915 driver does a gpiod_get(..., GPIOD_OUT_HIGH) for the panel-enable GPIO. The temporarily going low of that GPIO was causing the panel to reset itself after which it would not show an image until it was turned off and back on again (until a full modeset was done on it). This commit fixes this. This commit also updates the byt_gpio_direction_input() to use direct register accesses instead of going through pinctrl_gpio_direction_input(), to keep it consistent with byt_gpio_direction_output(). Note for backporting, this commit depends on: commit e2b74419e5cc ("pinctrl: baytrail: Replace WARN with dev_info_once when setting direct-irq pin to output") Cc: stable@vger.kernel.org Fixes: 86e3ef812fe3 ("pinctrl: baytrail: Update gpio chip operations") Signed-off-by: Hans de Goede Acked-by: Mika Westerberg Signed-off-by: Andy Shevchenko --- drivers/pinctrl/intel/pinctrl-baytrail.c | 67 +++++++++++++++++++++++++------- 1 file changed, 53 insertions(+), 14 deletions(-) (limited to 'drivers/pinctrl/intel/pinctrl-baytrail.c') diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c index e3ceb3dfeabe..a917a2df520e 100644 --- a/drivers/pinctrl/intel/pinctrl-baytrail.c +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c @@ -800,6 +800,21 @@ static void byt_gpio_disable_free(struct pinctrl_dev *pctl_dev, pm_runtime_put(vg->dev); } +static void byt_gpio_direct_irq_check(struct intel_pinctrl *vg, + unsigned int offset) +{ + void __iomem *conf_reg = byt_gpio_reg(vg, offset, BYT_CONF0_REG); + + /* + * Before making any direction modifications, do a check if gpio is set + * for direct IRQ. On Bay Trail, setting GPIO to output does not make + * sense, so let's at least inform the caller before they shoot + * themselves in the foot. + */ + if (readl(conf_reg) & BYT_DIRECT_IRQ_EN) + dev_info_once(vg->dev, "Potential Error: Setting GPIO with direct_irq_en to output"); +} + static int byt_gpio_set_direction(struct pinctrl_dev *pctl_dev, struct pinctrl_gpio_range *range, unsigned int offset, @@ -807,7 +822,6 @@ static int byt_gpio_set_direction(struct pinctrl_dev *pctl_dev, { struct intel_pinctrl *vg = pinctrl_dev_get_drvdata(pctl_dev); void __iomem *val_reg = byt_gpio_reg(vg, offset, BYT_VAL_REG); - void __iomem *conf_reg = byt_gpio_reg(vg, offset, BYT_CONF0_REG); unsigned long flags; u32 value; @@ -817,14 +831,8 @@ static int byt_gpio_set_direction(struct pinctrl_dev *pctl_dev, value &= ~BYT_DIR_MASK; if (input) value |= BYT_OUTPUT_EN; - else if (readl(conf_reg) & BYT_DIRECT_IRQ_EN) - /* - * Before making any direction modifications, do a check if gpio - * is set for direct IRQ. On baytrail, setting GPIO to output - * does not make sense, so let's at least inform the caller before - * they shoot themselves in the foot. - */ - dev_info_once(vg->dev, "Potential Error: Setting GPIO with direct_irq_en to output"); + else + byt_gpio_direct_irq_check(vg, offset); writel(value, val_reg); @@ -1165,19 +1173,50 @@ static int byt_gpio_get_direction(struct gpio_chip *chip, unsigned int offset) static int byt_gpio_direction_input(struct gpio_chip *chip, unsigned int offset) { - return pinctrl_gpio_direction_input(chip->base + offset); + struct intel_pinctrl *vg = gpiochip_get_data(chip); + void __iomem *val_reg = byt_gpio_reg(vg, offset, BYT_VAL_REG); + unsigned long flags; + u32 reg; + + raw_spin_lock_irqsave(&byt_lock, flags); + + reg = readl(val_reg); + reg &= ~BYT_DIR_MASK; + reg |= BYT_OUTPUT_EN; + writel(reg, val_reg); + + raw_spin_unlock_irqrestore(&byt_lock, flags); + return 0; } +/* + * Note despite the temptation this MUST NOT be converted into a call to + * pinctrl_gpio_direction_output() + byt_gpio_set() that does not work this + * MUST be done as a single BYT_VAL_REG register write. + * See the commit message of the commit adding this comment for details. + */ static int byt_gpio_direction_output(struct gpio_chip *chip, unsigned int offset, int value) { - int ret = pinctrl_gpio_direction_output(chip->base + offset); + struct intel_pinctrl *vg = gpiochip_get_data(chip); + void __iomem *val_reg = byt_gpio_reg(vg, offset, BYT_VAL_REG); + unsigned long flags; + u32 reg; - if (ret) - return ret; + raw_spin_lock_irqsave(&byt_lock, flags); + + byt_gpio_direct_irq_check(vg, offset); - byt_gpio_set(chip, offset, value); + reg = readl(val_reg); + reg &= ~BYT_DIR_MASK; + if (value) + reg |= BYT_LEVEL; + else + reg &= ~BYT_LEVEL; + writel(reg, val_reg); + + raw_spin_unlock_irqrestore(&byt_lock, flags); return 0; } -- cgit v1.2.3 From 0a0930206762b34c74bf14242765dc52d56f2792 Mon Sep 17 00:00:00 2001 From: "Gustavo A. R. Silva" Date: Thu, 16 Jul 2020 16:19:19 -0500 Subject: pinctrl: baytrail: Use fallthrough pseudo-keyword Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. [1] https://www.kernel.org/doc/html/latest/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through Signed-off-by: Gustavo A. R. Silva Signed-off-by: Andy Shevchenko --- drivers/pinctrl/intel/pinctrl-baytrail.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/pinctrl/intel/pinctrl-baytrail.c') diff --git a/drivers/pinctrl/intel/pinctrl-baytrail.c b/drivers/pinctrl/intel/pinctrl-baytrail.c index a917a2df520e..d6e35cba3065 100644 --- a/drivers/pinctrl/intel/pinctrl-baytrail.c +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c @@ -1372,13 +1372,13 @@ static void byt_irq_unmask(struct irq_data *d) switch (irqd_get_trigger_type(d)) { case IRQ_TYPE_LEVEL_HIGH: value |= BYT_TRIG_LVL; - /* fall through */ + fallthrough; case IRQ_TYPE_EDGE_RISING: value |= BYT_TRIG_POS; break; case IRQ_TYPE_LEVEL_LOW: value |= BYT_TRIG_LVL; - /* fall through */ + fallthrough; case IRQ_TYPE_EDGE_FALLING: value |= BYT_TRIG_NEG; break; -- cgit v1.2.3