netdev - Re: [PATCH] phylib: Add autoload support for the LXT973 phy.

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <20100626.221620.267962098.davem@davemloft.net>
Date:	Sat, 26 Jun 2010 22:16:20 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	dwmw2@...radead.org
Cc:	richardcochran@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH] phylib: Add autoload support for the LXT973 phy.

From: David Woodhouse <dwmw2@...radead.org>
Date: Tue, 22 Jun 2010 13:38:13 +0100

> Commit e13647c1 (phylib: Add support for the LXT973 phy.) added a new ID
> but neglected to also add it to the MODULE_DEVICE_TABLE.
> 
> Signed-off-by: David Woodhouse <David.Woodhouse@...el.com>

Applied, thanks.

> When I did this stuff, I did wonder if we should make this happen
> automatically somehow. I pondered some dirty macro hack in
> phy_driver_register() which would do it somehow, but couldn't come up
> with anything that'd work.
> 
> Removing the phy_id and phy_id_mask from struct phy_driver and having a
> pointer to a match table would suck, since each driver only really
> matches one device/mask. (Even where a single C file has multiple
> drivers, they often differ in some methods or flags.)
> 
> The best option I can come up with right now, is probably to remove
> phy_id and phy_id_mask from phy_driver and put a pointer to the driver
> into the ID table, and take the ID table as the argument to
> phy_driver_register(). I'm not sure I like that very much though -- I'd
> prefer that we just remember to update the table and don't need to be
> forced :)
> 
> (Another cheap option is to pass the ID table as an extra argument to
> the existing phy_device_register(), I suppose, and it can just print a
> warning if it doesn't find the same phy_id and phy_id_mask in the table)

As our experience shows, people aren't remembering to do it so we have
to do something hard handed to make sure this doesn't break.

A compile time error out is the best, but if that is too hard or ugly
and we do it at run time then we should fail the register (not just
print a warning) if the table is incomplete.

Otherwise we run into cases where a developer adds several new IDs,
forgets some of the table entries, but only tries testing the ones he
did remember to add and doesn't notice the warning message.
--
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