drivers: espi: xec: Add support for eSPI flash channel

[albertofloyd]

drivers: espi: xec: Add support for eSPI flash channel

Add driver implementation to support flash read/writ transactions
over eSPI bus.

Depends partially on #23796

[scottwcpg]

I did not see FC interrupt enables set in the read routine. The FC write routine does have interrupt enables for FC Done and channel enable change.

[albertofloyd]

I did not see FC interrupt enables set in the read routine. The FC write routine does have interrupt enables for FC Done and channel enable change.

@scottwcpg I have the FC interrupts enabled during initialization at espi_init_flash(), would that be enough? Should this be done for each transaction?
I was thinking to actually remove interrupt enabling from FC write routine

[scottwcpg]

I did not see FC interrupt enables set in the read routine. The FC write routine does have interrupt enables for FC Done and channel enable change.

@scottwcpg I have the FC interrupts enabled during initialization at espi_init_flash(), would that be enough? Should this be done for each transaction?
I was thinking to actually remove interrupt enabling from FC write routine

OK, I did not see the interrupt enable in the init routine. You should be OK with interrupt enable in init. The is one corner case if you ever set the ABORT_ACCESS bit then HW will set DONE and ABORTED_BY_SLAVE which will fire the interrupt.

[scottwcpg]

The eSPI flash channel status register bits are read-write-1-to-clear. Performing a bit-wise OR to clear one bit results in clearing all active bits at the time the register is read. The other eSPI channels' status registers are the same.

[albertofloyd]

The eSPI flash channel status register bits are read-write-1-to-clear. Performing a bit-wise OR to clear one bit results in clearing all active bits at the time the register is read. The other eSPI channels' status registers are the same.

@scottwcpg , could you further clarify? Or add the comment into the line of code?
I think you are referring to the operations inside espi_flash_isr()

1. read first the flash channel register status into a variable, 2) check for each bit using the saved status 3) finally I clear in each case by writing 1 ( OR'ed with corresponding mask ).
   Are you saying register status gets clear during 1)

[scottwcpg]

The eSPI flash channel status register bits are read-write-1-to-clear. Performing a bit-wise OR to clear one bit results in clearing all active bits at the time the register is read. The other eSPI channels' status registers are the same.

@scottwcpg , could you further clarify? Or add the comment into the line of code?
I think you are referring to the operations inside espi_flash_isr()

1. read first the flash channel register status into a variable, 2) check for each bit using the saved status 3) finally I clear in each case by writing 1 ( OR'ed with corresponding mask ).
   Are you saying register status gets clear during 1)

Lines 880 and 886 in espi_mchp_xec.c are both in the ISR but I observe all write to eSPI HW status registers are using bit-wise OR. All the eSPI status register implement read-only or read-write-1-clear bits. The bit-wise OR may clear other bits if the other bits are read as "1" in the read modify write sequence.

[albertofloyd]

The eSPI flash channel status register bits are read-write-1-to-clear. Performing a bit-wise OR to clear one bit results in clearing all active bits at the time the register is read. The other eSPI channels' status registers are the same.

@scottwcpg , could you further clarify? Or add the comment into the line of code?
I think you are referring to the operations inside espi_flash_isr()

1. read first the flash channel register status into a variable, 2) check for each bit using the saved status 3) finally I clear in each case by writing 1 ( OR'ed with corresponding mask ).
   Are you saying register status gets clear during 1)

Lines 880 and 886 in espi_mchp_xec.c are both in the ISR but I observe all write to eSPI HW status registers are using bit-wise OR. All the eSPI status register implement read-only or read-write-1-clear bits. The bit-wise OR may clear other bits if the other bits are read as "1" in the read modify write sequence.

Got it. We have a widespread problem in the driver that will cause failures in corner cases (more than 1 bit set in these registers) during OR'd
Let me address the flash portion in this PR, will submit these changes for other blocks after perform through analysis and regression tests.

[scottwcpg]

The eSPI flash channel status register bits are read-write-1-to-clear. Performing a bit-wise OR to clear one bit results in clearing all active bits at the time the register is read. The other eSPI channels' status registers are the same.

@scottwcpg , could you further clarify? Or add the comment into the line of code?
I think you are referring to the operations inside espi_flash_isr()

1. read first the flash channel register status into a variable, 2) check for each bit using the saved status 3) finally I clear in each case by writing 1 ( OR'ed with corresponding mask ).
   Are you saying register status gets clear during 1)

Lines 880 and 886 in espi_mchp_xec.c are both in the ISR but I observe all write to eSPI HW status registers are using bit-wise OR. All the eSPI status register implement read-only or read-write-1-clear bits. The bit-wise OR may clear other bits if the other bits are read as "1" in the read modify write sequence.

Got it. We have a widespread problem in the driver that will cause failures in corner cases (more than 1 bit set in these registers) during OR'd
Let me address the flash portion in this PR, will submit these changes for other blocks after perform through analysis and regression tests.

Ok, sounds like a plan.

[albertofloyd]

@scottwcpg Created #24464 to fix OOB and maybe peripheral channels

Update register flash status access as indicated.

[zephyrbot]

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.