On Wed, Sep 15, 2021 at 3:33 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > drivers/net/hamradio/6pack.c: In function 'sixpack_open': > drivers/net/hamradio/6pack.c:71:41: error: > unsigned conversion from 'int' to 'unsigned char' changes value from '256' to '0' > > patch: > https://lore.kernel.org/lkml/20210909035743.1247042-1-linux@xxxxxxxxxxxx/ > David says it is wrong, and I don't know the code well enough > to feel comfortable touching that code. That may be a lost cause. > "depends on BROKEN if ALPHA" may be appropriate here. David is wrong. The code here is bogus, and the docs clearly state that the transmit data is in units of "10ms": https://www.linux-ax25.org/wiki/6PACK and that #define SIXP_TXDELAY (HZ/4) /* in 1 s */ is just wrong, and the actual *uses* of that TX timeout seems correct for that 10ms value: mod_timer(&sp->tx_t, jiffies + ((when + 1) * HZ) / 100); ie that "when" is clearly given in 100ths of a second, aka 10ms (ok, that's mainly SIXP_SLOTTIME, with SIXP_TXDELAY being used mainly to transfer the data to the other side). So from everything I can see, your patch is correct. Of course, to make things more confusing, the RESYNC_TIMEOUTs are indeed given in ticks. I spent too much time looking at this, but I'm going to apply that patch. I suspect either nobody uses that driver any more, or the TXDELAY values don't actually much matter, since they have clearly been wrong and depended on random kernel configs for a long long time. I think the most common HZ value on x86 tends to be the modern default of 250Hz, so the old "HZ/4" means that most people got a TXDELAY of 620ms, rather than the traditional expected 250ms. The fact that this shows up as an actual compile error on alpha is just random luck, since alpha uses a 1024Hz clock. CONFIG_HZ_1000 isn't impossible on other platforms either, which happens to compile cleanly, but causes that TXDELAY byte to sent out as 250, for a 2.5Hz TX delay. Of course, it is possible that it's the documentation that is wrong, but considering that the documentation matches the code (see above on that "((when + 1) * HZ)/100"), and matches the "it doesn't cause compiler warnings", I think it's pretty clear that your patch is the correct fix. Linus
