Hi Gerhard,
...
> +static int ki2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
> +{
> + struct ki2c *ki2c = i2c_get_adapdata(adap);
> + int ret;
> +
> + ret = ki2c_inuse_lock(ki2c);
> + if (ret < 0)
> + return ret;
> +
> + for (int i = 0; i < num; i++) {
> + struct i2c_msg *m = &msgs[i];
> +
> + if (i == 0)
> + ret = ki2c_start_addr(ki2c, m);
do we want to keep the error if we fail to receive the data ack?
> + else
> + ret = ki2c_repstart_addr(ki2c, m);
> + if (ret < 0)
> + break;
> +
> + if (m->flags & I2C_M_RD)
> + ret = ki2c_read(ki2c, m->buf, m->len);
> + else
> + ret = ki2c_write(ki2c, m->buf, m->len);
> + if (ret < 0)
> + break;
> + }
> +
> + ki2c_stop(ki2c);
> +
> + ki2c_inuse_unlock(ki2c);
> +
> + return (ret < 0) ? ret : num;
> +}
> +
> +static void ki2c_unregister_devices(struct ki2c *ki2c)
> +{
> + for (int i = 0; i < ki2c->client_size; i++) {
> + struct i2c_client *client = ki2c->client[i];
> +
> + if (client)
> + i2c_unregister_device(client);
> + }
> +}
> +
> +static int ki2c_register_devices(struct ki2c *ki2c)
> +{
> + struct i2c_board_info *info = ki2c->auxdev->info;
> +
> + /* register all I2C devices from platform_data array */
> + for (int i = 0; i < ki2c->client_size; i++) {
Please declare the variable at the beginning of the function.
Did you run checkpatch.pl before sending?
> + struct i2c_client *client;
> + unsigned short const addr_list[2] = { info[i].addr,
> + I2C_CLIENT_END };
> +
> + client = i2c_new_scanned_device(&ki2c->adapter, &info[i],
> + addr_list, NULL);
> + if (!IS_ERR(client)) {
> + ki2c->client[i] = client;
> + } else if (PTR_ERR(client) != -ENODEV) {
> + ki2c_unregister_devices(ki2c);
> +
> + return PTR_ERR(client);
> + }
> + }
> +
> + return 0;
> +}
> +
> +static u32 ki2c_func(struct i2c_adapter *adap)
> +{
> + return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +}
> +
> +static const struct i2c_algorithm ki2c_algo = {
> + .master_xfer = ki2c_xfer,
> + .functionality = ki2c_func,
> +};
> +
> +static int ki2c_probe(struct auxiliary_device *auxdev,
> + const struct auxiliary_device_id *id)
> +{
> + struct device *dev = &auxdev->dev;
> + struct i2c_adapter *adap;
> + struct ki2c *ki2c;
> + int ret;
> +
> + ki2c = devm_kzalloc(dev, sizeof(*ki2c), GFP_KERNEL);
> + if (!ki2c)
> + return -ENOMEM;
> + ki2c->auxdev = container_of(auxdev, struct keba_i2c_auxdev, auxdev);
> + ki2c->client = devm_kcalloc(dev, ki2c->auxdev->info_size,
> + sizeof(*ki2c->client), GFP_KERNEL);
> + if (!ki2c->client)
> + return -ENOMEM;
> + ki2c->client_size = ki2c->auxdev->info_size;
> + auxiliary_set_drvdata(auxdev, ki2c);
> +
> + ki2c->base = devm_ioremap_resource(dev, &ki2c->auxdev->io);
> + if (IS_ERR(ki2c->base))
> + return PTR_ERR(ki2c->base);
> +
> + /* enable controller */
> + iowrite8(KI2C_CONTROL_MEN, ki2c->base + KI2C_CONTROL_REG);
Can we enable at the end? What's the point for enabling the
device so early? (We also save a few goto's)
> + adap = &ki2c->adapter;
> + strscpy(adap->name, "KEBA I2C adapter", sizeof(adap->name));
> + adap->owner = THIS_MODULE;
> + adap->class = I2C_CLASS_HWMON;
> + adap->algo = &ki2c_algo;
> + adap->dev.parent = dev;
> +
> + i2c_set_adapdata(adap, ki2c);
> +
> + /* reset bus before probing I2C devices */
> + ret = ki2c_reset_bus(ki2c);
> + if (ret)
> + goto out_disable;
> +
> + ret = i2c_add_adapter(adap);
Please use devm_i2c_add_adapter()
> + if (ret) {
> + dev_err(dev, "Failed to add adapter (%d)!\n", ret);
> + goto out_disable;
> + }
> +
> + ret = ki2c_register_devices(ki2c);
> + if (ret) {
> + dev_err(dev, "Failed to register devices (%d)!\n", ret);
> + goto out_delete;
> + }
> +
> + return 0;
> +
> +out_delete:
> + i2c_del_adapter(adap);
> +out_disable:
> + iowrite8(0, ki2c->base + KI2C_CONTROL_REG);
Can we define Value '0'? KI2C_CONTROL_DISABLE, perhaps?
Andi
> + return ret;
> +}
> +
> +static void ki2c_remove(struct auxiliary_device *auxdev)
> +{
> + struct ki2c *ki2c = auxiliary_get_drvdata(auxdev);
> +
> + ki2c_unregister_devices(ki2c);
> +
> + i2c_del_adapter(&ki2c->adapter);
> +
> + /* disable controller */
> + iowrite8(0, ki2c->base + KI2C_CONTROL_REG);
> +
> + auxiliary_set_drvdata(auxdev, NULL);
> +}
> +
> +static const struct auxiliary_device_id ki2c_devtype_aux[] = {
> + { .name = "keba.i2c" },
> + { },
> +};
> +MODULE_DEVICE_TABLE(auxiliary, ki2c_devtype_aux);
> +
> +static struct auxiliary_driver ki2c_driver_aux = {
> + .name = KI2C,
> + .id_table = ki2c_devtype_aux,
> + .probe = ki2c_probe,
> + .remove = ki2c_remove,
> +};
> +module_auxiliary_driver(ki2c_driver_aux);
> +
> +MODULE_AUTHOR("Gerhard Engleder <eg@xxxxxxxx>");
> +MODULE_DESCRIPTION("KEBA I2C bus controller driver");
> +MODULE_LICENSE("GPL");
> --
> 2.39.2
>
