[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aKnTYNsaBIkh6OBS@lore-rh-laptop>
Date: Sat, 23 Aug 2025 16:42:40 +0200
From: Lorenzo Bianconi <lorenzo@...nel.org>
To: Mikhail Kshevetskiy <mikhail.kshevetskiy@...sys.eu>
Cc: Ray Liu <ray.liu@...oha.com>, Mark Brown <broonie@...nel.org>,
Jyothi Kumar Seerapu <quic_jseerapu@...cinc.com>,
linux-arm-kernel@...ts.infradead.org, linux-spi@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 03/13] spi: airoha: add support of dual/quad wires spi
modes
>
> On 23.08.2025 11:16, Lorenzo Bianconi wrote:
> > [...]
> >> -static int airoha_snand_write_data(struct airoha_snand_ctrl *as_ctrl, u8 cmd,
> >> - const u8 *data, int len)
> >> +static int airoha_snand_write_data(struct airoha_snand_ctrl *as_ctrl,
> >> + const u8 *data, int len, int buswidth)
> >> {
> >> int i, data_len;
> >> + u8 cmd;
> >> +
> >> + switch (buswidth) {
> >> + case 0:
> >> + case 1:
> >> + cmd = SNAND_FIFO_TX_BUSWIDTH_SINGLE;
> >> + break;
> >> + case 2:
> >> + cmd = SNAND_FIFO_TX_BUSWIDTH_DUAL;
> >> + break;
> >> + case 4:
> >> + cmd = SNAND_FIFO_TX_BUSWIDTH_QUAD;
> >> + break;
> >> + default:
> >> + return -EINVAL;
> >> + }
> > Since this is used in airoha_snand_write_data() and in airoha_snand_read_data()
> > I guess you can define a routine for it.
>
> It's not the same. It looks similar, but different constants are used
> for rx and tx cases.
ops, I missed TX vs RX.
Regards,
Lorenzo
>
> >
> >>
> >> for (i = 0; i < len; i += data_len) {
> >> int err;
> >> @@ -409,16 +433,32 @@ static int airoha_snand_write_data(struct airoha_snand_ctrl *as_ctrl, u8 cmd,
> >> return 0;
> >> }
> >>
> >> -static int airoha_snand_read_data(struct airoha_snand_ctrl *as_ctrl, u8 *data,
> >> - int len)
> >> +static int airoha_snand_read_data(struct airoha_snand_ctrl *as_ctrl,
> >> + u8 *data, int len, int buswidth)
> >> {
> >> int i, data_len;
> >> + u8 cmd;
> >> +
> >> + switch (buswidth) {
> >> + case 0:
> >> + case 1:
> >> + cmd = SNAND_FIFO_RX_BUSWIDTH_SINGLE;
> >> + break;
> >> + case 2:
> >> + cmd = SNAND_FIFO_RX_BUSWIDTH_DUAL;
> >> + break;
> >> + case 4:
> >> + cmd = SNAND_FIFO_RX_BUSWIDTH_QUAD;
> >> + break;
> >> + default:
> >> + return -EINVAL;
> >> + }
> >>
> >> for (i = 0; i < len; i += data_len) {
> >> int err;
> >>
> >> data_len = min(len - i, SPI_MAX_TRANSFER_SIZE);
> >> - err = airoha_snand_set_fifo_op(as_ctrl, 0xc, data_len);
> >> + err = airoha_snand_set_fifo_op(as_ctrl, cmd, data_len);
> >> if (err)
> >> return err;
> >>
> >> @@ -895,12 +935,27 @@ static ssize_t airoha_snand_dirmap_write(struct spi_mem_dirmap_desc *desc,
> >> static int airoha_snand_exec_op(struct spi_mem *mem,
> >> const struct spi_mem_op *op)
> >> {
> >> - u8 data[8], cmd, opcode = op->cmd.opcode;
> >> struct airoha_snand_ctrl *as_ctrl;
> >> - int i, err;
> >> + char buf[20], *data;
> >> + int i, err, op_len, addr_len, dummy_len;
> > nit: can you please respect the 'reverse christmas tree' here?
> will fix
> >
> >>
> >> as_ctrl = spi_controller_get_devdata(mem->spi->controller);
> >>
> >> + op_len = op->cmd.nbytes;
> >> + addr_len = op->addr.nbytes;
> >> + dummy_len = op->dummy.nbytes;
> >> +
> >> + if (op_len + dummy_len + addr_len > sizeof(buf))
> >> + return -EIO;
> >> +
> >> + data = buf;
> >> + for (i = 0; i < op_len; i++)
> >> + *data++ = op->cmd.opcode >> (8 * (op_len - i - 1));
> >> + for (i = 0; i < addr_len; i++)
> >> + *data++ = op->addr.val >> (8 * (addr_len - i - 1));
> >> + for (i = 0; i < dummy_len; i++)
> >> + *data++ = 0xff;
> >> +
> >> /* switch to manual mode */
> >> err = airoha_snand_set_mode(as_ctrl, SPI_MODE_MANUAL);
> >> if (err < 0)
> >> @@ -911,40 +966,40 @@ static int airoha_snand_exec_op(struct spi_mem *mem,
> >> return err;
> >>
> >> /* opcode */
> >> - err = airoha_snand_write_data(as_ctrl, 0x8, &opcode, sizeof(opcode));
> >> + data = buf;
> >> + err = airoha_snand_write_data(as_ctrl, data, op_len,
> >> + op->cmd.buswidth);
> >> if (err)
> >> return err;
> >>
> >> /* addr part */
> >> - cmd = opcode == SPI_NAND_OP_GET_FEATURE ? 0x11 : 0x8;
> >> - put_unaligned_be64(op->addr.val, data);
> >> -
> >> - for (i = ARRAY_SIZE(data) - op->addr.nbytes;
> >> - i < ARRAY_SIZE(data); i++) {
> >> - err = airoha_snand_write_data(as_ctrl, cmd, &data[i],
> >> - sizeof(data[0]));
> >> + data += op_len;
> >> + if (addr_len) {
> >> + err = airoha_snand_write_data(as_ctrl, data, addr_len,
> >> + op->addr.buswidth);
> >> if (err)
> >> return err;
> >> }
> >>
> >> /* dummy */
> >> - data[0] = 0xff;
> >> - for (i = 0; i < op->dummy.nbytes; i++) {
> >> - err = airoha_snand_write_data(as_ctrl, 0x8, &data[0],
> >> - sizeof(data[0]));
> >> + data += addr_len;
> >> + if (dummy_len) {
> >> + err = airoha_snand_write_data(as_ctrl, data, dummy_len,
> >> + op->dummy.buswidth);
> >> if (err)
> >> return err;
> >> }
> >>
> >> /* data */
> >> - if (op->data.dir == SPI_MEM_DATA_IN) {
> >> - err = airoha_snand_read_data(as_ctrl, op->data.buf.in,
> >> - op->data.nbytes);
> >> - if (err)
> >> - return err;
> >> - } else {
> >> - err = airoha_snand_write_data(as_ctrl, 0x8, op->data.buf.out,
> >> - op->data.nbytes);
> >> + if (op->data.nbytes) {
> >> + if (op->data.dir == SPI_MEM_DATA_IN)
> >> + err = airoha_snand_read_data(as_ctrl, op->data.buf.in,
> >> + op->data.nbytes,
> >> + op->data.buswidth);
> >> + else
> >> + err = airoha_snand_write_data(as_ctrl, op->data.buf.out,
> >> + op->data.nbytes,
> >> + op->data.buswidth);
> >> if (err)
> >> return err;
> >> }
> >> --
> >> 2.50.1
> >>
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists