| Message ID | 20230312190435.3568212-2-xiang.ye@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add Intel LJCA device driver | expand |
On Mon, Mar 13, 2023 at 03:04:31AM +0800, Ye Xiang wrote: > This patch implements the USB part of Intel USB-I2C/GPIO/SPI adapter > device named "La Jolla Cove Adapter" (LJCA). > > The communication between the various LJCA module drivers and the > hardware will be muxed/demuxed by this driver. The sub-module of > LJCA can use ljca_transfer() to issue a transfer between host > and hardware. So you have 2 different things happening in this driver. One is the USB interaction and control and stuff, and one is the creation of an api that is to be used by other parts of the kernel. Can you split this up into 2 different commits, one for the api, and one for the USB stuff? I think you will find that the API is going to need a bunch of work, as it's not "normal" for what the kernel is expecting to have. Some other review comments below: > +struct ljca_event_cb_entry { > + ljca_event_cb_t notify; > + void *context; Why a void *? > +}; > + > +struct ljca_dev { > + struct usb_device *udev; > + struct usb_interface *intf; If you have the interface, you can get the usb device. Why store both? > + u8 in_ep; /* the address of the bulk in endpoint */ > + u8 out_ep; /* the address of the bulk out endpoint */ > + > + /* the urb/buffer for read */ > + struct urb *in_urb; > + unsigned char *ibuf; > + size_t ibuf_len; > + > + bool started; You can't just use a boolean as a "flag" without any locking, that is not going to work, sorry. > + struct list_head stubs_list; > + > + /* to wait for an ongoing write ack */ > + wait_queue_head_t ack_wq; > + > + struct mfd_cell *cells; > + int cell_count; > + /* mutex to protect package transfer with LJCA device */ > + struct mutex mutex; Why is this not protecting "started" as well as the other fields in this structure? > +}; > + > +struct ljca { > + u8 type; > + struct ljca_dev *dev; > +}; > + > +struct ljca_stub_packet { > + unsigned int *ibuf_len; > + u8 *ibuf; > +}; > + > +struct ljca_stub { > + struct list_head list; > + struct usb_interface *intf; > + struct ljca_stub_packet ipacket; > + u8 type; > + > + /* for identify ack */ > + bool acked; > + int cur_cmd; > + > + struct ljca_event_cb_entry event_entry; > + /* lock to protect event_entry */ > + spinlock_t event_cb_lock; > + > + struct ljca ljca; > + unsigned long priv[]; What is "priv" for? Why is it unsigned long and not a real type? > +}; > + > +static inline void *ljca_priv(struct ljca_stub *stub) > +{ > + return stub->priv; Why is this a void *? > +} > + > +static bool ljca_validate(struct ljca_msg *header, u32 data_len) > +{ > + return header->len + sizeof(*header) == data_len; > +} > + > +static struct ljca_stub *ljca_stub_alloc(struct ljca_dev *dev, u8 type, int priv_size) > +{ > + struct ljca_stub *stub; > + > + stub = kzalloc(struct_size(stub, priv, priv_size), GFP_KERNEL); > + if (!stub) > + return ERR_PTR(-ENOMEM); > + > + stub->type = type; > + stub->intf = dev->intf; > + stub->ljca.dev = dev; You are saving a reference to a reference counted device, yet never grabbing the reference? That is ripe for disaster. > + stub->ljca.type = stub->type; > + spin_lock_init(&stub->event_cb_lock); > + list_add_tail(&stub->list, &dev->stubs_list); Where is the reference counting on this new structure that you just created? Who controls the lifespan of it? > + return stub; > +} > + > +static struct ljca_stub *ljca_stub_find(struct ljca_dev *dev, u8 type) > +{ > + struct ljca_stub *stub; > + > + list_for_each_entry(stub, &dev->stubs_list, list) { > + if (stub->type == type) > + return stub; > + } > + > + dev_err(&dev->intf->dev, "USB stub not found, type:%d\n", type); > + > + return ERR_PTR(-ENODEV); > +} > + > +static void ljca_stub_notify(struct ljca_stub *stub, u8 cmd, const void *evt_data, int len) > +{ > + unsigned long flags; > + > + spin_lock_irqsave(&stub->event_cb_lock, flags); > + if (stub->event_entry.notify && stub->event_entry.context) > + stub->event_entry.notify(stub->event_entry.context, cmd, evt_data, len); > + spin_unlock_irqrestore(&stub->event_cb_lock, flags); > +} > + > +static int ljca_parse(struct ljca_dev *dev, struct ljca_msg *header) > +{ > + struct ljca_stub *stub; > + > + stub = ljca_stub_find(dev, header->type); > + if (IS_ERR(stub)) > + return PTR_ERR(stub); > + > + if (!(header->flags & LJCA_ACK_FLAG)) { > + ljca_stub_notify(stub, header->cmd, header->data, header->len); > + return 0; > + } > + > + if (stub->cur_cmd != header->cmd) { > + dev_err(&dev->intf->dev, "header and stub current command mismatch (%x vs %x)\n", > + header->cmd, stub->cur_cmd); > + return -EINVAL; > + } > + > + if (stub->ipacket.ibuf && stub->ipacket.ibuf_len) { > + unsigned int newlen; > + > + newlen = min_t(unsigned int, header->len, *stub->ipacket.ibuf_len); > + > + *stub->ipacket.ibuf_len = newlen; > + memcpy(stub->ipacket.ibuf, header->data, newlen); > + } > + > + stub->acked = true; > + wake_up(&dev->ack_wq); > + > + return 0; > +} > + > +static int ljca_stub_write(struct ljca_stub *stub, u8 cmd, const void *obuf, unsigned int obuf_len, Why is obuf a void *? It's a real structure (or u8 stream), make it so. > + void *ibuf, unsigned int *ibuf_len, bool wait_ack, unsigned long timeout) Same for ibuf. > +{ > + struct ljca_dev *dev = usb_get_intfdata(stub->intf); > + u8 flags = LJCA_CMPL_FLAG; > + struct ljca_msg *header; > + unsigned int msg_len = sizeof(*header) + obuf_len; > + int actual; > + int ret; > + > + if (msg_len > LJCA_MAX_PACKET_SIZE) > + return -EINVAL; > + > + if (wait_ack) > + flags |= LJCA_ACK_FLAG; > + > + header = kmalloc(msg_len, GFP_KERNEL); > + if (!header) > + return -ENOMEM; > + > + header->type = stub->type; > + header->cmd = cmd; > + header->flags = flags; > + header->len = obuf_len; > + > + if (obuf) > + memcpy(header->data, obuf, obuf_len); > + > + dev_dbg(&dev->intf->dev, "send: type:%d cmd:%d flags:%d len:%d\n", header->type, > + header->cmd, header->flags, header->len); > + > + usb_autopm_get_interface(dev->intf); > + if (!dev->started) { > + kfree(header); > + ret = -ENODEV; > + goto error_put; > + } > + > + mutex_lock(&dev->mutex); > + stub->cur_cmd = cmd; > + stub->ipacket.ibuf = ibuf; > + stub->ipacket.ibuf_len = ibuf_len; > + stub->acked = false; > + ret = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, dev->out_ep), header, msg_len, > + &actual, LJCA_USB_WRITE_TIMEOUT_MS); > + kfree(header); > + if (ret) { > + dev_err(&dev->intf->dev, "bridge write failed ret:%d\n", ret); > + goto error_unlock; > + } > + > + if (actual != msg_len) { > + dev_err(&dev->intf->dev, "bridge write length mismatch (%d vs %d)\n", msg_len, > + actual); > + ret = -EINVAL; > + goto error_unlock; > + } > + > + if (wait_ack) { > + ret = wait_event_timeout(dev->ack_wq, stub->acked, msecs_to_jiffies(timeout)); > + if (!ret) { > + dev_err(&dev->intf->dev, "acked wait timeout\n"); > + ret = -ETIMEDOUT; > + goto error_unlock; > + } > + } > + > + stub->ipacket.ibuf = NULL; > + stub->ipacket.ibuf_len = NULL; > + ret = 0; > +error_unlock: > + mutex_unlock(&dev->mutex); > +error_put: > + usb_autopm_put_interface(dev->intf); > + > + return ret; > +} > + > +static int ljca_transfer_internal(struct ljca *ljca, u8 cmd, const void *obuf, > + unsigned int obuf_len, void *ibuf, unsigned int *ibuf_len, > + bool wait_ack) > +{ > + struct ljca_stub *stub; > + > + stub = ljca_stub_find(ljca->dev, ljca->type); > + if (IS_ERR(stub)) > + return PTR_ERR(stub); > + > + return ljca_stub_write(stub, cmd, obuf, obuf_len, ibuf, ibuf_len, wait_ack, > + LJCA_USB_WRITE_ACK_TIMEOUT_MS); > +} > + > +int ljca_transfer(struct ljca *ljca, u8 cmd, const void *obuf, unsigned int obuf_len, void *ibuf, > + unsigned int *ibuf_len) > +{ > + return ljca_transfer_internal(ljca, cmd, obuf, obuf_len, ibuf, ibuf_len, true); > +} > +EXPORT_SYMBOL_NS_GPL(ljca_transfer, LJCA); > + > +int ljca_transfer_noack(struct ljca *ljca, u8 cmd, const void *obuf, unsigned int obuf_len) > +{ > + return ljca_transfer_internal(ljca, cmd, obuf, obuf_len, NULL, NULL, false); > +} > +EXPORT_SYMBOL_NS_GPL(ljca_transfer_noack, LJCA); > + > +int ljca_register_event_cb(struct ljca *ljca, ljca_event_cb_t event_cb, void *context) What are these magic events you are registering? What do they do and why would anyone need them? You have global functions here that other drivers are using, yet no documentation about them at all for any way to review that the api really is doing what you are wanting it to do. So again, please split this up into at least 2 changes, and document this new api you are creating, so that we have a chance to review it properly. Otherwise it's almost impossible to do so. thanks, greg k-h
Hi Ye, On top of what Greg has already said, few things from my side through the lines. [...] > +static int ljca_mng_link(struct ljca_dev *dev, struct ljca_stub *stub) > +{ > + int ret; > + > + ret = ljca_mng_reset_handshake(stub); > + if (ret) > + return ret; > + > + /* try to enum all the stubs */ > + ljca_mng_enum_gpio(stub); > + ljca_mng_enum_i2c(stub); > + ljca_mng_enum_spi(stub); We are ignoring here the return value. So either make the whole function call chain to be void or please check the return values here. [...] > +static ssize_t ljca_enable_dfu_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct usb_interface *intf = to_usb_interface(dev); > + struct ljca_dev *ljca_dev = usb_get_intfdata(intf); > + struct ljca_stub *mng_stub = ljca_stub_find(ljca_dev, LJCA_MNG_STUB); > + bool enable; > + int ret; > + > + ret = kstrtobool(buf, &enable); > + if (ret) > + return ret; > + > + if (enable) { > + ret = ljca_mng_set_dfu_mode(mng_stub); > + if (ret) > + return ret; > + } What is the DFU mode? Is it an operational mode? Do we enter and exit from it? Does the device leave this mode on its own? What if I write twice in a raw enable? Can I check if I am in DFU mode or not? Would you mind adding some comments here? > + > + return count; > +} > +static DEVICE_ATTR_WO(ljca_enable_dfu); > + > +static ssize_t ljca_trace_level_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct usb_interface *intf = to_usb_interface(dev); > + struct ljca_dev *ljca_dev = usb_get_intfdata(intf); > + struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB); > + u8 level; > + int ret; > + > + if (kstrtou8(buf, 0, &level)) > + return -EINVAL; > + > + ret = ljca_diag_set_trace_level(diag_stub, level); > + if (ret) > + return ret; do we need any range check for the levels? What happens if I do: echo "I am too cool" > /sys/.../ljca_trace_level As there were questions here, would you mind adding some comments so that the next reader won't make the same questions? > + > + return count; > +} > +static DEVICE_ATTR_WO(ljca_trace_level); [...] > +static int ljca_probe(struct usb_interface *intf, const struct usb_device_id *id) > +{ > + struct ljca_dev *dev; > + struct usb_endpoint_descriptor *bulk_in, *bulk_out; > + int ret; > + > + /* allocate memory for our device state and initialize it */ > + dev = kzalloc(sizeof(*dev), GFP_KERNEL); devm_kzalloc() Thanks, Andi > + if (!dev) > + return -ENOMEM;
On Mon, 13 Mar 2023, Ye Xiang wrote: > This patch implements the USB part of Intel USB-I2C/GPIO/SPI adapter > device named "La Jolla Cove Adapter" (LJCA). > > The communication between the various LJCA module drivers and the > hardware will be muxed/demuxed by this driver. The sub-module of > LJCA can use ljca_transfer() to issue a transfer between host > and hardware. > > Each sub-module of LJCA device is identified by type field within > the LJCA message header. > > The minimum code in ASL that covers this board is > Scope (\_SB.PCI0.DWC3.RHUB.HS01) > { > Device (GPIO) > { > Name (_ADR, Zero) > Name (_STA, 0x0F) > } > > Device (I2C) > { > Name (_ADR, One) > Name (_STA, 0x0F) > } > > Device (SPI) > { > Name (_ADR, 0x02) > Name (_STA, 0x0F) > } > } > > Signed-off-by: Ye Xiang <xiang.ye@intel.com> > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > --- > drivers/usb/misc/Kconfig | 13 + > drivers/usb/misc/Makefile | 1 + > drivers/usb/misc/ljca.c | 998 ++++++++++++++++++++++++++++++++++++++ > include/linux/usb/ljca.h | 95 ++++ > 4 files changed, 1107 insertions(+) > create mode 100644 drivers/usb/misc/ljca.c > create mode 100644 include/linux/usb/ljca.h > > diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig > index a5f7652db7da..59ec120c26d4 100644 > --- a/drivers/usb/misc/Kconfig > +++ b/drivers/usb/misc/Kconfig > @@ -273,6 +273,19 @@ config USB_LINK_LAYER_TEST > Layer Test Device. Say Y only when you want to conduct USB Super Speed > Link Layer Test for host controllers. > > +config USB_LJCA > + tristate "Intel La Jolla Cove Adapter support" > + select MFD_CORE > + depends on USB > + help > + This adds support for Intel La Jolla Cove USB-I2C/SPI/GPIO > + Master Adapter (LJCA). Additional drivers such as I2C_LJCA, > + GPIO_LJCA and SPI_LJCA must be enabled in order to use the > + functionality of the device. > + > + This driver can also be built as a module. If so, the module > + will be called ljca. > + > config USB_CHAOSKEY > tristate "ChaosKey random number generator driver support" > depends on HW_RANDOM > diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile > index 93581baec3a8..6f6adfbe17e0 100644 > --- a/drivers/usb/misc/Makefile > +++ b/drivers/usb/misc/Makefile > @@ -29,6 +29,7 @@ obj-$(CONFIG_USB_HUB_USB251XB) += usb251xb.o > obj-$(CONFIG_USB_HSIC_USB3503) += usb3503.o > obj-$(CONFIG_USB_HSIC_USB4604) += usb4604.o > obj-$(CONFIG_USB_CHAOSKEY) += chaoskey.o > +obj-$(CONFIG_USB_LJCA) += ljca.o > > obj-$(CONFIG_USB_SISUSBVGA) += sisusbvga/ > obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o > diff --git a/drivers/usb/misc/ljca.c b/drivers/usb/misc/ljca.c > new file mode 100644 > index 000000000000..ab98deaf0074 > --- /dev/null > +++ b/drivers/usb/misc/ljca.c > @@ -0,0 +1,998 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Intel La Jolla Cove Adapter USB driver > + * > + * Copyright (c) 2023, Intel Corporation. > + */ > + > +#include <linux/dev_printk.h> > +#include <linux/kernel.h> > +#include <linux/mfd/core.h> Please don't use the MFD API outside of drivers/mfd. If you wish to use the API, please do. Strip out (only) the MFD parts and move them into drivers/mfd. > +#include <linux/module.h> > +#include <linux/mod_devicetable.h> > +#include <linux/mutex.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/types.h> > +#include <linux/usb.h> > +#include <linux/usb/ljca.h> -- Lee Jones [李琼斯]
Hi Greq, Thanks for the review. On Mon, Mar 13, 2023 at 07:56:50AM +0100, Greg Kroah-Hartman wrote: > On Mon, Mar 13, 2023 at 03:04:31AM +0800, Ye Xiang wrote: > > This patch implements the USB part of Intel USB-I2C/GPIO/SPI adapter > > device named "La Jolla Cove Adapter" (LJCA). > > > > The communication between the various LJCA module drivers and the > > hardware will be muxed/demuxed by this driver. The sub-module of > > LJCA can use ljca_transfer() to issue a transfer between host > > and hardware. > > So you have 2 different things happening in this driver. One is the USB > interaction and control and stuff, and one is the creation of an api > that is to be used by other parts of the kernel. > > Can you split this up into 2 different commits, one for the api, and one > for the USB stuff? I think you will find that the API is going to need > a bunch of work, as it's not "normal" for what the kernel is expecting > to have. Thanks. I will split this into to commits on v6. > > Some other review comments below: > > > +struct ljca_event_cb_entry { > > + ljca_event_cb_t notify; > > + void *context; > > Why a void *? We use `void *` here because we don't know the data type of context on this driver. The `context` here is the first parameter of `notify` callback. And the `notify` callback is registered by sub-modules. So it represents the execution context of whom registered this callback and the type of it is determined by who registered the callback. > > > +}; > > + > > +struct ljca_dev { > > + struct usb_device *udev; > > + struct usb_interface *intf; > > If you have the interface, you can get the usb device. Why store both? okay, I can just store intf here. When udev are needed when submitting a urb, `interface_to_usbdev` can be used to get the udev. > > > + u8 in_ep; /* the address of the bulk in endpoint */ > > + u8 out_ep; /* the address of the bulk out endpoint */ > > + > > + /* the urb/buffer for read */ > > + struct urb *in_urb; > > + unsigned char *ibuf; > > + size_t ibuf_len; > > + > > + bool started; > > You can't just use a boolean as a "flag" without any locking, that is > not going to work, sorry. ok, I will use a mutex to protect it. > > > + struct list_head stubs_list; > > + > > + /* to wait for an ongoing write ack */ > > + wait_queue_head_t ack_wq; > > + > > + struct mfd_cell *cells; > > + int cell_count; > > + /* mutex to protect package transfer with LJCA device */ > > + struct mutex mutex; > > Why is this not protecting "started" as well as the other fields in this > structure? Ok, will use it to protect "started", and other fields if needed. > > > +}; > > + > > +struct ljca { > > + u8 type; > > + struct ljca_dev *dev; > > +}; > > + > > +struct ljca_stub_packet { > > + unsigned int *ibuf_len; > > + u8 *ibuf; > > +}; > > + > > +struct ljca_stub { > > + struct list_head list; > > + struct usb_interface *intf; > > + struct ljca_stub_packet ipacket; > > + u8 type; > > + > > + /* for identify ack */ > > + bool acked; > > + int cur_cmd; > > + > > + struct ljca_event_cb_entry event_entry; > > + /* lock to protect event_entry */ > > + spinlock_t event_cb_lock; > > + > > + struct ljca ljca; > > + unsigned long priv[]; > > What is "priv" for? Why is it unsigned long and not a real type? The priv type is different for each stub. So it can be several types and we use unsigned long type instead. > > > +}; > > + > > +static inline void *ljca_priv(struct ljca_stub *stub) > > +{ > > + return stub->priv; > > Why is this a void *? return void here can save one type casting when using `priv = ljca_priv(stub);` > > > > +} > > + > > +static bool ljca_validate(struct ljca_msg *header, u32 data_len) > > +{ > > + return header->len + sizeof(*header) == data_len; > > +} > > + > > +static struct ljca_stub *ljca_stub_alloc(struct ljca_dev *dev, u8 type, int priv_size) > > +{ > > + struct ljca_stub *stub; > > + > > + stub = kzalloc(struct_size(stub, priv, priv_size), GFP_KERNEL); > > + if (!stub) > > + return ERR_PTR(-ENOMEM); > > + > > + stub->type = type; > > + stub->intf = dev->intf; > > + stub->ljca.dev = dev; > > You are saving a reference to a reference counted device, yet never > grabbing the reference? That is ripe for disaster. ok, will use usb_get_intf to increment reference count. > > > + stub->ljca.type = stub->type; > > + spin_lock_init(&stub->event_cb_lock); > > + list_add_tail(&stub->list, &dev->stubs_list); > > Where is the reference counting on this new structure that you just > created? Who controls the lifespan of it? We didn't use reference counting for this `struct ljca_stub` structure. The stubs will be destroyed in ljca_stub_cleanup. So The life span of stubs is from create to ljca_disconnect->ljca_stub_cleanup. > > > + return stub; > > +} > > + > > +static struct ljca_stub *ljca_stub_find(struct ljca_dev *dev, u8 type) > > +{ > > + struct ljca_stub *stub; > > + > > + list_for_each_entry(stub, &dev->stubs_list, list) { > > + if (stub->type == type) > > + return stub; > > + } > > + > > + dev_err(&dev->intf->dev, "USB stub not found, type:%d\n", type); > > + > > + return ERR_PTR(-ENODEV); > > +} > > + > > +static void ljca_stub_notify(struct ljca_stub *stub, u8 cmd, const void *evt_data, int len) > > +{ > > + unsigned long flags; > > + > > + spin_lock_irqsave(&stub->event_cb_lock, flags); > > + if (stub->event_entry.notify && stub->event_entry.context) > > + stub->event_entry.notify(stub->event_entry.context, cmd, evt_data, len); > > + spin_unlock_irqrestore(&stub->event_cb_lock, flags); > > +} > > + > > +static int ljca_parse(struct ljca_dev *dev, struct ljca_msg *header) > > +{ > > + struct ljca_stub *stub; > > + > > + stub = ljca_stub_find(dev, header->type); > > + if (IS_ERR(stub)) > > + return PTR_ERR(stub); > > + > > + if (!(header->flags & LJCA_ACK_FLAG)) { > > + ljca_stub_notify(stub, header->cmd, header->data, header->len); > > + return 0; > > + } > > + > > + if (stub->cur_cmd != header->cmd) { > > + dev_err(&dev->intf->dev, "header and stub current command mismatch (%x vs %x)\n", > > + header->cmd, stub->cur_cmd); > > + return -EINVAL; > > + } > > + > > + if (stub->ipacket.ibuf && stub->ipacket.ibuf_len) { > > + unsigned int newlen; > > + > > + newlen = min_t(unsigned int, header->len, *stub->ipacket.ibuf_len); > > + > > + *stub->ipacket.ibuf_len = newlen; > > + memcpy(stub->ipacket.ibuf, header->data, newlen); > > + } > > + > > + stub->acked = true; > > + wake_up(&dev->ack_wq); > > + > > + return 0; > > +} > > + > > +static int ljca_stub_write(struct ljca_stub *stub, u8 cmd, const void *obuf, unsigned int obuf_len, > > Why is obuf a void *? It's a real structure (or u8 stream), make it so. obuf and ibuf here is the transfer payload of LJCA. It has several types for different packages (such as gpio package, i2c package andspi package). If change the type to u8, we need to add a type casting from real package type to u8 everywhere when calling ljca_transfer. > > > + void *ibuf, unsigned int *ibuf_len, bool wait_ack, unsigned long timeout) > > Same for ibuf. > > > +{ > > + struct ljca_dev *dev = usb_get_intfdata(stub->intf); > > + u8 flags = LJCA_CMPL_FLAG; > > + struct ljca_msg *header; > > + unsigned int msg_len = sizeof(*header) + obuf_len; > > + int actual; > > + int ret; > > + > > + if (msg_len > LJCA_MAX_PACKET_SIZE) > > + return -EINVAL; > > + > > + if (wait_ack) > > + flags |= LJCA_ACK_FLAG; > > + > > + header = kmalloc(msg_len, GFP_KERNEL); > > + if (!header) > > + return -ENOMEM; > > + > > + header->type = stub->type; > > + header->cmd = cmd; > > + header->flags = flags; > > + header->len = obuf_len; > > + > > + if (obuf) > > + memcpy(header->data, obuf, obuf_len); > > + > > + dev_dbg(&dev->intf->dev, "send: type:%d cmd:%d flags:%d len:%d\n", header->type, > > + header->cmd, header->flags, header->len); > > + > > + usb_autopm_get_interface(dev->intf); > > + if (!dev->started) { > > + kfree(header); > > + ret = -ENODEV; > > + goto error_put; > > + } > > + > > + mutex_lock(&dev->mutex); > > + stub->cur_cmd = cmd; > > + stub->ipacket.ibuf = ibuf; > > + stub->ipacket.ibuf_len = ibuf_len; > > + stub->acked = false; > > + ret = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, dev->out_ep), header, msg_len, > > + &actual, LJCA_USB_WRITE_TIMEOUT_MS); > > + kfree(header); > > + if (ret) { > > + dev_err(&dev->intf->dev, "bridge write failed ret:%d\n", ret); > > + goto error_unlock; > > + } > > + > > + if (actual != msg_len) { > > + dev_err(&dev->intf->dev, "bridge write length mismatch (%d vs %d)\n", msg_len, > > + actual); > > + ret = -EINVAL; > > + goto error_unlock; > > + } > > + > > + if (wait_ack) { > > + ret = wait_event_timeout(dev->ack_wq, stub->acked, msecs_to_jiffies(timeout)); > > + if (!ret) { > > + dev_err(&dev->intf->dev, "acked wait timeout\n"); > > + ret = -ETIMEDOUT; > > + goto error_unlock; > > + } > > + } > > + > > + stub->ipacket.ibuf = NULL; > > + stub->ipacket.ibuf_len = NULL; > > + ret = 0; > > +error_unlock: > > + mutex_unlock(&dev->mutex); > > +error_put: > > + usb_autopm_put_interface(dev->intf); > > + > > + return ret; > > +} > > + > > +static int ljca_transfer_internal(struct ljca *ljca, u8 cmd, const void *obuf, > > + unsigned int obuf_len, void *ibuf, unsigned int *ibuf_len, > > + bool wait_ack) > > +{ > > + struct ljca_stub *stub; > > + > > + stub = ljca_stub_find(ljca->dev, ljca->type); > > + if (IS_ERR(stub)) > > + return PTR_ERR(stub); > > + > > + return ljca_stub_write(stub, cmd, obuf, obuf_len, ibuf, ibuf_len, wait_ack, > > + LJCA_USB_WRITE_ACK_TIMEOUT_MS); > > +} > > + > > +int ljca_transfer(struct ljca *ljca, u8 cmd, const void *obuf, unsigned int obuf_len, void *ibuf, > > + unsigned int *ibuf_len) > > +{ > > + return ljca_transfer_internal(ljca, cmd, obuf, obuf_len, ibuf, ibuf_len, true); > > +} > > +EXPORT_SYMBOL_NS_GPL(ljca_transfer, LJCA); > > + > > +int ljca_transfer_noack(struct ljca *ljca, u8 cmd, const void *obuf, unsigned int obuf_len) > > +{ > > + return ljca_transfer_internal(ljca, cmd, obuf, obuf_len, NULL, NULL, false); > > +} > > +EXPORT_SYMBOL_NS_GPL(ljca_transfer_noack, LJCA); > > + > > +int ljca_register_event_cb(struct ljca *ljca, ljca_event_cb_t event_cb, void *context) > > What are these magic events you are registering? What do they do and > why would anyone need them? It provides a path for sub-devices to listening events from LJCA device. For gpio-ljca, it uses this API to implement gpio interrupt. > > You have global functions here that other drivers are using, yet no > documentation about them at all for any way to review that the api > really is doing what you are wanting it to do. Kernel-doc format comments are in ljca.h in this patch. It has some introduction about these for exported API. > > So again, please split this up into at least 2 changes, and document > this new api you are creating, so that we have a chance to review it > properly. Otherwise it's almost impossible to do so. Got it. Will spit this into 2 commit on next version. > -- Thanks Ye Xiang
Hi Andi, Thanks for the review. On Mon, Mar 13, 2023 at 05:21:46PM +0100, Andi Shyti wrote: > Hi Ye, > > On top of what Greg has already said, few things from my side > through the lines. > > [...] > > > +static int ljca_mng_link(struct ljca_dev *dev, struct ljca_stub *stub) > > +{ > > + int ret; > > + > > + ret = ljca_mng_reset_handshake(stub); > > + if (ret) > > + return ret; > > + > > + /* try to enum all the stubs */ > > + ljca_mng_enum_gpio(stub); > > + ljca_mng_enum_i2c(stub); > > + ljca_mng_enum_spi(stub); > > We are ignoring here the return value. So either make the > whole function call chain to be void or please check the return > values here. Ok, got it. > > [...] > > > +static ssize_t ljca_enable_dfu_store(struct device *dev, struct device_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + struct usb_interface *intf = to_usb_interface(dev); > > + struct ljca_dev *ljca_dev = usb_get_intfdata(intf); > > + struct ljca_stub *mng_stub = ljca_stub_find(ljca_dev, LJCA_MNG_STUB); > > + bool enable; > > + int ret; > > + > > + ret = kstrtobool(buf, &enable); > > + if (ret) > > + return ret; > > + > > + if (enable) { > > + ret = ljca_mng_set_dfu_mode(mng_stub); > > + if (ret) > > + return ret; > > + } > > What is the DFU mode? > Is it an operational mode? > Do we enter and exit from it? > Does the device leave this mode on its own? > What if I write twice in a raw enable? > Can I check if I am in DFU mode or not? > > Would you mind adding some comments here? DFU mode is used for updating LJCA device Firmware. We have a Documentation/ABI/testing/sysfs-bus-usb-devices-ljca in patch 5 of this patch series. It has information about the entry. Maybe it should be go after this patch (patch 2/5). > > > + > > + return count; > > +} > > +static DEVICE_ATTR_WO(ljca_enable_dfu); > > + > > +static ssize_t ljca_trace_level_store(struct device *dev, struct device_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + struct usb_interface *intf = to_usb_interface(dev); > > + struct ljca_dev *ljca_dev = usb_get_intfdata(intf); > > + struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB); > > + u8 level; > > + int ret; > > + > > + if (kstrtou8(buf, 0, &level)) > > + return -EINVAL; > > + > > + ret = ljca_diag_set_trace_level(diag_stub, level); > > + if (ret) > > + return ret; > > do we need any range check for the levels? What happens if I do: > > echo "I am too cool" > /sys/.../ljca_trace_level > > As there were questions here, would you mind adding some comments > so that the next reader won't make the same questions? We have a patch (patch 5 of this series) adding some Documentation/ABI/ entries to specify the correct value for each entry. > > > + > > + return count; > > +} > > +static DEVICE_ATTR_WO(ljca_trace_level); > > [...] > > > +static int ljca_probe(struct usb_interface *intf, const struct usb_device_id *id) > > +{ > > + struct ljca_dev *dev; > > + struct usb_endpoint_descriptor *bulk_in, *bulk_out; > > + int ret; > > + > > + /* allocate memory for our device state and initialize it */ > > + dev = kzalloc(sizeof(*dev), GFP_KERNEL); > > devm_kzalloc() Got it. Thanks. -- Thanks Ye Xiang
Hi Lee, Thanks for the review. On Mon, Mar 13, 2023 at 05:03:41PM +0000, Lee Jones wrote: > On Mon, 13 Mar 2023, Ye Xiang wrote: > > > This patch implements the USB part of Intel USB-I2C/GPIO/SPI adapter > > device named "La Jolla Cove Adapter" (LJCA). > > > > The communication between the various LJCA module drivers and the > > hardware will be muxed/demuxed by this driver. The sub-module of > > LJCA can use ljca_transfer() to issue a transfer between host > > and hardware. > > > > Each sub-module of LJCA device is identified by type field within > > the LJCA message header. > > > > The minimum code in ASL that covers this board is > > Scope (\_SB.PCI0.DWC3.RHUB.HS01) > > { > > Device (GPIO) > > { > > Name (_ADR, Zero) > > Name (_STA, 0x0F) > > } > > > > Device (I2C) > > { > > Name (_ADR, One) > > Name (_STA, 0x0F) > > } > > > > Device (SPI) > > { > > Name (_ADR, 0x02) > > Name (_STA, 0x0F) > > } > > } > > > > Signed-off-by: Ye Xiang <xiang.ye@intel.com> > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > --- > > drivers/usb/misc/Kconfig | 13 + > > drivers/usb/misc/Makefile | 1 + > > drivers/usb/misc/ljca.c | 998 ++++++++++++++++++++++++++++++++++++++ > > include/linux/usb/ljca.h | 95 ++++ > > 4 files changed, 1107 insertions(+) > > create mode 100644 drivers/usb/misc/ljca.c > > create mode 100644 include/linux/usb/ljca.h > > > > diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig > > index a5f7652db7da..59ec120c26d4 100644 > > --- a/drivers/usb/misc/Kconfig > > +++ b/drivers/usb/misc/Kconfig > > @@ -273,6 +273,19 @@ config USB_LINK_LAYER_TEST > > Layer Test Device. Say Y only when you want to conduct USB Super Speed > > Link Layer Test for host controllers. > > > > +config USB_LJCA > > + tristate "Intel La Jolla Cove Adapter support" > > + select MFD_CORE > > + depends on USB > > + help > > + This adds support for Intel La Jolla Cove USB-I2C/SPI/GPIO > > + Master Adapter (LJCA). Additional drivers such as I2C_LJCA, > > + GPIO_LJCA and SPI_LJCA must be enabled in order to use the > > + functionality of the device. > > + > > + This driver can also be built as a module. If so, the module > > + will be called ljca. > > + > > config USB_CHAOSKEY > > tristate "ChaosKey random number generator driver support" > > depends on HW_RANDOM > > diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile > > index 93581baec3a8..6f6adfbe17e0 100644 > > --- a/drivers/usb/misc/Makefile > > +++ b/drivers/usb/misc/Makefile > > @@ -29,6 +29,7 @@ obj-$(CONFIG_USB_HUB_USB251XB) += usb251xb.o > > obj-$(CONFIG_USB_HSIC_USB3503) += usb3503.o > > obj-$(CONFIG_USB_HSIC_USB4604) += usb4604.o > > obj-$(CONFIG_USB_CHAOSKEY) += chaoskey.o > > +obj-$(CONFIG_USB_LJCA) += ljca.o > > > > obj-$(CONFIG_USB_SISUSBVGA) += sisusbvga/ > > obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o > > diff --git a/drivers/usb/misc/ljca.c b/drivers/usb/misc/ljca.c > > new file mode 100644 > > index 000000000000..ab98deaf0074 > > --- /dev/null > > +++ b/drivers/usb/misc/ljca.c > > @@ -0,0 +1,998 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Intel La Jolla Cove Adapter USB driver > > + * > > + * Copyright (c) 2023, Intel Corporation. > > + */ > > + > > +#include <linux/dev_printk.h> > > +#include <linux/kernel.h> > > +#include <linux/mfd/core.h> > > Please don't use the MFD API outside of drivers/mfd. > > If you wish to use the API, please do. > > Strip out (only) the MFD parts and move them into drivers/mfd. I have no idea about how to split MFD parts out from this driver currently. The MFD part just have mfd cells filling and the call mfd_add_hotplug_devices to register mfd devices. How to module them as an independent driver? Would you give some hints or recommendations? And I am a little comfused about where this USB device driver should be put to (drivers/mfd or drivers/usb). As far as I know, where a driver should be put is based on what it provides. This driver just do some urb package transfer to provides multi-functions, such as GPIO function, I2C function, SPI function. so it should be under drivers/mfd folder. Please correct me, if something is wrong. Thanks > > > +#include <linux/module.h> > > +#include <linux/mod_devicetable.h> > > +#include <linux/mutex.h> > > +#include <linux/platform_device.h> > > +#include <linux/slab.h> > > +#include <linux/types.h> > > +#include <linux/usb.h> > > +#include <linux/usb/ljca.h> > -- Thanks Ye Xiang
Hi Xiang, On Tue, Mar 14, 2023 at 04:03:26PM +0800, Ye, Xiang wrote: > > Please don't use the MFD API outside of drivers/mfd. > > > > If you wish to use the API, please do. > > > > Strip out (only) the MFD parts and move them into drivers/mfd. > I have no idea about how to split MFD parts out from this driver > currently. The MFD part just have mfd cells filling and the call > mfd_add_hotplug_devices to register mfd devices. How to module them > as an independent driver? > Would you give some hints or recommendations? > > And I am a little comfused about where this USB device driver should > be put to (drivers/mfd or drivers/usb). > > As far as I know, where a driver should be put is based on what > it provides. This driver just do some urb package transfer to provides > multi-functions, such as GPIO function, I2C function, SPI function. > so it should be under drivers/mfd folder. Please correct me, if > something is wrong. Thanks You don't really seem to get any benefit from MFD. Perhaps it would be more appropriate and clear if you just registered auxiliary devices in this driver. Check drivers/base/auxiliary.c. thanks,
Hi Heikki, Thanks for the review. On Tue, Mar 14, 2023 at 10:36:57AM +0200, Heikki Krogerus wrote: > Hi Xiang, > > On Tue, Mar 14, 2023 at 04:03:26PM +0800, Ye, Xiang wrote: > > > Please don't use the MFD API outside of drivers/mfd. > > > > > > If you wish to use the API, please do. > > > > > > Strip out (only) the MFD parts and move them into drivers/mfd. > > I have no idea about how to split MFD parts out from this driver > > currently. The MFD part just have mfd cells filling and the call > > mfd_add_hotplug_devices to register mfd devices. How to module them > > as an independent driver? > > Would you give some hints or recommendations? > > > > And I am a little comfused about where this USB device driver should > > be put to (drivers/mfd or drivers/usb). > > > > As far as I know, where a driver should be put is based on what > > it provides. This driver just do some urb package transfer to provides > > multi-functions, such as GPIO function, I2C function, SPI function. > > so it should be under drivers/mfd folder. Please correct me, if > > something is wrong. Thanks > > You don't really seem to get any benefit from MFD. Perhaps it would be > more appropriate and clear if you just registered auxiliary devices in > this driver. Check drivers/base/auxiliary.c. Yes, it should be a work. I have a question. MFD provides the ACPI binding for sub-devices through struct mfd_cell_acpi_match. But I didn't see this in drivers/base/auxiliary.c. If using auxiliary bus to implement the LJCA sub-devices, we need to do the sub-devices acpi binding manually in ljca.c. Something Like: adr = LJCA_ACPI_MATCH_GPIO adev = acpi_find_child_device(parent, adr, false); ACPI_COMPANION_SET(&pdev->dev, adev ?: parent); Is that acceptable? -- Thanks Ye Xiang
On Tue, Mar 14, 2023 at 11:38:14PM +0800, Ye, Xiang wrote: > On Tue, Mar 14, 2023 at 10:36:57AM +0200, Heikki Krogerus wrote: > > On Tue, Mar 14, 2023 at 04:03:26PM +0800, Ye, Xiang wrote: ... > > You don't really seem to get any benefit from MFD. Perhaps it would be > > more appropriate and clear if you just registered auxiliary devices in > > this driver. Check drivers/base/auxiliary.c. > Yes, it should be a work. I have a question. > MFD provides the ACPI binding for sub-devices through > struct mfd_cell_acpi_match. But I didn't see this in drivers/base/auxiliary.c. > If using auxiliary bus to implement the LJCA sub-devices, we need to do > the sub-devices acpi binding manually in ljca.c. > > Something Like: > adr = LJCA_ACPI_MATCH_GPIO > adev = acpi_find_child_device(parent, adr, false); > ACPI_COMPANION_SET(&pdev->dev, adev ?: parent); > > Is that acceptable? Maybe you can implement this on the level of auxiliary bus.
On Tue, Mar 14, 2023 at 05:52:52PM +0200, Andy Shevchenko wrote: > On Tue, Mar 14, 2023 at 11:38:14PM +0800, Ye, Xiang wrote: > > On Tue, Mar 14, 2023 at 10:36:57AM +0200, Heikki Krogerus wrote: > > > On Tue, Mar 14, 2023 at 04:03:26PM +0800, Ye, Xiang wrote: > > ... > > > > You don't really seem to get any benefit from MFD. Perhaps it would be > > > more appropriate and clear if you just registered auxiliary devices in > > > this driver. Check drivers/base/auxiliary.c. > > Yes, it should be a work. I have a question. > > MFD provides the ACPI binding for sub-devices through > > struct mfd_cell_acpi_match. But I didn't see this in drivers/base/auxiliary.c. > > If using auxiliary bus to implement the LJCA sub-devices, we need to do > > the sub-devices acpi binding manually in ljca.c. > > > > Something Like: > > adr = LJCA_ACPI_MATCH_GPIO > > adev = acpi_find_child_device(parent, adr, false); > > ACPI_COMPANION_SET(&pdev->dev, adev ?: parent); > > > > Is that acceptable? Looks ok to me. > Maybe you can implement this on the level of auxiliary bus. I would actually prefer that the auxiliary bus itself does not make any assumptions regarding the whereabouts of the fwnodes at this stage. Maybe later, when(if) there are more users. thanks,
> -----Original Message----- > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > Sent: Wednesday, March 15, 2023 5:10 PM > > On Tue, Mar 14, 2023 at 05:52:52PM +0200, Andy Shevchenko wrote: > > On Tue, Mar 14, 2023 at 11:38:14PM +0800, Ye, Xiang wrote: > > > On Tue, Mar 14, 2023 at 10:36:57AM +0200, Heikki Krogerus wrote: > > > > On Tue, Mar 14, 2023 at 04:03:26PM +0800, Ye, Xiang wrote: > > > > ... > > > > > > You don't really seem to get any benefit from MFD. Perhaps it > > > > would be more appropriate and clear if you just registered > > > > auxiliary devices in this driver. Check drivers/base/auxiliary.c. > > > Yes, it should be a work. I have a question. > > > MFD provides the ACPI binding for sub-devices through struct > > > mfd_cell_acpi_match. But I didn't see this in drivers/base/auxiliary.c. > > > If using auxiliary bus to implement the LJCA sub-devices, we need to > > > do the sub-devices acpi binding manually in ljca.c. > > > > > > Something Like: > > > adr = LJCA_ACPI_MATCH_GPIO > > > adev = acpi_find_child_device(parent, adr, false); > > > ACPI_COMPANION_SET(&pdev->dev, adev ?: parent); > > > > > > Is that acceptable? This actually doesn't work, look at the acpi_find_child_device(), it compares the bus address specified by _ADR object, but there is no _ADR object in DSDT for these three devices because the relationship between the parent and children isn't bus type listed in ACPI spec, so it always return NULL. BR, Wentong > > Looks ok to me. > > > Maybe you can implement this on the level of auxiliary bus. > > I would actually prefer that the auxiliary bus itself does not make any > assumptions regarding the whereabouts of the fwnodes at this stage. Maybe > later, when(if) there are more users. > > thanks, > > -- > heikki
On Fri, Jun 30, 2023 at 07:40:48AM +0000, Wu, Wentong wrote: > > From: Heikki Krogerus <heikki.krogerus@linux.intel.com> > > Sent: Wednesday, March 15, 2023 5:10 PM > > On Tue, Mar 14, 2023 at 05:52:52PM +0200, Andy Shevchenko wrote: > > > On Tue, Mar 14, 2023 at 11:38:14PM +0800, Ye, Xiang wrote: > > > > On Tue, Mar 14, 2023 at 10:36:57AM +0200, Heikki Krogerus wrote: > > > > > On Tue, Mar 14, 2023 at 04:03:26PM +0800, Ye, Xiang wrote: ... > > > > > You don't really seem to get any benefit from MFD. Perhaps it > > > > > would be more appropriate and clear if you just registered > > > > > auxiliary devices in this driver. Check drivers/base/auxiliary.c. > > > > Yes, it should be a work. I have a question. > > > > MFD provides the ACPI binding for sub-devices through struct > > > > mfd_cell_acpi_match. But I didn't see this in drivers/base/auxiliary.c. > > > > If using auxiliary bus to implement the LJCA sub-devices, we need to > > > > do the sub-devices acpi binding manually in ljca.c. > > > > > > > > Something Like: > > > > adr = LJCA_ACPI_MATCH_GPIO > > > > adev = acpi_find_child_device(parent, adr, false); > > > > ACPI_COMPANION_SET(&pdev->dev, adev ?: parent); > > > > > > > > Is that acceptable? > > This actually doesn't work, look at the acpi_find_child_device(), it compares > the bus address specified by _ADR object, but there is no _ADR object in DSDT > for these three devices because the relationship between the parent and > children isn't bus type listed in ACPI spec, so it always return NULL. If you want to have this on ACPI enabled platform, ACPI table has to have the necessary bits. What you are describing is a BIOS bug _or_ somebody has to provide the SSDT overlay depending on the real connection of the device.. > > Looks ok to me. > > > > > Maybe you can implement this on the level of auxiliary bus. > > > > I would actually prefer that the auxiliary bus itself does not make any > > assumptions regarding the whereabouts of the fwnodes at this stage. Maybe > > later, when(if) there are more users.
diff --git a/drivers/usb/misc/Kconfig b/drivers/usb/misc/Kconfig index a5f7652db7da..59ec120c26d4 100644 --- a/drivers/usb/misc/Kconfig +++ b/drivers/usb/misc/Kconfig @@ -273,6 +273,19 @@ config USB_LINK_LAYER_TEST Layer Test Device. Say Y only when you want to conduct USB Super Speed Link Layer Test for host controllers. +config USB_LJCA + tristate "Intel La Jolla Cove Adapter support" + select MFD_CORE + depends on USB + help + This adds support for Intel La Jolla Cove USB-I2C/SPI/GPIO + Master Adapter (LJCA). Additional drivers such as I2C_LJCA, + GPIO_LJCA and SPI_LJCA must be enabled in order to use the + functionality of the device. + + This driver can also be built as a module. If so, the module + will be called ljca. + config USB_CHAOSKEY tristate "ChaosKey random number generator driver support" depends on HW_RANDOM diff --git a/drivers/usb/misc/Makefile b/drivers/usb/misc/Makefile index 93581baec3a8..6f6adfbe17e0 100644 --- a/drivers/usb/misc/Makefile +++ b/drivers/usb/misc/Makefile @@ -29,6 +29,7 @@ obj-$(CONFIG_USB_HUB_USB251XB) += usb251xb.o obj-$(CONFIG_USB_HSIC_USB3503) += usb3503.o obj-$(CONFIG_USB_HSIC_USB4604) += usb4604.o obj-$(CONFIG_USB_CHAOSKEY) += chaoskey.o +obj-$(CONFIG_USB_LJCA) += ljca.o obj-$(CONFIG_USB_SISUSBVGA) += sisusbvga/ obj-$(CONFIG_USB_LINK_LAYER_TEST) += lvstest.o diff --git a/drivers/usb/misc/ljca.c b/drivers/usb/misc/ljca.c new file mode 100644 index 000000000000..ab98deaf0074 --- /dev/null +++ b/drivers/usb/misc/ljca.c @@ -0,0 +1,998 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Intel La Jolla Cove Adapter USB driver + * + * Copyright (c) 2023, Intel Corporation. + */ + +#include <linux/dev_printk.h> +#include <linux/kernel.h> +#include <linux/mfd/core.h> +#include <linux/module.h> +#include <linux/mod_devicetable.h> +#include <linux/mutex.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/types.h> +#include <linux/usb.h> +#include <linux/usb/ljca.h> + +enum ljca_acpi_match_adr { + LJCA_ACPI_MATCH_GPIO, + LJCA_ACPI_MATCH_I2C1, + LJCA_ACPI_MATCH_I2C2, + LJCA_ACPI_MATCH_SPI1, + LJCA_ACPI_MATCH_SPI2, +}; + +static struct mfd_cell_acpi_match ljca_acpi_match_gpio = { + .adr = LJCA_ACPI_MATCH_GPIO, +}; + +static struct mfd_cell_acpi_match ljca_acpi_match_i2cs[] = { + { + .adr = LJCA_ACPI_MATCH_I2C1, + }, + { + .adr = LJCA_ACPI_MATCH_I2C2, + }, +}; + +static struct mfd_cell_acpi_match ljca_acpi_match_spis[] = { + { + .adr = LJCA_ACPI_MATCH_SPI1, + }, + { + .adr = LJCA_ACPI_MATCH_SPI2, + }, +}; + +struct ljca_msg { + u8 type; + u8 cmd; + u8 flags; + u8 len; + u8 data[]; +} __packed; + +struct fw_version { + u8 major; + u8 minor; + __le16 patch; + __le16 build; +} __packed; + +/** + * enum ljca_stub_type - Stub type supported by LJCA. + * @LJCA_MNG_STUB: Provides Management messages. + * @LJCA_DIAG_STUB: provides Diagnose messages. + * @LJCA_GPIO_STUB: provides GPIO functionality. + * @LJCA_I2C_STUB: provides I2C functionality. + * @LJCA_SPI_STUB: provides SPI functionality. + */ +enum ljca_stub_type { + LJCA_MNG_STUB = 1, + LJCA_DIAG_STUB, + LJCA_GPIO_STUB, + LJCA_I2C_STUB, + LJCA_SPI_STUB, +}; + +/* command Flags */ +#define LJCA_ACK_FLAG BIT(0) +#define LJCA_RESP_FLAG BIT(1) +#define LJCA_CMPL_FLAG BIT(2) + +/* MNG stub commands */ +enum ljca_mng_cmd { + LJCA_MNG_GET_VERSION = 1, + LJCA_MNG_RESET_NOTIFY, + LJCA_MNG_RESET, + LJCA_MNG_ENUM_GPIO, + LJCA_MNG_ENUM_I2C, + LJCA_MNG_POWER_STATE_CHANGE, + LJCA_MNG_SET_DFU_MODE, + LJCA_MNG_ENUM_SPI, +}; + +/* DIAG commands */ +enum ljca_diag_cmd { + LJCA_DIAG_GET_STATE = 1, + LJCA_DIAG_GET_STATISTIC, + LJCA_DIAG_SET_TRACE_LEVEL, + LJCA_DIAG_SET_ECHO_MODE, + LJCA_DIAG_GET_FW_LOG, + LJCA_DIAG_GET_FW_COREDUMP, + LJCA_DIAG_TRIGGER_WDT, + LJCA_DIAG_TRIGGER_FAULT, + LJCA_DIAG_FEED_WDT, + LJCA_DIAG_GET_SECURE_STATE, +}; + +struct ljca_i2c_ctr_info { + u8 id; + u8 capacity; + u8 intr_pin; +} __packed; + +struct ljca_i2c_descriptor { + u8 num; + struct ljca_i2c_ctr_info info[]; +} __packed; + +struct ljca_spi_ctr_info { + u8 id; + u8 capacity; +} __packed; + +struct ljca_spi_descriptor { + u8 num; + struct ljca_spi_ctr_info info[]; +} __packed; + +struct ljca_bank_descriptor { + u8 bank_id; + u8 pin_num; + + /* 1 bit for each gpio, 1 means valid */ + u32 valid_pins; +} __packed; + +struct ljca_gpio_descriptor { + u8 pins_per_bank; + u8 bank_num; + struct ljca_bank_descriptor bank_desc[]; +} __packed; + +#define LJCA_MAX_PACKET_SIZE 64 +#define LJCA_MAX_PAYLOAD_SIZE (LJCA_MAX_PACKET_SIZE - sizeof(struct ljca_msg)) +#define LJCA_USB_WRITE_TIMEOUT_MS 200 +#define LJCA_USB_WRITE_ACK_TIMEOUT_MS 500 +#define LJCA_USB_ENUM_STUB_TIMEOUT_MS 20 + +struct ljca_event_cb_entry { + ljca_event_cb_t notify; + void *context; +}; + +struct ljca_dev { + struct usb_device *udev; + struct usb_interface *intf; + u8 in_ep; /* the address of the bulk in endpoint */ + u8 out_ep; /* the address of the bulk out endpoint */ + + /* the urb/buffer for read */ + struct urb *in_urb; + unsigned char *ibuf; + size_t ibuf_len; + + bool started; + struct list_head stubs_list; + + /* to wait for an ongoing write ack */ + wait_queue_head_t ack_wq; + + struct mfd_cell *cells; + int cell_count; + /* mutex to protect package transfer with LJCA device */ + struct mutex mutex; +}; + +struct ljca { + u8 type; + struct ljca_dev *dev; +}; + +struct ljca_stub_packet { + unsigned int *ibuf_len; + u8 *ibuf; +}; + +struct ljca_stub { + struct list_head list; + struct usb_interface *intf; + struct ljca_stub_packet ipacket; + u8 type; + + /* for identify ack */ + bool acked; + int cur_cmd; + + struct ljca_event_cb_entry event_entry; + /* lock to protect event_entry */ + spinlock_t event_cb_lock; + + struct ljca ljca; + unsigned long priv[]; +}; + +static inline void *ljca_priv(struct ljca_stub *stub) +{ + return stub->priv; +} + +static bool ljca_validate(struct ljca_msg *header, u32 data_len) +{ + return header->len + sizeof(*header) == data_len; +} + +static struct ljca_stub *ljca_stub_alloc(struct ljca_dev *dev, u8 type, int priv_size) +{ + struct ljca_stub *stub; + + stub = kzalloc(struct_size(stub, priv, priv_size), GFP_KERNEL); + if (!stub) + return ERR_PTR(-ENOMEM); + + stub->type = type; + stub->intf = dev->intf; + stub->ljca.dev = dev; + stub->ljca.type = stub->type; + spin_lock_init(&stub->event_cb_lock); + list_add_tail(&stub->list, &dev->stubs_list); + return stub; +} + +static struct ljca_stub *ljca_stub_find(struct ljca_dev *dev, u8 type) +{ + struct ljca_stub *stub; + + list_for_each_entry(stub, &dev->stubs_list, list) { + if (stub->type == type) + return stub; + } + + dev_err(&dev->intf->dev, "USB stub not found, type:%d\n", type); + + return ERR_PTR(-ENODEV); +} + +static void ljca_stub_notify(struct ljca_stub *stub, u8 cmd, const void *evt_data, int len) +{ + unsigned long flags; + + spin_lock_irqsave(&stub->event_cb_lock, flags); + if (stub->event_entry.notify && stub->event_entry.context) + stub->event_entry.notify(stub->event_entry.context, cmd, evt_data, len); + spin_unlock_irqrestore(&stub->event_cb_lock, flags); +} + +static int ljca_parse(struct ljca_dev *dev, struct ljca_msg *header) +{ + struct ljca_stub *stub; + + stub = ljca_stub_find(dev, header->type); + if (IS_ERR(stub)) + return PTR_ERR(stub); + + if (!(header->flags & LJCA_ACK_FLAG)) { + ljca_stub_notify(stub, header->cmd, header->data, header->len); + return 0; + } + + if (stub->cur_cmd != header->cmd) { + dev_err(&dev->intf->dev, "header and stub current command mismatch (%x vs %x)\n", + header->cmd, stub->cur_cmd); + return -EINVAL; + } + + if (stub->ipacket.ibuf && stub->ipacket.ibuf_len) { + unsigned int newlen; + + newlen = min_t(unsigned int, header->len, *stub->ipacket.ibuf_len); + + *stub->ipacket.ibuf_len = newlen; + memcpy(stub->ipacket.ibuf, header->data, newlen); + } + + stub->acked = true; + wake_up(&dev->ack_wq); + + return 0; +} + +static int ljca_stub_write(struct ljca_stub *stub, u8 cmd, const void *obuf, unsigned int obuf_len, + void *ibuf, unsigned int *ibuf_len, bool wait_ack, unsigned long timeout) +{ + struct ljca_dev *dev = usb_get_intfdata(stub->intf); + u8 flags = LJCA_CMPL_FLAG; + struct ljca_msg *header; + unsigned int msg_len = sizeof(*header) + obuf_len; + int actual; + int ret; + + if (msg_len > LJCA_MAX_PACKET_SIZE) + return -EINVAL; + + if (wait_ack) + flags |= LJCA_ACK_FLAG; + + header = kmalloc(msg_len, GFP_KERNEL); + if (!header) + return -ENOMEM; + + header->type = stub->type; + header->cmd = cmd; + header->flags = flags; + header->len = obuf_len; + + if (obuf) + memcpy(header->data, obuf, obuf_len); + + dev_dbg(&dev->intf->dev, "send: type:%d cmd:%d flags:%d len:%d\n", header->type, + header->cmd, header->flags, header->len); + + usb_autopm_get_interface(dev->intf); + if (!dev->started) { + kfree(header); + ret = -ENODEV; + goto error_put; + } + + mutex_lock(&dev->mutex); + stub->cur_cmd = cmd; + stub->ipacket.ibuf = ibuf; + stub->ipacket.ibuf_len = ibuf_len; + stub->acked = false; + ret = usb_bulk_msg(dev->udev, usb_sndbulkpipe(dev->udev, dev->out_ep), header, msg_len, + &actual, LJCA_USB_WRITE_TIMEOUT_MS); + kfree(header); + if (ret) { + dev_err(&dev->intf->dev, "bridge write failed ret:%d\n", ret); + goto error_unlock; + } + + if (actual != msg_len) { + dev_err(&dev->intf->dev, "bridge write length mismatch (%d vs %d)\n", msg_len, + actual); + ret = -EINVAL; + goto error_unlock; + } + + if (wait_ack) { + ret = wait_event_timeout(dev->ack_wq, stub->acked, msecs_to_jiffies(timeout)); + if (!ret) { + dev_err(&dev->intf->dev, "acked wait timeout\n"); + ret = -ETIMEDOUT; + goto error_unlock; + } + } + + stub->ipacket.ibuf = NULL; + stub->ipacket.ibuf_len = NULL; + ret = 0; +error_unlock: + mutex_unlock(&dev->mutex); +error_put: + usb_autopm_put_interface(dev->intf); + + return ret; +} + +static int ljca_transfer_internal(struct ljca *ljca, u8 cmd, const void *obuf, + unsigned int obuf_len, void *ibuf, unsigned int *ibuf_len, + bool wait_ack) +{ + struct ljca_stub *stub; + + stub = ljca_stub_find(ljca->dev, ljca->type); + if (IS_ERR(stub)) + return PTR_ERR(stub); + + return ljca_stub_write(stub, cmd, obuf, obuf_len, ibuf, ibuf_len, wait_ack, + LJCA_USB_WRITE_ACK_TIMEOUT_MS); +} + +int ljca_transfer(struct ljca *ljca, u8 cmd, const void *obuf, unsigned int obuf_len, void *ibuf, + unsigned int *ibuf_len) +{ + return ljca_transfer_internal(ljca, cmd, obuf, obuf_len, ibuf, ibuf_len, true); +} +EXPORT_SYMBOL_NS_GPL(ljca_transfer, LJCA); + +int ljca_transfer_noack(struct ljca *ljca, u8 cmd, const void *obuf, unsigned int obuf_len) +{ + return ljca_transfer_internal(ljca, cmd, obuf, obuf_len, NULL, NULL, false); +} +EXPORT_SYMBOL_NS_GPL(ljca_transfer_noack, LJCA); + +int ljca_register_event_cb(struct ljca *ljca, ljca_event_cb_t event_cb, void *context) +{ + struct ljca_stub *stub; + unsigned long flags; + + stub = ljca_stub_find(ljca->dev, ljca->type); + if (IS_ERR(stub)) + return PTR_ERR(stub); + + spin_lock_irqsave(&stub->event_cb_lock, flags); + stub->event_entry.notify = event_cb; + stub->event_entry.context = context; + spin_unlock_irqrestore(&stub->event_cb_lock, flags); + + return 0; +} +EXPORT_SYMBOL_NS_GPL(ljca_register_event_cb, LJCA); + +void ljca_unregister_event_cb(struct ljca *ljca) +{ + struct ljca_stub *stub; + unsigned long flags; + + stub = ljca_stub_find(ljca->dev, ljca->type); + if (IS_ERR(stub)) + return; + + spin_lock_irqsave(&stub->event_cb_lock, flags); + stub->event_entry.notify = NULL; + stub->event_entry.context = NULL; + spin_unlock_irqrestore(&stub->event_cb_lock, flags); +} +EXPORT_SYMBOL_NS_GPL(ljca_unregister_event_cb, LJCA); + +static void ljca_stub_cleanup(struct ljca_dev *dev) +{ + struct ljca_stub *stub, *next; + + list_for_each_entry_safe(stub, next, &dev->stubs_list, list) { + list_del_init(&stub->list); + kfree(stub); + } +} + +static void ljca_read_complete(struct urb *urb) +{ + struct ljca_msg *header = urb->transfer_buffer; + struct ljca_dev *dev = urb->context; + int len = urb->actual_length; + int ret; + + WARN_ON_ONCE(!dev); + WARN_ON_ONCE(!header); + + if (urb->status) { + /* sync/async unlink faults aren't errors */ + if (urb->status == -ENOENT || urb->status == -ECONNRESET || + urb->status == -ESHUTDOWN) + return; + + dev_err(&dev->intf->dev, "read bulk urb transfer failed: %d\n", urb->status); + goto resubmit; + } + + dev_dbg(&dev->intf->dev, "receive: type:%d cmd:%d flags:%d len:%d\n", header->type, + header->cmd, header->flags, header->len); + + if (!ljca_validate(header, len)) { + dev_err(&dev->intf->dev, "data not correct header->len:%d payload_len:%d\n ", + header->len, len); + goto resubmit; + } + + ret = ljca_parse(dev, header); + if (ret) + dev_err(&dev->intf->dev, "failed to parse data: ret:%d type:%d len:%d\n", ret, + header->type, header->len); + +resubmit: + ret = usb_submit_urb(urb, GFP_ATOMIC); + if (ret) + dev_err(&dev->intf->dev, "failed submitting read urb, error %d\n", ret); +} + +static int ljca_start(struct ljca_dev *dev) +{ + int ret; + + usb_fill_bulk_urb(dev->in_urb, dev->udev, usb_rcvbulkpipe(dev->udev, dev->in_ep), dev->ibuf, + dev->ibuf_len, ljca_read_complete, dev); + + ret = usb_submit_urb(dev->in_urb, GFP_KERNEL); + if (ret) { + dev_err(&dev->intf->dev, "failed submitting read urb, error %d\n", ret); + return ret; + } + + dev->started = true; + + return 0; +} + +struct ljca_mng_priv { + u32 reset_id; +}; + +static int ljca_mng_reset_handshake(struct ljca_stub *stub) +{ + struct ljca_mng_priv *priv; + __le32 reset_id; + __le32 reset_id_ret = 0; + unsigned int ilen = sizeof(__le32); + int ret; + + priv = ljca_priv(stub); + reset_id = cpu_to_le32(priv->reset_id++); + ret = ljca_stub_write(stub, LJCA_MNG_RESET_NOTIFY, &reset_id, sizeof(reset_id), + &reset_id_ret, &ilen, true, LJCA_USB_WRITE_ACK_TIMEOUT_MS); + if (ret) + return ret; + + if (ilen != sizeof(reset_id_ret) || reset_id_ret != reset_id) { + dev_err(&stub->intf->dev, "mng reset notify failed reset_id:%u/%u\n", + le32_to_cpu(reset_id_ret), le32_to_cpu(reset_id)); + return -EINVAL; + } + + return 0; +} + +static int ljca_add_mfd_cell(struct ljca_dev *dev, struct mfd_cell *cell) +{ + struct mfd_cell *new_cells; + + new_cells = krealloc_array(dev->cells, dev->cell_count + 1, sizeof(*new_cells), GFP_KERNEL); + if (!new_cells) + return -ENOMEM; + + memcpy(&new_cells[dev->cell_count], cell, sizeof(*cell)); + dev->cells = new_cells; + dev->cell_count++; + + return 0; +} + +static int ljca_gpio_stub_init(struct ljca_dev *dev, struct ljca_gpio_descriptor *desc) +{ + u32 valid_pin[LJCA_MAX_GPIO_NUM / BITS_PER_TYPE(u32)]; + struct ljca_gpio_info *gpio_info; + struct mfd_cell cell = {}; + struct ljca_stub *stub; + int gpio_num; + int i; + + gpio_num = desc->pins_per_bank * desc->bank_num; + if (gpio_num > LJCA_MAX_GPIO_NUM) + return -EINVAL; + + stub = ljca_stub_alloc(dev, LJCA_GPIO_STUB, sizeof(*gpio_info)); + if (IS_ERR(stub)) + return PTR_ERR(stub); + + gpio_info = ljca_priv(stub); + gpio_info->ljca = &stub->ljca; + gpio_info->num = gpio_num; + + for (i = 0; i < desc->bank_num; i++) + valid_pin[i] = desc->bank_desc[i].valid_pins; + + bitmap_from_arr32(gpio_info->valid_pin_map, valid_pin, gpio_num); + + cell.name = "ljca-gpio"; + cell.platform_data = gpio_info; + cell.pdata_size = sizeof(*gpio_info); + cell.acpi_match = &ljca_acpi_match_gpio; + + return ljca_add_mfd_cell(dev, &cell); +} + +static int ljca_mng_enum_gpio(struct ljca_stub *stub) +{ + struct ljca_dev *dev = usb_get_intfdata(stub->intf); + char buf[LJCA_MAX_PAYLOAD_SIZE]; + struct ljca_gpio_descriptor *desc; + unsigned int len = LJCA_MAX_PAYLOAD_SIZE; + int ret; + + ret = ljca_stub_write(stub, LJCA_MNG_ENUM_GPIO, NULL, 0, buf, &len, true, + LJCA_USB_ENUM_STUB_TIMEOUT_MS); + if (ret) + return ret; + + desc = (struct ljca_gpio_descriptor *)buf; + if (len != struct_size(desc, bank_desc, desc->bank_num)) { + dev_err(&stub->intf->dev, "GPIO enumeration failed, len:%u bank_num:%u\n", len, + desc->bank_num); + return -EINVAL; + } + + return ljca_gpio_stub_init(dev, desc); +} + +static int ljca_i2c_stub_init(struct ljca_dev *dev, struct ljca_i2c_descriptor *desc) +{ + struct ljca_i2c_info *i2c_info; + struct ljca_stub *stub; + int ret; + int i; + + stub = ljca_stub_alloc(dev, LJCA_I2C_STUB, size_mul(desc->num, sizeof(*i2c_info))); + if (IS_ERR(stub)) + return PTR_ERR(stub); + + i2c_info = ljca_priv(stub); + + for (i = 0; i < desc->num; i++) { + struct mfd_cell cell = {}; + + i2c_info[i].ljca = &stub->ljca; + i2c_info[i].id = desc->info[i].id; + i2c_info[i].capacity = desc->info[i].capacity; + i2c_info[i].intr_pin = desc->info[i].intr_pin; + + cell.name = "ljca-i2c"; + cell.platform_data = &i2c_info[i]; + cell.pdata_size = sizeof(i2c_info[i]); + + if (i < ARRAY_SIZE(ljca_acpi_match_i2cs)) + cell.acpi_match = &ljca_acpi_match_i2cs[i]; + + ret = ljca_add_mfd_cell(dev, &cell); + if (ret) + return ret; + } + + return 0; +} + +static int ljca_mng_enum_i2c(struct ljca_stub *stub) +{ + struct ljca_dev *dev = usb_get_intfdata(stub->intf); + char buf[LJCA_MAX_PAYLOAD_SIZE]; + struct ljca_i2c_descriptor *desc; + unsigned int len = LJCA_MAX_PAYLOAD_SIZE; + int ret; + + ret = ljca_stub_write(stub, LJCA_MNG_ENUM_I2C, NULL, 0, buf, &len, true, + LJCA_USB_ENUM_STUB_TIMEOUT_MS); + if (ret) { + dev_err(&stub->intf->dev, "I2C enumeration failed, ret:%d len:%u\n", ret, len); + return ret; + } + + desc = (struct ljca_i2c_descriptor *)buf; + return ljca_i2c_stub_init(dev, desc); +} + +static int ljca_spi_stub_init(struct ljca_dev *dev, struct ljca_spi_descriptor *desc) +{ + struct ljca_spi_info *spi_info; + struct ljca_stub *stub; + int i; + int ret; + + stub = ljca_stub_alloc(dev, LJCA_SPI_STUB, size_mul(desc->num, sizeof(*spi_info))); + if (IS_ERR(stub)) + return PTR_ERR(stub); + + spi_info = ljca_priv(stub); + for (i = 0; i < desc->num; i++) { + struct mfd_cell cell = {}; + + spi_info[i].ljca = &stub->ljca; + spi_info[i].id = desc->info[i].id; + spi_info[i].capacity = desc->info[i].capacity; + + cell.name = "ljca-spi"; + cell.platform_data = &spi_info[i]; + cell.pdata_size = sizeof(spi_info[i]); + if (i < ARRAY_SIZE(ljca_acpi_match_spis)) + cell.acpi_match = &ljca_acpi_match_spis[i]; + + ret = ljca_add_mfd_cell(dev, &cell); + if (ret) + return ret; + } + + return 0; +} + +static int ljca_mng_enum_spi(struct ljca_stub *stub) +{ + struct ljca_dev *dev = usb_get_intfdata(stub->intf); + char buf[LJCA_MAX_PAYLOAD_SIZE]; + struct ljca_spi_descriptor *desc; + unsigned int len = LJCA_MAX_PAYLOAD_SIZE; + int ret; + + ret = ljca_stub_write(stub, LJCA_MNG_ENUM_SPI, NULL, 0, buf, &len, true, + LJCA_USB_ENUM_STUB_TIMEOUT_MS); + if (ret) { + dev_err(&stub->intf->dev, "SPI enumeration failed, ret:%d len:%d\n", ret, len); + return ret; + } + + desc = (struct ljca_spi_descriptor *)buf; + return ljca_spi_stub_init(dev, desc); +} + +static int ljca_mng_get_version(struct ljca_stub *stub, char *buf) +{ + struct fw_version version = {}; + unsigned int len = sizeof(version); + int ret; + + ret = ljca_stub_write(stub, LJCA_MNG_GET_VERSION, NULL, 0, &version, &len, true, + LJCA_USB_WRITE_ACK_TIMEOUT_MS); + if (ret) + return ret; + + if (len != sizeof(version)) { + dev_err(&stub->intf->dev, "get version failed, len:%d\n", len); + return -EINVAL; + } + + return sysfs_emit(buf, "%d.%d.%d.%d\n", version.major, version.minor, + le16_to_cpu(version.patch), le16_to_cpu(version.build)); +} + +static inline int ljca_mng_set_dfu_mode(struct ljca_stub *stub) +{ + return ljca_stub_write(stub, LJCA_MNG_SET_DFU_MODE, NULL, 0, NULL, NULL, true, + LJCA_USB_WRITE_ACK_TIMEOUT_MS); +} + +static int ljca_mng_link(struct ljca_dev *dev, struct ljca_stub *stub) +{ + int ret; + + ret = ljca_mng_reset_handshake(stub); + if (ret) + return ret; + + /* try to enum all the stubs */ + ljca_mng_enum_gpio(stub); + ljca_mng_enum_i2c(stub); + ljca_mng_enum_spi(stub); + + return 0; +} + +static int ljca_mng_init(struct ljca_dev *dev) +{ + struct ljca_stub *stub; + struct ljca_mng_priv *priv; + int ret; + + stub = ljca_stub_alloc(dev, LJCA_MNG_STUB, sizeof(*priv)); + if (IS_ERR(stub)) + return PTR_ERR(stub); + + priv = ljca_priv(stub); + if (!priv) + return -ENOMEM; + + priv->reset_id = 0; + + ret = ljca_mng_link(dev, stub); + if (ret) + dev_err(&dev->intf->dev, "mng stub link failed, ret:%d\n", ret); + + return ret; +} + +static inline int ljca_diag_set_trace_level(struct ljca_stub *stub, u8 level) +{ + return ljca_stub_write(stub, LJCA_DIAG_SET_TRACE_LEVEL, &level, sizeof(level), NULL, NULL, + true, LJCA_USB_WRITE_ACK_TIMEOUT_MS); +} + +static int ljca_diag_init(struct ljca_dev *dev) +{ + struct ljca_stub *stub; + + stub = ljca_stub_alloc(dev, LJCA_DIAG_STUB, 0); + + return PTR_ERR_OR_ZERO(stub); +} + +static void ljca_delete(struct ljca_dev *dev) +{ + mutex_destroy(&dev->mutex); + usb_free_urb(dev->in_urb); + usb_put_intf(dev->intf); + usb_put_dev(dev->udev); + kfree(dev->ibuf); + kfree(dev->cells); + kfree(dev); +} + +static int ljca_init(struct ljca_dev *dev) +{ + mutex_init(&dev->mutex); + init_waitqueue_head(&dev->ack_wq); + INIT_LIST_HEAD(&dev->stubs_list); + + return 0; +} + +static void ljca_stop(struct ljca_dev *dev) +{ + dev->started = false; + usb_kill_urb(dev->in_urb); +} + +static ssize_t ljca_enable_dfu_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct usb_interface *intf = to_usb_interface(dev); + struct ljca_dev *ljca_dev = usb_get_intfdata(intf); + struct ljca_stub *mng_stub = ljca_stub_find(ljca_dev, LJCA_MNG_STUB); + bool enable; + int ret; + + ret = kstrtobool(buf, &enable); + if (ret) + return ret; + + if (enable) { + ret = ljca_mng_set_dfu_mode(mng_stub); + if (ret) + return ret; + } + + return count; +} +static DEVICE_ATTR_WO(ljca_enable_dfu); + +static ssize_t ljca_trace_level_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + struct usb_interface *intf = to_usb_interface(dev); + struct ljca_dev *ljca_dev = usb_get_intfdata(intf); + struct ljca_stub *diag_stub = ljca_stub_find(ljca_dev, LJCA_DIAG_STUB); + u8 level; + int ret; + + if (kstrtou8(buf, 0, &level)) + return -EINVAL; + + ret = ljca_diag_set_trace_level(diag_stub, level); + if (ret) + return ret; + + return count; +} +static DEVICE_ATTR_WO(ljca_trace_level); + +static ssize_t ljca_version_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + struct usb_interface *intf = to_usb_interface(dev); + struct ljca_dev *ljca_dev = usb_get_intfdata(intf); + struct ljca_stub *stub = ljca_stub_find(ljca_dev, LJCA_MNG_STUB); + + return ljca_mng_get_version(stub, buf); +} +static DEVICE_ATTR_RO(ljca_version); + +static struct attribute *ljca_attrs[] = { + &dev_attr_ljca_version.attr, + &dev_attr_ljca_enable_dfu.attr, + &dev_attr_ljca_trace_level.attr, + NULL +}; +ATTRIBUTE_GROUPS(ljca); + +static int ljca_probe(struct usb_interface *intf, const struct usb_device_id *id) +{ + struct ljca_dev *dev; + struct usb_endpoint_descriptor *bulk_in, *bulk_out; + int ret; + + /* allocate memory for our device state and initialize it */ + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (!dev) + return -ENOMEM; + + ljca_init(dev); + dev->udev = usb_get_dev(interface_to_usbdev(intf)); + dev->intf = usb_get_intf(intf); + + /* set up the endpoint information use only the first bulk-in and bulk-out endpoints */ + ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL); + if (ret) { + dev_err(&intf->dev, "could not find both bulk-in and bulk-out endpoints\n"); + goto error; + } + + dev->ibuf_len = usb_endpoint_maxp(bulk_in); + dev->in_ep = bulk_in->bEndpointAddress; + dev->ibuf = kzalloc(dev->ibuf_len, GFP_KERNEL); + if (!dev->ibuf) { + ret = -ENOMEM; + goto error; + } + + dev->in_urb = usb_alloc_urb(0, GFP_KERNEL); + if (!dev->in_urb) { + ret = -ENOMEM; + goto error; + } + + dev->out_ep = bulk_out->bEndpointAddress; + /* save our data pointer in this intf device */ + usb_set_intfdata(intf, dev); + ret = ljca_start(dev); + if (ret) { + dev_err(&intf->dev, "bridge read start failed ret %d\n", ret); + goto error; + } + + ret = ljca_mng_init(dev); + if (ret) { + dev_err(&intf->dev, "register mng stub failed ret %d\n", ret); + goto error_stop; + } + + ret = ljca_diag_init(dev); + if (ret) { + dev_err(&intf->dev, "register diag stub failed ret %d\n", ret); + goto error_stop; + } + + ret = mfd_add_hotplug_devices(&intf->dev, dev->cells, dev->cell_count); + if (ret) { + dev_err(&intf->dev, "failed to add mfd devices\n"); + goto error_stop; + } + + usb_enable_autosuspend(dev->udev); + return 0; +error_stop: + ljca_stop(dev); +error: + dev_err(&intf->dev, "LJCA USB device init failed\n"); + /* this frees allocated memory */ + ljca_stub_cleanup(dev); + ljca_delete(dev); + + return ret; +} + +static void ljca_disconnect(struct usb_interface *intf) +{ + struct ljca_dev *dev = usb_get_intfdata(intf); + + ljca_stop(dev); + mfd_remove_devices(&intf->dev); + ljca_stub_cleanup(dev); + ljca_delete(dev); +} + +static int ljca_suspend(struct usb_interface *intf, pm_message_t message) +{ + struct ljca_dev *dev = usb_get_intfdata(intf); + + ljca_stop(dev); + return 0; +} + +static int ljca_resume(struct usb_interface *intf) +{ + struct ljca_dev *dev = usb_get_intfdata(intf); + + return ljca_start(dev); +} + +static const struct usb_device_id ljca_table[] = { + {USB_DEVICE(0x8086, 0x0b63)}, + {} +}; +MODULE_DEVICE_TABLE(usb, ljca_table); + +static struct usb_driver ljca_driver = { + .name = "ljca", + .probe = ljca_probe, + .disconnect = ljca_disconnect, + .suspend = ljca_suspend, + .resume = ljca_resume, + .id_table = ljca_table, + .dev_groups = ljca_groups, + .supports_autosuspend = 1, +}; +module_usb_driver(ljca_driver); + +MODULE_AUTHOR("Ye Xiang <xiang.ye@intel.com>"); +MODULE_AUTHOR("Wang Zhifeng <zhifeng.wang@intel.com>"); +MODULE_AUTHOR("Zhang Lixu <lixu.zhang@intel.com>"); +MODULE_DESCRIPTION("Intel La Jolla Cove Adapter USB driver"); +MODULE_LICENSE("GPL"); diff --git a/include/linux/usb/ljca.h b/include/linux/usb/ljca.h new file mode 100644 index 000000000000..9ae3ea242294 --- /dev/null +++ b/include/linux/usb/ljca.h @@ -0,0 +1,95 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +#ifndef _LINUX_USB_LJCA_H_ +#define _LINUX_USB_LJCA_H_ + +#include <linux/types.h> + +struct ljca; + +#define LJCA_MAX_GPIO_NUM 64 +struct ljca_gpio_info { + struct ljca *ljca; + unsigned int num; + DECLARE_BITMAP(valid_pin_map, LJCA_MAX_GPIO_NUM); +}; + +struct ljca_i2c_info { + struct ljca *ljca; + u8 id; + u8 capacity; + u8 intr_pin; +}; + +struct ljca_spi_info { + struct ljca *ljca; + u8 id; + u8 capacity; +}; + +/** + * typedef ljca_event_cb_t - event callback function signature + * + * @context: the execution context of who registered this callback + * @cmd: the command from device for this event + * @evt_data: the event data payload + * @len: the event data payload length + * + * The callback function is called in interrupt context and the data payload is + * only valid during the call. If the user needs later access of the data, it + * must copy it. + */ +typedef void (*ljca_event_cb_t)(void *context, u8 cmd, const void *evt_data, int len); + +/** + * ljca_register_event_cb - register a callback function to receive events + * + * @ljca: ljca device handle + * @event_cb: callback function + * @context: execution context of event callback + * + * Return: 0 in case of success, negative value in case of error + */ +int ljca_register_event_cb(struct ljca *ljca, ljca_event_cb_t event_cb, void *context); + +/** + * ljca_unregister_event_cb - unregister the callback function for an event + * + * @ljca: ljca device handle + */ +void ljca_unregister_event_cb(struct ljca *ljca); + +/** + * ljca_transfer - issue a LJCA command and wait for a response and the + * associated data + * + * @ljca: ljca device handle + * @cmd: the command to be sent to the device + * @obuf: the buffer to be sent to the device; it can be NULL if the user + * doesn't need to transmit data with this command + * @obuf_len: the size of the buffer to be sent to the device; it should + * be 0 when obuf is NULL + * @ibuf: any data associated with the response will be copied here; it can be + * NULL if the user doesn't need the response data + * @ibuf_len: must be initialized to the input buffer size; it will be modified + * to indicate the actual data transferred; it shouldn't be NULL as well + * when ibuf isn't NULL + * + * Return: 0 for success, negative value for errors + */ +int ljca_transfer(struct ljca *ljca, u8 cmd, const void *obuf, unsigned int obuf_len, + void *ibuf, unsigned int *ibuf_len); + +/** + * ljca_transfer_noack - issue a LJCA command without a response + * + * @ljca: ljca device handle + * @cmd: the command to be sent to the device + * @obuf: the buffer to be sent to the device; it can be NULL if the user + * doesn't need to transmit data with this command + * @obuf_len: the size of the buffer to be sent to the device + * + * Return: 0 for success, negative value for errors + */ +int ljca_transfer_noack(struct ljca *ljca, u8 cmd, const void *obuf, unsigned int obuf_len); + +#endif
This patch implements the USB part of Intel USB-I2C/GPIO/SPI adapter device named "La Jolla Cove Adapter" (LJCA). The communication between the various LJCA module drivers and the hardware will be muxed/demuxed by this driver. The sub-module of LJCA can use ljca_transfer() to issue a transfer between host and hardware. Each sub-module of LJCA device is identified by type field within the LJCA message header. The minimum code in ASL that covers this board is Scope (\_SB.PCI0.DWC3.RHUB.HS01) { Device (GPIO) { Name (_ADR, Zero) Name (_STA, 0x0F) } Device (I2C) { Name (_ADR, One) Name (_STA, 0x0F) } Device (SPI) { Name (_ADR, 0x02) Name (_STA, 0x0F) } } Signed-off-by: Ye Xiang <xiang.ye@intel.com> Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> --- drivers/usb/misc/Kconfig | 13 + drivers/usb/misc/Makefile | 1 + drivers/usb/misc/ljca.c | 998 ++++++++++++++++++++++++++++++++++++++ include/linux/usb/ljca.h | 95 ++++ 4 files changed, 1107 insertions(+) create mode 100644 drivers/usb/misc/ljca.c create mode 100644 include/linux/usb/ljca.h