[PATCH v6 01/12] spi: add driver for intel graphics on-die spi device

[Mark Brown]

On Mon, Sep 16, 2024 at 04:49:17PM +0300, Alexander Usyskin wrote:

> Add auxiliary driver for intel discrete graphics
> on-die spi device.

This doesn't actually do anything AFAICT?  It doesn't register with any
subsystem or anything.  Please don't submit empty boilerplate like this,
submit a driver that is at least minimally useful - assuming some other
patches in the series add functionality squash at least a basic set of
functionality into this.  This makes review and test easier.

> +++ b/drivers/spi/spi-intel-dg.c
> @@ -0,0 +1,142 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright(c) 2019-2024, Intel Corporation. All rights reserved.
> + */

Please make the entire comment a C++ one so things look more
intentional.

> +struct intel_dg_spi {
> +	struct kref refcnt;
> +	void __iomem *base;
> +	size_t size;
> +	unsigned int nregions;
> +	struct {
> +		const char *name;
> +		u8 id;
> +		u64 offset;
> +		u64 size;
> +	} regions[];

Use __counted_by() for the benefit of the static checkers.

> +	size = sizeof(*spi) + sizeof(spi->regions[0]) * nregions;
> +	spi = kzalloc(size, GFP_KERNEL);

Use at least array_size().

> +	kref_init(&spi->refcnt);

This refcount feels more complex than just freeing in the error and
release paths, it's not a common pattern for drivers.

> +	spi->nregions = nregions;
> +	for (n = 0, i = 0; i < INTEL_DG_SPI_REGIONS; i++) {
> +		if (ispi->regions[i].name) {
> +			name_size = strlen(dev_name(&aux_dev->dev)) +
> +				    strlen(ispi->regions[i].name) + 2; /* for point */
> +			name = kzalloc(name_size, GFP_KERNEL);
> +			if (!name)
> +				continue;
> +			snprintf(name, name_size, "%s.%s",
> +				 dev_name(&aux_dev->dev), ispi->regions[i].name);

kasprintf().

> +static void intel_dg_spi_remove(struct auxiliary_device *aux_dev)
> +{
> +	struct intel_dg_spi *spi = dev_get_drvdata(&aux_dev->dev);
> +
> +	if (!spi)
> +		return;
> +
> +	dev_set_drvdata(&aux_dev->dev, NULL);

If the above is doing anything there's a problem...
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-xe/attachments/20240918/35ddb87f/attachment.sig>