Hi Brad,
On 13/06/22 12:56PM, Brad Larson wrote:
> From: Brad Larson <blarson@xxxxxxx>
>
> The AMD Pensando Elba SoC has the Cadence QSPI controller integrated.
>
> The quirk CQSPI_NEEDS_APB_AHB_HAZARD_WAR is added and if enabled
> a dummy readback from the controller is performed to ensure
> synchronization.
>
> Signed-off-by: Brad Larson <blarson@xxxxxxx>
> ---
> drivers/spi/spi-cadence-quadspi.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index 72b1a5a2298c..ebb77ea8e6ba 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -39,6 +39,7 @@
> #define CQSPI_DISABLE_DAC_MODE BIT(1)
> #define CQSPI_SUPPORT_EXTERNAL_DMA BIT(2)
> #define CQSPI_NO_SUPPORT_WR_COMPLETION BIT(3)
> +#define CQSPI_NEEDS_APB_AHB_HAZARD_WAR BIT(4)
>
> /* Capabilities */
> #define CQSPI_SUPPORTS_OCTAL BIT(0)
> @@ -87,6 +88,7 @@ struct cqspi_st {
> bool use_dma_read;
> u32 pd_dev_id;
> bool wr_completion;
> + bool apb_ahb_hazard;
> };
>
> struct cqspi_driver_platdata {
> @@ -952,6 +954,13 @@ static int cqspi_indirect_write_execute(struct cqspi_flash_pdata *f_pdata,
> if (cqspi->wr_delay)
> ndelay(cqspi->wr_delay);
>
> + /*
> + * If a hazard exists between the APB and AHB interfaces, perform a
> + * dummy readback from the controller to ensure synchronization.
> + */
This is needed for TI's SoCs as well. APB and AHB accesses are
independent of each other on the interconnect and can be racy. I wrote a
couple patches [0][1] to fix this on TI's fork. I never got around to
sending them upstream. It would be great if you can pick those up. They
fix the race in all paths, not just indirect write.
I would also prefer if we do this unconditionally. I don't think it has
much downside even on platforms that do not strictly need this.
[0] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/drivers/spi/spi-cadence-quadspi.c?h=ti-linux-5.10.y&id=027f03a8512086e5ef05dc4e4ff53b2628848f95
[1] https://git.ti.com/cgit/ti-linux-kernel/ti-linux-kernel/commit/drivers/spi/spi-cadence-quadspi.c?h=ti-linux-5.10.y&id=4c367e58bab7d3f9c470c3778441f73546f20398
> + if (cqspi->apb_ahb_hazard)
> + (void)readl(reg_base + CQSPI_REG_INDIRECTWR);
> +
> while (remaining > 0) {
> size_t write_words, mod_bytes;
>
> @@ -1667,6 +1676,8 @@ static int cqspi_probe(struct platform_device *pdev)
> cqspi->use_dma_read = true;
> if (ddata->quirks & CQSPI_NO_SUPPORT_WR_COMPLETION)
> cqspi->wr_completion = false;
> + if (ddata->quirks & CQSPI_NEEDS_APB_AHB_HAZARD_WAR)
> + cqspi->apb_ahb_hazard = true;
>
> if (of_device_is_compatible(pdev->dev.of_node,
> "xlnx,versal-ospi-1.0"))
> @@ -1789,6 +1800,10 @@ static const struct cqspi_driver_platdata versal_ospi = {
> .get_dma_status = cqspi_get_versal_dma_status,
> };
>
> +static const struct cqspi_driver_platdata pen_cdns_qspi = {
> + .quirks = CQSPI_NEEDS_APB_AHB_HAZARD_WAR | CQSPI_DISABLE_DAC_MODE,
> +};
> +
> static const struct of_device_id cqspi_dt_ids[] = {
> {
> .compatible = "cdns,qspi-nor",
> @@ -1814,6 +1829,10 @@ static const struct of_device_id cqspi_dt_ids[] = {
> .compatible = "intel,socfpga-qspi",
> .data = &socfpga_qspi,
> },
> + {
> + .compatible = "amd,pensando-elba-qspi",
> + .data = &pen_cdns_qspi,
> + },
> { /* end of table */ }
> };
>
> --
> 2.17.1
>
--
Regards,
Pratyush Yadav
Texas Instruments Inc.
