Re: [PATCH v19 4/4] gpio: update Intel LJCA USB GPIO driver

[Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx>]

On Sun, Sep 17, 2023 at 02:53:36AM +0800, Wentong Wu wrote:
> This driver communicate with LJCA GPIO module with specific
> protocol through interfaces exported by LJCA USB driver.
> Update the driver according to LJCA USB driver's changes.
> 
> Signed-off-by: Wentong Wu <wentong.wu@xxxxxxxxx>
> Reviewed-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
> Acked-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Acked-by: Bartosz Golaszewski <bartosz.golaszewski@xxxxxxxxxx>
> Tested-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
>  drivers/gpio/Kconfig     |   4 +-
>  drivers/gpio/gpio-ljca.c | 246 +++++++++++++++++++++++++++--------------------
>  2 files changed, 145 insertions(+), 105 deletions(-)
> 
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 673bafb..8d5b6c3 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1312,9 +1312,9 @@ config GPIO_KEMPLD
>  
>  config GPIO_LJCA
>  	tristate "INTEL La Jolla Cove Adapter GPIO support"
> -	depends on MFD_LJCA
> +	depends on USB_LJCA
>  	select GPIOLIB_IRQCHIP
> -	default MFD_LJCA
> +	default USB_LJCA
>  	help
>  	  Select this option to enable GPIO driver for the INTEL
>  	  La Jolla Cove Adapter (LJCA) board.
> diff --git a/drivers/gpio/gpio-ljca.c b/drivers/gpio/gpio-ljca.c
> index 87863f0..7fae26d 100644
> --- a/drivers/gpio/gpio-ljca.c
> +++ b/drivers/gpio/gpio-ljca.c
> @@ -6,6 +6,7 @@
>   */
>  
>  #include <linux/acpi.h>
> +#include <linux/auxiliary_bus.h>
>  #include <linux/bitfield.h>
>  #include <linux/bitops.h>
>  #include <linux/dev_printk.h>
> @@ -13,19 +14,18 @@
>  #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>
> +#include <linux/usb/ljca.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_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

Why are you changing whitespace for no good reason?

Please don't do that, it makes finding the actual changes in this driver
impossible to notice and review.



>  
>  #define LJCA_GPIO_CONF_DISABLE		BIT(0)
>  #define LJCA_GPIO_CONF_INPUT		BIT(1)
> @@ -36,45 +36,49 @@
>  #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)
> +#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)
> +#define LJCA_GPIO_CONF_SET		BIT(3)
> +#define LJCA_GPIO_CONF_CLR		BIT(4)
>  
> -struct gpio_op {
> +#define LJCA_GPIO_BUF_SIZE		60u

Why "u"?  What requires that?

Odd, sorry, I know people are just getting tired of the constant churn
here, but really, you know better than making changes that are not
needed, or not documented.

greg k-h