Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-28 Thread Usyskin, Alexander
> >
> > > There is a Discreet Graphic device with embedded SPI (controller & flash).
> > > The embedded SPI is not visible to OS.
> > > There is another HW in the chip that gates access to the controller and
> > > exposes registers for:
> > > region select; read and write (4 and 8 bytes); erase (4K); error register;
> >
> > So assuming that's flash region select it sounds like this is a MTD
> > controller and the fact that there's SPI isn't really relevant at all
> > from a programming model point of view and it should probably be
> > described as a MTD controller of some kind.  Does that sound about
> > right?
> 
> Yeah in this case it seems the best option if the OS only has access to
> a very small subset of what the spi controller can do.
> 
> Thanks,
> Miquèl

So, the approach of patch series that started the whole thread is
right in general?
Is the series submitted to the right mailing lists to review?
If so, can you please review the series and evaluate it readiness to be merged?

-- 
Thanks,
Sasha



Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-27 Thread Miquel Raynal
Hi Mark,

broo...@kernel.org wrote on Wed, 27 Sep 2023 16:37:35 +0200:

> On Wed, Sep 27, 2023 at 02:11:47PM +, Usyskin, Alexander wrote:
> 
> > There is a Discreet Graphic device with embedded SPI (controller & flash).
> > The embedded SPI is not visible to OS.
> > There is another HW in the chip that gates access to the controller and
> > exposes registers for:
> > region select; read and write (4 and 8 bytes); erase (4K); error register;  
> >  
> 
> So assuming that's flash region select it sounds like this is a MTD
> controller and the fact that there's SPI isn't really relevant at all
> from a programming model point of view and it should probably be
> described as a MTD controller of some kind.  Does that sound about
> right?

Yeah in this case it seems the best option if the OS only has access to
a very small subset of what the spi controller can do.

Thanks,
Miquèl


Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-27 Thread Mark Brown
On Wed, Sep 27, 2023 at 02:11:47PM +, Usyskin, Alexander wrote:

> There is a Discreet Graphic device with embedded SPI (controller & flash).
> The embedded SPI is not visible to OS.
> There is another HW in the chip that gates access to the controller and
> exposes registers for:
> region select; read and write (4 and 8 bytes); erase (4K); error register; 

So assuming that's flash region select it sounds like this is a MTD
controller and the fact that there's SPI isn't really relevant at all
from a programming model point of view and it should probably be
described as a MTD controller of some kind.  Does that sound about
right?


signature.asc
Description: PGP signature


Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-27 Thread Usyskin, Alexander
> 
> > > This sounds like there's some sort of MFD rather than or as well as a 
> > > flash
> > > chip, or possibly multiple SPI devices?
> 
> > Yes, the driver doesn't talk to SPI controller directly it goes via
> > another layer, so all SPI standard HW is not accessible, but we wish
> > to expose the MTD interface.
> 
> Perhaps if you described clearly and directly the actual system you are
> trying to model then it would be easier to tell how it fits into the
> kernel?  What is the actual interface here - how does the host interact
> with the flash?
> 
> Also to repeat: please fix your mail client to word wrap within
> paragraphs at something substantially less than 80 columns.  Doing this
> makes your messages much easier to read and reply to.

Sorry for that, I'm fairly new in SPI and MTD subsystems.
Will try to describe as best as I could.

There is a Discreet Graphic device with embedded SPI (controller & flash).
The embedded SPI is not visible to OS.
There is another HW in the chip that gates access to the controller and
exposes registers for:
region select; read and write (4 and 8 bytes); erase (4K); error register; 

There are two main usages - user-space manufacturing and repair
application that requires unrestricted read/write/erase over flash chip
and kernel module that requires read access to one of flash regions
for configuration.

--
Alexander (Sasha) Usyskin

CSE FW Dev - Host SW
Intel Israel (74) Limited




Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-21 Thread Mark Brown
On Wed, Sep 20, 2023 at 09:00:08PM +, Winkler, Tomas wrote:

> > This sounds like there's some sort of MFD rather than or as well as a flash
> > chip, or possibly multiple SPI devices?

> Yes, the driver doesn't talk to SPI controller directly it goes via
> another layer, so all SPI standard HW is not accessible, but we wish
> to expose the MTD interface.

Perhaps if you described clearly and directly the actual system you are
trying to model then it would be easier to tell how it fits into the
kernel?  What is the actual interface here - how does the host interact
with the flash?

Also to repeat: please fix your mail client to word wrap within
paragraphs at something substantially less than 80 columns.  Doing this
makes your messages much easier to read and reply to.


signature.asc
Description: PGP signature


Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-20 Thread Winkler, Tomas


> 
> On Wed, Sep 20, 2023 at 01:52:07PM +, Usyskin, Alexander wrote:
> 
> > I've tried to register spi controller with a spi-mem ops, but I can't find 
> > a way
> to connect to mtd subsystem.
> > I took spi-intel as example, which connects to spi-nor but it relies on 
> > JDEC ID
> of flash to configure itself.
> 
> You should use the normal SPI device registration interfaces to register
> whatever devices are connected to the SPI controller.  What in concrete terms
> have you tried to do here and in what way did it not work?

