Hi Marteen, 2017-06-27 11:53 GMT+02:00 Maarten Brock <m.brock@xxxxxxxxxxxxx>: > Hello, > > Looking in the mcp3422.c iio kernel driver I see a few issues with the > initialization of the config word for the adc in mcp3422_probe(). > > 1. It uses MCP3422_CHANNEL_VALUE(1). Why is channel 2 the default? No particular reason here, it could be changed to channel 0 without problems. > 2. It tries to set MCP3422_CONT_SAMPLING, but if this first I2C transfer > fails it will never be set again. The initial config is not stored in > adc->config and adc->config is uninitialized! All subsequent writes will > leave MCP3422_CONT_SAMPLING off. Unfortunately this is true. I had should use adc->config instead of config there. > 3. The return value of mcp3422_update_config() is not checked. Not sure if > it should though. I wouldn't want a single failure to disable the driver. It > might even be that the adc is not available yet at linux boot time. And btw also the error condition should be checked. > Number 2 is the most important one. We saw it fail because our I2C goes > through an FPGA which was not loaded yet. > > I can write a patch, but welcome some insights first. I'll fix these in minutes, thanks for spotting! > > Kind regards, > Maarten Brock > -- Profile: http://it.linkedin.com/in/compagnucciangelo -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
