Re: [PATCH v9 3/5] rtc: max77686: Fail to probe if no RTC regmap irqchip is set

[Javier Martinez Canillas]

Hello Krzysztof,

Sorry for the late response, I was on holidays and slowly catching up on email.

On 08/28/2014 11:21 AM, Krzysztof Kozlowski wrote:
> On pon, 2014-08-18 at 10:34 +0200, Javier Martinez Canillas wrote:
>> The max77686 mfd driver adds a regmap IRQ chip which creates an
>> IRQ domain that is used to map the virtual RTC alarm1 interrupt.
>> 
>> The RTC driver assumes that this will always be true since the
>> PMIC IRQ is a required property according to the max77686 DT
>> binding doc. If an "interrupts" property is not defined for a
>> max77686 PMIC, then the mfd probe function will fail and the
>> RTC platform driver will never be probed. But even when it is
>> not possible to probe the rtc-max77686 driver without a regmap
>> IRQ chip, it's better to explicitly check if the IRQ chip data
>> is not NULL and gracefully fail instead of getting an OOPS.
> 
> The OOPS was possible only with Bartlomiej's patch because he changed
> the MFD driver probe function to skip IRQ setup on lack of interrupts.
> In current state the OOPS should not happen so mentioning OOPS in commit
> message may be misleading. Maybe just don't put the OOPS here?
> 
> Anyway the patch looks good and a check for non-null
> regmap_irq_chip_data is still a valid precaution so:
>

Yes I know but as you said the check for non-null is still a valid precaution
(albeit maybe paranoid) just in case someone find that not having the IRQ
hooked makes sense in a design and changes the mfd driver in the future.

> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@xxxxxxxxxxx>
> 
> Best regards,
> Krzysztof
> 
> 

Best regards,
Javier
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/