Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH v5 1/2] gpio: add GPO driver for PCA9570
To:     Andy Shevchenko <andy.shevchenko@gmail.com>,
        Bartosz Golaszewski <bgolaszewski@baylibre.com>
Cc:     Linus Walleij <linus.walleij@linaro.org>,
        Michael Walle <michael@walle.cc>,
        LKML <linux-kernel@vger.kernel.org>,
        linux-gpio <linux-gpio@vger.kernel.org>
References: <20200705133038.161547-1-mans0n@gorani.run>
 <CAMpxmJUxGq3_R7BRGv68ApeNC+g9PDm_kBd0r=8TjFSyTNxFWg@mail.gmail.com>
 <CAHp75Vf4440V5Oh1SA5tjVgss134qGkx591ANDY3aQ+oecEzmw@mail.gmail.com>
From:   Sungbo Eo <mans0n@gorani.run>
Message-ID: <15d8ae43-6905-b861-3b50-d1ba780edf2d@gorani.run>
Date:   Tue, 7 Jul 2020 23:03:00 +0900
MIME-Version: 1.0
In-Reply-To: <CAHp75Vf4440V5Oh1SA5tjVgss134qGkx591ANDY3aQ+oecEzmw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 20. 7. 6. 오후 9:00, Andy Shevchenko wrote:
> On Mon, Jul 6, 2020 at 2:21 PM Bartosz Golaszewski
> <bgolaszewski@baylibre.com> wrote:>
>> On Sun, Jul 5, 2020 at 3:31 PM Sungbo Eo <mans0n@gorani.run> wrote:
>>>
>>> NXP PCA9570 is a 4-bit I2C GPO expander without interrupt functionality.
>>> Its ports are controlled only by a data byte without register address.
>>>
> 
>>> Datasheet: https://www.nxp.com/docs/en/data-sheet/PCA9570.pdf
>>>
> 
> No blank line here.
> 
>>> Signed-off-by: Sungbo Eo <mans0n@gorani.run>
> 
>> This driver looks nice now but why did you remove the mutex in v3? I
>> think when Andy commented on that, he meant not understanding why the
>> error check is protected, not the i2c operations.
> 
> Right.

Oh, probably I misunderstood the comment... :(

But I don't really understand what mutex does here. The driver does not 
need consecutive commands, it only sends/receives only one byte at a 
time. And AFAIK each i2c_smbus function is already protected by a mutex. 
So what should be exactly inside the lock? Should we protect the output 
buffer as well? I'm not an expert on this so please enlighten me.

Thanks for your kind reviews, as always. :)

> 
>> Are you sure you don't need this lock?
> 
> It's a good point!
>