All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@amd64.org>
To: Kay Sievers <kay@vrfy.org>
Cc: Andreas Herrmann <andreas.herrmann3@amd.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Petkov, Borislav" <Borislav.Petkov@amd.com>,
	linux-kernel@vger.kernel.org, Dave Jones <davej@redhat.com>
Subject: Re: x86, microcode: Conversion from sysdev class caused regression
Date: Wed, 11 Apr 2012 22:04:27 +0200	[thread overview]
Message-ID: <20120411200427.GD18114@aftab> (raw)
In-Reply-To: <CAPXgP11mjyK7vJoQtsSf3kD-SYn6d_Y9feobY9KiiSYcFqOwwA@mail.gmail.com>

+ Dave.

On Wed, Apr 11, 2012 at 07:06:21PM +0200, Kay Sievers wrote:
> > The reason for the error is that subsys_interface_register() doesn't
> > handle the return value of sif->add_dev (and there's also no unwinding
> > of the interface registration). Instead subsys_interface_register
> > always returns 0.
>
> Which is the intended behaviour of 'subsystem interfaces' from the
> driver-core perspective. It should not matter if one of a bunch of
> devices do not 'like' this 'interface'. It is the same model as a
> 'driver', we do not cancel the link-in of a driver if one device does
> not like the driver.

But you're not looking at the return value of sif->add_dev which looks
strange to me. Let me put it this way: why do you have return values to
->add_dev's interface then if you're not going to look at them?

A warning that one of the dev_add's failed could probably make sense
here...

[..]

> I think a quick return in the microcode driver, for a device which has
> no active interface state is the best solution here.

Actually, it is even easier: the code clumsily does:

sysfs_create_group
microcode_init_cpu
if (err)
	sysfs_remove_group

so we go and create sysfs group, THEN check whether this CPU is
supported and if not, remove the group again which is a bunch of crap if
you ask me. The right way to go should be:

microcode_init_cpu
if (err)
	return;
sysfs_create_group

and then there's no need to do all that sysfs group dancing. Andreas,
let me know if you wanna do it, or I should take care of it.

Thanks.

-- 
Regards/Gruss,
Boris.

Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach
GM: Alberto Bozzo
Reg: Dornach, Landkreis Muenchen
HRB Nr. 43632 WEEE Registernr: 129 19551

  reply	other threads:[~2012-04-11 20:04 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-11 16:38 x86, microcode: Conversion from sysdev class caused regression Andreas Herrmann
2012-04-11 17:06 ` Kay Sievers
2012-04-11 20:04   ` Borislav Petkov [this message]
2012-04-11 20:06 ` Greg Kroah-Hartman
2012-04-11 20:10   ` Borislav Petkov
2012-04-12 16:23     ` Borislav Petkov
2012-04-12 16:30       ` [PATCH 1/2] x86, microcode: Fix sysfs warning during module unload Borislav Petkov
2012-04-12 22:45         ` Greg Kroah-Hartman
2012-04-12 16:34       ` [PATCH 2/2] x86, microcode: Ensure that module is only loaded for Borislav Petkov
2012-04-12 22:45         ` Greg Kroah-Hartman
2012-04-16  8:42         ` Srivatsa S. Bhat
2012-04-16 13:43           ` Borislav Petkov
2012-04-17 14:11             ` Srivatsa S. Bhat
2012-04-17 14:50               ` Borislav Petkov
2012-04-17 15:53                 ` Gene Heskett
2012-04-17 16:02                   ` Kay Sievers
2012-04-17 17:30                     ` Gene Heskett
2012-04-17 18:07                       ` Borislav Petkov
2012-04-22  2:55                     ` Henrique de Moraes Holschuh
2012-04-25 23:36                       ` Kay Sievers
2012-04-17 17:35                 ` Srivatsa S. Bhat
2012-05-08  4:28           ` [tip:x86/urgent] x86/microcode: Ensure that module is only loaded on supported Intel CPUs tip-bot for Srivatsa S. Bhat
2012-04-14 18:23 ` [tip:x86/urgent] x86, microcode: Fix sysfs warning during module unload on unsupported CPUs tip-bot for Andreas Herrmann
2012-04-14 18:24 ` [tip:x86/urgent] x86, microcode: Ensure that module is only loaded on supported AMD CPUs tip-bot for Andreas Herrmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120411200427.GD18114@aftab \
    --to=bp@amd64.org \
    --cc=Borislav.Petkov@amd.com \
    --cc=andreas.herrmann3@amd.com \
    --cc=davej@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=kay@vrfy.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.