On Mon, Jul 12, 2010 at 10:01:17PM -0400, Jason Chagas wrote:
> 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);
> };
Ick. I'm pretty sure this came up a few years ago and was shot down
then. Have you checked the archives?
> 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));
Doesn't this imply that you should just be changing your arch's readl()
function to handle this type of thing properly? Do you really want to
add this type of logic to _EVERY_ device driver out there (hint, no.)
I think you can do this today with the arch-specific hooks provided to
you, have you looked into that?
thanks,
greg k-h
--
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
