* [PATCH] net: fix OF fixed-link property handling on Freescale network device drivers
From: Grant Likely @ 2009-07-03 22:20 UTC (permalink / raw)
To: avorontsov, davem, leoli, afleming, netdev, linuxppc-dev
From: Grant Likely <grant.likely@secretlab.ca>
The MDIO rework patches broke the handling of fixed MII links. This
patch adds parsing of the fixed-link property to the gianfar, ucc-geth
and fs_eth network drivers, and ensures that the MAC will work without
a PHY attachment.
Note: This patch does not use the dummy phy approach previously used as I
think it is an abuse of the MDIO bus infrastructure and it doesn't account
for the possibility of MAC devices using a different binding for the
values in the fixed-link property. The current dummy phy setup code
(which this patch removes) assumes the same data format for all fixed-link
properties which is a bad assumption. fixed-link has not been standardized
for use by all Ethernet drivers.
If a generic interface is needed to control xMII speed, then I think it
would be better to compartmentalize the link speed interface and adapt
both phylib and fixed-link code to use it. That would also provide
an interface for non-phy, non-MDIO devices to manipulate the link state
without pretending to be something that doesn't exist. I think this
would be a simpler and more 'tasteful' structure for handling non-phy
cases.
This patch is not perfect, and I'm not sure that I'm programming the
speed registers in the best place (at of_phy_connect() time as opposed to
phy_start() time), but it does fix the fixed-link handling and keeps the
binding properly contained within the driver, so I think it is the right
approach for solving the fixed-link regression that I caused in 2.6.31.
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---
Anton, can you please review, comment and test? I've tested it on an
mpc8349 board, but that is the only hardware that I have. I've also
probably made mistakes and it needs to be split up into separate patches,
but this is probably a sufficient form for first review. I'll also give
it another once over tomorrow when after I've had a decent night sleep.
Cheers,
g.
arch/powerpc/sysdev/fsl_soc.c | 31 --------
drivers/net/fs_enet/fs_enet-main.c | 38 +++++-----
drivers/net/gianfar.c | 122 +++++++++++++++++---------------
drivers/net/phy/phy.c | 12 +++
drivers/net/phy/phy_device.c | 3 +
drivers/net/ucc_geth.c | 138 +++++++++++++++++++-----------------
6 files changed, 172 insertions(+), 172 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 95dbc64..0b969c6 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -177,37 +177,6 @@ u32 get_baudrate(void)
EXPORT_SYMBOL(get_baudrate);
#endif /* CONFIG_CPM2 */
-#ifdef CONFIG_FIXED_PHY
-static int __init of_add_fixed_phys(void)
-{
- int ret;
- struct device_node *np;
- u32 *fixed_link;
- struct fixed_phy_status status = {};
-
- for_each_node_by_name(np, "ethernet") {
- fixed_link = (u32 *)of_get_property(np, "fixed-link", NULL);
- if (!fixed_link)
- continue;
-
- status.link = 1;
- status.duplex = fixed_link[1];
- status.speed = fixed_link[2];
- status.pause = fixed_link[3];
- status.asym_pause = fixed_link[4];
-
- ret = fixed_phy_add(PHY_POLL, fixed_link[0], &status);
- if (ret) {
- of_node_put(np);
- return ret;
- }
- }
-
- return 0;
-}
-arch_initcall(of_add_fixed_phys);
-#endif /* CONFIG_FIXED_PHY */
-
static enum fsl_usb2_phy_modes determine_usb_phy(const char *phy_type)
{
if (!phy_type)
diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
index b892c3a..39244b2 100644
--- a/drivers/net/fs_enet/fs_enet-main.c
+++ b/drivers/net/fs_enet/fs_enet-main.c
@@ -722,8 +722,6 @@ static void generic_adjust_link(struct net_device *dev)
} else if (fep->oldlink) {
new_state = 1;
fep->oldlink = 0;
- fep->oldspeed = 0;
- fep->oldduplex = -1;
}
if (new_state && netif_msg_link(fep))
@@ -749,25 +747,21 @@ static void fs_adjust_link(struct net_device *dev)
static int fs_init_phy(struct net_device *dev)
{
struct fs_enet_private *fep = netdev_priv(dev);
- struct phy_device *phydev;
- fep->oldlink = 0;
- fep->oldspeed = 0;
- fep->oldduplex = -1;
+ /* If a link is already flagged, then set up initial state */
+ if (fep->oldlink) {
+ netif_carrier_on(dev);
+ fep->ops->restart(dev);
+ }
+
if(fep->fpi->phy_node)
- phydev = of_phy_connect(dev, fep->fpi->phy_node,
+ fep->phydev = of_phy_connect(dev, fep->fpi->phy_node,
&fs_adjust_link, 0,
PHY_INTERFACE_MODE_MII);
- else {
- printk("No phy bus ID specified in BSP code\n");
+ if (!fep->phydev && !fep->oldlink) {
+ dev_err(&dev->dev, "Could not attach to PHY\n");
return -EINVAL;
}
- if (IS_ERR(phydev)) {
- printk(KERN_ERR "%s: Could not attach to PHY\n", dev->name);
- return PTR_ERR(phydev);
- }
-
- fep->phydev = phydev;
return 0;
}
@@ -990,10 +984,8 @@ static int __devinit fs_enet_probe(struct of_device *ofdev,
fpi->rx_copybreak = 240;
fpi->use_napi = 1;
fpi->napi_weight = 17;
+
fpi->phy_node = of_parse_phandle(ofdev->node, "phy-handle", 0);
- if ((!fpi->phy_node) && (!of_get_property(ofdev->node, "fixed-link",
- NULL)))
- goto out_free_fpi;
privsize = sizeof(*fep) +
sizeof(struct sk_buff **) *
@@ -1013,6 +1005,16 @@ static int __devinit fs_enet_probe(struct of_device *ofdev,
fep->fpi = fpi;
fep->ops = match->data;
+ /* Setup the initial link state */
+ data = of_get_property(ofdev->node, "fixed-link", &len);
+ if (data && len >= sizeof(*data) * 3) {
+ fep->oldlink = 1;
+ fep->oldduplex = data[1];
+ fep->oldspeed = data[2];
+ }
+ if (!fpi->phy_node && !fep->oldlink)
+ goto out_free_dev;
+
ret = fep->ops->setup_data(ndev);
if (ret)
goto out_free_dev;
diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 4ae1d25..3cb33e9 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -90,7 +90,6 @@
#include <linux/crc32.h>
#include <linux/mii.h>
#include <linux/phy.h>
-#include <linux/phy_fixed.h>
#include <linux/of.h>
#include "gianfar.h"
@@ -179,6 +178,8 @@ static int gfar_of_init(struct net_device *dev)
const u32 *stash;
const u32 *stash_len;
const u32 *stash_idx;
+ const u32 *prop;
+ int prop_sz;
if (!np || !of_device_is_available(np))
return -ENODEV;
@@ -261,15 +262,18 @@ static int gfar_of_init(struct net_device *dev)
if (of_get_property(np, "fsl,magic-packet", NULL))
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;
+ /* Setup the initial link state */
+ prop = of_get_property(np, "fixed-link", &prop_sz);
+ if (prop && prop_sz >= sizeof(*prop) * 3) {
+ priv->oldlink = 1;
+ priv->oldduplex = prop[1];
+ priv->oldspeed = prop[2];
+ }
- fixed_link = (u32 *)of_get_property(np, "fixed-link", NULL);
- if (!fixed_link) {
- err = -ENODEV;
- goto err_out;
- }
+ priv->phy_node = of_parse_phandle(np, "phy-handle", 0);
+ if (!priv->phy_node && !priv->oldlink) {
+ err = -ENODEV;
+ goto err_out;
}
/* Find the TBI PHY. If it's not there, we don't support SGMII */
@@ -639,6 +643,48 @@ static phy_interface_t gfar_get_interface(struct net_device *dev)
return PHY_INTERFACE_MODE_MII;
}
+/**
+ * gfar_set_link - program MAC for current MII link speed
+ */
+static void gfar_set_link(struct net_device *dev)
+{
+ struct gfar_private *priv = netdev_priv(dev);
+ struct gfar __iomem *regs = priv->regs;
+ u32 tempval = gfar_read(®s->maccfg2);
+ u32 ecntrl = gfar_read(®s->ecntrl);
+
+ if (priv->oldduplex)
+ tempval |= MACCFG2_FULL_DUPLEX;
+ else
+ tempval &= ~(MACCFG2_FULL_DUPLEX);
+
+ switch (priv->oldspeed) {
+ case 1000:
+ tempval = ((tempval & ~(MACCFG2_IF)) | MACCFG2_GMII);
+ ecntrl &= ~(ECNTRL_R100);
+ break;
+ case 100:
+ case 10:
+ tempval = ((tempval & ~(MACCFG2_IF)) | MACCFG2_MII);
+ /* Reduced mode distinguishes
+ * between 10 and 100 */
+ if (priv->oldspeed == SPEED_100)
+ ecntrl |= ECNTRL_R100;
+ else
+ ecntrl &= ~(ECNTRL_R100);
+ break;
+ default:
+ if (netif_msg_link(priv))
+ dev_err(&dev->dev,
+ "error: speed (%d) is not 10/100/1000!\n",
+ priv->oldspeed);
+ break;
+ }
+
+ gfar_write(®s->maccfg2, tempval);
+ gfar_write(®s->ecntrl, ecntrl);
+}
+
/* Initializes driver's PHY state, and attaches to the PHY.
* Returns 0 on success.
@@ -651,9 +697,10 @@ static int init_phy(struct net_device *dev)
SUPPORTED_1000baseT_Full : 0;
phy_interface_t interface;
- priv->oldlink = 0;
- priv->oldspeed = 0;
- priv->oldduplex = -1;
+ if (priv->oldlink) {
+ netif_carrier_on(dev);
+ gfar_set_link(dev);
+ }
interface = gfar_get_interface(dev);
@@ -664,15 +711,15 @@ static int init_phy(struct net_device *dev)
dev_err(&dev->dev, "error: Could not attach to PHY\n");
return -ENODEV;
}
+
+ /* Remove any features not supported by the controller */
+ priv->phydev->supported &= (GFAR_SUPPORTED | gigabit_support);
+ priv->phydev->advertising = priv->phydev->supported;
}
if (interface == PHY_INTERFACE_MODE_SGMII)
gfar_configure_serdes(dev);
- /* Remove any features not supported by the controller */
- priv->phydev->supported &= (GFAR_SUPPORTED | gigabit_support);
- priv->phydev->advertising = priv->phydev->supported;
-
return 0;
}
@@ -2013,72 +2060,33 @@ static irqreturn_t gfar_interrupt(int irq, void *dev_id)
static void adjust_link(struct net_device *dev)
{
struct gfar_private *priv = netdev_priv(dev);
- struct gfar __iomem *regs = priv->regs;
unsigned long flags;
struct phy_device *phydev = priv->phydev;
int new_state = 0;
spin_lock_irqsave(&priv->txlock, flags);
if (phydev->link) {
- u32 tempval = gfar_read(®s->maccfg2);
- u32 ecntrl = gfar_read(®s->ecntrl);
-
/* Now we make sure that we can be in full duplex mode.
* If not, we operate in half-duplex mode. */
if (phydev->duplex != priv->oldduplex) {
new_state = 1;
- if (!(phydev->duplex))
- tempval &= ~(MACCFG2_FULL_DUPLEX);
- else
- tempval |= MACCFG2_FULL_DUPLEX;
-
priv->oldduplex = phydev->duplex;
}
if (phydev->speed != priv->oldspeed) {
new_state = 1;
- switch (phydev->speed) {
- case 1000:
- tempval =
- ((tempval & ~(MACCFG2_IF)) | MACCFG2_GMII);
-
- ecntrl &= ~(ECNTRL_R100);
- break;
- case 100:
- case 10:
- tempval =
- ((tempval & ~(MACCFG2_IF)) | MACCFG2_MII);
-
- /* Reduced mode distinguishes
- * between 10 and 100 */
- if (phydev->speed == SPEED_100)
- ecntrl |= ECNTRL_R100;
- else
- ecntrl &= ~(ECNTRL_R100);
- break;
- default:
- if (netif_msg_link(priv))
- printk(KERN_WARNING
- "%s: Ack! Speed (%d) is not 10/100/1000!\n",
- dev->name, phydev->speed);
- break;
- }
-
priv->oldspeed = phydev->speed;
}
- gfar_write(®s->maccfg2, tempval);
- gfar_write(®s->ecntrl, ecntrl);
-
if (!priv->oldlink) {
new_state = 1;
priv->oldlink = 1;
}
+
+ gfar_set_link(dev);
} else if (priv->oldlink) {
new_state = 1;
priv->oldlink = 0;
- priv->oldspeed = 0;
- priv->oldduplex = -1;
}
if (new_state && netif_msg_link(priv))
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 61755cb..021ead9 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -314,6 +314,9 @@ int phy_mii_ioctl(struct phy_device *phydev,
{
u16 val = mii_data->val_in;
+ if (!phydev)
+ return -EOPNOTSUPP;
+
switch (cmd) {
case SIOCGMIIPHY:
mii_data->phy_id = phydev->addr;
@@ -385,6 +388,9 @@ int phy_start_aneg(struct phy_device *phydev)
{
int err;
+ if (!phydev)
+ return -ENODEV;
+
mutex_lock(&phydev->lock);
if (AUTONEG_DISABLE == phydev->autoneg)
@@ -703,6 +709,9 @@ phy_err:
*/
void phy_stop(struct phy_device *phydev)
{
+ if (!phydev)
+ return;
+
mutex_lock(&phydev->lock);
if (PHY_HALTED == phydev->state)
@@ -741,6 +750,9 @@ out_unlock:
*/
void phy_start(struct phy_device *phydev)
{
+ if (!phydev)
+ return;
+
mutex_lock(&phydev->lock);
switch (phydev->state) {
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index eba937c..32e5934 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -365,6 +365,9 @@ EXPORT_SYMBOL(phy_connect);
*/
void phy_disconnect(struct phy_device *phydev)
{
+ if (!phydev)
+ return;
+
if (phydev->irq > 0)
phy_stop_interrupts(phydev);
diff --git a/drivers/net/ucc_geth.c b/drivers/net/ucc_geth.c
index 40c6eba..c216cd5 100644
--- a/drivers/net/ucc_geth.c
+++ b/drivers/net/ucc_geth.c
@@ -1443,6 +1443,53 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
return 0;
}
+static void ugeth_set_link(struct net_device *dev)
+{
+ struct ucc_geth_private *ugeth = netdev_priv(dev);
+ struct phy_device *phydev = ugeth->phydev;
+ struct ucc_geth __iomem *ug_regs = ugeth->ug_regs;
+ struct ucc_fast __iomem *uf_regs = ugeth->uccf->uf_regs;
+ u32 tempval = in_be32(&ug_regs->maccfg2);
+ u32 upsmr = in_be32(&uf_regs->upsmr);
+
+ if (ugeth->oldduplex)
+ tempval |= MACCFG2_FDX;
+ else
+ tempval &= ~(MACCFG2_FDX);
+
+ switch (ugeth->oldspeed) {
+ case SPEED_1000:
+ tempval = ((tempval & ~(MACCFG2_INTERFACE_MODE_MASK)) |
+ MACCFG2_INTERFACE_MODE_BYTE);
+ break;
+ case SPEED_100:
+ case SPEED_10:
+ tempval = ((tempval & ~(MACCFG2_INTERFACE_MODE_MASK)) |
+ MACCFG2_INTERFACE_MODE_NIBBLE);
+ /* if reduced mode, re-set UPSMR.R10M */
+ if ((ugeth->phy_interface == PHY_INTERFACE_MODE_RMII) ||
+ (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII) ||
+ (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) ||
+ (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
+ (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) ||
+ (ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
+ if (ugeth->oldspeed == SPEED_10)
+ upsmr |= UCC_GETH_UPSMR_R10M;
+ else
+ upsmr &= ~UCC_GETH_UPSMR_R10M;
+ }
+ break;
+ default:
+ if (netif_msg_link(ugeth))
+ ugeth_warn("%s: Ack! Speed (%d) is not 10/100/1000!",
+ dev->name, phydev->speed);
+ break;
+ }
+
+ out_be32(&ug_regs->maccfg2, tempval);
+ out_be32(&uf_regs->upsmr, upsmr);
+}
+
/* Called every time the controller might need to be made
* aware of new link state. The PHY code conveys this
* information through variables in the ugeth structure, and this
@@ -1453,79 +1500,34 @@ static int adjust_enet_interface(struct ucc_geth_private *ugeth)
static void adjust_link(struct net_device *dev)
{
struct ucc_geth_private *ugeth = netdev_priv(dev);
- struct ucc_geth __iomem *ug_regs;
- struct ucc_fast __iomem *uf_regs;
struct phy_device *phydev = ugeth->phydev;
unsigned long flags;
int new_state = 0;
- ug_regs = ugeth->ug_regs;
- uf_regs = ugeth->uccf->uf_regs;
-
spin_lock_irqsave(&ugeth->lock, flags);
if (phydev->link) {
- u32 tempval = in_be32(&ug_regs->maccfg2);
- u32 upsmr = in_be32(&uf_regs->upsmr);
/* Now we make sure that we can be in full duplex mode.
* If not, we operate in half-duplex mode. */
if (phydev->duplex != ugeth->oldduplex) {
new_state = 1;
- if (!(phydev->duplex))
- tempval &= ~(MACCFG2_FDX);
- else
- tempval |= MACCFG2_FDX;
ugeth->oldduplex = phydev->duplex;
}
if (phydev->speed != ugeth->oldspeed) {
new_state = 1;
- switch (phydev->speed) {
- case SPEED_1000:
- tempval = ((tempval &
- ~(MACCFG2_INTERFACE_MODE_MASK)) |
- MACCFG2_INTERFACE_MODE_BYTE);
- break;
- case SPEED_100:
- case SPEED_10:
- tempval = ((tempval &
- ~(MACCFG2_INTERFACE_MODE_MASK)) |
- MACCFG2_INTERFACE_MODE_NIBBLE);
- /* if reduced mode, re-set UPSMR.R10M */
- if ((ugeth->phy_interface == PHY_INTERFACE_MODE_RMII) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_ID) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_RXID) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RGMII_TXID) ||
- (ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
- if (phydev->speed == SPEED_10)
- upsmr |= UCC_GETH_UPSMR_R10M;
- else
- upsmr &= ~UCC_GETH_UPSMR_R10M;
- }
- break;
- default:
- if (netif_msg_link(ugeth))
- ugeth_warn(
- "%s: Ack! Speed (%d) is not 10/100/1000!",
- dev->name, phydev->speed);
- break;
- }
ugeth->oldspeed = phydev->speed;
}
- out_be32(&ug_regs->maccfg2, tempval);
- out_be32(&uf_regs->upsmr, upsmr);
-
if (!ugeth->oldlink) {
new_state = 1;
ugeth->oldlink = 1;
}
+
+ ugeth_set_link(dev);
} else if (ugeth->oldlink) {
- new_state = 1;
- ugeth->oldlink = 0;
- ugeth->oldspeed = 0;
- ugeth->oldduplex = -1;
+ new_state = 1;
+ ugeth->oldlink = 0;
}
if (new_state && netif_msg_link(ugeth))
@@ -1586,9 +1588,11 @@ static int init_phy(struct net_device *dev)
struct ucc_geth_info *ug_info = priv->ug_info;
struct phy_device *phydev;
- priv->oldlink = 0;
- priv->oldspeed = 0;
- priv->oldduplex = -1;
+ /* If link is marked as up, then set initial link speed */
+ if (priv->oldlink) {
+ netif_carrier_on(dev);
+ ugeth_set_link(dev);
+ }
if (!ug_info->phy_node)
return 0;
@@ -2042,7 +2046,6 @@ static void ucc_geth_set_multi(struct net_device *dev)
static void ucc_geth_stop(struct ucc_geth_private *ugeth)
{
struct ucc_geth __iomem *ug_regs = ugeth->ug_regs;
- struct phy_device *phydev = ugeth->phydev;
ugeth_vdbg("%s: IN", __func__);
@@ -2050,7 +2053,7 @@ static void ucc_geth_stop(struct ucc_geth_private *ugeth)
ugeth_disable(ugeth, COMM_DIR_RX_AND_TX);
/* Tell the kernel the link is down */
- phy_stop(phydev);
+ phy_stop(ugeth->phydev);
/* Mask all interrupts */
out_be32(ugeth->uccf->p_uccm, 0x00000000);
@@ -3608,10 +3611,9 @@ 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 u32 *prop;
+ int prop_sz;
const char *sprop;
const void *mac_addr;
phy_interface_t phy_interface;
@@ -3708,15 +3710,19 @@ 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;
+
+ /* Setup the initial link state */
+ prop = of_get_property(np, "fixed-link", &prop_sz);
+ if (prop && prop_sz >= sizeof(*prop) * 3) {
+ ugeth->oldlink = 1;
+ ugeth->oldduplex = prop[1];
+ ugeth->oldspeed = prop[2];
}
- ug_info->phy_node = phy;
+
+ /* Find the phy. Bail if there is no phy and no initial link speed */
+ ug_info->phy_node = of_parse_phandle(np, "phy-handle", 0);
+ if (!ug_info->phy_node && !ugeth->oldlink)
+ return -ENODEV;
/* 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 +3731,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
* Re: Inline assembly queries [2]
From: Andreas Schwab @ 2009-07-03 21:19 UTC (permalink / raw)
To: kernel mailz; +Cc: linuxppc-dev, linux-kernel, gcc-help
In-Reply-To: <abe8a1fd0907031305r5cb9e2e8pec170bffb1c497ff@mail.gmail.com>
kernel mailz <kernelmailz@googlemail.com> writes:
> My query was more on %U1%X1, I guess it is specifying U and/or X for %1 right ?
> what does U/X stand for (is it similar to u - unsigned and x for a hex address)
> are there any more literals like U/X/...
The 'U' and 'X' modifiers expand to 'u' and 'x' resp, depending on the
form of the referenced memory operand. That allows one to select the
right mnemonic among for example lwz, lwzu, lwzx and lwzux. Those
modifiers must always be used together with the "m" constraint.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: Inline assembly queries [2]
From: Andreas Schwab @ 2009-07-03 20:57 UTC (permalink / raw)
To: Brad Boyer; +Cc: gcc-help, linuxppc-dev, linux-kernel, kernel mailz
In-Reply-To: <20090703174031.GA12410@cynthia.pants.nu>
Brad Boyer <flar@allandria.com> writes:
> On Fri, Jul 03, 2009 at 12:14:41PM +0530, kernel mailz wrote:
>> b. using m or Z with a memory address. I tried replacing m/Z but no change
>> Is there some guideline ?
>> gcc documentation says Z is obsolete. Is m/Z replaceable ?
>
> No idea. I don't remember ever seeing 'Z' used in anything. Maybe somebody
> else remembers what it used to mean.
The 'Z' constraint is required for a memory operand for insns that don't
have an update form (which would be selected by the %U modifier). It
should only be used together with the %y operand modifier, which makes
sure that the first register is never 0. If the 'm' constraint were
used the operand could contain a pre-in/decrement operation, and without
%U the side effect would be lost.
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: Inline assembly queries [2]
From: kernel mailz @ 2009-07-03 20:05 UTC (permalink / raw)
To: Brad Boyer; +Cc: gcc-help, linuxppc-dev, linux-kernel
In-Reply-To: <20090703174031.GA12410@cynthia.pants.nu>
Hi Brad,
Thanks for responding.
My query was more on %U1%X1, I guess it is specifying U and/or X for %1 rig=
ht ?
what does U/X stand for (is it similar to u - unsigned and x for a hex addr=
ess)
are there any more literals like U/X/...
-Manish
On Fri, Jul 3, 2009 at 11:10 PM, Brad Boyer<flar@allandria.com> wrote:
> On Fri, Jul 03, 2009 at 12:14:41PM +0530, kernel mailz wrote:
>> Thanks for responding to my previous mail. A few more queries
>>
>> a. What is the use of adding format specifiers in inline assembly
>> like
>> asm volatile("ld%U1%X1 %0,%1":"=3Dr"(ret) : "m"(*ptr) : "memory");
>
> The format specifiers limit which registers or addressing modes will
> be chosen to access that parameter. For example, if you're using an
> instruction that treats r0 as a literal 0 value, you can't use the
> "r" specifier for that or you may see problems if it happens to allocate
> r0 for that particular argument. For memory access, the "m" lets you
> use any of the normal load/store patterns (which is why this particular
> choice also requires the "%U1%X1" part to allow changing the instruction)=
.
> The system was setup for an older style chip like x86 or 68k with many
> restrictions on which registers can be used where and large numbers of
> different addressing modes for accessing memory. It's a little clumsy
> for ppc by comparison to most other chips, but it's a fundamental part
> of inline assembly processing in gcc.
>
>> b. using m or Z =A0with a memory address. I tried replacing m/Z but no c=
hange
>> Is there some guideline ?
>> gcc documentation says Z is obsolete. Is m/Z replaceable ?
>
> No idea. I don't remember ever seeing 'Z' used in anything. Maybe somebod=
y
> else remembers what it used to mean.
>
> =A0 =A0 =A0 =A0Brad Boyer
> =A0 =A0 =A0 =A0flar@allandria.com
>
>
^ permalink raw reply
* Re: Inline assembly queries [2]
From: Brad Boyer @ 2009-07-03 17:40 UTC (permalink / raw)
To: kernel mailz; +Cc: gcc-help, linuxppc-dev, linux-kernel
In-Reply-To: <abe8a1fd0907022344k2eb9874ejdbda3209a7cadf40@mail.gmail.com>
On Fri, Jul 03, 2009 at 12:14:41PM +0530, kernel mailz wrote:
> Thanks for responding to my previous mail. A few more queries
>
> a. What is the use of adding format specifiers in inline assembly
> like
> asm volatile("ld%U1%X1 %0,%1":"=r"(ret) : "m"(*ptr) : "memory");
The format specifiers limit which registers or addressing modes will
be chosen to access that parameter. For example, if you're using an
instruction that treats r0 as a literal 0 value, you can't use the
"r" specifier for that or you may see problems if it happens to allocate
r0 for that particular argument. For memory access, the "m" lets you
use any of the normal load/store patterns (which is why this particular
choice also requires the "%U1%X1" part to allow changing the instruction).
The system was setup for an older style chip like x86 or 68k with many
restrictions on which registers can be used where and large numbers of
different addressing modes for accessing memory. It's a little clumsy
for ppc by comparison to most other chips, but it's a fundamental part
of inline assembly processing in gcc.
> b. using m or Z with a memory address. I tried replacing m/Z but no change
> Is there some guideline ?
> gcc documentation says Z is obsolete. Is m/Z replaceable ?
No idea. I don't remember ever seeing 'Z' used in anything. Maybe somebody
else remembers what it used to mean.
Brad Boyer
flar@allandria.com
^ permalink raw reply
* Re: mpc52xx_uart.c - Port Overruns
From: Detlev Zundel @ 2009-07-03 11:42 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <c788c1220907021823y384b78c1x2bb9e0fc3268c0e0@mail.gmail.com>
Hi Damien,
> I am writing to ask about some particular behaviour we saw with the MPC5121 PSC
> UART, using the 2.6.24 Freescle BSP kernel, although examining the code of the
> linux-2.6-denx tree (git commit 7cb16ec2590815a67e5fb5c8994ead536613d922), the
> behavior is almost identical except for incrementing an overrrun counter.
>
> In particular, yesterday we observed a port overrun (from the overrun flags
> being set when looking with the BDI) on one of our PSC Ports. When it was
> observed, we saw that every second byte coming from the serial port was 0x00.
>
> Examining the interrupt routine of the mpc52xx_uart.c:
>
> static inline int
> mpc52xx_uart_int_rx_chars(struct uart_port *port)
> {
> <snip>
> tty_insert_flip_char(tty, ch, flag);
> if (status & MPC52xx_PSC_SR_OE) {
> /*
> * Overrun is special, since it's
> * reported immediately, and doesn't
> * affect the current character
> */
> tty_insert_flip_char(tty, 0, TTY_OVERRUN);
> port->icount.overrun++;
> }
> <snip>
> }
>
> So, from my deduction, it is inserting a 0x00 for every overrun error that
> occurs, however, the overrun flag is never cleared. Therefore fro every byte
> that is received, the overrup flag is still set and therefore we're observing
> the 0x00 being inserted for every "real" byte coming into the port
>
> Is there a particular reason why the overrun flag is not cleared? That is,
> parity, framing and breaks are acknowledged with:
>
> /* Clear error condition */
> out_8(&PSC(port)->command, MPC52xx_PSC_RST_ERR_STAT);
>
> But the overrun isn't cleared. Is there are particular reason why? Is
> userspace meant to detect this condition and reset the port? Was it
> automatically cleared on the 5200 when reading the status, but the 5121 is
> exhibiting strange behavior?
This is likely only forgotten in the driver. I remember fixing this in
our 2.4 kernel tree a long time ago[1].
Cheers
Detlev
[1] http://git.denx.de/?p=linuxppc_2_4_devel.git;a=commitdiff;h=00097a16641865b88568b807c9680b50c74bda84
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu@denx.de
^ permalink raw reply
* Re: [PATCH 1/2] ASoC: Fix mpc5200-psc-ac97 to ensure the data ready bit is cleared
From: Mark Brown @ 2009-07-03 9:59 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, alsa-devel
In-Reply-To: <20090702175719.15773.58956.stgit@localhost.localdomain>
On Thu, Jul 02, 2009 at 11:57:19AM -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
>
> When doing register reads, it is possible for there to be a stale
> data ready bit set which will cause subsequent reads to return
> prematurely with incorrect data. This patch fixes the issues by
> ensuring stale data is cleared before starting another transaction.
Applied both, thanks.
^ permalink raw reply
* Re: ppc405ex + gigabit ethernet
From: Lada Podivin @ 2009-07-03 9:09 UTC (permalink / raw)
To: Cote, Sylvain; +Cc: linuxppc-dev@ozlabs.org
In-Reply-To: <579B119545DAEF4689C8FBEEFEC5793F01D4FF01A2D0@ATLMBX.verint.corp.verintsystems.com>
Hi Sylvain,
the interrupt coalescing sounds like good idea - I'm surprised this
feature is missing in the original ibm_newemac driver. You wrote you
had got this optimisation directly from AMCC. Is it part of any
framework? I'm just wondering how one can obtain it. I tried to find
any suitable patch but with no success - the old friend Google didn't
help this time :)
Thank you very much!
Lada
^ permalink raw reply
* Re: ppc405ex + gigabit ethernet
From: Lada Podivin @ 2009-07-03 8:22 UTC (permalink / raw)
To: LiuMing; +Cc: linuxppc-dev
In-Reply-To: <COL107-W5372858231522587839F1CB22F0@phx.gbl>
[-- Attachment #1: Type: text/plain, Size: 64 bytes --]
Many thanks for all responses! Now I know all I need to know :)
[-- Attachment #2: Type: text/html, Size: 71 bytes --]
^ permalink raw reply
* Re: [Patch 2/6] Introduce PPC64 specific Hardware Breakpoint interfaces
From: K.Prasad @ 2009-07-03 8:11 UTC (permalink / raw)
To: David Gibson
Cc: Michael Neuling, Benjamin Herrenschmidt, linuxppc-dev, paulus,
Alan Stern, Roland McGrath
In-Reply-To: <20090619050409.GC17986@yookeroo.seuss>
On Fri, Jun 19, 2009 at 03:04:09PM +1000, David Gibson wrote:
> On Thu, Jun 18, 2009 at 11:50:45PM +0530, K.Prasad wrote:
> > On Wed, Jun 17, 2009 at 02:32:24PM +1000, David Gibson wrote:
> > > On Wed, Jun 10, 2009 at 02:38:06PM +0530, K.Prasad wrote:
> [snip]
With apologies for the long delay here's my response. Some work that was
required to push the generic breakpoint patches into mainline (which is
now pushed to 2.6.32 merge window) and the preparation of a paper kept me
away from responding. Also to add that I will be on a week long vacation
starting next week followed by travel to Linux Symposium and I will not
be able to actively respond to mails till the week beginning 20th of
this month.
But kindly drop in your comments as I would like to have the PPC64
patches ready well before the next merge window opens!
>
> Ah, right, yes. This seems a really non obvious control flow for a
> NULL triggered value to lead to EINVAL. I'd prefer:
>
> if (!bp->triggered)
> return -EINVAL
>
> /* Then the kernel address test */
>
> return arch_store_info(bp, tsk);
>
Ok. It is a minor change and I will do it.
> [snip]
> > > > + /* Verify if dar lies within the address range occupied by the symbol
> > > > + * being watched. Since we cannot get the symbol size for
> > > > + * user-space requests we skip this check in that case
> > > > + */
> > > > + if ((hbp_kernel_pos == 0) &&
> > > > + !((bp->info.address <= dar) &&
> > > > + (dar <= (bp->info.address + bp->info.symbolsize))))
> > > > + /*
> > > > + * This exception is triggered not because of a memory access on
> > > > + * the monitored variable but in the double-word address range
> > > > + * in which it is contained. We will consume this exception,
> > > > + * considering it as 'noise'.
> > > > + */
> > > > + goto out;
> > > > +
> > > > + (bp->triggered)(bp, regs);
> > >
> > > So this confuses me. You go to great efforts to step over the
> > > instruction to generate a SIGTRAP after the instruction, for
> > > consistency with x86. But that SIGTRAP is *never* used, since the
> > > only way to set userspace breakpoints is through ptrace at the moment.
> > > At the same time, the triggered function is called here before the
> > > instruction is executed, so not consistent with x86 anyway.
If the SIGTRAP generation code is in ptrace_triggered() then it would
benefit only ptrace (although it is the lone user of HW-breakpoints at
the moment). Given that SIGTRAP is the only means for the user-space to
detect a breakpoint 'hit' on one of its monitored addresses, we will
generate it for all cases by keeping the code in hw_breakpoint.c (and
not in ptrace_triggered()).
However I believe that your concern here is about the timing of the
SIGTRAP signal generation and I agree with it...please find further
comments about it below.
> > >
> > > It just seems strange to me that sending a SIGTRAP is a special case
> > > anyway. Why can't sending a SIGTRAP be just a particular triggered
> > > function.
> >
> > The consistency in the interface across architectures that I referred to
> > in my previous mail was w.r.t. continuous triggering of breakpoints and
> > not to implement a trigger-before or trigger-after behaviour across
> > architectures. In fact, this behaviour differs even on the same
> > processor depending upon the breakpoint type (trigger-before for
> > instructions and trigger-after for data in x86), and introducing
> > uniformity might be a) at the cost of loss of some unique & innovative
> > uses for each of them b) may not be always possible e.g. trigger-after
> > to trigger-before.
>
> Hrm. Well (a) is why I was suggesting an option to allow trigger
> before on powerpc. Plus, I don't see why you think (a) is important
> for the triggered function, but not for the timing of a SIGTRAP to
> userspace.
>
True that the timing of signal generation is inconsistent for user-space
when requested through ptrace vs requested through the interface. The
requirement for having a continuous breakpoint (as opposed to the
one-shot behaviour of ptrace) doesn't allow the signal to be delivered
in the time-window between the exception and the instruction execution.
I'm contemplating a few ways to work-arounds to ensure consistent signal
delivery timing. One such work-around would be like this:
hbp_handler(1)-->SIGTRAP-->signal_handler-->hbp_handler(2)-->
disable_hbp+enable_ss-->single_step_handler(3)-->enable_hbp+disable_ss
This method would take three exceptions for every breakpoint-hit (as
enumerated above) and will be expensive. Any alternative suggestions
with lower run-time overhead would greatly benefit the patch.
> As for (b), well there are already a bunch of things which can only be
> done on certain archs/processors, so I don't see that's anything
> really new.
Yes, the timing of SIGTRAP generation will remain dependant on the host
processor but the user should be allowed to receive breakpoints
continuously irrespective of the architecture. A one-shot behaviour only
on PPC64 (and only in user-space) doesn't seem nice.
>
> How do you handle continuous breakpoints in the trigger-before case
> (instruction breakpoints) on x86? Do you need to step over an
> instruction is the same manner as you do for powerpc? In this case
> where does x86 send the SIGTRAP?
>
Instructions breakpoints are trigger-before in x86 but the hardware
eases the task by providing RF flag (in EFLAGS register) that would
temporarily disable the instruction breakpoint while the causative
instruction is executed.
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
Thanks,
K.Prasad
^ permalink raw reply
* Re: [PATCH 2/2] ASoC: add locking to mpc5200-psc-ac97 driver
From: Grant Likely @ 2009-07-03 7:12 UTC (permalink / raw)
To: michael; +Cc: linuxppc-dev, alsa-devel, broonie
In-Reply-To: <4A4D05AB.1090306@evidence.eu.com>
On Thu, Jul 2, 2009 at 1:08 PM, michael<michael@evidence.eu.com> wrote:
> Hi,
>
> Grant Likely wrote:
>>
>> From: Grant Likely <grant.likely@secretlab.ca>
>>
>> AC97 bus register read/write hooks need to provide locking, but the
>> mpc5200-psc-ac97 driver does not. =A0This patch adds a mutex around
>> the register access routines.
>>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> ---
>>
>> =A0sound/soc/fsl/mpc5200_dma.c =A0 =A0 =A0| =A0 =A01 +
>> =A0sound/soc/fsl/mpc5200_dma.h =A0 =A0 =A0| =A0 =A01 +
>> =A0sound/soc/fsl/mpc5200_psc_ac97.c | =A0 13 ++++++++++++-
>> =A03 files changed, 14 insertions(+), 1 deletions(-)
>>
>>
>> diff --git a/sound/soc/fsl/mpc5200_dma.c b/sound/soc/fsl/mpc5200_dma.c
>> index efec33a..f0a2d40 100644
>> --- a/sound/soc/fsl/mpc5200_dma.c
>> +++ b/sound/soc/fsl/mpc5200_dma.c
>> @@ -456,6 +456,7 @@ int mpc5200_audio_dma_create(struct of_device *op)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -ENODEV;
>> =A0 =A0 =A0 =A0 =A0spin_lock_init(&psc_dma->lock);
>> + =A0 =A0 =A0 mutex_init(&psc_dma->mutex);
>> =A0 =A0 =A0 =A0psc_dma->id =3D be32_to_cpu(*prop);
>> =A0 =A0 =A0 =A0psc_dma->irq =3D irq;
>> =A0 =A0 =A0 =A0psc_dma->psc_regs =3D regs;
>> diff --git a/sound/soc/fsl/mpc5200_dma.h b/sound/soc/fsl/mpc5200_dma.h
>> index 2000803..8d396bb 100644
>> --- a/sound/soc/fsl/mpc5200_dma.h
>> +++ b/sound/soc/fsl/mpc5200_dma.h
>> @@ -55,6 +55,7 @@ struct psc_dma {
>> =A0 =A0 =A0 =A0unsigned int irq;
>> =A0 =A0 =A0 =A0struct device *dev;
>> =A0 =A0 =A0 =A0spinlock_t lock;
>> + =A0 =A0 =A0 struct mutex mutex;
>> =A0 =A0 =A0 =A0u32 sicr;
>> =A0 =A0 =A0 =A0uint sysclk;
>> =A0 =A0 =A0 =A0int imr;
>> diff --git a/sound/soc/fsl/mpc5200_psc_ac97.c
>> b/sound/soc/fsl/mpc5200_psc_ac97.c
>> index 9b8503f..7eb5499 100644
>> --- a/sound/soc/fsl/mpc5200_psc_ac97.c
>> +++ b/sound/soc/fsl/mpc5200_psc_ac97.c
>> @@ -34,11 +34,14 @@ static unsigned short psc_ac97_read(struct snd_ac97
>> *ac97, unsigned short reg)
>> =A0 =A0 =A0 =A0int status;
>> =A0 =A0 =A0 =A0unsigned int val;
>> =A0+ =A0 =A0 =A0 mutex_lock(&psc_dma->mutex);
>> +
>> =A0 =A0 =A0 =A0/* Wait for command send status zero =3D ready */
>> =A0 =A0 =A0 =A0status =3D
>> spin_event_timeout(!(in_be16(&psc_dma->psc_regs->sr_csr.status) &
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0MPC52xx_P=
SC_SR_CMDSEND), 100, 0);
>> =A0 =A0 =A0 =A0if (status =3D=3D 0) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0pr_err("timeout on ac97 bus (rdy)\n");
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mutex_unlock(&psc_dma->mutex);
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return -ENODEV;
>>
>
> maybe define an err variable and and a goto out.
My first iteration did that, but after looking at it I decided I like
this better and it adds (slightly) fewer lines of code.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [spi-devel-general] [PATCH v4] powerpc/5200: Add mpc5200-spi (non-PSC) device driver
From: Grant Likely @ 2009-07-03 7:01 UTC (permalink / raw)
To: Wolfram Sang
Cc: linuxppc-dev, David Brownell, linux-kernel, spi-devel-general
In-Reply-To: <20090618142610.GC10629@pengutronix.de>
On Thu, Jun 18, 2009 at 8:26 AM, Wolfram Sang<w.sang@pengutronix.de> wrote:
>> There used to be a sysfs interface for dumping these, but it was an
>> ugly misuse. =A0I'd like to leave these in. =A0I still have the sysfs bi=
ts
>> in a private patch and I'm going to rework them for debugfs.
>
> Okay. Maybe a comment stating the future use will be nice.
okay
>> > But I wonder more about the usage of the SS pin and if this chipsel is=
needed
>> > at all (sadly I cannot test as I don't have any board with SPI connect=
ed to
>> > that device). You define the SS-pin as output, but do not set the SSOE=
-bit.
>> > More, you use the MODF-feature, so the SS-pin should be defined as inp=
ut?
>> > According to Table 17.3 in the PM, you have that pin defined as generi=
c purpose
>> > output.
>>
>> That's right. =A0The SS handling by the SPI device is completely
>> useless, so this driver uses it as a GPIO and asserts it manually.
>
> That definately needs a comment :D (perhaps with some more details if you=
know them).
>
>> The MODF irq is probably irrelevant, but I'd like to leave it in for
>> completeness.
>
> But it won't work if the pin is set to output, no?
yes
> Are you sure there are no side-effects?
I'm sure.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Inline assembly queries [2]
From: kernel mailz @ 2009-07-03 6:44 UTC (permalink / raw)
To: linuxppc-dev, gcc-help, linux-kernel
Hi,
Thanks for responding to my previous mail. A few more queries
a. What is the use of adding format specifiers in inline assembly
like
asm volatile("ld%U1%X1 %0,%1":"=r"(ret) : "m"(*ptr) : "memory");
b. using m or Z with a memory address. I tried replacing m/Z but no change
Is there some guideline ?
gcc documentation says Z is obsolete. Is m/Z replaceable ?
- Manish
^ permalink raw reply
* Re: [spi-devel-general] [PATCH -mm v4][POWERPC] mpc8xxx : allow SPI without cs.
From: David Brownell @ 2009-07-03 1:38 UTC (permalink / raw)
To: spi-devel-general, Rini van Zetten; +Cc: linuxppc-dev list
In-Reply-To: <4A3B9E38.9060004@arvoo.nl>
On Friday 19 June 2009, Rini van Zetten wrote:
> This patch adds the possibility to have a spi device without a cs.
Note that there's now the SPI_NO_CS bit in spi_device.mode
to describe this situation ... so no "-EEXIST" hackery should
ever tempt anyone again.
^ permalink raw reply
* RE: [PATCH 1/2] KVM/PPC: Fix PPC KVM e500_tlb.c build error
From: Liu Yu-B13201 @ 2009-07-03 1:35 UTC (permalink / raw)
To: Avi Kivity; +Cc: linuxppc-dev, Yang Shi, kvm-ppc, hollisb, kvm
In-Reply-To: <4A4C97C4.9090908@redhat.com>
=20
> -----Original Message-----
> From: Avi Kivity [mailto:avi@redhat.com]=20
> Sent: Thursday, July 02, 2009 7:20 PM
> To: Liu Yu-B13201
> Cc: Yang Shi; hollisb@us.ibm.com; kvm-ppc@vger.kernel.org;=20
> kvm@vger.kernel.org; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 1/2] KVM/PPC: Fix PPC KVM e500_tlb.c build error
>=20
> On 07/02/2009 06:09 AM, Liu Yu-B13201 wrote:
> > This fix is already accepted in kvm.git
> > =20
>=20
> What about the other? Is it needed?
>=20
No, it's already fixed by commit =
b57227e600f4ecc394d6ba3c2aaa558867b5addc.
^ permalink raw reply
* mpc52xx_uart.c - Port Overruns
From: Damien Dusha @ 2009-07-03 1:23 UTC (permalink / raw)
To: Linuxppc-dev
[-- Attachment #1: Type: text/plain, Size: 1865 bytes --]
Dear List,
I am writing to ask about some particular behaviour we saw with the MPC5121
PSC UART, using the 2.6.24 Freescle BSP kernel, although examining the code
of the linux-2.6-denx tree (git commit
7cb16ec2590815a67e5fb5c8994ead536613d922), the behavior is almost identical
except for incrementing an overrrun counter.
In particular, yesterday we observed a port overrun (from the overrun flags
being set when looking with the BDI) on one of our PSC Ports. When it was
observed, we saw that every second byte coming from the serial port was
0x00.
Examining the interrupt routine of the mpc52xx_uart.c:
static inline int
mpc52xx_uart_int_rx_chars(struct uart_port *port)
{
<snip>
tty_insert_flip_char(tty, ch, flag);
if (status & MPC52xx_PSC_SR_OE) {
/*
* Overrun is special, since it's
* reported immediately, and doesn't
* affect the current character
*/
tty_insert_flip_char(tty, 0, TTY_OVERRUN);
port->icount.overrun++;
}
<snip>
}
So, from my deduction, it is inserting a 0x00 for every overrun error that
occurs, however, the overrun flag is never cleared. Therefore fro every
byte that is received, the overrup flag is still set and therefore we're
observing the 0x00 being inserted for every "real" byte coming into the port
Is there a particular reason why the overrun flag is not cleared? That is,
parity, framing and breaks are acknowledged with:
/* Clear error condition */
out_8(&PSC(port)->command, MPC52xx_PSC_RST_ERR_STAT);
But the overrun isn't cleared. Is there are particular reason why? Is
userspace meant to detect this condition and reset the port? Was it
automatically cleared on the 5200 when reading the status, but the 5121 is
exhibiting strange behavior?
Best regards,
Damien Dusha.
[-- Attachment #2: Type: text/html, Size: 2075 bytes --]
^ permalink raw reply
* Re: Subject: [PATCH v8] spi: Add PPC4xx SPI driver
From: David Brownell @ 2009-07-03 0:44 UTC (permalink / raw)
To: Steven A. Falco; +Cc: linuxppc-dev@ozlabs.org, Stefan Roese
In-Reply-To: <4A44F643.3050505@harris.com>
On Friday 26 June 2009, Steven A. Falco wrote:
> +
> + /*
> + * If there are no chip selects at all, or if this is the special
> + * case of a non-existent (dummy) chip select, do nothing.
> + */
> +
> + if (!hw->master->num_chipselect || hw->gpios[cs] == -EEXIST)
> + return;
> +
I'm going to send this in, but please send a followup
patch making all this "non-existent (dummy) chip select"
stuff use the SPI_NO_CS flag.
> + /*
> + * A count of zero implies a single SPI device without any chip-select.
> + * Note that of_gpio_count counts all gpios assigned to this spi master.
> + * This includes both "null" gpio's and real ones.
> + */
^ permalink raw reply
* Re: Preemption question (4xx related)
From: Josh Boyer @ 2009-07-03 0:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Felix Radensky
In-Reply-To: <1246574460.7551.6.camel@pasglop>
On Fri, Jul 03, 2009 at 08:41:00AM +1000, Benjamin Herrenschmidt wrote:
>On Thu, 2009-07-02 at 07:12 -0400, Josh Boyer wrote:
>> On Thu, Jul 02, 2009 at 05:33:12PM +1000, Benjamin Herrenschmidt wrote:
>> >On Wed, 2009-07-01 at 20:14 -0400, Josh Boyer wrote:
>> >
>> >> I've toyed with that idea myself. I keep coming back to the fact that you need
>> >> a workload that would really leverage it, and I don't have one at the moment.
>> >
>> >To some extent that's true but just turning full preemption including
>> >kernel side with all the associated debug bits and lockdep should make a
>> >whole bunch of things show up even with ordinary workloads.
>>
>> I can look at doing that for ppc44x_defconfig. I'll be honest and say I don't
>> expect it to go well, particularly with lockdep :).
>>
>> >For 440 tend to boot an ubuntu distro off NFS root with all X & DRI 3D
>> >etc... and then run compiz :-)
>>
>> Yes. Because that's a totally realistic workload for a 440. I'm surprised you
>> don't have a p595 machine acting as your home router too! ;)
>
>It doesn't need to be realistic. In fact, a "realistic" workload is the
>worst thing to test with because it won't exercise all the "uncommon"
>code path which are the ones likely to bite.
>
>So yesm it's not a "realistic" workload, but it's a good "torture"
>workload to find bugs.
It was a joke. But yes, you make perfectly valid points :)
josh
^ permalink raw reply
* Re: Preemption question (4xx related)
From: Benjamin Herrenschmidt @ 2009-07-02 22:41 UTC (permalink / raw)
To: Josh Boyer; +Cc: linuxppc-dev, Felix Radensky
In-Reply-To: <20090702111246.GG6189@zod.rchland.ibm.com>
On Thu, 2009-07-02 at 07:12 -0400, Josh Boyer wrote:
> On Thu, Jul 02, 2009 at 05:33:12PM +1000, Benjamin Herrenschmidt wrote:
> >On Wed, 2009-07-01 at 20:14 -0400, Josh Boyer wrote:
> >
> >> I've toyed with that idea myself. I keep coming back to the fact that you need
> >> a workload that would really leverage it, and I don't have one at the moment.
> >
> >To some extent that's true but just turning full preemption including
> >kernel side with all the associated debug bits and lockdep should make a
> >whole bunch of things show up even with ordinary workloads.
>
> I can look at doing that for ppc44x_defconfig. I'll be honest and say I don't
> expect it to go well, particularly with lockdep :).
>
> >For 440 tend to boot an ubuntu distro off NFS root with all X & DRI 3D
> >etc... and then run compiz :-)
>
> Yes. Because that's a totally realistic workload for a 440. I'm surprised you
> don't have a p595 machine acting as your home router too! ;)
It doesn't need to be realistic. In fact, a "realistic" workload is the
worst thing to test with because it won't exercise all the "uncommon"
code path which are the ones likely to bite.
So yesm it's not a "realistic" workload, but it's a good "torture"
workload to find bugs.
Cheers,
Ben.
^ permalink raw reply
* Re: RAMDISK on EP88xc
From: Mikhail Zaturenskiy @ 2009-07-02 21:57 UTC (permalink / raw)
To: Scott Wood; +Cc: Frank Svendsbøe, linuxppc-dev, Gary Thomas
In-Reply-To: <4A4D2B70.1030505@freescale.com>
>> Frank! That worked like a charm! I downloaded linux-2.6.26 source from
>> git.kernel.org, inserted the config options previously mentioned,
>> compiled ok, keeping U-Boot ramdisk and my DTS same it booted right up
>> and no more "slow motion" issue.
>
> If you have time, could you bisect to see when the slowdown was introduced?
I'm leaving in 5 minutes, but I'll try to find some time next week to
check this out.
Mikhail
^ permalink raw reply
* Re: Non-contiguous physical memory on 8572
From: Aaron Pace @ 2009-07-02 21:56 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <4A4D2332.1060607@freescale.com>
On Thu, Jul 2, 2009 at 3:14 PM, Scott Wood<scottwood@freescale.com> wrote:
> Aaron Pace wrote:
>>
>> In MMU_init of arch/powerpc/mm/init_32.c, where the current code sets
>> lmb.memory.cnt to zero, I instead walk through the memory regions and
>> call lmb_reserve for each chunk of memory that lies in a 'hole'.
>> There are then some minor fixups to make sure that total_memory and
>> total_highmem get the right numbers. =A0This small change allows all
>> four gigabytes of memory to be accessed and used in my tests.
>>
>> Am I missing something obvious?
>
> The main downsides that I see are wasted memory for bookkeeping of the ho=
le
> (how acceptable this is depends on how large the hole is relative to the
> size of RAM -- it's a tradeoff against speed of looking up page structs),
> and that the reserved area may still be mapped in the TLB without the
> guarded bit set.
>
> -Scott
>
>
Ah, thanks for the response.
A couple of followup clarifications/questions, if you don't mind.
As far as wasted memory for bookkeeping, aren't the reserved regions
excluded from any zonelist/pagetable allocation? I'm looking through
to verify, but if you know off the top of your head where any extra
data would be required to keep track, I'd like to take a look to
further educate my memory manager understanding.
Secondly, can you elaborate on how/when the reserved area could be
mapped into the TLB? I don't by any means lay claim to a complete
understanding of this area, but aside from a direct ioremap/mmap call,
how would this area get mapped at all?
Thanks again,
Aaron
^ permalink raw reply
* Re: RAMDISK on EP88xc
From: Scott Wood @ 2009-07-02 21:49 UTC (permalink / raw)
To: Mikhail Zaturenskiy; +Cc: Frank Svendsbøe, linuxppc-dev, Gary Thomas
In-Reply-To: <97dd5fd20907021438ve8c72b9g168e4aa22e9f2f9f@mail.gmail.com>
Mikhail Zaturenskiy wrote:
>> This is very interesting. I recently experienced a similar problem when
>> upgrading from 2.6.26-rc6 to 2.6.31-rc1 (torvalds mainline kernel).
>>
>> I didn't had the time to figure out what was happening, so I went back to
>> 2.6.26-rc6. Just for the test, could you try to go back to 2.6.26 and see if
>> the problem remains?
>
> Frank! That worked like a charm! I downloaded linux-2.6.26 source from
> git.kernel.org, inserted the config options previously mentioned,
> compiled ok, keeping U-Boot ramdisk and my DTS same it booted right up
> and no more "slow motion" issue.
If you have time, could you bisect to see when the slowdown was introduced?
-Scott
^ permalink raw reply
* Re: RAMDISK on EP88xc
From: Mikhail Zaturenskiy @ 2009-07-02 21:38 UTC (permalink / raw)
To: Frank Svendsbøe; +Cc: linuxppc-dev, Gary Thomas
In-Reply-To: <1ba63b520907021338n3c0d6d33g11d4934ccc538a94@mail.gmail.com>
> This is very interesting. I recently experienced a similar problem when
> upgrading from 2.6.26-rc6 to 2.6.31-rc1 (torvalds mainline kernel).
>
> I didn't had the time to figure out what was happening, so I went back to
> 2.6.26-rc6. Just for the test, could you try to go back to 2.6.26 and see if
> the problem remains?
Frank! That worked like a charm! I downloaded linux-2.6.26 source from
git.kernel.org, inserted the config options previously mentioned,
compiled ok, keeping U-Boot ramdisk and my DTS same it booted right up
and no more "slow motion" issue.
Thanks! Now I just need to make myself a custom ramdisk.
^ permalink raw reply
* Re: Non-contiguous physical memory on 8572
From: Scott Wood @ 2009-07-02 21:14 UTC (permalink / raw)
To: Aaron Pace; +Cc: linuxppc-dev
In-Reply-To: <bc81dc640907021311y1395ed15l2d50cebb3749c29f@mail.gmail.com>
Aaron Pace wrote:
> In MMU_init of arch/powerpc/mm/init_32.c, where the current code sets
> lmb.memory.cnt to zero, I instead walk through the memory regions and
> call lmb_reserve for each chunk of memory that lies in a 'hole'.
> There are then some minor fixups to make sure that total_memory and
> total_highmem get the right numbers. This small change allows all
> four gigabytes of memory to be accessed and used in my tests.
>
> Am I missing something obvious?
The main downsides that I see are wasted memory for bookkeeping of the
hole (how acceptable this is depends on how large the hole is relative
to the size of RAM -- it's a tradeoff against speed of looking up page
structs), and that the reserved area may still be mapped in the TLB
without the guarded bit set.
-Scott
^ permalink raw reply
* Re: Non-contiguous physical memory on 8572
From: Kumar Gala @ 2009-07-02 21:05 UTC (permalink / raw)
To: Aaron Pace; +Cc: linuxppc-dev
In-Reply-To: <bc81dc640907021311y1395ed15l2d50cebb3749c29f@mail.gmail.com>
On Jul 2, 2009, at 3:11 PM, Aaron Pace wrote:
> Hello,
>
> I wrote to this list quite some time ago concerning a board that has 2
> gigs of ram mapped in at 0x0.0000.0000 - 0x0.7fff.ffff, and a second 2
> gigs of ram at 0x1.0000.0000 - 0x1.7fff.ffff. Kumar responded and
> mentioned that this wasn't currently supported in the powerpc
> architecture. I've had this on my 'curiosity' back burner to take a
> look at for some time.
> I found a little time to look at this, and what I discovered is that
> this seems to actually have been a fairly trivial problem to solve.
> I'd like to get some feedback on the method I used, if possible, to
> see if I've overlooked something blatantly obvious.
>
> In MMU_init of arch/powerpc/mm/init_32.c, where the current code sets
> lmb.memory.cnt to zero, I instead walk through the memory regions and
> call lmb_reserve for each chunk of memory that lies in a 'hole'.
> There are then some minor fixups to make sure that total_memory and
> total_highmem get the right numbers. This small change allows all
> four gigabytes of memory to be accessed and used in my tests.
>
> Am I missing something obvious?
> Would you like this in a patch for 32 or next, or is there a reason
> that this would not be desirable in the powerpc branch?
What you described is one possible solution. However when I was
thinking about support non-contiguous memory I was thinking about
doing at based on CONFIG_DISCONTIGMEM (see include/asm-generic/
memory_model.h). I think ppc64 supports SPARSEMEM which is a
variation on the them.
- k
^ permalink raw reply
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