sparc: struct device - replace bus_id with dev_name(), dev_set_name() - Kernel

This is a discussion on sparc: struct device - replace bus_id with dev_name(), dev_set_name() - Kernel ; (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 ...

+ Reply to Thread
Results 1 to 6 of 6

Thread: sparc: struct device - replace bus_id with dev_name(), dev_set_name()

  1. sparc: struct device - replace bus_id with dev_name(), dev_set_name()

    (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
    Subject: sparc: struct device - replace bus_id with dev_name(), dev_set_name()

    Cc: David S. Miller
    Cc: William L. Irwin
    Cc: sparclinux@vger.kernel.org
    Acked-by: Greg Kroah-Hartman
    Signed-off-by: Kay Sievers
    ---
    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;


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: sparc: struct device - replace bus_id with dev_name(), dev_set_name()

    From: Kay Sievers
    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.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: sparc: struct device - replace bus_id with dev_name(), dev_set_name()

    On Fri, Nov 7, 2008 at 08:35, David Miller wrote:
    > From: Kay Sievers
    > 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: sparc: struct device - replace bus_id with dev_name(), dev_set_name()

    From: "Kay Sievers"
    Date: Fri, 7 Nov 2008 09:33:16 +0100

    > On Fri, Nov 7, 2008 at 08:35, David Miller wrote:
    > > From: Kay Sievers
    > > 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.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: sparc: struct device - replace bus_id with dev_name(), dev_set_name()

    On Fri, Nov 7, 2008 at 10:38, David Miller wrote:
    > From: "Kay Sievers"
    > Date: Fri, 7 Nov 2008 09:33:16 +0100
    >
    >> On Fri, Nov 7, 2008 at 08:35, David Miller wrote:
    >> > From: Kay Sievers
    >> > 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: sparc: struct device - replace bus_id with dev_name(), dev_set_name()

    From: "Kay Sievers"
    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.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread