Hi Andy, Thanks for your feedback. On Sat, Jun 17, 2017 at 09:13:32PM +0300, Andy Shevchenko wrote: > On Tue, Jun 13, 2017 at 6:47 PM, Sebastian Reichel <sre@xxxxxxxxxx> wrote: > > This adds an I²C master driver for SPI -> I²C bus bridge chips. > > It currently supports NXP's SC18IS600 and SC18IS601, as well as > > Silicon Labs' CP2120. The driver was only tested on SC18IS600. > > > +static void sc18is600_setup_clock_frequency(struct sc18is600dev *dev) > > +{ > > + int reg = DIV_ROUND_UP(dev->clock_base, dev->i2c_clock_frequency); > > + > > > + if (reg < 5) > > + reg = 5; > > + if (reg > 255) > > + reg = 255; > > clamp_t() I will use clamp_val() > [...] > lot's of suggestions for improvements. > [...] Ack. > ...and better to use s600dev or alike to avoid confusion. > > > + return -ENOMEM; > > > + snprintf(dev->adapter.name, sizeof(dev->adapter.name), > > > + "SC18IS600 at SPI %02d device %02d", > > Isn't too much for adapter name? > I don't remember if it's part of ABI, in that case it's even worse. Well, it's displayed by "i2cdetect -l". Here are the names from some other drivers, which I used as reference: i2c-tiny-usb at bus %03d device %03d i2c-diolan-u2c at bus %03d device %03d OSIF at bus %03d device %03d SMBus Via Pro adapter at %04x SMBus SIS5595 adapter at %04x > [...] > lot's of suggestions for improvements. > [...] Ack. -- Sebastian
Attachment:
signature.asc
Description: PGP signature