On Thu, May 30, 2019 at 09:33:46PM -0700, Eduardo Valentin wrote:
> From: Haiyue Wang <haiyue.wang@xxxxxxxxxxxxxxx>
>
> Some protocols over I2C are designed for bi-directional transferring
> messages by using I2C Master Write protocol. Like the MCTP (Management
> Component Transport Protocol) and IPMB (Intelligent Platform Management
> Bus), they both require that the userspace can receive messages from
> I2C dirvers under slave mode.
>
> This new slave mqueue backend is used to receive and queue messages, it
> will exposes these messages to userspace by sysfs bin file.
>
> Note: DT interface and a couple of minor fixes here and there
> by Eduardo, so I kept the original authorship here.
> +#define MQ_MSGBUF_SIZE CONFIG_I2C_SLAVE_MQUEUE_MESSAGE_SIZE
> +#define MQ_QUEUE_SIZE CONFIG_I2C_SLAVE_MQUEUE_QUEUE_SIZE
> +#define MQ_QUEUE_NEXT(x) (((x) + 1) & (MQ_QUEUE_SIZE - 1))
Also possible ((x + 1) % ..._SIZE)
> + mq = dev_get_drvdata(container_of(kobj, struct device, kobj));
kobj_to_dev()
> +static int i2c_slave_mqueue_probe(struct i2c_client *client,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &client->dev;
> + struct mq_queue *mq;
> + int ret, i;
> + void *buf;
> +
> + mq = devm_kzalloc(dev, sizeof(*mq), GFP_KERNEL);
> + if (!mq)
> + return -ENOMEM;
> +
> + BUILD_BUG_ON(!is_power_of_2(MQ_QUEUE_SIZE));
Perhaps start function with this kind of assertions?
> +
> + buf = devm_kmalloc_array(dev, MQ_QUEUE_SIZE, MQ_MSGBUF_SIZE,
> + GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + for (i = 0; i < MQ_QUEUE_SIZE; i++)
> + mq->queue[i].buf = buf + i * MQ_MSGBUF_SIZE;
Just wondering if kfifo API can bring an advantage here?
> + return 0;
> +}
> +static const struct of_device_id i2c_slave_mqueue_of_match[] = {
> + {
> + .compatible = "i2c-slave-mqueue",
> + },
> + { },
No need for comma here.
> +};
> +
> +static struct i2c_driver i2c_slave_mqueue_driver = {
> + .driver = {
> + .name = "i2c-slave-mqueue",
> + .of_match_table = of_match_ptr(i2c_slave_mqueue_of_match),
Wouldn't compiler warn you due to unused data?
Perhaps drop of_match_ptr() for good...
> + },
> + .probe = i2c_slave_mqueue_probe,
> + .remove = i2c_slave_mqueue_remove,
> + .id_table = i2c_slave_mqueue_id,
> +};
--
With Best Regards,
Andy Shevchenko
