LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* RE: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: John Linn @ 2010-03-15 13:40 UTC (permalink / raw)
  To: michal.simek; +Cc: netdev, John Tyner, linuxppc-dev, john.williams
In-Reply-To: <4B9DF254.2030402@petalogix.com>

> -----Original Message-----
> From: Michal Simek [mailto:michal.simek@petalogix.com]
> Sent: Monday, March 15, 2010 2:40 AM
> To: John Linn
> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
grant.likely@secretlab.ca;
> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
> =

> John Linn wrote:
> > This patch adds support for using the LL TEMAC Ethernet driver on
> > non-Virtex 5 platforms by adding support for accessing the Soft DMA
> > registers as if they were memory mapped instead of solely through
the
> > DCR's (available on the Virtex 5).
> >
> > The patch also updates the driver so that it runs on the MicroBlaze.
> > The changes were tested on the PowerPC 440, PowerPC 405, and the
> > MicroBlaze platforms.
> =

> Which git-tree have you tested on? (Of course microblaze)

It was tested on the Xilinx tree for MicroBlaze which is based on the
mainline and the Petalogix tree as DMA was needed. I tried to build
against the mainline head but got errors with the DMA routines. I guess
it's possible that it was a configuration issue there as I didn't dig
real deep.

I tested the PowerPC against the head of mainline.

Thanks,
John

> =

> Michal
> =

> >
> > Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
> > Signed-off-by: John Linn <john.linn@xilinx.com>
> > ---
> >
> > V2 - Incorporated comments from Grant and added more logic to allow
the driver
> > to work on MicroBlaze.
> >
> >  drivers/net/Kconfig         |    1 -
> >  drivers/net/ll_temac.h      |   17 +++++-
> >  drivers/net/ll_temac_main.c |  124
++++++++++++++++++++++++++++++++++---------
> >  3 files changed, 113 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > index 9b6efe1..5402105 100644
> > --- a/drivers/net/Kconfig
> > +++ b/drivers/net/Kconfig
> > @@ -2443,7 +2443,6 @@ config MV643XX_ETH
> >  config XILINX_LL_TEMAC
> >  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC)
driver"
> >  	select PHYLIB
> > -	depends on PPC_DCR_NATIVE
> >  	help
> >  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
> >  	  core used in Xilinx Spartan and Virtex FPGAs
> > diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
> > index 1af66a1..915aa34 100644
> > --- a/drivers/net/ll_temac.h
> > +++ b/drivers/net/ll_temac.h
> > @@ -5,8 +5,11 @@
> >  #include <linux/netdevice.h>
> >  #include <linux/of.h>
> >  #include <linux/spinlock.h>
> > +
> > +#ifdef CONFIG_PPC_DCR
> >  #include <asm/dcr.h>
> >  #include <asm/dcr-regs.h>
> > +#endif
> >
> >  /* packet size info */
> >  #define XTE_HDR_SIZE			14      /* size of
Ethernet header */
> > @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
> >
> >  #define TX_CONTROL_CALC_CSUM_MASK   1
> >
> > +/* Align the IP data in the packet on word boundaries as MicroBlaze
> > + * needs it.
> > + */
> > +
> >  #define XTE_ALIGN       32
> > -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
> > +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)
> >
> >  #define MULTICAST_CAM_TABLE_NUM 4
> >
> > @@ -335,9 +342,15 @@ struct temac_local {
> >  	struct mii_bus *mii_bus;	/* MII bus reference */
> >  	int mdio_irqs[PHY_MAX_ADDR];	/* IRQs table for MDIO bus */
> >
> > -	/* IO registers and IRQs */
> > +	/* IO registers, dma functions and IRQs */
> >  	void __iomem *regs;
> > +	void __iomem *sdma_regs;
> > +#ifdef CONFIG_PPC_DCR
> >  	dcr_host_t sdma_dcrs;
> > +#endif
> > +	u32 (*dma_in)(struct temac_local *, int);
> > +	void (*dma_out)(struct temac_local *, int, u32);
> > +
> >  	int tx_irq;
> >  	int rx_irq;
> >  	int emac_num;
> > diff --git a/drivers/net/ll_temac_main.c
b/drivers/net/ll_temac_main.c
> > index a18e348..9aedf9b 100644
> > --- a/drivers/net/ll_temac_main.c
> > +++ b/drivers/net/ll_temac_main.c
> > @@ -20,9 +20,6 @@
> >   *   or rx, so this should be okay.
> >   *
> >   * TODO:
> > - * - Fix driver to work on more than just Virtex5.  Right now the
driver
> > - *   assumes that the locallink DMA registers are accessed via DCR
> > - *   instructions.
> >   * - Factor out locallink DMA code into separate driver
> >   * - Fix multicast assignment.
> >   * - Fix support for hardware checksumming.
> > @@ -115,17 +112,86 @@ void temac_indirect_out32(struct temac_local
*lp, int reg, u32 value)
> >  	temac_iow(lp, XTE_CTL0_OFFSET, CNTLREG_WRITE_ENABLE_MASK | reg);
> >  }
> >
> > +/**
> > + * temac_dma_in32 - Memory mapped DMA read, this function expects a
> > + * register input that is based on DCR word addresses which
> > + * are then converted to memory mapped byte addresses
> > + */
> >  static u32 temac_dma_in32(struct temac_local *lp, int reg)
> >  {
> > -	return dcr_read(lp->sdma_dcrs, reg);
> > +	return in_be32((u32 *)(lp->sdma_regs + (reg << 2)));
> >  }
> >
> > +/**
> > + * temac_dma_out32 - Memory mapped DMA read, this function expects
a
> > + * register input that is based on DCR word addresses which
> > + * are then converted to memory mapped byte addresses
> > + */
> >  static void temac_dma_out32(struct temac_local *lp, int reg, u32
value)
> >  {
> > +	out_be32((u32 *)(lp->sdma_regs + (reg << 2)), value);
> > +}
> > +
> > +/* DMA register access functions can be DCR based or memory mapped.
> > + * The PowerPC 440 is DCR based, the PowerPC 405 and MicroBlaze are
both
> > + * memory mapped.
> > + */
> > +#ifdef CONFIG_PPC_DCR
> > +
> > +/**
> > + * temac_dma_dcr_in32 - DCR based DMA read
> > + */
> > +static u32 temac_dma_dcr_in(struct temac_local *lp, int reg)
> > +{
> > +	return dcr_read(lp->sdma_dcrs, reg);
> > +}
> > +
> > +/**
> > + * temac_dma_dcr_out32 - DCR based DMA write
> > + */
> > +static void temac_dma_dcr_out(struct temac_local *lp, int reg, u32
value)
> > +{
> >  	dcr_write(lp->sdma_dcrs, reg, value);
> >  }
> >
> >  /**
> > + * temac_dcr_setup - If the DMA is DCR based, then setup the
address and
> > + * I/O  functions
> > + */
> > +static int temac_dcr_setup(struct temac_local *lp, struct of_device
*op,
> > +				struct device_node *np)
> > +{
> > +	unsigned int dcrs;
> > +
> > +	/* setup the dcr address mapping if it's in the device tree */
> > +
> > +	dcrs =3D dcr_resource_start(np, 0);
> > +	if (dcrs !=3D 0) {
> > +		lp->sdma_dcrs =3D dcr_map(np, dcrs, dcr_resource_len(np,
0));
> > +		lp->dma_in =3D temac_dma_dcr_in;
> > +		lp->dma_out =3D temac_dma_dcr_out;
> > +		dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
> > +		return 0;
> > +	}
> > +	/* no DCR in the device tree, indicate a failure */
> > +	return -1;
> > +}
> > +
> > +#else
> > +
> > +/*
> > + * temac_dcr_setup - This is a stub for when DCR is not supported,
> > + * such as with MicroBlaze
> > + */
> > +static int temac_dcr_setup(struct temac_local *lp, struct of_device
*op,
> > +				struct device_node *np)
> > +{
> > +	return -1;
> > +}
> > +
> > +#endif
> > +
> > +/**
> >   * temac_dma_bd_init - Setup buffer descriptor rings
> >   */
> >  static int temac_dma_bd_init(struct net_device *ndev)
> > @@ -172,23 +238,23 @@ static int temac_dma_bd_init(struct net_device
*ndev)
> >  		lp->rx_bd_v[i].app0 =3D STS_CTRL_APP0_IRQONEND;
> >  	}
> >
> > -	temac_dma_out32(lp, TX_CHNL_CTRL, 0x10220400 |
> > +	lp->dma_out(lp, TX_CHNL_CTRL, 0x10220400 |
> >  					  CHNL_CTRL_IRQ_EN |
> >  					  CHNL_CTRL_IRQ_DLY_EN |
> >  					  CHNL_CTRL_IRQ_COAL_EN);
> >  	/* 0x10220483 */
> >  	/* 0x00100483 */
> > -	temac_dma_out32(lp, RX_CHNL_CTRL, 0xff010000 |
> > +	lp->dma_out(lp, RX_CHNL_CTRL, 0xff010000 |
> >  					  CHNL_CTRL_IRQ_EN |
> >  					  CHNL_CTRL_IRQ_DLY_EN |
> >  					  CHNL_CTRL_IRQ_COAL_EN |
> >  					  CHNL_CTRL_IRQ_IOE);
> >  	/* 0xff010283 */
> >
> > -	temac_dma_out32(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
> > -	temac_dma_out32(lp, RX_TAILDESC_PTR,
> > +	lp->dma_out(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
> > +	lp->dma_out(lp, RX_TAILDESC_PTR,
> >  		       lp->rx_bd_p + (sizeof(*lp->rx_bd_v) * (RX_BD_NUM
- 1)));
> > -	temac_dma_out32(lp, TX_CURDESC_PTR, lp->tx_bd_p);
> > +	lp->dma_out(lp, TX_CURDESC_PTR, lp->tx_bd_p);
> >
> >  	return 0;
> >  }
> > @@ -426,9 +492,9 @@ static void temac_device_reset(struct net_device
*ndev)
> >  	temac_indirect_out32(lp, XTE_RXC1_OFFSET, val &
~XTE_RXC1_RXEN_MASK);
> >
> >  	/* Reset Local Link (DMA) */
> > -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
> > +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
> >  	timeout =3D 1000;
> > -	while (temac_dma_in32(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
> > +	while (lp->dma_in(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
> >  		udelay(1);
> >  		if (--timeout =3D=3D 0) {
> >  			dev_err(&ndev->dev,
> > @@ -436,7 +502,7 @@ static void temac_device_reset(struct net_device
*ndev)
> >  			break;
> >  		}
> >  	}
> > -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
> > +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
> >
> >  	temac_dma_bd_init(ndev);
> >
> > @@ -597,7 +663,7 @@ static int temac_start_xmit(struct sk_buff *skb,
struct net_device *ndev)
> >  		lp->tx_bd_tail =3D 0;
> >
> >  	/* Kick off the transfer */
> > -	temac_dma_out32(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
> > +	lp->dma_out(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
> >
> >  	return NETDEV_TX_OK;
> >  }
> > @@ -663,7 +729,7 @@ static void ll_temac_recv(struct net_device
*ndev)
> >  		cur_p =3D &lp->rx_bd_v[lp->rx_bd_ci];
> >  		bdstat =3D cur_p->app0;
> >  	}
> > -	temac_dma_out32(lp, RX_TAILDESC_PTR, tail_p);
> > +	lp->dma_out(lp, RX_TAILDESC_PTR, tail_p);
> >
> >  	spin_unlock_irqrestore(&lp->rx_lock, flags);
> >  }
> > @@ -674,8 +740,8 @@ static irqreturn_t ll_temac_tx_irq(int irq, void
*_ndev)
> >  	struct temac_local *lp =3D netdev_priv(ndev);
> >  	unsigned int status;
> >
> > -	status =3D temac_dma_in32(lp, TX_IRQ_REG);
> > -	temac_dma_out32(lp, TX_IRQ_REG, status);
> > +	status =3D lp->dma_in(lp, TX_IRQ_REG);
> > +	lp->dma_out(lp, TX_IRQ_REG, status);
> >
> >  	if (status & (IRQ_COAL | IRQ_DLY))
> >  		temac_start_xmit_done(lp->ndev);
> > @@ -692,8 +758,8 @@ static irqreturn_t ll_temac_rx_irq(int irq, void
*_ndev)
> >  	unsigned int status;
> >
> >  	/* Read and clear the status registers */
> > -	status =3D temac_dma_in32(lp, RX_IRQ_REG);
> > -	temac_dma_out32(lp, RX_IRQ_REG, status);
> > +	status =3D lp->dma_in(lp, RX_IRQ_REG);
> > +	lp->dma_out(lp, RX_IRQ_REG, status);
> >
> >  	if (status & (IRQ_COAL | IRQ_DLY))
> >  		ll_temac_recv(lp->ndev);
> > @@ -794,7 +860,7 @@ static ssize_t temac_show_llink_regs(struct
device *dev,
> >  	int i, len =3D 0;
> >
> >  	for (i =3D 0; i < 0x11; i++)
> > -		len +=3D sprintf(buf + len, "%.8x%s", temac_dma_in32(lp,
i),
> > +		len +=3D sprintf(buf + len, "%.8x%s", lp->dma_in(lp, i),
> >  			       (i % 8) =3D=3D 7 ? "\n" : " ");
> >  	len +=3D sprintf(buf + len, "\n");
> >
> > @@ -820,7 +886,6 @@ temac_of_probe(struct of_device *op, const
struct of_device_id *match)
> >  	struct net_device *ndev;
> >  	const void *addr;
> >  	int size, rc =3D 0;
> > -	unsigned int dcrs;
> >
> >  	/* Init network device structure */
> >  	ndev =3D alloc_etherdev(sizeof(*lp));
> > @@ -870,13 +935,20 @@ temac_of_probe(struct of_device *op, const
struct of_device_id *match)
> >  		goto nodev;
> >  	}
> >
> > -	dcrs =3D dcr_resource_start(np, 0);
> > -	if (dcrs =3D=3D 0) {
> > -		dev_err(&op->dev, "could not get DMA register
address\n");
> > -		goto nodev;
> > +	/* Setup the DMA register accesses, could be DCR or memory
mapped */
> > +	if (temac_dcr_setup(lp, op, np)) {
> > +
> > +		/* no DCR in the device tree, try non-DCR */
> > +		lp->sdma_regs =3D of_iomap(np, 0);
> > +		if (lp->sdma_regs) {
> > +			lp->dma_in =3D temac_dma_in32;
> > +			lp->dma_out =3D temac_dma_out32;
> > +			dev_dbg(&op->dev, "MEM base: %p\n",
lp->sdma_regs);
> > +		} else {
> > +			dev_err(&op->dev, "unable to map DMA
registers\n");
> > +			goto nodev;
> > +		}
> >  	}
> > -	lp->sdma_dcrs =3D dcr_map(np, dcrs, dcr_resource_len(np, 0));
> > -	dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
> >
> >  	lp->rx_irq =3D irq_of_parse_and_map(np, 0);
> >  	lp->tx_irq =3D irq_of_parse_and_map(np, 1);
> =

> =

> --
> Michal Simek, Ing. (M.Eng)
> PetaLogix - Linux Solutions for a Reconfigurable World
> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f:
+61-7-30090663


This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.

^ permalink raw reply

* Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: Michal Simek @ 2010-03-15 14:39 UTC (permalink / raw)
  To: John Linn; +Cc: netdev, John Tyner, linuxppc-dev, john.williams
In-Reply-To: <2d2cb4b6-627a-4e42-bc59-7845bbb6b680@SG2EHSMHS004.ehs.local>

John Linn wrote:
>> -----Original Message-----
>> From: Michal Simek [mailto:michal.simek@petalogix.com]
>> Sent: Monday, March 15, 2010 2:40 AM
>> To: John Linn
>> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
> grant.likely@secretlab.ca;
>> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
>> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
>>
>> John Linn wrote:
>>> This patch adds support for using the LL TEMAC Ethernet driver on
>>> non-Virtex 5 platforms by adding support for accessing the Soft DMA
>>> registers as if they were memory mapped instead of solely through
> the
>>> DCR's (available on the Virtex 5).
>>>
>>> The patch also updates the driver so that it runs on the MicroBlaze.
>>> The changes were tested on the PowerPC 440, PowerPC 405, and the
>>> MicroBlaze platforms.
>> Which git-tree have you tested on? (Of course microblaze)
> 
> It was tested on the Xilinx tree for MicroBlaze which is based on the
> mainline and the Petalogix tree as DMA was needed. I tried to build
> against the mainline head but got errors with the DMA routines. I guess
> it's possible that it was a configuration issue there as I didn't dig
> real deep.

New dma api is in for-linus branch.
I tested it on that version and I am seeing some weird things. :-(
Access to bad area. I will try your tree.

The second thing which I see is in ll_temac_recv function.
On the following line is read a packet length which could be 0-16k.
		length = cur_p->app4 & 0x3FFF;

But allocated skb has max size XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.

What happen if driver get packet greater than 9kB?
I got it (I don't know how) but skb_put has one checking mechanism which 
will cal skb_over_panic which caused panic.
That's why I think that will be good always to check that length is less 
than XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.

What do you think?

Thanks,
Michal

> 
> I tested the PowerPC against the head of mainline.
> 
> Thanks,
> John
> 
>> Michal
>>
>>> Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
>>> Signed-off-by: John Linn <john.linn@xilinx.com>
>>> ---
>>>
>>> V2 - Incorporated comments from Grant and added more logic to allow
> the driver
>>> to work on MicroBlaze.
>>>
>>>  drivers/net/Kconfig         |    1 -
>>>  drivers/net/ll_temac.h      |   17 +++++-
>>>  drivers/net/ll_temac_main.c |  124
> ++++++++++++++++++++++++++++++++++---------
>>>  3 files changed, 113 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>>> index 9b6efe1..5402105 100644
>>> --- a/drivers/net/Kconfig
>>> +++ b/drivers/net/Kconfig
>>> @@ -2443,7 +2443,6 @@ config MV643XX_ETH
>>>  config XILINX_LL_TEMAC
>>>  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC)
> driver"
>>>  	select PHYLIB
>>> -	depends on PPC_DCR_NATIVE
>>>  	help
>>>  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
>>>  	  core used in Xilinx Spartan and Virtex FPGAs
>>> diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
>>> index 1af66a1..915aa34 100644
>>> --- a/drivers/net/ll_temac.h
>>> +++ b/drivers/net/ll_temac.h
>>> @@ -5,8 +5,11 @@
>>>  #include <linux/netdevice.h>
>>>  #include <linux/of.h>
>>>  #include <linux/spinlock.h>
>>> +
>>> +#ifdef CONFIG_PPC_DCR
>>>  #include <asm/dcr.h>
>>>  #include <asm/dcr-regs.h>
>>> +#endif
>>>
>>>  /* packet size info */
>>>  #define XTE_HDR_SIZE			14      /* size of
> Ethernet header */
>>> @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
>>>
>>>  #define TX_CONTROL_CALC_CSUM_MASK   1
>>>
>>> +/* Align the IP data in the packet on word boundaries as MicroBlaze
>>> + * needs it.
>>> + */
>>> +
>>>  #define XTE_ALIGN       32
>>> -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
>>> +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)
>>>
>>>  #define MULTICAST_CAM_TABLE_NUM 4
>>>
>>> @@ -335,9 +342,15 @@ struct temac_local {
>>>  	struct mii_bus *mii_bus;	/* MII bus reference */
>>>  	int mdio_irqs[PHY_MAX_ADDR];	/* IRQs table for MDIO bus */
>>>
>>> -	/* IO registers and IRQs */
>>> +	/* IO registers, dma functions and IRQs */
>>>  	void __iomem *regs;
>>> +	void __iomem *sdma_regs;
>>> +#ifdef CONFIG_PPC_DCR
>>>  	dcr_host_t sdma_dcrs;
>>> +#endif
>>> +	u32 (*dma_in)(struct temac_local *, int);
>>> +	void (*dma_out)(struct temac_local *, int, u32);
>>> +
>>>  	int tx_irq;
>>>  	int rx_irq;
>>>  	int emac_num;
>>> diff --git a/drivers/net/ll_temac_main.c
> b/drivers/net/ll_temac_main.c
>>> index a18e348..9aedf9b 100644
>>> --- a/drivers/net/ll_temac_main.c
>>> +++ b/drivers/net/ll_temac_main.c
>>> @@ -20,9 +20,6 @@
>>>   *   or rx, so this should be okay.
>>>   *
>>>   * TODO:
>>> - * - Fix driver to work on more than just Virtex5.  Right now the
> driver
>>> - *   assumes that the locallink DMA registers are accessed via DCR
>>> - *   instructions.
>>>   * - Factor out locallink DMA code into separate driver
>>>   * - Fix multicast assignment.
>>>   * - Fix support for hardware checksumming.
>>> @@ -115,17 +112,86 @@ void temac_indirect_out32(struct temac_local
> *lp, int reg, u32 value)
>>>  	temac_iow(lp, XTE_CTL0_OFFSET, CNTLREG_WRITE_ENABLE_MASK | reg);
>>>  }
>>>
>>> +/**
>>> + * temac_dma_in32 - Memory mapped DMA read, this function expects a
>>> + * register input that is based on DCR word addresses which
>>> + * are then converted to memory mapped byte addresses
>>> + */
>>>  static u32 temac_dma_in32(struct temac_local *lp, int reg)
>>>  {
>>> -	return dcr_read(lp->sdma_dcrs, reg);
>>> +	return in_be32((u32 *)(lp->sdma_regs + (reg << 2)));
>>>  }
>>>
>>> +/**
>>> + * temac_dma_out32 - Memory mapped DMA read, this function expects
> a
>>> + * register input that is based on DCR word addresses which
>>> + * are then converted to memory mapped byte addresses
>>> + */
>>>  static void temac_dma_out32(struct temac_local *lp, int reg, u32
> value)
>>>  {
>>> +	out_be32((u32 *)(lp->sdma_regs + (reg << 2)), value);
>>> +}
>>> +
>>> +/* DMA register access functions can be DCR based or memory mapped.
>>> + * The PowerPC 440 is DCR based, the PowerPC 405 and MicroBlaze are
> both
>>> + * memory mapped.
>>> + */
>>> +#ifdef CONFIG_PPC_DCR
>>> +
>>> +/**
>>> + * temac_dma_dcr_in32 - DCR based DMA read
>>> + */
>>> +static u32 temac_dma_dcr_in(struct temac_local *lp, int reg)
>>> +{
>>> +	return dcr_read(lp->sdma_dcrs, reg);
>>> +}
>>> +
>>> +/**
>>> + * temac_dma_dcr_out32 - DCR based DMA write
>>> + */
>>> +static void temac_dma_dcr_out(struct temac_local *lp, int reg, u32
> value)
>>> +{
>>>  	dcr_write(lp->sdma_dcrs, reg, value);
>>>  }
>>>
>>>  /**
>>> + * temac_dcr_setup - If the DMA is DCR based, then setup the
> address and
>>> + * I/O  functions
>>> + */
>>> +static int temac_dcr_setup(struct temac_local *lp, struct of_device
> *op,
>>> +				struct device_node *np)
>>> +{
>>> +	unsigned int dcrs;
>>> +
>>> +	/* setup the dcr address mapping if it's in the device tree */
>>> +
>>> +	dcrs = dcr_resource_start(np, 0);
>>> +	if (dcrs != 0) {
>>> +		lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np,
> 0));
>>> +		lp->dma_in = temac_dma_dcr_in;
>>> +		lp->dma_out = temac_dma_dcr_out;
>>> +		dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
>>> +		return 0;
>>> +	}
>>> +	/* no DCR in the device tree, indicate a failure */
>>> +	return -1;
>>> +}
>>> +
>>> +#else
>>> +
>>> +/*
>>> + * temac_dcr_setup - This is a stub for when DCR is not supported,
>>> + * such as with MicroBlaze
>>> + */
>>> +static int temac_dcr_setup(struct temac_local *lp, struct of_device
> *op,
>>> +				struct device_node *np)
>>> +{
>>> +	return -1;
>>> +}
>>> +
>>> +#endif
>>> +
>>> +/**
>>>   * temac_dma_bd_init - Setup buffer descriptor rings
>>>   */
>>>  static int temac_dma_bd_init(struct net_device *ndev)
>>> @@ -172,23 +238,23 @@ static int temac_dma_bd_init(struct net_device
> *ndev)
>>>  		lp->rx_bd_v[i].app0 = STS_CTRL_APP0_IRQONEND;
>>>  	}
>>>
>>> -	temac_dma_out32(lp, TX_CHNL_CTRL, 0x10220400 |
>>> +	lp->dma_out(lp, TX_CHNL_CTRL, 0x10220400 |
>>>  					  CHNL_CTRL_IRQ_EN |
>>>  					  CHNL_CTRL_IRQ_DLY_EN |
>>>  					  CHNL_CTRL_IRQ_COAL_EN);
>>>  	/* 0x10220483 */
>>>  	/* 0x00100483 */
>>> -	temac_dma_out32(lp, RX_CHNL_CTRL, 0xff010000 |
>>> +	lp->dma_out(lp, RX_CHNL_CTRL, 0xff010000 |
>>>  					  CHNL_CTRL_IRQ_EN |
>>>  					  CHNL_CTRL_IRQ_DLY_EN |
>>>  					  CHNL_CTRL_IRQ_COAL_EN |
>>>  					  CHNL_CTRL_IRQ_IOE);
>>>  	/* 0xff010283 */
>>>
>>> -	temac_dma_out32(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
>>> -	temac_dma_out32(lp, RX_TAILDESC_PTR,
>>> +	lp->dma_out(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
>>> +	lp->dma_out(lp, RX_TAILDESC_PTR,
>>>  		       lp->rx_bd_p + (sizeof(*lp->rx_bd_v) * (RX_BD_NUM
> - 1)));
>>> -	temac_dma_out32(lp, TX_CURDESC_PTR, lp->tx_bd_p);
>>> +	lp->dma_out(lp, TX_CURDESC_PTR, lp->tx_bd_p);
>>>
>>>  	return 0;
>>>  }
>>> @@ -426,9 +492,9 @@ static void temac_device_reset(struct net_device
> *ndev)
>>>  	temac_indirect_out32(lp, XTE_RXC1_OFFSET, val &
> ~XTE_RXC1_RXEN_MASK);
>>>  	/* Reset Local Link (DMA) */
>>> -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
>>> +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
>>>  	timeout = 1000;
>>> -	while (temac_dma_in32(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
>>> +	while (lp->dma_in(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
>>>  		udelay(1);
>>>  		if (--timeout == 0) {
>>>  			dev_err(&ndev->dev,
>>> @@ -436,7 +502,7 @@ static void temac_device_reset(struct net_device
> *ndev)
>>>  			break;
>>>  		}
>>>  	}
>>> -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
>>> +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
>>>
>>>  	temac_dma_bd_init(ndev);
>>>
>>> @@ -597,7 +663,7 @@ static int temac_start_xmit(struct sk_buff *skb,
> struct net_device *ndev)
>>>  		lp->tx_bd_tail = 0;
>>>
>>>  	/* Kick off the transfer */
>>> -	temac_dma_out32(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
>>> +	lp->dma_out(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
>>>
>>>  	return NETDEV_TX_OK;
>>>  }
>>> @@ -663,7 +729,7 @@ static void ll_temac_recv(struct net_device
> *ndev)
>>>  		cur_p = &lp->rx_bd_v[lp->rx_bd_ci];
>>>  		bdstat = cur_p->app0;
>>>  	}
>>> -	temac_dma_out32(lp, RX_TAILDESC_PTR, tail_p);
>>> +	lp->dma_out(lp, RX_TAILDESC_PTR, tail_p);
>>>
>>>  	spin_unlock_irqrestore(&lp->rx_lock, flags);
>>>  }
>>> @@ -674,8 +740,8 @@ static irqreturn_t ll_temac_tx_irq(int irq, void
> *_ndev)
>>>  	struct temac_local *lp = netdev_priv(ndev);
>>>  	unsigned int status;
>>>
>>> -	status = temac_dma_in32(lp, TX_IRQ_REG);
>>> -	temac_dma_out32(lp, TX_IRQ_REG, status);
>>> +	status = lp->dma_in(lp, TX_IRQ_REG);
>>> +	lp->dma_out(lp, TX_IRQ_REG, status);
>>>
>>>  	if (status & (IRQ_COAL | IRQ_DLY))
>>>  		temac_start_xmit_done(lp->ndev);
>>> @@ -692,8 +758,8 @@ static irqreturn_t ll_temac_rx_irq(int irq, void
> *_ndev)
>>>  	unsigned int status;
>>>
>>>  	/* Read and clear the status registers */
>>> -	status = temac_dma_in32(lp, RX_IRQ_REG);
>>> -	temac_dma_out32(lp, RX_IRQ_REG, status);
>>> +	status = lp->dma_in(lp, RX_IRQ_REG);
>>> +	lp->dma_out(lp, RX_IRQ_REG, status);
>>>
>>>  	if (status & (IRQ_COAL | IRQ_DLY))
>>>  		ll_temac_recv(lp->ndev);
>>> @@ -794,7 +860,7 @@ static ssize_t temac_show_llink_regs(struct
> device *dev,
>>>  	int i, len = 0;
>>>
>>>  	for (i = 0; i < 0x11; i++)
>>> -		len += sprintf(buf + len, "%.8x%s", temac_dma_in32(lp,
> i),
>>> +		len += sprintf(buf + len, "%.8x%s", lp->dma_in(lp, i),
>>>  			       (i % 8) == 7 ? "\n" : " ");
>>>  	len += sprintf(buf + len, "\n");
>>>
>>> @@ -820,7 +886,6 @@ temac_of_probe(struct of_device *op, const
> struct of_device_id *match)
>>>  	struct net_device *ndev;
>>>  	const void *addr;
>>>  	int size, rc = 0;
>>> -	unsigned int dcrs;
>>>
>>>  	/* Init network device structure */
>>>  	ndev = alloc_etherdev(sizeof(*lp));
>>> @@ -870,13 +935,20 @@ temac_of_probe(struct of_device *op, const
> struct of_device_id *match)
>>>  		goto nodev;
>>>  	}
>>>
>>> -	dcrs = dcr_resource_start(np, 0);
>>> -	if (dcrs == 0) {
>>> -		dev_err(&op->dev, "could not get DMA register
> address\n");
>>> -		goto nodev;
>>> +	/* Setup the DMA register accesses, could be DCR or memory
> mapped */
>>> +	if (temac_dcr_setup(lp, op, np)) {
>>> +
>>> +		/* no DCR in the device tree, try non-DCR */
>>> +		lp->sdma_regs = of_iomap(np, 0);
>>> +		if (lp->sdma_regs) {
>>> +			lp->dma_in = temac_dma_in32;
>>> +			lp->dma_out = temac_dma_out32;
>>> +			dev_dbg(&op->dev, "MEM base: %p\n",
> lp->sdma_regs);
>>> +		} else {
>>> +			dev_err(&op->dev, "unable to map DMA
> registers\n");
>>> +			goto nodev;
>>> +		}
>>>  	}
>>> -	lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np, 0));
>>> -	dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
>>>
>>>  	lp->rx_irq = irq_of_parse_and_map(np, 0);
>>>  	lp->tx_irq = irq_of_parse_and_map(np, 1);
>>
>> --
>> Michal Simek, Ing. (M.Eng)
>> PetaLogix - Linux Solutions for a Reconfigurable World
>> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f:
> +61-7-30090663
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 


-- 
Michal Simek, Ing. (M.Eng)
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f: +61-7-30090663

^ permalink raw reply

* RE: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: John Linn @ 2010-03-15 14:51 UTC (permalink / raw)
  To: michal.simek; +Cc: netdev, John Tyner, linuxppc-dev, john.williams
In-Reply-To: <4B9E46B5.1060508@petalogix.com>

> -----Original Message-----
> From: Michal Simek [mailto:michal.simek@petalogix.com]
> Sent: Monday, March 15, 2010 8:40 AM
> To: John Linn
> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
grant.likely@secretlab.ca;
> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
> =

> John Linn wrote:
> >> -----Original Message-----
> >> From: Michal Simek [mailto:michal.simek@petalogix.com]
> >> Sent: Monday, March 15, 2010 2:40 AM
> >> To: John Linn
> >> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
> > grant.likely@secretlab.ca;
> >> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
> >> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC
driver
> >>
> >> John Linn wrote:
> >>> This patch adds support for using the LL TEMAC Ethernet driver on
> >>> non-Virtex 5 platforms by adding support for accessing the Soft
DMA
> >>> registers as if they were memory mapped instead of solely through
> > the
> >>> DCR's (available on the Virtex 5).
> >>>
> >>> The patch also updates the driver so that it runs on the
MicroBlaze.
> >>> The changes were tested on the PowerPC 440, PowerPC 405, and the
> >>> MicroBlaze platforms.
> >> Which git-tree have you tested on? (Of course microblaze)
> >
> > It was tested on the Xilinx tree for MicroBlaze which is based on
the
> > mainline and the Petalogix tree as DMA was needed. I tried to build
> > against the mainline head but got errors with the DMA routines. I
guess
> > it's possible that it was a configuration issue there as I didn't
dig
> > real deep.
> =

> New dma api is in for-linus branch.
> I tested it on that version and I am seeing some weird things. :-(
> Access to bad area. I will try your tree.
> =

> The second thing which I see is in ll_temac_recv function.
> On the following line is read a packet length which could be 0-16k.
> 		length =3D cur_p->app4 & 0x3FFF;
> =

> But allocated skb has max size XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.
> =

> What happen if driver get packet greater than 9kB?
> I got it (I don't know how) but skb_put has one checking mechanism
which
> will cal skb_over_panic which caused panic.

That's the whole reason for that panic to me and when I got it in the
past and looked it up it made sense to me.  I personally don't see a
good reason to check for it in the driver since the kernel catches it,
but maybe others do.

Thanks,
John

> That's why I think that will be good always to check that length is
less
> than XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.
> =

> What do you think?
> =

> Thanks,
> Michal
> =

> >
> > I tested the PowerPC against the head of mainline.
> >
> > Thanks,
> > John
> >
> >> Michal
> >>
> >>> Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
> >>> Signed-off-by: John Linn <john.linn@xilinx.com>
> >>> ---
> >>>
> >>> V2 - Incorporated comments from Grant and added more logic to
allow
> > the driver
> >>> to work on MicroBlaze.
> >>>
> >>>  drivers/net/Kconfig         |    1 -
> >>>  drivers/net/ll_temac.h      |   17 +++++-
> >>>  drivers/net/ll_temac_main.c |  124
> > ++++++++++++++++++++++++++++++++++---------
> >>>  3 files changed, 113 insertions(+), 29 deletions(-)
> >>>
> >>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> >>> index 9b6efe1..5402105 100644
> >>> --- a/drivers/net/Kconfig
> >>> +++ b/drivers/net/Kconfig
> >>> @@ -2443,7 +2443,6 @@ config MV643XX_ETH
> >>>  config XILINX_LL_TEMAC
> >>>  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC)
> > driver"
> >>>  	select PHYLIB
> >>> -	depends on PPC_DCR_NATIVE
> >>>  	help
> >>>  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
> >>>  	  core used in Xilinx Spartan and Virtex FPGAs
> >>> diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
> >>> index 1af66a1..915aa34 100644
> >>> --- a/drivers/net/ll_temac.h
> >>> +++ b/drivers/net/ll_temac.h
> >>> @@ -5,8 +5,11 @@
> >>>  #include <linux/netdevice.h>
> >>>  #include <linux/of.h>
> >>>  #include <linux/spinlock.h>
> >>> +
> >>> +#ifdef CONFIG_PPC_DCR
> >>>  #include <asm/dcr.h>
> >>>  #include <asm/dcr-regs.h>
> >>> +#endif
> >>>
> >>>  /* packet size info */
> >>>  #define XTE_HDR_SIZE			14      /* size of
> > Ethernet header */
> >>> @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
> >>>
> >>>  #define TX_CONTROL_CALC_CSUM_MASK   1
> >>>
> >>> +/* Align the IP data in the packet on word boundaries as
MicroBlaze
> >>> + * needs it.
> >>> + */
> >>> +
> >>>  #define XTE_ALIGN       32
> >>> -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
> >>> +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)
> >>>
> >>>  #define MULTICAST_CAM_TABLE_NUM 4
> >>>
> >>> @@ -335,9 +342,15 @@ struct temac_local {
> >>>  	struct mii_bus *mii_bus;	/* MII bus reference */
> >>>  	int mdio_irqs[PHY_MAX_ADDR];	/* IRQs table for MDIO bus */
> >>>
> >>> -	/* IO registers and IRQs */
> >>> +	/* IO registers, dma functions and IRQs */
> >>>  	void __iomem *regs;
> >>> +	void __iomem *sdma_regs;
> >>> +#ifdef CONFIG_PPC_DCR
> >>>  	dcr_host_t sdma_dcrs;
> >>> +#endif
> >>> +	u32 (*dma_in)(struct temac_local *, int);
> >>> +	void (*dma_out)(struct temac_local *, int, u32);
> >>> +
> >>>  	int tx_irq;
> >>>  	int rx_irq;
> >>>  	int emac_num;
> >>> diff --git a/drivers/net/ll_temac_main.c
> > b/drivers/net/ll_temac_main.c
> >>> index a18e348..9aedf9b 100644
> >>> --- a/drivers/net/ll_temac_main.c
> >>> +++ b/drivers/net/ll_temac_main.c
> >>> @@ -20,9 +20,6 @@
> >>>   *   or rx, so this should be okay.
> >>>   *
> >>>   * TODO:
> >>> - * - Fix driver to work on more than just Virtex5.  Right now the
> > driver
> >>> - *   assumes that the locallink DMA registers are accessed via
DCR
> >>> - *   instructions.
> >>>   * - Factor out locallink DMA code into separate driver
> >>>   * - Fix multicast assignment.
> >>>   * - Fix support for hardware checksumming.
> >>> @@ -115,17 +112,86 @@ void temac_indirect_out32(struct temac_local
> > *lp, int reg, u32 value)
> >>>  	temac_iow(lp, XTE_CTL0_OFFSET, CNTLREG_WRITE_ENABLE_MASK | reg);
> >>>  }
> >>>
> >>> +/**
> >>> + * temac_dma_in32 - Memory mapped DMA read, this function expects
a
> >>> + * register input that is based on DCR word addresses which
> >>> + * are then converted to memory mapped byte addresses
> >>> + */
> >>>  static u32 temac_dma_in32(struct temac_local *lp, int reg)
> >>>  {
> >>> -	return dcr_read(lp->sdma_dcrs, reg);
> >>> +	return in_be32((u32 *)(lp->sdma_regs + (reg << 2)));
> >>>  }
> >>>
> >>> +/**
> >>> + * temac_dma_out32 - Memory mapped DMA read, this function
expects
> > a
> >>> + * register input that is based on DCR word addresses which
> >>> + * are then converted to memory mapped byte addresses
> >>> + */
> >>>  static void temac_dma_out32(struct temac_local *lp, int reg, u32
> > value)
> >>>  {
> >>> +	out_be32((u32 *)(lp->sdma_regs + (reg << 2)), value);
> >>> +}
> >>> +
> >>> +/* DMA register access functions can be DCR based or memory
mapped.
> >>> + * The PowerPC 440 is DCR based, the PowerPC 405 and MicroBlaze
are
> > both
> >>> + * memory mapped.
> >>> + */
> >>> +#ifdef CONFIG_PPC_DCR
> >>> +
> >>> +/**
> >>> + * temac_dma_dcr_in32 - DCR based DMA read
> >>> + */
> >>> +static u32 temac_dma_dcr_in(struct temac_local *lp, int reg)
> >>> +{
> >>> +	return dcr_read(lp->sdma_dcrs, reg);
> >>> +}
> >>> +
> >>> +/**
> >>> + * temac_dma_dcr_out32 - DCR based DMA write
> >>> + */
> >>> +static void temac_dma_dcr_out(struct temac_local *lp, int reg,
u32
> > value)
> >>> +{
> >>>  	dcr_write(lp->sdma_dcrs, reg, value);
> >>>  }
> >>>
> >>>  /**
> >>> + * temac_dcr_setup - If the DMA is DCR based, then setup the
> > address and
> >>> + * I/O  functions
> >>> + */
> >>> +static int temac_dcr_setup(struct temac_local *lp, struct
of_device
> > *op,
> >>> +				struct device_node *np)
> >>> +{
> >>> +	unsigned int dcrs;
> >>> +
> >>> +	/* setup the dcr address mapping if it's in the device tree */
> >>> +
> >>> +	dcrs =3D dcr_resource_start(np, 0);
> >>> +	if (dcrs !=3D 0) {
> >>> +		lp->sdma_dcrs =3D dcr_map(np, dcrs, dcr_resource_len(np,
> > 0));
> >>> +		lp->dma_in =3D temac_dma_dcr_in;
> >>> +		lp->dma_out =3D temac_dma_dcr_out;
> >>> +		dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
> >>> +		return 0;
> >>> +	}
> >>> +	/* no DCR in the device tree, indicate a failure */
> >>> +	return -1;
> >>> +}
> >>> +
> >>> +#else
> >>> +
> >>> +/*
> >>> + * temac_dcr_setup - This is a stub for when DCR is not
supported,
> >>> + * such as with MicroBlaze
> >>> + */
> >>> +static int temac_dcr_setup(struct temac_local *lp, struct
of_device
> > *op,
> >>> +				struct device_node *np)
> >>> +{
> >>> +	return -1;
> >>> +}
> >>> +
> >>> +#endif
> >>> +
> >>> +/**
> >>>   * temac_dma_bd_init - Setup buffer descriptor rings
> >>>   */
> >>>  static int temac_dma_bd_init(struct net_device *ndev)
> >>> @@ -172,23 +238,23 @@ static int temac_dma_bd_init(struct
net_device
> > *ndev)
> >>>  		lp->rx_bd_v[i].app0 =3D STS_CTRL_APP0_IRQONEND;
> >>>  	}
> >>>
> >>> -	temac_dma_out32(lp, TX_CHNL_CTRL, 0x10220400 |
> >>> +	lp->dma_out(lp, TX_CHNL_CTRL, 0x10220400 |
> >>>  					  CHNL_CTRL_IRQ_EN |
> >>>  					  CHNL_CTRL_IRQ_DLY_EN |
> >>>  					  CHNL_CTRL_IRQ_COAL_EN);
> >>>  	/* 0x10220483 */
> >>>  	/* 0x00100483 */
> >>> -	temac_dma_out32(lp, RX_CHNL_CTRL, 0xff010000 |
> >>> +	lp->dma_out(lp, RX_CHNL_CTRL, 0xff010000 |
> >>>  					  CHNL_CTRL_IRQ_EN |
> >>>  					  CHNL_CTRL_IRQ_DLY_EN |
> >>>  					  CHNL_CTRL_IRQ_COAL_EN |
> >>>  					  CHNL_CTRL_IRQ_IOE);
> >>>  	/* 0xff010283 */
> >>>
> >>> -	temac_dma_out32(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
> >>> -	temac_dma_out32(lp, RX_TAILDESC_PTR,
> >>> +	lp->dma_out(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
> >>> +	lp->dma_out(lp, RX_TAILDESC_PTR,
> >>>  		       lp->rx_bd_p + (sizeof(*lp->rx_bd_v) * (RX_BD_NUM
> > - 1)));
> >>> -	temac_dma_out32(lp, TX_CURDESC_PTR, lp->tx_bd_p);
> >>> +	lp->dma_out(lp, TX_CURDESC_PTR, lp->tx_bd_p);
> >>>
> >>>  	return 0;
> >>>  }
> >>> @@ -426,9 +492,9 @@ static void temac_device_reset(struct
net_device
> > *ndev)
> >>>  	temac_indirect_out32(lp, XTE_RXC1_OFFSET, val &
> > ~XTE_RXC1_RXEN_MASK);
> >>>  	/* Reset Local Link (DMA) */
> >>> -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
> >>> +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
> >>>  	timeout =3D 1000;
> >>> -	while (temac_dma_in32(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
> >>> +	while (lp->dma_in(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
> >>>  		udelay(1);
> >>>  		if (--timeout =3D=3D 0) {
> >>>  			dev_err(&ndev->dev,
> >>> @@ -436,7 +502,7 @@ static void temac_device_reset(struct
net_device
> > *ndev)
> >>>  			break;
> >>>  		}
> >>>  	}
> >>> -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
> >>> +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
> >>>
> >>>  	temac_dma_bd_init(ndev);
> >>>
> >>> @@ -597,7 +663,7 @@ static int temac_start_xmit(struct sk_buff
*skb,
> > struct net_device *ndev)
> >>>  		lp->tx_bd_tail =3D 0;
> >>>
> >>>  	/* Kick off the transfer */
> >>> -	temac_dma_out32(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
> >>> +	lp->dma_out(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
> >>>
> >>>  	return NETDEV_TX_OK;
> >>>  }
> >>> @@ -663,7 +729,7 @@ static void ll_temac_recv(struct net_device
> > *ndev)
> >>>  		cur_p =3D &lp->rx_bd_v[lp->rx_bd_ci];
> >>>  		bdstat =3D cur_p->app0;
> >>>  	}
> >>> -	temac_dma_out32(lp, RX_TAILDESC_PTR, tail_p);
> >>> +	lp->dma_out(lp, RX_TAILDESC_PTR, tail_p);
> >>>
> >>>  	spin_unlock_irqrestore(&lp->rx_lock, flags);
> >>>  }
> >>> @@ -674,8 +740,8 @@ static irqreturn_t ll_temac_tx_irq(int irq,
void
> > *_ndev)
> >>>  	struct temac_local *lp =3D netdev_priv(ndev);
> >>>  	unsigned int status;
> >>>
> >>> -	status =3D temac_dma_in32(lp, TX_IRQ_REG);
> >>> -	temac_dma_out32(lp, TX_IRQ_REG, status);
> >>> +	status =3D lp->dma_in(lp, TX_IRQ_REG);
> >>> +	lp->dma_out(lp, TX_IRQ_REG, status);
> >>>
> >>>  	if (status & (IRQ_COAL | IRQ_DLY))
> >>>  		temac_start_xmit_done(lp->ndev);
> >>> @@ -692,8 +758,8 @@ static irqreturn_t ll_temac_rx_irq(int irq,
void
> > *_ndev)
> >>>  	unsigned int status;
> >>>
> >>>  	/* Read and clear the status registers */
> >>> -	status =3D temac_dma_in32(lp, RX_IRQ_REG);
> >>> -	temac_dma_out32(lp, RX_IRQ_REG, status);
> >>> +	status =3D lp->dma_in(lp, RX_IRQ_REG);
> >>> +	lp->dma_out(lp, RX_IRQ_REG, status);
> >>>
> >>>  	if (status & (IRQ_COAL | IRQ_DLY))
> >>>  		ll_temac_recv(lp->ndev);
> >>> @@ -794,7 +860,7 @@ static ssize_t temac_show_llink_regs(struct
> > device *dev,
> >>>  	int i, len =3D 0;
> >>>
> >>>  	for (i =3D 0; i < 0x11; i++)
> >>> -		len +=3D sprintf(buf + len, "%.8x%s", temac_dma_in32(lp,
> > i),
> >>> +		len +=3D sprintf(buf + len, "%.8x%s", lp->dma_in(lp, i),
> >>>  			       (i % 8) =3D=3D 7 ? "\n" : " ");
> >>>  	len +=3D sprintf(buf + len, "\n");
> >>>
> >>> @@ -820,7 +886,6 @@ temac_of_probe(struct of_device *op, const
> > struct of_device_id *match)
> >>>  	struct net_device *ndev;
> >>>  	const void *addr;
> >>>  	int size, rc =3D 0;
> >>> -	unsigned int dcrs;
> >>>
> >>>  	/* Init network device structure */
> >>>  	ndev =3D alloc_etherdev(sizeof(*lp));
> >>> @@ -870,13 +935,20 @@ temac_of_probe(struct of_device *op, const
> > struct of_device_id *match)
> >>>  		goto nodev;
> >>>  	}
> >>>
> >>> -	dcrs =3D dcr_resource_start(np, 0);
> >>> -	if (dcrs =3D=3D 0) {
> >>> -		dev_err(&op->dev, "could not get DMA register
> > address\n");
> >>> -		goto nodev;
> >>> +	/* Setup the DMA register accesses, could be DCR or memory
> > mapped */
> >>> +	if (temac_dcr_setup(lp, op, np)) {
> >>> +
> >>> +		/* no DCR in the device tree, try non-DCR */
> >>> +		lp->sdma_regs =3D of_iomap(np, 0);
> >>> +		if (lp->sdma_regs) {
> >>> +			lp->dma_in =3D temac_dma_in32;
> >>> +			lp->dma_out =3D temac_dma_out32;
> >>> +			dev_dbg(&op->dev, "MEM base: %p\n",
> > lp->sdma_regs);
> >>> +		} else {
> >>> +			dev_err(&op->dev, "unable to map DMA
> > registers\n");
> >>> +			goto nodev;
> >>> +		}
> >>>  	}
> >>> -	lp->sdma_dcrs =3D dcr_map(np, dcrs, dcr_resource_len(np, 0));
> >>> -	dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
> >>>
> >>>  	lp->rx_irq =3D irq_of_parse_and_map(np, 0);
> >>>  	lp->tx_irq =3D irq_of_parse_and_map(np, 1);
> >>
> >> --
> >> Michal Simek, Ing. (M.Eng)
> >> PetaLogix - Linux Solutions for a Reconfigurable World
> >> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f:
> > +61-7-30090663
> >
> >
> > This email and any attachments are intended for the sole use of the
named recipient(s) and
> contain(s) confidential information that may be proprietary,
privileged or copyrighted under
> applicable law. If you are not the intended recipient, do not read,
copy, or forward this email
> message or any attachments. Delete this email message and any
attachments immediately.
> >
> >
> =

> =

> --
> Michal Simek, Ing. (M.Eng)
> PetaLogix - Linux Solutions for a Reconfigurable World
> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f:
+61-7-30090663


This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.

^ permalink raw reply

* Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: Michal Simek @ 2010-03-15 14:57 UTC (permalink / raw)
  To: John Linn; +Cc: netdev, John Tyner, linuxppc-dev, john.williams
In-Reply-To: <89d94bf2-02cf-492c-89cb-31f0ac1501ba@SG2EHSMHS010.ehs.local>

John Linn wrote:
>> -----Original Message-----
>> From: Michal Simek [mailto:michal.simek@petalogix.com]
>> Sent: Monday, March 15, 2010 8:40 AM
>> To: John Linn
>> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
> grant.likely@secretlab.ca;
>> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
>> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
>>
>> John Linn wrote:
>>>> -----Original Message-----
>>>> From: Michal Simek [mailto:michal.simek@petalogix.com]
>>>> Sent: Monday, March 15, 2010 2:40 AM
>>>> To: John Linn
>>>> Cc: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
>>> grant.likely@secretlab.ca;
>>>> jwboyer@linux.vnet.ibm.com; john.williams@petalogix.com; John Tyner
>>>> Subject: Re: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC
> driver
>>>> John Linn wrote:
>>>>> This patch adds support for using the LL TEMAC Ethernet driver on
>>>>> non-Virtex 5 platforms by adding support for accessing the Soft
> DMA
>>>>> registers as if they were memory mapped instead of solely through
>>> the
>>>>> DCR's (available on the Virtex 5).
>>>>>
>>>>> The patch also updates the driver so that it runs on the
> MicroBlaze.
>>>>> The changes were tested on the PowerPC 440, PowerPC 405, and the
>>>>> MicroBlaze platforms.
>>>> Which git-tree have you tested on? (Of course microblaze)
>>> It was tested on the Xilinx tree for MicroBlaze which is based on
> the
>>> mainline and the Petalogix tree as DMA was needed. I tried to build
>>> against the mainline head but got errors with the DMA routines. I
> guess
>>> it's possible that it was a configuration issue there as I didn't
> dig
>>> real deep.
>> New dma api is in for-linus branch.
>> I tested it on that version and I am seeing some weird things. :-(
>> Access to bad area. I will try your tree.
>>
>> The second thing which I see is in ll_temac_recv function.
>> On the following line is read a packet length which could be 0-16k.
>> 		length = cur_p->app4 & 0x3FFF;
>>
>> But allocated skb has max size XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.
>>
>> What happen if driver get packet greater than 9kB?
>> I got it (I don't know how) but skb_put has one checking mechanism
> which
>> will cal skb_over_panic which caused panic.
> 
> That's the whole reason for that panic to me and when I got it in the
> past and looked it up it made sense to me.  I personally don't see a
> good reason to check for it in the driver since the kernel catches it,
> but maybe others do.

yes, kernel is checking it but caused panic which ends in fault in 
kernel and caused kernel crash. :-( This could be a good reason to check 
it in driver. I don't want to reset the board when ll_temac gets longer 
packet, do you?

I have seen the second type of fault which caused kernel access to bad 
area (+ kernel panic). Have you done any iperf tests or flood ping?

Thanks,
Michal

> 
> Thanks,
> John
> 
>> That's why I think that will be good always to check that length is
> less
>> than XTE_MAX_JUMBO_FRAME_SIZE + XTE_ALIGN.
>>
>> What do you think?
>>
>> Thanks,
>> Michal
>>
>>> I tested the PowerPC against the head of mainline.
>>>
>>> Thanks,
>>> John
>>>
>>>> Michal
>>>>
>>>>> Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
>>>>> Signed-off-by: John Linn <john.linn@xilinx.com>
>>>>> ---
>>>>>
>>>>> V2 - Incorporated comments from Grant and added more logic to
> allow
>>> the driver
>>>>> to work on MicroBlaze.
>>>>>
>>>>>  drivers/net/Kconfig         |    1 -
>>>>>  drivers/net/ll_temac.h      |   17 +++++-
>>>>>  drivers/net/ll_temac_main.c |  124
>>> ++++++++++++++++++++++++++++++++++---------
>>>>>  3 files changed, 113 insertions(+), 29 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
>>>>> index 9b6efe1..5402105 100644
>>>>> --- a/drivers/net/Kconfig
>>>>> +++ b/drivers/net/Kconfig
>>>>> @@ -2443,7 +2443,6 @@ config MV643XX_ETH
>>>>>  config XILINX_LL_TEMAC
>>>>>  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC)
>>> driver"
>>>>>  	select PHYLIB
>>>>> -	depends on PPC_DCR_NATIVE
>>>>>  	help
>>>>>  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
>>>>>  	  core used in Xilinx Spartan and Virtex FPGAs
>>>>> diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
>>>>> index 1af66a1..915aa34 100644
>>>>> --- a/drivers/net/ll_temac.h
>>>>> +++ b/drivers/net/ll_temac.h
>>>>> @@ -5,8 +5,11 @@
>>>>>  #include <linux/netdevice.h>
>>>>>  #include <linux/of.h>
>>>>>  #include <linux/spinlock.h>
>>>>> +
>>>>> +#ifdef CONFIG_PPC_DCR
>>>>>  #include <asm/dcr.h>
>>>>>  #include <asm/dcr-regs.h>
>>>>> +#endif
>>>>>
>>>>>  /* packet size info */
>>>>>  #define XTE_HDR_SIZE			14      /* size of
>>> Ethernet header */
>>>>> @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
>>>>>
>>>>>  #define TX_CONTROL_CALC_CSUM_MASK   1
>>>>>
>>>>> +/* Align the IP data in the packet on word boundaries as
> MicroBlaze
>>>>> + * needs it.
>>>>> + */
>>>>> +
>>>>>  #define XTE_ALIGN       32
>>>>> -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
>>>>> +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)
>>>>>
>>>>>  #define MULTICAST_CAM_TABLE_NUM 4
>>>>>
>>>>> @@ -335,9 +342,15 @@ struct temac_local {
>>>>>  	struct mii_bus *mii_bus;	/* MII bus reference */
>>>>>  	int mdio_irqs[PHY_MAX_ADDR];	/* IRQs table for MDIO bus */
>>>>>
>>>>> -	/* IO registers and IRQs */
>>>>> +	/* IO registers, dma functions and IRQs */
>>>>>  	void __iomem *regs;
>>>>> +	void __iomem *sdma_regs;
>>>>> +#ifdef CONFIG_PPC_DCR
>>>>>  	dcr_host_t sdma_dcrs;
>>>>> +#endif
>>>>> +	u32 (*dma_in)(struct temac_local *, int);
>>>>> +	void (*dma_out)(struct temac_local *, int, u32);
>>>>> +
>>>>>  	int tx_irq;
>>>>>  	int rx_irq;
>>>>>  	int emac_num;
>>>>> diff --git a/drivers/net/ll_temac_main.c
>>> b/drivers/net/ll_temac_main.c
>>>>> index a18e348..9aedf9b 100644
>>>>> --- a/drivers/net/ll_temac_main.c
>>>>> +++ b/drivers/net/ll_temac_main.c
>>>>> @@ -20,9 +20,6 @@
>>>>>   *   or rx, so this should be okay.
>>>>>   *
>>>>>   * TODO:
>>>>> - * - Fix driver to work on more than just Virtex5.  Right now the
>>> driver
>>>>> - *   assumes that the locallink DMA registers are accessed via
> DCR
>>>>> - *   instructions.
>>>>>   * - Factor out locallink DMA code into separate driver
>>>>>   * - Fix multicast assignment.
>>>>>   * - Fix support for hardware checksumming.
>>>>> @@ -115,17 +112,86 @@ void temac_indirect_out32(struct temac_local
>>> *lp, int reg, u32 value)
>>>>>  	temac_iow(lp, XTE_CTL0_OFFSET, CNTLREG_WRITE_ENABLE_MASK | reg);
>>>>>  }
>>>>>
>>>>> +/**
>>>>> + * temac_dma_in32 - Memory mapped DMA read, this function expects
> a
>>>>> + * register input that is based on DCR word addresses which
>>>>> + * are then converted to memory mapped byte addresses
>>>>> + */
>>>>>  static u32 temac_dma_in32(struct temac_local *lp, int reg)
>>>>>  {
>>>>> -	return dcr_read(lp->sdma_dcrs, reg);
>>>>> +	return in_be32((u32 *)(lp->sdma_regs + (reg << 2)));
>>>>>  }
>>>>>
>>>>> +/**
>>>>> + * temac_dma_out32 - Memory mapped DMA read, this function
> expects
>>> a
>>>>> + * register input that is based on DCR word addresses which
>>>>> + * are then converted to memory mapped byte addresses
>>>>> + */
>>>>>  static void temac_dma_out32(struct temac_local *lp, int reg, u32
>>> value)
>>>>>  {
>>>>> +	out_be32((u32 *)(lp->sdma_regs + (reg << 2)), value);
>>>>> +}
>>>>> +
>>>>> +/* DMA register access functions can be DCR based or memory
> mapped.
>>>>> + * The PowerPC 440 is DCR based, the PowerPC 405 and MicroBlaze
> are
>>> both
>>>>> + * memory mapped.
>>>>> + */
>>>>> +#ifdef CONFIG_PPC_DCR
>>>>> +
>>>>> +/**
>>>>> + * temac_dma_dcr_in32 - DCR based DMA read
>>>>> + */
>>>>> +static u32 temac_dma_dcr_in(struct temac_local *lp, int reg)
>>>>> +{
>>>>> +	return dcr_read(lp->sdma_dcrs, reg);
>>>>> +}
>>>>> +
>>>>> +/**
>>>>> + * temac_dma_dcr_out32 - DCR based DMA write
>>>>> + */
>>>>> +static void temac_dma_dcr_out(struct temac_local *lp, int reg,
> u32
>>> value)
>>>>> +{
>>>>>  	dcr_write(lp->sdma_dcrs, reg, value);
>>>>>  }
>>>>>
>>>>>  /**
>>>>> + * temac_dcr_setup - If the DMA is DCR based, then setup the
>>> address and
>>>>> + * I/O  functions
>>>>> + */
>>>>> +static int temac_dcr_setup(struct temac_local *lp, struct
> of_device
>>> *op,
>>>>> +				struct device_node *np)
>>>>> +{
>>>>> +	unsigned int dcrs;
>>>>> +
>>>>> +	/* setup the dcr address mapping if it's in the device tree */
>>>>> +
>>>>> +	dcrs = dcr_resource_start(np, 0);
>>>>> +	if (dcrs != 0) {
>>>>> +		lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np,
>>> 0));
>>>>> +		lp->dma_in = temac_dma_dcr_in;
>>>>> +		lp->dma_out = temac_dma_dcr_out;
>>>>> +		dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
>>>>> +		return 0;
>>>>> +	}
>>>>> +	/* no DCR in the device tree, indicate a failure */
>>>>> +	return -1;
>>>>> +}
>>>>> +
>>>>> +#else
>>>>> +
>>>>> +/*
>>>>> + * temac_dcr_setup - This is a stub for when DCR is not
> supported,
>>>>> + * such as with MicroBlaze
>>>>> + */
>>>>> +static int temac_dcr_setup(struct temac_local *lp, struct
> of_device
>>> *op,
>>>>> +				struct device_node *np)
>>>>> +{
>>>>> +	return -1;
>>>>> +}
>>>>> +
>>>>> +#endif
>>>>> +
>>>>> +/**
>>>>>   * temac_dma_bd_init - Setup buffer descriptor rings
>>>>>   */
>>>>>  static int temac_dma_bd_init(struct net_device *ndev)
>>>>> @@ -172,23 +238,23 @@ static int temac_dma_bd_init(struct
> net_device
>>> *ndev)
>>>>>  		lp->rx_bd_v[i].app0 = STS_CTRL_APP0_IRQONEND;
>>>>>  	}
>>>>>
>>>>> -	temac_dma_out32(lp, TX_CHNL_CTRL, 0x10220400 |
>>>>> +	lp->dma_out(lp, TX_CHNL_CTRL, 0x10220400 |
>>>>>  					  CHNL_CTRL_IRQ_EN |
>>>>>  					  CHNL_CTRL_IRQ_DLY_EN |
>>>>>  					  CHNL_CTRL_IRQ_COAL_EN);
>>>>>  	/* 0x10220483 */
>>>>>  	/* 0x00100483 */
>>>>> -	temac_dma_out32(lp, RX_CHNL_CTRL, 0xff010000 |
>>>>> +	lp->dma_out(lp, RX_CHNL_CTRL, 0xff010000 |
>>>>>  					  CHNL_CTRL_IRQ_EN |
>>>>>  					  CHNL_CTRL_IRQ_DLY_EN |
>>>>>  					  CHNL_CTRL_IRQ_COAL_EN |
>>>>>  					  CHNL_CTRL_IRQ_IOE);
>>>>>  	/* 0xff010283 */
>>>>>
>>>>> -	temac_dma_out32(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
>>>>> -	temac_dma_out32(lp, RX_TAILDESC_PTR,
>>>>> +	lp->dma_out(lp, RX_CURDESC_PTR,  lp->rx_bd_p);
>>>>> +	lp->dma_out(lp, RX_TAILDESC_PTR,
>>>>>  		       lp->rx_bd_p + (sizeof(*lp->rx_bd_v) * (RX_BD_NUM
>>> - 1)));
>>>>> -	temac_dma_out32(lp, TX_CURDESC_PTR, lp->tx_bd_p);
>>>>> +	lp->dma_out(lp, TX_CURDESC_PTR, lp->tx_bd_p);
>>>>>
>>>>>  	return 0;
>>>>>  }
>>>>> @@ -426,9 +492,9 @@ static void temac_device_reset(struct
> net_device
>>> *ndev)
>>>>>  	temac_indirect_out32(lp, XTE_RXC1_OFFSET, val &
>>> ~XTE_RXC1_RXEN_MASK);
>>>>>  	/* Reset Local Link (DMA) */
>>>>> -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
>>>>> +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_CONTROL_RST);
>>>>>  	timeout = 1000;
>>>>> -	while (temac_dma_in32(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
>>>>> +	while (lp->dma_in(lp, DMA_CONTROL_REG) & DMA_CONTROL_RST) {
>>>>>  		udelay(1);
>>>>>  		if (--timeout == 0) {
>>>>>  			dev_err(&ndev->dev,
>>>>> @@ -436,7 +502,7 @@ static void temac_device_reset(struct
> net_device
>>> *ndev)
>>>>>  			break;
>>>>>  		}
>>>>>  	}
>>>>> -	temac_dma_out32(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
>>>>> +	lp->dma_out(lp, DMA_CONTROL_REG, DMA_TAIL_ENABLE);
>>>>>
>>>>>  	temac_dma_bd_init(ndev);
>>>>>
>>>>> @@ -597,7 +663,7 @@ static int temac_start_xmit(struct sk_buff
> *skb,
>>> struct net_device *ndev)
>>>>>  		lp->tx_bd_tail = 0;
>>>>>
>>>>>  	/* Kick off the transfer */
>>>>> -	temac_dma_out32(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
>>>>> +	lp->dma_out(lp, TX_TAILDESC_PTR, tail_p); /* DMA start */
>>>>>
>>>>>  	return NETDEV_TX_OK;
>>>>>  }
>>>>> @@ -663,7 +729,7 @@ static void ll_temac_recv(struct net_device
>>> *ndev)
>>>>>  		cur_p = &lp->rx_bd_v[lp->rx_bd_ci];
>>>>>  		bdstat = cur_p->app0;
>>>>>  	}
>>>>> -	temac_dma_out32(lp, RX_TAILDESC_PTR, tail_p);
>>>>> +	lp->dma_out(lp, RX_TAILDESC_PTR, tail_p);
>>>>>
>>>>>  	spin_unlock_irqrestore(&lp->rx_lock, flags);
>>>>>  }
>>>>> @@ -674,8 +740,8 @@ static irqreturn_t ll_temac_tx_irq(int irq,
> void
>>> *_ndev)
>>>>>  	struct temac_local *lp = netdev_priv(ndev);
>>>>>  	unsigned int status;
>>>>>
>>>>> -	status = temac_dma_in32(lp, TX_IRQ_REG);
>>>>> -	temac_dma_out32(lp, TX_IRQ_REG, status);
>>>>> +	status = lp->dma_in(lp, TX_IRQ_REG);
>>>>> +	lp->dma_out(lp, TX_IRQ_REG, status);
>>>>>
>>>>>  	if (status & (IRQ_COAL | IRQ_DLY))
>>>>>  		temac_start_xmit_done(lp->ndev);
>>>>> @@ -692,8 +758,8 @@ static irqreturn_t ll_temac_rx_irq(int irq,
> void
>>> *_ndev)
>>>>>  	unsigned int status;
>>>>>
>>>>>  	/* Read and clear the status registers */
>>>>> -	status = temac_dma_in32(lp, RX_IRQ_REG);
>>>>> -	temac_dma_out32(lp, RX_IRQ_REG, status);
>>>>> +	status = lp->dma_in(lp, RX_IRQ_REG);
>>>>> +	lp->dma_out(lp, RX_IRQ_REG, status);
>>>>>
>>>>>  	if (status & (IRQ_COAL | IRQ_DLY))
>>>>>  		ll_temac_recv(lp->ndev);
>>>>> @@ -794,7 +860,7 @@ static ssize_t temac_show_llink_regs(struct
>>> device *dev,
>>>>>  	int i, len = 0;
>>>>>
>>>>>  	for (i = 0; i < 0x11; i++)
>>>>> -		len += sprintf(buf + len, "%.8x%s", temac_dma_in32(lp,
>>> i),
>>>>> +		len += sprintf(buf + len, "%.8x%s", lp->dma_in(lp, i),
>>>>>  			       (i % 8) == 7 ? "\n" : " ");
>>>>>  	len += sprintf(buf + len, "\n");
>>>>>
>>>>> @@ -820,7 +886,6 @@ temac_of_probe(struct of_device *op, const
>>> struct of_device_id *match)
>>>>>  	struct net_device *ndev;
>>>>>  	const void *addr;
>>>>>  	int size, rc = 0;
>>>>> -	unsigned int dcrs;
>>>>>
>>>>>  	/* Init network device structure */
>>>>>  	ndev = alloc_etherdev(sizeof(*lp));
>>>>> @@ -870,13 +935,20 @@ temac_of_probe(struct of_device *op, const
>>> struct of_device_id *match)
>>>>>  		goto nodev;
>>>>>  	}
>>>>>
>>>>> -	dcrs = dcr_resource_start(np, 0);
>>>>> -	if (dcrs == 0) {
>>>>> -		dev_err(&op->dev, "could not get DMA register
>>> address\n");
>>>>> -		goto nodev;
>>>>> +	/* Setup the DMA register accesses, could be DCR or memory
>>> mapped */
>>>>> +	if (temac_dcr_setup(lp, op, np)) {
>>>>> +
>>>>> +		/* no DCR in the device tree, try non-DCR */
>>>>> +		lp->sdma_regs = of_iomap(np, 0);
>>>>> +		if (lp->sdma_regs) {
>>>>> +			lp->dma_in = temac_dma_in32;
>>>>> +			lp->dma_out = temac_dma_out32;
>>>>> +			dev_dbg(&op->dev, "MEM base: %p\n",
>>> lp->sdma_regs);
>>>>> +		} else {
>>>>> +			dev_err(&op->dev, "unable to map DMA
>>> registers\n");
>>>>> +			goto nodev;
>>>>> +		}
>>>>>  	}
>>>>> -	lp->sdma_dcrs = dcr_map(np, dcrs, dcr_resource_len(np, 0));
>>>>> -	dev_dbg(&op->dev, "DCR base: %x\n", dcrs);
>>>>>
>>>>>  	lp->rx_irq = irq_of_parse_and_map(np, 0);
>>>>>  	lp->tx_irq = irq_of_parse_and_map(np, 1);
>>>> --
>>>> Michal Simek, Ing. (M.Eng)
>>>> PetaLogix - Linux Solutions for a Reconfigurable World
>>>> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f:
>>> +61-7-30090663
>>>
>>>
>>> This email and any attachments are intended for the sole use of the
> named recipient(s) and
>> contain(s) confidential information that may be proprietary,
> privileged or copyrighted under
>> applicable law. If you are not the intended recipient, do not read,
> copy, or forward this email
>> message or any attachments. Delete this email message and any
> attachments immediately.
>>>
>>
>> --
>> Michal Simek, Ing. (M.Eng)
>> PetaLogix - Linux Solutions for a Reconfigurable World
>> w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f:
> +61-7-30090663
> 
> 
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
> 
> 


-- 
Michal Simek, Ing. (M.Eng)
PetaLogix - Linux Solutions for a Reconfigurable World
w: www.petalogix.com p: +61-7-30090663,+42-0-721842854 f: +61-7-30090663

^ permalink raw reply

* RE: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: Stephen Neuendorffer @ 2010-03-15 17:03 UTC (permalink / raw)
  To: John Linn, netdev, linuxppc-dev, grant.likely, jwboyer
  Cc: michal.simek, John Tyner, john.williams
In-Reply-To: <bda26c8d-00b2-411d-b4b7-ad6954b9049e@SG2EHSMHS005.ehs.local>



> -----Original Message-----
> From: linuxppc-dev-bounces+stephen=3Dneuendorffer.name@lists.ozlabs.org
[mailto:linuxppc-dev-
> bounces+stephen=3Dneuendorffer.name@lists.ozlabs.org] On Behalf Of John
Linn
> Sent: Friday, March 12, 2010 5:06 PM
> To: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
grant.likely@secretlab.ca;
> jwboyer@linux.vnet.ibm.com
> Cc: michal.simek@petalogix.com; John Tyner; John Linn;
john.williams@petalogix.com
> Subject: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
> 
> This patch adds support for using the LL TEMAC Ethernet driver on
> non-Virtex 5 platforms by adding support for accessing the Soft DMA
> registers as if they were memory mapped instead of solely through the
> DCR's (available on the Virtex 5).
> =

> The patch also updates the driver so that it runs on the MicroBlaze.
> The changes were tested on the PowerPC 440, PowerPC 405, and the
> MicroBlaze platforms.
> =

> Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
> Signed-off-by: John Linn <john.linn@xilinx.com>
> ---
> =

> V2 - Incorporated comments from Grant and added more logic to allow
the driver
> to work on MicroBlaze.
> =

>  drivers/net/Kconfig         |    1 -
>  drivers/net/ll_temac.h      |   17 +++++-
>  drivers/net/ll_temac_main.c |  124
++++++++++++++++++++++++++++++++++---------
>  3 files changed, 113 insertions(+), 29 deletions(-)
> =

> diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> index 9b6efe1..5402105 100644
> --- a/drivers/net/Kconfig
> +++ b/drivers/net/Kconfig
> @@ -2443,7 +2443,6 @@ config MV643XX_ETH
>  config XILINX_LL_TEMAC
>  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC)
driver"
>  	select PHYLIB
> -	depends on PPC_DCR_NATIVE
>  	help
>  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
>  	  core used in Xilinx Spartan and Virtex FPGAs
> diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
> index 1af66a1..915aa34 100644
> --- a/drivers/net/ll_temac.h
> +++ b/drivers/net/ll_temac.h
> @@ -5,8 +5,11 @@
>  #include <linux/netdevice.h>
>  #include <linux/of.h>
>  #include <linux/spinlock.h>
> +
> +#ifdef CONFIG_PPC_DCR
>  #include <asm/dcr.h>
>  #include <asm/dcr-regs.h>
> +#endif
> =

>  /* packet size info */
>  #define XTE_HDR_SIZE			14      /* size of Ethernet
header */
> @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
> =

>  #define TX_CONTROL_CALC_CSUM_MASK   1
> =

> +/* Align the IP data in the packet on word boundaries as MicroBlaze
> + * needs it.
> + */
> +
>  #define XTE_ALIGN       32
> -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
> +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)

Is '34' really XTE_ALIGN + 2?  (I really have no idea.... it just looks
like a suspicious change.)

Steve

This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.

^ permalink raw reply

* RE: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
From: John Linn @ 2010-03-15 17:23 UTC (permalink / raw)
  To: Stephen Neuendorffer, netdev, linuxppc-dev, grant.likely, jwboyer
  Cc: michal.simek, John Tyner, john.williams
In-Reply-To: <977C41F842E66D4CB2E41332313B615009A27BCA@XSJ-EXCHVS1.xlnx.xilinx.com>


> -----Original Message-----
> From: Stephen Neuendorffer
> Sent: Monday, March 15, 2010 11:03 AM
> To: John Linn; netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
grant.likely@secretlab.ca;
> jwboyer@linux.vnet.ibm.com
> Cc: michal.simek@petalogix.com; John Tyner;
john.williams@petalogix.com
> Subject: RE: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
> =

> =

> =

> > -----Original Message-----
> > From:
linuxppc-dev-bounces+stephen=3Dneuendorffer.name@lists.ozlabs.org
[mailto:linuxppc-dev-
> > bounces+stephen=3Dneuendorffer.name@lists.ozlabs.org] On Behalf Of
John Linn
> > Sent: Friday, March 12, 2010 5:06 PM
> > To: netdev@vger.kernel.org; linuxppc-dev@ozlabs.org;
grant.likely@secretlab.ca;
> > jwboyer@linux.vnet.ibm.com
> > Cc: michal.simek@petalogix.com; John Tyner; John Linn;
john.williams@petalogix.com
> > Subject: [PATCH] [V2] Add non-Virtex5 support for LL TEMAC driver
> >
> > This patch adds support for using the LL TEMAC Ethernet driver on
> > non-Virtex 5 platforms by adding support for accessing the Soft DMA
> > registers as if they were memory mapped instead of solely through
the
> > DCR's (available on the Virtex 5).
> >
> > The patch also updates the driver so that it runs on the MicroBlaze.
> > The changes were tested on the PowerPC 440, PowerPC 405, and the
> > MicroBlaze platforms.
> >
> > Signed-off-by: John Tyner <jtyner@cs.ucr.edu>
> > Signed-off-by: John Linn <john.linn@xilinx.com>
> > ---
> >
> > V2 - Incorporated comments from Grant and added more logic to allow
the driver
> > to work on MicroBlaze.
> >
> >  drivers/net/Kconfig         |    1 -
> >  drivers/net/ll_temac.h      |   17 +++++-
> >  drivers/net/ll_temac_main.c |  124
++++++++++++++++++++++++++++++++++---------
> >  3 files changed, 113 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
> > index 9b6efe1..5402105 100644
> > --- a/drivers/net/Kconfig
> > +++ b/drivers/net/Kconfig
> > @@ -2443,7 +2443,6 @@ config MV643XX_ETH
> >  config XILINX_LL_TEMAC
> >  	tristate "Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC)
driver"
> >  	select PHYLIB
> > -	depends on PPC_DCR_NATIVE
> >  	help
> >  	  This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
> >  	  core used in Xilinx Spartan and Virtex FPGAs
> > diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
> > index 1af66a1..915aa34 100644
> > --- a/drivers/net/ll_temac.h
> > +++ b/drivers/net/ll_temac.h
> > @@ -5,8 +5,11 @@
> >  #include <linux/netdevice.h>
> >  #include <linux/of.h>
> >  #include <linux/spinlock.h>
> > +
> > +#ifdef CONFIG_PPC_DCR
> >  #include <asm/dcr.h>
> >  #include <asm/dcr-regs.h>
> > +#endif
> >
> >  /* packet size info */
> >  #define XTE_HDR_SIZE			14      /* size of
Ethernet header */
> > @@ -290,8 +293,12 @@ This option defaults to enabled (set) */
> >
> >  #define TX_CONTROL_CALC_CSUM_MASK   1
> >
> > +/* Align the IP data in the packet on word boundaries as MicroBlaze
> > + * needs it.
> > + */
> > +
> >  #define XTE_ALIGN       32
> > -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
> > +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN)
> =

> Is '34' really XTE_ALIGN + 2?  (I really have no idea.... it just
looks like a suspicious change.)
> =

> Steve

Valid point that it is XTE_ALIGN + 2. As the comment says, it aligns the
IP data in the packet.

-- John

This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.

^ permalink raw reply

* Re: [PATCH] powerpc/perf_events: Implement perf_arch_fetch_caller_regs for powerpc
From: Michael Neuling @ 2010-03-15 17:36 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: Peter Zijlstra, Frederic Weisbecker, linux-kernel, linuxppc-dev,
	anton, Ingo Molnar
In-Reply-To: <20100315054615.GA6245@drongo>

> This implements a powerpc version of perf_arch_fetch_caller_regs.
> It's implemented in assembly because that way we can be sure there
> isn't a stack frame for perf_arch_fetch_caller_regs.  If it was in
> C, gcc might or might not create a stack frame for it, which would
> affect the number of levels we have to skip.  

Should we put this comment in the code as well?

Mikey

> It's not ifdef'd
> because it is only 14 instructions long.
> 
> With this, we see results from perf record -e lock:lock_acquire like
> this:
> 
> # Samples: 24878
> #
> # Overhead         Command      Shared Object  Symbol
> # ........  ..............  .................  ......
> #
>     14.99%            perf  [kernel.kallsyms]  [k] ._raw_spin_lock
>                       |
>                       --- ._raw_spin_lock
>                          |          
>                          |--25.00%-- .alloc_fd
>                          |          (nil)
>                          |          |          
>                          |          |--50.00%-- .anon_inode_getfd
>                          |          |          .sys_perf_event_open
>                          |          |          syscall_exit
>                          |          |          syscall
>                          |          |          create_counter
>                          |          |          __cmd_record
>                          |          |          run_builtin
>                          |          |          main
>                          |          |          0xfd2e704
>                          |          |          0xfd2e8c0
>                          |          |          (nil)
> 
> ... etc.
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>
> ---
>  arch/powerpc/include/asm/asm-compat.h |    2 ++
>  arch/powerpc/kernel/misc.S            |   20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/asm-compat.h b/arch/powerpc/include/asm
/asm-compat.h
> index c1b475a..a9b91ed 100644
> --- a/arch/powerpc/include/asm/asm-compat.h
> +++ b/arch/powerpc/include/asm/asm-compat.h
> @@ -28,6 +28,7 @@
>  #define PPC_LLARX(t, a, b, eh)	PPC_LDARX(t, a, b, eh)
>  #define PPC_STLCX	stringify_in_c(stdcx.)
>  #define PPC_CNTLZL	stringify_in_c(cntlzd)
> +#define PPC_LR_STKOFF	16
>  
>  /* Move to CR, single-entry optimized version. Only available
>   * on POWER4 and later.
> @@ -51,6 +52,7 @@
>  #define PPC_STLCX	stringify_in_c(stwcx.)
>  #define PPC_CNTLZL	stringify_in_c(cntlzw)
>  #define PPC_MTOCRF	stringify_in_c(mtcrf)
> +#define PPC_LR_STKOFF	4
>  
>  #endif
>  
> diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S
> index 2d29752..4459500 100644
> --- a/arch/powerpc/kernel/misc.S
> +++ b/arch/powerpc/kernel/misc.S
> @@ -127,3 +127,23 @@ _GLOBAL(__setup_cpu_power7)
>  _GLOBAL(__restore_cpu_power7)
>  	/* place holder */
>  	blr
> +
> +/*
> + * Get a minimal set of registers for our caller's nth caller.
> + * r3 = regs pointer, r5 = n.
> + */
> +_GLOBAL(perf_arch_fetch_caller_regs)
> +	mr	r6,r1
> +	cmpwi	r5,0
> +	mflr	r4
> +	ble	2f
> +	mtctr	r5
> +1:	PPC_LL	r6,0(r6)
> +	bdnz	1b
> +	PPC_LL	r4,PPC_LR_STKOFF(r6)
> +2:	PPC_LL	r7,0(r6)
> +	PPC_LL	r7,PPC_LR_STKOFF(r7)
> +	PPC_STL	r6,GPR1-STACK_FRAME_OVERHEAD(r3)
> +	PPC_STL	r4,_NIP-STACK_FRAME_OVERHEAD(r3)
> +	PPC_STL	r7,_LINK-STACK_FRAME_OVERHEAD(r3)
> +	blr
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

^ permalink raw reply

* [PATCH 0/4] Some typo fixing
From: Thomas Weber @ 2010-03-15 20:55 UTC (permalink / raw)
  To: linux-kernel
  Cc: Karsten Keil, Lin Ming, Takashi Iwai, Jaroslav Kysela,
	Pavel Machek, David Brownell, linux-acpi, KOSAKI Motohiro,
	Bjorn Helgaas, Rusty Russell, Li Zefan, Steve Conklin,
	Ralph Campbell, Anton Vorontsov, cbe-oss-dev, KAMEZAWA Hiroyuki,
	Liam Girdwood, Anthony Liguori, Jiri Kosina, Randy Dunlap,
	Tejun Heo, Andrew Morton, linux-media, Pieter Palmers,
	Anand Gadiyar, alsa-devel, linux-doc, David Airlie,
	Clemens Ladisch, linux-mm, James E.J. Bottomley,
	Håkan Johansson, linux-s390, Markus Heidelberg, linuxppc-dev,
	Dave Airlie, Tobias Klauser, dri-devel, Hal Rosenstock, Len Brown,
	linux-kbuild, Mauro Carvalho Chehab, Rafael J. Wysocki, Amit Shah,
	Daniel Mack, spi-devel-general, containers, Mark Brown,
	Martin Schwidefsky, linux390, Michal Sojka, Christoph Lameter,
	linux-pci, Heiko Carstens, Jesse Barnes, Kenji Kaneshige,
	Dave Kleikamp, Sean Hefty, Sam Ravnborg, linux-rdma,
	Matthew Wilcox, Ingo Molnar, Jeff Garzik, Alexey Dobriyan,
	Alex Chiang, Mel Gorman, Zhenyu Wang, Pekka Enberg, Jing Huang,
	Jiri Pirko, Greg Kroah-Hartman, Stefan Richter, Jean Delvare,
	Joe Perches, David S. Miller, jfs-discussion, Jay Fenlason,
	Bartlomiej Zolnierkiewicz, Michael S. Tsirkin, Henrik Austad,
	linux-ide, Jonathan Cameron, Bernhard Schiffner, linux1394-devel,
	devel, Amit Kucheria, Douglas Schilling Landgraf, linux-scsi,
	Yu Zhao, André Goddard Rosa, Shirley Ma, Thomas Weber,
	Steve Glendinning, Paul Menage, Alan Cox, Michal Marek,
	Michal Simek, Ben Blum, netdev, Akihiro Tsukada, Johannes Berg,
	Inaky Perez-Gonzalez, Larry Finger

I have fixed some typos.

Thomas Weber (4):
  Fix typo: [Ss]ytem => [Ss]ystem
  Fix typo: udpate => update
  Fix typo: paramters => parameters
  Fix typo: orginal => original

 Documentation/cgroups/cgroups.txt               |    2 +-
 Documentation/kbuild/kconfig.txt                |    2 +-
 Documentation/sysfs-rules.txt                   |    2 +-
 Documentation/trace/events.txt                  |    8 ++++----
 drivers/acpi/osl.c                              |    4 ++--
 drivers/ata/ata_piix.c                          |    2 +-
 drivers/firewire/ohci.c                         |    2 +-
 drivers/gpu/drm/drm_bufs.c                      |    2 +-
 drivers/infiniband/hw/ipath/ipath_iba6110.c     |    2 +-
 drivers/infiniband/hw/ipath/ipath_iba6120.c     |    4 ++--
 drivers/infiniband/hw/ipath/ipath_iba7220.c     |    2 +-
 drivers/isdn/hisax/hfc4s8s_l1.c                 |    2 +-
 drivers/macintosh/windfarm_pm81.c               |    2 +-
 drivers/media/dvb/dvb-usb/friio-fe.c            |    2 +-
 drivers/net/smsc911x.c                          |    4 ++--
 drivers/pci/hotplug/cpqphp_core.c               |    2 +-
 drivers/pci/pci.c                               |    2 +-
 drivers/ps3/ps3-sys-manager.c                   |    2 +-
 drivers/regulator/core.c                        |    2 +-
 drivers/s390/char/sclp_cpi_sys.c                |    2 +-
 drivers/scsi/bfa/include/defs/bfa_defs_cee.h    |    2 +-
 drivers/scsi/bfa/include/defs/bfa_defs_status.h |    4 ++--
 drivers/spi/spi_mpc8xxx.c                       |    2 +-
 drivers/staging/iio/Documentation/overview.txt  |    2 +-
 drivers/staging/rt2860/rtmp.h                   |    2 +-
 drivers/staging/rtl8187se/r8180_core.c          |    4 ++--
 drivers/staging/rtl8187se/r8180_dm.c            |    2 +-
 drivers/staging/rtl8187se/r8185b_init.c         |    2 +-
 drivers/virtio/virtio_pci.c                     |    2 +-
 fs/jfs/jfs_dmap.c                               |    2 +-
 kernel/cgroup.c                                 |    2 +-
 mm/page_alloc.c                                 |    2 +-
 net/wimax/op-rfkill.c                           |    2 +-
 sound/pci/emu10k1/emu10k1_main.c                |    2 +-
 34 files changed, 42 insertions(+), 42 deletions(-)

^ permalink raw reply

* Re: [PATCH] powerpc/perf_events: Implement perf_arch_fetch_caller_regs for powerpc
From: Frederic Weisbecker @ 2010-03-15 21:04 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: Peter Zijlstra, linux-kernel, linuxppc-dev, anton, Ingo Molnar
In-Reply-To: <20100315054615.GA6245@drongo>

On Mon, Mar 15, 2010 at 04:46:15PM +1100, Paul Mackerras wrote:
> This implements a powerpc version of perf_arch_fetch_caller_regs.
> It's implemented in assembly because that way we can be sure there
> isn't a stack frame for perf_arch_fetch_caller_regs.  If it was in
> C, gcc might or might not create a stack frame for it, which would
> affect the number of levels we have to skip.  It's not ifdef'd
> because it is only 14 instructions long.
> 
> With this, we see results from perf record -e lock:lock_acquire like
> this:
> 
> # Samples: 24878
> #
> # Overhead         Command      Shared Object  Symbol
> # ........  ..............  .................  ......
> #
>     14.99%            perf  [kernel.kallsyms]  [k] ._raw_spin_lock
>                       |
>                       --- ._raw_spin_lock
>                          |          
>                          |--25.00%-- .alloc_fd
>                          |          (nil)
>                          |          |          
>                          |          |--50.00%-- .anon_inode_getfd
>                          |          |          .sys_perf_event_open
>                          |          |          syscall_exit
>                          |          |          syscall
>                          |          |          create_counter
>                          |          |          __cmd_record
>                          |          |          run_builtin
>                          |          |          main
>                          |          |          0xfd2e704
>                          |          |          0xfd2e8c0
>                          |          |          (nil)
> 
> ... etc.
> 
> Signed-off-by: Paul Mackerras <paulus@samba.org>


Cool!



> ---
>  arch/powerpc/include/asm/asm-compat.h |    2 ++
>  arch/powerpc/kernel/misc.S            |   20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/asm-compat.h b/arch/powerpc/include/asm/asm-compat.h
> index c1b475a..a9b91ed 100644
> --- a/arch/powerpc/include/asm/asm-compat.h
> +++ b/arch/powerpc/include/asm/asm-compat.h
> @@ -28,6 +28,7 @@
>  #define PPC_LLARX(t, a, b, eh)	PPC_LDARX(t, a, b, eh)
>  #define PPC_STLCX	stringify_in_c(stdcx.)
>  #define PPC_CNTLZL	stringify_in_c(cntlzd)
> +#define PPC_LR_STKOFF	16
>  
>  /* Move to CR, single-entry optimized version. Only available
>   * on POWER4 and later.
> @@ -51,6 +52,7 @@
>  #define PPC_STLCX	stringify_in_c(stwcx.)
>  #define PPC_CNTLZL	stringify_in_c(cntlzw)
>  #define PPC_MTOCRF	stringify_in_c(mtcrf)
> +#define PPC_LR_STKOFF	4
>  
>  #endif
>  
> diff --git a/arch/powerpc/kernel/misc.S b/arch/powerpc/kernel/misc.S
> index 2d29752..4459500 100644
> --- a/arch/powerpc/kernel/misc.S
> +++ b/arch/powerpc/kernel/misc.S
> @@ -127,3 +127,23 @@ _GLOBAL(__setup_cpu_power7)
>  _GLOBAL(__restore_cpu_power7)
>  	/* place holder */
>  	blr
> +
> +/*
> + * Get a minimal set of registers for our caller's nth caller.
> + * r3 = regs pointer, r5 = n.
> + */
> +_GLOBAL(perf_arch_fetch_caller_regs)
> +	mr	r6,r1
> +	cmpwi	r5,0
> +	mflr	r4
> +	ble	2f
> +	mtctr	r5
> +1:	PPC_LL	r6,0(r6)
> +	bdnz	1b
> +	PPC_LL	r4,PPC_LR_STKOFF(r6)
> +2:	PPC_LL	r7,0(r6)
> +	PPC_LL	r7,PPC_LR_STKOFF(r7)
> +	PPC_STL	r6,GPR1-STACK_FRAME_OVERHEAD(r3)
> +	PPC_STL	r4,_NIP-STACK_FRAME_OVERHEAD(r3)
> +	PPC_STL	r7,_LINK-STACK_FRAME_OVERHEAD(r3)
> +	blr

^ permalink raw reply

* Re: [PATCH 0/4] Some typo fixing
From: Randy Dunlap @ 2010-03-15 21:09 UTC (permalink / raw)
  To: Thomas Weber
  Cc: Karsten Keil, Lin Ming, Takashi Iwai, Jaroslav Kysela,
	Pavel Machek, David Brownell, linux-acpi, KOSAKI Motohiro,
	Bjorn Helgaas, Rusty Russell, Li Zefan, Steve Conklin,
	Ralph Campbell, Anton Vorontsov, cbe-oss-dev, KAMEZAWA Hiroyuki,
	Liam Girdwood, Anthony Liguori, Jiri Kosina, linux-kernel,
	Joe Perches, Andrew Morton, linux-media, Pieter Palmers,
	Anand Gadiyar, alsa-devel, linux-doc, David Airlie,
	Clemens Ladisch, linux-mm, James E.J. Bottomley,
	Håkan Johansson, linux-s390, Markus Heidelberg, linuxppc-dev,
	Dave Airlie, Tobias Klauser, dri-devel, Hal Rosenstock, Len Brown,
	linux-kbuild, Mauro Carvalho Chehab, Rafael J. Wysocki, Amit Shah,
	Daniel Mack, spi-devel-general, containers, Mark Brown,
	Martin Schwidefsky, linux390, Michal Sojka, Christoph Lameter,
	linux-pci, Heiko Carstens, Jesse Barnes, Kenji Kaneshige,
	Dave Kleikamp, Sean Hefty, Sam Ravnborg, linux-rdma,
	Matthew Wilcox, Ingo Molnar, Jeff Garzik, Alexey Dobriyan,
	Alex Chiang, Mel Gorman, Zhenyu Wang, Pekka Enberg, Jing Huang,
	Jiri Kosina, Jiri Pirko, Greg Kroah-Hartman, Stefan Richter,
	Jean Delvare, Tejun Heo, David S. Miller, jfs-discussion,
	Jay Fenlason, Bartlomiej Zolnierkiewicz, Michael S. Tsirkin,
	Henrik Austad, linux-ide, Jonathan Cameron, Bernhard Schiffner,
	linux1394-devel, devel, Amit Kucheria, Douglas Schilling Landgraf,
	linux-scsi, Yu Zhao, André Goddard Rosa, Shirley Ma,
	Steve Glendinning, Paul Menage, Alan Cox, Michal Marek,
	Michal Simek, Ben Blum, netdev, Akihiro Tsukada, Johannes Berg,
	Inaky Perez-Gonzalez, Larry Finger
In-Reply-To: <1268686558-28171-1-git-send-email-swirl@gmx.li>

On 03/15/10 13:55, Thomas Weber wrote:
> I have fixed some typos.

Acked-by: Randy Dunlap <rdunlap@xenotime.net>

Jiri, can you merge these, please, unless someone objects (?).


> Thomas Weber (4):
>   Fix typo: [Ss]ytem => [Ss]ystem
>   Fix typo: udpate => update
>   Fix typo: paramters => parameters
>   Fix typo: orginal => original
> 
>  Documentation/cgroups/cgroups.txt               |    2 +-
>  Documentation/kbuild/kconfig.txt                |    2 +-
>  Documentation/sysfs-rules.txt                   |    2 +-
>  Documentation/trace/events.txt                  |    8 ++++----
>  drivers/acpi/osl.c                              |    4 ++--
>  drivers/ata/ata_piix.c                          |    2 +-
>  drivers/firewire/ohci.c                         |    2 +-
>  drivers/gpu/drm/drm_bufs.c                      |    2 +-
>  drivers/infiniband/hw/ipath/ipath_iba6110.c     |    2 +-
>  drivers/infiniband/hw/ipath/ipath_iba6120.c     |    4 ++--
>  drivers/infiniband/hw/ipath/ipath_iba7220.c     |    2 +-
>  drivers/isdn/hisax/hfc4s8s_l1.c                 |    2 +-
>  drivers/macintosh/windfarm_pm81.c               |    2 +-
>  drivers/media/dvb/dvb-usb/friio-fe.c            |    2 +-
>  drivers/net/smsc911x.c                          |    4 ++--
>  drivers/pci/hotplug/cpqphp_core.c               |    2 +-
>  drivers/pci/pci.c                               |    2 +-
>  drivers/ps3/ps3-sys-manager.c                   |    2 +-
>  drivers/regulator/core.c                        |    2 +-
>  drivers/s390/char/sclp_cpi_sys.c                |    2 +-
>  drivers/scsi/bfa/include/defs/bfa_defs_cee.h    |    2 +-
>  drivers/scsi/bfa/include/defs/bfa_defs_status.h |    4 ++--
>  drivers/spi/spi_mpc8xxx.c                       |    2 +-
>  drivers/staging/iio/Documentation/overview.txt  |    2 +-
>  drivers/staging/rt2860/rtmp.h                   |    2 +-
>  drivers/staging/rtl8187se/r8180_core.c          |    4 ++--
>  drivers/staging/rtl8187se/r8180_dm.c            |    2 +-
>  drivers/staging/rtl8187se/r8185b_init.c         |    2 +-
>  drivers/virtio/virtio_pci.c                     |    2 +-
>  fs/jfs/jfs_dmap.c                               |    2 +-
>  kernel/cgroup.c                                 |    2 +-
>  mm/page_alloc.c                                 |    2 +-
>  net/wimax/op-rfkill.c                           |    2 +-
>  sound/pci/emu10k1/emu10k1_main.c                |    2 +-
>  34 files changed, 42 insertions(+), 42 deletions(-)
> 

Thanks.

-- 
~Randy

^ permalink raw reply

* [PATCH] sysdev: the cpu probe/release attributes should be sysdev_class_attributes
From: Stephen Rothwell @ 2010-03-15 23:33 UTC (permalink / raw)
  To: Greg KH; +Cc: Andi Kleen, ppc-dev, LKML

This fixes these warnings:

drivers/base/cpu.c:264: warning: initialization from incompatible pointer type
drivers/base/cpu.c:265: warning: initialization from incompatible pointer type

Cc: Andi Kleen <andi@firstfloor.org>
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 drivers/base/cpu.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

Hi Greg,

Could you pleas send this on to Linuse as this is now a bug in mainline.

Thanks.

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 7036e8e..b5242e1 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -79,24 +79,24 @@ void unregister_cpu(struct cpu *cpu)
 }
 
 #ifdef CONFIG_ARCH_CPU_PROBE_RELEASE
-static ssize_t cpu_probe_store(struct sys_device *dev,
-				struct sysdev_attribute *attr,
-				const char *buf,
+static ssize_t cpu_probe_store(struct sysdev_class *class,
+			       struct sysdev_class_attribute *attr,
+			       const char *buf,
 			       size_t count)
 {
 	return arch_cpu_probe(buf, count);
 }
 
-static ssize_t cpu_release_store(struct sys_device *dev,
-				struct sysdev_attribute *attr,
-				const char *buf,
+static ssize_t cpu_release_store(struct sysdev_class *class,
+				 struct sysdev_class_attribute *attr,
+				 const char *buf,
 				 size_t count)
 {
 	return arch_cpu_release(buf, count);
 }
 
-static SYSDEV_ATTR(probe, S_IWUSR, NULL, cpu_probe_store);
-static SYSDEV_ATTR(release, S_IWUSR, NULL, cpu_release_store);
+static SYSDEV_CLASS_ATTR(probe, S_IWUSR, NULL, cpu_probe_store);
+static SYSDEV_CLASS_ATTR(release, S_IWUSR, NULL, cpu_release_store);
 #endif /* CONFIG_ARCH_CPU_PROBE_RELEASE */
 
 #else /* ... !CONFIG_HOTPLUG_CPU */
-- 
1.7.0

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

^ permalink raw reply related

* Re: [PATCH] sysdev: the cpu probe/release attributes should be sysdev_class_attributes
From: Greg KH @ 2010-03-15 23:46 UTC (permalink / raw)
  To: Stephen Rothwell; +Cc: Andi Kleen, ppc-dev, LKML
In-Reply-To: <20100316103332.ade0b5d9.sfr@canb.auug.org.au>

On Tue, Mar 16, 2010 at 10:33:32AM +1100, Stephen Rothwell wrote:
> This fixes these warnings:
> 
> drivers/base/cpu.c:264: warning: initialization from incompatible pointer type
> drivers/base/cpu.c:265: warning: initialization from incompatible pointer type
> 
> Cc: Andi Kleen <andi@firstfloor.org>
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> ---
>  drivers/base/cpu.c |   16 ++++++++--------
>  1 files changed, 8 insertions(+), 8 deletions(-)
> 
> Hi Greg,
> 
> Could you pleas send this on to Linuse as this is now a bug in mainline.

Will do, thanks so much for fixing this up correctly.

greg k-h

^ permalink raw reply

* Re: [PATCH v4 05/11] swiotlb: add swiotlb_set_default_size()
From: FUJITA Tomonori @ 2010-03-16  1:44 UTC (permalink / raw)
  To: albert_herranz; +Cc: x86, linux-ia64, linux-usb, linuxppc-dev, linux-kernel
In-Reply-To: <1268421166-4791-6-git-send-email-albert_herranz@yahoo.es>

On Fri, 12 Mar 2010 20:12:40 +0100
Albert Herranz <albert_herranz@yahoo.es> wrote:

> The current SWIOTLB code uses a default of 64MB for the IO TLB area.
> This size can be influenced using a kernel command line parameter "swiotlb".
> Unfortunately, the parsing of the kernel command line is done _after_ the
> swiotlb is initialized on some architectures.
> 
> This patch adds a new function swiotlb_set_default_size() which can be used
> before swiotlb_init() to indicate the desired IO TLB area size in bytes.
> 
> This will be used later to implement a smaller IO TLB on the Nintendo Wii
> video game console which just comes with 24MB + 64MB of RAM.
> 
> CC: linuxppc-dev@lists.ozlabs.org
> CC: linux-kernel@vger.kernel.org
> CC: x86@kernel.org
> CC: linux-ia64@vger.kernel.org
> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
> ---
>  include/linux/swiotlb.h |    2 ++
>  lib/swiotlb.c           |   20 ++++++++++++++++++++
>  2 files changed, 22 insertions(+), 0 deletions(-)

Please fix the powerpc swiotlb initialization instead.

Calling swiotlb_init() before parsing kernel parameters sounds
wrong. Any reasons why you can't fix it?

^ permalink raw reply

* Re: [PATCH v4 04/11] swiotlb: support NOT_COHERENT_CACHE PowerPC platforms
From: FUJITA Tomonori @ 2010-03-16  1:54 UTC (permalink / raw)
  To: albert_herranz; +Cc: x86, linux-ia64, linux-usb, linuxppc-dev, linux-kernel
In-Reply-To: <1268421166-4791-5-git-send-email-albert_herranz@yahoo.es>

On Fri, 12 Mar 2010 20:12:39 +0100
Albert Herranz <albert_herranz@yahoo.es> wrote:

> The current SWIOTLB code does not support NOT_COHERENT_CACHE platforms.
> This patch adds support for NOT_COHERENT_CACHE platforms to SWIOTLB by
> adding two platform specific functions swiotlb_dma_sync_page() and
> swiotlb_dma_sync() which can be used to explicitly manage cache coherency.
> 
> On PowerPC these functions are mapped to their corresponding
> __dma_sync_page() and __dma_sync() functions.
> On other architectures using SWIOTLB these functions are optimized out.
> 
> This will be used later to support SWIOTLB on the Nintendo Wii video game
> console.
> 
> CC: linuxppc-dev@lists.ozlabs.org
> CC: linux-kernel@vger.kernel.org
> CC: x86@kernel.org
> CC: linux-ia64@vger.kernel.org
> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
> ---
>  arch/ia64/include/asm/swiotlb.h    |   10 ++++++++++
>  arch/powerpc/include/asm/swiotlb.h |    3 +++
>  arch/x86/include/asm/swiotlb.h     |   10 ++++++++++
>  lib/swiotlb.c                      |   30 ++++++++++++++++++++++++------
>  4 files changed, 47 insertions(+), 6 deletions(-)

Why can't you use dma_sync_single_* instead of inventing new
swiotlb sync functions?

^ permalink raw reply

* Re: [PATCH v4 04/11] swiotlb: support NOT_COHERENT_CACHE PowerPC platforms
From: FUJITA Tomonori @ 2010-03-16  2:25 UTC (permalink / raw)
  To: albert_herranz; +Cc: x86, linux-ia64, linux-usb, linuxppc-dev, linux-kernel
In-Reply-To: <20100316105423B.fujita.tomonori@lab.ntt.co.jp>

On Tue, 16 Mar 2010 10:54:40 +0900
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Fri, 12 Mar 2010 20:12:39 +0100
> Albert Herranz <albert_herranz@yahoo.es> wrote:
> 
> > The current SWIOTLB code does not support NOT_COHERENT_CACHE platforms.
> > This patch adds support for NOT_COHERENT_CACHE platforms to SWIOTLB by
> > adding two platform specific functions swiotlb_dma_sync_page() and
> > swiotlb_dma_sync() which can be used to explicitly manage cache coherency.
> > 
> > On PowerPC these functions are mapped to their corresponding
> > __dma_sync_page() and __dma_sync() functions.
> > On other architectures using SWIOTLB these functions are optimized out.
> > 
> > This will be used later to support SWIOTLB on the Nintendo Wii video game
> > console.
> > 
> > CC: linuxppc-dev@lists.ozlabs.org
> > CC: linux-kernel@vger.kernel.org
> > CC: x86@kernel.org
> > CC: linux-ia64@vger.kernel.org
> > Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
> > ---
> >  arch/ia64/include/asm/swiotlb.h    |   10 ++++++++++
> >  arch/powerpc/include/asm/swiotlb.h |    3 +++
> >  arch/x86/include/asm/swiotlb.h     |   10 ++++++++++
> >  lib/swiotlb.c                      |   30 ++++++++++++++++++++++++------
> >  4 files changed, 47 insertions(+), 6 deletions(-)
> 
> Why can't you use dma_sync_single_* instead of inventing new
> swiotlb sync functions?

If we want to make swiotlb generic (make on any architectures), we
need to handle more cache issues here, I think. So it's better to have
more generic ways instead of adding hooks to some archs.

^ permalink raw reply

* Re: [PATCH] powerpc/perf_events: Implement perf_arch_fetch_caller_regs for powerpc
From: Paul Mackerras @ 2010-03-16  3:22 UTC (permalink / raw)
  To: Frederic Weisbecker
  Cc: Peter Zijlstra, linux-kernel, linuxppc-dev, anton, Ingo Molnar
In-Reply-To: <20100315210450.GF5082@nowhere>

On Mon, Mar 15, 2010 at 10:04:54PM +0100, Frederic Weisbecker wrote:
> On Mon, Mar 15, 2010 at 04:46:15PM +1100, Paul Mackerras wrote:

> >     14.99%            perf  [kernel.kallsyms]  [k] ._raw_spin_lock
> >                       |
> >                       --- ._raw_spin_lock
> >                          |          
> >                          |--25.00%-- .alloc_fd
> >                          |          (nil)
> >                          |          |          
> >                          |          |--50.00%-- .anon_inode_getfd
> >                          |          |          .sys_perf_event_open
> >                          |          |          syscall_exit
> >                          |          |          syscall
> >                          |          |          create_counter
> >                          |          |          __cmd_record
> >                          |          |          run_builtin
> >                          |          |          main
> >                          |          |          0xfd2e704
> >                          |          |          0xfd2e8c0
> >                          |          |          (nil)
> > 
> > ... etc.
> > 
> > Signed-off-by: Paul Mackerras <paulus@samba.org>
> 
> 
> Cool!

By the way, I notice that gcc tends to inline the tracing functions,
which means that by going up 2 stack frames we miss some of the
functions.  For example, for the lock:lock_acquire event, we have
_raw_spin_lock() -> lock_acquire() -> trace_lock_acquire() ->
perf_trace_lock_acquire() -> perf_trace_templ_lock_acquire() ->
perf_fetch_caller_regs() -> perf_arch_fetch_caller_regs().

But in the ppc64 kernel binary I just built, gcc inlined
trace_lock_acquire in lock_acquire, and perf_trace_templ_lock_acquire
in perf_trace_lock_acquire.  Given that perf_fetch_caller_regs is
explicitly inlined, going up two levels from perf_fetch_caller_regs
gets us to _raw_spin_lock, whereas I think you intended it to get us
to trace_lock_acquire.  I'm not sure what to do about that - any
thoughts?

Paul.

^ permalink raw reply

* Linux Priority Levels
From: Ajay Jain @ 2010-03-16  4:06 UTC (permalink / raw)
  To: linuxppc-dev

Hi,

I need a deeper understanding of priorities of linux processes and
threads, especially on Linux PPC. I have done some good reading, but
found no material to be complete and therefore I am raising a few
questions below.

1) In Linux processes have a static priority level 0, with nice values
-20 to +19. Then it has RT priority levels 1 - 99. My question is, do
the nice values apply to all processes or do they apply only to
priority 0?

2) How many total priority levels does the kernel have? On one hand,
-20 is the highest priority, on the other hand +99 is the highest. How
do these values converge?

Please help me to answer these and depending on the reply I would
shoot more questions. In case you have a comprehensive reading
material to refer, please advise.

Thanks,
Ajay.

^ permalink raw reply

* Re: Linux Priority Levels
From: Michael Neuling @ 2010-03-16  4:22 UTC (permalink / raw)
  To: Ajay Jain; +Cc: linuxppc-dev
In-Reply-To: <b92b327b1003152106l4027ae6djb238fa5cdcd29810@mail.gmail.com>

> I need a deeper understanding of priorities of linux processes and
> threads, especially on Linux PPC. I have done some good reading, but
> found no material to be complete and therefore I am raising a few
> questions below.
> 
> 1) In Linux processes have a static priority level 0, with nice values
> -20 to +19. Then it has RT priority levels 1 - 99. My question is, do
> the nice values apply to all processes or do they apply only to
> priority 0?
> 
> 2) How many total priority levels does the kernel have? On one hand,
> -20 is the highest priority, on the other hand +99 is the highest. How
> do these values converge?
> 
> Please help me to answer these and depending on the reply I would
> shoot more questions. In case you have a comprehensive reading
> material to refer, please advise.

man sched_setscheduler has a good description of these levels and how RT
and non RT processes interact.

Mikey

^ permalink raw reply

* Re: Problem with PCI bus rescan on 460EX
From: Kenji Kaneshige @ 2010-03-16  5:40 UTC (permalink / raw)
  To: Felix Radensky; +Cc: linux-pci, Alex Chiang, linuxppc-dev@ozlabs.org
In-Reply-To: <4B9E18A7.2080605@embedded-sol.com>

Felix Radensky wrote:
> Hello, Kenji-san
>>
>> I think the device is expected to be ready to work if pci_enable_device()
>> returns without error. So I think pci_enable_device() should return an
>> error if it fails to enable the device (device is not ready to work). In
>> this case, detecting your bridge's failure seems PPC specific to me. So I
>> thought pcibios_enable_device() was the right to return an error. If
>> pcibios_enable_device() returned an error, pci_dev->enable_cnt would
>> decremented by pci_enable_device() (like pci_disable_device() does) and
>> this problem would not happen.
>>   
> 
> As far as I can see on 460EX pcibios_enable_device() just calls 
> pci_enable_resources()
> which does not return any error for my bridge, although it doesn't find 
> any memory or
> I/O resource it can enable. Do you think it is correct behavior ?
> 
> Another question is whether by bridge behaves correctly when no device 
> is connected
> to it. As you can see in dmesg output I've sent earlier
> 
> pci 0000:00:02.0:   bridge window [mem 0x00000000-0x000fffff]
> pci 0000:00:02.0:   bridge window [mem 0x00000000-0x000fffff 64bit pref]
> 
> and later PCI code disables these memory windows
> 
> pci 0000:00:02.0: disabling bridge window [mem 0xd00000000-0xd000fffff 
> pref] to [bus 01-01] (unused)
> pci 0000:00:02.0: disabling bridge window [mem 0xd00000000-0xd000fffff] 
> to [bus 01-01] (unused)
> 

I misunderstood the problem.
My understanding was memory resource was not enabled even though Linux set
the Memory Space bit in the command register. But it was not correct. The
bridge memory window was marked unused and Linux didn't try to set Memory
Space bit in the command register. Current my understanding is as follows.
Please correct me if I'm still misunderstanding something.

1) Your BIOS doesn't assign any resource to the bridge if its child PCI
   hot-plug slot is not occupied.

2) At the boot time, pci_assign_unassigned_resources() try to assign
   memory resouces to the bridge using pci_bus_assign_resource(), but
   it was disabled because there are no devices require memory resource.

3) And then pci_assign_unassigned_resouces() calls pci_enable_bridge(),
   but Memory Space bit in the command register was not set because no
   memory resource are assigned to the bridge. At the same time,
   pci_dev->enable_cnt was incremented.

4) At the rescan time, pci_setup_bridge() and pci_enable_bridge() doesn't
   work because the bridge is already marked "enabled" (i.e.
   pci_dev->enable_cnt is not zero).

I don't have any concrete idea how to fix that so far, but I can say my idea
(pcibios_enable_device() should return an error) was wrong.

BTW, on my PCI hotplug capable system (SHPC and PCIe), I/O and Memory windows
of the bridge are assigned by BIOS regardless of whether hotplug slot(s)
behind the bridge is occupied or not. Maybe that is the reason why I have
never encountered this problem before.

Thanks,
Kenji Kaneshige



> BTW, there's no problem accessing PCI_COMMAND register, as bus mastering 
> is enabled in the bridge.
> 
>>
>> On the other hand, as Ben suggested, handling this by specific hot-plug
>> driver would be one of the other candidate to fix the problem.
>>
>>
> 
> I'm not opposed to this idea, it's just that this bridge worked in an older
> system based on linux-2.6.22 and patched fakephp driver was used for 
> hotplug.
> There's existing userspace software that I don't really want to modify 
> heavily.
> But I'll do that if generic PCI rescan cannot be fixed.
> 
> Thanks a lot for your help.
> 
> Felix.
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

^ permalink raw reply

* Re: [PATCH v4 05/11] swiotlb: add swiotlb_set_default_size()
From: Albert Herranz @ 2010-03-16  5:58 UTC (permalink / raw)
  To: FUJITA Tomonori, Benjamin Herrenschmidt
  Cc: x86, linux-ia64, linux-usb, linuxppc-dev, linux-kernel
In-Reply-To: <20100316104440Y.fujita.tomonori@lab.ntt.co.jp>

FUJITA Tomonori wrote:
> On Fri, 12 Mar 2010 20:12:40 +0100
> Albert Herranz <albert_herranz@yahoo.es> wrote:
> 
>> The current SWIOTLB code uses a default of 64MB for the IO TLB area.
>> This size can be influenced using a kernel command line parameter "swiotlb".
>> Unfortunately, the parsing of the kernel command line is done _after_ the
>> swiotlb is initialized on some architectures.
>>
>> This patch adds a new function swiotlb_set_default_size() which can be used
>> before swiotlb_init() to indicate the desired IO TLB area size in bytes.
>>
>> This will be used later to implement a smaller IO TLB on the Nintendo Wii
>> video game console which just comes with 24MB + 64MB of RAM.
>>
>> CC: linuxppc-dev@lists.ozlabs.org
>> CC: linux-kernel@vger.kernel.org
>> CC: x86@kernel.org
>> CC: linux-ia64@vger.kernel.org
>> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
>> ---
>>  include/linux/swiotlb.h |    2 ++
>>  lib/swiotlb.c           |   20 ++++++++++++++++++++
>>  2 files changed, 22 insertions(+), 0 deletions(-)
> 
> Please fix the powerpc swiotlb initialization instead.
> 
> Calling swiotlb_init() before parsing kernel parameters sounds
> wrong. Any reasons why you can't fix it?
> 

I think that this would be better asked by a PowerPC maintainer. Ben?

If this is really a problem the swiotlb late init may be a solution too in this particular case.

Thanks,
Albert

^ permalink raw reply

* Re: [PATCH v4 04/11] swiotlb: support NOT_COHERENT_CACHE PowerPC platforms
From: Albert Herranz @ 2010-03-16  6:09 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: x86, linux-ia64, linux-usb, linuxppc-dev, linux-kernel
In-Reply-To: <20100316105423B.fujita.tomonori@lab.ntt.co.jp>

FUJITA Tomonori wrote:
> On Fri, 12 Mar 2010 20:12:39 +0100
> Albert Herranz <albert_herranz@yahoo.es> wrote:
> 
>> The current SWIOTLB code does not support NOT_COHERENT_CACHE platforms.
>> This patch adds support for NOT_COHERENT_CACHE platforms to SWIOTLB by
>> adding two platform specific functions swiotlb_dma_sync_page() and
>> swiotlb_dma_sync() which can be used to explicitly manage cache coherency.
>>
>> On PowerPC these functions are mapped to their corresponding
>> __dma_sync_page() and __dma_sync() functions.
>> On other architectures using SWIOTLB these functions are optimized out.
>>
>> This will be used later to support SWIOTLB on the Nintendo Wii video game
>> console.
>>
>> CC: linuxppc-dev@lists.ozlabs.org
>> CC: linux-kernel@vger.kernel.org
>> CC: x86@kernel.org
>> CC: linux-ia64@vger.kernel.org
>> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
>> ---
>>  arch/ia64/include/asm/swiotlb.h    |   10 ++++++++++
>>  arch/powerpc/include/asm/swiotlb.h |    3 +++
>>  arch/x86/include/asm/swiotlb.h     |   10 ++++++++++
>>  lib/swiotlb.c                      |   30 ++++++++++++++++++++++++------
>>  4 files changed, 47 insertions(+), 6 deletions(-)
> 
> Why can't you use dma_sync_single_* instead of inventing new
> swiotlb sync functions?
> 

At least on PowerPC, the DMA ops are per-device hooks. We attach the swiotlb DMA ops functions to those hooks when we are using swiotlb.
So calling dma_sync_single_*() would end up calling swiotlb_sync_single_*() which is not what we want.

Thanks,
Albert

^ permalink raw reply

* Re: [PATCH v4 04/11] swiotlb: support NOT_COHERENT_CACHE PowerPC platforms
From: Albert Herranz @ 2010-03-16  6:17 UTC (permalink / raw)
  To: FUJITA Tomonori; +Cc: x86, linux-ia64, linux-usb, linuxppc-dev, linux-kernel
In-Reply-To: <20100316112444D.fujita.tomonori@lab.ntt.co.jp>

FUJITA Tomonori wrote:
> If we want to make swiotlb generic (make on any architectures), we
> need to handle more cache issues here, I think. So it's better to have
> more generic ways instead of adding hooks to some archs.
> 

Ok. So what would be an acceptable way of handling this in a generic way?
Should we for example have 2 levels of DMA ops in the swiotlb case (swiotlb and "actual") and call the 2nd level ("actual") from the swiotlb code whenever we need to act on non-bounced buffers?
Any other ideas?

Thanks,
Albert

^ permalink raw reply

* Re: Problem with PCI bus rescan on 460EX
From: Felix Radensky @ 2010-03-16  8:39 UTC (permalink / raw)
  To: Kenji Kaneshige; +Cc: linux-pci, Alex Chiang, linuxppc-dev@ozlabs.org
In-Reply-To: <4B9F19E2.2090709@jp.fujitsu.com>

Hello, Kenji-san

Kenji Kaneshige wrote:
>
> I misunderstood the problem.
> My understanding was memory resource was not enabled even though Linux 
> set
> the Memory Space bit in the command register. But it was not correct. The
> bridge memory window was marked unused and Linux didn't try to set Memory
> Space bit in the command register. Current my understanding is as 
> follows.
> Please correct me if I'm still misunderstanding something.
>
> 1) Your BIOS doesn't assign any resource to the bridge if its child PCI
>   hot-plug slot is not occupied.
>
> 2) At the boot time, pci_assign_unassigned_resources() try to assign
>   memory resouces to the bridge using pci_bus_assign_resource(), but
>   it was disabled because there are no devices require memory resource.
>
> 3) And then pci_assign_unassigned_resouces() calls pci_enable_bridge(),
>   but Memory Space bit in the command register was not set because no
>   memory resource are assigned to the bridge. At the same time,
>   pci_dev->enable_cnt was incremented.
>
> 4) At the rescan time, pci_setup_bridge() and pci_enable_bridge() doesn't
>   work because the bridge is already marked "enabled" (i.e.
>   pci_dev->enable_cnt is not zero).
>
> I don't have any concrete idea how to fix that so far, but I can say 
> my idea
> (pcibios_enable_device() should return an error) was wrong.
>
> BTW, on my PCI hotplug capable system (SHPC and PCIe), I/O and Memory 
> windows
> of the bridge are assigned by BIOS regardless of whether hotplug slot(s)
> behind the bridge is occupied or not. Maybe that is the reason why I have
> never encountered this problem before.
>
> Thanks,
> Kenji Kaneshige
>
>
Yes, your understanding of the problem is correct. On this platform BIOS 
(bootloader, u-boot)
is not required to configure PCI, linux is capable of doing all 
configuration itself. But both u-boot
and linux do not assign memory resources to bridge if there's no device 
behind it.

Thanks a lot.

Felix.

^ permalink raw reply

* Re: [PATCH] irq: move some interrupt arch_* functions into struct irq_chip.
From: Ian Campbell @ 2010-03-16  8:50 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Jeremy Fitzhardinge, x86@kernel.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@ozlabs.org, Ingo Molnar, Paul Mackerras,
	H. Peter Anvin, Thomas Gleixner, Yinghai Lu
In-Reply-To: <m1ocit5aj1.fsf@fess.ebiederm.org>

On Sat, 2010-03-13 at 00:29 +0000, Eric W. Biederman wrote: 
> [...]
> > after that xen could use
> > irq_to_desc_alloc_node_f(irq, node, xen_init_chip_data);
> >
> > as need...
> >
> > at last we don't need to call x86_init_chip_data everywhere.

This was one of the things I was considering. It seems like one of the
easiest solutions to make work correctly with the current locking in
e.g. irq_to_desc_alloc_node since the callback would always happen under
the lock taken in that function.

> So trying to evaluate races.  The worse case for this particular piece
> of code appears to be create_irq_nr.  As this is the only place where
> we are setting up irqs and possibly repurposing the structure.

Yes, create_irq_nr was one of the functions I was struggling to solve
cleanly. There is a similar construct in the Xen code as well.

Part of the problem I'm having is the combination of lookup and allocate
in irq_to_desc_alloc_node but also the kind of "implicit" repurposing is
tricky to deal with. (by implicit I just mean that I can't find where
the previous user explicitly says they are finished with it, if you see
what I mean)

What do you think of adding an explicit free operation for the irq_desc
structs? (does one already exist? I couldn't find it). This would go
along with some tracking of allocation state, trivial in the sparse case
where you can treat a NULL node in the radix tree as unallocated, I
guess a flag would suffice in the static array non-sparse case?

Going further could we split the alloc and lookup functions into
separate operations instead of combining them in irq_to_desc_alloc_node?
We already have irq_to_desc for the lookup portion so this would largely
involve changes to the semantics of irq_to_desc_alloc_node, perhaps
returning ERR_PTR(-EBUSY) if the node was already allocated.

Having a variant which found a free IRQ rather than operating on a
specific requested IRQ could also be useful for create_irq_nr as well as
find_unbound_irq on the Xen side. I'm not convinced irq_alloc_virt on
powerpc isn't implementing broadly the same concept as well, although it
seems to work very differently from the other two.

>   Today
> we figure out if an irq has been assigned by looking at irq_cfg->vector,
> and if it is non-zero the irq has been assigned.

Which is tricky to move into generic code hence my suggestions of
explicitly freeing the irq_desc and tracking the allocation status in
the generic code. 

> The logic in x86_init_chip_data is correct we only assign desc->chip_data
> if the generic layers are above it.  However we need a lock to ensure that
> two paths don't race in that comparison and that assignment.  There is
> no lock in x86_init_chip_data.  Which unfortunately means as it stands
> this patchset is buggy.

Yes, unfortunately I think you are right. The callback idea fixes this.
I'll respin with that.

Ian.

^ permalink raw reply

* Re: [PATCH] irq: move some interrupt arch_* functions into struct irq_chip.
From: Eric W. Biederman @ 2010-03-16  9:18 UTC (permalink / raw)
  To: Ian Campbell
  Cc: Jeremy Fitzhardinge, x86@kernel.org, linux-kernel@vger.kernel.org,
	linuxppc-dev@ozlabs.org, Ingo Molnar, Paul Mackerras,
	H. Peter Anvin, Thomas Gleixner, Yinghai Lu
In-Reply-To: <1268729412.8652.4254.camel@localhost.localdomain>

Ian Campbell <Ian.Campbell@citrix.com> writes:

> On Sat, 2010-03-13 at 00:29 +0000, Eric W. Biederman wrote: 
>> [...]
>> > after that xen could use
>> > irq_to_desc_alloc_node_f(irq, node, xen_init_chip_data);
>> >
>> > as need...
>> >
>> > at last we don't need to call x86_init_chip_data everywhere.
>
> This was one of the things I was considering. It seems like one of the
> easiest solutions to make work correctly with the current locking in
> e.g. irq_to_desc_alloc_node since the callback would always happen under
> the lock taken in that function.
>
>> So trying to evaluate races.  The worse case for this particular piece
>> of code appears to be create_irq_nr.  As this is the only place where
>> we are setting up irqs and possibly repurposing the structure.
>
> Yes, create_irq_nr was one of the functions I was struggling to solve
> cleanly. There is a similar construct in the Xen code as well.
>
> Part of the problem I'm having is the combination of lookup and allocate
> in irq_to_desc_alloc_node but also the kind of "implicit" repurposing is
> tricky to deal with. (by implicit I just mean that I can't find where
> the previous user explicitly says they are finished with it, if you see
> what I mean)
>
> What do you think of adding an explicit free operation for the irq_desc
> structs? (does one already exist? I couldn't find it). This would go
> along with some tracking of allocation state, trivial in the sparse case
> where you can treat a NULL node in the radix tree as unallocated, I
> guess a flag would suffice in the static array non-sparse case?

We have free_one_irq_desc used in the numa irq migration code.
We have dynamic_irq_cleanup.

I don't expect it would be fundamentally hard to put all of the pieces
together.

> Going further could we split the alloc and lookup functions into
> separate operations instead of combining them in irq_to_desc_alloc_node?
> We already have irq_to_desc for the lookup portion so this would largely
> involve changes to the semantics of irq_to_desc_alloc_node, perhaps
> returning ERR_PTR(-EBUSY) if the node was already allocated.
>
> Having a variant which found a free IRQ rather than operating on a
> specific requested IRQ could also be useful for create_irq_nr as well as
> find_unbound_irq on the Xen side. I'm not convinced irq_alloc_virt on
> powerpc isn't implementing broadly the same concept as well, although it
> seems to work very differently from the other two.
>
>>   Today
>> we figure out if an irq has been assigned by looking at irq_cfg->vector,
>> and if it is non-zero the irq has been assigned.
>
> Which is tricky to move into generic code hence my suggestions of
> explicitly freeing the irq_desc and tracking the allocation status in
> the generic code. 

Agreed.  Getting an allocation assist into the generic code sounds
nlike something for the next round of patches after your current one.
At least for msi irqs, hi irqs and other irqs that are totally software
constructs this makes a lot of sense.

We really need to get to the point on x86 where sparse irq has no penalties
and stop making it an option.  Otherwise it gets a bit tricky to rely
on sparse irq for an allocation helper.

I need to go look up my old patchset.  I did something different with
create_irq(), that I think was cleaner and it may be worth resurrecting.
At the very least I generalized the msi case and the ht irq case into
using the same pointer in struct irq_desc.

>> The logic in x86_init_chip_data is correct we only assign desc->chip_data
>> if the generic layers are above it.  However we need a lock to ensure that
>> two paths don't race in that comparison and that assignment.  There is
>> no lock in x86_init_chip_data.  Which unfortunately means as it stands
>> this patchset is buggy.
>
> Yes, unfortunately I think you are right. The callback idea fixes this.
> I'll respin with that.

Thanks.

Eric

^ permalink raw reply


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