Hi Olof On Fri, 4 May 2012, Olof Johansson wrote: > On Thu, May 3, 2012 at 8:05 AM, Guennadi Liakhovetski > <g.liakhovetski@xxxxxx> wrote: > > Many MMC host drivers already use OF data to obtain their platform-specific > > configuration. Each of them is doing this in its special way, whereas many > > parameters are identical and can easily be generalised. This patch adds > > such a generic parser. So far it only adds and handles very few basic > > properties. New ones can be added in the future as required. > > The bindings need to be documented under Documentation/devicetree/mmc Right, I would do that, but I think, I'll drop this patch altogether for now. There is an alternative patch being developed at the moment, that standardises mmc bindings too. It doesn't provide a parser, but that can be added later if needed. Thanks Guennadi > It's common to use dashes (-) instead of underscores (_) in properties > > Also, no need to prefix with mmc. But also: > > > > + if (of_get_property(node, "mmc,4_bit_data", NULL)) > > + mmc->caps |= MMC_CAP_4_BIT_DATA; > > + if (of_get_property(node, "mmc,8_bit_data", NULL)) > > + mmc->caps |= MMC_CAP_8_BIT_DATA; > > In the device tree, do these encode the capability of the controller, > or how the device under the controller is wired up? It's not uncommon > to have an 4-bit device on a controller that could possibly support > 8-bit devices. It needs to be clear from the property name which is > which to not confuse everyone all the time. > > > + if (of_get_property(node, "mmc,sdio_irq", NULL)) > > + mmc->caps |= MMC_CAP_SDIO_IRQ; > > "supports-sdio-interrupts" > > > + if (of_get_property(node, "mmc,nonremovable", NULL)) > > + mmc->caps |= MMC_CAP_NONREMOVABLE; > > This is a property of the card (well, how the card is connected), not > the controller, right? Again, see above regarding confusion. > > > + if (of_get_property(node, "mmc,needs_poll", NULL)) > > + mmc->caps |= MMC_CAP_NEEDS_POLL; > > This should be implicit from lack of interrupt property in the device node. > > > + if (of_get_property(node, "mmc,inverted_cd", NULL)) > > + mmc->caps2 |= MMC_CAP2_INVERTED_CD; > > + if (of_get_property(node, "mmc,inverted_ro", NULL)) > > + mmc->caps2 |= MMC_CAP2_INVERTED_RO; > > The GPIO specifiers should be able to encode polarity. Is this needed > for other cases as well? > > > -Olof > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
