[PATCH] PPC64: EEH Recovery

[Linas Vepstas]

On Sat, Nov 20, 2004 at 05:11:53PM -0600, Milton D. Miller II was heard to remark:
> > The patch below implements hotplug style EEH error recovery. 
> 
> Here's some discussion :)
> 
> 1) why are you EXPORT_SYMBOL rtas_write_config?  eeh.c isn't a module


No reason. Force of habit. I'll remove that.

> 2) I object to grabing pci devices so they don't disappear and reappear.
>    I worry about duplicate devices across register/unregister and sysfs
>    kobject lifetimes getting confused and duplicate names.

I'll double-check, I was under the impression that the unregister
happened, and I was just avodiing the final free().  Maybe I'm sorely
mistaken.

>   I'd prefer we just kept the pci config stuff we are going to restore
>   off the of device node.

I could do that.  However, while coding this, I was anticipating the
comment "why did you add this stuff to struct device_node, when there's
already a spot for it in the pci_dev struct?"  and so I recycled the
pci-dev structs.  I don't much care one way or the other about this
issue.


> > -	if (!dev)
> > +	if (!dev) 
> >  		return val;
> >  
> >  	dn = pci_device_to_OF_node(dev);
> 
> adding trailing white space.  tsk tsk

Oops.

This comes from the nasty habit of the linux coding style not using
braces for single-line if's written as double-lines.  So if one 
inserts braces  during debug, one has to remember to remove 
them afterwards,  which can lead to inadvertent whitespace.  
Personally, I'd like to get the linux kernel coding style changed, 
this *is* an issue I do care about, a lot. However, I figure that 
would be an unpleasent battle.  Oh well.

> > +struct eeh_cfg_tree
> > +{
> > +	struct eeh_cfg_tree *sibling;
> > +	struct eeh_cfg_tree *child;
> > +	struct pci_dev *dev;
> > +	struct device_node *dn;
> > +};
> 
> Do we need a tree for this?

? Don't understand the question. PCI devices are arranged in a tree. 
One of the cards I test with has a bridge and several devices under it. 
So one has to walk the whole tree, which might be arbitrarily deep, to
get to all of the devices.

> IICK
> 
> 1) matching on a stirng when we have the device_node?  really?
>    please match on pci_device_to_OF_node(dev)
> 2) for_each_pcidev() please

Sorry, I copied pre-existing code that does this.
I'll fix it here, and in the other places as well. 

> How about a list of (dn *, pci config words to write)?
> or an array of dn

? Don't understand the question.

> > -EXPORT_SYMBOL_GPL(rpaphp_find_hotplug_slot);
> 
> not analyzed ...

??

--linas