Introduce cm-fx6 board

Currently we can define CONFIG_SPL_SPI_<any parameter except SPI MODE>. Define CONFIG_SPL_SPI_MODE option, and provide a default value for backwards compatibility.
Default values are also provided for the rest of the spi_flash_probe parameters (like we do in cmd_sf), to help with config file brevity.

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Quoted 6 lines · Nikita Kiryanov

Currently we can define CONFIG_SPL_SPI_<any parameter except SPI MODE>. Define CONFIG_SPL_SPI_MODE option, and provide a default value for backwards compatibility.
Default values are also provided for the rest of the spi_flash_probe parameters (like we do in cmd_sf), to help with config file brevity.

Quoted 3 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

You might actually be even more bold and check if you cannot fall back to the CONFIG_DEFAULT_SPI_MODE etc. What do you think ?

Best regards,
Marek Vasut

Quoted 7 lines · Marek Vasut

Quoted 6 lines · Nikita Kiryanov

Currently we can define CONFIG_SPL_SPI_<any parameter except SPI MODE>. Define CONFIG_SPL_SPI_MODE option, and provide a default value for backwards compatibility.
Default values are also provided for the rest of the spi_flash_probe parameters (like we do in cmd_sf), to help with config file brevity.

Quoted 6 lines

Quoted 3 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

You might actually be even more bold and check if you cannot fall back to the CONFIG_DEFAULT_SPI_MODE etc. What do you think ?

Not a fan of the idea. It will:
- Complicate the #ifdefs
- Complicate the relationship between CONFIG_DEFAULT_SPI_* and CONFIG_SPL_SPI_* #defines
- Not get much use: most boards do not #define CONFIG_DEFAULT_SPI_* values in the config files, and of the ones that do, only two (dra7xx_evm and cm_fx6) use SPI in SPL.

Quoted 15 lines · Nikita Kiryanov

Quoted 7 lines · Marek Vasut

Quoted 6 lines · Nikita Kiryanov

Currently we can define CONFIG_SPL_SPI_<any parameter except SPI MODE>. Define CONFIG_SPL_SPI_MODE option, and provide a default value for backwards compatibility.
Default values are also provided for the rest of the spi_flash_probe parameters (like we do in cmd_sf), to help with config file brevity.

Quoted 6 lines

Quoted 3 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

You might actually be even more bold and check if you cannot fall back to the CONFIG_DEFAULT_SPI_MODE etc. What do you think ?

Quoted 7 lines

Not a fan of the idea. It will:
- Complicate the #ifdefs
- Complicate the relationship between CONFIG_DEFAULT_SPI_* and CONFIG_SPL_SPI_* #defines
- Not get much use: most boards do not #define CONFIG_DEFAULT_SPI_* values in the config files, and of the ones that do, only two (dra7xx_evm and cm_fx6) use SPI in SPL.

On the other hand, it's now only a matter of time until we get CONFIG_TPL_SPI_* m which gives us _another_ set of defines. So the question is -- what is your proposition to keep the amount of new ad-hoc defines low and cater for this case?

Best regards,
Marek Vasut

Quoted 24 lines · Marek Vasut

Quoted 23 lines · Nikita Kiryanov

Quoted 7 lines · Marek Vasut

Quoted 6 lines · Nikita Kiryanov

Currently we can define CONFIG_SPL_SPI_<any parameter except SPI MODE>. Define CONFIG_SPL_SPI_MODE option, and provide a default value for backwards compatibility.
Default values are also provided for the rest of the spi_flash_probe parameters (like we do in cmd_sf), to help with config file brevity.

Quoted 6 lines

Quoted 3 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

You might actually be even more bold and check if you cannot fall back to the CONFIG_DEFAULT_SPI_MODE etc. What do you think ?

Not a fan of the idea. It will:
- Complicate the #ifdefs
- Complicate the relationship between CONFIG_DEFAULT_SPI_* and

CONFIG_SPL_SPI_* #defines

- Not get much use: most boards do not #define CONFIG_DEFAULT_SPI_*

Quoted 7 lines

Quoted 2 lines

values in the config files, and of the ones that do, only two (dra7xx_evm and cm_fx6) use SPI in SPL.

On the other hand, it's now only a matter of time until we get CONFIG_TPL_SPI_* m which gives us _another_ set of defines. So the question is -- what is your proposition to keep the amount of new ad-hoc defines low and cater for this case?

OK I think I may have misunderstood your suggestion. You wanted to replace CONFIG_SPL_SPI_* #defines with CONFIG_DEFAULT_SPI_* #defines, not use both, right?
Based on cursory grepping, this seems possible, though I think CONFIG_SF_DEFAULT_* is a better candidate.

I'll prepare a patch..

MXC SPI driver has a feature whereas a GPIO line can be used as a CS signal. This is set up by joining the CS and GPIO values into a single value using (cs | gpio << 8), and passing it off as a CS value. This breaks the sf probe command, because it is no longer possible to invoke it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio << 8>.

Fix this by introducing a new board function: board_spi_cs_gpio(). When called, board_spi_cs_gpio() will return the gpio number for the cs value it is given.

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Eric Nelson <[email protected]>

Cc Eric Benard <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Tim Harvey <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Quoted 10 lines · Nikita Kiryanov

MXC SPI driver has a feature whereas a GPIO line can be used as a CS signal. This is set up by joining the CS and GPIO values into a single value using (cs | gpio << 8), and passing it off as a CS value. This breaks the sf probe command, because it is no longer possible to invoke it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio << 8>.

Fix this by introducing a new board function: board_spi_cs_gpio(). When called, board_spi_cs_gpio() will return the gpio number for the cs value it is given.

Quoted 8 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Eric Nelson <[email protected]>

Cc Eric Benard <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Tim Harvey <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Just curious, but is this fixing generic SF code or MXC SPI driver ? I'd think the later, but it's not obvious from neither the description nor the subject. I don't quite understand the problem that you're trying to fix either, what happened, did the user command interface change ?

[...]
Best regards,
Marek Vasut

Hi Marek,

Quoted 21 lines · Marek Vasut

Quoted 11 lines · Nikita Kiryanov

MXC SPI driver has a feature whereas a GPIO line can be used as a CS signal. This is set up by joining the CS and GPIO values into a single value using (cs | gpio << 8), and passing it off as a CS value. This breaks the sf probe command, because it is no longer possible to invoke it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio << 8>.

Fix this by introducing a new board function: board_spi_cs_gpio(). When called, board_spi_cs_gpio() will return the gpio number for the cs value it is given.

Quoted 8 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Eric Nelson <[email protected]>

Cc Eric Benard <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Tim Harvey <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Quoted 4 lines

Just curious, but is this fixing generic SF code or MXC SPI driver ? I'd think the later, but it's not obvious from neither the description nor the subject. I don't quite understand the problem that you're trying to fix either, what happened, did the user command interface change ?

The U-Boot shell command "sf probe" can accept a chip select value, but if the SPI device on the other end requires an active chip-select over multiple transactions (achieved in the MXC SPI driver using a GPIO), simply typing something like "sf probe 0" will not work.

This is because whatever the user passes as chip select is propagated to the driver, and the driver expects this value to have GPIO information. So for example, if IMX_GPIO_NR(2, 30) is used to force active chip select 0, then instead of "sf probe 0" the user will have to type "sf probe 15872".

I agree that the subject line should be made a bit more specific to mxc_spi though...

Quoted 3 lines

[...]
Best regards,
Marek Vasut

Quoted 29 lines · Nikita Kiryanov

Hi Marek,

Quoted 21 lines · Marek Vasut

Quoted 11 lines · Nikita Kiryanov

MXC SPI driver has a feature whereas a GPIO line can be used as a CS signal. This is set up by joining the CS and GPIO values into a single value using (cs | gpio << 8), and passing it off as a CS value. This breaks the sf probe command, because it is no longer possible to invoke it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio << 8>.

Fix this by introducing a new board function: board_spi_cs_gpio(). When called, board_spi_cs_gpio() will return the gpio number for the cs value it is given.

Quoted 8 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Eric Nelson <[email protected]>

Cc Eric Benard <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Tim Harvey <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Quoted 4 lines

Just curious, but is this fixing generic SF code or MXC SPI driver ? I'd think the later, but it's not obvious from neither the description nor the subject. I don't quite understand the problem that you're trying to fix either, what happened, did the user command interface change ?

Quoted 4 lines

The U-Boot shell command "sf probe" can accept a chip select value, but if the SPI device on the other end requires an active chip-select over multiple transactions (achieved in the MXC SPI driver using a GPIO), simply typing something like "sf probe 0" will not work.

Why not ?

Quoted 5 lines

This is because whatever the user passes as chip select is propagated to the driver, and the driver expects this value to have GPIO information. So for example, if IMX_GPIO_NR(2, 30) is used to force active chip select 0, then instead of "sf probe 0" the user will have to type "sf probe 15872".

You mean sf probe 0:15872 , right ?

But then, you can use CONFIG_DEFAULT_SPI_CS to specify the default CS, no ?

Quoted 7 lines

I agree that the subject line should be made a bit more specific to mxc_spi though...

Quoted 3 lines

[...]
Best regards,
Marek Vasut

Best regards,
Marek Vasut

Quoted 44 lines · Marek Vasut

Quoted 30 lines · Nikita Kiryanov

Hi Marek,

Quoted 21 lines · Marek Vasut

Quoted 11 lines · Nikita Kiryanov

MXC SPI driver has a feature whereas a GPIO line can be used as a CS signal. This is set up by joining the CS and GPIO values into a single value using (cs | gpio << 8), and passing it off as a CS value. This breaks the sf probe command, because it is no longer possible to invoke it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio << 8>.

Fix this by introducing a new board function: board_spi_cs_gpio(). When called, board_spi_cs_gpio() will return the gpio number for the cs value it is given.

Quoted 8 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Eric Nelson <[email protected]>

Cc Eric Benard <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Tim Harvey <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Quoted 5 lines

Just curious, but is this fixing generic SF code or MXC SPI driver ? I'd think the later, but it's not obvious from neither the description nor the subject. I don't quite understand the problem that you're trying to fix either, what happened, did the user command interface change ?

Quoted 4 lines

The U-Boot shell command "sf probe" can accept a chip select value, but if the SPI device on the other end requires an active chip-select over multiple transactions (achieved in the MXC SPI driver using a GPIO), simply typing something like "sf probe 0" will not work.

Why not ?

Quoted 5 lines

This is because whatever the user passes as chip select is propagated to the driver, and the driver expects this value to have GPIO information. So for example, if IMX_GPIO_NR(2, 30) is used to force active chip select 0, then instead of "sf probe 0" the user will have to type "sf probe 15872".

Quoted 1 line

You mean sf probe 0:15872 , right ?

It's the same thing:
sf probe [[bus:]cs] [hz] [mode]

The point is that cs 0 has to be represented as "15872", instead of "0".

Quoted 2 lines

But then, you can use CONFIG_DEFAULT_SPI_CS to specify the default CS, no ?

This only works if you have just one SPI device that requires GPIO use. Otherwise, chances are the user will have no idea how to access devices on other chip selects without looking at the code.

Quoted 45 lines · Nikita Kiryanov

Quoted 44 lines · Marek Vasut

Quoted 30 lines · Nikita Kiryanov

Hi Marek,

Quoted 21 lines · Marek Vasut

Quoted 11 lines · Nikita Kiryanov

MXC SPI driver has a feature whereas a GPIO line can be used as a CS signal. This is set up by joining the CS and GPIO values into a single value using (cs | gpio << 8), and passing it off as a CS value. This breaks the sf probe command, because it is no longer possible to invoke it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio << 8>.

Fix this by introducing a new board function: board_spi_cs_gpio(). When called, board_spi_cs_gpio() will return the gpio number for the cs value it is given.

Quoted 8 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Eric Nelson <[email protected]>

Cc Eric Benard <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Tim Harvey <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Quoted 5 lines

Just curious, but is this fixing generic SF code or MXC SPI driver ? I'd think the later, but it's not obvious from neither the description nor the subject. I don't quite understand the problem that you're trying to fix either, what happened, did the user command interface change ?

Quoted 4 lines

The U-Boot shell command "sf probe" can accept a chip select value, but if the SPI device on the other end requires an active chip-select over multiple transactions (achieved in the MXC SPI driver using a GPIO), simply typing something like "sf probe 0" will not work.

Why not ?

Quoted 5 lines

This is because whatever the user passes as chip select is propagated to the driver, and the driver expects this value to have GPIO information. So for example, if IMX_GPIO_NR(2, 30) is used to force active chip select 0, then instead of "sf probe 0" the user will have to type "sf probe 15872".

Quoted 6 lines

Quoted 1 line

You mean sf probe 0:15872 , right ?

It's the same thing:
sf probe [[bus:]cs] [hz] [mode]

The point is that cs 0 has to be represented as "15872", instead of "0".

That's expected, yep.

Quoted 7 lines

Quoted 3 lines

But then, you can use CONFIG_DEFAULT_SPI_CS to specify the default CS, no ?

This only works if you have just one SPI device that requires GPIO use. Otherwise, chances are the user will have no idea how to access devices on other chip selects without looking at the code.

Oh, so you're confused by those crazy chipselect numbers ?

Best regards,
Marek Vasut

Quoted 48 lines · Marek Vasut

Quoted 47 lines · Nikita Kiryanov

Quoted 45 lines · Marek Vasut

Quoted 30 lines · Nikita Kiryanov

Hi Marek,

Quoted 21 lines · Marek Vasut

Quoted 11 lines · Nikita Kiryanov

MXC SPI driver has a feature whereas a GPIO line can be used as a CS signal. This is set up by joining the CS and GPIO values into a single value using (cs | gpio << 8), and passing it off as a CS value. This breaks the sf probe command, because it is no longer possible to invoke it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio << 8>.

Fix this by introducing a new board function: board_spi_cs_gpio(). When called, board_spi_cs_gpio() will return the gpio number for the cs value it is given.

Quoted 8 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Eric Nelson <[email protected]>

Cc Eric Benard <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Tim Harvey <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Quoted 5 lines

Just curious, but is this fixing generic SF code or MXC SPI driver ? I'd think the later, but it's not obvious from neither the description nor the subject. I don't quite understand the problem that you're trying to fix either, what happened, did the user command interface change ?

Quoted 4 lines

The U-Boot shell command "sf probe" can accept a chip select value, but if the SPI device on the other end requires an active chip-select over multiple transactions (achieved in the MXC SPI driver using a GPIO), simply typing something like "sf probe 0" will not work.

Why not ?

Quoted 5 lines

This is because whatever the user passes as chip select is propagated to the driver, and the driver expects this value to have GPIO information. So for example, if IMX_GPIO_NR(2, 30) is used to force active chip select 0, then instead of "sf probe 0" the user will have to type "sf probe 15872".

You mean sf probe 0:15872 , right ?

Quoted 6 lines

Quoted 4 lines

It's the same thing:
sf probe [[bus:]cs] [hz] [mode]

The point is that cs 0 has to be represented as "15872", instead of "0".

That's expected, yep.

By whom?

Quoted 43 lines · Marek Vasut

Quoted 29 lines · Nikita Kiryanov

Hi Marek,

Quoted 21 lines · Marek Vasut

Quoted 11 lines · Nikita Kiryanov

MXC SPI driver has a feature whereas a GPIO line can be used as a CS signal. This is set up by joining the CS and GPIO values into a single value using (cs | gpio << 8), and passing it off as a CS value. This breaks the sf probe command, because it is no longer possible to invoke it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio << 8>.

Fix this by introducing a new board function: board_spi_cs_gpio(). When called, board_spi_cs_gpio() will return the gpio number for the cs value it is given.

Quoted 8 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Eric Nelson <[email protected]>

Cc Eric Benard <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Tim Harvey <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Quoted 4 lines

Just curious, but is this fixing generic SF code or MXC SPI driver ? I'd think the later, but it's not obvious from neither the description nor the subject. I don't quite understand the problem that you're trying to fix either, what happened, did the user command interface change ?

Quoted 4 lines

The U-Boot shell command "sf probe" can accept a chip select value, but if the SPI device on the other end requires an active chip-select over multiple transactions (achieved in the MXC SPI driver using a GPIO), simply typing something like "sf probe 0" will not work.

Why not ?

Quoted 5 lines

This is because whatever the user passes as chip select is propagated to the driver, and the driver expects this value to have GPIO information. So for example, if IMX_GPIO_NR(2, 30) is used to force active chip select 0, then instead of "sf probe 0" the user will have to type "sf probe 15872".

Quoted 1 line

You mean sf probe 0:15872 , right ?

It's the same thing:
sf probe [[bus:]cs] [hz] [mode]

The point is that cs 0 has to be represented as "15872", instead of "0".

Eeep. That seems very likely to be gotten incorrect by users.

Can we do something like:
mxc_spi.c:
__weak int board_map_spi_cs_value(int desired_cs) { return -EINVAL; }

fooboard.c:
board_map_spi_cs_value(int desired_cs) {
if (desired_cs == 0)

    return IMX_GPIO_NR(2, 30);
  else
    return -EINVAL;
}

I think it'll be very bad if the user has to type 'sf probe 0:15872' or 'sf probe 15872' since that's a programming detail rather than saying bank 2, gpio 30 (which I assume is what IMX_GPIO_NR means).

Quoted 46 lines · Nikita Kiryanov

Quoted 43 lines · Marek Vasut

Quoted 29 lines · Nikita Kiryanov

Hi Marek,

Quoted 21 lines · Marek Vasut

Quoted 11 lines · Nikita Kiryanov

MXC SPI driver has a feature whereas a GPIO line can be used as a CS signal. This is set up by joining the CS and GPIO values into a single value using (cs | gpio << 8), and passing it off as a CS value. This breaks the sf probe command, because it is no longer possible to invoke it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio << 8>.

Fix this by introducing a new board function: board_spi_cs_gpio(). When called, board_spi_cs_gpio() will return the gpio number for the cs value it is given.

Quoted 8 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Eric Nelson <[email protected]>

Cc Eric Benard <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Tim Harvey <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Quoted 4 lines

Just curious, but is this fixing generic SF code or MXC SPI driver ? I'd think the later, but it's not obvious from neither the description nor the subject. I don't quite understand the problem that you're trying to fix either, what happened, did the user command interface change ?

Quoted 4 lines

The U-Boot shell command "sf probe" can accept a chip select value, but if the SPI device on the other end requires an active chip-select over multiple transactions (achieved in the MXC SPI driver using a GPIO), simply typing something like "sf probe 0" will not work.

Why not ?

Quoted 5 lines

This is because whatever the user passes as chip select is propagated to the driver, and the driver expects this value to have GPIO information. So for example, if IMX_GPIO_NR(2, 30) is used to force active chip select 0, then instead of "sf probe 0" the user will have to type "sf probe 15872".

Quoted 6 lines

Quoted 1 line

You mean sf probe 0:15872 , right ?

It's the same thing:
sf probe [[bus:]cs] [hz] [mode]

The point is that cs 0 has to be represented as "15872", instead of "0".

Eeep. That seems very likely to be gotten incorrect by users.

Can we do something like:
mxc_spi.c:
__weak int board_map_spi_cs_value(int desired_cs) { return -EINVAL; }

fooboard.c:

board_map_spi_cs_value(int desired_cs) {
   if (desired_cs == 0)
     return IMX_GPIO_NR(2, 30);
   else
     return -EINVAL;
}

I think it'll be very bad if the user has to type 'sf probe 0:15872' or 'sf probe 15872' since that's a programming detail rather than saying bank 2, gpio 30 (which I assume is what IMX_GPIO_NR means).

Quoted 60 lines · Tom Rini

Quoted 46 lines · Nikita Kiryanov

Quoted 43 lines · Marek Vasut

Quoted 29 lines · Nikita Kiryanov

Hi Marek,

Quoted 21 lines · Marek Vasut

Quoted 11 lines · Nikita Kiryanov

MXC SPI driver has a feature whereas a GPIO line can be used as a CS signal. This is set up by joining the CS and GPIO values into a single value using (cs | gpio << 8), and passing it off as a CS value. This breaks the sf probe command, because it is no longer possible to invoke it as sf probe <cs>. Instead, the user must use sf probe <cs | gpio << 8>.

Fix this by introducing a new board function: board_spi_cs_gpio(). When called, board_spi_cs_gpio() will return the gpio number for the cs value it is given.

Quoted 8 lines

Cc Jagannadha Sutradharudu Teki <[email protected]>

Cc Eric Nelson <[email protected]>

Cc Eric Benard <[email protected]>

Cc Fabio Estevam <[email protected]>

Cc Tim Harvey <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Quoted 4 lines

Just curious, but is this fixing generic SF code or MXC SPI driver ? I'd think the later, but it's not obvious from neither the description nor the subject. I don't quite understand the problem that you're trying to fix either, what happened, did the user command interface change ?

Quoted 4 lines

The U-Boot shell command "sf probe" can accept a chip select value, but if the SPI device on the other end requires an active chip-select over multiple transactions (achieved in the MXC SPI driver using a GPIO), simply typing something like "sf probe 0" will not work.

Why not ?

Quoted 5 lines

This is because whatever the user passes as chip select is propagated to the driver, and the driver expects this value to have GPIO information. So for example, if IMX_GPIO_NR(2, 30) is used to force active chip select 0, then instead of "sf probe 0" the user will have to type "sf probe 15872".

Quoted 6 lines

Quoted 1 line

You mean sf probe 0:15872 , right ?

It's the same thing:
sf probe [[bus:]cs] [hz] [mode]

The point is that cs 0 has to be represented as "15872", instead of "0".

Eeep. That seems very likely to be gotten incorrect by users.

Can we do something like:
mxc_spi.c:
__weak int board_map_spi_cs_value(int desired_cs) { return -EINVAL; }

Quoted 7 lines

fooboard.c:
board_map_spi_cs_value(int desired_cs) {
if (desired_cs == 0)

    return IMX_GPIO_NR(2, 30);
  else
    return -EINVAL;
}

That's pretty much what the patch does.

Quoted 1 line

Quoted 4 lines

I think it'll be very bad if the user has to type 'sf probe 0:15872' or 'sf probe 15872' since that's a programming detail rather than saying bank 2, gpio 30 (which I assume is what IMX_GPIO_NR means).

Agreed.

Add support for M25PE16 and M25PX16

Cc Jagannadha Sutradharudu Teki <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

Create CONFIG_SYS_I2C_EEPROM_BUS #define to tell the EEPROM module what I2C bus the EEPROM is located at. Make cl_eeprom_read() switch to that bus when reading EEPROM.

Cc Igor Grinberg <[email protected]>

Cc Dmitry Lifshitz <[email protected]>

Cc Tom Rini <[email protected]>

Acked-by Igor Grinberg <[email protected]>

Acked-by Dmitry Lifshitz <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
 board/compulab/common/eeprom.c | 10 +++++++++-
 include/configs/cm_t335.h      |  1 +
 include/configs/cm_t35.h       |  1 +
 include/configs/cm_t54.h       |  1 +
 4 files changed, 12 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/board/compulab/common/eeprom.c index 20fe3e1..b5c1c2a 100644

board/compulab/common/eeprom.c +0 -0

@@ -31,8 +31,16 @@ static int cl_eeprom_layout; /* Implicitly

LAYOUT_INVALID */

static int cl_eeprom_read(uint offset, uchar *buf, int len) {

Quoted 24 lines · Nikita Kiryanov

Create CONFIG_SYS_I2C_EEPROM_BUS #define to tell the EEPROM module what I2C bus the EEPROM is located at. Make cl_eeprom_read() switch to that bus when reading EEPROM.

Cc Igor Grinberg <[email protected]>

Cc Dmitry Lifshitz <[email protected]>

Cc Tom Rini <[email protected]>

Acked-by Igor Grinberg <[email protected]>

Acked-by Dmitry Lifshitz <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
  board/compulab/common/eeprom.c | 10 +++++++++-
  include/configs/cm_t335.h      |  1 +
  include/configs/cm_t35.h       |  1 +
  include/configs/cm_t54.h       |  1 +
  4 files changed, 12 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/board/compulab/common/eeprom.c index 20fe3e1..b5c1c2a 100644

board/compulab/common/eeprom.c +0 -0

@@ -31,8 +31,16 @@ static int cl_eeprom_layout; /* Implicitly

LAYOUT_INVALID */

board/compulab/common/eeprom.c static int cl_eeprom_layout; /* Implicitly LAYOUT_INVALID */ · line 37

Quoted 7 lines

static int cl_eeprom_read(uint offset, uchar *buf, int len) {

board/compulab/common/eeprom.c +4 -1

-	return i2c_read(CONFIG_SYS_I2C_EEPROM_ADDR, offset,
+	int res;
+	unsigned int current_i2c_bus = i2c_get_bus_num();
+
+	i2c_set_bus_num(CONFIG_SYS_I2C_EEPROM_BUS);

Please handle the return value of this function too.

[...]
BR,MV

Define the new common function sata_port_status() which can be used to query the sata driver for the state of ports, and implement it for dwc_ahsata.

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
 drivers/block/dwc_ahsata.c | 17 +++++++++++++++++
 include/sata.h             |  1 +
 2 files changed, 18 insertions(+)

drivers/block/dwc_ahsata.c +0 -0

index 15d65d7..e122ed9 100644

Quoted 20 lines · Nikita Kiryanov

Define the new common function sata_port_status() which can be used to query the sata driver for the state of ports, and implement it for dwc_ahsata.

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
  drivers/block/dwc_ahsata.c | 17 +++++++++++++++++
  include/sata.h             |  1 +
  2 files changed, 18 insertions(+)

drivers/block/dwc_ahsata.c +0 -0

index 15d65d7..e122ed9 100644
@@ -864,6 +864,23 @@ u32 ata_low_level_rw_lba28(int dev, u32 blknr,
 lbaint_t blkcnt, return blkcnt;
  }
 

drivers/block/dwc_ahsata.c u32 ata_low_level_rw_lba28(int dev, u32 blknr, lbaint_t blkcnt, · line 873

Quoted 7 lines

drivers/block/dwc_ahsata.c +7 -0

+int sata_port_status(int dev, int port)
+{
+	struct sata_port_regs *port_mmio;
+	struct ahci_probe_ent *probe_ent = NULL;
+
+	if (dev < 0 || dev > (CONFIG_SYS_SATA_MAX_DEVICE - 1))
+		return -1;

Please lets use values from errno.h here , let's set a good example and just do that please.

[...]
Best regards,
Marek Vasut

Quoted 23 lines · Marek Vasut

Quoted 22 lines · Nikita Kiryanov

Define the new common function sata_port_status() which can be used to query the sata driver for the state of ports, and implement it for dwc_ahsata.

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---

  drivers/block/dwc_ahsata.c | 17 +++++++++++++++++
  include/sata.h             |  1 +
  2 files changed, 18 insertions(+)

drivers/block/dwc_ahsata.c +0 -0

index 15d65d7..e122ed9 100644
@@ -864,6 +864,23 @@ u32 ata_low_level_rw_lba28(int dev, u32 blknr,
 lbaint_t blkcnt, return blkcnt;
 
  }
 

Quoted 10 lines

drivers/block/dwc_ahsata.c u32 ata_low_level_rw_lba28(int dev, u32 blknr, lbaint_t blkcnt, · line 873

Quoted 7 lines

drivers/block/dwc_ahsata.c +7 -0

+int sata_port_status(int dev, int port)
+{
+	struct sata_port_regs *port_mmio;
+	struct ahci_probe_ent *probe_ent = NULL;
+
+	if (dev < 0 || dev > (CONFIG_SYS_SATA_MAX_DEVICE - 1))
+		return -1;

Please lets use values from errno.h here , let's set a good example and just do that please.

Will do...

Add macro which defines i2c_pads_info structs for multiple SoC types, and a macro which selects the appropriate struct based on CPU type, thus eliminating the need to manage multiple i2c pad configurations manually when supporting multiple SoC types.

Cc Stefano Babic <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
 arch/arm/include/asm/imx-common/mxc_i2c.h | 33 +++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

arch/arm/include/asm/imx-common/mxc_i2c.h +33 -1

index 47a9edc..182c2f3 100644
@@ -19,6 +19,39 @@ struct i2c_pads_info {
        struct i2c_pin_ctrl sda;
 };
 
+#if defined(CONFIG_MX6QDL)
+#define I2C_PADS(name, scl_i2c, scl_gpio, scl_gp, sda_i2c, sda_gpio, sda_gp) \
+       struct i2c_pads_info mx6q_##name = {            \
+               .scl = {                                \
+                       .i2c_mode = MX6Q_##scl_i2c,     \
+                       .gpio_mode = MX6Q_##scl_gpio,   \
+                       .gp = scl_gp,                   \
+               },                                      \
+               .sda = {                                \
+                       .i2c_mode = MX6Q_##sda_i2c,     \
+                       .gpio_mode = MX6Q_##sda_gpio,   \
+                       .gp = sda_gp,                   \
+               }                                       \
+       };                                              \
+       struct i2c_pads_info mx6s_##name = {            \
+               .scl = {                                \
+                       .i2c_mode = MX6DL_##scl_i2c,    \
+                       .gpio_mode = MX6DL_##scl_gpio,  \
+                       .gp = scl_gp,                   \
+               },                                      \
+               .sda = {                                \
+                       .i2c_mode = MX6DL_##sda_i2c,    \
+                       .gpio_mode = MX6DL_##sda_gpio,  \
+                       .gp = sda_gp,                   \
+               }                                       \
+       };
+
+
+#define I2C_PADS_INFO(name)    \
+       (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D)) ? \
+                                       &mx6q_##name : &mx6s_##name
+#endif
+
 void setup_i2c(unsigned i2c_index, int speed, int slave_addr,
                struct i2c_pads_info *p);
 void bus_i2c_init(void *base, int speed, int slave_addr,
--

1.9.1

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

No functional changes.

Cc Stefano Babic <[email protected]>

Cc Tim Harvey <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
 arch/arm/cpu/armv7/mx6/ddr.c | 272 +++++++++++++++++++++----------------------
 1 file changed, 134 insertions(+), 138 deletions(-)

arch/arm/cpu/armv7/mx6/ddr.c +134 -139

index 0434211..af91314 100644
@@ -184,18 +184,18 @@ void mx6sdl_dram_iocfg(unsigned width,
  */
 #define MR(val, ba, cmd, cs1) \
        ((val << 16) | (1 << 15) | (cmd << 4) | (cs1 << 3) | ba)
-void mx6_dram_cfg(const struct mx6_ddr_sysinfo *i,
-                 const struct mx6_mmdc_calibration *c,
-                 const struct mx6_ddr3_cfg *m)
+void mx6_dram_cfg(const struct mx6_ddr_sysinfo *sysinfo,
+                 const struct mx6_mmdc_calibration *calib,
+                 const struct mx6_ddr3_cfg *ddr3_cfg)
 {
        volatile struct mmdc_p_regs *mmdc0;
        volatile struct mmdc_p_regs *mmdc1;
-       u32 reg;
+       u32 val;
        u8 tcke, tcksrx, tcksre, txpdll, taofpd, taonpd, trrd;
        u8 todtlon, taxpd, tanpd, tcwl, txp, tfaw, tcl;
        u8 todt_idle_off = 0x4; /* from DDR3 Script Aid spreadsheet */
        u16 trcd, trc, tras, twr, tmrd, trtp, trp, twtr, trfc, txs, txpr;
-       u16 CS0_END;
+       u16 cs0_end;
        u16 tdllk = 0x1ff; /* DLL locking time: 512 cycles (JEDEC DDR3) */
        int clkper; /* clock period in picoseconds */
        int clock; /* clock freq in mHz */

@@ -214,13 +214,12 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *i,
                clock = 400;
                tcwl = 3;
        }
-       clkper = (1000*1000)/clock; /* ps */
+       clkper = (1000 * 1000) / clock; /* pico seconds */
        todtlon = tcwl;
        taxpd = tcwl;
        tanpd = tcwl;
-       tcwl = tcwl;
 
-       switch (m->density) {
+       switch (ddr3_cfg->density) {
        case 1: /* 1Gb per chip */
                trfc = DIV_ROUND_UP(110000, clkper) - 1;
                txs = DIV_ROUND_UP(120000, clkper) - 1;

@@ -239,80 +238,84 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *i,
                break;
        default:
                /* invalid density */
-               printf("invalid chip density\n");
+               puts("invalid chip density\n");
                hang();
                break;
        }
        txpr = txs;
 
-       switch (m->mem_speed) {
+       switch (ddr3_cfg->mem_speed) {
        case 800:
-               txp = DIV_ROUND_UP(MAX(3*clkper, 7500), clkper) - 1;
-               tcke = DIV_ROUND_UP(MAX(3*clkper, 7500), clkper) - 1;
-               if (m->pagesz == 1) {
+               txp = DIV_ROUND_UP(MAX(3 * clkper, 7500), clkper) - 1;
+               tcke = DIV_ROUND_UP(MAX(3 * clkper, 7500), clkper) - 1;
+               if (ddr3_cfg->pagesz == 1) {
                        tfaw = DIV_ROUND_UP(40000, clkper) - 1;
-                       trrd = DIV_ROUND_UP(MAX(4*clkper, 10000), clkper) - 1;
+                       trrd = DIV_ROUND_UP(MAX(4 * clkper, 10000), clkper) - 1;
                } else {
                        tfaw = DIV_ROUND_UP(50000, clkper) - 1;
-                       trrd = DIV_ROUND_UP(MAX(4*clkper, 10000), clkper) - 1;
+                       trrd = DIV_ROUND_UP(MAX(4 * clkper, 10000), clkper) - 1;
                }
                break;
        case 1066:
-               txp = DIV_ROUND_UP(MAX(3*clkper, 7500), clkper) - 1;
-               tcke = DIV_ROUND_UP(MAX(3*clkper, 5625), clkper) - 1;
-               if (m->pagesz == 1) {
+               txp = DIV_ROUND_UP(MAX(3 * clkper, 7500), clkper) - 1;
+               tcke = DIV_ROUND_UP(MAX(3 * clkper, 5625), clkper) - 1;
+               if (ddr3_cfg->pagesz == 1) {
                        tfaw = DIV_ROUND_UP(37500, clkper) - 1;
-                       trrd = DIV_ROUND_UP(MAX(4*clkper, 7500), clkper) - 1;
+                       trrd = DIV_ROUND_UP(MAX(4 * clkper, 7500), clkper) - 1;
                } else {
                        tfaw = DIV_ROUND_UP(50000, clkper) - 1;
-                       trrd = DIV_ROUND_UP(MAX(4*clkper, 10000), clkper) - 1;
+                       trrd = DIV_ROUND_UP(MAX(4 * clkper, 10000), clkper) - 1;
                }
                break;
        case 1333:
-               txp = DIV_ROUND_UP(MAX(3*clkper, 6000), clkper) - 1;
-               tcke = DIV_ROUND_UP(MAX(3*clkper, 5625), clkper) - 1;
-               if (m->pagesz == 1) {
+               txp = DIV_ROUND_UP(MAX(3 * clkper, 6000), clkper) - 1;
+               tcke = DIV_ROUND_UP(MAX(3 * clkper, 5625), clkper) - 1;
+               if (ddr3_cfg->pagesz == 1) {
                        tfaw = DIV_ROUND_UP(30000, clkper) - 1;
-                       trrd = DIV_ROUND_UP(MAX(4*clkper, 6000), clkper) - 1;
+                       trrd = DIV_ROUND_UP(MAX(4 * clkper, 6000), clkper) - 1;
                } else {
                        tfaw = DIV_ROUND_UP(45000, clkper) - 1;
-                       trrd = DIV_ROUND_UP(MAX(4*clkper, 7500), clkper) - 1;
+                       trrd = DIV_ROUND_UP(MAX(4 * clkper, 7500), clkper) - 1;
                }
                break;
        case 1600:
-               txp = DIV_ROUND_UP(MAX(3*clkper, 6000), clkper) - 1;
-               tcke = DIV_ROUND_UP(MAX(3*clkper, 5000), clkper) - 1;
-               if (m->pagesz == 1) {
+               txp = DIV_ROUND_UP(MAX(3 * clkper, 6000), clkper) - 1;
+               tcke = DIV_ROUND_UP(MAX(3 * clkper, 5000), clkper) - 1;
+               if (ddr3_cfg->pagesz == 1) {
                        tfaw = DIV_ROUND_UP(30000, clkper) - 1;
-                       trrd = DIV_ROUND_UP(MAX(4*clkper, 6000), clkper) - 1;
+                       trrd = DIV_ROUND_UP(MAX(4 * clkper, 6000), clkper) - 1;
                } else {
                        tfaw = DIV_ROUND_UP(40000, clkper) - 1;
-                       trrd = DIV_ROUND_UP(MAX(4*clkper, 7500), clkper) - 1;
+                       trrd = DIV_ROUND_UP(MAX(4 * clkper, 7500), clkper) - 1;
                }
                break;
        default:
-               printf("invalid memory speed\n");
+               puts("invalid memory speed\n");
                hang();
                break;
        }
-       txpdll = DIV_ROUND_UP(MAX(10*clkper, 24000), clkper) - 1;
-       tcl = DIV_ROUND_UP(m->trcd, clkper/10) - 3;
-       tcksre = DIV_ROUND_UP(MAX(5*clkper, 10000), clkper);
-       tcksrx = tcksre;
+       txpdll = DIV_ROUND_UP(MAX(10 * clkper, 24000), clkper) - 1;
+       tcksre = DIV_ROUND_UP(MAX(5 * clkper, 10000), clkper);
        taonpd = DIV_ROUND_UP(2000, clkper) - 1;
+       tcksrx = tcksre;
        taofpd = taonpd;
-       trp = DIV_ROUND_UP(m->trcd, clkper/10) - 1;
+       twr  = DIV_ROUND_UP(15000, clkper) - 1;
+       tmrd = DIV_ROUND_UP(MAX(12 * clkper, 15000), clkper) - 1;
+       trc  = DIV_ROUND_UP(ddr3_cfg->trcmin, clkper / 10) - 1;
+       tras = DIV_ROUND_UP(ddr3_cfg->trasmin, clkper / 10) - 1;
+       tcl  = DIV_ROUND_UP(ddr3_cfg->trcd, clkper / 10) - 3;
+       trp  = DIV_ROUND_UP(ddr3_cfg->trcd, clkper / 10) - 1;
+       twtr = ROUND(MAX(4 * clkper, 7500) / clkper, 1) - 1;
        trcd = trp;
-       trc = DIV_ROUND_UP(m->trcmin, clkper/10) - 1;
-       tras = DIV_ROUND_UP(m->trasmin, clkper/10) - 1;
-       twr = DIV_ROUND_UP(15000, clkper) - 1;
-       tmrd = DIV_ROUND_UP(MAX(12*clkper, 15000), clkper) - 1;
-       twtr = ROUND(MAX(4*clkper, 7500)/clkper, 1) - 1;
        trtp = twtr;
-       CS0_END = ((4*i->cs_density) <= 120) ? (4*i->cs_density)+7 : 127;
-       debug("density:%d Gb (%d Gb per chip)\n", i->cs_density, m->density);
+       cs0_end = (4 * sysinfo->cs_density <= 120) ?
+                  4 * sysinfo->cs_density + 7 :
+                  127;
+
+       debug("density:%d Gb (%d Gb per chip)\n",
+             sysinfo->cs_density, ddr3_cfg->density);
        debug("clock: %dMHz (%d ps)\n", clock, clkper);
-       debug("memspd:%d\n", m->mem_speed);
+       debug("memspd:%d\n", ddr3_cfg->mem_speed);
        debug("tcke=%d\n", tcke);
        debug("tcksrx=%d\n", tcksrx);
        debug("tcksre=%d\n", tcksre);

@@ -339,11 +342,11 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *i,
        debug("twtr=%d\n", twtr);
        debug("trrd=%d\n", trrd);
        debug("txpr=%d\n", txpr);
-       debug("CS0_END=%d\n", CS0_END);
-       debug("ncs=%d\n", i->ncs);
-       debug("Rtt_wr=%d\n", i->rtt_wr);
-       debug("Rtt_nom=%d\n", i->rtt_nom);
-       debug("SRT=%d\n", m->SRT);
+       debug("cs0_end=%d\n", cs0_end);
+       debug("ncs=%d\n", sysinfo->ncs);
+       debug("Rtt_wr=%d\n", sysinfo->rtt_wr);
+       debug("Rtt_nom=%d\n", sysinfo->rtt_nom);
+       debug("SRT=%d\n", ddr3_cfg->SRT);
        debug("tcl=%d\n", tcl);
        debug("twr=%d\n", twr);
 

@@ -353,137 +356,130 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *i,
         *  see:
         *   appnote, ddr3 spreadsheet
         */
-       mmdc0->mpwldectrl0 = c->p0_mpwldectrl0;
-       mmdc0->mpwldectrl1 = c->p0_mpwldectrl1;
-       mmdc0->mpdgctrl0 = c->p0_mpdgctrl0;
-       mmdc0->mpdgctrl1 = c->p0_mpdgctrl1;
-       mmdc0->mprddlctl = c->p0_mprddlctl;
-       mmdc0->mpwrdlctl = c->p0_mpwrdlctl;
-       if (i->dsize > 1) {
-               mmdc1->mpwldectrl0 = c->p1_mpwldectrl0;
-               mmdc1->mpwldectrl1 = c->p1_mpwldectrl1;
-               mmdc1->mpdgctrl0 = c->p1_mpdgctrl0;
-               mmdc1->mpdgctrl1 = c->p1_mpdgctrl1;
-               mmdc1->mprddlctl = c->p1_mprddlctl;
-               mmdc1->mpwrdlctl = c->p1_mpwrdlctl;
+       mmdc0->mpwldectrl0 = calib->p0_mpwldectrl0;
+       mmdc0->mpwldectrl1 = calib->p0_mpwldectrl1;
+       mmdc0->mpdgctrl0 = calib->p0_mpdgctrl0;
+       mmdc0->mpdgctrl1 = calib->p0_mpdgctrl1;
+       mmdc0->mprddlctl = calib->p0_mprddlctl;
+       mmdc0->mpwrdlctl = calib->p0_mpwrdlctl;
+       if (sysinfo->dsize > 1) {
+               mmdc1->mpwldectrl0 = calib->p1_mpwldectrl0;
+               mmdc1->mpwldectrl1 = calib->p1_mpwldectrl1;
+               mmdc1->mpdgctrl0 = calib->p1_mpdgctrl0;
+               mmdc1->mpdgctrl1 = calib->p1_mpdgctrl1;
+               mmdc1->mprddlctl = calib->p1_mprddlctl;
+               mmdc1->mpwrdlctl = calib->p1_mpwrdlctl;
        }
 
        /* Read data DQ Byte0-3 delay */
-       mmdc0->mprddqby0dl = (u32)0x33333333;
-       mmdc0->mprddqby1dl = (u32)0x33333333;
-       if (i->dsize > 0) {
-               mmdc0->mprddqby2dl = (u32)0x33333333;
-               mmdc0->mprddqby3dl = (u32)0x33333333;
+       mmdc0->mprddqby0dl = 0x33333333;
+       mmdc0->mprddqby1dl = 0x33333333;
+       if (sysinfo->dsize > 0) {
+               mmdc0->mprddqby2dl = 0x33333333;
+               mmdc0->mprddqby3dl = 0x33333333;
        }
-       if (i->dsize > 1) {
-               mmdc1->mprddqby0dl = (u32)0x33333333;
-               mmdc1->mprddqby1dl = (u32)0x33333333;
-               mmdc1->mprddqby2dl = (u32)0x33333333;
-               mmdc1->mprddqby3dl = (u32)0x33333333;
+
+       if (sysinfo->dsize > 1) {
+               mmdc1->mprddqby0dl = 0x33333333;
+               mmdc1->mprddqby1dl = 0x33333333;
+               mmdc1->mprddqby2dl = 0x33333333;
+               mmdc1->mprddqby3dl = 0x33333333;
        }
 
        /* MMDC Termination: rtt_nom:2 RZQ/2(120ohm), rtt_nom:1 RZQ/4(60ohm) */
-       reg = (i->rtt_nom == 2) ? 0x00011117 : 0x00022227;
-       mmdc0->mpodtctrl = reg;
-       if (i->dsize > 1)
-               mmdc1->mpodtctrl = reg;
+       val = (sysinfo->rtt_nom == 2) ? 0x00011117 : 0x00022227;
+       mmdc0->mpodtctrl = val;
+       if (sysinfo->dsize > 1)
+               mmdc1->mpodtctrl = val;
 
        /* complete calibration */
-       reg = (1 << 11); /* Force measurement on delay-lines */
-       mmdc0->mpmur0 = reg;
-       if (i->dsize > 1)
-               mmdc1->mpmur0 = reg;
+       val = (1 << 11); /* Force measurement on delay-lines */
+       mmdc0->mpmur0 = val;
+       if (sysinfo->dsize > 1)
+               mmdc1->mpmur0 = val;
 
        /* Step 1: configuration request */
        mmdc0->mdscr = (u32)(1 << 15); /* config request */
 
        /* Step 2: Timing configuration */
-       reg = (trfc << 24) | (txs << 16) | (txp << 13) | (txpdll << 9) |
-             (tfaw << 4) | tcl;
-       mmdc0->mdcfg0 = reg;
-       reg = (trcd << 29) | (trp << 26) | (trc << 21) | (tras << 16) |
-             (1 << 15) |               /* trpa */
-             (twr << 9) | (tmrd << 5) | tcwl;
-       mmdc0->mdcfg1 = reg;
-       reg = (tdllk << 16) | (trtp << 6) | (twtr << 3) | trrd;
-       mmdc0->mdcfg2 = reg;
-       reg = (taofpd << 27) | (taonpd << 24) | (tanpd << 20) | (taxpd << 16) |
-             (todtlon << 12) | (todt_idle_off << 4);
-       mmdc0->mdotc = reg;
-       mmdc0->mdasp = CS0_END; /* CS addressing */
+       mmdc0->mdcfg0 = (trfc << 24) | (txs << 16) | (txp << 13) |
+                       (txpdll << 9) | (tfaw << 4) | tcl;
+       mmdc0->mdcfg1 = (trcd << 29) | (trp << 26) | (trc << 21) |
+                       (tras << 16) | (1 << 15) /* trpa */ |
+                       (twr << 9) | (tmrd << 5) | tcwl;
+       mmdc0->mdcfg2 = (tdllk << 16) | (trtp << 6) | (twtr << 3) | trrd;
+       mmdc0->mdotc = (taofpd << 27) | (taonpd << 24) | (tanpd << 20) |
+                      (taxpd << 16) | (todtlon << 12) | (todt_idle_off << 4);
+       mmdc0->mdasp = cs0_end; /* CS addressing */
 
        /* Step 3: Configure DDR type */
-       reg = (i->cs1_mirror << 19) | (i->walat << 16) | (i->bi_on << 12) |
-             (i->mif3_mode << 9) | (i->ralat << 6);
-       mmdc0->mdmisc = reg;
+       mmdc0->mdmisc = (sysinfo->cs1_mirror << 19) | (sysinfo->walat << 16) |
+                       (sysinfo->bi_on << 12) | (sysinfo->mif3_mode << 9) |
+                       (sysinfo->ralat << 6);
 
        /* Step 4: Configure delay while leaving reset */
-       reg = (txpr << 16) | (i->sde_to_rst << 8) | (i->rst_to_cke << 0);
-       mmdc0->mdor = reg;
+       mmdc0->mdor = (txpr << 16) | (sysinfo->sde_to_rst << 8) |
+                     (sysinfo->rst_to_cke << 0);
 
        /* Step 5: Configure DDR physical parameters (density and burst len) */
-       reg = (m->rowaddr - 11) << 24 |         /* ROW */
-             (m->coladdr - 9) << 20 |          /* COL */
-             (1 << 19) |                       /* Burst Length = 8 for DDR3 */
-             (i->dsize << 16);                 /* DDR data bus size */
-       mmdc0->mdctl = reg;
+       mmdc0->mdctl = (ddr3_cfg->rowaddr - 11) << 24 |         /* ROW */
+                      (ddr3_cfg->coladdr - 9) << 20 |          /* COL */
+                      (1 << 19) |              /* Burst Length = 8 for DDR3 */
+                      (sysinfo->dsize << 16);          /* DDR data bus size */
 
        /* Step 6: Perform ZQ calibration */
-       reg = (u32)0xa1390001; /* one-time HW ZQ calib */
-       mmdc0->mpzqhwctrl = reg;
-       if (i->dsize > 1)
-               mmdc1->mpzqhwctrl = reg;
+       val = 0xa1390001; /* one-time HW ZQ calib */
+       mmdc0->mpzqhwctrl = val;
+       if (sysinfo->dsize > 1)
+               mmdc1->mpzqhwctrl = val;
 
        /* Step 7: Enable MMDC with desired chip select */
-       reg = mmdc0->mdctl |
-             (1 << 31) |                       /* SDE_0 for CS0 */
-             ((i->ncs == 2) ? 1 : 0) << 30;    /* SDE_1 for CS1 */
-       mmdc0->mdctl = reg;
+       mmdc0->mdctl |= (1 << 31) |                          /* SDE_0 for CS0 */
+                       ((sysinfo->ncs == 2) ? 1 : 0) << 30; /* SDE_1 for CS1 */
 
        /* Step 8: Write Mode Registers to Init DDR3 devices */
-       for (cs = 0; cs < i->ncs; cs++) {
+       for (cs = 0; cs < sysinfo->ncs; cs++) {
                /* MR2 */
-               reg = (i->rtt_wr & 3) << 9 | (m->SRT & 1) << 7 |
+               val = (sysinfo->rtt_wr & 3) << 9 | (ddr3_cfg->SRT & 1) << 7 |
                      ((tcwl - 3) & 3) << 3;
-               mmdc0->mdscr = (u32)MR(reg, 2, 3, cs);
+               mmdc0->mdscr = MR(val, 2, 3, cs);
                /* MR3 */
-               mmdc0->mdscr = (u32)MR(0, 3, 3, cs);
+               mmdc0->mdscr = MR(0, 3, 3, cs);
                /* MR1 */
-               reg = ((i->rtt_nom & 1) ? 1 : 0) << 2 |
-                     ((i->rtt_nom & 2) ? 1 : 0) << 6;
-               mmdc0->mdscr = (u32)MR(reg, 1, 3, cs);
-               reg = ((tcl - 1) << 4) |        /* CAS */
+               val = ((sysinfo->rtt_nom & 1) ? 1 : 0) << 2 |
+                     ((sysinfo->rtt_nom & 2) ? 1 : 0) << 6;
+               mmdc0->mdscr = MR(val, 1, 3, cs);
+               /* MR0 */
+               val = ((tcl - 1) << 4) |        /* CAS */
                      (1 << 8)   |              /* DLL Reset */
                      ((twr - 3) << 9);         /* Write Recovery */
-               /* MR0 */
-               mmdc0->mdscr = (u32)MR(reg, 0, 3, cs);
+               mmdc0->mdscr = MR(val, 0, 3, cs);
                /* ZQ calibration */
-               reg = (1 << 10);
-               mmdc0->mdscr = (u32)MR(reg, 0, 4, cs);
+               val = (1 << 10);
+               mmdc0->mdscr = MR(val, 0, 4, cs);
        }
 
        /* Step 10: Power down control and self-refresh */
-       reg = (tcke & 0x7) << 16 |
-             5            << 12 |  /* PWDT_1: 256 cycles */
-             5            <<  8 |  /* PWDT_0: 256 cycles */
-             1            <<  6 |  /* BOTH_CS_PD */
-             (tcksrx & 0x7) << 3 |
-             (tcksre & 0x7);
-       mmdc0->mdpdc = reg;
-       mmdc0->mapsr = (u32)0x00011006; /* ADOPT power down enabled */
+       mmdc0->mdpdc = (tcke & 0x7) << 16 |
+                       5            << 12 |  /* PWDT_1: 256 cycles */
+                       5            <<  8 |  /* PWDT_0: 256 cycles */
+                       1            <<  6 |  /* BOTH_CS_PD */
+                       (tcksrx & 0x7) << 3 |
+                       (tcksre & 0x7);
+       mmdc0->mapsr = 0x00011006; /* ADOPT power down enabled */
 
        /* Step 11: Configure ZQ calibration: one-time and periodic 1ms */
-       mmdc0->mpzqhwctrl = (u32)0xa1390003;
-       if (i->dsize > 1)
-               mmdc1->mpzqhwctrl = (u32)0xa1390003;
+       val = 0xa1390003;
+       mmdc0->mpzqhwctrl = val;
+       if (sysinfo->dsize > 1)
+               mmdc1->mpzqhwctrl = val;
 
        /* Step 12: Configure and activate periodic refresh */
-       reg = (1 << 14) |       /* REF_SEL: Periodic refresh cycles of 32kHz */
-             (7 << 11);        /* REFR: Refresh Rate - 8 refreshes */
-       mmdc0->mdref = reg;
+       mmdc0->mdref = (1 << 14) | /* REF_SEL: Periodic refresh cycle: 32kHz */
+                      (7 << 11);  /* REFR: Refresh Rate - 8 refreshes */
 
        /* Step 13: Deassert config request - init complete */
-       mmdc0->mdscr = (u32)0x00000000;
+       mmdc0->mdscr = 0x00000000;
 
        /* wait for auto-ZQ calibration to complete */
        mdelay(1);
--

1.9.1

Bit 16 in mapsr register is in a reserved field. Don't write to it.

Cc Stefano Babic <[email protected]>

Cc Tim Harvey <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
 arch/arm/cpu/armv7/mx6/ddr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/cpu/armv7/mx6/ddr.c +1 -2

index af91314..70ce38f 100644
@@ -466,7 +466,7 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *sysinfo,
                        1            <<  6 |  /* BOTH_CS_PD */
                        (tcksrx & 0x7) << 3 |
                        (tcksre & 0x7);
-       mmdc0->mapsr = 0x00011006; /* ADOPT power down enabled */
+       mmdc0->mapsr = 0x00001006; /* ADOPT power down enabled */
 
        /* Step 11: Configure ZQ calibration: one-time and periodic 1ms */
        val = 0xa1390003;
--

1.9.1

Quoted 23 lines · Nikita Kiryanov

Bit 16 in mapsr register is in a reserved field. Don't write to it.

Cc Stefano Babic <[email protected]>

Cc Tim Harvey <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
  arch/arm/cpu/armv7/mx6/ddr.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/cpu/armv7/mx6/ddr.c +1 -2

index af91314..70ce38f 100644
@@ -466,7 +466,7 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *sysinfo,
                         1            <<  6 |  /* BOTH_CS_PD */
                         (tcksrx & 0x7) << 3 |
                         (tcksre & 0x7);
-       mmdc0->mapsr = 0x00011006; /* ADOPT power down enabled */
+       mmdc0->mapsr = 0x00001006; /* ADOPT power down enabled */
 
         /* Step 11: Configure ZQ calibration: one-time and periodic 1ms */
         val = 0xa1390003;
--

Quoted 2 lines

1.9.1

Nikita,

This makes sense per the reference manual, but does not agree with the i.Mx6DQSDL DDR3 Script Aid spreadsheet
(https://community.freescale.com/docs/DOC-94917). I'm curious if you found any other explanation of this or anything else that makes you feel the spreadsheet is in error (vs the RM's).

I've asked our Freescale FAE to clarify.

Hi Tim,

Quoted 34 lines · Tim Harvey

Quoted 24 lines · Nikita Kiryanov

Bit 16 in mapsr register is in a reserved field. Don't write to it.

Cc Stefano Babic <[email protected]>

Cc Tim Harvey <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
  arch/arm/cpu/armv7/mx6/ddr.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

arch/arm/cpu/armv7/mx6/ddr.c +1 -1

index af91314..70ce38f 100644
@@ -466,7 +466,7 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo
 *sysinfo,
                         1            <<  6 |  /* BOTH_CS_PD */
                         (tcksrx & 0x7) << 3 |
                         (tcksre & 0x7);
-       mmdc0->mapsr = 0x00011006; /* ADOPT power down enabled */
+       mmdc0->mapsr = 0x00001006; /* ADOPT power down enabled */
 
         /* Step 11: Configure ZQ calibration: one-time and periodic 1ms

*/

Quoted 4 lines

         val = 0xa1390003;

--
1.9.1

Nikita,

Quoted 5 lines

This makes sense per the reference manual, but does not agree with the i.Mx6DQSDL DDR3 Script Aid spreadsheet
(https://community.freescale.com/docs/DOC-94917). I'm curious if you found any other explanation of this or anything else that makes you feel the spreadsheet is in error (vs the RM's).

Nothing specific, I just don't like to use undocumented features. It's probably benign, but still...

Quoted 1 line

I've asked our Freescale FAE to clarify.

Looking forward to that...

Regards,
Nikita Kiryanov

Quoted 8 lines

Quoted 2 lines

I've asked our Freescale FAE to clarify.

Looking forward to that...

Regards,
Nikita Kiryanov

Nikita,

Freescale confirmed its an error in their spreadsheet
(https://community.freescale.com/docs/DOC-94917#comment-12921), so:

Acked-by Tim Harvey <[email protected]>

According to MX6 TRM, both MMDC and DRAM should be configured to the same powerdown precharge. Currently, mx6_dram_cfg() configures MMDC for fast pd, and the DRAM for slow pd.

Configure MMDC for slow pd.

Cc Stefano Babic <[email protected]>

Cc Tim Harvey <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
 arch/arm/cpu/armv7/mx6/ddr.c | 1 +
 1 file changed, 1 insertion(+)

arch/arm/cpu/armv7/mx6/ddr.c +1 -1

index 70ce38f..c0fb749 100644
@@ -463,6 +463,7 @@ void mx6_dram_cfg(const struct mx6_ddr_sysinfo *sysinfo,
        mmdc0->mdpdc = (tcke & 0x7) << 16 |
                        5            << 12 |  /* PWDT_1: 256 cycles */
                        5            <<  8 |  /* PWDT_0: 256 cycles */
+                       1            <<  7 |  /* SLOW_PD */
                        1            <<  6 |  /* BOTH_CS_PD */
                        (tcksrx & 0x7) << 3 |
                        (tcksre & 0x7);
--

1.9.1

Quoted 3 lines · Nikita Kiryanov

According to MX6 TRM, both MMDC and DRAM should be configured to the same powerdown precharge. Currently, mx6_dram_cfg() configures MMDC for fast pd, and the DRAM for slow pd.

Nikita,

I'm inclined to agree with you. A glance at some of the existing non-spl board config's show this same discrepancy which probably comes form an error in the IMX DDR3 Script Aid spreadsheet
(https://community.freescale.com/docs/DOC-94917).

I would at least add to the description the fact that the precharge pd is MR0 bit12 for DRAM and the current value of 0 indicates 'Slow exit (DLL off)'. I've asked our Freescale FAE for clarification to see if the spreadsheet is in error.

Regards,

Tim

Add initial support for Compulab CM-FX6 CoM.
Support includes MMC, SPI flash, and SPL with dynamic DRAM detection.

Cc Igor Grinberg <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
 arch/arm/cpu/armv7/mx6/ddr.c       |   1 -
 board/compulab/cm_fx6/Makefile     |  12 ++
 board/compulab/cm_fx6/cm_fx6.c     | 108 ++++++++++
 board/compulab/cm_fx6/common.c     |  83 ++++++++
 board/compulab/cm_fx6/common.h     |  36 ++++
 board/compulab/cm_fx6/imximage.cfg |   8 +
 board/compulab/cm_fx6/spl.c        | 400
+++++++++++++++++++++++++++++++++++++ boards.cfg                         |
  2 +
 include/configs/cm_fx6.h           | 227 +++++++++++++++++++++
 9 files changed, 876 insertions(+), 1 deletion(-)
 create mode 100644 board/compulab/cm_fx6/Makefile
 create mode 100644 board/compulab/cm_fx6/cm_fx6.c
 create mode 100644 board/compulab/cm_fx6/common.c
 create mode 100644 board/compulab/cm_fx6/common.h
 create mode 100644 board/compulab/cm_fx6/imximage.cfg
 create mode 100644 board/compulab/cm_fx6/spl.c
 create mode 100644 include/configs/cm_fx6.h

arch/arm/cpu/armv7/mx6/ddr.c +0 -0

index d3891dc..219263a 100644

+++ b/board/compulab/cm_fx6/cm_fx6.c

arch/arm/cpu/armv7/mx6/ddr.c +8 -0

+static ulong bank1_size;
+static ulong bank2_size;
+
+#define MMDC1_MDCTL 0x21B0000
+static int probe_mmdc_config(void)
+{
+	u32 val = readl(0x21B0000);
+	switch (val) {

Quoted 27 lines · Nikita Kiryanov

Add initial support for Compulab CM-FX6 CoM.
Support includes MMC, SPI flash, and SPL with dynamic DRAM detection.

Cc Igor Grinberg <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---
  arch/arm/cpu/armv7/mx6/ddr.c       |   1 -
  board/compulab/cm_fx6/Makefile     |  12 ++
  board/compulab/cm_fx6/cm_fx6.c     | 108 ++++++++++
  board/compulab/cm_fx6/common.c     |  83 ++++++++
  board/compulab/cm_fx6/common.h     |  36 ++++
  board/compulab/cm_fx6/imximage.cfg |   8 +
  board/compulab/cm_fx6/spl.c        | 400
+++++++++++++++++++++++++++++++++++++ boards.cfg                         |
   2 +
  include/configs/cm_fx6.h           | 227 +++++++++++++++++++++
  9 files changed, 876 insertions(+), 1 deletion(-)

create mode 100644 board/compulab/cm_fx6/Makefile create mode 100644 board/compulab/cm_fx6/cm_fx6.c create mode 100644 board/compulab/cm_fx6/common.c create mode 100644 board/compulab/cm_fx6/common.h create mode 100644 board/compulab/cm_fx6/imximage.cfg create mode 100644 board/compulab/cm_fx6/spl.c create mode 100644 include/configs/cm_fx6.h

arch/arm/cpu/armv7/mx6/ddr.c line 9

Quoted 12 lines

arch/arm/cpu/armv7/mx6/ddr.c +0 -1

index d3891dc..219263a 100644
@@ -4,7 +4,6 @@
   *
   * SPDX-License-Identifier:     GPL-2.0+
   */
-
  #include <common.h>
  #include <linux/types.h>
  #include <asm/arch/mx6-ddr.h>

Drop this piece ;-)

[...]

Quoted 1 line

+++ b/board/compulab/cm_fx6/cm_fx6.c

[...]

Quoted 12 lines

arch/arm/cpu/armv7/mx6/ddr.c +12 -0

+static ulong bank1_size;
+static ulong bank2_size;
+
+#define MMDC1_MDCTL 0x21B0000
+static int probe_mmdc_config(void)
+{
+	u32 val = readl(0x21B0000);
+	switch (val) {
+	case 0x83180000: /* DDR_16BIT_256MB */
+		gd->ram_size	= 0x10000000;
+		bank1_size	= 0x10000000;
+		bank2_size	= 0;

arch/arm/cpu/armv7/mx6/ddr.c

Quoted 6 lines

arch/arm/cpu/armv7/mx6/ddr.c +6 -0

+		break;
+	case 0x83190000: /* DDR_32BIT_512MB */
+		gd->ram_size	= 0x20000000;
+		bank1_size	= 0x20000000;
+		bank2_size	= 0;
+		break;

imx_ddr_size() won't cut it here ?

[...]

Quoted 43 lines · Marek Vasut

Quoted 42 lines · Nikita Kiryanov

Add initial support for Compulab CM-FX6 CoM.
Support includes MMC, SPI flash, and SPL with dynamic DRAM detection.

Cc Igor Grinberg <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---

  arch/arm/cpu/armv7/mx6/ddr.c       |   1 -
  board/compulab/cm_fx6/Makefile     |  12 ++
  board/compulab/cm_fx6/cm_fx6.c     | 108 ++++++++++
  board/compulab/cm_fx6/common.c     |  83 ++++++++
  board/compulab/cm_fx6/common.h     |  36 ++++
  board/compulab/cm_fx6/imximage.cfg |   8 +
  board/compulab/cm_fx6/spl.c        | 400

+++++++++++++++++++++++++++++++++++++ boards.cfg |

   2 +

  include/configs/cm_fx6.h           | 227 +++++++++++++++++++++
  9 files changed, 876 insertions(+), 1 deletion(-)

create mode 100644 board/compulab/cm_fx6/Makefile create mode 100644 board/compulab/cm_fx6/cm_fx6.c create mode 100644 board/compulab/cm_fx6/common.c create mode 100644 board/compulab/cm_fx6/common.h create mode 100644 board/compulab/cm_fx6/imximage.cfg create mode 100644 board/compulab/cm_fx6/spl.c create mode 100644 include/configs/cm_fx6.h

arch/arm/cpu/armv7/mx6/ddr.c +0 -0

index d3891dc..219263a 100644
@@ -4,7 +4,6 @@
 
   *
   * SPDX-License-Identifier:     GPL-2.0+
   */
 

Quoted 7 lines

Quoted 5 lines

arch/arm/cpu/armv7/mx6/ddr.c +0 -1

-
 

  #include <common.h>
  #include <linux/types.h>
  #include <asm/arch/mx6-ddr.h>

Drop this piece ;-)

Yep...

Quoted 18 lines

[...]

Quoted 1 line

+++ b/board/compulab/cm_fx6/cm_fx6.c

[...]

Quoted 12 lines

arch/arm/cpu/armv7/mx6/ddr.c +12 -0

+static ulong bank1_size;
+static ulong bank2_size;
+
+#define MMDC1_MDCTL 0x21B0000
+static int probe_mmdc_config(void)
+{
+	u32 val = readl(0x21B0000);
+	switch (val) {
+	case 0x83180000: /* DDR_16BIT_256MB */
+		gd->ram_size	= 0x10000000;
+		bank1_size	= 0x10000000;
+		bank2_size	= 0;

Quoted 8 lines

arch/arm/cpu/armv7/mx6/ddr.c

Quoted 6 lines

arch/arm/cpu/armv7/mx6/ddr.c +6 -0

+		break;
+	case 0x83190000: /* DDR_32BIT_512MB */
+		gd->ram_size	= 0x20000000;
+		bank1_size	= 0x20000000;
+		bank2_size	= 0;
+		break;

imx_ddr_size() won't cut it here ?

It doesn't handle 4GB correctly (returns 0). I suppose I can make a patch which caps the return value of imx_ddr_size() for MX6 socs to 3840MB.
What do you think?

Quoted 81 lines · Nikita Kiryanov

Quoted 43 lines · Marek Vasut

Quoted 42 lines · Nikita Kiryanov

Add initial support for Compulab CM-FX6 CoM.
Support includes MMC, SPI flash, and SPL with dynamic DRAM detection.

Cc Igor Grinberg <[email protected]>

Cc Stefano Babic <[email protected]>

Cc Tom Rini <[email protected]>

Signed-off-by Nikita Kiryanov <[email protected]>

---

   arch/arm/cpu/armv7/mx6/ddr.c       |   1 -
   board/compulab/cm_fx6/Makefile     |  12 ++
   board/compulab/cm_fx6/cm_fx6.c     | 108 ++++++++++
   board/compulab/cm_fx6/common.c     |  83 ++++++++
   board/compulab/cm_fx6/common.h     |  36 ++++
   board/compulab/cm_fx6/imximage.cfg |   8 +
   board/compulab/cm_fx6/spl.c        | 400

+++++++++++++++++++++++++++++++++++++ boards.cfg |

    2 +

   include/configs/cm_fx6.h           | 227 +++++++++++++++++++++
   9 files changed, 876 insertions(+), 1 deletion(-)

create mode 100644 board/compulab/cm_fx6/Makefile create mode 100644 board/compulab/cm_fx6/cm_fx6.c create mode 100644 board/compulab/cm_fx6/common.c create mode 100644 board/compulab/cm_fx6/common.h create mode 100644 board/compulab/cm_fx6/imximage.cfg create mode 100644 board/compulab/cm_fx6/spl.c create mode 100644 include/configs/cm_fx6.h

arch/arm/cpu/armv7/mx6/ddr.c +0 -0

index d3891dc..219263a 100644
@@ -4,7 +4,6 @@
 
    *
    * SPDX-License-Identifier:     GPL-2.0+
    */
 

Quoted 7 lines

Quoted 5 lines

arch/arm/cpu/armv7/mx6/ddr.c +0 -1

-
 

   #include <common.h>
   #include <linux/types.h>
   #include <asm/arch/mx6-ddr.h>

Drop this piece ;-)

Yep...

Quoted 26 lines

[...]

Quoted 1 line

+++ b/board/compulab/cm_fx6/cm_fx6.c

[...]

Quoted 8 lines

arch/arm/cpu/armv7/mx6/ddr.c +8 -0

+static ulong bank1_size;
+static ulong bank2_size;
+
+#define MMDC1_MDCTL 0x21B0000
+static int probe_mmdc_config(void)
+{
+	u32 val = readl(0x21B0000);
+	switch (val) {

arch/arm/cpu/armv7/mx6/ddr.c

Quoted 10 lines

arch/arm/cpu/armv7/mx6/ddr.c +10 -0

+	case 0x83180000: /* DDR_16BIT_256MB */
+		gd->ram_size	= 0x10000000;
+		bank1_size	= 0x10000000;
+		bank2_size	= 0;
+		break;
+	case 0x83190000: /* DDR_32BIT_512MB */
+		gd->ram_size	= 0x20000000;
+		bank1_size	= 0x20000000;
+		bank2_size	= 0;
+		break;

imx_ddr_size() won't cut it here ?

Quoted 4 lines

It doesn't handle 4GB correctly (returns 0). I suppose I can make a patch which caps the return value of imx_ddr_size() for MX6 socs to 3840MB.
What do you think?

That you should check the U-Boot ML, since that's what I did yesterday ;-) But still, this is rather sad practice -- instead of fixing a bug in code which you do know about, you implement such a workaround :-(

Quoted 7 lines

Quoted 6 lines

Quoted 1 line

imx_ddr_size() won't cut it here ?

It doesn't handle 4GB correctly (returns 0). I suppose I can make a patch which caps the return value of imx_ddr_size() for MX6 socs to 3840MB.
What do you think?

Quoted 3 lines

That you should check the U-Boot ML, since that's what I did yesterday ;-) But still, this is rather sad practice -- instead of fixing a bug in code which you do know about, you implement such a workaround :-(

Actually, I only learned of this bug yesterday after you asked about imx_ddr_size(). Glad to hear you already fixed it; I'll look at it for the v2.

Add initial support for Compulab CM-FX6 CoM.
Support includes MMC, SPI flash, and SPL with dynamic DRAM detection.

board/compulab/cm_fx6/spl.c new file +46 -0

+
+static void spl_mx6s_dram_init(enum ddr_config dram_config, int reset)
+{
+       struct mx6_mmdc_calibration calib;
+       struct mx6_ddr_sysinfo sysinfo;
+       struct mx6_ddr3_cfg ddr3_cfg;
+
+       if (reset)
+               ((struct mmdc_p_regs *)MX6_MMDC_P0_MDCTL)->mdmisc = 2;
+
+       calib.p0_mpwldectrl0    = 0x005B0061;
+       calib.p0_mpwldectrl1    = 0x004F0055;
+       calib.p0_mpdgctrl0      = 0x0314030C;
+       calib.p0_mpdgctrl1      = 0x025C0268;
+       calib.p0_mprddlctl      = 0x42464646;
+       calib.p0_mpwrdlctl      = 0x36322C34;
+       ddr3_cfg.mem_speed      = 800;
+       ddr3_cfg.density        = 4;
+       ddr3_cfg.rowaddr        = 14;
+       ddr3_cfg.coladdr        = 10;
+       ddr3_cfg.pagesz         = 2;
+       ddr3_cfg.trcd           = 1800;
+       ddr3_cfg.trcmin         = 5200;
+       ddr3_cfg.trasmin        = 3600;
+       ddr3_cfg.SRT            = 0;
+       sysinfo.cs1_mirror      = 1;
+       sysinfo.cs_density      = 16;
+       sysinfo.bi_on           = 1;
+       sysinfo.rtt_nom         = 1;
+       sysinfo.rtt_wr          = 0;
+       sysinfo.ralat           = 5;
+       sysinfo.walat           = 1;
+       sysinfo.mif3_mode       = 3;
+       sysinfo.rst_to_cke      = 0x23;
+       sysinfo.sde_to_rst      = 0x10;
+       switch (dram_config) {
+       case DDR_16BIT_256MB:
+               sysinfo.dsize = 0;
+               sysinfo.ncs = 1;
+               break;
+       case DDR_32BIT_512MB:
+               sysinfo.dsize = 1;
+               sysinfo.ncs = 1;
+               break;
+       case DDR_32BIT_1GB:
+               sysinfo.dsize = 1;