Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fel-spiflash: Add support for V853 family #208

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fel-spiflash: Add support for V853 family #208

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions fel-spiflash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ void fel_writel(feldev_handle *dev, uint32_t addr, uint32_t val);
#define H6_CCM_SPI_BGR (0x03001000 + 0x96C)
#define H6_CCM_SPI0_GATE_RESET (1 << 0 | 1 << 16)

#define V853_CCM_SPI0_CLK (0x02001000 + 0x940)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please align this the same way as the other lines before and after, so remove two spaces.

#define V853_CCM_SPI_BGR (0x02001000 + 0x96C)
#define V853_CCM_SPI0_GATE_RESET (1 << 0 | 1 << 16)

#define SUNIV_GPC_SPI0 (2)
#define SUNXI_GPC_SPI0 (3)
#define SUN50I_GPC_SPI0 (4)
Expand Down Expand Up @@ -117,6 +121,8 @@ void fel_writel(feldev_handle *dev, uint32_t addr, uint32_t val);
#define SUN6I_SPI0_TXD (spi_base(dev) + 0x200)
#define SUN6I_SPI0_RXD (spi_base(dev) + 0x300)

#define V853_SPI_BA_CCR (spi_base(dev) + 0x44)

#define CCM_SPI0_CLK_DIV_BY_2 (0x1000)
#define CCM_SPI0_CLK_DIV_BY_4 (0x1001)
#define CCM_SPI0_CLK_DIV_BY_6 (0x1002)
Expand All @@ -126,6 +132,8 @@ static uint32_t gpio_base(feldev_handle *dev)
{
soc_info_t *soc_info = dev->soc_info;
switch (soc_info->soc_id) {
case 0x1886: /* V853 */
return 0x02000018;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, but that looks like a horrible hack to accommodate for the larger per-port GPIO block of the newer SoCs (0x30 instead of 0x24 per port). We have support for that GPIO IP in uart0-helloworld-sdboot.c, please take some inspiration there for a proper solution. You should put the datasheet value of 0x02000000 here, as the other SoCs do.

case 0x1816: /* V536 */
case 0x1817: /* V831 */
case 0x1728: /* H6 */
Expand All @@ -146,6 +154,8 @@ static uint32_t spi_base(feldev_handle *dev)
case 0x1663: /* F1C100s */
case 0x1701: /* R40 */
return 0x01C05000;
case 0x1886: /* V853 */
return 0x04025000;
case 0x1816: /* V536 */
case 0x1817: /* V831 */
case 0x1728: /* H6 */
Expand All @@ -166,7 +176,7 @@ static void gpio_set_cfgpin(feldev_handle *dev, int port_num, int pin_num,
uint32_t cfg_reg = port_base + 4 * (pin_num / 8);
uint32_t pin_idx = pin_num % 8;
uint32_t x = readl(cfg_reg);
x &= ~(0x7 << (pin_idx * 4));
x &= ~(0xF << (pin_idx * 4));
Copy link
Member

@paulkocialkowski paulkocialkowski Aug 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apparently this SoC has the pin function select on 4 bits while others have it on 3 bits + 1 bit reserved.
It probably works to mask out the reserved bit in every case (especially since default values are 0 for other SoCs) but this should be mentioned it the commit log. Especially since the reset value is all 1s instead of all 0s like it was on other SoCs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's fine, 0xf should work everywhere. Bit 3 in each field is not implemented on the "older" SoCs, so it's always 0, even if you write a 1 in there. And the reset value seems to 0x7 for quite a while now (after the A20 gen, I think).

x |= val << (pin_idx * 4);
writel(x, cfg_reg);
}
Expand All @@ -192,6 +202,7 @@ static bool soc_is_h6_style(feldev_handle *dev)
case 0x1817: /* V831 */
case 0x1728: /* H6 */
case 0x1823: /* H616 */
case 0x1886: /* V853 */
return true;
default:
return false;
Expand Down Expand Up @@ -244,6 +255,7 @@ static bool spi0_init(feldev_handle *dev)
break;
case 0x1816: /* Allwinner V536 */
case 0x1817: /* Allwinner V831 */
case 0x1886: /* Allwinner V853 */
gpio_set_cfgpin(dev, PC, 1, SUN50I_GPC_SPI0); /* SPI0-CS */
/* fall-through */
case 0x1728: /* Allwinner H6 */
Expand All @@ -265,7 +277,11 @@ static bool spi0_init(feldev_handle *dev)
return false;
}

if (soc_is_h6_style(dev)) {
if (soc_info->soc_id == 0x1886) { /* V853 */
reg_val = readl(V853_CCM_SPI_BGR);
reg_val |= V853_CCM_SPI0_GATE_RESET;
writel(reg_val, V853_CCM_SPI_BGR);
} else if (soc_is_h6_style(dev)) {
reg_val = readl(H6_CCM_SPI_BGR);
reg_val |= H6_CCM_SPI0_GATE_RESET;
writel(reg_val, H6_CCM_SPI_BGR);
Expand Down Expand Up @@ -296,6 +312,11 @@ static bool spi0_init(feldev_handle *dev)
writel(0x00003180, SUNIV_AHB_APB_CFG);
/* divide by 32 */
writel(CCM_SPI0_CLK_DIV_BY_32, SUN6I_SPI0_CCTL);
} else if (soc_info->soc_id == 0x1886) { /* V853 */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is the same as the next code block, only that you are using a different register address.
Please fold this into the next block and add variables and conditionals to set the right address for sun6i / v853 / sun4i, so we don't duplicate the logic.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree. It looks a bit like some refactoring would help here, as I see all newer SoCs being in the same area, for instance the D1/T113 and A523/T527 require the very same SPI and GPIO changes.

/* Divide 24MHz OSC by 4 */
writel(CCM_SPI0_CLK_DIV_BY_4, V853_SPI_BA_CCR);
/* Choose 24MHz from OSC24M and enable clock */
writel((1U << 31), V853_CCM_SPI0_CLK);
} else {
/* divide 24MHz OSC by 4 */
writel(CCM_SPI0_CLK_DIV_BY_4,
Expand Down