RE: [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

Usyskin, Alexander
2023-09-12T10:50:42Z

>
> > Add driver for access to the discrete graphics card
> > internal SPI device.
> > Expose device on auxiliary bus and provide driver to register
> > this device with MTD framework.
>
> Maybe you can explain why you think auxiliary bus is relevant here? The
> cover letter might maybe be a bit more verbose to give us more context?
>
> I've looked at the series, it looks like you try to expose a spi
> memory connected to a spi controller on your hardware. We usually
> expect the spi controller driver to register in the spi core and
> provide spi-mem operations for that.
>
> I don't know if this memory is supposed to be used as general purpose,
> but if it's not I would advise to use some kind of firmware mechanism
> instead. Also, what is the purpose of exposing this content in this
> case?
>
> Well, I'm partially convinced here, I would like to hear from the other
> maintainers, maybe your choices are legitimate and I'm off topic.
>
> Thanks,
> Miquèl
>

The main purpose of creating this driver is to allow device manufacturing and recovery.
In these flows the firmware may be unavailable and direct spi access is the only way.
There is a use-case that require another kernel driver to access the spi partitions directly.
This use-case is not upstreamed yet.

The spi controller on discreet graphics card is not visible to user-space.
Spi access flows are supported by another hardware module and relevant registers are
available on graphics device memory bar.

Auxiliary bus device is created to separate spi code and flows from already overloaded i915 driver.

--
Thanks,
Sasha

> > This series is intended to be upstreamed through drm tree.
> >
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> >
> >
> > Alexander Usyskin (3):
> > drm/i915/spi: align 64bit read and write
> > drm/i915/spi: wake card on operations
> > drm/i915/spi: add support for access mode
> >
> > Jani Nikula (1):
> > drm/i915/spi: add spi device for discrete graphics
> >
> > Tomas Winkler (6):
> > drm/i915/spi: add intel_spi_region map
> > drm/i915/spi: add driver for on-die spi device
> > drm/i915/spi: implement region enumeration
> > drm/i915/spi: implement spi access functions
> > drm/i915/spi: spi register with mtd
> > drm/i915/spi: mtd: implement access handlers
> >
> > drivers/gpu/drm/i915/Kconfig | 1 +
> > drivers/gpu/drm/i915/Makefile | 6 +
> > drivers/gpu/drm/i915/i915_driver.c | 7 +
> > drivers/gpu/drm/i915/i915_drv.h | 4 +
> > drivers/gpu/drm/i915/i915_reg.h | 1 +
> > drivers/gpu/drm/i915/spi/intel_spi.c | 101 +++
> > drivers/gpu/drm/i915/spi/intel_spi.h | 33 +
> > drivers/gpu/drm/i915/spi/intel_spi_drv.c | 865
> +++++++++++++++++++++++
> > 8 files changed, 1018 insertions(+)
> > create mode 100644 drivers/gpu/drm/i915/spi/intel_spi.c
> > create mode 100644 drivers/gpu/drm/i915/spi/intel_spi.h
> > create mode 100644 drivers/gpu/drm/i915/spi/intel_spi_drv.c
>
> Thanks,
> Miquèl