> -----Original Message----- > From: Lijun Pan [mailto:Lijun.Pan@xxxxxxxxxxxxx] > Sent: Sunday, October 25, 2015 5:41 PM > To: gregkh@xxxxxxxxxxxxxxxxxxx; arnd@xxxxxxxx; devel@xxxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > Cc: Yoder Stuart-B08248; katz Itai-RM05202; Rivera Jose-B46482; Li Yang-Leo-R58472; Wood Scott-B07421; > agraf@xxxxxxx; Hamciuc Bogdan-BHAMCIU1; Marginean Alexandru-R89243; Sharma Bhupesh-B45370; Erez Nir-RM30794; > Schmitt Richard-B43082; dan.carpenter@xxxxxxxxxx; Pan Lijun-B44306 > Subject: [PATCH 5/5] staging: fsl-mc: Management Complex restool driver > > The kernel support for the restool (a user space resource management > tool) is a driver for the /dev/dprc.N device file. > Its purpose is to provide an ioctl interface, > which the restool uses to interact with the MC bus driver > and with the MC firmware. > We allocate a dpmcp at driver initialization, > and keep that dpmcp until driver exit. > We use that dpmcp by default. > If that dpmcp is in use, we create another portal at run time > and destroy the newly created portal after use. > The ioctl RESTOOL_SEND_MC_COMMAND sends user space command to fsl-mc > bus and utilizes the fsl-mc bus to communicate with MC firmware. > The ioctl RESTOOL_DPRC_SYNC request the mc-bus launch > objects scan under root dprc. > In order to support multiple root dprc, we utilize the bus notify > mechanism to scan fsl_mc_bus_type for the newly added root dprc. > After discovering the root dprc, it creates a miscdevice > /dev/dprc.N to associate with this root dprc. > > Signed-off-by: Lijun Pan <Lijun.Pan@xxxxxxxxxxxxx> > --- > drivers/staging/fsl-mc/bus/Kconfig | 7 +- > drivers/staging/fsl-mc/bus/Makefile | 3 + > drivers/staging/fsl-mc/bus/mc-ioctl.h | 24 ++ > drivers/staging/fsl-mc/bus/mc-restool.c | 488 ++++++++++++++++++++++++++++++++ > 4 files changed, 521 insertions(+), 1 deletion(-) > create mode 100644 drivers/staging/fsl-mc/bus/mc-ioctl.h > create mode 100644 drivers/staging/fsl-mc/bus/mc-restool.c > > diff --git a/drivers/staging/fsl-mc/bus/Kconfig b/drivers/staging/fsl-mc/bus/Kconfig > index 0d779d9..39c6ef9 100644 > --- a/drivers/staging/fsl-mc/bus/Kconfig > +++ b/drivers/staging/fsl-mc/bus/Kconfig > @@ -21,4 +21,9 @@ config FSL_MC_BUS > Only enable this option when building the kernel for > Freescale QorQIQ LS2xxxx SoCs. > > - > +config FSL_MC_RESTOOL > + tristate "Freescale Management Complex (MC) restool driver" > + depends on FSL_MC_BUS > + help > + Driver that provides kernel support for the Freescale Management > + Complex resource manager user-space tool. > diff --git a/drivers/staging/fsl-mc/bus/Makefile b/drivers/staging/fsl-mc/bus/Makefile > index 25433a9..28b5fc0 100644 > --- a/drivers/staging/fsl-mc/bus/Makefile > +++ b/drivers/staging/fsl-mc/bus/Makefile > @@ -15,3 +15,6 @@ mc-bus-driver-objs := mc-bus.o \ > mc-allocator.o \ > dpmcp.o \ > dpbp.o > + > +# MC restool kernel support > +obj-$(CONFIG_FSL_MC_RESTOOL) += mc-restool.o > diff --git a/drivers/staging/fsl-mc/bus/mc-ioctl.h b/drivers/staging/fsl-mc/bus/mc-ioctl.h > new file mode 100644 > index 0000000..e52f907 > --- /dev/null > +++ b/drivers/staging/fsl-mc/bus/mc-ioctl.h > @@ -0,0 +1,24 @@ > +/* > + * Freescale Management Complex (MC) ioclt interface > + * > + * Copyright (C) 2014 Freescale Semiconductor, Inc. > + * Author: Lijun Pan <Lijun.Pan@xxxxxxxxxxxxx> > + * > + * 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. > + */ > +#ifndef _FSL_MC_IOCTL_H_ > +#define _FSL_MC_IOCTL_H_ > + > +#include <linux/ioctl.h> > + > +#define RESTOOL_IOCTL_TYPE 'R' > + > +#define RESTOOL_DPRC_SYNC \ > + _IO(RESTOOL_IOCTL_TYPE, 0x2) We no longer need a sync ioctl, as a the /sys/bus/fsl-mc/rescan mechanism in the previous patch replaces this. > +#define RESTOOL_SEND_MC_COMMAND \ > + _IOWR(RESTOOL_IOCTL_TYPE, 0x4, struct mc_command) > + > +#endif /* _FSL_MC_IOCTL_H_ */ > diff --git a/drivers/staging/fsl-mc/bus/mc-restool.c b/drivers/staging/fsl-mc/bus/mc-restool.c > new file mode 100644 > index 0000000..a219172 > --- /dev/null > +++ b/drivers/staging/fsl-mc/bus/mc-restool.c > @@ -0,0 +1,488 @@ > +/* > + * Freescale Management Complex (MC) restool driver > + * > + * Copyright (C) 2014 Freescale Semiconductor, Inc. > + * Author: Lijun Pan <Lijun.Pan@xxxxxxxxxxxxx> > + * > + * 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 "../include/mc-private.h" > +#include <linux/module.h> > +#include <linux/fs.h> > +#include <linux/miscdevice.h> > +#include <linux/mm.h> > +#include <linux/slab.h> > +#include <linux/uaccess.h> > +#include <linux/mutex.h> > +#include <linux/platform_device.h> > +#include "mc-ioctl.h" > +#include "../include/mc-sys.h" > +#include "../include/mc-cmd.h" > +#include "../include/dpmng.h" > + > +/** > + * Maximum number of DPRCs that can be opened at the same time > + */ > +#define MAX_DPRC_HANDLES 64 > + > +/** > + * restool_misc - information associated with the newly added miscdevice > + * @misc: newly created miscdevice associated with root dprc > + * @miscdevt: device id of this miscdevice > + * @list: a linked list node representing this miscdevcie > + * @static_mc_io: pointer to the static MC I/O object used by the restool > + * @dynamic_instance_count: number of dynamically created instances > + * @static_instance_in_use: static instance is in use or not > + * @mutex: mutex lock to serialze the operations > + * @dev: root dprc associated with this miscdevice > + */ > +struct restool_misc { > + struct miscdevice misc; > + dev_t miscdevt; > + struct list_head list; > + struct fsl_mc_io *static_mc_io; > + uint32_t dynamic_instance_count; > + bool static_instance_in_use; > + struct mutex mutex; > + struct device *dev; > +}; > + > +/* > + * initialize a global list to link all > + * the miscdevice nodes (struct restool_misc) > + */ > +LIST_HEAD(misc_list); > + > +static int fsl_mc_restool_dev_open(struct inode *inode, struct file *filep) > +{ > + struct fsl_mc_device *root_mc_dev; > + int error = 0; > + struct fsl_mc_io *dynamic_mc_io = NULL; > + struct restool_misc *restool_misc; > + struct restool_misc *restool_misc_cursor; > + > + pr_debug("%s: inode's dev_t == %u\n", __func__, inode->i_rdev); > + > + list_for_each_entry(restool_misc_cursor, &misc_list, list) { > + if (restool_misc_cursor->miscdevt == inode->i_rdev) { > + pr_debug("%s: Found the restool_misc\n", __func__); > + restool_misc = restool_misc_cursor; > + break; > + } > + } > + > + if (!restool_misc) > + return -EINVAL; > + > + if (WARN_ON(restool_misc->dev == NULL)) > + return -EINVAL; > + > + mutex_lock(&restool_misc->mutex); > + > + if (!restool_misc->static_instance_in_use) { > + restool_misc->static_instance_in_use = true; > + filep->private_data = restool_misc->static_mc_io; > + } else { > + dynamic_mc_io = kzalloc(sizeof(struct fsl_mc_io), GFP_KERNEL); > + if (dynamic_mc_io == NULL) { > + error = -ENOMEM; > + goto error; > + } > + > + root_mc_dev = to_fsl_mc_device(restool_misc->dev); > + error = fsl_mc_portal_allocate(root_mc_dev, 0, &dynamic_mc_io); > + if (error < 0) { > + pr_err("Not able to allocate MC portal\n"); > + goto error; > + } > + ++restool_misc->dynamic_instance_count; > + filep->private_data = dynamic_mc_io; > + } > + > + mutex_unlock(&restool_misc->mutex); > + > + return 0; > +error: > + if (dynamic_mc_io != NULL) { > + fsl_mc_portal_free(dynamic_mc_io); > + kfree(dynamic_mc_io); > + } > + > + mutex_unlock(&restool_misc->mutex); > + > + return error; > +} > + > +static int fsl_mc_restool_dev_release(struct inode *inode, struct file *filep) > +{ > + struct fsl_mc_io *local_mc_io = filep->private_data; > + struct restool_misc *restool_misc; > + struct restool_misc *restool_misc_cursor; > + > + if (WARN_ON(filep->private_data == NULL)) > + return -EINVAL; > + > + pr_debug("%s: inode's dev_t == %u\n", __func__, inode->i_rdev); > + > + list_for_each_entry(restool_misc_cursor, &misc_list, list) { > + if (restool_misc_cursor->miscdevt == inode->i_rdev) { > + pr_debug("%s: Found the restool_misc\n", __func__); > + restool_misc = restool_misc_cursor; > + break; > + } > + } > + > + if (!restool_misc) > + return -EINVAL; > + > + mutex_lock(&restool_misc->mutex); > + > + if (WARN_ON(restool_misc->dynamic_instance_count == 0 && > + !restool_misc->static_instance_in_use)) { > + mutex_unlock(&restool_misc->mutex); > + return -EINVAL; > + } > + > + /* Globally clean up opened/untracked handles */ > + fsl_mc_portal_reset(local_mc_io); > + > + pr_debug("dynamic instance count: %d\n", > + restool_misc->dynamic_instance_count); > + pr_debug("static instance count: %d\n", > + restool_misc->static_instance_in_use); > + > + /* > + * must check > + * whether local_mc_io is dynamic or static instance > + * Otherwise it will free up the reserved portal by accident > + * or even not free up the dynamic allocated portal > + * if 2 or more instances running concurrently > + */ > + if (local_mc_io == restool_misc->static_mc_io) { > + pr_debug("this is reserved portal"); > + pr_debug("reserved portal not in use\n"); > + restool_misc->static_instance_in_use = false; > + } else { > + pr_debug("this is dynamically allocated portal"); > + pr_debug("free one dynamically allocated portal\n"); > + fsl_mc_portal_free(local_mc_io); > + kfree(filep->private_data); > + --restool_misc->dynamic_instance_count; > + } > + > + filep->private_data = NULL; > + mutex_unlock(&restool_misc->mutex); > + > + return 0; > +} > + > +static int restool_dprc_sync(struct inode *inode) > +{ > + int error = 0; > + struct fsl_mc_device *root_mc_dev; > + struct fsl_mc_bus *root_mc_bus; > + struct restool_misc *restool_misc; > + struct restool_misc *restool_misc_cursor; > + > + pr_debug("%s: inode's dev_t == %u\n", __func__, inode->i_rdev); > + > + list_for_each_entry(restool_misc_cursor, &misc_list, list) { > + if (restool_misc_cursor->miscdevt == inode->i_rdev) { > + pr_debug("%s: Found the restool_misc\n", __func__); > + restool_misc = restool_misc_cursor; > + break; > + } > + } > + > + if (!restool_misc) > + return -EINVAL; > + > + root_mc_dev = to_fsl_mc_device(restool_misc->dev); > + root_mc_bus = to_fsl_mc_bus(root_mc_dev); > + > + mutex_lock(&root_mc_bus->scan_mutex); > + error = dprc_scan_objects(root_mc_dev); > + mutex_unlock(&root_mc_bus->scan_mutex); > + pr_debug("sync_error = %d\n", error); > + > + return error; > +} We no longer need/use the ioctl interface for sync. > +static int restool_send_mc_command(unsigned long arg, > + struct fsl_mc_io *local_mc_io) > +{ > + int error = -EINVAL; > + struct mc_command mc_cmd; > + > + error = copy_from_user(&mc_cmd, (void __user *)arg, sizeof(mc_cmd)); > + if (error < 0) { > + pr_err("copy_to_user() failed with error %d\n", error); > + goto error; > + } > + > + /* > + * Send MC command to the MC: > + */ > + error = mc_send_command(local_mc_io, &mc_cmd); > + if (error < 0) > + goto error; > + > + error = copy_to_user((void __user *)arg, &mc_cmd, sizeof(mc_cmd)); > + if (error < 0) { > + pr_err("copy_to_user() failed with error %d\n", error); > + goto error; > + } > + > + return 0; > +error: > + return error; > +} > + > +static long > +fsl_mc_restool_dev_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > +{ > + int error = 0; > + > + switch (cmd) { > + case RESTOOL_DPRC_SYNC: > + pr_debug("syncing...\n"); > + error = restool_dprc_sync(file->f_inode); > + pr_debug("syncing finished...\n"); > + break; Remove the sync ioctl. > + case RESTOOL_SEND_MC_COMMAND: > + error = restool_send_mc_command(arg, file->private_data); > + break; > + default: > + pr_err("%s: unexpected ioctl call number\n", __func__); > + error = -EINVAL; > + } > + > + return error; > +} > + > +static const struct file_operations fsl_mc_restool_dev_fops = { > + .owner = THIS_MODULE, > + .open = fsl_mc_restool_dev_open, > + .release = fsl_mc_restool_dev_release, > + .unlocked_ioctl = fsl_mc_restool_dev_ioctl, > +}; > + > +static int restool_add_device_file(struct device *dev) > +{ > + uint32_t name1 = 0; > + char name2[20] = {0}; > + int error = 0; > + struct fsl_mc_device *root_mc_dev; > + struct restool_misc *restool_misc; > + > + pr_debug("newly scanned/notified device: %s, whose parent:%s\n", > + dev_name(dev), dev_name(dev->parent)); > + > + if (dev->bus == &platform_bus_type && dev->driver_data) { > + if (sscanf(dev_name(dev), "%x.%s", &name1, name2) != 2) { > + pr_err("sscanf failure\n"); > + return -EINVAL; > + } > + if (strcmp(name2, "fsl-mc") == 0) > + pr_debug("platform's root dprc name is: %s\n", > + dev_name(&(((struct fsl_mc *)(dev->driver_data)) > + ->root_mc_bus_dev->dev))); > + } > + > + if (dev->bus == &fsl_mc_bus_type) > + pr_debug("%s's bus type: fsl_mc_bus_type\n", dev_name(dev)); > + else if (dev->bus == &platform_bus_type) > + pr_debug("%s's bus type: platform_bus_type\n", dev_name(dev)); > + else > + pr_debug("%s's bus type: NEITHER fsl_mc_bus_type NOR platform_bus_type\n", > + dev_name(dev)); > + > + if (is_root_dprc(dev)) { > + pr_debug("I am root dprc, create /dev/%s\n", dev_name(dev)); > + restool_misc = kzalloc(sizeof(struct restool_misc), GFP_KERNEL); > + if (restool_misc == NULL) > + return -ENOMEM; > + > + restool_misc->dev = dev; > + root_mc_dev = to_fsl_mc_device(dev); > + error = fsl_mc_portal_allocate(root_mc_dev, 0, > + &restool_misc->static_mc_io); > + if (error < 0) { > + pr_err("Not able to allocate MC portal\n"); > + goto err_portal; > + } > + > + restool_misc->misc.minor = MISC_DYNAMIC_MINOR; > + restool_misc->misc.name = dev_name(dev); > + restool_misc->misc.fops = &fsl_mc_restool_dev_fops; > + > + error = misc_register(&restool_misc->misc); > + if (error < 0) { > + pr_err("misc_register() failed: %d\n", error); > + goto err_reg; > + } > + > + restool_misc->miscdevt = restool_misc->misc.this_device->devt; > + mutex_init(&restool_misc->mutex); > + list_add(&restool_misc->list, &misc_list); > + pr_info("/dev/%s driver registered\n", dev_name(dev)); > + } else > + pr_info("%s is not root dprc, miscdevice cannot be created/associated\n", > + dev_name(dev)); > + > + return 0; > +err_reg: > + misc_deregister(&restool_misc->misc); > + > +err_portal: > + if (restool_misc->static_mc_io) > + fsl_mc_portal_free(restool_misc->static_mc_io); > + if (restool_misc) > + kfree(restool_misc); > + > + return error; > +} > + > +static int restool_bus_notifier(struct notifier_block *nb, > + unsigned long action, void *data) > +{ > + struct device *dev = data; > + > + pr_debug("entering %s...\n", __func__); > + pr_debug("being notified by device: %s\n", dev_name(dev)); > + > + if (dev->bus == &fsl_mc_bus_type) > + pr_debug("%s's bus type: fsl_mc_bus_type\n", dev_name(dev)); > + else if (dev->bus == &platform_bus_type) > + pr_debug("%s's bus type: platform_bus_type\n", dev_name(dev)); > + else > + pr_debug("%s's bus type: NEITHER fsl_mc_bus_type NOR platform_bus_type\n", > + dev_name(dev)); > + > + switch (action) { > + case BUS_NOTIFY_ADD_DEVICE: > + pr_info("bus notify device added: %s\n", dev_name(dev)); > + restool_add_device_file(dev); The only time we want to do add a new device file is for root DPRCs. This code would seem to add a device file any time any device is added to the bus? How are you filtering out all the other object types?...it's not clear to me. > + break; > + case BUS_NOTIFY_DEL_DEVICE: > + pr_info("bus notify device to be removed: %s\n", dev_name(dev)); > + break; > + case BUS_NOTIFY_REMOVED_DEVICE: > + pr_info("bus notify device removed: %s\n", dev_name(dev)); > + break; > + case BUS_NOTIFY_BIND_DRIVER: > + pr_info("bus notify driver about to be bound to device: %s\n", dev_name(dev)); > + break; > + case BUS_NOTIFY_BOUND_DRIVER: > + pr_info("bus notify driver bound to device: %s\n", dev_name(dev)); > + break; > + case BUS_NOTIFY_UNBIND_DRIVER: > + pr_info("bus notify driver about to unbind from device: %s\n", dev_name(dev)); > + break; > + case BUS_NOTIFY_UNBOUND_DRIVER: > + pr_info("bus notify driver unbind from device: %s\n", dev_name(dev)); > + break; > + default: > + pr_err("%s: unrecognized device action from %s\n", __func__, dev_name(dev)); > + return -EINVAL; > + } > + > + pr_debug("leaving %s...\n", __func__); > + > + return 0; > +} There is only a single line of functionality in the above function. All the rest pr_debug and pr_info. We certainly don't need pr_info statements every time a bus notify occurs. In general I think this driver has too many debug prints in it. Thanks, Stuart _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel