[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zml5DD5s7OZopVkU@finisterre.sirena.org.uk>
Date: Wed, 12 Jun 2024 11:31:40 +0100
From: Mark Brown <broonie@...nel.org>
To: Witold Sadowski <wsadowski@...vell.com>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-spi@...r.kernel.org" <linux-spi@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"robh@...nel.org" <robh@...nel.org>,
"krzysztof.kozlowski+dt@...aro.org" <krzysztof.kozlowski+dt@...aro.org>,
"conor+dt@...nel.org" <conor+dt@...nel.org>,
"pthombar@...ence.com" <pthombar@...ence.com>
Subject: Re: [EXTERNAL] Re: [PATCH v8 2/4] spi: cadence: Add Marvell xSPI IP
overlay changes
On Tue, Jun 11, 2024 at 09:51:58PM +0000, Witold Sadowski wrote:
> > On Fri, Jun 07, 2024 at 08:18:29AM -0700, Witold Sadowski wrote:
> > > 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.
> That changes are in single commit as, using not all of them will result in
> total xSPI failure. Configuring PHY makes no sense if clock is not enabled.
> But I can try to split that into 3 separate commits.
They won't actually do anything until we detect the Marvell IP.
> > > +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.
> I will check that, but code is checking alignment of that pointer.
A cast won't do anything to fix alignment issues.
> > > 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.
> Won't it cause big code duplication? There are some differences, but whole
> Part of SPI stig mode configuration is the same.
No more than having a bunch of functions which are called a single time
with checks in them will?
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists