Re: [PATCH] west bridge, device modules fixes

From: Randy Dunlap
Date: Tue Sep 07 2010 - 16:48:15 EST


On Tue, 07 Sep 2010 12:40:14 -0700 David Cross wrote:

> This patch makes minor fixes to the device module for locking issues,
> as well as some changes for the HAL/Kconfig changes made in a separate
> patch. Please let me know if there are issues or concerns with this
> patch.
> Thanks,
> David
>
> Signed-off-by: David Cross <david.cross@xxxxxxxxxxx>
>
> diff -uprN -X linux-next-vanilla/Documentation/dontdiff linux-next-vanilla/drivers/staging/westbridge/astoria/device/cyasdevice.c linux-next-incl-sdk/drivers/staging/westbridge/astoria/device/cyasdevice.c
> --- linux-next-vanilla/drivers/staging/westbridge/astoria/device/cyasdevice.c 2010-08-31 19:32:51.000000000 -0700
> +++ linux-next-incl-sdk/drivers/staging/westbridge/astoria/device/cyasdevice.c 2010-09-07 11:32:36.000000000 -0700
> @@ -48,6 +48,8 @@ typedef struct cyasdevice {
> cy_as_device_handle dev_handle;
> /* Handle to the HAL */
> cy_as_hal_device_tag hal_tag;
> + spinlock_t common_lock;
> + unsigned long flags;
> } cyasdevice ;

No space before the ';'. (in many many places & in other patches)


>
> /* global ptr to astoria device */
> @@ -81,13 +83,19 @@ static void cyasdevice_deinit(cyasdevice
>
> if (cy_as_dev->hal_tag) {
>
> - #ifdef CONFIG_MACH_OMAP3_WESTBRIDGE_AST_PNAND_HAL
> - if (stop_o_m_a_p_kernel(dev_handle_name,
> + #ifdef CONFIG_MACH_OMAP3_WESTBRIDGE_AST_PNAND_HAL

It's better _not_ to indent the #ifdef and #endif lines.
Indenting them obscures them, and that's not good.


> + if (cy_as_hal_omap_pnand_stop(dev_handle_name,
> cy_as_dev->hal_tag) != 0)
> cy_as_hal_print_message("<1>_cy_as_device: stopping "
> - "OMAP kernel HAL failed\n");
> + "OMAP PNAND HAL failed\n");
> + #endif
>
> - #endif
> + #ifdef CONFIG_MACH_OMAP3_WESTBRIDGE_AST_CRAM_HAL
> + if (cy_as_hal_omap_cram_stop(dev_handle_name,
> + cy_as_dev->hal_tag) != 0)
> + cy_as_hal_print_message("<1>_cy_as_device: stopping "
> + "OMAP CRAM HAL failed\n");
> + #endif
> }
> cy_as_hal_print_message("<1>_cy_as_device:HAL layer stopped\n") ;
>
> @@ -126,6 +134,28 @@ static void cy_misc_callback(cy_as_devic
> }
> }
>
> +void cy_as_acquire_common_lock()
> +{
> + /*printk("%s: lock address is 0x%x, is_locked=0x%x\n", __func__,
> + &cy_as_device_controller->common_lock,
> + spin_is_locked(&cy_as_device_controller->common_lock));*/
> + spin_lock_irqsave(&cy_as_device_controller->common_lock,
> + cy_as_device_controller->flags);/*
> + printk("%s: -- is_locked=0x%x\n", __func__,
> + spin_is_locked(&cy_as_device_controller->common_lock));*/

The printk line should not be indented.

> +}
> +EXPORT_SYMBOL(cy_as_acquire_common_lock);
> +
> +void cy_as_release_common_lock()
> +{
> + /*printk("%s: lock address is 0x%x, is_locked=0x%x\n", __func__,
> + &cy_as_device_controller->common_lock,
> + spin_is_locked(&cy_as_device_controller->common_lock));*/
> + spin_unlock_irqrestore(&cy_as_device_controller->common_lock,
> + cy_as_device_controller->flags);

The line above should be indented more than the line above it.

> +}
> +EXPORT_SYMBOL(cy_as_release_common_lock);
> +
> /* reset astoria and reinit all regs */
> #define PNAND_REG_CFG_INIT_VAL 0x0000
> void hal_reset(cy_as_hal_device_tag tag)



---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
--
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/