From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-10.7 required=3.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED,DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E6A49C433F5 for ; Fri, 10 Sep 2021 11:42:18 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id AE3F961153 for ; Fri, 10 Sep 2021 11:42:18 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org AE3F961153 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=lcOSmKn8z2Pyj+CRqZmITjqvc9qk72xYCK5EufNrx/I=; b=f+RKtH+UrY/AU4 UoasjKrr9IgLNq1uHDHhDPcpc+SKXj044yhqpuP163f5r5874YM2aGrihukHcCTf+mnGsy1PF6pUc /8fqz+0v/evksiahZQGxR7qVbWxyaqK8wQPQobR93cv5/ld+kQZeg2ZT73hw9u0VG3vM+UQ5NHZNj np6QUI1d2vIcwobaEHVryb32eEgAuQlBEF/JsgBWfumkmR3bqnymUvymkw0KEnFevK9lvUif8+u8T vylsJUeGOoCSysJzURZ3+/VJTj5AYt4xlukHvRmIHc6/wdoaBNyeEA0Yen+xb7pdH9h/s1DQiy08d qPJIok6t5U8aTK2f4Zjw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mOetH-00CdZx-Ky; Fri, 10 Sep 2021 11:40:19 +0000 Received: from mail-pj1-x1036.google.com ([2607:f8b0:4864:20::1036]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mOetD-00CdY9-Bh; Fri, 10 Sep 2021 11:40:17 +0000 Received: by mail-pj1-x1036.google.com with SMTP id oc9so1163492pjb.4; Fri, 10 Sep 2021 04:40:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=vt09EgmCFNaOpoRWV9K80XX9Kd5es86lQ5YaOnlS2Mo=; b=bASbeF9GEJGPuSgIhYlbSMyrzz1KEQ90Aq8gYRMx/KHAe9qIF4K1HaiAxhTIR0/X2l bQGz9342k0YfCkGqjs+vCAyHvCF3S1XXVBstLvF4GCJDPceRCS2bkCEJe0YddxXIvKOB zUz3zH0DAKju4ntSi5JcgOSTf/li3uA9E/I6D4IPH5U6cFG3sDXtUobEY0DdSDMwu808 CQGXnvxZJso2IexkBlrNom2jRqsRlJFvF6OSuEVW4k7XjSkzRbRFYYAA+OX4qInCtcQ9 tQ4tnutmlNUlGlLnstbkTTauA/RQlc4p5t5iwRzcTJOwq670c+jLVFju4RFOHJXeF75X tBhA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=vt09EgmCFNaOpoRWV9K80XX9Kd5es86lQ5YaOnlS2Mo=; b=PAdE8bfFiKpiqBbktr5a1iCqMef8nJ3DnPEQn8/B6ru2JxNd9u82qnSQvOcdmIbQ/b P+QWUAKkoaVYoMSpaEKYrZYa8C97oR1HAAB3RYrhEfeSbAacSstoT6BKmFUaRqFBofZ3 9I4X6y2P3PAQ6MWRWAZvkAkWuUOOdM19/Cu5sCWLGj9Dju0eahoDTCSplXl9PRQrWtam 8ib0YdITAn9XeQ6TfJzu6JiXMWQhtDGsiqlAI/XpMdYR7GE1U31SYAeTD2+fh9cCuqLi bEg7vQA+5a5QtUmpEnYd1fqMDfcqBPHnd+Z1/264OlU0istcg1yIkJH1xQCuNXhgCcAE 576Q== X-Gm-Message-State: AOAM530MqHw6/D4yQxMZ3GxLTbmu6dfQA0+eaXvFOfX5cSK0muuL3oab h0eCqNVIhhBf73EYdr+dQZI= X-Google-Smtp-Source: ABdhPJwGW9y9T7sPMAt8pvmQ2B+oECa7k9fdxvj/BjNeQIxv55IIO2AkxK6wzUbt4vxK6tAafip71A== X-Received: by 2002:a17:90b:1d0e:: with SMTP id on14mr8957109pjb.97.1631274011397; Fri, 10 Sep 2021 04:40:11 -0700 (PDT) Received: from ojas ([122.161.49.57]) by smtp.gmail.com with ESMTPSA id h7sm5014684pfk.96.2021.09.10.04.40.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Sep 2021 04:40:11 -0700 (PDT) Date: Fri, 10 Sep 2021 17:10:04 +0530 From: Ojaswin Mujoo To: Greg Kroah-Hartman Cc: linux-staging@lists.linux.dev, Nicolas Saenz Julienne , Stefan Wahren , Arnd Bergmann , Dan Carpenter , Phil Elwell , bcm-kernel-feedback-list@broadcom.com, linux-rpi-kernel@lists.infradead.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH] staging: vchiq: convert to use a miscdevice Message-ID: <20210910114004.GA23656@ojas> References: <20210907115045.2206083-1-gregkh@linuxfoundation.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210907115045.2206083-1-gregkh@linuxfoundation.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210910_044015_456776_8B637D4F X-CRM114-Status: GOOD ( 26.94 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, Sep 07, 2021 at 01:50:45PM +0200, Greg Kroah-Hartman wrote: > Using a struct class, a cdev, and another device just for a single minor > device is total overkill. Just use a dynamic misc device instead, > saving lots of logic and memory. Hello Greg, I got some time to test this out at my end. This seems to work correctly however there's a small change in permissions applied to /dev/vchiq that is causing tests to break. * Permissions before the patch * $ ls -l /dev/vchiq crw-rw---- 1 root video 235, 0 May 7 17:33 vchiq * Permissions after the patch * $ ls -l /dev/vchiq crw------- 1 root root 10, 125 May 7 17:30 vchiq As seen above, after the patch, the cdev is only accessible by root user, which is causing the tests ($ vchiq_test -f 10) to fail when run as non-root. I believe assigning the permission and "video" group to /dev/vchiq is handled by udev, in the downstream pi OS, as seen in this line in /lib/udev/rules.d/10-local-rpi.rules file: SUBSYSTEM=="vchiq", GROUP="video", MODE="0660" I'm not completely sure how the SUBSYSTEM part is passed to udev from the kernel modules, however seems like the miscdevice is not notifying udev correctly (?). I'm still looking into this but thought I'd point this out so someone more experienced can check and see if this could be an issue. Regards, Ojaswin > > Cc: Nicolas Saenz Julienne > Cc: Stefan Wahren > Cc: Arnd Bergmann > Cc: Dan Carpenter > Cc: Ojaswin Mujoo > Cc: Phil Elwell > Cc: bcm-kernel-feedback-list@broadcom.com > Cc: linux-rpi-kernel@lists.infradead.org > Cc: linux-arm-kernel@lists.infradead.org > Signed-off-by: Greg Kroah-Hartman > --- > .../interface/vchiq_arm/vchiq_dev.c | 71 +++---------------- > 1 file changed, 11 insertions(+), 60 deletions(-) > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c > index bf1a88c9d1ee..788fa5a987a3 100644 > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_dev.c > @@ -9,18 +9,13 @@ > #include > #include > #include > +#include > > #include "vchiq_core.h" > #include "vchiq_ioctl.h" > #include "vchiq_arm.h" > #include "vchiq_debugfs.h" > > -#define DEVICE_NAME "vchiq" > - > -static struct cdev vchiq_cdev; > -static dev_t vchiq_devid; > -static struct class *vchiq_class; > - > static const char *const ioctl_names[] = { > "CONNECT", > "SHUTDOWN", > @@ -1364,6 +1359,13 @@ vchiq_fops = { > .read = vchiq_read > }; > > +static struct miscdevice vchiq_miscdev = { > + .fops = &vchiq_fops, > + .minor = MISC_DYNAMIC_MINOR, > + .name = "vchiq", > + > +}; > + > /** > * vchiq_register_chrdev - Register the char driver for vchiq > * and create the necessary class and > @@ -1374,57 +1376,9 @@ vchiq_fops = { > */ > int vchiq_register_chrdev(struct device *parent) > { > - struct device *vchiq_dev; > - int ret; > - > - vchiq_class = class_create(THIS_MODULE, DEVICE_NAME); > - if (IS_ERR(vchiq_class)) { > - pr_err("Failed to create vchiq class\n"); > - ret = PTR_ERR(vchiq_class); > - goto error_exit; > - } > - > - ret = alloc_chrdev_region(&vchiq_devid, 0, 1, DEVICE_NAME); > - if (ret) { > - pr_err("vchiq: Failed to allocate vchiq's chrdev region\n"); > - goto alloc_region_error; > - } > - > - cdev_init(&vchiq_cdev, &vchiq_fops); > - vchiq_cdev.owner = THIS_MODULE; > - ret = cdev_add(&vchiq_cdev, vchiq_devid, 1); > - if (ret) { > - vchiq_log_error(vchiq_arm_log_level, > - "Unable to register vchiq char device"); > - goto cdev_add_error; > - } > - > - vchiq_dev = device_create(vchiq_class, parent, vchiq_devid, NULL, > - DEVICE_NAME); > - if (IS_ERR(vchiq_dev)) { > - vchiq_log_error(vchiq_arm_log_level, > - "Failed to create vchiq char device node"); > - ret = PTR_ERR(vchiq_dev); > - goto device_create_error; > - } > - > - vchiq_log_info(vchiq_arm_log_level, > - "vchiq char dev initialised successfully - device %d.%d", > - MAJOR(vchiq_devid), MINOR(vchiq_devid)); > + vchiq_miscdev.parent = parent; > > - return 0; > - > -device_create_error: > - cdev_del(&vchiq_cdev); > - > -cdev_add_error: > - unregister_chrdev_region(vchiq_devid, 1); > - > -alloc_region_error: > - class_destroy(vchiq_class); > - > -error_exit: > - return ret; > + return misc_register(&vchiq_miscdev); > } > > /** > @@ -1433,8 +1387,5 @@ int vchiq_register_chrdev(struct device *parent) > */ > void vchiq_deregister_chrdev(void) > { > - device_destroy(vchiq_class, vchiq_devid); > - cdev_del(&vchiq_cdev); > - unregister_chrdev_region(vchiq_devid, 1); > - class_destroy(vchiq_class); > + misc_deregister(&vchiq_miscdev); > } > -- > 2.33.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel