Re: [U-Boot] efi_loader: runtime services implementation broken?

2018-07-03 Thread Bin Meng
Hi Heinrich,

On Wed, Jul 4, 2018 at 3:39 AM, Heinrich Schuchardt  wrote:
> On 07/03/2018 06:51 PM, Bin Meng wrote:
>> Hi Alex,
>>
>> On Tue, Jul 3, 2018 at 11:05 PM, Alexander Graf  wrote:
>>> On 07/03/2018 04:56 PM, Bin Meng wrote:

 Hi Alex,

 On Tue, Jul 3, 2018 at 10:32 PM, Alexander Graf  wrote:
>
> On 07/03/2018 04:24 PM, Bin Meng wrote:
>>
>> Hi Alex, Heinrich,
>>
>> At present not all interfaces in lib/efi_loader/efi_runtime.c are
>> declared as __efi_runtime. But only declaring those as __efi_runtime
>> is not enough. The data these interfaces use should be declared as
>> __efi_runtime_data too. More worse, any U-Boot APIs called by these
>> interfaces should be __efi_runtime and __efi_runtime_data
>> respectively.
>
>
> Correct. This is done for everything right now, no?

 For example, efi_set_virtual_address_map() is not declared as
 __efi_runtime.
>>>
>>>
>>> Is that called post exit boot services on x86?
>>
>> Yes, if you look at efi_runtime_services structure in
>> lib/efi_loader/efi_runtime.c, you can see
>> efi_set_virtual_address_map() is hooked up there, and all interfaces
>> in efi_runtime_services are supposed to be called by OS as their names
>> indicate, runtime not bootime.
>
> No, this not correct. Runtime functions may be called before and after
> ExitBootServices. Otherwise the EFI shell would not be able to read
> variables for instance.
>

Yes, I meant to say runtime services interfaces should be available
after ExitBootServices, and that's for runtime like OS to call.

> The SetVirtualAdressMap service is only called after ExitBootServices.
> At that time the caller is the owner of the memory map. So this function
> must be marked as __efi_runtime.
>
> Our current coding makes the invalid assumption that U-Boot is owner of
> the memory until SetVirtualAddressMap() is called.
>
> Other deficiencies are that we do not provide the ConvertPointer()
> service and do not raise notify an event when SetVirtualAddressMap() is
> called.
>
>>
>>>

>> Eventually we need mark the RAM where the whole U-Boot image lives as
>> runtime service code and data and end up leaving whole U-Boot image in
>> the RAM after kernel boots?
>
>
> Why?
>
 Unless we track down every APIs called by runtime services and mark
 them correctly as __efi_runtime code/data.
>>>
>>>
>>> That's the idea, yes. The vector should be quite small as long as we don't
>>> actually implement / reuse drivers.
>>
>> However this currently seems not to be the case ..
>>
>>>

>> Right now I am testing booting Linux on Intel Galileo with 'bootefi'
>> and kernel just hangs during the boot. Initial debugging shows that
>> kernel crashes when calling runtime service
>> efi_set_virtual_address_map().
>
>
> Is this a regression or did it never work?
>

I believe it never worked on x86.

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Vasily Khoruzhick
On Tue, Jul 3, 2018 at 5:45 PM, André Przywara  wrote:

>>
>> I tried enabling DM for MMC on A64 recently and unfortunately it results in
>> SPL exceeding 32kb size limit.
>
> Mmh, worked for me with this preliminary branch:
> [1] https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
> (which I forgot to link in my original email).
> Did you enable SPL_DM_MMC or SPL_DM as well? I think there is not much
> benefit in doing so, especially given the code size issue.

Yes, I did. We'll need it to handle all the controller quirks
eventually (new clock mode, calibration, delays).
Currently we have a number of ifdefs in the driver.

>> So I guess we'll have to find a workaround for this issue as well...
>
> I recently made a patch that shrinks the ARMv8 exception table code by
> 250 bytes. Not much, but might help. How much did you overflow?
> I think eventually we should generate the exception table and put it
> somewhere else than in SRAM A1. That has the potential of freeing up
> about 2KB or so. I started rewriting the vectors to make this easier,
> but it's not very high on my TODO list.

That won't be enough:

aarch64-linux-gnu-ld.bfd: region `.sram' overflowed by 10584 bytes

>
> Cheers,
> Andre.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread André Przywara
On 07/04/2018 01:25 AM, Vasily Khoruzhick wrote:
> On Tue, Jul 3, 2018 at 2:22 PM, André Przywara  wrote:
> 
>>> Adding a few more people to the list.  It looks like b62cdbddedc3 was in
>>> response to fef73766d9ad.  So, this close to the release, what do we
>>> need to do to get things back to the state things were in for v2018.05?
>>> And then what are the combinations that aren't working and need to be
>>> addressed again in v2018.09 so that we can make forward progress?
>>
>> Without going into much detail here, the clock dependency for two
>> companion controllers on those A64 is weird, and we hack it somehow
>> since we don't have a DM_CLK driver for sunxi (yet, see below). That
>> works for init, since we just set already set bits. For shutdown, we
>> *happen* to take down the AHB gate clocks and resets correctly, because
>> the order of shutdown matches the dependency (EHCI first, the OHCI). Not
>> sure if this is intentional, though. It's fragile, but works.
>>
>> What we don't (and can't easily) model is another oddity: the USB clock
>> for [OE]HCI0 is actually the parent of [OE]HCI1. So if we need to bring
>> down *two* controllers and do it in the natural order, the second one is
>> dead before it can be disabled properly.
>>
>> This was somewhat hidden before, since we had only one controller in
>> operation. b62cdbddedc3 somewhat fixed that, but the DT for the Pine64
>> (which was the test vehicle) has the controller still disabled. Enabling
>> this (what I did) or using other boards (BananaPi and NanoPi from Jagan)
>> triggered this bug though.
>> AFAICS this parent relation only affects the A64 with its two
>> controllers, so:
>> - We could just fix it for now by *not* disabling the USB clocks (and
>> only those, gates and reset still go down) at all. This is basically one
>> part of my patch from yesterday (the second part is not needed).
>> - We do more effort and skip disabling for OHCI0, but disable both
>> clocks for OHCI1. This still relies on the order, but would probably
>> shut down the controllers. A bit hackish to implement, though.
>> I will try the second solution now and let you decide on the list.
>>
>> Long term:
>> The proper solution is a DT based DM_CLK driver, which models it like
>> Linux does. Work is underway[1], but this somewhat opens a can of worms
>> (needs DM support for UART, MMC, a DM_RESET driver, pinctrl ...), so it
>> takes a bit of time.
> 
> I tried enabling DM for MMC on A64 recently and unfortunately it results in
> SPL exceeding 32kb size limit.

Mmh, worked for me with this preliminary branch:
[1] https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
(which I forgot to link in my original email).
Did you enable SPL_DM_MMC or SPL_DM as well? I think there is not much
benefit in doing so, especially given the code size issue.

> So I guess we'll have to find a workaround for this issue as well...

I recently made a patch that shrinks the ARMv8 exception table code by
250 bytes. Not much, but might help. How much did you overflow?
I think eventually we should generate the exception table and put it
somewhere else than in SRAM A1. That has the potential of freeing up
about 2KB or so. I started rewriting the vectors to make this easier,
but it's not very high on my TODO list.

Cheers,
Andre.


>> Fixing the current ad-hoc solution somewhat properly with ref-counting
>> is not easy (two driver files) and not really worthwhile, I believe, but
>> we can make it work like described above until the proper solution comes
>> into play.
>>
>> Cheers,
>> Andre.

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


[U-Boot] [PATCH 5/5] net: Consolidate the parsing of bootfile

2018-07-03 Thread Joe Hershberger
The same basic parsing was implemented in tftp and nfs, so add a helper
function to do the work once.

Signed-off-by: Joe Hershberger 

---

 include/net.h | 11 +++
 net/net.c | 20 
 net/nfs.c | 15 ++-
 net/tftp.c| 13 +
 4 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/include/net.h b/include/net.h
index de2d7bba19..62f82c4dca 100644
--- a/include/net.h
+++ b/include/net.h
@@ -842,6 +842,17 @@ void copy_filename(char *dst, const char *src, int size);
 /* check if serverip is specified in filename from the command line */
 int is_serverip_in_cmd(void);
 
+/**
+ * net_parse_bootfile - Parse the bootfile env var / cmd line param
+ *
+ * @param ipaddr - a pointer to the ipaddr to populate if included in bootfile
+ * @param filename - a pointer to the string to save the filename part
+ * @param max_len - The longest - 1 that the filename part can be
+ *
+ * return 1 if parsed, 0 if bootfile is empty
+ */
+int net_parse_bootfile(struct in_addr *ipaddr, char *filename, int max_len);
+
 /* get a random source port */
 unsigned int random_port(void);
 
diff --git a/net/net.c b/net/net.c
index 1b6781d358..31cf306ae7 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1517,6 +1517,26 @@ int is_serverip_in_cmd(void)
return !!strchr(net_boot_file_name, ':');
 }
 
+int net_parse_bootfile(struct in_addr *ipaddr, char *filename, int max_len)
+{
+   char *colon;
+
+   if (net_boot_file_name[0] == '\0')
+   return 0;
+
+   colon = strchr(net_boot_file_name, ':');
+   if (colon) {
+   if (ipaddr)
+   *ipaddr = string_to_ip(net_boot_file_name);
+   strncpy(filename, colon + 1, max_len);
+   } else {
+   strncpy(filename, net_boot_file_name, max_len);
+   }
+   filename[max_len - 1] = '\0';
+
+   return 1;
+}
+
 #ifdefined(CONFIG_CMD_NFS) || \
defined(CONFIG_CMD_SNTP)|| \
defined(CONFIG_CMD_DNS)
diff --git a/net/nfs.c b/net/nfs.c
index 9a16765ba1..ed6a64d722 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -859,7 +859,8 @@ void nfs_start(void)
return;
}
 
-   if (net_boot_file_name[0] == '\0') {
+   if (!net_parse_bootfile(_server_ip, nfs_path,
+   sizeof(nfs_path_buff))) {
sprintf(nfs_path, "/nfsroot/%02X%02X%02X%02X.img",
net_ip.s_addr & 0xFF,
(net_ip.s_addr >>  8) & 0xFF,
@@ -868,18 +869,6 @@ void nfs_start(void)
 
debug("*** Warning: no boot file name; using '%s'\n",
  nfs_path);
-   } else {
-   char *p = net_boot_file_name;
-
-   p = strchr(p, ':');
-
-   if (p != NULL) {
-   nfs_server_ip = string_to_ip(net_boot_file_name);
-   ++p;
-   strcpy(nfs_path, p);
-   } else {
-   strcpy(nfs_path, net_boot_file_name);
-   }
}
 
nfs_filename = basename(nfs_path);
diff --git a/net/tftp.c b/net/tftp.c
index 6671b1f7ca..68ffd81414 100644
--- a/net/tftp.c
+++ b/net/tftp.c
@@ -735,7 +735,7 @@ void tftp_start(enum proto_t protocol)
  tftp_block_size_option, timeout_ms);
 
tftp_remote_ip = net_server_ip;
-   if (net_boot_file_name[0] == '\0') {
+   if (!net_parse_bootfile(_remote_ip, tftp_filename, MAX_LEN)) {
sprintf(default_filename, "%02X%02X%02X%02X.img",
net_ip.s_addr & 0xFF,
(net_ip.s_addr >>  8) & 0xFF,
@@ -747,17 +747,6 @@ void tftp_start(enum proto_t protocol)
 
printf("*** Warning: no boot file name; using '%s'\n",
   tftp_filename);
-   } else {
-   char *p = strchr(net_boot_file_name, ':');
-
-   if (p == NULL) {
-   strncpy(tftp_filename, net_boot_file_name, MAX_LEN);
-   tftp_filename[MAX_LEN - 1] = 0;
-   } else {
-   tftp_remote_ip = string_to_ip(net_boot_file_name);
-   strncpy(tftp_filename, p + 1, MAX_LEN);
-   tftp_filename[MAX_LEN - 1] = 0;
-   }
}
 
printf("Using %s device\n", eth_get_name());
-- 
2.11.0

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


[U-Boot] [PATCH 4/5] net: Read bootfile from env on netboot_common()

2018-07-03 Thread Joe Hershberger
Instead of depending on a env callback for bootfile, read it explicitly.

We do this because the bootfile can be specified on the command line and
if it is, we will overwrite the internal variable. If a netboot_common()
is called again with no bootfile parameter, we want to use the one in
the environment.

Signed-off-by: Joe Hershberger 
---

 cmd/net.c |  6 ++
 net/net.c | 20 
 2 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/cmd/net.c b/cmd/net.c
index eca6dd8918..89721b8f8b 100644
--- a/cmd/net.c
+++ b/cmd/net.c
@@ -192,6 +192,9 @@ static int netboot_common(enum proto_t proto, cmd_tbl_t 
*cmdtp, int argc,
 
switch (argc) {
case 1:
+   /* refresh bootfile name from env */
+   copy_filename(net_boot_file_name, env_get("bootfile"),
+ sizeof(net_boot_file_name));
break;
 
case 2: /*
@@ -203,6 +206,9 @@ static int netboot_common(enum proto_t proto, cmd_tbl_t 
*cmdtp, int argc,
addr = simple_strtoul(argv[1], , 16);
if (end == (argv[1] + strlen(argv[1]))) {
load_addr = addr;
+   /* refresh bootfile name from env */
+   copy_filename(net_boot_file_name, env_get("bootfile"),
+ sizeof(net_boot_file_name));
} else {
net_boot_file_name_explicit = true;
copy_filename(net_boot_file_name, argv[1],
diff --git a/net/net.c b/net/net.c
index 333102ea79..1b6781d358 100644
--- a/net/net.c
+++ b/net/net.c
@@ -216,26 +216,6 @@ int __maybe_unused net_busy_flag;
 
 /**/
 
-static int on_bootfile(const char *name, const char *value, enum env_op op,
-   int flags)
-{
-   if (flags & H_PROGRAMMATIC)
-   return 0;
-
-   switch (op) {
-   case env_op_create:
-   case env_op_overwrite:
-   copy_filename(net_boot_file_name, value,
- sizeof(net_boot_file_name));
-   break;
-   default:
-   break;
-   }
-
-   return 0;
-}
-U_BOOT_ENV_CALLBACK(bootfile, on_bootfile);
-
 static int on_ipaddr(const char *name, const char *value, enum env_op op,
int flags)
 {
-- 
2.11.0

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


[U-Boot] [PATCH 3/5] net: Make copy_filename() accept NULL src

2018-07-03 Thread Joe Hershberger
Rather than crashing, check the src ptr and set dst to empty string.

Signed-off-by: Joe Hershberger 
---

 net/net.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/net.c b/net/net.c
index 42a50e60f8..333102ea79 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1522,12 +1522,12 @@ void net_set_udp_header(uchar *pkt, struct in_addr 
dest, int dport, int sport,
 
 void copy_filename(char *dst, const char *src, int size)
 {
-   if (*src && (*src == '"')) {
+   if (src && *src && (*src == '"')) {
++src;
--size;
}
 
-   while ((--size > 0) && *src && (*src != '"'))
+   while ((--size > 0) && src && *src && (*src != '"'))
*dst++ = *src++;
*dst = '\0';
 }
-- 
2.11.0

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


[U-Boot] [PATCH 1/5] net: When checking prerequisites, consider boot_file_name

2018-07-03 Thread Joe Hershberger
For net_boot_common, we allow the serverip to be specified as part of
the boot file name. For net commands that require serverip, include that
source as a valid specification of serverip.

Signed-off-by: Joe Hershberger 
---

 include/net.h | 3 +++
 net/net.c | 7 ++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/net.h b/include/net.h
index f9984ae86c..de2d7bba19 100644
--- a/include/net.h
+++ b/include/net.h
@@ -839,6 +839,9 @@ ushort env_get_vlan(char *);
 /* copy a filename (allow for "..." notation, limit length) */
 void copy_filename(char *dst, const char *src, int size);
 
+/* check if serverip is specified in filename from the command line */
+int is_serverip_in_cmd(void);
+
 /* get a random source port */
 unsigned int random_port(void);
 
diff --git a/net/net.c b/net/net.c
index f35695b4fc..bff3e9c5b5 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1341,7 +1341,7 @@ static int net_check_prereq(enum proto_t protocol)
/* Fall through */
case TFTPGET:
case TFTPPUT:
-   if (net_server_ip.s_addr == 0) {
+   if (net_server_ip.s_addr == 0 && !is_serverip_in_cmd()) {
puts("*** ERROR: `serverip' not set\n");
return 1;
}
@@ -1512,6 +1512,11 @@ void copy_filename(char *dst, const char *src, int size)
*dst = '\0';
 }
 
+int is_serverip_in_cmd(void)
+{
+   return !!strchr(net_boot_file_name, ':');
+}
+
 #ifdefined(CONFIG_CMD_NFS) || \
defined(CONFIG_CMD_SNTP)|| \
defined(CONFIG_CMD_DNS)
-- 
2.11.0

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


[U-Boot] [PATCH 2/5] net: Re-check prerequisites when autoloading

2018-07-03 Thread Joe Hershberger
With net autoload, we check the prerequisites for the initial command,
but the greater prerequisites when autoloading are not checked.

If we would attempt to autoload, check those prerequisites too.

If we are not expecting a serverip from the server, then don't worry
about it not being set, but don't attempt to load if it isn't.

Signed-off-by: Joe Hershberger 
---

 net/net.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/net/net.c b/net/net.c
index bff3e9c5b5..42a50e60f8 100644
--- a/net/net.c
+++ b/net/net.c
@@ -332,6 +332,16 @@ void net_auto_load(void)
const char *s = env_get("autoload");
 
if (s != NULL && strcmp(s, "NFS") == 0) {
+   if (net_check_prereq(NFS)) {
+/* We aren't expecting to get a serverip, so just accept the assigned IP */
+#ifdef CONFIG_BOOTP_SERVERIP
+   net_set_state(NETLOOP_SUCCESS);
+#else
+   printf("Cannot autoload with NFS\n");
+   net_set_state(NETLOOP_FAIL);
+#endif
+   return;
+   }
/*
 * Use NFS to load the bootfile.
 */
@@ -347,6 +357,16 @@ void net_auto_load(void)
net_set_state(NETLOOP_SUCCESS);
return;
}
+   if (net_check_prereq(TFTPGET)) {
+/* We aren't expecting to get a serverip, so just accept the assigned IP */
+#ifdef CONFIG_BOOTP_SERVERIP
+   net_set_state(NETLOOP_SUCCESS);
+#else
+   printf("Cannot autoload with TFTPGET\n");
+   net_set_state(NETLOOP_FAIL);
+#endif
+   return;
+   }
tftp_start(TFTPGET);
 }
 
-- 
2.11.0

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


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Vasily Khoruzhick
On Tue, Jul 3, 2018 at 2:22 PM, André Przywara  wrote:

>> Adding a few more people to the list.  It looks like b62cdbddedc3 was in
>> response to fef73766d9ad.  So, this close to the release, what do we
>> need to do to get things back to the state things were in for v2018.05?
>> And then what are the combinations that aren't working and need to be
>> addressed again in v2018.09 so that we can make forward progress?
>
> Without going into much detail here, the clock dependency for two
> companion controllers on those A64 is weird, and we hack it somehow
> since we don't have a DM_CLK driver for sunxi (yet, see below). That
> works for init, since we just set already set bits. For shutdown, we
> *happen* to take down the AHB gate clocks and resets correctly, because
> the order of shutdown matches the dependency (EHCI first, the OHCI). Not
> sure if this is intentional, though. It's fragile, but works.
>
> What we don't (and can't easily) model is another oddity: the USB clock
> for [OE]HCI0 is actually the parent of [OE]HCI1. So if we need to bring
> down *two* controllers and do it in the natural order, the second one is
> dead before it can be disabled properly.
>
> This was somewhat hidden before, since we had only one controller in
> operation. b62cdbddedc3 somewhat fixed that, but the DT for the Pine64
> (which was the test vehicle) has the controller still disabled. Enabling
> this (what I did) or using other boards (BananaPi and NanoPi from Jagan)
> triggered this bug though.
> AFAICS this parent relation only affects the A64 with its two
> controllers, so:
> - We could just fix it for now by *not* disabling the USB clocks (and
> only those, gates and reset still go down) at all. This is basically one
> part of my patch from yesterday (the second part is not needed).
> - We do more effort and skip disabling for OHCI0, but disable both
> clocks for OHCI1. This still relies on the order, but would probably
> shut down the controllers. A bit hackish to implement, though.
> I will try the second solution now and let you decide on the list.
>
> Long term:
> The proper solution is a DT based DM_CLK driver, which models it like
> Linux does. Work is underway[1], but this somewhat opens a can of worms
> (needs DM support for UART, MMC, a DM_RESET driver, pinctrl ...), so it
> takes a bit of time.

I tried enabling DM for MMC on A64 recently and unfortunately it results in
SPL exceeding 32kb size limit.
So I guess we'll have to find a workaround for this issue as well...

> Fixing the current ad-hoc solution somewhat properly with ref-counting
> is not easy (two driver files) and not really worthwhile, I believe, but
> we can make it work like described above until the proper solution comes
> into play.
>
> Cheers,
> Andre.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v2 2/3] net: Check subnet against the actual ip address in use for nfs

2018-07-03 Thread Joe Hershberger
The check for sending to the gateway was not using the correct variable
for comparison, so it was reporting that packets are sent to the gateway
when they were not.

Signed-off-by: Joe Hershberger 
---

Changes in v2: None

 net/nfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/nfs.c b/net/nfs.c
index 9a16765ba1..7e8af28e9f 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -896,7 +896,7 @@ void nfs_start(void)
struct in_addr server_net;
 
our_net.s_addr = net_ip.s_addr & net_netmask.s_addr;
-   server_net.s_addr = net_server_ip.s_addr & net_netmask.s_addr;
+   server_net.s_addr = nfs_server_ip.s_addr & net_netmask.s_addr;
if (our_net.s_addr != server_net.s_addr)
debug("; sending through gateway %pI4",
  _gateway);
-- 
2.11.0

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


[U-Boot] [PATCH v2 3/3] net: Always print basic info for nfs, just like tftp

2018-07-03 Thread Joe Hershberger
nfs was only printing basic info about the transfer in the case of a
DEBUG build. Print the same level of detail as tftp always.

Signed-off-by: Joe Hershberger 
---

Changes in v2: None

 net/nfs.c | 26 +-
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/net/nfs.c b/net/nfs.c
index 7e8af28e9f..86dfe9a494 100644
--- a/net/nfs.c
+++ b/net/nfs.c
@@ -533,7 +533,7 @@ static int nfs_lookup_reply(uchar *pkt, unsigned len)
switch (ntohl(rpc_pkt.u.reply.data[0])) {
/* Minimal supported NFS version */
case 3:
-   debug("*** Waring: NFS version not supported: 
Requested: V%d, accepted: min V%d - max V%d\n",
+   debug("*** Warning: NFS version not supported: 
Requested: V%d, accepted: min V%d - max V%d\n",
  (supported_nfs_versions & NFSV2_FLAG) ?
2 : 3,
  ntohl(rpc_pkt.u.reply.data[0]),
@@ -855,7 +855,7 @@ void nfs_start(void)
 
if (nfs_path == NULL) {
net_set_state(NETLOOP_FAIL);
-   debug("*** ERROR: Fail allocate memory\n");
+   printf("*** ERROR: Fail allocate memory\n");
return;
}
 
@@ -866,8 +866,8 @@ void nfs_start(void)
(net_ip.s_addr >> 16) & 0xFF,
(net_ip.s_addr >> 24) & 0xFF);
 
-   debug("*** Warning: no boot file name; using '%s'\n",
- nfs_path);
+   printf("*** Warning: no boot file name; using '%s'\n",
+  nfs_path);
} else {
char *p = net_boot_file_name;
 
@@ -885,10 +885,10 @@ void nfs_start(void)
nfs_filename = basename(nfs_path);
nfs_path = dirname(nfs_path);
 
-   debug("Using %s device\n", eth_get_name());
+   printf("Using %s device\n", eth_get_name());
 
-   debug("File transfer via NFS from server %pI4; our IP address is %pI4",
- _server_ip, _ip);
+   printf("File transfer via NFS from server %pI4; our IP address is %pI4",
+  _server_ip, _ip);
 
/* Check if we need to send across this subnet */
if (net_gateway.s_addr && net_netmask.s_addr) {
@@ -898,17 +898,17 @@ void nfs_start(void)
our_net.s_addr = net_ip.s_addr & net_netmask.s_addr;
server_net.s_addr = nfs_server_ip.s_addr & net_netmask.s_addr;
if (our_net.s_addr != server_net.s_addr)
-   debug("; sending through gateway %pI4",
- _gateway);
+   printf("; sending through gateway %pI4",
+  _gateway);
}
-   debug("\nFilename '%s/%s'.", nfs_path, nfs_filename);
+   printf("\nFilename '%s/%s'.", nfs_path, nfs_filename);
 
if (net_boot_file_expected_size_in_blocks) {
-   debug(" Size is 0x%x Bytes = ",
- net_boot_file_expected_size_in_blocks << 9);
+   printf(" Size is 0x%x Bytes = ",
+  net_boot_file_expected_size_in_blocks << 9);
print_size(net_boot_file_expected_size_in_blocks << 9, "");
}
-   debug("\nLoad address: 0x%lx\nLoading: *\b", load_addr);
+   printf("\nLoad address: 0x%lx\nLoading: *\b", load_addr);
 
net_set_timeout_handler(nfs_timeout, nfs_timeout_handler);
net_set_udp_handler(nfs_handler);
-- 
2.11.0

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


[U-Boot] [PATCH v2 1/3] net: Correct size of NFS buffers

2018-07-03 Thread Joe Hershberger
Reported-by: Coverity (CID: 152888)
Signed-off-by: Joe Hershberger 
---

Changes in v2:
- Take into account the attributes that could be there
- Tested with v2 and v3

 net/nfs.h | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/nfs.h b/net/nfs.h
index 6359cd2848..a377c90088 100644
--- a/net/nfs.h
+++ b/net/nfs.h
@@ -42,6 +42,7 @@
  * case, most NFS servers are optimized for a power of 2.
  */
 #define NFS_READ_SIZE  1024/* biggest power of two that fits Ether frame */
+#define NFS_MAX_ATTRS  26
 
 /* Values for Accept State flag on RPC answers (See: rfc1831) */
 enum rpc_accept_stat {
@@ -55,7 +56,8 @@ enum rpc_accept_stat {
 
 struct rpc_t {
union {
-   uint8_t data[2048];
+   uint8_t data[NFS_READ_SIZE + (6 + NFS_MAX_ATTRS) *
+   sizeof(uint32_t)];
struct {
uint32_t id;
uint32_t type;
@@ -72,7 +74,8 @@ struct rpc_t {
uint32_t verifier;
uint32_t v2;
uint32_t astatus;
-   uint32_t data[NFS_READ_SIZE];
+   uint32_t data[NFS_READ_SIZE / sizeof(uint32_t) +
+   NFS_MAX_ATTRS];
} reply;
} u;
 } __attribute__((packed));
-- 
2.11.0

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


[U-Boot] [PATCH] sunxi: A64: OHCI: prevent turning off shared USB clock

2018-07-03 Thread Andre Przywara
On the A64 the clock for the first USB controller is actually the parent
of the clock for the second controller, so turning them off in that order
makes the system hang.
Fix this by *not* turning off any clock for OHCI0, but both clocks when
OHCI1 is brought down.

Signed-off-by: Andre Przywara 
---
Hi,

this is a new approach to fix the USB hang we see with mainline U-Boot.
Compared to the previous patch it just deals with the USB clock (the AHB
gate was a red herring), and it eventually turns both clocks off instead
of leaving them running. Please have a test on A64 boards!

Cheers,
Andre.

 drivers/usb/host/ohci-sunxi.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c
index 0ddbdbe460..8f108b48a8 100644
--- a/drivers/usb/host/ohci-sunxi.c
+++ b/drivers/usb/host/ohci-sunxi.c
@@ -114,6 +114,7 @@ no_phy:
 static int ohci_usb_remove(struct udevice *dev)
 {
struct ohci_sunxi_priv *priv = dev_get_priv(dev);
+   fdt_addr_t base_addr = devfdt_get_addr(dev);
int ret;
 
if (generic_phy_valid(>phy)) {
@@ -130,7 +131,17 @@ static int ohci_usb_remove(struct udevice *dev)
 
if (priv->cfg->has_reset)
clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
-   clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
+   /*
+* On the A64 CLK_USB_OHCI0 is the parent of CLK_USB_OHCI1, so
+* we have to bring down none for OHCI0, but both for OHCI1.
+*/
+   if (!priv->cfg->extra_usb_gate_mask || base_addr >= SUNXI_USB2_BASE) {
+   u32 usb_gate_mask = priv->usb_gate_mask;
+
+   usb_gate_mask |= priv->cfg->extra_usb_gate_mask;
+   clrbits_le32(>ccm->usb_clk_cfg, usb_gate_mask);
+   }
+
clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
 
return 0;
-- 
2.14.4

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


Re: [U-Boot] SoCFPGA PL330 DMA driver and ECC scrubbing

2018-07-03 Thread Jason Rush
On 7/3/2018 9:08 AM, Marek Vasut wrote:
> On 07/03/2018 03:58 PM, Jason Rush wrote:
>> On 6/29/2018 10:17 AM, Marek Vasut wrote:
>>> On 06/29/2018 05:06 PM, Jason Rush wrote:
 On 6/29/2018 9:52 AM, Marek Vasut wrote:
> On 06/29/2018 04:44 PM, Jason Rush wrote:
>> On 6/29/2018 9:34 AM, Marek Vasut wrote:
>>> On 06/29/2018 04:31 PM, Jason Rush wrote:
 Dinh,
>>> Hi,
>>>
 A while ago, you posted the following patchset for SoCFPGA to add the 
 PL330
 DMA driver, and updated the SoCFPGA SDRAM init to write zeros to SDRAM 
 to
 initialize the ECC bits if ECC was enabled:

 https://lists.denx.de/pipermail/u-boot/2016-October/269643.html

 I know it's been a long time, so I'll summarize some of the 
 conversation...

 At the time, you had a problem with the patchset causing the SPL to 
 fail to
 find the MMC.  You had tracked it down to an issue with the following 
 commit
 "a78cd8613204 ARM: Rework and correct barrier definitions".  You and 
 Marek
 discussed it a bit, but I don't think there was a real conclusion.  You
 submitted a second version of the patchset asking for advice on 
 debugging
 the issue:

 https://lists.denx.de/pipermail/u-boot/2016-December/275822.html

 No real conversation came from the second patchset, and that was the 
 end of
 the patch.

 I was hoping we could revisit adding your patchset again. I am working 
 on a
 custom SoCFPGA board with a Cyclone V and ECC SDRAM. I rebased your 
 patchset
 against v2018.05 and it is working on my custom board (although I 
 don't have
 an MMC). I also tested it on a SoCKit booting from an MMC (I forced it 
 to
 scrub the SDRAM on the SoCKit, because it doesn't have ECC RAM), and 
 the
 SoCKit finds the MMC and boots.

 I don't have any suggestions on why it is working now on my board and 
 not
 back when you first submitted the patchset.  Maybe something else was 
 fixed
 in the MMC? I was hoping you and Marek could test this patch again on 
 some
 different SoCFPGA boards to see if you get the same results.
>>> Look at this patch
>>> http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=9bb8a249b292d26f152c20e3641600b3d7b3924b
>>>
>>> You likely want similar approach, it's faster then the DMA and much 
>>> simpler.
>>>
>> Thanks Marek.  I'll give it a try.  Would you be interested in a similar 
>> patch for the Gen 5?
> I don't have any Gen5 board which uses ECC, do you ?
> If so, yes, prepare a patch, it should be very similar.
>
> Make sure to measure how long it takes to scrub the memory and how much
> memory you have, I'd be interested in the numbers.
>
 Looking at the master branch, it doesn't look like that code is ever being 
 called?
 The sdram_init_ecc_bits() function is called from the 
 ddr_calibration_sequence function(),
 but I can't find where ddr_calibration_sequence is called().
>>> git grep for it, it's called from somewhere in the arch/arm/mach-socfpga/
>>>
 Either way, I can test it. I have a custom Cyclone V board with ECC, and 
 the Intel Arria V SoC
 Dev Kit I can test it on too which I think has ECC.
>>> Please do.
>>>
>> I implemented a similar memset approach for the gen 5 socfpga.  It's 
>> basically the same
>> code as in that patch; however, when I performed a single memset the 
>> processor would
>> reset for some reason.  I changed it to loop over calling memset with a size 
>> of 32MB over
>> the entire address the address, and that worked as opposed to doing a single 
>> memset on
>> the RAM.
> Can you do grep MEMSET .config in your U-Boot build dir ? The arch
> memset is implemented in assembler and doesn't trigger WDT , so if it
> takes too long, it could be that the WDT resets the platform.

Both CONFIG_USE_ARCH_MEMSET and CONFIG_SPL_USE_ARCH_MEMSET
are set in my .config, so it must be the WDT triggering as you suspect.

>> I started on a SoCKit because it was handy, I know it doesn't have ECC
> It doesn't by default.
>
>> , but I forced it to
>> initialize the RAM as a quick test.  It seems much slower than the DMA 
>> approach.  It
>> should be noted, I didn't implement any code to time the scrubbing, but 
>> rather just
>> roughly monitored the time to get a rough idea of how long it took.
>>
>> On the SoCKit, which has 1GB of RAM, the memset takes around 8 seconds to 
>> complete,
>> and the DMA takes under 2 seconds.
> Did you enable i/d cache in the SPL ? It's mandatory, otherwise it's
> slow.

I have calls to icache_enable() and dcache_enable() just as you do in
the Arria 10 sdram_init_ecc_bits() function.

I did 

Re: [U-Boot] [PATCH 6/7] usb: rockchip: be quiet on serial port while transferring data

2018-07-03 Thread Lukasz Majewski
Hi Alberto,

> While downloading or uploading megabytes of data we had thousands of
> dummy lines like:
> 
> transfer 0x1 bytes done
> OR
> Uploading 0x1000 bytes
> 
> even on non-debug builds. This because transfers are chunked and
> code running on target does not have any clue about when the current
> chunk is the last one.

Is there any other indication to show if the board is still working?

Something like '' present in fastboot?

> 
> Signed-off-by: Alberto Panizzo 
> ---
>  drivers/usb/gadget/f_rockusb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_rockusb.c
> b/drivers/usb/gadget/f_rockusb.c index 230fbec..4fc8484 100644
> --- a/drivers/usb/gadget/f_rockusb.c
> +++ b/drivers/usb/gadget/f_rockusb.c
> @@ -501,7 +501,7 @@ static void tx_handler_ul_image(struct usb_ep
> *ep, struct usb_request *req) memcpy(in_req->buf, rbuffer,
> transfer_size); in_req->length = transfer_size;
>   in_req->complete = tx_handler_ul_image;
> - printf("Uploading 0x%x bytes\n", transfer_size);
> + debug("Uploading 0x%x bytes\n", transfer_size);
>   usb_ep_dequeue(rockusb_func->in_ep, in_req);
>   ret = usb_ep_queue(rockusb_func->in_ep, in_req, 0);
>   if (ret)
> @@ -563,7 +563,7 @@ static void rx_handler_dl_image(struct usb_ep
> *ep, struct usb_request *req) req->complete = rx_handler_command;
>   req->length = EP_BUFFER_SIZE;
>   f_rkusb->buf = f_rkusb->buf_head;
> - printf("transfer 0x%x bytes done\n",
> f_rkusb->dl_size);
> + debug("transfer 0x%x bytes done\n",
> f_rkusb->dl_size); f_rkusb->dl_size = 0;
>   rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_GOOD,
>USB_BULK_CS_WRAP_LEN);




Best regards,

Lukasz Majewski

--

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


pgpwGTkqksXuT.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 5/7] usb: rockchip: implement K_FW_LBA_ERASE_10 command

2018-07-03 Thread Lukasz Majewski
Hi Alberto,

> This command is part of the write LBA sector sequence performed by
> rkdeveloptool

You mentioned the LBA sector and the need of erasing it.

AFAIK, block devices (eMMC) perform internally erase on write.

Is this protocol also supposed to work with NAND flash? If yes, please
adjust the patch description.

Also, please extend the README.rockusb file.

> 
> Signed-off-by: Alberto Panizzo 
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
>  drivers/usb/gadget/f_rockusb.c | 46
> ++ 2 files changed, 47 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h
> b/arch/arm/include/asm/arch-rockchip/f_rockusb.h index
> f04d401..77c3a87 100644 ---
> a/arch/arm/include/asm/arch-rockchip/f_rockusb.h +++
> b/arch/arm/include/asm/arch-rockchip/f_rockusb.h @@ -62,6 +62,7 @@
> K_FW_LOW_FORMAT = 0x1C, K_FW_SET_RESET_FLAG = 0x1E,
>  K_FW_SPI_READ_10 = 0x21,
>  K_FW_SPI_WRITE_10 = 0x22,
> +K_FW_LBA_ERASE_10 = 0x25,
>  
>  K_FW_SESSION = 0X30,
>  K_FW_RESET = 0xff,
> diff --git a/drivers/usb/gadget/f_rockusb.c
> b/drivers/usb/gadget/f_rockusb.c index dbf31cb..230fbec 100644
> --- a/drivers/usb/gadget/f_rockusb.c
> +++ b/drivers/usb/gadget/f_rockusb.c
> @@ -693,6 +693,48 @@ static void cb_write_lba(struct usb_ep *ep,
> struct usb_request *req) }
>  }
>  
> +static void cb_erase_lba(struct usb_ep *ep, struct usb_request *req)
> +{
> + ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
> +  sizeof(struct fsg_bulk_cb_wrap));
> + struct f_rockusb *f_rkusb = get_rkusb();
> + int sector_count, lba, blks;
> +
> + memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
> + sector_count = (int)get_unaligned_be16(>CDB[7]);
> + lba = get_unaligned_be32(>CDB[2]);
> + f_rkusb->tag = cbw->tag;
> + debug("require erase %x sectors from lba %x\n",
> +   sector_count, lba);
> +
> + if (!f_rkusb->desc) {
> + char *type = f_rkusb->dev_type;
> + int index = f_rkusb->dev_index;
> +
> + f_rkusb->desc = blk_get_dev(type, index);
> + if (!f_rkusb->desc ||
> + f_rkusb->desc->type == DEV_TYPE_UNKNOWN) {
> + puts("invalid mmc device\n");
> + rockusb_tx_write_csw(f_rkusb->tag,
> + cbw->data_transfer_length, CSW_FAIL,
> + USB_BULK_CS_WRAP_LEN);
> + return;
> + }
> + }
> + blks = blk_derase(f_rkusb->desc, lba, sector_count);
> + if (blks != sector_count) {
> + printf("failed erasing device %s: %d\n",
> f_rkusb->dev_type,
> +f_rkusb->dev_index);
> + rockusb_tx_write_csw(f_rkusb->tag,
> + cbw->data_transfer_length, CSW_FAIL,
> + USB_BULK_CS_WRAP_LEN);
> + return;
> + }
> +
> + rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length,
> CSW_GOOD,
> +  USB_BULK_CS_WRAP_LEN);
> +}
> +
>  void __weak rkusb_set_reboot_flag(int flag)
>  {
>   struct f_rockusb *f_rkusb = get_rkusb();
> @@ -825,6 +867,10 @@ static const struct cmd_dispatch_info
> cmd_dispatch_info[] = { .cb = cb_not_support,
>   },
>   {
> + .cmd = K_FW_LBA_ERASE_10,
> + .cb = cb_erase_lba,
> + },
> + {
>   .cmd = K_FW_SESSION,
>   .cb = cb_not_support,
>   },


Best regards,

Lukasz Majewski

--

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


pgpslGHggRdB_.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 4/7] usb: rockchip: implement K_FW_LBA_READ_10 command

2018-07-03 Thread Lukasz Majewski
Hi Alberto,

> It is now possible to read from block device al logic layer.
  ^^^ - what do you
  mean by logic layer?

I suppose this code is to read N blocks (512B) from eMMC device?

NOTE:

Please consider adding tests into test/py/tests/ as we already have
such tests for test_ums.py and test_dfu.py

> Corresponding command on workstation is:
> rkdeveloptool rl   
> 
> Signed-off-by: Alberto Panizzo 
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h |   2 +
>  drivers/usb/gadget/f_rockusb.c | 102
> - 2 files changed, 103 insertions(+), 1
> deletion(-)
> 
> diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h
> b/arch/arm/include/asm/arch-rockchip/f_rockusb.h index
> f5cad8e..f04d401 100644 ---
> a/arch/arm/include/asm/arch-rockchip/f_rockusb.h +++
> b/arch/arm/include/asm/arch-rockchip/f_rockusb.h @@ -120,6 +120,8 @@
> struct f_rockusb { unsigned int lba;
>   unsigned int dl_size;
>   unsigned int dl_bytes;
> + unsigned int ul_size;
> + unsigned int ul_bytes;

We had similar problem with Samsung's THOR. unsigned int may be too
little in a while (we got int) ...

>   struct blk_desc *desc;
>   int reboot_flag;
>   void *buf;
> diff --git a/drivers/usb/gadget/f_rockusb.c
> b/drivers/usb/gadget/f_rockusb.c index 7612871..dbf31cb 100644
> --- a/drivers/usb/gadget/f_rockusb.c
> +++ b/drivers/usb/gadget/f_rockusb.c
> @@ -340,6 +340,7 @@ static int rockusb_tx_write(const char *buffer,
> unsigned int buffer_size) 
>   memcpy(in_req->buf, buffer, buffer_size);
>   in_req->length = buffer_size;
> + debug("Transferring 0x%x bytes\n", buffer_size);
>   usb_ep_dequeue(rockusb_func->in_ep, in_req);
>   ret = usb_ep_queue(rockusb_func->in_ep, in_req, 0);
>   if (ret)
> @@ -434,6 +435,79 @@ static unsigned int rx_bytes_expected(struct
> usb_ep *ep) return rx_remain;
>  }
>  
> +/* usb_request complete call back to handle upload image */
> +static void tx_handler_ul_image(struct usb_ep *ep, struct
> usb_request *req) +{
> +#define RBUFFER_SIZE 4096

This can be moved to header file.

> + ALLOC_CACHE_ALIGN_BUFFER(char, rbuffer, RBUFFER_SIZE);
> + struct f_rockusb *f_rkusb = get_rkusb();
> + struct usb_request *in_req = rockusb_func->in_req;
> + int ret;
> +
> + if (!f_rkusb->desc) {
> + char *type = f_rkusb->dev_type;
> + int index = f_rkusb->dev_index;
> +
> + f_rkusb->desc = blk_get_dev(type, index);
> + if (!f_rkusb->desc ||
> + f_rkusb->desc->type == DEV_TYPE_UNKNOWN) {
> + puts("invalid mmc device\n");
> + rockusb_tx_write_csw(f_rkusb->tag, 0,
> CSW_FAIL,
> +  USB_BULK_CS_WRAP_LEN);
> + return;
> + }
> + }
> +
> + /* Print error status of previous transfer */
> + if (req->status)
> + debug("status: %d ep '%s' trans: %d len %d\n",
> req->status,
> +   ep->name, req->actual, req->length);
> +
> + /* On transfer complete reset in_req and feedback host with
> CSW_GOOD */
> + if (f_rkusb->ul_bytes >= f_rkusb->ul_size) {
> + in_req->length = 0;
> + in_req->complete = rockusb_complete;
> +
> + rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_GOOD,
> +  USB_BULK_CS_WRAP_LEN);
> + return;
> + }
> +
> + /* Proceed with current chunk */
> + unsigned int transfer_size = f_rkusb->ul_size -
> f_rkusb->ul_bytes; +
> + if (transfer_size > RBUFFER_SIZE)
> + transfer_size = RBUFFER_SIZE;
> + /* Read at least one block */
> + unsigned int blkcount = (transfer_size + 511) / 512;
> +
> + debug("ul %x bytes, %x blks, read lba %x, ul_size:%x,
> ul_bytes:%x, ",
> +   transfer_size, blkcount, f_rkusb->lba,
> +   f_rkusb->ul_size, f_rkusb->ul_bytes);
> +
> + int blks = blk_dread(f_rkusb->desc, f_rkusb->lba, blkcount,
> rbuffer); +
> + if (blks != blkcount) {
> + printf("failed reading from device %s: %d\n",
> +f_rkusb->dev_type, f_rkusb->dev_index);
> + rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_FAIL,
> +  USB_BULK_CS_WRAP_LEN);
> + return;
> + }
> + f_rkusb->lba += blkcount;
> + f_rkusb->ul_bytes += transfer_size;
> +
> + /* Proceed with USB request */
> + memcpy(in_req->buf, rbuffer, transfer_size);
> + in_req->length = transfer_size;
> + in_req->complete = tx_handler_ul_image;
> + printf("Uploading 0x%x bytes\n", transfer_size);
> + usb_ep_dequeue(rockusb_func->in_ep, in_req);
> + ret = usb_ep_queue(rockusb_func->in_ep, in_req, 0);
> + if (ret)
> + printf("Error %d on queue\n", ret);
> +}
> +
>  /* usb_request complete 

Re: [U-Boot] [PATCH 1/2] arm: timer: factor out FSL arch timer erratum workaround

2018-07-03 Thread André Przywara
On 07/03/2018 09:59 PM, Alexander Graf wrote:
> 
> 
>> Am 03.07.2018 um 22:51 schrieb Andreas Färber :
>>
>>> Am 03.07.2018 um 01:08 schrieb Andreas Färber:
 Am 02.07.2018 um 10:01 schrieb Jagan Teki:
> On Wed, Jun 27, 2018 at 6:12 AM, Andre Przywara  
> wrote:
> At the moment we have the workaround for the Freescale arch timer
> erratum A-008585 merged into the generic timer_read_counter() routine.
> Split those two up, so that we can add other errata workaround more
> easily. Also add an explaining comment on the way.
>
> Signed-off-by: Andre Przywara 
> ---

 Applied both patches, to u-boot-sunxi/master
>>>
>>> Tested both on top of v2018.07-rc2, fixes the boot for me.
>>
>> Actually I saw it again just now, without having touched U-Boot at all.
>> Unplugged power, retried, worked. So it seems we've reduced the
>> likelihood, but something might still be astray...
> 
> So maybe we need to instead apply some logic that loops until cnt == 
> prev_cnt+1?
> 
> Also, is there any way to just trap counter reads from EL3?

I can't find anything for EL3, and I believe we can't trap the virtual
timer at all except for EL0. Besides, that would be rather costly. The
current solution normally gets away with just one sysreg read, so it
would be just the comparison overhead we have to pay. You don't want to
give this away easily.

> It'd be quite tedious to fix up all OSs out there.

Well, bad luck, it's a hardware erratum - and not the first one in this
area. So chances are you can add just another one quite easily, as we do
in Linux - where we actually have somewhat of an "arch timer errata
framework".

Cheers,
Andre.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 2/7] usb: rockchip: implement skeleton for K_FW_GET_CHIP_VER command

2018-07-03 Thread Lukasz Majewski
Hi Alberto,

> Chip Version is a string saved in BOOTROM address space Little Endian.
> 
> Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030
> which brings:  320A20140813V200
> 
> Note that memory version do invert MSB/LSB so printing the char
> buffer will show: A02341023180002V
> 
> Signed-off-by: Alberto Panizzo 
> ---
>  drivers/usb/gadget/f_rockusb.c | 38
> +- 1 file changed, 37
> insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/gadget/f_rockusb.c
> b/drivers/usb/gadget/f_rockusb.c index a39ad51..7612871 100644
> --- a/drivers/usb/gadget/f_rockusb.c
> +++ b/drivers/usb/gadget/f_rockusb.c
> @@ -532,6 +532,42 @@ static void cb_read_storage_id(struct usb_ep
> *ep, struct usb_request *req) CSW_GOOD);
>  }
>  
> +int __weak rk_get_bootrom_chip_version(unsigned int *chip_info, int
> size) +{
> + return 0;
> +}
> +
> +static void cb_get_chip_version(struct usb_ep *ep, struct
> usb_request *req) +{
> + ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
> +  sizeof(struct fsg_bulk_cb_wrap));
> + unsigned int chip_info[4], i;
> +
> + memset(chip_info, 0, sizeof(chip_info));
> + rk_get_bootrom_chip_version(chip_info, 4);
> +
> + /*
> +  * Chip Version is a string saved in BOOTROM address space
> Little Endian
> +  *
> +  * Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030
> +  * which brings:  320A20140813V200
> +  *
> +  * Note that memory version do invert MSB/LSB so printing
> the char
> +  * buffer will show: A02341023180002V
> +  */
> + printf("read chip version: ");
> + for (i = 0; i < 16; i++) {
> + int shift = (3 - (i % 4)) * 8;
> +
> + printf("%c", (char)((chip_info[i / 4] >> shift) &
> 0xFF));

A lot of magic numbers. Just to ask - isn't this the same type of
conversion as we got with the network code?

Cannot we have simple macro (or static inline) with byte swap called
three times?

> + }
> + printf("\n");
> + memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
> + rockusb_tx_write((char *)chip_info, sizeof(chip_info));
^ [1]

> + rockusb_tx_write_csw_on_complete(cbw->tag,
^-[2]

> cbw->data_transfer_length,
> +  CSW_GOOD);

Just to be sure if I understand the protocol -> you write the data in
[1]
And then immediately you prepare next block (structure) [2] to be
written back after receiving reply data from host?

Is this behaviour in sync with README in ./doc/README.rockusb ?

> +}
> +
>  static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
>  {
>   ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
> @@ -670,7 +706,7 @@ static const struct cmd_dispatch_info
> cmd_dispatch_info[] = { },
>   {
>   .cmd = K_FW_GET_CHIP_VER,
> - .cb = cb_not_support,
> + .cb = cb_get_chip_version,
>   },
>   {
>   .cmd = K_FW_LOW_FORMAT,




Best regards,

Lukasz Majewski

--

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


pgpFsoZTc_f_I.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/7] usb: rockchip: fix command failed on host side due to missing data

2018-07-03 Thread Lukasz Majewski
Hi Alberto,

> Two consecutive rockusb_tx_write without waiting for request complete
> do results in transfer reset of first request and thus no or
> incomplete data transfer. This because rockusb_tx_write do use just
> une request to keep serialization.
> 
> So calls like:
> rockusb_tx_write_str(emmc_id);
> rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD);
> 
> was succeeding only when DEBUG was defined because the time spent
> printing debug info was enough for request to complete.

Serialization by printf. 

> 
> This patch add a way to postpone sending csw after first
> rockusb_tx_write is completed (indeed inside rockusb_complete) fixing
> execution of: $ rkdeveloptool rfi
> when DEBUG is not defined.
> 
> Signed-off-by: Alberto Panizzo 
> ---
>  arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
>  drivers/usb/gadget/f_rockusb.c | 37
> ++ 2 files changed, 33 insertions(+), 5
> deletions(-)
> 
> diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h
> b/arch/arm/include/asm/arch-rockchip/f_rockusb.h index
> 0b62771..f5cad8e 100644 ---
> a/arch/arm/include/asm/arch-rockchip/f_rockusb.h +++
> b/arch/arm/include/asm/arch-rockchip/f_rockusb.h @@ -124,6 +124,7 @@
> struct f_rockusb { int reboot_flag;
>   void *buf;
>   void *buf_head;
> + struct bulk_cs_wrap *next_csw;
>  };
>  
>  /* init rockusb device, tell rockusb which device you want to
> read/write*/ diff --git a/drivers/usb/gadget/f_rockusb.c
> b/drivers/usb/gadget/f_rockusb.c index b8833d0..a39ad51 100644
> --- a/drivers/usb/gadget/f_rockusb.c
> +++ b/drivers/usb/gadget/f_rockusb.c
> @@ -97,6 +97,7 @@ static struct usb_gadget_strings *rkusb_strings[] =
> { 
>  static struct f_rockusb *rockusb_func;
>  static void rx_handler_command(struct usb_ep *ep, struct usb_request
> *req); +static int rockusb_tx_write(const char *buffer, unsigned int
> buffer_size); static int rockusb_tx_write_csw(u32 tag, int residue,
> u8 status, int size); 
>  struct f_rockusb *get_rkusb(void)
> @@ -136,11 +137,22 @@ struct usb_endpoint_descriptor *hs)
>  
>  static void rockusb_complete(struct usb_ep *ep, struct usb_request
> *req) {
> + struct f_rockusb *f_rkusb = get_rkusb();
>   int status = req->status;
>  
> - if (!status)
> - return;
> - debug("status: %d ep '%s' trans: %d\n", status, ep->name,
> req->actual);
> + if (status)
> + debug("status: %d ep '%s' trans: %d\n",
> +   status, ep->name, req->actual);
> +
> + /* Send Command Status on previous transfer complete */
> + if (f_rkusb->next_csw) {
  - isn't this a bit misleading? We send
 the status for the previous transfer.

> +#ifdef DEBUG
> + printcsw((char *)f_rkusb->next_csw);
> +#endif
> + rockusb_tx_write((char *)f_rkusb->next_csw,
> +  USB_BULK_CS_WRAP_LEN);
> + }
> + f_rkusb->next_csw = NULL;
>  }
>  
>  /* config the rockusb device*/
> @@ -388,6 +400,21 @@ static int rockusb_tx_write_csw(u32 tag, int
> residue, u8 status, int size) return rockusb_tx_write((char *)csw,
> size); }
>  
> +struct bulk_cs_wrap g_next_csw;

You have added the pointer to struct bulk_cs_wrap_g to struct
f_rockusb, and here we do have global definition.

Two issues with cache; alignment and padding.

Maybe it would be better to allocate it and store pointer int struct
f_rockusb ?

> +static void rockusb_tx_write_csw_on_complete(u32 tag, int residue,
> u8 status) +{
> + struct f_rockusb *f_rkusb = get_rkusb();
> +
> + g_next_csw.signature = cpu_to_le32(USB_BULK_CS_SIG);
> + g_next_csw.tag = tag;
> + g_next_csw.residue = cpu_to_be32(residue);
> + g_next_csw.status = status;
> +#ifdef DEBUG
> + printcsw((char *)_next_csw);
> +#endif
> + f_rkusb->next_csw = _next_csw;
> +}
> +
>  static unsigned int rx_bytes_expected(struct usb_ep *ep)
>  {
>   struct f_rockusb *f_rkusb = get_rkusb();
> @@ -501,8 +528,8 @@ static void cb_read_storage_id(struct usb_ep *ep,
> struct usb_request *req) printf("read storage id\n");
>   memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
>   rockusb_tx_write_str(emmc_id);
> - rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length,
> CSW_GOOD,
> -  USB_BULK_CS_WRAP_LEN);
> + rockusb_tx_write_csw_on_complete(cbw->tag,
> cbw->data_transfer_length,
> +  CSW_GOOD);

It seems like you are preparing the content of the csw structure to be
ready when the completion is called. Am I right?

What I'm concerned about - with your patch we do have two functions
with almost the same code - namely rockusb_tx_write_csw() and
rockusb_tx_write_csw_on_complete(). 

Would it be possible to unify (reuse) the code?

One more remark - shouldn't we set content of g_next_csw in the
rockusb_complete() ?

>  }
>  
>  static void cb_write_lba(struct usb_ep *ep, struct 

Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread André Przywara
On 07/03/2018 03:52 PM, Tom Rini wrote:
> On Tue, Jul 03, 2018 at 06:06:37PM +0530, Jagan Teki wrote:
>> On Tue, Jul 3, 2018 at 3:10 AM, Tom Rini  wrote:
>>> On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
 On 07/02/2018 10:53 PM, Jagan Teki wrote:
> During usb shutdown or 'usb reset' all the necessary clocks
> on the specific controller will disable. Usually this shutdown
> happen during U-Boot proper handoff to Linux.

 No, 'usb reset' can be triggered by the user any time.
>>>
>>> Yes, and it's also triggered as part of the hand-off, which is the use
>>> case in question.
>>>
> There is an issue in Allwinner A64, is during OHCI1 shutdown
> the controller is unable to access the register space
> so the Linux boot hangs at this place.

 This doesn't make any sense, Linux should enable the necessary clock
 before accessing any controller registers, unless there is a bug in Linux.
>>>
>>> Should but doesn't always?  So yes, there's possibly / hopefully a bug
>>> in the dts files.
>>>
> This particular condition occur when we enable both the
> controllers, so this patch will disable OHCI1 and EHCI1 for
> bananapi-m64 and nanopi-a64 boards. It will re-enable the same
> once the issue got fixed.
>
> Log:
> => usb reset
> resetting USB...
>
> PHY0: mask = 0x101, usb_clk_cfg = 0x30202
> sunxi_musb_exit: ahb_gate0  = 0x33004540, reset0_cfg = 0x33004540
> EHCI failed to shut down host controller.
> ehci_usb_remove: ahb_gate0  = 0x32004540, reset0_cfg = 0x32004540
> ohci_usb_remove: ahb_gate0  = 0x22004540, reset0_cfg = 0x22004540
> ohci_usb_remove: mask = 0x1, usb_clk_cfg = 0x20202
>
> PHY1: mask = 0x202, usb_clk_cfg = 0x0
> ehci_usb_remove: ahb_gate0  = 0x20004540, reset0_cfg = 0x20004540

 Why is this usb reset so noisy ?
>>>
>>> ... I would assume additional debug messages.
>>>
> << hang here >>

 Please fix this properly, this patch is pure attempt to hide a bug.
 NAK from me.
>>>
>>> Well, the point of this patch as Jagan says is to hide the bug for the
>>> release so that Linux can boot, which is an important case.
>>>
>>> Jagan, can you bisect down to when this started happening?  I assume
>>> it's a recent'ish thing.  Thanks!
>>
>> commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
>> is effecting this change, but functionally this commit is proper but
>> handling clock directly in drivers look not effective particularly in
>> the case of A64 where sharing of clocks and gates between OHCI and
>> EHCI.
>>
>> Here are the possible changes to look at it.
>>
>> 1)  with this patch, disabling [oe]hci1 controllers on two boards (bpi
>> and nanopi):
>>   this change specific to two boards, rest of A64 boards are OK.
>>
>> 2)  turn-off gate and clocks for H3/H5/A64 from Andre [1]
>>   this would change all H3/H5/A64 shutdown behavior not to disable
>> gates and clocks during .remove
>>
>> 3)  turn-off clock only for A64
>>  this would fix, two board problems in 1) and also specific to A64.
>>
>> 4)  add counter mechanism to disable all clock at once, suggested by Marek
>>  either we can add counter mechanism to A64 or for all other SOC.
>> this need to test in all Allwinner boards if the counter is globally
>> managed in ohci-sunxi.c
>>
>> Again, there is an ongoing work for syncing all DT changes from Linux,
>> by Andre. 1) change will update later in the release. rest of 2), 3)
>> and 4) will make changes in driver [eo]hci-sunxi.c and this drivers
>> indeed remove once we have dm clock which would also planned for
>> upcoming releases.
>>
>> [1] https://patchwork.ozlabs.org/patch/938279/
> 
> Adding a few more people to the list.  It looks like b62cdbddedc3 was in
> response to fef73766d9ad.  So, this close to the release, what do we
> need to do to get things back to the state things were in for v2018.05?
> And then what are the combinations that aren't working and need to be
> addressed again in v2018.09 so that we can make forward progress?

Without going into much detail here, the clock dependency for two
companion controllers on those A64 is weird, and we hack it somehow
since we don't have a DM_CLK driver for sunxi (yet, see below). That
works for init, since we just set already set bits. For shutdown, we
*happen* to take down the AHB gate clocks and resets correctly, because
the order of shutdown matches the dependency (EHCI first, the OHCI). Not
sure if this is intentional, though. It's fragile, but works.

What we don't (and can't easily) model is another oddity: the USB clock
for [OE]HCI0 is actually the parent of [OE]HCI1. So if we need to bring
down *two* controllers and do it in the natural order, the second one is
dead before it can be disabled properly.

This was somewhat hidden before, since we had only one controller in
operation. b62cdbddedc3 somewhat fixed that, but the DT for the Pine64
(which was 

Re: [U-Boot] [PATCH 1/2] arm: timer: factor out FSL arch timer erratum workaround

2018-07-03 Thread Alexander Graf


> Am 03.07.2018 um 22:51 schrieb Andreas Färber :
> 
>> Am 03.07.2018 um 01:08 schrieb Andreas Färber:
>>> Am 02.07.2018 um 10:01 schrieb Jagan Teki:
 On Wed, Jun 27, 2018 at 6:12 AM, Andre Przywara  
 wrote:
 At the moment we have the workaround for the Freescale arch timer
 erratum A-008585 merged into the generic timer_read_counter() routine.
 Split those two up, so that we can add other errata workaround more
 easily. Also add an explaining comment on the way.
 
 Signed-off-by: Andre Przywara 
 ---
>>> 
>>> Applied both patches, to u-boot-sunxi/master
>> 
>> Tested both on top of v2018.07-rc2, fixes the boot for me.
> 
> Actually I saw it again just now, without having touched U-Boot at all.
> Unplugged power, retried, worked. So it seems we've reduced the
> likelihood, but something might still be astray...

So maybe we need to instead apply some logic that loops until cnt == prev_cnt+1?

Also, is there any way to just trap counter reads from EL3? It'd be quite 
tedious to fix up all OSs out there.

Alex


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


Re: [U-Boot] [PATCH 1/2] arm: timer: factor out FSL arch timer erratum workaround

2018-07-03 Thread Andreas Färber
Am 03.07.2018 um 01:08 schrieb Andreas Färber:
> Am 02.07.2018 um 10:01 schrieb Jagan Teki:
>> On Wed, Jun 27, 2018 at 6:12 AM, Andre Przywara  
>> wrote:
>>> At the moment we have the workaround for the Freescale arch timer
>>> erratum A-008585 merged into the generic timer_read_counter() routine.
>>> Split those two up, so that we can add other errata workaround more
>>> easily. Also add an explaining comment on the way.
>>>
>>> Signed-off-by: Andre Przywara 
>>> ---
>>
>> Applied both patches, to u-boot-sunxi/master
> 
> Tested both on top of v2018.07-rc2, fixes the boot for me.

Actually I saw it again just now, without having touched U-Boot at all.
Unplugged power, retried, worked. So it seems we've reduced the
likelihood, but something might still be astray...

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH 7/7] usb: rockchip: boost up write speed from 4MB/s to 15MB/s

2018-07-03 Thread Alberto Panizzo
Speedup transfers increasing the max chunk size.
Buffers are allocated with memalign thus developer is noticed when heap is
full and in current configuration a buffer allocation of 64K till now
is safe.

Signed-off-by: Alberto Panizzo 
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h 
b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
index 77c3a87..41c6188 100644
--- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h
+++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
@@ -19,7 +19,7 @@
 #define RX_ENDPOINT_MAXIMUM_PACKET_SIZE_1_1  0x0040
 #define TX_ENDPOINT_MAXIMUM_PACKET_SIZE  0x0040
 
-#define EP_BUFFER_SIZE 4096
+#define EP_BUFFER_SIZE 65536
 /*
  * EP_BUFFER_SIZE must always be an integral multiple of maxpacket size
  * (64 or 512 or 1024), else we break on certain controllers like DWC3
-- 
2.7.4

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


[U-Boot] [PATCH 2/7] usb: rockchip: implement skeleton for K_FW_GET_CHIP_VER command

2018-07-03 Thread Alberto Panizzo
Chip Version is a string saved in BOOTROM address space Little Endian.

Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030
which brings:  320A20140813V200

Note that memory version do invert MSB/LSB so printing the char
buffer will show: A02341023180002V

Signed-off-by: Alberto Panizzo 
---
 drivers/usb/gadget/f_rockusb.c | 38 +-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index a39ad51..7612871 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -532,6 +532,42 @@ static void cb_read_storage_id(struct usb_ep *ep, struct 
usb_request *req)
 CSW_GOOD);
 }
 
+int __weak rk_get_bootrom_chip_version(unsigned int *chip_info, int size)
+{
+   return 0;
+}
+
+static void cb_get_chip_version(struct usb_ep *ep, struct usb_request *req)
+{
+   ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
+sizeof(struct fsg_bulk_cb_wrap));
+   unsigned int chip_info[4], i;
+
+   memset(chip_info, 0, sizeof(chip_info));
+   rk_get_bootrom_chip_version(chip_info, 4);
+
+   /*
+* Chip Version is a string saved in BOOTROM address space Little Endian
+*
+* Ex for rk3288: 0x33323041 0x32303134 0x30383133 0x56323030
+* which brings:  320A20140813V200
+*
+* Note that memory version do invert MSB/LSB so printing the char
+* buffer will show: A02341023180002V
+*/
+   printf("read chip version: ");
+   for (i = 0; i < 16; i++) {
+   int shift = (3 - (i % 4)) * 8;
+
+   printf("%c", (char)((chip_info[i / 4] >> shift) & 0xFF));
+   }
+   printf("\n");
+   memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
+   rockusb_tx_write((char *)chip_info, sizeof(chip_info));
+   rockusb_tx_write_csw_on_complete(cbw->tag, cbw->data_transfer_length,
+CSW_GOOD);
+}
+
 static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
 {
ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
@@ -670,7 +706,7 @@ static const struct cmd_dispatch_info cmd_dispatch_info[] = 
{
},
{
.cmd = K_FW_GET_CHIP_VER,
-   .cb = cb_not_support,
+   .cb = cb_get_chip_version,
},
{
.cmd = K_FW_LOW_FORMAT,
-- 
2.7.4

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


[U-Boot] [PATCH 3/7] rockchip: rk3288: implement reading chip version from bootrom code

2018-07-03 Thread Alberto Panizzo
This allows rockusb code to reply correctly to K_FW_GET_CHIP_VER
command.

On RK3288 chip version is at 0x4ff0 and on tested hardware it
corresponds at the string "320A20140813V200"

Signed-off-by: Alberto Panizzo 
---
 arch/arm/mach-rockchip/rk3288/Makefile |  1 +
 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c | 30 ++
 2 files changed, 31 insertions(+)
 create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c

diff --git a/arch/arm/mach-rockchip/rk3288/Makefile 
b/arch/arm/mach-rockchip/rk3288/Makefile
index a0033a0..da0eb4a 100644
--- a/arch/arm/mach-rockchip/rk3288/Makefile
+++ b/arch/arm/mach-rockchip/rk3288/Makefile
@@ -7,3 +7,4 @@
 obj-y += clk_rk3288.o
 obj-y += rk3288.o
 obj-y += syscon_rk3288.o
+obj-$(CONFIG_USB_FUNCTION_ROCKUSB) += rockusb_rk3288.o
diff --git a/arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c 
b/arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c
new file mode 100644
index 000..62057c1
--- /dev/null
+++ b/arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018 Amarula Solutions
+ * Written by Alberto Panizzo 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ROM_PHYS   0x
+
+#define ROM_CHIP_VER_ADDR_ADDR (ROM_PHYS + 0x4FF0)
+#define ROM_CHIP_VER_ADDR_SIZE 16
+
+int rk_get_bootrom_chip_version(unsigned int *chip_info, int size)
+{
+   if (!chip_info)
+   return -1;
+   if (size < ROM_CHIP_VER_ADDR_SIZE / sizeof(int))
+   return -1;
+
+   memcpy((char *)chip_info, (char *)ROM_CHIP_VER_ADDR_ADDR,
+  ROM_CHIP_VER_ADDR_SIZE);
+
+   return 0;
+}
-- 
2.7.4

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


[U-Boot] [PATCH 6/7] usb: rockchip: be quiet on serial port while transferring data

2018-07-03 Thread Alberto Panizzo
While downloading or uploading megabytes of data we had thousands of
dummy lines like:

transfer 0x1 bytes done
OR
Uploading 0x1000 bytes

even on non-debug builds. This because transfers are chunked and
code running on target does not have any clue about when the current
chunk is the last one.

Signed-off-by: Alberto Panizzo 
---
 drivers/usb/gadget/f_rockusb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index 230fbec..4fc8484 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -501,7 +501,7 @@ static void tx_handler_ul_image(struct usb_ep *ep, struct 
usb_request *req)
memcpy(in_req->buf, rbuffer, transfer_size);
in_req->length = transfer_size;
in_req->complete = tx_handler_ul_image;
-   printf("Uploading 0x%x bytes\n", transfer_size);
+   debug("Uploading 0x%x bytes\n", transfer_size);
usb_ep_dequeue(rockusb_func->in_ep, in_req);
ret = usb_ep_queue(rockusb_func->in_ep, in_req, 0);
if (ret)
@@ -563,7 +563,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct 
usb_request *req)
req->complete = rx_handler_command;
req->length = EP_BUFFER_SIZE;
f_rkusb->buf = f_rkusb->buf_head;
-   printf("transfer 0x%x bytes done\n", f_rkusb->dl_size);
+   debug("transfer 0x%x bytes done\n", f_rkusb->dl_size);
f_rkusb->dl_size = 0;
rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_GOOD,
 USB_BULK_CS_WRAP_LEN);
-- 
2.7.4

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


[U-Boot] [PATCH 4/7] usb: rockchip: implement K_FW_LBA_READ_10 command

2018-07-03 Thread Alberto Panizzo
It is now possible to read from block device al logic layer.
Corresponding command on workstation is:
rkdeveloptool rl   

Signed-off-by: Alberto Panizzo 
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |   2 +
 drivers/usb/gadget/f_rockusb.c | 102 -
 2 files changed, 103 insertions(+), 1 deletion(-)

diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h 
b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
index f5cad8e..f04d401 100644
--- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h
+++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
@@ -120,6 +120,8 @@ struct f_rockusb {
unsigned int lba;
unsigned int dl_size;
unsigned int dl_bytes;
+   unsigned int ul_size;
+   unsigned int ul_bytes;
struct blk_desc *desc;
int reboot_flag;
void *buf;
diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index 7612871..dbf31cb 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -340,6 +340,7 @@ static int rockusb_tx_write(const char *buffer, unsigned 
int buffer_size)
 
memcpy(in_req->buf, buffer, buffer_size);
in_req->length = buffer_size;
+   debug("Transferring 0x%x bytes\n", buffer_size);
usb_ep_dequeue(rockusb_func->in_ep, in_req);
ret = usb_ep_queue(rockusb_func->in_ep, in_req, 0);
if (ret)
@@ -434,6 +435,79 @@ static unsigned int rx_bytes_expected(struct usb_ep *ep)
return rx_remain;
 }
 
+/* usb_request complete call back to handle upload image */
+static void tx_handler_ul_image(struct usb_ep *ep, struct usb_request *req)
+{
+#define RBUFFER_SIZE   4096
+   ALLOC_CACHE_ALIGN_BUFFER(char, rbuffer, RBUFFER_SIZE);
+   struct f_rockusb *f_rkusb = get_rkusb();
+   struct usb_request *in_req = rockusb_func->in_req;
+   int ret;
+
+   if (!f_rkusb->desc) {
+   char *type = f_rkusb->dev_type;
+   int index = f_rkusb->dev_index;
+
+   f_rkusb->desc = blk_get_dev(type, index);
+   if (!f_rkusb->desc ||
+   f_rkusb->desc->type == DEV_TYPE_UNKNOWN) {
+   puts("invalid mmc device\n");
+   rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_FAIL,
+USB_BULK_CS_WRAP_LEN);
+   return;
+   }
+   }
+
+   /* Print error status of previous transfer */
+   if (req->status)
+   debug("status: %d ep '%s' trans: %d len %d\n", req->status,
+ ep->name, req->actual, req->length);
+
+   /* On transfer complete reset in_req and feedback host with CSW_GOOD */
+   if (f_rkusb->ul_bytes >= f_rkusb->ul_size) {
+   in_req->length = 0;
+   in_req->complete = rockusb_complete;
+
+   rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_GOOD,
+USB_BULK_CS_WRAP_LEN);
+   return;
+   }
+
+   /* Proceed with current chunk */
+   unsigned int transfer_size = f_rkusb->ul_size - f_rkusb->ul_bytes;
+
+   if (transfer_size > RBUFFER_SIZE)
+   transfer_size = RBUFFER_SIZE;
+   /* Read at least one block */
+   unsigned int blkcount = (transfer_size + 511) / 512;
+
+   debug("ul %x bytes, %x blks, read lba %x, ul_size:%x, ul_bytes:%x, ",
+ transfer_size, blkcount, f_rkusb->lba,
+ f_rkusb->ul_size, f_rkusb->ul_bytes);
+
+   int blks = blk_dread(f_rkusb->desc, f_rkusb->lba, blkcount, rbuffer);
+
+   if (blks != blkcount) {
+   printf("failed reading from device %s: %d\n",
+  f_rkusb->dev_type, f_rkusb->dev_index);
+   rockusb_tx_write_csw(f_rkusb->tag, 0, CSW_FAIL,
+USB_BULK_CS_WRAP_LEN);
+   return;
+   }
+   f_rkusb->lba += blkcount;
+   f_rkusb->ul_bytes += transfer_size;
+
+   /* Proceed with USB request */
+   memcpy(in_req->buf, rbuffer, transfer_size);
+   in_req->length = transfer_size;
+   in_req->complete = tx_handler_ul_image;
+   printf("Uploading 0x%x bytes\n", transfer_size);
+   usb_ep_dequeue(rockusb_func->in_ep, in_req);
+   ret = usb_ep_queue(rockusb_func->in_ep, in_req, 0);
+   if (ret)
+   printf("Error %d on queue\n", ret);
+}
+
 /* usb_request complete call back to handle down load image */
 static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req)
 {
@@ -568,6 +642,32 @@ static void cb_get_chip_version(struct usb_ep *ep, struct 
usb_request *req)
 CSW_GOOD);
 }
 
+static void cb_read_lba(struct usb_ep *ep, struct usb_request *req)
+{
+   ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
+sizeof(struct fsg_bulk_cb_wrap));
+   struct f_rockusb *f_rkusb = get_rkusb();
+   int sector_count;
+
+   

[U-Boot] [PATCH 5/7] usb: rockchip: implement K_FW_LBA_ERASE_10 command

2018-07-03 Thread Alberto Panizzo
This command is part of the write LBA sector sequence performed by
rkdeveloptool

Signed-off-by: Alberto Panizzo 
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
 drivers/usb/gadget/f_rockusb.c | 46 ++
 2 files changed, 47 insertions(+)

diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h 
b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
index f04d401..77c3a87 100644
--- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h
+++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
@@ -62,6 +62,7 @@ K_FW_LOW_FORMAT = 0x1C,
 K_FW_SET_RESET_FLAG = 0x1E,
 K_FW_SPI_READ_10 = 0x21,
 K_FW_SPI_WRITE_10 = 0x22,
+K_FW_LBA_ERASE_10 = 0x25,
 
 K_FW_SESSION = 0X30,
 K_FW_RESET = 0xff,
diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index dbf31cb..230fbec 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -693,6 +693,48 @@ static void cb_write_lba(struct usb_ep *ep, struct 
usb_request *req)
}
 }
 
+static void cb_erase_lba(struct usb_ep *ep, struct usb_request *req)
+{
+   ALLOC_CACHE_ALIGN_BUFFER(struct fsg_bulk_cb_wrap, cbw,
+sizeof(struct fsg_bulk_cb_wrap));
+   struct f_rockusb *f_rkusb = get_rkusb();
+   int sector_count, lba, blks;
+
+   memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
+   sector_count = (int)get_unaligned_be16(>CDB[7]);
+   lba = get_unaligned_be32(>CDB[2]);
+   f_rkusb->tag = cbw->tag;
+   debug("require erase %x sectors from lba %x\n",
+ sector_count, lba);
+
+   if (!f_rkusb->desc) {
+   char *type = f_rkusb->dev_type;
+   int index = f_rkusb->dev_index;
+
+   f_rkusb->desc = blk_get_dev(type, index);
+   if (!f_rkusb->desc ||
+   f_rkusb->desc->type == DEV_TYPE_UNKNOWN) {
+   puts("invalid mmc device\n");
+   rockusb_tx_write_csw(f_rkusb->tag,
+   cbw->data_transfer_length, CSW_FAIL,
+   USB_BULK_CS_WRAP_LEN);
+   return;
+   }
+   }
+   blks = blk_derase(f_rkusb->desc, lba, sector_count);
+   if (blks != sector_count) {
+   printf("failed erasing device %s: %d\n", f_rkusb->dev_type,
+  f_rkusb->dev_index);
+   rockusb_tx_write_csw(f_rkusb->tag,
+   cbw->data_transfer_length, CSW_FAIL,
+   USB_BULK_CS_WRAP_LEN);
+   return;
+   }
+
+   rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD,
+USB_BULK_CS_WRAP_LEN);
+}
+
 void __weak rkusb_set_reboot_flag(int flag)
 {
struct f_rockusb *f_rkusb = get_rkusb();
@@ -825,6 +867,10 @@ static const struct cmd_dispatch_info cmd_dispatch_info[] 
= {
.cb = cb_not_support,
},
{
+   .cmd = K_FW_LBA_ERASE_10,
+   .cb = cb_erase_lba,
+   },
+   {
.cmd = K_FW_SESSION,
.cb = cb_not_support,
},
-- 
2.7.4

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


[U-Boot] [PATCH 1/7] usb: rockchip: fix command failed on host side due to missing data

2018-07-03 Thread Alberto Panizzo
Two consecutive rockusb_tx_write without waiting for request complete
do results in transfer reset of first request and thus no or incomplete
data transfer. This because rockusb_tx_write do use just une request
to keep serialization.

So calls like:
rockusb_tx_write_str(emmc_id);
rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD);

was succeeding only when DEBUG was defined because the time spent
printing debug info was enough for request to complete.

This patch add a way to postpone sending csw after first rockusb_tx_write
is completed (indeed inside rockusb_complete) fixing execution of:
$ rkdeveloptool rfi
when DEBUG is not defined.

Signed-off-by: Alberto Panizzo 
---
 arch/arm/include/asm/arch-rockchip/f_rockusb.h |  1 +
 drivers/usb/gadget/f_rockusb.c | 37 ++
 2 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/arch-rockchip/f_rockusb.h 
b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
index 0b62771..f5cad8e 100644
--- a/arch/arm/include/asm/arch-rockchip/f_rockusb.h
+++ b/arch/arm/include/asm/arch-rockchip/f_rockusb.h
@@ -124,6 +124,7 @@ struct f_rockusb {
int reboot_flag;
void *buf;
void *buf_head;
+   struct bulk_cs_wrap *next_csw;
 };
 
 /* init rockusb device, tell rockusb which device you want to read/write*/
diff --git a/drivers/usb/gadget/f_rockusb.c b/drivers/usb/gadget/f_rockusb.c
index b8833d0..a39ad51 100644
--- a/drivers/usb/gadget/f_rockusb.c
+++ b/drivers/usb/gadget/f_rockusb.c
@@ -97,6 +97,7 @@ static struct usb_gadget_strings *rkusb_strings[] = {
 
 static struct f_rockusb *rockusb_func;
 static void rx_handler_command(struct usb_ep *ep, struct usb_request *req);
+static int rockusb_tx_write(const char *buffer, unsigned int buffer_size);
 static int rockusb_tx_write_csw(u32 tag, int residue, u8 status, int size);
 
 struct f_rockusb *get_rkusb(void)
@@ -136,11 +137,22 @@ struct usb_endpoint_descriptor *hs)
 
 static void rockusb_complete(struct usb_ep *ep, struct usb_request *req)
 {
+   struct f_rockusb *f_rkusb = get_rkusb();
int status = req->status;
 
-   if (!status)
-   return;
-   debug("status: %d ep '%s' trans: %d\n", status, ep->name, req->actual);
+   if (status)
+   debug("status: %d ep '%s' trans: %d\n",
+ status, ep->name, req->actual);
+
+   /* Send Command Status on previous transfer complete */
+   if (f_rkusb->next_csw) {
+#ifdef DEBUG
+   printcsw((char *)f_rkusb->next_csw);
+#endif
+   rockusb_tx_write((char *)f_rkusb->next_csw,
+USB_BULK_CS_WRAP_LEN);
+   }
+   f_rkusb->next_csw = NULL;
 }
 
 /* config the rockusb device*/
@@ -388,6 +400,21 @@ static int rockusb_tx_write_csw(u32 tag, int residue, u8 
status, int size)
return rockusb_tx_write((char *)csw, size);
 }
 
+struct bulk_cs_wrap g_next_csw;
+static void rockusb_tx_write_csw_on_complete(u32 tag, int residue, u8 status)
+{
+   struct f_rockusb *f_rkusb = get_rkusb();
+
+   g_next_csw.signature = cpu_to_le32(USB_BULK_CS_SIG);
+   g_next_csw.tag = tag;
+   g_next_csw.residue = cpu_to_be32(residue);
+   g_next_csw.status = status;
+#ifdef DEBUG
+   printcsw((char *)_next_csw);
+#endif
+   f_rkusb->next_csw = _next_csw;
+}
+
 static unsigned int rx_bytes_expected(struct usb_ep *ep)
 {
struct f_rockusb *f_rkusb = get_rkusb();
@@ -501,8 +528,8 @@ static void cb_read_storage_id(struct usb_ep *ep, struct 
usb_request *req)
printf("read storage id\n");
memcpy((char *)cbw, req->buf, USB_BULK_CB_WRAP_LEN);
rockusb_tx_write_str(emmc_id);
-   rockusb_tx_write_csw(cbw->tag, cbw->data_transfer_length, CSW_GOOD,
-USB_BULK_CS_WRAP_LEN);
+   rockusb_tx_write_csw_on_complete(cbw->tag, cbw->data_transfer_length,
+CSW_GOOD);
 }
 
 static void cb_write_lba(struct usb_ep *ep, struct usb_request *req)
-- 
2.7.4

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


[U-Boot] [PATCH 0/7] Improve rockusb support in U-Boot

2018-07-03 Thread Alberto Panizzo
rockusb protocol has been introduced by Eddie Cai in U-Boot mainline
allowing to write internal eMMC of RK3288 based boards (and potentially
all other Rockchip's CPUs).

On workstation side the open source project rkdeveloptool do implement
the rockusb protocol. You can find it on GitHub here:
https://github.com/rockchip-linux/rkdeveloptool

This patchset increase the supported functionalities on target side
allowing developers to:
- Read flash: rl command of rkdeveloptool
- Read chip version: rci command of rkdeveloptool
- Complete the write cycle implementing block erase
- Improve read/write speed

Alberto Panizzo (7):
  usb: rockchip: fix command failed on host side due to missing data
  usb: rockchip: implement skeleton for K_FW_GET_CHIP_VER command
  rockchip: rk3288: implement reading chip version from bootrom code
  usb: rockchip: implement K_FW_LBA_READ_10 command
  usb: rockchip: implement K_FW_LBA_ERASE_10 command
  usb: rockchip: be quiet on serial port while transferring data
  usb: rockchip: boost up write speed from 4MB/s to 15MB/s

 arch/arm/include/asm/arch-rockchip/f_rockusb.h |   6 +-
 arch/arm/mach-rockchip/rk3288/Makefile |   1 +
 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c |  30 
 drivers/usb/gadget/f_rockusb.c | 225 -
 4 files changed, 253 insertions(+), 9 deletions(-)
 create mode 100644 arch/arm/mach-rockchip/rk3288/rockusb_rk3288.c

-- 
2.7.4

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


Re: [U-Boot] efi_loader: runtime services implementation broken?

2018-07-03 Thread Mark Kettenis
> From: Alexander Graf 
> Date: Tue, 3 Jul 2018 22:08:05 +0200
> 
> It's only ever been tested on ARM, so maybe the x86 Linux EFI stub
> behaves differently. If set_virtual_address_map needs to be intact after
> exiting boot services, we need to move all of the relocation information
> and functionality into runtime sections as well.

The OpenBSD EFI bootloader calls ExitBootServices() before handing
control to the kernel.  The OpenBSD/arm64 kernel then calls
SetVirtualAddressmap().  This works, but probably because it happens
very early when the kernel is only using memory inside a 32M block
that has been allocated by the bootloader.

So yes,  I think that functionality needs to marked as __efi_runtime.

Cheers,

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


Re: [U-Boot] efi_loader: runtime services implementation broken?

2018-07-03 Thread Alexander Graf


On 03.07.18 21:39, Heinrich Schuchardt wrote:
> On 07/03/2018 06:51 PM, Bin Meng wrote:
>> Hi Alex,
>>
>> On Tue, Jul 3, 2018 at 11:05 PM, Alexander Graf  wrote:
>>> On 07/03/2018 04:56 PM, Bin Meng wrote:

 Hi Alex,

 On Tue, Jul 3, 2018 at 10:32 PM, Alexander Graf  wrote:
>
> On 07/03/2018 04:24 PM, Bin Meng wrote:
>>
>> Hi Alex, Heinrich,
>>
>> At present not all interfaces in lib/efi_loader/efi_runtime.c are
>> declared as __efi_runtime. But only declaring those as __efi_runtime
>> is not enough. The data these interfaces use should be declared as
>> __efi_runtime_data too. More worse, any U-Boot APIs called by these
>> interfaces should be __efi_runtime and __efi_runtime_data
>> respectively.
>
>
> Correct. This is done for everything right now, no?

 For example, efi_set_virtual_address_map() is not declared as
 __efi_runtime.
>>>
>>>
>>> Is that called post exit boot services on x86?
>>
>> Yes, if you look at efi_runtime_services structure in
>> lib/efi_loader/efi_runtime.c, you can see
>> efi_set_virtual_address_map() is hooked up there, and all interfaces
>> in efi_runtime_services are supposed to be called by OS as their names
>> indicate, runtime not bootime.
> 
> No, this not correct. Runtime functions may be called before and after
> ExitBootServices. Otherwise the EFI shell would not be able to read
> variables for instance.
> 
> The SetVirtualAdressMap service is only called after ExitBootServices.
> At that time the caller is the owner of the memory map. So this function
> must be marked as __efi_runtime.
> 
> Our current coding makes the invalid assumption that U-Boot is owner of
> the memory until SetVirtualAddressMap() is called.
> 
> Other deficiencies are that we do not provide the ConvertPointer()
> service and do not raise notify an event when SetVirtualAddressMap() is
> called.
> 
>>
>>>

>> Eventually we need mark the RAM where the whole U-Boot image lives as
>> runtime service code and data and end up leaving whole U-Boot image in
>> the RAM after kernel boots?
>
>
> Why?
>
 Unless we track down every APIs called by runtime services and mark
 them correctly as __efi_runtime code/data.
>>>
>>>
>>> That's the idea, yes. The vector should be quite small as long as we don't
>>> actually implement / reuse drivers.
>>
>> However this currently seems not to be the case ..
>>
>>>

>> Right now I am testing booting Linux on Intel Galileo with 'bootefi'
>> and kernel just hangs during the boot. Initial debugging shows that
>> kernel crashes when calling runtime service
>> efi_set_virtual_address_map().
>
> 
> Is this a regression or did it never work?

It's only ever been tested on ARM, so maybe the x86 Linux EFI stub
behaves differently. If set_virtual_address_map needs to be intact after
exiting boot services, we need to move all of the relocation information
and functionality into runtime sections as well.


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


Re: [U-Boot] efi_loader: runtime services implementation broken?

2018-07-03 Thread Heinrich Schuchardt
On 07/03/2018 06:51 PM, Bin Meng wrote:
> Hi Alex,
> 
> On Tue, Jul 3, 2018 at 11:05 PM, Alexander Graf  wrote:
>> On 07/03/2018 04:56 PM, Bin Meng wrote:
>>>
>>> Hi Alex,
>>>
>>> On Tue, Jul 3, 2018 at 10:32 PM, Alexander Graf  wrote:

 On 07/03/2018 04:24 PM, Bin Meng wrote:
>
> Hi Alex, Heinrich,
>
> At present not all interfaces in lib/efi_loader/efi_runtime.c are
> declared as __efi_runtime. But only declaring those as __efi_runtime
> is not enough. The data these interfaces use should be declared as
> __efi_runtime_data too. More worse, any U-Boot APIs called by these
> interfaces should be __efi_runtime and __efi_runtime_data
> respectively.


 Correct. This is done for everything right now, no?
>>>
>>> For example, efi_set_virtual_address_map() is not declared as
>>> __efi_runtime.
>>
>>
>> Is that called post exit boot services on x86?
> 
> Yes, if you look at efi_runtime_services structure in
> lib/efi_loader/efi_runtime.c, you can see
> efi_set_virtual_address_map() is hooked up there, and all interfaces
> in efi_runtime_services are supposed to be called by OS as their names
> indicate, runtime not bootime.

No, this not correct. Runtime functions may be called before and after
ExitBootServices. Otherwise the EFI shell would not be able to read
variables for instance.

The SetVirtualAdressMap service is only called after ExitBootServices.
At that time the caller is the owner of the memory map. So this function
must be marked as __efi_runtime.

Our current coding makes the invalid assumption that U-Boot is owner of
the memory until SetVirtualAddressMap() is called.

Other deficiencies are that we do not provide the ConvertPointer()
service and do not raise notify an event when SetVirtualAddressMap() is
called.

> 
>>
>>>
> Eventually we need mark the RAM where the whole U-Boot image lives as
> runtime service code and data and end up leaving whole U-Boot image in
> the RAM after kernel boots?


 Why?

>>> Unless we track down every APIs called by runtime services and mark
>>> them correctly as __efi_runtime code/data.
>>
>>
>> That's the idea, yes. The vector should be quite small as long as we don't
>> actually implement / reuse drivers.
> 
> However this currently seems not to be the case ..
> 
>>
>>>
> Right now I am testing booting Linux on Intel Galileo with 'bootefi'
> and kernel just hangs during the boot. Initial debugging shows that
> kernel crashes when calling runtime service
> efi_set_virtual_address_map().


Is this a regression or did it never work?

Best regards

Heinrich


 Can you please try with my efi-next branch? I added a few patches that
 allow
 gcc to put implicit data and code into the runtime section. Without
 those,
 you may get constant propagation into a the common .rodata segment for
 example.

>>> Still the same.
>>
>>
>> Ok, maybe the x86 efi stub does things different from the ARM one then.
>>
> 
> maybe :(
> 
> Regards,
> Bin
> 

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


Re: [U-Boot] [PATCH 1/5] net: phy: add ofnode node to struct phy_device

2018-07-03 Thread Grygorii Strashko



On 07/02/2018 04:10 PM, Joe Hershberger wrote:

On Thu, Jun 28, 2018 at 2:47 PM, Grygorii Strashko
 wrote:

Now the UCLASS_ETH device "node" field is owerwritten by some network drivers in
case of Ethernet PHYs which are linked to UCLASS_ETH device using
"phy-handle" DT property and when Ethernet PHY driver needs to read some
additional information from DT. In such cases following happens (in
general):

- network drivers
 priv->phydev = phy_connect(priv->bus, priv->phyaddr, dev,
priv->interface);
 <-- phydev is connected to dev which is UCLASS_ETH device

 if (priv->phy_of_handle > 0)
 dev_set_of_offset(priv->phydev->dev, priv->phy_of_handle);
 <-- phydev->dev->node is overwritten by phy-handle DT node

- PHY driver in .config() callback
 int node = dev_of_offset(dev);
 <-- PHY driver uses overwritten dev->node
 const void *fdt = gd->fdt_blob;

  if (fdtdec_get_bool(fdt, node, "property"))
 ...

As result, UCLASS_ETH device can't be used any more for DT accessing.

This patch adds additional ofnode node field to struct phy_device which can
be set explicitly by network drivers and used by PHY drivers, so
overwriting can be avoided. Also add helper function phy_get_ofnode()
which will check and return phy_device->node or dev_ofnode(phydev->dev) for
backward compatibility with existing drivers.

Signed-off-by: Grygorii Strashko 


Acked-by: Joe Hershberger 



I will resend v2 of this, as we found a bug - ofnode field need to be
explicitly initialized in phy_device_create() for !CONFIG_OF_LIVE

--
regards,
-grygorii
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Jagan Teki
On Tue, Jul 3, 2018 at 11:46 PM, Marek Vasut  wrote:
> On 07/03/2018 08:02 PM, Jagan Teki wrote:
>> On Tue, Jul 3, 2018 at 11:26 PM, Marek Vasut  wrote:
>>> On 07/03/2018 07:09 PM, Jagan Teki wrote:
 On Tue, Jul 3, 2018 at 10:14 PM, Marek Vasut  wrote:
> On 07/03/2018 05:22 PM, Andre Przywara wrote:
>> Hi,
>>
>> On 02/07/18 22:57, Marek Vasut wrote:
>>> On 07/02/2018 11:40 PM, Tom Rini wrote:
 On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
> On 07/02/2018 10:53 PM, Jagan Teki wrote:
>> During usb shutdown or 'usb reset' all the necessary clocks
>> on the specific controller will disable. Usually this shutdown
>> happen during U-Boot proper handoff to Linux.
>
> No, 'usb reset' can be triggered by the user any time.

 Yes, and it's also triggered as part of the hand-off, which is the use
 case in question.
>>>
>>> No, that's not true. The USB controllers are torn down when starting the
>>> OS, which is a different thing from usb reset, which brings them back up
>>> and rescans the bus too.
>>>
>> There is an issue in Allwinner A64, is during OHCI1 shutdown
>> the controller is unable to access the register space
>> so the Linux boot hangs at this place.
>
> This doesn't make any sense, Linux should enable the necessary clock
> before accessing any controller registers, unless there is a bug in 
> Linux.

 Should but doesn't always?  So yes, there's possibly / hopefully a bug
 in the dts files.
>>>
>>> How did you reach that conclusion about the DTS files ? There is a bug
>>> in Linux, but it's likely in the driver which doesn't enable the clock
>>> before accessing the registers.
>>>
>>> But maybe the description here is completely confusing, since the output
>>> down below would indicate this hang is still in U-Boot.
>>
>> Yes, it is. There is no bug in Linux.
>>
>> U-Boot trips over its own feet when bringing down the USB controllers:
>> It shutdowns one part (EHCI or OHCI) on the register level, then turns
>> off the clocks and reset gates. But because they are shared between
>> controllers, this turns off the other controller as well. Then it tries
>> to bring down the the second part (OHCI or EHCI, respectively) on the
>> USB register level, which hangs, because the AHB clock is already off.
>> As this just happens quite late, it looks like U-Boot already said
>> goodbye, but it actually hasn't completely finished.
>> So Linux is completely fine and the bug is entirely in U-Boot.
>> My patch [1] tries to paper^Wsolve this in a different way, though it
>> isn't perfect either. I think there is a bit more to it than I assumed
>> yesterday, so I need to go back to the code later tonight to see what's
>> really going on (I suspect it's about OHCI 0 and 1 sharing a clock, not
>> about EHCI and OHCI).
>
> Well, please keep poking.
>
> Maybe a dumb idea, but what about enabling the clock at the beginning of
> remove function to guarantee they are ON and then disabling the clock at
> the end of the function. Would that work maybe ? ie.
>
> .remove() {
> clk_enable(..);
> readl()/writel()/...
> clk_disable(..);
> }

 I've verified clock bits before disabling, and bits are enabled as
 usual. and even verified your idea of enabling before disable[2]
>>>
>>> Verified ... with what result ?
>>
>> Same hang, with properly disabling clock during OHCI0 shutdown.
>
> So the clock were enabled and yet there was a hang ? Why ?

ie what I really not understand.

>
> I was under the impression that disabling clock was the problem, maybe
> that is not the case ?
>
> Are you sure you enabled all the clock ?

Yes I'm sure about it. usb_clk_cfg = 0x30303 is final out of clock
init. It iterated twice during initialization sequence.
- 1st iteration bit 0, 8 were enabled by PHY driver and bit 16 enabled by OHCI0
- 2nd iteration bit 1, 9 were enabled by PHY driver and bit 17 enabled by OHCI1
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Marek Vasut
On 07/03/2018 08:02 PM, Jagan Teki wrote:
> On Tue, Jul 3, 2018 at 11:26 PM, Marek Vasut  wrote:
>> On 07/03/2018 07:09 PM, Jagan Teki wrote:
>>> On Tue, Jul 3, 2018 at 10:14 PM, Marek Vasut  wrote:
 On 07/03/2018 05:22 PM, Andre Przywara wrote:
> Hi,
>
> On 02/07/18 22:57, Marek Vasut wrote:
>> On 07/02/2018 11:40 PM, Tom Rini wrote:
>>> On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
 On 07/02/2018 10:53 PM, Jagan Teki wrote:
> During usb shutdown or 'usb reset' all the necessary clocks
> on the specific controller will disable. Usually this shutdown
> happen during U-Boot proper handoff to Linux.

 No, 'usb reset' can be triggered by the user any time.
>>>
>>> Yes, and it's also triggered as part of the hand-off, which is the use
>>> case in question.
>>
>> No, that's not true. The USB controllers are torn down when starting the
>> OS, which is a different thing from usb reset, which brings them back up
>> and rescans the bus too.
>>
> There is an issue in Allwinner A64, is during OHCI1 shutdown
> the controller is unable to access the register space
> so the Linux boot hangs at this place.

 This doesn't make any sense, Linux should enable the necessary clock
 before accessing any controller registers, unless there is a bug in 
 Linux.
>>>
>>> Should but doesn't always?  So yes, there's possibly / hopefully a bug
>>> in the dts files.
>>
>> How did you reach that conclusion about the DTS files ? There is a bug
>> in Linux, but it's likely in the driver which doesn't enable the clock
>> before accessing the registers.
>>
>> But maybe the description here is completely confusing, since the output
>> down below would indicate this hang is still in U-Boot.
>
> Yes, it is. There is no bug in Linux.
>
> U-Boot trips over its own feet when bringing down the USB controllers:
> It shutdowns one part (EHCI or OHCI) on the register level, then turns
> off the clocks and reset gates. But because they are shared between
> controllers, this turns off the other controller as well. Then it tries
> to bring down the the second part (OHCI or EHCI, respectively) on the
> USB register level, which hangs, because the AHB clock is already off.
> As this just happens quite late, it looks like U-Boot already said
> goodbye, but it actually hasn't completely finished.
> So Linux is completely fine and the bug is entirely in U-Boot.
> My patch [1] tries to paper^Wsolve this in a different way, though it
> isn't perfect either. I think there is a bit more to it than I assumed
> yesterday, so I need to go back to the code later tonight to see what's
> really going on (I suspect it's about OHCI 0 and 1 sharing a clock, not
> about EHCI and OHCI).

 Well, please keep poking.

 Maybe a dumb idea, but what about enabling the clock at the beginning of
 remove function to guarantee they are ON and then disabling the clock at
 the end of the function. Would that work maybe ? ie.

 .remove() {
 clk_enable(..);
 readl()/writel()/...
 clk_disable(..);
 }
>>>
>>> I've verified clock bits before disabling, and bits are enabled as
>>> usual. and even verified your idea of enabling before disable[2]
>>
>> Verified ... with what result ?
> 
> Same hang, with properly disabling clock during OHCI0 shutdown.

So the clock were enabled and yet there was a hang ? Why ?

I was under the impression that disabling clock was the problem, maybe
that is not the case ?

Are you sure you enabled all the clock ?

>>> => usb reset
>>> resetting USB...
>>> ohci_usb_remove: input mask = 0x1, input usb_clk_cfg = 0x30303
>>> ohci_usb_remove: usb_clk_cfg = 0x20303
>>> EHCI failed to shut down host controller.
> 
> << hang here >>
> 
>>>
>>> [2] https://paste.ubuntu.com/p/V9KKxMx6Cj/
>>
>> (no need to use pastebin to share 20 line patch, in fact it doesn't help
>> the ML archives at all)
> 
> Sorry, copying same here.
> 
> --- a/drivers/usb/host/ohci-sunxi.c
> +++ b/drivers/usb/host/ohci-sunxi.c
> @@ -128,10 +128,14 @@ static int ohci_usb_remove(struct udevice *dev)
> if (ret)
> return ret;
> 
> +   printf("%s: mask = 0x%x, usb_clk_cfg = 0x%x\n", __func__,
> +   priv->usb_gate_mask, readl(>ccm->usb_clk_cfg));
> +   setbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
> if (priv->cfg->has_reset)
> clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
> -   clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
> clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
> +   clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
> +   printf("%s: usb_clk_cfg = 0x%x\n", __func__,
> readl(>ccm->usb_clk_cfg));

-- 
Best regards,
Marek 

Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Jagan Teki
On Tue, Jul 3, 2018 at 11:26 PM, Marek Vasut  wrote:
> On 07/03/2018 07:09 PM, Jagan Teki wrote:
>> On Tue, Jul 3, 2018 at 10:14 PM, Marek Vasut  wrote:
>>> On 07/03/2018 05:22 PM, Andre Przywara wrote:
 Hi,

 On 02/07/18 22:57, Marek Vasut wrote:
> On 07/02/2018 11:40 PM, Tom Rini wrote:
>> On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
>>> On 07/02/2018 10:53 PM, Jagan Teki wrote:
 During usb shutdown or 'usb reset' all the necessary clocks
 on the specific controller will disable. Usually this shutdown
 happen during U-Boot proper handoff to Linux.
>>>
>>> No, 'usb reset' can be triggered by the user any time.
>>
>> Yes, and it's also triggered as part of the hand-off, which is the use
>> case in question.
>
> No, that's not true. The USB controllers are torn down when starting the
> OS, which is a different thing from usb reset, which brings them back up
> and rescans the bus too.
>
 There is an issue in Allwinner A64, is during OHCI1 shutdown
 the controller is unable to access the register space
 so the Linux boot hangs at this place.
>>>
>>> This doesn't make any sense, Linux should enable the necessary clock
>>> before accessing any controller registers, unless there is a bug in 
>>> Linux.
>>
>> Should but doesn't always?  So yes, there's possibly / hopefully a bug
>> in the dts files.
>
> How did you reach that conclusion about the DTS files ? There is a bug
> in Linux, but it's likely in the driver which doesn't enable the clock
> before accessing the registers.
>
> But maybe the description here is completely confusing, since the output
> down below would indicate this hang is still in U-Boot.

 Yes, it is. There is no bug in Linux.

 U-Boot trips over its own feet when bringing down the USB controllers:
 It shutdowns one part (EHCI or OHCI) on the register level, then turns
 off the clocks and reset gates. But because they are shared between
 controllers, this turns off the other controller as well. Then it tries
 to bring down the the second part (OHCI or EHCI, respectively) on the
 USB register level, which hangs, because the AHB clock is already off.
 As this just happens quite late, it looks like U-Boot already said
 goodbye, but it actually hasn't completely finished.
 So Linux is completely fine and the bug is entirely in U-Boot.
 My patch [1] tries to paper^Wsolve this in a different way, though it
 isn't perfect either. I think there is a bit more to it than I assumed
 yesterday, so I need to go back to the code later tonight to see what's
 really going on (I suspect it's about OHCI 0 and 1 sharing a clock, not
 about EHCI and OHCI).
>>>
>>> Well, please keep poking.
>>>
>>> Maybe a dumb idea, but what about enabling the clock at the beginning of
>>> remove function to guarantee they are ON and then disabling the clock at
>>> the end of the function. Would that work maybe ? ie.
>>>
>>> .remove() {
>>> clk_enable(..);
>>> readl()/writel()/...
>>> clk_disable(..);
>>> }
>>
>> I've verified clock bits before disabling, and bits are enabled as
>> usual. and even verified your idea of enabling before disable[2]
>
> Verified ... with what result ?

Same hang, with properly disabling clock during OHCI0 shutdown.

>
>> => usb reset
>> resetting USB...
>> ohci_usb_remove: input mask = 0x1, input usb_clk_cfg = 0x30303
>> ohci_usb_remove: usb_clk_cfg = 0x20303
>> EHCI failed to shut down host controller.

<< hang here >>

>>
>> [2] https://paste.ubuntu.com/p/V9KKxMx6Cj/
>
> (no need to use pastebin to share 20 line patch, in fact it doesn't help
> the ML archives at all)

Sorry, copying same here.

--- a/drivers/usb/host/ohci-sunxi.c
+++ b/drivers/usb/host/ohci-sunxi.c
@@ -128,10 +128,14 @@ static int ohci_usb_remove(struct udevice *dev)
if (ret)
return ret;

+   printf("%s: mask = 0x%x, usb_clk_cfg = 0x%x\n", __func__,
+   priv->usb_gate_mask, readl(>ccm->usb_clk_cfg));
+   setbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
if (priv->cfg->has_reset)
clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
-   clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
+   clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
+   printf("%s: usb_clk_cfg = 0x%x\n", __func__,
readl(>ccm->usb_clk_cfg));
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Marek Vasut
On 07/03/2018 07:09 PM, Jagan Teki wrote:
> On Tue, Jul 3, 2018 at 10:14 PM, Marek Vasut  wrote:
>> On 07/03/2018 05:22 PM, Andre Przywara wrote:
>>> Hi,
>>>
>>> On 02/07/18 22:57, Marek Vasut wrote:
 On 07/02/2018 11:40 PM, Tom Rini wrote:
> On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
>> On 07/02/2018 10:53 PM, Jagan Teki wrote:
>>> During usb shutdown or 'usb reset' all the necessary clocks
>>> on the specific controller will disable. Usually this shutdown
>>> happen during U-Boot proper handoff to Linux.
>>
>> No, 'usb reset' can be triggered by the user any time.
>
> Yes, and it's also triggered as part of the hand-off, which is the use
> case in question.

 No, that's not true. The USB controllers are torn down when starting the
 OS, which is a different thing from usb reset, which brings them back up
 and rescans the bus too.

>>> There is an issue in Allwinner A64, is during OHCI1 shutdown
>>> the controller is unable to access the register space
>>> so the Linux boot hangs at this place.
>>
>> This doesn't make any sense, Linux should enable the necessary clock
>> before accessing any controller registers, unless there is a bug in 
>> Linux.
>
> Should but doesn't always?  So yes, there's possibly / hopefully a bug
> in the dts files.

 How did you reach that conclusion about the DTS files ? There is a bug
 in Linux, but it's likely in the driver which doesn't enable the clock
 before accessing the registers.

 But maybe the description here is completely confusing, since the output
 down below would indicate this hang is still in U-Boot.
>>>
>>> Yes, it is. There is no bug in Linux.
>>>
>>> U-Boot trips over its own feet when bringing down the USB controllers:
>>> It shutdowns one part (EHCI or OHCI) on the register level, then turns
>>> off the clocks and reset gates. But because they are shared between
>>> controllers, this turns off the other controller as well. Then it tries
>>> to bring down the the second part (OHCI or EHCI, respectively) on the
>>> USB register level, which hangs, because the AHB clock is already off.
>>> As this just happens quite late, it looks like U-Boot already said
>>> goodbye, but it actually hasn't completely finished.
>>> So Linux is completely fine and the bug is entirely in U-Boot.
>>> My patch [1] tries to paper^Wsolve this in a different way, though it
>>> isn't perfect either. I think there is a bit more to it than I assumed
>>> yesterday, so I need to go back to the code later tonight to see what's
>>> really going on (I suspect it's about OHCI 0 and 1 sharing a clock, not
>>> about EHCI and OHCI).
>>
>> Well, please keep poking.
>>
>> Maybe a dumb idea, but what about enabling the clock at the beginning of
>> remove function to guarantee they are ON and then disabling the clock at
>> the end of the function. Would that work maybe ? ie.
>>
>> .remove() {
>> clk_enable(..);
>> readl()/writel()/...
>> clk_disable(..);
>> }
> 
> I've verified clock bits before disabling, and bits are enabled as
> usual. and even verified your idea of enabling before disable[2]

Verified ... with what result ?

> => usb reset
> resetting USB...
> ohci_usb_remove: input mask = 0x1, input usb_clk_cfg = 0x30303
> ohci_usb_remove: usb_clk_cfg = 0x20303
> EHCI failed to shut down host controller.
> 
> [2] https://paste.ubuntu.com/p/V9KKxMx6Cj/

(no need to use pastebin to share 20 line patch, in fact it doesn't help
the ML archives at all)

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Jagan Teki
On Tue, Jul 3, 2018 at 10:14 PM, Marek Vasut  wrote:
> On 07/03/2018 05:22 PM, Andre Przywara wrote:
>> Hi,
>>
>> On 02/07/18 22:57, Marek Vasut wrote:
>>> On 07/02/2018 11:40 PM, Tom Rini wrote:
 On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
> On 07/02/2018 10:53 PM, Jagan Teki wrote:
>> During usb shutdown or 'usb reset' all the necessary clocks
>> on the specific controller will disable. Usually this shutdown
>> happen during U-Boot proper handoff to Linux.
>
> No, 'usb reset' can be triggered by the user any time.

 Yes, and it's also triggered as part of the hand-off, which is the use
 case in question.
>>>
>>> No, that's not true. The USB controllers are torn down when starting the
>>> OS, which is a different thing from usb reset, which brings them back up
>>> and rescans the bus too.
>>>
>> There is an issue in Allwinner A64, is during OHCI1 shutdown
>> the controller is unable to access the register space
>> so the Linux boot hangs at this place.
>
> This doesn't make any sense, Linux should enable the necessary clock
> before accessing any controller registers, unless there is a bug in Linux.

 Should but doesn't always?  So yes, there's possibly / hopefully a bug
 in the dts files.
>>>
>>> How did you reach that conclusion about the DTS files ? There is a bug
>>> in Linux, but it's likely in the driver which doesn't enable the clock
>>> before accessing the registers.
>>>
>>> But maybe the description here is completely confusing, since the output
>>> down below would indicate this hang is still in U-Boot.
>>
>> Yes, it is. There is no bug in Linux.
>>
>> U-Boot trips over its own feet when bringing down the USB controllers:
>> It shutdowns one part (EHCI or OHCI) on the register level, then turns
>> off the clocks and reset gates. But because they are shared between
>> controllers, this turns off the other controller as well. Then it tries
>> to bring down the the second part (OHCI or EHCI, respectively) on the
>> USB register level, which hangs, because the AHB clock is already off.
>> As this just happens quite late, it looks like U-Boot already said
>> goodbye, but it actually hasn't completely finished.
>> So Linux is completely fine and the bug is entirely in U-Boot.
>> My patch [1] tries to paper^Wsolve this in a different way, though it
>> isn't perfect either. I think there is a bit more to it than I assumed
>> yesterday, so I need to go back to the code later tonight to see what's
>> really going on (I suspect it's about OHCI 0 and 1 sharing a clock, not
>> about EHCI and OHCI).
>
> Well, please keep poking.
>
> Maybe a dumb idea, but what about enabling the clock at the beginning of
> remove function to guarantee they are ON and then disabling the clock at
> the end of the function. Would that work maybe ? ie.
>
> .remove() {
> clk_enable(..);
> readl()/writel()/...
> clk_disable(..);
> }

I've verified clock bits before disabling, and bits are enabled as
usual. and even verified your idea of enabling before disable[2]

=> usb reset
resetting USB...
ohci_usb_remove: input mask = 0x1, input usb_clk_cfg = 0x30303
ohci_usb_remove: usb_clk_cfg = 0x20303
EHCI failed to shut down host controller.

[2] https://paste.ubuntu.com/p/V9KKxMx6Cj/
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

2018-07-03 Thread Marek Vasut
On 07/03/2018 05:10 PM, See, Chin Liang wrote:
> On Sat, 2018-06-23 at 06:23 +0200, Marek Vasut wrote:
>> On 06/23/2018 05:55 AM, Marek Vasut wrote:
>>>
>>> On 06/06/2018 08:47 PM, Marek Vasut wrote:

 On 05/31/2018 12:00 PM, Marek Vasut wrote:
>
> On 05/31/2018 11:52 AM, See, Chin Liang wrote:
>>
>> On Tue, 2018-05-29 at 18:39 +0200, Marek Vasut wrote:
>>>
>>> Make sure the ARM ACTLR register has correct configuration,
>>> otherwise
>>> the Linux kernel refuses to boot. In particular, the "Write
>>> Full Line
>>> of Zeroes" bit must be cleared.
>>>
>>> Signed-off-by: Marek Vasut 
>>> Cc: Chin Liang See 
>>> Cc: Dinh Nguyen 
>>> ---
>>> NOTE: This gem was well hidden in the Altera U-Boot fork
>>> and is
>>> really needed.
>>>   What is not entirely clear to me is WHY ? So why is
>>> this needed
>>> ?
>> I vaguely recall it's related to HW constrain. And check back
>> the
>> downstream U-Boot, actually we need to set the bit instead
>> clearing
>> it. https://github.com/altera-opensource/u-boot-socfpga/blob/
>> socfpga_v2
>> 014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/pl3
>> 10.c
> You are clearing it here:
> https://github.com/altera-opensource/u-boot-socfpga/blob/socfpg
> a_v2014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/l
> owlevel_init.S#L35
>
> The PL310 configuration is a different register. What HW
> constraint ?
> I'd really like to understand this problem.
 Bump ? I had a report the register even has to be set to 0 ,
 otherwise
 there are ethernet problems. What is going on with this register
 ?
>>> Bump again ?
>>>
>> Expanding the CC list. I am tempted to stop accepting any socfpga
>> patches until this is resolved.
>>
> 
> Just realize this is not addressed as Ley Foon talked to me

I am real grateful for that.

> We checked with HW team that the full line of zeroes is a special setup
> of signals between A9 and L2 cache controller. As they recalled that
> this signal is not 100% AXI compliance, it may hang the system if L2
> cache is disabled while full line of zeroes is set.
> 
> In A10, one thing new is we enabled L2 cache in BootROM and this bit is
> set. When entering the U-Boot, we are ensuring all cache are disabled.
> At this point, we need to ensure full line of zeroes bit is cleared.
> Only when U-Boot renable the cache including L2, this bit is set again.
> 
> Hope this explains.

I'll have to give it some thought, but I think so.

Is there an errata or some piece of documentation I can refer to about
this behavior ? I'd like to add link to the commit message.

Thanks, really appreciated!

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] efi_loader: runtime services implementation broken?

2018-07-03 Thread Bin Meng
Hi Alex,

On Tue, Jul 3, 2018 at 11:05 PM, Alexander Graf  wrote:
> On 07/03/2018 04:56 PM, Bin Meng wrote:
>>
>> Hi Alex,
>>
>> On Tue, Jul 3, 2018 at 10:32 PM, Alexander Graf  wrote:
>>>
>>> On 07/03/2018 04:24 PM, Bin Meng wrote:

 Hi Alex, Heinrich,

 At present not all interfaces in lib/efi_loader/efi_runtime.c are
 declared as __efi_runtime. But only declaring those as __efi_runtime
 is not enough. The data these interfaces use should be declared as
 __efi_runtime_data too. More worse, any U-Boot APIs called by these
 interfaces should be __efi_runtime and __efi_runtime_data
 respectively.
>>>
>>>
>>> Correct. This is done for everything right now, no?
>>
>> For example, efi_set_virtual_address_map() is not declared as
>> __efi_runtime.
>
>
> Is that called post exit boot services on x86?

Yes, if you look at efi_runtime_services structure in
lib/efi_loader/efi_runtime.c, you can see
efi_set_virtual_address_map() is hooked up there, and all interfaces
in efi_runtime_services are supposed to be called by OS as their names
indicate, runtime not bootime.

>
>>
 Eventually we need mark the RAM where the whole U-Boot image lives as
 runtime service code and data and end up leaving whole U-Boot image in
 the RAM after kernel boots?
>>>
>>>
>>> Why?
>>>
>> Unless we track down every APIs called by runtime services and mark
>> them correctly as __efi_runtime code/data.
>
>
> That's the idea, yes. The vector should be quite small as long as we don't
> actually implement / reuse drivers.

However this currently seems not to be the case ..

>
>>
 Right now I am testing booting Linux on Intel Galileo with 'bootefi'
 and kernel just hangs during the boot. Initial debugging shows that
 kernel crashes when calling runtime service
 efi_set_virtual_address_map().
>>>
>>>
>>> Can you please try with my efi-next branch? I added a few patches that
>>> allow
>>> gcc to put implicit data and code into the runtime section. Without
>>> those,
>>> you may get constant propagation into a the common .rodata segment for
>>> example.
>>>
>> Still the same.
>
>
> Ok, maybe the x86 efi stub does things different from the ARM one then.
>

maybe :(

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Marek Vasut
On 07/03/2018 06:25 PM, Jagan Teki wrote:
> On Tue, Jul 3, 2018 at 8:52 PM, Andre Przywara  wrote:
>> Hi,
>>
>> On 02/07/18 22:57, Marek Vasut wrote:
>>> On 07/02/2018 11:40 PM, Tom Rini wrote:
 On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
> On 07/02/2018 10:53 PM, Jagan Teki wrote:
>> During usb shutdown or 'usb reset' all the necessary clocks
>> on the specific controller will disable. Usually this shutdown
>> happen during U-Boot proper handoff to Linux.
>
> No, 'usb reset' can be triggered by the user any time.

 Yes, and it's also triggered as part of the hand-off, which is the use
 case in question.
>>>
>>> No, that's not true. The USB controllers are torn down when starting the
>>> OS, which is a different thing from usb reset, which brings them back up
>>> and rescans the bus too.
>>>
>> There is an issue in Allwinner A64, is during OHCI1 shutdown
>> the controller is unable to access the register space
>> so the Linux boot hangs at this place.
>
> This doesn't make any sense, Linux should enable the necessary clock
> before accessing any controller registers, unless there is a bug in Linux.

 Should but doesn't always?  So yes, there's possibly / hopefully a bug
 in the dts files.
>>>
>>> How did you reach that conclusion about the DTS files ? There is a bug
>>> in Linux, but it's likely in the driver which doesn't enable the clock
>>> before accessing the registers.
>>>
>>> But maybe the description here is completely confusing, since the output
>>> down below would indicate this hang is still in U-Boot.
>>
>> Yes, it is. There is no bug in Linux.
>>
>> U-Boot trips over its own feet when bringing down the USB controllers:
>> It shutdowns one part (EHCI or OHCI) on the register level, then turns
>> off the clocks and reset gates. But because they are shared between
>> controllers, this turns off the other controller as well. Then it tries
>> to bring down the the second part (OHCI or EHCI, respectively) on the
>> USB register level, which hangs, because the AHB clock is already off.
>> As this just happens quite late, it looks like U-Boot already said
>> goodbye, but it actually hasn't completely finished.
>> So Linux is completely fine and the bug is entirely in U-Boot.
>> My patch [1] tries to paper^Wsolve this in a different way, though it
>> isn't perfect either. I think there is a bit more to it than I assumed
>> yesterday, so I need to go back to the code later tonight to see what's
>> really going on (I suspect it's about OHCI 0 and 1 sharing a clock, not
>> about EHCI and OHCI).
>>
>> Cheers,
>> Andre.
>>
>> [1] https://lists.denx.de/pipermail/u-boot/2018-July/333533.html
> 
> Do we really need turn-off ahb and reset gates? these are gracefully
> disabled during shutdown.

Given that this SoC is mostly operated from battery OR in constrained
environments where useless high thermal dissipation might cause trouble,
yes, turning off as much as possible is desired.

Besides, it's a good practice to keep things in order.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Marek Vasut
On 07/03/2018 05:22 PM, Andre Przywara wrote:
> Hi,
> 
> On 02/07/18 22:57, Marek Vasut wrote:
>> On 07/02/2018 11:40 PM, Tom Rini wrote:
>>> On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
 On 07/02/2018 10:53 PM, Jagan Teki wrote:
> During usb shutdown or 'usb reset' all the necessary clocks
> on the specific controller will disable. Usually this shutdown
> happen during U-Boot proper handoff to Linux.

 No, 'usb reset' can be triggered by the user any time.
>>>
>>> Yes, and it's also triggered as part of the hand-off, which is the use
>>> case in question.
>>
>> No, that's not true. The USB controllers are torn down when starting the
>> OS, which is a different thing from usb reset, which brings them back up
>> and rescans the bus too.
>>
> There is an issue in Allwinner A64, is during OHCI1 shutdown
> the controller is unable to access the register space
> so the Linux boot hangs at this place.

 This doesn't make any sense, Linux should enable the necessary clock
 before accessing any controller registers, unless there is a bug in Linux.
>>>
>>> Should but doesn't always?  So yes, there's possibly / hopefully a bug
>>> in the dts files.
>>
>> How did you reach that conclusion about the DTS files ? There is a bug
>> in Linux, but it's likely in the driver which doesn't enable the clock
>> before accessing the registers.
>>
>> But maybe the description here is completely confusing, since the output
>> down below would indicate this hang is still in U-Boot.
> 
> Yes, it is. There is no bug in Linux.
> 
> U-Boot trips over its own feet when bringing down the USB controllers:
> It shutdowns one part (EHCI or OHCI) on the register level, then turns
> off the clocks and reset gates. But because they are shared between
> controllers, this turns off the other controller as well. Then it tries
> to bring down the the second part (OHCI or EHCI, respectively) on the
> USB register level, which hangs, because the AHB clock is already off.
> As this just happens quite late, it looks like U-Boot already said
> goodbye, but it actually hasn't completely finished.
> So Linux is completely fine and the bug is entirely in U-Boot.
> My patch [1] tries to paper^Wsolve this in a different way, though it
> isn't perfect either. I think there is a bit more to it than I assumed
> yesterday, so I need to go back to the code later tonight to see what's
> really going on (I suspect it's about OHCI 0 and 1 sharing a clock, not
> about EHCI and OHCI).

Well, please keep poking.

Maybe a dumb idea, but what about enabling the clock at the beginning of
remove function to guarantee they are ON and then disabling the clock at
the end of the function. Would that work maybe ? ie.

.remove() {
clk_enable(..);
readl()/writel()/...
clk_disable(..);
}

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] Pull request: u-boot-sunxi/master

2018-07-03 Thread Jagan Teki
Hi Tom,

Please pull this PR.

thanks,
Jagan.

The following changes since commit f58e779513be36e30ce46838fb467e12ac6a5539:

  Merge tag 'arc-updates-for-2018.07-rc2' of git://git.denx.de/u-boot-arc 
(2018-06-15 09:38:16 -0400)

are available in the Git repository at:

  git://git.denx.de/u-boot-sunxi.git master

for you to fetch changes up to be0d21795b2bd3ed071de9bb0c66d8cc80d9:

  arm: timer: sunxi: add Allwinner timer erratum workaround (2018-07-03 
22:00:00 +0530)


Andre Przywara (2):
  arm: timer: factor out FSL arch timer erratum workaround
  arm: timer: sunxi: add Allwinner timer erratum workaround

Hauke Mehrtens (2):
  sun8i: h2: Add initial Orange Pi R1 support
  sun50i: h5: Add initial Orange Pi Zero Plus support

 arch/arm/cpu/armv8/generic_timer.c|  55 --
 arch/arm/dts/Makefile |   2 +
 arch/arm/dts/sun50i-h5-orangepi-zero-plus.dts | 145 ++
 arch/arm/dts/sun8i-h2-plus-orangepi-r1.dts| 101 ++
 arch/arm/mach-sunxi/Kconfig   |   4 +
 board/sunxi/MAINTAINERS   |  10 ++
 configs/orangepi_r1_defconfig |  16 +++
 configs/orangepi_zero_plus_defconfig  |  16 +++
 8 files changed, 343 insertions(+), 6 deletions(-)
 create mode 100644 arch/arm/dts/sun50i-h5-orangepi-zero-plus.dts
 create mode 100644 arch/arm/dts/sun8i-h2-plus-orangepi-r1.dts
 create mode 100644 configs/orangepi_r1_defconfig
 create mode 100644 configs/orangepi_zero_plus_defconfig
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/2] arm: timer: factor out FSL arch timer erratum workaround

2018-07-03 Thread Jagan Teki
On Tue, Jul 3, 2018 at 5:20 PM, Guillaume Gardet
 wrote:
>
>
> Le 03/07/2018 à 01:08, Andreas Färber a écrit :
>>
>> Am 02.07.2018 um 10:01 schrieb Jagan Teki:
>>>
>>> On Wed, Jun 27, 2018 at 6:12 AM, Andre Przywara 
>>> wrote:

 At the moment we have the workaround for the Freescale arch timer
 erratum A-008585 merged into the generic timer_read_counter() routine.
 Split those two up, so that we can add other errata workaround more
 easily. Also add an explaining comment on the way.

 Signed-off-by: Andre Przywara 
 ---
>>>
>>> Applied both patches, to u-boot-sunxi/master
>>
>> Tested both on top of v2018.07-rc2, fixes the boot for me.
>
>
> Tested both on top of v2018.07-rc3, and it fixes the boot.

Thanks, I've collected Tested-by
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Jagan Teki
On Tue, Jul 3, 2018 at 8:52 PM, Andre Przywara  wrote:
> Hi,
>
> On 02/07/18 22:57, Marek Vasut wrote:
>> On 07/02/2018 11:40 PM, Tom Rini wrote:
>>> On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
 On 07/02/2018 10:53 PM, Jagan Teki wrote:
> During usb shutdown or 'usb reset' all the necessary clocks
> on the specific controller will disable. Usually this shutdown
> happen during U-Boot proper handoff to Linux.

 No, 'usb reset' can be triggered by the user any time.
>>>
>>> Yes, and it's also triggered as part of the hand-off, which is the use
>>> case in question.
>>
>> No, that's not true. The USB controllers are torn down when starting the
>> OS, which is a different thing from usb reset, which brings them back up
>> and rescans the bus too.
>>
> There is an issue in Allwinner A64, is during OHCI1 shutdown
> the controller is unable to access the register space
> so the Linux boot hangs at this place.

 This doesn't make any sense, Linux should enable the necessary clock
 before accessing any controller registers, unless there is a bug in Linux.
>>>
>>> Should but doesn't always?  So yes, there's possibly / hopefully a bug
>>> in the dts files.
>>
>> How did you reach that conclusion about the DTS files ? There is a bug
>> in Linux, but it's likely in the driver which doesn't enable the clock
>> before accessing the registers.
>>
>> But maybe the description here is completely confusing, since the output
>> down below would indicate this hang is still in U-Boot.
>
> Yes, it is. There is no bug in Linux.
>
> U-Boot trips over its own feet when bringing down the USB controllers:
> It shutdowns one part (EHCI or OHCI) on the register level, then turns
> off the clocks and reset gates. But because they are shared between
> controllers, this turns off the other controller as well. Then it tries
> to bring down the the second part (OHCI or EHCI, respectively) on the
> USB register level, which hangs, because the AHB clock is already off.
> As this just happens quite late, it looks like U-Boot already said
> goodbye, but it actually hasn't completely finished.
> So Linux is completely fine and the bug is entirely in U-Boot.
> My patch [1] tries to paper^Wsolve this in a different way, though it
> isn't perfect either. I think there is a bit more to it than I assumed
> yesterday, so I need to go back to the code later tonight to see what's
> really going on (I suspect it's about OHCI 0 and 1 sharing a clock, not
> about EHCI and OHCI).
>
> Cheers,
> Andre.
>
> [1] https://lists.denx.de/pipermail/u-boot/2018-July/333533.html

Do we really need turn-off ahb and reset gates? these are gracefully
disabled during shutdown.

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


[U-Boot] tools: kwboot: properly quit when read() returns 0

2018-07-03 Thread Tom Rini
From: Willy Tarreau 

When kwboot is attached to a terminal which disappears such as one
connected via an unplugged USB cable, read() returns 0, making kwboot
loop until a key is pressed in the terminal. The only case where read()
may return 0 here is when the terminal is closed anyway, so let's
properly handle this one and report is similar to other errors.

Signed-off-by: Willy Tarreau 
---
 tools/kwboot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 0a77060..50ae2b4 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -182,7 +182,7 @@ kwboot_tty_recv(int fd, void *buf, size_t len, int timeo)
}
 
n = read(fd, buf, len);
-   if (n < 0)
+   if (n <= 0)
goto out;
 
buf = (char *)buf + n;
@@ -466,7 +466,7 @@ kwboot_term_pipe(int in, int out, char *quit, int *s)
char _buf[128], *buf = _buf;
 
nin = read(in, buf, sizeof(buf));
-   if (nin < 0)
+   if (nin <= 0)
return -1;
 
if (quit) {
-- 
1.7.12.1

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


[U-Boot] tools: kwboot: unbreak terminal-only mode

2018-07-03 Thread Tom Rini
From: Willy Tarreau 

Commit 84899e2 ("tools/kwboot: Sync with latest barebox version to
support Armada XP") accidently broke the terminal-only mode (-t) by
removing the test on the bootmsg. Thus even when trying to use kwboot
as a plain terminal, it asks to reboot the target.

This commit simply reintroduces the lost test so that it is possible
again to use kwboot to attach to the target system's console.

Signed-off-by: Willy Tarreau 
---
 tools/kwboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/kwboot.c b/tools/kwboot.c
index 8a421cd..0a77060 100644
--- a/tools/kwboot.c
+++ b/tools/kwboot.c
@@ -821,7 +821,7 @@ main(int argc, char **argv)
perror("debugmsg");
goto out;
}
-   } else {
+   } else if (bootmsg) {
rc = kwboot_bootmsg(tty, bootmsg);
if (rc) {
perror("bootmsg");
-- 
1.7.12.1

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


Re: [U-Boot] CONFIG_BOOTDELAY and env_default.h

2018-07-03 Thread Tom Rini
On Mon, Jul 02, 2018 at 11:00:05AM +0100, Alex Kiernan wrote:
> On Mon, Jul 2, 2018 at 3:25 AM Tom Rini  wrote:
> >
> > On Fri, Jun 29, 2018 at 09:19:34PM -0700, Simon Glass wrote:
> > > +Tom
> > >
> > > Hi Alex,
> > >
> > > On 29 June 2018 at 02:31, Alex Kiernan  wrote:
> > > >
> > > > I've just been digging into a problem where I've got both
> > > > CONFIG_ENV_IS_NOWHERE set and CONFIG_BOOTDELAY set to -2 and it turns
> > > > out in env_default.h we have:
> > > >
> > > > #if defined(CONFIG_BOOTDELAY) && (CONFIG_BOOTDELAY >= 0)
> > > > "bootdelay="__stringify(CONFIG_BOOTDELAY)   "\0"
> > > > #endifreason
> > > >
> > > > So the -ve values never make it into the default environment, which
> > > > means I don't have it at all when CONFIG_ENV_IS_NOWHERE.
> > > >
> > > > The (CONFIG_BOOTDELAY >= 0) seems to have been there forever
> > > > (c609719b8d1b2dca590e0ed499016d041203e403, Sun Nov 3 00:24:07 2002
> > > > + is as far back as I've gone), but we've then changed the
> > > > behaviours of the bootdelay values in (the commit I was looking at was
> > > > 2fbb8462b0e18893b4b739705db047ffda82d4fc from Mon Jun 27 16:23:01 2016
> > > > +0900, but I'm not sure that's really the right one)
> > > >
> > > > I think we should change the code to a simple #if 
> > > > defined(CONFIG_BOOTDELAY)
> > > >
> > > > ?
> > >
> > > I don't know what the check was supposed to do and the comment on the
> > > 'bootdelay' env variable just says 'see CONFIG_BOOTDELAY'. Your
> > > solution sounds reasonable to me but perhaps Tom or Wolfgang have more
> > > insight.
> >
> > It seems like a historical oversight.  But.. what happens before and
> > after when you have a negative bootdelay value in the default
> > environment?
> >
> 
> It's a bit convoluted...
> 
> There's bootmenu, where (CONFIG_BOOTDELAY >= 0) supplies a compile
> time default, which can be overridden by options to the command, or
> the bootmenu_delay environment variable. This never looks at the
> bootdelay environment variable, so I think we can ignore this one.
> 
> Then there's autoboot, which reads the environment for bootdelay, but
> if that's not set then uses CONFIG_BOOTDELAY as a default.
> 
> So whilst you end up with the default in all cases, it's only stored
> in the environment if it's >= 0.
> 
> Looking at the only place bootdelay is read from the environment (in
> bootdelay_process):
> 
> s = env_get("bootdelay");
> bootdelay = s ? (int)simple_strtol(s, NULL, 10) : CONFIG_BOOTDELAY;
> 
> So if you have a compiled in -ve CONFIG_BOOTDELAY, there won't be a
> value for bootdelay in the default environment and we'll use the
> compile time default. Swap to storing all values of CONFIG_BOOTDELAY
> and you'll fetch it from the environment, rather than using the
> compile time default.
> 
> So long as we don't remove the CONFIG_BOOTDELAY in this ternary
> expression, I think you end up with the same behaviour in every
> circumstance.
> 
> The reason I tripped over it was some scripting in CONFIG_PREBOOT
> which was common between two configurations, but was expecting to
> differentiate between them based on bootdelay (which broke when it
> turned out to not be set).

OK.  Please put out a patch and I'll pick it up after the release,
thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [upstream-release] [PATCH v2 5/7] armv8: ls1046a: initial icid setup support

2018-07-03 Thread Laurentiu Tudor
Hi Bharat,

Thanks for the review! Comments inline.

On 03.07.2018 17:09, Bharat Bhushan wrote:
> 
> 
>> -Original Message-
>> From: upstream-release-boun...@linux.freescale.net [mailto:upstream-
>> release-boun...@linux.freescale.net] On Behalf Of Laurentiu Tudor
>> Sent: Tuesday, July 3, 2018 5:42 PM
>> To: York Sun ; Prabhakar Kushwaha
>> ; u-boot@lists.denx.de
>> Cc: Laurentiu Tudor 
>> Subject: [upstream-release] [PATCH v2 5/7] armv8: ls1046a: initial icid setup
>> support
>>
>> Add infrastructure for ICID setup and device tree
>> fixup on ARM platforms. This include basic ICID setup
>> for several devices.
>>
>> Signed-off-by: Laurentiu Tudor 
>> ---
>>   arch/arm/cpu/armv8/fsl-layerscape/Makefile|   1 +
>>   arch/arm/cpu/armv8/fsl-layerscape/icid.c  | 111 ++
>>   .../arm/cpu/armv8/fsl-layerscape/ls1046_ids.c |  29 +
>>   arch/arm/cpu/armv8/fsl-layerscape/soc.c   |   3 +
>>   .../asm/arch-fsl-layerscape/fsl_icid.h|  80 +
>>   board/freescale/ls1046aqds/ls1046aqds.c   |   2 +
>>   board/freescale/ls1046ardb/ls1046ardb.c   |   3 +
>>   7 files changed, 229 insertions(+)
>>   create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/icid.c
>>   create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
>>   create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
>>
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
>> b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
>> index 1e9e4680fe..5d6f68aad6 100644
>> --- a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
>> @@ -37,6 +37,7 @@ endif
>>
>>   ifneq ($(CONFIG_ARCH_LS1046A),)
>>   obj-$(CONFIG_SYS_HAS_SERDES) += ls1046a_serdes.o
>> +obj-y += icid.o ls1046_ids.o
>>   endif
>>
>>   ifneq ($(CONFIG_ARCH_LS1088A),)
>> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/icid.c
>> b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
>> new file mode 100644
>> index 00..8694bd6fa1
>> --- /dev/null
>> +++ b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
>> @@ -0,0 +1,111 @@
>> +// SPDX-License-Identifier: GPL-2.0+
>> +/*
>> + * Copyright 2018 NXP
>> + */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +static void set_icid(struct icid_id_table *tbl, int size)
>> +{
>> +int i;
>> +
>> +for (i = 0; i < size; i++)
>> +out_be32((u32 *)(tbl[i].reg_addr), tbl[i].reg);
>> +}
>> +
>> +void set_icids(void)
>> +{
>> +/* setup general icid offsets */
>> +set_icid(icid_tbl, icid_tbl_sz);
> 
> Icid_tbl[] is currently defined for ls1046 but this code is generic and later 
> can be used for ls1043 later.
> We can let caller provide table pointer and size.

Right, but "icid_tbl" is defined in the SoC specific ls1046_ids.c file. 
I think that if we add the corresponding ls1043_ids.c this code can be 
reused as is.

>> +}
>> +
>> +int fdt_set_iommu_prop(void *blob, int off, int smmu_ph, u32 *ids, int
>> num_ids)
>> +{
>> +int i, ret;
>> +u32 prop[8];
>> +
>> +for (i = 0; i < num_ids; i++) {
>> +prop[i * 2] = cpu_to_fdt32(smmu_ph);
>> +prop[i * 2 + 1] = cpu_to_fdt32(ids[i]);
>> +}
>> +ret = fdt_setprop(blob, off, "iommus",
>> +  prop, sizeof(u32) * num_ids * 2);
>> +if (ret > 0) {
>> +printf("WARNING unable to set iommus: %s\n",
>> fdt_strerror(off));
>> +return off;
>> +}
>> +ret = fdt_setprop_empty(blob, off, "dma-coherent");
>> +if (ret > 0) {
>> +printf("WARNING unable to set dma-coherent: %s\n",
>> +   fdt_strerror(off));
>> +return off;
>> +}
>> +
>> +return 0;
>> +}
>> +
>> +int fdt_fixup_icid_tbl(void *blob, int smmu_ph,
>> +   struct icid_id_table *tbl, int size)
> 
> What we set in device tree is stream-id, while stream-id is = [PL: BMT : 
> ICID];

Also TBU_ID. As far as i know, PL, BMT bits are not implemented on this 
platform.

>   I think we should use stream-id to the point when we are patching 
> device-tree and value set in h/w.

Not sure i understand this statement? Are you commenting that i should 
have used a different naming scheme, that is stream_id instead of icid?

> We can say PL = BMT = 0 for now and leave space for setting this in future 
> for any specific device for any platform?

I think so. As i mentioned, PL, BMT bits are not implemented on these chips.

>> +{
>> +int i, err, off;
>> +
>> +for (i = 0; i < size; i++) {
>> +if (!tbl[i].compat)
>> +continue;
>> +
>> +off = fdt_node_offset_by_compat_reg(blob,
>> +tbl[i].compat,
>> +tbl[i].compat_addr);
>> +if (off > 0) {
>> +err = fdt_set_iommu_prop(blob, off, smmu_ph,
>> + [i].id, 1);
>> +if 

Re: [U-Boot] [PATCH 1/2 v2] board: freescale: ls1012afrx:Common files to support

2018-07-03 Thread York Sun
On 06/06/2018 04:16 AM, Pramod Kumar wrote:
> LS1012A-FRDM and LS1012A-FRWY board.
> 
> 1-Move all common files applicable for both boards LS1012A-FRDM
>   and LS1012A-FRWY into board directory ls1012afrx.
> 2-Restructure LS1012A-FRDM code. Only board specific files are
>   in LS1012A-FRDM board directory.
> 
> Signed-off-by: Pramod Kumar 
> ---
> Depends on:
> http://patchwork.ozlabs.org/patch/918935/
> http://patchwork.ozlabs.org/patch/918933/
> http://patchwork.ozlabs.org/patch/918932/
> 
> Changes for v2:
>  - Rebased patch to above dependency patches 
> 

Previous patch 918933 added FRWY board into FRDM Kconfig. If you want to
separate them, send a patch to do that. Don't mix with adding a new board.

York

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


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Andre Przywara
Hi,

On 02/07/18 22:57, Marek Vasut wrote:
> On 07/02/2018 11:40 PM, Tom Rini wrote:
>> On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
>>> On 07/02/2018 10:53 PM, Jagan Teki wrote:
 During usb shutdown or 'usb reset' all the necessary clocks
 on the specific controller will disable. Usually this shutdown
 happen during U-Boot proper handoff to Linux.
>>>
>>> No, 'usb reset' can be triggered by the user any time.
>>
>> Yes, and it's also triggered as part of the hand-off, which is the use
>> case in question.
> 
> No, that's not true. The USB controllers are torn down when starting the
> OS, which is a different thing from usb reset, which brings them back up
> and rescans the bus too.
> 
 There is an issue in Allwinner A64, is during OHCI1 shutdown
 the controller is unable to access the register space
 so the Linux boot hangs at this place.
>>>
>>> This doesn't make any sense, Linux should enable the necessary clock
>>> before accessing any controller registers, unless there is a bug in Linux.
>>
>> Should but doesn't always?  So yes, there's possibly / hopefully a bug
>> in the dts files.
> 
> How did you reach that conclusion about the DTS files ? There is a bug
> in Linux, but it's likely in the driver which doesn't enable the clock
> before accessing the registers.
> 
> But maybe the description here is completely confusing, since the output
> down below would indicate this hang is still in U-Boot.

Yes, it is. There is no bug in Linux.

U-Boot trips over its own feet when bringing down the USB controllers:
It shutdowns one part (EHCI or OHCI) on the register level, then turns
off the clocks and reset gates. But because they are shared between
controllers, this turns off the other controller as well. Then it tries
to bring down the the second part (OHCI or EHCI, respectively) on the
USB register level, which hangs, because the AHB clock is already off.
As this just happens quite late, it looks like U-Boot already said
goodbye, but it actually hasn't completely finished.
So Linux is completely fine and the bug is entirely in U-Boot.
My patch [1] tries to paper^Wsolve this in a different way, though it
isn't perfect either. I think there is a bit more to it than I assumed
yesterday, so I need to go back to the code later tonight to see what's
really going on (I suspect it's about OHCI 0 and 1 sharing a clock, not
about EHCI and OHCI).

Cheers,
Andre.

[1] https://lists.denx.de/pipermail/u-boot/2018-July/333533.html

 This particular condition occur when we enable both the
 controllers, so this patch will disable OHCI1 and EHCI1 for
 bananapi-m64 and nanopi-a64 boards. It will re-enable the same
 once the issue got fixed.

 Log:
 => usb reset
 resetting USB...

 PHY0: mask = 0x101, usb_clk_cfg = 0x30202
 sunxi_musb_exit: ahb_gate0  = 0x33004540, reset0_cfg = 0x33004540
 EHCI failed to shut down host controller.
 ehci_usb_remove: ahb_gate0  = 0x32004540, reset0_cfg = 0x32004540
 ohci_usb_remove: ahb_gate0  = 0x22004540, reset0_cfg = 0x22004540
 ohci_usb_remove: mask = 0x1, usb_clk_cfg = 0x20202

 PHY1: mask = 0x202, usb_clk_cfg = 0x0
 ehci_usb_remove: ahb_gate0  = 0x20004540, reset0_cfg = 0x20004540
>>>
>>> Why is this usb reset so noisy ?
>>
>> ... I would assume additional debug messages.
>>
 << hang here >>
>>>
>>> Please fix this properly, this patch is pure attempt to hide a bug.
>>> NAK from me.
>>
>> Well, the point of this patch as Jagan says is to hide the bug for the
>> release so that Linux can boot, which is an important case.
> 
> But the above implies that Linux can boot and the hang happens while
> still in U-Boot due to some ordering bug between clock and register
> access in the .remove function of some driver (I guess). That is what
> needs to be debugged and fixed.
> 
>> Jagan, can you bisect down to when this started happening?  I assume
>> it's a recent'ish thing.  Thanks!
>>
> 
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] ARM: socfpga: Assure correct ACTLR configuration

2018-07-03 Thread See, Chin Liang
On Sat, 2018-06-23 at 06:23 +0200, Marek Vasut wrote:
> On 06/23/2018 05:55 AM, Marek Vasut wrote:
> > 
> > On 06/06/2018 08:47 PM, Marek Vasut wrote:
> > > 
> > > On 05/31/2018 12:00 PM, Marek Vasut wrote:
> > > > 
> > > > On 05/31/2018 11:52 AM, See, Chin Liang wrote:
> > > > > 
> > > > > On Tue, 2018-05-29 at 18:39 +0200, Marek Vasut wrote:
> > > > > > 
> > > > > > Make sure the ARM ACTLR register has correct configuration,
> > > > > > otherwise
> > > > > > the Linux kernel refuses to boot. In particular, the "Write
> > > > > > Full Line
> > > > > > of Zeroes" bit must be cleared.
> > > > > > 
> > > > > > Signed-off-by: Marek Vasut 
> > > > > > Cc: Chin Liang See 
> > > > > > Cc: Dinh Nguyen 
> > > > > > ---
> > > > > > NOTE: This gem was well hidden in the Altera U-Boot fork
> > > > > > and is
> > > > > > really needed.
> > > > > >   What is not entirely clear to me is WHY ? So why is
> > > > > > this needed
> > > > > > ?
> > > > > I vaguely recall it's related to HW constrain. And check back
> > > > > the
> > > > > downstream U-Boot, actually we need to set the bit instead
> > > > > clearing
> > > > > it. https://github.com/altera-opensource/u-boot-socfpga/blob/
> > > > > socfpga_v2
> > > > > 014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/pl3
> > > > > 10.c
> > > > You are clearing it here:
> > > > https://github.com/altera-opensource/u-boot-socfpga/blob/socfpg
> > > > a_v2014.10_arria10_bringup/arch/arm/cpu/armv7/socfpga_arria10/l
> > > > owlevel_init.S#L35
> > > > 
> > > > The PL310 configuration is a different register. What HW
> > > > constraint ?
> > > > I'd really like to understand this problem.
> > > Bump ? I had a report the register even has to be set to 0 ,
> > > otherwise
> > > there are ethernet problems. What is going on with this register
> > > ?
> > Bump again ?
> > 
> Expanding the CC list. I am tempted to stop accepting any socfpga
> patches until this is resolved.
> 

Just realize this is not addressed as Ley Foon talked to me

We checked with HW team that the full line of zeroes is a special setup
of signals between A9 and L2 cache controller. As they recalled that
this signal is not 100% AXI compliance, it may hang the system if L2
cache is disabled while full line of zeroes is set.

In A10, one thing new is we enabled L2 cache in BootROM and this bit is
set. When entering the U-Boot, we are ensuring all cache are disabled.
At this point, we need to ensure full line of zeroes bit is cleared.
Only when U-Boot renable the cache including L2, this bit is set again.

Hope this explains.

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


Re: [U-Boot] efi_loader: runtime services implementation broken?

2018-07-03 Thread Alexander Graf

On 07/03/2018 04:56 PM, Bin Meng wrote:

Hi Alex,

On Tue, Jul 3, 2018 at 10:32 PM, Alexander Graf  wrote:

On 07/03/2018 04:24 PM, Bin Meng wrote:

Hi Alex, Heinrich,

At present not all interfaces in lib/efi_loader/efi_runtime.c are
declared as __efi_runtime. But only declaring those as __efi_runtime
is not enough. The data these interfaces use should be declared as
__efi_runtime_data too. More worse, any U-Boot APIs called by these
interfaces should be __efi_runtime and __efi_runtime_data
respectively.


Correct. This is done for everything right now, no?

For example, efi_set_virtual_address_map() is not declared as __efi_runtime.


Is that called post exit boot services on x86?




Eventually we need mark the RAM where the whole U-Boot image lives as
runtime service code and data and end up leaving whole U-Boot image in
the RAM after kernel boots?


Why?


Unless we track down every APIs called by runtime services and mark
them correctly as __efi_runtime code/data.


That's the idea, yes. The vector should be quite small as long as we 
don't actually implement / reuse drivers.





Right now I am testing booting Linux on Intel Galileo with 'bootefi'
and kernel just hangs during the boot. Initial debugging shows that
kernel crashes when calling runtime service
efi_set_virtual_address_map().


Can you please try with my efi-next branch? I added a few patches that allow
gcc to put implicit data and code into the runtime section. Without those,
you may get constant propagation into a the common .rodata segment for
example.


Still the same.


Ok, maybe the x86 efi stub does things different from the ARM one then.


Alex

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


Re: [U-Boot] [PATCH 4/4] x86: Switch to use DM sysreset driver

2018-07-03 Thread Bin Meng
Hi Andy,

On Tue, Jul 3, 2018 at 6:42 PM, Andy Shevchenko
 wrote:
> On Tue, 2018-07-03 at 02:48 -0700, Bin Meng wrote:
>> This converts all x86 boards over to DM sysreset.
>
>> -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
>> argv[])
>> -{
>> - printf("resetting ...\n");
>> -
>> - /* wait 50 ms */
>> - udelay(5);
>> - disable_interrupts();
>> - reset_cpu(0);
>
>> -}
>
>> --- a/arch/x86/cpu/tangier/tangier.c
>> +++ b/arch/x86/cpu/tangier/tangier.c
>> @@ -25,7 +25,15 @@ int print_cpuinfo(void)
>>   return default_print_cpuinfo();
>>  }
>>
>> +/* TODO: convert to DM sysreset */
>>  void reset_cpu(ulong addr)
>>  {
>>   scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
>>  }
>> +
>> +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
>> argv[])
>> +{
>> + reset_cpu(0);
>> +
>> + return 0;
>
> This is not equivalent to the above.
>
> First of all, in some cases would be good to have at least a debug
> message that we got into do_reset().
>
> Second, I didn't test if udelay() + disable_interrupts() make any
> difference. So, I would leave them for now.
>

OK, will leave them in v2.

>> +}
>
>> diff --git a/arch/x86/dts/edison.dts b/arch/x86/dts/edison.dts
>> index 9033532..a1d3c90 100644
>> --- a/arch/x86/dts/edison.dts
>> +++ b/arch/x86/dts/edison.dts
>> @@ -9,6 +9,7 @@
>>  #include 
>>
>>  /include/ "skeleton.dtsi"
>
>> +/include/ "reset.dtsi"
>
> If i read this right we are not using generic reset sequence.
> Why do we include this here?
>

This is a mistake. Will fix in v2.

BTW: do you have time to convert the tangier SoC reset driver to DM?

>>  /include/ "rtc.dtsi"
>>  /include/ "tsc_timer.dtsi"
>
>> --- a/configs/edison_defconfig
>> +++ b/configs/edison_defconfig
>
>> +# CONFIG_SYSRESET is not set
>
> --

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] efi_loader: runtime services implementation broken?

2018-07-03 Thread Bin Meng
Hi Alex,

On Tue, Jul 3, 2018 at 10:32 PM, Alexander Graf  wrote:
> On 07/03/2018 04:24 PM, Bin Meng wrote:
>>
>> Hi Alex, Heinrich,
>>
>> At present not all interfaces in lib/efi_loader/efi_runtime.c are
>> declared as __efi_runtime. But only declaring those as __efi_runtime
>> is not enough. The data these interfaces use should be declared as
>> __efi_runtime_data too. More worse, any U-Boot APIs called by these
>> interfaces should be __efi_runtime and __efi_runtime_data
>> respectively.
>
>
> Correct. This is done for everything right now, no?

For example, efi_set_virtual_address_map() is not declared as __efi_runtime.

>
>> Eventually we need mark the RAM where the whole U-Boot image lives as
>> runtime service code and data and end up leaving whole U-Boot image in
>> the RAM after kernel boots?
>
>
> Why?
>

Unless we track down every APIs called by runtime services and mark
them correctly as __efi_runtime code/data.

>> Right now I am testing booting Linux on Intel Galileo with 'bootefi'
>> and kernel just hangs during the boot. Initial debugging shows that
>> kernel crashes when calling runtime service
>> efi_set_virtual_address_map().
>
>
> Can you please try with my efi-next branch? I added a few patches that allow
> gcc to put implicit data and code into the runtime section. Without those,
> you may get constant propagation into a the common .rodata segment for
> example.
>

Still the same.

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Tom Rini
On Tue, Jul 03, 2018 at 06:06:37PM +0530, Jagan Teki wrote:
> On Tue, Jul 3, 2018 at 3:10 AM, Tom Rini  wrote:
> > On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
> >> On 07/02/2018 10:53 PM, Jagan Teki wrote:
> >> > During usb shutdown or 'usb reset' all the necessary clocks
> >> > on the specific controller will disable. Usually this shutdown
> >> > happen during U-Boot proper handoff to Linux.
> >>
> >> No, 'usb reset' can be triggered by the user any time.
> >
> > Yes, and it's also triggered as part of the hand-off, which is the use
> > case in question.
> >
> >> > There is an issue in Allwinner A64, is during OHCI1 shutdown
> >> > the controller is unable to access the register space
> >> > so the Linux boot hangs at this place.
> >>
> >> This doesn't make any sense, Linux should enable the necessary clock
> >> before accessing any controller registers, unless there is a bug in Linux.
> >
> > Should but doesn't always?  So yes, there's possibly / hopefully a bug
> > in the dts files.
> >
> >> > This particular condition occur when we enable both the
> >> > controllers, so this patch will disable OHCI1 and EHCI1 for
> >> > bananapi-m64 and nanopi-a64 boards. It will re-enable the same
> >> > once the issue got fixed.
> >> >
> >> > Log:
> >> > => usb reset
> >> > resetting USB...
> >> >
> >> > PHY0: mask = 0x101, usb_clk_cfg = 0x30202
> >> > sunxi_musb_exit: ahb_gate0  = 0x33004540, reset0_cfg = 0x33004540
> >> > EHCI failed to shut down host controller.
> >> > ehci_usb_remove: ahb_gate0  = 0x32004540, reset0_cfg = 0x32004540
> >> > ohci_usb_remove: ahb_gate0  = 0x22004540, reset0_cfg = 0x22004540
> >> > ohci_usb_remove: mask = 0x1, usb_clk_cfg = 0x20202
> >> >
> >> > PHY1: mask = 0x202, usb_clk_cfg = 0x0
> >> > ehci_usb_remove: ahb_gate0  = 0x20004540, reset0_cfg = 0x20004540
> >>
> >> Why is this usb reset so noisy ?
> >
> > ... I would assume additional debug messages.
> >
> >> > << hang here >>
> >>
> >> Please fix this properly, this patch is pure attempt to hide a bug.
> >> NAK from me.
> >
> > Well, the point of this patch as Jagan says is to hide the bug for the
> > release so that Linux can boot, which is an important case.
> >
> > Jagan, can you bisect down to when this started happening?  I assume
> > it's a recent'ish thing.  Thanks!
> 
> commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
> is effecting this change, but functionally this commit is proper but
> handling clock directly in drivers look not effective particularly in
> the case of A64 where sharing of clocks and gates between OHCI and
> EHCI.
> 
> Here are the possible changes to look at it.
> 
> 1)  with this patch, disabling [oe]hci1 controllers on two boards (bpi
> and nanopi):
>   this change specific to two boards, rest of A64 boards are OK.
> 
> 2)  turn-off gate and clocks for H3/H5/A64 from Andre [1]
>   this would change all H3/H5/A64 shutdown behavior not to disable
> gates and clocks during .remove
> 
> 3)  turn-off clock only for A64
>  this would fix, two board problems in 1) and also specific to A64.
> 
> 4)  add counter mechanism to disable all clock at once, suggested by Marek
>  either we can add counter mechanism to A64 or for all other SOC.
> this need to test in all Allwinner boards if the counter is globally
> managed in ohci-sunxi.c
> 
> Again, there is an ongoing work for syncing all DT changes from Linux,
> by Andre. 1) change will update later in the release. rest of 2), 3)
> and 4) will make changes in driver [eo]hci-sunxi.c and this drivers
> indeed remove once we have dm clock which would also planned for
> upcoming releases.
> 
> [1] https://patchwork.ozlabs.org/patch/938279/

Adding a few more people to the list.  It looks like b62cdbddedc3 was in
response to fef73766d9ad.  So, this close to the release, what do we
need to do to get things back to the state things were in for v2018.05?
And then what are the combinations that aren't working and need to be
addressed again in v2018.09 so that we can make forward progress?
Thanks!

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] Support of device-tree for PowerPC platform: Query

2018-07-03 Thread Bin Meng
Hi Prabhakar,

On Mon, Jul 2, 2018 at 11:27 PM, Prabhakar Kushwaha
 wrote:
> Dear Bin,
>
> Coming back to x86 example.
>
>> -Original Message-
>> From: Bin Meng [mailto:bmeng...@gmail.com]
>> Sent: Friday, June 29, 2018 7:03 AM
>> To: York Sun 
>> Cc: Simon Glass ; Prabhakar Kushwaha
>> ; u-boot@lists.denx.de
>> Subject: Re: [U-Boot] Support of device-tree for PowerPC platform: Query
>>
>> Hi York,
>>
>> On Thu, Jun 28, 2018 at 11:32 PM, York Sun  wrote:
>> > On 06/27/2018 10:53 PM, Bin Meng wrote:
>> > 
>> >>
>> >>> Can the DT perhaps go before U-Boot in the flash? We would need a
>> >>> way to find it though.
>> >>>
>> >>
>> >> I don't see any issue that DT go after U-Boot in the flash. x86 does
>> >> this way, and its reset vector is also the last address of flash.
>> >>
>> >
>> > Big issue. e500 runs from the last address of the flash. We cannot put
>> > DT after U-Boot.
>>
>> Looks you did not get it. I know e500 reset vector is the last 4 bytes below 
>> 4G.
>> This is similar to x86. DTB can be put after the u-boot image without reset
>> vector. You may check how x86 does this.
>>
>
> I tried to search thing for x86 but did not succeed ☹
>

You can test x86 build easily with QEMU.

$ make qemu-x86_defconfig
$ make V=1

This way you can see how u-boot.rom image is built.

> I request you to help me with sample code and boot flow.
> I will try to understand and see how it can help PowerPC.
>

Please see the sample build flow below:

1.  objcopy --gap-fill=0xff  -O binary -R .start16 -R .resetvec u-boot
u-boot-nodtb.bin

.start16 and .resetvec is the x86 boot vector. This is similar to PPC
BookE's reset vector. These two sections are removed to generate a new
u-boot-nodtb binary.

2. cat arch/x86/dts/qemu-x86_i440fx.dtb > dts/dt.dtb
cat u-boot-nodtb.bin dts/dt.dtb > u-boot-dtb.bin

Append dtb to the end of the u-boot-nodtb binary and get a new
u-boot-dtb binary.

3. cp dts/dt.dtb u-boot.dtb
objcopy --gap-fill=0xff  -O binary -j .start16 -j .resetvec u-boot
u-boot-x86-16bit.bin

Create a binary which contains the reset vector.

4. ./tools/binman/binman -d u-boot.dtb -O . -I . -I
./board/emulation/qemu-x86 u-boot-x86-16bit.bin

Use binman to assemble the final u-boot.rom image.

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [upstream-release] [PATCH v2 3/7] misc: fsl_portals: setup QMAN_BAR{E} also on ARM platforms

2018-07-03 Thread Laurentiu Tudor
Hi Bharat,

Thanks for the review! Comments inline.

On 03.07.2018 16:38, Bharat Bhushan wrote:
> 
> 
>> -Original Message-
>> From: upstream-release-boun...@linux.freescale.net [mailto:upstream-
>> release-boun...@linux.freescale.net] On Behalf Of Laurentiu Tudor
>> Sent: Tuesday, July 3, 2018 5:42 PM
>> To: York Sun ; Prabhakar Kushwaha
>> ; u-boot@lists.denx.de
>> Cc: Laurentiu Tudor 
>> Subject: [upstream-release] [PATCH v2 3/7] misc: fsl_portals: setup
>> QMAN_BAR{E} also on ARM platforms
>>
>> QMAN_BAR{E} register setup was disabled on ARM platforms,
>> however the register does need to be set. Add code that
>> sets it up on ARMs.
>>
>> Signed-off-by: Laurentiu Tudor 
>> ---
>>   drivers/misc/fsl_portals.c | 7 ++-
>>   1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/misc/fsl_portals.c b/drivers/misc/fsl_portals.c
>> index 7c22b8d209..5b1233740b 100644
>> --- a/drivers/misc/fsl_portals.c
>> +++ b/drivers/misc/fsl_portals.c
>> @@ -24,14 +24,19 @@ void setup_qbman_portals(void)
>>  CONFIG_SYS_BMAN_SWP_ISDR_REG;
>>  void __iomem *qpaddr = (void *)CONFIG_SYS_QMAN_CINH_BASE +
>>  CONFIG_SYS_QMAN_SWP_ISDR_REG;
>> -#ifdef CONFIG_PPC
>>  struct ccsr_qman *qman = (void *)CONFIG_SYS_FSL_QMAN_ADDR;
>>
>>  /* Set the Qman initiator BAR to match the LAW (for DQRR stashing)
>> */
>> +#ifdef CONFIG_PPC32
>>   #ifdef CONFIG_PHYS_64BIT
>>  out_be32(>qcsp_bare,
>> (u32)(CONFIG_SYS_QMAN_MEM_PHYS >> 32));
>>   #endif
>>  out_be32(>qcsp_bar,
>> (u32)CONFIG_SYS_QMAN_MEM_PHYS);
>> +#else
>> +#ifdef CONFIG_PHYS_64BIT
>> +out_be32(>qcsp_bare,
>> (u32)(CONFIG_SYS_QMAN_MEM_BASE >> 32));
>> +#endif
>> +out_be32(>qcsp_bar,
>> (u32)CONFIG_SYS_QMAN_MEM_BASE);
>>   #endif
> 
> Can we have same hash define name for PPC and non-PPC?
> We do not need then if-else

Hmm, good point. I could use CONFIG_SYS_QMAN_MEM_PHYS which is also 
defined on ls104x [1] but seems to be incorrectly set ... maybe 
copypaste error from powerpc? Another thing i noticed is that it's not 
actually used at all on ARMs.
I could rewrite the patch to fix the define and simplify the code above, 
if there aren't any objections.

[1]
  #define CONFIG_SYS_QMAN_MEM_PHYS (0xfull + 
CONFIG_SYS_QMAN_MEM_BASE)

---
Best Regards, Laurentiu

> 
>>   #ifdef CONFIG_FSL_CORENET
>>  int i;
>> --
>> 2.17.1
>>
>> ___
>> upstream-release mailing list
>> upstream-rele...@linux.freescale.net
>> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flinu
>> x.freescale.net%2Fmailman%2Flistinfo%2Fupstream-
>> release=02%7C01%7Cbharat.bhushan%40nxp.com%7Ca0e936ecc69f40
>> 6be68208d5e0de4f7b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
>> C636662167788206253=WGcw1oH3ktggbybsoMllewQU5Uvbyv8OWa9
>> nBJipK70%3D=0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] efi_loader: runtime services implementation broken?

2018-07-03 Thread Alexander Graf

On 07/03/2018 04:24 PM, Bin Meng wrote:

Hi Alex, Heinrich,

At present not all interfaces in lib/efi_loader/efi_runtime.c are
declared as __efi_runtime. But only declaring those as __efi_runtime
is not enough. The data these interfaces use should be declared as
__efi_runtime_data too. More worse, any U-Boot APIs called by these
interfaces should be __efi_runtime and __efi_runtime_data
respectively.


Correct. This is done for everything right now, no?


Eventually we need mark the RAM where the whole U-Boot image lives as
runtime service code and data and end up leaving whole U-Boot image in
the RAM after kernel boots?


Why?


Right now I am testing booting Linux on Intel Galileo with 'bootefi'
and kernel just hangs during the boot. Initial debugging shows that
kernel crashes when calling runtime service
efi_set_virtual_address_map().


Can you please try with my efi-next branch? I added a few patches that 
allow gcc to put implicit data and code into the runtime section. 
Without those, you may get constant propagation into a the common 
.rodata segment for example.



Alex

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


[U-Boot] efi_loader: runtime services implementation broken?

2018-07-03 Thread Bin Meng
Hi Alex, Heinrich,

At present not all interfaces in lib/efi_loader/efi_runtime.c are
declared as __efi_runtime. But only declaring those as __efi_runtime
is not enough. The data these interfaces use should be declared as
__efi_runtime_data too. More worse, any U-Boot APIs called by these
interfaces should be __efi_runtime and __efi_runtime_data
respectively.

Eventually we need mark the RAM where the whole U-Boot image lives as
runtime service code and data and end up leaving whole U-Boot image in
the RAM after kernel boots?

Right now I am testing booting Linux on Intel Galileo with 'bootefi'
and kernel just hangs during the boot. Initial debugging shows that
kernel crashes when calling runtime service
efi_set_virtual_address_map().

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [upstream-release] [PATCH v2 5/7] armv8: ls1046a: initial icid setup support

2018-07-03 Thread Bharat Bhushan


> -Original Message-
> From: upstream-release-boun...@linux.freescale.net [mailto:upstream-
> release-boun...@linux.freescale.net] On Behalf Of Laurentiu Tudor
> Sent: Tuesday, July 3, 2018 5:42 PM
> To: York Sun ; Prabhakar Kushwaha
> ; u-boot@lists.denx.de
> Cc: Laurentiu Tudor 
> Subject: [upstream-release] [PATCH v2 5/7] armv8: ls1046a: initial icid setup
> support
> 
> Add infrastructure for ICID setup and device tree
> fixup on ARM platforms. This include basic ICID setup
> for several devices.
> 
> Signed-off-by: Laurentiu Tudor 
> ---
>  arch/arm/cpu/armv8/fsl-layerscape/Makefile|   1 +
>  arch/arm/cpu/armv8/fsl-layerscape/icid.c  | 111 ++
>  .../arm/cpu/armv8/fsl-layerscape/ls1046_ids.c |  29 +
>  arch/arm/cpu/armv8/fsl-layerscape/soc.c   |   3 +
>  .../asm/arch-fsl-layerscape/fsl_icid.h|  80 +
>  board/freescale/ls1046aqds/ls1046aqds.c   |   2 +
>  board/freescale/ls1046ardb/ls1046ardb.c   |   3 +
>  7 files changed, 229 insertions(+)
>  create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/icid.c
>  create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
>  create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
> 
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> index 1e9e4680fe..5d6f68aad6 100644
> --- a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
> @@ -37,6 +37,7 @@ endif
> 
>  ifneq ($(CONFIG_ARCH_LS1046A),)
>  obj-$(CONFIG_SYS_HAS_SERDES) += ls1046a_serdes.o
> +obj-y += icid.o ls1046_ids.o
>  endif
> 
>  ifneq ($(CONFIG_ARCH_LS1088A),)
> diff --git a/arch/arm/cpu/armv8/fsl-layerscape/icid.c
> b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
> new file mode 100644
> index 00..8694bd6fa1
> --- /dev/null
> +++ b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright 2018 NXP
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +static void set_icid(struct icid_id_table *tbl, int size)
> +{
> + int i;
> +
> + for (i = 0; i < size; i++)
> + out_be32((u32 *)(tbl[i].reg_addr), tbl[i].reg);
> +}
> +
> +void set_icids(void)
> +{
> + /* setup general icid offsets */
> + set_icid(icid_tbl, icid_tbl_sz);

Icid_tbl[] is currently defined for ls1046 but this code is generic and later 
can be used for ls1043 later.
We can let caller provide table pointer and size.

> +}
> +
> +int fdt_set_iommu_prop(void *blob, int off, int smmu_ph, u32 *ids, int
> num_ids)
> +{
> + int i, ret;
> + u32 prop[8];
> +
> + for (i = 0; i < num_ids; i++) {
> + prop[i * 2] = cpu_to_fdt32(smmu_ph);
> + prop[i * 2 + 1] = cpu_to_fdt32(ids[i]);
> + }
> + ret = fdt_setprop(blob, off, "iommus",
> +   prop, sizeof(u32) * num_ids * 2);
> + if (ret > 0) {
> + printf("WARNING unable to set iommus: %s\n",
> fdt_strerror(off));
> + return off;
> + }
> + ret = fdt_setprop_empty(blob, off, "dma-coherent");
> + if (ret > 0) {
> + printf("WARNING unable to set dma-coherent: %s\n",
> +fdt_strerror(off));
> + return off;
> + }
> +
> + return 0;
> +}
> +
> +int fdt_fixup_icid_tbl(void *blob, int smmu_ph,
> +struct icid_id_table *tbl, int size)

What we set in device tree is stream-id, while stream-id is = [PL: BMT : ICID];
 I think we should use stream-id to the point when we are patching device-tree 
and value set in h/w.
We can say PL = BMT = 0 for now and leave space for setting this in future for 
any specific device for any platform?

> +{
> + int i, err, off;
> +
> + for (i = 0; i < size; i++) {
> + if (!tbl[i].compat)
> + continue;
> +
> + off = fdt_node_offset_by_compat_reg(blob,
> + tbl[i].compat,
> + tbl[i].compat_addr);
> + if (off > 0) {
> + err = fdt_set_iommu_prop(blob, off, smmu_ph,
> +  [i].id, 1);
> + if (err)
> + return err;
> + } else {
> + printf("WARNING could not find node %s: %s.\n",
> +tbl[i].compat, fdt_strerror(off));
> + }
> + }
> +
> + return 0;
> +}
> +
> +int fdt_get_smmu_phandle(void *blob)
> +{
> + int noff, smmu_ph;
> +
> + noff = fdt_node_offset_by_compatible(blob, -1, "arm,mmu-500");
> + if (noff < 0) {
> + printf("WARNING failed to get smmu node: %s\n",
> +fdt_strerror(noff));
> + return noff;
> + }
> +
> + smmu_ph = fdt_get_phandle(blob, noff);
> + if (!smmu_ph) {
> + 

Re: [U-Boot] SoCFPGA PL330 DMA driver and ECC scrubbing

2018-07-03 Thread Marek Vasut
On 07/03/2018 03:58 PM, Jason Rush wrote:
> On 6/29/2018 10:17 AM, Marek Vasut wrote:
>> On 06/29/2018 05:06 PM, Jason Rush wrote:
>>> On 6/29/2018 9:52 AM, Marek Vasut wrote:
 On 06/29/2018 04:44 PM, Jason Rush wrote:
> On 6/29/2018 9:34 AM, Marek Vasut wrote:
>> On 06/29/2018 04:31 PM, Jason Rush wrote:
>>> Dinh,
>> Hi,
>>
>>> A while ago, you posted the following patchset for SoCFPGA to add the 
>>> PL330
>>> DMA driver, and updated the SoCFPGA SDRAM init to write zeros to SDRAM 
>>> to
>>> initialize the ECC bits if ECC was enabled:
>>>
>>> https://lists.denx.de/pipermail/u-boot/2016-October/269643.html
>>>
>>> I know it's been a long time, so I'll summarize some of the 
>>> conversation...
>>>
>>> At the time, you had a problem with the patchset causing the SPL to 
>>> fail to
>>> find the MMC.  You had tracked it down to an issue with the following 
>>> commit
>>> "a78cd8613204 ARM: Rework and correct barrier definitions".  You and 
>>> Marek
>>> discussed it a bit, but I don't think there was a real conclusion.  You
>>> submitted a second version of the patchset asking for advice on 
>>> debugging
>>> the issue:
>>>
>>> https://lists.denx.de/pipermail/u-boot/2016-December/275822.html
>>>
>>> No real conversation came from the second patchset, and that was the 
>>> end of
>>> the patch.
>>>
>>> I was hoping we could revisit adding your patchset again. I am working 
>>> on a
>>> custom SoCFPGA board with a Cyclone V and ECC SDRAM. I rebased your 
>>> patchset
>>> against v2018.05 and it is working on my custom board (although I don't 
>>> have
>>> an MMC). I also tested it on a SoCKit booting from an MMC (I forced it 
>>> to
>>> scrub the SDRAM on the SoCKit, because it doesn't have ECC RAM), and the
>>> SoCKit finds the MMC and boots.
>>>
>>> I don't have any suggestions on why it is working now on my board and 
>>> not
>>> back when you first submitted the patchset.  Maybe something else was 
>>> fixed
>>> in the MMC? I was hoping you and Marek could test this patch again on 
>>> some
>>> different SoCFPGA boards to see if you get the same results.
>> Look at this patch
>> http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=9bb8a249b292d26f152c20e3641600b3d7b3924b
>>
>> You likely want similar approach, it's faster then the DMA and much 
>> simpler.
>>
> Thanks Marek.  I'll give it a try.  Would you be interested in a similar 
> patch for the Gen 5?
 I don't have any Gen5 board which uses ECC, do you ?
 If so, yes, prepare a patch, it should be very similar.

 Make sure to measure how long it takes to scrub the memory and how much
 memory you have, I'd be interested in the numbers.

>>> Looking at the master branch, it doesn't look like that code is ever being 
>>> called?
>>> The sdram_init_ecc_bits() function is called from the 
>>> ddr_calibration_sequence function(),
>>> but I can't find where ddr_calibration_sequence is called().
>> git grep for it, it's called from somewhere in the arch/arm/mach-socfpga/
>>
>>> Either way, I can test it. I have a custom Cyclone V board with ECC, and 
>>> the Intel Arria V SoC
>>> Dev Kit I can test it on too which I think has ECC.
>> Please do.
>>
> I implemented a similar memset approach for the gen 5 socfpga.  It's 
> basically the same
> code as in that patch; however, when I performed a single memset the 
> processor would
> reset for some reason.  I changed it to loop over calling memset with a size 
> of 32MB over
> the entire address the address, and that worked as opposed to doing a single 
> memset on
> the RAM.

Can you do grep MEMSET .config in your U-Boot build dir ? The arch
memset is implemented in assembler and doesn't trigger WDT , so if it
takes too long, it could be that the WDT resets the platform.

> I started on a SoCKit because it was handy, I know it doesn't have ECC

It doesn't by default.

>, but I forced it to
> initialize the RAM as a quick test.  It seems much slower than the DMA 
> approach.  It
> should be noted, I didn't implement any code to time the scrubbing, but 
> rather just
> roughly monitored the time to get a rough idea of how long it took.
> 
> On the SoCKit, which has 1GB of RAM, the memset takes around 8 seconds to 
> complete,
> and the DMA takes under 2 seconds.

Did you enable i/d cache in the SPL ? It's mandatory, otherwise it's
slow. Just be careful about the MMU tables placement, they are big and
if you place them in RAM, make sure you don't overwrite them with the
memset. The trick might be to memset the first 1 MiB of RAM, then put
MMU tables at some offset therein (since 0x0 can be used for ARM
vectors) and then turn on i/d cache and memset the rest.

> Maybe I ported something wrong?  I used very similar code as the 
> sdram_init_ecc_bits()

Re: [U-Boot] SoCFPGA PL330 DMA driver and ECC scrubbing

2018-07-03 Thread Jason Rush
On 6/29/2018 10:17 AM, Marek Vasut wrote:
> On 06/29/2018 05:06 PM, Jason Rush wrote:
>> On 6/29/2018 9:52 AM, Marek Vasut wrote:
>>> On 06/29/2018 04:44 PM, Jason Rush wrote:
 On 6/29/2018 9:34 AM, Marek Vasut wrote:
> On 06/29/2018 04:31 PM, Jason Rush wrote:
>> Dinh,
> Hi,
>
>> A while ago, you posted the following patchset for SoCFPGA to add the 
>> PL330
>> DMA driver, and updated the SoCFPGA SDRAM init to write zeros to SDRAM to
>> initialize the ECC bits if ECC was enabled:
>>
>> https://lists.denx.de/pipermail/u-boot/2016-October/269643.html
>>
>> I know it's been a long time, so I'll summarize some of the 
>> conversation...
>>
>> At the time, you had a problem with the patchset causing the SPL to fail 
>> to
>> find the MMC.  You had tracked it down to an issue with the following 
>> commit
>> "a78cd8613204 ARM: Rework and correct barrier definitions".  You and 
>> Marek
>> discussed it a bit, but I don't think there was a real conclusion.  You
>> submitted a second version of the patchset asking for advice on debugging
>> the issue:
>>
>> https://lists.denx.de/pipermail/u-boot/2016-December/275822.html
>>
>> No real conversation came from the second patchset, and that was the end 
>> of
>> the patch.
>>
>> I was hoping we could revisit adding your patchset again. I am working 
>> on a
>> custom SoCFPGA board with a Cyclone V and ECC SDRAM. I rebased your 
>> patchset
>> against v2018.05 and it is working on my custom board (although I don't 
>> have
>> an MMC). I also tested it on a SoCKit booting from an MMC (I forced it to
>> scrub the SDRAM on the SoCKit, because it doesn't have ECC RAM), and the
>> SoCKit finds the MMC and boots.
>>
>> I don't have any suggestions on why it is working now on my board and not
>> back when you first submitted the patchset.  Maybe something else was 
>> fixed
>> in the MMC? I was hoping you and Marek could test this patch again on 
>> some
>> different SoCFPGA boards to see if you get the same results.
> Look at this patch
> http://git.denx.de/?p=u-boot/u-boot-socfpga.git;a=commit;h=9bb8a249b292d26f152c20e3641600b3d7b3924b
>
> You likely want similar approach, it's faster then the DMA and much 
> simpler.
>
 Thanks Marek.  I'll give it a try.  Would you be interested in a similar 
 patch for the Gen 5?
>>> I don't have any Gen5 board which uses ECC, do you ?
>>> If so, yes, prepare a patch, it should be very similar.
>>>
>>> Make sure to measure how long it takes to scrub the memory and how much
>>> memory you have, I'd be interested in the numbers.
>>>
>> Looking at the master branch, it doesn't look like that code is ever being 
>> called?
>> The sdram_init_ecc_bits() function is called from the 
>> ddr_calibration_sequence function(),
>> but I can't find where ddr_calibration_sequence is called().
> git grep for it, it's called from somewhere in the arch/arm/mach-socfpga/
>
>> Either way, I can test it. I have a custom Cyclone V board with ECC, and the 
>> Intel Arria V SoC
>> Dev Kit I can test it on too which I think has ECC.
> Please do.
>
I implemented a similar memset approach for the gen 5 socfpga.  It's basically 
the same
code as in that patch; however, when I performed a single memset the processor 
would
reset for some reason.  I changed it to loop over calling memset with a size of 
32MB over
the entire address the address, and that worked as opposed to doing a single 
memset on
the RAM.

I started on a SoCKit because it was handy, I know it doesn't have ECC, but I 
forced it to
initialize the RAM as a quick test.  It seems much slower than the DMA 
approach.  It
should be noted, I didn't implement any code to time the scrubbing, but rather 
just
roughly monitored the time to get a rough idea of how long it took.

On the SoCKit, which has 1GB of RAM, the memset takes around 8 seconds to 
complete,
and the DMA takes under 2 seconds.

Maybe I ported something wrong?  I used very similar code as the 
sdram_init_ecc_bits()
function, with the exception I looped over the second memset with a size of 
32MB.  I
wasn't sure why the TLB address/size was being initialized, or if the values 
used on the
Arria 10 are even correct for the Cyclone 5, but I used that part of the code 
as is from
the Arria 10 patch as well as enabling the icache and dcache just like the 
Arria 10 code.

Any suggestions on what to look at?

Jason

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


Re: [U-Boot] [upstream-release] [PATCH v2 3/7] misc: fsl_portals: setup QMAN_BAR{E} also on ARM platforms

2018-07-03 Thread Bharat Bhushan


> -Original Message-
> From: upstream-release-boun...@linux.freescale.net [mailto:upstream-
> release-boun...@linux.freescale.net] On Behalf Of Laurentiu Tudor
> Sent: Tuesday, July 3, 2018 5:42 PM
> To: York Sun ; Prabhakar Kushwaha
> ; u-boot@lists.denx.de
> Cc: Laurentiu Tudor 
> Subject: [upstream-release] [PATCH v2 3/7] misc: fsl_portals: setup
> QMAN_BAR{E} also on ARM platforms
> 
> QMAN_BAR{E} register setup was disabled on ARM platforms,
> however the register does need to be set. Add code that
> sets it up on ARMs.
> 
> Signed-off-by: Laurentiu Tudor 
> ---
>  drivers/misc/fsl_portals.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/fsl_portals.c b/drivers/misc/fsl_portals.c
> index 7c22b8d209..5b1233740b 100644
> --- a/drivers/misc/fsl_portals.c
> +++ b/drivers/misc/fsl_portals.c
> @@ -24,14 +24,19 @@ void setup_qbman_portals(void)
>   CONFIG_SYS_BMAN_SWP_ISDR_REG;
>   void __iomem *qpaddr = (void *)CONFIG_SYS_QMAN_CINH_BASE +
>   CONFIG_SYS_QMAN_SWP_ISDR_REG;
> -#ifdef CONFIG_PPC
>   struct ccsr_qman *qman = (void *)CONFIG_SYS_FSL_QMAN_ADDR;
> 
>   /* Set the Qman initiator BAR to match the LAW (for DQRR stashing)
> */
> +#ifdef CONFIG_PPC32
>  #ifdef CONFIG_PHYS_64BIT
>   out_be32(>qcsp_bare,
> (u32)(CONFIG_SYS_QMAN_MEM_PHYS >> 32));
>  #endif
>   out_be32(>qcsp_bar,
> (u32)CONFIG_SYS_QMAN_MEM_PHYS);
> +#else
> +#ifdef CONFIG_PHYS_64BIT
> + out_be32(>qcsp_bare,
> (u32)(CONFIG_SYS_QMAN_MEM_BASE >> 32));
> +#endif
> + out_be32(>qcsp_bar,
> (u32)CONFIG_SYS_QMAN_MEM_BASE);
>  #endif

Can we have same hash define name for PPC and non-PPC?
We do not need then if-else 

Thanks
-Bharat

>  #ifdef CONFIG_FSL_CORENET
>   int i;
> --
> 2.17.1
> 
> ___
> upstream-release mailing list
> upstream-rele...@linux.freescale.net
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flinu
> x.freescale.net%2Fmailman%2Flistinfo%2Fupstream-
> release=02%7C01%7Cbharat.bhushan%40nxp.com%7Ca0e936ecc69f40
> 6be68208d5e0de4f7b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7
> C636662167788206253=WGcw1oH3ktggbybsoMllewQU5Uvbyv8OWa9
> nBJipK70%3D=0
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v1 1/5] ARM: PSCI: initialize stack pointer on secondary CPUs

2018-07-03 Thread Tom Rini
On Tue, Jul 03, 2018 at 02:32:28PM +0200, Stefan Agner wrote:
> Hi Tom, Hi Stefano,
> 
> On 02.07.2018 11:08, Patrick DELAUNAY wrote:
> > Hi Stefan
> > 
> >> From: Stefan Agner [mailto:ste...@agner.ch]
> >> Sent: mercredi 27 juin 2018 10:36
> >> Subject: Re: [PATCH v1 1/5] ARM: PSCI: initialize stack pointer on 
> >> secondary CPUs
> >>
> >> On 24.06.2018 21:09, Stefan Agner wrote:
> >> > From: Stefan Agner 
> >> >
> >> > A proper stack is required to safely use C code in psci_arch_cpu_entry.
> >>
> >> Patrick, I prefer to have your ack on this since you introduced
> >> psci_arch_cpu_entry.
> >>
> >> As far as I can tell STM32MP1 uses C code in psci_arch_cpu_entry. The same
> >> function in i.MX 7's PSCI implementation the compiler actually pushed 
> >> stuff on
> >> the (uninitialized) stack, which caused the newly brought up CPU to 
> >> immediately
> >> crash.
> >>
> >> Not sure if in your case the stack pointer is already setup by some other 
> >> means
> >> or your compiler does not use the stack.
> >>
> >> In any case, I think it is better to just setup the stack properly as done 
> >> in this
> >> patch...
> > 
> > I expected that the secure stack is initialized by bootROM, but after
> > check on 2018.07-rc2,
> > I got a crash also with the stm32mp1 platform.
> > 
> > After code review, my behavior  is clearly not safe: I don't sure that
> > the initial BootROM stack
> > is not overlapping the installed PSCI monitor code or data.
> > So I agree:  it is needed to initialize the stack in psci_cpu_entry.
> > 
> > Moreover after your patch the crash is solved for my platform stm32mp1.
> 
> Given that this fixes two platforms I guess it definitely should go into
> v2018.07.
> 
> Technically, for i.MX 7 only patch 1, 2 and 3 are necessary, but 4 and 5
> are fairly straight forward and seem to work fine here.
> 
> Patch 1 is generic, so not sure through which trees the patchset should
> go...

I'm OK with the series coming in if Stefano is.

-- 
Tom


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Jagan Teki
On Tue, Jul 3, 2018 at 3:10 AM, Tom Rini  wrote:
> On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
>> On 07/02/2018 10:53 PM, Jagan Teki wrote:
>> > During usb shutdown or 'usb reset' all the necessary clocks
>> > on the specific controller will disable. Usually this shutdown
>> > happen during U-Boot proper handoff to Linux.
>>
>> No, 'usb reset' can be triggered by the user any time.
>
> Yes, and it's also triggered as part of the hand-off, which is the use
> case in question.
>
>> > There is an issue in Allwinner A64, is during OHCI1 shutdown
>> > the controller is unable to access the register space
>> > so the Linux boot hangs at this place.
>>
>> This doesn't make any sense, Linux should enable the necessary clock
>> before accessing any controller registers, unless there is a bug in Linux.
>
> Should but doesn't always?  So yes, there's possibly / hopefully a bug
> in the dts files.
>
>> > This particular condition occur when we enable both the
>> > controllers, so this patch will disable OHCI1 and EHCI1 for
>> > bananapi-m64 and nanopi-a64 boards. It will re-enable the same
>> > once the issue got fixed.
>> >
>> > Log:
>> > => usb reset
>> > resetting USB...
>> >
>> > PHY0: mask = 0x101, usb_clk_cfg = 0x30202
>> > sunxi_musb_exit: ahb_gate0  = 0x33004540, reset0_cfg = 0x33004540
>> > EHCI failed to shut down host controller.
>> > ehci_usb_remove: ahb_gate0  = 0x32004540, reset0_cfg = 0x32004540
>> > ohci_usb_remove: ahb_gate0  = 0x22004540, reset0_cfg = 0x22004540
>> > ohci_usb_remove: mask = 0x1, usb_clk_cfg = 0x20202
>> >
>> > PHY1: mask = 0x202, usb_clk_cfg = 0x0
>> > ehci_usb_remove: ahb_gate0  = 0x20004540, reset0_cfg = 0x20004540
>>
>> Why is this usb reset so noisy ?
>
> ... I would assume additional debug messages.
>
>> > << hang here >>
>>
>> Please fix this properly, this patch is pure attempt to hide a bug.
>> NAK from me.
>
> Well, the point of this patch as Jagan says is to hide the bug for the
> release so that Linux can boot, which is an important case.
>
> Jagan, can you bisect down to when this started happening?  I assume
> it's a recent'ish thing.  Thanks!

commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
is effecting this change, but functionally this commit is proper but
handling clock directly in drivers look not effective particularly in
the case of A64 where sharing of clocks and gates between OHCI and
EHCI.

Here are the possible changes to look at it.

1)  with this patch, disabling [oe]hci1 controllers on two boards (bpi
and nanopi):
  this change specific to two boards, rest of A64 boards are OK.

2)  turn-off gate and clocks for H3/H5/A64 from Andre [1]
  this would change all H3/H5/A64 shutdown behavior not to disable
gates and clocks during .remove

3)  turn-off clock only for A64
 this would fix, two board problems in 1) and also specific to A64.

4)  add counter mechanism to disable all clock at once, suggested by Marek
 either we can add counter mechanism to A64 or for all other SOC.
this need to test in all Allwinner boards if the counter is globally
managed in ohci-sunxi.c

Again, there is an ongoing work for syncing all DT changes from Linux,
by Andre. 1) change will update later in the release. rest of 2), 3)
and 4) will make changes in driver [eo]hci-sunxi.c and this drivers
indeed remove once we have dm clock which would also planned for
upcoming releases.

[1] https://patchwork.ozlabs.org/patch/938279/

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


Re: [U-Boot] [PATCH v1 1/5] ARM: PSCI: initialize stack pointer on secondary CPUs

2018-07-03 Thread Stefan Agner
Hi Tom, Hi Stefano,

On 02.07.2018 11:08, Patrick DELAUNAY wrote:
> Hi Stefan
> 
>> From: Stefan Agner [mailto:ste...@agner.ch]
>> Sent: mercredi 27 juin 2018 10:36
>> Subject: Re: [PATCH v1 1/5] ARM: PSCI: initialize stack pointer on secondary 
>> CPUs
>>
>> On 24.06.2018 21:09, Stefan Agner wrote:
>> > From: Stefan Agner 
>> >
>> > A proper stack is required to safely use C code in psci_arch_cpu_entry.
>>
>> Patrick, I prefer to have your ack on this since you introduced
>> psci_arch_cpu_entry.
>>
>> As far as I can tell STM32MP1 uses C code in psci_arch_cpu_entry. The same
>> function in i.MX 7's PSCI implementation the compiler actually pushed stuff 
>> on
>> the (uninitialized) stack, which caused the newly brought up CPU to 
>> immediately
>> crash.
>>
>> Not sure if in your case the stack pointer is already setup by some other 
>> means
>> or your compiler does not use the stack.
>>
>> In any case, I think it is better to just setup the stack properly as done 
>> in this
>> patch...
> 
> I expected that the secure stack is initialized by bootROM, but after
> check on 2018.07-rc2,
> I got a crash also with the stm32mp1 platform.
> 
> After code review, my behavior  is clearly not safe: I don't sure that
> the initial BootROM stack
> is not overlapping the installed PSCI monitor code or data.
> So I agree:  it is needed to initialize the stack in psci_cpu_entry.
> 
> Moreover after your patch the crash is solved for my platform stm32mp1.

Given that this fixes two platforms I guess it definitely should go into
v2018.07.

Technically, for i.MX 7 only patch 1, 2 and 3 are necessary, but 4 and 5
are fairly straight forward and seem to work fine here.

Patch 1 is generic, so not sure through which trees the patchset should
go...

--
Stefan

> 
>>
>> Stefano, I think we really want patch 2/3 applied before the release since 
>> it fixes
>> i.MX 7 PSCI. Right now the implementation is really broken and not PSCI 1.0
>> conformant. But patch 2/3 require this patch to be applied... Not sure how we
>> should handle this.
>>
>> --
>> Stefan
> 
> Acked-by: Patrick DELAUNAY 
> Tested-by: Patrick DELAUNAY 
> 
> Test done on stm32mp1 platform
> 
> Thanks!
> Patrick
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH v2 6/7] armv8: ls1046a: add icid setup for qman portals

2018-07-03 Thread Laurentiu Tudor
Add support for ICID setting of qman portals and
the required device tree fixups.
Also fix an endiness issue in portal setup code.

Signed-off-by: Laurentiu Tudor 
---
 .../arm/cpu/armv8/fsl-layerscape/ls1046_ids.c | 16 +++
 .../asm/arch-fsl-layerscape/fsl_portals.h | 23 ++
 drivers/misc/fsl_portals.c| 43 +++
 3 files changed, 74 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/fsl_portals.h

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c 
b/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
index d9e03fcb89..62bad3ab9c 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
@@ -6,6 +6,22 @@
 #include 
 #include 
 #include 
+#include 
+
+#ifdef CONFIG_SYS_DPAA_QBMAN
+struct qportal_info qp_info[CONFIG_SYS_QMAN_NUM_PORTALS] = {
+   SET_QP_INFO(FSL_DPAA1_STREAM_ID_END, 0),
+   SET_QP_INFO(FSL_DPAA1_STREAM_ID_END, 0),
+   SET_QP_INFO(FSL_DPAA1_STREAM_ID_END, 0),
+   SET_QP_INFO(FSL_DPAA1_STREAM_ID_END, 0),
+   SET_QP_INFO(FSL_DPAA1_STREAM_ID_END, 0),
+   SET_QP_INFO(FSL_DPAA1_STREAM_ID_END, 0),
+   SET_QP_INFO(FSL_DPAA1_STREAM_ID_END, 0),
+   SET_QP_INFO(FSL_DPAA1_STREAM_ID_END, 0),
+   SET_QP_INFO(FSL_DPAA1_STREAM_ID_END, 0),
+   SET_QP_INFO(FSL_DPAA1_STREAM_ID_END, 0),
+};
+#endif
 
 struct icid_id_table icid_tbl[] = {
 #ifdef CONFIG_SYS_DPAA_QBMAN
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/fsl_portals.h 
b/arch/arm/include/asm/arch-fsl-layerscape/fsl_portals.h
new file mode 100644
index 00..bd8d3fb49a
--- /dev/null
+++ b/arch/arm/include/asm/arch-fsl-layerscape/fsl_portals.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2018 NXP
+ */
+
+#ifndef _FSL_PORTALS_H_
+#define _FSL_PORTALS_H_
+
+struct qportal_info {
+   u16 dicid;  /* DQRR ICID */
+   u16 ficid;  /* frame data ICID */
+   u16 icid;
+   u8  sdest;
+};
+
+#define SET_QP_INFO(_icid, dest) \
+   { .dicid = _icid, .ficid = _icid, .icid = _icid, .sdest = dest }
+
+extern struct qportal_info qp_info[];
+void fdt_portal(void *blob, const char *compat, const char *container,
+   u64 addr, u32 size);
+
+#endif
diff --git a/drivers/misc/fsl_portals.c b/drivers/misc/fsl_portals.c
index 5b1233740b..da174fc7e2 100644
--- a/drivers/misc/fsl_portals.c
+++ b/drivers/misc/fsl_portals.c
@@ -13,6 +13,9 @@
 #ifdef CONFIG_PPC
 #include 
 #include 
+#else
+#include 
+#include 
 #endif
 #include 
 
@@ -52,6 +55,22 @@ void setup_qbman_portals(void)
/* set frame liodn */
out_be32(>qcsp[i].qcsp_io_cfg, (sdest << 16) | fliodn);
}
+#else
+#ifdef CONFIG_ARM
+   int i;
+
+   for (i = 0; i < CONFIG_SYS_QMAN_NUM_PORTALS; i++) {
+   u8 sdest = qp_info[i].sdest;
+   u16 ficid = qp_info[i].ficid;
+   u16 dicid = qp_info[i].dicid;
+   u16 icid = qp_info[i].icid;
+
+   out_be32(>qcsp[i].qcsp_lio_cfg, (icid << 16) |
+   dicid);
+   /* set frame icid */
+   out_be32(>qcsp[i].qcsp_io_cfg, (sdest << 16) | ficid);
+   }
+#endif
 #endif
 
/* Change default state of BMan ISDR portals to all 1s */
@@ -185,6 +204,10 @@ void fdt_fixup_qportals(void *blob)
char compat[64];
int compat_len;
 
+#ifndef CONFIG_PPC
+   int smmu_ph = fdt_get_smmu_phandle(blob);
+#endif
+
maj = (rev_1 >> 8) & 0xff;
min = rev_1 & 0xff;
ip_cfg = rev_2 & 0xff;
@@ -195,7 +218,6 @@ void fdt_fixup_qportals(void *blob)
 
off = fdt_node_offset_by_compatible(blob, -1, "fsl,qman-portal");
while (off != -FDT_ERR_NOTFOUND) {
-#ifdef CONFIG_PPC
 #ifdef CONFIG_FSL_CORENET
u32 liodns[2];
 #endif
@@ -205,12 +227,7 @@ void fdt_fixup_qportals(void *blob)
if (!ci)
goto err;
 
-   i = *ci;
-#ifdef CONFIG_SYS_DPAA_FMAN
-   int j;
-#endif
-
-#endif /* CONFIG_PPC */
+   i = fdt32_to_cpu(*ci);
err = fdt_setprop(blob, off, "compatible", compat, compat_len);
if (err < 0)
goto err;
@@ -242,7 +259,7 @@ void fdt_fixup_qportals(void *blob)
 #endif
 
 #ifdef CONFIG_SYS_DPAA_FMAN
-   for (j = 0; j < CONFIG_SYS_NUM_FMAN; j++) {
+   for (int j = 0; j < CONFIG_SYS_NUM_FMAN; j++) {
char name[] = "fman@0";
 
name[sizeof(name) - 2] = '0' + j;
@@ -258,6 +275,16 @@ void fdt_fixup_qportals(void *blob)
if (err < 0)
goto err;
 #endif
+#else
+   if (smmu_ph >= 0) {
+   u32 icids[3];
+
+   icids[0] = qp_info[i].icid;
+   icids[1] = qp_info[i].dicid;
+   icids[2] = qp_info[i].ficid;
+
+ 

[U-Boot] [PATCH v2 3/7] misc: fsl_portals: setup QMAN_BAR{E} also on ARM platforms

2018-07-03 Thread Laurentiu Tudor
QMAN_BAR{E} register setup was disabled on ARM platforms,
however the register does need to be set. Add code that
sets it up on ARMs.

Signed-off-by: Laurentiu Tudor 
---
 drivers/misc/fsl_portals.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/fsl_portals.c b/drivers/misc/fsl_portals.c
index 7c22b8d209..5b1233740b 100644
--- a/drivers/misc/fsl_portals.c
+++ b/drivers/misc/fsl_portals.c
@@ -24,14 +24,19 @@ void setup_qbman_portals(void)
CONFIG_SYS_BMAN_SWP_ISDR_REG;
void __iomem *qpaddr = (void *)CONFIG_SYS_QMAN_CINH_BASE +
CONFIG_SYS_QMAN_SWP_ISDR_REG;
-#ifdef CONFIG_PPC
struct ccsr_qman *qman = (void *)CONFIG_SYS_FSL_QMAN_ADDR;
 
/* Set the Qman initiator BAR to match the LAW (for DQRR stashing) */
+#ifdef CONFIG_PPC32
 #ifdef CONFIG_PHYS_64BIT
out_be32(>qcsp_bare, (u32)(CONFIG_SYS_QMAN_MEM_PHYS >> 32));
 #endif
out_be32(>qcsp_bar, (u32)CONFIG_SYS_QMAN_MEM_PHYS);
+#else
+#ifdef CONFIG_PHYS_64BIT
+   out_be32(>qcsp_bare, (u32)(CONFIG_SYS_QMAN_MEM_BASE >> 32));
+#endif
+   out_be32(>qcsp_bar, (u32)CONFIG_SYS_QMAN_MEM_BASE);
 #endif
 #ifdef CONFIG_FSL_CORENET
int i;
-- 
2.17.1

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


[U-Boot] [PATCH v2 5/7] armv8: ls1046a: initial icid setup support

2018-07-03 Thread Laurentiu Tudor
Add infrastructure for ICID setup and device tree
fixup on ARM platforms. This include basic ICID setup
for several devices.

Signed-off-by: Laurentiu Tudor 
---
 arch/arm/cpu/armv8/fsl-layerscape/Makefile|   1 +
 arch/arm/cpu/armv8/fsl-layerscape/icid.c  | 111 ++
 .../arm/cpu/armv8/fsl-layerscape/ls1046_ids.c |  29 +
 arch/arm/cpu/armv8/fsl-layerscape/soc.c   |   3 +
 .../asm/arch-fsl-layerscape/fsl_icid.h|  80 +
 board/freescale/ls1046aqds/ls1046aqds.c   |   2 +
 board/freescale/ls1046ardb/ls1046ardb.c   |   3 +
 7 files changed, 229 insertions(+)
 create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/icid.c
 create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
 create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Makefile 
b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
index 1e9e4680fe..5d6f68aad6 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/Makefile
+++ b/arch/arm/cpu/armv8/fsl-layerscape/Makefile
@@ -37,6 +37,7 @@ endif
 
 ifneq ($(CONFIG_ARCH_LS1046A),)
 obj-$(CONFIG_SYS_HAS_SERDES) += ls1046a_serdes.o
+obj-y += icid.o ls1046_ids.o
 endif
 
 ifneq ($(CONFIG_ARCH_LS1088A),)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/icid.c 
b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
new file mode 100644
index 00..8694bd6fa1
--- /dev/null
+++ b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2018 NXP
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+static void set_icid(struct icid_id_table *tbl, int size)
+{
+   int i;
+
+   for (i = 0; i < size; i++)
+   out_be32((u32 *)(tbl[i].reg_addr), tbl[i].reg);
+}
+
+void set_icids(void)
+{
+   /* setup general icid offsets */
+   set_icid(icid_tbl, icid_tbl_sz);
+}
+
+int fdt_set_iommu_prop(void *blob, int off, int smmu_ph, u32 *ids, int num_ids)
+{
+   int i, ret;
+   u32 prop[8];
+
+   for (i = 0; i < num_ids; i++) {
+   prop[i * 2] = cpu_to_fdt32(smmu_ph);
+   prop[i * 2 + 1] = cpu_to_fdt32(ids[i]);
+   }
+   ret = fdt_setprop(blob, off, "iommus",
+ prop, sizeof(u32) * num_ids * 2);
+   if (ret > 0) {
+   printf("WARNING unable to set iommus: %s\n", fdt_strerror(off));
+   return off;
+   }
+   ret = fdt_setprop_empty(blob, off, "dma-coherent");
+   if (ret > 0) {
+   printf("WARNING unable to set dma-coherent: %s\n",
+  fdt_strerror(off));
+   return off;
+   }
+
+   return 0;
+}
+
+int fdt_fixup_icid_tbl(void *blob, int smmu_ph,
+  struct icid_id_table *tbl, int size)
+{
+   int i, err, off;
+
+   for (i = 0; i < size; i++) {
+   if (!tbl[i].compat)
+   continue;
+
+   off = fdt_node_offset_by_compat_reg(blob,
+   tbl[i].compat,
+   tbl[i].compat_addr);
+   if (off > 0) {
+   err = fdt_set_iommu_prop(blob, off, smmu_ph,
+[i].id, 1);
+   if (err)
+   return err;
+   } else {
+   printf("WARNING could not find node %s: %s.\n",
+  tbl[i].compat, fdt_strerror(off));
+   }
+   }
+
+   return 0;
+}
+
+int fdt_get_smmu_phandle(void *blob)
+{
+   int noff, smmu_ph;
+
+   noff = fdt_node_offset_by_compatible(blob, -1, "arm,mmu-500");
+   if (noff < 0) {
+   printf("WARNING failed to get smmu node: %s\n",
+  fdt_strerror(noff));
+   return noff;
+   }
+
+   smmu_ph = fdt_get_phandle(blob, noff);
+   if (!smmu_ph) {
+   smmu_ph = fdt_create_phandle(blob, noff);
+   if (!smmu_ph) {
+   printf("WARNING failed to get smmu phandle\n");
+   return -1;
+   }
+   }
+
+   return smmu_ph;
+}
+
+void fdt_fixup_icid(void *blob)
+{
+   int smmu_ph;
+
+   smmu_ph = fdt_get_smmu_phandle(blob);
+   if (smmu_ph < 0)
+   return;
+
+   fdt_fixup_icid_tbl(blob, smmu_ph, icid_tbl, icid_tbl_sz);
+}
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c 
b/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
new file mode 100644
index 00..d9e03fcb89
--- /dev/null
+++ b/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2018 NXP
+ */
+
+#include 
+#include 
+#include 
+
+struct icid_id_table icid_tbl[] = {
+#ifdef CONFIG_SYS_DPAA_QBMAN
+   SET_QMAN_ICID(FSL_DPAA1_STREAM_ID_START),
+   SET_BMAN_ICID(FSL_DPAA1_STREAM_ID_START + 1),
+#endif
+
+   

[U-Boot] [PATCH v2 7/7] armv8: ls1046a: setup fman ports ICIDs and device tree

2018-07-03 Thread Laurentiu Tudor
Add support for ICID setting of fman ports and
the required device tree fixups.

Signed-off-by: Laurentiu Tudor 
---
 arch/arm/cpu/armv8/fsl-layerscape/icid.c  | 82 +++
 .../arm/cpu/armv8/fsl-layerscape/ls1046_ids.c | 30 +++
 .../asm/arch-fsl-layerscape/fsl_icid.h| 10 +++
 3 files changed, 122 insertions(+)

diff --git a/arch/arm/cpu/armv8/fsl-layerscape/icid.c 
b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
index 8694bd6fa1..9502f83ac8 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/icid.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static void set_icid(struct icid_id_table *tbl, int size)
 {
@@ -19,10 +20,27 @@ static void set_icid(struct icid_id_table *tbl, int size)
out_be32((u32 *)(tbl[i].reg_addr), tbl[i].reg);
 }
 
+#ifdef CONFIG_SYS_FMAN_V3
+void set_fman_icids(struct fman_icid_id_table *tbl, int size)
+{
+   int i;
+   ccsr_fman_t *fm = (void *)CONFIG_SYS_FSL_FM1_ADDR;
+
+   for (i = 0; i < size; i++) {
+   out_be32(>fm_bmi_common.fmbm_ppid[tbl[i].port_id - 1],
+tbl[i].icid);
+   }
+}
+#endif
+
 void set_icids(void)
 {
/* setup general icid offsets */
set_icid(icid_tbl, icid_tbl_sz);
+
+#ifdef CONFIG_SYS_FMAN_V3
+   set_fman_icids(fman_icid_tbl, fman_icid_tbl_sz);
+#endif
 }
 
 int fdt_set_iommu_prop(void *blob, int off, int smmu_ph, u32 *ids, int num_ids)
@@ -76,6 +94,66 @@ int fdt_fixup_icid_tbl(void *blob, int smmu_ph,
return 0;
 }
 
+#ifdef CONFIG_SYS_FMAN_V3
+int get_fman_port_icid(int port_id, struct fman_icid_id_table *tbl,
+  const int size)
+{
+   int i;
+
+   for (i = 0; i < size; i++) {
+   if (tbl[i].port_id == port_id)
+   return tbl[i].icid;
+   }
+
+   return -1;
+}
+
+void fdt_fixup_fman_port_icid_by_compat(void *blob, int smmu_ph,
+   const char *compat)
+{
+   int noff, len, icid;
+   const u32 *prop;
+
+   noff = fdt_node_offset_by_compatible(blob, -1, compat);
+   while (noff > 0) {
+   prop = fdt_getprop(blob, noff, "cell-index", );
+   if (!prop) {
+   printf("WARNING missing cell-index for fman port\n");
+   continue;
+   }
+   if (len != 4) {
+   printf("WARNING bad cell-index size for fman port\n");
+   continue;
+   }
+
+   icid = get_fman_port_icid(fdt32_to_cpu(*prop),
+ fman_icid_tbl, fman_icid_tbl_sz);
+   if (icid < 0) {
+   printf("WARNING unknown ICID for fman port %d\n",
+  *prop);
+   continue;
+   }
+
+   fdt_set_iommu_prop(blob, noff, smmu_ph, (u32 *), 1);
+
+   noff = fdt_node_offset_by_compatible(blob, noff, compat);
+   }
+}
+
+void fdt_fixup_fman_icids(void *blob, int smmu_ph)
+{
+   static const char * const compats[] = {
+   "fsl,fman-v3-port-oh",
+   "fsl,fman-v3-port-rx",
+   "fsl,fman-v3-port-tx",
+   };
+   int i;
+
+   for (i = 0; i < ARRAY_SIZE(compats); i++)
+   fdt_fixup_fman_port_icid_by_compat(blob, smmu_ph, compats[i]);
+}
+#endif
+
 int fdt_get_smmu_phandle(void *blob)
 {
int noff, smmu_ph;
@@ -108,4 +186,8 @@ void fdt_fixup_icid(void *blob)
return;
 
fdt_fixup_icid_tbl(blob, smmu_ph, icid_tbl, icid_tbl_sz);
+
+#ifdef CONFIG_SYS_FMAN_V3
+   fdt_fixup_fman_icids(blob, smmu_ph);
+#endif
 }
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c 
b/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
index 62bad3ab9c..1592029a60 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
@@ -43,3 +43,33 @@ struct icid_id_table icid_tbl[] = {
 };
 
 int icid_tbl_sz = ARRAY_SIZE(icid_tbl);
+
+#ifdef CONFIG_SYS_DPAA_FMAN
+struct fman_icid_id_table fman_icid_tbl[] = {
+   /* port id, icid */
+   SET_FMAN_ICID_ENTRY(0x02, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x03, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x04, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x05, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x06, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x07, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x08, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x09, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x0a, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x0b, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x0c, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x0d, FSL_DPAA1_STREAM_ID_END),
+   SET_FMAN_ICID_ENTRY(0x28, FSL_DPAA1_STREAM_ID_END),
+   

[U-Boot] [PATCH v2 4/7] armv8: fsl-layerscape: add missing debug stream ID

2018-07-03 Thread Laurentiu Tudor
Add a define with a value for the missing debug stream ID.

Signed-off-by: Laurentiu Tudor 
---
 arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch2.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch2.h 
b/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch2.h
index 61c6e533c6..1b02d484d9 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch2.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/stream_id_lsch2.h
@@ -50,6 +50,7 @@
 #define FSL_QDMA_STREAM_ID 7
 #define FSL_EDMA_STREAM_ID 8
 #define FSL_ETR_STREAM_ID  9
+#define FSL_DEBUG_STREAM_ID10
 
 /* PCI - programmed in PEXn_LUT */
 #define FSL_PEX_STREAM_ID_START11
-- 
2.17.1

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


[U-Boot] [PATCH v2 2/7] armv8: ls1046a: advertise QMan v3 in configuration

2018-07-03 Thread Laurentiu Tudor
The QMan IP block in this SoC is version 3.2 so advertise
this in the SoC configuration header.

Signed-off-by: Laurentiu Tudor 
---
 arch/arm/include/asm/arch-fsl-layerscape/config.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/include/asm/arch-fsl-layerscape/config.h 
b/arch/arm/include/asm/arch-fsl-layerscape/config.h
index 23faffd9fc..8a05148136 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/config.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/config.h
@@ -257,6 +257,7 @@
 
 #elif defined(CONFIG_ARCH_LS1046A)
 #define CONFIG_SYS_FMAN_V3
+#define CONFIG_SYS_FSL_QMAN_V3
 #define CONFIG_SYS_NUM_FMAN1
 #define CONFIG_SYS_NUM_FM1_DTSEC   8
 #define CONFIG_SYS_NUM_FM1_10GEC   2
-- 
2.17.1

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


[U-Boot] [PATCH v2 1/7] armv8: fsl-layerscape: add missing register blocks base address defines

2018-07-03 Thread Laurentiu Tudor
Add defines for the edma and qdma register block base addresses.

Signed-off-by: Laurentiu Tudor 
---
 arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h 
b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
index 5b4767e0fe..644a16dd30 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/immap_lsch2.h
@@ -88,8 +88,12 @@
 
 #define LPUART_BASE(CONFIG_SYS_IMMR + 0x0195)
 
+#define EDMA_BASE_ADDR (CONFIG_SYS_IMMR + 0x01c0)
+
 #define AHCI_BASE_ADDR (CONFIG_SYS_IMMR + 0x0220)
 
+#define QDMA_BASE_ADDR (CONFIG_SYS_IMMR + 0x0738)
+
 #define CONFIG_SYS_PCIE1_PHYS_ADDR 0x40ULL
 #define CONFIG_SYS_PCIE2_PHYS_ADDR 0x48ULL
 #define CONFIG_SYS_PCIE3_PHYS_ADDR 0x50ULL
-- 
2.17.1

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


[U-Boot] [PATCH v2 0/7] LS1046A SMMU enabling patches

2018-07-03 Thread Laurentiu Tudor
This patch series adds the required devices setup and device tree
fixups for SMMU enablement on LS1046A chips. The approach taken tries
to mimic the implementation of PAMU LIODN setup on booke powerpc.

First 4 patches contain some fixes and add some missing bits & pieces.
Last 3 patches add the actual infrastructure for ICID setup, qman
portal and fman ICID configuration.

Changes in v2:
 - drop CONFIG_SYS_ prefix from newly introduced defines in patch [1/7]

Laurentiu Tudor (7):
  armv8: fsl-layerscape: add missing register blocks base address
defines
  armv8: ls1046a: advertise QMan v3 in configuration
  misc: fsl_portals: setup QMAN_BAR{E} also on ARM platforms
  armv8: fsl-layerscape: add missing debug stream ID
  armv8: ls1046a: initial icid setup support
  armv8: ls1046a: add icid setup for qman portals
  armv8: ls1046a: setup fman ports ICIDs and device tree

 arch/arm/cpu/armv8/fsl-layerscape/Makefile|   1 +
 arch/arm/cpu/armv8/fsl-layerscape/icid.c  | 193 ++
 .../arm/cpu/armv8/fsl-layerscape/ls1046_ids.c |  75 +++
 arch/arm/cpu/armv8/fsl-layerscape/soc.c   |   3 +
 .../include/asm/arch-fsl-layerscape/config.h  |   1 +
 .../asm/arch-fsl-layerscape/fsl_icid.h|  90 
 .../asm/arch-fsl-layerscape/fsl_portals.h |  23 +++
 .../asm/arch-fsl-layerscape/immap_lsch2.h |   4 +
 .../asm/arch-fsl-layerscape/stream_id_lsch2.h |   1 +
 board/freescale/ls1046aqds/ls1046aqds.c   |   2 +
 board/freescale/ls1046ardb/ls1046ardb.c   |   3 +
 drivers/misc/fsl_portals.c|  50 -
 12 files changed, 437 insertions(+), 9 deletions(-)
 create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/icid.c
 create mode 100644 arch/arm/cpu/armv8/fsl-layerscape/ls1046_ids.c
 create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
 create mode 100644 arch/arm/include/asm/arch-fsl-layerscape/fsl_portals.h

-- 
2.17.1

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


Re: [U-Boot] [PATCH 1/2] arm: timer: factor out FSL arch timer erratum workaround

2018-07-03 Thread Guillaume Gardet



Le 03/07/2018 à 01:08, Andreas Färber a écrit :

Am 02.07.2018 um 10:01 schrieb Jagan Teki:

On Wed, Jun 27, 2018 at 6:12 AM, Andre Przywara  wrote:

At the moment we have the workaround for the Freescale arch timer
erratum A-008585 merged into the generic timer_read_counter() routine.
Split those two up, so that we can add other errata workaround more
easily. Also add an explaining comment on the way.

Signed-off-by: Andre Przywara 
---

Applied both patches, to u-boot-sunxi/master

Tested both on top of v2018.07-rc2, fixes the boot for me.


Tested both on top of v2018.07-rc3, and it fixes the boot.

Thanks.

Guillaume



Thanks,
Andreas



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


Re: [U-Boot] [PATCH 4/4] x86: Switch to use DM sysreset driver

2018-07-03 Thread Wolfgang Denk
Dear Andy,

In message <18255ae55e4faa5733fb8e93dc8ebbc1559a3694.ca...@linux.intel.com> you 
wrote:
...
> > This converts all x86 boards over to DM sysreset.
> 
> > -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> > argv[])
> > -{
> > -   printf("resetting ...\n");
> > -
> > -   /* wait 50 ms */
> > -   udelay(5);

I am willing to bet that the delay here is a dirty surrogate for
flushing/closing the standard output channel, i. e. we wait until
the characters have actually been set over the serial console.

Don't we have a way to close() the device in DM (remove ?) ?


> First of all, in some cases would be good to have at least a debug
> message that we got into do_reset().

This should not only be a debug message, but a standard message;
actually even something that goes to STDERR.

> Second, I didn't test if udelay() + disable_interrupts() make any
> difference. So, I would leave them for now.

See above... I think the delay should be replaced by the proper way
to close the device in DM (remove?).


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
But the only way of discovering the limits  of  the  possible  is  to
venture a little way past them into the impossible.
 - _Profiles of the Future_ (1962; rev. 1973)
  ``Hazards of Prophecy: The Failure of Imagination''
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 4/4] x86: Switch to use DM sysreset driver

2018-07-03 Thread Andy Shevchenko
On Tue, 2018-07-03 at 02:48 -0700, Bin Meng wrote:
> This converts all x86 boards over to DM sysreset.

> -int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[])
> -{
> - printf("resetting ...\n");
> -
> - /* wait 50 ms */
> - udelay(5);
> - disable_interrupts();
> - reset_cpu(0);

> -}

> --- a/arch/x86/cpu/tangier/tangier.c
> +++ b/arch/x86/cpu/tangier/tangier.c
> @@ -25,7 +25,15 @@ int print_cpuinfo(void)
>   return default_print_cpuinfo();
>  }
>  
> +/* TODO: convert to DM sysreset */
>  void reset_cpu(ulong addr)
>  {
>   scu_ipc_simple_command(IPCMSG_COLD_RESET, 0);
>  }
> +
> +int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const
> argv[])
> +{
> + reset_cpu(0);
> +
> + return 0;

This is not equivalent to the above.

First of all, in some cases would be good to have at least a debug
message that we got into do_reset().

Second, I didn't test if udelay() + disable_interrupts() make any
difference. So, I would leave them for now.

> +}

> diff --git a/arch/x86/dts/edison.dts b/arch/x86/dts/edison.dts
> index 9033532..a1d3c90 100644
> --- a/arch/x86/dts/edison.dts
> +++ b/arch/x86/dts/edison.dts
> @@ -9,6 +9,7 @@
>  #include 
>  
>  /include/ "skeleton.dtsi"

> +/include/ "reset.dtsi"

If i read this right we are not using generic reset sequence. 
Why do we include this here?

>  /include/ "rtc.dtsi"
>  /include/ "tsc_timer.dtsi"

> --- a/configs/edison_defconfig
> +++ b/configs/edison_defconfig

> +# CONFIG_SYSRESET is not set

-- 
Andy Shevchenko 
Intel Finland Oy
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates

2018-07-03 Thread Marek Vasut
On 07/03/2018 12:08 PM, Andre Przywara wrote:
> Hi,

Hi,

> On 03/07/18 09:24, Marek Vasut wrote:
>> On 07/03/2018 01:44 AM, Andre Przywara wrote:
>>> The USB host controllers on the H3, H5 and A64 have the oddity of
>>> sharing some clock and reset gates, so both the OHCI and EHCI bits have
>>> to be enabled to make only one of them working. We take care of this, and
>>> initialisation works fine (due to setting already set bits).
>>> However on shutdown we turn the clocks and reset gates off already when
>>> deregistering one controller, so the other one is no longer functional.
>>> In the result U-Boot complains just before launching the kernel and
>>> then hangs.
>>> Fix this by not turning off the clocks and resets on the OHCI side, so
>>> that the EHCI controller has a chance to properly shut down.
>>> This still isn't perfect, but at least prevents the hang.
>>>
>>> Signed-off-by: Andre Przywara 
>>
>> What about adding some enable/disable counter to those clock somehow and
>> then turning them off when the counter reaches zero ?
> 
> Yes, that's what I meant with "proper ref-counting" below. The problem
> is that this would need to go across the two files ([oe]hci-sunxi.c),
> and be per bit. So it wouldn't be too pretty or easy.

Some temporary enable/disable_clock() function shared by those two
drivers might be doable in this short timeframe, no ?

> But the clocks and resets should be handled by a proper DM driver, which
> is just around the corner (check the link to this branch on my github
> below).

Good. Last time I asked why there is no proper clock driver, I got quite
a lot of flak in that I am asking for too much.

> So I don't like to spend too much time on it. We don't need USB
> for the SPL, so we can actually remove this code once we have DM_CLK.
> For now I just need to fix that hang to unblock the DT updates.

I want something which stops the clock properly for this release. So if
you can come up with something like that, great. And then the clock
driver would be awesome for the next release or so.

[...]

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates

2018-07-03 Thread Marek Vasut
On 07/03/2018 12:11 PM, Andre Przywara wrote:
> Hi,
> 
> On 03/07/18 07:19, Jagan Teki wrote:
>> On Tue, Jul 3, 2018 at 5:14 AM, Andre Przywara  
>> wrote:
>>> The USB host controllers on the H3, H5 and A64 have the oddity of
>>> sharing some clock and reset gates, so both the OHCI and EHCI bits have
>>> to be enabled to make only one of them working. We take care of this, and
>>> initialisation works fine (due to setting already set bits).
>>> However on shutdown we turn the clocks and reset gates off already when
>>> deregistering one controller, so the other one is no longer functional.
>>> In the result U-Boot complains just before launching the kernel and
>>> then hangs.
>>> Fix this by not turning off the clocks and resets on the OHCI side, so
>>> that the EHCI controller has a chance to properly shut down.
>>> This still isn't perfect, but at least prevents the hang.
>>>
>>> Signed-off-by: Andre Przywara 
>>> ---
>>> Hi,
>>>
>>> commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
>>> introduced the proper reset and clock gates for the A64. While the patch
>>> itself is correct, it broke Linux as soon as one actually enables USB0 in
>>> the DT (in the moment we keep this "disabled" in U-Boot's DT version).
>>>
>>> I understand that this patch here is somewhat of a hack, but the proper
>>> ref-counting is not easy to implement between the separate EHCI and OHCI
>>> drivers. Those two files are doomed anyway, since driver model clocks
>>> and reset drivers are already on the horizon:
>>> https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
>>> So lets fix this up for now so that we can use the Linux kernel DTs with
>>> U-Boot itself.
>>>
>>> Cheers,
>>> Andre.
>>>
>>>  drivers/usb/host/ohci-sunxi.c | 16 
>>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c
>>> index 0ddbdbe460..42ffb6cbcb 100644
>>> --- a/drivers/usb/host/ohci-sunxi.c
>>> +++ b/drivers/usb/host/ohci-sunxi.c
>>> @@ -128,10 +128,18 @@ static int ohci_usb_remove(struct udevice *dev)
>>> if (ret)
>>> return ret;
>>>
>>> -   if (priv->cfg->has_reset)
>>> -   clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
>>> -   clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>>> -   clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
>>> +   /*
>>> +* For those SoCs that share the clock and reset gates with the EHCI
>>> +* controller, we should not turn them off here, to prevent the
>>> +* other one hanging (when the EHCI driver tries to shut itself 
>>> down).
>>
>> I've tried this "not to disable usb_clk_cfg" before sending "arm64:
>> allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi" patch.
>> But, I thought improper shutdown may effect other issue may be with
>> Linux.
> 
> That's probably true. Did you try to prevent shutdown at all? Or just
> for *one* of OHCI and EHCI?
> My patch does it only for OHCI, so the EHCI still does the shutdown.
> This somewhat relies on some shutdown ordering, though.
> 
>> Initially I found an issue of disabling OHCI1 gate clock during OHCI0
>> shutdown. ohci driver is trying to clear BIT(16) for OHCI0, but it
>> also clears BIT(17), which is for OHCI1. By adding proper shutdown
>> support for musb and other things I came to a situation where all
>> shutdown happen properly. But ofcourse I still see hang.
> 
> I believe with all those shared gates and dependencies we would need
> refcounting per *bit*, for the gates, USB clocks and resets. As
> mentioned, I am not sure it's worth the effort, if a DM clock driver
> would solve this much more elegantly, and in one place.
> I just need this fix here to make the kernel DTs work, so that I can
> send those out.
> After this I would revive the DM_CLK work.

Yes, the clock driver would solve this issue, since it would be able to
do some sort of reference counting for the shared clock.

-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates

2018-07-03 Thread Jagan Teki
On Tue, Jul 3, 2018 at 3:41 PM, Andre Przywara  wrote:
> Hi,
>
> On 03/07/18 07:19, Jagan Teki wrote:
>> On Tue, Jul 3, 2018 at 5:14 AM, Andre Przywara  
>> wrote:
>>> The USB host controllers on the H3, H5 and A64 have the oddity of
>>> sharing some clock and reset gates, so both the OHCI and EHCI bits have
>>> to be enabled to make only one of them working. We take care of this, and
>>> initialisation works fine (due to setting already set bits).
>>> However on shutdown we turn the clocks and reset gates off already when
>>> deregistering one controller, so the other one is no longer functional.
>>> In the result U-Boot complains just before launching the kernel and
>>> then hangs.
>>> Fix this by not turning off the clocks and resets on the OHCI side, so
>>> that the EHCI controller has a chance to properly shut down.
>>> This still isn't perfect, but at least prevents the hang.
>>>
>>> Signed-off-by: Andre Przywara 
>>> ---
>>> Hi,
>>>
>>> commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
>>> introduced the proper reset and clock gates for the A64. While the patch
>>> itself is correct, it broke Linux as soon as one actually enables USB0 in
>>> the DT (in the moment we keep this "disabled" in U-Boot's DT version).
>>>
>>> I understand that this patch here is somewhat of a hack, but the proper
>>> ref-counting is not easy to implement between the separate EHCI and OHCI
>>> drivers. Those two files are doomed anyway, since driver model clocks
>>> and reset drivers are already on the horizon:
>>> https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
>>> So lets fix this up for now so that we can use the Linux kernel DTs with
>>> U-Boot itself.
>>>
>>> Cheers,
>>> Andre.
>>>
>>>  drivers/usb/host/ohci-sunxi.c | 16 
>>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c
>>> index 0ddbdbe460..42ffb6cbcb 100644
>>> --- a/drivers/usb/host/ohci-sunxi.c
>>> +++ b/drivers/usb/host/ohci-sunxi.c
>>> @@ -128,10 +128,18 @@ static int ohci_usb_remove(struct udevice *dev)
>>> if (ret)
>>> return ret;
>>>
>>> -   if (priv->cfg->has_reset)
>>> -   clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
>>> -   clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>>> -   clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
>>> +   /*
>>> +* For those SoCs that share the clock and reset gates with the EHCI
>>> +* controller, we should not turn them off here, to prevent the
>>> +* other one hanging (when the EHCI driver tries to shut itself 
>>> down).
>>
>> I've tried this "not to disable usb_clk_cfg" before sending "arm64:
>> allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi" patch.
>> But, I thought improper shutdown may effect other issue may be with
>> Linux.
>
> That's probably true. Did you try to prevent shutdown at all? Or just
> for *one* of OHCI and EHCI?
> My patch does it only for OHCI, so the EHCI still does the shutdown.
> This somewhat relies on some shutdown ordering, though.

Yes I understand it, but here I think we can turn-off usb_clk_cfg no
need to turn-off gates as well.

>
>> Initially I found an issue of disabling OHCI1 gate clock during OHCI0
>> shutdown. ohci driver is trying to clear BIT(16) for OHCI0, but it
>> also clears BIT(17), which is for OHCI1. By adding proper shutdown
>> support for musb and other things I came to a situation where all
>> shutdown happen properly. But ofcourse I still see hang.
>
> I believe with all those shared gates and dependencies we would need
> refcounting per *bit*, for the gates, USB clocks and resets. As
> mentioned, I am not sure it's worth the effort, if a DM clock driver
> would solve this much more elegantly, and in one place.
> I just need this fix here to make the kernel DTs work, so that I can
> send those out.
> After this I would revive the DM_CLK work.

This is what my next plan would be. Once we have dm_clk ready we can
use ehci/ohci generic drivers directly since we have PHY handling
driver.

thanks,
Jagan.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [ANN] U-Boot v2018.07-rc3 released

2018-07-03 Thread Heiko Schocher

Hello Tom,

Am 03.07.2018 um 05:25 schrieb Tom Rini:

Hey all,

It's a week until release and I've put out v2018.07-rc3.  I think things
are looking good for release, but I do want to see what we can / can't


Yes, my tbot tests last night looking good (Ok, just the last 2 boards
got the rc3 commit ...)

http://xeidos.ddns.net/tests/test_db_auslesen.php


do about the sunxi arm64 USB issue that just cropped up.

Thanks all!


bye,
Heiko
--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates

2018-07-03 Thread Andre Przywara
Hi,

On 03/07/18 07:19, Jagan Teki wrote:
> On Tue, Jul 3, 2018 at 5:14 AM, Andre Przywara  wrote:
>> The USB host controllers on the H3, H5 and A64 have the oddity of
>> sharing some clock and reset gates, so both the OHCI and EHCI bits have
>> to be enabled to make only one of them working. We take care of this, and
>> initialisation works fine (due to setting already set bits).
>> However on shutdown we turn the clocks and reset gates off already when
>> deregistering one controller, so the other one is no longer functional.
>> In the result U-Boot complains just before launching the kernel and
>> then hangs.
>> Fix this by not turning off the clocks and resets on the OHCI side, so
>> that the EHCI controller has a chance to properly shut down.
>> This still isn't perfect, but at least prevents the hang.
>>
>> Signed-off-by: Andre Przywara 
>> ---
>> Hi,
>>
>> commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
>> introduced the proper reset and clock gates for the A64. While the patch
>> itself is correct, it broke Linux as soon as one actually enables USB0 in
>> the DT (in the moment we keep this "disabled" in U-Boot's DT version).
>>
>> I understand that this patch here is somewhat of a hack, but the proper
>> ref-counting is not easy to implement between the separate EHCI and OHCI
>> drivers. Those two files are doomed anyway, since driver model clocks
>> and reset drivers are already on the horizon:
>> https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
>> So lets fix this up for now so that we can use the Linux kernel DTs with
>> U-Boot itself.
>>
>> Cheers,
>> Andre.
>>
>>  drivers/usb/host/ohci-sunxi.c | 16 
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c
>> index 0ddbdbe460..42ffb6cbcb 100644
>> --- a/drivers/usb/host/ohci-sunxi.c
>> +++ b/drivers/usb/host/ohci-sunxi.c
>> @@ -128,10 +128,18 @@ static int ohci_usb_remove(struct udevice *dev)
>> if (ret)
>> return ret;
>>
>> -   if (priv->cfg->has_reset)
>> -   clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
>> -   clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>> -   clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
>> +   /*
>> +* For those SoCs that share the clock and reset gates with the EHCI
>> +* controller, we should not turn them off here, to prevent the
>> +* other one hanging (when the EHCI driver tries to shut itself 
>> down).
> 
> I've tried this "not to disable usb_clk_cfg" before sending "arm64:
> allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi" patch.
> But, I thought improper shutdown may effect other issue may be with
> Linux.

That's probably true. Did you try to prevent shutdown at all? Or just
for *one* of OHCI and EHCI?
My patch does it only for OHCI, so the EHCI still does the shutdown.
This somewhat relies on some shutdown ordering, though.

> Initially I found an issue of disabling OHCI1 gate clock during OHCI0
> shutdown. ohci driver is trying to clear BIT(16) for OHCI0, but it
> also clears BIT(17), which is for OHCI1. By adding proper shutdown
> support for musb and other things I came to a situation where all
> shutdown happen properly. But ofcourse I still see hang.

I believe with all those shared gates and dependencies we would need
refcounting per *bit*, for the gates, USB clocks and resets. As
mentioned, I am not sure it's worth the effort, if a DM clock driver
would solve this much more elegantly, and in one place.
I just need this fix here to make the kernel DTs work, so that I can
send those out.
After this I would revive the DM_CLK work.

Cheers,
Andre.

> => usb reset
> resetting USB...
> 
> PHY0: mask = 0x101, usb_clk_cfg = 0x30202
> sunxi_musb_exit: ahb_gate0  = 0x33004540, reset0_cfg = 0x33004540
> EHCI failed to shut down host controller.
> ehci_usb_remove: ahb_gate0  = 0x32004540, reset0_cfg = 0x32004540
> ohci_usb_remove: ahb_gate0  = 0x22004540, reset0_cfg = 0x22004540
> ohci_usb_remove: mask = 0x1, usb_clk_cfg = 0x20202
> 
> PHY1: mask = 0x202, usb_clk_cfg = 0x0
> ehci_usb_remove: ahb_gate0  = 0x20004540, reset0_cfg = 0x20004540
> << hang  >>
> 
> Do you think this is still an issue with clock? considering proper
> shutdown sequence happened above.
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates

2018-07-03 Thread Andre Przywara
Hi,

On 03/07/18 09:24, Marek Vasut wrote:
> On 07/03/2018 01:44 AM, Andre Przywara wrote:
>> The USB host controllers on the H3, H5 and A64 have the oddity of
>> sharing some clock and reset gates, so both the OHCI and EHCI bits have
>> to be enabled to make only one of them working. We take care of this, and
>> initialisation works fine (due to setting already set bits).
>> However on shutdown we turn the clocks and reset gates off already when
>> deregistering one controller, so the other one is no longer functional.
>> In the result U-Boot complains just before launching the kernel and
>> then hangs.
>> Fix this by not turning off the clocks and resets on the OHCI side, so
>> that the EHCI controller has a chance to properly shut down.
>> This still isn't perfect, but at least prevents the hang.
>>
>> Signed-off-by: Andre Przywara 
> 
> What about adding some enable/disable counter to those clock somehow and
> then turning them off when the counter reaches zero ?

Yes, that's what I meant with "proper ref-counting" below. The problem
is that this would need to go across the two files ([oe]hci-sunxi.c),
and be per bit. So it wouldn't be too pretty or easy.
But the clocks and resets should be handled by a proper DM driver, which
is just around the corner (check the link to this branch on my github
below). So I don't like to spend too much time on it. We don't need USB
for the SPL, so we can actually remove this code once we have DM_CLK.
For now I just need to fix that hang to unblock the DT updates.

Cheers,
Andre.

>> ---
>> Hi,
>>
>> commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
>> introduced the proper reset and clock gates for the A64. While the patch
>> itself is correct, it broke Linux as soon as one actually enables USB0 in
>> the DT (in the moment we keep this "disabled" in U-Boot's DT version).
>>
>> I understand that this patch here is somewhat of a hack, but the proper
>> ref-counting is not easy to implement between the separate EHCI and OHCI
>> drivers. Those two files are doomed anyway, since driver model clocks
>> and reset drivers are already on the horizon:
>> https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
>> So lets fix this up for now so that we can use the Linux kernel DTs with
>> U-Boot itself.
>>
>> Cheers,
>> Andre.
>>
>>  drivers/usb/host/ohci-sunxi.c | 16 
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c
>> index 0ddbdbe460..42ffb6cbcb 100644
>> --- a/drivers/usb/host/ohci-sunxi.c
>> +++ b/drivers/usb/host/ohci-sunxi.c
>> @@ -128,10 +128,18 @@ static int ohci_usb_remove(struct udevice *dev)
>>  if (ret)
>>  return ret;
>>  
>> -if (priv->cfg->has_reset)
>> -clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
>> -clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>> -clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
>> +/*
>> + * For those SoCs that share the clock and reset gates with the EHCI
>> + * controller, we should not turn them off here, to prevent the
>> + * other one hanging (when the EHCI driver tries to shut itself down).
>> + */
>> +if (!priv->cfg->extra_ahb_gate_mask) {
>> +if (priv->cfg->has_reset)
>> +clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
>> +clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
>> +}
>> +if (!priv->cfg->extra_usb_gate_mask)
>> +clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>>  
>>  return 0;
>>  }
>>
> 
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates

2018-07-03 Thread Jagan Teki
On Tue, Jul 3, 2018 at 1:54 PM, Marek Vasut  wrote:
> On 07/03/2018 01:44 AM, Andre Przywara wrote:
>> The USB host controllers on the H3, H5 and A64 have the oddity of
>> sharing some clock and reset gates, so both the OHCI and EHCI bits have
>> to be enabled to make only one of them working. We take care of this, and
>> initialisation works fine (due to setting already set bits).
>> However on shutdown we turn the clocks and reset gates off already when
>> deregistering one controller, so the other one is no longer functional.
>> In the result U-Boot complains just before launching the kernel and
>> then hangs.
>> Fix this by not turning off the clocks and resets on the OHCI side, so
>> that the EHCI controller has a chance to properly shut down.
>> This still isn't perfect, but at least prevents the hang.
>>
>> Signed-off-by: Andre Przywara 
>
> What about adding some enable/disable counter to those clock somehow and
> then turning them off when the counter reaches zero ?

Like disable all controller clocks, at the end of last controller
shutdown? right now it is doing opposite for A64. Disabling all clocks
at the end work for me, with preserving previous mask bit.

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


[U-Boot] [PATCH 4/4] x86: Switch to use DM sysreset driver

2018-07-03 Thread Bin Meng
This converts all x86 boards over to DM sysreset.

Signed-off-by: Bin Meng 

---

 arch/Kconfig  |  2 ++
 arch/x86/cpu/baytrail/valleyview.c|  6 --
 arch/x86/cpu/braswell/braswell.c  |  6 --
 arch/x86/cpu/cpu.c| 26 --
 arch/x86/cpu/ivybridge/early_me.c |  7 ---
 arch/x86/cpu/ivybridge/sdram.c|  3 ++-
 arch/x86/cpu/qemu/qemu.c  |  6 --
 arch/x86/cpu/quark/quark.c|  6 --
 arch/x86/cpu/tangier/tangier.c|  8 
 arch/x86/dts/bayleybay.dts|  1 +
 arch/x86/dts/baytrail_som-db5800-som-6867.dts |  1 +
 arch/x86/dts/broadwell_som-6896.dts   |  1 +
 arch/x86/dts/cherryhill.dts   |  1 +
 arch/x86/dts/chromebook_link.dts  |  1 +
 arch/x86/dts/chromebook_samus.dts |  1 +
 arch/x86/dts/chromebox_panther.dts|  1 +
 arch/x86/dts/conga-qeval20-qa3-e3845.dts  |  1 +
 arch/x86/dts/cougarcanyon2.dts|  1 +
 arch/x86/dts/crownbay.dts |  1 +
 arch/x86/dts/dfi-bt700.dtsi   |  1 +
 arch/x86/dts/edison.dts   |  1 +
 arch/x86/dts/efi-x86_payload.dts  |  1 +
 arch/x86/dts/galileo.dts  |  1 +
 arch/x86/dts/minnowmax.dts|  1 +
 arch/x86/dts/qemu-x86_i440fx.dts  |  1 +
 arch/x86/dts/qemu-x86_q35.dts |  1 +
 arch/x86/dts/reset.dtsi   |  6 ++
 arch/x86/include/asm/processor.h  |  5 -
 arch/x86/include/asm/u-boot-x86.h |  1 -
 configs/chromebook_link64_defconfig   |  1 +
 configs/edison_defconfig  |  1 +
 configs/efi-x86_app_defconfig |  1 +
 32 files changed, 42 insertions(+), 60 deletions(-)
 create mode 100644 arch/x86/dts/reset.dtsi

diff --git a/arch/Kconfig b/arch/Kconfig
index dd5a887..cbeb9f6 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -118,6 +118,8 @@ config X86
imply DM_SPI_FLASH
imply DM_USB
imply DM_VIDEO
+   imply SYSRESET
+   imply SYSRESET_X86
imply CMD_FPGA_LOADMK
imply CMD_GETTIME
imply CMD_IO
diff --git a/arch/x86/cpu/baytrail/valleyview.c 
b/arch/x86/cpu/baytrail/valleyview.c
index b7d481a..8882a76 100644
--- a/arch/x86/cpu/baytrail/valleyview.c
+++ b/arch/x86/cpu/baytrail/valleyview.c
@@ -55,9 +55,3 @@ int arch_misc_init(void)
 
return 0;
 }
-
-void reset_cpu(ulong addr)
-{
-   /* cold reset */
-   x86_full_reset();
-}
diff --git a/arch/x86/cpu/braswell/braswell.c b/arch/x86/cpu/braswell/braswell.c
index 32a6a5e..7a83b06 100644
--- a/arch/x86/cpu/braswell/braswell.c
+++ b/arch/x86/cpu/braswell/braswell.c
@@ -27,9 +27,3 @@ int arch_misc_init(void)
 
return 0;
 }
-
-void reset_cpu(ulong addr)
-{
-   /* cold reset */
-   x86_full_reset();
-}
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index 3a45677..395f845 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -76,37 +76,11 @@ int x86_init_cache(void)
 }
 int init_cache(void) __attribute__((weak, alias("x86_init_cache")));
 
-int do_reset(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
-{
-   printf("resetting ...\n");
-
-   /* wait 50 ms */
-   udelay(5);
-   disable_interrupts();
-   reset_cpu(0);
-
-   /*NOTREACHED*/
-   return 0;
-}
-
 void  flush_cache(unsigned long dummy1, unsigned long dummy2)
 {
asm("wbinvd\n");
 }
 
-__weak void reset_cpu(ulong addr)
-{
-   /* Do a hard reset through the chipset's reset control register */
-   outb(SYS_RST | RST_CPU, IO_PORT_RESET);
-   for (;;)
-   cpu_hlt();
-}
-
-void x86_full_reset(void)
-{
-   outb(FULL_RST | SYS_RST | RST_CPU, IO_PORT_RESET);
-}
-
 /* Define these functions to allow ehch-hcd to function */
 void flush_dcache_range(unsigned long start, unsigned long stop)
 {
diff --git a/arch/x86/cpu/ivybridge/early_me.c 
b/arch/x86/cpu/ivybridge/early_me.c
index 1a15229..219d5be 100644
--- a/arch/x86/cpu/ivybridge/early_me.c
+++ b/arch/x86/cpu/ivybridge/early_me.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -138,17 +139,17 @@ int intel_early_me_init_done(struct udevice *dev, struct 
udevice *me_dev,
case ME_HFS_ACK_RESET:
/* Non-power cycle reset */
set_global_reset(dev, 0);
-   reset_cpu(0);
+   sysreset_walk_halt(SYSRESET_COLD);
break;
case ME_HFS_ACK_PWR_CYCLE:
/* Power cycle reset */
set_global_reset(dev, 0);
-   x86_full_reset();
+   sysreset_walk_halt(SYSRESET_COLD);
break;
case ME_HFS_ACK_GBL_RESET:
/* Global reset */
set_global_reset(dev, 1);
-

[U-Boot] [PATCH 3/4] x86: fsp: Eliminate the reset_cpu() call

2018-07-03 Thread Bin Meng
In preparation for the reset driver conversion, eliminate the
reset_cpu() call in the FSP init path as it's too early for the
reset driver to work.

Signed-off-by: Bin Meng 
---

 arch/x86/lib/fsp/fsp_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c
index b4ba129..d5ed1d5 100644
--- a/arch/x86/lib/fsp/fsp_common.c
+++ b/arch/x86/lib/fsp/fsp_common.c
@@ -132,7 +132,7 @@ int arch_fsp_init(void)
chipset_clear_sleep_state();
/* Reboot */
debug("Rebooting..\n");
-   reset_cpu(0);
+   outb(SYS_RST | RST_CPU, IO_PORT_RESET);
/* Should not reach here.. */
panic("Reboot System");
}
-- 
2.7.4

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


[U-Boot] [PATCH 2/4] dm: sysreset: x86: Add a sysreset driver

2018-07-03 Thread Bin Meng
This adds a generic reset driver for x86 processor.

Signed-off-by: Bin Meng 
---

 drivers/sysreset/Kconfig|  6 +
 drivers/sysreset/Makefile   |  1 +
 drivers/sysreset/sysreset_x86.c | 49 +
 3 files changed, 56 insertions(+)
 create mode 100644 drivers/sysreset/sysreset_x86.c

diff --git a/drivers/sysreset/Kconfig b/drivers/sysreset/Kconfig
index a6d48e8..2afeadc 100644
--- a/drivers/sysreset/Kconfig
+++ b/drivers/sysreset/Kconfig
@@ -37,4 +37,10 @@ config SYSRESET_WATCHDOG
help
  Reboot support for generic watchdog reset.
 
+config SYSRESET_X86
+   bool "Enable support for x86 processor reboot driver"
+   depends on X86
+   help
+ Reboot support for generic x86 processor reset.
+
 endmenu
diff --git a/drivers/sysreset/Makefile b/drivers/sysreset/Makefile
index 0da58a1..0eb0dc7 100644
--- a/drivers/sysreset/Makefile
+++ b/drivers/sysreset/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_SYSRESET) += sysreset-uclass.o
 obj-$(CONFIG_SYSRESET_PSCI) += sysreset_psci.o
 obj-$(CONFIG_SYSRESET_SYSCON) += sysreset_syscon.o
 obj-$(CONFIG_SYSRESET_WATCHDOG) += sysreset_watchdog.o
+obj-$(CONFIG_SYSRESET_X86) += sysreset_x86.o
 obj-$(CONFIG_ARCH_ROCKCHIP) += sysreset_rockchip.o
 obj-$(CONFIG_SANDBOX) += sysreset_sandbox.o
 obj-$(CONFIG_ARCH_STI) += sysreset_sti.o
diff --git a/drivers/sysreset/sysreset_x86.c b/drivers/sysreset/sysreset_x86.c
new file mode 100644
index 000..5943a63
--- /dev/null
+++ b/drivers/sysreset/sysreset_x86.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018, Bin Meng 
+ *
+ * Generic reset driver for x86 processor
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int x86_sysreset_request(struct udevice *dev, enum sysreset_t type)
+{
+   int value;
+
+   switch (type) {
+   case SYSRESET_WARM:
+   value = SYS_RST | RST_CPU;
+   break;
+   case SYSRESET_COLD:
+   value = SYS_RST | RST_CPU | FULL_RST;
+   break;
+   default:
+   return -ENOSYS;
+   }
+
+   outb(value, IO_PORT_RESET);
+
+   return -EINPROGRESS;
+}
+
+static const struct udevice_id x86_sysreset_ids[] = {
+   { .compatible = "x86,reset" },
+   { }
+};
+
+static struct sysreset_ops x86_sysreset_ops = {
+   .request = x86_sysreset_request,
+};
+
+U_BOOT_DRIVER(x86_sysreset) = {
+   .name = "x86-sysreset",
+   .id = UCLASS_SYSRESET,
+   .of_match = x86_sysreset_ids,
+   .ops = _sysreset_ops,
+   .flags = DM_FLAG_PRE_RELOC,
+};
-- 
2.7.4

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


[U-Boot] [PATCH 1/4] x86: quark: acpi: Add full reset bit to the reset register value in FADT

2018-07-03 Thread Bin Meng
This adds full reset bit in the reset register value in the ACPI FADT
table, so that kernel can do a thorough reboot.

Signed-off-by: Bin Meng 
---

 arch/x86/cpu/quark/acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/cpu/quark/acpi.c b/arch/x86/cpu/quark/acpi.c
index 4a02720..7b6fc2f 100644
--- a/arch/x86/cpu/quark/acpi.c
+++ b/arch/x86/cpu/quark/acpi.c
@@ -67,7 +67,7 @@ void acpi_create_fadt(struct acpi_fadt *fadt, struct 
acpi_facs *facs,
fadt->reset_reg.access_size = ACPI_ACCESS_SIZE_BYTE_ACCESS;
fadt->reset_reg.addrl = IO_PORT_RESET;
fadt->reset_reg.addrh = 0;
-   fadt->reset_value = SYS_RST | RST_CPU;
+   fadt->reset_value = SYS_RST | RST_CPU | FULL_RST;
 
fadt->x_firmware_ctl_l = (u32)facs;
fadt->x_firmware_ctl_h = 0;
-- 
2.7.4

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


Re: [U-Boot] persistent logo on LCD on imx6ull

2018-07-03 Thread Michael Nazzareno Trimarchi
Hi Peng

Ok I have fixed it :)

Michael

On Mon, Jul 2, 2018 at 4:26 PM, Michael Nazzareno Trimarchi
 wrote:
> Hi Peng
>
> I'm working to make persistent the uboot logo on imx6ull platform but
> I have some problem with the framebuffer. Now this is the main idea
>
> I use this to recalculate allocation
>
> ubi part ubiblock; ubi read ${loadaddr} updater; run load_dtb;
> fdt addr ${fdt_addr}
> fdt set /reserved-memory/framebuffer reg "<0x$video_address
> 0x$video_memory_size>"
> fdt set /chosen/framebuffer reg "<0x$video_address 0x$video_memory_size>"
>
>
> In linux I have
>
> +   aliases {
> +   display0 = 
> +   };
> +
> +   reserved-memory {
> +   #address-cells = <1>;
> +   #size-cells = <1>;
> +   ranges;
> +
> +   display_reserved: framebuffer@86fa2000 {
> +   reg = <0x86fa2000 0x8>;
> +   no-map;
> +   };
> +
> +   };
> +
> +   chosen {
> +   #address-cells = <1>;
> +   #size-cells = <1>;
> +   ranges;
> +
> +   stdout-path = "display0";
> +   framebuffer0: framebuffer@86fa2000 {
> +   #clock-cells = <1>;
> +   pinctrl-names = "default";
> +   pinctrl-0 = <_lcdif>;
> +   compatible = "simple-framebuffer";
> +   reg = <0x86fa2000 (480 * 272 * 4)>;
> +   width = <480>;
> +   height = <272>;
> +   stride = <(480 * 4)>;
> +   format = "a8r8g8b8";
> +   clocks = < IMX6UL_CLK_LCDIF_PIX>,
> +< IMX6UL_CLK_LCDIF_APB>;
> +   nshut-supply = <_lcd_nshut>;
> +   nreset-supply = <_lcd_nreset>;
> +   display = <>;
> +   };
> +   };
>
> and this in uboot
>
> index 4011066..beddd4a 100644
> --- a/drivers/video/cfb_console.c
> +++ b/drivers/video/cfb_console.c
> @@ -69,6 +69,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
>  #if defined(CONFIG_VIDEO_MXS)
> @@ -2107,6 +2108,9 @@ static int cfg_video_init(void)
> video_console_address = video_fb_address;
>  #endif
>
> +   env_set_hex("video_address", (unsigned long)VIDEO_FB_ADRS);
> +   env_set_hex("video_memory_size", (unsigned long)
> roundup(VIDEO_SIZE, PAGE_SIZE));
> +
> /* Initialize the console */
> console_col = 0;
> console_row = 0;
> diff --git a/drivers/video/mxsfb.c b/drivers/video/mxsfb.c
> index 4e3e3d7..e2a5c05 100644
> --- a/drivers/video/mxsfb.c
> +++ b/drivers/video/mxsfb.c
> @@ -7,6 +7,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  #include 
>  #include 
> @@ -214,8 +215,8 @@ void *video_hw_init(void)
> panel.memSize = mode.xres * mode.yres * panel.gdfBytesPP;
>
> /* Allocate framebuffer */
> -   fb = memalign(ARCH_DMA_MINALIGN,
> - roundup(panel.memSize, ARCH_DMA_MINALIGN));
> +   fb = memalign(PAGE_SIZE,
> + roundup(panel.memSize, PAGE_SIZE));
> if (!fb) {
> printf("MXSFB: Error allocating framebuffer!\n");
> return NULL;
>
> I have two problems. (Using the latest uboot and linux)
>
> 1) I have a 50uS glitch on gpios so they get down and up again (nshut
> and nreset) even I don't touch them
> 2) Even clock tree is ok pixelclock seems that stops to go out
>
> Do you have any suggestion?
>
> Michael
>
>
>
> --
> | Michael Nazzareno Trimarchi Amarula Solutions BV |
> | COO  -  Founder  Cruquiuskade 47 |
> | +31(0)851119172 Amsterdam 1018 AM NL |
> |  [`as] http://www.amarulasolutions.com   |



-- 
| Michael Nazzareno Trimarchi Amarula Solutions BV |
| COO  -  Founder  Cruquiuskade 47 |
| +31(0)851119172 Amsterdam 1018 AM NL |
|  [`as] http://www.amarulasolutions.com   |
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates

2018-07-03 Thread Marek Vasut
On 07/03/2018 01:44 AM, Andre Przywara wrote:
> The USB host controllers on the H3, H5 and A64 have the oddity of
> sharing some clock and reset gates, so both the OHCI and EHCI bits have
> to be enabled to make only one of them working. We take care of this, and
> initialisation works fine (due to setting already set bits).
> However on shutdown we turn the clocks and reset gates off already when
> deregistering one controller, so the other one is no longer functional.
> In the result U-Boot complains just before launching the kernel and
> then hangs.
> Fix this by not turning off the clocks and resets on the OHCI side, so
> that the EHCI controller has a chance to properly shut down.
> This still isn't perfect, but at least prevents the hang.
> 
> Signed-off-by: Andre Przywara 

What about adding some enable/disable counter to those clock somehow and
then turning them off when the counter reaches zero ?

> ---
> Hi,
> 
> commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
> introduced the proper reset and clock gates for the A64. While the patch
> itself is correct, it broke Linux as soon as one actually enables USB0 in
> the DT (in the moment we keep this "disabled" in U-Boot's DT version).
> 
> I understand that this patch here is somewhat of a hack, but the proper
> ref-counting is not easy to implement between the separate EHCI and OHCI
> drivers. Those two files are doomed anyway, since driver model clocks
> and reset drivers are already on the horizon:
> https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
> So lets fix this up for now so that we can use the Linux kernel DTs with
> U-Boot itself.
> 
> Cheers,
> Andre.
> 
>  drivers/usb/host/ohci-sunxi.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c
> index 0ddbdbe460..42ffb6cbcb 100644
> --- a/drivers/usb/host/ohci-sunxi.c
> +++ b/drivers/usb/host/ohci-sunxi.c
> @@ -128,10 +128,18 @@ static int ohci_usb_remove(struct udevice *dev)
>   if (ret)
>   return ret;
>  
> - if (priv->cfg->has_reset)
> - clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
> - clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
> - clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
> + /*
> +  * For those SoCs that share the clock and reset gates with the EHCI
> +  * controller, we should not turn them off here, to prevent the
> +  * other one hanging (when the EHCI driver tries to shut itself down).
> +  */
> + if (!priv->cfg->extra_ahb_gate_mask) {
> + if (priv->cfg->has_reset)
> + clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
> + clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
> + }
> + if (!priv->cfg->extra_usb_gate_mask)
> + clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>  
>   return 0;
>  }
> 


-- 
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] arm64: allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi

2018-07-03 Thread Maxime Ripard
On Mon, Jul 02, 2018 at 11:57:45PM +0200, Marek Vasut wrote:
> On 07/02/2018 11:40 PM, Tom Rini wrote:
> > On Mon, Jul 02, 2018 at 11:27:58PM +0200, Marek Vasut wrote:
> >> On 07/02/2018 10:53 PM, Jagan Teki wrote:
> >>> During usb shutdown or 'usb reset' all the necessary clocks
> >>> on the specific controller will disable. Usually this shutdown
> >>> happen during U-Boot proper handoff to Linux.
> >>
> >> No, 'usb reset' can be triggered by the user any time.
> > 
> > Yes, and it's also triggered as part of the hand-off, which is the use
> > case in question.
> 
> No, that's not true. The USB controllers are torn down when starting the
> OS, which is a different thing from usb reset, which brings them back up
> and rescans the bus too.
> 
> >>> There is an issue in Allwinner A64, is during OHCI1 shutdown
> >>> the controller is unable to access the register space
> >>> so the Linux boot hangs at this place.
> >>
> >> This doesn't make any sense, Linux should enable the necessary clock
> >> before accessing any controller registers, unless there is a bug in Linux.
> > 
> > Should but doesn't always?  So yes, there's possibly / hopefully a bug
> > in the dts files.
> 
> How did you reach that conclusion about the DTS files ? There is a bug
> in Linux, but it's likely in the driver which doesn't enable the clock
> before accessing the registers.
> 
> But maybe the description here is completely confusing, since the output
> down below would indicate this hang is still in U-Boot.
> 
> >>> This particular condition occur when we enable both the
> >>> controllers, so this patch will disable OHCI1 and EHCI1 for
> >>> bananapi-m64 and nanopi-a64 boards. It will re-enable the same
> >>> once the issue got fixed.
> >>>
> >>> Log:
> >>> => usb reset
> >>> resetting USB...
> >>>
> >>> PHY0: mask = 0x101, usb_clk_cfg = 0x30202
> >>> sunxi_musb_exit: ahb_gate0  = 0x33004540, reset0_cfg = 0x33004540
> >>> EHCI failed to shut down host controller.
> >>> ehci_usb_remove: ahb_gate0  = 0x32004540, reset0_cfg = 0x32004540
> >>> ohci_usb_remove: ahb_gate0  = 0x22004540, reset0_cfg = 0x22004540
> >>> ohci_usb_remove: mask = 0x1, usb_clk_cfg = 0x20202
> >>>
> >>> PHY1: mask = 0x202, usb_clk_cfg = 0x0
> >>> ehci_usb_remove: ahb_gate0  = 0x20004540, reset0_cfg = 0x20004540
> >>
> >> Why is this usb reset so noisy ?
> > 
> > ... I would assume additional debug messages.
> > 
> >>> << hang here >>
> >>
> >> Please fix this properly, this patch is pure attempt to hide a bug.
> >> NAK from me.
> > 
> > Well, the point of this patch as Jagan says is to hide the bug for the
> > release so that Linux can boot, which is an important case.
> 
> But the above implies that Linux can boot and the hang happens while
> still in U-Boot due to some ordering bug between clock and register
> access in the .remove function of some driver (I guess). That is what
> needs to be debugged and fixed.

Yeah, I agree. If we have a bug in U-Boot or in Linux, we should fix
whatever it is. Papering over it will just keep it, with no one
interested in fixing it anymore.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates

2018-07-03 Thread Jagan Teki
On Tue, Jul 3, 2018 at 5:14 AM, Andre Przywara  wrote:
> The USB host controllers on the H3, H5 and A64 have the oddity of
> sharing some clock and reset gates, so both the OHCI and EHCI bits have
> to be enabled to make only one of them working. We take care of this, and
> initialisation works fine (due to setting already set bits).
> However on shutdown we turn the clocks and reset gates off already when
> deregistering one controller, so the other one is no longer functional.
> In the result U-Boot complains just before launching the kernel and
> then hangs.
> Fix this by not turning off the clocks and resets on the OHCI side, so
> that the EHCI controller has a chance to properly shut down.
> This still isn't perfect, but at least prevents the hang.
>
> Signed-off-by: Andre Przywara 
> ---
> Hi,
>
> commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
> introduced the proper reset and clock gates for the A64. While the patch
> itself is correct, it broke Linux as soon as one actually enables USB0 in
> the DT (in the moment we keep this "disabled" in U-Boot's DT version).
>
> I understand that this patch here is somewhat of a hack, but the proper
> ref-counting is not easy to implement between the separate EHCI and OHCI
> drivers. Those two files are doomed anyway, since driver model clocks
> and reset drivers are already on the horizon:
> https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
> So lets fix this up for now so that we can use the Linux kernel DTs with
> U-Boot itself.
>
> Cheers,
> Andre.
>
>  drivers/usb/host/ohci-sunxi.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c
> index 0ddbdbe460..42ffb6cbcb 100644
> --- a/drivers/usb/host/ohci-sunxi.c
> +++ b/drivers/usb/host/ohci-sunxi.c
> @@ -128,10 +128,18 @@ static int ohci_usb_remove(struct udevice *dev)
> if (ret)
> return ret;
>
> -   if (priv->cfg->has_reset)
> -   clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
> -   clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
> -   clrbits_le32(>ccm->ahb_gate0, priv->ahb_gate_mask);
> +   /*
> +* For those SoCs that share the clock and reset gates with the EHCI
> +* controller, we should not turn them off here, to prevent the
> +* other one hanging (when the EHCI driver tries to shut itself down).

I've tried this "not to disable usb_clk_cfg" before sending "arm64:
allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi" patch.
But, I thought improper shutdown may effect other issue may be with
Linux.

Initially I found an issue of disabling OHCI1 gate clock during OHCI0
shutdown. ohci driver is trying to clear BIT(16) for OHCI0, but it
also clears BIT(17), which is for OHCI1. By adding proper shutdown
support for musb and other things I came to a situation where all
shutdown happen properly. But ofcourse I still see hang.

=> usb reset
resetting USB...

PHY0: mask = 0x101, usb_clk_cfg = 0x30202
sunxi_musb_exit: ahb_gate0  = 0x33004540, reset0_cfg = 0x33004540
EHCI failed to shut down host controller.
ehci_usb_remove: ahb_gate0  = 0x32004540, reset0_cfg = 0x32004540
ohci_usb_remove: ahb_gate0  = 0x22004540, reset0_cfg = 0x22004540
ohci_usb_remove: mask = 0x1, usb_clk_cfg = 0x20202

PHY1: mask = 0x202, usb_clk_cfg = 0x0
ehci_usb_remove: ahb_gate0  = 0x20004540, reset0_cfg = 0x20004540
<< hang  >>

Do you think this is still an issue with clock? considering proper
shutdown sequence happened above.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH 1/1] ARM: qemu-arm: enable RTC

2018-07-03 Thread Heinrich Schuchardt
On 07/03/2018 07:29 AM, Takahiro AKASHI wrote:
> On Tue, Jul 03, 2018 at 04:07:31AM +0200, Heinrich Schuchardt wrote:
>> On 07/03/2018 01:51 AM, Takahiro AKASHI wrote:
>>> On Mon, Jul 02, 2018 at 07:07:55PM +0300, Tuomas Tynkkynen wrote:
 Hi Heinrich,

 On 06/29/2018 01:34 AM, Heinrich Schuchardt wrote:
> QEMU provides an emulated ARM AMBA PrimeCell PL031 RTC.
>
> The patch sets the base address in the board include file according to the
> definition in hw/arm/virt.c of the QEMU source. It defines the Kconfig
> option for the existing driver, and enables the RTC driver in
> qemu_arm64_defconfig and qemu_arm_defconfig as well as the date command.
>
> We need an RTC to provide the GetTime() runtime service in the UEFI
> subsystem.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  configs/qemu_arm64_defconfig | 2 ++
>  configs/qemu_arm_defconfig   | 2 ++
>  drivers/rtc/Kconfig  | 7 +++
>  include/configs/qemu-arm.h   | 3 +++
>  4 files changed, 14 insertions(+)
>

 Reviewed-by: Tuomas Tynkkynen 
 Tested-by: Tuomas Tynkkynen 

 - Tuomas
>>>
>>> Well, it is a good time. Why not change the driver to driver model
>>> like below:
>>>  * I intentionally leave CONFIG_DM_RTC not mandatory here
>>>  * The patch may be split into two parts; one for rtc, the other for 
>>> qemu-arm
>>
>> Hello Takahiro,
>>
>> thank you for your suggestion. Yes we should convert drivers to the
>> driver model. Unfortunately the patch that you appended below is not
>> applicable to u-boot/master.
> 
> Thank you for your review. It is very helpful as I have not fully
> tested my change.
> 
>> Error: patch failed: include/configs/qemu-arm.h:36
>> error: include/configs/qemu-arm.h: patch does not apply
>> Patch failed at 0001 ARM: qemu-arm: enable RTC
>>
>> So I copied the changes to qemu-arm.h manually. Instead of defining the
>> base address here it would be preferable to read the address from the
>> device tree. This will become important if we get
>>
>> Compiling with CONFIG_DM_RTC and CONFIG_RTC_PL031 resulted in warnings:
>>
>>   CC  drivers/rtc/pl031.o
>> drivers/rtc/pl031.c: In function ‘pl031_rtc_set’:
>> drivers/rtc/pl031.c:141:17: warning: passing argument 1 of ‘rtc_set’
>> discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
>>   return rtc_set(tm);
>>  ^~
>> drivers/rtc/pl031.c:72:5: note: expected ‘struct rtc_time *’ but
>> argument is of type ‘const struct rtc_time *’
>>  int rtc_set(struct rtc_time *tmp)
>>  ^~~
>>
>> I tested both with qemu-arm64_defconfig and qemu-arm_defconfig. The date
>> command is running fine in both cases.
>>
>> The driver with your patch cannot be compiled without DM_RTC (due to
>> missing symbol CONFIG_SYS_RTC_PL031_BASE) so in Kconfig it should depend
>> on DM_RTC.
> 
> Ouch.
> 
>> There is no need to keep any old style stuff. I suggest to drop legacy
>> functions and #ifdef CONFIG_DM_RTC. Also the following line can be removed:
> 
> My concern was that it would break any downstream code.
> 
> 
>> scripts/config_whitelist.txt:4118:CONFIG_SYS_RTC_PL031_BASE
>>
>> In qemu/hw/arm/virt.c, function create_rtc() a device tree node is
>> created which describes the pl031 RTC including the base address. From
>> what I read in the code the node should look like this:
>>
>> /{
>>
>>  pl011@0901 {
>>  compatible = "arm,pl011", "arm,primecell";
>>  #address-cells = <2>;
>>  #size-cells = <2>;
>>  reg = reg = <0x0901 0x1000>;
>>  ...
>>  };
>>
>> };
>>
>> So there is no need to define SYS_RTC_BASE and using the U_BOOT_DEVICE
>> macro. We can set up the platform data in the probe function by reading
>> the reg property of the device node.
> 
> Since no dts for qemu-arm is provided in u-boot tree, I'm not sure
> that this change makes sense.

There is no device-tree provided by U-Boot for qemu-arm because it is
already provided by QEMU itself. You can verify with U-Boot command 'fdt
print' that this device tree provides a reg property for the clock.

fdt addr ${fdtcontroladdr}
fdt print /

Cf. function board_fdt_blob_setup() in board/emulation/qemu-arm/qemu-arm.c.

Looking at the device trees in the Linux kernel for boards that we do
not currently support like
linux/arch/arm/boot/dts/arm-realview-eb.dtsi
you will find that all boards but one provide a reg property for the
PL031 with the address of the clock registers. And this address depends
on the board.

So if you rely on the QEMU delivered device tree to provide the address
of the RTC registers this will enable us to support the PL031 RTC on
other boards like the HiKey 960 in the future.

> 
>> Should we also add .compatible="arm,primecell" to the list of IDs?
> 
> Yeah, I know "arm,primecell" is also added for other arm IPs, but
> think that it is too vague in contrast to pl031, isn't it?

You are