lkml.org 
[lkml]   [2016]   [Apr]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH V5 1/6] SLIMbus: Device management on SLIMbus
Date
On Wednesday 27 April 2016 17:58:04 Sagar Dharia wrote:

> +/**
> + * slim_driver_register: Client driver registration with slimbus
> + * @drv:Client driver to be associated with client-device.
> + * This API will register the client driver with the slimbus
> + * It is called from the driver's module-init function.
> + */
> +int slim_driver_register(struct slim_driver *drv)
> +{
> + drv->driver.bus = &slimbus_type;
> +
> + return driver_register(&drv->driver);
> +}
> +EXPORT_SYMBOL_GPL(slim_driver_register);

Please make this use the same trick as platform_driver_register() to implicitly
set the .owner field of the driver to THIS_MODULE.

> +/**
> + * slim_add_device: Add a new device without register board info.
> + * @ctrl: Controller to which this device is to be added to.
> + * Called when device doesn't have an explicit client-driver to be probed, or
> + * the client-driver is a module installed dynamically.
> + */
> +int slim_add_device(struct slim_controller *ctrl, struct slim_device *sbdev)

This looks like an artifact of ancient pre-DT times. I'd say kill it off before
someone starts using it.

> +struct sbi_boardinfo {
> + struct list_head list;
> + struct slim_boardinfo board_info;
> +};
> +
> +static LIST_HEAD(board_list);
> +static LIST_HEAD(slim_ctrl_list);
> +static DEFINE_MUTEX(board_lock);
> +

> +/**
> + * slim_register_board_info: Board-initialization routine.
> + * @info: List of all devices on all controllers present on the board.
> + * @n: number of entries.
> + * API enumerates respective devices on corresponding controller.
> + * Called from board-init function.
> + */
> +int slim_register_board_info(struct slim_boardinfo const *info, unsigned n)
> +{

Same for all of this.

> +struct slim_device_id {
> + __u16 manf_id, prod_code;
> + __u8 dev_index, instance;
> +
> + /* Data private to the driver */
> + kernel_ulong_t driver_data;
> +};

Can you add explicit padding here to avoid having the anonymous two bytes in the middle?

Also, I think a void pointer instead of a kernel_ulong_t makes it more useful,
but there are always cases where one or the other fits better.

Arnd

\
 
 \ /
  Last update: 2016-04-28 12:21    [W:0.187 / U:7.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site