On Fri, Jun 07, 2024 at 08:18:29AM -0700, Witold Sadowski wrote:
> This commit adds support for the basic v2 Marvell overlay block. Key
> features included are:
> - Clock configuration
> - PHY configuration
> - Interrupt configuration (enabling)
This feels like it could usefully be split up so these three bits are
separate, and there appear to be other changes buried in here as well.
I can't tell what changes either the PHY or interrupt configuration
might be referencing.
> @@ -295,6 +450,10 @@ static void cdns_xspi_set_interrupts(struct cdns_xspi_dev *cdns_xspi,
> bool enabled)
> {
> u32 intr_enable;
> + u32 irq_status;
> +
> + irq_status = readl(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
> + writel(irq_status, cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
>
> intr_enable = readl(cdns_xspi->iobase + CDNS_XSPI_INTR_ENABLE_REG);
> if (enabled)
This seems like a separate change which applies to everything, not just
Marvell versions of the IP, and should be split out and explained.
> @@ -319,6 +478,9 @@ static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
> return -EIO;
> }
>
> + writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
> + cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
> +
This wasn't clearly mentioned in the changelog and is again being done
unconditionally for all instances of the IP, probably best to split out
and explain.
> +static void mrvl_ioreadq(void __iomem *addr, void *buf, int len)
> +{
> + int i = 0;
> + int rcount = len / 8;
> + int rcount_nf = len % 8;
> + uint64_t tmp;
> + uint64_t *buf64 = (uint64_t *)buf;
Any need to cast away from void * indicates a problem.
> @@ -337,13 +563,11 @@ static void cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi)
>
> switch (sdma_dir) {
> case CDNS_XSPI_SDMA_DIR_READ:
> - ioread8_rep(cdns_xspi->sdmabase,
> - cdns_xspi->in_buffer, sdma_size);
> + cdns_xspi_sdma_memread(cdns_xspi, sdma_size);
> break;
It's feeling like it might make sense to have an ops structure rather
than sprinkling checks for the Marvell overlay everywhere.
Attachment:
signature.asc
Description: PGP signature
