[PATCH] board: imx8mn_s2: Update timing with production one

2024-06-10 Thread Michael Trimarchi
The timing upstream was wrong corresponding to the production.
This come evident after commit b614ddb5d33
(ddr: imx: Save the FW loading if it hasn't changed). This
change fix booting from usb

Signed-off-by: Michael Trimarchi 
---
 board/bsh/imx8mn_smm_s2/ddr3l_timing_256m.c | 23 -
 board/bsh/imx8mn_smm_s2/ddr3l_timing_512m.c | 14 +++--
 2 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/board/bsh/imx8mn_smm_s2/ddr3l_timing_256m.c 
b/board/bsh/imx8mn_smm_s2/ddr3l_timing_256m.c
index 0da641834d..33452d2ad5 100644
--- a/board/bsh/imx8mn_smm_s2/ddr3l_timing_256m.c
+++ b/board/bsh/imx8mn_smm_s2/ddr3l_timing_256m.c
@@ -18,15 +18,15 @@ struct dram_cfg_param ddr_ddrc_cfg[] = {
{ 0x3d400304, 0x1 },
{ 0x3d400030, 0x20 },
{ 0x3d40, 0xa1040001 },
-   { 0x3d400064, 0x610040 },
+   { 0x3d400064, 0x300040 },
{ 0x3d4000d0, 0xc00200c5 },
{ 0x3d4000d4, 0x1000b },
{ 0x3d4000dc, 0x1d74 },
-   { 0x3d4000e0, 0x18 },
+   { 0x3d4000e0, 0x58 },
{ 0x3d4000e4, 0x9 },
-   { 0x3d4000f0, 0x0 },
+   { 0x3d4000f0, 0x2 },
{ 0x3d4000f4, 0xee5 },
-   { 0x3d400100, 0xc101b0e },
+   { 0x3d400100, 0xc100d0e },
{ 0x3d400104, 0x30314 },
{ 0x3d400108, 0x4060509 },
{ 0x3d40010c, 0x2006 },
@@ -67,10 +67,10 @@ struct dram_cfg_param ddr_ddrc_cfg[] = {
{ 0x3d400498, 0x7ff },
{ 0x3d40049c, 0xe00 },
{ 0x3d4004a0, 0x7ff },
-   { 0x3d402064, 0x28001b },
+   { 0x3d402064, 0x14001b },
{ 0x3d4020dc, 0x1224 },
-   { 0x3d4020e0, 0x0 },
-   { 0x3d402100, 0x7090b07 },
+   { 0x3d4020e0, 0x40 },
+   { 0x3d402100, 0x7090507 },
{ 0x3d402104, 0x20209 },
{ 0x3d402108, 0x3030407 },
{ 0x3d40210c, 0x2006 },
@@ -680,12 +680,13 @@ struct dram_cfg_param ddr_fsp0_cfg[] = {
{ 0x54006, 0x140 },
{ 0x54007, 0x1000 },
{ 0x54008, 0x101 },
+   { 0x54009, 0x200 },
{ 0x5400b, 0x31f },
{ 0x5400c, 0xc8 },
{ 0x54012, 0x1 },
{ 0x5402f, 0x1d70 },
{ 0x54030, 0x4 },
-   { 0x54031, 0x18 },
+   { 0x54031, 0x58 },
{ 0x5403a, 0x1323 },
{ 0xd, 0x1 },
 };
@@ -700,11 +701,13 @@ struct dram_cfg_param ddr_fsp1_cfg[] = {
{ 0x54006, 0x140 },
{ 0x54007, 0x1000 },
{ 0x54008, 0x101 },
+   { 0x54009, 0x200 },
{ 0x5400b, 0x21f },
{ 0x5400c, 0xc8 },
{ 0x54012, 0x1 },
{ 0x5402f, 0x1220 },
{ 0x54030, 0x4 },
+   { 0x54031, 0x40 },
{ 0x5403a, 0x1323 },
{ 0xd, 0x1 },
 };
@@ -886,11 +889,11 @@ struct dram_cfg_param ddr_phy_pie[] = {
{ 0xd00e7, 0x400 },
{ 0x90017, 0x0 },
{ 0x90026, 0x2b },
-   { 0x2000b, 0x32 },
+   { 0x2000b, 0x1c2 },
{ 0x2000c, 0x64 },
{ 0x2000d, 0x3e8 },
{ 0x2000e, 0x2c },
-   { 0x12000b, 0x14 },
+   { 0x12000b, 0xbb },
{ 0x12000c, 0x26 },
{ 0x12000d, 0x1a1 },
{ 0x12000e, 0x10 },
diff --git a/board/bsh/imx8mn_smm_s2/ddr3l_timing_512m.c 
b/board/bsh/imx8mn_smm_s2/ddr3l_timing_512m.c
index f845395ad9..ca14a47442 100644
--- a/board/bsh/imx8mn_smm_s2/ddr3l_timing_512m.c
+++ b/board/bsh/imx8mn_smm_s2/ddr3l_timing_512m.c
@@ -18,15 +18,15 @@ struct dram_cfg_param ddr_ddrc_cfg[] = {
{ 0x3d400304, 0x1 },
{ 0x3d400030, 0x20 },
{ 0x3d40, 0xa1040001 },
-   { 0x3d400064, 0x610068 },
+   { 0x3d400064, 0x300068 },
{ 0x3d4000d0, 0xc00200c5 },
{ 0x3d4000d4, 0x1000b },
{ 0x3d4000dc, 0x1d74 },
-   { 0x3d4000e0, 0x18 },
+   { 0x3d4000e0, 0x58 },
{ 0x3d4000e4, 0x9 },
-   { 0x3d4000f0, 0x0 },
+   { 0x3d4000f0, 0x2 },
{ 0x3d4000f4, 0xee5 },
-   { 0x3d400100, 0xc101b0e },
+   { 0x3d400100, 0xc100d0e },
{ 0x3d400104, 0x30314 },
{ 0x3d400108, 0x4060509 },
{ 0x3d40010c, 0x2006 },
@@ -700,11 +700,13 @@ struct dram_cfg_param ddr_fsp1_cfg[] = {
{ 0x54006, 0x140 },
{ 0x54007, 0x1000 },
{ 0x54008, 0x101 },
+   { 0x54009, 0x200 },
{ 0x5400b, 0x21f },
{ 0x5400c, 0xc8 },
{ 0x54012, 0x1 },
{ 0x5402f, 0x1220 },
{ 0x54030, 0x4 },
+   { 0x54031, 0x40 },
{ 0x5403a, 0x1323 },
{ 0xd, 0x1 },
 };
@@ -886,11 +888,11 @@ struct dram_cfg_param ddr_phy_pie[] = {
{ 0xd00e7, 0x400 },
{ 0x90017, 0x0 },
{ 0x90026, 0x2b },
-   { 0x2000b, 0x32 },
+   { 0x2000b, 0x1c2 },
{ 0x2000c, 0x64 },
{ 0x2000d, 0x3e8 },
{ 0x2000e, 0x2c },
-   { 0x12000b, 0x14 },
+   { 0x12000b, 0xbb },
{ 0x12000c, 0x26 },
{ 0x12000d, 0x1a1 },
{ 0x12000e, 0x10 },
-- 
2.43.0



Re: obscure microsd detection issue between U-Boot and kernel

2024-06-04 Thread Michael Walle
On Tue Jun 4, 2024 at 9:47 AM CEST, Christian Loehle wrote:
> On 6/3/24 22:28, Tim Harvey wrote:
> > On Mon, Jun 3, 2024 at 1:18 AM Christian Loehle
> >  wrote:
> >>
> >> On 5/31/24 21:47, Tim Harvey wrote:
> >>> Greetings,
> >>>
> >>> I'm seeing an issue on an imx8mm board (imx8mm-venice-gw73xx) where
> >>> for a specific set of microsd cards if I have accessed the microsd in
> >>> U-Boot with UHS/1.8V the kernel will not recognize that microsd when
> >>> scanning.
> >>>
> >>> The issue does not occur with all microsd cards but seems to appear
> >>> with a large sample size of a specific card/model (Kingston SDC32 32GB
> >>> SDR104 card). I do not see a signal integrity issue on the scope.
> >>>
> >>> Instrumenting the kernel the issue is that the host reports a CRC
> >>> error as soon as the first mmc_send_if_cond call which occurs in
> >>> mmc_rescan_try_freq.
> >>>
> >>> I can avoid the issue by either not accessing the microsd in U-Boot or
> >>> by disabling UHS/1.8V mode in U-Boot therefore what I think is
> >>> happening is that U-Boot leaves the card in UHS/1.8V signalling mode
> >>> and when the kernel scans it sets the voltage back to 3.3V
> >>> standard/default and default timings then issues its clock cycles to
> >>> 'reset' the card and the card does not recognize the reset. I'm
> >>> wondering if this is because the reset is done via clock cycles after
> >>> the kernel has set the I/O voltage back to 3.3V when perhaps the card
> >>> is still in 1.8V mode (although I don't see how that would cause an
> >>> issue)?
> >>
> >> It will cause an issue for many cards and might break some cards.
> >>
> >>>
> >>> Is there some sort of MMC 'reset' I can/should do in U-Boot before
> >>> booting the kernel? Has anyone encountered anything like this before?
> >>
> >> There is no 'switching back' to 3.3V signalling from UHS 1.8V.
> >> The only way this can be done is therefore a full power-off.
> >> Is that done correctly for your system?
> >> AFAIR spec dictates 500ms of <0.5V on VCC. Note that  driving CLK/signal
> >> lines can also sustain the card somewhat, as leakage is only limited
> >> within operating voltage.
> > 
> > Hi Christian,
> > 
> > Are you saying the only way to properly reset from 1.8V is to have a
> > VDD supply on the microSD card that can be turned off before booting
> > to Linux? We have never had that before and never encountered
> > something like this.
>
> Yes, the only safe way to use UHS-I really anyway.

I can second that. And also keep in mind that the actual supply
voltage must be below that threshold. Thus, the power-off time also
depends on the capacitance on that supply line after the power load
switch. There are switches with dedicated output discharge circuits
built-in.

That being said, from my experience there are cards which will work
when switching back from 1V8 to 3V3 signalling and some don't. I
haven't digged deeper into that topic, though.

-michael

> You could disable UHS for u-boot but that still leaves (potentially)
> problematic warm-reboots of the board.
> Having a (software-controlled) switchable regulator on SD VCC is pretty
> common for this reason and you should be able to find it in most dts
> for host controllers with sd-uhs-* property.
> I'm afraid that the relevant spec section isn't available in the
> simplified version.
>
> Kind Regards,
> Christian



signature.asc
Description: PGP signature


Re: [PATCH] Makefile: Fix include directory for OF_UPSTREAM

2024-05-31 Thread Michael Nazzareno Trimarchi
Hi Patrick

On Thu, May 30, 2024 at 11:13 AM Patrick Barsanti
 wrote:
>
> Hi Sumit,
>
> On Wed, 29 May 2024 at 14:00, Sumit Garg  wrote:
>>
>> On Wed, 29 May 2024 at 14:45, Patrick Barsanti
>>  wrote:
>> >
>> > Hi Sumit,
>> >
>> > On Wed, 29 May 2024 at 08:57, Sumit Garg  wrote:
>> >>
>> >> Hi Patrick,
>> >>
>> >> On Tue, 28 May 2024 at 14:16, Patrick Barsanti
>> >>  wrote:
>> >> >
>> >> > Always prioritizing u-boot includes causes problems when trying to
>> >> > migrate boards to OF_UPSTREAM that have divergent devicetree files with
>> >> > respect to the upstream ones.
>> >> >
>> >> > For example, migrating a board based on `imx6ul.dtsi` to OF_UPSTREAM
>> >> > breaks it, as there are some missing defines in the local dtsi file;
>> >> > the solutions would be to either patch it, which defeats the purpose of
>> >> > OF_UPSTREAM, or delete it entirely. This last option would then break 
>> >> > all
>> >> > the other boards which have not yet been migrated to OF_UPSTREAM.
>> >>
>> >> Can you elaborate more here regarding which dt-bindings headers
>> >> conflict? Also, is it only the DTS files consumer for those headers or
>> >> there are U-Boot drivers depending on them too?
>> >>
>> >> -Sumit
>> >
>> >
>> > Sorry, I think I have worded my commit message wrong. I should
>> > have used differ instead of diverge, which is slightly misleading.
>> >
>> > The specific case I am talking about can be found in:
>> > include/dt-bindings/clock/imx6ul-clock.h
>> > dts/upstream/include/dt-bindings/clock/imx6ul-clock.h
>> >
>> > The local header is missing the last commit from the kernel, which is
>> > 4e197ee880c2 ("clk: imx6ul: add ethernet refclock mux support").
>> > This added some new defines, which are not present in the u-boot
>> > header.
>> > Following this commit, the `imx6ul.dtsi` was patched in the kernel to
>> > use one of the new defines.
>> >
>> > Because of this, at the current state, migrating a board which is
>> > somehow based on `imx6ul.dtsi` will give a dtc error given by a value
>> > being used in the upstream dtsi which is not defined in the local
>> > header, because local includes always have priority with respect
>> > to upstream ones even when setting OF_UPSTREAM.
>>
>> So you should just drop the local DT bindings header:
>> include/dt-bindings/clock/imx6ul-clock.h and that should resolve the
>> problem for you, right?
>
>
> Yes, that indeed solves my problem.
> But if I drop it, I will force all other boards which are not yet
> migrated to OF_UPSTREAM and include `imx6ul.dtsi` to
> use the upstream header instead of the local one.
> I did not feel comfortable in doing so, because if any change is made
> to the upstream header in the future which is somehow not backwards
> compatible, then all boards which are still not using OF_UPSTREAM
> would not compile.
>
> I also thought this would not be limited to the single header file which
> caused my problem. I imagine there would be other cases of local
> headers which are missing one or more commits from mainline kernel
> and cause the same type of problem when moving to OF_UPSTREAM.
>
> The opposite problem also exists.
> For example:
> 675575880f ("phycore-am64x: Migrate to OF_UPSTREAM")
> does not drop include/dt-bindings/net/ti-dp83867.h, and I think the
> migration worked properly only because at the moment there is no
> difference between local and upstream headers.
> If and when the upstream headers and devicetrees will be patched,
> this will cause problems, because the local include will be used
> even if it's out-of-date.
> I have tested this: by simulating a kernel patch to the upstream files,
> which adds an extra define in ti-dp83867.h and updates
> k3-am64-phycore-som.dtsi to use this new define, current state
> u-boot will fail to build because that define is not present in the local
> include header.
> By including my patch, the build is successful.
>
> This is the reason why I proposed this Makefile patch, but of course I
> am completely open to suggestions and ideas better than mine, which
> I suspect are fairly easy to come by :)
>

Based on the discussion so far, you can extend the commit message so
other people can comment on it

Michael

> Thank you,
> Regards,
> Patrick
>
>>
>>
>> -Sumit
>>
>

Re: [PATCH v2 0/6] Introduce UBI block device

2024-05-23 Thread Michael Nazzareno Trimarchi
Hi

We queue them this afternoon

Michael

On Thu, May 23, 2024 at 12:49 PM Alexey Romanov
 wrote:
>
>
> Hi Michael,
>
> On Mon, May 06, 2024 at 03:59:52PM +0200, Michael Nazzareno Trimarchi wrote:
> > Hi Alexey
> >
> > Sorry will will put on CI
>
> Any news?
>
> >
> > Michael
> >
> > On Mon, May 6, 2024 at 3:58 PM Alexey Romanov
> >  wrote:
> > >
> > > Hello! Ping.
> > >
> > > On Mon, Mar 25, 2024 at 05:41:42PM +0300, Alexey Romanov wrote:
> > > > Hello!
> > > >
> > > > This series adds support for the UBI block device, which
> > > > allows to read/write data block by block. For example,
> > > > it can now be used for BCB or Android AB command:
> > > >
> > > >   $ bcb load ubi 0 part_name
> > > >
> > > > Tested only on SPI NAND, so bind is made only for
> > > > SPI NAND drivers. Can be used with mtdblock device [1].
> > > >
> > > > ---
> > > >
> > > > Changes V1 -> V2 [2]:
> > > >
> > > >  - Rebased over mtdblock v2 patchset [3].
> > > >  - Compile UBI partitions support only if CONFIG_BLK option
> > > >is enabled.
> > > >
> > > > - Links:
> > > >
> > > >   [1] 
> > > > https://lore.kernel.org/all/20240227100441.1811047-1-avroma...@salutedevices.com/
> > > >   [2] 
> > > > https://lore.kernel.org/all/20240306134906.1179285-1-avroma...@salutedevices.com/
> > > >   [3] 
> > > > https://lore.kernel.org/all/20240307130726.1582487-1-avroma...@salutedevices.com/
> > > >
> > > > Alexey Romanov (6):
> > > >   ubi: allow to read from volume with offset
> > > >   ubi: allow to write to volume with offset
> > > >   drivers: introduce UBI block abstraction
> > > >   disk: don't try search for partition type if already set
> > > >   disk: support UBI partitions
> > > >   spinand: bind UBI block
> > > >
> > > >  cmd/ubi.c   |  77 +++--
> > > >  disk/part.c |   7 ++
> > > >  drivers/block/blk-uclass.c  |   1 +
> > > >  drivers/mtd/nand/spi/core.c |   8 ++-
> > > >  drivers/mtd/ubi/Makefile|   1 +
> > > >  drivers/mtd/ubi/block.c | 130 
> > > >  drivers/mtd/ubi/part.c  |  99 +++
> > > >  env/ubi.c   |  16 ++---
> > > >  include/part.h  |   2 +
> > > >  include/ubi_uboot.h |   8 ++-
> > > >  10 files changed, 332 insertions(+), 17 deletions(-)
> > > >  create mode 100644 drivers/mtd/ubi/block.c
> > > >  create mode 100644 drivers/mtd/ubi/part.c
> > > >
> > > > --
> > > > 2.34.1
> > > >
> > >
> > > --
> > > Thank you,
> > > Alexey
> >
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > mich...@amarulasolutions.com
> > __
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > i...@amarulasolutions.com
> > www.amarulasolutions.com
>
> --
> Thank you,
> Alexey



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [EXTERNAL] Re: [PATCH] mtd: nand: pxa3xx: Incorrect bitflip return on page read

2024-05-21 Thread Michael Nazzareno Trimarchi
Hi Dario

Can you add to next pull?

Michael

On Tue, May 21, 2024, 4:31 PM Ravi Minnikanti 
wrote:

> Hi,
>
> Can you please merge this PR, if there are no more review comments?
>
> Thanks,
> Ravi
> On 5/6/24 11:28, Michael Nazzareno Trimarchi wrote:
>
> > --
> > Hi Ravi
> >
> > On Mon, May 6, 2024 at 7:33 PM Ravi Minnikanti 
> wrote:
> >>
> >> On 5/6/24 00:35, Michael Nazzareno Trimarchi wrote:
> >>> Hi Ravi
> >>>
> >>> On Tue, Apr 30, 2024 at 6:25 AM Ravi Minnikanti <
> rminnika...@marvell.com> wrote:
> >>>>
> >>>> On 4/29/24 09:59, Michael Nazzareno Trimarchi wrote:
> >>>>
> >>>>>
> --
> >>>>> On Mon, Apr 29, 2024 at 6:22 PM Chris Packham <
> judge.pack...@gmail.com> wrote:
> >>>>>>
> >>>>>> On Sun, Apr 28, 2024 at 4:15 AM Ravi Minnikanti <
> rminnika...@marvell.com> wrote:
> >>>>>>>
> >>>>>>> Once a page is read with higher bitflips all subsequent reads
> >>>>>>> are returning the same bitflip value even though they have none.
> >>>>>>> max_bitflip variable is not being reset to 0 across page reads.
> >>>>>>>
> >>>>>>> This is causing problems like incorrectly
> >>>>>>> marking erase blocks bad by UBI and causing read failures.
> >>>>>>>
> >>>>>>> Verified the change with both MTD reads and UBI.
> >>>>>>> This change is inline with other NFC drivers.
> >>>>>>>
> >>>>>>> Sample error log where a block is marked bad incorrectly:
> >>>>>>>
> >>>>>>> ubi0: fixable bit-flip detected at PEB 125
> >>>>>>> ubi0: run torture test for PEB 125
> >>>>>>> ubi0: fixable bit-flip detected at PEB 125
> >>>>>>> ubi0 error: torture_peb: read problems on freshly erased PEB 125,
> >>>>>>> must be bad
> >>>>>>> ubi0 error: erase_worker: failed to erase PEB 125, error -5
> >>>>>>> ubi0: mark PEB 125 as bad
> >>>>>>>
> >>>>>>> Signed-off-by: rminnikanti 
> >>>>>>
> >>>>>> Looks good to me
> >>>>>>
> >>>>>> Reviewed-by: Chris Packham 
> >>>>>>
> >>>>>>> ---
> >>>>>>>  drivers/mtd/nand/raw/pxa3xx_nand.c | 5 +
> >>>>>>>  1 file changed, 5 insertions(+)
> >>>>>>>
> >>>>>>> diff --git a/drivers/mtd/nand/raw/pxa3xx_nand.c
> b/drivers/mtd/nand/raw/pxa3xx_nand.c
> >>>>>>> index 1d9a6d107b..d2a4faad56 100644
> >>>>>>> --- a/drivers/mtd/nand/raw/pxa3xx_nand.c
> >>>>>>> +++ b/drivers/mtd/nand/raw/pxa3xx_nand.c
> >>>>>>> @@ -800,6 +800,11 @@ static void prepare_start_command(struct
> pxa3xx_nand_info *info, int command)
> >>>>>>> info->ecc_err_cnt   = 0;
> >>>>>>> info->ndcb3 = 0;
> >>>>>>> info->need_wait = 0;
> >>>>>>> +   /*
> >>>>>>> +* Reset max_bitflips to zero. Once command is complete,
> >>>>>>> +* max_bitflips for this READ is returned in
> ecc.read_page()
> >>>>>>> +*/
> >>>>>>> +   info->max_bitflips  = 0;
> >>>>>>>
> >>>>>
> >>>>> Why this should not be put to 0 in read_page instead on
> prepare_start_command?
> >>>>>
> >>>>> Michael
> >>>>>
> >>>>
> >>>> ecc.read_page is invoked after the read command execution.
> >>>> First chip->cmdfunc is executed with NAND_CMD_READ0 and then
> ecc.read_page is invoked
> >>>> to read the page from buffer. So, by the time read_page is invoked,
> info->max_bitflips
> >>>> must already have the bit flip value.
> >>>>
> >>>
> >>> All the other implementation has a slight different way to handle.
&

AW: [PATCH] env: Invert gd->env_valid for env_mmc_save

2024-05-21 Thread Michael Glembotzki
Hi Tom,

> ​Can you please explain a little more on how you get to this problem? The
> same code / test exists in env/fat.c, env/sf.c and env/ubi.c. In the
> case of env/mmc.c it's been that way since introduction in:
> commit d196bd880347373237d73e0d115b4d51c68cf2ad

Sure! We have limited the writable u-boot envs to a few (ENV_WRITELIST) and
otherwise only use the default set. That means our env_get_location looks
like this:

enum env_location env_get_location(enum env_operation op, int prio)
{
if (op == ENVOP_SAVE) {
if (prio == 0)
return ENVL_MMC;
} else {
if (prio == 0)
return ENVL_NOWHERE;
if (prio == 1)
return ENVL_MMC;
}
return ENVL_UNKNOWN;
}

The env location when saving:
0. EMMC

The env location when loading:
0. NOWHERE (default envs) and is appended by the filtered envs from the
1. EMMC

We also have CONFIG_ENV_OFFSET_REDUND and CONFIG_SYS_REDUNDAND_ENVIRONMENT
active.

When booting for the first time, the following happens to the u-boot env's
during load():
1. ​env/nowhere.c: env_nowhere_load sets gd->env_valid = ENV_INVALID
2. env/mmc.c: env_mmc_load dont touch gd->env_valid and stays ENV_INVALID
  env_mmc_load -> env_import_redund -> env_check_redund -> return -ENOMSG

gd->env_valid remains unchanged at ENV_INVALID because there are no valid
env's in both MMC areas (bad CRC) and this creates a problem with
env_mmc_save.

env_mmc_save saves to slot A in the ENV_INVALID and ENV_REDUND cases.

if (gd->env_valid == ENV_VALID)
copy = 1;
mmc_get_env_addr(mmc, copy, );
printf("Writing to %sMMC(%d)... ", copy ? "redundant " : "", dev);
write_env(mmc, CONFIG_ENV_SIZE, offset, (u_char *)env_new);


Does it perhaps make sense to set gd->env_valid == ENV_VALID in
env_nowhere_load? Otherwise, I have 2 suggestions. Either we use the patch
provided, or I think my preferred solution would be to change:

if (gd->env_valid == ENV_VALID)
copy = 1;

in

if (gd->env_valid != ENV_REDUND)
copy = 1;

For the other storage locations env/fat.c, env/sf.c, env/ubi.c and
probably others, you would have to straighten this out accordingly. But I
could currently only test it for MMC. What do you think?

Another note: Once a valid save has been made, the problem no longer
occurs. And the problem doesn't occur with the default env_get_location
because gd->env_valid is set to ENV_VALID in env_init (env/enc.c).

Best regards
Michael


[PATCH] sunxi: SPL SPI: add support for the V3s SoC

2024-05-13 Thread Michael Walle
The V3s is identical regarding register layout, clocks and resets to
the sun6i variants. Therefore, we can just add the MACH_SUN8I_V3S to
the sun6i compatible ones.

SPI boot was tested on a custom board with a Gigadevice GD25Q64 8MiB
SPI flash.

Signed-off-by: Michael Walle 
---
 arch/arm/mach-sunxi/Kconfig | 2 +-
 arch/arm/mach-sunxi/spl_spi_sunxi.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index ddf9414b08e..17666814c52 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -1078,7 +1078,7 @@ config SPL_STACK_R_ADDR
 
 config SPL_SPI_SUNXI
bool "Support for SPI Flash on Allwinner SoCs in SPL"
-   depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUNXI_H3_H5 
|| MACH_SUN50I || MACH_SUN8I_R40 || SUN50I_GEN_H6 || MACH_SUNIV || 
SUNXI_GEN_NCAT2
+   depends on MACH_SUN4I || MACH_SUN5I || MACH_SUN7I || MACH_SUNXI_H3_H5 
|| MACH_SUN50I || MACH_SUN8I_R40 || MACH_SUN8I_V3S || SUN50I_GEN_H6 || 
MACH_SUNIV || SUNXI_GEN_NCAT2
help
  Enable support for SPI Flash. This option allows SPL to read from
  sunxi SPI Flash. It uses the same method as the boot ROM, so does
diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c 
b/arch/arm/mach-sunxi/spl_spi_sunxi.c
index 7acb44f52ae..d7abdc2e401 100644
--- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
+++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
@@ -140,7 +140,8 @@ static bool is_sun6i_gen_spi(void)
 {
return IS_ENABLED(CONFIG_SUNXI_GEN_SUN6I) ||
   IS_ENABLED(CONFIG_SUN50I_GEN_H6) ||
-  IS_ENABLED(CONFIG_SUNXI_GEN_NCAT2);
+  IS_ENABLED(CONFIG_SUNXI_GEN_NCAT2) ||
+  IS_ENABLED(CONFIG_MACH_SUN8I_V3S);
 }
 
 static uintptr_t spi0_base_address(void)
-- 
2.39.2



[PATCH 2/2] net: sun8i_emac: add support for the V3s

2024-05-13 Thread Michael Walle
Add the compatible string for the emac found on the V3s SoC. The SoC
only supports the internal PHY. There are no (R)MII signals on any pins.

Signed-off-by: Michael Walle 
---
 drivers/net/sun8i_emac.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 8bff4fe9a9e..94bcd40acb8 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -893,6 +893,11 @@ static const struct emac_variant emac_variant_r40 = {
.syscon_offset  = 0x164,
 };
 
+static const struct emac_variant emac_variant_v3s = {
+   .syscon_offset  = 0x30,
+   .soc_has_internal_phy   = true,
+};
+
 static const struct emac_variant emac_variant_a64 = {
.syscon_offset  = 0x30,
.support_rmii   = true,
@@ -910,6 +915,8 @@ static const struct udevice_id sun8i_emac_eth_ids[] = {
  .data = (ulong)_variant_h3 },
{ .compatible = "allwinner,sun8i-r40-gmac",
  .data = (ulong)_variant_r40 },
+   { .compatible = "allwinner,sun8i-v3s-emac",
+ .data = (ulong)_variant_v3s },
{ .compatible = "allwinner,sun50i-a64-emac",
  .data = (ulong)_variant_a64 },
{ .compatible = "allwinner,sun50i-h6-emac",
-- 
2.39.2



[PATCH 1/2] clk: sunxi: add EMAC and EPHY clocks and resets for the V3s SoC

2024-05-13 Thread Michael Walle
Add the clock gate registers as well as the reset register bits for the
EMAC and EPHY for the V3s. These are needed by the sun8i network driver.

Signed-off-by: Michael Walle 
---
 drivers/clk/sunxi/clk_v3s.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clk/sunxi/clk_v3s.c b/drivers/clk/sunxi/clk_v3s.c
index 6524c13540e..0402d5ed190 100644
--- a/drivers/clk/sunxi/clk_v3s.c
+++ b/drivers/clk/sunxi/clk_v3s.c
@@ -17,6 +17,7 @@ static struct ccu_clk_gate v3s_gates[] = {
[CLK_BUS_MMC0]  = GATE(0x060, BIT(8)),
[CLK_BUS_MMC1]  = GATE(0x060, BIT(9)),
[CLK_BUS_MMC2]  = GATE(0x060, BIT(10)),
+   [CLK_BUS_EMAC]  = GATE(0x060, BIT(17)),
[CLK_BUS_SPI0]  = GATE(0x060, BIT(20)),
[CLK_BUS_OTG]   = GATE(0x060, BIT(24)),
 
@@ -31,6 +32,8 @@ static struct ccu_clk_gate v3s_gates[] = {
[CLK_BUS_UART1] = GATE(0x06c, BIT(17)),
[CLK_BUS_UART2] = GATE(0x06c, BIT(18)),
 
+   [CLK_BUS_EPHY]  = GATE(0x070, BIT(0)),
+
[CLK_SPI0]  = GATE(0x0a0, BIT(31)),
 
[CLK_USB_PHY0]  = GATE(0x0cc, BIT(8)),
@@ -45,12 +48,15 @@ static struct ccu_reset v3s_resets[] = {
[RST_BUS_MMC0]  = RESET(0x2c0, BIT(8)),
[RST_BUS_MMC1]  = RESET(0x2c0, BIT(9)),
[RST_BUS_MMC2]  = RESET(0x2c0, BIT(10)),
+   [RST_BUS_EMAC]  = RESET(0x2c0, BIT(17)),
[RST_BUS_SPI0]  = RESET(0x2c0, BIT(20)),
[RST_BUS_OTG]   = RESET(0x2c0, BIT(24)),
 
[RST_BUS_TCON0] = RESET(0x2c4, BIT(4)),
[RST_BUS_DE]= RESET(0x2c4, BIT(12)),
 
+   [RST_BUS_EPHY]  = RESET(0x2c8, BIT(2)),
+
[RST_BUS_I2C0]  = RESET(0x2d8, BIT(0)),
[RST_BUS_I2C1]  = RESET(0x2d8, BIT(1)),
[RST_BUS_UART0] = RESET(0x2d8, BIT(16)),
-- 
2.39.2



[PATCH 0/2] sunxi: v3s: add network support

2024-05-13 Thread Michael Walle
Add network support for the V3s which only supports the internal
PHY. Adding support was straight forward. The emac driver just needs
the compatible string and some platform data and the clock driver
needs to know the bits for the clock gating as well as the reset
bits.

This was tested on a custom board.

Michael Walle (2):
  clk: sunxi: add EMAC and EPHY clocks and resets for the V3s SoC
  net: sun8i_emac: add support for the V3s

 drivers/clk/sunxi/clk_v3s.c | 6 ++
 drivers/net/sun8i_emac.c| 7 +++
 2 files changed, 13 insertions(+)

-- 
2.39.2



Re: imx8mp: Flashing U-Boot into eMMC hardware partition via UUU

2024-05-10 Thread Michael Nazzareno Trimarchi
Hi Fabio

On Fri, May 10, 2024 at 5:10 PM Fabio Estevam  wrote:
>
> Hi Michael,
>
> On Fri, May 10, 2024 at 11:28 AM Michael Nazzareno Trimarchi
>  wrote:
>
> > You can just change as you want. We have this file in buildroot, uuu
> > can run command on the device
> > using FB command. Example how call it
>
> Thanks for sharing the example.
>
> I adapted the UUU script like this:
>
> SDPS: boot -f flash.bin
> FB: ucmd setenv fastboot_buffer ${loadaddr}
> FB: ucmd mmc dev 2 1
> FB: download -f flash.bin
> FB: ucmd setexpr blkcnt $filesize + 0x1ff
> FB: ucmd setexpr blkcnt $blkcnt / 0x200
> FB: ucmd mmc write $loadaddr 0 $blkcnt

My suggestion is use timeout of some command when is possible

> FB: reboot
> FB: done
>
> Did the following changes based on imx8mn_bsh_smm_s2pro:
>
> index 024b46ef8bc2..0b6026c34309 100644
> --- a/board/freescale/imx8mp_evk/imx8mp_evk.c
> +++ b/board/freescale/imx8mp_evk/imx8mp_evk.c
> @@ -3,6 +3,8 @@
>   * Copyright 2019 NXP
>   */
>
> +#include 
> +#include 
>  #include 
>
>  int board_init(void)
> @@ -17,5 +19,11 @@ int board_late_init(void)
> env_set("board_rev", "iMX8MP");
>  #endif
>
> +   if (is_usb_boot()) {
> +   printf("* Entering in USB download mode\n");
> +   env_set("bootcmd", "fastboot usb 0");
> +   env_set("bootdelay", "0");
> +   }
> +

I think that is kind of good example

> return 0;
>  }
> diff --git a/include/configs/imx8mp_evk.h b/include/configs/imx8mp_evk.h
> index 1759318fdd35..148b36bd3169 100644
> --- a/include/configs/imx8mp_evk.h
> +++ b/include/configs/imx8mp_evk.h
> @@ -25,8 +25,17 @@
>
>  #include 
>
> +#define EMMCARGS \
> +   "fastboot_partition_alias_all=" \
> +   __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) ".0:0\0" \
> +   "fastboot_partition_alias_bootloader=" \
> +   __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) ".1:0\0" \
> +   "emmc_dev=" __stringify(CONFIG_FASTBOOT_FLASH_MMC_DEV) "\0" \
> +   "emmc_ack=1\0" \
> +
>  /* Initial environment variables */
>  #define CFG_EXTRA_ENV_SETTINGS \
> +   EMMCARGS \
> BOOTENV \
> "scriptaddr=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \
> "kernel_addr_r=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \
>
> and now UUU correctly flashes the eMMC hardware partition.
>
> Thanks a lot,

No problem

Micheal

>
> Fabio Estevam



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: imx8mp: Flashing U-Boot into eMMC hardware partition via UUU

2024-05-10 Thread Michael Nazzareno Trimarchi
On Fri, May 10, 2024 at 4:19 PM Fabio Estevam  wrote:
>
> Hi,
>
> I am using an imx8mp-evk board and I can flash the U-Boot into the
> eMMC hardware partition 0 by running:
>
>=> tftpboot $loadaddr flash.bin
>=> setexpr blkcnt $filesize + 0x1ff && setexpr blkcnt $blkcnt / 0x200
>=> mmc dev 2 1
>=> mmc write $loadaddr 0 $blkcnt
>
> Now I want to do the same via UUU.
>
> I tried to create a script called emmc_flash:
>
> uuu_version 1.5.21
>
> SDPS: boot -f flash.bin
> SDPS: done
> FB: ucmd setenv fastboot_dev mmc
> FB: ucmd mmc dev 2 1
> FB: flash bootloader flash.bin
> FB: Done
>
> Then on the PC: uuu emmc_flash
>
> U-Boot is loaded to RAM, but the eMMC hardware partition is not programmed.
>
> What is the correct script for doing this?
>
> Any suggestions?

Create an lst file

Hi Fabio

Top posting

# @_flash.bin| bootloader
# @_image   [_flash.bin] | image burn to nand, default is the same as bootloader
# @_filesystem   | filesystem to burn
# @_kernel   | kernel image
# @_dtb  | dtb image

# This command will be run when ROM support stream mode
# i.MX8QXP, i.MX8QM
SDPS: boot -f _flash.bin

FB: ucmd setenv fastboot_buffer ${loadaddr}
FB: download -f _image
# Burn image to nandfit partition if needed
FB: ucmd if env exists nandfit_part; then nand erase.part nandfit;
nand write ${fastboot_buffer} nandfit ${filesize}; else true; fi;
FB: ucmd nandbcb init ${fastboot_buffer} nandboot ${filesize}

FB[-t 1]: ucmd ubi part nandrootfs
FB[-t 1]: ucmd ubi create root -
FB: download -f _filesystem
FB[-t 6]: ucmd ubi write ${loadaddr} root ${filesize}

FB: download -f _kernel
FB[-t 1]: ucmd nand write ${loadaddr} nandkernel ${filesize}

FB: download -f _dtb
FB[-t 8000]: ucmd nand write ${loadaddr} nanddtb ${filesize}

FB: reboot
FB: done

You can just change as you want. We have this file in buildroot, uuu
can run command on the device
using FB command. Example how call it

${OUTPUT_DIR}/host/bin/uuu -v -b ${IMAGES_DIR}/nand-full.lst \
  ${IMAGES_DIR}/flash.bin \
  ${IMAGES_DIR}/flash.bin \
  ${IMAGES_DIR}/rootfs.ubifs \
  ${IMAGES_DIR}/Image \
  ${IMAGES_DIR}/freescale/imx8mn-bsh-smm-s2.dtb


Michael

>
> Thanks



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH] mtd: nand: pxa3xx: Incorrect bitflip return on page read

2024-05-06 Thread Michael Nazzareno Trimarchi
Hi Ravi

On Mon, May 6, 2024 at 7:33 PM Ravi Minnikanti  wrote:
>
> On 5/6/24 00:35, Michael Nazzareno Trimarchi wrote:
> > Hi Ravi
> >
> > On Tue, Apr 30, 2024 at 6:25 AM Ravi Minnikanti  
> > wrote:
> >>
> >> On 4/29/24 09:59, Michael Nazzareno Trimarchi wrote:
> >>
> >>> --
> >>> On Mon, Apr 29, 2024 at 6:22 PM Chris Packham  
> >>> wrote:
> >>>>
> >>>> On Sun, Apr 28, 2024 at 4:15 AM Ravi Minnikanti 
> >>>>  wrote:
> >>>>>
> >>>>> Once a page is read with higher bitflips all subsequent reads
> >>>>> are returning the same bitflip value even though they have none.
> >>>>> max_bitflip variable is not being reset to 0 across page reads.
> >>>>>
> >>>>> This is causing problems like incorrectly
> >>>>> marking erase blocks bad by UBI and causing read failures.
> >>>>>
> >>>>> Verified the change with both MTD reads and UBI.
> >>>>> This change is inline with other NFC drivers.
> >>>>>
> >>>>> Sample error log where a block is marked bad incorrectly:
> >>>>>
> >>>>> ubi0: fixable bit-flip detected at PEB 125
> >>>>> ubi0: run torture test for PEB 125
> >>>>> ubi0: fixable bit-flip detected at PEB 125
> >>>>> ubi0 error: torture_peb: read problems on freshly erased PEB 125,
> >>>>> must be bad
> >>>>> ubi0 error: erase_worker: failed to erase PEB 125, error -5
> >>>>> ubi0: mark PEB 125 as bad
> >>>>>
> >>>>> Signed-off-by: rminnikanti 
> >>>>
> >>>> Looks good to me
> >>>>
> >>>> Reviewed-by: Chris Packham 
> >>>>
> >>>>> ---
> >>>>>  drivers/mtd/nand/raw/pxa3xx_nand.c | 5 +
> >>>>>  1 file changed, 5 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/mtd/nand/raw/pxa3xx_nand.c 
> >>>>> b/drivers/mtd/nand/raw/pxa3xx_nand.c
> >>>>> index 1d9a6d107b..d2a4faad56 100644
> >>>>> --- a/drivers/mtd/nand/raw/pxa3xx_nand.c
> >>>>> +++ b/drivers/mtd/nand/raw/pxa3xx_nand.c
> >>>>> @@ -800,6 +800,11 @@ static void prepare_start_command(struct 
> >>>>> pxa3xx_nand_info *info, int command)
> >>>>> info->ecc_err_cnt   = 0;
> >>>>> info->ndcb3 = 0;
> >>>>> info->need_wait = 0;
> >>>>> +   /*
> >>>>> +    * Reset max_bitflips to zero. Once command is complete,
> >>>>> +* max_bitflips for this READ is returned in ecc.read_page()
> >>>>> +*/
> >>>>> +   info->max_bitflips  = 0;
> >>>>>
> >>>
> >>> Why this should not be put to 0 in read_page instead on 
> >>> prepare_start_command?
> >>>
> >>> Michael
> >>>
> >>
> >> ecc.read_page is invoked after the read command execution.
> >> First chip->cmdfunc is executed with NAND_CMD_READ0 and then ecc.read_page 
> >> is invoked
> >> to read the page from buffer. So, by the time read_page is invoked, 
> >> info->max_bitflips
> >> must already have the bit flip value.
> >>
> >
> > All the other implementation has a slight different way to handle.
> > From what you said the reset should
> > be done on for NAND_CMD_READ0 command and should be sufficient.
> > Technically should be moved
> > in switch and not unconditionally.
> >
> > Michael
> >
>
> max_bitflip is not being reset to 0 across page reads.
> Once a page is read with higher bitflips all subsequent reads
> are returning the same bitflip value even though they have none.
>
> This is causing problems like incorrectly
> marking erase blocks bad by UBI and read failures.
>
> Tested it with both MTD reads and UBI attach.
> This change is inline with other NFC drivers.
>
> Sample error log where a block is marked bad incorrectly:
>
> ubi0: fixable bit-flip detected at PEB 125
> ubi0: run torture test for PEB 125
> ubi0: fixable bit-flip detected at PEB 125
> ubi0 error: torture_peb: read problems on freshly erased PEB 125,
> must be bad
> ubi0 err

Re: [PATCH v2 0/6] Introduce UBI block device

2024-05-06 Thread Michael Nazzareno Trimarchi
Hi Alexey

Sorry will will put on CI

Michael

On Mon, May 6, 2024 at 3:58 PM Alexey Romanov
 wrote:
>
> Hello! Ping.
>
> On Mon, Mar 25, 2024 at 05:41:42PM +0300, Alexey Romanov wrote:
> > Hello!
> >
> > This series adds support for the UBI block device, which
> > allows to read/write data block by block. For example,
> > it can now be used for BCB or Android AB command:
> >
> >   $ bcb load ubi 0 part_name
> >
> > Tested only on SPI NAND, so bind is made only for
> > SPI NAND drivers. Can be used with mtdblock device [1].
> >
> > ---
> >
> > Changes V1 -> V2 [2]:
> >
> >  - Rebased over mtdblock v2 patchset [3].
> >  - Compile UBI partitions support only if CONFIG_BLK option
> >is enabled.
> >
> > - Links:
> >
> >   [1] 
> > https://lore.kernel.org/all/20240227100441.1811047-1-avroma...@salutedevices.com/
> >   [2] 
> > https://lore.kernel.org/all/20240306134906.1179285-1-avroma...@salutedevices.com/
> >   [3] 
> > https://lore.kernel.org/all/20240307130726.1582487-1-avroma...@salutedevices.com/
> >
> > Alexey Romanov (6):
> >   ubi: allow to read from volume with offset
> >   ubi: allow to write to volume with offset
> >   drivers: introduce UBI block abstraction
> >   disk: don't try search for partition type if already set
> >   disk: support UBI partitions
> >   spinand: bind UBI block
> >
> >  cmd/ubi.c   |  77 +++--
> >  disk/part.c |   7 ++
> >  drivers/block/blk-uclass.c  |   1 +
> >  drivers/mtd/nand/spi/core.c |   8 ++-
> >  drivers/mtd/ubi/Makefile|   1 +
> >  drivers/mtd/ubi/block.c | 130 
> >  drivers/mtd/ubi/part.c  |  99 +++
> >  env/ubi.c   |  16 ++---
> >  include/part.h  |   2 +
> >  include/ubi_uboot.h |   8 ++-
> >  10 files changed, 332 insertions(+), 17 deletions(-)
> >  create mode 100644 drivers/mtd/ubi/block.c
> >  create mode 100644 drivers/mtd/ubi/part.c
> >
> > --
> > 2.34.1
> >
>
> --
> Thank you,
> Alexey



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [EXTERNAL] Re: [PATCH] mtd: nand: pxa3xx: Incorrect bitflip return on page read

2024-05-06 Thread Michael Nazzareno Trimarchi
Hi Ravi

On Tue, Apr 30, 2024 at 6:25 AM Ravi Minnikanti  wrote:
>
> On 4/29/24 09:59, Michael Nazzareno Trimarchi wrote:
>
> > --
> > On Mon, Apr 29, 2024 at 6:22 PM Chris Packham  
> > wrote:
> >>
> >> On Sun, Apr 28, 2024 at 4:15 AM Ravi Minnikanti  
> >> wrote:
> >>>
> >>> Once a page is read with higher bitflips all subsequent reads
> >>> are returning the same bitflip value even though they have none.
> >>> max_bitflip variable is not being reset to 0 across page reads.
> >>>
> >>> This is causing problems like incorrectly
> >>> marking erase blocks bad by UBI and causing read failures.
> >>>
> >>> Verified the change with both MTD reads and UBI.
> >>> This change is inline with other NFC drivers.
> >>>
> >>> Sample error log where a block is marked bad incorrectly:
> >>>
> >>> ubi0: fixable bit-flip detected at PEB 125
> >>> ubi0: run torture test for PEB 125
> >>> ubi0: fixable bit-flip detected at PEB 125
> >>> ubi0 error: torture_peb: read problems on freshly erased PEB 125,
> >>> must be bad
> >>> ubi0 error: erase_worker: failed to erase PEB 125, error -5
> >>> ubi0: mark PEB 125 as bad
> >>>
> >>> Signed-off-by: rminnikanti 
> >>
> >> Looks good to me
> >>
> >> Reviewed-by: Chris Packham 
> >>
> >>> ---
> >>>  drivers/mtd/nand/raw/pxa3xx_nand.c | 5 +
> >>>  1 file changed, 5 insertions(+)
> >>>
> >>> diff --git a/drivers/mtd/nand/raw/pxa3xx_nand.c 
> >>> b/drivers/mtd/nand/raw/pxa3xx_nand.c
> >>> index 1d9a6d107b..d2a4faad56 100644
> >>> --- a/drivers/mtd/nand/raw/pxa3xx_nand.c
> >>> +++ b/drivers/mtd/nand/raw/pxa3xx_nand.c
> >>> @@ -800,6 +800,11 @@ static void prepare_start_command(struct 
> >>> pxa3xx_nand_info *info, int command)
> >>> info->ecc_err_cnt   = 0;
> >>> info->ndcb3 = 0;
> >>> info->need_wait = 0;
> >>> +   /*
> >>> +* Reset max_bitflips to zero. Once command is complete,
> >>> +* max_bitflips for this READ is returned in ecc.read_page()
> >>> +*/
> >>> +   info->max_bitflips  = 0;
> >>>
> >
> > Why this should not be put to 0 in read_page instead on 
> > prepare_start_command?
> >
> > Michael
> >
>
> ecc.read_page is invoked after the read command execution.
> First chip->cmdfunc is executed with NAND_CMD_READ0 and then ecc.read_page is 
> invoked
> to read the page from buffer. So, by the time read_page is invoked, 
> info->max_bitflips
> must already have the bit flip value.
>

All the other implementation has a slight different way to handle.
>From what you said the reset should
be done on for NAND_CMD_READ0 command and should be sufficient.
Technically should be moved
in switch and not unconditionally.

Michael

> Thanks, Ravi.
>
> >>> switch (command) {
> >>> case NAND_CMD_READ0:
> >>> --
> >>> 2.17.1
> >
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: imx8mn: bootcount does not increment

2024-05-02 Thread Michael Nazzareno Trimarchi
Hi Fabio

On Thu, May 2, 2024 at 2:45 PM Fabio Estevam  wrote:
>
> Hi Heiko,
>
> On Fri, Apr 26, 2024 at 12:40 AM Heiko Schocher  wrote:
>
> > No chance for a bootcounter in a register?
>
> Yes, this is a better approach. I changed it to:
>
> CONFIG_BOOTCOUNT_LIMIT=y
> CONFIG_SYS_BOOTCOUNT_MAGIC=0xB0C4
> CONFIG_SYS_BOOTCOUNT_ADDR=0x30370090
> CONFIG_SYS_BOOTCOUNT_SINGLEWORD=y
>
> and now bootcount increments.
>

Maybe you have anyway a bug on the evk when was not incrementing on environment

Michael

> Thanks,
>
> Fabio Estevam



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH 032/149] board: buffalo: Remove and add needed includes

2024-05-02 Thread Michael Walle
On Wed May 1, 2024 at 4:41 AM CEST, Tom Rini wrote:
> Remove  from this board vendor directory and when needed
> add missing include files directly.
>
> Signed-off-by: Tom Rini 

Acked-by: Michael Walle 


Re: [PATCH 080/149] board: kontron: Remove and add needed includes

2024-05-02 Thread Michael Walle
On Wed May 1, 2024 at 4:42 AM CEST, Tom Rini wrote:
> Remove  from this board vendor directory and when needed
> add missing include files directly.
>
> Signed-off-by: Tom Rini 

Acked-by: Michael Walle 


Re: [PATCH 030/149] board: bsh: Remove and add needed includes

2024-05-01 Thread Michael Nazzareno Trimarchi
Hi

On Wed, May 1, 2024 at 4:43 AM Tom Rini  wrote:
>
> Remove  from this board vendor directory and when needed
> add missing include files directly.
>
> Signed-off-by: Tom Rini 
> ---
> Cc: Michael Trimarchi 
> ---
>  board/bsh/imx6ulz_smm_m2/imx6ulz_smm_m2.c | 1 -
>  board/bsh/imx6ulz_smm_m2/spl.c| 1 -
>  board/bsh/imx8mn_smm_s2/imx8mn_smm_s2.c   | 1 -
>  3 files changed, 3 deletions(-)
>
> diff --git a/board/bsh/imx6ulz_smm_m2/imx6ulz_smm_m2.c 
> b/board/bsh/imx6ulz_smm_m2/imx6ulz_smm_m2.c
> index c82eabbfbea1..c03e390762a9 100644
> --- a/board/bsh/imx6ulz_smm_m2/imx6ulz_smm_m2.c
> +++ b/board/bsh/imx6ulz_smm_m2/imx6ulz_smm_m2.c
> @@ -14,7 +14,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>
> diff --git a/board/bsh/imx6ulz_smm_m2/spl.c b/board/bsh/imx6ulz_smm_m2/spl.c
> index 5b4812e129e3..724841b57456 100644
> --- a/board/bsh/imx6ulz_smm_m2/spl.c
> +++ b/board/bsh/imx6ulz_smm_m2/spl.c
> @@ -1,6 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0+
>
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/board/bsh/imx8mn_smm_s2/imx8mn_smm_s2.c 
> b/board/bsh/imx8mn_smm_s2/imx8mn_smm_s2.c
> index 0ebf208be82a..c99896873991 100644
> --- a/board/bsh/imx8mn_smm_s2/imx8mn_smm_s2.c
> +++ b/board/bsh/imx8mn_smm_s2/imx8mn_smm_s2.c
> @@ -3,7 +3,6 @@
>   * Copyright 2021 Collabora Ltd.
>   */
>
> -#include 
>  #include 
>  #include 
>
> --
> 2.34.1
>

Reviewed-by: Michael Trimarchi 


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH] mtd: nand: pxa3xx: Incorrect bitflip return on page read

2024-04-29 Thread Michael Nazzareno Trimarchi
On Mon, Apr 29, 2024 at 6:22 PM Chris Packham  wrote:
>
> On Sun, Apr 28, 2024 at 4:15 AM Ravi Minnikanti  
> wrote:
> >
> > Once a page is read with higher bitflips all subsequent reads
> > are returning the same bitflip value even though they have none.
> > max_bitflip variable is not being reset to 0 across page reads.
> >
> > This is causing problems like incorrectly
> > marking erase blocks bad by UBI and causing read failures.
> >
> > Verified the change with both MTD reads and UBI.
> > This change is inline with other NFC drivers.
> >
> > Sample error log where a block is marked bad incorrectly:
> >
> > ubi0: fixable bit-flip detected at PEB 125
> > ubi0: run torture test for PEB 125
> > ubi0: fixable bit-flip detected at PEB 125
> > ubi0 error: torture_peb: read problems on freshly erased PEB 125,
> > must be bad
> > ubi0 error: erase_worker: failed to erase PEB 125, error -5
> > ubi0: mark PEB 125 as bad
> >
> > Signed-off-by: rminnikanti 
>
> Looks good to me
>
> Reviewed-by: Chris Packham 
>
> > ---
> >  drivers/mtd/nand/raw/pxa3xx_nand.c | 5 +
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/pxa3xx_nand.c 
> > b/drivers/mtd/nand/raw/pxa3xx_nand.c
> > index 1d9a6d107b..d2a4faad56 100644
> > --- a/drivers/mtd/nand/raw/pxa3xx_nand.c
> > +++ b/drivers/mtd/nand/raw/pxa3xx_nand.c
> > @@ -800,6 +800,11 @@ static void prepare_start_command(struct 
> > pxa3xx_nand_info *info, int command)
> > info->ecc_err_cnt   = 0;
> > info->ndcb3 = 0;
> > info->need_wait = 0;
> > +   /*
> > +* Reset max_bitflips to zero. Once command is complete,
> > +* max_bitflips for this READ is returned in ecc.read_page()
> > +*/
> > +   info->max_bitflips  = 0;
> >

Why this should not be put to 0 in read_page instead on prepare_start_command?

Michael

> > switch (command) {
> > case NAND_CMD_READ0:
> > --
> > 2.17.1

-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH] ubi: Depend on MTD

2024-04-26 Thread Michael Nazzareno Trimarchi
Hi Dario

On Fri, Apr 26, 2024 at 5:52 PM Tom Rini  wrote:
>
> On Fri, Apr 26, 2024 at 07:18:18AM +0200, Heiko Schocher wrote:
> > Hello Tom,
> >
> > On 11.04.24 08:09, Michael Nazzareno Trimarchi wrote:
> > > Hi
> > >
> > > On Thu, Apr 11, 2024 at 7:06 AM John Watts  wrote:
> > > >
> > > > UBI required MTD to build correctly, add it as a Kconfig dependency.
> > > >
> > > > Signed-off-by: John Watts 
> > > > ---
> > > > While working with UBI on my SPI NAND patch series I found it was
> > > > possible to enable it without enabling the MTD subsystem.
> > > > Add a Kconfig option to solve this.
> > > > ---
> > > >   drivers/mtd/ubi/Kconfig | 1 +
> > > >   1 file changed, 1 insertion(+)
> >
> > Seems I am too dummy to find this patch in Patchwork, nor is it
> > applied in master ... can you pick it up, or should I do this and
> > send you a pull request?
>
> It's over at
> https://patchwork.ozlabs.org/project/uboot/patch/20240411-mtd-v1-1-fe300f6ab...@jookia.org/
> currently but you can take it up and send it to me if you like.
>

Can you pick it up? Seems that you are delegate and was already reviewed by me

Michael

> --
> Tom



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: Capsule GUIDs and LVFS

2024-04-25 Thread Michael Walle
Hi,

On Thu Apr 25, 2024 at 8:19 AM CEST, Ilias Apalodimas wrote:
> I've cc'ed all the people I could find in board specific MAINTAINER files.
> Can you respond to Richard with the proper company name & board name
> so we can bind the following GUIDs to a vendor properly?
> Richard any guidance on how this should be done properly is
> appreciated, since I am not too aware of LVFS internals.

Thanks for taking care!

> Kontron KONTRON_PITX_IMX8M_FIT_IMAGE_GUID c898e959-5b1f-4e6d-88e0-40d45cca1399

This (board) should likely be dropped to lower maintenance burden as
it is EOL and "not recommended for new designs".

> Kontron KONTRON_SL_MX8MM_FIT_IMAGE_GUID d488e45a-4929-4b55-8c14-86cea2cd6629

This is probably (at least) the wrong name. SL is a
System-on-Module, a small PCB that will be soldered on a carrier
board and cannot run on it's own and a customer using that SoM will
likely have their own bootloader. There is, though the "BL" which is
the in-house board for this SoM.

Company name is "Kontron Electronics GmbH".

> Kontron KONTRON_SL28_FIT_IMAGE_GUID 86ebd44f-feb8-466f-8bb8-890618456d8b

Company name is "Kontron Europe GmbH".

Both entities belong to the Kontron Group, not sure how this is
handled in LVFS though.

-michael


signature.asc
Description: PGP signature


Re: [PATCH] clk: imx8mn: add video clocks support

2024-04-22 Thread Michael Nazzareno Trimarchi
Hi Fabio

On Sun, Apr 21, 2024 at 10:54 PM Michael Nazzareno Trimarchi
 wrote:
>
> Hi Fabio
>
> On Sun, Apr 21, 2024 at 10:24 PM Fabio Estevam  wrote:
> >
> > Hi Michael,
> >
> > On Sun, Apr 21, 2024 at 11:07 AM Michael Trimarchi
> >  wrote:
> > >
> > > Add clocks support for the video subsystem.
> > >
> > > Signed-off-by: Michael Trimarchi 
> >
> > Which target will make use of these clocks?
> >
> > As-is this patch adds only dead code.
> >
> > Adding a defconfig that uses these newly introduced clocks would be nice.
> >
>
> You are right, I will wrap it and enable only on CONFIG_VIDEO
>
> > Also, to avoid size increase, please protect this code against
> > CONFIG_VIDEO or something, like done here:
> > https://source.denx.de/u-boot/custodians/u-boot-imx/-/commit/2b3310ef13998dfd03196a0806e03035212b102c
>
> Working on display panel integration on imx8m, trying to progress
> merging things up.
>

Are you considering if I wrap properly with VIDEO IS_ENABLED?

Michael

> Michael
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> mich...@amarulasolutions.com
> __
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> i...@amarulasolutions.com
> www.amarulasolutions.com



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH] clk: imx8mn: add video clocks support

2024-04-21 Thread Michael Nazzareno Trimarchi
Hi Fabio

On Sun, Apr 21, 2024 at 10:24 PM Fabio Estevam  wrote:
>
> Hi Michael,
>
> On Sun, Apr 21, 2024 at 11:07 AM Michael Trimarchi
>  wrote:
> >
> > Add clocks support for the video subsystem.
> >
> > Signed-off-by: Michael Trimarchi 
>
> Which target will make use of these clocks?
>
> As-is this patch adds only dead code.
>
> Adding a defconfig that uses these newly introduced clocks would be nice.
>

You are right, I will wrap it and enable only on CONFIG_VIDEO

> Also, to avoid size increase, please protect this code against
> CONFIG_VIDEO or something, like done here:
> https://source.denx.de/u-boot/custodians/u-boot-imx/-/commit/2b3310ef13998dfd03196a0806e03035212b102c

Working on display panel integration on imx8m, trying to progress
merging things up.

Michael



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


[PATCH] clk: imx8mn: add video clocks support

2024-04-21 Thread Michael Trimarchi
Add clocks support for the video subsystem.

Signed-off-by: Michael Trimarchi 
---
 drivers/clk/imx/clk-imx8mn.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index 457acb8a40..baac79dd29 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -23,6 +23,7 @@ static const char *arm_pll_bypass_sels[] = {"arm_pll", 
"arm_pll_ref_sel", };
 static const char *sys_pll1_bypass_sels[] = {"sys_pll1", "sys_pll1_ref_sel", };
 static const char *sys_pll2_bypass_sels[] = {"sys_pll2", "sys_pll2_ref_sel", };
 static const char *sys_pll3_bypass_sels[] = {"sys_pll3", "sys_pll3_ref_sel", };
+static const char *video_pll_bypass_sels[] = {"video_pll", 
"video_pll_ref_sel", };
 
 static const char *imx8mn_a53_sels[] = {"clock-osc-24m", "arm_pll_out", 
"sys_pll2_500m", "sys_pll2_1000m",
"sys_pll1_800m", "sys_pll1_400m", 
"audio_pll1_out", "sys_pll3_out", };
@@ -30,6 +31,10 @@ static const char *imx8mn_a53_sels[] = {"clock-osc-24m", 
"arm_pll_out", "sys_pll
 static const char *imx8mn_ahb_sels[] = {"clock-osc-24m", "sys_pll1_133m", 
"sys_pll1_800m", "sys_pll1_400m",
"sys_pll2_125m", "sys_pll3_out", 
"audio_pll1_out", "video_pll_out", };
 
+static const char *imx8mn_disp_pixel_sels[] = {"osc_24m", "video_pll_out", 
"audio_pll2_out",
+  "audio_pll1_out", 
"sys_pll1_800m", "sys_pll2_1000m",
+  "sys_pll3_out", "clk_ext4", };
+
 static const char *imx8mn_enet_axi_sels[] = {"clock-osc-24m", "sys_pll1_266m", 
"sys_pll1_800m", "sys_pll2_250m",
 "sys_pll2_200m", "audio_pll1_out", 
"video_pll_out", "sys_pll3_out", };
 
@@ -139,6 +144,9 @@ static int imx8mn_clk_probe(struct udevice *dev)
clk_dm(IMX8MN_SYS_PLL3_REF_SEL,
   imx_clk_mux("sys_pll3_ref_sel", base + 0x114, 0, 2,
   pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
+   clk_dm(IMX8MN_VIDEO_PLL1_REF_SEL,
+  imx_clk_mux("video_pll_ref_sel", base + 0x28, 0, 2,
+  pll_ref_sels, ARRAY_SIZE(pll_ref_sels)));
 
clk_dm(IMX8MN_DRAM_PLL,
   imx_clk_pll14xx("dram_pll", "dram_pll_ref_sel",
@@ -155,6 +163,9 @@ static int imx8mn_clk_probe(struct udevice *dev)
clk_dm(IMX8MN_SYS_PLL3,
   imx_clk_pll14xx("sys_pll3", "sys_pll3_ref_sel",
   base + 0x114, _1416x_pll));
+   clk_dm(IMX8MN_VIDEO_PLL1,
+  imx_clk_pll14xx("video_pll", "video_pll_ref_sel",
+  base + 0x28, _1443x_pll));
 
/* PLL bypass out */
clk_dm(IMX8MN_DRAM_PLL_BYPASS,
@@ -183,6 +194,12 @@ static int imx8mn_clk_probe(struct udevice *dev)
 ARRAY_SIZE(sys_pll3_bypass_sels),
 CLK_SET_RATE_PARENT));
 
+   clk_dm(IMX8MN_VIDEO_PLL1_BYPASS,
+  imx_clk_mux_flags("video_pll_bypass", base + 0x28, 16, 1,
+video_pll_bypass_sels,
+ARRAY_SIZE(video_pll_bypass_sels),
+CLK_SET_RATE_PARENT));
+
/* PLL out gate */
clk_dm(IMX8MN_DRAM_PLL_OUT,
   imx_clk_gate("dram_pll_out", "dram_pll_bypass",
@@ -199,6 +216,9 @@ static int imx8mn_clk_probe(struct udevice *dev)
clk_dm(IMX8MN_SYS_PLL3_OUT,
   imx_clk_gate("sys_pll3_out", "sys_pll3_bypass",
base + 0x114, 11));
+   clk_dm(IMX8MN_VIDEO_PLL1_OUT,
+  imx_clk_gate("video_pll_out", "video_pll_bypass",
+   base + 0x28, 13));
 
/* SYS PLL fixed output */
clk_dm(IMX8MN_SYS_PLL1_40M,
@@ -275,6 +295,9 @@ static int imx8mn_clk_probe(struct udevice *dev)
clk_dm(IMX8MN_CLK_USDHC2,
   imx8m_clk_composite("usdhc2", imx8mn_usdhc2_sels,
   base + 0xac80));
+
+   clk_dm(IMX8MN_CLK_DISP_PIXEL,
+  imx8m_clk_composite("disp_pixel", imx8mn_disp_pixel_sels, base + 
0xa500));
clk_dm(IMX8MN_CLK_I2C1,
   imx8m_clk_composite("i2c1", imx8mn_i2c1_sels, base + 0xad00));
clk_dm(IMX8MN_CLK_I2C2,
-- 
2.40.1



Re: [PATCH v1] mtd: rawnand: macronix: OTP access for MX30LFxG18AC

2024-04-17 Thread Michael Nazzareno Trimarchi
Hi

Dario did you add those patches in CI and test them again?

Michael

On Wed, Apr 17, 2024 at 8:44 PM Arseniy Krasnov
 wrote:
>
> Hello,
>
> Sorry, pls ping
>
> Thanks, Arseniy
>
> On 13.03.2024 09:46, Michael Nazzareno Trimarchi wrote:
> > Hi  Dario
> >
> > Can apply this series and put in CI?
> >
> > Michael
> >
> > On Wed, Mar 13, 2024 at 7:43 AM Arseniy Krasnov
> >  wrote:
> >>
> >> Sorry, please ping
> >>
> >> Thanks, Arseniy
> >>
> >> On 11.02.2024 02:16, Arseniy Krasnov wrote:
> >>> Sorry, pls ping
> >>>
> >>> Thanks, Arseniy
> >>>
> >>> On 08.01.2024 21:33, Arseniy Krasnov wrote:
> >>>> Sorry, pls ping
> >>>>
> >>>> Thanks, Arseniy
> >
> >
> >



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH 1/1] net: wget: fix TCP sequence number wrap around issue

2024-04-15 Thread Michael Nazzareno Trimarchi
Hi

On Mon, Apr 15, 2024 at 3:55 PM Michael Nazzareno Trimarchi
 wrote:
>
> Hi
>
> On Mon, Apr 15, 2024 at 3:48 PM Yasuharu Shibata
>  wrote:
> >
> > Hi Michael,
> >
> > On Mon, 15 Apr 2024 at 22:03, Michael Nazzareno Trimarchi
> >  wrote:
> > >
> > > I think I have sent some time ago ;)
> > >
> > > Anyway look sane. I was having the same feeling on code inspection
> > >
> > > Reviewed-by: Michael Trimarchi 
> >
> > Thank you for your review.
> > I already checked the thread, sorry I couldn't find your patch and
> > I couldn't see whether it is the same.
> > In any case, I consider there is a potential issue about
> > wrap around, so I submitted a patch.
> >
>
> Very good job ;) to fix it. Just add Suggest-by: ;)
>

https://lore.kernel.org/all/caomzo5ao5x3ahr0ayriijya309usua0hj6okrhtqqvhw7i8...@mail.gmail.com/T/

Mine was here

Michael

> Michael
>
> > --
> > Best regards,
> > Yasuharu Shibata
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> mich...@amarulasolutions.com
> __
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> i...@amarulasolutions.com
> www.amarulasolutions.com



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH 1/1] net: wget: fix TCP sequence number wrap around issue

2024-04-15 Thread Michael Nazzareno Trimarchi
Hi

On Mon, Apr 15, 2024 at 3:48 PM Yasuharu Shibata
 wrote:
>
> Hi Michael,
>
> On Mon, 15 Apr 2024 at 22:03, Michael Nazzareno Trimarchi
>  wrote:
> >
> > I think I have sent some time ago ;)
> >
> > Anyway look sane. I was having the same feeling on code inspection
> >
> > Reviewed-by: Michael Trimarchi 
>
> Thank you for your review.
> I already checked the thread, sorry I couldn't find your patch and
> I couldn't see whether it is the same.
> In any case, I consider there is a potential issue about
> wrap around, so I submitted a patch.
>

Very good job ;) to fix it. Just add Suggest-by: ;)

Michael

> --
> Best regards,
> Yasuharu Shibata



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH 1/1] net: wget: fix TCP sequence number wrap around issue

2024-04-15 Thread Michael Nazzareno Trimarchi
Hi

On Mon, Apr 15, 2024 at 3:01 PM Yasuharu Shibata
 wrote:
>
> If tcp_seq_num is wrap around, tcp_seq_num >= initial_data_seq_num
> isn't satisfied and store_block() isn't called.
> The condition has a wrap around issue, so it is fixed in this patch.
>
> Signed-off-by: Yasuharu Shibata 
> ---
>  net/wget.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/net/wget.c b/net/wget.c
> index 71bac92d84..abab371e58 100644
> --- a/net/wget.c
> +++ b/net/wget.c
> @@ -404,9 +404,7 @@ static void wget_handler(uchar *pkt, u16 dport,
> }
> next_data_seq_num = tcp_seq_num + len;
>
> -   if (tcp_seq_num >= initial_data_seq_num &&
> -   store_block(pkt, tcp_seq_num - initial_data_seq_num,
> -   len) != 0) {
> +   if (store_block(pkt, tcp_seq_num - initial_data_seq_num, len) 
> != 0) {
> wget_fail("wget: store error\n",
>   tcp_seq_num, tcp_ack_num, action);
> net_set_state(NETLOOP_FAIL);

I think I have sent some time ago ;)

Anyway look sane. I was having the same feeling on code inspection

Reviewed-by: Michael Trimarchi 

> --
> 2.25.1
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH] board: sl28: move to OF_UPSTREAM

2024-04-15 Thread Michael Walle
On Wed Mar 6, 2024 at 5:19 PM CET, Michael Walle wrote:
> Use the new device devicetree files in dts/upstream/ and delete the old
> ones. Still keep the -u-boot.dtsi with all u-boot specifics around.
>
> There is one catch and that is fsl-ls1028a-kontron-sl28-var3.dts which
> is not available upstream (yet!). For now, the base dts is used for this
> variant as this only differ in the compatible and the (human readable)
> model name.
>
> Signed-off-by: Michael Walle 

Ping. Any news here?

-michael


signature.asc
Description: PGP signature


Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries

2024-04-12 Thread Michael Walle
Hi,

On Fri Apr 12, 2024 at 5:03 AM CEST, Neha Malcom Francis wrote:
> On 05/04/24 13:12, Michael Walle wrote:
> > On Thu Apr 4, 2024 at 11:10 AM CEST, Neha Malcom Francis wrote:
> >> But again in the interest of time... this would mean this cleaning up 
> >> effort be
> >> kept on hold. If we can agree to move to using the generator later as the 
> >> final
> >> solution, can we pick up this series for now?
> > 
> > Agreed. I just saw the new RFC for the j722s support. It should also
> > make use of this cleanup then, btw.
> > 
>
> Right, I'll sync with J722S efforts as well.
>
> So is this series good to go?

If the ultimate goal is to support the generator, sure.

-michael


Re: [PATCH v3 0/3] Introduce mtdblock device

2024-04-11 Thread Michael Nazzareno Trimarchi
Hi

I will review tomorrow, I need have a time window to test even on my board

Mihcael

On Thu, Apr 11, 2024 at 6:09 PM Alexey Romanov
 wrote:
>
> Hello! Ping.
>
> On Thu, Apr 04, 2024 at 01:58:10PM +0300, Alexey Romanov wrote:
> > Hello!
> >
> > This series adds support for the mtdblock device, which
> > allows to read/write data block by block. For example,
> > it can now be used for BCB or Android AB command:
> >
> >   $ bcb load mtd 0 part_name
> >
> > Tested only on SPI NAND, so bind is made only for
> > SPI NAND drivers.
> >
> > ---
> >
> > Changes V1 -> V2 [1]:
> >
> >   - Drop patch [2].
> >   - Add warning if bind NAND mtdblock device.
> >   - Move documentation of mtdblock implementation
> > from commit message to the source code.
> >   - Remove __maybe_unused from mtd partition functions
> > description.
> >   - Use blk_enabled() instead of #ifdefs.
> >
> > Changes V2 -> V3 [2]:
> >
> >   - Rebased over [3].
> >   - Rename mtd_bread/bwrite -> mtd_blk_read/write.
> >   - Fix GPL-2.0 license short name definiton in headers.
> >   - Add empty line after MTD_ENTRY_NUMBERS define.
> >
> > Links:
> >
> >   - [1] 
> > https://lore.kernel.org/all/20240227100441.1811047-1-avroma...@salutedevices.com/
> >   - [2] 
> > https://lore.kernel.org/all/20240227100441.1811047-5-avroma...@salutedevices.com/
> >   - [3] 
> > https://lore.kernel.org/u-boot/20240403114047.84030-1-heinrich.schucha...@canonical.com/T/#u
> >
> > Alexey Romanov (3):
> >   disk: support MTD partitions
> >   drivers: introduce mtdblock abstraction
> >   spinand: bind mtdblock
> >
> >  disk/part.c |   3 +-
> >  drivers/block/blk-uclass.c  |   1 +
> >  drivers/mtd/Kconfig |   1 +
> >  drivers/mtd/Makefile|   1 +
> >  drivers/mtd/mtdblock.c  | 227 
> >  drivers/mtd/mtdpart.c   |  69 +++
> >  drivers/mtd/nand/spi/core.c |  20 
> >  include/linux/mtd/mtd.h |  12 ++
> >  include/part.h  |   3 +
> >  9 files changed, 336 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/mtd/mtdblock.c
> >
> > --
> > 2.34.1
> >
>
> --
> Thank you,
> Alexey



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH] ubi: Depend on MTD

2024-04-11 Thread Michael Nazzareno Trimarchi
Hi

On Thu, Apr 11, 2024 at 7:06 AM John Watts  wrote:
>
> UBI required MTD to build correctly, add it as a Kconfig dependency.
>
> Signed-off-by: John Watts 
> ---
> While working with UBI on my SPI NAND patch series I found it was
> possible to enable it without enabling the MTD subsystem.
> Add a Kconfig option to solve this.
> ---
>  drivers/mtd/ubi/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/mtd/ubi/Kconfig b/drivers/mtd/ubi/Kconfig
> index 5783d36c04..fd446d6efb 100644
> --- a/drivers/mtd/ubi/Kconfig
> +++ b/drivers/mtd/ubi/Kconfig
> @@ -9,6 +9,7 @@ config UBI_SILENCE_MSG
>
>  config MTD_UBI
> bool "Enable UBI - Unsorted block images"
> +   depends on MTD
> select RBTREE
> select MTD_PARTITIONS
> help
>
> ---

Reviewed-by: Michael Trimarchi 

> base-commit: 777c28460947371ada40868dc994dfe8537d7115
> change-id: 20240411-mtd-d2e811e17cc8
>


> Best regards,
> --
> John Watts 
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries

2024-04-05 Thread Michael Walle
Hi,

On Thu Apr 4, 2024 at 11:10 AM CEST, Neha Malcom Francis wrote:
> But again in the interest of time... this would mean this cleaning up effort 
> be 
> kept on hold. If we can agree to move to using the generator later as the 
> final 
> solution, can we pick up this series for now?

Agreed. I just saw the new RFC for the j722s support. It should also
make use of this cleanup then, btw.

-michael


Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries

2024-04-03 Thread Michael Walle
> > > > > specifics, like the FIT configuration. I mean I could just copy all
> > > > 
> > > > Correct me if I'm wrong, but my understanding is that you would want to
> > > > add more FDT blobs in the *-binman.dtsi correct? That is still possible,
> > > > adding another "fdt-1" and "conf-1" in the
> > > > 
> > > > Something like this in your -u-boot.dtsi,
> > > > 
> > > > tispl {
> > > > insert-template = <_spl>;
> > > > fit {
> > > > images {
> > > > fdt-1 {
> > > > ...
> > > > };
> > > > };
> > > > configurations {
> > > > conf-1 {
> > > > ...
> > > > };
> > > > };
> > > > };
> > > > };
> > > 
> > > Then you have the information at two places. One being the "#define
> > > SPL_BOARD_DTB" stuff and the other one being in this additional DT
> > > fragment. That is really confusing.
> > > 
> > 
> > Hm... maybe. I personally don't see it as confusing. Even when picking
> > between multiple DTBs, you will have a default DTB in any case, marking that
> > as a macro wouldn't be confusing right? We'll need to get a third opinion on
> > here then, I had seen your ping on IRC [1], putting it here for the others
> > as well.
> > 
>
> As I see it, it's not like we are making the fdt-0 non overridable, you
> can still override it in your configs to make it cleaner if you want for
> your board template, I don't think that -

Though it is not overriding but rather merging, correct? So one
would need to first erase the node just to create it again. Which
looks more like a workaround.

> tispl {
>   insert-template = <_spl>;
>   fit {
>   images {
>   fdt-0 {
>   ... 
>   };
>   fdt-1 {
>       ...
>   };
>   };
>   configurations {
>   conf-0 {
>   ...
>   };
>   conf-1 {
>   ...
>   };
>   };
>   };
> };

>
> is not doable. It might be a bit duplicate but if I think about it but
> we are not losing out on extending the templates for multiple DTBs even
> with this design. I know it might not be what you want but I feel that
> for single DTB it's really convenient with the macro stuff and we don't
> have to override any of the other binman nodes.

I've raised my concern about stuffing board dependent stuff into the
now generic "k3--binman.dtsi". I get it that it will work for
90% of the boards and that it is very convenient. I'd have rather
seen a split of lets say
  k3--binman.dtsi
and
  k3-one-dtb-template-binman.dtsi

All the generic stuff goes into k3-soc-binman.dtsi whereas 90% of
the boards might still use the second dtsi with some define magic.
But it seems you've already made your mind up on that :)

-michael


signature.asc
Description: PGP signature


[PATCH] net: ti: am65-cpsw: Fix buffer overflow

2024-04-03 Thread Michael Walle
The device name is a concatenation of the device node name of the cpsw
device and of the device node name of the port. In my case that is

  ethernet@800
  port@1

First the buffer is really too small, but more importantly, there is no
boundary check. Use snprintf() and increase the buffer size.

Fixes: 38922b1f4acc ("net: ti: am65-cpsw: Add support for multi port 
independent MAC mode")
Signed-off-by: Michael Walle 
---
 drivers/net/ti/am65-cpsw-nuss.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ti/am65-cpsw-nuss.c b/drivers/net/ti/am65-cpsw-nuss.c
index d68ed671836..b151e25d6a4 100644
--- a/drivers/net/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ti/am65-cpsw-nuss.c
@@ -664,7 +664,7 @@ static int am65_cpsw_port_probe(struct udevice *dev)
struct am65_cpsw_priv *priv = dev_get_priv(dev);
struct eth_pdata *pdata = dev_get_plat(dev);
struct am65_cpsw_common *cpsw_common;
-   char portname[15];
+   char portname[32];
int ret;
 
priv->dev = dev;
@@ -672,7 +672,7 @@ static int am65_cpsw_port_probe(struct udevice *dev)
cpsw_common = dev_get_priv(dev->parent);
priv->cpsw_common = cpsw_common;
 
-   sprintf(portname, "%s%s", dev->parent->name, dev->name);
+   snprintf(portname, sizeof(portname), "%s%s", dev->parent->name, 
dev->name);
device_set_name(dev, portname);
 
ret = am65_cpsw_ofdata_parse_phy(dev);
-- 
2.39.2



Re: [PATCH] net: phy: broadcom: Configure LEDs on BCM54210E

2024-04-02 Thread Michael Walle
On Thu Mar 28, 2024 at 4:09 PM CET, Tom Rini wrote:
> On Mon, Jan 01, 2024 at 10:07:47PM +0100, Marek Vasut wrote:
>
> > Configure LEDs on BCM54210E so they would blink on activity
> > and indicate link speed. Without this the LEDs are always on
> > if cable is plugged in.
> > 
> > Signed-off-by: Marek Vasut 
>
> Applied to u-boot/next, thanks!

Pretty late and I'm not implying this should be reverted. I just
want to point out, that this is really board dependent and might
even break boards which have this PHY and are using its default
configuration for the attached LEDs.

FWIW, linux now have LED PHY DT bindings.

-michael


signature.asc
Description: PGP signature


Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries

2024-03-28 Thread Michael Walle
Hi,

On Thu Mar 28, 2024 at 12:18 PM CET, Neha Malcom Francis wrote:
> On 27-Mar-24 8:03 PM, Michael Walle wrote:
> > On Wed Mar 27, 2024 at 8:01 AM CET, Neha Malcom Francis wrote:
> >> On 26/03/24 19:18, Michael Walle wrote:
> >>> On Fri Mar 22, 2024 at 2:10 PM CET, Neha Malcom Francis wrote:
> >>>> Clean up templatized boot binaries for all K3 boards. This includes
> >>>> modifying the k3-binman.dtsi to use SPL_BOARD_DTB, BOARD_DESCRIPTION and
> >>>> UBOOT_BOARD_DESCRIPTION from the files that include it to further reuse
> >>>> code.
> >>>>
> >>>> All k3--binman.dtsi will contain only templates. Only required boot
> >>>> binaries can be built from the templates in the boards' respective
> >>>> -u-boot.dtsi file (or k3--binman.dtsi if it exists). This allows
> >>>> clear distinction between the SoC common stuff vs. what is additionally
> >>>> needed to boot up a specific board.
> >>>
> >>> I appreciate the cleanup. But as far as I can see, a board might
> >>> only have one device tree. How would that work if the uboot proper
> >>> must support multiple device trees?
> >>>
> >>
> >>   From the discussions that took place in the mailing list [1] the 
> >> consensus
> >> seems to be to not focus on multiple devicetree support as it leads to 
> >> confusion
> >> for downstream users.
> > 
> > What are users in this regard? I don't think you'd confuse
> > developers.
> > 
> > Anyway, I'm planning on upstreaming a TI board which will have
> > different memory configurations and different variants of the board.
>
> I am assuming you are reusing an existing TI SoC?

Not really yet. It's the j722s.


> > And on top of that, it will just be a base board and there will
> > likely be some carrier device trees (overlay? I'm not sure yet).
> > 
> > As far as I can tell, you've put the memory configuration into the
> > device tree, so I'll probably need to switch between them somehow.
>
> The "k3--ddr.dtsi" file will be present in your k3-r5.dts 
> which makes sense, the memory configuration depends on the board.

And one board might have multiple configuration depending on the
variant of the board. Typically, one board is available with
different memory options. i.e. 1GiB, 4GiB and so on. The actual RAM
chips can come from different manufacturers. So all all, I presume
there will be different RAM settings, i.e. different
k3--ddr.dtsi. But I have to switch between the setting during
runtime because there will be only one boot image for that board.

> > Also, regarding the board variants, I'll probably need to choose
> > between multiple device trees. That is invisible to the user,
> > because u-boot will choose the correct DTB according a board
> > strapping, which btw. works really fine, see for example
> > (boards/kontron/sl28/spl.c:board_fit_config_name_match).
>
> Again, this is assuming that there is some HW blown register available 
> for the board to use (or in our earlier K3 case, the EEPROM) but that is 
> not necessarily true every time.

No, that is of course board dependent. It is just an example that
there are boards with more than one DTB.

Let's step back a bit. Right now, there is
  k3---binman.dtsi
which is fine. But it seems, that TI is heading towards a common
  k3--binman.dtsi
which is intended to be used by all the boards that are using that
particular SoC, correct me if I'm wrong here. Now the problem with
that is that you hardcode the FIT configuations which are really
board dependent and assume that there will be exactly one DTB per
board, i.e. your "#define SPL_BOARD_DTB" etc.

Thus, what I was trying to say is that you should split all the
board independent configuration (dt fragments) from the board
specific configuration.

And again, of course I could just ignore the k3--binman.dtsi
and just use a suitable copy "k3---binman.dtsi" for my
board. But as I said, I'm not sure, this is the way to go and I have
a slight feeling I will be asked to reuse the "k3--binman.dtsi"
when I submit my board support.

> > 
> > I don't think it makes much sense to hardcode your generic
> > *-binman.dtsi to just one FIT configuration. I'd rather see a split
> > between generic things which are shared across all boards and board
> > specifics, like the FIT configuration. I mean I could just copy all
>
> Correct me if I'm wrong, but my understanding is that you would want to 
> add more FDT blobs in the *-binman.dtsi correct? That is still possible, 
> adding another "fdt-1" and "conf-1" in the
>

Re: [PATCH v2] arm: dts: kirkwood: Enable upstream DT on Kirkwood boards

2024-03-28 Thread Michael Walle
Hi,

On Thu Mar 28, 2024 at 3:18 AM CET, Tony Dinh wrote:
> Enable OF_UPSTREAM to use upstream DT and add marvell/ prefix to the
> DEFAULT_DEVICE_TREE for Kirkwood boards. And so we can directly build
> DTBs from dts/upstream/src/arm/marvell, and including *-u-boot.dtsi
> files from arch/arm/dts/ directory.
>
> Background:
>
> Hi Stefan,
> Hi Michael,
>
> I did a survey and we currently have 28 Kirkwood boards. Using some
> commands and filters, here are the finding.
>
> git grep -li arch_kirkwood configs | xargs grep DEVICE_TREE | cut -d '"' -f2 
> | xargs -n1 sh -c 'diff -qs  arch/arm/dts/$1.dts 
> dts/upstream/src/arm/marvell/$1.dts' sh | grep differ
>
> diff: dts/upstream/src/arm/marvell/kirkwood-atl-sbx81lifkw.dts: No such file 
> or directory
> diff: dts/upstream/src/arm/marvell/kirkwood-atl-sbx81lifxcat.dts: No such 
> file or directory

...

Are you sure you want to have all this text in the commit log?

You seem to have forgotten my tag:
Tested-by: Michael Walle  # on lschv2


Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries

2024-03-27 Thread Michael Walle
Hi,

On Wed Mar 27, 2024 at 8:01 AM CET, Neha Malcom Francis wrote:
> On 26/03/24 19:18, Michael Walle wrote:
> > On Fri Mar 22, 2024 at 2:10 PM CET, Neha Malcom Francis wrote:
> >> Clean up templatized boot binaries for all K3 boards. This includes
> >> modifying the k3-binman.dtsi to use SPL_BOARD_DTB, BOARD_DESCRIPTION and
> >> UBOOT_BOARD_DESCRIPTION from the files that include it to further reuse
> >> code.
> >>
> >> All k3--binman.dtsi will contain only templates. Only required boot
> >> binaries can be built from the templates in the boards' respective
> >> -u-boot.dtsi file (or k3--binman.dtsi if it exists). This allows
> >> clear distinction between the SoC common stuff vs. what is additionally
> >> needed to boot up a specific board.
> > 
> > I appreciate the cleanup. But as far as I can see, a board might
> > only have one device tree. How would that work if the uboot proper
> > must support multiple device trees?
> > 
>
>  From the discussions that took place in the mailing list [1] the consensus 
> seems to be to not focus on multiple devicetree support as it leads to 
> confusion 
> for downstream users.

What are users in this regard? I don't think you'd confuse
developers.

Anyway, I'm planning on upstreaming a TI board which will have
different memory configurations and different variants of the board.
And on top of that, it will just be a base board and there will
likely be some carrier device trees (overlay? I'm not sure yet).

As far as I can tell, you've put the memory configuration into the
device tree, so I'll probably need to switch between them somehow.
Also, regarding the board variants, I'll probably need to choose
between multiple device trees. That is invisible to the user,
because u-boot will choose the correct DTB according a board
strapping, which btw. works really fine, see for example
(boards/kontron/sl28/spl.c:board_fit_config_name_match).

I don't think it makes much sense to hardcode your generic
*-binman.dtsi to just one FIT configuration. I'd rather see a split
between generic things which are shared across all boards and board
specifics, like the FIT configuration. I mean I could just copy all
the binman and tiboot3.bin and tispl.bin magic and put it into my
own "-u-boot.dtsi". But I'm not sure that will make things any
better.

-michael


Re: [PATCH 3/4] arm: dts: k3-*-binman.dtsi: Clean up and templatize boot binaries

2024-03-26 Thread Michael Walle
Hi,

On Fri Mar 22, 2024 at 2:10 PM CET, Neha Malcom Francis wrote:
> Clean up templatized boot binaries for all K3 boards. This includes
> modifying the k3-binman.dtsi to use SPL_BOARD_DTB, BOARD_DESCRIPTION and
> UBOOT_BOARD_DESCRIPTION from the files that include it to further reuse
> code.
>
> All k3--binman.dtsi will contain only templates. Only required boot
> binaries can be built from the templates in the boards' respective
> -u-boot.dtsi file (or k3--binman.dtsi if it exists). This allows
> clear distinction between the SoC common stuff vs. what is additionally
> needed to boot up a specific board.

I appreciate the cleanup. But as far as I can see, a board might
only have one device tree. How would that work if the uboot proper
must support multiple device trees?

Thanks,
-michael


signature.asc
Description: PGP signature


[PATCH] tools: binman: ti_board_cfg: improve error message

2024-03-26 Thread Michael Walle
When there is a lint error the user gets the following cryptic message:

  binman: Node '/path/to/some/node': Yamllint error: 18: comments

This isn't very helpful. Improve the message to tell the user that the
number is actually a line number and also tell the user in which file
they have to look.

Signed-off-by: Michael Walle 
---
 tools/binman/etype/ti_board_config.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/binman/etype/ti_board_config.py 
b/tools/binman/etype/ti_board_config.py
index 2c3bb8f7b56..c10d66edcb1 100644
--- a/tools/binman/etype/ti_board_config.py
+++ b/tools/binman/etype/ti_board_config.py
@@ -248,7 +248,7 @@ class Entry_ti_board_config(Entry_section):
 
 yaml_config = config.YamlLintConfig("extends: default")
 for p in yamllint.linter.run(open(self._config_file, "r"), 
yaml_config):
-self.Raise(f"Yamllint error: {p.line}: {p.rule}")
+self.Raise(f"Yamllint error: Line {p.line} in 
{self._config_file}: {p.rule}")
 try:
 validate(self.file_yaml, self.schema_yaml)
 except Exception as e:
-- 
2.39.2



Re: [PATCH] arm: dts: kirkwood: Enable upstream DT on Kirkwood boards

2024-03-22 Thread Michael Walle
Hi Tony,

On Fri Mar 22, 2024 at 3:17 AM CET, Tony Dinh wrote:
> Enable OF_UPSTREAM to use upstream DT and add marvell/ prefix to the
> DEFAULT_DEVICE_TREE for Kirkwood boards. And so we can directly build
> DTBs from dts/upstream/src/arm/marvell, and including *-u-boot.dtsi
> files from arch/arm/dts/ directory.

Thanks for taking care.

> Signed-off-by: Tony Dinh 

Tested-by: Michael Walle  # on lschlv2

lsxl should work too as it is just different in memory and cpu
frequency settings.

> ---
>
>  arch/arm/dts/kirkwood-lschlv2-u-boot.dtsi   | 6 --
>  arch/arm/dts/kirkwood-lsxhl-u-boot.dtsi | 6 --

arch/arm/dts/kirkwood-lschlv2.dts
arch/arm/dts/kirkwood-lsxhl.dts

Should be then be removed.

-michael


Re: [PATCH v2 6/6] cmd: nand: Add new optional sub-command 'onfi'

2024-03-22 Thread Michael Nazzareno Trimarchi
HI

On Fri, Mar 22, 2024 at 1:02 PM Alexander Dahl  wrote:
>
> Hello Michael,
>
> Am Fri, Mar 22, 2024 at 12:54:27PM +0100 schrieb Michael Nazzareno Trimarchi:
> > HI
> >
> > On Fri, Mar 22, 2024 at 12:46 PM Alexander Dahl  wrote:
> > >
> > > Hello Mihai,
> > >
> > > Am Fri, Mar 22, 2024 at 10:02:29AM + schrieb mihai.s...@microchip.com:
> > > > Hi Michael,
> > > >
> > > > ---
> > > >
> > > > I think this command can be really useful.
> > > > Let try to have more testing on more boards
> > > >
> > > > -
> > > >
> > > > I managed to test the command on sama7g54-curiosity board.
> > >
> > > Thanks for that.  Nice to see it works on other variants of the SoC
> > > family.
> > >
> > > > I also forced timing mode 5 from controller driver 
> > > > (conf->timings.sdr.tRC_min < 2).
> > >
> > > You did a similar thing for the sam9x75.  These boards/socs seem to
> > > have a newer SMC / HSMC controller than sama5d2 or sam9x60?  The
> > > driver claims all the (H)SMC incarnations do _not_ support these EDO
> > > modes 4 and 5.  Maybe someone could have a deeper look at the
> > > datasheets of the newer SoCs and propose a patch to support those
> > > newer controllers in the atmel nand-controller driver?  I guess the
> > > problem is the same in Linux, right?
> > >
> > > Greets
> > > Alex
> > >
> > > >
> > > > => nand onfi 0
> > > > => hsmc decode
> > > >
> > > > MCK rate: 200 MHz
> > > >
> > > > HSMC_SETUP3:0x0004
> > > > HSMC_PULSE3:0x140a140a
> > > > HSMC_CYCLE3:0x00140014
> > > > HSMC_TIMINGS3:  0x880805f4
> > > > HSMC_MODE3: 0x001f0003
> > > > NCS_RD: setup: 0 (0 ns), pulse: 20 (100 ns), hold: 0 (0 ns), cycle: 20 
> > > > (100 ns)
> > > >NRD: setup: 0 (0 ns), pulse: 10 (50 ns), hold: 10 (50 ns), cycle: 20 
> > > > (100 ns)
> > > > NCS_WR: setup: 0 (0 ns), pulse: 20 (100 ns), hold: 0 (0 ns), cycle: 20 
> > > > (100 ns)
> > > >NWE: setup: 4 (20 ns), pulse: 10 (50 ns), hold: 6 (30 ns), cycle: 20 
> > > > (100 ns)
> > > > TDF optimization enabled
> > > > TDF cycles: 15 (75 ns)
> > > > Data Bus Width: 8-bit bus
> > > > NWAIT Mode: 0
> > > > Write operation controlled by NWE signal
> > > > Read operation controlled by NRD signal
> > > > NFSEL (NAND Flash Selection) is set
> > > > OCMS (Off Chip Memory Scrambling) is disabled
> > > > TWB (WEN High to REN to Busy): 64 (320 ns)
> > > > TRR (Ready to REN Low Delay):  64 (320 ns)
> > > > TAR (ALE to REN Low Delay):5 (25 ns)
> > > > TADL (ALE to Data Start):  71 (355 ns)
> > > > TCLR (CLE to REN Low Delay):   4 (20 ns)
> > > >
> > > > => time nand torture 0x100 0x100
> > > >
> > > > NAND torture: device 0 offset 0x100 size 0x100 (block size 
> > > > 0x4)
> > > >  Passed: 64, failed: 0
> > > >
> > > > time: 22.638 seconds
> > > >
> > > > => nand onfi 5
> > > > => hsmc decode
> > > >
> > > > MCK rate: 200 MHz
> > > >
> > > > HSMC_SETUP3:0x0001
> > > > HSMC_PULSE3:0x07040502
> > > > HSMC_CYCLE3:0x00070005
> > > > HSMC_TIMINGS3:  0x880402f2
> > > > HSMC_MODE3: 0x001f0003
> > > > NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 
> > > > ns)
> > > >NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 
> > > > (35 ns)
> > > > NCS_WR: setup: 0 (0 ns), pulse: 5 (25 ns), hold: 0 (0 ns), cycle: 5 (25 
> > > > ns)
> > > >NWE: setup: 1 (5 ns), pulse: 2 (10 ns), hold: 2 (10 ns), cycle: 5 
> > > > (25 ns)
> > > > TDF optimization enabled
> > > > TDF cycles: 15 (75 ns)
> > > > Data Bus Width: 8-bit bus
> > > > NWAIT Mode: 0
> > > > Write operation controlled by NWE signal
> > > > Read operation controlled by NRD signal
> > > > NFSEL (NAND Flash Selection) is set
> > > > OCMS (Off Chip Memory Scrambl

Re: [PATCH v2 6/6] cmd: nand: Add new optional sub-command 'onfi'

2024-03-22 Thread Michael Nazzareno Trimarchi
HI

On Fri, Mar 22, 2024 at 12:46 PM Alexander Dahl  wrote:
>
> Hello Mihai,
>
> Am Fri, Mar 22, 2024 at 10:02:29AM + schrieb mihai.s...@microchip.com:
> > Hi Michael,
> >
> > ---
> >
> > I think this command can be really useful.
> > Let try to have more testing on more boards
> >
> > -
> >
> > I managed to test the command on sama7g54-curiosity board.
>
> Thanks for that.  Nice to see it works on other variants of the SoC
> family.
>
> > I also forced timing mode 5 from controller driver 
> > (conf->timings.sdr.tRC_min < 2).
>
> You did a similar thing for the sam9x75.  These boards/socs seem to
> have a newer SMC / HSMC controller than sama5d2 or sam9x60?  The
> driver claims all the (H)SMC incarnations do _not_ support these EDO
> modes 4 and 5.  Maybe someone could have a deeper look at the
> datasheets of the newer SoCs and propose a patch to support those
> newer controllers in the atmel nand-controller driver?  I guess the
> problem is the same in Linux, right?
>
> Greets
> Alex
>
> >
> > => nand onfi 0
> > => hsmc decode
> >
> > MCK rate: 200 MHz
> >
> > HSMC_SETUP3:0x0004
> > HSMC_PULSE3:0x140a140a
> > HSMC_CYCLE3:0x00140014
> > HSMC_TIMINGS3:  0x880805f4
> > HSMC_MODE3: 0x001f0003
> > NCS_RD: setup: 0 (0 ns), pulse: 20 (100 ns), hold: 0 (0 ns), cycle: 20 (100 
> > ns)
> >NRD: setup: 0 (0 ns), pulse: 10 (50 ns), hold: 10 (50 ns), cycle: 20 
> > (100 ns)
> > NCS_WR: setup: 0 (0 ns), pulse: 20 (100 ns), hold: 0 (0 ns), cycle: 20 (100 
> > ns)
> >NWE: setup: 4 (20 ns), pulse: 10 (50 ns), hold: 6 (30 ns), cycle: 20 
> > (100 ns)
> > TDF optimization enabled
> > TDF cycles: 15 (75 ns)
> > Data Bus Width: 8-bit bus
> > NWAIT Mode: 0
> > Write operation controlled by NWE signal
> > Read operation controlled by NRD signal
> > NFSEL (NAND Flash Selection) is set
> > OCMS (Off Chip Memory Scrambling) is disabled
> > TWB (WEN High to REN to Busy): 64 (320 ns)
> > TRR (Ready to REN Low Delay):  64 (320 ns)
> > TAR (ALE to REN Low Delay):5 (25 ns)
> > TADL (ALE to Data Start):  71 (355 ns)
> > TCLR (CLE to REN Low Delay):   4 (20 ns)
> >
> > => time nand torture 0x100 0x100
> >
> > NAND torture: device 0 offset 0x100 size 0x100 (block size 0x4)
> >  Passed: 64, failed: 0
> >
> > time: 22.638 seconds
> >
> > => nand onfi 5
> > => hsmc decode
> >
> > MCK rate: 200 MHz
> >
> > HSMC_SETUP3:0x0001
> > HSMC_PULSE3:0x07040502
> > HSMC_CYCLE3:0x00070005
> > HSMC_TIMINGS3:  0x880402f2
> > HSMC_MODE3: 0x001f0003
> > NCS_RD: setup: 0 (0 ns), pulse: 7 (35 ns), hold: 0 (0 ns), cycle: 7 (35 ns)
> >NRD: setup: 0 (0 ns), pulse: 4 (20 ns), hold: 3 (15 ns), cycle: 7 (35 ns)
> > NCS_WR: setup: 0 (0 ns), pulse: 5 (25 ns), hold: 0 (0 ns), cycle: 5 (25 ns)
> >NWE: setup: 1 (5 ns), pulse: 2 (10 ns), hold: 2 (10 ns), cycle: 5 (25 ns)
> > TDF optimization enabled
> > TDF cycles: 15 (75 ns)
> > Data Bus Width: 8-bit bus
> > NWAIT Mode: 0
> > Write operation controlled by NWE signal
> > Read operation controlled by NRD signal
> > NFSEL (NAND Flash Selection) is set
> > OCMS (Off Chip Memory Scrambling) is disabled
> > TWB (WEN High to REN to Busy): 64 (320 ns)
> > TRR (Ready to REN Low Delay):  4 (20 ns)
> > TAR (ALE to REN Low Delay):2 (10 ns)
> > TADL (ALE to Data Start):  71 (355 ns)
> > TCLR (CLE to REN Low Delay):   2 (10 ns)
> >
> > => time nand torture 0x100 0x100
> >
> > NAND torture: device 0 offset 0x100 size 0x100 (block size 0x4)
> >  Passed: 64, failed: 0
> >
> > time: 11.661 seconds
> >
> > => nand info
> >
> > Device 0: nand0, sector size 256 KiB
> >   Manufacturer  MACRONIX
> >   Model MX30LF4G28AD
> >   Device size512 MiB
> >   Page size 4096 b
> >   OOB size   256 b
> >   Erase size  262144 b
> >   ecc strength 8 bits
> >   ecc step size  512 b
> >   subpagesize   4096 b
> >   options   0x40004200
> >   bbt options   0x00028000
> >
> > Best regards,
> > Mihai Sain

I'm in favor to have it even cover by one soc family. I would like to
confirm on imx6 and imx8. If you are not in a rush.
Let's us test too

Michael

-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH v4] cmd: mtd: OTP access support

2024-03-22 Thread Michael Nazzareno Trimarchi
Hi Arseniy

On Fri, Mar 22, 2024 at 9:14 AM Arseniy Krasnov
 wrote:
>
> Hi,
>
> On 22.03.2024 11:12, Michael Nazzareno Trimarchi wrote:
> > Hi Arseniy
> >
> > On Wed, Mar 20, 2024 at 8:14 PM Arseniy Krasnov
> >  wrote:
> >>
> >> Add access to OTP region. It supports info, dump, write and lock
> >> operations. Usage example:
> >>
> >> 'mtd otpread nand0 u 0 1024' - dump 1024 bytes of user area starting
> >>  from offset 0 of device 'nand0'.
> >>
> >> 'mtd otpwrite nand0 10 11223344' - write binary data 0x11, 0x22, 0x33,
> >>  0x44 to offset 10 to user area of device 'nand0'.
> >>
> >> 'mtd otplock nand0 0 1024' - lock 1024 bytes of user area starting
> >>  from offset 0 of device 'nand0'.
> >>
> >> 'mtd otpinfo nand0 f' - show info about factory area of device 'nand0'.
> >>
> >> Signed-off-by: Arseniy Krasnov 
> >> Reviewed-by: Michael Trimarchi 
> >> ---
> >>  Changelog:
> >>  v1 -> v2:
> >>   * Remove warning that OTP can't be erased after write.
> >>  v2 -> v3:
> >>   * Commit message updated by adding usage.
> >>   * R-b added.
> >>  v3 -> v4:
> >>   * Fix build failure due to invalid format strings for 'printf()'.
> >>   * Rebase over latest version of cmd/mtd.c.
> >>
> >>  cmd/Kconfig |   1 +
> >>  cmd/mtd.c   | 224 
> >>  2 files changed, 225 insertions(+)
> >>
> >> diff --git a/cmd/Kconfig b/cmd/Kconfig
> >> index 7292a150f5..f218dc6cf2 100644
> >> --- a/cmd/Kconfig
> >> +++ b/cmd/Kconfig
> >> @@ -1363,6 +1363,7 @@ config CMD_MTD
> >> bool "mtd"
> >> depends on MTD
> >> select MTD_PARTITIONS
> >> +   select HEXDUMP
> >
> > Make those change forced in increase the size of uboot and break this build
> >
> > https://source.denx.de/u-boot/custodians/u-boot-nand-flash/-/jobs/802378
> >
> > I think that those command could be optional and not forced to be
> > included by default
> >
>
> Ah, Ok:) I'll add it as config option which depends on CMD_MTD in the next 
> version
>

I am really glad how you interact with the mailing list ;) and how you
are fast on feedback


Michael

> Thanks
>
> > Michael
> >
> >
> >> help
> >>   MTD commands support.
> >>
> >> diff --git a/cmd/mtd.c b/cmd/mtd.c
> >> index e63c011e79..2b03a85ef0 100644
> >> --- a/cmd/mtd.c
> >> +++ b/cmd/mtd.c
> >> @@ -11,6 +11,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -202,6 +203,219 @@ static bool mtd_oob_write_is_empty(struct 
> >> mtd_oob_ops *op)
> >> return true;
> >>  }
> >>
> >> +static int do_mtd_otp_read(struct cmd_tbl *cmdtp, int flag, int argc,
> >> +  char *const argv[])
> >> +{
> >> +   struct mtd_info *mtd;
> >> +   size_t retlen;
> >> +   off_t from;
> >> +   size_t len;
> >> +   bool user;
> >> +   int ret;
> >> +   u8 *buf;
> >> +
> >> +   if (argc != 5)
> >> +   return CMD_RET_USAGE;
> >> +
> >> +   if (!strcmp(argv[2], "u"))
> >> +   user = true;
> >> +   else if (!strcmp(argv[2], "f"))
> >> +   user = false;
> >> +   else
> >> +   return CMD_RET_USAGE;
> >> +
> >> +   mtd = get_mtd_by_name(argv[1]);
> >> +   if (IS_ERR_OR_NULL(mtd))
> >> +   return CMD_RET_FAILURE;
> >> +
> >> +   from = simple_strtoul(argv[3], NULL, 0);
> >> +   len = simple_strtoul(argv[4], NULL, 0);
> >> +
> >> +   ret = CMD_RET_FAILURE;
> >> +
> >> +   buf = malloc(len);
> >> +   if (!buf)
> >> +   goto put_mtd;
> >> +
> >> +   printf("Reading %s OTP from 0x%lx, %zu bytes\n",
> >> +  user ? "user" : "factory", from, len);
> >> +
> >> +   if (user)
> >> +   ret = mtd_read_user_prot_reg(mtd, from, len, , buf);
> >> +   else
> >> +   ret = mtd_read_fact_prot_reg(mtd, 

Re: [PATCH v4] cmd: mtd: OTP access support

2024-03-22 Thread Michael Nazzareno Trimarchi
Hi Arseniy

On Wed, Mar 20, 2024 at 8:14 PM Arseniy Krasnov
 wrote:
>
> Add access to OTP region. It supports info, dump, write and lock
> operations. Usage example:
>
> 'mtd otpread nand0 u 0 1024' - dump 1024 bytes of user area starting
>  from offset 0 of device 'nand0'.
>
> 'mtd otpwrite nand0 10 11223344' - write binary data 0x11, 0x22, 0x33,
>  0x44 to offset 10 to user area of device 'nand0'.
>
> 'mtd otplock nand0 0 1024' - lock 1024 bytes of user area starting
>  from offset 0 of device 'nand0'.
>
> 'mtd otpinfo nand0 f' - show info about factory area of device 'nand0'.
>
> Signed-off-by: Arseniy Krasnov 
> Reviewed-by: Michael Trimarchi 
> ---
>  Changelog:
>  v1 -> v2:
>   * Remove warning that OTP can't be erased after write.
>  v2 -> v3:
>   * Commit message updated by adding usage.
>   * R-b added.
>  v3 -> v4:
>   * Fix build failure due to invalid format strings for 'printf()'.
>   * Rebase over latest version of cmd/mtd.c.
>
>  cmd/Kconfig |   1 +
>  cmd/mtd.c   | 224 
>  2 files changed, 225 insertions(+)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 7292a150f5..f218dc6cf2 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1363,6 +1363,7 @@ config CMD_MTD
> bool "mtd"
> depends on MTD
> select MTD_PARTITIONS
> +   select HEXDUMP

Make those change forced in increase the size of uboot and break this build

https://source.denx.de/u-boot/custodians/u-boot-nand-flash/-/jobs/802378

I think that those command could be optional and not forced to be
included by default

Michael


> help
>   MTD commands support.
>
> diff --git a/cmd/mtd.c b/cmd/mtd.c
> index e63c011e79..2b03a85ef0 100644
> --- a/cmd/mtd.c
> +++ b/cmd/mtd.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -202,6 +203,219 @@ static bool mtd_oob_write_is_empty(struct mtd_oob_ops 
> *op)
> return true;
>  }
>
> +static int do_mtd_otp_read(struct cmd_tbl *cmdtp, int flag, int argc,
> +  char *const argv[])
> +{
> +   struct mtd_info *mtd;
> +   size_t retlen;
> +   off_t from;
> +   size_t len;
> +   bool user;
> +   int ret;
> +   u8 *buf;
> +
> +   if (argc != 5)
> +   return CMD_RET_USAGE;
> +
> +   if (!strcmp(argv[2], "u"))
> +   user = true;
> +   else if (!strcmp(argv[2], "f"))
> +   user = false;
> +   else
> +   return CMD_RET_USAGE;
> +
> +   mtd = get_mtd_by_name(argv[1]);
> +   if (IS_ERR_OR_NULL(mtd))
> +   return CMD_RET_FAILURE;
> +
> +   from = simple_strtoul(argv[3], NULL, 0);
> +   len = simple_strtoul(argv[4], NULL, 0);
> +
> +   ret = CMD_RET_FAILURE;
> +
> +   buf = malloc(len);
> +   if (!buf)
> +   goto put_mtd;
> +
> +   printf("Reading %s OTP from 0x%lx, %zu bytes\n",
> +  user ? "user" : "factory", from, len);
> +
> +   if (user)
> +   ret = mtd_read_user_prot_reg(mtd, from, len, , buf);
> +   else
> +   ret = mtd_read_fact_prot_reg(mtd, from, len, , buf);
> +   if (ret) {
> +   free(buf);
> +   pr_err("OTP read failed: %d\n", ret);
> +   ret = CMD_RET_FAILURE;
> +   goto put_mtd;
> +   }
> +
> +   if (retlen != len)
> +   pr_err("OTP read returns %zu, but %zu expected\n",
> +  retlen, len);
> +
> +   print_hex_dump("", 0, 16, 1, buf, retlen, true);
> +
> +   free(buf);
> +
> +   ret = CMD_RET_SUCCESS;
> +
> +put_mtd:
> +   put_mtd_device(mtd);
> +
> +   return ret;
> +}
> +
> +static int do_mtd_otp_lock(struct cmd_tbl *cmdtp, int flag, int argc,
> +  char *const argv[])
> +{
> +   struct mtd_info *mtd;
> +   off_t from;
> +   size_t len;
> +   int ret;
> +
> +   if (argc != 4)
> +   return CMD_RET_USAGE;
> +
> +   mtd = get_mtd_by_name(argv[1]);
> +   if (IS_ERR_OR_NULL(mtd))
> +   return CMD_RET_FAILURE;
> +
> +   from = simple_strtoul(argv[2], NULL, 0);
> +   len = simple_strtoul(argv[3], NULL, 0);
> +
> +   ret = mtd_lock_user_prot_reg(mtd, from, len);
> +   if (ret) {
> +   pr_err("OTP lock failed: %d\n", ret);
> +   ret = CMD_RET_FAILURE

Re: [PATCH v2 6/6] cmd: nand: Add new optional sub-command 'onfi'

2024-03-21 Thread Michael Nazzareno Trimarchi
Hi

I think this command can be really useful.

On Wed, Mar 20, 2024 at 3:09 PM  wrote:
>
> Hi Alex,
>
> 
>
> Override the ONFI timing mode at runtime.
>
> Signed-off-by: Alexander Dahl 
> ---
>
> I used the same board sam9x75-curiosity to test mode 5 
>
> I forced in nfc driver the mode 5:
> +   if (conf->timings.sdr.tRC_min < 2)
>
> And I ran the nand torture on 16 MiB:
>
> => nand onfi 0
> => hsmc decode
>
> MCK rate: 270 MHz
>
> SMC_SETUP2: 0x0007
> SMC_PULSE2: 0x22112211
> SMC_CYCLE2: 0x00220022
> SMC_MODE2:  0x001f0003
> NCS_RD: setup: 0 (0 ns), pulse: 34 (102 ns), hold: 0 (0 ns), cycle: 34 (102 
> ns)
>NRD: setup: 0 (0 ns), pulse: 17 (51 ns), hold: 17 (51 ns), cycle: 34 (102 
> ns)
> NCS_WR: setup: 0 (0 ns), pulse: 34 (102 ns), hold: 0 (0 ns), cycle: 34 (102 
> ns)
>NWE: setup: 7 (21 ns), pulse: 17 (51 ns), hold: 10 (30 ns), cycle: 34 (102 
> ns)
> Standard read applied
> TDF optimization enabled
> TDF cycles: 15 (45 ns)
> Data Bus Width: 8-bit bus
> NWAIT Mode: 0
> Write operation controlled by NWE signal
> Read operation controlled by NRD signal
>
> => time nand torture 0x80 0x100
>
> NAND torture: device 0 offset 0x80 size 0x100 (block size 0x4)
>  Passed: 64, failed: 0
>
> time: 30.152 seconds
>
> => nand onfi 5
> => hsmc decode
>
> MCK rate: 270 MHz
>
> SMC_SETUP2: 0x0001
> SMC_PULSE2: 0x0b060804
> SMC_CYCLE2: 0x000b0008
> SMC_MODE2:  0x001f0003
> NCS_RD: setup: 0 (0 ns), pulse: 11 (33 ns), hold: 0 (0 ns), cycle: 11 (33 ns)
>NRD: setup: 0 (0 ns), pulse: 6 (18 ns), hold: 5 (15 ns), cycle: 11 (33 ns)
> NCS_WR: setup: 0 (0 ns), pulse: 8 (24 ns), hold: 0 (0 ns), cycle: 8 (24 ns)
>NWE: setup: 1 (3 ns), pulse: 4 (12 ns), hold: 3 (9 ns), cycle: 8 (24 ns)
> Standard read applied
> TDF optimization enabled
> TDF cycles: 15 (45 ns)
> Data Bus Width: 8-bit bus
> NWAIT Mode: 0
> Write operation controlled by NWE signal
> Read operation controlled by NRD signal
>
> => time nand torture 0x80 0x100
>
> NAND torture: device 0 offset 0x80 size 0x100 (block size 0x4)
>  Passed: 64, failed: 0
>
> time: 15.891 seconds
>

Let try to have more testing on more boards

Michael

> Best regards,
> Mihai Sain



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH 1/1] fs: ext4: all file paths are absolute

2024-03-20 Thread Michael Nazzareno Trimarchi
On Wed, Mar 20, 2024 at 2:25 PM Heinrich Schuchardt
 wrote:
>
> U-Boot only knows absolute file paths. It is inconsistent to require that
> saving to an ext4 file system should use a leading '/' wile reading does
> not. Remove the superfluous check.
>

Just a typo, "wile reading"

Michael

> Reported-by: Patrice Chotard 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  fs/ext4/ext4_common.c | 5 -
>  1 file changed, 5 deletions(-)
>
> diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
> index ea9b92298ba..9eac6beef3b 100644
> --- a/fs/ext4/ext4_common.c
> +++ b/fs/ext4/ext4_common.c
> @@ -765,11 +765,6 @@ int ext4fs_get_parent_inode_num(const char *dirname, 
> char *dname, int flags)
> struct ext2_inode *first_inode = NULL;
> struct ext2_inode temp_inode;
>
> -   if (*dirname != '/') {
> -   printf("Please supply Absolute path\n");
> -   return -1;
> -   }
> -
> /* TODO: input validation make equivalent to linux */
> depth_dirname = zalloc(strlen(dirname) + 1);
> if (!depth_dirname)
> --
> 2.43.0
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH v3] cmd: mtd: OTP access support

2024-03-20 Thread Michael Nazzareno Trimarchi
Hi

On Wed, Mar 13, 2024 at 8:27 AM Arseniy Krasnov
 wrote:
>
> Add access to OTP region. It supports info, dump, write and lock
> operations. Usage example:
>
> 'mtd otpread nand0 u 0 1024' - dump 1024 bytes of user area starting
>  from offset 0 of device 'nand0'.
>
> 'mtd otpwrite nand0 10 11223344' - write binary data 0x11, 0x22, 0x33,
>  0x44 to offset 10 to user area of device 'nand0'.
>
> 'mtd otplock nand0 0 1024' - lock 1024 bytes of user area starting
>  from offset 0 of device 'nand0'.
>
> 'mtd otpinfo nand0 f' - show info about factory area of device 'nand0'.
>
> Signed-off-by: Arseniy Krasnov 
> Reviewed-by: Michael Trimarchi 
> ---
>  Changelog:
>  v1 -> v2:
>   * Remove warning that OTP can't be erased after write.
>  v2 -> v3:
>   * Commit message updated by adding usage.
>   * R-b added.
>

Your patch build fail

https://source.denx.de/u-boot/custodians/u-boot-nand-flash/-/jobs/801919

Michael

>  cmd/Kconfig |   1 +
>  cmd/mtd.c   | 224 
>  2 files changed, 225 insertions(+)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 90e4ef93e0..c47523a03b 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -1354,6 +1354,7 @@ config CMD_MTD
> bool "mtd"
> depends on MTD
> select MTD_PARTITIONS
> +   select HEXDUMP
> help
>   MTD commands support.
>
> diff --git a/cmd/mtd.c b/cmd/mtd.c
> index eb6e2d6892..1ab69b108b 100644
> --- a/cmd/mtd.c
> +++ b/cmd/mtd.c
> @@ -11,6 +11,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -202,6 +203,219 @@ static bool mtd_oob_write_is_empty(struct mtd_oob_ops 
> *op)
> return true;
>  }
>
> +static int do_mtd_otp_read(struct cmd_tbl *cmdtp, int flag, int argc,
> +  char *const argv[])
> +{
> +   struct mtd_info *mtd;
> +   size_t retlen;
> +   off_t from;
> +   size_t len;
> +   bool user;
> +   int ret;
> +   u8 *buf;
> +
> +   if (argc != 5)
> +   return CMD_RET_USAGE;
> +
> +   if (!strcmp(argv[2], "u"))
> +   user = true;
> +   else if (!strcmp(argv[2], "f"))
> +   user = false;
> +   else
> +   return CMD_RET_USAGE;
> +
> +   mtd = get_mtd_by_name(argv[1]);
> +   if (IS_ERR_OR_NULL(mtd))
> +   return CMD_RET_FAILURE;
> +
> +   from = simple_strtoul(argv[3], NULL, 0);
> +   len = simple_strtoul(argv[4], NULL, 0);
> +
> +   ret = CMD_RET_FAILURE;
> +
> +   buf = malloc(len);
> +   if (!buf)
> +   goto put_mtd;
> +
> +   printf("Reading %s OTP from 0x%lx, %lu bytes\n",
> +  user ? "user" : "factory", from, len);
> +
> +   if (user)
> +   ret = mtd_read_user_prot_reg(mtd, from, len, , buf);
> +   else
> +   ret = mtd_read_fact_prot_reg(mtd, from, len, , buf);
> +   if (ret) {
> +   free(buf);
> +   pr_err("OTP read failed: %d\n", ret);
> +   ret = CMD_RET_FAILURE;
> +   goto put_mtd;
> +   }
> +
> +   if (retlen != len)
> +   pr_err("OTP read returns %zu, but %zu expected\n",
> +  retlen, len);
> +
> +   print_hex_dump("", 0, 16, 1, buf, retlen, true);
> +
> +   free(buf);
> +
> +   ret = CMD_RET_SUCCESS;
> +
> +put_mtd:
> +   put_mtd_device(mtd);
> +
> +   return ret;
> +}
> +
> +static int do_mtd_otp_lock(struct cmd_tbl *cmdtp, int flag, int argc,
> +  char *const argv[])
> +{
> +   struct mtd_info *mtd;
> +   off_t from;
> +   size_t len;
> +   int ret;
> +
> +   if (argc != 4)
> +   return CMD_RET_USAGE;
> +
> +   mtd = get_mtd_by_name(argv[1]);
> +   if (IS_ERR_OR_NULL(mtd))
> +   return CMD_RET_FAILURE;
> +
> +   from = simple_strtoul(argv[2], NULL, 0);
> +   len = simple_strtoul(argv[3], NULL, 0);
> +
> +   ret = mtd_lock_user_prot_reg(mtd, from, len);
> +   if (ret) {
> +   pr_err("OTP lock failed: %d\n", ret);
> +   ret = CMD_RET_FAILURE;
> +   goto put_mtd;
> +   }
> +
> +   ret = CMD_RET_SUCCESS;
> +
> +put_mtd:
> +   put_mtd_device(mtd);
> +
> +   return ret;
> +}
> +
> +static int do_mtd_otp_write(struct cmd_tbl *cmdtp, int flag, int argc,
> +

Re: [PATCH v2 5/6] mtd: nand: raw: atmel: Fix comment in timings preparation

2024-03-20 Thread Michael Nazzareno Trimarchi
On Wed, Mar 20, 2024 at 10:02 AM Alexander Dahl  wrote:
>
> Introduced with commit 6a8dfd57220d ("nand: atmel: Add DM based NAND
> driver") when driver was initially ported from Linux.  The context
> around this and especially the code itself suggests 'read' is meant
> instead of write.
>
> The fix is the same as accepted in Linux already with mainline Linux
> kernel commit 1c60e027ffde ("mtd: nand: raw: atmel: Fix comment in
> timings preparation").
>
> Link: 
> https://lore.kernel.org/linux-mtd/20240307172835.3453880-1-miquel.ray...@bootlin.com/T/#t
> Signed-off-by: Alexander Dahl 
> ---
>
> Notes:
> v2:
> - initial patch version (not present in v1)
>
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c 
> b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index 75da15c157b..bbafc88e44c 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1271,7 +1271,7 @@ static int atmel_smc_nand_prepare_smcconf(struct 
> atmel_nand *nand,
> return ret;
>
> /*
> -* The write cycle timing is directly matching tWC, but is also
> +* The read cycle timing is directly matching tRC, but is also
>  * dependent on the setup and hold timings we calculated earlier,
>  * which gives:
>  *
> --

Reviewed-by: Michael Trimarchi 

> 2.39.2
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH v2] cmd: mtd: OTP access support

2024-03-13 Thread Michael Nazzareno Trimarchi
Hi

On Wed, Mar 13, 2024 at 7:43 AM Arseniy Krasnov
 wrote:
>
> Sorry, please ping
>
> Thanks, Arseniy
>
>
> On 11.02.2024 02:16, Arseniy Krasnov wrote:
> > Sorry, pls ping
> >
> > Thanks, Arseniy
> >
> > On 08.01.2024 21:33, Arseniy Krasnov wrote:
> >> Sorry, pls ping
> >>
> >> Thanks, Arseniy
> >>
> >> On 20.12.2023 22:36, Arseniy Krasnov wrote:
> >>> Add access to OTP region. It supports info, dump, write and lock
> >>> operations.
> >>>
> >>> Signed-off-by: Arseniy Krasnov 
> >>> ---

Please extend the commit message with some example of the usage otherwise

Reviewed-by: Michael Trimarchi 

> >>>  Changelog:
> >>>  v1 -> v2:
> >>>   * Remove warning that OTP can't be erased after write.
> >>>
> >>>  cmd/Kconfig |   1 +
> >>>  cmd/mtd.c   | 224 
> >>>  2 files changed, 225 insertions(+)
> >>>
> >>> diff --git a/cmd/Kconfig b/cmd/Kconfig
> >>> index 90e4ef93e0..c47523a03b 100644
> >>> --- a/cmd/Kconfig
> >>> +++ b/cmd/Kconfig
> >>> @@ -1354,6 +1354,7 @@ config CMD_MTD
> >>> bool "mtd"
> >>> depends on MTD
> >>> select MTD_PARTITIONS
> >>> +   select HEXDUMP
> >>> help
> >>>   MTD commands support.
> >>>
> >>> diff --git a/cmd/mtd.c b/cmd/mtd.c
> >>> index eb6e2d6892..1ab69b108b 100644
> >>> --- a/cmd/mtd.c
> >>> +++ b/cmd/mtd.c
> >>> @@ -11,6 +11,7 @@
> >>>  #include 
> >>>  #include 
> >>>  #include 
> >>> +#include 
> >>>  #include 
> >>>  #include 
> >>>  #include 
> >>> @@ -202,6 +203,219 @@ static bool mtd_oob_write_is_empty(struct 
> >>> mtd_oob_ops *op)
> >>> return true;
> >>>  }
> >>>
> >>> +static int do_mtd_otp_read(struct cmd_tbl *cmdtp, int flag, int argc,
> >>> +  char *const argv[])
> >>> +{
> >>> +   struct mtd_info *mtd;
> >>> +   size_t retlen;
> >>> +   off_t from;
> >>> +   size_t len;
> >>> +   bool user;
> >>> +   int ret;
> >>> +   u8 *buf;
> >>> +
> >>> +   if (argc != 5)
> >>> +   return CMD_RET_USAGE;
> >>> +
> >>> +   if (!strcmp(argv[2], "u"))
> >>> +   user = true;
> >>> +   else if (!strcmp(argv[2], "f"))
> >>> +   user = false;
> >>> +   else
> >>> +   return CMD_RET_USAGE;
> >>> +
> >>> +   mtd = get_mtd_by_name(argv[1]);
> >>> +   if (IS_ERR_OR_NULL(mtd))
> >>> +   return CMD_RET_FAILURE;
> >>> +
> >>> +   from = simple_strtoul(argv[3], NULL, 0);
> >>> +   len = simple_strtoul(argv[4], NULL, 0);
> >>> +
> >>> +   ret = CMD_RET_FAILURE;
> >>> +
> >>> +   buf = malloc(len);
> >>> +   if (!buf)
> >>> +   goto put_mtd;
> >>> +
> >>> +   printf("Reading %s OTP from 0x%lx, %lu bytes\n",
> >>> +  user ? "user" : "factory", from, len);
> >>> +
> >>> +   if (user)
> >>> +   ret = mtd_read_user_prot_reg(mtd, from, len, , buf);
> >>> +   else
> >>> +   ret = mtd_read_fact_prot_reg(mtd, from, len, , buf);
> >>> +   if (ret) {
> >>> +   free(buf);
> >>> +   pr_err("OTP read failed: %d\n", ret);
> >>> +   ret = CMD_RET_FAILURE;
> >>> +   goto put_mtd;
> >>> +   }
> >>> +
> >>> +   if (retlen != len)
> >>> +   pr_err("OTP read returns %zu, but %zu expected\n",
> >>> +  retlen, len);
> >>> +
> >>> +   print_hex_dump("", 0, 16, 1, buf, retlen, true);
> >>> +
> >>> +   free(buf);
> >>> +
> >>> +   ret = CMD_RET_SUCCESS;
> >>> +
> >>> +put_mtd:
> >>> +   put_mtd_device(mtd);
> >>> +
> >>> +   return ret;
> >>> +}
> >>> +
> >>> +static int do_mtd_otp_lock(struct cmd_tbl *cmdtp, int flag, int argc,
> >>

Re: [PATCH v1] mtd: rawnand: macronix: OTP access for MX30LFxG18AC

2024-03-13 Thread Michael Nazzareno Trimarchi
Hi  Dario

Can apply this series and put in CI?

Michael

On Wed, Mar 13, 2024 at 7:43 AM Arseniy Krasnov
 wrote:
>
> Sorry, please ping
>
> Thanks, Arseniy
>
> On 11.02.2024 02:16, Arseniy Krasnov wrote:
> > Sorry, pls ping
> >
> > Thanks, Arseniy
> >
> > On 08.01.2024 21:33, Arseniy Krasnov wrote:
> >> Sorry, pls ping
> >>
> >> Thanks, Arseniy



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH] mtd: nand: raw: mt7621-nand: allow writing ecc region in raw mode

2024-03-13 Thread Michael Nazzareno Trimarchi
Hi

On Wed, Mar 13, 2024 at 4:38 AM Weijie Gao  wrote:
>
> Allow writing ecc parity region in raw mode. This makes sure the
> nand write.raw command can write the flash data as-is.
>
> Change-Id: Ibed3bdf13c9cf81e54041c5ac7a78192b97dcedc
> Signed-off-by: Weijie Gao 
> CR-Id: WCNCR00180092

I think this is for internal tracking

> ---
>  drivers/mtd/nand/raw/mt7621_nand.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/mt7621_nand.c 
> b/drivers/mtd/nand/raw/mt7621_nand.c
> index f6eddb84a9..341ef0bf2d 100644
> --- a/drivers/mtd/nand/raw/mt7621_nand.c
> +++ b/drivers/mtd/nand/raw/mt7621_nand.c
> @@ -1003,9 +1003,9 @@ static int mt7621_nfc_write_page_raw(struct mtd_info 
> *mtd,
> mt7621_nfc_write_data(nfc, oob_fdm_ptr(nand, i),
>   NFI_FDM_SIZE);
>
> -   /* Write dummy ECC parity data */
> -   mt7621_nfc_write_data_empty(nfc, nfc->spare_per_sector -
> -   NFI_FDM_SIZE);
> +   /* Write ECC parity data */
> +   mt7621_nfc_write_data(nfc, oob_ecc_ptr(nfc, i),
> + nfc->spare_per_sector - NFI_FDM_SIZE);
> }
>

Please describe better what regression you are trying to fix in the
commit message
Reviewed-by: Michael Trimarchi 

Michael

> mt7621_nfc_wait_write_completion(nfc, nand);
> --
> 2.34.1
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH 3/4] mtd: nand: raw: Fix (most) Kconfig indentation

2024-03-07 Thread Michael Nazzareno Trimarchi
 SPL_SYS_NAND_SELF_INIT
> imply CMD_NAND
> -   ---help---
> -   Enable support for NAND. This option enables the standard and
> -   SPL drivers.
> -   The SPL driver only supports reading from the NAND using DMA
> -   transfers.
> +   help
> + Enable support for NAND. This option enables the standard and
> + SPL drivers.
> + The SPL driver only supports reading from the NAND using DMA
> + transfers.
>
>  if NAND_SUNXI
>
> @@ -577,16 +577,16 @@ config NAND_OCTEONTX
> select SYS_NAND_SELF_INIT
> imply CMD_NAND
> help
> -This enables Nand flash controller hardware found on the OcteonTX
> -processors.
> + This enables Nand flash controller hardware found on the OcteonTX
> + processors.
>
>  config NAND_OCTEONTX_HW_ECC
> bool "Support Hardware ECC for OcteonTX NAND controller"
> depends on NAND_OCTEONTX
> default y
> help
> -This enables Hardware BCH engine found on the OcteonTX processors to
> -support ECC for NAND flash controller.
> + This enables Hardware BCH engine found on the OcteonTX processors to
> + support ECC for NAND flash controller.
>
>  config NAND_STM32_FMC2
> bool "Support for NAND controller on STM32MP SoCs"
> @@ -751,37 +751,37 @@ config SYS_NAND_BAD_BLOCK_POS
>  config SYS_NAND_U_BOOT_LOCATIONS
> bool "Define U-Boot binaries locations in NAND"
> help
> -   Enable CONFIG_SYS_NAND_U_BOOT_OFFS though Kconfig.
> -   This option should not be enabled when compiling U-Boot for boards
> -   defining CONFIG_SYS_NAND_U_BOOT_OFFS in their 
> include/configs/.h
> -   file.
> + Enable CONFIG_SYS_NAND_U_BOOT_OFFS though Kconfig.
> + This option should not be enabled when compiling U-Boot for boards
> + defining CONFIG_SYS_NAND_U_BOOT_OFFS in their 
> include/configs/.h
> + file.
>
>  config SYS_NAND_U_BOOT_OFFS
> hex "Location in NAND to read U-Boot from"
> default 0x80 if NAND_SUNXI
> depends on SYS_NAND_U_BOOT_LOCATIONS
> help
> -   Set the offset from the start of the nand where u-boot should be
> -   loaded from.
> + Set the offset from the start of the nand where u-boot should be
> + loaded from.
>
>  config SYS_NAND_U_BOOT_OFFS_REDUND
> hex "Location in NAND to read U-Boot from"
> default SYS_NAND_U_BOOT_OFFS
> depends on SYS_NAND_U_BOOT_LOCATIONS
> help
> -   Set the offset from the start of the nand where the redundant u-boot
> -   should be loaded from.
> + Set the offset from the start of the nand where the redundant u-boot
> + should be loaded from.
>
>  config SPL_NAND_AM33XX_BCH
> bool "Enables SPL-NAND driver which supports ELM based"
> depends on SPL_NAND_SUPPORT && NAND_OMAP_GPMC && !OMAP34XX
> default y
> -help
> +   help
>   Hardware ECC correction. This is useful for platforms which have ELM
>   hardware engine and use NAND boot mode.
>   Some legacy platforms like OMAP3xx do not have in-built ELM h/w 
> engine,
>   so those platforms should use CONFIG_SPL_NAND_SIMPLE for enabling
> -  SPL-NAND driver with software ECC correction support.
> + SPL-NAND driver with software ECC correction support.
>
>  config SPL_NAND_DENALI
> bool "Support Denali NAND controller for SPL"
> @@ -810,6 +810,6 @@ config SYS_NAND_HW_ECC_OOBFIRST
> bool "In SPL, read the OOB first and then the data from NAND"
> depends on SPL_NAND_SIMPLE
>
> -endif
> +endif  # if SPL
>
> -endif   # if NAND
> +endif  # if MTD_RAW_NAND
> --
> 2.39.2
>
Reviewed-by: Michael Trimarchi 

-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH 2/4] mtd: nand: raw: Port another option flag from Linux

2024-03-07 Thread Michael Nazzareno Trimarchi
Hi

On Thu, Mar 7, 2024 at 10:10 AM Alexander Dahl  wrote:
>
> Introduced in upstream Linux with commit 7a08dbaedd365 for release v5.0.
>
> When the new atmel nand driver was backported to U-Boot with commit
> 6a8dfd57220d ("nand: atmel: Add DM based NAND driver") that definition
> was added to the driver instead of the header file.  Move it over to the
> other definitions with the same help text it has in Linux.
>
> Code actually using this has not been ported over to raw nand base yet.
>
> Signed-off-by: Alexander Dahl 
> ---
>  drivers/mtd/nand/raw/atmel/nand-controller.c | 2 --
>  include/linux/mtd/rawnand.h  | 7 +++
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c 
> b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index 0e0441472b8..e06523f3298 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -1429,8 +1429,6 @@ static int atmel_nand_setup_data_interface(struct 
> mtd_info *mtd, int csline,
> return nc->caps->ops->setup_data_interface(nand, csline, conf);
>  }
>
> -#define NAND_KEEP_TIMINGS   0x0080
> -
>  static void atmel_nand_init(struct atmel_nand_controller *nc,
> struct atmel_nand *nand)
>  {
> diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> index fb002ae6411..4abaf4734cf 100644
> --- a/include/linux/mtd/rawnand.h
> +++ b/include/linux/mtd/rawnand.h
> @@ -249,6 +249,13 @@ enum nand_ecc_algo {
>   */
>  #define NAND_USE_BOUNCE_BUFFER 0x0010
>
> +/*
> + * Do not try to tweak the timings at runtime. This is needed when the
> + * controller initializes the timings on itself or when it relies on
> + * configuration done by the bootloader.
> + */
> +#define NAND_KEEP_TIMINGS  0x0080
> +
>  /* Options set by nand scan */
>  /* bbt has already been read */
>  #define NAND_BBT_SCANNED   0x4000

Reviewed-by: Michael Trimarchi 

> --
> 2.39.2
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH 1/4] mtd: nand: raw: Use macro nand_to_mtd() where appropriate

2024-03-07 Thread Michael Nazzareno Trimarchi
On Thu, Mar 7, 2024 at 10:10 AM Alexander Dahl  wrote:
>
> In every other place in this file the macro is used, make it consistent.
>
> Fixes: 9d1806fadc24 ("mtd: nand: Get rid of mtd variable in function calls")
> Signed-off-by: Alexander Dahl 
> ---
>  drivers/mtd/nand/raw/nand_base.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c 
> b/drivers/mtd/nand/raw/nand_base.c
> index c40a0f23d7b..688d17ba3c2 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -4118,7 +4118,7 @@ static int nand_get_bits_per_cell(u8 cellinfo)
>   */
>  void nand_decode_ext_id(struct nand_chip *chip)
>  {
> -   struct mtd_info *mtd = >mtd;
> +   struct mtd_info *mtd = nand_to_mtd(chip);
> int extid;
> /* The 3rd id byte holds MLC / multichip data */
> chip->bits_per_cell = nand_get_bits_per_cell(chip->id.data[2]);
> @@ -4185,7 +4185,7 @@ static int nand_manufacturer_init(struct nand_chip 
> *chip)
>   */
>  static void nand_decode_id(struct nand_chip *chip, struct nand_flash_dev 
> *type)
>  {
> -   struct mtd_info *mtd = >mtd;
> +   struct mtd_info *mtd = nand_to_mtd(chip);
>
> mtd->erasesize = type->erasesize;
> mtd->writesize = type->pagesize;
> @@ -4265,7 +4265,7 @@ static const struct nand_manufacturer 
> *nand_get_manufacturer_desc(u8 id)
>  int nand_detect(struct nand_chip *chip, int *maf_id,
> int *dev_id, struct nand_flash_dev *type)
>  {
> -   struct mtd_info *mtd = >mtd;
> +   struct mtd_info *mtd = nand_to_mtd(chip);
> const struct nand_manufacturer *manufacturer_desc;
> int busw, ret;
> u8 *id_data = chip->id.data;
> --
> 2.39.2
>

Reviewed-By: Michael Trimarchi 

-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


[PATCH] board: sl28: move to OF_UPSTREAM

2024-03-06 Thread Michael Walle
Use the new device devicetree files in dts/upstream/ and delete the old
ones. Still keep the -u-boot.dtsi with all u-boot specifics around.

There is one catch and that is fsl-ls1028a-kontron-sl28-var3.dts which
is not available upstream (yet!). For now, the base dts is used for this
variant as this only differ in the compatible and the (human readable)
model name.

Signed-off-by: Michael Walle 
---
I'll send a patch to linux to add the var3.dts, then I'll add the
correct var3 dts again.
---
 .../arm/dts/fsl-ls1028a-kontron-sl28-var1.dts |  59 
 .../arm/dts/fsl-ls1028a-kontron-sl28-var2.dts |  65 
 .../arm/dts/fsl-ls1028a-kontron-sl28-var3.dts |  15 -
 .../arm/dts/fsl-ls1028a-kontron-sl28-var4.dts |  47 ---
 arch/arm/dts/fsl-ls1028a-kontron-sl28.dts | 308 --
 board/kontron/sl28/spl.c  |  11 +-
 configs/kontron_sl28_defconfig|   5 +-
 7 files changed, 8 insertions(+), 502 deletions(-)
 delete mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28-var1.dts
 delete mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28-var2.dts
 delete mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28-var3.dts
 delete mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28-var4.dts
 delete mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28.dts

diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var1.dts 
b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var1.dts
deleted file mode 100644
index 7cd29ab970..00
--- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var1.dts
+++ /dev/null
@@ -1,59 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Device Tree file for the Kontron SMARC-sAL28 board.
- *
- * This is for the network variant 1 which has one ethernet port. It is
- * different than the base variant, which also has one port, but here the
- * port is connected via RGMII. This port is not TSN aware.
- * None of the  four SerDes lanes are used by the module, instead they are
- * all led out to the carrier for customer use.
- *
- * Copyright (C) 2021 Michael Walle 
- *
- */
-
-/dts-v1/;
-#include "fsl-ls1028a-kontron-sl28.dts"
-#include 
-
-/ {
-   model = "Kontron SMARC-sAL28 (4 Lanes)";
-   compatible = "kontron,sl28-var1", "kontron,sl28", "fsl,ls1028a";
-};
-
-_mdio_pf3 {
-   /* Delete unused phy node */
-   /delete-node/ ethernet-phy@5;
-
-   phy0: ethernet-phy@4 {
-   reg = <0x4>;
-   eee-broken-1000t;
-   eee-broken-100tx;
-   qca,clk-out-frequency = <12500>;
-   qca,clk-out-strength = ;
-   qca,keep-pll-enabled;
-   vddio-supply = <>;
-
-   vddio: vddio-regulator {
-   regulator-name = "VDDIO";
-   regulator-min-microvolt = <180>;
-   regulator-max-microvolt = <180>;
-   };
-
-   vddh: vddh-regulator {
-   regulator-name = "VDDH";
-   };
-   };
-};
-
-_port0 {
-   status = "disabled";
-   /* Delete the phy-handle to the old phy0 label */
-   /delete-property/ phy-handle;
-};
-
-_port1 {
-   phy-handle = <>;
-   phy-mode = "rgmii-id";
-   status = "okay";
-};
diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var2.dts 
b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var2.dts
deleted file mode 100644
index 330e34f933..00
--- a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var2.dts
+++ /dev/null
@@ -1,65 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * Device Tree file for the Kontron SMARC-sAL28 board.
- *
- * This is for the network variant 2 which has two ethernet ports. These
- * ports are connected to the internal switch.
- *
- * Copyright (C) 2021 Michael Walle 
- *
- */
-
-/dts-v1/;
-#include "fsl-ls1028a-kontron-sl28.dts"
-
-/ {
-   model = "Kontron SMARC-sAL28 (TSN-on-module)";
-   compatible = "kontron,sl28-var2", "kontron,sl28", "fsl,ls1028a";
-};
-
-_mdio_pf3 {
-   phy1: ethernet-phy@4 {
-   reg = <0x4>;
-   eee-broken-1000t;
-   eee-broken-100tx;
-   };
-};
-
-_port0 {
-   status = "disabled";
-   /*
-* In the base device tree the PHY at address 5 was assigned for
-* this port. On this module this PHY is connected to a switch
-* port instead. Therefore, delete the phy-handle property here.
-*/
-   /delete-property/ phy-handle;
-};
-
-_port2 {
-   status = "okay";
-};
-
-_felix {
-   status = "okay";
-};
-
-_felix_port0 {
-   label = "swp0";
-   managed = "in-band-status";
-   phy-handle = <>;
-   phy-mode = "sgmii";
-   status = "okay";
-};
-
-_felix_port1 {
-   label = "swp1";
-   managed = "i

Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot

2024-03-06 Thread Michael Walle
Hi,

On Wed Mar 6, 2024 at 3:56 AM CET, Marek Vasut wrote:
> > I'd argue if one wants to use the locking at all, you have to set
> > UNLOCK_ALL=n. Otherwise, the bootloader might come alone and just
> > clear your locking bits again. Clearing the WPS bit there is just
> > one more thing which IMHO doesn't make much sense.
>
> On the other hand, if UNLOCK_ALL=y is supposed to work and reset 
> locking, then the SR3 WPS bit has to be configured to select the 
> standard SPI NOR locking scheme, so the locking can actually be reset 
> using that scheme.

Yes, that's what I've meant with "the unlock all works as
advertised" and your patch is fine.

-michael


signature.asc
Description: PGP signature


Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot

2024-03-05 Thread Michael Walle
On Tue Mar 5, 2024 at 7:54 PM CET, Marek Vasut wrote:
> On 3/5/24 5:55 PM, Michael Walle wrote:
>
> [...]
>
> >>>>>> Clearing this SR3 WPS bit fixes that problem, both in U-Boot and in
> >>>>>> Linux, since Linux that is booted afterward then gets a device that has
> >>>>>> locking scheme configured in a way that Linux expects and can operate.
> >>>>>>
> >>>>>> Better yet, if some old LTS version of the Linux kernel is in use, it
> >>>>>> will also work correctly, because this patch will configure the SPI NOR
> >>>>>> locking scheme to what Linux expects it to be, before the SPI NOR is
> >>>>>> handed over to that old kernel.
> >>>>>
> >>>>> Agreed, but it should *not* be done automatically and nor
> >>>>> unconditionally. Please don't just overwrite any locking bits just
> >>>>> because there is one flash which have it set.
> >>>>
> >>>> The unlock code is not triggered unconditionally, it is protected by
> >>>>
> >>>> if (IS_ENABLED(CONFIG_SPI_FLASH_UNLOCK_ALL)...
> >>>>
> >>>> Kconfig option, so this can be turned on/off in board config already.
> >>>
> >>> Ahh, OK then :)
> >>>
> >>> But keep in mind that enabling this option is foobar and I've gone
> >>> lengths to eliminate it in linux. And actually made that option in
> >>> u-boot.
> >>>
> >>> See linux commit 31ad3eff093c ("mtd: spi-nor: keep lock bits if they
> >>> are non-volatile").
> >>
> >> So, how shall we proceed here?
> > 
> > Regarding this patch, I think it's fine. It will unlock the whole
> > flash as advertised.
>
> This change won't unlock the flash, it will switch to the supported 
> locking scheme, one which can then be used to unlock the flash.

I can't follow you here. The code is added right below the
write_sr(0), which will clear all the BP bits, thus unlocking
everything if WPS=0. Therefore, no locking will be enabled anymore
afterwards. What did I miss?

> > But u-boot should really consider making that a "default n" option.
> > And most likely adding =y to every existing defconfig out there so
> > that at least new boards will benefit from it.
>
> Yes, I agree with that.
>
> >> The way I see this, if Linux ever implements this scheme, Linux can set
> >> the SR3 WPS bit as needed, it does not matter which way bootloader sets
> >> the bit as the protection bits are not cleared when the bit is cleared,
> >> they seem to be stored elsewhere.
> > 
> > On each reboot you'd wear out that cell with two erases/writes.
> > That's another reason why that whole unlocking thing is foobar for
> > non-volatile bits. For me, non-volatile bits are for provisioning,
> > so during a normal boot they should not be touched at all. Just
> > during board manufacturing or because the user actively want to
> > protect something.
>
> That is what happens here, the write to clear the bit is triggered only 
> if the bit is set , so OK .
>
> And if Linux wants to use the new locking scheme, then the bootloader 
> should ideally be updated to match, i.e. SPI_FLASH_UNLOCK_ALL=n etc, so 
> that is also OK .

I'd argue if one wants to use the locking at all, you have to set
UNLOCK_ALL=n. Otherwise, the bootloader might come alone and just
clear your locking bits again. Clearing the WPS bit there is just
one more thing which IMHO doesn't make much sense.

> In other words, there should be no writes into the non-volatile bits, 
> unless U-Boot and Linux disagree on the locking scheme,

Agreed.

> in which case 
> writes are unavoidable (if you want unlock to work in both projects).

But this should only happen if the user want to change the locking
at all. u-boot should not just do "oh this bit is set, I'm clearing
it" during SPI flash probe. Again, I'm not caring much if this is
guarded by the UNLOCK_ALL=y, because u-boot is already doing "oh the
BP bits are set, lets clear em".

> > If you clear this bit during the unlock all command, all the locking
> > bits are cleared anyway. Or do you mean, the individual bits are
> > still retained?
>
> The lock bits themselves are retained, this SR3 WPS only selects which 
> of those bits take effect, whether the SR ones (standard locking scheme) 
> or the per-page ones (winbond custom locking scheme).

Ok. So switching back to WPS=1 might come with some suprises :)

-michael

> >> But, if Linux starts to use SR3

Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot

2024-03-05 Thread Michael Walle
On Tue Mar 5, 2024 at 5:28 PM CET, Marek Vasut wrote:
> On 3/5/24 4:53 PM, Michael Walle wrote:
> > On Tue Mar 5, 2024 at 4:37 PM CET, Marek Vasut wrote:
> >> On 3/5/24 1:50 PM, Michael Walle wrote:
> >>> On Tue Mar 5, 2024 at 1:31 PM CET, Marek Vasut wrote:
> >>>> On 3/5/24 9:55 AM, Michael Walle wrote:
> >>>>> On Mon Mar 4, 2024 at 5:16 PM CET, Marek Vasut wrote:
> >>>>>> Some Winbond SPI NORs have special SR3 register which is
> >>>>>> used among other things to control whether non-standard
> >>>>>> "Individual Block/Sector Write Protection" (WPS bit)
> >>>>>> locking scheme is activated. This non-standard locking
> >>>>>> scheme is not supported by either U-Boot or Linux SPI
> >>>>>> NOR stack so make sure it is disabled, otherwise the
> >>>>>> SPI NOR may appear locked for no obvious reason.
> >>>>>
> >>>>> I don't think it is a good idea to just disable the WPS bit.
> >>>>> Usually, that bit is non-volatile and the default is not set.
> >>>>
> >>>> Yes, that's right, the bit is non-volatile and should not be set unless
> >>>> the MTD stack can handle it correctly. Currently, neither U-Boot nor
> >>>> Linux does handle the bit at all, instead both attempt to use the
> >>>> standard SPI NOR locking scheme which is also implemented by this SPI
> >>>> NOR model and they both fail to unlock the SPI NOR that way.
> >>>>
> >>>> Note that the SR3 WPS bit only switches between two different locking
> >>>> schemes (unset = standard SPI NOR locking scheme, set = custom winbond
> >>>> locking scheme), setting SR3 WPS does not immediately imply the SPI NOR
> >>>> is locked, rather the opposite. Out of manufacturing, the SPI NOR is
> >>>> unlocked in either locking scheme. Depending on the SR3 WPS bit state,
> >>>> different commands are used to lock and unlock the SPI NOR.
> >>>>
> >>>> I recently ran across a device which had the SR3 WPS bit incorrectly set
> >>>> out of manufacturing of that device (i.e. before it was populated on a
> >>>> board at board manufacturer) and the device was locked using the custom
> >>>> locking scheme. I was not able to unlock or use that device because both
> >>>> U-Boot and Linux tried to use the standard scheme for that purpose.
> >>>
> >>> Still, I don't think it makes any sense, to disable that bit for all
> >>> winbond flashes just because there was one vendor which set it the
> >>> wrong way - or the board manufacturer didn't test it and programmed
> >>> the flash correctly.
> >>
> >> OK
> >>
> >>>> Clearing this SR3 WPS bit fixes that problem, both in U-Boot and in
> >>>> Linux, since Linux that is booted afterward then gets a device that has
> >>>> locking scheme configured in a way that Linux expects and can operate.
> >>>>
> >>>> Better yet, if some old LTS version of the Linux kernel is in use, it
> >>>> will also work correctly, because this patch will configure the SPI NOR
> >>>> locking scheme to what Linux expects it to be, before the SPI NOR is
> >>>> handed over to that old kernel.
> >>>
> >>> Agreed, but it should *not* be done automatically and nor
> >>> unconditionally. Please don't just overwrite any locking bits just
> >>> because there is one flash which have it set.
> >>
> >> The unlock code is not triggered unconditionally, it is protected by
> >>
> >> if (IS_ENABLED(CONFIG_SPI_FLASH_UNLOCK_ALL)...
> >>
> >> Kconfig option, so this can be turned on/off in board config already.
> > 
> > Ahh, OK then :)
> > 
> > But keep in mind that enabling this option is foobar and I've gone
> > lengths to eliminate it in linux. And actually made that option in
> > u-boot.
> > 
> > See linux commit 31ad3eff093c ("mtd: spi-nor: keep lock bits if they
> > are non-volatile").
>
> So, how shall we proceed here?

Regarding this patch, I think it's fine. It will unlock the whole
flash as advertised.

But u-boot should really consider making that a "default n" option.
And most likely adding =y to every existing defconfig out there so
that at least new boards will benefit from it.

> The way I see this, if Linux ever implements this scheme, Lin

Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot

2024-03-05 Thread Michael Walle
On Tue Mar 5, 2024 at 4:37 PM CET, Marek Vasut wrote:
> On 3/5/24 1:50 PM, Michael Walle wrote:
> > Hi Marek,
>
> Hi,
>
> > On Tue Mar 5, 2024 at 1:31 PM CET, Marek Vasut wrote:
> >> On 3/5/24 9:55 AM, Michael Walle wrote:
> >>> On Mon Mar 4, 2024 at 5:16 PM CET, Marek Vasut wrote:
> >>>> Some Winbond SPI NORs have special SR3 register which is
> >>>> used among other things to control whether non-standard
> >>>> "Individual Block/Sector Write Protection" (WPS bit)
> >>>> locking scheme is activated. This non-standard locking
> >>>> scheme is not supported by either U-Boot or Linux SPI
> >>>> NOR stack so make sure it is disabled, otherwise the
> >>>> SPI NOR may appear locked for no obvious reason.
> >>>
> >>> I don't think it is a good idea to just disable the WPS bit.
> >>> Usually, that bit is non-volatile and the default is not set.
> >>
> >> Yes, that's right, the bit is non-volatile and should not be set unless
> >> the MTD stack can handle it correctly. Currently, neither U-Boot nor
> >> Linux does handle the bit at all, instead both attempt to use the
> >> standard SPI NOR locking scheme which is also implemented by this SPI
> >> NOR model and they both fail to unlock the SPI NOR that way.
> >>
> >> Note that the SR3 WPS bit only switches between two different locking
> >> schemes (unset = standard SPI NOR locking scheme, set = custom winbond
> >> locking scheme), setting SR3 WPS does not immediately imply the SPI NOR
> >> is locked, rather the opposite. Out of manufacturing, the SPI NOR is
> >> unlocked in either locking scheme. Depending on the SR3 WPS bit state,
> >> different commands are used to lock and unlock the SPI NOR.
> >>
> >> I recently ran across a device which had the SR3 WPS bit incorrectly set
> >> out of manufacturing of that device (i.e. before it was populated on a
> >> board at board manufacturer) and the device was locked using the custom
> >> locking scheme. I was not able to unlock or use that device because both
> >> U-Boot and Linux tried to use the standard scheme for that purpose.
> > 
> > Still, I don't think it makes any sense, to disable that bit for all
> > winbond flashes just because there was one vendor which set it the
> > wrong way - or the board manufacturer didn't test it and programmed
> > the flash correctly.
>
> OK
>
> >> Clearing this SR3 WPS bit fixes that problem, both in U-Boot and in
> >> Linux, since Linux that is booted afterward then gets a device that has
> >> locking scheme configured in a way that Linux expects and can operate.
> >>
> >> Better yet, if some old LTS version of the Linux kernel is in use, it
> >> will also work correctly, because this patch will configure the SPI NOR
> >> locking scheme to what Linux expects it to be, before the SPI NOR is
> >> handed over to that old kernel.
> > 
> > Agreed, but it should *not* be done automatically and nor
> > unconditionally. Please don't just overwrite any locking bits just
> > because there is one flash which have it set.
>
> The unlock code is not triggered unconditionally, it is protected by
>
> if (IS_ENABLED(CONFIG_SPI_FLASH_UNLOCK_ALL)...
>
> Kconfig option, so this can be turned on/off in board config already.

Ahh, OK then :)

But keep in mind that enabling this option is foobar and I've gone
lengths to eliminate it in linux. And actually made that option in
u-boot.

See linux commit 31ad3eff093c ("mtd: spi-nor: keep lock bits if they
are non-volatile").

-michael

> > This should be either be a board level option or maybe expose some
> > command line interface to let the user change the settings.
> > 
> >>> Thus,
> >>> there is likely someone, probably the manufacturer of the board,
> >>> who intentionally set this bit. Now, with this patch it will get
> >>> disabled *unconditionally*, forever.
> >>
> >> In my case, it is exactly the opposite, the SR3 WPS shouldn't have been
> >> set and the device should not have been locked, but it was and that
> >> confused both U-Boot and Linux.
> >>
> >> I would argue that if the board manufacturer intention was to lock the
> >> SPI NOR, they would have had multiple better options at their disposal,
> >> and those options should have been aligned with the software support:
> >> - nWP pin of the SPI NOR could be tied low to enable WRITE PROTECT
> >> - OTP bits could have been programmed to enable permanent WRITE PROTECT
> >> - standard SPI NOR locking scheme could have been used too
> >>
> >> If they did set SR3 WPS and hoped that U-Boot or Linux would fail to
> >> unlock the SPI NOR using standard locking scheme commands, that is I
> >> think broken design.
> > 
> > There might be software/OS which could handle that correctly. Also,
> > if linux will ever learn to use the new locking scheme
> > unconditionally, u-boot will always mess it up then.
>
> See CONFIG_SPI_FLASH_UNLOCK_ALL above.



signature.asc
Description: PGP signature


Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot

2024-03-05 Thread Michael Walle
Hi Marek,

On Tue Mar 5, 2024 at 1:31 PM CET, Marek Vasut wrote:
> On 3/5/24 9:55 AM, Michael Walle wrote:
> > On Mon Mar 4, 2024 at 5:16 PM CET, Marek Vasut wrote:
> >> Some Winbond SPI NORs have special SR3 register which is
> >> used among other things to control whether non-standard
> >> "Individual Block/Sector Write Protection" (WPS bit)
> >> locking scheme is activated. This non-standard locking
> >> scheme is not supported by either U-Boot or Linux SPI
> >> NOR stack so make sure it is disabled, otherwise the
> >> SPI NOR may appear locked for no obvious reason.
> > 
> > I don't think it is a good idea to just disable the WPS bit.
> > Usually, that bit is non-volatile and the default is not set.
>
> Yes, that's right, the bit is non-volatile and should not be set unless 
> the MTD stack can handle it correctly. Currently, neither U-Boot nor 
> Linux does handle the bit at all, instead both attempt to use the 
> standard SPI NOR locking scheme which is also implemented by this SPI 
> NOR model and they both fail to unlock the SPI NOR that way.
>
> Note that the SR3 WPS bit only switches between two different locking 
> schemes (unset = standard SPI NOR locking scheme, set = custom winbond 
> locking scheme), setting SR3 WPS does not immediately imply the SPI NOR 
> is locked, rather the opposite. Out of manufacturing, the SPI NOR is 
> unlocked in either locking scheme. Depending on the SR3 WPS bit state, 
> different commands are used to lock and unlock the SPI NOR.
>
> I recently ran across a device which had the SR3 WPS bit incorrectly set 
> out of manufacturing of that device (i.e. before it was populated on a 
> board at board manufacturer) and the device was locked using the custom 
> locking scheme. I was not able to unlock or use that device because both 
> U-Boot and Linux tried to use the standard scheme for that purpose.

Still, I don't think it makes any sense, to disable that bit for all
winbond flashes just because there was one vendor which set it the
wrong way - or the board manufacturer didn't test it and programmed
the flash correctly.

> Clearing this SR3 WPS bit fixes that problem, both in U-Boot and in 
> Linux, since Linux that is booted afterward then gets a device that has 
> locking scheme configured in a way that Linux expects and can operate.
>
> Better yet, if some old LTS version of the Linux kernel is in use, it 
> will also work correctly, because this patch will configure the SPI NOR 
> locking scheme to what Linux expects it to be, before the SPI NOR is 
> handed over to that old kernel.

Agreed, but it should *not* be done automatically and nor
unconditionally. Please don't just overwrite any locking bits just
because there is one flash which have it set.

This should be either be a board level option or maybe expose some
command line interface to let the user change the settings.

> > Thus,
> > there is likely someone, probably the manufacturer of the board,
> > who intentionally set this bit. Now, with this patch it will get
> > disabled *unconditionally*, forever.
>
> In my case, it is exactly the opposite, the SR3 WPS shouldn't have been 
> set and the device should not have been locked, but it was and that 
> confused both U-Boot and Linux.
>
> I would argue that if the board manufacturer intention was to lock the 
> SPI NOR, they would have had multiple better options at their disposal, 
> and those options should have been aligned with the software support:
> - nWP pin of the SPI NOR could be tied low to enable WRITE PROTECT
> - OTP bits could have been programmed to enable permanent WRITE PROTECT
> - standard SPI NOR locking scheme could have been used too
>
> If they did set SR3 WPS and hoped that U-Boot or Linux would fail to 
> unlock the SPI NOR using standard locking scheme commands, that is I 
> think broken design.

There might be software/OS which could handle that correctly. Also,
if linux will ever learn to use the new locking scheme
unconditionally, u-boot will always mess it up then.

-michael


signature.asc
Description: PGP signature


Re: [PATCH] mtd: spi-nor: Clear Winbond SR3 WPS bit on boot

2024-03-05 Thread Michael Walle
[+ linux-mtd ]

Hi Marek,

On Mon Mar 4, 2024 at 5:16 PM CET, Marek Vasut wrote:
> Some Winbond SPI NORs have special SR3 register which is
> used among other things to control whether non-standard
> "Individual Block/Sector Write Protection" (WPS bit)
> locking scheme is activated. This non-standard locking
> scheme is not supported by either U-Boot or Linux SPI
> NOR stack so make sure it is disabled, otherwise the
> SPI NOR may appear locked for no obvious reason.

I don't think it is a good idea to just disable the WPS bit.
Usually, that bit is non-volatile and the default is not set. Thus,
there is likely someone, probably the manufacturer of the board,
who intentionally set this bit. Now, with this patch it will get
disabled *unconditionally*, forever.

-michael

> W25Q16DW, but the W25Q16DW does not implement the SR3 WPS bit.
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Hai Pham 
> Cc: Heinrich Schuchardt 
> Cc: Jagan Teki 
> Cc: Johann Neuhauser 
> Cc: Simon Glass 
> Cc: Takahiro Kuwano 
> Cc: Venkatesh Yadav Abbarapu 
> Cc: Vignesh R 
> ---
>  drivers/mtd/spi/spi-nor-core.c | 47 ++
>  include/linux/mtd/spi-nor.h|  5 
>  2 files changed, 52 insertions(+)
>
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index 9a1801ba93d..68dee57258d 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -544,6 +544,24 @@ static int read_cr(struct spi_nor *nor)
>  }
>  #endif
>  
> +/**
> + * read_sr3() - Read status register 3 unique to newer Winbond flashes
> + * @nor: pointer to a 'struct spi_nor'
> + */
> +static int read_sr3(struct spi_nor *nor)
> +{
> + int ret;
> + u8 val;
> +
> + ret = nor->read_reg(nor, SPINOR_OP_RDSR3, , 1);
> + if (ret < 0) {
> + dev_dbg(nor->dev, "error %d reading SR3\n", ret);
> + return ret;
> + }
> +
> + return val;
> +}
> +
>  /*
>   * Write status register 1 byte
>   * Returns negative if error occurred.
> @@ -554,6 +572,17 @@ static int write_sr(struct spi_nor *nor, u8 val)
>   return nor->write_reg(nor, SPINOR_OP_WRSR, nor->cmd_buf, 1);
>  }
>  
> +/**
> + * write_sr3() - Write status register 3 unique to newer Winbond flashes
> + * @nor: pointer to a 'struct spi_nor'
> + * @val: value to be written into SR3
> + */
> +static int write_sr3(struct spi_nor *nor, u8 val)
> +{
> + nor->cmd_buf[0] = val;
> + return nor->write_reg(nor, SPINOR_OP_WRSR3, nor->cmd_buf, 1);
> +}
> +
>  /*
>   * Set write enable latch with Write Enable command.
>   * Returns negative if error occurred.
> @@ -3890,6 +3919,24 @@ static int spi_nor_init(struct spi_nor *nor)
>   write_enable(nor);
>   write_sr(nor, 0);
>   spi_nor_wait_till_ready(nor);
> +
> + /*
> +  * Some Winbond SPI NORs have special SR3 register which is
> +  * used among other things to control whether non-standard
> +  * "Individual Block/Sector Write Protection" (WPS bit)
> +  * locking scheme is activated. This non-standard locking
> +  * scheme is not supported by either U-Boot or Linux SPI
> +  * NOR stack so make sure it is disabled, otherwise the
> +  * SPI NOR may appear locked for no obvious reason.
> +  */
> + if (JEDEC_MFR(nor->info) == SNOR_MFR_WINBOND) {
> + err = read_sr3(nor);
> + if (err > 0 && err & SR3_WPS) {
> + write_enable(nor);
> + write_sr3(nor, err & ~SR3_WPS);
> + write_disable(nor);
> + }
> + }
>   }
>  
>   if (nor->quad_enable) {
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 2861b73edbc..ceb32e3906f 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -45,6 +45,8 @@
>  #define SPINOR_OP_WRSR   0x01/* Write status register 1 byte 
> */
>  #define SPINOR_OP_RDSR2  0x3f/* Read status register 2 */
>  #define SPINOR_OP_WRSR2  0x3e/* Write status register 2 */
> +#define SPINOR_OP_RDSR3  0x15/* Read status register 3 */
> +#define SPINOR_OP_WRSR3  0x11/* Write status register 3 */
>  #define SPINOR_OP_READ   0x03/* Read data bytes (low 
> frequency) */
>  #define SPINOR_OP_READ_FAST  0x0b/* Read data bytes (high frequency) */
>  #define SPINOR_OP_READ_

Re: [PATCH] cmd: sf: Fix sf probe crash

2024-03-04 Thread Michael Nazzareno Trimarchi
Hi

On Mon, Mar 4, 2024 at 4:44 PM Heinrich Schuchardt  wrote:
>
> On 04.03.24 15:47, Weizhao Ouyang wrote:
> > Gentle ping. Not merged yet.
> >
> > BR,
> > Weizhao
>
> Hello Dario,
>
> that patch is in your patchwork review queue. Could you, please, have a
> look.

He is with me, I will ping about all the patches that we need to queue

Michael

>
> Best regards
>
> Heinrich
>
> >
> > On Thu, Jan 4, 2024 at 7:46 PM Weizhao Ouyang  wrote:
> >>
> >> Handle the return value of spi_flash_probe_bus_cs() to avoid sf probe
> >> crashes.
> >>
> >> Signed-off-by: Weizhao Ouyang 
> >> ---
> >>   cmd/sf.c | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/cmd/sf.c b/cmd/sf.c
> >> index 730996c02b..e3866899f6 100644
> >> --- a/cmd/sf.c
> >> +++ b/cmd/sf.c
> >> @@ -135,8 +135,9 @@ static int do_spi_flash_probe(int argc, char *const 
> >> argv[])
> >>  }
> >>  flash = NULL;
> >>  if (use_dt) {
> >> -   spi_flash_probe_bus_cs(bus, cs, );
> >> -   flash = dev_get_uclass_priv(new);
> >> +   ret = spi_flash_probe_bus_cs(bus, cs, );
> >> +   if (!ret)
> >> +   flash = dev_get_uclass_priv(new);
> >>  } else {
> >>  flash = spi_flash_probe(bus, cs, speed, mode);
> >>  }
> >> --
> >> 2.39.2
> >>
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH] mtd: spinand: Add support for XTX XT26xxxDxxxxx

2024-02-29 Thread Michael Nazzareno Trimarchi
Hi

I will ask dario to pick them up. I wil review too We have a bunch of
patches to get in nand-next

Michael

On Thu, Feb 29, 2024 at 10:24 AM Bruce Sun  wrote:
>
> Thans for your advice, I will port the driver from linux driver(xtx.c)
> later.
>
> On 2/29/2024 5:05 PM, Frieder Schrempf wrote:
> > Hi Bruce,
> >
> > On 26.12.23 08:58, Bruce Suen wrote:
> >> [Sie erhalten nicht häufig E-Mails von bruce_s...@163.com. Weitere 
> >> Informationen, warum dies wichtig ist, finden Sie unter 
> >> https://aka.ms/LearnAboutSenderIdentification ]
> >>
> >> Add Support XTX Technology XT26G01DX, XT26G11DX, XT26Q01DX,
> >> XT26G02DX, XT26G12DX, XT26Q02DX, XT26G04DX, and
> >> XT26Q04DX SPI NAND.
> >>
> >> These are 3V/1.8V 1G/2G/4Gbit serial SLC NAND flash device with on-die
> >> ECC(8bit strength per 512bytes).
> >>
> >> Datasheet Links:
> >> - http://www.xtxtech.com/download/?AId=458
> >> - http://www.xtxtech.com/download/?AId=495
> >>
> >> Signed-off-by: Bruce Suen 
> > Below you seem to have written a new driver based on gigadevice.c. Can
> > you please instead port the existing Linux driver (xtx.c). This will
> > help us to sync the drivers in the future if new chips will be added.
> >
> > Thanks
> > Frieder
> >
> >> ---
> >>   drivers/mtd/nand/spi/Makefile |   1 +
> >>   drivers/mtd/nand/spi/core.c   |   1 +
> >>   drivers/mtd/nand/spi/xtx.c| 180 ++
> >>   include/linux/mtd/spinand.h   |   1 +
> >>   4 files changed, 183 insertions(+)
> >>   create mode 100644 drivers/mtd/nand/spi/xtx.c
> >>
> >> diff --git a/drivers/mtd/nand/spi/Makefile b/drivers/mtd/nand/spi/Makefile
> >> index 3051de4f7e..e46cfed446 100644
> >> --- a/drivers/mtd/nand/spi/Makefile
> >> +++ b/drivers/mtd/nand/spi/Makefile
> >> @@ -1,4 +1,5 @@
> >>   # SPDX-License-Identifier: GPL-2.0
> >>
> >>   spinand-objs := core.o gigadevice.o macronix.o micron.o paragon.o 
> >> toshiba.o winbond.o
> >> +spinand-objs += xtx.o
> >>   obj-$(CONFIG_MTD_SPI_NAND) += spinand.o
> >> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> >> index 597b088ca7..0b4c067425 100644
> >> --- a/drivers/mtd/nand/spi/core.c
> >> +++ b/drivers/mtd/nand/spi/core.c
> >> @@ -828,6 +828,7 @@ static const struct spinand_manufacturer 
> >> *spinand_manufacturers[] = {
> >>  _spinand_manufacturer,
> >>  _spinand_manufacturer,
> >>  _spinand_manufacturer,
> >> +   _spinand_manufacturer,
> >>   };
> >>
> >>   static int spinand_manufacturer_match(struct spinand_device *spinand,
> >> diff --git a/drivers/mtd/nand/spi/xtx.c b/drivers/mtd/nand/spi/xtx.c
> >> new file mode 100644
> >> index 00..602861c77b
> >> --- /dev/null
> >> +++ b/drivers/mtd/nand/spi/xtx.c
> >> @@ -0,0 +1,180 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (C) 2018 Stefan Roese 
> >> + *
> >> + * Derived from drivers/mtd/nand/spi/micron.c
> >> + *   Copyright (c) 2016-2017 Micron Technology, Inc.
> >> + */
> >> +
> >> +#ifndef __UBOOT__
> >> +#include 
> >> +#include 
> >> +#endif
> >> +#include 
> >> +#include 
> >> +
> >> +#define SPINAND_MFR_XTX0x0B
> >> +
> >> +#define XT26XXXD_STATUS_ECC3_ECC2_MASK GENMASK(7, 6)
> >> +#define XT26XXXD_STATUS_ECC_NO_DETECTED (0)
> >> +#define XT26XXXD_STATUS_ECC_1_7_CORRECTED   (1)
> >> +#define XT26XXXD_STATUS_ECC_8_CORRECTED (3)
> >> +#define XT26XXXD_STATUS_ECC_UNCOR_ERROR (2)
> >> +
> >> +static SPINAND_OP_VARIANTS(read_cache_variants,
> >> +   SPINAND_PAGE_READ_FROM_CACHE_QUADIO_OP(0, 1, NULL, 0),
> >> +   SPINAND_PAGE_READ_FROM_CACHE_X4_OP(0, 1, NULL, 0),
> >> +   SPINAND_PAGE_READ_FROM_CACHE_DUALIO_OP(0, 1, NULL, 0),
> >> +   SPINAND_PAGE_READ_FROM_CACHE_X2_OP(0, 1, NULL, 0),
> >> +   SPINAND_PAGE_READ_FROM_CACHE_OP(true, 0, 1, NULL, 0),
> >> +   SPINAND_PAGE_READ_FROM_CACHE_OP(false, 0, 1, NULL, 0));
> >> +
> >> +static SPINAND_OP_VARIANTS(write_cache_variants,
> >> +   SPINAND_PROG_LOAD_X4(true, 0, NULL, 0),
> >> +   SPINAND_PROG_LOAD(true, 0, NULL, 0)

Re: Need help: alternative module inclusion - duplicate symbols

2024-02-26 Thread Michael Lawnick
Sorry forget about this, wrong mailing list !

Am 26.02.2024 um 11:36 schrieb Michael Lawnick:
> Hi group,
>
> hope you can help me:
> I have modules pci_fpga.c and pci_fpga_emul.c with same functions in
> them but different implementation, one for the real device, the other
> one is an emulation.
>
> What I now want is being able to include one of both versions through
> build command. I started with this:
>
> Makefile.core.def:
> ...
> module = {
>   name = pci_fpga;
>   common = startlib/board/pci_fpga.c;
>   enable = efi;
> };
>
> module = {
>   name = pci_fpga_emul;
>   common = startlib/board/pci_fpga_emul.c;
>   enable = efi;
> };
> ...
>
> and then give pci_fpga or pci_fpga_emu on the build but then I get
> duplicate symbols error.
>
> For using different packages than common like this
> module = {
>   name = pci_fpga;
>   pci_fpga = startlib/board/pci_fpga.c;
>   enable = pci_fpga;
> };
> I could not find means to get pci_fpga included.
>
> Same problem if trying to approach it via different enable flag: How to?
>
> Can anybody help? In docs I couldn't find usable hints.
>
> --
> KR
> Michael


Need help: alternative module inclusion - duplicate symbols

2024-02-26 Thread Michael Lawnick
Hi group,

hope you can help me:
I have modules pci_fpga.c and pci_fpga_emul.c with same functions in
them but different implementation, one for the real device, the other
one is an emulation.

What I now want is being able to include one of both versions through
build command. I started with this:

Makefile.core.def:
...
module = {
  name = pci_fpga;
  common = startlib/board/pci_fpga.c;
  enable = efi;
};

module = {
  name = pci_fpga_emul;
  common = startlib/board/pci_fpga_emul.c;
  enable = efi;
};
...

and then give pci_fpga or pci_fpga_emu on the build but then I get
duplicate symbols error.

For using different packages than common like this
module = {
  name = pci_fpga;
  pci_fpga = startlib/board/pci_fpga.c;
  enable = pci_fpga;
};
I could not find means to get pci_fpga included.

Same problem if trying to approach it via different enable flag: How to?

Can anybody help? In docs I couldn't find usable hints.

--
KR
Michael


[PATCH V2] arm: dts: k3-binman: Make optee optional as requirement

2024-02-25 Thread Michael Trimarchi
Allow boards that use ti_spl_template to not use optee part in
configuration.
Vendor can have module with 256 Mb of memory and they try to optimize
the available memory just using the essential components.
This change allow to remove tee from configuration without binman
fail.

configurations {
default = "conf-0";

conf-0 {
description = "k3-am62_ccm_m3";
firmware = "atf";
loadables = "dm", "spl";
    fdt = "fdt-0";
};
};

Signed-off-by: Michael Trimarchi 
---
V1->V2:
Adjust commit message
---
 arch/arm/dts/k3-binman.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
index 758c8bf6ea..5ef5af315a 100644
--- a/arch/arm/dts/k3-binman.dtsi
+++ b/arch/arm/dts/k3-binman.dtsi
@@ -293,6 +293,7 @@
keyfile = "custMpk.pem";
};
tee: tee-os {
+   optional;
};
};
 
@@ -360,6 +361,7 @@
entry = <0x9e80>;
tee-os {
filename = "tee-raw.bin";
+   optional;
};
};
 
-- 
2.40.1



Re: [PATCH] arm: dts: k3-binman: Make optee optional as requirement

2024-02-25 Thread Michael Nazzareno Trimarchi
Hi

On Mon, Feb 26, 2024 at 4:17 AM Neha Malcom Francis  wrote:
>
> Hi Michael
>
> + Vignesh
>
> On 25/02/24 23:08, Michael Trimarchi wrote:
> > Boards can use the ti_spl_template but avoid to define a tee
> > node to be loaded. This is true form board with 512Mb or less
>
> s/form board/for boards? I am guessing that is what was meant. I am not really
> understanding the commit message... does <512MB memory always mean no using
> OPTEE? (AM62 SIP would be an exception) The commit message is putting out a
> misleading reason why we don't need OPTEE in some cases, I don't think memory 
> is
> the primary reason.
>

I will adjust commit message. In general when you have module with 256
Mb of memory you
try to arrange the available memory to put the essential components.
Anyway having still not
submitted board I found out that is convenient to use the template but
in the same time remove
from my configuration what is not mandatory and let binman to warning
and not to fail

Michael

> > memory. We can limit this in configuation removing the tee
>
> s/configuation/configuration
>
> > node
> >
> > configurations {
> >   default = "conf-0";
> >
> >   conf-0 {
> >   description = "k3-am62_ccm_m3";
> >   firmware = "atf";
> >   loadables = "dm", "spl";
> >   fdt = "fdt-0";
> >   };
> > };
> >
> > Signed-off-by: Michael Trimarchi 
> > ---
> >   arch/arm/dts/k3-binman.dtsi | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
> > index 758c8bf6ea..5ef5af315a 100644
> > --- a/arch/arm/dts/k3-binman.dtsi
> > +++ b/arch/arm/dts/k3-binman.dtsi
> > @@ -293,6 +293,7 @@
> >   keyfile = "custMpk.pem";
> >   };
> >   tee: tee-os {
> > + optional;
> >   };
> >   };
> >
> > @@ -360,6 +361,7 @@
> >   entry = <0x9e80>;
> >   tee-os {
> >   filename = "tee-raw.bin";
> > + optional;
> >   };
> >   };
> >
>
> The patch excluding the commit message LGTM.
>
> --
> Thanking You
> Neha Malcom Francis



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


[PATCH] arm: dts: k3-binman: Make optee optional as requirement

2024-02-25 Thread Michael Trimarchi
Boards can use the ti_spl_template but avoid to define a tee
node to be loaded. This is true form board with 512Mb or less
memory. We can limit this in configuation removing the tee
node

configurations {
default = "conf-0";

conf-0 {
description = "k3-am62_ccm_m3";
firmware = "atf";
loadables = "dm", "spl";
    fdt = "fdt-0";
};
};

Signed-off-by: Michael Trimarchi 
---
 arch/arm/dts/k3-binman.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/dts/k3-binman.dtsi b/arch/arm/dts/k3-binman.dtsi
index 758c8bf6ea..5ef5af315a 100644
--- a/arch/arm/dts/k3-binman.dtsi
+++ b/arch/arm/dts/k3-binman.dtsi
@@ -293,6 +293,7 @@
keyfile = "custMpk.pem";
};
tee: tee-os {
+   optional;
};
};
 
@@ -360,6 +361,7 @@
entry = <0x9e80>;
tee-os {
filename = "tee-raw.bin";
+   optional;
};
};
 
-- 
2.40.1



Re: [PATCH v3] mtd: rawnand: Meson NAND controller support

2024-02-12 Thread Michael Nazzareno Trimarchi
   }
> +
> +   meson_chip->nsels = nsels;
> +   nand = _chip->nand;
> +
> +   nand->flash_node = node;
> +   nand_set_controller_data(nand, nfc);
> +   /* Set the driver entry points for MTD */
> +   nand->cmdfunc = meson_nfc_nand_cmd_function;
> +   nand->cmd_ctrl = meson_nfc_cmd_ctrl;
> +   nand->select_chip = meson_nfc_nand_select_chip;
> +   nand->read_byte = meson_nfc_nand_read_byte;
> +   nand->write_byte = meson_nfc_nand_write_byte;
> +   nand->dev_ready = meson_nfc_dev_ready;
> +
> +   /* Buffer read/write routines */
> +   nand->read_buf = meson_nfc_read_buf;
> +   nand->write_buf = meson_nfc_write_buf;
> +   nand->options |= NAND_NO_SUBPAGE_WRITE;
> +
> +   nand->ecc.mode = NAND_ECC_HW;
> +   nand->ecc.hwctl = NULL;
> +   nand->ecc.read_page = meson_nfc_read_page_hwecc;
> +   nand->ecc.write_page = meso

Re: [PATCH] lib: sparse: Fix error checking for write_sparse_chunk_raw

2024-02-01 Thread Michael Nazzareno Trimarchi
On Thu, Feb 1, 2024 at 7:19 PM Sean Anderson  wrote:
>
> The return value of write_sparse_chunk_raw is unsigned, so the existing
> check has no effect. Use IS_ERR_VALUE to detect error instead, which is
> what write_sparse_chunk_raw does itself.
>
> Fixes: 62649165cb0 ("lib: sparse: Make CHUNK_TYPE_RAW buffer aligned")
> Reported-by: Dan Carpenter 
> Link: 
> https://lore.kernel.org/u-boot/1b323ec3-59b0-490b-a2f0-fd961dafcf49@moroto.mountain/
> Signed-off-by: Sean Anderson 
> ---
>
>  lib/image-sparse.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/image-sparse.c b/lib/image-sparse.c
> index f8289064692..09225692e9b 100644
> --- a/lib/image-sparse.c
> +++ b/lib/image-sparse.c
> @@ -211,7 +211,7 @@ int write_sparse_image(struct sparse_storage *info,
>
> blks = write_sparse_chunk_raw(info, blk, blkcnt,
>   data, response);
> -   if (blks < 0)
> +   if (IS_ERR_VALUE(blks))
> return -1;
>
>     blk += blks;
> --
> 2.35.1.1320.gc452695387.dirty
>

Reviewed-by: Michael Trimarchi 

Michael

--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH v2] boot: add support for button commands

2024-01-18 Thread Michael Walle

Using CONFIG_EXTRA_ENV_SETTINGS should be good enough to provide
the fallback defaults.  However, the users can still mess the things 
up,

but again, they can do that already in many places.


I disagree. In my case that is a last resort recovery. And it should
work in any case. Even if the user has messed up anything (except
from erasing the bootloader in the SPI flash ;)).


Maybe the solution could be another compile-time option to "lock down"
the built-in defaults provided through CONFIG_EXTRA_ENV_SETTINGS?  If
that new option is selected, changes to the environment would make no
changes to the built-in defaults, i.e. those parts of the environment
would actually be ignored.


Not sure locking down the whole environment is a good idea.


In summary, the registered (compiled-in) command should always take
precedence. If one wants to supply a default command which can be
changed later, that can go via the (compiled-in) default 
environment.


Sorry, this is a bit confusing to me.  Didn't you write above that
the users should be able to change the associated commands through
the environment variables?


I had two kinds of button commands in mind: immutable ones and mutable
ones. The first can be achieved with compiled-in commands, the second
with a default environment and environment variables.

Also, whether a command is a mutable one or not is the decision of
the developer (or the one who's compiling/configuring u-boot),
not the user.


I believe that the additional compile-time option, which I proposed
above, could be extended to specify which of the built-in default
button-command associations are immutable, and which are allowed to
be modified through the environment variables.


IIRC there is already a mechanism for that. Environment hooks
or something like that. But I'm not sure that has other implications
and qualify as simple and lightweight for this use-case.

Anyway, we digress. I just wanted to make you aware of another
use-case, which btw. is already done today in the lsxl board for
example.

-michael


Re: [PATCH v2] boot: add support for button commands

2024-01-17 Thread Michael Walle

Hi,


Using CONFIG_EXTRA_ENV_SETTINGS should be good enough to provide
the fallback defaults.  However, the users can still mess the things 
up,

but again, they can do that already in many places.


I disagree. In my case that is a last resort recovery. And it should
work in any case. Even if the user has messed up anything (except
from erasing the bootloader in the SPI flash ;)).


In summary, the registered (compiled-in) command should always take
precedence. If one wants to supply a default command which can be
changed later, that can go via the (compiled-in) default environment.


Sorry, this is a bit confusing to me.  Didn't you write above that
the users should be able to change the associated commands through
the environment variables?


I had two kinds of button commands in mind: immutable ones and mutable
ones. The first can be achieved with compiled-in commands, the second
with a default environment and environment variables.

Also, whether a command is a mutable one or not is the decision of
the developer (or the one who's compiling/configuring u-boot),
not the user.

-michael


Re: [PATCH v2 07/14] mtdparts: use negative error codes

2024-01-12 Thread Michael Nazzareno Trimarchi
gt;  {
> @@ -1493,7 +1493,7 @@ static int spread_partitions(void)
> dev = list_entry(dentry, struct mtd_device, link);
>
> if (get_mtd_info(dev->id->type, dev->id->num, ))
> -   return 1;
> +   return -EINVAL;
>
> part_num = 0;
> cur_offs = 0;
> @@ -1519,7 +1519,7 @@ static int spread_partitions(void)
>
> if (generate_mtdparts_save(last_parts, MTDPARTS_MAXLEN) != 0) {
> printf("generated mtdparts too long, resetting to null\n");
> -   return 1;
> +   return -EINVAL;
> }
> return 0;
>  }
> @@ -1547,7 +1547,7 @@ static const char *env_get_mtdparts(char *buf)
>   * for each entry. Add created devices to the global devices list.
>   *
>   * @param mtdparts string specifing mtd partitions
> - * Return: 0 on success, 1 otherwise
> + * Return: 0 on success, -errno otherwise
>   */
>  static int parse_mtdparts(const char *const mtdparts)
>  {
> @@ -1605,7 +1605,7 @@ static int parse_mtdparts(const char *const mtdparts)
>   * to the global mtdids list.
>   *
>   * @param ids mapping string
> - * Return: 0 on success, 1 otherwise
> + * Return: 0 on success, -errno otherwise
>   */
>  static int parse_mtdids(const char *const ids)
>  {
> @@ -1646,7 +1646,7 @@ static int parse_mtdids(const char *const ids)
>
> /* check if requested device exists */
> if (mtd_device_validate(type, num, ) != 0)
> -   return 1;
> +   return -EINVAL;
>
> /* locate  */
> mtd_id = p;
> @@ -1704,7 +1704,7 @@ static int parse_mtdids(const char *const ids)
> list_del(entry);
> free(id_tmp);
> }
> -   return 1;
> +   return -EINVAL;
> }
>
> return 0;
> @@ -1715,7 +1715,7 @@ static int parse_mtdids(const char *const ids)
>   * Parse and initialize global mtdids mapping and create global
>   * device/partition list.
>   *
> - * Return: 0 on success, 1 otherwise
> + * Return: 0 on success, -errno otherwise
>   */
>  int mtdparts_init(void)
>  {
> @@ -1768,12 +1768,12 @@ int mtdparts_init(void)
> env_set("mtdids", (char *)ids);
>         } else {
> printf("mtdids not defined, no default present\n");
> -   return 1;
> +   return -ENXIO;
> }
> }
> if (strlen(ids) > MTDIDS_MAXLEN - 1) {
> printf("mtdids too long (> %d)\n", MTDIDS_MAXLEN);
> -   return 1;
> +   return -EINVAL;
> }
>
> /* use defaults when mtdparts variable is not defined
> @@ -1789,7 +1789,7 @@ int mtdparts_init(void)
>
> if (parts && (strlen(parts) > MTDPARTS_MAXLEN - 1)) {
> printf("mtdparts too long (> %d)\n", MTDPARTS_MAXLEN);
> -   return 1;
> +   return -EINVAL;
> }
>
> /* check if we have already parsed those mtdids */
> @@ -1800,7 +1800,7 @@ int mtdparts_init(void)
>
> if (parse_mtdids(ids) != 0) {
> mtd_devices_init();
> -   return 1;
> +   return -EINVAL;
> }
>
> /* ok it's good, save new ids */
> @@ -1810,11 +1810,11 @@ int mtdparts_init(void)
> /* parse partitions if either mtdparts or mtdids were updated */
> if (parts && ((last_parts[0] == '\0') || ((strcmp(last_parts, parts) 
> != 0)) || ids_changed)) {
> if (parse_mtdparts(parts) != 0)
> -   return 1;
> +   return -EINVAL;
>
> if (list_empty()) {
> printf("mtdparts_init: no valid partitions\n");
> -   return 1;
> +   return -ENXIO;
> }
>
> /* ok it's good, save new parts */
> @@ -2022,7 +2022,7 @@ static int do_mtdparts(struct cmd_tbl *cmdtp, int flag, 
> int argc,
> debug("add tmpbuf: %s\n", tmpbuf);
>
> if ((device_parse(tmpbuf, NULL, ) != 0) || (!dev))
> -   return 1;
> +   return -EINVAL;
>
> debug("+ %s\t%d\t%s\n", MTD_DEV_TYPE(dev->id->type),
> dev->id->num, dev->id->mtd_id);
> --
> 2.30.1
>

Reviewed-by: Michael Trimarchi 


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH v2 0/2] mtd: nand: omap_gpmc: Fix NAND for AM335x

2024-01-11 Thread Michael Nazzareno Trimarchi
Hi

Il gio 11 gen 2024, 14:06 Roger Quadros  ha scritto:

> Hi,
>
> On 11/12/2023 13:45, Roger Quadros wrote:
> > Hi,
> >
> > These patches fix NAND and ELM for AM335x and related legacy platforms
> > that use HW BCH and ELM modules.
> >
> > All CI tests pass: https://github.com/u-boot/u-boot/pull/453
> >
> > Changelog:
> >
> > v2:
> > - added __maybe_unused to omap_calculate_ecc_bch. fixes CI tests
> > - Added Tested-by Tags
> > - Explained about omap_elm single sector support in commit log
> >
> > cheers,
> > -roger
> >
> > Roger Quadros (2):
> >   mtd: nand: omap_gpmc: Fix NAND in SPL for AM335x
> >   mtd: rawnand: omap_elm: Fix elm_init definition
> >
> >  drivers/mtd/nand/raw/omap_elm.c  |  4 +-
> >  drivers/mtd/nand/raw/omap_elm.h  |  6 --
> >  drivers/mtd/nand/raw/omap_gpmc.c | 95 ++--
> >  3 files changed, 31 insertions(+), 74 deletions(-)
> >
> >
> > base-commit: 2f0282922b2c458eea7f85c500a948a587437b63
>
> If no objections can this be Acked and picked up please?
> Without these NAND is broken on some TI platforms.
>


We will queue them and send over the weekend

Michael

>
>
>
> Thanks!
>
> --
> cheers,
> -roger
>


Re: [PATCH v2] boot: add support for button commands

2024-01-11 Thread Michael Walle
>> This is simply awesome, but I see one possible issue -- the need to have
>> proper environment variables defined for a particular board or device,
>> to make the buttons work as expected.  Obviously, those environment
>> variables can be absent or can become missing for numerous reasons.
> 
> Is CFG_EXTRA_ENV_SETTINGS not persistent enough?

IMHO no. Because a user might accidentially mess up the environment
variables.

>> I think that we should have an additional mechanism in place that
>> defines the buttons and the associated commands even if no environment
>> variables are found.  Like a set of fallback defaults for a particular
>> board or device, built into the U-Boot image.  For example, Rockchip
>> boards have those defaults pretty well defined.
> 
> A programmatic API for register button/cmd mapping from
> board_late_init() (for example) sounds sensible to me.

I don't know if it has to be that complex, or if it will be enough
to just have some compile-time constants like CONFIG_BUTTON_CMD_N.

> Is this really an issue that invalidates the implementation proposed
> here though? It feels much more like a nice-to-have addition that maybe
> we could leave out for now?

Agreed. Looks like one can add it to get_button_cmd() later.

> It also has a MUCH wider scope imo - should board override env or vice
> versa? What about triggering default AND custom actions for one button
> press? What if a board wants to register a callback function instead of
> running a command?) - these are questions I don't want to answer with
> this patch.

One use case I have is restoring the default environment *in any case*;
regardless what the state of the board is. In this case, the environment
must not override the button settings. This can be a compiled-in
command, which always takes precedence.

If you want to be able to overwrite a button command, then I guess you
can already do that with the environment setting. Provide sane defaults
via CFG_EXTRA_ENV_SETTINGS and a user can then overwrite it.

In summary, the registered (compiled-in) command should always take
precedence. If one wants to supply a default command which can be
changed later, that can go via the (compiled-in) default environment.

-michael


Re: [PATCH v2] mtd: rawnand: Meson NAND controller support

2024-01-09 Thread Michael Nazzareno Trimarchi
e;
> +   nand->write_byte = meson_nfc_nand_write_byte;
> +   nand->dev_ready = meson_nfc_dev_ready;
> +
> +   /* Buffer read/write routines */
> +   nand->read_buf = meson_nfc_read_buf;
> +   nand->write_buf = meson_nfc_write_buf;
> +   nand->options |= NAND_NO_SUBPAGE_WRITE;
> +
> +   nand->ecc.mode = NAND_ECC_HW;
> +   nand->ecc.hwctl = NULL;
> +   nand->ecc.read_page = meson_nfc_read_page_hwecc;
> +   nand->ecc.write_page = meson_nfc_write_page_hwecc;
> +   nand->ecc.read_page_raw = meson_nfc_read_page_raw;
> +   nand->ecc.write_page_raw = meson_nfc_write_page_raw;
> +
> +   nand->ecc.read_oob = meson_nfc_read_oob;
> +   nand->ecc.write_oob = meson_nfc_write_oob;
> +   nand->ecc.read_oob_raw = meson_nfc_read_oob_raw;
> +   nand->ecc.write_oob_raw = meson_nfc_write_oob_raw;
> +
> +   nand->ecc.algo = NAND_ECC_BCH;
> +
> +   mtd = nand_to_mtd(nand);
> +
> +   ret = nand_scan_ident(mtd, 1, NULL);
> +   if (ret) {
> +   dev_err(dev, "'nand_scan_ident()' failed: %d\n", ret);
> +   goto err_chip_free;
> +   }
> +
> +   ret = meson_nfc_init_ecc(nand, node);
> +   if (ret) {
> +   dev_err(dev, "failed to init ECC settings: %d\n", ret);
> +   goto err_chip_free;
> +   }
> +
> +   ret = meson_chip_buffer_init(nand);
> +   if (ret) {
> +   dev_err(dev, "failed to init DMA buffers: %d\n", ret);
> +   goto err_chip_free;
> +   }
> +
> +   /* 'nand_scan_tail()' needs ECC parameters to be already
> +* set and correct.
> +*/

