Question about StellarisWare, SW-RDK-S2E, stellarisif.c

And I hope to fix it with ... __packed

IIRC, the Cortex-M3 CPU in the Luminary MCU's supports unaligned accesses. That is, no need to use __packed, it will only lead to less efficient code.

According to the standards, doing this causes undefined behaviour, it is not permitted and cannot be done.

Unless the code is only run on a very specific CPU that supports this, in which case this is OK.

And I hope to fix it with ... __packed

IIRC, the Cortex-M3 CPU in the Luminary MCU's supports unaligned accesses. That is, no need to use __packed, it will only lead to less efficient code.

According to the standards, doing this causes undefined behaviour, it is not permitted and cannot be done.

Unless the code is only run on a very specific CPU that supports this, in which case this is OK.

Many Thanks to All.

The mentioned code:

\third_party\lwip-1.3.2\ports\stellaris\netif\stellarisif.c

/**
 * This function should do the actual transmission of the packet. The packet is
 * contained in the pbuf that is passed to the function. This pbuf might be
 * chained.
 *
 * @param netif the lwip network interface structure for this ethernetif
 * @param p the MAC packet to send (e.g. IP packet including MAC addresses and type)
 * @return ERR_OK if the packet could be sent
 *         an err_t value if the packet couldn't be sent
 * @note This function MUST be called with interrupts disabled or with the
 *       Stellaris Ethernet transmit fifo protected.
 */
static err_t
stellarisif_transmit(struct netif *netif, struct pbuf *p)
{
  int iBuf;
  unsigned char *pucBuf;
  unsigned long *pulBuf;
  struct pbuf *q;
  int iGather;
  unsigned long ulGather;
  unsigned char *pucGather;

  /**
   * Fill in the first two bytes of the payload data (configured as padding
   * with ETH_PAD_SIZE = 2) with the total length of the payload data
   * (minus the Ethernet MAC layer header).
   *
   */
  *((unsigned short *)(p->payload)) = p->tot_len - 16;

  /* Initialize the gather register. */
  iGather = 0;
  pucGather = (unsigned char *)&ulGather;
  ulGather = 0;

  /* Copy data from the pbuf(s) into the TX Fifo. */
  for(q = p; q != NULL; q = q->next) {
    /* Intialize a char pointer and index to the pbuf payload data. */
    pucBuf = (unsigned char *)q->payload;
    iBuf = 0;

    /**
     * If the gather buffer has leftover data from a previous pbuf
     * in the chain, fill it up and write it to the Tx FIFO.
     *
     */
    while((iBuf < q->len) && (iGather != 0)) {
      /* Copy a byte from the pbuf into the gather buffer. */
      pucGather[iGather] = pucBuf[iBuf++];

      /* Increment the gather buffer index modulo 4. */
      iGather = ((iGather + 1) % 4);
    }

    /**
     * If the gather index is 0 and the pbuf index is non-zero,
     * we have a gather buffer to write into the Tx FIFO.
     *
     */
    if((iGather == 0) && (iBuf != 0)) {
      HWREG(ETH_BASE + MAC_O_DATA) = ulGather;
      ulGather = 0;
    }

    /* Initialze a long pointer into the pbuf for 32-bit access. */
    pulBuf = (unsigned long *)&pucBuf[iBuf];

    /**
     * Copy words of pbuf data into the Tx FIFO, but don't go past
     * the end of the pbuf.
     *
     */
    while((iBuf + 4) <= q->len) {
      HWREG(ETH_BASE + MAC_O_DATA) = *pulBuf++;
      iBuf += 4;
    }
[Cut]

\third_party\lwip-1.3.2\src\include\lwip\pbuf.h

struct pbuf {
  /** next pbuf in singly linked pbuf chain */
  struct pbuf *next;

  /** pointer to the actual data in the buffer */
  void *payload;

  /**
   * total length of this buffer and all next buffers in chain
   * belonging to the same packet.
   *
   * For non-queue packet chains this is the invariant:
   * p->tot_len == p->len + (p->next? p->next->tot_len: 0)
   */
  u16_t tot_len;

  /** length of this buffer */
  u16_t len;

  /** pbuf_type as u8_t instead of enum to save space */
  u8_t /*pbuf_type*/ type;

  /** misc flags */
  u8_t flags;

  /**
   * the reference count always equals the number of pointers
   * that refer to this pbuf. This can be pointers from an application,
   * the stack itself, or pbuf->next pointers from a chain.
   */
  u16_t ref;

#if LWIP_PTPD
  /* the time at which the packet was received, seconds component */
  u32_t time_s;

  /* the time at which the packet was received, nanoseconds component */
  u32_t time_ns;
#endif /* #if LWIP_PTPD */
};

Maybe I need to further check the pbuf,

stellarisif_transmit(struct netif *netif, struct pbuf *p)

but it doesn't look like the address is always correctly aligned.

Yes, the LMI MCU has the unaligned data access feature, enables data to be efficiently packed into memory. But I am not sure if I properly enabled this feature, cause I created a completely new KEIL project to include the necessary software components only.

What you need to know, is where payload comes from. Maybe the payload is guaranteed to always be perfectly aligned.

That while loop with iGather seems to indicate that they do try to keep track of data moduly 4 bytes, so it's quite likely that they make sure that the data is 4-byte-aligned.

Yes - that has been confirmed by TI on the TI forum.

I say they should have made this abundantly clear in the function comments, though.

Some people are of the oppinion that source code should not contain any comments. That comments are only needed when people have selected bad symbol names. Hopefully, they only have a view, but don't spend time actually writing code.

Good code allows the reader to understand what happens just from looking at the code without need for comments. But it's really important that source code comments exists that does describe reasons for the actions. It doesn't matter if I can see code and understand that the code is sorting something, if it isn't obvious why that something needs to be sorted.

But an even more important thing is that any assumptions should always be very clearly documented. And the code should, if possible, try to contain (at least for debug versions) code to catch invalidated assumptions.

A problem is of course that we make a lot of assumptions without realizing that we are. But assuming that buffers have specific alignments really are assumptions that the code creator must have been well aware of making. And it should have been just as obvious that such an assumption has to be clearly documented. All functions that does rely on this should basically prove their right to do so.

Dummy post just to correct the extra characters I managed to get in my name. Some times when I try to answer a thread, the web browser moves the focus to the First Name field after I have already clicked the Message box and started to print characters. I haven't checked if there is any javascript OnLoad function that gives this result, but it has happend quite a number of times :(

The latest response from TI/Luminary:

e2e.ti.com/.../261784.aspx

John,

Cortex-M3 will handle some unaligned data accesses, but not all. If you do a "ldr" or "str" to load or store a value, unaligned addresses are handled seamlessly (unless you explicitly disable unaligned accesses, which is a control bit in the NVIC CFG_CTRL register). However, if you do a "ldm" or "stm" to load or store multiple values (where "multiple" is defined as 1 or more), unaligned addresses are never allowed.

In the Keil compiler, there are certain situations where "*pulPtr++" is coded as a "ldr", and others where it is coded as a "ldm". This seems to be a recent change in their compiler, as we have seen this problem crop up with some of our applications due to this exact code construct (though we have not seen any issues in lwIP). I do not know what the heuristic is to determine when they will and when they won't use a "ldm". However, it appears as though the "__packed" qualifier will force it to use a "ldr", which is compatible with unaligned addresses. This explains why the use of "__packed" makes your problems go away.

Avoiding unaligned pointers will also solve the problem. Note that this is a problem that is specific to Keil; this code is compiled to "ldr" instructions by all the other supported tool chains.

--Brian

This afternoon, right after I posted some more information on the TI/Luminary forum, and it was almost time to go home, my supervisor told me that, I will be laid off before the end of December, maybe earlier, before the end of November.

The company I am working for, have been suffering persistent losses for quite a long time, and our main products are all classic 8051 based. As the only ARM software engineer in the company, such a lay off did not surprise me.

But I do hope that, I can have more time to finish all my three projects, then ask them to freeze all the source code and binary. I just don't want someone fiddles my projects, then adds some terrible bugs.

I understand that, it is just a hope, it is never possible.

Very sorry to hear that.

All the best for the future...

Sorry to hear.

Hope you'll find some other job quickly.

Maybe your company should have taken a closer look at their options - maybe some of their losses are caused by they staying with 8051 chips when competitors moves to ARM chips. The ARM chips tends to give a bigger flexibility, and have better peripherials. So way easier to react to a customer requirement late in a product life cycle.

About the align problem - funny guys. Writing C code where they ignore the C standard and don't make sure themselves that their casts are aligned. And then blame the compiler for their oopses. I think they may have a developer or two who should consider a different way of earning his/her daily food.

A similar though occurred to me: they have a loss-making, lacklustre 8051-based product range - so they've fired the guy who could give them the shiny new product to rescue the company...

John,

I'm very sorry to hear that. Per and Andy made a very good point - the low performance processors might play a role in this affair - but I'm afraid that now it is too late - not only because you were fired, but mainly because the transition will be costly in terms of man hours and hardware design and compliance tests etc...My employer made that very same transition (there are still some 8051 products solved, of course, but the future belongs to ARM7 and in particular, CM3/LPC1788 chips) about 2 years ago - they hired me and some others and we just nailed it. Either way, I believe I speak for all of us by wishing you all the best in the future.

Many thanks to all. (My English ability is very limited, not able to discuss something non-technique properly and fluently.)

==> About the align problem - funny guys. Writing C code where they ignore the C standard and don't make sure themselves that their casts are aligned. And then blame the compiler for their oopses. I think they may have a developer or two who should consider a different way of earning his/her daily food.

I was confused by the latest response from TI/Luminary. I thought it was due to my lack of programming knowledge.

But now, I think my confusion is quite reasonable. But I am not sure if I have enough knowledge to deal with TI/Luminary and this issue, and I am running out of time.

Hi, good morning, I want to go out with my wife and kids for breakfast now. I chose this job, because it gives me an opportunity to accompany my family in southern Taiwan.

My supervisor told me that, the lay off is actually a 20% - 40% employee downsizing. So I am not the only one involved.

It seems that, this downsizing was originally scheduled in next year Q1 or Q2, but brought forward to the end of December, due to the USA Fed is launching its second round of quantitative easing, which leads to sharp appreciation of the Taiwan Dollar exchange rate.

It seems that, Euro/Great Britain Pound/Swedish Krona are relatively stable? Many countries in Non-Japan Asia will be seriously impacted.

As Tamir pointed,

==> now it is too late - not only because you were fired, but mainly because the transition will be costly in terms of man hours and hardware design and compliance tests etc...

And the marketing research will also be needed, if we want to do it right.

Maybe my software development skill is better than the "average" level of our "local" competitors, but one software engineer is not able to do much under such a tough condition.

Many local small companies here in Taiwan, were just suddenly shutdown; and the owner of company just magically disappeared, left the angry employees and suppliers. Even the famous TSMC, in 2008, they forced their employees to resign, so that they did not need to pay the severance fee. And then, in 2009, when they needed the experienced employees, they said it was a mistake/misunderstanding, they were very sorry, they wanted to re-employ all the laid off employees. So, I believe that, the owner of the company which I am working for, is a good man; he chose to "try to" officially lay off the employees. Though he made a lot of wrong business decisions.

I don't like northern Taiwan, but I may have to work there once again, since there are more jobs.