From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1424170AbdEYN2x (ORCPT <rfc822;w@1wt.eu>);
        Thu, 25 May 2017 09:28:53 -0400
Received: from mail.linuxfoundation.org ([140.211.169.12]:41648 "EHLO
        mail.linuxfoundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1424155AbdEYN2p (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 25 May 2017 09:28:45 -0400
Date: Thu, 25 May 2017 15:28:34 +0200
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Andreas Noever <andreas.noever@gmail.com>,
        Michael Jamet <michael.jamet@intel.com>,
        Yehezkel Bernat <yehezkel.bernat@intel.com>,
        Lukas Wunner <lukas@wunner.de>, Amir Levy <amir.jer.levy@intel.com>,
        Andy Lutomirski <luto@kernel.org>, Mario.Limonciello@dell.com,
        Jared.Dominguez@dell.com,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH 22/24] thunderbolt: Add support for host and device NVM
 firmware upgrade
Message-ID: <20170525132834.GJ16244@kroah.com>
References: <20170518143914.60902-1-mika.westerberg@linux.intel.com>
 <20170518143914.60902-23-mika.westerberg@linux.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20170518143914.60902-23-mika.westerberg@linux.intel.com>
User-Agent: Mutt/1.8.2 (2017-04-18)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, May 18, 2017 at 05:39:12PM +0300, Mika Westerberg wrote:
> +static int tb_switch_nvm_add(struct tb_switch *sw)
> +{
> +	struct nvmem_device *nvm_dev;
> +	struct tb_switch_nvm *nvm;
> +	u32 val;
> +	int ret;
> +
> +	if (!sw->dma_port)
> +		return 0;
> +
> +	nvm = kzalloc(sizeof(*nvm), GFP_KERNEL);
> +	if (!nvm)
> +		return -ENOMEM;
> +
> +	nvm->id = ida_simple_get(&nvm_ida, 0, 0, GFP_KERNEL);
> +
> +	/*
> +	 * If the switch is in safe-mode the only accessible portion of
> +	 * the NVM is the non-active one where userspace is expected to
> +	 * write new functional NVM.
> +	 */
> +	if (!sw->safe_mode) {
> +		u32 nvm_size, hdr_size;
> +
> +		ret = dma_port_flash_read(sw->dma_port, NVM_FLASH_SIZE, &val,
> +					  sizeof(val));
> +		if (ret)
> +			goto err_ida;
> +
> +		hdr_size = sw->generation < 3 ? SZ_8K : SZ_16K;
> +		nvm_size = (SZ_1M << (val & 7)) / 8;
> +		nvm_size = (nvm_size - hdr_size) / 2;
> +
> +		ret = dma_port_flash_read(sw->dma_port, NVM_VERSION, &val,
> +					  sizeof(val));
> +		if (ret)
> +			goto err_ida;
> +
> +		nvm->major = val >> 16 & 0xff;
> +		nvm->minor = val >> 8 & 0xff;
> +
> +		nvm_dev = register_nvmem(sw, nvm->id, nvm_size, true);
> +		if (IS_ERR(nvm_dev)) {
> +			ret = PTR_ERR(nvm_dev);
> +			goto err_ida;
> +		}
> +		nvm->active = nvm_dev;
> +	}
> +
> +	nvm_dev = register_nvmem(sw, nvm->id, NVM_MAX_SIZE, false);
> +	if (IS_ERR(nvm_dev)) {
> +		ret = PTR_ERR(nvm_dev);
> +		goto err_nvm_active;
> +	}
> +	nvm->non_active = nvm_dev;
> +
> +	sw->nvm = nvm;
> +
> +	ret = sysfs_create_group(&sw->dev.kobj, &nvm_group);

Why are you adding this to the sw device?  And doing so _after_ it was
announced to userspace?  Why can't you make it part of the device's
default groups so that the driver core can handle it properly?

Hint, if you ever have to call sysfs_* from within a driver, something
might really be wrong :)

thanks,

greg k-h