wrong definition in cpm_8260.h?

wrong definition in cpm_8260.h?

[Shen Rong]

Hi,

In cpm_8260.h, BD_ENET_RX_STATS is defined to 0x1ff, which leads
to the fcc driver can't clean the BD_ENET_RX_LAST&BD_ENET_RX_FIRST bits
with the following statement:
    bdp->cbd_sc &= ~BD_ENET_RX_STATS;

The first&last bits are set by the CPM not the CPU, so they
should be cleared after the frame is received. So it should be:

#define BD_ENET_RX_STATS ((ushort)0x0fff) /* All status bits */

The same problem in ppcboot.


Shenrong


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: wrong definition in cpm_8260.h?

[Joakim Tjernlund]

Hi

The same error is in commproc.h for 8xx as well.

>
>
> Hi,
>
> In cpm_8260.h, BD_ENET_RX_STATS is defined to 0x1ff, which leads
> to the fcc driver can't clean the BD_ENET_RX_LAST&BD_ENET_RX_FIRST bits
> with the following statement:
>     bdp->cbd_sc &= ~BD_ENET_RX_STATS;
>
> The first&last bits are set by the CPM not the CPU, so they
> should be cleared after the frame is received. So it should be:
>
> #define BD_ENET_RX_STATS ((ushort)0x0fff) /* All status bits */
>
> The same problem in ppcboot.
>
>
> Shenrong
>
>
>
>

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: wrong definition in cpm_8260.h?

[Dan Malek]

Joakim Tjernlund wrote:

> The same error is in commproc.h for 8xx as well.

It's not an error in either driver since first/last will always be
set in every buffer.  There isn't any reason for the driver to test
or take any action based upon these indicators.

These are not data transfer error or status indicators in the sense
they indicate some condition on the network itself.  These are buffer
management indicators that should be managed as such if they are used.

Thanks.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

RE: wrong definition in cpm_8260.h?

[Joakim Tjernlund]

> Joakim Tjernlund wrote:
>
> > The same error is in commproc.h for 8xx as well.
>
> It's not an error in either driver since first/last will always be
> set in every buffer.  There isn't any reason for the driver to test
> or take any action based upon these indicators.

Yes, the CPM will set these flags, but will it also clear them if the
buffer is not first/last in a frame? I think not.

I know that the driver is designed to always set them so in practise it
won't be problem. However this could change some day.

Also, is it not time to remove the code contained in #ifndef final_version macros?

>
> These are not data transfer error or status indicators in the sense
> they indicate some condition on the network itself.  These are buffer
> management indicators that should be managed as such if they are used.

yes I know, but I still think those flags should be included in the RX_STATS macro.


    Jocke
>
> Thanks.
>
>
> 	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: wrong definition in cpm_8260.h?

[Dan Malek]

Joakim Tjernlund wrote:

> Yes, the CPM will set these flags, but will it also clear them if the
> buffer is not first/last in a frame? I think not.

It doesn't matter.  The flags are always going to be set.  Why bother
clearing them when you always set them?

> I know that the driver is designed to always set them so in practise it
> won't be problem. However this could change some day.

Well, when it changes we can change the code to set/clear the flags
according to the buffer management algorithm.


> yes I know, but I still think those flags should be included in the RX_STATS macro.

Why?  Like I said in the previous message, they aren't part of any network
status.  They are buffer control flags and need to properly indicate how the
buffers are managed, not cleared as part of network status.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: wrong definition in cpm_8260.h?

[Joakim Tjernlund]

From: "Dan Malek" <dan@embeddededge.com>
> Joakim Tjernlund wrote:
>
> > Yes, the CPM will set these flags, but will it also clear them if the
> > buffer is not first/last in a frame? I think not.
>
> It doesn't matter.  The flags are always going to be set.  Why bother
> clearing them when you always set them?
>
> > I know that the driver is designed to always set them so in practise it
> > won't be problem. However this could change some day.
>
> Well, when it changes we can change the code to set/clear the flags
> according to the buffer management algorithm.
>
>
> > yes I know, but I still think those flags should be included in the RX_STATS macro.
>
> Why?  Like I said in the previous message, they aren't part of any network
> status.  They are buffer control flags and need to properly indicate how the
> buffers are managed, not cleared as part of network status.

Suppose there is a bug in the driver(or someone introduces a bug or flaky HW) that will
generate a buffer that is not the first(or last) in this frame every now and then.
Since previously received frames did not clear these bits when they were done with
the RX BD, these bits will still be set and the driver won't detect the "bad" frames.

         Jocke

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: wrong definition in cpm_8260.h?

[Shen Rong]

Hi,

I write following message just for information:)

I am changing the fcc enet buffer management and interrupt
strategy to reduce the interrupt number when receiving the
small frame(say 64 bytes). I mask the frame received interrupt,
and unmask the buffer completed(i.e. RXB) interrupt, then use
the I bit and Last bit to control the interrupt number, that's to
say not to set all the I bit in BDs. When the cpm uses up several buffers
(maybe not receiving one frame), it will interrupt the cpu. It took me
some minutes to find the First&Last bits have not been included
in the RX_STATS. Well, I personly think there is no need to
change the RX_STATS if there are some comments just like what
Dan has said. But at least the so called final_version in fcc_enet_rx
need to be changed since it's do nothing but eat the cpu.

I still have the problems with the above solution:
1. If all the I bits are set only on the rx frames' last buffer, no interrupts
are trigered, and the cpm will silently use up all the BDs without notifying
the cpu.
2. There are some delay for cpu to know whether the packets are coming
if all the buffers previously received have no I bits set in the BDs.
3. maybe more
If you have any ideas, pls tell me. Thanks.

Shenrong

----- Original Message -----
From: "Joakim Tjernlund" <Joakim.Tjernlund@lumentis.se>
To: "Dan Malek" <dan@embeddededge.com>
Cc: "Shen Rong" <rshen@udtech.com.cn>; <linuxppc-embedded@lists.linuxppc.org>
Sent: Thursday, January 23, 2003 7:08 AM
Subject: Re: wrong definition in cpm_8260.h?


> From: "Dan Malek" <dan@embeddededge.com>
> > Joakim Tjernlund wrote:
> >
> > > Yes, the CPM will set these flags, but will it also clear them if the
> > > buffer is not first/last in a frame? I think not.
> >
> > It doesn't matter.  The flags are always going to be set.  Why bother
> > clearing them when you always set them?
> >
> > > I know that the driver is designed to always set them so in practise it
> > > won't be problem. However this could change some day.
> >
> > Well, when it changes we can change the code to set/clear the flags
> > according to the buffer management algorithm.
> >
> >
> > > yes I know, but I still think those flags should be included in the RX_STATS macro.
> >
> > Why?  Like I said in the previous message, they aren't part of any network
> > status.  They are buffer control flags and need to properly indicate how the
> > buffers are managed, not cleared as part of network status.
>
> Suppose there is a bug in the driver(or someone introduces a bug or flaky HW) that will
> generate a buffer that is not the first(or last) in this frame every now and then.
> Since previously received frames did not clear these bits when they were done with
> the RX BD, these bits will still be set and the driver won't detect the "bad" frames.
>
>          Jocke
>

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: wrong definition in cpm_8260.h?

[Dan Malek]

Shen Rong wrote:

> I am changing the fcc enet buffer management and interrupt
> strategy to reduce the interrupt number when receiving the
> small frame(say 64 bytes).

I don't understand.  The current driver will get exactly one
interrupt per frame (or maybe less if frames are coming in fast
and they are queued), regardless of the frame size.

> .... But at least the so called final_version in fcc_enet_rx
> need to be changed since it's do nothing but eat the cpu.

This was just for debugging, I don't even think that code is compiled anymore.
Take it out if you wish.  This driver is....hell....six or seven years old
already and was used to debug 8xx silicon.  We were happy when it actually
sent and received frames on the wire, so there is likely to be some debug
stuff here that can be removed.

The purpose for using first/last is so you can split a frame across multiple
BDs, making the use of memory a little more efficient.  Using this method,
you can't dma directly into sk buffers, you will have to check for the
buffer sequences and copy the data into the sk buffers.  This method also
increases the number of interrupts you will receive (all high performance
network devices are polled with carefully tuned data transfer functions)
and increases the amount of CPU needed for processing every frame.


	-- Dan


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

Re: wrong definition in cpm_8260.h?

[Shen Rong]

> I don't understand.  The current driver will get exactly one
> interrupt per frame (or maybe less if frames are coming in fast
> and they are queued), regardless of the frame size.

Sorry, I didn't figure it out clearly. As you know the I bit in BDs is just
to let the cpm interrupt the cpu when a buffer not a frame is received.
(yes, in init_fcc_param(), set the rx BDs to
bdp->cbd_sc = BD_ENET_RX_EMPTY | BD_ENET_RX_INTR;
or
bdp->cbd_sc = BD_ENET_RX_EMPTY;
has the same result in the current driver. That's to say we will receive an
interrupt when a "frame" is received. The cpm will issue RXF interrupt
wheneven a frame is received if it's not masked whether you set the I bit or not)

If I want to use the I bit control the interrupt, I can only mask the RXF interrupt,
and unmask the RXB interrupt. When a buffer is full filled with the I bit set in the BD,
a RXB interrupt will occur. But there is a problem, if the buffer is lager enough
to hold an frame (that's the case of the current driver:
 ep->fen_genfcc.fcc_mrblr = PKT_MAXBLR_SIZE; /* 1536 */)
no RXB interrupt is issued since the RXF will be isssed though it is masked by me.
My solution is set the mrblr to a small value(64), and the interrupts will be issued
whenever an buffer is used up by the cpm. The trick is not to set the I bit in all BDs,
but set it one by one or one by more. With this way the interrupt number will be
controled by the I bits in the BDs. Yes, it still has problems, if the last buffer of a
frame has the I bit set in the BD, no RXB interrupt will be issued for the above reason.
The worst case is the I bits are all set only in the last buffers of receiving frames, no
interrupts are issued.

I hope I have explained all now, but maybe my poor Englist won't let me do so:)

>
> The purpose for using first/last is so you can split a frame across multiple
> BDs, making the use of memory a little more efficient.  Using this method,
> you can't dma directly into sk buffers, you will have to check for the
> buffer sequences and copy the data into the sk buffers.  This method also

I do can use dma into small buffers, and then copy to a sk buffer.Since the
current driver copys data to sk buffer too, it won't cost much more time,
though it does incur more bus accesses to the BDs(both the cpu & cpm).


Shenrong

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/