Re: [PATCH 3/3] EDAC: amd64_edac: decide if driver can load successfully early.

[Dmitry Torokhov]

On Thu, Mar 19, 2015 at 04:59:53PM +0100, Borislav Petkov wrote:
> On Thu, Mar 19, 2015 at 08:52:53AM -0700, Dmitry Torokhov wrote:
> > Why does the number of bridges matter? Yo can have bazillion bridges, it
> 
> Because it only makes sense to leave the driver loaded when at least one
> northbridge supports and has ECC enabled.

By the same token it only makes sense to load e1000e when I have e1000e
device loaded, but we allow it to load anyway. Or psmouse. Or pretty
much any other drivers (sans some platform code). The fact is that we
for long time have separated module loading and driver binding. Loading
driver even without the devices is standard behavior.

> 
> > doe snot mean you'll have more than one copy of driver code. Note that
> > even with the changes we do not leave the driver bound to the devices if
> > there is no ECC.
> 
> I know, but the driver remains loaded and it could mislead people that
> they actually have error decoding enabled.

Does anyone look at the loaded modules to see if functionality is
available? They should look if any devices are actually bound to the
driver (in sysfs). I mean, even without the changes I can unbind the
edac driver from northbridge via sysfs thus disabling error decoding and
"mislead" people.

What is worse is that if I unbind node 0 device the pci_ctl still stays
around, it is not disabled. And it is not reenabled after I rebind that
node 0 device. So actually, right now the driver is broken WRT the
dynamic binding/unbinding.

> 
> Btw, why are you guys jumping on this driver?
> 
> I don't really believe it would matter if it loaded asynchronously on
> the type of boxes it is supposed to be running. So why not mark it as
> sync and we all forget about the whole issue?

Yes, we can. We have to keep the FORCE_SYNCHRONOUS flag for platform
drivers registered with platform_driver_probe() anyways (or maybe not if
we decide to kill platform_driver_probe too).

Still most of the patch 3 is needed for correct operation and I think
patches 1 and 2 improve the code as well.

Thanks.

-- 
Dmitry
--
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/