Hi Ben,
thank for your review. Find my inline comments.
Thanks and best regards,
luigi
On Mon, Oct 5, 2009 at 8:27 AM, Ben Warren biggerbadderben@gmail.com wrote:
Hi Luigi,
Luigi 'Comio' Mantellini wrote:
From: Luigi 'Comio' Mantellini luigi.mantellini@idf-hit.com
Please add some descriptive information here. This is a big patch and deserves a changelong
+#define BBMII_RELOATE(v,off) (v += (v?off:0))
s/RELOATE/RELOCATE/
Please apply globally
Ok.
+struct bbmiibus bbmiibusses[] = {
Elsewhere, you name things 'bb_mii', but here it's 'bbmii'. I personally prefer adding the underscores, but please be consistent.
What do you mean? change bbmiibus -> bb_mii_bus and bbmiibuses -> bb_mii_buses?
- {
- .name = BB_MII_DEVNAME,
BB_MII_DEVNAME will be "bb_mii". is it ok?
- .init = bb_mii_init_wrap,
- .mdio_active = bb_mdio_active_wrap,
- .mdio_tristate = bb_mdio_tristate_wrap,
- .set_mdio = bb_set_mdio_wrap,
- .get_mdio = bb_get_mdio_wrap,
- .set_mdc = bb_set_mdc_wrap,
- .delay = bb_delay_wrap,
- }
+};
+#if !defined(CONFIG_RELOC_FIXUP_WORKS)
- /* Reloate the hooks pointers*/
s/Reloate/Relocate/ s/hooks/hook/
ok
*/ -static void miiphy_pre (char read, unsigned char addr, unsigned char reg) +static void miiphy_pre (struct bbmiibus *bus, char read, unsigned char addr, unsigned char reg)
This line's too long (please keep < 78 characters). Apply globally
ok
*/
- MDC (0);
- MDIO (0);
- MIIDELAY;
...
- bus->set_mdc (bus, 0);
- bus->set_mdio (bus, 0);
- bus->delay(bus);
Please be consistent with whitespace. I prefer no space before the opening paren, although I think Wolfgang likes that. He's funny that way. Note that this only relates to function calls. Control logic (if, for, while etc.) should always have a space before the opening paren.
I just replaced MD{C,IO} with bus->set_md{c,io}. Ok.
- bus = bb_miiphy_getbus(devname);
- if (bus == NULL) {
- /* Bus not found! */
Comment's kinda superfluous...
I know that the source code is always sufficiently clear regard what it does. :) ok
Thanks for this submission. It's a really big improvement.
regards, Ben
I use GPL code on my devices... It obvious that my improvements will be shred with community.
thanks again and best regards,
luigi