2003-08-19 12:48:46

by Marc Zyngier

[permalink] [raw]
Subject: [PATCH] EISA bus update

Linus,

The enclosed patch fixes a few things for the EISA bus :

- Don't leave resource name uninitialized if CONFIG_EISA_NAME is not
set.
- Print root device bus_id (so we know which bridge is probed).
- From Zwane Mwaikambo : Add a release method to virtual root, so it
stays quiet if probing fails (because some pci-eisa bridge have been
found before).

Please apply.

M.

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
# ChangeSet 1.1125 -> 1.1126
# drivers/eisa/eisa-bus.c 1.18 -> 1.19
# drivers/eisa/virtual_root.c 1.7 -> 1.8
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/08/19 [email protected] 1.1126
# Don't leave resource name uninitialized if CONFIG_EISA_NAME is not set.
# Print root device bus_id (so we know which bridge is probed).
# From Zwane Mwaikambo :
# Add a release method to virtual root, so it stays quiet
# if probing fails (because some pci-eisa bridge have been found before).
# --------------------------------------------
#
diff -Nru a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
--- a/drivers/eisa/eisa-bus.c Tue Aug 19 14:44:43 2003
+++ b/drivers/eisa/eisa-bus.c Tue Aug 19 14:44:43 2003
@@ -172,6 +172,7 @@
{
char *sig;
unsigned long sig_addr;
+ int i;

sig_addr = SLOT_ADDRESS (root, slot) + EISA_VENDOR_ID_OFFSET;

@@ -189,13 +190,13 @@
edev->dev.dma_mask = &edev->dma_mask;
sprintf (edev->dev.bus_id, "%02X:%02X", root->bus_nr, slot);

+ for (i = 0; i < EISA_MAX_RESOURCES; i++) {
#ifdef CONFIG_EISA_NAMES
- {
- int i;
- for (i = 0; i < EISA_MAX_RESOURCES; i++)
edev->res[i].name = edev->pretty_name;
- }
+#else
+ edev->res[i].name = edev->id.sig;
#endif
+ }

if (is_forced_dev (enable_dev, root, edev))
edev->state = EISA_CONFIG_ENABLED | EISA_CONFIG_FORCED;
@@ -274,7 +275,8 @@
int i, c;
struct eisa_device *edev;

- printk (KERN_INFO "EISA: Probing bus %d\n", root->bus_nr);
+ printk (KERN_INFO "EISA: Probing bus %d at %s\n",
+ root->bus_nr, root->dev->bus_id);

/* First try to get hold of slot 0. If there is no device
* here, simply fail, unless root->force_probe is set. */
diff -Nru a/drivers/eisa/virtual_root.c b/drivers/eisa/virtual_root.c
--- a/drivers/eisa/virtual_root.c Tue Aug 19 14:44:43 2003
+++ b/drivers/eisa/virtual_root.c Tue Aug 19 14:44:43 2003
@@ -22,6 +22,7 @@
#endif

static int force_probe = EISA_FORCE_PROBE_DEFAULT;
+static void virtual_eisa_release (struct device *);

/* The default EISA device parent (virtual root device).
* Now use a platform device, since that's the obvious choice. */
@@ -29,6 +30,9 @@
static struct platform_device eisa_root_dev = {
.name = "eisa",
.id = 0,
+ .dev = {
+ .release = virtual_eisa_release,
+ },
};

static struct eisa_root_device eisa_bus_root = {
@@ -38,6 +42,11 @@
.slots = EISA_MAX_SLOTS,
.dma_mask = 0xffffffff,
};
+
+static void virtual_eisa_release (struct device *dev)
+{
+ /* nothing really to do here */
+}

static int virtual_eisa_root_init (void)
{

--
Places change, faces change. Life is so very strange.


2003-08-19 17:49:46

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] EISA bus update

On Tue, Aug 19, 2003 at 02:48:24PM +0200, Marc Zyngier wrote:
> +static void virtual_eisa_release (struct device *dev)
> +{
> + /* nothing really to do here */
> +}

Um, no. That's not correct. Don't just paper over valid warning
messages in the kernel by adding functions that do not do anything.

Marc, why do you think that you do not need to do anything in this
function? Don't you need to handle the fact that your code could be
removed before the release function is called?

Linus, please do not apply this just yet.

thanks,

greg k-h

2003-08-19 18:28:08

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH] EISA bus update

>>>>> "Greg" == Greg KH <[email protected]> writes:

Greg,

Greg> Marc, why do you think that you do not need to do anything in
Greg> this function? Don't you need to handle the fact that your code
Greg> could be removed before the release function is called?

Well, there is nothing to do in this function, because that's what the
whole driver does: nothing. It just presents a range of IO ports to be
probed to the main EISA code, and nothing else.

If the driver is removed from the kernel (which can't happen at the
moment, since it is not modular), it doesn't matter... Once it has
registered as an EISA bus root, it doesn't get called anymore, the
core code does it all by itself.

So no, I truly don't think there's anything to do in this release
function.

Regards,

M.
--
Places change, faces change. Life is so very strange.

2003-08-19 18:47:30

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] EISA bus update

On Tue, Aug 19, 2003 at 08:16:12PM +0200, Marc Zyngier wrote:
> >>>>> "Greg" == Greg KH <[email protected]> writes:
>
> Greg,
>
> Greg> Marc, why do you think that you do not need to do anything in
> Greg> this function? Don't you need to handle the fact that your code
> Greg> could be removed before the release function is called?
>
> Well, there is nothing to do in this function, because that's what the
> whole driver does: nothing. It just presents a range of IO ports to be
> probed to the main EISA code, and nothing else.

But it exports something in sysfs, right? Any reason you just don't
dynamically create it? It's real hard to get static allocation of
struct device correct.

> If the driver is removed from the kernel (which can't happen at the
> moment, since it is not modular), it doesn't matter...

Will this code ever be able to be built as a module? If so, this will
not be correct.

> Once it has registered as an EISA bus root, it doesn't get called
> anymore, the core code does it all by itself.

So the release function never gets called at all then? Why would this
be needed at all?

thanks,

greg k-h

2003-08-19 20:00:38

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH] EISA bus update

>>>>> "Greg" == Greg KH <[email protected]> writes:

>> Well, there is nothing to do in this function, because that's what the
>> whole driver does: nothing. It just presents a range of IO ports to be
>> probed to the main EISA code, and nothing else.

Greg> But it exports something in sysfs, right? Any reason you just
Greg> don't dynamically create it? It's real hard to get static
Greg> allocation of struct device correct.

Indeed, it registers a platform device. On the dynamic vs static
subject, it shouldn't matter here. Could switch to dynamic allocation
if needed.

Greg> Will this code ever be able to be built as a module? If so,
Greg> this will not be correct.

No, it won't ever be a module. It woudn't make sense. Most of the time,
it is needed to boot the system...

>> Once it has registered as an EISA bus root, it doesn't get called
>> anymore, the core code does it all by itself.

Greg> So the release function never gets called at all then? Why
Greg> would this be needed at all?

The only case in which it is called is when registration to the EISA
framework fails (because there is no EISA mainboard, or some PCI/EISA
bridge has registered before, with the same IO range). Thus we call
plateform_device_unregister, which calls the release function. And
this only happens at init time. Never after.

Thanks,

M.
--
Places change, faces change. Life is so very strange.