Re: [PATCH 2/2] uacce: add uacce module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Greg

On 2019/8/19 下午6:24, Greg Kroah-Hartman wrote:
+static int uacce_create_chrdev(struct uacce *uacce)
+{
+	int ret;
+
+	ret = idr_alloc(&uacce_idr, uacce, 0, 0, GFP_KERNEL);
+	if (ret < 0)
+		return ret;
+
Shouldn't this function create the memory needed for this structure?
You are relying ont he caller to do it for you, why?
I think you mean uacce structure here.
Yes, currently we count on caller to prepare uacce structure and call
uacce_register(uacce).
We still think this method is simpler, prepare uacce, register uacce.
And there are other system using the same method, like crypto
(crypto_register_acomp), nand, etc.
crypto is not a subsystem to ever try to emulate :)

You are creating a structure with a lifetime that you control, don't
have someone else create your memory, that's almost never what you want
to do.  Most all driver subsystems create their own memory chunks for
what they need to do, it's a much better pattern.

Especially when you get into pointer lifetime issues...
OK, understand now, thanks for your patience.
will use this instead.
struct uacce_interface {
        char name[32];
        unsigned int flags;
        struct uacce_ops *ops;
};
struct uacce *uacce_register(struct device *dev, struct uacce_interface *interface);
+}
+
+static int uacce_dev_match(struct device *dev, void *data)
+{
+	if (dev->parent == data)
+		return -EBUSY;
There should be in-kernel functions for this now, no need for you to
roll your own.
Sorry, do not find this function.
Only find class_find_device, which still require match.
It is in linux-next, look there...

Suppose you mean the funcs: device_match_name, device_match_of_node,device_match_devt etc.
Here we need dev->parent, there still no such func.

Thanks




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux