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

Lorenzo Bianconi lorenzo at kernel.org
Wed Apr 24 07:38:02 PDT 2024


> On Wed, Apr 24, 2024 at 1:07 PM Lorenzo Bianconi <lorenzo at kernel.org> 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();
>      ...
>   }

ack, I will fix it in v4.

Rregards,
Lorenzo

> 
> 
> > > > +           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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20240424/7113312c/attachment.sig>


More information about the linux-arm-kernel mailing list