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()
> +
> + dev_dbg(&dev->spi->dev, "i2c clock frequency: %08x", reg);
> + regmap_write(dev->regmap, SC18IS600_REG_I2C_CLOCK, reg);
> +}
> +static void sc18is600_setup_timeout(struct sc18is600dev *dev,
> + bool enable, int timeout_ms)
> +{
> + int timeout = DIV_ROUND_UP(timeout_ms * dev->chip->timeout_base, 10000);
> + u8 reg;
> +
> + if (timeout <= 0)
> + timeout = 1;
> + if (timeout > 255)
> + timeout = 255;
Ditto.
> +
> + reg = (timeout & 0x7F) << 1;
> + reg |= (!!enable);
Redundant parens.
Might be one line.
> +
> + dev_dbg(&dev->spi->dev, "i2c timeout: %08x", reg);
> + regmap_write(dev->regmap, SC18IS600_REG_I2C_TIMEOUT, reg);
> +}
> +static int sc18is600_xfer(struct i2c_adapter *adapter,
> + struct i2c_msg *msgs, int num)
> +{
> + if (num == 1 && read_operations == 1)
> + err = sc18is600_read(dev, &msgs[0]);
> + else if (num == 1)
> + err = sc18is600_write(dev, &msgs[0]);
> + else if (num == 2 && read_operations == 1)
> + err = sc18is600_read_after_write(dev, &msgs[0], &msgs[1]);
> + else if (num == 2)
> + err = sc18is600_write_after_write(dev, &msgs[0], &msgs[1]);
> + else
> + return -EOPNOTSUPP;
It will look better
if (x == 1) {
if (y)
else
} else if (x == 2) {
if (y)
else
}
> + switch (dev->state) {
> + case SC18IS600_STAT_OK:
> + break;
> + case SC18IS600_STAT_NAK_ADDR:
> + return -EIO;
> + case SC18IS600_STAT_NAK_DATA:
> + return -EREMOTEIO;
> + case SC18IS600_STAT_SIZE:
> + return -EINVAL;
> + case SC18IS600_STAT_TIMEOUT:
> + return -ETIMEDOUT;
> + case SC18IS600_STAT_TIMEOUT2:
> + return -ETIMEDOUT;
> + case SC18IS600_STAT_BLOCKED:
> + return -ETIMEDOUT;
You may use
case X:
case Y:
return Z;
> + default:
> + case SC18IS600_STAT_BUSY:
> + dev_err(&dev->spi->dev, "device hangup detected, reset!");
> + sc18is600_reset(dev);
> + return -EAGAIN;
> + }
> +static int sc18is600_probe(struct spi_device *spi)
> +{
> + const struct of_device_id *of_id;
> + struct sc18is600dev *dev;
> + int err;
> +
> + of_id = of_match_device(sc18is600_of_match, &spi->dev);
> + if (!of_id)
> + return -ENODEV;
of_device_get_match_data() ?
> + dev = devm_kzalloc(&spi->dev, sizeof(*dev), GFP_KERNEL);
> + if (dev == NULL)
if (!dev)
...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.
> + spi->master->bus_num, spi->chip_select);
> + if (!dev->chip->clock_base) {
> + dev->clk = devm_clk_get(&spi->dev, "clkin");
> + if (IS_ERR(dev->clk)) {
> + err = PTR_ERR(dev->clk);
> + dev_err(&spi->dev, "could not acquire vdd: %d\n", err);
vdd-> Vdd ?
> + return err;
> + }
> +
> + clk_prepare_enable(dev->clk);
This might fail.
> + dev->clock_base = clk_get_rate(dev->clk) / 4;
Magic.
> + } else {
> + dev->clock_base = dev->chip->clock_base;
> + }
--
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
