| Message ID | 20240328122236.1718111-6-alexander.usyskin@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | spi: add driver for Intel discrete graphics | expand |
On 28/03/2024 13:22, Alexander Usyskin wrote: > Add auxiliary driver for intel discrete graphics > on-die spi device. > > CC: Rodrigo Vivi <rodrigo.vivi@intel.com> > Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com> > --- > drivers/spi/Kconfig | 11 +++ > drivers/spi/Makefile | 1 + > drivers/spi/spi-intel-dg.c | 146 +++++++++++++++++++++++++++++++++++++ Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC (and consider --no-git-fallback argument). It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts/get_maintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, instead use mainline), work on fork of kernel (don't, instead use mainline) or you ignore some maintainers (really don't). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. ... > + > + spi->base = devm_ioremap_resource(device, &ispi->bar); > + if (IS_ERR(spi->base)) { > + dev_err(device, "mmio not mapped\n"); > + ret = PTR_ERR(spi->base); > + goto err; > + } > + > + dev_set_drvdata(&aux_dev->dev, spi); > + > + dev_dbg(device, "bound\n"); Sorry, no. No silly function success messages. There is existing infrastructure for this. Drop. > + > + return 0; > + > +err: > + kref_put(&spi->refcnt, intel_dg_spi_release); > + return ret; > +} > + > +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); > + > + kref_put(&spi->refcnt, intel_dg_spi_release); > +} > + > +static const struct auxiliary_device_id intel_dg_spi_id_table[] = { > + { > + .name = "i915.spi", > + }, > + { > + .name = "xe.spi", > + }, > + { > + /* sentinel */ > + } > +}; > +MODULE_DEVICE_TABLE(auxiliary, intel_dg_spi_id_table); > + > +static struct auxiliary_driver intel_dg_spi_driver = { > + .probe = intel_dg_spi_probe, > + .remove = intel_dg_spi_remove, > + .driver = { > + /* auxiliary_driver_register() sets .name to be the modname */ > + }, > + .id_table = intel_dg_spi_id_table > +}; > + > +module_auxiliary_driver(intel_dg_spi_driver); > + > +MODULE_ALIAS("auxiliary:i915.spi"); > +MODULE_ALIAS("auxiliary:xe.spi"); You should not need MODULE_ALIAS() in normal cases. If you need it, usually it means your device ID table is wrong (e.g. misses either entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute for incomplete ID table. Best regards, Krzysztof
On Thu, Mar 28, 2024 at 02:22:28PM +0200, Alexander Usyskin wrote: >Add auxiliary driver for intel discrete graphics >on-die spi device. > >CC: Rodrigo Vivi <rodrigo.vivi@intel.com> >Signed-off-by: Tomas Winkler <tomas.winkler@intel.com> >Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> did you mean to Cc me? I never gave my s-o-b afair. The order of the s-o-b in these patches also seem to be messed up. Who is the author of this patch (and others)? Is it you or Tomas? If it's Tomas, then the commit in your tree is wrong and you have to fix it up so when you send the patch git adds a "From:" to the body. Otherwise, please fix the order. Lucas De Marchi
> > +static const struct auxiliary_device_id intel_dg_spi_id_table[] = { > > + { > > + .name = "i915.spi", > > + }, > > + { > > + .name = "xe.spi", > > + }, > > + { > > + /* sentinel */ > > + } > > +}; > > +MODULE_DEVICE_TABLE(auxiliary, intel_dg_spi_id_table); > > + > > +static struct auxiliary_driver intel_dg_spi_driver = { > > + .probe = intel_dg_spi_probe, > > + .remove = intel_dg_spi_remove, > > + .driver = { > > + /* auxiliary_driver_register() sets .name to be the modname */ > > + }, > > + .id_table = intel_dg_spi_id_table > > +}; > > + > > +module_auxiliary_driver(intel_dg_spi_driver); > > + > > +MODULE_ALIAS("auxiliary:i915.spi"); > > +MODULE_ALIAS("auxiliary:xe.spi"); > > You should not need MODULE_ALIAS() in normal cases. If you need it, > usually it means your device ID table is wrong (e.g. misses either > entries or MODULE_DEVICE_TABLE()). MODULE_ALIAS() is not a substitute > for incomplete ID table. > > > Best regards, > Krzysztof You are right, the auxiliary bus process aliases in the right way, this is remnants from the platform device usage, will drop.
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig index ddae0fde798e..d84896e6e7a0 100644 --- a/drivers/spi/Kconfig +++ b/drivers/spi/Kconfig @@ -507,6 +507,17 @@ config SPI_INTEL_PLATFORM To compile this driver as a module, choose M here: the module will be called spi-intel-platform. +config SPI_INTEL_DG + tristate "Intel Discrete Graphic SPI flash driver" + depends on AUXILIARY_BUS || COMPILE_TEST + depends on MTD + help + This enables support for Intel Discrete Graphic SPI + auxiliary device. + + To compile this driver as a module, choose M here: the module + will be called spi-intel-dg. + config SPI_JCORE tristate "J-Core SPI Master" depends on OF && (SUPERH || COMPILE_TEST) diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile index 4ff8d725ba5e..e158761d6844 100644 --- a/drivers/spi/Makefile +++ b/drivers/spi/Makefile @@ -69,6 +69,7 @@ obj-$(CONFIG_SPI_INGENIC) += spi-ingenic.o obj-$(CONFIG_SPI_INTEL) += spi-intel.o obj-$(CONFIG_SPI_INTEL_PCI) += spi-intel-pci.o obj-$(CONFIG_SPI_INTEL_PLATFORM) += spi-intel-platform.o +obj-$(CONFIG_SPI_INTEL_DG) += spi-intel-dg.o obj-$(CONFIG_SPI_LANTIQ_SSC) += spi-lantiq-ssc.o obj-$(CONFIG_SPI_JCORE) += spi-jcore.o obj-$(CONFIG_SPI_LJCA) += spi-ljca.o diff --git a/drivers/spi/spi-intel-dg.c b/drivers/spi/spi-intel-dg.c new file mode 100644 index 000000000000..8dc5448fc901 --- /dev/null +++ b/drivers/spi/spi-intel-dg.c @@ -0,0 +1,146 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright(c) 2019-2024, Intel Corporation. All rights reserved. + */ + +#include <linux/device.h> +#include <linux/intel_dg_spi_aux.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/string.h> +#include <linux/slab.h> +#include <linux/types.h> + +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[]; +}; + +static void intel_dg_spi_release(struct kref *kref) +{ + struct intel_dg_spi *spi = container_of(kref, struct intel_dg_spi, refcnt); + int i; + + pr_debug("freeing spi memory\n"); + for (i = 0; i < spi->nregions; i++) + kfree(spi->regions[i].name); + kfree(spi); +} + +static int intel_dg_spi_probe(struct auxiliary_device *aux_dev, + const struct auxiliary_device_id *aux_dev_id) +{ + struct intel_dg_spi_dev *ispi = auxiliary_dev_to_intel_dg_spi_dev(aux_dev); + struct device *device; + struct intel_dg_spi *spi; + unsigned int nregions; + unsigned int i, n; + size_t size; + char *name; + size_t name_size; + int ret; + + device = &aux_dev->dev; + + /* count available regions */ + for (nregions = 0, i = 0; i < INTEL_DG_SPI_REGIONS; i++) { + if (ispi->regions[i].name) + nregions++; + } + + if (!nregions) { + dev_err(device, "no regions defined\n"); + return -ENODEV; + } + + size = sizeof(*spi) + sizeof(spi->regions[0]) * nregions; + spi = kzalloc(size, GFP_KERNEL); + if (!spi) + return -ENOMEM; + + kref_init(&spi->refcnt); + + 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); + spi->regions[n].name = name; + spi->regions[n].id = i; + n++; + } + } + + spi->base = devm_ioremap_resource(device, &ispi->bar); + if (IS_ERR(spi->base)) { + dev_err(device, "mmio not mapped\n"); + ret = PTR_ERR(spi->base); + goto err; + } + + dev_set_drvdata(&aux_dev->dev, spi); + + dev_dbg(device, "bound\n"); + + return 0; + +err: + kref_put(&spi->refcnt, intel_dg_spi_release); + return ret; +} + +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); + + kref_put(&spi->refcnt, intel_dg_spi_release); +} + +static const struct auxiliary_device_id intel_dg_spi_id_table[] = { + { + .name = "i915.spi", + }, + { + .name = "xe.spi", + }, + { + /* sentinel */ + } +}; +MODULE_DEVICE_TABLE(auxiliary, intel_dg_spi_id_table); + +static struct auxiliary_driver intel_dg_spi_driver = { + .probe = intel_dg_spi_probe, + .remove = intel_dg_spi_remove, + .driver = { + /* auxiliary_driver_register() sets .name to be the modname */ + }, + .id_table = intel_dg_spi_id_table +}; + +module_auxiliary_driver(intel_dg_spi_driver); + +MODULE_ALIAS("auxiliary:i915.spi"); +MODULE_ALIAS("auxiliary:xe.spi"); +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Intel Corporation"); +MODULE_DESCRIPTION("Intel DGFX SPI driver");