(I did not compile or test it, please let me know, or help fixing
it, if something is wrong with the conversion)
This patch is part of a larger patch series which will remove
the "char bus_id[20]" name string from struct device. The device
name is managed in the kobject anyway, and without any size
limitation, and just needlessly copied into "struct device".
To set and read the device name dev_name(dev) and dev_set_name(dev)
must be used. If your code uses static kobjects, which it shouldn't
do, "const char *init_name" can be used to statically provide the
name the registered device should have. At registration time, the
init_name field is cleared, to enforce the use of dev_name(dev) to
access the device name at a later time.
We need to get rid of all occurrences of bus_id in the entire tree
to be able to enable the new interface. Please apply this patch,
and possibly convert any remaining remaining occurrences of bus_id.
We want to submit a patch to -next, which will remove bus_id from
"struct device", to find the remaining pieces to convert, and finally
switch over to the new api, which will remove the 20 bytes array
and does no longer have a size limitation.
Thanks,
Kay
From: Kay Sievers <[email protected]>
Subject: sparc: struct device - replace bus_id with dev_name(), dev_set_name()
Cc: David S. Miller <[email protected]>
Cc: William L. Irwin <[email protected]>
Cc: [email protected]
Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Kay Sievers <[email protected]>
---
arch/sparc/kernel/of_device.c | 4 ++--
arch/sparc64/kernel/vio.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
--- a/arch/sparc/kernel/of_device.c
+++ b/arch/sparc/kernel/of_device.c
@@ -563,9 +563,9 @@ build_resources:
op->dev.parent = parent;
op->dev.bus = &of_platform_bus_type;
if (!parent)
- strcpy(op->dev.bus_id, "root");
+ dev_set_name(&op->dev, "root");
else
- sprintf(op->dev.bus_id, "%08x", dp->node);
+ dev_set_name(&op->dev, "%08x", dp->node);
if (of_device_register(op)) {
printk("%s: Could not register of device.\n",
--- a/arch/sparc64/kernel/vio.c
+++ b/arch/sparc64/kernel/vio.c
@@ -224,7 +224,7 @@ static struct vio_dev *vio_create_one(st
if (!strcmp(type, "domain-services-port"))
bus_id_name = "ds";
- if (strlen(bus_id_name) >= BUS_ID_SIZE - 4) {
+ if (strlen(bus_id_name) >= 20 - 4) {
printk(KERN_ERR "VIO: bus_id_name [%s] is too long.\n",
bus_id_name);
return NULL;
From: Kay Sievers <[email protected]>
Date: Fri, 07 Nov 2008 01:43:59 +0100
> --- a/arch/sparc/kernel/of_device.c
> +++ b/arch/sparc/kernel/of_device.c
> @@ -563,9 +563,9 @@ build_resources:
> op->dev.parent = parent;
> op->dev.bus = &of_platform_bus_type;
> if (!parent)
> - strcpy(op->dev.bus_id, "root");
> + dev_set_name(&op->dev, "root");
> else
> - sprintf(op->dev.bus_id, "%08x", dp->node);
> + dev_set_name(&op->dev, "%08x", dp->node);
>
> if (of_device_register(op)) {
> printk("%s: Could not register of device.\n",
This part is OK.
> --- a/arch/sparc64/kernel/vio.c
> +++ b/arch/sparc64/kernel/vio.c
> @@ -224,7 +224,7 @@ static struct vio_dev *vio_create_one(st
> if (!strcmp(type, "domain-services-port"))
> bus_id_name = "ds";
>
> - if (strlen(bus_id_name) >= BUS_ID_SIZE - 4) {
> + if (strlen(bus_id_name) >= 20 - 4) {
> printk(KERN_ERR "VIO: bus_id_name [%s] is too long.\n",
> bus_id_name);
> return NULL;
>
>
But I don't like this.
Could you please keep the macro around until everything is converted?
Then you can remove the test entirely.
Leaving it with just constants there is inviting confusion, no matter
how short amount of time it will be there.
On Fri, Nov 7, 2008 at 08:35, David Miller <[email protected]> wrote:
> From: Kay Sievers <[email protected]>
> Date: Fri, 07 Nov 2008 01:43:59 +0100
>
>> --- a/arch/sparc/kernel/of_device.c
>> +++ b/arch/sparc/kernel/of_device.c
>> @@ -563,9 +563,9 @@ build_resources:
>> op->dev.parent = parent;
>> op->dev.bus = &of_platform_bus_type;
>> if (!parent)
>> - strcpy(op->dev.bus_id, "root");
>> + dev_set_name(&op->dev, "root");
>> else
>> - sprintf(op->dev.bus_id, "%08x", dp->node);
>> + dev_set_name(&op->dev, "%08x", dp->node);
>>
>> if (of_device_register(op)) {
>> printk("%s: Could not register of device.\n",
>
> This part is OK.
>
>> --- a/arch/sparc64/kernel/vio.c
>> +++ b/arch/sparc64/kernel/vio.c
>> @@ -224,7 +224,7 @@ static struct vio_dev *vio_create_one(st
>> if (!strcmp(type, "domain-services-port"))
>> bus_id_name = "ds";
>>
>> - if (strlen(bus_id_name) >= BUS_ID_SIZE - 4) {
>> + if (strlen(bus_id_name) >= 20 - 4) {
>> printk(KERN_ERR "VIO: bus_id_name [%s] is too long.\n",
>> bus_id_name);
>> return NULL;
>>
>>
>
> But I don't like this.
>
> Could you please keep the macro around until everything is converted?
> Then you can remove the test entirely.
>
> Leaving it with just constants there is inviting confusion, no matter
> how short amount of time it will be there.
I can add a SPARC_BUS_ID_SIZE, to that file, or in a sparc header, but
the core will not provide any such value, and have to, go to catch all
remaining occurrences across the tree. Where should I add it?
Thanks,
Kay
From: "Kay Sievers" <[email protected]>
Date: Fri, 7 Nov 2008 09:33:16 +0100
> On Fri, Nov 7, 2008 at 08:35, David Miller <[email protected]> wrote:
> > From: Kay Sievers <[email protected]>
> > Date: Fri, 07 Nov 2008 01:43:59 +0100
> >
> >> --- a/arch/sparc64/kernel/vio.c
> >> +++ b/arch/sparc64/kernel/vio.c
> >> @@ -224,7 +224,7 @@ static struct vio_dev *vio_create_one(st
> >> if (!strcmp(type, "domain-services-port"))
> >> bus_id_name = "ds";
> >>
> >> - if (strlen(bus_id_name) >= BUS_ID_SIZE - 4) {
> >> + if (strlen(bus_id_name) >= 20 - 4) {
> >> printk(KERN_ERR "VIO: bus_id_name [%s] is too long.\n",
> >> bus_id_name);
> >> return NULL;
> >>
> >>
> >
> > But I don't like this.
> >
> > Could you please keep the macro around until everything is converted?
> > Then you can remove the test entirely.
> >
> > Leaving it with just constants there is inviting confusion, no matter
> > how short amount of time it will be there.
>
> I can add a SPARC_BUS_ID_SIZE, to that file, or in a sparc header, but
> the core will not provide any such value, and have to, go to catch all
> remaining occurrences across the tree. Where should I add it?
You should keep BUS_ID_SIZE in the device.h header or wherever it is
now.
Then it's a simply grep to kill that off and all the references (and
you have to systematically eliminate these no-longer-needed tests
anyways) in one fell swoop.
Otherwise someone will have to grep for "20" (!!) in order to do that
cleanup.
On Fri, Nov 7, 2008 at 10:38, David Miller <[email protected]> wrote:
> From: "Kay Sievers" <[email protected]>
> Date: Fri, 7 Nov 2008 09:33:16 +0100
>
>> On Fri, Nov 7, 2008 at 08:35, David Miller <[email protected]> wrote:
>> > From: Kay Sievers <[email protected]>
>> > Date: Fri, 07 Nov 2008 01:43:59 +0100
>> >
>> >> --- a/arch/sparc64/kernel/vio.c
>> >> +++ b/arch/sparc64/kernel/vio.c
>> >> @@ -224,7 +224,7 @@ static struct vio_dev *vio_create_one(st
>> >> if (!strcmp(type, "domain-services-port"))
>> >> bus_id_name = "ds";
>> >>
>> >> - if (strlen(bus_id_name) >= BUS_ID_SIZE - 4) {
>> >> + if (strlen(bus_id_name) >= 20 - 4) {
>> >> printk(KERN_ERR "VIO: bus_id_name [%s] is too long.\n",
>> >> bus_id_name);
>> >> return NULL;
>> >>
>> >>
>> >
>> > But I don't like this.
>> >
>> > Could you please keep the macro around until everything is converted?
>> > Then you can remove the test entirely.
>> >
>> > Leaving it with just constants there is inviting confusion, no matter
>> > how short amount of time it will be there.
>>
>> I can add a SPARC_BUS_ID_SIZE, to that file, or in a sparc header, but
>> the core will not provide any such value, and have to, go to catch all
>> remaining occurrences across the tree. Where should I add it?
>
> You should keep BUS_ID_SIZE in the device.h header or wherever it is
> now.
>
> Then it's a simply grep to kill that off and all the references (and
> you have to systematically eliminate these no-longer-needed tests
> anyways) in one fell swoop.
>
> Otherwise someone will have to grep for "20" (!!) in order to do that
> cleanup.
Yeah, but the thing is, that _I_ don't want to remove any checks or
limits from subsystem code. While touching hundreds of files the last
days, there are several instances which could break if I remove any
limits. And most of the stuff I can not even compile.
I can let the BUS_ID_SIZE in the header, but I rather have the people
who understand the code to change the behavior.
Anyway, care to apply the first hunk, and skip the "20" part? I'll
remove the "20" part from the net/ patch too and resend it.
Thanks,
Kay
From: "Kay Sievers" <[email protected]>
Date: Fri, 7 Nov 2008 10:58:09 +0100
> Yeah, but the thing is, that _I_ don't want to remove any checks or
> limits from subsystem code. While touching hundreds of files the last
> days, there are several instances which could break if I remove any
> limits. And most of the stuff I can not even compile.
>
> I can let the BUS_ID_SIZE in the header, but I rather have the people
> who understand the code to change the behavior.
That's right. My main point is that if you don't keep the key
in there to grep for, nobody is likely to make the cleanups.
Nobody is going to grep for "20".
> Anyway, care to apply the first hunk, and skip the "20" part? I'll
> remove the "20" part from the net/ patch too and resend it.
Sure.
On Fri, Nov 7, 2008 at 10:38, David Miller <[email protected]> wrote:
> From: "Kay Sievers" <[email protected]>
>> >> - if (strlen(bus_id_name) >= BUS_ID_SIZE - 4) {
>> >> + if (strlen(bus_id_name) >= 20 - 4) {
>> > But I don't like this.
>> >
>> > Could you please keep the macro around until everything is converted?
>> > Then you can remove the test entirely.
>> >
>> > Leaving it with just constants there is inviting confusion, no matter
>> > how short amount of time it will be there.
>>
>> I can add a SPARC_BUS_ID_SIZE, to that file, or in a sparc header, but
>> the core will not provide any such value, and have to, go to catch all
>> remaining occurrences across the tree. Where should I add it?
>
> You should keep BUS_ID_SIZE in the device.h header or wherever it is
> now.
>
> Then it's a simply grep to kill that off and all the references (and
> you have to systematically eliminate these no-longer-needed tests
> anyways) in one fell swoop.
Hey David,
it took a while, but it happened now, so this issue is coming back. :)
The name size limit is gone, there is no BUS_ID_SIZE thing anymore in
the driver core. BUS_ID_SIZE should go away in the 2.6.30 timeframe.
Could you please convert/remove BUS_ID_SIZE value from:
drivers/net/gianfar.h: char phy_bus_id[BUS_ID_SIZE];
drivers/net/ucc_geth.h: char phy_bus_id[BUS_ID_SIZE];
drivers/net/pasemi_mac.h: char phy_id[BUS_ID_SIZE];
include/linux/phy.h:#define MII_BUS_ID_SIZE (BUS_ID_SIZE - 3)
include/linux/phy.h: char bus_id[BUS_ID_SIZE];
Or let me know how you want to convert it, so I can do it?
Thanks a lot,
Kay
From: Kay Sievers <[email protected]>
Date: Fri, 27 Mar 2009 13:21:56 +0100
> it took a while, but it happened now, so this issue is coming back. :)
>
> The name size limit is gone, there is no BUS_ID_SIZE thing anymore in
> the driver core. BUS_ID_SIZE should go away in the 2.6.30 timeframe.
>
> Could you please convert/remove BUS_ID_SIZE value from:
> drivers/net/gianfar.h: char phy_bus_id[BUS_ID_SIZE];
> drivers/net/ucc_geth.h: char phy_bus_id[BUS_ID_SIZE];
> drivers/net/pasemi_mac.h: char phy_id[BUS_ID_SIZE];
> include/linux/phy.h:#define MII_BUS_ID_SIZE (BUS_ID_SIZE - 3)
> include/linux/phy.h: char bus_id[BUS_ID_SIZE];
>
> Or let me know how you want to convert it, so I can do it?
Thanks a lot Kay, I'll look into these cases and take care
of it.
On Fri, 2009-03-27 at 17:27 -0700, David Miller wrote:
> From: Kay Sievers <[email protected]>
> Date: Fri, 27 Mar 2009 13:21:56 +0100
>
> > it took a while, but it happened now, so this issue is coming back. :)
> >
> > The name size limit is gone, there is no BUS_ID_SIZE thing anymore in
> > the driver core. BUS_ID_SIZE should go away in the 2.6.30 timeframe.
> >
> > Could you please convert/remove BUS_ID_SIZE value from:
> > drivers/net/gianfar.h: char phy_bus_id[BUS_ID_SIZE];
> > drivers/net/ucc_geth.h: char phy_bus_id[BUS_ID_SIZE];
> > drivers/net/pasemi_mac.h: char phy_id[BUS_ID_SIZE];
> > include/linux/phy.h:#define MII_BUS_ID_SIZE (BUS_ID_SIZE - 3)
> > include/linux/phy.h: char bus_id[BUS_ID_SIZE];
> >
> > Or let me know how you want to convert it, so I can do it?
>
> Thanks a lot Kay, I'll look into these cases and take care
> of it.
This moves the 20 into the netdevice header, we need to get rid
of the driver core define now, otherwise people start adding new
code using it, and we want to finish this thankless job finally. :)
Thanks,
Kay
From: Kay Sievers <[email protected]>
Subject: net: remove driver-core BUS_ID_SIZE
The name size limit is gone from the driver-core, the BUS_ID_SIZE
value will be removed.
This moves the BUS_ID_SIZE limit to the netdevice as NET_BUS_ID_SIZE.
The limit can probably removed after review and possible adaption of
the netdevice code.
Cc: [email protected]
Cc: [email protected]
Acked-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: Kay Sievers <[email protected]>
---
drivers/net/arm/ixp4xx_eth.c | 4 ++--
drivers/net/cpmac.c | 2 +-
drivers/net/fec_mpc52xx.c | 2 +-
drivers/net/gianfar.h | 2 +-
drivers/net/pasemi_mac.h | 2 +-
drivers/net/sh_eth.c | 2 +-
drivers/net/ucc_geth.h | 2 +-
include/linux/netdevice.h | 3 +++
include/linux/phy.h | 5 +++--
net/core/net-sysfs.c | 2 +-
net/wireless/nl80211.c | 2 +-
11 files changed, 16 insertions(+), 12 deletions(-)
--- a/drivers/net/arm/ixp4xx_eth.c
+++ b/drivers/net/arm/ixp4xx_eth.c
@@ -1149,7 +1149,7 @@ static int __devinit eth_init_one(struct
struct net_device *dev;
struct eth_plat_info *plat = pdev->dev.platform_data;
u32 regs_phys;
- char phy_id[BUS_ID_SIZE];
+ char phy_id[NET_BUS_ID_SIZE];
int err;
if (!(dev = alloc_etherdev(sizeof(struct port))))
@@ -1212,7 +1212,7 @@ static int __devinit eth_init_one(struct
__raw_writel(DEFAULT_CORE_CNTRL, &port->regs->core_control);
udelay(50);
- snprintf(phy_id, BUS_ID_SIZE, PHY_ID_FMT, "0", plat->phy);
+ snprintf(phy_id, NET_BUS_ID_SIZE, PHY_ID_FMT, "0", plat->phy);
port->phydev = phy_connect(dev, phy_id, &ixp4xx_adjust_link, 0,
PHY_INTERFACE_MODE_MII);
if (IS_ERR(port->phydev)) {
--- a/drivers/net/cpmac.c
+++ b/drivers/net/cpmac.c
@@ -202,7 +202,7 @@ struct cpmac_priv {
void __iomem *regs;
struct mii_bus *mii_bus;
struct phy_device *phy;
- char phy_name[BUS_ID_SIZE];
+ char phy_name[NET_BUS_ID_SIZE];
int oldlink, oldspeed, oldduplex;
u32 msg_enable;
struct net_device *dev;
--- a/drivers/net/fec_mpc52xx.c
+++ b/drivers/net/fec_mpc52xx.c
@@ -215,7 +215,7 @@ static int mpc52xx_fec_init_phy(struct n
{
struct mpc52xx_fec_priv *priv = netdev_priv(dev);
struct phy_device *phydev;
- char phy_id[BUS_ID_SIZE];
+ char phy_id[NET_BUS_ID_SIZE];
snprintf(phy_id, sizeof(phy_id), "%x:%02x",
(unsigned int)dev->base_addr, priv->phy_addr);
--- a/drivers/net/gianfar.h
+++ b/drivers/net/gianfar.h
@@ -779,7 +779,7 @@ struct gfar_private {
spinlock_t bflock;
phy_interface_t interface;
- char phy_bus_id[BUS_ID_SIZE];
+ char phy_bus_id[NET_BUS_ID_SIZE];
u32 device_flags;
unsigned char rx_csum_enable:1,
extended_hash:1,
--- a/drivers/net/pasemi_mac.h
+++ b/drivers/net/pasemi_mac.h
@@ -100,7 +100,7 @@ struct pasemi_mac {
int duplex;
unsigned int msg_enable;
- char phy_id[BUS_ID_SIZE];
+ char phy_id[NET_BUS_ID_SIZE];
};
/* Software status descriptor (ring_info) */
--- a/drivers/net/sh_eth.c
+++ b/drivers/net/sh_eth.c
@@ -804,7 +804,7 @@ static void sh_eth_adjust_link(struct ne
static int sh_eth_phy_init(struct net_device *ndev)
{
struct sh_eth_private *mdp = netdev_priv(ndev);
- char phy_id[BUS_ID_SIZE];
+ char phy_id[NET_BUS_ID_SIZE];
struct phy_device *phydev = NULL;
snprintf(phy_id, sizeof(phy_id), PHY_ID_FMT,
--- a/drivers/net/ucc_geth.h
+++ b/drivers/net/ucc_geth.h
@@ -1100,7 +1100,7 @@ struct ucc_geth_info {
u32 eventRegMask;
u16 pausePeriod;
u16 extensionField;
- char phy_bus_id[BUS_ID_SIZE];
+ char phy_bus_id[NET_BUS_ID_SIZE];
u8 weightfactor[NUM_TX_QUEUES];
u8 interruptcoalescingmaxvalue[NUM_RX_QUEUES];
u8 l2qt[UCC_GETH_VLAN_PRIORITY_MAX];
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -93,6 +93,9 @@ struct wireless_dev;
#ifdef __KERNEL__
+/* old driver core device name limit */
+#define NET_BUS_ID_SIZE 20
+
/*
* Compute the worst case header length according to the protocols
* used.
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -20,6 +20,7 @@
#include <linux/spinlock.h>
#include <linux/device.h>
+#include <linux/netdevice.h>
#include <linux/ethtool.h>
#include <linux/mii.h>
#include <linux/timer.h>
@@ -79,7 +80,7 @@ typedef enum {
* Need to be a little smaller than phydev->dev.bus_id to leave room
* for the ":%02x"
*/
-#define MII_BUS_ID_SIZE (BUS_ID_SIZE - 3)
+#define MII_BUS_ID_SIZE (NET_BUS_ID_SIZE - 3)
/*
* The Bus class for PHYs. Devices which provide access to
@@ -407,7 +408,7 @@ struct phy_driver {
/* A Structure for boards to register fixups with the PHY Lib */
struct phy_fixup {
struct list_head list;
- char bus_id[BUS_ID_SIZE];
+ char bus_id[NET_BUS_ID_SIZE];
u32 phy_uid;
u32 phy_uid_mask;
int (*run)(struct phy_device *phydev);
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -497,7 +497,7 @@ int netdev_register_kobject(struct net_d
dev->platform_data = net;
dev->groups = groups;
- BUILD_BUG_ON(BUS_ID_SIZE < IFNAMSIZ);
+ BUILD_BUG_ON(NET_BUS_ID_SIZE < IFNAMSIZ);
dev_set_name(dev, "%s", net->name);
#ifdef CONFIG_SYSFS
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -57,7 +57,7 @@ static int get_drv_dev_by_info_ifindex(s
static struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] __read_mostly = {
[NL80211_ATTR_WIPHY] = { .type = NLA_U32 },
[NL80211_ATTR_WIPHY_NAME] = { .type = NLA_NUL_STRING,
- .len = BUS_ID_SIZE-1 },
+ .len = NET_BUS_ID_SIZE-1 },
[NL80211_ATTR_WIPHY_TXQ_PARAMS] = { .type = NLA_NESTED },
[NL80211_ATTR_WIPHY_FREQ] = { .type = NLA_U32 },
[NL80211_ATTR_WIPHY_CHANNEL_TYPE] = { .type = NLA_U32 },
From: Kay Sievers <[email protected]>
Date: Thu, 16 Apr 2009 18:21:52 +0200
> On Fri, 2009-03-27 at 17:27 -0700, David Miller wrote:
>> From: Kay Sievers <[email protected]>
>> Date: Fri, 27 Mar 2009 13:21:56 +0100
>>
>> > it took a while, but it happened now, so this issue is coming back. :)
>> >
>> > The name size limit is gone, there is no BUS_ID_SIZE thing anymore in
>> > the driver core. BUS_ID_SIZE should go away in the 2.6.30 timeframe.
>> >
>> > Could you please convert/remove BUS_ID_SIZE value from:
>> > drivers/net/gianfar.h: char phy_bus_id[BUS_ID_SIZE];
>> > drivers/net/ucc_geth.h: char phy_bus_id[BUS_ID_SIZE];
>> > drivers/net/pasemi_mac.h: char phy_id[BUS_ID_SIZE];
>> > include/linux/phy.h:#define MII_BUS_ID_SIZE (BUS_ID_SIZE - 3)
>> > include/linux/phy.h: char bus_id[BUS_ID_SIZE];
>> >
>> > Or let me know how you want to convert it, so I can do it?
>>
>> Thanks a lot Kay, I'll look into these cases and take care
>> of it.
>
> This moves the 20 into the netdevice header, we need to get rid
> of the driver core define now, otherwise people start adding new
> code using it, and we want to finish this thankless job finally. :)
I was working on fixing this properly Kay, please be patient.
I'll have it killed off in net-next-2.6 soon enough.