LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* _extending_ platform support options?
From: Joachim Foerster @ 2010-12-01 13:25 UTC (permalink / raw)
  To: linuxppc-dev

Hi all,

currently I'm wondering what the preferred/recommend way of _extending_ an existing 
"Platform support" option is?

We are working with custom design/boards based on Virtex4/5. So we are primarily using the 
  CONFIG_XILINX_VIRTEX*_GENERIC_BOARD options. In our case we have some special needs, 
like custom ppc_md.restart(), ppc_md.power_off() or ppc_md.show_cpuinfo().

Till now, we just duplicated arch/powerpc/platforms/4?x/virtex.c and added our special 
stuff. Properly renaming everything, etc ...

An alternative could be to add a virtex_my.c which extends virtex.c, like this
(also like virtex_ml510.c extends virtex.c):

static void virtex_my_show_cpuinfo(struct seq_file *m)
{
	seq_printf(m, something);
}

static int __init virtex_mle_init(void)
{
	ppc_md.show_cpuinfo = virtex_my_show_cpuinfo;
	return 0;
}
machine_core_initcall(virtex, virtex_my_init);

Though, to me, it does not seem really OK to assign ppc_md members that way. The original 
struct machdep for "virtex" (which is defined in virtex.c with define_machine()) is not 
adjusted either. Ok, we could modify that one, too.
Especially I'm not sure if it is OK to use machine_core_initcall() for such modifications.

So my question is: Is there any recommended way for doing such "extensions"? Or is it OK 
to just duplicate virtex.c (which does not seem really OK, too)?

Thanks,
  Joachim

^ permalink raw reply

* Re: Basic driver devel questions ?
From: Michael Ellerman @ 2010-12-01 12:19 UTC (permalink / raw)
  To: Guillaume Dargaud; +Cc: linuxppc-dev
In-Reply-To: <201012011115.00960.dargaud@lpsc.in2p3.fr>

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

On Wed, 2010-12-01 at 11:15 +0100, Guillaume Dargaud wrote:
> Hello all,
> is it OK if I ask basic driver development questions here ?
> Could you recommend a better forum for that maybe ?

Hi Guillaume,

I guess it depends how basic they are :)

If they're basic _powerpc_ driver questions then this is probably the
right place.

But I'd say just ask and maybe someone will be able to help, or maybe
they'll point you somewhere else.

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply

* Basic driver devel questions ?
From: Guillaume Dargaud @ 2010-12-01 10:15 UTC (permalink / raw)
  To: linuxppc-dev

Hello all,
is it OK if I ask basic driver development questions here ?
Could you recommend a better forum for that maybe ?
Thanks
-- 
Guillaume Dargaud
http://www.gdargaud.net/

^ permalink raw reply

* [PATCH] eSPI: change the read behavior of the SPIRF
From: Mingkai Hu @ 2010-12-01  9:29 UTC (permalink / raw)
  To: linuxppc-dev, spi-devel-general; +Cc: kumar.gala, Mingkai Hu

The user must read N bytes of SPIRF (1 <= N <= 4) that do not exceed the
amount of data in the receive FIFO, so read the SPIRF byte by byte when
the data in receive FIFO is less than 4 bytes.

On Simics, when read N bytes that exceed the amout of data in receive
FIFO, we can't read the data out, that is we can't clear the rx FIFO,
then the CPU will loop on the espi rx interrupt.

Signed-off-by: Mingkai Hu <Mingkai.hu@freescale.com>
---
 drivers/spi/spi_fsl_espi.c |   19 ++++++++++++++++---
 1 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi_fsl_espi.c b/drivers/spi/spi_fsl_espi.c
index e3b4f64..ae78926 100644
--- a/drivers/spi/spi_fsl_espi.c
+++ b/drivers/spi/spi_fsl_espi.c
@@ -507,16 +507,29 @@ void fsl_espi_cpu_irq(struct mpc8xxx_spi *mspi, u32 events)
 
 	/* We need handle RX first */
 	if (events & SPIE_NE) {
-		u32 rx_data;
+		u32 rx_data, tmp;
+		u8 rx_data_8;
 
 		/* Spin until RX is done */
 		while (SPIE_RXCNT(events) < min(4, mspi->len)) {
 			cpu_relax();
 			events = mpc8xxx_spi_read_reg(&reg_base->event);
 		}
-		mspi->len -= 4;
 
-		rx_data = mpc8xxx_spi_read_reg(&reg_base->receive);
+		if (mspi->len >= 4) {
+			rx_data = mpc8xxx_spi_read_reg(&reg_base->receive);
+		} else {
+			tmp = mspi->len;
+			rx_data = 0;
+			while (tmp--) {
+				rx_data_8 = in_8((u8 *)&reg_base->receive);
+				rx_data |= (rx_data_8 << (tmp * 8));
+			}
+
+			rx_data <<= (4 - mspi->len) * 8;
+		}
+
+		mspi->len -= 4;
 
 		if (mspi->rx)
 			mspi->get_rx(rx_data, mspi);
-- 
1.7.0.4

^ permalink raw reply related

* Re: [MPC52xx]Latency issue with DMA on FEC
From: Jean-Michel Hautbois @ 2010-12-01  9:59 UTC (permalink / raw)
  To: linuxppc-dev, linux-rt-users; +Cc: Eric Dumazet, Steven Rostedt
In-Reply-To: <AANLkTi=vAEcPKLqJ_yJt7NLz5JoYvEnt8S8QB99ZBNSX@mail.gmail.com>

2010/12/1 Jean-Michel Hautbois <jhautbois@gmail.com>:
> Hi lists !
>
> I measured the latency and the jitter of the RX and TX ethernet paths
> on my MPC5200 board.
> The RX path is quite good, but the TX path can be slow.
>
> [ 1218.976762] [mpc52xx_fec_start_xmit]Delay >30us for dma_map_single
> =3D> 76364 ns
> [ 1219.188405] [mpc52xx_fec_tx_interrupt]Delay >30us for
> dma_unmap_single =3D> 34515 ns
> [ 1220.628785] [mpc52xx_fec_start_xmit]Delay >30us for
> bcom_submit_next_buffer =3D> 97273 ns
> [ 1225.776784] [mpc52xx_fec_tx_interrupt]Delay >30us for
> dma_unmap_single =3D> 95273 ns
>
> As one can see, this is obviously problematic.
> The first function I analyzed is bcom_submit_next_buffer() =3D> This
> function doesn't do lots of things, except a call to mb().
>
> I have been looking to the "MPC603e RISC Microprocessor User's Manual"
> and especially the chapter named "2.3.4.7 Memory Synchronization
> Instructions=E2=80=94UISA".
>
> Here is a paragraph which explains a lot :
>
> "The functions performed by the sync instruction normally take a
> signi=EF=AC=81cant amount of time
> to complete; as a result, frequent use of this instruction may
> adversely affect performance.
> In addition, the number of cycles required to complete a sync
> instruction depends on system
> parameters and on the processor's state when the instruction is issued."
>
> I am using a real time kernel, and this is a problem, as it is not
> deterministic to use this instruction.
> Is there a way to avoid this ?
>
> I will now focus on the dma_map_single() and dma_unmap_single functions..=
.
>
> Thanks in advance for your help,
> Best Regards,
>
> JM
>

dma_map_single() and dma_unmap_single() have the same instruction set
used inside (sync) because there is a cleaning of cache.
eieio instruction doesn't seem to be faster and I think that because
cache is not inhibited, this is not a good way to do that.

The delay introduced by the use of these instructions can be really
big (about 70-90=C2=B5s) whereas in most cases it is relatively good (about
10-20=C2=B5s).
This jitter is a problem in my use case, and I think I am not the only one =
:).

One other thing to say : I am using little packets (about 200 bytes).

JM

^ permalink raw reply

* [PATCH] of/address: use propper endianess in get_flags
From: Sebastian Andrzej Siewior @ 2010-12-01  9:54 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: sodaville

This patch changes u32 to __be32 for all "ranges", "prop" and "addr" and
such. Those variables are pointing to the device tree which containts
intergers in big endian format.
Most functions are doing it right because of_read_number() is doing the
right thing for them. of_bus_isa_get_flags(), of_bus_pci_get_flags() and
of_bus_isa_map() were accessing the data directly and were doing it wrong.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/powerpc/include/asm/prom.h |    2 +-
 drivers/of/address.c            |   54 ++++++++++++++++++++------------------
 include/linux/of_address.h      |    6 ++--
 3 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
index ae26f2e..ab34f60 100644
--- a/arch/powerpc/include/asm/prom.h
+++ b/arch/powerpc/include/asm/prom.h
@@ -42,7 +42,7 @@ extern void pci_create_OF_bus_map(void);
 
 /* Translate a DMA address from device space to CPU space */
 extern u64 of_translate_dma_address(struct device_node *dev,
-				    const u32 *in_addr);
+				    const __be32 *in_addr);
 
 #ifdef CONFIG_PCI
 extern unsigned long pci_address_to_pio(phys_addr_t address);
diff --git a/drivers/of/address.c b/drivers/of/address.c
index 3a1c7e7..b4559c5 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -12,13 +12,13 @@
 			(ns) > 0)
 
 static struct of_bus *of_match_bus(struct device_node *np);
