Patchwork PowerPC: const intspec pointers

login
register
mail settings
Submitter Roman Fietze
Date Dec. 8, 2009, 12:39 p.m.
Message ID <200912081339.50722.roman.fietze@telemotive.de>
Download mbox | patch
Permalink /patch/40617/
State Accepted
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

Roman Fietze - Dec. 8, 2009, 12:39 p.m.
Hello,

Writing a driver using SCLPC on the MPC5200B I detected, that the
intspec arrays to map irqs to Linux virq cannot be const, because the
mapping and xlate functions only take non const pointers. All those
functions do not modify the intspec, so a const pointer could be used.

I tried to compile that patch on different platforms and got no
compiler errors, so I'd like to ask you to review that patch, and
check if it's worth using it.


Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
---
 arch/powerpc/include/asm/irq.h                  |    4 ++--
 arch/powerpc/kernel/irq.c                       |    2 +-
 arch/powerpc/platforms/52xx/media5200.c         |    2 +-
 arch/powerpc/platforms/52xx/mpc52xx_gpt.c       |    2 +-
 arch/powerpc/platforms/52xx/mpc52xx_pic.c       |    2 +-
 arch/powerpc/platforms/85xx/socrates_fpga_pic.c |    2 +-
 arch/powerpc/platforms/86xx/gef_pic.c           |    2 +-
 arch/powerpc/platforms/cell/beat_interrupt.c    |    4 ++--
 arch/powerpc/platforms/cell/interrupt.c         |    2 +-
 arch/powerpc/platforms/cell/spider-pic.c        |    2 +-
 arch/powerpc/platforms/powermac/pic.c           |    2 +-
 arch/powerpc/platforms/pseries/xics.c           |    2 +-
 arch/powerpc/sysdev/cpm2_pic.c                  |    2 +-
 arch/powerpc/sysdev/i8259.c                     |    2 +-
 arch/powerpc/sysdev/ipic.c                      |    2 +-
 arch/powerpc/sysdev/mpc8xx_pic.c                |    2 +-
 arch/powerpc/sysdev/mpic.c                      |    2 +-
 arch/powerpc/sysdev/qe_lib/qe_ic.c              |    2 +-
 arch/powerpc/sysdev/tsi108_pci.c                |    2 +-
 arch/powerpc/sysdev/uic.c                       |    2 +-
 arch/powerpc/sysdev/xilinx_intc.c               |    2 +-
 21 files changed, 23 insertions(+), 23 deletions(-)
Benjamin Herrenschmidt - Dec. 9, 2009, 2:45 a.m.
On Tue, 2009-12-08 at 13:39 +0100, Roman Fietze wrote:
> Hello,
> 
> Writing a driver using SCLPC on the MPC5200B I detected, that the
> intspec arrays to map irqs to Linux virq cannot be const, because the
> mapping and xlate functions only take non const pointers. All those
> functions do not modify the intspec, so a const pointer could be used.
> 
> I tried to compile that patch on different platforms and got no
> compiler errors, so I'd like to ask you to review that patch, and
> check if it's worth using it.

Looks good.

I'll give it a go and if it doesn't break anything, will apply.

Cheers,
Ben.

> Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
> ---
>  arch/powerpc/include/asm/irq.h                  |    4 ++--
>  arch/powerpc/kernel/irq.c                       |    2 +-
>  arch/powerpc/platforms/52xx/media5200.c         |    2 +-
>  arch/powerpc/platforms/52xx/mpc52xx_gpt.c       |    2 +-
>  arch/powerpc/platforms/52xx/mpc52xx_pic.c       |    2 +-
>  arch/powerpc/platforms/85xx/socrates_fpga_pic.c |    2 +-
>  arch/powerpc/platforms/86xx/gef_pic.c           |    2 +-
>  arch/powerpc/platforms/cell/beat_interrupt.c    |    4 ++--
>  arch/powerpc/platforms/cell/interrupt.c         |    2 +-
>  arch/powerpc/platforms/cell/spider-pic.c        |    2 +-
>  arch/powerpc/platforms/powermac/pic.c           |    2 +-
>  arch/powerpc/platforms/pseries/xics.c           |    2 +-
>  arch/powerpc/sysdev/cpm2_pic.c                  |    2 +-
>  arch/powerpc/sysdev/i8259.c                     |    2 +-
>  arch/powerpc/sysdev/ipic.c                      |    2 +-
>  arch/powerpc/sysdev/mpc8xx_pic.c                |    2 +-
>  arch/powerpc/sysdev/mpic.c                      |    2 +-
>  arch/powerpc/sysdev/qe_lib/qe_ic.c              |    2 +-
>  arch/powerpc/sysdev/tsi108_pci.c                |    2 +-
>  arch/powerpc/sysdev/uic.c                       |    2 +-
>  arch/powerpc/sysdev/xilinx_intc.c               |    2 +-
>  21 files changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
> index bbcd1aa..a0bcafc 100644
> --- a/arch/powerpc/include/asm/irq.h
> +++ b/arch/powerpc/include/asm/irq.h
> @@ -99,7 +99,7 @@ struct irq_host_ops {
>  	 * interrupt controller has for that line)
>  	 */
>  	int (*xlate)(struct irq_host *h, struct device_node *ctrler,
> -		     u32 *intspec, unsigned int intsize,
> +		     const u32 *intspec, unsigned int intsize,
>  		     irq_hw_number_t *out_hwirq, unsigned int *out_type);
>  };
>  
> @@ -313,7 +313,7 @@ extern void irq_free_virt(unsigned int virq, unsigned int 
> count);
>   * of the of_irq_map_*() functions.
>   */
>  extern unsigned int irq_create_of_mapping(struct device_node *controller,
> -					  u32 *intspec, unsigned int intsize);
> +					  const u32 *intspec, unsigned int intsize);
>  
>  /**
>   * irq_of_parse_and_map - Parse and Map an interrupt into linux virq space
> diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
> index e5d1211..6e294e0 100644
> --- a/arch/powerpc/kernel/irq.c
> +++ b/arch/powerpc/kernel/irq.c
> @@ -699,7 +699,7 @@ unsigned int irq_create_mapping(struct irq_host *host,
>  EXPORT_SYMBOL_GPL(irq_create_mapping);
>  
>  unsigned int irq_create_of_mapping(struct device_node *controller,
> -				   u32 *intspec, unsigned int intsize)
> +				   const u32 *intspec, unsigned int intsize)
>  {
>  	struct irq_host *host;
>  	irq_hw_number_t hwirq;
> diff --git a/arch/powerpc/platforms/52xx/media5200.c 
> b/arch/powerpc/platforms/52xx/media5200.c
> index 68e4f16..b1ec9bb 100644
> --- a/arch/powerpc/platforms/52xx/media5200.c
> +++ b/arch/powerpc/platforms/52xx/media5200.c
> @@ -127,7 +127,7 @@ static int media5200_irq_map(struct irq_host *h, unsigned 
> int virq,
>  }
>  
>  static int media5200_irq_xlate(struct irq_host *h, struct device_node *ct,
> -				 u32 *intspec, unsigned int intsize,
> +				 const u32 *intspec, unsigned int intsize,
>  				 irq_hw_number_t *out_hwirq,
>  				 unsigned int *out_flags)
>  {
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c 
> b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> index bfbcd41..78f0ab6 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
> @@ -182,7 +182,7 @@ static int mpc52xx_gpt_irq_map(struct irq_host *h, 
> unsigned int virq,
>  }
>  
>  static int mpc52xx_gpt_irq_xlate(struct irq_host *h, struct device_node *ct,
> -				 u32 *intspec, unsigned int intsize,
> +				 const u32 *intspec, unsigned int intsize,
>  				 irq_hw_number_t *out_hwirq,
>  				 unsigned int *out_flags)
>  {
> diff --git a/arch/powerpc/platforms/52xx/mpc52xx_pic.c 
> b/arch/powerpc/platforms/52xx/mpc52xx_pic.c
> index 480f806..53d44f6 100644
> --- a/arch/powerpc/platforms/52xx/mpc52xx_pic.c
> +++ b/arch/powerpc/platforms/52xx/mpc52xx_pic.c
> @@ -355,7 +355,7 @@ static int mpc52xx_is_extirq(int l1, int l2)
>   * mpc52xx_irqhost_xlate - translate virq# from device tree interrupts 
> property
>   */
>  static int mpc52xx_irqhost_xlate(struct irq_host *h, struct device_node *ct,
> -				 u32 *intspec, unsigned int intsize,
> +				 const u32 *intspec, unsigned int intsize,
>  				 irq_hw_number_t *out_hwirq,
>  				 unsigned int *out_flags)
>  {
> diff --git a/arch/powerpc/platforms/85xx/socrates_fpga_pic.c 
> b/arch/powerpc/platforms/85xx/socrates_fpga_pic.c
> index 60edf63..380858a 100644
> --- a/arch/powerpc/platforms/85xx/socrates_fpga_pic.c
> +++ b/arch/powerpc/platforms/85xx/socrates_fpga_pic.c
> @@ -253,7 +253,7 @@ static int socrates_fpga_pic_host_map(struct irq_host *h, 
> unsigned int virq,
>  }
>  
>  static int socrates_fpga_pic_host_xlate(struct irq_host *h,
> -		struct device_node *ct,	u32 *intspec, unsigned int intsize,
> +		struct device_node *ct,	const u32 *intspec, unsigned int intsize,
>  		irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  {
>  	struct socrates_fpga_irq_info *fpga_irq = &fpga_irqs[intspec[0]];
> diff --git a/arch/powerpc/platforms/86xx/gef_pic.c 
> b/arch/powerpc/platforms/86xx/gef_pic.c
> index 50d0a2b..8d4479f 100644
> --- a/arch/powerpc/platforms/86xx/gef_pic.c
> +++ b/arch/powerpc/platforms/86xx/gef_pic.c
> @@ -170,7 +170,7 @@ static int gef_pic_host_map(struct irq_host *h, unsigned 
> int virq,
>  }
>  
>  static int gef_pic_host_xlate(struct irq_host *h, struct device_node *ct,
> -			    u32 *intspec, unsigned int intsize,
> +			    const u32 *intspec, unsigned int intsize,
>  			    irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  {
>  
> diff --git a/arch/powerpc/platforms/cell/beat_interrupt.c 
> b/arch/powerpc/platforms/cell/beat_interrupt.c
> index 7225484..1f2fd65 100644
> --- a/arch/powerpc/platforms/cell/beat_interrupt.c
> +++ b/arch/powerpc/platforms/cell/beat_interrupt.c
> @@ -166,11 +166,11 @@ static void beatic_pic_host_remap(struct irq_host *h, 
> unsigned int virq,
>   * Note: We have only 1 entry to translate.
>   */
>  static int beatic_pic_host_xlate(struct irq_host *h, struct device_node *ct,
> -				 u32 *intspec, unsigned int intsize,
> +				 const u32 *intspec, unsigned int intsize,
>  				 irq_hw_number_t *out_hwirq,
>  				 unsigned int *out_flags)
>  {
> -	u64 *intspec2 = (u64 *)intspec;
> +	const u64 *intspec2 = (const u64 *)intspec;
>  
>  	*out_hwirq = *intspec2;
>  	*out_flags |= IRQ_TYPE_LEVEL_LOW;
> diff --git a/arch/powerpc/platforms/cell/interrupt.c 
> b/arch/powerpc/platforms/cell/interrupt.c
> index 882e470..d9d33cb 100644
> --- a/arch/powerpc/platforms/cell/interrupt.c
> +++ b/arch/powerpc/platforms/cell/interrupt.c
> @@ -297,7 +297,7 @@ static int iic_host_map(struct irq_host *h, unsigned int 
> virq,
>  }
>  
>  static int iic_host_xlate(struct irq_host *h, struct device_node *ct,
> -			   u32 *intspec, unsigned int intsize,
> +			   const u32 *intspec, unsigned int intsize,
>  			   irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  
>  {
> diff --git a/arch/powerpc/platforms/cell/spider-pic.c 
> b/arch/powerpc/platforms/cell/spider-pic.c
> index 4e56556..37c2e4e 100644
> --- a/arch/powerpc/platforms/cell/spider-pic.c
> +++ b/arch/powerpc/platforms/cell/spider-pic.c
> @@ -187,7 +187,7 @@ static int spider_host_map(struct irq_host *h, unsigned 
> int virq,
>  }
>  
>  static int spider_host_xlate(struct irq_host *h, struct device_node *ct,
> -			   u32 *intspec, unsigned int intsize,
> +			   const u32 *intspec, unsigned int intsize,
>  			   irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  
>  {
> diff --git a/arch/powerpc/platforms/powermac/pic.c 
> b/arch/powerpc/platforms/powermac/pic.c
> index d212006..da6b4b9 100644
> --- a/arch/powerpc/platforms/powermac/pic.c
> +++ b/arch/powerpc/platforms/powermac/pic.c
> @@ -303,7 +303,7 @@ static int pmac_pic_host_map(struct irq_host *h, unsigned 
> int virq,
>  }
>  
>  static int pmac_pic_host_xlate(struct irq_host *h, struct device_node *ct,
> -			       u32 *intspec, unsigned int intsize,
> +			       const u32 *intspec, unsigned int intsize,
>  			       irq_hw_number_t *out_hwirq,
>  			       unsigned int *out_flags)
>  
> diff --git a/arch/powerpc/platforms/pseries/xics.c 
> b/arch/powerpc/platforms/pseries/xics.c
> index b9bf0ee..1821eae 100644
> --- a/arch/powerpc/platforms/pseries/xics.c
> +++ b/arch/powerpc/platforms/pseries/xics.c
> @@ -434,7 +434,7 @@ static int xics_host_map(struct irq_host *h, unsigned int 
> virq,
>  }
>  
>  static int xics_host_xlate(struct irq_host *h, struct device_node *ct,
> -			   u32 *intspec, unsigned int intsize,
> +			   const u32 *intspec, unsigned int intsize,
>  			   irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  
>  {
> diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/cpm2_pic.c
> index 78f1f7c..3d2be56 100644
> --- a/arch/powerpc/sysdev/cpm2_pic.c
> +++ b/arch/powerpc/sysdev/cpm2_pic.c
> @@ -216,7 +216,7 @@ static int cpm2_pic_host_map(struct irq_host *h, unsigned 
> int virq,
>  }
>  
>  static int cpm2_pic_host_xlate(struct irq_host *h, struct device_node *ct,
> -			    u32 *intspec, unsigned int intsize,
> +			    const u32 *intspec, unsigned int intsize,
>  			    irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  {
>  	*out_hwirq = intspec[0];
> diff --git a/arch/powerpc/sysdev/i8259.c b/arch/powerpc/sysdev/i8259.c
> index a96584a..ccc9a3b 100644
> --- a/arch/powerpc/sysdev/i8259.c
> +++ b/arch/powerpc/sysdev/i8259.c
> @@ -198,7 +198,7 @@ static void i8259_host_unmap(struct irq_host *h, unsigned 
> int virq)
>  }
>  
>  static int i8259_host_xlate(struct irq_host *h, struct device_node *ct,
> -			    u32 *intspec, unsigned int intsize,
> +			    const u32 *intspec, unsigned int intsize,
>  			    irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  {
>  	static unsigned char map_isa_senses[4] = {
> diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
> index cb7689c..27259f4 100644
> --- a/arch/powerpc/sysdev/ipic.c
> +++ b/arch/powerpc/sysdev/ipic.c
> @@ -697,7 +697,7 @@ static int ipic_host_map(struct irq_host *h, unsigned int 
> virq,
>  }
>  
>  static int ipic_host_xlate(struct irq_host *h, struct device_node *ct,
> -			   u32 *intspec, unsigned int intsize,
> +			   const u32 *intspec, unsigned int intsize,
>  			   irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  
>  {
> diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c 
> b/arch/powerpc/sysdev/mpc8xx_pic.c
> index 5d2d552..72544a1 100644
> --- a/arch/powerpc/sysdev/mpc8xx_pic.c
> +++ b/arch/powerpc/sysdev/mpc8xx_pic.c
> @@ -130,7 +130,7 @@ static int mpc8xx_pic_host_map(struct irq_host *h, 
> unsigned int virq,
>  
> 
>  static int mpc8xx_pic_host_xlate(struct irq_host *h, struct device_node *ct,
> -			    u32 *intspec, unsigned int intsize,
> +			    const u32 *intspec, unsigned int intsize,
>  			    irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  {
>  	static unsigned char map_pic_senses[4] = {
> diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
> index 30c44e6..1452a4e 100644
> --- a/arch/powerpc/sysdev/mpic.c
> +++ b/arch/powerpc/sysdev/mpic.c
> @@ -994,7 +994,7 @@ static int mpic_host_map(struct irq_host *h, unsigned int 
> virq,
>  }
>  
>  static int mpic_host_xlate(struct irq_host *h, struct device_node *ct,
> -			   u32 *intspec, unsigned int intsize,
> +			   const u32 *intspec, unsigned int intsize,
>  			   irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  
>  {
> diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c 
> b/arch/powerpc/sysdev/qe_lib/qe_ic.c
> index 3faa42e..8192f58 100644
> --- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
> +++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
> @@ -271,7 +271,7 @@ static int qe_ic_host_map(struct irq_host *h, unsigned int 
> virq,
>  }
>  
>  static int qe_ic_host_xlate(struct irq_host *h, struct device_node *ct,
> -			    u32 * intspec, unsigned int intsize,
> +			    const u32 * intspec, unsigned int intsize,
>  			    irq_hw_number_t * out_hwirq,
>  			    unsigned int *out_flags)
>  {
> diff --git a/arch/powerpc/sysdev/tsi108_pci.c 
> b/arch/powerpc/sysdev/tsi108_pci.c
> index cf244a4..a23223d 100644
> --- a/arch/powerpc/sysdev/tsi108_pci.c
> +++ b/arch/powerpc/sysdev/tsi108_pci.c
> @@ -384,7 +384,7 @@ static struct irq_chip tsi108_pci_irq = {
>  };
>  
>  static int pci_irq_host_xlate(struct irq_host *h, struct device_node *ct,
> -			    u32 *intspec, unsigned int intsize,
> +			    const u32 *intspec, unsigned int intsize,
>  			    irq_hw_number_t *out_hwirq, unsigned int *out_flags)
>  {
>  	*out_hwirq = intspec[0];
> diff --git a/arch/powerpc/sysdev/uic.c b/arch/powerpc/sysdev/uic.c
> index 466ce9a..1c315d8 100644
> --- a/arch/powerpc/sysdev/uic.c
> +++ b/arch/powerpc/sysdev/uic.c
> @@ -202,7 +202,7 @@ static int uic_host_map(struct irq_host *h, unsigned int 
> virq,
>  }
>  
>  static int uic_host_xlate(struct irq_host *h, struct device_node *ct,
> -			  u32 *intspec, unsigned int intsize,
> +			  const u32 *intspec, unsigned int intsize,
>  			  irq_hw_number_t *out_hwirq, unsigned int *out_type)
>  
>  {
> diff --git a/arch/powerpc/sysdev/xilinx_intc.c 
> b/arch/powerpc/sysdev/xilinx_intc.c
> index 40edad5..368db32 100644
> --- a/arch/powerpc/sysdev/xilinx_intc.c
> +++ b/arch/powerpc/sysdev/xilinx_intc.c
> @@ -148,7 +148,7 @@ static struct irq_chip xilinx_intc_edge_irqchip = {
>   * xilinx_intc_xlate - translate virq# from device tree interrupts property
>   */
>  static int xilinx_intc_xlate(struct irq_host *h, struct device_node *ct,
> -				u32 *intspec, unsigned int intsize,
> +				const u32 *intspec, unsigned int intsize,
>  				irq_hw_number_t *out_hwirq,
>  				unsigned int *out_flags)
>  {
> -- 
> 1.6.5.3
> 
> 
> Thanks a lot for your help
> 
> 
> Roman
>
Grant Likely - Dec. 11, 2009, 6:07 a.m.
On Tue, Dec 8, 2009 at 7:45 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2009-12-08 at 13:39 +0100, Roman Fietze wrote:
>> Hello,
>>
>> Writing a driver using SCLPC on the MPC5200B I detected, that the
>> intspec arrays to map irqs to Linux virq cannot be const, because the
>> mapping and xlate functions only take non const pointers. All those
>> functions do not modify the intspec, so a const pointer could be used.
>>
>> I tried to compile that patch on different platforms and got no
>> compiler errors, so I'd like to ask you to review that patch, and
>> check if it's worth using it.
>
> Looks good.

Okay by me too.

Acked-by: Grant Likely <grant.likely@secretlab.ca>

g.
Grant Likely - Dec. 11, 2009, 6:13 a.m.
On Tue, Dec 8, 2009 at 5:39 AM, Roman Fietze <roman.fietze@telemotive.de> wrote:
> Hello,
>
> Writing a driver using SCLPC on the MPC5200B I detected, that the
> intspec arrays to map irqs to Linux virq cannot be const, because the
> mapping and xlate functions only take non const pointers. All those
> functions do not modify the intspec, so a const pointer could be used.

BTW, if you're interested, there is a driver for the SCLPC FIFO about
to be merged into 2.6.33.  It's in Ben's tree waiting to be pulled
into mainline.

Cheers,
g.
Roman Fietze - Dec. 15, 2009, 10:59 a.m.
Hallo Grant,

On Friday 11 December 2009 07:13:45 Grant Likely wrote:

> BTW, if you're interested, there is a driver for the SCLPC FIFO about
> to be merged into 2.6.33.  It's in Ben's tree waiting to be pulled
> into mainline.

I've had a "look" into it. This means I rewrote it in some parts to
get it running.

First I tried to do it in nice little steps, so the git commits are
clean and simple. But with my knowldege about the SCLPC, the code your
old SCLPC test driver and my 2.4.25 FPGA driver using SCLPS and
BestComm, one of the commits just got a big rewrite including fixes
for quite some bugs.

The driver is now running using DMA TX and RX. Without BestComm it's
running partly. I'm really windering if this code ever ran before?
There are some historical or future items in there like measurement of
the times, a list_header maybe for future request queueing, and so on.

If you or Ben are interested in my work I can post the patches
here. Of course just to get some comments, because the driver can't be
ready in the state it is. The commits would go on top of Ben's next
branch.


Roman
Grant Likely - Dec. 15, 2009, 7:50 p.m.
On Tue, Dec 15, 2009 at 3:59 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
> Hallo Grant,
>
> On Friday 11 December 2009 07:13:45 Grant Likely wrote:
>
>> BTW, if you're interested, there is a driver for the SCLPC FIFO about
>> to be merged into 2.6.33.  It's in Ben's tree waiting to be pulled
>> into mainline.
>
> I've had a "look" into it. This means I rewrote it in some parts to
> get it running.
>
> First I tried to do it in nice little steps, so the git commits are
> clean and simple. But with my knowldege about the SCLPC, the code your
> old SCLPC test driver and my 2.4.25 FPGA driver using SCLPS and
> BestComm, one of the commits just got a big rewrite including fixes
> for quite some bugs.
>
> The driver is now running using DMA TX and RX. Without BestComm it's
> running partly. I'm really windering if this code ever ran before?
> There are some historical or future items in there like measurement of
> the times, a list_header maybe for future request queueing, and so on

Yes, I'm using the driver in a couple of projects.  It works for me
for both RX and TX (although TX+DMA has been troublesome).  I'll
double check to make sure I've merged all of my patches for the
driver.

The test driver on the other hand is pretty poor code.  Don't expect
much from it other than some hints.  There's a reason I didn't merge
that chunk.

> If you or Ben are interested in my work I can post the patches
> here. Of course just to get some comments, because the driver can't be
> ready in the state it is. The commits would go on top of Ben's next
> branch.

Yes, please post the patches and cc: me.  I'll review, test, and make comments.

g.
Roman Fietze - Dec. 17, 2009, 12:55 p.m.
Hello Grant,

On Tuesday 15 December 2009 20:50:05 Grant Likely wrote:

> Yes, I'm using the driver in a couple of projects.  It works for me
> for both RX and TX (although TX+DMA has been troublesome).

That's what I found out.

> The test driver on the other hand is pretty poor code.  Don't expect
> much from it other than some hints.  There's a reason I didn't merge
> that chunk.

I brought it up to speed before using it.

> Yes, please post the patches and cc: me. I'll review, test, and make
> comments.

I hope it's ok that I've copied them to my web space instead of
providing patches in a mail. The URL is:

  http://www.fietze-home.de/telemotive/linux-2.6-telemotive-mpc.git

Branches:

  benh-next-lpbfifo
    the modified version of the LocalPlus platform driver

  benh-next-localplus-test
    your old test driver, now using the platform driver


Both branches are on top of the benh next branch.


I could only test the driver using fifo and bcom mode in read mode,
using your old test driver, because I currently do not yet have a
target with a writeable device on the LocalPlus. The test ran fine
using different transer sizes starting at 4 up to 128KiB.

I will now port our FPGA driver to use the platform driver, which can
deliver a very high load, and supports writing.

The biggest problem using DMA is the unpredictable order of arrival of
the two interrupts. They both depend on the appropriate load of the
system generating them. In the case of the BestComm a parallel load on
e.g. FEC or ATA can delay the bcom gen bd interrupt until it's task
gets scheduled again, which can take "some time" due its low BestComm
prio of 2. The second problem was, that the original driver does not
use the flush bit to avoid stale RX data.


Roman
Grant Likely - Dec. 22, 2009, 12:11 a.m.
On Thu, Dec 17, 2009 at 5:55 AM, Roman Fietze
<roman.fietze@telemotive.de> wrote:
> Hello Grant,
>
> On Tuesday 15 December 2009 20:50:05 Grant Likely wrote:
>
>> Yes, I'm using the driver in a couple of projects.  It works for me
>> for both RX and TX (although TX+DMA has been troublesome).
>
> That's what I found out.
>
>> The test driver on the other hand is pretty poor code.  Don't expect
>> much from it other than some hints.  There's a reason I didn't merge
>> that chunk.
>
> I brought it up to speed before using it.
>
>> Yes, please post the patches and cc: me. I'll review, test, and make
>> comments.
>
> I hope it's ok that I've copied them to my web space instead of
> providing patches in a mail. The URL is:
>
>  http://www.fietze-home.de/telemotive/linux-2.6-telemotive-mpc.git

"Access Forbidden!"

It is easier for me though if you post them to the list.  Then I can
just hit "reply to all" and make comments on the code.

Cheers,
g.
Roman Fietze - Dec. 22, 2009, 6:55 a.m.
Hello Grant,

On Tuesday 22 December 2009 01:11:30 Grant Likely wrote:

> "Access Forbidden!"

git-fetch should have worked. It's a git tree prepared using
git-update-server-info.

> It is easier for me though if you post them to the list. Then I can
> just hit "reply to all" and make comments on the code.

Ok, I'll do so. I hope I've understood what
http://www.kernel.org/pub/linux/docs/lkml/ says about creating
suitable patches for mailing lists.


Patch description:

This is a series of patches on top of the benh next branch modifying
the platform LocalPlus driver for the MPC5200B. This includes
formatting changes in the first step, switching to a structure
describing the SCLPC registers, making DMA work, making unloading the
module work and so on.


Roman

Patch

diff --git a/arch/powerpc/include/asm/irq.h b/arch/powerpc/include/asm/irq.h
index bbcd1aa..a0bcafc 100644
--- a/arch/powerpc/include/asm/irq.h
+++ b/arch/powerpc/include/asm/irq.h
@@ -99,7 +99,7 @@  struct irq_host_ops {
 	 * interrupt controller has for that line)
 	 */
 	int (*xlate)(struct irq_host *h, struct device_node *ctrler,
-		     u32 *intspec, unsigned int intsize,
+		     const u32 *intspec, unsigned int intsize,
 		     irq_hw_number_t *out_hwirq, unsigned int *out_type);
 };
 
@@ -313,7 +313,7 @@  extern void irq_free_virt(unsigned int virq, unsigned int 
count);
  * of the of_irq_map_*() functions.
  */
 extern unsigned int irq_create_of_mapping(struct device_node *controller,
-					  u32 *intspec, unsigned int intsize);
+					  const u32 *intspec, unsigned int intsize);
 
 /**
  * irq_of_parse_and_map - Parse and Map an interrupt into linux virq space
diff --git a/arch/powerpc/kernel/irq.c b/arch/powerpc/kernel/irq.c
index e5d1211..6e294e0 100644
--- a/arch/powerpc/kernel/irq.c
+++ b/arch/powerpc/kernel/irq.c
@@ -699,7 +699,7 @@  unsigned int irq_create_mapping(struct irq_host *host,
 EXPORT_SYMBOL_GPL(irq_create_mapping);
 
 unsigned int irq_create_of_mapping(struct device_node *controller,
-				   u32 *intspec, unsigned int intsize)
+				   const u32 *intspec, unsigned int intsize)
 {
 	struct irq_host *host;
 	irq_hw_number_t hwirq;
diff --git a/arch/powerpc/platforms/52xx/media5200.c 
b/arch/powerpc/platforms/52xx/media5200.c
index 68e4f16..b1ec9bb 100644
--- a/arch/powerpc/platforms/52xx/media5200.c
+++ b/arch/powerpc/platforms/52xx/media5200.c
@@ -127,7 +127,7 @@  static int media5200_irq_map(struct irq_host *h, unsigned 
int virq,
 }
 
 static int media5200_irq_xlate(struct irq_host *h, struct device_node *ct,
-				 u32 *intspec, unsigned int intsize,
+				 const u32 *intspec, unsigned int intsize,
 				 irq_hw_number_t *out_hwirq,
 				 unsigned int *out_flags)
 {
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c 
b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
index bfbcd41..78f0ab6 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_gpt.c
@@ -182,7 +182,7 @@  static int mpc52xx_gpt_irq_map(struct irq_host *h, 
unsigned int virq,
 }
 
 static int mpc52xx_gpt_irq_xlate(struct irq_host *h, struct device_node *ct,
-				 u32 *intspec, unsigned int intsize,
+				 const u32 *intspec, unsigned int intsize,
 				 irq_hw_number_t *out_hwirq,
 				 unsigned int *out_flags)
 {
diff --git a/arch/powerpc/platforms/52xx/mpc52xx_pic.c 
b/arch/powerpc/platforms/52xx/mpc52xx_pic.c
index 480f806..53d44f6 100644
--- a/arch/powerpc/platforms/52xx/mpc52xx_pic.c
+++ b/arch/powerpc/platforms/52xx/mpc52xx_pic.c
@@ -355,7 +355,7 @@  static int mpc52xx_is_extirq(int l1, int l2)
  * mpc52xx_irqhost_xlate - translate virq# from device tree interrupts 
property
  */
 static int mpc52xx_irqhost_xlate(struct irq_host *h, struct device_node *ct,
-				 u32 *intspec, unsigned int intsize,
+				 const u32 *intspec, unsigned int intsize,
 				 irq_hw_number_t *out_hwirq,
 				 unsigned int *out_flags)
 {
diff --git a/arch/powerpc/platforms/85xx/socrates_fpga_pic.c 
b/arch/powerpc/platforms/85xx/socrates_fpga_pic.c
index 60edf63..380858a 100644
--- a/arch/powerpc/platforms/85xx/socrates_fpga_pic.c
+++ b/arch/powerpc/platforms/85xx/socrates_fpga_pic.c
@@ -253,7 +253,7 @@  static int socrates_fpga_pic_host_map(struct irq_host *h, 
unsigned int virq,
 }
 
 static int socrates_fpga_pic_host_xlate(struct irq_host *h,
-		struct device_node *ct,	u32 *intspec, unsigned int intsize,
+		struct device_node *ct,	const u32 *intspec, unsigned int intsize,
 		irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 {
 	struct socrates_fpga_irq_info *fpga_irq = &fpga_irqs[intspec[0]];
diff --git a/arch/powerpc/platforms/86xx/gef_pic.c 
b/arch/powerpc/platforms/86xx/gef_pic.c
index 50d0a2b..8d4479f 100644
--- a/arch/powerpc/platforms/86xx/gef_pic.c
+++ b/arch/powerpc/platforms/86xx/gef_pic.c
@@ -170,7 +170,7 @@  static int gef_pic_host_map(struct irq_host *h, unsigned 
int virq,
 }
 
 static int gef_pic_host_xlate(struct irq_host *h, struct device_node *ct,
-			    u32 *intspec, unsigned int intsize,
+			    const u32 *intspec, unsigned int intsize,
 			    irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 {
 
diff --git a/arch/powerpc/platforms/cell/beat_interrupt.c 
b/arch/powerpc/platforms/cell/beat_interrupt.c
index 7225484..1f2fd65 100644
--- a/arch/powerpc/platforms/cell/beat_interrupt.c
+++ b/arch/powerpc/platforms/cell/beat_interrupt.c
@@ -166,11 +166,11 @@  static void beatic_pic_host_remap(struct irq_host *h, 
unsigned int virq,
  * Note: We have only 1 entry to translate.
  */
 static int beatic_pic_host_xlate(struct irq_host *h, struct device_node *ct,
-				 u32 *intspec, unsigned int intsize,
+				 const u32 *intspec, unsigned int intsize,
 				 irq_hw_number_t *out_hwirq,
 				 unsigned int *out_flags)
 {
-	u64 *intspec2 = (u64 *)intspec;
+	const u64 *intspec2 = (const u64 *)intspec;
 
 	*out_hwirq = *intspec2;
 	*out_flags |= IRQ_TYPE_LEVEL_LOW;
diff --git a/arch/powerpc/platforms/cell/interrupt.c 
b/arch/powerpc/platforms/cell/interrupt.c
index 882e470..d9d33cb 100644
--- a/arch/powerpc/platforms/cell/interrupt.c
+++ b/arch/powerpc/platforms/cell/interrupt.c
@@ -297,7 +297,7 @@  static int iic_host_map(struct irq_host *h, unsigned int 
virq,
 }
 
 static int iic_host_xlate(struct irq_host *h, struct device_node *ct,
-			   u32 *intspec, unsigned int intsize,
+			   const u32 *intspec, unsigned int intsize,
 			   irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 
 {
diff --git a/arch/powerpc/platforms/cell/spider-pic.c 
b/arch/powerpc/platforms/cell/spider-pic.c
index 4e56556..37c2e4e 100644
--- a/arch/powerpc/platforms/cell/spider-pic.c
+++ b/arch/powerpc/platforms/cell/spider-pic.c
@@ -187,7 +187,7 @@  static int spider_host_map(struct irq_host *h, unsigned 
int virq,
 }
 
 static int spider_host_xlate(struct irq_host *h, struct device_node *ct,
-			   u32 *intspec, unsigned int intsize,
+			   const u32 *intspec, unsigned int intsize,
 			   irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 
 {
diff --git a/arch/powerpc/platforms/powermac/pic.c 
b/arch/powerpc/platforms/powermac/pic.c
index d212006..da6b4b9 100644
--- a/arch/powerpc/platforms/powermac/pic.c
+++ b/arch/powerpc/platforms/powermac/pic.c
@@ -303,7 +303,7 @@  static int pmac_pic_host_map(struct irq_host *h, unsigned 
int virq,
 }
 
 static int pmac_pic_host_xlate(struct irq_host *h, struct device_node *ct,
-			       u32 *intspec, unsigned int intsize,
+			       const u32 *intspec, unsigned int intsize,
 			       irq_hw_number_t *out_hwirq,
 			       unsigned int *out_flags)
 
diff --git a/arch/powerpc/platforms/pseries/xics.c 
b/arch/powerpc/platforms/pseries/xics.c
index b9bf0ee..1821eae 100644
--- a/arch/powerpc/platforms/pseries/xics.c
+++ b/arch/powerpc/platforms/pseries/xics.c
@@ -434,7 +434,7 @@  static int xics_host_map(struct irq_host *h, unsigned int 
virq,
 }
 
 static int xics_host_xlate(struct irq_host *h, struct device_node *ct,
-			   u32 *intspec, unsigned int intsize,
+			   const u32 *intspec, unsigned int intsize,
 			   irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 
 {
diff --git a/arch/powerpc/sysdev/cpm2_pic.c b/arch/powerpc/sysdev/cpm2_pic.c
index 78f1f7c..3d2be56 100644
--- a/arch/powerpc/sysdev/cpm2_pic.c
+++ b/arch/powerpc/sysdev/cpm2_pic.c
@@ -216,7 +216,7 @@  static int cpm2_pic_host_map(struct irq_host *h, unsigned 
int virq,
 }
 
 static int cpm2_pic_host_xlate(struct irq_host *h, struct device_node *ct,
-			    u32 *intspec, unsigned int intsize,
+			    const u32 *intspec, unsigned int intsize,
 			    irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 {
 	*out_hwirq = intspec[0];
diff --git a/arch/powerpc/sysdev/i8259.c b/arch/powerpc/sysdev/i8259.c
index a96584a..ccc9a3b 100644
--- a/arch/powerpc/sysdev/i8259.c
+++ b/arch/powerpc/sysdev/i8259.c
@@ -198,7 +198,7 @@  static void i8259_host_unmap(struct irq_host *h, unsigned 
int virq)
 }
 
 static int i8259_host_xlate(struct irq_host *h, struct device_node *ct,
-			    u32 *intspec, unsigned int intsize,
+			    const u32 *intspec, unsigned int intsize,
 			    irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 {
 	static unsigned char map_isa_senses[4] = {
diff --git a/arch/powerpc/sysdev/ipic.c b/arch/powerpc/sysdev/ipic.c
index cb7689c..27259f4 100644
--- a/arch/powerpc/sysdev/ipic.c
+++ b/arch/powerpc/sysdev/ipic.c
@@ -697,7 +697,7 @@  static int ipic_host_map(struct irq_host *h, unsigned int 
virq,
 }
 
 static int ipic_host_xlate(struct irq_host *h, struct device_node *ct,
-			   u32 *intspec, unsigned int intsize,
+			   const u32 *intspec, unsigned int intsize,
 			   irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 
 {
diff --git a/arch/powerpc/sysdev/mpc8xx_pic.c 
b/arch/powerpc/sysdev/mpc8xx_pic.c
index 5d2d552..72544a1 100644
--- a/arch/powerpc/sysdev/mpc8xx_pic.c
+++ b/arch/powerpc/sysdev/mpc8xx_pic.c
@@ -130,7 +130,7 @@  static int mpc8xx_pic_host_map(struct irq_host *h, 
unsigned int virq,
 
 
 static int mpc8xx_pic_host_xlate(struct irq_host *h, struct device_node *ct,
-			    u32 *intspec, unsigned int intsize,
+			    const u32 *intspec, unsigned int intsize,
 			    irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 {
 	static unsigned char map_pic_senses[4] = {
diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
index 30c44e6..1452a4e 100644
--- a/arch/powerpc/sysdev/mpic.c
+++ b/arch/powerpc/sysdev/mpic.c
@@ -994,7 +994,7 @@  static int mpic_host_map(struct irq_host *h, unsigned int 
virq,
 }
 
 static int mpic_host_xlate(struct irq_host *h, struct device_node *ct,
-			   u32 *intspec, unsigned int intsize,
+			   const u32 *intspec, unsigned int intsize,
 			   irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 
 {
diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c 
b/arch/powerpc/sysdev/qe_lib/qe_ic.c
index 3faa42e..8192f58 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
@@ -271,7 +271,7 @@  static int qe_ic_host_map(struct irq_host *h, unsigned int 
virq,
 }
 
 static int qe_ic_host_xlate(struct irq_host *h, struct device_node *ct,
-			    u32 * intspec, unsigned int intsize,
+			    const u32 * intspec, unsigned int intsize,
 			    irq_hw_number_t * out_hwirq,
 			    unsigned int *out_flags)
 {
diff --git a/arch/powerpc/sysdev/tsi108_pci.c 
b/arch/powerpc/sysdev/tsi108_pci.c
index cf244a4..a23223d 100644
--- a/arch/powerpc/sysdev/tsi108_pci.c
+++ b/arch/powerpc/sysdev/tsi108_pci.c
@@ -384,7 +384,7 @@  static struct irq_chip tsi108_pci_irq = {
 };
 
 static int pci_irq_host_xlate(struct irq_host *h, struct device_node *ct,
-			    u32 *intspec, unsigned int intsize,
+			    const u32 *intspec, unsigned int intsize,
 			    irq_hw_number_t *out_hwirq, unsigned int *out_flags)
 {
 	*out_hwirq = intspec[0];
diff --git a/arch/powerpc/sysdev/uic.c b/arch/powerpc/sysdev/uic.c
index 466ce9a..1c315d8 100644
--- a/arch/powerpc/sysdev/uic.c
+++ b/arch/powerpc/sysdev/uic.c
@@ -202,7 +202,7 @@  static int uic_host_map(struct irq_host *h, unsigned int 
virq,
 }
 
 static int uic_host_xlate(struct irq_host *h, struct device_node *ct,
-			  u32 *intspec, unsigned int intsize,
+			  const u32 *intspec, unsigned int intsize,
 			  irq_hw_number_t *out_hwirq, unsigned int *out_type)
 
 {
diff --git a/arch/powerpc/sysdev/xilinx_intc.c 
b/arch/powerpc/sysdev/xilinx_intc.c
index 40edad5..368db32 100644
--- a/arch/powerpc/sysdev/xilinx_intc.c
+++ b/arch/powerpc/sysdev/xilinx_intc.c
@@ -148,7 +148,7 @@  static struct irq_chip xilinx_intc_edge_irqchip = {
  * xilinx_intc_xlate - translate virq# from device tree interrupts property
  */
 static int xilinx_intc_xlate(struct irq_host *h, struct device_node *ct,
-				u32 *intspec, unsigned int intsize,
+				const u32 *intspec, unsigned int intsize,
 				irq_hw_number_t *out_hwirq,
 				unsigned int *out_flags)
 {