public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed

From: Chi-Wen Weng <cwweng.linux@gmail•com>
To: Mark Brown <broonie@kernel•org>
Cc: robh@kernel•org, krzk+dt@kernel•org, conor+dt@kernel•org,
	linux-arm-kernel@lists•infradead.org, linux-spi@vger•kernel.org,
	devicetree@vger•kernel.org, linux-kernel@vger•kernel.org,
	cwweng@nuvoton•com
Subject: Re: [PATCH 2/2] spi: Add Nuvoton MA35D1 QSPI controller support
Date: Wed, 3 Jun 2026 19:03:16 +0800	[thread overview]
Message-ID: <5d6ecdf7-1987-4ecf-ad59-06595742cc8f@gmail.com> (raw)
In-Reply-To: <d32634eb-4eed-46e6-b378-d42df0b1c75e@sirena.org.uk>

Hi Mark,

Thanks for the review.

I will address these in v2:
- add "depends on ARCH_MA35 || COMPILE_TEST" to the Kconfig entry;
- convert the file header to // comments;
- split the low-level CS register update from the SPI core .set_cs()
   callback, and make the spi-mem direct CS path handle SPI_CS_HIGH
   explicitly;
- use op->max_freq for spi-mem operations instead of spi->max_speed_hz.

I will also fix the subject lines in the next version.

Best regards,
Chi-Wen Weng

Mark Brown 於 2026/6/3 下午 05:30 寫道:
> On Wed, Jun 03, 2026 at 12:35:51PM +0800, Chi-Wen Weng wrote:
>
>> +config SPI_MA35D1_QSPI
>> +	tristate "Nuvoton MA35D1 QSPI Controller"
>> +	help
>> +	  This driver provides support for Nuvoton MA35D1
>> +	  QSPI controller in master mode.
>> +
> Other drivers for this SoC seem to have ARCH_MA35 || COMPILE_TEST?
>
>> @@ -0,0 +1,579 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Nuvoton MA35D1 QSPI controller driver
>> + *
>> + * Copyright (c) 2026 Nuvoton Technology Corp.
>> + * Author: Chi-Wen Weng <cwweng@nuvoton•com>
>> + */
> Please make the entire comment a C++ one so things look more
> intentional.
>
>> +static void nuvoton_qspi_set_cs(struct spi_device *spi, bool enable)
>> +{
>> +	struct nuvoton_qspi *qspi = spi_controller_get_devdata(spi->controller);
>> +	unsigned int cs = spi_get_chipselect(spi, 0);
>> +	u32 mask;
>> +	u32 val;
>> +
>> +	if (cs == 0)
>> +		mask = NUVOTON_QSPI_SSCTL_SS0_MASK;
>> +	else
>> +		mask = NUVOTON_QSPI_SSCTL_SS1_MASK;
>> +
>> +	val = nuvoton_qspi_read(qspi, NUVOTON_QSPI_SSCTL_OFFSET);
>> +
>> +	/* SPI core passes enable=true when CS is asserted (typically active-low) */
>> +	if (enable)
>> +		val |= mask;
>> +	else
>> +		val &= ~mask;
>> +
>> +	nuvoton_qspi_write(qspi, val, NUVOTON_QSPI_SSCTL_OFFSET);
>> +}
> Note that the core deals with SPI_CS_HIGH, the driver doesn't need to...
>
>> +static int nuvoton_qspi_mem_exec_op(struct spi_mem *mem,
>> +				    const struct spi_mem_op *op)
>> +{
>> +	struct spi_device *spi = mem->spi;
>> +	struct nuvoton_qspi *qspi = spi_controller_get_devdata(spi->controller);
>> +	u8 addr[4];
>> +	int ret;
>> +	int i;
>> +
>> +	ret = nuvoton_qspi_setup_transfer(spi, NULL);
>> +	if (ret)
>> +		return ret;
> This uses spi->max_speed_hz but spi_mem configures p->max_freq which you
> should use (it might be lower).
>
>> +
>> +	nuvoton_qspi_set_cs(spi, true);
> ...except where you're calling in directly at which point the driver
> needs to figure this out.

     prev parent reply	other threads:[~2026-06-03 11:03 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-03  4:35 [PATCH 0/2] Add support for nuvoton ma35d1 qspi controller Chi-Wen Weng
2026-06-03  4:35 ` [PATCH 1/2] dt-bindings: spi: Add for Nuvoton MA35D1 SoC QSPI Controller Chi-Wen Weng
2026-06-03  9:04   ` Mark Brown
2026-06-03  9:29     ` Chi-Wen Weng
2026-06-03 15:24   ` Conor Dooley
2026-06-04  7:07     ` Chi-Wen Weng
2026-06-04  8:55       ` Conor Dooley
2026-06-04  9:25         ` Chi-Wen Weng
2026-06-03  4:35 ` [PATCH 2/2] spi: Add Nuvoton MA35D1 QSPI controller support Chi-Wen Weng
2026-06-03  9:30   ` Mark Brown
2026-06-03 11:03     ` Chi-Wen Weng [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5d6ecdf7-1987-4ecf-ad59-06595742cc8f@gmail.com \
    --to=cwweng.linux@gmail$(echo .)com \
    --cc=broonie@kernel$(echo .)org \
    --cc=conor+dt@kernel$(echo .)org \
    --cc=cwweng@nuvoton$(echo .)com \
    --cc=devicetree@vger$(echo .)kernel.org \
    --cc=krzk+dt@kernel$(echo .)org \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=linux-spi@vger$(echo .)kernel.org \
    --cc=robh@kernel$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox