RE: [PATCH RFC 1/4] spi: spi-mem: Add set_iofv() callback

Biju Das
2023-11-09T08:28:27Z

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH RFC 1/4] spi: spi-mem: Add set_iofv() callback
>
> Hi Biju,
>
> On Wed, Nov 8, 2023 at 6:12 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > As per section 8.14 on the AT25QL128A hardware manual,
> > IO0..IO3 must be set to Hi-Z state for this flash for fast read quad IO.
> > Snippet from HW manual section 8.14:
> > The upper nibble of the Mode(M7-4) controls the length of the next
> > FAST Read Quad IO instruction through the inclusion or exclusion of
> > the first byte instruction code. The lower nibble bits of the
> > Mode(M3-0) are don't care. However, the IO pins must be high-impedance
> > before the falling edge of the first data out clock.
> >
> > Add set_iofv() callback for configuring IO fixed values to control the
> > pin state.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/spi/spi-mem.c
> > +++ b/drivers/spi/spi-mem.c
> > @@ -297,6 +297,26 @@ static void spi_mem_access_end(struct spi_mem *mem)
> > pm_runtime_put(ctlr->dev.parent); }
> >
> > +/**
> > + * spi_mem_set_iofv() - Set IO fixed values to control the pin state
> > + * @mem: the SPI memory
> > + * @val: the IO fixed values
>
> Please document the meaning of this value (i.e. what does a set or cleared
> bit mean?).

I will document the value as below, 2 bits for setting IO fixed value for a pin and
their values are

00: The output value is 0.
01: The output value is 1.
10: The output value is that of the last bit in the previous transfer (the pin is placed
in the Hi-Z state if that was the case in the previous transfer).
11: The pin is placed in the Hi-Z state.

Cheers,
Biju

>
> > + *
> > + * Set IO fixed values to control the pin state.
> > + *
> > + * Return: 0 in case of success, a negative error code otherwise.
> > + */
> > +int spi_mem_set_iofv(struct spi_mem *mem, u32 val) {
> > + struct spi_controller *ctlr = mem->spi->controller;
> > +
> > + if (ctlr->mem_ops && ctlr->mem_ops->set_iofv)
> > + return ctlr->mem_ops->set_iofv(mem, val);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(spi_mem_set_iofv);
> > +
> > /**
> > * spi_mem_exec_op() - Execute a memory operation
> > * @mem: the SPI memory
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
>
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
> -- Linus Torvalds