All of lore.kernel.org
 help / color / mirror / Atom feed
From: Youngmin Nam <ym0914@gmail.com>
To: Tomasz Figa <tomasz.figa@gmail.com>
Cc: "Krzysztof Kozłowski" <k.kozlowski@samsung.com>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Kukjin Kim" <kgene@kernel.org>,
	"Thomas Abraham" <thomas.ab@samsung.com>,
	"linux-samsung-soc@vger.kernel.org"
	<linux-samsung-soc@vger.kernel.org>
Subject: Re: [PATCH] pinctrl: samsung: Fixes samsung_gpio_direction_in/output releated with spinlock
Date: Sun, 24 Jan 2016 14:23:38 +0900	[thread overview]
Message-ID: <56A45FDA.6020004@gmail.com> (raw)
In-Reply-To: <CA+Ln22FA4iBQs=uh4=q1qEcyy5hN-ujex=T+LQrg_hRFcfhsHg@mail.gmail.com>

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@gmail.com>:
>> 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
> 

  reply	other threads:[~2016-01-24  5:23 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-18 15:21 [PATCH] pinctrl: samsung: Fixes samsung_gpio_direction_in/output releated with spinlock Youngmin Nam
2016-01-22  4:27 ` Tomasz Figa
2016-01-24  5:23   ` Youngmin Nam [this message]
  -- strict thread matches above, loose matches on Subject: below --
2016-01-17  7:41 Youngmin Nam
2016-01-18  3:01 ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56A45FDA.6020004@gmail.com \
    --to=ym0914@gmail.com \
    --cc=k.kozlowski@samsung.com \
    --cc=kgene@kernel.org \
    --cc=linus.walleij@linaro.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=thomas.ab@samsung.com \
    --cc=tomasz.figa@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.