On Mon, May 7, 2018 at 4:58 AM, Oliver Neukum <oneukum@xxxxxxxx> wrote:
>
> Am Freitag, den 04.05.2018, 08:00 -0500 schrieb David R. Bild :
> >
> > +config USB_XAPEA00X
> > + tristate "Xaptum ENF Access card support (XAP-EA-00x)"
> > + depends on USB_SUPPORT && SPI && TCG_TPM
>
> You'd have to know how the device internally works. It would be better
> to select SPI.
That was my logic to Greg too. I'll change the SPI back to select.
> [..]
> > +/**
> > + * xapea00x_spi_probe - Register and configure the SPI master.
> > + * @dev: the device whose SPI master to register
> > + *
> > + * Return: If successful, 0. Otherwise a negative error number.
> > + */
> > +static int xapea00x_spi_probe(struct xapea00x_device *dev)
> > +{
> > + struct spi_master *spi_master;
> > + int retval;
> > +
> > + spi_master = spi_alloc_master(&dev->interface->dev, sizeof(void *));
> > + if (!spi_master) {
> > + retval = -ENOMEM;
> > + goto err_out;
> > + }
> > +
> > + spi_master_set_devdata(spi_master, dev);
> > +
> > + spi_master->min_speed_hz = 93 * 1000 + 800; /* 93.9kHz */
> > + spi_master->max_speed_hz = 12 * 1000 * 1000; /* 12 MHz */
> > +
> > + spi_master->bus_num = -1; /* dynamically assigned */
> > + spi_master->num_chipselect = 1;
> > + spi_master->mode_bits = SPI_MODE_0;
> > +
> > + spi_master->flags = 0;
> > + spi_master->setup = xapea00x_spi_setup;
> > + spi_master->transfer_one_message = xapea00x_spi_transfer_one_message;
> > +
> > + retval = spi_register_master(spi_master);
> > +
> > + if (retval)
> > + goto free_spi;
> > +
> > + dev->spi_master = spi_master;
>
> Race condition.
>
What race condition do you see? (I appreciate the review, but need
some more specific help here.)
>
> > +
> > + return 0;
> > +
> > +free_spi:
> > + spi_master_put(spi_master);
> > + dev->spi_master = NULL;
> > +
> > +err_out:
> > + return retval;
> > +}
> > +
>
> > +
> > +static struct spi_board_info tpm_board_info = {
> > + .modalias = XAPEA00X_TPM_MODALIAS,
> > + .max_speed_hz = 43 * 1000 * 1000, // Hz
>
> Are you hardcoding HZ ?
Yes. But it's the max speed of the SPI clock which is actually
specified in real HZ. It's not kernel "HZ".
Thanks for the other comments too; I'll incorporate into the next version.
Best,
David
