Re: [PATCH 1/2] misc: rtsx: Move Realtek Card Reader Driver to misc

[Lee Jones]

On Wed, 25 Oct 2017, rui_feng@xxxxxxxxxxxxxx wrote:

> From: rui_feng <rui_feng@xxxxxxxxxxxxxx>
> 
> Because Realtek PCIE card reader driver is a pcie driver,
> and it bridges mmc subsystem and memstick subsystem, it's
> not a mfd driver. Greg and Lee Jones had a discussion about
> where to put the driver, the result is that misc is a good
> place for it, so I move all files to misc. If I don't move
> it to a right place, I can't add any patch for this driver.
> 
> Signed-off-by: Rui Feng <rui_feng@xxxxxxxxxxxxxx>
> Acked-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
>  drivers/memstick/host/rtsx_pci_ms.c      |  2 +-
>  drivers/mfd/Kconfig                      | 11 -----------
>  drivers/mfd/Makefile                     |  2 --
>  drivers/misc/Kconfig                     |  1 +
>  drivers/misc/Makefile                    |  1 +
>  drivers/misc/realtek/Kconfig             | 10 ++++++++++
>  drivers/misc/realtek/Makefile            |  3 +++
>  drivers/{mfd => misc/realtek}/rtl8411.c  |  1 -
>  drivers/{mfd => misc/realtek}/rts5209.c  |  1 -
>  drivers/{mfd => misc/realtek}/rts5227.c  |  1 -
>  drivers/{mfd => misc/realtek}/rts5229.c  |  1 -
>  drivers/{mfd => misc/realtek}/rts5249.c  |  2 --
>  drivers/{mfd => misc/realtek}/rtsx_pcr.c |  1 -
>  drivers/{mfd => misc/realtek}/rtsx_pcr.h |  2 +-
>  drivers/mmc/host/rtsx_pci_sdmmc.c        |  2 +-
>  include/linux/{mfd => }/rtsx_common.h    |  0
>  include/linux/{mfd => }/rtsx_pci.h       |  0
>  17 files changed, 18 insertions(+), 23 deletions(-)
>  create mode 100644 drivers/misc/realtek/Kconfig
>  create mode 100644 drivers/misc/realtek/Makefile
>  rename drivers/{mfd => misc/realtek}/rtl8411.c (99%)
>  rename drivers/{mfd => misc/realtek}/rts5209.c (99%)
>  rename drivers/{mfd => misc/realtek}/rts5227.c (99%)
>  rename drivers/{mfd => misc/realtek}/rts5229.c (99%)
>  rename drivers/{mfd => misc/realtek}/rts5249.c (99%)
>  rename drivers/{mfd => misc/realtek}/rtsx_pcr.c (99%)
>  rename drivers/{mfd => misc/realtek}/rtsx_pcr.h (99%)
>  rename include/linux/{mfd => }/rtsx_common.h (100%)
>  rename include/linux/{mfd => }/rtsx_pci.h (100%)
> 
> diff --git a/drivers/memstick/host/rtsx_pci_ms.c b/drivers/memstick/host/rtsx_pci_ms.c
> index 818fa94..a44b457 100644
> --- a/drivers/memstick/host/rtsx_pci_ms.c
> +++ b/drivers/memstick/host/rtsx_pci_ms.c
> @@ -24,7 +24,7 @@
>  #include <linux/delay.h>
>  #include <linux/platform_device.h>
>  #include <linux/memstick.h>
> -#include <linux/mfd/rtsx_pci.h>
> +#include <linux/rtsx_pci.h>
>  #include <asm/unaligned.h>
>  
>  struct realtek_pci_ms {
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index fc5e4fe..97c0ee5 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -916,17 +916,6 @@ config MFD_RDC321X
>  	  southbridge which provides access to GPIOs and Watchdog using the
>  	  southbridge PCI device configuration space.
>  
> -config MFD_RTSX_PCI
> -	tristate "Realtek PCI-E card reader"
> -	depends on PCI
> -	select MFD_CORE
> -	help
> -	  This supports for Realtek PCI-Express card reader including rts5209,
> -	  rts5227, rts522A, rts5229, rts5249, rts524A, rts525A, rtl8411, etc.
> -	  Realtek card reader supports access to many types of memory cards,
> -	  such as Memory Stick, Memory Stick Pro, Secure Digital and
> -	  MultiMediaCard.
> -
>  config MFD_RT5033
>  	tristate "Richtek RT5033 Power Management IC"
>  	depends on I2C
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c3d0a1b..5398aca 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -18,8 +18,6 @@ obj-$(CONFIG_MFD_CROS_EC_I2C)	+= cros_ec_i2c.o
>  obj-$(CONFIG_MFD_CROS_EC_SPI)	+= cros_ec_spi.o
>  obj-$(CONFIG_MFD_EXYNOS_LPASS)	+= exynos-lpass.o
>  
> -rtsx_pci-objs			:= rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
> -obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
>  obj-$(CONFIG_MFD_RTSX_USB)	+= rtsx_usb.o

Why have you chosen not to move *all* rtsx drivers?

>  obj-$(CONFIG_HTC_PASIC3)	+= htc-pasic3.o
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 8136dc7..5389d62 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -518,4 +518,5 @@ source "drivers/misc/mic/Kconfig"
>  source "drivers/misc/genwqe/Kconfig"
>  source "drivers/misc/echo/Kconfig"
>  source "drivers/misc/cxl/Kconfig"
> +source "drivers/misc/realtek/Kconfig"

What about:

  source "drivers/misc/cardreader/Kconfig"

a) to make things easier to understand just by looking at the
filesystem hierarchy and b) with a view to perhaps becoming its own
subsystem one day.

