Re: [PATCH] Intel Management Engine Interface

From: Alan Cox
Date: Tue Aug 12 2008 - 15:42:49 EST


> This source code was designed few years ago and there is already
> written userspace software.

Diddums that is what you get for keeping stuff away from upstream

> Removal of driver internal error codes may affect software.
> I think it is more risky than keeping these own errno numbers.

I would disagree. We use ERR_PTR() and other macros with errors. Using
strange numbers is asking for risk.
>
> >
> > E.g. unlocked_ioctl switch hasn't been done plus other things mentioned
> > below too (not all of them)
> >
> unlocked_ioctl would require to add lock as big kernel lock replacement.
> It is risky in such complex code, especially if speed increase is not expected.
>
> We don't want to init_timer() here. It is done later, to avoid race condition.
> We only configure timer, but we are not ready to start it.

init_timer does not start a timer.

> >
> >>> +/* IOCTL commands */
> >>> +#define IOCTL_HECI_GET_VERSION \
> >>> + _IOWR('H' , 0x0, struct heci_message_data)
> > ... and conflicts with hid as I commented before.
>
> Sorry for missing explanations.
> This conflict can't be avoided, because there is already written
> software that depends on these ioctl definitions.

See "diddums" message above. You mess up tools like strace if you don't
fix this and strace is more important than some minor driver detail
caused by not upstreaming code earlier.

The non unlocked version of ioctl() is going away and the BKL will in
time follow so you might as well deal with that now too.

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