Re: [PATCH v3 3/3] spi: airoha: add SPI-NAND Flash controller driver

[Andy Shevchenko <andy.shevchenko@xxxxxxxxx>]

On Wed, Apr 24, 2024 at 1:07 PM Lorenzo Bianconi <lorenzo@xxxxxxxxxx> wrote:
> > Tue, Apr 23, 2024 at 12:16:37PM +0200, Lorenzo Bianconi kirjoitti:

...

> > > +   /* addr part */
> > > +   for (i = 0; i < op->addr.nbytes; i++) {
> > > +           u8 cmd = opcode == SPI_NAND_OP_GET_FEATURE ? 0x11 : 0x8;
> > > +
> > > +           data = op->addr.val >> ((op->addr.nbytes - i - 1) * 8);
> >
> > Seems like you wanted to have always the same endianess and hence can be done
> > outside the loop via cpu_to_xxx()?
>
> sorry, I did not get what you mean here, data value relies on the loop
> iteration.

  u8 byte_stream[8];
  u8 cmd;

  cmd = ...;

  // find what suits your case(s)
  put_unaligned_be64(op->addr.val, byte_stream);
  for-loop {
     err = _write_data();
     ...
  }


> > > +           err = airoha_snand_write_data(as_ctrl, cmd, &data,
> > > +                                         sizeof(data));
> > > +           if (err)
> > > +                   return err;
> > > +   }

...

> > Why not utilising cleanup.h? (__free(), no_free_ptr(), etc)
>
> I guess we can just allocate as_dev and as_dev->txrx_buf with devm_kzalloc()
> here, agree?

If this is a solely part of ->probe() stage, yes.

-- 
With Best Regards,
Andy Shevchenko