Re: PATCH 6/7] tpm: new 1.2 sysfs files

[Greg KH <greg@kroah.com>]

On Mon, Apr 03, 2006 at 11:42:41AM -0500, Kylene Jo Hall wrote:
> +ssize_t tpm_show_state(struct device * dev, struct device_attribute * attr,
> +		       char *buf)
> +{
> +	u8 data[35];
> +	ssize_t len;
> +	char *str = buf;
> +
> +	struct tpm_chip *chip = dev_get_drvdata(dev);
> +	if (chip == NULL)
> +		return -ENODEV;
> +
> +	memcpy(data, tpm_cap, sizeof(tpm_cap));
> +	data[TPM_CAP_IDX] = TPM_CAP_FLAG;
> +	data[TPM_CAP_SUBCAP_IDX] = TPM_CAP_FLAG_PERM;
> +
> +	if ((len = tpm_transmit(chip, data, sizeof(data)))
> +	    <= TPM_ERROR_SIZE)
> +		dev_dbg(chip->dev, "A TPM error (%d) occurred "
> +			"attempting to determine the permanent state\n",
> +			be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
> +	else {
> +		str +=
> +		    sprintf(str, "%s\n",
> +			    (data[TPM_GET_CAP_PERM_DISABLE_IDX] ==
> +			     1) ? "Disabled" : "Enabled");
> +		str +=
> +		    sprintf(str, "%s\n",
> +			    (data[TPM_GET_CAP_PERM_INACTIVE_IDX] ==
> +			     1) ? "Inactive" : "Active");
> +	}
> +	memcpy(data, tpm_cap, sizeof(tpm_cap));
> +	data[TPM_CAP_IDX] = TPM_CAP_PROP;
> +	data[TPM_CAP_SUBCAP_IDX] = TPM_CAP_PROP_OWNER;
> +
> +	if ((len = tpm_transmit(chip, data, sizeof(data)))
> +	    <= TPM_ERROR_SIZE)
> +		dev_dbg(chip->dev, "A TPM error (%d) occurred "
> +			"attempting to determine the owner state\n",
> +			be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
> +	else
> +		str +=
> +		    sprintf(str, "%s\n",
> +			    (data[TPM_GET_CAP_RET_BOOL_1_IDX] ==
> +			     1) ? "Owned" : "Unowned");
> +
> +	memcpy(data, tpm_cap, sizeof(tpm_cap));
> +	data[TPM_CAP_IDX] = TPM_CAP_FLAG;
> +	data[TPM_CAP_SUBCAP_IDX] = TPM_CAP_FLAG_VOL;
> +
> +	if ((len = tpm_transmit(chip, data, sizeof(data)))
> +	    <= TPM_ERROR_SIZE) {
> +		dev_dbg(chip->dev, "A TPM error (%d) occurred "
> +			"attempting to determine the temporary state\n",
> +			be32_to_cpu(*((__be32 *) (data + TPM_RET_CODE_IDX))));
> +		goto out;
> +	}
> +
> +	if (data[TPM_GET_CAP_TEMP_INACTIVE_IDX] != 0)
> +		str += sprintf(str, "Deactivated for this boot\n");
> +out:
> +	return str - buf;
> +}
> +EXPORT_SYMBOL_GPL(tpm_show_state);

That is more than one value per file.  Please make unique files for the
different capabilities.  As it stands the file doesn't make too much
sense for someone reading it and not understanding that each line is a
different portion of the state.

thanks,

greg k-h