>  endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index d84819d..1900ca9 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -55,6 +55,7 @@ obj-$(CONFIG_CXL_BASE)		+= cxl/
>  obj-$(CONFIG_ASPEED_LPC_CTRL)	+= aspeed-lpc-ctrl.o
>  obj-$(CONFIG_ASPEED_LPC_SNOOP)	+= aspeed-lpc-snoop.o
>  obj-$(CONFIG_PCI_ENDPOINT_TEST)	+= pci_endpoint_test.o
> +obj-$(CONFIG_MFD_RTSX_PCI)	+=realtek/

"MFD" no longer makes sense.  Perhaps:

  CONFIG_MISC_RTSX_PCI
or
  CONFIG_CARDREADER_RTSX_PCI

>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
> diff --git a/drivers/misc/realtek/Kconfig b/drivers/misc/realtek/Kconfig
> new file mode 100644
> index 0000000..10a8f97
> --- /dev/null
> +++ b/drivers/misc/realtek/Kconfig
> @@ -0,0 +1,10 @@
> +config MFD_RTSX_PCI
> +	tristate "Realtek PCI-E card reader"
> +	depends on PCI
> +	select MFD_CORE
> +	help
> +	  This supports for Realtek PCI-Express card reader including rts5209,

"This adds support for Realtek PCI-Express card readers ..."

> +	  rts5227, rts522A, rts5229, rts5249, rts524A, rts525A, rtl8411, etc.

Etc?  Perhaps a definitive list would be better.

> +	  Realtek card reader supports access to many types of memory cards,

"readers support"

> +	  such as Memory Stick, Memory Stick Pro, Secure Digital and
> +	  MultiMediaCard.


> diff --git a/drivers/misc/realtek/Makefile b/drivers/misc/realtek/Makefile
> new file mode 100644
> index 0000000..67c5cf3
> --- /dev/null
> +++ b/drivers/misc/realtek/Makefile
> @@ -0,0 +1,3 @@
> +rtsx_pci-objs := rtsx_pcr.o rts5209.o rts5229.o rtl8411.o rts5227.o rts5249.o
> +
> +obj-$(CONFIG_MFD_RTSX_PCI)	+= rtsx_pci.o
> \ No newline at end of file

?

> diff --git a/drivers/mfd/rtl8411.c b/drivers/misc/realtek/rtl8411.c
> similarity index 99%
> rename from drivers/mfd/rtl8411.c
> rename to drivers/misc/realtek/rtl8411.c
> index b3ae659..aba05ad 100644
> --- a/drivers/mfd/rtl8411.c
> +++ b/drivers/misc/realtek/rtl8411.c
> @@ -23,7 +23,6 @@
>  #include <linux/module.h>
>  #include <linux/bitops.h>
>  #include <linux/delay.h>
> -#include <linux/mfd/rtsx_pci.h>

Didn't you move this to:

<include>/linux/rtsx_pci.h

[...]

> index 7fcf37b..3602356 100644
> --- a/drivers/mfd/rts5249.c
> +++ b/drivers/misc/realtek/rts5249.c
> @@ -21,7 +21,6 @@
>  
>  #include <linux/module.h>
>  #include <linux/delay.h>
> -#include <linux/mfd/rtsx_pci.h>
>  
>  #include "rtsx_pcr.h"
>  
> @@ -738,4 +737,3 @@ void rts525a_init_params(struct rtsx_pcr *pcr)
>  	pcr->reg_pm_ctrl3 = RTS524A_PM_CTRL3;
>  	pcr->ops = &rts525a_pcr_ops;
>  }
> -

?

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org â Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog