Re: Continual reading from the PowerPc time base register is not stable
Hi guys In fact my problem is gettimeofday cannot return right value sometimes, and this will bring instability to our system software. You can find a law from the log that there is a 17592 seconds' shift every time error occurs. 2010/3/26 Csdncannon csdncan...@gmail.com Yes, the missing 64-bit conversion is the key problem, I will try removing isync later. Thanks for your support. 2010/3/26 Segher Boessenkool seg...@kernel.crashing.org Yes indeed. Could you post the relevant piece if disassembly from your original binary (the one that has the problem)? Or send me the binary (not to the mailing list), I'll do it then. Ah scratch that. I compiled your original code (after fixing the compile errors -- there is no such type as bool in C). The problem is that (upper 32) | lower thing. upper is a 32-bit type, so shifting it by 32 or more bits is undefined. GCC compiles this to (shortened): 0: mftbu 9 ; mftbl 11 ; mftbu 0 ; cmpw 0,9 ; bne 0b # so far so good slwi 0,0,0 ; or 4,0,11 ; li 3,0 ; blr so it shifts by 0, i.e. it does upper | lower . Case closed, no hardware problem :-) Segher gettime.c Description: Binary data gettime.log Description: Binary data ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 4/7] hvc_console: Fix race between hvc_close and hvc_remove
Hi,
Looking at the commit e74d098c66543d0731de62eb747ccd5b636a6f4c,
i see that for every tty_kref_get() there is a corresponding
tty_kref_put() except maybe for the one in the following patch snippet
spin_lock_irqsave(hp-lock, flags);
/* Check and then increment for fast path open. */
if (hp-count++ 0) {
+ tty_kref_get(tty);
spin_unlock_irqrestore(hp-lock, flags);
hvc_kick();
return 0;
I don't know this code very well but we might be missing a
corresponding tty_kref_put() some place ?
See hvc_hangup:
temp_open_count = hp-count;
...
while(temp_open_count) {
--temp_open_count;
tty_kref_put(tty);
kref_put(hp-kref, destroy_hvc_struct);
}
I don't claim to understand the tty layer, but it seems like hvc_open and
hvc_close should be balanced in their kref reference counting.
Right now we get a kref every call to hvc_open:
if (hp-count++ 0) {
tty_kref_get(tty); - here
spin_unlock_irqrestore(hp-lock, flags);
hvc_kick();
return 0;
} /* else count == 0 */
tty-driver_data = hp;
hp-tty = tty_kref_get(tty); -- or here if hp-count was 0
But hvc_close has:
tty_kref_get(tty);
if (--hp-count == 0) {
...
/* Put the ref obtained in hvc_open() */
tty_kref_put(tty);
...
}
tty_kref_put(tty);
Since the outside kref get/put balance we only do a single kref_put when
count reaches 0.
The patch below changes things to call tty_kref_put once for every
hvc_close call, and with that my machine boots fine.
Signed-off-by: Anton Blanchard an...@samba.org
---
diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
index d3890e8..35cca4c 100644
--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c
@@ -368,16 +368,12 @@ static void hvc_close(struct tty_struct *tty, struct file
* filp)
hp = tty-driver_data;
spin_lock_irqsave(hp-lock, flags);
- tty_kref_get(tty);
if (--hp-count == 0) {
/* We are done with the tty pointer now. */
hp-tty = NULL;
spin_unlock_irqrestore(hp-lock, flags);
- /* Put the ref obtained in hvc_open() */
- tty_kref_put(tty);
-
if (hp-ops-notifier_del)
hp-ops-notifier_del(hp, hp-data);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 4/7] hvc_console: Fix race between hvc_close and hvc_remove
On (Tue) Apr 06 2010 [21:42:38], Anton Blanchard wrote:
Hi,
Looking at the commit e74d098c66543d0731de62eb747ccd5b636a6f4c,
i see that for every tty_kref_get() there is a corresponding
tty_kref_put() except maybe for the one in the following patch snippet
spin_lock_irqsave(hp-lock, flags);
/* Check and then increment for fast path open. */
if (hp-count++ 0) {
+ tty_kref_get(tty);
spin_unlock_irqrestore(hp-lock, flags);
hvc_kick();
return 0;
I don't know this code very well but we might be missing a
corresponding tty_kref_put() some place ?
See hvc_hangup:
temp_open_count = hp-count;
...
while(temp_open_count) {
--temp_open_count;
tty_kref_put(tty);
kref_put(hp-kref, destroy_hvc_struct);
}
I don't claim to understand the tty layer, but it seems like hvc_open and
hvc_close should be balanced in their kref reference counting.
Right now we get a kref every call to hvc_open:
if (hp-count++ 0) {
tty_kref_get(tty); - here
spin_unlock_irqrestore(hp-lock, flags);
hvc_kick();
return 0;
} /* else count == 0 */
tty-driver_data = hp;
hp-tty = tty_kref_get(tty); -- or here if hp-count was 0
But hvc_close has:
tty_kref_get(tty);
if (--hp-count == 0) {
...
/* Put the ref obtained in hvc_open() */
tty_kref_put(tty);
...
}
tty_kref_put(tty);
Since the outside kref get/put balance we only do a single kref_put when
count reaches 0.
OK, makes sense and since it works for you,
Acked-by: Amit Shah amit.s...@redhat.com
The patch below changes things to call tty_kref_put once for every
hvc_close call, and with that my machine boots fine.
Signed-off-by: Anton Blanchard an...@samba.org
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 4/7] hvc_console: Fix race between hvc_close and hvc_remove
Anton Blanchard wrote:
The patch below changes things to call tty_kref_put once for every
hvc_close call, and with that my machine boots fine.
Signed-off-by: Anton Blanchard an...@samba.org
---
Works for me. Thanks Anton.
Regards
-Sachin
diff --git a/drivers/char/hvc_console.c b/drivers/char/hvc_console.c
index d3890e8..35cca4c 100644
--- a/drivers/char/hvc_console.c
+++ b/drivers/char/hvc_console.c
@@ -368,16 +368,12 @@ static void hvc_close(struct tty_struct *tty, struct file
* filp)
hp = tty-driver_data;
spin_lock_irqsave(hp-lock, flags);
- tty_kref_get(tty);
if (--hp-count == 0) {
/* We are done with the tty pointer now. */
hp-tty = NULL;
spin_unlock_irqrestore(hp-lock, flags);
- /* Put the ref obtained in hvc_open() */
- tty_kref_put(tty);
-
if (hp-ops-notifier_del)
hp-ops-notifier_del(hp, hp-data);
--
-
Sachin Sant
IBM Linux Technology Center
India Systems and Technology Labs
Bangalore, India
-
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 4/7] hvc_console: Fix race between hvc_close and hvc_remove
I don't claim to understand the tty layer, but it seems like hvc_open and hvc_close should be balanced in their kref reference counting. They should yes. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
On Mon, 2010-04-05 at 15:33 -0600, John Linn wrote: +/* Align the IP data in the packet on word boundaries as MicroBlaze + * needs it. + */ + #define XTE_ALIGN 32 -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN) +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN) Very interesting way of doing this, but why such convoluted thing ? Grant might have insight into why this started this way, I just updated to help with MicroBlaze alignment. Because of the % 32, this is equivalent to : #define BUFFER_ALIGN(adr) ((2 - ((u32) adr)) % XTE_ALIGN) But wait, dont we recognise the magic constant NET_IP_ALIGN ? So, I ask, cant you use netdev_alloc_skb_ip_align() in this driver ? That should work. I switched to that in the older xilinx_lltemac driver without any problem. Steven J. Magnani I claim this network for MARS! www.digidescorp.com Earthling, return my space modulator! #include standard.disclaimer ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] 460EX on-chip SATA driverkernel 2.6.33 resubmission : 02
On 04/06/2010 07:41 AM, Rupjyoti Sarmah wrote:
General comment: remove inline and let the compiler select those
functions that need it.
+struct sata_dwc_host_priv {
+
+ void __iomem *scr_addr_sstatus;
+ u32 sata_dwc_sactive_issued;
+ u32 sata_dwc_sactive_queued;
+ u32 dma_interrupt_count;
+ struct ahb_dma_regs *sata_dma_regs;
+ struct device *dwc_dev;
+
+};
use proper indentation (separate type from member name with tabs)
+struct sata_dwc_host_priv host_pvt;
+
+/*
+ * Prototypes
+ */
+static void sata_dwc_bmdma_start_by_tag(struct ata_queued_cmd *qc, u8 tag);
+static int sata_dwc_qc_complete(struct ata_port *ap, struct ata_queued_cmd *qc,
+ u32 check_status);
+static void sata_dwc_dma_xfer_complete(struct ata_port *ap, u32 check_status);
+static void sata_dwc_port_stop(struct ata_port *ap);
+static void sata_dwc_clear_dmacr(struct sata_dwc_device_port *hsdevp, u8 tag);
+
+static int dma_dwc_init(struct sata_dwc_device *hsdev, int irq);
+static void dma_dwc_exit(struct sata_dwc_device *hsdev);
+static int dma_dwc_xfer_setup(struct scatterlist *sg, int num_elems,
+ struct lli *lli, dma_addr_t dma_lli,
+ void __iomem *addr, int dir);
+static void dma_dwc_xfer_start(int dma_ch);
+
+static const char *dir_2_txt(enum dma_data_direction dir)
+{
+ switch (dir) {
+ case DMA_BIDIRECTIONAL:
+ return bi;
+ case DMA_FROM_DEVICE:
+ return from;
+ case DMA_TO_DEVICE:
+ return to;
+ case DMA_NONE:
+ return none;
+ default:
+ return err;
+ }
+}
+
+static const char *prot_2_txt(enum ata_tf_protocols protocol)
+{
+ switch (protocol) {
+ case ATA_PROT_UNKNOWN:
+ return unknown;
+ case ATA_PROT_NODATA:
+ return nodata;
+ case ATA_PROT_PIO:
+ return pio;
+ case ATA_PROT_DMA:
+ return dma;
+ case ATA_PROT_NCQ:
+ return ncq;
+ case ATAPI_PROT_PIO:
+ return atapi pio;
+ case ATAPI_PROT_NODATA:
+ return atapi nodata;
+ case ATAPI_PROT_DMA:
+ return atapi dma;
+ default:
+ return err;
+ }
+}
+
+inline const char *ata_cmd_2_txt(const struct ata_taskfile *tf)
+{
+ switch (tf-command) {
+ case ATA_CMD_CHK_POWER:
+ return ATA_CMD_CHK_POWER;
+ case ATA_CMD_EDD:
+ return ATA_CMD_EDD;
+ case ATA_CMD_FLUSH:
+ return ATA_CMD_FLUSH;
+ case ATA_CMD_FLUSH_EXT:
+ return ATA_CMD_FLUSH_EXT;
+ case ATA_CMD_ID_ATA:
+ return ATA_CMD_ID_ATA;
+ case ATA_CMD_ID_ATAPI:
+ return ATA_CMD_ID_ATAPI;
+ case ATA_CMD_FPDMA_READ:
+ return ATA_CMD_FPDMA_READ;
+ case ATA_CMD_FPDMA_WRITE:
+ return ATA_CMD_FPDMA_WRITE;
+ case ATA_CMD_READ:
+ return ATA_CMD_READ;
+ case ATA_CMD_READ_EXT:
+ return ATA_CMD_READ_EXT;
+ case ATA_CMD_READ_NATIVE_MAX_EXT:
+ return ATA_CMD_READ_NATIVE_MAX_EXT;
+ case ATA_CMD_VERIFY_EXT:
+ return ATA_CMD_VERIFY_EXT;
+ case ATA_CMD_WRITE:
+ return ATA_CMD_WRITE;
+ case ATA_CMD_WRITE_EXT:
+ return ATA_CMD_WRITE_EXT;
+ case ATA_CMD_PIO_READ:
+ return ATA_CMD_PIO_READ;
+ case ATA_CMD_PIO_READ_EXT:
+ return ATA_CMD_PIO_READ_EXT;
+ case ATA_CMD_PIO_WRITE:
+ return ATA_CMD_PIO_WRITE;
+ case ATA_CMD_PIO_WRITE_EXT:
+ return ATA_CMD_PIO_WRITE_EXT;
+ case ATA_CMD_SET_FEATURES:
+ return ATA_CMD_SET_FEATURES;
+ case ATA_CMD_PACKET:
+ return ATA_CMD_PACKET;
+ default:
+ return ATA_CMD_???;
+ }
use ata_get_cmd_descript() rather than duplicating it
+static irqreturn_t dma_dwc_interrupt(int irq, void *hsdev_instance)
+{
+ int chan;
+ u32 tfr_reg, err_reg;
+
+ struct sata_dwc_device *hsdev =
+ (struct sata_dwc_device *) hsdev_instance;
+ struct ata_host *host = (struct ata_host *) hsdev-host;
+ struct ata_port *ap;
+ struct sata_dwc_device_port *hsdevp;
+ u8 tag = 0;
+ unsigned int port = 0;
+ struct sata_dwc_host_priv *hp;
+ hp = kmalloc(sizeof(*hp), GFP_KERNEL);
1) interrupt is not GFP_KERNEL
2) you must failure kmalloc failure
3) it is not clear to me where you initialize this structure???
+ spin_lock(host-lock);
+
+ ap = host-ports[port];
+ hsdevp = HSDEVP_FROM_AP(ap);
+ tag = ap-link.active_tag;
+
+ tfr_reg = in_le32((host_pvt.sata_dma_regs-interrupt_status.tfr.low));
+ err_reg = in_le32((host_pvt.sata_dma_regs- \
+
RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
-Original Message- From: Eric Dumazet [mailto:eric.duma...@gmail.com] Sent: Monday, April 05, 2010 3:30 PM To: John Linn Cc: net...@vger.kernel.org; linuxppc-...@ozlabs.org; grant.lik...@secretlab.ca; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver Le lundi 05 avril 2010 à 15:11 -0600, John Linn a écrit : This patch adds support for using the LL TEMAC Ethernet driver on non-Virtex 5 platforms by adding support for accessing the Soft DMA registers as if they were memory mapped instead of solely through the DCR's (available on the Virtex 5). The patch also updates the driver so that it runs on the MicroBlaze. The changes were tested on the PowerPC 440, PowerPC 405, and the MicroBlaze platforms. Signed-off-by: John Tyner jty...@cs.ucr.edu Signed-off-by: John Linn john.l...@xilinx.com --- +/* Align the IP data in the packet on word boundaries as MicroBlaze + * needs it. + */ + #define XTE_ALIGN 32 -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN) +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN) Very interesting way of doing this, but why such convoluted thing ? This is trying to align for a cache line (32 bytes) before my change. My change was then also making it align the IP data on a word boundary. Because of the % 32, this is equivalent to : #define BUFFER_ALIGN(adr) ((2 - ((u32) adr)) % XTE_ALIGN) Yes, but I'm not sure that's clearer IMHO. But wait, dont we recognise the magic constant NET_IP_ALIGN ? Yes it could be used. I'm struggling with how to make this all be clearer. How about this? #define BUFFER_ALIGN(adr) (((XTE_ALIGN + NET_IP_ALIGN) - ((u32) adr)) % XTE_ALIGN) So, I ask, cant you use netdev_alloc_skb_ip_align() in this driver ? From what I can tell, this wouldn't work as it only reserves the 2 bytes to align with a word boundary. Thanks, John This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
Le mardi 06 avril 2010 à 10:12 -0600, John Linn a écrit : -Original Message- From: Eric Dumazet [mailto:eric.duma...@gmail.com] Sent: Monday, April 05, 2010 3:30 PM To: John Linn Cc: net...@vger.kernel.org; linuxppc-...@ozlabs.org; grant.lik...@secretlab.ca; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver Le lundi 05 avril 2010 à 15:11 -0600, John Linn a écrit : This patch adds support for using the LL TEMAC Ethernet driver on non-Virtex 5 platforms by adding support for accessing the Soft DMA registers as if they were memory mapped instead of solely through the DCR's (available on the Virtex 5). The patch also updates the driver so that it runs on the MicroBlaze. The changes were tested on the PowerPC 440, PowerPC 405, and the MicroBlaze platforms. Signed-off-by: John Tyner jty...@cs.ucr.edu Signed-off-by: John Linn john.l...@xilinx.com --- +/* Align the IP data in the packet on word boundaries as MicroBlaze + * needs it. + */ + #define XTE_ALIGN 32 -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN) +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN) Very interesting way of doing this, but why such convoluted thing ? This is trying to align for a cache line (32 bytes) before my change. My change was then also making it align the IP data on a word boundary. Because of the % 32, this is equivalent to : #define BUFFER_ALIGN(adr) ((2 - ((u32) adr)) % XTE_ALIGN) Yes, but I'm not sure that's clearer IMHO. But wait, dont we recognise the magic constant NET_IP_ALIGN ? Yes it could be used. I'm struggling with how to make this all be clearer. I am not saying its clearer, I am saying we have a standard way to handle this exact problem (aligning rcvs buffer so that IP header is aligned) There is no need to invent new ones, this makes reviewing of this driver more difficult. How about this? #define BUFFER_ALIGN(adr) (((XTE_ALIGN + NET_IP_ALIGN) - ((u32) adr)) % XTE_ALIGN) Sorry, I still dont understand why you need XTE_ALIGN + ... ((A + B) - C) % A is equal to (B - C) % A Which one is more readable ? Please take a look at existing and clean code, no magic macro, and we can understand the intention. find drivers/net | xargs grep -n netdev_alloc_skb_ip_align ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
-Original Message- From: Eric Dumazet [mailto:eric.duma...@gmail.com] Sent: Tuesday, April 06, 2010 11:01 AM To: John Linn Cc: net...@vger.kernel.org; linuxppc-...@ozlabs.org; grant.lik...@secretlab.ca; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver Le mardi 06 avril 2010 à 10:12 -0600, John Linn a écrit : -Original Message- From: Eric Dumazet [mailto:eric.duma...@gmail.com] Sent: Monday, April 05, 2010 3:30 PM To: John Linn Cc: net...@vger.kernel.org; linuxppc-...@ozlabs.org; grant.lik...@secretlab.ca; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver Le lundi 05 avril 2010 à 15:11 -0600, John Linn a écrit : This patch adds support for using the LL TEMAC Ethernet driver on non-Virtex 5 platforms by adding support for accessing the Soft DMA registers as if they were memory mapped instead of solely through the DCR's (available on the Virtex 5). The patch also updates the driver so that it runs on the MicroBlaze. The changes were tested on the PowerPC 440, PowerPC 405, and the MicroBlaze platforms. Signed-off-by: John Tyner jty...@cs.ucr.edu Signed-off-by: John Linn john.l...@xilinx.com --- +/* Align the IP data in the packet on word boundaries as MicroBlaze + * needs it. + */ + #define XTE_ALIGN 32 -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN) +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN) Very interesting way of doing this, but why such convoluted thing ? This is trying to align for a cache line (32 bytes) before my change. My change was then also making it align the IP data on a word boundary. Because of the % 32, this is equivalent to : #define BUFFER_ALIGN(adr) ((2 - ((u32) adr)) % XTE_ALIGN) Yes, but I'm not sure that's clearer IMHO. But wait, dont we recognise the magic constant NET_IP_ALIGN ? Yes it could be used. I'm struggling with how to make this all be clearer. I am not saying its clearer, I am saying we have a standard way to handle this exact problem (aligning rcvs buffer so that IP header is aligned) There is no need to invent new ones, this makes reviewing of this driver more difficult. How about this? #define BUFFER_ALIGN(adr) (((XTE_ALIGN + NET_IP_ALIGN) - ((u32) adr)) % XTE_ALIGN) Sorry, I still dont understand why you need XTE_ALIGN + ... ((A + B) - C) % A is equal to (B - C) % A Which one is more readable ? I'm fine with your suggestion. #define BUFFER_ALIGN(adr) ((2 - ((u32) adr)) % XTE_ALIGN) Please take a look at existing and clean code, no magic macro, and we can understand the intention. find drivers/net | xargs grep -n netdev_alloc_skb_ip_align Yes I see how it's used, but it only allows you to reserve 2 bytes in the skb with no options. This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
Yes I see how it's used, but it only allows you to reserve 2 bytes in the skb with no options. Really ? This would be a bug ! __alloc_skb() uses kmalloc(), this gives you the guarantee you want, or maybe comment you wrote is not what is _really_ necessary ? /* Align the IP data in the packet on word boundaries as MicroBlaze * needs it. */ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
On Mon, Apr 5, 2010 at 3:33 PM, John Linn john.l...@xilinx.com wrote: From: Eric Dumazet [mailto:eric.duma...@gmail.com] +/* Align the IP data in the packet on word boundaries as MicroBlaze + * needs it. + */ + #define XTE_ALIGN 32 -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN) +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN) Very interesting way of doing this, but why such convoluted thing ? Grant might have insight into why this started this way, I just updated to help with MicroBlaze alignment. It was that way in the code I received from Yoshio Kashiwagi and David H. Lynch. I have no problem with it being changed. Personally I would probably write a followup patch to change this to netdev_alloc_skb_ip_align(), just to keep changes logically separated. But I'm okay with it rolled into this patch also. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
On Tue, Apr 6, 2010 at 11:11 AM, John Linn john.l...@xilinx.com wrote: -Original Message- From: Eric Dumazet [mailto:eric.duma...@gmail.com] Sent: Tuesday, April 06, 2010 11:01 AM To: John Linn Cc: net...@vger.kernel.org; linuxppc-...@ozlabs.org; grant.lik...@secretlab.ca; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver Le mardi 06 avril 2010 à 10:12 -0600, John Linn a écrit : -Original Message- From: Eric Dumazet [mailto:eric.duma...@gmail.com] Sent: Monday, April 05, 2010 3:30 PM To: John Linn Cc: net...@vger.kernel.org; linuxppc-...@ozlabs.org; grant.lik...@secretlab.ca; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver Le lundi 05 avril 2010 à 15:11 -0600, John Linn a écrit : This patch adds support for using the LL TEMAC Ethernet driver on non-Virtex 5 platforms by adding support for accessing the Soft DMA registers as if they were memory mapped instead of solely through the DCR's (available on the Virtex 5). The patch also updates the driver so that it runs on the MicroBlaze. The changes were tested on the PowerPC 440, PowerPC 405, and the MicroBlaze platforms. Signed-off-by: John Tyner jty...@cs.ucr.edu Signed-off-by: John Linn john.l...@xilinx.com --- +/* Align the IP data in the packet on word boundaries as MicroBlaze + * needs it. + */ + #define XTE_ALIGN 32 -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN) +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN) Very interesting way of doing this, but why such convoluted thing ? This is trying to align for a cache line (32 bytes) before my change. My change was then also making it align the IP data on a word boundary. Because of the % 32, this is equivalent to : #define BUFFER_ALIGN(adr) ((2 - ((u32) adr)) % XTE_ALIGN) Yes, but I'm not sure that's clearer IMHO. But wait, dont we recognise the magic constant NET_IP_ALIGN ? Yes it could be used. I'm struggling with how to make this all be clearer. I am not saying its clearer, I am saying we have a standard way to handle this exact problem (aligning rcvs buffer so that IP header is aligned) There is no need to invent new ones, this makes reviewing of this driver more difficult. Hold on BUFFER_ALIGN is being used to align the DMA buffer on a cache line boundary. I don't think netdev_alloc_skb() makes any guarantees about how the start of the IP header lines up against cache line boundaries. The amount of padding needed is not known until an skbuff is obtained from netdev_alloc_skb(), and netdev_alloc_skb_ip_align() can only handle a fixed size padding, It doesn't look like netdev_alloc_skb_ip_align() is the right thing in this regard. How about this? #define BUFFER_ALIGN(adr) (((XTE_ALIGN + NET_IP_ALIGN) - ((u32) adr)) % XTE_ALIGN) Sorry, I still dont understand why you need XTE_ALIGN + ... ((A + B) - C) % A is equal to (B - C) % A Which one is more readable ? I'm fine with your suggestion. #define BUFFER_ALIGN(adr) ((2 - ((u32) adr)) % XTE_ALIGN) Please take a look at existing and clean code, no magic macro, and we can understand the intention. find drivers/net | xargs grep -n netdev_alloc_skb_ip_align Yes I see how it's used, but it only allows you to reserve 2 bytes in the skb with no options. Eric is here. The mod operation means that BUFFER_ALIGN using either 2 or 34 is equivalent. g. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
On Tue, 2010-04-06 at 12:53 -0600, Grant Likely wrote: Hold on BUFFER_ALIGN is being used to align the DMA buffer on a cache line boundary. I don't think netdev_alloc_skb() makes any guarantees about how the start of the IP header lines up against cache line boundaries. The amount of padding needed is not known until an skbuff is obtained from netdev_alloc_skb(), and netdev_alloc_skb_ip_align() can only handle a fixed size padding, It doesn't look like netdev_alloc_skb_ip_align() is the right thing in this regard. __netdev_alloc_skb reserves NET_SKB_PAD bytes which gets us cacheline alignment on Microblaze. From include/linux/skbuff.h: /* * The networking layer reserves some headroom in skb data (via * dev_alloc_skb). This is used to avoid having to reallocate skb data when * the header has to grow. In the default case, if the header has to grow * 32 bytes or less we avoid the reallocation. * * Unfortunately this headroom changes the DMA alignment of the resulting * network packet. As for NET_IP_ALIGN, this unaligned DMA is expensive * on some architectures. An architecture can override this value, * perhaps setting it to a cacheline in size (since that will maintain * cacheline alignment of the DMA). It must be a power of 2. * * Various parts of the networking layer expect at least 32 bytes of * headroom, you should not reduce this. */ #ifndef NET_SKB_PAD #define NET_SKB_PAD 32 #endif If this doesn't work for some of the PPC variants with larger cache lines, maybe one of the PPC header files needs to define NET_SKB_PAD? And if we want to guard against possible future changes to the default NET_SKB_PAD breaking Microblaze operation, maybe one of its headers should define NET_SKB_PAD as well? Steven J. Magnani I claim this network for MARS! www.digidescorp.com Earthling, return my space modulator! #include standard.disclaimer ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
-Original Message- From: glik...@secretlab.ca [mailto:glik...@secretlab.ca] On Behalf Of Grant Likely Sent: Tuesday, April 06, 2010 12:54 PM To: John Linn Cc: Eric Dumazet; net...@vger.kernel.org; linuxppc-...@ozlabs.org; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver On Tue, Apr 6, 2010 at 11:11 AM, John Linn john.l...@xilinx.com wrote: -Original Message- From: Eric Dumazet [mailto:eric.duma...@gmail.com] Sent: Tuesday, April 06, 2010 11:01 AM To: John Linn Cc: net...@vger.kernel.org; linuxppc-...@ozlabs.org; grant.lik...@secretlab.ca; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver Le mardi 06 avril 2010 à 10:12 -0600, John Linn a écrit : -Original Message- From: Eric Dumazet [mailto:eric.duma...@gmail.com] Sent: Monday, April 05, 2010 3:30 PM To: John Linn Cc: net...@vger.kernel.org; linuxppc-...@ozlabs.org; grant.lik...@secretlab.ca; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver Le lundi 05 avril 2010 à 15:11 -0600, John Linn a écrit : This patch adds support for using the LL TEMAC Ethernet driver on non-Virtex 5 platforms by adding support for accessing the Soft DMA registers as if they were memory mapped instead of solely through the DCR's (available on the Virtex 5). The patch also updates the driver so that it runs on the MicroBlaze. The changes were tested on the PowerPC 440, PowerPC 405, and the MicroBlaze platforms. Signed-off-by: John Tyner jty...@cs.ucr.edu Signed-off-by: John Linn john.l...@xilinx.com --- +/* Align the IP data in the packet on word boundaries as MicroBlaze + * needs it. + */ + #define XTE_ALIGN 32 -#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN) +#define BUFFER_ALIGN(adr) ((34 - ((u32) adr)) % XTE_ALIGN) Very interesting way of doing this, but why such convoluted thing ? This is trying to align for a cache line (32 bytes) before my change. My change was then also making it align the IP data on a word boundary. Because of the % 32, this is equivalent to : #define BUFFER_ALIGN(adr) ((2 - ((u32) adr)) % XTE_ALIGN) Yes, but I'm not sure that's clearer IMHO. But wait, dont we recognise the magic constant NET_IP_ALIGN ? Yes it could be used. I'm struggling with how to make this all be clearer. I am not saying its clearer, I am saying we have a standard way to handle this exact problem (aligning rcvs buffer so that IP header is aligned) There is no need to invent new ones, this makes reviewing of this driver more difficult. Hold on BUFFER_ALIGN is being used to align the DMA buffer on a cache line boundary. I don't think netdev_alloc_skb() makes any guarantees about how the start of the IP header lines up against cache line boundaries. The amount of padding needed is not known until an skbuff is obtained from netdev_alloc_skb(), and netdev_alloc_skb_ip_align() can only handle a fixed size padding, It doesn't look like netdev_alloc_skb_ip_align() is the right thing in this regard. How about this? #define BUFFER_ALIGN(adr) (((XTE_ALIGN + NET_IP_ALIGN) - ((u32) adr)) % XTE_ALIGN) Sorry, I still dont understand why you need XTE_ALIGN + ... ((A + B) - C) % A is equal to (B - C) % A Which one is more readable ? I'm fine with your suggestion. #define BUFFER_ALIGN(adr) ((2 - ((u32) adr)) % XTE_ALIGN) Please take a look at existing and clean code, no magic macro, and we can understand the intention. find drivers/net | xargs grep -n netdev_alloc_skb_ip_align Yes I see how it's used, but it only allows you to reserve 2 bytes in the skb with no options. Eric is here. The mod operation means that BUFFER_ALIGN using either 2 or 34 is equivalent. g. I can spin another patch with the following and with Grant's Kconfig changes, just looking for confirmation that's acceptable. #define BUFFER_ALIGN(adr) ((2 - ((u32) adr)) % XTE_ALIGN) Thanks, John This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org
RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
-Original Message- From: Steven J. Magnani [mailto:st...@digidescorp.com] Sent: Tuesday, April 06, 2010 2:04 PM To: grant.lik...@secretlab.ca Cc: John Linn; Eric Dumazet; net...@vger.kernel.org; linuxppc-...@ozlabs.org; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver On Tue, 2010-04-06 at 12:53 -0600, Grant Likely wrote: Hold on BUFFER_ALIGN is being used to align the DMA buffer on a cache line boundary. I don't think netdev_alloc_skb() makes any guarantees about how the start of the IP header lines up against cache line boundaries. The amount of padding needed is not known until an skbuff is obtained from netdev_alloc_skb(), and netdev_alloc_skb_ip_align() can only handle a fixed size padding, It doesn't look like netdev_alloc_skb_ip_align() is the right thing in this regard. __netdev_alloc_skb reserves NET_SKB_PAD bytes which gets us cacheline alignment on Microblaze. From include/linux/skbuff.h: Good find. I'll give it a test on MicroBlaze and PowerPC. /* * The networking layer reserves some headroom in skb data (via * dev_alloc_skb). This is used to avoid having to reallocate skb data when * the header has to grow. In the default case, if the header has to grow * 32 bytes or less we avoid the reallocation. * * Unfortunately this headroom changes the DMA alignment of the resulting * network packet. As for NET_IP_ALIGN, this unaligned DMA is expensive * on some architectures. An architecture can override this value, * perhaps setting it to a cacheline in size (since that will maintain * cacheline alignment of the DMA). It must be a power of 2. * * Various parts of the networking layer expect at least 32 bytes of * headroom, you should not reduce this. */ #ifndef NET_SKB_PAD #define NET_SKB_PAD 32 #endif If this doesn't work for some of the PPC variants with larger cache lines, maybe one of the PPC header files needs to define NET_SKB_PAD? Looks like it is defined in system.h in powerpc so that it works. And if we want to guard against possible future changes to the default NET_SKB_PAD breaking Microblaze operation, maybe one of its headers should define NET_SKB_PAD as well? Good idea, we can add that to system.h for MicroBlaze also. Thanks, John Steven J. Magnani I claim this network for MARS! www.digidescorp.com Earthling, return my space modulator! #include standard.disclaimer This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
Le mardi 06 avril 2010 à 12:53 -0600, Grant Likely a écrit : Hold on BUFFER_ALIGN is being used to align the DMA buffer on a cache line boundary. I don't think netdev_alloc_skb() makes any guarantees about how the start of the IP header lines up against cache line boundaries. The amount of padding needed is not known until an skbuff is obtained from netdev_alloc_skb(), and netdev_alloc_skb_ip_align() can only handle a fixed size padding, It doesn't look like netdev_alloc_skb_ip_align() is the right thing in this regard. OK, time to have a long explanation : netdev_alloc_skb_ip_align() is doing the right thing, but it seems you guys insist to invent a new private stuff. I am only wondering if you really know why you do this. Many drivers do have same requirements, so every driver should reinvent the wheel ? Really this is beyond me. Original code was aligning the buffer on a 32 bytes boundary (because of a hardware requirement on NIC, I only trust original code on this). Then you want to change this to align buffer on this 32 bytes boundary PLUS 2. What is this kind of new requirement ? 1) Hardware requirement really changed that much. (firmware changed on NIC). If not using this new alignement, NIC doesnt work at all. 2) or Microblaze arch requires that IP header is aligned on a word boundary to avoid unaligned traps in IP stack ? (like many arches) If this is the latest requirement, then use standard mechanism. skb data is naturally aligned on L1_CACHE_SIZE + SKB_PAD boundaries. (32 bytes alignment) netdev_alloc_skb_ip_align()() then skips 2 bytes to make skb data aligned so that 2 + 14 (sizeof eth header) = 16 : IP header is aligned (modulo 16) It just works. If not, we should correct it, please fill a bug report. Thanks ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
-Original Message- From: Eric Dumazet [mailto:eric.duma...@gmail.com] Sent: Tuesday, April 06, 2010 2:24 PM To: grant.lik...@secretlab.ca Cc: John Linn; net...@vger.kernel.org; linuxppc-...@ozlabs.org; jwbo...@linux.vnet.ibm.com; john.willi...@petalogix.com; michal.si...@petalogix.com; John Tyner Subject: Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver Le mardi 06 avril 2010 à 12:53 -0600, Grant Likely a écrit : Hold on BUFFER_ALIGN is being used to align the DMA buffer on a cache line boundary. I don't think netdev_alloc_skb() makes any guarantees about how the start of the IP header lines up against cache line boundaries. The amount of padding needed is not known until an skbuff is obtained from netdev_alloc_skb(), and netdev_alloc_skb_ip_align() can only handle a fixed size padding, It doesn't look like netdev_alloc_skb_ip_align() is the right thing in this regard. OK, time to have a long explanation : netdev_alloc_skb_ip_align() is doing the right thing, but it seems you guys insist to invent a new private stuff. I am only wondering if you really know why you do this. Many drivers do have same requirements, so every driver should reinvent the wheel ? Really this is beyond me. Original code was aligning the buffer on a 32 bytes boundary (because of a hardware requirement on NIC, I only trust original code on this). Then you want to change this to align buffer on this 32 bytes boundary PLUS 2. What is this kind of new requirement ? 1) Hardware requirement really changed that much. (firmware changed on NIC). If not using this new alignement, NIC doesnt work at all. 2) or Microblaze arch requires that IP header is aligned on a word boundary to avoid unaligned traps in IP stack ? (like many arches) Yes. If this is the latest requirement, then use standard mechanism. skb data is naturally aligned on L1_CACHE_SIZE + SKB_PAD boundaries. (32 bytes alignment) netdev_alloc_skb_ip_align()() then skips 2 bytes to make skb data aligned so that 2 + 14 (sizeof eth header) = 16 : IP header is aligned (modulo 16) It just works. If not, we should correct it, please fill a bug report. Trying it now, thanks for the help and patience :) -- John Thanks This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH v3 5/6] RapidIO, powerpc/85xx: Add MChk handler for SRIO port
From: Alexandre Bounine alexandre.boun...@idt.com
Add Machine Check exception handling into RapidIO port driver
for Freescale SoCs (MPC85xx).
Signed-off-by: Alexandre Bounine alexandre.boun...@idt.com
Tested-by: Thomas Moll thomas.m...@sysgo.com
Cc: Matt Porter mpor...@kernel.crashing.org
---
fsl_rio.c | 74 ++
1 files changed, 70 insertions(+), 4 deletions(-)
diff -x '*.pj' -X dontdiff_2.6.32-rc5 -pNur
w34r3a/arch/powerpc/sysdev/fsl_rio.c w34r3b/arch/powerpc/sysdev/fsl_rio.c
--- w34r3a/arch/powerpc/sysdev/fsl_rio.c2010-04-06 15:23:58.772536000
-0400
+++ w34r3b/arch/powerpc/sysdev/fsl_rio.c2010-04-06 15:25:50.446381000
-0400
@@ -31,6 +31,8 @@
#include linux/kfifo.h
#include asm/io.h
+#include asm/machdep.h
+#include asm/uaccess.h
#undef DEBUG_PW/* Port-Write debugging */
@@ -46,6 +48,8 @@
#define RIO_ESCSR 0x158
#define RIO_CCSR 0x15c
#define RIO_LTLEDCSR 0x0608
+#define RIO_LTLEDCSR_IER 0x8000
+#define RIO_LTLEDCSR_PRT 0x0100
#define RIO_LTLEECSR 0x060c
#define RIO_EPWISR 0x10010
#define RIO_ISR_AACR 0x10120
@@ -213,6 +217,54 @@ struct rio_priv {
spinlock_t pw_fifo_lock;
};
+#define __fsl_read_rio_config(x, addr, err, op)\
+ __asm__ __volatile__( \
+ 1: op %1,0(%2)\n\
+ eieio\n\
+ 2:\n \
+ .section .fixup,\ax\\n \
+ 3: li %1,-1\n \
+ li %0,%3\n \
+ b 2b\n \
+ .section __ex_table,\a\\n \
+ .align 2\n \
+ .long 1b,3b\n \
+ .text \
+ : =r (err), =r (x) \
+ : b (addr), i (-EFAULT), 0 (err))
+
+static void __iomem *rio_regs_win;
+
+static int (*saved_mcheck_exception)(struct pt_regs *regs);
+
+static int fsl_rio_mcheck_exception(struct pt_regs *regs)
+{
+ const struct exception_table_entry *entry = NULL;
+ unsigned long reason = (mfspr(SPRN_MCSR) MCSR_MASK);
+
+ if (reason MCSR_BUS_RBERR) {
+ reason = in_be32((u32 *)(rio_regs_win + RIO_LTLEDCSR));
+ if (reason (RIO_LTLEDCSR_IER | RIO_LTLEDCSR_PRT)) {
+ /* Check if we are prepared to handle this fault */
+ entry = search_exception_tables(regs-nip);
+ if (entry) {
+ pr_debug(RIO: %s - MC Exception handled\n,
+__func__);
+ out_be32((u32 *)(rio_regs_win + RIO_LTLEDCSR),
+0);
+ regs-msr |= MSR_RI;
+ regs-nip = entry-fixup;
+ return 1;
+ }
+ }
+ }
+
+ if (saved_mcheck_exception)
+ return saved_mcheck_exception(regs);
+ else
+ return cur_cpu_spec-machine_check(regs);
+}
+
/**
* fsl_rio_doorbell_send - Send a MPC85xx doorbell message
* @mport: RapidIO master port info
@@ -313,6 +365,7 @@ fsl_rio_config_read(struct rio_mport *mp
{
struct rio_priv *priv = mport-priv;
u8 *data;
+ u32 rval, err = 0;
pr_debug
(fsl_rio_config_read: index %d destid %d hopcount %d offset %8.8x
len %d\n,
@@ -323,17 +376,24 @@ fsl_rio_config_read(struct rio_mport *mp
data = (u8 *) priv-maint_win + offset;
switch (len) {
case 1:
- *val = in_8((u8 *) data);
+ __fsl_read_rio_config(rval, data, err, lbz);
break;
case 2:
- *val = in_be16((u16 *) data);
+ __fsl_read_rio_config(rval, data, err, lhz);
break;
default:
- *val = in_be32((u32 *) data);
+ __fsl_read_rio_config(rval, data, err, lwz);
break;
}
- return 0;
+ if (err) {
+ pr_debug(RIO: cfg_read error %d for %x:%x:%x\n,
+err, destid, hopcount, offset);
+ }
+
+ *val = rval;
+
+ return err;
}
/**
@@ -1364,6 +1424,7 @@ int fsl_rio_setup(struct of_device *dev)
rio_register_mport(port);
priv-regs_win = ioremap(regs.start, regs.end - regs.start + 1);
+ rio_regs_win = priv-regs_win;
/* Probe the master port phy type */
ccsr = in_be32(priv-regs_win + RIO_CCSR);
@@ -1432,6 +1493,11 @@ int fsl_rio_setup(struct of_device *dev)
fsl_rio_doorbell_init(port);
[PATCH v3 6/6] RapidIO: Add enabling SRIO port RX and TX
From: Thomas Moll thomas.m...@sysgo.com
Add the functionality to enable Input receiver and Output
transmitter of every port, to allow non-maintenance traffic.
Signed-off-by: Thomas Moll thomas.m...@sysgo.com
Cc: Matt Porter mpor...@kernel.crashing.org
---
drivers/rapidio/Kconfig| 11 ++
drivers/rapidio/rio-scan.c | 79 -
include/linux/rio_regs.h |5 ++
3 files changed, 94 insertions(+), 1 deletion(-)
diff -x '*.pj' -X dontdiff_2.6.32-rc5 -pNur w34r3a/drivers/rapidio/Kconfig
w34r3b/drivers/rapidio/Kconfig
--- w34r3a/drivers/rapidio/Kconfig 2010-04-06 13:44:13.980893000 -0400
+++ w34r3b/drivers/rapidio/Kconfig 2010-04-06 15:48:55.168883000 -0400
@@ -9,4 +9,15 @@ config RAPIDIO_DISC_TIMEOUT
Amount of time a discovery node waits for a host to complete
enumeration before giving up.
+config RAPIDIO_ENABLE_RX_TX_PORTS
+ bool Enable RapidIO Input/Output Ports
+ depends on RAPIDIO
+ ---help---
+ The RapidIO specification describes a Output port transmit
+ enable and a Input port receive enable. The recommended state
+ for Input ports and Output ports should be disabled. When
+ this switch is set the RapidIO subsystem will enable all
+ ports for Input/Output direction to allow other traffic
+ than Maintenance transfers.
+
source drivers/rapidio/switches/Kconfig
diff -x '*.pj' -X dontdiff_2.6.32-rc5 -pNur w34r3a/drivers/rapidio/rio-scan.c
w34r3b/drivers/rapidio/rio-scan.c
--- w34r3a/drivers/rapidio/rio-scan.c 2010-04-06 14:51:48.356439000 -0400
+++ w34r3b/drivers/rapidio/rio-scan.c 2010-04-06 15:47:21.668548000 -0400
@@ -8,6 +8,10 @@
* Alex Bounine alexandre.boun...@idt.com
* - Added Port-Write/Error Management initialization and handling
*
+ * Copyright 2009 Sysgo AG
+ * Thomas Moll thomas.m...@sysgo.com
+ * - Added Input- Output- enable functionality, to allow full communication
+ *
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; either version 2 of the License, or (at your
@@ -328,6 +332,65 @@ static int __devinit rio_add_device(stru
}
/**
+ * rio_enable_rx_tx_port - enable input reciever and output transmitter of
+ * given port
+ * @port: Master port associated with the RIO network
+ * @local: local=1 select local port otherwise a far device is reached
+ * @destid: Destination ID of the device to check host bit
+ * @hopcount: Number of hops to reach the target
+ * @port_num: Port (-number on switch) to enable on a far end device
+ *
+ * Returns 0 or 1 from on General Control Command and Status Register
+ * (EXT_PTR+0x3C)
+ */
+inline int rio_enable_rx_tx_port(struct rio_mport *port,
+int local, u16 destid,
+u8 hopcount, u8 port_num) {
+#ifdef CONFIG_RAPIDIO_ENABLE_RX_TX_PORTS
+ u32 regval;
+ u32 ext_ftr_ptr;
+
+ /*
+ * enable rx input tx output port
+ */
+ pr_debug(rio_enable_rx_tx_port(local = %d, destid = %d, hopcount =
+%d, port_num = %d)\n, local, destid, hopcount, port_num);
+
+ ext_ftr_ptr = rio_mport_get_physefb(port, local, destid, hopcount);
+
+ if (local) {
+ rio_local_read_config_32(port, ext_ftr_ptr +
+ RIO_PORT_N_CTL_CSR(0),
+ regval);
+ } else {
+ if (rio_mport_read_config_32(port, destid, hopcount,
+ ext_ftr_ptr + RIO_PORT_N_CTL_CSR(port_num), regval) 0)
+ return -EIO;
+ }
+
+ if (regval RIO_PORT_N_CTL_P_TYP_SER) {
+ /* serial */
+ regval = regval | RIO_PORT_N_CTL_EN_RX_SER
+ | RIO_PORT_N_CTL_EN_TX_SER;
+ } else {
+ /* parallel */
+ regval = regval | RIO_PORT_N_CTL_EN_RX_PAR
+ | RIO_PORT_N_CTL_EN_TX_PAR;
+ }
+
+ if (local) {
+ rio_local_write_config_32(port, ext_ftr_ptr +
+ RIO_PORT_N_CTL_CSR(0), regval);
+ } else {
+ if (rio_mport_write_config_32(port, destid, hopcount,
+ ext_ftr_ptr + RIO_PORT_N_CTL_CSR(port_num), regval) 0)
+ return -EIO;
+ }
+#endif
+ return 0;
+}
+
+/**
* rio_setup_device- Allocates and sets up a RIO device
* @net: RIO network
* @port: Master port to send transactions
@@ -430,9 +493,14 @@ static struct rio_dev __devinit *rio_set
list_add_tail(rswitch-node, rio_switches);
- } else
+ } else {
+ if (do_enum)
+ /*Enable Input Output Port (transmitter reviever)*/
+ rio_enable_rx_tx_port(port, 0, destid, hopcount, 0);
+
[PATCH v3 4/6] RapidIO, powerpc/85xx: Add Port-Write message handler for SRIO port
From: Alexandre Bounine alexandre.boun...@idt.com
Add RapidIO Port-Write message handler for Freescale SoCs with RapidIO port.
Signed-off-by: Alexandre Bounine alexandre.boun...@idt.com
Tested-by: Thomas Moll thomas.m...@sysgo.com
Cc: Matt Porter mpor...@kernel.crashing.org
---
fsl_rio.c | 263 +-
1 files changed, 260 insertions(+), 3 deletions(-)
diff -x '*.pj' -X dontdiff_2.6.32-rc5 -pNur
w34r3a/arch/powerpc/sysdev/fsl_rio.c w34r3b/arch/powerpc/sysdev/fsl_rio.c
--- w34r3a/arch/powerpc/sysdev/fsl_rio.c2010-04-06 14:51:48.274437000
-0400
+++ w34r3b/arch/powerpc/sysdev/fsl_rio.c2010-04-06 14:54:05.413394000
-0400
@@ -1,6 +1,11 @@
/*
* Freescale MPC85xx/MPC86xx RapidIO support
*
+ * Copyright 2009 Integrated Device Technology, Inc.
+ * Alex Bounine alexandre.boun...@idt.com
+ * - Added Port-Write message handling
+ * - Added Machine Check exception handling
+ *
* Copyright (C) 2007, 2008 Freescale Semiconductor, Inc.
* Zhang Wei wei.zh...@freescale.com
*
@@ -23,19 +28,26 @@
#include linux/rio_drv.h
#include linux/of_platform.h
#include linux/delay.h
+#include linux/kfifo.h
#include asm/io.h
+#undef DEBUG_PW/* Port-Write debugging */
+
/* RapidIO definition irq, which read from OF-tree */
#define IRQ_RIO_BELL(m)(((struct rio_priv
*)(m-priv))-bellirq)
#define IRQ_RIO_TX(m) (((struct rio_priv *)(m-priv))-txirq)
#define IRQ_RIO_RX(m) (((struct rio_priv *)(m-priv))-rxirq)
+#define IRQ_RIO_PW(m) (((struct rio_priv *)(m-priv))-pwirq)
#define RIO_ATMU_REGS_OFFSET 0x10c00
#define RIO_P_MSG_REGS_OFFSET 0x11000
#define RIO_S_MSG_REGS_OFFSET 0x13000
#define RIO_ESCSR 0x158
#define RIO_CCSR 0x15c
+#define RIO_LTLEDCSR 0x0608
+#define RIO_LTLEECSR 0x060c
+#define RIO_EPWISR 0x10010
#define RIO_ISR_AACR 0x10120
#define RIO_ISR_AACR_AA0x1 /* Accept All ID */
#define RIO_MAINT_WIN_SIZE 0x40
@@ -54,6 +66,18 @@
#define RIO_MSG_ISR_QFI0x0010
#define RIO_MSG_ISR_DIQI 0x0001
+#define RIO_IPWMR_SEN 0x0010
+#define RIO_IPWMR_QFIE 0x0100
+#define RIO_IPWMR_EIE 0x0020
+#define RIO_IPWMR_CQ 0x0002
+#define RIO_IPWMR_PWE 0x0001
+
+#define RIO_IPWSR_QF 0x0010
+#define RIO_IPWSR_TE 0x0080
+#define RIO_IPWSR_QFI 0x0010
+#define RIO_IPWSR_PWD 0x0008
+#define RIO_IPWSR_PWB 0x0004
+
#define RIO_MSG_DESC_SIZE 32
#define RIO_MSG_BUFFER_SIZE4096
#define RIO_MIN_TX_RING_SIZE 2
@@ -120,7 +144,7 @@ struct rio_msg_regs {
u32 pad10[26];
u32 pwmr;
u32 pwsr;
- u32 pad11;
+ u32 epwqbar;
u32 pwqbar;
};
@@ -159,6 +183,14 @@ struct rio_msg_rx_ring {
void *dev_id;
};
+struct rio_port_write_msg {
+ void *virt;
+ dma_addr_t phys;
+ u32 msg_count;
+ u32 err_count;
+ u32 discard_count;
+};
+
struct rio_priv {
struct device *dev;
void __iomem *regs_win;
@@ -171,9 +203,14 @@ struct rio_priv {
struct rio_dbell_ring dbell_ring;
struct rio_msg_tx_ring msg_tx_ring;
struct rio_msg_rx_ring msg_rx_ring;
+ struct rio_port_write_msg port_write_msg;
int bellirq;
int txirq;
int rxirq;
+ int pwirq;
+ struct work_struct pw_work;
+ struct kfifo pw_fifo;
+ spinlock_t pw_fifo_lock;
};
/**
@@ -929,6 +966,223 @@ static int fsl_rio_doorbell_init(struct
return rc;
}
+/**
+ * fsl_rio_port_write_handler - MPC85xx port write interrupt handler
+ * @irq: Linux interrupt number
+ * @dev_instance: Pointer to interrupt-specific data
+ *
+ * Handles port write interrupts. Parses a list of registered
+ * port write event handlers and executes a matching event handler.
+ */
+static irqreturn_t
+fsl_rio_port_write_handler(int irq, void *dev_instance)
+{
+ u32 ipwmr, ipwsr;
+ struct rio_mport *port = (struct rio_mport *)dev_instance;
+ struct rio_priv *priv = port-priv;
+ u32 epwisr, tmp;
+
+ ipwmr = in_be32(priv-msg_regs-pwmr);
+ ipwsr = in_be32(priv-msg_regs-pwsr);
+
+ epwisr = in_be32(priv-regs_win + RIO_EPWISR);
+ if (epwisr 0x8000) {
+ tmp = in_be32(priv-regs_win + RIO_LTLEDCSR);
+ pr_info(RIO_LTLEDCSR = 0x%x\n, tmp);
+ out_be32(priv-regs_win + RIO_LTLEDCSR, 0);
+ }
+
+ if (!(epwisr 0x0001))
+ return IRQ_HANDLED;
+
+#ifdef DEBUG_PW
+ pr_debug(PW Int-IPWMR: 0x%08x IPWSR: 0x%08x (, ipwmr, ipwsr);
+ if (ipwsr RIO_IPWSR_QF)
+ pr_debug( QF);
+ if (ipwsr RIO_IPWSR_TE)
+ pr_debug( TE);
+ if (ipwsr RIO_IPWSR_QFI)
+ pr_debug( QFI);
+ if (ipwsr RIO_IPWSR_PWD)
+
[PATCH v3 2/6] RapidIO: Add switch locking during discovery
From: Alexandre Bounine alexandre.boun...@idt.com
Add switch access locking during RapidIO discovery. Access lock is
required when reading switch routing table contents due to indexed mechanism
of RT addressing.
Signed-off-by: Alexandre Bounine alexandre.boun...@idt.com
Tested-by: Thomas Moll thomas.m...@sysgo.com
Cc: Matt Porter mpor...@kernel.crashing.org
---
rio-scan.c | 164 +
1 files changed, 144 insertions(+), 20 deletions(-)
diff -x '*.pj' -X dontdiff_2.6.32-rc5 -pNur w34r3a/drivers/rapidio/rio-scan.c
w34r3b/drivers/rapidio/rio-scan.c
--- w34r3a/drivers/rapidio/rio-scan.c 2010-04-06 13:44:13.994872000 -0400
+++ w34r3b/drivers/rapidio/rio-scan.c 2010-04-06 14:19:21.218017000 -0400
@@ -457,12 +457,87 @@ rio_sport_is_active(struct rio_mport *po
}
/**
+ * rio_lock_device - Acquires host device lock for specified device
+ * @port: Master port to send transaction
+ * @destid: Destination ID for device/switch
+ * @hopcount: Hopcount to reach switch
+ * @wait_ms: Max wait time in msec (0 = no timeout)
+ *
+ * Attepts to acquire host device lock for specified device
+ * Returns 0 if device lock acquired or EINVAL if timeout expires.
+ */
+static int
+rio_lock_device(struct rio_mport *port, u16 destid, u8 hopcount, int wait_ms)
+{
+ u32 result;
+ int tcnt = 0;
+
+ /* Attempt to acquire device lock */
+ rio_mport_write_config_32(port, destid, hopcount,
+ RIO_HOST_DID_LOCK_CSR, port-host_deviceid);
+ rio_mport_read_config_32(port, destid, hopcount,
+RIO_HOST_DID_LOCK_CSR, result);
+
+ while (result != port-host_deviceid) {
+ if (wait_ms != 0 tcnt == wait_ms) {
+ pr_debug(RIO: timeout when locking device %x:%x\n,
+ destid, hopcount);
+ return -EINVAL;
+ }
+
+ /* Delay a bit */
+ mdelay(1);
+ tcnt++;
+ /* Try to acquire device lock again */
+ rio_mport_write_config_32(port, destid,
+ hopcount,
+ RIO_HOST_DID_LOCK_CSR,
+ port-host_deviceid);
+ rio_mport_read_config_32(port, destid,
+ hopcount,
+ RIO_HOST_DID_LOCK_CSR, result);
+ }
+
+ return 0;
+}
+
+/**
+ * rio_unlock_device - Releases host device lock for specified device
+ * @port: Master port to send transaction
+ * @destid: Destination ID for device/switch
+ * @hopcount: Hopcount to reach switch
+ *
+ * Returns 0 if device lock released or EINVAL if fails.
+ */
+static int
+rio_unlock_device(struct rio_mport *port, u16 destid, u8 hopcount)
+{
+ u32 result;
+
+ /* Release device lock */
+ rio_mport_write_config_32(port, destid,
+ hopcount,
+ RIO_HOST_DID_LOCK_CSR,
+ port-host_deviceid);
+ rio_mport_read_config_32(port, destid, hopcount,
+ RIO_HOST_DID_LOCK_CSR, result);
+ if ((result 0x) != 0x) {
+ pr_debug(RIO: badness when releasing device lock %x:%x\n,
+destid, hopcount);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+/**
* rio_route_add_entry- Add a route entry to a switch routing table
* @mport: Master port to send transaction
* @rswitch: Switch device
* @table: Routing table ID
* @route_destid: Destination ID to be routed
* @route_port: Port number to be routed
+ * @lock: lock switch device flag
*
* Calls the switch specific add_entry() method to add a route entry
* on a switch. The route table can be specified using the @table
@@ -471,12 +546,26 @@ rio_sport_is_active(struct rio_mport *po
* %RIO_GLOBAL_TABLE in @table. Returns %0 on success or %-EINVAL
* on failure.
*/
-static int rio_route_add_entry(struct rio_mport *mport, struct rio_switch
*rswitch,
- u16 table, u16 route_destid, u8 route_port)
+static int
+rio_route_add_entry(struct rio_mport *mport, struct rio_switch *rswitch,
+ u16 table, u16 route_destid, u8 route_port, int lock)
{
- return rswitch-add_entry(mport, rswitch-destid,
+ int rc;
+
+ if (lock) {
+ rc = rio_lock_device(mport, rswitch-destid,
+rswitch-hopcount, 1000);
+ if (rc)
+ return rc;
+ }
+
+ rc = rswitch-add_entry(mport, rswitch-destid,
rswitch-hopcount, table,
route_destid, route_port);
+ if (lock)
+ rio_unlock_device(mport, rswitch-destid, rswitch-hopcount);
+
+ return rc;
}
/**
@@ -486,6 +575,7 @@ static int rio_route_add_entry(struct ri
* @table:
[PATCH v3 1/6] RapidIO: Add IDT CPS/TSI switches
From: Alexandre Bounine alexandre.boun...@idt.com
Extentions to RapidIO switch support:
1. modify switch route operation declarations to allow using single
switch-specific file for family of switches that share the same route
table operations.
2. add standard route table operations for switches that that support
route table manipulation registers as defined in the Rev.1.3 of RapidIO
specification.
3. add clear-route-table operation for switches
4. add CPSxx and TSIxxx families of RapidIO switches
Signed-off-by: Alexandre Bounine alexandre.boun...@idt.com
Tested-by: Thomas Moll thomas.m...@sysgo.com
Cc: Matt Porter mpor...@kernel.crashing.org
---
drivers/rapidio/Kconfig |2
drivers/rapidio/rio-scan.c| 20 ++-
drivers/rapidio/rio.c | 104 +
drivers/rapidio/rio.h | 20 +--
drivers/rapidio/switches/Kconfig | 28 ++
drivers/rapidio/switches/Makefile |5 +
drivers/rapidio/switches/idtcps.c | 89 +++
drivers/rapidio/switches/tsi500.c |2
drivers/rapidio/switches/tsi568.c | 106 ++
drivers/rapidio/switches/tsi57x.c | 106 ++
include/linux/rio.h |6 ++
include/linux/rio_ids.h | 14 +
include/linux/rio_regs.h | 14 -
13 files changed, 505 insertions(+), 11 deletions(-)
diff -x '*.pj' -X dontdiff_2.6.32-rc5 -pNur w34r3a/drivers/rapidio/Kconfig
w34r3b/drivers/rapidio/Kconfig
--- w34r3a/drivers/rapidio/Kconfig 2010-03-30 12:24:39.0 -0400
+++ w34r3b/drivers/rapidio/Kconfig 2010-04-06 10:51:52.809762000 -0400
@@ -8,3 +8,5 @@ config RAPIDIO_DISC_TIMEOUT
---help---
Amount of time a discovery node waits for a host to complete
enumeration before giving up.
+
+source drivers/rapidio/switches/Kconfig
diff -x '*.pj' -X dontdiff_2.6.32-rc5 -pNur w34r3a/drivers/rapidio/rio-scan.c
w34r3b/drivers/rapidio/rio-scan.c
--- w34r3a/drivers/rapidio/rio-scan.c 2010-03-30 12:24:39.0 -0400
+++ w34r3b/drivers/rapidio/rio-scan.c 2010-04-06 10:55:12.634848000 -0400
@@ -55,6 +55,7 @@ static int rio_mport_phys_table[] = {
static int rio_sport_phys_table[] = {
RIO_EFB_PAR_EP_FREE_ID,
RIO_EFB_SER_EP_FREE_ID,
+ RIO_EFB_SER_EP_FREC_ID,
-1,
};
@@ -246,10 +247,20 @@ static void rio_route_set_ops(struct rio
pr_debug(RIO: adding routing ops for %s\n,
rio_name(rdev));
rdev-rswitch-add_entry = cur-add_hook;
rdev-rswitch-get_entry = cur-get_hook;
+ rdev-rswitch-clr_table = cur-clr_hook;
+ break;
}
cur++;
}
+ if ((cur = end) (rdev-pef RIO_PEF_STD_RT)) {
+ pr_debug(RIO: adding STD routing ops for %s\n,
+ rio_name(rdev));
+ rdev-rswitch-add_entry = rio_std_route_add_entry;
+ rdev-rswitch-get_entry = rio_std_route_get_entry;
+ rdev-rswitch-clr_table = rio_std_route_clr_table;
+ }
+
if (!rdev-rswitch-add_entry || !rdev-rswitch-get_entry)
printk(KERN_ERR RIO: missing routing ops for %s\n,
rio_name(rdev));
@@ -349,7 +360,7 @@ static struct rio_dev __devinit *rio_set
if (rio_is_switch(rdev)) {
rio_mport_read_config_32(port, destid, hopcount,
RIO_SWP_INFO_CAR, rdev-swpinfo);
- rswitch = kmalloc(sizeof(struct rio_switch), GFP_KERNEL);
+ rswitch = kzalloc(sizeof(struct rio_switch), GFP_KERNEL);
if (!rswitch)
goto cleanup;
rswitch-switchid = next_switchid;
@@ -369,6 +380,10 @@ static struct rio_dev __devinit *rio_set
rdev-rswitch-switchid);
rio_route_set_ops(rdev);
+ if (do_enum rdev-rswitch-clr_table)
+ rdev-rswitch-clr_table(port, destid, hopcount,
+RIO_GLOBAL_TABLE);
+
list_add_tail(rswitch-node, rio_switches);
} else
@@ -866,6 +881,9 @@ static void rio_update_route_tables(stru
continue;
if (RIO_INVALID_ROUTE == rswitch-route_table[destid]) {
+ /* Skip if destid ends in empty switch*/
+ if (rswitch-destid == destid)
+ continue;
sport = rio_get_swpinfo_inport(port,
rswitch-destid,
rswitch-hopcount);
diff -x '*.pj' -X dontdiff_2.6.32-rc5 -pNur w34r3a/drivers/rapidio/rio.c
w34r3b/drivers/rapidio/rio.c
--- w34r3a/drivers/rapidio/rio.c2010-03-30
[PATCH v3 3/6] RapidIO: Add Port-Write handling for EM
From: Alexandre Bounine alexandre.boun...@idt.com
Add RapidIO Port-Write message handling in the context
of Error Management Extensions Specification Rev.1.3.
Signed-off-by: Alexandre Bounine alexandre.boun...@idt.com
Tested-by: Thomas Moll thomas.m...@sysgo.com
Cc: Matt Porter mpor...@kernel.crashing.org
---
arch/powerpc/sysdev/fsl_rio.c |2
drivers/rapidio/rio-scan.c| 166 +++
drivers/rapidio/rio.c | 328 ++
drivers/rapidio/rio.h | 30 +++
drivers/rapidio/switches/tsi568.c | 24 ++
drivers/rapidio/switches/tsi57x.c | 153 +
include/asm-generic/vmlinux.lds.h |5
include/linux/rio.h | 42
include/linux/rio_drv.h |6
include/linux/rio_regs.h | 61 ++-
10 files changed, 776 insertions(+), 41 deletions(-)
diff -x '*.pj' -X dontdiff_2.6.32-rc5 -pNur
w34r3a/arch/powerpc/sysdev/fsl_rio.c w34r3b/arch/powerpc/sysdev/fsl_rio.c
--- w34r3a/arch/powerpc/sysdev/fsl_rio.c2010-03-30 12:24:39.0
-0400
+++ w34r3b/arch/powerpc/sysdev/fsl_rio.c2010-04-06 14:28:04.817953000
-0400
@@ -1056,7 +1056,7 @@ int fsl_rio_setup(struct of_device *dev)
dev_info(dev-dev, LAW start 0x%016llx, size 0x%016llx.\n,
law_start, law_size);
- ops = kmalloc(sizeof(struct rio_ops), GFP_KERNEL);
+ ops = kzalloc(sizeof(struct rio_ops), GFP_KERNEL);
if (!ops) {
rc = -ENOMEM;
goto err_ops;
diff -x '*.pj' -X dontdiff_2.6.32-rc5 -pNur w34r3a/drivers/rapidio/rio-scan.c
w34r3b/drivers/rapidio/rio-scan.c
--- w34r3a/drivers/rapidio/rio-scan.c 2010-04-06 14:24:31.913278000 -0400
+++ w34r3b/drivers/rapidio/rio-scan.c 2010-04-06 14:28:04.851955000 -0400
@@ -4,6 +4,10 @@
* Copyright 2005 MontaVista Software, Inc.
* Matt Porter mpor...@kernel.crashing.org
*
+ * Copyright 2009 Integrated Device Technology, Inc.
+ * Alex Bounine alexandre.boun...@idt.com
+ * - Added Port-Write/Error Management initialization and handling
+ *
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the
* Free Software Foundation; either version 2 of the License, or (at your
@@ -31,15 +35,16 @@
LIST_HEAD(rio_devices);
static LIST_HEAD(rio_switches);
-#define RIO_ENUM_CMPL_MAGIC0xdeadbeef
-
static void rio_enum_timeout(unsigned long);
+static void rio_init_em(struct rio_dev *rdev);
+
DEFINE_SPINLOCK(rio_global_list_lock);
static int next_destid = 0;
static int next_switchid = 0;
static int next_net = 0;
+static int next_comptag;
static struct timer_list rio_enum_timer =
TIMER_INITIALIZER(rio_enum_timeout, 0, 0);
@@ -52,13 +57,6 @@ static int rio_mport_phys_table[] = {
-1,
};
-static int rio_sport_phys_table[] = {
- RIO_EFB_PAR_EP_FREE_ID,
- RIO_EFB_SER_EP_FREE_ID,
- RIO_EFB_SER_EP_FREC_ID,
- -1,
-};
-
/**
* rio_get_device_id - Get the base/extended device id for a device
* @port: RIO master port
@@ -119,12 +117,26 @@ static int rio_clear_locks(struct rio_mp
u32 result;
int ret = 0;
- /* Write component tag CSR magic complete value */
- rio_local_write_config_32(port, RIO_COMPONENT_TAG_CSR,
- RIO_ENUM_CMPL_MAGIC);
- list_for_each_entry(rdev, rio_devices, global_list)
- rio_write_config_32(rdev, RIO_COMPONENT_TAG_CSR,
- RIO_ENUM_CMPL_MAGIC);
+ /* Assign component tag to all devices */
+ next_comptag = 1;
+ rio_local_write_config_32(port, RIO_COMPONENT_TAG_CSR, next_comptag++);
+
+ list_for_each_entry(rdev, rio_devices, global_list) {
+ /* Mark device as discovered */
+ rio_read_config_32(rdev,
+ rdev-phys_efptr + RIO_PORT_GEN_CTL_CSR,
+ result);
+ rio_write_config_32(rdev,
+ rdev-phys_efptr + RIO_PORT_GEN_CTL_CSR,
+ result | RIO_PORT_GEN_DISCOVERED);
+
+ rio_write_config_32(rdev, RIO_COMPONENT_TAG_CSR, next_comptag);
+ rdev-comp_tag = next_comptag++;
+ if (next_comptag = 0x1) {
+ pr_err(RIO: Component Tag Counter Overflow\n);
+ break;
+ }
+ }
/* Release host device id locks */
rio_local_write_config_32(port, RIO_HOST_DID_LOCK_CSR,
@@ -267,6 +279,30 @@ static void rio_route_set_ops(struct rio
}
/**
+ * rio_em_set_ops- Sets Error Managment operations for a particular vendor
switch
+ * @rdev: RIO device
+ *
+ * Searches the RIO EM ops table for known switch types. If the vid
+ * and did match a switch table entry, then set the em_init() and
+ * em_handle() ops to the table entry values.
+ */
+static void
[PATCH] [V4] Add non-Virtex5 support for LL TEMAC driver
This patch adds support for using the LL TEMAC Ethernet driver on
non-Virtex 5 platforms by adding support for accessing the Soft DMA
registers as if they were memory mapped instead of solely through the
DCR's (available on the Virtex 5).
The patch also updates the driver so that it runs on the MicroBlaze.
The changes were tested on the PowerPC 440, PowerPC 405, and the
MicroBlaze platforms.
Signed-off-by: John Tyner jty...@cs.ucr.edu
Signed-off-by: John Linn john.l...@xilinx.com
---
V2 - Incorporated comments from Grant and added more logic to allow the driver
to work on MicroBlaze.
V3 - Only updated it to apply to head, minor change to include slab.h. Also
verified that it now builds for MicroBlaze. Retested on PowerPC and MicroBlaze.
V4 - Removed buffer alignment for skb and called the network functions that
already do the alignment for cache line and word alignment. Added constants
to MicroBlaze system to make sure network alignment is maintained. Also updated
the Kconfig so it depends on Microblaze or PPC based on Grant's comment.
---
arch/microblaze/include/asm/system.h | 11 +++
drivers/net/Kconfig |2 +-
drivers/net/ll_temac.h | 14 +++-
drivers/net/ll_temac_main.c | 137 +
4 files changed, 126 insertions(+), 38 deletions(-)
diff --git a/arch/microblaze/include/asm/system.h
b/arch/microblaze/include/asm/system.h
index 59efb3f..48c4f03 100644
--- a/arch/microblaze/include/asm/system.h
+++ b/arch/microblaze/include/asm/system.h
@@ -12,6 +12,7 @@
#include asm/registers.h
#include asm/setup.h
#include asm/irqflags.h
+#include asm/cache.h
#include asm-generic/cmpxchg.h
#include asm-generic/cmpxchg-local.h
@@ -96,4 +97,14 @@ extern struct dentry *of_debugfs_root;
#define arch_align_stack(x) (x)
+/*
+ * MicroBlaze doesn't handle unaligned accesses in hardware.
+ *
+ * Based on this we force the IP header alignment in network drivers.
+ * We also modify NET_SKB_PAD to be a cacheline in size, thus maintaining
+ * cacheline alignment of buffers.
+ */
+#define NET_IP_ALIGN 2
+#define NET_SKB_PADL1_CACHE_BYTES
+
#endif /* _ASM_MICROBLAZE_SYSTEM_H */
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 0ba5b8e..207a57d 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -2434,8 +2434,8 @@ config MV643XX_ETH
config XILINX_LL_TEMAC
tristate Xilinx LL TEMAC (LocalLink Tri-mode Ethernet MAC) driver
+ depends on PPC || MICROBLAZE
select PHYLIB
- depends on PPC_DCR_NATIVE
help
This driver supports the Xilinx 10/100/1000 LocalLink TEMAC
core used in Xilinx Spartan and Virtex FPGAs
diff --git a/drivers/net/ll_temac.h b/drivers/net/ll_temac.h
index 1af66a1..c033584 100644
--- a/drivers/net/ll_temac.h
+++ b/drivers/net/ll_temac.h
@@ -5,8 +5,11 @@
#include linux/netdevice.h
#include linux/of.h
#include linux/spinlock.h
+
+#ifdef CONFIG_PPC_DCR
#include asm/dcr.h
#include asm/dcr-regs.h
+#endif
/* packet size info */
#define XTE_HDR_SIZE 14 /* size of Ethernet header */
@@ -290,9 +293,6 @@ This option defaults to enabled (set) */
#define TX_CONTROL_CALC_CSUM_MASK 1
-#define XTE_ALIGN 32
-#define BUFFER_ALIGN(adr) ((XTE_ALIGN - ((u32) adr)) % XTE_ALIGN)
-
#define MULTICAST_CAM_TABLE_NUM 4
/* TX/RX CURDESC_PTR points to first descriptor */
@@ -335,9 +335,15 @@ struct temac_local {
struct mii_bus *mii_bus;/* MII bus reference */
int mdio_irqs[PHY_MAX_ADDR];/* IRQs table for MDIO bus */
- /* IO registers and IRQs */
+ /* IO registers, dma functions and IRQs */
void __iomem *regs;
+ void __iomem *sdma_regs;
+#ifdef CONFIG_PPC_DCR
dcr_host_t sdma_dcrs;
+#endif
+ u32 (*dma_in)(struct temac_local *, int);
+ void (*dma_out)(struct temac_local *, int, u32);
+
int tx_irq;
int rx_irq;
int emac_num;
diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index ba617e3..6270bb0 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -20,9 +20,6 @@
* or rx, so this should be okay.
*
* TODO:
- * - Fix driver to work on more than just Virtex5. Right now the driver
- * assumes that the locallink DMA registers are accessed via DCR
- * instructions.
* - Factor out locallink DMA code into separate driver
* - Fix multicast assignment.
* - Fix support for hardware checksumming.
@@ -116,17 +113,86 @@ void temac_indirect_out32(struct temac_local *lp, int
reg, u32 value)
temac_iow(lp, XTE_CTL0_OFFSET, CNTLREG_WRITE_ENABLE_MASK | reg);
}
+/**
+ * temac_dma_in32 - Memory mapped DMA read, this function expects a
+ * register input that is based on DCR word addresses which
+ * are then converted to memory mapped byte addresses
+ */
static u32 temac_dma_in32(struct temac_local *lp, int reg)
{
- return dcr_read(lp-sdma_dcrs, reg);
+ return
Re: ppc4xx emac support of phy-less devices patch + dts example
On Fri, 2010-02-19 at 13:32 +0100, staale.aakerm...@kongsberg.com wrote:
Hi,
Hi !
Sorry for the delay, I've been busy with other things and your message
slipped a bit through the cracks.
I'm currently working on a custom embedded card with ppc405ex
installed.
On this card, EMAC1 is phy-less, and connected directly to a MAC on
a micrel switch (ks8995ma).
I've tried the default phy-less mode currently supported by the core
driver for ibm_newmac, but found
this insufficient. I've made a patch for the core driver so it
supports some more scenarios.
.../...
So first thing first, your patch is completely whitespace damaged :-)
Now, as for your aproach:
+if (emac_read_uint_prop(np, phy-duplex, dev-phy.duplex, 0))
+dev-phy.duplex = 1;
+
+if (emac_read_uint_prop(np, phy-speed, dev-phy.speed, 0))
+dev-phy.speed = 100;
+
+if (emac_read_uint_prop(np, phy-autoneg, dev-phy.autoneg, 0))
+dev-phy.autoneg = 0;
+
No objection on the method above. It might be nicer to call those
fixed-phy-duplex, fixed-phy-speed, etc... and have a fixed-phy empty
property as a cleaner way to represent the phyless mode but that's not a
very big deal.
dev-phy.address = -1;
dev-phy.features = SUPPORTED_MII;
- if (emac_phy_supports_gige(dev-phy_mode))
- dev-phy.features |= SUPPORTED_1000baseT_Full;
- else
- dev-phy.features |= SUPPORTED_100baseT_Full;
So you remove the above which sets the 1000bT support...
+
+if (dev-phy.autoneg == 1 )
+dev-phy.features |= SUPPORTED_Autoneg;
+
+switch (dev-phy.duplex)
+{
+case 0:
+
+if (dev-phy.speed == 10 )
+dev-phy.features |= SUPPORTED_10baseT_Half;
+
+if (dev-phy.speed == 100 )
+dev-phy.features |= SUPPORTED_100baseT_Half;
+
+if (dev-phy.speed == 100 )
+dev-phy.features |= SUPPORTED_100baseT_Half;
+
+default:
Ditto...
+if (dev-phy.speed == 10 )
+dev-phy.features |= SUPPORTED_10baseT_Full;
+
+if (dev-phy.speed == 100 )
+dev-phy.features |= SUPPORTED_100baseT_Full;
+
+if (dev-phy.speed == 100 )
+dev-phy.features |= SUPPORTED_100baseT_Full;
+}
+
And never add back any option for 1000bT ... Not nice :-)
You should add the case back for 1000bT (btw, turn the above into a
switch/case statement please). And if phy.speed is none of the above
(default, ie, the property is not present in the device-tree), then
revert to the old method, or something like that. Or you may break
existing stuff.
dev-phy.pause = 1;
+#if defined (CONFIG_PPC_DCR_NATIVE) defined (CONFIG_405EX)
+if (of_get_property(np, phy-clock-internal, NULL))
+dcri_clrset(SDR0, SDR0_MFR, ( dev-rgmii_port ?
SDR0_MFR_ECS 1 : SDR0_MFR_ECS ),0 );
+#endif
+
Ok.
Cheers,
Ben.
return 0;
}
DTS example:
EMAC1: ether...@ef600a00 {
linux,network-index = 0x1;
device_type = network;
compatible = ibm,emac-405ex, ibm,emac4sync;
interrupt-parent = EMAC1;
interrupts = 0x0 0x1;
#interrupt-cells = 1;
#address-cells = 0;
#size-cells = 0;
interrupt-map = /*Status*/ 0x0 UIC0 0x19 0x4
/*Wake*/ 0x1 UIC1 0x1f 0x4;
reg = 0xef600a00 0x00c4;
local-mac-address = [00];
mal-device = MAL0;
mal-tx-channel = 1;
mal-rx-channel = 1;
cell-index = 1;
max-frame-size = 9000;
rx-fifo-size = 4096;
tx-fifo-size = 2048;
phy-mode = rgmii;
phy-map = 0x; // Be sure that the emac use
phy-less configuration
phy-address = 0x; // Be sure that the emac use
phy-less configuration
phy-speed = 100;
phy-duplex = 1;
phy-autoneg = 0;
phy-clock-internal;
rgmii-device = RGMII0;
rgmii-channel = 1;
has-inverted-stacr-oc;
has-new-stacr-staopc;
};
Best regards
Staale Aakermann, Systems Engineer
Kongsberg Defence Systems / Defence Communication
Olav Brunborgsv 6, P.O.Box 87, 1375 Billingstad, Norway
Phone: +47 66 84 24 00, Direct Phone: +47 66 74 48 51,
Fax: +47 66 84 82 30, Mobile: +47 928 29 879
Mail: staale.aakerm...@kongsberg.com
Url: www.kongsberg.com
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing
[PATCH] powerpc/pseries: Flush lazy kernel mappings after unplug operations
This ensures that the translations for unmapped IO mappings or unmapped memory are properly removed from the MMU hash table before such an unplug. Without this, the hypervisor refuses the unplug operations due to those resources still being mapped by the partition. Signed-off-by: Benjamin Herrenschmidt b...@kernel.crashing.org --- arch/powerpc/platforms/pseries/hotplug-memory.c |7 +++ drivers/pci/hotplug/rpadlpar_core.c |3 +++ drivers/pci/hotplug/rpaphp_core.c |3 +++ 3 files changed, 13 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c index 9b21ee6..01e7b5b 100644 --- a/arch/powerpc/platforms/pseries/hotplug-memory.c +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c @@ -11,6 +11,7 @@ #include linux/of.h #include linux/lmb.h +#include linux/vmalloc.h #include asm/firmware.h #include asm/machdep.h #include asm/pSeries_reconfig.h @@ -54,6 +55,12 @@ static int pseries_remove_lmb(unsigned long base, unsigned int lmb_size) */ start = (unsigned long)__va(base); ret = remove_section_mapping(start, start + lmb_size); + + /* Ensure all vmalloc mappings are flushed in case they also +* hit that section of memory +*/ + vm_unmap_aliases(); + return ret; } diff --git a/drivers/pci/hotplug/rpadlpar_core.c b/drivers/pci/hotplug/rpadlpar_core.c index 4e3e038..0830347 100644 --- a/drivers/pci/hotplug/rpadlpar_core.c +++ b/drivers/pci/hotplug/rpadlpar_core.c @@ -20,6 +20,7 @@ #include linux/init.h #include linux/pci.h #include linux/string.h +#include linux/vmalloc.h #include asm/pci-bridge.h #include linux/mutex.h @@ -430,6 +431,8 @@ int dlpar_remove_slot(char *drc_name) rc = dlpar_remove_pci_slot(drc_name, dn); break; } + vm_unmap_aliases(); + printk(KERN_INFO %s: slot %s removed\n, DLPAR_MODULE_NAME, drc_name); exit: mutex_unlock(rpadlpar_mutex); diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c index dcaae72..0aefc96 100644 --- a/drivers/pci/hotplug/rpaphp_core.c +++ b/drivers/pci/hotplug/rpaphp_core.c @@ -30,6 +30,7 @@ #include linux/slab.h #include linux/smp.h #include linux/init.h +#include linux/vmalloc.h #include asm/eeh.h /* for eeh_add_device() */ #include asm/rtas.h /* rtas_call */ #include asm/pci-bridge.h/* for pci_controller */ @@ -419,6 +420,8 @@ static int disable_slot(struct hotplug_slot *hotplug_slot) return -EINVAL; pcibios_remove_pci_devices(slot-bus); + vm_unmap_aliases(); + slot-state = NOT_CONFIGURED; return 0; } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC] powerpc: add support for new hcall H_BEST_ENERGY
On Wed, 2010-03-03 at 23:48 +0530, Vaidyanathan Srinivasan wrote:
Hi,
diff --git a/arch/powerpc/kernel/setup-common.c
b/arch/powerpc/kernel/setup-common.c
index 03dd6a2..fbd93e3 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -359,6 +359,8 @@ void __init check_for_initrd(void)
#ifdef CONFIG_SMP
int threads_per_core, threads_shift;
+EXPORT_SYMBOL_GPL(threads_per_core);
While I agree it should be exported for the APIs in cputhread.h to be
usable from a module, this variable shouldn't be -used- directly, but
only via the API functions in there.
.../...
+
+#define MODULE_VERS 1.0
+#define MODULE_NAME pseries_energy
+
+/* Helper Routines to convert between drc_index to cpu numbers */
+
+static u32 cpu_to_drc_index(int cpu)
+{
+ struct device_node *dn = NULL;
+ const int *indexes;
+ int i;
+ dn = of_find_node_by_name(dn, cpus);
Check the result. Also that's not a nice way to do that, you should look
for /cpus by path I reckon.
+ indexes = of_get_property(dn, ibm,drc-indexes, NULL);
Check the result here too.
+ /* Convert logical cpu number to core number */
+ i = cpu / threads_per_core;
Don't use that variable as I said earlier. Use cpu_thread_to_core()
+ /*
+ * The first element indexes[0] is the number of drc_indexes
+ * returned in the list. Hence i+1 will get the drc_index
+ * corresponding to core number i.
+ */
+ WARN_ON(i indexes[0]);
+ return indexes[i + 1];
+}
+
+static int drc_index_to_cpu(u32 drc_index)
+{
+ struct device_node *dn = NULL;
+ const int *indexes;
+ int i, cpu;
+ dn = of_find_node_by_name(dn, cpus);
+ indexes = of_get_property(dn, ibm,drc-indexes, NULL);
Same comments, check results and use /cpus path
+ /*
+ * First element in the array is the number of drc_indexes
+ * returned. Search through the list to find the matching
+ * drc_index and get the core number
+ */
+ for (i = 0; i indexes[0]; i++) {
+ if (indexes[i + 1] == drc_index)
+ break;
+ }
+ /* Convert core number to logical cpu number */
+ cpu = i * threads_per_core;
Here's more annoying as we don't have an API in cputhread.h for that.
In fact, we have a confusion in there since cpu_first_thread_in_core()
doesn't actually take a core number ...
I'm going to recommend a complicated approach but that's the best in the
long run:
- First do a patch that renames cpu_first_thread_in_core() to something
clearer like cpu_first_thread_in_same_core() or cpu_leftmost_sibling()
(I think I prefer the later). Rename the few users in
arch/powerpc/mm/mmu_context_nohash.c and arch/powerpc/kernel/smp.c
- Then do a patch that adds a cpu_first_thread_of_core() that takes a
core number, basically does:
static inline int cpu_first_thread_of_core(int core)
{
return core threads_shift;
}
- Then add your modified H_BEST_ENERGY patch on top of it using the
above two as pre-reqs :-)
+ return cpu;
+}
+
+/*
+ * pseries hypervisor call H_BEST_ENERGY provides hints to OS on
+ * preferred logical cpus to activate or deactivate for optimized
+ * energy consumption.
+ */
+
+#define FLAGS_MODE1 0x004E20080E01
+#define FLAGS_MODE2 0x004E20080401
+#define FLAGS_ACTIVATE 0x100
+
+static ssize_t get_best_energy_list(char *page, int activate)
+{
+ int rc, cnt, i, cpu;
+ unsigned long retbuf[PLPAR_HCALL9_BUFSIZE];
+ unsigned long flags = 0;
+ u32 *buf_page;
+ char *s = page;
+
+ buf_page = (u32 *) get_zeroed_page(GFP_KERNEL);
+
Why that blank line ?
+ if (!buf_page)
+ return 0;
So here you return 0 instead of -ENOMEM
+ flags = FLAGS_MODE1;
+ if (activate)
+ flags |= FLAGS_ACTIVATE;
+
+ rc = plpar_hcall9(H_BEST_ENERGY, retbuf, flags, 0, __pa(buf_page),
+ 0, 0, 0, 0, 0, 0);
+
+
Again, no need for a blank line before xx = foo() and if (xx)
if (rc != H_SUCCESS) {
+ free_page((unsigned long) buf_page);
+ return -EINVAL;
+ }
And here you return an error code. Which one is right ?
+ cnt = retbuf[0];
+ for (i = 0; i cnt; i++) {
+ cpu = drc_index_to_cpu(buf_page[2*i+1]);
+ if ((cpu_online(cpu) !activate) ||
+ (!cpu_online(cpu) activate))
+ s += sprintf(s, %d,, cpu);
+ }
+ if (s page) { /* Something to show */
+ s--; /* Suppress last comma */
+ s += sprintf(s, \n);
+ }
+
+ free_page((unsigned long) buf_page);
+ return s-page;
+}
+
+static ssize_t get_best_energy_data(struct sys_device *dev,
+ char *page, int activate)
+{
+ int rc;
+ unsigned long retbuf[PLPAR_HCALL9_BUFSIZE];
+ unsigned long flags = 0;
+
+ flags =
Re: [PATCH] [V3] Add non-Virtex5 support for LL TEMAC driver
From: Eric Dumazet eric.duma...@gmail.com Date: Mon, 05 Apr 2010 23:29:53 +0200 So, I ask, cant you use netdev_alloc_skb_ip_align() in this driver ? Thanks to everyone for getting this patch into shape. I applied version 4 of the patch to net-next-2.6, thanks! ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: powermac/low_i2c.c: three minor problems
On Sat, 2010-02-06 at 12:13 +, d binderman wrote: Hello there, I just ran the sourceforge tool cppcheck over the source code of the new Linux kernel 2.6.33-rc6 Hi ! All your patches have various problems with the actual patch format. Patches should be submitted so that they apply with patch -p1, yours don't. In fact, the base directory you diffed from seems to differ from patch to patch. I'm manually fixing those this time around, but please get that sorted out before you submit a new batch. Cheers, Ben. It said [./arch/powerpc/platforms/powermac/low_i2c.c:594]: (style) The scope of the variable chans can be reduced [./arch/powerpc/platforms/powermac/low_i2c.c:594]: (style) The scope of the variable i can be reduced [./arch/powerpc/platforms/powermac/low_i2c.c:1260]: (style) Redundant condition. It is safe to deallocate a NULL pointer Please find attached a patch to fix these minor problems. The patched version compiles, but I have not been able to test it at run time. Regards David Binderman _ Do you have a story that started on Hotmail? Tell us now http://clk.atdmt.com/UKM/go/195013117/direct/01/ ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC] powerpc: add support for new hcall H_BEST_ENERGY
* Benjamin Herrenschmidt b...@kernel.crashing.org [2010-04-07 12:04:49]:
On Wed, 2010-03-03 at 23:48 +0530, Vaidyanathan Srinivasan wrote:
Hi,
diff --git a/arch/powerpc/kernel/setup-common.c
b/arch/powerpc/kernel/setup-common.c
index 03dd6a2..fbd93e3 100644
--- a/arch/powerpc/kernel/setup-common.c
+++ b/arch/powerpc/kernel/setup-common.c
@@ -359,6 +359,8 @@ void __init check_for_initrd(void)
#ifdef CONFIG_SMP
int threads_per_core, threads_shift;
+EXPORT_SYMBOL_GPL(threads_per_core);
While I agree it should be exported for the APIs in cputhread.h to be
usable from a module, this variable shouldn't be -used- directly, but
only via the API functions in there.
Ok agreed. This will be the first module to use this and hence will
have to implement the API and export the function.
.../...
+
+#define MODULE_VERS 1.0
+#define MODULE_NAME pseries_energy
+
+/* Helper Routines to convert between drc_index to cpu numbers */
+
+static u32 cpu_to_drc_index(int cpu)
+{
+ struct device_node *dn = NULL;
+ const int *indexes;
+ int i;
+ dn = of_find_node_by_name(dn, cpus);
Check the result. Also that's not a nice way to do that, you should look
for /cpus by path I reckon.
I will check the return code, but why do you feel getting the node by
path is better? Is there any race, or we may have duplicate cpus
node?
+ indexes = of_get_property(dn, ibm,drc-indexes, NULL);
Check the result here too.
Yes, will do.
+ /* Convert logical cpu number to core number */
+ i = cpu / threads_per_core;
Don't use that variable as I said earlier. Use cpu_thread_to_core()
Ack
+ /*
+* The first element indexes[0] is the number of drc_indexes
+* returned in the list. Hence i+1 will get the drc_index
+* corresponding to core number i.
+*/
+ WARN_ON(i indexes[0]);
+ return indexes[i + 1];
+}
+
+static int drc_index_to_cpu(u32 drc_index)
+{
+ struct device_node *dn = NULL;
+ const int *indexes;
+ int i, cpu;
+ dn = of_find_node_by_name(dn, cpus);
+ indexes = of_get_property(dn, ibm,drc-indexes, NULL);
Same comments, check results and use /cpus path
Sure. Will do.
+ /*
+* First element in the array is the number of drc_indexes
+* returned. Search through the list to find the matching
+* drc_index and get the core number
+*/
+ for (i = 0; i indexes[0]; i++) {
+ if (indexes[i + 1] == drc_index)
+ break;
+ }
+ /* Convert core number to logical cpu number */
+ cpu = i * threads_per_core;
Here's more annoying as we don't have an API in cputhread.h for that.
In fact, we have a confusion in there since cpu_first_thread_in_core()
doesn't actually take a core number ...
I'm going to recommend a complicated approach but that's the best in the
long run:
- First do a patch that renames cpu_first_thread_in_core() to something
clearer like cpu_first_thread_in_same_core() or cpu_leftmost_sibling()
(I think I prefer the later). Rename the few users in
arch/powerpc/mm/mmu_context_nohash.c and arch/powerpc/kernel/smp.c
- Then do a patch that adds a cpu_first_thread_of_core() that takes a
core number, basically does:
static inline int cpu_first_thread_of_core(int core)
{
return core threads_shift;
}
- Then add your modified H_BEST_ENERGY patch on top of it using the
above two as pre-reqs :-)
Ok, I can do that change and then base this patch on the new API.
+ return cpu;
+}
+
+/*
+ * pseries hypervisor call H_BEST_ENERGY provides hints to OS on
+ * preferred logical cpus to activate or deactivate for optimized
+ * energy consumption.
+ */
+
+#define FLAGS_MODE10x004E20080E01
+#define FLAGS_MODE20x004E20080401
+#define FLAGS_ACTIVATE 0x100
+
+static ssize_t get_best_energy_list(char *page, int activate)
+{
+ int rc, cnt, i, cpu;
+ unsigned long retbuf[PLPAR_HCALL9_BUFSIZE];
+ unsigned long flags = 0;
+ u32 *buf_page;
+ char *s = page;
+
+ buf_page = (u32 *) get_zeroed_page(GFP_KERNEL);
+
Why that blank line ?
I will remove them
+ if (!buf_page)
+ return 0;
So here you return 0 instead of -ENOMEM
Will fix. -ENOMEM is correct.
+ flags = FLAGS_MODE1;
+ if (activate)
+ flags |= FLAGS_ACTIVATE;
+
+ rc = plpar_hcall9(H_BEST_ENERGY, retbuf, flags, 0, __pa(buf_page),
+ 0, 0, 0, 0, 0, 0);
+
+
Again, no need for a blank line before xx = foo() and if (xx)
Ack.
if (rc != H_SUCCESS) {
+ free_page((unsigned long) buf_page);
+ return -EINVAL;
+ }
And here you return an error code. Which one is right ?
Returning an error code is correct to user space. The call will check
for return value on read while getting the list.
Thanks for the detailed review. I will fix the issues that you
pointed
Re: [RFC] powerpc: add support for new hcall H_BEST_ENERGY
On Wed, 2010-04-07 at 10:27 +0530, Vaidyanathan Srinivasan wrote: Check the result. Also that's not a nice way to do that, you should look for /cpus by path I reckon. I will check the return code, but why do you feel getting the node by path is better? Is there any race, or we may have duplicate cpus node? Well, you never really know what might be down the device-tree :-) /cpus is well defined where it is, while god knows what would happen if somebody had the weird idea to have something called cpus below some device driver for whatever reason they might even. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
