sblk->status_tag on drivers/net/ethernet/broadcom/tg3.c

sblk->status_tag on drivers/net/ethernet/broadcom/tg3.c

[Jagan Teki]

Hi All,

I have a board setup with PCIe host bridge links to BCRM NIC/tg3 EP.

I'm verifying legacy INT# for ifconfig up operation, typically for
link up EP trigger an interrupt as
Assert_INT# and PCIe RC driver ISR will called and then EP ISR will
clear and send Dessert_INT#

But I got the continues interrupts from EP which will keep ISR's in
both PCIe RC and EP in busy loop.

Log dump:
# ifconfig eth0 172.16.0.2 up
RC: handler
tg3_interrupt_tagged: write status = 0x1
RC: handler
tg3_interrupt_tagged: write status = 0x1
RC: handler
tg3_interrupt_tagged: write status = 0x1
RC: handler
tg3_interrupt_tagged: write status = 0x1
............................
..............

I debugged further on to EP driver where I could see there is
interrupt re-enabling is happining
by writing 0x0 to onto MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW

Debug code:
static irqreturn_t tg3_interrupt_tagged(int irq, void *dev_id)
{
      ...................
      printk(" ++status_tag = 0x%x", sblk->status_tag);
      .......................

      tw32_mailbox_f(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW, 0x00000001);
      printk(" write status = 0x%x", tr32(MAILBOX_INTERRUPT_0 +
TG3_64BIT_REG_LOW));
      ..................
      tnapi->last_irq_tag = sblk->status_tag;
      printk(" --status_tag = 0x%x\n", tnapi->last_irq_tag);
}

static int tg3_poll(struct napi_struct *napi, int budget)
{

               if (tg3_flag(tp, TAGGED_STATUS)) {
                        /* tp->last_tag is used in tg3_int_reenable() below
                         * to tell the hw how much work has been processed,
                         * so we must read it before checking for more work.
                         */
                        tnapi->last_tag = sblk->status_tag;
                        tnapi->last_irq_tag = tnapi->last_tag;
                        rmb();
                } else
                        sblk->status &= ~SD_STATUS_UPDATED;

                printk(" tag = 0x%x\n", sblk->status_tag);
                if (likely(!tg3_has_work(tnapi))) {
                        napi_complete(napi);
                        printk("calling tg3_int_reenable\n");
                        tg3_int_reenable(tnapi);
                        break;
              }
}

static void tg3_int_reenable(struct tg3_napi *tnapi)
{
        struct tg3 *tp = tnapi->tp;

        printk("%s val = 0x%x\n", __func__, tnapi->last_tag << 24);
        tw32_mailbox(tnapi->int_mbox, tnapi->last_tag << 24);
        ..............................
}

Log dump:

# ifconfig eth0 172.16.0.2 up
RC: handler
tg3_interrupt_tagged: ++status_tag = 0x0 write status = 0x1 --status_tag = 0x0
RC: handler
tg3_interrupt_tagged: write status = 0x1
RC: handler
tg3_interrupt_tagged: write status = 0x1
RC: handler
tg3_interrupt_tagged: write status = 0x1
............................
..............








thanks!
-- 
Jagan.

Re: sblk->status_tag on drivers/net/ethernet/broadcom/tg3.c

[Jagan Teki]

Accidental sent please ignore this it's incomplete

On Tue, Mar 4, 2014 at 11:09 PM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> Hi All,
>
> I have a board setup with PCIe host bridge links to BCRM NIC/tg3 EP.
>
> I'm verifying legacy INT# for ifconfig up operation, typically for
> link up EP trigger an interrupt as
> Assert_INT# and PCIe RC driver ISR will called and then EP ISR will
> clear and send Dessert_INT#
>
> But I got the continues interrupts from EP which will keep ISR's in
> both PCIe RC and EP in busy loop.
>
> Log dump:
> # ifconfig eth0 172.16.0.2 up
> RC: handler
> tg3_interrupt_tagged: write status = 0x1
> RC: handler
> tg3_interrupt_tagged: write status = 0x1
> RC: handler
> tg3_interrupt_tagged: write status = 0x1
> RC: handler
> tg3_interrupt_tagged: write status = 0x1
> ............................
> ..............
>
> I debugged further on to EP driver where I could see there is
> interrupt re-enabling is happining
> by writing 0x0 to onto MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW
>
> Debug code:
> static irqreturn_t tg3_interrupt_tagged(int irq, void *dev_id)
> {
>       ...................
>       printk(" ++status_tag = 0x%x", sblk->status_tag);
>       .......................
>
>       tw32_mailbox_f(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW, 0x00000001);
>       printk(" write status = 0x%x", tr32(MAILBOX_INTERRUPT_0 +
> TG3_64BIT_REG_LOW));
>       ..................
>       tnapi->last_irq_tag = sblk->status_tag;
>       printk(" --status_tag = 0x%x\n", tnapi->last_irq_tag);
> }
>
> static int tg3_poll(struct napi_struct *napi, int budget)
> {
>
>                if (tg3_flag(tp, TAGGED_STATUS)) {
>                         /* tp->last_tag is used in tg3_int_reenable() below
>                          * to tell the hw how much work has been processed,
>                          * so we must read it before checking for more work.
>                          */
>                         tnapi->last_tag = sblk->status_tag;
>                         tnapi->last_irq_tag = tnapi->last_tag;
>                         rmb();
>                 } else
>                         sblk->status &= ~SD_STATUS_UPDATED;
>
>                 printk(" tag = 0x%x\n", sblk->status_tag);
>                 if (likely(!tg3_has_work(tnapi))) {
>                         napi_complete(napi);
>                         printk("calling tg3_int_reenable\n");
>                         tg3_int_reenable(tnapi);
>                         break;
>               }
> }
>
> static void tg3_int_reenable(struct tg3_napi *tnapi)
> {
>         struct tg3 *tp = tnapi->tp;
>
>         printk("%s val = 0x%x\n", __func__, tnapi->last_tag << 24);
>         tw32_mailbox(tnapi->int_mbox, tnapi->last_tag << 24);
>         ..............................
> }
>
> Log dump:
>
> # ifconfig eth0 172.16.0.2 up
> RC: handler
> tg3_interrupt_tagged: ++status_tag = 0x0 write status = 0x1 --status_tag = 0x0
> RC: handler
> tg3_interrupt_tagged: write status = 0x1
> RC: handler
> tg3_interrupt_tagged: write status = 0x1
> RC: handler
> tg3_interrupt_tagged: write status = 0x1
> ............................
> ..............
>
>
>
>
>
>
>
>
> thanks!
> --
> Jagan.



-- 
Jagan.

Re: sblk->status_tag on drivers/net/ethernet/broadcom/tg3.c

[Jagan Teki]

Hi All,

I have a board setup with PCIe host bridge links to BCRM NIC/tg3 EP.

I'm verifying legacy INT# for ifconfig up operation, typically for
link up EP trigger an interrupt as Assert_INT# and PCIe RC driver
ISR will called and then EP ISR will clear and send Dessert_INT#

But I got the continues interrupts from EP which will keep ISR's in
both PCIe RC and EP in busy loop.

Log dump:
# ifconfig eth0 172.16.0.2 up
RC: handler
tg3_interrupt_tagged: write status = 0x1
RC: handler
tg3_interrupt_tagged: write status = 0x1
RC: handler
tg3_interrupt_tagged: write status = 0x1
RC: handler
tg3_interrupt_tagged: write status = 0x1
............................
..............

I debugged further on EP driver where I could see interrupt
re-enabling is happening continuously by writing 0x0 to onto
MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW

Debug code:
static irqreturn_t tg3_interrupt_tagged(int irq, void *dev_id)
{
      ...................
      printk(" ++status_tag = 0x%x", sblk->status_tag);
      .......................

      tw32_mailbox_f(MAILBOX_
INTERRUPT_0 + TG3_64BIT_REG_LOW, 0x00000001);
      printk(" write status = 0x%x", tr32(MAILBOX_INTERRUPT_0 +
TG3_64BIT_REG_LOW));
      ..................
      tnapi->last_irq_tag = sblk->status_tag;
      printk(" --status_tag = 0x%x\n", tnapi->last_irq_tag);
}

static int tg3_poll(struct napi_struct *napi, int budget)
{
               printk("%s: ", __func__);
               if (tg3_flag(tp, TAGGED_STATUS)) {
                        /* tp->last_tag is used in tg3_int_reenable() below
                         * to tell the hw how much work has been processed,
                         * so we must read it before checking for more work.
                         */
                        tnapi->last_tag = sblk->status_tag;
                        tnapi->last_irq_tag = tnapi->last_tag;
                        rmb();
                } else
                        sblk->status &= ~SD_STATUS_UPDATED;

                printk(" tag = 0x%x\n", sblk->status_tag);
                if (likely(!tg3_has_work(tnapi))) {
                        napi_complete(napi);
                        printk("calling tg3_int_reenable\n");
                        tg3_int_reenable(tnapi);
                        break;
              }
}

static void tg3_int_reenable(struct tg3_napi *tnapi)
{
        struct tg3 *tp = tnapi->tp;

        printk("%s val = 0x%x\n", __func__, tnapi->last_tag << 24);
        tw32_mailbox(tnapi->int_mbox, tnapi->last_tag << 24);
        ..............................
}

Log dump:
# ifconfig eth0 172.16.0.2 up
RC: handler
tg3_interrupt_tagged: ++status_tag = 0x0 write status = 0x1
--status_tag = 0x0 tg3_poll: tag = 0x0 calling tg3_int_reenable val =
0x0
RC: handler
tg3_interrupt_tagged: ++status_tag = 0x0 write status = 0x1
--status_tag = 0x0 tg3_poll: tag = 0x0 calling tg3_int_reenable val =
0x0
RC: handler
tg3_interrupt_tagged: ++status_tag = 0x0 write status = 0x1
--status_tag = 0x0 tg3_poll: tag = 0x0 calling tg3_int_reenable val =
0x0
RC: handler
tg3_interrupt_tagged: ++status_tag = 0x0 write status = 0x1
--status_tag = 0x0 tg3_poll: tag = 0x0 calling tg3_int_reenable val =
0x0
............................
..............

>From the comments in ISR any non-zero value to intr-mbox-0 clear
the interrupt ie write status as 1 so I assume interrupt is clear and then
in tg3_int_reenable is going to write 0x0 on to intr-mbox-0
which again reenables the interrupt, was my guess is true?

And sblk->status_tag is always 0 in ISR and BH(poll) why is that so?
If it's is 1(unsigned non-zero) then possibly tg3_int_reenable will write
non-zero value on intr-mbox-0 which will clear the interrupt.

Request for any help who updates sblk->status_tag value and
how many time the ISR in PCIe RC and ISR in EP will call for link up?

thanks!
--
Jagan.

Re: sblk->status_tag on drivers/net/ethernet/broadcom/tg3.c

[Michael Chan]

On Tue, 2014-03-04 at 23:25 +0530, Jagan Teki wrote: 
> From the comments in ISR any non-zero value to intr-mbox-0 clear
> the interrupt ie write status as 1 so I assume interrupt is clear and then
> in tg3_int_reenable is going to write 0x0 on to intr-mbox-0
> which again reenables the interrupt, was my guess is true?

Yes, the (last_tag << 24) is also written to the intr-mbox-0 to
acknowledge the status block.


> And sblk->status_tag is always 0 in ISR and BH(poll) why is that so?
> If it's is 1(unsigned non-zero) then possibly tg3_int_reenable will write
> non-zero value on intr-mbox-0 which will clear the interrupt.

You should print out the entire status block.  It shouldn't be zero.
The status tag should be incrementing for each interrupt.

> 
> Request for any help who updates sblk->status_tag value and
> how many time the ISR in PCIe RC and ISR in EP will call for link up?
> 
> 
If you are expecting link up, the first 32-bit word of the status block
should have bit 1 set (bit 0 should also be set).  I think the driver is
not seeing the status block correctly, so it is not acknowledging the
link change interrupt and the status_tag.

Re: sblk->status_tag on drivers/net/ethernet/broadcom/tg3.c

[Jagan Teki]

On Wed, Mar 5, 2014 at 4:52 AM, Michael Chan <mchan@broadcom.com> wrote:
> On Tue, 2014-03-04 at 23:25 +0530, Jagan Teki wrote:
>> From the comments in ISR any non-zero value to intr-mbox-0 clear
>> the interrupt ie write status as 1 so I assume interrupt is clear and then
>> in tg3_int_reenable is going to write 0x0 on to intr-mbox-0
>> which again reenables the interrupt, was my guess is true?
>
> Yes, the (last_tag << 24) is also written to the intr-mbox-0 to
> acknowledge the status block.

Yes, I saw it.

>
>> And sblk->status_tag is always 0 in ISR and BH(poll) why is that so?
>> If it's is 1(unsigned non-zero) then possibly tg3_int_reenable will write
>> non-zero value on intr-mbox-0 which will clear the interrupt.
>
> You should print out the entire status block.  It shouldn't be zero.
> The status tag should be incrementing for each interrupt.

I see the status_tag is 0, at starting of ISR.
OK will try to print entire block members.

>> Request for any help who updates sblk->status_tag value and
>> how many time the ISR in PCIe RC and ISR in EP will call for link up?
>>
>>
> If you are expecting link up, the first 32-bit word of the status block
> should have bit 1 set (bit 0 should also be set).  I think the driver is
> not seeing the status block correctly, so it is not acknowledging the
> link change interrupt and the status_tag.

The driver is not seeing the status block mean - tg3.c driver?
Does it relates to ISR or addressing of PCIe RC driver, I'm doing
nothing on my PCIe RC driver just read the legacy INT# status and return.

Any inputs if you see it on PCIe RC, let me know.

thanks!
-- 
Jagan.

Re: sblk->status_tag on drivers/net/ethernet/broadcom/tg3.c

[Michael Chan]

On Wed, 2014-03-05 at 09:33 +0530, Jagan Teki wrote:
> The driver is not seeing the status block mean - tg3.c driver?

Yes, tg3 driver.  All IRQ events (link change, rx, tx) are in status
block.  If tg3 does not see the correct bits and indexes in the status
block, it won't know what to do.  In this case, it doesn't see the link
change bit in status block (I suspect), so it is not acknowledging and
clearing the link change event.  Status block is DMA'ed from the NIC to
memory.

> Does it relates to ISR or addressing of PCIe RC driver, I'm doing
> nothing on my PCIe RC driver just read the legacy INT# status and
> return.

RC stands for what?  Root Complex?  I don't know what an RC driver does.

Re: sblk->status_tag on drivers/net/ethernet/broadcom/tg3.c

[Jagan Teki]

Hi Michael,

Thanks for your inputs.

On Wed, Mar 5, 2014 at 9:42 AM, Michael Chan <mchan@broadcom.com> wrote:
> On Wed, 2014-03-05 at 09:33 +0530, Jagan Teki wrote:
>> The driver is not seeing the status block mean - tg3.c driver?
>
> Yes, tg3 driver.  All IRQ events (link change, rx, tx) are in status
> block.  If tg3 does not see the correct bits and indexes in the status
> block, it won't know what to do.  In this case, it doesn't see the link
> change bit in status block (I suspect), so it is not acknowledging and
> clearing the link change event.  Status block is DMA'ed from the NIC to
> memory.

OK.

As per your point I've a question here like - the status block is not properly
updated (DMA'ed), does this depends on the address range we are advertised
on PCIe ranges filed to make sure to use EP-
http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge

The reason for asking this is we have two kind of PCIe bridge in separate
boards where one is working fine by showing below log, and other
we could see this issue.

# ifconfig eth0 172.16.0.2 up
RC: handler
tg3_interrupt_tagged: ++status_tag = 0x1 write status = 0x1
--status_tag = 0x1
RC: handler
tg3_interrupt_tagged: ++status_tag = 0x1 write status = 0x1
--status_tag = 0x1 tg3_poll: tag = 0x1 calling tg3_int_reenable val =
0x01000000

>
>> Does it relates to ISR or addressing of PCIe RC driver, I'm doing
>> nothing on my PCIe RC driver just read the legacy INT# status and
>> return.
>
> RC stands for what?  Root Complex?  I don't know what an RC driver does.

Yes, Root Complex driver in PCIe bridge.

-- 
Jagan.

Re: sblk->status_tag on drivers/net/ethernet/broadcom/tg3.c

[Michael Chan]

On Wed, 2014-03-05 at 11:43 +0530, Jagan Teki wrote: 
> As per your point I've a question here like - the status block is not properly
> updated (DMA'ed), does this depends on the address range we are advertised
> on PCIe ranges filed to make sure to use EP-
> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge 

The status block is in host memory (CPU memory).  This memory is
allocated by the driver.  The DMA address of the status block is then
programmed into the NIC register HOSTCC_STATUS_BLK_HOST_ADDR so that the
NIC knows where to DMA the status block.  When there is an event, the
NIC DMAs a new status block and then generates IRQ.

The status block is not in PCI memory.  PCI memory is where the
registers are mapped.  Hope this helps.

Re: sblk->status_tag on drivers/net/ethernet/broadcom/tg3.c

[Jagan Teki]

On Wed, Mar 5, 2014 at 12:20 PM, Michael Chan <mchan@broadcom.com> wrote:
> On Wed, 2014-03-05 at 11:43 +0530, Jagan Teki wrote:
>> As per your point I've a question here like - the status block is not properly
>> updated (DMA'ed), does this depends on the address range we are advertised
>> on PCIe ranges filed to make sure to use EP-
>> http://devicetree.org/Device_Tree_Usage#PCI_Host_Bridge
>
> The status block is in host memory (CPU memory).  This memory is
> allocated by the driver.  The DMA address of the status block is then
> programmed into the NIC register HOSTCC_STATUS_BLK_HOST_ADDR so that the
> NIC knows where to DMA the status block.  When there is an event, the
> NIC DMAs a new status block and then generates IRQ.
>
> The status block is not in PCI memory.  PCI memory is where the
> registers are mapped.  Hope this helps.

Thanks this really help my understanding.

BTW: I got all 0's on tnapi->hw_status, looks like host memory contents were
violated or not-been updated properly I guess.

thanks!
-- 
Jagan.