Can you split in
/*
 *
?

> +   ret = nand_scan_tail(mtd);
> +   if (ret) {
> +   dev_err(dev, "'nand_scan_tail()' failed: %d\n", ret);
> +   goto err_chip_buf_free;
> +   }
> +
> +   ret = nand_register(0, mtd);
> +   if (ret) {
> +   dev_err(dev, "'nand_register()' failed: %d\n", ret);
> +   goto err_chip_buf_free;
> +   }
> +
> +   list_add_tail(_chip->node, >chips);
> +
> +   return 0;
> +
> +err_chip_buf_free:
> +   dma_free_coherent(meson_chip->info_buf);
> +   dma_free_coherent(meson_chip->data_buf);
> +
> +err_chip_free:
> +   free(meson_chip);
> +
> +   return ret;
> +}
> +
> +static int meson_nfc_nand_chips_init(struct udevice *dev,
> +struct meson_nfc *nfc)
> +{
> +   ofnode parent = dev_ofnode(dev);
> +   ofnode node;
> +
> +   ofnode_for_each_subnode(node, parent) {
> +   int ret = meson_nfc_nand_chip_init(dev, nfc, node);
> +
> +   if (ret)
> +   return ret;
> +   }
> +
> +   return 0;
> +}
> +
> +static void meson_nfc_clk_init(struct meson_nfc *nfc)
> +{
> +   u32 bus_cycle = NFC_DEFAULT_BUS_CYCLE;
> +   u32 bus_timing = NFC_DEFAULT_BUS_TIMING;
> +   u32 bus_cfg_val;
> +
> +   writel(CLK_ALWAYS_ON_NAND | CLK_SELECT_NAND | CLK_ENABLE_VALUE, 
> nfc->reg_clk);
> +   writel(0, nfc->reg_base + NFC_REG_CFG);
> +
> +   bus_cfg_val = (((bus_cycle - 1) & 31) | ((bus_timing & 31) << 5));
> +   writel(bus_cfg_val, nfc->reg_base + NFC_REG_CFG);
> +   writel(BIT(31), nfc->reg_base + NFC_REG_CMD);
> +}
> +
> +static int meson_probe(struct udevice *dev)
> +{
> +   struct meson_nfc *nfc = dev_get_priv(dev);
> +   void *addr;
> +   int ret;
> +
> +   addr = dev_read_addr_ptr(dev);
> +   if (!addr) {
> +   dev_err(dev, "base register address not found\n");
> +   return -EINVAL;
> +   }
> +
> +   nfc->reg_base = addr;
> +
> +   addr = dev_read_addr_index_ptr(dev, 1);
> +   if (!addr) {
> +   dev_err(dev, "clk register address not found\n");
> +   return -EINVAL;
> +   }
> +
> +   nfc->reg_clk = addr;
> +   nfc->dev = dev;
> +
> +   meson_nfc_clk_init(nfc);
> +
> +   ret = meson_nfc_nand_chips_init(dev, nfc);
> +   if (ret) {
> +   dev_err(nfc->dev, "failed to init chips\n");
> +   return ret;
> +   }
> +
> +   return 0;
> +}
> +
> +static const struct udevice_id meson_nand_dt_ids[] = {
> +   {.compatible = "amlogic,meson-axg-nfc",},
> +   { /* sentinel */ }
> +};
> +
> +U_BOOT_DRIVER(meson_nand) = {
> +   .name = "meson_nand",
> +   .id = UCLASS_MTD,
> +   .of_match = meson_nand_dt_ids,
> +   .probe = meson_probe,
> +   .priv_auto = sizeof(struct meson_nfc),
> +};
> +
> +void board_nand_init(void)
> +{
> +   struct udevice *dev;
> +   int ret;
> +
> +   ret = uclass_get_device_by_driver(UCLASS_MTD,
> + DM_DRIVER_GET(meson_nand), );
> +
> +   if (ret && ret != -ENODEV)
> +   pr_err("Failed to initialize: %d\n", ret);
> +}
> --
> 2.35.0
>


--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH v2] mtd: rawnand: Meson NAND controller support

2024-01-08 Thread Michael Nazzareno Trimarchi
Hi

On Mon, Jan 8, 2024 at 7:41 PM Arseniy Krasnov
 wrote:
>
> Sorry, pls ping
>

Sorry to be late, ;) I will give a review tomorrow

Michael

> Thanks, Arseniy
>
> On 15.12.2023 15:23, Arseniy Krasnov wrote:
> > Basic support for Amlogic Meson NAND controller on AXG.
> >
> > Based on Linux version 6.7.0-rc4.
> >
> > Signed-off-by: Arseniy Krasnov 
> > ---
> >  Changelog:
> >  v1 -> v2:
> >   * Update commit message with 'Based on Linux ...'.
> >   * Add Linux driver author to .c file header.
> >   * Add comment for defines 'NFC_DEFAULT_BUS_CYCLE' and
> > 'NFC_DEFAULT_BUS_TIMING'.
> >   * Use 'dev_read_addr_index_ptr()' instead of 'dev_read_addr()'.
> >
> >  drivers/mtd/nand/raw/Kconfig  |9 +
> >  drivers/mtd/nand/raw/Makefile |1 +
> >  drivers/mtd/nand/raw/meson_nand.c | 1241 +
> >  3 files changed, 1251 insertions(+)
> >  create mode 100644 drivers/mtd/nand/raw/meson_nand.c
> >
> > diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> > index d624589a89..7b7b0226ab 100644
> > --- a/drivers/mtd/nand/raw/Kconfig
> > +++ b/drivers/mtd/nand/raw/Kconfig
> > @@ -488,6 +488,15 @@ config NAND_ARASAN
> > controller. This uses the hardware ECC for read and
> > write operations.
> >
> > +config NAND_MESON
> > + bool "Meson NAND support"
> > + select SYS_NAND_SELF_INIT
> > + depends on DM_MTD && ARCH_MESON
> > + imply CMD_NAND
> > + help
> > +   This enables Nand driver support for Meson raw NAND flash
> > +   controller.
> > +
> >  config NAND_MXC
> >   bool "MXC NAND support"
> >   depends on CPU_ARM926EJS || CPU_ARM1136 || MX5
> > diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
> > index add2b4cf65..5b4efd52c9 100644
> > --- a/drivers/mtd/nand/raw/Makefile
> > +++ b/drivers/mtd/nand/raw/Makefile
> > @@ -61,6 +61,7 @@ obj-$(CONFIG_NAND_KMETER1) += kmeter1_nand.o
> >  obj-$(CONFIG_NAND_LPC32XX_MLC) += lpc32xx_nand_mlc.o
> >  obj-$(CONFIG_NAND_LPC32XX_SLC) += lpc32xx_nand_slc.o
> >  obj-$(CONFIG_NAND_VF610_NFC) += vf610_nfc.o
> > +obj-$(CONFIG_NAND_MESON) += meson_nand.o
> >  obj-$(CONFIG_NAND_MXC) += mxc_nand.o
> >  obj-$(CONFIG_NAND_MXS) += mxs_nand.o
> >  obj-$(CONFIG_NAND_MXS_DT) += mxs_nand_dt.o
> > diff --git a/drivers/mtd/nand/raw/meson_nand.c 
> > b/drivers/mtd/nand/raw/meson_nand.c
> > new file mode 100644
> > index 00..a6dbe99d84
> > --- /dev/null
> > +++ b/drivers/mtd/nand/raw/meson_nand.c
> > @@ -0,0 +1,1241 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Amlogic Meson Nand Flash Controller Driver
> > + *
> > + * Copyright (c) 2018 Amlogic, inc.
> > + * Author: Liang Yang 
> > + *
> > + * Copyright (c) 2023 SaluteDevices, Inc.
> > + * Author: Arseniy Krasnov 
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define NFC_CMD_IDLE (0xc << 14)
> > +#define NFC_CMD_CLE  (0x5 << 14)
> > +#define NFC_CMD_ALE  (0x6 << 14)
> > +#define NFC_CMD_DWR  (0x4 << 14)
> > +#define NFC_CMD_DRD  (0x8 << 14)
> > +#define NFC_CMD_ADL  ((0 << 16) | (3 << 20))
> > +#define NFC_CMD_ADH  ((1 << 16) | (3 << 20))
> > +#define NFC_CMD_AIL  ((2 << 16) | (3 << 20))
> > +#define NFC_CMD_AIH  ((3 << 16) | (3 << 20))
> > +#define NFC_CMD_SEED ((8 << 16) | (3 << 20))
> > +#define NFC_CMD_M2N  ((0 << 17) | (2 << 20))
> > +#define NFC_CMD_N2M  ((1 << 17) | (2 << 20))
> > +#define NFC_CMD_RB   BIT(20)
> > +#define NFC_CMD_SCRAMBLER_ENABLE BIT(19)
> > +#define NFC_CMD_SCRAMBLER_DISABLE0
> > +#define NFC_CMD_SHORTMODE_DISABLE0
> > +#define NFC_CMD_RB_INT   BIT(14)
> > +#define NFC_CMD_RB_INT_NO_PIN((0xb << 10) | BIT(18) | 
> > BIT(16))
> > +
> > +#define NFC_CMD_GET_SIZE(x)  (((x) >> 22) & GENMASK(4,

[RFC PATCH V2] net: wget: Take in account tcp sequence number wrap around

2024-01-07 Thread Michael Trimarchi
Coming from some discussion on mailing about wget unconsistent. It's
just and idea to play around

Signed-off-by: Michael Trimarchi 
---
RFC V1 -> RFC V2:
- drop random train change in unsigned difference

---
 include/net/tcp.h | 10 ++
 net/wget.c|  2 +-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index c29d4ce24a..584813b637 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -88,6 +88,16 @@ struct ip_tcp_hdr {
 #define TCP_MSS1460/* Max segment size 
*/
 #define TCP_SCALE  0x01/* Scale*/
 
+/*
+ * The next routines deal with comparing 32 bit unsigned ints
+ * and worry about wraparound (automatic with unsigned arithmetic).
+ */
+static inline bool before(__u32 seq1, __u32 seq2)
+{
+   return (__s32)(seq1-seq2) < 0;
+}
+#define after(seq2, seq1)  before(seq1, seq2)
+
 /**
  * struct tcp_mss - TCP option structure for MSS (Max segment size)
  * @kind: Field ID
diff --git a/net/wget.c b/net/wget.c
index 8bb4d72db1..020503b763 100644
--- a/net/wget.c
+++ b/net/wget.c
@@ -333,7 +333,7 @@ static void wget_handler(uchar *pkt, u16 dport,
   "wget: Transferring, seq=%x, ack=%x,len=%x\n",
   tcp_seq_num, tcp_ack_num, len);
 
-   if (tcp_seq_num >= initial_data_seq_num &&
+   if (!before(initial_data_seq_num, tcp_seq_num) &&
store_block(pkt, tcp_seq_num - initial_data_seq_num,
len) != 0) {
wget_fail("wget: store error\n",
-- 
2.40.1



Re: inconsistent wget behavior

2024-01-07 Thread Michael Nazzareno Trimarchi
Hi Fabio

On Sun, Jan 7, 2024 at 5:19 PM Michael Nazzareno Trimarchi
 wrote:
>
> Hi
>
> Il dom 7 gen 2024, 17:08 Fabio Estevam  ha scritto:
>>
>> Hi Michael,
>>
>> On Sat, Jan 6, 2024 at 5:49 AM Michael Nazzareno Trimarchi
>>  wrote:
>> >
>> > Hi
>> >
>> > Is this code correct?
>> >
>> > if (tcp_seq_num >= initial_data_seq_num &&
>> > store_block(pkt, tcp_seq_num - initial_data_seq_num,
>> > len) != 0) {
>> > wget_fail("wget: store error\n",
>> >   tcp_seq_num, tcp_ack_num, action);
>> > return;
>> > }
>> >
>> > Can not be seq_num wrap around? Another point seems that is not
>> > guarantee packet reassembly
>>
>> If you submit a patch, I will be glad to test it.
>>
>> Cheers
>
>
> I have already wrote something but I can not test. Will send tonight. My 
> feeling is that sometime the initial sequence number of TCP ip is next to 
> wrap around but not sure.
>

I have sent but not sure about it, just compile for now ;)

> Michael



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


[RFC PATCH] net: wget: Take in account tcp sequence number wrap around

2024-01-07 Thread Michael Trimarchi
Coming from some discussion on mailing about wget unconsistent. It's
just and idea to play around

Signed-off-by: Michael Trimarchi 
---

I'm on a train ;). No board to test

---
 include/net/tcp.h | 10 ++
 net/wget.c|  8 
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index c29d4ce24a..584813b637 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -88,6 +88,16 @@ struct ip_tcp_hdr {
 #define TCP_MSS1460/* Max segment size 
*/
 #define TCP_SCALE  0x01/* Scale*/
 
+/*
+ * The next routines deal with comparing 32 bit unsigned ints
+ * and worry about wraparound (automatic with unsigned arithmetic).
+ */
+static inline bool before(__u32 seq1, __u32 seq2)
+{
+   return (__s32)(seq1-seq2) < 0;
+}
+#define after(seq2, seq1)  before(seq1, seq2)
+
 /**
  * struct tcp_mss - TCP option structure for MSS (Max segment size)
  * @kind: Field ID
diff --git a/net/wget.c b/net/wget.c
index 8bb4d72db1..99a688846f 100644
--- a/net/wget.c
+++ b/net/wget.c
@@ -260,8 +260,8 @@ static void wget_connected(uchar *pkt, unsigned int 
tcp_seq_num,
(phys_addr_t)(pkt_q[i].pkt),
pkt_q[i].len);
store_block(ptr1,
-   pkt_q[i].tcp_seq_num -
-   initial_data_seq_num,
+   (unsigned 
int)((s32)pkt_q[i].tcp_seq_num -
+   (s32)initial_data_seq_num),
pkt_q[i].len);
unmap_sysmem(ptr1);
debug_cond(DEBUG_WGET,
@@ -333,8 +333,8 @@ static void wget_handler(uchar *pkt, u16 dport,
   "wget: Transferring, seq=%x, ack=%x,len=%x\n",
   tcp_seq_num, tcp_ack_num, len);
 
-   if (tcp_seq_num >= initial_data_seq_num &&
-   store_block(pkt, tcp_seq_num - initial_data_seq_num,
+   if (!before(initial_data_seq_num, tcp_seq_num) &&
+   store_block(pkt, (unsigned int)((s32)tcp_seq_num - 
(s32)initial_data_seq_num),
len) != 0) {
wget_fail("wget: store error\n",
  tcp_seq_num, tcp_ack_num, action);
-- 
2.40.1



Re: inconsistent wget behavior

2024-01-07 Thread Michael Nazzareno Trimarchi
Hi

Il dom 7 gen 2024, 17:08 Fabio Estevam  ha scritto:

> Hi Michael,
>
> On Sat, Jan 6, 2024 at 5:49 AM Michael Nazzareno Trimarchi
>  wrote:
> >
> > Hi
> >
> > Is this code correct?
> >
> > if (tcp_seq_num >= initial_data_seq_num &&
> > store_block(pkt, tcp_seq_num - initial_data_seq_num,
> > len) != 0) {
> > wget_fail("wget: store error\n",
> >   tcp_seq_num, tcp_ack_num, action);
> > return;
> > }
> >
> > Can not be seq_num wrap around? Another point seems that is not
> > guarantee packet reassembly
>
> If you submit a patch, I will be glad to test it.
>
> Cheers
>

I have already wrote something but I can not test. Will send tonight. My
feeling is that sometime the initial sequence number of TCP ip is next to
wrap around but not sure.

Michael

>


Re: inconsistent wget behavior

2024-01-06 Thread Michael Nazzareno Trimarchi
Hi

On Sat, Jan 6, 2024 at 9:49 AM Michael Nazzareno Trimarchi
 wrote:
>
> Hi
>
> Is this code correct?
>
> if (tcp_seq_num >= initial_data_seq_num &&
> store_block(pkt, tcp_seq_num - initial_data_seq_num,
> len) != 0) {
> wget_fail("wget: store error\n",
>   tcp_seq_num, tcp_ack_num, action);
> return;
> }
>
> Can not be seq_num wrap around? Another point seems that is not
> guarantee packet reassembly
>

And what happen if you are going to generate out of sequence packet
and with large seq_number on purpose?

I think that lwip has much more sense

Michael

> Michael
>
> On Fri, Jan 5, 2024 at 8:53 PM Fabio Estevam  wrote:
> >
> > On Fri, Jan 5, 2024 at 4:49 PM Michael Nazzareno Trimarchi
> >  wrote:
> >
> > > I was thinking that was lwip integration
> >
> > That's a different issue.
> >
> > If you want to test lwip integration, you can try it from:
> > https://github.com/muvarov/u-boot/tree/master_lwip_test_v10
>
>
>
> --
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> mich...@amarulasolutions.com
> ______
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> i...@amarulasolutions.com
> www.amarulasolutions.com



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: inconsistent wget behavior

2024-01-06 Thread Michael Nazzareno Trimarchi
Hi

Is this code correct?

if (tcp_seq_num >= initial_data_seq_num &&
store_block(pkt, tcp_seq_num - initial_data_seq_num,
len) != 0) {
wget_fail("wget: store error\n",
  tcp_seq_num, tcp_ack_num, action);
return;
}

Can not be seq_num wrap around? Another point seems that is not
guarantee packet reassembly

Michael

On Fri, Jan 5, 2024 at 8:53 PM Fabio Estevam  wrote:
>
> On Fri, Jan 5, 2024 at 4:49 PM Michael Nazzareno Trimarchi
>  wrote:
>
> > I was thinking that was lwip integration
>
> That's a different issue.
>
> If you want to test lwip integration, you can try it from:
> https://github.com/muvarov/u-boot/tree/master_lwip_test_v10



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: inconsistent wget behavior

2024-01-05 Thread Michael Nazzareno Trimarchi
Hi Fabio

On Fri, Jan 5, 2024 at 8:32 PM Fabio Estevam  wrote:
>
> Hi Michael,
>
> On Fri, Jan 5, 2024 at 4:12 PM Michael Nazzareno Trimarchi
>  wrote:
>
> > Can you reproduce with dcache off?
>
> I haven't tried it.
>
> > Where are the patches to test?
>
> The wget issue can be reproduced with U-Boot master.
>
> No need for extra patches. Please see the first message of this thread
> where Tim explains how to reproduce it.

I was thinking that was lwip integration

Michael

-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: inconsistent wget behavior

2024-01-05 Thread Michael Nazzareno Trimarchi
Hi

On Fri, Jan 5, 2024 at 7:57 PM Paul Liu  wrote:
>
> Hi Fabio,
>
> I tried to investigate this by U-boot sandbox. But it seems to me that I
> cannot reproduce this issue.
> I put a file on localhost apache server and tried to download it from
> localhost.
> I might need a more persistent way to reproduce this bug.
>

Can you reproduce with dcache off?

Where are the patches to test?

Michael


Re: [PATCH] cmd: sf: Fix sf probe crash

2024-01-04 Thread Michael Nazzareno Trimarchi
Hi

On Thu, Jan 4, 2024 at 1:40 PM Weizhao Ouyang  wrote:
>
> On Thu, Jan 4, 2024 at 8:29 PM Michael Nazzareno Trimarchi
>  wrote:
> >
> > Hi
> >
> > On Thu, Jan 4, 2024 at 1:16 PM Weizhao Ouyang  wrote:
> > >
> > > On Thu, Jan 4, 2024 at 8:00 PM Michal Simek  wrote:
> > > >
> > > >
> > > >
> > > > On 1/4/24 12:46, Weizhao Ouyang wrote:
> > > > > Handle the return value of spi_flash_probe_bus_cs() to avoid sf probe
> > > > > crashes.
> > > > >
> > > > > Signed-off-by: Weizhao Ouyang 
> > > > > ---
> > > > >   cmd/sf.c | 5 +++--
> > > > >   1 file changed, 3 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/cmd/sf.c b/cmd/sf.c
> > > > > index 730996c02b..e3866899f6 100644
> > > > > --- a/cmd/sf.c
> > > > > +++ b/cmd/sf.c
> > > > > @@ -135,8 +135,9 @@ static int do_spi_flash_probe(int argc, char 
> > > > > *const argv[])
> > > > >   }
> > > > >   flash = NULL;
> > > > >   if (use_dt) {
> > > > > - spi_flash_probe_bus_cs(bus, cs, );
> > > > > - flash = dev_get_uclass_priv(new);
> > > > > + ret = spi_flash_probe_bus_cs(bus, cs, );
> > > >
> > > > if (ret)
> > > > return ret;
> > > >
> > > > don't you want to rather propagate that error?
> > > >
> > >
> > > Well, since the spi_flash is empty, the following code will
> > > print the error message and return.
> > >
> >
> > flash = NULL;
> >
> >   if (!flash) {
> > printf("Failed to initialize SPI flash at %u:%u (error 
> > %d)\n",
> >bus, cs, ret);
> > return 1;
> >   }
> >
> > This code does not change error handling that is already present here.
>
> Hi Michael,
>
> Sorry I didn't get your point. This patch is designed to avoid
> spi_flash_probe_bus_cs() crash by null pointer, and let the
> current existing error handling to prompt it. So I'm not
> trying to change the current error handling.
>

I have said exactly the same so I agree with you.

Michael

> BR,
> Weizhao
>
> > Michael
> >
> > > BR,
> > > Weizhao
> >
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > mich...@amarulasolutions.com
> > __
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > i...@amarulasolutions.com
> > www.amarulasolutions.com



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH] cmd: sf: Fix sf probe crash

2024-01-04 Thread Michael Nazzareno Trimarchi
Hi

On Thu, Jan 4, 2024 at 1:16 PM Weizhao Ouyang  wrote:
>
> On Thu, Jan 4, 2024 at 8:00 PM Michal Simek  wrote:
> >
> >
> >
> > On 1/4/24 12:46, Weizhao Ouyang wrote:
> > > Handle the return value of spi_flash_probe_bus_cs() to avoid sf probe
> > > crashes.
> > >
> > > Signed-off-by: Weizhao Ouyang 
> > > ---
> > >   cmd/sf.c | 5 +++--
> > >   1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/cmd/sf.c b/cmd/sf.c
> > > index 730996c02b..e3866899f6 100644
> > > --- a/cmd/sf.c
> > > +++ b/cmd/sf.c
> > > @@ -135,8 +135,9 @@ static int do_spi_flash_probe(int argc, char *const 
> > > argv[])
> > >   }
> > >   flash = NULL;
> > >   if (use_dt) {
> > > - spi_flash_probe_bus_cs(bus, cs, );
> > > - flash = dev_get_uclass_priv(new);
> > > + ret = spi_flash_probe_bus_cs(bus, cs, );
> >
> > if (ret)
> > return ret;
> >
> > don't you want to rather propagate that error?
> >
>
> Well, since the spi_flash is empty, the following code will
> print the error message and return.
>

flash = NULL;

  if (!flash) {
    printf("Failed to initialize SPI flash at %u:%u (error %d)\n",
   bus, cs, ret);
return 1;
  }

This code does not change error handling that is already present here.

Michael

> BR,
> Weizhao



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH] cmd: sf: Fix sf probe crash

2024-01-04 Thread Michael Nazzareno Trimarchi
Hi

On Thu, Jan 4, 2024 at 12:46 PM Weizhao Ouyang  wrote:
>
> Handle the return value of spi_flash_probe_bus_cs() to avoid sf probe
> crashes.
>
> Signed-off-by: Weizhao Ouyang 
> ---
>  cmd/sf.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/cmd/sf.c b/cmd/sf.c
> index 730996c02b..e3866899f6 100644
> --- a/cmd/sf.c
> +++ b/cmd/sf.c
> @@ -135,8 +135,9 @@ static int do_spi_flash_probe(int argc, char *const 
> argv[])
> }
> flash = NULL;
> if (use_dt) {
> -   spi_flash_probe_bus_cs(bus, cs, );
> -   flash = dev_get_uclass_priv(new);
> +   ret = spi_flash_probe_bus_cs(bus, cs, );
> +   if (!ret)
> +   flash = dev_get_uclass_priv(new);
> } else {
> flash = spi_flash_probe(bus, cs, speed, mode);
> }
> --
> 2.39.2
>

Reviewed-by: Michael Trimarchi 

Suggest to add documentation of spi_flash_probe_bus_cs in a separated patch

Michael


Re: [PATCH v1 07/12] mtdparts: use negative error codes

2024-01-03 Thread Michael Nazzareno Trimarchi
CMD_RET_FAILURE;
> }
>
> if (find_dev_and_part(argv[1], , , ) != 0)
> -   return 1;
> +   return CMD_RET_FAILURE;
>
> current_mtd_dev = dev;
> current_mtd_partnum = pnum;
> @@ -1978,7 +1978,7 @@ static int do_mtdparts(struct cmd_tbl *cmdtp, int flag, 
> int argc,
>
> /* make sure we are in sync with env variables */
> if (mtdparts_init() != 0)
> -   return 1;
> +   return CMD_RET_FAILURE;
>
> if (argc == 1) {
> list_partitions();
> @@ -2000,11 +2000,11 @@ static int do_mtdparts(struct cmd_tbl *cmdtp, int 
> flag, int argc,
> struct part_info *p;
>
> if (mtd_id_parse(argv[2], NULL, , ) != 0)
> -   return 1;
> +   return CMD_RET_FAILURE;
>
> if ((id = id_find(type, num)) == NULL) {
> printf("no such device %s defined in mtdids 
> variable\n", argv[2]);
> -   return 1;
> +   return CMD_RET_FAILURE;
> }
>
> len = strlen(id->mtd_id) + 1;   /* 'mtd_id:' */
> @@ -2015,14 +2015,14 @@ static int do_mtdparts(struct cmd_tbl *cmdtp, int 
> flag, int argc,
>
> if (len >= PART_ADD_DESC_MAXLEN) {
> printf("too long partition description\n");
> -   return 1;
> +   return CMD_RET_FAILURE;
> }
> sprintf(tmpbuf, "%s:%s(%s)%s",
> id->mtd_id, argv[3], argv[4], argv[5] ? 
> argv[5] : "");
> debug("add tmpbuf: %s\n", tmpbuf);
>
> if ((device_parse(tmpbuf, NULL, ) != 0) || (!dev))
> -   return 1;
> +   return -EINVAL;
>
> debug("+ %s\t%d\t%s\n", MTD_DEV_TYPE(dev->id->type),
> dev->id->num, dev->id->mtd_id);
> @@ -2031,7 +2031,7 @@ static int do_mtdparts(struct cmd_tbl *cmdtp, int flag, 
> int argc,
>
>  #if defined(CONFIG_CMD_MTDPARTS_SPREAD)
> if (get_mtd_info(dev->id->type, dev->id->num, ))
> -   return 1;
> +   return CMD_RET_FAILURE;
>
> if (!strcmp([1][3], ".spread")) {
> spread_partition(mtd, p, _offset);
> @@ -2045,12 +2045,12 @@ static int do_mtdparts(struct cmd_tbl *cmdtp, int 
> flag, int argc,
> } else if (part_add(dev_tmp, p) != 0) {
> /* merge new partition with existing ones*/
> device_del(dev);
> -   return 1;
> +   return CMD_RET_FAILURE;
> }
>
> if (generate_mtdparts_save(last_parts, MTDPARTS_MAXLEN) != 0) 
> {
> printf("generated mtdparts too long, resetting to 
> null\n");
> -   return 1;
> +   return CMD_RET_FAILURE;
> }
>
> return 0;
> --
> 2.30.1
>

Reviewed-by: Michael Trimarchi 


Re: [PATCH v1 04/12] nand: move nand_erase_opts() to core NAND folder

2024-01-03 Thread Michael Nazzareno Trimarchi
e_length = lldiv(opts->length + mtd->erasesize - 1,
> +mtd->erasesize);
> +
> +   cleanmarker.magic = cpu_to_je16(JFFS2_MAGIC_BITMASK);
> +   cleanmarker.nodetype = cpu_to_je16(JFFS2_NODETYPE_CLEANMARKER);
> +   cleanmarker.totlen = cpu_to_je32(8);
> +
> +   /* scrub option allows to erase badblock. To prevent internal
> +* check from erase() method, set block check method to dummy
> +* and disable bad block table while erasing.
> +*/
> +   if (opts->scrub) {
> +   erase.scrub = opts->scrub;
> +
> +   /*
> +* We don't need the bad block table anymore...
> +* after scrub, there are no bad blocks left!
> +*/
> +#if CONFIG_IS_ENABLED(MTD_RAW_NAND)
> +   if (chip->bbt) {
> +   kfree(chip->bbt);
> +   }
> +   chip->bbt = NULL;
> +   chip->options &= ~NAND_BBT_SCANNED;
> +#elif CONFIG_IS_ENABLED(MTD_SPI_NAND)
> +   if (nanddev_bbt_is_initialized(nand))
> +   nanddev_bbt_cleanup(nand);
> +#endif
> +   }
> +
> +   for (erased_length = 0;
> +erased_length < erase_length;
> +erase.addr += mtd->erasesize) {
> +
> +   schedule();
> +
> +   if (opts->lim && (erase.addr >= (opts->offset + opts->lim))) {
> +   puts("Size of erase exceeds limit\n");
> +   return -EFBIG;
> +   }
> +   if (!opts->scrub) {
> +   int ret = mtd_block_isbad(mtd, erase.addr);
> +   if (ret > 0) {
> +   if (!opts->quiet)
> +   printf("\rSkipping %s at  "
> +  "0x%08llx "
> +  " \n",
> +  ret == 1 ? "bad block" : "bbt 
> reserved",
> +  erase.addr);
> +
> +   if (!opts->spread)
> +   erased_length++;
> +
> +   continue;
> +
> +   } else if (ret < 0) {
> +   printf("\n%s: MTD get bad block failed: %d\n",
> +  mtd_device,
> +  ret);
> +   return -1;
> +   }
> +   }
> +
> +   erased_length++;
> +
> +   result = mtd_erase(mtd, );
> +   if (result != 0) {
> +   printf("\n%s: MTD Erase failure: %d\n",
> +  mtd_device, result);
> +   continue;
> +   }
> +
> +   /* format for JFFS2 ? */
> +   if (opts->jffs2 && oobavail >= 8) {
> +   struct mtd_oob_ops ops;
> +   ops.ooblen = 8;
> +   ops.datbuf = NULL;
> +   ops.oobbuf = (uint8_t *)
> +   ops.ooboffs = 0;
> +   ops.mode = MTD_OPS_AUTO_OOB;
> +
> +   result = mtd_write_oob(mtd, erase.addr, );
> +   if (result != 0) {
> +   printf("\n%s: MTD writeoob failure: %d\n",
> +  mtd_device, result);
> +   continue;
> +   }
> +   }
> +
> +   if (!opts->quiet) {
> +   unsigned long long n = erased_length * 100ULL;
> +   int percent;
> +
> +   do_div(n, erase_length);
> +   percent = (int)n;
> +
> +   /* output progress message only at whole percent
> +* steps to reduce the number of messages printed
> +* on (slow) serial consoles
> +*/
> +   if (percent != percent_complete) {
> +   percent_complete = percent;
> +
> +   printf("\rErasing at 0x%llx -- %3d%% 
> complete.",
> +  erase.addr, percent);
> +
> +   if (opts->jffs2 && result == 0)
> +   printf(" Cleanmarker written at 
> 0x%llx.",
> +  erase.addr);
> +   }
> +   }
> +   }
> +   if (!opts->quiet)
> +   printf("\n");
> +
> +   return 0;
> +}
> +
>  /**
>   * check_skip_len
>   *
> --
> 2.30.1
>

Reviewed-By: Michael Trimarchi 


Re: Adding EFI runtime support to the Arm's FF-A bus

2023-12-19 Thread Michael Walle

Hi Mark,


> Any runtime device drivers for variable storage should not be in the
> U-Boot runtime but live in the secure world (e.g. OP-TEE) FF-A is the
> new ARM protocol for talking to the secure world and hence fits into
> the picture.

What if I just want a simple embedded boot stack where I don't
want any secure world and just want to be able to boot a COTS linux
distribution via EFI?


That already works for many Linux distros.  As long as the distro
installs the appropriate BOOTxxx.EFI file you don't actually need to
set any EFI variables for the OS to boot.  It can't get any simpler
than that.  Of the main Linux distros it seems that only Debian
doesn't do this.  Someone should probably lobby Debian to do this as
well as it would mean that Debian would just work on an EBBR compliant
system.


I know. Last time I checked CentOS (or was it Ubuntu?) tried to
set EFI variables and the installer just failed. Might be fixed now,
though.


Things get more complicated if you want to install multiple OSes.
Then having EFI variable support makes things a lot more
straightforward.

And of course EFI secure boot needs EFI variable support as well (with
proper support) for authenticated EFI variables.  But IMHO that no
longer falls into "simple embedded boot stack" territory.


Thats clear.


Assuming, that there might be a simple dedicated EEPROM to store the
variables which is not exposed to linux, is that something which would
be rejected by u-boot mainline now?


Not necessarily.  But such an approach will have limitations:

* Completely hiding the EEPROM from the OS may be hard.  Even if you
  have a dedicated SPI controller for the EEPROM things like the SPI
  bus clock or power domains may still be under OS control.


Fair point, but I was thinking about the ls1028a for example, where - if
I remember correctly - there was one dedicated i2c controller in a sense
of isolation, probably to use with a secure OS. Also there is no dynamic
clocking.

So, technically it should be possible, even with a low overhead, like no
device model etc, which could reside in the efi os services. Just 
testing

the waters here, not that I'm interested in adding support for that in
u-boot. Just a bit concerned that it (EFI variables) will only work with
a full stack (tf-a, optee) in the future.


* It is not possible to properly implement authenticated variables for
  secure boot if the EEPROM and associated hardware is just removed
  from the device tree but still accessable to the OS.  An
  implementation that pretends the variables are "secure" will
  probably be rejected.


Sure. I excluded any secure stuff. But, with that i2c controller i was
talking about earlier, it should be possible to mark it as EL3 access
only.

Thanks,
-michael


Re: Adding EFI runtime support to the Arm's FF-A bus

2023-12-19 Thread Michael Walle
Hi Heinrich,

> Any runtime device drivers for variable storage should not be in the
> U-Boot runtime but live in the secure world (e.g. OP-TEE) FF-A is the
> new ARM protocol for talking to the secure world and hence fits into
> the picture.

What if I just want a simple embedded boot stack where I don't
want any secure world and just want to be able to boot a COTS linux
distribution via EFI?

Assuming, that there might be a simple dedicated EEPROM to store the
variables which is not exposed to linux, is that something which would
be rejected by u-boot mainline now?

-michael


Re: [PATCH] mtd: nand: omap_gpmc: Fix NAND in SPL for AM335x

2023-12-08 Thread Michael Nazzareno Trimarchi
Hi Roger

On Sat, Nov 25, 2023 at 12:16 PM Roger Quadros  wrote:
>
> AM335x uses a special driver "am335x_spl_bch.c" as SPL
> NAND loader. This driver expects 1 sector at a time ECC
> and doesn't work well with multi-sector ECC that was implemented in
> commit 04fcd2587321 ("mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based 
> correction")
>
> Switch back to 1 sector at a time read/ECC.
>
> Fixes: 04fcd2587321 ("mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based 
> correction")
> Signed-off-by: Roger Quadros 
> ---
>  drivers/mtd/nand/raw/omap_gpmc.c | 95 ++--
>  1 file changed, 29 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/omap_gpmc.c 
> b/drivers/mtd/nand/raw/omap_gpmc.c
> index 1a5ed0de31..2d2d2c2b6d 100644
> --- a/drivers/mtd/nand/raw/omap_gpmc.c
> +++ b/drivers/mtd/nand/raw/omap_gpmc.c
> @@ -293,7 +293,7 @@ static void __maybe_unused omap_enable_hwecc_bch(struct 
> mtd_info *mtd,
> break;
> case OMAP_ECC_BCH8_CODE_HW:
> bch_type = 1;
> -   nsectors = chip->ecc.steps;
> +   nsectors = 1;
> if (mode == NAND_ECC_READ) {
> wr_mode   = BCH_WRAPMODE_1;
> ecc_size0 = BCH8R_ECC_SIZE0;
> @@ -306,7 +306,7 @@ static void __maybe_unused omap_enable_hwecc_bch(struct 
> mtd_info *mtd,
> break;
> case OMAP_ECC_BCH16_CODE_HW:
> bch_type = 0x2;
> -   nsectors = chip->ecc.steps;
> +   nsectors = 1;
> if (mode == NAND_ECC_READ) {
> wr_mode   = 0x01;
> ecc_size0 = 52; /* ECC bits in nibbles per sector */
> @@ -345,17 +345,16 @@ static void __maybe_unused omap_enable_hwecc_bch(struct 
> mtd_info *mtd,
>  }
>

If the changes impact only one family can you just restrict it to those family?

Michael
>  /**
> - * _omap_calculate_ecc_bch - Generate BCH ECC bytes for one sector
> + * omap_calculate_ecc_bch - Generate BCH ECC bytes for one sector
>   * @mtd:MTD device structure
>   * @dat:The pointer to data on which ecc is computed
>   * @ecc_code:   The ecc_code buffer
> - * @sector: The sector number (for a multi sector page)
>   *
>   * Support calculating of BCH4/8/16 ECC vectors for one sector
>   * within a page. Sector number is in @sector.
>   */
> -static int _omap_calculate_ecc_bch(struct mtd_info *mtd, const u8 *dat,
> -  u8 *ecc_code, int sector)
> +static int omap_calculate_ecc_bch(struct mtd_info *mtd, const u8 *dat,
> + u8 *ecc_code)
>  {
> struct nand_chip *chip = mtd_to_nand(mtd);
> struct omap_nand_info *info = nand_get_controller_data(chip);
> @@ -368,7 +367,7 @@ static int _omap_calculate_ecc_bch(struct mtd_info *mtd, 
> const u8 *dat,
> case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
>  #endif
> case OMAP_ECC_BCH8_CODE_HW:
> -   ptr = _cfg->bch_result_0_3[sector].bch_result_x[3];
> +   ptr = _cfg->bch_result_0_3[0].bch_result_x[3];
> val = readl(ptr);
> ecc_code[i++] = (val >>  0) & 0xFF;
> ptr--;
> @@ -383,21 +382,21 @@ static int _omap_calculate_ecc_bch(struct mtd_info 
> *mtd, const u8 *dat,
>
> break;
> case OMAP_ECC_BCH16_CODE_HW:
> -   val = 
> readl(_cfg->bch_result_4_6[sector].bch_result_x[2]);
> +   val = readl(_cfg->bch_result_4_6[0].bch_result_x[2]);
> ecc_code[i++] = (val >>  8) & 0xFF;
> ecc_code[i++] = (val >>  0) & 0xFF;
> -   val = 
> readl(_cfg->bch_result_4_6[sector].bch_result_x[1]);
> +   val = readl(_cfg->bch_result_4_6[0].bch_result_x[1]);
> ecc_code[i++] = (val >> 24) & 0xFF;
> ecc_code[i++] = (val >> 16) & 0xFF;
> ecc_code[i++] = (val >>  8) & 0xFF;
> ecc_code[i++] = (val >>  0) & 0xFF;
> -   val = 
> readl(_cfg->bch_result_4_6[sector].bch_result_x[0]);
> +   val = readl(_cfg->bch_result_4_6[0].bch_result_x[0]);
> ecc_code[i++] = (val >> 24) & 0xFF;
> ecc_code[i++] = (val >> 16) & 0xFF;
> ecc_code[i++] = (val >>  8) & 0xFF;
> ecc_code[i++] = (val >>  0) & 0xFF;
> for (j = 3; j >= 0; j--) {
> -   val = 
> readl(_cfg->bch_result_0_3[sector].bch_result_

Re: [PATCH] mtd: nand: omap_gpmc: Fix NAND in SPL for AM335x

2023-12-08 Thread Michael Nazzareno Trimarchi
gt; > - chip->cmdfunc(mtd, NAND_CMD_RNDOUT, 0, -1);
> > - chip->read_buf(mtd, buf, mtd->writesize);
> > + /* read data */
> > + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, data_pos, -1);
> > + chip->read_buf(mtd, p, eccsize);
> >
> > - /* read all ecc bytes from oob area */
> > - chip->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_pos, -1);
> > - chip->read_buf(mtd, oob, ecctotal);
> > + /* read respective ecc from oob area */
> > + chip->cmdfunc(mtd, NAND_CMD_RNDOUT, oob_pos, -1);
> > + chip->read_buf(mtd, oob, eccbytes);
> > + /* read syndrome */
> > + chip->ecc.calculate(mtd, p, _calc[i]);
> >
> > - /* Calculate ecc bytes */
> > - omap_calculate_ecc_bch_multi(mtd, buf, ecc_calc);
> > + data_pos += eccsize;
> > + oob_pos += eccbytes;
> > + }
> >
> >   for (i = 0; i < chip->ecc.total; i++)
> >   ecc_code[i] = chip->oob_poi[eccpos[i]];
> > @@ -945,6 +905,7 @@ static int omap_select_ecc_scheme(struct nand_chip 
> > *nand,
> >   nand->ecc.hwctl = omap_enable_hwecc_bch;
> >   nand->ecc.correct   = omap_correct_data_bch_sw;
> >   nand->ecc.calculate = omap_calculate_ecc_bch;
> > + nand->ecc.steps = eccsteps;
> >   /* define ecc-layout */
> >   ecclayout->eccbytes = nand->ecc.bytes * eccsteps;
> >   ecclayout->eccpos[0]= BADBLOCK_MARKER_LENGTH;
> > @@ -987,6 +948,7 @@ static int omap_select_ecc_scheme(struct nand_chip 
> > *nand,
> >   nand->ecc.correct   = omap_correct_data_bch;
> >   nand->ecc.calculate = omap_calculate_ecc_bch;
> >   nand->ecc.read_page = omap_read_page_bch;
> > + nand->ecc.steps = eccsteps;
> >   /* define ecc-layout */
> >   ecclayout->eccbytes = nand->ecc.bytes * eccsteps;
> >   for (i = 0; i < ecclayout->eccbytes; i++)
> > @@ -1020,6 +982,7 @@ static int omap_select_ecc_scheme(struct nand_chip 
> > *nand,
> >   nand->ecc.correct   = omap_correct_data_bch;
> >   nand->ecc.calculate = omap_calculate_ecc_bch;
> >   nand->ecc.read_page = omap_read_page_bch;
> > + nand->ecc.steps = eccsteps;
> >   /* define ecc-layout */
> >   ecclayout->eccbytes = nand->ecc.bytes * eccsteps;
> >   for (i = 0; i < ecclayout->eccbytes; i++)
> >
> > base-commit: 9e53e45292ee2f1d9d2ccc59914b161bef9b10d7
>
> --
> cheers,
> -roger



-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
mich...@amarulasolutions.com
__

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
i...@amarulasolutions.com
www.amarulasolutions.com


Re: [PATCH v1] mtd: rawnand: Meson NAND controller support

2023-12-08 Thread Michael Nazzareno Trimarchi
 CC CC CC CC CC CC CC CC CC CC CC CC CC
> + * 0x20: AA AA DD DD DD DD DD DD DD DD DD DD DD DD DD DD
> + * 0x30: AA AA EE EE EE EE EE EE EE EE EE EE EE EE EE EE
> + *
> + * AA - user bytes.
> + * BB, CC, DD, EE - ECC code bytes for each step.
> + */
> +static struct nand_ecclayout nand_oob;
> +
> +static void meson_nfc_init_nand_oob(struct nand_chip *nand)
> +{
> +   int section_size = 2 + nand->ecc.bytes;
> +   int i;
> +   int k;
> +
> +   nand_oob.eccbytes = nand->ecc.steps * nand->ecc.bytes;
> +   k = 0;
> +
> +   for (i = 0; i < nand->ecc.steps; i++) {
> +   int j;
> +
> +   for (j = 0; j < nand->ecc.bytes; j++)
> +   nand_oob.eccpos[k++] = (i * section_size) + 2 + j;
> +
> +   nand_oob.oobfree[i].offset = (i * section_size);
> +   nand_oob.oobfree[i].length = 2;
> +   }
> +
> +   nand_oob.oobavail = 2 * nand->ecc.steps;
> +   nand->ecc.layout = _oob;
> +}
> +
> +static int meson_nfc_init_ecc(struct nand_chip *nand, ofnode node)
> +{
> +   const struct mtd_info *mtd = nand_to_mtd(nand);
> +   int ret;
> +   int i;
> +
> +   ret = nand_check_ecc_caps(nand, _axg_ecc_caps, mtd->oobsize - 
> 2);
> +   if (ret)
> +   return ret;
> +
> +   for (i = 0; i < ARRAY_SIZE(meson_ecc); i++) {
> +   if (meson_ecc[i].strength == nand->ecc.strength &&
> +   meson_ecc[i].size == nand->ecc.size) {
> +   struct meson_nfc_nand_chip *meson_chip = 
> to_meson_nand(nand);
> +
> +   nand->ecc.steps = mtd->writesize / nand->ecc.size;
> +   meson_chip->bch_mode = meson_ecc[i].bch;
> +
> +   meson_nfc_init_nand_oob(nand);
> +
> +   return 0;
> +   }
> +   }
> +
> +   return -EINVAL;
> +}
> +
> +static int meson_nfc_nand_chip_init(struct udevice *dev, struct meson_nfc 
> *nfc,
> +   ofnode node)
> +{
> +   struct meson_nfc_nand_chip *meson_chip;
> +   struct nand_chip *nand;
> +   struct mtd_info *mtd;
> +   u32 cs[MAX_CE_NUM];
> +   u32 nsels;
> +   int ret;
> +   int i;
> +
> +   if (!ofnode_get_property(node, "reg", )) {
> +   dev_err(dev, "\"reg\" property is not found\n");
> +   return -ENODEV;
> +   }
> +
> +   nsels /= sizeof(u32);
> +   if (nsels >= MAX_CE_NUM) {
> +   dev_err(dev, "invalid size of CS array, max is %d\n",
> +   MAX_CE_NUM);
> +   return -EINVAL;
> +   }
> +
> +   ret = ofnode_read_u32_array(node, "reg", cs, nsels);
> +   if (ret < 0) {
> +   dev_err(dev, "failed to read \"reg\" property\n");
> +   return ret;
> +   }
> +
> +   for (i = 0; i < nsels; i++) {
> +   if (test_and_set_bit(cs[i], >assigned_cs)) {
> +   dev_err(dev, "CS %d already assigned\n", cs[i]);
> +   return -EINVAL;
> +   }
> +   }
> +
> +   meson_chip = malloc(sizeof(*meson_chip) + nsels * 
> sizeof(meson_chip->sels[0]));
> +   if (!meson_chip) {
> +   dev_err(dev, "failed to allocate memory for chip\n");
> +   return -ENOMEM;
> +   }
> +
> +   meson_chip->nsels = nsels;
> +   nand = _chip->nand;
> +
> +   nand->flash_node = node;
> +   nand_set_controller_data(nand, nfc);
> +   /* Set the driver entry points for MTD */
> +   nand->cmdfunc = meson_nfc_nand_cmd_function;
> +   nand->cmd_ctrl = meson_nfc_cmd_ctrl;
> +   nand->select_chip = meson_nfc_nand_select_chip;
> +   nand->read_byte = meson_nfc_nand_read_byte;
> +   nand->write_byte = meson_nfc_nand_write_byte;
> +   nand->dev_ready = meson_nfc_dev_ready;
> +
> +   /* Buffer read/write routines */
> +   nand->read_buf = meson_nfc_read_buf;
> +   nand->write_buf = meson_nfc_write_buf;
> +   nand->options |= NAND_NO_SUBPAGE_WRITE;
> +
> +   nand->ecc.mode = NAND_ECC_HW;
> +   nand->ecc.hwctl = NULL;
> +   nand->ecc.read_page = meson_nfc_read_page_hwecc;
> +   nand->ecc.write_page = meson_nfc_write_page_hwecc;
> +   nand->ecc.read_page_raw = meson_nfc_read_page_raw;
> +   nand->ecc.write_page_raw = meson_nfc_write_page_raw;
> +
> +   nand->

[PATCH] arm: mach-k3: am625: Relax emmc boot condition

2023-12-07 Thread Michael Trimarchi
spl_mmc_emmc_boot_partition return a number different from 0
if the partition is a boot one. We can have the uboot img
for instance in a raw offset in emmc partition 0 so we would
like to continue to load the next stage. If the user want
to use EMMC as boot device allow him to use any part of the
emmc and not only boot partition

Signed-off-by: Michael Trimarchi 
---
 arch/arm/mach-k3/am625_init.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/arch/arm/mach-k3/am625_init.c b/arch/arm/mach-k3/am625_init.c
index 8fa36f7b91..cb4c3a05b5 100644
--- a/arch/arm/mach-k3/am625_init.c
+++ b/arch/arm/mach-k3/am625_init.c
@@ -222,11 +222,8 @@ u32 spl_mmc_boot_mode(struct mmc *mmc, const u32 
boot_device)
 
switch (bootmode) {
case BOOT_DEVICE_EMMC:
-   if (IS_ENABLED(CONFIG_SUPPORT_EMMC_BOOT)) {
-   if (spl_mmc_emmc_boot_partition(mmc))
-   return MMCSD_MODE_EMMCBOOT;
-   return MMCSD_MODE_FS;
-   }
+   if (IS_ENABLED(CONFIG_SUPPORT_EMMC_BOOT))
+   return MMCSD_MODE_EMMCBOOT;
if (IS_ENABLED(CONFIG_SPL_FS_FAT) || 
IS_ENABLED(CONFIG_SPL_FS_EXT4))
return MMCSD_MODE_FS;
return MMCSD_MODE_EMMCBOOT;
-- 
2.40.1



[PATCH] pxe_utils: Increase feedback to user when fdt file is not found

2023-12-07 Thread Michael Trimarchi
extlinux.conf can set fdtdir. fdtdir look for fdt file using
information found in the enviroment variable. The function does
not report any error in the case the file is not found

Scanning for bootflows in all bootdevs
Seq  Method   State   UclassPart  Name  Filename
---  ---  --        

Scanning global bootmeth 'efi_mgr':
No EFI system partition
No EFI system partition
Failed to persist EFI variables
Scanning bootdev 'mmc@fa1.bootdev':
  0  extlinux ready   mmc  1  mmc@fa1.bootdev.part_ 
/boot/extlinux/extlinux.conf
** Booting bootflow 'mmc@fa1.bootdev.part_1' with extlinux
1:  am62x-sk-buildroot
Retrieving file: /boot/Image
append: console=ttyS2,115200n8 
root=PARTUUID=c586a30c-0bf1-4323-aba8-779c814ee135 rw
rootfstype=ext4 rootwait earlycon=ns16550a,mmio32,0x0280
Retrieving file: /boot/k3-am623_ccm_m3.dtb
Skipping fdtdir /boot/ for failure retrieving dts

Signed-off-by: Michael Trimarchi 
---
 boot/pxe_utils.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index a92bb896c6..83bc167785 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -700,6 +700,11 @@ static int label_boot(struct pxe_context *ctx, struct 
pxe_label *label)
   label->name);
goto cleanup;
}
+
+   if (label->fdtdir) {
+   printf("Skipping fdtdir %s for failure 
retrieving dts\n",
+   label->fdtdir);
+   }
}
 
if (label->kaslrseed)
-- 
2.40.1



  1   2   3   4   5   6   7   8   9   10   >