Re: [PATCH v2 2/2] usb: musb-new: sunxi: make compatible with UDC/DM gadget model
On 6/27/24 09:06, Andre Przywara wrote:
On Thu, 8 Jun 2023 13:56:31 -0600
Sam Edwards wrote:
Hi,
John asked me have a look at this.
Hi Andre, it's good to hear from you again,
I'd first like to make sure you're aware that the date on this patch is
June *2023,* not June 2024. It's possible things have changed
substantially in the past year. I do not know if this patch is still a
necessity; though if John is nudging about it, it probably is.
Since many sunxi boards do not implement a `board_usb_init`, it's
I am confused, what has this have to do with gadget support? *No* sunxi
board build provides board_usb_init(), but apparently this works fine for
now.
I am all for this converting to DM part, but the rationale seems a bit
off.
For context, board_usb_init() is (was?) the non-DM entry point for USB
functionality; it is (was?) *the* implementation of
usb_gadget_initialize() when !DM_USB_GADGET.
Also can you give some reason for this patch? What does this fix or
improve? "it's better" is a bit thin, "complying with DM" would already be
sufficient, but maybe there is more?
Eh, yeah, "better" is something of a question-begging word isn't it? :)
The main point is to be compatible with DM's view of UDC, which as you
said is a worthy goal in itself. It's "better" because this allows using
DM's all-purpose implementation of usb_gadget_initialize(), which is
(was?) necessary for those targets lacking board_usb_init().
better if we just make the sunxi USB driver compatible with the
DM gadget model, as many other musb-new variants already are.
This change has been verified working on a T113s.
Signed-off-by: Sam Edwards
---
drivers/usb/musb-new/sunxi.c | 50 +++-
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index 510b254f7d..6658cd995d 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -444,6 +444,16 @@ static struct musb_hdrc_config musb_config_h3 = {
.ram_bits = SUNXI_MUSB_RAM_BITS,
};
+#if CONFIG_IS_ENABLED(DM_USB_GADGET)
Please no more #ifdef's. Is there any reason to *not* force
DM_USB_GADGET now, for all sunxi boards, in Kconfig?
Either by "select"ing it in the USB Kconfig, or in arch/arm/Kconfig, like
other platforms do.
Then you don't need to care about the !DM_USB_GADGET definition of this
function and can drop the #ifdef.
I wouldn't be the one to ask. I can't think of any such reason myself.
But to me it sounds like since *no sunxi board provides
board_usb_init()* the only way USB gadgets *could* work is with
DM_USB_GADGET? That'd be reason enough to force it.
+int dm_usb_gadget_handle_interrupts(struct udevice *dev) {
coding style
Sentence fragments are harder to understand. I am assuming you are
saying, "Please put the opening '{' on its own line."
+ struct sunxi_glue *glue = dev_get_priv(dev);
+ struct musb_host_data *host = >mdata;
+
+ host->host->isr(0, host->host);
+ return 0;
+}
+#endif
+
static int musb_usb_probe(struct udevice *dev)
{
struct sunxi_glue *glue = dev_get_priv(dev);
@@ -452,10 +462,6 @@ static int musb_usb_probe(struct udevice *dev)
void *base = dev_read_addr_ptr(dev);
int ret;
-#ifdef CONFIG_USB_MUSB_HOST
- struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
-#endif
-
if (!base)
return -EINVAL;
@@ -486,23 +492,31 @@ static int musb_usb_probe(struct udevice *dev)
pdata.platform_ops = _musb_ops;
pdata.config = glue->cfg->config;
-#ifdef CONFIG_USB_MUSB_HOST
- priv->desc_before_addr = true;
+ if (IS_ENABLED(CONFIG_USB_MUSB_HOST)) {
+ struct usb_bus_priv *priv = dev_get_uclass_priv(dev);
+ priv->desc_before_addr = true;
- pdata.mode = MUSB_HOST;
- host->host = musb_init_controller(, >dev, base);
- if (!host->host)
- return -EIO;
+ pdata.mode = MUSB_HOST;
+ host->host = musb_init_controller(, >dev, base);
+ if (!host->host)
+ return -EIO;
- return musb_lowlevel_init(host);
-#else
- pdata.mode = MUSB_PERIPHERAL;
- host->host = musb_register(, >dev, base);
- if (IS_ERR_OR_NULL(host->host))
- return -EIO;
+ return musb_lowlevel_init(host);
+ } else if (CONFIG_IS_ENABLED(DM_USB_GADGET)) {
+ pdata.mode = MUSB_PERIPHERAL;
+ host->host = musb_init_controller(, >dev, base);
+ if (!host->host)
+ return -EIO;
- return 0;
-#endif
+ return usb_add_gadget_udc(>dev, >host->g);
+ } else {
+ pdata.mode = MUSB_PERIPHERAL;
+ host->host = musb_reg
[PATCH v3] configs: rk3588-turing-rk1: disable SPI flash by default
While the Turing RK1 board has a pad on the PCB for SPI flash, it is
not populated at the factory: supporting SPI flash boot is a user
modification, not an out-of-the-box feature. The defconfig for this
board should therefore not be enabling the SPI flash image nor SPI
support in the SPL, as it causes confusion among downstream users as to
whether the SPI image needs to be distributed.
Fixes: 153ac950a599 ("board: rockchip: Add the Turing RK1 SoM")
Suggested-by: Florian Klink
Signed-off-by: Sam Edwards
Acked-by: Joshua Riek
Reviewed-by: Jonas Karlman
---
Changes v1->v2 (both requested by Jonas):
- Added back (i.e. made unremoved by patch) CONFIG_USB_GADGET_PRODUCT_NUM value
previously automatically removed by savedefconfig.
- Also disable SPI/SFC driver entirely until SPI/SFC lands in the DT (if ever).
Changes v2->v3:
- Rebased onto latest master for clean apply
---
configs/turing-rk1-rk3588_defconfig | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/configs/turing-rk1-rk3588_defconfig
b/configs/turing-rk1-rk3588_defconfig
index 038b14769e..39ef05477a 100644
--- a/configs/turing-rk1-rk3588_defconfig
+++ b/configs/turing-rk1-rk3588_defconfig
@@ -3,17 +3,12 @@ CONFIG_SKIP_LOWLEVEL_INIT=y
CONFIG_SYS_HAS_NONCACHED_MEMORY=y
CONFIG_COUNTER_FREQUENCY=2400
CONFIG_ARCH_ROCKCHIP=y
-CONFIG_SF_DEFAULT_SPEED=2400
-CONFIG_SF_DEFAULT_MODE=0x2000
CONFIG_DEFAULT_DEVICE_TREE="rk3588-turing-rk1"
CONFIG_ROCKCHIP_RK3588=y
-CONFIG_ROCKCHIP_SPI_IMAGE=y
CONFIG_SPL_SERIAL=y
CONFIG_TARGET_TURINGRK1_RK3588=y
CONFIG_DEBUG_UART_BASE=0xFEBC
CONFIG_DEBUG_UART_CLOCK=2400
-CONFIG_SPL_SPI_FLASH_SUPPORT=y
-CONFIG_SPL_SPI=y
CONFIG_SYS_LOAD_ADDR=0xc00800
CONFIG_PCI=y
CONFIG_DEBUG_UART=y
@@ -29,8 +24,6 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_SPL_MAX_SIZE=0x4
CONFIG_SPL_PAD_TO=0x7f8000
# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
-CONFIG_SPL_SPI_LOAD=y
-CONFIG_SYS_SPI_U_BOOT_OFFS=0x6
CONFIG_SPL_ATF=y
CONFIG_CMD_GPIO=y
CONFIG_CMD_GPT=y
@@ -64,11 +57,7 @@ CONFIG_MMC_DW_ROCKCHIP=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_SDMA=y
CONFIG_MMC_SDHCI_ROCKCHIP=y
-CONFIG_SF_DEFAULT_BUS=5
-CONFIG_SPI_FLASH_SFDP_SUPPORT=y
-CONFIG_SPI_FLASH_MACRONIX=y
-CONFIG_SPI_FLASH_XMC=y
-CONFIG_SPI_FLASH_XTX=y
+# CONFIG_SPI_FLASH is not set
CONFIG_PHY_REALTEK=y
CONFIG_DWC_ETH_QOS=y
CONFIG_DWC_ETH_QOS_ROCKCHIP=y
@@ -84,7 +73,6 @@ CONFIG_SPL_RAM=y
CONFIG_SCSI=y
CONFIG_DEBUG_UART_SHIFT=2
CONFIG_SYS_NS16550_MEM32=y
-CONFIG_ROCKCHIP_SFC=y
CONFIG_SYSRESET=y
CONFIG_USB=y
CONFIG_USB_XHCI_HCD=y
--
2.43.2
[RESEND PATCH v2] configs: rk3588-turing-rk1: disable SPI flash by default
While the Turing RK1 board has a pad on the PCB for SPI flash, it is
not populated at the factory: supporting SPI flash boot is a user
modification, not an out-of-the-box feature. The defconfig for this
board should therefore not be enabling the SPI flash image nor SPI
support in the SPL, as it causes confusion among downstream users as to
whether the SPI image needs to be distributed.
Fixes: 153ac950a599 ("board: rockchip: Add the Turing RK1 SoM")
Suggested-by: Florian Klink
Signed-off-by: Sam Edwards
Acked-by: Joshua Riek
Reviewed-by: Jonas Karlman
---
Changes v1->v2 (both requested by Jonas):
- Added back (i.e. made unremoved by patch) CONFIG_USB_GADGET_PRODUCT_NUM value
previously automatically removed by savedefconfig.
- Also disable SPI/SFC driver entirely until SPI/SFC lands in the DT (if ever).
---
configs/turing-rk1-rk3588_defconfig | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/configs/turing-rk1-rk3588_defconfig
b/configs/turing-rk1-rk3588_defconfig
index 07f7b84852..f500b0cf75 100644
--- a/configs/turing-rk1-rk3588_defconfig
+++ b/configs/turing-rk1-rk3588_defconfig
@@ -4,17 +4,12 @@ CONFIG_SYS_HAS_NONCACHED_MEMORY=y
CONFIG_COUNTER_FREQUENCY=2400
CONFIG_ARCH_ROCKCHIP=y
CONFIG_NR_DRAM_BANKS=2
-CONFIG_SF_DEFAULT_SPEED=2400
-CONFIG_SF_DEFAULT_MODE=0x2000
CONFIG_DEFAULT_DEVICE_TREE="rk3588-turing-rk1"
CONFIG_ROCKCHIP_RK3588=y
-CONFIG_ROCKCHIP_SPI_IMAGE=y
CONFIG_SPL_SERIAL=y
CONFIG_TARGET_TURINGRK1_RK3588=y
CONFIG_DEBUG_UART_BASE=0xFEBC
CONFIG_DEBUG_UART_CLOCK=2400
-CONFIG_SPL_SPI_FLASH_SUPPORT=y
-CONFIG_SPL_SPI=y
CONFIG_SYS_LOAD_ADDR=0xc00800
CONFIG_PCI=y
CONFIG_DEBUG_UART=y
@@ -31,8 +26,6 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_SPL_MAX_SIZE=0x4
CONFIG_SPL_PAD_TO=0x7f8000
# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
-CONFIG_SPL_SPI_LOAD=y
-CONFIG_SYS_SPI_U_BOOT_OFFS=0x6
CONFIG_SPL_ATF=y
CONFIG_CMD_GPIO=y
CONFIG_CMD_GPT=y
@@ -66,11 +59,7 @@ CONFIG_MMC_DW_ROCKCHIP=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_SDMA=y
CONFIG_MMC_SDHCI_ROCKCHIP=y
-CONFIG_SF_DEFAULT_BUS=5
-CONFIG_SPI_FLASH_SFDP_SUPPORT=y
-CONFIG_SPI_FLASH_MACRONIX=y
-CONFIG_SPI_FLASH_XMC=y
-CONFIG_SPI_FLASH_XTX=y
+# CONFIG_SPI_FLASH is not set
CONFIG_PHY_REALTEK=y
CONFIG_DWC_ETH_QOS=y
CONFIG_DWC_ETH_QOS_ROCKCHIP=y
@@ -87,7 +76,6 @@ CONFIG_SPL_RAM=y
CONFIG_SCSI=y
CONFIG_DEBUG_UART_SHIFT=2
CONFIG_SYS_NS16550_MEM32=y
-CONFIG_ROCKCHIP_SFC=y
CONFIG_SYSRESET=y
CONFIG_USB=y
CONFIG_USB_XHCI_HCD=y
--
2.43.2
[PATCH] configs: turing-rk1: disable SPI flash by default
While the Turing RK1 board has a pad on the PCB for SPI flash, it is
not populated at the factory: supporting SPI flash boot is a user
modification, not an out-of-the-box feature. The defconfig for this
board should therefore not be enabling the SPI flash image nor SPI
support in the SPL, as it causes confusion among downstream users as to
whether the SPI image needs to be distributed.
Fixes: 153ac950a599 ("board: rockchip: Add the Turing RK1 SoM")
Suggested-by: Florian Klink
Signed-off-by: Sam Edwards
Acked-by: Joshua Riek
Reviewed-by: Jonas Karlman
---
Changes v1->v2 (both requested by Jonas):
- Added back (i.e. made unremoved by patch) CONFIG_USB_GADGET_PRODUCT_NUM value
previously automatically removed by savedefconfig.
- Also disable SPI/SFC driver entirely until SPI/SFC lands in the DT (if ever).
---
configs/turing-rk1-rk3588_defconfig | 14 +-
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/configs/turing-rk1-rk3588_defconfig
b/configs/turing-rk1-rk3588_defconfig
index 07f7b84852..f500b0cf75 100644
--- a/configs/turing-rk1-rk3588_defconfig
+++ b/configs/turing-rk1-rk3588_defconfig
@@ -4,17 +4,12 @@ CONFIG_SYS_HAS_NONCACHED_MEMORY=y
CONFIG_COUNTER_FREQUENCY=2400
CONFIG_ARCH_ROCKCHIP=y
CONFIG_NR_DRAM_BANKS=2
-CONFIG_SF_DEFAULT_SPEED=2400
-CONFIG_SF_DEFAULT_MODE=0x2000
CONFIG_DEFAULT_DEVICE_TREE="rk3588-turing-rk1"
CONFIG_ROCKCHIP_RK3588=y
-CONFIG_ROCKCHIP_SPI_IMAGE=y
CONFIG_SPL_SERIAL=y
CONFIG_TARGET_TURINGRK1_RK3588=y
CONFIG_DEBUG_UART_BASE=0xFEBC
CONFIG_DEBUG_UART_CLOCK=2400
-CONFIG_SPL_SPI_FLASH_SUPPORT=y
-CONFIG_SPL_SPI=y
CONFIG_SYS_LOAD_ADDR=0xc00800
CONFIG_PCI=y
CONFIG_DEBUG_UART=y
@@ -31,8 +26,6 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_SPL_MAX_SIZE=0x4
CONFIG_SPL_PAD_TO=0x7f8000
# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
-CONFIG_SPL_SPI_LOAD=y
-CONFIG_SYS_SPI_U_BOOT_OFFS=0x6
CONFIG_SPL_ATF=y
CONFIG_CMD_GPIO=y
CONFIG_CMD_GPT=y
@@ -66,11 +59,7 @@ CONFIG_MMC_DW_ROCKCHIP=y
CONFIG_MMC_SDHCI=y
CONFIG_MMC_SDHCI_SDMA=y
CONFIG_MMC_SDHCI_ROCKCHIP=y
-CONFIG_SF_DEFAULT_BUS=5
-CONFIG_SPI_FLASH_SFDP_SUPPORT=y
-CONFIG_SPI_FLASH_MACRONIX=y
-CONFIG_SPI_FLASH_XMC=y
-CONFIG_SPI_FLASH_XTX=y
+# CONFIG_SPI_FLASH is not set
CONFIG_PHY_REALTEK=y
CONFIG_DWC_ETH_QOS=y
CONFIG_DWC_ETH_QOS_ROCKCHIP=y
@@ -87,7 +76,6 @@ CONFIG_SPL_RAM=y
CONFIG_SCSI=y
CONFIG_DEBUG_UART_SHIFT=2
CONFIG_SYS_NS16550_MEM32=y
-CONFIG_ROCKCHIP_SFC=y
CONFIG_SYSRESET=y
CONFIG_USB=y
CONFIG_USB_XHCI_HCD=y
--
2.43.2
[PATCH] configs: turing-rk1: disable SPI flash by default
While the Turing RK1 board has a pad on the PCB for SPI flash, it is
not populated at the factory: supporting SPI flash boot is a user
modification, not an out-of-the-box feature. The defconfig for this
board should therefore not be enabling the SPI flash image nor SPI
support in the SPL, as it causes confusion among downstream users as to
whether the SPI image needs to be distributed.
Fixes: 153ac950a599 ("board: rockchip: Add the Turing RK1 SoM")
Suggested-by: Florian Klink
Signed-off-by: Sam Edwards
Acked-by: Joshua Riek
---
configs/turing-rk1-rk3588_defconfig | 6 --
1 file changed, 6 deletions(-)
diff --git a/configs/turing-rk1-rk3588_defconfig
b/configs/turing-rk1-rk3588_defconfig
index 07f7b84852..28c3e50ed4 100644
--- a/configs/turing-rk1-rk3588_defconfig
+++ b/configs/turing-rk1-rk3588_defconfig
@@ -8,13 +8,10 @@ CONFIG_SF_DEFAULT_SPEED=2400
CONFIG_SF_DEFAULT_MODE=0x2000
CONFIG_DEFAULT_DEVICE_TREE="rk3588-turing-rk1"
CONFIG_ROCKCHIP_RK3588=y
-CONFIG_ROCKCHIP_SPI_IMAGE=y
CONFIG_SPL_SERIAL=y
CONFIG_TARGET_TURINGRK1_RK3588=y
CONFIG_DEBUG_UART_BASE=0xFEBC
CONFIG_DEBUG_UART_CLOCK=2400
-CONFIG_SPL_SPI_FLASH_SUPPORT=y
-CONFIG_SPL_SPI=y
CONFIG_SYS_LOAD_ADDR=0xc00800
CONFIG_PCI=y
CONFIG_DEBUG_UART=y
@@ -31,8 +28,6 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_SPL_MAX_SIZE=0x4
CONFIG_SPL_PAD_TO=0x7f8000
# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
-CONFIG_SPL_SPI_LOAD=y
-CONFIG_SYS_SPI_U_BOOT_OFFS=0x6
CONFIG_SPL_ATF=y
CONFIG_CMD_GPIO=y
CONFIG_CMD_GPT=y
@@ -109,7 +104,6 @@ CONFIG_USB_ETHER_MCS7830=y
CONFIG_USB_ETHER_RTL8152=y
CONFIG_USB_ETHER_SMSC95XX=y
CONFIG_USB_GADGET=y
-CONFIG_USB_GADGET_PRODUCT_NUM=0x350b
CONFIG_USB_GADGET_DOWNLOAD=y
CONFIG_USB_FUNCTION_ROCKUSB=y
CONFIG_ERRNO_STR=y
--
2.43.2
Re: [PATCH RFC 00/15] Support SPI NAND booting on the T113
On Wed, Apr 10, 2024 at 10:26 PM John Watts wrote:
>
> This series is my current working and tested setup for booting from
> SPI NAND chips on the Allwinner T113.
>
> I have included the following patches from others. I may have modified
> them to work with the latest mainline:
>
> https://lore.kernel.org/all/20221014030520.3067228-1-...@icenowy.me/
> https://lore.kernel.org/all/2023133432.755363-2-biguncle...@gmail.com/
>
> Hopefully this can get the ball rolling on how to properly implement
> SPI NAND support in mainline U-Boot.
>
> Signed-off-by: John Watts
Hi John,
It doesn't look like I was sent the whole series (only 00 and 01), but
I was able to find it on Patchwork and sift through it. A few general
comments follow:
The introduction of `SUNXI_BOOTED_FROM_SPINAND` is the right call,
since the newer sunxis use this for SPI-NAND, and
`SUNXI_BOOTED_FROM_SPI` for SPI-NOR. The older sunxis, however, will
use `SUNXI_BOOTED_FROM_SPI` for "it's either SPI-NAND or SPI-NOR, have
fun figuring out which." While the rationale in 09/15 ("Instead of
trying to boot from SPI NAND then SPI NOR in series, select one based
on the current boot device.") is solid, we still need some code
(perhaps in/called from `sunxi_get_boot_device`?) to handle the
`SUNXI_BOOTED_FROM_SPI` case by performing flash type detection --
disabled for those sunxis known to use `SUNXI_BOOTED_FROM_SPINAND`, of
course.
06/15 ("sunxi: enable support for SPI NAND booting on SUNIV") should
be dropped from the series. You are updating `suniv_get_boot_source`
when introducing `SUNXI_BOOTED_FROM_SPINAND` anyway, so you can just
hook up `SUNIV_BOOTED_FROM_NAND` at that time (and a heads-up: I think
you got it wired backwards in this version of the series).
On a more fundamental note: I am hesitant about the overall approach
of having NAND reading code in `arch/arm/mach-sunxi/spl_spi_sunxi.c`.
NANDs are more complex than NORs (requiring e.g. bad block handling)
and U-Boot generally keeps the SPL load methods in
`common/spl/spl_*.c`. It's good that the UBI-on-SPI-NAND method is
hooked up there, but the "U-Boot image follows the (good) blocks of
the SPL in NAND" method should probably live in the same directory.
Here's what I'd like to explore: can we introduce a
`common/spl/spl_spinand.c` and update the `drivers/mtd/nand/spi/*.c`
code to support running in SPL? This way
`arch/arm/mach-sunxi/spl_spi_sunxi.c` must only provide the SPL-mode
implementation of SPI -- that is, the actual sunxi-specific bit. Also,
updating the `drivers/mtd/nand/spi/*.c` drivers for SPL-friendliness
will mean dropping those `SPL_SPINAND_{PAGE,BLOCK}_SIZE` configuration
options: the tables in the drivers will be the ones providing those
after NAND autodetection.
Thoughts?
Cheers,
Sam
> ---
> Icenowy Zheng (5):
> sunxi: SPL SPI: extract code for doing SPI transfer
> sunxi: SPL SPI: add support for read command with 2 byte address
> sunxi: SPL SPI: allow multiple boot attempt
> sunxi: SPL SPI: add initial support for booting from SPI NAND
> sunxi: enable support for SPI NAND booting on SUNIV
>
> John Watts (9):
> sunxi: Separate boot device and boot position
> spl: Add BOOT_DEVICE_SPINAND option
> sunxi: Implement BOOT_DEVICE_SPINAND in SPL
> spl: Add SPL_SPINAND configuration options
> sunxi: Use SPL_SPINAND for configuration
> nand: Add spinand_ helper functions
> sunxi: Implement spinand_ helpers
> spl: Support SPI NAND boot in UBI
> spl: Support loading FIT images in UBI
>
> Maksim Kiselev (1):
> sunxi: SPL SPI: Add SPI boot support for the Allwinner R528/T113 SoCs
>
> arch/arm/include/asm/arch-sunxi/spl.h | 3 +-
> arch/arm/include/asm/spl.h| 1 +
> arch/arm/mach-sunxi/Kconfig | 2 +-
> arch/arm/mach-sunxi/board.c | 31 +--
> arch/arm/mach-sunxi/spl_spi_sunxi.c | 348
> +-
> arch/mips/include/asm/spl.h | 1 +
> arch/riscv/include/asm/spl.h | 1 +
> arch/sandbox/include/asm/spl.h| 1 +
> common/spl/Kconfig| 21 ++
> common/spl/spl_ubi.c | 49 -
> include/nand.h| 3 +
> 11 files changed, 354 insertions(+), 107 deletions(-)
> ---
> base-commit: 777c28460947371ada40868dc994dfe8537d7115
> change-id: 20240411-spinand-eb7d8319813b
>
> Best regards,
> --
> John Watts
>
Re: [PATCH v2] board: rockchip: Add the Turing RK1 SoM
On Thu, Apr 11, 2024 at 1:29 AM Florian Klink wrote:
>
> On 23-12-14 18:46:47, Joshua Riek wrote:
> >The Turing RK1 is a Rockchip RK3588 based SoM from Turing Machines.
> >
> >Specifications:
> >
> >Rockchip RK3588 SoC
> >4x ARM Cortex-A76, 4x ARM Cortex-A55
> >8/16/32GB memory LPDDR4x
> >Mali G610MC4 GPU
> >32GB eMMC HS400
> >2x USB 2.0, 2x USB 3.0
> >2x MIPI CSI 4x lanes
> >1x MIPI-DSI DPHY 2x lanes
> >PCIe 2.0 x1, PCIe 3.0 x4
> >1x HDMI 2.1 output, 1x DP 1.4 output
> >Gigabit Ethernet
> >Size: 69.6mm x 45mm (260-pin SO-DIMM connector)
> >
> >Kernel commit:
> >2806a69f3fef ("arm64: dts: rockchip: Add Turing RK1 SoM support")
>
> […]
>
> >diff --git a/configs/turing-rk1-rk3588_defconfig
> >b/configs/turing-rk1-rk3588_defconfig
> >new file mode 100644
> >index 00..289f2da775
> >--- /dev/null
> >+++ b/configs/turing-rk1-rk3588_defconfig
> >@@ -0,0 +1,133 @@
> >+CONFIG_ARM=y
> >+CONFIG_SKIP_LOWLEVEL_INIT=y
> >+CONFIG_SYS_HAS_NONCACHED_MEMORY=y
> >+CONFIG_COUNTER_FREQUENCY=2400
> >+CONFIG_ARCH_ROCKCHIP=y
> >+CONFIG_TEXT_BASE=0x00a0
> >+CONFIG_SPL_LIBCOMMON_SUPPORT=y
> >+CONFIG_SPL_LIBGENERIC_SUPPORT=y
> >+CONFIG_NR_DRAM_BANKS=2
> >+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> >+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xc0
> >+CONFIG_SF_DEFAULT_SPEED=2400
> >+CONFIG_SF_DEFAULT_MODE=0x2000
> >+CONFIG_DEFAULT_DEVICE_TREE="rk3588-turing-rk1"
> >+CONFIG_ROCKCHIP_RK3588=y
> >+CONFIG_SPL_ROCKCHIP_COMMON_BOARD=y
> >+CONFIG_ROCKCHIP_SPI_IMAGE=y
>
Hi Florian,
Thanks for getting in touch!
> Does the RK1 have an SPI chip attached, and is is possible to flash
> u-boot into SPI and boot from it?
The answer you want is "no." Though the RK1 does have an unpopulated
pad for SPI flash, actually installing one would be a pretty involved
user modification, and those users almost certainly can/will build the
SPI boot image themselves.
> This has sparked some confusion on whether "u-boot-rockchip-spi.bin"
> should be provided in a downstream build or not.
Ah yeah the CONFIG_ROCKCHIP_SPI_IMAGE=y might not be a sensible
default given that 99.9% of users don't need it. Is that config entry
the main thing creating the confusion? I think it should be removed
here in U-Boot if so.
Note that the RK1 is a little different from most RK3588 boards in
that UART9 at 115200 baud is the generally-accepted debug UART (due
both to the popularity of pairing it with the Turing Pi 2
clusterboard, and for pin-compatibility with most NVIDIA Jetson SoMs),
and setting this very early in boot requires using Rockchip's
"ddrbin_tool" to change the configuration embedded in the ddrbin/TPL
image. If you're already supporting other targets that require ddrbin
configuration changes, please add these for RK1:
uart id=9
uart iomux=0
uart baudrate=115200
...but if this would require going significantly out of your way,
don't worry about it. IIUC this is only required to get TPL+SPL output
routed correctly: the U-Boot monitor + kernel will still
(re)initialize UART9 appropriately.
Cheers,
Sam
>
> […]
>
> Thanks,
> Florian
Re: [PATCH v2 0/2] sunxi, usb: UDC/DM gadget model support
On Thu, Apr 11, 2024 at 1:40 AM John Watts wrote: > > On Thu, Apr 11, 2024 at 12:52:14AM -0600, Sam Edwards wrote: > > Hi John, > > > > This patch was developed against (and used very heavily on) the Turing > > Pi 2, which has an Allwinner T113-s3 SoC. Likely it should work for > > any T113/D1 board. I haven't been encountering any USB errors but also > > my use case hasn't gone much beyond the `udc` command. What > > device/errors do you have over there? > > > > Cheers, > > Sam > > Hi Sam, > > I made a list of things that do work: > > - DFU (slowly, probably due to no DMA) to RAM > - CDC serial console > > Running a command like this: > > ubi part ubi; ubifsmount ubi:root; ums 0 ubi 0; Hi John, Ahh I see the problem. In U-Boot, `ubi` isn't actually a block device: it's implemented as a stub in the block layer, and the filesystem layer redirects `ubi` accesses to the currently-mounted ubifs instead. But the `ums` command works on the block layer, so it's not being intercepted, and instead hitting that stub and likely crashing. In the spirit of the Linux ubiblock driver, I have another patchset[1] I've been working on[2] to expose static ubivols as true read-only block devices. Note that this only works for static volumes: the access semantics of dynamic volumes are too flash-like to support block device access, so accessing them with `ums` likely will never work like users expect. Still, there could be some interaction issue between `ums`<->USB that I haven't identified. Could you try with mmc (if available) or a ramdisk (otherwise) just to confirm that `ums` is fine? Regards, Sam [1] https://lore.kernel.org/u-boot/20230812000606.72319-1-cfswo...@gmail.com/T/ [2] Not very diligently; if you're interested in helping test it, I'd love to get back to it. > > Gives this output in dmesg: > > [3633079.772330] usb-storage 1-1.1:1.0: USB Mass Storage device detected > [3633079.772506] scsi host9: usb-storage 1-1.1:1.0 > [3633080.794607] scsi 9:0:0:0: Direct-Access LinuxUMS disk 0 > PQ: 0 ANSI: 2 > [3633080.794941] sd 9:0:0:0: Attached scsi generic sg6 type 0 > [3633080.795214] sd 9:0:0:0: [sdg] 3942645758 512-byte logical blocks: (2.02 > TB/1.83 TiB) > [3633080.795220] sd 9:0:0:0: [sdg] 3925868545-byte physical blocks > [3633080.795341] sd 9:0:0:0: [sdg] Write Protect is off > [3633080.795345] sd 9:0:0:0: [sdg] Mode Sense: 0f 00 00 00 > [3633080.795462] sd 9:0:0:0: [sdg] Write cache: enabled, read cache: enabled, > doesn't support DPO or FUA > [3633080.907359] usb 1-1.1: reset high-speed USB device number 44 using > xhci_hcd > [3633081.021905] usb 1-1.1: device descriptor read/64, error -71 > [3633081.238566] usb 1-1.1: device descriptor read/64, error -71 > [3633081.448573] usb 1-1.1: reset high-speed USB device number 44 using > xhci_hcd > [3633081.558566] usb 1-1.1: device descriptor read/64, error -71 > [3633081.775236] usb 1-1.1: device descriptor read/64, error -71 > [3633081.988559] usb 1-1.1: reset high-speed USB device number 44 using > xhci_hcd > [3633086.788615] usb 1-1.1: Device not responding to setup address. > [3633091.799190] usb 1-1.1: Device not responding to setup address. > [3633092.008482] usb 1-1.1: device not accepting address 44, error -71 > [3633092.747719] usb 1-1.1: USB disconnect, device number 44 > [3633092.748488] device offline error, dev sdg, sector 0 op 0x0:(READ) flags > 0x0 phys_seg 2 prio class 2 > [3633092.748502] buffer_io_error: 10 callbacks suppressed > [3633092.748504] Buffer I/O error on dev sdg, logical block 0, async page read > [3633092.748511] Buffer I/O error on dev sdg, logical block 1, async page read > [3633092.748520] device offline error, dev sdg, sector 4 op 0x0:(READ) flags > 0x0 phys_seg 2 prio class 2 > [3633092.748525] Buffer I/O error on dev sdg, logical block 2, async page read > [3633092.748529] Buffer I/O error on dev sdg, logical block 3, async page read > [3633092.748582] device offline error, dev sdg, sector 0 op 0x0:(READ) flags > 0x0 phys_seg 4 prio class 2 > [3633092.748594] Buffer I/O error on dev sdg, logical block 0, async page read > [3633092.748600] Buffer I/O error on dev sdg, logical block 1, async page read > [3633092.748605] Buffer I/O error on dev sdg, logical block 2, async page read > [3633092.748609] Buffer I/O error on dev sdg, logical block 3, async page read > [3633092.748621] ldm_validate_partition_table(): Disk read failed. > [3633092.748638] device offline error, dev sdg, sector 0 op 0x0:(READ) flags > 0x0 phys_seg 2 prio class 2 > [3633092.748644] Buffer I/O error on dev sdg, logical block 0, async page read > [3633092.748649] Buffer I/O error on dev sdg, logical block 1, async page read > [3633092.748665] device offline error, dev sdg,
Re: [PATCH v3 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end
On 3/13/24 10:23, Ilias Apalodimas wrote:
commit 3ebd1cbc49f0 ("arm: make __bss_start and __bss_end__ compiler-generated")
and
commit f84a7b8f54db ("ARM: Fix __bss_start and __bss_end in linker scripts")
were moving the bss_start/end on c generated variables that were
injected in their own sections. The reason was that we needed relative
relocations for position independent code and linker bugs back then
prevented us from doing so [0].
However, the linker documentation pages states that symbols that are
defined within a section definition will create a relocatable type with
the value being a fixed offset from the base of a section [1].
So let's start cleaning this up starting with the bss_start and bss_end
variables. Convert them into symbols within the .bss section definition.
[0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for
shared object")
[1] https://sourceware.org/binutils/docs/ld/Expression-Section.html
Tested-by: Caleb Connolly # Qualcomm sdm845
Signed-off-by: Ilias Apalodimas
---
arch/arm/cpu/armv8/u-boot-spl.lds| 18 +++---
arch/arm/cpu/armv8/u-boot.lds| 16
arch/arm/cpu/u-boot.lds | 22 +++---
arch/arm/lib/sections.c | 2 --
arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 15 ---
arch/arm/mach-zynq/u-boot.lds| 22 +++---
6 files changed, 29 insertions(+), 66 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds
b/arch/arm/cpu/armv8/u-boot-spl.lds
index 7cb9d731246d..692414fe46fb 100644
--- a/arch/arm/cpu/armv8/u-boot-spl.lds
+++ b/arch/arm/cpu/armv8/u-boot-spl.lds
@@ -63,18 +63,11 @@ SECTIONS
_image_binary_end = .;
- .bss_start (NOLOAD) : {
- . = ALIGN(8);
- KEEP(*(.__bss_start));
- } >.sdram
-
- .bss (NOLOAD) : {
+ .bss : {
+ __bss_start = .;
*(.bss*)
-. = ALIGN(8);
- } >.sdram
-
- .bss_end (NOLOAD) : {
- KEEP(*(.__bss_end));
+ . = ALIGN(8);
+ __bss_end = .;
} >.sdram
/DISCARD/ : { *(.rela*) }
@@ -89,3 +82,6 @@ SECTIONS
#include "linux-kernel-image-header-vars.h"
#endif
}
+ASSERT(CONFIG_SPL_BSS_START_ADDR % 8 == 0, \
+ "CONFIG_SPL_BSS_START_ADDR must be 8-byte aligned");
+
Git complains about this added blank line at the end of the file. (My
personal preference would be a blank line before the ASSERT, if the
ASSERT is truly necessary.)
But beyond that:
Tested-by: Sam Edwards # Binary output identical
Still really excited for this to land! I'm going to have to blow the
dust off of my Clang/LLD support series here soon. :)
Best,
Sam
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index fb6a30c922f7..9640cc7a04b8 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -149,19 +149,11 @@ SECTIONS
_end = .;
- . = ALIGN(8);
-
- .bss_start : {
- KEEP(*(.__bss_start));
- }
-
- .bss : {
+ .bss ALIGN(8): {
+ __bss_start = .;
*(.bss*)
-. = ALIGN(8);
- }
-
- .bss_end : {
- KEEP(*(.__bss_end));
+ . = ALIGN(8);
+ __bss_end = .;
}
/DISCARD/ : { *(.dynsym) }
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 7724c9332c3b..0dfe5f633b16 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -207,23 +207,15 @@ SECTIONS
}
/*
- * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
- * __bss_base and __bss_limit are for linker only (overlay ordering)
+ * These sections occupy the same memory, but their lifetimes do
+ * not overlap: U-Boot initializes .bss only after applying dynamic
+ * relocations and therefore after it doesn't need .rel.dyn any more.
*/
-
- .bss_start __rel_dyn_start (OVERLAY) : {
- KEEP(*(.__bss_start));
- __bss_base = .;
- }
-
- .bss __bss_base (OVERLAY) : {
+ .bss ADDR(.rel.dyn) (OVERLAY): {
+ __bss_start = .;
*(.bss*)
-. = ALIGN(4);
-__bss_limit = .;
- }
-
- .bss_end __bss_limit (OVERLAY) : {
- KEEP(*(.__bss_end));
+ . = ALIGN(4);
+ __bss_end = .;
}
.dynsym _image_binary_end : { *(.dynsym) }
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index 857879711c6a..8e8bd5797e16 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -19,8 +19,6 @@
* aliasing warnings.
*/
-char __bss_start[0] __section(".__bss_start");
-char __bss_end[0] __section(".__bss_end");
char __image_copy_start[0] __section(".__image_copy_start");
cha
Re: [PATCH 6/6] arm: move image_copy_start/end to linker symbols
On 3/6/24 06:23, Ilias Apalodimas wrote:
On Wed, 6 Mar 2024 at 12:37, Ilias Apalodimas
wrote:
On Wed, 6 Mar 2024 at 11:35, Ilias Apalodimas
wrote:
Hi Sam,
On Wed, 6 Mar 2024 at 10:22, Sam Edwards wrote:
On 3/4/24 02:01, Ilias Apalodimas wrote:
image_copy_start/end are defined as c variables in order to force the compiler
emit relative references. However, defining those within a section definition
will do the same thing.
So let's remove the special sections from the linker scripts, the
variable definitions from sections.c and define them as a symbols within
a section.
Signed-off-by: Ilias Apalodimas
Tested-by: Sam Edwards # Binary output identical
Since the __image_copy_* symbols are marking boundaries in the whole
image as opposed to a single section, I'd find it clearer if they were
loose in the SECTIONS { ... } block, so as not to imply that they are
coupled to any particular section. Thoughts?
The reason I included it within a section is my understanding of the
ld (way older) manual [0].
Copy-pasting from that:
" Assignment statements may appear:
- as commands in their own right in an ld script; or
- as independent statements within a SECTIONS command; or
- as part of the contents of a section definition in a SECTIONS command.
The first two cases are equivalent in effect--both define a symbol
with an absolute address. The last case defines a symbol whose address
is relative to a particular section."
So, since we need relocatable entries, I included it within a section.
Looking at the latest ld [1] the wording has changed a bit it says
"Expressions appearing outside an output section definition treat all
numbers as absolute addresses" and it also has the following example
SECTIONS
{
. = 0x100;
__executable_start = 0x100;
.data :
{
. = 0x10;
__data_start = 0x10;
*(.data)
}
…
}
both . and __executable_start are set to the absolute address 0x100 in
the first two assignments, then both . and __data_start are set to
0x10 relative to the .data section in the second two assignments.
So I assume the same behavior persists?
I just tested moving image_binary_end outside the section definition
and it's still emitted properly. I'll try to figure this out and on v3
I'll move both image_copy_start/end outside the sections.
I also noticed that I forgot to change
arch/arm/cpu/armv8/u-boot-spl.lds and get rid of the .image_copy_end.
Reading the manual again, the symbols will be emitted as relocatable
entries regardless of their placement after that LD bug you mentioned
is fixed.
The only thing that will change when moving it outside the section
definition is that an absolute value will be set, instead of a
relative one to the start of the section. But that won't change the
final binary placement in our case.
As far as I know, the linker is smart enough to understand that *any*
symbol defined in terms of a memory location (e.g. `.` or
`_other_symname`, ...) cannot be absolute when linking a PIE. The [1]
documentation excerpt you cited seems to be saying that the exception to
this rule is when a linker symbol is assigned to a constant value loose
in the SECTIONS block -- and that apparently within the context of a
`.section : {...}`, number literals are treated as offsets from the
section's beginning, not absolute addresses: so we get relative symbols
once again. This seems to be what your [0] documentation excerpt is also
trying to say: "numbers in a section definition are relative to the
section," not "relative symbols must be assigned in a section definition."
I played around a bit and removed the .image_copy_end section from the
SPL in favor of a symbol.
Compiling for xilinx_zynqmp_kria_defconfig which builds u-boot & SPL
(note that I am building natively on an arm64 box)
# Before -- image_copy_end is .efi_runtime_rel and spl still has a
section for .image_copy_end
$:~/work/u-boot-tpm>readelf -s u-boot | grep image_cop
9339: 0811f550 0 NOTYPE GLOBAL DEFAULT 10 __image_copy_end
10614: 0800 0 NOTYPE GLOBAL DEFAULT1 __image_copy_start
$:~/work/u-boot-tpm>readelf -s spl/u-boot-spl | grep image_cop
5: fffe0dc8 0 SECTION LOCAL DEFAULT5 .image_copy_end
1705: fffc 0 NOTYPE GLOBAL DEFAULT1 __image_copy_start
# After -- image_copy_end moved outside .efi_runtime_rel and
.image_copy_end converted to a linker symbol
$:~/work/u-boot-tpm>readelf -s u-boot | grep image_cop
9339: 0811f550 0 NOTYPE GLOBAL DEFAULT 10 __image_copy_end
10614: 0800 0 NOTYPE GLOBAL DEFAULT1 __image_copy_start
$:~/work/u-boot-tpm>readelf -s spl/u-boot-spl | grep image_cop
1349: fffe0dc8 0 NOTYPE GLOBAL DEFAULT4 __image_copy_end
1705: fffc 0 NOTYPE GLOBAL DEFAULT1 __image_copy_start
Nothing changes in offsets and sizes.
The
Re: [PATCH 5/6] arm: fix __efi_runtime_start/end definitions
On 3/6/24 02:13, Ilias Apalodimas wrote:
Hi Sam,
Again thank you for the elaborate review. This really helps a lot.
On Wed, 6 Mar 2024 at 10:14, Sam Edwards wrote:
On 3/4/24 02:01, Ilias Apalodimas wrote:
__efi_runtime_start/end are defined as c variables for arm7 only in
order to force the compiler emit relative references. However, defining
those within a section definition will do the same thing. On top of that
the v8 linker scripts define it as a symbol.
So let's remove the special sections from the linker scripts, the
variable definitions from sections.c and define them as a symbols within
the correct section.
Signed-off-by: Ilias Apalodimas
Tested-by: Sam Edwards # Binary output identical
Thanks for the cleanup,
Sam
---
arch/arm/cpu/u-boot.lds| 12 +++-
arch/arm/lib/sections.c| 2 --
arch/arm/mach-zynq/u-boot.lds | 12 +++-
include/asm-generic/sections.h | 1 +
4 files changed, 7 insertions(+), 20 deletions(-)
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 7c6e7891d360..df55bb716e35 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -43,18 +43,12 @@ SECTIONS
}
/* This needs to come before *(.text*) */
- .__efi_runtime_start : {
- *(.__efi_runtime_start)
- }
-
- .efi_runtime : {
+ .efi_runtime ALIGN(4) : {
Do we truly require the ALIGN(4)? If I understand correctly, by default,
the linker calculates the alignment of an output section as the least
common multiple of the input sections' alignment requirements -- meaning
most (perhaps all) of our ALIGN()s today are redundant.
I don't think we do. But I preserved those for a few reasons.
For the time
being, I'm in favor of merging existing `. = ALIGN(x)` into each
following section for clarity and to avoid the testing overhead of
removing them in the same patch as other changes. However, in the near
future (perhaps even as "near" as v2 of this series?), I'd also like to
see a patch that eliminates unnecessary ALIGN()s altogether. Introducing
additional ALIGN()s where we already know they aren't needed may be a
step away from that goal.
So as you already mentioned the reason I preserved this is:
- Reduce the testing overhead by preserving the same layout for now
- In the future, I am playing around with the idea of mapping U-Boot
(not SPL but the relocated full U-Boot) in sections with proper memory
permissions (R), RW^X etc). In that case, we will need a 4k section
alignment and we can repurpose the ALIGN(4/8) to
ALIGN(CONSTANT(COMMONPAGESIZE)).
Thoughts?
Ah, right: I had forgotten for a moment that the ultimate goal was to
clean up the memory protection bit situation.
Okay, as long as that future cleanup will also remove all unnecessary
uses of ALIGN() (i.e. any that don't result in an actual change in
memory layout) then that sounds great to me.
Reviewed-by: Sam Edwards
Cheers,
Sam
Thanks
/Ilias
+ __efi_runtime_start = .;
*(.text.efi_runtime*)
*(.rodata.efi_runtime*)
*(.data.efi_runtime*)
- }
-
- .__efi_runtime_stop : {
- *(.__efi_runtime_stop)
+ __efi_runtime_stop = .;
}
.text_rest :
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index 1ee3dd3667ba..a4d4202e99f5 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -25,6 +25,4 @@ char __secure_start[0] __section(".__secure_start");
char __secure_end[0] __section(".__secure_end");
char __secure_stack_start[0] __section(".__secure_stack_start");
char __secure_stack_end[0] __section(".__secure_stack_end");
-char __efi_runtime_start[0] __section(".__efi_runtime_start");
-char __efi_runtime_stop[0] __section(".__efi_runtime_stop");
char _end[0] __section(".__end");
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index 71dea4a1f60a..fcd0f42a7106 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -22,18 +22,12 @@ SECTIONS
}
/* This needs to come before *(.text*) */
- .__efi_runtime_start : {
- *(.__efi_runtime_start)
- }
-
- .efi_runtime : {
+ .efi_runtime ALIGN(4) : {
Ditto above
+ __efi_runtime_start = .;
*(.text.efi_runtime*)
*(.rodata.efi_runtime*)
*(.data.efi_runtime*)
- }
-
- .__efi_runtime_stop : {
- *(.__efi_runtime_stop)
+ __efi_runtime_stop = .;
}
.text_rest :
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 60949200dd93..b6bca53db10d 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[];
extern char __ctors_start[], __ctors_end[];
extern char __efi
Re: [PATCH 6/6] arm: move image_copy_start/end to linker symbols
On 3/4/24 02:01, Ilias Apalodimas wrote:
image_copy_start/end are defined as c variables in order to force the compiler
emit relative references. However, defining those within a section definition
will do the same thing.
So let's remove the special sections from the linker scripts, the
variable definitions from sections.c and define them as a symbols within
a section.
Signed-off-by: Ilias Apalodimas
Tested-by: Sam Edwards # Binary output identical
Since the __image_copy_* symbols are marking boundaries in the whole
image as opposed to a single section, I'd find it clearer if they were
loose in the SECTIONS { ... } block, so as not to imply that they are
coupled to any particular section. Thoughts?
Thanks for the cleanup,
Sam
---
arch/arm/cpu/armv8/u-boot.lds| 10 ++
arch/arm/cpu/u-boot.lds | 10 ++
arch/arm/lib/sections.c | 2 --
arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 8 ++--
arch/arm/mach-zynq/u-boot.lds| 9 ++---
5 files changed, 8 insertions(+), 31 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index e737de761a9d..340acf5c043b 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -23,7 +23,7 @@ SECTIONS
. = ALIGN(8);
.text :
{
- *(.__image_copy_start)
+ __image_copy_start = .;
CPUDIR/start.o (.text*)
}
@@ -120,13 +120,7 @@ SECTIONS
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
__efi_runtime_rel_stop = .;
- }
-
- . = ALIGN(8);
-
- .image_copy_end :
- {
- *(.__image_copy_end)
+ __image_copy_end = .;
}
.rela.dyn ALIGN(8) : {
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index df55bb716e35..7c4f9c2d4dd3 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -37,7 +37,7 @@ SECTIONS
. = ALIGN(4);
.text :
{
- *(.__image_copy_start)
+ __image_copy_start = .;
*(.vectors)
CPUDIR/start.o (.text*)
}
@@ -151,13 +151,7 @@ SECTIONS
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
__efi_runtime_rel_stop = .;
- }
-
- . = ALIGN(4);
-
- .image_copy_end :
- {
- *(.__image_copy_end)
+ __image_copy_end = .;
}
.rel.dyn ALIGN(4) : {
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index a4d4202e99f5..db5463b2bbbc 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -19,8 +19,6 @@
* aliasing warnings.
*/
-char __image_copy_start[0] __section(".__image_copy_start");
-char __image_copy_end[0] __section(".__image_copy_end");
char __secure_start[0] __section(".__secure_start");
char __secure_end[0] __section(".__secure_end");
char __secure_stack_start[0] __section(".__secure_stack_start");
diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
index b7887194026e..4b480ebb0c4d 100644
--- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
+++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
@@ -24,7 +24,7 @@ SECTIONS
.text : {
. = ALIGN(8);
- *(.__image_copy_start)
+ __image_copy_start = .;
CPUDIR/start.o (.text*)
*(.text*)
}
@@ -42,11 +42,7 @@ SECTIONS
__u_boot_list : {
. = ALIGN(8);
KEEP(*(SORT(__u_boot_list*)));
- }
-
- .image_copy_end : {
- . = ALIGN(8);
- *(.__image_copy_end)
+ __image_copy_end = .;
}
.end : {
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index fcd0f42a7106..553bcf182272 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -16,7 +16,7 @@ SECTIONS
. = ALIGN(4);
.text :
{
- *(.__image_copy_start)
+ __image_copy_start = .;
*(.vectors)
CPUDIR/start.o (.text*)
}
@@ -57,12 +57,7 @@ SECTIONS
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
__efi_runtime_rel_stop = .;
- }
-
- . = ALIGN(8);
- .image_copy_end :
- {
- *(.__image_copy_end)
+ __image_copy_end = .;
}
.rel.dyn ALIGN(8) : {
Re: [PATCH 5/6] arm: fix __efi_runtime_start/end definitions
On 3/4/24 02:01, Ilias Apalodimas wrote:
__efi_runtime_start/end are defined as c variables for arm7 only in
order to force the compiler emit relative references. However, defining
those within a section definition will do the same thing. On top of that
the v8 linker scripts define it as a symbol.
So let's remove the special sections from the linker scripts, the
variable definitions from sections.c and define them as a symbols within
the correct section.
Signed-off-by: Ilias Apalodimas
Tested-by: Sam Edwards # Binary output identical
Thanks for the cleanup,
Sam
---
arch/arm/cpu/u-boot.lds| 12 +++-
arch/arm/lib/sections.c| 2 --
arch/arm/mach-zynq/u-boot.lds | 12 +++-
include/asm-generic/sections.h | 1 +
4 files changed, 7 insertions(+), 20 deletions(-)
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 7c6e7891d360..df55bb716e35 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -43,18 +43,12 @@ SECTIONS
}
/* This needs to come before *(.text*) */
- .__efi_runtime_start : {
- *(.__efi_runtime_start)
- }
-
- .efi_runtime : {
+ .efi_runtime ALIGN(4) : {
Do we truly require the ALIGN(4)? If I understand correctly, by default,
the linker calculates the alignment of an output section as the least
common multiple of the input sections' alignment requirements -- meaning
most (perhaps all) of our ALIGN()s today are redundant. For the time
being, I'm in favor of merging existing `. = ALIGN(x)` into each
following section for clarity and to avoid the testing overhead of
removing them in the same patch as other changes. However, in the near
future (perhaps even as "near" as v2 of this series?), I'd also like to
see a patch that eliminates unnecessary ALIGN()s altogether. Introducing
additional ALIGN()s where we already know they aren't needed may be a
step away from that goal.
+ __efi_runtime_start = .;
*(.text.efi_runtime*)
*(.rodata.efi_runtime*)
*(.data.efi_runtime*)
- }
-
- .__efi_runtime_stop : {
- *(.__efi_runtime_stop)
+ __efi_runtime_stop = .;
}
.text_rest :
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index 1ee3dd3667ba..a4d4202e99f5 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -25,6 +25,4 @@ char __secure_start[0] __section(".__secure_start");
char __secure_end[0] __section(".__secure_end");
char __secure_stack_start[0] __section(".__secure_stack_start");
char __secure_stack_end[0] __section(".__secure_stack_end");
-char __efi_runtime_start[0] __section(".__efi_runtime_start");
-char __efi_runtime_stop[0] __section(".__efi_runtime_stop");
char _end[0] __section(".__end");
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index 71dea4a1f60a..fcd0f42a7106 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -22,18 +22,12 @@ SECTIONS
}
/* This needs to come before *(.text*) */
- .__efi_runtime_start : {
- *(.__efi_runtime_start)
- }
-
- .efi_runtime : {
+ .efi_runtime ALIGN(4) : {
Ditto above
+ __efi_runtime_start = .;
*(.text.efi_runtime*)
*(.rodata.efi_runtime*)
*(.data.efi_runtime*)
- }
-
- .__efi_runtime_stop : {
- *(.__efi_runtime_stop)
+ __efi_runtime_stop = .;
}
.text_rest :
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 60949200dd93..b6bca53db10d 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[];
extern char __ctors_start[], __ctors_end[];
extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[];
+extern char __efi_runtime_start[], __efi_runtime_stop[];
/* function descriptor handling (if any). Override
* in asm/sections.h */
Re: [PATCH 4/6] arm: clean up v7 and v8 linker scripts for __rel_dyn_start/end
On 3/4/24 02:01, Ilias Apalodimas wrote:
commit 47bd65ef057f ("arm: make __rel_dyn_{start, end} compiler-generated")
were moving the __rel_dyn_start/end on c generated variables that were
injected in their own sections. The reason was that we needed relative
relocations for position independent code and linker bugs back then
prevented us from doing so.
However, the linker documentation pages states that symbols that are
defined within a section definition will create a relocatable
type with the value being a fixed offset from the base of a section [0].
This have been fixed since 2016 [1]
[0] https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_mono/ld.html#SEC13
[1] commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared
object")
Signed-off-by: Ilias Apalodimas
Reviewed-by: Sam Edwards
Tested-by: Sam Edwards # Binary output identical
Thanks for the cleanup,
Sam
---
arch/arm/cpu/armv8/u-boot.lds | 16 +++-
arch/arm/cpu/u-boot.lds | 14 +++---
arch/arm/lib/sections.c | 2 --
arch/arm/mach-zynq/u-boot.lds | 14 +++---
4 files changed, 9 insertions(+), 37 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index eccb116d3cfa..e737de761a9d 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -129,20 +129,10 @@ SECTIONS
*(.__image_copy_end)
}
- . = ALIGN(8);
-
- .rel_dyn_start :
- {
- *(.__rel_dyn_start)
- }
-
- .rela.dyn : {
+ .rela.dyn ALIGN(8) : {
+ __rel_dyn_start = .;
*(.rela*)
- }
-
- .rel_dyn_end :
- {
- *(.__rel_dyn_end)
+ __rel_dyn_end = .;
}
_end = .;
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 70e78ce46672..7c6e7891d360 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -166,18 +166,10 @@ SECTIONS
*(.__image_copy_end)
}
- .rel_dyn_start :
- {
- *(.__rel_dyn_start)
- }
-
- .rel.dyn : {
+ .rel.dyn ALIGN(4) : {
+ __rel_dyn_start = .;
*(.rel*)
- }
-
- .rel_dyn_end :
- {
- *(.__rel_dyn_end)
+ __rel_dyn_end = .;
}
.end :
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index ddfde52163fc..1ee3dd3667ba 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -21,8 +21,6 @@
char __image_copy_start[0] __section(".__image_copy_start");
char __image_copy_end[0] __section(".__image_copy_end");
-char __rel_dyn_start[0] __section(".__rel_dyn_start");
-char __rel_dyn_end[0] __section(".__rel_dyn_end");
char __secure_start[0] __section(".__secure_start");
char __secure_end[0] __section(".__secure_end");
char __secure_stack_start[0] __section(".__secure_stack_start");
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index 66a9e37f9198..71dea4a1f60a 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -71,18 +71,10 @@ SECTIONS
*(.__image_copy_end)
}
- .rel_dyn_start :
- {
- *(.__rel_dyn_start)
- }
-
- .rel.dyn : {
+ .rel.dyn ALIGN(8) : {
+ __rel_dyn_start = .;
*(.rel*)
- }
-
- .rel_dyn_end :
- {
- *(.__rel_dyn_end)
+ __rel_dyn_end = .;
}
.end :
Re: [PATCH 3/6] arm: fix __efi_runtime_rel_start/end definitions
On 3/4/24 02:01, Ilias Apalodimas wrote:
__efi_runtime_rel_start/end are defined as c variables for arm7 only in
order to force the compiler emit relative references. However, defining
those within a section definition will do the same thing. On top of that
the v8 linker scripts define it as a symbol.
So let's remove the special sections from the linker scripts, the
variable definitions from sections.c and define them as a symbols within
the correct section.
Signed-off-by: Ilias Apalodimas
Reviewed-by: Sam Edwards
Tested-by: Sam Edwards # Binary output identical
Thanks for the cleanup,
Sam
---
arch/arm/cpu/armv8/u-boot.lds | 4 +---
arch/arm/cpu/u-boot.lds| 16 +++-
arch/arm/lib/sections.c| 2 --
arch/arm/mach-zynq/u-boot.lds | 16 +++-
include/asm-generic/sections.h | 2 ++
lib/efi_loader/efi_runtime.c | 1 +
6 files changed, 10 insertions(+), 31 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index c4ee10ebc3ff..eccb116d3cfa 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -115,9 +115,7 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}
- . = ALIGN(8);
-
- .efi_runtime_rel : {
+ .efi_runtime_rel ALIGN(8) : {
__efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 90d329b1ebe0..70e78ce46672 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -152,21 +152,11 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}
- . = ALIGN(4);
-
- .efi_runtime_rel_start :
- {
- *(.__efi_runtime_rel_start)
- }
-
- .efi_runtime_rel : {
+ .efi_runtime_rel ALIGN(4) : {
+ __efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
- }
-
- .efi_runtime_rel_stop :
- {
- *(.__efi_runtime_rel_stop)
+ __efi_runtime_rel_stop = .;
}
. = ALIGN(4);
diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
index 8e8bd5797e16..ddfde52163fc 100644
--- a/arch/arm/lib/sections.c
+++ b/arch/arm/lib/sections.c
@@ -29,6 +29,4 @@ char __secure_stack_start[0]
__section(".__secure_stack_start");
char __secure_stack_end[0] __section(".__secure_stack_end");
char __efi_runtime_start[0] __section(".__efi_runtime_start");
char __efi_runtime_stop[0] __section(".__efi_runtime_stop");
-char __efi_runtime_rel_start[0] __section(".__efi_runtime_rel_start");
-char __efi_runtime_rel_stop[0] __section(".__efi_runtime_rel_stop");
char _end[0] __section(".__end");
diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
index 8d3259821719..66a9e37f9198 100644
--- a/arch/arm/mach-zynq/u-boot.lds
+++ b/arch/arm/mach-zynq/u-boot.lds
@@ -58,21 +58,11 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}
- . = ALIGN(4);
-
- .efi_runtime_rel_start :
- {
- *(.__efi_runtime_rel_start)
- }
-
- .efi_runtime_rel : {
+ .efi_runtime_rel ALIGN(4) : {
+ __efi_runtime_rel_start = .;
*(.rel*.efi_runtime)
*(.rel*.efi_runtime.*)
- }
-
- .efi_runtime_rel_stop :
- {
- *(.__efi_runtime_rel_stop)
+ __efi_runtime_rel_stop = .;
}
. = ALIGN(8);
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 1e1657a01673..60949200dd93 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -34,6 +34,8 @@ extern char __priv_data_start[], __priv_data_end[];
/* Start and end of .ctors section - used for constructor calls. */
extern char __ctors_start[], __ctors_end[];
+extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[];
+
/* function descriptor handling (if any). Override
* in asm/sections.h */
#ifndef dereference_function_descriptor
diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index 18da6892e796..9185f1894c47 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
/* For manual relocation support */
DECLARE_GLOBAL_DATA_PTR;
Re: [PATCH 2/6] arm: clean up v7 and v8 linker scripts for bss_start/end
On 3/4/24 02:01, Ilias Apalodimas wrote:
commit 3ebd1cbc49f0 ("arm: make __bss_start and __bss_end__ compiler-generated")
and
commit f84a7b8f54db ("ARM: Fix __bss_start and __bss_end in linker scripts")
were moving the bss_start/end on c generated variables that were
injected in their own sections. The reason was that we needed relative
relocations for position independent code and linker bugs back then
prevented us from doing so.
However, the linker documentation pages states that symbols that are
defined within a section definition will create a relocatable
type with the value being a fixed offset from the base of a section.
This have been fixed since 2016 [1]. So let's start cleaning this up
starting with the bss_start and bss_end variables. Convert them into
symbols within the .bss section definition.
[0] https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_mono/ld.html#SEC13
[1] commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only for shared
object")
Signed-off-by: Ilias Apalodimas
Tested-by: Caleb Connolly # Qualcomm sdm845
---
arch/arm/cpu/armv8/u-boot-spl.lds| 14 +++---
arch/arm/cpu/armv8/u-boot.lds| 15 +++
arch/arm/cpu/u-boot.lds | 22 --
arch/arm/lib/sections.c | 2 --
arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 14 +++---
arch/arm/mach-zynq/u-boot.lds| 21 +++--
6 files changed, 16 insertions(+), 72 deletions(-)
diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds
b/arch/arm/cpu/armv8/u-boot-spl.lds
index 7cb9d731246d..16fddb46e9cb 100644
--- a/arch/arm/cpu/armv8/u-boot-spl.lds
+++ b/arch/arm/cpu/armv8/u-boot-spl.lds
@@ -63,18 +63,10 @@ SECTIONS
_image_binary_end = .;
- .bss_start (NOLOAD) : {
- . = ALIGN(8);
- KEEP(*(.__bss_start));
- } >.sdram
-
- .bss (NOLOAD) : {
+ .bss : {
+ __bss_start = .;
*(.bss*)
-. = ALIGN(8);
- } >.sdram
-
- .bss_end (NOLOAD) : {
- KEEP(*(.__bss_end));
+ __bss_end = .;
} >.sdram
/DISCARD/ : { *(.rela*) }
diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
index fb6a30c922f7..c4ee10ebc3ff 100644
--- a/arch/arm/cpu/armv8/u-boot.lds
+++ b/arch/arm/cpu/armv8/u-boot.lds
@@ -149,19 +149,10 @@ SECTIONS
_end = .;
- . = ALIGN(8);
-
- .bss_start : {
- KEEP(*(.__bss_start));
- }
-
- .bss : {
+ .bss ALIGN(8): {
+ __bss_start = .;
*(.bss*)
-. = ALIGN(8);
- }
-
- .bss_end : {
- KEEP(*(.__bss_end));
+ __bss_end = .;
}
/DISCARD/ : { *(.dynsym) }
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 7724c9332c3b..90d329b1ebe0 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -206,27 +206,13 @@ SECTIONS
*(.mmutable)
}
-/*
- * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c
- * __bss_base and __bss_limit are for linker only (overlay ordering)
- */
-
- .bss_start __rel_dyn_start (OVERLAY) : {
- KEEP(*(.__bss_start));
- __bss_base = .;
- }
-
- .bss __bss_base (OVERLAY) : {
+ .bss ALIGN(4): {
Hi Ilias,
I have been reviewing this patchset by diffing output binaries and
linker maps between successive patches. Most of the patches in this
series result in no change to the output binary whatsoever (which also
means, no regressions!) but this one does have a change: .bss is being
moved after .mmutable. You should be able to see this yourself by
comparing `u-boot.map` after a successful build, before and after
applying this patch.
Since the current u-boot.lds puts .bss right after __image_copy_end
(which makes sense) and uses OVERLAY to overlap .rel.dyn (which... I
guess makes sense, if U-Boot zero-initializes .bss after applying
relocations), perhaps this patch should be moving the .bss section up
there in the .lds, putting (OVERLAY) back, and adding a comment to the
effect of "These sections occupy the same memory, but their lifetimes do
not overlap: U-Boot initializes .bss only after applying relocations and
therefore after it doesn't need .rel.dyn any more."
We might also decide that the overlay memory-saving trick isn't actually
all that important anymore and that .bss should have a new home. Someone
far more seasoned than I should be the one to make that decision though.
The rest of the patchset looks great! I'll add my tags to those in a bit.
Cheers,
Sam
+ __bss_start = .;
*(.bss*)
-. = ALIGN(4);
-__bss_limit = .;
- }
-
- .bss_end __bss_limit (OVERLAY) : {
- KEEP(*(.__bss_end));
+ __bss_end = .;
}
- .dynsym _image_binary_end : { *(.dynsym) }
+
Re: [RFC PATCH 0/6] Clean up arm linker scripts
On 2/28/24 04:15, Ilias Apalodimas wrote:
On Wed, 28 Feb 2024 at 13:11, Peter Robinson wrote:
On Wed, 28 Feb 2024 at 10:58, Ilias Apalodimas
wrote:
The arm linker scripts had a mix of symbols and C defined variables in an
effort to emit relative references instead of absolute ones e.g [0].
This has led to confusion over the years, ending up with mixed section
definitions. Some sections being defined with overlays and different
definitions between v7 and v8 architectures.
For example __efi_runtime_rel_start/end is defined as a linker symbol for
armv8 and a C variable in armv7.
I am not sure if this used to be a compiler bug, but linker scripts nowadays
can emit relative references, as long as the symbol definition is contained
within the section definition. So let's switch most of the C defined variables
Should we be setting/upping the minimum version of the linker version
as part of this?
The patch that added those as C-defined variables, was in 2013. So any
linker that's not ancient history should emit those correctly.
GNU ld fixed the problem on 2016-02-23, with this entry in the bfd
changelog:
* elflink.c (bfd_elf_record_link_assignment): Check for shared
library, instead of PIC, and don't check PDE when making linker
assigned symbol dynamic.
It looks like the change was first included in binutils 2.27, released
2016-08-03. I don't know where the minimum linker version requirement is
memorialized but there's a good chance that U-Boot already requires a
version more recent than that. (Someone who knows where that is will
have to check.)
In either case, I strongly agree: sections.c is an unnecessary
workaround with any reasonably recent linker version, it's
fickle/incompatible with non-GNU linkers such as LLD, and the marker
symbols belong in the linker script. This patchset is a big step in the
right direction!
and clean up the arm sections.c file. There's still a few symbols remaining --
__secure_start/end, __secure_stack_start/end and __end which can be cleaned up
in a followup series.
The resulting binary (tested in QEMU v7/v8) had no size differences apart from
the emited sections and object types of those variables. I've also added prints
throughout the U-Boot init sequence. The offsets and delta for 'end - start'
section sizes is unchanged.
For example on QEMU v8:
$~ ./scripts/bloat-o-meter u-boot u-boot.new
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
Function old new delta
Total: Before=798861, After=798861, chg +0.00%
$~ readelf -sW u-boot | grep bss_s
12: 001029b8 0 SECTION LOCAL DEFAULT 12 .bss_start
8088: 0018 0 NOTYPE GLOBAL DEFAULT1 _bss_start_ofs
8376: 001029b8 0 OBJECT GLOBAL DEFAULT 12 __bss_start
$~ readelf -sW u-boot.new | grep bss_s
8085: 0018 0 NOTYPE GLOBAL DEFAULT1 _bss_start_ofs
8373: 001029b8 0 NOTYPE GLOBAL DEFAULT 12 __bss_start
For QEMU v7 the differences are a bit bigger but only affect the variables
placed in the .bss section because that was defined as an OVERLAY in the
existing linker script.
For example:
$~ nm u-boot | grep tftp_prev_block
000ca0dc ? tftp_prev_block
$~ nm u-boot.new | grep tftp_prev_block
000e0a5c b tftp_prev_block -> The symbol is now placed into .bss
It's worth noting that since the efi regions are affected by the change, booting
with EFI is preferable while testing. Booting the kernel only should be enough
since the efi stub and the kernel proper do request boottime and runtime
services respectively.
Something along the lines of
virtio scan && load virtio 0 $kernel_addr_r Image && bootefi $kernel_addr_r
will work for QEMU aarch64.
Tested platforms:
- QEMU aarch64
- Xilinx kv260 kria starter kit & zynq
- QEMU armv7
- STM32MP157C-DK2
[0] commit 3ebd1cbc49f0 ("arm: make __bss_start and __bss_end__
compiler-generated")
Ilias Apalodimas (6):
arm: baltos: remove custom linker script
arm: clean up v7 and v8 linker scripts for bss_start/end
arm: fix __efi_runtime_rel_start/end definitions
arm: clean up v7 and v8 linker scripts for __rel_dyn_start/end
arm: fix __efi_runtime_start/end definitions
arm: move image_copy_start/end to linker symbols
arch/arm/cpu/armv8/u-boot-spl.lds | 14 +--
arch/arm/cpu/armv8/u-boot.lds | 45 ++--
arch/arm/cpu/u-boot.lds | 74 +++--
arch/arm/lib/sections.c | 10 --
arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 22 +---
arch/arm/mach-zynq/u-boot.lds | 72 +++-
board/qualcomm/dragonboard820c/u-boot.lds | 41 ++-
board/vscom/baltos/u-boot.lds | 128 --
include/asm-generic/sections.h| 3 +
lib/efi_loader/efi_runtime.c | 1 +
10 files changed, 58 insertions(+), 352 deletions(-)
delete mode 100644
Re: [PATCH v2] board: rockchip: Add the Turing RK1 SoM
Hi Joshua, I just updated my own modules to this version of the patch and all continues to be well. On 12/14/23 16:46, Joshua Riek wrote: Signed-off-by: Joshua Riek Cc: Sam Edwards Tested-by: Sam Edwards Thanks for the continued efforts, my friend! Happy Friday, Sam
Re: [PATCH] fs: btrfs: fix reading when length specified
On 11/15/23 21:43, Qu Wenruo wrote: I'm not sure why this happend for the EFI environment. Doesn't the EFI runtime should also try to read the whole file? Or that EFI environment has specified the length to read instead? Hi Qu, The Linux EFISTUB file loading routine (handle_cmdline_files) reads files in chunks, and not (necessarily) the whole file in one go. On x86 platforms, the chunk size is 1MB, and the comments explain that this is to work around firmware implementations that have problems with large reads. On non-x86 platforms, the chunk size is ULONG_MAX -- which in practice means it reads the whole file in one exact-filesize chunk. So, to answer your questions: "either/both depending on platform." (The bug in the U-Boot implementation doubtlessly affects more than just EFI; I only happened to discover it while trying to use EFI.) Signed-off-by: Sam Edwards Anyway, the fix looks good to me. Reviewed-by: Qu Wenruo Thank you greatly for the review! Thanks, Qu Likewise, Sam
[PATCH] fs: btrfs: fix reading when length specified
The btrfs read function limits the read length to ensure that it
and the read offset do not together exceed the size of the file.
However, this size was only being queried if the read length was
passed a value of zero (meaning "whole file"), and the size is
defaulted to 0 otherwise. This means the clamp will just zero out
the length if one is specified, preventing reading of the file.
Fix this by checking the file size unconditionally, and unifying
the default length and clamping logic as a single range check instead.
This bug was discovered when trying to boot Linux with initrd= via
'bootefi' from a btrfs partition. The EFI stub entered an infinite
loop of zero-length reads while trying to read the initrd, and the
boot process stalled indefinitely.
Signed-off-by: Sam Edwards
---
fs/btrfs/btrfs.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/fs/btrfs/btrfs.c b/fs/btrfs/btrfs.c
index 4cdbbbe3d0..1149a3b200 100644
--- a/fs/btrfs/btrfs.c
+++ b/fs/btrfs/btrfs.c
@@ -228,7 +228,7 @@ int btrfs_read(const char *file, void *buf, loff_t offset,
loff_t len,
{
struct btrfs_fs_info *fs_info = current_fs_info;
struct btrfs_root *root;
- loff_t real_size = 0;
+ loff_t real_size;
u64 ino;
u8 type;
int ret;
@@ -246,16 +246,13 @@ int btrfs_read(const char *file, void *buf, loff_t
offset, loff_t len,
return -EINVAL;
}
- if (!len) {
- ret = btrfs_size(file, _size);
- if (ret < 0) {
- error("Failed to get inode size: %s", file);
- return ret;
- }
- len = real_size;
+ ret = btrfs_size(file, _size);
+ if (ret < 0) {
+ error("Failed to get inode size: %s", file);
+ return ret;
}
- if (len > real_size - offset)
+ if (!len || len > real_size - offset)
len = real_size - offset;
ret = btrfs_file_read(root, ino, offset, len, buf);
--
2.41.0
[PATCH v4 4/4] sunxi: psci: implement PSCI on R528
This patch adds the necessary code to make nonsec booting and PSCI
secondary core management functional on the R528/T113.
Signed-off-by: Sam Edwards
Tested-by: Maksim Kiselev
Tested-by: Kevin Amadiva
---
arch/arm/cpu/armv7/Kconfig | 3 ++-
arch/arm/cpu/armv7/sunxi/psci.c | 47 -
arch/arm/mach-sunxi/Kconfig | 4 +++
3 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv7/Kconfig b/arch/arm/cpu/armv7/Kconfig
index f015d133cb..4eb34b7b44 100644
--- a/arch/arm/cpu/armv7/Kconfig
+++ b/arch/arm/cpu/armv7/Kconfig
@@ -61,8 +61,9 @@ config ARMV7_SECURE_MAX_SIZE
config ARM_GIC_BASE_ADDRESS
hex
depends on ARMV7_NONSEC
- depends on ARCH_EXYNOS5
+ depends on ARCH_EXYNOS5 || MACH_SUN8I_R528
default 0x1048 if ARCH_EXYNOS5
+ default 0x0302 if MACH_SUN8I_R528
help
Override the GIC base address if the Arm Cortex defined
CBAR/PERIPHBASE system register holds the wrong value.
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 207aa6bc4b..b03a865483 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -47,6 +47,19 @@
#define SUN8I_R40_PWR_CLAMP(cpu) (0x120 + (cpu) * 0x4)
#define SUN8I_R40_SRAMC_SOFT_ENTRY_REG0(0xbc)
+/*
+ * R528 is also different, as it has both cores powered up (but held in reset
+ * state) after the SoC is reset. Like the R40, it uses a "soft" entry point
+ * address register, but unlike the R40, it uses a newer "CPUX" block to manage
+ * CPU state, rather than the older CPUCFG system.
+ */
+#define SUN8I_R528_SOFT_ENTRY (0x1c8)
+#define SUN8I_R528_C0_RST_CTRL (0x)
+#define SUN8I_R528_C0_CTRL_REG0(0x0010)
+#define SUN8I_R528_C0_CPU_STATUS (0x0080)
+
+#define SUN8I_R528_C0_STATUS_STANDBYWFI(16)
+
static void __secure cp15_write_cntp_tval(u32 tval)
{
asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
@@ -103,10 +116,12 @@ static void __secure clamp_set(u32 *clamp)
static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
{
- /* secondary core entry address is programmed differently on R40 */
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
writel((u32)entry,
SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ writel((u32)entry,
+ SUNXI_R_CPUCFG_BASE + SUN8I_R528_SOFT_ENTRY);
} else {
writel((u32)entry, SUNXI_CPUCFG_BASE + SUNXI_PRIV0);
}
@@ -125,6 +140,9 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
clamp = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWR_CLAMP(cpu);
pwroff = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWROFF;
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ /* R528 leaves both cores powered up, manages them via reset */
+ return;
} else {
if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
IS_ENABLED(CONFIG_MACH_SUN8I_H3))
@@ -152,11 +170,27 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ if (reset)
+ clrbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_RST_CTRL,
+BIT(cpu));
+ else
+ setbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_RST_CTRL,
+BIT(cpu));
+
+ return;
+ }
+
writel(reset ? 0b00 : 0b11, SUNXI_CPUCFG_BASE + SUNXI_CPU_RST(cpu));
}
static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ /* Not required on R528 */
+ return;
+ }
+
if (lock)
clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
else
@@ -165,11 +199,22 @@ static void __secure sunxi_cpu_set_locking(int cpu, bool
lock)
static bool __secure sunxi_cpu_poll_wfi(int cpu)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ return !!(readl(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_CPU_STATUS) &
+ BIT(SUN8I_R528_C0_STATUS_STANDBYWFI + cpu));
+ }
+
return !!(readl(SUNXI_CPUCFG_BASE + SUNXI_CPU_STATUS(cpu)) & BIT(2));
}
static void __secure sunxi_cpu_invalidate_cache(int cpu)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ clrbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_CTRL_REG0,
+BIT(cpu));
+ return;
+ }
+
clrbits_
[PATCH v4 2/4] sunxi: psci: refactor register access to separate functions
This is to prepare for R528, which does not have the typical
"CPUCFG" block; it has a "CPUX" block which provides these
same functions but is organized differently.
Moving the hardware-access bits to their own functions separates the
logic from the hardware so we can reuse the same logic.
Signed-off-by: Sam Edwards
Reviewed-by: Andre Przywara
---
arch/arm/cpu/armv7/sunxi/psci.c | 66 +++--
1 file changed, 47 insertions(+), 19 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 69fa3f3c2e..27ca9c39e1 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -92,7 +92,7 @@ static void __secure clamp_set(u32 *clamp)
writel(0xff, clamp);
}
-static void __secure sunxi_set_entry_address(void *entry)
+static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
{
/* secondary core entry address is programmed differently on R40 */
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
@@ -149,30 +149,60 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
}
}
-void __secure sunxi_cpu_power_off(u32 cpuid)
+static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
{
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ writel(reset ? 0b00 : 0b11, >cpu[cpu].rst);
+}
+
+static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ if (lock)
+ clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ else
+ setbits_le32(>dbg_ctrl1, BIT(cpu));
+}
+
+static bool __secure sunxi_cpu_poll_wfi(int cpu)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ return !!(readl(>cpu[cpu].status) & BIT(2));
+}
+
+static void __secure sunxi_cpu_invalidate_cache(int cpu)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ clrbits_le32(>gen_ctrl, BIT(cpu));
+}
+
+static void __secure sunxi_cpu_power_off(u32 cpuid)
+{
u32 cpu = cpuid & 0x3;
/* Wait for the core to enter WFI */
- while (1) {
- if (readl(>cpu[cpu].status) & BIT(2))
- break;
+ while (!sunxi_cpu_poll_wfi(cpu))
__mdelay(1);
- }
/* Assert reset on target CPU */
- writel(0, >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, true);
/* Lock CPU (Disable external debug access) */
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ sunxi_cpu_set_locking(cpu, true);
/* Power down CPU */
sunxi_cpu_set_power(cpuid, false);
- /* Unlock CPU (Disable external debug access) */
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ /* Unlock CPU (Reenable external debug access) */
+ sunxi_cpu_set_locking(cpu, false);
}
static u32 __secure cp15_read_scr(void)
@@ -229,33 +259,31 @@ out:
int __secure psci_cpu_on(u32 __always_unused unused, u32 mpidr, u32 pc,
u32 context_id)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
u32 cpu = (mpidr & 0x3);
/* store target PC and context id */
psci_save(cpu, pc, context_id);
/* Set secondary core power on PC */
- sunxi_set_entry_address(_cpu_entry);
+ sunxi_cpu_set_entry(cpu, _cpu_entry);
/* Assert reset on target CPU */
- writel(0, >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, true);
/* Invalidate L1 cache */
- clrbits_le32(>gen_ctrl, BIT(cpu));
+ sunxi_cpu_invalidate_cache(cpu);
/* Lock CPU (Disable external debug access) */
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ sunxi_cpu_set_locking(cpu, true);
/* Power up target CPU */
sunxi_cpu_set_power(cpu, true);
/* De-assert reset on target CPU */
- writel(BIT(1) | BIT(0), >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, false);
- /* Unlock CPU (Disable external debug access) */
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ /* Unlock CPU (Reenable external debug access) */
+ sunxi_cpu_set_locking(cpu, false);
return ARM_PSCI_RET_SUCCESS;
}
--
2.41.0
[PATCH v4 3/4] sunxi: psci: stop modeling register layout with C structs
Since the sunxi support nowadays generally prefers #defined register
offsets instead of modeling register layouts using C structs, now is a
good time to do this for PSCI as well. This patch moves away from using
the structs `sunxi_cpucfg_reg` and `sunxi_prcm_reg` in psci.c.
The former struct and its associated header file existed only to support
PSCI code, so also delete them altogether.
Signed-off-by: Sam Edwards
---
arch/arm/cpu/armv7/sunxi/psci.c | 57
arch/arm/include/asm/arch-sunxi/cpucfg.h | 67
2 files changed, 23 insertions(+), 101 deletions(-)
delete mode 100644 arch/arm/include/asm/arch-sunxi/cpucfg.h
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 27ca9c39e1..207aa6bc4b 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -11,8 +11,6 @@
#include
#include
-#include
-#include
#include
#include
#include
@@ -27,6 +25,17 @@
#defineGICD_BASE (SUNXI_GIC400_BASE + GIC_DIST_OFFSET)
#defineGICC_BASE (SUNXI_GIC400_BASE + GIC_CPU_OFFSET_A15)
+/*
+ * Offsets into the CPUCFG block applicable to most SUNXIs.
+ */
+#define SUNXI_CPU_RST(cpu) (0x40 + (cpu) * 0x40 + 0x0)
+#define SUNXI_CPU_STATUS(cpu) (0x40 + (cpu) * 0x40 + 0x8)
+#define SUNXI_GEN_CTRL (0x184)
+#define SUNXI_PRIV0(0x1a4)
+#define SUN7I_CPU1_PWR_CLAMP (0x1b0)
+#define SUN7I_CPU1_PWROFF (0x1b4)
+#define SUNXI_DBG_CTRL1(0x1e4)
+
/*
* R40 is different from other single cluster SoCs.
*
@@ -99,10 +108,7 @@ static void __secure sunxi_cpu_set_entry(int
__always_unused cpu, void *entry)
writel((u32)entry,
SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
} else {
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- writel((u32)entry, >priv0);
+ writel((u32)entry, SUNXI_CPUCFG_BASE + SUNXI_PRIV0);
}
}
@@ -110,26 +116,21 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
{
u32 *clamp = NULL;
u32 *pwroff;
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
/* sun7i (A20) is different from other single cluster SoCs */
if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
- clamp = >cpu1_pwr_clamp;
- pwroff = >cpu1_pwroff;
+ clamp = (void *)SUNXI_CPUCFG_BASE + SUN7I_CPU1_PWR_CLAMP;
+ pwroff = (void *)SUNXI_CPUCFG_BASE + SUN7I_CPU1_PWROFF;
cpu = 0;
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
- clamp = (void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu);
- pwroff = (void *)cpucfg + SUN8I_R40_PWROFF;
+ clamp = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWR_CLAMP(cpu);
+ pwroff = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWROFF;
} else {
- struct sunxi_prcm_reg *prcm =
- (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
-
if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
IS_ENABLED(CONFIG_MACH_SUN8I_H3))
- clamp = >cpu_pwr_clamp[cpu];
+ clamp = (void *)SUNXI_PRCM_BASE + 0x140 + cpu * 0x4;
- pwroff = >cpu_pwroff;
+ pwroff = (void *)SUNXI_PRCM_BASE + 0x100;
}
if (on) {
@@ -151,37 +152,25 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- writel(reset ? 0b00 : 0b11, >cpu[cpu].rst);
+ writel(reset ? 0b00 : 0b11, SUNXI_CPUCFG_BASE + SUNXI_CPU_RST(cpu));
}
static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
if (lock)
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
else
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ setbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
}
static bool __secure sunxi_cpu_poll_wfi(int cpu)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- return !!(readl(>cpu[cpu].status) & BIT(2));
+ return !!(readl(SUNXI_CPUCFG_BASE + SUNXI_CPU_STATUS(cpu)) & BIT(2));
}
static void __secure sunxi_cpu_invalidate_cache(int cpu)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- clrbits_le32(>gen
[PATCH v4 1/4] sunxi: psci: clean away preprocessor macros
This patch restructures psci.c to get away from the "many different
function definitions switched by #ifdef" paradigm to the preferred style
of having a single function definition with `if (IS_ENABLED(...))` to
make the optimizer include only the appropriate function bodies instead.
There are no functional changes here.
Signed-off-by: Sam Edwards
Reviewed-by: Andre Przywara
---
arch/arm/cpu/armv7/sunxi/psci.c | 103 +---
1 file changed, 43 insertions(+), 60 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index e1d3638b5c..69fa3f3c2e 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -76,11 +76,8 @@ static void __secure __mdelay(u32 ms)
isb();
}
-static void __secure clamp_release(u32 __maybe_unused *clamp)
+static void __secure clamp_release(u32 *clamp)
{
-#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
- defined(CONFIG_MACH_SUN8I_H3) || \
- defined(CONFIG_MACH_SUN8I_R40)
u32 tmp = 0x1ff;
do {
tmp >>= 1;
@@ -88,83 +85,69 @@ static void __secure clamp_release(u32 __maybe_unused
*clamp)
} while (tmp);
__mdelay(10);
-#endif
}
-static void __secure clamp_set(u32 __maybe_unused *clamp)
+static void __secure clamp_set(u32 *clamp)
{
-#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
- defined(CONFIG_MACH_SUN8I_H3) || \
- defined(CONFIG_MACH_SUN8I_R40)
writel(0xff, clamp);
-#endif
}
-static void __secure sunxi_power_switch(u32 *clamp, u32 *pwroff, bool on,
- int cpu)
+static void __secure sunxi_set_entry_address(void *entry)
{
- if (on) {
- /* Release power clamp */
- clamp_release(clamp);
-
- /* Clear power gating */
- clrbits_le32(pwroff, BIT(cpu));
+ /* secondary core entry address is programmed differently on R40 */
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
+ writel((u32)entry,
+ SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
} else {
- /* Set power gating */
- setbits_le32(pwroff, BIT(cpu));
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
- /* Activate power clamp */
- clamp_set(clamp);
+ writel((u32)entry, >priv0);
}
}
-#ifdef CONFIG_MACH_SUN8I_R40
-/* secondary core entry address is programmed differently on R40 */
-static void __secure sunxi_set_entry_address(void *entry)
-{
- writel((u32)entry,
- SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
-}
-#else
-static void __secure sunxi_set_entry_address(void *entry)
+static void __secure sunxi_cpu_set_power(int cpu, bool on)
{
+ u32 *clamp = NULL;
+ u32 *pwroff;
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
- writel((u32)entry, >priv0);
-}
-#endif
+ /* sun7i (A20) is different from other single cluster SoCs */
+ if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
+ clamp = >cpu1_pwr_clamp;
+ pwroff = >cpu1_pwroff;
+ cpu = 0;
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
+ clamp = (void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu);
+ pwroff = (void *)cpucfg + SUN8I_R40_PWROFF;
+ } else {
+ struct sunxi_prcm_reg *prcm =
+ (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
-#ifdef CONFIG_MACH_SUN7I
-/* sun7i (A20) is different from other single cluster SoCs */
-static void __secure sunxi_cpu_set_power(int __always_unused cpu, bool on)
-{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
+ IS_ENABLED(CONFIG_MACH_SUN8I_H3))
+ clamp = >cpu_pwr_clamp[cpu];
- sunxi_power_switch(>cpu1_pwr_clamp, >cpu1_pwroff,
- on, 0);
-}
-#elif defined CONFIG_MACH_SUN8I_R40
-static void __secure sunxi_cpu_set_power(int cpu, bool on)
-{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ pwroff = >cpu_pwroff;
+ }
- sunxi_power_switch((void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu),
- (void *)cpucfg + SUN8I_R40_PWROFF,
- on, cpu);
-}
-#else /* ! CONFIG_MACH_SUN7I && ! CONFIG_MACH_SUN8I_R40 */
-static void __secure sunxi_cpu_set_power(int cpu, bool on)
-{
- struct sunxi_prcm_reg *prcm =
- (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
+ if (on) {
+ /* Release power clamp */
+ if (clamp)
+ clamp_release(clamp);
[PATCH v4 0/4] Allwinner R528/T113s PSCI
Hi list,
This is the third, and hopefully final, version of my patchset for PSCI support
on R528/T113-s3. This one, as always, depends on Andre Przywara's T113s support
series (v2), available on the list and also located on a Git branch at:
https://source.denx.de/u-boot/custodians/u-boot-sunxi.git t113s-v2
NOTE THAT THIS ALSO depends on the following commit on master:
3d5e52bd97 ("ARM: psci: move GIC address override to Kconfig")
For testing: I can confirm that patch 2/4 results in no change to machine code
whatsoever on any supported target. Patch 1/4 results in a minor machine code
change on R40 only. Patch 3/4 will likely require testing on each of the 4
"special case" sunxi targets (sun6i, sun7i, R40, H3) to ensure that register
offsets are kept consistent. Patch 4/4 needs testing on R528 only.
Warm regards,
Sam
Changes v3->v4:
- The GIC base address override is done through Kconfig, instead of in
sunxi-common.h
Changes v2->v3:
- Fix missing `cpu=0;` for the sun7i power management case.
- Make sunxi_cpu_power_off() a static function, per feedback on v2.
- Kevin Amadiva of MEC electronics got in touch with me off-list, reported
success bringing up CPU1 of a T113 with this patchset, and kindly provided
me with a Tested-by tag for patch 4/4.
- Remove a comment about R40/R528 being special, per feedback on v2.
- Simplify an if statement, per feedback on v2.
- Add missing 'select' directives to the R528 Kconfig, per feedback on v2.
Changes v1->v2:
- Power clamp is now adjusted ONLY on sun{6,7}i, H3, R40. The previous version
was mistakenly doing this EXCEPT on those machines.
- Flattened sunxi_power_switch() into sunxi_cpu_set_power() for simplicity's
sake.
- Moved the "power clamp is not NULL" conditional into sunxi_cpu_set_power().
- Removed unnecessary H6 special-case, since H6 is actually ARM64.
- Renamed SUNXI_CPUX_BASE to SUNXI_CPUCFG_BASE, to mirror expected changes in
Andre's v2 of the R528 series (we decided against using a new name for this
block).
- Removed sunxi_cpucfg_reg struct, and stopped using the PRCM struct in psci.c.
Sam Edwards (4):
sunxi: psci: clean away preprocessor macros
sunxi: psci: refactor register access to separate functions
sunxi: psci: stop modeling register layout with C structs
sunxi: psci: implement PSCI on R528
arch/arm/cpu/armv7/Kconfig | 3 +-
arch/arm/cpu/armv7/sunxi/psci.c | 185 ++-
arch/arm/include/asm/arch-sunxi/cpucfg.h | 67
arch/arm/mach-sunxi/Kconfig | 4 +
4 files changed, 121 insertions(+), 138 deletions(-)
delete mode 100644 arch/arm/include/asm/arch-sunxi/cpucfg.h
--
2.41.0
Re: [PATCH] ARM: psci: move GIC address override to Kconfig
On 10/1/23 16:52, Andre Przywara wrote: As the code to switch an ARM core from secure to the non-secure state needs to know the base address of the Generic Interrupt Controller (GIC), we read an Arm Cortex defined system register that is supposed to hold that base address. However there are SoCs out there that get this wrong, and this CBAR register either reads as 0 or points to the wrong address. To accommodate those systems, so far we use a macro defined in some platform specific header files, for affected boards. To simplify future extensions, replace that macro with a Kconfig variable that holds this override address, and define a default value for SoCs that need it. Hi Andre, Looks great to me. I'll update my PSCI series atop this once either this or the R528 series lands (I don't want my series to depend on *two* pending patchsets). Signed-off-by: Andre Przywara Reviewed-by: Sam Edwards Cheers, Sam
[RFC PATCH v2 4/4] HACK: enable access to `ubi 0:volname` block devices
---
disk/part.c | 55 +
1 file changed, 55 insertions(+)
diff --git a/disk/part.c b/disk/part.c
index a4b6d265da..7c995f583c 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -14,6 +14,9 @@
#include
#include
#include
+#include
+#include
+#include
#undef PART_DEBUG
@@ -524,6 +527,58 @@ int blk_get_device_part_str(const char *ifname, const char
*dev_part_str,
}
#endif
+#if IS_ENABLED(CONFIG_CMD_UBI) && !IS_ENABLED(CONFIG_SPL_BUILD)
+ /*
+* Also special-case UBI, which may use names to find the specific
+* volumes, so this deviates a bit from the typical devnum:partnum
+* syntax.
+*/
+ if (!strcmp(ifname, "ubi")) {
+ dev = dectoul(dev_part_str, );
+ if (*ep == ':') {
+ struct udevice *ubi_dev = NULL;
+ struct udevice *vol_dev = NULL;
+ part_str = [1];
+
+ ret = uclass_find_device(UCLASS_UBI, dev, _dev);
+ if (!ubi_dev || ret) {
+ printf("** Cannot find UBI %x\n", dev);
+ return -EINVAL;
+ }
+
+ part = dectoul(part_str, );
+ if (!*ep) {
+ struct udevice *tmp_dev;
+ device_foreach_child(tmp_dev, ubi_dev) {
+ struct blk_desc *desc =
dev_get_uclass_plat(tmp_dev);
+ if (desc->devnum == part) {
+ vol_dev = tmp_dev;
+ break;
+ }
+ }
+ } else {
+ ret = device_find_child_by_name(ubi_dev,
part_str, _dev);
+ }
+
+ if (!vol_dev || ret) {
+ printf("** UBI volume %s not found\n",
part_str);
+ return -EINVAL;
+ }
+
+ ret = device_probe(vol_dev);
+ if (ret)
+ return ret;
+
+ *desc = dev_get_uclass_plat(vol_dev);
+ part_get_info_whole_disk(*desc, info);
+ return 0;
+ }
+
+ printf("UBIFS not mounted, use ubifsmount to mount volume
first!\n");
+ return -EINVAL;
+ }
+#endif
+
/* If no dev_part_str, use bootdevice environment variable */
if (CONFIG_IS_ENABLED(ENV_SUPPORT)) {
if (!dev_part_str || !strlen(dev_part_str) ||
--
2.41.0
[RFC PATCH v2 3/4] disk: part: fall-through if "ubi" requested but ubifs not mounted
Since we're adding the ability to access static UBI volumes as block
devices, it is no longer an error to use the "ubi" ifname with UBIFS
unmounted.
Ideally, the access to UBIFS should instead be called "ubifs" but it
would break backwards compatibility to change this. Instead, use the
UBIFS mount status to disambiguate what the user means by "ubi"
There is no change in functionality in this patch: UBIFS access works
the same and an error still occurs when using "ubi" without UBIFS
mounted. The only difference is that now, the error message is a plain
"Bad device specification" and does not suggest using ubifsmount.
Signed-off-by: Sam Edwards
---
disk/part.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/disk/part.c b/disk/part.c
index 72241b7b23..a4b6d265da 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -511,15 +511,13 @@ int blk_get_device_part_str(const char *ifname, const
char *dev_part_str,
#if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD)
/*
-* Special-case ubi, ubi goes through a mtd, rather than through
-* a regular block device.
+* Special-case ubifs, which does not go through the block device layer
+* and also must be mounted ahead of time. U-Boot has historically
+* called this "ubi" too, even though *static* UBI volumes are
+* accessible as block devices. For backward compatibility, assume that
+* when UBIFS is mounted, the user intends "ubi" to mean "ubifs."
*/
- if (!strcmp(ifname, "ubi")) {
- if (!ubifs_is_mounted()) {
- printf("UBIFS not mounted, use ubifsmount to mount
volume first!\n");
- return -EINVAL;
- }
-
+ if (ubifs_is_mounted() && !strcmp(ifname, "ubi")) {
strcpy((char *)info->type, BOOT_PART_TYPE);
strcpy((char *)info->name, "UBI");
return 0;
--
2.41.0
[RFC PATCH v2 1/4] mtd: ubi: register UBI attachments as DM devices
This is in preparation for exposing static UBI volumes as block devices.
A UBI uclass and driver are introduced, and a "ubi0" virtual device
with the proper driver is created below whichever MTD device is
attached as the active UBI partition. This virtual device will soon
be the parent for the BLK devices that represent the static volumes.
Signed-off-by: Sam Edwards
---
cmd/ubi.c| 11 ++
drivers/mtd/ubi/Makefile | 1 +
drivers/mtd/ubi/ubi-uclass.c | 74
include/dm/uclass-id.h | 1 +
include/ubi_uboot.h | 5 +++
5 files changed, 92 insertions(+)
create mode 100644 drivers/mtd/ubi/ubi-uclass.c
diff --git a/cmd/ubi.c b/cmd/ubi.c
index 0a6a80bdd1..314c7f60f4 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -560,6 +560,13 @@ static int ubi_detach(void)
cmd_ubifs_umount();
#endif
+#ifdef CONFIG_DM_MTD
+ /*
+* Clean up any UBI devices in DM
+*/
+ ubi_dm_unbind_all();
+#endif
+
/*
* Call ubi_exit() before re-initializing the UBI subsystem
*/
@@ -598,6 +605,10 @@ int ubi_part(char *part_name, const char
*vid_header_offset)
return err;
}
+#ifdef CONFIG_DM_MTD
+ ubi_dm_bind(0);
+#endif
+
ubi = ubi_devices[0];
return 0;
diff --git a/drivers/mtd/ubi/Makefile b/drivers/mtd/ubi/Makefile
index 30d00fbdfe..375075f75e 100644
--- a/drivers/mtd/ubi/Makefile
+++ b/drivers/mtd/ubi/Makefile
@@ -7,3 +7,4 @@ obj-y += attach.o build.o vtbl.o vmt.o upd.o kapi.o eba.o io.o
wl.o crc32.o
obj-$(CONFIG_MTD_UBI_FASTMAP) += fastmap.o
obj-y += misc.o
obj-y += debug.o
+obj-$(CONFIG_DM_MTD) += ubi-uclass.o
diff --git a/drivers/mtd/ubi/ubi-uclass.c b/drivers/mtd/ubi/ubi-uclass.c
new file mode 100644
index 00..f8971e793e
--- /dev/null
+++ b/drivers/mtd/ubi/ubi-uclass.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * ubi-uclass.c - UBI partition and volume block device uclass driver
+ *
+ * Copyright (C) 2023 Sam Edwards
+ */
+
+#define LOG_CATEGORY UCLASS_UBI
+
+#include
+#include
+#include
+#include
+
+int ubi_dm_bind(unsigned int index)
+{
+ struct udevice *dev;
+ int ret;
+ char name[16];
+ const char *name_dup;
+ struct ubi_device *ubi = ubi_devices[index];
+ const struct mtd_info *mtd = ubi->mtd;
+
+ /* MTD partitions are not in DM; navigate to the real MTD device */
+ if (mtd->parent)
+ mtd = mtd->parent;
+
+ snprintf(name, sizeof(name), "ubi%u", index);
+ name_dup = strdup(name);
+ ret = device_bind(mtd->dev, DM_DRIVER_GET(ubi), name_dup, ubi,
+ ofnode_null(), );
+ if (ret) {
+ free((void *)name_dup);
+ return ret;
+ }
+
+ device_set_name_alloced(dev);
+
+ return 0;
+}
+
+int ubi_dm_unbind_all(void)
+{
+ int ret;
+ struct uclass *uc;
+ struct udevice *dev;
+ struct udevice *next;
+
+ ret = uclass_get(UCLASS_UBI, );
+ if (ret)
+ return ret;
+
+ uclass_foreach_dev_safe(dev, next, uc) {
+ ret = device_remove(dev, DM_REMOVE_NORMAL);
+ if (ret)
+ return ret;
+
+ ret = device_unbind(dev);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+U_BOOT_DRIVER(ubi) = {
+ .id = UCLASS_UBI,
+ .name = "ubi_dev",
+};
+
+UCLASS_DRIVER(ubi) = {
+ .id = UCLASS_UBI,
+ .name = "ubi",
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 0432c95c9e..ee11fbb38d 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -140,6 +140,7 @@ enum uclass_id {
UCLASS_THERMAL, /* Thermal sensor */
UCLASS_TIMER, /* Timer device */
UCLASS_TPM, /* Trusted Platform Module TIS interface */
+ UCLASS_UBI, /* Unsorted Block Images MTD partition */
UCLASS_UFS, /* Universal Flash Storage */
UCLASS_USB, /* USB bus */
UCLASS_USB_DEV_GENERIC, /* USB generic device */
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index 6da348eb62..9d37848f03 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -52,6 +52,11 @@ extern int ubi_part(char *part_name, const char
*vid_header_offset);
extern int ubi_volume_write(char *volume, void *buf, size_t size);
extern int ubi_volume_read(char *volume, char *buf, size_t size);
+#ifdef CONFIG_DM_MTD
+extern int ubi_dm_bind(unsigned int);
+extern int ubi_dm_unbind_all(void);
+#endif
+
extern struct ubi_device *ubi_devices[];
int cmd_ubifs_mount(char *vol_name);
int cmd_ubifs_umount(void);
--
2.41.0
[RFC PATCH v2 2/4] mtd: ubi: bind block device driver for static volumes
This makes static UBI volumes readable as block devices, however
no mechanism for selecting these volume devices yet exists.
Signed-off-by: Sam Edwards
---
drivers/mtd/ubi/ubi-uclass.c | 111 +++
1 file changed, 111 insertions(+)
diff --git a/drivers/mtd/ubi/ubi-uclass.c b/drivers/mtd/ubi/ubi-uclass.c
index f8971e793e..b2c47bfe0c 100644
--- a/drivers/mtd/ubi/ubi-uclass.c
+++ b/drivers/mtd/ubi/ubi-uclass.c
@@ -8,10 +8,120 @@
#define LOG_CATEGORY UCLASS_UBI
#include
+#include
#include
#include
#include
+static ulong ubi_bread(struct udevice *dev, lbaint_t lba, lbaint_t blkcnt,
+ void *dst)
+{
+ int err, lnum;
+ struct blk_desc *blk = dev_get_uclass_plat(dev);
+ struct ubi_device *ubi = dev_get_plat(dev->parent);
+ struct ubi_volume *vol = ubi->volumes[blk->devnum];
+ lbaint_t lba_per_peb = vol->usable_leb_size / blk->blksz;
+ lbaint_t lba_off, lba_len, total = 0;
+
+ while (blkcnt) {
+ lnum = lba / lba_per_peb;
+ lba_off = lba % lba_per_peb;
+ lba_len = lba_per_peb - lba_off;
+ if (lba_len > blkcnt)
+ lba_len = blkcnt;
+
+ err = ubi_eba_read_leb(ubi, vol, lnum, dst,
+ lba_off << blk->log2blksz,
+ lba_len << blk->log2blksz, 0);
+ if (err) {
+ pr_err("UBI read error %x\n", err);
+ break;
+ }
+
+ lba += lba_len;
+ blkcnt -= lba_len;
+ dst += lba_len << blk->log2blksz;
+ total += lba_len;
+ }
+
+ return total;
+}
+
+static const struct blk_ops ubi_block_ops = {
+ .read = ubi_bread,
+};
+
+U_BOOT_DRIVER(ubi_block) = {
+ .name = "ubi_block",
+ .id = UCLASS_BLK,
+ .ops= _block_ops,
+};
+
+static bool is_power_of_two(unsigned int x)
+{
+ return (x & -x) == x;
+}
+
+static unsigned int choose_blksz_for_volume(const struct ubi_volume *vol)
+{
+ /*
+* U-Boot assumes a power-of-two blksz; however, UBI LEBs are
+* very often not suitably sized. To solve this, we divide the
+* LEBs into a whole number of LBAs per LEB, such that each LBA
+* addresses a power-of-two-sized block. To choose the blksz,
+* we either:
+* 1) Use the volume alignment, if it's a non-unity power of
+*two. The LEB size is a multiple of this alignment, and it
+*allows the user to force a particular blksz if needed for
+*their use case.
+* 2) Otherwise, find the greatest power-of-two factor of the
+*LEB size.
+*/
+ if (vol->alignment > 1 && is_power_of_two(vol->alignment))
+ return vol->alignment;
+
+ unsigned int blksz = 1;
+ while ((vol->usable_leb_size & blksz) == 0)
+ blksz <<= 1;
+ return blksz;
+}
+
+static int ubi_post_bind(struct udevice *dev)
+{
+ int i;
+ int ret;
+ unsigned int blksz;
+ lbaint_t lba;
+ struct udevice *blkdev;
+ struct ubi_device *ubi = dev_get_plat(dev);
+
+ for (i = 0; i < ubi->vtbl_slots; i++) {
+ struct ubi_volume *vol = ubi->volumes[i];
+ if (!vol || vol->vol_id >= UBI_INTERNAL_VOL_START ||
+ vol->vol_type != UBI_STATIC_VOLUME)
+ continue;
+
+ if (vol->updating || vol->upd_marker) {
+ pr_err("** UBI volume %d (\"%s\") midupdate; ignored\n",
+ vol->vol_id, vol->name);
+ continue;
+ }
+
+ blksz = choose_blksz_for_volume(vol);
+ lba = DIV_ROUND_UP((unsigned long long)vol->used_bytes, blksz);
+
+ pr_debug("UBI volume %d (\"%s\"): %lu blocks, %d bytes each\n",
+vol->vol_id, vol->name, lba, blksz);
+
+ ret = blk_create_device(dev, "ubi_block", vol->name, UCLASS_UBI,
+ vol->vol_id, blksz, lba, );
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
int ubi_dm_bind(unsigned int index)
{
struct udevice *dev;
@@ -71,4 +181,5 @@ U_BOOT_DRIVER(ubi) = {
UCLASS_DRIVER(ubi) = {
.id = UCLASS_UBI,
.name = "ubi",
+ .post_bind = ubi_post_bind,
};
--
2.41.0
[RFC PATCH v2 0/4] mtd: ubi: Enable accessing RO filesystems in UBI vols
Hi list,
This is the second version of my RFC patchset to treat static UBI volumes as DM
block devices, allowing users to access read-only filesystems (SquashFS, EROFS)
contained within such volumes.
This is a rebased and updated version, as requested by Heiko.
Previously, we have agreed on a syntax, which my downstream is now starting to
use:
=> ls ubi 0:rootfs /boot
=> ls ubi 0:2 /boot
This is still not yet ready for mainline inclusion, because the actual UBI DM
access is happening in disk/part.c, which is not "clean" with the way the
DM/legacy switching is currently plumbed. I'm still looking for guidance on how
to name/implement block functions for looking up a *subvolume* block device by
type+parentidx+{name,ID}.
Changes v1->v2:
- Rebased onto next post-2023.10's release.
- Fix NULL dereference caused by passing NULL to `blk_create_device`
- Parse UBI index/volume numbers with `dectoul` instead of `hextoul`, to match
Linux's behavior of treating these numbers as decimal.
- Do not treat a valid decimal number as a volume name, even if the volume ID
doesn't exist, to match Linux's behavior of always treating decimal numbers
as volume IDs.
Cheers,
Sam
Sam Edwards (4):
mtd: ubi: register UBI attachments as DM devices
mtd: ubi: bind block device driver for static volumes
disk: part: fall-through if "ubi" requested but ubifs not mounted
HACK: enable access to `ubi 0:volname` block devices
cmd/ubi.c| 11 +++
disk/part.c | 69 +++--
drivers/mtd/ubi/Makefile | 1 +
drivers/mtd/ubi/ubi-uclass.c | 185 +++
include/dm/uclass-id.h | 1 +
include/ubi_uboot.h | 5 +
6 files changed, 264 insertions(+), 8 deletions(-)
create mode 100644 drivers/mtd/ubi/ubi-uclass.c
--
2.41.0
[PATCH v3 4/4] sunxi: psci: implement PSCI on R528
This patch adds the necessary code to make nonsec booting and PSCI
secondary core management functional on the R528/T113.
Signed-off-by: Sam Edwards
Tested-by: Maksim Kiselev
Tested-by: Kevin Amadiva
---
arch/arm/cpu/armv7/sunxi/psci.c | 47 -
arch/arm/mach-sunxi/Kconfig | 4 +++
include/configs/sunxi-common.h | 8 ++
3 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 207aa6bc4b..b03a865483 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -47,6 +47,19 @@
#define SUN8I_R40_PWR_CLAMP(cpu) (0x120 + (cpu) * 0x4)
#define SUN8I_R40_SRAMC_SOFT_ENTRY_REG0(0xbc)
+/*
+ * R528 is also different, as it has both cores powered up (but held in reset
+ * state) after the SoC is reset. Like the R40, it uses a "soft" entry point
+ * address register, but unlike the R40, it uses a newer "CPUX" block to manage
+ * CPU state, rather than the older CPUCFG system.
+ */
+#define SUN8I_R528_SOFT_ENTRY (0x1c8)
+#define SUN8I_R528_C0_RST_CTRL (0x)
+#define SUN8I_R528_C0_CTRL_REG0(0x0010)
+#define SUN8I_R528_C0_CPU_STATUS (0x0080)
+
+#define SUN8I_R528_C0_STATUS_STANDBYWFI(16)
+
static void __secure cp15_write_cntp_tval(u32 tval)
{
asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
@@ -103,10 +116,12 @@ static void __secure clamp_set(u32 *clamp)
static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
{
- /* secondary core entry address is programmed differently on R40 */
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
writel((u32)entry,
SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ writel((u32)entry,
+ SUNXI_R_CPUCFG_BASE + SUN8I_R528_SOFT_ENTRY);
} else {
writel((u32)entry, SUNXI_CPUCFG_BASE + SUNXI_PRIV0);
}
@@ -125,6 +140,9 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
clamp = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWR_CLAMP(cpu);
pwroff = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWROFF;
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ /* R528 leaves both cores powered up, manages them via reset */
+ return;
} else {
if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
IS_ENABLED(CONFIG_MACH_SUN8I_H3))
@@ -152,11 +170,27 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ if (reset)
+ clrbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_RST_CTRL,
+BIT(cpu));
+ else
+ setbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_RST_CTRL,
+BIT(cpu));
+
+ return;
+ }
+
writel(reset ? 0b00 : 0b11, SUNXI_CPUCFG_BASE + SUNXI_CPU_RST(cpu));
}
static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ /* Not required on R528 */
+ return;
+ }
+
if (lock)
clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
else
@@ -165,11 +199,22 @@ static void __secure sunxi_cpu_set_locking(int cpu, bool
lock)
static bool __secure sunxi_cpu_poll_wfi(int cpu)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ return !!(readl(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_CPU_STATUS) &
+ BIT(SUN8I_R528_C0_STATUS_STANDBYWFI + cpu));
+ }
+
return !!(readl(SUNXI_CPUCFG_BASE + SUNXI_CPU_STATUS(cpu)) & BIT(2));
}
static void __secure sunxi_cpu_invalidate_cache(int cpu)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ clrbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_CTRL_REG0,
+BIT(cpu));
+ return;
+ }
+
clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_GEN_CTRL, BIT(cpu));
}
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index e891e291f1..23b5b27b97 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -355,6 +355,10 @@ config MACH_SUN8I_R40
config MACH_SUN8I_R528
bool "sun8i (Allwinner R528)"
select CPU_V7A
+ select CPU_V7_HAS_NONSEC
+ select CPU_V7_HAS_VIRT
+ select ARCH_SUPPORT_PSCI
+ select SPL_ARMV7_SET_CORTEX_SMPEN
select SUNXI_GEN_NCAT2
select SUNXI_NEW_PINCTRL
select MMC_SUNXI_HAS_NEW_
[PATCH v3 3/4] sunxi: psci: stop modeling register layout with C structs
Since the sunxi support nowadays generally prefers #defined register
offsets instead of modeling register layouts using C structs, now is a
good time to do this for PSCI as well. This patch moves away from using
the structs `sunxi_cpucfg_reg` and `sunxi_prcm_reg` in psci.c.
The former struct and its associated header file existed only to support
PSCI code, so also delete them altogether.
Signed-off-by: Sam Edwards
---
arch/arm/cpu/armv7/sunxi/psci.c | 57
arch/arm/include/asm/arch-sunxi/cpucfg.h | 67
2 files changed, 23 insertions(+), 101 deletions(-)
delete mode 100644 arch/arm/include/asm/arch-sunxi/cpucfg.h
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 27ca9c39e1..207aa6bc4b 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -11,8 +11,6 @@
#include
#include
-#include
-#include
#include
#include
#include
@@ -27,6 +25,17 @@
#defineGICD_BASE (SUNXI_GIC400_BASE + GIC_DIST_OFFSET)
#defineGICC_BASE (SUNXI_GIC400_BASE + GIC_CPU_OFFSET_A15)
+/*
+ * Offsets into the CPUCFG block applicable to most SUNXIs.
+ */
+#define SUNXI_CPU_RST(cpu) (0x40 + (cpu) * 0x40 + 0x0)
+#define SUNXI_CPU_STATUS(cpu) (0x40 + (cpu) * 0x40 + 0x8)
+#define SUNXI_GEN_CTRL (0x184)
+#define SUNXI_PRIV0(0x1a4)
+#define SUN7I_CPU1_PWR_CLAMP (0x1b0)
+#define SUN7I_CPU1_PWROFF (0x1b4)
+#define SUNXI_DBG_CTRL1(0x1e4)
+
/*
* R40 is different from other single cluster SoCs.
*
@@ -99,10 +108,7 @@ static void __secure sunxi_cpu_set_entry(int
__always_unused cpu, void *entry)
writel((u32)entry,
SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
} else {
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- writel((u32)entry, >priv0);
+ writel((u32)entry, SUNXI_CPUCFG_BASE + SUNXI_PRIV0);
}
}
@@ -110,26 +116,21 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
{
u32 *clamp = NULL;
u32 *pwroff;
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
/* sun7i (A20) is different from other single cluster SoCs */
if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
- clamp = >cpu1_pwr_clamp;
- pwroff = >cpu1_pwroff;
+ clamp = (void *)SUNXI_CPUCFG_BASE + SUN7I_CPU1_PWR_CLAMP;
+ pwroff = (void *)SUNXI_CPUCFG_BASE + SUN7I_CPU1_PWROFF;
cpu = 0;
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
- clamp = (void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu);
- pwroff = (void *)cpucfg + SUN8I_R40_PWROFF;
+ clamp = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWR_CLAMP(cpu);
+ pwroff = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWROFF;
} else {
- struct sunxi_prcm_reg *prcm =
- (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
-
if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
IS_ENABLED(CONFIG_MACH_SUN8I_H3))
- clamp = >cpu_pwr_clamp[cpu];
+ clamp = (void *)SUNXI_PRCM_BASE + 0x140 + cpu * 0x4;
- pwroff = >cpu_pwroff;
+ pwroff = (void *)SUNXI_PRCM_BASE + 0x100;
}
if (on) {
@@ -151,37 +152,25 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- writel(reset ? 0b00 : 0b11, >cpu[cpu].rst);
+ writel(reset ? 0b00 : 0b11, SUNXI_CPUCFG_BASE + SUNXI_CPU_RST(cpu));
}
static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
if (lock)
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
else
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ setbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
}
static bool __secure sunxi_cpu_poll_wfi(int cpu)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- return !!(readl(>cpu[cpu].status) & BIT(2));
+ return !!(readl(SUNXI_CPUCFG_BASE + SUNXI_CPU_STATUS(cpu)) & BIT(2));
}
static void __secure sunxi_cpu_invalidate_cache(int cpu)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- clrbits_le32(>gen
[PATCH v3 2/4] sunxi: psci: refactor register access to separate functions
This is to prepare for R528, which does not have the typical
"CPUCFG" block; it has a "CPUX" block which provides these
same functions but is organized differently.
Moving the hardware-access bits to their own functions separates the
logic from the hardware so we can reuse the same logic.
Signed-off-by: Sam Edwards
Reviewed-by: Andre Przywara
---
arch/arm/cpu/armv7/sunxi/psci.c | 66 +++--
1 file changed, 47 insertions(+), 19 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 69fa3f3c2e..27ca9c39e1 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -92,7 +92,7 @@ static void __secure clamp_set(u32 *clamp)
writel(0xff, clamp);
}
-static void __secure sunxi_set_entry_address(void *entry)
+static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
{
/* secondary core entry address is programmed differently on R40 */
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
@@ -149,30 +149,60 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
}
}
-void __secure sunxi_cpu_power_off(u32 cpuid)
+static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
{
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ writel(reset ? 0b00 : 0b11, >cpu[cpu].rst);
+}
+
+static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ if (lock)
+ clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ else
+ setbits_le32(>dbg_ctrl1, BIT(cpu));
+}
+
+static bool __secure sunxi_cpu_poll_wfi(int cpu)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ return !!(readl(>cpu[cpu].status) & BIT(2));
+}
+
+static void __secure sunxi_cpu_invalidate_cache(int cpu)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ clrbits_le32(>gen_ctrl, BIT(cpu));
+}
+
+static void __secure sunxi_cpu_power_off(u32 cpuid)
+{
u32 cpu = cpuid & 0x3;
/* Wait for the core to enter WFI */
- while (1) {
- if (readl(>cpu[cpu].status) & BIT(2))
- break;
+ while (!sunxi_cpu_poll_wfi(cpu))
__mdelay(1);
- }
/* Assert reset on target CPU */
- writel(0, >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, true);
/* Lock CPU (Disable external debug access) */
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ sunxi_cpu_set_locking(cpu, true);
/* Power down CPU */
sunxi_cpu_set_power(cpuid, false);
- /* Unlock CPU (Disable external debug access) */
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ /* Unlock CPU (Reenable external debug access) */
+ sunxi_cpu_set_locking(cpu, false);
}
static u32 __secure cp15_read_scr(void)
@@ -229,33 +259,31 @@ out:
int __secure psci_cpu_on(u32 __always_unused unused, u32 mpidr, u32 pc,
u32 context_id)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
u32 cpu = (mpidr & 0x3);
/* store target PC and context id */
psci_save(cpu, pc, context_id);
/* Set secondary core power on PC */
- sunxi_set_entry_address(_cpu_entry);
+ sunxi_cpu_set_entry(cpu, _cpu_entry);
/* Assert reset on target CPU */
- writel(0, >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, true);
/* Invalidate L1 cache */
- clrbits_le32(>gen_ctrl, BIT(cpu));
+ sunxi_cpu_invalidate_cache(cpu);
/* Lock CPU (Disable external debug access) */
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ sunxi_cpu_set_locking(cpu, true);
/* Power up target CPU */
sunxi_cpu_set_power(cpu, true);
/* De-assert reset on target CPU */
- writel(BIT(1) | BIT(0), >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, false);
- /* Unlock CPU (Disable external debug access) */
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ /* Unlock CPU (Reenable external debug access) */
+ sunxi_cpu_set_locking(cpu, false);
return ARM_PSCI_RET_SUCCESS;
}
--
2.41.0
[PATCH v3 1/4] sunxi: psci: clean away preprocessor macros
This patch restructures psci.c to get away from the "many different
function definitions switched by #ifdef" paradigm to the preferred style
of having a single function definition with `if (IS_ENABLED(...))` to
make the optimizer include only the appropriate function bodies instead.
There are no functional changes here.
Signed-off-by: Sam Edwards
Reviewed-by: Andre Przywara
---
arch/arm/cpu/armv7/sunxi/psci.c | 103 +---
1 file changed, 43 insertions(+), 60 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index e1d3638b5c..69fa3f3c2e 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -76,11 +76,8 @@ static void __secure __mdelay(u32 ms)
isb();
}
-static void __secure clamp_release(u32 __maybe_unused *clamp)
+static void __secure clamp_release(u32 *clamp)
{
-#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
- defined(CONFIG_MACH_SUN8I_H3) || \
- defined(CONFIG_MACH_SUN8I_R40)
u32 tmp = 0x1ff;
do {
tmp >>= 1;
@@ -88,83 +85,69 @@ static void __secure clamp_release(u32 __maybe_unused
*clamp)
} while (tmp);
__mdelay(10);
-#endif
}
-static void __secure clamp_set(u32 __maybe_unused *clamp)
+static void __secure clamp_set(u32 *clamp)
{
-#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
- defined(CONFIG_MACH_SUN8I_H3) || \
- defined(CONFIG_MACH_SUN8I_R40)
writel(0xff, clamp);
-#endif
}
-static void __secure sunxi_power_switch(u32 *clamp, u32 *pwroff, bool on,
- int cpu)
+static void __secure sunxi_set_entry_address(void *entry)
{
- if (on) {
- /* Release power clamp */
- clamp_release(clamp);
-
- /* Clear power gating */
- clrbits_le32(pwroff, BIT(cpu));
+ /* secondary core entry address is programmed differently on R40 */
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
+ writel((u32)entry,
+ SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
} else {
- /* Set power gating */
- setbits_le32(pwroff, BIT(cpu));
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
- /* Activate power clamp */
- clamp_set(clamp);
+ writel((u32)entry, >priv0);
}
}
-#ifdef CONFIG_MACH_SUN8I_R40
-/* secondary core entry address is programmed differently on R40 */
-static void __secure sunxi_set_entry_address(void *entry)
-{
- writel((u32)entry,
- SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
-}
-#else
-static void __secure sunxi_set_entry_address(void *entry)
+static void __secure sunxi_cpu_set_power(int cpu, bool on)
{
+ u32 *clamp = NULL;
+ u32 *pwroff;
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
- writel((u32)entry, >priv0);
-}
-#endif
+ /* sun7i (A20) is different from other single cluster SoCs */
+ if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
+ clamp = >cpu1_pwr_clamp;
+ pwroff = >cpu1_pwroff;
+ cpu = 0;
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
+ clamp = (void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu);
+ pwroff = (void *)cpucfg + SUN8I_R40_PWROFF;
+ } else {
+ struct sunxi_prcm_reg *prcm =
+ (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
-#ifdef CONFIG_MACH_SUN7I
-/* sun7i (A20) is different from other single cluster SoCs */
-static void __secure sunxi_cpu_set_power(int __always_unused cpu, bool on)
-{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
+ IS_ENABLED(CONFIG_MACH_SUN8I_H3))
+ clamp = >cpu_pwr_clamp[cpu];
- sunxi_power_switch(>cpu1_pwr_clamp, >cpu1_pwroff,
- on, 0);
-}
-#elif defined CONFIG_MACH_SUN8I_R40
-static void __secure sunxi_cpu_set_power(int cpu, bool on)
-{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ pwroff = >cpu_pwroff;
+ }
- sunxi_power_switch((void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu),
- (void *)cpucfg + SUN8I_R40_PWROFF,
- on, cpu);
-}
-#else /* ! CONFIG_MACH_SUN7I && ! CONFIG_MACH_SUN8I_R40 */
-static void __secure sunxi_cpu_set_power(int cpu, bool on)
-{
- struct sunxi_prcm_reg *prcm =
- (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
+ if (on) {
+ /* Release power clamp */
+ if (clamp)
+ clamp_release(clamp);
[PATCH v3 0/4] Allwinner R528/T113s PSCI
Hi list,
This is the third version of my patchset for supporting PSCI on
R528/T113-s3. It is meant to be applied atop Andre Przywara's T113s support
series (v2). For convenience, the latter exists in a Git branch at:
https://source.denx.de/u-boot/custodians/u-boot-sunxi.git t113s-v2
It's looking like we're very close to finished here. The only "controversy"
that I expect is that patch 4/4 defines CFG_ARM_GIC_BASE_ADDRESS in
sunxi-common.h. There has previously been discussion about moving this to
Kconfig. I agree that this is a good change in principle, but I don't have the
available cycles to look into adding another Kconfig symbol (and cleaning up
Arndale, the other target which uses this) and so have opted to defer that
cleanup for another day.
For testing: I can confirm that patch 2/4 results in no change to machine code
whatsoever on any supported target. Patch 1/4 results in a minor machine code
change on R40 only. Patch 3/4 will likely require testing on each of the 4
"special case" sunxi targets (sun6i, sun7i, R40, H3) to ensure that register
offsets are kept consistent. Patch 4/4 needs testing on R528 only.
Changes v2->v3:
- Fix missing `cpu=0;` for the sun7i power management case.
- Make sunxi_cpu_power_off() a static function, per feedback on v2.
- Kevin Amadiva of MEC electronics got in touch with me off-list, reported
success bringing up CPU1 of a T113 with this patchset, and kindly provided
me with a Tested-by tag for patch 4/4.
- Remove a comment about R40/R528 being special, per feedback on v2.
- Simplify an if statement, per feedback on v2.
- Add missing 'select' directives to the R528 Kconfig, per feedback on v2.
Changes v1->v2:
- Power clamp is now adjusted ONLY on sun{6,7}i, H3, R40. The previous version
was mistakenly doing this EXCEPT on those machines.
- Flattened sunxi_power_switch() into sunxi_cpu_set_power() for simplicity's
sake.
- Moved the "power clamp is not NULL" conditional into sunxi_cpu_set_power().
- Removed unnecessary H6 special-case, since H6 is actually ARM64.
- Renamed SUNXI_CPUX_BASE to SUNXI_CPUCFG_BASE, to mirror expected changes in
Andre's v2 of the R528 series (we decided against using a new name for this
block).
- Removed sunxi_cpucfg_reg struct, and stopped using the PRCM struct in psci.c.
Happy Saturday all,
Sam
Sam Edwards (4):
sunxi: psci: clean away preprocessor macros
sunxi: psci: refactor register access to separate functions
sunxi: psci: stop modeling register layout with C structs
sunxi: psci: implement PSCI on R528
arch/arm/cpu/armv7/sunxi/psci.c | 185 ++-
arch/arm/include/asm/arch-sunxi/cpucfg.h | 67
arch/arm/mach-sunxi/Kconfig | 4 +
include/configs/sunxi-common.h | 8 +
4 files changed, 127 insertions(+), 137 deletions(-)
delete mode 100644 arch/arm/include/asm/arch-sunxi/cpucfg.h
--
2.41.0
Re: [PATCH v2 4/5] sunxi: psci: implement PSCI on R528
On 9/27/23 10:31, Andre Przywara wrote:
On Wed, 16 Aug 2023 10:34:19 -0700
Sam Edwards wrote:
Hi Sam,
Hi Andre,
@@ -103,10 +116,13 @@ static void __secure clamp_set(u32 *clamp)
static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
{
- /* secondary core entry address is programmed differently on R40 */
+ /* secondary core entry address is programmed differently on R40/528 */
I think that's somewhat obvious now from the code, so you can remove this
comment.
Done, change will be included in v3.
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
writel((u32)entry,
SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ writel((u32)entry,
+ SUNXI_R_CPUCFG_BASE + SUN8I_R528_SOFT_ENTRY);
} else {
writel((u32)entry, SUNXI_CPUCFG_BASE + SUNXI_PRIV0);
}
@@ -124,6 +140,9 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
clamp = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWR_CLAMP(cpu);
pwroff = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWROFF;
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ /* R528 leaves both cores powered up, manages them via reset */
+ return;
} else {
if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
IS_ENABLED(CONFIG_MACH_SUN8I_H3))
@@ -151,11 +170,27 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ if (reset) {
I think you can lose the brackets here, since it's a single statement
branch, even if it spans multiple lines. The indentation should make this
clear.
FWIW a lot of reviewers insist on braces surrounding *any* multiline
blocks, even if said block is only a single statement. This is to
prevent mishaps where another developer comes along later to add another
statement to the same block (at the same indentation level), but doesn't
think to look for missing brackets because the block is already bigger
than one line.
I could go either way on it, but would like to be sure that your
feedback stands in light of that counterpoint.
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 0a3454a51a..d46fd8c0bc 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -355,6 +355,8 @@ config MACH_SUN8I_R40
config MACH_SUN8I_R528
bool "sun8i (Allwinner R528)"
select CPU_V7A
+ select CPU_V7_HAS_NONSEC
+ select ARCH_SUPPORT_PSCI
Please add
select CPU_V7_HAS_VIRT
here, as the cores are perfectly capable of virtualisation. Granted,
support for KVM is long gone from Linux, but at least Xen still supports it.
Good catch; will be done in v3.
And I believe you also need:
select SPL_ARMV7_SET_CORTEX_SMPEN
At least this is what the other cores do. The PSCI code sets this bit for
the secondaries, but for the primary core we need to set it as early as
possible. Probably not a biggie on an A7, in reality, but good to have,
and be it for correctness and consistency's sake.
That's already enabled down below:
# The sun8i SoCs share a lot, this helps to avoid a lot of "if A23 || A33"
config MACH_SUN8I
bool
select SPL_ARMV7_SET_CORTEX_SMPEN if !ARM64
diff --git a/include/configs/sunxi-common.h b/include/configs/sunxi-common.h
index b8ca77d031..67eb0d25db 100644
--- a/include/configs/sunxi-common.h
+++ b/include/configs/sunxi-common.h
@@ -33,6 +33,14 @@
/* CPU */
+/*
+ * Newer ARM SoCs have moved the GIC, but have not updated their ARM cores to
+ * reflect the correct address in CBAR/PERIPHBASE.
+ */
+#if defined(CONFIG_SUN50I_GEN_H6) || defined(CONFIG_SUNXI_GEN_NCAT2)
+#define CFG_ARM_GIC_BASE_ADDRESS 0x0302
+#endif
I feel this should go into Kconfig. I can make a patch, unless you want to
beat me to it.
Note that you had previously [1] suggested placing this here, though
even then speculated that it belonged in Kconfig. I'm probably holding
off on sending a PSCI v3 until you send your R528 v2, so that might be a
good place to patch it. I'll remove this hunk if it's unnecessary by then.
[1]:
https://lore.kernel.org/u-boot/20230531161937.20d37...@donnerap.cambridge.arm.com/
Cheers,
Andre
Likewise,
Sam
Re: [PATCH v2 1/5] sunxi: psci: clean away preprocessor macros
On 9/27/23 10:34, Andre Przywara wrote:
In the majority of cases, there are no changes to the text section
introduced by this patch. In the R40 case, there's a small change where
the compiler adds a NULL check onto the result of the `(void *)cpucfg +
SUN8I_R40_PWR_CLAMP(cpu)` computation, which we can ignore as it won't
affect anything in practice. In the sun7i case, the only changes are
because I am NOT hardcoding the CPU to 0, which does look like I broke
it (since that means it will use cpu=1). So I'm going to need to fix
that in v3.
^^^
Do you have an update on this? I will try to test it on an R40 ASAP.
In my (not yet submitted) v3, I have the following change done:
if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
clamp = >cpu1_pwr_clamp;
pwroff = >cpu1_pwroff;
+ cpu = 0;
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
That does negate any binary change in the SUN7I case. R40 is the only
one that needs testing still. If you feel like testing on any SUN7Is
just for good measure, you'll want to add in that missing line.
Cheers,
Andre
Likewise,
Sam
Re: [PATCH v2 5/5] HACK: sunxi: psci: be compatible with v1 of R528 patchset
On 9/27/23 10:32, Andre Przywara wrote: On Wed, 16 Aug 2023 10:34:20 -0700 Sam Edwards wrote: Hi Sam, Hi Andre, Mmh, I didn't find a better solution than keeping this in. I'll keep it if your R528 v2 doesn't find some other way to address it. +#endif +#if defined(SUNXI_CPUX_BASE) && defined(SUNXI_CPUCFG_BASE) +#undef SUNXI_CPUCFG_BASE +#define SUNXI_CPUCFG_BASE SUNXI_CPUX_BASE So what's the story with this? Do we name this differently (SUNXI_CPUX_BASE) because the IP block is different from the other SoCs? Or is there another SUNXI_CPUCFG IP block on the R528/T113s SoCs? If not, I think we should use the SUNXI_CPUCFG_BASE name directly in cpu_sunxi_ncat2.h, as we never claimed that same names for some MMIO address blocks means they are compatible. Please let me know if I miss something. That's just for compatibility with R528 series v1. It's expected that you'll rename it to SUNXI_CPUCFG_BASE for v2. The preprocessor trickery looks for *both* being defined and applies the update. The rest of the code proceeds using SUNXI_CPUCFG_BASE. (Keep in mind this is particular patch is a hack patch, it's not considered for inclusion.) Warm regards, Sam
Re: [RFC PATCH 0/4] mtd: ubi: Enable accessing RO filesystems in UBI vols
Hi Heiko and Simon, Thought I'd follow-up to keep this discussion going. The main thing I would like to decide first (as it lets me start relying on it in boot scripts) would be the UBI access syntax: => ls ubi 0:rootfs /boot => ls ubi 0:2 /boot Do those look good? Should I be trying to mimic the accepted syntax of fs/ubifs/super.c:open_ubi()? Perhaps "ubi 0!rootfs" and/or "ubi 0_2"? Not using ':' leaves open the possibility for logical volumes (LVM2/UBI) to contain partitions - not that I expect anyone will want that. :) Cheers, Sam
Re: [PATCH] sunxi: psci: remove redundant initialization from psci_arch_init
On 8/26/23 04:22, Marc Zyngier wrote: Hi Marc! The GIC definitely has the NS bit routed to it. Otherwise, the secure configuration would just be an utter joke. Just try it. Thank you for your response. I'd like to revisit my prior point about the distinction between the NS bit and AxPROT[1] bits in the context of monitor mode: in monitor mode, the NS bit does not determine the security state of the CPU core (monitor mode is always secure). But even then, the NS bit is still significant for other purposes, such as to bank accesses to certain CP15 registers -- and if I understand Chen-Yu correctly, some GIC registers also. That would require a special NS bit signal routed to the GIC so that it can distinguish between "secure, NS=0" and "secure, NS=1" accesses, which is why I asked if such a thing exists. I understand that the GIC is designed to be aware of the security state (using the existing AxPROT[1] signals) so that it can protect the sensitive registers. And unless I misunderstand, this seems to be the point that you made here (my interpretation -- correct me if I'm wrong -- is that you are using "NS bit" as a metonym for "security state"). However I must clarify that my question was to seek further information from Chen-Yu about the possibility that NS is significant when accessing the GIC, even in monitor mode. Alternatively, his point might be merely highlighting that the GIC permits different types of access depending on the CPU's security state, which aligns with the viewpoint you've reiterated. I apologize if my previous message didn't convey this context clearly enough. My goal was to unravel this nuanced aspect of the NS bit when in monitor mode, and to determine if NS needs to be getting set/cleared during GIC setup to maneuver around the banking, or if the value of the NS bit when in psci_arch_init() is truly of no consequence due to monitor mode. Well, history is unfortunately against you on that front. Running on the secure side definitely was a requirement when this code was initially written, as the AW BSP *required* to run on the secure side. If that requirement is no more, great. But I don't think you can decide that unilaterally. I have no idea when/if this requirement was changed. It might have never happened "formally": perhaps at some point, the SCR.NS=1 code got added after the call to psci_arch_init(), breaking (that version of) the AW BSP, and nobody ever complained or changed it back... so it stayed. But we're starting to digress from what _this_ patch does. The intent here is only to remove two lines of code that (we're discussing to confirm) have no effect. I'm not touching the code that *actually* determines the world into which monitor mode exits. Cheers, Sam
Re: [PATCH] sunxi: psci: remove redundant initialization from psci_arch_init
On 8/25/23 00:20, Chen-Yu Tsai wrote: Hi Chen-Yu, IIRC the GIC manual says that the secure bit is set or cleared to select which bank of registers is accessed. Which secure bit are we talking about here? Do we mean the *configured* secure bit (SCR.NS, what the code is attempting to clear) or the *effective* secure bit (AWPROT[1], et al)? The distinction is important in monitor mode (where this function runs) since there (and only there) the CPU core ignores the configured setting and runs in the secure world unconditionally. I'm guessing it's most likely the latter since the former isn't exposed outside of the CPU core, unless the GIC has some special signal going to it... And I suppose it is here to be more robust. ...but if it is the former (i.e. SCR.NS is significant in this function) the code should be retained, but moved *before* the GIC register accesses, and the old value of SCR.NS should be restored *after*. Either way: I don't think this line should be kept in its current form, because it's written in a way that strongly suggests that we want to run in secure mode after exiting monitor mode, which is flatly not the case. Happy Friday all, Sam
Re: [PATCH 3/3] sunxi: psci: implement PSCI on R528
On 8/18/23 07:27, Andre Przywara wrote: Hi Andre, So instead of trying to derive some pattern from where there is none, I'd rather do: config SUNXI_CPU_HOTPLUG_ADDRESS hex default 0x01c000bc if MACH_SUN8I_R40 But the hotplug flag register is at 0x01c000b8 for R40? default 0x01c25da4 and then use that symbol in both places (v7 PSCI + v8 FEL). As mentioned, I can fix the pieces that break when naively doing so in the FEL code. That has also the rewarding property of allowing to remove both SUNXI_R_CPUCFG_BASE and SUNXI_RTC_BASE from the code base completely. I might hold on to SUNXI_RTC_BASE for a bit, since the SPL may need it for the various reset-to-FEL mechanisms. I still want a nice way of getting mainline U-Boot to do this in the near term. What do you think? I don't think this really solves your underlying grievance, since various sunxis need distinct offsets to jump from the hotplug address register to the secondary CPU soft entry point: CONFIG_SUNXI_CPU_HOTPLUG_ADDRESS + 4 // for R40 CONFIG_SUNXI_CPU_HOTPLUG_ADDRESS + 8 // for R528 ...at which point we're back to the "Kconfig symbol is really pointing to a sort of control block" idea. We could take advantage of the fact that ARMv7 psci.c and ARMv8 fel_utils.S are never both built and call it something horrible like "SUNXI_CPU_HOTPLUG_OR_SOFT_ENTRY_ADDR" but then it wouldn't be clear from a glance *which* it is. We could go with *two* (three, if we need R528's CPU0 entry for a reset-to-FEL mechanism) Kconfig symbols, but that's a lot for configuration symbols that are really only needed in pretty much one place each. Cheers, Andre Likewise, Sam
Re: [PATCH v2 1/5] sunxi: psci: clean away preprocessor macros
On 8/18/23 10:40, Sam Edwards wrote: On 8/18/23 07:11, Andre Przywara wrote: Hi Andre, The resulting object file is different (8 byte larger, even), so it's hard to prove I'm no stranger to reading object code. Since the output should be identical in principle, I'll spend a little bit of time today seeing if I can identify what's changing. If it's easy enough, I'd like to adjust my patch so that the optimizer does produce the same output. (Keep in mind I'm on Clang, though. If Clang already gives the same output for both, I'll just report back to use that when comparing.) I built only psci.o from every ARM32 sunxi for which we have a defconfig (and for which PSCI is supported), for 81 targets total (though there are only 4 variations: R40, sun7i, H3/sun6i, and "everything else"). I am working with Clang version 16.0.6. I compared only the secure text section. The command to extract this looks like: llvm-objcopy -O binary --only-section=._secure.text psci.o text.bin This is important because there are debug sections that will change when the source file line numbers change, so we must ignore those when comparing. In the majority of cases, there are no changes to the text section introduced by this patch. In the R40 case, there's a small change where the compiler adds a NULL check onto the result of the `(void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu)` computation, which we can ignore as it won't affect anything in practice. In the sun7i case, the only changes are because I am NOT hardcoding the CPU to 0, which does look like I broke it (since that means it will use cpu=1). So I'm going to need to fix that in v3. For good measure, I also applied the same methodology to patch 2 in this series, and that introduces no text section changes whatsoever in any of the tested cases. So patch 2 (theoretically, anyway) needs no bugfixes or hardware testing. Patch 3 does cause a text section change for all targets. I will have to investigate why, in case I messed up any of the offsets when migrating off of structs. Regards, Sam
Re: [PATCH v2 1/5] sunxi: psci: clean away preprocessor macros
On 8/18/23 07:11, Andre Przywara wrote: Hi Andre, The resulting object file is different (8 byte larger, even), so it's hard to prove I'm no stranger to reading object code. Since the output should be identical in principle, I'll spend a little bit of time today seeing if I can identify what's changing. If it's easy enough, I'd like to adjust my patch so that the optimizer does produce the same output. (Keep in mind I'm on Clang, though. If Clang already gives the same output for both, I'll just report back to use that when comparing.) Signed-off-by: Sam Edwards Reviewed-by: Andre Przywara Thank you! Cheers, Andre Likewise, Sam
Re: [PATCH v2 2/5] sunxi: psci: refactor register access to separate functions
On 8/18/23 07:57, Andre Przywara wrote: On Wed, 16 Aug 2023 10:34:17 -0700 Sam Edwards wrote: Hi Sam, Likewise Andre, -static void __secure sunxi_set_entry_address(void *entry) +static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry) So what is the reasoning behind this change? The primary reason is to be consistent with every other sunxi_cpu_* function, while the *tertiary* reason is that it was useful to have that argument preserved in a debug build, so that when I was tracing execution I could be *sure* that the correct CPU was being chosen (this was before I found out that the GIC400 base was being determined incorrectly). As for the secondary reason... If *none* of the Allwinner SoCs have independent secondary entry point registers, we should not give the impression some do in the prototype. Should later a SoC emerge that changes this, adjusting this one is the least of our problems then. Ah, but the T113 is already such an SoC. From the user manual: - The Soft Entry Address Register of CPU0 is 0x070005C4. - The Soft Entry Address Register of CPU1 is 0x070005C8. ...so my second reason for the function prototype being the way it is (and perhaps something which I should be driving home in patch 4/5) is indeed to give that impression: that it is *not* the case that none of the sunxis have independent secondary entry point registers. +void __secure sunxi_cpu_power_off(u32 cpuid) Can you please mark this as static on the way? That does not seem to be used anywhere, and even the name suggests it's local. Saves 8 bytes of .text ;-) Easy enough, it shall be done! Thanks, Sam
[PATCH] pci: pcie-brcmstb: do not rely on CLKREQ# signal
When the Broadcom STB PCIe controller is initialized, it must be set into one of three CLKREQ# modes: "none"/"aspm"/"l1ss". The Linux driver, through today, hard-codes "aspm" since the vast majority of boards using this driver have a fixed PCIe bus with the CLKREQ# signal wired up. The Raspberry Pi CM4, however, can be connected to a plethora of PCIe devices, some of which do not connect the CLKREQ# line (they just leave it floating). So "aspm" mode is no longer appropriate in all cases. In Linux, there is a proposed patchset [1] to determine the proper mode. This doesn't really make sense in U-Boot's case, so we just change the assumption from "aspm" to "none" (which is always safe). This patch DOES resolve a real-world crash that occurs when U-Boot is running on a Raspberry Pi CM4 installed in slot 3 of a Turing Pi 2 cluster board. [1]: https://lore.kernel.org/all/20230428223500.23337-1-jim2101...@gmail.com/ Signed-off-by: Sam Edwards --- drivers/pci/pcie_brcmstb.c | 18 +- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c index 1de2802113..b8105654c5 100644 --- a/drivers/pci/pcie_brcmstb.c +++ b/drivers/pci/pcie_brcmstb.c @@ -33,6 +33,9 @@ #define PCIE_RC_CFG_PRIV1_ID_VAL3 0x043c #define CFG_PRIV1_ID_VAL3_CLASS_CODE_MASK 0xff +#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY 0x04dc +#define PCIE_RC_CFG_PRIV1_LINK_CAPABILITY_ASPM_SUPPORT_MASK 0xc00 + #define PCIE_RC_DL_MDIO_ADDR 0x1100 #define PCIE_RC_DL_MDIO_WR_DATA0x1104 #define PCIE_RC_DL_MDIO_RD_DATA0x1108 @@ -88,7 +91,6 @@ PCIE_MISC_CPU_2_PCIE_MEM_WIN0_LIMIT_HI + ((win) * 8) #define PCIE_MISC_HARD_PCIE_HARD_DEBUG 0x4204 -#define PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK 0x2 #define PCIE_HARD_DEBUG_SERDES_IDDQ_MASK 0x0800 #define PCIE_MSI_INTR2_CLR 0x4508 @@ -562,12 +564,18 @@ static int brcm_pcie_probe(struct udevice *dev) clrsetbits_le32(base + PCIE_RC_CFG_VENDOR_SPECIFIC_REG1, VENDOR_SPECIFIC_REG1_ENDIAN_MODE_BAR2_MASK, VENDOR_SPECIFIC_REG1_LITTLE_ENDIAN); + /* -* Refclk from RC should be gated with CLKREQ# input when ASPM L0s,L1 -* is enabled => setting the CLKREQ_DEBUG_ENABLE field to 1. +* We used to enable the CLKREQ# input here, but a few PCIe cards don't +* attach anything to the CLKREQ# line, so we shouldn't assume that +* it's connected and working. The controller does allow detecting +* whether the port on the other side of our link is/was driving this +* signal, so we could check before we assume. But because this signal +* is for power management, which doesn't make sense in a bootloader, +* let's instead just unadvertise ASPM support. */ - setbits_le32(base + PCIE_MISC_HARD_PCIE_HARD_DEBUG, -PCIE_HARD_DEBUG_CLKREQ_DEBUG_ENABLE_MASK); + clrbits_le32(base + PCIE_RC_CFG_PRIV1_LINK_CAPABILITY, +PCIE_RC_CFG_PRIV1_LINK_CAPABILITY_ASPM_SUPPORT_MASK); return 0; } -- 2.41.0
[PATCH v2 5/5] HACK: sunxi: psci: be compatible with v1 of R528 patchset
This is a hack for reviewer QoL. It is not being submitted for mainline
inclusion.
---
arch/arm/cpu/armv7/sunxi/psci.c | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index b4ce4f6def..27bac291d5 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -60,6 +60,18 @@
#define SUN8I_R528_C0_STATUS_STANDBYWFI(16)
+/* 3 hacks for compatibility across v1/v2 of Andre's R528 support series */
+#ifndef SUNXI_R_CPUCFG_BASE
+#define SUNXI_R_CPUCFG_BASE0
+#endif
+#ifndef SUNXI_PRCM_BASE
+#define SUNXI_PRCM_BASE0
+#endif
+#if defined(SUNXI_CPUX_BASE) && defined(SUNXI_CPUCFG_BASE)
+#undef SUNXI_CPUCFG_BASE
+#define SUNXI_CPUCFG_BASE SUNXI_CPUX_BASE
+#endif
+
static void __secure cp15_write_cntp_tval(u32 tval)
{
asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
--
2.41.0
[PATCH v2 4/5] sunxi: psci: implement PSCI on R528
This patch adds the necessary code to make nonsec booting and PSCI
secondary core management functional on the R528/T113.
Signed-off-by: Sam Edwards
Tested-by: Maksim Kiselev
---
arch/arm/cpu/armv7/sunxi/psci.c | 48 -
arch/arm/mach-sunxi/Kconfig | 2 ++
include/configs/sunxi-common.h | 8 ++
3 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 6ecdd05250..b4ce4f6def 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -47,6 +47,19 @@
#define SUN8I_R40_PWR_CLAMP(cpu) (0x120 + (cpu) * 0x4)
#define SUN8I_R40_SRAMC_SOFT_ENTRY_REG0(0xbc)
+/*
+ * R528 is also different, as it has both cores powered up (but held in reset
+ * state) after the SoC is reset. Like the R40, it uses a "soft" entry point
+ * address register, but unlike the R40, it uses a newer "CPUX" block to manage
+ * CPU state, rather than the older CPUCFG system.
+ */
+#define SUN8I_R528_SOFT_ENTRY (0x1c8)
+#define SUN8I_R528_C0_RST_CTRL (0x)
+#define SUN8I_R528_C0_CTRL_REG0(0x0010)
+#define SUN8I_R528_C0_CPU_STATUS (0x0080)
+
+#define SUN8I_R528_C0_STATUS_STANDBYWFI(16)
+
static void __secure cp15_write_cntp_tval(u32 tval)
{
asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
@@ -103,10 +116,13 @@ static void __secure clamp_set(u32 *clamp)
static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
{
- /* secondary core entry address is programmed differently on R40 */
+ /* secondary core entry address is programmed differently on R40/528 */
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
writel((u32)entry,
SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ writel((u32)entry,
+ SUNXI_R_CPUCFG_BASE + SUN8I_R528_SOFT_ENTRY);
} else {
writel((u32)entry, SUNXI_CPUCFG_BASE + SUNXI_PRIV0);
}
@@ -124,6 +140,9 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
clamp = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWR_CLAMP(cpu);
pwroff = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWROFF;
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ /* R528 leaves both cores powered up, manages them via reset */
+ return;
} else {
if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
IS_ENABLED(CONFIG_MACH_SUN8I_H3))
@@ -151,11 +170,27 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ if (reset) {
+ clrbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_RST_CTRL,
+BIT(cpu));
+ } else {
+ setbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_RST_CTRL,
+BIT(cpu));
+ }
+ return;
+ }
+
writel(reset ? 0b00 : 0b11, SUNXI_CPUCFG_BASE + SUNXI_CPU_RST(cpu));
}
static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ /* Not required on R528 */
+ return;
+ }
+
if (lock)
clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
else
@@ -164,11 +199,22 @@ static void __secure sunxi_cpu_set_locking(int cpu, bool
lock)
static bool __secure sunxi_cpu_poll_wfi(int cpu)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ return !!(readl(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_CPU_STATUS) &
+ BIT(SUN8I_R528_C0_STATUS_STANDBYWFI + cpu));
+ }
+
return !!(readl(SUNXI_CPUCFG_BASE + SUNXI_CPU_STATUS(cpu)) & BIT(2));
}
static void __secure sunxi_cpu_invalidate_cache(int cpu)
{
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ clrbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_CTRL_REG0,
+BIT(cpu));
+ return;
+ }
+
clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_GEN_CTRL, BIT(cpu));
}
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 0a3454a51a..d46fd8c0bc 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -355,6 +355,8 @@ config MACH_SUN8I_R40
config MACH_SUN8I_R528
bool "sun8i (Allwinner R528)"
select CPU_V7A
+ select CPU_V7_HAS_NONSEC
+ select ARCH_SUPPORT_PSCI
select SUNXI_GEN_NCAT2
select SUNXI_NEW_PINCTRL
select MMC_SUNXI_HAS_NEW_
[PATCH v2 3/5] sunxi: psci: stop modeling register layout with C structs
Since the sunxi support nowadays generally prefers #defined register
offsets instead of modeling register layouts using C structs, now is a
good time to do this for PSCI as well. This patch moves away from using
the structs `sunxi_cpucfg_reg` and `sunxi_prcm_reg` in psci.c.
The former struct and its associated header file existed only to support
PSCI code, so also delete them altogether.
Signed-off-by: Sam Edwards
---
arch/arm/cpu/armv7/sunxi/psci.c | 57
arch/arm/include/asm/arch-sunxi/cpucfg.h | 67
2 files changed, 23 insertions(+), 101 deletions(-)
delete mode 100644 arch/arm/include/asm/arch-sunxi/cpucfg.h
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index e2845f21ab..6ecdd05250 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -11,8 +11,6 @@
#include
#include
-#include
-#include
#include
#include
#include
@@ -27,6 +25,17 @@
#defineGICD_BASE (SUNXI_GIC400_BASE + GIC_DIST_OFFSET)
#defineGICC_BASE (SUNXI_GIC400_BASE + GIC_CPU_OFFSET_A15)
+/*
+ * Offsets into the CPUCFG block applicable to most SUNXIs.
+ */
+#define SUNXI_CPU_RST(cpu) (0x40 + (cpu) * 0x40 + 0x0)
+#define SUNXI_CPU_STATUS(cpu) (0x40 + (cpu) * 0x40 + 0x8)
+#define SUNXI_GEN_CTRL (0x184)
+#define SUNXI_PRIV0(0x1a4)
+#define SUN7I_CPU1_PWR_CLAMP (0x1b0)
+#define SUN7I_CPU1_PWROFF (0x1b4)
+#define SUNXI_DBG_CTRL1(0x1e4)
+
/*
* R40 is different from other single cluster SoCs.
*
@@ -99,10 +108,7 @@ static void __secure sunxi_cpu_set_entry(int
__always_unused cpu, void *entry)
writel((u32)entry,
SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
} else {
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- writel((u32)entry, >priv0);
+ writel((u32)entry, SUNXI_CPUCFG_BASE + SUNXI_PRIV0);
}
}
@@ -110,25 +116,20 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
{
u32 *clamp = NULL;
u32 *pwroff;
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
/* sun7i (A20) is different from other single cluster SoCs */
if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
- clamp = >cpu1_pwr_clamp;
- pwroff = >cpu1_pwroff;
+ clamp = (void *)SUNXI_CPUCFG_BASE + SUN7I_CPU1_PWR_CLAMP;
+ pwroff = (void *)SUNXI_CPUCFG_BASE + SUN7I_CPU1_PWROFF;
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
- clamp = (void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu);
- pwroff = (void *)cpucfg + SUN8I_R40_PWROFF;
+ clamp = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWR_CLAMP(cpu);
+ pwroff = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWROFF;
} else {
- struct sunxi_prcm_reg *prcm =
- (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
-
if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
IS_ENABLED(CONFIG_MACH_SUN8I_H3))
- clamp = >cpu_pwr_clamp[cpu];
+ clamp = (void *)SUNXI_PRCM_BASE + 0x140 + cpu * 0x4;
- pwroff = >cpu_pwroff;
+ pwroff = (void *)SUNXI_PRCM_BASE + 0x100;
}
if (on) {
@@ -150,37 +151,25 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- writel(reset ? 0b00 : 0b11, >cpu[cpu].rst);
+ writel(reset ? 0b00 : 0b11, SUNXI_CPUCFG_BASE + SUNXI_CPU_RST(cpu));
}
static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
if (lock)
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
else
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ setbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
}
static bool __secure sunxi_cpu_poll_wfi(int cpu)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- return !!(readl(>cpu[cpu].status) & BIT(2));
+ return !!(readl(SUNXI_CPUCFG_BASE + SUNXI_CPU_STATUS(cpu)) & BIT(2));
}
static void __secure sunxi_cpu_invalidate_cache(int cpu)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- clrbits_le32(>gen_ctrl, BIT(cpu));
[PATCH v2 2/5] sunxi: psci: refactor register access to separate functions
This is to prepare for R528, which does not have the typical
"CPUCFG" block; it has a "CPUX" block which provides these
same functions but is organized differently.
Moving the hardware-access bits to their own functions separates the
logic from the hardware so we can reuse the same logic.
Signed-off-by: Sam Edwards
---
arch/arm/cpu/armv7/sunxi/psci.c | 66 +++--
1 file changed, 47 insertions(+), 19 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 7804e0933b..e2845f21ab 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -92,7 +92,7 @@ static void __secure clamp_set(u32 *clamp)
writel(0xff, clamp);
}
-static void __secure sunxi_set_entry_address(void *entry)
+static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
{
/* secondary core entry address is programmed differently on R40 */
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
@@ -148,30 +148,60 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
}
}
-void __secure sunxi_cpu_power_off(u32 cpuid)
+static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ writel(reset ? 0b00 : 0b11, >cpu[cpu].rst);
+}
+
+static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
{
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ if (lock)
+ clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ else
+ setbits_le32(>dbg_ctrl1, BIT(cpu));
+}
+
+static bool __secure sunxi_cpu_poll_wfi(int cpu)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ return !!(readl(>cpu[cpu].status) & BIT(2));
+}
+
+static void __secure sunxi_cpu_invalidate_cache(int cpu)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ clrbits_le32(>gen_ctrl, BIT(cpu));
+}
+
+void __secure sunxi_cpu_power_off(u32 cpuid)
+{
u32 cpu = cpuid & 0x3;
/* Wait for the core to enter WFI */
- while (1) {
- if (readl(>cpu[cpu].status) & BIT(2))
- break;
+ while (!sunxi_cpu_poll_wfi(cpu))
__mdelay(1);
- }
/* Assert reset on target CPU */
- writel(0, >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, true);
/* Lock CPU (Disable external debug access) */
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ sunxi_cpu_set_locking(cpu, true);
/* Power down CPU */
sunxi_cpu_set_power(cpuid, false);
- /* Unlock CPU (Disable external debug access) */
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ /* Unlock CPU (Reenable external debug access) */
+ sunxi_cpu_set_locking(cpu, false);
}
static u32 __secure cp15_read_scr(void)
@@ -228,33 +258,31 @@ out:
int __secure psci_cpu_on(u32 __always_unused unused, u32 mpidr, u32 pc,
u32 context_id)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
u32 cpu = (mpidr & 0x3);
/* store target PC and context id */
psci_save(cpu, pc, context_id);
/* Set secondary core power on PC */
- sunxi_set_entry_address(_cpu_entry);
+ sunxi_cpu_set_entry(cpu, _cpu_entry);
/* Assert reset on target CPU */
- writel(0, >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, true);
/* Invalidate L1 cache */
- clrbits_le32(>gen_ctrl, BIT(cpu));
+ sunxi_cpu_invalidate_cache(cpu);
/* Lock CPU (Disable external debug access) */
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ sunxi_cpu_set_locking(cpu, true);
/* Power up target CPU */
sunxi_cpu_set_power(cpu, true);
/* De-assert reset on target CPU */
- writel(BIT(1) | BIT(0), >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, false);
- /* Unlock CPU (Disable external debug access) */
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ /* Unlock CPU (Reenable external debug access) */
+ sunxi_cpu_set_locking(cpu, false);
return ARM_PSCI_RET_SUCCESS;
}
--
2.41.0
[PATCH v2 1/5] sunxi: psci: clean away preprocessor macros
This patch restructures psci.c to get away from the "many different
function definitions switched by #ifdef" paradigm to the preferred style
of having a single function definition with `if (IS_ENABLED(...))` to
make the optimizer include only the appropriate function bodies instead.
There are no functional changes here.
Signed-off-by: Sam Edwards
---
arch/arm/cpu/armv7/sunxi/psci.c | 102 +---
1 file changed, 42 insertions(+), 60 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index e1d3638b5c..7804e0933b 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -76,11 +76,8 @@ static void __secure __mdelay(u32 ms)
isb();
}
-static void __secure clamp_release(u32 __maybe_unused *clamp)
+static void __secure clamp_release(u32 *clamp)
{
-#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
- defined(CONFIG_MACH_SUN8I_H3) || \
- defined(CONFIG_MACH_SUN8I_R40)
u32 tmp = 0x1ff;
do {
tmp >>= 1;
@@ -88,83 +85,68 @@ static void __secure clamp_release(u32 __maybe_unused
*clamp)
} while (tmp);
__mdelay(10);
-#endif
}
-static void __secure clamp_set(u32 __maybe_unused *clamp)
+static void __secure clamp_set(u32 *clamp)
{
-#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
- defined(CONFIG_MACH_SUN8I_H3) || \
- defined(CONFIG_MACH_SUN8I_R40)
writel(0xff, clamp);
-#endif
}
-static void __secure sunxi_power_switch(u32 *clamp, u32 *pwroff, bool on,
- int cpu)
+static void __secure sunxi_set_entry_address(void *entry)
{
- if (on) {
- /* Release power clamp */
- clamp_release(clamp);
-
- /* Clear power gating */
- clrbits_le32(pwroff, BIT(cpu));
+ /* secondary core entry address is programmed differently on R40 */
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
+ writel((u32)entry,
+ SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
} else {
- /* Set power gating */
- setbits_le32(pwroff, BIT(cpu));
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
- /* Activate power clamp */
- clamp_set(clamp);
+ writel((u32)entry, >priv0);
}
}
-#ifdef CONFIG_MACH_SUN8I_R40
-/* secondary core entry address is programmed differently on R40 */
-static void __secure sunxi_set_entry_address(void *entry)
-{
- writel((u32)entry,
- SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
-}
-#else
-static void __secure sunxi_set_entry_address(void *entry)
+static void __secure sunxi_cpu_set_power(int cpu, bool on)
{
+ u32 *clamp = NULL;
+ u32 *pwroff;
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
- writel((u32)entry, >priv0);
-}
-#endif
+ /* sun7i (A20) is different from other single cluster SoCs */
+ if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
+ clamp = >cpu1_pwr_clamp;
+ pwroff = >cpu1_pwroff;
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
+ clamp = (void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu);
+ pwroff = (void *)cpucfg + SUN8I_R40_PWROFF;
+ } else {
+ struct sunxi_prcm_reg *prcm =
+ (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
-#ifdef CONFIG_MACH_SUN7I
-/* sun7i (A20) is different from other single cluster SoCs */
-static void __secure sunxi_cpu_set_power(int __always_unused cpu, bool on)
-{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
+ IS_ENABLED(CONFIG_MACH_SUN8I_H3))
+ clamp = >cpu_pwr_clamp[cpu];
- sunxi_power_switch(>cpu1_pwr_clamp, >cpu1_pwroff,
- on, 0);
-}
-#elif defined CONFIG_MACH_SUN8I_R40
-static void __secure sunxi_cpu_set_power(int cpu, bool on)
-{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ pwroff = >cpu_pwroff;
+ }
- sunxi_power_switch((void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu),
- (void *)cpucfg + SUN8I_R40_PWROFF,
- on, cpu);
-}
-#else /* ! CONFIG_MACH_SUN7I && ! CONFIG_MACH_SUN8I_R40 */
-static void __secure sunxi_cpu_set_power(int cpu, bool on)
-{
- struct sunxi_prcm_reg *prcm =
- (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
+ if (on) {
+ /* Release power clamp */
+ if (clamp)
+ clamp_release(clamp);
- sunxi_power_switch(>cpu_pwr_clamp[cp
[PATCH v2 0/5] Allwinner R528/T113s PSCI
Hi list,
This is the second version of my patchset for supporting PSCI (i.e. second core
bringup) on R528/T113-s#, incorporating most of the feedback from Andre.
I do not expect the final two patches in this series to be winners yet. There's
still some back-and-forth going on to sync them up with an anticipated v2 of
Andre's patchset for R528 support (on which this work depends). My v2 is being
sent now mostly just to smoothen that.
However, the first 3 patches in this series ARE meant to be reviewed seriously:
the refactoring is totally independent of the R528 effort, and I see no reason
to wait for the R528 stuff to be nailed down to get started reviewing them.
Going forward, any more changes will most likely be form over function, so
aggressive testing of the whole 5-patch series -- on sunxis old and new -- is
nonetheless very valuable at this point. :)
Changes v1->v2:
- Power clamp is now adjusted ONLY on sun{6,7}i, H3, R40. The previous version
was mistakenly doing this EXCEPT on those machines.
- Flattened sunxi_power_switch() into sunxi_cpu_set_power() for simplicity's
sake.
- Moved the "power clamp is not NULL" conditional into sunxi_cpu_set_power().
- Removed unnecessary H6 special-case, since H6 is actually ARM64.
- Renamed SUNXI_CPUX_BASE to SUNXI_CPUCFG_BASE, to mirror expected changes in
Andre's v2 of the R528 series (we decided against using a new name for this
block).
- Removed sunxi_cpucfg_reg struct, and stopped using the PRCM struct in psci.c.
Cheers,
Sam
Sam Edwards (5):
sunxi: psci: clean away preprocessor macros
sunxi: psci: refactor register access to separate functions
sunxi: psci: stop modeling register layout with C structs
sunxi: psci: implement PSCI on R528
HACK: sunxi: psci: be compatible with v1 of R528 patchset
arch/arm/cpu/armv7/sunxi/psci.c | 195 +++
arch/arm/include/asm/arch-sunxi/cpucfg.h | 67
arch/arm/mach-sunxi/Kconfig | 2 +
include/configs/sunxi-common.h | 8 +
4 files changed, 136 insertions(+), 136 deletions(-)
delete mode 100644 arch/arm/include/asm/arch-sunxi/cpucfg.h
--
2.41.0
Re: [PATCH 3/3] sunxi: psci: implement PSCI on R528
On 8/15/23 15:59, Andre Przywara wrote: Hi Sam, Hi Andre, So that's a bit more nasty indeed. I don't even know if R_CPUCFG really makes sense here, as the _R_ term typically refers to the management processor, which the D1/R528 don't have. Or at least the always-on power domain, but then again this hardly relates to the secondary entry point. I think the name was just used because the address matches the one used in the H6. Oh, no. That was my doing (and my reasoning) by suggesting that for inclusion in your series. Yours is good reasoning to be rid of it. So taking a step back, I wonder if we should actually just define a CONFIG_SUNXI_CPU_SOFT_ENTRY (or so) *Kconfig* symbol, which holds that address, and let the per-SoC definition be solved in Kconfig instead. Because SUNXI_R_CPUCFG_BASE and also SUNXI_RTC_BASE seem to be just used as the base address for that purpose, with some magic offset added, across all of U-Boot (ARMv8 FEL and v7 PSCI). Mmh, since this is a block of soft registers for managing several functions of both cores, I think I'd rather point to the base of the block and still use an offset to get to the specific soft register. Allwinner may keep this layout for a 4-core chip in the near future or U-Boot may want to add code that sets the CPU0 hotplug flag, for example. I'm not unwilling to do the Kconfig route, but just out of curiosity, what would your fallback plan be? So can you try to work on that base? I will take care of armv8/fel_utils.S, which uses some post-increment assembly trick to keep the code small, which wouldn't work anymore. But I have an idea how to solve this. Before that, I think now might be a good time for me to send in the v2 that I have so far; I doubt the final patch of my v2 series will pass review, but I'd like to keep us synced up (and clear away any patches in that series that do pass review off from my mental desktop). Cheers, Andre Likewise, Sam
Re: [PATCH 3/3] sunxi: psci: implement PSCI on R528
On 8/14/23 08:16, Andre Przywara wrote: Hi Sam, This patch adds the necessary code to make nonsec booting and PSCI secondary core management functional on the R528/T113. Unfortunately this patch breaks the build on older 32-bit SoCs, as SUNXI_CPUX_BASE is not defined there. That's a typical problem of the "C-if" approach, but typically there is a clean, albeit not trivial, solution: It seems like SUNXI_CPUX_BASE and SUNXI_CPUCFG_BASE are mutually exclusive, and I actually carry a "#define SUNXI_CPUCFG_BASE 0" hack in my patches already. So I wonder if we should unify those two into SUNXI_CPUCFG_BASE, with the following rework: The SUNXI_CPUX_BASE -> SUNXI_CPUCFG_BASE rename worked excellently. We're having the same problem with SUNXI_R_CPUCFG_BASE as well, though. How do you want to handle that? Thanks, Sam
[PATCH] pci: pcie-brcmstb: bring over some robustness improvements from Linux
Since the initial U-Boot driver was ported here from Linux, the latter has had a few changes for robustness/stability. This patch brings over two of them: - Do not attempt to access the configuration space of a PCIe device if the link has gone down, as that will result in an asynchronous SError interrupt which will crash U-Boot. - Wait for the recommended 100ms after PERST# is deasserted. I sent this patch while debugging a crash involving PCIe, but these are unrelated improvements. I do not believe that this patch fixes any real-world bug. Signed-off-by: Sam Edwards --- drivers/pci/pcie_brcmstb.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c index 1de2802113..a159952822 100644 --- a/drivers/pci/pcie_brcmstb.c +++ b/drivers/pci/pcie_brcmstb.c @@ -223,6 +223,10 @@ static int brcm_pcie_config_address(const struct udevice *dev, pci_dev_t bdf, return 0; } + /* An access to our HW w/o link-up will cause a CPU Abort */ + if (!brcm_pcie_link_up(pcie)) + return -EINVAL; + /* For devices, write to the config space index register */ idx = PCIE_ECAM_OFFSET(pci_bus, pci_dev, pci_func, 0); @@ -505,6 +509,12 @@ static int brcm_pcie_probe(struct udevice *dev) clrbits_le32(pcie->base + PCIE_RGR1_SW_INIT_1, RGR1_SW_INIT_1_PERST_MASK); + /* +* Wait for 100ms after PERST# deassertion; see PCIe CEM specification +* sections 2.2, PCIe r5.0, 6.6.1. +*/ + mdelay(100); + /* Give the RC/EP time to wake up, before trying to configure RC. * Intermittently check status for link-up, up to a total of 100ms. */ -- 2.41.0
Re: [PATCH 1/3] sunxi: psci: clean away preprocessor macros
On 8/14/23 15:05, Andre Przywara wrote: Yes, I will add this to the header file, either defined as 0, or to its actual address. Gotcha; my v2 will also assume you've taken care of merging these guys: +#define SUNXI_CPUX_BASE0x0901 +#define SUNXI_CPUCFG_BASE 0 (As I presume from your comments on 3/3 that it's better to consider "CPUX" simply an updated CPUCFG than to distinguish between them as fundamentally different concepts.) Yes, but you handle both above explicitly. No, I was passing a NULL clamp address in those cases too. So I'm noting that this must also be fixed in v2. Thanks, Sam
Re: [PATCH 0/3] Allwinner R528/T113s PSCI
On 8/14/23 08:06, Andre Przywara wrote: Hi Sam, many many thanks for sending this, I especially like your clean up around the #ifdef's! The patches looks good on the first glance (apart from some regression in patch 3/3), but I will reply to them individually. Cheers, Andre Thanks for your attention to these! While you're in a PSCI headspace, could you also look at my patch titled: sunxi: psci: remove redundant initialization from psci_arch_init ...dated May 31? This one should be pretty uncontroversial and straightforward to review. :) Many thanks, Sam
Re: [PATCH 1/3] sunxi: psci: clean away preprocessor macros
Hi Andre, On 8/14/23 10:37, Andre Przywara wrote: So I think we can get rid of this: - GEN_H6 never compiles this code here, as both H6 and H616 are arm64. Easy! - We can define SUNXI_PRCM_BASE for NCAT2, I believe Samuel once mentioned that the D1/T113 does have such a block, actually. Will you be taking care of this in v2 of your T113s series, or should I be adding it (in which case I'll need to know the location of the block)? - The non-existing cpu_pwr_clamp member should go away when you switch to a BASE_ADDR + REG_OFFSET approach, I think. Less easy, but still can do. Shouldn't that be the opposite? In the existing code, sun6i and H3 DO program the clamp (see the "-" section above). And sun7i and R40, as well. It appears I simply read the #if defined(...) mess backwards. I'll fix that for v2. As a bonus, this lends itself to a rather nice refactoring of sunxi_cpu_set_power() where I can have the if block only determine the pwroff/clamp addresses, and have a single tail-call to sunxi_power_switch() at the bottom. Since the latter function is so simple, I may as well just inline it into sunxi_cpu_set_power() (which I suspect might be more readable). Cheers, Andre Likewise, Sam
[PATCH 3/3] sunxi: psci: implement PSCI on R528
This patch adds the necessary code to make nonsec booting and PSCI
secondary core management functional on the R528/T113.
Signed-off-by: Sam Edwards
Tested-by: Maksim Kiselev
---
arch/arm/cpu/armv7/sunxi/psci.c | 47 -
arch/arm/mach-sunxi/Kconfig | 2 ++
include/configs/sunxi-common.h | 8 ++
3 files changed, 56 insertions(+), 1 deletion(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 94120e7526..f3c1c459c2 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -38,6 +38,19 @@
#define SUN8I_R40_PWR_CLAMP(cpu) (0x120 + (cpu) * 0x4)
#define SUN8I_R40_SRAMC_SOFT_ENTRY_REG0(0xbc)
+/*
+ * R528 is also different, as it has both cores powered up (but held in reset
+ * state) after the SoC is reset. Like the R40, it uses a "soft" entry point
+ * address register, but unlike the R40, it uses a newer "CPUX" block to manage
+ * CPU state, rather than the older CPUCFG system.
+ */
+#define SUN8I_R528_SOFT_ENTRY (0x1c8)
+#define SUN8I_R528_C0_RST_CTRL (0x)
+#define SUN8I_R528_C0_CTRL_REG0(0x0010)
+#define SUN8I_R528_C0_CPU_STATUS (0x0080)
+
+#define SUN8I_R528_C0_STATUS_STANDBYWFI(16)
+
static void __secure cp15_write_cntp_tval(u32 tval)
{
asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
@@ -116,10 +129,13 @@ static void __secure sunxi_power_switch(u32 *clamp, u32
*pwroff, bool on,
static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
{
- /* secondary core entry address is programmed differently on R40 */
+ /* secondary core entry address is programmed differently on R40/528 */
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
writel((u32)entry,
SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ writel((u32)entry,
+ SUNXI_R_CPUCFG_BASE + SUN8I_R528_SOFT_ENTRY);
} else {
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
@@ -139,6 +155,8 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
sunxi_power_switch(NULL, (void *)cpucfg + SUN8I_R40_PWROFF,
on, cpu);
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ /* R528 leaves both cores powered up, manages them via reset */
} else {
#if !defined(CONFIG_SUN50I_GEN_H6) && !defined(CONFIG_SUNXI_GEN_NCAT2)
struct sunxi_prcm_reg *prcm =
@@ -159,6 +177,17 @@ static void __secure sunxi_cpu_set_reset(int cpu, bool
reset)
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ if (reset) {
+ clrbits_le32(SUNXI_CPUX_BASE + SUN8I_R528_C0_RST_CTRL,
+BIT(cpu));
+ } else {
+ setbits_le32(SUNXI_CPUX_BASE + SUN8I_R528_C0_RST_CTRL,
+BIT(cpu));
+ }
+ return;
+ }
+
writel(reset ? 0b00 : 0b11, >cpu[cpu].rst);
}
@@ -167,6 +196,11 @@ static void __secure sunxi_cpu_set_locking(int cpu, bool
lock)
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ /* Not required on R528 */
+ return;
+ }
+
if (lock)
clrbits_le32(>dbg_ctrl1, BIT(cpu));
else
@@ -178,6 +212,11 @@ static bool __secure sunxi_cpu_poll_wfi(int cpu)
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ return !!(readl(SUNXI_CPUX_BASE + SUN8I_R528_C0_CPU_STATUS) &
+ BIT(SUN8I_R528_C0_STATUS_STANDBYWFI + cpu));
+ }
+
return !!(readl(>cpu[cpu].status) & BIT(2));
}
@@ -186,6 +225,12 @@ static void __secure sunxi_cpu_invalidate_cache(int cpu)
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+ clrbits_le32(SUNXI_CPUX_BASE + SUN8I_R528_C0_CTRL_REG0,
+BIT(cpu));
+ return;
+ }
+
clrbits_le32(>gen_ctrl, BIT(cpu));
}
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 0a3454a51a..d46fd8c0bc 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -355,6 +355,8 @@ config MACH_S
[PATCH 2/3] sunxi: psci: refactor register access to separate functions
This is to prepare for R528, which does not have the typical
"CPUCFG" block; it has a "CPUX" block which provides these
same functions but is organized differently.
Moving the hardware-access bits to their own functions separates the
logic from the hardware so we can reuse the same logic.
Signed-off-by: Sam Edwards
---
arch/arm/cpu/armv7/sunxi/psci.c | 66 +++--
1 file changed, 47 insertions(+), 19 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 7809b074f8..94120e7526 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -114,7 +114,7 @@ static void __secure sunxi_power_switch(u32 *clamp, u32
*pwroff, bool on,
}
}
-static void __secure sunxi_set_entry_address(void *entry)
+static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
{
/* secondary core entry address is programmed differently on R40 */
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
@@ -154,30 +154,60 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
}
}
-void __secure sunxi_cpu_power_off(u32 cpuid)
+static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ writel(reset ? 0b00 : 0b11, >cpu[cpu].rst);
+}
+
+static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
{
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ if (lock)
+ clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ else
+ setbits_le32(>dbg_ctrl1, BIT(cpu));
+}
+
+static bool __secure sunxi_cpu_poll_wfi(int cpu)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ return !!(readl(>cpu[cpu].status) & BIT(2));
+}
+
+static void __secure sunxi_cpu_invalidate_cache(int cpu)
+{
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+ clrbits_le32(>gen_ctrl, BIT(cpu));
+}
+
+void __secure sunxi_cpu_power_off(u32 cpuid)
+{
u32 cpu = cpuid & 0x3;
/* Wait for the core to enter WFI */
- while (1) {
- if (readl(>cpu[cpu].status) & BIT(2))
- break;
+ while (!sunxi_cpu_poll_wfi(cpu))
__mdelay(1);
- }
/* Assert reset on target CPU */
- writel(0, >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, true);
/* Lock CPU (Disable external debug access) */
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ sunxi_cpu_set_locking(cpu, true);
/* Power down CPU */
sunxi_cpu_set_power(cpuid, false);
- /* Unlock CPU (Disable external debug access) */
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ /* Unlock CPU (Reenable external debug access) */
+ sunxi_cpu_set_locking(cpu, false);
}
static u32 __secure cp15_read_scr(void)
@@ -234,33 +264,31 @@ out:
int __secure psci_cpu_on(u32 __always_unused unused, u32 mpidr, u32 pc,
u32 context_id)
{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
u32 cpu = (mpidr & 0x3);
/* store target PC and context id */
psci_save(cpu, pc, context_id);
/* Set secondary core power on PC */
- sunxi_set_entry_address(_cpu_entry);
+ sunxi_cpu_set_entry(cpu, _cpu_entry);
/* Assert reset on target CPU */
- writel(0, >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, true);
/* Invalidate L1 cache */
- clrbits_le32(>gen_ctrl, BIT(cpu));
+ sunxi_cpu_invalidate_cache(cpu);
/* Lock CPU (Disable external debug access) */
- clrbits_le32(>dbg_ctrl1, BIT(cpu));
+ sunxi_cpu_set_locking(cpu, true);
/* Power up target CPU */
sunxi_cpu_set_power(cpu, true);
/* De-assert reset on target CPU */
- writel(BIT(1) | BIT(0), >cpu[cpu].rst);
+ sunxi_cpu_set_reset(cpu, false);
- /* Unlock CPU (Disable external debug access) */
- setbits_le32(>dbg_ctrl1, BIT(cpu));
+ /* Unlock CPU (Reenable external debug access) */
+ sunxi_cpu_set_locking(cpu, false);
return ARM_PSCI_RET_SUCCESS;
}
--
2.41.0
[PATCH 1/3] sunxi: psci: clean away preprocessor macros
This patch restructures psci.c to get away from the "many different
function definitions switched by #ifdef" paradigm to the preferred style
of having a single function definition with `if (IS_ENABLED(...))` to
make the optimizer include only the appropriate function bodies instead.
There are no functional changes here.
Signed-off-by: Sam Edwards
---
arch/arm/cpu/armv7/sunxi/psci.c | 94 ++---
1 file changed, 41 insertions(+), 53 deletions(-)
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index e1d3638b5c..7809b074f8 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -76,28 +76,24 @@ static void __secure __mdelay(u32 ms)
isb();
}
-static void __secure clamp_release(u32 __maybe_unused *clamp)
+static void __secure clamp_release(u32 *clamp)
{
-#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
- defined(CONFIG_MACH_SUN8I_H3) || \
- defined(CONFIG_MACH_SUN8I_R40)
- u32 tmp = 0x1ff;
- do {
- tmp >>= 1;
- writel(tmp, clamp);
- } while (tmp);
-
- __mdelay(10);
-#endif
+ if (clamp) {
+ u32 tmp = 0x1ff;
+ do {
+ tmp >>= 1;
+ writel(tmp, clamp);
+ } while (tmp);
+
+ __mdelay(10);
+ }
}
-static void __secure clamp_set(u32 __maybe_unused *clamp)
+static void __secure clamp_set(u32 *clamp)
{
-#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
- defined(CONFIG_MACH_SUN8I_H3) || \
- defined(CONFIG_MACH_SUN8I_R40)
- writel(0xff, clamp);
-#endif
+ if (clamp) {
+ writel(0xff, clamp);
+ }
}
static void __secure sunxi_power_switch(u32 *clamp, u32 *pwroff, bool on,
@@ -118,53 +114,45 @@ static void __secure sunxi_power_switch(u32 *clamp, u32
*pwroff, bool on,
}
}
-#ifdef CONFIG_MACH_SUN8I_R40
-/* secondary core entry address is programmed differently on R40 */
static void __secure sunxi_set_entry_address(void *entry)
{
- writel((u32)entry,
- SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
-}
-#else
-static void __secure sunxi_set_entry_address(void *entry)
-{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+ /* secondary core entry address is programmed differently on R40 */
+ if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
+ writel((u32)entry,
+ SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
+ } else {
+ struct sunxi_cpucfg_reg *cpucfg =
+ (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
- writel((u32)entry, >priv0);
+ writel((u32)entry, >priv0);
+ }
}
-#endif
-#ifdef CONFIG_MACH_SUN7I
-/* sun7i (A20) is different from other single cluster SoCs */
-static void __secure sunxi_cpu_set_power(int __always_unused cpu, bool on)
-{
- struct sunxi_cpucfg_reg *cpucfg =
- (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
- sunxi_power_switch(>cpu1_pwr_clamp, >cpu1_pwroff,
- on, 0);
-}
-#elif defined CONFIG_MACH_SUN8I_R40
static void __secure sunxi_cpu_set_power(int cpu, bool on)
{
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
- sunxi_power_switch((void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu),
- (void *)cpucfg + SUN8I_R40_PWROFF,
- on, cpu);
-}
-#else /* ! CONFIG_MACH_SUN7I && ! CONFIG_MACH_SUN8I_R40 */
-static void __secure sunxi_cpu_set_power(int cpu, bool on)
-{
- struct sunxi_prcm_reg *prcm =
- (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
+ /* sun7i (A20) is different from other single cluster SoCs */
+ if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
+ sunxi_power_switch(NULL, >cpu1_pwroff, on, 0);
+ } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
+ sunxi_power_switch(NULL, (void *)cpucfg + SUN8I_R40_PWROFF,
+ on, cpu);
+ } else {
+#if !defined(CONFIG_SUN50I_GEN_H6) && !defined(CONFIG_SUNXI_GEN_NCAT2)
+ struct sunxi_prcm_reg *prcm =
+ (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
- sunxi_power_switch(>cpu_pwr_clamp[cpu], >cpu_pwroff,
- on, cpu);
+ u32 *clamp = >cpu_pwr_clamp[cpu];
+ if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
+ IS_ENABLED(CONFIG_MACH_SUN8I_H3))
+ clamp = NULL;
+
+ sunxi_power_switch(clamp, >cpu_pwroff, on, cpu);
+#endif
+ }
}
-#endif /* CONFIG_MACH_SUN7I */
void __secure sunxi_cpu_power_off(u32 cpuid)
{
--
2.41.0
[PATCH 0/3] Allwinner R528/T113s PSCI
Hi list, With Andre's R528 support (hopefully) landing soon, I figure now is time to submit these patches for review. They refactor the ARMv7/sunxi PSCI implementation to allow for the new NCAT2 CPU complex control registers, and add support for the R528/T113s. Depends on series 365063. Thanks, Sam Sam Edwards (3): sunxi: psci: clean away preprocessor macros sunxi: psci: refactor register access to separate functions sunxi: psci: implement PSCI on R528 arch/arm/cpu/armv7/sunxi/psci.c | 185 +--- arch/arm/mach-sunxi/Kconfig | 2 + include/configs/sunxi-common.h | 8 ++ 3 files changed, 133 insertions(+), 62 deletions(-) -- 2.41.0
[RFC PATCH 4/4] HACK: enable access to `ubi 0:volname` block devices
---
disk/part.c | 56 +
1 file changed, 56 insertions(+)
diff --git a/disk/part.c b/disk/part.c
index 1ad8277b65..85eb51429a 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -14,6 +14,9 @@
#include
#include
#include
+#include
+#include
+#include
#undef PART_DEBUG
@@ -505,6 +508,59 @@ int blk_get_device_part_str(const char *ifname, const char
*dev_part_str,
}
#endif
+#if IS_ENABLED(CONFIG_CMD_UBI) && !IS_ENABLED(CONFIG_SPL_BUILD)
+ /*
+* Also special-case UBI, which may use names to find the specific
+* volumes, so this deviates a bit from the typical devnum:partnum
+* syntax.
+*/
+ if (!strcmp(ifname, "ubi")) {
+ dev = hextoul(dev_part_str, );
+ if (*ep == ':') {
+ struct udevice *ubi_dev = NULL;
+ struct udevice *vol_dev = NULL;
+ part_str = [1];
+
+ ret = uclass_find_device(UCLASS_UBI, dev, _dev);
+ if (!ubi_dev || ret) {
+ printf("** Cannot find UBI %x\n", dev);
+ return -EINVAL;
+ }
+
+ part = hextoul(part_str, );
+ if (!*ep) {
+ struct udevice *tmp_dev;
+ device_foreach_child(tmp_dev, ubi_dev) {
+ struct blk_desc *desc =
dev_get_uclass_plat(tmp_dev);
+ if (desc->devnum == part) {
+ vol_dev = tmp_dev;
+ break;
+ }
+ }
+ }
+
+ if (!vol_dev)
+ ret = device_find_child_by_name(ubi_dev,
part_str, _dev);
+
+ if (!vol_dev || ret) {
+ printf("** UBI volume %s not found\n",
part_str);
+ return -EINVAL;
+ }
+
+ ret = device_probe(vol_dev);
+ if (ret)
+ return ret;
+
+ *dev_desc = dev_get_uclass_plat(vol_dev);
+ part_get_info_whole_disk(*dev_desc, info);
+ return 0;
+ }
+
+ printf("UBIFS not mounted, use ubifsmount to mount volume
first!\n");
+ return -EINVAL;
+ }
+#endif
+
/* If no dev_part_str, use bootdevice environment variable */
if (!dev_part_str || !strlen(dev_part_str) ||
!strcmp(dev_part_str, "-"))
--
2.41.0
[RFC PATCH 3/4] disk: part: fall-through if "ubi" requested but ubifs not mounted
Since we're adding the ability to access static UBI volumes as block
devices, it is no longer an error to use the "ubi" ifname with UBIFS
unmounted.
Ideally, the access to UBIFS should instead be called "ubifs" but it
would break backwards compatibility to change this. Instead, use the
UBIFS mount status to disambiguate what the user means by "ubi"
There is no change in functionality in this patch: UBIFS access works
the same and an error still occurs when using "ubi" without UBIFS
mounted. The only difference is that now, the error message is a plain
"Bad device specification" and does not suggest using ubifsmount.
Signed-off-by: Sam Edwards
---
disk/part.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/disk/part.c b/disk/part.c
index 0a03b8213d..1ad8277b65 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -492,15 +492,13 @@ int blk_get_device_part_str(const char *ifname, const
char *dev_part_str,
#if IS_ENABLED(CONFIG_CMD_UBIFS) && !IS_ENABLED(CONFIG_SPL_BUILD)
/*
-* Special-case ubi, ubi goes through a mtd, rather than through
-* a regular block device.
+* Special-case ubifs, which does not go through the block device layer
+* and also must be mounted ahead of time. U-Boot has historically
+* called this "ubi" too, even though *static* UBI volumes are
+* accessible as block devices. For backward compatibility, assume that
+* when UBIFS is mounted, the user intends "ubi" to mean "ubifs."
*/
- if (!strcmp(ifname, "ubi")) {
- if (!ubifs_is_mounted()) {
- printf("UBIFS not mounted, use ubifsmount to mount
volume first!\n");
- return -EINVAL;
- }
-
+ if (ubifs_is_mounted() && !strcmp(ifname, "ubi")) {
strcpy((char *)info->type, BOOT_PART_TYPE);
strcpy((char *)info->name, "UBI");
return 0;
--
2.41.0
[RFC PATCH 2/4] mtd: ubi: bind block device driver for static volumes
This makes static UBI volumes readable as block devices, however
no mechanism for selecting these volume devices yet exists.
Signed-off-by: Sam Edwards
---
drivers/mtd/ubi/ubi-uclass.c | 110 +++
1 file changed, 110 insertions(+)
diff --git a/drivers/mtd/ubi/ubi-uclass.c b/drivers/mtd/ubi/ubi-uclass.c
index f8971e793e..231d6d90c7 100644
--- a/drivers/mtd/ubi/ubi-uclass.c
+++ b/drivers/mtd/ubi/ubi-uclass.c
@@ -8,10 +8,119 @@
#define LOG_CATEGORY UCLASS_UBI
#include
+#include
#include
#include
#include
+static ulong ubi_bread(struct udevice *dev, lbaint_t lba, lbaint_t blkcnt,
+ void *dst)
+{
+ int err, lnum;
+ struct blk_desc *blk = dev_get_uclass_plat(dev);
+ struct ubi_device *ubi = dev_get_plat(dev->parent);
+ struct ubi_volume *vol = ubi->volumes[blk->devnum];
+ lbaint_t lba_per_peb = vol->usable_leb_size / blk->blksz;
+ lbaint_t lba_off, lba_len, total = 0;
+
+ while (blkcnt) {
+ lnum = lba / lba_per_peb;
+ lba_off = lba % lba_per_peb;
+ lba_len = lba_per_peb - lba_off;
+ if (lba_len > blkcnt)
+ lba_len = blkcnt;
+
+ err = ubi_eba_read_leb(ubi, vol, lnum, dst,
+ lba_off << blk->log2blksz,
+ lba_len << blk->log2blksz, 0);
+ if (err) {
+ pr_err("UBI read error %x\n", err);
+ break;
+ }
+
+ lba += lba_len;
+ blkcnt -= lba_len;
+ dst += lba_len << blk->log2blksz;
+ total += lba_len;
+ }
+
+ return total;
+}
+
+static const struct blk_ops ubi_block_ops = {
+ .read = ubi_bread,
+};
+
+U_BOOT_DRIVER(ubi_block) = {
+ .name = "ubi_block",
+ .id = UCLASS_BLK,
+ .ops= _block_ops,
+};
+
+static bool is_power_of_two(unsigned int x)
+{
+ return (x & -x) == x;
+}
+
+static unsigned int choose_blksz_for_volume(const struct ubi_volume *vol)
+{
+ /*
+* U-Boot assumes a power-of-two blksz; however, UBI LEBs are
+* very often not suitably sized. To solve this, we divide the
+* LEBs into a whole number of LBAs per LEB, such that each LBA
+* addresses a power-of-two-sized block. To choose the blksz,
+* we either:
+* 1) Use the volume alignment, if it's a non-unity power of
+*two. The LEB size is a multiple of this alignment, and it
+*allows the user to force a particular blksz if needed for
+*their use case.
+* 2) Otherwise, find the greatest power-of-two factor of the
+*LEB size.
+*/
+ if (vol->alignment > 1 && is_power_of_two(vol->alignment))
+ return vol->alignment;
+
+ unsigned int blksz = 1;
+ while ((vol->usable_leb_size & blksz) == 0)
+ blksz <<= 1;
+ return blksz;
+}
+
+static int ubi_post_bind(struct udevice *dev)
+{
+ int i;
+ int ret;
+ unsigned int blksz;
+ lbaint_t lba;
+ struct ubi_device *ubi = dev_get_plat(dev);
+
+ for (i = 0; i < ubi->vtbl_slots; i++) {
+ struct ubi_volume *vol = ubi->volumes[i];
+ if (!vol || vol->vol_id >= UBI_INTERNAL_VOL_START ||
+ vol->vol_type != UBI_STATIC_VOLUME)
+ continue;
+
+ if (vol->updating || vol->upd_marker) {
+ pr_err("** UBI volume %d (\"%s\") midupdate; ignored\n",
+ vol->vol_id, vol->name);
+ continue;
+ }
+
+ blksz = choose_blksz_for_volume(vol);
+ lba = DIV_ROUND_UP((unsigned long long)vol->used_bytes, blksz);
+
+ pr_debug("UBI volume %d (\"%s\"): %lu blocks, %d bytes each\n",
+vol->vol_id, vol->name, lba, blksz);
+
+ ret = blk_create_device(dev, "ubi_block", vol->name, UCLASS_UBI,
+ vol->vol_id, blksz, lba, NULL);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
int ubi_dm_bind(unsigned int index)
{
struct udevice *dev;
@@ -71,4 +180,5 @@ U_BOOT_DRIVER(ubi) = {
UCLASS_DRIVER(ubi) = {
.id = UCLASS_UBI,
.name = "ubi",
+ .post_bind = ubi_post_bind,
};
--
2.41.0
[RFC PATCH 1/4] mtd: ubi: register UBI attachments as DM devices
This is in preparation for exposing static UBI volumes as block devices.
A UBI uclass and driver are introduced, and a "ubi0" virtual device
with the proper driver is created below whichever MTD device is
attached as the active UBI partition. This virtual device will soon
be the parent for the BLK devices that represent the static volumes.
Signed-off-by: Sam Edwards
---
cmd/ubi.c| 11 ++
drivers/mtd/ubi/Makefile | 1 +
drivers/mtd/ubi/ubi-uclass.c | 74
include/dm/uclass-id.h | 1 +
include/ubi_uboot.h | 5 +++
5 files changed, 92 insertions(+)
create mode 100644 drivers/mtd/ubi/ubi-uclass.c
diff --git a/cmd/ubi.c b/cmd/ubi.c
index b61ae1efea..ab336d30f1 100644
--- a/cmd/ubi.c
+++ b/cmd/ubi.c
@@ -560,6 +560,13 @@ static int ubi_detach(void)
cmd_ubifs_umount();
#endif
+#ifdef CONFIG_DM_MTD
+ /*
+* Clean up any UBI devices in DM
+*/
+ ubi_dm_unbind_all();
+#endif
+
/*
* Call ubi_exit() before re-initializing the UBI subsystem
*/
@@ -598,6 +605,10 @@ int ubi_part(char *part_name, const char
*vid_header_offset)
return err;
}
+#ifdef CONFIG_DM_MTD
+ ubi_dm_bind(0);
+#endif
+
ubi = ubi_devices[0];
return 0;
diff --git a/drivers/mtd/ubi/Makefile b/drivers/mtd/ubi/Makefile
index 30d00fbdfe..375075f75e 100644
--- a/drivers/mtd/ubi/Makefile
+++ b/drivers/mtd/ubi/Makefile
@@ -7,3 +7,4 @@ obj-y += attach.o build.o vtbl.o vmt.o upd.o kapi.o eba.o io.o
wl.o crc32.o
obj-$(CONFIG_MTD_UBI_FASTMAP) += fastmap.o
obj-y += misc.o
obj-y += debug.o
+obj-$(CONFIG_DM_MTD) += ubi-uclass.o
diff --git a/drivers/mtd/ubi/ubi-uclass.c b/drivers/mtd/ubi/ubi-uclass.c
new file mode 100644
index 00..f8971e793e
--- /dev/null
+++ b/drivers/mtd/ubi/ubi-uclass.c
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0
+/**
+ * ubi-uclass.c - UBI partition and volume block device uclass driver
+ *
+ * Copyright (C) 2023 Sam Edwards
+ */
+
+#define LOG_CATEGORY UCLASS_UBI
+
+#include
+#include
+#include
+#include
+
+int ubi_dm_bind(unsigned int index)
+{
+ struct udevice *dev;
+ int ret;
+ char name[16];
+ const char *name_dup;
+ struct ubi_device *ubi = ubi_devices[index];
+ const struct mtd_info *mtd = ubi->mtd;
+
+ /* MTD partitions are not in DM; navigate to the real MTD device */
+ if (mtd->parent)
+ mtd = mtd->parent;
+
+ snprintf(name, sizeof(name), "ubi%u", index);
+ name_dup = strdup(name);
+ ret = device_bind(mtd->dev, DM_DRIVER_GET(ubi), name_dup, ubi,
+ ofnode_null(), );
+ if (ret) {
+ free((void *)name_dup);
+ return ret;
+ }
+
+ device_set_name_alloced(dev);
+
+ return 0;
+}
+
+int ubi_dm_unbind_all(void)
+{
+ int ret;
+ struct uclass *uc;
+ struct udevice *dev;
+ struct udevice *next;
+
+ ret = uclass_get(UCLASS_UBI, );
+ if (ret)
+ return ret;
+
+ uclass_foreach_dev_safe(dev, next, uc) {
+ ret = device_remove(dev, DM_REMOVE_NORMAL);
+ if (ret)
+ return ret;
+
+ ret = device_unbind(dev);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+U_BOOT_DRIVER(ubi) = {
+ .id = UCLASS_UBI,
+ .name = "ubi_dev",
+};
+
+UCLASS_DRIVER(ubi) = {
+ .id = UCLASS_UBI,
+ .name = "ubi",
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 307ad6931c..407166d080 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -133,6 +133,7 @@ enum uclass_id {
UCLASS_THERMAL, /* Thermal sensor */
UCLASS_TIMER, /* Timer device */
UCLASS_TPM, /* Trusted Platform Module TIS interface */
+ UCLASS_UBI, /* Unsorted Block Images MTD partition */
UCLASS_UFS, /* Universal Flash Storage */
UCLASS_USB, /* USB bus */
UCLASS_USB_DEV_GENERIC, /* USB generic device */
diff --git a/include/ubi_uboot.h b/include/ubi_uboot.h
index 6da348eb62..9d37848f03 100644
--- a/include/ubi_uboot.h
+++ b/include/ubi_uboot.h
@@ -52,6 +52,11 @@ extern int ubi_part(char *part_name, const char
*vid_header_offset);
extern int ubi_volume_write(char *volume, void *buf, size_t size);
extern int ubi_volume_read(char *volume, char *buf, size_t size);
+#ifdef CONFIG_DM_MTD
+extern int ubi_dm_bind(unsigned int);
+extern int ubi_dm_unbind_all(void);
+#endif
+
extern struct ubi_device *ubi_devices[];
int cmd_ubifs_mount(char *vol_name);
int cmd_ubifs_umount(void);
--
2.41.0
[RFC PATCH 0/4] mtd: ubi: Enable accessing RO filesystems in UBI vols
Hi UBI maintainers,
My target's rootfs is a read-only filesystem stored in a static UBI volume,
mounted via a "ubiblock" device after boot. I'd also like to keep the boot
files in the same filesystem, so that it's all coupled together. To that end,
I'm working on a patchset so that U-Boot can read static UBI volumes as
read-only block devices (paralleling Linux's ubiblock mechanism).
I'm very happy with how the first 3 patches in this series turned out (so I'm
not asking about them per se, though feedback is certainly welcome). The fourth
is where I got stuck: while the code definitely works, it requires bringing DM
headers into disk/part.c, which certainly will not fly in mainline. I need to
plumb this through drivers/block/blk-uclass.c to do it "properly."
Part of the problem here is that these are *volumes,* but they are (optionally)
*named.* In U-Boot's current view of block devices, it is only a partition, and
not a volume, that may have a name. These aren't "partitions" in U-Boot's view,
since a partition is a contiguous slice of a bigger block device, while these
are (logically) separate block devices.
So, I would need to invent a new function that can look up a named (sub)volume.
I also probably need to invent new syntax, so that I'm not overloading the 0:1
syntax for partitions. I'm not especially beholden to the ':', but I do want to
use the same syntax for volume numbers and volume names, to mirror Linux's
`ubi.block=x,y` syntax.
I'm also trying to reclaim the name "ubi" to refer to a UBI volume, while
U-Boot currently thinks it should refer to the presently-mounted UBIFS. In my
current series, the meaning of "ubi" depends on whether ubifs is mounted, for
backwards-compatibility. If this isn't palpable, I could consider other options
like "ubivol"/"ubiblock"/"ubiblk"/"ubistatic"/...
So, the feedback I'm hoping for would be:
1) What is a good syntax for referring to a logical volume by name or ID?
Keeping in mind this may affect more than just UBI, if e.g. U-Boot learns to
peer inside LVM2s in the future.
2) What should I call the block functions for looking up a block device's
subvolume by type+parentidx+{name,ID}?
3) Is my strategy of reclaiming "ubi" sound, or should I be conceding that to
UBIFS and using a new type name for static UBI volumes?
4) Does my choose_blksz_for_volume() function make sense, or should I always be
using a preferred block size (like 512) if possible?
Cheers,
Sam
Sam Edwards (4):
mtd: ubi: register UBI attachments as DM devices
mtd: ubi: bind block device driver for static volumes
disk: part: fall-through if "ubi" requested but ubifs not mounted
HACK: enable access to `ubi 0:volname` block devices
cmd/ubi.c| 11 +++
disk/part.c | 70 +++--
drivers/mtd/ubi/Makefile | 1 +
drivers/mtd/ubi/ubi-uclass.c | 184 +++
include/dm/uclass-id.h | 1 +
include/ubi_uboot.h | 5 +
6 files changed, 264 insertions(+), 8 deletions(-)
create mode 100644 drivers/mtd/ubi/ubi-uclass.c
--
2.41.0
Re: [PATCH 09/20] pinctrl: sunxi: add Allwinner D1 pinctrl description
On 7/21/23 07:45, Andre Przywara wrote:
diff --git a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
index fc80fe50b14..c6115112688 100644
--- a/drivers/pinctrl/sunxi/pinctrl-sunxi.c
+++ b/drivers/pinctrl/sunxi/pinctrl-sunxi.c
@@ -748,6 +748,28 @@ static const struct sunxi_pinctrl_desc __maybe_unused
sun50i_h616_r_pinctrl_desc
.num_banks = 1,
};
+static const struct sunxi_pinctrl_function sun20i_d1_pinctrl_functions[] = {
+ { "emac", 8 },/* PE0-PE15 */
+ { "gpio_in", 0 },
+ { "gpio_out", 1 },
+ { "mmc0", 2 },/* PF0-PF5 */
+ { "mmc2", 3 },/* PC1-PC7 */
+ { "spi0", 2 },/* PC2-PC7 */
+#if IS_ENABLED(CONFIG_UART0_PORT_F)
+ { "uart0",3 },/* PF2-PF4 */
/* PF2, PF4 */ ?
+#else
+ { "uart0",6 },/* PB2-PB3 */
/* PB0-PB1 */ ?
+#endif
+ { "uart3",7 },/* PB6-PB9 */
/* PB6-PB7 */ ?
+};
Cheers,
Sam
Re: [PATCH 19/20] ARM: dts: sunxi: add Allwinner T113-s SoC .dtsi
On 7/21/23 07:46, Andre Przywara wrote:
The Allwinner T113-s SoC is apparently using the same (or at least a very
similar) die as the D1/D1s, but replaces the single RISC-V core with
two Arm Cortex-A7 cores.
Since the D1 core .dtsi already describes all common peripherals, we
just need a DT describing the ARM specific peripherals: the CPU cores,
the Generic Timer, the GIC and the PMU.
We include the core .dtsi directly from the riscv DT directory.
The ARM core version of the DT specifies the CPUX watchdog as
"reserved", which means it won't be recognised by U-Boot. Override this
in our generic sunxi-u-boot.dtsi, to let U-Boot pick up this watchdog,
so that the generic reset driver will work.
Signed-off-by: Andre Przywara
Reviewed-by: Sam Edwards
Tested-by: Sam Edwards
diff --git a/arch/arm/dts/sunxi-u-boot.dtsi b/arch/arm/dts/sunxi-u-boot.dtsi
index af419c7e590..a0c8abb7033 100644
--- a/arch/arm/dts/sunxi-u-boot.dtsi
+++ b/arch/arm/dts/sunxi-u-boot.dtsi
@@ -23,6 +23,13 @@
};
};
+/* Let U-Boot be the firmware layer that controls the watchdog. */
+#ifdef CONFIG_MACH_SUN8I_R528
+ {
+ status = "okay";
+};
+#endif
+
{
u-boot-sunxi-with-spl {
filename = "u-boot-sunxi-with-spl.bin";
Nice!!
Thanks,
Sam
Re: [PATCH 08/20] sunxi: introduce NCAT2 generation model
On 7/21/23 07:45, Andre Przywara wrote: Allwinner seems to typically stick to a common MMIO memory map for several SoCs, but from time to time does some breaking changes, which also introduce new generations of some peripherals. The last time this happened with the H6, which apart from re-organising the base addresses also changed the clock controller significantly. We added a CONFIG_SUN50I_GEN_H6 symbol back then to mark SoCs sharing those traits. Now the Allwinner D1 changes the memory map again, and also extends the pincontroller, among other peripherals. To mark this generation of SoCs, add a CONFIG_SUNXI_GEN_NCAT2 symbol, this name is reportedly used in the Allwinner BSP code, and prevents us from inventing our own name. Add this new symbol to some guards that were already checking for the H6 generation, since many features are shared between the two (like the renovated clock controller). This paves the way to introduce a first user of this generation. Signed-off-by: Andre Przywara Reviewed-by: Sam Edwards Tested-by: Sam Edwards Thanks, Sam
Re: [PATCH 07/20] pinctrl: sunxi: add new D1 pinctrl support
On 7/21/23 07:45, Andre Przywara wrote: For the first time since at least the Allwinner A10 SoCs, the D1 (and related cores) use a new pincontroller MMIO register layout, so we cannot use our hardcoded, fixed offsets anymore. Ideally this would all be handled by devicetree and DM drivers, but for the DT-less SPL we still need the legacy interfaces. Add a new Kconfig symbol to differenciate between the two generations of pincontrollers, and just use that to just switch some basic symbols. The rest is already abstracted enough, so works out of the box. Signed-off-by: Andre Przywara Reviewed-by: Sam Edwards Tested-by: Sam Edwards Thanks, Sam
Re: [PATCH 03/20] pinctrl: sunxi: add GPIO in/out wrappers
On 7/21/23 07:45, Andre Przywara wrote:
So far we were open-coding the pincontroller's GPIO output/input access
in each function using that.
Provide functions that wrap that nicely, and follow the existing pattern
(set/get_{bank,}), so users don't need to know about the internals, and
we can abstract the new D1 pinctrl more easily.
Signed-off-by: Andre Przywara
Reviewed-by: Sam Edwards
Tested-by: Sam Edwards
Thanks,
Sam
Re: [PATCH 02/20] sunxi: remove CONFIG_MACPWR
Hi Andre, Ditto my response to the previous patch. On 7/21/23 07:45, Andre Przywara wrote: The CONFIG_MACPWR Kconfig symbol is used to point to a GPIO that enables the power for the Ethernet "MAC" (mostly PHY, really). In the DT this is described with the phy-supply property in the MAC DT node, pointing to a (GPIO controlled) regulator. Since we need Ethernet only in U-Boot proper, and use a DM driver there, we should use the DT instead of hardcoding this. Add code to the sun8i_emac and sunxi_emac drivers to check the DT for that regulator and enable it, at probe time. Then drop the current code from board.c, which was doing that job before. This allows us to remove the MACPWR Kconfig definition and the respective values from the defconfigs. Signed-off-by: Andre Przywara Reviewed-by: Sam Edwards Thanks, Sam
Re: [PATCH 01/20] net: sunxi_emac: chase DT nodes to find PHY regulator
Hi Andre, Looks good to me! My target doesn't have PHY on a regulator, so no test result for this one. On 7/21/23 07:45, Andre Przywara wrote: At the moment the sun4i EMAC driver relies on hardcoded CONFIG_MACPWR Kconfig symbols to enable potential PHY regulators. As we want to get rid of those, we need to find the regulator by chasing up the DT. The sun4i-emac binding puts the PHY regulator into the MDIO node, which is the parent of the PHY device. U-Boot does not have (and does not need) an MDIO driver, so we need to chase down the regulator through the EMAC node: we follow the "phy-handle" property to find the PHY node, then go up to its parent, where we find the "phy-supply" link to the regulator. Let U-Boot find the associated regulator device, and put that into the private device struct, so we can find and enable the regulator at probe time, later. Signed-off-by: Andre Przywara Reviewed-by: Sam Edwards Thanks, Sam
Re: [PATCH 15/20] sunxi: add R528/T113-s3/D1(s) DRAM initialisation code
I've had countless successful boots of a T113-s3 using this DRAM controller code, so: On 7/21/23 07:46, Andre Przywara wrote: Signed-off-by: Andre Przywara Tested-by: Sam Edwards
Re: [PATCH] fs/erofs: Remove an unnecessary assertion
Thanks greatly for the speedy turnaround on this patch!
On 7/25/23 22:56, Yifan Zhao wrote:
Fixes: 3a21e92fc255 ("fs/erofs: Introduce new features including ztailpacking,
fragments and dedupe")
Signed-off-by: Yifan Zhao
Tested-by: Sam Edwards
Re: [PATCH] sunxi: remove CONFIG_SATAPWR
On 7/26/23 17:47, Andre Przywara wrote:
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b3115b054c8..422cc01e529 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1158,6 +1158,8 @@ config ARCH_SUNXI
imply CMD_GPT
imply CMD_UBI if MTD_RAW_NAND
imply DISTRO_DEFAULTS
+ imply DM_REGULATOR
+ imply DM_REGULATOR_FIXED
imply FAT_WRITE
imply FIT
imply OF_LIBFDT_OVERLAY
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index e20c3a3ee92..a7c5ae80a1e 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -1008,14 +1008,6 @@ config VIDEO_LCD_TL059WV5C0
endchoice
-config SATAPWR
- string "SATA power pin"
- default ""
- help
- Set the pins used to power the SATA. This takes a string in the
- format understood by sunxi_name_to_gpio, e.g. PH1 for pin 1 of
- port H.
-
config GMAC_TX_DELAY
int "GMAC Transmit Clock Delay Chain"
default 0
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index f321cd58a6e..7ac50c4ae8c 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -187,7 +187,7 @@ enum env_location env_get_location(enum env_operation op,
int prio)
/* add board specific code here */
int board_init(void)
{
- __maybe_unused int id_pfr1, ret, satapwr_pin, macpwr_pin;
+ __maybe_unused int id_pfr1, ret, macpwr_pin;
gd->bd->bi_boot_params = (PHYS_SDRAM_0 + 0x100);
@@ -225,20 +225,6 @@ int board_init(void)
return ret;
/* strcmp() would look better, but doesn't get optimised away. */
- if (CONFIG_SATAPWR[0]) {
- satapwr_pin = sunxi_name_to_gpio(CONFIG_SATAPWR);
- if (satapwr_pin >= 0) {
- gpio_request(satapwr_pin, "satapwr");
- gpio_direction_output(satapwr_pin, 1);
-
- /*
-* Give the attached SATA device time to power-up
-* to avoid link timeouts
-*/
- mdelay(500);
- }
- }
-
if (CONFIG_MACPWR[0]) {
macpwr_pin = sunxi_name_to_gpio(CONFIG_MACPWR);
if (macpwr_pin >= 0) {
...
diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c
index 94a3379c532..9064774e661 100644
--- a/drivers/ata/ahci_sunxi.c
+++ b/drivers/ata/ahci_sunxi.c
@@ -7,6 +7,7 @@
#include
#include
#include
+#include
#define AHCI_PHYCS0R 0x00c0
#define AHCI_PHYCS1R 0x00c4
@@ -74,6 +75,7 @@ static int sunxi_ahci_phy_init(u8 *reg_base)
static int sunxi_sata_probe(struct udevice *dev)
{
+ struct udevice *reg_dev;
ulong base;
u8 *reg;
int ret;
@@ -89,6 +91,13 @@ static int sunxi_sata_probe(struct udevice *dev)
debug("%s: Failed to init phy (err=%d)\n", __func__, ret);
return ret;
}
+
+ ret = device_get_supply_regulator(dev, "target-supply", _dev);
+ if (ret == 0) {
+ regulator_set_enable(reg_dev, true);
+ mdelay(500);
+ }
+
ret = ahci_probe_scsi(dev, base);
if (ret) {
debug("%s: Failed to probe (err=%d)\n", __func__, ret);
Love these sorts of clean-ups. I don't have a sunxi with SATA so I can't
test it, but I've been running my target on this patch in some form or
another for several weeks, and the code looks good, so:
Reviewed-by: Sam Edwards
[PATCH] i2c: mvtwsi: reset controller if stuck in "bus error" state
The MVTWSI controller can act either as a master or slave device. When
acting as a master, the FSM is driven by the CPU. As a slave, the FSM is
driven by the bus directly. In what is (apparently) a safety mechanism,
if the bus transitions our FSM in any improper way, the FSM goes to a
"bus error" state (0x00). I could find no documented or experimental way
to get the FSM out of this state, except for a controller reset.
Since U-Boot only uses the MVTWSI controller as a bus master, this
feature only gets in the way: we do not care what happened on the bus
previously as long as the bus is ready for a new transaction. So, when
trying to start a new transaction, check for this state and reset the
controller if necessary.
Note that this should not be confused with the "deblocking" technique
(used by the `i2c reset` command), which involves pulsing SCL repeatedly
if SDA is found to be held low, in an attempt to force the bus back to
an idle state. This patch only resets the controller in case something
else had previously upset it, and (in principle) results in no
externally-observable change in behavior.
Signed-off-by: Sam Edwards
---
drivers/i2c/mvtwsi.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index 93bbc6916e..14cdb0f663 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -142,6 +142,8 @@ enum mvtwsi_ctrl_register_fields {
* code.
*/
enum mvstwsi_status_values {
+ /* Protocol violation on bus; this is a terminal state */
+ MVTWSI_BUS_ERROR= 0x00,
/* START condition transmitted */
MVTWSI_STATUS_START = 0x08,
/* Repeated START condition transmitted */
@@ -525,6 +527,36 @@ static void __twsi_i2c_init(struct mvtwsi_registers *twsi,
int speed,
#endif
}
+/*
+ * __twsi_i2c_reinit() - Reset and reinitialize the I2C controller.
+ *
+ * This function should be called to get the MVTWSI controller out of the
+ * "bus error" state. It saves and restores the baud and address registers.
+ *
+ * @twsi: The MVTWSI register structure to use.
+ * @tick: The duration of a clock cycle at the current I2C speed.
+ */
+static void __twsi_i2c_reinit(struct mvtwsi_registers *twsi, uint tick)
+{
+ uint baud;
+ uint slaveadd;
+
+ /* Save baud, address registers */
+ baud = readl(>baudrate);
+ slaveadd = readl(>slave_address);
+
+ /* Reset controller */
+ twsi_reset(twsi);
+
+ /* Restore baud, address registers */
+ writel(baud, >baudrate);
+ writel(slaveadd, >slave_address);
+ writel(0, >xtnd_slave_addr);
+
+ /* Assert STOP, but don't care for the result */
+ (void) twsi_stop(twsi, tick);
+}
+
/*
* i2c_begin() - Start a I2C transaction.
*
@@ -621,6 +653,11 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi,
uchar chip,
int stop_status;
int expected_start = MVTWSI_STATUS_START;
+ /* Check for (and clear) a bus error from a previous failed transaction
+* or another master on the same bus */
+ if (readl(>status) == MVTWSI_BUS_ERROR)
+ __twsi_i2c_reinit(twsi, tick);
+
if (alen > 0) {
/* Begin i2c write to send the address bytes */
status = i2c_begin(twsi, expected_start, (chip << 1), tick);
@@ -668,6 +705,11 @@ static int __twsi_i2c_write(struct mvtwsi_registers *twsi,
uchar chip,
{
int status, stop_status;
+ /* Check for (and clear) a bus error from a previous failed transaction
+* or another master on the same bus */
+ if (readl(>status) == MVTWSI_BUS_ERROR)
+ __twsi_i2c_reinit(twsi, tick);
+
/* Begin i2c write to send first the address bytes, then the
* data bytes */
status = i2c_begin(twsi, MVTWSI_STATUS_START, (chip << 1), tick);
--
2.41.0
Re: [PATCH] fs/erofs: Introduce new features including ztailpacking, fragments and dedupe
Hi folks, On 7/7/23 09:52, Yifan Zhao wrote: diff --git a/fs/erofs/internal.h b/fs/erofs/internal.h index 4af7c91560..433a3c6c1e 100644 --- a/fs/erofs/internal.h +++ b/fs/erofs/internal.h +/* make sure that any user of the erofs headers has at least 64bit off_t type */ +extern int erofs_assert_largefile[sizeof(off_t) - 8]; This assertion does not hold true on 32-bit platforms, and as a result this patch prevents building U-Boot on those systems with EROFS support enabled. Did you perhaps mean to use `erofs_off_t` (which is always 64-bit) somewhere that you're using `off_t` instead? I have had to revert this patch on my target, pending a solution for 32-bit users. Thanks, Sam
Re: [PATCH 02/20] sunxi: remove CONFIG_MACPWR
Hi Andre,
Series looks good so far! I'm trying to move my testing build over to it
now; I will report back when I've been running on it for a little bit.
On 7/21/23 07:45, Andre Przywara wrote:
diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index 7ac50c4ae8c..2db4a2d73ca 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -187,7 +187,7 @@ enum env_location env_get_location(enum env_operation op,
int prio)
/* add board specific code here */
int board_init(void)
{
- __maybe_unused int id_pfr1, ret, macpwr_pin;
+ __maybe_unused int id_pfr1, ret;
gd->bd->bi_boot_params = (PHYS_SDRAM_0 + 0x100);
@@ -224,15 +224,6 @@ int board_init(void)
if (ret)
return ret;
- /* strcmp() would look better, but doesn't get optimised away. */
- if (CONFIG_MACPWR[0]) {
- macpwr_pin = sunxi_name_to_gpio(CONFIG_MACPWR);
- if (macpwr_pin >= 0) {
- gpio_request(macpwr_pin, "macpwr");
- gpio_direction_output(macpwr_pin, 1);
- }
- }
This version of your series does not include the CONFIG_SATAPWR removal
(is that intentional or did you miss it?) so these hunks do not apply
against master. I was able to cherry-pick the rc series version of the
CONFIG_SATAPWR removal and everything applied fine. Just thought you
should know!
Cheers,
Sam
Re: [RFC PATCH 00/17] sunxi: rework pinctrl and add T113s support
On 6/21/23 04:55, Andre Przywara wrote: On Tue, 20 Jun 2023 16:11:48 -0600 Sam Edwards wrote: Hi Sam, pleasure to write with you ;-) Hi Andre, Likewise! Well, so this is actually the fallback implementation which should somewhat work on most SoCs: set a flag, reset, and catch the flag in the SPL. For modern SoCs with CPU hotplug support (the H616 is one one of those, and it looks like the T113s is as well), there is actually a more direct route: Oh man, I would definitely prefer a direct route that doesn't require the SPL coming up a second time, but... We put some magic and the FEL entry address into some special memory locations, then just reset. Now the *BootROM* will do the check already, and branch to the provided entry point, which would be the FEL routine. This doesn't rely on a prepared SPL to be loaded, so works without a boot device with mainline U-Boot around. Refer to section 3.4.2.3 and 3.4.2.4 of the T113-S3 user manual (v1.1). According to this, the magic would be 0xfa50392f, the magic's address is 0x070005C0, and CPU0's entry point address would be in 0x070005C4. I had a proof of concept implementation for the H616 using this method. ...I tried this and it seems that the 070005C* block hardware-resets to zero before BROM runs. Is there a softer reset method you had in mind that would avoid this? The only problem left would be that someone needs to clean the magic afterwards, otherwise any follow-up reset would trigger FEL mode again. That's at least pretty fixable though: instead of setting the entry address to the FEL entry point, set it to a thunk placed in SRAM that clears the flag before continuing onward to FEL. The "fel-reset" command (which is easier to type than what I have, "run fel_do_enter") would then call sunxi_fel_flag_set() and initiate a reset, and the SPL's early init just has to do sunxi_fel_flag_test() -> sunxi_fel_flag_clear() -> go_to_fel(). Seems easy enough. Could you recommend to me a sufficiently different chip to test my abstractions against? Something ARMv8 and *without* RTC? I think all ARMv8 parts have an RTC, so your generic approach might work there as well. The complication is that the SPL switches to AArch64 very early, in hand-stitched AArch32 assembly, check out arch/arm/include/asm/arch-sunxi/boot0.h. The check would need to be coded like this, then. I can then send in a series adding FEL support for that. (Also, did that H3 patch actually land? I didn't see anything but want to know if I should be refactoring my approach to extend what that H3 patch does or not.) https://patchwork.ozlabs.org/project/uboot/patch/c211aa414c59e7275fef82bf6f0035276d6e29f3.1656875482.git.msucha...@suse.de/ This approach seems close to mine, only my go_to_fel() enters by way of return_to_fel() after first modifying fel_stash.lr, since the return_to_fel() mechanism already takes care of restoring the core to a BROM-friendly state. One might abuse the board as a T113s dev board, maybe ;-) Does it work without any of the modules populated? Sure, if you're thinking about getting one. You just need an ATX-pinout PSU to power the BMC (it runs off of the 5V standby rail). ... having a board. So far you are the one contributor with access to the hardware, so: thanks for volunteering! ;-) Andre, please, I know you're being tongue-in-cheek here, but I said "no." We should have reached the agree-to-disagree point 2 emails ago: you've made your (very compelling) case for why downstream would benefit from the early expertise of the upstream DT reviewers, and how upstream would benefit from having the DT for a second "real" T113-using board, but at some point you need to trust that I understand that and that I must therefore have very good reasons not to be distracting myself with trying to (dual-)boot a mainline kernel yet. One thing at a time, y'know? :) The U-Boot build system support some kind of build time DT "overlay" feature: You put a file with the same name, but ending in "-u-boot.dtsi" in the arch/arm/dts directory, and it will be included into the DT which gets embedded into the U-Boot image. See arch/arm/dts/sun50i-a64-sopine-baseboard-u-boot.dts for an example, and doc/develop/devicetree/control.rst for the proper documentation. So we upstream a minimal, non-controversial and non-contradicting base .dts into the kernel tree, and can fix things up for the time being using this method. This hack can then go away if either the mainline kernel DT gets fixed and/or U-Boot learns the quad-SPI trick. Oh, good to know! I'll try to remember that this option exists when the time comes to use it. Someone from the board vendor company actually actively adding upstream support for their device early. There were some examples in the past where employees participated in upstreaming, but I cannot remember seeing this too often when it comes to the initial DT suppo
Re: [RFC PATCH 00/17] sunxi: rework pinctrl and add T113s support
Hi Andre, On 6/20/23 06:42, Andre Przywara wrote: So yeah, the request of a "Enter FEL" command came up multiple times, but so far no one could be bothered to implement this properly. The idea would be to have a generic command (more like "fel-reset" than efex), and allow each SoC (family) to implement this differently, as every SoC requires something a bit different here (32-bit vs. 64-bit, having an RTC vs not, etc). If you could post your solution somewhere, we could start this effort. There was some patch for the H3 already, and it's relatively straight-forward on the newer SoCs (H616, IIRC), so if at least two popular, but different SoCs would be supported, we could make sure to have the right abstractions in place. I already have a "go_to_fel()" that does the right thing to enter FEL from the SPL; I would pretty much just need to introduce the following per-SoC(-family) functions: - bool sunxi_fel_flag_test(void) - void sunxi_fel_flag_clear(void) - void sunxi_fel_flag_set(void) The "fel-reset" command (which is easier to type than what I have, "run fel_do_enter") would then call sunxi_fel_flag_set() and initiate a reset, and the SPL's early init just has to do sunxi_fel_flag_test() -> sunxi_fel_flag_clear() -> go_to_fel(). Seems easy enough. Could you recommend to me a sufficiently different chip to test my abstractions against? Something ARMv8 and *without* RTC? I can then send in a series adding FEL support for that. (Also, did that H3 patch actually land? I didn't see anything but want to know if I should be refactoring my approach to extend what that H3 patch does or not.) Ah, depending on the BSP kernel is indeed quite bad. I wonder what features of the kernel you rely on that upstream does not have? Or is it more about the BMC userland parts that are married to the Allwinner kernel and its own interfaces? I don't fully know; getting the kernel back on mainline is, as I said, a push for another day. I'm very much making a point of not looking into it before the bootloader can be upgraded to something that isn't a crashy, hard-to-update, failure-prone mess. (I'm working in "biggest fire, first out" order.) That said, the first such dependent feature that leaps to mind is the AWNAND driver's CONFIG_AW_SPINAND_SIMULATE_MULTIPLANE, which logically interleaves pages of the NAND in a different ordering vs. what the physical NAND (and mainline's spi-nand driver) does. Alas this is a feature we're dependent on not because it provides benefits to our users (it does not, and in downstream discussions I've been soapboxing about how it's likely wearing down people's NANDs) but because the boards are flashed at the factory with this flag enabled so we need it set for the NAND to be accessible. We've experimented with reflashing the board with that flag disabled, but that has so far only resulted in corrupted flash. Hope is not lost, though, for I have a half-written tool which shows some promise in being able to "unscramble the egg" and migrate existing NANDs over to the correct layout. That should be sufficient to get mainline U-Boot (and Tina Linux with the flag disabled) working, but I have no idea about mainline Linux still: this would only peel back one layer of the onion, and I don't know whether the next obstacle will be easier, harder, or about the same difficulty. But it does mean that, for now, we're stuck with Tina Linux. Final DT is a noble goal, but in reality there will always be room for improvement and additions. So what we typically do is to start with a simple .dts for the kernel tree, describing the basic peripherals, and everything that already works and is not subject to debate. If in doubt, include a node, and we will comment. Could you prepare such a patch? The peripheral-describing .dts that I have is for Tina Linux, and uses incompatible compatibles (ha), includes, dt-bindings, temporary hacks while better driver support can be developed, and would otherwise not fly upstream. I can send it in *anyway* if for some reason you think that's a good idea, but I really don't see that as being anything other than a waste of time. As well, I can't write a fresh .dts for mainline (one more likely to be accepted on the list). A mainline kernel has never been booted on this board, so I would do no better at this than a kernel contributor selected at random. The best I can do now is write something that *looks* like the correct .dts. As I keep saying, that may change in the future. But the answer today is still "no, I cannot." This should not contradict any DT nodes that U-Boot uses, so it's not a double effort. True, in theory it *shouldn't* but in practice, I've found it does. One way I've been bitten is that the sunxi SPI driver in U-Boot doesn't support Quad-SPI, so if the DT says the SPI-NAND is connected with a bus width of 4, the SPI-NAND driver requests Quad-SPI transfers, but the SPI master driver has no idea
Re: [PATCH v2 3/7] sunxi: Kconfig: rework PHY_USB_SUN4I selection
On 6/11/23 17:32, Andre Przywara wrote: At the moment we use "select" in each Allwinner SoC's Kconfig section to include the USB PHY driver in the build. This means it cannot be disabled via Kconfig, although USB is not really a strictly required core functionality, and a particular board might not even include USB ports. Rework the Kconfig part by removing the "select" lines for each SoC's section, and instead letting it default to "y" in the PHY driver section itself. We use "depends on !" to exclude the few SoCs we don't support (yet). The Allwinner V3s does not enable USB (PHY) support at the moment, even though it should work: let the PHY default to "n" to keep the current behaviour. Signed-off-by: Andre Przywara Reviewed-by: Jernej Skrabec --- arch/arm/mach-sunxi/Kconfig | 11 --- drivers/phy/allwinner/Kconfig | 6 +- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig index 6dcbb096f74..e0b1bde35a9 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -207,7 +207,6 @@ endif config MACH_SUNXI_H3_H5 bool - select PHY_SUN4I_USB select SUNXI_DE2 select SUNXI_DRAM_DW select SUNXI_DRAM_DW_32BIT @@ -236,7 +235,6 @@ config MACH_SUNIV config MACH_SUN4I bool "sun4i (Allwinner A10)" select CPU_V7A - select PHY_SUN4I_USB select DRAM_SUN4I select SUNXI_GEN_SUN4I select SUPPORT_SPL @@ -247,7 +245,6 @@ config MACH_SUN5I bool "sun5i (Allwinner A13)" select CPU_V7A select DRAM_SUN4I - select PHY_SUN4I_USB select SUNXI_GEN_SUN4I select SUPPORT_SPL imply SPL_SYS_I2C_LEGACY @@ -261,7 +258,6 @@ config MACH_SUN6I select ARCH_SUPPORT_PSCI select SPL_ARMV7_SET_CORTEX_SMPEN select DRAM_SUN6I - select PHY_SUN4I_USB select SPL_I2C select SUN6I_PRCM select SUNXI_GEN_SUN6I @@ -277,7 +273,6 @@ config MACH_SUN7I select ARCH_SUPPORT_PSCI select SPL_ARMV7_SET_CORTEX_SMPEN select DRAM_SUN4I - select PHY_SUN4I_USB select SUNXI_GEN_SUN4I select SUPPORT_SPL select ARMV7_BOOT_SEC_DEFAULT if OLD_SUNXI_KERNEL_COMPAT @@ -291,7 +286,6 @@ config MACH_SUN8I_A23 select CPU_V7_HAS_VIRT select ARCH_SUPPORT_PSCI select DRAM_SUN8I_A23 - select PHY_SUN4I_USB select SPL_I2C select SUNXI_GEN_SUN6I select SUPPORT_SPL @@ -305,7 +299,6 @@ config MACH_SUN8I_A33 select CPU_V7_HAS_VIRT select ARCH_SUPPORT_PSCI select DRAM_SUN8I_A33 - select PHY_SUN4I_USB select SPL_I2C select SUNXI_GEN_SUN6I select SUPPORT_SPL @@ -316,7 +309,6 @@ config MACH_SUN8I_A83T bool "sun8i (Allwinner A83T)" select CPU_V7A select DRAM_SUN8I_A83T - select PHY_SUN4I_USB select SPL_I2C select SUNXI_GEN_SUN6I select MMC_SUNXI_HAS_NEW_MODE @@ -344,7 +336,6 @@ config MACH_SUN8I_R40 select SUPPORT_SPL select SUNXI_DRAM_DW select SUNXI_DRAM_DW_32BIT - select PHY_SUN4I_USB imply SPL_SYS_I2C_LEGACY config MACH_SUN8I_V3S @@ -372,7 +363,6 @@ config MACH_SUN9I config MACH_SUN50I bool "sun50i (Allwinner A64)" select ARM64 - select PHY_SUN4I_USB select SUN6I_PRCM select SUNXI_DE2 select SUNXI_GEN_SUN6I @@ -395,7 +385,6 @@ config MACH_SUN50I_H5 config MACH_SUN50I_H6 bool "sun50i (Allwinner H6)" select ARM64 - select PHY_SUN4I_USB select DRAM_SUN50I_H6 select SUN50I_GEN_H6 diff --git a/drivers/phy/allwinner/Kconfig b/drivers/phy/allwinner/Kconfig index f8f1e99c4f5..565b4617b01 100644 --- a/drivers/phy/allwinner/Kconfig +++ b/drivers/phy/allwinner/Kconfig @@ -4,6 +4,10 @@ config PHY_SUN4I_USB bool "Allwinner Sun4I USB PHY driver" depends on ARCH_SUNXI + depends on !MACH_SUN9I + depends on !MACH_SUN50I_H616 + default n if MACH_SUN8I_V3S + default y select DM_REGULATOR select PHY help @@ -11,7 +15,7 @@ config PHY_SUN4I_USB sunxi SoCs. This driver controls the entire USB PHY block, both the USB OTG - parts, as well as the 2 regular USB 2 host PHYs. + parts, as well as the regular USB HCI host PHYs. config INITIAL_USB_SCAN_DELAY int "Delay initial USB scan by x ms to allow builtin devices to init" This does result in PHY_USB_SUN4I being enabled by default, so I guess: Tested-by: Sam Edwards However, it's now possible to (attempt to) build the mUSB driver with PHY_USB_SUN4I switched off, resulting in link errors. Should the proper "depends on" be added under USB_MUSB_SUNXI? Best, Sam
Re: [RFC PATCH 00/17] sunxi: rework pinctrl and add T113s support
Hi Andre, On 6/14/23 18:07, Andre Przywara wrote: So I finally found some time to address some issues in the series, especially in the first patches (pinctrl rework and preparation). I pushed a branch to https://github.com/apritzel/u-boot/commits/r528-rc I need to do more testing, most importantly regression testing on other SoCs, and will only be able to post something next week, I guess. If you could briefly list the things that are still missing, I could try to pick some low hanging fruits. Rebasing onto this branch ended up eliminating a good chunk of my local hack commits. I've verified that everything is still working (but have not yet retested NAND SPL). The remaining local changes I have are two additions to cpu_sunxi_ncat2.h: +#define SUNXI_R_CPUCFG_BASE0x07000400 /* for PSCI */ +#define SUNXI_RTC_BASE 0x0709 /* for FEL */ The former can probably be brought into my PSCI series somehow (unless we expect more chips with CPUX blocks which might move those soft entry registers around, then it should be defined in cpu_sunxi_*.h). The latter is to support a reimplementation of Allwinner's `efex` command that I'm using for development (it pokes the magic number 0x5AA5A55A into RTC's GP_DATA_REG[2] and then resets; SPL clears that magic number and then does an early branch to BROM+0x0020 -- exactly what Allwinner's fork does). I've also noticed exactly(!) one formatting difference in our clk_d1.c: - .num_gates = ARRAY_SIZE(d1_gates), + .num_gates = ARRAY_SIZE(d1_gates), Up to you if you prefer to align the = or not, but it does look inconsistent when .gates and .resets are aligned and .num_* aren't - might be a nitpick that comes up in patch review. Interesting, indeed this is left at 0, which I think will result in 288 MHz. Correct, at least that's what I was seeing. What is that frequency in your case? Do you know what the BSP programs? 1008 MHz, both. Traditionally we used something conservative that works without cooling and with the default voltage, but I don't know that value for the T113s. For what it's worth, this board has a bare T113-s3 and the current OS does not reclock from 1008 MHz at all, and I don't know of any users of the board having stability issues. In my own case, it idles at that clock at around ~35°C. I think CLK_SUN20I_D1 should be set by default now, so can you check that this is fixed? It is now gone from my defconfig and still working, so indeed this is fixed. Why would we need H6 PSCI support? On the ARMv8 parts we use Trusted Firmware-A (TF-A) to provide PSCI services, which has a much more mature implementation. It's not about the H6 and more about me being unsure whether R528/T113 is the first ARMv7-based SoC to use the new CPU management registers. If it's not, and there's another such chip supported in U-Boot that just lacks PSCI, it would make more sense for me to land my PSCI series independently of our work here, and then you can add the R528 case later. It sounds like R528/T113 may be the first such chip needing this new code, though, so this may have to wait until the R528 series lands. How would this conflict, exactly? I don't see any other I2C2 definition? Well, no, the other definitions haven't landed in U-Boot yet. But they do exist in the kernel, datasheets, and physical chips themselves: PB0/PB1/PB8/PB9/PE4/PE5: i2c2 defined as muxval 4 PC0/PC1/PD20/PD21/PG6/PG7/PG14/PG15: i2c2 defined as muxval 3 PE12/PE13: i2c2 defined as muxval 2 Defining i2c2=2 universally would mean that the pins for i2c2 cannot be changed, since it would conflict with every other definition. And what do you need I2C2 for, exactly? Pins PE12/PE13 host an I²C bus with the board ID EEPROM and an Ethernet switch that should be reset (and have a few registers set to configure proper port isolation) very shortly after power-on. Well, there are shortcuts. I sketched some simpler idea in the comment at the top of pinctrl-sunxi.c. My shortcut for the time being will probably be, "downstream patch." At this time I have no interest in upstreaming the DT. Why not? That might change in the future, but for now it's very much meant to be out-of-tree. Why is this? This only increases your update burden, and we might break something and not realise that, if your DT is not in the tree. The question to ask should be rather: why *not* to upstream the DT? Please keep in mind that this would block U-Boot support, since we need the DT approved in the kernel before we could merge it into U-Boot. Currently, downstream is still fairly dependent on the Tina Linux kernel, not mainline. This is a situation I'd like to change, but it's a push for another day -- my focus right now is only on improving the bootloader situation. This means that there are actually two DTs: one for the kernel, using the Tina Linux binding values, and one for U-Boot's control FDT, which can only support
Re: [PATCH 0/8] SUNIV SPI NAND support in SPL
Hi again Icenowy, On 6/6/23 17:09, Sam Edwards wrote: Still, I believe it's sensible that, when we know for sure we're coming from SPI-NAND (because it's a newer sunxi that reports 0x04, or we know from the suniv stack-checking hack), we should call that its own SPL load method, which does not fall back to SPI-NOR. The SPI(-NOR) load method naturally has to implement the try-NAND-first logic for some of these SoCs, but perhaps we could call it a "quirk" and only do that for chips that aren't known to report SPI-NAND directly? Since other methods that care about flash type (e.g. env_get_location) don't understand BOOT_DEVICE_SPI to mean "SPI, but the actual flash type is ambiguous," I'm starting to think we need to resolve the ambiguity *before* the load method runs. What are your thoughts on this: 1. Introduce SUNXI_BOOTED_FROM_SPI_NAND, value 4 (matching the value used by Allwinner's newer BROMs). 2. Map SUNIV_BOOTED_FROM_NAND to SUNXI_BOOTED_FROM_SPI_NAND. 3. Map SUNXI_BOOTED_FROM_SPI_NAND to BOOT_DEVICE_NAND (and not _SPI). 4. Implement the SPI-NAND stuff to provide `nand_spl_load_image` (so that the common SPL framework's NAND loader can take care of it). Possibly go even further and implement the methods needed by the UBI loader too. 5. On sunxis (e.g. V3s) which are known to report SUNXI_BOOTED_FROM_SPI for SPI-NAND, add a small check to sunxi_get_boot_device() which disambiguates this situation by probing for SPI-NAND. If #4 isn't really feasible, we might instead want to introduce a BOOT_DEVICE_SPI_NAND, but I don't know if adding sunxi to the list of platforms that have their own BOOT_DEVICE_ names is a great call. In any case, I'd hope also to see some kind of bad block handling in here -- a simple check for a bad block each time we get to page 0 (which skips to the next block) should be pretty easy, don't you think? Any thoughts? Any other way I can be of service? (Feel free to delegate some of these ideas to me if you think they're good but don't have the time to execute them!) Warm regards, Sam
[PATCH v4 2/2] usb: musb-new: sunxi: clarify the purpose of SRAM initialization
This is largely a cosmetic change, with one functional distinction:
We are now only setting BIT(0), and no longer clearing BIT(1).
The A20 manual confirms the purpose and bitwidth of this field, and we
have also been doing it this way for a while in Linux-land: The prior
narrative about this initialization being about configuring a FIFO has
pretty much been debunked for years now.
Signed-off-by: Sam Edwards
Reviewed-by: Andre Przywara
---
drivers/usb/musb-new/sunxi.c | 31 ++-
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index a67eaf..a8b1a8f870 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -171,15 +171,22 @@ static void USBC_ForceVbusValidToHigh(__iomem void *base)
musb_writel(base, USBC_REG_o_ISCR, reg_val);
}
-static void USBC_ConfigFIFO_Base(void)
-{
- u32 reg_value;
+/**
+ * Non-USBC register access needed for initialization
+
**/
- /* config usb fifo, 8kb mode */
- reg_value = readl(SUNXI_SRAMC_BASE + 0x04);
- reg_value &= ~(0x03 << 0);
- reg_value |= BIT(0);
- writel(reg_value, SUNXI_SRAMC_BASE + 0x04);
+/*
+ * A10(s), A13, GR8, A20:
+ * switch ownership of SRAM block 'D' to the USB-OTG controller
+ */
+static void sunxi_musb_claim_sram(uintptr_t syscon_base)
+{
+ /*
+* BIT(0) of SRAM_CTRL_REG1 (syscon+0x04) controls SRAM-D ownership:
+* '0' -> exclusive access by CPU
+* '1' -> exclusive access by USB0
+*/
+ setbits_le32(syscon_base + 0x04, BIT(0));
}
/**
@@ -313,7 +320,13 @@ static int sunxi_musb_init(struct musb *musb)
musb->isr = sunxi_musb_interrupt;
if (glue->cfg->has_sram) {
- USBC_ConfigFIFO_Base();
+ /*
+* This is an older USB-OTG controller that Allwinner did not
+* endow with a dedicated SRAM block; it instead uses SRAM
+* block 'D', ownership of which needs to be handed over by
+* the CPU
+*/
+ sunxi_musb_claim_sram(SUNXI_SRAMC_BASE);
}
USBC_EnableDpDmPullUp(musb->mregs);
--
2.39.2
[PATCH v4 1/2] usb: musb-new: sunxi: only perform SRAM initialization when necessary
Only the older (ca. A10, A20) sunxis need this poke for the MUSB to
function. Mimic the Linux kernel and add a `has_sram` flag to the config
structure that is only set for the specific compatibles that require
this initialization.
Signed-off-by: Sam Edwards
Reviewed-by: Andre Przywara
Tested-by: Andre Przywara
---
drivers/usb/musb-new/sunxi.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index c5c63249aa..a67eaf 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -83,6 +83,7 @@
struct sunxi_musb_config {
struct musb_hdrc_config *config;
+ bool has_sram;
};
struct sunxi_glue {
@@ -311,7 +312,10 @@ static int sunxi_musb_init(struct musb *musb)
musb->isr = sunxi_musb_interrupt;
- USBC_ConfigFIFO_Base();
+ if (glue->cfg->has_sram) {
+ USBC_ConfigFIFO_Base();
+ }
+
USBC_EnableDpDmPullUp(musb->mregs);
USBC_EnableIdPullUp(musb->mregs);
@@ -517,6 +521,7 @@ static int musb_usb_remove(struct udevice *dev)
static const struct sunxi_musb_config sun4i_a10_cfg = {
.config = _config,
+ .has_sram = true,
};
static const struct sunxi_musb_config sun6i_a31_cfg = {
--
2.39.2
[PATCH v4 0/2] sunxi, usb: Clean up SRAM initialization code
Hi list, Only change from v4 is that I reworded the commit message in 2/2 not to mention the TODO comment block that I removed in v3. Cheers, Sam Sam Edwards (2): usb: musb-new: sunxi: only perform SRAM initialization when necessary usb: musb-new: sunxi: clarify the purpose of SRAM initialization drivers/usb/musb-new/sunxi.c | 36 +++- 1 file changed, 27 insertions(+), 9 deletions(-) -- 2.39.2
Re: [RFC PATCH 00/17] sunxi: rework pinctrl and add T113s support
Hey Andre, On 6/11/23 18:20, Andre Przywara wrote: Thanks for the update and the list! Can you confirm where you still needed code changes compared to say my github branch plus the changes we already discussed? Trying some guesses below, please confirm or deny: Pretyyy much everything I've changed locally has been submitted to the list or discussed in the relevant patchset. Have you updated your GitHub branch recently (past couple of weeks)? I haven't been watching it but I can pull it again and see which of my local changes are still required. I have a build of U-Boot for my target, complete with: - UART3 initialized correctly this is problematic because of the other pinmux used on your board, which cannot easily be encoded alongside the existing UART3 pinmux? Actually no, my board's UART3 is on PB6/PB7, nice and standard. - DRAM coming up correctly - SPL sets configured boot clock correctly This should work as per github? Yep, everything was working satisfactorily once I figured out I needed to set CONFIG_SYS_CLK_FREQ to get acceptable boot speeds. - SPI-NAND support (SPL and U-Boot proper) This is with Icenow's series? Any D1 specific changes needed there? Yes, with Icenowy's series (322733). I learned that the BROM sets the boot medium code to 0x04 when it's an SPI-*NAND* (while older chips used 0x03 for "it's either SPI-NOR or SPI-NAND, good luck figuring out which"). Since `env_get_location` assumes BOOT_DEVICE_SPI is NOR (and my target needs to load env from UBI iff booting from NAND), I'm hoping I can convince Icenowy to separate out the SPI-NAND and SPI-NOR load methods entirely (vs. her current try-NAND-then-NOR approach) with the aid of some disambiguation logic to probe for an SPI-NAND on the older chips known to report these as 0x03. I also needed Maksim's patch series (355747) to support the D1 SPI master. - MMC support (SPL and U-Boot proper) - SPL boot from FEL again worked already in github? Yes and yes. I was just confirming they're good; no local work needed from me here. - USB gadget support So with the fixed SUNXI_SRAMC_BASE you said it worked? What about the USB PHY? That needs at least wiring in the compatible string? If you have such a patch, can you please rebase it on top of my v2 USB PHY series and post that? Yes, with corrected SUNXI_SRAMC_BASE -- though I also needed my patches to make musb-new/sunxi.c use the UDC gadget model in DM (series 358842), as I don't think there's another way to init the controller at runtime. I can't say whether the endpoint limit is correct or that mUSB *host* operation works. The USB PHY only required that CONFIG_PHY_SUN4I_USB be enabled. The correct compatible is already wired up. It does look like your PHY series drops the explicit requirement to set PHY_SUN4I_USB so that's better than what I was doing (adding a `select` directive under R528). I can test your series if you want but I doubt it intersects my work here in any significant way. - Ethernet MAC+PHY support Anything surprising here? Is that using an already supported external PHY? The only surprise was this was how I noticed that CONFIG_CLK_SUN20I_D1 was not being implicitly enabled. Enabling that was then how I found that the clock driver wasn't compatible with current upstream (which I already mentioned). The PHY is external and already supported, adding it to my DT required very little work. - I²C support * - GPIO support (LEDs, buttons, misc. board management) again should work out of the box, minus your board specific configuration? GPIO is completely OotB, yes. I²C is OotB once MVTWSI_CONTROL_CLEAR_IFLG is set correctly. (The pinctrl requirements for it are a little harder, more on that below.) - `reset` working (requries CONFIG_SYSRESET unset, WDT key) Isn't "CONFIG_SYSRESET unset" a hack? I dimly remember we had this for some other SoC initially, but later got rid of it? Unsetting it is required for reset_cpu() to be defined. Your patchset updates that function (albeit without adding the WDT key, so the current patch is broken) to support NCAT2 already. U-Boot has no driver for "allwinner,sun20i-d1-wdt-reset", so this is the only way for `reset` to work. For the WDT key: it seems like Linux got a nice patch to integrate this neatly into the driver without quirking this too much, do you have something ready for U-Boot as well? Would love to see it on the list then ;-) I just hacked the correct value into the function; nothing really suitable for the list, sorry. - PSCI, nonsec ah yeah, owe you some reviews on this one ... It occurs to me that my work *might* support H6 as well (they both use CPUX blocks, right?) so perhaps it would be better if I de-RFC'd my series and instead worked to upstream it chasing H6, for you to come along later and tack on NCAT2 support with your R528 patchset? - Able to boot Linux ;) * Requires nonzero
Re: [RFC PATCH] i2c: mvtwsi: reinitialize controller to clear bus errors
Hey there Heiko,
On 6/12/23 06:35, Heiko Schocher wrote:
I have not the deep knowledge of this specific i2c driver, but may
also an option is to set
int (*deblock)(struct udevice *bus);
in
static const struct dm_i2c_ops mvtwsi_i2c_ops = {
for this driver and do there the stuff needed to deblock the bus,
as you have done in twsi_i2c_reinit() in your patch?
I'm not sure that this is a good fit. The comment explaining deblock
says this is for situations where "Resetting the I2C master does not
help. The only way is to force the bus through a series of transitions
to make sure that all slaves are done with the transaction."
Which reads to me like it's for situations where some slave is holding
down SDA (making it impossible for us to send a start/stop) and several
SCL pulses are required in order to shake it loose.
In this case, the *bus* is okay and the *master* is upset (ironically, I
think, because the Realtek just did its own "deblock" equivalent), so a
master reset is really all that's required here. We also know the
specific state that it runs off to, so it's easy to detect this
situation and resolve it.
Currently this deblock function is only used from i2c core in
i2c command, but may it makes sense to add in dm_i2c_xfer function in
drivers/i2c/i2c-uclass.c
a call to i2c_deblock() in case bus is blocked?
I wouldn't object to such a change, but since deblock is an "active"
operation that might have side effects, this probably needs a bigger
discussion (beyond what I'm trying to do here) since other users might
be unhappy about this happening without their explicit say-so.
Thanks for your feedback,
Sam
[PATCH v3 2/2] usb: musb-new: sunxi: clarify the purpose of SRAM initialization
This is largely a cosmetic change, with one functional distinction:
We are now only setting BIT(0), and no longer clearing BIT(1).
The A20 manual confirms the purpose and bitwidth of this field, and we
have also been doing it this way for a while in Linux-land: The prior
narrative about this initialization being about configuring a FIFO has
pretty much been debunked for years now.
This cleanup also adds a TODO comment about runtime discovery
of the SYSCON base, per discussion with Andre.
Signed-off-by: Sam Edwards
Cc: Andre Przywara
---
drivers/usb/musb-new/sunxi.c | 31 ++-
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index a67eaf..a8b1a8f870 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -171,15 +171,22 @@ static void USBC_ForceVbusValidToHigh(__iomem void *base)
musb_writel(base, USBC_REG_o_ISCR, reg_val);
}
-static void USBC_ConfigFIFO_Base(void)
-{
- u32 reg_value;
+/**
+ * Non-USBC register access needed for initialization
+
**/
- /* config usb fifo, 8kb mode */
- reg_value = readl(SUNXI_SRAMC_BASE + 0x04);
- reg_value &= ~(0x03 << 0);
- reg_value |= BIT(0);
- writel(reg_value, SUNXI_SRAMC_BASE + 0x04);
+/*
+ * A10(s), A13, GR8, A20:
+ * switch ownership of SRAM block 'D' to the USB-OTG controller
+ */
+static void sunxi_musb_claim_sram(uintptr_t syscon_base)
+{
+ /*
+* BIT(0) of SRAM_CTRL_REG1 (syscon+0x04) controls SRAM-D ownership:
+* '0' -> exclusive access by CPU
+* '1' -> exclusive access by USB0
+*/
+ setbits_le32(syscon_base + 0x04, BIT(0));
}
/**
@@ -313,7 +320,13 @@ static int sunxi_musb_init(struct musb *musb)
musb->isr = sunxi_musb_interrupt;
if (glue->cfg->has_sram) {
- USBC_ConfigFIFO_Base();
+ /*
+* This is an older USB-OTG controller that Allwinner did not
+* endow with a dedicated SRAM block; it instead uses SRAM
+* block 'D', ownership of which needs to be handed over by
+* the CPU
+*/
+ sunxi_musb_claim_sram(SUNXI_SRAMC_BASE);
}
USBC_EnableDpDmPullUp(musb->mregs);
--
2.39.2
[PATCH v3 1/2] usb: musb-new: sunxi: only perform SRAM initialization when necessary
Only the older (ca. A10, A20) sunxis need this poke for the MUSB to
function. Mimic the Linux kernel and add a `has_sram` flag to the config
structure that is only set for the specific compatibles that require
this initialization.
Signed-off-by: Sam Edwards
Reviewed-by: Andre Przywara
Tested-by: Andre Przywara
---
drivers/usb/musb-new/sunxi.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index c5c63249aa..a67eaf 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -83,6 +83,7 @@
struct sunxi_musb_config {
struct musb_hdrc_config *config;
+ bool has_sram;
};
struct sunxi_glue {
@@ -311,7 +312,10 @@ static int sunxi_musb_init(struct musb *musb)
musb->isr = sunxi_musb_interrupt;
- USBC_ConfigFIFO_Base();
+ if (glue->cfg->has_sram) {
+ USBC_ConfigFIFO_Base();
+ }
+
USBC_EnableDpDmPullUp(musb->mregs);
USBC_EnableIdPullUp(musb->mregs);
@@ -517,6 +521,7 @@ static int musb_usb_remove(struct udevice *dev)
static const struct sunxi_musb_config sun4i_a10_cfg = {
.config = _config,
+ .has_sram = true,
};
static const struct sunxi_musb_config sun6i_a31_cfg = {
--
2.39.2
[PATCH v3 0/2] sunxi, usb: Clean up SRAM initialization code
Hello again, The only change from v2 is that `syscon_base` is now a param of `sunxi_musb_claim_sram`, and the TODO comment has been removed. Cheers, Sam Sam Edwards (2): usb: musb-new: sunxi: only perform SRAM initialization when necessary usb: musb-new: sunxi: clarify the purpose of SRAM initialization drivers/usb/musb-new/sunxi.c | 36 +++- 1 file changed, 27 insertions(+), 9 deletions(-) -- 2.39.2
[RFC PATCH] i2c: mvtwsi: reinitialize controller to clear bus errors
Hi I²C maintainers,
My target has the following devices sharing one bus:
- 24C02 EEPROM
- Realtek 8370 Ethernet switch
- Allwinner T113-s3 (running U-Boot, interfacing via MVTWSI)
The RTL8370 is configured in "EEPROM autoload" mode, so on reset
it will load the full contents of the EEPROM. During this sequence,
it does an odd move where it sends a re-start, stop, pulses scl low,
and then a fresh start.
Something about this sequence (I'm betting the scl pulse after stop)
upsets the MVTWSI controller, causing it to retreat into state 0x00,
which the documentation for my chip names "bus error." I'd guess this
is a feature for slave operation: in slave mode, the controller FSM
is completely at the mercy of the bus, and so a misbehaving/glitching
bus can result in essentially a random walk through the states. Rather
than allow that (and risk a potentially dangerous accidental write),
the controller goes to a failsafe "bus error" state and remains there,
effectively shutting down the whole controller.
However, in master mode (which U-Boot uses exclusively), this feature
is a nuisance. We do not care what another master was doing on the bus
previously, as long as it is in the proper idle state when we want to
use it. We also don't care if the bus error was our fault in a previous
transaction, since the error would have been reported at that time. I
reckon that it makes sense to check for this "bus error" state at the
beginning of each new read/write and clear it if detected.
Unfortunately, I couldn't find any way to coax the FSM out of the error
state just by poking at the control register. It's possible I didn't
look hard enough (I'm willing to try other things), but I'm otherwise
left with only the obvious way out: a reset. Since that also resets the
baud and address registers, I have to save and restore those too.
Attached here is my RFC patch (which DOES resolve my problem), for
feedback and suggestions on what I might try differently, as I'm not
sure whether or not I like this approach:
- I would be happier if the code did a fresh init instead of saving
and restoring register state, but this driver is plumbed in a way
that the config struct isn't readily accessible at the low level.
- I don't really like having to duplicate the state check in the read
and write functions; is there anything I can do that's more DRY?
- Avoiding a reset would be nice, ideally avoiding the "bus error"
state altogether by disabling the error detection somehow.
Thoughts?
Cheers,
Sam
---
drivers/i2c/mvtwsi.c | 42 ++
1 file changed, 42 insertions(+)
diff --git a/drivers/i2c/mvtwsi.c b/drivers/i2c/mvtwsi.c
index d088ea75b9..38a3bdade0 100644
--- a/drivers/i2c/mvtwsi.c
+++ b/drivers/i2c/mvtwsi.c
@@ -142,6 +142,8 @@ enum mvtwsi_ctrl_register_fields {
* code.
*/
enum mvstwsi_status_values {
+ /* Protocol violation on bus; this is a terminal state */
+ MVTWSI_BUS_ERROR= 0x00,
/* START condition transmitted */
MVTWSI_STATUS_START = 0x08,
/* Repeated START condition transmitted */
@@ -525,6 +527,36 @@ static void __twsi_i2c_init(struct mvtwsi_registers *twsi,
int speed,
#endif
}
+/*
+ * __twsi_i2c_reinit() - Reset and reinitialize the I2C controller.
+ *
+ * This function should be called to get the MVTWSI controller out of the
+ * "bus error" state. It saves and restores the baud and address registers.
+ *
+ * @twsi: The MVTWSI register structure to use.
+ * @tick: The duration of a clock cycle at the current I2C speed.
+ */
+static void __twsi_i2c_reinit(struct mvtwsi_registers *twsi, uint tick)
+{
+ uint baud;
+ uint slaveadd;
+
+ /* Save baud, address registers */
+ baud = readl(>baudrate);
+ slaveadd = readl(>slave_address);
+
+ /* Reset controller */
+ twsi_reset(twsi);
+
+ /* Restore baud, address registers */
+ writel(baud, >baudrate);
+ writel(slaveadd, >slave_address);
+ writel(0, >xtnd_slave_addr);
+
+ /* Assert STOP, but don't care for the result */
+ (void) twsi_stop(twsi, tick);
+}
+
/*
* i2c_begin() - Start a I2C transaction.
*
@@ -621,6 +653,11 @@ static int __twsi_i2c_read(struct mvtwsi_registers *twsi,
uchar chip,
int stop_status;
int expected_start = MVTWSI_STATUS_START;
+ /* Check for (and clear) a bus error from a previous failed transaction
+* or another master on the same bus */
+ if (readl(>status) == MVTWSI_BUS_ERROR)
+ __twsi_i2c_reinit(twsi, tick);
+
if (alen > 0) {
/* Begin i2c write to send the address bytes */
status = i2c_begin(twsi, expected_start, (chip << 1), tick);
@@ -668,6 +705,11 @@ static int __twsi_i2c_write(struct mvtwsi_registers *twsi,
uchar chip,
{
int status, stop_status;
+ /* Check for (and clear) a bus error from a previous failed transaction
+* or
Re: [RFC PATCH 00/17] sunxi: rework pinctrl and add T113s support
Hi Andre, On 12/5/22 17:45, Andre Przywara wrote: Please let me know if you have any opinions! I believe I promised you last month I'd let you know once I had a build I'm happy with, and I'm pleased to say that I think I've reached that point. I'm running quite rapidly out of sharp edges to sand down, too. I have a build of U-Boot for my target, complete with: - UART3 initialized correctly - DRAM coming up correctly - SPL sets configured boot clock correctly - SPI-NAND support (SPL and U-Boot proper) - MMC support (SPL and U-Boot proper) - SPL boot from FEL - USB gadget support - Ethernet MAC+PHY support - I²C support * - GPIO support (LEDs, buttons, misc. board management) - `reset` working (requries CONFIG_SYSRESET unset, WDT key) - PSCI, nonsec - Able to boot Linux ;) * Requires nonzero `MVTWSI_CONTROL_CLEAR_IFLG` for NCAT2, and a patch to the pinctrl driver to configure the proper mux function for my necessary pins. I figured I'd share this list as a sort of checklist for your own work, too. The remainder of my efforts now will probably be focused on mainlining this stuff (let me know how else I can be of help), and then I'm afraid I'll have to disappear back downstream to the Turing Pi 2 development effort, but maybe our paths will cross again in the kernel lists. :) Thank you greatly, Sam P.S. I figure the reason there aren't I²C function defs in the d1 pinctrl table already is because Allwinner tends to kick around the I²C mux values a lot and we would need a per-pin lookup table that would eat up too much valuable image space? In an entirely JUST FOR FUN exercise to give myself a break from staring at datasheets/patches and do a "pure CS" coding challenge for a change, I came up with a terse encoding scheme for this table. Here is the size (in bits) for a selection of D1's functions (pin assignments harvested from Linux): 'emac': 50, 'i2c0': 101, 'i2c1': 64, 'i2c2': 109, 'i2c3': 91, 'mmc0': 23, 'mmc1': 23, 'mmc2': 20, 'spi0': 41, 'spi1': 48, 'uart0': 78, 'uart1': 87, 'uart2': 88, 'uart3': 102, 'uart4': 68, 'uart5': 66, ...and yes, it also identifies invalid pin assignments! I'd be willing to contribute something like this if there's big interest, but I figure needing to compress this at build-time might be a bit too complicated for the U-Boot project's liking.
[PATCH v2 2/2] usb: musb-new: sunxi: clarify the purpose of SRAM initialization
This is largely a cosmetic change, with one functional distinction:
We are now only setting BIT(0), and no longer clearing BIT(1).
The A20 manual confirms the purpose and bitwidth of this field, and we
have also been doing it this way for a while in Linux-land: The prior
narrative about this initialization being about configuring a FIFO has
pretty much been debunked for years now.
This cleanup also adds a TODO comment about runtime discovery
of the SYSCON base, per discussion with Andre.
Signed-off-by: Sam Edwards
Cc: Andre Przywara
---
drivers/usb/musb-new/sunxi.c | 38 +++-
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index a67eaf..be7faaa11e 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -171,15 +171,29 @@ static void USBC_ForceVbusValidToHigh(__iomem void *base)
musb_writel(base, USBC_REG_o_ISCR, reg_val);
}
-static void USBC_ConfigFIFO_Base(void)
-{
- u32 reg_value;
+/**
+ * Non-USBC register access needed for initialization
+
**/
- /* config usb fifo, 8kb mode */
- reg_value = readl(SUNXI_SRAMC_BASE + 0x04);
- reg_value &= ~(0x03 << 0);
- reg_value |= BIT(0);
- writel(reg_value, SUNXI_SRAMC_BASE + 0x04);
+/*
+ * A10(s), A13, GR8, A20:
+ * switch ownership of SRAM block 'D' to the USB-OTG controller
+ */
+static void sunxi_musb_claim_sram(void)
+{
+ /*
+* TODO: Do not use hardcoded SUNXI_SRAMC_BASE; find the syscon base by
+* traversing this OTG device's `allwinner,sram` FDT property and
+* working upward to the system controller.
+*/
+ void *syscon_base = (void *)SUNXI_SRAMC_BASE;
+
+ /*
+* BIT(0) of SRAM_CTRL_REG1 (syscon+0x04) controls SRAM-D ownership:
+* '0' -> exclusive access by CPU
+* '1' -> exclusive access by USB0
+*/
+ setbits_le32(syscon_base + 0x04, BIT(0));
}
/**
@@ -313,7 +327,13 @@ static int sunxi_musb_init(struct musb *musb)
musb->isr = sunxi_musb_interrupt;
if (glue->cfg->has_sram) {
- USBC_ConfigFIFO_Base();
+ /*
+* This is an older USB-OTG controller that Allwinner did not
+* endow with a dedicated SRAM block; it instead uses SRAM
+* block 'D', ownership of which needs to be handed over by
+* the CPU
+*/
+ sunxi_musb_claim_sram();
}
USBC_EnableDpDmPullUp(musb->mregs);
--
2.39.2
[PATCH v2 1/2] usb: musb-new: sunxi: only perform SRAM initialization when necessary
Only the older (ca. A10, A20) sunxis need this poke for the MUSB to
function. Mimic the Linux kernel and add a `has_sram` flag to the config
structure that is only set for the specific compatibles that require
this initialization.
Signed-off-by: Sam Edwards
Reviewed-by: Andre Przywara
Tested-by: Andre Przywara
---
drivers/usb/musb-new/sunxi.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb-new/sunxi.c b/drivers/usb/musb-new/sunxi.c
index c5c63249aa..a67eaf 100644
--- a/drivers/usb/musb-new/sunxi.c
+++ b/drivers/usb/musb-new/sunxi.c
@@ -83,6 +83,7 @@
struct sunxi_musb_config {
struct musb_hdrc_config *config;
+ bool has_sram;
};
struct sunxi_glue {
@@ -311,7 +312,10 @@ static int sunxi_musb_init(struct musb *musb)
musb->isr = sunxi_musb_interrupt;
- USBC_ConfigFIFO_Base();
+ if (glue->cfg->has_sram) {
+ USBC_ConfigFIFO_Base();
+ }
+
USBC_EnableDpDmPullUp(musb->mregs);
USBC_EnableIdPullUp(musb->mregs);
@@ -517,6 +521,7 @@ static int musb_usb_remove(struct udevice *dev)
static const struct sunxi_musb_config sun4i_a10_cfg = {
.config = _config,
+ .has_sram = true,
};
static const struct sunxi_musb_config sun6i_a31_cfg = {
--
2.39.2
[PATCH v2 0/2] sunxi, usb: Clean up SRAM initialization code
Hi list,
This is the second version of cleaning up the SRAM initialization code in the
musb-new variant for sunxis.
Patch 1 ("only perform ... when necessary") has not changed since v1.
Patch 2 has had the following changes:
- Remove one-off bit/reg #defines, per feedback from Andre.
- Switch to the Linux multiline comment style, per feedback from Andre.
- Reword the commit message, since this change isn't as "speculative" as I had
originally thought.
- Perhaps controversially, introduce a `void *syscon_base;` under that TODO
comment I mentioned before, to further invite somebody to "DO" it. :)
Thank you once again for your continued efforts,
Sam
Sam Edwards (2):
usb: musb-new: sunxi: only perform SRAM initialization when necessary
usb: musb-new: sunxi: clarify the purpose of SRAM initialization
drivers/usb/musb-new/sunxi.c | 43
1 file changed, 34 insertions(+), 9 deletions(-)
--
2.39.2
Re: [PATCH 2/2] usb: musb-new: sunxi: clarify the purpose of SRAM initialization
Hi Andre, I've applied most of this feedback (most of which comes as a relief; I dislike inventing names for mystery bits) in preparation to send a v2, but had two questions: On 6/9/23 04:13, Andre Przywara wrote: The new comments and function name are not from any official source, but are updated to mirror how the Linux kernel sources treat this mystery magic bit. If we wanted to confirm that this speculation is correct, we could verify that SRAM-D is inaccessible whenever the bit is set, and then try clearing it again while the MUSB is in use to see what debris gets left behind in SRAM-D. This cleanup also adds a TODO comment about runtime discovery of the SYSCON base, per discussion with Andre. thanks for sending this, looks good. Some stylistic comments below. I take it that this (in combination with your review on 1/2) means you concur with my speculated purpose of the mystery bit. If so, I'd like to rephrase the above paragraph in the commit message to: """ The new comments and function name are not from any official source, but are updated to mirror how the Linux kernel sources treat this mystery magic bit. This also reflects what's been observed on actual hardware: SRAM-D is inaccessible by the CPU when the bit is set, and the MUSB unit crashes when this bit is cleared while USB is in use, leaving behind debris in SRAM-D from its use as a "scratch space." """ Does this accurately reflect what you've seen, particularly (especially) that last line, or should I end the commentary at "is in use."? I think we should use "non-net" commenting style, with the "/*" on a line on its own. This seems to be an obscure term of art, and searching "non-net comment" and "non-net style" on Google isn't finding me any style guide or set of rules to check against. (Amusingly: if I search for the "net" style, I get a lot of .NET suggestions.) The main thing I'm trying to figure out is if it also demands */ on its own line, which I would intuitively think makes sense (it does look better to me that way), but I'm unsure if your lack of critique at the closing side of my multiline comments means you would prefer: /* * A block of text that's long enough to become a * multiline comment and ends up looking like this */ Thanks for your quick and thorough feedback, Sam
