Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757624AbcLRIC7 (ORCPT ); Sun, 18 Dec 2016 03:02:59 -0500 Received: from mail.linuxfoundation.org ([140.211.169.12]:45702 "EHLO mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751532AbcLRIC6 (ORCPT ); Sun, 18 Dec 2016 03:02:58 -0500 Date: Sat, 17 Dec 2016 23:51:41 -0800 From: Greg Kroah-Hartman To: Logan Gunthorpe Cc: Bjorn Helgaas , Geert Uytterhoeven , Jonathan Corbet , "David S. Miller" , Andrew Morton , Emil Velikov , Mauro Carvalho Chehab , Guenter Roeck , Kurt Schwemmer , Stephen Bates , linux-pci@vger.kernel.org, linux-doc@vger.kernel.org, linux-nvme@lists.infradead.org, linux-kernel@vger.kernel.org Subject: Re: [RFC 1/1] MicroSemi Switchtec management interface driver Message-ID: <20161218075141.GE29850@kroah.com> References: <1481994562-9283-1-git-send-email-logang@deltatee.com> <1481994562-9283-2-git-send-email-logang@deltatee.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1481994562-9283-2-git-send-email-logang@deltatee.com> User-Agent: Mutt/1.7.2 (2016-11-26) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1240 Lines: 42 On Sat, Dec 17, 2016 at 10:09:22AM -0700, Logan Gunthorpe wrote: > +struct switchtec_dev { > + struct pci_dev *pdev; > + struct msix_entry *msix; > + struct device *dev; > + struct kref kref; Why do you have a pointer to a device, yet a kref as well? Just have this structure embed a 'struct device' in itself, like you did for a kref, and you will be fine. Otherwise you are dealing with two different sets of reference counting here, for no good reason. > + > + unsigned int event_irq; > + > + void __iomem *mmio; > + struct mrpc_regs __iomem *mmio_mrpc; > + struct ntb_info_regs __iomem *mmio_ntb; > + struct part_cfg_regs __iomem *mmio_part_cfg; > + > + /* > + * The mrpc mutex must be held when accessing the other > + * mrpc fields > + */ > + struct mutex mrpc_mutex; > + struct list_head mrpc_queue; > + int mrpc_busy; > + struct work_struct mrpc_work; > + struct delayed_work mrpc_timeout; > +}; > + > +#define stdev_pdev(stdev) ((stdev)->pdev) > +#define stdev_pdev_dev(stdev) (&stdev_pdev(stdev)->dev) > +#define stdev_name(stdev) pci_name(stdev_pdev(stdev)) > +#define stdev_dev(stdev) ((stdev)->dev) Ick, just open code these please. That's a huge hint your use of the driver model is not correct :) thanks, greg k-h