Re: Problem upon startup with halted USB device on RaspberryPi 4
Am 15.08.23 um 13:49 schrieb Massimo Pegorer:
>
>
> Il giorno lun 14 ago 2023 alle ore 11:11 Harry Waschkeit
> mailto:harry.waschk...@conplement.de>>
> ha scritto:
>
> Am 13.08.23 um 19:37 schrieb Michal Suchánek:
> > Hello,
>
> Hi again,
>
> thanks for your answers!
>
> > On Sat, Aug 12, 2023 at 08:31:56PM +0200, Massimo Pegorer wrote:
> >> Hi Harry,
> >>
> >> Il giorno lun 7 ago 2023 alle ore 11:02 Harry Waschkeit <
> >> harry.waschk...@conplement.de
> <mailto:harry.waschk...@conplement.de>> ha scritto:
> >>
> >>> Hi,
> >>>
> >>> I have a RaspberryPi 4 where on one USB port a Sierra Wireless LTE
> >>> module (EM7455) is attached via an PCIe-to-USB adapter.
> >>> The boot image I use gets built by Yocto with the help of poky
> >>> (kirkstone) and meta-raspberry (beside a few other layers) which
> >>> incorporates U-Boot 22.01.
> >>>
> >>> When I apply power to the board it will come up until scanning USB
> >>> devices (for potential boot devices), but then throw a BUG end
> reset the
> >>> board.
> >>
> >>
> >> Do you need USB boot devices? If not, I would build U-Boot
> without USB
> >> support.
>
> Yeah, I am aware of that possibility, thanks for your hint anyway,
> but ...
>
> >
> > That would be great workaround, However, enabling a device should not
> > break the board. If that's the case the driver is clearly broken.
>
>
> Of course, mine is just a workaround, nothing more. To make hardware combo
> working, and just for specific use case (if I properly understood). Not
> for typical
> usage of rPi. About the driver: can be broken, or the root cause can be the
> PCIe-to-USB bridge misbehaving, or both.
>
> > Also rPi typically uses USB keyboard for boot-time input which
> makes the
> > workaround not so great.
>
>
> For U-Boot booting phase, or later on for Linux or other OS boot phase? I do
> not know too much about this specific board.
>
> ... but as Michal pointed out, the rpi - at least rpi 400 - should be
> considered to regularly use USB devices for normal operation, so
> completely disabling USB is not really an option here.
>
>
> Disabling USB support in U-Boot (via defconfig) would not affect Linux (or
> other OS) USB functions in any way, if this is what you are concerning about
> with "completely disabling USB".
>
> My USB knowledge is too little to point the finger on the problem
> but my
> guess still is that U-Boot is only using a part of Linux's USB driver
> for bringing up devices, thereby omitting some of the error handling
> which would be done in the Linux kernel in some (maybe concurrent) way.
>
>
> I suggest to address this email also directly to maintainers:
>
> ./scripts/get_maintainer.pl <http://get_maintainer.pl>
> drivers/usb/host/xhci-ring.c
> Bin Meng mailto:bmeng...@gmail.com>>
> (maintainer:USB xHCI)
> Marek Vasut mailto:ma...@denx.de>> (maintainer:USB)
> u-boot@lists.denx.de <mailto:u-boot@lists.denx.de> (open list)
yes, thanks for the hint, I should have done this right from the start ...
@Bin and @Marek: even though I decided to deactivate USB in my u-boot
for the time being, could you please have a look at my observations and
give me your thoughts about whether and how this problem will be
addressed in future?
Thanks in advance! :-)
Regards,
Harry
> Regards,
> Massimo
>
> And given the bunch of boot messages "WARN halted endpoint." with the
> patch I mentioned, chances are that the patch is either not the right
> one or at least incomplete.
> But the general approach to ignore halted endpoints appears completely
> reasonable to me when it avoids a hanging of boot looping system.
> I cannot tell whether there should instead be some cleverer
> treatment to
> get such a device running ("Reset Endpoint" from what I read), though.
>
> Regards,
> Harry
>
> > Thanks
> >
> > Michal
> >
> >>
> >> Massimo
> >>
> >>> This continues as long as the modem is unplugged.
> >>> The exact error message is:
> >>>
> >>> scanning bus xhci_pci for devices... WARN halted endpoint,
> queuing URB
> >&g
Re: Problem upon startup with halted USB device on RaspberryPi 4
Am 13.08.23 um 19:37 schrieb Michal Suchánek:
> Hello,
Hi again,
thanks for your answers!
> On Sat, Aug 12, 2023 at 08:31:56PM +0200, Massimo Pegorer wrote:
>> Hi Harry,
>>
>> Il giorno lun 7 ago 2023 alle ore 11:02 Harry Waschkeit <
>> harry.waschk...@conplement.de> ha scritto:
>>
>>> Hi,
>>>
>>> I have a RaspberryPi 4 where on one USB port a Sierra Wireless LTE
>>> module (EM7455) is attached via an PCIe-to-USB adapter.
>>> The boot image I use gets built by Yocto with the help of poky
>>> (kirkstone) and meta-raspberry (beside a few other layers) which
>>> incorporates U-Boot 22.01.
>>>
>>> When I apply power to the board it will come up until scanning USB
>>> devices (for potential boot devices), but then throw a BUG end reset the
>>> board.
>>
>>
>> Do you need USB boot devices? If not, I would build U-Boot without USB
>> support.
Yeah, I am aware of that possibility, thanks for your hint anyway, but ...
>
> That would be great workaround, However, enabling a device should not
> break the board. If that's the case the driver is clearly broken.
>
> Also rPi typically uses USB keyboard for boot-time input which makes the
> workaround not so great.
... but as Michal pointed out, the rpi - at least rpi 400 - should be
considered to regularly use USB devices for normal operation, so
completely disabling USB is not really an option here.
My USB knowledge is too little to point the finger on the problem but my
guess still is that U-Boot is only using a part of Linux's USB driver
for bringing up devices, thereby omitting some of the error handling
which would be done in the Linux kernel in some (maybe concurrent) way.
And given the bunch of boot messages "WARN halted endpoint." with the
patch I mentioned, chances are that the patch is either not the right
one or at least incomplete.
But the general approach to ignore halted endpoints appears completely
reasonable to me when it avoids a hanging of boot looping system.
I cannot tell whether there should instead be some cleverer treatment to
get such a device running ("Reset Endpoint" from what I read), though.
Regards,
Harry
> Thanks
>
> Michal
>
>>
>> Massimo
>>
>>> This continues as long as the modem is unplugged.
>>> The exact error message is:
>>>
>>> scanning bus xhci_pci for devices... WARN halted endpoint, queuing URB
>>> anyway.
>>> Unexpected XHCI event TRB, skipping... (3af519b0 0004 1300
>>> 02008401)
>>> BUG at drivers/usb/host/xhci-ring.c:503/abort_td()!
>>> BUG!
>>> resetting ...
>>>
>>> Some internet research brought up a patch (1) for
>>> drivers/usb/host/xhci-ring.c where halted devices simply get ignored
>>> during enumeration:
>>>
>>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>>> index acf437104a..6d995f446e 100644
>>> --- a/drivers/usb/host/xhci-ring.c
>>> +++ b/drivers/usb/host/xhci-ring.c
>>> @@ -227,7 +227,8 @@ static int prepare_ring(struct xhci_ctrl *ctrl,
>>> struct xhci_ring *ep_ring,
>>> puts("WARN waiting for error on ep to be cleared\n");
>>> return -EINVAL;
>>> case EP_STATE_HALTED:
>>> - puts("WARN halted endpoint, queueing URB anyway.\n");
>>> + puts("WARN halted endpoint.\n");
>>> + return -EPIPE;
>>> case EP_STATE_STOPPED:
>>> case EP_STATE_RUNNING:
>>> debug("EP STATE RUNNING.\n");
>>>
>>> The driver file itself stems from the Linux sources, so I'm pretty sure
>>> that it is correct in that context.
>>> Even though I'm not really into USB stuff and therefore I cannot not
>>> really follow the discussion for the proposed change, in my eyes the
>>> patch could be reasonable for U-Boot nevertheless given that a) in my
>>> experience driver code is often used in a simplified way in U-Boot
>>> compared to Linux kernel, and b) it's all about board start-up only and
>>> not about full OS use with all bells, whistles and full error handling.
>>>
>>> Of course, I might be wrong while missing some important other use or
>>> corner cases, so I wanted to check here :-)
>>>
>>> At least, what I can say: with this patch I see a bunch of these warning
>>> messages but the board comes up and is usable in the end by Linux.
>>>
>>> fwiw: my internet search also showed another patch labelled in the first
>>> place "[PATCH] pi4: fix crash when issuing usb reset&
Problem upon startup with halted USB device on RaspberryPi 4
Hi,
I have a RaspberryPi 4 where on one USB port a Sierra Wireless LTE
module (EM7455) is attached via an PCIe-to-USB adapter.
The boot image I use gets built by Yocto with the help of poky
(kirkstone) and meta-raspberry (beside a few other layers) which
incorporates U-Boot 22.01.
When I apply power to the board it will come up until scanning USB
devices (for potential boot devices), but then throw a BUG end reset the
board.
This continues as long as the modem is unplugged.
The exact error message is:
scanning bus xhci_pci for devices... WARN halted endpoint, queuing URB
anyway.
Unexpected XHCI event TRB, skipping... (3af519b0 0004 1300 02008401)
BUG at drivers/usb/host/xhci-ring.c:503/abort_td()!
BUG!
resetting ...
Some internet research brought up a patch (1) for
drivers/usb/host/xhci-ring.c where halted devices simply get ignored
during enumeration:
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index acf437104a..6d995f446e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -227,7 +227,8 @@ static int prepare_ring(struct xhci_ctrl *ctrl,
struct xhci_ring *ep_ring,
puts("WARN waiting for error on ep to be cleared\n");
return -EINVAL;
case EP_STATE_HALTED:
- puts("WARN halted endpoint, queueing URB anyway.\n");
+ puts("WARN halted endpoint.\n");
+ return -EPIPE;
case EP_STATE_STOPPED:
case EP_STATE_RUNNING:
debug("EP STATE RUNNING.\n");
The driver file itself stems from the Linux sources, so I'm pretty sure
that it is correct in that context.
Even though I'm not really into USB stuff and therefore I cannot not
really follow the discussion for the proposed change, in my eyes the
patch could be reasonable for U-Boot nevertheless given that a) in my
experience driver code is often used in a simplified way in U-Boot
compared to Linux kernel, and b) it's all about board start-up only and
not about full OS use with all bells, whistles and full error handling.
Of course, I might be wrong while missing some important other use or
corner cases, so I wanted to check here :-)
At least, what I can say: with this patch I see a bunch of these warning
messages but the board comes up and is usable in the end by Linux.
fwiw: my internet search also showed another patch labelled in the first
place "[PATCH] pi4: fix crash when issuing usb reset" (2) that didn't
make it into the particular U-Boot 22.01 but was integrated right after
that version in commit "Prepare v2022.04" with hash
e4b6ebd3de982ae7185dbf689a030e73fd06e0d2 (3).
As I first hoped I could address my problem by just adding this patch I
got promptly disappointed. The only thing I gained was to now get
endless error messages followed by retries until I power-cycled the
board (causing to start all over):
USB XHCI 1.00
scanning bus xhci_pci for devices... WARN halted endpoint, queueing URB
anyway.
Unexpected XHCI event TRB, skipping... (3afd6b30 0004 1300 02008401)
XHCI abort timeout
WARN halted endpoint, queueing URB anyway.
Unexpected XHCI event TRB, skipping... (3afd6b40 0004 1300 02008401)
XHCI abort timeout
WARN halted endpoint, queueing URB anyway.
Unexpected XHCI event TRB, skipping... (3afd6b50 0004 1300 02008401)
XHCI abort timeout
WARN halted endpoint, queueing URB anyway.
[...]
To me it means that this specific PCIe-to-USB bridge might misbehave
somehow,
but the final question is: what are the odds to get that patch into
official U-boot, or any other fix/quirk to make my hardware combo working?
And also interesting: if I cannot hope for an upstream change, what
potential risks I would have to accept when keeping my patch?
Regards,
Harry
(1)
https://linux-usb.vger.kernel.narkive.com/VW4VTVDU/patch-usb-host-xhci-fix-halted-endpoint-handling
(2)
https://lore.kernel.org/all/3d4ece94-932a-25dd-ef29-0ddfb4a51...@denx.de/T/
(3)
https://source.denx.de/u-boot/u-boot/-/commit/e4b6ebd3de982ae7185dbf689a030e73fd06e0d2
--
i.A. Harry Waschkeit
Senior Embedded Engineer
conplement AG
Südwestpark 92
90449 Nürnberg
Handelsregister: HRB 22863
Registergericht: Nürnberg
Vertreten durch: Britta Waligora und Thomas Wahle
Vorsitzender des Aufsichtsrates: Lorenz von Schröder
Problem upon startup with halted USB device on RaspberryPi 4
Hi,
I have a RaspberryPi 4 where on one USB port a Sierra Wireless LTE
module (EM7455) is attached via an PCIe-to-USB adapter.
The boot image I use gets built by Yocto with the help of poky
(kirkstone) and meta-raspberry (beside a few other layers) which
incorporates U-Boot 22.01.
When I apply power to the board it will come up until scanning USB
devices (for potential boot devices), but then throw a BUG end reset the
board.
This continues as long as the modem is unplugged.
The exact error message is:
scanning bus xhci_pci for devices... WARN halted endpoint, queuing URB
anyway.
Unexpected XHCI event TRB, skipping... (3af519b0 0004 1300 02008401)
BUG at drivers/usb/host/xhci-ring.c:503/abort_td()!
BUG!
resetting ...
Some internet research brought up a patch (1) for
drivers/usb/host/xhci-ring.c where halted devices simply get ignored
during enumeration:
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index acf437104a..6d995f446e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -227,7 +227,8 @@ static int prepare_ring(struct xhci_ctrl *ctrl,
struct xhci_ring *ep_ring,
puts("WARN waiting for error on ep to be cleared\n");
return -EINVAL;
case EP_STATE_HALTED:
- puts("WARN halted endpoint, queueing URB anyway.\n");
+ puts("WARN halted endpoint.\n");
+ return -EPIPE;
case EP_STATE_STOPPED:
case EP_STATE_RUNNING:
debug("EP STATE RUNNING.\n");
The driver file itself stems from the Linux sources, so I'm pretty sure
that it is correct in that context.
Even though I'm not really into USB stuff and therefore I cannot not
really follow the discussion for the proposed change, in my eyes the
patch could be reasonable for U-Boot nevertheless given that a) in my
experience driver code is often used in a simplified way in U-Boot
compared to Linux kernel, and b) it's all about board start-up only and
not about full OS use with all bells, whistles and full error handling.
Of course, I might be wrong while missing some important other use or
corner cases, so I wanted to check here :-)
At least, what I can say: with this patch I see a bunch of these warning
messages but the board comes up and is usable in the end by Linux.
fwiw: my internet search also showed another patch labelled in the first
place "[PATCH] pi4: fix crash when issuing usb reset" (2) that didn't
make it into the particular U-Boot 22.01 but was integrated right after
that version in commit "Prepare v2022.04" with hash
e4b6ebd3de982ae7185dbf689a030e73fd06e0d2 (3).
As I first hoped I could address my problem by just adding this patch I
got promptly disappointed. The only thing I gained was to now get
endless error messages followed by retries until I power-cycled the
board (causing to start all over):
USB XHCI 1.00
scanning bus xhci_pci for devices... WARN halted endpoint, queueing URB
anyway.
Unexpected XHCI event TRB, skipping... (3afd6b30 0004 1300 02008401)
XHCI abort timeout
WARN halted endpoint, queueing URB anyway.
Unexpected XHCI event TRB, skipping... (3afd6b40 0004 1300 02008401)
XHCI abort timeout
WARN halted endpoint, queueing URB anyway.
Unexpected XHCI event TRB, skipping... (3afd6b50 0004 1300 02008401)
XHCI abort timeout
WARN halted endpoint, queueing URB anyway.
[...]
To me it means that this specific PCIe-to-USB bridge might misbehave
somehow,
but the final question is: what are the odds to get that patch into
official U-boot, or any other fix/quirk to make my hardware combo working?
And also interesting: if I cannot hope for an upstream change, what
potential risks I would have to accept when keeping my patch?
Regards,
Harry
(1)
https://linux-usb.vger.kernel.narkive.com/VW4VTVDU/patch-usb-host-xhci-fix-halted-endpoint-handling
(2)
https://lore.kernel.org/all/3d4ece94-932a-25dd-ef29-0ddfb4a51...@denx.de/T/
(3)
https://source.denx.de/u-boot/u-boot/-/commit/e4b6ebd3de982ae7185dbf689a030e73fd06e0d2
--
i.A. Harry Waschkeit
Senior Embedded Engineer
conplement AG
Südwestpark 92
90449 Nürnberg
Handelsregister: HRB 22863
Registergericht: Nürnberg
Vertreten durch: Britta Waligora und Thomas Wahle
Vorsitzender des Aufsichtsrates: Lorenz von Schröder
Re: [PATCH v3 1/1] env: sf: single function env_sf_save()
On 03.03.21 00:06, Sean Anderson wrote:
On 3/2/21 1:09 PM, Harry Waschkeit wrote:
Hello Sean,
On 02.03.21 00:10, Sean Anderson wrote:
On 3/1/21 11:39 AM, Harry Waschkeit wrote:
Hi again,
gentle ping for that patch, also in view of subsequently sent patch ...
https://lists.denx.de/pipermail/u-boot/2021-February/439797.html
... which saw a competing patch by Patrick Delaunay a week later:
https://lists.denx.de/pipermail/u-boot/2021-February/440769.html
However, the latter doesn't seem to take embedded environments into account.
Can you give an example of where your patch would work while Patrick's wouldn't?
I didn't dig too deep into Patrick's patch and maybe I simply miss
something important, but I don't even see an spi_flash_erase() in his
code, so I'm wondering how it could work at all.
Writing to SPI flash can only set bits to 0. To set bits to 1 you must
erase them. Conceptually, write(buf) does
flash[i] &= buf[i];
And erase() does
flash[i] = -1;
So writing 0s always succeeds, and we are certain to invalidate the
environment (as long as our hash has a non-zero value for an all-zero
input).
Sure.
So it's only the name that is confusing because there's no erase happening at
all in env_sf_erase(), a real flash erase is necessary afterwards if you want
to write data to environment space.
I think it would be worth a comment in the function then, maybe also others get
confused what a function called something with erase actually does with flash
data ;-)
What I do see in his code is spi_flash_write() with a CONFIG_ENV_SIZE
worth of zeroes to the environment environment offset, but this can
only work for an erased flash as far as I know.
env_sf_save always erases the environment before flashing. So your patch
effectively erases env twice before writing to it.
Yeah, you're right, it was made under the false assumption that the erase
function actually should erase something ... ;-/
And erasing the flash part where the environment is stored should take
an environment sizes below sector size into account; the rest of the
environment sector could be used otherwise, i.e. CONFIG_ENV_SIZE <
CONFIG_ENV_SECT_SIZE.
Right, but typically we have a write granularity of one byte for SPI
flashes. So you can clear only the environment, and let enf_sf_save deal
with other data in the sector.
Correct.
So my patches are actually worthless ... interesting that Patrick came up with
his patches only right after my attempts to add that functionality, but not
discussing my approach.
I'd loved to make a small contribution to U-Boot, maybe I've more luck next
time ... ;-)
Best regards,
Harry
--Sean
Function env_sf_save() takes care of all of that, so does the
env_sf_erase() in my patch, Patrick's version of env_sf_erase() does
not.
(side note: using more than a flash sector for environment and sharing
the last one with different data isn't handled at all at the moment,
probably because it was never needed)
Best regards,
Harry
Thanks.
--Sean
Best regards,
Harry
On 02.02.21 09:21, Harry Waschkeit wrote:
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
Reviewed-by: Stefan Roese
---
Change in v3:
- no change in patch, only added "reviewed-by" to commit log
Change in v2:
- remove one more #ifdef, instead take advantage of compiler attribute
__maybe_unused for one variable used only in case of redundant
environments
env/sf.c | 130 ++-
1 file changed, 41 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..199114fd3b 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,14 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char *saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+ __maybe_unused char flag = ENV_REDUND_OBSOLETE;
ret = setup_flash_device();
if (ret)
@@ -81,27 +82,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABL
Re: [PATCH v3 1/1] env: sf: single function env_sf_save()
Hello Sean,
On 02.03.21 00:10, Sean Anderson wrote:
On 3/1/21 11:39 AM, Harry Waschkeit wrote:
Hi again,
gentle ping for that patch, also in view of subsequently sent patch ...
https://lists.denx.de/pipermail/u-boot/2021-February/439797.html
... which saw a competing patch by Patrick Delaunay a week later:
https://lists.denx.de/pipermail/u-boot/2021-February/440769.html
However, the latter doesn't seem to take embedded environments into account.
Can you give an example of where your patch would work while Patrick's wouldn't?
I didn't dig too deep into Patrick's patch and maybe I simply miss something
important, but I don't even see an spi_flash_erase() in his code, so I'm
wondering how it could work at all.
What I do see in his code is spi_flash_write() with a CONFIG_ENV_SIZE worth of
zeroes to the environment environment offset, but this can only work for an
erased flash as far as I know.
And erasing the flash part where the environment is stored should take an
environment sizes below sector size into account; the rest of the environment
sector could be used otherwise, i.e. CONFIG_ENV_SIZE < CONFIG_ENV_SECT_SIZE.
Function env_sf_save() takes care of all of that, so does the env_sf_erase() in
my patch, Patrick's version of env_sf_erase() does not.
(side note: using more than a flash sector for environment and sharing the last
one with different data isn't handled at all at the moment, probably because it
was never needed)
Best regards,
Harry
Thanks.
--Sean
Best regards,
Harry
On 02.02.21 09:21, Harry Waschkeit wrote:
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
Reviewed-by: Stefan Roese
---
Change in v3:
- no change in patch, only added "reviewed-by" to commit log
Change in v2:
- remove one more #ifdef, instead take advantage of compiler attribute
__maybe_unused for one variable used only in case of redundant
environments
env/sf.c | 130 ++-
1 file changed, 41 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..199114fd3b 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,14 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char *saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+ __maybe_unused char flag = ENV_REDUND_OBSOLETE;
ret = setup_flash_device();
if (ret)
@@ -81,27 +82,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +116,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
if (ret)
goto
Re: [PATCH v3 1/1] env: sf: single function env_sf_save()
Hi again,
gentle ping for that patch, also in view of subsequently sent patch ...
https://lists.denx.de/pipermail/u-boot/2021-February/439797.html
... which saw a competing patch by Patrick Delaunay a week later:
https://lists.denx.de/pipermail/u-boot/2021-February/440769.html
However, the latter doesn't seem to take embedded environments into account.
Best regards,
Harry
On 02.02.21 09:21, Harry Waschkeit wrote:
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
Reviewed-by: Stefan Roese
---
Change in v3:
- no change in patch, only added "reviewed-by" to commit log
Change in v2:
- remove one more #ifdef, instead take advantage of compiler attribute
__maybe_unused for one variable used only in case of redundant
environments
env/sf.c | 130 ++-
1 file changed, 41 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..199114fd3b 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,14 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char*saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char*saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+ __maybe_unused char flag = ENV_REDUND_OBSOLETE;
ret = setup_flash_device();
if (ret)
@@ -81,27 +82,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +116,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
if (ret)
goto done;
puts("Writing to SPI flash...");
- ret = spi_flash_write(env_flash, env_new_offset,
- CONFIG_ENV_SIZE, &env_new);
+ ret = spi_flash_write(env_flash, offset,
+ CONFIG_ENV_SIZE, &env_new);
if (ret)
goto done;
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
- ret = spi_flash_write(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_write(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
- ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
- sizeof(env_new.flags), &flag);
- if (ret)
-
[PATCH 1/1] env: sf: add support for env erase
One new function env_sf_erase() is added to support clearing flash
region(s) holding environment configuration.
It treats redundant configuration properly when U-Boot config option
SYS_REDUNDAND_ENVIRONMENT is active.
Unfortunately, none of the #if CONFIG_IS_ENABLED() sections can be
avoided.
Signed-off-by: Harry Waschkeit
---
This patch is based on patch ...
"[PATCH v2 1/1] env: sf: single function env_sf_save()"
(https://lists.denx.de/pipermail/u-boot/2021-February/439418.html)
... or ...
"[PATCH v3 1/1] env: sf: single function env_sf_save()"
(https://lists.denx.de/pipermail/u-boot/2021-February/439577.html)
... which only added the "reviewed-by" from Stefan Roese compared to v2.
env/sf.c | 83
1 file changed, 83 insertions(+)
diff --git a/env/sf.c b/env/sf.c
index 199114fd3b..eedf59f2df 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -157,6 +157,86 @@ static int env_sf_save(void)
return ret;
}
+#if CONFIG_IS_ENABLED(CMD_ERASEENV)
+static int env_sf_erase(void)
+{
+ char*saved_buffer = NULL;
+ u32 saved_size, saved_offset, sector;
+ ulong offset;
+ ulong offsets[] = {
+ CONFIG_ENV_OFFSET,
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ CONFIG_ENV_OFFSET_REDUND
+#endif
+ };
+ int i;
+ int ret;
+
+ ret = setup_flash_device();
+ if (ret)
+ return ret;
+
+ /* get temporary storage if sector is larger than env (i.e. embedded) */
+ if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
+ saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
+ saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
+ if (!saved_buffer) {
+ ret = -ENOMEM;
+ goto done;
+ }
+ }
+
+ sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
+
+ /* simply erase both environments, retaining non-env data (if any) */
+ for (i = 0; i < ARRAY_SIZE(offsets); i++) {
+ offset = offsets[i];
+
+ if (saved_buffer) {
+ saved_offset = offset + CONFIG_ENV_SIZE;
+ ret = spi_flash_read(env_flash, saved_offset,
+saved_size, saved_buffer);
+ if (ret)
+ goto done;
+ }
+
+ /* next print will only happen with redundant environments */
+ if (i)
+ puts("Redund:");
+
+ puts("Erasing SPI flash...");
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
+ if (ret)
+ goto done;
+
+ if (saved_buffer) {
+ puts("Writing non-environment data to SPI flash...");
+ ret = spi_flash_write(env_flash, saved_offset,
+ saved_size, saved_buffer);
+ if (ret)
+ goto done;
+ }
+
+ puts("done\n");
+ }
+
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ /* here we know that both env sections are cleared */
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+
+ gd->env_valid = ENV_INVALID;
+#endif
+
+ done:
+ if (saved_buffer)
+ free(saved_buffer);
+
+ return ret;
+}
+#endif /* CONFIG_IS_ENABLED(CMD_ERASEENV) */
+
#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
static int env_sf_load(void)
{
@@ -263,4 +343,7 @@ U_BOOT_ENV_LOCATION(sf) = {
#if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0)
.init = env_sf_init,
#endif
+#if CONFIG_IS_ENABLED(CMD_ERASEENV)
+ .erase = env_sf_erase,
+#endif
};
--
2.29.0
duagon Germany GmbH
Neuwieder Straße 1-7 90411 Nuernberg
Geschaeftsfuehrer: Dr. Michael Goldbach - Matthias Kamolz - Kalina Scott -
Handelsregister AG Nuernberg HRB 5540
Re: [PATCH v3 1/1] env: sf: single function env_sf_save()
On 02.02.21 15:54, Stefan Roese wrote:
On 02.02.21 15:43, Harry Waschkeit wrote:
On 02.02.21 10:30, Stefan Roese wrote:
On 02.02.21 09:21, Harry Waschkeit wrote:
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
Reviewed-by: Stefan Roese
---
Change in v3:
- no change in patch, only added "reviewed-by" to commit log
JFYI:
No need to re-send this patch with this added RB tag, as I already did
send a new RB to the last mail as reply. Patchwork collects these tags
when sent to the list. So you only need to include them, if you send
a new patch version.
thanks for this hint, obviously I step into it all the time ...
Ok, lesson learnt, let's see what I can do wrong next time ... ;-/
Back on topic: I guess that was all I needed to do so that the patch will get
merged when its time comes.
Yes, now we (you) need a bit of patience, so that other people might
review this patch as well. And usually it will get handled after some
time (depending on the development stage of U-Boot, merge window open
or shortly before release etc).
Ok, no problem with that.
That also means that I should wait with submission of the other patch (adding "env
erase" support) until this one is merged.
Otherwise the patch would need to be significantly different ...
Hmm, I guess it would have been better to not send that patch standalone but
instead as a first one in a two-patch series where the second one adds the new
functionality on top of the clean-up.
Anyway, something to keep in mind for next time :-)
It does not hurt of course to check this from time to time and
"trigger" the maintainer of the subsystem or the custodian this
patch is delegated to nothing is happening here for a too long
time (like more than 1 month).
Alright, good to know :-)
Thanks,
Harry
Thanks,
Stefan
If not, please let me know.
Thanks,
Harry
Thanks,
Stefan
Change in v2:
- remove one more #ifdef, instead take advantage of compiler attribute
__maybe_unused for one variable used only in case of redundant
environments
env/sf.c | 130 ++-
1 file changed, 41 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..199114fd3b 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,14 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char *saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+ __maybe_unused char flag = ENV_REDUND_OBSOLETE;
ret = setup_flash_device();
if (ret)
@@ -81,27 +82,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +116,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = sp
Re: [PATCH v3 1/1] env: sf: single function env_sf_save()
On 02.02.21 10:30, Stefan Roese wrote:
On 02.02.21 09:21, Harry Waschkeit wrote:
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
Reviewed-by: Stefan Roese
---
Change in v3:
- no change in patch, only added "reviewed-by" to commit log
JFYI:
No need to re-send this patch with this added RB tag, as I already did
send a new RB to the last mail as reply. Patchwork collects these tags
when sent to the list. So you only need to include them, if you send
a new patch version.
thanks for this hint, obviously I step into it all the time ...
Ok, lesson learnt, let's see what I can do wrong next time ... ;-/
Back on topic: I guess that was all I needed to do so that the patch will get
merged when its time comes.
If not, please let me know.
Thanks,
Harry
Thanks,
Stefan
Change in v2:
- remove one more #ifdef, instead take advantage of compiler attribute
__maybe_unused for one variable used only in case of redundant
environments
env/sf.c | 130 ++-
1 file changed, 41 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..199114fd3b 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,14 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char *saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+ __maybe_unused char flag = ENV_REDUND_OBSOLETE;
ret = setup_flash_device();
if (ret)
@@ -81,27 +82,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +116,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
if (ret)
goto done;
puts("Writing to SPI flash...");
- ret = spi_flash_write(env_flash, env_new_offset,
- CONFIG_ENV_SIZE, &env_new);
+ ret = spi_flash_write(env_flash, offset,
+ CONFIG_ENV_SIZE, &env_new);
if (ret)
goto done;
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
- ret = spi_flash_write(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_write(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
- ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
- sizeof(env_new.flags), &flag);
- if (ret)
- goto done;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
+ sizeof(env_new.flags), &flag);
+ if (ret)
+ goto done;
-
[PATCH v3 1/1] env: sf: single function env_sf_save()
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
Reviewed-by: Stefan Roese
---
Change in v3:
- no change in patch, only added "reviewed-by" to commit log
Change in v2:
- remove one more #ifdef, instead take advantage of compiler attribute
__maybe_unused for one variable used only in case of redundant
environments
env/sf.c | 130 ++-
1 file changed, 41 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..199114fd3b 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,14 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char*saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char*saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+ __maybe_unused char flag = ENV_REDUND_OBSOLETE;
ret = setup_flash_device();
if (ret)
@@ -81,27 +82,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +116,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
if (ret)
goto done;
puts("Writing to SPI flash...");
- ret = spi_flash_write(env_flash, env_new_offset,
- CONFIG_ENV_SIZE, &env_new);
+ ret = spi_flash_write(env_flash, offset,
+ CONFIG_ENV_SIZE, &env_new);
if (ret)
goto done;
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
- ret = spi_flash_write(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_write(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
- ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
- sizeof(env_new.flags), &flag);
- if (ret)
- goto done;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ ret = spi_flash_write(env_flash, env_offset + offsetof(env_t,
flags),
+ sizeof(env_new.flags), &flag);
+ if (ret)
+ goto done;
- puts("done\n");
+ puts("done\n");
- gd->env_valid = gd->env_val
Re: [PATCH v2 1/1] env: sf: single function env_sf_save()
On 01.02.21 13:07, Stefan Roese wrote:
On 01.02.21 13:03, Harry Waschkeit wrote:
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
Reviewed-by: Stefan Roese
BTW: Please re-add the tags (RB etc) into subsequent patch versions,
so that they are available in the git history.
Oh, yeah, of course, sorry for that ... :-(
Thanks,
Harry
Thanks,
Stefan
---
Changes in v2:
- remove one more #ifdef, instead take advantage of compiler attribute
__maybe_unused for one variable used only in case of redundant
environments
env/sf.c | 130 ++-
1 file changed, 41 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..199114fd3b 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,14 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char *saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+ __maybe_unused char flag = ENV_REDUND_OBSOLETE;
ret = setup_flash_device();
if (ret)
@@ -81,27 +82,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +116,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
if (ret)
goto done;
puts("Writing to SPI flash...");
- ret = spi_flash_write(env_flash, env_new_offset,
- CONFIG_ENV_SIZE, &env_new);
+ ret = spi_flash_write(env_flash, offset,
+ CONFIG_ENV_SIZE, &env_new);
if (ret)
goto done;
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
- ret = spi_flash_write(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_write(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
- ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
- sizeof(env_new.flags), &flag);
- if (ret)
- goto done;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
+ sizeof(env_new.flags), &flag);
+ if (ret)
+ goto done;
- puts("done\n");
+ puts("done\n");
- gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
+ gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
- printf("Valid environment: %d\n", (int)gd->env_valid);
+ printf("Valid environment: %d\n", (int)gd->env_valid);
+#else
+ puts("done\n");
+#endif
don
[PATCH v2 1/1] env: sf: single function env_sf_save()
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
---
Changes in v2:
- remove one more #ifdef, instead take advantage of compiler attribute
__maybe_unused for one variable used only in case of redundant
environments
env/sf.c | 130 ++-
1 file changed, 41 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..199114fd3b 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,14 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char*saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char*saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+ __maybe_unused char flag = ENV_REDUND_OBSOLETE;
ret = setup_flash_device();
if (ret)
@@ -81,27 +82,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +116,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
if (ret)
goto done;
puts("Writing to SPI flash...");
- ret = spi_flash_write(env_flash, env_new_offset,
- CONFIG_ENV_SIZE, &env_new);
+ ret = spi_flash_write(env_flash, offset,
+ CONFIG_ENV_SIZE, &env_new);
if (ret)
goto done;
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
- ret = spi_flash_write(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_write(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
- ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
- sizeof(env_new.flags), &flag);
- if (ret)
- goto done;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ ret = spi_flash_write(env_flash, env_offset + offsetof(env_t,
flags),
+ sizeof(env_new.flags), &flag);
+ if (ret)
+ goto done;
- puts("done\n");
+ puts("done\n");
- gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
+ gd->env_valid = gd->env_valid == ENV_REDUND ? ENV
Re: [PATCH] env: sf: single function env_sf_save()
On 30.01.21 10:07, Stefan Roese wrote:
Hi Harry,
Hi Stefan,
On 28.01.21 08:21, Harry Waschkeit wrote:
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
---
env/sf.c | 132 ++-
1 file changed, 43 insertions(+), 89 deletions(-)
Nice diffstat. ;)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..c60bd1deed 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,16 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char *saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ char flag = ENV_REDUND_OBSOLETE;
+#endif
I would prefer this to the #ifdef here:
__maybe_unused char flag = ENV_REDUND_OBSOLETE;
Ok, will change that and send a v2.
Looks good aside from this:
Reviewed-by: Stefan Roese
Thanks for your review! :-)
Cheers,
Harry
Thanks,
Stefan
ret = setup_flash_device();
if (ret)
@@ -81,27 +84,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +118,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
if (ret)
goto done;
puts("Writing to SPI flash...");
- ret = spi_flash_write(env_flash, env_new_offset,
- CONFIG_ENV_SIZE, &env_new);
+ ret = spi_flash_write(env_flash, offset,
+ CONFIG_ENV_SIZE, &env_new);
if (ret)
goto done;
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
- ret = spi_flash_write(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_write(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
- ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
- sizeof(env_new.flags), &flag);
- if (ret)
- goto done;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
+ sizeof(env_new.flags), &flag);
+ if (ret)
+ goto done;
- puts("done\n");
+ puts("done\n");
- gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
+ gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
- printf("Valid environment: %d\n", (int)gd->env_valid);
+ printf("Valid environment: %d\n", (int)gd->env_valid);
+#else
+ puts("done\n");
+#endif
done:
if (saved_buffer)
@@ -146,6 +159,7 @@ static int
Re: [PATCH] env: sf: single function env_sf_save()
Hi again Stefan,
On 29.01.21 08:16, Stefan Roese wrote:
On 28.01.21 12:21, Harry Waschkeit wrote:
Even though an "if (CONFIG_IS_ENABLED(...))" would statically evaluate to '0'
without active redundancy in environments, the parser sees the syntax error of the
non-existing structure element.
So, I don't see a chance for avoiding the #if construct here, too.
Let me know if I miss something :-)
I agree its not easy or perhaps even impossible. One idea would be
to introduce a union in the struct with "flags" also defines in the
non-redundant case but not being used here. But this would result
in very non-intuitive code AFAICT. So better not go this way.
That would feel very strange to me, too.
Perhaps somebody else has a quick idea on how to handle this without
introduncing #ifdef's here?
It would be possible to introduce a macro like env_set_flags(envp, flag) in
env.h that only evaluates to something in case of active redundancy, sure.
But that's not even half of a solution, because also variables env_offset and
env_new_offset which are set in that section are only defined if necessary,
i.e. if CONFIG_SYS_REDUNDAND_ENVIRONMENT is set (btw also protected by #if
while not possible otherwise).
The trade-off here is between code duplication - which I removed by combining
two very similar separate functions for the two situations w/ and w/o
redundancy in one - and in-line pre-compiler protected regions within one
function.
While I fully agree that the latter should be avoided as far as possible, it is
not avoidable here afaics.
And avoiding code duplication here outweighs the few pre-compiler occurrences
in my eyes, but that's just my € 0,02 :-)
I also agree (now). If nobody else comes up with a better idea, then
please proceed with the current implementation to remove the code
duplication.
sorry for the newbie question, I'm not familiar (yet) with the normal procedure
and I don't want to keep that ball from rolling ;-/
As far as I understood you finally agreed on my patch, but you didn't give it a
"reviewed-by" all the same; so do you still expect me to change my patch in some way or
are you waiting for a "reviewed-by" from someone else to give that a go?
Why I am also asking: I have another small patch in he pipe that bases on the changed
sources and that adds "env erase" support on top of that :-)
Thanks,
Harry
Thanks,
Stefan
Viele Grüße,
Harry
Thanks,
Stefan
Cheers,
Harry
Thanks,
Stefan
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +118,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
if (ret)
goto done;
puts("Writing to SPI flash...");
- ret = spi_flash_write(env_flash, env_new_offset,
- CONFIG_ENV_SIZE, &env_new);
+ ret = spi_flash_write(env_flash, offset,
+ CONFIG_ENV_SIZE, &env_new);
if (ret)
goto done;
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
- ret = spi_flash_write(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_write(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
- ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
- sizeof(env_new.flags), &flag);
- if (ret)
- goto done;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
+ sizeof(env_new.flags), &flag);
+ if (ret)
+ goto done;
- puts("done\n");
+ puts("done\n");
- gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
+ gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
- printf("Valid environment: %d\n", (int)gd->env_valid);
+ printf(&quo
Re: [PATCH] env: sf: single function env_sf_save()
On 28.01.21 11:11, Stefan Roese wrote:
Hi Harry,
On 28.01.21 11:00, Harry Waschkeit wrote:
Hi Stefan,
thanks a lot for your prompt reply :-)
And sorry that I didn't manage to continue on that for such a long time ...
On 28.01.21 09:50, Stefan Roese wrote:
Hi Harry,
On 28.01.21 08:21, Harry Waschkeit wrote:
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
I like this idea very much, thanks for working on this. One comment
below..
---
env/sf.c | 132 ++-
1 file changed, 43 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..c60bd1deed 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,16 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char *saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ char flag = ENV_REDUND_OBSOLETE;
+#endif
No more #idef's if possible please. See below...
As checkpatch.pl throws warnings for each such #if[def] occurrence, I already
tried to get rid of them.
But I can't see how this shall be possible in a structure declaration.
I'm eager to learn how to do that if possible, though :-)
Ah, right. It's probably not possible for this for a struct declaration.
But the variable above can be included always, right?
Sure it could, but then we would have to live with a compiler warning about an
unused variable I think.
ret = setup_flash_device();
if (ret)
@@ -81,27 +84,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
Can you please try to add this without adding #ifdef's but using
if (CONFIG_IS_ENABLE(SYS_REDUNDAND_ENVIRONMENT)) instead?
I tried that, too, but it was doomed to fail, because element flags in env_t
structure is only defined if redundant environment is enabled.
Too bad we didn't include "flag" for non-redundant env in the beginning.
Now its too late.
That would probably have been the best solution to avoid #if's, at least in
that file.
But keep in mind: as a last consequence of this approach you would end up in
structures bearing variables for all eventualities, however unlikely these are
needed, blowing up the memory footprint for no reason.
In my opinion such things need to be thought through thoroughly :-)
Even though an "if (CONFIG_IS_ENABLED(...))" would statically evaluate to '0'
without active redundancy in environments, the parser sees the syntax error of the
non-existing structure element.
So, I don't see a chance for avoiding the #if construct here, too.
Let me know if I miss something :-)
I agree its not easy or perhaps even impossible. One idea would be
to introduce a union in the struct with "flags" also defines in the
non-redundant case but not being used here. But this would result
in very non-intuitive code AFAICT. So better not go this way.
That would feel very strange to me, too.
Perhaps somebody else has a quick idea on how to handle this without
introduncing #ifdef's here?
It would be possible to introduce a macro like env_set_flags(envp, flag) in
env.h that only evaluates to something in case of active redundancy, sure.
But that's not even half of a solution, because also variables env_offset and
env_new_offset which are set in that section are only defined if necessary,
i.e. if CONFIG_SYS_REDUNDAND_ENVIRONMENT is set (btw also protected by #if
while not possible otherwise).
The trade-off here is between code duplication - which I removed by combining
two very s
Re: [PATCH] env: sf: single function env_sf_save()
Hi Stefan,
thanks a lot for your prompt reply :-)
And sorry that I didn't manage to continue on that for such a long time ...
On 28.01.21 09:50, Stefan Roese wrote:
Hi Harry,
On 28.01.21 08:21, Harry Waschkeit wrote:
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
I like this idea very much, thanks for working on this. One comment
below..
---
env/sf.c | 132 ++-
1 file changed, 43 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..c60bd1deed 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,16 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char *saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char *saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ char flag = ENV_REDUND_OBSOLETE;
+#endif
No more #idef's if possible please. See below...
As checkpatch.pl throws warnings for each such #if[def] occurrence, I already
tried to get rid of them.
But I can't see how this shall be possible in a structure declaration.
I'm eager to learn how to do that if possible, though :-)
ret = setup_flash_device();
if (ret)
@@ -81,27 +84,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
Can you please try to add this without adding #ifdef's but using
if (CONFIG_IS_ENABLE(SYS_REDUNDAND_ENVIRONMENT)) instead?
I tried that, too, but it was doomed to fail, because element flags in env_t
structure is only defined if redundant environment is enabled.
Even though an "if (CONFIG_IS_ENABLED(...))" would statically evaluate to '0'
without active redundancy in environments, the parser sees the syntax error of the
non-existing structure element.
So, I don't see a chance for avoiding the #if construct here, too.
Let me know if I miss something :-)
Cheers,
Harry
Thanks,
Stefan
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +118,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
if (ret)
goto done;
puts("Writing to SPI flash...");
- ret = spi_flash_write(env_flash, env_new_offset,
- CONFIG_ENV_SIZE, &env_new);
+ ret = spi_flash_write(env_flash, offset,
+ CONFIG_ENV_SIZE, &env_new);
if (ret)
goto done;
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
- ret = spi_flash_write(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_write(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
- ret = spi_flash_write
[PATCH] env: sf: single function env_sf_save()
Instead of implementing redundant environments in two very similar
functions env_sf_save(), handle redundancy in one function, placing the
few differences in appropriate pre-compiler sections depending on config
option CONFIG_ENV_OFFSET_REDUND.
Additionally, several checkpatch complaints were addressed.
This patch is in preparation for adding support for env erase.
Signed-off-by: Harry Waschkeit
---
env/sf.c | 132 ++-
1 file changed, 43 insertions(+), 89 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..c60bd1deed 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -66,13 +66,16 @@ static int setup_flash_device(void)
return 0;
}
-#if defined(CONFIG_ENV_OFFSET_REDUND)
static int env_sf_save(void)
{
env_t env_new;
- char*saved_buffer = NULL, flag = ENV_REDUND_OBSOLETE;
+ char*saved_buffer = NULL;
u32 saved_size, saved_offset, sector;
+ ulong offset;
int ret;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ charflag = ENV_REDUND_OBSOLETE;
+#endif
ret = setup_flash_device();
if (ret)
@@ -81,27 +84,33 @@ static int env_sf_save(void)
ret = env_export(&env_new);
if (ret)
return -EIO;
- env_new.flags = ENV_REDUND_ACTIVE;
- if (gd->env_valid == ENV_VALID) {
- env_new_offset = CONFIG_ENV_OFFSET_REDUND;
- env_offset = CONFIG_ENV_OFFSET;
- } else {
- env_new_offset = CONFIG_ENV_OFFSET;
- env_offset = CONFIG_ENV_OFFSET_REDUND;
- }
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ env_new.flags = ENV_REDUND_ACTIVE;
+
+ if (gd->env_valid == ENV_VALID) {
+ env_new_offset = CONFIG_ENV_OFFSET_REDUND;
+ env_offset = CONFIG_ENV_OFFSET;
+ } else {
+ env_new_offset = CONFIG_ENV_OFFSET;
+ env_offset = CONFIG_ENV_OFFSET_REDUND;
+ }
+ offset = env_new_offset;
+#else
+ offset = CONFIG_ENV_OFFSET;
+#endif
/* Is the sector larger than the env (i.e. embedded) */
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
- saved_offset = env_new_offset + CONFIG_ENV_SIZE;
+ saved_offset = offset + CONFIG_ENV_SIZE;
saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
if (!saved_buffer) {
ret = -ENOMEM;
goto done;
}
- ret = spi_flash_read(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_read(env_flash, saved_offset, saved_size,
+saved_buffer);
if (ret)
goto done;
}
@@ -109,35 +118,39 @@ static int env_sf_save(void)
sector = DIV_ROUND_UP(CONFIG_ENV_SIZE, CONFIG_ENV_SECT_SIZE);
puts("Erasing SPI flash...");
- ret = spi_flash_erase(env_flash, env_new_offset,
- sector * CONFIG_ENV_SECT_SIZE);
+ ret = spi_flash_erase(env_flash, offset,
+ sector * CONFIG_ENV_SECT_SIZE);
if (ret)
goto done;
puts("Writing to SPI flash...");
- ret = spi_flash_write(env_flash, env_new_offset,
- CONFIG_ENV_SIZE, &env_new);
+ ret = spi_flash_write(env_flash, offset,
+ CONFIG_ENV_SIZE, &env_new);
if (ret)
goto done;
if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
- ret = spi_flash_write(env_flash, saved_offset,
- saved_size, saved_buffer);
+ ret = spi_flash_write(env_flash, saved_offset, saved_size,
+ saved_buffer);
if (ret)
goto done;
}
- ret = spi_flash_write(env_flash, env_offset + offsetof(env_t, flags),
- sizeof(env_new.flags), &flag);
- if (ret)
- goto done;
+#if CONFIG_IS_ENABLED(SYS_REDUNDAND_ENVIRONMENT)
+ ret = spi_flash_write(env_flash, env_offset + offsetof(env_t,
flags),
+ sizeof(env_new.flags), &flag);
+ if (ret)
+ goto done;
- puts("done\n");
+ puts("done\n");
- gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
+ gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID :
ENV_REDUND;
- printf("Valid environment: %d\n", (int)gd->env_valid);
+ printf("
Re: [PATCH] env: sf: add support for env erase
On 10/9/20 7:00 PM, Sean Anderson wrote: On 10/9/20 12:43 PM, Harry Waschkeit wrote: Hi Sean, thanks for your try and sorry for the inconvenience my beginner's mistakes have caused :-( It is definitely no good idea to use copy&paste with patch data, I should have guessed that beforehand ... You *can* do it, you just have to configure your mail client correctly. However, it gets pretty tedious when you have a lot of patches :) Yeah, guess so ... ;-/ I suggest configuring git send-email. If you are going to be making more patch series, also check out tools/patman. I'll definitely have a look at that, sooner or later. Anyway, concerning the complaints from checkpatch.pl: I indeed ran checkpatch.pl on my patch prior to sending it - the real one, not the one as pasted into the mail ;-/ The alignment warnings simply result from the fact that I adhered to the style used in that file already, you can easily verify that by running checkpatch.pl on the complete file. Please keep new code in the correct style. For example, you have + ret = spi_flash_read(env_flash, saved_offset, +saved_size, saved_buffer); which is aligned properly, but later on you have + ret = spi_flash_read(env_flash, saved_offset, + saved_size, saved_buffer); which is not. Ok, got it. For the warnings with "IS_ENABLED(CONFIG...)" I don't see what I could do to get around them: all three occurrences are about compiling functions into the code depending on CONFIG_CMD_ERASEENV. Two times it is the new function env_sf_erase(), one variant for normal and the other for redundand environment handling. The third time it is used to define this new method into the structure U_BOOT_ENV_LOCATION(sf). The macro IS_ENABLED can be used both in C code and in preprocessor directives. See include/linux/kconfig.h for details. Hmm, that's strange, I tried that one but the complaints remained. Chances are I did something wrong so I will have a look at kconfig.h to get also around that. The sign-off problem I guess is probably caused by the check not accepting name in reverse order, isn't it? Yes. The format is "First Last ". This will be the easiest part then :-) Anyway, I will change my user.name to match the order in the mail address so next patch is hopefully correct. So please let me know what else I should do beside sending a properly formatted patch ;-/ See below. I will take care of that before resending my patch (v2 then, right?). Yes. On 10/9/20 3:55 PM, Sean Anderson wrote: On 10/8/20 1:27 PM, Harry Waschkeit wrote: Command "env erase" didn't work even though CONFIG_CMD_ERASEENV was defined, because serial flash environment routines didn't implement erase method. Signed-off-by: Waschkeit, Harry --- Hi Harry, I wanted to test out your patch, but I couldn't get it to apply. It appears that your mail program has replaced the tabs with spaces, so git can't figure out how to apply it. I tried to fix it by performing the substitutions s/^\(.\) /\1\t/g s//\t/g but it still wouldn't apply. In addition, checkpatch has a few warnings: $ scripts/checkpatch.pl env-sf-add-support-for-env-erase.patch WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible #152: FILE: env/sf.c:149: +#ifdef CONFIG_CMD_ERASEENV WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible #240: FILE: env/sf.c:318: +#ifdef CONFIG_CMD_ERASEENV CHECK: Alignment should match open parenthesis #260: FILE: env/sf.c:338: +ret = spi_flash_read(env_flash, saved_offset, +saved_size, saved_buffer); CHECK: Alignment should match open parenthesis #269: FILE: env/sf.c:347: +ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET, +sector * CONFIG_ENV_SECT_SIZE); CHECK: Alignment should match open parenthesis #276: FILE: env/sf.c:354: +ret = spi_flash_write(env_flash, saved_offset, +saved_size, saved_buffer); WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible #307: FILE: env/sf.c:437: +#if defined(CONFIG_CMD_ERASEENV) && defined(CONFIG_ENV_ADDR) WARNING: Missing Signed-off-by: line by nominal patch author 'Harry Waschkeit ' total: 0 errors, 4 warnings, 3 checks, 158 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. env-sf-add-support-for-env-erase.patch has style problems, please review. NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE PREFER_ETHER_ADDR_COPY USLEEP_RANGE NOTE: If any of the errors are false
Re: [PATCH] env: sf: add support for env erase
Hi Sean,
thanks for your try and sorry for the inconvenience my beginner's mistakes have
caused :-(
It is definitely no good idea to use copy&paste with patch data, I should have
guessed that beforehand ...
Anyway, concerning the complaints from checkpatch.pl: I indeed ran checkpatch.pl
on my patch prior to sending it - the real one, not the one as pasted into the
mail ;-/
The alignment warnings simply result from the fact that I adhered to the style
used in that file already, you can easily verify that by running checkpatch.pl
on the complete file.
For the warnings with "IS_ENABLED(CONFIG...)" I don't see what I could do to
get around them: all three occurrences are about compiling functions into the
code depending on CONFIG_CMD_ERASEENV.
Two times it is the new function env_sf_erase(), one variant for normal and the
other for redundand environment handling.
The third time it is used to define this new method into the structure
U_BOOT_ENV_LOCATION(sf).
The sign-off problem I guess is probably caused by the check not accepting name
in reverse order, isn't it?
Anyway, I will change my user.name to match the order in the mail address so
next patch is hopefully correct.
So please let me know what else I should do beside sending a properly formatted
patch ;-/
I will take care of that before resending my patch (v2 then, right?).
On 10/9/20 3:55 PM, Sean Anderson wrote:
On 10/8/20 1:27 PM, Harry Waschkeit wrote:
Command "env erase" didn't work even though CONFIG_CMD_ERASEENV was
defined, because serial flash environment routines didn't implement
erase method.
Signed-off-by: Waschkeit, Harry
---
Hi Harry,
I wanted to test out your patch, but I couldn't get it to apply. It
appears that your mail program has replaced the tabs with spaces, so git
can't figure out how to apply it. I tried to fix it by performing the
substitutions
s/^\(.\) /\1\t/g
s//\t/g
but it still wouldn't apply. In addition, checkpatch has a few warnings:
$ scripts/checkpatch.pl env-sf-add-support-for-env-erase.patch
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
possible
#152: FILE: env/sf.c:149:
+#ifdef CONFIG_CMD_ERASEENV
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
possible
#240: FILE: env/sf.c:318:
+#ifdef CONFIG_CMD_ERASEENV
CHECK: Alignment should match open parenthesis
#260: FILE: env/sf.c:338:
+ ret = spi_flash_read(env_flash, saved_offset,
+ saved_size, saved_buffer);
CHECK: Alignment should match open parenthesis
#269: FILE: env/sf.c:347:
+ ret = spi_flash_erase(env_flash, CONFIG_ENV_OFFSET,
+ sector * CONFIG_ENV_SECT_SIZE);
CHECK: Alignment should match open parenthesis
#276: FILE: env/sf.c:354:
+ ret = spi_flash_write(env_flash, saved_offset,
+ saved_size, saved_buffer);
WARNING: Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
possible
#307: FILE: env/sf.c:437:
+#if defined(CONFIG_CMD_ERASEENV) && defined(CONFIG_ENV_ADDR)
WARNING: Missing Signed-off-by: line by nominal patch author 'Harry Waschkeit
'
total: 0 errors, 4 warnings, 3 checks, 158 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
env-sf-add-support-for-env-erase.patch has style problems, please review.
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
MULTISTATEMENT_MACRO_USE_DO_WHILE NETWORKING_BLOCK_COMMENT_STYLE
PREFER_ETHER_ADDR_COPY USLEEP_RANGE
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
Please fix these issues and resend, thanks!
--Sean
env/sf.c | 130 ++-
1 file changed, 128 insertions(+), 2 deletions(-)
diff --git a/env/sf.c b/env/sf.c
index 937778aa37..9cda192a73 100644
--- a/env/sf.c
+++ b/env/sf.c
@@ -146,6 +146,78 @@ static int env_sf_save(void)
return ret;
}
+#ifdef CONFIG_CMD_ERASEENV
+static int env_sf_erase(void)
+{
+ char*saved_buffer = NULL;
+ u32 saved_size, saved_offset, sector;
+ ulong offset;
+ ulong offsets[2] = { CONFIG_ENV_OFFSET, CONFIG_ENV_OFFSET_REDUND };
+ int i;
+ int ret;
+
+ ret = setup_flash_device();
+ if (ret)
+ return ret;
+
+ /* get temporary storage if sector is larger than env (i.e. embedded) */
+ if (CONFIG_ENV_SECT_SIZE > CONFIG_ENV_SIZE) {
+ saved_size = CONFIG_ENV_SECT_SIZE - CONFIG_ENV_SIZE;
+ saved_buffer = memalign(ARCH_DMA_MINALIGN, saved_size);
+ if (!saved_buffer) {
+ ret = -ENOMEM;
+ goto done
[PATCH] env: sf: add support for env erase
DUND) #else */
#if CONFIG_ENV_ADDR != 0x0
__weak void *env_sf_get_env_addr(void)
@@ -311,4 +434,7 @@ U_BOOT_ENV_LOCATION(sf) = {
#if defined(INITENV) && (CONFIG_ENV_ADDR != 0x0)
.init = env_sf_init,
#endif
+#if defined(CONFIG_CMD_ERASEENV) && defined(CONFIG_ENV_ADDR)
+ .erase = env_sf_erase,
+#endif
};
--
2.28.0
--
Harry Waschkeit - Software Engineer
