Subject: Re: [PATCH linux] drivers/fsi: Add SBEFIFO FSI client device driver
To: gregkh@linuxfoundation.org
References: <6363b930-6a69-daf7-c1b5-b1e5ccbc5170@linux.vnet.ibm.com>
From: Eddie James <eajames@linux.vnet.ibm.com>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
        Brad Bishop <bradleyb@fuzziesquirrel.com>, Jeremy Kerr <jk@ozlabs.org>,
        Christopher Bostic <cbostic@linux.vnet.ibm.com>,
        Joel Stanley <joel@jms.id.au>, Andrew Jeffery <andrew@aj.id.au>
Date: Tue, 20 Jun 2017 11:34:41 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.1.0
MIME-Version: 1.0
In-Reply-To: <6363b930-6a69-daf7-c1b5-b1e5ccbc5170@linux.vnet.ibm.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Content-Language: en-US
Message-Id: <930fe55f-bdf1-a2b7-a96b-d9077a3fc302@linux.vnet.ibm.com>
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1354
Lines: 41


On 06/15/2017 5:24 AM, gregkh@linuxfoundation.org 
<https://patchwork.kernel.org/project/LKML/list/?submitter=37061> wrote:
> On Wed, Jun 14, 2017 at 02:47:27PM -0500, Edward A. James wrote:
>> +struct sbefifo {
>> +    struct timer_list poll_timer;
>> +    struct fsi_device *fsi_dev;
>> +    struct miscdevice mdev;
>> +    wait_queue_head_t wait;
>> +    struct list_head link;
>> +    struct list_head xfrs;
>> +    struct kref kref;
>> +    spinlock_t lock;
>> +    char name[32];
>> +    int idx;
>> +    int rc;
>> +};
>
> You have a misc device, a pointer to a fsi_device, and a kref in this
> structure.  Which one actually does the reference counting?  It seems
> there are 3 different ways it could happen.  That's not right at all,
> and ripe for lots and lots of confusion.  Only use one please.

Thanks for the comments, the problem here is that there is no way to 
init the miscdevice device kobject with a kobj_type. So we'd have to 
just do raw kref get/put there so we can get our release function. Does 
any other linux driver use miscdevice for reference counting? I couldn't 
find anything. It seems cleaner to me to have our own kref. The fsi_dev 
is really a parent device, so we shouldn't use that for reference counting.

What do you think? I can add comments to make things clearer?

Thanks,
Eddie

>
> thanks,
>
> greg k-h
>