[U-Boot] [PATCH 36/51] drivers: Add ihs_axi driver
Mario Six
mario.six at gdsys.cc
Tue Jul 25 12:27:32 UTC 2017
Hi Simon,
On Wed, Jul 19, 2017 at 11:06 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Mario,
>
> On 14 July 2017 at 05:55, Mario Six <mario.six at gdsys.cc> wrote:
>> Add a driver for the IHS AXI bus on IHS FPGAs.
>
> Can we make this uclass more generic by using AXI instead of IHS_AXI?
>
This should work. I'll have to take a look at how to handle different bus
widths, but it should work in general.
>>
>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>> ---
>>
>> drivers/misc/Kconfig | 7 ++
>> drivers/misc/Makefile | 1 +
>> drivers/misc/ihs_axi.c | 208 +++++++++++++++++++++++++++++++++++++++++++++++++
>> include/dm/uclass-id.h | 1 +
>> include/ihs_axi.h | 69 ++++++++++++++++
>> 5 files changed, 286 insertions(+)
>> create mode 100644 drivers/misc/ihs_axi.c
>> create mode 100644 include/ihs_axi.h
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 8b59a444ce..c53f9f195e 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -202,4 +202,11 @@ config IHS_FPGA
>> depends on MISC
>> help
>> Support for IHS FPGA.
>> +
>> +config IHS_AXI
>> + bool "Enable IHS AXI driver"
>> + depends on MISC
>> + help
>> + Support for IHS AXI bus.
>
> More help? Spell out the abbreviations and point to more info.
>
OK, will add some more information in v2.
>> +
>> endmenu
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index d2e46fc7d6..a6c71acedd 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -53,3 +53,4 @@ obj-$(CONFIG_WINBOND_W83627) += winbond_w83627.o
>> obj-$(CONFIG_QFW) += qfw.o
>> obj-$(CONFIG_ROCKCHIP_EFUSE) += rockchip-efuse.o
>> obj-$(CONFIG_IHS_FPGA) += ihs_fpga.o gdsys_soc.o
>> +obj-$(CONFIG_IHS_AXI) += ihs_axi.o
>> diff --git a/drivers/misc/ihs_axi.c b/drivers/misc/ihs_axi.c
>> new file mode 100644
>> index 0000000000..07150e2262
>> --- /dev/null
>> +++ b/drivers/misc/ihs_axi.c
>> @@ -0,0 +1,208 @@
>> +/*
>> + * (C) Copyright 2016
>> + * Dirk Eibach, Guntermann & Drunck GmbH, dirk.eibach at gdsys.cc
>> + *
>> + * (C) Copyright 2017
>> + * Mario Six, Guntermann & Drunck GmbH, mario.six at gdsys.cc
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <dm/device-internal.h>
>> +#include <ihs_axi.h>
>> +#include <ihs_fpga.h>
>> +#include <gdsys_soc.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +enum {
>> + REG_INTERRUPT_STATUS = 0x00,
>> + REG_INTERRUPT_ENABLE_CONTROL = 0x02,
>> + REG_ADDRESS_LSB = 0x04,
>> + REG_ADDRESS_MSB = 0x06,
>> + REG_WRITE_DATA_LSB = 0x08,
>> + REG_WRITE_DATA_MSB = 0x0A,
>> + REG_READ_DATA_LSB = 0x0C,
>> + REG_READ_DATA_MSB = 0x0E,
>> +};
>> +
>> +struct ihs_axi_priv {
>> + fdt_addr_t addr;
>> +};
>> +
>> +enum {
>> + STATUS_EVENT_MASK = GENMASK(15, 12),
>> +
>> + STATUS_READ_COMPLETE_EVENT = BIT(15),
>> + STATUS_WRITE_COMPLETE_EVENT = BIT(14),
>> + STATUS_TIMEOUT_EVENT = BIT(13),
>> + STATUS_ERROR_EVENT = BIT(12),
>> + STATUS_AXI_INT = BIT(11),
>> + STATUS_READ_DATA_AVAILABLE = BIT(7),
>> + STATUS_BUSY = BIT(6),
>> + STATUS_INIT_DONE = BIT(5),
>> +};
>> +
>> +enum {
>> + CONTROL_EVENT_ENABLE_MASK = GENMASK(15, 11),
>> + CONTROL_CMD_MASK = GENMASK(3, 0),
>> +
>> + CONTROL_READ_COMPLETE_EVENT_ENABLE = BIT(15),
>> + CONTROL_WRITE_COMPLETE_EVENT_ENABLE = BIT(14),
>> + CONTROL_TIMEOUT_EVENT_ENABLE = BIT(13),
>> + CONTROL_ERROR_EVENT_ENABLE = BIT(12),
>> + CONTROL_AXI_INT_ENABLE = BIT(11),
>> +
>> + CONTROL_CMD_NOP = 0x0,
>> + CONTROL_CMD_WRITE = 0x8,
>> + CONTROL_CMD_WRITE_POST_INC = 0x9,
>> + CONTROL_CMD_READ = 0xa,
>> + CONTROL_CMD_READ_POST_INC = 0xb,
>> +
>> +};
>> +
>> +static int axi_transfer(struct udevice *bus, u32 address, u16 cmd,
>> + u16 complete_flag)
>> +{
>> + struct ihs_axi_priv *priv = dev_get_priv(bus);
>> + struct gdsys_soc_child_platdata *pplat = dev_get_parent_platdata(bus);
>> + u16 wait_mask = complete_flag | STATUS_TIMEOUT_EVENT |
>> + STATUS_ERROR_EVENT;
>> + u16 status;
>> + uint k;
>> +
>> + cmd &= CONTROL_CMD_MASK;
>> +
>> + fpga_out_le16(pplat->fpga, priv->addr + REG_ADDRESS_LSB,
>> + address & 0xffff);
>> + fpga_out_le16(pplat->fpga, priv->addr + REG_ADDRESS_MSB,
>> + (address >> 16) & 0xffff);
>> +
>> + fpga_out_le16(pplat->fpga, priv->addr + REG_INTERRUPT_STATUS,
>> + wait_mask);
>> + fpga_out_le16(pplat->fpga, priv->addr + REG_INTERRUPT_ENABLE_CONTROL,
>> + cmd);
>> +
>> + for (k = 10; k > 0; --k) {
>> + status = fpga_in_le16(pplat->fpga,
>> + priv->addr + REG_INTERRUPT_STATUS);
>> + if (status & wait_mask)
>> + break;
>> + udelay(1);
>> + }
>> +
>> + if (!k)
>> + status = fpga_in_le16(pplat->fpga,
>> + priv->addr + REG_INTERRUPT_STATUS);
>> +
>> + if (status & complete_flag)
>> + return 0;
>> +
>> + if (status & STATUS_ERROR_EVENT)
>> + return -EIO;
>> +
>> + return -ETIMEDOUT;
>> +}
>> +
>> +int axi_read(struct udevice *dev, u32 address, u32 *data)
>> +{
>> + struct ihs_axi_ops *ops = ihs_axi_get_ops(dev);
>> +
>> + return ops->read(dev, address, data);
>> +}
>> +
>> +int axi_write(struct udevice *dev, u32 address, u32 data)
>> +{
>> + struct ihs_axi_ops *ops = ihs_axi_get_ops(dev);
>> +
>> + return ops->write(dev, address, data);
>> +}
>> +
>> +/*
>> + * API
>> + */
>> +
>> +int ihs_axi_read(struct udevice *dev, u32 address, u32 *data)
>> +{
>> + struct ihs_axi_priv *priv = dev_get_priv(dev);
>> + struct gdsys_soc_child_platdata *pplat = dev_get_parent_platdata(dev);
>> + int res = 0;
>> + u16 data_lsb, data_msb;
>> +
>> + res = axi_transfer(dev, address, CONTROL_CMD_READ,
>> + STATUS_READ_COMPLETE_EVENT);
>> + if (res < 0)
>> + return res;
>> +
>> + data_lsb = fpga_in_le16(pplat->fpga,
>> + priv->addr + REG_READ_DATA_LSB);
>> + data_msb = fpga_in_le16(pplat->fpga,
>> + priv->addr + REG_READ_DATA_MSB);
>> +
>> + *data = (data_msb << 16) | data_lsb;
>> +
>> + return res;
>> +}
>> +
>> +int ihs_axi_write(struct udevice *dev, u32 address, u32 data)
>> +{
>> + struct ihs_axi_priv *priv = dev_get_priv(dev);
>> + struct gdsys_soc_child_platdata *pplat = dev_get_parent_platdata(dev);
>> + int res = 0;
>> +
>> + fpga_out_le16(pplat->fpga, priv->addr + REG_READ_DATA_LSB,
>> + data & 0xffff);
>> + fpga_out_le16(pplat->fpga, priv->addr + REG_READ_DATA_MSB,
>> + (data >> 16) & 0xffff);
>> +
>> + res = axi_transfer(dev, address, CONTROL_CMD_WRITE,
>> + STATUS_WRITE_COMPLETE_EVENT);
>> +
>> + return res;
>> +}
>> +
>> +UCLASS_DRIVER(ihs_axi) = {
>> + .id = UCLASS_IHS_AXI,
>> + .name = "ihs_axi",
>> + .post_bind = dm_scan_fdt_dev,
>> + .flags = DM_UC_FLAG_SEQ_ALIAS,
>> +};
>> +
>> +static const struct udevice_id ihs_axi_ids[] = {
>> + { .compatible = "gdsys,ihs_axi" },
>> + { /* sentinel */ }
>> +};
>> +
>> +static const struct ihs_axi_ops ihs_axi_ops = {
>> + .read = ihs_axi_read,
>> + .write = ihs_axi_write,
>> +};
>> +
>> +int ihs_axi_probe(struct udevice *dev)
>> +{
>> + struct ihs_axi_priv *priv = dev_get_priv(dev);
>> + struct udevice *child = NULL;
>> + int addr;
>> +
>> + addr = fdtdec_get_int(gd->fdt_blob, dev_of_offset(dev), "reg", -1);
>
> dev_read_...
>
Older code again :-) Will fix in v2.
>> +
>> + priv->addr = addr;
>> +
>> + for (device_find_first_child(dev, &child);
>> + child;
>> + device_find_next_child(&child))
>> + device_probe(child);
>
> Should not probe children. Instead when you probe a child this bus
> should be auto-probed by DM.
>
OK, that's fine for the device, since only the CON really uses the AXI
interface. Will fix in v2.
>> +
>> + return 0;
>> +}
>> +
>> +U_BOOT_DRIVER(ihs_axi_bus) = {
>> + .name = "ihs_axi_bus",
>> + .id = UCLASS_IHS_AXI,
>> + .of_match = ihs_axi_ids,
>> + .ops = &ihs_axi_ops,
>> + .priv_auto_alloc_size = sizeof(struct ihs_axi_priv),
>> + .probe = ihs_axi_probe,
>> +};
>> diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
>> index 8eee8534ab..1a24de10b4 100644
>> --- a/include/dm/uclass-id.h
>> +++ b/include/dm/uclass-id.h
>> @@ -42,6 +42,7 @@ enum uclass_id {
>> UCLASS_I2C_EEPROM, /* I2C EEPROM device */
>> UCLASS_I2C_GENERIC, /* Generic I2C device */
>> UCLASS_I2C_MUX, /* I2C multiplexer */
>> + UCLASS_IHS_AXI, /* gdsys IHS AXI bus */
>> UCLASS_IHS_FPGA, /* gdsys IHS FPGAs */
>> UCLASS_IRQ, /* Interrupt controller */
>> UCLASS_KEYBOARD, /* Keyboard input device */
>> diff --git a/include/ihs_axi.h b/include/ihs_axi.h
>> new file mode 100644
>> index 0000000000..8640d45a0a
>> --- /dev/null
>> +++ b/include/ihs_axi.h
>> @@ -0,0 +1,69 @@
>> +/*
>> + * (C) Copyright 2017
>> + * Mario Six, Guntermann & Drunck GmbH, mario.six at gdsys.cc
>> + *
>> + * SPDX-License-Identifier: GPL-2.0+
>> + */
>> +
>> +#ifndef _AXI_H_
>> +#define _AXI_H_
>> +
>> +/**
>> + * struct ihs_axi_ops - driver operations for IHS AXI uclass
>> + *
>> + * Drivers should support these operations unless otherwise noted. These
>> + * operations are intended to be used by uclass code, not directly from
>> + * other code.
>> + */
>> +struct ihs_axi_ops {
>> + /**
>> + * read() - Read a single 32 bit value from a specified address on a
>> + * IHS AXI bus
>> + *
>> + * @dev: IHS AXI bus to read from.
>> + * @address: The address to read from.
>> + * @data: Pointer to a variable that takes the data value read
>> + * from the address on the IHS AXI bus.
>> + * @return 0 if OK, -ve on error.
>> + */
>> + int (*read)(struct udevice *dev, u32 address, u32* data);
>
> How about using ulong for address so can support 64-bit machines.
>
> Also should we have a data length parameter? Or rename this to read32()?
>
> When adding a new uclass we should have a simple sandbox driver and test.
>
AXI supports data bus widths of 8, 16, 32, 64, 128, 256, 512, and 1024 bits, so
on one hand we have discrete possible values for the data length, but too many
to support separate functions for each width.
Do you have any suggestions? A length parameter and a test in the uclass works,
of course, but then you have to do the check for each transmit.
>> +
>> + /**
>> + * write() - Write a single 32 bit value to a specified address on a
>> + * IHS AXI bus
>> + *
>> + * @dev: IHS AXI bus to write to.
>> + * @address: The address to write to.
>> + * @data: Data value to be written to the address on the IHS AXI
>> + * bus.
>> + * @return 0 if OK, -ve on error.
>> + */
>> + int (*write)(struct udevice *dev, u32 address, u32 data);
>> +};
>> +
>> +#define ihs_axi_get_ops(dev) ((struct ihs_axi_ops *)(dev)->driver->ops)
>> +
>> +/**
>> + * axi_read() - Read a single 32 bit value from a specified address on a
>> + * IHS AXI bus
>> + *
>> + * @dev: IHS AXI bus to read from.
>> + * @address: The address to read from.
>> + * @data: Pointer to a variable that takes the data value read from the
>> + * address on the IHS AXI bus.
>> + * @return 0 if OK, -ve on error.
>> + */
>> +int axi_read(struct udevice *dev, u32 address, u32* data);
>> +
>> +/**
>> + * axi_write() - Write a single 32 bit value to a specified address on a
>> + * IHS AXI bus
>> + *
>> + * @dev: IHS AXI bus to write to.
>> + * @address: The address to write to.
>> + * @data: Data value to be written to the address on the IHS AXI bus.
>> + * @return 0 if OK, -ve on error.
>> + */
>> +int axi_write(struct udevice *dev, u32 address, u32 data);
>> +
>> +#endif
>> --
>> 2.11.0
>>
>
> Regards,
> Simon
Best regards,
Mario
More information about the U-Boot
mailing list