Re: [PATCH] mmc: support sdhci-mmp2

Philip Rakity
2011-05-28T16:25:13Z

On May 28, 2011, at 1:52 AM, Arnd Bergmann wrote:

> On Saturday 28 May 2011 07:00:38 Philip Rakity wrote:
>> In some sense all of this is backwards. Once the SoC is known (pxa910 or mmp2),
>> it knows that SDHCI controller it supports. It should set a config option
>> like USES_MRVL_V3_SD_CONTROLLER. The Kconfig entry in mmc/host should
>> depend on THAT selection to allow the user to enable the SD option.
>
> I would actually prefer in general if the Kconfig file listed only
> the strictly necessary dependencies for building the driver.
> If this driver can be built anywhere, I would list no dependency at
> all. If it depends on something ARM specific, I'd make it depend
> on CONFIG_ARM.
>
> Then change the defconfig for the particular board to enable the
> driver.
>
> The main advantage of this is to increase build coverage on test
> building machines doing an allyesconfig and randconfig once we
> get there (right now, these have too many build errors, but we
> have plans to work on that).

The controller is built into the mmp2 SoC. No build error could
occur if once the SoC is determined it selected the type of
controller available (in the arch/arm). Like the patch you
helped me with a while ago (which never was accepted).

The Kconfig entry for MMP2 in drivers/mmc/host would
add the line
depends on

This is not a general as depending on ARM but at least
the code would work.

The best solution would be far more general and involve
generic probing and registration but that is a lot of work and
should be done for all of the arm/ directory.

>
>>> --- /dev/null
>>> +++ b/drivers/mmc/host/sdhci-mmp2.c
>>
>> It is probably better to NOT name the file sdhci-mmp2.c but
>> something like sdchi-pxaV3.c since other SoC's may use this controller.
>
> Good point.
>
>>> + pxa = kzalloc(sizeof(struct sdhci_pxa), GFP_KERNEL);
>>> + if (!pxa) {
>>> + ret = -ENOMEM;
>>> + goto out;
>>> + }
>>> + pxa->ops = kzalloc(sizeof(struct sdhci_ops), GFP_KERNEL);
>> why is pxa->ops needed ?
>
> I guess the idea was to be able to free the structure later. I already
> commented that it should be statically allocation instead of kzalloc,
> so that would make the pointer unnecessary.

I do not understand why pxa->ops is needed at all. More general
question.

>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html