> 
> > We have predefined set of operations unrelated to flash behind the
> controller.
> 
> This sounds like there's some sort of MFD rather than or as well as a flash
> chip, or possibly multiple SPI devices?

Yes, the driver doesn't talk to SPI controller directly it goes via another 
layer, so all SPI standard HW is not accessible, but we wish to expose the MTD 
interface.

Thanks
Tomas



Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-20 Thread Mark Brown
On Wed, Sep 20, 2023 at 01:52:07PM +, Usyskin, Alexander wrote:

> I've tried to register spi controller with a spi-mem ops, but I can't find a 
> way to connect to mtd subsystem.
> I took spi-intel as example, which connects to spi-nor but it relies on JDEC 
> ID of flash to configure itself.

You should use the normal SPI device registration interfaces to register
whatever devices are connected to the SPI controller.  What in concrete
terms have you tried to do here and in what way did it not work?

> We have predefined set of operations unrelated to flash behind the controller.

This sounds like there's some sort of MFD rather than or as well as a
flash chip, or possibly multiple SPI devices?


signature.asc
Description: PGP signature


Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-20 Thread Usyskin, Alexander
> > > > No SPI controllers are directly visible to userspace, some SPI devices
> > > > are selectively exposed but that needs to be explicitly requested and is
> > > > generally discouraged.
> 
> > > What are the options here? Explicitly request exception is the one.
> > > Any other way to add access to flash memory connected in such way?
> 
> > Register a spi controller with at least spi-mem ops, as suggested
> > previously, is the standard way I guess. If you're not willing to do
> > so, it must be justified, I guess?
> 
> Right, we already have a way of describing flash chips so that should be
> used to describe any flash chips.

Hi

I've tried to register spi controller with a spi-mem ops, but I can't find a 
way to connect to mtd subsystem.
I took spi-intel as example, which connects to spi-nor but it relies on JDEC ID 
of flash to configure itself.
We have predefined set of operations unrelated to flash behind the controller.
What is the right way to simulate the general operations?
Should I add dummy flash device? Or there is another option to connect 
spi-mem-only controller to mtd?
Or this is too much and warrant the exception to have direct MTD device?

-- 
Thanks,
Sasha





Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-18 Thread Miquel Raynal
Hi,

alexander.usys...@intel.com wrote on Tue, 12 Sep 2023 13:15:58 +:

> >   
> > > 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.  
> > 
> > No SPI controllers are directly visible to userspace, some SPI devices
> > are selectively exposed but that needs to be explicitly requested and is
> > generally discouraged.  
> 
> What are the options here? Explicitly request exception is the one.
> Any other way to add access to flash memory connected in such way?

Register a spi controller with at least spi-mem ops, as suggested
previously, is the standard way I guess. If you're not willing to do
so, it must be justified, I guess?

Thanks,
Miquèl


Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-12 Thread Mark Brown
On Tue, Sep 12, 2023 at 03:21:02PM +0200, Miquel Raynal wrote:
> alexander.usys...@intel.com wrote on Tue, 12 Sep 2023 13:15:58 +:

> > > No SPI controllers are directly visible to userspace, some SPI devices
> > > are selectively exposed but that needs to be explicitly requested and is
> > > generally discouraged.  

> > What are the options here? Explicitly request exception is the one.
> > Any other way to add access to flash memory connected in such way?

> Register a spi controller with at least spi-mem ops, as suggested
> previously, is the standard way I guess. If you're not willing to do
> so, it must be justified, I guess?

Right, we already have a way of describing flash chips so that should be
used to describe any flash chips.


signature.asc
Description: PGP signature


Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-12 Thread Usyskin, Alexander
> 
> > 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.
> 
> No SPI controllers are directly visible to userspace, some SPI devices
> are selectively exposed but that needs to be explicitly requested and is
> generally discouraged.

What are the options here? Explicitly request exception is the one.
Any other way to add access to flash memory connected in such way?

-- 
Thanks,
Sasha




Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-12 Thread Mark Brown
On Tue, Sep 12, 2023 at 10:50:22AM +, Usyskin, Alexander wrote:

> 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.

No SPI controllers are directly visible to userspace, some SPI devices
are selectively exposed but that needs to be explicitly requested and is
generally discouraged.


signature.asc
Description: PGP signature


Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-12 Thread Usyskin, Alexander
> 
> > 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 (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


Re: [Intel-gfx] [PATCH 00/10] drm/i915/spi: spi access for discrete graphics

2023-09-11 Thread Miquel Raynal
Hi Alexander,

+ Mark Brown + spi list
+ spi-nor maintainers

alexander.usys...@intel.com wrote on Sun, 10 Sep 2023 15:39:39 +0300:

> 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

> This series is intended to be upstreamed through drm tree.
>
> Signed-off-by: Alexander Usyskin 
> 
> 
> 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