Re: [PATCH 04/10] efi_loader: capsule: add capsule_on_disk support

2020-05-06 Thread AKASHI Takahiro
On Thu, Apr 30, 2020 at 09:51:51PM +0200, Heinrich Schuchardt wrote:
> On 4/30/20 2:52 PM, Sughosh Ganu wrote:
> >
> > On Tue, 28 Apr 2020 at 05:58, AKASHI Takahiro
> > mailto:takahiro.aka...@linaro.org>> wrote:
> >
> > Heinrich,
> >
> > On Mon, Apr 27, 2020 at 10:28:35PM +0200, Heinrich Schuchardt wrote:
> > > On 4/27/20 11:48 AM, AKASHI Takahiro wrote:
> > > > Capsule data can be loaded into the system either via UpdateCapsule
> > > > runtime service or files on a file system (of boot device).
> > > > The latter case is called "capsules on disk", and actual updates
> > will
> > > > take place at the next boot time.
> > > >
> > > > In this commit, we will support capsule on disk mechanism.
> > > >
> > > > Please note that U-Boot itself has no notion of "boot device" and
> > > > all the capsule files to be executed will be detected only if they
> > > > are located in a specific directory, \EFI\UpdateCapsule, on a device
> > > > that is identified as a boot device by "Boot" variables.
> > > >
> > > > Signed-off-by: AKASHI Takahiro  > >
> > > > ---
> > > >  common/main.c                |   4 +
> > > >  include/efi_loader.h         |  16 ++
> > > >  lib/efi_loader/Kconfig       |  22 ++
> > > >  lib/efi_loader/efi_capsule.c | 449
> > +++
> > > >  lib/efi_loader/efi_setup.c   |   9 +
> > > >  5 files changed, 500 insertions(+)
> > > >
> > > > diff --git a/common/main.c b/common/main.c
> > > > index 06d7ff56d60c..877ae63b708d 100644
> > > > --- a/common/main.c
> > > > +++ b/common/main.c
> > > > @@ -14,6 +14,7 @@
> > > >  #include 
> > > >  #include 
> > > >  #include 
> > > > +#include 
> > > >
> > > >  static void run_preboot_environment_command(void)
> > > >  {
> > > > @@ -51,6 +52,9 @@ void main_loop(void)
> > > >     if (IS_ENABLED(CONFIG_UPDATE_TFTP))
> > > >             update_tftp(0UL, NULL, NULL);
> > > >
> > > > +   if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
> > > > +           efi_launch_capsules();
> > > > +
> > >
> > > Can't we move this to efi_init_obj_list() and do away with
> > > CONFIG_EFI_CAPSULE_ON_DISK_EARLY?
> >
> > With CONFIG_EFI_CAPSULE_ON_DISK_EARLY disabled,
> > efi_launch_capsules() will be called in efi_init_obj_list()
> > as you expect. See the code below in efi_setup.c.
> >
> >
> > Instead of calling efi_launch_capsules in efi_init_obj_list, can we
> > invoke the function explicitly through a dedicated command line, under
> > the 'efidebug capsule' class of commands. I think that would be a
> > cleaner approach, since efi_init_obj_list gets called for a lot of efi
> > functions, which are unrelated to capsule update.
> 
> Who would invoke that command line on an IoT device?
> 
> My understanding of the UEFI spec is that capsule updates should be
> invoked automatically.

Right. We must ensure that capsule updates immediately must happen
after reboot.

> I suggested to Takahiro to use the first EFI system partition that we
> find when scanning the available block devices to identify the boot
> device holding the capsules but he dismissed it as contradicting the
> UEFI spec.

Yeah ...

> According to the UEFI 2.8 spec we have to first check BootNext and then
> BootOrder to find the boot option with the highest priority (just like
> the boot manager does). When analysing BootNext and BootOrder we have to
> ignore entries pointing to devices that are not present. This gives us
> the active boot entry.
> 
> On the device identified by the FilePathList field of the active boot
> entry we look for the directory \EFI\UpdateCapsule.
> 
> The UEFI spec says it does not require to check for other EFI system
> partitions. - This could mean it is not forbidden to check other EFI
> system partitions for update capsules.
> 
> The problem with the UEFI spec is that it assumes that variables
> BootNext and BootOrder exist. If they do not exist, the UEFI spec gives
> no hint what to do.

Thank you for detailed explanation instead of me!
The UEFI specification sounds a bit odd, but I can't read it
differently than my interpretation.

> One way to solve this is to populate BootOrder with all block devices.
> This is exactly what my laptop does:
> 
> BootOrder: 0001,,0016,0017,0018,0019,001A,001B
> Boot* Windows Boot Manager
> Boot0001* debian
> Boot0010  Setup
> Boot0011  Boot Menu
> Boot0012  Diagnostic Splash Screen
> Boot0013  Diagnostics
> Boot0014  Startup Interrupt Menu
> Boot0015  Rescue and Recovery
> Boot0016* USB CD
> Boot0017* USB FDD
> Boot0018* NVMe0
> Boot0019* ATA HDD0
> Boot001A* USB HDD
> Boot001B* PCI LAN
> 
> Please, observe that this list contains entries USB CD, USB FDD, USB HDD
> that aren't or even never were physically present on my laptop.
> 
> Another approach is just to wait until 

Re: [PATCH 3/8] qemu: arm64: Add support for efi firmware management protocol routines

2020-05-06 Thread Akashi Takahiro
On Fri, May 01, 2020 at 11:33:42AM +0200, Heinrich Schuchardt wrote:
> On 4/30/20 9:13 PM, Sughosh Ganu wrote:
> >
> > On Fri, 1 May 2020 at 00:09, Heinrich Schuchardt  > > wrote:
> >
> > On 4/30/20 7:36 PM, Sughosh Ganu wrote:
> > > Add support for the get_image_info and set_image routines, which are
> > > part of the efi firmware management protocol.
> > >
> > > The current implementation uses the set_image routine for updating the
> > > u-boot binary image for the qemu arm64 platform. This is supported
> > > using the capsule-on-disk feature of the uefi specification, wherein
> > > the firmware image to be updated is placed on the efi system partition
> > > as a efi capsule under EFI/UpdateCapsule/ directory. Support has been
> > > added for updating the u-boot image on platforms booting with arm
> > > trusted firmware(tf-a), where the u-boot image gets booted as the BL33
> > > payload(bl33.bin).
> > >
> > > The feature can be enabled by the following config options
> > >
> > > CONFIG_EFI_CAPSULE_ON_DISK=y
> > > CONFIG_EFI_FIRMWARE_MANAGEMENT_PROTOCOL=y
> > >
> > > Signed-off-by: Sughosh Ganu  > >
> >
> > U-Boot's UEFI subsystem should work in the same way for x86, ARM, and
> > RISC-V. Please, come up with an architecture independent solution.
> >
> >
> > Please check the explanation that I gave in the other mail. If you check
> > the patch series, the actual capsule authentication logic has been kept
> > architecture agnostic, in efi_capsule.c. The fmp protocol is very much
> > intended for allowing platforms to define their firmware update
> > routines. Edk2 also has platform specific implementation of the fmp
> > protocol under the edk2-platforms directory.
> >
> > -sughosh
> >  
> >
> 
> My idea is that for most platforms it will be enough to have a common
> FMP implementation that consumes a capsule
> 
> * with one or more binaries

Does this assumption apply to most platforms?
If so ("one"),

> * a media device path, a start address, and a truncation flag
>   for each of the binaries

my FIT-based patch[1] meets this assumption and there already
are backend drivers for many media (but not for semihosting :)
as dfu.
(I see little reason to re-invent another set of backend drivers.)

[1] https://lists.denx.de/pipermail/u-boot/2020-April/408767.html


> The protocol implementation then will write the binaries to the device
> paths:
> 
> * to an SD-Card or eMMC exposing the Block IO protocol
>   for most devices
> * to a file in case of the Raspberry Pi or the Sandbox or QEMU
>   (and truncate it if the truncation flag is set)
> 
> If for some devices like a SPI flash we do not have a media device path
> yet, then the only platform specific bit would be the block device
> driver exposing the media device path.
> 
> Same with a semi-hosted file: just add a driver exposing it as a media
> path with an EFI_BLOCK_IO_PROTOCOL.
> 
> For security reasons it may be advisable to make the device read-only
> when reaching ExitBootServices() or even better before the first
> execution of StartImage(). For this purpose we could use the Reset()
> service of the EFI_BLOCK_IO_PROTOCOL or provide a U-Boot specific
> service in the EFI_BLOCK_IO_PROTOCOL.
> 
> Best regards
> 
> Heinrich


Re: [PATCH 8/8] qemu: arm64: Add documentation for capsule update

2020-05-06 Thread Akashi Takahiro
On Fri, May 01, 2020 at 11:17:27AM +0530, Sughosh Ganu wrote:
> On Fri, 1 May 2020 at 00:57, Tom Rini  wrote:
> 
> > On Fri, May 01, 2020 at 12:38:45AM +0530, Sughosh Ganu wrote:
> > > On Fri, 1 May 2020 at 00:07, Heinrich Schuchardt 
> > wrote:
> > >
> > > > On 4/30/20 7:36 PM, Sughosh Ganu wrote:
> > > > > Add documentation highlighting the steps for using the uefi capsule
> > > > > update feature for updating the u-boot firmware image.
> > > > >
> > > > > Signed-off-by: Sughosh Ganu 
> > > >
> > > > UEFI capsule updates should be architecture independent. I would expect
> > > > that the submitted code should work for x86, ARM, and RISC-V. Why does
> > > > this documentation live under the ARM emulation tree?
> > > >
> > >
> > > While the implementation of the core capsule update functionality is
> > indeed
> > > architecture agnostic, this series is for implementing the routines of
> > the
> > > firmware management protocol, which is very much platform specific -- the
> > > routines to perform the actual firmware update would be very much tied to
> > > the platform for which the firmware is being updated. So Takahiro's patch
> > > series, which adds the core capsule update changes is architecture
> > > independent, while this series is adding the routines for the firmware
> > > management protocol, which would be very much platform specific.
> >
> > Since we're talking QEMU here, how much of this can be easily dropped in
> > to QEMU x86_64 and QEMU RISC-V?  If not almost all of it, why?  Can it
> > be reworked as such?
> >
> 
> I don't think it would be too difficult to extend it on other
> architectures, provided there is some mechanism to access and overwrite the
> u-boot binary file from the qemu target. It is currently being done using
> the semihosting interface for the arm architecture. I am not aware if there
> is an interface like semihosting for accessing the u-boot binary on the
> other architectures that you mentioned. Will check on this.

Obviously, another choice would be my FIT-based FMP[1]
as it uses update_tftp(), more specifically dfu_tftp_write(),
internally.

[1] https://lists.denx.de/pipermail/u-boot/2020-April/408767.html

Thanks,
-Takahiro Akashi


> -sughosh


Re: [PATCH 10/13] imx: load calibration parameters from fuse for i.MX8MP

2020-05-06 Thread Fabio Estevam
Hi Peng,

On Wed, May 6, 2020 at 9:08 PM Peng Fan  wrote:

> I'll give a look. Thanks for raising the issue.

I understand the issue now. I will send a patch soon.

Thanks


Re: [PATCH 08/10] cmd: add "efidebug capsule" command

2020-05-06 Thread AKASHI Takahiro
On Thu, Apr 30, 2020 at 06:08:11PM +0530, Sughosh Ganu wrote:
> On Mon, 27 Apr 2020 at 15:19, AKASHI Takahiro 
> wrote:
> 
> > "efidebug capsule" is more or less a debugging utility.
> >   efidebug capsule update: invoke UpdateCapsule against data on memory
> >   efidebug capsule show: show a capsule header
> >   efidebug capsule result: dump a capsule result variable
> >
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >  cmd/efidebug.c | 234 +
> >  1 file changed, 234 insertions(+)
> >
> > diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> > index 02ef01969443..8956fa1d50be 100644
> > --- a/cmd/efidebug.c
> > +++ b/cmd/efidebug.c
> > @@ -19,6 +19,227 @@
> >  #define BS systab.boottime
> >  #define RT systab.runtime
> >
> > +#ifdef CONFIG_EFI_CAPSULE_UPDATE
> >
> 
> This config option is no longer used in the non-rfc series. I think this
> needs to be CONFIG_EFI_HAVE_CAPSULE_SUPPORT.

Correct.

Thanks,
-Takahiro Akashi


> -sughosh


Re: [PATCH 1/1] test: stabilize test_efi_secboot

2020-05-06 Thread AKASHI Takahiro
Heinrich,

On Mon, May 04, 2020 at 12:33:26PM +0200, Heinrich Schuchardt wrote:
> When setting up the console via function efi_console_register() we call
> query_console_serial(). This functions sends an escape sequence to the
> terminal to query the display size. The response is another escape
> sequence.
> 
> console.run_command_list() is looking for a regular expression '^==>'.
> If the escape sequence for the screen size precedes the prompt without a
> line break, no match is found.
> 
> When efi_disk_register() is called before efi_console_register() this leads
> to a test failuere of the UEFI secure boot tests.

Why does the order of those calls affect test results?

> We can avoid the problem if the first UEFI command passed to
> u_boot_console.run_command_list() produces output. This patch achieves this
> by appending '; echo' to the first UEFI related command of the problematic
> tests.

It looks to be a workaround.
We'd better have another approach to fix the problem.
Otherwise, similar issues will occur again.

Thanks,
-Takahiro Akashi

> Signed-off-by: Heinrich Schuchardt 
> ---
>  test/py/tests/test_efi_secboot/test_authvar.py  | 8 
>  test/py/tests/test_efi_secboot/test_signed.py   | 4 ++--
>  test/py/tests/test_efi_secboot/test_unsigned.py | 6 +++---
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/test/py/tests/test_efi_secboot/test_authvar.py 
> b/test/py/tests/test_efi_secboot/test_authvar.py
> index 55dcaa95f1..9912694a3e 100644
> --- a/test/py/tests/test_efi_secboot/test_authvar.py
> +++ b/test/py/tests/test_efi_secboot/test_authvar.py
> @@ -133,7 +133,7 @@ class TestEfiAuthVar(object):
>  output = u_boot_console.run_command_list([
>  'host bind 0 %s' % disk_img,
>  'fatload host 0:1 400 PK.auth',
> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
>  'fatload host 0:1 400 KEK.auth',
>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
>  'fatload host 0:1 400 db.auth',
> @@ -174,7 +174,7 @@ class TestEfiAuthVar(object):
>  output = u_boot_console.run_command_list([
>  'host bind 0 %s' % disk_img,
>  'fatload host 0:1 400 PK.auth',
> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
>  'fatload host 0:1 400 KEK.auth',
>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
>  'fatload host 0:1 400 db.auth',
> @@ -215,7 +215,7 @@ class TestEfiAuthVar(object):
>  output = u_boot_console.run_command_list([
>  'host bind 0 %s' % disk_img,
>  'fatload host 0:1 400 PK.auth',
> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
>  'fatload host 0:1 400 KEK.auth',
>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
>  'fatload host 0:1 400 db.auth',
> @@ -249,7 +249,7 @@ class TestEfiAuthVar(object):
>  output = u_boot_console.run_command_list([
>  'host bind 0 %s' % disk_img,
>  'fatload host 0:1 400 PK.auth',
> -'setenv -e -nv -bs -rt -at -i 400,$filesize PK',
> +'setenv -e -nv -bs -rt -at -i 400,$filesize PK; echo',
>  'fatload host 0:1 400 KEK.auth',
>  'setenv -e -nv -bs -rt -at -i 400,$filesize KEK',
>  'fatload host 0:1 400 db.auth',
> diff --git a/test/py/tests/test_efi_secboot/test_signed.py 
> b/test/py/tests/test_efi_secboot/test_signed.py
> index 584282b338..fc722ab506 100644
> --- a/test/py/tests/test_efi_secboot/test_signed.py
> +++ b/test/py/tests/test_efi_secboot/test_signed.py
> @@ -29,7 +29,7 @@ class TestEfiSignedImage(object):
>  # Test Case 1a, run signed image if no db/dbx
>  output = u_boot_console.run_command_list([
>  'host bind 0 %s' % disk_img,
> -'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed 
> ""',
> +'efidebug boot add 1 HELLO1 host 0:1 /helloworld.efi.signed 
> ""; echo',
>  'efidebug boot next 1',
>  'bootefi bootmgr'])
>  assert(re.search('Hello, world!', ''.join(output)))
> @@ -81,7 +81,7 @@ class TestEfiSignedImage(object):
>  output = u_boot_console.run_command_list([
>  'host bind 0 %s' % disk_img,
>  'fatload host 0:1 400 db.auth',
> -'setenv -e -nv -bs -rt -at -i 400,$filesize dbx',
> +'setenv -e -nv -bs -rt -at -i 400,$filesize dbx; echo',
>  

efi_loader: pkcs7_parse_message() returns error pointer

2020-05-06 Thread Patrick Wildt
Since pkcs7_parse_message() returns an error pointer, we must not
check for NULL.  We have to explicitly set msg to NULL in the error
case, otherwise the call to pkcs7_free_message() on the goto err
path will assume it's a valid object.

Signed-off-by: Patrick Wildt 

diff --git a/lib/efi_loader/efi_image_loader.c 
b/lib/efi_loader/efi_image_loader.c
index 5a9a6424cc..43a53d3dd1 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -538,8 +538,9 @@ static bool efi_image_authenticate(void *efi, size_t 
efi_size)
}
msg = pkcs7_parse_message((void *)wincert + sizeof(*wincert),
  wincert->dwLength - sizeof(*wincert));
-   if (!msg) {
+   if (IS_ERR(msg)) {
debug("Parsing image's signature failed\n");
+   msg = NULL;
goto err;
}
 


efi_loader: efi_variable_parse_signature() returns NULL on error

2020-05-06 Thread Patrick Wildt
efi_variable_parse_signature() returns NULL on error, so IS_NULL()
is an incorrect check.  The goto err leads to pkcs7_free_message(),
which works fine on a NULL ptr.

Signed-off-by: Patrick Wildt 

diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c
index 58f8fae358..c5fe896de2 100644
--- a/lib/efi_loader/efi_variable.c
+++ b/lib/efi_loader/efi_variable.c
@@ -519,9 +519,8 @@ static efi_status_t efi_variable_authenticate(u16 *variable,
var_sig = efi_variable_parse_signature(auth->auth_info.cert_data,
   auth->auth_info.hdr.dwLength
   - sizeof(auth->auth_info));
-   if (IS_ERR(var_sig)) {
+   if (!var_sig) {
debug("Parsing variable's signature failed\n");
-   var_sig = NULL;
goto err;
}
 


RE: [PATCH 10/13] imx: load calibration parameters from fuse for i.MX8MP

2020-05-06 Thread Peng Fan
> Subject: Re: [PATCH 10/13] imx: load calibration parameters from fuse for
> i.MX8MP
> 
> Hi Peng,
> 
> On Mon, May 4, 2020 at 11:50 PM Peng Fan  wrote:
> 
> > Busfreq could be disabled by set the device tree node to disabled.
> 
> This does not help. If I use the NXP U-Boot I can boot the NXP 5.4.3 without
> problems.
> 
> > > I have also tried to boot NXP 5.4.3 on a i.MX8MM EVK with the latest
> > > U-Boot and I also observe a hang.
> >
> > I'll give a try tomorrow when I back to office. There is a ARM clock
> > issue that could cause all i.MX8M kernel hang, but that should be a
> > bit late, should not be that early just after console enabled.
> 
> Currently, we can not even boot to the U-Boot prompt with mainline U-Boot.
> 
> Reverting f24dea4e1b ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP
> EVK") allows it to boot again.
> 
> As discussed in the other thread, this means that the i.MX8MP clock driver is
> broken.
> 
> Please let me know if you can look at these issues.

I'll give a look. Thanks for raising the issue.

Thanks,
Peng.

> 
> Thanks


Re: [PATCH 1/2] test: describe naming conventions for macro UNIT_TEST

2020-05-06 Thread Stephen Warren
On 5/6/20 10:26 AM, Heinrich Schuchardt wrote:
> Strict naming conventions have to be followed for Python function
> generate_ut_subtest() to collect C unit tests to be executed via
> command 'ut'.
> 
> Describe the requirements both on the C as well on the Python side.

> +/**
> + * UNIT_TEST() - create linker generated list entry for unit a unit test
> + *
> + * The macro UNIT_TEST() is used to create a linker generated list entry. 
> These
> + * list entries are enumerate tests that can be execute using the ut command.
> + * The list entries are used both by the implementation of the ut command as
> + * well as in a related Python test.
> + *
> + * For Python testing the subtests are collected in Python function
> + * generate_ut_subtest() by applying a regular expression to the lines of 
> file
> + * u-boot.sym. The list entries have to follow strict naming conventions to 
> be
> + * matched by the expression.
> + *
> + * Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in test suite
> + * foo that can be executed via command 'ut foo bar' and is implemented in
> + * function foo_test_bar().
> + *
> + * @_name:   concatenation of name of the test suite, "_test_", and the name
> + *   of the test
> + * @_flags:  an integer field that can be evaluated by the test suite
> + *   implementation
> + * @_suite:  name of the test suite concatenated with "_test"
> + */

Perhaps the macro could simply take "foo" and "bar" as parameters, and
generate the function name foo_test_bar internally rather than having
the user pass it in. That way, compilation will actively fail if the
function isn't named correctly, since it won't match the reference
created by this macro.

To help make this easier, we could add another macro e.g.
UNIT_TEST_FUNC() that evaluates to just the expected function name, so
that people wouldn't have to know the naming convention when they
implement the function; they'd just write e.g.:

static int UNIT_TEST_FUNC(log, nolog_err)(struct unit_test_state *uts)

But certainly this series is a good first step, and fine even if we
don't implement this suggestion.

The series,
Reviewed-by: Stephen Warren 

TPM: remove duplicate definitions

2020-05-06 Thread Patrick Wildt
With the recent change to tpm-v2.h, some enums are now defined twice
and tpm2_tis_spi.c fails to build.  Unfortunately I fear removing
the defines from tpm_tis.h, like in this diff, will break the TPMv1
drivers tpm_tis_infineon.c and pm_tis_st33zp24_i2c.c, which depend
on those defines.  Maybe they should be added to tpm-v1.h as well?
Or maybe they should be moved from tpm-v2.h to tpm_tis.h?  I have
no solution.

Signed-off-by: Patrick Wildt 

diff --git a/drivers/tpm/tpm_tis.h b/drivers/tpm/tpm_tis.h
index 947585f8e3..4043f1aeaf 100644
--- a/drivers/tpm/tpm_tis.h
+++ b/drivers/tpm/tpm_tis.h
@@ -113,19 +113,4 @@ struct tpm_cmd_t {
  */
 #define MAX_COUNT_LONG 50
 
-enum tis_access {
-   TPM_ACCESS_VALID= 0x80,
-   TPM_ACCESS_ACTIVE_LOCALITY  = 0x20,
-   TPM_ACCESS_REQUEST_PENDING  = 0x04,
-   TPM_ACCESS_REQUEST_USE  = 0x02,
-};
-
-enum tis_status {
-   TPM_STS_VALID   = 0x80,
-   TPM_STS_COMMAND_READY   = 0x40,
-   TPM_STS_GO  = 0x20,
-   TPM_STS_DATA_AVAIL  = 0x10,
-   TPM_STS_DATA_EXPECT = 0x08,
-};
-
 #endif


[PATCH 3/6] patman: Don't try to process checkpatch lines twice

2020-05-06 Thread Simon Glass
Once we have determined what the line refers to there is no point in
processing it further. Update the logic to continue to the next line in
these cases.

Signed-off-by: Simon Glass 
---

 tools/patman/checkpatch.py | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index 16d534b6ae..2cfa9778c6 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -91,9 +91,11 @@ def CheckPatch(fname, verbose=False):
 print(line)
 
 # A blank line indicates the end of a message
-if not line and item:
-result.problems.append(item)
-item = {}
+if not line:
+if item:
+result.problems.append(item)
+item = {}
+continue
 match = re_stats_full.match(line)
 if not match:
 match = re_stats.match(line)
@@ -105,10 +107,13 @@ def CheckPatch(fname, verbose=False):
 result.lines = int(match.group(4))
 else:
 result.lines = int(match.group(3))
+continue
 elif re_ok.match(line):
 result.ok = True
+continue
 elif re_bad.match(line):
 result.ok = False
+continue
 err_match = re_error.match(line)
 warn_match = re_warning.match(line)
 file_match = re_file.match(line)
-- 
2.26.2.645.ge9eca65c58-goog



[PATCH 5/6] patman: Support warnings in the patch subject

2020-05-06 Thread Simon Glass
Sometimes checkpatch outputs problems in the patch subject. Add support
for parsing this output and reporting it correctly.

Signed-off-by: Simon Glass 
---

 tools/patman/checkpatch.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index 5426bb9e9e..5ae450d771 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -127,6 +127,7 @@ def CheckPatch(fname, verbose=False):
 warn_match = re_warning.match(line)
 file_match = re_file.match(line)
 check_match = re_check.match(line)
+subject_match = line.startswith('Subject:')
 if err_match:
 item['msg'] = err_match.group(1)
 item['type'] = 'error'
@@ -139,6 +140,9 @@ def CheckPatch(fname, verbose=False):
 elif file_match:
 item['file'] = file_match.group(1)
 item['line'] = int(file_match.group(2))
+elif subject_match:
+item['file'] = ''
+item['line'] = None
 
 return result
 
@@ -157,7 +161,8 @@ def GetWarningMsg(col, msg_type, fname, line, msg):
 msg_type = col.Color(col.RED, msg_type)
 elif msg_type == 'check':
 msg_type = col.Color(col.MAGENTA, msg_type)
-return '%s:%d: %s: %s\n' % (fname, line, msg_type, msg)
+line_str = '' if line is None else '%d' % line
+return '%s:%s: %s: %s\n' % (fname, line_str, msg_type, msg)
 
 def CheckPatches(verbose, args):
 '''Run the checkpatch.pl script on each patch'''
-- 
2.26.2.645.ge9eca65c58-goog



[PATCH 4/6] patman: Handle checkpatch output with notes and code

2020-05-06 Thread Simon Glass
If checkpatch is configured to output code we should ignore it. Similarly,
notes should be ignored.

Update the logic to handle these situations.

Signed-off-by: Simon Glass 
---

 tools/patman/checkpatch.py | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index 2cfa9778c6..5426bb9e9e 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -85,7 +85,8 @@ def CheckPatch(fname, verbose=False):
 re_warning = re.compile(emacs_prefix + 'WARNING:(?:[A-Z_]+:)? (.*)')
 re_check = re.compile('CHECK: (.*)')
 re_file = re.compile('#\d+: FILE: ([^:]*):(\d+):')
-
+re_note = re.compile('NOTE: (.*)')
+indent = ' ' * 6
 for line in result.stdout.splitlines():
 if verbose:
 print(line)
@@ -96,6 +97,14 @@ def CheckPatch(fname, verbose=False):
 result.problems.append(item)
 item = {}
 continue
+if re_note.match(line):
+continue
+# Skip lines which quote code
+if line.startswith(indent):
+continue
+# Skip code quotes and #
+if line.startswith('+') or line.startswith('#'):
+continue
 match = re_stats_full.match(line)
 if not match:
 match = re_stats.match(line)
-- 
2.26.2.645.ge9eca65c58-goog



[PATCH 6/6] patman: Complain if a checkpatch line is not understood

2020-05-06 Thread Simon Glass
Rather than suffering in silence, output a warning if something about the
checkpatch output cannot be understood.

Signed-off-by: Simon Glass 
---

 tools/patman/checkpatch.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index 5ae450d771..98c63af1dd 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -143,6 +143,8 @@ def CheckPatch(fname, verbose=False):
 elif subject_match:
 item['file'] = ''
 item['line'] = None
+else:
+print('bad line "%s", %d' % (line, len(line)))
 
 return result
 
-- 
2.26.2.645.ge9eca65c58-goog



Re: [PATCH v4 4/4] patman: Modify functional tests for new behavior

2020-05-06 Thread Simon Glass
On Mon, 4 May 2020 at 14:29, Sean Anderson  wrote:
>
> This patch adds or modifies functional tests for the Cover-changes,
> Commit-changes, and Series-process-log tags in order to account for new
> behavior added in the previous few patches. The '(no changes since v1)'
> case is not tested for, since that would need an additional commit to test
> in addition to testing the existing code paths.
>
> Signed-off-by: Sean Anderson 
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - New
>
>  tools/patman/func_test.py | 58 ---
>  .../0001-pci-Correct-cast-for-sandbox.patch   |  3 +
>  ...-for-sandbox-in-fdtdec_setup_mem_siz.patch | 12 +++-
>  tools/patman/test/test01.txt  | 15 -
>  4 files changed, 79 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass 


[PATCH 2/6] patman: Support emacs mode with checkpatch

2020-05-06 Thread Simon Glass
If checkpatch is run in 'emacs' mode it shows the filename at the
start of each line. Add support for this so that the warnings and errors
are correctly detected.

Signed-off-by: Simon Glass 
---

 tools/patman/checkpatch.py | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index a2611a8a82..16d534b6ae 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -72,13 +72,17 @@ def CheckPatch(fname, verbose=False):
 # total: 0 errors, 0 warnings, 159 lines checked
 # or:
 # total: 0 errors, 2 warnings, 7 checks, 473 lines checked
-re_stats = re.compile('total: (\\d+) errors, (\d+) warnings, (\d+)')
-re_stats_full = re.compile('total: (\\d+) errors, (\d+) warnings, (\d+)'
+emacs_prefix = '(?:[0-9]{4}.*\.patch:[0-9]+: )?'
+emacs_stats = '(?:[0-9]{4}.*\.patch )?'
+re_stats = re.compile(emacs_stats +
+  'total: (\\d+) errors, (\d+) warnings, (\d+)')
+re_stats_full = re.compile(emacs_stats +
+   'total: (\\d+) errors, (\d+) warnings, (\d+)'
' checks, (\d+)')
 re_ok = re.compile('.*has no obvious style problems')
 re_bad = re.compile('.*has style problems, please review')
 re_error = re.compile('ERROR: (.*)')
-re_warning = re.compile('WARNING: (.*)')
+re_warning = re.compile(emacs_prefix + 'WARNING:(?:[A-Z_]+:)? (.*)')
 re_check = re.compile('CHECK: (.*)')
 re_file = re.compile('#\d+: FILE: ([^:]*):(\d+):')
 
-- 
2.26.2.645.ge9eca65c58-goog



[PATCH 1/6] patman: Fix 'warning' typo

2020-05-06 Thread Simon Glass
If no warnings are detected due to checkpatch having unexpected options,
patman currently shows an error:

   TypeError: unsupported operand type(s) for +=: 'int' and 'property'

Fix this by initing the variable correctly.

Signed-off-by: Simon Glass 
---

 tools/patman/checkpatch.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py
index 795b519314..a2611a8a82 100644
--- a/tools/patman/checkpatch.py
+++ b/tools/patman/checkpatch.py
@@ -59,7 +59,7 @@ def CheckPatch(fname, verbose=False):
   'stdout']
 result = collections.namedtuple('CheckPatchResult', fields)
 result.ok = False
-result.errors, result.warning, result.checks = 0, 0, 0
+result.errors, result.warnings, result.checks = 0, 0, 0
 result.lines = 0
 result.problems = []
 chk = FindCheckPatch()
-- 
2.26.2.645.ge9eca65c58-goog



[PATCH 0/6] patman: Fixes to allow patman to work with Zephyr OS

2020-05-06 Thread Simon Glass
At present patman doesn't correctly parse the output of checkpatch if the
--emacs and --show-types options are given in the .checkpatch.conf file.

This series corrects these problems, allowing patman to be used to check
patches intended for Zephyr OS.


Simon Glass (6):
  patman: Fix 'warning' typo
  patman: Support emacs mode with checkpatch
  patman: Don't try to process checkpatch lines twice
  patman: Handle checkpatch output with notes and code
  patman: Support warnings in the patch subject
  patman: Complain if a checkpatch line is not understood

 tools/patman/checkpatch.py | 43 ++
 1 file changed, 34 insertions(+), 9 deletions(-)

-- 
2.26.2.645.ge9eca65c58-goog



[PATCH v4 10/12] phy: atheros: add device tree bindings and config

2020-05-06 Thread Michael Walle
Add support for configuring the CLK_25M pin as well as the RGMII I/O
voltage by the device tree.

By default the AT803x PHYs outputs the 25MHz clock of the XTAL input.
But this output can also be changed by software to other frequencies.
This commit introduces a generic way to configure this output.

Also the PHY supports different RGMII I/O voltages: 1.5V, 1.8V and 2.5V.
An internal LDO is able to provide 1.5V (default) and 1.8V. The 2.5V
option needs an external supply voltage. This commit adds support to
switch the internal LDO to 1.8V.

Signed-off-by: Michael Walle 
Acked-by: Joe Hershberger 
---
 doc/device-tree-bindings/net/phy/atheros.txt |  35 +++
 drivers/net/phy/atheros.c| 224 ++-
 include/dt-bindings/net/qca-ar803x.h |  13 ++
 3 files changed, 270 insertions(+), 2 deletions(-)
 create mode 100644 doc/device-tree-bindings/net/phy/atheros.txt
 create mode 100644 include/dt-bindings/net/qca-ar803x.h

diff --git a/doc/device-tree-bindings/net/phy/atheros.txt 
b/doc/device-tree-bindings/net/phy/atheros.txt
new file mode 100644
index 00..97e97b8c13
--- /dev/null
+++ b/doc/device-tree-bindings/net/phy/atheros.txt
@@ -0,0 +1,35 @@
+* Qualcomm Atheros PHY Device Tree binding
+
+Required properties:
+- reg: PHY address
+
+Optional properties:
+- qca,clk-out-frequency: Clock frequency of the CLK_25M pin in Hz.
+   Either 2500, 5000, 6250 or 12500.
+- qca,clk-out-strength: Clock output buffer driver strength.
+Supported values are defined in dt-bindings/net/qca-ar803x.h
+- qca,keep-pll-enabled: Keep the PLL running if no link is present.
+   Don't go into hibernation mode.
+   Only supported on the AR8031/AR8033.
+- vddio-supply: RGMII I/O voltage regulator
+   Only supported on the AR8031/AR8033.
+
+Optional subnodes:
+- vddio-regulator: Initial data for the VDDIO regulator, as covered
+doc/device-tree-bindings/regulator/regulator.txt
+
+Example:
+   #include 
+
+   ethernet-phy@0 {
+   reg = <0>;
+   qca-clk-out-frequency = <12500>;
+   qca,keep-pll-enabled;
+
+   vddio-supply = <>;
+
+   vddio: vddio-regulator {
+   regulator-min-microvolt = <180>;
+   regulator-max-microvolt = <180>;
+   };
+   };
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 22035c2496..3cd301c50e 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -4,21 +4,41 @@
  *
  * Copyright 2011, 2013 Freescale Semiconductor, Inc.
  * author Andy Fleming
+ * Copyright (c) 2019 Michael Walle 
  */
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #define AR803x_PHY_DEBUG_ADDR_REG  0x1d
 #define AR803x_PHY_DEBUG_DATA_REG  0x1e
 
+/* Debug registers */
+#define AR803x_DEBUG_REG_0 0x0
+#define AR803x_RGMII_RX_CLK_DLYBIT(15)
+
 #define AR803x_DEBUG_REG_5 0x5
 #define AR803x_RGMII_TX_CLK_DLYBIT(8)
 
-#define AR803x_DEBUG_REG_0 0x0
-#define AR803x_RGMII_RX_CLK_DLYBIT(15)
+#define AR803x_DEBUG_REG_1F0x1f
+#define AR803x_PLL_ON  BIT(2)
+#define AR803x_RGMII_1V8   BIT(3)
 
 /* CLK_25M register is at MMD 7, address 0x8016 */
 #define AR803x_CLK_25M_SEL_REG 0x8016
+
+#define AR803x_CLK_25M_MASKGENMASK(4, 2)
+#define AR803x_CLK_25M_25MHZ_XTAL  0
+#define AR803x_CLK_25M_25MHZ_DSP   1
+#define AR803x_CLK_25M_50MHZ_PLL   2
+#define AR803x_CLK_25M_50MHZ_DSP   3
+#define AR803x_CLK_25M_62_5MHZ_PLL 4
+#define AR803x_CLK_25M_62_5MHZ_DSP 5
+#define AR803x_CLK_25M_125MHZ_PLL  6
+#define AR803x_CLK_25M_125MHZ_DSP  7
+
 /* AR8035: Select frequency on CLK_25M pin through bits 4:3 */
 #define AR8035_CLK_25M_FREQ_25M(0 | 0)
 #define AR8035_CLK_25M_FREQ_50M(0 | BIT(3))
@@ -26,10 +46,23 @@
 #define AR8035_CLK_25M_FREQ_125M   (BIT(4) | BIT(3))
 #define AR8035_CLK_25M_MASKGENMASK(4, 3)
 
+#define AR803x_CLK_25M_DR_MASK GENMASK(8, 7)
+#define AR803x_CLK_25M_DR_FULL 0
+#define AR803x_CLK_25M_DR_HALF 1
+#define AR803x_CLK_25M_DR_QUARTER  2
+
 #define AR8021_PHY_ID 0x004dd040
 #define AR8031_PHY_ID 0x004dd074
 #define AR8035_PHY_ID 0x004dd072
 
+struct ar803x_priv {
+   int flags;
+#define AR803x_FLAG_KEEP_PLL_ENABLED   BIT(0) /* don't turn off internal PLL */
+#define AR803x_FLAG_RGMII_1V8  BIT(1) /* use 1.8V RGMII I/O voltage */
+   u16 clk_25m_reg;
+   u16 clk_25m_mask;
+};
+
 static int ar803x_debug_reg_read(struct phy_device *phydev, u16 reg)
 {
int ret;
@@ -113,14 +146,193 @@ static int ar803x_delay_config(struct phy_device *phydev)
return ret;
 }
 
+static int ar803x_regs_config(struct phy_device *phydev)
+{
+   struct ar803x_priv *priv = phydev->priv;
+   u16 set = 0, clear = 0;
+   int 

[PATCH v4 12/12] phy: atheros: consolidate {ar8031|ar8035}_config()

2020-05-06 Thread Michael Walle
The two functions are now exactly the same, remove one of them.

Signed-off-by: Michael Walle 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 30 +++---
 1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 3142733105..47ff9f8d44 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -311,31 +311,7 @@ static int ar803x_of_init(struct phy_device *phydev)
return 0;
 }
 
-static int ar8031_config(struct phy_device *phydev)
-{
-   int ret;
-
-   ret = ar803x_of_init(phydev);
-   if (ret < 0)
-   return ret;
-
-   ret = ar803x_delay_config(phydev);
-   if (ret < 0)
-   return ret;
-
-   ret = ar803x_regs_config(phydev);
-   if (ret < 0)
-   return ret;
-
-   phydev->supported = phydev->drv->features;
-
-   genphy_config_aneg(phydev);
-   genphy_restart_aneg(phydev);
-
-   return 0;
-}
-
-static int ar8035_config(struct phy_device *phydev)
+static int ar803x_config(struct phy_device *phydev)
 {
int ret;
 
@@ -374,7 +350,7 @@ static struct phy_driver AR8031_driver =  {
.uid = AR8031_PHY_ID,
.mask = 0xffef,
.features = PHY_GBIT_FEATURES,
-   .config = ar8031_config,
+   .config = ar803x_config,
.startup = genphy_startup,
.shutdown = genphy_shutdown,
 };
@@ -384,7 +360,7 @@ static struct phy_driver AR8035_driver =  {
.uid = AR8035_PHY_ID,
.mask = 0xffef,
.features = PHY_GBIT_FEATURES,
-   .config = ar8035_config,
+   .config = ar803x_config,
.startup = genphy_startup,
.shutdown = genphy_shutdown,
 };
-- 
2.20.1



[PATCH v4 11/12] phy: atheros: ar8035: remove static clock config

2020-05-06 Thread Michael Walle
We can configure the clock output in the device tree. Disable the
hardcoded one in here. This is highly board-specific and should have
never been enabled in the PHY driver.

If bisecting shows that this commit breaks your board it probably
depends on the clock output of your Atheros AR8035 PHY. Please have a
look at doc/device-tree-bindings/net/phy/atheros.txt. You need to set
"clk-out-frequency = <12500>" because that value was the hardcoded
value until this commit.

Signed-off-by: Michael Walle 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 3cd301c50e..3142733105 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -38,12 +38,6 @@
 #define AR803x_CLK_25M_62_5MHZ_DSP 5
 #define AR803x_CLK_25M_125MHZ_PLL  6
 #define AR803x_CLK_25M_125MHZ_DSP  7
-
-/* AR8035: Select frequency on CLK_25M pin through bits 4:3 */
-#define AR8035_CLK_25M_FREQ_25M(0 | 0)
-#define AR8035_CLK_25M_FREQ_50M(0 | BIT(3))
-#define AR8035_CLK_25M_FREQ_62M(BIT(4) | 0)
-#define AR8035_CLK_25M_FREQ_125M   (BIT(4) | BIT(3))
 #define AR8035_CLK_25M_MASKGENMASK(4, 3)
 
 #define AR803x_CLK_25M_DR_MASK GENMASK(8, 7)
@@ -344,18 +338,11 @@ static int ar8031_config(struct phy_device *phydev)
 static int ar8035_config(struct phy_device *phydev)
 {
int ret;
-   int regval;
 
ret = ar803x_of_init(phydev);
if (ret < 0)
return ret;
 
-   /* Configure CLK_25M output clock at 125 MHz */
-   regval = phy_read_mmd(phydev, MDIO_MMD_AN, AR803x_CLK_25M_SEL_REG);
-   regval &= ~AR8035_CLK_25M_MASK; /* No surprises */
-   regval |= AR8035_CLK_25M_FREQ_125M;
-   phy_write_mmd(phydev, MDIO_MMD_AN, AR803x_CLK_25M_SEL_REG, regval);
-
ret = ar803x_delay_config(phydev);
if (ret < 0)
return ret;
-- 
2.20.1



[PATCH v4 08/12] phy: atheros: introduce debug read and write functions

2020-05-06 Thread Michael Walle
Provide functions to read and write the Atheros debug registers.

Signed-off-by: Michael Walle 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 57 ---
 1 file changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 5ff5875d3d..660dcd9491 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -30,32 +30,57 @@
 #define AR8031_PHY_ID 0x004dd074
 #define AR8035_PHY_ID 0x004dd072
 
-static void ar803x_enable_rx_delay(struct phy_device *phydev, bool on)
+static int ar803x_debug_reg_read(struct phy_device *phydev, u16 reg)
 {
-   int regval;
+   int ret;
+
+   ret = phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+   reg);
+   if (ret < 0)
+   return ret;
+
+   return phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
+}
+
+static int ar803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
+u16 clear, u16 set)
+{
+   int val;
+
+   val = ar803x_debug_reg_read(phydev, reg);
+   if (val < 0)
+   return val;
+
+   val &= 0x;
+   val &= ~clear;
+   val |= set;
+
+   return phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
+val);
+}
+
+static int ar803x_enable_rx_delay(struct phy_device *phydev, bool on)
+{
+   u16 clear = 0, set = 0;
 
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
- AR803x_DEBUG_REG_0);
-   regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
if (on)
-   regval |= AR803x_RGMII_RX_CLK_DLY;
+   set = AR803x_RGMII_RX_CLK_DLY;
else
-   regval &= ~AR803x_RGMII_RX_CLK_DLY;
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval);
+   clear = AR803x_RGMII_RX_CLK_DLY;
+
+   return ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_0, clear, set);
 }
 
-static void ar803x_enable_tx_delay(struct phy_device *phydev, bool on)
+static int ar803x_enable_tx_delay(struct phy_device *phydev, bool on)
 {
-   int regval;
+   u16 clear = 0, set = 0;
 
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
- AR803x_DEBUG_REG_5);
-   regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
if (on)
-   regval |= AR803x_RGMII_TX_CLK_DLY;
+   set = AR803x_RGMII_TX_CLK_DLY;
else
-   regval &= ~AR803x_RGMII_TX_CLK_DLY;
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval);
+   clear = AR803x_RGMII_TX_CLK_DLY;
+
+   return ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_5, clear, set);
 }
 
 static int ar8021_config(struct phy_device *phydev)
-- 
2.20.1



[PATCH v4 09/12] phy: atheros: move delay config to common function

2020-05-06 Thread Michael Walle
Signed-off-by: Michael Walle 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 38 ++
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 660dcd9491..22035c2496 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -94,19 +94,32 @@ static int ar8021_config(struct phy_device *phydev)
return 0;
 }
 
-static int ar8031_config(struct phy_device *phydev)
+static int ar803x_delay_config(struct phy_device *phydev)
 {
+   int ret;
+
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
-   ar803x_enable_tx_delay(phydev, true);
+   ret = ar803x_enable_tx_delay(phydev, true);
else
-   ar803x_enable_tx_delay(phydev, false);
+   ret = ar803x_enable_tx_delay(phydev, false);
 
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
-   ar803x_enable_rx_delay(phydev, true);
+   ret = ar803x_enable_rx_delay(phydev, true);
else
-   ar803x_enable_rx_delay(phydev, false);
+   ret = ar803x_enable_rx_delay(phydev, false);
+
+   return ret;
+}
+
+static int ar8031_config(struct phy_device *phydev)
+{
+   int ret;
+
+   ret = ar803x_delay_config(phydev);
+   if (ret < 0)
+   return ret;
 
phydev->supported = phydev->drv->features;
 
@@ -118,6 +131,7 @@ static int ar8031_config(struct phy_device *phydev)
 
 static int ar8035_config(struct phy_device *phydev)
 {
+   int ret;
int regval;
 
/* Configure CLK_25M output clock at 125 MHz */
@@ -126,17 +140,9 @@ static int ar8035_config(struct phy_device *phydev)
regval |= AR8035_CLK_25M_FREQ_125M;
phy_write_mmd(phydev, MDIO_MMD_AN, AR803x_CLK_25M_SEL_REG, regval);
 
-   if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
-   ar803x_enable_tx_delay(phydev, true);
-   else
-   ar803x_enable_tx_delay(phydev, false);
-
-   if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID))
-   ar803x_enable_rx_delay(phydev, true);
-   else
-   ar803x_enable_rx_delay(phydev, false);
+   ret = ar803x_delay_config(phydev);
+   if (ret < 0)
+   return ret;
 
phydev->supported = phydev->drv->features;
 
-- 
2.20.1



[PATCH v4 07/12] phy: atheros: use defines for PHY IDs

2020-05-06 Thread Michael Walle
Signed-off-by: Michael Walle 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 01953a1390..5ff5875d3d 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -26,6 +26,10 @@
 #define AR8035_CLK_25M_FREQ_125M   (BIT(4) | BIT(3))
 #define AR8035_CLK_25M_MASKGENMASK(4, 3)
 
+#define AR8021_PHY_ID 0x004dd040
+#define AR8031_PHY_ID 0x004dd074
+#define AR8035_PHY_ID 0x004dd072
+
 static void ar803x_enable_rx_delay(struct phy_device *phydev, bool on)
 {
int regval;
@@ -119,7 +123,7 @@ static int ar8035_config(struct phy_device *phydev)
 
 static struct phy_driver AR8021_driver =  {
.name = "AR8021",
-   .uid = 0x4dd040,
+   .uid = AR8021_PHY_ID,
.mask = 0xfff0,
.features = PHY_GBIT_FEATURES,
.config = ar8021_config,
@@ -129,7 +133,7 @@ static struct phy_driver AR8021_driver =  {
 
 static struct phy_driver AR8031_driver =  {
.name = "AR8031/AR8033",
-   .uid = 0x4dd074,
+   .uid = AR8031_PHY_ID,
.mask = 0xffef,
.features = PHY_GBIT_FEATURES,
.config = ar8031_config,
@@ -139,7 +143,7 @@ static struct phy_driver AR8031_driver =  {
 
 static struct phy_driver AR8035_driver =  {
.name = "AR8035",
-   .uid = 0x4dd072,
+   .uid = AR8035_PHY_ID,
.mask = 0xffef,
.features = PHY_GBIT_FEATURES,
.config = ar8035_config,
-- 
2.20.1



[PATCH v4 04/12] phy: atheros: Explicitly disable RGMII delays

2020-05-06 Thread Michael Walle
From: Vladimir Oltean 

To eliminate any doubts about the out-of-reset value of the PHY, that
the driver previously relied on.

If bisecting shows that this commit breaks your board you probably have
a wrong PHY interface mode. You probably want the
PHY_INTERFACE_MODE_RGMII_RXID or PHY_INTERFACE_MODE_RGMII_ID mode.

Signed-off-by: Vladimir Oltean 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 1da18eb5d4..3e59c3f391 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -70,10 +70,14 @@ static int ar8031_config(struct phy_device *phydev)
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
ar803x_enable_tx_delay(phydev, true);
+   else
+   ar803x_enable_tx_delay(phydev, false);
 
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
ar803x_enable_rx_delay(phydev, true);
+   else
+   ar803x_enable_rx_delay(phydev, false);
 
phydev->supported = phydev->drv->features;
 
@@ -96,10 +100,14 @@ static int ar8035_config(struct phy_device *phydev)
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
ar803x_enable_tx_delay(phydev, true);
+   else
+   ar803x_enable_tx_delay(phydev, false);
 
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID))
ar803x_enable_rx_delay(phydev, true);
+   else
+   ar803x_enable_rx_delay(phydev, false);
 
phydev->supported = phydev->drv->features;
 
-- 
2.20.1



[PATCH v4 06/12] phy: atheros: fix AR8021 PHY ID mask

2020-05-06 Thread Michael Walle
The upper bits are all the OUI.

Signed-off-by: Michael Walle 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 3cc162828c..01953a1390 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -120,7 +120,7 @@ static int ar8035_config(struct phy_device *phydev)
 static struct phy_driver AR8021_driver =  {
.name = "AR8021",
.uid = 0x4dd040,
-   .mask = 0x40,
+   .mask = 0xfff0,
.features = PHY_GBIT_FEATURES,
.config = ar8021_config,
.startup = genphy_startup,
-- 
2.20.1



[PATCH v4 05/12] phy: atheros: Clarify the intention of ar8021_config

2020-05-06 Thread Michael Walle
From: Vladimir Oltean 

Debug register 5 contains TX_CLK DELAY at bit 8 and reserved values at
the other bit positions, just like the other PHYs in the family do.
Therefore, it is not necessary to hardcode the reserved values, but
instead simply follow the read-modify-write procedure from the common
function.

Signed-off-by: Vladimir Oltean 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 3e59c3f391..3cc162828c 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -56,10 +56,10 @@ static void ar803x_enable_tx_delay(struct phy_device 
*phydev, bool on)
 
 static int ar8021_config(struct phy_device *phydev)
 {
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200);
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
- AR803x_DEBUG_REG_5);
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, 0x3D47);
+   phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR,
+ BMCR_ANENABLE | BMCR_ANRESTART);
+
+   ar803x_enable_tx_delay(phydev, true);
 
phydev->supported = phydev->drv->features;
return 0;
-- 
2.20.1



[PATCH v4 00/12] phy: atheros: dt bindings and cleanup

2020-05-06 Thread Michael Walle
This patch series superseeds the following two:
>From Vladimir Oltean
  https://patchwork.ozlabs.org/cover/1031360/
>From me:
  https://patchwork.ozlabs.org/cover/1184507/

Although the first is marked as accepted into u-boot-net I guess it was
removed due to broken boards ("DT as ABI", RGMII delay was fixed and thus
breaks the board).

After disussing with Vladimir, I've integrated his patches with this
series. Also the first one
  Address packet drops at low traffic rate due to SmartEEE feature
was dropped because it will likely be fixed by making u-boot support the
eee-broken-X device tree properties. Apart from that, only the subject was
changed and a note about possible board breakage was added the patch which
changes the delay behaviour.

For all of those, who will test this patchset, the device tree binding
needs the phydev->node property, which needs to be set in every network
driver. If the device tree binding is not working for you have a look at
the
  ar803x_of_init: found PHY node: phy@0
output. In the case above "phy@0" is the phy node in the device tree. If
instead the node of your network device is displayed, you have to set
the phydev->node property in your network device driver.

For the fsl_enetc driver this patchset will add it:
  https://patchwork.ozlabs.org/cover/1188043/

changes since v3:
 - add acked-by's, thanks Joe!

changes since v2:
 - rebased onto latest master, esp. #include 

changes since v1:
 - pull all Vladimirs Oltan's patches and rebase mine onto them
 - fix the CLK_25M settings for the AR8035
 - add two new patches "fix AR8021 PHY ID mask" and "use defines for PHY
   IDs"
 - use the new kernel device tree binding for the AR803x PHYs:
   https://patchwork.ozlabs.org/patch/1188293/
 - add debugging output

Michael Walle (7):
  phy: atheros: fix AR8021 PHY ID mask
  phy: atheros: use defines for PHY IDs
  phy: atheros: introduce debug read and write functions
  phy: atheros: move delay config to common function
  phy: atheros: add device tree bindings and config
  phy: atheros: ar8035: remove static clock config
  phy: atheros: consolidate {ar8031|ar8035}_config()

Vladimir Oltean (5):
  phy: atheros: Make RGMII Tx delays actually configurable for AR8035
  phy: atheros: Use common functions for RGMII internal delays
  phy: atheros: Clarify the configuration of the CLK_25M output pin
  phy: atheros: Explicitly disable RGMII delays
  phy: atheros: Clarify the intention of ar8021_config

 doc/device-tree-bindings/net/phy/atheros.txt |  35 ++
 drivers/net/phy/atheros.c| 350 ---
 include/dt-bindings/net/qca-ar803x.h |  13 +
 3 files changed, 345 insertions(+), 53 deletions(-)
 create mode 100644 doc/device-tree-bindings/net/phy/atheros.txt
 create mode 100644 include/dt-bindings/net/qca-ar803x.h

-- 
2.20.1



[PATCH v4 02/12] phy: atheros: Use common functions for RGMII internal delays

2020-05-06 Thread Michael Walle
From: Vladimir Oltean 

Signed-off-by: Vladimir Oltean 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 69 +++
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 537c1a9125..c0c2b4db39 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -12,16 +12,45 @@
 #define AR803x_PHY_DEBUG_DATA_REG  0x1e
 
 #define AR803x_DEBUG_REG_5 0x5
-#define AR803x_RGMII_TX_CLK_DLY0x100
+#define AR803x_RGMII_TX_CLK_DLYBIT(8)
 
 #define AR803x_DEBUG_REG_0 0x0
-#define AR803x_RGMII_RX_CLK_DLY0x8000
+#define AR803x_RGMII_RX_CLK_DLYBIT(15)
+
+static void ar803x_enable_rx_delay(struct phy_device *phydev, bool on)
+{
+   int regval;
+
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+ AR803x_DEBUG_REG_0);
+   regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
+   if (on)
+   regval |= AR803x_RGMII_RX_CLK_DLY;
+   else
+   regval &= ~AR803x_RGMII_RX_CLK_DLY;
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval);
+}
+
+static void ar803x_enable_tx_delay(struct phy_device *phydev, bool on)
+{
+   int regval;
+
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+ AR803x_DEBUG_REG_5);
+   regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
+   if (on)
+   regval |= AR803x_RGMII_TX_CLK_DLY;
+   else
+   regval &= ~AR803x_RGMII_TX_CLK_DLY;
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval);
+}
 
 static int ar8021_config(struct phy_device *phydev)
 {
phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x3D47);
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+ AR803x_DEBUG_REG_5);
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, 0x3D47);
 
phydev->supported = phydev->drv->features;
return 0;
@@ -30,20 +59,12 @@ static int ar8021_config(struct phy_device *phydev)
 static int ar8031_config(struct phy_device *phydev)
 {
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
-   phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
- AR803x_DEBUG_REG_5);
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
- AR803x_RGMII_TX_CLK_DLY);
-   }
+   phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+   ar803x_enable_tx_delay(phydev, true);
 
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
-   phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
- AR803x_DEBUG_REG_0);
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
- AR803x_RGMII_RX_CLK_DLY);
-   }
+   phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+   ar803x_enable_rx_delay(phydev, true);
 
phydev->supported = phydev->drv->features;
 
@@ -64,20 +85,12 @@ static int ar8035_config(struct phy_device *phydev)
phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
 
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
-   /* select debug reg 5 */
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1D, 0x5);
-   /* enable tx delay */
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1E, 0x0100);
-   }
+   (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
+   ar803x_enable_tx_delay(phydev, true);
 
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)) {
-   /* select debug reg 0 */
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1D, 0x0);
-   /* enable rx delay */
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1E, 0x8000);
-   }
+   (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID))
+   ar803x_enable_rx_delay(phydev, true);
 
phydev->supported = phydev->drv->features;
 
-- 
2.20.1



[PATCH v4 01/12] phy: atheros: Make RGMII Tx delays actually configurable for AR8035

2020-05-06 Thread Michael Walle
From: Vladimir Oltean 

Delete the extraneous write to debug reg 5 that enables Tx delay

When the driver was originally introduced in commit "6027384a phylib:
Add Atheros AR8035 GETH PHY support", the Tx delay was being
unconditionally enabled.

Then during "2ec4d10b phy: atheros: add support for RGMII_ID, RGMII_TXID
and RGMII_RXID", the author did not notice that code for enabling Tx
delay code was already. Therefore, the if condition for Tx delay has
always been useless for this PHY since this commit introduced it.

Prior to this patch, every AR8035 PHY in U-boot had Tx delay enabled.
After this patch, only those who define the interface as RGMII_TXID or
RGMII_ID will. This is to be expected, but will nonetheless break the
setups of those who didn't know they rely on Tx delay implicitly.

Signed-off-by: Vladimir Oltean 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 3783d155e7..537c1a9125 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -63,10 +63,6 @@ static int ar8035_config(struct phy_device *phydev)
regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe);
phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
 
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
-   regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, (regval|0x0100));
-
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
/* select debug reg 5 */
-- 
2.20.1



[PATCH v4 03/12] phy: atheros: Clarify the configuration of the CLK_25M output pin

2020-05-06 Thread Michael Walle
From: Vladimir Oltean 

Also take the opportunity to use the phy_read_mmd and phy_write_mmd
convenience functions.

Signed-off-by: Vladimir Oltean 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index c0c2b4db39..1da18eb5d4 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -17,6 +17,15 @@
 #define AR803x_DEBUG_REG_0 0x0
 #define AR803x_RGMII_RX_CLK_DLYBIT(15)
 
+/* CLK_25M register is at MMD 7, address 0x8016 */
+#define AR803x_CLK_25M_SEL_REG 0x8016
+/* AR8035: Select frequency on CLK_25M pin through bits 4:3 */
+#define AR8035_CLK_25M_FREQ_25M(0 | 0)
+#define AR8035_CLK_25M_FREQ_50M(0 | BIT(3))
+#define AR8035_CLK_25M_FREQ_62M(BIT(4) | 0)
+#define AR8035_CLK_25M_FREQ_125M   (BIT(4) | BIT(3))
+#define AR8035_CLK_25M_MASKGENMASK(4, 3)
+
 static void ar803x_enable_rx_delay(struct phy_device *phydev, bool on)
 {
int regval;
@@ -78,11 +87,11 @@ static int ar8035_config(struct phy_device *phydev)
 {
int regval;
 
-   phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007);
-   regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
+   /* Configure CLK_25M output clock at 125 MHz */
+   regval = phy_read_mmd(phydev, MDIO_MMD_AN, AR803x_CLK_25M_SEL_REG);
+   regval &= ~AR8035_CLK_25M_MASK; /* No surprises */
+   regval |= AR8035_CLK_25M_FREQ_125M;
+   phy_write_mmd(phydev, MDIO_MMD_AN, AR803x_CLK_25M_SEL_REG, regval);
 
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
-- 
2.20.1



Re: raspberrypi cm3 v2019.07 not working

2020-05-06 Thread Belisko Marek
On Tue, May 5, 2020 at 10:18 PM Belisko Marek  wrote:
>
> Hi,
>
> I'm trying to run 2019.07 u-boot on raspberrypi cm3 module but I got
> no feedback on console. When used 2018.01 it boots fine. Probably I
> need to do git bisect but maybe someone have some idea or experience
> what can cause that. Thanks.
I give a try 2020.01 and I got only this:
U-Boot 2020.01 (May 06 2020 - 20:09:45 +)

DRAM:  948 MiB
RPI Compute Module 3 (0xa020a0)

I'll try to debug it and let you know.

BR,

marek


-- 
as simple and primitive as possible
-
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com


Re: [PATCH 32/36] bdinfo: m68k: Move m68k-specific info into its own file

2020-05-06 Thread Angelo Dureghello
On Tue, May 5, 2020 at 1:19 AM Simon Glass  wrote:
>
> We don't really want to have m68k-specific code in a generic file. Create
> a new arch-specific function to hold it, and move it into that.
>
> Make the function weak so that any arch can implement it.
>
> Signed-off-by: Simon Glass 
> ---
>
>  arch/m68k/lib/Makefile |  1 +
>  arch/m68k/lib/bdinfo.c | 29 +
>  cmd/bdinfo.c   | 15 ---
>  3 files changed, 30 insertions(+), 15 deletions(-)
>  create mode 100644 arch/m68k/lib/bdinfo.c

Tested on stmark2

stmark2 $ bdi
boot_params = 0x47d96770
DRAM bank   = 0x
-> start= 0x4000
-> size = 0x0800
memstart= 0x4000
memsize = 0x0800
flashstart  = 0x
flashsize   = 0x
flashoffset = 0x
baudrate= 115200 bps
relocaddr   = 0x47dd5000
reloc off   = 0xfffd4c00
Build   = 32-bit
fdt_blob= 0x47d90670
new_fdt = 0x47d90670
fdt_size= 0x2860
sramstart   = 0x
sramsize= 0x
busfreq =120 MHz
mbar= 0xfc00
cpufreq =240 MHz
flbfreq = 60 MHz
inpfreq = 30 MHz


Tested-by: Angelo Dureghello 


Re: [PATCH] lib: rsa: Fix unaligned 64-bit fdt accesses

2020-05-06 Thread Tom Rini
On Wed, May 06, 2020 at 06:32:03PM +0200, Jan Kiszka wrote:

> From: Jan Kiszka 
> 
> The fdt only provides 32-bit alignment of data. If the public_exponent
> happens to be not 64-bit aligned, we can trigger an exception on certain
> architectures. Seen on TI AM64x.
> 
> Note that the normal way of accessing such a number would be
> fdtdec_get_number. However, this is not available for tools, and this
> is one use case for lib/rsa.
> 
> Signed-off-by: Jan Kiszka 

This is the same as:
http://patchwork.ozlabs.org/project/uboot/patch/20200503112634.590399-1-he...@sntech.de/
I think which I'm testing right now.  Can you please confirm and
tested-by?  Thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3 00/12] phy: atheros: dt bindings and cleanup

2020-05-06 Thread Tom Rini
On Wed, May 06, 2020 at 09:47:10PM +0200, Michael Walle wrote:
> Am 2020-05-06 21:14, schrieb Michael Walle:
> > This patch series superseeds the following two:
> > From Vladimir Oltean
> >   https://patchwork.ozlabs.org/cover/1031360/
> > From me:
> >   https://patchwork.ozlabs.org/cover/1184507/
> > 
> > Although the first is marked as accepted into u-boot-net I guess it was
> > removed due to broken boards ("DT as ABI", RGMII delay was fixed and
> > thus
> > breaks the board).
> > 
> > After disussing with Vladimir, I've integrated his patches with this
> > series. Also the first one
> >   Address packet drops at low traffic rate due to SmartEEE feature
> > was dropped because it will likely be fixed by making u-boot support the
> > eee-broken-X device tree properties. Apart from that, only the subject
> > was
> > changed and a note about possible board breakage was added the patch
> > which
> > changes the delay behaviour.
> > 
> > For all of those, who will test this patchset, the device tree binding
> > needs the phydev->node property, which needs to be set in every network
> > driver. If the device tree binding is not working for you have a look at
> > the
> >   ar803x_of_init: found PHY node: phy@0
> > output. In the case above "phy@0" is the phy node in the device tree. If
> > instead the node of your network device is displayed, you have to set
> > the phydev->node property in your network device driver.
> > 
> > For the fsl_enetc driver this patchset will add it:
> >   https://patchwork.ozlabs.org/cover/1188043/
> > 
> > changes since v2:
> >  - rebased onto latest master, esp. #include 
> > 
> > changes since v1:
> >  - pull all Vladimirs Oltan's patches and rebase mine onto them
> >  - fix the CLK_25M settings for the AR8035
> >  - add two new patches "fix AR8021 PHY ID mask" and "use defines for PHY
> >IDs"
> >  - use the new kernel device tree binding for the AR803x PHYs:
> >https://patchwork.ozlabs.org/patch/1188293/
> >  - add debugging output
> > 
> > Michael Walle (7):
> >   phy: atheros: fix AR8021 PHY ID mask
> >   phy: atheros: use defines for PHY IDs
> >   phy: atheros: introduce debug read and write functions
> >   phy: atheros: move delay config to common function
> >   phy: atheros: add device tree bindings and config
> >   phy: atheros: ar8035: remove static clock config
> >   phy: atheros: consolidate {ar8031|ar8035}_config()
> 
> Oh I've actually forgot the "Acked-by:" from Joe from the v2 series.
> 
> All but the
> >   phy: atheros: fix AR8021 PHY ID mask
> were acked by Joe.
> 
> Tom, should I send a v4 where I drop that patch and could you pull
> this series?

Yes please, sounds like a plan, thanks for the explanation!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3 00/12] phy: atheros: dt bindings and cleanup

2020-05-06 Thread Michael Walle

Am 2020-05-06 21:14, schrieb Michael Walle:

This patch series superseeds the following two:
From Vladimir Oltean
  https://patchwork.ozlabs.org/cover/1031360/
From me:
  https://patchwork.ozlabs.org/cover/1184507/

Although the first is marked as accepted into u-boot-net I guess it was
removed due to broken boards ("DT as ABI", RGMII delay was fixed and 
thus

breaks the board).

After disussing with Vladimir, I've integrated his patches with this
series. Also the first one
  Address packet drops at low traffic rate due to SmartEEE feature
was dropped because it will likely be fixed by making u-boot support 
the
eee-broken-X device tree properties. Apart from that, only the subject 
was
changed and a note about possible board breakage was added the patch 
which

changes the delay behaviour.

For all of those, who will test this patchset, the device tree binding
needs the phydev->node property, which needs to be set in every network
driver. If the device tree binding is not working for you have a look 
at

the
  ar803x_of_init: found PHY node: phy@0
output. In the case above "phy@0" is the phy node in the device tree. 
If

instead the node of your network device is displayed, you have to set
the phydev->node property in your network device driver.

For the fsl_enetc driver this patchset will add it:
  https://patchwork.ozlabs.org/cover/1188043/

changes since v2:
 - rebased onto latest master, esp. #include 

changes since v1:
 - pull all Vladimirs Oltan's patches and rebase mine onto them
 - fix the CLK_25M settings for the AR8035
 - add two new patches "fix AR8021 PHY ID mask" and "use defines for 
PHY

   IDs"
 - use the new kernel device tree binding for the AR803x PHYs:
   https://patchwork.ozlabs.org/patch/1188293/
 - add debugging output

Michael Walle (7):
  phy: atheros: fix AR8021 PHY ID mask
  phy: atheros: use defines for PHY IDs
  phy: atheros: introduce debug read and write functions
  phy: atheros: move delay config to common function
  phy: atheros: add device tree bindings and config
  phy: atheros: ar8035: remove static clock config
  phy: atheros: consolidate {ar8031|ar8035}_config()


Oh I've actually forgot the "Acked-by:" from Joe from the v2 series.

All but the

  phy: atheros: fix AR8021 PHY ID mask

were acked by Joe.

Tom, should I send a v4 where I drop that patch and could you pull
this series?

-michael



Vladimir Oltean (5):
  phy: atheros: Make RGMII Tx delays actually configurable for AR8035
  phy: atheros: Use common functions for RGMII internal delays
  phy: atheros: Clarify the configuration of the CLK_25M output pin
  phy: atheros: Explicitly disable RGMII delays
  phy: atheros: Clarify the intention of ar8021_config

 doc/device-tree-bindings/net/phy/atheros.txt |  35 ++
 drivers/net/phy/atheros.c| 350 ---
 include/dt-bindings/net/qca-ar803x.h |  13 +
 3 files changed, 345 insertions(+), 53 deletions(-)
 create mode 100644 doc/device-tree-bindings/net/phy/atheros.txt
 create mode 100644 include/dt-bindings/net/qca-ar803x.h


--
-michael


[PATCH v2 1/2] armv8: ls1028a: move FSL_LAYERSCAPE to kconfig

2020-05-06 Thread Michael Walle
CONFIG_FSL_LAYERSCAPE is available in kconfig. There is no need to
define it per board; the ls1028a_common.h is really board dependent and
only fits to the NXP eval boards. Instead select CONFIG_FSL_LAYERSCAPE
when ARCH_LS1028A is selected.

Signed-off-by: Michael Walle 
---
changes since v1:
  none

 arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 1 +
 include/configs/ls1028a_common.h  | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig 
b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
index 2f75b2cdd3..116b6b0617 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
+++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
@@ -23,6 +23,7 @@ config ARCH_LS1012A
 config ARCH_LS1028A
bool
select ARMV8_SET_SMPEN
+   select FSL_LAYERSCAPE
select FSL_LSCH3
select NXP_LSCH3_2
select SYS_FSL_HAS_CCI400
diff --git a/include/configs/ls1028a_common.h b/include/configs/ls1028a_common.h
index 6905694d10..971288f86e 100644
--- a/include/configs/ls1028a_common.h
+++ b/include/configs/ls1028a_common.h
@@ -7,7 +7,6 @@
 #define __L1028A_COMMON_H
 
 #define CONFIG_REMAKE_ELF
-#define CONFIG_FSL_LAYERSCAPE
 #define CONFIG_MP
 
 #include 
-- 
2.20.1



[PATCH v2 2/2] board: kontron: add sl28 support

2020-05-06 Thread Michael Walle
Add basic support for the Kontron SMARC-sAL28 board. This includes just
the bare minimum to be able to bring up the board and boot linux.

Also only one board variant is supported for now. That is the Single PHY
variant. Other variants will fall back to this one.

In particular, there is no watchdog support for now. This means that you
have to disable the default watchdog, otherwise you'll end up in the
recovery bootloader. See the board README for details.

Signed-off-by: Michael Walle 
---
changes since v1:
  - fix watchdog device tree reference

Sorry for the noise!

 arch/arm/Kconfig  |  10 ++
 arch/arm/dts/Makefile |   2 +
 .../arm/dts/fsl-ls1028a-kontron-sl28-var3.dts |  14 ++
 arch/arm/dts/fsl-ls1028a-kontron-sl28.dts | 128 ++
 board/kontron/sl28/Kconfig|  18 +++
 board/kontron/sl28/MAINTAINERS|   6 +
 board/kontron/sl28/Makefile   |   8 ++
 board/kontron/sl28/README |  63 +
 board/kontron/sl28/common.c   |  10 ++
 board/kontron/sl28/ddr.c  |  98 ++
 board/kontron/sl28/sl28.c |  90 
 board/kontron/sl28/spl.c  |   9 ++
 configs/kontron_sl28_defconfig| 102 ++
 include/configs/kontron_sl28.h|  85 
 14 files changed, 643 insertions(+)
 create mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28-var3.dts
 create mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28.dts
 create mode 100644 board/kontron/sl28/Kconfig
 create mode 100644 board/kontron/sl28/MAINTAINERS
 create mode 100644 board/kontron/sl28/Makefile
 create mode 100644 board/kontron/sl28/README
 create mode 100644 board/kontron/sl28/common.c
 create mode 100644 board/kontron/sl28/ddr.c
 create mode 100644 board/kontron/sl28/sl28.c
 create mode 100644 board/kontron/sl28/spl.c
 create mode 100644 configs/kontron_sl28_defconfig
 create mode 100644 include/configs/kontron_sl28.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b494bcae95..7201905c50 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1563,6 +1563,15 @@ config TARGET_LS1046AFRWY
  development platform that supports the QorIQ LS1046A
  Layerscape Architecture processor.
 
+config TARGET_SL28
+   bool "Support sl28"
+   select ARCH_LS1028A
+   select ARM64
+   select ARMV8_MULTIENTRY
+   select SUPPORT_SPL
+   help
+ Support for Kontron SMARC-sAL28 board.
+
 config TARGET_COLIBRI_PXA270
bool "Support colibri_pxa270"
select CPU_PXA
@@ -1893,6 +1902,7 @@ source "board/hisilicon/hikey/Kconfig"
 source "board/hisilicon/hikey960/Kconfig"
 source "board/hisilicon/poplar/Kconfig"
 source "board/isee/igep003x/Kconfig"
+source "board/kontron/sl28/Kconfig"
 source "board/phytec/pcm051/Kconfig"
 source "board/silica/pengwyn/Kconfig"
 source "board/spear/spear300/Kconfig"
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 559d3ab6a7..97e439a87f 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -379,6 +379,8 @@ dtb-$(CONFIG_FSL_LSCH3) += fsl-ls2080a-qds.dtb \
fsl-ls2088a-rdb-qspi.dtb \
fsl-ls1088a-rdb.dtb \
fsl-ls1088a-qds.dtb \
+   fsl-ls1028a-kontron-sl28.dtb \
+   fsl-ls1028a-kontron-sl28-var3.dtb \
fsl-ls1028a-rdb.dtb \
fsl-ls1028a-qds-duart.dtb \
fsl-ls1028a-qds-lpuart.dtb \
diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var3.dts 
b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var3.dts
new file mode 100644
index 00..23f89eadb3
--- /dev/null
+++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var3.dts
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Kontron SMARC-sAL28 device tree source (Single PHY version)
+ */
+
+/dts-v1/;
+
+#include "fsl-ls1028a-kontron-sl28.dts"
+
+/ {
+   u-boot,dm-pre-reloc;
+   model = "Kontron SMARC-sAL28 Board (Single PHY)";
+   compatible = "kontron,sl28-var3", "kontron,sl28", "fsl,ls1028a";
+};
diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28.dts 
b/arch/arm/dts/fsl-ls1028a-kontron-sl28.dts
new file mode 100644
index 00..8d4c1102c4
--- /dev/null
+++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28.dts
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Kontron SMARC-sAL28 device tree source (base version)
+ */
+
+/dts-v1/;
+
+#include "fsl-ls1028a.dtsi"
+
+/ {
+   u-boot,dm-pre-reloc;
+   model = "Kontron SMARC-sAL28 Board";
+   compatible = "kontron,sl28", "fsl,ls1028a";
+
+   aliases {
+   serial0 = 
+   serial1 = 
+   spi0 = 
+   spi1 = 
+   mmc0 = 
+   mmc1 = 
+   i2c0 = 
+   i2c1 = 
+   i2c2 = 
+   rtc0 = 
+   ethernet0 = 
+   ethernet1 = 
+   ethernet2 = 
+   ethernet3 = 
+   };
+

[PATCH v3 11/12] phy: atheros: ar8035: remove static clock config

2020-05-06 Thread Michael Walle
We can configure the clock output in the device tree. Disable the
hardcoded one in here. This is highly board-specific and should have
never been enabled in the PHY driver.

If bisecting shows that this commit breaks your board it probably
depends on the clock output of your Atheros AR8035 PHY. Please have a
look at doc/device-tree-bindings/net/phy/atheros.txt. You need to set
"clk-out-frequency = <12500>" because that value was the hardcoded
value until this commit.

Signed-off-by: Michael Walle 
---
 drivers/net/phy/atheros.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 3cd301c50e..3142733105 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -38,12 +38,6 @@
 #define AR803x_CLK_25M_62_5MHZ_DSP 5
 #define AR803x_CLK_25M_125MHZ_PLL  6
 #define AR803x_CLK_25M_125MHZ_DSP  7
-
-/* AR8035: Select frequency on CLK_25M pin through bits 4:3 */
-#define AR8035_CLK_25M_FREQ_25M(0 | 0)
-#define AR8035_CLK_25M_FREQ_50M(0 | BIT(3))
-#define AR8035_CLK_25M_FREQ_62M(BIT(4) | 0)
-#define AR8035_CLK_25M_FREQ_125M   (BIT(4) | BIT(3))
 #define AR8035_CLK_25M_MASKGENMASK(4, 3)
 
 #define AR803x_CLK_25M_DR_MASK GENMASK(8, 7)
@@ -344,18 +338,11 @@ static int ar8031_config(struct phy_device *phydev)
 static int ar8035_config(struct phy_device *phydev)
 {
int ret;
-   int regval;
 
ret = ar803x_of_init(phydev);
if (ret < 0)
return ret;
 
-   /* Configure CLK_25M output clock at 125 MHz */
-   regval = phy_read_mmd(phydev, MDIO_MMD_AN, AR803x_CLK_25M_SEL_REG);
-   regval &= ~AR8035_CLK_25M_MASK; /* No surprises */
-   regval |= AR8035_CLK_25M_FREQ_125M;
-   phy_write_mmd(phydev, MDIO_MMD_AN, AR803x_CLK_25M_SEL_REG, regval);
-
ret = ar803x_delay_config(phydev);
if (ret < 0)
return ret;
-- 
2.20.1



[PATCH v3 08/12] phy: atheros: introduce debug read and write functions

2020-05-06 Thread Michael Walle
Provide functions to read and write the Atheros debug registers.

Signed-off-by: Michael Walle 
---
 drivers/net/phy/atheros.c | 57 ---
 1 file changed, 41 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 5ff5875d3d..660dcd9491 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -30,32 +30,57 @@
 #define AR8031_PHY_ID 0x004dd074
 #define AR8035_PHY_ID 0x004dd072
 
-static void ar803x_enable_rx_delay(struct phy_device *phydev, bool on)
+static int ar803x_debug_reg_read(struct phy_device *phydev, u16 reg)
 {
-   int regval;
+   int ret;
+
+   ret = phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+   reg);
+   if (ret < 0)
+   return ret;
+
+   return phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
+}
+
+static int ar803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
+u16 clear, u16 set)
+{
+   int val;
+
+   val = ar803x_debug_reg_read(phydev, reg);
+   if (val < 0)
+   return val;
+
+   val &= 0x;
+   val &= ~clear;
+   val |= set;
+
+   return phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
+val);
+}
+
+static int ar803x_enable_rx_delay(struct phy_device *phydev, bool on)
+{
+   u16 clear = 0, set = 0;
 
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
- AR803x_DEBUG_REG_0);
-   regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
if (on)
-   regval |= AR803x_RGMII_RX_CLK_DLY;
+   set = AR803x_RGMII_RX_CLK_DLY;
else
-   regval &= ~AR803x_RGMII_RX_CLK_DLY;
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval);
+   clear = AR803x_RGMII_RX_CLK_DLY;
+
+   return ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_0, clear, set);
 }
 
-static void ar803x_enable_tx_delay(struct phy_device *phydev, bool on)
+static int ar803x_enable_tx_delay(struct phy_device *phydev, bool on)
 {
-   int regval;
+   u16 clear = 0, set = 0;
 
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
- AR803x_DEBUG_REG_5);
-   regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
if (on)
-   regval |= AR803x_RGMII_TX_CLK_DLY;
+   set = AR803x_RGMII_TX_CLK_DLY;
else
-   regval &= ~AR803x_RGMII_TX_CLK_DLY;
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval);
+   clear = AR803x_RGMII_TX_CLK_DLY;
+
+   return ar803x_debug_reg_mask(phydev, AR803x_DEBUG_REG_5, clear, set);
 }
 
 static int ar8021_config(struct phy_device *phydev)
-- 
2.20.1



[PATCH v3 07/12] phy: atheros: use defines for PHY IDs

2020-05-06 Thread Michael Walle
Signed-off-by: Michael Walle 
---
 drivers/net/phy/atheros.c | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 01953a1390..5ff5875d3d 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -26,6 +26,10 @@
 #define AR8035_CLK_25M_FREQ_125M   (BIT(4) | BIT(3))
 #define AR8035_CLK_25M_MASKGENMASK(4, 3)
 
+#define AR8021_PHY_ID 0x004dd040
+#define AR8031_PHY_ID 0x004dd074
+#define AR8035_PHY_ID 0x004dd072
+
 static void ar803x_enable_rx_delay(struct phy_device *phydev, bool on)
 {
int regval;
@@ -119,7 +123,7 @@ static int ar8035_config(struct phy_device *phydev)
 
 static struct phy_driver AR8021_driver =  {
.name = "AR8021",
-   .uid = 0x4dd040,
+   .uid = AR8021_PHY_ID,
.mask = 0xfff0,
.features = PHY_GBIT_FEATURES,
.config = ar8021_config,
@@ -129,7 +133,7 @@ static struct phy_driver AR8021_driver =  {
 
 static struct phy_driver AR8031_driver =  {
.name = "AR8031/AR8033",
-   .uid = 0x4dd074,
+   .uid = AR8031_PHY_ID,
.mask = 0xffef,
.features = PHY_GBIT_FEATURES,
.config = ar8031_config,
@@ -139,7 +143,7 @@ static struct phy_driver AR8031_driver =  {
 
 static struct phy_driver AR8035_driver =  {
.name = "AR8035",
-   .uid = 0x4dd072,
+   .uid = AR8035_PHY_ID,
.mask = 0xffef,
.features = PHY_GBIT_FEATURES,
.config = ar8035_config,
-- 
2.20.1



[PATCH v3 09/12] phy: atheros: move delay config to common function

2020-05-06 Thread Michael Walle
Signed-off-by: Michael Walle 
---
 drivers/net/phy/atheros.c | 38 ++
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 660dcd9491..22035c2496 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -94,19 +94,32 @@ static int ar8021_config(struct phy_device *phydev)
return 0;
 }
 
-static int ar8031_config(struct phy_device *phydev)
+static int ar803x_delay_config(struct phy_device *phydev)
 {
+   int ret;
+
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
-   ar803x_enable_tx_delay(phydev, true);
+   ret = ar803x_enable_tx_delay(phydev, true);
else
-   ar803x_enable_tx_delay(phydev, false);
+   ret = ar803x_enable_tx_delay(phydev, false);
 
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
-   ar803x_enable_rx_delay(phydev, true);
+   ret = ar803x_enable_rx_delay(phydev, true);
else
-   ar803x_enable_rx_delay(phydev, false);
+   ret = ar803x_enable_rx_delay(phydev, false);
+
+   return ret;
+}
+
+static int ar8031_config(struct phy_device *phydev)
+{
+   int ret;
+
+   ret = ar803x_delay_config(phydev);
+   if (ret < 0)
+   return ret;
 
phydev->supported = phydev->drv->features;
 
@@ -118,6 +131,7 @@ static int ar8031_config(struct phy_device *phydev)
 
 static int ar8035_config(struct phy_device *phydev)
 {
+   int ret;
int regval;
 
/* Configure CLK_25M output clock at 125 MHz */
@@ -126,17 +140,9 @@ static int ar8035_config(struct phy_device *phydev)
regval |= AR8035_CLK_25M_FREQ_125M;
phy_write_mmd(phydev, MDIO_MMD_AN, AR803x_CLK_25M_SEL_REG, regval);
 
-   if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
-   ar803x_enable_tx_delay(phydev, true);
-   else
-   ar803x_enable_tx_delay(phydev, false);
-
-   if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID))
-   ar803x_enable_rx_delay(phydev, true);
-   else
-   ar803x_enable_rx_delay(phydev, false);
+   ret = ar803x_delay_config(phydev);
+   if (ret < 0)
+   return ret;
 
phydev->supported = phydev->drv->features;
 
-- 
2.20.1



[PATCH v3 12/12] phy: atheros: consolidate {ar8031|ar8035}_config()

2020-05-06 Thread Michael Walle
The two functions are now exactly the same, remove one of them.

Signed-off-by: Michael Walle 
---
 drivers/net/phy/atheros.c | 30 +++---
 1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 3142733105..47ff9f8d44 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -311,31 +311,7 @@ static int ar803x_of_init(struct phy_device *phydev)
return 0;
 }
 
-static int ar8031_config(struct phy_device *phydev)
-{
-   int ret;
-
-   ret = ar803x_of_init(phydev);
-   if (ret < 0)
-   return ret;
-
-   ret = ar803x_delay_config(phydev);
-   if (ret < 0)
-   return ret;
-
-   ret = ar803x_regs_config(phydev);
-   if (ret < 0)
-   return ret;
-
-   phydev->supported = phydev->drv->features;
-
-   genphy_config_aneg(phydev);
-   genphy_restart_aneg(phydev);
-
-   return 0;
-}
-
-static int ar8035_config(struct phy_device *phydev)
+static int ar803x_config(struct phy_device *phydev)
 {
int ret;
 
@@ -374,7 +350,7 @@ static struct phy_driver AR8031_driver =  {
.uid = AR8031_PHY_ID,
.mask = 0xffef,
.features = PHY_GBIT_FEATURES,
-   .config = ar8031_config,
+   .config = ar803x_config,
.startup = genphy_startup,
.shutdown = genphy_shutdown,
 };
@@ -384,7 +360,7 @@ static struct phy_driver AR8035_driver =  {
.uid = AR8035_PHY_ID,
.mask = 0xffef,
.features = PHY_GBIT_FEATURES,
-   .config = ar8035_config,
+   .config = ar803x_config,
.startup = genphy_startup,
.shutdown = genphy_shutdown,
 };
-- 
2.20.1



[PATCH v3 10/12] phy: atheros: add device tree bindings and config

2020-05-06 Thread Michael Walle
Add support for configuring the CLK_25M pin as well as the RGMII I/O
voltage by the device tree.

By default the AT803x PHYs outputs the 25MHz clock of the XTAL input.
But this output can also be changed by software to other frequencies.
This commit introduces a generic way to configure this output.

Also the PHY supports different RGMII I/O voltages: 1.5V, 1.8V and 2.5V.
An internal LDO is able to provide 1.5V (default) and 1.8V. The 2.5V
option needs an external supply voltage. This commit adds support to
switch the internal LDO to 1.8V.

Signed-off-by: Michael Walle 
---
 doc/device-tree-bindings/net/phy/atheros.txt |  35 +++
 drivers/net/phy/atheros.c| 224 ++-
 include/dt-bindings/net/qca-ar803x.h |  13 ++
 3 files changed, 270 insertions(+), 2 deletions(-)
 create mode 100644 doc/device-tree-bindings/net/phy/atheros.txt
 create mode 100644 include/dt-bindings/net/qca-ar803x.h

diff --git a/doc/device-tree-bindings/net/phy/atheros.txt 
b/doc/device-tree-bindings/net/phy/atheros.txt
new file mode 100644
index 00..97e97b8c13
--- /dev/null
+++ b/doc/device-tree-bindings/net/phy/atheros.txt
@@ -0,0 +1,35 @@
+* Qualcomm Atheros PHY Device Tree binding
+
+Required properties:
+- reg: PHY address
+
+Optional properties:
+- qca,clk-out-frequency: Clock frequency of the CLK_25M pin in Hz.
+   Either 2500, 5000, 6250 or 12500.
+- qca,clk-out-strength: Clock output buffer driver strength.
+Supported values are defined in dt-bindings/net/qca-ar803x.h
+- qca,keep-pll-enabled: Keep the PLL running if no link is present.
+   Don't go into hibernation mode.
+   Only supported on the AR8031/AR8033.
+- vddio-supply: RGMII I/O voltage regulator
+   Only supported on the AR8031/AR8033.
+
+Optional subnodes:
+- vddio-regulator: Initial data for the VDDIO regulator, as covered
+doc/device-tree-bindings/regulator/regulator.txt
+
+Example:
+   #include 
+
+   ethernet-phy@0 {
+   reg = <0>;
+   qca-clk-out-frequency = <12500>;
+   qca,keep-pll-enabled;
+
+   vddio-supply = <>;
+
+   vddio: vddio-regulator {
+   regulator-min-microvolt = <180>;
+   regulator-max-microvolt = <180>;
+   };
+   };
diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 22035c2496..3cd301c50e 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -4,21 +4,41 @@
  *
  * Copyright 2011, 2013 Freescale Semiconductor, Inc.
  * author Andy Fleming
+ * Copyright (c) 2019 Michael Walle 
  */
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #define AR803x_PHY_DEBUG_ADDR_REG  0x1d
 #define AR803x_PHY_DEBUG_DATA_REG  0x1e
 
+/* Debug registers */
+#define AR803x_DEBUG_REG_0 0x0
+#define AR803x_RGMII_RX_CLK_DLYBIT(15)
+
 #define AR803x_DEBUG_REG_5 0x5
 #define AR803x_RGMII_TX_CLK_DLYBIT(8)
 
-#define AR803x_DEBUG_REG_0 0x0
-#define AR803x_RGMII_RX_CLK_DLYBIT(15)
+#define AR803x_DEBUG_REG_1F0x1f
+#define AR803x_PLL_ON  BIT(2)
+#define AR803x_RGMII_1V8   BIT(3)
 
 /* CLK_25M register is at MMD 7, address 0x8016 */
 #define AR803x_CLK_25M_SEL_REG 0x8016
+
+#define AR803x_CLK_25M_MASKGENMASK(4, 2)
+#define AR803x_CLK_25M_25MHZ_XTAL  0
+#define AR803x_CLK_25M_25MHZ_DSP   1
+#define AR803x_CLK_25M_50MHZ_PLL   2
+#define AR803x_CLK_25M_50MHZ_DSP   3
+#define AR803x_CLK_25M_62_5MHZ_PLL 4
+#define AR803x_CLK_25M_62_5MHZ_DSP 5
+#define AR803x_CLK_25M_125MHZ_PLL  6
+#define AR803x_CLK_25M_125MHZ_DSP  7
+
 /* AR8035: Select frequency on CLK_25M pin through bits 4:3 */
 #define AR8035_CLK_25M_FREQ_25M(0 | 0)
 #define AR8035_CLK_25M_FREQ_50M(0 | BIT(3))
@@ -26,10 +46,23 @@
 #define AR8035_CLK_25M_FREQ_125M   (BIT(4) | BIT(3))
 #define AR8035_CLK_25M_MASKGENMASK(4, 3)
 
+#define AR803x_CLK_25M_DR_MASK GENMASK(8, 7)
+#define AR803x_CLK_25M_DR_FULL 0
+#define AR803x_CLK_25M_DR_HALF 1
+#define AR803x_CLK_25M_DR_QUARTER  2
+
 #define AR8021_PHY_ID 0x004dd040
 #define AR8031_PHY_ID 0x004dd074
 #define AR8035_PHY_ID 0x004dd072
 
+struct ar803x_priv {
+   int flags;
+#define AR803x_FLAG_KEEP_PLL_ENABLED   BIT(0) /* don't turn off internal PLL */
+#define AR803x_FLAG_RGMII_1V8  BIT(1) /* use 1.8V RGMII I/O voltage */
+   u16 clk_25m_reg;
+   u16 clk_25m_mask;
+};
+
 static int ar803x_debug_reg_read(struct phy_device *phydev, u16 reg)
 {
int ret;
@@ -113,14 +146,193 @@ static int ar803x_delay_config(struct phy_device *phydev)
return ret;
 }
 
+static int ar803x_regs_config(struct phy_device *phydev)
+{
+   struct ar803x_priv *priv = phydev->priv;
+   u16 set = 0, clear = 0;
+   int val;
+   int ret;
+
+ 

[PATCH v3 03/12] phy: atheros: Clarify the configuration of the CLK_25M output pin

2020-05-06 Thread Michael Walle
From: Vladimir Oltean 

Also take the opportunity to use the phy_read_mmd and phy_write_mmd
convenience functions.

Signed-off-by: Vladimir Oltean 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index c0c2b4db39..1da18eb5d4 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -17,6 +17,15 @@
 #define AR803x_DEBUG_REG_0 0x0
 #define AR803x_RGMII_RX_CLK_DLYBIT(15)
 
+/* CLK_25M register is at MMD 7, address 0x8016 */
+#define AR803x_CLK_25M_SEL_REG 0x8016
+/* AR8035: Select frequency on CLK_25M pin through bits 4:3 */
+#define AR8035_CLK_25M_FREQ_25M(0 | 0)
+#define AR8035_CLK_25M_FREQ_50M(0 | BIT(3))
+#define AR8035_CLK_25M_FREQ_62M(BIT(4) | 0)
+#define AR8035_CLK_25M_FREQ_125M   (BIT(4) | BIT(3))
+#define AR8035_CLK_25M_MASKGENMASK(4, 3)
+
 static void ar803x_enable_rx_delay(struct phy_device *phydev, bool on)
 {
int regval;
@@ -78,11 +87,11 @@ static int ar8035_config(struct phy_device *phydev)
 {
int regval;
 
-   phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x0007);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0xe, 0x8016);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0xd, 0x4007);
-   regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
+   /* Configure CLK_25M output clock at 125 MHz */
+   regval = phy_read_mmd(phydev, MDIO_MMD_AN, AR803x_CLK_25M_SEL_REG);
+   regval &= ~AR8035_CLK_25M_MASK; /* No surprises */
+   regval |= AR8035_CLK_25M_FREQ_125M;
+   phy_write_mmd(phydev, MDIO_MMD_AN, AR803x_CLK_25M_SEL_REG, regval);
 
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
-- 
2.20.1



[PATCH v3 05/12] phy: atheros: Clarify the intention of ar8021_config

2020-05-06 Thread Michael Walle
From: Vladimir Oltean 

Debug register 5 contains TX_CLK DELAY at bit 8 and reserved values at
the other bit positions, just like the other PHYs in the family do.
Therefore, it is not necessary to hardcode the reserved values, but
instead simply follow the read-modify-write procedure from the common
function.

Signed-off-by: Vladimir Oltean 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 3e59c3f391..3cc162828c 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -56,10 +56,10 @@ static void ar803x_enable_tx_delay(struct phy_device 
*phydev, bool on)
 
 static int ar8021_config(struct phy_device *phydev)
 {
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200);
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
- AR803x_DEBUG_REG_5);
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, 0x3D47);
+   phy_write(phydev, MDIO_DEVAD_NONE, MII_BMCR,
+ BMCR_ANENABLE | BMCR_ANRESTART);
+
+   ar803x_enable_tx_delay(phydev, true);
 
phydev->supported = phydev->drv->features;
return 0;
-- 
2.20.1



[PATCH v3 02/12] phy: atheros: Use common functions for RGMII internal delays

2020-05-06 Thread Michael Walle
From: Vladimir Oltean 

Signed-off-by: Vladimir Oltean 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 69 +++
 1 file changed, 41 insertions(+), 28 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 537c1a9125..c0c2b4db39 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -12,16 +12,45 @@
 #define AR803x_PHY_DEBUG_DATA_REG  0x1e
 
 #define AR803x_DEBUG_REG_5 0x5
-#define AR803x_RGMII_TX_CLK_DLY0x100
+#define AR803x_RGMII_TX_CLK_DLYBIT(8)
 
 #define AR803x_DEBUG_REG_0 0x0
-#define AR803x_RGMII_RX_CLK_DLY0x8000
+#define AR803x_RGMII_RX_CLK_DLYBIT(15)
+
+static void ar803x_enable_rx_delay(struct phy_device *phydev, bool on)
+{
+   int regval;
+
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+ AR803x_DEBUG_REG_0);
+   regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
+   if (on)
+   regval |= AR803x_RGMII_RX_CLK_DLY;
+   else
+   regval &= ~AR803x_RGMII_RX_CLK_DLY;
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval);
+}
+
+static void ar803x_enable_tx_delay(struct phy_device *phydev, bool on)
+{
+   int regval;
+
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+ AR803x_DEBUG_REG_5);
+   regval = phy_read(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG);
+   if (on)
+   regval |= AR803x_RGMII_TX_CLK_DLY;
+   else
+   regval &= ~AR803x_RGMII_TX_CLK_DLY;
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, regval);
+}
 
 static int ar8021_config(struct phy_device *phydev)
 {
phy_write(phydev, MDIO_DEVAD_NONE, 0x00, 0x1200);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, 0x3D47);
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
+ AR803x_DEBUG_REG_5);
+   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG, 0x3D47);
 
phydev->supported = phydev->drv->features;
return 0;
@@ -30,20 +59,12 @@ static int ar8021_config(struct phy_device *phydev)
 static int ar8031_config(struct phy_device *phydev)
 {
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
-   phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
- AR803x_DEBUG_REG_5);
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
- AR803x_RGMII_TX_CLK_DLY);
-   }
+   phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+   ar803x_enable_tx_delay(phydev, true);
 
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
-   phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_ADDR_REG,
- AR803x_DEBUG_REG_0);
-   phy_write(phydev, MDIO_DEVAD_NONE, AR803x_PHY_DEBUG_DATA_REG,
- AR803x_RGMII_RX_CLK_DLY);
-   }
+   phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
+   ar803x_enable_rx_delay(phydev, true);
 
phydev->supported = phydev->drv->features;
 
@@ -64,20 +85,12 @@ static int ar8035_config(struct phy_device *phydev)
phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
 
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
-   /* select debug reg 5 */
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1D, 0x5);
-   /* enable tx delay */
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1E, 0x0100);
-   }
+   (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
+   ar803x_enable_tx_delay(phydev, true);
 
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
-   (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)) {
-   /* select debug reg 0 */
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1D, 0x0);
-   /* enable rx delay */
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1E, 0x8000);
-   }
+   (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID))
+   ar803x_enable_rx_delay(phydev, true);
 
phydev->supported = phydev->drv->features;
 
-- 
2.20.1



[PATCH v3 04/12] phy: atheros: Explicitly disable RGMII delays

2020-05-06 Thread Michael Walle
From: Vladimir Oltean 

To eliminate any doubts about the out-of-reset value of the PHY, that
the driver previously relied on.

If bisecting shows that this commit breaks your board you probably have
a wrong PHY interface mode. You probably want the
PHY_INTERFACE_MODE_RGMII_RXID or PHY_INTERFACE_MODE_RGMII_ID mode.

Signed-off-by: Vladimir Oltean 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 1da18eb5d4..3e59c3f391 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -70,10 +70,14 @@ static int ar8031_config(struct phy_device *phydev)
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
ar803x_enable_tx_delay(phydev, true);
+   else
+   ar803x_enable_tx_delay(phydev, false);
 
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
ar803x_enable_rx_delay(phydev, true);
+   else
+   ar803x_enable_rx_delay(phydev, false);
 
phydev->supported = phydev->drv->features;
 
@@ -96,10 +100,14 @@ static int ar8035_config(struct phy_device *phydev)
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID))
ar803x_enable_tx_delay(phydev, true);
+   else
+   ar803x_enable_tx_delay(phydev, false);
 
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID))
ar803x_enable_rx_delay(phydev, true);
+   else
+   ar803x_enable_rx_delay(phydev, false);
 
phydev->supported = phydev->drv->features;
 
-- 
2.20.1



[PATCH v3 00/12] phy: atheros: dt bindings and cleanup

2020-05-06 Thread Michael Walle
This patch series superseeds the following two:
>From Vladimir Oltean
  https://patchwork.ozlabs.org/cover/1031360/
>From me:
  https://patchwork.ozlabs.org/cover/1184507/

Although the first is marked as accepted into u-boot-net I guess it was
removed due to broken boards ("DT as ABI", RGMII delay was fixed and thus
breaks the board).

After disussing with Vladimir, I've integrated his patches with this
series. Also the first one
  Address packet drops at low traffic rate due to SmartEEE feature
was dropped because it will likely be fixed by making u-boot support the
eee-broken-X device tree properties. Apart from that, only the subject was
changed and a note about possible board breakage was added the patch which
changes the delay behaviour.

For all of those, who will test this patchset, the device tree binding
needs the phydev->node property, which needs to be set in every network
driver. If the device tree binding is not working for you have a look at
the
  ar803x_of_init: found PHY node: phy@0
output. In the case above "phy@0" is the phy node in the device tree. If
instead the node of your network device is displayed, you have to set
the phydev->node property in your network device driver.

For the fsl_enetc driver this patchset will add it:
  https://patchwork.ozlabs.org/cover/1188043/

changes since v2:
 - rebased onto latest master, esp. #include 

changes since v1:
 - pull all Vladimirs Oltan's patches and rebase mine onto them
 - fix the CLK_25M settings for the AR8035
 - add two new patches "fix AR8021 PHY ID mask" and "use defines for PHY
   IDs"
 - use the new kernel device tree binding for the AR803x PHYs:
   https://patchwork.ozlabs.org/patch/1188293/
 - add debugging output

Michael Walle (7):
  phy: atheros: fix AR8021 PHY ID mask
  phy: atheros: use defines for PHY IDs
  phy: atheros: introduce debug read and write functions
  phy: atheros: move delay config to common function
  phy: atheros: add device tree bindings and config
  phy: atheros: ar8035: remove static clock config
  phy: atheros: consolidate {ar8031|ar8035}_config()

Vladimir Oltean (5):
  phy: atheros: Make RGMII Tx delays actually configurable for AR8035
  phy: atheros: Use common functions for RGMII internal delays
  phy: atheros: Clarify the configuration of the CLK_25M output pin
  phy: atheros: Explicitly disable RGMII delays
  phy: atheros: Clarify the intention of ar8021_config

 doc/device-tree-bindings/net/phy/atheros.txt |  35 ++
 drivers/net/phy/atheros.c| 350 ---
 include/dt-bindings/net/qca-ar803x.h |  13 +
 3 files changed, 345 insertions(+), 53 deletions(-)
 create mode 100644 doc/device-tree-bindings/net/phy/atheros.txt
 create mode 100644 include/dt-bindings/net/qca-ar803x.h

-- 
2.20.1



[PATCH v3 06/12] phy: atheros: fix AR8021 PHY ID mask

2020-05-06 Thread Michael Walle
The upper bits are all the OUI.

Signed-off-by: Michael Walle 
---
 drivers/net/phy/atheros.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 3cc162828c..01953a1390 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -120,7 +120,7 @@ static int ar8035_config(struct phy_device *phydev)
 static struct phy_driver AR8021_driver =  {
.name = "AR8021",
.uid = 0x4dd040,
-   .mask = 0x40,
+   .mask = 0xfff0,
.features = PHY_GBIT_FEATURES,
.config = ar8021_config,
.startup = genphy_startup,
-- 
2.20.1



[PATCH v3 01/12] phy: atheros: Make RGMII Tx delays actually configurable for AR8035

2020-05-06 Thread Michael Walle
From: Vladimir Oltean 

Delete the extraneous write to debug reg 5 that enables Tx delay

When the driver was originally introduced in commit "6027384a phylib:
Add Atheros AR8035 GETH PHY support", the Tx delay was being
unconditionally enabled.

Then during "2ec4d10b phy: atheros: add support for RGMII_ID, RGMII_TXID
and RGMII_RXID", the author did not notice that code for enabling Tx
delay code was already. Therefore, the if condition for Tx delay has
always been useless for this PHY since this commit introduced it.

Prior to this patch, every AR8035 PHY in U-boot had Tx delay enabled.
After this patch, only those who define the interface as RGMII_TXID or
RGMII_ID will. This is to be expected, but will nonetheless break the
setups of those who didn't know they rely on Tx delay implicitly.

Signed-off-by: Vladimir Oltean 
Acked-by: Joe Hershberger 
---
 drivers/net/phy/atheros.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index 3783d155e7..537c1a9125 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -63,10 +63,6 @@ static int ar8035_config(struct phy_device *phydev)
regval = phy_read(phydev, MDIO_DEVAD_NONE, 0xe);
phy_write(phydev, MDIO_DEVAD_NONE, 0xe, (regval|0x0018));
 
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1d, 0x05);
-   regval = phy_read(phydev, MDIO_DEVAD_NONE, 0x1e);
-   phy_write(phydev, MDIO_DEVAD_NONE, 0x1e, (regval|0x0100));
-
if ((phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) ||
(phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID)) {
/* select debug reg 5 */
-- 
2.20.1



[PATCH 3/6] efi_loader: Implement EFI variable handling via OP-TEE

2020-05-06 Thread Ilias Apalodimas
In OP-TEE we can run EDK2's StandAloneMM on a secure partition.
StandAloneMM is responsible for the UEFI variable support. In
combination with OP-TEE and it's U-Boot supplicant, variables are
authenticated/validated in secure world and stored on an RPMB partition.

So let's add a new config option in U-Boot implementing the necessary
calls to OP-TEE for the variable management.

Signed-off-by: Ilias Apalodimas 
Signed-off-by: Pipat Methavanitpong 
Signed-off-by: Sughosh Ganu 
---
 lib/efi_loader/Kconfig|   9 +
 lib/efi_loader/Makefile   |   4 +
 lib/efi_loader/efi_variable_tee.c | 645 ++
 3 files changed, 658 insertions(+)
 create mode 100644 lib/efi_loader/efi_variable_tee.c

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 1cfa24ffcf72..e385cd0b9dae 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -164,4 +164,13 @@ config EFI_SECURE_BOOT
  it is signed with a trusted key. To do that, you need to install,
  at least, PK, KEK and db.
 
+config EFI_MM_COMM_TEE
+   bool "UEFI variables storage service via OP-TEE"
+   depends on SUPPORT_EMMC_RPMB
+   default n
+   help
+ If OP-TEE is present and running StandAloneMM dispatch all UEFI 
variable
+ related operations to that. The application will verify, authenticate 
and
+ store the variables on an RPMB
+
 endif
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index eff3c25ec301..dba652121eab 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -34,7 +34,11 @@ obj-y += efi_root_node.o
 obj-y += efi_runtime.o
 obj-y += efi_setup.o
 obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o
+ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
+obj-y += efi_variable_tee.o
+else
 obj-y += efi_variable.o
+endif
 obj-y += efi_watchdog.o
 obj-$(CONFIG_LCD) += efi_gop.o
 obj-$(CONFIG_DM_VIDEO) += efi_gop.o
diff --git a/lib/efi_loader/efi_variable_tee.c 
b/lib/efi_loader/efi_variable_tee.c
new file mode 100644
index ..d4e206e22073
--- /dev/null
+++ b/lib/efi_loader/efi_variable_tee.c
@@ -0,0 +1,645 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  EFI variable service via OP-TEE
+ *
+ *  Copyright (C) 2019 Linaro Ltd. 
+ *  Copyright (C) 2019 Linaro Ltd. 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define PTA_STMM_CMDID_COMMUNICATE 0
+#define PTA_STMM_UUID { 0xed32d533, 0x99e6, 0x4209, {\
+   0x9c, 0xc0, 0x2d, 0x72, 0xcd, 0xd9, 0x98, 0xa7 } }
+
+#define EFI_MM_VARIABLE_GUID \
+   EFI_GUID(0xed32d533, 0x99e6, 0x4209, \
+0x9c, 0xc0, 0x2d, 0x72, 0xcd, 0xd9, 0x98, 0xa7)
+
+static efi_uintn_t max_buffer_size;/* comm + var + func + data */
+static efi_uintn_t max_payload_size;   /* func + data */
+
+struct mm_connection {
+   struct udevice *tee;
+   u32 session;
+};
+
+int get_connection(struct mm_connection *conn)
+{
+   static const struct tee_optee_ta_uuid uuid = PTA_STMM_UUID;
+   struct udevice *tee = NULL;
+   struct tee_open_session_arg arg;
+   int rc;
+
+   tee = tee_find_device(tee, NULL, NULL, NULL);
+   if (!tee)
+   return -ENODEV;
+
+   memset(, 0, sizeof(arg));
+   tee_optee_ta_uuid_to_octets(arg.uuid, );
+   rc = tee_open_session(tee, , 0, NULL);
+   if (!rc) {
+   conn->tee = tee;
+   conn->session = arg.session;
+   }
+
+   return rc;
+}
+
+/**
+ * optee_mm_communicate() - Pass a buffer to StandaloneMM running in OP-TEE
+ *
+ * @comm_buf:  Locally allocted communcation buffer
+ * @dsize: Buffer size
+ * Return: status code
+ */
+static efi_status_t optee_mm_communicate(void *comm_buf, ulong dsize)
+{
+   ulong buf_size;
+   efi_status_t ret;
+   struct mm_communicate_header *mm_hdr;
+   struct mm_connection conn = { NULL, 0 };
+   struct tee_invoke_arg arg;
+   struct tee_param param[2];
+   struct tee_shm *shm = NULL;
+   int rc;
+
+   if (!comm_buf)
+   return EFI_INVALID_PARAMETER;
+
+   mm_hdr = (struct mm_communicate_header *)comm_buf;
+   buf_size = mm_hdr->message_len + sizeof(efi_guid_t) + sizeof(size_t);
+
+   if (dsize != buf_size)
+   return EFI_INVALID_PARAMETER;
+
+   rc = get_connection();
+   if (rc) {
+   log_err("Unable to open OP-TEE session (err=%d)\n", rc);
+   return EFI_UNSUPPORTED;
+   }
+
+   if (tee_shm_register(conn.tee, comm_buf, buf_size, 0, )) {
+   log_err("Unable to register shared memory\n");
+   return EFI_UNSUPPORTED;
+   }
+
+   memset(, 0, sizeof(arg));
+   arg.func = PTA_STMM_CMDID_COMMUNICATE;
+   arg.session = conn.session;
+
+   memset(param, 0, sizeof(param));
+   param[0].attr = TEE_PARAM_ATTR_TYPE_MEMREF_INOUT;
+   param[0].u.memref.size = buf_size;
+   

[PATCH 6/6] doc: uefi.rst: Add OP-TEE variable storage config options

2020-05-06 Thread Ilias Apalodimas
If OP-TEE is compiled with an EDK2 application running in secure world
it can process and store UEFI variables in an RPMB.
Add documentation for the config options enabling this

Signed-off-by: Ilias Apalodimas 
---
 doc/uefi/uefi.rst | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/doc/uefi/uefi.rst b/doc/uefi/uefi.rst
index 4fda00d68721..93b0faadd26e 100644
--- a/doc/uefi/uefi.rst
+++ b/doc/uefi/uefi.rst
@@ -188,6 +188,16 @@ on the sandbox
 cd 
 pytest.py test/py/tests/test_efi_secboot/test_signed.py --bd sandbox
 
+Using OP-TEE for EFI variables
+~~
+
+If an RPMB and it's drivers is available in U-Boot, OP-TEE can be used for
+variable services.
+Enabling CONFIG_EFI_MM_COMM_TEE=y will dispatch the variables services to
+OP-TEE. OP-TEE needs to be compiled with a secure application (coming from 
EDK2)
+which will process variables in the Secure World and store them in the RPMB
+using the OP-TEE supplicant.
+
 Executing the boot manager
 ~~
 
-- 
2.26.2



[PATCH 2/6] efi_loader: Add headers for EDK2 StandAloneMM communication

2020-05-06 Thread Ilias Apalodimas
From: Sughosh Ganu 

In Arm devices OP-TEE has the ability to run StandAloneMM (from EDK2)
in a separate partition and handle UEFI variables.
A following patch introduces this functionality.

Add the headers needed for OP-TEE <--> StandAloneMM communication

Signed-off-by: Sughosh Ganu 
Signed-off-by: Ilias Apalodimas 
---
 include/mm_communication.h | 28 ++
 include/mm_variable.h  | 78 ++
 2 files changed, 106 insertions(+)
 create mode 100644 include/mm_communication.h
 create mode 100644 include/mm_variable.h

diff --git a/include/mm_communication.h b/include/mm_communication.h
new file mode 100644
index ..fb4c91103400
--- /dev/null
+++ b/include/mm_communication.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ *  Headers for EFI variable service via StandAloneMM, EDK2 application running
+ *  in OP-TEE
+ *
+ *  Copyright (C) 2020 Linaro Ltd. 
+ *  Copyright (C) 2020 Linaro Ltd. 
+ */
+
+#if !defined _MM_COMMUNICATION_H_
+#define _MM_COMMUNICATION_H_
+
+/* defined in EDK2 MmCommunication.h */
+struct mm_communicate_header {
+   efi_guid_t header_guid;
+   size_t message_len;
+   u8 data[1];
+};
+
+#define MM_COMMUNICATE_HEADER_SIZE \
+   (offsetof(struct mm_communicate_header, data))
+
+#define MM_RET_SUCCESS 0
+#define MM_RET_INVALID_PARAMS -2
+#define MM_RET_DENIED -3
+#define MM_RET_NO_MEMORY  -4
+
+#endif /* _MM_COMMUNICATION_H_*/
diff --git a/include/mm_variable.h b/include/mm_variable.h
new file mode 100644
index ..f56c52597629
--- /dev/null
+++ b/include/mm_variable.h
@@ -0,0 +1,78 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ *  Headers for EFI variable service via StandAloneMM, EDK2 application running
+ *  in OP-TEE
+ *
+ *  Copyright (C) 2020 Linaro Ltd. 
+ *  Copyright (C) 2020 Linaro Ltd. 
+ */
+
+#if !defined _MM_VARIABLE_H_
+#define _MM_VARIABLE_H_
+
+#include 
+
+/* defined in EDK2 SmmVariableCommon.h */
+struct mm_variable_communicate_header {
+   efi_uintn_t  function;
+   efi_status_t ret_status;
+   u8   data[1];
+};
+
+#define MM_VARIABLE_COMMUNICATE_SIZE \
+   (offsetof(struct mm_variable_communicate_header, data))
+
+#define MM_VARIABLE_FUNCTION_GET_VARIABLE  1
+
+#define MM_VARIABLE_FUNCTION_GET_NEXT_VARIABLE_NAME2
+
+#define MM_VARIABLE_FUNCTION_SET_VARIABLE  3
+
+#define MM_VARIABLE_FUNCTION_QUERY_VARIABLE_INFO   4
+
+#define MM_VARIABLE_FUNCTION_READY_TO_BOOT 5
+
+#define MM_VARIABLE_FUNCTION_EXIT_BOOT_SERVICE 6
+
+#define MM_VARIABLE_FUNCTION_GET_STATISTICS7
+
+#define MM_VARIABLE_FUNCTION_LOCK_VARIABLE 8
+
+#define MM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_SET   9
+
+#define MM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET   10
+
+#define MM_VARIABLE_FUNCTION_GET_PAYLOAD_SIZE  11
+
+struct mm_variable_access {
+   efi_guid_t  guid;
+   efi_uintn_t data_size;
+   efi_uintn_t name_size;
+   u32 attr;
+   u16 name[1];
+};
+
+#define MM_VARIABLE_ACCESS_HEADER_SIZE \
+   (offsetof(struct mm_variable_access, name))
+
+struct mm_variable_payload_size {
+   efi_uintn_t size;
+};
+
+struct mm_variable_getnext {
+   efi_guid_t  guid;
+   efi_uintn_t name_size;
+   u16 name[1];
+};
+
+#define MM_VARIABLE_GET_NEXT_HEADER_SIZE \
+   (offsetof(struct mm_variable_getnext, name))
+
+struct mm_variable_query_info {
+   u64 max_variable_storage;
+   u64 remaining_variable_storage;
+   u64 max_variable_size;
+   u32 attr;
+};
+
+#endif /* _MM_VARIABLE_H_ */
-- 
2.26.2



[PATCH 4/6] cmd: efidebug: Add support for querying UEFI variable storage

2020-05-06 Thread Ilias Apalodimas
With the previous patches that use OP-TEE and StandAloneMM for UEFI
variable storage we've added functionality for efi_query_variable_info.
So let's add the relevant command to efidebug and retrieve information
about the container used to store UEFI variables

Signed-off-by: Ilias Apalodimas 
---
 cmd/efidebug.c | 45 -
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index d8a76d78a388..17e36ef76d69 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -1160,6 +1160,45 @@ static int do_efi_test(cmd_tbl_t *cmdtp, int flag,
return cp->cmd(cmdtp, flag, argc, argv);
 }
 
+/**
+ * do_efi_query_info() - QueryVariableInfo EFI service
+ *
+ * @cmdtp: Command table
+ * @flag:  Command flag
+ * @argc:  Number of arguments
+ * @argv:  Argument array
+ * Return: CMD_RET_SUCCESS on success,
+ * CMD_RET_USAGE or CMD_RET_FAILURE on failure
+ *
+ * Implement efidebug "test" sub-command.
+ */
+
+static int do_efi_query_info(cmd_tbl_t *cmdtp, int flag,
+int argc, char * const argv[])
+{
+   efi_status_t ret;
+   u32 attr = EFI_VARIABLE_BOOTSERVICE_ACCESS |
+   EFI_VARIABLE_RUNTIME_ACCESS |
+   EFI_VARIABLE_NON_VOLATILE;
+   u64 max_variable_storage_size;
+   u64 remain_variable_storage_size;
+   u64 max_variable_size;
+
+   ret = EFI_CALL(efi_query_variable_info(attr,
+  _variable_storage_size,
+  _variable_storage_size,
+  _variable_size));
+   if (ret != EFI_SUCCESS)
+   return CMD_RET_FAILURE;
+
+   printf("%.*s \n", EFI_HANDLE_WIDTH, sep);
+   printf("Max storage size %llu\n", max_variable_storage_size);
+   printf("Remaining storage size %llu\n", remain_variable_storage_size);
+   printf("Max variable size %llu\n", max_variable_size);
+
+   return CMD_RET_SUCCESS;
+}
+
 static cmd_tbl_t cmd_efidebug_sub[] = {
U_BOOT_CMD_MKENT(boot, CONFIG_SYS_MAXARGS, 1, do_efi_boot_opt, "", ""),
U_BOOT_CMD_MKENT(devices, CONFIG_SYS_MAXARGS, 1, do_efi_show_devices,
@@ -1176,6 +1215,8 @@ static cmd_tbl_t cmd_efidebug_sub[] = {
 "", ""),
U_BOOT_CMD_MKENT(test, CONFIG_SYS_MAXARGS, 1, do_efi_test,
 "", ""),
+   U_BOOT_CMD_MKENT(query, CONFIG_SYS_MAXARGS, 1, do_efi_query_info,
+"", ""),
 };
 
 /**
@@ -1247,7 +1288,9 @@ static char efidebug_help_text[] =
"efidebug tables\n"
"  - show UEFI configuration tables\n"
"efidebug test bootmgr\n"
-   "  - run simple bootmgr for test\n";
+   "  - run simple bootmgr for test\n"
+   "efidebug query\n"
+   "  - show information of the container used to store UEFI variables\n";
 #endif
 
 U_BOOT_CMD(
-- 
2.26.2



[PATCH 5/6] MAINTAINERS: Add maintainer for EFI variables via OP-TEE

2020-05-06 Thread Ilias Apalodimas
Add myself as maintainer for the OP-TEE related UEFI variable storage
and add the headers files on the existing EFI list

Signed-off-by: Ilias Apalodimas 
---
 MAINTAINERS | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ec59ce8b8802..f33fd74b330b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -625,6 +625,8 @@ F:  include/cp437.h
 F: include/efi*
 F: include/pe.h
 F: include/asm-generic/pe.h
+F: include/mm_communication.h
+F: include/mm_variable.h
 F: lib/charset.c
 F: lib/efi*/
 F: test/py/tests/test_efi*
@@ -635,6 +637,11 @@ F: cmd/efidebug.c
 F: cmd/nvedit_efi.c
 F: tools/file2include.c
 
+EFI VARIABLES VIA OP-TEE
+M: Ilias Apalodimas 
+S: Maintained
+F: lib/efi_loader/efi_variable_tee.c
+
 ENVIRONMENT
 M: Joe Hershberger 
 R: Wolfgang Denk 
-- 
2.26.2



[PATCH 1/6] charset: Add support for calculating bytes occupied by a u16 string

2020-05-06 Thread Ilias Apalodimas
From: Sughosh Ganu 

The current code uses 'u16_strlen(x) + 1) * sizeof(u16)' in various
places to calculate the number of bytes occupied by a u16 string.
Let's introduce a wrapper around this. This wrapper is used on following
patches

Signed-off-by: Sughosh Ganu 
---
 include/charset.h | 11 +++
 lib/charset.c |  5 +
 2 files changed, 16 insertions(+)

diff --git a/include/charset.h b/include/charset.h
index fde6bddbc2fb..30faa72285e6 100644
--- a/include/charset.h
+++ b/include/charset.h
@@ -195,6 +195,17 @@ int u16_strncmp(const u16 *s1, const u16 *s2, size_t n);
  */
 size_t u16_strlen(const void *in);
 
+/**
+ * u16_strsize - count size of u16 string in bytes including the null character
+ *
+ * Counts the number of bytes occupied by a u16 string
+ *
+ * @in:null terminated u16 string
+ * Return: bytes in a u16 string
+ *
+ */
+size_t u16_strsize(const void *in);
+
 /**
  * u16_strlen - count non-zero words
  *
diff --git a/lib/charset.c b/lib/charset.c
index 1c6a7f693de4..a28034ee1f1e 100644
--- a/lib/charset.c
+++ b/lib/charset.c
@@ -379,6 +379,11 @@ size_t u16_strnlen(const u16 *in, size_t count)
return i;
 }
 
+size_t u16_strsize(const void *in)
+{
+   return (u16_strlen(in) + 1) * sizeof(u16);
+}
+
 u16 *u16_strcpy(u16 *dest, const u16 *src)
 {
u16 *tmp = dest;
-- 
2.26.2



[PATCH 0/6] EFI variable support via OP-TEE

2020-05-06 Thread Ilias Apalodimas
With new OP-TEE and EDK2 patches (not yet upstreamed [1][2]) we can run a
secure world application which manages UEFI variables. 
Leveraging the U-Boot's OP-TEE supplicant we can then store those values 
in an RPMB device.

The Secure World application responsible for doing that is coming from
EDK2 (StandAloneMM). The StandAloneMM application is compiled from EDK2 
and then appended in the OP-TEE binary which executes that in Secure World.

There are various advantages in using StandAloneMM directly. Apart from the
obvious ones, such as running the whole code in Secure World, we are using
the EDK2 Fault Tolerant Write protocol to protect the variables against
corruption. We also avoid adding complicated code to U-Boot for the variable 
management. The only code U-Boot needs is a set OP-TEE APIs to access the new
application.

>From a user's perspective this changes nothing to the usual way UEFI variables
are treated. The user will still use 'setenv/printenv -e' to perform the 
variable operations.
An 'efidebug query' command is added to retrieve information about the 
container used to store UEFI variables.

Since patches for the rest of the projects are not yet upstreamed I've tried to
provide a QEMU emulation for anyone interested in testing the patchset.

1. Download precompiled TF-A, OP-TEE binaries and a DTB from
   https://people.linaro.org/~ilias.apalodimas/secure_boot/
   bl33.bin is a precompiled U-Boot binary. If you use that skip (2)

2. git clone --single-branch --branch rpmb_hack \
https://git.linaro.org/people/ilias.apalodimas/u-boot.git
   
   I've included a defconfig to make your life easier
   export CROSS_COMPILE=aarch64-linux-gnu-
   export ARCH=arm64
   make qemu_tfa_mm_defconfig && make -j4
   Safely ignore the warnings, they are a result of the RPMB emulation
   monstrosity ...
   The branch contains an extra patch which adds RPMB emulation into OP-TEE
   supplicant but breaks proper RPMB hardware. The patch is a gross hack 
   (but can be upstreamed later for Gitlab testing if needed).
   Since QEMU has no RPMB emulation providing one through the OP-TEE supplicant
   was the easiest way to test the patchset.

3. Create PK, KEK etc
   - PK:
   openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_PK/ -keyout \
PK.key -out PK.crt -nodes -days 365
   cert-to-efi-sig-list -g ----123456789abc PK.crt \
PK.esl; sign-efi-sig-list -c PK.crt -k PK.key PK PK.esl PK.auth

   - KEK:
   openssl req -x509 -sha256 -newkey rsa:2048 -subj /CN=TEST_KEK/ -keyout \
KEK.key -out KEK.crt -nodes -days 365
   cert-to-efi-sig-list -g ----123456789abc KEK.crt \
KEK.esl; sign-efi-sig-list -c PK.crt -k PK.key KEK KEK.esl KEK.auth

   - put them in a file and create an image
   mkdir tmp && cp PK.auth KEK.auth tmp/
   virt-make-fs -s 1M -t ext4 tmp certs.img

4. Launch QEMU (note tested on QEMU 3.1 and 5.0)
   qemu-system-aarch64  -m 1024 -smp 2 -show-cursor -serial stdio \
   -monitor null -nographic \
   -cpu cortex-a57 -bios bl1.bin  -machine virt,secure=on -d unimp \
   -semihosting-config enable,target=native -dtb ledge-qemuarm64.dtb \
   -drive if=none,file=certs.img,id=mydisk \
   -device nvme,drive=mydisk,serial=foo

   You can now load and test the certificates 
   nvme scan
   load nvme 0 0x7000 PK.auth
   setenv -e -nv -bs -rt -at -i 0x7000,$filesize PK; 
   load nvme 0 0x7000 KEK.auth
   setenv -e -nv -bs -rt -at -i 0x7000,$filesize KEK; 
   efidebug query
   printenv -e -all

This has been tested against U-Boot efi_sefltest but since StandAloneMM will
write a non-existent variable if APPEND_WRITE is specified, this test will fail.
It has also been tested for the runtime variable support (or rather absence of
it) with FWTS (https://wiki.ubuntu.com/FirmwareTestSuite).

This patchset adds the needed APIs between U-Boot and OP-TEE and StandAloneMM
to perform the variable storage.

[1] https://github.com/apalos/optee_os/tree/stmm_upstream_03_clean
[2] 
https://git.linaro.org/people/ilias.apalodimas/edk2-platforms.git/log/?h=patch_pcd


Ilias Apalodimas (4):
  efi_loader: Implement EFI variable handling via OP-TEE
  cmd: efidebug: Add support for querying UEFI variable storage
  MAINTAINERS: Add maintainer for EFI variables via OP-TEE
  doc: uefi.rst: Add OP-TEE variable storage config options

Sughosh Ganu (2):
  charset: Add support for calculating bytes occupied by a u16 string
  efi_loader: Add headers for EDK2 StandAloneMM communication

 MAINTAINERS   |   7 +
 cmd/efidebug.c|  45 ++-
 doc/uefi/uefi.rst |  10 +
 include/charset.h |  11 +
 include/mm_communication.h|  28 ++
 include/mm_variable.h |  78 
 lib/charset.c |   5 +
 lib/efi_loader/Kconfig|   9 +
 lib/efi_loader/Makefile   |   4 +
 lib/efi_loader/efi_variable_tee.c | 645 ++
 10 files changed, 841 

Re: [PATCH 1/2] JFFS2: Process obsolete nodes as well as accurate ones

2020-05-06 Thread Tom Rini
On Mon, Apr 27, 2020 at 06:43:25PM +0200, petr.bors...@i.cz wrote:

> From: Petr Borsodi 
> 
> Obsolete nodes (ie. without the JFFS2_NODE_ACCURATE flag) were ignored because
> they had seemingly invalid crc. This could lead to finding the phantom node
> header in obsolete node data.
> 
> Signed-off-by: Petr Borsodi 

This (and 2/2) have a number of checkpatch warnings and while some of
them cannot be fixed easily (such as CamelCase stuff that you're
re-using and not introducing) others such as spacing, NULL comparisons,
etc, need to be fixed.  Thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v2 1/2] net: cortina_ni: Addd eth support for Cortina Access CAxxxx SoCs

2020-05-06 Thread Tom Rini
On Wed, Apr 29, 2020 at 05:07:29PM -0700, Alex Nemirovsky wrote:
Now,
> From: Aaron Tseng 
> 
> Add Cortina Access Ethernet device driver for CA SoCs.
> This driver supports both legacy and DM_ETH network models.
> 
> Signed-off-by: Aaron Tseng 
> Signed-off-by: Alex Nemirovsky 
> Signed-off-by: Abbie Chang 
> Cc: Joe Hershberger 
> 
> Series changes: 2
> - cleanup MAINTAINERS

Sorry for the late review, checkpatch reports a number of issues, please
address those (and check the other I/O patches, etc, for checkpatch
problems as well).  Thanks!

-- 
Tom


signature.asc
Description: PGP signature


[PATCH 2/2] board: kontron: add sl28 support

2020-05-06 Thread Michael Walle
Add basic support for the Kontron SMARC-sAL28 board. This includes just
the bare minimum to be able to bring up the board and boot linux.

Also only one board variant is supported for now. That is the Single PHY
variant. Other variants will fall back to this one.

In particular, there is no watchdog support for now. This means that you
have to disable the default watchdog, otherwise you'll end up in the
recovery bootloader.  See the boards README.md for details.

Signed-off-by: Michael Walle 
---

This is part 1 of the basic board support. Part 2 depends on the Atheros
RGMII PHY device tree bindings which are still pending.

 arch/arm/Kconfig  |  10 ++
 arch/arm/dts/Makefile |   2 +
 .../arm/dts/fsl-ls1028a-kontron-sl28-var3.dts |  14 ++
 arch/arm/dts/fsl-ls1028a-kontron-sl28.dts | 130 ++
 board/kontron/sl28/Kconfig|  18 +++
 board/kontron/sl28/MAINTAINERS|   6 +
 board/kontron/sl28/Makefile   |   8 ++
 board/kontron/sl28/README |  63 +
 board/kontron/sl28/common.c   |  10 ++
 board/kontron/sl28/ddr.c  |  98 +
 board/kontron/sl28/sl28.c |  90 
 board/kontron/sl28/spl.c  |   9 ++
 configs/kontron_sl28_defconfig| 102 ++
 include/configs/kontron_sl28.h|  85 
 14 files changed, 645 insertions(+)
 create mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28-var3.dts
 create mode 100644 arch/arm/dts/fsl-ls1028a-kontron-sl28.dts
 create mode 100644 board/kontron/sl28/Kconfig
 create mode 100644 board/kontron/sl28/MAINTAINERS
 create mode 100644 board/kontron/sl28/Makefile
 create mode 100644 board/kontron/sl28/README
 create mode 100644 board/kontron/sl28/common.c
 create mode 100644 board/kontron/sl28/ddr.c
 create mode 100644 board/kontron/sl28/sl28.c
 create mode 100644 board/kontron/sl28/spl.c
 create mode 100644 configs/kontron_sl28_defconfig
 create mode 100644 include/configs/kontron_sl28.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b494bcae95..7201905c50 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1563,6 +1563,15 @@ config TARGET_LS1046AFRWY
  development platform that supports the QorIQ LS1046A
  Layerscape Architecture processor.
 
+config TARGET_SL28
+   bool "Support sl28"
+   select ARCH_LS1028A
+   select ARM64
+   select ARMV8_MULTIENTRY
+   select SUPPORT_SPL
+   help
+ Support for Kontron SMARC-sAL28 board.
+
 config TARGET_COLIBRI_PXA270
bool "Support colibri_pxa270"
select CPU_PXA
@@ -1893,6 +1902,7 @@ source "board/hisilicon/hikey/Kconfig"
 source "board/hisilicon/hikey960/Kconfig"
 source "board/hisilicon/poplar/Kconfig"
 source "board/isee/igep003x/Kconfig"
+source "board/kontron/sl28/Kconfig"
 source "board/phytec/pcm051/Kconfig"
 source "board/silica/pengwyn/Kconfig"
 source "board/spear/spear300/Kconfig"
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 559d3ab6a7..97e439a87f 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -379,6 +379,8 @@ dtb-$(CONFIG_FSL_LSCH3) += fsl-ls2080a-qds.dtb \
fsl-ls2088a-rdb-qspi.dtb \
fsl-ls1088a-rdb.dtb \
fsl-ls1088a-qds.dtb \
+   fsl-ls1028a-kontron-sl28.dtb \
+   fsl-ls1028a-kontron-sl28-var3.dtb \
fsl-ls1028a-rdb.dtb \
fsl-ls1028a-qds-duart.dtb \
fsl-ls1028a-qds-lpuart.dtb \
diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28-var3.dts 
b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var3.dts
new file mode 100644
index 00..23f89eadb3
--- /dev/null
+++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28-var3.dts
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Kontron SMARC-sAL28 device tree source (Single PHY version)
+ */
+
+/dts-v1/;
+
+#include "fsl-ls1028a-kontron-sl28.dts"
+
+/ {
+   u-boot,dm-pre-reloc;
+   model = "Kontron SMARC-sAL28 Board (Single PHY)";
+   compatible = "kontron,sl28-var3", "kontron,sl28", "fsl,ls1028a";
+};
diff --git a/arch/arm/dts/fsl-ls1028a-kontron-sl28.dts 
b/arch/arm/dts/fsl-ls1028a-kontron-sl28.dts
new file mode 100644
index 00..a14f4e52e1
--- /dev/null
+++ b/arch/arm/dts/fsl-ls1028a-kontron-sl28.dts
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Kontron SMARC-sAL28 device tree source (base version)
+ */
+
+/dts-v1/;
+
+#include "fsl-ls1028a.dtsi"
+
+/ {
+   u-boot,dm-pre-reloc;
+   model = "Kontron SMARC-sAL28 Board";
+   compatible = "kontron,sl28", "fsl,ls1028a";
+
+   aliases {
+   serial0 = 
+   serial1 = 
+   spi0 = 
+   spi1 = 
+   mmc0 = 
+   mmc1 = 
+   i2c0 = 
+   i2c1 = 
+   i2c2 = 
+   rtc0 = 
+   watchdog0 = 
+   watchdog1 = _core0_watchdog;

[PATCH 1/2] armv8: ls1028a: move FSL_LAYERSCAPE to kconfig

2020-05-06 Thread Michael Walle
CONFIG_FSL_LAYERSCAPE is available in kconfig. There is no need to
define it per board; the ls1028a_common.h is really board dependent and
only fits to the NXP eval boards. Instead select CONFIG_FSL_LAYERSCAPE
when ARCH_LS1028A is selected.

Signed-off-by: Michael Walle 
---
 arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 1 +
 include/configs/ls1028a_common.h  | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig 
b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
index 2f75b2cdd3..116b6b0617 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
+++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig
@@ -23,6 +23,7 @@ config ARCH_LS1012A
 config ARCH_LS1028A
bool
select ARMV8_SET_SMPEN
+   select FSL_LAYERSCAPE
select FSL_LSCH3
select NXP_LSCH3_2
select SYS_FSL_HAS_CCI400
diff --git a/include/configs/ls1028a_common.h b/include/configs/ls1028a_common.h
index 6905694d10..971288f86e 100644
--- a/include/configs/ls1028a_common.h
+++ b/include/configs/ls1028a_common.h
@@ -7,7 +7,6 @@
 #define __L1028A_COMMON_H
 
 #define CONFIG_REMAKE_ELF
-#define CONFIG_FSL_LAYERSCAPE
 #define CONFIG_MP
 
 #include 
-- 
2.20.1



[PATCH 1/1] efi_loader: put device tree into EfiACPIReclaimMemory

2020-05-06 Thread Heinrich Schuchardt
According to the UEFI spec ACPI tables should be placed in
EfiACPIReclaimMemory. Let's do the same with the device tree.

Suggested-by: Ard Biesheuvel 
Cc: Grant Likely 
Signed-off-by: Heinrich Schuchardt 
---
 cmd/bootefi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 54b4b8f984..06573b14e9 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -127,13 +127,13 @@ static efi_status_t copy_fdt(void **fdtp)
new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f0 +
 fdt_size, 0);
ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-EFI_BOOT_SERVICES_DATA, fdt_pages,
+EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
 _fdt_addr);
if (ret != EFI_SUCCESS) {
/* If we can't put it there, put it somewhere */
new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-EFI_BOOT_SERVICES_DATA, fdt_pages,
+EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
 _fdt_addr);
if (ret != EFI_SUCCESS) {
printf("ERROR: Failed to reserve space for FDT\n");
--
2.26.2



Re: [PATCH V2] mkimage: fit: Do not tail-pad fitImage with external data

2020-05-06 Thread Marek Vasut
On 5/6/20 7:02 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 06:35:52PM +0200, Marek Vasut wrote:
>> On 5/6/20 6:33 PM, Tom Rini wrote:
>>> On Wed, May 06, 2020 at 06:17:47PM +0200, Marek Vasut wrote:
 On 5/6/20 6:04 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 05:52:45PM +0200, Marek Vasut wrote:
>> On 5/6/20 5:43 PM, Alex Kiernan wrote:
>>> On Wed, May 6, 2020 at 3:41 PM Marek Vasut  wrote:

 On 5/6/20 4:37 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 04:33:37PM +0200, Marek Vasut wrote:
>> On 5/6/20 4:27 PM, Tom Rini wrote:
>>> On Wed, May 06, 2020 at 04:17:35PM +0200, Marek Vasut wrote:
 On 5/6/20 3:48 PM, Tom Rini wrote:
> On Tue, May 05, 2020 at 11:17:19PM +0200, Michael Walle wrote:
>> Hi all,
>>
>> Am 2020-05-05 20:41, schrieb Simon Glass:
>>> Hi Tom,
>>>
>>> On Tue, 5 May 2020 at 11:50, Tom Rini  
>>> wrote:

 On Tue, May 05, 2020 at 06:39:58PM +0200, Marek Vasut wrote:
> On 5/5/20 6:37 PM, Alex Kiernan wrote:
>> On Tue, May 5, 2020 at 2:28 PM Marek Vasut  
>> wrote:
>>>
>>> On 5/5/20 3:22 PM, Alex Kiernan wrote:
 On Mon, May 4, 2020 at 12:28 PM Tom Rini 
  wrote:
>
> On Fri, May 01, 2020 at 05:40:25PM +0200, Marek Vasut 
> wrote:
>
>> There is no reason to tail-pad fitImage with external 
>> data to 4-bytes,
>> while fitImage without external data does not have any 
>> such padding and
>> is often unaligned. DT spec also does not mandate any 
>> such padding.
>>
>> Moreover, the tail-pad fills the last few bytes with 
>> uninitialized data,
>> which could lead to a potential information leak.
>>
>> $ echo -n xy > /tmp/data ; \
>>   ./tools/mkimage -E -f auto -d /tmp/data 
>> /tmp/fitImage ; \
>>   hexdump -vC /tmp/fitImage | tail -n 3
>>
>> before:
>> 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 
>> 69  |a-offset.data-si|
>> 0270  7a 65 00 00 78 79 64 64
>>|ze..xydd|
>>^^   ^^ ^^
>> after:
>> 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 
>> 69  |a-offset.data-si|
>> 0270  7a 65 00 78 79 
>>|ze.xy|
>>
>> Signed-off-by: Marek Vasut 
>> Reviewed-by: Simon Glass 
>> Cc: Heinrich Schuchardt 
>> Cc: Tom Rini 
>
> Applied to u-boot/master, thanks!
>

 This breaks booting on my board (am3352, eMMC boot, FIT 
 u-boot,
 CONFIG_SPL_LOAD_FIT). Not got any useful diagnostics - if 
 I boot it
 from eMMC I get nothing at all on the console, if I boot 
 over ymodem
 it stalls at 420k, before continuing to 460k. My guess is 
 there's some
 error going to the console at the 420k mark, but obviously 
 it's lost
 in the ymodem... I have two DTBs in the FIT image, 420k 
 would about
 align to the point between them.
>>>
>>> My bet would be on some padding / unaligned access problem 
>>> that this
>>> patch uncovered. Can you take a look ?
>>
>> Seems plausible. With this change my external data starts at 
>> 0x483 and
>> everything after it is non-aligned:
>
> Should the beginning of external data be aligned ?

 If in U-Boot we revert 
 e8c2d25845c72c7202a628a97d45e31beea40668 does
 the
 problem go away?  If so, that's not a fix outright, it means 
 we need
 to
 dig back in to the libfdt thread and find the "make this work 
 without
 killing performance everywhere all the time" option.
>>>
>>> If it is a 

[PATCH 6/6] qemu-x86*_defconfig: Enable CONFIG_PCI_INIT_R

2020-05-06 Thread Ovidiu Panait
Enable CONFIG_PCI_INIT_R for qemux86 and qemux86-64 pci enumeration during
boot in order to eliminate the custom preboot commands in
include/configs/qemu-x86.h.

Signed-off-by: Ovidiu Panait 
---
 configs/qemu-x86_64_defconfig | 1 +
 configs/qemu-x86_defconfig| 1 +
 include/configs/qemu-x86.h| 2 --
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/configs/qemu-x86_64_defconfig b/configs/qemu-x86_64_defconfig
index 0cb123eb4a..6f722c1d23 100644
--- a/configs/qemu-x86_64_defconfig
+++ b/configs/qemu-x86_64_defconfig
@@ -34,6 +34,7 @@ CONFIG_SPL_NET_SUPPORT=y
 CONFIG_SPL_PCI=y
 CONFIG_SPL_PCH_SUPPORT=y
 CONFIG_SPL_RTC_SUPPORT=y
+CONFIG_PCI_INIT_R=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_BOOTEFI_SELFTEST=y
 CONFIG_CMD_NVEDIT_EFI=y
diff --git a/configs/qemu-x86_defconfig b/configs/qemu-x86_defconfig
index a562f213e1..565f232b4f 100644
--- a/configs/qemu-x86_defconfig
+++ b/configs/qemu-x86_defconfig
@@ -18,6 +18,7 @@ CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro"
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_LAST_STAGE_INIT=y
+CONFIG_PCI_INIT_R=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_BOOTEFI_SELFTEST=y
 CONFIG_CMD_NVEDIT_EFI=y
diff --git a/include/configs/qemu-x86.h b/include/configs/qemu-x86.h
index 49e307b430..52c33600b3 100644
--- a/include/configs/qemu-x86.h
+++ b/include/configs/qemu-x86.h
@@ -22,8 +22,6 @@
 #include 
 #include 
 
-#define CONFIG_PREBOOT "pci enum"
-
 #define CONFIG_SYS_MONITOR_LEN (1 << 20)
 
 #define CONFIG_STD_DEVICES_SETTINGS"stdin=serial,i8042-kbd\0" \
-- 
2.17.1



[PATCH 5/6] qemu_arm_defconfig: Enable CONFIG_PCI_INIT_R

2020-05-06 Thread Ovidiu Panait
Replace the "pci enum" preboot sequence with CONFIG_PCI_INIT_R=y.

Signed-off-by: Ovidiu Panait 
---
 configs/qemu_arm_defconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
index f807dfc10e..a8473988bd 100644
--- a/configs/qemu_arm_defconfig
+++ b/configs/qemu_arm_defconfig
@@ -12,10 +12,9 @@ CONFIG_FIT_SIGNATURE=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_BEST_MATCH=y
 CONFIG_LEGACY_IMAGE_FORMAT=y
-CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="pci enum"
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_PCI_INIT_R=y
 CONFIG_CMD_BOOTEFI_SELFTEST=y
 CONFIG_CMD_NVEDIT_EFI=y
 CONFIG_CMD_PCI=y
-- 
2.17.1



[PATCH 1/6] env: Convert CONFIG_DELAY_ENVIRONMENT to Kconfig

2020-05-06 Thread Ovidiu Panait
This converts ad-hoc CONFIG_DELAY_ENVIRONMENT to Kconfig.

Signed-off-by: Ovidiu Panait 
---
 env/Kconfig  | 12 
 scripts/config_whitelist.txt |  1 -
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/env/Kconfig b/env/Kconfig
index af63ac52f7..ed94e83ec1 100644
--- a/env/Kconfig
+++ b/env/Kconfig
@@ -592,6 +592,18 @@ config ENV_VARS_UBOOT_RUNTIME_CONFIG
  run-time determined information about the hardware to the
  environment.  These will be named board_name, board_rev.
 
+config DELAY_ENVIRONMENT
+   bool "Delay environment loading"
+   depends on !OF_CONTROL
+   help
+ Enable this to inhibit loading the environment during board
+ initialization. This can address the security risk of untrusted data
+ being used during boot. Normally the environment is loaded when the
+ board is initialised so that it is available to U-Boot. This inhibits
+ that so that the environment is not available until explicitly loaded
+ later by U-Boot code. With CONFIG_OF_CONTROL this is instead
+ controlled by the value of /config/load-environment.
+
 if SPL_ENV_SUPPORT
 config SPL_ENV_IS_NOWHERE
bool "SPL Environment is not stored"
diff --git a/scripts/config_whitelist.txt b/scripts/config_whitelist.txt
index 19c9218060..bc086363ca 100644
--- a/scripts/config_whitelist.txt
+++ b/scripts/config_whitelist.txt
@@ -306,7 +306,6 @@ CONFIG_DEFAULT
 CONFIG_DEFAULT_CONSOLE
 CONFIG_DEFAULT_IMMR
 CONFIG_DEF_HWCONFIG
-CONFIG_DELAY_ENVIRONMENT
 CONFIG_DESIGNWARE_ETH
 CONFIG_DEVELOP
 CONFIG_DEVICE_TREE_LIST
-- 
2.17.1



[PATCH 4/6] qemu_arm64_defconfig: Enable CONFIG_PCI_INIT_R

2020-05-06 Thread Ovidiu Panait
Replace the "pci enum" preboot sequence with CONFIG_PCI_INIT_R=y.

Signed-off-by: Ovidiu Panait 
---
 configs/qemu_arm64_defconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
index 80e0ad55e0..53c653df21 100644
--- a/configs/qemu_arm64_defconfig
+++ b/configs/qemu_arm64_defconfig
@@ -10,10 +10,9 @@ CONFIG_FIT_SIGNATURE=y
 CONFIG_FIT_VERBOSE=y
 CONFIG_FIT_BEST_MATCH=y
 CONFIG_LEGACY_IMAGE_FORMAT=y
-CONFIG_USE_PREBOOT=y
-CONFIG_PREBOOT="pci enum"
 # CONFIG_DISPLAY_CPUINFO is not set
 # CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_PCI_INIT_R=y
 CONFIG_CMD_BOOTEFI_SELFTEST=y
 CONFIG_CMD_NVEDIT_EFI=y
 CONFIG_CMD_PCI=y
-- 
2.17.1



[PATCH 2/6] board_r: env: Use IS_ENABLED() instead of #ifdefs

2020-05-06 Thread Ovidiu Panait
Use IS_ENABLED() instead of #ifdef in should_load_env and initr_env
functions.

No functional change intended.

Signed-off-by: Ovidiu Panait 
---
 common/board_r.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index d9015cd057..f6770f2300 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -464,13 +464,14 @@ static int initr_mmc(void)
  */
 static int should_load_env(void)
 {
-#ifdef CONFIG_OF_CONTROL
-   return fdtdec_get_config_int(gd->fdt_blob, "load-environment", 1);
-#elif defined CONFIG_DELAY_ENVIRONMENT
-   return 0;
-#else
+   if (IS_ENABLED(CONFIG_OF_CONTROL))
+   return fdtdec_get_config_int(gd->fdt_blob,
+   "load-environment", 1);
+
+   if (IS_ENABLED(CONFIG_DELAY_ENVIRONMENT))
+   return 0;
+
return 1;
-#endif
 }
 
 static int initr_env(void)
@@ -480,10 +481,10 @@ static int initr_env(void)
env_relocate();
else
env_set_default(NULL, 0);
-#ifdef CONFIG_OF_CONTROL
-   env_set_hex("fdtcontroladdr",
-   (unsigned long)map_to_sysmem(gd->fdt_blob));
-#endif
+
+   if (IS_ENABLED(CONFIG_OF_CONTROL))
+   env_set_hex("fdtcontroladdr",
+   (unsigned long)map_to_sysmem(gd->fdt_blob));
 
/* Initialize from environment */
image_load_addr = env_get_ulong("loadaddr", 16, image_load_addr);
-- 
2.17.1



[PATCH 3/6] board_r: Introduce CONFIG_PCI_INIT_R Kconfig option

2020-05-06 Thread Ovidiu Panait
With CONFIG_DM_PCI enabled, PCI buses are not enumerated at boot, as they
are without that config option enabled. However, there are cases such as DM
PCI-based Ethernet devices that need the PCI bus enumerated so that they
can be discovered by their drivers.

Currently, to solve this, some boards enumerate the pci bus using
"pci enum" preboot command, while others do it manually in board files
(in board_init/board_late_init/etc. functions).

In order to possibly make the pci enumeration process uniform across all
boards, introduce CONFIG_PCI_INIT_R Kconfig option.

This change also preserves the current behavior in the !DM_PCI case
(pci_init is run unconditionally at boot).

Signed-off-by: Ovidiu Panait 
---
 common/Kconfig   | 10 ++
 common/board_r.c |  5 ++---
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index 30cba15948..2d86dd7e63 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -932,6 +932,16 @@ config LAST_STAGE_INIT
  U-Boot calls last_stage_init() before the command-line interpreter is
  started.
 
+config PCI_INIT_R
+   bool "Enumerate PCI buses during init"
+   depends on PCI
+   default y if !DM_PCI
+   help
+ With this option U-Boot will call pci_init() soon after relocation,
+ which will enumerate PCI buses. This is needed, for instance, in the
+ case of DM PCI-based Ethernet devices, which will not be detected
+ without having the enumeration performed earlier.
+
 endmenu
 
 menu "Security support"
diff --git a/common/board_r.c b/common/board_r.c
index f6770f2300..96034b874e 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -232,9 +232,8 @@ static int initr_unlock_ram_in_cache(void)
 #ifdef CONFIG_PCI
 static int initr_pci(void)
 {
-#ifndef CONFIG_DM_PCI
-   pci_init();
-#endif
+   if (IS_ENABLED(CONFIG_PCI_INIT_R))
+   pci_init();
 
return 0;
 }
-- 
2.17.1



Re: [PATCH V2] mkimage: fit: Do not tail-pad fitImage with external data

2020-05-06 Thread Tom Rini
On Wed, May 06, 2020 at 06:35:52PM +0200, Marek Vasut wrote:
> On 5/6/20 6:33 PM, Tom Rini wrote:
> > On Wed, May 06, 2020 at 06:17:47PM +0200, Marek Vasut wrote:
> >> On 5/6/20 6:04 PM, Tom Rini wrote:
> >>> On Wed, May 06, 2020 at 05:52:45PM +0200, Marek Vasut wrote:
>  On 5/6/20 5:43 PM, Alex Kiernan wrote:
> > On Wed, May 6, 2020 at 3:41 PM Marek Vasut  wrote:
> >>
> >> On 5/6/20 4:37 PM, Tom Rini wrote:
> >>> On Wed, May 06, 2020 at 04:33:37PM +0200, Marek Vasut wrote:
>  On 5/6/20 4:27 PM, Tom Rini wrote:
> > On Wed, May 06, 2020 at 04:17:35PM +0200, Marek Vasut wrote:
> >> On 5/6/20 3:48 PM, Tom Rini wrote:
> >>> On Tue, May 05, 2020 at 11:17:19PM +0200, Michael Walle wrote:
>  Hi all,
> 
>  Am 2020-05-05 20:41, schrieb Simon Glass:
> > Hi Tom,
> >
> > On Tue, 5 May 2020 at 11:50, Tom Rini  
> > wrote:
> >>
> >> On Tue, May 05, 2020 at 06:39:58PM +0200, Marek Vasut wrote:
> >>> On 5/5/20 6:37 PM, Alex Kiernan wrote:
>  On Tue, May 5, 2020 at 2:28 PM Marek Vasut  
>  wrote:
> >
> > On 5/5/20 3:22 PM, Alex Kiernan wrote:
> >> On Mon, May 4, 2020 at 12:28 PM Tom Rini 
> >>  wrote:
> >>>
> >>> On Fri, May 01, 2020 at 05:40:25PM +0200, Marek Vasut 
> >>> wrote:
> >>>
>  There is no reason to tail-pad fitImage with external 
>  data to 4-bytes,
>  while fitImage without external data does not have any 
>  such padding and
>  is often unaligned. DT spec also does not mandate any 
>  such padding.
> 
>  Moreover, the tail-pad fills the last few bytes with 
>  uninitialized data,
>  which could lead to a potential information leak.
> 
>  $ echo -n xy > /tmp/data ; \
>    ./tools/mkimage -E -f auto -d /tmp/data 
>  /tmp/fitImage ; \
>    hexdump -vC /tmp/fitImage | tail -n 3
> 
>  before:
>  0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 
>  69  |a-offset.data-si|
>  0270  7a 65 00 00 78 79 64 64
> |ze..xydd|
> ^^   ^^ ^^
>  after:
>  0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 
>  69  |a-offset.data-si|
>  0270  7a 65 00 78 79 
> |ze.xy|
> 
>  Signed-off-by: Marek Vasut 
>  Reviewed-by: Simon Glass 
>  Cc: Heinrich Schuchardt 
>  Cc: Tom Rini 
> >>>
> >>> Applied to u-boot/master, thanks!
> >>>
> >>
> >> This breaks booting on my board (am3352, eMMC boot, FIT 
> >> u-boot,
> >> CONFIG_SPL_LOAD_FIT). Not got any useful diagnostics - if 
> >> I boot it
> >> from eMMC I get nothing at all on the console, if I boot 
> >> over ymodem
> >> it stalls at 420k, before continuing to 460k. My guess is 
> >> there's some
> >> error going to the console at the 420k mark, but obviously 
> >> it's lost
> >> in the ymodem... I have two DTBs in the FIT image, 420k 
> >> would about
> >> align to the point between them.
> >
> > My bet would be on some padding / unaligned access problem 
> > that this
> > patch uncovered. Can you take a look ?
> 
>  Seems plausible. With this change my external data starts at 
>  0x483 and
>  everything after it is non-aligned:
> >>>
> >>> Should the beginning of external data be aligned ?
> >>
> >> If in U-Boot we revert 
> >> e8c2d25845c72c7202a628a97d45e31beea40668 does
> >> the
> >> problem go away?  If so, that's not a fix outright, it means 
> >> we need
> >> to
> >> dig back in to the libfdt thread and find the "make this work 
> >> without
> >> killing performance everywhere all the time" option.
> >
> > If it is a device tree, it must be 32-bit 

Re: [PATCH] misc: i2c_eeprom: implement different probe test eeprom offset

2020-05-06 Thread Baruch Siach
Hi Eugen,

On Wed, May 06 2020, Eugen Hristev wrote:
> Because of this commit :
> 5ae84860b0 ("misc: i2c_eeprom: verify that the chip is functional at probe()")
> at probe time, each eeprom is tested for read at offset 0.
>
> The Atmel AT24MAC402 eeprom has different mapping. One i2c slave address is
> used for the lower 0x80 bytes and another i2c slave address is used for the
> upper 0x80 bytes. Because of this basically the i2c master sees 2 different
> slaves. We need the upper bytes because we read the unique MAC address from
> this EEPROM area.
>
> However this implies that our slave address will return error on reads
> from address 0x0 to 0x80.
>
> To solve this, implemented a way to figure out from the compatible udevice_id
> 'data' field which offset we should attempt to read.
> Added the offset as one byte in the data field (byte number 3).
> Created two macros that will convert offset to 'data' field and back.
>
> The probe function will now read this offset and use it, instead of blindly
> checking offset 0.
>
> This will fix the regression noticed on these EEPROMs since the commit
> abovementioned that introduces the probe failed issue.

Judging from the Linux at24 driver that commit appears to break a number
of other eeprom chips. I suggest to just revert it.

baruch

>
> The offset field can for sure be updated later at a 2byte offset if needed
> by anyone.
>
> Signed-off-by: Eugen Hristev 
> ---
>  drivers/misc/i2c_eeprom.c | 18 --
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c
> index 3755dbf74b..79805cc46e 100644
> --- a/drivers/misc/i2c_eeprom.c
> +++ b/drivers/misc/i2c_eeprom.c
> @@ -11,6 +11,13 @@
>  #include 
>  #include 
>  
> +/* These macros are used to encode/decode the starting EEPROM offset into the
> + * udevice_id structure's data field 3rd byte.
> + * Lower 2 bytes of the data field are used for pagewidth.
> + */
> +#define I2C_EEPROM_OFFSET_TO_DATA(v) ((v) << 16)
> +#define I2C_EEPROM_DATA_TO_OFFSET(v) ((v) >> 16)
> +
>  int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, int size)
>  {
>   const struct i2c_eeprom_ops *ops = device_get_ops(dev);
> @@ -85,11 +92,17 @@ static int i2c_eeprom_std_ofdata_to_platdata(struct 
> udevice *dev)
>  
>  static int i2c_eeprom_std_probe(struct udevice *dev)
>  {
> + u64 data = dev_get_driver_data(dev);
>   u8 test_byte;
>   int ret;
>  
>   /* Verify that the chip is functional */
> - ret = i2c_eeprom_read(dev, 0, _byte, 1);
> + /*
> +  * Not all eeproms start from offset 0. Valid offset is encoded in
> +  * upper bits of the data (byte 3).
> +  */
> + ret = i2c_eeprom_read(dev, I2C_EEPROM_DATA_TO_OFFSET(data) & 0xFF,
> +   _byte, 1);
>   if (ret)
>   return -ENODEV;
>  
> @@ -105,7 +118,8 @@ static const struct udevice_id i2c_eeprom_std_ids[] = {
>   { .compatible = "atmel,24c08", .data = 4 },
>   { .compatible = "atmel,24c08a", .data = 4 },
>   { .compatible = "atmel,24c16a", .data = 4 },
> - { .compatible = "atmel,24mac402", .data = 4 },
> + { .compatible = "atmel,24mac402",
> + .data = (4 | I2C_EEPROM_OFFSET_TO_DATA(0x80))},
>   { .compatible = "atmel,24c32", .data = 5 },
>   { .compatible = "atmel,24c64", .data = 5 },
>   { .compatible = "atmel,24c128", .data = 6 },


-- 
 ~. .~   Tk Open Systems
=}ooO--U--Ooo{=
   - bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -


Re: [PATCH V2] mkimage: fit: Do not tail-pad fitImage with external data

2020-05-06 Thread Marek Vasut
On 5/6/20 6:33 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 06:17:47PM +0200, Marek Vasut wrote:
>> On 5/6/20 6:04 PM, Tom Rini wrote:
>>> On Wed, May 06, 2020 at 05:52:45PM +0200, Marek Vasut wrote:
 On 5/6/20 5:43 PM, Alex Kiernan wrote:
> On Wed, May 6, 2020 at 3:41 PM Marek Vasut  wrote:
>>
>> On 5/6/20 4:37 PM, Tom Rini wrote:
>>> On Wed, May 06, 2020 at 04:33:37PM +0200, Marek Vasut wrote:
 On 5/6/20 4:27 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 04:17:35PM +0200, Marek Vasut wrote:
>> On 5/6/20 3:48 PM, Tom Rini wrote:
>>> On Tue, May 05, 2020 at 11:17:19PM +0200, Michael Walle wrote:
 Hi all,

 Am 2020-05-05 20:41, schrieb Simon Glass:
> Hi Tom,
>
> On Tue, 5 May 2020 at 11:50, Tom Rini  wrote:
>>
>> On Tue, May 05, 2020 at 06:39:58PM +0200, Marek Vasut wrote:
>>> On 5/5/20 6:37 PM, Alex Kiernan wrote:
 On Tue, May 5, 2020 at 2:28 PM Marek Vasut  
 wrote:
>
> On 5/5/20 3:22 PM, Alex Kiernan wrote:
>> On Mon, May 4, 2020 at 12:28 PM Tom Rini 
>>  wrote:
>>>
>>> On Fri, May 01, 2020 at 05:40:25PM +0200, Marek Vasut wrote:
>>>
 There is no reason to tail-pad fitImage with external data 
 to 4-bytes,
 while fitImage without external data does not have any 
 such padding and
 is often unaligned. DT spec also does not mandate any such 
 padding.

 Moreover, the tail-pad fills the last few bytes with 
 uninitialized data,
 which could lead to a potential information leak.

 $ echo -n xy > /tmp/data ; \
   ./tools/mkimage -E -f auto -d /tmp/data 
 /tmp/fitImage ; \
   hexdump -vC /tmp/fitImage | tail -n 3

 before:
 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69 
  |a-offset.data-si|
 0270  7a 65 00 00 78 79 64 64  
  |ze..xydd|
^^   ^^ ^^
 after:
 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69 
  |a-offset.data-si|
 0270  7a 65 00 78 79   
  |ze.xy|

 Signed-off-by: Marek Vasut 
 Reviewed-by: Simon Glass 
 Cc: Heinrich Schuchardt 
 Cc: Tom Rini 
>>>
>>> Applied to u-boot/master, thanks!
>>>
>>
>> This breaks booting on my board (am3352, eMMC boot, FIT 
>> u-boot,
>> CONFIG_SPL_LOAD_FIT). Not got any useful diagnostics - if I 
>> boot it
>> from eMMC I get nothing at all on the console, if I boot 
>> over ymodem
>> it stalls at 420k, before continuing to 460k. My guess is 
>> there's some
>> error going to the console at the 420k mark, but obviously 
>> it's lost
>> in the ymodem... I have two DTBs in the FIT image, 420k 
>> would about
>> align to the point between them.
>
> My bet would be on some padding / unaligned access problem 
> that this
> patch uncovered. Can you take a look ?

 Seems plausible. With this change my external data starts at 
 0x483 and
 everything after it is non-aligned:
>>>
>>> Should the beginning of external data be aligned ?
>>
>> If in U-Boot we revert e8c2d25845c72c7202a628a97d45e31beea40668 
>> does
>> the
>> problem go away?  If so, that's not a fix outright, it means we 
>> need
>> to
>> dig back in to the libfdt thread and find the "make this work 
>> without
>> killing performance everywhere all the time" option.
>
> If it is a device tree, it must be 32-bit aligned.

 This commit actually breaks my board too (which I was just about 
 to send
 upstream, but realized it was broken).

 Said board uses SPL and main U-Boot. SPL runs fine and main u-boot 
 doesn't
 output 

Re: [PATCH V2] mkimage: fit: Do not tail-pad fitImage with external data

2020-05-06 Thread Tom Rini
On Wed, May 06, 2020 at 06:17:47PM +0200, Marek Vasut wrote:
> On 5/6/20 6:04 PM, Tom Rini wrote:
> > On Wed, May 06, 2020 at 05:52:45PM +0200, Marek Vasut wrote:
> >> On 5/6/20 5:43 PM, Alex Kiernan wrote:
> >>> On Wed, May 6, 2020 at 3:41 PM Marek Vasut  wrote:
> 
>  On 5/6/20 4:37 PM, Tom Rini wrote:
> > On Wed, May 06, 2020 at 04:33:37PM +0200, Marek Vasut wrote:
> >> On 5/6/20 4:27 PM, Tom Rini wrote:
> >>> On Wed, May 06, 2020 at 04:17:35PM +0200, Marek Vasut wrote:
>  On 5/6/20 3:48 PM, Tom Rini wrote:
> > On Tue, May 05, 2020 at 11:17:19PM +0200, Michael Walle wrote:
> >> Hi all,
> >>
> >> Am 2020-05-05 20:41, schrieb Simon Glass:
> >>> Hi Tom,
> >>>
> >>> On Tue, 5 May 2020 at 11:50, Tom Rini  wrote:
> 
>  On Tue, May 05, 2020 at 06:39:58PM +0200, Marek Vasut wrote:
> > On 5/5/20 6:37 PM, Alex Kiernan wrote:
> >> On Tue, May 5, 2020 at 2:28 PM Marek Vasut  
> >> wrote:
> >>>
> >>> On 5/5/20 3:22 PM, Alex Kiernan wrote:
>  On Mon, May 4, 2020 at 12:28 PM Tom Rini 
>   wrote:
> >
> > On Fri, May 01, 2020 at 05:40:25PM +0200, Marek Vasut wrote:
> >
> >> There is no reason to tail-pad fitImage with external data 
> >> to 4-bytes,
> >> while fitImage without external data does not have any 
> >> such padding and
> >> is often unaligned. DT spec also does not mandate any such 
> >> padding.
> >>
> >> Moreover, the tail-pad fills the last few bytes with 
> >> uninitialized data,
> >> which could lead to a potential information leak.
> >>
> >> $ echo -n xy > /tmp/data ; \
> >>   ./tools/mkimage -E -f auto -d /tmp/data 
> >> /tmp/fitImage ; \
> >>   hexdump -vC /tmp/fitImage | tail -n 3
> >>
> >> before:
> >> 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69 
> >>  |a-offset.data-si|
> >> 0270  7a 65 00 00 78 79 64 64  
> >>  |ze..xydd|
> >>^^   ^^ ^^
> >> after:
> >> 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69 
> >>  |a-offset.data-si|
> >> 0270  7a 65 00 78 79   
> >>  |ze.xy|
> >>
> >> Signed-off-by: Marek Vasut 
> >> Reviewed-by: Simon Glass 
> >> Cc: Heinrich Schuchardt 
> >> Cc: Tom Rini 
> >
> > Applied to u-boot/master, thanks!
> >
> 
>  This breaks booting on my board (am3352, eMMC boot, FIT 
>  u-boot,
>  CONFIG_SPL_LOAD_FIT). Not got any useful diagnostics - if I 
>  boot it
>  from eMMC I get nothing at all on the console, if I boot 
>  over ymodem
>  it stalls at 420k, before continuing to 460k. My guess is 
>  there's some
>  error going to the console at the 420k mark, but obviously 
>  it's lost
>  in the ymodem... I have two DTBs in the FIT image, 420k 
>  would about
>  align to the point between them.
> >>>
> >>> My bet would be on some padding / unaligned access problem 
> >>> that this
> >>> patch uncovered. Can you take a look ?
> >>
> >> Seems plausible. With this change my external data starts at 
> >> 0x483 and
> >> everything after it is non-aligned:
> >
> > Should the beginning of external data be aligned ?
> 
>  If in U-Boot we revert e8c2d25845c72c7202a628a97d45e31beea40668 
>  does
>  the
>  problem go away?  If so, that's not a fix outright, it means we 
>  need
>  to
>  dig back in to the libfdt thread and find the "make this work 
>  without
>  killing performance everywhere all the time" option.
> >>>
> >>> If it is a device tree, it must be 32-bit aligned.
> >>
> >> This commit actually breaks my board too (which I was just about 
> >> to send
> >> upstream, but realized it was broken).
> >>
> >> Said board uses SPL and main U-Boot. SPL runs fine and main u-boot 
> >> doesn't
> >> output anything. The only difference which I 

[PATCH] lib: rsa: Fix unaligned 64-bit fdt accesses

2020-05-06 Thread Jan Kiszka
From: Jan Kiszka 

The fdt only provides 32-bit alignment of data. If the public_exponent
happens to be not 64-bit aligned, we can trigger an exception on certain
architectures. Seen on TI AM64x.

Note that the normal way of accessing such a number would be
fdtdec_get_number. However, this is not available for tools, and this
is one use case for lib/rsa.

Signed-off-by: Jan Kiszka 
---
 lib/rsa/rsa-mod-exp.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/rsa/rsa-mod-exp.c b/lib/rsa/rsa-mod-exp.c
index 420ab2eba0..4b9c4b1459 100644
--- a/lib/rsa/rsa-mod-exp.c
+++ b/lib/rsa/rsa-mod-exp.c
@@ -246,6 +246,11 @@ static void rsa_convert_big_endian(uint32_t *dst, const 
uint32_t *src, int len)
dst[i] = fdt32_to_cpu(src[len - 1 - i]);
 }
 
+static uint64_t fdt64_get(const uint32_t *data)
+{
+   return ((uint64_t)fdt32_to_cpu(data[0]) << 32) | fdt32_to_cpu(data[1]);
+}
+
 int rsa_mod_exp_sw(const uint8_t *sig, uint32_t sig_len,
struct key_prop *prop, uint8_t *out)
 {
@@ -262,8 +267,7 @@ int rsa_mod_exp_sw(const uint8_t *sig, uint32_t sig_len,
if (!prop->public_exponent)
key.exponent = RSA_DEFAULT_PUBEXP;
else
-   key.exponent =
-   fdt64_to_cpu(*((uint64_t *)(prop->public_exponent)));
+   key.exponent = fdt64_get(prop->public_exponent);
 
if (!key.len || !prop->modulus || !prop->rr) {
debug("%s: Missing RSA key info", __func__);
-- 
2.26.1


[PATCH 1/2] test: describe naming conventions for macro UNIT_TEST

2020-05-06 Thread Heinrich Schuchardt
Strict naming conventions have to be followed for Python function
generate_ut_subtest() to collect C unit tests to be executed via
command 'ut'.

Describe the requirements both on the C as well on the Python side.

Signed-off-by: Heinrich Schuchardt 
---
 include/test/test.h  | 24 +++-
 test/py/tests/test_ut.py | 17 -
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/include/test/test.h b/include/test/test.h
index 2a75211008..029288de88 100644
--- a/include/test/test.h
+++ b/include/test/test.h
@@ -41,7 +41,29 @@ struct unit_test {
int flags;
 };

-/* Declare a new unit test */
+/**
+ * UNIT_TEST() - create linker generated list entry for unit a unit test
+ *
+ * The macro UNIT_TEST() is used to create a linker generated list entry. These
+ * list entries are enumerate tests that can be execute using the ut command.
+ * The list entries are used both by the implementation of the ut command as
+ * well as in a related Python test.
+ *
+ * For Python testing the subtests are collected in Python function
+ * generate_ut_subtest() by applying a regular expression to the lines of file
+ * u-boot.sym. The list entries have to follow strict naming conventions to be
+ * matched by the expression.
+ *
+ * Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in test suite
+ * foo that can be executed via command 'ut foo bar' and is implemented in
+ * function foo_test_bar().
+ *
+ * @_name: concatenation of name of the test suite, "_test_", and the name
+ * of the test
+ * @_flags:an integer field that can be evaluated by the test suite
+ * implementation
+ * @_suite:name of the test suite concatenated with "_test"
+ */
 #define UNIT_TEST(_name, _flags, _suite)   \
ll_entry_declare(struct unit_test, _name, _suite) = {   \
.file = __FILE__,   \
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index 6c7b8dd2b3..01c2b3ffa1 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -22,7 +22,22 @@ def test_ut_dm_init(u_boot_console):
 fh.write(data)

 def test_ut(u_boot_console, ut_subtest):
-"""Execute a "ut" subtest."""
+"""Execute a "ut" subtest.
+
+The subtests are collected in function generate_ut_subtest() from linker
+generated lists by applying a regular expression to the lines of file
+u-boot.sym. The list entries are created using the C macro UNIT_TEST().
+
+Strict naming conventions have to be followed to match the regular
+expression. Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in
+test suite foo that can be executed via command 'ut foo bar' and is
+implemented in C function foo_test_bar().
+
+Args:
+u_boot_console (ConsoleBase): U-Boot console
+ut_subtest (str): test to be executed via command ut, e.g 'foo bar' to
+execute command 'ut foo bar'
+"""

 output = u_boot_console.run_command('ut ' + ut_subtest)
 assert output.endswith('Failures: 0')
--
2.26.2



[PATCH 0/2] test: fix naming of test functions in the log test suite

2020-05-06 Thread Heinrich Schuchardt
Some Python subtests executed via the 'ut' console command were not
executed due to not following a required naming convention.

Add a description of the naming convention.
Fix the tests.

Heinrich Schuchardt (2):
  test: describe naming conventions for macro UNIT_TEST
  test: fix naming of test functions in the log test suite

 include/test/test.h  | 24 +++-
 test/log/nolog_test.c| 24 ++--
 test/log/syslog_test.c   | 48 
 test/py/tests/test_ut.py | 17 +-
 4 files changed, 75 insertions(+), 38 deletions(-)

--
2.26.2



[PATCH 2/2] test: fix naming of test functions in the log test suite

2020-05-06 Thread Heinrich Schuchardt
Both the nolog as well as the syslog tests were not found by Python
function generate_ut_subtest() due to not following the nameing
requirements imposed by the regular expression used to find linker
generated list entries in file u-boot.sym.

Adjust the naming of test functions.

With the patch the following tests are executed successfully for
sandbox_defconfig:

test/py/tests/test_ut.py::test_ut[ut_log_syslog_debug] PASSED
test/py/tests/test_ut.py::test_ut[ut_log_syslog_err] PASSED
test/py/tests/test_ut.py::test_ut[ut_log_syslog_info] PASSED
test/py/tests/test_ut.py::test_ut[ut_log_syslog_nodebug] PASSED
test/py/tests/test_ut.py::test_ut[ut_log_syslog_notice] PASSED
test/py/tests/test_ut.py::test_ut[ut_log_syslog_warning] PASSED

The nolog tests are only executed if CONFIG_LOG=n and
CONFIG_CONSOLE_RECORD=y.

Reported-by: Simon Glass 
Signed-off-by: Heinrich Schuchardt 
---
 test/log/nolog_test.c  | 24 ++---
 test/log/syslog_test.c | 48 +-
 2 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/test/log/nolog_test.c b/test/log/nolog_test.c
index 84619521c9..c418ed07c9 100644
--- a/test/log/nolog_test.c
+++ b/test/log/nolog_test.c
@@ -19,7 +19,7 @@ DECLARE_GLOBAL_DATA_PTR;

 #define BUFFSIZE 32

-static int nolog_test_log_err(struct unit_test_state *uts)
+static int log_test_nolog_err(struct unit_test_state *uts)
 {
char buf[BUFFSIZE];

@@ -31,9 +31,9 @@ static int nolog_test_log_err(struct unit_test_state *uts)
ut_assertok(ut_check_console_end(uts));
return 0;
 }
-LOG_TEST(nolog_test_log_err);
+LOG_TEST(log_test_nolog_err);

-static int nolog_test_log_warning(struct unit_test_state *uts)
+static int log_test_nolog_warning(struct unit_test_state *uts)
 {
char buf[BUFFSIZE];

@@ -45,9 +45,9 @@ static int nolog_test_log_warning(struct unit_test_state *uts)
ut_assertok(ut_check_console_end(uts));
return 0;
 }
-LOG_TEST(nolog_test_log_warning);
+LOG_TEST(log_test_nolog_warning);

-static int nolog_test_log_notice(struct unit_test_state *uts)
+static int log_test_nolog_notice(struct unit_test_state *uts)
 {
char buf[BUFFSIZE];

@@ -59,9 +59,9 @@ static int nolog_test_log_notice(struct unit_test_state *uts)
ut_assertok(ut_check_console_end(uts));
return 0;
 }
-LOG_TEST(nolog_test_log_notice);
+LOG_TEST(log_test_nolog_notice);

-static int nolog_test_log_info(struct unit_test_state *uts)
+static int log_test_nolog_info(struct unit_test_state *uts)
 {
char buf[BUFFSIZE];

@@ -73,7 +73,7 @@ static int nolog_test_log_info(struct unit_test_state *uts)
ut_assertok(ut_check_console_end(uts));
return 0;
 }
-LOG_TEST(nolog_test_log_info);
+LOG_TEST(log_test_nolog_info);

 #undef _DEBUG
 #define _DEBUG 0
@@ -90,7 +90,7 @@ static int nolog_test_nodebug(struct unit_test_state *uts)
 }
 LOG_TEST(nolog_test_nodebug);

-static int nolog_test_log_nodebug(struct unit_test_state *uts)
+static int log_test_nolog_nodebug(struct unit_test_state *uts)
 {
char buf[BUFFSIZE];

@@ -102,7 +102,7 @@ static int nolog_test_log_nodebug(struct unit_test_state 
*uts)
ut_assertok(ut_check_console_end(uts));
return 0;
 }
-LOG_TEST(nolog_test_log_nodebug);
+LOG_TEST(log_test_nolog_nodebug);

 #undef _DEBUG
 #define _DEBUG 1
@@ -120,7 +120,7 @@ static int nolog_test_debug(struct unit_test_state *uts)
 }
 LOG_TEST(nolog_test_debug);

-static int nolog_test_log_debug(struct unit_test_state *uts)
+static int log_test_nolog_debug(struct unit_test_state *uts)
 {
char buf[BUFFSIZE];

@@ -132,4 +132,4 @@ static int nolog_test_log_debug(struct unit_test_state *uts)
ut_assertok(ut_check_console_end(uts));
return 0;
 }
-LOG_TEST(nolog_test_log_debug);
+LOG_TEST(log_test_nolog_debug);
diff --git a/test/log/syslog_test.c b/test/log/syslog_test.c
index 6ca5760eac..26536ebca7 100644
--- a/test/log/syslog_test.c
+++ b/test/log/syslog_test.c
@@ -92,12 +92,12 @@ static int sb_log_tx_handler(struct udevice *dev, void 
*packet,
 }

 /**
- * syslog_test_log_err() - test log_err() function
+ * log_test_syslog_err() - test log_err() function
  *
  * @uts:   unit test state
  * Return: 0 = success
  */
-static int syslog_test_log_err(struct unit_test_state *uts)
+static int log_test_syslog_err(struct unit_test_state *uts)
 {
int old_log_level = gd->default_log_level;
struct sb_log_env env;
@@ -106,7 +106,7 @@ static int syslog_test_log_err(struct unit_test_state *uts)
gd->default_log_level = LOGL_INFO;
env_set("ethact", "eth@10002000");
env_set("log_hostname", "sandbox");
-   env.expected = "<3>sandbox uboot: syslog_test_log_err() "
+   env.expected = "<3>sandbox uboot: log_test_syslog_err() "
   "testing log_err\n";
env.uts = uts;
sandbox_eth_set_tx_handler(0, sb_log_tx_handler);
@@ -119,15 +119,15 @@ static int syslog_test_log_err(struct unit_test_state 
*uts)


Re: [PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

2020-05-06 Thread Marek Vasut
On 5/6/20 5:31 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 05:23:55PM +0200, Marek Vasut wrote:
>> On 5/6/20 5:19 PM, Tom Rini wrote:
>>> On Wed, May 06, 2020 at 05:08:12PM +0200, Marek Vasut wrote:
 On 5/6/20 5:05 PM, Tom Rini wrote:
> This has been reported to break booting of U-Boot from SPL on a number
> of platforms due to a lack of alignment of the external data.  The
> issues this commit is addressing will need to be resolved another way.
>
> This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
>
> Reported-by: Alex Kiernan 
> Reported-by: Michael Walle 
> Signed-off-by: Tom Rini 

 The commit message should also warn that this re-opens the data leak in
 the padding.
>>>
>>> Sure.  Are you going to send a patch switching to calloc or should I?
>>
>> I think the discussion about the padding requirement isn't finished yet?
>> See my remark about the load = <>; part.
> 
> I'm unsure what more you need to provide a potential fix to the
> platforms that have reported breakage.  I suspect you might even have a
> platform handy that is broken, given the imx platform.

Let's continue the discussion in one thread.

I think we should only ever enforce the padding if really required.
Apply this one and add the calloc fix, but we should fix the root cause,
not go back to papering over this problem.


Re: [PATCH V2] mkimage: fit: Do not tail-pad fitImage with external data

2020-05-06 Thread Marek Vasut
On 5/6/20 6:04 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 05:52:45PM +0200, Marek Vasut wrote:
>> On 5/6/20 5:43 PM, Alex Kiernan wrote:
>>> On Wed, May 6, 2020 at 3:41 PM Marek Vasut  wrote:

 On 5/6/20 4:37 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 04:33:37PM +0200, Marek Vasut wrote:
>> On 5/6/20 4:27 PM, Tom Rini wrote:
>>> On Wed, May 06, 2020 at 04:17:35PM +0200, Marek Vasut wrote:
 On 5/6/20 3:48 PM, Tom Rini wrote:
> On Tue, May 05, 2020 at 11:17:19PM +0200, Michael Walle wrote:
>> Hi all,
>>
>> Am 2020-05-05 20:41, schrieb Simon Glass:
>>> Hi Tom,
>>>
>>> On Tue, 5 May 2020 at 11:50, Tom Rini  wrote:

 On Tue, May 05, 2020 at 06:39:58PM +0200, Marek Vasut wrote:
> On 5/5/20 6:37 PM, Alex Kiernan wrote:
>> On Tue, May 5, 2020 at 2:28 PM Marek Vasut  wrote:
>>>
>>> On 5/5/20 3:22 PM, Alex Kiernan wrote:
 On Mon, May 4, 2020 at 12:28 PM Tom Rini  
 wrote:
>
> On Fri, May 01, 2020 at 05:40:25PM +0200, Marek Vasut wrote:
>
>> There is no reason to tail-pad fitImage with external data 
>> to 4-bytes,
>> while fitImage without external data does not have any such 
>> padding and
>> is often unaligned. DT spec also does not mandate any such 
>> padding.
>>
>> Moreover, the tail-pad fills the last few bytes with 
>> uninitialized data,
>> which could lead to a potential information leak.
>>
>> $ echo -n xy > /tmp/data ; \
>>   ./tools/mkimage -E -f auto -d /tmp/data /tmp/fitImage 
>> ; \
>>   hexdump -vC /tmp/fitImage | tail -n 3
>>
>> before:
>> 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69  
>> |a-offset.data-si|
>> 0270  7a 65 00 00 78 79 64 64   
>> |ze..xydd|
>>^^   ^^ ^^
>> after:
>> 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69  
>> |a-offset.data-si|
>> 0270  7a 65 00 78 79
>> |ze.xy|
>>
>> Signed-off-by: Marek Vasut 
>> Reviewed-by: Simon Glass 
>> Cc: Heinrich Schuchardt 
>> Cc: Tom Rini 
>
> Applied to u-boot/master, thanks!
>

 This breaks booting on my board (am3352, eMMC boot, FIT u-boot,
 CONFIG_SPL_LOAD_FIT). Not got any useful diagnostics - if I 
 boot it
 from eMMC I get nothing at all on the console, if I boot over 
 ymodem
 it stalls at 420k, before continuing to 460k. My guess is 
 there's some
 error going to the console at the 420k mark, but obviously 
 it's lost
 in the ymodem... I have two DTBs in the FIT image, 420k would 
 about
 align to the point between them.
>>>
>>> My bet would be on some padding / unaligned access problem that 
>>> this
>>> patch uncovered. Can you take a look ?
>>
>> Seems plausible. With this change my external data starts at 
>> 0x483 and
>> everything after it is non-aligned:
>
> Should the beginning of external data be aligned ?

 If in U-Boot we revert e8c2d25845c72c7202a628a97d45e31beea40668 
 does
 the
 problem go away?  If so, that's not a fix outright, it means we 
 need
 to
 dig back in to the libfdt thread and find the "make this work 
 without
 killing performance everywhere all the time" option.
>>>
>>> If it is a device tree, it must be 32-bit aligned.
>>
>> This commit actually breaks my board too (which I was just about to 
>> send
>> upstream, but realized it was broken).
>>
>> Said board uses SPL and main U-Boot. SPL runs fine and main u-boot 
>> doesn't
>> output anything. The only difference which I found is that 
>> fit-dtb.blob is
>> 2 bytes shorter. And the content is shifted by one byte although
>> data-offset is the same. Strange. In the non-working case, the inner
>> FDT magic isn't 4 byte aligned.
>>
>> You can find the two fit-dtb.blobs here:

Re: [PATCH 10/13] imx: load calibration parameters from fuse for i.MX8MP

2020-05-06 Thread Fabio Estevam
Hi Peng,

On Mon, May 4, 2020 at 11:50 PM Peng Fan  wrote:

> Busfreq could be disabled by set the device tree node to disabled.

This does not help. If I use the NXP U-Boot I can boot the NXP 5.4.3
without problems.

> > I have also tried to boot NXP 5.4.3 on a i.MX8MM EVK with the latest U-Boot
> > and I also observe a hang.
>
> I'll give a try tomorrow when I back to office. There is a ARM clock issue 
> that
> could cause all i.MX8M kernel hang, but that should be a bit late, should not
> be that early just after console enabled.

Currently, we can not even boot to the U-Boot prompt with mainline U-Boot.

Reverting f24dea4e1b ("ARM: imx8m: Fix reset in SPL on NXP iMX8MP
EVK") allows it to boot again.

As discussed in the other thread, this means that the i.MX8MP clock
driver is broken.

Please let me know if you can look at these issues.

Thanks


Re: [PATCH 30/36] bdinfo: arm: Move ARM-specific info into its own file

2020-05-06 Thread Daniel Schwierzeck



Am 05.05.20 um 01:17 schrieb Simon Glass:
> We don't really want to have ARM-specific code in a generic file. Create
> a new arch-specific function to hold it, and move it into that.
> 
> Make the function weak so that any arch can implement it.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  arch/arm/lib/Makefile |  1 +
>  arch/arm/lib/bdinfo.c | 51 +++
>  cmd/bdinfo.c  | 44 +
>  include/init.h|  3 +++
>  4 files changed, 61 insertions(+), 38 deletions(-)
>  create mode 100644 arch/arm/lib/bdinfo.c
> 
> diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
> index b839aa7a50..27b12e7f2b 100644
> --- a/arch/arm/lib/Makefile
> +++ b/arch/arm/lib/Makefile
> @@ -43,6 +43,7 @@ obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMSET) += memset.o
>  obj-$(CONFIG_$(SPL_TPL_)USE_ARCH_MEMCPY) += memcpy.o
>  obj-$(CONFIG_SEMIHOSTING) += semihosting.o
>  
> +obj-y+= bdinfo.o
>  obj-y+= sections.o
>  obj-y+= stack.o
>  ifdef CONFIG_CPU_V7M
> diff --git a/arch/arm/lib/bdinfo.c b/arch/arm/lib/bdinfo.c
> new file mode 100644
> index 00..ce8edd0313
> --- /dev/null
> +++ b/arch/arm/lib/bdinfo.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * ARM-specific information for the 'bd' command
> + *
> + * (C) Copyright 2003
> + * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
> + */
> +
> +#include 
> +#include 
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +void arch_print_bdinfo(void)
> +{
> +#ifdef CONFIG_SYS_MEM_RESERVE_SECURE
> + if (gd->arch.secure_ram & MEM_RESERVE_SECURE_SECURED) {
> + bdinfo_print_num("Secure ram",
> +  gd->arch.secure_ram &
> +  MEM_RESERVE_SECURE_ADDR_MASK);
> + }
> +#endif
> +#ifdef CONFIG_RESV_RAM
> + if (gd->arch.resv_ram)
> + bdinfo_print_num("Reserved ram", gd->arch.resv_ram);
> +#endif
> +#if !(CONFIG_IS_ENABLED(SYS_ICACHE_OFF) && CONFIG_IS_ENABLED(SYS_DCACHE_OFF))
> + bdinfo_print_num("TLB addr", gd->arch.tlb_addr);
> +#endif
> + bdinfo_print_num("irq_sp", gd->irq_sp); /* irq stack pointer */
> + bdinfo_print_num("sp start ", gd->start_addr_sp);
> + /*
> +  * TODO: Currently only support for davinci SOC's is added.
> +  * Remove this check once all the board implement this.
> +  */
> +#ifdef CONFIG_CLOCKS
> + printf("ARM frequency = %ld MHz\n", gd->bd->bi_arm_freq);
> + printf("DSP frequency = %ld MHz\n", gd->bd->bi_dsp_freq);
> + printf("DDR frequency = %ld MHz\n", gd->bd->bi_ddr_freq);
> +#endif
> +#ifdef CONFIG_BOARD_TYPES
> + printf("Board Type  = %ld\n", gd->board_type);
> +#endif



> +#if CONFIG_VAL(SYS_MALLOC_F_LEN)
> + printf("Early malloc usage: %lx / %x\n", gd->malloc_ptr,
> +CONFIG_VAL(SYS_MALLOC_F_LEN));
> +#endif
> +#if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
> + bdinfo_print_num("multi_dtb_fit", (ulong)gd->multi_dtb_fit);
> +#endif
> +}

that part is not really ARM specific. Maybe it's useful in the generic
function?

-- 
- Daniel


[PATCH] misc: i2c_eeprom: implement different probe test eeprom offset

2020-05-06 Thread Eugen Hristev
Because of this commit :
5ae84860b0 ("misc: i2c_eeprom: verify that the chip is functional at probe()")
at probe time, each eeprom is tested for read at offset 0.

The Atmel AT24MAC402 eeprom has different mapping. One i2c slave address is
used for the lower 0x80 bytes and another i2c slave address is used for the
upper 0x80 bytes. Because of this basically the i2c master sees 2 different
slaves. We need the upper bytes because we read the unique MAC address from
this EEPROM area.

However this implies that our slave address will return error on reads
from address 0x0 to 0x80.

To solve this, implemented a way to figure out from the compatible udevice_id
'data' field which offset we should attempt to read.
Added the offset as one byte in the data field (byte number 3).
Created two macros that will convert offset to 'data' field and back.

The probe function will now read this offset and use it, instead of blindly
checking offset 0.

This will fix the regression noticed on these EEPROMs since the commit
abovementioned that introduces the probe failed issue.

The offset field can for sure be updated later at a 2byte offset if needed
by anyone.

Signed-off-by: Eugen Hristev 
---
 drivers/misc/i2c_eeprom.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/i2c_eeprom.c b/drivers/misc/i2c_eeprom.c
index 3755dbf74b..79805cc46e 100644
--- a/drivers/misc/i2c_eeprom.c
+++ b/drivers/misc/i2c_eeprom.c
@@ -11,6 +11,13 @@
 #include 
 #include 
 
+/* These macros are used to encode/decode the starting EEPROM offset into the
+ * udevice_id structure's data field 3rd byte.
+ * Lower 2 bytes of the data field are used for pagewidth.
+ */
+#define I2C_EEPROM_OFFSET_TO_DATA(v) ((v) << 16)
+#define I2C_EEPROM_DATA_TO_OFFSET(v) ((v) >> 16)
+
 int i2c_eeprom_read(struct udevice *dev, int offset, uint8_t *buf, int size)
 {
const struct i2c_eeprom_ops *ops = device_get_ops(dev);
@@ -85,11 +92,17 @@ static int i2c_eeprom_std_ofdata_to_platdata(struct udevice 
*dev)
 
 static int i2c_eeprom_std_probe(struct udevice *dev)
 {
+   u64 data = dev_get_driver_data(dev);
u8 test_byte;
int ret;
 
/* Verify that the chip is functional */
-   ret = i2c_eeprom_read(dev, 0, _byte, 1);
+   /*
+* Not all eeproms start from offset 0. Valid offset is encoded in
+* upper bits of the data (byte 3).
+*/
+   ret = i2c_eeprom_read(dev, I2C_EEPROM_DATA_TO_OFFSET(data) & 0xFF,
+ _byte, 1);
if (ret)
return -ENODEV;
 
@@ -105,7 +118,8 @@ static const struct udevice_id i2c_eeprom_std_ids[] = {
{ .compatible = "atmel,24c08", .data = 4 },
{ .compatible = "atmel,24c08a", .data = 4 },
{ .compatible = "atmel,24c16a", .data = 4 },
-   { .compatible = "atmel,24mac402", .data = 4 },
+   { .compatible = "atmel,24mac402",
+   .data = (4 | I2C_EEPROM_OFFSET_TO_DATA(0x80))},
{ .compatible = "atmel,24c32", .data = 5 },
{ .compatible = "atmel,24c64", .data = 5 },
{ .compatible = "atmel,24c128", .data = 6 },
-- 
2.25.1



Re: [PATCH V2] mkimage: fit: Do not tail-pad fitImage with external data

2020-05-06 Thread Tom Rini
On Wed, May 06, 2020 at 05:52:45PM +0200, Marek Vasut wrote:
> On 5/6/20 5:43 PM, Alex Kiernan wrote:
> > On Wed, May 6, 2020 at 3:41 PM Marek Vasut  wrote:
> >>
> >> On 5/6/20 4:37 PM, Tom Rini wrote:
> >>> On Wed, May 06, 2020 at 04:33:37PM +0200, Marek Vasut wrote:
>  On 5/6/20 4:27 PM, Tom Rini wrote:
> > On Wed, May 06, 2020 at 04:17:35PM +0200, Marek Vasut wrote:
> >> On 5/6/20 3:48 PM, Tom Rini wrote:
> >>> On Tue, May 05, 2020 at 11:17:19PM +0200, Michael Walle wrote:
>  Hi all,
> 
>  Am 2020-05-05 20:41, schrieb Simon Glass:
> > Hi Tom,
> >
> > On Tue, 5 May 2020 at 11:50, Tom Rini  wrote:
> >>
> >> On Tue, May 05, 2020 at 06:39:58PM +0200, Marek Vasut wrote:
> >>> On 5/5/20 6:37 PM, Alex Kiernan wrote:
>  On Tue, May 5, 2020 at 2:28 PM Marek Vasut  wrote:
> >
> > On 5/5/20 3:22 PM, Alex Kiernan wrote:
> >> On Mon, May 4, 2020 at 12:28 PM Tom Rini  
> >> wrote:
> >>>
> >>> On Fri, May 01, 2020 at 05:40:25PM +0200, Marek Vasut wrote:
> >>>
>  There is no reason to tail-pad fitImage with external data 
>  to 4-bytes,
>  while fitImage without external data does not have any such 
>  padding and
>  is often unaligned. DT spec also does not mandate any such 
>  padding.
> 
>  Moreover, the tail-pad fills the last few bytes with 
>  uninitialized data,
>  which could lead to a potential information leak.
> 
>  $ echo -n xy > /tmp/data ; \
>    ./tools/mkimage -E -f auto -d /tmp/data /tmp/fitImage 
>  ; \
>    hexdump -vC /tmp/fitImage | tail -n 3
> 
>  before:
>  0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69  
>  |a-offset.data-si|
>  0270  7a 65 00 00 78 79 64 64   
>  |ze..xydd|
> ^^   ^^ ^^
>  after:
>  0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69  
>  |a-offset.data-si|
>  0270  7a 65 00 78 79
>  |ze.xy|
> 
>  Signed-off-by: Marek Vasut 
>  Reviewed-by: Simon Glass 
>  Cc: Heinrich Schuchardt 
>  Cc: Tom Rini 
> >>>
> >>> Applied to u-boot/master, thanks!
> >>>
> >>
> >> This breaks booting on my board (am3352, eMMC boot, FIT u-boot,
> >> CONFIG_SPL_LOAD_FIT). Not got any useful diagnostics - if I 
> >> boot it
> >> from eMMC I get nothing at all on the console, if I boot over 
> >> ymodem
> >> it stalls at 420k, before continuing to 460k. My guess is 
> >> there's some
> >> error going to the console at the 420k mark, but obviously 
> >> it's lost
> >> in the ymodem... I have two DTBs in the FIT image, 420k would 
> >> about
> >> align to the point between them.
> >
> > My bet would be on some padding / unaligned access problem that 
> > this
> > patch uncovered. Can you take a look ?
> 
>  Seems plausible. With this change my external data starts at 
>  0x483 and
>  everything after it is non-aligned:
> >>>
> >>> Should the beginning of external data be aligned ?
> >>
> >> If in U-Boot we revert e8c2d25845c72c7202a628a97d45e31beea40668 
> >> does
> >> the
> >> problem go away?  If so, that's not a fix outright, it means we 
> >> need
> >> to
> >> dig back in to the libfdt thread and find the "make this work 
> >> without
> >> killing performance everywhere all the time" option.
> >
> > If it is a device tree, it must be 32-bit aligned.
> 
>  This commit actually breaks my board too (which I was just about to 
>  send
>  upstream, but realized it was broken).
> 
>  Said board uses SPL and main U-Boot. SPL runs fine and main u-boot 
>  doesn't
>  output anything. The only difference which I found is that 
>  fit-dtb.blob is
>  2 bytes shorter. And the content is shifted by one byte although
>  data-offset is the same. Strange. In the non-working case, the inner
>  FDT magic isn't 4 byte aligned.
> 
>  You can find the two fit-dtb.blobs here:
> 
>  

Re: [PATCH 07/36] bdinfo: microblaze: Use the generic bd command

2020-05-06 Thread Daniel Schwierzeck



Am 05.05.20 um 01:17 schrieb Simon Glass:
> Microblaze prints out ethernet and FDT information. This is useful to
> most archs, so move it into the generic code and move microblaze over to
> use it.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  cmd/bdinfo.c | 30 +++---
>  1 file changed, 7 insertions(+), 23 deletions(-)
> 
> diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
> index 405a915861..dc5a09f8ce 100644
> --- a/cmd/bdinfo.c
> +++ b/cmd/bdinfo.c
> @@ -224,29 +224,7 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char 
> * const argv[])
>  
>  #elif defined(CONFIG_MICROBLAZE)
>  
> -int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> -{
> - bd_t *bd = gd->bd;
> -
> - print_bi_dram(bd);
> - print_bi_flash(bd);
> -#if defined(CONFIG_SYS_SRAM_BASE)
> - print_num("sram start ",(ulong)bd->bi_sramstart);
> - print_num("sram size  ",(ulong)bd->bi_sramsize);
> -#endif
> -#if defined(CONFIG_CMD_NET) && !defined(CONFIG_DM_ETH)
> - print_eths();
> -#endif
> - print_baudrate();
> - print_num("relocaddr", gd->relocaddr);
> - print_num("reloc off", gd->reloc_off);
> - print_num("fdt_blob", (ulong)gd->fdt_blob);
> - print_num("new_fdt", (ulong)gd->new_fdt);
> - print_num("fdt_size", (ulong)gd->fdt_size);
> - print_cpu_word_size();
> -
> - return 0;
> -}
> +#define USE_GENERIC
>  
>  #elif defined(CONFIG_M68K)
>  
> @@ -463,6 +441,12 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char 
> *const argv[])
>   print_num("relocaddr", gd->relocaddr);
>   print_num("reloc off", gd->reloc_off);
>   print_cpu_word_size();
> +#if defined(CONFIG_CMD_NET) && !defined(CONFIG_DM_ETH)
> + print_eths();
> +#endif
> + print_num("fdt_blob", (ulong)gd->fdt_blob);
> + print_num("new_fdt", (ulong)gd->new_fdt);
> + print_num("fdt_size", (ulong)gd->fdt_size);

shouldn't that be guarded with CONFIG_IS_ENABLED(OF_LIBFDT) ?

>  
>   return 0;
>  }
> 

-- 
- Daniel


Re: [PATCH V2] mkimage: fit: Do not tail-pad fitImage with external data

2020-05-06 Thread Marek Vasut
On 5/6/20 5:43 PM, Alex Kiernan wrote:
> On Wed, May 6, 2020 at 3:41 PM Marek Vasut  wrote:
>>
>> On 5/6/20 4:37 PM, Tom Rini wrote:
>>> On Wed, May 06, 2020 at 04:33:37PM +0200, Marek Vasut wrote:
 On 5/6/20 4:27 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 04:17:35PM +0200, Marek Vasut wrote:
>> On 5/6/20 3:48 PM, Tom Rini wrote:
>>> On Tue, May 05, 2020 at 11:17:19PM +0200, Michael Walle wrote:
 Hi all,

 Am 2020-05-05 20:41, schrieb Simon Glass:
> Hi Tom,
>
> On Tue, 5 May 2020 at 11:50, Tom Rini  wrote:
>>
>> On Tue, May 05, 2020 at 06:39:58PM +0200, Marek Vasut wrote:
>>> On 5/5/20 6:37 PM, Alex Kiernan wrote:
 On Tue, May 5, 2020 at 2:28 PM Marek Vasut  wrote:
>
> On 5/5/20 3:22 PM, Alex Kiernan wrote:
>> On Mon, May 4, 2020 at 12:28 PM Tom Rini  
>> wrote:
>>>
>>> On Fri, May 01, 2020 at 05:40:25PM +0200, Marek Vasut wrote:
>>>
 There is no reason to tail-pad fitImage with external data to 
 4-bytes,
 while fitImage without external data does not have any such 
 padding and
 is often unaligned. DT spec also does not mandate any such 
 padding.

 Moreover, the tail-pad fills the last few bytes with 
 uninitialized data,
 which could lead to a potential information leak.

 $ echo -n xy > /tmp/data ; \
   ./tools/mkimage -E -f auto -d /tmp/data /tmp/fitImage ; \
   hexdump -vC /tmp/fitImage | tail -n 3

 before:
 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69  
 |a-offset.data-si|
 0270  7a 65 00 00 78 79 64 64   
 |ze..xydd|
^^   ^^ ^^
 after:
 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69  
 |a-offset.data-si|
 0270  7a 65 00 78 79
 |ze.xy|

 Signed-off-by: Marek Vasut 
 Reviewed-by: Simon Glass 
 Cc: Heinrich Schuchardt 
 Cc: Tom Rini 
>>>
>>> Applied to u-boot/master, thanks!
>>>
>>
>> This breaks booting on my board (am3352, eMMC boot, FIT u-boot,
>> CONFIG_SPL_LOAD_FIT). Not got any useful diagnostics - if I boot 
>> it
>> from eMMC I get nothing at all on the console, if I boot over 
>> ymodem
>> it stalls at 420k, before continuing to 460k. My guess is 
>> there's some
>> error going to the console at the 420k mark, but obviously it's 
>> lost
>> in the ymodem... I have two DTBs in the FIT image, 420k would 
>> about
>> align to the point between them.
>
> My bet would be on some padding / unaligned access problem that 
> this
> patch uncovered. Can you take a look ?

 Seems plausible. With this change my external data starts at 0x483 
 and
 everything after it is non-aligned:
>>>
>>> Should the beginning of external data be aligned ?
>>
>> If in U-Boot we revert e8c2d25845c72c7202a628a97d45e31beea40668 does
>> the
>> problem go away?  If so, that's not a fix outright, it means we need
>> to
>> dig back in to the libfdt thread and find the "make this work without
>> killing performance everywhere all the time" option.
>
> If it is a device tree, it must be 32-bit aligned.

 This commit actually breaks my board too (which I was just about to 
 send
 upstream, but realized it was broken).

 Said board uses SPL and main U-Boot. SPL runs fine and main u-boot 
 doesn't
 output anything. The only difference which I found is that 
 fit-dtb.blob is
 2 bytes shorter. And the content is shifted by one byte although
 data-offset is the same. Strange. In the non-working case, the inner
 FDT magic isn't 4 byte aligned.

 You can find the two fit-dtb.blobs here:

 https://walle.cc/u-boot/fit-dtb.blob.working
 https://walle.cc/u-boot/fit-dtb.blob.non-working


 Reverting e8c2d25845c72c7202a628a97d45e31beea40668 doesn't help (I 
 might
 reverted it the wrong way, there is actually a conflict).

 I'll dig deeper into that tomorrow, but maybe you have some 

Re: [PATCH V2] mkimage: fit: Do not tail-pad fitImage with external data

2020-05-06 Thread Alex Kiernan
On Wed, May 6, 2020 at 3:41 PM Marek Vasut  wrote:
>
> On 5/6/20 4:37 PM, Tom Rini wrote:
> > On Wed, May 06, 2020 at 04:33:37PM +0200, Marek Vasut wrote:
> >> On 5/6/20 4:27 PM, Tom Rini wrote:
> >>> On Wed, May 06, 2020 at 04:17:35PM +0200, Marek Vasut wrote:
>  On 5/6/20 3:48 PM, Tom Rini wrote:
> > On Tue, May 05, 2020 at 11:17:19PM +0200, Michael Walle wrote:
> >> Hi all,
> >>
> >> Am 2020-05-05 20:41, schrieb Simon Glass:
> >>> Hi Tom,
> >>>
> >>> On Tue, 5 May 2020 at 11:50, Tom Rini  wrote:
> 
>  On Tue, May 05, 2020 at 06:39:58PM +0200, Marek Vasut wrote:
> > On 5/5/20 6:37 PM, Alex Kiernan wrote:
> >> On Tue, May 5, 2020 at 2:28 PM Marek Vasut  wrote:
> >>>
> >>> On 5/5/20 3:22 PM, Alex Kiernan wrote:
>  On Mon, May 4, 2020 at 12:28 PM Tom Rini  
>  wrote:
> >
> > On Fri, May 01, 2020 at 05:40:25PM +0200, Marek Vasut wrote:
> >
> >> There is no reason to tail-pad fitImage with external data to 
> >> 4-bytes,
> >> while fitImage without external data does not have any such 
> >> padding and
> >> is often unaligned. DT spec also does not mandate any such 
> >> padding.
> >>
> >> Moreover, the tail-pad fills the last few bytes with 
> >> uninitialized data,
> >> which could lead to a potential information leak.
> >>
> >> $ echo -n xy > /tmp/data ; \
> >>   ./tools/mkimage -E -f auto -d /tmp/data /tmp/fitImage ; \
> >>   hexdump -vC /tmp/fitImage | tail -n 3
> >>
> >> before:
> >> 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69  
> >> |a-offset.data-si|
> >> 0270  7a 65 00 00 78 79 64 64   
> >> |ze..xydd|
> >>^^   ^^ ^^
> >> after:
> >> 0260  61 2d 6f 66 66 73 65 74  00 64 61 74 61 2d 73 69  
> >> |a-offset.data-si|
> >> 0270  7a 65 00 78 79
> >> |ze.xy|
> >>
> >> Signed-off-by: Marek Vasut 
> >> Reviewed-by: Simon Glass 
> >> Cc: Heinrich Schuchardt 
> >> Cc: Tom Rini 
> >
> > Applied to u-boot/master, thanks!
> >
> 
>  This breaks booting on my board (am3352, eMMC boot, FIT u-boot,
>  CONFIG_SPL_LOAD_FIT). Not got any useful diagnostics - if I boot 
>  it
>  from eMMC I get nothing at all on the console, if I boot over 
>  ymodem
>  it stalls at 420k, before continuing to 460k. My guess is 
>  there's some
>  error going to the console at the 420k mark, but obviously it's 
>  lost
>  in the ymodem... I have two DTBs in the FIT image, 420k would 
>  about
>  align to the point between them.
> >>>
> >>> My bet would be on some padding / unaligned access problem that 
> >>> this
> >>> patch uncovered. Can you take a look ?
> >>
> >> Seems plausible. With this change my external data starts at 0x483 
> >> and
> >> everything after it is non-aligned:
> >
> > Should the beginning of external data be aligned ?
> 
>  If in U-Boot we revert e8c2d25845c72c7202a628a97d45e31beea40668 does
>  the
>  problem go away?  If so, that's not a fix outright, it means we need
>  to
>  dig back in to the libfdt thread and find the "make this work without
>  killing performance everywhere all the time" option.
> >>>
> >>> If it is a device tree, it must be 32-bit aligned.
> >>
> >> This commit actually breaks my board too (which I was just about to 
> >> send
> >> upstream, but realized it was broken).
> >>
> >> Said board uses SPL and main U-Boot. SPL runs fine and main u-boot 
> >> doesn't
> >> output anything. The only difference which I found is that 
> >> fit-dtb.blob is
> >> 2 bytes shorter. And the content is shifted by one byte although
> >> data-offset is the same. Strange. In the non-working case, the inner
> >> FDT magic isn't 4 byte aligned.
> >>
> >> You can find the two fit-dtb.blobs here:
> >>
> >> https://walle.cc/u-boot/fit-dtb.blob.working
> >> https://walle.cc/u-boot/fit-dtb.blob.non-working
> >>
> >>
> >> Reverting e8c2d25845c72c7202a628a97d45e31beea40668 doesn't help (I 
> >> might
> >> reverted it the wrong way, there is actually a conflict).
> >>
> >> I'll dig deeper into that tomorrow, but maybe you have some pointers 
> >> where
> >> to look.
> 

Re: [PATCH 00/36] Tidy up the 'bd' command.

2020-05-06 Thread Simon Glass
Hi Alexey,


On Wed, 6 May 2020 at 03:28, Alexey Brodkin  wrote:
>
> Hi Simon,
>
> > -Original Message-
> > From: Simon Glass 
> > Sent: Tuesday, May 5, 2020 2:17 AM
> > To: U-Boot Mailing List 
>
> [snip]
>
> > Subject: [PATCH 00/36] Tidy up the 'bd' command.
> >
> > The code for the 'bd' command never got the 'generic board' treatment many
> > years ago when global_data and bd_info were converted. As a result it
> > still has a lot of arch-specific duplication of generic code.
> >
> > This series aims to make as much code in this file generic as possible, so
> > that it is easy to add new info on all architectures.
> >
> > For the three architectures that actually need additional code (ARM, PPC
> > and m68k) this is moved into arch-specific files.
> >
> > With this series, bdinfo.c drops from nearly 500 lines to just over 100.
> >
> > It also makes x86 report the frame buffer address properly (the original
> > goal of my effort).
> >
> > Simon Glass (36):
> >   bdinfo: nds32: Use generic bd_info
> >   bdinfo: riscv: Use generic bd_info
> >   bdinfo: m68k: Drop bd_info->bi_ipbfreq
> >   bdinfo: xtensa: Create a generic do_bdinfo for xtensa
> >   bdinfo: mips: Use the generic bd command
> >   bdinfo: nios2: Use the generic bd command
> >   bdinfo: microblaze: Use the generic bd command
> >   bdinfo: sh: Use the generic bd command
> >   bdinfo: x86: Use the generic bd command
> >   bdinfo: sandbox: Use the generic bd command
> >   bdinfo: nds32: Use the generic bd command
> >   bdinfo: riscv: Use the generic bd command
> >   bdinfo: arm: Use the generic bd command
>
> Looks like this one contains much more than just ARM changes.
> Instead it looks like a massive migration to a generic bdinfo.
> So I would propose either split it into per-arch changes
> (though given this series size it might not be a good idea) or
> alternatively maybe just ask for affected architecture maintainers
> for an ack?

This is not a very large migration, just a small change to one
command, mostly. We already have generic bdinfo (same 'struct bd') for
all but two archs.

The series is split by arch - see the tags for each commit. That is
why some maintainers get some patches and not others, or at least that
is my hope, since patman runs get_maintainers separately on each
commit.

>
> Also IMHO it would be very convenient to have a link to a git tree
> with all these changes - it will significantly simplify review process.

See u-boot-dm/bd-working for the tree. I always post a series there
before sending, but don't always mention the name, sorry.

> But regardless all my comments above I'd like to note that this series
> is very welcome as a long needed clean-up of that per-arch nonsense.
> Thanks for doing this significant and useful work!

:-)

Regards,
Simon


Re: [PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

2020-05-06 Thread Tom Rini
On Wed, May 06, 2020 at 05:23:55PM +0200, Marek Vasut wrote:
> On 5/6/20 5:19 PM, Tom Rini wrote:
> > On Wed, May 06, 2020 at 05:08:12PM +0200, Marek Vasut wrote:
> >> On 5/6/20 5:05 PM, Tom Rini wrote:
> >>> This has been reported to break booting of U-Boot from SPL on a number
> >>> of platforms due to a lack of alignment of the external data.  The
> >>> issues this commit is addressing will need to be resolved another way.
> >>>
> >>> This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
> >>>
> >>> Reported-by: Alex Kiernan 
> >>> Reported-by: Michael Walle 
> >>> Signed-off-by: Tom Rini 
> >>
> >> The commit message should also warn that this re-opens the data leak in
> >> the padding.
> > 
> > Sure.  Are you going to send a patch switching to calloc or should I?
> 
> I think the discussion about the padding requirement isn't finished yet?
> See my remark about the load = <>; part.

I'm unsure what more you need to provide a potential fix to the
platforms that have reported breakage.  I suspect you might even have a
platform handy that is broken, given the imx platform.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 05/36] bdinfo: mips: Use the generic bd command

2020-05-06 Thread Daniel Schwierzeck



Am 05.05.20 um 01:17 schrieb Simon Glass:
> MIPS currently has a few extra things which are generally useful. Add them
> to the generic function and move MIPS over to use it.
> 
> Signed-off-by: Simon Glass 
> ---
> 
>  cmd/bdinfo.c | 13 -
>  1 file changed, 4 insertions(+), 9 deletions(-)

Reviewed-by: Daniel Schwierzeck 

> 
> diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
> index 9247180a29..6ccbd2f50f 100644
> --- a/cmd/bdinfo.c
> +++ b/cmd/bdinfo.c
> @@ -299,15 +299,7 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char 
> * const argv[])
>  
>  #elif defined(CONFIG_MIPS)
>  
> -int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> -{
> - print_std_bdinfo(gd->bd);
> - print_num("relocaddr", gd->relocaddr);
> - print_num("reloc off", gd->reloc_off);
> - print_cpu_word_size();
> -
> - return 0;
> -}
> +#define USE_GENERIC
>  
>  #elif defined(CONFIG_ARM)
>  
> @@ -485,6 +477,9 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char 
> * const argv[])
>  int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char *const argv[])
>  {
>   print_std_bdinfo(gd->bd);
> + print_num("relocaddr", gd->relocaddr);
> + print_num("reloc off", gd->reloc_off);
> + print_cpu_word_size();
>  
>   return 0;
>  }
> 

-- 
- Daniel


Re: [PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

2020-05-06 Thread Marek Vasut
On 5/6/20 5:19 PM, Tom Rini wrote:
> On Wed, May 06, 2020 at 05:08:12PM +0200, Marek Vasut wrote:
>> On 5/6/20 5:05 PM, Tom Rini wrote:
>>> This has been reported to break booting of U-Boot from SPL on a number
>>> of platforms due to a lack of alignment of the external data.  The
>>> issues this commit is addressing will need to be resolved another way.
>>>
>>> This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
>>>
>>> Reported-by: Alex Kiernan 
>>> Reported-by: Michael Walle 
>>> Signed-off-by: Tom Rini 
>>
>> The commit message should also warn that this re-opens the data leak in
>> the padding.
> 
> Sure.  Are you going to send a patch switching to calloc or should I?

I think the discussion about the padding requirement isn't finished yet?
See my remark about the load = <>; part.


Re: [PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

2020-05-06 Thread Tom Rini
On Wed, May 06, 2020 at 05:08:12PM +0200, Marek Vasut wrote:
> On 5/6/20 5:05 PM, Tom Rini wrote:
> > This has been reported to break booting of U-Boot from SPL on a number
> > of platforms due to a lack of alignment of the external data.  The
> > issues this commit is addressing will need to be resolved another way.
> > 
> > This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
> > 
> > Reported-by: Alex Kiernan 
> > Reported-by: Michael Walle 
> > Signed-off-by: Tom Rini 
> 
> The commit message should also warn that this re-opens the data leak in
> the padding.

Sure.  Are you going to send a patch switching to calloc or should I?

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 3/3] lib: Allow hexdump to be used in SPL

2020-05-06 Thread Stefan Roese

On 06.05.20 16:03, Simon Glass wrote:

It is sometimes useful to output hex dumps in SPL. Add a config option to
allow this.

Signed-off-by: Simon Glass 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---

  lib/Kconfig  | 6 ++
  lib/Makefile | 2 +-
  2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/Kconfig b/lib/Kconfig
index c3f694afc0..df052e1d59 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -491,6 +491,12 @@ config HEXDUMP
help
  This enables functions for printing dumps of binary data.
  
+config SPL_HEXDUMP

+   bool "Enable hexdump in SPL"
+   help
+ This enables functions for printing dumps of binary data in
+ SPL.
+
  config OF_LIBFDT
bool "Enable the FDT library"
default y if OF_CONTROL
diff --git a/lib/Makefile b/lib/Makefile
index 6e688afa68..a022fb9455 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -104,7 +104,7 @@ obj-$(CONFIG_REGEX) += slre.o
  obj-y += string.o
  obj-y += tables_csum.o
  obj-y += time.o
-obj-y += hexdump.o
+obj-$(CONFIG_$(SPL_)HEXDUMP) += hexdump.o
  obj-$(CONFIG_TRACE) += trace.o
  obj-$(CONFIG_LIB_UUID) += uuid.o
  obj-$(CONFIG_LIB_RAND) += rand.o




Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH 33/36] bdinfo: m68k: ppc: Move arch-specific code from bdinfo

2020-05-06 Thread Stefan Roese

On 05.05.20 01:17, Simon Glass wrote:

We don't have an easy way to share these three lines of code with two
architectures. We also want to make it clear that this code is actually
arch-specific.

So just duplicate it in each arch-specific file.

Signed-off-by: Simon Glass 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---

  arch/m68k/lib/bdinfo.c| 5 +
  arch/powerpc/lib/bdinfo.c | 5 +
  cmd/bdinfo.c  | 8 
  3 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/arch/m68k/lib/bdinfo.c b/arch/m68k/lib/bdinfo.c
index 09a1abfc21..971c47c306 100644
--- a/arch/m68k/lib/bdinfo.c
+++ b/arch/m68k/lib/bdinfo.c
@@ -15,6 +15,11 @@ void arch_print_bdinfo(void)
  {
bd_t *bd = gd->bd;
  
+#if defined(CONFIG_SYS_INIT_RAM_ADDR)

+   bdinfo_print_num("sramstart", (ulong)bd->bi_sramstart);
+   bdinfo_print_num("sramsize", (ulong)bd->bi_sramsize);
+#endif
+   bdinfo_print_mhz("busfreq", bd->bi_busfreq);
  #if defined(CONFIG_SYS_MBAR)
bdinfo_print_num("mbar", bd->bi_mbar_base);
  #endif
diff --git a/arch/powerpc/lib/bdinfo.c b/arch/powerpc/lib/bdinfo.c
index da09bb276f..d8c64155f0 100644
--- a/arch/powerpc/lib/bdinfo.c
+++ b/arch/powerpc/lib/bdinfo.c
@@ -20,6 +20,11 @@ void arch_print_bdinfo(void)
  {
bd_t *bd = gd->bd;
  
+#if defined(CONFIG_SYS_INIT_RAM_ADDR)

+   bdinfo_print_num("sramstart", (ulong)bd->bi_sramstart);
+   bdinfo_print_num("sramsize", (ulong)bd->bi_sramsize);
+#endif
+   bdinfo_print_mhz("busfreq", bd->bi_busfreq);
  #if defined(CONFIG_MPC8xx) || defined(CONFIG_E500)
bdinfo_print_num("immr_base", bd->bi_immr_base);
  #endif
diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
index f0afdb153f..aea7cf904a 100644
--- a/cmd/bdinfo.c
+++ b/cmd/bdinfo.c
@@ -98,14 +98,6 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char 
*const argv[])
  
  	arch_print_bdinfo();
  
-	/* This is used by m68k and ppc */

-#if defined(CONFIG_SYS_INIT_RAM_ADDR)
-   bdinfo_print_num("sramstart", (ulong)bd->bi_sramstart);
-   bdinfo_print_num("sramsize", (ulong)bd->bi_sramsize);
-#endif
-   if (IS_ENABLED(CONFIG_PPC) || IS_ENABLED(CONFIG_M68K))
-   bdinfo_print_mhz("busfreq", bd->bi_busfreq);
-
return 0;
  }
  




Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH 31/36] bdinfo: ppc: Move PPC-specific info into its own file

2020-05-06 Thread Stefan Roese

On 05.05.20 01:17, Simon Glass wrote:

We don't really want to have PPC-specific code in a generic file. Create
a new arch-specific function to hold it, and move it into that.

Make the function weak so that any arch can implement it.

Signed-off-by: Simon Glass 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---

  arch/powerpc/lib/Makefile |  2 ++
  arch/powerpc/lib/bdinfo.c | 41 +++
  cmd/bdinfo.c  | 27 --
  3 files changed, 43 insertions(+), 27 deletions(-)
  create mode 100644 arch/powerpc/lib/bdinfo.c

diff --git a/arch/powerpc/lib/Makefile b/arch/powerpc/lib/Makefile
index 01c9dd51be..f61809ab05 100644
--- a/arch/powerpc/lib/Makefile
+++ b/arch/powerpc/lib/Makefile
@@ -15,6 +15,8 @@ MINIMAL=y
  endif
  endif
  
+obj-y	+= bdinfo.o

+
  ifdef MINIMAL
  obj-y += cache.o time.o
  ifndef CONFIG_TIMER
diff --git a/arch/powerpc/lib/bdinfo.c b/arch/powerpc/lib/bdinfo.c
new file mode 100644
index 00..da09bb276f
--- /dev/null
+++ b/arch/powerpc/lib/bdinfo.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * PPC-specific information for the 'bd' command
+ *
+ * (C) Copyright 2003
+ * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
+ */
+
+#include 
+#include 
+
+DECLARE_GLOBAL_DATA_PTR;
+
+void __weak board_detail(void)
+{
+   /* Please define board_detail() for your PPC platform */
+}
+
+void arch_print_bdinfo(void)
+{
+   bd_t *bd = gd->bd;
+
+#if defined(CONFIG_MPC8xx) || defined(CONFIG_E500)
+   bdinfo_print_num("immr_base", bd->bi_immr_base);
+#endif
+   bdinfo_print_num("bootflags", bd->bi_bootflags);
+   bdinfo_print_mhz("intfreq", bd->bi_intfreq);
+#ifdef CONFIG_ENABLE_36BIT_PHYS
+   if (IS_ENABLED(CONFIG_PHYS_64BIT))
+   puts("addressing  = 36-bit\n");
+   else
+   puts("addressing  = 32-bit\n");
+#endif
+   board_detail();
+#if defined(CONFIG_CPM2)
+   bdinfo_print_mhz("cpmfreq", bd->bi_cpmfreq);
+   bdinfo_print_mhz("vco", bd->bi_vco);
+   bdinfo_print_mhz("sccfreq", bd->bi_sccfreq);
+   bdinfo_print_mhz("brgfreq", bd->bi_brgfreq);
+#endif
+}
diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
index 55a9244aef..570022052c 100644
--- a/cmd/bdinfo.c
+++ b/cmd/bdinfo.c
@@ -60,11 +60,6 @@ static void print_bi_dram(const bd_t *bd)
  #endif
  }
  
-void __weak board_detail(void)

-{
-   /* Please define board_detail() for your PPC platform */
-}
-
  __weak void arch_print_bdinfo(void)
  {
  }
@@ -103,28 +98,6 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char 
*const argv[])
  
  	arch_print_bdinfo();
  
-	/* This section is used only by ppc */

-#if defined(CONFIG_MPC8xx) || defined(CONFIG_E500)
-   bdinfo_print_num("immr_base", bd->bi_immr_base);
-#endif
-   if (IS_ENABLED(CONFIG_PPC)) {
-   bdinfo_print_num("bootflags", bd->bi_bootflags);
-   bdinfo_print_mhz("intfreq", bd->bi_intfreq);
-#ifdef CONFIG_ENABLE_36BIT_PHYS
-   if (IS_ENABLED(CONFIG_PHYS_64BIT))
-   puts("addressing  = 36-bit\n");
-   else
-   puts("addressing  = 32-bit\n");
-#endif
-   board_detail();
-   }
-#if defined(CONFIG_CPM2)
-   bdinfo_print_mhz("cpmfreq", bd->bi_cpmfreq);
-   bdinfo_print_mhz("vco", bd->bi_vco);
-   bdinfo_print_mhz("sccfreq", bd->bi_sccfreq);
-   bdinfo_print_mhz("brgfreq", bd->bi_brgfreq);
-#endif
-
/* This is used by m68k and ppc */
  #if defined(CONFIG_SYS_INIT_RAM_ADDR)
bdinfo_print_num("sramstart", (ulong)bd->bi_sramstart);




Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH 25/36] bdinfo: net: ppc: Drop prints for CONFIG_HAS_ETHn

2020-05-06 Thread Stefan Roese

On 05.05.20 01:17, Simon Glass wrote:

These config options have not been migrated to Kconfig. This should be
handled using driver model, iterating over the available Ethernet devices.
For now, remove the code.

Signed-off-by: Simon Glass 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---

  cmd/bdinfo.c | 15 ---
  1 file changed, 15 deletions(-)

diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
index d0afef5b5e..6fa8b32389 100644
--- a/cmd/bdinfo.c
+++ b/cmd/bdinfo.c
@@ -86,21 +86,6 @@ static void print_eth_ip_addr(void)
  {
  #if defined(CONFIG_CMD_NET)
print_eth(0);
-#if defined(CONFIG_HAS_ETH1)
-   print_eth(1);
-#endif
-#if defined(CONFIG_HAS_ETH2)
-   print_eth(2);
-#endif
-#if defined(CONFIG_HAS_ETH3)
-   print_eth(3);
-#endif
-#if defined(CONFIG_HAS_ETH4)
-   print_eth(4);
-#endif
-#if defined(CONFIG_HAS_ETH5)
-   print_eth(5);
-#endif
printf("IP addr = %s\n", env_get("ipaddr"));
  #endif
  }




Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH 24/36] bdinfo: net: ppc: Drop bi_enet1addr and other similar info

2020-05-06 Thread Stefan Roese

On 05.05.20 01:17, Simon Glass wrote:

These values were 'old' in 2013 so it should be safe to remove them. They
are never set in U-Boot anyway, so the values will always be zero.

Signed-off-by: Simon Glass 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---

  common/board_r.c | 16 +---
  include/asm-generic/u-boot.h | 17 -
  2 files changed, 1 insertion(+), 32 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index d9015cd057..4876afba6b 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -525,21 +525,7 @@ static int initr_ethaddr(void)
  
  	/* kept around for legacy kernels only ... ignore the next section */

eth_env_get_enetaddr("ethaddr", bd->bi_enetaddr);
-#ifdef CONFIG_HAS_ETH1
-   eth_env_get_enetaddr("eth1addr", bd->bi_enet1addr);
-#endif
-#ifdef CONFIG_HAS_ETH2
-   eth_env_get_enetaddr("eth2addr", bd->bi_enet2addr);
-#endif
-#ifdef CONFIG_HAS_ETH3
-   eth_env_get_enetaddr("eth3addr", bd->bi_enet3addr);
-#endif
-#ifdef CONFIG_HAS_ETH4
-   eth_env_get_enetaddr("eth4addr", bd->bi_enet4addr);
-#endif
-#ifdef CONFIG_HAS_ETH5
-   eth_env_get_enetaddr("eth5addr", bd->bi_enet5addr);
-#endif
+
return 0;
  }
  #endif /* CONFIG_CMD_NET */
diff --git a/include/asm-generic/u-boot.h b/include/asm-generic/u-boot.h
index be0014c3b3..0ec4522653 100644
--- a/include/asm-generic/u-boot.h
+++ b/include/asm-generic/u-boot.h
@@ -67,23 +67,6 @@ typedef struct bd_info {
unsigned long bi_vcofreq;   /* vco Freq in MHz */
unsigned long bi_flbfreq;   /* Flexbus Freq in MHz */
  #endif
-
-#ifdef CONFIG_HAS_ETH1
-   unsigned char   bi_enet1addr[6];/* OLD: see README.enetaddr */
-#endif
-#ifdef CONFIG_HAS_ETH2
-   unsigned char   bi_enet2addr[6];/* OLD: see README.enetaddr */
-#endif
-#ifdef CONFIG_HAS_ETH3
-   unsigned char   bi_enet3addr[6];/* OLD: see README.enetaddr */
-#endif
-#ifdef CONFIG_HAS_ETH4
-   unsigned char   bi_enet4addr[6];/* OLD: see README.enetaddr */
-#endif
-#ifdef CONFIG_HAS_ETH5
-   unsigned char   bi_enet5addr[6];/* OLD: see README.enetaddr */
-#endif
-
ulong   bi_arch_number; /* unique id for this board */
ulong   bi_boot_params; /* where this board expects params */
  #ifdef CONFIG_NR_DRAM_BANKS




Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH 19/36] bdinfo: ppc: Drop arch-specific print_baudrate()

2020-05-06 Thread Stefan Roese

On 05.05.20 01:17, Simon Glass wrote:

This function outputs the same basic info. Since the baud rate is commonly
115200 these is often no difference. Drop the arch-specific code and
inline it to avoid a one-line function.

Signed-off-by: Simon Glass 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---

  cmd/bdinfo.c | 12 +---
  1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
index 4e08d9e40a..62eea010c5 100644
--- a/cmd/bdinfo.c
+++ b/cmd/bdinfo.c
@@ -137,15 +137,6 @@ static void print_eth_ip_addr(void)
  #endif
  }
  
-static void print_baudrate(void)

-{
-#if defined(CONFIG_PPC)
-   printf("baudrate= %6u bps\n", gd->baudrate);
-#else
-   printf("baudrate= %u bps\n", gd->baudrate);
-#endif
-}
-
  void __weak board_detail(void)
  {
/* Please define board_detail() for your PPC platform */
@@ -165,8 +156,7 @@ int do_bdinfo(cmd_tbl_t *cmdtp, int flag, int argc, char 
*const argv[])
print_bi_mem(bd);
print_bi_flash(bd);
print_eth_ip_addr();
-   print_baudrate();
-   print_std_bdinfo(bd);
+   printf("baudrate= %u bps\n", gd->baudrate);
print_num("relocaddr", gd->relocaddr);
print_num("reloc off", gd->reloc_off);
print_cpu_word_size();




Viele Grüße,
Stefan

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Re: [PATCH] Revert "mkimage: fit: Do not tail-pad fitImage with external data"

2020-05-06 Thread Marek Vasut
On 5/6/20 5:05 PM, Tom Rini wrote:
> This has been reported to break booting of U-Boot from SPL on a number
> of platforms due to a lack of alignment of the external data.  The
> issues this commit is addressing will need to be resolved another way.
> 
> This reverts commit 20a154f95bfe0a3b5bfba90bea7f001c58217536.
> 
> Reported-by: Alex Kiernan 
> Reported-by: Michael Walle 
> Signed-off-by: Tom Rini 

The commit message should also warn that this re-opens the data leak in
the padding.


  1   2   3   >