> Il 23/04/24 12:16, Lorenzo Bianconi ha scritto: [...] > > + > > + err = airoha_snand_nfi_config(as_ctrl); > > + if (err) > > + return err; > > + > > + dma_sync_single_for_device(as_ctrl->dev, as_dev->dma_addr, > > + as_dev->buf_len, DMA_BIDIRECTIONAL); > > + mb(); > > Are you sure you need this memory barrier here? > > P.S.: Just in case... any other write or read will anyway add a barrier ;-) > ack, I will fix it in v4. > > + > > + /* set dma addr */ > > + err = regmap_write(as_ctrl->regmap_nfi, REG_SPI_NFI_STRADDR, > > + as_dev->dma_addr); > > + if (err) > > + return err; > > + > > ..snip.. > > > + > > +static ssize_t airoha_snand_dirmap_write(struct spi_mem_dirmap_desc *desc, > > + u64 offs, size_t len, const void *buf) > > +{ > > + struct spi_device *spi = desc->mem->spi; > > + 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; > > + > > + dma_sync_single_for_cpu(as_ctrl->dev, as_dev->dma_addr, > > + as_dev->buf_len, DMA_BIDIRECTIONAL); > > + memcpy(as_dev->txrx_buf + offs, buf, len); > > + dma_sync_single_for_device(as_ctrl->dev, as_dev->dma_addr, > > + as_dev->buf_len, DMA_BIDIRECTIONAL); > > + mb(); > > + > > + err = airoha_snand_set_mode(as_ctrl, SPI_MODE_DMA); > > + if (err < 0) > > + return err; > > + > > + err = airoha_snand_nfi_config(as_ctrl); > > + if (err) > > + return err; > > + > > + if (op->cmd.opcode == SPI_NAND_OP_PROGRAM_LOAD_QUAD || > > + op->cmd.opcode == SPI_NAND_OP_PROGRAM_LOAD_RAMDON_QUAD) > > + wr_mode = 2; > > I'm mostly sure that this '2' is supposed to be BIT(1) instead ack, I will fix it in v4. > > > + else > > + wr_mode = 0; > > + > > + err = regmap_write(as_ctrl->regmap_nfi, REG_SPI_NFI_STRADDR, > > + as_dev->dma_addr); > > + if (err) > > + return err; > > + > > + val = as_ctrl->nfi_cfg.sec_size * as_ctrl->nfi_cfg.sec_num; > > + val = FIELD_PREP(SPI_NFI_PROG_LOAD_BYTE_NUM, val); > > val = FIELD_PREP(SPI_NFI_PROG_LOAD_BYTE_NUM, > as_ctrl->nfi_cfg.sec_size * as_ctrl->nfi_cfg.sec_num); > > Being this 100 cols eventually, you wouldn't even need two lines - but > if you don't want to go 100, it's still more readable (imo) like this, > even if in two lines. I prefer to be below 79 columns, so I will go for the 2 lines :) I will fix it in v4. > > > + err = regmap_update_bits(as_ctrl->regmap_nfi, > > + REG_SPI_NFI_SNF_MISC_CTL2, > > + SPI_NFI_PROG_LOAD_BYTE_NUM, val); > > + if (err) > > + return err; > > + > > + err = regmap_write(as_ctrl->regmap_nfi, REG_SPI_NFI_PG_CTL1, > > + FIELD_PREP(SPI_NFI_PG_LOAD_CMD, > > + op->cmd.opcode)); > > + if (err) > > + return err; > > + > > + err = regmap_write(as_ctrl->regmap_nfi, REG_SPI_NFI_SNF_MISC_CTL, > > + FIELD_PREP(SPI_NFI_DATA_READ_WR_MODE, wr_mode)); > > + if (err) > > + return err; > > + > > ..snip.. > > > + > > +#define SPI_NAND_CACHE_SIZE (4096 + 256) > > #include <linux/sizes.h> <--- at the beginning > > #define SPI_NAND_CACHE_SIZE (SZ_4K + SZ_256) > > ...there are macros for that, might as well use them, right? :-) ack, I will fix it in v4. > > > + > > +static int airoha_snand_setup(struct spi_device *spi) > > +{ > > + struct airoha_snand_dev *as_dev = spi_get_ctldata(spi); > > + struct airoha_snand_ctrl *as_ctrl; > > + > > + as_dev = kzalloc(sizeof(*as_dev), GFP_KERNEL); > > + if (!as_dev) > > + return -ENOMEM; > > + > > + spi_set_ctldata(spi, as_dev); > > + as_dev->data_need_update = true; > > + > > + /* prepare device buffer */ > > + as_dev->buf_len = SPI_NAND_CACHE_SIZE; > > + as_dev->txrx_buf = kzalloc(as_dev->buf_len, GFP_KERNEL); > > + if (!as_dev->txrx_buf) > > + goto error_dev_free; > > + > > + as_ctrl = spi_controller_get_devdata(spi->controller); > > + as_dev->dma_addr = dma_map_single(as_ctrl->dev, as_dev->txrx_buf, > > + as_dev->buf_len, DMA_BIDIRECTIONAL); > > + if (dma_mapping_error(as_ctrl->dev, as_dev->dma_addr)) > > + goto error_buf_free; > > + > > + return 0; > > + > > +error_buf_free: > > + kfree(as_dev->txrx_buf); > > +error_dev_free: > > + kfree(as_dev); > > + > > + return -EINVAL; > > +} > > + > > +static void airoha_snand_cleanup(struct spi_device *spi) > > +{ > > + 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); > > + kfree(as_dev->txrx_buf); > > + > > + spi_set_ctldata(spi, NULL); > > +} > > + > > +static int airoha_snand_nfi_setup(struct airoha_snand_ctrl *as_ctrl) > > +{ > > + u32 val, sec_size, sec_num; > > + int err; > > + > > + err = regmap_read(as_ctrl->regmap_nfi, REG_SPI_NFI_CON, &val); > > + if (err) > > + return err; > > + > > + sec_num = FIELD_GET(SPI_NFI_SEC_NUM, val); > > + > > + err = regmap_read(as_ctrl->regmap_nfi, > > + REG_SPI_NFI_SECCUS_SIZE, &val); > > + if (err) > > + return err; > > + > > + sec_size = FIELD_GET(SPI_NFI_CUS_SEC_SIZE, val); > > + > > + /* init default value */ > > + as_ctrl->nfi_cfg.sec_size = sec_size; > > + as_ctrl->nfi_cfg.sec_num = sec_num; > > + as_ctrl->nfi_cfg.page_size = rounddown(sec_size * sec_num, 1024); > > + as_ctrl->nfi_cfg.spare_size = 16; > > + > > + err = airoha_snand_nfi_init(as_ctrl); > > + if (err) > > + return err; > > + > > + return airoha_snand_nfi_config(as_ctrl); > > +} > > + > > +static const struct regmap_config spi_ctrl_regmap_config = { > > + .name = "ctrl", > > + .reg_bits = 32, > > + .val_bits = 32, > > + .reg_stride = 4, > > + .max_register = REG_SPI_CTRL_NFI2SPI_EN, > > +}; > > + > > +static const struct regmap_config spi_nfi_regmap_config = { > > + .name = "nfi", > > + .reg_bits = 32, > > + .val_bits = 32, > > + .reg_stride = 4, > > + .max_register = REG_SPI_NFI_SNF_NFI_CNFG, > > +}; > > + > > +static const struct of_device_id airoha_snand_ids[] = { > > + { .compatible = "airoha,en7581-snand" }, > > + { } > > { /* sentinel */ } > > ...please! ack, I will fix it in v4. Regards, Lorenzo > > > +}; > > +MODULE_DEVICE_TABLE(of, airoha_snand_ids); > > + > > Cheers, > Angelo > >
Attachment:
signature.asc
Description: PGP signature