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

[Mark Brown]

On Thu, Sep 19, 2024 at 09:54:24AM +0000, Winkler, Tomas wrote:
> > On Mon, Sep 16, 2024 at 04:49:17PM +0300, Alexander Usyskin wrote:

> > > @@ -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.

> This is how it is required by Linux spdx checker, 

There is no incompatibility between SPDX and what I'm asking for...

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

> > Use at least array_size().

> Regions is not fixed size array, it will not work.

Yes, that's the wrong helper - there is a relevent one though which I'm
not remembering right now.

> > > +	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.

> What are you suggesting

Just do normal open coded allocations, the reference counting is just
obscure.

> > > +		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().

> As I understand kasprintf allocates memory, this is not required here.

There is a kzalloc() in the above code block?

> > > +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...
o
> It makes sure the data is set to NULL.

Which is needed because...?
-------------- 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-gfx/attachments/20240919/0ce97361/attachment-0001.sig>