Received: by 2002:a05:6a10:d5a5:0:0:0:0 with SMTP id gn37csp4451945pxb; Tue, 5 Oct 2021 03:29:10 -0700 (PDT) X-Google-Smtp-Source: ABdhPJz3lSK3nWK/CmBazJb+Lo6acloRB9dSCHfQJOhfuhtfm8/HiF5/WzMFnkuvujKwVI7smWB5 X-Received: by 2002:a05:6402:21ef:: with SMTP id ce15mr24872352edb.19.1633429750289; Tue, 05 Oct 2021 03:29:10 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1633429750; cv=none; d=google.com; s=arc-20160816; b=MOQpU4rGvUGEO59iZEFr1mBeFm8+S1gzBzOnTXQm5EwQg/xdYfN4/gpJq5bnMAPT9X IYipysM1dgah3AA2W/iwStOWsqehN+CHsGXSqc946nfsKOPgtMv+YDkMvWP3jGb72oir jQg0k8+dkK/GaPZQiis4PwbNfEVvm6LRMj6QEA1XWchv7bDiZDQJGW9IBsdvzRiIKDj4 kG1oDZLQmlrWpHydjt0XCFka3v/aXW5PEvi8YGPMc5+kSzsAlypq+K23GS0rPH1c4IJ2 R8a0T1v9x0Ilwzvu5Q/jLpXT5joYQat3wrCK9xPyRqu12lt6HNdvDgAS0SVYtrnMWwc/ i+og== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date:dkim-signature; bh=7F2/kY3Yqztt+aVt1cySUkSVeLPI4mN2ZeroRuH8Uc0=; b=nYlOCNExloVSK6doWFDGxd2Kngv/VvNNKJmkLKOCmkb1NBOoeUfDHqTXsk3ohlT4V9 RNO5fCVqilkvDNouXj5cdobNX46qxW3Fd8KpvVF9g0gGTAekaxikbof4YKlOC8nwuzCo wufLOQrFUUo/65pkbxDSS/4cOyCkPiPR6f91FjJycyiBgiQgv7mKn5OzCl4k2R7UUPaY R3Cx0BWAE0RRCXVbqLgS7GCZx9uGnz1tiTWpNfgHRx48e6oA2aqb8I8p92NgG2jtygBS 7HkB0FbZCjiWD5FQuHpvizBO3tpdyAFmNxnJmNPYCIa2EroNTneNQIekK8Gmex9Nr9D5 a0iA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=ZcFpHVUs; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id he43si31012649ejc.718.2021.10.05.03.28.46; Tue, 05 Oct 2021 03:29:10 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@linuxfoundation.org header.s=korg header.b=ZcFpHVUs; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linuxfoundation.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233941AbhJEK3I (ORCPT + 99 others); Tue, 5 Oct 2021 06:29:08 -0400 Received: from mail.kernel.org ([198.145.29.99]:58634 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232658AbhJEK3C (ORCPT ); Tue, 5 Oct 2021 06:29:02 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 6C529610EA; Tue, 5 Oct 2021 10:27:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linuxfoundation.org; s=korg; t=1633429631; bh=f2XWKf2iAFj6A+NX/6JZz2MseITRKAzBcYBBcrKWIMM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=ZcFpHVUsocIb71TcBsvd4vA7ZMBcTMS36XsnH+C/YSaQ4ozoRfct2LpnBuZYSNtaT r2tf709bXF7/m9LaEYQigPsniPVAoCpRk7aDF5wmX0aQRevZQqvAaB8UWa81D1Mi2j 1du8VTd0md0ucnmWyu3FNlKzdDMFZWY+uRMXv+QY= Date: Tue, 5 Oct 2021 12:27:09 +0200 From: Greg Kroah-Hartman To: Nikita Yushchenko Cc: Lee Jones , linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org Subject: Re: [PATCH] staging: most: dim2: fix device registration Message-ID: References: <20210929205619.2800-1-nikita.yoush@cogentembedded.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210929205619.2800-1-nikita.yoush@cogentembedded.com> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 29, 2021 at 11:56:20PM +0300, Nikita Yushchenko wrote: > Commit 723de0f9171e ("staging: most: remove device from interface > structure") moved registration of driver-provided struct device to > the most subsystem, but did not properly update dim2 driver to > work with that change. > > After most subsystem passes driver's dev to register_device(), it > becomes refcounted, and can be only deallocated in the release method. > Provide that by: > - not using devres to allocate the device, > - moving shutdown actions from _remove() to the device release method, > - not calling shutdown actions in _probe() after the device becomes > refcounted. Should this be 3 patches? > Also, driver used to register it's dev itself, to provide a custom > attribute. With the modified most subsystem, this causes duplicate > registration of the same device object. Fix that by adding that custom > attribute to the platform device - that is a better location for > a platform-specific attribute anyway. Nope, it should be 4 patches. Whenever you have to list a bunch of different things you are doing in a single change, that's a hint that this should be more than one patch. Also, why have you not cc:ed the original author of the commit you are "fixing" here? They are the maintainer of this code, right? One note on your change that would keep me from accepting it even if all of the above was not an issue: > diff --git a/drivers/staging/most/dim2/sysfs.c b/drivers/staging/most/dim2/sysfs.c > index c85b2cdcdca3..22836c8ed554 100644 > --- a/drivers/staging/most/dim2/sysfs.c > +++ b/drivers/staging/most/dim2/sysfs.c > @@ -39,11 +39,10 @@ static const struct attribute_group *dev_attr_groups[] = { > > int dim2_sysfs_probe(struct device *dev) > { > - dev->groups = dev_attr_groups; > - return device_register(dev); > + return sysfs_create_groups(&dev->kobj, dev_attr_groups); No driver code should ever be calling a sysfs_* function, which is a huge hint that this is incorrect. You also just raced with userspace and lost, please use the default attributes for the driver or bus for this, but NEVER manually add and remove sysfs files, that way lies madness and hard to maintain code. thanks, greg k-h