lkml.org 
[lkml]   [2017]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH net-next v2 08/10] net: dsa: lan9303: Added ALR/fdb/mdb handling
From
Date
On 26. juli 2017 19:41, Andrew Lunn wrote:
> Hi Egil
>
>> +/* This function will wait a while until mask & reg == value */
>> +/* Otherwise, return timeout */
>> +static int lan9303_csr_reg_wait(struct lan9303 *chip, int regno,
>> + int mask, char value)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < 0x1000; i++) {
>> + u32 reg;
>> +
>> + lan9303_read_switch_reg(chip, regno, &reg);
>> + if ((reg & mask) == value)
>> + return 0;
>> + }
>> + return -ETIMEDOUT;
>
> Busy looping is probably not a good idea. Can you add a usleep()?
>

Yes

>> +}
>> +
>> +static int _lan9303_alr_make_entry_raw(struct lan9303 *chip, u32 dat0, u32 dat1)
>
> What does the _ indicate. I could understand having it when you have
> lan9303_alr_make_entry_raw() call _lan9303_alr_make_entry_raw() after
> taking a lock, but i don't see anything like that here.
>
Just my sloppy convention for something private, deep down. I can remove
the _.

>> +{
>> + struct lan9303_alr_cache_entry *entr = lan9303_alr_cache_find_mac(
>> + chip, mac);
>
> A long line like this should be split into a declaration and an
> assignment.
>

OK


>
> I would probably make this a separate patch.
>
> Andrew
>

Got it.

\
 
 \ /
  Last update: 2017-07-27 13:05    [W:0.056 / U:19.744 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site