LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2 4/6] mtd: m25p80: add a read function to read page by page
From: Grant Likely @ 2010-08-10  7:00 UTC (permalink / raw)
  To: Mingkai Hu, David Woodhouse; +Cc: linuxppc-dev, kumar.gala, spi-devel-general
In-Reply-To: <1280735524-17547-5-git-send-email-Mingkai.hu@freescale.com>

[adding David Woodhouse]

(Btw, you should cc: David Woodhouse and the mtd list on the MTD patches).

On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu <Mingkai.hu@freescale.com> wrote=
:
> For Freescale's eSPI controller, the max transaction length one time
> is limitted by the SPCOM[TRANSLEN] field which is 0xFFFF. When used
> mkfs.ext2 command to create ext2 filesystem on the flash, the read
> length will exceed the max value of the SPCOM[TRANSLEN] field, so
> change the read function to read page by page.
>
> For other SPI flash driver, also needed to supply the read function
> if used the eSPI controller.
>
> Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>

This one bothers me, but I can't put my finger on it.  The flag feels
like a controller specific hack.  David, can you take a look at this
patch?

g.

> ---
>
> v2:
> =A0- Add SPI_MASTER_TRANS_LIMIT flag to indicate the master's trans lengt=
h
> =A0 limitation, so the MTD driver can select the correct transfer behavio=
ur
> =A0 at driver probe time
>
> =A0drivers/mtd/devices/m25p80.c | =A0 78 ++++++++++++++++++++++++++++++++=
++++++++++
> =A0drivers/spi/spi_fsl_espi.c =A0 | =A0 =A01 +
> =A0include/linux/spi/spi.h =A0 =A0 =A0| =A0 =A01 +
> =A03 files changed, 80 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 5f00075..30e4568 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -376,6 +376,81 @@ static int m25p80_read(struct mtd_info *mtd, loff_t =
from, size_t len,
> =A0}
>
> =A0/*
> + * Read an address range from the flash chip page by page.
> + * Some controller has transaction length limitation such as the
> + * Freescale's eSPI controller can only trasmit 0xFFFF bytes one
> + * time, so we have to read page by page if the len is more than
> + * the limitation.
> + */
> +static int m25p80_page_read(struct mtd_info *mtd, loff_t from, size_t le=
n,
> + =A0 =A0 =A0 size_t *retlen, u_char *buf)
> +{
> + =A0 =A0 =A0 struct m25p *flash =3D mtd_to_m25p(mtd);
> + =A0 =A0 =A0 struct spi_transfer t[2];
> + =A0 =A0 =A0 struct spi_message m;
> + =A0 =A0 =A0 u32 i, page_size =3D 0;
> +
> + =A0 =A0 =A0 DEBUG(MTD_DEBUG_LEVEL2, "%s: %s %s 0x%08x, len %zd\n",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_name(&flash->spi->dev),=
 __func__, "from",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 (u32)from, len);
> +
> + =A0 =A0 =A0 /* sanity checks */
> + =A0 =A0 =A0 if (!len)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 0;
> +
> + =A0 =A0 =A0 if (from + len > flash->mtd.size)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return -EINVAL;
> +
> + =A0 =A0 =A0 spi_message_init(&m);
> + =A0 =A0 =A0 memset(t, 0, (sizeof t));
> +
> + =A0 =A0 =A0 /* NOTE:
> + =A0 =A0 =A0 =A0* OPCODE_FAST_READ (if available) is faster.
> + =A0 =A0 =A0 =A0* Should add 1 byte DUMMY_BYTE.
> + =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 t[0].tx_buf =3D flash->command;
> + =A0 =A0 =A0 t[0].len =3D m25p_cmdsz(flash) + FAST_READ_DUMMY_BYTE;
> + =A0 =A0 =A0 spi_message_add_tail(&t[0], &m);
> +
> + =A0 =A0 =A0 t[1].rx_buf =3D buf;
> + =A0 =A0 =A0 spi_message_add_tail(&t[1], &m);
> +
> + =A0 =A0 =A0 /* Byte count starts at zero. */
> + =A0 =A0 =A0 if (retlen)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 *retlen =3D 0;
> +
> + =A0 =A0 =A0 mutex_lock(&flash->lock);
> +
> + =A0 =A0 =A0 /* Wait till previous write/erase is done. */
> + =A0 =A0 =A0 if (wait_till_ready(flash)) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* REVISIT status return?? */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mutex_unlock(&flash->lock);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return 1;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 /* Set up the write data buffer. */
> + =A0 =A0 =A0 flash->command[0] =3D OPCODE_READ;
> +
> + =A0 =A0 =A0 for (i =3D page_size; i < len; i +=3D page_size) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 page_size =3D len - i;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (page_size > flash->page_size)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 page_size =3D flash->page_s=
ize;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 m25p_addr2cmd(flash, from + i, flash->comma=
nd);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 t[1].len =3D page_size;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 t[1].rx_buf =3D buf + i;
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 spi_sync(flash->spi, &m);
> +
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 *retlen +=3D m.actual_length - m25p_cmdsz(f=
lash)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 - FAST_READ_DUMMY_BYTE;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 mutex_unlock(&flash->lock);
> +
> + =A0 =A0 =A0 return 0;
> +}
> +
> +/*
> =A0* Write an address range to the flash chip. =A0Data must be written in
> =A0* FLASH_PAGESIZE chunks. =A0The address range may be any size provided
> =A0* it is within the physical boundaries.
> @@ -877,6 +952,9 @@ static int __devinit m25p_probe(struct spi_device *sp=
i)
> =A0 =A0 =A0 =A0flash->mtd.erase =3D m25p80_erase;
> =A0 =A0 =A0 =A0flash->mtd.read =3D m25p80_read;
>
> + =A0 =A0 =A0 if (spi->master->flags & SPI_MASTER_TRANS_LIMIT)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 flash->mtd.read =3D m25p80_page_read;
> +
> =A0 =A0 =A0 =A0/* sst flash chips use AAI word program */
> =A0 =A0 =A0 =A0if (info->jedec_id >> 16 =3D=3D 0xbf)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0flash->mtd.write =3D sst_write;
> diff --git a/drivers/spi/spi_fsl_espi.c b/drivers/spi/spi_fsl_espi.c
> index 61987cf..e15b7dc 100644
> --- a/drivers/spi/spi_fsl_espi.c
> +++ b/drivers/spi/spi_fsl_espi.c
> @@ -470,6 +470,7 @@ static struct spi_master * __devinit fsl_espi_probe(s=
truct device *dev,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto err_probe;
>
> =A0 =A0 =A0 =A0master->setup =3D fsl_espi_setup;
> + =A0 =A0 =A0 master->flags =3D SPI_MASTER_TRANS_LIMIT;
>
> =A0 =A0 =A0 =A0mpc8xxx_spi =3D spi_master_get_devdata(master);
> =A0 =A0 =A0 =A0mpc8xxx_spi->spi_do_one_msg =3D fsl_espi_do_one_msg;
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index af56071..0729cbd 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -261,6 +261,7 @@ struct spi_master {
> =A0#define SPI_MASTER_HALF_DUPLEX BIT(0) =A0 =A0 =A0 =A0 =A0/* can't do f=
ull duplex */
> =A0#define SPI_MASTER_NO_RX =A0 =A0 =A0 BIT(1) =A0 =A0 =A0 =A0 =A0/* can'=
t do buffer read */
> =A0#define SPI_MASTER_NO_TX =A0 =A0 =A0 BIT(2) =A0 =A0 =A0 =A0 =A0/* can'=
t do buffer write */
> +#define SPI_MASTER_TRANS_LIMIT BIT(3) =A0 =A0 =A0 =A0 =A0/* have trans l=
ength limit */
>
> =A0 =A0 =A0 =A0/* Setup mode and clock, etc (spi driver may call many tim=
es).
> =A0 =A0 =A0 =A0 *
> --
> 1.6.4
>
>
>



--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH v2 3/6] mtd: m25p80: add support to parse the SPI flash's partitions
From: Grant Likely @ 2010-08-10  6:56 UTC (permalink / raw)
  To: Mingkai Hu; +Cc: linuxppc-dev, kumar.gala, spi-devel-general
In-Reply-To: <1280735524-17547-4-git-send-email-Mingkai.hu@freescale.com>

On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu <Mingkai.hu@freescale.com> wrote=
:
> Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
> ---
>
> v2:
> =A0- Move the flash partition function from of_spi.c to MTD driver
>
> =A0drivers/mtd/devices/m25p80.c | =A0 29 +++++++++++++++++++++++++++++
> =A01 files changed, 29 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mtd/devices/m25p80.c b/drivers/mtd/devices/m25p80.c
> index 81e49a9..5f00075 100644
> --- a/drivers/mtd/devices/m25p80.c
> +++ b/drivers/mtd/devices/m25p80.c
> @@ -752,6 +752,31 @@ static const struct spi_device_id *__devinit jedec_p=
robe(struct spi_device *spi)
> =A0 =A0 =A0 =A0return NULL;
> =A0}
>
> +/*
> + * parse_flash_partition - Parse the flash partition on the SPI bus
> + * @spi: Pointer to spi_device device
> + */
> +void parse_flash_partition(struct spi_device *spi)
> +{
> + =A0 =A0 =A0 struct mtd_partition *parts;
> + =A0 =A0 =A0 struct flash_platform_data *pdata;
> + =A0 =A0 =A0 int nr_parts =3D 0;
> + =A0 =A0 =A0 struct device_node *np =3D spi->dev.of_node;
> +
> + =A0 =A0 =A0 nr_parts =3D of_mtd_parse_partitions(&spi->dev, np, &parts)=
;
> + =A0 =A0 =A0 if (!nr_parts)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
> +
> + =A0 =A0 =A0 pdata =3D kzalloc(sizeof(*pdata), GFP_KERNEL);
> + =A0 =A0 =A0 if (!pdata)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
> +
> + =A0 =A0 =A0 pdata->parts =3D parts;
> + =A0 =A0 =A0 pdata->nr_parts =3D nr_parts;
> + =A0 =A0 =A0 spi->dev.platform_data =3D pdata;

The driver is not allowed to write to the platform_data pointer in the
device structure because it leaves an absolute mess in trying to
figure out who owns the pdata pointer.  Should it be freed when the
driver is unbound?  Is it statically allocated? etc.  There is a safer
way to get the data (see below).

> +
> + =A0 =A0 =A0 return;
> +}
>
> =A0/*
> =A0* board specific setup should have ensured the SPI clock used here
> @@ -771,6 +796,10 @@ static int __devinit m25p_probe(struct spi_device *s=
pi)
> =A0 =A0 =A0 =A0 * a chip ID, try the JEDEC id commands; they'll work for =
most
> =A0 =A0 =A0 =A0 * newer chips, even if we don't recognize the particular =
chip.
> =A0 =A0 =A0 =A0 */
> +
> + =A0 =A0 =A0 /* Parse the flash partition */
> + =A0 =A0 =A0 parse_flash_partition(spi);
> +

Doing this unconditionally is dangerous, and if a platform_data
structure is provided, then it must alwasy take precedence over the
device tree data (because the platform code has gone to extra lengths
to add a pdata structure; that must mean it is important!)  :-)

Instead what can be done is to call of_mtd_parse_partitions() directly
from the probe routine after checking to see if the pdata structure
exists and has partition information.  If it doesn't, then call
of_mtd_parse_partitions() to get values for parts and nr_parts.  Doing
it that way completely eliminates the uncertainty over pdata pointer
ownership, and eliminates having an anonymous pointer buried in the
driver.

Cheers,
g.

^ permalink raw reply

* Re: [PATCH v2 2/6] eSPI: add eSPI controller support
From: Grant Likely @ 2010-08-10  6:44 UTC (permalink / raw)
  To: Mingkai Hu; +Cc: linuxppc-dev, kumar.gala, spi-devel-general
In-Reply-To: <1280735524-17547-3-git-send-email-Mingkai.hu@freescale.com>

On Mon, Aug 2, 2010 at 1:52 AM, Mingkai Hu <Mingkai.hu@freescale.com> wrote=
:
> Add eSPI controller support based on the library code spi_fsl_lib.c.
>
> The eSPI controller is newer controller 85xx/Pxxx devices supported.
> There're some differences comparing to the SPI controller:
>
> 1. Has different register map and different bit definition
> =A0 So leave the code operated the register to the driver code, not
> =A0 the common code.
>
> 2. Support 4 dedicated chip selects
> =A0 The software can't controll the chip selects directly, The SPCOM[CS]
> =A0 field is used to select which chip selects is used, and the
> =A0 SPCOM[TRANLEN] field is set to tell the controller how long the CS
> =A0 signal need to be asserted. So the driver doesn't need the chipselect
> =A0 related function when transfering data, just set corresponding regist=
er
> =A0 fields to controll the chipseclect.
>
> 3. Different Transmit/Receive FIFO access register behavior
> =A0 For SPI controller, the Tx/Rx FIFO access register can hold only
> =A0 one character regardless of the character length, but for eSPI
> =A0 controller, the register can hold 4 or 2 characters according to
> =A0 the character lengths. Access the Tx/Rx FIFO access register of the
> =A0 eSPI controller will shift out/in 4/2 characters one time. For SPI
> =A0 subsystem, the command and data are put into different transfers, so
> =A0 we need to combine all the transfers to one transfer in order to pass
> =A0 the transfer to eSPI controller.
>
> Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>

I've not dug deep into this patch, but it seems pretty good.  I did
notice one thing below...
[...]

> diff --git a/drivers/spi/spi_fsl_lib.h b/drivers/spi/spi_fsl_lib.h
> index 774e1c8..0772c98 100644
> --- a/drivers/spi/spi_fsl_lib.h
> +++ b/drivers/spi/spi_fsl_lib.h
> @@ -22,10 +22,12 @@
> =A0struct mpc8xxx_spi {
> =A0 =A0 =A0 =A0struct device *dev;
> =A0 =A0 =A0 =A0struct fsl_spi_reg __iomem *base;
> + =A0 =A0 =A0 struct fsl_espi_reg __iomem *espi_base;

There's got to be a cleaner way to do this.  Rather than putting both
pointers into mpc8xxx_spi, I suggest each driver use its own
fsl_spi_priv and fsl_espi_priv wrapper structures that contain the
controller specific properties.

cheers,
g.

^ permalink raw reply

* Re: [PATCH v2 1/6] spi/mpc8xxx: refactor the common code for SPI/eSPI controller
From: Grant Likely @ 2010-08-10  6:40 UTC (permalink / raw)
  To: Mingkai Hu; +Cc: linuxppc-dev, kumar.gala, spi-devel-general
In-Reply-To: <1280735524-17547-2-git-send-email-Mingkai.hu@freescale.com>

On Mon, Aug 2, 2010 at 1:51 AM, Mingkai Hu <Mingkai.hu@freescale.com> wrote=
:
> Refactor the common code in file spi_mpc8xxx.c to spi_fsl_lib.c
> used by SPI/eSPI controller driver as a library, move the SPI
> controller driver to a new file spi_fsl_spi.c, and leave the
> QE/CPM SPI controller code in this file.
>
> Because the register map of the SPI controller and eSPI controller
> is so different, also leave the code operated the register to the
> driver code, not the common code.
>
> Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
> ---
> v2:
> =A0- Rename spi_mpc8xxx.c to spi_fsl_lib.c, also the config name
> =A0- Rename fsl_spi.c to spi_fsl_spi.c, also the config name
> =A0- Move register map definiton from spi_fsl_lib.c to spi_fsl_spi.c
> =A0- Break some funcions line in the arguments instead of the declaration
> =A0- Init bits_per_word to 0 to eliminate the else clause
> =A0- Add brace for the else clause to match if clause
> =A0- Drop the last entry's comma in the match table
> =A0- move module_init() immediately after the init fsl_spi_init() functio=
n
>
> =A0drivers/spi/Kconfig =A0 =A0 =A0 | =A0 20 +-
> =A0drivers/spi/Makefile =A0 =A0 =A0| =A0 =A03 +-
> =A0drivers/spi/spi_fsl_lib.c | =A0237 ++++++++
> =A0drivers/spi/spi_fsl_lib.h | =A0119 ++++
> =A0drivers/spi/spi_fsl_spi.c | 1173 +++++++++++++++++++++++++++++++++++++
> =A0drivers/spi/spi_mpc8xxx.c | 1421 -------------------------------------=
--------

This patch seems pretty good now.  However, the rename from
spi_mpc8xxx.c to spi_fsl_lib.c should be done in a separate patch.  It
is too difficult to track what has changed, when the file gets moved
at the same time.  Move the file in one patch verbatim (with the
required Makefile change), and then move out the fsl_spi specific bits
in a second patch.

g.

^ permalink raw reply

* Re: Review Request: New proposal for device tree clock binding.
From: Grant Likely @ 2010-08-10  5:04 UTC (permalink / raw)
  To: Li Yang-R58472; +Cc: devicetree-discuss, linuxppc-dev, Jeremy Kerr
In-Reply-To: <F9BD3E0A8083BE4ABAA94D7EDD7E3F631093B6@zch01exm26.fsl.freescale.net>

On Mon, Aug 9, 2010 at 10:56 PM, Li Yang-R58472 <r58472@freescale.com> wrot=
e:
>>>> I've avoided requiring clock nodes to have a separate sub node for
>>>> each output because it is more verbose and it prevents clock
>>>> providers from having child nodes for other purposes. =A0Are you
>>>> concerned that
>>>
>>> I don't see why there should be child nodes for other purposes under
>>clock node.
>>>
>>>> having the <phandle>+output name pair will be difficult to manage?
>>>
>>> That's part of my concern.
>>
>>I was concerned about this too until I found precedence for doing the
>>exact same thing in the pci binding (and ePAPR). =A0Mixing phandle and a
>>string in this way doesn't bother me anymore.
>
> Where exactly can I get the sample code for handling this binding?

In my test-devicetree branch.  See the file drivers/of/clock.c[1] from
commit [2]:

[1] http://git.secretlab.ca/?p=3Dlinux-2.6.git;a=3Dblob;f=3Ddrivers/of/cloc=
k.c;h=3D26bd70c293d3ec23cbef3f67e0853069b6c24dc0;hb=3Dfadbfb859485148756533=
b28203b7b0188a17250
[2] http://git.secretlab.ca/?p=3Dlinux-2.6.git;a=3Dcommit;h=3Dfadbfb8594851=
48756533b28203b7b0188a17250

g.

^ permalink raw reply

* RE: Review Request: New proposal for device tree clock binding.
From: Li Yang-R58472 @ 2010-08-10  4:56 UTC (permalink / raw)
  To: Grant Likely; +Cc: devicetree-discuss, linuxppc-dev, Jeremy Kerr
In-Reply-To: <AANLkTinQnFBcLiDyQtRRwzdAg_M48BnNsa2OyKWCiSUh@mail.gmail.com>

>>> I've avoided requiring clock nodes to have a separate sub node for
>>> each output because it is more verbose and it prevents clock
>>> providers from having child nodes for other purposes. =A0Are you
>>> concerned that
>>
>> I don't see why there should be child nodes for other purposes under
>clock node.
>>
>>> having the <phandle>+output name pair will be difficult to manage?
>>
>> That's part of my concern.
>
>I was concerned about this too until I found precedence for doing the
>exact same thing in the pci binding (and ePAPR).  Mixing phandle and a
>string in this way doesn't bother me anymore.

Where exactly can I get the sample code for handling this binding?

- Leo

^ permalink raw reply

* Re: Questions about powerpc kernel tree
From: Benjamin Herrenschmidt @ 2010-08-10  4:19 UTC (permalink / raw)
  To: Terren Chow; +Cc: linuxppc-dev
In-Reply-To: <AANLkTinqKv1OXDBtQfg0jOyw=04FQcAvoE8CPmMN7M5_@mail.gmail.com>

On Tue, 2010-08-10 at 10:39 +0800, Terren Chow wrote:
> Hi Ben:
>     I'm not quite clearly know about the powerpc kernel tree histroy.
> why the tags in the powerpc kernel tree are only update to 2.6.26-rc9
> at about 2 years ago?  

Not sure what you're looking at but your shouldn't need to use the
"powerpc kernel tree" whatever tree this is :-) All our work goes into
Linus tree on a regular basis, so unless you are specifically looking at
the powerpc "next" git branch aimed at the next merge window, there
should be nothing of interest to you there.

Ben.

^ permalink raw reply

* Questions about powerpc kernel tree
From: Terren Chow @ 2010-08-10  2:39 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

Hi Ben:
    I'm not quite clearly know about the powerpc kernel tree histroy. why
the tags in the powerpc kernel tree are only update to 2.6.26-rc9 at about 2
years ago?

-- 
Terren Chow
College of informatics, SCAU
Graduate student
Lab of Embedded System and Wireless Sensor Network
MSN: terren.chow@hotmail.com

[-- Attachment #2: Type: text/html, Size: 412 bytes --]

^ permalink raw reply

* Re: mpc870: hctosys.c unable to open rtc device rtc0
From: Shawn Jin @ 2010-08-09 21:05 UTC (permalink / raw)
  To: Scott Wood; +Cc: ppcdev
In-Reply-To: <20100809122933.5a314af3@schlenkerla.am.freescale.net>

>> Reading the fsl i2c bindings in the documentation, I found an example
>> as follows.
>> =A0 27 =A0 =A0 =A0i2c@860 {
>> =A0 28 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compatible =3D "fsl,mpc823-i2c",
>> =A0 29 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "fsl,cpm1=
-i2c";
>> =A0 30 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D <0x860 0x20 0x3c80 0x30>;
>> =A0 31 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0interrupts =3D <16>;
>> =A0 32 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0interrupt-parent =3D <&CPM_PIC>;
>> =A0 33 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0fsl,cpm-command =3D <0x10>;
>> =A0 34 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#address-cells =3D <1>;
>> =A0 35 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0#size-cells =3D <0>;
>> =A0 36
>> =A0 37 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rtc@68 {
>> =A0 38 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0compatible =3D "da=
llas,ds1307";
>> =A0 39 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0reg =3D <0x68>;
>> =A0 40 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0};
>> =A0 41 =A0 =A0 =A0 =A0};
>> =A0 42
>>
>> In the above example the rtc was explicitly declared as a subnode of
>> the i2c node. Is this the way to connect (or bind) a RTC to the I2C
>> driver?
>
> Yes.

Thanks Scott for the confirmation. I added that to my dts file and the
driver did try to probe the device. But accessing the device timed
out. There are some microcode patches related to I2C that I've not
applied. I'll try the patch(es) later. But how can I find out which
patch should be applied to my MPC870?

Thanks,
-Shawn.

^ permalink raw reply

* Re: [PATCH 8/8] v5  Update memory-hotplug documentation
From: Nishanth Aravamudan @ 2010-08-09 20:48 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linuxppc-dev, Greg KH, linux-kernel, Dave Hansen, linux-mm,
	KAMEZAWA Hiroyuki
In-Reply-To: <201008091344.37878.nacc@us.ibm.com>

On Monday, August 09, 2010 01:44:37 pm Nishanth Aravamudan wrote:
> On Monday, August 09, 2010 11:43:46 am Nathan Fontenot wrote:
> > Update the memory hotplug documentation to reflect the new behaviors of
> > memory blocks reflected in sysfs.
> 
> <snip>
> 
> > Index: linux-2.6/Documentation/memory-hotplug.txt
> > ===================================================================
> > --- linux-2.6.orig/Documentation/memory-hotplug.txt	2010-08-09 07:36:48.000000000 -0500
> > +++ linux-2.6/Documentation/memory-hotplug.txt	2010-08-09 07:59:54.000000000 -0500
> 
> <snip>
> 
> > -/sys/devices/system/memory/memoryXXX/phys_index
> > +/sys/devices/system/memory/memoryXXX/start_phys_index
> > +/sys/devices/system/memory/memoryXXX/end_phys_index
> >  /sys/devices/system/memory/memoryXXX/phys_device
> >  /sys/devices/system/memory/memoryXXX/state
> >  /sys/devices/system/memory/memoryXXX/removable
> > 
> > -'phys_index' : read-only and contains section id, same as XXX.
> 
> <snip>
> 
> > +'phys_index'      : read-only and contains section id of the first section
> 
> Shouldn't this be "start_phys_index"?

Ah, actually it's that the Documentation change doesn't seem to agree with
patch 2/8 ? That is, 2/8 leaves phys_index in place, but changes several
variables, while this patch indicates its removal?

Thanks,
Nish

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
Linux Technology Center

^ permalink raw reply

* Re: [PATCH 8/8] v5  Update memory-hotplug documentation
From: Nishanth Aravamudan @ 2010-08-09 20:44 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: linuxppc-dev, Greg KH, linux-kernel, Dave Hansen, linux-mm,
	KAMEZAWA Hiroyuki
In-Reply-To: <4C604C62.7060509@austin.ibm.com>

On Monday, August 09, 2010 11:43:46 am Nathan Fontenot wrote:
> Update the memory hotplug documentation to reflect the new behaviors of
> memory blocks reflected in sysfs.

<snip>

> Index: linux-2.6/Documentation/memory-hotplug.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/memory-hotplug.txt	2010-08-09 07:36:48.000000000 -0500
> +++ linux-2.6/Documentation/memory-hotplug.txt	2010-08-09 07:59:54.000000000 -0500

<snip>

> -/sys/devices/system/memory/memoryXXX/phys_index
> +/sys/devices/system/memory/memoryXXX/start_phys_index
> +/sys/devices/system/memory/memoryXXX/end_phys_index
>  /sys/devices/system/memory/memoryXXX/phys_device
>  /sys/devices/system/memory/memoryXXX/state
>  /sys/devices/system/memory/memoryXXX/removable
> 
> -'phys_index' : read-only and contains section id, same as XXX.

<snip>

> +'phys_index'      : read-only and contains section id of the first section

Shouldn't this be "start_phys_index"?

Thanks,
Nish

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
Linux Technology Center

^ permalink raw reply

* [PATCH 8/8] v5  Update memory-hotplug documentation
From: Nathan Fontenot @ 2010-08-09 18:43 UTC (permalink / raw)
  To: linux-kernel, linux-mm, linuxppc-dev
  Cc: Greg KH, KAMEZAWA Hiroyuki, Dave Hansen
In-Reply-To: <4C60407C.2080608@austin.ibm.com>

Update the memory hotplug documentation to reflect the new behaviors of
memory blocks reflected in sysfs.

Signed-off-by: Nathan Fontent <nfont@austin.ibm.com>

---
 Documentation/memory-hotplug.txt |   46 +++++++++++++++++++++++++--------------
 1 file changed, 30 insertions(+), 16 deletions(-)

Index: linux-2.6/Documentation/memory-hotplug.txt
===================================================================
--- linux-2.6.orig/Documentation/memory-hotplug.txt	2010-08-09 07:36:48.000000000 -0500
+++ linux-2.6/Documentation/memory-hotplug.txt	2010-08-09 07:59:54.000000000 -0500
@@ -126,36 +126,50 @@ config options.
 --------------------------------
 4 sysfs files for memory hotplug
 --------------------------------
-All sections have their device information under /sys/devices/system/memory as
+All sections have their device information in sysfs.  Each section is part of
+a memory block under /sys/devices/system/memory as
 
 /sys/devices/system/memory/memoryXXX
-(XXX is section id.)
+(XXX is the section id.)
 
-Now, XXX is defined as start_address_of_section / section_size.
+Now, XXX is defined as (start_address_of_section / section_size) of the first
+section contained in the memory block.  The files 'phys_index' and
+'end_phys_index' under each directory report the beginning and end section id's
+for the memory block covered by the sysfs directory.  It is expected that all
+memory sections in this range are present and no memory holes exist in the
+range. Currently there is no way to determine if there is a memory hole, but
+the existence of one should not affect the hotplug capabilities of the memory
+block.
 
 For example, assume 1GiB section size. A device for a memory starting at
 0x100000000 is /sys/device/system/memory/memory4
 (0x100000000 / 1Gib = 4)
 This device covers address range [0x100000000 ... 0x140000000)
 
-Under each section, you can see 4 files.
+Under each section, you can see 5 files.
 
-/sys/devices/system/memory/memoryXXX/phys_index
+/sys/devices/system/memory/memoryXXX/start_phys_index
+/sys/devices/system/memory/memoryXXX/end_phys_index
 /sys/devices/system/memory/memoryXXX/phys_device
 /sys/devices/system/memory/memoryXXX/state
 /sys/devices/system/memory/memoryXXX/removable
 
-'phys_index' : read-only and contains section id, same as XXX.
-'state'      : read-write
-               at read:  contains online/offline state of memory.
-               at write: user can specify "online", "offline" command
-'phys_device': read-only: designed to show the name of physical memory device.
-               This is not well implemented now.
-'removable'  : read-only: contains an integer value indicating
-               whether the memory section is removable or not
-               removable.  A value of 1 indicates that the memory
-               section is removable and a value of 0 indicates that
-               it is not removable.
+'phys_index'      : read-only and contains section id of the first section
+		    in the memory block, same as XXX.
+'end_phys_index'  : read-only and contains section id of the last section
+		    in the memory block.
+'state'           : read-write
+                    at read:  contains online/offline state of memory.
+                    at write: user can specify "online", "offline" command
+                    which will be performed on al sections in the block.
+'phys_device'     : read-only: designed to show the name of physical memory
+                    device.  This is not well implemented now.
+'removable'       : read-only: contains an integer value indicating
+                    whether the memory block is removable or not
+                    removable.  A value of 1 indicates that the memory
+                    block is removable and a value of 0 indicates that
+                    it is not removable. A memory block is removable only if
+                    every section in the block is removable.
 
 NOTE:
   These directories/files appear after physical memory hotplug phase.

^ permalink raw reply

* [PATCH 7/8] v5  Define memory_block_size_bytes() for ppc/pseries
From: Nathan Fontenot @ 2010-08-09 18:42 UTC (permalink / raw)
  To: linux-kernel, linux-mm, linuxppc-dev
  Cc: Greg KH, KAMEZAWA Hiroyuki, Dave Hansen
In-Reply-To: <4C60407C.2080608@austin.ibm.com>

Define a version of memory_block_size_bytes() for powerpc/pseries such that
a memory block spans an entire lmb.

Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>

---
 arch/powerpc/platforms/pseries/hotplug-memory.c |   66 +++++++++++++++++++-----
 1 file changed, 53 insertions(+), 13 deletions(-)

Index: linux-2.6/arch/powerpc/platforms/pseries/hotplug-memory.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/pseries/hotplug-memory.c	2010-08-09 07:36:49.000000000 -0500
+++ linux-2.6/arch/powerpc/platforms/pseries/hotplug-memory.c	2010-08-09 07:54:00.000000000 -0500
@@ -17,6 +17,54 @@
 #include <asm/pSeries_reconfig.h>
 #include <asm/sparsemem.h>
 
+static u32 get_memblock_size(void)
+{
+	struct device_node *np;
+	unsigned int memblock_size = 0;
+
+	np = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
+	if (np) {
+		const unsigned long *size;
+
+		size = of_get_property(np, "ibm,lmb-size", NULL);
+		memblock_size = size ? *size : 0;
+
+		of_node_put(np);
+	} else {
+		unsigned int memzero_size = 0;
+		const unsigned int *regs;
+
+		np = of_find_node_by_path("/memory@0");
+		if (np) {
+			regs = of_get_property(np, "reg", NULL);
+			memzero_size = regs ? regs[3] : 0;
+			of_node_put(np);
+		}
+
+		if (memzero_size) {
+			/* We now know the size of memory@0, use this to find
+			 * the first memoryblock and get its size.
+			 */
+			char buf[64];
+
+			sprintf(buf, "/memory@%x", memzero_size);
+			np = of_find_node_by_path(buf);
+			if (np) {
+				regs = of_get_property(np, "reg", NULL);
+				memblock_size = regs ? regs[3] : 0;
+				of_node_put(np);
+			}
+		}
+	}
+
+	return memblock_size;
+}
+
+u32 memory_block_size_bytes(void)
+{
+	return get_memblock_size();
+}
+
 static int pseries_remove_memblock(unsigned long base, unsigned int memblock_size)
 {
 	unsigned long start, start_pfn;
@@ -127,30 +175,22 @@ static int pseries_add_memory(struct dev
 
 static int pseries_drconf_memory(unsigned long *base, unsigned int action)
 {
-	struct device_node *np;
-	const unsigned long *lmb_size;
+	unsigned long memblock_size;
 	int rc;
 
-	np = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
-	if (!np)
+	memblock_size = get_memblock_size();
+	if (!memblock_size)
 		return -EINVAL;
 
-	lmb_size = of_get_property(np, "ibm,lmb-size", NULL);
-	if (!lmb_size) {
-		of_node_put(np);
-		return -EINVAL;
-	}
-
 	if (action == PSERIES_DRCONF_MEM_ADD) {
-		rc = memblock_add(*base, *lmb_size);
+		rc = memblock_add(*base, memblock_size);
 		rc = (rc < 0) ? -EINVAL : 0;
 	} else if (action == PSERIES_DRCONF_MEM_REMOVE) {
-		rc = pseries_remove_memblock(*base, *lmb_size);
+		rc = pseries_remove_memblock(*base, memblock_size);
 	} else {
 		rc = -EINVAL;
 	}
 
-	of_node_put(np);
 	return rc;
 }
 

^ permalink raw reply

* [PATCH 6/8] v5  Update the node sysfs code
From: Nathan Fontenot @ 2010-08-09 18:41 UTC (permalink / raw)
  To: linux-kernel, linux-mm, linuxppc-dev
  Cc: Greg KH, KAMEZAWA Hiroyuki, Dave Hansen
In-Reply-To: <4C60407C.2080608@austin.ibm.com>

Update the node sysfs code to be aware of the new capability for a memory
block to contain multiple memory sections.  This requires an additional
parameter to unregister_mem_sect_under_nodes so that we know which memory
section of the memory block to unregister.

Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>

---
 drivers/base/memory.c |    2 +-
 drivers/base/node.c   |   12 ++++++++----
 include/linux/node.h  |    6 ++++--
 3 files changed, 13 insertions(+), 7 deletions(-)

Index: linux-2.6/drivers/base/node.c
===================================================================
--- linux-2.6.orig/drivers/base/node.c	2010-08-09 07:36:50.000000000 -0500
+++ linux-2.6/drivers/base/node.c	2010-08-09 07:53:30.000000000 -0500
@@ -346,8 +346,10 @@ int register_mem_sect_under_node(struct
 		return -EFAULT;
 	if (!node_online(nid))
 		return 0;
-	sect_start_pfn = section_nr_to_pfn(mem_blk->phys_index);
-	sect_end_pfn = sect_start_pfn + PAGES_PER_SECTION - 1;
+
+	sect_start_pfn = section_nr_to_pfn(mem_blk->start_phys_index);
+	sect_end_pfn = section_nr_to_pfn(mem_blk->end_phys_index);
+	sect_end_pfn += PAGES_PER_SECTION - 1;
 	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
 		int page_nid;
 
@@ -371,7 +373,8 @@ int register_mem_sect_under_node(struct
 }
 
 /* unregister memory section under all nodes that it spans */
-int unregister_mem_sect_under_nodes(struct memory_block *mem_blk)
+int unregister_mem_sect_under_nodes(struct memory_block *mem_blk,
+				    unsigned long phys_index)
 {
 	NODEMASK_ALLOC(nodemask_t, unlinked_nodes, GFP_KERNEL);
 	unsigned long pfn, sect_start_pfn, sect_end_pfn;
@@ -383,7 +386,8 @@ int unregister_mem_sect_under_nodes(stru
 	if (!unlinked_nodes)
 		return -ENOMEM;
 	nodes_clear(*unlinked_nodes);
-	sect_start_pfn = section_nr_to_pfn(mem_blk->phys_index);
+
+	sect_start_pfn = section_nr_to_pfn(phys_index);
 	sect_end_pfn = sect_start_pfn + PAGES_PER_SECTION - 1;
 	for (pfn = sect_start_pfn; pfn <= sect_end_pfn; pfn++) {
 		int nid;
Index: linux-2.6/drivers/base/memory.c
===================================================================
--- linux-2.6.orig/drivers/base/memory.c	2010-08-09 07:50:28.000000000 -0500
+++ linux-2.6/drivers/base/memory.c	2010-08-09 07:53:30.000000000 -0500
@@ -555,9 +555,9 @@ int remove_memory_block(unsigned long no
 
 	mutex_lock(&mem_sysfs_mutex);
 	mem = find_memory_block(section);
+	unregister_mem_sect_under_nodes(mem, __section_nr(section));
 
 	if (atomic_dec_and_test(&mem->section_count)) {
-		unregister_mem_sect_under_nodes(mem);
 		mem_remove_simple_file(mem, phys_index);
 		mem_remove_simple_file(mem, end_phys_index);
 		mem_remove_simple_file(mem, state);
Index: linux-2.6/include/linux/node.h
===================================================================
--- linux-2.6.orig/include/linux/node.h	2010-08-09 07:36:50.000000000 -0500
+++ linux-2.6/include/linux/node.h	2010-08-09 07:53:30.000000000 -0500
@@ -44,7 +44,8 @@ extern int register_cpu_under_node(unsig
 extern int unregister_cpu_under_node(unsigned int cpu, unsigned int nid);
 extern int register_mem_sect_under_node(struct memory_block *mem_blk,
 						int nid);
-extern int unregister_mem_sect_under_nodes(struct memory_block *mem_blk);
+extern int unregister_mem_sect_under_nodes(struct memory_block *mem_blk,
+					   unsigned long phys_index);
 
 #ifdef CONFIG_HUGETLBFS
 extern void register_hugetlbfs_with_node(node_registration_func_t doregister,
@@ -72,7 +73,8 @@ static inline int register_mem_sect_unde
 {
 	return 0;
 }
-static inline int unregister_mem_sect_under_nodes(struct memory_block *mem_blk)
+static inline int unregister_mem_sect_under_nodes(struct memory_block *mem_blk,
+						  unsigned long phys_index)
 {
 	return 0;
 }

^ permalink raw reply

* [PATCH 5/8] v5  Allow memory_block to span multiple memory sections
From: Nathan Fontenot @ 2010-08-09 18:39 UTC (permalink / raw)
  To: linux-kernel, linux-mm, linuxppc-dev
  Cc: Greg KH, KAMEZAWA Hiroyuki, Dave Hansen
In-Reply-To: <4C60407C.2080608@austin.ibm.com>

Update the memory sysfs code that each sysfs memory directory is now
considered a memory block that can contain multiple memory sections per
memory block.  The default size of each memory block is SECTION_SIZE_BITS
to maintain the current behavior of having a single memory section per
memory block (i.e. one sysfs directory per memory section).

For architectures that want to have memory blocks span multiple
memory sections they need only define their own memory_block_size_bytes()
routine.

Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>

---
 drivers/base/memory.c |  148 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 103 insertions(+), 45 deletions(-)

Index: linux-2.6/drivers/base/memory.c
===================================================================
--- linux-2.6.orig/drivers/base/memory.c	2010-08-09 07:50:20.000000000 -0500
+++ linux-2.6/drivers/base/memory.c	2010-08-09 07:50:28.000000000 -0500
@@ -30,6 +30,14 @@
 static DEFINE_MUTEX(mem_sysfs_mutex);
 
 #define MEMORY_CLASS_NAME	"memory"
+#define MIN_MEMORY_BLOCK_SIZE	(1 << SECTION_SIZE_BITS)
+
+static int sections_per_block;
+
+static inline int base_memory_block_id(int section_nr)
+{
+	return (section_nr / sections_per_block) * sections_per_block;
+}
 
 static struct sysdev_class memory_sysdev_class = {
 	.name = MEMORY_CLASS_NAME,
@@ -84,22 +92,21 @@ EXPORT_SYMBOL(unregister_memory_isolate_
  * register_memory - Setup a sysfs device for a memory block
  */
 static
-int register_memory(struct memory_block *memory, struct mem_section *section)
+int register_memory(struct memory_block *memory)
 {
 	int error;
 
 	memory->sysdev.cls = &memory_sysdev_class;
-	memory->sysdev.id = __section_nr(section);
+	memory->sysdev.id = memory->start_phys_index;
 
 	error = sysdev_register(&memory->sysdev);
 	return error;
 }
 
 static void
-unregister_memory(struct memory_block *memory, struct mem_section *section)
+unregister_memory(struct memory_block *memory)
 {
 	BUG_ON(memory->sysdev.cls != &memory_sysdev_class);
-	BUG_ON(memory->sysdev.id != __section_nr(section));
 
 	/* drop the ref. we got in remove_memory_block() */
 	kobject_put(&memory->sysdev.kobj);
@@ -133,13 +140,16 @@ static ssize_t show_mem_end_phys_index(s
 static ssize_t show_mem_removable(struct sys_device *dev,
 			struct sysdev_attribute *attr, char *buf)
 {
-	unsigned long start_pfn;
-	int ret;
+	unsigned long i, pfn;
+	int ret = 1;
 	struct memory_block *mem =
 		container_of(dev, struct memory_block, sysdev);
 
-	start_pfn = section_nr_to_pfn(mem->start_phys_index);
-	ret = is_mem_section_removable(start_pfn, PAGES_PER_SECTION);
+	for (i = mem->start_phys_index; i <= mem->end_phys_index; i++) {
+		pfn = section_nr_to_pfn(i);
+		ret &= is_mem_section_removable(pfn, PAGES_PER_SECTION);
+	}
+
 	return sprintf(buf, "%d\n", ret);
 }
 
@@ -192,17 +202,14 @@ int memory_isolate_notify(unsigned long
  * OK to have direct references to sparsemem variables in here.
  */
 static int
-memory_block_action(struct memory_block *mem, unsigned long action)
+memory_section_action(unsigned long phys_index, unsigned long action)
 {
 	int i;
-	unsigned long psection;
 	unsigned long start_pfn, start_paddr;
 	struct page *first_page;
 	int ret;
-	int old_state = mem->state;
 
-	psection = mem->start_phys_index;
-	first_page = pfn_to_page(psection << PFN_SECTION_SHIFT);
+	first_page = pfn_to_page(phys_index << PFN_SECTION_SHIFT);
 
 	/*
 	 * The probe routines leave the pages reserved, just
@@ -215,8 +222,8 @@ memory_block_action(struct memory_block
 				continue;
 
 			printk(KERN_WARNING "section number %ld page number %d "
-				"not reserved, was it already online? \n",
-				psection, i);
+				"not reserved, was it already online?\n",
+				phys_index, i);
 			return -EBUSY;
 		}
 	}
@@ -227,18 +234,13 @@ memory_block_action(struct memory_block
 			ret = online_pages(start_pfn, PAGES_PER_SECTION);
 			break;
 		case MEM_OFFLINE:
-			mem->state = MEM_GOING_OFFLINE;
 			start_paddr = page_to_pfn(first_page) << PAGE_SHIFT;
 			ret = remove_memory(start_paddr,
 					    PAGES_PER_SECTION << PAGE_SHIFT);
-			if (ret) {
-				mem->state = old_state;
-				break;
-			}
 			break;
 		default:
-			WARN(1, KERN_WARNING "%s(%p, %ld) unknown action: %ld\n",
-					__func__, mem, action, action);
+			WARN(1, KERN_WARNING "%s(%ld, %ld) unknown action: "
+			     "%ld\n", __func__, phys_index, action, action);
 			ret = -EINVAL;
 	}
 
@@ -248,7 +250,7 @@ memory_block_action(struct memory_block
 static int memory_block_change_state(struct memory_block *mem,
 		unsigned long to_state, unsigned long from_state_req)
 {
-	int ret = 0;
+	int i, ret = 0;
 	mutex_lock(&mem->state_mutex);
 
 	if (mem->state != from_state_req) {
@@ -256,8 +258,21 @@ static int memory_block_change_state(str
 		goto out;
 	}
 
-	ret = memory_block_action(mem, to_state);
-	if (!ret)
+	if (to_state == MEM_OFFLINE)
+		mem->state = MEM_GOING_OFFLINE;
+
+	for (i = mem->start_phys_index; i <= mem->end_phys_index; i++) {
+		ret = memory_section_action(i, to_state);
+		if (ret)
+			break;
+	}
+
+	if (ret) {
+		for (i = mem->start_phys_index; i <= mem->end_phys_index; i++)
+			memory_section_action(i, from_state_req);
+
+		mem->state = from_state_req;
+	} else
 		mem->state = to_state;
 
 out:
@@ -270,20 +285,15 @@ store_mem_state(struct sys_device *dev,
 		struct sysdev_attribute *attr, const char *buf, size_t count)
 {
 	struct memory_block *mem;
-	unsigned int phys_section_nr;
 	int ret = -EINVAL;
 
 	mem = container_of(dev, struct memory_block, sysdev);
-	phys_section_nr = mem->start_phys_index;
-
-	if (!present_section_nr(phys_section_nr))
-		goto out;
 
 	if (!strncmp(buf, "online", min((int)count, 6)))
 		ret = memory_block_change_state(mem, MEM_ONLINE, MEM_OFFLINE);
 	else if(!strncmp(buf, "offline", min((int)count, 7)))
 		ret = memory_block_change_state(mem, MEM_OFFLINE, MEM_ONLINE);
-out:
+
 	if (ret)
 		return ret;
 	return count;
@@ -460,12 +470,13 @@ struct memory_block *find_memory_block(s
 	struct sys_device *sysdev;
 	struct memory_block *mem;
 	char name[sizeof(MEMORY_CLASS_NAME) + 9 + 1];
+	int block_id = base_memory_block_id(__section_nr(section));
 
 	/*
 	 * This only works because we know that section == sysdev->id
 	 * slightly redundant with sysdev_register()
 	 */
-	sprintf(&name[0], "%s%d", MEMORY_CLASS_NAME, __section_nr(section));
+	sprintf(&name[0], "%s%d", MEMORY_CLASS_NAME, block_id);
 
 	kobj = kset_find_obj(&memory_sysdev_class.kset, name);
 	if (!kobj)
@@ -477,26 +488,26 @@ struct memory_block *find_memory_block(s
 	return mem;
 }
 
-static int add_memory_block(int nid, struct mem_section *section,
-			unsigned long state, enum mem_add_context context)
+static int init_memory_block(struct memory_block **memory,
+			     struct mem_section *section, unsigned long state)
 {
-	struct memory_block *mem = kzalloc(sizeof(*mem), GFP_KERNEL);
+	struct memory_block *mem;
 	unsigned long start_pfn;
 	int ret = 0;
 
+	mem = kzalloc(sizeof(*mem), GFP_KERNEL);
 	if (!mem)
 		return -ENOMEM;
 
-	mutex_lock(&mem_sysfs_mutex);
-
-	mem->start_phys_index = __section_nr(section);
+	mem->start_phys_index = base_memory_block_id(__section_nr(section));
+	mem->end_phys_index = mem->start_phys_index + sections_per_block - 1;
 	mem->state = state;
 	atomic_inc(&mem->section_count);
 	mutex_init(&mem->state_mutex);
 	start_pfn = section_nr_to_pfn(mem->start_phys_index);
 	mem->phys_device = arch_get_memory_phys_device(start_pfn);
 
-	ret = register_memory(mem, section);
+	ret = register_memory(mem);
 	if (!ret)
 		ret = mem_create_simple_file(mem, phys_index);
 	if (!ret)
@@ -507,8 +518,29 @@ static int add_memory_block(int nid, str
 		ret = mem_create_simple_file(mem, phys_device);
 	if (!ret)
 		ret = mem_create_simple_file(mem, removable);
+
+	*memory = mem;
+	return ret;
+}
+
+static int add_memory_section(int nid, struct mem_section *section,
+			unsigned long state, enum mem_add_context context)
+{
+	struct memory_block *mem;
+	int ret = 0;
+
+	mutex_lock(&mem_sysfs_mutex);
+
+	mem = find_memory_block(section);
+	if (mem) {
+		atomic_inc(&mem->section_count);
+		kobject_put(&mem->sysdev.kobj);
+	} else
+		ret = init_memory_block(&mem, section, state);
+
 	if (!ret) {
-		if (context == HOTPLUG)
+		if (context == HOTPLUG &&
+		    atomic_read(&mem->section_count) == sections_per_block)
 			ret = register_mem_sect_under_node(mem, nid);
 	}
 
@@ -531,8 +563,10 @@ int remove_memory_block(unsigned long no
 		mem_remove_simple_file(mem, state);
 		mem_remove_simple_file(mem, phys_device);
 		mem_remove_simple_file(mem, removable);
-		unregister_memory(mem, section);
-	}
+		unregister_memory(mem);
+		kfree(mem);
+	} else
+		kobject_put(&mem->sysdev.kobj);
 
 	mutex_unlock(&mem_sysfs_mutex);
 	return 0;
@@ -544,7 +578,7 @@ int remove_memory_block(unsigned long no
  */
 int register_new_memory(int nid, struct mem_section *section)
 {
-	return add_memory_block(nid, section, MEM_OFFLINE, HOTPLUG);
+	return add_memory_section(nid, section, MEM_OFFLINE, HOTPLUG);
 }
 
 int unregister_memory_section(struct mem_section *section)
@@ -555,6 +589,26 @@ int unregister_memory_section(struct mem
 	return remove_memory_block(0, section, 0);
 }
 
+u32 __weak memory_block_size_bytes(void)
+{
+	return MIN_MEMORY_BLOCK_SIZE;
+}
+
+static u32 get_memory_block_size(void)
+{
+	u32 block_sz;
+
+	block_sz = memory_block_size_bytes();
+
+	/* Validate blk_sz is a power of 2 and not less than section size */
+	if ((block_sz & (block_sz - 1)) || (block_sz < MIN_MEMORY_BLOCK_SIZE)) {
+		WARN_ON(1);
+		block_sz = MIN_MEMORY_BLOCK_SIZE;
+	}
+
+	return block_sz;
+}
+
 /*
  * Initialize the sysfs support for memory devices...
  */
@@ -563,12 +617,16 @@ int __init memory_dev_init(void)
 	unsigned int i;
 	int ret;
 	int err;
+	int block_sz;
 
 	memory_sysdev_class.kset.uevent_ops = &memory_uevent_ops;
 	ret = sysdev_class_register(&memory_sysdev_class);
 	if (ret)
 		goto out;
 
+	block_sz = get_memory_block_size();
+	sections_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
+
 	/*
 	 * Create entries for memory sections that were found
 	 * during boot and have been initialized
@@ -576,8 +634,8 @@ int __init memory_dev_init(void)
 	for (i = 0; i < NR_MEM_SECTIONS; i++) {
 		if (!present_section_nr(i))
 			continue;
-		err = add_memory_block(0, __nr_to_section(i), MEM_ONLINE,
-				       BOOT);
+		err = add_memory_section(0, __nr_to_section(i), MEM_ONLINE,
+					 BOOT);
 		if (!ret)
 			ret = err;
 	}

^ permalink raw reply

* [PATCH 4/8] v5 Add mutex for add/remove of memory blocks
From: Nathan Fontenot @ 2010-08-09 18:38 UTC (permalink / raw)
  To: linux-kernel, linux-mm, linuxppc-dev
  Cc: Greg KH, KAMEZAWA Hiroyuki, Dave Hansen
In-Reply-To: <4C60407C.2080608@austin.ibm.com>

Add a new mutex for use in adding and removing of memory blocks.  This
is needed to avoid any race conditions in which the same memory block could
be added and removed at the same time.

Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>

---
 drivers/base/memory.c |    7 +++++++
 1 file changed, 7 insertions(+)

Index: linux-2.6/drivers/base/memory.c
===================================================================
--- linux-2.6.orig/drivers/base/memory.c	2010-08-09 07:49:04.000000000 -0500
+++ linux-2.6/drivers/base/memory.c	2010-08-09 07:50:20.000000000 -0500
@@ -27,6 +27,8 @@
 #include <asm/atomic.h>
 #include <asm/uaccess.h>
 
+static DEFINE_MUTEX(mem_sysfs_mutex);
+
 #define MEMORY_CLASS_NAME	"memory"
 
 static struct sysdev_class memory_sysdev_class = {
@@ -485,6 +487,8 @@ static int add_memory_block(int nid, str
 	if (!mem)
 		return -ENOMEM;
 
+	mutex_lock(&mem_sysfs_mutex);
+
 	mem->start_phys_index = __section_nr(section);
 	mem->state = state;
 	atomic_inc(&mem->section_count);
@@ -508,6 +512,7 @@ static int add_memory_block(int nid, str
 			ret = register_mem_sect_under_node(mem, nid);
 	}
 
+	mutex_unlock(&mem_sysfs_mutex);
 	return ret;
 }
 
@@ -516,6 +521,7 @@ int remove_memory_block(unsigned long no
 {
 	struct memory_block *mem;
 
+	mutex_lock(&mem_sysfs_mutex);
 	mem = find_memory_block(section);
 
 	if (atomic_dec_and_test(&mem->section_count)) {
@@ -528,6 +534,7 @@ int remove_memory_block(unsigned long no
 		unregister_memory(mem, section);
 	}
 
+	mutex_unlock(&mem_sysfs_mutex);
 	return 0;
 }
 

^ permalink raw reply

* [PATCH 3/8] v5 Add section count to memory_block
From: Nathan Fontenot @ 2010-08-09 18:37 UTC (permalink / raw)
  To: linux-kernel, linux-mm, linuxppc-dev
  Cc: Greg KH, KAMEZAWA Hiroyuki, Dave Hansen
In-Reply-To: <4C60407C.2080608@austin.ibm.com>

Add a section count property to the memory_block struct to track the number
of memory sections that have been added/removed from a memory block. This
alolws us to know when the lasat memory section of a memory block has been
removed so we can remove the memory block.

Signed-off-by: Nathan Fontenot <nfont@asutin.ibm.com>

---
 drivers/base/memory.c  |   18 +++++++++++-------
 include/linux/memory.h |    2 ++
 2 files changed, 13 insertions(+), 7 deletions(-)

Index: linux-2.6/drivers/base/memory.c
===================================================================
--- linux-2.6.orig/drivers/base/memory.c	2010-08-09 07:44:31.000000000 -0500
+++ linux-2.6/drivers/base/memory.c	2010-08-09 07:49:04.000000000 -0500
@@ -487,6 +487,7 @@ static int add_memory_block(int nid, str
 
 	mem->start_phys_index = __section_nr(section);
 	mem->state = state;
+	atomic_inc(&mem->section_count);
 	mutex_init(&mem->state_mutex);
 	start_pfn = section_nr_to_pfn(mem->start_phys_index);
 	mem->phys_device = arch_get_memory_phys_device(start_pfn);
@@ -516,13 +517,16 @@ int remove_memory_block(unsigned long no
 	struct memory_block *mem;
 
 	mem = find_memory_block(section);
-	unregister_mem_sect_under_nodes(mem);
-	mem_remove_simple_file(mem, phys_index);
-	mem_remove_simple_file(mem, end_phys_index);
-	mem_remove_simple_file(mem, state);
-	mem_remove_simple_file(mem, phys_device);
-	mem_remove_simple_file(mem, removable);
-	unregister_memory(mem, section);
+
+	if (atomic_dec_and_test(&mem->section_count)) {
+		unregister_mem_sect_under_nodes(mem);
+		mem_remove_simple_file(mem, phys_index);
+		mem_remove_simple_file(mem, end_phys_index);
+		mem_remove_simple_file(mem, state);
+		mem_remove_simple_file(mem, phys_device);
+		mem_remove_simple_file(mem, removable);
+		unregister_memory(mem, section);
+	}
 
 	return 0;
 }
Index: linux-2.6/include/linux/memory.h
===================================================================
--- linux-2.6.orig/include/linux/memory.h	2010-08-09 07:44:31.000000000 -0500
+++ linux-2.6/include/linux/memory.h	2010-08-09 07:49:04.000000000 -0500
@@ -19,11 +19,13 @@
 #include <linux/node.h>
 #include <linux/compiler.h>
 #include <linux/mutex.h>
+#include <asm/atomic.h>
 
 struct memory_block {
 	unsigned long start_phys_index;
 	unsigned long end_phys_index;
 	unsigned long state;
+	atomic_t section_count;
 	/*
 	 * This serializes all state change requests.  It isn't
 	 * held during creation because the control files are

^ permalink raw reply

* [PATCH 2/8] v5 Add new phys_index properties
From: Nathan Fontenot @ 2010-08-09 18:36 UTC (permalink / raw)
  To: linux-kernel, linux-mm, linuxppc-dev
  Cc: Greg KH, KAMEZAWA Hiroyuki, Dave Hansen
In-Reply-To: <4C60407C.2080608@austin.ibm.com>

Update the 'phys_index' properties of a memory block to include a
'start_phys_index' which is the same as the current 'phys_index' property.
The property still appears as 'phys_index' in sysfs but the memory_block
struct name is updated to indicate the start and end values.
This also adds an 'end_phys_index' property to indicate the id of the
last section in th memory block.

Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>

---
 drivers/base/memory.c  |   28 ++++++++++++++++++++--------
 include/linux/memory.h |    3 ++-
 2 files changed, 22 insertions(+), 9 deletions(-)

Index: linux-2.6/drivers/base/memory.c
===================================================================
--- linux-2.6.orig/drivers/base/memory.c	2010-08-09 07:44:21.000000000 -0500
+++ linux-2.6/drivers/base/memory.c	2010-08-09 07:44:31.000000000 -0500
@@ -109,12 +109,20 @@ unregister_memory(struct memory_block *m
  * uses.
  */
 
-static ssize_t show_mem_phys_index(struct sys_device *dev,
+static ssize_t show_mem_start_phys_index(struct sys_device *dev,
 			struct sysdev_attribute *attr, char *buf)
 {
 	struct memory_block *mem =
 		container_of(dev, struct memory_block, sysdev);
-	return sprintf(buf, "%08lx\n", mem->phys_index);
+	return sprintf(buf, "%08lx\n", mem->start_phys_index);
+}
+
+static ssize_t show_mem_end_phys_index(struct sys_device *dev,
+			struct sysdev_attribute *attr, char *buf)
+{
+	struct memory_block *mem =
+		container_of(dev, struct memory_block, sysdev);
+	return sprintf(buf, "%08lx\n", mem->end_phys_index);
 }
 
 /*
@@ -128,7 +136,7 @@ static ssize_t show_mem_removable(struct
 	struct memory_block *mem =
 		container_of(dev, struct memory_block, sysdev);
 
-	start_pfn = section_nr_to_pfn(mem->phys_index);
+	start_pfn = section_nr_to_pfn(mem->start_phys_index);
 	ret = is_mem_section_removable(start_pfn, PAGES_PER_SECTION);
 	return sprintf(buf, "%d\n", ret);
 }
@@ -191,7 +199,7 @@ memory_block_action(struct memory_block
 	int ret;
 	int old_state = mem->state;
 
-	psection = mem->phys_index;
+	psection = mem->start_phys_index;
 	first_page = pfn_to_page(psection << PFN_SECTION_SHIFT);
 
 	/*
@@ -264,7 +272,7 @@ store_mem_state(struct sys_device *dev,
 	int ret = -EINVAL;
 
 	mem = container_of(dev, struct memory_block, sysdev);
-	phys_section_nr = mem->phys_index;
+	phys_section_nr = mem->start_phys_index;
 
 	if (!present_section_nr(phys_section_nr))
 		goto out;
@@ -296,7 +304,8 @@ static ssize_t show_phys_device(struct s
 	return sprintf(buf, "%d\n", mem->phys_device);
 }
 
-static SYSDEV_ATTR(phys_index, 0444, show_mem_phys_index, NULL);
+static SYSDEV_ATTR(phys_index, 0444, show_mem_start_phys_index, NULL);
+static SYSDEV_ATTR(end_phys_index, 0444, show_mem_end_phys_index, NULL);
 static SYSDEV_ATTR(state, 0644, show_mem_state, store_mem_state);
 static SYSDEV_ATTR(phys_device, 0444, show_phys_device, NULL);
 static SYSDEV_ATTR(removable, 0444, show_mem_removable, NULL);
@@ -476,16 +485,18 @@ static int add_memory_block(int nid, str
 	if (!mem)
 		return -ENOMEM;
 
-	mem->phys_index = __section_nr(section);
+	mem->start_phys_index = __section_nr(section);
 	mem->state = state;
 	mutex_init(&mem->state_mutex);
-	start_pfn = section_nr_to_pfn(mem->phys_index);
+	start_pfn = section_nr_to_pfn(mem->start_phys_index);
 	mem->phys_device = arch_get_memory_phys_device(start_pfn);
 
 	ret = register_memory(mem, section);
 	if (!ret)
 		ret = mem_create_simple_file(mem, phys_index);
 	if (!ret)
+		ret = mem_create_simple_file(mem, end_phys_index);
+	if (!ret)
 		ret = mem_create_simple_file(mem, state);
 	if (!ret)
 		ret = mem_create_simple_file(mem, phys_device);
@@ -507,6 +518,7 @@ int remove_memory_block(unsigned long no
 	mem = find_memory_block(section);
 	unregister_mem_sect_under_nodes(mem);
 	mem_remove_simple_file(mem, phys_index);
+	mem_remove_simple_file(mem, end_phys_index);
 	mem_remove_simple_file(mem, state);
 	mem_remove_simple_file(mem, phys_device);
 	mem_remove_simple_file(mem, removable);
Index: linux-2.6/include/linux/memory.h
===================================================================
--- linux-2.6.orig/include/linux/memory.h	2010-08-09 07:36:55.000000000 -0500
+++ linux-2.6/include/linux/memory.h	2010-08-09 07:44:31.000000000 -0500
@@ -21,7 +21,8 @@
 #include <linux/mutex.h>
 
 struct memory_block {
-	unsigned long phys_index;
+	unsigned long start_phys_index;
+	unsigned long end_phys_index;
 	unsigned long state;
 	/*
 	 * This serializes all state change requests.  It isn't

^ permalink raw reply

* [PATCH 1/8] v5 Move the find_memory_block() routine up
From: Nathan Fontenot @ 2010-08-09 18:35 UTC (permalink / raw)
  To: linux-kernel, linux-mm, linuxppc-dev
  Cc: Greg KH, KAMEZAWA Hiroyuki, Dave Hansen
In-Reply-To: <4C60407C.2080608@austin.ibm.com>

Move the find_me mory_block() routine up to avoid needing a forward
declaration in subsequent patches.

Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>

---
 drivers/base/memory.c |   62 +++++++++++++++++++++++++-------------------------
 1 file changed, 31 insertions(+), 31 deletions(-)

Index: linux-2.6/drivers/base/memory.c
===================================================================
--- linux-2.6.orig/drivers/base/memory.c	2010-08-09 07:36:55.000000000 -0500
+++ linux-2.6/drivers/base/memory.c	2010-08-09 07:44:21.000000000 -0500
@@ -435,6 +435,37 @@ int __weak arch_get_memory_phys_device(u
 	return 0;
 }
 
+/*
+ * For now, we have a linear search to go find the appropriate
+ * memory_block corresponding to a particular phys_index. If
+ * this gets to be a real problem, we can always use a radix
+ * tree or something here.
+ *
+ * This could be made generic for all sysdev classes.
+ */
+struct memory_block *find_memory_block(struct mem_section *section)
+{
+	struct kobject *kobj;
+	struct sys_device *sysdev;
+	struct memory_block *mem;
+	char name[sizeof(MEMORY_CLASS_NAME) + 9 + 1];
+
+	/*
+	 * This only works because we know that section == sysdev->id
+	 * slightly redundant with sysdev_register()
+	 */
+	sprintf(&name[0], "%s%d", MEMORY_CLASS_NAME, __section_nr(section));
+
+	kobj = kset_find_obj(&memory_sysdev_class.kset, name);
+	if (!kobj)
+		return NULL;
+
+	sysdev = container_of(kobj, struct sys_device, kobj);
+	mem = container_of(sysdev, struct memory_block, sysdev);
+
+	return mem;
+}
+
 static int add_memory_block(int nid, struct mem_section *section,
 			unsigned long state, enum mem_add_context context)
 {
@@ -468,37 +499,6 @@ static int add_memory_block(int nid, str
 	return ret;
 }
 
-/*
- * For now, we have a linear search to go find the appropriate
- * memory_block corresponding to a particular phys_index. If
- * this gets to be a real problem, we can always use a radix
- * tree or something here.
- *
- * This could be made generic for all sysdev classes.
- */
-struct memory_block *find_memory_block(struct mem_section *section)
-{
-	struct kobject *kobj;
-	struct sys_device *sysdev;
-	struct memory_block *mem;
-	char name[sizeof(MEMORY_CLASS_NAME) + 9 + 1];
-
-	/*
-	 * This only works because we know that section == sysdev->id
-	 * slightly redundant with sysdev_register()
-	 */
-	sprintf(&name[0], "%s%d", MEMORY_CLASS_NAME, __section_nr(section));
-
-	kobj = kset_find_obj(&memory_sysdev_class.kset, name);
-	if (!kobj)
-		return NULL;
-
-	sysdev = container_of(kobj, struct sys_device, kobj);
-	mem = container_of(sysdev, struct memory_block, sysdev);
-
-	return mem;
-}
-
 int remove_memory_block(unsigned long node_id, struct mem_section *section,
 		int phys_device)
 {

^ permalink raw reply

* Re: [PATCH 1/3 v2] sdhci: Add auto CMD12 support for eSDHC driver
From: Michał Mirosław @ 2010-08-09 18:24 UTC (permalink / raw)
  To: Roy Zang; +Cc: linuxppc-dev, akpm, linux-mmc
In-Reply-To: <1280805072-26112-1-git-send-email-tie-fei.zang@freescale.com>

2010/8/3 Roy Zang <tie-fei.zang@freescale.com>:
[...]
> @@ -240,6 +240,8 @@ struct sdhci_host {
> =A0#define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN =A0 =A0 =A0 =A0 =A0 =A0 =A0(=
1<<25)
> =A0/* Controller cannot support End Attribute in NOP ADMA descriptor */
> =A0#define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC =A0 =A0 =A0 =A0 =A0 =A0 =A0(=
1<<26)
> +/* Controller uses Auto CMD12 command to stop the transfer */
> +#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 =A0 =A0 =A0 =A0 =A0 =A0 (1<<2=
7)
>
> =A0 =A0 =A0 =A0int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 irq; =A0 =A0 =
=A0 =A0 =A0 =A0/* Device IRQ */
> =A0 =A0 =A0 =A0void __iomem * =A0 =A0 =A0 =A0 =A0ioaddr; =A0 =A0 =A0 =A0 =
/* Mapped address */

Just a cosmetic hint: I suggest SDHCI_QUIRK_MULTIBLOCK_READ_AUTO_CMD12
or something for the quirk name, because ACMD12 part might be confused
with MMC/SD App CMD 12 (CMD55+CMD12 combo) if/whenever that gets used.

Best Regards,
Micha=B3 Miros=B3aw

^ permalink raw reply

* [PATCH 0/8] v5 De-couple sysfs memory directories from memory sections
From: Nathan Fontenot @ 2010-08-09 17:53 UTC (permalink / raw)
  To: linux-kernel, linux-mm, linuxppc-dev
  Cc: Greg KH, KAMEZAWA Hiroyuki, Dave Hansen

This set of patches de-couples the idea that there is a single
directory in sysfs for each memory section.  The intent of the
patches is to reduce the number of sysfs directories created to
resolve a boot-time performance issue.  On very large systems
boot time are getting very long (as seen on powerpc hardware)
due to the enormous number of sysfs directories being created.
On a system with 1 TB of memory we create ~63,000 directories.
For even larger systems boot times are being measured in hours.

This set of patches allows for each directory created in sysfs
to cover more than one memory section.  The default behavior for
sysfs directory creation is the same, in that each directory
represents a single memory section.  A new file 'end_phys_index'
in each directory contains the physical_id of the last memory
section covered by the directory so that users can easily
determine the memory section range of a directory.

Updates for version 5 of the patchset include the following:

Patch 4/8 Add mutex for add/remove of memory blocks
- Define the mutex using DEFINE_MUTEX macro.

Patch 8/8 Update memory-hotplug documentation
- Add information concerning memory holes in phys_index..end_phys_index.
 
Thanks,

Nathan Fontenot

^ permalink raw reply

* Re: [PATCH 1/3 v2] sdhci: Add auto CMD12 support for eSDHC driver
From: Anton Vorontsov @ 2010-08-09 17:43 UTC (permalink / raw)
  To: Grant Likely; +Cc: linux-mmc, linuxppc-dev, akpm
In-Reply-To: <AANLkTinJKO3-iXOXuL7CMWCj5qMMHyMrtnadwPpqTeBu@mail.gmail.com>

On Wed, Aug 04, 2010 at 07:02:56PM -0600, Grant Likely wrote:
> On Mon, Aug 2, 2010 at 9:11 PM, Roy Zang <tie-fei.zang@freescale.com> wrote:
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > Add auto CMD12 command support for eSDHC driver.
> > This is needed by P4080 and P1022 for block read/write.
> > Manual asynchronous CMD12 abort operation causes protocol violations on
> > these silicons.
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > ---
> >  drivers/mmc/host/sdhci-of-core.c |    4 ++++
> >  drivers/mmc/host/sdhci.c         |   14 ++++++++++++--
> >  drivers/mmc/host/sdhci.h         |    2 ++
> >  3 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index c6d1bd8..a92566e 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -817,8 +817,12 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
> >        WARN_ON(!host->data);
> >
> >        mode = SDHCI_TRNS_BLK_CNT_EN;
> > -       if (data->blocks > 1)
> > -               mode |= SDHCI_TRNS_MULTI;
> > +       if (data->blocks > 1) {
> > +               if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> > +                       mode |= SDHCI_TRNS_MULTI | SDHCI_TRNS_ACMD12;
> > +               else
> > +                       mode |= SDHCI_TRNS_MULTI;
> 
> nit:
> +               mode |= SDHCI_TRNS_MULTI;
> +               if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> +                       mode |= SDHCI_TRNS_ACMD12;
> 
> Clearer, no?

Much clearer. I would prefer the nit incorporated.

Another nit:

> @@ -154,6 +154,10 @@ static int __devinit sdhci_of_probe(struct of_device *ofdev,
>                 host->ops = &sdhci_of_data->ops;
>         }
> 
> +       if (of_get_property(np, "sdhci,auto-cmd12", NULL))
> +               host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
> +
> +      

^^ No need for the two empty lines.

>        if (of_get_property(np, "sdhci,1-bit-only", NULL))

Though, technically the patch looks OK, feel free to add my

  Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

on the next resend (if any).

Thanks Roy!

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: [PATCH 2/3 v2] dts: Add sdhci,auto-cmd12 field for p4080 device tree
From: Anton Vorontsov @ 2010-08-09 17:36 UTC (permalink / raw)
  To: Roy Zang; +Cc: linuxppc-dev, akpm, linux-mmc
In-Reply-To: <1280805072-26112-2-git-send-email-tie-fei.zang@freescale.com>

On Tue, Aug 03, 2010 at 11:11:11AM +0800, Roy Zang wrote:
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
>  Documentation/powerpc/dts-bindings/fsl/esdhc.txt |    2 ++
>  arch/powerpc/boot/dts/p4080ds.dts                |    1 +
>  2 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
> index 8a00407..64bcb8b 100644
> --- a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
> +++ b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
> @@ -14,6 +14,8 @@ Required properties:
>      reports inverted write-protect state;
>    - sdhci,1-bit-only : (optional) specifies that a controller can
>      only handle 1-bit data transfers.
> +  - sdhci,auto-cmd12: (optional) specifies that a controller can
> +    only handle auto CMD12.

Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

>  Example:
>  
> diff --git a/arch/powerpc/boot/dts/p4080ds.dts b/arch/powerpc/boot/dts/p4080ds.dts
> index 6b29eab..efa0091 100644
> --- a/arch/powerpc/boot/dts/p4080ds.dts
> +++ b/arch/powerpc/boot/dts/p4080ds.dts
> @@ -280,6 +280,7 @@
>  			reg = <0x114000 0x1000>;
>  			interrupts = <48 2>;
>  			interrupt-parent = <&mpic>;
> +			sdhci,auto-cmd12;
>  		};
>  
>  		i2c@118000 {

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply

* Re: [PATCH 3/3 v2] dts: Add ESDHC weird voltage bits workaround
From: Anton Vorontsov @ 2010-08-09 17:33 UTC (permalink / raw)
  To: Roy Zang; +Cc: linuxppc-dev, akpm, linux-mmc
In-Reply-To: <1280805072-26112-3-git-send-email-tie-fei.zang@freescale.com>

On Tue, Aug 03, 2010 at 11:11:12AM +0800, Roy Zang wrote:
> P4080 ESDHC controller does not support 1.8V and 3.0V voltage. but the
> host controller capabilities register wrongly set the bits.
> This patch adds the workaround to correct the weird voltage setting bits.
> Only 3.3V voltage is supported for P4080 ESDHC controller.
> 
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>

Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

Btw, where is implementation for the voltage-ranges handling?

> ---
>  arch/powerpc/boot/dts/p4080ds.dts |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/p4080ds.dts b/arch/powerpc/boot/dts/p4080ds.dts
> index efa0091..2f0de24 100644
> --- a/arch/powerpc/boot/dts/p4080ds.dts
> +++ b/arch/powerpc/boot/dts/p4080ds.dts
> @@ -280,6 +280,7 @@
>  			reg = <0x114000 0x1000>;
>  			interrupts = <48 2>;
>  			interrupt-parent = <&mpic>;
> +			voltage-ranges = <3300 3300>;
>  			sdhci,auto-cmd12;
>  		};
>  
> -- 
> 1.5.6.5

^ permalink raw reply

* Re: mpc870: hctosys.c unable to open rtc device rtc0
From: Scott Wood @ 2010-08-09 17:29 UTC (permalink / raw)
  To: Shawn Jin; +Cc: ppcdev
In-Reply-To: <AANLkTinjLBPG9YfpoDnSoKeGvUO2OEjdTXrYLTZ9c8LB@mail.gmail.com>

On Sun, 8 Aug 2010 23:37:00 -0700
Shawn Jin <shawnxjin@gmail.com> wrote:

> Reading the fsl i2c bindings in the documentation, I found an example
> as follows.
>   27      i2c@860 {
>   28                compatible = "fsl,mpc823-i2c",
>   29                             "fsl,cpm1-i2c";
>   30                reg = <0x860 0x20 0x3c80 0x30>;
>   31                interrupts = <16>;
>   32                interrupt-parent = <&CPM_PIC>;
>   33                fsl,cpm-command = <0x10>;
>   34                #address-cells = <1>;
>   35                #size-cells = <0>;
>   36
>   37                rtc@68 {
>   38                        compatible = "dallas,ds1307";
>   39                        reg = <0x68>;
>   40                };
>   41        };
>   42
> 
> In the above example the rtc was explicitly declared as a subnode of
> the i2c node. Is this the way to connect (or bind) a RTC to the I2C
> driver?

Yes.

> What is the reg (0x68) under rtc node?

It's the 7-bit I2C address (without the low-order direction bit).

> I set breakpoint at ds1037_probe() and was hoping that it might be hit
> during the driver registration. But it didn't. Would the
> ds1037_probe() be called during when the ds1037_driver was registered
> as an I2C driver?

The probe function is called only if the device is declared.  There is
no autodetection.

-Scott

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox