[PATCH 2/3] siimage: add sil_* I/O ops - Kernel

This is a discussion on [PATCH 2/3] siimage: add sil_* I/O ops - Kernel ; Add sil_iowrite{8,16,32}() and sil_ioread{8,16}() helpers, then use them to merge code accessing configuration registers through PCI and MMIO together. [ because of this SATA initialization bits from setup_mmio_siimage() are moved to init_chipset_siimage() ] This also cuts code size a bit: ...

+ Reply to Thread
Results 1 to 4 of 4

Thread: [PATCH 2/3] siimage: add sil_* I/O ops

  1. [PATCH 2/3] siimage: add sil_* I/O ops

    Add sil_iowrite{8,16,32}() and sil_ioread{8,16}() helpers, then use them to
    merge code accessing configuration registers through PCI and MMIO together.

    [ because of this SATA initialization bits from setup_mmio_siimage() are
    moved to init_chipset_siimage() ]

    This also cuts code size a bit:

    text data bss dec hex filename
    4437 164 0 4601 11f9 drivers/ide/pci/siimage.o.before
    3979 164 0 4143 102f drivers/ide/pci/siimage.o.after

    While at it:

    * Use I/O ops directly instead of using ->IN{B,W} and ->OUT{B,W}.

    * Fixup CodingStyle in setup_mmio_siimage().

    * Rename 'tmpbyte' variable to 'tmp' in init_chipset_siimage().

    There should be no functional changes caused by this patch.

    Signed-off-by: Bartlomiej Zolnierkiewicz
    ---
    drivers/ide/pci/siimage.c | 303 +++++++++++++++++++++-------------------------
    1 file changed, 142 insertions(+), 161 deletions(-)

    Index: b/drivers/ide/pci/siimage.c
    ================================================== =================
    --- a/drivers/ide/pci/siimage.c
    +++ b/drivers/ide/pci/siimage.c
    @@ -2,7 +2,7 @@
    * Copyright (C) 2001-2002 Andre Hedrick
    * Copyright (C) 2003 Red Hat
    * Copyright (C) 2007 MontaVista Software, Inc.
    - * Copyright (C) 2007 Bartlomiej Zolnierkiewicz
    + * Copyright (C) 2007-2008 Bartlomiej Zolnierkiewicz
    *
    * May be copied or modified under the terms of the GNU General Public License
    *
    @@ -124,6 +124,54 @@ static inline unsigned long siimage_seld
    return base;
    }

    +static u8 sil_ioread8(struct pci_dev *dev, unsigned long addr)
    +{
    + u8 tmp = 0;
    +
    + if (pci_get_drvdata(dev))
    + tmp = readb((void __iomem *)addr);
    + else
    + pci_read_config_byte(dev, addr, &tmp);
    +
    + return tmp;
    +}
    +
    +static u16 sil_ioread16(struct pci_dev *dev, unsigned long addr)
    +{
    + u16 tmp = 0;
    +
    + if (pci_get_drvdata(dev))
    + tmp = readw((void __iomem *)addr);
    + else
    + pci_read_config_word(dev, addr, &tmp);
    +
    + return tmp;
    +}
    +
    +static void sil_iowrite8(struct pci_dev *dev, u8 val, unsigned long addr)
    +{
    + if (pci_get_drvdata(dev))
    + writeb(val, (void __iomem *)addr);
    + else
    + pci_write_config_byte(dev, addr, val);
    +}
    +
    +static void sil_iowrite16(struct pci_dev *dev, u16 val, unsigned long addr)
    +{
    + if (pci_get_drvdata(dev))
    + writew(val, (void __iomem *)addr);
    + else
    + pci_write_config_word(dev, addr, val);
    +}
    +
    +static void sil_iowrite32(struct pci_dev *dev, u32 val, unsigned long addr)
    +{
    + if (pci_get_drvdata(dev))
    + writel(val, (void __iomem *)addr);
    + else
    + pci_write_config_dword(dev, addr, val);
    +}
    +
    /**
    * sil_udma_filter - compute UDMA mask
    * @drive: IDE device
    @@ -139,12 +187,9 @@ static u8 sil_pata_udma_filter(ide_drive
    ide_hwif_t *hwif = drive->hwif;
    struct pci_dev *dev = to_pci_dev(hwif->dev);
    unsigned long base = (unsigned long) hwif->hwif_data;
    - u8 mask = 0, scsc = 0;
    + u8 mask = 0, scsc;

    - if (hwif->mmio)
    - scsc = hwif->INB(base + 0x4A);
    - else
    - pci_read_config_byte(dev, 0x8A, &scsc);
    + scsc = sil_ioread8(dev, base + (hwif->mmio ? 0x4A : 0x8A));

    if ((scsc & 0x30) == 0x10) /* 133 */
    mask = ATA_UDMA6;
    @@ -179,6 +224,7 @@ static void sil_set_pio_mode(ide_drive_t
    const u16 data_speed[] = { 0x328a, 0x2283, 0x1104, 0x10c3, 0x10c1 };

    ide_hwif_t *hwif = HWIF(drive);
    + struct pci_dev *dev = to_pci_dev(hwif->dev);
    ide_drive_t *pair = ide_get_paired_drive(drive);
    u32 speedt = 0;
    u16 speedp = 0;
    @@ -203,36 +249,20 @@ static void sil_set_pio_mode(ide_drive_t
    speedp = data_speed[pio];
    speedt = tf_speed[tf_pio];

    - if (hwif->mmio) {
    - hwif->OUTW(speedp, addr);
    - hwif->OUTW(speedt, tfaddr);
    - /* Now set up IORDY */
    - if (pio > 2)
    - hwif->OUTW(hwif->INW(tfaddr-2)|0x200, tfaddr-2);
    - else
    - hwif->OUTW(hwif->INW(tfaddr-2)&~0x200, tfaddr-2);
    -
    - mode = hwif->INB(base + addr_mask);
    - mode &= ~(unit ? 0x30 : 0x03);
    - mode |= (unit ? 0x10 : 0x01);
    - hwif->OUTB(mode, base + addr_mask);
    - } else {
    - struct pci_dev *dev = to_pci_dev(hwif->dev);
    + sil_iowrite16(dev, speedp, addr);
    + sil_iowrite16(dev, speedt, tfaddr);

    - pci_write_config_word(dev, addr, speedp);
    - pci_write_config_word(dev, tfaddr, speedt);
    - pci_read_config_word(dev, tfaddr - 2, &speedp);
    - speedp &= ~0x200;
    - /* Set IORDY for mode 3 or 4 */
    - if (pio > 2)
    - speedp |= 0x200;
    - pci_write_config_word(dev, tfaddr - 2, speedp);
    -
    - pci_read_config_byte(dev, addr_mask, &mode);
    - mode &= ~(unit ? 0x30 : 0x03);
    - mode |= (unit ? 0x10 : 0x01);
    - pci_write_config_byte(dev, addr_mask, mode);
    - }
    + /* now set up IORDY */
    + speedp = sil_ioread16(dev, tfaddr - 2);
    + speedp &= ~0x200;
    + if (pio > 2)
    + speedp |= 0x200;
    + sil_iowrite16(dev, speedp, tfaddr - 2);
    +
    + mode = sil_ioread8(dev, base + addr_mask);
    + mode &= ~(unit ? 0x30 : 0x03);
    + mode |= (unit ? 0x10 : 0x01);
    + sil_iowrite8(dev, mode, base + addr_mask);
    }

    /**
    @@ -261,17 +291,10 @@ static void sil_set_dma_mode(ide_drive_t
    unsigned long ma = siimage_seldev(drive, 0x08);
    unsigned long ua = siimage_seldev(drive, 0x0C);

    - if (hwif->mmio) {
    - scsc = hwif->INB(base + 0x4A);
    - mode = hwif->INB(base + addr_mask);
    - multi = hwif->INW(ma);
    - ultra = hwif->INW(ua);
    - } else {
    - pci_read_config_byte(dev, 0x8A, &scsc);
    - pci_read_config_byte(dev, addr_mask, &mode);
    - pci_read_config_word(dev, ma, &multi);
    - pci_read_config_word(dev, ua, &ultra);
    - }
    + scsc = sil_ioread8(dev, base + (hwif->mmio ? 0x4A : 0x8A));
    + mode = sil_ioread8(dev, base + addr_mask);
    + multi = sil_ioread16(dev, ma);
    + ultra = sil_ioread16(dev, ua);

    mode &= ~((unit) ? 0x30 : 0x03);
    ultra &= ~0x3F;
    @@ -289,15 +312,9 @@ static void sil_set_dma_mode(ide_drive_t
    mode |= (unit ? 0x20 : 0x02);
    }

    - if (hwif->mmio) {
    - hwif->OUTB(mode, base + addr_mask);
    - hwif->OUTW(multi, ma);
    - hwif->OUTW(ultra, ua);
    - } else {
    - pci_write_config_byte(dev, addr_mask, mode);
    - pci_write_config_word(dev, ma, multi);
    - pci_write_config_word(dev, ua, ultra);
    - }
    + sil_iowrite8(dev, mode, base + addr_mask);
    + sil_iowrite16(dev, multi, ma);
    + sil_iowrite16(dev, ultra, ua);
    }

    /* returns 1 if dma irq issued, 0 otherwise */
    @@ -460,26 +477,21 @@ static unsigned int setup_mmio_siimage (
    {
    resource_size_t bar5 = pci_resource_start(dev, 5);
    unsigned long barsize = pci_resource_len(dev, 5);
    - u8 tmpbyte = 0;
    void __iomem *ioaddr;
    - u32 tmp, irq_mask;

    /*
    * Drop back to PIO if we can't map the mmio. Some
    * systems seem to get terminally confused in the PCI
    * spaces.
    */
    -
    - if(!request_mem_region(bar5, barsize, name))
    - {
    + if (!request_mem_region(bar5, barsize, name)) {
    printk(KERN_WARNING "siimage: IDE controller MMIO ports not available.\n");
    return 0;
    }
    -
    +
    ioaddr = ioremap(bar5, barsize);

    - if (ioaddr == NULL)
    - {
    + if (ioaddr == NULL) {
    release_mem_region(bar5, barsize);
    return 0;
    }
    @@ -487,62 +499,6 @@ static unsigned int setup_mmio_siimage (
    pci_set_master(dev);
    pci_set_drvdata(dev, (void *) ioaddr);

    - if (pdev_is_sata(dev)) {
    - /* make sure IDE0/1 interrupts are not masked */
    - irq_mask = (1 << 22) | (1 << 23);
    - tmp = readl(ioaddr + 0x48);
    - if (tmp & irq_mask) {
    - tmp &= ~irq_mask;
    - writel(tmp, ioaddr + 0x48);
    - readl(ioaddr + 0x48); /* flush */
    - }
    - writel(0, ioaddr + 0x148);
    - writel(0, ioaddr + 0x1C8);
    - }
    -
    - writeb(0, ioaddr + 0xB4);
    - writeb(0, ioaddr + 0xF4);
    - tmpbyte = readb(ioaddr + 0x4A);
    -
    - switch(tmpbyte & 0x30) {
    - case 0x00:
    - /* In 100 MHz clocking, try and switch to 133 */
    - writeb(tmpbyte|0x10, ioaddr + 0x4A);
    - break;
    - case 0x10:
    - /* On 133Mhz clocking */
    - break;
    - case 0x20:
    - /* On PCIx2 clocking */
    - break;
    - case 0x30:
    - /* Clocking is disabled */
    - /* 133 clock attempt to force it on */
    - writeb(tmpbyte & ~0x20, ioaddr + 0x4A);
    - break;
    - }
    -
    - tmpbyte = readb(ioaddr + 0x4A);
    -
    - writeb( 0x72, ioaddr + 0xA1);
    - writew( 0x328A, ioaddr + 0xA2);
    - writel(0x62DD62DD, ioaddr + 0xA4);
    - writel(0x43924392, ioaddr + 0xA8);
    - writel(0x40094009, ioaddr + 0xAC);
    - writeb( 0x72, ioaddr + 0xE1);
    - writew( 0x328A, ioaddr + 0xE2);
    - writel(0x62DD62DD, ioaddr + 0xE4);
    - writel(0x43924392, ioaddr + 0xE8);
    - writel(0x40094009, ioaddr + 0xEC);
    -
    - if (pdev_is_sata(dev)) {
    - writel(0xFFFF0000, ioaddr + 0x108);
    - writel(0xFFFF0000, ioaddr + 0x188);
    - writel(0x00680000, ioaddr + 0x148);
    - writel(0x00680000, ioaddr + 0x1C8);
    - }
    -
    - proc_reports_siimage(dev, (tmpbyte>>4), name);
    return 1;
    }

    @@ -557,50 +513,80 @@ static unsigned int setup_mmio_siimage (

    static unsigned int __devinit init_chipset_siimage(struct pci_dev *dev, const char *name)
    {
    - u8 rev = dev->revision, tmpbyte = 0, BA5_EN = 0;
    + unsigned long base, scsc_addr;
    + void __iomem *ioaddr = NULL;
    + u8 rev = dev->revision, tmp = 0, BA5_EN = 0;

    pci_write_config_byte(dev, PCI_CACHE_LINE_SIZE, rev ? 1 : 255);

    pci_read_config_byte(dev, 0x8A, &BA5_EN);
    - if ((BA5_EN & 0x01) || (pci_resource_start(dev, 5))) {
    - if (setup_mmio_siimage(dev, name)) {
    - return 0;
    +
    + if ((BA5_EN & 0x01) || pci_resource_start(dev, 5)) {
    + if (setup_mmio_siimage(dev, name))
    + ioaddr = pci_get_drvdata(dev);
    + }
    +
    + base = (unsigned long)ioaddr;
    +
    + if (ioaddr && pdev_is_sata(dev)) {
    + u32 tmp32, irq_mask;
    +
    + /* make sure IDE0/1 interrupts are not masked */
    + irq_mask = (1 << 22) | (1 << 23);
    + tmp32 = readl(ioaddr + 0x48);
    + if (tmp32 & irq_mask) {
    + tmp32 &= ~irq_mask;
    + writel(tmp32, ioaddr + 0x48);
    + readl(ioaddr + 0x48); /* flush */
    }
    + writel(0, ioaddr + 0x148);
    + writel(0, ioaddr + 0x1C8);
    + }
    +
    + sil_iowrite8(dev, 0, base ? (base + 0xB4) : 0x80);
    + sil_iowrite8(dev, 0, base ? (base + 0xF4) : 0x84);
    +
    + scsc_addr = base ? (base + 0x4A) : 0x8A;
    + tmp = sil_ioread8(dev, scsc_addr);
    +
    + switch (tmp & 0x30) {
    + case 0x00:
    + /* On 100MHz clocking, try and switch to 133MHz */
    + sil_iowrite8(dev, tmp | 0x10, scsc_addr);
    + break;
    + case 0x30:
    + /* Clocking is disabled, attempt to force 133MHz clocking. */
    + sil_iowrite8(dev, tmp & ~0x20, scsc_addr);
    + case 0x10:
    + /* On 133Mhz clocking. */
    + break;
    + case 0x20:
    + /* On PCIx2 clocking. */
    + break;
    + }
    +
    + tmp = sil_ioread8(dev, scsc_addr);
    +
    + sil_iowrite8(dev, 0x72, base + 0xA1);
    + sil_iowrite16(dev, 0x328A, base + 0xA2);
    + sil_iowrite32(dev, 0x62DD62DD, base + 0xA4);
    + sil_iowrite32(dev, 0x43924392, base + 0xA8);
    + sil_iowrite32(dev, 0x40094009, base + 0xAC);
    + sil_iowrite8(dev, 0x72, base ? (base + 0xE1) : 0xB1);
    + sil_iowrite16(dev, 0x328A, base ? (base + 0xE2) : 0xB2);
    + sil_iowrite32(dev, 0x62DD62DD, base ? (base + 0xE4) : 0xB4);
    + sil_iowrite32(dev, 0x43924392, base ? (base + 0xE8) : 0xB8);
    + sil_iowrite32(dev, 0x40094009, base ? (base + 0xEC) : 0xBC);
    +
    + if (base && pdev_is_sata(dev)) {
    + writel(0xFFFF0000, ioaddr + 0x108);
    + writel(0xFFFF0000, ioaddr + 0x188);
    + writel(0x00680000, ioaddr + 0x148);
    + writel(0x00680000, ioaddr + 0x1C8);
    }

    - pci_write_config_byte(dev, 0x80, 0x00);
    - pci_write_config_byte(dev, 0x84, 0x00);
    - pci_read_config_byte(dev, 0x8A, &tmpbyte);
    - switch(tmpbyte & 0x30) {
    - case 0x00:
    - /* 133 clock attempt to force it on */
    - pci_write_config_byte(dev, 0x8A, tmpbyte|0x10);
    - case 0x30:
    - /* if clocking is disabled */
    - /* 133 clock attempt to force it on */
    - pci_write_config_byte(dev, 0x8A, tmpbyte & ~0x20);
    - case 0x10:
    - /* 133 already */
    - break;
    - case 0x20:
    - /* BIOS set PCI x2 clocking */
    - break;
    - }
    -
    - pci_read_config_byte(dev, 0x8A, &tmpbyte);
    -
    - pci_write_config_byte(dev, 0xA1, 0x72);
    - pci_write_config_word(dev, 0xA2, 0x328A);
    - pci_write_config_dword(dev, 0xA4, 0x62DD62DD);
    - pci_write_config_dword(dev, 0xA8, 0x43924392);
    - pci_write_config_dword(dev, 0xAC, 0x40094009);
    - pci_write_config_byte(dev, 0xB1, 0x72);
    - pci_write_config_word(dev, 0xB2, 0x328A);
    - pci_write_config_dword(dev, 0xB4, 0x62DD62DD);
    - pci_write_config_dword(dev, 0xB8, 0x43924392);
    - pci_write_config_dword(dev, 0xBC, 0x40094009);
    + proc_reports_siimage(dev, tmp >> 4, name);

    - proc_reports_siimage(dev, (tmpbyte>>4), name);
    return 0;
    }

    @@ -752,12 +738,7 @@ static u8 __devinit sil_cable_detect(ide
    {
    struct pci_dev *dev = to_pci_dev(hwif->dev);
    unsigned long addr = siimage_selreg(hwif, 0);
    - u8 ata66 = 0;
    -
    - if (pci_get_drvdata(dev) == NULL)
    - pci_read_config_byte(dev, addr, &ata66);
    - else
    - ata66 = hwif->INB(addr);
    + u8 ata66 = sil_ioread8(dev, addr);

    return (ata66 & 0x01) ? ATA_CBL_PATA80 : ATA_CBL_PATA40;
    }
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH 2/3] siimage: add sil_* I/O ops

    Bartlomiej Zolnierkiewicz wrote:

    > Add sil_iowrite{8,16,32}() and sil_ioread{8,16}() helpers, then use them to
    > merge code accessing configuration registers through PCI and MMIO together.


    Sigh, my coding style cleanup patch heavily clashed with this one, so I had
    to recast it...

    > [ because of this SATA initialization bits from setup_mmio_siimage() are
    > moved to init_chipset_siimage() ]


    > This also cuts code size a bit:


    > text data bss dec hex filename
    > 4437 164 0 4601 11f9 drivers/ide/pci/siimage.o.before
    > 3979 164 0 4143 102f drivers/ide/pci/siimage.o.after


    Good work. :-)

    > While at it:


    > * Use I/O ops directly instead of using ->IN{B,W} and ->OUT{B,W}.


    > * Fixup CodingStyle in setup_mmio_siimage().


    > * Rename 'tmpbyte' variable to 'tmp' in init_chipset_siimage().


    > There should be no functional changes caused by this patch.


    > Signed-off-by: Bartlomiej Zolnierkiewicz


    Acked-by: Sergei Shtylyov

    > Index: b/drivers/ide/pci/siimage.c
    > ================================================== =================
    > --- a/drivers/ide/pci/siimage.c
    > +++ b/drivers/ide/pci/siimage.c
    > @@ -2,7 +2,7 @@
    > * Copyright (C) 2001-2002 Andre Hedrick
    > * Copyright (C) 2003 Red Hat
    > * Copyright (C) 2007 MontaVista Software, Inc.
    > - * Copyright (C) 2007 Bartlomiej Zolnierkiewicz
    > + * Copyright (C) 2007-2008 Bartlomiej Zolnierkiewicz
    > *
    > * May be copied or modified under the terms of the GNU General Public License
    > *
    > @@ -124,6 +124,54 @@ static inline unsigned long siimage_seld
    > return base;
    > }
    >
    > +static u8 sil_ioread8(struct pci_dev *dev, unsigned long addr)
    > +{
    > + u8 tmp = 0;
    > +
    > + if (pci_get_drvdata(dev))
    > + tmp = readb((void __iomem *)addr);
    > + else
    > + pci_read_config_byte(dev, addr, &tmp);
    > +
    > + return tmp;
    > +}
    > +
    > +static u16 sil_ioread16(struct pci_dev *dev, unsigned long addr)
    > +{
    > + u16 tmp = 0;
    > +
    > + if (pci_get_drvdata(dev))
    > + tmp = readw((void __iomem *)addr);
    > + else
    > + pci_read_config_word(dev, addr, &tmp);
    > +
    > + return tmp;
    > +}
    > +
    > +static void sil_iowrite8(struct pci_dev *dev, u8 val, unsigned long addr)
    > +{
    > + if (pci_get_drvdata(dev))
    > + writeb(val, (void __iomem *)addr);
    > + else
    > + pci_write_config_byte(dev, addr, val);
    > +}
    > +
    > +static void sil_iowrite16(struct pci_dev *dev, u16 val, unsigned long addr)
    > +{
    > + if (pci_get_drvdata(dev))
    > + writew(val, (void __iomem *)addr);
    > + else
    > + pci_write_config_word(dev, addr, val);
    > +}
    > +
    > +static void sil_iowrite32(struct pci_dev *dev, u32 val, unsigned long addr)
    > +{
    > + if (pci_get_drvdata(dev))
    > + writel(val, (void __iomem *)addr);
    > + else
    > + pci_write_config_dword(dev, addr, val);
    > +}
    > +


    I think this could be further imporoved -- since we have to call
    pci_get_drvdata() in the accessors anyway, we could have used it to get the
    MMIO base right there, and thus be freed from the necessity to add it to the
    MMIO offset in the callers and also most probably from having it copied to
    hwif->hwif_data.

    > @@ -203,36 +249,20 @@ static void sil_set_pio_mode(ide_drive_t

    [...]
    > + mode |= (unit ? 0x10 : 0x01);


    Useless parens -- I'm getting rid of them in my patch anyway...

    > @@ -557,50 +513,80 @@ static unsigned int setup_mmio_siimage (

    [...]
    > + sil_iowrite8(dev, 0x72, base + 0xA1);
    > + sil_iowrite16(dev, 0x328A, base + 0xA2);
    > + sil_iowrite32(dev, 0x62DD62DD, base + 0xA4);
    > + sil_iowrite32(dev, 0x43924392, base + 0xA8);
    > + sil_iowrite32(dev, 0x40094009, base + 0xAC);
    > + sil_iowrite8(dev, 0x72, base ? (base + 0xE1) : 0xB1);
    > + sil_iowrite16(dev, 0x328A, base ? (base + 0xE2) : 0xB2);
    > + sil_iowrite32(dev, 0x62DD62DD, base ? (base + 0xE4) : 0xB4);
    > + sil_iowrite32(dev, 0x43924392, base ? (base + 0xE8) : 0xB8);
    > + sil_iowrite32(dev, 0x40094009, base ? (base + 0xEC) : 0xBC);
    > +


    Sigh, I was going to send a patch getting rid of these writes altogether
    last year -- there should be no point in setting PIO/DMA/UDMA timings here.
    Maybe I'll submit it...

    MBR, Sergei
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH 2/3] siimage: add sil_* I/O ops

    On Saturday 19 April 2008, Sergei Shtylyov wrote:

    [...]

    > > @@ -124,6 +124,54 @@ static inline unsigned long siimage_seld
    > > return base;
    > > }
    > >
    > > +static u8 sil_ioread8(struct pci_dev *dev, unsigned long addr)
    > > +{
    > > + u8 tmp = 0;
    > > +
    > > + if (pci_get_drvdata(dev))
    > > + tmp = readb((void __iomem *)addr);
    > > + else
    > > + pci_read_config_byte(dev, addr, &tmp);
    > > +
    > > + return tmp;
    > > +}
    > > +
    > > +static u16 sil_ioread16(struct pci_dev *dev, unsigned long addr)
    > > +{
    > > + u16 tmp = 0;
    > > +
    > > + if (pci_get_drvdata(dev))
    > > + tmp = readw((void __iomem *)addr);
    > > + else
    > > + pci_read_config_word(dev, addr, &tmp);
    > > +
    > > + return tmp;
    > > +}
    > > +
    > > +static void sil_iowrite8(struct pci_dev *dev, u8 val, unsigned long addr)
    > > +{
    > > + if (pci_get_drvdata(dev))
    > > + writeb(val, (void __iomem *)addr);
    > > + else
    > > + pci_write_config_byte(dev, addr, val);
    > > +}
    > > +
    > > +static void sil_iowrite16(struct pci_dev *dev, u16 val, unsigned long addr)
    > > +{
    > > + if (pci_get_drvdata(dev))
    > > + writew(val, (void __iomem *)addr);
    > > + else
    > > + pci_write_config_word(dev, addr, val);
    > > +}
    > > +
    > > +static void sil_iowrite32(struct pci_dev *dev, u32 val, unsigned long addr)
    > > +{
    > > + if (pci_get_drvdata(dev))
    > > + writel(val, (void __iomem *)addr);
    > > + else
    > > + pci_write_config_dword(dev, addr, val);
    > > +}
    > > +

    >
    > I think this could be further imporoved -- since we have to call
    > pci_get_drvdata() in the accessors anyway, we could have used it to get the
    > MMIO base right there, and thus be freed from the necessity to add it to the
    > MMIO offset in the callers and also most probably from having it copied to
    > hwif->hwif_data.


    [...]

    Or maybe we can remove sil_io* and always use PCI access (on the second
    look none of sil_io* users seems to be performance critical)...

    > > @@ -557,50 +513,80 @@ static unsigned int setup_mmio_siimage (

    > [...]
    > > + sil_iowrite8(dev, 0x72, base + 0xA1);
    > > + sil_iowrite16(dev, 0x328A, base + 0xA2);
    > > + sil_iowrite32(dev, 0x62DD62DD, base + 0xA4);
    > > + sil_iowrite32(dev, 0x43924392, base + 0xA8);
    > > + sil_iowrite32(dev, 0x40094009, base + 0xAC);
    > > + sil_iowrite8(dev, 0x72, base ? (base + 0xE1) : 0xB1);
    > > + sil_iowrite16(dev, 0x328A, base ? (base + 0xE2) : 0xB2);
    > > + sil_iowrite32(dev, 0x62DD62DD, base ? (base + 0xE4) : 0xB4);
    > > + sil_iowrite32(dev, 0x43924392, base ? (base + 0xE8) : 0xB8);
    > > + sil_iowrite32(dev, 0x40094009, base ? (base + 0xEC) : 0xBC);
    > > +

    >
    > Sigh, I was going to send a patch getting rid of these writes altogether
    > last year -- there should be no point in setting PIO/DMA/UDMA timings here.


    Heh, I know the feeling damn too well - I still have few low-prio patches
    back from *2004* which need to be refreshed and pushed out...

    > Maybe I'll submit it...


    Just do it.

    Thanks,
    Bart
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH 2/3] siimage: add sil_* I/O ops

    BTW. siimage crashes on boot here due to dma_ops being mostly full of
    NULL... This is with current linus.

    Ben.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread