[PATCH v2] boot: android: fix booting without a ramdisk

2024-07-29 Thread Michael Walle
android_image_get_ramdisk() will return an error if there is no ramdisk.
Using the android image without a ramdisk worked until commit
1ce8e10f3b4b ("image: Fix up ANDROID_BOOT_IMAGE ramdisk code") because
the return code wasn't checked until then. Return -ENOENT in case
there is no ramdisk and translate that into -ENOPKG in the calling
code, which will then indicate "no ramdisk" to its caller
(boot_get_ramdisk()).

This way, we can get rid of the "*rd_data = *rd_len = 0;" in the error
path, too.

With this, I'm able to boot a linux kernel using fastboot again:

  fastboot --base 0x4100 --header-version 2 --dtb /path/to/dtb \
  --cmdline "root=/dev/mmcblk0p1 rootwait" boot path/to/Image

Signed-off-by: Michael Walle 
---
 boot/image-android.c | 7 +++
 boot/image-board.c   | 4 +++-
 include/image.h  | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/boot/image-android.c b/boot/image-android.c
index 09c7a44e058..774565fd1fe 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -393,10 +393,9 @@ int android_image_get_ramdisk(const void *hdr, const void 
*vendor_boot_img,
if (!android_image_get_data(hdr, vendor_boot_img, _data))
return -EINVAL;
 
-   if (!img_data.ramdisk_size) {
-   *rd_data = *rd_len = 0;
-   return -1;
-   }
+   if (!img_data.ramdisk_size)
+   return -ENOENT;
+
if (img_data.header_version > 2) {
ramdisk_ptr = img_data.ramdisk_addr;
memcpy((void *)(ramdisk_ptr), (void 
*)img_data.vendor_ramdisk_ptr,
diff --git a/boot/image-board.c b/boot/image-board.c
index f2124013046..eca1b1d2bff 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -427,7 +427,9 @@ static int select_ramdisk(struct bootm_headers *images, 
const char *select, u8 a
unmap_sysmem(ptr);
}
 
-   if (ret)
+   if (ret == -ENOENT)
+   return -ENOPKG;
+   else if (ret)
return ret;
done = true;
}
diff --git a/include/image.h b/include/image.h
index dd4042d1bd9..2ab17075287 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1858,7 +1858,7 @@ int android_image_get_kernel(const void *hdr,
  * @vendor_boot_img : Pointer to vendor boot image header
  * @rd_data:   Pointer to a ulong variable, will hold ramdisk address
  * @rd_len:Pointer to a ulong variable, will hold ramdisk length
- * Return: 0 if succeeded, -1 if ramdisk size is 0
+ * Return: 0 if OK, -ENOPKG if no ramdisk, -EINVAL if invalid image
  */
 int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img,
  ulong *rd_data, ulong *rd_len);
-- 
2.39.2



[PATCH] boot: android: fix booting without a ramdisk

2024-07-26 Thread Michael Walle
android_image_get_ramdisk() will return an error if there is no ramdisk.
Using the android image without a ramdisk worked until commit
1ce8e10f3b4b ("image: Fix up ANDROID_BOOT_IMAGE ramdisk code") because
that return code wasn't checked. Now that it is checked, don't return an
error in the (valid) case that there is no ramdisk in the image.

With this, I'm able to boot a linux kernel using fastboot again:

  fastboot --base 0x4100 --header-version 2 --dtb /path/to/dtb \
  --cmdline "root=/dev/mmcblk0p1 rootwait" boot path/to/Image

Signed-off-by: Michael Walle 
---
 boot/image-android.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/image-android.c b/boot/image-android.c
index 09c7a44e058..0900579ee8c 100644
--- a/boot/image-android.c
+++ b/boot/image-android.c
@@ -395,7 +395,7 @@ int android_image_get_ramdisk(const void *hdr, const void 
*vendor_boot_img,
 
if (!img_data.ramdisk_size) {
*rd_data = *rd_len = 0;
-   return -1;
+   return 0;
}
if (img_data.header_version > 2) {
ramdisk_ptr = img_data.ramdisk_addr;
-- 
2.39.2



Re: [PATCH V2] clk: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present for gate and mux

2024-07-26 Thread Michael Nazzareno Trimarchi
Hi Sam

On Thu, Jul 25, 2024 at 4:41 AM Sam Protsenko
 wrote:
>
> On Wed, Jul 24, 2024 at 4:44 AM Michael Nazzareno Trimarchi
>  wrote:
>
> [snip]
>
> > > Ok , so you suggest to have something like in the core
> >
> > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> > index 8faf5a56e1..a309108dd4 100644
> > --- a/drivers/clk/clk-uclass.c
> > +++ b/drivers/clk/clk-uclass.c
> > @@ -467,8 +467,14 @@ ulong clk_get_rate(struct clk *clk)
> > return 0;
> > ops = clk_dev_ops(clk->dev);
> >
> > -   if (!ops->get_rate)
> > -   return -ENOSYS;
> > +   if (!ops->get_rate) {
> > +   parent = clk_get_parent(clk);
> > +   if (!clk_is_valid(parent))
> > +   return -ENOSYS;
> > +   ops = clk_dev_ops(parent->dev);
> > +   if (!ops->get_rate)
> > +   return -ENOSYS;
> > +   }
> >
> > return ops->get_rate(clk);
> >  }
> >
> > You suggest to remove get_rate, set_rate function in mux and gate and
> > use some propagation in the core, am I right?
>
> Something like that, yes. But maybe instead of
>
> if (!ops->get_rate)
>
> use something like:
>
> while (!ops->get_rate)
>
> like I did in my patch [1] for clk_set_rate(). That while loop is
> needed to handle cases when there are multiple clocks without
> .get_rate() operation connected together (like mux or gate clocks). So
> you'd have to iterate them all up the tree to find some DIV clock
> which actually has .get_rate() defined.
>
> But at the moment I'm more concerned about .set_rate() propagation of
> course, which is really needed for my E850-96 MMC enablement series
> [2], which has already been pending on the review for a while.
>
> [1] https://lists.denx.de/pipermail/u-boot/2024-March/547719.html
> [2] https://lists.denx.de/pipermail/u-boot/2024-July/559602.html
>
> [snip]
>
> > > > implementations simple and brings the propagation logic to the actual
> > > > clk API (clk_set_rate()), which in turn is able to cover all possible
> > > > cases: even if some new clock types are implemented later, they will
> > > > be covered already. That also reduces some code duplication and makes
> > > > it easier to control that behavior in a centralized manner. The patch
> > > > with the discussed implementation was already submitted a while ago
> > > > [2], and still pending on the review.
> > >
> > > I have seen it and because I'm extending the clk framework a bit more [1]
> > > I was thinking about a more complete approach. Nothing against your
> > > patch, but just
> > > I have observed in my use case (display) that I need more from the
> > > clock framework we have
> > >
>
> I see. Do you think it'd possible for you to use my patch for
> clk_set_rate() propagation for your case, and base your patches (for
> CLK_OPS_PARENT_ENABLE, etc) on top of that? Will it cover your
> requirements?
>

If I remember how it works now we don't need the loop. Each get_clk_rate
or clk_set_rate should implement propagation in their implementation if the
CLK PARENT SET is present

Michael


> Would be also nice to hear what maintainers think about that :)
>
> Thanks!
>
> > > Michael
> > >
> > > [1] https://patchwork.amarulasolutions.com/patch/3291/
> > >
> > > >
>
> [snip]



-- 
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: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present

2024-07-25 Thread Michael Nazzareno Trimarchi
Hi Sam

Il mar 23 lug 2024, 22:53 Sam Protsenko  ha
scritto:

> On Wed, Jul 3, 2024 at 10:00 AM Michael Nazzareno Trimarchi
>  wrote:
> >
> > Hi all
> >
> > Working on the clock now. I have done a different implementation
> >
> > clk-mux, clk-gate I have added a clk_generic_set_rate that is a stub
> > that propagate to the parent, I think that should work the same but
> > wihout this while and even on preparation on more
> > changes I need. Can be work on your case too?
> >
>
> Hi Michael,
>
> I've provided some thoughts on your gate/mux implementation here: [1].
> As you've submitted your (different) implementation of the same
> feature, but 4 months later, I guess my patch doesn't cover your
> cases, or perhaps it has some design flaws? Please let me know.
>
> If anything, that shows there is a rising need in this propagation
> feature. I'd like to encourage the maintainers to take a look at both
> implementations, and maybe at least comment on why there is hesitance
> to accept this patch. It's been almost 5 months that I'm trying to
> push that through, and my MMC work (DDR mode on E850-96 board to be
> precise) still very much depends on this.
>


My series is still not ready and I have no problem to go with your while
for now

Michael

>
>
>
> Thanks!
>
> [1] https://lists.denx.de/pipermail/u-boot/2024-July/559753.html
>
>
> > Michael
> >
> > On Thu, Jun 13, 2024 at 5:33 PM Tom Rini  wrote:
> > >
> > > On Mon, Jun 10, 2024 at 02:43:17PM -0500, Sam Protsenko wrote:
> > > > On Tue, May 14, 2024 at 3:26 PM Sam Protsenko
> > > >  wrote:
> > > > >
> > > > > On Wed, May 1, 2024 at 4:12 PM Sam Protsenko <
> semen.protse...@linaro.org> wrote:
> > > > > >
> > > > > > On Wed, Apr 10, 2024 at 9:53 PM Sean Anderson 
> wrote:
> > > > > > >
> > > > > > > On 3/7/24 19:04, Sam Protsenko wrote:
> > > > > > > > Sometimes clocks provided to a consumer might not have
> .set_rate
> > > > > > > > operation (like gate or mux clocks), but have
> CLK_SET_PARENT_RATE flag
> > > > > > > > set. In that case it's usually possible to find a parent up
> the tree
> > > > > > > > which is capable of setting the rate (div, pll, etc).
> Implement a simple
> > > > > > > > lookup procedure for such cases, to traverse the clock tree
> until
> > > > > > > > .set_rate capable parent is found, and use that parent to
> actually
> > > > > > > > change the rate. The search will stop once the first
> .set_rate capable
> > > > > > > > clock is found, which is usually enough to handle most cases.
> > > > > > > >
> > > > > > > > Signed-off-by: Sam Protsenko 
> > > > > > > > ---
> > > > > >
> > > > > > [snip]
> > > > > >
> > > > > > >
> > > > > > > Can you give an example of where this is needed?
> > > > > > >
> > > > > >
> > > > > > Sure. In my case it's needed for eMMC enablement on E850-96
> board.
> > > > > > eMMC node in the device tree [1] is a gate clock
> > > > > > (CLK_GOUT_MMC_EMBD_SDCLKIN), so the MMC driver won't be able to
> change
> > > > > > its rate. But that clock actually has CLK_SET_RATE_PARENT flag
> set in
> > > > > > the clock driver [2]. So the right thing to do in this case (and
> > > > > > that's basically how it's done in Linux kernel too) is to
> traverse the
> > > > > > clock tree upwards, and try to find the parent capable to do
> .set_rate
> > > > > > (which is usually a divider clock), and use it to change the
> clock
> > > > > > rate. I'm working on exynos_dw_mmc driver [3] right now, making
> it use
> > > > > > CCF clocks, but I can't do that before this patch is applied.
> > > > > >
> > > > > > Grepping the U-Boot tree I can see the CLK_SET_RATE_PARENT flag
> is
> > > > > > also used in various IMX clock drivers and STM32MP13 clock
> driver. I
> > > > > > guess without this change those flags will be basically ignored.
> > > > > >
> > > > > > Thanks!
> > > > > >
> > > > >
> > > > > Hi Sean,
> > > > >
> > > > > Just wanted to check if you think my explanation above is ok and
> the
> > > > > patch can be applied? I'm finishing my new patch series for
> enabling
> > > > > MMC on E850-96, but this patch has to be applied first.
> > > > >
> > > > > Thanks!
> > > > >
> > > >
> > > > Hi Tom,
> > > >
> > > > Would it be reasonable to ask you to take care of this patch? It's
> > > > been ignored for 3 months now, but it's needed for eMMC enablement on
> > > > E850-96 board.
> > >
> > > Any comments at this point Sean?
> > >
> > > --
> > > 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: [PATCH v2 00/40] mmc: dw_mmc: Enable eMMC on E850-96 board

2024-07-25 Thread Michael Nazzareno Trimarchi
Hi all

Il mar 9 lug 2024, 02:48 Sam Protsenko  ha
scritto:

> On Thu, Jun 27, 2024 at 9:42 AM Tom Rini  wrote:
> >
> > On Wed, Jun 26, 2024 at 10:12:12PM +0530, Anand Moon wrote:
> > > Hi Sam,
> > >
> > > On Wed, 19 Jun 2024 at 02:26, Sam Protsenko <
> semen.protse...@linaro.org> wrote:
> > > >
> > > > If there are no new comments on this series, can you please apply it?
> > > >
> > > > Thanks!
> > >
> > > Tested on Odroid XU4 and Odrroid U3 boards
> > >
> > > Please add my
> > >
> > > Reviewed-by: Anand Moon 
> > > Tested-by: Anand Moon 
> >
> > Thanks, Minkyu will you be able to pick these up (and other outstanding
> > patches) for a pull request to -next soon?
> >
>
> Hi Minkyu,
>
> Gentle nudge :) Could you please apply this series? I'm working on
> some other features requiring eMMC enabled, so it would be great to
> have it merged soon.
>

I think that we should merge your clock patch. I will rethink my series and
make a differente design but agree that 5 months are too much

Michael

>
> Thanks!
>
> > --
> > Tom
>


Re: [PATCH] clk: fix ccf_clk_get_rate

2024-07-25 Thread Michael Nazzareno Trimarchi
Hi

On Thu, Jul 25, 2024 at 11:58 AM Z.Q. Hou  wrote:
>
> Hi Michael,
>
> > -Original Message-
> > From: Michael Nazzareno Trimarchi 
> > Sent: Thursday, July 25, 2024 5:26 PM
> > To: Z.Q. Hou ; Simon Glass 
> > Cc: u-boot@lists.denx.de; tr...@konsulko.com; lu...@denx.de;
> > sean...@gmail.com
> > Subject: Re: [PATCH] clk: fix ccf_clk_get_rate
> >
> > Hi
> >
> > On Thu, Jul 25, 2024 at 10:53 AM Z.Q. Hou  wrote:
> > >
> > > Hi Michael,
> > >
> > > > -Original Message-
> > > > From: Michael Nazzareno Trimarchi 
> > > > Sent: Thursday, July 25, 2024 3:32 PM
> > > > To: Z.Q. Hou 
> > > > Cc: u-boot@lists.denx.de; tr...@konsulko.com; lu...@denx.de;
> > > > sean...@gmail.com
> > > > Subject: Re: [PATCH] clk: fix ccf_clk_get_rate
> > > >
> > > > Hi
> > > >
> > > > On Thu, Jul 25, 2024 at 9:16 AM Zhiqiang Hou 
> > > > wrote:
> > > > >
> > > > > From: Hou Zhiqiang 
> > > > >
> > > > > As the type of return value is 'ulong', when clk_get_by_id()
> > > > > failed, it should return 0 to indicate the get_rate operation doesn't
> > succeed.
> > > > >
> > > >
> > > > I understand your point here but the clk_get_rate that can call the
> > > > ccf clk_get_rate can already return -ENOSYS.
> > >
> > > will also fix it.
> > >
> >
> > Is there any usage of the error set on the latest bit of the clock? We need 
> > to
> > be sure that this is correct use accross the uboot. The clk-uclass define 
> > the
> > error that can return
>
> The problem is this function's return value is 'ulong', if use the error set, 
> the caller will treat it as a good value instead of a error number. There are 
> several APIs have the same problem.
>
> 2 methods to fix it:
> 1. keep the API's prototype, and use '0' to indicate error condition, but a 
> real '0' return value is also considered as error.
> 2. Change the return type to like 'long long int', and use a negative error 
> number to indicate error condition. Need to update the check of return value 
> for all the callers.
>
> Any suggestion?
>

unit test suggest that error condition are evaluated so we have 32 bit
on arm of unsigned long so we can not
map all the positive clock

rate = clk_set_rate(clk, 8000);
ut_asserteq(rate, -ENOSYS);

Now, can we have a clock greater then 2Ghz if yes this not work always
on 32 bit, then the macro
IS_ERR_VALUE should help on clk return

Michael




> Thanks,
> Zhiqiang



--
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: nand: raw: atmel: Use ONFI ECC params if available

2024-07-25 Thread Michael Nazzareno Trimarchi
Hi

On Thu, Jul 25, 2024 at 11:36 AM
 wrote:
>
> Acked-by: Balamanikandan Gunasundar
> 
>
> On 22/07/24 3:15 am, Zixun LI wrote:
> > [Some people who received this message don't often get email from 
> > ad...@hifiphile.com. Learn why this is important at 
> > https://aka.ms/LearnAboutSenderIdentification ]
> >
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the 
> > content is safe
> >
> > When ECC parameters are not specified in DT, first try ONFI ECC parameters
> > before fallback to maximum strength.
> >
> > It's the Linux driver behavior since the driver rewriting in f88fc12.
> >
> >  From then 2 nand system refactors have been done in 6a1b66d6 and 53576c7b,
> > chip->ecc_strength_ds and chip->ecc_step_ds became
> > nanddev_get_ecc_requirements(). U-Boot didn't follow the refactor and
> > always use these 2 fields.
> >
> > v2: Fix formatting, add upstream commit hash.
> >
> > Signed-off-by: Zixun LI 
> > ---
> >   drivers/mtd/nand/raw/atmel/nand-controller.c | 4 
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c 
> > b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > index ee4ec6da58..817fab4ca3 100644
> > --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> > +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> > @@ -1029,11 +1029,15 @@ static int atmel_nand_pmecc_init(struct nand_chip 
> > *chip)
> >  req.ecc.strength = ATMEL_PMECC_MAXIMIZE_ECC_STRENGTH;
> >  else if (chip->ecc.strength)
> >  req.ecc.strength = chip->ecc.strength;
> > +   else if (chip->ecc_strength_ds)
> > +   req.ecc.strength = chip->ecc_strength_ds;
> >  else
> >  req.ecc.strength = ATMEL_PMECC_MAXIMIZE_ECC_STRENGTH;
> >
> >  if (chip->ecc.size)
> >  req.ecc.sectorsize = chip->ecc.size;
> > +   else if (chip->ecc_step_ds)
> > +   req.ecc.sectorsize = chip->ecc_step_ds;
> >  else
> >  req.ecc.sectorsize = ATMEL_PMECC_SECTOR_SIZE_AUTO;
> >
> > --
> > 2.45.2
> >
>

Feel free to apply on microchip uboot branch if you are on hurry

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] clk: fix ccf_clk_get_rate

2024-07-25 Thread Michael Nazzareno Trimarchi
Hi

On Thu, Jul 25, 2024 at 10:53 AM Z.Q. Hou  wrote:
>
> Hi Michael,
>
> > -Original Message-
> > From: Michael Nazzareno Trimarchi 
> > Sent: Thursday, July 25, 2024 3:32 PM
> > To: Z.Q. Hou 
> > Cc: u-boot@lists.denx.de; tr...@konsulko.com; lu...@denx.de;
> > sean...@gmail.com
> > Subject: Re: [PATCH] clk: fix ccf_clk_get_rate
> >
> > Hi
> >
> > On Thu, Jul 25, 2024 at 9:16 AM Zhiqiang Hou 
> > wrote:
> > >
> > > From: Hou Zhiqiang 
> > >
> > > As the type of return value is 'ulong', when clk_get_by_id() failed,
> > > it should return 0 to indicate the get_rate operation doesn't succeed.
> > >
> >
> > I understand your point here but the clk_get_rate that can call the ccf
> > clk_get_rate can already return -ENOSYS.
>
> will also fix it.
>

Is there any usage of the error set on the latest bit of the clock? We
need to be sure that this
is correct use accross the uboot. The clk-uclass define the error that
can return

Michael

> Thanks,
> Zhiqiang




--
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: fix ccf_clk_get_rate

2024-07-25 Thread Michael Nazzareno Trimarchi
Hi

On Thu, Jul 25, 2024 at 9:16 AM Zhiqiang Hou  wrote:
>
> From: Hou Zhiqiang 
>
> As the type of return value is 'ulong', when clk_get_by_id() failed,
> it should return 0 to indicate the get_rate operation doesn't succeed.
>

I understand your point here but the clk_get_rate that can call the
ccf clk_get_rate
can already return -ENOSYS.

Michael

> Signed-off-by: Hou Zhiqiang 
> ---
>  drivers/clk/clk.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index b8c2e8d531..4c2c372cd3 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -84,7 +84,8 @@ ulong ccf_clk_get_rate(struct clk *clk)
> int err = clk_get_by_id(clk->id, );
>
> if (err)
> -   return err;
> +   return 0;
> +
> return clk_get_rate(c);
>  }
>
> --
> 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 V2] clk: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present for gate and mux

2024-07-24 Thread Michael Nazzareno Trimarchi
Hi Sam

On Wed, Jul 24, 2024 at 9:25 AM Michael Nazzareno Trimarchi
 wrote:
>
> Hi Sam
>
> On Tue, Jul 23, 2024 at 10:37 PM Sam Protsenko
>  wrote:
> >
> > On Fri, Jul 5, 2024 at 2:14 AM Michael Trimarchi
> >  wrote:
> > >
> > > Gate and mux does not have .set_rate operation, but they could have
> > > CLK_SET_PARENT_RATE flag set. In that case it's usually possible to find a
> > > parent up the tree which is capable of setting the rate (div, pll, etc).
> > > Add clk_generic_set_rate to allow them to trasverse the clock tree.
> > >
> > > Cc: Sam Protsenko 
> > > Signed-off-by: Michael Trimarchi 
> > > ---
> > >  drivers/clk/clk-gate.c   |  1 +
> > >  drivers/clk/clk-mux.c|  2 +-
> > >  drivers/clk/clk-uclass.c | 20 
> > >  drivers/clk/clk.c|  9 +
> > >  include/clk.h|  9 +
> > >  include/linux/clk-provider.h |  1 +
> > >  6 files changed, 41 insertions(+), 1 deletion(-)
> > > ---
> > > V1->V2:
> > > Fix missing include
> > > ---
> > > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> > > index cfd90b717e7..c86083ac5a3 100644
> > > --- a/drivers/clk/clk-gate.c
> > > +++ b/drivers/clk/clk-gate.c
> > > @@ -116,6 +116,7 @@ const struct clk_ops clk_gate_ops = {
> > > .enable = clk_gate_enable,
> > > .disable = clk_gate_disable,
> > > .get_rate = clk_generic_get_rate,
> > > +   .set_rate = clk_generic_set_rate,
> >
> > I can see that clk_generic_get_rate() already exists here, and adding
> > clk_generic_set_rate() tries to mimic that existing design. But
> > frankly, I'm not sure it was a very good design choice in the first
> > place. I mean, to have get/set rate operations for the clock types
> > that clearly don't have such ability. I think it would be better to
> > model the CCF clocks after real world clocks, and handle the
> > propagation on the clock logic level. In fact, that's how it's already
> > implemented in Linux kernel [1]. That keeps the particular clock
>
> Ok , so you suggest to have something like in the core
>
> if (p->set_rate)
> {
>  set_rate())
>
> }
> else
> {
> set_generic_rate()
> }
>

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 8faf5a56e1..a309108dd4 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -467,8 +467,14 @@ ulong clk_get_rate(struct clk *clk)
return 0;
ops = clk_dev_ops(clk->dev);

-   if (!ops->get_rate)
-   return -ENOSYS;
+   if (!ops->get_rate) {
+   parent = clk_get_parent(clk);
+   if (!clk_is_valid(parent))
+   return -ENOSYS;
+   ops = clk_dev_ops(parent->dev);
+   if (!ops->get_rate)
+   return -ENOSYS;
+   }

return ops->get_rate(clk);
 }

You suggest to remove get_rate, set_rate function in mux and gate and
use some propagation in the core, am I right?


Michael
> The same for get_rate()
>
> > implementations simple and brings the propagation logic to the actual
> > clk API (clk_set_rate()), which in turn is able to cover all possible
> > cases: even if some new clock types are implemented later, they will
> > be covered already. That also reduces some code duplication and makes
> > it easier to control that behavior in a centralized manner. The patch
> > with the discussed implementation was already submitted a while ago
> > [2], and still pending on the review.
>
> I have seen it and because I'm extending the clk framework a bit more [1]
> I was thinking about a more complete approach. Nothing against your
> patch, but just
> I have observed in my use case (display) that I need more from the
> clock framework we have
>
> Michael
>
> [1] https://patchwork.amarulasolutions.com/patch/3291/
>
> >
> > Can I ask you if patch [2] doesn't cover some of your cases? Or maybe
> > I'm missing something out and having .set_rate op in gate/mux would
> > actually be better for some particular reason?
> >
> > Thanks!
> >
> > [1] 
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-gate.c#n120
> > [2] https://lists.denx.de/pipermail/u-boot/2024-March/547719.html
> >
> >
> > >  };
> > >
> > >  struct clk *clk_register_gate(struct device *dev, const char *name,
> > > diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
>

Re: [PATCH V2] clk: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present for gate and mux

2024-07-24 Thread Michael Nazzareno Trimarchi
Hi Sam

On Tue, Jul 23, 2024 at 10:37 PM Sam Protsenko
 wrote:
>
> On Fri, Jul 5, 2024 at 2:14 AM Michael Trimarchi
>  wrote:
> >
> > Gate and mux does not have .set_rate operation, but they could have
> > CLK_SET_PARENT_RATE flag set. In that case it's usually possible to find a
> > parent up the tree which is capable of setting the rate (div, pll, etc).
> > Add clk_generic_set_rate to allow them to trasverse the clock tree.
> >
> > Cc: Sam Protsenko 
> > Signed-off-by: Michael Trimarchi 
> > ---
> >  drivers/clk/clk-gate.c   |  1 +
> >  drivers/clk/clk-mux.c|  2 +-
> >  drivers/clk/clk-uclass.c | 20 
> >  drivers/clk/clk.c|  9 +
> >  include/clk.h|  9 +
> >  include/linux/clk-provider.h |  1 +
> >  6 files changed, 41 insertions(+), 1 deletion(-)
> > ---
> > V1->V2:
> > Fix missing include
> > ---
> > diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
> > index cfd90b717e7..c86083ac5a3 100644
> > --- a/drivers/clk/clk-gate.c
> > +++ b/drivers/clk/clk-gate.c
> > @@ -116,6 +116,7 @@ const struct clk_ops clk_gate_ops = {
> > .enable = clk_gate_enable,
> > .disable = clk_gate_disable,
> > .get_rate = clk_generic_get_rate,
> > +   .set_rate = clk_generic_set_rate,
>
> I can see that clk_generic_get_rate() already exists here, and adding
> clk_generic_set_rate() tries to mimic that existing design. But
> frankly, I'm not sure it was a very good design choice in the first
> place. I mean, to have get/set rate operations for the clock types
> that clearly don't have such ability. I think it would be better to
> model the CCF clocks after real world clocks, and handle the
> propagation on the clock logic level. In fact, that's how it's already
> implemented in Linux kernel [1]. That keeps the particular clock

Ok , so you suggest to have something like in the core

if (p->set_rate)
{
 set_rate())

}
else
{
set_generic_rate()
}

The same for get_rate()

> implementations simple and brings the propagation logic to the actual
> clk API (clk_set_rate()), which in turn is able to cover all possible
> cases: even if some new clock types are implemented later, they will
> be covered already. That also reduces some code duplication and makes
> it easier to control that behavior in a centralized manner. The patch
> with the discussed implementation was already submitted a while ago
> [2], and still pending on the review.

I have seen it and because I'm extending the clk framework a bit more [1]
I was thinking about a more complete approach. Nothing against your
patch, but just
I have observed in my use case (display) that I need more from the
clock framework we have

Michael

[1] https://patchwork.amarulasolutions.com/patch/3291/

>
> Can I ask you if patch [2] doesn't cover some of your cases? Or maybe
> I'm missing something out and having .set_rate op in gate/mux would
> actually be better for some particular reason?
>
> Thanks!
>
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clk/clk-gate.c#n120
> [2] https://lists.denx.de/pipermail/u-boot/2024-March/547719.html
>
>
> >  };
> >
> >  struct clk *clk_register_gate(struct device *dev, const char *name,
> > diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
> > index e3481be95fa..f99a67ebd35 100644
> > --- a/drivers/clk/clk-mux.c
> > +++ b/drivers/clk/clk-mux.c
> > @@ -151,13 +151,13 @@ static int clk_mux_set_parent(struct clk *clk, struct 
> > clk *parent)
> >  #else
> > writel(reg, mux->reg);
> >  #endif
> > -
> > return 0;
> >  }
> >
> >  const struct clk_ops clk_mux_ops = {
> > .get_rate = clk_generic_get_rate,
> > .set_parent = clk_mux_set_parent,
> > +   .set_rate = clk_generic_set_rate,
> >  };
> >
> >  struct clk *clk_hw_register_mux_table(struct device *dev, const char *name,
> > diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
> > index ed6e60bc484..638864e6774 100644
> > --- a/drivers/clk/clk-uclass.c
> > +++ b/drivers/clk/clk-uclass.c
> > @@ -517,6 +517,26 @@ ulong clk_get_parent_rate(struct clk *clk)
> > return pclk->rate;
> >  }
> >
> > +ulong clk_set_parent_rate(struct clk *clk, ulong rate)
> > +{
> > +   const struct clk_ops *ops;
> > +   struct clk *pclk;
> > +
> > +   debug("%s(clk=%p)\n", __func__, clk);
> > +   if (!clk_valid(clk))
> > +   return 0;
> > +
> > 

How is it possible to flash an image (wic-file) to emmc?

2024-07-23 Thread Stahl, Michael
Hi,
after I swichted to the new uboot (2024.01) I can't flash a wic-file to the 
emmc anymore.

I use a uboot version for imx8 processors but I think the fastboot parts are 
equal. There is a special tool supported from NXP called uuu-tool to flash 
files by the fastboot protocol

With the old uboot (2021.04) I can flash the uboot-image as the wic-image as 
follows:

uuu.exe -b emmc flash.bin -> (flashes only the uboot)
uuu.exe -b emmc_all flash.bin wic.bin   -> (load the uboot into ram, start 
it and flash the wic-image)

My emmc has two separate boot-partitions with 16MB per partition. The emmc 
itself has 8GB

With the parameter -b the uuu-tool uses internal scripts.

Here is the simplified scirpt of emmc_all:
# @_flash.bin| bootloader
# @_image   [_flash.bin] | image burn to emmc, default is the same as bootloader

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

FB: ucmd setenv fastboot_dev mmc
FB: ucmd setenv mmcdev ${emmc_dev}
FB: ucmd mmc dev ${emmc_dev}
FB: flash -raw2sparse all _image
FB: flash -scanterm -scanlimited 0x80 bootloader _flash.bin
FB: ucmd if env exists emmc_ack; then ; else setenv emmc_ack 0; fi;
FB: ucmd mmc partconf ${emmc_dev} ${emmc_ack} 1 0
FB: done


My emmc device number is 2. So the environment variable emmc_dev is also 2. The 
-raw2sparse parameter let the uuu tool download and flash chunks of the 
wic-image-file.

I found out that build-in script "emmc" also not work anymore.

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

FB: ucmd setenv fastboot_dev mmc
FB: ucmd setenv mmcdev ${emmc_dev}
FB: ucmd mmc dev ${emmc_dev}
FB: flash bootloader _image
FB: ucmd if env exists emmc_ack; then ; else setenv emmc_ack 0; fi;
FB: ucmd mmc partconf ${emmc_dev} ${emmc_ack} 1 0
FB: Done

I have to change the bold marked line into:
"FB: flash boot0 _image" or "FB: flash boot1 _image"
to flash the bootloade into the boot partitions of my emmc

But I did not find out how to modify the "emmc_all" script to flash the entire 
emmc

The wic-file is an image which contains:
partition 1: kernel
partition 2: rootfs
partition 3: kernel
partition 4: rootfs
partition 5: user memory

This is what I expect what the fb_mmc.c  and  image-sparse.c  should do in 
Linux-like-syntax
dd if=wic-image of=/dev/mmcblk2 bs=1M

I put some debug messages into fb_mmc.c to have a closer look to the thinks 
what happen during the flashing.
It seems that always is looked for partitions and its not possible to write to 
the raw emmc.


Which fastboot command can I use to flash the raw emmc regardless of whether 
there are already partitions or not?





Mit freundlichen Grüßen / Best Regards

Michael Stahl
Staatl. gepr. Techniker
Lead Engineer - Advanced Development

MOBA Mobile Automation AG


[Location]  Kapellenstraße 15,  6 Limburg, Deutschland  [Globe] 
www.moba-automation.de<https://moba-automation.de>
[Mail.png]  mst...@moba.de<mailto:mst...@moba.de>   [phone] +49 
6431 9577-439
[MOBA Logo] [cid:2e5b8e33-e905-441f-ac10-1803e0a80da3] 
<https://www.xing.com/companies/mobamobileautomationag>   
<https://twitter.com/MOBA_Automation> 
[cid:d7cfd9f4-059e-414b-814d-493142624df5] 
<https://twitter.com/MOBA_Automation>   
[cid:117f85e5-b05d-4481-9dc6-5120cafd20f3] 
<https://de.linkedin.com/company/moba-mobile-automation>   
[cid:cb580079-18a8-43db-bdc1-db3fadc8d0fc] 
<https://www.facebook.com/MOBAWorldwide>   
[cid:682a418e-9b49-4793-a415-54f1aef33637] 
<https://www.youtube.com/user/MOBALive>

Chairman of the supervisory board: Ralf Konrad;
Executive board: Volker G. Harms, Dr. Holger Barthel;
Corporate headquarters: Limburg;
Register court: Limburg, HRB 2552, USt-IdNr.: DE 113865988

Information about the protection of your personal data can be found here:
https://moba-automation.de/datenschutzinfo-geschaeftspartner

This message is intended for the addressee only as it contains private and 
confidential information.
The contents are not to be disclosed to anyone other than the addressee.
Unauthorized recipients are requested to comply with the above and inform the 
sender immediately of any error in transmission
P Before printing this email, please consider if it´s really necessary


Re: [PATCH 13/14] spl: Create a function to init spl_load_info

2024-07-20 Thread Michael Nazzareno Trimarchi
into
> -* @return number of bytes read, 0 on error
> -*/
> -   ulong (*read)(struct spl_load_info *load, ulong sector, ulong count,
> - void *buf);
>  #if IS_ENABLED(CONFIG_SPL_LOAD_BLOCK)
> int bl_len;
>  #endif
> @@ -328,6 +332,18 @@ static inline void spl_set_bl_len(struct spl_load_info 
> *info, int bl_len)
>  #endif
>  }
>
> +/**
> + * spl_load_init() - Set up a new spl_load_info structure
> + */
> +static inline void spl_load_init(struct spl_load_info *load,
> +spl_load_reader h_read, void *priv,
> +uint bl_len)
> +{
> +   load->read = h_read;
> +   load->priv = priv;
> +   spl_set_bl_len(load, bl_len);
> +}
> +
>  /*
>   * We need to know the position of U-Boot in memory so we can jump to it. We
>   * allow any U-Boot binary to be used (u-boot.bin, u-boot-nodtb.bin,
> diff --git a/test/image/spl_load.c b/test/image/spl_load.c
> index 7cbad40ea0c..3b6206955d3 100644
> --- a/test/image/spl_load.c
> +++ b/test/image/spl_load.c
> @@ -343,9 +343,7 @@ static int spl_test_image(struct unit_test_state *uts, 
> const char *test_name,
>     } else {
>     struct spl_load_info load;
>
> -   spl_set_bl_len(, 1);
> -   load.priv = img;
> -   load.read = spl_test_read;
> +   spl_load_init(, spl_test_read, img, 1);
> if (type == IMX8)
> ut_assertok(spl_load_imx_container(_read, ,
>0));
> diff --git a/test/image/spl_load_os.c b/test/image/spl_load_os.c
> index 7d5fb9b07e0..dc09ba191bf 100644
> --- a/test/image/spl_load_os.c
> +++ b/test/image/spl_load_os.c
> @@ -49,9 +49,7 @@ static int spl_test_load(struct unit_test_state *uts)
> int ret;
> int fd;
>
> -   memset(, '\0', sizeof(load));
> -   spl_set_bl_len(, 512);
> -   load.read = read_fit_image;
> +   spl_load_init(, read_fit_image, _ctx, 512);
>
> ret = sandbox_find_next_phase(fname, sizeof(fname), true);
> if (ret)
> @@ -64,8 +62,6 @@ static int spl_test_load(struct unit_test_state *uts)
> ut_asserteq(512, os_read(fd, header, 512));
> text_ctx.fd = fd;
>
> -   load.priv = _ctx;
> -
> ut_assertok(spl_load_simple_fit(, , 0, header));
>
> return 0;
> --
> 2.34.1
>

Look straight forward with nice clean up

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 v2 2/2] spi: sunxi: fix clock divider calculation for max frequency setting

2024-07-18 Thread Michael Walle
If the maximum frequency is requested, we still fall into the CDR2
handling. But there the minimal divider is 2. For the sun6i and sun8i we
can do better with the CDR1 setting where the minimal divider is 1:
  SPI_CLK = MOD_CLK / 2 ^ cdr with cdr = 0

Thus, handle the div = 1 case specially.

While at it, correct the comment above the calculation.

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

diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index f110a8b7658..81f1298adea 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -249,6 +249,8 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
 * We have two choices there. Either we can use the clock
 * divide rate 1, which is calculated thanks to this formula:
 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
+* Or for sun6i/sun8i variants:
+* SPI_CLK = MOD_CLK / (2 ^ cdr)
 * Or we can use CDR2, which is calculated with the formula:
 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
 * Whether we use the former or the latter is set through the
@@ -256,13 +258,16 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
 *
 * First try CDR2, and if we can't reach the expected
 * frequency, fall back to CDR1.
+* There is one exception if the requested clock is the input
+* clock. In that case we always use CDR1 because we'll get a
+* 1:1 ration for sun6i/sun8i variants.
 */
 
div = DIV_ROUND_UP(SUNXI_INPUT_CLOCK, priv->freq);
div_cdr2 = DIV_ROUND_UP(div, 2);
reg = readl(SPI_REG(priv, SPI_CCR));
 
-   if (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
+   if (div != 1 && (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1))) {
reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
reg |= SUN4I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN4I_CLK_CTL_DRS;
} else {
-- 
2.39.2



[PATCH v2 1/2] spi: sunxi: fix CDR2 calculation

2024-07-18 Thread Michael Walle
The CDR2 divider calculation always yield a frequency greater than the
requested one. Use DIV_ROUND_UP() to keep the frequency equal or below
the requested one. This way, we can also drop the "if div > 0" check
because we know for a fact that div cannot be zero.

FWIW, this aligns the CDR2 calculation with the linux driver.

Suggested-by: Andre Przywara 
Signed-off-by: Michael Walle 
---
 drivers/spi/spi-sunxi.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index bfb402902b8..f110a8b7658 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -233,7 +233,7 @@ err_ahb:
 static void sun4i_spi_set_speed_mode(struct udevice *dev)
 {
struct sun4i_spi_priv *priv = dev_get_priv(dev);
-   unsigned int div;
+   unsigned int div, div_cdr2;
u32 reg;
 
/*
@@ -259,15 +259,12 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
 */
 
div = DIV_ROUND_UP(SUNXI_INPUT_CLOCK, priv->freq);
+   div_cdr2 = DIV_ROUND_UP(div, 2);
reg = readl(SPI_REG(priv, SPI_CCR));
 
-   if ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
-   div /= 2;
-   if (div > 0)
-   div--;
-
+   if (div_cdr2 <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
reg &= ~(SUN4I_CLK_CTL_CDR2_MASK | SUN4I_CLK_CTL_DRS);
-   reg |= SUN4I_CLK_CTL_CDR2(div) | SUN4I_CLK_CTL_DRS;
+   reg |= SUN4I_CLK_CTL_CDR2(div_cdr2 - 1) | SUN4I_CLK_CTL_DRS;
} else {
div = fls(div - 1);
/* The F1C100s encodes the divider as 2^(n+1) */
-- 
2.39.2



Re: [PATCH] mtd: nand: raw: atmel: Use ONFI ECC params if available

2024-07-18 Thread Michael Nazzareno Trimarchi
Hi

On Thu, Jul 18, 2024 at 9:40 PM Zixun Li  wrote:
>
> On 10/07/2024 09:28, Alexander Dahl wrote:
>
> > Despite the missing space between 'if' and the opening bracket (checkpatch
> > should have told you?) this looks very much like what got into linux with 
> > v4.11
> > but was changed/reworked multiple times afterwards.  I wonder why this was
> > not ported to U-Boot when introducing this driver?  In 6a8dfd57220d ("nand:
> > atmel: Add DM based NAND driver") it was claimed the driver was ported
> > from linux-5.4-at91, so this feature was probably dropped intentionally?
> > Does anyone know why?
> >
>
> Hi,
>
> Sorry for the format issue. Is there any comment from maintainers ?
>
> I'll push a v2 if this patch is considered.

Looking on it. Anyway expand the commit message with original information of it.
Like:

Upstream commit 

Try to report some original context from kernel

Micahel


> --
> Zixun LI
>
>


-- 
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 v6 0/6] Introduce UBI block device

2024-07-17 Thread Michael Nazzareno Trimarchi
Hi Alexey

On Thu, Jul 18, 2024 at 7:45 AM 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].
>

Did you run the CI on it? It's important not only that review get pass
but even we don't
create regression on other build

Michael

> ---
>
> Changes V1 -> V2 [2]:
>
>  - Rebased over mtdblock v2 patchset [3].
>  - Compile UBI partitions support only if CONFIG_BLK option
>is enabled.
>
> Changes V2 -> V3 [4]:
>
>  - Fix build warnings: use LBAF printf format string for lbaint_t types.
>
> Changes V3 -> V4 [5]:
>
>  - Rebased over u-boot/master.
>  - Fix build errors and warnings if CONFIG_BLK isn't enabled.
>  - Fix failed tests in cases is ubiblock device isn't binded.
>
> Changes V4 -> V5 [6]:
>
>   - Rebased over u-boot/master.
>   - Fix build errors when CONFIG_MTD and CONFIG_MTD_UBI isn't enabled.
>   - Use auto blk device devnum generation when call
> blk_create_devicef (pass -1 parameter instead of dev_seq(dev)).
>
> Changes V5 -> V6 [7]:
>
>   - Rebased over u-boot/master.
>   - Introduce CONFIG_UBI_BLOCK which is disabled by default.
>   - Fix undefined reference to `__aeabi_ldivmod' compiler error for 32-bit 
> ARM targets.
>
> 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/
>   [4] 
> https://lore.kernel.org/all/20240325144148.3738195-1-avroma...@salutedevices.com/
>   [5] 
> https://lore.kernel.org/all/20240524111319.3512009-1-avroma...@salutedevices.com/
>   [6] 
> https://lore.kernel.org/all/20240603155740.1840571-1-avroma...@salutedevices.com/
>   [7] 
> https://lore.kernel.org/all/20240626104527.2811828-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   |  78 --
>  disk/part.c |   7 ++
>  drivers/block/blk-uclass.c  |   1 +
>  drivers/mtd/nand/spi/core.c |  12 +++-
>  drivers/mtd/ubi/Kconfig |   7 ++
>  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 |  13 +++-
>  11 files changed, 348 insertions(+), 18 deletions(-)
>  create mode 100644 drivers/mtd/ubi/block.c
>  create mode 100644 drivers/mtd/ubi/part.c
>
> --
> 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 2/2] spi: sunxi: fix clock divider calculation for max frequency setting

2024-07-16 Thread Michael Walle
> > -   if ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
> > +   if (div != 1 && ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1))) {
> > div /= 2;
>
> This is still not fully correct, is it? If I ask for 10 MHz, the
> algorithm should select 8 MHz (24/3) or actually 6 MHz (24/4), but it
> chooses 12 MHz (24/2), which is too much.
> So I think this division here should be either:
>   div = (div + 1) / 2;
> or:
>   div = DIV_ROUND_UP(div, 2);
>
> Can someone confirm this?

When I've written this patch, I've looked at how linux does it (and
it's history) and I'm sure you know that linux has two drivers, for
sun4i and sun6i/sun8i. Somehow u-boot conflates them into one with
just one being correct with the SPI_CLK in the CDR2 case (?).

But anyway, this is about CDR1 and it seems you're right. But OTOH
I've tested this briefly with "sf probe 0:0 " and looked at the
SCK frequency of the readid command with a scope and it was always
less than my requested frequency. At least after the second probe
(there must be another bug which will still keep the frequency of
the probe at the former speed). Soo.. I'm not sure. Mh.

While this might be a bug, it doesn't affect this patch which will
just make sure we can get a 1:1 ratio on SoCs where this is
possible, i.e. not on the sun4i variant.

-michael


Re: [PATCH 1/2] spi: sunxi: drop max_hz handling

2024-07-16 Thread Michael Walle
Hi,

> > The driver is trying to read the "spi-max-frequency" property of the
> > *controller* driver node. There is no such property. The
> > "spi-max-frequency" property belongs to the SPI devices on the bus.
>
> Ah, indeed, good catch! Many thanks for sending this!
>  
> > Right now, the driver will always fall back to the default value of 1MHz
> > and thus flash reads are very slow with just about 215kb/s.
>
> That's even slower, right? I guess around 125 KB/s?

Yes of course :) 1Mhz/8 at most. I was fooled by the "sf update"
command which will skip the same sectors and then the overall speed
will be faster.

> > In fact, the SPI uclass will already take care of everything and we just
> > have to clamp the frequency to the values the driver/hardware supports.
> > Thus, drop the whole max_hz handling.
>
> Looks good to me, I verified this by timing the read, this patch indeed 
> significantly increases the performance. Also changing the limit in the
> DT gets reflected in the driver and in the read speed. Also verified
> that the values read from the SPI flash are the same in all cases.
>
> > Signed-off-by: Michael Walle 
>
> Reviewed-by: Andre Przywara 
> Tested-by: Andre Przywara 
>
> I will make this part of the first 2024.10 PR.

This means just 1/2 or both? Because there was no Rb on the second
patch.

-michael


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

2024-07-14 Thread Michael Nazzareno Trimarchi
Hi Fabio

On Mon, Apr 22, 2024 at 1:17 PM Fabio Estevam  wrote:
>
> Hi Michael,
>
> On Mon, Apr 22, 2024 at 7:36 AM Michael Nazzareno Trimarchi
>  wrote:
>
> > Are you considering if I wrap properly with VIDEO IS_ENABLED?
>
> I prefer you resend this patch as part of a complete series that adds
> panel support to an imx8mn board target.
>
> Otherwise, if this gets in as-is, it will become a dead code, which we
> want to avoid.

You can find here how video series is going, I will post it soon

https://patchwork.amarulasolutions.com/project/linux-amarula/list/

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 0/2] spi: sunxi: Improve the loading speed

2024-07-12 Thread Michael Walle

Right now, the maximal transfer speed from an SPI flash on a V3s is
about 240kb/s. That is pretty slow. It turns out, that due to an
error u-boot is setting the maximum frequency to 1MHz. By fixing
that another bug is unearthed: one cannot set a clock divider of 1:1
due to the handling between CDR1 and CDR2 handling. By fixing that
I achieved loading speeds of about 1.5MB/s.


Minor nit, should the clock fix go first so there's not a regression
if someone needs to do a bisect on the first commit?


Sure can do for the next version.

-michael


[PATCH 2/2] spi: sunxi: fix clock divider calculation for max frequency setting

2024-07-12 Thread Michael Walle
If the maximum frequency is requested, we still fall into the CDR2
handling. But there the minimal divider is 2. For the sun6i and sun8i we
can do better with the CDR1 setting where the minimal divider is 1:
  SPI_CLK = MOD_CLK / 2 ^ cdr with cdr = 0

Thus, handle the div = 1 case specially.

While at it, correct the comment above the calculation.

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

diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index bfb402902b8..3048ab0ecf7 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -249,6 +249,8 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
 * We have two choices there. Either we can use the clock
 * divide rate 1, which is calculated thanks to this formula:
 * SPI_CLK = MOD_CLK / (2 ^ (cdr + 1))
+* Or for sun6i/sun8i variants:
+* SPI_CLK = MOD_CLK / (2 ^ cdr)
 * Or we can use CDR2, which is calculated with the formula:
 * SPI_CLK = MOD_CLK / (2 * (cdr + 1))
 * Whether we use the former or the latter is set through the
@@ -256,12 +258,15 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
 *
 * First try CDR2, and if we can't reach the expected
 * frequency, fall back to CDR1.
+* There is one exception if the requested clock is the input
+* clock. In that case we always use CDR1 because we'll get a
+* 1:1: ration for sun6i/sun8i variants.
 */
 
div = DIV_ROUND_UP(SUNXI_INPUT_CLOCK, priv->freq);
reg = readl(SPI_REG(priv, SPI_CCR));
 
-   if ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1)) {
+   if (div != 1 && ((div / 2) <= (SUN4I_CLK_CTL_CDR2_MASK + 1))) {
div /= 2;
if (div > 0)
div--;
-- 
2.39.2



[PATCH 1/2] spi: sunxi: drop max_hz handling

2024-07-12 Thread Michael Walle
The driver is trying to read the "spi-max-frequency" property of the
*controller* driver node. There is no such property. The
"spi-max-frequency" property belongs to the SPI devices on the bus.

Right now, the driver will always fall back to the default value of 1MHz
and thus flash reads are very slow with just about 215kb/s.

In fact, the SPI uclass will already take care of everything and we just
have to clamp the frequency to the values the driver/hardware supports.
Thus, drop the whole max_hz handling.

Signed-off-by: Michael Walle 
---
 drivers/spi/spi-sunxi.c | 21 +
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c
index 13725ee7a2d..bfb402902b8 100644
--- a/drivers/spi/spi-sunxi.c
+++ b/drivers/spi/spi-sunxi.c
@@ -135,7 +135,6 @@ struct sun4i_spi_variant {
 struct sun4i_spi_plat {
struct sun4i_spi_variant *variant;
u32 base;
-   u32 max_hz;
 };
 
 struct sun4i_spi_priv {
@@ -237,6 +236,13 @@ static void sun4i_spi_set_speed_mode(struct udevice *dev)
unsigned int div;
u32 reg;
 
+   /*
+* The uclass should take care that this won't happen. But anyway,
+* avoid a div-by-zero exception.
+*/
+   if (!priv->freq)
+   return;
+
/*
 * Setup clock divider.
 *
@@ -401,11 +407,10 @@ static int sun4i_spi_xfer(struct udevice *dev, unsigned 
int bitlen,
 
 static int sun4i_spi_set_speed(struct udevice *dev, uint speed)
 {
-   struct sun4i_spi_plat *plat = dev_get_plat(dev);
struct sun4i_spi_priv *priv = dev_get_priv(dev);
 
-   if (speed > plat->max_hz)
-   speed = plat->max_hz;
+   if (speed > SUN4I_SPI_MAX_RATE)
+   speed = SUN4I_SPI_MAX_RATE;
 
if (speed < SUN4I_SPI_MIN_RATE)
speed = SUN4I_SPI_MIN_RATE;
@@ -458,7 +463,6 @@ static int sun4i_spi_probe(struct udevice *bus)
 
priv->variant = plat->variant;
priv->base = plat->base;
-   priv->freq = plat->max_hz;
 
return 0;
 }
@@ -466,16 +470,9 @@ static int sun4i_spi_probe(struct udevice *bus)
 static int sun4i_spi_of_to_plat(struct udevice *bus)
 {
struct sun4i_spi_plat *plat = dev_get_plat(bus);
-   int node = dev_of_offset(bus);
 
plat->base = dev_read_addr(bus);
plat->variant = (struct sun4i_spi_variant *)dev_get_driver_data(bus);
-   plat->max_hz = fdtdec_get_int(gd->fdt_blob, node,
- "spi-max-frequency",
- SUN4I_SPI_DEFAULT_RATE);
-
-   if (plat->max_hz > SUN4I_SPI_MAX_RATE)
-   plat->max_hz = SUN4I_SPI_MAX_RATE;
 
return 0;
 }
-- 
2.39.2



[PATCH 0/2] spi: sunxi: Improve the loading speed

2024-07-12 Thread Michael Walle
Right now, the maximal transfer speed from an SPI flash on a V3s is
about 240kb/s. That is pretty slow. It turns out, that due to an
error u-boot is setting the maximum frequency to 1MHz. By fixing
that another bug is unearthed: one cannot set a clock divider of 1:1
due to the handling between CDR1 and CDR2 handling. By fixing that
I achieved loading speeds of about 1.5MB/s.

Michael Walle (2):
  spi: sunxi: drop max_hz handling
  spi: sunxi: fix clock divider calculation for max frequency setting

 drivers/spi/spi-sunxi.c | 28 +++-
 1 file changed, 15 insertions(+), 13 deletions(-)

-- 
2.39.2



[PATCH] sunxi: board: probe USB gadget

2024-07-11 Thread Michael Walle
Due to the lazy probing, the gadget driver might not be probed when the
u-boot cli is active. In this case the "ums" command won't work, for
example. If enabled, probe the USB gadget during board_init().

Signed-off-by: Michael Walle 
---
 board/sunxi/board.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/board/sunxi/board.c b/board/sunxi/board.c
index ed86f1df5dc..7f93aba9ee7 100644
--- a/board/sunxi/board.c
+++ b/board/sunxi/board.c
@@ -189,6 +189,7 @@ enum env_location env_get_location(enum env_operation op, 
int prio)
 int board_init(void)
 {
__maybe_unused int id_pfr1, ret;
+   struct udevice *dev;
 
gd->bd->bi_boot_params = (PHYS_SDRAM_0 + 0x100);
 
@@ -225,6 +226,9 @@ int board_init(void)
if (ret)
return ret;
 
+   if (CONFIG_IS_ENABLED(USB_GADGET))
+   uclass_get_device(UCLASS_USB_GADGET_GENERIC, 0, );
+
eth_init_board();
 
return 0;
-- 
2.39.2



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

2024-07-11 Thread Michael Walle
On Tue May 14, 2024 at 1:43 AM CEST, Michael Walle wrote:
> 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.

Also here.. any news?

-michael


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

2024-07-11 Thread Michael Walle
Hi,

On Mon May 13, 2024 at 10:56 PM CEST, Michael Walle wrote:
> 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.

Any news here?

-michael

>
> 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(+)



Re: [PATCH 1/4] clk: imx: clk-imx8mn Fix nand and spi clock parent

2024-07-10 Thread Michael Nazzareno Trimarchi
HI Fabio

On Wed, Jul 10, 2024 at 3:43 PM Fabio Estevam  wrote:
>
> On Wed, Jul 10, 2024 at 10:39 AM Michael Nazzareno Trimarchi
>  wrote:
>
> > clk dump
> >
> > It's how uboot is registering the clk in dts. My idea now was not to
> > change it to not impact other architectures.
> > I have a lot of changes but I don't want to address regression and fix
> > and test before send more patches
>
> Ok, I see that 'clock-osc-24m' is already used in U-Boot by the other
> peripherals.
>
> Your change looks correct.
>
> Ideally, we should use 'osc_24m' in the future to align with the
> kernel, but that's a different task.

I will rephrase the commit message

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 1/4] clk: imx: clk-imx8mn Fix nand and spi clock parent

2024-07-10 Thread Michael Nazzareno Trimarchi
Hi

On Wed, Jul 10, 2024 at 3:34 PM Fabio Estevam  wrote:
>
> Hi Michael,
>
> On Sun, Jul 7, 2024 at 5:20 AM Michael Trimarchi
>  wrote:
>
> > -static const char *imx8mn_ecspi1_sels[] = {"osc_24m", "sys_pll2_200m", 
> > "sys_pll1_40m",
> > +static const char *imx8mn_ecspi1_sels[] = {"clock-osc-24m", 
> > "sys_pll2_200m", "sys_pll1_40m",
>
> The kernel uses osc_24m. Why does U-Boot need to be different?

clk dump

It's how uboot is registering the clk in dts. My idea now was not to
change it to not impact other architectures.
I have a lot of changes but I don't want to address regression and fix
and test before send more patches

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: clk-uclass: Print clk name in clk_enable/clk_disable

2024-07-09 Thread Michael Trimarchi
Print clk name in clk_enable and clk_disable. Make sense to know
what clock get disabled/enabled before a system crash or system
hang.

Signed-off-by: Michael Trimarchi 
---
 drivers/clk/clk-uclass.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index d768e5ad2e..76f1026164 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -630,7 +630,7 @@ int clk_enable(struct clk *clk)
struct clk *clkp = NULL;
int ret;
 
-   debug("%s(clk=%p)\n", __func__, clk);
+   debug("%s(clk=%p name=%s)\n", __func__, clk, clk->dev->name);
if (!clk_valid(clk))
return 0;
ops = clk_dev_ops(clk->dev);
@@ -691,7 +691,7 @@ int clk_disable(struct clk *clk)
struct clk *clkp = NULL;
int ret;
 
-   debug("%s(clk=%p)\n", __func__, clk);
+   debug("%s(clk=%p name=%s)\n", __func__, clk, clk->dev->name);
if (!clk_valid(clk))
return 0;
ops = clk_dev_ops(clk->dev);
-- 
2.43.0



[PATCH 4/4] clk: imx8mp: Make parent names arrays const pointers

2024-07-07 Thread Michael Trimarchi
The arrays containing the mux selectors need to be of const pointer
to const char.

Signed-off-by: Michael Trimarchi 
---
 drivers/clk/imx/clk-imx8mp.c | 242 +--
 1 file changed, 121 insertions(+), 121 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mp.c b/drivers/clk/imx/clk-imx8mp.c
index 1f498b6ba4..6b18483c81 100644
--- a/drivers/clk/imx/clk-imx8mp.c
+++ b/drivers/clk/imx/clk-imx8mp.c
@@ -14,178 +14,178 @@
 
 #include "clk.h"
 
-static const char *pll_ref_sels[] = { "clock-osc-24m", "dummy", "dummy", 
"dummy", };
-static const char *dram_pll_bypass_sels[] = {"dram_pll", "dram_pll_ref_sel", };
-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 * const pll_ref_sels[] = { "clock-osc-24m", "dummy", 
"dummy", "dummy", };
+static const char * const dram_pll_bypass_sels[] = {"dram_pll", 
"dram_pll_ref_sel", };
+static const char * const arm_pll_bypass_sels[] = {"arm_pll", 
"arm_pll_ref_sel", };
+static const char * const sys_pll1_bypass_sels[] = {"sys_pll1", 
"sys_pll1_ref_sel", };
+static const char * const sys_pll2_bypass_sels[] = {"sys_pll2", 
"sys_pll2_ref_sel", };
+static const char * const sys_pll3_bypass_sels[] = {"sys_pll3", 
"sys_pll3_ref_sel", };
 
-static const char *imx8mp_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", };
+static const char * const imx8mp_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", };
 
-static const char *imx8mp_hsio_axi_sels[] = {"clock-osc-24m", "sys_pll2_500m", 
"sys_pll1_800m",
-"sys_pll2_100m", "sys_pll2_200m", 
"clk_ext2",
-"clk_ext4", "audio_pll2_out", };
+static const char * const imx8mp_hsio_axi_sels[] = {"clock-osc-24m", 
"sys_pll2_500m", "sys_pll1_800m",
+   "sys_pll2_100m", 
"sys_pll2_200m", "clk_ext2",
+   "clk_ext4", 
"audio_pll2_out", };
 
-static const char *imx8mp_main_axi_sels[] = {"clock-osc-24m", "sys_pll2_333m", 
"sys_pll1_800m",
-"sys_pll2_250m", "sys_pll2_1000m", 
"audio_pll1_out",
-"video_pll1_out", 
"sys_pll1_100m",};
+static const char * const imx8mp_main_axi_sels[] = {"clock-osc-24m", 
"sys_pll2_333m", "sys_pll1_800m",
+   "sys_pll2_250m", 
"sys_pll2_1000m", "audio_pll1_out",
+   "video_pll1_out", 
"sys_pll1_100m",};
 
-static const char *imx8mp_enet_axi_sels[] = {"clock-osc-24m", "sys_pll1_266m", 
"sys_pll1_800m",
-"sys_pll2_250m", "sys_pll2_200m", 
"audio_pll1_out",
-"video_pll1_out", "sys_pll3_out", 
};
+static const char * const imx8mp_enet_axi_sels[] = {"clock-osc-24m", 
"sys_pll1_266m", "sys_pll1_800m",
+   "sys_pll2_250m", 
"sys_pll2_200m", "audio_pll1_out",
+   "video_pll1_out", 
"sys_pll3_out", };
 
-static const char *imx8mp_nand_usdhc_sels[] = {"clock-osc-24m", 
"sys_pll1_266m", "sys_pll1_800m",
-  "sys_pll2_200m", 
"sys_pll1_133m", "sys_pll3_out",
-   

[PATCH 3/4] clk: imx8mm: Make parent names arrays const pointers

2024-07-07 Thread Michael Trimarchi
The arrays containing the mux selectors need to be of const pointer
to const char.

Signed-off-by: Michael Trimarchi 
---
 drivers/clk/imx/clk-imx8mm.c | 157 +--
 1 file changed, 93 insertions(+), 64 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mm.c b/drivers/clk/imx/clk-imx8mm.c
index 70e2e53bde..e538f047b3 100644
--- a/drivers/clk/imx/clk-imx8mm.c
+++ b/drivers/clk/imx/clk-imx8mm.c
@@ -14,108 +14,137 @@
 
 #include "clk.h"
 
-static const char *pll_ref_sels[] = { "clock-osc-24m", "dummy", "dummy", 
"dummy", };
-static const char *dram_pll_bypass_sels[] = {"dram_pll", "dram_pll_ref_sel", };
-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 * const pll_ref_sels[] = { "clock-osc-24m", "dummy", 
"dummy", "dummy", };
+static const char * const dram_pll_bypass_sels[] = {"dram_pll", 
"dram_pll_ref_sel", };
+static const char * const arm_pll_bypass_sels[] = {"arm_pll", 
"arm_pll_ref_sel", };
+static const char * const sys_pll1_bypass_sels[] = {"sys_pll1", 
"sys_pll1_ref_sel", };
+static const char * const sys_pll2_bypass_sels[] = {"sys_pll2", 
"sys_pll2_ref_sel", };
+static const char * const sys_pll3_bypass_sels[] = {"sys_pll3", 
"sys_pll3_ref_sel", };
 
-static const char *imx8mm_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", };
+static const char * const imx8mm_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", };
 
-static const char *imx8mm_ahb_sels[] = {"clock-osc-24m", "sys_pll1_133m", 
"sys_pll1_800m", "sys_pll1_400m",
-   "sys_pll2_125m", "sys_pll3_out", 
"audio_pll1_out", "video_pll1_out", };
+static const char * const imx8mm_ahb_sels[] = {"clock-osc-24m", 
"sys_pll1_133m", "sys_pll1_800m",
+  "sys_pll1_400m", 
"sys_pll2_125m", "sys_pll3_out",
+  "audio_pll1_out", 
"video_pll1_out", };
 
 #ifndef CONFIG_SPL_BUILD
-static const char *imx8mm_enet_axi_sels[] = {"clock-osc-24m", "sys_pll1_266m", 
"sys_pll1_800m", "sys_pll2_250m",
-"sys_pll2_200m", "audio_pll1_out", 
"video_pll1_out", "sys_pll3_out", };
+static const char * const imx8mm_enet_axi_sels[] = {"clock-osc-24m", 
"sys_pll1_266m", "sys_pll1_800m",
+   "sys_pll2_250m", 
"sys_pll2_200m", "audio_pll1_out",
+   "video_pll1_out", 
"sys_pll3_out", };
 
-static const char *imx8mm_enet_ref_sels[] = {"clock-osc-24m", "sys_pll2_125m", 
"sys_pll2_50m", "sys_pll2_100m",
-"sys_pll1_160m", "audio_pll1_out", 
"video_pll1_out", "clk_ext4", };
+static const char * const imx8mm_enet_ref_sels[] = {"clock-osc-24m", 
"sys_pll2_125m", "sys_pll2_50m",
+   "sys_pll2_100m", 
"sys_pll1_160m", "audio_pll1_out",
+   "video_pll1_out", 
"clk_ext4", };
 
-static const char *imx8mm_enet_timer_sels[] = {"clock-osc-24m", 
"sys_pll2_100m", "audio_pll1_out", "clk_ext1", "clk_ext2",
-  "clk_ext3", "clk_ext4", 
"video_pll1_out", };
+static const char * const imx8mm_enet_timer_sels[] = {"clock-osc-24m", 
"sys_pll2_100m", "audio_pll1_

[PATCH 2/4] clk: imx8mn: Make parent names arrays const pointers

2024-07-07 Thread Michael Trimarchi
The arrays containing the mux selectors need to be of const pointer
to const char.

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

diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index bfd1677520..8911e342f1 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -16,106 +16,126 @@
 
 static u32 share_count_nand;
 
-static const char *pll_ref_sels[] = { "clock-osc-24m", "dummy", "dummy", 
"dummy", };
-static const char *dram_pll_bypass_sels[] = {"dram_pll", "dram_pll_ref_sel", };
-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 * const pll_ref_sels[] = { "clock-osc-24m", "dummy", 
"dummy", "dummy", };
+static const char * const dram_pll_bypass_sels[] = {"dram_pll", 
"dram_pll_ref_sel", };
+static const char * const arm_pll_bypass_sels[] = {"arm_pll", 
"arm_pll_ref_sel", };
+static const char * const sys_pll1_bypass_sels[] = {"sys_pll1", 
"sys_pll1_ref_sel", };
+static const char * const sys_pll2_bypass_sels[] = {"sys_pll2", 
"sys_pll2_ref_sel", };
+static const char * const sys_pll3_bypass_sels[] = {"sys_pll3", 
"sys_pll3_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", };
+static const char * const 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", };
 
-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 * const 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_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", };
+static const char * const 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", };
 
 #ifndef CONFIG_SPL_BUILD
-static const char *imx8mn_enet_ref_sels[] = {"clock-osc-24m", "sys_pll2_125m", 
"sys_pll2_50m", "sys_pll2_100m",
-"sys_pll1_160m", "audio_pll1_out", 
"video_pll_out", "clk_ext4", };
+static const char * const imx8mn_enet_ref_sels[] = {"clock-osc-24m", 
"sys_pll2_125m", "sys_pll2_50m",
+   "sys_pll2_100m", 
"sys_pll1_160m", "audio_pll1_out",
+   "video_pll_out", 
"clk_ext4", };
 
-static const char *imx8mn_enet_timer_sels[] = {"clock-osc-24m", 
"sys_pll2_100m", "audio_pll1_out", "clk_ext1", "clk_ext2",
-  "clk_ext3", "clk_ext4", 
"video_pll_out", };
+static const char * const imx8mn_enet_timer_sels[] = {"clock-osc-24m", 
"sys_pll2_100m", "audio_pll1_out",
+

[PATCH 1/4] clk: imx: clk-imx8mn Fix nand and spi clock parent

2024-07-07 Thread Michael Trimarchi
The osc_24m is the clock-output-name and not the one that
is used as internal name reference from the strcmp. The clock
that use osc_24m, will not be able to reparent it as they should.
We need anyway register the osc_24m clock fixed factor in the clock
tree.

Fixes: 710c4ffb890 ("clk: imx: clk-imx8mn add gpmi nand clocks")
Fixes: 85b1c11989c ("clk: imx: Add ECSPI to iMX8MN")
Cc: Marek Vasut 
Signed-off-by: Michael Trimarchi 
---
 drivers/clk/imx/clk-imx8mn.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/imx/clk-imx8mn.c b/drivers/clk/imx/clk-imx8mn.c
index ed9e16d7c1..bfd1677520 100644
--- a/drivers/clk/imx/clk-imx8mn.c
+++ b/drivers/clk/imx/clk-imx8mn.c
@@ -57,15 +57,15 @@ static const char *imx8mn_usdhc2_sels[] = {"clock-osc-24m", 
"sys_pll1_400m", "sy
   "sys_pll3_out", "sys_pll1_266m", 
"audio_pll2_out", "sys_pll1_100m", };
 
 #if CONFIG_IS_ENABLED(DM_SPI)
-static const char *imx8mn_ecspi1_sels[] = {"osc_24m", "sys_pll2_200m", 
"sys_pll1_40m",
+static const char *imx8mn_ecspi1_sels[] = {"clock-osc-24m", "sys_pll2_200m", 
"sys_pll1_40m",
   "sys_pll1_160m", "sys_pll1_800m", 
"sys_pll3_out",
   "sys_pll2_250m", "audio_pll2_out", };
 
-static const char *imx8mn_ecspi2_sels[] = {"osc_24m", "sys_pll2_200m", 
"sys_pll1_40m",
+static const char *imx8mn_ecspi2_sels[] = {"clock-osc-24m", "sys_pll2_200m", 
"sys_pll1_40m",
   "sys_pll1_160m", "sys_pll1_800m", 
"sys_pll3_out",
   "sys_pll2_250m", "audio_pll2_out", };
 
-static const char *imx8mn_ecspi3_sels[] = {"osc_24m", "sys_pll2_200m", 
"sys_pll1_40m",
+static const char *imx8mn_ecspi3_sels[] = {"clock-osc-24m", "sys_pll2_200m", 
"sys_pll1_40m",
   "sys_pll1_160m", "sys_pll1_800m", 
"sys_pll3_out",
   "sys_pll2_250m", "audio_pll2_out", };
 #endif
@@ -105,7 +105,7 @@ static const char *imx8mn_usdhc3_sels[] = {"clock-osc-24m", 
"sys_pll1_400m", "sy
 static const char *imx8mn_qspi_sels[] = {"clock-osc-24m", "sys_pll1_400m", 
"sys_pll2_333m", "sys_pll2_500m",
   "audio_pll2_out", "sys_pll1_266m", 
"sys_pll3_out", "sys_pll1_100m", };
 
-static const char * const imx8mn_nand_sels[] = {"osc_24m", "sys_pll2_500m", 
"audio_pll1_out",
+static const char * const imx8mn_nand_sels[] = {"clock-osc-24m", 
"sys_pll2_500m", "audio_pll1_out",
"sys_pll1_400m", 
"audio_pll2_out", "sys_pll3_out",
"sys_pll2_250m", 
"video_pll_out", };
 
@@ -119,7 +119,9 @@ static const char * const imx8mn_usb_phy_sels[] = 
{"clock-osc-24m", "sys_pll1_10
 
 static int imx8mn_clk_probe(struct udevice *dev)
 {
+   struct clk osc_24m_clk;
void __iomem *base;
+   int ret;
 
base = (void *)ANATOP_BASE_ADDR;
 
@@ -238,6 +240,11 @@ static int imx8mn_clk_probe(struct udevice *dev)
clk_dm(IMX8MN_SYS_PLL2_1000M,
   imx_clk_fixed_factor("sys_pll2_1000m", "sys_pll2_out", 1, 1));
 
+   ret = clk_get_by_name(dev, "osc_24m", _24m_clk);
+   if (ret)
+   return ret;
+   clk_dm(IMX8MN_CLK_24M, dev_get_clk_ptr(osc_24m_clk.dev));
+
base = dev_read_addr_ptr(dev);
if (!base)
return -EINVAL;
-- 
2.43.0



[PATCH 0/4] IMX8 series small clock update

2024-07-07 Thread Michael Trimarchi
This series is cover other part of my internal review work, that
will allow us to support the video subsystem of imx8mn. During this work
I found some small issues that fix several small problems but it will
help as preparation for the video one

Michael Trimarchi (4):
  clk: imx: clk-imx8mn Fix nand and spi clock parent
  clk: imx8mn: Make parent names arrays const pointers
  clk: imx8mm: Make parent names arrays const pointers
  clk: imx8mp: Make parent names arrays const pointers

 drivers/clk/imx/clk-imx8mm.c | 157 ++-
 drivers/clk/imx/clk-imx8mn.c | 151 +-
 drivers/clk/imx/clk-imx8mp.c | 242 +--
 3 files changed, 303 insertions(+), 247 deletions(-)

-- 
2.43.0



[PATCH V2] clk: imx: add mux ops for i.MX8M composite clk

2024-07-05 Thread Michael Trimarchi
Upstream Linux commit f90b68d6c8b0.

The CORE/BUS root slice has following design, simplied graph:
The difference is core not have pre_div block.
A composite core/bus clk has 8 inputs for mux to select, saying clk[0-7].

It support target(smart) interface and normal interface. Target interface
is exported for programmer easy to configure ccm root. Normal interface
is also exported, but we not use it in our driver, because it will
introduce more complexity compared with target interface.

The normal interface simplified as below:
SEL_A  GA
+--+  +-+
|  +->+ +--+
CLK[0-7]--->+  |  +-+  |
   ||  |  +v---+++
   |+--+  |pre_diva+>|  +-+
   |  ++|mux +--+post_div |
   |+--+  |pre_divb+--->+|  +-+
   ||  |  +^---+++
   +--->+  |  +-+  |
|  +->+ +--+
+--+  +-+
SEL_B  GB

The mux in the upper pic is not the target interface MUX, target
interface MUX is hiding SEL_A and SEL_B. When you choose clk[0-7],
you are actually writing SEL_A or SEL_B depends on the internal
counter which will also control the internal "mux".

The target interface simplified as below which is used by Linux Kernel:
CLK[0-7]--->MUX-->Gate-->pre_div-->post_div

A requirement of the Target Interface's software is that the
target clock source is active, it means when setting SEL_A, the
current input clk to SEL_A must be active, same to SEL_B.

We touch target interface, but hardware logic actually also need
configure normal interface.

There will be system hang, when doing the following steps:
The initial state:
SEL_A/SEL_B are both sourcing from clk0, the internal counter
choose SEL_A.
1. switch mux from clk0 to clk1
   The hardware logic will choose SEL_B and configure SEL_B to clk1.
   SEL_A no changed.
2. gate off clk0
   Disable clk0, then the input to SEL_A is off.
3. switch from clk1 to clk2
   The hardware logic will choose SEL_A and configure SEL_A to clk2,
   however the current SEL_A input clk0 is off, the system hang.

The solution to fix the issue is in step 1, write twice to
target interface MUX, it will make SEL_A/SEL_B both sources
from clk1, then no need to care about the state of clk0. And
finally system performs well.

Signed-off-by: Peng Fan 
Reviewed-by: Dong Aisheng 
Signed-off-by: Shawn Guo 
Signed-off-by: Michael Trimarchi 
---
V1->V2:
add original submitter in linux kernel, suggested by Fabio
Estevam
---
 drivers/clk/imx/clk-composite-8m.c | 37 +-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-composite-8m.c 
b/drivers/clk/imx/clk-composite-8m.c
index 560d74aac80..2cb7d135000 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -117,6 +117,41 @@ static const struct clk_ops 
imx8m_clk_composite_divider_ops = {
.set_rate = imx8m_clk_composite_divider_set_rate,
 };
 
+static int imx8m_clk_mux_set_parent(struct clk *clk, struct clk *parent)
+{
+   struct clk_mux *mux = to_clk_mux(clk);
+   int index;
+   u32 val;
+   u32 reg;
+
+   index = clk_mux_fetch_parent_index(clk, parent);
+   if (index < 0) {
+   log_err("Could not fetch index\n");
+   return index;
+   }
+
+   val = clk_mux_index_to_val(mux->table, mux->flags, index);
+
+   reg = readl(mux->reg);
+   reg &= ~(mux->mask << mux->shift);
+   val = val << mux->shift;
+   reg |= val;
+
+   /*
+* write twice to make sure non-target interface
+* SEL_A/B point the same clk input.
+*/
+   writel(reg, mux->reg);
+   writel(reg, mux->reg);
+
+   return 0;
+}
+
+const struct clk_ops imx8m_clk_mux_ops = {
+   .get_rate = clk_generic_get_rate,
+   .set_parent = imx8m_clk_mux_set_parent,
+};
+
 struct clk *imx8m_clk_composite_flags(const char *name,
  const char * const *parent_names,
  int num_parents, void __iomem *reg,
@@ -155,7 +190,7 @@ struct clk *imx8m_clk_composite_flags(const char *name,
 
clk = clk_register_composite(NULL, name,
 parent_names, num_parents,
->clk, _mux_ops, >clk,
+>clk, _clk_mux_ops, >clk,
 _clk_composite_divider_ops,
 >clk, _gate_ops, flags);
if (IS_ERR(clk))
-- 
2.43.0



[PATCH V2 1/2] clk: clk-mux: Make public the clk_fetch_parent_index

2024-07-05 Thread Michael Trimarchi
Make public the clk_fetch_parent_index and rename it. This allow
us to be reused in driver specialization

Signed-off-by: Michael Trimarchi 
---
V1->V2:
Nothing changed
---
 drivers/clk/clk-mux.c| 5 ++---
 include/linux/clk-provider.h | 1 +
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index f410518461e..e3481be95fa 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -102,8 +102,7 @@ u8 clk_mux_get_parent(struct clk *clk)
return clk_mux_val_to_index(clk, mux->table, mux->flags, val);
 }
 
-static int clk_fetch_parent_index(struct clk *clk,
- struct clk *parent)
+int clk_mux_fetch_parent_index(struct clk *clk, struct clk *parent)
 {
struct clk_mux *mux = to_clk_mux(clk);
 
@@ -127,7 +126,7 @@ static int clk_mux_set_parent(struct clk *clk, struct clk 
*parent)
u32 val;
u32 reg;
 
-   index = clk_fetch_parent_index(clk, parent);
+   index = clk_mux_fetch_parent_index(clk, parent);
if (index < 0) {
log_err("Could not fetch index\n");
return index;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index b8acacd49ee..59f9c241b84 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -74,6 +74,7 @@ struct clk_mux {
 #define to_clk_mux(_clk) container_of(_clk, struct clk_mux, clk)
 extern const struct clk_ops clk_mux_ops;
 u8 clk_mux_get_parent(struct clk *clk);
+int clk_mux_fetch_parent_index(struct clk *clk, struct clk *parent);
 
 /**
  * clk_mux_index_to_val() - Convert the parent index to the register value
-- 
2.43.0



[PATCH V2 0/2] Support imx8m composite mux

2024-07-05 Thread Michael Trimarchi
The mux ops for i.MX8M composite clk needs to be specialized.
In order to reduce the code duplication we need to make public some
interface and write a specific function in the mux ops. Those patches
implement the behavior.

Addressed comment in V1

Michael Trimarchi (2):
  clk: clk-mux: Make public the clk_fetch_parent_index
  clk: imx: add mux ops for i.MX8M composite clk

 drivers/clk/clk-mux.c  |  5 ++--
 drivers/clk/imx/clk-composite-8m.c | 37 +-
 include/linux/clk-provider.h   |  1 +
 3 files changed, 39 insertions(+), 4 deletions(-)

-- 
2.43.0



[PATCH V2] clk: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present for gate and mux

2024-07-05 Thread Michael Trimarchi
Gate and mux does not have .set_rate operation, but they could have
CLK_SET_PARENT_RATE flag set. In that case it's usually possible to find a
parent up the tree which is capable of setting the rate (div, pll, etc).
Add clk_generic_set_rate to allow them to trasverse the clock tree.

Cc: Sam Protsenko 
Signed-off-by: Michael Trimarchi 
---
 drivers/clk/clk-gate.c   |  1 +
 drivers/clk/clk-mux.c|  2 +-
 drivers/clk/clk-uclass.c | 20 
 drivers/clk/clk.c|  9 +
 include/clk.h|  9 +
 include/linux/clk-provider.h |  1 +
 6 files changed, 41 insertions(+), 1 deletion(-)
---
V1->V2:
Fix missing include
---
diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index cfd90b717e7..c86083ac5a3 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -116,6 +116,7 @@ const struct clk_ops clk_gate_ops = {
.enable = clk_gate_enable,
.disable = clk_gate_disable,
.get_rate = clk_generic_get_rate,
+   .set_rate = clk_generic_set_rate,
 };
 
 struct clk *clk_register_gate(struct device *dev, const char *name,
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index e3481be95fa..f99a67ebd35 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -151,13 +151,13 @@ static int clk_mux_set_parent(struct clk *clk, struct clk 
*parent)
 #else
writel(reg, mux->reg);
 #endif
-
return 0;
 }
 
 const struct clk_ops clk_mux_ops = {
.get_rate = clk_generic_get_rate,
.set_parent = clk_mux_set_parent,
+   .set_rate = clk_generic_set_rate,
 };
 
 struct clk *clk_hw_register_mux_table(struct device *dev, const char *name,
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index ed6e60bc484..638864e6774 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -517,6 +517,26 @@ ulong clk_get_parent_rate(struct clk *clk)
return pclk->rate;
 }
 
+ulong clk_set_parent_rate(struct clk *clk, ulong rate)
+{
+   const struct clk_ops *ops;
+   struct clk *pclk;
+
+   debug("%s(clk=%p)\n", __func__, clk);
+   if (!clk_valid(clk))
+   return 0;
+
+   pclk = clk_get_parent(clk);
+   if (IS_ERR(pclk))
+   return -ENODEV;
+
+   ops = clk_dev_ops(pclk->dev);
+   if (!ops->set_rate)
+   return -ENOSYS;
+
+   return clk_set_rate(pclk, rate);
+}
+
 ulong clk_round_rate(struct clk *clk, ulong rate)
 {
const struct clk_ops *ops;
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 6ede1b4d4dc..febd5314df2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 int clk_register(struct clk *clk, const char *drv_name,
 const char *name, const char *parent_name)
@@ -61,6 +62,14 @@ ulong clk_generic_get_rate(struct clk *clk)
return clk_get_parent_rate(clk);
 }
 
+ulong clk_generic_set_rate(struct clk *clk, ulong rate)
+{
+   if (clk->flags & CLK_SET_RATE_PARENT)
+   return clk_set_parent_rate(clk, rate);
+
+   return clk_get_parent_rate(clk);
+}
+
 const char *clk_hw_get_name(const struct clk *hw)
 {
assert(hw);
diff --git a/include/clk.h b/include/clk.h
index af23e4f3475..1900377eddb 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -452,6 +452,15 @@ struct clk *clk_get_parent(struct clk *clk);
  */
 ulong clk_get_parent_rate(struct clk *clk);
 
+/**
+ * clk_set_parent_rate() - Set parent of current clock rate.
+ * @clk:   A clock struct that was previously successfully requested by
+ * clk_request/get_by_*().
+ *
+ * Return: clock rate in Hz, or -ve error code.
+ */
+ulong clk_set_parent_rate(struct clk *clk, ulong rate);
+
 /**
  * clk_round_rate() - Adjust a rate to the exact rate a clock can provide
  * @clk: A clock struct that was previously successfully requested by
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 59f9c241b84..459fa2d15ce 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -253,6 +253,7 @@ struct clk *clk_register_fixed_rate(struct device *dev, 
const char *name,
 
 const char *clk_hw_get_name(const struct clk *hw);
 ulong clk_generic_get_rate(struct clk *clk);
+ulong clk_generic_set_rate(struct clk *clk, ulong rate);
 
 struct clk *dev_get_clk_ptr(struct udevice *dev);
 
-- 
2.43.0



[PATCH] clk: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present for gate and mux

2024-07-04 Thread Michael Trimarchi
Gate and mux does not have .set_rate operation, but they could have
CLK_SET_PARENT_RATE flag set. In that case it's usually possible to find a
parent up the tree which is capable of setting the rate (div, pll, etc).
Add clk_generic_set_rate to allow them to trasverse the clock tree.

Cc: Sam Protsenko 
Signed-off-by: Michael Trimarchi 
---
 drivers/clk/clk-gate.c   |  1 +
 drivers/clk/clk-mux.c|  2 +-
 drivers/clk/clk-uclass.c | 20 
 drivers/clk/clk.c|  9 +
 4 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-gate.c b/drivers/clk/clk-gate.c
index cfd90b717e7..c86083ac5a3 100644
--- a/drivers/clk/clk-gate.c
+++ b/drivers/clk/clk-gate.c
@@ -116,6 +116,7 @@ const struct clk_ops clk_gate_ops = {
.enable = clk_gate_enable,
.disable = clk_gate_disable,
.get_rate = clk_generic_get_rate,
+   .set_rate = clk_generic_set_rate,
 };
 
 struct clk *clk_register_gate(struct device *dev, const char *name,
diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index e3481be95fa..f99a67ebd35 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -151,13 +151,13 @@ static int clk_mux_set_parent(struct clk *clk, struct clk 
*parent)
 #else
writel(reg, mux->reg);
 #endif
-
return 0;
 }
 
 const struct clk_ops clk_mux_ops = {
.get_rate = clk_generic_get_rate,
.set_parent = clk_mux_set_parent,
+   .set_rate = clk_generic_set_rate,
 };
 
 struct clk *clk_hw_register_mux_table(struct device *dev, const char *name,
diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index ed6e60bc484..638864e6774 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -517,6 +517,26 @@ ulong clk_get_parent_rate(struct clk *clk)
return pclk->rate;
 }
 
+ulong clk_set_parent_rate(struct clk *clk, ulong rate)
+{
+   const struct clk_ops *ops;
+   struct clk *pclk;
+
+   debug("%s(clk=%p)\n", __func__, clk);
+   if (!clk_valid(clk))
+   return 0;
+
+   pclk = clk_get_parent(clk);
+   if (IS_ERR(pclk))
+   return -ENODEV;
+
+   ops = clk_dev_ops(pclk->dev);
+   if (!ops->set_rate)
+   return -ENOSYS;
+
+   return clk_set_rate(pclk, rate);
+}
+
 ulong clk_round_rate(struct clk *clk, ulong rate)
 {
const struct clk_ops *ops;
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 6ede1b4d4dc..febd5314df2 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -14,6 +14,7 @@
 #include 
 #include 
 #include 
+#include 
 
 int clk_register(struct clk *clk, const char *drv_name,
 const char *name, const char *parent_name)
@@ -61,6 +62,14 @@ ulong clk_generic_get_rate(struct clk *clk)
return clk_get_parent_rate(clk);
 }
 
+ulong clk_generic_set_rate(struct clk *clk, ulong rate)
+{
+   if (clk->flags & CLK_SET_RATE_PARENT)
+   return clk_set_parent_rate(clk, rate);
+
+   return clk_get_parent_rate(clk);
+}
+
 const char *clk_hw_get_name(const struct clk *hw)
 {
assert(hw);
-- 
2.43.0



Re: [PATCH] clk: Propagate clk_set_rate() if CLK_SET_PARENT_RATE present

2024-07-03 Thread Michael Nazzareno Trimarchi
Hi all

Working on the clock now. I have done a different implementation

clk-mux, clk-gate I have added a clk_generic_set_rate that is a stub
that propagate to the parent, I think that should work the same but
wihout this while and even on preparation on more
changes I need. Can be work on your case too?

Michael

On Thu, Jun 13, 2024 at 5:33 PM Tom Rini  wrote:
>
> On Mon, Jun 10, 2024 at 02:43:17PM -0500, Sam Protsenko wrote:
> > On Tue, May 14, 2024 at 3:26 PM Sam Protsenko
> >  wrote:
> > >
> > > On Wed, May 1, 2024 at 4:12 PM Sam Protsenko  
> > > wrote:
> > > >
> > > > On Wed, Apr 10, 2024 at 9:53 PM Sean Anderson  wrote:
> > > > >
> > > > > On 3/7/24 19:04, Sam Protsenko wrote:
> > > > > > Sometimes clocks provided to a consumer might not have .set_rate
> > > > > > operation (like gate or mux clocks), but have CLK_SET_PARENT_RATE 
> > > > > > flag
> > > > > > set. In that case it's usually possible to find a parent up the tree
> > > > > > which is capable of setting the rate (div, pll, etc). Implement a 
> > > > > > simple
> > > > > > lookup procedure for such cases, to traverse the clock tree until
> > > > > > .set_rate capable parent is found, and use that parent to actually
> > > > > > change the rate. The search will stop once the first .set_rate 
> > > > > > capable
> > > > > > clock is found, which is usually enough to handle most cases.
> > > > > >
> > > > > > Signed-off-by: Sam Protsenko 
> > > > > > ---
> > > >
> > > > [snip]
> > > >
> > > > >
> > > > > Can you give an example of where this is needed?
> > > > >
> > > >
> > > > Sure. In my case it's needed for eMMC enablement on E850-96 board.
> > > > eMMC node in the device tree [1] is a gate clock
> > > > (CLK_GOUT_MMC_EMBD_SDCLKIN), so the MMC driver won't be able to change
> > > > its rate. But that clock actually has CLK_SET_RATE_PARENT flag set in
> > > > the clock driver [2]. So the right thing to do in this case (and
> > > > that's basically how it's done in Linux kernel too) is to traverse the
> > > > clock tree upwards, and try to find the parent capable to do .set_rate
> > > > (which is usually a divider clock), and use it to change the clock
> > > > rate. I'm working on exynos_dw_mmc driver [3] right now, making it use
> > > > CCF clocks, but I can't do that before this patch is applied.
> > > >
> > > > Grepping the U-Boot tree I can see the CLK_SET_RATE_PARENT flag is
> > > > also used in various IMX clock drivers and STM32MP13 clock driver. I
> > > > guess without this change those flags will be basically ignored.
> > > >
> > > > Thanks!
> > > >
> > >
> > > Hi Sean,
> > >
> > > Just wanted to check if you think my explanation above is ok and the
> > > patch can be applied? I'm finishing my new patch series for enabling
> > > MMC on E850-96, but this patch has to be applied first.
> > >
> > > Thanks!
> > >
> >
> > Hi Tom,
> >
> > Would it be reasonable to ask you to take care of this patch? It's
> > been ignored for 3 months now, but it's needed for eMMC enablement on
> > E850-96 board.
>
> Any comments at this point Sean?
>
> --
> 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: [PATCH 2/2] clk: imx: add mux ops for i.MX8M composite clk

2024-07-03 Thread Michael Nazzareno Trimarchi
Hi Fabio

On Wed, Jul 3, 2024 at 3:44 PM Fabio Estevam  wrote:
>
> Hi Michael,
>
> On Wed, Jul 3, 2024 at 6:15 AM Michael Trimarchi
>  wrote:
> >
> > Upstream Linux commit f90b68d6c8b0.
> 
>
> > The CORE/BUS root slice has following design, simplied graph:
> > The difference is core not have pre_div block.
> > A composite core/bus clk has 8 inputs for mux to select, saying clk[0-7].
> >
> > It support target(smart) interface and normal interface. Target interface
> > is exported for programmer easy to configure ccm root. Normal interface
> > is also exported, but we not use it in our driver, because it will
> > introduce more complexity compared with target interface.
> >
> > The normal interface simplified as below:
> > SEL_A  GA
> > +--+  +-+
> > |  +->+ +--+
> > CLK[0-7]--->+  |  +-+  |
> >||  |  +v---+++
> >|+--+  |pre_diva+>|  +-+
> >|  ++|mux +--+post_div |
> >|+--+  |pre_divb+--->+|  +-+
> >||  |  +^---+++
> >+--->+  |  +-+  |
> > |  +->+ +--+
> > +--+  +-+
> > SEL_B  GB
> >
> > The mux in the upper pic is not the target interface MUX, target
> > interface MUX is hiding SEL_A and SEL_B. When you choose clk[0-7],
> > you are actually writing SEL_A or SEL_B depends on the internal
> > counter which will also control the internal "mux".
> >
> > The target interface simplified as below which is used by Linux Kernel:
> > CLK[0-7]--->MUX-->Gate-->pre_div-->post_div
> >
> > A requirement of the Target Interface's software is that the
> > target clock source is active, it means when setting SEL_A, the
> > current input clk to SEL_A must be active, same to SEL_B.
> >
> > We touch target interface, but hardware logic actually also need
> > configure normal interface.
> >
> > There will be system hang, when doing the following steps:
> > The initial state:
> > SEL_A/SEL_B are both sourcing from clk0, the internal counter
> > choose SEL_A.
> > 1. switch mux from clk0 to clk1
> >The hardware logic will choose SEL_B and configure SEL_B to clk1.
> >SEL_A no changed.
> > 2. gate off clk0
> >Disable clk0, then the input to SEL_A is off.
> > 3. switch from clk1 to clk2
> >The hardware logic will choose SEL_A and configure SEL_A to clk2,
> >however the current SEL_A input clk0 is off, the system hang.
> >
> > The solution to fix the issue is in step 1, write twice to
> > target interface MUX, it will make SEL_A/SEL_B both sources
> > from clk1, then no need to care about the state of clk0. And
> > finally system performs well.
>
> Shouldn't this also contain the original kernel commit author's Signed-off-by?
>
> > Signed-off-by: Michael Trimarchi 

Fine to me to add. I made a mistake. Let's wait on other reviews

Michael


[PATCH 2/2] clk: imx: add mux ops for i.MX8M composite clk

2024-07-03 Thread Michael Trimarchi
Upstream Linux commit f90b68d6c8b0.

The CORE/BUS root slice has following design, simplied graph:
The difference is core not have pre_div block.
A composite core/bus clk has 8 inputs for mux to select, saying clk[0-7].

It support target(smart) interface and normal interface. Target interface
is exported for programmer easy to configure ccm root. Normal interface
is also exported, but we not use it in our driver, because it will
introduce more complexity compared with target interface.

The normal interface simplified as below:
SEL_A  GA
+--+  +-+
|  +->+ +--+
CLK[0-7]--->+  |  +-+  |
   ||  |  +v---+++
   |+--+  |pre_diva+>|  +-+
   |  ++|mux +--+post_div |
   |+--+  |pre_divb+--->+|  +-+
   ||  |  +^---+++
   +--->+  |  +-+  |
|  +->+ +--+
+--+  +-+
SEL_B  GB

The mux in the upper pic is not the target interface MUX, target
interface MUX is hiding SEL_A and SEL_B. When you choose clk[0-7],
you are actually writing SEL_A or SEL_B depends on the internal
counter which will also control the internal "mux".

The target interface simplified as below which is used by Linux Kernel:
CLK[0-7]--->MUX-->Gate-->pre_div-->post_div

A requirement of the Target Interface's software is that the
target clock source is active, it means when setting SEL_A, the
current input clk to SEL_A must be active, same to SEL_B.

We touch target interface, but hardware logic actually also need
configure normal interface.

There will be system hang, when doing the following steps:
The initial state:
SEL_A/SEL_B are both sourcing from clk0, the internal counter
choose SEL_A.
1. switch mux from clk0 to clk1
   The hardware logic will choose SEL_B and configure SEL_B to clk1.
   SEL_A no changed.
2. gate off clk0
   Disable clk0, then the input to SEL_A is off.
3. switch from clk1 to clk2
   The hardware logic will choose SEL_A and configure SEL_A to clk2,
   however the current SEL_A input clk0 is off, the system hang.

The solution to fix the issue is in step 1, write twice to
target interface MUX, it will make SEL_A/SEL_B both sources
from clk1, then no need to care about the state of clk0. And
finally system performs well.

Signed-off-by: Michael Trimarchi 
---
 drivers/clk/imx/clk-composite-8m.c | 37 +-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/imx/clk-composite-8m.c 
b/drivers/clk/imx/clk-composite-8m.c
index 560d74aac80..2cb7d135000 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -117,6 +117,41 @@ static const struct clk_ops 
imx8m_clk_composite_divider_ops = {
.set_rate = imx8m_clk_composite_divider_set_rate,
 };
 
+static int imx8m_clk_mux_set_parent(struct clk *clk, struct clk *parent)
+{
+   struct clk_mux *mux = to_clk_mux(clk);
+   int index;
+   u32 val;
+   u32 reg;
+
+   index = clk_mux_fetch_parent_index(clk, parent);
+   if (index < 0) {
+   log_err("Could not fetch index\n");
+   return index;
+   }
+
+   val = clk_mux_index_to_val(mux->table, mux->flags, index);
+
+   reg = readl(mux->reg);
+   reg &= ~(mux->mask << mux->shift);
+   val = val << mux->shift;
+   reg |= val;
+
+   /*
+* write twice to make sure non-target interface
+* SEL_A/B point the same clk input.
+*/
+   writel(reg, mux->reg);
+   writel(reg, mux->reg);
+
+   return 0;
+}
+
+const struct clk_ops imx8m_clk_mux_ops = {
+   .get_rate = clk_generic_get_rate,
+   .set_parent = imx8m_clk_mux_set_parent,
+};
+
 struct clk *imx8m_clk_composite_flags(const char *name,
  const char * const *parent_names,
  int num_parents, void __iomem *reg,
@@ -155,7 +190,7 @@ struct clk *imx8m_clk_composite_flags(const char *name,
 
clk = clk_register_composite(NULL, name,
 parent_names, num_parents,
->clk, _mux_ops, >clk,
+>clk, _clk_mux_ops, >clk,
 _clk_composite_divider_ops,
 >clk, _gate_ops, flags);
if (IS_ERR(clk))
-- 
2.43.0



[PATCH 1/2] clk: clk-mux: Make public the clk_fetch_parent_index

2024-07-03 Thread Michael Trimarchi
Make public the clk_fetch_parent_index and rename it. This allow
us to be reused in driver specialization

Signed-off-by: Michael Trimarchi 
---
 drivers/clk/clk-mux.c| 5 ++---
 include/linux/clk-provider.h | 1 +
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/clk-mux.c b/drivers/clk/clk-mux.c
index f410518461e..e3481be95fa 100644
--- a/drivers/clk/clk-mux.c
+++ b/drivers/clk/clk-mux.c
@@ -102,8 +102,7 @@ u8 clk_mux_get_parent(struct clk *clk)
return clk_mux_val_to_index(clk, mux->table, mux->flags, val);
 }
 
-static int clk_fetch_parent_index(struct clk *clk,
- struct clk *parent)
+int clk_mux_fetch_parent_index(struct clk *clk, struct clk *parent)
 {
struct clk_mux *mux = to_clk_mux(clk);
 
@@ -127,7 +126,7 @@ static int clk_mux_set_parent(struct clk *clk, struct clk 
*parent)
u32 val;
u32 reg;
 
-   index = clk_fetch_parent_index(clk, parent);
+   index = clk_mux_fetch_parent_index(clk, parent);
if (index < 0) {
log_err("Could not fetch index\n");
return index;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index b8acacd49ee..59f9c241b84 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -74,6 +74,7 @@ struct clk_mux {
 #define to_clk_mux(_clk) container_of(_clk, struct clk_mux, clk)
 extern const struct clk_ops clk_mux_ops;
 u8 clk_mux_get_parent(struct clk *clk);
+int clk_mux_fetch_parent_index(struct clk *clk, struct clk *parent);
 
 /**
  * clk_mux_index_to_val() - Convert the parent index to the register value
-- 
2.43.0



[PATCH 0/2] Support imx8m composite mux

2024-07-03 Thread Michael Trimarchi
The mux ops for i.MX8M composite clk needs to be specialized.
In order to reduce the code duplication we need to make public some
interface and write a specific function in the mux ops. Those patches
implement the behavior

Michael Trimarchi (2):
  clk: clk-mux: Make public the clk_fetch_parent_index
  clk: imx: add mux ops for i.MX8M composite clk

 drivers/clk/clk-mux.c  |  5 ++--
 drivers/clk/imx/clk-composite-8m.c | 37 +-
 include/linux/clk-provider.h   |  1 +
 3 files changed, 39 insertions(+), 4 deletions(-)

-- 
2.43.0



[PATCH 2/2] clk: imx: Fix wrong flags assignment clk-composite-93

2024-07-02 Thread Michael Trimarchi
The mux flags (u8), div flags (u8), and gate flags (u8)  are not the clk
flags (unsigned long). They have different meanings

Signed-off-by: Michael Trimarchi 
---
 drivers/clk/imx/clk-composite-93.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/clk/imx/clk-composite-93.c 
b/drivers/clk/imx/clk-composite-93.c
index 6d71c0c03ff..34026c5e42f 100644
--- a/drivers/clk/imx/clk-composite-93.c
+++ b/drivers/clk/imx/clk-composite-93.c
@@ -103,7 +103,6 @@ struct clk *imx93_clk_composite_flags(const char *name,
mux->mask = CCM_MUX_MASK;
mux->num_parents = num_parents;
mux->parent_names = parent_names;
-   mux->flags = flags;
 
div = kzalloc(sizeof(*div), GFP_KERNEL);
if (!div)
@@ -120,7 +119,6 @@ struct clk *imx93_clk_composite_flags(const char *name,
 
gate->reg = reg;
gate->bit_idx = CCM_OFF_SHIFT;
-   gate->flags = flags;
 
clk = clk_register_composite(NULL, name,
 parent_names, num_parents,
-- 
2.43.0



[PATCH 1/2] clk: imx: Fix wrong flags assignment clk-composite-8m

2024-07-02 Thread Michael Trimarchi
The mux flags (u8), div flags (u8), and gate flags (u8)  are not the clk
flags (unsigned long). They have different meanings

Signed-off-by: Michael Trimarchi 
---
 drivers/clk/imx/clk-composite-8m.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/clk/imx/clk-composite-8m.c 
b/drivers/clk/imx/clk-composite-8m.c
index 494156751da..560d74aac80 100644
--- a/drivers/clk/imx/clk-composite-8m.c
+++ b/drivers/clk/imx/clk-composite-8m.c
@@ -135,7 +135,6 @@ struct clk *imx8m_clk_composite_flags(const char *name,
mux->shift = PCG_PCS_SHIFT;
mux->mask = PCG_PCS_MASK;
mux->num_parents = num_parents;
-   mux->flags = flags;
mux->parent_names = parent_names;
 
div = kzalloc(sizeof(*div), GFP_KERNEL);
@@ -145,7 +144,7 @@ struct clk *imx8m_clk_composite_flags(const char *name,
div->reg = reg;
div->shift = PCG_PREDIV_SHIFT;
div->width = PCG_PREDIV_WIDTH;
-   div->flags = CLK_DIVIDER_ROUND_CLOSEST | flags;
+   div->flags = CLK_DIVIDER_ROUND_CLOSEST;
 
gate = kzalloc(sizeof(*gate), GFP_KERNEL);
if (!gate)
@@ -153,7 +152,6 @@ struct clk *imx8m_clk_composite_flags(const char *name,
 
gate->reg = reg;
gate->bit_idx = PCG_CGC_SHIFT;
-   gate->flags = flags;
 
clk = clk_register_composite(NULL, name,
 parent_names, num_parents,
-- 
2.43.0



Re: [PATCH v3 19/19] CI: Allow running tests on sjg lab

2024-06-24 Thread Michael Nazzareno Trimarchi
Hi Simon

On Mon, Jun 24, 2024 at 2:52 PM Andrejs Cainikovs
 wrote:
>
> On Sun, Jun 23, 2024 at 02:32:13PM -0600, Simon Glass wrote:
> > Add a way to run tests on a real hardware lab. This is in the very early
> > experimental stages. There are only 23 boards and 3 of those are broken!
> > (bob, ff3399, samus). A fourth fails due to problems with the TPM tests.
> >
> > To try this, assuming you have gitlab access, set SJG_LAB=1, e.g.:
> >
> >git push -o ci.variable="SJG_LAB=1" dm HEAD:try
> >
> > This relies on the two previous series targeted at -next as well as the
> > bugfix series for -master
> >
> > Signed-off-by: Simon Glass 
>
> Reviewed-by: Andrejs Cainikovs 
>

Do you have documentation on how to set it? We would like to do in our
company too

Michael

> > ---
> >
> > Changes in v3:
> > - Split out most patches into two new series and update cover letter
> >
> > Changes in v2:
> > - Avoid running a docker image for skipped lab tests
> >
> >  .gitlab-ci.yml | 153 +
> >  1 file changed, 153 insertions(+)
> >
> > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> > index 165f765a833..75c18a0f2f7 100644
> > --- a/.gitlab-ci.yml
> > +++ b/.gitlab-ci.yml
> > @@ -17,6 +17,7 @@ stages:
> >- testsuites
> >- test.py
> >- world build
> > +  - sjg-lab
> >
> >  .buildman_and_testpy_template: _and_testpy_dfn
> >stage: test.py
> > @@ -482,3 +483,155 @@ coreboot test.py:
> >  TEST_PY_TEST_SPEC: "not sleep"
> >  TEST_PY_ID: "--id qemu"
> ><<: *buildman_and_testpy_dfn
> > +
> > +.lab_template: _dfn
> > +  stage: sjg-lab
> > +  rules:
> > +- if: $SJG_LAB == "1"
> > +  when: always
> > +- when: manual
> > +  tags: [ 'lab' ]
> > +  script:
> > +- if [[ -z "${SJG_LAB}" ]]; then
> > +exit 0;
> > +  fi
> > +# Environment:
> > +#   SRC  - source tree
> > +#   OUT  - output directory for builds
> > +- export SRC="$(pwd)"
> > +- export OUT="${SRC}/build/${BOARD}"
> > +- export PATH=$PATH:~/bin
> > +- export PATH=$PATH:/vid/software/devel/ubtest/u-boot-test-hooks/bin
> > +
> > +# Load it on the device
> > +- ret=0
> > +- echo "role ${ROLE}"
> > +- export strategy="-s uboot -e off"
> > +# export verbose="-v"
> > +- ${SRC}/test/py/test.py --role ${ROLE} --build-dir "${OUT}"
> > +--capture=tee-sys -k "not bootstd"|| ret=$?
> > +- U_BOOT_BOARD_IDENTITY="${ROLE}" u-boot-test-release || true
> > +- if [[ $ret -ne 0 ]]; then
> > +exit $ret;
> > +  fi
> > +  artifacts:
> > +when: always
> > +paths:
> > +  - "build/${BOARD}/test-log.html"
> > +  - "build/${BOARD}/multiplexed_log.css"
> > +expire_in: 1 week
> > +
> > +rpi3:
> > +  variables:
> > +ROLE: rpi3
> > +  <<: *lab_dfn
> > +
> > +opi_pc:
> > +  variables:
> > +ROLE: opi_pc
> > +  <<: *lab_dfn
> > +
> > +pcduino3_nano:
> > +  variables:
> > +ROLE: pcduino3_nano
> > +  <<: *lab_dfn
> > +
> > +samus:
> > +  variables:
> > +ROLE: samus
> > +  <<: *lab_dfn
> > +
> > +link:
> > +  variables:
> > +ROLE: link
> > +  <<: *lab_dfn
> > +
> > +jerry:
> > +  variables:
> > +ROLE: jerry
> > +  <<: *lab_dfn
> > +
> > +minnowmax:
> > +  variables:
> > +ROLE: minnowmax
> > +  <<: *lab_dfn
> > +
> > +opi_pc2:
> > +  variables:
> > +ROLE: opi_pc2
> > +  <<: *lab_dfn
> > +
> > +bpi:
> > +  variables:
> > +ROLE: bpi
> > +  <<: *lab_dfn
> > +
> > +rpi2:
> > +  variables:
> > +ROLE: rpi2
> > +  <<: *lab_dfn
> > +
> > +bob:
> > +  variables:
> > +ROLE: bob
> > +  <<: *lab_dfn
> > +
> > +ff3399:
> > +  variables:
> > +ROLE: ff3399
> > +  <<: *lab_dfn
> > +
> > +coral:
> > +  variables:
> > +ROLE: coral
> > +  <<: *lab_dfn
> > +
> > +rpi3z:
> > +  variables:
> > +ROLE: rpi3z
> > +  <<: *lab_dfn
> > +
> > +bbb:
> > +  variables:
> > +ROLE: bbb
> > +  <<: *lab_dfn
> > +
> > +kevin:
> > +  variables:
> > +ROLE: kevin
> > +  <<: *lab_dfn
> > +
> > +pine64:
> > +  variables:
> > +ROLE: pine64
> > +  <<: *lab_dfn
> > +
> > +c4:
> > +  variables:
> > +ROLE: c4
> > +  <<: *lab_dfn
> > +
> > +rpi4:
> > +  variables:
> > +ROLE: rpi4
> > +  <<: *lab_dfn
> > +
> > +rpi0:
> > +  variables:
> > +ROLE: rpi0
> > +  <<: *lab_dfn
> > +
> > +snow:
> > +  variables:
> > +ROLE: snow
> > +  <<: *lab_dfn
> > +
> > +pcduino3:
> > +  variables:
> > +ROLE: pcduino3
> > +  <<: *lab_dfn
> > +
> > +nyan-big:
> > +  variables:
> > +ROLE: nyan-big
> > +  <<: *lab_dfn
> > --
> > 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 v1 1/3] mtd: rawnand: nand_base: support for 'NAND_IS_BOOT_MEDIUM' flag

2024-06-24 Thread Michael Nazzareno Trimarchi
Hi

Yes I have seen and I will review today and cross-check. Did you lunch
the testing on the other patches series? I would like to merge
at all but we were having some build breakage

Michael

On Mon, Jun 24, 2024 at 7:16 AM Arseniy Krasnov
 wrote:
>
> Hi, sorry, pls ping :)
>
> Thanks
>
> On 02.06.2024 23:08, Arseniy Krasnov wrote:
> > Based on Linux kernel:
> > commit f922bd798bb9 ("mtd: rawnand: add an option to specify NAND chip as a 
> > boot device")
> >
> > Allow to define a NAND chip as a boot device. This can be helpful
> > for the selection of the ECC algorithm and strength in case the boot
> > ROM supports only a subset of controller provided options.
> >
> > Signed-off-by: Arseniy Krasnov 
> > ---
> >  drivers/mtd/nand/raw/nand_base.c | 3 +++
> >  include/linux/mtd/rawnand.h  | 6 ++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/drivers/mtd/nand/raw/nand_base.c 
> > b/drivers/mtd/nand/raw/nand_base.c
> > index c40a0f23d7..ed605b4af5 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -4458,6 +4458,9 @@ static int nand_dt_init(struct mtd_info *mtd, struct 
> > nand_chip *chip, ofnode nod
> >   if (ret == 16)
> >   chip->options |= NAND_BUSWIDTH_16;
> >
> > + if (ofnode_read_bool(node, "nand-is-boot-medium"))
> > + chip->options |= NAND_IS_BOOT_MEDIUM;
> > +
> >   if (ofnode_read_bool(node, "nand-on-flash-bbt"))
> >   chip->bbt_options |= NAND_BBT_USE_FLASH;
> >
> > diff --git a/include/linux/mtd/rawnand.h b/include/linux/mtd/rawnand.h
> > index fb002ae641..4eb880d8fb 100644
> > --- a/include/linux/mtd/rawnand.h
> > +++ b/include/linux/mtd/rawnand.h
> > @@ -218,6 +218,12 @@ enum nand_ecc_algo {
> >  /* Device needs 3rd row address cycle */
> >  #define NAND_ROW_ADDR_3  0x4000
> >
> > +/*
> > + * Whether the NAND chip is a boot medium. Drivers might use this 
> > information
> > + * to select ECC algorithms supported by the boot ROM or similar 
> > restrictions.
> > + */
> > +#define NAND_IS_BOOT_MEDIUM  0x0040
> > +
> >  /* Options valid for Samsung large page devices */
> >  #define NAND_SAMSUNG_LP_OPTIONS NAND_CACHEPRG
> >



-- 
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/3] pxe: Add debugging for booting

2024-06-22 Thread Michael Nazzareno Trimarchi
Hi

On Wed, Jun 19, 2024 at 2:34 PM Simon Glass  wrote:
>
> Show which boot protocol is being used.
>
> Signed-off-by: Simon Glass 
> ---
>
>  boot/pxe_utils.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
> index 4b22bb6f525..53ddb16be73 100644
> --- a/boot/pxe_utils.c
> +++ b/boot/pxe_utils.c
> @@ -4,6 +4,8 @@
>   * Copyright (c) 2014, NVIDIA CORPORATION.  All rights reserved.
>   */
>
> +#define LOG_CATEGORY   LOGC_BOOT
> +
>  #include 
>  #include 
>  #include 
> @@ -762,17 +764,22 @@ static int label_boot(struct pxe_context *ctx, struct 
> pxe_label *label)
>
> /* Try bootm for legacy and FIT format image */
> if (genimg_get_format(buf) != IMAGE_FORMAT_INVALID &&
> -IS_ENABLED(CONFIG_CMD_BOOTM))
> +   IS_ENABLED(CONFIG_CMD_BOOTM)) {
> +   log_debug("using bootm\n");
> do_bootm(ctx->cmdtp, 0, bootm_argc, bootm_argv);
> /* Try booting an AArch64 Linux kernel image */
> -   else if (IS_ENABLED(CONFIG_CMD_BOOTI))
> +   } else if (IS_ENABLED(CONFIG_CMD_BOOTI)) {
> +   log_debug("using booti\n");
> do_booti(ctx->cmdtp, 0, bootm_argc, bootm_argv);
> /* Try booting a Image */
> -   else if (IS_ENABLED(CONFIG_CMD_BOOTZ))
> +   } else if (IS_ENABLED(CONFIG_CMD_BOOTZ)) {
> +   log_debug("using bootz\n");
> do_bootz(ctx->cmdtp, 0, bootm_argc, bootm_argv);
> /* Try booting an x86_64 Linux kernel image */
> -   else if (IS_ENABLED(CONFIG_CMD_ZBOOT))
> +   } else if (IS_ENABLED(CONFIG_CMD_ZBOOT)) {
> +   log_debug("using zboot\n");
> do_zboot_parent(ctx->cmdtp, 0, zboot_argc, zboot_argv, NULL);
> +   }
>
> unmap_sysmem(buf);
>
> --
> 2.34.1
>

Reviewed-by: Michael Trimarchi 


Re: [PATCH] dt-bindings: imx: Drop redundant imports with dts/upstream

2024-06-20 Thread Michael Nazzareno Trimarchi
Hi Sumit

On Thu, Jun 20, 2024 at 6:55 AM Sumit Garg  wrote:
>
> On Wed, 19 Jun 2024 at 18:51, Michael Nazzareno Trimarchi
>  wrote:
> >
> > Hi
> >
> > On Wed, Jun 19, 2024 at 3:07 PM Sumit Garg  wrote:
> > >
> > > On Wed, 19 Jun 2024 at 18:26, Adam Ford  wrote:
> > > >
> > > > On Wed, Jun 19, 2024 at 7:53 AM Sumit Garg  
> > > > wrote:
> > > > >
> > > > > Drop redundant header imports with dts/upstream already providing
> > > > > updated headers which have been checked to be backwards compatibility.
> > > > >
> > > > > The imx headers which aren't present in dts/upstream are as follows:
> > > > >
> > > > > - include/dt-bindings/clock/imxrt1020-clock.h
> > > > > - include/dt-bindings/clock/imx8qm-clock.h
> > > > > - include/dt-bindings/clock/imxrt1170-clock.h
> > > > > - include/dt-bindings/clock/imx8qxp-clock.h
> > > > > - include/dt-bindings/memory/imxrt-sdram.h
> > > > > - include/dt-bindings/pinctrl/pads-imx8qxp.h
> > > > > - include/dt-bindings/pinctrl/pads-imx8qm.h
> > > > > - include/dt-bindings/soc/imx8_pd.h
> > > > > - include/dt-bindings/soc/imx_rsrc.h
> > > > >
> > > > > hence these aren't dropped yet but there was an unused header:
> > > > >
> > > > > - include/dt-bindings/pinctrl/pins-imx8mq.h
> > > > >
> > > > > which has been dropped as well. There shouldn't be any funtional 
> > > > > impact
> > > > > with this change but it rather allows iMX platforms to use upstream
> > > > > dt-bindings headers in a backwards compatible manner.
> > > >
> > > > Will this have any impact on board with -u-boot.dtsi files that might
> > > > reference these files or will they point to the upstream path?
> > > >
> > >
> > > No it won't have any impact on existing boards, the headers would
> > > rather just be included from dts/upstream/include/dt-bindings/
> > > instead. The DT files would just be including headers the same way as
> > > before: #include . The world build is ongoing
> > > here [1] which should be able to justify it.
> > >
> > > [1] https://github.com/u-boot/u-boot/pull/587
> > >
> >
> > They will point to the upstream part but the fact it's that you can
> > test only that they compile ;).
>
> Majority of the headers removed have been compared to be identical and
> the rest headers have been checked to be DT ABI compatible with
> upstream counterparts. IMHO, that should be enough to believe the
> correctness of this patch. If you would like to manually review them
> or give this patch a try on a real iMX device then please feel free to
> do that.
>
> -Sumit

I would like to move this on. I prefer two step approach and fix
broken things and then maybe drop
legacy

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] Makefile: Fix include directory for OF_UPSTREAM

2024-06-17 Thread Michael Nazzareno Trimarchi
HI Sumit

On Mon, Jun 17, 2024 at 10:22 AM Sumit Garg  wrote:
>
> Hi Michael,
>
> Sorry for the delayed response as I had some health issues last week.
>

We are not in a hurry and I think discussion has his own time. Glad that you are
getting better

> On Thu, 13 Jun 2024 at 12:44, Michael Nazzareno Trimarchi
>  wrote:
> >
> > Hi Sumit
> >
> > On Thu, Jun 13, 2024 at 9:02 AM Sumit Garg  wrote:
> > >
> > > On Mon, 3 Jun 2024 at 20:38, Patrick Barsanti
> > >  wrote:
> > > >
> > > > Always prioritizing u-boot includes causes problems when trying to 
> > > > migrate
> > > > boards to OF_UPSTREAM that have different local devicetree files with
> > > > respect to the upstream ones, if local DT headers are not dropped.
> > > > At the same time if local and upstream files are the same, migrations
> > > > can be, and have been, introduced without dropping local DT headers.
> > > > This also causes problems whenever upstream DTS and DT headers are 
> > > > patched.
> > > >
> > > > For example, now migrating a board based on `imx6ul.dtsi` to OF_UPSTREAM
> > > > breaks it, as there are some missing defines in a local DT header file
> > > > (`imx6ul-clock.h`); the solution would be to drop it, which has not 
> > > > always
> > > > been done in previous OF_UPSTREAM migration patches for other boards
> > > > because most of the time the two are the same. This solution is also
> > > > vulnerable to ABI breakage, although this has not yet happened since the
> > > > introduction of OF_UPSTREAM support and is unlikely.
> > > >
> > > > Maintainers assure backwards compatibility for DT headers when syncing 
> > > > the
> > > > upstream folder with the kernel.
> > > > The problem is that, at the current state, all boards that have not 
> > > > dropped
> > > > their local headers when migrating to OF_UPSTREAM will break once the
> > > > upstream devicetrees are patched, for example, to use any newly added
> > > > define which is not present in the local DT header file, even if those
> > > > changes are backwards compatible.
> > > >
> > > > This patch fixes these problems by prioritizing upstream includes when
> > > > `CONFIG_OF_UPSTREAM=y`, while keeping current prioritization when it is
> > > > not.
> > > >
> > > > Also in case of ABI breakage in the kernel, keeping redundant header 
> > > > files
> > > > (only until strictly necessary, e.g. until all boards including them 
> > > > have
> > > > been migrated to OF_UPSTREAM) together with this selective 
> > > > prioritization
> > > > of the include folder is a solution for keeping not-yet-migrated boards
> > > > from breaking.
> > >
> > > Let's just not try to make assumptions about DT ABI breakage due to
> > > using upstream headers for not-yet-migrated boards but rather talk
> > > about some real world examples. Have you come across any DT ABI
> > > breakage with usage of upstream headers? The breakage you have
> > > reported is due to usage of an old local copy of DT header.
> > >
> >
> > The include priority is broken, and this a Makefile problem anyway.
> > This patch fix
> > anyway includes priorities.
>
> The include priority only becomes an issue if you want to keep
> redundant local copies of DT headers. And I have been advocating to
> drop local copies from U-Boot as much as possible. Once we only have
> upstream DT headers being used (Qcom platforms can be a good example
> here), we don't need to care about trickier priorities.

Yes but this is the case. All we build now can include the local
redundant copy and we can
not guarantee that already board builds are built using the latest
version of the include.

>
> > The imx6 board migration for us does not work
>
> Patrick did mention here [1] that dropping
> include/dt-bindings/clock/imx6ul-clock.h fixes the problem...
>

Yes but suppose that the include was ok in the sense of compilation
but not consistent
in term of functionality. When you build using upstream headers you
must use them
to be consistent and it's not as it is for now. Anyway we can not drop
for one board upstream
that file, unless we force everyone to have broken board or all upstream now

> >
> > > However, if this patch is only needed to address fear of DT ABI
> > > breakage (which hasn't happened in the past) then I can live with it
> > > such that it can help convince more people for OF_UPSTREAM migration.
> > > We can drop local DT headers as and when people feel comfortable with
> > > upstream.
> >
> > It's not abi breakage only but path inclusion order.
>
> ...however it's the ABI breakage part that worries Patrick. So if this
> patch only solves that worry then I can live with it in the hope that
> people will start to gain confidence in DT ABI stability.
>
> [1] 
> https://lore.kernel.org/u-boot/cabxfhnfwdd0njpnskaxzxdtg9pdodx5vbhrn9chvmh5xo_7...@mail.gmail.com/
>
> -Sumit

Michael


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

2024-06-13 Thread Michael Nazzareno Trimarchi
Hi Sumit

On Thu, Jun 13, 2024 at 9:02 AM Sumit Garg  wrote:
>
> On Mon, 3 Jun 2024 at 20:38, Patrick Barsanti
>  wrote:
> >
> > Always prioritizing u-boot includes causes problems when trying to migrate
> > boards to OF_UPSTREAM that have different local devicetree files with
> > respect to the upstream ones, if local DT headers are not dropped.
> > At the same time if local and upstream files are the same, migrations
> > can be, and have been, introduced without dropping local DT headers.
> > This also causes problems whenever upstream DTS and DT headers are patched.
> >
> > For example, now migrating a board based on `imx6ul.dtsi` to OF_UPSTREAM
> > breaks it, as there are some missing defines in a local DT header file
> > (`imx6ul-clock.h`); the solution would be to drop it, which has not always
> > been done in previous OF_UPSTREAM migration patches for other boards
> > because most of the time the two are the same. This solution is also
> > vulnerable to ABI breakage, although this has not yet happened since the
> > introduction of OF_UPSTREAM support and is unlikely.
> >
> > Maintainers assure backwards compatibility for DT headers when syncing the
> > upstream folder with the kernel.
> > The problem is that, at the current state, all boards that have not dropped
> > their local headers when migrating to OF_UPSTREAM will break once the
> > upstream devicetrees are patched, for example, to use any newly added
> > define which is not present in the local DT header file, even if those
> > changes are backwards compatible.
> >
> > This patch fixes these problems by prioritizing upstream includes when
> > `CONFIG_OF_UPSTREAM=y`, while keeping current prioritization when it is
> > not.
> >
> > Also in case of ABI breakage in the kernel, keeping redundant header files
> > (only until strictly necessary, e.g. until all boards including them have
> > been migrated to OF_UPSTREAM) together with this selective prioritization
> > of the include folder is a solution for keeping not-yet-migrated boards
> > from breaking.
>
> Let's just not try to make assumptions about DT ABI breakage due to
> using upstream headers for not-yet-migrated boards but rather talk
> about some real world examples. Have you come across any DT ABI
> breakage with usage of upstream headers? The breakage you have
> reported is due to usage of an old local copy of DT header.
>

The include priority is broken, and this a Makefile problem anyway.
This patch fix
anyway includes priorities. The imx6 board migration for us does not work

> However, if this patch is only needed to address fear of DT ABI
> breakage (which hasn't happened in the past) then I can live with it
> such that it can help convince more people for OF_UPSTREAM migration.
> We can drop local DT headers as and when people feel comfortable with
> upstream.

It's not abi breakage only but path inclusion order.

Michael


>
> -Sumit
>
> >
> > Link: 
> > https://lore.kernel.org/u-boot/20240528084224.113385-1-patrick.barsa...@amarulasolutions.com/
> > Signed-off-by: Patrick Barsanti 
> > Tested-by: Michael Trimarchi 
> > ---
> >
> > Changes in v2:
> > - Reword and correct the commit message following the discussion
> > with Sumit, per Michael's suggestion.
> >
> >  Makefile | 14 ++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 79b28c2d81..899ae664ca 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -826,6 +826,19 @@ KBUILD_HOSTCFLAGS += $(if $(CONFIG_TOOLS_DEBUG),-g)
> >
> >  # Use UBOOTINCLUDE when you must reference the include/ directory.
> >  # Needed to be compatible with the O= option
> > +ifeq ($(CONFIG_OF_UPSTREAM),y)
> > +UBOOTINCLUDE:= \
> > +   -I$(srctree)/dts/upstream/include \
> > +   -Iinclude \
> > +   $(if $(KBUILD_SRC), -I$(srctree)/include) \
> > +   $(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \
> > +   $(if $(CONFIG_HAS_THUMB2), \
> > +   $(if $(CONFIG_CPU_V7M), \
> > +   -I$(srctree)/arch/arm/thumb1/include), \
> > +   -I$(srctree)/arch/arm/thumb1/include)) \
> > +   -I$(srctree)/arch/$(ARCH)/include \
> > +   -include $(srctree)/include/linux/kconfig.h
> > +else
> >  UBOOTINCLUDE:= \
> > -Iinclude \
> > $(if $(KBUILD_SRC), -I$(srctree)/include) \
> > @@ -837,6 +850,7 @@ UBOOTINCLUDE:= \
> > -I$(srctree)/arch/$(ARCH)/include \
> > -include $(srctree)/include/linux/kconfig.h \
> > -I$(srctree)/dts/upstream/include
> > +endif
> >
> >  NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) 
> > -print-file-name=include)
> >
> > --
> > 2.45.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 v2] Makefile: Fix include directory for OF_UPSTREAM

2024-06-12 Thread Michael Nazzareno Trimarchi
HI

On Wed, Jun 12, 2024 at 7:25 PM Tom Rini  wrote:
>
> On Wed, Jun 12, 2024 at 10:17:12AM +0200, Michael Nazzareno Trimarchi wrote:
> > Hi Tom
> >
> > On Mon, Jun 3, 2024 at 5:08 PM Patrick Barsanti
> >  wrote:
> > >
> > > Always prioritizing u-boot includes causes problems when trying to migrate
> > > boards to OF_UPSTREAM that have different local devicetree files with
> > > respect to the upstream ones, if local DT headers are not dropped.
> > > At the same time if local and upstream files are the same, migrations
> > > can be, and have been, introduced without dropping local DT headers.
> > > This also causes problems whenever upstream DTS and DT headers are 
> > > patched.
> > >
> > > For example, now migrating a board based on `imx6ul.dtsi` to OF_UPSTREAM
> > > breaks it, as there are some missing defines in a local DT header file
> > > (`imx6ul-clock.h`); the solution would be to drop it, which has not always
> > > been done in previous OF_UPSTREAM migration patches for other boards
> > > because most of the time the two are the same. This solution is also
> > > vulnerable to ABI breakage, although this has not yet happened since the
> > > introduction of OF_UPSTREAM support and is unlikely.
> > >
> > > Maintainers assure backwards compatibility for DT headers when syncing the
> > > upstream folder with the kernel.
> > > The problem is that, at the current state, all boards that have not 
> > > dropped
> > > their local headers when migrating to OF_UPSTREAM will break once the
> > > upstream devicetrees are patched, for example, to use any newly added
> > > define which is not present in the local DT header file, even if those
> > > changes are backwards compatible.
> > >
> > > This patch fixes these problems by prioritizing upstream includes when
> > > `CONFIG_OF_UPSTREAM=y`, while keeping current prioritization when it is
> > > not.
> >
> > Do you have some comments here? We would like to move upstream dts for
> > imx6 boards
> > too
>
> I've been waiting for the people that wanted the commit message
> clarified to Ack/Review the patch. Did I just miss that? Sorry.
>

Agreed, let's wait for more feedback

Michael

> --
> Tom


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

2024-06-12 Thread Michael Nazzareno Trimarchi
Hi Marek

On Wed, Jun 12, 2024 at 11:32 AM Marek Vasut  wrote:
>
> On 6/3/24 5:07 PM, Patrick Barsanti wrote:
> > Always prioritizing u-boot includes causes problems when trying to migrate
> > boards to OF_UPSTREAM that have different local devicetree files with
> > respect to the upstream ones, if local DT headers are not dropped.
> > At the same time if local and upstream files are the same, migrations
> > can be, and have been, introduced without dropping local DT headers.
> > This also causes problems whenever upstream DTS and DT headers are patched.
> >
> > For example, now migrating a board based on `imx6ul.dtsi` to OF_UPSTREAM
> > breaks it, as there are some missing defines in a local DT header file
> > (`imx6ul-clock.h`); the solution would be to drop it, which has not always
> > been done in previous OF_UPSTREAM migration patches for other boards
> > because most of the time the two are the same. This solution is also
> > vulnerable to ABI breakage, although this has not yet happened since the
> > introduction of OF_UPSTREAM support and is unlikely.
> >
> > Maintainers assure backwards compatibility for DT headers when syncing the
> > upstream folder with the kernel.
>
> ... upstream directory ...
>
> > The problem is that, at the current state, all boards that have not dropped
> > their local headers when migrating to OF_UPSTREAM will break once the
> > upstream devicetrees are patched, for example, to use any newly added
> > define which is not present in the local DT header file, even if those
> > changes are backwards compatible.
>
> Why not simply remove the headers ?
>

If we remove the headers then other boards that are not migrated, can
have some problem
on building. We should consider not only new boards but even those
that ones are still not migrated.
I think that is already described here

> Add a WARNING if there are local duplicates .
>

Why should a WARNING help here? We would like to have some migration
plans but not break them.

Michael


> [...]


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

2024-06-12 Thread Michael Nazzareno Trimarchi
Hi Tom

On Mon, Jun 3, 2024 at 5:08 PM Patrick Barsanti
 wrote:
>
> Always prioritizing u-boot includes causes problems when trying to migrate
> boards to OF_UPSTREAM that have different local devicetree files with
> respect to the upstream ones, if local DT headers are not dropped.
> At the same time if local and upstream files are the same, migrations
> can be, and have been, introduced without dropping local DT headers.
> This also causes problems whenever upstream DTS and DT headers are patched.
>
> For example, now migrating a board based on `imx6ul.dtsi` to OF_UPSTREAM
> breaks it, as there are some missing defines in a local DT header file
> (`imx6ul-clock.h`); the solution would be to drop it, which has not always
> been done in previous OF_UPSTREAM migration patches for other boards
> because most of the time the two are the same. This solution is also
> vulnerable to ABI breakage, although this has not yet happened since the
> introduction of OF_UPSTREAM support and is unlikely.
>
> Maintainers assure backwards compatibility for DT headers when syncing the
> upstream folder with the kernel.
> The problem is that, at the current state, all boards that have not dropped
> their local headers when migrating to OF_UPSTREAM will break once the
> upstream devicetrees are patched, for example, to use any newly added
> define which is not present in the local DT header file, even if those
> changes are backwards compatible.
>
> This patch fixes these problems by prioritizing upstream includes when
> `CONFIG_OF_UPSTREAM=y`, while keeping current prioritization when it is
> not.

Do you have some comments here? We would like to move upstream dts for
imx6 boards
too

Michael

>
> Also in case of ABI breakage in the kernel, keeping redundant header files
> (only until strictly necessary, e.g. until all boards including them have
> been migrated to OF_UPSTREAM) together with this selective prioritization
> of the include folder is a solution for keeping not-yet-migrated boards
> from breaking.
>
> Link: 
> https://lore.kernel.org/u-boot/20240528084224.113385-1-patrick.barsa...@amarulasolutions.com/
> Signed-off-by: Patrick Barsanti 
> Tested-by: Michael Trimarchi 
> ---
>
> Changes in v2:
> - Reword and correct the commit message following the discussion
> with Sumit, per Michael's suggestion.
>
>  Makefile | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 79b28c2d81..899ae664ca 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -826,6 +826,19 @@ KBUILD_HOSTCFLAGS += $(if $(CONFIG_TOOLS_DEBUG),-g)
>
>  # Use UBOOTINCLUDE when you must reference the include/ directory.
>  # Needed to be compatible with the O= option
> +ifeq ($(CONFIG_OF_UPSTREAM),y)
> +UBOOTINCLUDE:= \
> +   -I$(srctree)/dts/upstream/include \
> +   -Iinclude \
> +   $(if $(KBUILD_SRC), -I$(srctree)/include) \
> +   $(if $(CONFIG_$(SPL_)SYS_THUMB_BUILD), \
> +   $(if $(CONFIG_HAS_THUMB2), \
> +   $(if $(CONFIG_CPU_V7M), \
> +   -I$(srctree)/arch/arm/thumb1/include), \
> +   -I$(srctree)/arch/arm/thumb1/include)) \
> +   -I$(srctree)/arch/$(ARCH)/include \
> +   -include $(srctree)/include/linux/kconfig.h
> +else
>  UBOOTINCLUDE:= \
> -Iinclude \
> $(if $(KBUILD_SRC), -I$(srctree)/include) \
> @@ -837,6 +850,7 @@ UBOOTINCLUDE:= \
> -I$(srctree)/arch/$(ARCH)/include \
> -include $(srctree)/include/linux/kconfig.h \
> -I$(srctree)/dts/upstream/include
> +endif
>
>  NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
>
> --
> 2.45.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


[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

  1   2   3   4   5   6   7   8   9   10   >