[PATCH 0/3 RFC] MMC SPI support for OpenFirmware platforms - Kernel

This is a discussion on [PATCH 0/3 RFC] MMC SPI support for OpenFirmware platforms - Kernel ; Hi all, Here are the bindings, again. Still RFC. This patch series depends on: http://lkml.org/lkml/2008/10/16/250 http://lkml.org/lkml/2008/10/24/416 + of_gpio_flags enum (Trent Piepho will post an updated patch soon, I believe). Pierre, the approach is somewhat similar to this one: http://lkml.org/lkml/2008/5/26/135 Posted ...

+ Reply to Thread
Results 1 to 9 of 9

Thread: [PATCH 0/3 RFC] MMC SPI support for OpenFirmware platforms

  1. [PATCH 0/3 RFC] MMC SPI support for OpenFirmware platforms

    Hi all,

    Here are the bindings, again. Still RFC.

    This patch series depends on:
    http://lkml.org/lkml/2008/10/16/250
    http://lkml.org/lkml/2008/10/24/416
    + of_gpio_flags enum (Trent Piepho will post an updated patch soon,
    I believe).

    Pierre, the approach is somewhat similar to this one:
    http://lkml.org/lkml/2008/5/26/135
    Posted few months ago.

    I know you don't like it, but I ask you to reconsider it. The
    I2C and SPI cases are similar, and recently we tried to write
    bindings for some I2C GPIO controllers.

    There we've learned that we:

    1. Don't like the bus notifiers approach b/c we can't place the OF
    code into the module.
    http://lkml.org/lkml/2008/10/21/425
    Hacks are possible, but they're are ugly.
    2. Don't want to write new drivers to solely handle the platform
    data:
    http://lkml.org/lkml/2008/10/28/257
    http://lkml.org/lkml/2008/10/28/268

    And personally I don't want to do refactoring for every driver that
    we'd want to use with the OpenFirmware...

    If I understood correctly, for GPIO controllers David agreed that
    we can live with the platform data accessors, at least for now:
    http://lkml.org/lkml/2008/10/23/24

    And when/if we'll find something better I'll be the first who
    will offer help to convert the bindings code to this "something
    better".

    Thanks,

    --
    Anton Vorontsov
    email: cbouatmailru@gmail.com
    irc://irc.freenode.net/bd2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [PATCH 3/3] mmc_spi: Add support for OpenFirmware bindings

    The support is implemented via platform data accessors, new module
    (of_mmc_spi) will be created automatically when the driver compiles
    on OpenFirmware platforms. Link-time dependency will load the module
    automatically.

    Signed-off-by: Anton Vorontsov
    ---
    drivers/mmc/host/Makefile | 3 +
    drivers/mmc/host/mmc_spi.c | 4 +-
    drivers/mmc/host/of_mmc_spi.c | 147 +++++++++++++++++++++++++++++++++++++++++
    include/linux/spi/mmc_spi.h | 8 ++
    4 files changed, 161 insertions(+), 1 deletions(-)
    create mode 100644 drivers/mmc/host/of_mmc_spi.c

    diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
    index c794cc5..f485328 100644
    --- a/drivers/mmc/host/Makefile
    +++ b/drivers/mmc/host/Makefile
    @@ -19,6 +19,9 @@ obj-$(CONFIG_MMC_AT91) += at91_mci.o
    obj-$(CONFIG_MMC_ATMELMCI) += atmel-mci.o
    obj-$(CONFIG_MMC_TIFM_SD) += tifm_sd.o
    obj-$(CONFIG_MMC_SPI) += mmc_spi.o
    +ifeq ($(CONFIG_OF),y)
    +obj-$(CONFIG_MMC_SPI) += of_mmc_spi.o
    +endif
    obj-$(CONFIG_MMC_S3C) += s3cmci.o
    obj-$(CONFIG_MMC_SDRICOH_CS) += sdricoh_cs.o
    obj-$(CONFIG_MMC_TMIO) += tmio_mmc.o
    diff --git a/drivers/mmc/host/mmc_spi.c b/drivers/mmc/host/mmc_spi.c
    index 07faf54..416a0e3 100644
    --- a/drivers/mmc/host/mmc_spi.c
    +++ b/drivers/mmc/host/mmc_spi.c
    @@ -1285,7 +1285,7 @@ static int mmc_spi_probe(struct spi_device *spi)
    /* Platform data is used to hook up things like card sensing
    * and power switching gpios.
    */
    - host->pdata = spi->dev.platform_data;
    + host->pdata = mmc_spi_get_pdata(spi);
    if (host->pdata)
    mmc->ocr_avail = host->pdata->ocr_mask;
    if (!mmc->ocr_avail) {
    @@ -1368,6 +1368,7 @@ fail_glue_init:

    fail_nobuf1:
    mmc_free_host(mmc);
    + mmc_spi_put_pdata(spi);
    dev_set_drvdata(&spi->dev, NULL);

    nomem:
    @@ -1402,6 +1403,7 @@ static int __devexit mmc_spi_remove(struct spi_device *spi)

    spi->max_speed_hz = mmc->f_max;
    mmc_free_host(mmc);
    + mmc_spi_put_pdata(spi);
    dev_set_drvdata(&spi->dev, NULL);
    }
    return 0;
    diff --git a/drivers/mmc/host/of_mmc_spi.c b/drivers/mmc/host/of_mmc_spi.c
    new file mode 100644
    index 0000000..b5e5555
    --- /dev/null
    +++ b/drivers/mmc/host/of_mmc_spi.c
    @@ -0,0 +1,147 @@
    +/*
    + * OpenFirmware bindings for the MMC-over-SPI driver
    + *
    + * Copyright (c) MontaVista Software, Inc. 2008.
    + *
    + * Author: Anton Vorontsov
    + *
    + * This program is free software; you can redistribute it and/or modify it
    + * under the terms of the GNU General Public License as published by the
    + * Free Software Foundation; either version 2 of the License, or (at your
    + * option) any later version.
    + */
    +
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +#include
    +
    +enum {
    + CD_GPIO = 0,
    + WP_GPIO,
    + NUM_GPIOS,
    +};
    +
    +struct of_mmc_spi {
    + int gpios[NUM_GPIOS];
    + bool alow_gpios[NUM_GPIOS];
    + struct mmc_spi_platform_data pdata;
    +};
    +
    +static struct of_mmc_spi *to_of_mmc_spi(struct device *dev)
    +{
    + return container_of(dev->platform_data, struct of_mmc_spi, pdata);
    +}
    +
    +static int of_mmc_spi_read_gpio(struct device *dev, int gpio_num)
    +{
    + struct of_mmc_spi *oms = to_of_mmc_spi(dev);
    + const bool active_low = oms->alow_gpios[gpio_num];
    + const bool value = gpio_get_value(oms->gpios[gpio_num]);
    +
    + return active_low ^ value;
    +}
    +
    +static int of_mmc_spi_get_cd(struct device *dev)
    +{
    + return of_mmc_spi_read_gpio(dev, CD_GPIO);
    +}
    +
    +static int of_mmc_spi_get_ro(struct device *dev)
    +{
    + return of_mmc_spi_read_gpio(dev, WP_GPIO);
    +}
    +
    +struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi)
    +{
    + struct device *dev = &spi->dev;
    + struct device_node *np = dev_archdata_get_node(&dev->archdata);
    + struct of_mmc_spi *oms;
    + const u32 *voltage_ranges;
    + int num_ranges;
    + int i;
    + int ret = -EINVAL;
    +
    + if (dev->platform_data || !np)
    + return dev->platform_data;
    +
    + oms = kzalloc(sizeof(*oms), GFP_KERNEL);
    + if (!oms)
    + return NULL;
    +
    + voltage_ranges = of_get_property(np, "voltage-ranges", &num_ranges);
    + num_ranges = num_ranges / sizeof(*voltage_ranges) / 2;
    + if (!voltage_ranges || !num_ranges) {
    + dev_err(dev, "OF: voltage-ranges unspecified\n");
    + goto err_ocr;
    + }
    +
    + for (i = 0; i < num_ranges; i++) {
    + const int j = i * 2;
    +
    + ret = mmc_vddrange_to_ocrmask(voltage_ranges[j],
    + voltage_ranges[j + 1],
    + &oms->pdata.ocr_mask);
    + if (ret) {
    + dev_err(dev, "OF: voltage-range #%d is invalid\n", i);
    + goto err_ocr;
    + }
    + }
    +
    + for (i = 0; i < ARRAY_SIZE(oms->gpios); i++) {
    + enum of_gpio_flags gpio_flags;
    +
    + oms->gpios[i] = of_get_gpio(np, i, &gpio_flags);
    + if (!gpio_is_valid(oms->gpios[i]))
    + continue;
    +
    + ret = gpio_request(oms->gpios[i], dev->bus_id);
    + if (ret < 0) {
    + oms->gpios[i] = -EINVAL;
    + continue;
    + }
    +
    + if (gpio_flags & OF_GPIO_ACTIVE_LOW)
    + oms->alow_gpios[i] = true;
    + }
    +
    + if (gpio_is_valid(oms->gpios[CD_GPIO]))
    + oms->pdata.get_cd = of_mmc_spi_get_cd;
    + if (gpio_is_valid(oms->gpios[CD_GPIO]))
    + oms->pdata.get_ro = of_mmc_spi_get_ro;
    +
    + /* We don't support interrupts yet, let's poll. */
    + oms->pdata.caps |= MMC_CAP_NEEDS_POLL;
    +
    + dev->platform_data = &oms->pdata;
    + return dev->platform_data;
    +err_ocr:
    + kfree(oms);
    + return NULL;
    +}
    +EXPORT_SYMBOL(mmc_spi_get_pdata);
    +
    +void mmc_spi_put_pdata(struct spi_device *spi)
    +{
    + struct device *dev = &spi->dev;
    + struct device_node *np = dev_archdata_get_node(&dev->archdata);
    + struct of_mmc_spi *oms = to_of_mmc_spi(dev);
    + int i;
    +
    + if (!dev->platform_data || !np)
    + return;
    +
    + for (i = 0; i < ARRAY_SIZE(oms->gpios); i++) {
    + if (gpio_is_valid(oms->gpios[i]))
    + gpio_free(oms->gpios[i]);
    + }
    + kfree(oms);
    + dev->platform_data = NULL;
    +}
    +EXPORT_SYMBOL(mmc_spi_put_pdata);
    diff --git a/include/linux/spi/mmc_spi.h b/include/linux/spi/mmc_spi.h
    index a3626ae..87ee555 100644
    --- a/include/linux/spi/mmc_spi.h
    +++ b/include/linux/spi/mmc_spi.h
    @@ -41,4 +41,12 @@ struct mmc_spi_platform_data {
    void (*setpower)(struct device *, unsigned int maskval);
    };

    +#ifdef CONFIG_OF
    +extern struct mmc_spi_platform_data *mmc_spi_get_pdata(struct spi_device *spi);
    +extern void mmc_spi_put_pdata(struct spi_device *spi);
    +#else
    +#define mmc_spi_get_pdata(spi) ((spi)->dev.platform_data)
    +#define mmc_spi_put_pdata(spi)
    +#endif /* CONFIG_OF */
    +
    #endif /* __LINUX_SPI_MMC_SPI_H */
    --
    1.5.6.3
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. [PATCH 1/3] powerpc: Add mmc-spi-slot bindings

    The bindings describes a case where MMC/SD/SDIO slot directly connected
    to a SPI bus. Such setups are widely used on embedded PowerPC boards.

    The patch also adds the mmc-spi-slot entry to the OpenFirmware modalias
    table.

    Signed-off-by: Anton Vorontsov
    ---
    .../powerpc/dts-bindings/mmc-spi-slot.txt | 23 ++++++++++++++++++++
    drivers/of/base.c | 1 +
    2 files changed, 24 insertions(+), 0 deletions(-)
    create mode 100644 Documentation/powerpc/dts-bindings/mmc-spi-slot.txt

    diff --git a/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt b/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt
    new file mode 100644
    index 0000000..c39ac28
    --- /dev/null
    +++ b/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt
    @@ -0,0 +1,23 @@
    +MMC/SD/SDIO slot directly connected to a SPI bus
    +
    +Required properties:
    +- compatible : should be "mmc-spi-slot".
    +- reg : should specify SPI address (chip-select number).
    +- spi-max-frequency : maximum frequency for this device (Hz).
    +- voltage-ranges : two cells are required, first cell specifies minimum
    + slot voltage (mV), second cell specifies maximum slot voltage (mV).
    + Several ranges could be specified.
    +- gpios : (optional) may specify GPIOs in this order: Card-Detect GPIO,
    + Write-Protect GPIO.
    +
    +Example:
    +
    + mmc-slot@0 {
    + compatible = "fsl,mpc8323rdb-mmc-slot",
    + "mmc-spi-slot";
    + reg = <0>;
    + gpios = <&qe_pio_d 14 1
    + &qe_pio_d 15 0>;
    + voltage-ranges = <3300 3300>;
    + spi-max-frequency = <50000000>;
    + };
    diff --git a/drivers/of/base.c b/drivers/of/base.c
    index 7c79e94..c6797ca 100644
    --- a/drivers/of/base.c
    +++ b/drivers/of/base.c
    @@ -411,6 +411,7 @@ struct of_modalias_table {
    };
    static struct of_modalias_table of_modalias_table[] = {
    { "fsl,mcu-mpc8349emitx", "mcu-mpc8349emitx" },
    + { "mmc-spi-slot", "mmc_spi" },
    };

    /**
    --
    1.5.6.3

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 1/3] powerpc: Add mmc-spi-slot bindings

    On Thu, Oct 30, 2008 at 1:56 PM, Anton Vorontsov
    wrote:
    > The bindings describes a case where MMC/SD/SDIO slot directly connected
    > to a SPI bus. Such setups are widely used on embedded PowerPC boards.
    >
    > The patch also adds the mmc-spi-slot entry to the OpenFirmware modalias
    > table.
    >
    > Signed-off-by: Anton Vorontsov


    Mostly looks good to me. A few comments below.

    > ---
    > .../powerpc/dts-bindings/mmc-spi-slot.txt | 23 ++++++++++++++++++++
    > drivers/of/base.c | 1 +
    > 2 files changed, 24 insertions(+), 0 deletions(-)
    > create mode 100644 Documentation/powerpc/dts-bindings/mmc-spi-slot.txt
    >
    > diff --git a/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt b/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt
    > new file mode 100644
    > index 0000000..c39ac28
    > --- /dev/null
    > +++ b/Documentation/powerpc/dts-bindings/mmc-spi-slot.txt
    > @@ -0,0 +1,23 @@
    > +MMC/SD/SDIO slot directly connected to a SPI bus
    > +
    > +Required properties:
    > +- compatible : should be "mmc-spi-slot".
    > +- reg : should specify SPI address (chip-select number).
    > +- spi-max-frequency : maximum frequency for this device (Hz).
    > +- voltage-ranges : two cells are required, first cell specifies minimum
    > + slot voltage (mV), second cell specifies maximum slot voltage (mV).
    > + Several ranges could be specified.
    > +- gpios : (optional) may specify GPIOs in this order: Card-Detect GPIO,
    > + Write-Protect GPIO.


    I wonder if we're following the example of irq mappings too closely
    for the gpios property. I like the layout of the property
    ( ), but I think the 'gpios' name is getting
    too overloaded. In this case a single property 'gpios' is being used
    to encode 2 unrelated bits of information; the write protect pin and
    the card detect pins.

    In this particular case I think it is better to use 2 properties in
    this case; something like 'spi-writeprotect-gpio' and
    'spi-carddetect-gpio' using the same specifier format. Doing so adds
    a bit more clarity to the purpose of the properties.

    I my mind I differentiate this from other examples (for instance a
    series of CS pins) based on how closely related the pin functions are.
    So I would say for the following examples...
    1) GPIO data bus (SPI, MDIO and I2C are great examples); all pins must
    be present - single gpio property
    2) This MMC case (pins are optional and unrelated); separate gpio properties
    3) LCD with backlight and contrast control pins; one gpio property for
    backlight pins, one for constrast pins.

    Thoughts?

    > +
    > +Example:
    > +
    > + mmc-slot@0 {
    > + compatible = "fsl,mpc8323rdb-mmc-slot",
    > + "mmc-spi-slot";
    > + reg = <0>;
    > + gpios = <&qe_pio_d 14 1
    > + &qe_pio_d 15 0>;
    > + voltage-ranges = <3300 3300>;
    > + spi-max-frequency = <50000000>;
    > + };
    > diff --git a/drivers/of/base.c b/drivers/of/base.c
    > index 7c79e94..c6797ca 100644
    > --- a/drivers/of/base.c
    > +++ b/drivers/of/base.c
    > @@ -411,6 +411,7 @@ struct of_modalias_table {
    > };
    > static struct of_modalias_table of_modalias_table[] = {
    > { "fsl,mcu-mpc8349emitx", "mcu-mpc8349emitx" },
    > + { "mmc-spi-slot", "mmc_spi" },
    > };
    >
    > /**
    > --
    > 1.5.6.3
    >
    >




    --
    Grant Likely, B.Sc., P.Eng.
    Secret Lab Technologies Ltd.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH 1/3] powerpc: Add mmc-spi-slot bindings

    On Thu, Oct 30, 2008 at 02:37:31PM -0600, Grant Likely wrote:
    [...]
    > > +- gpios : (optional) may specify GPIOs in this order: Card-Detect GPIO,
    > > + Write-Protect GPIO.

    >
    > I wonder if we're following the example of irq mappings too closely
    > for the gpios property. I like the layout of the property
    > ( ), but I think the 'gpios' name is getting
    > too overloaded. In this case a single property 'gpios' is being used
    > to encode 2 unrelated bits of information; the write protect pin and
    > the card detect pins.
    >
    > In this particular case I think it is better to use 2 properties in
    > this case; something like 'spi-writeprotect-gpio' and
    > 'spi-carddetect-gpio' using the same specifier format. Doing so adds
    > a bit more clarity to the purpose of the properties.
    >
    > I my mind I differentiate this from other examples (for instance a
    > series of CS pins) based on how closely related the pin functions are.
    > So I would say for the following examples...
    > 1) GPIO data bus (SPI, MDIO and I2C are great examples); all pins must
    > be present - single gpio property
    > 2) This MMC case (pins are optional and unrelated); separate gpio properties
    > 3) LCD with backlight and contrast control pins; one gpio property for
    > backlight pins, one for constrast pins.
    >
    > Thoughts?


    It's pretty trivial to implement (of_get_named_gpio() -- could be just
    factored out of of_get_gpio()).

    Though,

    1. The idea is quite extreme. It needs discussion, and furthermore,
    we need to define when do we use gpios = <> and when something-gpio =
    <>; We need to be consistent, and to be consistent, the rules should
    be clear and written.

    2. We should think about it very very carefully. Do we want to lose the
    track of gpios? For example, there are quite defined rules when (and
    in what properties) you may encounter memory addresses, when and
    where you can encounter interrupt specifiers. We do the same for
    gpios, and so far it works great. We need to think about any possible
    drawbacks of the scheme you purpose (we would never know where to
    expect gpios - it isn't a problem per se, but maybe it could lead
    to some problem in future? I don't know.)

    Quite honestly I don't like the idea... maybe I just used to
    interrupts = <>, reg = <>, ranges = <>, interrupt-map = <> and so
    forth, and now my subconsciousness tells me "it's wrong to do
    something-interrupt = <> stuff." ;-)

    Anyway, your proposal is forward and backward compatible with the
    existing scheme, and can even coexist. Thus I'd prefer to stay with
    the today's gpios = <>. We can always start use the new scheme when
    it will be thought out enough.

    Thanks,

    p.s. I'd prefer a new thread for this discussion, somewhere
    in devicetree-discuss@ozlabs.org, so that it won't relate to this
    particular patch.

    --
    Anton Vorontsov
    email: cbouatmailru@gmail.com
    irc://irc.freenode.net/bd2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH 1/3] powerpc: Add mmc-spi-slot bindings

    On Fri, Oct 31, 2008 at 02:02:53AM +0300, Anton Vorontsov wrote:
    [...]
    > It's pretty trivial to implement (of_get_named_gpio() -- could be just
    > factored out of of_get_gpio()).
    >
    > Though,
    >
    > 1. The idea is quite extreme. It needs discussion, and furthermore,
    > we need to define when do we use gpios = <> and when something-gpio =
    > <>; We need to be consistent, and to be consistent, the rules should
    > be clear and written.
    >
    > 2. We should think about it very very carefully. Do we want to lose the
    > track of gpios? For example, there are quite defined rules when (and
    > in what properties) you may encounter memory addresses, when and
    > where you can encounter interrupt specifiers. We do the same for
    > gpios, and so far it works great. We need to think about any possible
    > drawbacks of the scheme you purpose (we would never know where to
    > expect gpios - it isn't a problem per se, but maybe it could lead
    > to some problem in future? I don't know.)
    >
    > Quite honestly I don't like the idea... maybe I just used to
    > interrupts = <>, reg = <>, ranges = <>, interrupt-map = <> and so
    > forth, and now my subconsciousness tells me "it's wrong to do
    > something-interrupt = <> stuff." ;-)


    Btw, not that I hate this new scheme, sometimes the scheme is even
    inevitable. For example when we have gpios with two or more ellipsis:
    gpios = <... ...>.

    But this should be a separate discussion, really.

    --
    Anton Vorontsov
    email: cbouatmailru@gmail.com
    irc://irc.freenode.net/bd2
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH 1/3] powerpc: Add mmc-spi-slot bindings

    On Fri, Oct 31, 2008 at 02:02:53AM +0300, Anton Vorontsov wrote:
    > On Thu, Oct 30, 2008 at 02:37:31PM -0600, Grant Likely wrote:
    > [...]
    > > > +- gpios : (optional) may specify GPIOs in this order: Card-Detect GPIO,
    > > > + Write-Protect GPIO.

    > >
    > > I wonder if we're following the example of irq mappings too closely
    > > for the gpios property. I like the layout of the property
    > > ( ), but I think the 'gpios' name is getting
    > > too overloaded. In this case a single property 'gpios' is being used
    > > to encode 2 unrelated bits of information; the write protect pin and
    > > the card detect pins.
    > >
    > > In this particular case I think it is better to use 2 properties in
    > > this case; something like 'spi-writeprotect-gpio' and
    > > 'spi-carddetect-gpio' using the same specifier format. Doing so adds
    > > a bit more clarity to the purpose of the properties.
    > >
    > > I my mind I differentiate this from other examples (for instance a
    > > series of CS pins) based on how closely related the pin functions are.
    > > So I would say for the following examples...
    > > 1) GPIO data bus (SPI, MDIO and I2C are great examples); all pins must
    > > be present - single gpio property
    > > 2) This MMC case (pins are optional and unrelated); separate gpio properties
    > > 3) LCD with backlight and contrast control pins; one gpio property for
    > > backlight pins, one for constrast pins.
    > >
    > > Thoughts?

    >
    > It's pretty trivial to implement (of_get_named_gpio() -- could be just
    > factored out of of_get_gpio()).
    >
    > Though,
    >
    > 1. The idea is quite extreme. It needs discussion, and furthermore,
    > we need to define when do we use gpios = <> and when something-gpio =
    > <>; We need to be consistent, and to be consistent, the rules should
    > be clear and written.
    >
    > 2. We should think about it very very carefully. Do we want to lose the
    > track of gpios? For example, there are quite defined rules when (and
    > in what properties) you may encounter memory addresses, when and
    > where you can encounter interrupt specifiers. We do the same for
    > gpios, and so far it works great. We need to think about any possible
    > drawbacks of the scheme you purpose (we would never know where to
    > expect gpios - it isn't a problem per se, but maybe it could lead
    > to some problem in future? I don't know.)
    >
    > Quite honestly I don't like the idea... maybe I just used to
    > interrupts = <>, reg = <>, ranges = <>, interrupt-map = <> and so
    > forth, and now my subconsciousness tells me "it's wrong to do
    > something-interrupt = <> stuff." ;-)


    Fwiw, I agree. The current scheme works, adding new places to look
    for gpio specifiers will just complexify things. Long lists of gpios
    may be somewhat awkward to work with, but I don't think it's
    sufficiently bad to warrant another scheme.

    --
    David Gibson | I'll have my music baroque, and my code
    david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
    | _way_ _around_!
    http://www.ozlabs.org/~dgibson
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH 0/3 RFC] MMC SPI support for OpenFirmware platforms

    On Thu, 30 Oct 2008 22:55:46 +0300
    Anton Vorontsov wrote:

    >
    > Pierre, the approach is somewhat similar to this one:
    > http://lkml.org/lkml/2008/5/26/135
    > Posted few months ago.
    >
    > I know you don't like it, but I ask you to reconsider it. The
    > I2C and SPI cases are similar, and recently we tried to write
    > bindings for some I2C GPIO controllers.
    >


    This new version is a bit better in that you've generalised thing more.
    I'd still prefer if we can have an interface where the driver doesn't
    have to know that it is on an ACPI/OF/EFI/whatnot host, but I can live
    with this model for now.

    Rgds
    --
    -- Pierre Ossman

    Linux kernel, MMC maintainer http://www.kernel.org
    rdesktop, core developer http://www.rdesktop.org

    WARNING: This correspondence is being monitored by the
    Swedish government. Make sure your server uses encryption
    for SMTP traffic and consider using PGP for end-to-end
    encryption.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [PATCH 2/3] mmc: Add mmc_vddrange_to_ocrmask() helper function

    On Thu, 30 Oct 2008 22:56:32 +0300
    Anton Vorontsov wrote:

    > +/**
    > + * mmc_vddrange_to_ocrmask - Convert a voltage range to the OCR mask
    > + * @vdd_min: minimum voltage value (mV)
    > + * @vdd_max: maximum voltage value (mV)
    > + * @mask: pointer to the mask
    > + *


    Why the pointer? Why not let the caller handle the aggregation? That
    would be a lot safer.

    > + /* fill the mask, from max bit to min bit */
    > + while (vdd_max >= vdd_min)
    > + *mask |= 1 << vdd_max--;
    > + return 0;


    Many cards get a bit uppity with a single bit set. If possible, try to
    make this function set two bits when the voltage is right on the
    boundary (e.g. 3.3V).

    Rgds
    --
    -- Pierre Ossman

    Linux kernel, MMC maintainer http://www.kernel.org
    rdesktop, core developer http://www.rdesktop.org

    WARNING: This correspondence is being monitored by the
    Swedish government. Make sure your server uses encryption
    for SMTP traffic and consider using PGP for end-to-end
    encryption.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread