| Message ID | 20230309071100.2856899-3-xiang.ye@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Intel LJCA device driver | expand |
On 09.03.23 08:10, Ye Xiang wrote: > +#define LJCA_GPIO_BUF_SIZE 60 > +struct ljca_gpio_dev { > + struct platform_device *pdev; > + struct gpio_chip gc; > + struct ljca_gpio_info *gpio_info; > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > + u8 *connect_mode; > + /* mutex to protect irq bus */ > + struct mutex irq_lock; > + struct work_struct work; > + /* lock to protect package transfer to Hardware */ > + struct mutex trans_lock; > + > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; And here we have a violation of DMA coherency rules. Basically you cannot embed buffers into other data structures if they can be subject to DMA. > +static int ljca_gpio_remove(struct platform_device *pdev) > +{ > + struct ljca_gpio_dev *ljca_gpio = platform_get_drvdata(pdev); > + > + gpiochip_remove(&ljca_gpio->gc); > + ljca_unregister_event_cb(ljca_gpio->gpio_info->ljca); > + mutex_destroy(&ljca_gpio->irq_lock); > + mutex_destroy(&ljca_gpio->trans_lock); At this time, what has made sure that no work is scheduled? > + return 0; > +} Regards Oliver
On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote: > On 09.03.23 08:10, Ye Xiang wrote: > > > +#define LJCA_GPIO_BUF_SIZE 60 > > +struct ljca_gpio_dev { > > + struct platform_device *pdev; > > + struct gpio_chip gc; > > + struct ljca_gpio_info *gpio_info; > > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > > + u8 *connect_mode; > > + /* mutex to protect irq bus */ > > + struct mutex irq_lock; > > + struct work_struct work; > > + /* lock to protect package transfer to Hardware */ > > + struct mutex trans_lock; > > + > > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > > And here we have a violation of DMA coherency rules. > Basically you cannot embed buffers into other data structures > if they can be subject to DMA. Huh?! The problem here is alignment. But other than that I can't see the issue with embedding into structures the instances of which will be allocated on the heap.
On Thu, Mar 09, 2023 at 03:52:59PM +0200, Andy Shevchenko wrote: > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote: > > On 09.03.23 08:10, Ye Xiang wrote: > > > > > +#define LJCA_GPIO_BUF_SIZE 60 > > > +struct ljca_gpio_dev { > > > + struct platform_device *pdev; > > > + struct gpio_chip gc; > > > + struct ljca_gpio_info *gpio_info; > > > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > > > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > > > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > > > + u8 *connect_mode; > > > + /* mutex to protect irq bus */ > > > + struct mutex irq_lock; > > > + struct work_struct work; > > > + /* lock to protect package transfer to Hardware */ > > > + struct mutex trans_lock; > > > + > > > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > > > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > > > > And here we have a violation of DMA coherency rules. > > Basically you cannot embed buffers into other data structures > > if they can be subject to DMA. > > Huh?! > > The problem here is alignment. But other than that I can't see the issue with > embedding into structures the instances of which will be allocated on the heap. As you said, the problem is alignment.
On Thu, Mar 9, 2023 at 2:53 PM Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote: > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote: > > On 09.03.23 08:10, Ye Xiang wrote: > > > > > +#define LJCA_GPIO_BUF_SIZE 60 > > > +struct ljca_gpio_dev { > > > + struct platform_device *pdev; > > > + struct gpio_chip gc; > > > + struct ljca_gpio_info *gpio_info; > > > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > > > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > > > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > > > + u8 *connect_mode; > > > + /* mutex to protect irq bus */ > > > + struct mutex irq_lock; > > > + struct work_struct work; > > > + /* lock to protect package transfer to Hardware */ > > > + struct mutex trans_lock; > > > + > > > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > > > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > > > > And here we have a violation of DMA coherency rules. > > Basically you cannot embed buffers into other data structures > > if they can be subject to DMA. > > Huh?! > > The problem here is alignment. But other than that I can't see the issue with > embedding into structures the instances of which will be allocated on the heap. Yups. And I think the solution looks something like this: u8 obuf[LJCA_GPIO_BUF_SIZE] __aligned(8); u8 ibuf[LJCA_GPIO_BUF_SIZE] __aligned(8); __aligned(4) if it's 32bit DMA I guess? 8 always works that's why we use it all over the IIO subsystem. Yours, Linus Walleij
On Thu, Mar 09, 2023 at 03:18:53PM +0100, Linus Walleij wrote: > On Thu, Mar 9, 2023 at 2:53 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote: > > > On 09.03.23 08:10, Ye Xiang wrote: > > > > > > > +#define LJCA_GPIO_BUF_SIZE 60 > > > > +struct ljca_gpio_dev { > > > > + struct platform_device *pdev; > > > > + struct gpio_chip gc; > > > > + struct ljca_gpio_info *gpio_info; > > > > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > > > > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > > > > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > > > > + u8 *connect_mode; > > > > + /* mutex to protect irq bus */ > > > > + struct mutex irq_lock; > > > > + struct work_struct work; > > > > + /* lock to protect package transfer to Hardware */ > > > > + struct mutex trans_lock; > > > > + > > > > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > > > > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > > > > > > And here we have a violation of DMA coherency rules. > > > Basically you cannot embed buffers into other data structures > > > if they can be subject to DMA. > > > > Huh?! > > > > The problem here is alignment. But other than that I can't see the issue with > > embedding into structures the instances of which will be allocated on the heap. > > Yups. And I think the solution looks something like this: > > u8 obuf[LJCA_GPIO_BUF_SIZE] __aligned(8); > u8 ibuf[LJCA_GPIO_BUF_SIZE] __aligned(8); > > __aligned(4) if it's 32bit DMA I guess? 8 always works that's > why we use it all over the IIO subsystem. To make it all simple, just make obuf and ibuf pointers to the data you allocate with a call to kmalloc(). thanks, greg k-h
On Thu, Mar 9, 2023, at 15:18, Linus Walleij wrote: > On Thu, Mar 9, 2023 at 2:53 PM Andy Shevchenko > <andriy.shevchenko@linux.intel.com> wrote: > > Yups. And I think the solution looks something like this: > > u8 obuf[LJCA_GPIO_BUF_SIZE] __aligned(8); > u8 ibuf[LJCA_GPIO_BUF_SIZE] __aligned(8); > > __aligned(4) if it's 32bit DMA I guess? 8 always works that's > why we use it all over the IIO subsystem. I think it must be __aligned(ARCH_DMA_MINALIGN), to account for architectures with non-coherent DMA. Arnd
On 09.03.23 14:52, Andy Shevchenko wrote: > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote: >> On 09.03.23 08:10, Ye Xiang wrote: >>> + u8 obuf[LJCA_GPIO_BUF_SIZE]; >>> + u8 ibuf[LJCA_GPIO_BUF_SIZE]; >> >> And here we have a violation of DMA coherency rules. >> Basically you cannot embed buffers into other data structures >> if they can be subject to DMA. > > Huh?! > > The problem here is alignment. But other than that I can't see the issue with > embedding into structures the instances of which will be allocated on the heap. Hi, These constraints are documented in dma-api-howto, but perhaps this is inconvenient to read through. Let me explain for the input case. On certain CPUs DMA does not update CPU caches. Hence when you want to read data generated by DMA you must read from RAM. Hence you invalidate the cache line with dma_map_* operations. Those cache lines must stay invalidated. If you wish to guarantee that, you cannot access a data structure that shares a cache line with a buffer, until you are sure that DMA is finished. On the affected architectures kmalloc() makes sure that no allocation straddles cache lines. Regards Oliver
On 09.03.23 15:48, Arnd Bergmann wrote: > On Thu, Mar 9, 2023, at 15:18, Linus Walleij wrote: >> On Thu, Mar 9, 2023 at 2:53 PM Andy Shevchenko >> <andriy.shevchenko@linux.intel.com> wrote: >> >> Yups. And I think the solution looks something like this: >> >> u8 obuf[LJCA_GPIO_BUF_SIZE] __aligned(8); >> u8 ibuf[LJCA_GPIO_BUF_SIZE] __aligned(8); >> >> __aligned(4) if it's 32bit DMA I guess? 8 always works that's >> why we use it all over the IIO subsystem. > > I think it must be __aligned(ARCH_DMA_MINALIGN), to account > for architectures with non-coherent DMA. In theory, yes, you can do that. Is that a better solution than using what kmalloc() is designed to provide. I don't think so. Regards Oliver
On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote: > > > On 09.03.23 08:10, Ye Xiang wrote: > > > +#define LJCA_GPIO_BUF_SIZE 60 > > +struct ljca_gpio_dev { > > + struct platform_device *pdev; > > + struct gpio_chip gc; > > + struct ljca_gpio_info *gpio_info; > > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > > + u8 *connect_mode; > > + /* mutex to protect irq bus */ > > + struct mutex irq_lock; > > + struct work_struct work; > > + /* lock to protect package transfer to Hardware */ > > + struct mutex trans_lock; > > + > > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > > And here we have a violation of DMA coherency rules. > Basically you cannot embed buffers into other data structures > if they can be subject to DMA. But obuf and ibuf does not used to do DMA transfer here. It is actually copied from or to ljca buffer to do URB transfer. Should it still need to follow the DMA coherency rules? > > > > > > +static int ljca_gpio_remove(struct platform_device *pdev) > > +{ > > + struct ljca_gpio_dev *ljca_gpio = platform_get_drvdata(pdev); > > + > > + gpiochip_remove(&ljca_gpio->gc); > > + ljca_unregister_event_cb(ljca_gpio->gpio_info->ljca); > > + mutex_destroy(&ljca_gpio->irq_lock); > > + mutex_destroy(&ljca_gpio->trans_lock); > > At this time, what has made sure that no work is scheduled? Can't make sure of that. Could Adding cancel_work_sync(&ljca_gpio->work) before mutex_destroy can address it? > > > + return 0; > > +} > > -- Thanks Ye Xiang
On Fri, Mar 10, 2023 at 01:01:11PM +0800, Ye, Xiang wrote: > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote: > > > > > > On 09.03.23 08:10, Ye Xiang wrote: > > > > > +#define LJCA_GPIO_BUF_SIZE 60 > > > +struct ljca_gpio_dev { > > > + struct platform_device *pdev; > > > + struct gpio_chip gc; > > > + struct ljca_gpio_info *gpio_info; > > > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > > > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > > > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > > > + u8 *connect_mode; > > > + /* mutex to protect irq bus */ > > > + struct mutex irq_lock; > > > + struct work_struct work; > > > + /* lock to protect package transfer to Hardware */ > > > + struct mutex trans_lock; > > > + > > > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > > > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > > > > And here we have a violation of DMA coherency rules. > > Basically you cannot embed buffers into other data structures > > if they can be subject to DMA. > But obuf and ibuf does not used to do DMA transfer here. > It is actually copied from or to ljca buffer to do URB transfer. urb transfers _ARE_ DMA transfers. > Should it still need to follow the DMA coherency rules? Yes, all buffers for USB urbs are required to follow those rules. thanks, greg k-h
On Fri, Mar 10, 2023 at 08:11:04AM +0100, Greg Kroah-Hartman wrote: > On Fri, Mar 10, 2023 at 01:01:11PM +0800, Ye, Xiang wrote: > > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote: > > > > > > > > > On 09.03.23 08:10, Ye Xiang wrote: > > > > > > > +#define LJCA_GPIO_BUF_SIZE 60 > > > > +struct ljca_gpio_dev { > > > > + struct platform_device *pdev; > > > > + struct gpio_chip gc; > > > > + struct ljca_gpio_info *gpio_info; > > > > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > > > > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > > > > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > > > > + u8 *connect_mode; > > > > + /* mutex to protect irq bus */ > > > > + struct mutex irq_lock; > > > > + struct work_struct work; > > > > + /* lock to protect package transfer to Hardware */ > > > > + struct mutex trans_lock; > > > > + > > > > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > > > > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > > > > > > And here we have a violation of DMA coherency rules. > > > Basically you cannot embed buffers into other data structures > > > if they can be subject to DMA. > > But obuf and ibuf does not used to do DMA transfer here. > > It is actually copied from or to ljca buffer to do URB transfer. > > urb transfers _ARE_ DMA transfers. > > > Should it still need to follow the DMA coherency rules? > > Yes, all buffers for USB urbs are required to follow those rules. But these two buffers are not used to do USB urb transfer directly. For the "u8 obuf[LJCA_GPIO_BUF_SIZE]", it will be copied to ljca buffer ("header->data" as below code [1] showed) first. Then the "header" is used to do the actual urb transfer. And the "header" is allocated by using kmalloc. It should has met the DMA coherency rules. [1] """ struct ljca_msg *header; ... header = kmalloc(msg_len, GFP_KERNEL); if (!header) return -ENOMEM; ... if (obuf) memcpy(header->data, obuf, obuf_len); """ -- Thanks Ye Xiang
On Fri, Mar 10, 2023 at 03:39:07PM +0800, Ye, Xiang wrote: > On Fri, Mar 10, 2023 at 08:11:04AM +0100, Greg Kroah-Hartman wrote: > > On Fri, Mar 10, 2023 at 01:01:11PM +0800, Ye, Xiang wrote: > > > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote: > > > > > > > > > > > > On 09.03.23 08:10, Ye Xiang wrote: > > > > > > > > > +#define LJCA_GPIO_BUF_SIZE 60 > > > > > +struct ljca_gpio_dev { > > > > > + struct platform_device *pdev; > > > > > + struct gpio_chip gc; > > > > > + struct ljca_gpio_info *gpio_info; > > > > > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > > > > > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > > > > > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > > > > > + u8 *connect_mode; > > > > > + /* mutex to protect irq bus */ > > > > > + struct mutex irq_lock; > > > > > + struct work_struct work; > > > > > + /* lock to protect package transfer to Hardware */ > > > > > + struct mutex trans_lock; > > > > > + > > > > > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > > > > > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > > > > > > > > And here we have a violation of DMA coherency rules. > > > > Basically you cannot embed buffers into other data structures > > > > if they can be subject to DMA. > > > But obuf and ibuf does not used to do DMA transfer here. > > > It is actually copied from or to ljca buffer to do URB transfer. > > > > urb transfers _ARE_ DMA transfers. > > > > > Should it still need to follow the DMA coherency rules? > > > > Yes, all buffers for USB urbs are required to follow those rules. > But these two buffers are not used to do USB urb transfer directly. > For the "u8 obuf[LJCA_GPIO_BUF_SIZE]", it will be copied to ljca buffer > ("header->data" as below code [1] showed) first. Then the "header" is used > to do the actual urb transfer. > > And the "header" is allocated by using kmalloc. It should has met the DMA > coherency rules. > > [1] """ > struct ljca_msg *header; > ... > header = kmalloc(msg_len, GFP_KERNEL); > if (!header) > return -ENOMEM; Ok, that's good, but why have 2 buffers for this then? thanks, greg k-h
On Fri, Mar 10, 2023 at 08:53:10AM +0100, Greg Kroah-Hartman wrote: > On Fri, Mar 10, 2023 at 03:39:07PM +0800, Ye, Xiang wrote: > > On Fri, Mar 10, 2023 at 08:11:04AM +0100, Greg Kroah-Hartman wrote: > > > On Fri, Mar 10, 2023 at 01:01:11PM +0800, Ye, Xiang wrote: > > > > On Thu, Mar 09, 2023 at 02:40:10PM +0100, Oliver Neukum wrote: > > > > > > > > > > > > > > > On 09.03.23 08:10, Ye Xiang wrote: > > > > > > > > > > > +#define LJCA_GPIO_BUF_SIZE 60 > > > > > > +struct ljca_gpio_dev { > > > > > > + struct platform_device *pdev; > > > > > > + struct gpio_chip gc; > > > > > > + struct ljca_gpio_info *gpio_info; > > > > > > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > > > > > > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > > > > > > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > > > > > > + u8 *connect_mode; > > > > > > + /* mutex to protect irq bus */ > > > > > > + struct mutex irq_lock; > > > > > > + struct work_struct work; > > > > > > + /* lock to protect package transfer to Hardware */ > > > > > > + struct mutex trans_lock; > > > > > > + > > > > > > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > > > > > > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > > > > > > > > > > And here we have a violation of DMA coherency rules. > > > > > Basically you cannot embed buffers into other data structures > > > > > if they can be subject to DMA. > > > > But obuf and ibuf does not used to do DMA transfer here. > > > > It is actually copied from or to ljca buffer to do URB transfer. > > > > > > urb transfers _ARE_ DMA transfers. > > > > > > > Should it still need to follow the DMA coherency rules? > > > > > > Yes, all buffers for USB urbs are required to follow those rules. > > But these two buffers are not used to do USB urb transfer directly. > > For the "u8 obuf[LJCA_GPIO_BUF_SIZE]", it will be copied to ljca buffer > > ("header->data" as below code [1] showed) first. Then the "header" is used > > to do the actual urb transfer. > > > > And the "header" is allocated by using kmalloc. It should has met the DMA > > coherency rules. > > > > [1] """ > > struct ljca_msg *header; > > ... > > header = kmalloc(msg_len, GFP_KERNEL); > > if (!header) > > return -ENOMEM; > > Ok, that's good, but why have 2 buffers for this then? "u8 obuf[LJCA_GPIO_BUF_SIZE]" is used as a out buffer of LJCA GPIO package. "u8 ibuf[LJCA_GPIO_BUF_SIZE]" is used as a in buffer of LJCA GPIO package. Both obuf and ibuf here are not used to do URB transfer directly. This obuf and ibuf somtimes are using at the same time when calling ljca_transfer. So we need these two buffers. -- Thanks Ye Xiang
On 09/03/2023 08:10, Ye Xiang wrote: > This patch implements the GPIO function of Intel USB-I2C/GPIO/SPI adapter > device named "La Jolla Cove Adapter" (LJCA). It communicate with LJCA > GPIO module with specific protocol through interfaces exported by LJCA USB > driver. > > Signed-off-by: Ye Xiang <xiang.ye@intel.com> > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/gpio/Kconfig | 12 ++ > drivers/gpio/Makefile | 1 + > drivers/gpio/gpio-ljca.c | 454 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 467 insertions(+) > create mode 100644 drivers/gpio/gpio-ljca.c > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 13be729710f2..8be697f9f621 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -1253,6 +1253,18 @@ config GPIO_KEMPLD > This driver can also be built as a module. If so, the module will be > called gpio-kempld. > > +config GPIO_LJCA > + tristate "INTEL La Jolla Cove Adapter GPIO support" > + depends on MFD_LJCA > + select GPIOLIB_IRQCHIP > + default MFD_LJCA > + help > + Select this option to enable GPIO driver for the INTEL > + La Jolla Cove Adapter (LJCA) board. > + > + This driver can also be built as a module. If so, the module > + will be called gpio-ljca. > + > config GPIO_LP3943 > tristate "TI/National Semiconductor LP3943 GPIO expander" > depends on MFD_LP3943 > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > index c048ba003367..eb59524d18c0 100644 > --- a/drivers/gpio/Makefile > +++ b/drivers/gpio/Makefile > @@ -77,6 +77,7 @@ obj-$(CONFIG_GPIO_IXP4XX) += gpio-ixp4xx.o > obj-$(CONFIG_GPIO_JANZ_TTL) += gpio-janz-ttl.o > obj-$(CONFIG_GPIO_KEMPLD) += gpio-kempld.o > obj-$(CONFIG_GPIO_LATCH) += gpio-latch.o > +obj-$(CONFIG_GPIO_LJCA) += gpio-ljca.o > obj-$(CONFIG_GPIO_LOGICVC) += gpio-logicvc.o > obj-$(CONFIG_GPIO_LOONGSON1) += gpio-loongson1.o > obj-$(CONFIG_GPIO_LOONGSON) += gpio-loongson.o > diff --git a/drivers/gpio/gpio-ljca.c b/drivers/gpio/gpio-ljca.c > new file mode 100644 > index 000000000000..87863f0230f5 > --- /dev/null > +++ b/drivers/gpio/gpio-ljca.c > @@ -0,0 +1,454 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Intel La Jolla Cove Adapter USB-GPIO driver > + * > + * Copyright (c) 2023, Intel Corporation. > + */ > + > +#include <linux/acpi.h> > +#include <linux/bitfield.h> > +#include <linux/bitops.h> > +#include <linux/dev_printk.h> > +#include <linux/gpio/driver.h> > +#include <linux/irq.h> > +#include <linux/kernel.h> > +#include <linux/kref.h> > +#include <linux/mfd/ljca.h> > +#include <linux/module.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/types.h> > + > +/* GPIO commands */ > +#define LJCA_GPIO_CONFIG 1 > +#define LJCA_GPIO_READ 2 > +#define LJCA_GPIO_WRITE 3 > +#define LJCA_GPIO_INT_EVENT 4 > +#define LJCA_GPIO_INT_MASK 5 > +#define LJCA_GPIO_INT_UNMASK 6 > + > +#define LJCA_GPIO_CONF_DISABLE BIT(0) > +#define LJCA_GPIO_CONF_INPUT BIT(1) > +#define LJCA_GPIO_CONF_OUTPUT BIT(2) > +#define LJCA_GPIO_CONF_PULLUP BIT(3) > +#define LJCA_GPIO_CONF_PULLDOWN BIT(4) > +#define LJCA_GPIO_CONF_DEFAULT BIT(5) > +#define LJCA_GPIO_CONF_INTERRUPT BIT(6) > +#define LJCA_GPIO_INT_TYPE BIT(7) > + > +#define LJCA_GPIO_CONF_EDGE FIELD_PREP(LJCA_GPIO_INT_TYPE, 1) > +#define LJCA_GPIO_CONF_LEVEL FIELD_PREP(LJCA_GPIO_INT_TYPE, 0) > + > +/* Intentional overlap with PULLUP / PULLDOWN */ > +#define LJCA_GPIO_CONF_SET BIT(3) > +#define LJCA_GPIO_CONF_CLR BIT(4) > + > +struct gpio_op { > + u8 index; > + u8 value; > +} __packed; > + > +struct gpio_packet { > + u8 num; > + struct gpio_op item[]; > +} __packed; > + > +#define LJCA_GPIO_BUF_SIZE 60 > +struct ljca_gpio_dev { > + struct platform_device *pdev; > + struct gpio_chip gc; > + struct ljca_gpio_info *gpio_info; > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > + u8 *connect_mode; > + /* mutex to protect irq bus */ > + struct mutex irq_lock; > + struct work_struct work; > + /* lock to protect package transfer to Hardware */ > + struct mutex trans_lock; > + > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > +}; > + > +static int gpio_config(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id, u8 config) > +{ > + struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf; > + int ret; > + > + mutex_lock(&ljca_gpio->trans_lock); > + packet->item[0].index = gpio_id; > + packet->item[0].value = config | ljca_gpio->connect_mode[gpio_id]; > + packet->num = 1; > + > + ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_CONFIG, packet, > + struct_size(packet, item, packet->num), NULL, NULL); > + mutex_unlock(&ljca_gpio->trans_lock); > + return ret; > +} > + > +static int ljca_gpio_read(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id) > +{ > + struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf; > + struct gpio_packet *ack_packet = (struct gpio_packet *)ljca_gpio->ibuf; > + unsigned int ibuf_len = LJCA_GPIO_BUF_SIZE; > + int ret; > + > + mutex_lock(&ljca_gpio->trans_lock); > + packet->num = 1; > + packet->item[0].index = gpio_id; > + ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_READ, packet, > + struct_size(packet, item, packet->num), ljca_gpio->ibuf, &ibuf_len); > + if (ret) > + goto out_unlock; > + > + if (!ibuf_len || ack_packet->num != packet->num) { > + dev_err(&ljca_gpio->pdev->dev, "failed gpio_id:%u %u", gpio_id, ack_packet->num); > + ret = -EIO; > + } > + > +out_unlock: > + mutex_unlock(&ljca_gpio->trans_lock); > + if (ret) > + return ret; > + return ack_packet->item[0].value > 0; > +} > + > +static int ljca_gpio_write(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id, > + int value) > +{ > + struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf; > + int ret; > + > + mutex_lock(&ljca_gpio->trans_lock); > + packet->num = 1; > + packet->item[0].index = gpio_id; > + packet->item[0].value = value & 1; > + > + ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_WRITE, packet, > + struct_size(packet, item, packet->num), NULL, NULL); > + mutex_unlock(&ljca_gpio->trans_lock); Everywhere you have unusual coding pattern around return. There is almost always line break before return. Adjust to the kernel style. > + return ret; > +} (...) > + > +#define LJCA_GPIO_DRV_NAME "ljca-gpio" > +static const struct platform_device_id ljca_gpio_id[] = { > + { LJCA_GPIO_DRV_NAME, 0 }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(platform, ljca_gpio_id); ljca_gpio_id is unused (except module autoloading). How do you bind your devices? > + > +static struct platform_driver ljca_gpio_driver = { > + .driver.name = LJCA_GPIO_DRV_NAME, > + .probe = ljca_gpio_probe, > + .remove = ljca_gpio_remove, Best regards, Krzysztof
Hi Krzysztof, Thanks for your review. On Sat, Mar 11, 2023 at 01:13:30PM +0100, Krzysztof Kozlowski wrote: > On 09/03/2023 08:10, Ye Xiang wrote: > > This patch implements the GPIO function of Intel USB-I2C/GPIO/SPI adapter > > device named "La Jolla Cove Adapter" (LJCA). It communicate with LJCA > > GPIO module with specific protocol through interfaces exported by LJCA USB > > driver. > > > > Signed-off-by: Ye Xiang <xiang.ye@intel.com> > > Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > --- > > drivers/gpio/Kconfig | 12 ++ > > drivers/gpio/Makefile | 1 + > > drivers/gpio/gpio-ljca.c | 454 +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 467 insertions(+) > > create mode 100644 drivers/gpio/gpio-ljca.c > > > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > > index 13be729710f2..8be697f9f621 100644 > > --- a/drivers/gpio/Kconfig > > +++ b/drivers/gpio/Kconfig > > @@ -1253,6 +1253,18 @@ config GPIO_KEMPLD > > This driver can also be built as a module. If so, the module will be > > called gpio-kempld. > > > > +config GPIO_LJCA > > + tristate "INTEL La Jolla Cove Adapter GPIO support" > > + depends on MFD_LJCA > > + select GPIOLIB_IRQCHIP > > + default MFD_LJCA > > + help > > + Select this option to enable GPIO driver for the INTEL > > + La Jolla Cove Adapter (LJCA) board. > > + > > + This driver can also be built as a module. If so, the module > > + will be called gpio-ljca. > > + > > config GPIO_LP3943 > > tristate "TI/National Semiconductor LP3943 GPIO expander" > > depends on MFD_LP3943 > > diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile > > index c048ba003367..eb59524d18c0 100644 > > --- a/drivers/gpio/Makefile > > +++ b/drivers/gpio/Makefile > > @@ -77,6 +77,7 @@ obj-$(CONFIG_GPIO_IXP4XX) += gpio-ixp4xx.o > > obj-$(CONFIG_GPIO_JANZ_TTL) += gpio-janz-ttl.o > > obj-$(CONFIG_GPIO_KEMPLD) += gpio-kempld.o > > obj-$(CONFIG_GPIO_LATCH) += gpio-latch.o > > +obj-$(CONFIG_GPIO_LJCA) += gpio-ljca.o > > obj-$(CONFIG_GPIO_LOGICVC) += gpio-logicvc.o > > obj-$(CONFIG_GPIO_LOONGSON1) += gpio-loongson1.o > > obj-$(CONFIG_GPIO_LOONGSON) += gpio-loongson.o > > diff --git a/drivers/gpio/gpio-ljca.c b/drivers/gpio/gpio-ljca.c > > new file mode 100644 > > index 000000000000..87863f0230f5 > > --- /dev/null > > +++ b/drivers/gpio/gpio-ljca.c > > @@ -0,0 +1,454 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Intel La Jolla Cove Adapter USB-GPIO driver > > + * > > + * Copyright (c) 2023, Intel Corporation. > > + */ > > + > > +#include <linux/acpi.h> > > +#include <linux/bitfield.h> > > +#include <linux/bitops.h> > > +#include <linux/dev_printk.h> > > +#include <linux/gpio/driver.h> > > +#include <linux/irq.h> > > +#include <linux/kernel.h> > > +#include <linux/kref.h> > > +#include <linux/mfd/ljca.h> > > +#include <linux/module.h> > > +#include <linux/platform_device.h> > > +#include <linux/slab.h> > > +#include <linux/types.h> > > + > > +/* GPIO commands */ > > +#define LJCA_GPIO_CONFIG 1 > > +#define LJCA_GPIO_READ 2 > > +#define LJCA_GPIO_WRITE 3 > > +#define LJCA_GPIO_INT_EVENT 4 > > +#define LJCA_GPIO_INT_MASK 5 > > +#define LJCA_GPIO_INT_UNMASK 6 > > + > > +#define LJCA_GPIO_CONF_DISABLE BIT(0) > > +#define LJCA_GPIO_CONF_INPUT BIT(1) > > +#define LJCA_GPIO_CONF_OUTPUT BIT(2) > > +#define LJCA_GPIO_CONF_PULLUP BIT(3) > > +#define LJCA_GPIO_CONF_PULLDOWN BIT(4) > > +#define LJCA_GPIO_CONF_DEFAULT BIT(5) > > +#define LJCA_GPIO_CONF_INTERRUPT BIT(6) > > +#define LJCA_GPIO_INT_TYPE BIT(7) > > + > > +#define LJCA_GPIO_CONF_EDGE FIELD_PREP(LJCA_GPIO_INT_TYPE, 1) > > +#define LJCA_GPIO_CONF_LEVEL FIELD_PREP(LJCA_GPIO_INT_TYPE, 0) > > + > > +/* Intentional overlap with PULLUP / PULLDOWN */ > > +#define LJCA_GPIO_CONF_SET BIT(3) > > +#define LJCA_GPIO_CONF_CLR BIT(4) > > + > > +struct gpio_op { > > + u8 index; > > + u8 value; > > +} __packed; > > + > > +struct gpio_packet { > > + u8 num; > > + struct gpio_op item[]; > > +} __packed; > > + > > +#define LJCA_GPIO_BUF_SIZE 60 > > +struct ljca_gpio_dev { > > + struct platform_device *pdev; > > + struct gpio_chip gc; > > + struct ljca_gpio_info *gpio_info; > > + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); > > + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); > > + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); > > + u8 *connect_mode; > > + /* mutex to protect irq bus */ > > + struct mutex irq_lock; > > + struct work_struct work; > > + /* lock to protect package transfer to Hardware */ > > + struct mutex trans_lock; > > + > > + u8 obuf[LJCA_GPIO_BUF_SIZE]; > > + u8 ibuf[LJCA_GPIO_BUF_SIZE]; > > +}; > > + > > +static int gpio_config(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id, u8 config) > > +{ > > + struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf; > > + int ret; > > + > > + mutex_lock(&ljca_gpio->trans_lock); > > + packet->item[0].index = gpio_id; > > + packet->item[0].value = config | ljca_gpio->connect_mode[gpio_id]; > > + packet->num = 1; > > + > > + ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_CONFIG, packet, > > + struct_size(packet, item, packet->num), NULL, NULL); > > + mutex_unlock(&ljca_gpio->trans_lock); > > + return ret; > > +} > > + > > +static int ljca_gpio_read(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id) > > +{ > > + struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf; > > + struct gpio_packet *ack_packet = (struct gpio_packet *)ljca_gpio->ibuf; > > + unsigned int ibuf_len = LJCA_GPIO_BUF_SIZE; > > + int ret; > > + > > + mutex_lock(&ljca_gpio->trans_lock); > > + packet->num = 1; > > + packet->item[0].index = gpio_id; > > + ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_READ, packet, > > + struct_size(packet, item, packet->num), ljca_gpio->ibuf, &ibuf_len); > > + if (ret) > > + goto out_unlock; > > + > > + if (!ibuf_len || ack_packet->num != packet->num) { > > + dev_err(&ljca_gpio->pdev->dev, "failed gpio_id:%u %u", gpio_id, ack_packet->num); > > + ret = -EIO; > > + } > > + > > +out_unlock: > > + mutex_unlock(&ljca_gpio->trans_lock); > > + if (ret) > > + return ret; > > + return ack_packet->item[0].value > 0; > > +} > > + > > +static int ljca_gpio_write(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id, > > + int value) > > +{ > > + struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf; > > + int ret; > > + > > + mutex_lock(&ljca_gpio->trans_lock); > > + packet->num = 1; > > + packet->item[0].index = gpio_id; > > + packet->item[0].value = value & 1; > > + > > + ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_WRITE, packet, > > + struct_size(packet, item, packet->num), NULL, NULL); > > + mutex_unlock(&ljca_gpio->trans_lock); > > Everywhere you have unusual coding pattern around return. There is > almost always line break before return. Adjust to the kernel style. Got it. Will add a blank line before return. > > > + return ret; > > +} > > (...) > > > + > > +#define LJCA_GPIO_DRV_NAME "ljca-gpio" > > +static const struct platform_device_id ljca_gpio_id[] = { > > + { LJCA_GPIO_DRV_NAME, 0 }, > > + { /* sentinel */ } > > +}; > > +MODULE_DEVICE_TABLE(platform, ljca_gpio_id); > > ljca_gpio_id is unused (except module autoloading). How do you bind your > devices? driver.name = LJCA_GPIO_DRV_NAME, this driver name is used to bind with devices. > > + > > +static struct platform_driver ljca_gpio_driver = { > > + .driver.name = LJCA_GPIO_DRV_NAME, > > + .probe = ljca_gpio_probe, > > + .remove = ljca_gpio_remove, > > -- Thanks Ye Xiang
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 13be729710f2..8be697f9f621 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1253,6 +1253,18 @@ config GPIO_KEMPLD This driver can also be built as a module. If so, the module will be called gpio-kempld. +config GPIO_LJCA + tristate "INTEL La Jolla Cove Adapter GPIO support" + depends on MFD_LJCA + select GPIOLIB_IRQCHIP + default MFD_LJCA + help + Select this option to enable GPIO driver for the INTEL + La Jolla Cove Adapter (LJCA) board. + + This driver can also be built as a module. If so, the module + will be called gpio-ljca. + config GPIO_LP3943 tristate "TI/National Semiconductor LP3943 GPIO expander" depends on MFD_LP3943 diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index c048ba003367..eb59524d18c0 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -77,6 +77,7 @@ obj-$(CONFIG_GPIO_IXP4XX) += gpio-ixp4xx.o obj-$(CONFIG_GPIO_JANZ_TTL) += gpio-janz-ttl.o obj-$(CONFIG_GPIO_KEMPLD) += gpio-kempld.o obj-$(CONFIG_GPIO_LATCH) += gpio-latch.o +obj-$(CONFIG_GPIO_LJCA) += gpio-ljca.o obj-$(CONFIG_GPIO_LOGICVC) += gpio-logicvc.o obj-$(CONFIG_GPIO_LOONGSON1) += gpio-loongson1.o obj-$(CONFIG_GPIO_LOONGSON) += gpio-loongson.o diff --git a/drivers/gpio/gpio-ljca.c b/drivers/gpio/gpio-ljca.c new file mode 100644 index 000000000000..87863f0230f5 --- /dev/null +++ b/drivers/gpio/gpio-ljca.c @@ -0,0 +1,454 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Intel La Jolla Cove Adapter USB-GPIO driver + * + * Copyright (c) 2023, Intel Corporation. + */ + +#include <linux/acpi.h> +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/dev_printk.h> +#include <linux/gpio/driver.h> +#include <linux/irq.h> +#include <linux/kernel.h> +#include <linux/kref.h> +#include <linux/mfd/ljca.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/slab.h> +#include <linux/types.h> + +/* GPIO commands */ +#define LJCA_GPIO_CONFIG 1 +#define LJCA_GPIO_READ 2 +#define LJCA_GPIO_WRITE 3 +#define LJCA_GPIO_INT_EVENT 4 +#define LJCA_GPIO_INT_MASK 5 +#define LJCA_GPIO_INT_UNMASK 6 + +#define LJCA_GPIO_CONF_DISABLE BIT(0) +#define LJCA_GPIO_CONF_INPUT BIT(1) +#define LJCA_GPIO_CONF_OUTPUT BIT(2) +#define LJCA_GPIO_CONF_PULLUP BIT(3) +#define LJCA_GPIO_CONF_PULLDOWN BIT(4) +#define LJCA_GPIO_CONF_DEFAULT BIT(5) +#define LJCA_GPIO_CONF_INTERRUPT BIT(6) +#define LJCA_GPIO_INT_TYPE BIT(7) + +#define LJCA_GPIO_CONF_EDGE FIELD_PREP(LJCA_GPIO_INT_TYPE, 1) +#define LJCA_GPIO_CONF_LEVEL FIELD_PREP(LJCA_GPIO_INT_TYPE, 0) + +/* Intentional overlap with PULLUP / PULLDOWN */ +#define LJCA_GPIO_CONF_SET BIT(3) +#define LJCA_GPIO_CONF_CLR BIT(4) + +struct gpio_op { + u8 index; + u8 value; +} __packed; + +struct gpio_packet { + u8 num; + struct gpio_op item[]; +} __packed; + +#define LJCA_GPIO_BUF_SIZE 60 +struct ljca_gpio_dev { + struct platform_device *pdev; + struct gpio_chip gc; + struct ljca_gpio_info *gpio_info; + DECLARE_BITMAP(unmasked_irqs, LJCA_MAX_GPIO_NUM); + DECLARE_BITMAP(enabled_irqs, LJCA_MAX_GPIO_NUM); + DECLARE_BITMAP(reenable_irqs, LJCA_MAX_GPIO_NUM); + u8 *connect_mode; + /* mutex to protect irq bus */ + struct mutex irq_lock; + struct work_struct work; + /* lock to protect package transfer to Hardware */ + struct mutex trans_lock; + + u8 obuf[LJCA_GPIO_BUF_SIZE]; + u8 ibuf[LJCA_GPIO_BUF_SIZE]; +}; + +static int gpio_config(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id, u8 config) +{ + struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf; + int ret; + + mutex_lock(&ljca_gpio->trans_lock); + packet->item[0].index = gpio_id; + packet->item[0].value = config | ljca_gpio->connect_mode[gpio_id]; + packet->num = 1; + + ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_CONFIG, packet, + struct_size(packet, item, packet->num), NULL, NULL); + mutex_unlock(&ljca_gpio->trans_lock); + return ret; +} + +static int ljca_gpio_read(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id) +{ + struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf; + struct gpio_packet *ack_packet = (struct gpio_packet *)ljca_gpio->ibuf; + unsigned int ibuf_len = LJCA_GPIO_BUF_SIZE; + int ret; + + mutex_lock(&ljca_gpio->trans_lock); + packet->num = 1; + packet->item[0].index = gpio_id; + ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_READ, packet, + struct_size(packet, item, packet->num), ljca_gpio->ibuf, &ibuf_len); + if (ret) + goto out_unlock; + + if (!ibuf_len || ack_packet->num != packet->num) { + dev_err(&ljca_gpio->pdev->dev, "failed gpio_id:%u %u", gpio_id, ack_packet->num); + ret = -EIO; + } + +out_unlock: + mutex_unlock(&ljca_gpio->trans_lock); + if (ret) + return ret; + return ack_packet->item[0].value > 0; +} + +static int ljca_gpio_write(struct ljca_gpio_dev *ljca_gpio, u8 gpio_id, + int value) +{ + struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf; + int ret; + + mutex_lock(&ljca_gpio->trans_lock); + packet->num = 1; + packet->item[0].index = gpio_id; + packet->item[0].value = value & 1; + + ret = ljca_transfer(ljca_gpio->gpio_info->ljca, LJCA_GPIO_WRITE, packet, + struct_size(packet, item, packet->num), NULL, NULL); + mutex_unlock(&ljca_gpio->trans_lock); + return ret; +} + +static int ljca_gpio_get_value(struct gpio_chip *chip, unsigned int offset) +{ + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip); + + return ljca_gpio_read(ljca_gpio, offset); +} + +static void ljca_gpio_set_value(struct gpio_chip *chip, unsigned int offset, + int val) +{ + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip); + int ret; + + ret = ljca_gpio_write(ljca_gpio, offset, val); + if (ret) + dev_err(chip->parent, "offset:%u val:%d set value failed %d\n", offset, val, ret); +} + +static int ljca_gpio_direction_input(struct gpio_chip *chip, + unsigned int offset) +{ + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip); + u8 config = LJCA_GPIO_CONF_INPUT | LJCA_GPIO_CONF_CLR; + + return gpio_config(ljca_gpio, offset, config); +} + +static int ljca_gpio_direction_output(struct gpio_chip *chip, + unsigned int offset, int val) +{ + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip); + u8 config = LJCA_GPIO_CONF_OUTPUT | LJCA_GPIO_CONF_CLR; + int ret; + + ret = gpio_config(ljca_gpio, offset, config); + if (ret) + return ret; + + ljca_gpio_set_value(chip, offset, val); + return 0; +} + +static int ljca_gpio_set_config(struct gpio_chip *chip, unsigned int offset, + unsigned long config) +{ + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip); + + ljca_gpio->connect_mode[offset] = 0; + switch (pinconf_to_config_param(config)) { + case PIN_CONFIG_BIAS_PULL_UP: + ljca_gpio->connect_mode[offset] |= LJCA_GPIO_CONF_PULLUP; + break; + case PIN_CONFIG_BIAS_PULL_DOWN: + ljca_gpio->connect_mode[offset] |= LJCA_GPIO_CONF_PULLDOWN; + break; + case PIN_CONFIG_DRIVE_PUSH_PULL: + case PIN_CONFIG_PERSIST_STATE: + break; + default: + return -ENOTSUPP; + } + + return 0; +} + +static int ljca_gpio_init_valid_mask(struct gpio_chip *chip, unsigned long *valid_mask, + unsigned int ngpios) +{ + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(chip); + + WARN_ON_ONCE(ngpios != ljca_gpio->gpio_info->num); + bitmap_copy(valid_mask, ljca_gpio->gpio_info->valid_pin_map, ngpios); + + return 0; +} + +static void ljca_gpio_irq_init_valid_mask(struct gpio_chip *chip, unsigned long *valid_mask, + unsigned int ngpios) +{ + ljca_gpio_init_valid_mask(chip, valid_mask, ngpios); +} + +static int ljca_enable_irq(struct ljca_gpio_dev *ljca_gpio, int gpio_id, bool enable) +{ + struct gpio_packet *packet = (struct gpio_packet *)ljca_gpio->obuf; + int ret; + + mutex_lock(&ljca_gpio->trans_lock); + packet->num = 1; + packet->item[0].index = gpio_id; + packet->item[0].value = 0; + + ret = ljca_transfer(ljca_gpio->gpio_info->ljca, + enable ? LJCA_GPIO_INT_UNMASK : LJCA_GPIO_INT_MASK, packet, + struct_size(packet, item, packet->num), NULL, NULL); + mutex_unlock(&ljca_gpio->trans_lock); + return ret; +} + +static void ljca_gpio_async(struct work_struct *work) +{ + struct ljca_gpio_dev *ljca_gpio = container_of(work, struct ljca_gpio_dev, work); + int gpio_id; + int unmasked; + + for_each_set_bit(gpio_id, ljca_gpio->reenable_irqs, ljca_gpio->gc.ngpio) { + clear_bit(gpio_id, ljca_gpio->reenable_irqs); + unmasked = test_bit(gpio_id, ljca_gpio->unmasked_irqs); + if (unmasked) + ljca_enable_irq(ljca_gpio, gpio_id, true); + } +} + +static void ljca_gpio_event_cb(void *context, u8 cmd, const void *evt_data, int len) +{ + const struct gpio_packet *packet = evt_data; + struct ljca_gpio_dev *ljca_gpio = context; + int i; + int irq; + + if (cmd != LJCA_GPIO_INT_EVENT) + return; + + for (i = 0; i < packet->num; i++) { + irq = irq_find_mapping(ljca_gpio->gc.irq.domain, packet->item[i].index); + if (!irq) { + dev_err(ljca_gpio->gc.parent, "gpio_id %u does not mapped to IRQ yet\n", + packet->item[i].index); + return; + } + + generic_handle_domain_irq(ljca_gpio->gc.irq.domain, irq); + set_bit(packet->item[i].index, ljca_gpio->reenable_irqs); + } + + schedule_work(&ljca_gpio->work); +} + +static void ljca_irq_unmask(struct irq_data *irqd) +{ + struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd); + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(gc); + int gpio_id = irqd_to_hwirq(irqd); + + gpiochip_enable_irq(gc, gpio_id); + set_bit(gpio_id, ljca_gpio->unmasked_irqs); +} + +static void ljca_irq_mask(struct irq_data *irqd) +{ + struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd); + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(gc); + int gpio_id = irqd_to_hwirq(irqd); + + clear_bit(gpio_id, ljca_gpio->unmasked_irqs); + gpiochip_disable_irq(gc, gpio_id); +} + +static int ljca_irq_set_type(struct irq_data *irqd, unsigned int type) +{ + struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd); + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(gc); + int gpio_id = irqd_to_hwirq(irqd); + + ljca_gpio->connect_mode[gpio_id] = LJCA_GPIO_CONF_INTERRUPT; + switch (type) { + case IRQ_TYPE_LEVEL_HIGH: + ljca_gpio->connect_mode[gpio_id] |= (LJCA_GPIO_CONF_LEVEL | LJCA_GPIO_CONF_PULLUP); + break; + case IRQ_TYPE_LEVEL_LOW: + ljca_gpio->connect_mode[gpio_id] |= (LJCA_GPIO_CONF_LEVEL | LJCA_GPIO_CONF_PULLDOWN); + break; + case IRQ_TYPE_EDGE_BOTH: + break; + case IRQ_TYPE_EDGE_RISING: + ljca_gpio->connect_mode[gpio_id] |= (LJCA_GPIO_CONF_EDGE | LJCA_GPIO_CONF_PULLUP); + break; + case IRQ_TYPE_EDGE_FALLING: + ljca_gpio->connect_mode[gpio_id] |= (LJCA_GPIO_CONF_EDGE | LJCA_GPIO_CONF_PULLDOWN); + break; + default: + return -EINVAL; + } + + return 0; +} + +static void ljca_irq_bus_lock(struct irq_data *irqd) +{ + struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd); + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(gc); + + mutex_lock(&ljca_gpio->irq_lock); +} + +static void ljca_irq_bus_unlock(struct irq_data *irqd) +{ + struct gpio_chip *gc = irq_data_get_irq_chip_data(irqd); + struct ljca_gpio_dev *ljca_gpio = gpiochip_get_data(gc); + int gpio_id = irqd_to_hwirq(irqd); + int enabled; + int unmasked; + + enabled = test_bit(gpio_id, ljca_gpio->enabled_irqs); + unmasked = test_bit(gpio_id, ljca_gpio->unmasked_irqs); + + if (enabled != unmasked) { + if (unmasked) { + gpio_config(ljca_gpio, gpio_id, 0); + ljca_enable_irq(ljca_gpio, gpio_id, true); + set_bit(gpio_id, ljca_gpio->enabled_irqs); + } else { + ljca_enable_irq(ljca_gpio, gpio_id, false); + clear_bit(gpio_id, ljca_gpio->enabled_irqs); + } + } + + mutex_unlock(&ljca_gpio->irq_lock); +} + +static const struct irq_chip ljca_gpio_irqchip = { + .name = "ljca-irq", + .irq_mask = ljca_irq_mask, + .irq_unmask = ljca_irq_unmask, + .irq_set_type = ljca_irq_set_type, + .irq_bus_lock = ljca_irq_bus_lock, + .irq_bus_sync_unlock = ljca_irq_bus_unlock, + .flags = IRQCHIP_IMMUTABLE, + GPIOCHIP_IRQ_RESOURCE_HELPERS, +}; + +static int ljca_gpio_probe(struct platform_device *pdev) +{ + struct ljca_gpio_dev *ljca_gpio; + struct gpio_irq_chip *girq; + int ret; + + ljca_gpio = devm_kzalloc(&pdev->dev, sizeof(*ljca_gpio), GFP_KERNEL); + if (!ljca_gpio) + return -ENOMEM; + + ljca_gpio->gpio_info = dev_get_platdata(&pdev->dev); + ljca_gpio->connect_mode = devm_kcalloc(&pdev->dev, ljca_gpio->gpio_info->num, + sizeof(*ljca_gpio->connect_mode), GFP_KERNEL); + if (!ljca_gpio->connect_mode) + return -ENOMEM; + + mutex_init(&ljca_gpio->irq_lock); + mutex_init(&ljca_gpio->trans_lock); + ljca_gpio->pdev = pdev; + ljca_gpio->gc.direction_input = ljca_gpio_direction_input; + ljca_gpio->gc.direction_output = ljca_gpio_direction_output; + ljca_gpio->gc.get = ljca_gpio_get_value; + ljca_gpio->gc.set = ljca_gpio_set_value; + ljca_gpio->gc.set_config = ljca_gpio_set_config; + ljca_gpio->gc.init_valid_mask = ljca_gpio_init_valid_mask; + ljca_gpio->gc.can_sleep = true; + ljca_gpio->gc.parent = &pdev->dev; + + ljca_gpio->gc.base = -1; + ljca_gpio->gc.ngpio = ljca_gpio->gpio_info->num; + ljca_gpio->gc.label = ACPI_COMPANION(&pdev->dev) ? + acpi_dev_name(ACPI_COMPANION(&pdev->dev)) : + dev_name(&pdev->dev); + ljca_gpio->gc.owner = THIS_MODULE; + + platform_set_drvdata(pdev, ljca_gpio); + ljca_register_event_cb(ljca_gpio->gpio_info->ljca, ljca_gpio_event_cb, ljca_gpio); + + girq = &ljca_gpio->gc.irq; + gpio_irq_chip_set_chip(girq, &ljca_gpio_irqchip); + girq->parent_handler = NULL; + girq->num_parents = 0; + girq->parents = NULL; + girq->default_type = IRQ_TYPE_NONE; + girq->handler = handle_simple_irq; + girq->init_valid_mask = ljca_gpio_irq_init_valid_mask; + + INIT_WORK(&ljca_gpio->work, ljca_gpio_async); + ret = gpiochip_add_data(&ljca_gpio->gc, ljca_gpio); + if (ret) { + ljca_unregister_event_cb(ljca_gpio->gpio_info->ljca); + mutex_destroy(&ljca_gpio->irq_lock); + mutex_destroy(&ljca_gpio->trans_lock); + } + + return ret; +} + +static int ljca_gpio_remove(struct platform_device *pdev) +{ + struct ljca_gpio_dev *ljca_gpio = platform_get_drvdata(pdev); + + gpiochip_remove(&ljca_gpio->gc); + ljca_unregister_event_cb(ljca_gpio->gpio_info->ljca); + mutex_destroy(&ljca_gpio->irq_lock); + mutex_destroy(&ljca_gpio->trans_lock); + return 0; +} + +#define LJCA_GPIO_DRV_NAME "ljca-gpio" +static const struct platform_device_id ljca_gpio_id[] = { + { LJCA_GPIO_DRV_NAME, 0 }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(platform, ljca_gpio_id); + +static struct platform_driver ljca_gpio_driver = { + .driver.name = LJCA_GPIO_DRV_NAME, + .probe = ljca_gpio_probe, + .remove = ljca_gpio_remove, +}; +module_platform_driver(ljca_gpio_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-GPIO driver"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS(LJCA);