From: Nathan Lynch <ntl@pobox•com>
To: Linas Vepstas <linas@austin•ibm.com>
Cc: linuxppc-dev@ozlabs•org, Paul Mackerras <paulus@samba•org>
Subject: Re: [PATCH 1/4] powerpc: Add EEH sysfs blinkenlights
Date: Wed, 23 May 2007 18:22:56 -0500 [thread overview]
Message-ID: <20070523232256.GJ29914@localdomain> (raw)
In-Reply-To: <20070523171646.GA32380@austin.ibm.com>
Hi Linas-
Linas Vepstas wrote:
>
> Add sysfs blinkenlights for EEH statistics. Shuffle the
> eeh_add_device_tree() call so that it appears in the correct
> sequence.
( blinkenlights? :)
To me this seems a somewhat terse changelog considering that the patch
introduces a user-visible interface. The changelog does not really
say what the code is doing or why, or who will use it.
> Index: linux-2.6.22-rc1/arch/powerpc/platforms/pseries/eeh_sysfs.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.22-rc1/arch/powerpc/platforms/pseries/eeh_sysfs.c 2007-05-23 11:57:23.000000000 -0500
> @@ -0,0 +1,84 @@
> +/*
> + * Sysfs entries for PCI Error Recovery for PAPR-compliant platform.
platforms?
> +#include <linux/pci.h>
> +#include <asm/ppc-pci.h>
> +#include <asm/pci-bridge.h>
> +#include <linux/kobject.h>
Include linux/ before asm/ please.
> +/**
> + * EEH_SHOW_ATTR -- create sysfs entry for eeh statistic
> + * @_name: name of file in sysfs directory
> + * @_memb: name of member in struct pci_dn to access
> + * @_format: printf format for display
> + *
> + * All of the attributes look very similar, so just
> + * auto-gen a cut-n-paste routine to display them.
> + */
> +#define EEH_SHOW_ATTR(_name,_memb,_format) \
> +static ssize_t eeh_show_##_name(struct device *dev, \
> + struct device_attribute *attr, char *buf) \
I have been frustrated by similar constructions more than once in the
midst of debugging. I know this has become a common practice with
sysfs-related code, but using cpp to generate function names
completely defeats grep etc. when you're trying to track down a
problem, and I'd not like to see this sort of thing propagated.
> --- linux-2.6.22-rc1.orig/include/asm-powerpc/ppc-pci.h 2007-05-23 11:43:42.000000000 -0500
> +++ linux-2.6.22-rc1/include/asm-powerpc/ppc-pci.h 2007-05-23 11:44:43.000000000 -0500
> @@ -139,6 +139,9 @@ void eeh_clear_slot (struct device_node
> */
> struct device_node * find_device_pe(struct device_node *dn);
>
> +void eeh_sysfs_add_device(struct pci_dev *pdev);
> +void eeh_sysfs_remove_device(struct pci_dev *pdev);
Don't you need dummy static inline placeholders for CONFIG_EEH=n?
next prev parent reply other threads:[~2007-05-23 23:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-05-23 17:13 [PATCH 0/4] powerpc: EEH blinkenlights, copyright cleanup Linas Vepstas
2007-05-23 17:16 ` [PATCH 1/4] powerpc: Add EEH sysfs blinkenlights Linas Vepstas
2007-05-23 23:22 ` Nathan Lynch [this message]
2007-05-24 17:34 ` Linas Vepstas
2007-05-23 23:42 ` Stephen Rothwell
2007-05-23 17:20 ` [PATCH 2/4] powerpc: show EEH per-device false positives Linas Vepstas
2007-05-23 17:23 ` [PATCH 3/4] powerpc: remove dead EEH code Linas Vepstas
2007-05-23 23:53 ` Stephen Rothwell
2007-05-24 18:00 ` Linas Vepstas
2007-05-23 17:28 ` [PATCH 4/4] powerpc: tweak EEH copyright info Linas Vepstas
2007-05-23 23:55 ` Stephen Rothwell
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=20070523232256.GJ29914@localdomain \
--to=ntl@pobox$(echo .)com \
--cc=linas@austin$(echo .)ibm.com \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=paulus@samba$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox