Re: [PATCH v4 2/2] test: gpio: Add tests for the managed API

2020-09-08 Thread Pratyush Yadav
On 08/09/20 05:56PM, Simon Glass wrote:
> On Mon, 7 Sep 2020 at 23:40, Pratyush Yadav  wrote:
> >
> > From: Jean-Jacques Hiblot 
> >
> > Add a test to verify that GPIOs can be acquired/released using the managed
> > API. Also check that the GPIOs are released when the consumer device is
> > removed.
> >
> > Signed-off-by: Jean-Jacques Hiblot 
> > Signed-off-by: Pratyush Yadav 
> > ---
> >  arch/sandbox/dts/test.dts |  10 
> >  test/dm/bus.c |   2 +-
> >  test/dm/gpio.c| 102 ++
> >  test/dm/test-fdt.c|   6 +--
> >  4 files changed, 116 insertions(+), 4 deletions(-)
> 
> Reviewed-by: Simon Glass 

Thanks.
 
> Although I don't see a change log, so this is a bit blind.

The changelog is in the cover letter [0]. Copy-pasing the parts relevant 
to this patch:

- Move "another-test" node down the order to make sure
  dm_test_fdt_uclass_seq() continues to work.
- Bump num_devices to 9 in dm_test_bus_children(), dm_test_fdt(), and
  dm_test_uclass_foreach() to fix broken tests.
- s/DM_TESTF/UT_TESTF/g

[0] I haven't figured out a workflow that lets me easily record the
changelog on a per-patch basis. Maybe I can write some scripts around 
git-notes for that? Dunno.

-- 
Regards,
Pratyush Yadav
Texas Instruments India


Re: [RFCv2] common: Drop remaining includes in common.h

2020-09-08 Thread Simon Glass
Hi Tom,

(adding Heinrich as he mentioned this series on irc)

On Wed, 19 Aug 2020 at 07:09, Tom Rini  wrote:
>
> I've picked up Simon's v1 of this series and moved it to an RFC with
> this v2.  I don't intend for this series to go in as-is but rather since
> I spent a good bit of time iterating over the problems of trying a
> conversion (in a few places) where we only selectively add back in the
> header being removed from common.h in the case of a fail to build, I
> didn't want the work lost.
>
> What I think needs to be done moving forward is even smaller series here
> where we focus on removing one or two headers, but then only re-add them
> where required.
>
> Also note that this series shows a few funny issues.  The patch to
> remove  and selectively re-add it shows:
> bcm958712k : all -4 text -4
>u-boot: add: 1/0, grow: 0/-3 bytes: 24/-28 (-4)
>  function   old new   
> delta
>  blk_dread-  24 
> +24
>  part_test_efi  184 180  
> -4
>  is_gpt_valid   736 724 
> -12
>  fs_devread 600 588 
> -12
>
> everywhere that code is used.  I don't see why, but there's some
> underlying problem exposed in the move.  I believe it's also that patch

After an hour's ceaseless searching I narrowed this down to 'inline'.
In linux/compiler.h it  defines it to __gnu_inline
__inline_maybe_unused, etc which changes the inlining behaviour of the
compiler.

I should have guessed this from what you said below.

> which shows, for every big-endian platform something like:
> T4160RDB   : all -592 text -592
>u-boot: add: 2/0, grow: 1/-14 bytes: 68/-656 (-588)
>  function   old new   
> delta
>  ___arch__swab32  -  48 
> +48
>  blk_dread-  12 
> +12
>  ext4fs_bg_get_inode_table_id76  84  
> +8
>  ehci_td_buffer 164 156  
> -8
>  _ehci_destroy_int_queue244 236  
> -8
>  static.ehci_update_endpt2_dev_n_port   116 104 
> -12
>  ext4fs_open200 188 
> -12
>  _ehci_poll_int_queue   236 224 
> -12
>  usb_lowlevel_init  916 892 
> -24
>  ext4fs_mount   332 304 
> -28
>  fs_devread 572 540 
> -32
>  ext4fs_find_file1  756 724 
> -32
>  ext4fs_iterate_dir 848 812 
> -36
>  ext4fs_read_inode  520 452 
> -68
>  static._ehci_create_int_queue 1000 908 
> -92
>  ehci_submit_async 16321520
> -112
>  read_allocated_block  25322352
> -180
>
> and I lack hardware to see (and it looks like qemu-ppce500 can't be
> given a disk atm) if the problem is that ext2/4 is broken before and
> fixed now, or fixed now and broken with this patch, as that's my first
> concern on seeing ___arch__swab32 show up.  But maybe it's a harmless
> "no, don't inline ..." decision the compiler is now able to make.  But
> very non-obvious and needing a run-time sanity check to be sure.
>

I suppose this is the same.

Basically we need to include  whenever inline is used. I
should have guessed that. It is annoying that 'inline' is defined to
something else when that header is included. This is another gotcha.

It would be great to fix up this series and get it applied before the
new merge window makes everything harder. I don't think we have a lot
of unnecessary header inclusions, and here we have just found more we
need to add.

Regards,
Simon


Re: [PATCH 0/7] riscv: Correctly handle IPIs already pending upon boot

2020-09-08 Thread Sean Anderson
On 9/8/20 10:38 PM, Sean Anderson wrote:
> On 9/8/20 10:02 PM, Rick Chen wrote:
>> Hi Sean
>>
>>> On the K210, the prior stage bootloader does not clear IPIs. This presents
>>> a problem, because U-Boot up until this point assumes (with one exception)
>>> that IPIs are cleared when it starts. This series attempts to fix this in a
>>> robust manner, and fix several concurrency bugs I noticed while fixing
>>> these other areas. Heinrich previously submitted a patch addressing part of
>>> this problem in [1].
>>>
>>> [1] 
>>> https://patchwork.ozlabs.org/project/uboot/patch/20200811035648.3284-1-xypron.g...@gmx.de/
>>>
>>
>> It sounds like that the bootloader does not deal with SMP flow well
>> and jump to u-boot-spl, right ?
>>
>> I have a question that why not try to fix the prior stage bootloader
>> to clear IPIs correctly?
> 
> Because it is a ROM :)

Err, perhaps I should clarify. When I say "prior stage bootloader," I
mean that in the general sense of "anything which comes before U-Boot,"
and not to refer specifically to SPL or TPL. For the k210, this is
something akin to the ZSBL on an fu540.

--Sean


Re: [PATCH 0/7] riscv: Correctly handle IPIs already pending upon boot

2020-09-08 Thread Sean Anderson
On 9/8/20 10:02 PM, Rick Chen wrote:
> Hi Sean
> 
>> On the K210, the prior stage bootloader does not clear IPIs. This presents
>> a problem, because U-Boot up until this point assumes (with one exception)
>> that IPIs are cleared when it starts. This series attempts to fix this in a
>> robust manner, and fix several concurrency bugs I noticed while fixing
>> these other areas. Heinrich previously submitted a patch addressing part of
>> this problem in [1].
>>
>> [1] 
>> https://patchwork.ozlabs.org/project/uboot/patch/20200811035648.3284-1-xypron.g...@gmx.de/
>>
> 
> It sounds like that the bootloader does not deal with SMP flow well
> and jump to u-boot-spl, right ?
> 
> I have a question that why not try to fix the prior stage bootloader
> to clear IPIs correctly?

Because it is a ROM :)

> 
> Actually I have encounter a similar SMP issue like you.
> Our prior stage bootloader will jump to u-boot-spl with the incorrect
> mstatus and result in the SMP working abnormal in u-boot-spl.

Perhaps we should just clear MIE then? I originally had a patch in this
series which moved the handle_ipi code into handle_trap, and got rid of
the manual checks on the interrupt. Something like

secondary_hart_loop:
wfi
j   secondary_hart_loop

Of course as part of that we would need to explicitly enable and disable
interrupts. Perhaps not the worst idea, but I didn't include it here
because I figure the current system work OK, even if it is not what one
might expect.

> I mean this is an individual case, not a general case.
> If we try to cover any errors which come from any prior stage bootloaders,
> the SMP flow will become more and more complicated and hard to debug.

Of course, if a prior bootloader doesn't hold up its end of the
contract, we can be left with some awful bugs to fix. U-Boot is
generally not too bad to debug, but I've had an awful time whenever some
concurrency sneaks into the mix. I think it's much better to confine the
(necessary) complexity to as few files as possible, so that the rest of
the code can be ignorant. I think part of that is verifying that we have
everything in a known state, so that when we see something unexpected,
we can handle it/panic/whatever instead of silently getting a bug.

--Sean


Re: [PATCH 0/7] riscv: Correctly handle IPIs already pending upon boot

2020-09-08 Thread Rick Chen
Hi Sean

> On the K210, the prior stage bootloader does not clear IPIs. This presents
> a problem, because U-Boot up until this point assumes (with one exception)
> that IPIs are cleared when it starts. This series attempts to fix this in a
> robust manner, and fix several concurrency bugs I noticed while fixing
> these other areas. Heinrich previously submitted a patch addressing part of
> this problem in [1].
>
> [1] 
> https://patchwork.ozlabs.org/project/uboot/patch/20200811035648.3284-1-xypron.g...@gmx.de/
>

It sounds like that the bootloader does not deal with SMP flow well
and jump to u-boot-spl, right ?

I have a question that why not try to fix the prior stage bootloader
to clear IPIs correctly?

Actually I have encounter a similar SMP issue like you.
Our prior stage bootloader will jump to u-boot-spl with the incorrect
mstatus and result in the SMP working abnormal in u-boot-spl.

I mean this is an individual case, not a general case.
If we try to cover any errors which come from any prior stage bootloaders,
the SMP flow will become more and more complicated and hard to debug.

That is why it does not need implement SMP flow in U-Boot proper with
SBI v0.2 HSM extension.

Thanks,
Rick

>
> Sean Anderson (7):
>   Revert "riscv: Clear pending interrupts before enabling IPIs"
>   riscv: Match memory barriers between send_ipi_many and handle_ipi
>   riscv: Use NULL as a sentinel value for smp_call_function
>   riscv: Clear pending IPIs on initialization
>   riscv: Add fence to available_harts_lock
>   riscv: Ensure gp is NULL or points to valid data
>   riscv: Add some comments to start.S
>
>  arch/riscv/cpu/cpu.c| 18 ++
>  arch/riscv/cpu/start.S  | 47 +
>  arch/riscv/lib/interrupts.c |  3 ++-
>  arch/riscv/lib/smp.c| 26 +---
>  4 files changed, 80 insertions(+), 14 deletions(-)
>
> --
> 2.28.0
>


Re: [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Bin Meng
Hi Marek,

On Wed, Sep 9, 2020 at 12:15 AM Marek Vasut  wrote:
>
> On 9/8/20 5:45 PM, Bin Meng wrote:
> > Hi Marek,
>
> Hi,
>
> > On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut  wrote:
> >>
> >> On 9/8/20 3:44 AM, Bin Meng wrote:
> >>> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun  
> >>> wrote:
> 
>  Use readx_poll_sleep_timeout() to poll the register status
> 
>  Signed-off-by: Chunfeng Yun 
>  ---
>  v3: no changes
> 
>  v2: fix typo of title suggested by Frank
>  ---
>   drivers/usb/host/xhci.c | 25 +++--
>   1 file changed, 11 insertions(+), 14 deletions(-)
> 
> >>>
> >>> Reviewed-by: Bin Meng 
> >>
> >> Thanks. So now I have half of a RESEND patchset with RB tags, and
> >> another half of just RB tags without patchset. Can someone of you please
> >> collect the tags and resend the patchset once more with the collected
> >> tags, so it's all in one place ?
> >>
> >
> > I believe the latest version has all the RB tags.
>
> Thank you, I cannot apply a patchset which only has "Re:" in my mailbox,
> because the original patches are just not there.

You can use patchwork to help the custodian work. I found it very
helpful to download the patches from patchwork and apply.

Regards,
Bin


[PATCH] cmd: mem: fix range of bitflip test

2020-09-08 Thread Ralph Siemsen
The bitflip test uses two equal sized memory buffers. This is achived
by splitting the range of memory into two pieces. The address of the
second buffer was not correctly calulated, thus the bitflip test would
accessing memory beyond the end of the specified range.

A second problem arises because u-boot "mtest" command expects the
ending address to be inclusive. When computing (end - start) this
results in missing 1 byte of the requested length. The bitflip test in
turn misses the last word.

Fixes: 8e434cb705d463bc8cff935160e4fb4c77cb99ab ("cmd: mem: Add bitflip
memory test to alternate mtest")

Signed-off-by: Ralph Siemsen 
--
TODO/FIXME: maybe the ending address should be automatically aligned?

Change-Id: Ie641d04e731fc5bc6a3bbef914bf7fad136cdc94
---
 cmd/mem.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cmd/mem.c b/cmd/mem.c
index 9b97f7bf69..88e15b2d61 100644
--- a/cmd/mem.c
+++ b/cmd/mem.c
@@ -988,8 +988,9 @@ static int do_mem_mtest(struct cmd_tbl *cmdtp, int flag, 
int argc,
break;
count += errs;
errs = test_bitflip_comparison(buf,
-  buf + (end - start) / 2,
-  (end - start) /
+  buf + (end - start + 1) 
/ 2 /
+  sizeof(unsigned long),
+  (end - start + 1) / 2 /
   sizeof(unsigned long));
} else {
errs = mem_test_quick(buf, start, end, pattern,
-- 
2.17.1



Re: [PATCH] time: Fix get_ticks being non-monotonic

2020-09-08 Thread Sean Anderson
On 9/8/20 8:01 PM, Simon Glass wrote:
> Hi Sean,
> 
> On Tue, 8 Sep 2020 at 17:59, Sean Anderson  wrote:
>>
>> On 9/8/20 7:56 PM, Simon Glass wrote:
>>> Hi Sean,
>>>
>>> On Mon, 7 Sep 2020 at 09:51, Sean Anderson  wrote:

 On 9/7/20 9:57 AM, Simon Glass wrote:
> Hi Sean,
>
> On Sun, 6 Sep 2020 at 20:02, Sean Anderson  wrote:
>>
>> On 9/6/20 9:43 PM, Simon Glass wrote:
>>> Hi Sean,
>>>
>>> On Tue, 1 Sep 2020 at 13:56, Sean Anderson  wrote:

 get_ticks does not always succeed. Sometimes it can be called before 
 the
 timer has been initialized. If it does, it returns a negative errno.
 This causes the timer to appear non-monotonic, because the value will
 become much smaller after the timer is initialized.

 No users of get_ticks which I checked handle errors of this kind. 
 Further,
 functions like tick_to_time mangle the result of get_ticks, making it 
 very
 unlikely that one could check for an error without suggesting a patch 
 such
 as this one.

 This patch changes get_ticks to always return 0 when there is an error.
 0 is the least unsigned integer, ensuring get_ticks appears monotonic. 
 This
 has the side effect of time apparently not passing until the timer is
 initialized. However, without this patch, time does not pass anyway,
 because the error value is likely to be the same.

 Fixes: c8a7ba9e6a5
 Signed-off-by: Sean Anderson 
 ---

  lib/time.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> Would it be better to panic so people can fix the bug?
>>
>> I thought this was expected behavior. It's only a bug if you do
>> something like udelay before any timers are created. We just can't
>> report errors through get_ticks, because its users assume that it always
>> returns a time of some kind.
>
> I think it indicates a bug. If you use a device before it is ready you
> don't really know what it will do. I worry that this patch is just
> going to cause confusion, since the behaviour depends on when you call
> it. If we panic, people can figure out why the timer is being inited
> too late, or being used too early.

 Hm, maybe. I don't think it's as clear cut as "us[ing] a device before
 it is ready," because get_ticks tries to initialize the timer if it
 isn't already initialized. Unless someone else does it first, the first
 call to get_ticks will always be before the timer is initialized.

 The specific problem I ran into was that after relocation, the watchdog
 may be initialized before the timer. This occurs on RISC-V because
 without [1] a timer only exists after arch_early_init_r. So, for the
 first few calls to watchdog_reset there is no timer.

 The second return could probably be turned into a panic. I checked, and
 all current timer drivers always succeed in getting the time (except for
 the RISC-V timer, which is fixed in [1]), so the only way for
 timer_get_count to fail is if timer_ops.get_count doesn't exist. That is
 almost certainly an error on the driver author's part, so I think
 panicking there is the only reasonable option.
>>>
>>> OK good, let's do that and update docs in timer.h
>>
>> That being to panic both times, or just panic the second time?
> 
> Well I like a panic if the call is invalid, ie. in both cases.

Ok, sounds good to me.

--Sean


Re: [PATCH] time: Fix get_ticks being non-monotonic

2020-09-08 Thread Simon Glass
Hi Sean,

On Tue, 8 Sep 2020 at 17:59, Sean Anderson  wrote:
>
> On 9/8/20 7:56 PM, Simon Glass wrote:
> > Hi Sean,
> >
> > On Mon, 7 Sep 2020 at 09:51, Sean Anderson  wrote:
> >>
> >> On 9/7/20 9:57 AM, Simon Glass wrote:
> >>> Hi Sean,
> >>>
> >>> On Sun, 6 Sep 2020 at 20:02, Sean Anderson  wrote:
> 
>  On 9/6/20 9:43 PM, Simon Glass wrote:
> > Hi Sean,
> >
> > On Tue, 1 Sep 2020 at 13:56, Sean Anderson  wrote:
> >>
> >> get_ticks does not always succeed. Sometimes it can be called before 
> >> the
> >> timer has been initialized. If it does, it returns a negative errno.
> >> This causes the timer to appear non-monotonic, because the value will
> >> become much smaller after the timer is initialized.
> >>
> >> No users of get_ticks which I checked handle errors of this kind. 
> >> Further,
> >> functions like tick_to_time mangle the result of get_ticks, making it 
> >> very
> >> unlikely that one could check for an error without suggesting a patch 
> >> such
> >> as this one.
> >>
> >> This patch changes get_ticks to always return 0 when there is an error.
> >> 0 is the least unsigned integer, ensuring get_ticks appears monotonic. 
> >> This
> >> has the side effect of time apparently not passing until the timer is
> >> initialized. However, without this patch, time does not pass anyway,
> >> because the error value is likely to be the same.
> >>
> >> Fixes: c8a7ba9e6a5
> >> Signed-off-by: Sean Anderson 
> >> ---
> >>
> >>  lib/time.c | 4 ++--
> >>  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > Would it be better to panic so people can fix the bug?
> 
>  I thought this was expected behavior. It's only a bug if you do
>  something like udelay before any timers are created. We just can't
>  report errors through get_ticks, because its users assume that it always
>  returns a time of some kind.
> >>>
> >>> I think it indicates a bug. If you use a device before it is ready you
> >>> don't really know what it will do. I worry that this patch is just
> >>> going to cause confusion, since the behaviour depends on when you call
> >>> it. If we panic, people can figure out why the timer is being inited
> >>> too late, or being used too early.
> >>
> >> Hm, maybe. I don't think it's as clear cut as "us[ing] a device before
> >> it is ready," because get_ticks tries to initialize the timer if it
> >> isn't already initialized. Unless someone else does it first, the first
> >> call to get_ticks will always be before the timer is initialized.
> >>
> >> The specific problem I ran into was that after relocation, the watchdog
> >> may be initialized before the timer. This occurs on RISC-V because
> >> without [1] a timer only exists after arch_early_init_r. So, for the
> >> first few calls to watchdog_reset there is no timer.
> >>
> >> The second return could probably be turned into a panic. I checked, and
> >> all current timer drivers always succeed in getting the time (except for
> >> the RISC-V timer, which is fixed in [1]), so the only way for
> >> timer_get_count to fail is if timer_ops.get_count doesn't exist. That is
> >> almost certainly an error on the driver author's part, so I think
> >> panicking there is the only reasonable option.
> >
> > OK good, let's do that and update docs in timer.h
>
> That being to panic both times, or just panic the second time?

Well I like a panic if the call is invalid, ie. in both cases.

Regards,
Simon


Re: [PATCH] timer: Return count from timer_ops.get_count

2020-09-08 Thread Sean Anderson
On 9/8/20 7:56 PM, Simon Glass wrote:
> Hi Sean,
> 
> 
> On Mon, 7 Sep 2020 at 12:19, Sean Anderson  wrote:
>>
>> No timer drivers return an error from get_count. Instead of possibly
>> returning an error, just return the count directly.
>>
>> Signed-off-by: Sean Anderson 
>> ---
>> Passing CI (but not otherwise tested):
>> https://dev.azure.com/seanga2/u-boot/_build/results?buildId=25=results
>>
>> This patch depends on
>> https://patchwork.ozlabs.org/project/uboot/list/?series=198797
>>
>>  arch/riscv/lib/andes_plmt.c   |  6 ++
>>  arch/riscv/lib/sifive_clint.c |  6 ++
>>  drivers/timer/ag101p_timer.c  |  5 ++---
>>  drivers/timer/altera_timer.c  |  6 ++
>>  drivers/timer/arc_timer.c |  6 ++
>>  drivers/timer/ast_timer.c |  6 ++
>>  drivers/timer/atcpit100_timer.c   |  5 ++---
>>  drivers/timer/atmel_pit_timer.c   |  6 ++
>>  drivers/timer/cadence-ttc.c   |  6 ++
>>  drivers/timer/dw-apb-timer.c  |  6 ++
>>  drivers/timer/mpc83xx_timer.c |  6 ++
>>  drivers/timer/mtk_timer.c |  6 ++
>>  drivers/timer/nomadik-mtu-timer.c |  6 ++
>>  drivers/timer/omap-timer.c|  6 ++
>>  drivers/timer/ostm_timer.c|  6 ++
>>  drivers/timer/riscv_timer.c   | 21 +
>>  drivers/timer/rockchip_timer.c|  5 ++---
>>  drivers/timer/sandbox_timer.c |  6 ++
>>  drivers/timer/sti-timer.c |  6 ++
>>  drivers/timer/stm32_timer.c   |  6 ++
>>  drivers/timer/timer-uclass.c  |  3 ++-
>>  drivers/timer/tsc_timer.c |  6 ++
>>  include/timer.h   |  5 ++---
>>  23 files changed, 53 insertions(+), 93 deletions(-)
>>
> 
> Reviewed-by: Simon Glass 
> 
> This seems OK to me. The function should not be called until the
> driver is probed. It might be worth mentioning that probing must make
> the timer valid (always from then on)?
> 

Sure. I will update the docs in the next revision.

--Sean


Re: [PATCH] time: Fix get_ticks being non-monotonic

2020-09-08 Thread Sean Anderson
On 9/8/20 7:56 PM, Simon Glass wrote:
> Hi Sean,
> 
> On Mon, 7 Sep 2020 at 09:51, Sean Anderson  wrote:
>>
>> On 9/7/20 9:57 AM, Simon Glass wrote:
>>> Hi Sean,
>>>
>>> On Sun, 6 Sep 2020 at 20:02, Sean Anderson  wrote:

 On 9/6/20 9:43 PM, Simon Glass wrote:
> Hi Sean,
>
> On Tue, 1 Sep 2020 at 13:56, Sean Anderson  wrote:
>>
>> get_ticks does not always succeed. Sometimes it can be called before the
>> timer has been initialized. If it does, it returns a negative errno.
>> This causes the timer to appear non-monotonic, because the value will
>> become much smaller after the timer is initialized.
>>
>> No users of get_ticks which I checked handle errors of this kind. 
>> Further,
>> functions like tick_to_time mangle the result of get_ticks, making it 
>> very
>> unlikely that one could check for an error without suggesting a patch 
>> such
>> as this one.
>>
>> This patch changes get_ticks to always return 0 when there is an error.
>> 0 is the least unsigned integer, ensuring get_ticks appears monotonic. 
>> This
>> has the side effect of time apparently not passing until the timer is
>> initialized. However, without this patch, time does not pass anyway,
>> because the error value is likely to be the same.
>>
>> Fixes: c8a7ba9e6a5
>> Signed-off-by: Sean Anderson 
>> ---
>>
>>  lib/time.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> Would it be better to panic so people can fix the bug?

 I thought this was expected behavior. It's only a bug if you do
 something like udelay before any timers are created. We just can't
 report errors through get_ticks, because its users assume that it always
 returns a time of some kind.
>>>
>>> I think it indicates a bug. If you use a device before it is ready you
>>> don't really know what it will do. I worry that this patch is just
>>> going to cause confusion, since the behaviour depends on when you call
>>> it. If we panic, people can figure out why the timer is being inited
>>> too late, or being used too early.
>>
>> Hm, maybe. I don't think it's as clear cut as "us[ing] a device before
>> it is ready," because get_ticks tries to initialize the timer if it
>> isn't already initialized. Unless someone else does it first, the first
>> call to get_ticks will always be before the timer is initialized.
>>
>> The specific problem I ran into was that after relocation, the watchdog
>> may be initialized before the timer. This occurs on RISC-V because
>> without [1] a timer only exists after arch_early_init_r. So, for the
>> first few calls to watchdog_reset there is no timer.
>>
>> The second return could probably be turned into a panic. I checked, and
>> all current timer drivers always succeed in getting the time (except for
>> the RISC-V timer, which is fixed in [1]), so the only way for
>> timer_get_count to fail is if timer_ops.get_count doesn't exist. That is
>> almost certainly an error on the driver author's part, so I think
>> panicking there is the only reasonable option.
> 
> OK good, let's do that and update docs in timer.h

That being to panic both times, or just panic the second time?

--Sean



Re: [PATCH] time: Fix get_ticks being non-monotonic

2020-09-08 Thread Simon Glass
Hi Sean,

On Mon, 7 Sep 2020 at 09:51, Sean Anderson  wrote:
>
> On 9/7/20 9:57 AM, Simon Glass wrote:
> > Hi Sean,
> >
> > On Sun, 6 Sep 2020 at 20:02, Sean Anderson  wrote:
> >>
> >> On 9/6/20 9:43 PM, Simon Glass wrote:
> >>> Hi Sean,
> >>>
> >>> On Tue, 1 Sep 2020 at 13:56, Sean Anderson  wrote:
> 
>  get_ticks does not always succeed. Sometimes it can be called before the
>  timer has been initialized. If it does, it returns a negative errno.
>  This causes the timer to appear non-monotonic, because the value will
>  become much smaller after the timer is initialized.
> 
>  No users of get_ticks which I checked handle errors of this kind. 
>  Further,
>  functions like tick_to_time mangle the result of get_ticks, making it 
>  very
>  unlikely that one could check for an error without suggesting a patch 
>  such
>  as this one.
> 
>  This patch changes get_ticks to always return 0 when there is an error.
>  0 is the least unsigned integer, ensuring get_ticks appears monotonic. 
>  This
>  has the side effect of time apparently not passing until the timer is
>  initialized. However, without this patch, time does not pass anyway,
>  because the error value is likely to be the same.
> 
>  Fixes: c8a7ba9e6a5
>  Signed-off-by: Sean Anderson 
>  ---
> 
>   lib/time.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> Would it be better to panic so people can fix the bug?
> >>
> >> I thought this was expected behavior. It's only a bug if you do
> >> something like udelay before any timers are created. We just can't
> >> report errors through get_ticks, because its users assume that it always
> >> returns a time of some kind.
> >
> > I think it indicates a bug. If you use a device before it is ready you
> > don't really know what it will do. I worry that this patch is just
> > going to cause confusion, since the behaviour depends on when you call
> > it. If we panic, people can figure out why the timer is being inited
> > too late, or being used too early.
>
> Hm, maybe. I don't think it's as clear cut as "us[ing] a device before
> it is ready," because get_ticks tries to initialize the timer if it
> isn't already initialized. Unless someone else does it first, the first
> call to get_ticks will always be before the timer is initialized.
>
> The specific problem I ran into was that after relocation, the watchdog
> may be initialized before the timer. This occurs on RISC-V because
> without [1] a timer only exists after arch_early_init_r. So, for the
> first few calls to watchdog_reset there is no timer.
>
> The second return could probably be turned into a panic. I checked, and
> all current timer drivers always succeed in getting the time (except for
> the RISC-V timer, which is fixed in [1]), so the only way for
> timer_get_count to fail is if timer_ops.get_count doesn't exist. That is
> almost certainly an error on the driver author's part, so I think
> panicking there is the only reasonable option.

OK good, let's do that and update docs in timer.h

>
> (Does get_count even need to have a return value? I think it's
> reasonable to always expect the timer to return a value.)

I saw your patch, seems OK.

Regards,
Simon

>
> --Sean
>
> [1] https://patchwork.ozlabs.org/project/uboot/list/?series=198797


Re: [PATCH v4 1/3] binman: Allow selecting default FIT configuration

2020-09-08 Thread Simon Glass
Hi Alper,

On Tue, 8 Sep 2020 at 11:33, Alper Nebi Yasak  wrote:
>
> On 06/09/2020 19:39, Simon Glass wrote:
> > Add a new entry argument to the fit entry which allows selection of the
> > default configuration to use. This is the 'default' property in the
> > 'configurations' node.
> >
> > Update the Makefile to pass in the value of DEVICE_TREE or
> > CONFIG_DEFAULT_DEVICE_TREE to provide this information.
> >
> > Signed-off-by: Simon Glass 
> > Suggested-by: Michal Simek 
> > ---
> >
> > Changes in v4:
> > - Add more documentation for DEFAULT-SEQ
>
> I might be too late to say this but the SEQ thing looks ugly to me.
> Maybe there could be some generic control-flow-like nodes that could
> generate and insert things in their own place. If it makes sense, I'm
> imagining something like:
>
> fit {
> images {
> __for__ {
> for,variable = "name";
> for,in-args = "of-list";
>
> fdt-#name {
> description = "fdt-$name.dtb";
> type = "flat_dt";
> compression = "none";
> };
> };
> };
>
> configurations {
> __for__ {
> for,variable = "name"
> for,in-args = "of-list";
>
> __if__ {
> if,arg-equals = "default-dt", "$name";
> default = "config-#name";
> };
>
> config-#name {
> description = "conf-$name.dtb";
> fdt = "fdt-#name";
> };
> };
> };
> };

I certainly like the flexibility of this and I fiddled with something
similar (without the __if__ as I didn't have 'default support) before
going with a hard-coded approach. I agree what I have is ugly and I'm
pleased to get some feedback.

I think we could use _for instead of __for__.

For $name I avoided that because it only works if there is a
non-character afterwards, e.g. $namex is ambiguous. I briefly played
with ${name} and {name} but ended up with the ugly capitals.

The biggest question in my mind is whether we want our device-tree
templates to be turing-complete. It seems nice but I feel that what
you have above is much harder to understand and get right.

Regards,
Simon


Re: [PATCH v4 2/3] binman: Support help messages for missing blobs

2020-09-08 Thread Simon Glass
Hi Alper,

On Tue, 8 Sep 2020 at 10:37, Alper Nebi Yasak  wrote:
>
> On 06/09/2020 19:39, Simon Glass wrote:
> > When an external blob is missing it can be quite confusing for the user.
> > Add a way to provide a help message that is shown.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > (no changes since v3)
> >
> > Changes in v3:
> > - Add a way to show help messages for missing blobs
> >
> >  tools/binman/README|  6 ++
> >  tools/binman/control.py| 69 +-
> >  tools/binman/entry.py  |  9 +++
> >  tools/binman/ftest.py  | 18 +-
> >  tools/binman/missing-blob-help | 11 
> >  tools/binman/test/168_fit_missing_blob.dts |  9 ++-
> >  6 files changed, 119 insertions(+), 3 deletions(-)
> >  create mode 100644 tools/binman/missing-blob-help
> >
> > diff --git a/tools/binman/README b/tools/binman/README
> > index 37ee3fc2d3b..f7bf285a915 100644
> > --- a/tools/binman/README
> > +++ b/tools/binman/README
> > @@ -343,6 +343,12 @@ compress:
> >   Sets the compression algortihm to use (for blobs only). See the entry
> >   documentation for details.
> >
> > +missing-msg:
> > + Sets the tag of the message to show if this entry is missing. This is
> > + used for external blobs. When they are missing it is helpful to show
> > + information about what needs to be fixed. See missing-blob-help for 
> > the
> > + message for each tag.
> > +
> >  The attributes supported for images and sections are described below. 
> > Several
> >  are similar to those for entries.
> >
> > diff --git a/tools/binman/control.py b/tools/binman/control.py
> > index 15bfac582a7..ee5771e7292 100644
> > --- a/tools/binman/control.py
> > +++ b/tools/binman/control.py
> > @@ -9,6 +9,7 @@ from collections import OrderedDict
> >  import glob
> >  import os
> >  import pkg_resources
> > +import re
> >
> >  import sys
> >  from patman import tools
> > @@ -22,6 +23,11 @@ from patman import tout
> >  # Make this global so that it can be referenced from tests
> >  images = OrderedDict()
> >
> > +# Help text for each type of missing blob, dict:
> > +#key: Value of the entry's 'missing-msg' or entry name
> > +#value: Text for the help
> > +missing_blob_help = {}
> > +
> >  def _ReadImageDesc(binman_node):
> >  """Read the image descriptions from the /binman node
> >
> > @@ -54,6 +60,66 @@ def _FindBinmanNode(dtb):
> >  return node
> >  return None
> >
> > +def _ReadMissingBlobHelp():
> > +"""Read the missing-blob-help file
> > +
> > +This file containins help messages explaining what to do when external 
> > blobs
> > +are missing.
> > +
> > +Returns:
> > +Dict:
> > +key: Message tag (str)
> > +value: Message text (str)
> > +"""
> > +
> > +def _FinishTag(tag, msg, result):
> > +if tag:
> > +result[tag] = msg.rstrip()
> > +tag = None
> > +msg = ''
> > +return tag, msg
> > +
> > +my_data = pkg_resources.resource_string(__name__, 'missing-blob-help')
> > +re_tag = re.compile('^([-a-z0-9]+):$')
> > +result = {}
> > +tag = None
> > +msg = ''
> > +for line in my_data.decode('utf-8').splitlines():
> > +if not line.startswith('#'):
> > +m_tag = re_tag.match(line)
> > +if m_tag:
> > +_, msg = _FinishTag(tag, msg, result)
> > +tag = m_tag.group(1)
> > +elif tag:
> > +msg += line + '\n'
> > +_FinishTag(tag, msg, result)
> > +return result
>
> This looks a bit complex, I think something like this would be clearer:
>
> result = {}
> tag = None
> for line in my_data.decode('utf-8').splitlines():
> m_tag = re_tag.match(line)
> if line.startswith('#'):
> continue
> elif m_tag:
> tag = m_tag.group(1)
> result[tag] = []
> elif tag:
> result[tag].append(line)
> for tag, lines in result.items():
> result[tag] = "".join(lines).rstrip()
> return result
>

Yes that is easier, thank you. I'll use this in v4 and perhaps you can
reply with your sign-off.

Regards,
Simon


Re: [PATCH v4 2/2] test: gpio: Add tests for the managed API

2020-09-08 Thread Simon Glass
On Mon, 7 Sep 2020 at 23:40, Pratyush Yadav  wrote:
>
> From: Jean-Jacques Hiblot 
>
> Add a test to verify that GPIOs can be acquired/released using the managed
> API. Also check that the GPIOs are released when the consumer device is
> removed.
>
> Signed-off-by: Jean-Jacques Hiblot 
> Signed-off-by: Pratyush Yadav 
> ---
>  arch/sandbox/dts/test.dts |  10 
>  test/dm/bus.c |   2 +-
>  test/dm/gpio.c| 102 ++
>  test/dm/test-fdt.c|   6 +--
>  4 files changed, 116 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass 

Although I don't see a change log, so this is a bit blind.


Re: [PATCH 1/1] lib: rsa: fix data abort in br_i32_decode()

2020-09-08 Thread Simon Glass
HI Heinrich,

On Tue, 8 Sep 2020 at 04:29, Heinrich Schuchardt  wrote:
>
> After removing leading zeros the RSA modulus may be unaligned. On
> architectures like ARM 32bit unaligned access may lead to a data abort,
> e.g. when executing 'ut lib lib_asn1_pkcs7'.
>
> Use memcpy() to transfer from unaligned to aligned memory.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  lib/rsa/rsa-keyprop.c | 22 ++
>  1 file changed, 14 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass 

>
> diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
> index 1e83eedc82..6153cb00b3 100644
> --- a/lib/rsa/rsa-keyprop.c
> +++ b/lib/rsa/rsa-keyprop.c
> @@ -17,23 +17,29 @@
>  #include 
>
>  /**
> - * br_dec16be() - Convert 16-bit big-endian integer to native
> - * @src:   Pointer to data
> - * Return: Native-endian integer
> + * br_dec16be() - convert unaligned 16-bit big-endian integer to native
> + * @src:   unaligned pointer to data
> + * Return: native-endian 16-bit integer
>   */
>  static unsigned br_dec16be(const void *src)
>  {
> -   return be16_to_cpup(src);
> +   u16 buf;
> +
> +   memcpy(, src, sizeof(buf));
> +   return be16_to_cpu(buf);

Is it possible to use __get_unaligned_be() ?

Regards,
Simon


Re: [PATCH] timer: Return count from timer_ops.get_count

2020-09-08 Thread Simon Glass
Hi Sean,


On Mon, 7 Sep 2020 at 12:19, Sean Anderson  wrote:
>
> No timer drivers return an error from get_count. Instead of possibly
> returning an error, just return the count directly.
>
> Signed-off-by: Sean Anderson 
> ---
> Passing CI (but not otherwise tested):
> https://dev.azure.com/seanga2/u-boot/_build/results?buildId=25=results
>
> This patch depends on
> https://patchwork.ozlabs.org/project/uboot/list/?series=198797
>
>  arch/riscv/lib/andes_plmt.c   |  6 ++
>  arch/riscv/lib/sifive_clint.c |  6 ++
>  drivers/timer/ag101p_timer.c  |  5 ++---
>  drivers/timer/altera_timer.c  |  6 ++
>  drivers/timer/arc_timer.c |  6 ++
>  drivers/timer/ast_timer.c |  6 ++
>  drivers/timer/atcpit100_timer.c   |  5 ++---
>  drivers/timer/atmel_pit_timer.c   |  6 ++
>  drivers/timer/cadence-ttc.c   |  6 ++
>  drivers/timer/dw-apb-timer.c  |  6 ++
>  drivers/timer/mpc83xx_timer.c |  6 ++
>  drivers/timer/mtk_timer.c |  6 ++
>  drivers/timer/nomadik-mtu-timer.c |  6 ++
>  drivers/timer/omap-timer.c|  6 ++
>  drivers/timer/ostm_timer.c|  6 ++
>  drivers/timer/riscv_timer.c   | 21 +
>  drivers/timer/rockchip_timer.c|  5 ++---
>  drivers/timer/sandbox_timer.c |  6 ++
>  drivers/timer/sti-timer.c |  6 ++
>  drivers/timer/stm32_timer.c   |  6 ++
>  drivers/timer/timer-uclass.c  |  3 ++-
>  drivers/timer/tsc_timer.c |  6 ++
>  include/timer.h   |  5 ++---
>  23 files changed, 53 insertions(+), 93 deletions(-)
>

Reviewed-by: Simon Glass 

This seems OK to me. The function should not be called until the
driver is probed. It might be worth mentioning that probing must make
the timer valid (always from then on)?

Regards,
Simon


Re: [PATCH v1] x86: edison: Move config SYS_MALLOC_LEN to Kconfig

2020-09-08 Thread Simon Glass
On Tue, 8 Sep 2020 at 07:57, Andy Shevchenko
 wrote:
>
> This patch moves the the config SYS_MALLOC_LEN to Kconfig
> as it is already done for zynq arch in commit 01aa5b8f0503
> ("Kconfig: Move config SYS_MALLOC_LEN to Kconfig for zynq").
>
> Signed-off-by: Andy Shevchenko 
> ---
>  board/intel/edison/Kconfig | 3 +++
>  include/configs/edison.h   | 4 
>  2 files changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH v7 05/21] arm: mvebu: x530: Use tiny SPI NOR

2020-09-08 Thread Chris Packham

On 7/09/20 3:05 pm, Pratyush Yadav wrote:
> On 06/09/20 08:34PM, Chris Packham wrote:
>> On 5/09/20 3:39 am, Pratyush Yadav wrote:
>>> Chris,
>>>
>>> On 04/09/20 09:04PM, Pratyush Yadav wrote:
 The SPI NOR core will get Octal DTR in following commits. This has
 presented a significant challenge of keeping the SPL size in check on
 the x530 platform.

 On a previous iteration of the series, adding a set of compile-time
 switches got the build working. But rebasing on the latest master breaks
 the build again. We are fighting a losing battle here. Every addition to
 either the SPI NOR core in the future, or any other core part of U-Boot
 will potentially lead to the SPL size going beyond the limit and the
 build failing.

 To combat this we will have to keep adding more and more compile-time
 switches, increasing the complexity of the code in the process. This is
 not sustainable. So use tiny SPI NOR instead. It is designed with
 size-limited SPL binaries in mind, and will afford us more breathing
 room.

 To enable tiny SPI NOR, CONFIG_SPI_FLASH_BAR has to be disabled.

 Signed-off-by: Pratyush Yadav 
 ---
configs/x530_defconfig | 1 -
1 file changed, 1 deletion(-)
>>> Can you please test these changes on your board and confirm that tiny
>>> SPI NOR works for you?
>>>
>> I'll take a look when I get a chance. I'm not actually sure what the
>> true SPL size limit is for Armada-385. It's possible we could avoid the
>> build issues simply by bumping the limit up.
> Bumping the limit up would be the best solution. Please check if we can
> do that.

I've reached out to Marvell to see if they can tell me what the actual 
size limit is.

Stefan, do you happen to know? The Armada-XP talks about using the L2 
cache as SRAM but it's not immediately clear if that's whats going on 
with the Armada-385.

 diff --git a/configs/x530_defconfig b/configs/x530_defconfig
 index 890c94b5c1..0570dbe9ea 100644
 --- a/configs/x530_defconfig
 +++ b/configs/x530_defconfig
 @@ -62,7 +62,6 @@ CONFIG_SYS_NAND_USE_FLASH_BBT=y
CONFIG_NAND_PXA3XX=y
CONFIG_SF_DEFAULT_BUS=1
CONFIG_SF_DEFAULT_SPEED=5000
 -CONFIG_SPI_FLASH_BAR=y
CONFIG_SPI_FLASH_MACRONIX=y
CONFIG_SPI_FLASH_STMICRO=y
CONFIG_SPI_FLASH_SST=y

Re: [PATCH] defconfig: espressobin: enable NET_RANDOM_ETHADDR

2020-09-08 Thread Pali Rohár
On Tuesday 08 September 2020 08:52:56 Tom Rini wrote:
> On Tue, Sep 08, 2020 at 10:14:15AM +0200, Andre Heider wrote:
> > On 08/09/2020 09:42, Pali Rohár wrote:
> > > On Tuesday 08 September 2020 08:35:00 Andre Heider wrote:
> > > > The hardware does not provide a MAC address. Enable this so that
> > > > network access works with just the default environment.
> > > 
> > > Well, this is not fully truth as MAC address is stored in SPI, just in
> > > non-standard format, in U-Boot env stored in env partition and it is
> > > hard to use outside of U-Boot, plus easy to erase / overwrite / lost.
> > 
> > True, but updating the bootloader usually implies wiping the env, so it's
> > very easy to lose it.
> 
> It most certainly should NOT wipe out the existing environment, it
> should be using the same environment location as before.

The main issue is that downstream distributions are doing it. Plus they
probably have custom U-Boot patches. Nothing which mainline U-Boot can
forbid or change.

Also if somebody is updating from old Marvell factory U-Boot to mainline
U-Boot, SPI env offset may be different and therefore it wipes env.

And also, if U-Boot is upgrading from ancient version to new, it is
suggested to reset env to have e.g. new distroboot support.

So any of above situation can lead to erasing env and therefore it is a
very good idea to create backup of MAC address(es).

> > > I'm not a big fan of this change. This looks like a workaround / hack
> > > for boards where MAC address was erased (e.g. by broken U-Boot distro
> > > scripts) or for early boards where MAC address was not written at all
> > > (as I was told).
> > > 
> > > And on these boards this patch would cause that U-Boot would see on
> > > every boot different MAC address. This would cause another mess in
> > > network for U-Boot netboot as DHCP/TFTP server would see for one board
> > > every time different MAC address.
> > > 
> > > Is not really better to instruct user how to fix board where e.g. broken
> > > distro scripts erased MAC address? We have already paragraph in
> > > README.marvell about it.
> > > 
> > > Also this change affects "default" defconfig value. And based on above
> > > arguments I do not think that this change should be enabled by default.
> > > 
> > > I understand that for some situations it may be useful (e.g. mass board
> > > reparation process via netboot), but as this is config option, users in
> > > such situation can enable this option manually.
> > > 
> > > I think that for default behavior is not provide network access in
> > > U-Boot if for some reasons factory permanent MAC address was removed.
> > > User can easier and faster detect this issue and fix it.
> > 
> > It can be argued both ways I guess. If this option wouldn't make sense it
> > wouldn't exist.

As I wrote, I understand that this option is useful in some situations.

> > Out of the box working network access is probably what most users care
> > about.

Of course! But what does "working network access" means? Somebody wants
to have DHCP, even semi/half-working and does not care about RFC
compliance. Somebody does not want to have mess in the network and so
every device must use only assigned MAC / IP (v4/v6) address and not any
different. And somebody just to want working TFTP, does not care about
DHCP, IP or MAC address.

And for some of these situation is CONFIG_NET_RANDOM_ETHADDR great, for
some harmless and for other is harmful (in context that all these
devices got assigned MAC address in factory).

Also I know that in some networks is forbidden to use other than factory
MAC address (users are not allowed to connect device with changed MAC
address).

> > Changing this default means that you need to build the whole firmware
> > yourself, and let's face it: building it for this board is a total
> > clusterfuck. Most users will just download a binary. I don't care too much
> > for this patch, but I would consider what fits most users.
> > 
> > Right now most users will probably run the downstream binaries provided by
> > armbian, and as you know, that even has single hardcoded MAC, used for all
> > boards. So this would already be an improvement ;)

I think that we do not have to care about people who use downstream
patched U-Boot. In most case default U-Boot defconfig may be changed or
be different.

Change is in this patch is only about default mainline U-Boot
configuration. I think it should be sane default and for me sane default
is the least surprise. So if configuration is missing either throw error
so can immediately fix it or make it predicable / deterministic.

> Note that when CONFIG_NET_RANDOM_ETHADDR is set, we only use a random
> MAC address when we haven't found one either on the hardware or
> environment.

I know.

> It also prints a warning that you are using a random MAC,
> so if it's documented on how to recover the real MAC a user should see
> that warning and fix it.

In case you did backup of MAC address or you have MAC 

Re: [PATCH v2 2/3] arm64: Bail out PIE builds early if load address is not 4K aligned

2020-09-08 Thread Stephen Warren
On 9/7/20 3:52 AM, Edgar E. Iglesias wrote:
> On Fri, Sep 04, 2020 at 12:43:57PM -0600, Stephen Warren wrote:
>> On 9/4/20 3:07 AM, Edgar E. Iglesias wrote:
>>> From: "Edgar E. Iglesias" 
>>>
>>> PIE requires a 4K aligned load address. If this is not met, trap
>>> the startup sequence in a WFI loop rather than running into obscure
>>> failures.
>>
>>> diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
>>>  #if CONFIG_POSITION_INDEPENDENT
>>> +   /* Verify that we're 4K aligned.  */
>>
>> Similar to the comment on the previous patch: I believe the code that
>> implements this check should be outside the #if check, since it's always
>> needed.
> 
> But a check for non-PIE would have to be stricter, wouldn't it?
> I.e the load address needs to exactly match the link-time address.

Oh yes, I guess that is true.


Re: [PATCH U-BOOT v3 00/30] PLEASE TEST fs: btrfs: Re-implement btrfs support using code from btrfs-progs

2020-09-08 Thread Tom Rini
On Wed, Jun 24, 2020 at 06:02:46PM +0200, Marek Behún wrote:

> Hello,
> 
> this is a cleaned up version of Qu's patches that reimplements U-Boot's
> btrfs driver with code from btrfs-progs.
> 
> I have tested this series, found and corrected one bug (failure when
> accesing files via symlinks), and it looks it is working now, but I
> would like more people to do some testing.
> 
> There are a lot of checkpatch warnings and errors left, I shall fix
> this in the future.
> 
> Marek
> 
> Changes since v2:
> - fixed btrfs_lookup_path() in patch 19 to correctly handle symlinks
> - commit messages were updated some
>   - for example they used the word "crossport" in 3 formats:
> "crossport", "cross-port" and "cross port", this was changed
> to "crossport"
>   - corrected some typos
>   - some English sentences were a bit weirdly written
> - fixed 2 compiler warnings (one use of maybe uninitialized variable and
>   one printf specifier warning)
> - indentation in some places was wrong (usage of 8 spaces instead of a
>   tab, for example)
> - added my Reviewed-by
> - the last patch, adding btrfs mailing list to MAINTAINRES, also adds
>   Qu as designated reviewer, so that people add him to Cc when they send
>   patches
> 
> Changes since v1:
> - Implement btrfs_list_subvols()
>   In v1 it's completely removed thus would cause problem if btrfsolume
>   command is compiled in.
> - Rebased to latest master
>   Only minor conflicts due to header changes.
> - Allow next_legnth() to return value > BTRFS_NAME_LEN
> 
> Below is Qu's explanation, from cover letter of v2:
> 
> The current btrfs code in U-boot is using a creative way to read on-disk
> data.
> It's pretty simple, involving the least amount of code, but pretty
> different from btrfs-progs nor kernel, making it pretty hard to sync
> code between different projects.
> 
> This big patchset will rework the btrfs support, to use code mostly from
> btrfs-progs, thus all existing btrfs developers will feel at home.
> 
> During the rework, the following new features are added:
> - More hash algorithm support
>   SHA256 and XXHASH support are added.
>   BLAKE2 needs more backport, will happen in a separate patchset.
> 
> - The ability to read degraded RAID1
>   Although we still only support one device btrfs, the new code base
>   can choose from different copies already.
>   As long as new device scan interface is provided, multi-device support
>   is pretty simple.
> 
> - More correct handling of compressed extents with offset
>   For compressed extent with offset, we should read the whole compressed
>   extent, decompress them, then copy the referred part.
> 
> There are some more features incoming, with the new code base it would
> be much easier to implement:
> - Data checksum support
>   The check would happen in read_extent_data(), btrfs-progs has the
>   needed facility to locate data csum.
> 
> - BLAKE2 support
>   Need BLAKE2 library cross ported.
>   For btrfs it's just several lines of modification.
> 
> - Multi-device support along wit degraded RAID support
>   We only need an interface to scan one device without opening it.
>   The infrastructure is already provided in this patchset.
> 
> These new features would be submitted after the patchset get merged,
> since the patchset is already large, I don't want to make it more scary.
> 
> Although this patchset look horribly large, most of them are code copy
> from btrfs-progs.
> E.g extent-cache.[ch], rbtree-utils.[ch], btrfs_btree.h.
> And ctree.h has over 1000 lines copied just for various accessors.
> 
> While for disk-io.[ch] and volumes-io.[ch], they have some small
> modifications to adapt the interface of U-boot.
> E.g. btrfs_device::fd is replace with blkdev_desc and disk_partition_t.
> 
> The new code for U-boot are related to the following functions:
> - btrfs_readlink()
> - btrfs_lookup_path()
> - btrfs_read_extent_inline()
> - btrfs_read_extent_reg()
> - lookup_data_extent()
> - btrfs_file_read()
> - btrfs_list_subvols()
> 
> Qu Wenruo (30):
>   fs: btrfs: Sync btrfs_btree.h from kernel
>   fs: btrfs: Add more checksum algorithms
>   fs: btrfs: Crossport btrfs_read_dev_super() from btrfs-progs
>   fs: btrfs: Crossport rbtree-utils from btrfs-progs
>   fs: btrfs: Crossport extent-cache.[ch] from btrfs-progs
>   fs: btrfs: Crossport extent-io.[ch] from btrfs-progs
>   fs: btrfs: Crossport structure accessor into ctree.h
>   fs: btrfs: Crossport volumes.[ch] from btrfs-progs
>   fs: btrfs: Crossport read_tree_block() from btrfs-progs
>   fs: btrfs: Rename struct btrfs_path to struct __btrfs_path
>   fs: btrfs: Rename btrfs_root to __btrfs_root
>   fs: btrfs: Crossport struct btrfs_root to ctree.h
>   fs: btrfs: Crossport btrfs_search_slot() from btrfs-progs
>   fs: btrfs: Crossport btrfs_read_sys_array() and
> btrfs_read_chunk_tree()
>   fs: btrfs: Crossport open_ctree_fs_info() from btrfs-progs
>   fs: btrfs: Rename path resolve related functions to avoid name
> conflicts
>   fs: 

Re: [PATCH v7 1/2] board: kontron: add sl28 support

2020-09-08 Thread Tom Rini
On Mon, Sep 07, 2020 at 11:07:58PM +0200, Michael Walle wrote:

> Add basic support for the Kontron SMARC-sAL28 board. This includes just
> the bare minimum to be able to bring up the board and boot linux.
> 
> For now, the Single and Dual PHY variant is supported. Other variants
> will fall back to the basic variant.
> 
> In particular, there is no watchdog support for now. This means that you
> have to disable the default watchdog, otherwise you'll end up in the
> recovery bootloader. See the board README for details.
> 
> Signed-off-by: Michael Walle 

Thanks for updating things.

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v4 1/3] binman: Allow selecting default FIT configuration

2020-09-08 Thread Alper Nebi Yasak
On 06/09/2020 19:39, Simon Glass wrote:
> Add a new entry argument to the fit entry which allows selection of the
> default configuration to use. This is the 'default' property in the
> 'configurations' node.
> 
> Update the Makefile to pass in the value of DEVICE_TREE or
> CONFIG_DEFAULT_DEVICE_TREE to provide this information.
> 
> Signed-off-by: Simon Glass 
> Suggested-by: Michal Simek 
> ---
> 
> Changes in v4:
> - Add more documentation for DEFAULT-SEQ

I might be too late to say this but the SEQ thing looks ugly to me.
Maybe there could be some generic control-flow-like nodes that could
generate and insert things in their own place. If it makes sense, I'm
imagining something like:

fit {
images {
__for__ {
for,variable = "name";
for,in-args = "of-list";

fdt-#name {
description = "fdt-$name.dtb";
type = "flat_dt";
compression = "none";
};
};
};

configurations {
__for__ {
for,variable = "name"
for,in-args = "of-list";

__if__ {
if,arg-equals = "default-dt", "$name";
default = "config-#name";
};

config-#name {
description = "conf-$name.dtb";
fdt = "fdt-#name";
};
};
};
};


Re: [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Marek Vasut
On 9/8/20 7:14 PM, Frank Wunderlich wrote:
> 
> 
> Am 8. September 2020 18:47:47 MESZ schrieb Marek Vasut :
> 
>> Resend please, if I wanted to comment on any of those patches, I won't
>> be able.
> 
> Have resend with bit reduced recipients (afair ryder and weijie are in GSS 
> mailinglist),because this floods others mailbox. It's only a workaround.
> 
> Have you found problem or any idea why you not able to receive from chunfeng 
> directly?

No, however I have no problems receiving emails from others.


Re: make_fit_atf.py and multiple U-Boot ELF segments

2020-09-08 Thread Simon Glass
Hi Tom,

On Tue, 8 Sep 2020 at 07:31, Tom Rini  wrote:
>
> Hey all,
>
> As part of reviewing
> http://patchwork.ozlabs.org/project/uboot/list/?series=187450=*
> I've run in to an odd problem.  With patch 4/7 of that series, building
> firefly-rk3399 fails on the make_fit_atf.py step as those changes end up
> with a U-Boot ELF that has len(segments) of 2, rather than 1, and the
> generator blows up.  It's not clear to me if we can just check for
> non-zero number of segments here or what.  Thanks!

I don't see that but it is probably a toolchain difference.

I haven't looked at the script in detail, but we should convert it to
binman soon and add some tests. It seems odd to me that the number of
LOAD sections could change because of that patch, if that is actually
what is happening.

Regards,
Simon


Re: [PATCH] ARM: at91: common: guard ATMEL_PIT code by ifdef

2020-09-08 Thread Eugen.Hristev
On 20.08.2020 16:11, Eugen Hristev wrote:
> Atmel PIT timer is not available for next products that
> have another timer hardware block.
> To be able to use the common at91 code, guard the code that uses PIT
> by ifdefs.
> 
> Signed-off-by: Eugen Hristev 
> ---

Applied to u-boot-atmel/next


Re: [PATCH v3 00/21] clk: at91: add sama7g5 support

2020-09-08 Thread Eugen.Hristev
On 07.09.2020 17:46, Claudiu Beznea wrote:
> The purpose of this series is to add clock support for SAMA7G5.
> Along with this, clock drivers were switched to CCF and aligned
> with their corresponding versions present in Linux.
> Some changes were done for CCF, patches 1, 3, 4, 5 (I don't know
> if they were as is by intention of a fixes tag is needed in there).
> 
> Also patch 3/21 has been added to support clock re-reparenting (this
> is minimal support and hope it doesn't break anything if used).
> 
> Thank you,
> Claudiu Beznea
> 
> Changes in v3:
> - collected remaining Reviewed-by tags
> - rebase on top of latest master branch:
>e5df264e7aac ("Merge 
> https://gitlab.denx.de/u-boot/custodians/u-boot-marvell;)
>to adapt patch 8/21 at changes from commit
>702e57e113d8 ("treewide: convert devfdt_get_addr_ptr() to 
> dev_read_addr_ptr()")
> 
> Changes in v2:
> - use assert() in patch 1/21
> - drop patch 2/22 from previous series
> - add sandbox tests for clock reparenting, device reparenting and
>critical clocks
> - adapt review comments
> - collected Reviewed-by tags
> 
> Claudiu Beznea (21):
>clk: check hw and hw->dev before dereference it
>dm: core: add support for device re-parenting
>clk: bind clk to new parent device
>clk: do not disable clock if it is critical
>clk: get clock pointer before proceeding
>clk: at91: add pre-requisite headers for AT91 clock architecture
>clk: at91: pmc: add helpers for clock drivers
>clk: at91: move clock code to compat.c
>clk: at91: sckc: add driver compatible with ccf
>clk: at91: clk-main: add driver compatible with ccf
>clk: at91: sam9x60-pll: add driver compatible with ccf
>clk: at91: clk-master: add driver compatible with ccf
>clk: at91: clk-master: add support for sama7g5
>clk: at91: clk-utmi: add driver compatible with ccf
>clk: at91: clk-utmi: add support for sama7g5
>clk: at91: clk-programmable: add driver compatible with ccf
>clk: at91: clk-system: add driver compatible with ccf
>clk: at91: clk-peripheral: add driver compatible with ccf
>clk: at91: clk-generic: add driver compatible with ccf
>clk: at91: pmc: add generic clock ops
>clk: at91: sama7g5: add clock support
> 
>   drivers/clk/at91/Kconfig|7 +
>   drivers/clk/at91/Makefile   |   15 +-
>   drivers/clk/at91/clk-generated.c|  178 -
>   drivers/clk/at91/clk-generic.c  |  202 +
>   drivers/clk/at91/clk-h32mx.c|   56 --
>   drivers/clk/at91/clk-main.c |  381 +-
>   drivers/clk/at91/clk-master.c   |  331 -
>   drivers/clk/at91/clk-peripheral.c   |  291 ++--
>   drivers/clk/at91/clk-plla.c |   54 --
>   drivers/clk/at91/clk-plladiv.c  |   85 ---
>   drivers/clk/at91/clk-programmable.c |  208 ++
>   drivers/clk/at91/clk-sam9x60-pll.c  |  442 +++
>   drivers/clk/at91/clk-slow.c |   36 -
>   drivers/clk/at91/clk-system.c   |  143 ++--
>   drivers/clk/at91/clk-usb.c  |  147 
>   drivers/clk/at91/clk-utmi.c |  234 --
>   drivers/clk/at91/compat.c   | 1023 +
>   drivers/clk/at91/pmc.c  |  218 +++---
>   drivers/clk/at91/pmc.h  |  140 +++-
>   drivers/clk/at91/sama7g5.c  | 1401 
> +++
>   drivers/clk/at91/sckc.c |  169 -
>   drivers/clk/clk-uclass.c|   51 +-
>   drivers/clk/clk.c   |3 +
>   drivers/core/device.c   |   22 +
>   include/dm/device-internal.h|9 +
>   include/dt-bindings/clk/at91.h  |   22 +
>   include/linux/clk/at91_pmc.h|  247 ++
>   test/dm/clk_ccf.c   |   57 ++
>   test/dm/core.c  |  160 
>   29 files changed, 5407 insertions(+), 925 deletions(-)
>   delete mode 100644 drivers/clk/at91/clk-generated.c
>   create mode 100644 drivers/clk/at91/clk-generic.c
>   delete mode 100644 drivers/clk/at91/clk-h32mx.c
>   delete mode 100644 drivers/clk/at91/clk-plla.c
>   delete mode 100644 drivers/clk/at91/clk-plladiv.c
>   create mode 100644 drivers/clk/at91/clk-programmable.c
>   create mode 100644 drivers/clk/at91/clk-sam9x60-pll.c
>   delete mode 100644 drivers/clk/at91/clk-slow.c
>   delete mode 100644 drivers/clk/at91/clk-usb.c
>   create mode 100644 drivers/clk/at91/compat.c
>   create mode 100644 drivers/clk/at91/sama7g5.c
>   create mode 100644 include/dt-bindings/clk/at91.h
>   create mode 100644 include/linux/clk/at91_pmc.h
> 

Applied whole series to u-boot-atmel/next, thanks !


Re: [PATCH] board: atmel: common: introduce at91_set_eth1addr for second interface

2020-09-08 Thread Eugen.Hristev
On 05.08.2020 15:30, Eugen Hristev wrote:
> We already have a function to retrieve the mac address from one EEPROM.
> For boards with a second Ethernet interface, however, we would
> require another EEPROM with a second unique MAC address.
> Introduce at91_set_eth1addr which will look for a second EEPROM
> and set the 'eth1addr' variable with the obtained MAC address.
> 
> Signed-off-by: Eugen Hristev 
> ---

Applied to u-boot-atmel/next


Re: [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Frank Wunderlich



Am 8. September 2020 18:47:47 MESZ schrieb Marek Vasut :

>Resend please, if I wanted to comment on any of those patches, I won't
>be able.

Have resend with bit reduced recipients (afair ryder and weijie are in GSS 
mailinglist),because this floods others mailbox. It's only a workaround.

Have you found problem or any idea why you not able to receive from chunfeng 
directly?
regards Frank


[PATCH RESEND v4 8/9] usb: xhci: use macros with parameter to fill ep_info2

2020-09-08 Thread Frank Wunderlich
From: Chunfeng Yun 

Use macros with parameter to fill ep_info2, then some macros
for MASK and SHIFT can be removed

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
 drivers/usb/host/xhci-mem.c | 15 +--
 drivers/usb/host/xhci.c |  6 ++
 include/usb/xhci.h  |  6 --
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d627aa..0b49614995 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -825,25 +825,22 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl 
*ctrl,
 
/* Step 4 - ring already allocated */
/* Step 5 */
-   ep0_ctx->ep_info2 = cpu_to_le32(CTRL_EP << EP_TYPE_SHIFT);
+   ep0_ctx->ep_info2 = cpu_to_le32(EP_TYPE(CTRL_EP));
debug("SPEED = %d\n", speed);
 
switch (speed) {
case USB_SPEED_SUPER:
-   ep0_ctx->ep_info2 |= cpu_to_le32(((512 & MAX_PACKET_MASK) <<
-   MAX_PACKET_SHIFT));
+   ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(512));
debug("Setting Packet size = 512bytes\n");
break;
case USB_SPEED_HIGH:
/* USB core guesses at a 64-byte max packet first for FS devices */
case USB_SPEED_FULL:
-   ep0_ctx->ep_info2 |= cpu_to_le32(((64 & MAX_PACKET_MASK) <<
-   MAX_PACKET_SHIFT));
+   ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(64));
debug("Setting Packet size = 64bytes\n");
break;
case USB_SPEED_LOW:
-   ep0_ctx->ep_info2 |= cpu_to_le32(((8 & MAX_PACKET_MASK) <<
-   MAX_PACKET_SHIFT));
+   ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(8));
debug("Setting Packet size = 8bytes\n");
break;
default:
@@ -852,9 +849,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
}
 
/* EP 0 can handle "burst" sizes of 1, so Max Burst Size field is 0 */
-   ep0_ctx->ep_info2 |=
-   cpu_to_le32(((0 & MAX_BURST_MASK) << MAX_BURST_SHIFT) |
-   ((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
+   ep0_ctx->ep_info2 |= cpu_to_le32(MAX_BURST(0) | ERROR_COUNT(3));
 
trb_64 = virt_to_phys(virt_dev->eps[0].ring->first_seg->trbs);
ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5f3a0fba4b..fe30101d93 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -618,8 +618,7 @@ static int xhci_set_configuration(struct usb_device *udev)
cpu_to_le32(EP_MAX_ESIT_PAYLOAD_HI(max_esit_payload) |
EP_INTERVAL(interval) | EP_MULT(mult));
 
-   ep_ctx[ep_index]->ep_info2 =
-   cpu_to_le32(ep_type << EP_TYPE_SHIFT);
+   ep_ctx[ep_index]->ep_info2 = cpu_to_le32(EP_TYPE(ep_type));
ep_ctx[ep_index]->ep_info2 |=
cpu_to_le32(MAX_PACKET
(get_unaligned(_desc->wMaxPacketSize)));
@@ -832,8 +831,7 @@ int xhci_check_maxpacket(struct usb_device *udev)
ctrl->devs[slot_id]->out_ctx, ep_index);
in_ctx = ctrl->devs[slot_id]->in_ctx;
ep_ctx = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
-   ep_ctx->ep_info2 &= cpu_to_le32(~((0x & MAX_PACKET_MASK)
-   << MAX_PACKET_SHIFT));
+   ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET(MAX_PACKET_MASK));
ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size));
 
/*
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 07b1aebc69..e1d382369a 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -632,11 +632,8 @@ struct xhci_ep_ctx {
  */
 #defineFORCE_EVENT (0x1)
 #define ERROR_COUNT(p) (((p) & 0x3) << 1)
-#define ERROR_COUNT_SHIFT  (1)
-#define ERROR_COUNT_MASK   (0x3)
 #define CTX_TO_EP_TYPE(p)  (((p) >> 3) & 0x7)
 #define EP_TYPE(p) ((p) << 3)
-#define EP_TYPE_SHIFT  (3)
 #define ISOC_OUT_EP1
 #define BULK_OUT_EP2
 #define INT_OUT_EP 3
@@ -647,13 +644,10 @@ struct xhci_ep_ctx {
 /* bit 6 reserved */
 /* bit 7 is Host Initiate Disable - for disabling stream selection */
 #define MAX_BURST(p)   (((p)&0xff) << 8)
-#define MAX_BURST_MASK (0xff)
-#define MAX_BURST_SHIFT(8)
 #define CTX_TO_MAX_BURST(p)(((p) >> 8) & 0xff)
 #define MAX_PACKET(p)  (((p)&0x) << 16)
 #define MAX_PACKET_MASK(0x)
 #define MAX_PACKET_DECODED(p)  (((p) >> 16) & 0x)
-#define MAX_PACKET_SHIFT   (16)
 
 /* Get max packet size from ep desc. Bit 10..0 specify the max 

[PATCH RESEND v4 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Frank Wunderlich
From: Chunfeng Yun 

Use readx_poll_sleep_timeout() to poll the register status

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
 drivers/usb/host/xhci.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index fe30101d93..3547a9bad1 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
@@ -143,23 +144,19 @@ struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev)
  * @param usec time to wait till
  * @return 0 if handshake is success else < 0 on failure
  */
-static int handshake(uint32_t volatile *ptr, uint32_t mask,
-   uint32_t done, int usec)
+static int
+handshake(uint32_t volatile *ptr, uint32_t mask, uint32_t done, int usec)
 {
uint32_t result;
+   int ret;
+
+   ret = readx_poll_sleep_timeout(xhci_readl, ptr, result,
+(result & mask) == done || result == U32_MAX,
+1, usec);
+   if (result == U32_MAX)  /* card removed */
+   return -ENODEV;
 
-   do {
-   result = xhci_readl(ptr);
-   if (result == ~(uint32_t)0)
-   return -ENODEV;
-   result &= mask;
-   if (result == done)
-   return 0;
-   usec--;
-   udelay(1);
-   } while (usec > 0);
-
-   return -ETIMEDOUT;
+   return ret;
 }
 
 /**
-- 
2.25.1



[PATCH RESEND v4 5/9] usb: xhci: convert to TRB_TYPE()

2020-09-08 Thread Frank Wunderlich
From: Chunfeng Yun 

Use TRB_TYPE(p) instead of ((p) << TRB_TYPE_SHIFT)

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
 drivers/usb/host/xhci-mem.c  |  3 +--
 drivers/usb/host/xhci-ring.c | 11 +--
 include/usb/xhci.h   |  1 -
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 1da0524aa0..d627aa 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -236,8 +236,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
 */
val = le32_to_cpu(prev->trbs[TRBS_PER_SEGMENT-1].link.control);
val &= ~TRB_TYPE_BITMASK;
-   val |= (TRB_LINK << TRB_TYPE_SHIFT);
-
+   val |= TRB_TYPE(TRB_LINK);
prev->trbs[TRBS_PER_SEGMENT-1].link.control = cpu_to_le32(val);
}
 }
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 3f915ae115..13c98fb09a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -696,7 +696,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
trb_fields[0] = lower_32_bits(addr);
trb_fields[1] = upper_32_bits(addr);
trb_fields[2] = length_field;
-   trb_fields[3] = field | (TRB_NORMAL << TRB_TYPE_SHIFT);
+   trb_fields[3] = field | TRB_TYPE(TRB_NORMAL);
 
queue_trb(ctrl, ring, (num_trbs > 1), trb_fields);
 
@@ -823,7 +823,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
/* Queue setup TRB - see section 6.4.1.2.1 */
/* FIXME better way to translate setup_packet into two u32 fields? */
field = 0;
-   field |= TRB_IDT | (TRB_SETUP << TRB_TYPE_SHIFT);
+   field |= TRB_IDT | TRB_TYPE(TRB_SETUP);
if (start_cycle == 0)
field |= 0x1;
 
@@ -860,9 +860,9 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
/* If there's data, queue data TRBs */
/* Only set interrupt on short packet for IN endpoints */
if (usb_pipein(pipe))
-   field = TRB_ISP | (TRB_DATA << TRB_TYPE_SHIFT);
+   field = TRB_ISP | TRB_TYPE(TRB_DATA);
else
-   field = (TRB_DATA << TRB_TYPE_SHIFT);
+   field = TRB_TYPE(TRB_DATA);
 
remainder = xhci_td_remainder(ctrl, 0, length, length,
  usb_maxpacket(udev, pipe), true);
@@ -904,8 +904,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
trb_fields[2] = ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
/* Event on completion */
trb_fields[3] = field | TRB_IOC |
-   (TRB_STATUS << TRB_TYPE_SHIFT) |
-   ep_ring->cycle_state;
+   TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state;
 
queue_trb(ctrl, ep_ring, false, trb_fields);
 
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index cf4c0208b2..bdba51df59 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -903,7 +903,6 @@ union xhci_trb {
 /* TRB bit mask */
 #defineTRB_TYPE_BITMASK(0xfc00)
 #define TRB_TYPE(p)((p) << 10)
-#define TRB_TYPE_SHIFT (10)
 #define TRB_FIELD_TO_TYPE(p)   (((p) & TRB_TYPE_BITMASK) >> 10)
 
 /* TRB type IDs */
-- 
2.25.1



[PATCH RESEND v4 7/9] usb: xhci: convert to TRB_TX_TYPE()

2020-09-08 Thread Frank Wunderlich
From: Chunfeng Yun 

Use TRB_TX_TYPE() instead of (TRB_DATA_OUT/IN << TRB_TX_TYPE_SHIFT)

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
 drivers/usb/host/xhci-ring.c | 4 ++--
 include/usb/xhci.h   | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9ef72efe95..b118207d93 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -830,9 +830,9 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
if (ctrl->hci_version >= 0x100 || ctrl->quirks & XHCI_MTK_HOST) {
if (length > 0) {
if (req->requesttype & USB_DIR_IN)
-   field |= (TRB_DATA_IN << TRB_TX_TYPE_SHIFT);
+   field |= TRB_TX_TYPE(TRB_DATA_IN);
else
-   field |= (TRB_DATA_OUT << TRB_TX_TYPE_SHIFT);
+   field |= TRB_TX_TYPE(TRB_DATA_OUT);
}
}
 
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 35c66042ba..07b1aebc69 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -879,7 +879,6 @@ struct xhci_event_cmd {
 /* Control transfer TRB specific fields */
 #define TRB_DIR_IN (1<<16)
 #defineTRB_TX_TYPE(p)  ((p) << 16)
-#defineTRB_TX_TYPE_SHIFT   (16)
 #defineTRB_DATA_OUT2
 #defineTRB_DATA_IN 3
 
-- 
2.25.1



[PATCH RESEND v4 3/9] usb: xhci: add quirks flag to support MediaTek xHCI 0.96

2020-09-08 Thread Frank Wunderlich
From: Chunfeng Yun 

There some vendor quirks for MTK xHCI 0.96 host controller:
1. It defines some extra SW scheduling parameters for HW
   to minimize the scheduling effort for synchronous and
   interrupt endpoints. The parameters are put into reserved
   DWs of slot context and endpoint context.
2. Its TDS in  Normal TRB defines a number of packets that
   remains to be transferred for a TD after processing all
   Max packets in all previous TRBs.

Signed-off-by: Chunfeng Yun 
Tested-by: Frank Wunderlich 
Reviewed-by: Bin Meng 
---
 drivers/usb/host/xhci-mtk.c  | 1 +
 drivers/usb/host/xhci-ring.c | 9 +++--
 drivers/usb/host/xhci.c  | 2 +-
 include/usb/xhci.h   | 2 ++
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 8ff71854fc..f3f181dae0 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -258,6 +258,7 @@ static int xhci_mtk_probe(struct udevice *dev)
if (ret)
goto ssusb_init_err;
 
+   mtk->ctrl.quirks = XHCI_MTK_HOST;
hcor = (struct xhci_hcor *)((uintptr_t)mtk->hcd +
HC_LENGTH(xhci_readl(>hcd->cr_capbase)));
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 603e0e5b76..3f915ae115 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -332,7 +332,8 @@ static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int 
transferred,
 {
u32 total_packet_count;
 
-   if (ctrl->hci_version < 0x100)
+   /* MTK xHCI 0.96 contains some features from 1.0 */
+   if (ctrl->hci_version < 0x100 && !(ctrl->quirks & XHCI_MTK_HOST))
return ((td_total_len - transferred) >> 10);
 
/* One TRB with a zero-length data packet. */
@@ -340,6 +341,10 @@ static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int 
transferred,
trb_buff_len == td_total_len)
return 0;
 
+   /* for MTK xHCI 0.96, TD size include this TRB, but not in 1.x */
+   if ((ctrl->quirks & XHCI_MTK_HOST) && (ctrl->hci_version < 0x100))
+   trb_buff_len = 0;
+
total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
 
/* Queueing functions don't count the current TRB into transferred */
@@ -823,7 +828,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
field |= 0x1;
 
/* xHCI 1.0 6.4.1.2.1: Transfer Type field */
-   if (ctrl->hci_version >= 0x100) {
+   if (ctrl->hci_version >= 0x100 || ctrl->quirks & XHCI_MTK_HOST) {
if (length > 0) {
if (req->requesttype & USB_DIR_IN)
field |= (TRB_DATA_IN << TRB_TX_TYPE_SHIFT);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4be1411243..51edeb22c1 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -650,7 +650,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 * are put into reserved DWs in Slot and Endpoint Contexts
 * for synchronous endpoints.
 */
-   if (IS_ENABLED(CONFIG_USB_XHCI_MTK)) {
+   if (ctrl->quirks & XHCI_MTK_HOST) {
ep_ctx[ep_index]->reserved[0] =
cpu_to_le32(EP_BPKTS(1) | EP_BBM(1));
}
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 15926eb9f4..3de46cd95e 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -1230,6 +1230,8 @@ struct xhci_ctrl {
struct xhci_virt_device *devs[MAX_HC_SLOTS];
int rootdev;
u16 hci_version;
+   u32 quirks;
+#define XHCI_MTK_HOST  BIT(0)
 };
 
 unsigned long trb_addr(struct xhci_segment *seg, union xhci_trb *trb);
-- 
2.25.1



[PATCH RESEND v4 2/9] usb: xhci: create one unified function to calculate TRB TD remainder

2020-09-08 Thread Frank Wunderlich
From: Chunfeng Yun 

xhci versions 1.0 and later report the untransferred data remaining in a
TD a bit differently than older hosts.

We used to have separate functions for these, and needed to check host
version before calling the right function.

Now Mediatek host has an additional quirk on how it uses the TD Size
field for remaining data. To prevent yet another function for calculating
remainder we instead want to make one quirk friendly unified function.

Porting from the Linux:
c840d6ce772d("xhci: create one unified function to calculate TRB TD remainder.")
124c39371114("xhci: use boolean to indicate last trb in td remainder 
calculation")

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
 drivers/usb/host/xhci-ring.c | 105 +--
 include/usb/xhci.h   |   2 +
 2 files changed, 52 insertions(+), 55 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 79bfc349f4..603e0e5b76 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -298,55 +298,52 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, 
u32 slot_id,
xhci_writel(>dba->doorbell[0], DB_VALUE_HOST);
 }
 
-/**
- * The TD size is the number of bytes remaining in the TD (including this TRB),
- * right shifted by 10.
- * It must fit in bits 21:17, so it can't be bigger than 31.
+/*
+ * For xHCI 1.0 host controllers, TD size is the number of max packet sized
+ * packets remaining in the TD (*not* including this TRB).
  *
- * @param remainderremaining packets to be sent
- * @return remainder if remainder is less than max else max
- */
-static u32 xhci_td_remainder(unsigned int remainder)
-{
-   u32 max = (1 << (21 - 17 + 1)) - 1;
-
-   if ((remainder >> 10) >= max)
-   return max << 17;
-   else
-   return (remainder >> 10) << 17;
-}
-
-/**
- * Finds out the remanining packets to be sent
+ * Total TD packet count = total_packet_count =
+ * DIV_ROUND_UP(TD size in bytes / wMaxPacketSize)
+ *
+ * Packets transferred up to and including this TRB = packets_transferred =
+ * rounddown(total bytes transferred including this TRB / wMaxPacketSize)
+ *
+ * TD size = total_packet_count - packets_transferred
  *
- * @param running_totaltotal size sent so far
+ * For xHCI 0.96 and older, TD size field should be the remaining bytes
+ * including this TRB, right shifted by 10
+ *
+ * For all hosts it must fit in bits 21:17, so it can't be bigger than 31.
+ * This is taken care of in the TRB_TD_SIZE() macro
+ *
+ * The last TRB in a TD must have the TD size set to zero.
+ *
+ * @param ctrl host controller data structure
+ * @param transferred  total size sent so far
  * @param trb_buff_len length of the TRB Buffer
- * @param total_packet_count   total packet count
- * @param maxpacketsizemax packet size of current pipe
- * @param num_trbs_leftnumber of TRBs left to be processed
- * @return 0 if running_total or trb_buff_len is 0, else remainder
+ * @param td_total_len total packet count
+ * @param maxp max packet size of current pipe
+ * @param more_trbs_coming indicate last trb in TD
+ * @return remainder
  */
-static u32 xhci_v1_0_td_remainder(int running_total,
-   int trb_buff_len,
-   unsigned int total_packet_count,
-   int maxpacketsize,
-   unsigned int num_trbs_left)
+static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int transferred,
+int trb_buff_len, unsigned int td_total_len,
+int maxp, bool more_trbs_coming)
 {
-   int packets_transferred;
+   u32 total_packet_count;
+
+   if (ctrl->hci_version < 0x100)
+   return ((td_total_len - transferred) >> 10);
 
/* One TRB with a zero-length data packet. */
-   if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == 0))
+   if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) ||
+   trb_buff_len == td_total_len)
return 0;
 
-   /*
-* All the TRB queueing functions don't count the current TRB in
-* running_total.
-*/
-   packets_transferred = (running_total + trb_buff_len) / maxpacketsize;
+   total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
 
-   if ((total_packet_count - packets_transferred) > 31)
-   return 31 << 17;
-   return (total_packet_count - packets_transferred) << 17;
+   /* Queueing functions don't count the current TRB into transferred */
+   return (total_packet_count - ((transferred + trb_buff_len) / maxp));
 }
 
 /**
@@ -572,7 +569,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
union xhci_trb *event;
 
int running_total, trb_buff_len;
-   unsigned int total_packet_count;
+   bool more_trbs_coming = true;
int 

[PATCH RESEND v4 6/9] usb: xhci: convert to TRB_LEN() and TRB_INTR_TARGET()

2020-09-08 Thread Frank Wunderlich
From: Chunfeng Yun 

For normal TRB fields:
use TRB_LEN(x) instead of ((x) & TRB_LEN_MASK);
and use TRB_INTR_TARGET(x) instead of
(((x) & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT)

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
 drivers/usb/host/xhci-ring.c | 16 +++-
 include/usb/xhci.h   |  3 ---
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 13c98fb09a..9ef72efe95 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -688,10 +688,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
  length, maxpacketsize,
  more_trbs_coming);
 
-   length_field = ((trb_buff_len & TRB_LEN_MASK) |
+   length_field = (TRB_LEN(trb_buff_len) |
TRB_TD_SIZE(remainder) |
-   ((0 & TRB_INTR_TARGET_MASK) <<
-   TRB_INTR_TARGET_SHIFT));
+   TRB_INTR_TARGET(0));
 
trb_fields[0] = lower_32_bits(addr);
trb_fields[1] = upper_32_bits(addr);
@@ -849,8 +848,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
trb_fields[1] = le16_to_cpu(req->index) |
le16_to_cpu(req->length) << 16;
/* TRB_LEN | (TRB_INTR_TARGET) */
-   trb_fields[2] = (8 | ((0 & TRB_INTR_TARGET_MASK) <<
-   TRB_INTR_TARGET_SHIFT));
+   trb_fields[2] = (TRB_LEN(8) | TRB_INTR_TARGET(0));
/* Immediate data in pointer */
trb_fields[3] = field;
queue_trb(ctrl, ep_ring, true, trb_fields);
@@ -866,11 +864,11 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
remainder = xhci_td_remainder(ctrl, 0, length, length,
  usb_maxpacket(udev, pipe), true);
-   length_field = (length & TRB_LEN_MASK) | TRB_TD_SIZE(remainder) |
-   ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
+   length_field = TRB_LEN(length) | TRB_TD_SIZE(remainder) |
+  TRB_INTR_TARGET(0);
debug("length_field = %d, length = %d,"
"xhci_td_remainder(length) = %d , TRB_INTR_TARGET(0) = %d\n",
-   length_field, (length & TRB_LEN_MASK),
+   length_field, TRB_LEN(length),
TRB_TD_SIZE(remainder), 0);
 
if (length > 0) {
@@ -901,7 +899,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
trb_fields[0] = 0;
trb_fields[1] = 0;
-   trb_fields[2] = ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
+   trb_fields[2] = TRB_INTR_TARGET(0);
/* Event on completion */
trb_fields[3] = field | TRB_IOC |
TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state;
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index bdba51df59..35c66042ba 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -847,12 +847,9 @@ struct xhci_event_cmd {
 /* Normal TRB fields */
 /* transfer_len bitmasks - bits 0:16 */
 #defineTRB_LEN(p)  ((p) & 0x1)
-#defineTRB_LEN_MASK(0x1)
 /* TD Size, packets remaining in this TD, bits 21:17 (5 bits, so max 31) */
 #define TRB_TD_SIZE(p)  (min((p), (u32)31) << 17)
 /* Interrupter Target - which MSI-X vector to target the completion event at */
-#defineTRB_INTR_TARGET_SHIFT   (22)
-#defineTRB_INTR_TARGET_MASK(0x3ff)
 #define TRB_INTR_TARGET(p) (((p) & 0x3ff) << 22)
 #define GET_INTR_TARGET(p) (((p) >> 22) & 0x3ff)
 #define TRB_TBC(p) (((p) & 0x3) << 7)
-- 
2.25.1



[PATCH RESEND v4 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct

2020-09-08 Thread Frank Wunderlich
From: Chunfeng Yun 

Add a member to save xHCI version, it's used some times.

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
 drivers/usb/host/xhci-ring.c | 4 ++--
 drivers/usb/host/xhci.c  | 1 +
 include/usb/xhci.h   | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 092ed6eaf1..79bfc349f4 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -682,7 +682,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
field |= TRB_ISP;
 
/* Set the TRB length, TD size, and interrupter fields. */
-   if (HC_VERSION(xhci_readl(>hccr->cr_capbase)) < 0x100)
+   if (ctrl->hci_version < 0x100)
remainder = xhci_td_remainder(length - running_total);
else
remainder = xhci_v1_0_td_remainder(running_total,
@@ -830,7 +830,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
field |= 0x1;
 
/* xHCI 1.0 6.4.1.2.1: Transfer Type field */
-   if (HC_VERSION(xhci_readl(>hccr->cr_capbase)) >= 0x100) {
+   if (ctrl->hci_version >= 0x100) {
if (length > 0) {
if (req->requesttype & USB_DIR_IN)
field |= (TRB_DATA_IN << TRB_TX_TYPE_SHIFT);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 126dabc11b..4be1411243 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1283,6 +1283,7 @@ static int xhci_lowlevel_init(struct xhci_ctrl *ctrl)
 
reg = HC_VERSION(xhci_readl(>cr_capbase));
printf("USB XHCI %x.%02x\n", reg >> 8, reg & 0xff);
+   ctrl->hci_version = reg;
 
return 0;
 }
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 7d34103fd5..a3e5914b10 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -1227,6 +1227,7 @@ struct xhci_ctrl {
struct xhci_scratchpad *scratchpad;
struct xhci_virt_device *devs[MAX_HC_SLOTS];
int rootdev;
+   u16 hci_version;
 };
 
 unsigned long trb_addr(struct xhci_segment *seg, union xhci_trb *trb);
-- 
2.25.1



[PATCH RESEND v4 4/9] usb: xhci: convert to HCS_MAX_PORTS()

2020-09-08 Thread Frank Wunderlich
From: Chunfeng Yun 

Use HCS_MAX_PORTS(p) instead of
((p & HCS_MAX_PORTS_MASK) >> HCS_MAX_PORTS_SHIFT)

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
 drivers/usb/host/xhci.c | 3 +--
 include/usb/xhci.h  | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 51edeb22c1..5f3a0fba4b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1257,8 +1257,7 @@ static int xhci_lowlevel_init(struct xhci_ctrl *ctrl)
return -ENOMEM;
 
reg = xhci_readl(>cr_hcsparams1);
-   descriptor.hub.bNbrPorts = ((reg & HCS_MAX_PORTS_MASK) >>
-   HCS_MAX_PORTS_SHIFT);
+   descriptor.hub.bNbrPorts = HCS_MAX_PORTS(reg);
printf("Register %x NbrPorts %d\n", reg, descriptor.hub.bNbrPorts);
 
/* Port Indicators */
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 3de46cd95e..cf4c0208b2 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -101,8 +101,6 @@ struct xhci_hccr {
 /* bits 8:18, Max Interrupters */
 #define HCS_MAX_INTRS(p)   (((p) >> 8) & 0x7ff)
 /* bits 24:31, Max Ports - max value is 0x7F = 127 ports */
-#define HCS_MAX_PORTS_SHIFT24
-#define HCS_MAX_PORTS_MASK (0xff << HCS_MAX_PORTS_SHIFT)
 #define HCS_MAX_PORTS(p)   (((p) >> 24) & 0xff)
 
 /* HCSPARAMS2 - hcs_params2 - bitmasks */
-- 
2.25.1



Re: [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Marek Vasut
On 9/8/20 6:34 PM, Frank Wunderlich wrote:
> 
> 
> Am 8. September 2020 18:15:05 MESZ schrieb Marek Vasut :
>> Thank you, I cannot apply a patchset which only has "Re:" in my
>> mailbox,
>> because the original patches are just not there.
> 
> Should i resend v4? Or could you apply from Patchwork (series link in upper 
> right of Patch)? Thats the way i use :)
> 
> https://patchwork.ozlabs.org/project/uboot/list/?series=200140

Resend please, if I wanted to comment on any of those patches, I won't
be able.


Re: [PATCH v4 2/3] binman: Support help messages for missing blobs

2020-09-08 Thread Alper Nebi Yasak
On 06/09/2020 19:39, Simon Glass wrote:
> When an external blob is missing it can be quite confusing for the user.
> Add a way to provide a help message that is shown.
> 
> Signed-off-by: Simon Glass 
> ---
> 
> (no changes since v3)
> 
> Changes in v3:
> - Add a way to show help messages for missing blobs
> 
>  tools/binman/README|  6 ++
>  tools/binman/control.py| 69 +-
>  tools/binman/entry.py  |  9 +++
>  tools/binman/ftest.py  | 18 +-
>  tools/binman/missing-blob-help | 11 
>  tools/binman/test/168_fit_missing_blob.dts |  9 ++-
>  6 files changed, 119 insertions(+), 3 deletions(-)
>  create mode 100644 tools/binman/missing-blob-help
> 
> diff --git a/tools/binman/README b/tools/binman/README
> index 37ee3fc2d3b..f7bf285a915 100644
> --- a/tools/binman/README
> +++ b/tools/binman/README
> @@ -343,6 +343,12 @@ compress:
>   Sets the compression algortihm to use (for blobs only). See the entry
>   documentation for details.
>  
> +missing-msg:
> + Sets the tag of the message to show if this entry is missing. This is
> + used for external blobs. When they are missing it is helpful to show
> + information about what needs to be fixed. See missing-blob-help for the
> + message for each tag.
> +
>  The attributes supported for images and sections are described below. Several
>  are similar to those for entries.
>  
> diff --git a/tools/binman/control.py b/tools/binman/control.py
> index 15bfac582a7..ee5771e7292 100644
> --- a/tools/binman/control.py
> +++ b/tools/binman/control.py
> @@ -9,6 +9,7 @@ from collections import OrderedDict
>  import glob
>  import os
>  import pkg_resources
> +import re
>  
>  import sys
>  from patman import tools
> @@ -22,6 +23,11 @@ from patman import tout
>  # Make this global so that it can be referenced from tests
>  images = OrderedDict()
>  
> +# Help text for each type of missing blob, dict:
> +#key: Value of the entry's 'missing-msg' or entry name
> +#value: Text for the help
> +missing_blob_help = {}
> +
>  def _ReadImageDesc(binman_node):
>  """Read the image descriptions from the /binman node
>  
> @@ -54,6 +60,66 @@ def _FindBinmanNode(dtb):
>  return node
>  return None
>  
> +def _ReadMissingBlobHelp():
> +"""Read the missing-blob-help file
> +
> +This file containins help messages explaining what to do when external 
> blobs
> +are missing.
> +
> +Returns:
> +Dict:
> +key: Message tag (str)
> +value: Message text (str)
> +"""
> +
> +def _FinishTag(tag, msg, result):
> +if tag:
> +result[tag] = msg.rstrip()
> +tag = None
> +msg = ''
> +return tag, msg
> +
> +my_data = pkg_resources.resource_string(__name__, 'missing-blob-help')
> +re_tag = re.compile('^([-a-z0-9]+):$')
> +result = {}
> +tag = None
> +msg = ''
> +for line in my_data.decode('utf-8').splitlines():
> +if not line.startswith('#'):
> +m_tag = re_tag.match(line)
> +if m_tag:
> +_, msg = _FinishTag(tag, msg, result)
> +tag = m_tag.group(1)
> +elif tag:
> +msg += line + '\n'
> +_FinishTag(tag, msg, result)
> +return result

This looks a bit complex, I think something like this would be clearer:

result = {}
tag = None
for line in my_data.decode('utf-8').splitlines():
m_tag = re_tag.match(line)
if line.startswith('#'):
continue
elif m_tag:
tag = m_tag.group(1)
result[tag] = []
elif tag:
result[tag].append(line)
for tag, lines in result.items():
result[tag] = "".join(lines).rstrip()
return result

> +
> +def _ShowBlobHelp(path, text):
> +tout.Warning('\n%s:' % path)
> +for line in text.splitlines():
> +tout.Warning('   %s' % line)
> +
> +def _ShowHelpForMissingBlobs(missing_list):
> +"""Show help for each missing blob to help the user take action
> +
> +Args:
> +missing_list: List of Entry objects to show help for
> +"""
> +global missing_blob_help
> +
> +if not missing_blob_help:
> +missing_blob_help = _ReadMissingBlobHelp()
> +
> +for entry in missing_list:
> +tags = entry.GetHelpTags()
> +
> +# Show the first match help message
> +for tag in tags:
> +if tag in missing_blob_help:
> +_ShowBlobHelp(entry._node.path, missing_blob_help[tag])
> +break
> +
>  def GetEntryModules(include_testing=True):
>  """Get a set of entry class implementations
>  
> @@ -478,6 +544,7 @@ def ProcessImage(image, update_fdt, write_map, 
> get_contents=True,
>  if missing_list:
>  tout.Warning("Image '%s' is missing external blobs and is 
> non-functional: %s" %
>   

Re: [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Frank Wunderlich



Am 8. September 2020 18:15:05 MESZ schrieb Marek Vasut :
>Thank you, I cannot apply a patchset which only has "Re:" in my
>mailbox,
>because the original patches are just not there.

Should i resend v4? Or could you apply from Patchwork (series link in upper 
right of Patch)? Thats the way i use :)

https://patchwork.ozlabs.org/project/uboot/list/?series=200140
regards Frank


Re: [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Marek Vasut
On 9/8/20 5:45 PM, Bin Meng wrote:
> Hi Marek,

Hi,

> On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut  wrote:
>>
>> On 9/8/20 3:44 AM, Bin Meng wrote:
>>> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun  
>>> wrote:

 Use readx_poll_sleep_timeout() to poll the register status

 Signed-off-by: Chunfeng Yun 
 ---
 v3: no changes

 v2: fix typo of title suggested by Frank
 ---
  drivers/usb/host/xhci.c | 25 +++--
  1 file changed, 11 insertions(+), 14 deletions(-)

>>>
>>> Reviewed-by: Bin Meng 
>>
>> Thanks. So now I have half of a RESEND patchset with RB tags, and
>> another half of just RB tags without patchset. Can someone of you please
>> collect the tags and resend the patchset once more with the collected
>> tags, so it's all in one place ?
>>
> 
> I believe the latest version has all the RB tags.

Thank you, I cannot apply a patchset which only has "Re:" in my mailbox,
because the original patches are just not there.


Re: [PATCH v1] cmd: acpi: Print revisions in hex format

2020-09-08 Thread Andy Shevchenko
On Tue, Sep 08, 2020 at 05:32:08PM +0200, Wolfgang Wallner wrote:
> -"Andy Shevchenko"  schrieb: -
> > On Tue, Sep 8, 2020 at 5:58 PM Wolfgang Wallner
> >  wrote:
> > > -"Andy Shevchenko"  schrieb: -

...

> > > Related to "acpi list":
> > > During my recent ACPI debugging I found it very useful to have the 
> > > checksum
> > > printed for each table with "acpi list". Would there be interest to have 
> > > that
> > > upstream? If so I would send a patch.
> > 
> > Can you elaborate what was the problem that checksum helped?
> 
> Sure. I saw two strange things with the ACPI checksums:
> 
> 1) The DSDT length included uninitialized bytes from alignment. This is
> described in the following link:
> 
>https://lists.denx.de/pipermail/u-boot/2020-September/425378.html
>
> This was the actual bug I was looking for.
>
> 2) acpi_create_spcr() is missing a memset(). The other acpi_create_()
> functions perform a memset on their structure, acpi_create_spcr() does not
> and as a result the contents of this table are party uninitialized.
> 
> I plan to send a patch for both of them.

I'm not sure I understood how checksum pointed to uninitialized data?

-- 
With Best Regards,
Andy Shevchenko




Re: [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Bin Meng
Hi Marek,

On Tue, Sep 8, 2020 at 7:13 PM Marek Vasut  wrote:
>
> On 9/8/20 3:44 AM, Bin Meng wrote:
> > On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun  
> > wrote:
> >>
> >> Use readx_poll_sleep_timeout() to poll the register status
> >>
> >> Signed-off-by: Chunfeng Yun 
> >> ---
> >> v3: no changes
> >>
> >> v2: fix typo of title suggested by Frank
> >> ---
> >>  drivers/usb/host/xhci.c | 25 +++--
> >>  1 file changed, 11 insertions(+), 14 deletions(-)
> >>
> >
> > Reviewed-by: Bin Meng 
>
> Thanks. So now I have half of a RESEND patchset with RB tags, and
> another half of just RB tags without patchset. Can someone of you please
> collect the tags and resend the patchset once more with the collected
> tags, so it's all in one place ?
>

I believe the latest version has all the RB tags.

Regards,
Bin


Re: [PATCH v1] cmd: acpi: Print revisions in hex format

2020-09-08 Thread Wolfgang Wallner
Hi Andy,

-"Andy Shevchenko"  schrieb: -
> Betreff: Re: [PATCH v1] cmd: acpi: Print revisions in hex format
> 
> On Tue, Sep 8, 2020 at 5:58 PM Wolfgang Wallner
>  wrote:
> > -"Andy Shevchenko"  schrieb: -
> > > Betreff: [PATCH v1] cmd: acpi: Print revisions in hex format
> > >
> > > The revisions are usually dates in hex-decimal format representing
> > > mmdd. Print them in hex to see this clearly.
> > >
> > > Before:
> > >   ...
> > >   FACP 000e5420 f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0)
> > >   DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870)
> > >   ...
> > > After:
> > >   ...
> > >   FACP 000e5420 f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0)
> > >   DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 1 INTL 20200326)
> > >   ...
> > >
> > > Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command")
> > > Cc: Wolfgang Wallner 
> > > Signed-off-by: Andy Shevchenko 
> > > ---
> > >  cmd/acpi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Reviewed-by: Wolfgang Wallner 
> > Tested-by: Wolfgang Wallner 
> > Tested on a custom Apollolake board.
> 
> Thanks!

You're welcome.

> 
> > Related to "acpi list":
> > During my recent ACPI debugging I found it very useful to have the checksum
> > printed for each table with "acpi list". Would there be interest to have 
> > that
> > upstream? If so I would send a patch.
> 
> Can you elaborate what was the problem that checksum helped?

Sure. I saw two strange things with the ACPI checksums:

1) The DSDT length included uninitialized bytes from alignment. This is
described in the following link:

   https://lists.denx.de/pipermail/u-boot/2020-September/425378.html
   
This was the actual bug I was looking for.
   
2) acpi_create_spcr() is missing a memset(). The other acpi_create_()
functions perform a memset on their structure, acpi_create_spcr() does not
and as a result the contents of this table are party uninitialized.

I plan to send a patch for both of them.

Regards, Wolfgang







Re: [PATCH v4 1/2] drivers: gpio: Add a managed API to get a GPIO from the device-tree

2020-09-08 Thread Heinrich Schuchardt
Am 8. September 2020 17:15:25 MESZ schrieb Pratyush Yadav :
>On 08/09/20 04:12PM, Heinrich Schuchardt wrote:
>> On 08.09.20 07:40, Pratyush Yadav wrote:
>> > From: Jean-Jacques Hiblot 
>> >
>> > Add managed functions to get a gpio from the devce-tree, based on a
>> > property name (minus the '-gpios' suffix) and optionally an index.
>> >
>> > When the device is unbound, the GPIO is automatically released and
>the
>> > data structure is freed.
>> >
>> > Signed-off-by: Jean-Jacques Hiblot 
>> > Reviewed-by: Simon Glass 
>> > Signed-off-by: Pratyush Yadav 
>> > ---
>> >  drivers/gpio/gpio-uclass.c | 71
>++
>> >  include/asm-generic/gpio.h | 47 +
>> >  2 files changed, 118 insertions(+)
>> >
>> > diff --git a/drivers/gpio/gpio-uclass.c
>b/drivers/gpio/gpio-uclass.c
>> > index 9c53299b6a..0c01413b58 100644
>> > --- a/drivers/gpio/gpio-uclass.c
>> > +++ b/drivers/gpio/gpio-uclass.c
>> > @@ -6,6 +6,8 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> > +#include 
>> >  #include 
>> >  #include 
>> >  #include 
>> > @@ -1209,6 +1211,75 @@ int gpio_dev_request_index(struct udevice
>*dev, const char *nodename,
>> > flags, 0, dev);
>> >  }
>> >
>> > +static void devm_gpiod_release(struct udevice *dev, void *res)
>> > +{
>> > +  dm_gpio_free(dev, res);
>> > +}
>> > +
>> > +static int devm_gpiod_match(struct udevice *dev, void *res, void
>*data)
>> > +{
>> > +  return res == data;
>> > +}
>> > +
>> > +struct gpio_desc *devm_gpiod_get_index(struct udevice *dev, const
>char *id,
>> > + unsigned int index, int flags)
>> > +{
>> > +  int rc;
>> > +  struct gpio_desc *desc;
>> > +  char *propname;
>> > +  static const char suffix[] = "-gpios";
>> > +
>> > +  propname = malloc(strlen(id) + sizeof(suffix));
>> > +  if (!propname) {
>> > +  rc = -ENOMEM;
>> > +  goto end;
>> > +  }
>> > +
>> > +  strcpy(propname, id);
>> > +  strcat(propname, suffix);
>> > +
>> > +  desc = devres_alloc(devm_gpiod_release, sizeof(struct gpio_desc),
>> > +  __GFP_ZERO);
>> > +  if (unlikely(!desc)) {
>> > +  rc = -ENOMEM;
>> > +  goto end;
>> > +  }
>> > +
>> > +  rc = gpio_request_by_name(dev, propname, index, desc, flags);
>> > +
>> > +end:
>> > +  if (propname)
>> > +  free(propname);
>> > +
>> > +  if (rc)
>> > +  return ERR_PTR(rc);
>> > +
>> > +  devres_add(dev, desc);
>> > +
>> > +  return desc;
>> > +}
>> > +
>> > +struct gpio_desc *devm_gpiod_get_index_optional(struct udevice
>*dev,
>> > +  const char *id,
>> > +  unsigned int index,
>> > +  int flags)
>> > +{
>> > +  struct gpio_desc *desc = devm_gpiod_get_index(dev, id, index,
>flags);
>> > +
>> > +  if (IS_ERR(desc))
>> > +  return NULL;
>> > +
>> > +  return desc;
>> > +}
>> > +
>> > +void devm_gpiod_put(struct udevice *dev, struct gpio_desc *desc)
>> > +{
>> > +  int rc;
>> > +
>> > +  rc = devres_release(dev, devm_gpiod_release, devm_gpiod_match,
>desc);
>> > +  WARN_ON(rc);
>> > +}
>> > +
>> >  static int gpio_post_bind(struct udevice *dev)
>> >  {
>> >struct udevice *child;
>> > diff --git a/include/asm-generic/gpio.h
>b/include/asm-generic/gpio.h
>> > index a57dd2665c..ad6979a8ce 100644
>> > --- a/include/asm-generic/gpio.h
>> > +++ b/include/asm-generic/gpio.h
>> > @@ -701,4 +701,51 @@ int gpio_get_number(const struct gpio_desc
>*desc);
>> >   */
>> >  int gpio_get_acpi(const struct gpio_desc *desc, struct acpi_gpio
>*gpio);
>> >
>> > +/**
>> > + * devm_gpiod_get_index - Resource-managed gpiod_get()
>> > + * @dev:  GPIO consumer
>> > + * @con_id:   function within the GPIO consumer
>> > + * @index:index of the GPIO to obtain in the consumer
>> > + * @flags:optional GPIO initialization flags
>> > + *
>> > + * Managed gpiod_get(). GPIO descriptors returned from this
>function are
>> > + * automatically disposed on driver detach.
>> 
>> You pass in a device but write "driver detach". Shouldn't the object
>be
>> "device" in the description as in your commit message?
>
>Yes, it should be device.
> 
>> Devices have methods remove() and unbind() but no detach. In the
>commit
>> message you speak of unbind(). Please, don't use alternative
>language.
>
>Ok.
> 
>> Please, include the API in the HTML documentation created by 'make
>> htmldocs'.
>
>I tried searching for the GPIO API documentation under doc/ but I can't
>
>find anything. README.gpio doesn't mention it anywhere and doc/api/ has
>
>no file for gpio. Where do I document the newly added APIs then?

Please, add a new file doc/api/gpio.rst and include your include there. Add 
gpio.rst to doc/api/index.rst.

Check that make htmldocs does not show warnings.

The documentation will be generated in doc/output.

Best regards

Heinrich


> 
>> Why did you choose the unbind() and 

Re: [PATCH v3 6/8] firmware: scmi: sandbox test for SCMI clocks

2020-09-08 Thread Simon Glass
Hi Etienne,

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
 wrote:
>
> Add tests for SCMI clocks. A test device driver sandbox-scmi_devices.c
> is used to get clock resources, allowing further clock manipulation.
>
> Change sandbox-smci_agent to emulate 3 clocks exposed through 2 agents.
> Add DM test scmi_clocks to test these 3 clocks.
> Update DM test sandbox_scmi_agent with load/remove test sequences
> factorized by {load|remove}_sandbox_scmi_test_devices() helper functions.
>
> Signed-off-by: Etienne Carriere 
> Cc: Simon Glass 
> Cc: Peng Fan 
> Cc: Sudeep Holla 
> ---
>
> Changes in v3:
> - New commit in the series, addresses review comments on test support.
>   ut_dm_scmi_clocks test SCMI are found and behave as expected for the
>   implemented clk uclass methods.
> ---
>  arch/sandbox/dts/test.dts|  15 ++
>  arch/sandbox/include/asm/scmi_test.h |  37 
>  configs/sandbox_defconfig|   1 +
>  drivers/firmware/scmi/Makefile   |   2 +-
>  drivers/firmware/scmi/sandbox-scmi_agent.c   | 169 ++-
>  drivers/firmware/scmi/sandbox-scmi_devices.c |  86 ++
>  test/dm/scmi.c   | 139 ++-
>  7 files changed, 438 insertions(+), 11 deletions(-)
>  create mode 100644 drivers/firmware/scmi/sandbox-scmi_devices.c

Reviewed-by: Simon Glass 

Nits below

[..]

> diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c 
> b/drivers/firmware/scmi/sandbox-scmi_devices.c
> new file mode 100644
> index 00..2ce8e664df
> --- /dev/null
> +++ b/drivers/firmware/scmi/sandbox-scmi_devices.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2020, Linaro Limited
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/*
> + * Simulate to some extend a SCMI exhange.

extend, exchange

> + * This drivers gets SCMI resources and offers API function to the
> + * SCMI test sequence manipulate the resources.
> + */
> +
> +#define SCMI_TEST_DEVICES_CLK_COUNT3
> +
> +/*
> + * These tables store de handles used in the various uclasses device function

s/de/the/ ?

> + * that are instancied when probed through the SCMI agent. Use a static

spelling

> + * memory allocation to ease sharing with test sequence implementation.
> + */
> +static struct clk sandbox_scmi_clk_device[SCMI_TEST_DEVICES_CLK_COUNT];
> +static struct sandbox_scmi_devices sandbox_scmi_devhld;

This should really be in a struct, I think, pointed to by
dev_get_priv() on this device. I do try to avoid BSS with driver
model, although it is not a hard rule with test code.

> +
> +struct sandbox_scmi_devices *sandbox_scmi_devices_ctx(void)
> +{
> +   return _scmi_devhld;
> +}
> +
> +static void dereference_device_handles(struct sandbox_scmi_devices *devices)
> +{
> +   devices->clk = NULL;
> +   devices->clk_count = 0;
> +}
> +
> +static int sandbox_scmi_devices_remove(struct udevice *dev)
> +{
> +   struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
> +
> +   dereference_device_handles(devices);
> +
> +   return 0;
> +}
> +
> +static int sandbox_scmi_devices_probe(struct udevice *dev)
> +{
> +   struct sandbox_scmi_devices *devices = sandbox_scmi_devices_ctx();
> +   int rc;
> +   size_t n;
> +
> +   devices->clk = sandbox_scmi_clk_device;
> +   devices->clk_count = SCMI_TEST_DEVICES_CLK_COUNT;
> +
> +   for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {
> +   rc = clk_get_by_index(dev, n, devices->clk + n);
> +   if (rc) {
> +   dev_err(dev, "%s: Failed on clk %zu\n", __func__, n);
> +   goto err_clk;
> +   }
> +   }
> +
> +   return 0;
> +
> +err_clk:
> +   dereference_device_handles(devices);
> +
> +   return rc;
> +}
> +
> +static const struct udevice_id sandbox_scmi_devices_ids[] = {
> +   { .compatible = "sandbox,scmi-devices" },
> +   { }
> +};
> +
> +U_BOOT_DRIVER(sandbox_scmi_devices) = {
> +   .name = "sandbox-scmi_devices",
> +   .id = UCLASS_MISC,
> +   .of_match = sandbox_scmi_devices_ids,
> +   .remove = sandbox_scmi_devices_remove,
> +   .probe = sandbox_scmi_devices_probe,
> +};
[..]

Regards,
Simon


Re: [PATCH v3 7/8] reset: add reset controller driver for SCMI agents

2020-09-08 Thread Simon Glass
On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
 wrote:
>
> This change introduces a reset controller driver for SCMI agent devices.
> When SCMI agent and SCMI reset domain drivers are enabled, SCMI agent
> binds a reset controller device for each SCMI reset domain protocol
> devices enabled in the FDT.
>
> SCMI reset driver is embedded upon CONFIG_RESET_SCMI=y. If enabled,
> CONFIG_SCMI_AGENT is also enabled.
>
> SCMI Reset Domain protocol is defined in the SCMI specification [1].
>
> Links: [1] 
> https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> Signed-off-by: Etienne Carriere 
> Cc: Simon Glass 
> Cc: Peng Fan 
> Cc: Sudeep Holla 
> ---
>
> Changes in v3:
> - Upgrade to rename into devm_scmi_process_msg() and scmi.h split
>   into scmi_*.h.
> - Fix message ID used in scmi_reset_request().
>
> Changes in v2:
> - Change reset request() method to at least check the reset domain
>   exists by sending a SCMI RESET_DOMAIN_ATTRIBUTE message.
> - Add inline description for the several structures.
> - Patch v1 R-b tag not applied since the above changes in this v2.
>
> BACKPORTED FROM v2020.10-rc2 to V2020.04
> ---
>  drivers/firmware/scmi/scmi_agent-uclass.c |  3 +
>  drivers/reset/Kconfig |  8 +++
>  drivers/reset/Makefile|  1 +
>  drivers/reset/reset-scmi.c| 81 +++
>  include/scmi_protocols.h  | 60 +
>  5 files changed, 153 insertions(+)
>  create mode 100644 drivers/reset/reset-scmi.c

Reviewed-by: Simon Glass 


Re: [PATCH v3 2/8] firmware: scmi: mailbox/smt agent device

2020-09-08 Thread Simon Glass
Hi Etienne,

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
 wrote:
>
> This change implements a mailbox transport using SMT format for SCMI
> exchanges. This implementation follows the Linux kernel and
> SCP-firmware [1] as references implementation for SCMI message
> processing using SMT format for communication channel meta-data.
>
> Use of mailboxes in SCMI FDT bindings are defined in the Linux kernel
> DT bindings since v4.17.
>
> Links: [1] https://github.com/ARM-software/SCP-firmware
> Signed-off-by: Etienne Carriere 
> Cc: Simon Glass 
> Cc: Peng Fan 
> Cc: Sudeep Holla 
> ---
>
> Changes in v3:
> - This is a followup of the SCMI agent patches posted in
>   https://patchwork.ozlabs.org/project/uboot/list/?series=196253
>   The v3 splits commits and introduces a new uclass as requested.
> - This patch implements the same mailbox SCMI agent proposed in v2
>   but split over few source files.
> ---
>  drivers/firmware/scmi/Kconfig |   6 +-
>  drivers/firmware/scmi/Makefile|   2 +
>  drivers/firmware/scmi/mailbox_agent.c | 107 
>  drivers/firmware/scmi/smt.c   | 139 ++
>  drivers/firmware/scmi/smt.h   |  86 
>  5 files changed, 338 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/firmware/scmi/mailbox_agent.c
>  create mode 100644 drivers/firmware/scmi/smt.c
>  create mode 100644 drivers/firmware/scmi/smt.h

Reviewed-by: Simon Glass 

>
> diff --git a/drivers/firmware/scmi/Kconfig b/drivers/firmware/scmi/Kconfig
> index 57e2ebbe42..c501bf4943 100644
> --- a/drivers/firmware/scmi/Kconfig
> +++ b/drivers/firmware/scmi/Kconfig
> @@ -2,7 +2,7 @@ config SCMI_FIRMWARE
> bool "Enable SCMI support"
> select FIRMWARE
> select OF_TRANSLATE
> -   depends on SANDBOX
> +   depends on SANDBOX || DM_MAILBOX
> help
>   System Control and Management Interface (SCMI) is a communication
>   protocol that defines standard interfaces for power, performance
> @@ -14,4 +14,6 @@ config SCMI_FIRMWARE
>   or a companion host in the CPU system.
>
>   Communications between agent (client) and the SCMI server are
> - based on message exchange.
> + based on message exchange. Messages can be exchange over tranport
> + channels as a mailbox device with some piece of identified shared
> + memory.
> diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
> index 336ea1f2a3..d22f53efe7 100644
> --- a/drivers/firmware/scmi/Makefile
> +++ b/drivers/firmware/scmi/Makefile
> @@ -1,2 +1,4 @@
>  obj-y  += scmi_agent-uclass.o
> +obj-y  += smt.o
> +obj-$(CONFIG_DM_MAILBOX)   += mailbox_agent.o
>  obj-$(CONFIG_SANDBOX)  += sandbox-scmi_agent.o
> diff --git a/drivers/firmware/scmi/mailbox_agent.c 
> b/drivers/firmware/scmi/mailbox_agent.c
> new file mode 100644
> index 00..9a7b0a5858
> --- /dev/null
> +++ b/drivers/firmware/scmi/mailbox_agent.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Linaro Limited.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "smt.h"
> +
> +#define TIMEOUT_US_10MS1
> +
> +/**
> + * struct scmi_mbox_channel - Description of an SCMI mailbox transport
> + * @smt:   Shared memory buffer
> + * @mbox:  Mailbox channel description
> + * @timeout_us:Timeout in microseconds for the mailbox transfer
> + */
> +struct scmi_mbox_channel {
> +   struct scmi_smt smt;
> +   struct mbox_chan mbox;
> +   ulong timeout_us;
> +};
> +
> +static struct scmi_mbox_channel *scmi_mbox_get_priv(struct udevice *dev)
> +{
> +   return (struct scmi_mbox_channel *)dev_get_priv(dev);
> +}
> +
> +static int scmi_mbox_process_msg(struct udevice *dev, struct scmi_msg *msg)
> +{
> +   struct scmi_mbox_channel *chan = scmi_mbox_get_priv(dev);
> +   int ret;
> +
> +   ret = scmi_write_msg_to_smt(dev, >smt, msg);
> +   if (ret)
> +   return ret;
> +
> +   /* Give shm addr to mbox in case it is meaningful */
> +   ret = mbox_send(>mbox, chan->smt.buf);
> +   if (ret) {
> +   dev_err(dev, "Message send failed: %d\n", ret);
> +   goto out;
> +   }
> +
> +   /* Receive the response */
> +   ret = mbox_recv(>mbox, chan->smt.buf, chan->timeout_us);
> +   if (ret) {
> +   dev_err(dev, "Response failed: %d, abort\n", ret);
> +   goto out;
> +   }
> +
> +   ret = scmi_read_resp_from_smt(dev, >smt, msg);
> +
> +out:
> +   scmi_clear_smt_channel(>smt);
> +
> +   return ret;
> +}
> +
> +int scmi_mbox_probe(struct udevice *dev)
> +{
> +   struct scmi_mbox_channel *chan = scmi_mbox_get_priv(dev);
> +   int ret;
> +
> +   chan->timeout_us = TIMEOUT_US_10MS;
> +
> +   ret = mbox_get_by_index(dev, 0, >mbox);
> +   

Re: [PATCH v3 8/8] firmware: smci: sandbox test for SCMI reset controllers

2020-09-08 Thread Simon Glass
Hi Etienne,

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
 wrote:
>
> Add tests for SCMI reset controllers. A test device driver
> sandbox-scmi_devices.c is used to get reset resources, allowing further
> resets manipulation.
>
> Change sandbox-smci_agent to emulate 1 reset controller exposed through
> an agent. Add DM test scmi_resets to test this reset controller.
>
> Signed-off-by: Etienne Carriere 
> Cc: Simon Glass 
> Cc: Peng Fan 
> Cc: Sudeep Holla 
> ---
>
> Changes in v3:
> - New commit in the series, addresses review comments on test support.
>   ut_dm_scmi_resets() tests SCMI resources are found and behave as
>   expected for the implemented reset uclass methods.
> ---
>  arch/sandbox/dts/test.dts|   6 +
>  arch/sandbox/include/asm/scmi_test.h |  17 +++
>  configs/sandbox_defconfig|   1 +
>  drivers/firmware/scmi/sandbox-scmi_agent.c   | 117 ++-
>  drivers/firmware/scmi/sandbox-scmi_devices.c |  30 -
>  test/dm/scmi.c   |  35 ++
>  6 files changed, 200 insertions(+), 6 deletions(-)
>

Reviewed-by: Simon Glass 

Similar comment to the other patch about putting the BSS data in a
struct allocated by driver-private data.

Regards,
SImon


Re: [PATCH v3 1/8] firmware: add SCMI agent uclass

2020-09-08 Thread Simon Glass
On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
 wrote:
>
> This change introduces SCMI agent uclass to interact with a firmware
> using the SCMI protocols [1].
>
> SCMI agent uclass currently supports a single method to request
> processing of the SCMI message by an identified server. A SCMI message
> is made of a byte payload associated to a protocol ID and a message ID,
> all defined by the SCMI specification [1]. On return from process_msg()
> method, the caller gets the service response.
>
> SCMI agent uclass defines a post bind generic sequence for all devices.
> The sequence binds all the SCMI protocols listed in the FDT for that
> SCMI agent device. Currently none, but later change will introduce
> protocols.
>
> This change implements a simple sandbox device for the SCMI agent uclass.
> The sandbox nicely answers SCMI_NOT_SUPPORTED to SCMI messages.
> To prepare for further test support, the sandbox exposes a architecture
> function for test application to read the sandbox emulated devices state.
> Currently supports 2 SCMI agents, identified by an ID in the FDT device
> name. The simplistic DM test does nothing yet.
>
> SCMI agent uclass is designed for platforms that embed a SCMI server in
> a firmware hosted somewhere, for example in a companion co-processor or
> in the secure world of the executing processor. SCMI protocols allow an
> SCMI agent to discover and access external resources as clock, reset
> controllers and more. SCMI agent and server communicate following the
> SCMI specification [1]. This SCMI agent implementation complies with
> the DT bindings defined in the Linux kernel source tree regarding
> SCMI agent description since v5.8.
>
> Links: [1] 
> https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> Signed-off-by: Etienne Carriere 
> Cc: Simon Glass 
> Cc: Peng Fan 
> Cc: Sudeep Holla 
> ---
>
> Changes in v3:
> - Address comments about adding a new uclass and some sandbox test from
>   v2 in https://patchwork.ozlabs.org/project/uboot/list/?series=196253
> - New directory drivers/firmware/scmi/. The path mimics Linux kernel
>   source tree for the equivalent driver.
> - Split scmi.h (patch v2) into scmi_protocols.h, scmi_agent.h and
>   scmi_agent-uclass.h.
> - Create new uclass UCLASS_SCMI_AGENT.
> - Introduce a simple sandbox on that agent. Mailbox and smccc agents are
>   moved to specific commits in the series.
>
> Changes in v2:
> - Fix CONFIG_SCMI_FIRMWARE description with explicit SCMI reference.
> - Move struct, enum and macro definitions at source file top and
>   add inline comment description for the structures and local functions.
> - Replace rc with ret as return value local variable label.
> - Use explicit return 0 on successful return paths.
> - Replace EINVAL with more accurate error numbers.
> - Use dev_read_u32() instead of ofnode_read_u32(dev_ofnode(), ...).
> - Use memcpy_toio()/memcpy_fromio() when copying message payload
>   to/from IO memory.
> - Embed mailbox transport resources upon CONFIG_DM_MAILBOX and
>   SMCCC transport resources upon CONFIG_ARM_SMCCC.
>
> Note: review comments on defining a uclass and sandbox for SCMI
> transport drivers are NOT addressed in this v2. Main issue is that
> there is no driver/device defined for SCMI transport layer as well as
> and no defined compatible ID in the SCMI DT bindings documentation.
> ---
>  arch/sandbox/dts/test.dts  |  16 +++
>  arch/sandbox/include/asm/scmi_test.h   |  43 ++
>  configs/sandbox_defconfig  |   2 +
>  drivers/firmware/Kconfig   |   2 +
>  drivers/firmware/Makefile  |   1 +
>  drivers/firmware/scmi/Kconfig  |  17 +++
>  drivers/firmware/scmi/Makefile |   2 +
>  drivers/firmware/scmi/sandbox-scmi_agent.c | 147 +
>  drivers/firmware/scmi/scmi_agent-uclass.c  | 107 +++
>  include/dm/uclass-id.h |   1 +
>  include/scmi_agent-uclass.h|  24 
>  include/scmi_agent.h   |  68 ++
>  include/scmi_protocols.h   |  41 ++
>  test/dm/Makefile   |   1 +
>  test/dm/scmi.c |  38 ++
>  15 files changed, 510 insertions(+)
>  create mode 100644 arch/sandbox/include/asm/scmi_test.h
>  create mode 100644 drivers/firmware/scmi/Kconfig
>  create mode 100644 drivers/firmware/scmi/Makefile
>  create mode 100644 drivers/firmware/scmi/sandbox-scmi_agent.c
>  create mode 100644 drivers/firmware/scmi/scmi_agent-uclass.c
>  create mode 100644 include/scmi_agent-uclass.h
>  create mode 100644 include/scmi_agent.h
>  create mode 100644 include/scmi_protocols.h
>  create mode 100644 test/dm/scmi.c
>

Reviewed-by: Simon Glass 


Re: [PATCH v3 3/8] firmware: scmi: support Arm SMCCC transport

2020-09-08 Thread Simon Glass
Hi Etienne,

On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
 wrote:
>
> This change implements a SMCCC transport for SCMI exchanges. This
> implementation follows the Linux kernel as references implementation
> for SCMI message processing, using the SMT format for communication
> channel meta-data.
>
> Use of SMCCC transport in SCMI FDT bindings are defined in the Linux
> kernel DT bindings since v5.8. SMCCC with SMT is implemented in OP-TEE
> from tag 3.9.0 [2].
>
> Links: [2] https://github.com/OP-TEE/optee_os/commit/a58c4d706d23
> Signed-off-by: Etienne Carriere 
> Cc: Simon Glass 
> Cc: Peng Fan 
> Cc: Sudeep Holla 
> ---
>
> Changes in v3:
> - This is a followup of the SCMI agent patches posted in
>   https://patchwork.ozlabs.org/project/uboot/list/?series=196253
>   The v3 splits commits and introduces a new uclass as requested.
> - This patch implements the same Arm SMCCC SCMI agent as presented
>   in v2 but in its own source file smccc_agent.c, and based in smt.h.
> ---
>  drivers/firmware/scmi/Kconfig   |  4 +-
>  drivers/firmware/scmi/Makefile  |  1 +
>  drivers/firmware/scmi/smccc_agent.c | 95 +
>  3 files changed, 98 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/firmware/scmi/smccc_agent.c
>

Reviewed-by: Simon Glass 

nits below

> diff --git a/drivers/firmware/scmi/Kconfig b/drivers/firmware/scmi/Kconfig
> index c501bf4943..335d09c821 100644
> --- a/drivers/firmware/scmi/Kconfig
> +++ b/drivers/firmware/scmi/Kconfig
> @@ -15,5 +15,5 @@ config SCMI_FIRMWARE
>
>   Communications between agent (client) and the SCMI server are
>   based on message exchange. Messages can be exchange over tranport
> - channels as a mailbox device with some piece of identified shared
> - memory.
> + channels as a mailbox device or an Arm SMCCC service with some
> + piece of identified shared memory.
> diff --git a/drivers/firmware/scmi/Makefile b/drivers/firmware/scmi/Makefile
> index d22f53efe7..2f782bbd55 100644
> --- a/drivers/firmware/scmi/Makefile
> +++ b/drivers/firmware/scmi/Makefile
> @@ -1,4 +1,5 @@
>  obj-y  += scmi_agent-uclass.o
>  obj-y  += smt.o
> +obj-$(CONFIG_ARM_SMCCC)+= smccc_agent.o
>  obj-$(CONFIG_DM_MAILBOX)   += mailbox_agent.o
>  obj-$(CONFIG_SANDBOX)  += sandbox-scmi_agent.o
> diff --git a/drivers/firmware/scmi/smccc_agent.c 
> b/drivers/firmware/scmi/smccc_agent.c
> new file mode 100644
> index 00..90707710e2
> --- /dev/null
> +++ b/drivers/firmware/scmi/smccc_agent.c
> @@ -0,0 +1,95 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2020 Linaro Limited.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
That should go below the next one.
> +
> +#include 
> +#include 
> +
> +#include "smt.h"
> +
> +#define SMCCC_RET_NOT_SUPPORTED ((unsigned long)-1)
> +
> +/**
> + * struct scmi_smccc_channel - Description of an SCMI SMCCC transport
> + * @func_id:   SMCCC function ID used by the SCMI transport
> + * @smt:   Shared memory buffer
> + */
> +struct scmi_smccc_channel {
> +   ulong func_id;
> +   struct scmi_smt smt;
> +};
> +
> +static struct scmi_smccc_channel *scmi_smccc_get_priv(struct udevice *dev)
> +{
> +   return (struct scmi_smccc_channel *)dev_get_priv(dev);

You shouldn't need that cast

[..]

Regards,
Simon


Re: [PATCH v3 5/8] clk: add clock driver for SCMI agents

2020-09-08 Thread Simon Glass
On Mon, 7 Sep 2020 at 08:50, Etienne Carriere
 wrote:
>
> This change introduces a clock driver for SCMI agent devices. When
> SCMI agent and SCMI clock drivers are enabled, SCMI agent binds a
> clock device for each SCMI clock protocol devices enabled in the FDT.
>
> SCMI clock driver is embedded upon CONFIG_CLK_SCMI=y. If enabled,
> CONFIG_SCMI_AGENT is also enabled.
>
> SCMI Clock protocol is defined in the SCMI specification [1].
>
> Links: [1] 
> https://developer.arm.com/architectures/system-architectures/software-standards/scmi
> Signed-off-by: Etienne Carriere 
> Cc: Lukasz Majewski 
> Cc: Simon Glass 
> Cc: Peng Fan 
> Cc: Sudeep Holla 
> ---
>
> Changes in v3:
>
> Changes in v2:
> - CONFIG_CLK_SCMI depends on CONFIG_SCMI_FIRMWARE instead of
>   selecting CONFIG_SCMI_FIRMWARE.
> - Add inline comment description for structures and moves them to
>   source file top. Add/fixup some functions inline description comments.
> - Replace rc with ret as return value local variable label.
> - Fix scmi_clk_get_rate() return value to propagate error number.
> - Fix scmi_clk_set_rate() to request synchronous rate set operation:
>   drop flag SCMI_CLK_RATE_ASYNC_NORESP in the SCMI message payload.
> - Fix scmi_clk_set_rate() return value to return clock effective rate
>   on success.
> ---
>  drivers/clk/Kconfig   |  8 ++
>  drivers/clk/Makefile  |  1 +
>  drivers/clk/clk_scmi.c| 99 +++
>  drivers/firmware/scmi/scmi_agent-uclass.c |  3 +
>  include/scmi_protocols.h  | 78 ++
>  5 files changed, 189 insertions(+)
>  create mode 100644 drivers/clk/clk_scmi.c

Reviewed-by: Simon Glass 


Re: [PATCH v1] cmd: acpi: Print revisions in hex format

2020-09-08 Thread Andy Shevchenko
On Tue, Sep 8, 2020 at 5:58 PM Wolfgang Wallner
 wrote:
> -"Andy Shevchenko"  schrieb: -
> > Betreff: [PATCH v1] cmd: acpi: Print revisions in hex format
> >
> > The revisions are usually dates in hex-decimal format representing
> > mmdd. Print them in hex to see this clearly.
> >
> > Before:
> >   ...
> >   FACP 000e5420 f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0)
> >   DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870)
> >   ...
> > After:
> >   ...
> >   FACP 000e5420 f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0)
> >   DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 1 INTL 20200326)
> >   ...
> >
> > Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command")
> > Cc: Wolfgang Wallner 
> > Signed-off-by: Andy Shevchenko 
> > ---
> >  cmd/acpi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Reviewed-by: Wolfgang Wallner 
> Tested-by: Wolfgang Wallner 
> Tested on a custom Apollolake board.

Thanks!

> Related to "acpi list":
> During my recent ACPI debugging I found it very useful to have the checksum
> printed for each table with "acpi list". Would there be interest to have that
> upstream? If so I would send a patch.

Can you elaborate what was the problem that checksum helped?

-- 
With Best Regards,
Andy Shevchenko


Re: [PATCH v4 1/2] drivers: gpio: Add a managed API to get a GPIO from the device-tree

2020-09-08 Thread Pratyush Yadav
On 08/09/20 04:12PM, Heinrich Schuchardt wrote:
> On 08.09.20 07:40, Pratyush Yadav wrote:
> > From: Jean-Jacques Hiblot 
> >
> > Add managed functions to get a gpio from the devce-tree, based on a
> > property name (minus the '-gpios' suffix) and optionally an index.
> >
> > When the device is unbound, the GPIO is automatically released and the
> > data structure is freed.
> >
> > Signed-off-by: Jean-Jacques Hiblot 
> > Reviewed-by: Simon Glass 
> > Signed-off-by: Pratyush Yadav 
> > ---
> >  drivers/gpio/gpio-uclass.c | 71 ++
> >  include/asm-generic/gpio.h | 47 +
> >  2 files changed, 118 insertions(+)
> >
> > diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> > index 9c53299b6a..0c01413b58 100644
> > --- a/drivers/gpio/gpio-uclass.c
> > +++ b/drivers/gpio/gpio-uclass.c
> > @@ -6,6 +6,8 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -1209,6 +1211,75 @@ int gpio_dev_request_index(struct udevice *dev, 
> > const char *nodename,
> >  flags, 0, dev);
> >  }
> >
> > +static void devm_gpiod_release(struct udevice *dev, void *res)
> > +{
> > +   dm_gpio_free(dev, res);
> > +}
> > +
> > +static int devm_gpiod_match(struct udevice *dev, void *res, void *data)
> > +{
> > +   return res == data;
> > +}
> > +
> > +struct gpio_desc *devm_gpiod_get_index(struct udevice *dev, const char *id,
> > +  unsigned int index, int flags)
> > +{
> > +   int rc;
> > +   struct gpio_desc *desc;
> > +   char *propname;
> > +   static const char suffix[] = "-gpios";
> > +
> > +   propname = malloc(strlen(id) + sizeof(suffix));
> > +   if (!propname) {
> > +   rc = -ENOMEM;
> > +   goto end;
> > +   }
> > +
> > +   strcpy(propname, id);
> > +   strcat(propname, suffix);
> > +
> > +   desc = devres_alloc(devm_gpiod_release, sizeof(struct gpio_desc),
> > +   __GFP_ZERO);
> > +   if (unlikely(!desc)) {
> > +   rc = -ENOMEM;
> > +   goto end;
> > +   }
> > +
> > +   rc = gpio_request_by_name(dev, propname, index, desc, flags);
> > +
> > +end:
> > +   if (propname)
> > +   free(propname);
> > +
> > +   if (rc)
> > +   return ERR_PTR(rc);
> > +
> > +   devres_add(dev, desc);
> > +
> > +   return desc;
> > +}
> > +
> > +struct gpio_desc *devm_gpiod_get_index_optional(struct udevice *dev,
> > +   const char *id,
> > +   unsigned int index,
> > +   int flags)
> > +{
> > +   struct gpio_desc *desc = devm_gpiod_get_index(dev, id, index, flags);
> > +
> > +   if (IS_ERR(desc))
> > +   return NULL;
> > +
> > +   return desc;
> > +}
> > +
> > +void devm_gpiod_put(struct udevice *dev, struct gpio_desc *desc)
> > +{
> > +   int rc;
> > +
> > +   rc = devres_release(dev, devm_gpiod_release, devm_gpiod_match, desc);
> > +   WARN_ON(rc);
> > +}
> > +
> >  static int gpio_post_bind(struct udevice *dev)
> >  {
> > struct udevice *child;
> > diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> > index a57dd2665c..ad6979a8ce 100644
> > --- a/include/asm-generic/gpio.h
> > +++ b/include/asm-generic/gpio.h
> > @@ -701,4 +701,51 @@ int gpio_get_number(const struct gpio_desc *desc);
> >   */
> >  int gpio_get_acpi(const struct gpio_desc *desc, struct acpi_gpio *gpio);
> >
> > +/**
> > + * devm_gpiod_get_index - Resource-managed gpiod_get()
> > + * @dev:   GPIO consumer
> > + * @con_id:function within the GPIO consumer
> > + * @index: index of the GPIO to obtain in the consumer
> > + * @flags: optional GPIO initialization flags
> > + *
> > + * Managed gpiod_get(). GPIO descriptors returned from this function are
> > + * automatically disposed on driver detach.
> 
> You pass in a device but write "driver detach". Shouldn't the object be
> "device" in the description as in your commit message?

Yes, it should be device.
 
> Devices have methods remove() and unbind() but no detach. In the commit
> message you speak of unbind(). Please, don't use alternative language.

Ok.
 
> Please, include the API in the HTML documentation created by 'make
> htmldocs'.

I tried searching for the GPIO API documentation under doc/ but I can't 
find anything. README.gpio doesn't mention it anywhere and doc/api/ has 
no file for gpio. Where do I document the newly added APIs then?
 
> Why did you choose the unbind() and not the remove() event for releasing
> the GPIOs?

I did not. Whoever added the devres API did (git blames Masahiro 
Yamada). device_unbind() calls devres_release_all() so as a consequence 
GPIO is released when the device is unbound.
 
> Best regards
> 
> Heinrich
> 
> > + * Return the GPIO descriptor corresponding to the function con_id of 
> > device
> > + * dev, -ENOENT if no GPIO has been assigned to the requested 

Re: [PATCH v1] cmd: acpi: Print revisions in hex format

2020-09-08 Thread Wolfgang Wallner
Hi Andy,

-"Andy Shevchenko"  schrieb: -
> Betreff: [PATCH v1] cmd: acpi: Print revisions in hex format
> 
> The revisions are usually dates in hex-decimal format representing
> mmdd. Print them in hex to see this clearly.
> 
> Before:
>   ...
>   FACP 000e5420 f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0)
>   DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870)
>   ...
> After:
>   ...
>   FACP 000e5420 f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0)
>   DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 1 INTL 20200326)
>   ...
> 
> Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command")
> Cc: Wolfgang Wallner 
> Signed-off-by: Andy Shevchenko 
> ---
>  cmd/acpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Wolfgang Wallner 
Tested-by: Wolfgang Wallner 
Tested on a custom Apollolake board.

Related to "acpi list":
During my recent ACPI debugging I found it very useful to have the checksum
printed for each table with "acpi list". Would there be interest to have that
upstream? If so I would send a patch.

regards, Wolfgang




[PATCH v1] cmd: acpi: Print revisions in hex format

2020-09-08 Thread Andy Shevchenko
The revisions are usually dates in hex-decimal format representing
mmdd. Print them in hex to see this clearly.

Before:
  ...
  FACP 000e5420 f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0)
  DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870)
  ...
After:
  ...
  FACP 000e5420 f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0)
  DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 1 INTL 20200326)
  ...

Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command")
Cc: Wolfgang Wallner 
Signed-off-by: Andy Shevchenko 
---
 cmd/acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cmd/acpi.c b/cmd/acpi.c
index 085a3a650d1a..a3419b42b556 100644
--- a/cmd/acpi.c
+++ b/cmd/acpi.c
@@ -26,7 +26,7 @@ static void dump_hdr(struct acpi_table_header *hdr)
printf("%.*s %08lx %06x", ACPI_NAME_LEN, hdr->signature,
   (ulong)map_to_sysmem(hdr), hdr->length);
if (has_hdr) {
-   printf(" (v%02d %.6s %.8s %u %.4s %d)\n", hdr->revision,
+   printf(" (v%02d %.6s %.8s %x %.4s %x)\n", hdr->revision,
   hdr->oem_id, hdr->oem_table_id, hdr->oem_revision,
   hdr->aslc_id, hdr->aslc_revision);
} else {
-- 
2.28.0



Re: [PATCH v4 1/2] drivers: gpio: Add a managed API to get a GPIO from the device-tree

2020-09-08 Thread Heinrich Schuchardt
On 08.09.20 07:40, Pratyush Yadav wrote:
> From: Jean-Jacques Hiblot 
>
> Add managed functions to get a gpio from the devce-tree, based on a
> property name (minus the '-gpios' suffix) and optionally an index.
>
> When the device is unbound, the GPIO is automatically released and the
> data structure is freed.
>
> Signed-off-by: Jean-Jacques Hiblot 
> Reviewed-by: Simon Glass 
> Signed-off-by: Pratyush Yadav 
> ---
>  drivers/gpio/gpio-uclass.c | 71 ++
>  include/asm-generic/gpio.h | 47 +
>  2 files changed, 118 insertions(+)
>
> diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
> index 9c53299b6a..0c01413b58 100644
> --- a/drivers/gpio/gpio-uclass.c
> +++ b/drivers/gpio/gpio-uclass.c
> @@ -6,6 +6,8 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1209,6 +1211,75 @@ int gpio_dev_request_index(struct udevice *dev, const 
> char *nodename,
>flags, 0, dev);
>  }
>
> +static void devm_gpiod_release(struct udevice *dev, void *res)
> +{
> + dm_gpio_free(dev, res);
> +}
> +
> +static int devm_gpiod_match(struct udevice *dev, void *res, void *data)
> +{
> + return res == data;
> +}
> +
> +struct gpio_desc *devm_gpiod_get_index(struct udevice *dev, const char *id,
> +unsigned int index, int flags)
> +{
> + int rc;
> + struct gpio_desc *desc;
> + char *propname;
> + static const char suffix[] = "-gpios";
> +
> + propname = malloc(strlen(id) + sizeof(suffix));
> + if (!propname) {
> + rc = -ENOMEM;
> + goto end;
> + }
> +
> + strcpy(propname, id);
> + strcat(propname, suffix);
> +
> + desc = devres_alloc(devm_gpiod_release, sizeof(struct gpio_desc),
> + __GFP_ZERO);
> + if (unlikely(!desc)) {
> + rc = -ENOMEM;
> + goto end;
> + }
> +
> + rc = gpio_request_by_name(dev, propname, index, desc, flags);
> +
> +end:
> + if (propname)
> + free(propname);
> +
> + if (rc)
> + return ERR_PTR(rc);
> +
> + devres_add(dev, desc);
> +
> + return desc;
> +}
> +
> +struct gpio_desc *devm_gpiod_get_index_optional(struct udevice *dev,
> + const char *id,
> + unsigned int index,
> + int flags)
> +{
> + struct gpio_desc *desc = devm_gpiod_get_index(dev, id, index, flags);
> +
> + if (IS_ERR(desc))
> + return NULL;
> +
> + return desc;
> +}
> +
> +void devm_gpiod_put(struct udevice *dev, struct gpio_desc *desc)
> +{
> + int rc;
> +
> + rc = devres_release(dev, devm_gpiod_release, devm_gpiod_match, desc);
> + WARN_ON(rc);
> +}
> +
>  static int gpio_post_bind(struct udevice *dev)
>  {
>   struct udevice *child;
> diff --git a/include/asm-generic/gpio.h b/include/asm-generic/gpio.h
> index a57dd2665c..ad6979a8ce 100644
> --- a/include/asm-generic/gpio.h
> +++ b/include/asm-generic/gpio.h
> @@ -701,4 +701,51 @@ int gpio_get_number(const struct gpio_desc *desc);
>   */
>  int gpio_get_acpi(const struct gpio_desc *desc, struct acpi_gpio *gpio);
>
> +/**
> + * devm_gpiod_get_index - Resource-managed gpiod_get()
> + * @dev: GPIO consumer
> + * @con_id:  function within the GPIO consumer
> + * @index:   index of the GPIO to obtain in the consumer
> + * @flags:   optional GPIO initialization flags
> + *
> + * Managed gpiod_get(). GPIO descriptors returned from this function are
> + * automatically disposed on driver detach.

You pass in a device but write "driver detach". Shouldn't the object be
"device" in the description as in your commit message?

Devices have methods remove() and unbind() but no detach. In the commit
message you speak of unbind(). Please, don't use alternative language.

Please, include the API in the HTML documentation created by 'make
htmldocs'.

Why did you choose the unbind() and not the remove() event for releasing
the GPIOs?

Best regards

Heinrich

> + * Return the GPIO descriptor corresponding to the function con_id of device
> + * dev, -ENOENT if no GPIO has been assigned to the requested function, or
> + * another IS_ERR() code if an error occurred while trying to acquire the 
> GPIO.
> + */
> +struct gpio_desc *devm_gpiod_get_index(struct udevice *dev, const char *id,
> +unsigned int index, int flags);
> +
> +#define devm_gpiod_get(dev, id, flags) devm_gpiod_get_index(dev, id, 0, 
> flags)
> +/**
> + * gpiod_get_optional - obtain an optional GPIO for a given GPIO function
> + * @dev: GPIO consumer, can be NULL for system-global GPIOs
> + * @con_id: function within the GPIO consumer
> + * @index:   index of the GPIO to obtain in the consumer
> + * @flags: optional GPIO initialization flags
> + *
> + * This is equivalent to 

Re: [PATCH v2 01/16] x86: Update the bootparam header

2020-09-08 Thread Andy Shevchenko
On Tue, Sep 01, 2020 at 04:23:45PM +0800, Bin Meng wrote:
> On Sun, Aug 30, 2020 at 5:42 AM Simon Glass  wrote:
> >
> > This header is missing a few of the newer features from the specification.
> > Add these as well as a link to the spec. Also use the BIT() macros where
> > appropriate.

> > +/**
> > + * struct setup_header - Information needed by Linux to boot
> > + *
> > + * See https://www.kernel.org/doc/html/latest/x86/boot.html
> 
> Now I am confused. This kernel document says:
> 
> Protocol 2.14 BURNT BY INCORRECT COMMIT
> ae7e1238e68f2a472a125673ab506d49158c1889 (x86/boot: Add ACPI RSDP
> address to setup_header) DO NOT USE!!! ASSUME SAME AS 2.13.

Where did you get this "DO NOT USE!!! ASSUME SAME AS 2.13" from?


> However in U-Boot there was a commit from Andy saying that:
> 
> commit d905aa8a4277e200e11fdf6d73a7c76d0e6f34a4 (<=== U-Boot commit)
> Author: Andy Shevchenko 
> Date:   Fri Sep 13 18:42:00 2019 +0300
> 
> x86: zImage: Propagate acpi_rsdp_addr to kernel via boot parameters
> 
> ...
> 
> after upstream got eventually the Linux kernel
> 
> commit e6e094e053af75cbc164e950814d3d084fb1e698
> Author: Juergen Gross 
> Date:   Tue Nov 20 08:25:29 2018 +0100
> 
> x86/acpi, x86/boot: Take RSDP address from boot params if available
> 
> So there are 2 commits in the Linux kernel that do the same thing, one
> in boot_params, the other one in setup_header?

I think this
384184044981 ("x86/boot: Mostly revert commit ae7e1238e68f2a ("Add ACPI RSDP 
address to setup_header")")
explains what happened.

> e6e094e053af75cbc164e950814d3d084fb1e698 : x86/acpi, x86/boot: Take
> RSDP address from boot params if available
> ae7e1238e68f2a472a125673ab506d49158c1889 : x86/boot: Add ACPI RSDP
> address to setup_header
> 
> > + */
> >  struct setup_header {
> > __u8setup_sects;
> > __u16   root_flags;
> > @@ -43,15 +48,16 @@ struct setup_header {
> > __u16   kernel_version;
> > __u8type_of_loader;
> > __u8loadflags;
> > -#define LOADED_HIGH(1<<0)
> > -#define QUIET_FLAG (1<<5)
> > -#define KEEP_SEGMENTS  (1<<6)
> > -#define CAN_USE_HEAP   (1<<7)
> > +#define LOADED_HIGHBIT(0)
> > +#define KASLR_FLAG BIT(1)
> > +#define QUIET_FLAG BIT(5)
> > +#define KEEP_SEGMENTS  BIT(6)  /* Obsolete */
> > +#define CAN_USE_HEAP   BIT(7)
> > __u16   setup_move_size;
> > __u32   code32_start;
> > __u32   ramdisk_image;
> > __u32   ramdisk_size;
> > -   __u32   bootsect_kludge;
> > +   __u32   bootsect_kludge;/* Obsolete */
> > __u16   heap_end_ptr;
> > __u8ext_loader_ver;
> > __u8ext_loader_type;
> > @@ -59,7 +65,13 @@ struct setup_header {
> > __u32   initrd_addr_max;
> > __u32   kernel_alignment;
> > __u8relocatable_kernel;
> > -   __u8_pad2[3];
> > +   u8  min_alignment;
> > +#define XLF_KERNEL_64  BIT(0)
> > +#define XLF_CAN_BE_LOADED_ABOVE_4G BIT(1)
> > +#define XLF_EFI_HANDOVER_32BIT(2)
> > +#define XLF_EFI_HANDOVER_64BIT(3)
> > +#define XLF_EFI_KEXEC  BIT(4)
> > +   u16 xloadflags;
> > __u32   cmdline_size;
> > __u32   hardware_subarch;
> > __u64   hardware_subarch_data;
> > @@ -69,6 +81,7 @@ struct setup_header {
> > __u64   pref_address;
> > __u32   init_size;
> > __u32   handover_offset;
> > +   u32 kernel_info_offset;
> >  } __attribute__((packed));
> >
> >  struct sys_desc_table {
> 
> Regards,
> Bin

-- 
With Best Regards,
Andy Shevchenko




[PATCH v1] x86: edison: Move config SYS_MALLOC_LEN to Kconfig

2020-09-08 Thread Andy Shevchenko
This patch moves the the config SYS_MALLOC_LEN to Kconfig
as it is already done for zynq arch in commit 01aa5b8f0503
("Kconfig: Move config SYS_MALLOC_LEN to Kconfig for zynq").

Signed-off-by: Andy Shevchenko 
---
 board/intel/edison/Kconfig | 3 +++
 include/configs/edison.h   | 4 
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/board/intel/edison/Kconfig b/board/intel/edison/Kconfig
index ef9b14aa2bfd..05d65445e407 100644
--- a/board/intel/edison/Kconfig
+++ b/board/intel/edison/Kconfig
@@ -12,6 +12,9 @@ config SYS_SOC
 config SYS_CONFIG_NAME
default "edison"
 
+config SYS_MALLOC_LEN
+   default 0x0800
+
 config SYS_TEXT_BASE
default 0x01101000
 
diff --git a/include/configs/edison.h b/include/configs/edison.h
index 606c656a7272..0e1205bdb54c 100644
--- a/include/configs/edison.h
+++ b/include/configs/edison.h
@@ -23,10 +23,6 @@
 #define CONFIG_SYS_MONITOR_BASECONFIG_SYS_TEXT_BASE
 #define CONFIG_SYS_MONITOR_LEN (256 * 1024)
 
-#define CONFIG_SYS_MALLOC_LEN  (128 * 1024 * 1024)
-
-/* Environment */
-
 /* RTC */
 #define CONFIG_SYS_ISA_IO_BASE_ADDRESS 0
 
-- 
2.28.0



make_fit_atf.py and multiple U-Boot ELF segments

2020-09-08 Thread Tom Rini
Hey all,

As part of reviewing
http://patchwork.ozlabs.org/project/uboot/list/?series=187450=*
I've run in to an odd problem.  With patch 4/7 of that series, building
firefly-rk3399 fails on the make_fit_atf.py step as those changes end up
with a U-Boot ELF that has len(segments) of 2, rather than 1, and the
generator blows up.  It's not clear to me if we can just check for
non-zero number of segments here or what.  Thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] defconfig: espressobin: enable NET_RANDOM_ETHADDR

2020-09-08 Thread Tom Rini
On Tue, Sep 08, 2020 at 10:14:15AM +0200, Andre Heider wrote:
> On 08/09/2020 09:42, Pali Rohár wrote:
> > On Tuesday 08 September 2020 08:35:00 Andre Heider wrote:
> > > The hardware does not provide a MAC address. Enable this so that
> > > network access works with just the default environment.
> > 
> > Well, this is not fully truth as MAC address is stored in SPI, just in
> > non-standard format, in U-Boot env stored in env partition and it is
> > hard to use outside of U-Boot, plus easy to erase / overwrite / lost.
> 
> True, but updating the bootloader usually implies wiping the env, so it's
> very easy to lose it.

It most certainly should NOT wipe out the existing environment, it
should be using the same environment location as before.

> > I'm not a big fan of this change. This looks like a workaround / hack
> > for boards where MAC address was erased (e.g. by broken U-Boot distro
> > scripts) or for early boards where MAC address was not written at all
> > (as I was told).
> > 
> > And on these boards this patch would cause that U-Boot would see on
> > every boot different MAC address. This would cause another mess in
> > network for U-Boot netboot as DHCP/TFTP server would see for one board
> > every time different MAC address.
> > 
> > Is not really better to instruct user how to fix board where e.g. broken
> > distro scripts erased MAC address? We have already paragraph in
> > README.marvell about it.
> > 
> > Also this change affects "default" defconfig value. And based on above
> > arguments I do not think that this change should be enabled by default.
> > 
> > I understand that for some situations it may be useful (e.g. mass board
> > reparation process via netboot), but as this is config option, users in
> > such situation can enable this option manually.
> > 
> > I think that for default behavior is not provide network access in
> > U-Boot if for some reasons factory permanent MAC address was removed.
> > User can easier and faster detect this issue and fix it.
> 
> It can be argued both ways I guess. If this option wouldn't make sense it
> wouldn't exist.
> 
> Out of the box working network access is probably what most users care
> about.
> 
> Changing this default means that you need to build the whole firmware
> yourself, and let's face it: building it for this board is a total
> clusterfuck. Most users will just download a binary. I don't care too much
> for this patch, but I would consider what fits most users.
> 
> Right now most users will probably run the downstream binaries provided by
> armbian, and as you know, that even has single hardcoded MAC, used for all
> boards. So this would already be an improvement ;)

Note that when CONFIG_NET_RANDOM_ETHADDR is set, we only use a random
MAC address when we haven't found one either on the hardware or
environment.  It also prints a warning that you are using a random MAC,
so if it's documented on how to recover the real MAC a user should see
that warning and fix it.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 5/5] arm: mach-omap2: am33xx: Add device structure for spi

2020-09-08 Thread Faiz Abbas
Hi Vignesh,

On 07/09/20 5:49 pm, Vignesh Raghavendra wrote:
> Hi,
> 
> On 9/7/20 4:02 PM, Faiz Abbas wrote:
>> Hi Vignesh,
>>
>> On 07/09/20 1:48 pm, Vignesh Raghavendra wrote:
>>>
>>>
>>> On 9/7/20 12:36 PM, Faiz Abbas wrote:
 Hi Lokesh,

 On 07/09/20 12:08 pm, Lokesh Vutla wrote:
>
> [...]
>>  struct omap3_spi_priv {
>>  struct mcspi *regs;
>>  unsigned int cs;
>> diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
>> index 9c4ef369c5..db1a89ad30 100644
>> --- a/include/configs/am335x_evm.h
>> +++ b/include/configs/am335x_evm.h
>> @@ -281,6 +281,10 @@
>>  #endif
>>  
>>  /* SPI flash. */
>> +#if CONFIG_IS_ENABLED(DM_SPI)
>> +#define AM33XX_SPI_BASE 0x4803
>>>
>>> Could this be more specific? AM33XX_SPI_BASE?
>>
>> Isn't there only one McSPI instance in am335x?
>>
> 
> No, there are 2 SPI ports on AM335x (see arch/arm/dts/am33xx.dtsi).

I see. I'll make the change in v2.

> 
>>>
>> +#define AM33XX_SPI_OFFSET   (AM33XX_SPI_BASE + 
>> OMAP4_MCSPI_REG_OFFSET)
>
> Can we get the SPI base from DT?
>

 We are doing that in U-boot (see the ofdata_to_platdata() callback in 
 patch 4).
 We need hardcoded static platdata for SPL. Was this not clear from the 
 commit
 message?

>>>
>>> Then why not move these defines to arch/arm/mach-omap2/am33xx/board.c as
>>> well?
>>
>> All the other base addresses used in arch/arm/mach-omap2/am33xx/board.c are 
>> included
>> from here. For example see UART platdata (struct ns16550_platdata 
>> am33xx_serial[]).
>>
> 
> UART is bad example as those #defines were added in 2012 which predate
> platdata introduction...
> 
> Besides what happens when derivative of AM335x (see
> include/configs/am335x_*.h) want to enable SPI boot? Would each such
> files need to duplicate this snippet?
> 
Ok. I'll move the base address to the board file.

Thanks,
Faiz


Re: [PATCH v3 1/7] riscv: Rework riscv timer driver to only support S-mode

2020-09-08 Thread Sean Anderson
On 9/8/20 3:57 AM, Pragnesh Patel wrote:
> Hi Sean,
> 
>> -Original Message-
>> From: Sean Anderson 
>> Sent: 01 September 2020 16:02
>> To: u-boot@lists.denx.de
>> Cc: Rick Chen ; Bin Meng ;
>> Pragnesh Patel ; Sean Anderson
>> ; Bin Meng ; Anup Patel
>> 
>> Subject: [PATCH v3 1/7] riscv: Rework riscv timer driver to only support 
>> S-mode
>>
>> [External Email] Do not click links or attachments unless you recognize the
>> sender and know the content is safe
>>
>> The riscv-timer driver currently serves as a shim for several riscv timer 
>> drivers.
>> This is not too desirable because it bypasses the usual timer selection via 
>> the
>> driver model. There is no easy way to specify an alternate timing driver, or 
>> have
>> the tick rate depend on the cpu's configured frequency. The timer drivers 
>> also do
>> not have device structs, and so have to rely on storing parameters in gd_t. 
>> Lastly,
>> there is no initialization call, so driver init is done in the same function 
>> which
>> reads the time. This can result in confusing error messages. To a user, it 
>> looks like
>> the driver failed when trying to read the time, whereas it may have failed 
>> while
>> initializing.
>>
>> This patch removes the shim functionality from the riscv-timer driver, and 
>> has it
>> instead implement the former rdtime.c timer driver. This is because existing 
>> u-
>> boot users who pass in a device tree (e.g. qemu) do not create a timer 
>> device for
>> S-mode u-boot. The existing behavior of creating the riscv-timer device in 
>> the
>> riscv cpu driver must be kept. The actual reading of the CSRs has been 
>> redone in
>> the style of Linux's get_cycles64.
>>
>> Signed-off-by: Sean Anderson 
>> Reviewed-by: Bin Meng 
>> ---
>>
>> (no changes since v2)
>>
>> Changes in v2:
>> - Remove RISCV_RDTIME KConfig option
>>
>> arch/riscv/Kconfig  |  8 
>> arch/riscv/lib/Makefile |  1 -
>> arch/riscv/lib/rdtime.c | 38 
>> drivers/timer/Kconfig   |  6 +++---
>> drivers/timer/riscv_timer.c | 39 +++--
>> 5 files changed, 23 insertions(+), 69 deletions(-)  delete mode 100644
>> arch/riscv/lib/rdtime.c
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 
>> 009a545fcf..21e6690f4d
>> 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -185,14 +185,6 @@ config ANDES_PLMT
>>  The Andes PLMT block holds memory-mapped mtime register
>>  associated with timer tick.
>>
>> -config RISCV_RDTIME
>> -   bool
>> -   default y if RISCV_SMODE || SPL_RISCV_SMODE
>> -   help
>> - The provides the riscv_get_time() API that is implemented using the
>> - standard rdtime instruction. This is the case for S-mode U-Boot, 
>> and
>> - is useful for processors that support rdtime in M-mode too.
>> -
>> config SYS_MALLOC_F_LEN
>>default 0x1000
>>
>> diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index
>> 6c503ff2b2..10ac5b06d3 100644
>> --- a/arch/riscv/lib/Makefile
>> +++ b/arch/riscv/lib/Makefile
>> @@ -15,7 +15,6 @@ obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
>> obj-$(CONFIG_ANDES_PLIC) += andes_plic.o
>> obj-$(CONFIG_ANDES_PLMT) += andes_plmt.o  else
>> -obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
>> obj-$(CONFIG_SBI) += sbi.o
>> obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
>> endif
>> diff --git a/arch/riscv/lib/rdtime.c b/arch/riscv/lib/rdtime.c deleted file 
>> mode
>> 100644 index e128d7fce6..00
>> --- a/arch/riscv/lib/rdtime.c
>> +++ /dev/null
>> @@ -1,38 +0,0 @@
>> -// SPDX-License-Identifier: GPL-2.0+
>> -/*
>> - * Copyright (C) 2018, Anup Patel 
>> - * Copyright (C) 2018, Bin Meng 
>> - *
>> - * The riscv_get_time() API implementation that is using the
>> - * standard rdtime instruction.
>> - */
>> -
>> -#include 
>> -
>> -/* Implement the API required by RISC-V timer driver */ -int 
>> riscv_get_time(u64
>> *time) -{ -#ifdef CONFIG_64BIT
>> -   u64 n;
>> -
>> -   __asm__ __volatile__ (
>> -   "rdtime %0"
>> -   : "=r" (n));
>> -
>> -   *time = n;
>> -#else
>> -   u32 lo, hi, tmp;
>> -
>> -   __asm__ __volatile__ (
>> -   "1:\n"
>> -   "rdtimeh %0\n"
>> -   "rdtime %1\n"
>> -   "rdtimeh %2\n"
>> -   "bne %0, %2, 1b"
>> -   : "=" (hi), "=" (lo), "=" (tmp));
>> -
>> -   *time = ((u64)hi << 32) | lo;
>> -#endif
>> -
>> -   return 0;
>> -}
>> diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index
>> 637024445c..b85fa33e47 100644
>> --- a/drivers/timer/Kconfig
>> +++ b/drivers/timer/Kconfig
>> @@ -144,10 +144,10 @@ config OMAP_TIMER
>>
>> config RISCV_TIMER
>>bool "RISC-V timer support"
>> -   depends on TIMER && RISCV
>> +   depends on TIMER && RISCV_SMODE
> 
> What about SPL_RISCV_SMODE ?

That should be added.

>>help
>> - Select this to enable support for the timer as 

RE: [PATCH v3 7/7] riscv: Update SiFive device tree for new CLINT driver

2020-09-08 Thread Pragnesh Patel
>-Original Message-
>From: Sean Anderson 
>Sent: 01 September 2020 16:02
>To: u-boot@lists.denx.de
>Cc: Rick Chen ; Bin Meng ;
>Pragnesh Patel ; Sean Anderson
>
>Subject: [PATCH v3 7/7] riscv: Update SiFive device tree for new CLINT driver
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>We may need to add a clock-frequency binding like for the K210.
>
>Signed-off-by: Sean Anderson 
>---
>This patch builds but has NOT been tested.
>
>Changes in v3:
>- Rebase
>
>Changes in v2:
>- Fix SiFive CLINT not getting tick-rate from rtcclk
>
> arch/riscv/dts/fu540-c000-u-boot.dtsi   | 8 ++--
> arch/riscv/dts/hifive-unleashed-a00-u-boot.dtsi | 4 
> 2 files changed, 10 insertions(+), 2 deletions(-)

Reviewed-by: Pragnesh Patel 



RE: [PATCH v3 4/7] riscv: Rework Sifive CLINT as UCLASS_TIMER driver

2020-09-08 Thread Pragnesh Patel
>-Original Message-
>From: Sean Anderson 
>Sent: 01 September 2020 16:02
>To: u-boot@lists.denx.de
>Cc: Rick Chen ; Bin Meng ;
>Pragnesh Patel ; Sean Anderson
>
>Subject: [PATCH v3 4/7] riscv: Rework Sifive CLINT as UCLASS_TIMER driver
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>This converts the clint driver from the riscv-specific interface to be a 
>DM-based
>UCLASS_TIMER driver. We also need to re-add the initialization for IPI back 
>into
>the SPL code. This was previously implicitly done when the timer was 
>initialized.
>In addition, the SiFive DDR driver previously implicitly depended on the CLINT 
>to
>select REGMAP.
>
>Unlike Andes's PLMT/PLIC (which AFAIK never have anything pass it a dtb), the
>SiFive CLINT is part of the device tree passed in by qemu. This device tree 
>doesn't
>have a clocks or clock-frequency property on clint, so we need to fall back on 
>the
>timebase-frequency property. Perhaps in the future we can get a clock-frequency
>property added to the qemu dtb.
>
>Unlike with the Andes PLMT, the Sifive CLINT is also an IPI controller.
>RISCV_SYSCON_CLINT is retained for this purpose.
>
>Signed-off-by: Sean Anderson 
>---
>This patch builds but has only been tested on the K210 and QEMU. It has NOT
>been tested on a HiFive.
>
>Changes in v3:
>- Don't initialize the IPI in spl_invoke_opensbi. Further testing has
>  revealed it to be unnecessary.
>
> arch/riscv/Kconfig|  4 --
> arch/riscv/lib/sifive_clint.c | 87 +++
> drivers/ram/sifive/Kconfig|  2 +
> 3 files changed, 59 insertions(+), 34 deletions(-)

Reviewed-by: Pragnesh Patel 



Re: Changing U-boot relocation addres to SRAM (instead of DRAM)

2020-09-08 Thread Joakim Tjernlund
On Mon, 2020-09-07 at 22:24 +0300, Yusuf Altıparmak wrote:
> CAUTION: This email originated from outside of the organization. Do not click 
> links or open attachments unless you recognize the sender and know the 
> content is safe.
> 
> 
> Hello,
> 
> I want to modify U-boot to relocate itself to on-board SRAM of chip (T1042
> demo board) instead of DRAM.
> 
> At first, there is call list in u-boot/common/board_f.c and it has a line
> to call dram_init. I will change this line with my custom sram_init
> function.
> 
> [image: Capture.PNG]
> 
> The function board_f() is being called in start.S which is located at
> u-boot/arch/powerpc/cpu/mpc85xx/
> 
> start.S file also includes a region to relocate itself by changing Link
> Register. Code region is:
> 
> */**
> ** void relocate_code(addr_sp, gd, addr_moni)*
> ***
> ** This "function" does not return, instead it continues in RAM*
> ** after relocating the monitor code.*
> ***
> ** r3 = dest*
> ** r4 = src*
> ** r5 = length in bytes*
> ** r6 = cachelinesize*
> **/*
> *.globl relocate_code*
> *relocate_code:*
> *mr r1,r3 /* Set new stack pointer */*
> *mr r9,r4 /* Save copy of Init Data pointer */*
> *mr r10,r5 /* Save copy of Destination Address */*
> 
> *GET_GOT*
> *#ifndef CONFIG_SPL_SKIP_RELOCATE*
> *mr r3,r5 /* Destination Address */*
> *lis r4,CONFIG_SYS_MONITOR_BASE@h /* Source Address */*
> *ori r4,r4,CONFIG_SYS_MONITOR_BASE@l*
> *lwz r5,GOT(__init_end)*
> *sub r5,r5,r4*
> *li r6,CONFIG_SYS_CACHELINE_SIZE /* Cache Line Size */*
> 
> 
> My question is, in above code, how relocate_code() function is calculating
> the jump address of Link Register (CONFIG_SYS_MONITOR_BASE@h).
> 
> What should be done to change it to point SRAM of t1042 after we initialize
> it in board_init_f() function. ?

I don't think you should change anything here. Just make gd->relocaddr point to 
somewhere in SRAM

 Jocke



Re: [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Chunfeng Yun
Hi Marek,

 I've sent out v4;

Hi Frank,

  Please forward this email to Marek, thanks a lot


On Tue, 2020-09-08 at 13:13 +0200, Marek Vasut wrote:
> On 9/8/20 3:44 AM, Bin Meng wrote:
> > On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun  
> > wrote:
> >>
> >> Use readx_poll_sleep_timeout() to poll the register status
> >>
> >> Signed-off-by: Chunfeng Yun 
> >> ---
> >> v3: no changes
> >>
> >> v2: fix typo of title suggested by Frank
> >> ---
> >>  drivers/usb/host/xhci.c | 25 +++--
> >>  1 file changed, 11 insertions(+), 14 deletions(-)
> >>
> > 
> > Reviewed-by: Bin Meng 
> 
> Thanks. So now I have half of a RESEND patchset with RB tags, and
> another half of just RB tags without patchset. Can someone of you please
> collect the tags and resend the patchset once more with the collected
> tags, so it's all in one place ?
> 
> Thank you



Re: [PATCH v3 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Marek Vasut
On 9/8/20 3:44 AM, Bin Meng wrote:
> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun  wrote:
>>
>> Use readx_poll_sleep_timeout() to poll the register status
>>
>> Signed-off-by: Chunfeng Yun 
>> ---
>> v3: no changes
>>
>> v2: fix typo of title suggested by Frank
>> ---
>>  drivers/usb/host/xhci.c | 25 +++--
>>  1 file changed, 11 insertions(+), 14 deletions(-)
>>
> 
> Reviewed-by: Bin Meng 

Thanks. So now I have half of a RESEND patchset with RB tags, and
another half of just RB tags without patchset. Can someone of you please
collect the tags and resend the patchset once more with the collected
tags, so it's all in one place ?

Thank you


Re: [PATCH 1/1] efi_loader: efi_var_mem_notify_exit_boot_services

2020-09-08 Thread Ilias Apalodimas
On Tue, Sep 08, 2020 at 10:58:14AM +, Heinrich Schuchardt wrote:
> efi_var_mem_notify_exit_boot_services() is invoked when ExitBootServices()
> is called by the UEFI payload.
> 
> efi_var_mem_notify_exit_boot_services() should not be defined as
> __efi_runtime as it is invoking EFI_ENTRY() and EFI_EXIT() which themselves
> are not __efi_runtime.
> 
> Fixes: f1f990a8c958 ("efi_loader: memory buffer for variables")
> Fixes: e01aed47d6a0 ("efi_loader: Enable run-time variable support for tee 
> based variables")
> Signed-off-by: Heinrich Schuchardt 
> ---
>  lib/efi_loader/efi_var_mem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
> index 8f4a5a5e47..1d2b44580f 100644
> --- a/lib/efi_loader/efi_var_mem.c
> +++ b/lib/efi_loader/efi_var_mem.c
> @@ -211,7 +211,7 @@ static void efi_var_mem_bs_del(void)
>   * @event:   callback event
>   * @context: callback context
>   */
> -static void EFIAPI __efi_runtime
> +static void EFIAPI
>  efi_var_mem_notify_exit_boot_services(struct efi_event *event, void *context)
>  {
>   EFI_ENTRY("%p, %p", event, context);
> --
> 2.20.1
> 
Acked-by: Ilias Apalodimas 


Changing U-boot relocation addres to SRAM (instead of DRAM)

2020-09-08 Thread Yusuf Altıparmak
Hello,

I want to modify U-boot to relocate itself to on-board SRAM of chip (T1042
demo board) instead of DRAM.

At first, there is call list in u-boot/common/board_f.c and it has a line
to call dram_init. I will change this line with my custom sram_init
function.

[image: Capture.PNG]

The function board_f() is being called in start.S which is located at
u-boot/arch/powerpc/cpu/mpc85xx/

start.S file also includes a region to relocate itself by changing Link
Register. Code region is:

*/**
** void relocate_code(addr_sp, gd, addr_moni)*
***
** This "function" does not return, instead it continues in RAM*
** after relocating the monitor code.*
***
** r3 = dest*
** r4 = src*
** r5 = length in bytes*
** r6 = cachelinesize*
**/*
*.globl relocate_code*
*relocate_code:*
*mr r1,r3 /* Set new stack pointer */*
*mr r9,r4 /* Save copy of Init Data pointer */*
*mr r10,r5 /* Save copy of Destination Address */*

*GET_GOT*
*#ifndef CONFIG_SPL_SKIP_RELOCATE*
*mr r3,r5 /* Destination Address */*
*lis r4,CONFIG_SYS_MONITOR_BASE@h /* Source Address */*
*ori r4,r4,CONFIG_SYS_MONITOR_BASE@l*
*lwz r5,GOT(__init_end)*
*sub r5,r5,r4*
*li r6,CONFIG_SYS_CACHELINE_SIZE /* Cache Line Size */*


My question is, in above code, how relocate_code() function is calculating
the jump address of Link Register (CONFIG_SYS_MONITOR_BASE@h).

What should be done to change it to point SRAM of t1042 after we initialize
it in board_init_f() function. ?

Thanks.


[PATCH 1/1] efi_loader: efi_var_mem_notify_exit_boot_services

2020-09-08 Thread Heinrich Schuchardt
efi_var_mem_notify_exit_boot_services() is invoked when ExitBootServices()
is called by the UEFI payload.

efi_var_mem_notify_exit_boot_services() should not be defined as
__efi_runtime as it is invoking EFI_ENTRY() and EFI_EXIT() which themselves
are not __efi_runtime.

Fixes: f1f990a8c958 ("efi_loader: memory buffer for variables")
Fixes: e01aed47d6a0 ("efi_loader: Enable run-time variable support for tee 
based variables")
Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/efi_var_mem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
index 8f4a5a5e47..1d2b44580f 100644
--- a/lib/efi_loader/efi_var_mem.c
+++ b/lib/efi_loader/efi_var_mem.c
@@ -211,7 +211,7 @@ static void efi_var_mem_bs_del(void)
  * @event: callback event
  * @context:   callback context
  */
-static void EFIAPI __efi_runtime
+static void EFIAPI
 efi_var_mem_notify_exit_boot_services(struct efi_event *event, void *context)
 {
EFI_ENTRY("%p, %p", event, context);
--
2.20.1



[PATCH 1/1] lib: rsa: fix data abort in br_i32_decode()

2020-09-08 Thread Heinrich Schuchardt
After removing leading zeros the RSA modulus may be unaligned. On
architectures like ARM 32bit unaligned access may lead to a data abort,
e.g. when executing 'ut lib lib_asn1_pkcs7'.

Use memcpy() to transfer from unaligned to aligned memory.

Signed-off-by: Heinrich Schuchardt 
---
 lib/rsa/rsa-keyprop.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/lib/rsa/rsa-keyprop.c b/lib/rsa/rsa-keyprop.c
index 1e83eedc82..6153cb00b3 100644
--- a/lib/rsa/rsa-keyprop.c
+++ b/lib/rsa/rsa-keyprop.c
@@ -17,23 +17,29 @@
 #include 

 /**
- * br_dec16be() - Convert 16-bit big-endian integer to native
- * @src:   Pointer to data
- * Return: Native-endian integer
+ * br_dec16be() - convert unaligned 16-bit big-endian integer to native
+ * @src:   unaligned pointer to data
+ * Return: native-endian 16-bit integer
  */
 static unsigned br_dec16be(const void *src)
 {
-   return be16_to_cpup(src);
+   u16 buf;
+
+   memcpy(, src, sizeof(buf));
+   return be16_to_cpu(buf);
 }

 /**
- * br_dec32be() - Convert 32-bit big-endian integer to native
- * @src:   Pointer to data
- * Return: Native-endian integer
+ * br_dec32be() - convert unaligned 32-bit big-endian integer to native
+ * @src:   unaligned pointer to data
+ * Return: native-endian 32-bit integer
  */
 static uint32_t br_dec32be(const void *src)
 {
-   return be32_to_cpup(src);
+   u32 buf;
+
+   memcpy(, src, sizeof(buf));
+   return be32_to_cpu(buf);
 }

 /**
--
2.20.1



[PATCH] include: dt-bindings: Add MSCC header

2020-09-08 Thread Michal Simek
From: Harini Katakam 

Add MSCC header with delay definitions for VSC8531 and associated
family devices.

Signed-off-by: Harini Katakam 
Signed-off-by: Michal Simek 
---

Copy from Linux but with fixed intendation and SPDX header.
---
 include/dt-bindings/net/mscc-phy-vsc8531.h | 40 ++
 1 file changed, 40 insertions(+)
 create mode 100644 include/dt-bindings/net/mscc-phy-vsc8531.h

diff --git a/include/dt-bindings/net/mscc-phy-vsc8531.h 
b/include/dt-bindings/net/mscc-phy-vsc8531.h
new file mode 100644
index ..61f5287d7523
--- /dev/null
+++ b/include/dt-bindings/net/mscc-phy-vsc8531.h
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */
+/*
+ * Device Tree constants for Microsemi VSC8531 PHY
+ *
+ * Author: Nagaraju Lakkaraju
+ *
+ * Copyright (c) 2017 Microsemi Corporation
+ */
+
+#ifndef _DT_BINDINGS_MSCC_VSC8531_H
+#define _DT_BINDINGS_MSCC_VSC8531_H
+
+/* PHY LED Modes */
+#define VSC8531_LINK_ACTIVITY  0
+#define VSC8531_LINK_1000_ACTIVITY 1
+#define VSC8531_LINK_100_ACTIVITY  2
+#define VSC8531_LINK_10_ACTIVITY   3
+#define VSC8531_LINK_100_1000_ACTIVITY 4
+#define VSC8531_LINK_10_1000_ACTIVITY  5
+#define VSC8531_LINK_10_100_ACTIVITY   6
+#define VSC8584_LINK_100FX_1000X_ACTIVITY  7
+#define VSC8531_DUPLEX_COLLISION   8
+#define VSC8531_COLLISION  9
+#define VSC8531_ACTIVITY   10
+#define VSC8584_100FX_1000X_ACTIVITY   11
+#define VSC8531_AUTONEG_FAULT  12
+#define VSC8531_SERIAL_MODE13
+#define VSC8531_FORCE_LED_OFF  14
+#define VSC8531_FORCE_LED_ON   15
+
+#define VSC8531_RGMII_CLK_DELAY_0_2_NS 0
+#define VSC8531_RGMII_CLK_DELAY_0_8_NS 1
+#define VSC8531_RGMII_CLK_DELAY_1_1_NS 2
+#define VSC8531_RGMII_CLK_DELAY_1_7_NS 3
+#define VSC8531_RGMII_CLK_DELAY_2_0_NS 4
+#define VSC8531_RGMII_CLK_DELAY_2_3_NS 5
+#define VSC8531_RGMII_CLK_DELAY_2_6_NS 6
+#define VSC8531_RGMII_CLK_DELAY_3_4_NS 7
+
+#endif
-- 
2.28.0



Re: [PATCH] defconfig: espressobin: enable NET_RANDOM_ETHADDR

2020-09-08 Thread Andre Heider

On 08/09/2020 09:42, Pali Rohár wrote:

On Tuesday 08 September 2020 08:35:00 Andre Heider wrote:

The hardware does not provide a MAC address. Enable this so that
network access works with just the default environment.


Well, this is not fully truth as MAC address is stored in SPI, just in
non-standard format, in U-Boot env stored in env partition and it is
hard to use outside of U-Boot, plus easy to erase / overwrite / lost.


True, but updating the bootloader usually implies wiping the env, so 
it's very easy to lose it.



I'm not a big fan of this change. This looks like a workaround / hack
for boards where MAC address was erased (e.g. by broken U-Boot distro
scripts) or for early boards where MAC address was not written at all
(as I was told).

And on these boards this patch would cause that U-Boot would see on
every boot different MAC address. This would cause another mess in
network for U-Boot netboot as DHCP/TFTP server would see for one board
every time different MAC address.

Is not really better to instruct user how to fix board where e.g. broken
distro scripts erased MAC address? We have already paragraph in
README.marvell about it.

Also this change affects "default" defconfig value. And based on above
arguments I do not think that this change should be enabled by default.

I understand that for some situations it may be useful (e.g. mass board
reparation process via netboot), but as this is config option, users in
such situation can enable this option manually.

I think that for default behavior is not provide network access in
U-Boot if for some reasons factory permanent MAC address was removed.
User can easier and faster detect this issue and fix it.


It can be argued both ways I guess. If this option wouldn't make sense 
it wouldn't exist.


Out of the box working network access is probably what most users care 
about.


Changing this default means that you need to build the whole firmware 
yourself, and let's face it: building it for this board is a total 
clusterfuck. Most users will just download a binary. I don't care too 
much for this patch, but I would consider what fits most users.


Right now most users will probably run the downstream binaries provided 
by armbian, and as you know, that even has single hardcoded MAC, used 
for all boards. So this would already be an improvement ;)


Thanks,
Andre




Signed-off-by: Andre Heider 
---
  configs/mvebu_espressobin-88f3720_defconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/configs/mvebu_espressobin-88f3720_defconfig 
b/configs/mvebu_espressobin-88f3720_defconfig
index 7aabbba59f..5e9fcd1f26 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -84,3 +84,4 @@ CONFIG_USB_ETHER_RTL8152=y
  CONFIG_USB_ETHER_SMSC95XX=y
  CONFIG_SHA1=y
  CONFIG_SHA256=y
+CONFIG_NET_RANDOM_ETHADDR=y
--
2.28.0





[PATCH v4 6/9] usb: xhci: convert to TRB_LEN() and TRB_INTR_TARGET()

2020-09-08 Thread Chunfeng Yun
For normal TRB fields:
use TRB_LEN(x) instead of ((x) & TRB_LEN_MASK);
and use TRB_INTR_TARGET(x) instead of
(((x) & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT)

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
v4:  keep code alignment and add reviewed-by Bin

v3: merge patch [v2 6/11] and [v2 7/11] into one, both for normal TRB fileds

v2: no changes
---
 drivers/usb/host/xhci-ring.c | 16 +++-
 include/usb/xhci.h   |  3 ---
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 13c98fb..9ef72ef 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -688,10 +688,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
  length, maxpacketsize,
  more_trbs_coming);
 
-   length_field = ((trb_buff_len & TRB_LEN_MASK) |
+   length_field = (TRB_LEN(trb_buff_len) |
TRB_TD_SIZE(remainder) |
-   ((0 & TRB_INTR_TARGET_MASK) <<
-   TRB_INTR_TARGET_SHIFT));
+   TRB_INTR_TARGET(0));
 
trb_fields[0] = lower_32_bits(addr);
trb_fields[1] = upper_32_bits(addr);
@@ -849,8 +848,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
trb_fields[1] = le16_to_cpu(req->index) |
le16_to_cpu(req->length) << 16;
/* TRB_LEN | (TRB_INTR_TARGET) */
-   trb_fields[2] = (8 | ((0 & TRB_INTR_TARGET_MASK) <<
-   TRB_INTR_TARGET_SHIFT));
+   trb_fields[2] = (TRB_LEN(8) | TRB_INTR_TARGET(0));
/* Immediate data in pointer */
trb_fields[3] = field;
queue_trb(ctrl, ep_ring, true, trb_fields);
@@ -866,11 +864,11 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
remainder = xhci_td_remainder(ctrl, 0, length, length,
  usb_maxpacket(udev, pipe), true);
-   length_field = (length & TRB_LEN_MASK) | TRB_TD_SIZE(remainder) |
-   ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
+   length_field = TRB_LEN(length) | TRB_TD_SIZE(remainder) |
+  TRB_INTR_TARGET(0);
debug("length_field = %d, length = %d,"
"xhci_td_remainder(length) = %d , TRB_INTR_TARGET(0) = %d\n",
-   length_field, (length & TRB_LEN_MASK),
+   length_field, TRB_LEN(length),
TRB_TD_SIZE(remainder), 0);
 
if (length > 0) {
@@ -901,7 +899,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
 
trb_fields[0] = 0;
trb_fields[1] = 0;
-   trb_fields[2] = ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
+   trb_fields[2] = TRB_INTR_TARGET(0);
/* Event on completion */
trb_fields[3] = field | TRB_IOC |
TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state;
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index bdba51d..35c6604 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -847,12 +847,9 @@ struct xhci_event_cmd {
 /* Normal TRB fields */
 /* transfer_len bitmasks - bits 0:16 */
 #defineTRB_LEN(p)  ((p) & 0x1)
-#defineTRB_LEN_MASK(0x1)
 /* TD Size, packets remaining in this TD, bits 21:17 (5 bits, so max 31) */
 #define TRB_TD_SIZE(p)  (min((p), (u32)31) << 17)
 /* Interrupter Target - which MSI-X vector to target the completion event at */
-#defineTRB_INTR_TARGET_SHIFT   (22)
-#defineTRB_INTR_TARGET_MASK(0x3ff)
 #define TRB_INTR_TARGET(p) (((p) & 0x3ff) << 22)
 #define GET_INTR_TARGET(p) (((p) >> 22) & 0x3ff)
 #define TRB_TBC(p) (((p) & 0x3) << 7)
-- 
1.9.1


[PATCH v4 5/9] usb: xhci: convert to TRB_TYPE()

2020-09-08 Thread Chunfeng Yun
Use TRB_TYPE(p) instead of ((p) << TRB_TYPE_SHIFT)

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
v4: no changes

v3: add reviewed-by Bin

v2: no changes
---
 drivers/usb/host/xhci-mem.c  |  3 +--
 drivers/usb/host/xhci-ring.c | 11 +--
 include/usb/xhci.h   |  1 -
 3 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 1da0524..d627aa5 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -236,8 +236,7 @@ static void xhci_link_segments(struct xhci_segment *prev,
 */
val = le32_to_cpu(prev->trbs[TRBS_PER_SEGMENT-1].link.control);
val &= ~TRB_TYPE_BITMASK;
-   val |= (TRB_LINK << TRB_TYPE_SHIFT);
-
+   val |= TRB_TYPE(TRB_LINK);
prev->trbs[TRBS_PER_SEGMENT-1].link.control = cpu_to_le32(val);
}
 }
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 3f915ae..13c98fb 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -696,7 +696,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
trb_fields[0] = lower_32_bits(addr);
trb_fields[1] = upper_32_bits(addr);
trb_fields[2] = length_field;
-   trb_fields[3] = field | (TRB_NORMAL << TRB_TYPE_SHIFT);
+   trb_fields[3] = field | TRB_TYPE(TRB_NORMAL);
 
queue_trb(ctrl, ring, (num_trbs > 1), trb_fields);
 
@@ -823,7 +823,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
/* Queue setup TRB - see section 6.4.1.2.1 */
/* FIXME better way to translate setup_packet into two u32 fields? */
field = 0;
-   field |= TRB_IDT | (TRB_SETUP << TRB_TYPE_SHIFT);
+   field |= TRB_IDT | TRB_TYPE(TRB_SETUP);
if (start_cycle == 0)
field |= 0x1;
 
@@ -860,9 +860,9 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
/* If there's data, queue data TRBs */
/* Only set interrupt on short packet for IN endpoints */
if (usb_pipein(pipe))
-   field = TRB_ISP | (TRB_DATA << TRB_TYPE_SHIFT);
+   field = TRB_ISP | TRB_TYPE(TRB_DATA);
else
-   field = (TRB_DATA << TRB_TYPE_SHIFT);
+   field = TRB_TYPE(TRB_DATA);
 
remainder = xhci_td_remainder(ctrl, 0, length, length,
  usb_maxpacket(udev, pipe), true);
@@ -904,8 +904,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
trb_fields[2] = ((0 & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT);
/* Event on completion */
trb_fields[3] = field | TRB_IOC |
-   (TRB_STATUS << TRB_TYPE_SHIFT) |
-   ep_ring->cycle_state;
+   TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state;
 
queue_trb(ctrl, ep_ring, false, trb_fields);
 
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index cf4c020..bdba51d 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -903,7 +903,6 @@ union xhci_trb {
 /* TRB bit mask */
 #defineTRB_TYPE_BITMASK(0xfc00)
 #define TRB_TYPE(p)((p) << 10)
-#define TRB_TYPE_SHIFT (10)
 #define TRB_FIELD_TO_TYPE(p)   (((p) & TRB_TYPE_BITMASK) >> 10)
 
 /* TRB type IDs */
-- 
1.9.1


[PATCH v4 9/9] usb: xhci: convert to readx_poll_sleep_timeout()

2020-09-08 Thread Chunfeng Yun
Use readx_poll_sleep_timeout() to poll the register status

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
v4: add reviewed-by Bin

v3: no changes

v2: fix typo of title suggested by Frank
---
 drivers/usb/host/xhci.c | 25 +++--
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index fe30101..3547a9b 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -33,6 +33,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #ifndef CONFIG_USB_MAX_CONTROLLER_COUNT
@@ -143,23 +144,19 @@ struct xhci_ctrl *xhci_get_ctrl(struct usb_device *udev)
  * @param usec time to wait till
  * @return 0 if handshake is success else < 0 on failure
  */
-static int handshake(uint32_t volatile *ptr, uint32_t mask,
-   uint32_t done, int usec)
+static int
+handshake(uint32_t volatile *ptr, uint32_t mask, uint32_t done, int usec)
 {
uint32_t result;
+   int ret;
+
+   ret = readx_poll_sleep_timeout(xhci_readl, ptr, result,
+(result & mask) == done || result == U32_MAX,
+1, usec);
+   if (result == U32_MAX)  /* card removed */
+   return -ENODEV;
 
-   do {
-   result = xhci_readl(ptr);
-   if (result == ~(uint32_t)0)
-   return -ENODEV;
-   result &= mask;
-   if (result == done)
-   return 0;
-   usec--;
-   udelay(1);
-   } while (usec > 0);
-
-   return -ETIMEDOUT;
+   return ret;
 }
 
 /**
-- 
1.9.1


[PATCH v4 7/9] usb: xhci: convert to TRB_TX_TYPE()

2020-09-08 Thread Chunfeng Yun
Use TRB_TX_TYPE() instead of (TRB_DATA_OUT/IN << TRB_TX_TYPE_SHIFT)

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
v4: add reviewed-by Bin

v2~v3: no changes
---
 drivers/usb/host/xhci-ring.c | 4 ++--
 include/usb/xhci.h   | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 9ef72ef..b118207 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -830,9 +830,9 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
if (ctrl->hci_version >= 0x100 || ctrl->quirks & XHCI_MTK_HOST) {
if (length > 0) {
if (req->requesttype & USB_DIR_IN)
-   field |= (TRB_DATA_IN << TRB_TX_TYPE_SHIFT);
+   field |= TRB_TX_TYPE(TRB_DATA_IN);
else
-   field |= (TRB_DATA_OUT << TRB_TX_TYPE_SHIFT);
+   field |= TRB_TX_TYPE(TRB_DATA_OUT);
}
}
 
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 35c6604..07b1aeb 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -879,7 +879,6 @@ struct xhci_event_cmd {
 /* Control transfer TRB specific fields */
 #define TRB_DIR_IN (1<<16)
 #defineTRB_TX_TYPE(p)  ((p) << 16)
-#defineTRB_TX_TYPE_SHIFT   (16)
 #defineTRB_DATA_OUT2
 #defineTRB_DATA_IN 3
 
-- 
1.9.1


[PATCH v4 3/9] usb: xhci: add quirks flag to support MediaTek xHCI 0.96

2020-09-08 Thread Chunfeng Yun
There some vendor quirks for MTK xHCI 0.96 host controller:
1. It defines some extra SW scheduling parameters for HW
   to minimize the scheduling effort for synchronous and
   interrupt endpoints. The parameters are put into reserved
   DWs of slot context and endpoint context.
2. Its TDS in  Normal TRB defines a number of packets that
   remains to be transferred for a TD after processing all
   Max packets in all previous TRBs.

Signed-off-by: Chunfeng Yun 
Tested-by: Frank Wunderlich 
Reviewed-by: Bin Meng 
---
v4: no changes

v3: fix typo, and add reviewed-by Bin

v2: add Tested-by Frank
---
 drivers/usb/host/xhci-mtk.c  | 1 +
 drivers/usb/host/xhci-ring.c | 9 +++--
 drivers/usb/host/xhci.c  | 2 +-
 include/usb/xhci.h   | 2 ++
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 8ff7185..f3f181d 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -258,6 +258,7 @@ static int xhci_mtk_probe(struct udevice *dev)
if (ret)
goto ssusb_init_err;
 
+   mtk->ctrl.quirks = XHCI_MTK_HOST;
hcor = (struct xhci_hcor *)((uintptr_t)mtk->hcd +
HC_LENGTH(xhci_readl(>hcd->cr_capbase)));
 
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 603e0e5..3f915ae 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -332,7 +332,8 @@ static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int 
transferred,
 {
u32 total_packet_count;
 
-   if (ctrl->hci_version < 0x100)
+   /* MTK xHCI 0.96 contains some features from 1.0 */
+   if (ctrl->hci_version < 0x100 && !(ctrl->quirks & XHCI_MTK_HOST))
return ((td_total_len - transferred) >> 10);
 
/* One TRB with a zero-length data packet. */
@@ -340,6 +341,10 @@ static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int 
transferred,
trb_buff_len == td_total_len)
return 0;
 
+   /* for MTK xHCI 0.96, TD size include this TRB, but not in 1.x */
+   if ((ctrl->quirks & XHCI_MTK_HOST) && (ctrl->hci_version < 0x100))
+   trb_buff_len = 0;
+
total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
 
/* Queueing functions don't count the current TRB into transferred */
@@ -823,7 +828,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
field |= 0x1;
 
/* xHCI 1.0 6.4.1.2.1: Transfer Type field */
-   if (ctrl->hci_version >= 0x100) {
+   if (ctrl->hci_version >= 0x100 || ctrl->quirks & XHCI_MTK_HOST) {
if (length > 0) {
if (req->requesttype & USB_DIR_IN)
field |= (TRB_DATA_IN << TRB_TX_TYPE_SHIFT);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 4be1411..51edeb2 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -650,7 +650,7 @@ static int xhci_set_configuration(struct usb_device *udev)
 * are put into reserved DWs in Slot and Endpoint Contexts
 * for synchronous endpoints.
 */
-   if (IS_ENABLED(CONFIG_USB_XHCI_MTK)) {
+   if (ctrl->quirks & XHCI_MTK_HOST) {
ep_ctx[ep_index]->reserved[0] =
cpu_to_le32(EP_BPKTS(1) | EP_BBM(1));
}
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 15926eb..3de46cd 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -1230,6 +1230,8 @@ struct xhci_ctrl {
struct xhci_virt_device *devs[MAX_HC_SLOTS];
int rootdev;
u16 hci_version;
+   u32 quirks;
+#define XHCI_MTK_HOST  BIT(0)
 };
 
 unsigned long trb_addr(struct xhci_segment *seg, union xhci_trb *trb);
-- 
1.9.1


[PATCH v4 8/9] usb: xhci: use macros with parameter to fill ep_info2

2020-09-08 Thread Chunfeng Yun
Use macros with parameter to fill ep_info2, then some macros
for MASK and SHIFT can be removed

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
v4: add reviewed-by Bin

v3: merge patch [v2 9/11] and [v2 10/11] into one, both for ep_info2

v2: no changes
---
 drivers/usb/host/xhci-mem.c | 15 +--
 drivers/usb/host/xhci.c |  6 ++
 include/usb/xhci.h  |  6 --
 3 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index d627aa5..0b49614 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -825,25 +825,22 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl 
*ctrl,
 
/* Step 4 - ring already allocated */
/* Step 5 */
-   ep0_ctx->ep_info2 = cpu_to_le32(CTRL_EP << EP_TYPE_SHIFT);
+   ep0_ctx->ep_info2 = cpu_to_le32(EP_TYPE(CTRL_EP));
debug("SPEED = %d\n", speed);
 
switch (speed) {
case USB_SPEED_SUPER:
-   ep0_ctx->ep_info2 |= cpu_to_le32(((512 & MAX_PACKET_MASK) <<
-   MAX_PACKET_SHIFT));
+   ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(512));
debug("Setting Packet size = 512bytes\n");
break;
case USB_SPEED_HIGH:
/* USB core guesses at a 64-byte max packet first for FS devices */
case USB_SPEED_FULL:
-   ep0_ctx->ep_info2 |= cpu_to_le32(((64 & MAX_PACKET_MASK) <<
-   MAX_PACKET_SHIFT));
+   ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(64));
debug("Setting Packet size = 64bytes\n");
break;
case USB_SPEED_LOW:
-   ep0_ctx->ep_info2 |= cpu_to_le32(((8 & MAX_PACKET_MASK) <<
-   MAX_PACKET_SHIFT));
+   ep0_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(8));
debug("Setting Packet size = 8bytes\n");
break;
default:
@@ -852,9 +849,7 @@ void xhci_setup_addressable_virt_dev(struct xhci_ctrl *ctrl,
}
 
/* EP 0 can handle "burst" sizes of 1, so Max Burst Size field is 0 */
-   ep0_ctx->ep_info2 |=
-   cpu_to_le32(((0 & MAX_BURST_MASK) << MAX_BURST_SHIFT) |
-   ((3 & ERROR_COUNT_MASK) << ERROR_COUNT_SHIFT));
+   ep0_ctx->ep_info2 |= cpu_to_le32(MAX_BURST(0) | ERROR_COUNT(3));
 
trb_64 = virt_to_phys(virt_dev->eps[0].ring->first_seg->trbs);
ep0_ctx->deq = cpu_to_le64(trb_64 | virt_dev->eps[0].ring->cycle_state);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5f3a0fb..fe30101 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -618,8 +618,7 @@ static int xhci_set_configuration(struct usb_device *udev)
cpu_to_le32(EP_MAX_ESIT_PAYLOAD_HI(max_esit_payload) |
EP_INTERVAL(interval) | EP_MULT(mult));
 
-   ep_ctx[ep_index]->ep_info2 =
-   cpu_to_le32(ep_type << EP_TYPE_SHIFT);
+   ep_ctx[ep_index]->ep_info2 = cpu_to_le32(EP_TYPE(ep_type));
ep_ctx[ep_index]->ep_info2 |=
cpu_to_le32(MAX_PACKET
(get_unaligned(_desc->wMaxPacketSize)));
@@ -832,8 +831,7 @@ int xhci_check_maxpacket(struct usb_device *udev)
ctrl->devs[slot_id]->out_ctx, ep_index);
in_ctx = ctrl->devs[slot_id]->in_ctx;
ep_ctx = xhci_get_ep_ctx(ctrl, in_ctx, ep_index);
-   ep_ctx->ep_info2 &= cpu_to_le32(~((0x & MAX_PACKET_MASK)
-   << MAX_PACKET_SHIFT));
+   ep_ctx->ep_info2 &= cpu_to_le32(~MAX_PACKET(MAX_PACKET_MASK));
ep_ctx->ep_info2 |= cpu_to_le32(MAX_PACKET(max_packet_size));
 
/*
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 07b1aeb..e1d3823 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -632,11 +632,8 @@ struct xhci_ep_ctx {
  */
 #defineFORCE_EVENT (0x1)
 #define ERROR_COUNT(p) (((p) & 0x3) << 1)
-#define ERROR_COUNT_SHIFT  (1)
-#define ERROR_COUNT_MASK   (0x3)
 #define CTX_TO_EP_TYPE(p)  (((p) >> 3) & 0x7)
 #define EP_TYPE(p) ((p) << 3)
-#define EP_TYPE_SHIFT  (3)
 #define ISOC_OUT_EP1
 #define BULK_OUT_EP2
 #define INT_OUT_EP 3
@@ -647,13 +644,10 @@ struct xhci_ep_ctx {
 /* bit 6 reserved */
 /* bit 7 is Host Initiate Disable - for disabling stream selection */
 #define MAX_BURST(p)   (((p)&0xff) << 8)
-#define MAX_BURST_MASK (0xff)
-#define MAX_BURST_SHIFT(8)
 #define CTX_TO_MAX_BURST(p)(((p) >> 8) & 0xff)
 #define MAX_PACKET(p)  (((p)&0x) << 16)
 #define MAX_PACKET_MASK(0x)
 #define MAX_PACKET_DECODED(p)  (((p) >> 16) & 0x)
-#define MAX_PACKET_SHIFT

[PATCH v4 1/9] usb: xhci: add a member hci_version in xhci_ctrl struct

2020-09-08 Thread Chunfeng Yun
Add a member to save xHCI version, it's used some times.

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
v4: no changes

v3: add reviewed-by Bin

v2: no changes
---
 drivers/usb/host/xhci-ring.c | 4 ++--
 drivers/usb/host/xhci.c  | 1 +
 include/usb/xhci.h   | 1 +
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 092ed6e..79bfc34 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -682,7 +682,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
field |= TRB_ISP;
 
/* Set the TRB length, TD size, and interrupter fields. */
-   if (HC_VERSION(xhci_readl(>hccr->cr_capbase)) < 0x100)
+   if (ctrl->hci_version < 0x100)
remainder = xhci_td_remainder(length - running_total);
else
remainder = xhci_v1_0_td_remainder(running_total,
@@ -830,7 +830,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
pipe,
field |= 0x1;
 
/* xHCI 1.0 6.4.1.2.1: Transfer Type field */
-   if (HC_VERSION(xhci_readl(>hccr->cr_capbase)) >= 0x100) {
+   if (ctrl->hci_version >= 0x100) {
if (length > 0) {
if (req->requesttype & USB_DIR_IN)
field |= (TRB_DATA_IN << TRB_TX_TYPE_SHIFT);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 126dabc..4be1411 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1283,6 +1283,7 @@ static int xhci_lowlevel_init(struct xhci_ctrl *ctrl)
 
reg = HC_VERSION(xhci_readl(>cr_capbase));
printf("USB XHCI %x.%02x\n", reg >> 8, reg & 0xff);
+   ctrl->hci_version = reg;
 
return 0;
 }
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 7d34103..a3e5914 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -1227,6 +1227,7 @@ struct xhci_ctrl {
struct xhci_scratchpad *scratchpad;
struct xhci_virt_device *devs[MAX_HC_SLOTS];
int rootdev;
+   u16 hci_version;
 };
 
 unsigned long trb_addr(struct xhci_segment *seg, union xhci_trb *trb);
-- 
1.9.1


[PATCH v4 2/9] usb: xhci: create one unified function to calculate TRB TD remainder

2020-09-08 Thread Chunfeng Yun
xhci versions 1.0 and later report the untransferred data remaining in a
TD a bit differently than older hosts.

We used to have separate functions for these, and needed to check host
version before calling the right function.

Now Mediatek host has an additional quirk on how it uses the TD Size
field for remaining data. To prevent yet another function for calculating
remainder we instead want to make one quirk friendly unified function.

Porting from the Linux:
c840d6ce772d("xhci: create one unified function to calculate TRB TD remainder.")
124c39371114("xhci: use boolean to indicate last trb in td remainder 
calculation")

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
v4 changes:
1. remove the ending period in the commit title
2. use true instead of 1
3. add Reviewed-by Bin

v2~v3: no changes
---
 drivers/usb/host/xhci-ring.c | 105 +--
 include/usb/xhci.h   |   2 +
 2 files changed, 52 insertions(+), 55 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 79bfc34..603e0e5 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -298,55 +298,52 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, u8 *ptr, 
u32 slot_id,
xhci_writel(>dba->doorbell[0], DB_VALUE_HOST);
 }
 
-/**
- * The TD size is the number of bytes remaining in the TD (including this TRB),
- * right shifted by 10.
- * It must fit in bits 21:17, so it can't be bigger than 31.
+/*
+ * For xHCI 1.0 host controllers, TD size is the number of max packet sized
+ * packets remaining in the TD (*not* including this TRB).
  *
- * @param remainderremaining packets to be sent
- * @return remainder if remainder is less than max else max
- */
-static u32 xhci_td_remainder(unsigned int remainder)
-{
-   u32 max = (1 << (21 - 17 + 1)) - 1;
-
-   if ((remainder >> 10) >= max)
-   return max << 17;
-   else
-   return (remainder >> 10) << 17;
-}
-
-/**
- * Finds out the remanining packets to be sent
+ * Total TD packet count = total_packet_count =
+ * DIV_ROUND_UP(TD size in bytes / wMaxPacketSize)
+ *
+ * Packets transferred up to and including this TRB = packets_transferred =
+ * rounddown(total bytes transferred including this TRB / wMaxPacketSize)
+ *
+ * TD size = total_packet_count - packets_transferred
  *
- * @param running_totaltotal size sent so far
+ * For xHCI 0.96 and older, TD size field should be the remaining bytes
+ * including this TRB, right shifted by 10
+ *
+ * For all hosts it must fit in bits 21:17, so it can't be bigger than 31.
+ * This is taken care of in the TRB_TD_SIZE() macro
+ *
+ * The last TRB in a TD must have the TD size set to zero.
+ *
+ * @param ctrl host controller data structure
+ * @param transferred  total size sent so far
  * @param trb_buff_len length of the TRB Buffer
- * @param total_packet_count   total packet count
- * @param maxpacketsizemax packet size of current pipe
- * @param num_trbs_leftnumber of TRBs left to be processed
- * @return 0 if running_total or trb_buff_len is 0, else remainder
+ * @param td_total_len total packet count
+ * @param maxp max packet size of current pipe
+ * @param more_trbs_coming indicate last trb in TD
+ * @return remainder
  */
-static u32 xhci_v1_0_td_remainder(int running_total,
-   int trb_buff_len,
-   unsigned int total_packet_count,
-   int maxpacketsize,
-   unsigned int num_trbs_left)
+static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int transferred,
+int trb_buff_len, unsigned int td_total_len,
+int maxp, bool more_trbs_coming)
 {
-   int packets_transferred;
+   u32 total_packet_count;
+
+   if (ctrl->hci_version < 0x100)
+   return ((td_total_len - transferred) >> 10);
 
/* One TRB with a zero-length data packet. */
-   if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == 0))
+   if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) ||
+   trb_buff_len == td_total_len)
return 0;
 
-   /*
-* All the TRB queueing functions don't count the current TRB in
-* running_total.
-*/
-   packets_transferred = (running_total + trb_buff_len) / maxpacketsize;
+   total_packet_count = DIV_ROUND_UP(td_total_len, maxp);
 
-   if ((total_packet_count - packets_transferred) > 31)
-   return 31 << 17;
-   return (total_packet_count - packets_transferred) << 17;
+   /* Queueing functions don't count the current TRB into transferred */
+   return (total_packet_count - ((transferred + trb_buff_len) / maxp));
 }
 
 /**
@@ -572,7 +569,7 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned long 
pipe,
union xhci_trb *event;
 
int 

[PATCH v4 4/9] usb: xhci: convert to HCS_MAX_PORTS()

2020-09-08 Thread Chunfeng Yun
Use HCS_MAX_PORTS(p) instead of
((p & HCS_MAX_PORTS_MASK) >> HCS_MAX_PORTS_SHIFT)

Signed-off-by: Chunfeng Yun 
Reviewed-by: Bin Meng 
---
v4: no changes

v3: add reviewed-by Bin

v2: no changes
---
 drivers/usb/host/xhci.c | 3 +--
 include/usb/xhci.h  | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 51edeb2..5f3a0fb 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1257,8 +1257,7 @@ static int xhci_lowlevel_init(struct xhci_ctrl *ctrl)
return -ENOMEM;
 
reg = xhci_readl(>cr_hcsparams1);
-   descriptor.hub.bNbrPorts = ((reg & HCS_MAX_PORTS_MASK) >>
-   HCS_MAX_PORTS_SHIFT);
+   descriptor.hub.bNbrPorts = HCS_MAX_PORTS(reg);
printf("Register %x NbrPorts %d\n", reg, descriptor.hub.bNbrPorts);
 
/* Port Indicators */
diff --git a/include/usb/xhci.h b/include/usb/xhci.h
index 3de46cd..cf4c020 100644
--- a/include/usb/xhci.h
+++ b/include/usb/xhci.h
@@ -101,8 +101,6 @@ struct xhci_hccr {
 /* bits 8:18, Max Interrupters */
 #define HCS_MAX_INTRS(p)   (((p) >> 8) & 0x7ff)
 /* bits 24:31, Max Ports - max value is 0x7F = 127 ports */
-#define HCS_MAX_PORTS_SHIFT24
-#define HCS_MAX_PORTS_MASK (0xff << HCS_MAX_PORTS_SHIFT)
 #define HCS_MAX_PORTS(p)   (((p) >> 24) & 0xff)
 
 /* HCSPARAMS2 - hcs_params2 - bitmasks */
-- 
1.9.1


RE: [PATCH v3 1/7] riscv: Rework riscv timer driver to only support S-mode

2020-09-08 Thread Pragnesh Patel
Hi Sean,

>-Original Message-
>From: Sean Anderson 
>Sent: 01 September 2020 16:02
>To: u-boot@lists.denx.de
>Cc: Rick Chen ; Bin Meng ;
>Pragnesh Patel ; Sean Anderson
>; Bin Meng ; Anup Patel
>
>Subject: [PATCH v3 1/7] riscv: Rework riscv timer driver to only support S-mode
>
>[External Email] Do not click links or attachments unless you recognize the
>sender and know the content is safe
>
>The riscv-timer driver currently serves as a shim for several riscv timer 
>drivers.
>This is not too desirable because it bypasses the usual timer selection via the
>driver model. There is no easy way to specify an alternate timing driver, or 
>have
>the tick rate depend on the cpu's configured frequency. The timer drivers also 
>do
>not have device structs, and so have to rely on storing parameters in gd_t. 
>Lastly,
>there is no initialization call, so driver init is done in the same function 
>which
>reads the time. This can result in confusing error messages. To a user, it 
>looks like
>the driver failed when trying to read the time, whereas it may have failed 
>while
>initializing.
>
>This patch removes the shim functionality from the riscv-timer driver, and has 
>it
>instead implement the former rdtime.c timer driver. This is because existing u-
>boot users who pass in a device tree (e.g. qemu) do not create a timer device 
>for
>S-mode u-boot. The existing behavior of creating the riscv-timer device in the
>riscv cpu driver must be kept. The actual reading of the CSRs has been redone 
>in
>the style of Linux's get_cycles64.
>
>Signed-off-by: Sean Anderson 
>Reviewed-by: Bin Meng 
>---
>
>(no changes since v2)
>
>Changes in v2:
>- Remove RISCV_RDTIME KConfig option
>
> arch/riscv/Kconfig  |  8 
> arch/riscv/lib/Makefile |  1 -
> arch/riscv/lib/rdtime.c | 38 
> drivers/timer/Kconfig   |  6 +++---
> drivers/timer/riscv_timer.c | 39 +++--
> 5 files changed, 23 insertions(+), 69 deletions(-)  delete mode 100644
>arch/riscv/lib/rdtime.c
>
>diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 
>009a545fcf..21e6690f4d
>100644
>--- a/arch/riscv/Kconfig
>+++ b/arch/riscv/Kconfig
>@@ -185,14 +185,6 @@ config ANDES_PLMT
>  The Andes PLMT block holds memory-mapped mtime register
>  associated with timer tick.
>
>-config RISCV_RDTIME
>-   bool
>-   default y if RISCV_SMODE || SPL_RISCV_SMODE
>-   help
>- The provides the riscv_get_time() API that is implemented using the
>- standard rdtime instruction. This is the case for S-mode U-Boot, and
>- is useful for processors that support rdtime in M-mode too.
>-
> config SYS_MALLOC_F_LEN
>default 0x1000
>
>diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile index
>6c503ff2b2..10ac5b06d3 100644
>--- a/arch/riscv/lib/Makefile
>+++ b/arch/riscv/lib/Makefile
>@@ -15,7 +15,6 @@ obj-$(CONFIG_SIFIVE_CLINT) += sifive_clint.o
> obj-$(CONFIG_ANDES_PLIC) += andes_plic.o
> obj-$(CONFIG_ANDES_PLMT) += andes_plmt.o  else
>-obj-$(CONFIG_RISCV_RDTIME) += rdtime.o
> obj-$(CONFIG_SBI) += sbi.o
> obj-$(CONFIG_SBI_IPI) += sbi_ipi.o
> endif
>diff --git a/arch/riscv/lib/rdtime.c b/arch/riscv/lib/rdtime.c deleted file 
>mode
>100644 index e128d7fce6..00
>--- a/arch/riscv/lib/rdtime.c
>+++ /dev/null
>@@ -1,38 +0,0 @@
>-// SPDX-License-Identifier: GPL-2.0+
>-/*
>- * Copyright (C) 2018, Anup Patel 
>- * Copyright (C) 2018, Bin Meng 
>- *
>- * The riscv_get_time() API implementation that is using the
>- * standard rdtime instruction.
>- */
>-
>-#include 
>-
>-/* Implement the API required by RISC-V timer driver */ -int 
>riscv_get_time(u64
>*time) -{ -#ifdef CONFIG_64BIT
>-   u64 n;
>-
>-   __asm__ __volatile__ (
>-   "rdtime %0"
>-   : "=r" (n));
>-
>-   *time = n;
>-#else
>-   u32 lo, hi, tmp;
>-
>-   __asm__ __volatile__ (
>-   "1:\n"
>-   "rdtimeh %0\n"
>-   "rdtime %1\n"
>-   "rdtimeh %2\n"
>-   "bne %0, %2, 1b"
>-   : "=" (hi), "=" (lo), "=" (tmp));
>-
>-   *time = ((u64)hi << 32) | lo;
>-#endif
>-
>-   return 0;
>-}
>diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index
>637024445c..b85fa33e47 100644
>--- a/drivers/timer/Kconfig
>+++ b/drivers/timer/Kconfig
>@@ -144,10 +144,10 @@ config OMAP_TIMER
>
> config RISCV_TIMER
>bool "RISC-V timer support"
>-   depends on TIMER && RISCV
>+   depends on TIMER && RISCV_SMODE

What about SPL_RISCV_SMODE ?

>help
>- Select this to enable support for the timer as defined
>- by the RISC-V privileged architecture spec.
>+ Select this to enable support for a generic RISC-V S-Mode timer
>+ driver.
>
> config ROCKCHIP_TIMER
>bool "Rockchip timer support"
>diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c index
>9f9f070e0b..449fcfcfd5 100644
>--- 

Re: [PATCH] defconfig: espressobin: enable NET_RANDOM_ETHADDR

2020-09-08 Thread Pali Rohár
On Tuesday 08 September 2020 08:35:00 Andre Heider wrote:
> The hardware does not provide a MAC address. Enable this so that
> network access works with just the default environment.

Well, this is not fully truth as MAC address is stored in SPI, just in
non-standard format, in U-Boot env stored in env partition and it is
hard to use outside of U-Boot, plus easy to erase / overwrite / lost.

I'm not a big fan of this change. This looks like a workaround / hack
for boards where MAC address was erased (e.g. by broken U-Boot distro
scripts) or for early boards where MAC address was not written at all
(as I was told).

And on these boards this patch would cause that U-Boot would see on
every boot different MAC address. This would cause another mess in
network for U-Boot netboot as DHCP/TFTP server would see for one board
every time different MAC address.

Is not really better to instruct user how to fix board where e.g. broken
distro scripts erased MAC address? We have already paragraph in
README.marvell about it.

Also this change affects "default" defconfig value. And based on above
arguments I do not think that this change should be enabled by default.

I understand that for some situations it may be useful (e.g. mass board
reparation process via netboot), but as this is config option, users in
such situation can enable this option manually.

I think that for default behavior is not provide network access in
U-Boot if for some reasons factory permanent MAC address was removed.
User can easier and faster detect this issue and fix it.

> Signed-off-by: Andre Heider 
> ---
>  configs/mvebu_espressobin-88f3720_defconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/mvebu_espressobin-88f3720_defconfig 
> b/configs/mvebu_espressobin-88f3720_defconfig
> index 7aabbba59f..5e9fcd1f26 100644
> --- a/configs/mvebu_espressobin-88f3720_defconfig
> +++ b/configs/mvebu_espressobin-88f3720_defconfig
> @@ -84,3 +84,4 @@ CONFIG_USB_ETHER_RTL8152=y
>  CONFIG_USB_ETHER_SMSC95XX=y
>  CONFIG_SHA1=y
>  CONFIG_SHA256=y
> +CONFIG_NET_RANDOM_ETHADDR=y
> -- 
> 2.28.0
> 


Re: [PATCH v3 2/9] usb: xhci: create one unified function to calculate TRB TD remainder.

2020-09-08 Thread Chunfeng Yun
On Tue, 2020-09-08 at 13:41 +0800, Bin Meng wrote:
> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun  wrote:
> >
> 
> nits: please remove the ending period in the commit title
Ok, will fix it

> 
> > xhci versions 1.0 and later report the untransferred data remaining in a
> > TD a bit differently than older hosts.
> >
> > We used to have separate functions for these, and needed to check host
> > version before calling the right function.
> >
> > Now Mediatek host has an additional quirk on how it uses the TD Size
> > field for remaining data. To prevent yet another function for calculating
> > remainder we instead want to make one quirk friendly unified function.
> >
> > Porting from the Linux:
> > c840d6ce772d("xhci: create one unified function to calculate TRB TD 
> > remainder.")
> > 124c39371114("xhci: use boolean to indicate last trb in td remainder 
> > calculation")
> >
> > Signed-off-by: Chunfeng Yun 
> > ---
> > v2~v3: no changes
> > ---
> >  drivers/usb/host/xhci-ring.c | 105 
> > +--
> >  include/usb/xhci.h   |   2 +
> >  2 files changed, 52 insertions(+), 55 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > index 79bfc34..0f86b01 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -298,55 +298,52 @@ void xhci_queue_command(struct xhci_ctrl *ctrl, u8 
> > *ptr, u32 slot_id,
> > xhci_writel(>dba->doorbell[0], DB_VALUE_HOST);
> >  }
> >
> > -/**
> > - * The TD size is the number of bytes remaining in the TD (including this 
> > TRB),
> > - * right shifted by 10.
> > - * It must fit in bits 21:17, so it can't be bigger than 31.
> > +/*
> > + * For xHCI 1.0 host controllers, TD size is the number of max packet sized
> > + * packets remaining in the TD (*not* including this TRB).
> >   *
> > - * @param remainderremaining packets to be sent
> > - * @return remainder if remainder is less than max else max
> > - */
> > -static u32 xhci_td_remainder(unsigned int remainder)
> > -{
> > -   u32 max = (1 << (21 - 17 + 1)) - 1;
> > -
> > -   if ((remainder >> 10) >= max)
> > -   return max << 17;
> > -   else
> > -   return (remainder >> 10) << 17;
> > -}
> > -
> > -/**
> > - * Finds out the remanining packets to be sent
> > + * Total TD packet count = total_packet_count =
> > + * DIV_ROUND_UP(TD size in bytes / wMaxPacketSize)
> > + *
> > + * Packets transferred up to and including this TRB = packets_transferred =
> > + * rounddown(total bytes transferred including this TRB / 
> > wMaxPacketSize)
> > + *
> > + * TD size = total_packet_count - packets_transferred
> >   *
> > - * @param running_totaltotal size sent so far
> > + * For xHCI 0.96 and older, TD size field should be the remaining bytes
> > + * including this TRB, right shifted by 10
> > + *
> > + * For all hosts it must fit in bits 21:17, so it can't be bigger than 31.
> > + * This is taken care of in the TRB_TD_SIZE() macro
> > + *
> > + * The last TRB in a TD must have the TD size set to zero.
> > + *
> > + * @param ctrl host controller data structure
> > + * @param transferred  total size sent so far
> >   * @param trb_buff_len length of the TRB Buffer
> > - * @param total_packet_count   total packet count
> > - * @param maxpacketsizemax packet size of current pipe
> > - * @param num_trbs_leftnumber of TRBs left to be processed
> > - * @return 0 if running_total or trb_buff_len is 0, else remainder
> > + * @param td_total_len total packet count
> > + * @param maxp max packet size of current pipe
> > + * @param more_trbs_coming indicate last trb in TD
> > + * @return remainder
> >   */
> > -static u32 xhci_v1_0_td_remainder(int running_total,
> > -   int trb_buff_len,
> > -   unsigned int total_packet_count,
> > -   int maxpacketsize,
> > -   unsigned int num_trbs_left)
> > +static u32 xhci_td_remainder(struct xhci_ctrl *ctrl, int transferred,
> > +int trb_buff_len, unsigned int td_total_len,
> > +int maxp, bool more_trbs_coming)
> >  {
> > -   int packets_transferred;
> > +   u32 total_packet_count;
> > +
> > +   if (ctrl->hci_version < 0x100)
> > +   return ((td_total_len - transferred) >> 10);
> >
> > /* One TRB with a zero-length data packet. */
> > -   if (num_trbs_left == 0 || (running_total == 0 && trb_buff_len == 0))
> > +   if (!more_trbs_coming || (transferred == 0 && trb_buff_len == 0) ||
> > +   trb_buff_len == td_total_len)
> > return 0;
> >
> > -   /*
> > -* All the TRB queueing functions don't count the current TRB in
> > -* running_total.
> > -*/
> > -   packets_transferred = (running_total + trb_buff_len) / 
> > maxpacketsize;
> > +   

Re: [PATCH v3 6/9] usb: xhci: convert to TRB_LEN() and TRB_INTR_TARGET()

2020-09-08 Thread Chunfeng Yun
On Tue, 2020-09-08 at 09:30 +0800, Bin Meng wrote:
> On Mon, Sep 7, 2020 at 3:14 PM Chunfeng Yun  wrote:
> >
> > For normal TRB fields:
> > use TRB_LEN(x) instead of ((x) & TRB_LEN_MASK);
> > and use TRB_INTR_TARGET(x) instead of
> > (((x) & TRB_INTR_TARGET_MASK) << TRB_INTR_TARGET_SHIFT)
> >
> > Signed-off-by: Chunfeng Yun 
> > ---
> > v3: merge patch [v2 6/11] and [v2 7/11] into one, both for normal TRB fileds
> >
> > v2: no changes
> > ---
> >  drivers/usb/host/xhci-ring.c | 16 +++-
> >  include/usb/xhci.h   |  3 ---
> >  2 files changed, 7 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > index 87891fd..99c84f9 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -688,10 +688,9 @@ int xhci_bulk_tx(struct usb_device *udev, unsigned 
> > long pipe,
> >   length, maxpacketsize,
> >   more_trbs_coming);
> >
> > -   length_field = ((trb_buff_len & TRB_LEN_MASK) |
> > +   length_field = (TRB_LEN(trb_buff_len) |
> > TRB_TD_SIZE(remainder) |
> > -   ((0 & TRB_INTR_TARGET_MASK) <<
> > -   TRB_INTR_TARGET_SHIFT));
> > +   TRB_INTR_TARGET(0));
> 
> nits: should be aligned to TRB_LEN(length)
Ok, will check it again

> 
> >
> > trb_fields[0] = lower_32_bits(addr);
> > trb_fields[1] = upper_32_bits(addr);
> > @@ -849,8 +848,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
> > pipe,
> > trb_fields[1] = le16_to_cpu(req->index) |
> > le16_to_cpu(req->length) << 16;
> > /* TRB_LEN | (TRB_INTR_TARGET) */
> > -   trb_fields[2] = (8 | ((0 & TRB_INTR_TARGET_MASK) <<
> > -   TRB_INTR_TARGET_SHIFT));
> > +   trb_fields[2] = (TRB_LEN(8) | TRB_INTR_TARGET(0));
> > /* Immediate data in pointer */
> > trb_fields[3] = field;
> > queue_trb(ctrl, ep_ring, true, trb_fields);
> > @@ -866,11 +864,11 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned 
> > long pipe,
> >
> > remainder = xhci_td_remainder(ctrl, 0, length, length,
> >   usb_maxpacket(udev, pipe), 1);
> > -   length_field = (length & TRB_LEN_MASK) | TRB_TD_SIZE(remainder) |
> > -   ((0 & TRB_INTR_TARGET_MASK) << 
> > TRB_INTR_TARGET_SHIFT);
> > +   length_field = TRB_LEN(length) | TRB_TD_SIZE(remainder) |
> > +   TRB_INTR_TARGET(0);
> > debug("length_field = %d, length = %d,"
> > "xhci_td_remainder(length) = %d , TRB_INTR_TARGET(0) = 
> > %d\n",
> > -   length_field, (length & TRB_LEN_MASK),
> > +   length_field, TRB_LEN(length),
> > TRB_TD_SIZE(remainder), 0);
> >
> > if (length > 0) {
> > @@ -901,7 +899,7 @@ int xhci_ctrl_tx(struct usb_device *udev, unsigned long 
> > pipe,
> >
> > trb_fields[0] = 0;
> > trb_fields[1] = 0;
> > -   trb_fields[2] = ((0 & TRB_INTR_TARGET_MASK) << 
> > TRB_INTR_TARGET_SHIFT);
> > +   trb_fields[2] = TRB_INTR_TARGET(0);
> > /* Event on completion */
> > trb_fields[3] = field | TRB_IOC |
> > TRB_TYPE(TRB_STATUS) | ep_ring->cycle_state;
> > diff --git a/include/usb/xhci.h b/include/usb/xhci.h
> > index bdba51d..35c6604 100644
> > --- a/include/usb/xhci.h
> > +++ b/include/usb/xhci.h
> > @@ -847,12 +847,9 @@ struct xhci_event_cmd {
> >  /* Normal TRB fields */
> >  /* transfer_len bitmasks - bits 0:16 */
> >  #defineTRB_LEN(p)  ((p) & 0x1)
> > -#defineTRB_LEN_MASK(0x1)
> >  /* TD Size, packets remaining in this TD, bits 21:17 (5 bits, so max 31) */
> >  #define TRB_TD_SIZE(p)  (min((p), (u32)31) << 17)
> >  /* Interrupter Target - which MSI-X vector to target the completion event 
> > at */
> > -#defineTRB_INTR_TARGET_SHIFT   (22)
> > -#defineTRB_INTR_TARGET_MASK(0x3ff)
> >  #define TRB_INTR_TARGET(p) (((p) & 0x3ff) << 22)
> >  #define GET_INTR_TARGET(p) (((p) >> 22) & 0x3ff)
> >  #define TRB_TBC(p) (((p) & 0x3) << 7)
> > --
> 
> Reviewed-by: Bin Meng 
Thanks




[PATCH 2/2] cosmetic: reset: ast2500: Rename driver and configs

2020-09-08 Thread Chia-Wei, Wang
1. Rename AST2500 reset driver from ast2500-reset.c
   to reset-ast2500.c
2. Rename AST2500 reset kconfig option from AST2500_RESET
   to RESET_AST2500

Signed-off-by: Chia-Wei, Wang 
---
 drivers/reset/Kconfig  | 2 +-
 drivers/reset/{ast2500-reset.c => reset-ast2500.c} | 0
 2 files changed, 1 insertion(+), 1 deletion(-)
 rename drivers/reset/{ast2500-reset.c => reset-ast2500.c} (100%)

diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 796aa267c5..381d222524 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -72,7 +72,7 @@ config RESET_UNIPHIER
  Say Y if you want to control reset signals provided by System Control
  block, Media I/O block, Peripheral Block.
 
-config AST2500_RESET
+config RESET_AST2500
bool "Reset controller driver for AST2500 SoCs"
depends on DM_RESET
default y if ASPEED_AST2500
diff --git a/drivers/reset/ast2500-reset.c b/drivers/reset/reset-ast2500.c
similarity index 100%
rename from drivers/reset/ast2500-reset.c
rename to drivers/reset/reset-ast2500.c
-- 
2.17.1



[PATCH 1/2] reset: ast2500: Use SCU for reset control

2020-09-08 Thread Chia-Wei, Wang
The System Control Unit (SCU) controller of Aspeed
SoCs provides the reset control for each peripheral.

This patch refactors the reset method to leverage
the SCU reset control. Thus the driver dependency
on watchdog including dedicated WDT API and reset
flag encoding can be eliminated.

The Kconfig description is also updated accordingly.

Signed-off-by: Chia-Wei, Wang 
---
 arch/arm/dts/ast2500-u-boot.dtsi  |  7 +-
 drivers/reset/Kconfig |  9 +--
 drivers/reset/ast2500-reset.c | 97 ---
 include/dt-bindings/reset/ast2500-reset.h | 73 +
 4 files changed, 97 insertions(+), 89 deletions(-)

diff --git a/arch/arm/dts/ast2500-u-boot.dtsi b/arch/arm/dts/ast2500-u-boot.dtsi
index 8ac4215745..ca4aac2159 100644
--- a/arch/arm/dts/ast2500-u-boot.dtsi
+++ b/arch/arm/dts/ast2500-u-boot.dtsi
@@ -15,7 +15,6 @@
rst: reset-controller {
u-boot,dm-pre-reloc;
compatible = "aspeed,ast2500-reset";
-   aspeed,wdt = <>;
#reset-cells = <1>;
};
 
@@ -26,7 +25,7 @@
0x1e6e0200 0x1d4 >;
#reset-cells = <1>;
clocks = < PLL_MPLL>;
-   resets = < AST_RESET_SDRAM>;
+   resets = < ASPEED_RESET_SDRAM>;
};
 
ahb {
@@ -40,7 +39,7 @@
reg = <0x1e740100>;
#reset-cells = <1>;
clocks = < BCLK_SDCLK>;
-   resets = < AST_RESET_SDIO>;
+   resets = < ASPEED_RESET_SDIO>;
};
 
sdhci1: sdhci@1e740200 {
@@ -48,7 +47,7 @@
reg = <0x1e740200>;
#reset-cells = <1>;
clocks = < BCLK_SDCLK>;
-   resets = < AST_RESET_SDIO>;
+   resets = < ASPEED_RESET_SDIO>;
};
};
 
diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
index 253902ff57..796aa267c5 100644
--- a/drivers/reset/Kconfig
+++ b/drivers/reset/Kconfig
@@ -74,13 +74,12 @@ config RESET_UNIPHIER
 
 config AST2500_RESET
bool "Reset controller driver for AST2500 SoCs"
-   depends on DM_RESET && WDT_ASPEED
+   depends on DM_RESET
default y if ASPEED_AST2500
help
- Support for reset controller on AST2500 SoC. This controller uses
- watchdog to reset different peripherals and thus only supports
- resets that are supported by watchdog. The main limitation though
- is that some reset signals, like I2C or MISC reset multiple devices.
+ Support for reset controller on AST2500 SoC.
+ Say Y if you want to control reset signals of different peripherals
+ through System Control Unit (SCU).
 
 config RESET_ROCKCHIP
bool "Reset controller driver for Rockchip SoCs"
diff --git a/drivers/reset/ast2500-reset.c b/drivers/reset/ast2500-reset.c
index beb5cd8fa8..e7b5c7deca 100644
--- a/drivers/reset/ast2500-reset.c
+++ b/drivers/reset/ast2500-reset.c
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright 2017 Google, Inc
+ * Copyright 2020 ASPEED Technology Inc.
  */
 
 #include 
@@ -9,28 +10,26 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
-#include 
 
 struct ast2500_reset_priv {
-   /* WDT used to perform resets. */
-   struct udevice *wdt;
struct ast2500_scu *scu;
 };
 
-static int ast2500_ofdata_to_platdata(struct udevice *dev)
+static int ast2500_reset_request(struct reset_ctl *reset_ctl)
 {
-   struct ast2500_reset_priv *priv = dev_get_priv(dev);
-   int ret;
+   debug("%s(reset_ctl=%p) (dev=%p, id=%lu)\n", __func__, reset_ctl,
+ reset_ctl->dev, reset_ctl->id);
 
-   ret = uclass_get_device_by_phandle(UCLASS_WDT, dev, "aspeed,wdt",
-  >wdt);
-   if (ret) {
-   debug("%s: can't find WDT for reset controller", __func__);
-   return ret;
-   }
+   return 0;
+}
+
+static int ast2500_reset_free(struct reset_ctl *reset_ctl)
+{
+   debug("%s(reset_ctl=%p) (dev=%p, id=%lu)\n", __func__, reset_ctl,
+ reset_ctl->dev, reset_ctl->id);
 
return 0;
 }
@@ -38,47 +37,52 @@ static int ast2500_ofdata_to_platdata(struct udevice *dev)
 static int ast2500_reset_assert(struct reset_ctl *reset_ctl)
 {
struct ast2500_reset_priv *priv = dev_get_priv(reset_ctl->dev);
-   u32 reset_mode, reset_mask;
-   bool reset_sdram;
-   int ret;
-
-   /*
-* To reset SDRAM, a specifal flag in SYSRESET register
-* needs to be enabled first
-*/
-   reset_mode = ast_reset_mode_from_flags(reset_ctl->id);
-   reset_mask = ast_reset_mask_from_flags(reset_ctl->id);
-   reset_sdram = 

[PATCH 0/2] Refactor AST2500 reset control

2020-09-08 Thread Chia-Wei, Wang
This patch series refactors the reset method to use the
System Control Unit (SCU) reset control for simplicity.

In addition, the naming of reset driver and Kconfig
option is also refined for future consistency.

Chia-Wei, Wang (2):
  reset: ast2500: Use SCU for reset control
  cosmetic: reset: ast2500: Rename driver and configs

 arch/arm/dts/ast2500-u-boot.dtsi  |   7 +-
 drivers/reset/Kconfig |  11 +--
 drivers/reset/ast2500-reset.c | 104 -
 drivers/reset/reset-ast2500.c | 109 ++
 include/dt-bindings/reset/ast2500-reset.h |  73 ---
 5 files changed, 156 insertions(+), 148 deletions(-)
 delete mode 100644 drivers/reset/ast2500-reset.c
 create mode 100644 drivers/reset/reset-ast2500.c

-- 
2.17.1



Re: [v4, 00/11] mmc: fsl_esdhc: support eMMC HS200/HS400 modes

2020-09-08 Thread Stefan Roese

Hi All,

On 08.09.20 03:08, Peng Fan wrote:

Hi Y.b


Subject: RE: [v4, 00/11] mmc: fsl_esdhc: support eMMC HS200/HS400 modes

Hi Jaehoon and Peng,

Any comments on the v4 patch-set?


Sorry for late. I need postpone the pick up this patchset until next.
I'll give a check, if no issues, I'll pick this into next branch.


I've tested it on the LX2160A-RDB and it the speed improvement is
quite impressive. Thanks for working on this.

Tested-by: Stefan Roese 

Thanks,
Stefan


Thanks,
Peng.


Thank you.

Best regards,
Yangbo Lu


-Original Message-
From: Yangbo Lu 
Sent: Tuesday, September 1, 2020 4:58 PM
To: u-boot@lists.denx.de; Peng Fan ; Priyanka Jain
; 'Jaehoon Chung' 
Cc: Y.b. Lu 
Subject: [v4, 00/11] mmc: fsl_esdhc: support eMMC HS200/HS400 modes

This patch-set is to support eMMC HS200 and HS400 speed modes for
eSDHC, and enable them on LX2160ARDB board.

CI build link
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftrav
is-ci
.org%2Fgithub%2Fyangbolu1991%2Fu-boot-test%2Fbuilds%2F720875619&

a



mp;data=02%7C01%7Cyangbo.lu%40nxp.com%7Caf6b8adab90444f93e1208
d



84e563a07%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637345



479527036436sdata=L26QnuiL3ETvLbr4P1oJqxVJAoVlYY3ROg%2FWpb

J%2B6cc%3Dreserved=0

Changes for v2:
- Added two patches to fix stability issue.
Changes for v3:
- Explained more in commit messages.
- Added HS400 exit code for downgrade.
Changes for v4:
- Checked returning of mmc_hs400_prepare_ddr().
- Added Reviewed-by.
- Rebased.

Yangbo Lu (11):
   mmc: add a reinit() API
   mmc: fsl_esdhc: add a reinit() callback
   mmc: fsl_esdhc: support tuning for eMMC HS200
   mmc: fsl_esdhc: clean TBCTL[TB_EN] manually during init
   mmc: add a hs400_tuning flag
   mmc: add a mmc_hs400_prepare_ddr() interface
   mmc: fsl_esdhc: support eMMC HS400 mode
   mmc: fsl_esdhc: fix mmc->clock with actual clock
   mmc: fsl_esdhc: fix eMMC HS400 stability issue
   arm: dts: lx2160ardb: support eMMC HS400 mode
   configs: lx2160ardb: enable eMMC HS400 mode support

  arch/arm/dts/fsl-lx2160a-rdb.dts |   2 +
  configs/lx2160ardb_tfa_SECURE_BOOT_defconfig |   1 +
  configs/lx2160ardb_tfa_defconfig |   1 +
  configs/lx2160ardb_tfa_stmm_defconfig|   1 +
  drivers/mmc/fsl_esdhc.c  | 176
++-
  drivers/mmc/mmc-uclass.c |  30 +
  drivers/mmc/mmc.c|  14 ++-
  include/fsl_esdhc.h  |  29 -
  include/mmc.h|  26 +++-
  9 files changed, 270 insertions(+), 10 deletions(-)

--
2.7.4





Viele Grüße,
Stefan

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


[PATCH] defconfig: espressobin: enable NET_RANDOM_ETHADDR

2020-09-08 Thread Andre Heider
The hardware does not provide a MAC address. Enable this so that
network access works with just the default environment.

Signed-off-by: Andre Heider 
---
 configs/mvebu_espressobin-88f3720_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/mvebu_espressobin-88f3720_defconfig 
b/configs/mvebu_espressobin-88f3720_defconfig
index 7aabbba59f..5e9fcd1f26 100644
--- a/configs/mvebu_espressobin-88f3720_defconfig
+++ b/configs/mvebu_espressobin-88f3720_defconfig
@@ -84,3 +84,4 @@ CONFIG_USB_ETHER_RTL8152=y
 CONFIG_USB_ETHER_SMSC95XX=y
 CONFIG_SHA1=y
 CONFIG_SHA256=y
+CONFIG_NET_RANDOM_ETHADDR=y
-- 
2.28.0



RE: [PATCH v3 1/2] armv8: lx2162a: Add Soc changes to support LX2162A

2020-09-08 Thread Meenakshi Aggarwal
Thanks Tom,

We will plan it.

-Original Message-
From: Tom Rini  
Sent: Monday, September 7, 2020 6:42 PM
To: Meenakshi Aggarwal 
Cc: u-boot@lists.denx.de; Priyanka Jain ; Varun Sethi 

Subject: Re: [PATCH v3 1/2] armv8: lx2162a: Add Soc changes to support LX2162A

On Mon, Sep 07, 2020 at 03:42:06PM +0530, meenakshi.aggar...@nxp.com wrote:

> From: Meenakshi Aggarwal 
> 
> LX2162 is LX2160 based SoC, it has same die as of LX2160 with 
> different packaging.
> 
> LX2162A support 64-bit 2.9GT/s DDR4 memory, i2c, micro-click module, 
> microSD card, eMMC support, serial console, qspi nor flash, qsgmii, 
> sgmii, 25g, 40g, 50g network interface, one usb 3.0 and serdes 
> interface to support three PCIe gen3 interface.
> 
> Signed-off-by: Meenakshi Aggarwal 
> ---
>  arch/arm/cpu/armv8/Kconfig |  2 +-
>  arch/arm/cpu/armv8/fsl-layerscape/Kconfig  | 39 +--
>  arch/arm/cpu/armv8/fsl-layerscape/Makefile |  5 ++
>  arch/arm/cpu/armv8/fsl-layerscape/cpu.c|  9 ++--
>  arch/arm/cpu/armv8/fsl-layerscape/doc/README.soc   | 58 
> ++

I just want to make sure it's on the TODO list somewhere to convert this 
README.soc file to rST and move it under doc/board/ or similar, thanks!

--
Tom


Re: [RFC PATCH 0/1] Anti rollback protection for FIT Images

2020-09-08 Thread Rasmus Villemoes
On 02/09/2020 09.58, Rasmus Villemoes wrote:
> On 01/09/2020 22.48, Thirupathaiah Annapureddy wrote:
>> Anti rollback protection is required when there is a need to retire
>> previous versions of FIT images due to security flaws in them.
>> Currently U-Boot Verified boot does not have rollback protection to
>> protect against known security flaws.
> 
> This is definitely something we've had on our todo-list/wishlist. But we
> haven't had the time to sit down and work out the semantics and
> implementation, so thanks for doing this.

...

> The board callbacks would simply be given a pointer to the data part of
> that node; that would make the versioning scheme rather flexible instead
> of being limited to a single monotonically increasing u32 (hence also
> the comparison logic should be in the board callbacks, instead of a
> "get/set" interface).

Oh, and another reason for having the board callbacks being responsible
for the Yay/Nay verdict is that that makes it possible to hook up a gpio
that can be used to say "ignore rollback version check" - immensely
useful during development, and might also come in handy for the end
products.

Rasmus


Re: [PATCH v2 0/5] powerpc, mpc83xx: add DM_ETH support

2020-09-08 Thread Heiko Schocher

Hi Mario,

Am 17.08.2020 um 07:23 schrieb Heiko Schocher:

Hello Mario,

Am 27.05.2020 um 14:43 schrieb Heiko Schocher:


This patch series adds DM ethernet support for mpc83xx based
keymile boards.

Travis build:
https://travis-ci.org/github/hsdenx/u-boot-test/builds/691607214


Changes in v2:
- new in v2
- remove RFC
- fixed Codingstyle errors, therefore new patch
   powerpc, mpc83xx: fix codingstyle issues for qe_io.c
- moved DM part to drivers/pinctrl
- add comments from Qiang Zhao:
   - add device node documentation
   - I did not drop the dm_qe_uec_phy.c and use drivers/net/fsl_mdio.c
 because using drivers/net/fsl_mdio.c leads in none existent
 udevice mdio@3320
 instead boards with DM ETH support should use now this
 driver.
- remove RFC tag
- add patch which fixes Codingstyle errors in drivers/qe
- add patch which converts the mpc83xx based boards from
   keymile to DM_ETH

Heiko Schocher (5):
   mpc83xx: remove unneeded extern declaration in cpu_init
   powerpc, qe: fix codingstyle issues for drivers/qe
   powerpc, qe: add DTS support for parallel I/O ports
   net, qe: add DM support for QE UEC ethernet
   mpc83xx, keymile boards: enable DM_ETH and add DTS

  arch/powerpc/cpu/mpc83xx/Kconfig  |    8 +
  arch/powerpc/cpu/mpc83xx/Makefile |    2 +
  arch/powerpc/cpu/mpc83xx/cpu_init.c   |   11 +-
  arch/powerpc/cpu/mpc83xx/qe_io.c  |   98 +-
  arch/powerpc/dts/Makefile |    8 +
  arch/powerpc/dts/km8309-uboot.dtsi    |   33 +
  arch/powerpc/dts/km8321-uboot.dtsi    |   67 +
  arch/powerpc/dts/km8321.dtsi  |  220 
  arch/powerpc/dts/km836x-uboot.dtsi    |   61 +
  arch/powerpc/dts/km836x.dtsi  |  182 +++
  arch/powerpc/dts/kmcoge5ne-uboot.dtsi |   22 +
  arch/powerpc/dts/kmcoge5ne.dts    |  320 +
  arch/powerpc/dts/kmeter1-uboot.dtsi   |   42 +
  arch/powerpc/dts/kmeter1.dts  |  480 +++
  arch/powerpc/dts/kmopti2.dts  |  161 +++
  arch/powerpc/dts/kmsupc5.dts  |  139 ++
  arch/powerpc/dts/kmsupm5.dts  |  129 ++
  arch/powerpc/dts/kmtegr1.dts  |  392 ++
  arch/powerpc/dts/kmtepr2.dts  |  142 ++
  arch/powerpc/dts/kmtuge1.dts  |  100 ++
  arch/powerpc/dts/kmtuxa1.dts  |  100 ++
  board/keymile/km83xx/Kconfig  |   17 +
  board/keymile/km83xx/MAINTAINERS  |   23 +-
  board/keymile/km83xx/km83xx.c |   64 -
  configs/kmcoge5ne_defconfig   |   11 +-
  configs/kmeter1_defconfig |   10 +-
  configs/kmopti2_defconfig |   11 +-
  configs/kmsupx5_defconfig |   10 +-
  configs/kmtegr1_defconfig |   12 +-
  configs/kmtepr2_defconfig |   10 +-
  configs/tuge1_defconfig   |   10 +-
  configs/tuxx1_defconfig   |   11 +-
  .../soc/fsl/cpm_qe/qe/ucc.txt |   53 +
  drivers/net/Kconfig   |    2 +
  drivers/net/Makefile  |    1 +
  drivers/net/qe/Kconfig    |    9 +
  drivers/net/qe/Makefile   |    5 +
  drivers/net/qe/dm_qe_uec.c    | 1167 +
  drivers/net/qe/dm_qe_uec.h    |   22 +
  drivers/net/qe/dm_qe_uec_phy.c    |  163 +++
  drivers/net/qe/uccf.c |  507 +++
  drivers/net/qe/uccf.h |  119 ++
  drivers/net/qe/uec.h  |  693 ++
  drivers/pinctrl/Kconfig   |    7 +
  drivers/pinctrl/Makefile  |    1 +
  drivers/pinctrl/pinctrl-qe-io.c   |  255 
  drivers/qe/qe.c   |   96 +-
  drivers/qe/uccf.c |  449 ---
  drivers/qe/uccf.h |   90 +-
  drivers/qe/uec.c  |  598 -
  drivers/qe/uec.h  |  381 +++---
  drivers/qe/uec_phy.c  |  334 ++---
  drivers/qe/uec_phy.h  |   71 +-
  include/configs/km/km-mpc832x.h   |   14 -
  include/configs/km/km-mpc8360.h   |   14 -
  include/configs/km/km-mpc83xx.h   |   10 -
  include/fsl_qe.h  |    3 +
  57 files changed, 6826 insertions(+), 1144 deletions(-)
  create mode 100644 arch/powerpc/dts/km8309-uboot.dtsi
  create mode 100644 arch/powerpc/dts/km8321-uboot.dtsi
  create mode 100644 arch/powerpc/dts/km8321.dtsi
  create mode 100644 arch/powerpc/dts/km836x-uboot.dtsi
  create mode 100644 arch/powerpc/dts/km836x.dtsi
  create mode 100644 arch/powerpc/dts/kmcoge5ne-uboot.dtsi
  create mode 100644 arch/powerpc/dts/kmcoge5ne.dts
  create