Re: [RFC PATCH] memstick: Add Broadcom BCM57765/BCM57785 host driver

[Ulf Hansson]

On Sat, Sep 12, 2026 at 6:38 PM Ho Jie Feng <hjf3108@xxxxxxxxx> wrote:
>
> Add support for the MemoryStick function of Broadcom BCM57765 and BCM57785
> PCI card readers (14e4:16be).
>
> The driver is written by inspecting MMIO traces from the windows driver
> and experimentally probing the device registers.
>
> It was found that the controller exposes an SDHCI-like interface with
> TPC command handling. DMA mode was then inferred using the positions
> of the bits in sdhci.h.
>
> Signed-off-by: Ho Jie Feng <hjf3108@xxxxxxxxx>

Wow! It's been a while since we received new drivers from memstick controllers!

Overall this looks good to me, but I have few minor comments, see below.

> ---
>  MAINTAINERS                         |   6 +
>  drivers/memstick/host/Kconfig       |  10 +
>  drivers/memstick/host/Makefile      |   1 +
>  drivers/memstick/host/bcm577x5_ms.c | 754 ++++++++++++++++++++++++++++
>  drivers/memstick/host/bcm577x5_ms.h | 138 +++++
>  5 files changed, 909 insertions(+)
>  create mode 100644 drivers/memstick/host/bcm577x5_ms.c
>  create mode 100644 drivers/memstick/host/bcm577x5_ms.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 978fe6999d3b3..20faa8c2b1bcc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5234,6 +5234,12 @@ S:       Maintained
>  F:     arch/arm/boot/dts/broadcom/bcm47189*
>  F:     arch/arm/boot/dts/broadcom/bcm53573*
>
> +BROADCOM BCM57765/BCM57785 MEMORYSTICK DRIVER
> +M:     Ho Jie Feng <hjf3108@xxxxxxxxx>
> +L:     linux-mmc@xxxxxxxxxxxxxxx
> +S:     Maintained
> +F:     drivers/memstick/host/bcm577x5_ms.*
> +
>  BROADCOM BCM63XX/BCM33XX UDC DRIVER
>  M:     Kevin Cernekee <cernekee@xxxxxxxxx>
>  L:     linux-usb@xxxxxxxxxxxxxxx
> diff --git a/drivers/memstick/host/Kconfig b/drivers/memstick/host/Kconfig
> index fcd2c2cc3cb47..f9b18f0d85513 100644
> --- a/drivers/memstick/host/Kconfig
> +++ b/drivers/memstick/host/Kconfig
> @@ -53,3 +53,13 @@ config MEMSTICK_REALTEK_USB
>
>           To compile this driver as a module, choose M here: the module will
>           be called rts5139_ms.
> +
> +config MEMSTICK_BCM577X5
> +       tristate "Broadcom BCM57765/BCM57785 MemoryStick interface support"
> +       depends on PCI
> +       help
> +         Say Y here if you want to access MemoryStick cards with the
> +         Broadcom BCM57765/BCM57785 PCI card reader (14e4:16be).
> +
> +         To compile this driver as a module, choose M here: the module
> +         will be called bcm577x5_ms.
> diff --git a/drivers/memstick/host/Makefile b/drivers/memstick/host/Makefile
> index 0c90df33165de..8eafb70b26706 100644
> --- a/drivers/memstick/host/Makefile
> +++ b/drivers/memstick/host/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_MEMSTICK_TIFM_MS)          += tifm_ms.o
>  obj-$(CONFIG_MEMSTICK_JMICRON_38X)     += jmb38x_ms.o
>  obj-$(CONFIG_MEMSTICK_R592)            += r592.o
>  obj-$(CONFIG_MEMSTICK_REALTEK_USB)     += rtsx_usb_ms.o
> +obj-$(CONFIG_MEMSTICK_BCM577X5)                += bcm577x5_ms.o
> diff --git a/drivers/memstick/host/bcm577x5_ms.c b/drivers/memstick/host/bcm577x5_ms.c
> new file mode 100644
> index 0000000000000..81824fb4e9a82
> --- /dev/null
> +++ b/drivers/memstick/host/bcm577x5_ms.c
> @@ -0,0 +1,754 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/align.h>
> +#include <linux/bits.h>
> +#include <linux/completion.h>
> +#include <linux/container_of.h>
> +#include <linux/device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/jiffies.h>
> +#include <linux/memstick.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> +#include <linux/pm.h>
> +#include <linux/scatterlist.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/workqueue.h>
> +
> +#include "bcm577x5_ms.h"
> +
> +#define DRV_NAME "bcm577x5_ms"
> +
> +static int enable_dma = 2;
> +module_param(enable_dma, int, 0444);
> +MODULE_PARM_DESC(enable_dma,
> +                "Enable usage of the DMA (0 = no, 1 = yes, 2 = auto,default)");

I assume this is useful because the DMA functionality is a bit flaky, no?

In any case, I would rather not use a module parameter for this, can
you please drop this. If needed at all, can we perhaps use a debugfs
file instead to switch dynamically?

> +
> +static const struct pci_device_id bcm577x5_pci_id_tbl[] = {
> +       {
> +               PCI_VDEVICE(BROADCOM, 0x16be),
> +       },
> +       {},
> +};

[...]

> +
> +static int bcm577x5_reg_waitb(struct bcm577x5_device *dev, int address, u8 mask,
> +                             u8 value, int timeout)
> +{
> +       unsigned long wait_time = jiffies + msecs_to_jiffies(timeout);
> +       u8 reg;
> +
> +       do {
> +               reg = bcm577x5_reg_readb(dev, address);
> +               if ((reg & mask) == value)
> +                       return 0;
> +
> +               cpu_relax();
> +
> +       } while (time_before(jiffies, wait_time));

Please avoid the open coding and convert to the io polling helpers
instead (iopoll.h).

> +
> +       return -ETIME;
> +}
> +
> +static int bcm577x5_reg_waitl_different(struct bcm577x5_device *dev,
> +                                       int address, u32 value, int timeout)
> +{
> +       unsigned long wait_time = jiffies + msecs_to_jiffies(timeout);
> +       u32 reg;
> +
> +       do {
> +               reg = bcm577x5_reg_readl(dev, address);
> +               if (reg != value)
> +                       return 0;
> +
> +               cpu_relax();
> +
> +       } while (time_before(jiffies, wait_time));

Ditto.

> +
> +       return -ETIMEDOUT;
> +}
> +

[...]

Kind regards
Uffe