linux-kernel - Re: [PATCH 2/2] staging: comedi: addi_apci_1564: use addi_watchdog module to init watchdog subdevice

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] [day] [month] [year] [list]

Message-ID: <CA+jnaaWkfKXYzEJXgZHaEmR92k-U4a11GgGb5AW9Nrsrys_iBQ@mail.gmail.com>
Date:	Wed, 16 Jul 2014 21:35:46 -0500
From:	Chase Southwood <chase.southwood@...il.com>
To:	Ian Abbott <abbotti@....co.uk>
Cc:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	hsweeten@...ionengravers.com,
	"devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] staging: comedi: addi_apci_1564: use addi_watchdog
 module to init watchdog subdevice

On Tue, Jul 15, 2014 at 5:14 AM, Ian Abbott <abbotti@....co.uk> wrote:
> On 2014-07-15 05:00, Chase Southwood wrote:
>>
>> On Mon, Jul 14, 2014 at 4:22 AM, Ian Abbott <abbotti@....co.uk> wrote:
>>>
>>> On 2014-07-12 23:44, Chase Southwood wrote:
>>>>
>>>>
>>>> Use the addi_watchdog module to provide support for the watchdog
>>>> subdevice.
>>>>
>>>> Also, rearrange the subdevice init blocks so that the order makes sense.
>>>> Digital input/output subdevices and subdevices for DI/DO interrupt
>>>> support, followed by timer/counter/watchdog subdevices is the new order.
>>>>
>>>> Signed-off-by: Chase Southwood <chase.southwood@...il.com>
>>>> Cc: Ian Abbott <abbotti@....co.uk>
>>>> Cc: H Hartley Sweeten <hsweeten@...ionengravers.com>
>>>> ---
>>>>    drivers/staging/comedi/drivers/addi_apci_1564.c | 34
>>>> +++++++++++++++----------
>>>>    1 file changed, 20 insertions(+), 14 deletions(-)
>>>
>>>
>>>
>>> I don't think the subdevice order matters that much, and I prefer to keep
>>> them stable, but since this driver is in such a state of flux, it doesn't
>>> really matter.
>>>
>>
>> Hi Ian!
>> Quick question here about this.  First off, duly noted that grouping
>> subdevices by function isn't necessary and I won't shuffle them around
>> like this in the future.  Second, the reason I stuck the watchdog at
>> the end is because it causes an early return if addi_watchdog_init()
>> returns an error and it seemed  appropriate at the end so it doesn't
>> prevent the initialization of any other subdevices if that call should
>> fail.  Now I realize that it is very unlikely that that call fails,
>> but in any case should I put future subdevice inits above the watchdog
>> for the same reason (so they aren't at risk of not getting
>> initialized), or does that count for subdevice order not being stable
>> and you would prefer them all to go at the end?
>
>
> Since you return an error from the auto_attach handler
> apci1564_auto_attach() when addi_watchdog_init() fails, it makes little
> difference what order the subdevices are initialized in.  The error from
> auto_attach handler causes the comedi core to call the detach handler
> apci1564_detach() and tear everything down.  Ultimately,
> comedi_pci_auto_attach() will return an error back to the PCI probe function
> apci1564_pci_probe(), which will propagate it to the PCI subsystem.
>
> In general, if adding a new subdevice, either add it to the end or replace
> an "unused" subdevice.
>

Oh excellent.  Perfect, that makes sense.  Thanks for taking the time
to explain, I'll make sure everything goes at the end from now on.

Thanks,
Chase

>
> --
> -=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@....co.uk>        )=-
> -=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
--
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/