| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20140528205049.GA10468@kroah.com>
Date: Wed, 28 May 2014 13:50:49 -0700
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Sudeep Dutt <sudeep.dutt@...el.com>
Cc: Dan Williams <dan.j.williams@...el.com>,
Siva Yerramreddy <yshivakrishna@...il.com>,
Ashutosh Dixit <ashutosh.dixit@...el.com>,
Nikhil Rao <nikhil.rao@...el.com>,
linux-kernel@...r.kernel.org, dmaengine@...r.kernel.org
Subject: Re: [PATCH char-misc-next 2/8] misc: mic: add a bus driver for
virtual MIC devices
On Tue, May 27, 2014 at 07:36:11PM -0700, Sudeep Dutt wrote:
> +int register_mbus_driver(struct mbus_driver *driver)
> +{
> + driver->driver.bus = &mic_bus;
> + return driver_register(&driver->driver);
> +}
> +EXPORT_SYMBOL_GPL(register_mbus_driver);
mbus_register_driver()?
> +void unregister_mbus_driver(struct mbus_driver *driver)
> +{
> + driver_unregister(&driver->driver);
> +}
> +EXPORT_SYMBOL_GPL(unregister_mbus_driver);
mbus_unregister_driver()?
> +int register_mbus_device(struct mbus_device *dev)
mbus_register_device()?
Trying to keep the kernel namespace sane.
Why doesn't this function create the device structure?
> +{
> + int err;
> +
> + dev->dev.bus = &mic_bus;
> +
> + /* Assign a unique device index and hence name. */
> + err = ida_simple_get(&mbus_index_ida, 0, 0, GFP_KERNEL);
> + if (err < 0)
> + return err;
> +
> + dev->index = err;
> + dev_set_name(&dev->dev, "mbus-dev%u", dev->index);
> + /*
> + * device_register() causes the bus infrastructure to look for a
> + * matching driver.
> + */
> + err = device_register(&dev->dev);
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(register_mbus_device);
> +
> +void unregister_mbus_device(struct mbus_device *dev)
> +{
> + int index = dev->index; /* save for after device release */
> +
> + device_unregister(&dev->dev);
> + ida_simple_remove(&mbus_index_ida, index);
> +}
> +EXPORT_SYMBOL_GPL(unregister_mbus_device);
> +
> +static int __init mbus_init(void)
> +{
> + return bus_register(&mic_bus);
> +}
> +
> +static void __exit mbus_exit(void)
> +{
> + bus_unregister(&mic_bus);
> +}
> +
> +core_initcall(mbus_init);
> +module_exit(mbus_exit);
> +
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_DESCRIPTION("Intel(R) MIC Bus driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mic_bus.h b/include/linux/mic_bus.h
> new file mode 100644
> index 0000000..8297573
> --- /dev/null
> +++ b/include/linux/mic_bus.h
> @@ -0,0 +1,148 @@
> +/*
> + * Intel MIC Platform Software Stack (MPSS)
> + *
> + * Copyright(c) 2014 Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, version 2, as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * General Public License for more details.
> + *
> + * The full GNU General Public License is included in this distribution in
> + * the file called "COPYING".
> + *
> + * Intel MIC Bus driver.
> + *
> + * This implementation is very similar to the the virtio bus driver
> + * implementation @ include/linux/virtio.h.
> + */
> +#ifndef _MIC_BUS_H_
> +#define _MIC_BUS_H_
> +/*
> + * Everything a mbus driver needs to work with any particular mbus
> + * implementation.
> + */
> +#include <linux/types.h>
> +#include <linux/device.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/interrupt.h>
> +#include <linux/dma-mapping.h>
> +
> +struct mbus_device_id {
> + __u32 device;
> + __u32 vendor;
> +};
> +
> +#define MBUS_DEV_DMA_HOST 2
> +#define MBUS_DEV_DMA_MIC 3
> +#define MBUS_DEV_ANY_ID 0xffffffff
> +
> +/**
> + * mbus_device - representation of a device using mbus
> + * @priv: private pointer for the driver's use.
> + * @mmio_va: virtual address of mmio space
> + * @hw_ops: the hardware ops supported by this device.
> + * @id: the device type identification (used to match it with a driver).
> + * @dev: underlying device.
> + * be used to communicate with.
> + * @index: unique position on the mbus bus
> + */
> +struct mbus_device {
> + void *priv;
> + void __iomem *mmio_va;
> + struct mbus_hw_ops *hw_ops;
> + struct mbus_device_id id;
> + struct device dev;
> + int index;
> +};
> +
> +/**
> + * mbus_driver - operations for a mbus I/O driver
> + * @driver: underlying device driver (populate name and owner).
> + * @id_table: the ids serviced by this driver.
> + * @probe: the function to call when a device is found. Returns 0 or -errno.
> + * @remove: the function to call when a device is removed.
> + */
> +struct mbus_driver {
> + struct device_driver driver;
> + const struct mbus_device_id *id_table;
> + int (*probe)(struct mbus_device *dev);
> + void (*scan)(struct mbus_device *dev);
> + void (*remove)(struct mbus_device *dev);
> +};
> +
> +/**
> + * struct mic_irq - opaque pointer used as cookie
> + */
> +struct mic_irq;
> +
> +/**
> + * mbus_hw_ops - Hardware operations for accessing a MIC device on the MIC bus.
> + */
> +struct mbus_hw_ops {
> + struct mic_irq* (*request_threaded_irq)(struct mbus_device *mbdev,
> + irq_handler_t handler, irq_handler_t thread_fn,
> + const char *name, void *data, int intr_src);
> + void (*free_irq)(struct mbus_device *mbdev,
> + struct mic_irq *cookie, void *data);
> + void (*ack_interrupt)(struct mbus_device *mbdev, int num);
> +};
> +
> +int register_mbus_device(struct mbus_device *dev);
> +void unregister_mbus_device(struct mbus_device *dev);
> +
> +int register_mbus_driver(struct mbus_driver *drv);
> +void unregister_mbus_driver(struct mbus_driver *drv);
> +
> +static inline struct mbus_device *dev_to_mbus(struct device *_dev)
> +{
> + return container_of(_dev, struct mbus_device, dev);
> +}
> +
> +static inline struct mbus_driver *drv_to_mbus(struct device_driver *drv)
> +{
> + return container_of(drv, struct mbus_driver, driver);
> +}
> +
> +static inline void mbus_release_dev(struct device *d)
> +{
> +}
As per the kernel documentation rules, I get to publicly make fun of you
for doing this half-hearted attempt to get around the kernel providing
you with a valid kernel message as to why you always need to provide a
proper release function :(
Consider yourself mocked.
And go read the kernel documentation for why.
An inline function? Why?
> +static inline int
> +mbus_add_device(struct mbus_device *mbdev, struct device *pdev, int id,
> + struct dma_map_ops *dma_ops, struct mbus_hw_ops *hw_ops,
> + void __iomem *mmio_va)
inline? Why?
> +{
> + int ret;
> +
> + mbdev->mmio_va = mmio_va;
> + mbdev->dev.parent = pdev;
> + mbdev->id.device = id;
> + mbdev->id.vendor = MBUS_DEV_ANY_ID;
> + mbdev->dev.archdata.dma_ops = dma_ops;
> + mbdev->dev.dma_mask = &mbdev->dev.coherent_dma_mask;
> + dma_set_mask(&mbdev->dev, DMA_BIT_MASK(64));
> + mbdev->dev.release = mbus_release_dev;
> + mbdev->hw_ops = hw_ops;
> + dev_set_drvdata(&mbdev->dev, mbdev);
> +
> + ret = register_mbus_device(mbdev);
> + if (ret) {
> + dev_err(mbdev->dev.parent,
> + "Failed to register mbus device type %u\n", id);
> + return ret;
> + }
> + return 0;
> +}
> +
> +static inline void mbus_remove_device(struct mbus_device *mbdev)
> +{
> + unregister_mbus_device(mbdev);
> + memset(mbdev, 0x0, sizeof(*mbdev));
Why are you doing this? (hint, I think I know why, and it's totally and
unacceptably wrong.)
Also, inline?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists