diff mbox series

[RFC,3/9] drm/i915/spi: add driver for on-die spi device

Message ID 20210216181925.650082-4-tomas.winkler@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/spi: discrete graphics internal spi | expand

Commit Message

Winkler, Tomas Feb. 16, 2021, 6:19 p.m. UTC
Add the platform driver for i915 on-die spi device, exposed via mfd
framework.

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/Kconfig             |   2 +
 drivers/gpu/drm/i915/Makefile            |   3 +
 drivers/gpu/drm/i915/spi/intel_spi_drv.c | 116 +++++++++++++++++++++++
 3 files changed, 121 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/spi/intel_spi_drv.c

Comments

Jani Nikula Feb. 17, 2021, 10:56 a.m. UTC | #1
On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler@intel.com> wrote:
> Add the platform driver for i915 on-die spi device, exposed via mfd
> framework.
>
> 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/Kconfig             |   2 +
>  drivers/gpu/drm/i915/Makefile            |   3 +
>  drivers/gpu/drm/i915/spi/intel_spi_drv.c | 116 +++++++++++++++++++++++
>  3 files changed, 121 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/spi/intel_spi_drv.c
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index abcaa8da45ac..13c870e5878e 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -27,6 +27,8 @@ config DRM_I915
>  	select CEC_CORE if CEC_NOTIFIER
>  	select VMAP_PFN
>  	select MFD_CORE
> +	select MTD

Selecting MTD does not seem to be a popular thing to do, which is
usually a clue it's probably the wrong thing to do.

> +	select MTD_PARTITIONED_MASTER
>  	help
>  	  Choose this option if you have a system that has "Intel Graphics
>  	  Media Accelerator" or "HD Graphics" integrated graphics,
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 7fa9120feb8d..f209cd541eec 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -299,6 +299,9 @@ endif
>  obj-$(CONFIG_DRM_I915) += i915.o
>  obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
>  
> +obj-m += i915_spi.o

And literally nobody does this.

> +i915_spi-y := spi/intel_spi_drv.o

Time to add a separate Kconfig and Makefile in spi/?

BR,
Jani.

> +
>  # header test
>  
>  # exclude some broken headers from the test coverage
> diff --git a/drivers/gpu/drm/i915/spi/intel_spi_drv.c b/drivers/gpu/drm/i915/spi/intel_spi_drv.c
> new file mode 100644
> index 000000000000..23261f35b71f
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/spi/intel_spi_drv.c
> @@ -0,0 +1,116 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright(c) 2019-2021, Intel Corporation. All rights reserved.
> + */
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +#include <linux/io.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/platform_device.h>
> +#include <spi/intel_spi.h>

Should this have "" instead of <>?

> +
> +struct i915_spi {
> +	void __iomem *base;
> +	size_t size;
> +	unsigned int nregions;
> +	struct {
> +		const char *name;
> +		u8 id;
> +		u64 offset;
> +		u64 size;
> +	} regions[];
> +};
> +
> +static int i915_spi_probe(struct platform_device *platdev)
> +{
> +	struct resource *bar;
> +	struct device *device;
> +	struct i915_spi *spi;
> +	struct i915_spi_region *regions;
> +	unsigned int nregions;
> +	unsigned int i, n;
> +	size_t size;
> +	char *name;
> +	size_t name_size;
> +
> +	device = &platdev->dev;
> +
> +	regions = dev_get_platdata(&platdev->dev);
> +	if (!regions) {
> +		dev_err(device, "no regions defined\n");
> +		return -ENODEV;
> +	}
> +
> +	/* count available regions */
> +	for (nregions = 0, i = 0; i < I915_SPI_REGIONS; i++) {
> +		if (regions[i].name)
> +			nregions++;
> +	}
> +
> +	if (!nregions) {
> +		dev_err(device, "no regions defined\n");
> +		return -ENODEV;
> +	}
> +
> +	size = sizeof(*spi) + sizeof(spi->regions[0]) * nregions;
> +	spi = devm_kzalloc(device, size, GFP_KERNEL);
> +	if (!spi)
> +		return -ENOMEM;
> +
> +	spi->nregions = nregions;
> +	for (n = 0, i = 0; i < I915_SPI_REGIONS; i++) {
> +		if (regions[i].name) {
> +			name_size = strlen(dev_name(&platdev->dev)) +
> +				    strlen(regions[i].name) + 2; /* for point */
> +			name = devm_kzalloc(device, name_size, GFP_KERNEL);
> +			if (!name)
> +				continue;
> +			snprintf(name, name_size, "%s.%s",
> +				 dev_name(&platdev->dev), regions[i].name);
> +			spi->regions[n].name = name;
> +			spi->regions[n].id = i;
> +			n++;
> +		}
> +	}
> +
> +	bar = platform_get_resource(platdev, IORESOURCE_MEM, 0);
> +	if (!bar)
> +		return -ENODEV;
> +
> +	spi->base = devm_ioremap_resource(device, bar);
> +	if (IS_ERR(spi->base)) {
> +		dev_err(device, "mmio not mapped\n");
> +		return PTR_ERR(spi->base);
> +	}
> +
> +	platform_set_drvdata(platdev, spi);
> +
> +	dev_dbg(device, "i915-spi is bound\n");
> +
> +	return 0;
> +}
> +
> +static int i915_spi_remove(struct platform_device *platdev)
> +{
> +	platform_set_drvdata(platdev, NULL);
> +
> +	return 0;
> +}
> +
> +MODULE_ALIAS("platform:i915-spi");
> +static struct platform_driver i915_spi_driver = {
> +	.probe  = i915_spi_probe,
> +	.remove = i915_spi_remove,
> +	.driver = {
> +		.name = "i915-spi",
> +	},
> +};
> +
> +module_platform_driver(i915_spi_driver);
> +
> +MODULE_LICENSE("GPL and additional rights");
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_DESCRIPTION("Intel DGFX SPI driver");
Winkler, Tomas Feb. 17, 2021, 8:58 p.m. UTC | #2
> 
> On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler@intel.com> wrote:
> > Add the platform driver for i915 on-die spi device, exposed via mfd
> > framework.
> >
> > 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/Kconfig             |   2 +
> >  drivers/gpu/drm/i915/Makefile            |   3 +
> >  drivers/gpu/drm/i915/spi/intel_spi_drv.c | 116
> > +++++++++++++++++++++++
> >  3 files changed, 121 insertions(+)
> >  create mode 100644 drivers/gpu/drm/i915/spi/intel_spi_drv.c
> >
> > diff --git a/drivers/gpu/drm/i915/Kconfig
> > b/drivers/gpu/drm/i915/Kconfig index abcaa8da45ac..13c870e5878e 100644
> > --- a/drivers/gpu/drm/i915/Kconfig
> > +++ b/drivers/gpu/drm/i915/Kconfig
> > @@ -27,6 +27,8 @@ config DRM_I915
> >  	select CEC_CORE if CEC_NOTIFIER
> >  	select VMAP_PFN
> >  	select MFD_CORE
> > +	select MTD
> 
> Selecting MTD does not seem to be a popular thing to do, which is usually a
> clue it's probably the wrong thing to do.
Depends, if it is not selected you'll end with wrongly configured system. 