-static int __of_address_to_resource(struct device_node *dev, const u32 *addrp,
-				    u64 size, unsigned int flags,
+static int __of_address_to_resource(struct device_node *dev,
+		const __be32 *addrp, u64 size, unsigned int flags,
 				    struct resource *r);
 
 /* Debug utility */
 #ifdef DEBUG
-static void of_dump_addr(const char *s, const u32 *addr, int na)
+static void of_dump_addr(const char *s, const __be32 *addr, int na)
 {
 	printk(KERN_DEBUG "%s", s);
 	while (na--)
@@ -26,7 +26,7 @@ static void of_dump_addr(const char *s, const u32 *addr, int na)
 	printk("\n");
 }
 #else
-static void of_dump_addr(const char *s, const u32 *addr, int na) { }
+static void of_dump_addr(const char *s, const __be32 *addr, int na) { }
 #endif
 
 /* Callbacks for bus specific translators */
@@ -36,10 +36,10 @@ struct of_bus {
 	int		(*match)(struct device_node *parent);
 	void		(*count_cells)(struct device_node *child,
 				       int *addrc, int *sizec);
-	u64		(*map)(u32 *addr, const u32 *range,
+	u64		(*map)(u32 *addr, const __be32 *range,
 				int na, int ns, int pna);
 	int		(*translate)(u32 *addr, u64 offset, int na);
-	unsigned int	(*get_flags)(const u32 *addr);
+	unsigned int	(*get_flags)(const __be32 *addr);
 };
 
 /*
@@ -55,7 +55,7 @@ static void of_bus_default_count_cells(struct device_node *dev,
 		*sizec = of_n_size_cells(dev);
 }
 
-static u64 of_bus_default_map(u32 *addr, const u32 *range,
+static u64 of_bus_default_map(u32 *addr, const __be32 *range,
 		int na, int ns, int pna)
 {
 	u64 cp, s, da;
@@ -85,7 +85,7 @@ static int of_bus_default_translate(u32 *addr, u64 offset, int na)
 	return 0;
 }
 
-static unsigned int of_bus_default_get_flags(const u32 *addr)
+static unsigned int of_bus_default_get_flags(const __be32 *addr)
 {
 	return IORESOURCE_MEM;
 }
@@ -110,10 +110,10 @@ static void of_bus_pci_count_cells(struct device_node *np,
 		*sizec = 2;
 }
 
-static unsigned int of_bus_pci_get_flags(const u32 *addr)
+static unsigned int of_bus_pci_get_flags(const __be32 *addr)
 {
 	unsigned int flags = 0;
-	u32 w = addr[0];
+	u32 w = be32_to_cpup(addr);
 
 	switch((w >> 24) & 0x03) {
 	case 0x01:
@@ -129,7 +129,8 @@ static unsigned int of_bus_pci_get_flags(const u32 *addr)
 	return flags;
 }
 
-static u64 of_bus_pci_map(u32 *addr, const u32 *range, int na, int ns, int pna)
+static u64 of_bus_pci_map(u32 *addr, const __be32 *range, int na, int ns,
+		int pna)
 {
 	u64 cp, s, da;
 	unsigned int af, rf;
@@ -160,7 +161,7 @@ static int of_bus_pci_translate(u32 *addr, u64 offset, int na)
 	return of_bus_default_translate(addr + 1, offset, na - 1);
 }
 
-const u32 *of_get_pci_address(struct device_node *dev, int bar_no, u64 *size,
+const __be32 *of_get_pci_address(struct device_node *dev, int bar_no, u64 *size,
 			unsigned int *flags)
 {
 	const __be32 *prop;
@@ -207,7 +208,7 @@ EXPORT_SYMBOL(of_get_pci_address);
 int of_pci_address_to_resource(struct device_node *dev, int bar,
 			       struct resource *r)
 {
-	const u32	*addrp;
+	const __be32	*addrp;
 	u64		size;
 	unsigned int	flags;
 
@@ -237,12 +238,13 @@ static void of_bus_isa_count_cells(struct device_node *child,
 		*sizec = 1;
 }
 
-static u64 of_bus_isa_map(u32 *addr, const u32 *range, int na, int ns, int pna)
+static u64 of_bus_isa_map(u32 *addr, const __be32 *range, int na, int ns,
+		int pna)
 {
 	u64 cp, s, da;
 
 	/* Check address type match */
-	if ((addr[0] ^ range[0]) & 0x00000001)
+	if ((addr[0] ^ range[0]) & cpu_to_be32(1))
 		return OF_BAD_ADDR;
 
 	/* Read address values, skipping high cell */
@@ -264,10 +266,10 @@ static int of_bus_isa_translate(u32 *addr, u64 offset, int na)
 	return of_bus_default_translate(addr + 1, offset, na - 1);
 }
 
-static unsigned int of_bus_isa_get_flags(const u32 *addr)
+static unsigned int of_bus_isa_get_flags(const __be32 *addr)
 {
 	unsigned int flags = 0;
-	u32 w = addr[0];
+	u32 w = be32_to_cpup(addr);
 
 	if (w & 1)
 		flags |= IORESOURCE_IO;
@@ -330,7 +332,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
 			    struct of_bus *pbus, u32 *addr,
 			    int na, int ns, int pna, const char *rprop)
 {
-	const u32 *ranges;
+	const __be32 *ranges;
 	unsigned int rlen;
 	int rone;
 	u64 offset = OF_BAD_ADDR;
@@ -398,7 +400,7 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
  * that can be mapped to a cpu physical address). This is not really specified
  * that way, but this is traditionally the way IBM at least do things
  */
-u64 __of_translate_address(struct device_node *dev, const u32 *in_addr,
+u64 __of_translate_address(struct device_node *dev, const __be32 *in_addr,
 			   const char *rprop)
 {
 	struct device_node *parent = NULL;
@@ -475,22 +477,22 @@ u64 __of_translate_address(struct device_node *dev, const u32 *in_addr,
 	return result;
 }
 
-u64 of_translate_address(struct device_node *dev, const u32 *in_addr)
+u64 of_translate_address(struct device_node *dev, const __be32 *in_addr)
 {
 	return __of_translate_address(dev, in_addr, "ranges");
 }
 EXPORT_SYMBOL(of_translate_address);
 
-u64 of_translate_dma_address(struct device_node *dev, const u32 *in_addr)
+u64 of_translate_dma_address(struct device_node *dev, const __be32 *in_addr)
 {
 	return __of_translate_address(dev, in_addr, "dma-ranges");
 }
 EXPORT_SYMBOL(of_translate_dma_address);
 
-const u32 *of_get_address(struct device_node *dev, int index, u64 *size,
+const __be32 *of_get_address(struct device_node *dev, int index, u64 *size,
 		    unsigned int *flags)
 {
-	const u32 *prop;
+	const __be32 *prop;
 	unsigned int psize;
 	struct device_node *parent;
 	struct of_bus *bus;
@@ -525,8 +527,8 @@ const u32 *of_get_address(struct device_node *dev, int index, u64 *size,
 }
 EXPORT_SYMBOL(of_get_address);
 
-static int __of_address_to_resource(struct device_node *dev, const u32 *addrp,
-				    u64 size, unsigned int flags,
+static int __of_address_to_resource(struct device_node *dev,
+		const __be32 *addrp, u64 size, unsigned int flags,
 				    struct resource *r)
 {
 	u64 taddr;
@@ -564,7 +566,7 @@ static int __of_address_to_resource(struct device_node *dev, const u32 *addrp,
 int of_address_to_resource(struct device_node *dev, int index,
 			   struct resource *r)
 {
-	const u32	*addrp;
+	const __be32	*addrp;
 	u64		size;
 	unsigned int	flags;
 
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 8aea06f..2feda6e 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -3,7 +3,7 @@
 #include <linux/ioport.h>
 #include <linux/of.h>
 
-extern u64 of_translate_address(struct device_node *np, const u32 *addr);
+extern u64 of_translate_address(struct device_node *np, const __be32 *addr);
 extern int of_address_to_resource(struct device_node *dev, int index,
 				  struct resource *r);
 extern void __iomem *of_iomap(struct device_node *device, int index);
@@ -21,7 +21,7 @@ static inline unsigned long pci_address_to_pio(phys_addr_t addr) { return -1; }
 #endif
 
 #ifdef CONFIG_PCI
-extern const u32 *of_get_pci_address(struct device_node *dev, int bar_no,
+extern const __be32 *of_get_pci_address(struct device_node *dev, int bar_no,
 			       u64 *size, unsigned int *flags);
 extern int of_pci_address_to_resource(struct device_node *dev, int bar,
 				      struct resource *r);
@@ -32,7 +32,7 @@ static inline int of_pci_address_to_resource(struct device_node *dev, int bar,
 	return -ENOSYS;
 }
 
-static inline const u32 *of_get_pci_address(struct device_node *dev,
+static inline const __be32 *of_get_pci_address(struct device_node *dev,
 		int bar_no, u64 *size, unsigned int *flags)
 {
 	return NULL;
-- 
1.7.3.2

^ permalink raw reply related

* Re: PHY/FEC Network adapter failed to initialize on MPC52xx Board
From: Peter Kuennemann@Crane-Soft @ 2010-12-01  9:06 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <4CF31492.7040809@windriver.com>


Tanks to all for the reply to this thread. I got the tip from the thread Tiejun
mentioned.

Quotation: 'St. Strobel' in Xenomai-help

"I had this problem too. In my case the problem was caused by an incorrect port multiplex configuration in U-Boot, see CONIFG_SYS_GPS_PORT_CONFIG (I use 0x xxx5xxxx, which corresponds to 10/100Mbit
Eth with MD). "

That fixed it for me.

Regards, Peter

Am 29.11.2010 03:48, schrieb tiejun.chen:
> Peter wrote:
>> Hi all
>>
>> I got completely stuck with a network adapter problem on my
>> ppc board (MPC52xx style). The ntwork adapter does not seem
>> to intialize correctly when booted without 'help from uboot'
>>
> Looks your problem is very similar to one I replied here not long ago :) That is
> also issued from MPC5200. And I remember there was a wrong Port Multiplex
> Configuration.
>
> If possible maybe you can check the email subjected "Problem Ethernet
> Initialization MPC5200 +  LXT971A" on linuxppc-dev list.
>
> Hope its useful.
>
> Tiejun
>
>> The adapter works properly when I first use it with uboot. E.g.
>> using tftp to load the kernel or just issuing a dummy sntp
>> command. It does not get intialized if I boot linux without
>> using any network relevant command in ubboot
>>
>> The difference manifests on the boot message: (working)
>> PHY working
>> ...
>> mpc52xx MII bus: probed
>> TCP cubic registered
>> NET: Registered protocol family 17
>> IP-Config: Complete:
>>      device=eth0, addr=192.168.1.245, mask=255.254.0.0, gw=192.168.1.2,
>>      host=192.168.1.245, domain=, nis-domain=(none),
>>      bootserver=192.168.1.244, rootserver=192.168.1.244, rootpath=
>> Looking up port of RPC 100003/2 on 192.168.1.244
>> Looking up port of RPC 100005/1 on 192.168.1.244
>> VFS: Mounted root (nfs filesystem) on device 0:11.
>> Freeing unused kernel memory: 124k init
>> PHY: f0003000:00 - Link is Up - 100/Full
>>
>> # ping 192.168.1.2  returns proper results.
>>
>> PHY Not working:
>> ...
>> mpc52xx MII bus: probed
>> TCP cubic registered
>> NET: Registered protocol family 17
>> IP-Config: Complete:
>>      device=eth0, addr=192.168.1.245, mask=255.254.0.0, gw=192.168.1.2,
>>      host=192.168.1.245, domain=, nis-domain=(none),
>>      bootserver=192.168.1.244, rootserver=192.168.1.244, rootpath=
>> VFS: Mounted root (squashfs filesystem) readonly on device 31:3.
>> Freeing unused kernel memory: 124k init
>>
>> # ping 192.168.1.2  hangs
>>
>>
>> The second snipped does not have "Looking up.." messages because it
>> boots from flash. Main difference is "PHY: f0003000:00 - Link is Up - 100/Full"
>> which does not appear at the failing case.
>>
>> Linux Version is 2.6.35.7 patched with xenomai 2.5
>> U-Boot 2010.06 (Aug 05 2010 - 19:54:45)
>>
>> Linux configuration see below: ( i left most entries out that are not set)
>> I also experimented with different settings but finally only
>> CONFIG_FEC_MPC52xx=y  and CONFIG_FEC_MPC52xx_MDIO=y
>> seem to be of any relevance. If both are set, the adapter works
>> when initialized by uboot.
>>
>> Any help or tips will be very much appreciated,
>>
>> Regards, Peter
>>
>>
>> Linux .config
>>
>> ...
>> #
>> # Platform support
>> #
>> # CONFIG_PPC_CHRP is not set
>> # CONFIG_MPC5121_ADS is not set
>> # CONFIG_MPC5121_GENERIC is not set
>> CONFIG_PPC_MPC52xx=y
>> CONFIG_PPC_MPC5200_SIMPLE=y
>> # CONFIG_PPC_EFIKA is not set
>> CONFIG_PPC_LITE5200=y
>> # CONFIG_PPC_MEDIA5200 is not set
>> CONFIG_PPC_MPC5200_BUGFIX=y
>> # CONFIG_PPC_MPC5200_GPIO is not set
>> CONFIG_PPC_MPC5200_LPBFIFO=y
>>
>> CONFIG_PPC_BESTCOMM=y
>> CONFIG_PPC_BESTCOMM_FEC=y
>> CONFIG_PPC_BESTCOMM_GEN_BD=y
>> # CONFIG_SIMPLE_GPIO is not set
>> ..
>> # Bus options
>> #
>> CONFIG_ZONE_DMA=y
>> CONFIG_NEED_SG_DMA_LENGTH=y
>> CONFIG_GENERIC_ISA_DMA=y
>> CONFIG_PPC_PCI_CHOICE=y
>> ...
>> #
>> # Generic Driver Options
>> #
>> CONFIG_STANDALONE=y
>> CONFIG_PREVENT_FIRMWARE_BUILD=y
>> CONFIG_MTD=y
>> CONFIG_MTD_PARTITIONS=y
>> CONFIG_MTD_CMDLINE_PARTS=y
>>
>> #
>> # MII PHY device drivers
>> #
>> CONFIG_LXT_PHY=y                        ## Does not seem to have any influence
>> CONFIG_NET_ETHERNET=y
>> CONFIG_MII=y
>> CONFIG_ETHOC=y                           ## Does not seem to have any influence
>> CONFIG_FEC_MPC52xx=y              ## Must be Y in roder to get adapter working with uboot's init
>> CONFIG_FEC_MPC52xx_MDIO=y  ## Must be Y in roder to get adapter working with uboot's init
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* [MPC52xx]Latency issue with DMA on FEC
From: Jean-Michel Hautbois @ 2010-12-01  8:16 UTC (permalink / raw)
  To: linuxppc-dev, linux-rt-users; +Cc: Eric Dumazet, Steven Rostedt

Hi lists !

I measured the latency and the jitter of the RX and TX ethernet paths
on my MPC5200 board.
The RX path is quite good, but the TX path can be slow.

[ 1218.976762] [mpc52xx_fec_start_xmit]Delay >30us for dma_map_single
=3D> 76364 ns
[ 1219.188405] [mpc52xx_fec_tx_interrupt]Delay >30us for
dma_unmap_single =3D> 34515 ns
[ 1220.628785] [mpc52xx_fec_start_xmit]Delay >30us for
bcom_submit_next_buffer =3D> 97273 ns
[ 1225.776784] [mpc52xx_fec_tx_interrupt]Delay >30us for
dma_unmap_single =3D> 95273 ns

As one can see, this is obviously problematic.
The first function I analyzed is bcom_submit_next_buffer() =3D> This
function doesn't do lots of things, except a call to mb().

I have been looking to the "MPC603e RISC Microprocessor User's Manual"
and especially the chapter named "2.3.4.7 Memory Synchronization
Instructions=E2=80=94UISA".

Here is a paragraph which explains a lot :

"The functions performed by the sync instruction normally take a
signi=EF=AC=81cant amount of time
to complete; as a result, frequent use of this instruction may
adversely affect performance.
In addition, the number of cycles required to complete a sync
instruction depends on system
parameters and on the processor's state when the instruction is issued."

I am using a real time kernel, and this is a problem, as it is not
deterministic to use this instruction.
Is there a way to avoid this ?

I will now focus on the dma_map_single() and dma_unmap_single functions...

Thanks in advance for your help,
Best Regards,

JM

^ permalink raw reply

* Re: [PATCH 01/13] powerpc/nvram: Move things out of asm/nvram.h
From: Jim Keniston @ 2010-12-01  7:24 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev, jkensito
In-Reply-To: <1291096063-14386-1-git-send-email-benh@kernel.crashing.org>

On Tue, 2010-11-30 at 16:47 +1100, benh@kernel.crashing.org wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> This moves a bunch of definitions out of asm/nvram.h to the files
> that use them or just outright remove completely unused stuff.
> 
> We leave the partition signatures definitions, they will be useful
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

I applied the whole 13-patch set, and built and booted the result.
Seems fine.

Tested-by: Jim Keniston <jkenisto@us.ibm.com>

Jim

^ permalink raw reply

* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support
From: Stephen Boyd @ 2010-12-01  5:30 UTC (permalink / raw)
  To: Daniel Walker
  Cc: Randy Dunlap, Mike Frysinger, Arnd Bergmann, Nicolas Pitre,
	Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman, linux-kernel,
	FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox
In-Reply-To: <1291145141-18301-1-git-send-email-dwalker@codeaurora.org>

On 11/30/2010 11:25 AM, Daniel Walker wrote:
> @@ -682,6 +682,15 @@ config HVC_UDBG
>         select HVC_DRIVER
>         default n
>  
> +config HVC_DCC
> +       bool "ARM JTAG DCC console"
> +       depends on ARM
> +       select HVC_DRIVER
> +       help
> +         This console uses the JTAG DCC on ARM to create a console under the HVC

Looks like you added one too many spaces for indent here.

> diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c
> new file mode 100644
> index 0000000..6470f63
> --- /dev/null
> +++ b/drivers/char/hvc_dcc.c
> +static inline u32 __dcc_getstatus(void)
> +{
> +	u32 __ret;
> +
> +	asm("mrc p14, 0, %0, c0, c1, 0	@ read comms ctrl reg"
> +		: "=r" (__ret) : : "cc");
> +
> +	return __ret;
> +}

Without marking this asm volatile my compiler decides it can cache the
value of __ret in a register and then check the value of it continually
in hvc_dcc_put_chars() (I had to replace get_wait/put_wait with 1 and
fixup the branch otherwise my disassembler barfed on __dcc_(get|put)char).


00000000 <hvc_dcc_put_chars>:
   0:   ee103e11        mrc     14, 0, r3, cr0, cr1, {0}
   4:   e3a0c000        mov     ip, #0  ; 0x0
   8:   e2033202        and     r3, r3, #536870912      ; 0x20000000
   c:   ea000006        b       2c <hvc_dcc_put_chars+0x2c>
  10:   e3530000        cmp     r3, #0  ; 0x0
  14:   1afffffd        bne     10 <hvc_dcc_put_chars+0x10>
  18:   e7d1000c        ldrb    r0, [r1, ip]
  1c:   ee10fe11        mrc     14, 0, pc, cr0, cr1, {0}
  20:   2afffffd        bcs     1c <hvc_dcc_put_chars+0x1c>
  24:   ee000e15        mcr     14, 0, r0, cr0, cr5, {0}
  28:   e28cc001        add     ip, ip, #1      ; 0x1
  2c:   e15c0002        cmp     ip, r2
  30:   bafffff6        blt     10 <hvc_dcc_put_chars+0x10>
  34:   e1a00002        mov     r0, r2
  38:   e12fff1e        bx      lr

As you can see, the value of the mrc is checked against DCC_STATUS_TX
(bit 29) and then stored in r3 for later use. Marking this volatile
produces the following:

00000000 <hvc_dcc_put_chars>:
   0:   e3a03000        mov     r3, #0  ; 0x0
   4:   ea000007        b       28 <hvc_dcc_put_chars+0x28>
   8:   ee100e11        mrc     14, 0, r0, cr0, cr1, {0}
   c:   e3100202        tst     r0, #536870912  ; 0x20000000
  10:   1afffffc        bne     8 <hvc_dcc_put_chars+0x8>
  14:   e7d10003        ldrb    r0, [r1, r3]
  18:   ee10fe11        mrc     14, 0, pc, cr0, cr1, {0}
  1c:   2afffffd        bcs     18 <hvc_dcc_put_chars+0x18>
  20:   ee000e15        mcr     14, 0, r0, cr0, cr5, {0}
  24:   e2833001        add     r3, r3, #1      ; 0x1
  28:   e1530002        cmp     r3, r2
  2c:   bafffff5        blt     8 <hvc_dcc_put_chars+0x8>
  30:   e1a00002        mov     r0, r2
  34:   e12fff1e        bx      lr

which looks better.

I marked all the asm in this driver as volatile. Is that correct?

> +#if defined(CONFIG_CPU_V7)
> +static inline char __dcc_getchar(void)
> +{
> +	char __c;
> +
> +	asm("get_wait:	mrc p14, 0, pc, c0, c1, 0                          \n\
> +			bne get_wait                                       \n\
> +			mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
> +		: "=r" (__c) : : "cc");
> +
> +	return __c;
> +}
> +#else
> +static inline char __dcc_getchar(void)
> +{
> +	char __c;
> +
> +	asm("mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
> +		: "=r" (__c));
> +
> +	return __c;
> +}
> +#endif
> +
> +#if defined(CONFIG_CPU_V7)
> +static inline void __dcc_putchar(char c)
> +{
> +	asm("put_wait:	mrc p14, 0, pc, c0, c1, 0                 \n\
> +			bcs put_wait                              \n\
> +			mcr p14, 0, %0, c0, c5, 0                   "
> +	: : "r" (c) : "cc");
> +}
> +#else
> +static inline void __dcc_putchar(char c)
> +{
> +	asm("mcr p14, 0, %0, c0, c5, 0	@ write a char"
> +		: /* no output register */
> +		: "r" (c));
> +}
> +#endif
> +

I don't think both the v7 and v6 functions are necessary. It seems I can
get away with just the second version of __dcc_(get|put)char() on a v7.
The mrc p14, 0, pc, c0, c1, 0 will assign the top 4 bits (31-28) to the
condition codes NZCV on v7. It also looks like on an ARM11 (a v6) will
also do the same thing if I read the manuals right. The test in the
inline assembly is saying, wait for a character to be ready or wait for
a character to be read then actually write a character or read one. The
code in hvc_dcc_put_chars() is already doing the same thing, albeit in a
slightly different form. Instead of getting the status bits put into the
condition codes and looping with bne or bcs it will read the register,
and it with bit 29 or bit 28 to see if it should wait and then continue
with the writing/reading. I think you can just drop the looping for the
v7 version of the functions and have this driver work on v6 and v7.
Alternatively, you can make some function that says tx buffer is empty,
rx buffer is full or something but I don't see how saving a couple
instructions buys us much when we can have one driver for v6 and v7.

I see that Tony Lindgren modified the DCC macros for v7 in commit
200b7a8 (ARM: 5884/1: arm: Fix DCC console for v7, 2010-01-19). I'm not
sure why though, since it seems that a v6 and a v7 should really do the
same thing by waiting for the buffers to be ready before filling them or
reading them. Which probably means we can get low-level dcc debugging on
all targets if I'm not mistaken.

> +static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count)
> +{
> +	int i;
> +
> +	for (i = 0; i < count; i++) {
> +		while (__dcc_getstatus() & DCC_STATUS_TX)
> +			cpu_relax();
> +
> +		__dcc_putchar((char)(buf[i] & 0xFF));

Is this & 0xFF and cast to char unnecessary? buf is a char array, and
chars are always 8 bits. Can't we just do __dcc_putchar(buf[i])?

> +static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count)
> +{
> +	int i;
> +
> +	for (i = 0; i < count; ++i) {
> +		int c = -1;
> +
> +		if (__dcc_getstatus() & DCC_STATUS_RX)
> +			c = __dcc_getchar();
> +		if (c < 0)
> +			break;
> +		buf[i] = c;
> +	}

I think this for loop can be simplified. __dcc_getchar() returns a char.
It never returns -1, so the check for c < 0 can't be taken if
__dcc_getstatus() & DCC_STATUS_RX is true. The only case you break the
loop in then is if __dcc_getstatus() & DCC_STATUS_RX is false. So you
can have a simple if-else and assign buf[i] in the if branch and break
in the else branch.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

^ permalink raw reply

* Re: ppc_set_hwdebug vs ptrace_set_debugreg
From: K.Prasad @ 2010-12-01  4:37 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: linuxppc-dev, Dave Kleikamp, Srikar Dronamraju
In-Reply-To: <m38w0c77vs.fsf@linux-m68k.org>

On Mon, Nov 29, 2010 at 11:15:51AM +0100, Andreas Schwab wrote:
> "K.Prasad" <prasad@linux.vnet.ibm.com> writes:
> 
> > Although ppc_set_hwdebug() can set DABR through set_dabr() in
> > arch/powerpc/kernel/process.c, it is good to have it converted to use
> > register_user_hw_breakpoint().
> 
> What do you mean with "good to have"?  It doesn't work without it unless
> I disable PERF_EVENTS (which is the only way to disable
> HAVE_HW_BREAKPOINT).
> 
> Andreas.
>

Let me see if I can cook up a patch for this i.e. make set_dabr() invoke
register_user_hw_breakpoint() when CONFIG_PPC_BOOK3S is defined; before I
head out on my vacation (starting second week of this month).

Thanks,
K.Prasad
 

^ permalink raw reply

* Re: [PATCH 14/15] ppc64 iommu: use coherent_dma_mask for alloc_coherent
From: Nishanth Aravamudan @ 2010-12-01  0:31 UTC (permalink / raw)
  To: Benjamin Herrenschmidt; +Cc: linuxppc-dev, Paul Mackerras, Milton Miller
In-Reply-To: <1290992296.32570.215.camel@pasglop>

On 29.11.2010 [11:58:16 +1100], Benjamin Herrenschmidt wrote:
> On Wed, 2010-09-15 at 11:05 -0700, Nishanth Aravamudan wrote:
> > The IOMMU code has been passing the dma-mask instead of the
> > coherent_dma_mask to the iommu allocator.  Coherent allocations should
> > be made using the coherent_dma_mask.
> 
> Won't that break macio devices too ? afaik, they don't set
> coherent_dma_mask. Have you tried booting on a G5 with iommu enabled ?
> 
> (It may not be broken, just asking...)

I have not tried this. I unfortunately do not have immediate access to a
G5 to test on, but will ask around.

Thanks,
Nish

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

^ permalink raw reply

* Re: [RFC PATCH 3/7 v2] ppc: do not search for dma-window property on dlpar remove
From: Nishanth Aravamudan @ 2010-12-01  0:30 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: sonnyrao, miltonm, Paul Mackerras, Anton Blanchard, linuxppc-dev
In-Reply-To: <1290994721.32570.219.camel@pasglop>

On 29.11.2010 [12:38:41 +1100], Benjamin Herrenschmidt wrote:
> On Tue, 2010-10-26 at 20:35 -0700, Nishanth Aravamudan wrote:
> > The iommu_table pointer in the pci auxiliary struct of device_node has
> > not been used by the iommu ops since the dma refactor of
> > 12d04eef927bf61328af2c7cbe756c96f98ac3bf, however this code still uses
> > it to find tables for dlpar. By only setting the PCI_DN iommu_table
> > pointer on nodes with dma window properties, we will be able to quickly
> > find the node for later checks, and can remove the table without looking
> > for the the dma window property on dlpar remove.
> 
> The answer might well be yes but are we sure this works with busses &
> devices that don't have a dma,window ? ie. we always properly look for
> parents when assigning pci devices arch_data iommu table ? Did you test
> it ? :-) (Best way is to find a card with a P2P bridge on it).

I haven't tested this particular case. I'm getting a machine to do so
now, though.

Thanks,
Nish

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

^ permalink raw reply

* Re: [PATCH v2] PPC4xx: Adding PCI(E) MSI support
From: Tirumala Marri @ 2010-11-30 23:07 UTC (permalink / raw)
  To: michael; +Cc: linuxppc-dev
In-Reply-To: <1291080609.2648.102.camel@concordia>

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

On Mon, Nov 29, 2
>
> >
> > My apologies in the delay here.  I was on holiday for a while and never
> > got back to review this.  A few notes below.
> >
> > Also, I've added a few patches from Victor for suspend/idle support in
> > my next branch that cause a minor conflict with this one.  It's not a
> > big deal to fix, but if you rework the patch for the comments, rebasing
> > it to my next branch would be appreciated.
>
Sure thanks for reviewing. I will fix the suggested changes and send the
updated patch.

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

^ permalink raw reply

* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support
From: Arnd Bergmann @ 2010-11-30 21:17 UTC (permalink / raw)
  To: Nicolas Pitre
  Cc: Randy Dunlap, Daniel Walker, Mike Frysinger, Tony Lindgren,
	linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori,
	Andrew Morton, linuxppc-dev, Alan Cox
In-Reply-To: <alpine.LFD.2.00.1011301455400.9313@xanadu.home>

On Tuesday 30 November 2010, Nicolas Pitre wrote:
> > Cc: Tony Lindgren <tony@atomide.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Nicolas Pitre <nico@fluxnic.net>
> > Cc: Greg Kroah-Hartman <gregkh@suse.de>
> > Cc: Mike Frysinger <vapier@gentoo.org>
> > Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
> 
> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

Acked-by: Arnd Bergmann <arnd@arndb.de>

> This doesn't support both ARMv6 and ARMv7 at run time, but this can 
> trivially be added later when needed.

I was about to make a similar comment when I saw yours ;-)

^ permalink raw reply

* Re: [PATCH] drivers: char: hvc: add arm JTAG DCC console support
From: Nicolas Pitre @ 2010-11-30 19:57 UTC (permalink / raw)
  To: Daniel Walker
  Cc: Randy Dunlap, Mike Frysinger, Arnd Bergmann, Tony Lindgren,
	linux-arm-msm, Greg Kroah-Hartman, linux-kernel, FUJITA Tomonori,
	Andrew Morton, linuxppc-dev, Alan Cox
In-Reply-To: <1291145141-18301-1-git-send-email-dwalker@codeaurora.org>

On Tue, 30 Nov 2010, Daniel Walker wrote:

> This driver adds a basic console that uses the arm JTAG
> DCC to transfer data back and forth. It has support for
> ARMv6 and ARMv7.
> 
> This console is created under the HVC driver, and should be named
> /dev/hvcX (or /dev/hvc0 for example).
> 
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Nicolas Pitre <nico@fluxnic.net>
> Cc: Greg Kroah-Hartman <gregkh@suse.de>
> Cc: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Daniel Walker <dwalker@codeaurora.org>

Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

This doesn't support both ARMv6 and ARMv7 at run time, but this can 
trivially be added later when needed.


Nicolas

^ permalink raw reply

* Re: [PATCH] powerpc: fix call to subpage_protection()
From: Jim Keniston @ 2010-11-30 20:55 UTC (permalink / raw)
  To: Milton Miller; +Cc: Michael Neuling, linuxppc-dev
In-Reply-To: <mdm-initramfs-debug-fix@mdm.bga.com>

On Tue, 2010-11-30 at 05:08 -0600, Milton Miller wrote:
> [fixed Michael Neuling's address]
> 
> On Fri Nov 19 about 07:06:19 EST in 2010, Benjamin Herrenschmidt wrote:
> > On Thu, 2010-11-18 at 10:23 -0800, Jim Keniston wrote:
> > > FWIW, this failure isn't an obstacle for me.  I'm in no way attached to
> > > my legacy configuration; pseries_defconfig is fine for me.  On the other
> > > hand, I can continue testing fixes, and/or make my system available to
> > > other IBMers when I'm not using it, if you want to continue to pursue
> > > this problem.
> 
> > From the look of it your "legacy" config is lacking the necessary
> > storage drivers...
> > 
> 
> Looking closer, it would appear he intended to get them from initramfs
> but unpacking it failed.  Pulling from his log in the original message:

Bingo!  Not only was I using a "legacy" .config file, I'd also copied
and pasted an older yaboot.conf entry without updating the initrd-size
clause.  The initrd-size value was apparently big enough until I enabled
the extra DEBUG code.  Mr. Miller's diagnostic message reported the
unpacking failure at just a few bytes past the specified initrd-size.
Adjusting the initrd-size to match the actual size of the file enables
the kernel to boot.

So... cockpit error.  Very sorry for the fuss, but thanks for the
detective work.

As for Mr. Miller's patch...
Tested-by: Jim Keniston <jkenisto@us.ibm.com>

Jim

> 
> > > 
> > > Calling ibm,client-architecture-support... not implemented
> > > command line: root=/dev/disk/by-id/scsi-SIBM_ST373453LC_3HW1CQ1400007445Q2A4-part3 quiet profile=2 sysrq=1 insmod=sym53c8xx insmod=ipr crashkernel=256M-:128M loglevel=8 
> > > 
> > > memory layout at init:
> > >   memory_limit : 0000000000000000 (16 MB aligned)
> > >   alloc_bottom : 0000000003550000
> > >   alloc_top    : 0000000008000000
> > >   alloc_top_hi : 0000000070000000
> > >   amo_top      : 0000000008000000
> > >   ram_top      : 0000000070000000
> > > instantiating rtas at 0x00000000076a0000... done
> > > boot cpu hw idx 0
> > > starting cpu hw idx 2... done
> > > copying OF device tree...
> > > Building dt strings...
> > > Building dt structure...
> > > Device tree strings 0x0000000003560000 -> 0x000000000356129c
> > > Device tree struct  0x0000000003570000 -> 0x0000000003580000
> ..
> > > Found initrd at 0xc000000002d00000:0xc00000000354e28a
> ..
> > > NET: Registered protocol family 1
> > > PCI: CLS 128 bytes, default 128
> > > Unpacking initramfs...
> > > Initramfs unpacking failed: read error
> 
> "read error" occurs in lib/decompress_inflate.c if the fill routine
> returns a negative count, which would inlclude a NULL fill pointer like
> init/initramfs.c.
> 
> 
> Maybe this debugging patch (against 2.6.35, applys with small offset to 
> 2.6.37-rc4) can help isolate the corruption?
> 
> From: Milton Miller <miltonm.bga.com>
> initramfs: show input and output offsets on unpacking failures
> 
> When the initramfs input is corrupted, having the pointer to the buffer
> segment being decompressed, and the decompress input and output byte
> counts from the compressed stream can help isolate the source of the
> corruption.
> 
> Signed-off-by: Milton Miller <miltonm@bga.com>
> 
> ---
>  init/initramfs.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> Index: sim/init/initramfs.c
> ===================================================================
> --- sim.orig/init/initramfs.c	2009-07-13 18:44:56.000000000 -0500
> +++ sim/init/initramfs.c	2009-07-13 18:59:32.000000000 -0500
> @@ -384,6 +384,9 @@ static int __init write_buffer(char *buf
>  	return len - count;
>  }
> 
> +static unsigned my_inptr;   /* index of next byte to be processed in inbuf */
> +static unsigned bytes_out;  /* count of chars processed from current archive */
> +
>  static int __init flush_buffer(void *bufv, unsigned len)
>  {
>  	char *buf = (char *) bufv;
> @@ -393,6 +396,7 @@ static int __init flush_buffer(void *buf
>  		return -1;
>  	while ((written = write_buffer(buf, len)) < len && !message) {
>  		char c = buf[written];
> +		bytes_out += written;
>  		if (c == '0') {
>  			buf += written;
>  			len -= written;
> @@ -407,8 +411,6 @@ static int __init flush_buffer(void *buf
>  	return origLen;
>  }
> 
> -static unsigned my_inptr;   /* index of next byte to be processed in inbuf */
> -
>  #include <linux/decompress/generic.h>
> 
>  static char * __init unpack_to_rootfs(char *buf, unsigned len)
> @@ -430,6 +432,8 @@ static char * __init unpack_to_rootfs(ch
>  	message = NULL;
>  	while (!message && len) {
>  		loff_t saved_offset = this_header;
> +		bytes_out = 0;
> +		my_inptr = 0;
>  		if (*buf == '0' && !(this_header & 3)) {
>  			state = Start;
>  			written = write_buffer(buf, len);
> @@ -462,6 +466,9 @@ static char * __init unpack_to_rootfs(ch
>  		buf += my_inptr;
>  		len -= my_inptr;
>  	}
> +	if (message) {
> +		printk(KERN_ERR "Failed unpacking at %p input %d output %d \n",buf - my_inptr, my_inptr, bytes_out);
> +	}
>  	dir_utime();
>  	kfree(name_buf);
>  	kfree(symlink_buf);
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* RE: [PATCH 2/2][v3] rapidio, powerpc/85xx: Error interrupt handler for sRIO.
From: Bounine, Alexandre @ 2010-11-30 20:48 UTC (permalink / raw)
  To: Shaohui Xie, linuxppc-dev; +Cc: akpm, Kumar Gala
In-Reply-To: <1290063473-20950-1-git-send-email-b21989@freescale.com>

Applies correctly now. Lab tested on 8548/RIO setup.

Alex.=20

> -----Original Message-----
> From: Shaohui Xie [mailto:b21989@freescale.com]
> Sent: Thursday, November 18, 2010 1:58 AM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: akpm@linux-foundation.org; Shaohui Xie; Li Yang; Kumar Gala; Roy
Zang; Bounine, Alexandre
> Subject: [PATCH 2/2][v3] rapidio, powerpc/85xx: Error interrupt
handler for sRIO.
>=20
> The sRIO controller reports errors to the core with one signal, it
uses
> register EPWISR to provides the core quick access to where the error
occurred.
> The EPWISR indicates that there are 4 interrupts sources, port1,
port2, message
> unit and port write receive, but the sRIO driver does not support
port2
> for now, still the handler takes care of port2.
> Currently the handler only clear error status without any recovery.

^ permalink raw reply

* RE: [PATCH 1/2][v4] fsl_rio: move machine_check handler into machine_check_e500 & machine_check_e500mc
From: Bounine, Alexandre @ 2010-11-30 20:47 UTC (permalink / raw)
  To: Shaohui Xie, linuxppc-dev; +Cc: akpm, Kumar Gala
In-Reply-To: <1290063452-20873-1-git-send-email-b21989@freescale.com>

Tested on my 8548/RIO setup - works as expected.

Alex.

> -----Original Message-----
> From: Shaohui Xie [mailto:b21989@freescale.com]
> Sent: Thursday, November 18, 2010 1:58 AM
> To: linuxppc-dev@lists.ozlabs.org
> Cc: akpm@linux-foundation.org; Shaohui Xie; Li Yang; Kumar Gala; Roy
Zang; Bounine, Alexandre
> Subject: [PATCH 1/2][v4] fsl_rio: move machine_check handler into
machine_check_e500 &
> machine_check_e500mc
>=20

^ permalink raw reply

* [PATCH] drivers: char: hvc: add arm JTAG DCC console support
From: Daniel Walker @ 2010-11-30 19:25 UTC (permalink / raw)
  To: linux-kernel
  Cc: Randy Dunlap, Daniel Walker, Mike Frysinger, Arnd Bergmann,
	Nicolas Pitre, Tony Lindgren, linux-arm-msm, Greg Kroah-Hartman,
	FUJITA Tomonori, Andrew Morton, linuxppc-dev, Alan Cox

This driver adds a basic console that uses the arm JTAG
DCC to transfer data back and forth. It has support for
ARMv6 and ARMv7.

This console is created under the HVC driver, and should be named
/dev/hvcX (or /dev/hvc0 for example).

Cc: Tony Lindgren <tony@atomide.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Nicolas Pitre <nico@fluxnic.net>
Cc: Greg Kroah-Hartman <gregkh@suse.de>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Daniel Walker <dwalker@codeaurora.org>
---
 drivers/char/Kconfig   |    9 +++
 drivers/char/Makefile  |    1 +
 drivers/char/hvc_dcc.c |  133 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 143 insertions(+), 0 deletions(-)
 create mode 100644 drivers/char/hvc_dcc.c

diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 43d3395..d4a7776 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -682,6 +682,15 @@ config HVC_UDBG
        select HVC_DRIVER
        default n
 
+config HVC_DCC
+       bool "ARM JTAG DCC console"
+       depends on ARM
+       select HVC_DRIVER
+       help
+         This console uses the JTAG DCC on ARM to create a console under the HVC
+	 driver. This console is used through a JTAG only on ARM. If you don't have
+	 a JTAG then you probably don't want this option.
+
 config VIRTIO_CONSOLE
 	tristate "Virtio console"
 	depends on VIRTIO
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index ba53ec9..fa0b824 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_HVC_CONSOLE)	+= hvc_vio.o hvsi.o
 obj-$(CONFIG_HVC_ISERIES)	+= hvc_iseries.o
 obj-$(CONFIG_HVC_RTAS)		+= hvc_rtas.o
 obj-$(CONFIG_HVC_TILE)		+= hvc_tile.o
+obj-$(CONFIG_HVC_DCC)		+= hvc_dcc.o
 obj-$(CONFIG_HVC_BEAT)		+= hvc_beat.o
 obj-$(CONFIG_HVC_DRIVER)	+= hvc_console.o
 obj-$(CONFIG_HVC_IRQ)		+= hvc_irq.o
diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c
new file mode 100644
index 0000000..6470f63
--- /dev/null
+++ b/drivers/char/hvc_dcc.c
@@ -0,0 +1,133 @@
+/* Copyright (c) 2010, Code Aurora Forum. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+#include <linux/console.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/moduleparam.h>
+#include <linux/types.h>
+
+#include <asm/processor.h>
+
+#include "hvc_console.h"
+
+/* DCC Status Bits */
+#define DCC_STATUS_RX		(1 << 30)
+#define DCC_STATUS_TX		(1 << 29)
+
+static inline u32 __dcc_getstatus(void)
+{
+	u32 __ret;
+
+	asm("mrc p14, 0, %0, c0, c1, 0	@ read comms ctrl reg"
+		: "=r" (__ret) : : "cc");
+
+	return __ret;
+}
+
+
+#if defined(CONFIG_CPU_V7)
+static inline char __dcc_getchar(void)
+{
+	char __c;
+
+	asm("get_wait:	mrc p14, 0, pc, c0, c1, 0                          \n\
+			bne get_wait                                       \n\
+			mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
+		: "=r" (__c) : : "cc");
+
+	return __c;
+}
+#else
+static inline char __dcc_getchar(void)
+{
+	char __c;
+
+	asm("mrc p14, 0, %0, c0, c5, 0	@ read comms data reg"
+		: "=r" (__c));
+
+	return __c;
+}
+#endif
+
+#if defined(CONFIG_CPU_V7)
+static inline void __dcc_putchar(char c)
+{
+	asm("put_wait:	mrc p14, 0, pc, c0, c1, 0                 \n\
+			bcs put_wait                              \n\
+			mcr p14, 0, %0, c0, c5, 0                   "
+	: : "r" (c) : "cc");
+}
+#else
+static inline void __dcc_putchar(char c)
+{
+	asm("mcr p14, 0, %0, c0, c5, 0	@ write a char"
+		: /* no output register */
+		: "r" (c));
+}
+#endif
+
+static int hvc_dcc_put_chars(uint32_t vt, const char *buf, int count)
+{
+	int i;
+
+	for (i = 0; i < count; i++) {
+		while (__dcc_getstatus() & DCC_STATUS_TX)
+			cpu_relax();
+
+		__dcc_putchar((char)(buf[i] & 0xFF));
+	}
+
+	return count;
+}
+
+static int hvc_dcc_get_chars(uint32_t vt, char *buf, int count)
+{
+	int i;
+
+	for (i = 0; i < count; ++i) {
+		int c = -1;
+
+		if (__dcc_getstatus() & DCC_STATUS_RX)
+			c = __dcc_getchar();
+		if (c < 0)
+			break;
+		buf[i] = c;
+	}
+
+	return i;
+}
+
+static const struct hv_ops hvc_dcc_get_put_ops = {
+	.get_chars = hvc_dcc_get_chars,
+	.put_chars = hvc_dcc_put_chars,
+};
+
+static int __init hvc_dcc_console_init(void)
+{
+	hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
+	return 0;
+}
+console_initcall(hvc_dcc_console_init);
+
+static int __init hvc_dcc_init(void)
+{
+	hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
+	return 0;
+}
+device_initcall(hvc_dcc_init);
-- 
1.7.1

-- 
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

^ permalink raw reply related

* Re: [PATCH] powerpc: fix call to subpage_protection()
From: Milton Miller @ 2010-11-30 11:08 UTC (permalink / raw)
  To: Jim Keniston; +Cc: Michael Neuling, linuxppc-dev
In-Reply-To: <1290110779.32570.9.camel@pasglop>

[fixed Michael Neuling's address]

On Fri Nov 19 about 07:06:19 EST in 2010, Benjamin Herrenschmidt wrote:
> On Thu, 2010-11-18 at 10:23 -0800, Jim Keniston wrote:
> > FWIW, this failure isn't an obstacle for me.  I'm in no way attached to
> > my legacy configuration; pseries_defconfig is fine for me.  On the other
> > hand, I can continue testing fixes, and/or make my system available to
> > other IBMers when I'm not using it, if you want to continue to pursue
> > this problem.

> From the look of it your "legacy" config is lacking the necessary
> storage drivers...
> 

Looking closer, it would appear he intended to get them from initramfs
but unpacking it failed.  Pulling from his log in the original message:

> > 
> > Calling ibm,client-architecture-support... not implemented
> > command line: root=/dev/disk/by-id/scsi-SIBM_ST373453LC_3HW1CQ1400007445Q2A4-part3 quiet profile=2 sysrq=1 insmod=sym53c8xx insmod=ipr crashkernel=256M-:128M loglevel=8 
> > 
> > memory layout at init:
> >   memory_limit : 0000000000000000 (16 MB aligned)
> >   alloc_bottom : 0000000003550000
> >   alloc_top    : 0000000008000000
> >   alloc_top_hi : 0000000070000000
> >   amo_top      : 0000000008000000
> >   ram_top      : 0000000070000000
> > instantiating rtas at 0x00000000076a0000... done
> > boot cpu hw idx 0
> > starting cpu hw idx 2... done
> > copying OF device tree...
> > Building dt strings...
> > Building dt structure...
> > Device tree strings 0x0000000003560000 -> 0x000000000356129c
> > Device tree struct  0x0000000003570000 -> 0x0000000003580000
..
> > Found initrd at 0xc000000002d00000:0xc00000000354e28a
..
> > NET: Registered protocol family 1
> > PCI: CLS 128 bytes, default 128
> > Unpacking initramfs...
> > Initramfs unpacking failed: read error

"read error" occurs in lib/decompress_inflate.c if the fill routine
returns a negative count, which would inlclude a NULL fill pointer like
init/initramfs.c.


Maybe this debugging patch (against 2.6.35, applys with small offset to 
2.6.37-rc4) can help isolate the corruption?

From: Milton Miller <miltonm.bga.com>
initramfs: show input and output offsets on unpacking failures

When the initramfs input is corrupted, having the pointer to the buffer
segment being decompressed, and the decompress input and output byte
counts from the compressed stream can help isolate the source of the
corruption.

Signed-off-by: Milton Miller <miltonm@bga.com>

---
 init/initramfs.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: sim/init/initramfs.c
===================================================================
--- sim.orig/init/initramfs.c	2009-07-13 18:44:56.000000000 -0500
+++ sim/init/initramfs.c	2009-07-13 18:59:32.000000000 -0500
@@ -384,6 +384,9 @@ static int __init write_buffer(char *buf
 	return len - count;
 }
 
+static unsigned my_inptr;   /* index of next byte to be processed in inbuf */
+static unsigned bytes_out;  /* count of chars processed from current archive */
+
 static int __init flush_buffer(void *bufv, unsigned len)
 {
 	char *buf = (char *) bufv;
@@ -393,6 +396,7 @@ static int __init flush_buffer(void *buf
 		return -1;
 	while ((written = write_buffer(buf, len)) < len && !message) {
 		char c = buf[written];
+		bytes_out += written;
 		if (c == '0') {
 			buf += written;
 			len -= written;
@@ -407,8 +411,6 @@ static int __init flush_buffer(void *buf
 	return origLen;
 }
 
-static unsigned my_inptr;   /* index of next byte to be processed in inbuf */
-
 #include <linux/decompress/generic.h>
 
 static char * __init unpack_to_rootfs(char *buf, unsigned len)
@@ -430,6 +432,8 @@ static char * __init unpack_to_rootfs(ch
 	message = NULL;
 	while (!message && len) {
 		loff_t saved_offset = this_header;
+		bytes_out = 0;
+		my_inptr = 0;
 		if (*buf == '0' && !(this_header & 3)) {
 			state = Start;
 			written = write_buffer(buf, len);
@@ -462,6 +466,9 @@ static char * __init unpack_to_rootfs(ch
 		buf += my_inptr;
 		len -= my_inptr;
 	}
+	if (message) {
+		printk(KERN_ERR "Failed unpacking at %p input %d output %d \n",buf - my_inptr, my_inptr, bytes_out);
+	}
 	dir_utime();
 	kfree(name_buf);
 	kfree(symlink_buf);

^ permalink raw reply

* Re: [PATCH] powerpc: fix call to subpage_protection()
From: Milton Miller @ 2010-11-30 11:06 UTC (permalink / raw)
  To: Jim Keniston; +Cc: Michael Neuling <mikeyatneuling.org>, linuxppc-dev
In-Reply-To: <1290110779.32570.9.camel@pasglop>

On Fri Nov 19 about 07:06:19 EST in 2010, Benjamin Herrenschmidt wrote:
> On Thu, 2010-11-18 at 10:23 -0800, Jim Keniston wrote:
> > FWIW, this failure isn't an obstacle for me.  I'm in no way attached to
> > my legacy configuration; pseries_defconfig is fine for me.  On the other
> > hand, I can continue testing fixes, and/or make my system available to
> > other IBMers when I'm not using it, if you want to continue to pursue
> > this problem.

> From the look of it your "legacy" config is lacking the necessary
> storage drivers...
> 

Looking closer, it would appear he intended to get them from initramfs
but unpacking it failed.  Pulling from his log in the original message:

> > 
> > Calling ibm,client-architecture-support... not implemented
> > command line: root=/dev/disk/by-id/scsi-SIBM_ST373453LC_3HW1CQ1400007445Q2A4-part3 quiet profile=2 sysrq=1 insmod=sym53c8xx insmod=ipr crashkernel=256M-:128M loglevel=8 
> > 
> > memory layout at init:
> >   memory_limit : 0000000000000000 (16 MB aligned)
> >   alloc_bottom : 0000000003550000
> >   alloc_top    : 0000000008000000
> >   alloc_top_hi : 0000000070000000
> >   amo_top      : 0000000008000000
> >   ram_top      : 0000000070000000
> > instantiating rtas at 0x00000000076a0000... done
> > boot cpu hw idx 0
> > starting cpu hw idx 2... done
> > copying OF device tree...
> > Building dt strings...
> > Building dt structure...
> > Device tree strings 0x0000000003560000 -> 0x000000000356129c
> > Device tree struct  0x0000000003570000 -> 0x0000000003580000
..
> > Found initrd at 0xc000000002d00000:0xc00000000354e28a
..
> > NET: Registered protocol family 1
> > PCI: CLS 128 bytes, default 128
> > Unpacking initramfs...
> > Initramfs unpacking failed: read error

"read error" occurs in lib/decompress_inflate.c if the fill routine
returns a negative count, which would inlclude a NULL fill pointer like
init/initramfs.c.


Maybe this debugging patch (against 2.6.35, applys with small offset to 
2.6.37-rc4) can help isolate the corruption?

From: Milton Miller <miltonm.bga.com>
initramfs: show input and output offsets on unpacking failures

When the initramfs input is corrupted, having the pointer to the buffer
segment being decompressed, and the decompress input and output byte
counts from the compressed stream can help isolate the source of the
corruption.

Signed-off-by: Milton Miller <miltonm@bga.com>

---
 init/initramfs.c |    5 +++++
 1 file changed, 5 insertions(+)

Index: sim/init/initramfs.c
===================================================================
--- sim.orig/init/initramfs.c	2009-07-13 18:44:56.000000000 -0500
+++ sim/init/initramfs.c	2009-07-13 18:59:32.000000000 -0500
@@ -384,6 +384,9 @@ static int __init write_buffer(char *buf
 	return len - count;
 }
 
+static unsigned my_inptr;   /* index of next byte to be processed in inbuf */
+static unsigned bytes_out;  /* count of chars processed from current archive */
+
 static int __init flush_buffer(void *bufv, unsigned len)
 {
 	char *buf = (char *) bufv;
@@ -393,6 +396,7 @@ static int __init flush_buffer(void *buf
 		return -1;
 	while ((written = write_buffer(buf, len)) < len && !message) {
 		char c = buf[written];
+		bytes_out += written;
 		if (c == '0') {
 			buf += written;
 			len -= written;
@@ -407,8 +411,6 @@ static int __init flush_buffer(void *buf
 	return origLen;
 }
 
-static unsigned my_inptr;   /* index of next byte to be processed in inbuf */
-
 #include <linux/decompress/generic.h>
 
 static char * __init unpack_to_rootfs(char *buf, unsigned len)
@@ -430,6 +432,8 @@ static char * __init unpack_to_rootfs(ch
 	message = NULL;
 	while (!message && len) {
 		loff_t saved_offset = this_header;
+		bytes_out = 0;
+		my_inptr = 0;
 		if (*buf == '0' && !(this_header & 3)) {
 			state = Start;
 			written = write_buffer(buf, len);
@@ -462,6 +466,9 @@ static char * __init unpack_to_rootfs(ch
 		buf += my_inptr;
 		len -= my_inptr;
 	}
+	if (message) {
+		printk(KERN_ERR "Failed unpacking at %p input %d output %d \n",buf - my_inptr, my_inptr, bytes_out);
+	}
 	dir_utime();
 	kfree(name_buf);
 	kfree(symlink_buf);

^ permalink raw reply

* Re: [v4] ppc44x:PHY fixup for USB on canyonlands board
From: Milton Miller @ 2010-11-30  9:11 UTC (permalink / raw)
  To: Rupjyoti Sarmah; +Cc: linuxppc-dev, rsarmah, linux-kernel
In-Reply-To: <201011291313.oATDD1AF028975@amcc.com>

I prepared these comments for v1, but aparently forgot to send them,
so I'll do so now.


On Mon, 29 Nov 2010 about 03:13:01 -0000, Rupjyoti Sarmah wrote:
> This fix is a reset for USB PHY that requires some amount of time for power to be stable on Canyonlands.
> 
> Signed-off-by: Rupjyoti Sarmah <rsarmah@apm.com>
> 
> ---
> index a303703..3c5d63c 100644
> --- a/arch/powerpc/boot/dts/canyonlands.dts
> +++ b/arch/powerpc/boot/dts/canyonlands.dts
> @@ -224,6 +224,13 @@
>  					};
>  				};
>  
> +				cpld@2,0 {
> +					#address-cells = <1>;
> +					#size-cells = <1>;

why does this node have #address-cells and #size-cells but no children? 

> +					compatible = "amcc,ppc460ex-bcsr";
> +					reg = <2 0x0 0x9>;
> +				};
> +
>  				ndfc@3,0 {
>  					compatible = "ibm,ndfc";
>  					reg = <0x00000003 0x00000000 0x00002000>;
..
> diff --git a/arch/powerpc/platforms/44x/canyonlands.c b/arch/powerpc/platforms/44x/canyonlands.c
> new file mode 100644
> index 0000000..4917c31
> --- /dev/null
> +++ b/arch/powerpc/platforms/44x/canyonlands.c
> @@ -0,0 +1,120 @@
> +

> +static __initdata struct of_device_id ppc44x_of_bus[] = {
..
> +static int __init ppc44x_device_probe(void)
> +{
> +	of_platform_bus_probe(NULL, ppc44x_of_bus, NULL);
> +
> +	return 0;
> +}
> +machine_device_initcall(canyonlands, ppc44x_device_probe);
> +
> +/* Using this code only for the Canyonlands board.  */
> +
> +static int __init ppc44x_probe(void)
> +{
> +	unsigned long root = of_get_flat_dt_root();
> +	if (of_flat_dt_is_compatible(root, "amcc,canyonlands")) {
> +		ppc_pci_set_flags(PPC_PCI_REASSIGN_ALL_RSRC);
> +		return 1;
> +		}
> +	return 0;
> +}
> +
> +/* USB PHY fixup code on Canyonlands kit. */

All of the above have ppc44x prefix, but have been copied to this
canyonlands file.   While there is no build error because they
are all static it will be less confusing if they had a canyonlands
prefix instead.

> +
> +static int __init ppc460ex_canyonlands_fixup(void)
> +{
> +	u8 __iomem *bcsr ;
> +	void __iomem *vaddr;
> +	struct device_node *np;
> +
> +	np = of_find_compatible_node(NULL, NULL, "amcc,ppc460ex-bcsr");
> +	if (!np) {
> +		printk(KERN_ERR "failed did not find amcc, ppc460ex bcsr node\n");
> +		return -ENODEV;
> +	}
> +
> +	bcsr = of_iomap(np, 0);
> +	of_node_put(np);
> +
> +	if (!bcsr) {
> +		printk(KERN_CRIT "Could not remap bcsr\n");
> +		return -ENODEV;
> +	}
> +
> +	np = of_find_compatible_node(NULL, NULL, "ibm,ppc4xx-gpio");
> +	vaddr = of_iomap(np, 0);
> +	if (!vaddr) {
> +		printk(KERN_CRIT "Could not get gpio node address\n");
> +		return -ENODEV;
> +	}
> +	/* Disable USB, through the BCSR7 bits */
> +	setbits8(&bcsr[7], BCSR_USB_EN);
> +
> +	/* Wait for a while after reset */
> +	msleep(100);
> +
> +	/* Enable USB here */
> +	clrbits8(&bcsr[7], BCSR_USB_EN);
> +
> +	/*
> +	 * Configure multiplexed gpio16 and gpio19 as alternate1 output
> +	 * source after USB reset.This configuration is done through GPIO0_TSRH
> +	 * and GPIO0_OSRH bits 0:1 and 6:7.

Missing spaces before the second sentence.

Rather then telling us which bits are being set, which is fairly
obvoius from the code, it would be more helpful to say what
alternate1 output source means.

I'm curious why you set these after the reset when they were not
set before.  Should they be set to something else temporarly during
the reset?

> +	 */
> +	setbits32((vaddr + GPIO0_OSRH), 0x42000000);
> +	setbits32((vaddr + GPIO0_TSRH), 0x42000000);
> +	of_node_put(np);
> +	return 0;

The error paths in this function leave incorrect refcounts on the
nodes, and no path does of_unmap of the regions even though the
variables with the address goes out of scope.

> +}
> +machine_device_initcall(canyonlands, ppc460ex_canyonlands_fixup);
> +define_machine(canyonlands) {
> +	.name = "Canyonlands",
> +	.probe = ppc44x_probe,
> +	.progress = udbg_progress,
> +	.init_IRQ = uic_init_tree,
> +	.get_irq = uic_get_irq,
> +	.restart = ppc4xx_reset_system,
> +	.calibrate_decr = generic_calibrate_decr,
> +};


Also, you shouldn't need the select PPC44x_SIMPLE in the CANYONLANDS
stanza in arch/powerpc/platforms/44x/Kconfig.

milton

^ permalink raw reply

* [PATCH 06/13] powerpc/nvram: Shuffle code around in nvram_create_partition()
From: benh @ 2010-11-30  5:47 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: jkensito
In-Reply-To: <1291096063-14386-1-git-send-email-benh@kernel.crashing.org>

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

This error log stuff is really pseries specific. As a first step we move
the initialization of these variables to the caller of
nvram_create_partition(), which is also slightly reorganized so we
setup the free partition before we clear the new partition, so the
chance of an error during clear leaving us with invalid headers
is lessened.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/kernel/nvram_64.c |  108 +++++++++++++++++++++++-----------------
 1 files changed, 62 insertions(+), 46 deletions(-)

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 02737e6..eabee7c 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -313,9 +313,15 @@ static int __init nvram_remove_os_partition(void)
  * @sig: signature of the partition to create
  * @req_size: size of data to allocate in bytes
  * @min_size: minimum acceptable size (0 means req_size)
+ *
+ * Returns a negative error code or a positive nvram index
+ * of the beginning of the data area of the newly created
+ * partition. If you provided a min_size smaller than req_size
+ * you need to query for the actual size yourself after the
+ * call using nvram_partition_get_size().
  */
-static int __init nvram_create_partition(const char *name, int sig,
-					 int req_size, int min_size)
+static loff_t __init nvram_create_partition(const char *name, int sig,
+					    int req_size, int min_size)
 {
 	struct nvram_partition *part;
 	struct nvram_partition *new_part;
@@ -334,6 +340,8 @@ static int __init nvram_create_partition(const char *name, int sig,
 	 */
 	if (min_size == 0)
 		min_size = req_size;
+	if (min_size > req_size)
+		return -EINVAL;
 
 	/* Now add one block to each for the header */
 	req_size += 1;
@@ -362,7 +370,7 @@ static int __init nvram_create_partition(const char *name, int sig,
 	/* Create our OS partition */
 	new_part = kmalloc(sizeof(*new_part), GFP_KERNEL);
 	if (!new_part) {
-		printk(KERN_ERR "nvram_create_os_partition: kmalloc failed\n");
+		pr_err("nvram_create_os_partition: kmalloc failed\n");
 		return -ENOMEM;
 	}
 
@@ -374,12 +382,29 @@ static int __init nvram_create_partition(const char *name, int sig,
 
 	rc = nvram_write_header(new_part);
 	if (rc <= 0) {
-		printk(KERN_ERR "nvram_create_os_partition: nvram_write_header "
-				"failed (%d)\n", rc);
+		pr_err("nvram_create_os_partition: nvram_write_header "
+		       "failed (%d)\n", rc);
 		return rc;
 	}
+	list_add_tail(&new_part->partition, &free_part->partition);
+
+	/* Adjust or remove the partition we stole the space from */
+	if (free_part->header.length > size) {
+		free_part->index += size * NVRAM_BLOCK_LEN;
+		free_part->header.length -= size;
+		free_part->header.checksum = nvram_checksum(&free_part->header);
+		rc = nvram_write_header(free_part);
+		if (rc <= 0) {
+			pr_err("nvram_create_os_partition: nvram_write_header "
+			       "failed (%d)\n", rc);
+			return rc;
+		}
+	} else {
+		list_del(&free_part->partition);
+		kfree(free_part);
+	} 
 
-	/* Clear the partition */
+	/* Clear the new partition */
 	for (tmp_index = new_part->index + NVRAM_HEADER_LEN;
 	     tmp_index <  ((size - 1) * NVRAM_BLOCK_LEN);
 	     tmp_index += NVRAM_BLOCK_LEN) {
@@ -390,31 +415,24 @@ static int __init nvram_create_partition(const char *name, int sig,
 		}
 	}
 	
-	nvram_error_log_index = new_part->index + NVRAM_HEADER_LEN;
-	nvram_error_log_size = ((part->header.length - 1) *
-				NVRAM_BLOCK_LEN) - sizeof(struct err_log_info);
-	
-	list_add_tail(&new_part->partition, &free_part->partition);
-
-	if (free_part->header.length <= size) {
-		list_del(&free_part->partition);
-		kfree(free_part);
-		return 0;
-	} 
+	return new_part->index + NVRAM_HEADER_LEN;
+}
 
-	/* Adjust the partition we stole the space from */
-	free_part->index += size * NVRAM_BLOCK_LEN;
-	free_part->header.length -= size;
-	free_part->header.checksum = nvram_checksum(&free_part->header);
+/**
+ * nvram_get_partition_size - Get the data size of an nvram partition
+ * @data_index: This is the offset of the start of the data of
+ *              the partition. The same value that is returned by
+ *              nvram_create_partition().
+ */
+static int nvram_get_partition_size(loff_t data_index)
+{
+	struct nvram_partition *part;
 	
-	rc = nvram_write_header(free_part);
-	if (rc <= 0) {
-		printk(KERN_ERR "nvram_create_os_partition: nvram_write_header "
-		       "failed (%d)\n", rc);
-		return rc;
+	list_for_each_entry(part, &nvram_part->partition, partition) {
+		if (part->index + NVRAM_HEADER_LEN == data_index)
+			return (part->header.length - 1) * NVRAM_BLOCK_LEN;
 	}
-
-	return 0;
+	return -1;
 }
 
 
@@ -469,30 +487,28 @@ static int __init nvram_setup_partition(void)
 	}
 	
 	/* try creating a partition with the free space we have */
-	rc = 	nvram_create_partition("ppc64,linux", NVRAM_SIG_OS,
+	rc = nvram_create_partition("ppc64,linux", NVRAM_SIG_OS,
 				       NVRAM_MAX_REQ, NVRAM_MIN_REQ);
-	if (!rc)
-		return 0;
-		
-	/* need to free up some space */
-	rc = nvram_remove_os_partition();
-	if (rc) {
-		return rc;
-	}
-	
-	/* create a partition in this new space */
-	rc = 	nvram_create_partition("ppc64,linux", NVRAM_SIG_OS,
-				       NVRAM_MAX_REQ, NVRAM_MIN_REQ);
-	if (rc) {
-		printk(KERN_ERR "nvram_create_partition: Could not find a "
-		       "NVRAM partition large enough\n");
-		return rc;
+	if (rc < 0) {
+		/* need to free up some space */
+		rc = nvram_remove_os_partition();
+		if (rc)
+			return rc;	
+		/* create a partition in this new space */
+		rc = nvram_create_partition("ppc64,linux", NVRAM_SIG_OS,
+					    NVRAM_MAX_REQ, NVRAM_MIN_REQ);
+		if (rc < 0) {
+			pr_err("nvram_create_partition: Could not find"
+			       " enough space in NVRAM for partition\n");
+			return rc;
+		}
 	}
 	
+	nvram_error_log_index = rc;	
+	nvram_error_log_size = nvram_get_partition_size(rc) - sizeof(struct err_log_info);	
 	return 0;
 }
 
-
 static int __init nvram_scan_partitions(void)
 {
 	loff_t cur_index = 0;
-- 
1.7.1

^ permalink raw reply related

* [PATCH 08/13] powerpc/nvram: Add nvram_find_partition()
From: benh @ 2010-11-30  5:47 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: jkensito
In-Reply-To: <1291096063-14386-1-git-send-email-benh@kernel.crashing.org>

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 arch/powerpc/kernel/nvram_64.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c
index 6dd2700..01e6844 100644
--- a/arch/powerpc/kernel/nvram_64.c
+++ b/arch/powerpc/kernel/nvram_64.c
@@ -429,6 +429,28 @@ static int nvram_get_partition_size(loff_t data_index)
 }
 
 
+/**
+ * nvram_find_partition - Find an nvram partition by signature and name
+ * @name: Name of the partition or NULL for any name
+ * @sig: Signature to test against
+ * @out_size: if non-NULL, returns the size of the data part of the partition
+ */
+loff_t nvram_find_partition(const char *name, int sig, int *out_size)
+{
+	struct nvram_partition *p;
+
+	list_for_each_entry(p, &nvram_part->partition, partition) {
+		if (p->header.signature == sig &&
+		    (!name || !strncmp(p->header.name, name, 12))) {
+			if (out_size)
+				*out_size = (p->header.length - 1) *
+					NVRAM_BLOCK_LEN;
+			return p->index + NVRAM_HEADER_LEN;
+		}
+	}
+	return 0;
+}
+
 /* nvram_setup_partition
  *
  * This will setup the partition we need for buffering the
-- 
1.7.1

^ permalink raw reply related


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