On 24.02.15 23:21, J. German Rivera wrote:
> Platform device driver that sets up the basic bus infrastructure
> for the fsl-mc bus type, including support for adding/removing
> fsl-mc devices, register/unregister of fsl-mc drivers, and bus
> match support to bind devices to drivers.
>
> Signed-off-by: J. German Rivera <[email protected]>
> Signed-off-by: Stuart Yoder <[email protected]>
> ---
> Changes in v7:
> - Refactored MC bus data structures
> - Removed creation of fsl_mc_io object from fsl_mc_add_device()
> - Addressed comments from Alex Graf:
> * Use the 'ranges' property for the 'fsl,qoriq-mc' node as a way to
> translate MC-returned addresses (bus relative addresses) to
> system physical addresses.
>
> Changes addressed in v6:
> - Fixed new checkpatch warnings
>
> Changes addressed in v5:
> - Addressed comments from Alex Graf:
> * Renamed dprc_get_container_id() call as dpmng_get_container_id().
> * Added TODO comment to use the 'ranges' property of the
> fsl-mc DT node.
>
> Changes addressed in v4:
> - Addressed comments from Alex Graf:
> * Added missing scope limitations and more descriptive help
> text for the FSL_MC_BUS config option.
> - Fixed bug related to setting fsl_mc_bus_type.dev_root too
> late.
>
> Changes in v3:
> - Addressed changes from Kim Phillips:
> * Renamed files:
> drivers/bus/fsl-mc/fsl_mc_bus.c -> drivers/bus/fsl-mc/mc-bus.c
> include/linux/fsl_mc.h -> include/linux/fsl/mc.h
> include/linux/fsl_mc_private.h -> include/linux/fsl/mc-private.h
>
> - Addressed comments from Timur Tabi:
> * Changed all functions that had goto out/error when no common cleanup
> was done, to just have multiple return points.
> * Replaced error cleanup boolean flags with multiple exit points.
>
> Changes in v2:
> - Addressed comment from Joe Perches:
> * Changed pr_debug to dev_dbg in fsl_mc_bus_match
>
> - Addressed comments from Kim Phillips and Alex Graf:
> * Changed version check to allow the driver to run with MC
> firmware that has major version number greater than or equal
> to the driver's major version number.
> * Removed minor version check
>
> - Removed unused variable parent_dev in fsl_mc_device_remove
>
> drivers/bus/Kconfig | 3 +
> drivers/bus/Makefile | 3 +
> drivers/bus/fsl-mc/Kconfig | 24 ++
> drivers/bus/fsl-mc/Makefile | 14 +
> drivers/bus/fsl-mc/mc-bus.c | 758
> +++++++++++++++++++++++++++++++++++++++++
> include/linux/fsl/mc-private.h | 67 ++++
> include/linux/fsl/mc.h | 136 ++++++++
> 7 files changed, 1005 insertions(+)
> create mode 100644 drivers/bus/fsl-mc/Kconfig
> create mode 100644 drivers/bus/fsl-mc/Makefile
> create mode 100644 drivers/bus/fsl-mc/mc-bus.c
> create mode 100644 include/linux/fsl/mc-private.h
> create mode 100644 include/linux/fsl/mc.h
>
> diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
> index b99729e..dde3ec2 100644
> --- a/drivers/bus/Kconfig
> +++ b/drivers/bus/Kconfig
> @@ -67,4 +67,7 @@ config VEXPRESS_CONFIG
> help
> Platform configuration infrastructure for the ARM Ltd.
> Versatile Express.
> +
> +source "drivers/bus/fsl-mc/Kconfig"
> +
> endmenu
> diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
> index 2973c18..6abcab1 100644
> --- a/drivers/bus/Makefile
> +++ b/drivers/bus/Makefile
> @@ -15,3 +15,6 @@ obj-$(CONFIG_ARM_CCI) += arm-cci.o
> obj-$(CONFIG_ARM_CCN) += arm-ccn.o
>
> obj-$(CONFIG_VEXPRESS_CONFIG) += vexpress-config.o
> +
> +# Freescale Management Complex (MC) bus drivers
> +obj-$(CONFIG_FSL_MC_BUS) += fsl-mc/
> diff --git a/drivers/bus/fsl-mc/Kconfig b/drivers/bus/fsl-mc/Kconfig
> new file mode 100644
> index 0000000..0d779d9
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/Kconfig
> @@ -0,0 +1,24 @@
> +#
> +# Freescale Management Complex (MC) bus drivers
> +#
> +# Copyright (C) 2014 Freescale Semiconductor, Inc.
> +#
> +# This file is released under the GPLv2
> +#
> +
> +config FSL_MC_BUS
> + tristate "Freescale Management Complex (MC) bus driver"
> + depends on OF && ARM64
> + help
> + Driver to enable the bus infrastructure for the Freescale
> + QorIQ Management Complex (fsl-mc). The fsl-mc is a hardware
> + module of the QorIQ LS2 SoCs, that does resource management
> + for hardware building-blocks in the SoC that can be used
> + to dynamically create networking hardware objects such as
> + network interfaces (NICs), crypto accelerator instances,
> + or L2 switches.
> +
> + Only enable this option when building the kernel for
> + Freescale QorQIQ LS2xxxx SoCs.
> +
> +
> diff --git a/drivers/bus/fsl-mc/Makefile b/drivers/bus/fsl-mc/Makefile
> new file mode 100644
> index 0000000..decd339
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/Makefile
> @@ -0,0 +1,14 @@
> +#
> +# Freescale Management Complex (MC) bus drivers
> +#
> +# Copyright (C) 2014 Freescale Semiconductor, Inc.
> +#
> +# This file is released under the GPLv2
> +#
> +obj-$(CONFIG_FSL_MC_BUS) += mc-bus-driver.o
> +
> +mc-bus-driver-objs := mc-bus.o \
> + mc-sys.o \
> + dprc.o \
> + dpmng.o
> +
> diff --git a/drivers/bus/fsl-mc/mc-bus.c b/drivers/bus/fsl-mc/mc-bus.c
> new file mode 100644
> index 0000000..01407cc
> --- /dev/null
> +++ b/drivers/bus/fsl-mc/mc-bus.c
> @@ -0,0 +1,758 @@
> +/*
> + * Freescale Management Complex (MC) bus driver
> + *
> + * Copyright (C) 2014 Freescale Semiconductor, Inc.
> + * Author: German Rivera <[email protected]>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#include <linux/fsl/mc-private.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_address.h>
> +#include <linux/ioport.h>
> +#include <linux/slab.h>
> +#include <linux/limits.h>
> +#include <linux/fsl/dpmng.h>
> +#include <linux/fsl/mc-sys.h>
> +#include "dprc-cmd.h"
> +
> +static struct kmem_cache *mc_dev_cache;
> +
> +/**
> + * fsl_mc_bus_match - device to driver matching callback
> + * @dev: the MC object device structure to match against
> + * @drv: the device driver to search for matching MC object device id
> + * structures
> + *
> + * Returns 1 on success, 0 otherwise.
> + */
> +static int fsl_mc_bus_match(struct device *dev, struct device_driver *drv)
> +{
> + const struct fsl_mc_device_match_id *id;
> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> + struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(drv);
> + bool found = false;
> +
> + if (WARN_ON(!fsl_mc_bus_type.dev_root))
> + goto out;
> +
> + if (!mc_drv->match_id_table)
> + goto out;
> +
> + /*
> + * If the object is not 'plugged' don't match.
> + * Only exception is the root DPRC, which is a special case.
> + */
> + if ((mc_dev->obj_desc.state & DPRC_OBJ_STATE_PLUGGED) == 0 &&
> + &mc_dev->dev != fsl_mc_bus_type.dev_root)
Does this mean there can only be one fsl-mc per machine? Is this a
reasonable limitation?
Wouldn't it be a lot cleaner to have individual buses for each fsl-mc dt
node? If hardware only implements one today that's fine, but it means we
don't have to completely reengineer everything tomorrow then.
> + goto out;
> +
> + /*
> + * Traverse the match_id table of the given driver, trying to find
> + * a matching for the given MC object device.
> + */
> + for (id = mc_drv->match_id_table; id->vendor != 0x0; id++) {
> + if (id->vendor == mc_dev->obj_desc.vendor &&
> + strcmp(id->obj_type, mc_dev->obj_desc.type) == 0 &&
> + id->ver_major == mc_dev->obj_desc.ver_major &&
> + id->ver_minor == mc_dev->obj_desc.ver_minor) {
> + found = true;
> + break;
> + }
> + }
> +
> +out:
> + dev_dbg(dev, "%smatched\n", found ? "" : "not ");
> + return found;
> +}
> +
> +/**
> + * fsl_mc_bus_uevent - callback invoked when a device is added
> + */
> +static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
> +{
> + pr_debug("%s invoked\n", __func__);
> + return 0;
> +}
> +
> +struct bus_type fsl_mc_bus_type = {
> + .name = "fsl-mc",
> + .match = fsl_mc_bus_match,
> + .uevent = fsl_mc_bus_uevent,
> +};
> +EXPORT_SYMBOL_GPL(fsl_mc_bus_type);
Why does this have to get exported?
> +
> +static int fsl_mc_driver_probe(struct device *dev)
> +{
> + struct fsl_mc_driver *mc_drv;
> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> + int error;
> +
> + if (WARN_ON(!dev->driver))
> + return -EINVAL;
> +
> + mc_drv = to_fsl_mc_driver(dev->driver);
> + if (WARN_ON(!mc_drv->probe))
> + return -EINVAL;
> +
> + error = mc_drv->probe(mc_dev);
> + if (error < 0) {
> + dev_err(dev, "MC object device probe callback failed: %d\n",
> + error);
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static int fsl_mc_driver_remove(struct device *dev)
> +{
> + struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(dev->driver);
> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> + int error;
> +
> + if (WARN_ON(!dev->driver))
> + return -EINVAL;
> +
> + error = mc_drv->remove(mc_dev);
> + if (error < 0) {
> + dev_err(dev,
> + "MC object device remove callback failed: %d\n",
> + error);
> + return error;
> + }
> +
> + return 0;
> +}
> +
> +static void fsl_mc_driver_shutdown(struct device *dev)
> +{
> + struct fsl_mc_driver *mc_drv = to_fsl_mc_driver(dev->driver);
> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> +
> + mc_drv->shutdown(mc_dev);
> +}
> +
> +/**
> + * __fsl_mc_driver_register - registers a child device driver with the
> + * MC bus
> + *
> + * This function is implicitly invoked from the registration function of
> + * fsl_mc device drivers, which is generated by the
> + * module_fsl_mc_driver() macro.
> + */
> +int __fsl_mc_driver_register(struct fsl_mc_driver *mc_driver,
> + struct module *owner)
> +{
> + int error;
> +
> + mc_driver->driver.owner = owner;
> + mc_driver->driver.bus = &fsl_mc_bus_type;
> +
> + /*
> + * Set default driver callbacks, if not set by the child driver:
> + */
> + if (mc_driver->probe)
> + mc_driver->driver.probe = fsl_mc_driver_probe;
I don't understand this. I thought you want to put them as default in
case they're *not* set?
> +
> + if (mc_driver->remove)
> + mc_driver->driver.remove = fsl_mc_driver_remove;
> +
> + if (mc_driver->shutdown)
> + mc_driver->driver.shutdown = fsl_mc_driver_shutdown;
> +
> + error = driver_register(&mc_driver->driver);
> + if (error < 0) {
> + pr_err("driver_register() failed for %s: %d\n",
> + mc_driver->driver.name, error);
> + return error;
> + }
> +
> + pr_info("MC object device driver %s registered\n",
> + mc_driver->driver.name);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(__fsl_mc_driver_register);
> +
> +/**
> + * fsl_mc_driver_unregister - unregisters a device driver from the
> + * MC bus
> + */
> +void fsl_mc_driver_unregister(struct fsl_mc_driver *mc_driver)
> +{
> + driver_unregister(&mc_driver->driver);
> +}
> +EXPORT_SYMBOL_GPL(fsl_mc_driver_unregister);
> +
> +static int get_dprc_icid(struct fsl_mc_io *mc_io,
> + int container_id, uint16_t *icid)
> +{
> + uint16_t dprc_handle;
> + struct dprc_attributes attr;
> + int error;
> +
> + error = dprc_open(mc_io, container_id, &dprc_handle);
> + if (error < 0) {
> + pr_err("dprc_open() failed: %d\n", error);
> + return error;
> + }
> +
> + memset(&attr, 0, sizeof(attr));
> + error = dprc_get_attributes(mc_io, dprc_handle, &attr);
> + if (error < 0) {
> + pr_err("dprc_get_attributes() failed: %d\n", error);
> + goto common_cleanup;
> + }
> +
> + *icid = attr.icid;
> + error = 0;
> +
> +common_cleanup:
> + (void)dprc_close(mc_io, dprc_handle);
> + return error;
> +}
> +
> +static int translate_mc_addr(uint64_t mc_addr, phys_addr_t *phys_addr)
This should probably check that both start and end are within range.
> +{
> + int i;
> + struct fsl_mc *mc = dev_get_drvdata(fsl_mc_bus_type.dev_root->parent);
> +
> + if (mc->num_translation_ranges == 0) {
> + /*
> + * Do identity mapping:
> + */
> + *phys_addr = mc_addr;
> + return 0;
> + }
> +
[...]
> +/**
> + * fsl_mc_device_remove - Remove a MC object device from being visible to
> + * Linux
> + *
> + * @mc_dev: Pointer to a MC object device object
> + */
> +void fsl_mc_device_remove(struct fsl_mc_device *mc_dev)
> +{
> + struct fsl_mc_bus *mc_bus = NULL;
> +
> + kfree(mc_dev->regions);
> +
> + /*
> + * The device-specific remove callback will get invoked by device_del()
> + */
> + device_del(&mc_dev->dev);
> + put_device(&mc_dev->dev);
> +
> + if (strcmp(mc_dev->obj_desc.type, "dprc") == 0) {
I think it'll make the code easier to understand if you extract the
strcmp into a helper function that actually explains what it does.
Something like fsl_mc_dev_is_container().
> + struct fsl_mc_io *mc_io = mc_dev->mc_io;
> +
> + mc_bus = to_fsl_mc_bus(mc_dev);
> + fsl_destroy_mc_io(mc_io);
> + if (&mc_dev->dev == fsl_mc_bus_type.dev_root)
> + fsl_mc_bus_type.dev_root = NULL;
> + }
> +
> + mc_dev->mc_io = NULL;
> + if (mc_bus)
> + devm_kfree(mc_dev->dev.parent, mc_bus);
> + else
> + kmem_cache_free(mc_dev_cache, mc_dev);
> +}
> +EXPORT_SYMBOL_GPL(fsl_mc_device_remove);
[...]
> + dev_info(&pdev->dev,
> + "Freescale Management Complex Firmware version: %u.%u.%u\n",
> + mc_version.major, mc_version.minor, mc_version.revision);
> +
> + if (mc_version.major < MC_VER_MAJOR) {
> + dev_err(&pdev->dev,
> + "ERROR: MC firmware version not supported by driver
> (driver version: %u.%u)\n",
> + MC_VER_MAJOR, MC_VER_MINOR);
> + error = -ENOTSUPP;
> + goto error_cleanup_mc_io;
> + }
> +
> + if (mc_version.major > MC_VER_MAJOR) {
How about we introduce 2 defines for MIN/MAX from the beginning? Then we
can adjust the range by only touching the defines later :).
> + dev_warn(&pdev->dev,
> + "WARNING: driver may not support newer MC firmware
> features (driver version: %u.%u)\n",
> + MC_VER_MAJOR, MC_VER_MINOR);
> + }
[...]
> +static int __init fsl_mc_bus_driver_init(void)
> +{
> + int error;
> +
> + mc_dev_cache = kmem_cache_create("fsl_mc_device",
> + sizeof(struct fsl_mc_device), 0, 0,
> + NULL);
> + if (!mc_dev_cache) {
> + pr_err("Could not create fsl_mc_device cache\n");
> + return -ENOMEM;
> + }
> +
> + error = bus_register(&fsl_mc_bus_type);
> + if (error < 0) {
> + pr_err("fsl-mc bus type registration failed: %d\n", error);
> + goto error_cleanup_cache;
> + }
> +
> + pr_info("fsl-mc bus type registered\n");
> +
> + error = platform_driver_register(&fsl_mc_bus_driver);
> + if (error < 0) {
> + pr_err("platform_driver_register() failed: %d\n", error);
> + goto error_cleanup_bus;
> + }
> +
> + return 0;
> +
> +error_cleanup_bus:
> + bus_unregister(&fsl_mc_bus_type);
> +
> +error_cleanup_cache:
> + kmem_cache_destroy(mc_dev_cache);
> + return error;
> +}
> +
> +postcore_initcall(fsl_mc_bus_driver_init);
How does this get hooked up when compiled as a module?
Alex
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
