git.ipfire.org Git - thirdparty/kernel/stable-queue.git/blob

   1 From 45c11a927606c612e4898a9484867b71318699f6 Mon Sep 17 00:00:00 2001
   2 From: Hans de Goede <hdegoede@redhat.com>
   3 Date: Sat, 6 Jun 2020 11:31:50 +0200
   4 Subject: pinctrl: baytrail: Fix pin being driven low for a while on gpiod_get(..., GPIOD_OUT_HIGH)
   5
   6 From: Hans de Goede <hdegoede@redhat.com>
   7
   8 commit 45c11a927606c612e4898a9484867b71318699f6 upstream.
   9
  10 The pins on the Bay Trail SoC have separate input-buffer and output-buffer
  11 enable bits and a read of the level bit of the value register will always
  12 return the value from the input-buffer.
  13
  14 The BIOS of a device may configure a pin in output-only mode, only enabling
  15 the output buffer, and write 1 to the level bit to drive the pin high.
  16 This 1 written to the level bit will be stored inside the data-latch of the
  17 output buffer.
  18
  19 But a subsequent read of the value register will return 0 for the level bit
  20 because the input-buffer is disabled. This causes a read-modify-write as
  21 done by byt_gpio_set_direction() to write 0 to the level bit, driving the
  22 pin low!
  23
  24 Before this commit byt_gpio_direction_output() relied on
  25 pinctrl_gpio_direction_output() to set the direction, followed by a call
  26 to byt_gpio_set() to apply the selected value. This causes the pin to
  27 go low between the pinctrl_gpio_direction_output() and byt_gpio_set()
  28 calls.
  29
  30 Change byt_gpio_direction_output() to directly make the register
  31 modifications itself instead. Replacing the 2 subsequent writes to the
  32 value register with a single write.
  33
  34 Note that the pinctrl code does not keep track internally of the direction,
  35 so not going through pinctrl_gpio_direction_output() is not an issue.
  36
  37 This issue was noticed on a Trekstor SurfTab Twin 10.1. When the panel is
  38 already on at boot (no external monitor connected), then the i915 driver
  39 does a gpiod_get(..., GPIOD_OUT_HIGH) for the panel-enable GPIO. The
  40 temporarily going low of that GPIO was causing the panel to reset itself
  41 after which it would not show an image until it was turned off and back on
  42 again (until a full modeset was done on it). This commit fixes this.
  43
  44 This commit also updates the byt_gpio_direction_input() to use direct
  45 register accesses instead of going through pinctrl_gpio_direction_input(),
  46 to keep it consistent with byt_gpio_direction_output().
  47
  48 Note for backporting, this commit depends on:
  49 commit e2b74419e5cc ("pinctrl: baytrail: Replace WARN with dev_info_once
  50 when setting direct-irq pin to output")
  51
  52 Cc: stable@vger.kernel.org
  53 Fixes: 86e3ef812fe3 ("pinctrl: baytrail: Update gpio chip operations")
  54 Signed-off-by: Hans de Goede <hdegoede@redhat.com>
  55 Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
  56 Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
  57 Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  58
  59 ---
  60  drivers/pinctrl/intel/pinctrl-baytrail.c |   67 ++++++++++++++++++++++++-------
  61  1 file changed, 53 insertions(+), 14 deletions(-)
  62
  63 --- a/drivers/pinctrl/intel/pinctrl-baytrail.c
  64 +++ b/drivers/pinctrl/intel/pinctrl-baytrail.c
  65 @@ -800,6 +800,21 @@ static void byt_gpio_disable_free(struct
  66         pm_runtime_put(vg->dev);
  67  }
  68
  69 +static void byt_gpio_direct_irq_check(struct intel_pinctrl *vg,
  70 +                                     unsigned int offset)
  71 +{
  72 +       void __iomem *conf_reg = byt_gpio_reg(vg, offset, BYT_CONF0_REG);
  73 +
  74 +       /*
  75 +        * Before making any direction modifications, do a check if gpio is set
  76 +        * for direct IRQ. On Bay Trail, setting GPIO to output does not make
  77 +        * sense, so let's at least inform the caller before they shoot
  78 +        * themselves in the foot.
  79 +        */
  80 +       if (readl(conf_reg) & BYT_DIRECT_IRQ_EN)
  81 +               dev_info_once(vg->dev, "Potential Error: Setting GPIO with direct_irq_en to output");
  82 +}
  83 +
  84  static int byt_gpio_set_direction(struct pinctrl_dev *pctl_dev,
  85                                   struct pinctrl_gpio_range *range,
  86                                   unsigned int offset,
  87 @@ -807,7 +822,6 @@ static int byt_gpio_set_direction(struct
  88  {
  89         struct intel_pinctrl *vg = pinctrl_dev_get_drvdata(pctl_dev);
  90         void __iomem *val_reg = byt_gpio_reg(vg, offset, BYT_VAL_REG);
  91 -       void __iomem *conf_reg = byt_gpio_reg(vg, offset, BYT_CONF0_REG);
  92         unsigned long flags;
  93         u32 value;
  94
  95 @@ -817,14 +831,8 @@ static int byt_gpio_set_direction(struct
  96         value &= ~BYT_DIR_MASK;
  97         if (input)
  98                 value |= BYT_OUTPUT_EN;
  99 -       else if (readl(conf_reg) & BYT_DIRECT_IRQ_EN)
 100 -               /*
 101 -                * Before making any direction modifications, do a check if gpio
 102 -                * is set for direct IRQ.  On baytrail, setting GPIO to output
 103 -                * does not make sense, so let's at least inform the caller before
 104 -                * they shoot themselves in the foot.
 105 -                */
 106 -               dev_info_once(vg->dev, "Potential Error: Setting GPIO with direct_irq_en to output");
 107 +       else
 108 +               byt_gpio_direct_irq_check(vg, offset);
 109
 110         writel(value, val_reg);
 111
 112 @@ -1165,19 +1173,50 @@ static int byt_gpio_get_direction(struct
 113
 114  static int byt_gpio_direction_input(struct gpio_chip *chip, unsigned int offset)
 115  {
 116 -       return pinctrl_gpio_direction_input(chip->base + offset);
 117 +       struct intel_pinctrl *vg = gpiochip_get_data(chip);
 118 +       void __iomem *val_reg = byt_gpio_reg(vg, offset, BYT_VAL_REG);
 119 +       unsigned long flags;
 120 +       u32 reg;
 121 +
 122 +       raw_spin_lock_irqsave(&byt_lock, flags);
 123 +
 124 +       reg = readl(val_reg);
 125 +       reg &= ~BYT_DIR_MASK;
 126 +       reg |= BYT_OUTPUT_EN;
 127 +       writel(reg, val_reg);
 128 +
 129 +       raw_spin_unlock_irqrestore(&byt_lock, flags);
 130 +       return 0;
 131  }
 132
 133 +/*
 134 + * Note despite the temptation this MUST NOT be converted into a call to
 135 + * pinctrl_gpio_direction_output() + byt_gpio_set() that does not work this
 136 + * MUST be done as a single BYT_VAL_REG register write.
 137 + * See the commit message of the commit adding this comment for details.
 138 + */
 139  static int byt_gpio_direction_output(struct gpio_chip *chip,
 140                                      unsigned int offset, int value)
 141  {
 142 -       int ret = pinctrl_gpio_direction_output(chip->base + offset);
 143 +       struct intel_pinctrl *vg = gpiochip_get_data(chip);
 144 +       void __iomem *val_reg = byt_gpio_reg(vg, offset, BYT_VAL_REG);
 145 +       unsigned long flags;
 146 +       u32 reg;
 147
 148 -       if (ret)
 149 -               return ret;
 150 +       raw_spin_lock_irqsave(&byt_lock, flags);
 151 +
 152 +       byt_gpio_direct_irq_check(vg, offset);
 153 +
 154 +       reg = readl(val_reg);
 155 +       reg &= ~BYT_DIR_MASK;
 156 +       if (value)
 157 +               reg |= BYT_LEVEL;
 158 +       else
 159 +               reg &= ~BYT_LEVEL;
 160
 161 -       byt_gpio_set(chip, offset, value);
 162 +       writel(reg, val_reg);
 163
 164 +       raw_spin_unlock_irqrestore(&byt_lock, flags);
 165         return 0;
 166  }
 167