RFC: extending struct pci_ops to include HW specific read and write functions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



I would like to kindly request your comments on a proposal to expand
'struct pci_ops' to include hardware specific read/write functions as
follows:

===========
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 7cb0084..0ba739a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -467,6 +467,15 @@ static inline bool pci_dev_msi_enabled(struct
pci_dev *pci_dev) { return false;
struct pci_ops {
int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int
size, u32 *val);
int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int
size, u32 val);
+ u8 (*hw_readb)(u32 addr);
+ u16 (*hw_readw)(u32 addr);
+ u32 (*hw_readl)(u32 addr);
+ void (*hw_writeb)(u8 val, u32 addr);
+ void (*hw_writew)(u16 val, u32 addr);
+ void (*hw_writel)(u32 val, u32 addr);
+ void (*hw_memcpy_fromio)(void *to, const volatile void __iomem
*from, size_t count);
+ void (*hw_memcpy_toio)(volatile void __iomem *to, const void *from,
size_t count);
+ void (*hw_memset_io)(volatile void __iomem *dst, int c, size_t count);
};

/*
===========

This provides a hardware independent interface to access devices on
PCIe controllers that do not allow mapping PCIe memory onto CPU memory
space. For instance, unlike standard designs where the driver maps PCI
memory into virtual kernel space, the Marvell PXA168PXA PCIe
controller does not allow mapping of PCIe memory to CPU memory space.
It means that all driver data transfers must occur over DMA with phys
addresses.

For single byte/word/dword transactions, PXA168/PCIe provides a number
of 'PIO' channels (4, actually). Such 'PIO' channels are, in turn, hw
DMA interface abstractions for single 8/16/32bit access. Thus my
proposal to extend 'struct pci_ops' with generic
hw_read*()/hw_write*() functions that would abstract all the
idiosyncrasies of accessing the 'PIO' interface. This would also
provide an easy software migration path for future PCIe interface
designs awhile still maintain backwards compatibility with existing
software. Please refer to appended example of how the Yukon network
driver (~/devices/net/sky2.c) would change to take advantage of these
proposed extensions.

Comments welcome!

Thanks,

Jason


===================
EXAMPLE:

diff --git a/drivers/net/sky2.h b/drivers/net/sky2.h
index 084eff2..3769e1c 100644
--- a/drivers/net/sky2.h
+++ b/drivers/net/sky2.h
@@ -4,6 +4,8 @@
#ifndef _SKY2_H
#define _SKY2_H

+#define __hw_mem_pci(a)      ((u32)a)
+
#define ETH_JUMBO_MTU 9000 /* Maximum MTU supported */

/* PCI config registers */
@@ -2301,32 +2303,70 @@ static inline int sky2_is_copper(const struct
sky2_hw *hw)
/* Register accessor for memory mapped device */
static inline u32 sky2_read32(const struct sky2_hw *hw, unsigned reg)
{
- return readl(hw->regs + reg);
+ struct pci_ops *ops = hw->pdev->bus->ops;
+
+ return (ops->hw_readl)?(ops->hw_readl(__hw_mem_pci(hw->regs + reg))):
+ (readl(hw->regs + reg));
}

static inline u16 sky2_read16(const struct sky2_hw *hw, unsigned reg)
{
- return readw(hw->regs + reg);
+ struct pci_ops *ops = hw->pdev->bus->ops;
+
+ return (ops->hw_readw)?(ops->hw_readw(__hw_mem_pci(hw->regs + reg))):
+ (readw(hw->regs + reg));
}

static inline u8 sky2_read8(const struct sky2_hw *hw, unsigned reg)
{
- return readb(hw->regs + reg);
+ struct pci_ops *ops = hw->pdev->bus->ops;
+
+ return (ops->hw_readb)?(ops->hw_readb(__hw_mem_pci(hw->regs + reg))):
+ (readb(hw->regs + reg));
}

static inline void sky2_write32(const struct sky2_hw *hw, unsigned reg, u32 val)
{
- writel(val, hw->regs + reg);
+ struct pci_ops *ops = hw->pdev->bus->ops;
+
+ (ops->hw_writel)?(ops->hw_writel(val, __hw_mem_pci(hw->regs + reg))):
+ (writel(val, hw->regs + reg));
}

static inline void sky2_write16(const struct sky2_hw *hw, unsigned reg, u16 val)
{
- writew(val, hw->regs + reg);
+ struct pci_ops *ops = hw->pdev->bus->ops;
+
+ (ops->hw_writew)?(ops->hw_writew(val, __hw_mem_pci(hw->regs + reg))):
+ (writew(val, hw->regs + reg));
}

static inline void sky2_write8(const struct sky2_hw *hw, unsigned reg, u8 val)
{
- writeb(val, hw->regs + reg);
+ struct pci_ops *ops = hw->pdev->bus->ops;
+
+ (ops->hw_writeb)?(ops->hw_writeb(val, __hw_mem_pci(hw->regs + reg))):
+ (writeb(val, hw->regs + reg));
+}
+
+static inline void
+sky2_memcpy_fromio(const struct sky2_hw *hw, void *to,
+   const volatile void __iomem *from, size_t count)
+{
+ struct pci_ops *ops = hw->pdev->bus->ops;
+
+ (ops->hw_memcpy_fromio)?(ops->hw_memcpy_fromio(to, from, count)):
+ (memcpy_fromio(to, from, count));
+}
+
+static inline void
+sky2_memcpy_toio(const struct sky2_hw *hw, volatile void __iomem *to,
+ const void *from, size_t count)
+{
+ struct pci_ops *ops = hw->pdev->bus->ops;
+
+ (ops->hw_memcpy_toio)?(ops->hw_memcpy_toio(to, from, count)):
+ (memcpy_toio(to, from, count));
}

/* Yukon PHY related registers */

======

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 7985165..2519693 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -152,6 +152,23 @@ static const u32 portirq_msk[] = { Y2_IS_PORT_1,
Y2_IS_PORT_2 };

static void sky2_set_multicast(struct net_device *dev);

+static void *sky2_map_regs_base(struct pci_dev *pdev)
+{
+#if defined(CONFIG_PCI_NOCPU_PHYSICAL_MAPPING)
+ /* We shouldn't map an address that only the controller understands */
+ return (void *) pci_resource_start(pdev, 0);
+#else
+ return ioremap_nocache(pci_resource_start(pdev, 0), 0x4000);
+#endif
+}
+
+static void sky2_unmap_regs_base(void *addr)
+{
+#if !defined(CONFIG_PCI_NOCPU_PHYSICAL_MAPPING)
+ iounmap(addr);
+#endif
+}
+
/* Access to PHY via serial interconnect */
static int gm_phy_write(struct sky2_hw *hw, unsigned port, u16 reg, u16 val)
{
@@ -776,8 +793,8 @@ static void sky2_wol_init(struct sky2_port *sky2)
   GM_GPCR_DUP_FULL|GM_GPCR_FC_RX_DIS|GM_GPCR_AU_FCT_DIS);

/* Set WOL address */
- memcpy_toio(hw->regs + WOL_REGS(port, WOL_MAC_ADDR),
-    sky2->netdev->dev_addr, ETH_ALEN);
+ sky2_memcpy_toio(hw, hw->regs + WOL_REGS(port, WOL_MAC_ADDR),
+ sky2->netdev->dev_addr, ETH_ALEN);

/* Turn on appropriate WOL control bits */
sky2_write16(hw, WOL_REGS(port, WOL_CTRL_STAT), WOL_CTL_CLEAR_RESULT);
@@ -3671,10 +3688,10 @@ static int sky2_set_mac_address(struct
net_device *dev, void *p)
return -EADDRNOTAVAIL;

memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
- memcpy_toio(hw->regs + B2_MAC_1 + port * 8,
-    dev->dev_addr, ETH_ALEN);
- memcpy_toio(hw->regs + B2_MAC_2 + port * 8,
-    dev->dev_addr, ETH_ALEN);
+ sky2_memcpy_toio(hw, hw->regs + B2_MAC_1 + port * 8,
+ dev->dev_addr, ETH_ALEN);
+ sky2_memcpy_toio(hw, hw->regs + B2_MAC_2 + port * 8,
+ dev->dev_addr, ETH_ALEN);

/* virtual address for data */
gma_set_addr(hw, port, GM_SRC_ADDR_2L, dev->dev_addr);
@@ -4044,9 +4061,10 @@ static void sky2_get_regs(struct net_device
*dev, struct ethtool_regs *regs,
for (b = 0; b < 128; b++) {
/* skip poisonous diagnostic ram region in block 3 */
if (b == 3)
- memcpy_fromio(p + 0x10, io + 0x10, 128 - 0x10);
+ sky2_memcpy_fromio(sky2->hw, p + 0x10, io + 0x10,
+   128 - 0x10);
else if (sky2_reg_access_ok(sky2->hw, b))
- memcpy_fromio(p, io, 128);
+ sky2_memcpy_fromio(sky2->hw, p, io, 128);
else
memset(p, 0, 128);

@@ -4602,7 +4620,8 @@ static __devinit struct net_device
*sky2_init_netdev(struct sky2_hw *hw,
#endif

/* read the mac address */
- memcpy_fromio(dev->dev_addr, hw->regs + B2_MAC_1 + port * 8, ETH_ALEN);
+ sky2_memcpy_fromio(sky2->hw, dev->dev_addr,
+   hw->regs + B2_MAC_1 + port * 8, ETH_ALEN);
memcpy(dev->perm_addr, dev->dev_addr, dev->addr_len);

return dev;
@@ -4778,7 +4797,8 @@ static int __devinit sky2_probe(struct pci_dev *pdev,
hw->pdev = pdev;
sprintf(hw->irq_name, DRV_NAME "@pci:%s", pci_name(pdev));

- hw->regs = ioremap_nocache(pci_resource_start(pdev, 0), 0x4000);
+ hw->regs = sky2_map_regs_base(pdev);
+
if (!hw->regs) {
dev_err(&pdev->dev, "cannot map device registers\n");
goto err_out_free_hw;
@@ -4873,7 +4893,7 @@ err_out_free_pci:
err_out_reset:
sky2_write8(hw, B0_CTST, CS_RST_SET);
err_out_iounmap:
- iounmap(hw->regs);
+ sky2_unmap_regs_base(hw->regs);
err_out_free_hw:
kfree(hw);
err_out_free_regions:
@@ -4917,7 +4937,7 @@ static void __devexit sky2_remove(struct pci_dev *pdev)
for (i = hw->ports-1; i >= 0; --i)
free_netdev(hw->dev[i]);

- iounmap(hw->regs);
+ sky2_unmap_regs_base(hw->regs);
kfree(hw);

pci_set_drvdata(pdev, NULL);
===================
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux