All of lore.kernel.org
 help / color / mirror / Atom feed
From: cavokz@gmail.com (Domenico Andreoli)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] ARM: add DT support to the S3C SDI driver
Date: Wed, 13 Apr 2011 22:52:04 +0200	[thread overview]
Message-ID: <20110413205204.GA3390@dandreoli.com> (raw)
In-Reply-To: <20110413173558.GF2254@ponder.secretlab.ca>

On Wed, Apr 13, 2011 at 11:35:58AM -0600, Grant Likely wrote:
> On Wed, Apr 13, 2011 at 07:25:47PM +0200, Domenico Andreoli wrote:
> >
> > set_power() instead deserves some more considerations. Current
> > implementations boil down to gpio on/off settings, its DT implementation
> > is again straightforward. Problem would arise if any board requires
> > some naive implementation, would it require a different of_device_id
> > with proper .data set?
>
> It would be better to think out the binding a bit more, but I'd need
> to know more details about what is possible.

I looked at the arm tree only, MMC/MCI/SDI area. There are some
.set_power ops also on the LCD side and who know what else with a
different ops name.

The most common case is no .set_power op.

Then we have quite a lot of GPIO cases like this:

static void h1940_set_mmc_power(unsigned char power_mode, unsigned short vdd)
{
    switch (power_mode) {
    case MMC_POWER_OFF:
        gpio_set_value(H1940_LATCH_SD_POWER, 0);
        break;
    case MMC_POWER_UP:
    case MMC_POWER_ON:
        gpio_set_value(H1940_LATCH_SD_POWER, 1);
        break;
    default:
        break;
    };
}

and its derivative:

static int mmc_set_power(struct device *dev, int slot, int power_on, int vdd)
{
    gpio_set_value(H2_TPS_GPIO_MMC_PWR_EN, power_on);
    return 0;
}

then we have a bitwise case on some register (the fpga name is
misleading, they are just __raw_readb/__raw_writeb):

static int mmc_set_power(struct device *dev, int slot, int power_on, int vdd)
{
    if (power_on)
        fpga_write(fpga_read(OMAP1510_FPGA_POWER) | (1 << 3), OMAP1510_FPGA_POWER);
    else
        fpga_write(fpga_read(OMAP1510_FPGA_POWER) & ~(1 << 3), OMAP1510_FPGA_POWER);

    return 0;
}

a i2c bitwise case:

static int mmc_set_power(struct device *dev, int slot, int power_on, int vdd)
{
    int err;
    u8 dat = 0;

    err = sx1_i2c_read_byte(SOFIA_I2C_ADDR, SOFIA_POWER1_REG, &dat);
    if (err < 0)
        return err;

    if (power_on)
        dat |= SOFIA_MMC_POWER;
    else
        dat &= ~SOFIA_MMC_POWER;

    return sx1_i2c_write_byte(SOFIA_I2C_ADDR, SOFIA_POWER1_REG, dat);
}

and some other very special cases such as:

static int n8x0_mmc_set_power(struct device *dev, int slot, int power_on, int vdd)
{
    if (machine_is_nokia_n800() || slot == 0)
        return n8x0_mmc_set_power_menelaus(dev, slot, power_on, vdd);

    n810_set_power_emmc(dev, power_on);

    return 0;
}

static int n8x0_mmc_set_power_menelaus(struct device *dev, int slot, int power_on, int vdd)
{
    int mV; 

#ifdef CONFIG_MMC_DEBUG
    dev_dbg(dev, "Set slot %d power: %s (vdd %d)\n", slot + 1,
        power_on ? "on" : "off", vdd);
#endif
    if (slot == 0) {
        if (!power_on)
            return menelaus_set_vmmc(0);
        switch (1 << vdd) {
        case MMC_VDD_33_34:
        case MMC_VDD_32_33:
        case MMC_VDD_31_32:
            mV = 3100;
            break;
        case MMC_VDD_30_31:
            mV = 3000;
            break;
        case MMC_VDD_28_29:
            mV = 2800;
            break;
        case MMC_VDD_165_195:
            mV = 1850;
            break;
        default:
            BUG();
        }   
        return menelaus_set_vmmc(mV);
    } else {
        if (!power_on)
            return menelaus_set_vdcdc(3, 0); 
        switch (1 << vdd) {
        case MMC_VDD_33_34:
        case MMC_VDD_32_33:
            mV = 3300;
            break;
        case MMC_VDD_30_31:
        case MMC_VDD_29_30:
            mV = 3000;
            break;
        case MMC_VDD_28_29:
        case MMC_VDD_27_28:
            mV = 2800;
            break;
        case MMC_VDD_24_25:
        case MMC_VDD_23_24:
            mV = 2400;
            break;
        case MMC_VDD_22_23:
        case MMC_VDD_21_22:
            mV = 2200;
            break;
        case MMC_VDD_20_21:
            mV = 2000;
            break;
        case MMC_VDD_165_195:
            mV = 1800;
            break;
        default:
            BUG();
        }   
        return menelaus_set_vdcdc(3, mV);
    }   
    return 0;
}

static void n810_set_power_emmc(struct device *dev, int power_on)
{
    dev_dbg(dev, "Set EMMC power %s\n", power_on ? "on" : "off");

    if (power_on) {
        gpio_set_value(N810_EMMC_VSD_GPIO, 1);
        msleep(1);
        gpio_set_value(N810_EMMC_VIO_GPIO, 1);
        msleep(1);
    } else {
        gpio_set_value(N810_EMMC_VIO_GPIO, 0);
        msleep(50);
        gpio_set_value(N810_EMMC_VSD_GPIO, 0);
        msleep(50);
    }
}

the first three are straightforward, the i2c could be tricky but very
nicely expressed in the DT world, the others are so specific that I
doubt they could be re-used on any other board, so I don't see the gain
to port them to DT.

When I say port, I mean "import the interface" concept, of course :)

my two cents....

cheers,
Domenico

-----[ Domenico Andreoli, aka cavok
 --[ http://www.dandreoli.com/gpgkey.asc
   ---[ 3A0F 2F80 F79C 678A 8936  4FEE 0677 9033 A20E BC50

WARNING: multiple messages have this Message-ID (diff)
From: Domenico Andreoli <cavokz-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH] ARM: add DT support to the S3C SDI driver
Date: Wed, 13 Apr 2011 22:52:04 +0200	[thread overview]
Message-ID: <20110413205204.GA3390@dandreoli.com> (raw)
In-Reply-To: <20110413173558.GF2254-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>

On Wed, Apr 13, 2011 at 11:35:58AM -0600, Grant Likely wrote:
> On Wed, Apr 13, 2011 at 07:25:47PM +0200, Domenico Andreoli wrote:
> >
> > set_power() instead deserves some more considerations. Current
> > implementations boil down to gpio on/off settings, its DT implementation
> > is again straightforward. Problem would arise if any board requires
> > some naive implementation, would it require a different of_device_id
> > with proper .data set?
>
> It would be better to think out the binding a bit more, but I'd need
> to know more details about what is possible.

I looked at the arm tree only, MMC/MCI/SDI area. There are some
.set_power ops also on the LCD side and who know what else with a
different ops name.

The most common case is no .set_power op.

Then we have quite a lot of GPIO cases like this:

static void h1940_set_mmc_power(unsigned char power_mode, unsigned short vdd)
{
    switch (power_mode) {
    case MMC_POWER_OFF:
        gpio_set_value(H1940_LATCH_SD_POWER, 0);
        break;
    case MMC_POWER_UP:
    case MMC_POWER_ON:
        gpio_set_value(H1940_LATCH_SD_POWER, 1);
        break;
    default:
        break;
    };
}

and its derivative:

static int mmc_set_power(struct device *dev, int slot, int power_on, int vdd)
{
    gpio_set_value(H2_TPS_GPIO_MMC_PWR_EN, power_on);
    return 0;
}

then we have a bitwise case on some register (the fpga name is
misleading, they are just __raw_readb/__raw_writeb):

static int mmc_set_power(struct device *dev, int slot, int power_on, int vdd)
{
    if (power_on)
        fpga_write(fpga_read(OMAP1510_FPGA_POWER) | (1 << 3), OMAP1510_FPGA_POWER);
    else
        fpga_write(fpga_read(OMAP1510_FPGA_POWER) & ~(1 << 3), OMAP1510_FPGA_POWER);

    return 0;
}

a i2c bitwise case:

static int mmc_set_power(struct device *dev, int slot, int power_on, int vdd)
{
    int err;
    u8 dat = 0;

    err = sx1_i2c_read_byte(SOFIA_I2C_ADDR, SOFIA_POWER1_REG, &dat);
    if (err < 0)
        return err;

    if (power_on)
        dat |= SOFIA_MMC_POWER;
    else
        dat &= ~SOFIA_MMC_POWER;

    return sx1_i2c_write_byte(SOFIA_I2C_ADDR, SOFIA_POWER1_REG, dat);
}

and some other very special cases such as:

static int n8x0_mmc_set_power(struct device *dev, int slot, int power_on, int vdd)
{
    if (machine_is_nokia_n800() || slot == 0)
        return n8x0_mmc_set_power_menelaus(dev, slot, power_on, vdd);

    n810_set_power_emmc(dev, power_on);

    return 0;
}

static int n8x0_mmc_set_power_menelaus(struct device *dev, int slot, int power_on, int vdd)
{
    int mV; 

#ifdef CONFIG_MMC_DEBUG
    dev_dbg(dev, "Set slot %d power: %s (vdd %d)\n", slot + 1,
        power_on ? "on" : "off", vdd);
#endif
    if (slot == 0) {
        if (!power_on)
            return menelaus_set_vmmc(0);
        switch (1 << vdd) {
        case MMC_VDD_33_34:
        case MMC_VDD_32_33:
        case MMC_VDD_31_32:
            mV = 3100;
            break;
        case MMC_VDD_30_31:
            mV = 3000;
            break;
        case MMC_VDD_28_29:
            mV = 2800;
            break;
        case MMC_VDD_165_195:
            mV = 1850;
            break;
        default:
            BUG();
        }   
        return menelaus_set_vmmc(mV);
    } else {
        if (!power_on)
            return menelaus_set_vdcdc(3, 0); 
        switch (1 << vdd) {
        case MMC_VDD_33_34:
        case MMC_VDD_32_33:
            mV = 3300;
            break;
        case MMC_VDD_30_31:
        case MMC_VDD_29_30:
            mV = 3000;
            break;
        case MMC_VDD_28_29:
        case MMC_VDD_27_28:
            mV = 2800;
            break;
        case MMC_VDD_24_25:
        case MMC_VDD_23_24:
            mV = 2400;
            break;
        case MMC_VDD_22_23:
        case MMC_VDD_21_22:
            mV = 2200;
            break;
        case MMC_VDD_20_21:
            mV = 2000;
            break;
        case MMC_VDD_165_195:
            mV = 1800;
            break;
        default:
            BUG();
        }   
        return menelaus_set_vdcdc(3, mV);
    }   
    return 0;
}

static void n810_set_power_emmc(struct device *dev, int power_on)
{
    dev_dbg(dev, "Set EMMC power %s\n", power_on ? "on" : "off");

    if (power_on) {
        gpio_set_value(N810_EMMC_VSD_GPIO, 1);
        msleep(1);
        gpio_set_value(N810_EMMC_VIO_GPIO, 1);
        msleep(1);
    } else {
        gpio_set_value(N810_EMMC_VIO_GPIO, 0);
        msleep(50);
        gpio_set_value(N810_EMMC_VSD_GPIO, 0);
        msleep(50);
    }
}

the first three are straightforward, the i2c could be tricky but very
nicely expressed in the DT world, the others are so specific that I
doubt they could be re-used on any other board, so I don't see the gain
to port them to DT.

When I say port, I mean "import the interface" concept, of course :)

my two cents....

cheers,
Domenico

-----[ Domenico Andreoli, aka cavok
 --[ http://www.dandreoli.com/gpgkey.asc
   ---[ 3A0F 2F80 F79C 678A 8936  4FEE 0677 9033 A20E BC50

  reply	other threads:[~2011-04-13 20:52 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-13 17:25 [PATCH] ARM: add DT support to the S3C SDI driver Domenico Andreoli
2011-04-13 17:25 ` Domenico Andreoli
2011-04-13 17:33 ` Grant Likely
2011-04-13 17:33   ` Grant Likely
2011-04-13 17:35 ` Grant Likely
2011-04-13 17:35   ` Grant Likely
2011-04-13 20:52   ` Domenico Andreoli [this message]
2011-04-13 20:52     ` Domenico Andreoli
2011-04-14  8:51 ` Vasily Khoruzhick
2011-04-14  8:51   ` Vasily Khoruzhick
2011-04-14  9:37   ` Domenico Andreoli
2011-04-14  9:37     ` Domenico Andreoli
2011-05-04  1:04   ` Grant Likely
2011-05-04  1:04     ` Grant Likely
     [not found]     ` <20110504010448.GA4952-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2011-05-04  2:07       ` Domenico Andreoli
     [not found]         ` <BANLkTim9UT8epyp7ccFdKq6-uobU2h0wdA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-05-04  2:18           ` Grant Likely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110413205204.GA3390@dandreoli.com \
    --to=cavokz@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.