netdev - Re: Silicom bypass driver promote from staging

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140807215115.GB19520@electric-eye.fr.zoreil.com>
Date:	Thu, 7 Aug 2014 23:51:15 +0200
From:	Francois Romieu <romieu@...zoreil.com>
To:	David Hendel <david@...icom.co.il>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"arnd@...db.de" <arnd@...db.de>, Anna Lukin <annal@...icom.co.il>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
Subject: Re: Silicom bypass driver promote from staging

David Hendel <david@...icom.co.il> :
[...]
> Let me know how we can proceed with that. what should be the next step.

Location is the easy part. Getting things reviewed is a different story.

You shouldn't expect reviewers to swallow 300k of code (600k / 2 due to
removal). Please split the submitted patch into smaller, self-consistent,
logical ones.

Notwithstanding Stephen and Jeff's remarks, you should also:
- remove the (out-)commented code 
- reconsider the use of gorilla class macros vs plain functions
- stop casting ioremap return into long, then later into void *. It's
  void __iomem * and should stay so.
- use a consistent locking style and remove BP_SYNC_FLAG
- fix the 80 cols limit problem(s) (hardly surprizing after 5 levels of
  nested "if")
- avoid redefining stuff from include/uapi/linux/mii.h
  I may be wrong but BPCTLI_MII_CR_POWER_DOWN smells of BMCR_PDOWN and
  BPCTLI_PHY_CONTROL, well, MII_BMCR ?
- avoid returning with spinlock held (read_reg, wdt_pulse)
- start explaining what did change between two submissions

-- 
Ueimor
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html