Re: [PATCH v4 1/3] Staging: IndustryPack bus for the Linux Kernel

[Greg Kroah-Hartman]

On Thu, May 10, 2012 at 05:34:54PM +0100, Samuel Iglesias Gonsálvez wrote:
> >>> > +struct ipack_device {
> >>> > +   char board_name[IPACK_BOARD_NAME_SIZE];
> >>>
> >>> Why not use dev->name?
> >>
> >>May I be wrong, do you refer rename it to "name"?
> >
> >rename what?  Why do you need a board name for a device?
> >Shouldn't that
> >just be the "name" for the device?  And as such, just use the
> >field you
> >already have.
> >
> 
> In struct device there is the field "init_name". There is a "name"
> field in the corresponding struct kobject inside of dev. This is the
> reason of my misunderstanding.
> 
> I will change it.

Use the function call to set the name, not the field directly, otherwise
bad things could happen.

> >>> > +   char bus_name[IPACK_BOARD_NAME_SIZE];
> >
> >And, why do you need a bus name?  Shouldn't that be implied based on
> >what bus the device is attached to?
> >
> 
> This is the name of the bus device. The problem here is that the
> ipoctal mezzanine needs to save the IRQ vector in his memory space
> in a different address depending of the carrier board it is plugged
> to.
> 
> It is described in IP-OCTAL's datasheet. So this bus_name variable
> gives the way to do it.

But why is this a string?  I don't understand why this needs to be in
the device, shouldn't it just be determined on which bus the device was
assigned to?  Shouldn't this be part of the register function?  Pass in
the bus it should be registered to and all should be good.

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