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