[+cc linux-pci]

On Thu, Mar 16, 2017 at 10:50:05PM +0100, Thomas Gleixner wrote:
> x86 wants to get rid of the global pci_lock protecting the config space
> accessors so ECAM mode can operate completely lockless, but the CE4100 pci
> code relies on that to protect the simulation registers.

s/pci/PCI/

> Restructure the code so it uses the x86 specific pci_config_lock to
> serialize the inner workings of the CE4100 PCI magic. That allows to remove
> the global locking via pci_lock later.
> 
> Signed-off-by: Thomas Gleixner <t...@linutronix.de>
> ---
>  arch/x86/pci/ce4100.c |   87 
> +++++++++++++++++++++++++++-----------------------
>  1 file changed, 48 insertions(+), 39 deletions(-)
> 
> --- a/arch/x86/pci/ce4100.c
> +++ b/arch/x86/pci/ce4100.c
> @@ -65,6 +65,9 @@ struct sim_reg_op {
>  { PCI_DEVFN(device, func), offset, init_op, read_op, write_op,\
>       {0, SIZE_TO_MASK(size)} },
>  
> +/*
> + * All read/write functions are called with pci_config_lock held.
> + */
>  static void reg_init(struct sim_dev_reg *reg)
>  {
>       pci_direct_conf1.read(0, 1, reg->dev_func, reg->reg, 4,
> @@ -73,21 +76,13 @@ static void reg_init(struct sim_dev_reg
>  
>  static void reg_read(struct sim_dev_reg *reg, u32 *value)
>  {
> -     unsigned long flags;
> -
> -     raw_spin_lock_irqsave(&pci_config_lock, flags);
>       *value = reg->sim_reg.value;
> -     raw_spin_unlock_irqrestore(&pci_config_lock, flags);
>  }
>  
>  static void reg_write(struct sim_dev_reg *reg, u32 value)
>  {
> -     unsigned long flags;
> -
> -     raw_spin_lock_irqsave(&pci_config_lock, flags);
>       reg->sim_reg.value = (value & reg->sim_reg.mask) |
>               (reg->sim_reg.value & ~reg->sim_reg.mask);
> -     raw_spin_unlock_irqrestore(&pci_config_lock, flags);
>  }
>  
>  static void sata_reg_init(struct sim_dev_reg *reg)
> @@ -117,12 +112,8 @@ static void sata_revid_read(struct sim_d
>  
>  static void reg_noirq_read(struct sim_dev_reg *reg, u32 *value)
>  {
> -     unsigned long flags;
> -
> -     raw_spin_lock_irqsave(&pci_config_lock, flags);
>       /* force interrupt pin value to 0 */
>       *value = reg->sim_reg.value & 0xfff00ff;
> -     raw_spin_unlock_irqrestore(&pci_config_lock, flags);
>  }
>  
>  static struct sim_dev_reg bus1_fixups[] = {
> @@ -265,24 +256,33 @@ int bridge_read(unsigned int devfn, int
>       return retval;
>  }
>  
> -static int ce4100_conf_read(unsigned int seg, unsigned int bus,
> -                         unsigned int devfn, int reg, int len, u32 *value)
> +static int ce4100_bus1_read(unsigned int devfn, int reg, int len, u32 *value)
>  {
> +     unsigned long flags;
>       int i;
>  
> -     WARN_ON(seg);
> -     if (bus == 1) {
> -             for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
> -                     if (bus1_fixups[i].dev_func == devfn &&
> -                         bus1_fixups[i].reg == (reg & ~3) &&
> -                         bus1_fixups[i].read) {
> -                             bus1_fixups[i].read(&(bus1_fixups[i]),
> -                                                 value);
> -                             extract_bytes(value, reg, len);
> -                             return 0;
> -                     }
> +     for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
> +             if (bus1_fixups[i].dev_func == devfn &&
> +                 bus1_fixups[i].reg == (reg & ~3) &&
> +                 bus1_fixups[i].read) {
> +
> +                     raw_spin_lock_irqsave(&pci_config_lock, flags);
> +                     bus1_fixups[i].read(&(bus1_fixups[i]), value);
> +                     raw_spin_unlock_irqrestore(&pci_config_lock, flags);
> +                     extract_bytes(value, reg, len);
> +                     return 0;
>               }
>       }
> +     return -1;
> +}
> +
> +static int ce4100_conf_read(unsigned int seg, unsigned int bus,
> +                         unsigned int devfn, int reg, int len, u32 *value)
> +{
> +     WARN_ON(seg);
> +
> +     if (bus == 1 && !ce4100_bus1_read(devfn, reg, len, value))
> +             return 0;
>  
>       if (bus == 0 && (PCI_DEVFN(1, 0) == devfn) &&
>           !bridge_read(devfn, reg, len, value))
> @@ -291,23 +291,32 @@ static int ce4100_conf_read(unsigned int
>       return pci_direct_conf1.read(seg, bus, devfn, reg, len, value);
>  }
>  
> -static int ce4100_conf_write(unsigned int seg, unsigned int bus,
> -                          unsigned int devfn, int reg, int len, u32 value)
> +static int ce4100_bus1_write(unsigned int devfn, int reg, int len, u32 value)
>  {
> +     unsigned long flags;
>       int i;
>  
> -     WARN_ON(seg);
> -     if (bus == 1) {
> -             for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
> -                     if (bus1_fixups[i].dev_func == devfn &&
> -                         bus1_fixups[i].reg == (reg & ~3) &&
> -                         bus1_fixups[i].write) {
> -                             bus1_fixups[i].write(&(bus1_fixups[i]),
> -                                                  value);
> -                             return 0;
> -                     }
> +     for (i = 0; i < ARRAY_SIZE(bus1_fixups); i++) {
> +             if (bus1_fixups[i].dev_func == devfn &&
> +                 bus1_fixups[i].reg == (reg & ~3) &&
> +                 bus1_fixups[i].write) {
> +
> +                     raw_spin_lock_irqsave(&pci_config_lock, flags);
> +                     bus1_fixups[i].write(&(bus1_fixups[i]), value);
> +                     raw_spin_unlock_irqrestore(&pci_config_lock, flags);
> +                     return 0;
>               }
>       }
> +     return -1;
> +}
> +
> +static int ce4100_conf_write(unsigned int seg, unsigned int bus,
> +                          unsigned int devfn, int reg, int len, u32 value)
> +{
> +     WARN_ON(seg);
> +
> +     if (bus == 1 && !ce4100_bus1_write(devfn, reg, len, value))
> +             return 0;
>  
>       /* Discard writes to A/V bridge BAR. */
>       if (bus == 0 && PCI_DEVFN(1, 0) == devfn &&
> @@ -318,8 +327,8 @@ static int ce4100_conf_write(unsigned in
>  }
>  
>  static const struct pci_raw_ops ce4100_pci_conf = {
> -     .read = ce4100_conf_read,
> -     .write = ce4100_conf_write,
> +     .read   = ce4100_conf_read,
> +     .write  = ce4100_conf_write,
>  };
>  
>  int __init ce4100_pci_init(void)
> 
> 

Reply via email to