Hi Tomasz, Thank you for your review. I will update my patch applying your review and will resend. Best regards, Youngmin On 01/22/2016 01:27 PM, Tomasz Figa wrote: > Hi Youngmin, > > 2016-01-19 0:21 GMT+09:00 Youngmin Nam <ym0914@xxxxxxxxx>: >> Previously, samsung_gpio_drection_in/output function were not covered with >> one spinlock. >> > > Thanks for the patch, nice catch. One nitpick inline, though. > >> For example, samsung_gpio_direction_output function consists of two functions. >> 1. samsung_gpio_set >> 2. samsung_gpio_set_direction > [snip] >> --- a/drivers/pinctrl/samsung/pinctrl-samsung.c >> +++ b/drivers/pinctrl/samsung/pinctrl-samsung.c >> @@ -524,20 +524,26 @@ static void samsung_gpio_set(struct gpio_chip *gc, unsigned offset, int value) > > Can we, for consistency purposes, rename this function to > samsung_gpio_set_value() and keep the one that does locking as > samsung_gpio_set()? > > This way we would match the gpio_chip op name (.set) and also have > both functions with the same name template (set_value and > set_direction) follow the same semantics of not doing any locking. > > Also adding a comment above the new samsung_gpio_set_value() and > existing samsung_gpio_set_direction() that they have to be called with > bank->slock held would be helpful to avoid similar bugs in the future. > > Best regards, > Tomasz > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html