Re: [PATCH] Infineon TPM: move infineon driver off pci_dev

[matthieu castet <castet.matthieu@free.fr>]

Hi,

Marcel Selhorst wrote:
 > Dear all,
 >
 > the following patch moves the Infineon TPM driver off pci device
 > and makes it a pure pnp-driver. It was tested with IFX0101 and
 > IFX0102 and is now based on the new tpm patchset (1 to 5) from Kylene
 > Hall submitted two days ago. It now also includes pnp-port validation
 > and region requesting.
 >
 > Best regards,
 >
 > Marcel Selhorst
 >
 > Signed-off-by: Marcel Selhorst <selhorst@crypto.rub.de>
 > ---
 >

 > +	/* read IO-ports through PnP */
 > +	if (pnp_port_valid(dev, 0) && pnp_port_valid(dev, 1) &&
 > +	    !(pnp_port_flags(dev, 0) & IORESOURCE_DISABLED)) {
&& !(pnp_port_flags(dev, 1) & IORESOURCE_DISABLED)

 > +		TPM_INF_ADDR = pnp_port_start(dev, 0);
 > +		TPM_INF_DATA = (TPM_INF_ADDR + 1);
 > +		TPM_INF_BASE = pnp_port_start(dev, 1);
 > +		TPM_INF_PORT_LEN = pnp_port_len(dev, 1);
 > +		if (!TPM_INF_PORT_LEN)
 > +			return -EINVAL;
When I said to check the lenght I was thinking about
if (pnp_port_len(dev, 1) < 4) return -EINVAL;
According to infineon_tpm_register, the length is at least 4.

I whould also check that pnp_port_len(dev, 0) < 2.





 > +		dev_info(&dev->dev, "Found %s with ID %s\n",
 > +			 dev->name, dev_id->id);
 > +		if (!((TPM_INF_BASE >> 8) & 0xff))
 > +			return -EINVAL;
 > +		/* publish my base address and request region */
 > +		tpm_inf.base = TPM_INF_BASE;
 > +		if (request_region
 > +		    (tpm_inf.base, TPM_INF_PORT_LEN, "tpm_infineon0") == NULL) {
 > +			release_region(tpm_inf.base, TPM_INF_PORT_LEN);
if it failed, you don't need to release the region

a request_region (TPM_INF_ADDR, 2, "tpm_infineon0") could be usefull.

 > +			return -EINVAL;
 > +		}
 >  	} else {