* Re: [PATCH] powerpc/cell: strncpy does not null terminate string
From: Arnd Bergmann @ 2009-07-17 15:56 UTC (permalink / raw)
To: Roel Kluin; +Cc: linuxppc-dev, Andrew Morton, cbe-oss-dev
In-Reply-To: <4A608A69.4060204@gmail.com>
On Friday 17 July 2009, Roel Kluin wrote:
>
> static int __init celleb_machine_type_hack(char *ptr)
> {
> - strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
> + strlcpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
> celleb_machine_type[sizeof(celleb_machine_type)-1] = 0;
> return 0;
This still is pointless as long as you keep the explicit null-termination
in the next line, the patch still doesn't change anything significant.
The file is maintained by Ishizaki Kou, if he would prefer to take a
patch replacing the two lines with one, that's fine with me, otherwise
I just wouldn't bother. You still only gain a few bytes of inittext, but
that is discarded at boot time.
Arnd <><
^ permalink raw reply
* Re: [PATCH] powerpc/cell: strncpy does not null terminate string
From: roel kluin @ 2009-07-17 15:19 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: linuxppc-dev, Andrew Morton, linuxppc-dev, cbe-oss-dev
In-Reply-To: <200907171705.18578.arnd@arndb.de>
>> With `sizeof(string) - 1` strncpy() will null terminate the string.
>
> No, it won't.
> See the line after the strncpy. This is still required for proper zero-termination.
You're right, sorry for the noise.
Roel
^ permalink raw reply
* Re: [PATCH] powerpc/cell: strncpy does not null terminate string
From: Arnd Bergmann @ 2009-07-17 15:05 UTC (permalink / raw)
To: linuxppc-dev; +Cc: linuxppc-dev, Andrew Morton, Roel Kluin, cbe-oss-dev
In-Reply-To: <4A607185.6020302@gmail.com>
On Friday 17 July 2009, Roel Kluin wrote:
> With `sizeof(string) - 1` strncpy() will null terminate the string.
No, it won't. See the 'Warning' part of the strncpy man page.
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> To test this:
>
> #include <stdio.h>
> #include <string.h>
>
> char a[10];
> char b[10];
>
> int main()
> {
> const char* str = "0123456789012";
> strncpy(a, str, sizeof(a));
> strncpy(b, str, sizeof(b) - 1);
> printf("String a was %s, b was %s\n", a, b);
>
> return 0;
> }
>
> Output:
> String a was 0123456789012345678, b was 012345678
This is an invalid test case, it relies on b being zero-filled by the
compiler, which is not true for programs in general.
> diff --git a/arch/powerpc/platforms/cell/celleb_setup.c b/arch/powerpc/platforms/cell/celleb_setup.c
> index 07c234f..cfdbadb 100644
> --- a/arch/powerpc/platforms/cell/celleb_setup.c
> +++ b/arch/powerpc/platforms/cell/celleb_setup.c
> @@ -80,7 +80,7 @@ static void celleb_show_cpuinfo(struct seq_file *m)
>
> static int __init celleb_machine_type_hack(char *ptr)
> {
> - strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
> + strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type) - 1);
> celleb_machine_type[sizeof(celleb_machine_type)-1] = 0;
> return 0;
> }
See the line after the strncpy. This is still required for proper zero-termination.
Your patch tries to address a problem that doesn't exist, and does not have any
effect at all after celleb_machine_type_hack has completed.
Arnd <><
^ permalink raw reply
* Re: [PATCH 2/2 v4] MPC52xx FEC: be more conservative when setting MII_SPEED register
From: Grant Likely @ 2009-07-17 14:45 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: linuxppc-dev, netdev
In-Reply-To: <1247835545-18650-1-git-send-email-wd@denx.de>
On Fri, Jul 17, 2009 at 6:59 AM, Wolfgang Denk<wd@denx.de> wrote:
> This patch adds error checking and prevents clobbering unrelated bits
> (reserved bits or the DIS_PREAMBLE bit) when writing the MII_SPEED
> register on MPC52xx systems.
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: <netdev@vger.kernel.org>
> ---
> v3: - use maximum divider in case MPC512x IPS clock is unknown
> v4: - use the same code in the probe-function, too
>
> =A0drivers/net/fec_mpc52xx.c =A0 =A0 | =A0 25 ++++++++++++++++++++++---
> =A0drivers/net/fec_mpc52xx_phy.c | =A0 23 ++++++++++++++++++++---
Blech. now this block of duplicated code I don't like. This is all
one device, so surely the mdio speed can be calculated once and used
for both drivers.... let me think about this for a bit. 'course this
problem is all rolled up in the nastiness of having two drivers
working on the same device. I suspect is was a mistake to split up
all the powerpc ethernet drivers into separate of_platform drivers.
g.
> =A02 files changed, 42 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
> index cc78633..eed8d2b 100644
> --- a/drivers/net/fec_mpc52xx.c
> +++ b/drivers/net/fec_mpc52xx.c
> @@ -639,7 +639,7 @@ static void mpc52xx_fec_hw_init(struct net_device *de=
v)
> =A0 =A0 =A0 =A0/* set phy speed.
> =A0 =A0 =A0 =A0 * this can't be done in phy driver, since it needs to be =
called
> =A0 =A0 =A0 =A0 * before fec stuff (even on resume) */
> - =A0 =A0 =A0 out_be32(&fec->mii_speed, priv->mdio_speed);
> + =A0 =A0 =A0 clrsetbits_be32(&fec->mii_speed, 0x7E, priv->mdio_speed);
> =A0}
>
> =A0/**
> @@ -863,7 +863,7 @@ mpc52xx_fec_probe(struct of_device *op, const struct =
of_device_id *match)
> =A0 =A0 =A0 =A0struct mpc52xx_fec_priv *priv =3D NULL;
> =A0 =A0 =A0 =A0struct resource mem;
> =A0 =A0 =A0 =A0const u32 *prop;
> - =A0 =A0 =A0 int prop_size;
> + =A0 =A0 =A0 int prop_size, clock, speed;
>
> =A0 =A0 =A0 =A0phys_addr_t rx_fifo;
> =A0 =A0 =A0 =A0phys_addr_t tx_fifo;
> @@ -948,7 +948,26 @@ mpc52xx_fec_probe(struct of_device *op, const struct=
of_device_id *match)
> =A0 =A0 =A0 =A0/* Start with safe defaults for link connection */
> =A0 =A0 =A0 =A0priv->speed =3D 100;
> =A0 =A0 =A0 =A0priv->duplex =3D DUPLEX_HALF;
> - =A0 =A0 =A0 priv->mdio_speed =3D ((mpc5xxx_get_bus_frequency(op->node) =
>> 20) / 5) << 1;
> +
> + =A0 =A0 =A0 /* MII speed */
> + =A0 =A0 =A0 clock =3D mpc5xxx_get_bus_frequency(op->node);
> + =A0 =A0 =A0 if (!clock) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Use maximum divider if clock is unknown =
*/
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&op->dev, "could not determine IPB =
clock\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock =3D 0x3F * 5000000;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0* Scale for a MII clock <=3D 2.5 MHz
> + =A0 =A0 =A0 =A0* Note that only 6 bits (25:30) are available for MII sp=
eed.
> + =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 speed =3D (clock + 4999999) / 5000000;
> + =A0 =A0 =A0 if (speed > 0x3F) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 speed =3D 0x3F;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&op->dev, "MII clock (%d Hz) exceed=
s max (2.5 MHz)\n",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock / speed);
> + =A0 =A0 =A0 }
> + =A0 =A0 =A0 priv->mdio_speed =3D speed << 1;
>
> =A0 =A0 =A0 =A0/* The current speed preconfigures the speed of the MII li=
nk */
> =A0 =A0 =A0 =A0prop =3D of_get_property(op->node, "current-speed", &prop_=
size);
> diff --git a/drivers/net/fec_mpc52xx_phy.c b/drivers/net/fec_mpc52xx_phy.=
c
> index 31e6d62..d3537e1 100644
> --- a/drivers/net/fec_mpc52xx_phy.c
> +++ b/drivers/net/fec_mpc52xx_phy.c
> @@ -70,7 +70,7 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
> =A0 =A0 =A0 =A0struct mpc52xx_fec_mdio_priv *priv;
> =A0 =A0 =A0 =A0struct resource res =3D {};
> =A0 =A0 =A0 =A0int err;
> - =A0 =A0 =A0 int i;
> + =A0 =A0 =A0 int i, clock, speed;
>
> =A0 =A0 =A0 =A0bus =3D mdiobus_alloc();
> =A0 =A0 =A0 =A0if (bus =3D=3D NULL)
> @@ -105,8 +105,25 @@ static int mpc52xx_fec_mdio_probe(struct of_device *=
of,
> =A0 =A0 =A0 =A0dev_set_drvdata(dev, bus);
>
> =A0 =A0 =A0 =A0/* set MII speed */
> - =A0 =A0 =A0 out_be32(&priv->regs->mii_speed,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 ((mpc5xxx_get_bus_frequency(of->node) >> 20=
) / 5) << 1);
> + =A0 =A0 =A0 clock =3D mpc5xxx_get_bus_frequency(of->node);
> + =A0 =A0 =A0 if (!clock) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Use maximum divider if clock is unknown =
*/
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&of->dev, "could not determine IPB =
clock\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock =3D 0x3F * 5000000;
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0* Scale for a MII clock <=3D 2.5 MHz
> + =A0 =A0 =A0 =A0* Note that only 6 bits (25:30) are available for MII sp=
eed.
> + =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 speed =3D (clock + 4999999) / 5000000;
> + =A0 =A0 =A0 if (speed > 0x3F) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 speed =3D 0x3F;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&of->dev, "MII clock (%d Hz) exceed=
s max (2.5 MHz)\n",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock / speed);
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 clrsetbits_be32(&priv->regs->mii_speed, 0x7E, speed << 1);
>
> =A0 =A0 =A0 =A0err =3D of_mdiobus_register(bus, np);
> =A0 =A0 =A0 =A0if (err)
> --
> 1.6.0.6
>
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH 1/2 v5] fs_enet/mii-fec.c: fix MII speed calculation
From: Grant Likely @ 2009-07-17 14:41 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: linuxppc-dev, David Miller, netdev
In-Reply-To: <1247833628-15952-1-git-send-email-wd@denx.de>
On Fri, Jul 17, 2009 at 6:27 AM, Wolfgang Denk<wd@denx.de> wrote:
> The MII speed calculation was based on the CPU clock (ppc_proc_freq),
> but for MPC512x we must use the bus clock instead.
>
> This patch makes it use the correct clock and makes sure we don't
> clobber reserved bits in the MII_SPEED register.
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: <netdev@vger.kernel.org>
Acked-by: Grant Likely <grant.likely@secretlab.ca>
David, this isn't a critical bug fix or a regression, so I think it
should be merged for -next.
g.
> ---
> v5: - fix divider so we really use 2.5 MHz (instead of 1.25)
> =A0 =A0- use maximum divider in case MPC512x IPS clock is unknown
>
> =A0drivers/net/fs_enet/mii-fec.c | =A0 37 +++++++++++++++++++++++++++++++=
++----
> =A01 files changed, 33 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/fs_enet/mii-fec.c b/drivers/net/fs_enet/mii-fec.=
c
> index 75a0999..5176986 100644
> --- a/drivers/net/fs_enet/mii-fec.c
> +++ b/drivers/net/fs_enet/mii-fec.c
> @@ -36,6 +36,7 @@
> =A0#include <asm/pgtable.h>
> =A0#include <asm/irq.h>
> =A0#include <asm/uaccess.h>
> +#include <asm/mpc5xxx.h>
>
> =A0#include "fs_enet.h"
> =A0#include "fec.h"
> @@ -103,11 +104,11 @@ static int fs_enet_fec_mii_reset(struct mii_bus *bu=
s)
> =A0static int __devinit fs_enet_mdio_probe(struct of_device *ofdev,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 const struct of_device_id *match)
> =A0{
> - =A0 =A0 =A0 struct device_node *np =3D NULL;
> =A0 =A0 =A0 =A0struct resource res;
> =A0 =A0 =A0 =A0struct mii_bus *new_bus;
> =A0 =A0 =A0 =A0struct fec_info *fec;
> - =A0 =A0 =A0 int ret =3D -ENOMEM, i;
> + =A0 =A0 =A0 int (*get_bus_freq)(struct device_node *) =3D match->data;
> + =A0 =A0 =A0 int ret =3D -ENOMEM, clock, speed;
>
> =A0 =A0 =A0 =A0new_bus =3D mdiobus_alloc();
> =A0 =A0 =A0 =A0if (!new_bus)
> @@ -133,13 +134,35 @@ static int __devinit fs_enet_mdio_probe(struct of_d=
evice *ofdev,
> =A0 =A0 =A0 =A0if (!fec->fecp)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto out_fec;
>
> - =A0 =A0 =A0 fec->mii_speed =3D ((ppc_proc_freq + 4999999) / 5000000) <<=
1;
> + =A0 =A0 =A0 if (get_bus_freq) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock =3D get_bus_freq(ofdev->node);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (!clock) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* Use maximum divider if c=
lock is unknown */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_warn(&ofdev->dev, "coul=
d not determine IPS clock\n");
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock =3D 0x3F * 5000000;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 } else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock =3D ppc_proc_freq;
> +
> + =A0 =A0 =A0 /*
> + =A0 =A0 =A0 =A0* Scale for a MII clock <=3D 2.5 MHz
> + =A0 =A0 =A0 =A0* Note that only 6 bits (25:30) are available for MII sp=
eed.
> + =A0 =A0 =A0 =A0*/
> + =A0 =A0 =A0 speed =3D (clock + 4999999) / 5000000;
> + =A0 =A0 =A0 if (speed > 0x3F) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 speed =3D 0x3F;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 dev_err(&ofdev->dev,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 "MII clock (%d Hz) exceeds =
max (2.5 MHz)\n",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 clock / speed);
> + =A0 =A0 =A0 }
> +
> + =A0 =A0 =A0 fec->mii_speed =3D speed << 1;
>
> =A0 =A0 =A0 =A0setbits32(&fec->fecp->fec_r_cntrl, FEC_RCNTRL_MII_MODE);
> =A0 =A0 =A0 =A0setbits32(&fec->fecp->fec_ecntrl, FEC_ECNTRL_PINMUX |
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =
=A0 =A0 =A0FEC_ECNTRL_ETHER_EN);
> =A0 =A0 =A0 =A0out_be32(&fec->fecp->fec_ievent, FEC_ENET_MII);
> - =A0 =A0 =A0 out_be32(&fec->fecp->fec_mii_speed, fec->mii_speed);
> + =A0 =A0 =A0 clrsetbits_be32(&fec->fecp->fec_mii_speed, 0x7E, fec->mii_s=
peed);
>
> =A0 =A0 =A0 =A0new_bus->phy_mask =3D ~0;
> =A0 =A0 =A0 =A0new_bus->irq =3D kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_K=
ERNEL);
> @@ -188,6 +211,12 @@ static struct of_device_id fs_enet_mdio_fec_match[] =
=3D {
> =A0 =A0 =A0 =A0{
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0.compatible =3D "fsl,pq1-fec-mdio",
> =A0 =A0 =A0 =A0},
> +#if defined(CONFIG_PPC_MPC512x)
> + =A0 =A0 =A0 {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 .compatible =3D "fsl,mpc5121-fec-mdio",
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 .data =3D mpc5xxx_get_bus_frequency,
> + =A0 =A0 =A0 },
> +#endif
> =A0 =A0 =A0 =A0{},
> =A0};
>
> --
> 1.6.0.6
>
>
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH] powerpc/cell: strncpy does not null terminate string
From: Roel Kluin @ 2009-07-17 14:27 UTC (permalink / raw)
To: arnd; +Cc: linuxppc-dev, Andrew Morton, cbe-oss-dev
In-Reply-To: <4A607185.6020302@gmail.com>
strlcpy() will always null terminate the string.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
Please use this one instead
diff --git a/arch/powerpc/platforms/cell/celleb_setup.c b/arch/powerpc/platforms/cell/celleb_setup.c
index 07c234f..1896cd8 100644
--- a/arch/powerpc/platforms/cell/celleb_setup.c
+++ b/arch/powerpc/platforms/cell/celleb_setup.c
@@ -80,7 +80,7 @@ static void celleb_show_cpuinfo(struct seq_file *m)
static int __init celleb_machine_type_hack(char *ptr)
{
- strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
+ strlcpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
celleb_machine_type[sizeof(celleb_machine_type)-1] = 0;
return 0;
}
^ permalink raw reply related
* [PATCH 2/2 v4] MPC52xx FEC: be more conservative when setting MII_SPEED register
From: Wolfgang Denk @ 2009-07-17 12:59 UTC (permalink / raw)
To: linuxppc-dev; +Cc: netdev, Wolfgang Denk
In-Reply-To: <1247833628-15952-2-git-send-email-wd@denx.de>
This patch adds error checking and prevents clobbering unrelated bits
(reserved bits or the DIS_PREAMBLE bit) when writing the MII_SPEED
register on MPC52xx systems.
Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: <netdev@vger.kernel.org>
---
v3: - use maximum divider in case MPC512x IPS clock is unknown
v4: - use the same code in the probe-function, too
drivers/net/fec_mpc52xx.c | 25 ++++++++++++++++++++++---
drivers/net/fec_mpc52xx_phy.c | 23 ++++++++++++++++++++---
2 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index cc78633..eed8d2b 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -639,7 +639,7 @@ static void mpc52xx_fec_hw_init(struct net_device *dev)
/* set phy speed.
* this can't be done in phy driver, since it needs to be called
* before fec stuff (even on resume) */
- out_be32(&fec->mii_speed, priv->mdio_speed);
+ clrsetbits_be32(&fec->mii_speed, 0x7E, priv->mdio_speed);
}
/**
@@ -863,7 +863,7 @@ mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
struct mpc52xx_fec_priv *priv = NULL;
struct resource mem;
const u32 *prop;
- int prop_size;
+ int prop_size, clock, speed;
phys_addr_t rx_fifo;
phys_addr_t tx_fifo;
@@ -948,7 +948,26 @@ mpc52xx_fec_probe(struct of_device *op, const struct of_device_id *match)
/* Start with safe defaults for link connection */
priv->speed = 100;
priv->duplex = DUPLEX_HALF;
- priv->mdio_speed = ((mpc5xxx_get_bus_frequency(op->node) >> 20) / 5) << 1;
+
+ /* MII speed */
+ clock = mpc5xxx_get_bus_frequency(op->node);
+ if (!clock) {
+ /* Use maximum divider if clock is unknown */
+ dev_err(&op->dev, "could not determine IPB clock\n");
+ clock = 0x3F * 5000000;
+ }
+
+ /*
+ * Scale for a MII clock <= 2.5 MHz
+ * Note that only 6 bits (25:30) are available for MII speed.
+ */
+ speed = (clock + 4999999) / 5000000;
+ if (speed > 0x3F) {
+ speed = 0x3F;
+ dev_err(&op->dev, "MII clock (%d Hz) exceeds max (2.5 MHz)\n",
+ clock / speed);
+ }
+ priv->mdio_speed = speed << 1;
/* The current speed preconfigures the speed of the MII link */
prop = of_get_property(op->node, "current-speed", &prop_size);
diff --git a/drivers/net/fec_mpc52xx_phy.c b/drivers/net/fec_mpc52xx_phy.c
index 31e6d62..d3537e1 100644
--- a/drivers/net/fec_mpc52xx_phy.c
+++ b/drivers/net/fec_mpc52xx_phy.c
@@ -70,7 +70,7 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
struct mpc52xx_fec_mdio_priv *priv;
struct resource res = {};
int err;
- int i;
+ int i, clock, speed;
bus = mdiobus_alloc();
if (bus == NULL)
@@ -105,8 +105,25 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
dev_set_drvdata(dev, bus);
/* set MII speed */
- out_be32(&priv->regs->mii_speed,
- ((mpc5xxx_get_bus_frequency(of->node) >> 20) / 5) << 1);
+ clock = mpc5xxx_get_bus_frequency(of->node);
+ if (!clock) {
+ /* Use maximum divider if clock is unknown */
+ dev_err(&of->dev, "could not determine IPB clock\n");
+ clock = 0x3F * 5000000;
+ }
+
+ /*
+ * Scale for a MII clock <= 2.5 MHz
+ * Note that only 6 bits (25:30) are available for MII speed.
+ */
+ speed = (clock + 4999999) / 5000000;
+ if (speed > 0x3F) {
+ speed = 0x3F;
+ dev_err(&of->dev, "MII clock (%d Hz) exceeds max (2.5 MHz)\n",
+ clock / speed);
+ }
+
+ clrsetbits_be32(&priv->regs->mii_speed, 0x7E, speed << 1);
err = of_mdiobus_register(bus, np);
if (err)
--
1.6.0.6
^ permalink raw reply related
* [PATCH] powerpc/cell: strncpy does not null terminate string
From: Roel Kluin @ 2009-07-17 12:41 UTC (permalink / raw)
To: arnd, linuxppc-dev, cbe-oss-dev, Andrew Morton
With `sizeof(string) - 1` strncpy() will null terminate the string.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
To test this:
#include <stdio.h>
#include <string.h>
char a[10];
char b[10];
int main()
{
const char* str = "0123456789012";
strncpy(a, str, sizeof(a));
strncpy(b, str, sizeof(b) - 1);
printf("String a was %s, b was %s\n", a, b);
return 0;
}
Output:
String a was 0123456789012345678, b was 012345678
diff --git a/arch/powerpc/platforms/cell/celleb_setup.c b/arch/powerpc/platforms/cell/celleb_setup.c
index 07c234f..cfdbadb 100644
--- a/arch/powerpc/platforms/cell/celleb_setup.c
+++ b/arch/powerpc/platforms/cell/celleb_setup.c
@@ -80,7 +80,7 @@ static void celleb_show_cpuinfo(struct seq_file *m)
static int __init celleb_machine_type_hack(char *ptr)
{
- strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type));
+ strncpy(celleb_machine_type, ptr, sizeof(celleb_machine_type) - 1);
celleb_machine_type[sizeof(celleb_machine_type)-1] = 0;
return 0;
}
^ permalink raw reply related
* [PATCH] mtd/maps: add mtd-ram support to physmap_of
From: Wolfram Sang @ 2009-07-17 12:39 UTC (permalink / raw)
To: linux-mtd
Cc: devicetree-discuss, Vitaly Wool, Artem Bityutskiy,
Albrecht Dreß, linuxppc-dev, Ken MacLeod, David Woodhouse
Use physmap_of to access RAMs as mtd and add documenation for it. This approach
is a lot less intrusive as adding an of-wrapper around plat-ram.c. As most
extensions of plat-ram.c (e.g. custom map-functions) can't be mapped to the
device tree anyhow, extending physmap_of seems to be the cleanest approach.
Tested with a phyCORE-MPC5121e.
Signed-off-by: Wolfram Sang <w.sang@pengutronix.de>
Cc: Vitaly Wool <vwool@ru.mvista.com>
Cc: Artem Bityutskiy <dedekind@infradead.org>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Ken MacLeod <ken@bitsko.slc.ut.us>
Cc: Albrecht Dreß <albrecht.dress@arcor.de>
---
Documentation/powerpc/dts-bindings/mtd-physmap.txt | 42 ++++++++++++-------
drivers/mtd/maps/physmap_of.c | 4 ++
2 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/Documentation/powerpc/dts-bindings/mtd-physmap.txt b/Documentation/powerpc/dts-bindings/mtd-physmap.txt
index 667c9bd..80152cb 100644
--- a/Documentation/powerpc/dts-bindings/mtd-physmap.txt
+++ b/Documentation/powerpc/dts-bindings/mtd-physmap.txt
@@ -1,18 +1,19 @@
-CFI or JEDEC memory-mapped NOR flash
+CFI or JEDEC memory-mapped NOR flash, MTD-RAM (NVRAM...)
Flash chips (Memory Technology Devices) are often used for solid state
file systems on embedded devices.
- - compatible : should contain the specific model of flash chip(s)
- used, if known, followed by either "cfi-flash" or "jedec-flash"
- - reg : Address range(s) of the flash chip(s)
+ - compatible : should contain the specific model of mtd chip(s)
+ used, if known, followed by either "cfi-flash", "jedec-flash"
+ or "mtd-ram".
+ - reg : Address range(s) of the mtd chip(s)
It's possible to (optionally) define multiple "reg" tuples so that
- non-identical NOR chips can be described in one flash node.
- - bank-width : Width (in bytes) of the flash bank. Equal to the
+ non-identical chips can be described in one node.
+ - bank-width : Width (in bytes) of the bank. Equal to the
device width times the number of interleaved chips.
- - device-width : (optional) Width of a single flash chip. If
+ - device-width : (optional) Width of a single mtd chip. If
omitted, assumed to be equal to 'bank-width'.
- - #address-cells, #size-cells : Must be present if the flash has
+ - #address-cells, #size-cells : Must be present if the device has
sub-nodes representing partitions (see below). In this case
both #address-cells and #size-cells must be equal to 1.
@@ -22,24 +23,24 @@ are defined:
- vendor-id : Contains the flash chip's vendor id (1 byte).
- device-id : Contains the flash chip's device id (1 byte).
-In addition to the information on the flash bank itself, the
+In addition to the information on the mtd bank itself, the
device tree may optionally contain additional information
-describing partitions of the flash address space. This can be
+describing partitions of the address space. This can be
used on platforms which have strong conventions about which
-portions of the flash are used for what purposes, but which don't
+portions of a flash are used for what purposes, but which don't
use an on-flash partition table such as RedBoot.
-Each partition is represented as a sub-node of the flash device.
+Each partition is represented as a sub-node of the mtd device.
Each node's name represents the name of the corresponding
-partition of the flash device.
+partition of the mtd device.
Flash partitions
- - reg : The partition's offset and size within the flash bank.
- - label : (optional) The label / name for this flash partition.
+ - reg : The partition's offset and size within the mtd bank.
+ - label : (optional) The label / name for this partition.
If omitted, the label is taken from the node name (excluding
the unit address).
- read-only : (optional) This parameter, if present, is a hint to
- Linux that this flash partition should only be mounted
+ Linux that this partition should only be mounted
read-only. This is usually used for flash partitions
containing early-boot firmware images or data which should not
be clobbered.
@@ -78,3 +79,12 @@ Here an example with multiple "reg" tuples:
reg = <0 0x04000000>;
};
};
+
+An example using SRAM:
+
+ sram@2,0 {
+ compatible = "samsung,k6f1616u6a", "mtd-ram";
+ reg = <2 0 0x00200000>;
+ bank-width = <2>;
+ };
+
diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c
index 39d357b..45eee20 100644
--- a/drivers/mtd/maps/physmap_of.c
+++ b/drivers/mtd/maps/physmap_of.c
@@ -360,6 +360,10 @@ static struct of_device_id of_flash_match[] = {
.data = (void *)"jedec_probe",
},
{
+ .compatible = "mtd-ram",
+ .data = (void *)"map_ram",
+ },
+ {
.type = "rom",
.compatible = "direct-mapped"
},
--
1.6.3.1
^ permalink raw reply related
* Re: [PATCH 2/2 v2] MPC52xx FEC: be more conservative when setting MII_SPEED register
From: Wolfgang Denk @ 2009-07-17 12:35 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linuxppc-dev, netdev
In-Reply-To: <20090717094725.GC3150@pengutronix.de>
Dear Wolfram Sang,
In message <20090717094725.GC3150@pengutronix.de> you wrote:
>
> In the probe-function when mdio_speed is set, there is still the old formula
> used. Wouldn't that be better in sync?
Good point. Next version of patch following soon.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Prof: So the American government went to IBM to come up with a
data encryption standard and they came up with ...
Student: EBCDIC!
^ permalink raw reply
* Re: [PATCH 1/2 v4] fs_enet/mii-fec.c: fix MII speed calculation
From: Wolfgang Denk @ 2009-07-17 12:32 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linuxppc-dev, netdev
In-Reply-To: <20090717093307.GB3150@pengutronix.de>
Dear Wolfram Sang,
In message <20090717093307.GB3150@pengutronix.de> you wrote:
>
...
> > @@ -188,6 +209,12 @@ static struct of_device_id fs_enet_mdio_fec_match[] = {
> > {
> > .compatible = "fsl,pq1-fec-mdio",
> > },
> > +#if defined(CONFIG_PPC_MPC512x)
> > + {
> > + .compatible = "fsl,mpc5121-fec-mdio",
> > + .data = mpc5xxx_get_bus_frequency,
> > + },
> > +#endif
>
> Grepping through 'drivers/*' I see that #ifdefing compatible-entries is highly
> uncommon (just 3 hits). I think a guideline would be useful. Most people like
> to avoid #ifdefs at any cost, while I personally think it doesn't spoil
> readability too much here. Other opinions?
An older version of the patch tried to "hide" the ifdef in a 512x
specific header, so at least common code would remain clean, but I
agree with Grant that this current version looks cleaner globally.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"You can have my Unix system when you pry it from my cold, dead
fingers." - Cal Keegan
^ permalink raw reply
* [PATCH 1/2 v5] fs_enet/mii-fec.c: fix MII speed calculation
From: Wolfgang Denk @ 2009-07-17 12:27 UTC (permalink / raw)
To: linuxppc-dev; +Cc: netdev, Wolfgang Denk
In-Reply-To: <1247780546-4426-1-git-send-email-wd@denx.de>
The MII speed calculation was based on the CPU clock (ppc_proc_freq),
but for MPC512x we must use the bus clock instead.
This patch makes it use the correct clock and makes sure we don't
clobber reserved bits in the MII_SPEED register.
Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: <netdev@vger.kernel.org>
---
v5: - fix divider so we really use 2.5 MHz (instead of 1.25)
- use maximum divider in case MPC512x IPS clock is unknown
drivers/net/fs_enet/mii-fec.c | 37 +++++++++++++++++++++++++++++++++----
1 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/net/fs_enet/mii-fec.c b/drivers/net/fs_enet/mii-fec.c
index 75a0999..5176986 100644
--- a/drivers/net/fs_enet/mii-fec.c
+++ b/drivers/net/fs_enet/mii-fec.c
@@ -36,6 +36,7 @@
#include <asm/pgtable.h>
#include <asm/irq.h>
#include <asm/uaccess.h>
+#include <asm/mpc5xxx.h>
#include "fs_enet.h"
#include "fec.h"
@@ -103,11 +104,11 @@ static int fs_enet_fec_mii_reset(struct mii_bus *bus)
static int __devinit fs_enet_mdio_probe(struct of_device *ofdev,
const struct of_device_id *match)
{
- struct device_node *np = NULL;
struct resource res;
struct mii_bus *new_bus;
struct fec_info *fec;
- int ret = -ENOMEM, i;
+ int (*get_bus_freq)(struct device_node *) = match->data;
+ int ret = -ENOMEM, clock, speed;
new_bus = mdiobus_alloc();
if (!new_bus)
@@ -133,13 +134,35 @@ static int __devinit fs_enet_mdio_probe(struct of_device *ofdev,
if (!fec->fecp)
goto out_fec;
- fec->mii_speed = ((ppc_proc_freq + 4999999) / 5000000) << 1;
+ if (get_bus_freq) {
+ clock = get_bus_freq(ofdev->node);
+ if (!clock) {
+ /* Use maximum divider if clock is unknown */
+ dev_warn(&ofdev->dev, "could not determine IPS clock\n");
+ clock = 0x3F * 5000000;
+ }
+ } else
+ clock = ppc_proc_freq;
+
+ /*
+ * Scale for a MII clock <= 2.5 MHz
+ * Note that only 6 bits (25:30) are available for MII speed.
+ */
+ speed = (clock + 4999999) / 5000000;
+ if (speed > 0x3F) {
+ speed = 0x3F;
+ dev_err(&ofdev->dev,
+ "MII clock (%d Hz) exceeds max (2.5 MHz)\n",
+ clock / speed);
+ }
+
+ fec->mii_speed = speed << 1;
setbits32(&fec->fecp->fec_r_cntrl, FEC_RCNTRL_MII_MODE);
setbits32(&fec->fecp->fec_ecntrl, FEC_ECNTRL_PINMUX |
FEC_ECNTRL_ETHER_EN);
out_be32(&fec->fecp->fec_ievent, FEC_ENET_MII);
- out_be32(&fec->fecp->fec_mii_speed, fec->mii_speed);
+ clrsetbits_be32(&fec->fecp->fec_mii_speed, 0x7E, fec->mii_speed);
new_bus->phy_mask = ~0;
new_bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
@@ -188,6 +211,12 @@ static struct of_device_id fs_enet_mdio_fec_match[] = {
{
.compatible = "fsl,pq1-fec-mdio",
},
+#if defined(CONFIG_PPC_MPC512x)
+ {
+ .compatible = "fsl,mpc5121-fec-mdio",
+ .data = mpc5xxx_get_bus_frequency,
+ },
+#endif
{},
};
--
1.6.0.6
^ permalink raw reply related
* [PATCH 2/2 v3] MPC52xx FEC: be more conservative when setting MII_SPEED register
From: Wolfgang Denk @ 2009-07-17 12:27 UTC (permalink / raw)
To: linuxppc-dev; +Cc: netdev, Wolfgang Denk
In-Reply-To: <1247780546-4426-1-git-send-email-wd@denx.de>
This patch adds error checking and prevents clobbering unrelated bits
(reserved bits or the DIS_PREAMBLE bit) when writing the MII_SPEED
register on MPC52xx systems.
Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Grant Likely <grant.likely@secretlab.ca>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: <netdev@vger.kernel.org>
---
v3: - use maximum divider in case MPC512x IPS clock is unknown
drivers/net/fec_mpc52xx.c | 2 +-
drivers/net/fec_mpc52xx_phy.c | 23 ++++++++++++++++++++---
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
index cc78633..b69d440 100644
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -639,7 +639,7 @@ static void mpc52xx_fec_hw_init(struct net_device *dev)
/* set phy speed.
* this can't be done in phy driver, since it needs to be called
* before fec stuff (even on resume) */
- out_be32(&fec->mii_speed, priv->mdio_speed);
+ clrsetbits_be32(&fec->mii_speed, 0x7E, priv->mdio_speed);
}
/**
diff --git a/drivers/net/fec_mpc52xx_phy.c b/drivers/net/fec_mpc52xx_phy.c
index 31e6d62..d3537e1 100644
--- a/drivers/net/fec_mpc52xx_phy.c
+++ b/drivers/net/fec_mpc52xx_phy.c
@@ -70,7 +70,7 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
struct mpc52xx_fec_mdio_priv *priv;
struct resource res = {};
int err;
- int i;
+ int i, clock, speed;
bus = mdiobus_alloc();
if (bus == NULL)
@@ -105,8 +105,25 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
dev_set_drvdata(dev, bus);
/* set MII speed */
- out_be32(&priv->regs->mii_speed,
- ((mpc5xxx_get_bus_frequency(of->node) >> 20) / 5) << 1);
+ clock = mpc5xxx_get_bus_frequency(of->node);
+ if (!clock) {
+ /* Use maximum divider if clock is unknown */
+ dev_err(&of->dev, "could not determine IPB clock\n");
+ clock = 0x3F * 5000000;
+ }
+
+ /*
+ * Scale for a MII clock <= 2.5 MHz
+ * Note that only 6 bits (25:30) are available for MII speed.
+ */
+ speed = (clock + 4999999) / 5000000;
+ if (speed > 0x3F) {
+ speed = 0x3F;
+ dev_err(&of->dev, "MII clock (%d Hz) exceeds max (2.5 MHz)\n",
+ clock / speed);
+ }
+
+ clrsetbits_be32(&priv->regs->mii_speed, 0x7E, speed << 1);
err = of_mdiobus_register(bus, np);
if (err)
--
1.6.0.6
^ permalink raw reply related
* Re: [PATCH 2/2 v2] MPC52xx FEC: be more conservative when setting MII_SPEED register
From: Wolfgang Denk @ 2009-07-17 12:25 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, netdev
In-Reply-To: <fa686aa40907161548n1658afaei8dce2d893222019c@mail.gmail.com>
Dear Grant Likely,
In message <fa686aa40907161548n1658afaei8dce2d893222019c@mail.gmail.com> you wrote:
>
> Mostly good. One comment below. When it's resolved, feel free to add
> my acked-by line. Thanks for getting this done.
...
> Just thought of something. If it cannot find the clock, then wouldn't
> it be better to just use the maximum divider and print a warning
> instead of bailing completely? This goes for the other patch as well.
Will change this for both patches. Thanks.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Program maintenance is an entropy-increasing process, and even its
most skilfull execution only delays the subsidence of the system into
unfixable obsolescence. - Fred Brooks, "The Mythical Man Month"
^ permalink raw reply
* Re: [PATCH 1/2 v4] fs_enet/mii-fec.c: fix MII speed calculation
From: Wolfgang Denk @ 2009-07-17 12:24 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, netdev
In-Reply-To: <fa686aa40907161544je92317dy7abea6819746847@mail.gmail.com>
Dear Grant Likely,
In message <fa686aa40907161544je92317dy7abea6819746847@mail.gmail.com> you wrote:
>
> I assume this is tested. I have not tested this on my board, and I've
It was tested, but as it turns out not quite well :-(
> got one question below, but otherwise I think I can say....
...
> The calculation has changed here for non mpc5121 users. Shouldn't the
> "clock = ppc_proc_freq;" line above be "clock = ppc_proc_freq / 2;"?
> Or was this also a bug in the original driver?
You are right. That's a bug in the new code. Fixed in next version of
the patch.
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
Swap read error. You lose your mind.
^ permalink raw reply
* USB Device not working in MPC8280 PowerPC
From: Gurumurthy Gowdar @ 2009-07-17 10:31 UTC (permalink / raw)
To: linuxppc-dev; +Cc: dev-owner
[-- Attachment #1: Type: text/plain, Size: 1116 bytes --]
Dear LinuxPPC members,
am facing problem on writing USB
device port driver for MPC8280,when i connect it to the host PC through USB
cable .device descriptor is happening host is setting the address but device
configuration is not happening properly...it hangs there in USB busy
handler.
below is the output of the snippet in hyperterminal for usb debugging
Debug statements
GetDescriptor request length = 0x40
Device Descriptor...
bd status = 3ec0
In TX USB event register = 108
Recieved PID token = 0x3cc0
USB:SetAddress 2
bd status = 3800
bd status = 3ac0
In TX USB event register = 108
GetDescriptor request length = 0x12
Device Descriptor...
bd status = 3ec0
In TX USB event register = 109
Recieved PID token = 0x3cc0
In USB_BUSY HANDLER : 0x108
it hangs in usb busy handler , am feeling data pid toggling may be the
issue.
please help me out to sort this issue....let me know if any codes are
available for USB Device & USB Host testing for MPC8280.
am testing in FreeScale Code Warrior IDE
waiting for the reply ASAP
Regards,
Gurumurthy
--
Gurumurthy Gowdar
[-- Attachment #2: Type: text/html, Size: 1255 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2 v2] MPC52xx FEC: be more conservative when setting MII_SPEED register
From: Wolfram Sang @ 2009-07-17 9:47 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: linuxppc-dev, netdev
In-Reply-To: <1247780546-4426-2-git-send-email-wd@denx.de>
[-- Attachment #1: Type: text/plain, Size: 2892 bytes --]
Hi Wolfgang,
On Thu, Jul 16, 2009 at 11:42:26PM +0200, Wolfgang Denk wrote:
> This patch adds error checking and prevents clobbering unrelated bits
> (reserved bits or the DIS_PREAMBLE bit) when writing the MII_SPEED
> register on MPC52xx systems.
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: <netdev@vger.kernel.org>
> ---
> drivers/net/fec_mpc52xx.c | 2 +-
> drivers/net/fec_mpc52xx_phy.c | 21 ++++++++++++++++++---
> 2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/fec_mpc52xx.c b/drivers/net/fec_mpc52xx.c
> index cc78633..b69d440 100644
> --- a/drivers/net/fec_mpc52xx.c
> +++ b/drivers/net/fec_mpc52xx.c
> @@ -639,7 +639,7 @@ static void mpc52xx_fec_hw_init(struct net_device *dev)
> /* set phy speed.
> * this can't be done in phy driver, since it needs to be called
> * before fec stuff (even on resume) */
> - out_be32(&fec->mii_speed, priv->mdio_speed);
> + clrsetbits_be32(&fec->mii_speed, 0x7E, priv->mdio_speed);
> }
In the probe-function when mdio_speed is set, there is still the old formula
used. Wouldn't that be better in sync?
>
> /**
> diff --git a/drivers/net/fec_mpc52xx_phy.c b/drivers/net/fec_mpc52xx_phy.c
> index 31e6d62..4c33dc5 100644
> --- a/drivers/net/fec_mpc52xx_phy.c
> +++ b/drivers/net/fec_mpc52xx_phy.c
> @@ -70,7 +70,7 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
> struct mpc52xx_fec_mdio_priv *priv;
> struct resource res = {};
> int err;
> - int i;
> + int i, clock, speed;
>
> bus = mdiobus_alloc();
> if (bus == NULL)
> @@ -105,8 +105,23 @@ static int mpc52xx_fec_mdio_probe(struct of_device *of,
> dev_set_drvdata(dev, bus);
>
> /* set MII speed */
> - out_be32(&priv->regs->mii_speed,
> - ((mpc5xxx_get_bus_frequency(of->node) >> 20) / 5) << 1);
> + clock = mpc5xxx_get_bus_frequency(of->node);
> + if (!clock) {
> + dev_err(&of->dev, "could not determine IPS/IPB clock\n");
> + goto out_unmap;
> + }
> +
> + /* scale for a MII clock <= 2.5 MHz */
> + speed = (clock + 2499999) / 2500000;
> +
> + /* only 6 bits (25:30) available for MII speed */
> + if (speed > 0x3F) {
> + speed = 0x3F;
> + dev_err(&of->dev, "MII clock (%d Hz) exceeds max (2.5 MHz)\n",
> + clock / speed);
> + }
> +
> + clrsetbits_be32(&priv->regs->mii_speed, 0x7E, speed << 1);
>
> err = of_mdiobus_register(bus, np);
> if (err)
> --
> 1.6.0.6
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2 v4] fs_enet/mii-fec.c: fix MII speed calculation
From: Wolfram Sang @ 2009-07-17 9:33 UTC (permalink / raw)
To: Wolfgang Denk; +Cc: linuxppc-dev, netdev
In-Reply-To: <1247780546-4426-1-git-send-email-wd@denx.de>
[-- Attachment #1: Type: text/plain, Size: 3411 bytes --]
Hi,
On Thu, Jul 16, 2009 at 11:42:25PM +0200, Wolfgang Denk wrote:
> The MII speed calculation was based on the CPU clock (ppc_proc_freq),
> but for MPC512x we must use the bus clock instead.
>
> This patch makes it use the correct clock and makes sure we don't
> clobber reserved bits in the MII_SPEED register.
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Grant Likely <grant.likely@secretlab.ca>
> Cc: Kumar Gala <galak@kernel.crashing.org>
> Cc: <netdev@vger.kernel.org>
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> ---
> drivers/net/fs_enet/mii-fec.c | 35 +++++++++++++++++++++++++++++++----
> 1 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/fs_enet/mii-fec.c b/drivers/net/fs_enet/mii-fec.c
> index 75a0999..62b2d7a 100644
> --- a/drivers/net/fs_enet/mii-fec.c
> +++ b/drivers/net/fs_enet/mii-fec.c
> @@ -103,11 +103,11 @@ static int fs_enet_fec_mii_reset(struct mii_bus *bus)
> static int __devinit fs_enet_mdio_probe(struct of_device *ofdev,
> const struct of_device_id *match)
> {
> - struct device_node *np = NULL;
> struct resource res;
> struct mii_bus *new_bus;
> struct fec_info *fec;
> - int ret = -ENOMEM, i;
> + int (*get_bus_freq)(struct device_node *) = match->data;
> + int ret = -ENOMEM, clock, speed;
>
> new_bus = mdiobus_alloc();
> if (!new_bus)
> @@ -133,13 +133,34 @@ static int __devinit fs_enet_mdio_probe(struct of_device *ofdev,
> if (!fec->fecp)
> goto out_fec;
>
> - fec->mii_speed = ((ppc_proc_freq + 4999999) / 5000000) << 1;
> + if (get_bus_freq) {
> + clock = get_bus_freq(ofdev->node);
> +
> + if (!clock) {
> + dev_err(&ofdev->dev, "could not determine IPS/IPB clock\n");
> + goto out_unmap_regs;
> + }
> + } else
> + clock = ppc_proc_freq;
> +
> + /* scale for a MII clock <= 2.5 MHz */
> + speed = (clock + 2499999) / 2500000;
> +
> + /* only 6 bits (25:30) available for MII speed */
> + if (speed > 0x3F) {
> + speed = 0x3F;
> + dev_err(&ofdev->dev,
> + "MII clock (%d Hz) exceeds max (2.5 MHz)\n",
> + clock / speed);
> + }
> +
> + fec->mii_speed = speed << 1;
>
> setbits32(&fec->fecp->fec_r_cntrl, FEC_RCNTRL_MII_MODE);
> setbits32(&fec->fecp->fec_ecntrl, FEC_ECNTRL_PINMUX |
> FEC_ECNTRL_ETHER_EN);
> out_be32(&fec->fecp->fec_ievent, FEC_ENET_MII);
> - out_be32(&fec->fecp->fec_mii_speed, fec->mii_speed);
> + clrsetbits_be32(&fec->fecp->fec_mii_speed, 0x7E, fec->mii_speed);
>
> new_bus->phy_mask = ~0;
> new_bus->irq = kmalloc(sizeof(int) * PHY_MAX_ADDR, GFP_KERNEL);
> @@ -188,6 +209,12 @@ static struct of_device_id fs_enet_mdio_fec_match[] = {
> {
> .compatible = "fsl,pq1-fec-mdio",
> },
> +#if defined(CONFIG_PPC_MPC512x)
> + {
> + .compatible = "fsl,mpc5121-fec-mdio",
> + .data = mpc5xxx_get_bus_frequency,
> + },
> +#endif
Grepping through 'drivers/*' I see that #ifdefing compatible-entries is highly
uncommon (just 3 hits). I think a guideline would be useful. Most people like
to avoid #ifdefs at any cost, while I personally think it doesn't spoil
readability too much here. Other opinions?
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] kmemleak: Allow kmemleak to be built on powerpc
From: Catalin Marinas @ 2009-07-17 8:41 UTC (permalink / raw)
To: michael; +Cc: linuxppc-dev
In-Reply-To: <1247819536.19156.13.camel@concordia>
On Fri, 2009-07-17 at 18:32 +1000, Michael Ellerman wrote:
> On Fri, 2009-07-17 at 09:26 +0100, Catalin Marinas wrote:
> > On Fri, 2009-07-17 at 10:29 +1000, Michael Ellerman wrote:
> > > The wrinkle is that lmb never frees, so by definition it can't leak :)
> >
> > You can pass min_count = 0 to kmemleak_alloc() so that it would never
> > report such blocks as leaks (see the alloc_bootmem hooks).
>
> OK. I couldn't see any description of what min_count meant anywhere,
There isn't any, indeed. I'll try to add some description to the
kmemleak api. But it basically means the minimum number a pointer value
should be found during scanning so that it is not reported as a leak. If
this value is 0, it would never be reported. The
kmalloc/kmem_cache_alloc have this value 1 and vmalloc is higher because
of other structures like vm_area holding pointers o the same block.
--
Catalin
^ permalink raw reply
* Re: [PATCH] kmemleak: Allow kmemleak to be built on powerpc
From: Michael Ellerman @ 2009-07-17 8:32 UTC (permalink / raw)
To: Catalin Marinas; +Cc: linuxppc-dev
In-Reply-To: <1247819195.32760.7.camel@pc1117.cambridge.arm.com>
[-- Attachment #1: Type: text/plain, Size: 2462 bytes --]
On Fri, 2009-07-17 at 09:26 +0100, Catalin Marinas wrote:
> On Fri, 2009-07-17 at 10:29 +1000, Michael Ellerman wrote:
> > On Thu, 2009-07-16 at 18:52 +0100, Catalin Marinas wrote:
> > > On Thu, 2009-07-16 at 17:43 +1000, Michael Ellerman wrote:
> > > > On Thu, 2009-07-16 at 11:25 +1000, Michael Ellerman wrote:
> > > > > Very lightly tested, doesn't crash the kernel.
> > > > >
> > > > > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > > > > ---
> > > > >
> > > > > It doesn't look like we actually need to add any support in the
> > > > > arch code - or is there something I'm missing?
> > > >
> > > > Hmm, I think we want to add annotations in lib/lmb.c don't we? That's
> > > > our low-level pre-bootmem allocator.
> > >
> > > Yes, I think so (I'm not using this on ARM or x86 so I can't really test
> > > it). Without these hooks, there kmemleak reports aren't that useful
> > > (probably too many).
> >
> > The wrinkle is that lmb never frees, so by definition it can't leak :)
>
> You can pass min_count = 0 to kmemleak_alloc() so that it would never
> report such blocks as leaks (see the alloc_bootmem hooks).
OK. I couldn't see any description of what min_count meant anywhere,
I'll try that though.
> > But we could have memory allocated with lmb that has pointers to other
> > objects allocated later, and we want kmemleak to scan the lmb allocated
> > blocks to find those references.
> >
> > So the question is do we need to annotate lmb so that will happen, or
> > does kmemleak scan all kernel memory, regardless of where it's
> > allocated?
>
> Apart from the data and bss sections, it only scans the memory which was
> explicitly allocated. So, you need these hooks in the lmb code.
OK, cool, I'll do a patch to add them.
> The mainline kernel scans for the task stacks by going through the full
> tasks list. However, traversing this list requires a lock which
> increases the latency quite a lot. For the next merging window, I added
> hooks to the alloc_thread_info/free_thread_info functions. If the ppc
> code has __HAVE_ARCH_THREAD_INFO_ALLOCATOR defined, you may need to add
> some hooks in there as well (but note that kmallloc/kmem_cache_alloc are
> tracked by kmemleak already, so you only need the hooks if the
> thread_info allocator uses __get_free_pages).
We do have our own allocator, but it just uses a kmem_cache, so that
should be fine.
cheers
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: [PATCH] kmemleak: Allow kmemleak to be built on powerpc
From: Catalin Marinas @ 2009-07-17 8:26 UTC (permalink / raw)
To: michael; +Cc: linuxppc-dev
In-Reply-To: <1247790558.16836.4.camel@concordia>
On Fri, 2009-07-17 at 10:29 +1000, Michael Ellerman wrote:
> On Thu, 2009-07-16 at 18:52 +0100, Catalin Marinas wrote:
> > On Thu, 2009-07-16 at 17:43 +1000, Michael Ellerman wrote:
> > > On Thu, 2009-07-16 at 11:25 +1000, Michael Ellerman wrote:
> > > > Very lightly tested, doesn't crash the kernel.
> > > >
> > > > Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> > > > ---
> > > >
> > > > It doesn't look like we actually need to add any support in the
> > > > arch code - or is there something I'm missing?
> > >
> > > Hmm, I think we want to add annotations in lib/lmb.c don't we? That's
> > > our low-level pre-bootmem allocator.
> >
> > Yes, I think so (I'm not using this on ARM or x86 so I can't really test
> > it). Without these hooks, there kmemleak reports aren't that useful
> > (probably too many).
>
> The wrinkle is that lmb never frees, so by definition it can't leak :)
You can pass min_count = 0 to kmemleak_alloc() so that it would never
report such blocks as leaks (see the alloc_bootmem hooks).
> But we could have memory allocated with lmb that has pointers to other
> objects allocated later, and we want kmemleak to scan the lmb allocated
> blocks to find those references.
>
> So the question is do we need to annotate lmb so that will happen, or
> does kmemleak scan all kernel memory, regardless of where it's
> allocated?
Apart from the data and bss sections, it only scans the memory which was
explicitly allocated. So, you need these hooks in the lmb code.
The mainline kernel scans for the task stacks by going through the full
tasks list. However, traversing this list requires a lock which
increases the latency quite a lot. For the next merging window, I added
hooks to the alloc_thread_info/free_thread_info functions. If the ppc
code has __HAVE_ARCH_THREAD_INFO_ALLOCATOR defined, you may need to add
some hooks in there as well (but note that kmallloc/kmem_cache_alloc are
tracked by kmemleak already, so you only need the hooks if the
thread_info allocator uses __get_free_pages).
--
Catalin
^ permalink raw reply
* Re: rtas instantiation when commandline contains mem
From: Michael Ellerman @ 2009-07-17 7:53 UTC (permalink / raw)
To: Benjamin Krill; +Cc: linuxppc-dev
In-Reply-To: <20090717063620.GC22901@codiert.org>
[-- Attachment #1: Type: text/plain, Size: 824 bytes --]
On Fri, 2009-07-17 at 08:36 +0200, Benjamin Krill wrote:
> >> the rtas instantiation (prom_init.c) doesn't work correctly if the
> >> kernel parameter "mem=" is used. The current code doesn't evaluate
> >> the kernel parameter which causes the issue that alloc_down
> >> allocates somewhere in the "real" memory space. So it can
> >> happen that the allocation space is above "mem=".
> >>
> >> Commit 2babf5c2ec2f2d5de3e38d20f7df7fd815fd10c9 removes the
> >> evaluation of "mem=".
> >
> >Ah yes, we don't constraint prom_init.c to mem=, only the kernel
>
> Is that with intent? Or should I provide a patch to include it again?
It was, it seemed to me that mem=x was about limiting kernel memory,
which RTAS isn't really. But I guess it depends how you look at it, and
what you're trying to do.
cheers
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* [PATCH v2 4/4] ucc_geth: Revive fixed link support
From: Grant Likely @ 2009-07-17 7:31 UTC (permalink / raw)
To: avorontsov, davem, afleming, netdev, linuxppc-dev; +Cc: leoli
In-Reply-To: <20090717065220.15652.93331.stgit@localhost.localdomain>
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Since commit 0b9da337dca972e7a4144e298ec3adb8f244d4a4 ("Rework
ucc_geth driver to use of_mdio infrastructure") the fixed-link
support is broken.
This patch fixes the support by removing !ug_info->phy_node check,
and adds a call to of_phy_connect_fixed_link() if a phy is not attached
to the MAC.
Also, remove an old fixed-link code that we don't use any longer.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
drivers/net/ucc_geth.c | 23 +++++++----------------
1 files changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 40c6eba..3b957e6 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -1590,13 +1590,13 @@ static int init_phy(struct net_device *dev)
priv->oldspeed = 0;
priv->oldduplex = -1;
- if (!ug_info->phy_node)
- return 0;
-
phydev = of_phy_connect(dev, ug_info->phy_node, &adjust_link, 0,
priv->phy_interface);
+ if (!phydev)
+ phydev = of_phy_connect_fixed_link(dev, &adjust_link,
+ priv->phy_interface);
if (!phydev) {
- printk("%s: Could not attach to PHY\n", dev->name);
+ dev_err(&dev->dev, "Could not attach to PHY\n");
return -ENODEV;
}
@@ -3608,9 +3608,7 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
struct ucc_geth_private *ugeth = NULL;
struct ucc_geth_info *ug_info;
struct resource res;
- struct device_node *phy;
int err, ucc_num, max_speed = 0;
- const u32 *fixed_link;
const unsigned int *prop;
const char *sprop;
const void *mac_addr;
@@ -3708,15 +3706,8 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
ug_info->uf_info.regs = res.start;
ug_info->uf_info.irq = irq_of_parse_and_map(np, 0);
- fixed_link = of_get_property(np, "fixed-link", NULL);
- if (fixed_link) {
- phy = NULL;
- } else {
- phy = of_parse_phandle(np, "phy-handle", 0);
- if (phy == NULL)
- return -ENODEV;
- }
- ug_info->phy_node = phy;
+
+ ug_info->phy_node = of_parse_phandle(np, "phy-handle", 0);
/* Find the TBI PHY node. If it's not there, we don't support SGMII */
ug_info->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
@@ -3725,7 +3716,7 @@ static int ucc_geth_probe(struct of_device* ofdev, const struct of_device_id *ma
prop = of_get_property(np, "phy-connection-type", NULL);
if (!prop) {
/* handle interface property present in old trees */
- prop = of_get_property(phy, "interface", NULL);
+ prop = of_get_property(ug_info->phy_node, "interface", NULL);
if (prop != NULL) {
phy_interface = enet_to_phy_interface[*prop];
max_speed = enet_to_speed[*prop];
^ permalink raw reply related
* [PATCH v2 0/4] net: Revive fixed link support
From: Grant Likely @ 2009-07-17 7:31 UTC (permalink / raw)
To: avorontsov, davem, afleming, netdev, linuxppc-dev; +Cc: leoli
This series is based on the "Net: Revive fixed link support" series
posted by Anton Vorontsov on June 26.
On June 26, Anton Vorontsov wrote:
> The fixed link support is broken since The Big OF MDIO Rework,
> the rework simply removed most of the code that was needed for
> fixed links.
>
> Too bad I didn't notice this earlier, I saw a bunch of patches
> on the ml, but unfortunately I didn't look very close presuming
> that Grant knew what he was doing. :-) And obviously I didn't
> test linux-next on anything that requires a fixed link.
>
> Anyway, here are four patches. The first one adds the fixed link
> support to a framework, otherwise we'd duplicate the code across
> the drivers (as we did previously), and I tried to keep drivers'
> changes minimal.
I took issue with the approach on the basis of
a) I find the whole dummy phy approach to be a hack and an abuse of the
mdio bus,
b) Handling the lack of a phy is trivial and should just be done within
the MAC driver itself, and
c) the approach caused all drivers using of_phy_connect() to parse the
'fixed-link' property, when only three drivers actually define and
use it.
I wrote and posted a competing patch which removed the usage of a dummy
phy and made each driver handle the lack of a phy gracefully. However,
Anton rightly pointed out that my patch still leaves the regression of
userspace no longer being able to use ethtool to query the link speed
because the affected drivers all depend on phylib for the ethtool
ioctl implementations.
Part of the problem I think is that the phylib code merges two separate
constructs; the construct of an MDIO bus (on which many device may
reside, not all of them PHYs), and the construct of an MII link whose
speed and configuration need to be manipulated. I've run into problems
myself on how best to handle things like Ethernet switches which
definitely do not behave like PHYs and the phylib state machine cannot
be used on them. It seems to me that the whole 'dummy phy' approach
is just an artifact of the phylib model not being quite right yet. I
want to investigate the possibility of separating the two concepts, but
that will require a fair bit of thought and experimentation.
Right now this is a regression situation during the 2.6.31
stabilization, so I don't think it is appropriate to write a new set
of ioctl handlers at this late stage when there is another solution.
I've reworked Anton's patches to constrain the effects to just the
three drivers which use 'fixed-link'.
It kills me to leave the dummy phy approach in place, but the
alternative is to invasive and risky to apply at this stage.
It irks me to say so, but I think Anton's approach of reenabling
the dummy phy is the right think to do,
Anton, once again I don't have hardware to test this, so I rely on you
to tell be if I screwed it up. It has been compile tested.
Cheers,
g.
--
Grant Likely, B.Sc. P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* [PATCH v2 3/4] gianfar: Revive fixed link support
From: Grant Likely @ 2009-07-17 7:31 UTC (permalink / raw)
To: avorontsov, davem, afleming, netdev, linuxppc-dev; +Cc: leoli
In-Reply-To: <20090717065220.15652.93331.stgit@localhost.localdomain>
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Since commit fe192a49118f5b1272317d60c7930ece4e13ae49 ("Rework gianfar
driver to use of_mdio infrastructure") the fixed-link support is
broken, the driver oopses at init_phy():
Unable to handle kernel paging request for data at address 0x000000e4
Faulting instruction address: 0xc01cf298
Oops: Kernel access of bad area, sig: 11 [#1]
[...]
NIP [c01cf298] init_phy+0x80/0xdc
LR [c01cf250] init_phy+0x38/0xdc
Call Trace:
[cf81fe80] [c01d1cf8] gfar_enet_open+0x6c/0x19c
[cf81fea0] [c024494c] dev_open+0xfc/0x134
[cf81fec0] [c0242edc] dev_change_flags+0x84/0x1ac
[cf81fee0] [c0399ee0] ic_open_devs+0x168/0x2d8
[cf81ff20] [c039b2e8] ip_auto_config+0x90/0x2a4
[cf81ff60] [c0003884] do_one_initcall+0x34/0x1a8
This patch fixes the oops, and removes phy_node checks, and adds a call
to of_phy_connect_fixed_link() if a phy isn't attached..
Also, remove an old fixed-link code that we don't use any longer.
Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
drivers/net/gianfar.c | 24 ++++++++----------------
1 files changed, 8 insertions(+), 16 deletions(-)
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 4ae1d25..79fcc2c 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -262,15 +262,6 @@ static int gfar_of_init(struct net_device *dev)
priv->device_flags |= FSL_GIANFAR_DEV_HAS_MAGIC_PACKET;
priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
- if (!priv->phy_node) {
- u32 *fixed_link;
-
- fixed_link = (u32 *)of_get_property(np, "fixed-link", NULL);
- if (!fixed_link) {
- err = -ENODEV;
- goto err_out;
- }
- }
/* Find the TBI PHY. If it's not there, we don't support SGMII */
priv->tbi_node = of_parse_phandle(np, "tbi-handle", 0);
@@ -657,13 +648,14 @@ static int init_phy(struct net_device *dev)
interface = gfar_get_interface(dev);
- if (priv->phy_node) {
- priv->phydev = of_phy_connect(dev, priv->phy_node, &adjust_link,
- 0, interface);
- if (!priv->phydev) {
- dev_err(&dev->dev, "error: Could not attach to PHY\n");
- return -ENODEV;
- }
+ priv->phydev = of_phy_connect(dev, priv->phy_node, &adjust_link, 0,
+ interface);
+ if (!priv->phydev)
+ priv->phydev = of_phy_connect_fixed_link(dev, &adjust_link,
+ interface);
+ if (!priv->phydev) {
+ dev_err(&dev->dev, "could not attach to PHY\n");
+ return -ENODEV;
}
if (interface == PHY_INTERFACE_MODE_SGMII)
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox