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);


Thanks,
srini
--
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/

Reply via email to