Sender: Guenter Roeck <groeck7@gmail.com>
Subject: Re: [PATCH 2/2] hwmon: (dell-smm) Unify i8k_ioctl() and
 i8k_ioctl_unlocked()
To:     =?UTF-8?Q?Pali_Roh=c3=a1r?= <pali@kernel.org>,
        Armin Wolf <W_Armin@gmx.de>
Cc:     jdelvare@suse.com, linux-hwmon@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20211120170319.72369-1-W_Armin@gmx.de>
 <20211120170319.72369-3-W_Armin@gmx.de>
 <20211122160122.gf6i3qj6dnwi6wla@pali>
From:   Guenter Roeck <linux@roeck-us.net>
Message-ID: <c23caeab-dd27-4c95-2e25-9eb0ff7b33f6@roeck-us.net>
Date:   Mon, 22 Nov 2021 09:55:29 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <20211122160122.gf6i3qj6dnwi6wla@pali>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 11/22/21 8:01 AM, Pali Rohár wrote:
> On Saturday 20 November 2021 18:03:19 Armin Wolf wrote:
>> The only purpose of i8k_ioctl() is to call i8k_ioctl_unlocked()
>> with i8k_mutex held. Judging from the hwmon code, this mutex
>> only needs to be held when setting the fan speed/mode.
> 
> Really? I think that there is no difference between setting and getting
> fan speed/mode. At least I do not see why 'set' needs mutex and 'get' do
> not need it. Some more explanation is needed...
> 
I8K_SET_FAN sets the fan speed and returns the current status. Without
locking, the returned status may not match or be associated with the previous
set operation.

Maybe that doesn't matter, and the synchronization is not needed. If so,
you can probably remove the locking entirely.

Guenter