Re: [PATCH v12 3/9] mmc: cavium: Add MMC platform driver for Octeon SOCs

[Ulf Hansson]

On 20 March 2017 at 15:40, Jan Glauber <jan.glauber@xxxxxxxxxxxxxxxxxx> wrote:
> On Fri, Mar 17, 2017 at 02:35:34PM +0100, Ulf Hansson wrote:
>> On 10 March 2017 at 14:25, Jan Glauber <jglauber@xxxxxxxxxx> wrote:
>> > Add a platform driver for Octeon MIPS SOCs.
>> >
>> > Signed-off-by: Jan Glauber <jglauber@xxxxxxxxxx>
>> > Signed-off-by: David Daney <david.daney@xxxxxxxxxx>
>> > Signed-off-by: Steven J. Hill <steven.hill@xxxxxxxxxx>
>> > ---
>> >  drivers/mmc/host/Kconfig               |  10 ++
>> >  drivers/mmc/host/Makefile              |   2 +
>> >  drivers/mmc/host/cavium-pltfm-octeon.c | 183 +++++++++++++++++++++++++++++++++
>> >  3 files changed, 195 insertions(+)
>> >  create mode 100644 drivers/mmc/host/cavium-pltfm-octeon.c
>>
>> Please rename the file to cavium-octeon.c
>
> OK.
>
>> >
>> > diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> > index f08691a..68cc811 100644
>> > --- a/drivers/mmc/host/Kconfig
>> > +++ b/drivers/mmc/host/Kconfig
>> > @@ -622,6 +622,16 @@ config SDH_BFIN_MISSING_CMD_PULLUP_WORKAROUND
>> >         help
>> >           If you say yes here SD-Cards may work on the EZkit.
>> >
>> > +config MMC_CAVIUM_OCTEON
>> > +       tristate "Cavium OCTEON SD/MMC Card Interface support"
>> > +       depends on CAVIUM_OCTEON_SOC
>> > +       help
>> > +         This selects Cavium OCTEON SD/MMC card Interface.
>> > +         If you have an OCTEON board with a Multimedia Card slot,
>> > +         say Y or M here.
>> > +
>> > +         If unsure, say N.
>> > +
>>
>> Adding more new cavium variants becomes a bit messy in this approach.
>> May I suggest something similar we are using for SDHCI mmc driver.
>
> There will be no more new variants.
>
>> That is:
>>
>> For the core mmc cavium driver:
>> config MMC_CAVIUM (to build cavium-mmc.o)
>> I would also appreciate to rename that file to cavium.c
>> This also means you need to export the functions you provide from the
>> header cavium-mmc.h (rename to cavium.h)
>>
>> For the octeon variant:
>> config MMC_CAVIUM_OCTEON (to build cavium-octeon.o)
>> depends on MMC_CAVIUM && CAVIUM_OCTEON_SOC
>
> Why is this better? It creates a module out of the shared code that only
> has one single user, the ThunderX or the Octeon driver and they will
> never run both at the same time.
>
> Why not just link the shared code to the driver that uses it? Calling
> accross modules is also slower, although that doesn't matter here.

Okay, lets's keep it as is. We can always change it later on, if we
think it's needed.

Kind regards
Uffe