On Tue, Jun 11, 2019 at 10:12:52PM +0800, John Garry wrote:
Another thought here:
> if (addr < MMIO_UPPER_LIMIT) { \
> ret = read##bw(PCI_IOBASE + addr); \
> } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) { \
> - struct logic_pio_hwaddr *entry = find_io_range(addr); \
> + struct logic_pio_hwaddr *range = find_io_range(addr); \
> + size_t sz = sizeof(type); \
> \
> - if (entry && entry->ops) \
> - ret = entry->ops->in(entry->hostdata, \
> - addr, sizeof(type)); \
> + if (range && range->ops) \
> + ret = range->ops->in(range->hostdata, addr, sz);\
> else \
> WARN_ON_ONCE(1); \
Could this be simplified a little by requiring callers to set
range->ops for LOGIC_PIO_INDIRECT ranges *before* calling
logic_pio_register_range()? E.g.,
hisi_lpc_probe(...)
{
range = devm_kzalloc(...);
range->flags = LOGIC_PIO_INDIRECT;
range->ops = &hisi_lpc_ops;
logic_pio_register_range(range);
...
and
logic_pio_register_range(struct logic_pio_hwaddr *new_range)
{
if (new_range->flags == LOGIC_PIO_INDIRECT && !new_range->ops)
return -EINVAL;
...
Then maybe you wouldn't need to check range->ops in the accessors.
Bjorn
