Hi Andy, Thanks for your revew! [...] > > > +static int mcp3910_get_offset(struct mcp3911 *adc, int channel, int *val) > > +{ > > + return mcp3911_read(adc, MCP3910_OFFCAL(channel), val, 3); > > Just to be sure, the proper endianess conversion is done in mcp3911_read() > and mcp3911_write() calls? > > This question applies to all calls to that APIs. Yes it does. [...] > > > +static int mcp3910_get_osr(struct mcp3911 *adc, int *val) > > +{ > > + int ret = mcp3911_read(adc, MCP3910_REG_CONFIG0, val, 3); > > Have you run checkpatch? Here should be a blank line. Same in other several > places. Yes, checkpatch does not report any warning for this. [...] > > + adc->chip = (struct mcp3911_chip_info *)spi_get_device_id(spi)->driver_data; > > Can't you use spi_get_device_match_data()? > > ... Will go for spi_get_device_match_data(). [...] > > > + device_property_read_u32(&adc->spi->dev, "device-addr", &adc->dev_addr); > > With > > struct device *dev = &adc->spi->dev; > > lines like this will be neater. Indeed, but I think I will short it down to just spi->dev. > > > Best regards, Marcus Folkesson
Attachment:
signature.asc
Description: PGP signature