On 29/09/14 15:46, Srinivas Kandagatla wrote: > On 29/09/14 14:48, Pramod Gurav wrote: >> This change fixes below sparse error, >> drivers/pinctrl/pinctrl-st.c:1515:31: error: incompatible types for >> operation (>) >> drivers/pinctrl/pinctrl-st.c:1515:31: left side has type void >> [noderef] <asn:2>*irqmux_base >> drivers/pinctrl/pinctrl-st.c:1515:31: right side has type int >> >> The fix is done by removing a check on info->irqmux_base as >> info->irqmux_base has already been checked for error when allocating it. >> Hence there is no need to redo the check. >> >> Cc: Maxime Coquelin <[email protected]> >> Cc: Patrice Chotard <[email protected]> >> CC: Linus Walleij <[email protected]> >> Cc: Srinivas Kandagatla <[email protected]> >> Signed-off-by: Pramod Gurav <[email protected]> >> --- >> drivers/pinctrl/pinctrl-st.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c >> index 5475374..ddeaeda 100644 >> --- a/drivers/pinctrl/pinctrl-st.c >> +++ b/drivers/pinctrl/pinctrl-st.c >> @@ -1512,7 +1512,7 @@ static int st_gpiolib_register_bank(struct >> st_pinctrl *info, >> gpio_irq, st_gpio_irq_handler); >> } >> >> - if (info->irqmux_base > 0 || gpio_irq > 0) { >> + if (gpio_irq > 0) { >> err = gpiochip_irqchip_add(&bank->gpio_chip, &st_gpio_irqchip, >> 0, handle_simple_irq, >> IRQ_TYPE_LEVEL_LOW); > > This is not the correct fix. Please see why irqmux_base and gpio_irq are > used in the driver. > You are breaking the logic here... > > please read the below comment from the code. > > /** > * GPIO bank can have one of the two possible types of > * interrupt-wirings. > * > * First type is via irqmux, single interrupt is used by multiple > * gpio banks. This reduces number of overall interrupts numbers > * required. All these banks belong to a single pincontroller. > * _________ > * | |----> [gpio-bank (n) ] > * | |----> [gpio-bank (n + 1)] > * [irqN]-- | irq-mux |----> [gpio-bank (n + 2)] > * | |----> [gpio-bank (... )] > * |_________|----> [gpio-bank (n + 7)] > * > * Second type has a dedicated interrupt per each gpio bank. > * > * [irqN]----> [gpio-bank (n)] > */ > > > so irqmux_base is first type and gpio_irq is second type. > if you remove check for irqmux_base here you would end up NOT adding > irqchip the gpiochip in first type so you break the existing logic here. > > > I think the correct fix is: > > diff --git a/drivers/pinctrl/pinctrl-st.c b/drivers/pinctrl/pinctrl-st.c > index 5475374..4060c30 100644 > --- a/drivers/pinctrl/pinctrl-st.c > +++ b/drivers/pinctrl/pinctrl-st.c > @@ -1512,7 +1512,7 @@ static int st_gpiolib_register_bank(struct > st_pinctrl *info, > gpio_irq, > st_gpio_irq_handler); > } > > - if (info->irqmux_base > 0 || gpio_irq > 0) { > + if (!IS_ERR(info->irqmux_base) || gpio_irq > 0) { > err = gpiochip_irqchip_add(&bank->gpio_chip, > &st_gpio_irqchip, > 0, handle_simple_irq, > IRQ_TYPE_LEVEL_LOW);
IS_ERR() should be irrelavent because the allocation code bombs out on error. Shouldn't this just be a NULL pointer check? if(info->irqmux_base || gpio_irq > 0) Daniel. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
