[PATCH 1/6] common: pe: fix use of undefined macro for ARMv7

[PATCH 1/6] common: pe: fix use of undefined macro for ARMv7

[Ahmad Fatoum]

IMAGE_FILE_MACHINE_ARMNT is not defined in barebox, unlike U-Boot.
The macro we have is IMAGE_FILE_MACHINE_ARMV7, which has the same value
of 0x01c4, so use that instead.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/pe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/pe.c b/common/pe.c
index 5c33665dfa74..6164dd68b7ab 100644
--- a/common/pe.c
+++ b/common/pe.c
@@ -22,7 +22,7 @@ static int machines[] = {
 #elif defined(__arm__)
 	IMAGE_FILE_MACHINE_ARM,
 	IMAGE_FILE_MACHINE_THUMB,
-	IMAGE_FILE_MACHINE_ARMNT,
+	IMAGE_FILE_MACHINE_ARMV7,
 #endif
 
 #if defined(__x86_64__)
-- 
2.39.2

[PATCH 2/6] efi: fs: fix determination of read-only files

[Ahmad Fatoum]

The negation was erroneously applied to only the left-hand argument
falsifying the result. Fix this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 fs/efi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/efi.c b/fs/efi.c
index 11073e9961c4..cb0eb40da4c2 100644
--- a/fs/efi.c
+++ b/fs/efi.c
@@ -360,7 +360,7 @@ static int efifs_stat(struct device *dev, const char *filename,
 	s->st_size = info->FileSize;
 	s->st_mode = 00555;
 
-	if (!info->Attribute & EFI_FILE_READ_ONLY)
+	if (!(info->Attribute & EFI_FILE_READ_ONLY))
 		s->st_mode |= 00222;
 
 	if (info->Attribute & EFI_FILE_DIRECTORY)
-- 
2.39.2

[PATCH 3/6] filetype: fix else clause indentation

[Ahmad Fatoum]

clangd notes the unexpected indentation in the file, so remove that
stray space.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/filetype.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/filetype.c b/common/filetype.c
index db65d1ece25a..3690d4ae077c 100644
--- a/common/filetype.c
+++ b/common/filetype.c
@@ -189,7 +189,7 @@ int is_fat_boot_sector(const void *sect)
 			 */
 			if (slot == 1 && is_fat_with_no_mbr(sect))
 				return 1;
-			 else
+			else
 				return -EINVAL;
 		}
 	}
-- 
2.39.2

[PATCH 4/6] acpi: fix compilation for 32-bit

[Ahmad Fatoum]

Build test on x86 fails due to pointer cast and because of forcing the
bigger alignment on the flexible array.

The problematic pointer cast is for the XSDT, which is 64-bit-only, so
let's fix the cast and return an error if we somehow see an XSDT on
32-bit.

For the flexible array, the first struct member is already packed, so we
should arrive at the same result by just dropping the packed from the
second member, so they get normal alignment.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/bus/acpi.c | 6 ++++--
 include/acpi.h     | 4 ++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/bus/acpi.c b/drivers/bus/acpi.c
index 593617073345..61178db0c8db 100644
--- a/drivers/bus/acpi.c
+++ b/drivers/bus/acpi.c
@@ -174,10 +174,12 @@ static int acpi_register_devices(struct bus_type *bus)
 		sig = "RSDT";
 		root = (struct acpi_rsdt *)(unsigned long)rsdp->rsdt_addr;
 		entry_count = (root->sdt.len - sizeof(struct acpi_rsdt)) / sizeof(u32);
-	} else {
+	} else if (sizeof(void *) == 8) {
 		sig = "XSDT";
-		root = (struct acpi_rsdt *)((struct acpi2_rsdp *)rsdp)->xsdt_addr;
+		root = (struct acpi_rsdt *)(uintptr_t)((struct acpi2_rsdp *)rsdp)->xsdt_addr;
 		entry_count = (root->sdt.len - sizeof(struct acpi_rsdt)) / sizeof(u64);
+	} else {
+		return -EIO;
 	}
 
 	if (acpi_sigcmp(sig, root->sdt.signature)) {
diff --git a/include/acpi.h b/include/acpi.h
index 0756f94501cc..837476424664 100644
--- a/include/acpi.h
+++ b/include/acpi.h
@@ -113,9 +113,9 @@ struct __packed acpi_sdt { /* system description table header */
 	u32		creator_revision;
 };
 
-struct __packed acpi_rsdt { /* system description table header */
+struct acpi_rsdt { /* system description table header */
 	struct acpi_sdt	sdt;
-	struct acpi_sdt	* __aligned(8) entries[];
+	struct acpi_sdt	*entries[];
 };
 
 struct acpi_driver {
-- 
2.39.2

[PATCH 5/6] i2c: efi: avoid 64-bit division

[Ahmad Fatoum]

I2C message length is 16 bit, so doing a 64-bit division is overkill.
Let's remove the cast and just rely on the usual integer promotion
to 32-bit.

Cc: Tomas Marek <tomas.marek@elrest.cz>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/i2c/busses/i2c-efi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-efi.c b/drivers/i2c/busses/i2c-efi.c
index 5f6cc0eed28e..a666a28fab51 100644
--- a/drivers/i2c/busses/i2c-efi.c
+++ b/drivers/i2c/busses/i2c-efi.c
@@ -114,7 +114,7 @@ static unsigned int efi_i2c_msg_op_cnt(const struct efi_i2c_priv *i2c_priv,
 
 	max_len = efi_i2c_max_len(i2c_priv, msg);
 
-	return ((u64)msg->len + max_len - 1) / max_len;
+	return (msg->len + max_len - 1) / max_len;
 }
 
 static unsigned int efi_i2c_req_op_cnt(
-- 
2.39.2

[PATCH 6/6] net: fsl_enetc: fix compilation for 32-bit

[Ahmad Fatoum]

The driver is only used on the 64-bit LS1028, but compile testing on
32-bit fails mostly due to unnecessary pointer casts.

Fixing them cleans up the code a little, so let's do that even if
there's no functional change.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 drivers/net/fsl_enetc.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c
index 1695758fd3fe..7895e229e483 100644
--- a/drivers/net/fsl_enetc.c
+++ b/drivers/net/fsl_enetc.c
@@ -8,11 +8,12 @@
 #include <net.h>
 #include <linux/phy.h>
 #include <linux/pci.h>
-#include <io.h>
+#include <linux/io.h>
 #include <linux/mdio.h>
 #include <asm/system.h>
 #include <of_net.h>
 #include <asm/unaligned.h>
+#include <io-64-nonatomic-lo-hi.h>
 
 #include "fsl_enetc.h"
 
@@ -197,7 +198,7 @@ static void enetc_start_pcs(struct eth_device *edev)
  * LS1028A is the only part with IERB at this time and there are plans to
  * change its structure, keep this LS1028A specific for now.
  */
-#define LS1028A_IERB_BASE		0x1f0800000ULL
+#define LS1028A_IERB_BASE		IOMEM(0x1f0800000ULL)
 #define LS1028A_IERB_PSIPMAR0(pf, vf)	(LS1028A_IERB_BASE + 0x8000 \
 					 + (pf) * 0x100 + (vf) * 8)
 #define LS1028A_IERB_PSIPMAR1(pf, vf)	(LS1028A_IERB_PSIPMAR0(pf, vf) + 4)
@@ -283,7 +284,6 @@ static void enetc_setup_tx_bdr(struct eth_device *edev)
 {
 	struct enetc_priv *priv = edev->priv;
 	struct bd_ring *tx_bdr = &priv->tx_bdr;
-	u64 tx_bd_add = (u64)priv->enetc_txbd_phys;
 
 	/* used later to advance to the next Tx BD */
 	tx_bdr->bd_count = ENETC_BD_CNT;
@@ -296,9 +296,9 @@ static void enetc_setup_tx_bdr(struct eth_device *edev)
 
 	/* set Tx BD address */
 	enetc_bdr_write(priv, TX, ENETC_TX_BDR_ID, ENETC_TBBAR0,
-			lower_32_bits(tx_bd_add));
+			lower_32_bits(priv->enetc_txbd_phys));
 	enetc_bdr_write(priv, TX, ENETC_TX_BDR_ID, ENETC_TBBAR1,
-			upper_32_bits(tx_bd_add));
+			upper_32_bits(priv->enetc_txbd_phys));
 	/* set Tx 8 BD count */
 	enetc_bdr_write(priv, TX, ENETC_TX_BDR_ID, ENETC_TBLENR,
 			tx_bdr->bd_count);
@@ -323,7 +323,6 @@ static void enetc_setup_rx_bdr(struct eth_device *edev)
 {
 	struct enetc_priv *priv = edev->priv;
 	struct bd_ring *rx_bdr = &priv->rx_bdr;
-	u64 rx_bd_add = (u64)priv->enetc_rxbd_phys;
 	int i;
 
 	/* used later to advance to the next BD produced by ENETC HW */
@@ -337,9 +336,9 @@ static void enetc_setup_rx_bdr(struct eth_device *edev)
 
 	/* set Rx BD address */
 	enetc_bdr_write(priv, RX, ENETC_RX_BDR_ID, ENETC_RBBAR0,
-			lower_32_bits(rx_bd_add));
+			lower_32_bits(priv->enetc_rxbd_phys));
 	enetc_bdr_write(priv, RX, ENETC_RX_BDR_ID, ENETC_RBBAR1,
-			upper_32_bits(rx_bd_add));
+			upper_32_bits(priv->enetc_rxbd_phys));
 	/* set Rx BD count (multiple of 8) */
 	enetc_bdr_write(priv, RX, ENETC_RX_BDR_ID, ENETC_RBLENR,
 			rx_bdr->bd_count);
@@ -448,8 +447,8 @@ static int enetc_send(struct eth_device *edev, void *packet, int length)
 		return -ETIMEDOUT;
 	}
 
-	dev_vdbg(&edev->dev, "TxBD[%d]send: pkt_len=%d, buff @0x%x%08x\n", pi, length,
-		  upper_32_bits((u64)packet), lower_32_bits((u64)packet));
+	dev_vdbg(&edev->dev, "TxBD[%d]send: pkt_len=%d, buff @%p\n", pi, length,
+		  packet);
 
 	dma = dma_map_single(priv->dev, packet, length, DMA_TO_DEVICE);
 
-- 
2.39.2

Re: [PATCH 1/6] common: pe: fix use of undefined macro for ARMv7

[Sascha Hauer]

On Wed, 14 Aug 2024 11:24:19 +0200, Ahmad Fatoum wrote:
> IMAGE_FILE_MACHINE_ARMNT is not defined in barebox, unlike U-Boot.
> The macro we have is IMAGE_FILE_MACHINE_ARMV7, which has the same value
> of 0x01c4, so use that instead.
> 
> 

Applied, thanks!

[1/6] common: pe: fix use of undefined macro for ARMv7
      https://git.pengutronix.de/cgit/barebox/commit/?id=3b4666d14ca8 (link may not be stable)
[2/6] efi: fs: fix determination of read-only files
      https://git.pengutronix.de/cgit/barebox/commit/?id=e803828a0461 (link may not be stable)
[3/6] filetype: fix else clause indentation
      https://git.pengutronix.de/cgit/barebox/commit/?id=4aeff80d354c (link may not be stable)
[4/6] acpi: fix compilation for 32-bit
      https://git.pengutronix.de/cgit/barebox/commit/?id=4fa3b9d2b64b (link may not be stable)
[5/6] i2c: efi: avoid 64-bit division
      https://git.pengutronix.de/cgit/barebox/commit/?id=4e01eb4889ff (link may not be stable)
[6/6] net: fsl_enetc: fix compilation for 32-bit
      https://git.pengutronix.de/cgit/barebox/commit/?id=62489abd94cd (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>

Re: [PATCH 5/6] i2c: efi: avoid 64-bit division

[Tomas Marek]

Hello Ahmad,

Thanks for maintenance of the Barebox EFI.

On Wed, Aug 14, 2024 at 11:24:23AM +0200, Ahmad Fatoum wrote:
> I2C message length is 16 bit, so doing a 64-bit division is overkill.
> Let's remove the cast and just rely on the usual integer promotion
> to 32-bit.
> 
> Cc: Tomas Marek <tomas.marek@elrest.cz>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/i2c/busses/i2c-efi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/busses/i2c-efi.c b/drivers/i2c/busses/i2c-efi.c
> index 5f6cc0eed28e..a666a28fab51 100644
> --- a/drivers/i2c/busses/i2c-efi.c
> +++ b/drivers/i2c/busses/i2c-efi.c
> @@ -114,7 +114,7 @@ static unsigned int efi_i2c_msg_op_cnt(const struct efi_i2c_priv *i2c_priv,
>  
>  	max_len = efi_i2c_max_len(i2c_priv, msg);
>  
> -	return ((u64)msg->len + max_len - 1) / max_len;
> +	return (msg->len + max_len - 1) / max_len;

The issue here is that max_len, which represents the MaximumReceiveBytes
or MaximumTransmitBytes in EFI I2C capabilities, is 32-bit. In some EFI
implementations (including the one I use), the EFI I2C MaximumReceiveBytes
and MaximumTransmitBytes are set to the maximum value (0xFFFFFFFF). This
causes msg->len + max_len - 1 to overflow, leading to an incorrect number
of returned operations. To address this, the msg->len is extended to 64 bits.

Best regards

Tomas

>  }
>  
>  static unsigned int efi_i2c_req_op_cnt(
> -- 
> 2.39.2
> 

Re: [PATCH 5/6] i2c: efi: avoid 64-bit division

[Ahmad Fatoum]

Hello Tomas,

On 15.08.24 07:53, Tomas Marek wrote:
> Hello Ahmad,
> 
> Thanks for maintenance of the Barebox EFI.
> 
> On Wed, Aug 14, 2024 at 11:24:23AM +0200, Ahmad Fatoum wrote:
>> I2C message length is 16 bit, so doing a 64-bit division is overkill.
>> Let's remove the cast and just rely on the usual integer promotion
>> to 32-bit.
>>
>> Cc: Tomas Marek <tomas.marek@elrest.cz>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/i2c/busses/i2c-efi.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-efi.c b/drivers/i2c/busses/i2c-efi.c
>> index 5f6cc0eed28e..a666a28fab51 100644
>> --- a/drivers/i2c/busses/i2c-efi.c
>> +++ b/drivers/i2c/busses/i2c-efi.c
>> @@ -114,7 +114,7 @@ static unsigned int efi_i2c_msg_op_cnt(const struct efi_i2c_priv *i2c_priv,
>>  
>>  	max_len = efi_i2c_max_len(i2c_priv, msg);
>>  
>> -	return ((u64)msg->len + max_len - 1) / max_len;
>> +	return (msg->len + max_len - 1) / max_len;
> 
> The issue here is that max_len, which represents the MaximumReceiveBytes
> or MaximumTransmitBytes in EFI I2C capabilities, is 32-bit. In some EFI
> implementations (including the one I use), the EFI I2C MaximumReceiveBytes
> and MaximumTransmitBytes are set to the maximum value (0xFFFFFFFF). This
> causes msg->len + max_len - 1 to overflow, leading to an incorrect number
> of returned operations. To address this, the msg->len is extended to 64 bits.

Thanks for the info. See the patch I just Cc'd you on.

@Sascha, can you drop this one patch again?

Thanks,
Ahmad

> 
> Best regards
> 
> Tomas
> 
>>  }
>>  
>>  static unsigned int efi_i2c_req_op_cnt(
>> -- 
>> 2.39.2
>>
> 


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 5/6] i2c: efi: avoid 64-bit division

[Sascha Hauer]

On Thu, Aug 15, 2024 at 08:57:18AM +0200, Ahmad Fatoum wrote:
> Hello Tomas,
> 
> On 15.08.24 07:53, Tomas Marek wrote:
> > Hello Ahmad,
> > 
> > Thanks for maintenance of the Barebox EFI.
> > 
> > On Wed, Aug 14, 2024 at 11:24:23AM +0200, Ahmad Fatoum wrote:
> >> I2C message length is 16 bit, so doing a 64-bit division is overkill.
> >> Let's remove the cast and just rely on the usual integer promotion
> >> to 32-bit.
> >>
> >> Cc: Tomas Marek <tomas.marek@elrest.cz>
> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >> ---
> >>  drivers/i2c/busses/i2c-efi.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/i2c/busses/i2c-efi.c b/drivers/i2c/busses/i2c-efi.c
> >> index 5f6cc0eed28e..a666a28fab51 100644
> >> --- a/drivers/i2c/busses/i2c-efi.c
> >> +++ b/drivers/i2c/busses/i2c-efi.c
> >> @@ -114,7 +114,7 @@ static unsigned int efi_i2c_msg_op_cnt(const struct efi_i2c_priv *i2c_priv,
> >>  
> >>  	max_len = efi_i2c_max_len(i2c_priv, msg);
> >>  
> >> -	return ((u64)msg->len + max_len - 1) / max_len;
> >> +	return (msg->len + max_len - 1) / max_len;
> > 
> > The issue here is that max_len, which represents the MaximumReceiveBytes
> > or MaximumTransmitBytes in EFI I2C capabilities, is 32-bit. In some EFI
> > implementations (including the one I use), the EFI I2C MaximumReceiveBytes
> > and MaximumTransmitBytes are set to the maximum value (0xFFFFFFFF). This
> > causes msg->len + max_len - 1 to overflow, leading to an incorrect number
> > of returned operations. To address this, the msg->len is extended to 64 bits.
> 
> Thanks for the info. See the patch I just Cc'd you on.
> 
> @Sascha, can you drop this one patch again?

Ok, reverted it.

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |