spi-ar934x: Using GPIO CS issue

spi-ar934x: Using GPIO CS issue

[Dave Bender]

Under the mach-* era of configuring boards, I was able to use a second
SPI NOR flash with a GPIO chip select, as in:

static struct spi_board_info spi_info[] = {
       { ... },
       {
               .bus_num        = 0,
               .chip_select    = 1,
               .max_speed_hz   = 25000000,
               .modalias       = "m25p80",
       },
};

static int cs_gpios[2] = {
       -ENOENT,
       11,
};

static struct ath79_spi_platform_data mtriq_spi_data __initdata = {
       .bus_num = 0
       ,.num_chipselect = 2
       ,.cs_gpios = cs_gpios
};

static void __init board_setup(){
 ath79_register_spi(&spi_data, spi_info, 2);
}


However, under the new dts regime, I try to use a CS gpio but cannot
communicate successfully to the chip:

&spi {
    status = "okay";
    cs-gpios = <0>,<&gpio 11 GPIO_ACTIVE_LOW>;

    flash@1 {
    compatible = "jedec,spi-nor";
    spi-max-frequency = <25000000>;
    reg = <1>;
  };
};

Am I missing something here?

Re: spi-ar934x: Using GPIO CS issue

[Geert Uytterhoeven]

Hi Dave.

On Tue, Nov 2, 2021 at 11:31 PM Dave Bender <codehero@gmail.com> wrote:
> Under the mach-* era of configuring boards, I was able to use a second
> SPI NOR flash with a GPIO chip select, as in:
>
> static struct spi_board_info spi_info[] = {
>        { ... },
>        {
>                .bus_num        = 0,
>                .chip_select    = 1,
>                .max_speed_hz   = 25000000,
>                .modalias       = "m25p80",
>        },
> };
>
> static int cs_gpios[2] = {
>        -ENOENT,
>        11,
> };
>
> static struct ath79_spi_platform_data mtriq_spi_data __initdata = {
>        .bus_num = 0
>        ,.num_chipselect = 2
>        ,.cs_gpios = cs_gpios
> };
>
> static void __init board_setup(){
>  ath79_register_spi(&spi_data, spi_info, 2);
> }
>
>
> However, under the new dts regime, I try to use a CS gpio but cannot
> communicate successfully to the chip:
>
> &spi {
>     status = "okay";
>     cs-gpios = <0>,<&gpio 11 GPIO_ACTIVE_LOW>;
>
>     flash@1 {
>     compatible = "jedec,spi-nor";
>     spi-max-frequency = <25000000>;
>     reg = <1>;
>   };
> };
>
> Am I missing something here?

Does it work if you change GPIO_ACTIVE_LOW to GPIO_ACTIVE_HIGH?

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

Re: spi-ar934x: Using GPIO CS issue

[Dave Bender]

Geert,
  Thanks for the response.
  There is a pullup resistor on that CS line, so that GPIO must be active low.
  Digging deeper into the code, it looks like David Bauer made some
changes that limit the spi driver's CS usage to only the CS0, CS1, CS2
pins.
  There is also no bounds checking on the CS value.
  I don't understand how this driver can even use generic GPIO if it
forces use of the CS bits on the SPI_IOC register.

static int ar934x_spi_transfer_one_message(struct spi_controller *master,
             struct spi_message *m)
{
   ....
   ....
    for (trx_done = 0; trx_done < t->len; trx_done += 4) {

   .....
      reg = AR934X_SPI_SHIFT_VAL(spi->chip_select, term,
               trx_cur * 8);
      iowrite32(reg, sp->base + AR934X_SPI_REG_SHIFT_CTRL);



-Dave

On Thu, Nov 4, 2021 at 5:50 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Dave.
>
> On Tue, Nov 2, 2021 at 11:31 PM Dave Bender <codehero@gmail.com> wrote:
> > Under the mach-* era of configuring boards, I was able to use a second
> > SPI NOR flash with a GPIO chip select, as in:
> >
> > static struct spi_board_info spi_info[] = {
> >        { ... },
> >        {
> >                .bus_num        = 0,
> >                .chip_select    = 1,
> >                .max_speed_hz   = 25000000,
> >                .modalias       = "m25p80",
> >        },
> > };
> >
> > static int cs_gpios[2] = {
> >        -ENOENT,
> >        11,
> > };
> >
> > static struct ath79_spi_platform_data mtriq_spi_data __initdata = {
> >        .bus_num = 0
> >        ,.num_chipselect = 2
> >        ,.cs_gpios = cs_gpios
> > };
> >
> > static void __init board_setup(){
> >  ath79_register_spi(&spi_data, spi_info, 2);
> > }
> >
> >
> > However, under the new dts regime, I try to use a CS gpio but cannot
> > communicate successfully to the chip:
> >
> > &spi {
> >     status = "okay";
> >     cs-gpios = <0>,<&gpio 11 GPIO_ACTIVE_LOW>;
> >
> >     flash@1 {
> >     compatible = "jedec,spi-nor";
> >     spi-max-frequency = <25000000>;
> >     reg = <1>;
> >   };
> > };
> >
> > Am I missing something here?
>
> Does it work if you change GPIO_ACTIVE_LOW to GPIO_ACTIVE_HIGH?
>
> 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

Re: spi-ar934x: Using GPIO CS issue

[Geert Uytterhoeven]

Hi Dave,

On Thu, Nov 4, 2021 at 3:58 PM Dave Bender <codehero@gmail.com> wrote:
> On Thu, Nov 4, 2021 at 5:50 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > Does it work if you change GPIO_ACTIVE_LOW to GPIO_ACTIVE_HIGH?
>
>   There is a pullup resistor on that CS line, so that GPIO must be active low.

I do believe you.  But there's been some issues with inverted CS
gpio behavior when switching from board files to DT, so I thought
I'd better ask to try...

>   Digging deeper into the code, it looks like David Bauer made some
> changes that limit the spi driver's CS usage to only the CS0, CS1, CS2
> pins.
>   There is also no bounds checking on the CS value.
>   I don't understand how this driver can even use generic GPIO if it
> forces use of the CS bits on the SPI_IOC register.

OK, that sounds like a reasonable culprit...

>
> static int ar934x_spi_transfer_one_message(struct spi_controller *master,
>              struct spi_message *m)
> {
>    ....
>    ....
>     for (trx_done = 0; trx_done < t->len; trx_done += 4) {
>
>    .....
>       reg = AR934X_SPI_SHIFT_VAL(spi->chip_select, term,
>                trx_cur * 8);
>       iowrite32(reg, sp->base + AR934X_SPI_REG_SHIFT_CTRL);

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

Re: spi-ar934x: Using GPIO CS issue

[Dave Bender]

I've tried different combinations of GPIO_ACTIVE_HIGH/LOW.
I modified the ar9134x-spi.c driver to allow gpiod descriptors.
I've tried creating a virtual CS of 3, using cs-gpios.

Nothing seems to work and all I get are JEDEC IDs of 0's.

Is there something I else I am missing here?
For reference, this is a Carambola 2 module on a custom board, with an
extra SPI 32Mb chip on GPIO11, which is not "HW" chip select pin.

On Thu, Nov 4, 2021 at 11:20 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Dave,
>
> On Thu, Nov 4, 2021 at 3:58 PM Dave Bender <codehero@gmail.com> wrote:
> > On Thu, Nov 4, 2021 at 5:50 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> > > Does it work if you change GPIO_ACTIVE_LOW to GPIO_ACTIVE_HIGH?
> >
> >   There is a pullup resistor on that CS line, so that GPIO must be active low.
>
> I do believe you.  But there's been some issues with inverted CS
> gpio behavior when switching from board files to DT, so I thought
> I'd better ask to try...
>
> >   Digging deeper into the code, it looks like David Bauer made some
> > changes that limit the spi driver's CS usage to only the CS0, CS1, CS2
> > pins.
> >   There is also no bounds checking on the CS value.
> >   I don't understand how this driver can even use generic GPIO if it
> > forces use of the CS bits on the SPI_IOC register.
>
> OK, that sounds like a reasonable culprit...
>
> >
> > static int ar934x_spi_transfer_one_message(struct spi_controller *master,
> >              struct spi_message *m)
> > {
> >    ....
> >    ....
> >     for (trx_done = 0; trx_done < t->len; trx_done += 4) {
> >
> >    .....
> >       reg = AR934X_SPI_SHIFT_VAL(spi->chip_select, term,
> >                trx_cur * 8);
> >       iowrite32(reg, sp->base + AR934X_SPI_REG_SHIFT_CTRL);
>
> 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