On Fri, Nov 03, 2017 at 04:30:09PM -0700, Azhar Shaikh wrote:
> @@ -413,6 +413,8 @@ ssize_t tpm_transmit(struct tpm_chip *chip, struct tpm_space *space,
> if (chip->dev.parent)
> pm_runtime_get_sync(chip->dev.parent);
>
> + chip->ops->clk_toggle(chip, true);
You added this new op to the general code, but only updated two
drivers, surely this makes all the other tis drivers oops as
clk_toggle will be NULL?
Suggest checking for NULL before calling.
> +#ifdef CONFIG_X86
This is a good place to use IS_ENABLED instead of ifdef:
> +/**
> + * tpm_tis_clkrun_toggle() - Keep clkrun protocol disabled for entire duration
> + * of a single TPM command
> + * @chip: TPM chip to use
> + * @value: 1 - Disable CLKRUN protocol, so that clocks are free running
> + * 0 - Enable CLKRUN protocol
> + */
> +static void tpm_tis_clkrun_toggle(struct tpm_chip *chip, bool value)
> +{
> + struct tpm_tis_data *data = dev_get_drvdata(&chip->dev);
if (!IS_ENABLED(CONFIG_X86))
return;
> +
> + if (value) {
> + tpm_platform_begin_xfer(data);
> + data->flags |= TPM_TIS_CLK_ENABLE;
> + } else {
> + data->flags &= ~TPM_TIS_CLK_ENABLE;
> + tpm_platform_end_xfer(data);
> + }
> +}
> +#else
> +static void tpm_tis_clkrun_toggle(struct tpm_chip *chip, bool value)
> +{
> +}
> +#endif
> +#ifdef CONFIG_X86
> +void tpm_platform_begin_xfer(struct tpm_tis_data *data);
> +void tpm_platform_end_xfer(struct tpm_tis_data *data);
> +#else
> +void tpm_platform_begin_xfer(struct tpm_tis_data *data)
> +{
> +}
> +void tpm_platform_end_xfer(struct tpm_tis_data *data)
> +{
> +}
These empty stubs need inlines
Why are you using a mixture of callbacks and linked functions to solve
this problem?
Can't you do everything with callbacks?
Jason
