public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@arndb•de>
To: linuxppc-dev@ozlabs•org
Subject: Re: [PATCH 10/13] powerpc: Add arch/powerpc mv64x60 PCI setup
Date: Thu, 3 May 2007 09:17:46 +0200	[thread overview]
Message-ID: <200705030917.47037.arnd@arndb.de> (raw)
In-Reply-To: <20070502214609.GE27253@xyzzy.farnsworth.org>

On Wednesday 02 May 2007, Dale Farnsworth wrote:

> Index: linux-2.6-powerpc-df/arch/powerpc/sysdev/Makefile
> ===================================================================
> --- linux-2.6-powerpc-df.orig/arch/powerpc/sysdev/Makefile
> +++ linux-2.6-powerpc-df/arch/powerpc/sysdev/Makefile
> @@ -16,6 +16,10 @@ obj-$(CONFIG_TSI108_BRIDGE)	+= tsi108_pc
>  obj-$(CONFIG_QUICC_ENGINE)	+= qe_lib/
>  obj-$(CONFIG_MV64X60)		+= mv64x60_pic.o mv64x60_dev.o
>  
> +ifeq ($(CONFIG_PCI),y)
> +obj-$(CONFIG_MV64X60)		+= mv64x60_pci.o
> +endif
> +

I'd write this as

mv64x60-$(CONFIG_INDIRECT_PCI)	+= mv64x60_pci.o
obj-$(CONFIG_MV64X60)		+= mv64x60-y

though that doesn't make much difference any more

> +#ifdef CONFIG_SYSFS
> +/* 32-bit hex or dec stringified number + '\n' */
> +#define MV64X60_VAL_LEN_MAX		11
> +#define MV64X60_PCICFG_CPCI_HOTSWAP	0x68
> +
> +DECLARE_MUTEX(mv64x60_hs_lock);

Please avoid using struct semephores in new code, we now have struct mutex
for this, which gets defined as

static DEFINE_MUTEX(mv64x60_hs_mutex);

> +static ssize_t mv64x60_hs_reg_read(struct kobject *kobj, char *buf, loff_t off,
> +				   size_t count)
> +{
> +	u32 v;
> +	int save_exclude;
> +
> +	if (off > 0)
> +		return 0;
> +	if (count < MV64X60_VAL_LEN_MAX)
> +		return -EINVAL;
> +
> +	if (down_interruptible(&mv64x60_hs_lock))
> +		return -ERESTARTSYS;
> +	save_exclude = mv64x60_pci_exclude_bridge;
> +	mv64x60_pci_exclude_bridge = 0;
> +	early_read_config_dword(mv64x60_primary_hose, 0, PCI_DEVFN(0, 0),
> +				MV64X60_PCICFG_CPCI_HOTSWAP, &v);

Why do you use early_read_config_dword, not pci_read_config_dword()?

> +	mv64x60_pci_exclude_bridge = save_exclude;
> +	up(&mv64x60_hs_lock);
> +
> +	return sprintf(buf, "0x%08x\n", v);
> +}

<snip>

> +static int mv64x60_exclude_device(u_char bus, u_char devfn)
> +{
> +       if ((bus == 0 || bus == mv64x60_pci2_busno) &&
> +           PCI_SLOT(devfn) == 0 && mv64x60_pci_exclude_bridge)
> +               return PCIBIOS_DEVICE_NOT_FOUND;
> +
> +       return PCIBIOS_SUCCESSFUL;
> +}

The locking here looks wrong. If you call mv64x60_exclude_device() from one thread
thread while another one is calling mv64x60_hs_reg_read(), the bridge will
not be excluded.

	Arnd <><

  parent reply	other threads:[~2007-05-03  7:17 UTC|newest]

