On 11/25/2014 07:53 PM, Hartmut Knaack wrote: > Ezequiel Garcia schrieb am 25.11.2014 23:03: >> >> >> On 11/25/2014 06:41 PM, Hartmut Knaack wrote: >>>> >>>> Unless I'm missing something, that's exactly what XOR does. >>>> >>>> Example: >>>> >>>> reserved_channels = 0x2; >>>> channel_map = 0x7 ^ reserved_channels -> 0x5 >>>> >>> You miss to check ret for a valid range, which you don't need to do when masking out channels. >>> Example: >>> reserved_channels = 0x8; >>> channel_map = 0x7 ^ reserved_channels -> 0xf >>> And this is actually happening in the current revision (with the wrong channel_map assignment), that a reserved channel in DT would become usable. >> >> Right, definitely a check is needed. > No, that's not needed when using AND, as it can not add bits in the channel map. The following will do the job: > adc_dev->channel_map &= ~ret; Gah, of course, that's way much better. >> >>>>>> + >>>>>> + adc_dev->reg = devm_regulator_get(&pdev->dev, "vref"); >>>>>> + if (IS_ERR_OR_NULL(adc_dev->reg)) >>>>>> + return -EINVAL; >>>>> if (IS_ERR(adc_dev->reg)) >>>>> return PTR_ERR(adc_dev->reg); >>>> >>>> Are you sure? What if devm_regulator_get() returns NULL? >>>> >>> I had a look at it, but couldn't figure out a condition in which it would return NULL. But I'd be happy to be informed of anything different. >> >> If CONFIG_REGULATOR=n you get NULL there (although I added a select REGULATOR >> on the v4 I just posted). >> > Indeed, thanks. Now I'm thinking if it would make sense to handle this special case in a special way (as it would be caused by an invalid kernel config). Well, I guess, I leave it up to you. Well, adding the select REGULATOR that shouldn't happen, so let's better drop the NULL check entirely. -- Ezequiel -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
