On 10/10/24 12:04, Greg KH wrote:
On Thu, Oct 10, 2024 at 11:53:44AM +0200, Rodolfo Giometti wrote:
On 10/10/24 09:54, Greg KH wrote:
On Thu, Oct 10, 2024 at 09:32:22AM +0200, Rodolfo Giometti wrote:
On 10/10/24 09:15, Greg KH wrote:
On Wed, Oct 09, 2024 at 10:48:14AM +0200, Rodolfo Giometti wrote:
+#ifdef CONFIG_COMPAT
+static long pps_gen_cdev_compat_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ cmd = _IOC(_IOC_DIR(cmd), _IOC_TYPE(cmd), _IOC_NR(cmd), sizeof(void *));
+ return pps_gen_cdev_ioctl(file, cmd, arg);
+}
+#else
+#define pps_gen_cdev_compat_ioctl NULL
+#endif
+
+static struct pps_gen_device *pps_gen_idr_get(unsigned long id)
+{
+ struct pps_gen_device *pps_gen;
+
+ mutex_lock(&pps_gen_idr_lock);
+ pps_gen = idr_find(&pps_gen_idr, id);
+ if (pps_gen)
+ kobject_get(&pps_gen->dev->kobj);
+
+ mutex_unlock(&pps_gen_idr_lock);
Doesn't an idr have a lock in it? I can never remember...
As far as I know we must use a mutex...
If you do, someone will come along and remove it, please see:
https://lore.kernel.org/r/b1fcc6707ec2b6309d50060fa52ccc2c892afde2.1728507153.git.christophe.jaillet@xxxxxxxxxx
as an example (with links that show it is not needed).
Here is an example about ida API, but I'm using idr API.
Why not use ida then? :)
Because we need an ID associated with a pointer.
Ah, ok, but why? Why is the "id" here the mapping to the pointer? Why
not use the structures you already have to store this in (i.e. the
normal driver model stuff?)
All you should need an idr/ida for is to pick a unique "number" for
naming your class device. Everything after that should already be there
for you to get access to the structures you need to get access to.
OK, let me review the code and then I'm going to propose a new patchset.
Thanks for your suggestions. :)
Ciao,
Rodolfo
--
GNU/Linux Solutions e-mail: giometti@xxxxxxxxxxxx
Linux Device Driver giometti@xxxxxxxx
Embedded Systems phone: +39 349 2432127
UNIX programming