Thread overview: 87+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-04-25 23:46 [PATCH 0/13] powerpc: Add support for Marvell/mv64x60 and prpmc2800 Mark A. Greer
2007-04-25 23:55 ` [PATCH 1/13] powerpc: Add Makefile rule to wrap dts file in zImage Mark A. Greer
2007-04-30  6:06   ` David Gibson
2007-04-25 23:55 ` [PATCH 2/13] powerpc: Add dt_xlate_addr() to bootwrapper Mark A. Greer
2007-04-26 16:44   ` Scott Wood
2007-04-27  5:55   ` Paul Mackerras
2007-04-27 20:48     ` Mark A. Greer
2007-04-25 23:56 ` [PATCH 3/13] powerpc: Add bootwrapper support for Marvell/mv64x60 hostbridge Mark A. Greer
2007-04-27  6:01   ` Paul Mackerras
2007-04-27 22:02     ` Mark A. Greer
2007-05-03  5:25       ` Paul Mackerras
2007-05-03 18:44         ` Mark A. Greer
2007-05-03 19:00           ` Mark A. Greer
2007-05-05 23:27           ` Paul Mackerras
2007-05-07 18:15             ` Mark A. Greer
2007-04-25 23:57 ` [PATCH 4/13] powerpc: Add bootwrapper support for Marvell MPSC Mark A. Greer
2007-04-25 23:57 ` [PATCH 5/13] powerpc: Add bootwrapper support for Marvell/mv64x60 I2C Mark A. Greer
2007-04-25 23:58 ` [PATCH 6/13] powerpc: Add arch/powerpc support for Marvell/mv64x60 hostbridge Mark A. Greer
2007-04-26  0:42   ` Arnd Bergmann
2007-04-26  5:49     ` Dale Farnsworth
2007-05-02 21:38   ` [PATCH 6/13] powerpc: Add arch/powerpc interrupt handler for mv64x60 Dale Farnsworth
2007-05-03  1:47     ` Stephen Rothwell
2007-05-03  2:55       ` Dale Farnsworth
2007-04-25 23:59 ` [PATCH 7/13] powerpc: Add arch/powerpc mv64x60 MPSC platform data setup Mark A. Greer
2007-04-26  0:14   ` Arnd Bergmann
2007-04-26  5:57     ` Dale Farnsworth
2007-04-26 11:24       ` Arnd Bergmann
2007-04-26 14:30         ` Dale Farnsworth
2007-04-26 15:14           ` Arnd Bergmann
2007-05-02 21:41   ` Dale Farnsworth
2007-05-03  6:40     ` Arnd Bergmann
2007-05-04 21:03     ` [PATCH 7/13] powerpc: Create Marvell mv64x60 MPSC (serial) platform_data Dale Farnsworth
2007-05-05 12:26       ` Arnd Bergmann
2007-04-26  0:00 ` [PATCH 8/13] powerpc: Add arch/powerpc mv64x60_eth platform data setup Mark A. Greer
2007-04-26  0:18   ` Arnd Bergmann
2007-04-26  6:00     ` Dale Farnsworth
2007-05-02 21:43   ` Dale Farnsworth
2007-05-03  2:03     ` Stephen Rothwell
2007-05-03  6:43     ` Arnd Bergmann
2007-05-04 21:06     ` [PATCH 8/13] powerpc: Create Marvell mv64x60 ethernet platform_data Dale Farnsworth
2007-05-05 12:28       ` Arnd Bergmann
2007-04-26  0:00 ` [PATCH 9/13] powerpc: Add arch/powerpc mv64x60 I2C platform data setup Mark A. Greer
2007-04-26  0:21   ` Arnd Bergmann
2007-04-26  0:43     ` Mark A. Greer
2007-04-26  0:55       ` Arnd Bergmann
2007-04-26  1:13         ` Mark A. Greer
2007-04-26  2:02           ` Arnd Bergmann
2007-04-26  6:08             ` Dale Farnsworth
2007-04-26  9:00               ` Arnd Bergmann
2007-04-26 14:19                 ` Dale Farnsworth
2007-04-26 15:04                   ` Arnd Bergmann
2007-04-27 23:50                     ` Dale Farnsworth
2007-04-28  1:05                       ` Arnd Bergmann
2007-04-28  2:40                         ` Dale Farnsworth
2007-05-01  4:58                         ` Paul Mackerras
2007-05-01  4:45                     ` Paul Mackerras
2007-04-26  6:48             ` Mark A. Greer
2007-05-02 21:44   ` Dale Farnsworth
2007-05-03  6:53     ` Arnd Bergmann
2007-05-03 13:06       ` 
2007-05-04 21:08     ` [PATCH 9/13] powerpc: Create Marvell mv64x60 I2C platform_data Dale Farnsworth
2007-05-05 12:29       ` Arnd Bergmann
2007-04-26  0:01 ` [PATCH 10/13] powerpc: Add arch/powerpc mv64x60 PCI setup Mark A. Greer
2007-04-26  0:25   ` Arnd Bergmann
2007-04-26  6:33     ` Dale Farnsworth
2007-04-26 11:39       ` Arnd Bergmann
2007-04-26 14:42         ` Dale Farnsworth
2007-05-02 21:46   ` Dale Farnsworth
2007-05-03  2:13     ` Stephen Rothwell
2007-05-03  2:57       ` Dale Farnsworth
2007-05-03  7:17     ` Arnd Bergmann [this message]
2007-05-03 13:45       ` Dale Farnsworth
2007-05-04 21:10     ` [PATCH 10/13] powerpc: Add Marvell mv64x60 PCI bridge support Dale Farnsworth
2007-05-05 12:30       ` Arnd Bergmann
2007-04-26  0:01 ` [PATCH 11/13] powerpc: Add DTS file for the Motorola PrPMC2800 platform Mark A. Greer
2007-04-26 16:42   ` Scott Wood
2007-04-26 23:34     ` Mark A. Greer
2007-04-26 23:37     ` David Gibson
2007-04-27 20:41   ` Mark A. Greer
2007-04-30 16:45     ` Jon Loeliger
2007-04-30 18:08       ` Mark A. Greer
2007-04-30 22:29       ` Mark A. Greer
2007-04-26  0:02 ` [PATCH 12/13] powerpc: Add bootwrapper support for " Mark A. Greer
2007-04-26  0:02 ` [PATCH 13/13] powerpc: Add arch/powerpc support for the " Mark A. Greer
2007-04-26  0:45 ` [PATCH 0/13] powerpc: Add support for Marvell/mv64x60 and prpmc2800 David Gibson
2007-04-26  0:58   ` Mark A. Greer
2007-04-26  1:15     ` Mark A. Greer

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=200705030917.47037.arnd@arndb.de \
    --to=arnd@arndb$(echo .)de \
    --cc=linuxppc-dev@ozlabs$(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