Date: Tue, 1 Dec 2015 12:44:19 -0700
From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
To: Uwe =?iso-8859-1?Q?Kleine-K=F6nig?= 
	<u.kleine-koenig@pengutronix.de>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
        tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org,
        Martin Wilck <Martin.Wilck@ts.fujitsu.com>,
        Peter Huewe <peterhuewe@gmx.de>
Subject: Re: [PATCH v2 2/3] tpm_tis: Use devm_ioremap_resource
Message-ID: <20151201194419.GB16123@obsidianresearch.com>
References: <1448996309-15220-1-git-send-email-jgunthorpe@obsidianresearch.com>
 <1448996309-15220-3-git-send-email-jgunthorpe@obsidianresearch.com>
 <20151201192240.GE5072@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20151201192240.GE5072@pengutronix.de>
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1481
Lines: 46

On Tue, Dec 01, 2015 at 08:22:40PM +0100, Uwe Kleine-K?nig wrote:

> here, which IMHO reads nicer and maybe is even more efficient (I don't
> know much about x86).

Sure

> > +	if (resource_size(&tpm_info.res) == 0)
> > +		return -ENODEV;
> > +
> 
> Does this result in an error message from the upper layers?

I think so, yes. The probe will fail which causes the driver core to
report a message.

The scenario this triggers is if the acpi stuff doesn't have a mem
resource, which is a firmware bug, I think. It could get a dedicated
print if that is what you are thinking?

-	if (resource_size(&tpm_info.res) == 0)
+	if (tpm_info.res.flags == 0) {
+		dev_err(&pdev->dev, FW_BUG "no memory resource defined\n");
 		return -ENODEV;
+	}
 
 	if (is_itpm(acpi_dev))
 		itpm = true;

[resource_size is wrong as well since it will return 1 for
 0'd struct resource, sigh..]

Previously it would try to call devm_ioremap with start/len=0 as the
range which should also fails in broadly the same way. So this is just
moving the existing failure up.

Something was needed because the change to struct resource means the
new code would call devm_ioremap_resource with a 0'd resource struct,
which is not as safe as start/len=0 as before.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/