Re: [Intel-gfx] [RFC PATCH 0/9] drm/i915/spi: discrete graphics internal spi

From: "Winkler, Tomas" <tomas.winkler@intel.com>
To: Jani Nikula <jani.nikula@linux.intel.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	Joonas Lahtinen <joonas.lahtinen@linux.intel.com>,
	"Vivi, Rodrigo" <rodrigo.vivi@intel.com>
Cc: "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Usyskin, Alexander" <alexander.usyskin@intel.com>,
	"Lubart, Vitaly" <vitaly.lubart@intel.com>
Subject: Re: [Intel-gfx] [RFC PATCH 0/9] drm/i915/spi: discrete graphics internal spi
Date: Wed, 17 Feb 2021 13:56:56 +0000	[thread overview]
Message-ID: <c430158c27c5460ba13a989c6f9646d7@intel.com> (raw)
In-Reply-To: <878s7narev.fsf@intel.com>

> 
> On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler@intel.com> wrote:
> > Intel discrete graphic devices have internal spi storage, that holds
> > firmware and oprom images. The spi device is exposed to the user space
> > via mtd framework to be accessed during manufacturing.
> > The device is hardware locked after manufacturing and only read access
> > is provided.
> >
> > The i915 plays role of a multi function device (mfd) and spi device is
> > exposed as its child device. i915_spi platform driver binds to this
> > device.
> >
> > Because the graphic card may undergo reset at any time and basically
> > hot unplug all its child devices, this series also provides a fix to
> > the mtd framework to make the reset graceful.
> >
> > Tomas Winkler (9):
> >   drm/i915/spi: add spi device for discrete graphics
> >   drm/i915/spi: 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
> >   drm/i915/spi: serialize spi access
> >   mtd: use refcount to prevent corruption
> >
> >  drivers/gpu/drm/i915/Kconfig             |   3 +
> >  drivers/gpu/drm/i915/Makefile            |   6 +
> >  drivers/gpu/drm/i915/i915_drv.c          |   9 +
> >  drivers/gpu/drm/i915/i915_drv.h          |   4 +
> >  drivers/gpu/drm/i915/i915_reg.h          |   1 +
> >  drivers/gpu/drm/i915/spi/intel_spi.c     |  62 +++
> >  drivers/gpu/drm/i915/spi/intel_spi.h     |  24 +
> 
> I'm open to discussion, but after glancing through the series I've got a gut
> feeling spi/ subdir should be purely about the separate module, and the
> above two files should be in i915/ directory instead.

Maybe, I don't have strong feelings about that, it is just a decision from which point you want to look at that.
> 
> As it is, I think it's a bit confusing that spi/ is both about the spi kernel module
> and a singly .c file that's really part of i915.ko. Perhaps that messes up the
> conventional descending to subdirs in the kernel build too?

The intention was to make this capsulated from the file system point of view. 
In general the spi driver could be somewhere in mtd directory, but it doesn't really fit exactly there either.
I don't have a strong opinion about that, if you do I yield. 

Thanks
Tomas

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx