lkml.org 
[lkml]   [2021]   [Dec]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v5 3/3] mmc: Add driver for LiteX's LiteSDCard interface
    Hi Andy,

    Thanks for the feedback!

    On Sat, Dec 25, 2021 at 06:43:22PM +0200, Andy Shevchenko wrote:
    > On Wed, Dec 15, 2021 at 10:00 PM Gabriel Somlo <gsomlo@gmail.com> wrote:
    > >
    > > LiteX (https://github.com/enjoy-digital/litex) is a SoC framework
    > > that targets FPGAs. LiteSDCard is a small footprint, configurable
    > > SDCard core commonly used in LiteX designs.
    > >
    > > The driver was first written in May 2020 and has been maintained
    > > cooperatively by the LiteX community. Thanks to all contributors!
    >
    > ...
    >
    > > + int ret;
    > > +
    > > + host->irq = platform_get_irq_optional(host->dev, 0);
    > > + if (host->irq <= 0) {
    > > + dev_warn(dev, "Failed to get IRQ, using polling\n");
    > > + goto use_polling;
    > > + }
    >
    > [Same comment as per v3.]

    > This is wrong. It missed the deferred probe, for example.
    >
    > The best approach is
    >
    > ret = platform_get_irq_optional(...);
    > if (ret < 0 && ret != -ENXIO)
    > return ret;
    > if (ret > 0)
    > ...we got it...
    >
    > It will allow the future API fix of platform_get_irq_optional() to be
    > really optional.

    Thanks for the example. I still need to work in a decision to use
    polling, though. How about something like this instead:

    ret = platform_get_irq_optional(...);
    if (ret == -ENXIO)
    goto use_polling;
    if (ret < 0)
    return ret; // deferred probe (-EAGAIN likely?)
    if (ret > 0)
    ...we got it, keep going...

    >
    > ...
    >
    > > +#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
    >
    > Why under ifdeffery?

    Because I only want to do it on 64-bit capable architectures.

    The alternative would be to call

    dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));

    on *all* architectures, but ignore the returned error (-EIO,
    presumably on architetures that only support 32-bit DMA).

    Do you think that would be cleaner?

    Thanks,
    --Gabriel

    > > + /* increase from default 32 on 64-bit-DMA capable architectures */
    > > + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
    > > + if (ret)
    > > + goto err;
    > > +#endif
    >
    > With Best Regards,
    > Andy Shevchenko

    \
     
     \ /
      Last update: 2021-12-26 12:47    [W:3.410 / U:0.528 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site