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

["Ho Jie Feng" <hjf3108@gmail.com>]

On Mon Sep 14, 2026 at 6:31 PM +08, Ulf Hansson wrote:
> On Sat, Sep 12, 2026 at 6:38 PM Ho Jie Feng <hjf3108@gmail.com> 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@gmail.com>
>
> Wow! It's been a while since we received new drivers from memstick controllers!
Yep I wrote this quite a while back and only go around to upstreaming it recently.

>> +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?
>

The traces from the Windows drivers doesn't use DMA at all, at least only the nice
ones which call the kernel APIs to do MMIO operations, unlike some newer drivers which
do the MMIO within the driver. The machine doesn't have VT-d so that is not usable.
Due to this, I inferred DMA operation from the bits from sdhci.h and doing some
experimentation on the device. The flag is only there because I may have missed
something and the DMA path isn't reliable.

I will look into the debugfs stuff, thanks.

>> +
>> +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).
Will fix in v2.