Re: [PATCH v16 6/6] soc: amd: Add support for AMD Pensando SoC Controller

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Brad Larson <blarson@amd.com>
Cc: adrian.hunter@intel.com, alcooperx@gmail.com, arnd@arndb.de,
	brendan.higgins@linux.dev, briannorris@chromium.org,
	broonie@kernel.org, catalin.marinas@arm.com, conor+dt@kernel.org,
	davidgow@google.com, devicetree@vger.kernel.org,
	fancer.lancer@gmail.com, gerg@linux-m68k.org, gsomlo@gmail.com,
	hal.feng@starfivetech.com, hasegawa-hitomi@fujitsu.com,
	j.neuschaefer@gmx.net, joel@jms.id.au, kernel@esmil.dk,
	krzk@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	lee.jones@linaro.org, lee@kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mmc@vger.kernel.org,
	linux-spi@vger.kernel.org, p.zabel@pengutronix.de,
	rdunlap@infradead.org, robh+dt@kernel.org, samuel@sholland.org,
	skhan@linuxfoundation.org, suravee.suthikulpanit@amd.com,
	thomas.lendacky@amd.com, tonyhuang.sunplus@gmail.com,
	ulf.hansson@linaro.org, vaishnav.a@ti.com,
	walker.chen@starfivetech.com, will@kernel.org,
	zhuyinbo@loongson.cn
Subject: Re: [PATCH v16 6/6] soc: amd: Add support for AMD Pensando SoC Controller
Date: Wed, 27 Sep 2023 15:59:00 +0300	[thread overview]
Message-ID: <CAHp75Vd3Jj_giyWKhA2OyPrY5xKhyQ6We3qkz9-yDs15F+SFRg@mail.gmail.com> (raw)
In-Reply-To: <20230926200541.35787-1-blarson@amd.com>

On Tue, Sep 26, 2023 at 11:05 PM Brad Larson <blarson@amd.com> wrote:
> On Thu, Sep 21, 2023 at 18:19:57 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Thu, Sep 14, 2023 at 12:52 AM Brad Larson <blarson@amd.com> wrote:

...

> >> +       u8 tx_buf[PENCTRL_MAX_MSG_LEN];
> >> +       u8 rx_buf[PENCTRL_MAX_MSG_LEN];
> >
> > These are not DMA-safe, is this a problem?
>
> It's not a problem, the peripheral is PIO FIFO driven only.

The question was about the SPI controller itself.
Also, depending on the driver it may or may not require DMA-safe
pointers. I believe with the new pump queue used in the SPI core all
drivers that are using it are DMA-safe and the caller needs no
additional care.

...

> >> +       msg = memdup_user((struct penctrl_spi_xfer *)arg, size);
> >> +       if (IS_ERR(msg)) {
> >> +               ret = PTR_ERR(msg);
> >> +               goto out_unlock;
> >> +       }
> >
> > Wondering if you can start using cleanup.h.
>
> Perhaps if recommended, I don't see DEFINE_(FREE,UNLOCK,...) being used.

It's guard()() and scoped_guard() for locks and __free() for the allocations.

Plenty of uses in a few modules already (talking about Linux Next snapshot).

...

> >> +       spi->chip_select = current_cs;
> >
> > spi_set_chipselect()
>
> Yes, I'll change to inline function spi_set_chipselect(spi, 0, current_cs).  The
> second arg must be legacy as its unused.

Actually, it's a placeholder for the future support of indexed CS'.

...

> >> +       u8 txbuf[3];
> >> +       u8 rxbuf[1];
> >
> > Not DMA-safe. Is it a problem?
>
> Not a problem, the peripheral is PIO only using FIFOs.

Same as somewhere above.

-- 
With Best Regards,
Andy Shevchenko