[PATCH master 1/7] ddr: imx8m: align function definition with prototype

[PATCH master 1/7] ddr: imx8m: align function definition with prototype

[Ahmad Fatoum]

The values of enum dram_type and enum ddrc_type are defined, so they
don't overlap, which allows ddr_cfg_phy() to accept them OR-d.

It's thus wrong to use only one enum of them in the prototype, so adjust
it with the definition.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/soc/imx8m/ddr.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/soc/imx8m/ddr.h b/include/soc/imx8m/ddr.h
index 0b3c4d47e39d..e268892957db 100644
--- a/include/soc/imx8m/ddr.h
+++ b/include/soc/imx8m/ddr.h
@@ -432,7 +432,7 @@ static inline int imx8mp_ddr_init(struct dram_timing_info *dram_timing,
 	return imx8m_ddr_init(dram_timing, DDRC_TYPE_MP | dram_type);
 }
 
-int ddr_cfg_phy(struct dram_timing_info *timing_info, enum ddrc_type ddrc_type);
+int ddr_cfg_phy(struct dram_timing_info *timing_info, unsigned type);
 void load_lpddr4_phy_pie(void);
 void ddrphy_trained_csr_save(struct dram_cfg_param *param, unsigned int num);
 void dram_config_save(struct dram_timing_info *info, unsigned long base);
-- 
2.39.2

[PATCH master 2/7] include: bitmask: avoid -Wint-in-bool-context warning

[Ahmad Fatoum]

In case mask is an enum, GCC will complain that an enum is used in a
boolean context. This warning is easily avoided with no functional
change by doing an explicit comparison against 0.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/bitfield.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index cf2588d81148..44e8cb3a7df3 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -53,7 +53,7 @@
 	({								\
 		BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask),		\
 				 _pfx "mask is not constant");		\
-		BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero");	\
+		BUILD_BUG_ON_MSG(_mask == 0, _pfx "mask is zero");	\
 		BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ?		\
 				 ~((_mask) >> __bf_shf(_mask)) & (_val) : 0, \
 				 _pfx "value too large for the field"); \
-- 
2.39.2

[PATCH master 3/7] net: fec_mpc5200: fix false positive -Wmisleading-indentation

[Ahmad Fatoum]

After the preprocessor runs, the code in question looks like this:
SDMA_INT_DISABLE

	while ((counter--) && (!(fec->eth->ievent & FEC_IEVENT_GRA)));
	{
	     struct mpc5xxx_sdma *sdma = (struct mpc5xxx_sdma *)MPC5XXX_SDMA;
	     sdma->IntMask |= (1 << tasknum);
	}

Which understandably looks like a bug. Avoid this by using do { } while
(0) and moving the semicolon to a separate line.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/powerpc/mach-mpc5xxx/include/mach/sdma.h | 25 ++++++++-----------
 drivers/net/fec_mpc5200.c                     |  3 ++-
 2 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/mach-mpc5xxx/include/mach/sdma.h b/arch/powerpc/mach-mpc5xxx/include/mach/sdma.h
index 02f561675a03..176ed3e892e2 100644
--- a/arch/powerpc/mach-mpc5xxx/include/mach/sdma.h
+++ b/arch/powerpc/mach-mpc5xxx/include/mach/sdma.h
@@ -49,40 +49,35 @@ ALL PARAMETERS ARE ALL LONGWORDS (FOUR BYTES EACH).
 /*---------------------------------------------------------------------*/
 
 /* Disable SmartDMA task */
-#define SDMA_TASK_DISABLE(tasknum)                     \
-{                                                      \
+#define SDMA_TASK_DISABLE(tasknum) do { \
     volatile ushort *tcr = (ushort *)(MPC5XXX_SDMA + 0x0000001c + 2 * tasknum); \
     *tcr = (*tcr) & (~0x8000);                         \
-}
+} while (0)
 
 /* Enable SmartDMA task */
-#define SDMA_TASK_ENABLE(tasknum)                      \
-{                                                      \
+#define SDMA_TASK_ENABLE(tasknum) do { \
     volatile ushort *tcr = (ushort *) (MPC5XXX_SDMA + 0x0000001c + 2 * tasknum); \
     *tcr = (*tcr)  | 0x8000;                           \
-}
+} while (0)
 
 /* Enable interrupt */
-#define SDMA_INT_ENABLE(tasknum)                       \
-{                                                      \
+#define SDMA_INT_ENABLE(tasknum) do { \
     struct mpc5xxx_sdma *sdma = (struct mpc5xxx_sdma *)MPC5XXX_SDMA; \
     sdma->IntMask &= ~(1 << tasknum);                  \
-}
+} while (0)
 
 /* Disable interrupt */
-#define SDMA_INT_DISABLE(tasknum)   \
-{                                                      \
+#define SDMA_INT_DISABLE(tasknum) do { \
     struct mpc5xxx_sdma *sdma = (struct mpc5xxx_sdma *)MPC5XXX_SDMA; \
     sdma->IntMask |= (1 << tasknum);                   \
-}
+} while (0)
 
 
 /* Clear interrupt pending bits */
-#define SDMA_CLEAR_IEVENT(tasknum)  \
-{                                                      \
+#define SDMA_CLEAR_IEVENT(tasknum) do { \
     struct mpc5xxx_sdma *sdma = (struct mpc5xxx_sdma *)MPC5XXX_SDMA; \
     sdma->IntPend = (1 << tasknum);                    \
-}
+} while (0)
 
 /* get interrupt pending bit of a task */
 #define SDMA_GET_PENDINGBIT(tasknum)                   \
diff --git a/drivers/net/fec_mpc5200.c b/drivers/net/fec_mpc5200.c
index 53b5a861aeb8..9c9b795f140c 100644
--- a/drivers/net/fec_mpc5200.c
+++ b/drivers/net/fec_mpc5200.c
@@ -420,7 +420,8 @@ static void mpc5xxx_fec_halt(struct eth_device *dev)
 	/*
 	 * wait for graceful stop to register
 	 */
-	while ((counter--) && (!(fec->eth->ievent & FEC_IEVENT_GRA))) ;
+	while ((counter--) && (!(fec->eth->ievent & FEC_IEVENT_GRA)))
+		;
 
 	/*
 	 * Disable SmartDMA tasks
-- 
2.39.2

[PATCH master 4/7] net: gianfar: fix out of bounds read of local variable

[Ahmad Fatoum]

The MAC address will be written to two 32-bit registers. Because
MAC_ADDR_LEN == 6, this meant two bytes out-of-bounds where written to
the hardware register. Fix this by having them be in-bound and always
initialized to zero.

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

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 4b374b4a50de..10a95324920b 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -234,7 +234,7 @@ static int gfar_set_ethaddr(struct eth_device *edev, const unsigned char *mac)
 {
 	struct gfar_private *priv = edev->priv;
 	void __iomem *regs = priv->regs;
-	char tmpbuf[MAC_ADDR_LEN];
+	char tmpbuf[8] = {};
 	uint tempval;
 	int ix;
 
-- 
2.39.2

Re: [PATCH master 4/7] net: gianfar: fix out of bounds read of local variable

[Sascha Hauer]

On Mon, Jun 05, 2023 at 08:29:36AM +0200, Ahmad Fatoum wrote:
> The MAC address will be written to two 32-bit registers. Because
> MAC_ADDR_LEN == 6, this meant two bytes out-of-bounds where written to
> the hardware register. Fix this by having them be in-bound and always
> initialized to zero.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  drivers/net/gianfar.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index 4b374b4a50de..10a95324920b 100644
> --- a/drivers/net/gianfar.c
> +++ b/drivers/net/gianfar.c
> @@ -234,7 +234,7 @@ static int gfar_set_ethaddr(struct eth_device *edev, const unsigned char *mac)
>  {
>  	struct gfar_private *priv = edev->priv;
>  	void __iomem *regs = priv->regs;
> -	char tmpbuf[MAC_ADDR_LEN];
> +	char tmpbuf[8] = {};

I would prefer to adopt the Linux commit fixing this code which apart
from the out-of-bounds access also has endianess fixes and makes the
code simpler:

---------------------------8<-----------------------------------

>From 35cc52aacd65886a2ae46e68f727cadd09a3e8f2 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Tue, 6 Jun 2023 11:29:51 +0200
Subject: [PATCH] net: gianfar: make MAC addr setup endian safe, cleanup

This is an adoption of Linux commit:

| commit 83bfc3c4765c35ef0dfff8a3d6dedab88f3f50ea
| Author: Claudiu Manoil <claudiu.manoil@freescale.com>
| Date:   Tue Oct 7 10:44:33 2014 +0300
|
|     gianfar: Make MAC addr setup endian safe, cleanup
|
|     Fix the 32-bit memory access that is not endian safe,
|     i.e. not giving the desired byte layout for a LE CPU:
|     tempval = *((u32 *) (tmpbuf + 4)), where 'char tmpbuf[]'.
|
|     Get rid of rendundant local vars (tmpbuf[] and idx) and
|     forced casts.  Cleanup comments.
|
|     Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
|     Signed-off-by: David S. Miller <davem@davemloft.net>

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/net/gianfar.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 4b374b4a50..1a07059db4 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -234,19 +234,13 @@ static int gfar_set_ethaddr(struct eth_device *edev, const unsigned char *mac)
 {
 	struct gfar_private *priv = edev->priv;
 	void __iomem *regs = priv->regs;
-	char tmpbuf[MAC_ADDR_LEN];
 	uint tempval;
-	int ix;
-
-	for (ix = 0; ix < MAC_ADDR_LEN; ix++)
-		tmpbuf[MAC_ADDR_LEN - 1 - ix] = mac[ix];
 
-	tempval = (tmpbuf[0] << 24) | (tmpbuf[1] << 16) | (tmpbuf[2] << 8) |
-		  tmpbuf[3];
+	tempval = (mac[5] << 24) | (mac[4] << 16) | (mac[3] << 8)  |  mac[2];
 
 	out_be32(regs + GFAR_MACSTRADDR1_OFFSET, tempval);
 
-	tempval = *((uint *)(tmpbuf + 4));
+	tempval = (mac[1] << 24) | (mac[0] << 16);
 
 	out_be32(regs + GFAR_MACSTRADDR2_OFFSET, tempval);
 
-- 
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 master 4/7] net: gianfar: fix out of bounds read of local variable

[Ahmad Fatoum]

On 06.06.23 11:33, Sascha Hauer wrote:
> On Mon, Jun 05, 2023 at 08:29:36AM +0200, Ahmad Fatoum wrote:
>> The MAC address will be written to two 32-bit registers. Because
>> MAC_ADDR_LEN == 6, this meant two bytes out-of-bounds where written to
>> the hardware register. Fix this by having them be in-bound and always
>> initialized to zero.
>>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>>  drivers/net/gianfar.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
>> index 4b374b4a50de..10a95324920b 100644
>> --- a/drivers/net/gianfar.c
>> +++ b/drivers/net/gianfar.c
>> @@ -234,7 +234,7 @@ static int gfar_set_ethaddr(struct eth_device *edev, const unsigned char *mac)
>>  {
>>  	struct gfar_private *priv = edev->priv;
>>  	void __iomem *regs = priv->regs;
>> -	char tmpbuf[MAC_ADDR_LEN];
>> +	char tmpbuf[8] = {};
> 
> I would prefer to adopt the Linux commit fixing this code which apart
> from the out-of-bounds access also has endianess fixes and makes the
> code simpler:
> 
> ---------------------------8<-----------------------------------
> 
> From 35cc52aacd65886a2ae46e68f727cadd09a3e8f2 Mon Sep 17 00:00:00 2001
> From: Sascha Hauer <s.hauer@pengutronix.de>
> Date: Tue, 6 Jun 2023 11:29:51 +0200
> Subject: [PATCH] net: gianfar: make MAC addr setup endian safe, cleanup
> 
> This is an adoption of Linux commit:
> 
> | commit 83bfc3c4765c35ef0dfff8a3d6dedab88f3f50ea
> | Author: Claudiu Manoil <claudiu.manoil@freescale.com>
> | Date:   Tue Oct 7 10:44:33 2014 +0300
> |
> |     gianfar: Make MAC addr setup endian safe, cleanup
> |
> |     Fix the 32-bit memory access that is not endian safe,
> |     i.e. not giving the desired byte layout for a LE CPU:
> |     tempval = *((u32 *) (tmpbuf + 4)), where 'char tmpbuf[]'.
> |
> |     Get rid of rendundant local vars (tmpbuf[] and idx) and
> |     forced casts.  Cleanup comments.
> |
> |     Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
> |     Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  drivers/net/gianfar.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> index 4b374b4a50..1a07059db4 100644
> --- a/drivers/net/gianfar.c
> +++ b/drivers/net/gianfar.c
> @@ -234,19 +234,13 @@ static int gfar_set_ethaddr(struct eth_device *edev, const unsigned char *mac)
>  {
>  	struct gfar_private *priv = edev->priv;
>  	void __iomem *regs = priv->regs;
> -	char tmpbuf[MAC_ADDR_LEN];
>  	uint tempval;
> -	int ix;
> -
> -	for (ix = 0; ix < MAC_ADDR_LEN; ix++)
> -		tmpbuf[MAC_ADDR_LEN - 1 - ix] = mac[ix];
>  
> -	tempval = (tmpbuf[0] << 24) | (tmpbuf[1] << 16) | (tmpbuf[2] << 8) |
> -		  tmpbuf[3];
> +	tempval = (mac[5] << 24) | (mac[4] << 16) | (mac[3] << 8)  |  mac[2];
>  
>  	out_be32(regs + GFAR_MACSTRADDR1_OFFSET, tempval);
>  
> -	tempval = *((uint *)(tmpbuf + 4));
> +	tempval = (mac[1] << 24) | (mac[0] << 16);
>  
>  	out_be32(regs + GFAR_MACSTRADDR2_OFFSET, tempval);
>  

-- 
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 |

[PATCH master 5/7] MIPS: boot: main_entry: use malloc_end instead of _stext

[Ahmad Fatoum]

GCC is not fond of subtracting from array base address.
The code already defines and uses a malloc_end pointer, so let's use
that instead to rid us of the warning.

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

diff --git a/arch/mips/boot/main_entry.c b/arch/mips/boot/main_entry.c
index 2c18bc81c3ff..73082adb3fba 100644
--- a/arch/mips/boot/main_entry.c
+++ b/arch/mips/boot/main_entry.c
@@ -89,7 +89,7 @@ void __bare_init main_entry(void *fdt, u32 fdt_size)
 	pr_debug("initializing malloc pool at 0x%08lx (size 0x%08lx)\n",
 			malloc_start, malloc_end - malloc_start);
 
-	mem_malloc_init((void *)malloc_start, (void *)_stext - 1);
+	mem_malloc_init((void *)malloc_start, (void *)malloc_end - 1);
 	mips_stack_top = malloc_start;
 
 	glob_fdt = fdt;
-- 
2.39.2

[PATCH master 6/7] MIPS: longsoon: restart: hide access to zero page

[Ahmad Fatoum]

Watchdog on Longsoon appears to be at address 0. Let's use
OPTIMIZER_HIDE_VAR, so GCC doesn't warn about it.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/mips/mach-loongson/loongson1_reset.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/mips/mach-loongson/loongson1_reset.c b/arch/mips/mach-loongson/loongson1_reset.c
index 85752f4ab825..9b4da217f80d 100644
--- a/arch/mips/mach-loongson/loongson1_reset.c
+++ b/arch/mips/mach-loongson/loongson1_reset.c
@@ -11,9 +11,13 @@
 
 static void __noreturn longhorn_restart_soc(struct restart_handler *rst)
 {
-	__raw_writel(0x1, WDT_EN);
-	__raw_writel(0x1, WDT_SET);
-	__raw_writel(0x1, WDT_TIMER);
+	void __iomem *wdt = IOMEM(0);
+
+	OPTIMIZER_HIDE_VAR(wdt);
+
+	__raw_writel(0x1, wdt + WDT_EN);
+	__raw_writel(0x1, wdt + WDT_SET);
+	__raw_writel(0x1, wdt + WDT_TIMER);
 
 	hang();
 }
-- 
2.39.2

[PATCH master 7/7] bootm: booti: fix false positive uninitialized variable access

[Ahmad Fatoum]

devicetree is only initialized when oftree is non-NULL and it's only
printed when oftree is non-NULL, so warning about devicetree use in the
printf looks like a false positive. Given that devicetree = *oftree, but
with a different type, we can avoid the warning and reduce devicetree
scope in the progress, by just using *oftree in the printf (%pad takes
a reference to phys_addr_t).

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/booti.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/booti.c b/common/booti.c
index 302d7440abd7..e745ff696376 100644
--- a/common/booti.c
+++ b/common/booti.c
@@ -33,7 +33,7 @@ void *booti_load_image(struct image_data *data, phys_addr_t *oftree)
 {
 	const void *kernel_header =
 			data->os_fit ? data->fit_kernel : data->os_header;
-	unsigned long text_offset, image_size, devicetree, kernel;
+	unsigned long text_offset, image_size, kernel;
 	unsigned long image_end;
 	int ret;
 	void *fdt;
@@ -57,6 +57,8 @@ void *booti_load_image(struct image_data *data, phys_addr_t *oftree)
 	image_end = PAGE_ALIGN(kernel + image_size);
 
 	if (oftree) {
+		unsigned long devicetree;
+
 		if (bootm_has_initrd(data)) {
 			ret = bootm_load_initrd(data, image_end);
 			if (ret)
@@ -84,7 +86,7 @@ void *booti_load_image(struct image_data *data, phys_addr_t *oftree)
 
 	printf("Loaded kernel to 0x%08lx", kernel);
 	if (oftree)
-		printf(", devicetree at 0x%08lx", devicetree);
+		printf(", devicetree at %pa", oftree);
 	printf("\n");
 
 	return (void *)kernel;
-- 
2.39.2

Re: [PATCH master 1/7] ddr: imx8m: align function definition with prototype

[Sascha Hauer]

On Mon, Jun 05, 2023 at 08:29:33AM +0200, Ahmad Fatoum wrote:
> The values of enum dram_type and enum ddrc_type are defined, so they
> don't overlap, which allows ddr_cfg_phy() to accept them OR-d.
> 
> It's thus wrong to use only one enum of them in the prototype, so adjust
> it with the definition.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  include/soc/imx8m/ddr.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks

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 |