linux-kernel - Re: [RFC PATCH 2/2] hwmon: (f71805f): Use request_region() in f71805f

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <db84d2d3-cb16-7633-afeb-913096c86e8d@huawei.com>
Date:   Thu, 14 Mar 2019 17:21:32 +0000
From:   John Garry <john.garry@...wei.com>
To:     Guenter Roeck <linux@...ck-us.net>
CC:     <jdelvare@...e.com>, <bhelgaas@...gle.com>, <rafael@...nel.org>,
        <arnd@...db.de>, <lorenzo.pieralisi@....com>, <bp@...e.de>,
        <linux-hwmon@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-pci@...r.kernel.org>, <wangkefeng.wang@...wei.com>,
        <linuxarm@...wei.com>
Subject: Re: [RFC PATCH 2/2] hwmon: (f71805f): Use request_region() in
 f71805f_init()

On 14/03/2019 17:05, Guenter Roeck wrote:
> On Fri, Mar 15, 2019 at 12:55:16AM +0800, John Garry wrote:
>> Currently the driver does not call request_region() prior to
>> accessing IO port regions in f71805f_init(), so add it.
>>
>> Signed-off-by: John Garry <john.garry@...wei.com>
>> ---
>>  drivers/hwmon/f71805f.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwmon/f71805f.c b/drivers/hwmon/f71805f.c
>> index 73c681162653..910082c7f184 100644
>> --- a/drivers/hwmon/f71805f.c
>> +++ b/drivers/hwmon/f71805f.c
>> @@ -1617,10 +1617,21 @@ static int __init f71805f_init(void)
>>  	int err;
>>  	unsigned short address;
>>  	struct f71805f_sio_data sio_data;
>> +	struct resource *res;
>> +	size_t size = 0x4e - 0x2e + SIO_REG_ADDR + 4;
>> +
>> +	/* Request the whole 0x2e - 0x4e region */
>> +	res = request_region(0x2e, size, "f71805f");
>

Hi Guenter,

> request_muxed_region() would be a better choice here since it doesn't
> bail out immediately if the region is temporarily busy.

Are you saying that this region could be busy due to another driver 
accessing this same region (probably for the same purpose)?

If so, that would be reasonable.

Thanks,
John

>
> Thanks,
> Guenter
>
>> +	if (!res)
>> +		return -EBUSY;
>>
>>  	if (f71805f_find(0x2e, &address, &sio_data)
>> -	 && f71805f_find(0x4e, &address, &sio_data))
>> +	 && f71805f_find(0x4e, &address, &sio_data)) {
>> +		release_region(0x2e, size);
>>  		return -ENODEV;
>> +	}
>> +
>> +	release_region(0x2e, size);
>>
>>  	err = platform_driver_register(&f71805f_driver);
>>  	if (err)
>> --
>> 2.17.1
>>
>
> .
>