Re: [PATCH v6 3/3] spi: airoha: Add spi-nand flash controller driver

Lorenzo Bianconi
2024-05-03T13:20:19Z

[-- Attachment #1: Type: text/plain, Size: 4776 bytes --]

> On Fri, May 3, 2024 at 3:15 PM Lorenzo Bianconi <lorenzo@kernel.org> wrote:
> >
> > > On Wed, May 01, 2024 at 04:06:43PM +0200, Lorenzo Bianconi wrote:
> > > > Introduce support for spi-nand driver of the Airoha NAND Flash Interface
> > > > found on Airoha ARM SoCs.
> > >
> > > This doesn't apply against current code, please check and resend.
> >
> > Hi Mark,
> >
> > patch v6 3/3 has just a couple of cosmetic changes requested by Andy with
> > respect to v5 3/3.
> >
> > @Andy: do you think we can drop these changes or do you prefer to add them? (in
> > the latter case I can post an incremental patch).
>
> I am not sure what this is about, do you mean the changes asked by me
> made this driver not applicable?
>

These are the only changes between patch v5 3/3 (applied by Mark) and patch v6 3/3:

--- /tmp/spi-airoha-snfi.c 2024-05-03 15:13:35.020903623 +0200
+++ drivers/spi/spi-airoha-snfi.c 2024-05-03 15:13:41.836924124 +0200
@@ -638,9 +638,9 @@
u64 offs, size_t len, void *buf)
{
struct spi_device *spi = desc->mem->spi;
+ struct airoha_snand_ctrl *as_ctrl = spi_controller_get_devdata(spi->controller);
struct airoha_snand_dev *as_dev = spi_get_ctldata(spi);
struct spi_mem_op *op = &desc->info.op_tmpl;
- struct airoha_snand_ctrl *as_ctrl;
u32 val, rd_mode;
int err;

@@ -661,7 +661,6 @@
break;
}

- as_ctrl = spi_controller_get_devdata(spi->controller);
err = airoha_snand_set_mode(as_ctrl, SPI_MODE_DMA);
if (err < 0)
return err;
@@ -768,13 +767,12 @@
u64 offs, size_t len, const void *buf)
{
struct spi_device *spi = desc->mem->spi;
+ struct airoha_snand_ctrl *as_ctrl = spi_controller_get_devdata(spi->controller);
struct airoha_snand_dev *as_dev = spi_get_ctldata(spi);
struct spi_mem_op *op = &desc->info.op_tmpl;
- struct airoha_snand_ctrl *as_ctrl;
u32 wr_mode, val;
int err;

- as_ctrl = spi_controller_get_devdata(spi->controller);
err = airoha_snand_set_mode(as_ctrl, SPI_MODE_MANUAL);
if (err < 0)
return err;
@@ -885,12 +883,12 @@
static int airoha_snand_exec_op(struct spi_mem *mem,
const struct spi_mem_op *op)
{
- struct airoha_snand_dev *as_dev = spi_get_ctldata(mem->spi);
+ struct spi_device *spi = mem->spi;
+ struct airoha_snand_ctrl *as_ctrl = spi_controller_get_devdata(spi->controller);
+ struct airoha_snand_dev *as_dev = spi_get_ctldata(spi);
u8 data[8], cmd, opcode = op->cmd.opcode;
- struct airoha_snand_ctrl *as_ctrl;
int i, err;

- as_ctrl = spi_controller_get_devdata(mem->spi->controller);
if (opcode == SPI_NAND_OP_PROGRAM_EXECUTE &&
op->addr.val == as_dev->cur_page_num) {
as_dev->data_need_update = true;
@@ -965,11 +963,9 @@

static int airoha_snand_setup(struct spi_device *spi)
{
- struct airoha_snand_ctrl *as_ctrl;
+ struct airoha_snand_ctrl *as_ctrl = spi_controller_get_devdata(spi->controller);
struct airoha_snand_dev *as_dev;

- as_ctrl = spi_controller_get_devdata(spi->controller);
-
as_dev = devm_kzalloc(as_ctrl->dev, sizeof(*as_dev), GFP_KERNEL);
if (!as_dev)
return -ENOMEM;
@@ -994,10 +990,9 @@

static void airoha_snand_cleanup(struct spi_device *spi)
{
+ struct airoha_snand_ctrl *as_ctrl = spi_controller_get_devdata(spi->controller);
struct airoha_snand_dev *as_dev = spi_get_ctldata(spi);
- struct airoha_snand_ctrl *as_ctrl;

- as_ctrl = spi_controller_get_devdata(spi->controller);
dma_unmap_single(as_ctrl->dev, as_dev->dma_addr,
as_dev->buf_len, DMA_BIDIRECTIONAL);
spi_set_ctldata(spi, NULL);
@@ -1049,12 +1044,6 @@
.max_register = REG_SPI_NFI_SNF_NFI_CNFG,
};

-static const struct of_device_id airoha_snand_ids[] = {
- { .compatible = "airoha,en7581-snand" },
- { /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, airoha_snand_ids);
-
static int airoha_snand_probe(struct platform_device *pdev)
{
struct airoha_snand_ctrl *as_ctrl;
@@ -1114,6 +1103,12 @@
return devm_spi_register_controller(dev, ctrl);
}

+static const struct of_device_id airoha_snand_ids[] = {
+ { .compatible = "airoha,en7581-snand" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, airoha_snand_ids);
+
static struct platform_driver airoha_snand_driver = {
.driver = {
.name = "airoha-spi",

As you can see the only differences are your comments about patch v5 3/3:
- initialize struct airoha_snand_ctrl *as_ctrl initialization as we did
for struct airoha_snand_dev *as_dev pointer.
- move struct of_device_id airoha_snand_ids below airoha_snand_probe routine.

My question is if we can drop those changes (they are just cosmetics) or do
you prefer to add them as incremental patch.

Regards,
Lorenzo

>
> --
> With Best Regards,
> Andy Shevchenko

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]