Re: [PATCH v2 5/6] mmc: actions: add MMC driver for Actions OWL S700

[Amit Tomer]

On Wed, Dec 23, 2020 at 5:57 AM André Przywara  wrote:
>
> On 19/12/2020 14:51, Amit Singh Tomar wrote:
> > From: Amit Singh Tomar 
> >
> > This commit adds support for MMC controllers found on Actions OWL
> > S700 SoC platform.
> >
> > Signed-off-by: Amit Singh Tomar 
> > ---
> > Changes since previous version
> >   * Corrected block count to 512.
> >   * Changed the command timeout value to 30ms.
> >   * Used readl_poll_timeout.
> >   * Read DMA parameters from DT instead of hardcoding it.
> >   * Reduced number of arguments passed to own_dma_cofig.
> >   * Removed debug leftover.
> >   * Used mmc_of_parse().
> > ---
> >  drivers/mmc/Kconfig   |   7 +
> >  drivers/mmc/Makefile  |   1 +
> >  drivers/mmc/owl_mmc.c | 399 
> > ++
> >  3 files changed, 407 insertions(+)
> >  create mode 100644 drivers/mmc/owl_mmc.c
> >
> > diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
> > index 14d7913..61f9c67 100644
> > --- a/drivers/mmc/Kconfig
> > +++ b/drivers/mmc/Kconfig
> > @@ -289,6 +289,13 @@ config MMC_MXC
> >
> > If unsure, say N.
> >
> > +config MMC_OWL
> > + bool "Actions OWL Multimedia Card Interface support"
> > + depends on ARCH_OWL && DM_MMC && BLK
> > + help
> > +   This selects the OWL SD/MMC host controller found on board
> > +   based on Actions S700 SoC.
>
> And S900 as well?
>
> > +
> >  config MMC_MXS
> >   bool "Freescale MXS Multimedia Card Interface support"
> >   depends on MX23 || MX28 || MX6 || MX7
> > diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> > index 1c849cb..f270f6c 100644
> > --- a/drivers/mmc/Makefile
> > +++ b/drivers/mmc/Makefile
> > @@ -38,6 +38,7 @@ obj-$(CONFIG_MMC_OMAP_HS)   += omap_hsmmc.o
> >  obj-$(CONFIG_MMC_MXC)+= mxcmmc.o
> >  obj-$(CONFIG_MMC_MXS)+= mxsmmc.o
> >  obj-$(CONFIG_MMC_OCTEONTX)   += octeontx_hsmmc.o
> > +obj-$(CONFIG_MMC_OWL)+= owl_mmc.o
> >  obj-$(CONFIG_MMC_PCI)+= pci_mmc.o
> >  obj-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
> >  obj-$(CONFIG_$(SPL_TPL_)SUPPORT_EMMC_RPMB) += rpmb.o
> > diff --git a/drivers/mmc/owl_mmc.c b/drivers/mmc/owl_mmc.c
> > new file mode 100644
> > index 000..5c48307
> > --- /dev/null
> > +++ b/drivers/mmc/owl_mmc.c
> > @@ -0,0 +1,399 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2020 Amit Singh Tomar 
> > + *
> > + * Driver for SD/MMC controller present on Actions Semi S700 SoC, based
> > + * on Linux Driver "drivers/mmc/host/owl-mmc.c".
> > + *
> > + * Though, there is a bit (BSEL, BUS or DMA Special Channel Selection) that
> > + * controls the data transfer from SDx_DAT register either using CPU AHB 
> > Bus
> > + * or DMA channel, but seems like, it only works correctly using external 
> > DMA
> > + * channel, and those special bits used in this driver is picked from 
> > vendor
> > + * source exclusively for MMC/SD.
> > + */
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/*
> > + * SDC registers
> > + */
> > +#define OWL_REG_SD_EN   0x
> > +#define OWL_REG_SD_CTL  0x0004
> > +#define OWL_REG_SD_STATE0x0008
> > +#define OWL_REG_SD_CMD  0x000c
> > +#define OWL_REG_SD_ARG  0x0010
> > +#define OWL_REG_SD_RSPBUF0  0x0014
> > +#define OWL_REG_SD_RSPBUF1  0x0018
> > +#define OWL_REG_SD_RSPBUF2  0x001c
> > +#define OWL_REG_SD_RSPBUF3  0x0020
> > +#define OWL_REG_SD_RSPBUF4  0x0024
> > +#define OWL_REG_SD_DAT  0x0028
> > +#define OWL_REG_SD_BLK_SIZE 0x002c
> > +#define OWL_REG_SD_BLK_NUM  0x0030
> > +#define OWL_REG_SD_BUF_SIZE 0x0034
> > +
> > +/* SD_EN Bits */
> > +#define OWL_SD_EN_RANE  BIT(31)
> > +#define OWL_SD_EN_RESE  BIT(10)
> > +#define OWL_SD_ENABLE   BIT(7)
> > +#define OWL_SD_EN_BSEL  BIT(6)
> > +#define OWL_SD_EN_DATAWID(x)(((x) & 0x3) << 0)
> > +#define OWL_SD_EN_DATAWID_MASK   0x03
> > +
> > +/* SD_CTL Bits */
> > +#define OWL_SD_CTL_TOUTEN   BIT(31)
> > +#define OWL_SD_CTL_DELAY_MSKGENMASK(23, 16)
> > +#define OWL_SD_CTL_RDELAY(x)(((x) & 0xf) << 20)
> > +#define OWL_SD_CTL_WDELAY(x)(((x) & 0xf) << 16)
> > +#define OWL_SD_CTL_TS   BIT(7)
> > +#define OWL_SD_CTL_LBE  BIT(6)
> > +#define OWL_SD_CTL_TM(x)(((x) & 0xf) << 0)
> > +
> > +#define OWL_SD_DELAY_LOW_CLK0x0f
> > +#define OWL_SD_DELAY_MID_CLK0x0a
> > +#define OWL_SD_RDELAY_HIGH   0x08
> > +#define OWL_SD_WDELAY_HIGH   0x09
>
> w/s? Here and 

Re: [PATCH 1/6] clk: actions: Introduce dummy get/set_rate callbacks

[Amit Tomer]

Hi,

On Mon, Dec 14, 2020 at 6:45 AM André Przywara  wrote:
>
> On 13/12/2020 09:43, Amit Singh Tomar wrote:
> > This commit introduces get/set_rate callbacks, these are dummy at
> > the moment, and can be used to get/set clock for various devices
> > based on the clk id.
> >
> > Signed-off-by: Amit Singh Tomar 
> > ---
> >  drivers/clk/owl/clk_owl.c | 28 
> >  1 file changed, 28 insertions(+)
> >
> > diff --git a/drivers/clk/owl/clk_owl.c b/drivers/clk/owl/clk_owl.c
> > index 1999c87..c9bc5c2 100644
> > --- a/drivers/clk/owl/clk_owl.c
> > +++ b/drivers/clk/owl/clk_owl.c
> > @@ -128,6 +128,32 @@ int owl_clk_disable(struct clk *clk)
> >   return 0;
> >  }
> >
> > +static ulong owl_clk_get_rate(struct clk *clk)
> > +{
> > + struct owl_clk_priv *priv = dev_get_priv(clk->dev);
>
> This is a bit premature and leads to compilation warnings.

Sorry, I should have compile-tested the individual commit as well.
I Would fix it in the next version.

Thanks
-Amit

Re: [PATCH 2/6] clk: actions: Add SD/MMC clocks

[Amit Tomer]

Hi,

Thanks for having the detailed look and providing comments:

> According to the datasheet the clock source could also be NAND_PLL,
> depending on bit 9.
> Both PLLs use the same rate calculation, so it's just matter of the PLL
> address offset to use for covering both.

Ok, should I change the comment as Clock output of DEV/NAND_PLL ?

> > +
> > + /* Clock output of DEV_PLL
> > +  * Range: 48M ~ 756M
> > +  * Frequency= DEVPLLCLK * 6
> > +  */
> > + parent_rate = readl(priv->base + CMU_DEVPLL) & 0x7f;
> > + parent_rate *= 600;
> > +
> > + rate *= 2;
>
> Where does this come from?

While experimenting about it, I found that doubling input rate doubles the
speed , and BSP code does the same.

Not sure if it is a good enough explanation or maybe its side effect of not
programming divider value well when its >=128. Would check it again.

> > + div = (parent_rate / rate) - 1;
>
> Please check rate being not zero before you dividing by it. And I would
> keep the real divider value for now, so drop the -1 here.

Ok. I would make the check.

> > +
> > + if (div >= 128)
> > + div |= 0x100;
>
> This does not seem right. You would need to divide the value by 128,
> once you decided to use the "divide-by-128" bit (which is bit 8).
> What about moving this below, after you know the register value?
> Then you can divide by 128 and set bit 8.

Sure, Thanks for explaining it in detail. It helped understanding
ACTIONS not so usual
clock design.

> > + val = readl(priv->base + (CMU_SD0CLK + sd_index*0x4));
> > + /* Bits 4..0 is used to program div value */
> > + val &= ~0x1f;
> > + val |= div;
>
> Please mask div before applying.

But I am masking the first 4.0 bit above , ~0x1f;

> > + /* As per Manuals Bits 31..10 are reserved but Bits 11 and
> > +  * 10 needed to be set for proper operation
> > +  */
> > + val |= 0xc00;
>
> Where does this come from? I don't see the Linux driver doing that?

Actually if I don't set 11th and 10th bit , I see SD speed of 2KiB/s
and unstable behaviour
while loading files from the card.
BSP U-boot driver chooses a value 0xfce0 for it.


Thanks
Amit

Re: [PATCH 00/15] net: sun8i-emac fixes and cleanups

[Amit Tomer]

Hi,

On Mon, Jul 6, 2020 at 6:12 AM Andre Przywara  wrote:
>
> Hi,
>
> while looking at several U-Boot network drivers in the past year, I
> typically compared them to the sun8i-emac driver, as a kind of personal
> reference. While doing so, I figured that there are quite some things
> broken in here, and other things are not so nice.
> This series attempts the fix those shortcomings.
> Fix-wise we get proper handling of PHY failures (try without a
> cable connected), support for external RMII PHYs (as seen on the
> Pine64-non-plus model), and more future-proof internal PHY handling.
> The rest of the patches are cleanups, which fix things that are wrong,
> but we get away with so far, for one or another reason.
> This also cleans up a good part of the cache maintenance. There is more
> to be done (and I have patches for that), but that requires to drop
> the overzealous alignment checks in cache_v7.c first, which is part of
> another, upcoming series.
>
> A git repo with those patches can be found here:
> https://github.com/apritzel/u-boot/commits/sun8i-emac-cleanup

Tested this on Pine64+, and it worked without any issue.

Tested-by: Amit Singh Tomar 

Thanks
-Amit

Re: [PATCH v4 0/2] Calculate SDRAM size for Actions OWL SoCs

[Amit Tomer]

Hi Tom

On Sat, May 9, 2020 at 1:45 PM Amit Singh Tomar  wrote:
>
> Mani pointed out that changes in previous version were not good enough
> for S900 and he provide snippet that seems to work on S900.
>
> This series v4 fixes the S900 Support by taking in changes suggested
> by Mani.
>
> -
>
> Realized that sent the wrong version(v2) that has typos and didn't get
> compile for S900. Just fixed this in v3.
>
> This small series allows us to calculate SDRAM size instead
> of guessing it.
>
> Patch (1/2) is re-worked to support S900 SoC along with S700.
>
> These changes have been tested on S700 based Cubieboard7-lite board, and
> it would be great if this can be tested on S900.
>
> Amit Singh Tomar (2):
>   Actions: OWL: Calculate SDRAM size
>   arm: actions: remove "CONFIG_SYS_SDRAM_SIZE" for Actions Owl Semi SoCs
>
>  arch/arm/mach-owl/soc.c  | 22 +-
>  include/configs/owl-common.h |  1 -
>  2 files changed, 21 insertions(+), 2 deletions(-)
>
> --
> 2.7.4
>

Do you have a plan to take these in ?

Thanks
-Amit.

Re: [PATCHv2 8/8] cubieboard7: Remove ARCH= references from documentation

[Amit Tomer]

Hi,

On Wed, May 27, 2020 at 12:07 AM Tom Rini  wrote:
>
> When building U-Boot we select the architecture via Kconfig and not ARCH
> being passed in via the environment or make cmdline.
>
> While in here, add the doc file to the MAINTAINERS entry.
>
> Cc: Amit Singh Tomar 
> Cc: Manivannan Sadhasivam 
> Signed-off-by: Tom Rini 
> ---
> Changes in v2:
> - New patch
> ---
>  MAINTAINERS   | 1 +
>  doc/board/actions/cubieboard7.rst | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Amit Singh Tomar 

Re: [PATCH 2/7] imx: Remove ARCH= references from documentation

[Amit Tomer]

Hi

On Tue, May 19, 2020 at 8:21 PM Tom Rini  wrote:
>
> When building U-Boot we select the architecture via Kconfig and not ARCH
> being passed in via the environment or make cmdline.
>
> Cc: Adam Ford 
> Cc: Vanessa Maegima 
> Cc: Otavio Salvador 
> Cc: Igor Opaniuk 
> Cc: Amit Singh Tomar 
> Cc: Manivannan Sadhasivam 
> Signed-off-by: Tom Rini 
> ---
> As an aside, the README files that haven't already been converted to rST
> should be converted rST and moved to doc/board/ and the MAINTAINERS file
> updated to include the new documentation file too.  Thanks!
> ---
>  board/beacon/imx8mm/README | 2 +-
>  board/logicpd/imx6/README  | 2 +-
>  board/technexion/pico-imx7d/README.pico-imx7d_BL33 | 2 +-
>  doc/board/actions/cubieboard7.rst  | 2 +-

It is clubbed with imx platform but cubieboard7.rst is for Actions platform
which is completely different from imx.

Thanks
-Amit

Re: [PATCH v1 5/7] arm: dts: s700: add node for ethernet controller

[Amit Tomer]

> So, we're at -rc2 for v2020.07.  The DDR calculation stuff I can see
> getting pulled in.  Is the ethernet driver for this SoC so far from done
> that it's not ready for linux-next?  Things don't have to be in mainline
> proper, but the expectation is that it's making reasonable progress
> there and been reviewed so that binding changes between what we take in
> U-Boot at first and a final re-sync once it is in mainline are minimal.

To be honest, we haven't put any of the Ethernet bits for Linux yet.
We have put few patches for review to support MMC for S700 in Linux,
and once those gets merged we would start working on Ethernet.

Thanks
-Amit

Re: [PATCH v1 5/7] arm: dts: s700: add node for ethernet controller

[Amit Tomer]

Hi,

> The general way to move forward here is that bindings should be getting
> proposed to Linux and accepted there, and U-Boot can take a WIP of them,
> that gets updated later on to match, but we shouldn't get it here first.

I do have a plan to propose this binding to Linux but this is kind of
ready(I mean, the other bits
like PHY and driver) for U-boot. So thought of posting it to the list here.

Thanks
-Amit

Re: [PATCH v1 5/7] arm: dts: s700: add node for ethernet controller

[Amit Tomer]

Hi,

On Tue, May 12, 2020 at 7:49 PM André Przywara  wrote:
>
> On 09/05/2020 15:25, Amit Singh Tomar wrote:
> > This patch adds node for ethernet controller found on Action Semi OWL
> > S700 SoC.
> >
> > Since, there is no upstream Linux binding exist for S700 ethernet
> > controller, Changes are put in u-boot specific dtsi file.
>
> But that should not be the S700 SoC .dtsi, instead the cubieboard .dts
> file, since you specify the PHY mode in here (which is board specific).

But MAC is present on SoC , so I thought of adding it s700 specific
u-boot.dtsi as
it is already hosting few other things .

> > Signed-off-by: Amit Singh Tomar 
> > ---
> >  arch/arm/dts/s700-u-boot.dtsi | 13 +
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/arch/arm/dts/s700-u-boot.dtsi b/arch/arm/dts/s700-u-boot.dtsi
> > index a52775f2..1b2768272c62 100644
> > --- a/arch/arm/dts/s700-u-boot.dtsi
> > +++ b/arch/arm/dts/s700-u-boot.dtsi
> > @@ -6,6 +6,19 @@
> >  /{
> >   soc {
> >   u-boot,dm-pre-reloc;
> > +
> > + gmac:  ethernet@e022 {
> > + compatible = "actions,s700-ethernet";
> > + reg = <0 0xe022 0 0x2000>;
> > + interrupts = ;
> > + interrupt-names = "macirq";
> > + local-mac-address = [ 00 18 fe 66 66 66 ];
>
> Is there another solution to that? Maybe put that in the environment
> instead? Or generate this randomly or ideally by hashing some serial number?

Yeah, I tried having "CONFIG_NET_RANDOM_ETHADDR" other day which seems
to work(would double check it).

But is there a reason not to use "local-mac-address", since driver
does not support parsing this
property ?

Thanks
-Amit.

Re: [PATCH v2 1/2] Actions: OWL: Calculate SDRAM size

[Amit Tomer]

Hi Mani,
>
> This doesn't work on Bubblegum96. But poking into the vendor tree, I'm able
> to come up with below working code:
>
> val = (readl(DMM_INTERLEAVE_PER_CH_CFG) >> 8) & 0xf;
> cap =  64 * (1 << val);
>
> So, you can use this and remove other stuffs. Also this function should be 
> named
> as owl_get_ddr_cap().

Thanks for pointing this out, and I just sent v3 as v2 didn't even
compile for Bubblegum96.

Would send the updated version that works on S900.

Thanks
Amit.

Re: [PATCH] Actions: S700 Calculate SDRAM size

[Amit Tomer]

Hi,

> Have you checked if this setting can be reused for S900? IMO we should use
> this helper for all Owl SoCs. Using CONFIG_SYS_SDRAM_SIZE won't scale.

Looks like S900 uses different registers to find out the size, but
can't really test
this as I don't have the Hardware based on S900.

Thanks
-Amit.

Re: [PATCH v9 00/12] Actions S700 SoC support

[Amit Tomer]

Hi,

> Mostly is looks very good now. I just had few nit picks which you can
> address in v10.

Thanks for providing review comments, just sent the v10 addressing
the comments.

> This series won't make it for v2020.04 but that's not critical IMO.

No issues, It would be good if it gets merge in next cycle.

> Andre, thanks to you too for helping with the reviews. Much appreciated!

Yeah, Thanks for Andre for detailed review and guiding me along the way.

Thanks
-Amit

Re: [PATCH v8 00/12] Actions S700 SoC support

[Amit Tomer]

Hi Mani,

> Just tested v8 on bubblegum96 and it doesn't boot. I can't see any debug
> print on the console, so I'm guessing something basic going wrong. Will
> try to find the regression if I find some time and keep you posted.

Thanks for trying it. Looks like issue is with clock driver (not
reading the driver data
properly). Can you please try it with below series:

https://github.com/Atomar25/u-boot/commits/s700_v9

Thanks
-Amit

Re: [PATCH v4 03/11] clk: actions: Add common clock driver

[Amit Tomer]

Hi,

> Both those include files do not exist yet. This breaks bisectability for
> bubblegum_96.
> So I would suggest you remove the s700 lines for now, and change the
> s900 filename to match the existing one.
> Then change CLK_UART to CLOCK_UART below to make it compile.
>
> Please check that bubblegum_96_defconfig still builds after *every* commit.
>
> Once you sync up the binding include file for S900 later on, you should
> revert those changes back in the same commit.

Did you mean, I should first send sync up the binding patches and
patch 5/11, then this particular(common
clock driver) patch ?

Thanks
-Amit

Re: [PATCH v3 03/21] clk: actions: Add common clock driver

[Amit Tomer]

Hi,

> Indeed, but it's here to not give false impressions: Despite the joint code, 
> the clock driver only works for one SoC at a time. Having only that one 
> respective compatible string in here makes this obvious.
>
> The main reason for this approach is that the register offsets are quite 
> different, dissecting this at runtime does not sound worthwhile for just 
> those few clocks that we need.
> So the idea was to go with one source file, to make use of the commonalities 
> of the clocks, but use #ifdef guards where needed.
>
> This admittedly starts to look quite messy. And with the Ethernet clocks in 
> it is probably already at a point where proper separation would be better.
>
> So to make this easier: Amit, can you remove the Ethernet clocks from this 
> patch, and add them later in your Ethernet series? Maybe this buys us some 
> time to refactor this meanwhile?

Already have taken care of it here:
https://github.com/Atomar25/u-boot/commit/7721ce67d601945b5883fcfa3157207d67c95b1b#diff-0bbc60147217c87a2656365e222cb7b3R81

Thanks
-Amit

Re: [PATCH v3 09/21] arm: dts: Use consistent name "CLK_ETHERNET" for the Ethernet clock binding

[Amit Tomer]

Hi,

> So either you send this patch to the kernel first, or, probably better, you 
> drop this change here, and unify the name at the point where it's used 
> (#ifndef CLK_ETHERNET )

But this is something mentioned in cover letter:

"Patch(9/21) uses same name for ethernet clock binding and if it's ok,
would like to send it to LKML
as well."

anyways as suggested by Mani to drop Ethernet related patches for now,
this patch is not necessary in that case.
Meanwhile, would send it to LKML first for a review.

Thanks
-Amit

Re: [PATCH v3 03/21] clk: actions: Add common clock driver

[Amit Tomer]

Hi,

Thanks for having a look.

> Either use `priv->soc` or the guard throughout the driver. Please don't mix
> both.

But have used #ifdef guard only where it is really necessary and to
keep implementation
clean used priv->soc.

> > +
> > +static const struct udevice_id owl_clk_ids[] = {
> > +#if defined(CONFIG_MACH_S900)
> > + { .compatible = "actions,s900-cmu", .data = S900 },
> > +#elif defined(CONFIG_MACH_S700)
> > + { .compatible = "actions,s700-cmu", .data = S700 },
> > + { }
> > +#endif
>
> Guard is not necessary here.

But this is something suggested by Andre in previous review round

"just protect the compatible strings below with MACH_S[79]00 ifdefs,
so that both SoCs share the
file, but compile to different drivers supporting only one compatible
(due to the different reg_sx00.h and the #ifdef protected parts)."

Thanks
Amit

Re: [RFC PATCH 00/10] arm: add support for SoC S5P4418

[Amit Tomer]

Hi,

On Tue, Feb 4, 2020 at 1:12 AM Stefan Bosch  wrote:
>
>
> This patch adds support for SAMSUNG's/NEXELL's ARM Cortex-A9 based
> S5P4418 SoC, especially FriendlyARM's NanoPi2 and NanoPC-T2 boards.
> It is based on the following FriendlyARM's U-Boot version:
> https://github.com/friendlyarm/u-boot/tree/nanopi2-v2016.01.
>
I don't think this is the right approach, i.e. to take everything from
BSP source as it is and put
it into mainline U-BOOT. AFAIR,  Some of the peripherals present on
these NEXELL SoC's are
compatible with SAMSUNG IP (for instance the UART).
So, are we sure that some of the already existing code in U-BOOT can't
be re-used to drive those
compatible peripherals at-least ?

Thanks
-Amit

Re: [PATCH v3 00/21] Actions S700 SoC support

[Amit Tomer]

Hi Mani,

>
> Sorry for the late reply. Got swamped with lot of stuffs :(
>
> Thanks a lot for the series! I will review it soon and test it on my S900
> based Bubblegum 96 board.

No worries, Thanks for the update!

Thanks
-Amit

Re: [PATCH v3 0/3] Ethernet support for Raspberry Pi 4

[Amit Tomer]

Hi,

> The kernel panic just after with "OF: reserved mem: failed to allocate memory 
> for node 'linux,cma'" but that's another story.

But this comes even without having Ethernet patches and when one use
booti instead of bootefi, right ?

Thanks
-Amit

Re: [PATCH v2 0/3] Ethernet support for Raspberry Pi 4

[Amit Tomer]

Hi,

Thanks for having a look.

> Yeah, I had some success with 5.5-rc, at least till it goes into userland, 
> which is good enough for this purpose.
> And indeed I could reproduce the early crash with genet compiled in vs. 
> mainline U-Boot.

 Could not reproduce it with raspbian image but after using some
ubuntu image definitely see this crash.

>
> writel(0, priv->mac_reg + SYS_RBUF_FLUSH_CTRL);
> udelay(10);
> /* disable MAC while updating its registers */
> writel(0, priv->mac_reg + UMAC_CMD);
> /* issue soft reset with (rg)mii loopback to ensure a stable rxclk */
> writel(CMD_SW_RESET | CMD_LCL_LOOP_EN, priv->mac_reg + UMAC_CMD);
>
> before calling bcmgenet_mdio_init() in bcmgenet_eth_probe() and give this a 
> try.
>
Indeed, it worked for me and can boot both the mainline and distro
kernel with it.
https://paste.ubuntu.com/p/vTY3RJF7Zy/

Also, with images loaded by TFTP
https://pastebin.com/bAgfKhdq

Thanks
-Amit

Re: [PATCH 3/3] riscv: sifive: fu540: add SPL configuration

[Amit Tomer]

Hi Pragnesh,

Minor comments regarding coding style, see below.

> +   // Probably don't need to do this, since
> +   // all the other stuff has been happening.
> +   // But it is on the wave form.

U-boot is mostly implemented in C, we should *not* use C++ style comments(//).
is this something picked from BSP code ?

> +#include "include/ccache.h"
> +#include "include/fu540-memory-map.h"

This looks bit strange, I mean the double include above

> +// Inlining header functions in C
> +// https://stackoverflow.com/a/23699777/7433423

This could be conveyed in comments rather then pointing some external
link.

Thanks
-Amit.

Re: [PATCH 1/3] net: Add support for Broadcom GENETv5 Ethernet controller

[Amit Tomer]

Hi Sascha,

> Are any other cases i could test ? don't worry, one of three boards i
> could damage ! (but it's better if not ... ;-))

Just curious to know what data transfer rate, you see while tftp?

Thanks
-Amit

Re: Sunxi (pine64-lts) SPL load BL32

[Amit Tomer]

Hi Adam,

> I am attempting to boot the Pine64-lts with a Trusted OS.  In the ARM-TF
> documentation it states "make sure the loader (SPL) loads the Trusted OS
> binary to the beginning of DRAM (0x4000)."  I have yet to find a
> mechanism by which U-Boot's SPL for this platform can be configured to do
> this.  Any help would be appreciated.

You can use "sunxi-fel" tool(which is part of sunxi-tool[0]) to load
Secure OS on pine64.

Also, you have to use specific SPL binary[1] to make it work.Besides
that you need USB Type A Male to Type A male cable(connected
from host to upper USB socket on Pine64 machine)

So, you can try following from host machine(after building sunxi-tool package)

# /sunxi-fel -v -p spl sunxi-a64-spl32-ddr3.bin write 0x44000 bl31.bin
write 0x4000 tee-pager_v2.bin write 0x4a00 u-boot.bin write
0x4208 Image write 0x4308 sun50i-a64-pine64-plus.dtb reset64
0x44000

Hi Andre,

Right now, we club different binaries together using FIT image(if I am
right ?), is it a good idea to club Secure OS image as well
with in FIT for A64 based platform ?

[0]: https://github.com/linux-sunxi/sunxi-tools
[1]: https://github.com/apritzel/pine64/tree/master/binaries

Thanks
-Amit

Re: [RFC PATCH 2/2] net: Add Support for GENET Ethernet controller

[Amit Tomer]

Hi,

Thanks for having the look.

>
> I am afraid that needs to be more elaborate. You could mention Broadcom, also 
> that it's only for v5 of it. And that it's based on reverse engineering the 
> Linux driver.
> Text in the cover letter will not make it into the repo, so you should 
> mention this here again.

Ok, would take care of it next version.

> > Signed-off-by: Amit Singh Tomar 
> > ---
> >  configs/rpi_4_defconfig |   2 +
> >  drivers/net/Kconfig |   7 +
> >  drivers/net/Makefile|   1 +
> >  drivers/net/bcmgenet.c  | 770 
> > 
> >  4 files changed, 780 insertions(+)
> >  create mode 100644 drivers/net/bcmgenet.c
> >
> > diff --git a/configs/rpi_4_defconfig b/configs/rpi_4_defconfig
> > index 8cf1bb8..b0f9cf1 100644
> > --- a/configs/rpi_4_defconfig
> > +++ b/configs/rpi_4_defconfig
> > @@ -24,6 +24,8 @@ CONFIG_DM_KEYBOARD=y
> >  CONFIG_DM_MMC=y
> >  CONFIG_MMC_SDHCI=y
> >  CONFIG_MMC_SDHCI_BCM2835=y
> > +CONFIG_DM_ETH=y
>
> I am not sure that belongs into the individual defconfig file, I guess it 
> should be in some Kconfig instead?
>
Ok, Would check it.

>
> > +
> >  config DWC_ETH_QOS
> >   bool "Synopsys DWC Ethernet QOS device support"
> >   depends on DM_ETH
> > diff --git a/drivers/net/Makefile b/drivers/net/Makefile
> > index 3099183..6e0a688 100644
> > --- a/drivers/net/Makefile
> > +++ b/drivers/net/Makefile
> > @@ -8,6 +8,7 @@ obj-$(CONFIG_AG7XXX) += ag7xxx.o
> >  obj-$(CONFIG_ARMADA100_FEC) += armada100_fec.o

> Broadcom GENETv5.
>
> And you could tell a bit about the history here, that it's based on the Linux 
> driver, but limited to support v5 and do away with all the priority queues, 
> etc.
Ok.

> > + *
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +/* Register definitions derived from Linux source */
> > +#define SYS_REV_CTRL  0x00
> > +
> > +#define SYS_PORT_CTRL 0x04
> > +#define PORT_MODE_EXT_GPHY3
> > +
> > +#define GENET_SYS_OFF 0x
> > +#define SYS_RBUF_FLUSH_CTRL   (GENET_SYS_OFF  + 0x08)
> > +#define SYS_TBUF_FLUSH_CTRL   (GENET_SYS_OFF  + 0x0C)
> > +
> > +#define GENET_EXT_OFF 0x0080
> > +#define EXT_RGMII_OOB_CTRL   (GENET_EXT_OFF + 0x0C)
> > +#define RGMII_MODE_EN_V123   BIT(0)
> > +#define RGMII_LINK   BIT(4)
> > +#define OOB_DISABLE  BIT(5)
> > +#define RGMII_MODE_ENBIT(6)
> > +#define ID_MODE_DIS  BIT(16)
> > +
> > +#define GENET_RBUF_OFF0x0300
> > +#define RBUF_FLUSH_CTRL_V1(GENET_RBUF_OFF + 0x04)
> > +#define RBUF_TBUF_SIZE_CTRL  (GENET_RBUF_OFF + 0xb4)
> > +#define RBUF_CTRL(GENET_RBUF_OFF + 0x00)
> > +#define RBUF_64B_EN  BIT(0)
> > +#define RBUF_ALIGN_2BBIT(1)
> > +#define RBUF_BAD_DIS BIT(2)
>
> Thanks for aligning this - I guess it would be in an editor ;-)
> But please try to use tabs consistently for indenting this.
>
> > +
> > +#define GENET_UMAC_OFF0x0800
> > +#define UMAC_MIB_CTRL (GENET_UMAC_OFF + 0x580)
> > +#define UMAC_MAX_FRAME_LEN(GENET_UMAC_OFF + 0x014)
> > +#define UMAC_MAC0 (GENET_UMAC_OFF + 0x00C)
> > +#define UMAC_MAC1 (GENET_UMAC_OFF + 0x010)
> > +#define UMAC_CMD  (GENET_UMAC_OFF + 0x008)
> > +#define MDIO_CMD  (GENET_UMAC_OFF + 0x614)
> > +#define UMAC_TX_FLUSH (GENET_UMAC_OFF + 0x334)
> > +#define MDIO_START_BUSY  BIT(29)
> > +#define MDIO_READ_FAIL   BIT(28)
> > +#define MDIO_RD  (2 << 26)
> > +#define MDIO_WR  BIT(26)
> > +#define MDIO_PMD_SHIFT   21
> > +#define MDIO_PMD_MASK0x1F
> > +#define MDIO_REG_SHIFT   16
> > +#define MDIO_REG_MASK0x1F
> > +
> > +#define CMD_TX_EN BIT(0)
> > +#define CMD_RX_EN BIT(1)
> > +#define UMAC_SPEED_10 0
> > +#define UMAC_SPEED_1001
> > +#define UMAC_SPEED_1000   2
> > +#define UMAC_SPEED_2500   3
> > +#define CMD_SPEED_SHIFT   2
> > +#define CMD_SPEED_MASK3
> > +#define CMD_SW_RESET  BIT(13)
> > +#define CMD_LCL_LOOP_EN   BIT(15)
> > +#define CMD_TX_EN BIT(0)
> > +#define CMD_RX_EN BIT(1)
> > +
> > +#define MIB_RESET_RX  BIT(0)
> > 

Re: [U-Boot-Board-Maintainers] Raspberry pi 4 - u-boot - genet / scb

[Amit Tomer]

Hello Sascha,

> > Am 07.12.19 um 07:23 schrieb Sascha Dewald:
> > > Hello,
> > >
> > > is there any progress yet ?

Sorry for being late on this.

We have made some progress, ping from RPI to host is working now.
Also, managed to received short files(< 63KB) via TFTP.
but haven't managed to get proper handshaking going(that would enable
us to receive bigger files too).

Working on it and keep you posted about the progress.

Thanks
-Amit

Re: [U-Boot] [RFC PATCH 5/8] arm: actions: add S700 SoC device tree

[Amit Tomer]

> Ah, good catch, I agree on the -u-boot.dtsi approach.

Thanks for talking about it( I was expecting this comment)

> Amit, please don't claim it's "sync with Linux" if it isn't. At least
> document this change.

Yeah, I was aware of this situation but rightly so, I should have documented
it in commit messages.
Sorry, I didn't.

> Also there is another instance above, directly on the /soc node.
>
> Maybe we should do as others do and set DM_FLAG_PRE_RELOC on the driver,
> to avoid changing the DT at all?

Yeah, I had something similar in my mind(same was suggested in this
thread[1] few days back)
but wasn't able to test it.

Also, if you see current s900.dtsi in u-boot , it uses those flag in
dtsi itself.

Thanks
-Amit.

[1] : https://lists.denx.de/pipermail/u-boot/2019-January/353438.html

Hate to get up in morning, its cold out here(with bit of rain too) :(
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 1/3] bcm2835_gpio: Add support for pinmux

[Amit Tomer]

Hi,

> +   val = readl(>reg->gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);
> +   val &= ~(BCM2835_GPIO_FSEL_MASK << BCM2835_GPIO_FSEL_SHIFT(gpio));
> +   val |= (func << BCM2835_GPIO_FSEL_SHIFT(gpio));
> +   writel(val, >reg->gpfsel[BCM2835_GPIO_FSEL_BANK(gpio)]);

Can clrsetbits_le32 be used here ?

Thanks
-Amit
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] MXC Spi driver

[Amit Tomer]

> I'm assuming this was supposed to be devfdt_get_addr.

Or, you could try dev_read_addr_ptr.

Thanks
-Amit
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH 2/2] sunxi: A64: enable USB support

[Amit Tomer]

Hello!

>
> Since the driver is dm-driven this config not need.

I tried not to use this config but then driver don't get compiled.

You're talking about   CONFIG_USB_EHCI and CONFIG_USB_EHCI_SUNXI, right ?

Thanks
Amit.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 1/4] net: sun8i_emac: Fix DMA alignment issues with the rx / tx buffers

[Amit Tomer]

Hello,

> index 7c088c3..877859c 100644
> --- a/drivers/net/sun8i_emac.c
> +++ b/drivers/net/sun8i_emac.c
> @@ -32,7 +32,8 @@
>
>  #define CONFIG_TX_DESCR_NUM32
>  #define CONFIG_RX_DESCR_NUM32
> -#define CONFIG_ETH_BUFSIZE 2024
> +#define CONFIG_ETH_BUFSIZE 2048
> +#define CONFIG_ETH_RXSIZE  2024 /* Note most fit in ETH_BUFSIZE */

As per the following comment made in Linux driver.

/* The datasheet said that each descriptor can transfers up to 4096bytes
+ * But latter, a register documentation reduce that value to 2048
+ * Anyway using 2048 cause strange behaviours and even BSP driver use 2047
+ */

Can we keep CONFIG_ETH_BUFSIZE to 2044 ?

Thanks,
Amit.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] Broken H3 EMAC Driver

[Amit Tomer]

Hello,

Trying to add support for H3 EMAC driver.

http://comments.gmane.org/gmane.comp.boot-loaders.u-boot/261544

It was working well until commit "
b733c278d7adc48c71bd06faf359db3d9e385185" was introduced.

net: phy: Handle phy_startup() error codes properly

-   genphy_update_link(phydev);
-   genphy_parse_link(phydev);
+   int ret;
-   return 0;
+   ret = genphy_update_link(phydev);
+   if (ret)
+   return ret;

H3 based SoC's has an internal PHY which is different from having on board
external PHY and its operations doesn't depends upon the success of normal
PHY auto negotiation(Driver works well even if there is a PHY auto
negotiation TIMEOUT).

Please, let me know if it is worth reverting particular part of this patch
in order to make H3 EMAC driver work.

Thanks,
Amit.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [U-BOOT] [PATCH] net: Add EMAC driver for H3/A83T/A64

[Amit Tomer]

Hello,

Thanks for your comments, Sorry for the delayed response.

> 1 is too high

Ok, I will fix it but as per data sheet its 64k region(0x1).

> Space after dot. Try also to keep a minimum of alphabetical order of config.

Ok, I will fix it.

> Sort also headers

Sorry, didn't get your point here.

> I do not know the memory pressure on uboot but the number of descriptors 
> could be less since uboot seems to not have any scattergather for network (so 
> 64 or perhaps 32 could be enought)

Ok, I will keep it to 64 and test.

> You do not check for any reception error.

Yes, I missed it, would take care of it in next version.

> You disable interrupt but I do not see any enabling of it.

Yes, I knew going to get this comment but don't you think after packet
transmission
H/W may enable the interrupt, so it is to take care of that particular case.

Thanks.
Amit.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH 5/5] ls2080ardb: Convert to distro boot

[Amit Tomer]

On Sun, May 15, 2016 at 2:51 AM, Bhupesh Sharma  wrote:
> Note that UEFI firmware support is already available on LS2080A-RDB and allows
> Booting distributions like CentOS on the same.
>

Sorry to intervene here but it's about having the EFI support inside
u-boot itself that
may *not* required to have separate UEFI firmware to boot various distributions.

Please correct, if it's not right ?

Thanks,
Amit
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Re: [U-Boot] [PATCH] net: Add ag7xxx driver for Atheros MIPS

[Amit Tomer]

Hello!

> +
> +#define CONFIG_TX_DESCR_NUM8
> +#define CONFIG_RX_DESCR_NUM8
> +#define CONFIG_ETH_BUFSIZE 2048

 Isn't it too BIG size for normal ethernet frame, unless we need Jumbo Frame ?

Thanks,
Amit.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot