On Tue, Nov 12, 2019 at 05:57:25PM +0000, Vijay Khemka wrote: > > > On 11/12/19, 4:36 AM, "Corey Minyard" <tcminy...@gmail.com on behalf of > miny...@acm.org> wrote: > > On Mon, Nov 11, 2019 at 06:36:09PM -0800, Vijay Khemka wrote: > > Many IPMB devices doesn't support smbus protocol and current driver > > support only smbus devices. So added support for raw i2c packets. > > I haven't reviewed this, really, because I have a more general > concern... > > Is it possible to not do this with a config item? Can you add something > to the device tree and/or via an ioctl to make this dynamically > configurable? That's more flexible (it can support mixed devices) and > is friendlier to users (don't have to get the config right). > I agree with you, I was also not comfortable using config and couldn't find > other > Options, I will look into more option now and update patch.
IMHO, device tree is the right way to do this. You should also have a sysfs setting for this, I think. -corey > > Config items for adding new functionality are generally ok. Config > items for choosing between two mutually exclusive choices are > generally not. > > -corey > > > > > Signed-off-by: Vijay Khemka <vijaykhe...@fb.com> > > --- > > drivers/char/ipmi/Kconfig | 6 ++++++ > > drivers/char/ipmi/ipmb_dev_int.c | 30 ++++++++++++++++++++++++++++++ > > 2 files changed, 36 insertions(+) > > > > diff --git a/drivers/char/ipmi/Kconfig b/drivers/char/ipmi/Kconfig > > index a9cfe4c05e64..e5268443b478 100644 > > --- a/drivers/char/ipmi/Kconfig > > +++ b/drivers/char/ipmi/Kconfig > > @@ -139,3 +139,9 @@ config IPMB_DEVICE_INTERFACE > > Provides a driver for a device (Satellite MC) to > > receive requests and send responses back to the BMC via > > the IPMB interface. This module requires I2C support. > > + > > +config IPMB_SMBUS_DISABLE > > + bool 'Disable SMBUS protocol for sending packet to IPMB device' > > + depends on IPMB_DEVICE_INTERFACE > > + help > > + provides functionality of sending raw i2c packets to IPMB > device. > > diff --git a/drivers/char/ipmi/ipmb_dev_int.c > b/drivers/char/ipmi/ipmb_dev_int.c > > index ae3bfba27526..2419b9a928b2 100644 > > --- a/drivers/char/ipmi/ipmb_dev_int.c > > +++ b/drivers/char/ipmi/ipmb_dev_int.c > > @@ -118,6 +118,10 @@ static ssize_t ipmb_write(struct file *file, const > char __user *buf, > > struct ipmb_dev *ipmb_dev = to_ipmb_dev(file); > > u8 rq_sa, netf_rq_lun, msg_len; > > union i2c_smbus_data data; > > +#ifdef CONFIG_IPMB_SMBUS_DISABLE > > + unsigned char *i2c_buf; > > + struct i2c_msg i2c_msg; > > +#endif > > u8 msg[MAX_MSG_LEN]; > > ssize_t ret; > > > > @@ -133,6 +137,31 @@ static ssize_t ipmb_write(struct file *file, const > char __user *buf, > > rq_sa = GET_7BIT_ADDR(msg[RQ_SA_8BIT_IDX]); > > netf_rq_lun = msg[NETFN_LUN_IDX]; > > > > +#ifdef CONFIG_IPMB_SMBUS_DISABLE > > + /* > > + * subtract 1 byte (rq_sa) from the length of the msg passed to > > + * raw i2c_transfer > > + */ > > + msg_len = msg[IPMB_MSG_LEN_IDX] - 1; > > + > > + i2c_buf = kzalloc(msg_len, GFP_KERNEL); > > + if (!i2c_buf) > > + return -EFAULT; > > + > > + /* Copy message to buffer except first 2 bytes (length and > address) */ > > + memcpy(i2c_buf, msg+2, msg_len); > > + > > + i2c_msg.addr = rq_sa; > > + i2c_msg.flags = ipmb_dev->client->flags & > > + (I2C_M_TEN | I2C_CLIENT_PEC | I2C_CLIENT_SCCB); > > + i2c_msg.len = msg_len; > > + i2c_msg.buf = i2c_buf; > > + > > + ret = i2c_transfer(ipmb_dev->client->adapter, &i2c_msg, 1); > > + kfree(i2c_buf); > > + > > + return (ret == 1) ? count : ret; > > +#else > > /* > > * subtract rq_sa and netf_rq_lun from the length of the msg > passed to > > * i2c_smbus_xfer > > @@ -149,6 +178,7 @@ static ssize_t ipmb_write(struct file *file, const > char __user *buf, > > I2C_SMBUS_BLOCK_DATA, &data); > > > > return ret ? : count; > > +#endif > > } > > > > static unsigned int ipmb_poll(struct file *file, poll_table *wait) > > -- > > 2.17.1 > > > > _______________________________________________ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer