Re: [PATCH] regmap: clarify struct regmap::max_register value some more
Thanks!
Acked-by: Robin van der Gracht
On Thu, 11 Jan 2024 08:34:12 +0100
Ahmad Fatoum wrote:
> We already have documentation for the member that was taken from Linux,
> but still managed to use it wrongly at multiple places that were
> recently fixed. To prevent such issues from reoccurring in the future,
> expand the documentation a bit.
>
> Suggested-by: Robin van der Gracht
> Signed-off-by: Ahmad Fatoum
> ---
> include/linux/regmap.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/include/linux/regmap.h b/include/linux/regmap.h
> index 690dc3b1dccb..9e38fbc12e16 100644
> --- a/include/linux/regmap.h
> +++ b/include/linux/regmap.h
> @@ -31,6 +31,9 @@ enum regmap_endian {
> * data.
> *
> * @max_register: Optional, specifies the maximum valid register index.
> + * This must be a valid register address and thus a multiple
> + * of the register stride returned by regmap_get_reg_stride()
> + * after registration.
> *
> * @read_flag_mask: Mask to be set in the top byte of the register when doing
> * a read.
Re: [PATCH master 2/7] nvmem: bsec: correct regmap's max_register
On Mon, 8 Jan 2024 12:17:09 +0100 Robin van der Gracht wrote: ... > > > > barebox@Linux Automation MC-1 board:/ ls -l /dev/stm32-bsec > > crw---384 /dev/stm32-bsec > > Thats more like it. I'll apply the full series and recheck. This works as expected. Thanks. Minor note: As you mention in your patch notes: "struct regmap::max_register is in units of struct regmap::reg_stride" This used to be the value of the maximum register number (index). The doc in include/linux/regmap.h line 33 mentions 'index'. Maybe that needs some mentioning of stride as well.. Regardless: Tested-by: Robin van der Gracht
Re: [PATCH master 2/7] nvmem: bsec: correct regmap's max_register
Hello Ahmad,
On Mon, 8 Jan 2024 11:44:00 +0100
Ahmad Fatoum wrote:
> Hello Robin,
>
> On 08.01.24 11:29, Robin van der Gracht wrote:
> > Hi Ahmad,
> >
> > Comments are below.
> >
> > On Tue, 2 Jan 2024 18:00:55 +0100
> > Ahmad Fatoum wrote:
> >
> >> The max_register must be a multiple of the register stride, which is not
> >> the case for (384 / 4) - 1 == 95. Instead, we should be setting 380, so
> >> fix the calculation to do this.
> >>
> >> Fixes: 094ce0ee7cdf ("nvmem: bsec: correct regmap's max_register")
> >> Reported-by: Robin van der Gracht
> >> Signed-off-by: Ahmad Fatoum
> >> ---
> >> drivers/nvmem/bsec.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/nvmem/bsec.c b/drivers/nvmem/bsec.c
> >> index 889f14428d59..22e30c6c2e82 100644
> >> --- a/drivers/nvmem/bsec.c
> >> +++ b/drivers/nvmem/bsec.c
> >> @@ -218,7 +218,7 @@ static int stm32_bsec_probe(struct device *dev)
> >>priv->map_config.reg_bits = 32;
> >>priv->map_config.val_bits = 32;
> >>priv->map_config.reg_stride = 4;
> >> - priv->map_config.max_register = (data->size / 4) - 1;
> >> + priv->map_config.max_register = data->size -
> >> priv->map_config.reg_stride;
> >>
> >>priv->lower = data->lower;
> >>
> >
> > This patch gives a bsec device size of 1520 bytes. Which means I'm now
> > allowed to read/write beyond register 95 without an error.
> >
> > barebox@board:/ ls -l /dev/stm32-bsec
> > crw--- 1520 /dev/stm32-bsec
> >
> > The device size is now in bytes, but the read/write offsets given to
> > the md and mw commands is still in bytes/stride.
> >
> > I.e. to read register 95:
> > md -l -s /dev/stm32-bsec 380+4
> > 017c:
> >
> > I can now also read register 100:
> > md -l -s /dev/stm32-bsec 400+4
> > 0190:
> >
> > This doesn't seem right.
> >
> > max_register should probably stay 95. See doc[1]
> >
> > 1:https://git.pengutronix.de/cgit/barebox/tree/include/linux/regmap.h?h=v2023.12.0#n33
> >
>
> Did you apply the whole series? With the whole series applied I have:
Argh. No. I missed it was part of a series. I was only cc'd to this
one.
>
> barebox@Linux Automation MC-1 board:/ ls -l /dev/stm32-bsec
> crw---384 /dev/stm32-bsec
Thats more like it. I'll apply the full series and recheck.
Robin
Re: [PATCH master 2/7] nvmem: bsec: correct regmap's max_register
Hi Ahmad,
Comments are below.
On Tue, 2 Jan 2024 18:00:55 +0100
Ahmad Fatoum wrote:
> The max_register must be a multiple of the register stride, which is not
> the case for (384 / 4) - 1 == 95. Instead, we should be setting 380, so
> fix the calculation to do this.
>
> Fixes: 094ce0ee7cdf ("nvmem: bsec: correct regmap's max_register")
> Reported-by: Robin van der Gracht
> Signed-off-by: Ahmad Fatoum
> ---
> drivers/nvmem/bsec.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/nvmem/bsec.c b/drivers/nvmem/bsec.c
> index 889f14428d59..22e30c6c2e82 100644
> --- a/drivers/nvmem/bsec.c
> +++ b/drivers/nvmem/bsec.c
> @@ -218,7 +218,7 @@ static int stm32_bsec_probe(struct device *dev)
> priv->map_config.reg_bits = 32;
> priv->map_config.val_bits = 32;
> priv->map_config.reg_stride = 4;
> - priv->map_config.max_register = (data->size / 4) - 1;
> + priv->map_config.max_register = data->size -
> priv->map_config.reg_stride;
>
> priv->lower = data->lower;
>
This patch gives a bsec device size of 1520 bytes. Which means I'm now
allowed to read/write beyond register 95 without an error.
barebox@board:/ ls -l /dev/stm32-bsec
crw--- 1520 /dev/stm32-bsec
The device size is now in bytes, but the read/write offsets given to
the md and mw commands is still in bytes/stride.
I.e. to read register 95:
md -l -s /dev/stm32-bsec 380+4
017c:
I can now also read register 100:
md -l -s /dev/stm32-bsec 400+4
0190:
This doesn't seem right.
max_register should probably stay 95. See doc[1]
1:https://git.pengutronix.de/cgit/barebox/tree/include/linux/regmap.h?h=v2023.12.0#n33
Robin
Re: [PATCH v2] nvmem: regmap: Fix nvmem size
Hi Ahmad,
On 2023-12-20 10:00, Ahmad Fatoum wrote:
Hello Robin,
Thanks for the fix.
On 20.12.23 09:29, Robin van der Gracht wrote:
- if (roffset + rbytes > stride * regmap_get_max_register(map))
+ if (roffset + rbytes > regmap_size_bytes(map) * stride)
Shouldn't stride on the right hand side be dropped?
roffset = register index * stride.
I.e. 380 for register with index 95.
For stm32mp1x bsec:
map->format.val_bytes = 4
map->reg_stride = 4
regmap_size_bytes() = map->format.val_bytes * (95 + 1) / map->reg_stride
= 96
So the result with the stride on the right size is correct.
I moved stride from left to right to be consistent with the size
calculation
in nvmem_regmap_register_with_pp()
Kind regards,
Robin
Cheers,
Ahmad
return -EINVAL;
for (i = roffset; i < roffset + rbytes; i += stride) {
@@ -78,7 +78,7 @@ nvmem_regmap_register_with_pp(struct regmap *map,
const char *name,
config.priv = map;
config.stride = 1;
config.word_size = 1;
- config.size = regmap_get_max_register(map) *
regmap_get_reg_stride(map);
+ config.size = regmap_size_bytes(map) * regmap_get_reg_stride(map);
config.cell_post_process = cell_post_process;
config.reg_write = nvmem_regmap_write;
config.reg_read = nvmem_regmap_read;
[PATCH v2] nvmem: regmap: Fix nvmem size
We should add 1 to the max_register index since counting is zero based.
i.e. the stm32mp151 bsec has registers 0 - 95 with reg_stride 4.
Size should be (95 + 1) * 4 = 384 bytes otherwise we can't access bsec
register 95 (last one).
regmap_size_bytes() does take the +1 into account so we can use that.
Signed-off-by: Robin van der Gracht
---
v2: Fix the size calculation in nvmem_regmap_read() as well.
drivers/nvmem/regmap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nvmem/regmap.c b/drivers/nvmem/regmap.c
index fa5405d7a8..b923c5787d 100644
--- a/drivers/nvmem/regmap.c
+++ b/drivers/nvmem/regmap.c
@@ -38,7 +38,7 @@ static int nvmem_regmap_read(void *ctx, unsigned offset, void
*buf, size_t bytes
skip_bytes = offset & (stride - 1);
rbytes = roundup(bytes + skip_bytes, stride);
- if (roffset + rbytes > stride * regmap_get_max_register(map))
+ if (roffset + rbytes > regmap_size_bytes(map) * stride)
return -EINVAL;
for (i = roffset; i < roffset + rbytes; i += stride) {
@@ -78,7 +78,7 @@ nvmem_regmap_register_with_pp(struct regmap *map, const char
*name,
config.priv = map;
config.stride = 1;
config.word_size = 1;
- config.size = regmap_get_max_register(map) * regmap_get_reg_stride(map);
+ config.size = regmap_size_bytes(map) * regmap_get_reg_stride(map);
config.cell_post_process = cell_post_process;
config.reg_write = nvmem_regmap_write;
config.reg_read = nvmem_regmap_read;
--
2.40.1
Re: [PATCH] nvmem: regmap: Fix nvmem size
After applying this patch the last register is writable, but still not readable. The size is now correct: barebox:/ ls -l /dev/stm32-bsec crw---384 /dev/stm32-bsec This now works: mw -l -d /dev/stm32-bsec 0x017c+4 0x12345678 This still doesn't: barebox:/ md -l -s /dev/stm32-bsec 0x017c+4 read: Invalid argument On 2023-12-19 15:14, Robin van der Gracht wrote: We should add 1 to the max_register index since counting is zero based. i.e. the stm32mp151 bsec has registers 0 - 95 with reg_stride 4. Size should be (95 + 1) * 4 = 384 bytes otherwise we can't access bsec register 95 (last one). Signed-off-by: Robin van der Gracht --- drivers/nvmem/regmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvmem/regmap.c b/drivers/nvmem/regmap.c index fa5405d7a8..ffc96a310f 100644 --- a/drivers/nvmem/regmap.c +++ b/drivers/nvmem/regmap.c @@ -78,7 +78,7 @@ nvmem_regmap_register_with_pp(struct regmap *map, const char *name, config.priv = map; config.stride = 1; config.word_size = 1; - config.size = regmap_get_max_register(map) * regmap_get_reg_stride(map); + config.size = (regmap_get_max_register(map) + 1) * regmap_get_reg_stride(map); config.cell_post_process = cell_post_process; config.reg_write = nvmem_regmap_write; config.reg_read = nvmem_regmap_read;
[PATCH] nvmem: regmap: Fix nvmem size
We should add 1 to the max_register index since counting is zero based. i.e. the stm32mp151 bsec has registers 0 - 95 with reg_stride 4. Size should be (95 + 1) * 4 = 384 bytes otherwise we can't access bsec register 95 (last one). Signed-off-by: Robin van der Gracht --- drivers/nvmem/regmap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/nvmem/regmap.c b/drivers/nvmem/regmap.c index fa5405d7a8..ffc96a310f 100644 --- a/drivers/nvmem/regmap.c +++ b/drivers/nvmem/regmap.c @@ -78,7 +78,7 @@ nvmem_regmap_register_with_pp(struct regmap *map, const char *name, config.priv = map; config.stride = 1; config.word_size = 1; - config.size = regmap_get_max_register(map) * regmap_get_reg_stride(map); + config.size = (regmap_get_max_register(map) + 1) * regmap_get_reg_stride(map); config.cell_post_process = cell_post_process; config.reg_write = nvmem_regmap_write; config.reg_read = nvmem_regmap_read; -- 2.40.1
[PATCH] ARM: mach-imx: Add missing ocotp dependency to protonic-imx6 platform
The protonic-imx6 board code calls imx_ocotp_read/write_field functions. Compiling without IMX_OCOTP results in a build failure. Signed-off-by: Robin van der Gracht --- arch/arm/mach-imx/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig index dcb70c8c1a..b95c6e456e 100644 --- a/arch/arm/mach-imx/Kconfig +++ b/arch/arm/mach-imx/Kconfig @@ -350,6 +350,7 @@ config MACH_PROTONIC_IMX6 select ARCH_IMX6 select ARCH_IMX6UL select ARM_USE_COMPRESSED_DTB + select IMX_OCOTP config MACH_PROTONIC_IMX8M bool "Protonic-Holland i.MX8Mx based boards" -- 2.37.2
[PATCH] mci: imx-esdhc-common: Don't clear unhandled status bits
A DMA cmd + data transfer can finish or stop (i.e. on a block gap) before the status register is cleared. In that case we'll lose track of state causing sdhci_transfer_data_dma() to loop forever waiting for status bits that are already cleared. Clearing SDHCI_INT_CMD_COMPLETE should suffice here. Since it's not evaluated a second time, clearing it at the start of the function is sufficient so we can just remove the erroneous status write. Signed-off-by: Robin van der Gracht --- drivers/mci/imx-esdhc-common.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/mci/imx-esdhc-common.c b/drivers/mci/imx-esdhc-common.c index a12db43c88..27293e44b7 100644 --- a/drivers/mci/imx-esdhc-common.c +++ b/drivers/mci/imx-esdhc-common.c @@ -176,7 +176,6 @@ int __esdhc_send_cmd(struct fsl_esdhc_host *host, struct mci_cmd *cmd, } irqstat = sdhci_read32(>sdhci, SDHCI_INT_STATUS); - sdhci_write32(>sdhci, SDHCI_INT_STATUS, irqstat); if (irqstat & CMD_ERR) return -EIO; -- 2.34.1
Re: Boot hangs during sdhci_transfer_data_dma
On 2022-06-21 09:46, Sascha Hauer wrote: Hi Robin, ... We should only ever clear the bits we have handled, like sdhci_transfer_data_dma() does with sdhci_write32(sdhci, SDHCI_INT_STATUS, SDHCI_INT_DMA); I just noticed that the tegra-sdmmc mci driver might have the same issue. https://git.pengutronix.de/cgit/barebox/tree/drivers/mci/tegra-sdmmc.c#n149 It can prevent the following conditional from ever evaluating true trapping the process in the while loop. https://git.pengutronix.de/cgit/barebox/tree/drivers/mci/tegra-sdmmc.c#n203 This driver clears the status register on error (not on start/end of function) with an old value 'val' which can be lacking status bits that popped up along the way... I can't test code changes for this platform. - Robin
Re: Boot hangs during sdhci_transfer_data_dma
Hi Sascha, On 2022-06-21 09:46, Sascha Hauer wrote: Hi Robin, On Mon, Jun 20, 2022 at 04:33:02PM +0200, Robin van der Gracht wrote: Hi, Today I tried to run barebox with CONFIG_KEYBOARD_GPIO=y added to my config. and noticed my board hangs during boot. When I modify the probe function to run without registering the poller[1] it boots as expected. I started digging into the code to see how far the boot gets when I do register the poller. I found that Barebox hangs in a do/while loop in sdhci_transfer_data_dma[2]. The contents of the interrupt status (SDHCI_INT_STATUS) is 0 and stays that way forever trapping the process in the loop. Call stack: initcall -> barebox_of_populate state_probe drivers/misc/state.c state_new_from_nodecommon/state/state.c of_find_path_by_node drivers/of/of_path.c __of_find_path drivers/of/of_path.c device_detect drivers/base/driver.c mci_detect_card drivers/mci/mci-core.c mci_card_probe drivers/mci/mci-core.c mci_startup drivers/mci/mci-core.c mci_startup_mmc drivers/mci/mci-core.c mmc_compare_ext_csds drivers/mci/mci-core.c mci_send_ext_csd drivers/mci/mci-core.c mci_send_cmd drivers/mci/mci-core.c esdhc_send_cmd drivers/mci/imx-esdhc.c __esdhc_send_cmd drivers/mci/imx-esdhc-common.c sdhci_transfer_data_dma drivers/mci/sdhci.c I'm not sure how this happens. It's not the first transfer taking place. I figured that mayby the poller[1] just adds some cpu load that opens up a window for this to occur. Maybe something else cleared the status register right before we entered the loop. Thats when I spotted this read/write construction[3]. It's executed right before we enter the do/while loop and (over)writes to the irq status register. I removed the line with the write command[3] and my board boots as expected. Why are we (over)writing the status register right after reading it? The idea is likely that we clear the interrupts that we just handled. It seems by the time the status register is overwritten the DMA transfer is already ongoing, and in your case even already done. I can confirm this. When the data transfer is still ongoing the status register holds 0x1 (SDHCI_INT_CMD_COMPLETE). A few lines above the write there is a busy wait for this bit to be set. When my board hangs the status register holds 0x000b at the timen it is cleared. Which means the DMA engine has stopped on a buffer boundary. Apparently this can sometimes happen shortly after starting. We should only ever clear the bits we have handled, like sdhci_transfer_data_dma() does with sdhci_write32(sdhci, SDHCI_INT_STATUS, SDHCI_INT_DMA); Ack. This would suffice: sdhci_write32(>sdhci, SDHCI_INT_STATUS, SDHCI_INT_CMD_COMPLETE); Apart from this line the code never checks the same bit twice, so clearing anything shoudn't be necessary. Clearing the status register once either at the start or the end of the function should be enough. I think the right thing to do is just to remove the erroneous status register write like you already did. I'll submit a patch that removes the write. Thank you for thinking along. - Robin
Re: [PATCH 1/9] ARM: boards: protonic-imx6: prtvt7 hardkey inputs are active low
Hello Ahmad,
On 2022-06-20 09:51, Ahmad Fatoum wrote:
Hello,
On 17.06.22 10:44, Marco Felsch wrote:
On 22-06-17, Sascha Hauer wrote:
Or, it is active low and your patch is correct :D
If they are, can we add a comment or _N suffix to the names?
Does barebox not have gpiod? The board code just should check if it is
active or not. Whatever active means in this case.
There is gpiod_get(), but there's also gpio-keys support. See
drivers/input/specialkeys.c for an example on how to register an input
notifier.
I'm interested in two keys pressed at the same time. When I implement a
notifyer like in specialkeys.c I'll have to check if the other key is pressed
at that time as well.
I noticed that the input subsystem is already maintaining a bitmask with keys
pressed. If I tap into that, I can implement it like so:
diff --git a/arch/arm/boards/protonic-imx6/board.c
b/arch/arm/boards/protonic-imx6/board.c
index 8436903fd8..1e1a22ae8e 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -8,11 +8,13 @@
#include
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -742,17 +744,13 @@ static int prt_imx6_init_kvg_yaco(struct prt_imx6_priv
*priv)
return prt_imx6_init_kvg_power(priv, PW_MODE_KVG_WITH_YACO);
}
-#define GPIO_KEY_F6 (0xe0 + 5)
-#define GPIO_KEY_CYCLE (0xe0 + 2)
-
static int prt_imx6_init_prtvt7(struct prt_imx6_priv *priv)
{
- /* This function relies heavely on the gpio-pca9539 driver */
+ unsigned long keys[BITS_TO_LONGS(KEY_CYCLEWINDOWS)];
- gpio_direction_input(GPIO_KEY_F6);
- gpio_direction_input(GPIO_KEY_CYCLE);
+ input_key_get_status(keys, KEY_CYCLEWINDOWS);
- if (gpio_get_value(GPIO_KEY_CYCLE) || gpio_get_value(GPIO_KEY_F6))
+ if (!(test_bit(KEY_CYCLEWINDOWS, keys) && test_bit(KEY_F6, keys)))
priv->no_usb_check = 1;
return 0;
- Robin
Boot hangs during sdhci_transfer_data_dma
Hi, Today I tried to run barebox with CONFIG_KEYBOARD_GPIO=y added to my config. and noticed my board hangs during boot. When I modify the probe function to run without registering the poller[1] it boots as expected. I started digging into the code to see how far the boot gets when I do register the poller. I found that Barebox hangs in a do/while loop in sdhci_transfer_data_dma[2]. The contents of the interrupt status (SDHCI_INT_STATUS) is 0 and stays that way forever trapping the process in the loop. Call stack: initcall -> barebox_of_populate state_probe drivers/misc/state.c state_new_from_nodecommon/state/state.c of_find_path_by_node drivers/of/of_path.c __of_find_path drivers/of/of_path.c device_detectdrivers/base/driver.c mci_detect_carddrivers/mci/mci-core.c mci_card_probe drivers/mci/mci-core.c mci_startupdrivers/mci/mci-core.c mci_startup_mmc drivers/mci/mci-core.c mmc_compare_ext_csds drivers/mci/mci-core.c mci_send_ext_csd drivers/mci/mci-core.c mci_send_cmd drivers/mci/mci-core.c esdhc_send_cmd drivers/mci/imx-esdhc.c __esdhc_send_cmd drivers/mci/imx-esdhc-common.c sdhci_transfer_data_dma drivers/mci/sdhci.c I'm not sure how this happens. It's not the first transfer taking place. I figured that mayby the poller[1] just adds some cpu load that opens up a window for this to occur. Maybe something else cleared the status register right before we entered the loop. Thats when I spotted this read/write construction[3]. It's executed right before we enter the do/while loop and (over)writes to the irq status register. I removed the line with the write command[3] and my board boots as expected. Why are we (over)writing the status register right after reading it? Other theories on how this could occur are also very welcome :) - Robin 1: https://git.pengutronix.de/cgit/barebox/tree/drivers/input/gpio_keys.c#n168 2: https://git.pengutronix.de/cgit/barebox/tree/drivers/mci/sdhci.c#n167 3: https://git.pengutronix.de/cgit/barebox/tree/drivers/mci/imx-esdhc-common.c#n179
Re: [PATCH 1/9] ARM: boards: protonic-imx6: prtvt7 hardkey inputs are active low
Hi Ahmand,
On 2022-06-20 09:51, Ahmad Fatoum wrote:
Hello,
On 17.06.22 10:44, Marco Felsch wrote:
On 22-06-17, Sascha Hauer wrote:
Or, it is active low and your patch is correct :D
If they are, can we add a comment or _N suffix to the names?
Does barebox not have gpiod? The board code just should check if it is
active or not. Whatever active means in this case.
There is gpiod_get(), but there's also gpio-keys support. See
drivers/input/specialkeys.c for an example on how to register an input
notifier.
I dont think I can use gpiod_get() due to the device tree format for
gpio-keys. I didn't know about the input_notifier framework. I'll check
if I can get it to work with that as-well.
Robin, did you test this works with barebox v2022.05.0?
I'd have assumed this to be broken by f349b662674e ("gpio: allocate dynamic
gpio numbers top down"). Especially, with deep probe, you can't and
shouldn't
depend on GPIO expanders numbering being fixed. If you use an input
notifier,
you should sidestep this issue altogether.
I just notiched that as well. I was just making some changes including the
feadback from this thread:
diff --git a/arch/arm/boards/protonic-imx6/board.c
b/arch/arm/boards/protonic-imx6/board.c
index cdbb8debe6..374ec11364 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -635,17 +635,24 @@ static int prt_imx6_init_kvg_yaco(struct prt_imx6_priv
*priv)
return prt_imx6_init_kvg_power(priv, PW_MODE_KVG_WITH_YACO);
}
-#define GPIO_KEY_F6 (0xe0 + 5)
-#define GPIO_KEY_CYCLE (0xe0 + 2)
-
static int prt_imx6_init_prtvt7(struct prt_imx6_priv *priv)
{
- /* This function relies heavely on the gpio-pca9539 driver */
+ int gpio_f6, gpio_cycle;
+ struct device_d *gpio_expander;
+
+ gpio_expander = get_device_by_name("pca95390");
+ if (!gpio_expander) {
+ dev_err(priv->dev, "Can't find pca9539 gpio expander\n");
+ return -ENODEV;
+ }
+
+ gpio_cycle = gpio_get_num(gpio_expander, 2);
+ gpio_request_one(gpio_cycle, GPIOF_ACTIVE_LOW | GPIOF_DIR_IN,
"Cycle");
- gpio_direction_input(GPIO_KEY_F6);
- gpio_direction_input(GPIO_KEY_CYCLE);
+ gpio_f6 = gpio_get_num(gpio_expander, 5);
+ gpio_request_one(gpio_f6, GPIOF_ACTIVE_LOW | GPIOF_DIR_IN, "F6");
- if (gpio_get_value(GPIO_KEY_CYCLE) && gpio_get_value(GPIO_KEY_F6))
+ if (!(gpio_is_active(gpio_cycle) && gpio_is_active(gpio_f6)))
priv->no_usb_check = 1;
return 0;
I'll make a proposal with the input_notifier aswell.
- Robin
Re: [PATCH 1/9] ARM: boards: protonic-imx6: prtvt7 hardkey inputs are active low
On 2022-06-16 18:38, Oleksij Rempel wrote: Am 16.06.22 um 18:28 schrieb Oleksij Rempel: Hi Robin, On Thu, Jun 16, 2022 at 03:11:06PM +0200, Robin van der Gracht wrote: The usb check needs to be skipped unless both keys are pressed simultaneously. Signed-off-by: Robin van der Gracht --- arch/arm/boards/protonic-imx6/board.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boards/protonic-imx6/board.c b/arch/arm/boards/protonic-imx6/board.c index cdbb8debe6..8f8a0c745e 100644 --- a/arch/arm/boards/protonic-imx6/board.c +++ b/arch/arm/boards/protonic-imx6/board.c @@ -645,7 +645,7 @@ static int prt_imx6_init_prtvt7(struct prt_imx6_priv *priv) gpio_direction_input(GPIO_KEY_F6); gpio_direction_input(GPIO_KEY_CYCLE); - if (gpio_get_value(GPIO_KEY_CYCLE) && gpio_get_value(GPIO_KEY_F6)) + if (gpio_get_value(GPIO_KEY_CYCLE) || gpio_get_value(GPIO_KEY_F6)) priv->no_usb_check = 1; Hm, you probably wont: if (!(gpio_get_value(GPIO_KEY_CYCLE) && gpio_get_value(GPIO_KEY_F6))) otherwise usb check will be always skipped. Or, it is active low and your patch is correct :D They are active low. I mentoned that in the patch subject ;) I want both keys pressed simultaniously as a requirement for the usb check because it adds a delay (autoboot_timeout) to the boot process. - Robin
Re: [PATCH 2/9] ARM: boards: protonic-imx6: Update comment to match a recent code update
On 2022-06-17 09:00, Sascha Hauer wrote:
On Thu, Jun 16, 2022 at 03:11:07PM +0200, Robin van der Gracht wrote:
Fixes: 1e817b64bc45968320597344e6b05463edd9c5e5
We don't have this commit in mainline, could you reference the
corresponding upstream commit instead? Please write as
aabbccddeeff ("subject")
I must have mixed those up. The commit that my commit amends:
commit 8d4698e793d823590f050b36b26f67f73d5f9e85
Author: Oleksij Rempel
Date: Mon Mar 28 14:09:56 2022 +0200
ARM: boards: protonic-imx6: fix file system access warning
We should not access a file system from the poller. So, do it from the
worker. This patch will fix warning on FS access for Protonic board
code.
Signed-off-by: Oleksij Rempel
Link:
https://lore.barebox.org/20220328120956.2402132-1-o.rem...@pengutronix.de
Signed-off-by: Sascha Hauer
Re: [PATCH] lib: parameter: Free previous string value in param_string_set
On 2022-06-16 15:50, Sascha Hauer wrote:
On Thu, Jun 16, 2022 at 02:50:40PM +0200, Robin van der Gracht wrote:
When the parameter has no set function the previous (saved_value) is not
freed up on completion.
Signed-off-by: Robin van der Gracht
---
lib/parameter.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/parameter.c b/lib/parameter.c
index adc3c7cdea..fe7e7b1ba4 100644
--- a/lib/parameter.c
+++ b/lib/parameter.c
@@ -262,8 +262,10 @@ static int param_string_set(struct device_d *dev,
struct param_d *p, const char
value_new = strim(value_new);
*ps->value = value_new;
- if (!ps->set)
+ if (!ps->set) {
+ free(value_save);
return 0;
+ }
Fixing this bug reveals another one. We do a:
value_new = xstrdup(val);
value_new = strim(value_new);
When 'val' has whitespaces at the beginning we no longer free the
pointer originally returned from xstrdup, but the pointer with
whitespaces stripped. I'll queue a fix for this before this patch.
Good catch.
ACK
- Robin
[PATCH 6/9] ARM: boards: protonic-imx6: Register prt-usb boot entry
The worker that polls for a bootable usb device and the first boot.default
target are executed at the same time (after usb_delay seconds). Since
inspecting the usb drive's contents takes some time it loses the race
breaking usb boot.
If we make the usb boot routine a boot entry and prepend it to boot.default
it will always run first when the autoboot timeout expires. If the usb boot
entry fails boot will just fallback to the next entry (i.e. bootchooser).
Signed-off-by: Robin van der Gracht
---
arch/arm/boards/protonic-imx6/board.c | 108 +++---
1 file changed, 79 insertions(+), 29 deletions(-)
diff --git a/arch/arm/boards/protonic-imx6/board.c
b/arch/arm/boards/protonic-imx6/board.c
index e67d8d2f3b..2c4f323799 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -4,10 +4,13 @@
// SPDX-FileCopyrightText: 2020 Oleksij Rempel, Pengutronix
#include
+#include
+#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -21,7 +24,6 @@
#include
#include
#include
-#include
#define GPIO_HW_REV_ID {\
{IMX_GPIO_NR(2, 8), GPIOF_DIR_IN | GPIOF_ACTIVE_LOW, "rev_id0"}, \
@@ -88,8 +90,6 @@ struct prt_imx6_priv {
const char *name;
unsigned int usb_delay;
unsigned int no_usb_check;
- struct work_queue wq;
- struct work_struct work;
};
struct prti6q_rfid_contents {
@@ -286,25 +286,21 @@ exit_usb_mount:
#define OTG_PORTSC1 (MX6_OTG_BASE_ADDR+0x184)
-static void prt_imx6_check_usb_boot_do_work(struct work_struct *w)
+static int prt_imx6_usb_boot(struct bootentry *entry, int verbose, int dryrun)
{
- struct prt_imx6_priv *priv = container_of(w, struct prt_imx6_priv,
work);
+ struct prt_imx6_priv *priv = prt_priv;
struct device_d *dev = priv->dev;
- char *second_word, *bootsrc;
+ char *second_word;
char buf[sizeof("vicut1q recovery")] = {};
- unsigned int v;
+ struct bootm_data bootm_data = {};
ssize_t size;
int fd, ret;
- v = readl(OTG_PORTSC1);
- if ((v & 0x0c00) == 0) /* LS == SE0 ==> nothing connected */
- return;
-
usb_rescan();
ret = prt_imx6_usb_mount(priv);
if (ret)
- return;
+ return ret;
fd = open("/usb/boot_target", O_RDONLY);
if (fd < 0) {
@@ -343,35 +339,90 @@ static void prt_imx6_check_usb_boot_do_work(struct
work_struct *w)
goto exit_usb_boot;
}
+ bootm_data_init_defaults(_data);
+
second_word++;
if (strncmp(second_word, "usb", 3) == 0) {
- bootsrc = "usb";
+ dev_info(dev, "Booting from USB drive\n");
+ bootm_data.os_file = "/usb/linuximage.fit";
} else if (strncmp(second_word, "recovery", 8) == 0) {
- bootsrc = "recovery";
+ dev_info(dev, "Booting internal recovery OS\n");
+ bootm_data.os_file = "/dev/mmc2.5";
} else {
dev_err(dev, "Unknown boot target!\n");
ret = -ENODEV;
goto exit_usb_boot;
}
- dev_info(dev, "detected valid usb boot target file, overwriting boot
to: %s\n", bootsrc);
- ret = setenv("global.boot.default", bootsrc);
+ ret = globalvar_add_simple("linux.bootargs.root",
+ "root=/dev/ram rw rootwait ramdisk_size=196608");
+ if (ret)
+ goto exit_usb_boot;
+
+ if (verbose)
+ bootm_data.verbose = verbose;
+ if (dryrun)
+ bootm_data.dryrun = dryrun;
+
+ ret = bootm_boot(_data);
if (ret)
goto exit_usb_boot;
- return;
+ return 0;
exit_usb_boot:
dev_err(dev, "Failed to run usb boot: %s\n", strerror(-ret));
- return;
+ return ret;
+}
+
+static void prt_imx6_bootentry_release(struct bootentry *entry)
+{
+ free(entry);
+}
+
+static int prt_imx6_bootentry_create(struct bootentries *bootentries, const
char *name)
+{
+ struct bootentry *entry;
+
+ entry = xzalloc(sizeof(*entry));
+ if (!entry)
+ return -ENOMEM;
+
+ entry->me.type = MENU_ENTRY_NORMAL;
+ entry->release = prt_imx6_bootentry_release;
+ entry->boot = prt_imx6_usb_boot;
+ entry->title = xstrdup(name);
+ entry->description = xstrdup("Boot FIT image of a USB drive");
+ bootentries_add_entry(bootentries, entry);
+
+ return 0;
+}
+
+static int prt_imx6_bootentry_provider(struct bootentries *bootentries,
+const char *name)
+{
+ int found = 0;
+ unsigned int v;
+
+ if (strncmp(name, "prt-usb",
[PATCH 2/9] ARM: boards: protonic-imx6: Update comment to match a recent code update
Fixes: 1e817b64bc45968320597344e6b05463edd9c5e5
Signed-off-by: Robin van der Gracht
---
arch/arm/boards/protonic-imx6/board.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boards/protonic-imx6/board.c
b/arch/arm/boards/protonic-imx6/board.c
index 8f8a0c745e..3116f3ac42 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -392,7 +392,7 @@ static int prt_imx6_env_init(struct prt_imx6_priv *priv)
else
priv->usb_delay = 1;
- /* the usb_delay value is used for poller_call_async() */
+ /* the usb_delay value is used for wq_queue_delayed_work() */
delay = basprintf("%d", priv->usb_delay);
ret = setenv("global.autoboot_timeout", delay);
if (ret)
--
2.34.1
[PATCH 8/9] ARM: boards: protonic-imx6: Read serial and mac from fuses if available
Read board serial number from GP1 fuses if available and fall-back to the
i2c RFID eeprom (current method) otherwise.
Signed-off-by: Robin van der Gracht
---
arch/arm/boards/protonic-imx6/board.c | 44 ++-
1 file changed, 37 insertions(+), 7 deletions(-)
diff --git a/arch/arm/boards/protonic-imx6/board.c
b/arch/arm/boards/protonic-imx6/board.c
index 95ec7be793..dd2c041e07 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -211,12 +212,11 @@ static int prt_imx6_set_mac(struct prt_imx6_priv *priv,
return 0;
}
-static int prt_imx6_set_serial(struct prt_imx6_priv *priv,
- struct prti6q_rfid_contents *rfid)
+static int prt_imx6_set_serial(struct prt_imx6_priv *priv, char *serial)
{
- rfid->serial[9] = 0; /* Failsafe */
- dev_info(priv->dev, "Serial number: %s\n", rfid->serial);
- barebox_set_serial_number(rfid->serial);
+ serial[9] = 0; /* Failsafe */
+ dev_info(priv->dev, "Serial number: %s\n", serial);
+ barebox_set_serial_number(serial);
return 0;
}
@@ -240,13 +240,36 @@ static int prt_imx6_read_i2c_mac_serial(struct
prt_imx6_priv *priv)
if (ret)
return ret;
- ret = prt_imx6_set_serial(priv, );
+ ret = prt_imx6_set_serial(priv, rfid.serial);
if (ret)
return ret;
return 0;
}
+#define PRT_IMX6_GP1_FMT_DEC BIT(31)
+
+static int prt_imx6_read_ocotp_serial(struct prt_imx6_priv *priv)
+{
+ int ret;
+ char serial[11];
+ unsigned val;
+
+ ret = imx_ocotp_read_field(OCOTP_GP1, );
+ if (ret) {
+ dev_err(priv->dev, "Failed to read ocotp serial (%i)\n", ret);
+ return ret;
+ }
+
+ if (!(val & PRT_IMX6_GP1_FMT_DEC))
+ return -EINVAL;
+ val &= PRT_IMX6_GP1_FMT_DEC - 1;
+
+ snprintf(serial, sizeof(serial), "%u", val);
+
+ return prt_imx6_set_serial(priv, serial);
+}
+
static int prt_imx6_usb_mount(struct prt_imx6_priv *priv)
{
struct device_d *dev = priv->dev;
@@ -522,7 +545,14 @@ static int prt_imx6_devices_init(void)
prt_imx6_bbu(priv);
- prt_imx6_read_i2c_mac_serial(priv);
+ /*
+* Read serial number from fuses. On success we'll assume the imx_ocotp
+* driver takes care of providing the mac address if needed. On
+* failure we'll fallback to reading and setting serial and mac from an
+* attached RFID eeprom.
+*/
+ if (prt_imx6_read_ocotp_serial(priv) != 0)
+ prt_imx6_read_i2c_mac_serial(priv);
bootentry_register_provider(prt_imx6_bootentry_provider);
--
2.34.1
[PATCH 3/9] ARM: boards: protonic-imx6: Free allocated autoboot_timeout string
Signed-off-by: Robin van der Gracht
---
arch/arm/boards/protonic-imx6/board.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boards/protonic-imx6/board.c
b/arch/arm/boards/protonic-imx6/board.c
index 3116f3ac42..76c26fe296 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -395,6 +395,7 @@ static int prt_imx6_env_init(struct prt_imx6_priv *priv)
/* the usb_delay value is used for wq_queue_delayed_work() */
delay = basprintf("%d", priv->usb_delay);
ret = setenv("global.autoboot_timeout", delay);
+ free(delay);
if (ret)
goto exit_env_init;
}
--
2.34.1
[PATCH 0/9] ARM: boards: protonic-imx6: Updates
Our USB FIT boot is now broken and we can no longer aquire the RFID I2C eeprom chips we're using to store board serial and MAC address. With this patchstack I'm proposing fusing the serial in the ocotp GP1 register as an alternative to the RFID eeprom and making the usb boot sequence a boot entry to avoid the race condition we now have. The patchstack also includes some minor fixes/changes I found during implementation of the above. - Robin Robin van der Gracht (9): ARM: boards: protonic-imx6: prtvt7 hardkey inputs are active low ARM: boards: protonic-imx6: Update comment to match a recent code update ARM: boards: protonic-imx6: Free allocated autoboot_timeout string ARM: boards: protonic-imx6: Always free allocated alias string ARM: boards: protonic-imx6: Remove unsused argument from prt_imx6_usb_mount ARM: boards: protonic-imx6: Register prt-usb boot entry ARM: boards: protonic-imx6: Remove usb_delay from the priv struct ARM: boards: protonic-imx6: Read serial and mac from fuses if available ARM: boards: protonic-imx6: Register serial_number parameter with ocotp arch/arm/boards/protonic-imx6/board.c | 215 +++--- 1 file changed, 159 insertions(+), 56 deletions(-) -- 2.34.1
[PATCH 9/9] ARM: boards: protonic-imx6: Register serial_number parameter with ocotp
Signed-off-by: Robin van der Gracht
---
arch/arm/boards/protonic-imx6/board.c | 39 ---
1 file changed, 36 insertions(+), 3 deletions(-)
diff --git a/arch/arm/boards/protonic-imx6/board.c
b/arch/arm/boards/protonic-imx6/board.c
index dd2c041e07..8436903fd8 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -90,6 +90,7 @@ struct prt_imx6_priv {
unsigned int hw_rev;
const char *name;
unsigned int no_usb_check;
+ char *ocotp_serial;
};
struct prti6q_rfid_contents {
@@ -252,7 +253,6 @@ static int prt_imx6_read_i2c_mac_serial(struct
prt_imx6_priv *priv)
static int prt_imx6_read_ocotp_serial(struct prt_imx6_priv *priv)
{
int ret;
- char serial[11];
unsigned val;
ret = imx_ocotp_read_field(OCOTP_GP1, );
@@ -265,9 +265,31 @@ static int prt_imx6_read_ocotp_serial(struct prt_imx6_priv
*priv)
return -EINVAL;
val &= PRT_IMX6_GP1_FMT_DEC - 1;
- snprintf(serial, sizeof(serial), "%u", val);
+ priv->ocotp_serial = xasprintf("%u", val);
- return prt_imx6_set_serial(priv, serial);
+ return prt_imx6_set_serial(priv, priv->ocotp_serial);
+}
+
+static int prt_imx6_set_ocotp_serial(struct param_d *param, void *driver_priv)
+{
+ struct prt_imx6_priv *priv = driver_priv;
+ int ret;
+ unsigned val;
+
+ ret = kstrtouint(priv->ocotp_serial, 10, );
+ if (ret)
+ return ret;
+
+ if (val & PRT_IMX6_GP1_FMT_DEC)
+ return -ERANGE;
+ val |= PRT_IMX6_GP1_FMT_DEC;
+
+ ret = imx_ocotp_write_field(OCOTP_GP1, val);
+ if (ret)
+ return ret;
+
+ barebox_set_serial_number(priv->ocotp_serial);
+ return 0;
}
static int prt_imx6_usb_mount(struct prt_imx6_priv *priv)
@@ -536,6 +558,8 @@ exit_bbu:
static int prt_imx6_devices_init(void)
{
struct prt_imx6_priv *priv = prt_priv;
+ struct device_d *ocotp_dev;
+ struct param_d *p;
if (!priv)
return 0;
@@ -558,6 +582,15 @@ static int prt_imx6_devices_init(void)
prt_imx6_env_init(priv);
+ ocotp_dev = get_device_by_name("ocotp0");
+ if (ocotp_dev) {
+ p = dev_add_param_string(ocotp_dev, "serial_number",
+prt_imx6_set_ocotp_serial, NULL,
+>ocotp_serial, priv);
+ if (IS_ERR(p))
+ return PTR_ERR(p);
+ }
+
return 0;
}
late_initcall(prt_imx6_devices_init);
--
2.34.1
[PATCH 7/9] ARM: boards: protonic-imx6: Remove usb_delay from the priv struct
It's only used in one function right now.
Signed-off-by: Robin van der Gracht
---
arch/arm/boards/protonic-imx6/board.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boards/protonic-imx6/board.c
b/arch/arm/boards/protonic-imx6/board.c
index 2c4f323799..95ec7be793 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -88,7 +88,6 @@ struct prt_imx6_priv {
unsigned int hw_id;
unsigned int hw_rev;
const char *name;
- unsigned int usb_delay;
unsigned int no_usb_check;
};
@@ -423,6 +422,7 @@ static int prt_imx6_env_init(struct prt_imx6_priv *priv)
const struct prt_machine_data *dcfg = priv->dcfg;
struct device_d *dev = priv->dev;
char *delay, *bootsrc, *boot_targets;
+ unsigned int autoboot_timeout;
int ret;
ret = setenv("global.linux.bootargs.base", "consoleblank=0
vt.color=0x00");
@@ -433,12 +433,11 @@ static int prt_imx6_env_init(struct prt_imx6_priv *priv)
set_autoboot_state(AUTOBOOT_BOOT);
} else {
if (dcfg->flags & PRT_IMX6_USB_LONG_DELAY)
- priv->usb_delay = 4;
+ autoboot_timeout = 4;
else
- priv->usb_delay = 1;
+ autoboot_timeout = 1;
- /* the usb_delay value is used for wq_queue_delayed_work() */
- delay = basprintf("%d", priv->usb_delay);
+ delay = basprintf("%d", autoboot_timeout);
ret = setenv("global.autoboot_timeout", delay);
free(delay);
if (ret)
--
2.34.1
[PATCH 1/9] ARM: boards: protonic-imx6: prtvt7 hardkey inputs are active low
The usb check needs to be skipped unless both keys are pressed simultaneously. Signed-off-by: Robin van der Gracht --- arch/arm/boards/protonic-imx6/board.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boards/protonic-imx6/board.c b/arch/arm/boards/protonic-imx6/board.c index cdbb8debe6..8f8a0c745e 100644 --- a/arch/arm/boards/protonic-imx6/board.c +++ b/arch/arm/boards/protonic-imx6/board.c @@ -645,7 +645,7 @@ static int prt_imx6_init_prtvt7(struct prt_imx6_priv *priv) gpio_direction_input(GPIO_KEY_F6); gpio_direction_input(GPIO_KEY_CYCLE); - if (gpio_get_value(GPIO_KEY_CYCLE) && gpio_get_value(GPIO_KEY_F6)) + if (gpio_get_value(GPIO_KEY_CYCLE) || gpio_get_value(GPIO_KEY_F6)) priv->no_usb_check = 1; return 0; -- 2.34.1
[PATCH 4/9] ARM: boards: protonic-imx6: Always free allocated alias string
The memory required to store the string needs to be freed on success
as well since it's no longer used.
Signed-off-by: Robin van der Gracht
---
arch/arm/boards/protonic-imx6/board.c | 12
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/arm/boards/protonic-imx6/board.c
b/arch/arm/boards/protonic-imx6/board.c
index 76c26fe296..7978553cb8 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -681,24 +681,22 @@ static int prt_imx6_rfid_fixup(struct prt_imx6_priv *priv,
}
i2c_node = of_find_node_by_alias(root, alias);
+ kfree(alias);
if (!i2c_node) {
dev_err(priv->dev, "Unsupported i2c adapter\n");
- ret = -ENODEV;
- goto free_alias;
+ return -ENODEV;
}
eeprom_node_name = basprintf("/eeprom@%x", dcfg->i2c_addr);
if (!eeprom_node_name) {
- ret = -ENOMEM;
- goto free_alias;
+ return -ENOMEM;
}
node = of_create_node(i2c_node, eeprom_node_name);
if (!node) {
dev_err(priv->dev, "Failed to create node %s\n",
eeprom_node_name);
- ret = -ENOMEM;
- goto free_eeprom;
+ return -ENOMEM;
}
ret = of_property_write_string(node, "compatible", "atmel,24c256");
@@ -722,8 +720,6 @@ static int prt_imx6_rfid_fixup(struct prt_imx6_priv *priv,
return 0;
free_eeprom:
kfree(eeprom_node_name);
-free_alias:
- kfree(alias);
exit_error:
dev_err(priv->dev, "Failed to apply fixup: %pe\n", ERR_PTR(ret));
return ret;
--
2.34.1
[PATCH 5/9] ARM: boards: protonic-imx6: Remove unsused argument from prt_imx6_usb_mount
Removing this unused pointer pointer relieves the caller from freeing the
memory afterwards.
Signed-off-by: Robin van der Gracht
---
arch/arm/boards/protonic-imx6/board.c | 12 +++-
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/arch/arm/boards/protonic-imx6/board.c
b/arch/arm/boards/protonic-imx6/board.c
index 7978553cb8..e67d8d2f3b 100644
--- a/arch/arm/boards/protonic-imx6/board.c
+++ b/arch/arm/boards/protonic-imx6/board.c
@@ -248,7 +248,7 @@ static int prt_imx6_read_i2c_mac_serial(struct
prt_imx6_priv *priv)
return 0;
}
-static int prt_imx6_usb_mount(struct prt_imx6_priv *priv, char **usbdisk)
+static int prt_imx6_usb_mount(struct prt_imx6_priv *priv)
{
struct device_d *dev = priv->dev;
const char *path;
@@ -267,8 +267,6 @@ static int prt_imx6_usb_mount(struct prt_imx6_priv *priv,
char **usbdisk)
ret = mount(path, NULL, "usb", NULL);
if (ret)
goto exit_usb_mount;
-
- *usbdisk = strdup("disk0.0");
return 0;
}
@@ -278,8 +276,6 @@ static int prt_imx6_usb_mount(struct prt_imx6_priv *priv,
char **usbdisk)
ret = mount(path, NULL, "usb", NULL);
if (ret)
goto exit_usb_mount;
-
- *usbdisk = strdup("disk0");
return 0;
}
@@ -294,7 +290,7 @@ static void prt_imx6_check_usb_boot_do_work(struct
work_struct *w)
{
struct prt_imx6_priv *priv = container_of(w, struct prt_imx6_priv,
work);
struct device_d *dev = priv->dev;
- char *second_word, *bootsrc, *usbdisk;
+ char *second_word, *bootsrc;
char buf[sizeof("vicut1q recovery")] = {};
unsigned int v;
ssize_t size;
@@ -306,7 +302,7 @@ static void prt_imx6_check_usb_boot_do_work(struct
work_struct *w)
usb_rescan();
- ret = prt_imx6_usb_mount(priv, );
+ ret = prt_imx6_usb_mount(priv);
if (ret)
return;
@@ -363,12 +359,10 @@ static void prt_imx6_check_usb_boot_do_work(struct
work_struct *w)
if (ret)
goto exit_usb_boot;
- free(usbdisk);
return;
exit_usb_boot:
dev_err(dev, "Failed to run usb boot: %s\n", strerror(-ret));
- free(usbdisk);
return;
}
--
2.34.1
[PATCH] lib: parameter: Free previous string value in param_string_set
When the parameter has no set function the previous (saved_value) is not
freed up on completion.
Signed-off-by: Robin van der Gracht
---
lib/parameter.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/parameter.c b/lib/parameter.c
index adc3c7cdea..fe7e7b1ba4 100644
--- a/lib/parameter.c
+++ b/lib/parameter.c
@@ -262,8 +262,10 @@ static int param_string_set(struct device_d *dev, struct
param_d *p, const char
value_new = strim(value_new);
*ps->value = value_new;
- if (!ps->set)
+ if (!ps->set) {
+ free(value_save);
return 0;
+ }
ret = ps->set(p, p->driver_priv);
if (ret) {
--
2.34.1
[PATCH] hab: habv4: Don't reset index when checking for more events
If index is reset and the event record pointer is NULL the size of the
indexed event will be written to 'len'.
Currently this results in always printing the buffer overflow error if
there is a HAB event. The index shouldn't be reset to get the size of the
'next' unhandled event. This can occur if there are more events of a single
type than the event buffer (data) can hold.
Telling the user to recompile with an incomplete additional size
suggestion seems useless so I changed it to something more generic.
Signed-off-by: Robin van der Gracht
---
drivers/hab/habv4.c | 11 ++-
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/hab/habv4.c b/drivers/hab/habv4.c
index e94f82754..3df1d8d3d 100644
--- a/drivers/hab/habv4.c
+++ b/drivers/hab/habv4.c
@@ -504,7 +504,7 @@ static int habv4_get_status(const struct habv4_rvt *rvt)
{
uint8_t data[256];
uint32_t len;
- uint32_t index = 0;
+ uint32_t cnt, index = 0;
enum hab_status status;
enum hab_config config = 0x0;
enum hab_state state = 0x0;
@@ -540,6 +540,7 @@ static int habv4_get_status(const struct habv4_rvt *rvt)
len = sizeof(data);
index++;
}
+ cnt = index;
len = sizeof(data);
index = 0;
@@ -551,12 +552,12 @@ static int habv4_get_status(const struct habv4_rvt *rvt)
len = sizeof(data);
index++;
}
+ cnt += index;
- /* Check reason for stopping */
+ /* Check if there are more events */
len = sizeof(data);
- index = 0;
- if (rvt->report_event(HAB_STATUS_ANY, index, NULL, ) ==
HAB_STATUS_SUCCESS)
- pr_err("ERROR: Recompile with larger event data buffer (at
least %d bytes)\n\n", len);
+ if (rvt->report_event(HAB_STATUS_ANY, cnt, NULL, ) ==
HAB_STATUS_SUCCESS)
+ pr_err("WARNING: There are more (undisplayed) events\n");
return -EPERM;
}
--
2.25.1
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
[PATCH] nvmem: ocotp: fix fusing on fsl, imx6q-ocotp compatible boards
The imx6q_ocotp_data struct is incomplete resulting in a crash when trying
to read/blow fuses.
Signed-off-by: Robin van der Gracht
---
drivers/nvmem/ocotp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/nvmem/ocotp.c b/drivers/nvmem/ocotp.c
index 356e9bf11..cee50955e 100644
--- a/drivers/nvmem/ocotp.c
+++ b/drivers/nvmem/ocotp.c
@@ -833,6 +833,8 @@ static struct imx_ocotp_data imx6q_ocotp_data = {
.mac_offsets = { MAC_OFFSET_0 },
.format_mac = imx_ocotp_format_mac,
.set_timing = imx6_ocotp_set_timing,
+ .fuse_blow = imx6_fuse_blow_addr,
+ .fuse_read = imx6_fuse_read_addr,
};
static struct imx_ocotp_data imx6sl_ocotp_data = {
--
2.25.1
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
simplefb question
Hello Andre, Sascha,
I'm trying to setup simplefb on my imx6 solo board, and ran into some
issues and I'm hoping you could
clear things up.
I'm running:
- Barebox 2018.07.0
- Linux 4.19
I checked with latest versions but the relevant code seems unchanged.
On Barebox console I ran:
barebox@BOARD:/ of_dump /framebuffer
framebuffer {
display = <0x5a>;
};
barebox@BOARD:/ fb0.enable=1
imx-ipuv3-crtc imx-ipuv3-crtc0: ipu_crtc_mode_set: mode->xres: 800
imx-ipuv3-crtc imx-ipuv3-crtc0: ipu_crtc_mode_set: mode->yres: 480
WARNING: imx-ipuv3-crtc imx-ipuv3-crtc0: videomode adapted for IPU
restriction
barebox@BOARD:/ fb0.register_simplefb=1
barebox@BOARD:/ oftree -s dump.dtb
fb0: created r8g8b8 node(I added that mode)
barebox@BOARD:/ of_dump -f dump.dtb /framebuffer
framebuffer {
display = <0x5a>;
compatible = "simple-framebuffer";
reg = <0x19ca9000 0x177000>;
width = <0x320>;
height = <0x1e0>;
stride = <0xc80>;
format = "r8g8b8";
status = "okay";
};
# Side-note: Kernel doc says this device-tree node has to in the /chosen
section..?
The framebuffer is allocated in 'drivers/video/imx-ipu-v3/ipufb.c'
ipufb_activate_var() with dma_alloc_writecombine().
This failed with my Barebox version, and found that Sascha fixed that
upstream, so I applied these:
- ARM: MMU: fix arch_remap_range() across section boundaries
- ARM: mmu: fix cache flushing when replacing a section with a PTE
- ARM: MMU: fix wrong dma_flush_range in arm_create_pte()
# So far so good.
When I boot Linux I get the following WARN()
[0.071950] [ cut here ]
[0.071974] WARNING: CPU: 0 PID: 1 at arch/arm/mm/ioremap.c:309
__arm_ioremap_pfn_caller+0x1ec/0x210
[0.071980] Modules linked in:
[0.071998] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
4.19.0-20190401-1-00290-gd141d156c111f-dirty #1023
[0.072007] Hardware name: Freescale i.MX6 Quad/DualLite (Device
Tree)
[0.072036] [<8010ffec>] (unwind_backtrace) from [<8010b8dc>]
(show_stack+0x10/0x14)
[0.072053] [<8010b8dc>] (show_stack) from [<8091d5fc>]
(dump_stack+0x88/0x9c)
[0.072074] [<8091d5fc>] (dump_stack) from [<8011e5d8>]
(__warn+0xd4/0xf0)
[0.072092] [<8011e5d8>] (__warn) from [<8011e634>]
(warn_slowpath_null+0x40/0x48)
[0.072109] [<8011e634>] (warn_slowpath_null) from [<80115230>]
(__arm_ioremap_pfn_caller+0x1ec/0x210)
[0.072124] [<80115230>] (__arm_ioremap_pfn_caller) from [<80115298>]
(__arm_ioremap_caller+0x44/0x54)
[0.072141] [<80115298>] (__arm_ioremap_caller) from [<80407900>]
(simplefb_probe+0x1fc/0x898)
[0.072162] [<80407900>] (simplefb_probe) from [<804ade34>]
(platform_drv_probe+0x48/0x98)
[0.072179] [<804ade34>] (platform_drv_probe) from [<804ac23c>]
(really_probe+0x1f4/0x2b8)
[0.072193] [<804ac23c>] (really_probe) from [<804ac464>]
(driver_probe_device+0x60/0x164)
[0.072207] [<804ac464>] (driver_probe_device) from [<804ac644>]
(__driver_attach+0xdc/0xe0)
[0.072224] [<804ac644>] (__driver_attach) from [<804aa540>]
(bus_for_each_dev+0x74/0xb4)
[0.072242] [<804aa540>] (bus_for_each_dev) from [<804ab6cc>]
(bus_add_driver+0x1bc/0x200)
[0.072258] [<804ab6cc>] (bus_add_driver) from [<804acf50>]
(driver_register+0x7c/0x114)
[0.072276] [<804acf50>] (driver_register) from [<80e1b1f8>]
(simplefb_init+0x14/0x8c)
[0.072293] [<80e1b1f8>] (simplefb_init) from [<80102618>]
(do_one_initcall+0x7c/0x1a8)
[0.072313] [<80102618>] (do_one_initcall) from [<80e00e40>]
(kernel_init_freeable+0x148/0x1dc)
[0.072334] [<80e00e40>] (kernel_init_freeable) from [<80930c78>]
(kernel_init+0x8/0x110)
[0.072349] [<80930c78>] (kernel_init) from [<801010d8>]
(ret_from_fork+0x14/0x3c)
[0.072358] Exception stack(0x8b84bfb0 to 0x8b84bff8)
[0.072369] bfa0:
[0.072381] bfc0:
[0.072391] bfe0: 0013
[0.072435] ---[ end trace d197b304a56deeb0 ]---
[0.072463] simple-framebuffer: probe of 19ca9000.framebuffer failed
with error -12
If I understand correctly, this is due to the use of ioremap_wc on a RAM
region. I was hoping you could tell me what I'm doing wrong here, or if
I'm looking at a bug.
My mtype = MT_DEVICE_WC. (Due to the in kernel use of ioremap_wc())
My pfn = 0x19ca9
In my kernel .config i have:
CONFIG_HAVE_ARCH_PFN_VALID=y
Kind regards,
Robin van der Gracht
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
[PATCH v2] clk: imx: clk-imx6ul: The i.mx6ul has no aips_tz3 clock
The clock was mapped on CG15 (gpio2_clocks) in the CCRG0 register.
Reviewed-by: Fabio Estevam <fabio.este...@nxp.com>
Signed-off-by: Robin van der Gracht <ro...@protonic.nl>
---
drivers/clk/imx/clk-imx6ul.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
index f28660d..5879661 100644
--- a/drivers/clk/imx/clk-imx6ul.c
+++ b/drivers/clk/imx/clk-imx6ul.c
@@ -71,7 +71,7 @@ static struct clk *clks[IMX6UL_CLK_END];
static struct clk_onecell_data clk_data;
static int const clks_init_on[] __initconst = {
- IMX6UL_CLK_AIPSTZ1, IMX6UL_CLK_AIPSTZ2, IMX6UL_CLK_AIPSTZ3,
+ IMX6UL_CLK_AIPSTZ1, IMX6UL_CLK_AIPSTZ2,
IMX6UL_CLK_AXI, IMX6UL_CLK_ARM, IMX6UL_CLK_ROM,
IMX6UL_CLK_MMDC_P0_FAST, IMX6UL_CLK_MMDC_P0_IPG,
};
@@ -275,7 +275,6 @@ static int imx6_ccm_probe(struct device_d *dev)
clks[IMX6UL_CLK_GPT2_SERIAL]= imx_clk_gate2("gpt2_serial",
"perclk", base + 0x68,26);
clks[IMX6UL_CLK_UART2_IPG] = imx_clk_gate2("uart2_ipg","ipg",
base + 0x68,28);
clks[IMX6UL_CLK_UART2_SERIAL] = imx_clk_gate2("uart2_serial",
"uart_podf",base + 0x68,28);
- clks[IMX6UL_CLK_AIPSTZ3]= imx_clk_gate2("aips_tz3", "ahb",
base + 0x68,30);
/* CCGR1 */
clks[IMX6UL_CLK_ECSPI1] = imx_clk_gate2("ecspi1",
"ecspi_podf", base + 0x6c,0);
--
2.9.3
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
[PATCH] clk: imx: clk-imx6ul: The i.mx6ul has no aips_tx3 clock
The clock was mapped on CG15 (gpio2_clocks) in the CCRG0 register.
Signed-off-by: Robin van der Gracht <ro...@protonic.nl>
---
drivers/clk/imx/clk-imx6ul.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
index f28660d..5879661 100644
--- a/drivers/clk/imx/clk-imx6ul.c
+++ b/drivers/clk/imx/clk-imx6ul.c
@@ -71,7 +71,7 @@ static struct clk *clks[IMX6UL_CLK_END];
static struct clk_onecell_data clk_data;
static int const clks_init_on[] __initconst = {
- IMX6UL_CLK_AIPSTZ1, IMX6UL_CLK_AIPSTZ2, IMX6UL_CLK_AIPSTZ3,
+ IMX6UL_CLK_AIPSTZ1, IMX6UL_CLK_AIPSTZ2,
IMX6UL_CLK_AXI, IMX6UL_CLK_ARM, IMX6UL_CLK_ROM,
IMX6UL_CLK_MMDC_P0_FAST, IMX6UL_CLK_MMDC_P0_IPG,
};
@@ -275,7 +275,6 @@ static int imx6_ccm_probe(struct device_d *dev)
clks[IMX6UL_CLK_GPT2_SERIAL]= imx_clk_gate2("gpt2_serial",
"perclk", base + 0x68,26);
clks[IMX6UL_CLK_UART2_IPG] = imx_clk_gate2("uart2_ipg","ipg",
base + 0x68,28);
clks[IMX6UL_CLK_UART2_SERIAL] = imx_clk_gate2("uart2_serial",
"uart_podf",base + 0x68,28);
- clks[IMX6UL_CLK_AIPSTZ3]= imx_clk_gate2("aips_tz3", "ahb",
base + 0x68,30);
/* CCGR1 */
clks[IMX6UL_CLK_ECSPI1] = imx_clk_gate2("ecspi1",
"ecspi_podf", base + 0x6c,0);
--
2.9.3
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
[PATCH] clk: imx: clk-imx6ul: Use anatop_base pointer for consistency
The anatop_base pointer was unused, but instead of removing it, assign
and use it for readability like clk-imx6 and clk-imx6sx do.
Signed-off-by: Robin van der Gracht <ro...@protonic.nl>
---
drivers/clk/imx/clk-imx6ul.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
index 0ca0652..f28660d 100644
--- a/drivers/clk/imx/clk-imx6ul.c
+++ b/drivers/clk/imx/clk-imx6ul.c
@@ -91,6 +91,7 @@ static int imx6_ccm_probe(struct device_d *dev)
int i;
struct device_node *ccm_node = dev->device_node;
+ anatop_base = IOMEM(MX6_ANATOP_BASE_ADDR);
iores = dev_request_mem_resource(dev, 0);
if (IS_ERR(iores))
return PTR_ERR(iores);
@@ -100,8 +101,6 @@ static int imx6_ccm_probe(struct device_d *dev)
clks[IMX6UL_CLK_DUMMY] = clk_fixed("dummy", 0);
- base = IOMEM(MX6_ANATOP_BASE_ADDR);
-
clks[IMX6UL_PLL1_BYPASS_SRC] = imx_clk_mux("pll1_bypass_src", base +
0x00, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
clks[IMX6UL_PLL2_BYPASS_SRC] = imx_clk_mux("pll2_bypass_src", base +
0x30, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
clks[IMX6UL_PLL3_BYPASS_SRC] = imx_clk_mux("pll3_bypass_src", base +
0x10, 14, 1, pll_bypass_src_sels, ARRAY_SIZE(pll_bypass_src_sels));
--
2.9.3
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
[PATCH] clk: imx: clk-imx6ul: Fixed conditional for enabling USB phy clocks
Signed-off-by: Robin van der Gracht <ro...@protonic.nl>
---
drivers/clk/imx/clk-imx6ul.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
index 72b5fa2..0ca0652 100644
--- a/drivers/clk/imx/clk-imx6ul.c
+++ b/drivers/clk/imx/clk-imx6ul.c
@@ -392,7 +392,7 @@ static int imx6_ccm_probe(struct device_d *dev)
for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
clk_enable(clks[clks_init_on[i]]);
- if (IS_ENABLED(CONFIG_USB_MXS_PHY)) {
+ if (IS_ENABLED(CONFIG_USB_IMX_PHY)) {
clk_enable(clks[IMX6UL_CLK_USBPHY1_GATE]);
clk_enable(clks[IMX6UL_CLK_USBPHY2_GATE]);
}
--
2.9.3
___
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox
Imx28 sdram initialisation?
Hello, Currently i'm working on our new board, equipped with a imx287 processor, using the 'karo-tx28' and 'freescale-mx28-evk' bsp as reference for our own bsp. But I can't find sdram initialisation, lowlevel routines, or dcd support for imx28. So i'm wondering how and where this is done for the boards mentioned before. Reguards, -- Robin van der Gracht Protonic Holland. tel.: +31 (0) 229 212928 fax.: +31 (0) 229 210930 Factorij 36 / 1689 AL Zwaag ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
drivers/power ? drivers/hwmon ?
Hello, Currently i'm writing a BSP for our new board. I quickly figured out there are no driver sections: hwmon and power. Why don't they exist (yet)? Can i give a shot at creating them? reguards, -- Robin van der Gracht Protonic Holland. tel.: +31 (0) 229 212928 fax.: +31 (0) 229 210930 Factorij 36 / 1689 AL Zwaag ___ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox
