Re: [RFC PATCH 2/9] drm/i915/spi: intel_spi_region map

From: Jani Nikula <jani.nikula@linux.intel.com>
To: Tomas Winkler <tomas.winkler@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>,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Alexander Usyskin <alexander.usyskin@intel.com>,
	intel-gfx@lists.freedesktop.org,
	Lucas De Marchi <lucas.demarchi@intel.com>,
	linux-mtd@lists.infradead.org,
	Tomas Winkler <tomas.winkler@intel.com>,
	Vitaly Lubart <vitaly.lubart@intel.com>
Subject: Re: [RFC PATCH 2/9] drm/i915/spi: intel_spi_region map
Date: Wed, 17 Feb 2021 12:46:22 +0200	[thread overview]
Message-ID: <87eehfas5d.fsf@intel.com> (raw)
In-Reply-To: <20210216181925.650082-3-tomas.winkler@intel.com>

On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler@intel.com> wrote:
> Add the dGFX spi region map and convey it via mfd cell platform data
> to the spi child device.
>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
> ---
>  drivers/gpu/drm/i915/spi/intel_spi.c | 9 +++++++++
>  drivers/gpu/drm/i915/spi/intel_spi.h | 5 +++++
>  2 files changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/spi/intel_spi.c b/drivers/gpu/drm/i915/spi/intel_spi.c
> index 07da7197bd5d..6f83f24f7208 100644
> --- a/drivers/gpu/drm/i915/spi/intel_spi.c
> +++ b/drivers/gpu/drm/i915/spi/intel_spi.c
> @@ -14,11 +14,20 @@ static const struct resource spi_resources[] = {
>  	DEFINE_RES_MEM_NAMED(GEN12_GUNIT_SPI_BASE, 0x80, "i915-spi-mmio"),
>  };
>  
> +static const struct i915_spi_region regions[I915_SPI_REGIONS] = {
> +	[0] = { .name = "DESCRIPTOR", },
> +	[2] = { .name = "GSC", },
> +	[11] = { .name = "OptionROM", },
> +	[12] = { .name = "DAM", },
> +};
> +
>  static const struct mfd_cell intel_spi_cell = {
>  	.id = 2,
>  	.name = "i915-spi",
>  	.num_resources = ARRAY_SIZE(spi_resources),
>  	.resources = spi_resources,
> +	.platform_data = (void *)regions,
> +	.pdata_size    = sizeof(regions),
>  };
>  
>  void intel_spi_init(struct intel_spi *spi, struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/spi/intel_spi.h b/drivers/gpu/drm/i915/spi/intel_spi.h
> index 276551fed993..6b5bf053f7d3 100644
> --- a/drivers/gpu/drm/i915/spi/intel_spi.h
> +++ b/drivers/gpu/drm/i915/spi/intel_spi.h
> @@ -8,6 +8,11 @@
>  
>  struct drm_i915_private;
>  
> +#define I915_SPI_REGIONS 13
> +struct i915_spi_region {
> +	const char *name;
> +};

Does this need to be exposed to the rest of i915? If we're trying to
isolate spi/, I'd prefer it if this header was the only header included
from the rest of i915, and contained the minimum required information.

As the driver has grown bigger, we've tried to minimize the
interconnections between the modules, and it's slow going. Let's try to
keep the new parts isolated.

BR,
Jani.

> +
>  struct intel_spi {
>  	struct drm_i915_private *i915;
>  };

-- 
Jani Nikula, Intel Open Source Graphics Center

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/