On Mon, Jun 02, 2014 at 02:12:07PM -0700, eric.er...@linux.intel.com wrote: > From: Eric Ernst <eric.er...@linux.intel.com> > > For Baytrail, you should never set a GPIO set to direct_irq > to output mode. When direct_irq_en is set for a GPIO, it is > tied directly to an APIC internally, and making the pad output > does not make any sense. Assert a WARN() in the event this happens.
Subject should probably be: pinctrl: baytrail: Warn if direct IRQ GPIO is set to output > Signed-off-by: Eric Ernst <eric.er...@linux.intel.com> > --- > drivers/pinctrl/pinctrl-baytrail.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/drivers/pinctrl/pinctrl-baytrail.c > b/drivers/pinctrl/pinctrl-baytrail.c > index e59983423991..fde3767df254 100644 > --- a/drivers/pinctrl/pinctrl-baytrail.c > +++ b/drivers/pinctrl/pinctrl-baytrail.c > @@ -47,6 +47,7 @@ > #define BYT_TRIG_POS BIT(25) > #define BYT_TRIG_LVL BIT(24) > #define BYT_PIN_MUX 0x07 > +#define BYT_DIRECTIRQ BIT(27) Please move this definition to be first, like: /* BYT_CONF0_REG register bits */ #define BYT_DIRECT_IRQ_EN BIT(27) #define BYT_TRIG_NEG BIT(26) #define BYT_TRIG_POS BIT(25) and I would call it BYT_DIRECT_IRQ_EN since it's name in datasheet is "direct_irq_en". > > /* BYT_VAL_REG register bits */ > #define BYT_INPUT_EN BIT(2) /* 0: input enabled (active low)*/ > @@ -256,19 +257,29 @@ static int byt_gpio_direction_output(struct gpio_chip > *chip, > unsigned gpio, int value) > { > struct byt_gpio *vg = to_byt_gpio(chip); > - void __iomem *reg = byt_gpio_reg(chip, gpio, BYT_VAL_REG); > + void __iomem *conf_reg = byt_gpio_reg(chip, gpio, BYT_CONF0_REG); > + void __iomem *value_reg = byt_gpio_reg(chip, gpio, BYT_VAL_REG); Not sure if it is necessary to rename reg -> value_reg. It just makes the patch bigger than it has to be since you also need to rename stuff below. Otherwise looks good. > unsigned long flags; > u32 reg_val; > > spin_lock_irqsave(&vg->lock, flags); > > - reg_val = readl(reg) | BYT_DIR_MASK; > + /* > + * 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 warn the caller before they shoot > + * themselves in the foot. > + */ > + WARN((readl(conf_reg) & BYT_DIRECTIRQ), > + "Potential Error: Setting GPIO with direct_irq_en to output"); > + > + reg_val = readl(value_reg) | BYT_DIR_MASK; > reg_val &= ~BYT_OUTPUT_EN; > > if (value) > - writel(reg_val | BYT_LEVEL, reg); > + writel(reg_val | BYT_LEVEL, value_reg); > else > - writel(reg_val & ~BYT_LEVEL, reg); > + writel(reg_val & ~BYT_LEVEL, value_reg); > > spin_unlock_irqrestore(&vg->lock, flags); > > -- > 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/