linux-kernel - Re: [PATCH 1/5] iTCO_wdt: Expose watchdog properties using platform data

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: <55B8F6D7.6030005@roeck-us.net>
Date:	Wed, 29 Jul 2015 08:52:55 -0700
From:	Guenter Roeck <linux@...ck-us.net>
To:	Lee Jones <lee.jones@...aro.org>,
	Aaron Sierra <asierra@...-inc.com>
CC:	Matt Fleming <matt@...eblueprint.co.uk>,
	Wim Van Sebroeck <wim@...ana.be>,
	linux-kernel@...r.kernel.org, linux-watchdog@...r.kernel.org,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Jean Delvare <jdelvare@...e.com>,
	Wolfram Sang <wsa@...-dreams.de>,
	Matt Fleming <matt.fleming@...el.com>,
	Peter Tyser <ptyser@...-inc.com>,
	Samuel Ortiz <sameo@...ux.intel.com>
Subject: Re: [PATCH 1/5] iTCO_wdt: Expose watchdog properties using platform
 data

On 07/29/2015 08:32 AM, Lee Jones wrote:
> On Wed, 29 Jul 2015, Aaron Sierra wrote:
>
>>> From: "Lee Jones" <lee.jones@...aro.org>
>>> Sent: Wednesday, July 29, 2015 2:38:41 AM
>>>
>>> On Tue, 28 Jul 2015, Aaron Sierra wrote:
>>>
>>>>>>>> @@ -933,7 +956,7 @@ gpe0_done:
>>>>>>>>   	lpc_chipset_info[priv->chipset].use_gpio = ret;
>>>>>>>>   	lpc_ich_enable_gpio_space(dev);
>>>>>>>>
>>>>>>>> -	lpc_ich_finalize_cell(dev, &lpc_ich_cells[LPC_GPIO]);
>>>>>>>> +	lpc_ich_finalize_gpio_cell(dev);
>>>>>>>>   	ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
>>>>>>>>   			      &lpc_ich_cells[LPC_GPIO], 1, NULL, 0, NULL);
>>>>>>>>
>>>>>>>> @@ -1007,7 +1030,10 @@ static int lpc_ich_init_wdt(struct pci_dev
>>>>>>>> *dev)
>>>>>>>>   		res->end = base_addr + ACPIBASE_PMC_END;
>>>>>>>>   	}
>>>>>>>>
>>>>>>>> -	lpc_ich_finalize_cell(dev, &lpc_ich_cells[LPC_WDT]);
>>>>>>>> +	ret = lpc_ich_finalize_wdt_cell(dev);
>>>>>>>> +	if (ret)
>>>>>>>> +		goto wdt_done;
>>>>>>>> +
>>>>>>>>   	ret = mfd_add_devices(&dev->dev, PLATFORM_DEVID_AUTO,
>>>>>>>>   			      &lpc_ich_cells[LPC_WDT], 1, NULL, 0, NULL);
>>>>>>>
>>>>>>> Why do you have an mfd_add_devices() call for each device?
>>>>>>
>>>>>> Good question. This call has been present since March 2012 when support
>>>>>> was first added for iTCO_wdt in commit 887c8ec7219f ("watchdog: Convert
>>>>>> iTCO_wdt driver to mfd model").
>>>>>>
>>>>>> There's no good reason that I can see. Aaron?
>>>>
>>>> I chose to call mfd_add_devices() in each device init function
>>>> because I thought it was the easiest way to avoid registering an
>>>> incomplete/invalid MFD cell should an error occur during init.
>>>>
>>>> That way device registration wouldn't be an all-or-nothing affair.
>>>>
>>>> Doesn't mfd_add_devices() bail out after the first unsuccessful
>>>> mfd to platform device translation?
>>>
>>> Right, as it should.
>>>
>>> Under what circumstance would an error occur and you'd wish to carry
>>> on registering devices?
>>
>> Lee,
>>
>> The two devices that this driver is responsible for are conceptually
>> independent; they simply are lumped together in one PCI device. No
>> failure while preparing resources for the watchdog device should
>> prevent the GPIO device from being registered.
>
> This makes me think that perhaps this isn't an MFD at all then?
>
> Perhaps I should invest some time to looking into that.
>

The alternative, unless I am missing something, would be to
bind two drivers to the same pci device, which is not currently
possible in Linux. How would you suggest to do that if not with
an mfd driver ?

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/