> > +	select MTD_PARTITIONED_MASTER
> >  	help
> >  	  Choose this option if you have a system that has "Intel Graphics
> >  	  Media Accelerator" or "HD Graphics" integrated graphics, diff
> > --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> > index 7fa9120feb8d..f209cd541eec 100644
> > --- a/drivers/gpu/drm/i915/Makefile
> > +++ b/drivers/gpu/drm/i915/Makefile
> > @@ -299,6 +299,9 @@ endif
> >  obj-$(CONFIG_DRM_I915) += i915.o
> >  obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
> >
> > +obj-m += i915_spi.o
> 
> And literally nobody does this.
> 
> > +i915_spi-y := spi/intel_spi_drv.o
> 
> Time to add a separate Kconfig and Makefile in spi/?
Can be done. 
> 
> BR,
> Jani.
> 
> > +
> >  # header test
> >
> >  # exclude some broken headers from the test coverage diff --git
> > a/drivers/gpu/drm/i915/spi/intel_spi_drv.c
> > b/drivers/gpu/drm/i915/spi/intel_spi_drv.c
> > new file mode 100644
> > index 000000000000..23261f35b71f
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/spi/intel_spi_drv.c
> > @@ -0,0 +1,116 @@
> > +// SPDX-License-Identifier: MIT
> > +/*
> > + * Copyright(c) 2019-2021, Intel Corporation. All rights reserved.
> > + */
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +#include <linux/kernel.h>
> > +#include <linux/string.h>
> > +#include <linux/io.h>
> > +#include <linux/device.h>
> > +#include <linux/slab.h>
> > +#include <linux/platform_device.h>
> > +#include <spi/intel_spi.h>
> 
> Should this have "" instead of <>?
> 
> > +
> > +struct i915_spi {
> > +	void __iomem *base;
> > +	size_t size;
> > +	unsigned int nregions;
> > +	struct {
> > +		const char *name;
> > +		u8 id;
> > +		u64 offset;
> > +		u64 size;
> > +	} regions[];
> > +};
> > +
> > +static int i915_spi_probe(struct platform_device *platdev) {
> > +	struct resource *bar;
> > +	struct device *device;
> > +	struct i915_spi *spi;
> > +	struct i915_spi_region *regions;
> > +	unsigned int nregions;
> > +	unsigned int i, n;
> > +	size_t size;
> > +	char *name;
> > +	size_t name_size;
> > +
> > +	device = &platdev->dev;
> > +
> > +	regions = dev_get_platdata(&platdev->dev);
> > +	if (!regions) {
> > +		dev_err(device, "no regions defined\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	/* count available regions */
> > +	for (nregions = 0, i = 0; i < I915_SPI_REGIONS; i++) {
> > +		if (regions[i].name)
> > +			nregions++;
> > +	}
> > +
> > +	if (!nregions) {
> > +		dev_err(device, "no regions defined\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	size = sizeof(*spi) + sizeof(spi->regions[0]) * nregions;
> > +	spi = devm_kzalloc(device, size, GFP_KERNEL);
> > +	if (!spi)
> > +		return -ENOMEM;
> > +
> > +	spi->nregions = nregions;
> > +	for (n = 0, i = 0; i < I915_SPI_REGIONS; i++) {
> > +		if (regions[i].name) {
> > +			name_size = strlen(dev_name(&platdev->dev)) +
> > +				    strlen(regions[i].name) + 2; /* for point */
> > +			name = devm_kzalloc(device, name_size,
> GFP_KERNEL);
> > +			if (!name)
> > +				continue;
> > +			snprintf(name, name_size, "%s.%s",
> > +				 dev_name(&platdev->dev),
> regions[i].name);
> > +			spi->regions[n].name = name;
> > +			spi->regions[n].id = i;
> > +			n++;
> > +		}
> > +	}
> > +
> > +	bar = platform_get_resource(platdev, IORESOURCE_MEM, 0);
> > +	if (!bar)
> > +		return -ENODEV;
> > +
> > +	spi->base = devm_ioremap_resource(device, bar);
> > +	if (IS_ERR(spi->base)) {
> > +		dev_err(device, "mmio not mapped\n");
> > +		return PTR_ERR(spi->base);
> > +	}
> > +
> > +	platform_set_drvdata(platdev, spi);
> > +
> > +	dev_dbg(device, "i915-spi is bound\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static int i915_spi_remove(struct platform_device *platdev) {
> > +	platform_set_drvdata(platdev, NULL);
> > +
> > +	return 0;
> > +}
> > +
> > +MODULE_ALIAS("platform:i915-spi");
> > +static struct platform_driver i915_spi_driver = {
> > +	.probe  = i915_spi_probe,
> > +	.remove = i915_spi_remove,
> > +	.driver = {
> > +		.name = "i915-spi",
> > +	},
> > +};
> > +
> > +module_platform_driver(i915_spi_driver);
> > +
> > +MODULE_LICENSE("GPL and additional rights"); MODULE_AUTHOR("Intel
> > +Corporation"); MODULE_DESCRIPTION("Intel DGFX SPI driver");
> 
> --
> Jani Nikula, Intel Open Source Graphics Center
Lucas De Marchi Feb. 18, 2021, 9:49 a.m. UTC | #3
On Wed, Feb 17, 2021 at 08:58:12PM +0000, Winkler, Tomas wrote:
>>
>> On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler@intel.com> wrote:
>> > Add the platform driver for i915 on-die spi device, exposed via mfd
>> > framework.
>> >
>> > 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/Kconfig             |   2 +
>> >  drivers/gpu/drm/i915/Makefile            |   3 +
>> >  drivers/gpu/drm/i915/spi/intel_spi_drv.c | 116
>> > +++++++++++++++++++++++
>> >  3 files changed, 121 insertions(+)
>> >  create mode 100644 drivers/gpu/drm/i915/spi/intel_spi_drv.c
>> >
>> > diff --git a/drivers/gpu/drm/i915/Kconfig
>> > b/drivers/gpu/drm/i915/Kconfig index abcaa8da45ac..13c870e5878e 100644
>> > --- a/drivers/gpu/drm/i915/Kconfig
>> > +++ b/drivers/gpu/drm/i915/Kconfig
>> > @@ -27,6 +27,8 @@ config DRM_I915
>> >  	select CEC_CORE if CEC_NOTIFIER
>> >  	select VMAP_PFN
>> >  	select MFD_CORE
>> > +	select MTD
>>
>> Selecting MTD does not seem to be a popular thing to do, which is usually a
>> clue it's probably the wrong thing to do.
>Depends, if it is not selected you'll end with wrongly configured system.

no. I believe the idea is that having a CONFIG_I915_SPI, you could do

	depends on MTD

like the other drivers doing similar thing:

	git grep MTD -- ':(exclude)drivers/mtd' ':(exclude)arch/' '*Kconfig'

Lucas De Marchi
Winkler, Tomas Feb. 18, 2021, 10:50 a.m. UTC | #4
> 
> On Wed, Feb 17, 2021 at 08:58:12PM +0000, Winkler, Tomas wrote:
> >>
> >> On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler@intel.com> wrote:
> >> > Add the platform driver for i915 on-die spi device, exposed via mfd
> >> > framework.
> >> >
> >> > 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/Kconfig             |   2 +
> >> >  drivers/gpu/drm/i915/Makefile            |   3 +
> >> >  drivers/gpu/drm/i915/spi/intel_spi_drv.c | 116
> >> > +++++++++++++++++++++++
> >> >  3 files changed, 121 insertions(+)  create mode 100644
> >> > drivers/gpu/drm/i915/spi/intel_spi_drv.c
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/Kconfig
> >> > b/drivers/gpu/drm/i915/Kconfig index abcaa8da45ac..13c870e5878e
> >> > 100644
> >> > --- a/drivers/gpu/drm/i915/Kconfig
> >> > +++ b/drivers/gpu/drm/i915/Kconfig
> >> > @@ -27,6 +27,8 @@ config DRM_I915
> >> >  	select CEC_CORE if CEC_NOTIFIER
> >> >  	select VMAP_PFN
> >> >  	select MFD_CORE
> >> > +	select MTD
> >>
> >> Selecting MTD does not seem to be a popular thing to do, which is
> >> usually a clue it's probably the wrong thing to do.
> >Depends, if it is not selected you'll end with wrongly configured system.
> 
> no. I believe the idea is that having a CONFIG_I915_SPI, you could do
> 
> 	depends on MTD
> 
> like the other drivers doing similar thing:
> 
> 	git grep MTD -- ':(exclude)drivers/mtd' ':(exclude)arch/' '*Kconfig'
MTD is usually used on embedded systems rather than on general distros, i915_spi and actually a bit redefines its usage 
so w/o forceful select, it won't be enabled.

> 
> Lucas De Marchi
Winkler, Tomas Feb. 19, 2021, 6:06 a.m. UTC | #5
> 
> On Wed, Feb 17, 2021 at 08:58:12PM +0000, Winkler, Tomas wrote:
> >>
> >> On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler@intel.com> wrote:
> >> > Add the platform driver for i915 on-die spi device, exposed via mfd
> >> > framework.
> >> >
> >> > 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/Kconfig             |   2 +
> >> >  drivers/gpu/drm/i915/Makefile            |   3 +
> >> >  drivers/gpu/drm/i915/spi/intel_spi_drv.c | 116
> >> > +++++++++++++++++++++++
> >> >  3 files changed, 121 insertions(+)  create mode 100644
> >> > drivers/gpu/drm/i915/spi/intel_spi_drv.c
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/Kconfig
> >> > b/drivers/gpu/drm/i915/Kconfig index abcaa8da45ac..13c870e5878e
> >> > 100644
> >> > --- a/drivers/gpu/drm/i915/Kconfig
> >> > +++ b/drivers/gpu/drm/i915/Kconfig
> >> > @@ -27,6 +27,8 @@ config DRM_I915
> >> >  	select CEC_CORE if CEC_NOTIFIER
> >> >  	select VMAP_PFN
> >> >  	select MFD_CORE
> >> > +	select MTD
> >>
> >> Selecting MTD does not seem to be a popular thing to do, which is
> >> usually a clue it's probably the wrong thing to do.
> >Depends, if it is not selected you'll end with wrongly configured system.
> 
> no. I believe the idea is that having a CONFIG_I915_SPI, you could do
> 
> 	depends on MTD
> 
> like the other drivers doing similar thing:
> 
> 	git grep MTD -- ':(exclude)drivers/mtd' ':(exclude)arch/' '*Kconfig'
 I know the pattern and it can be done, the issue is that mtd is used mostly in embedded systems so it is not selected by the desktop distros.
The intel spi both on PCH and in GFX takes this into different direction and usage.

Thanks
Tomas
Lucas De Marchi Feb. 19, 2021, 10:59 p.m. UTC | #6
On Thu, Feb 18, 2021 at 10:06:08PM -0800, Winkler, Tomas wrote:
>
>>
>> On Wed, Feb 17, 2021 at 08:58:12PM +0000, Winkler, Tomas wrote:
>> >>
>> >> On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler@intel.com> wrote:
>> >> > Add the platform driver for i915 on-die spi device, exposed via mfd
>> >> > framework.
>> >> >
>> >> > 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/Kconfig             |   2 +
>> >> >  drivers/gpu/drm/i915/Makefile            |   3 +
>> >> >  drivers/gpu/drm/i915/spi/intel_spi_drv.c | 116
>> >> > +++++++++++++++++++++++
>> >> >  3 files changed, 121 insertions(+)  create mode 100644
>> >> > drivers/gpu/drm/i915/spi/intel_spi_drv.c
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/Kconfig
>> >> > b/drivers/gpu/drm/i915/Kconfig index abcaa8da45ac..13c870e5878e
>> >> > 100644
>> >> > --- a/drivers/gpu/drm/i915/Kconfig
>> >> > +++ b/drivers/gpu/drm/i915/Kconfig
>> >> > @@ -27,6 +27,8 @@ config DRM_I915
>> >> >  	select CEC_CORE if CEC_NOTIFIER
>> >> >  	select VMAP_PFN
>> >> >  	select MFD_CORE
>> >> > +	select MTD
>> >>
>> >> Selecting MTD does not seem to be a popular thing to do, which is
>> >> usually a clue it's probably the wrong thing to do.
>> >Depends, if it is not selected you'll end with wrongly configured system.
>>
>> no. I believe the idea is that having a CONFIG_I915_SPI, you could do
>>
>> 	depends on MTD
>>
>> like the other drivers doing similar thing:
>>
>> 	git grep MTD -- ':(exclude)drivers/mtd' ':(exclude)arch/' '*Kconfig'
> I know the pattern and it can be done, the issue is that mtd is used mostly in embedded systems so it is not selected by the desktop distros.
>The intel spi both on PCH and in GFX takes this into different direction and usage.

humn... but then we have a problem here. You're saying most of the
people won't need it because it's used only for manufacturing*.
And yet you want it to be force selected on everybody?  That
doesn't sound like a good plan.

Lucas De Marchi


* it may actually also be useful for kernel developers too, to dump its
content and validate the parser, help debug other systems, etc.

>
>Thanks
>Tomas
>
Winkler, Tomas Feb. 20, 2021, 5:56 p.m. UTC | #7
> On Thu, Feb 18, 2021 at 10:06:08PM -0800, Winkler, Tomas wrote:
> >
> >>
> >> On Wed, Feb 17, 2021 at 08:58:12PM +0000, Winkler, Tomas wrote:
> >> >>
> >> >> On Tue, 16 Feb 2021, Tomas Winkler <tomas.winkler@intel.com>
> wrote:
> >> >> > Add the platform driver for i915 on-die spi device, exposed via
> >> >> > mfd framework.
> >> >> >
> >> >> > 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/Kconfig             |   2 +
> >> >> >  drivers/gpu/drm/i915/Makefile            |   3 +
> >> >> >  drivers/gpu/drm/i915/spi/intel_spi_drv.c | 116
> >> >> > +++++++++++++++++++++++
> >> >> >  3 files changed, 121 insertions(+)  create mode 100644
> >> >> > drivers/gpu/drm/i915/spi/intel_spi_drv.c
> >> >> >
> >> >> > diff --git a/drivers/gpu/drm/i915/Kconfig
> >> >> > b/drivers/gpu/drm/i915/Kconfig index abcaa8da45ac..13c870e5878e
> >> >> > 100644
> >> >> > --- a/drivers/gpu/drm/i915/Kconfig
> >> >> > +++ b/drivers/gpu/drm/i915/Kconfig
> >> >> > @@ -27,6 +27,8 @@ config DRM_I915
> >> >> >  	select CEC_CORE if CEC_NOTIFIER
> >> >> >  	select VMAP_PFN
> >> >> >  	select MFD_CORE
> >> >> > +	select MTD
> >> >>
> >> >> Selecting MTD does not seem to be a popular thing to do, which is
> >> >> usually a clue it's probably the wrong thing to do.
> >> >Depends, if it is not selected you'll end with wrongly configured system.
> >>
> >> no. I believe the idea is that having a CONFIG_I915_SPI, you could do
> >>
> >> 	depends on MTD
> >>
> >> like the other drivers doing similar thing:
> >>
> >> 	git grep MTD -- ':(exclude)drivers/mtd' ':(exclude)arch/' '*Kconfig'
> > I know the pattern and it can be done, the issue is that mtd is used mostly
> in embedded systems so it is not selected by the desktop distros.
> >The intel spi both on PCH and in GFX takes this into different direction and
> usage.
> 
> humn... but then we have a problem here. You're saying most of the people
> won't need it because it's used only for manufacturing*.
> And yet you want it to be force selected on everybody?  That doesn't sound
> like a good plan.
It depends, whether manufacturing is done on shipping OS or not, both approaches are in use.
One approach for the first case can be that the distro prevent the module loading via modprobe.d 
and it's forcefully loaded during manufacturing.  Still it should be compiled and signed.
> 
> Lucas De Marchi
> 
> 
> * it may actually also be useful for kernel developers too, to dump its content
> and validate the parser, help debug other systems, etc.
True.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index abcaa8da45ac..13c870e5878e 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -27,6 +27,8 @@  config DRM_I915
 	select CEC_CORE if CEC_NOTIFIER
 	select VMAP_PFN
 	select MFD_CORE
+	select MTD
+	select MTD_PARTITIONED_MASTER
 	help
 	  Choose this option if you have a system that has "Intel Graphics
 	  Media Accelerator" or "HD Graphics" integrated graphics,
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 7fa9120feb8d..f209cd541eec 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -299,6 +299,9 @@  endif
 obj-$(CONFIG_DRM_I915) += i915.o
 obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
 
+obj-m += i915_spi.o
+i915_spi-y := spi/intel_spi_drv.o
+
 # header test
 
 # exclude some broken headers from the test coverage
diff --git a/drivers/gpu/drm/i915/spi/intel_spi_drv.c b/drivers/gpu/drm/i915/spi/intel_spi_drv.c
new file mode 100644
index 000000000000..23261f35b71f
--- /dev/null
+++ b/drivers/gpu/drm/i915/spi/intel_spi_drv.c
@@ -0,0 +1,116 @@ 
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright(c) 2019-2021, Intel Corporation. All rights reserved.
+ */
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/io.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <spi/intel_spi.h>
+
+struct i915_spi {
+	void __iomem *base;
+	size_t size;
+	unsigned int nregions;
+	struct {
+		const char *name;
+		u8 id;
+		u64 offset;
+		u64 size;
+	} regions[];
+};
+
+static int i915_spi_probe(struct platform_device *platdev)
+{
+	struct resource *bar;
+	struct device *device;
+	struct i915_spi *spi;
+	struct i915_spi_region *regions;
+	unsigned int nregions;
+	unsigned int i, n;
+	size_t size;
+	char *name;
+	size_t name_size;
+
+	device = &platdev->dev;
+
+	regions = dev_get_platdata(&platdev->dev);
+	if (!regions) {
+		dev_err(device, "no regions defined\n");
+		return -ENODEV;
+	}
+
+	/* count available regions */
+	for (nregions = 0, i = 0; i < I915_SPI_REGIONS; i++) {
+		if (regions[i].name)
+			nregions++;
+	}
+
+	if (!nregions) {
+		dev_err(device, "no regions defined\n");
+		return -ENODEV;
+	}
+
+	size = sizeof(*spi) + sizeof(spi->regions[0]) * nregions;
+	spi = devm_kzalloc(device, size, GFP_KERNEL);
+	if (!spi)
+		return -ENOMEM;
+
+	spi->nregions = nregions;
+	for (n = 0, i = 0; i < I915_SPI_REGIONS; i++) {
+		if (regions[i].name) {
+			name_size = strlen(dev_name(&platdev->dev)) +
+				    strlen(regions[i].name) + 2; /* for point */
+			name = devm_kzalloc(device, name_size, GFP_KERNEL);
+			if (!name)
+				continue;
+			snprintf(name, name_size, "%s.%s",
+				 dev_name(&platdev->dev), regions[i].name);
+			spi->regions[n].name = name;
+			spi->regions[n].id = i;
+			n++;
+		}
+	}
+
+	bar = platform_get_resource(platdev, IORESOURCE_MEM, 0);
+	if (!bar)
+		return -ENODEV;
+
+	spi->base = devm_ioremap_resource(device, bar);
+	if (IS_ERR(spi->base)) {
+		dev_err(device, "mmio not mapped\n");
+		return PTR_ERR(spi->base);
+	}
+
+	platform_set_drvdata(platdev, spi);
+
+	dev_dbg(device, "i915-spi is bound\n");
+
+	return 0;
+}
+
+static int i915_spi_remove(struct platform_device *platdev)
+{
+	platform_set_drvdata(platdev, NULL);
+
+	return 0;
+}
+
+MODULE_ALIAS("platform:i915-spi");
+static struct platform_driver i915_spi_driver = {
+	.probe  = i915_spi_probe,
+	.remove = i915_spi_remove,
+	.driver = {
+		.name = "i915-spi",
+	},
+};
+
+module_platform_driver(i915_spi_driver);
+
+MODULE_LICENSE("GPL and additional rights");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_DESCRIPTION("Intel DGFX SPI driver");