Re: [PATCH 06/17] gpio: mvebu: add suspend/resume support

[Alexandre Courbot <gnurou@xxxxxxxxx>]

On Sat, Oct 25, 2014 at 5:45 AM, Andrew Lunn <andrew@xxxxxxx> wrote:
>> > +   switch (mvchip->soc_variant) {
>> > +   case MVEBU_GPIO_SOC_VARIANT_ORION:
>> > +           mvchip->edge_mask_regs[0] =
>> > +                   readl(mvchip->membase + GPIO_EDGE_MASK_OFF);
>> > +           mvchip->level_mask_regs[0] =
>> > +                   readl(mvchip->membase + GPIO_LEVEL_MASK_OFF);
>> > +           break;
>> > +   case MVEBU_GPIO_SOC_VARIANT_MV78200:
>> > +           for (i = 0; i < 2; i++) {
>> > +                   mvchip->edge_mask_regs[i] =
>> > +                           readl(mvchip->membase +
>> > +                                 GPIO_EDGE_MASK_MV78200_OFF(i));
>> > +                   mvchip->level_mask_regs[i] =
>> > +                           readl(mvchip->membase +
>> > +                                 GPIO_LEVEL_MASK_MV78200_OFF(i));
>> > +           }
>> > +           break;
>> > +   case MVEBU_GPIO_SOC_VARIANT_ARMADAXP:
>> > +           for (i = 0; i < 4; i++) {
>> > +                   mvchip->edge_mask_regs[i] =
>> > +                           readl(mvchip->membase +
>> > +                                 GPIO_EDGE_MASK_ARMADAXP_OFF(i));
>> > +                   mvchip->level_mask_regs[i] =
>> > +                           readl(mvchip->membase +
>> > +                                 GPIO_LEVEL_MASK_ARMADAXP_OFF(i));
>> > +           }
>> > +           break;
>> > +   default:
>> > +           BUG();
>>
>> Isn't it too severe? Is the platform going too unstable if driver
>> reaches this case?
>> I'd consider a WARN() instead.
>
> This is a common pattern in this driver. So i guess Thomas just
> cut/pasted the switch statement from _probe(), which also has the
> BUG().
>
> Given that _probe() should of thrown a BUG() in this situation, if it
> happens here, it means mvchip->soc_variant has been corrupted, and so
> bad things are happening. So a BUG() is maybe called for?

I agree that BUG() is adequate here. probe() should recognize the
exact same set of chips - if we reach this point this means that
either the data has been corrupted or we added support for a new chip
in probe() and forgot suspend/resume. In both cases the driver should
express its discontent.

Acked-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html