[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Pablo Sebastián Greco]

A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I 
found that it wasn't booting, 2019.01 was working ok.
Bisecting indicated that the problem was after 
http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93

Here's what I see when booting:

U-Boot SPL 2019.04-rc4 (Apr 02 2019 - 16:17:43 +0000)
DRAM: 2048 MiB
Trying to boot from MMC1


U-Boot 2019.04-rc4 (Apr 02 2019 - 16:17:43 +0000) Allwinner Technology

CPU:   Allwinner R40 (SUN8I 1701)
Model: Banana Pi BPI-M2-Ultra
I2C:   ready
DRAM:  2 GiB
MMC:   Device 'mmc at 1c11000': seq 1 is in use by 'mmc at 1c10000'
mmc at 1c0f000: 0, mmc at 1c10000: 2, mmc at 1c11000: 1
Loading Environment from FAT... ** No valid partitions found **
In:    serial at 1c28000
Out:   serial at 1c28000
Err:   serial at 1c28000
SCSI:  Target spinup took 0 ms.
AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
flags: ncq stag pm led clo only pmp pio slum part ccc apst

Net:   phy interface7
eth0: ethernet at 1c50000
starting USB...
No controllers found
Hit any key to stop autoboot:  0
switch to partitions #0, OK
mmc0 is current device
Scanning mmc 0:1...
switch to partitions #0, OK
mmc1(part 0) is current device
** Invalid partition 1 **
scanning bus for devices...
data abort
pc : [<bff9c6da>]          lr : [<bff98991>]
reloc pc : [<4a01a6da>]    lr : [<4a016991>]
sp : bbf5d620  ip : 0000001c     fp : 000000c0
r10: 00000000  r9 : bbf61ed8     r8 : 00000000
r7 : bbf5d8a0  r6 : bffdbff0     r5 : bffdbff0  r4 : bffdbff0
r3 : 00000000  r2 : 00000000     r1 : ea000016  r0 : bffdbff0
Flags: nZcv  IRQs off  FIQs off  Mode SVC_32
Code: e92dbd10 f8d045f0 b0858080 1000f8d8 (f8d14604)
Resetting CPU ...

resetting ...

Any pointers on what to text next?

Thanks.
Pablo.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Paul Kocialkowski]

Hi,

On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I 
> found that it wasn't booting, 2019.01 was working ok.
> Bisecting indicated that the problem was after 
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93

I think the patch should be reverted ASAP since it obviously breaks
some supported configs. Sadly, the offending commit doesn't say
anything about the test coverage for the change and what the status is
after it. There is probably a reason why it was enabled for sun4i only
before and there must have been a motivation for doing this on all
sunxi platforms, but then again, the commit message says nothing about
those underlying reasons.

I believe we should be more strict on patch review and not let any
change bringing such a major change get applied with a commit message
that provides no context about why the change is okay and how it was
tested.

Cheers,

Paul

> Here's what I see when booting:
> 
> U-Boot SPL 2019.04-rc4 (Apr 02 2019 - 16:17:43 +0000)
> DRAM: 2048 MiB
> Trying to boot from MMC1
> 
> 
> U-Boot 2019.04-rc4 (Apr 02 2019 - 16:17:43 +0000) Allwinner Technology
> 
> CPU:   Allwinner R40 (SUN8I 1701)
> Model: Banana Pi BPI-M2-Ultra
> I2C:   ready
> DRAM:  2 GiB
> MMC:   Device 'mmc at 1c11000': seq 1 is in use by 'mmc at 1c10000'
> mmc at 1c0f000: 0, mmc at 1c10000: 2, mmc at 1c11000: 1
> Loading Environment from FAT... ** No valid partitions found **
> In:    serial at 1c28000
> Out:   serial at 1c28000
> Err:   serial at 1c28000
> SCSI:  Target spinup took 0 ms.
> AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> flags: ncq stag pm led clo only pmp pio slum part ccc apst
> 
> Net:   phy interface7
> eth0: ethernet at 1c50000
> starting USB...
> No controllers found
> Hit any key to stop autoboot:  0
> switch to partitions #0, OK
> mmc0 is current device
> Scanning mmc 0:1...
> switch to partitions #0, OK
> mmc1(part 0) is current device
> ** Invalid partition 1 **
> scanning bus for devices...
> data abort
> pc : [<bff9c6da>]          lr : [<bff98991>]
> reloc pc : [<4a01a6da>]    lr : [<4a016991>]
> sp : bbf5d620  ip : 0000001c     fp : 000000c0
> r10: 00000000  r9 : bbf61ed8     r8 : 00000000
> r7 : bbf5d8a0  r6 : bffdbff0     r5 : bffdbff0  r4 : bffdbff0
> r3 : 00000000  r2 : 00000000     r1 : ea000016  r0 : bffdbff0
> Flags: nZcv  IRQs off  FIQs off  Mode SVC_32
> Code: e92dbd10 f8d045f0 b0858080 1000f8d8 (f8d14604)
> Resetting CPU ...
> 
> resetting ...
> 
> Any pointers on what to text next?
> 
> Thanks.
> Pablo.
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Thu, Apr 4, 2019 at 5:12 PM Pablo Sebastián Greco
<pgreco@centosproject.org> wrote:
>
> A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> found that it wasn't booting, 2019.01 was working ok.
> Bisecting indicated that the problem was after
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
>
> Here's what I see when booting:
>
> U-Boot SPL 2019.04-rc4 (Apr 02 2019 - 16:17:43 +0000)
> DRAM: 2048 MiB
> Trying to boot from MMC1
>
>
> U-Boot 2019.04-rc4 (Apr 02 2019 - 16:17:43 +0000) Allwinner Technology
>
> CPU:   Allwinner R40 (SUN8I 1701)
> Model: Banana Pi BPI-M2-Ultra
> I2C:   ready
> DRAM:  2 GiB
> MMC:   Device 'mmc at 1c11000': seq 1 is in use by 'mmc at 1c10000'
> mmc at 1c0f000: 0, mmc at 1c10000: 2, mmc at 1c11000: 1
> Loading Environment from FAT... ** No valid partitions found **

Is your SD partitioned?

> In:    serial at 1c28000
> Out:   serial at 1c28000
> Err:   serial at 1c28000
> SCSI:  Target spinup took 0 ms.
> AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
> flags: ncq stag pm led clo only pmp pio slum part ccc apst
>
> Net:   phy interface7
> eth0: ethernet at 1c50000
> starting USB...
> No controllers found
> Hit any key to stop autoboot:  0
> switch to partitions #0, OK
> mmc0 is current device
> Scanning mmc 0:1...
> switch to partitions #0, OK
> mmc1(part 0) is current device
> ** Invalid partition 1 **
> scanning bus for devices...
> data abort
> pc : [<bff9c6da>]          lr : [<bff98991>]
> reloc pc : [<4a01a6da>]    lr : [<4a016991>]
> sp : bbf5d620  ip : 0000001c     fp : 000000c0
> r10: 00000000  r9 : bbf61ed8     r8 : 00000000
> r7 : bbf5d8a0  r6 : bffdbff0     r5 : bffdbff0  r4 : bffdbff0
> r3 : 00000000  r2 : 00000000     r1 : ea000016  r0 : bffdbff0
> Flags: nZcv  IRQs off  FIQs off  Mode SVC_32
> Code: e92dbd10 f8d045f0 b0858080 1000f8d8 (f8d14604)
> Resetting CPU ...
>
> resetting ...
>
> Any pointers on what to text next?

Check the below logs? I have booted Linux and showed you all the
devices been probed in u-boot.

U-Boot SPL 2019.04-rc4-00124-gafbc31948a (Apr 08 2019 - 18:10:42 +0530)
DRAM: 2048 MiB
Trying to boot from MMC1


U-Boot 2019.04-rc4-00124-gafbc31948a (Apr 08 2019 - 18:10:42 +0530)
Allwinner Technology

CPU:   Allwinner R40 (SUN8I 1701)
Model: Banana Pi BPI-M2-Ultra
I2C:   ready
DRAM:  2 GiB
MMC:   Device 'mmc at 1c11000': seq 1 is in use by 'mmc at 1c10000'
mmc at 1c0f000: 0, mmc at 1c10000: 2, mmc at 1c11000: 1
Loading Environment from FAT... Unable to use mmc 1:0... In:    serial at 1c28000
Out:   serial at 1c28000
Err:   serial at 1c28000
SCSI:  SATA link 0 timeout.
AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
flags: ncq stag pm led clo only pmp pio slum part ccc apst

Net:   phy interface7
eth0: ethernet at 1c50000
starting USB...
No controllers found
Hit any key to stop autoboot:  0
switch to partitions #0, OK
mmc0 is current device
Scanning mmc 0:1...
Found /extlinux/extlinux.conf
Retrieving file: /extlinux/extlinux.conf
157 bytes read in 0 ms
1:      linux-4.18.0-rc3
Retrieving file: /zImage
4128984 bytes read in 186 ms (21.2 MiB/s)
append: console=ttyS0,115200 earlyprintk root=/dev/mmcblk0p2 rootwait
Retrieving file: /sun8i-r40-bananapi-m2-ultra.dtb
19613 bytes read in 5 ms (3.7 MiB/s)
## Flattened Device Tree blob at 43000000
   Booting using the fdt blob at 0x43000000
   Loading Device Tree to 49ff8000, end 49fffc9c ... OK

Starting kernel ...

[    0.000000] Booting Linux on physical CPU 0x0
[    0.000000] Linux version
5.0.0-next-20190306-00055-g12f3f7e4961a-dirty
(jagan at jagan-XPS-13-9350) (gcc version 6.3.1 20170109 (Linaro GCC
6.3-2017.02)
) #51 SMP Fri Mar 22 23:18:35 IST 2019
[    0.000000] CPU: ARMv7 Processor [410fc075] revision 5 (ARMv7), cr=10c5387d
[    0.000000] CPU: div instructions available: patching division code
[    0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing
instruction cache
[    0.000000] OF: fdt: Machine model: Banana Pi BPI-M2-Ultra


U-Boot SPL 2019.04-rc4-00124-gafbc31948a (Apr 08 2019 - 18:10:42 +0530)
DRAM: 2048 MiB
Trying to boot from MMC1


U-Boot 2019.04-rc4-00124-gafbc31948a (Apr 08 2019 - 18:10:42 +0530)
Allwinner Technology

CPU:   Allwinner R40 (SUN8I 1701)
Model: Banana Pi BPI-M2-Ultra
I2C:   ready
DRAM:  2 GiB
MMC:   Device 'mmc at 1c11000': seq 1 is in use by 'mmc at 1c10000'
mmc at 1c0f000: 0, mmc at 1c10000: 2, mmc at 1c11000: 1
Loading Environment from FAT... Unable to use mmc 1:0... In:    serial at 1c28000
Out:   serial at 1c28000
Err:   serial at 1c28000
SCSI:  SATA link 0 timeout.
AHCI 0001.0100 32 slots 1 ports 3 Gbps 0x1 impl SATA mode
flags: ncq stag pm led clo only pmp pio slum part ccc apst

Net:   phy interface7
eth0: ethernet at 1c50000
starting USB...
No controllers found
Hit any key to stop autoboot:  0
=>
=> mmc list
mmc at 1c0f000: 0
mmc at 1c10000: 2
mmc at 1c11000: 1 (eMMC)
=> mmc dev 0
switch to partitions #0, OK
mmc0 is current device
=> fatls mmc 0:1
  4128984   zImage
            extlinux/
    19613   sun8i-r40-bananapi-m2-ultra.dtb

2 file(s), 1 dir(s)

=> ext4ls mmc 0:2
<DIR>       1024 .
<DIR>       1024 ..
<DIR>      12288 lost+found
<DIR>       1024 proc
<DIR>       1024 etc
<DIR>       1024 lib
<DIR>       1024 media
<DIR>       1024 tmp
<DIR>       1024 dev
<SYM>          3 lib32
<DIR>       1024 var
<DIR>       1024 usr
<DIR>       1024 run
<DIR>       1024 sbin
<DIR>       1024 root
<DIR>       2048 bin
<DIR>       1024 opt
<DIR>       1024 mnt
<SYM>         11 linuxrc
<DIR>       1024 sys
=> mmc list
mmc at 1c0f000: 0 (SD)
mmc at 1c10000: 2
mmc at 1c11000: 1 (eMMC)
=> mmc dev 1
switch to partitions #0, OK
mmc1(part 0) is current device

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

Hi Paul,

On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > found that it wasn't booting, 2019.01 was working ok.
> > Bisecting indicated that the problem was after
> > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
>
> I think the patch should be reverted ASAP since it obviously breaks
> some supported configs. Sadly, the offending commit doesn't say
> anything about the test coverage for the change and what the status is
> after it. There is probably a reason why it was enabled for sun4i only
> before and there must have been a motivation for doing this on all
> sunxi platforms, but then again, the commit message says nothing about
> those underlying reasons.
>
> I believe we should be more strict on patch review and not let any
> change bringing such a major change get applied with a commit message
> that provides no context about why the change is okay and how it was
> tested.

Appropriate your concern.

If you please list what all boards are not working with this effect,
please write back. we will defiantly look into it. All these changes
were merged in MW which is 2.5 months back, commenting in final stage
like this is not the professional way.

Jagan.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Paul Kocialkowski]

Hi,

Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> Hi Paul,
> 
> On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > found that it wasn't booting, 2019.01 was working ok.
> > > Bisecting indicated that the problem was after
> > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > 
> > I think the patch should be reverted ASAP since it obviously breaks
> > some supported configs. Sadly, the offending commit doesn't say
> > anything about the test coverage for the change and what the status is
> > after it. There is probably a reason why it was enabled for sun4i only
> > before and there must have been a motivation for doing this on all
> > sunxi platforms, but then again, the commit message says nothing about
> > those underlying reasons.
> > 
> > I believe we should be more strict on patch review and not let any
> > change bringing such a major change get applied with a commit message
> > that provides no context about why the change is okay and how it was
> > tested.
> 
> Appropriate your concern.
> 
> If you please list what all boards are not working with this effect,
> please write back. we will defiantly look into it. All these changes
> were merged in MW which is 2.5 months back, commenting in final stage
> like this is not the professional way.

I really do not think this is a sane approach to follow. You can't make
a change like this, with no context whatsoever in the commit message,
which ends up breaking other people's setups and wait for others to
debug subsequent issues it introduces that you don't encounter.

Sorry but your commit should never have been merged. Sure, I wasn't
there to review it either, but the code review process definitely did
not go as planned here.

The commit you made was not appropriate when you submitted it, it turns
out it broke something and it should be fixed. As for professionalism,
are you actually suggesting that we void fixing your commit as a
courtesy because it went through patch review already? That makes no
sense to me.

Clearly, your work was the issue here so please don't try and dodge the
bullet. I already had the occasion to tell you that your commit
messages are not appropriate when reviewing other patches and this is a
clear result of what happens when a patch skips through patch review
and nobody pushed you to produce a proper commit message. It would be
great if you could take this as a clear example why this is a problem.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> Hi Paul,
> 
> On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> >
> > Hi,
> >
> > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > found that it wasn't booting, 2019.01 was working ok.
> > > Bisecting indicated that the problem was after
> > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> >
> > I think the patch should be reverted ASAP since it obviously breaks
> > some supported configs. Sadly, the offending commit doesn't say
> > anything about the test coverage for the change and what the status is
> > after it. There is probably a reason why it was enabled for sun4i only
> > before and there must have been a motivation for doing this on all
> > sunxi platforms, but then again, the commit message says nothing about
> > those underlying reasons.
> >
> > I believe we should be more strict on patch review and not let any
> > change bringing such a major change get applied with a commit message
> > that provides no context about why the change is okay and how it was
> > tested.
> 
> Appropriate your concern.
> 
> If you please list what all boards are not working with this effect,
> please write back. we will defiantly look into it. All these changes
> were merged in MW which is 2.5 months back, commenting in final stage
> like this is not the professional way.

OK, but what platforms was this all tested on?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/a98b9cbc/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

+ Andre

On Mon, Apr 8, 2019 at 6:40 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > Hi Paul,
> >
> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > found that it wasn't booting, 2019.01 was working ok.
> > > > Bisecting indicated that the problem was after
> > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > >
> > > I think the patch should be reverted ASAP since it obviously breaks
> > > some supported configs. Sadly, the offending commit doesn't say
> > > anything about the test coverage for the change and what the status is
> > > after it. There is probably a reason why it was enabled for sun4i only
> > > before and there must have been a motivation for doing this on all
> > > sunxi platforms, but then again, the commit message says nothing about
> > > those underlying reasons.
> > >
> > > I believe we should be more strict on patch review and not let any
> > > change bringing such a major change get applied with a commit message
> > > that provides no context about why the change is okay and how it was
> > > tested.
> >
> > Appropriate your concern.
> >
> > If you please list what all boards are not working with this effect,
> > please write back. we will defiantly look into it. All these changes
> > were merged in MW which is 2.5 months back, commenting in final stage
> > like this is not the professional way.
>
> OK, but what platforms was this all tested on?  Thanks!

We have tested DM_MMC all the Allwinner SoC platforms and I couldn't
find any mmc issues so-far on my boards atleast.

Jagan.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Mon, Apr 08, 2019 at 06:51:48PM +0530, Jagan Teki wrote:
> + Andre
> 
> On Mon, Apr 8, 2019 at 6:40 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > Hi Paul,
> > >
> > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > Bisecting indicated that the problem was after
> > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > >
> > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > some supported configs. Sadly, the offending commit doesn't say
> > > > anything about the test coverage for the change and what the status is
> > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > before and there must have been a motivation for doing this on all
> > > > sunxi platforms, but then again, the commit message says nothing about
> > > > those underlying reasons.
> > > >
> > > > I believe we should be more strict on patch review and not let any
> > > > change bringing such a major change get applied with a commit message
> > > > that provides no context about why the change is okay and how it was
> > > > tested.
> > >
> > > Appropriate your concern.
> > >
> > > If you please list what all boards are not working with this effect,
> > > please write back. we will defiantly look into it. All these changes
> > > were merged in MW which is 2.5 months back, commenting in final stage
> > > like this is not the professional way.
> >
> > OK, but what platforms was this all tested on?  Thanks!
> 
> We have tested DM_MMC all the Allwinner SoC platforms and I couldn't
> find any mmc issues so-far on my boards atleast.

Sorry, let me try and be clearer.  Which hardware platforms did you test
this on?  There's quite a lot of Allwinner SoCs, and it's not clear at
all if Paul's problem is because of something on the SoC on the Bananapi
M2 Ultra (because it's not clear if another platform with that same SoC
was tested) or how the board is wired (DM MMC is fine on that SoC, but
was tested on something where uSD/MMC are opposite of that platform or
...).

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/bbe7e7c7/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Paul Kocialkowski]

Hi,

Le lundi 08 avril 2019 à 09:25 -0400, Tom Rini a écrit :
> On Mon, Apr 08, 2019 at 06:51:48PM +0530, Jagan Teki wrote:
> > + Andre
> > 
> > On Mon, Apr 8, 2019 at 6:40 PM Tom Rini <trini@konsulko.com> wrote:
> > > On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > > Hi Paul,
> > > > 
> > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > > 
> > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > Bisecting indicated that the problem was after
> > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > 
> > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > anything about the test coverage for the change and what the status is
> > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > before and there must have been a motivation for doing this on all
> > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > those underlying reasons.
> > > > > 
> > > > > I believe we should be more strict on patch review and not let any
> > > > > change bringing such a major change get applied with a commit message
> > > > > that provides no context about why the change is okay and how it was
> > > > > tested.
> > > > 
> > > > Appropriate your concern.
> > > > 
> > > > If you please list what all boards are not working with this effect,
> > > > please write back. we will defiantly look into it. All these changes
> > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > like this is not the professional way.
> > > 
> > > OK, but what platforms was this all tested on?  Thanks!
> > 
> > We have tested DM_MMC all the Allwinner SoC platforms and I couldn't
> > find any mmc issues so-far on my boards atleast.
> 
> Sorry, let me try and be clearer.  Which hardware platforms did you test
> this on?  There's quite a lot of Allwinner SoCs, and it's not clear at
> all if Paul's problem is because of something on the SoC on the Bananapi
> M2 Ultra (because it's not clear if another platform with that same SoC
> was tested) or how the board is wired (DM MMC is fine on that SoC, but
> was tested on something where uSD/MMC are opposite of that platform or
> ...).

For the record, it's not my setup that was broken but Pablo Sebastián
Greco's. I suggested that we revert the offending patch for now and
replace it with a proper one that provides some context about the
change.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Mon, Apr 8, 2019 at 6:55 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Apr 08, 2019 at 06:51:48PM +0530, Jagan Teki wrote:
> > + Andre
> >
> > On Mon, Apr 8, 2019 at 6:40 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > > Hi Paul,
> > > >
> > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > Bisecting indicated that the problem was after
> > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > >
> > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > anything about the test coverage for the change and what the status is
> > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > before and there must have been a motivation for doing this on all
> > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > those underlying reasons.
> > > > >
> > > > > I believe we should be more strict on patch review and not let any
> > > > > change bringing such a major change get applied with a commit message
> > > > > that provides no context about why the change is okay and how it was
> > > > > tested.
> > > >
> > > > Appropriate your concern.
> > > >
> > > > If you please list what all boards are not working with this effect,
> > > > please write back. we will defiantly look into it. All these changes
> > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > like this is not the professional way.
> > >
> > > OK, but what platforms was this all tested on?  Thanks!
> >
> > We have tested DM_MMC all the Allwinner SoC platforms and I couldn't
> > find any mmc issues so-far on my boards atleast.
>
> Sorry, let me try and be clearer.  Which hardware platforms did you test
> this on?  There's quite a lot of Allwinner SoCs, and it's not clear at
> all if Paul's problem is because of something on the SoC on the Bananapi
> M2 Ultra (because it's not clear if another platform with that same SoC
> was tested) or how the board is wired (DM MMC is fine on that SoC, but
> was tested on something where uSD/MMC are opposite of that platform or
> ...).

A13, A20, A83T, R40, H3, H6, A64.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > Hi Paul,
> >
> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi,
> > >
> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > found that it wasn't booting, 2019.01 was working ok.
> > > > Bisecting indicated that the problem was after
> > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > >
> > > I think the patch should be reverted ASAP since it obviously breaks
> > > some supported configs. Sadly, the offending commit doesn't say
> > > anything about the test coverage for the change and what the status is
> > > after it. There is probably a reason why it was enabled for sun4i only
> > > before and there must have been a motivation for doing this on all
> > > sunxi platforms, but then again, the commit message says nothing about
> > > those underlying reasons.
> > >
> > > I believe we should be more strict on patch review and not let any
> > > change bringing such a major change get applied with a commit message
> > > that provides no context about why the change is okay and how it was
> > > tested.
> >
> > Appropriate your concern.
> >
> > If you please list what all boards are not working with this effect,
> > please write back. we will defiantly look into it. All these changes
> > were merged in MW which is 2.5 months back, commenting in final stage
> > like this is not the professional way.
>
> I really do not think this is a sane approach to follow. You can't make
> a change like this, with no context whatsoever in the commit message,
> which ends up breaking other people's setups and wait for others to
> debug subsequent issues it introduces that you don't encounter.
>
> Sorry but your commit should never have been merged. Sure, I wasn't
> there to review it either, but the code review process definitely did
> not go as planned here.

Which commit message your referring to? are you referring this
patch[1] commit message. let me point what exactly is the issue?

[1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Mon, Apr 08, 2019 at 07:03:29PM +0530, Jagan Teki wrote:
> On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> >
> > Hi,
> >
> > Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > > Hi Paul,
> > >
> > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > > Hi,
> > > >
> > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > Bisecting indicated that the problem was after
> > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > >
> > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > some supported configs. Sadly, the offending commit doesn't say
> > > > anything about the test coverage for the change and what the status is
> > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > before and there must have been a motivation for doing this on all
> > > > sunxi platforms, but then again, the commit message says nothing about
> > > > those underlying reasons.
> > > >
> > > > I believe we should be more strict on patch review and not let any
> > > > change bringing such a major change get applied with a commit message
> > > > that provides no context about why the change is okay and how it was
> > > > tested.
> > >
> > > Appropriate your concern.
> > >
> > > If you please list what all boards are not working with this effect,
> > > please write back. we will defiantly look into it. All these changes
> > > were merged in MW which is 2.5 months back, commenting in final stage
> > > like this is not the professional way.
> >
> > I really do not think this is a sane approach to follow. You can't make
> > a change like this, with no context whatsoever in the commit message,
> > which ends up breaking other people's setups and wait for others to
> > debug subsequent issues it introduces that you don't encounter.
> >
> > Sorry but your commit should never have been merged. Sure, I wasn't
> > there to review it either, but the code review process definitely did
> > not go as planned here.
> 
> Which commit message your referring to? are you referring this
> patch[1] commit message. let me point what exactly is the issue?
> 
> [1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93

Yes, that message right there does not say what boards nor SoCs you
tested the conversion on, and is not a great commit message for such a
large scale change.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/9186eef7/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Paul Kocialkowski]

Hi,

Le lundi 08 avril 2019 à 19:03 +0530, Jagan Teki a écrit :
> On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > > Hi Paul,
> > > 
> > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > > Hi,
> > > > 
> > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > Bisecting indicated that the problem was after
> > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > 
> > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > some supported configs. Sadly, the offending commit doesn't say
> > > > anything about the test coverage for the change and what the status is
> > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > before and there must have been a motivation for doing this on all
> > > > sunxi platforms, but then again, the commit message says nothing about
> > > > those underlying reasons.
> > > > 
> > > > I believe we should be more strict on patch review and not let any
> > > > change bringing such a major change get applied with a commit message
> > > > that provides no context about why the change is okay and how it was
> > > > tested.
> > > 
> > > Appropriate your concern.
> > > 
> > > If you please list what all boards are not working with this effect,
> > > please write back. we will defiantly look into it. All these changes
> > > were merged in MW which is 2.5 months back, commenting in final stage
> > > like this is not the professional way.
> > 
> > I really do not think this is a sane approach to follow. You can't make
> > a change like this, with no context whatsoever in the commit message,
> > which ends up breaking other people's setups and wait for others to
> > debug subsequent issues it introduces that you don't encounter.
> > 
> > Sorry but your commit should never have been merged. Sure, I wasn't
> > there to review it either, but the code review process definitely did
> > not go as planned here.
> 
> Which commit message your referring to? are you referring this
> patch[1] commit message. let me point what exactly is the issue?
> 
> [1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93

Yes, this is the patch I'm talking about.

The issue with it is that the commit message is totally redundant: the
description does not say more than what the diff does.

A git commit description should provide context about what the change
does above the modified lines: what problem it tries to resolve and how
, on what hardware, how it was tested, etc. Especially for a commit
making such a big change, the commit message must have all this
information.

Do you see why I think it's a problem?

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Mon, Apr 08, 2019 at 06:59:29PM +0530, Jagan Teki wrote:
> On Mon, Apr 8, 2019 at 6:55 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Apr 08, 2019 at 06:51:48PM +0530, Jagan Teki wrote:
> > > + Andre
> > >
> > > On Mon, Apr 8, 2019 at 6:40 PM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > > > Hi Paul,
> > > > >
> > > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > > Bisecting indicated that the problem was after
> > > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > >
> > > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > anything about the test coverage for the change and what the status is
> > > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > before and there must have been a motivation for doing this on all
> > > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > those underlying reasons.
> > > > > >
> > > > > > I believe we should be more strict on patch review and not let any
> > > > > > change bringing such a major change get applied with a commit message
> > > > > > that provides no context about why the change is okay and how it was
> > > > > > tested.
> > > > >
> > > > > Appropriate your concern.
> > > > >
> > > > > If you please list what all boards are not working with this effect,
> > > > > please write back. we will defiantly look into it. All these changes
> > > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > like this is not the professional way.
> > > >
> > > > OK, but what platforms was this all tested on?  Thanks!
> > >
> > > We have tested DM_MMC all the Allwinner SoC platforms and I couldn't
> > > find any mmc issues so-far on my boards atleast.
> >
> > Sorry, let me try and be clearer.  Which hardware platforms did you test
> > this on?  There's quite a lot of Allwinner SoCs, and it's not clear at
> > all if Paul's problem is because of something on the SoC on the Bananapi
> > M2 Ultra (because it's not clear if another platform with that same SoC
> > was tested) or how the board is wired (DM MMC is fine on that SoC, but
> > was tested on something where uSD/MMC are opposite of that platform or
> > ...).
> 
> A13, A20, A83T, R40, H3, H6, A64.

That's a list of SoCs and not boards that the SoCs are installed on.  It
also doesn't say of those boards which you tested uSD and/or eMMC on.
All of which is really important to know and have recorded for history,
even if we didn't have this problem right here right now.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/6319649d/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Mon, Apr 08, 2019 at 03:28:40PM +0200, Paul Kocialkowski wrote:
> Hi,
> 
> Le lundi 08 avril 2019 à 09:25 -0400, Tom Rini a écrit :
> > On Mon, Apr 08, 2019 at 06:51:48PM +0530, Jagan Teki wrote:
> > > + Andre
> > > 
> > > On Mon, Apr 8, 2019 at 6:40 PM Tom Rini <trini@konsulko.com> wrote:
> > > > On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > > > Hi Paul,
> > > > > 
> > > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > > Bisecting indicated that the problem was after
> > > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > > 
> > > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > anything about the test coverage for the change and what the status is
> > > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > before and there must have been a motivation for doing this on all
> > > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > those underlying reasons.
> > > > > > 
> > > > > > I believe we should be more strict on patch review and not let any
> > > > > > change bringing such a major change get applied with a commit message
> > > > > > that provides no context about why the change is okay and how it was
> > > > > > tested.
> > > > > 
> > > > > Appropriate your concern.
> > > > > 
> > > > > If you please list what all boards are not working with this effect,
> > > > > please write back. we will defiantly look into it. All these changes
> > > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > like this is not the professional way.
> > > > 
> > > > OK, but what platforms was this all tested on?  Thanks!
> > > 
> > > We have tested DM_MMC all the Allwinner SoC platforms and I couldn't
> > > find any mmc issues so-far on my boards atleast.
> > 
> > Sorry, let me try and be clearer.  Which hardware platforms did you test
> > this on?  There's quite a lot of Allwinner SoCs, and it's not clear at
> > all if Paul's problem is because of something on the SoC on the Bananapi
> > M2 Ultra (because it's not clear if another platform with that same SoC
> > was tested) or how the board is wired (DM MMC is fine on that SoC, but
> > was tested on something where uSD/MMC are opposite of that platform or
> > ...).
> 
> For the record, it's not my setup that was broken but Pablo Sebastián
> Greco's. I suggested that we revert the offending patch for now and
> replace it with a proper one that provides some context about the
> change.

Ah, thanks for clarifying.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/e136c6da/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Mon, Apr 8, 2019 at 7:06 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Apr 08, 2019 at 07:03:29PM +0530, Jagan Teki wrote:
> > On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > >
> > > Hi,
> > >
> > > Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > > > Hi Paul,
> > > >
> > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > >
> > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > Bisecting indicated that the problem was after
> > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > >
> > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > anything about the test coverage for the change and what the status is
> > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > before and there must have been a motivation for doing this on all
> > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > those underlying reasons.
> > > > >
> > > > > I believe we should be more strict on patch review and not let any
> > > > > change bringing such a major change get applied with a commit message
> > > > > that provides no context about why the change is okay and how it was
> > > > > tested.
> > > >
> > > > Appropriate your concern.
> > > >
> > > > If you please list what all boards are not working with this effect,
> > > > please write back. we will defiantly look into it. All these changes
> > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > like this is not the professional way.
> > >
> > > I really do not think this is a sane approach to follow. You can't make
> > > a change like this, with no context whatsoever in the commit message,
> > > which ends up breaking other people's setups and wait for others to
> > > debug subsequent issues it introduces that you don't encounter.
> > >
> > > Sorry but your commit should never have been merged. Sure, I wasn't
> > > there to review it either, but the code review process definitely did
> > > not go as planned here.
> >
> > Which commit message your referring to? are you referring this
> > patch[1] commit message. let me point what exactly is the issue?
> >
> > [1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
>
> Yes, that message right there does not say what boards nor SoCs you
> tested the conversion on, and is not a great commit message for such a
> large scale change.

Let me explains what I though when I prepare the patch.

This patch is the last patch of the series, that means all the
preceding patches are supporting DM_MMC on respective or missing SoC's
and boards[2]. as a final patch I enable DM_MMC for Allwinner
globally.

commit message:
"
arm: sunxi: Enable DM_MMC

Enable DM_MMC for all Allwinner SoCs, this will eventually
enable BLK.
"

the commit head says it enable DM_MMC all the SoC's in Allwinner
globally. Since the series is part of DM_MMC migration and previous
commits has enough information of what boards its's been tested. Do we
need to mentioned all the tested board information in this commit
message?

[2] http://git.denx.de/?p=u-boot.git;a=log;h=a7cca5793774ee139b75a704d6efaa4d29f09f93

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Mon, Apr 8, 2019 at 7:06 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> Le lundi 08 avril 2019 à 19:03 +0530, Jagan Teki a écrit :
> > On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi,
> > >
> > > Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > > > Hi Paul,
> > > >
> > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > >
> > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > Bisecting indicated that the problem was after
> > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > >
> > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > anything about the test coverage for the change and what the status is
> > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > before and there must have been a motivation for doing this on all
> > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > those underlying reasons.
> > > > >
> > > > > I believe we should be more strict on patch review and not let any
> > > > > change bringing such a major change get applied with a commit message
> > > > > that provides no context about why the change is okay and how it was
> > > > > tested.
> > > >
> > > > Appropriate your concern.
> > > >
> > > > If you please list what all boards are not working with this effect,
> > > > please write back. we will defiantly look into it. All these changes
> > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > like this is not the professional way.
> > >
> > > I really do not think this is a sane approach to follow. You can't make
> > > a change like this, with no context whatsoever in the commit message,
> > > which ends up breaking other people's setups and wait for others to
> > > debug subsequent issues it introduces that you don't encounter.
> > >
> > > Sorry but your commit should never have been merged. Sure, I wasn't
> > > there to review it either, but the code review process definitely did
> > > not go as planned here.
> >
> > Which commit message your referring to? are you referring this
> > patch[1] commit message. let me point what exactly is the issue?
> >
> > [1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
>
> Yes, this is the patch I'm talking about.
>
> The issue with it is that the commit message is totally redundant: the
> description does not say more than what the diff does.
>
> A git commit description should provide context about what the change
> does above the modified lines: what problem it tries to resolve and how
> , on what hardware, how it was tested, etc. Especially for a commit
> making such a big change, the commit message must have all this
> information.
>
> Do you see why I think it's a problem?

I can't agree if we consider the series of changes in one set. As I
mentioned in another mail, this patch is last from the DM_MMC
migration series and as the last one it enable the DM_MMC global to
Allwinner (not respective to board). The previous patches are trying
to support and fix DM_MMC on respective SoC's and board. ie the reason
I didn't mention any thing related to board or any other information
since It is global SoC change.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Mon, Apr 8, 2019 at 7:07 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Apr 08, 2019 at 06:59:29PM +0530, Jagan Teki wrote:
> > On Mon, Apr 8, 2019 at 6:55 PM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Apr 08, 2019 at 06:51:48PM +0530, Jagan Teki wrote:
> > > > + Andre
> > > >
> > > > On Mon, Apr 8, 2019 at 6:40 PM Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > > > > Hi Paul,
> > > > > >
> > > > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > > > Bisecting indicated that the problem was after
> > > > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > > >
> > > > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > > anything about the test coverage for the change and what the status is
> > > > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > > before and there must have been a motivation for doing this on all
> > > > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > > those underlying reasons.
> > > > > > >
> > > > > > > I believe we should be more strict on patch review and not let any
> > > > > > > change bringing such a major change get applied with a commit message
> > > > > > > that provides no context about why the change is okay and how it was
> > > > > > > tested.
> > > > > >
> > > > > > Appropriate your concern.
> > > > > >
> > > > > > If you please list what all boards are not working with this effect,
> > > > > > please write back. we will defiantly look into it. All these changes
> > > > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > > like this is not the professional way.
> > > > >
> > > > > OK, but what platforms was this all tested on?  Thanks!
> > > >
> > > > We have tested DM_MMC all the Allwinner SoC platforms and I couldn't
> > > > find any mmc issues so-far on my boards atleast.
> > >
> > > Sorry, let me try and be clearer.  Which hardware platforms did you test
> > > this on?  There's quite a lot of Allwinner SoCs, and it's not clear at
> > > all if Paul's problem is because of something on the SoC on the Bananapi
> > > M2 Ultra (because it's not clear if another platform with that same SoC
> > > was tested) or how the board is wired (DM MMC is fine on that SoC, but
> > > was tested on something where uSD/MMC are opposite of that platform or
> > > ...).
> >
> > A13, A20, A83T, R40, H3, H6, A64.
>
> That's a list of SoCs and not boards that the SoCs are installed on.  It
> also doesn't say of those boards which you tested uSD and/or eMMC on.
> All of which is really important to know and have recorded for history,
> even if we didn't have this problem right here right now.

You can consider one board on each SoC family. Since this series
dealing with entire Allwinner family, I have mentioned respective
tested families.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Paul Kocialkowski]

Hi,

Le lundi 08 avril 2019 à 19:21 +0530, Jagan Teki a écrit :
> On Mon, Apr 8, 2019 at 7:06 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > Le lundi 08 avril 2019 à 19:03 +0530, Jagan Teki a écrit :
> > > On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > > Hi,
> > > > 
> > > > Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > > > > Hi Paul,
> > > > > 
> > > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > > Bisecting indicated that the problem was after
> > > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > > 
> > > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > anything about the test coverage for the change and what the status is
> > > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > before and there must have been a motivation for doing this on all
> > > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > those underlying reasons.
> > > > > > 
> > > > > > I believe we should be more strict on patch review and not let any
> > > > > > change bringing such a major change get applied with a commit message
> > > > > > that provides no context about why the change is okay and how it was
> > > > > > tested.
> > > > > 
> > > > > Appropriate your concern.
> > > > > 
> > > > > If you please list what all boards are not working with this effect,
> > > > > please write back. we will defiantly look into it. All these changes
> > > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > like this is not the professional way.
> > > > 
> > > > I really do not think this is a sane approach to follow. You can't make
> > > > a change like this, with no context whatsoever in the commit message,
> > > > which ends up breaking other people's setups and wait for others to
> > > > debug subsequent issues it introduces that you don't encounter.
> > > > 
> > > > Sorry but your commit should never have been merged. Sure, I wasn't
> > > > there to review it either, but the code review process definitely did
> > > > not go as planned here.
> > > 
> > > Which commit message your referring to? are you referring this
> > > patch[1] commit message. let me point what exactly is the issue?
> > > 
> > > [1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > 
> > Yes, this is the patch I'm talking about.
> > 
> > The issue with it is that the commit message is totally redundant: the
> > description does not say more than what the diff does.
> > 
> > A git commit description should provide context about what the change
> > does above the modified lines: what problem it tries to resolve and how
> > , on what hardware, how it was tested, etc. Especially for a commit
> > making such a big change, the commit message must have all this
> > information.
> > 
> > Do you see why I think it's a problem?
> 
> I can't agree if we consider the series of changes in one set. As I
> mentioned in another mail, this patch is last from the DM_MMC
> migration series and as the last one it enable the DM_MMC global to
> Allwinner (not respective to board). The previous patches are trying
> to support and fix DM_MMC on respective SoC's and board. ie the reason
> I didn't mention any thing related to board or any other information
> since It is global SoC change.

I don't think this is relevant. You can't expect people to go through
the list archive, find which series this commit was attached to,
etc. Especially when running a bisect, where you'll end up with a
single offending commit.

You need to make sure that each commit message provides context about
what it's doing, and not just rephrase the diff.

In order words, it's *never* okay to just re-work the diff, you
*always* need to describe the context. That's not something optional to
only do on special occasions.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Mon, Apr 8, 2019 at 7:31 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> Le lundi 08 avril 2019 à 19:21 +0530, Jagan Teki a écrit :
> > On Mon, Apr 8, 2019 at 7:06 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > > Hi,
> > >
> > > Le lundi 08 avril 2019 à 19:03 +0530, Jagan Teki a écrit :
> > > > On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > >
> > > > > Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > > > > > Hi Paul,
> > > > > >
> > > > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > > > Bisecting indicated that the problem was after
> > > > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > > >
> > > > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > > anything about the test coverage for the change and what the status is
> > > > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > > before and there must have been a motivation for doing this on all
> > > > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > > those underlying reasons.
> > > > > > >
> > > > > > > I believe we should be more strict on patch review and not let any
> > > > > > > change bringing such a major change get applied with a commit message
> > > > > > > that provides no context about why the change is okay and how it was
> > > > > > > tested.
> > > > > >
> > > > > > Appropriate your concern.
> > > > > >
> > > > > > If you please list what all boards are not working with this effect,
> > > > > > please write back. we will defiantly look into it. All these changes
> > > > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > > like this is not the professional way.
> > > > >
> > > > > I really do not think this is a sane approach to follow. You can't make
> > > > > a change like this, with no context whatsoever in the commit message,
> > > > > which ends up breaking other people's setups and wait for others to
> > > > > debug subsequent issues it introduces that you don't encounter.
> > > > >
> > > > > Sorry but your commit should never have been merged. Sure, I wasn't
> > > > > there to review it either, but the code review process definitely did
> > > > > not go as planned here.
> > > >
> > > > Which commit message your referring to? are you referring this
> > > > patch[1] commit message. let me point what exactly is the issue?
> > > >
> > > > [1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > >
> > > Yes, this is the patch I'm talking about.
> > >
> > > The issue with it is that the commit message is totally redundant: the
> > > description does not say more than what the diff does.
> > >
> > > A git commit description should provide context about what the change
> > > does above the modified lines: what problem it tries to resolve and how
> > > , on what hardware, how it was tested, etc. Especially for a commit
> > > making such a big change, the commit message must have all this
> > > information.
> > >
> > > Do you see why I think it's a problem?
> >
> > I can't agree if we consider the series of changes in one set. As I
> > mentioned in another mail, this patch is last from the DM_MMC
> > migration series and as the last one it enable the DM_MMC global to
> > Allwinner (not respective to board). The previous patches are trying
> > to support and fix DM_MMC on respective SoC's and board. ie the reason
> > I didn't mention any thing related to board or any other information
> > since It is global SoC change.
>
> I don't think this is relevant. You can't expect people to go through
> the list archive, find which series this commit was attached to,
> etc. Especially when running a bisect, where you'll end up with a
> single offending commit.
>
> You need to make sure that each commit message provides context about
> what it's doing, and not just rephrase the diff.

Do you still think this message shows the diff? The change is as
simple as enable DM_MMC for Allwinner and doesn't make any direct
changes to underlying boards on the same change.

"Enable DM_MMC for all Allwinner SoCs, this will eventually
enable BLK."

>
> In order words, it's *never* okay to just re-work the diff, you
> *always* need to describe the context. That's not something optional to
> only do on special occasions.

Mentioning again, I don't see any point to mention board information
for this commit since the diff or change clearly focusing on global
Allwinner platform enablement for DM_MMC and the same mentioned in the
body.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Paul Kocialkowski]

Hi,

Le lundi 08 avril 2019 à 19:47 +0530, Jagan Teki a écrit :
> On Mon, Apr 8, 2019 at 7:31 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> > Hi,
> > 
> > Le lundi 08 avril 2019 à 19:21 +0530, Jagan Teki a écrit :
> > > On Mon, Apr 8, 2019 at 7:06 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > > Hi,
> > > > 
> > > > Le lundi 08 avril 2019 à 19:03 +0530, Jagan Teki a écrit :
> > > > > On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
> > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > > > > > > Hi Paul,
> > > > > > > 
> > > > > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > > > Hi,
> > > > > > > > 
> > > > > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > > > > Bisecting indicated that the problem was after
> > > > > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > > > > 
> > > > > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > > > anything about the test coverage for the change and what the status is
> > > > > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > > > before and there must have been a motivation for doing this on all
> > > > > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > > > those underlying reasons.
> > > > > > > > 
> > > > > > > > I believe we should be more strict on patch review and not let any
> > > > > > > > change bringing such a major change get applied with a commit message
> > > > > > > > that provides no context about why the change is okay and how it was
> > > > > > > > tested.
> > > > > > > 
> > > > > > > Appropriate your concern.
> > > > > > > 
> > > > > > > If you please list what all boards are not working with this effect,
> > > > > > > please write back. we will defiantly look into it. All these changes
> > > > > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > > > like this is not the professional way.
> > > > > > 
> > > > > > I really do not think this is a sane approach to follow. You can't make
> > > > > > a change like this, with no context whatsoever in the commit message,
> > > > > > which ends up breaking other people's setups and wait for others to
> > > > > > debug subsequent issues it introduces that you don't encounter.
> > > > > > 
> > > > > > Sorry but your commit should never have been merged. Sure, I wasn't
> > > > > > there to review it either, but the code review process definitely did
> > > > > > not go as planned here.
> > > > > 
> > > > > Which commit message your referring to? are you referring this
> > > > > patch[1] commit message. let me point what exactly is the issue?
> > > > > 
> > > > > [1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > 
> > > > Yes, this is the patch I'm talking about.
> > > > 
> > > > The issue with it is that the commit message is totally redundant: the
> > > > description does not say more than what the diff does.
> > > > 
> > > > A git commit description should provide context about what the change
> > > > does above the modified lines: what problem it tries to resolve and how
> > > > , on what hardware, how it was tested, etc. Especially for a commit
> > > > making such a big change, the commit message must have all this
> > > > information.
> > > > 
> > > > Do you see why I think it's a problem?
> > > 
> > > I can't agree if we consider the series of changes in one set. As I
> > > mentioned in another mail, this patch is last from the DM_MMC
> > > migration series and as the last one it enable the DM_MMC global to
> > > Allwinner (not respective to board). The previous patches are trying
> > > to support and fix DM_MMC on respective SoC's and board. ie the reason
> > > I didn't mention any thing related to board or any other information
> > > since It is global SoC change.
> > 
> > I don't think this is relevant. You can't expect people to go through
> > the list archive, find which series this commit was attached to,
> > etc. Especially when running a bisect, where you'll end up with a
> > single offending commit.
> > 
> > You need to make sure that each commit message provides context about
> > what it's doing, and not just rephrase the diff.
> 
> Do you still think this message shows the diff? The change is as
> simple as enable DM_MMC for Allwinner and doesn't make any direct
> changes to underlying boards on the same change.
> 
> "Enable DM_MMC for all Allwinner SoCs, this will eventually
> enable BLK."

Of course! Enabling DM_MMC is a major change to Allwinner MMC support
and it is not "as simple as enable DM_MMC for Allwinner". You need to
think about the context and implications of the change, beyond the
change itself and present that. That's the whole point of having git
commit messages: to present changes and their implications.

I don't see how I could make myself any more clear here, so let me give
you a simple example. Consider the following code:

void foo(int *bar, int *baz)
{
	if (bar && baz)
		*bar = *baz;
	else if (baz)
		*baz = 2;
}

Now we change that to:

void foo(int *bar, int *baz)
{
	if (bar)
		*bar = *baz;
	else if (baz)
		*baz = 2;
}

Here is one commit message about it:

"Remove check before dereferencing baz

The check on baz is removed from the if."

Here is another commit message about it:

"Remove check before dereferencing baz

The core (which is by design the only caller to this function)
guarantees that whenever foo is called and bar is non-NULL, baz is also
a valid pointer. Drop the check on baz since it is redundant."

Now in the first case, we have no context or explanation and we have no
idea why the change is okay. In the second one, we know precisely why
we can remove the check on baz and why it's safe to reference it.

Without knowing this, it's impossible to tell if the patch is just
wrong (it removes a check on a pointer before dereferencing it) or if
it is legitimate and makes sense.

Do you see my point?

> > In order words, it's *never* okay to just re-work the diff, you
> > *always* need to describe the context. That's not something optional to
> > only do on special occasions.
> 
> Mentioning again, I don't see any point to mention board information
> for this commit since the diff or change clearly focusing on global
> Allwinner platform enablement for DM_MMC and the same mentioned in the
> body.

That's the point: of course you don't see why need to explain things
because they are already obvious to you as the author of the path. You
must think about everyone else who is not you and who will read the
patch with no other context. They must be able to understand what is
going on just by reading the patch. This is one of the most important
areas to contributing to public free software projects and it is what
allows us to keep sane code bases, understand previous decisions, etc.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Mon, Apr 08, 2019 at 07:47:13PM +0530, Jagan Teki wrote:
> On Mon, Apr 8, 2019 at 7:31 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> >
> > Hi,
> >
> > Le lundi 08 avril 2019 à 19:21 +0530, Jagan Teki a écrit :
> > > On Mon, Apr 8, 2019 at 7:06 PM Paul Kocialkowski
> > > <paul.kocialkowski@bootlin.com> wrote:
> > > > Hi,
> > > >
> > > > Le lundi 08 avril 2019 à 19:03 +0530, Jagan Teki a écrit :
> > > > > On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
> > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > Hi,
> > > > > >
> > > > > > Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > > > > > > Hi Paul,
> > > > > > >
> > > > > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > > > > Bisecting indicated that the problem was after
> > > > > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > > > >
> > > > > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > > > anything about the test coverage for the change and what the status is
> > > > > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > > > before and there must have been a motivation for doing this on all
> > > > > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > > > those underlying reasons.
> > > > > > > >
> > > > > > > > I believe we should be more strict on patch review and not let any
> > > > > > > > change bringing such a major change get applied with a commit message
> > > > > > > > that provides no context about why the change is okay and how it was
> > > > > > > > tested.
> > > > > > >
> > > > > > > Appropriate your concern.
> > > > > > >
> > > > > > > If you please list what all boards are not working with this effect,
> > > > > > > please write back. we will defiantly look into it. All these changes
> > > > > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > > > like this is not the professional way.
> > > > > >
> > > > > > I really do not think this is a sane approach to follow. You can't make
> > > > > > a change like this, with no context whatsoever in the commit message,
> > > > > > which ends up breaking other people's setups and wait for others to
> > > > > > debug subsequent issues it introduces that you don't encounter.
> > > > > >
> > > > > > Sorry but your commit should never have been merged. Sure, I wasn't
> > > > > > there to review it either, but the code review process definitely did
> > > > > > not go as planned here.
> > > > >
> > > > > Which commit message your referring to? are you referring this
> > > > > patch[1] commit message. let me point what exactly is the issue?
> > > > >
> > > > > [1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > >
> > > > Yes, this is the patch I'm talking about.
> > > >
> > > > The issue with it is that the commit message is totally redundant: the
> > > > description does not say more than what the diff does.
> > > >
> > > > A git commit description should provide context about what the change
> > > > does above the modified lines: what problem it tries to resolve and how
> > > > , on what hardware, how it was tested, etc. Especially for a commit
> > > > making such a big change, the commit message must have all this
> > > > information.
> > > >
> > > > Do you see why I think it's a problem?
> > >
> > > I can't agree if we consider the series of changes in one set. As I
> > > mentioned in another mail, this patch is last from the DM_MMC
> > > migration series and as the last one it enable the DM_MMC global to
> > > Allwinner (not respective to board). The previous patches are trying
> > > to support and fix DM_MMC on respective SoC's and board. ie the reason
> > > I didn't mention any thing related to board or any other information
> > > since It is global SoC change.
> >
> > I don't think this is relevant. You can't expect people to go through
> > the list archive, find which series this commit was attached to,
> > etc. Especially when running a bisect, where you'll end up with a
> > single offending commit.
> >
> > You need to make sure that each commit message provides context about
> > what it's doing, and not just rephrase the diff.
> 
> Do you still think this message shows the diff? The change is as
> simple as enable DM_MMC for Allwinner and doesn't make any direct
> changes to underlying boards on the same change.
> 
> "Enable DM_MMC for all Allwinner SoCs, this will eventually
> enable BLK."
> 
> >
> > In order words, it's *never* okay to just re-work the diff, you
> > *always* need to describe the context. That's not something optional to
> > only do on special occasions.
> 
> Mentioning again, I don't see any point to mention board information
> for this commit since the diff or change clearly focusing on global
> Allwinner platform enablement for DM_MMC and the same mentioned in the
> body.

Here's the problem.  None of the commits in the series say what hardware
you tested this on.  If it's not in the commit message then it's not
preserved for easy access.  You _need_ to say what combinations of
hardware you tested changes on in the commit message.  Yes, some of the
previous commits are about a specific SoC but that doesn't enumerate
what it was all tested on.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/a4807738/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Mon, Apr 8, 2019 at 8:18 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Apr 08, 2019 at 07:47:13PM +0530, Jagan Teki wrote:
> > On Mon, Apr 8, 2019 at 7:31 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > >
> > > Hi,
> > >
> > > Le lundi 08 avril 2019 à 19:21 +0530, Jagan Teki a écrit :
> > > > On Mon, Apr 8, 2019 at 7:06 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > >
> > > > > Le lundi 08 avril 2019 à 19:03 +0530, Jagan Teki a écrit :
> > > > > > On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
> > > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > > > > > > > Hi Paul,
> > > > > > > >
> > > > > > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > > > > > Bisecting indicated that the problem was after
> > > > > > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > > > > >
> > > > > > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > > > > anything about the test coverage for the change and what the status is
> > > > > > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > > > > before and there must have been a motivation for doing this on all
> > > > > > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > > > > those underlying reasons.
> > > > > > > > >
> > > > > > > > > I believe we should be more strict on patch review and not let any
> > > > > > > > > change bringing such a major change get applied with a commit message
> > > > > > > > > that provides no context about why the change is okay and how it was
> > > > > > > > > tested.
> > > > > > > >
> > > > > > > > Appropriate your concern.
> > > > > > > >
> > > > > > > > If you please list what all boards are not working with this effect,
> > > > > > > > please write back. we will defiantly look into it. All these changes
> > > > > > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > > > > like this is not the professional way.
> > > > > > >
> > > > > > > I really do not think this is a sane approach to follow. You can't make
> > > > > > > a change like this, with no context whatsoever in the commit message,
> > > > > > > which ends up breaking other people's setups and wait for others to
> > > > > > > debug subsequent issues it introduces that you don't encounter.
> > > > > > >
> > > > > > > Sorry but your commit should never have been merged. Sure, I wasn't
> > > > > > > there to review it either, but the code review process definitely did
> > > > > > > not go as planned here.
> > > > > >
> > > > > > Which commit message your referring to? are you referring this
> > > > > > patch[1] commit message. let me point what exactly is the issue?
> > > > > >
> > > > > > [1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > >
> > > > > Yes, this is the patch I'm talking about.
> > > > >
> > > > > The issue with it is that the commit message is totally redundant: the
> > > > > description does not say more than what the diff does.
> > > > >
> > > > > A git commit description should provide context about what the change
> > > > > does above the modified lines: what problem it tries to resolve and how
> > > > > , on what hardware, how it was tested, etc. Especially for a commit
> > > > > making such a big change, the commit message must have all this
> > > > > information.
> > > > >
> > > > > Do you see why I think it's a problem?
> > > >
> > > > I can't agree if we consider the series of changes in one set. As I
> > > > mentioned in another mail, this patch is last from the DM_MMC
> > > > migration series and as the last one it enable the DM_MMC global to
> > > > Allwinner (not respective to board). The previous patches are trying
> > > > to support and fix DM_MMC on respective SoC's and board. ie the reason
> > > > I didn't mention any thing related to board or any other information
> > > > since It is global SoC change.
> > >
> > > I don't think this is relevant. You can't expect people to go through
> > > the list archive, find which series this commit was attached to,
> > > etc. Especially when running a bisect, where you'll end up with a
> > > single offending commit.
> > >
> > > You need to make sure that each commit message provides context about
> > > what it's doing, and not just rephrase the diff.
> >
> > Do you still think this message shows the diff? The change is as
> > simple as enable DM_MMC for Allwinner and doesn't make any direct
> > changes to underlying boards on the same change.
> >
> > "Enable DM_MMC for all Allwinner SoCs, this will eventually
> > enable BLK."
> >
> > >
> > > In order words, it's *never* okay to just re-work the diff, you
> > > *always* need to describe the context. That's not something optional to
> > > only do on special occasions.
> >
> > Mentioning again, I don't see any point to mention board information
> > for this commit since the diff or change clearly focusing on global
> > Allwinner platform enablement for DM_MMC and the same mentioned in the
> > body.
>
> Here's the problem.  None of the commits in the series say what hardware
> you tested this on.  If it's not in the commit message then it's not
> preserved for easy access.  You _need_ to say what combinations of
> hardware you tested changes on in the commit message.  Yes, some of the
> previous commits are about a specific SoC but that doesn't enumerate
> what it was all tested on.

First of all, how can I or anyone mention the tested board information
on the commit messaged w/o Tested-by. This seems new to community,
atleast as I saw from my experience. I didn't aware of any commit show
this kind of information and last but not least the commit that
support any new change may not require to mention the board or
hardware information intentionally it is by  default that the commit
tested on respective changes with effecting hardware.

Hope it clear.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Michael Nazzareno Trimarchi]

Hi

On Mon, Apr 8, 2019 at 4:48 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Apr 08, 2019 at 07:47:13PM +0530, Jagan Teki wrote:
> > On Mon, Apr 8, 2019 at 7:31 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > >
> > > Hi,
> > >
> > > Le lundi 08 avril 2019 à 19:21 +0530, Jagan Teki a écrit :
> > > > On Mon, Apr 8, 2019 at 7:06 PM Paul Kocialkowski
> > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > Hi,
> > > > >
> > > > > Le lundi 08 avril 2019 à 19:03 +0530, Jagan Teki a écrit :
> > > > > > On Mon, Apr 8, 2019 at 6:40 PM Paul Kocialkowski
> > > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > Le lundi 08 avril 2019 à 18:23 +0530, Jagan Teki a écrit :
> > > > > > > > Hi Paul,
> > > > > > > >
> > > > > > > > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > > > > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > > > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > > > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > > > > > Bisecting indicated that the problem was after
> > > > > > > > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > > > > >
> > > > > > > > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > > > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > > > > anything about the test coverage for the change and what the status is
> > > > > > > > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > > > > before and there must have been a motivation for doing this on all
> > > > > > > > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > > > > those underlying reasons.
> > > > > > > > >
> > > > > > > > > I believe we should be more strict on patch review and not let any
> > > > > > > > > change bringing such a major change get applied with a commit message
> > > > > > > > > that provides no context about why the change is okay and how it was
> > > > > > > > > tested.
> > > > > > > >
> > > > > > > > Appropriate your concern.
> > > > > > > >
> > > > > > > > If you please list what all boards are not working with this effect,
> > > > > > > > please write back. we will defiantly look into it. All these changes
> > > > > > > > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > > > > like this is not the professional way.
> > > > > > >
> > > > > > > I really do not think this is a sane approach to follow. You can't make
> > > > > > > a change like this, with no context whatsoever in the commit message,
> > > > > > > which ends up breaking other people's setups and wait for others to
> > > > > > > debug subsequent issues it introduces that you don't encounter.
> > > > > > >
> > > > > > > Sorry but your commit should never have been merged. Sure, I wasn't
> > > > > > > there to review it either, but the code review process definitely did
> > > > > > > not go as planned here.
> > > > > >
> > > > > > Which commit message your referring to? are you referring this
> > > > > > patch[1] commit message. let me point what exactly is the issue?
> > > > > >
> > > > > > [1] http://git.denx.de/?p=u-boot.git;a=commit;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > >
> > > > > Yes, this is the patch I'm talking about.
> > > > >
> > > > > The issue with it is that the commit message is totally redundant: the
> > > > > description does not say more than what the diff does.
> > > > >
> > > > > A git commit description should provide context about what the change
> > > > > does above the modified lines: what problem it tries to resolve and how
> > > > > , on what hardware, how it was tested, etc. Especially for a commit
> > > > > making such a big change, the commit message must have all this
> > > > > information.
> > > > >
> > > > > Do you see why I think it's a problem?
> > > >
> > > > I can't agree if we consider the series of changes in one set. As I
> > > > mentioned in another mail, this patch is last from the DM_MMC
> > > > migration series and as the last one it enable the DM_MMC global to
> > > > Allwinner (not respective to board). The previous patches are trying
> > > > to support and fix DM_MMC on respective SoC's and board. ie the reason
> > > > I didn't mention any thing related to board or any other information
> > > > since It is global SoC change.
> > >
> > > I don't think this is relevant. You can't expect people to go through
> > > the list archive, find which series this commit was attached to,
> > > etc. Especially when running a bisect, where you'll end up with a
> > > single offending commit.
> > >
> > > You need to make sure that each commit message provides context about
> > > what it's doing, and not just rephrase the diff.
> >
> > Do you still think this message shows the diff? The change is as
> > simple as enable DM_MMC for Allwinner and doesn't make any direct
> > changes to underlying boards on the same change.
> >
> > "Enable DM_MMC for all Allwinner SoCs, this will eventually
> > enable BLK."
> >
> > >
> > > In order words, it's *never* okay to just re-work the diff, you
> > > *always* need to describe the context. That's not something optional to
> > > only do on special occasions.
> >
> > Mentioning again, I don't see any point to mention board information
> > for this commit since the diff or change clearly focusing on global
> > Allwinner platform enablement for DM_MMC and the same mentioned in the
> > body.
>
> Here's the problem.  None of the commits in the series say what hardware
> you tested this on.  If it's not in the commit message then it's not
> preserved for easy access.  You _need_ to say what combinations of
> hardware you tested changes on in the commit message.  Yes, some of the
> previous commits are about a specific SoC but that doesn't enumerate
> what it was all tested on.
>

Let me try to go back to the topic. We have one setup that does not
work and it was
working. It's not first time that this happen and every time that
large changes are merged
sometime some setup stop to work even for multiple releases. If this
change will be reverted
we just give a commit message that explain the reason why needs to be
reverted, including
all the information.

Michael

> --
> Tom
> _______________________________________________
> U-Boot mailing list
> U-Boot at Let's give a summer. You think that commit message was not having enough description even was part
on some large change and that now is braking one board. This is not
the first time that is happen and
I don't see such long thread in the past. Right now we know that it
breaks one setup and your suggestion is to revert.
I think that such things happen and it's part of uboot update. Can we
go back to initial email?
> https://lists.denx.de/listinfo/u-boot

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> Hi Paul,
> 
> On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> <paul.kocialkowski@bootlin.com> wrote:
> >
> > Hi,
> >
> > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > found that it wasn't booting, 2019.01 was working ok.
> > > Bisecting indicated that the problem was after
> > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> >
> > I think the patch should be reverted ASAP since it obviously breaks
> > some supported configs. Sadly, the offending commit doesn't say
> > anything about the test coverage for the change and what the status is
> > after it. There is probably a reason why it was enabled for sun4i only
> > before and there must have been a motivation for doing this on all
> > sunxi platforms, but then again, the commit message says nothing about
> > those underlying reasons.
> >
> > I believe we should be more strict on patch review and not let any
> > change bringing such a major change get applied with a commit message
> > that provides no context about why the change is okay and how it was
> > tested.
> 
> Appropriate your concern.
> 
> If you please list what all boards are not working with this effect,
> please write back. we will defiantly look into it. All these changes
> were merged in MW which is 2.5 months back, commenting in final stage
> like this is not the professional way.

Being release day, here's my big concern.  How bad is this?  Is it a
single platform?  Later in the thread Jagan did enumerate the SoCs he
tested the overall series on.  But there's a lot of Allwinner SoCs and
boards.  I have a pine64 somewhere around here, but that's already been
checked off.  My other allwinner platform I took out of my testing loop
due to it not being a reliable piece of hardware.  So, does anyone have
a feel for how many platforms may or may not be broken right now?
Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/6bb1e7b2/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Michael Nazzareno Trimarchi]

Hi jagan


On Mon., 8 Apr. 2019, 8:26 pm Tom Rini, <trini@konsulko.com> wrote:

> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > Hi Paul,
> >
> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > <paul.kocialkowski@bootlin.com> wrote:
> > >
> > > Hi,
> > >
> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > found that it wasn't booting, 2019.01 was working ok.
> > > > Bisecting indicated that the problem was after
> > > >
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > >
> > > I think the patch should be reverted ASAP since it obviously breaks
> > > some supported configs. Sadly, the offending commit doesn't say
> > > anything about the test coverage for the change and what the status is
> > > after it. There is probably a reason why it was enabled for sun4i only
> > > before and there must have been a motivation for doing this on all
> > > sunxi platforms, but then again, the commit message says nothing about
> > > those underlying reasons.
> > >
> > > I believe we should be more strict on patch review and not let any
> > > change bringing such a major change get applied with a commit message
> > > that provides no context about why the change is okay and how it was
> > > tested.
> >
> > Appropriate your concern.
> >
> > If you please list what all boards are not working with this effect,
> > please write back. we will defiantly look into it. All these changes
> > were merged in MW which is 2.5 months back, commenting in final stage
> > like this is not the professional way.
>
> Being release day, here's my big concern.  How bad is this?  Is it a
> single platform?  Later in the thread Jagan did enumerate the SoCs he
> tested the overall series on.  But there's a lot of Allwinner SoCs and
> boards.  I have a pine64 somewhere around here, but that's already been
> checked off.  My other allwinner platform I took out of my testing loop
> due to it not being a reliable piece of hardware.  So, does anyone have
> a feel for how many platforms may or may not be broken right now?
> Thanks!
>

You have 13 to 15 boards. Can you just report all of them?

Michael

>
> --
> Tom
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Tue, Apr 9, 2019 at 12:02 AM Michael Nazzareno Trimarchi
<michael@amarulasolutions.com> wrote:
>
> Hi jagan
>
>
> On Mon., 8 Apr. 2019, 8:26 pm Tom Rini, <trini@konsulko.com> wrote:
>>
>> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
>> > Hi Paul,
>> >
>> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
>> > <paul.kocialkowski@bootlin.com> wrote:
>> > >
>> > > Hi,
>> > >
>> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
>> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
>> > > > found that it wasn't booting, 2019.01 was working ok.
>> > > > Bisecting indicated that the problem was after
>> > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
>> > >
>> > > I think the patch should be reverted ASAP since it obviously breaks
>> > > some supported configs. Sadly, the offending commit doesn't say
>> > > anything about the test coverage for the change and what the status is
>> > > after it. There is probably a reason why it was enabled for sun4i only
>> > > before and there must have been a motivation for doing this on all
>> > > sunxi platforms, but then again, the commit message says nothing about
>> > > those underlying reasons.
>> > >
>> > > I believe we should be more strict on patch review and not let any
>> > > change bringing such a major change get applied with a commit message
>> > > that provides no context about why the change is okay and how it was
>> > > tested.
>> >
>> > Appropriate your concern.
>> >
>> > If you please list what all boards are not working with this effect,
>> > please write back. we will defiantly look into it. All these changes
>> > were merged in MW which is 2.5 months back, commenting in final stage
>> > like this is not the professional way.
>>
>> Being release day, here's my big concern.  How bad is this?  Is it a
>> single platform?  Later in the thread Jagan did enumerate the SoCs he
>> tested the overall series on.  But there's a lot of Allwinner SoCs and
>> boards.  I have a pine64 somewhere around here, but that's already been
>> checked off.  My other allwinner platform I took out of my testing loop
>> due to it not being a reliable piece of hardware.  So, does anyone have
>> a feel for how many platforms may or may not be broken right now?
>> Thanks!
>
>
> You have 13 to 15 boards. Can you just report all of them?

Issue, seems to be on SCSI side. MMC is able to probe and boot (you
may see initial logs on the thread). Enabling DM_MMC breaking SCSI
reads, debugging same with Pablo will get back.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Tue, Apr 09, 2019 at 12:17:15AM +0530, Jagan Teki wrote:
> On Tue, Apr 9, 2019 at 12:02 AM Michael Nazzareno Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > Hi jagan
> >
> >
> > On Mon., 8 Apr. 2019, 8:26 pm Tom Rini, <trini@konsulko.com> wrote:
> >>
> >> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> >> > Hi Paul,
> >> >
> >> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> >> > <paul.kocialkowski@bootlin.com> wrote:
> >> > >
> >> > > Hi,
> >> > >
> >> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> >> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> >> > > > found that it wasn't booting, 2019.01 was working ok.
> >> > > > Bisecting indicated that the problem was after
> >> > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> >> > >
> >> > > I think the patch should be reverted ASAP since it obviously breaks
> >> > > some supported configs. Sadly, the offending commit doesn't say
> >> > > anything about the test coverage for the change and what the status is
> >> > > after it. There is probably a reason why it was enabled for sun4i only
> >> > > before and there must have been a motivation for doing this on all
> >> > > sunxi platforms, but then again, the commit message says nothing about
> >> > > those underlying reasons.
> >> > >
> >> > > I believe we should be more strict on patch review and not let any
> >> > > change bringing such a major change get applied with a commit message
> >> > > that provides no context about why the change is okay and how it was
> >> > > tested.
> >> >
> >> > Appropriate your concern.
> >> >
> >> > If you please list what all boards are not working with this effect,
> >> > please write back. we will defiantly look into it. All these changes
> >> > were merged in MW which is 2.5 months back, commenting in final stage
> >> > like this is not the professional way.
> >>
> >> Being release day, here's my big concern.  How bad is this?  Is it a
> >> single platform?  Later in the thread Jagan did enumerate the SoCs he
> >> tested the overall series on.  But there's a lot of Allwinner SoCs and
> >> boards.  I have a pine64 somewhere around here, but that's already been
> >> checked off.  My other allwinner platform I took out of my testing loop
> >> due to it not being a reliable piece of hardware.  So, does anyone have
> >> a feel for how many platforms may or may not be broken right now?
> >> Thanks!
> >
> >
> > You have 13 to 15 boards. Can you just report all of them?
> 
> Issue, seems to be on SCSI side. MMC is able to probe and boot (you
> may see initial logs on the thread). Enabling DM_MMC breaking SCSI
> reads, debugging same with Pablo will get back.

OK, thanks, please keep us up to date!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/4c0d4d91/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

Hi Tom,

On Tue, Apr 9, 2019 at 12:19 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Apr 09, 2019 at 12:17:15AM +0530, Jagan Teki wrote:
> > On Tue, Apr 9, 2019 at 12:02 AM Michael Nazzareno Trimarchi
> > <michael@amarulasolutions.com> wrote:
> > >
> > > Hi jagan
> > >
> > >
> > > On Mon., 8 Apr. 2019, 8:26 pm Tom Rini, <trini@konsulko.com> wrote:
> > >>
> > >> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > >> > Hi Paul,
> > >> >
> > >> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > >> > <paul.kocialkowski@bootlin.com> wrote:
> > >> > >
> > >> > > Hi,
> > >> > >
> > >> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > >> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > >> > > > found that it wasn't booting, 2019.01 was working ok.
> > >> > > > Bisecting indicated that the problem was after
> > >> > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > >> > >
> > >> > > I think the patch should be reverted ASAP since it obviously breaks
> > >> > > some supported configs. Sadly, the offending commit doesn't say
> > >> > > anything about the test coverage for the change and what the status is
> > >> > > after it. There is probably a reason why it was enabled for sun4i only
> > >> > > before and there must have been a motivation for doing this on all
> > >> > > sunxi platforms, but then again, the commit message says nothing about
> > >> > > those underlying reasons.
> > >> > >
> > >> > > I believe we should be more strict on patch review and not let any
> > >> > > change bringing such a major change get applied with a commit message
> > >> > > that provides no context about why the change is okay and how it was
> > >> > > tested.
> > >> >
> > >> > Appropriate your concern.
> > >> >
> > >> > If you please list what all boards are not working with this effect,
> > >> > please write back. we will defiantly look into it. All these changes
> > >> > were merged in MW which is 2.5 months back, commenting in final stage
> > >> > like this is not the professional way.
> > >>
> > >> Being release day, here's my big concern.  How bad is this?  Is it a
> > >> single platform?  Later in the thread Jagan did enumerate the SoCs he
> > >> tested the overall series on.  But there's a lot of Allwinner SoCs and
> > >> boards.  I have a pine64 somewhere around here, but that's already been
> > >> checked off.  My other allwinner platform I took out of my testing loop
> > >> due to it not being a reliable piece of hardware.  So, does anyone have
> > >> a feel for how many platforms may or may not be broken right now?
> > >> Thanks!
> > >
> > >
> > > You have 13 to 15 boards. Can you just report all of them?
> >
> > Issue, seems to be on SCSI side. MMC is able to probe and boot (you
> > may see initial logs on the thread). Enabling DM_MMC breaking SCSI
> > reads, debugging same with Pablo will get back.
>
> OK, thanks, please keep us up to date!

As expected, SCSI reads failing. DM_MMC is forcing BLK so which
expecting boards to enable DM_SCSI. So the migration plan for DM_SCSI
and DM_MMC should be in same line, as of now it's not.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Tue, Apr 09, 2019 at 12:37:04AM +0530, Jagan Teki wrote:
> Hi Tom,
> 
> On Tue, Apr 9, 2019 at 12:19 AM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Apr 09, 2019 at 12:17:15AM +0530, Jagan Teki wrote:
> > > On Tue, Apr 9, 2019 at 12:02 AM Michael Nazzareno Trimarchi
> > > <michael@amarulasolutions.com> wrote:
> > > >
> > > > Hi jagan
> > > >
> > > >
> > > > On Mon., 8 Apr. 2019, 8:26 pm Tom Rini, <trini@konsulko.com> wrote:
> > > >>
> > > >> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > >> > Hi Paul,
> > > >> >
> > > >> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > >> > <paul.kocialkowski@bootlin.com> wrote:
> > > >> > >
> > > >> > > Hi,
> > > >> > >
> > > >> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > >> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > >> > > > found that it wasn't booting, 2019.01 was working ok.
> > > >> > > > Bisecting indicated that the problem was after
> > > >> > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > >> > >
> > > >> > > I think the patch should be reverted ASAP since it obviously breaks
> > > >> > > some supported configs. Sadly, the offending commit doesn't say
> > > >> > > anything about the test coverage for the change and what the status is
> > > >> > > after it. There is probably a reason why it was enabled for sun4i only
> > > >> > > before and there must have been a motivation for doing this on all
> > > >> > > sunxi platforms, but then again, the commit message says nothing about
> > > >> > > those underlying reasons.
> > > >> > >
> > > >> > > I believe we should be more strict on patch review and not let any
> > > >> > > change bringing such a major change get applied with a commit message
> > > >> > > that provides no context about why the change is okay and how it was
> > > >> > > tested.
> > > >> >
> > > >> > Appropriate your concern.
> > > >> >
> > > >> > If you please list what all boards are not working with this effect,
> > > >> > please write back. we will defiantly look into it. All these changes
> > > >> > were merged in MW which is 2.5 months back, commenting in final stage
> > > >> > like this is not the professional way.
> > > >>
> > > >> Being release day, here's my big concern.  How bad is this?  Is it a
> > > >> single platform?  Later in the thread Jagan did enumerate the SoCs he
> > > >> tested the overall series on.  But there's a lot of Allwinner SoCs and
> > > >> boards.  I have a pine64 somewhere around here, but that's already been
> > > >> checked off.  My other allwinner platform I took out of my testing loop
> > > >> due to it not being a reliable piece of hardware.  So, does anyone have
> > > >> a feel for how many platforms may or may not be broken right now?
> > > >> Thanks!
> > > >
> > > >
> > > > You have 13 to 15 boards. Can you just report all of them?
> > >
> > > Issue, seems to be on SCSI side. MMC is able to probe and boot (you
> > > may see initial logs on the thread). Enabling DM_MMC breaking SCSI
> > > reads, debugging same with Pablo will get back.
> >
> > OK, thanks, please keep us up to date!
> 
> As expected, SCSI reads failing. DM_MMC is forcing BLK so which
> expecting boards to enable DM_SCSI. So the migration plan for DM_SCSI
> and DM_MMC should be in same line, as of now it's not.

Ah, is the scsi driver for Allwinner DM-ified yet?  And for the release
I'd really like to do today can we just switch off DM_MMC on the DM_SCSI
Allwinner platforms?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/a171b714/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Tue, Apr 9, 2019 at 12:39 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Apr 09, 2019 at 12:37:04AM +0530, Jagan Teki wrote:
> > Hi Tom,
> >
> > On Tue, Apr 9, 2019 at 12:19 AM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Tue, Apr 09, 2019 at 12:17:15AM +0530, Jagan Teki wrote:
> > > > On Tue, Apr 9, 2019 at 12:02 AM Michael Nazzareno Trimarchi
> > > > <michael@amarulasolutions.com> wrote:
> > > > >
> > > > > Hi jagan
> > > > >
> > > > >
> > > > > On Mon., 8 Apr. 2019, 8:26 pm Tom Rini, <trini@konsulko.com> wrote:
> > > > >>
> > > > >> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > > >> > Hi Paul,
> > > > >> >
> > > > >> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > >> > <paul.kocialkowski@bootlin.com> wrote:
> > > > >> > >
> > > > >> > > Hi,
> > > > >> > >
> > > > >> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > >> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > >> > > > found that it wasn't booting, 2019.01 was working ok.
> > > > >> > > > Bisecting indicated that the problem was after
> > > > >> > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > >> > >
> > > > >> > > I think the patch should be reverted ASAP since it obviously breaks
> > > > >> > > some supported configs. Sadly, the offending commit doesn't say
> > > > >> > > anything about the test coverage for the change and what the status is
> > > > >> > > after it. There is probably a reason why it was enabled for sun4i only
> > > > >> > > before and there must have been a motivation for doing this on all
> > > > >> > > sunxi platforms, but then again, the commit message says nothing about
> > > > >> > > those underlying reasons.
> > > > >> > >
> > > > >> > > I believe we should be more strict on patch review and not let any
> > > > >> > > change bringing such a major change get applied with a commit message
> > > > >> > > that provides no context about why the change is okay and how it was
> > > > >> > > tested.
> > > > >> >
> > > > >> > Appropriate your concern.
> > > > >> >
> > > > >> > If you please list what all boards are not working with this effect,
> > > > >> > please write back. we will defiantly look into it. All these changes
> > > > >> > were merged in MW which is 2.5 months back, commenting in final stage
> > > > >> > like this is not the professional way.
> > > > >>
> > > > >> Being release day, here's my big concern.  How bad is this?  Is it a
> > > > >> single platform?  Later in the thread Jagan did enumerate the SoCs he
> > > > >> tested the overall series on.  But there's a lot of Allwinner SoCs and
> > > > >> boards.  I have a pine64 somewhere around here, but that's already been
> > > > >> checked off.  My other allwinner platform I took out of my testing loop
> > > > >> due to it not being a reliable piece of hardware.  So, does anyone have
> > > > >> a feel for how many platforms may or may not be broken right now?
> > > > >> Thanks!
> > > > >
> > > > >
> > > > > You have 13 to 15 boards. Can you just report all of them?
> > > >
> > > > Issue, seems to be on SCSI side. MMC is able to probe and boot (you
> > > > may see initial logs on the thread). Enabling DM_MMC breaking SCSI
> > > > reads, debugging same with Pablo will get back.
> > >
> > > OK, thanks, please keep us up to date!
> >
> > As expected, SCSI reads failing. DM_MMC is forcing BLK so which
> > expecting boards to enable DM_SCSI. So the migration plan for DM_SCSI
> > and DM_MMC should be in same line, as of now it's not.
>
> Ah, is the scsi driver for Allwinner DM-ified yet?  And for the release
> I'd really like to do today can we just switch off DM_MMC on the DM_SCSI
> Allwinner platforms?

Since the migration plan still there for DM_SCSI (v2019.07) we haven't
enabled DM_SCSI yet.

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Tue, Apr 09, 2019 at 12:41:48AM +0530, Jagan Teki wrote:
> On Tue, Apr 9, 2019 at 12:39 AM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Apr 09, 2019 at 12:37:04AM +0530, Jagan Teki wrote:
> > > Hi Tom,
> > >
> > > On Tue, Apr 9, 2019 at 12:19 AM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Tue, Apr 09, 2019 at 12:17:15AM +0530, Jagan Teki wrote:
> > > > > On Tue, Apr 9, 2019 at 12:02 AM Michael Nazzareno Trimarchi
> > > > > <michael@amarulasolutions.com> wrote:
> > > > > >
> > > > > > Hi jagan
> > > > > >
> > > > > >
> > > > > > On Mon., 8 Apr. 2019, 8:26 pm Tom Rini, <trini@konsulko.com> wrote:
> > > > > >>
> > > > > >> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > > > >> > Hi Paul,
> > > > > >> >
> > > > > >> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > >> > <paul.kocialkowski@bootlin.com> wrote:
> > > > > >> > >
> > > > > >> > > Hi,
> > > > > >> > >
> > > > > >> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > >> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > >> > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > >> > > > Bisecting indicated that the problem was after
> > > > > >> > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > >> > >
> > > > > >> > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > >> > > some supported configs. Sadly, the offending commit doesn't say
> > > > > >> > > anything about the test coverage for the change and what the status is
> > > > > >> > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > >> > > before and there must have been a motivation for doing this on all
> > > > > >> > > sunxi platforms, but then again, the commit message says nothing about
> > > > > >> > > those underlying reasons.
> > > > > >> > >
> > > > > >> > > I believe we should be more strict on patch review and not let any
> > > > > >> > > change bringing such a major change get applied with a commit message
> > > > > >> > > that provides no context about why the change is okay and how it was
> > > > > >> > > tested.
> > > > > >> >
> > > > > >> > Appropriate your concern.
> > > > > >> >
> > > > > >> > If you please list what all boards are not working with this effect,
> > > > > >> > please write back. we will defiantly look into it. All these changes
> > > > > >> > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > >> > like this is not the professional way.
> > > > > >>
> > > > > >> Being release day, here's my big concern.  How bad is this?  Is it a
> > > > > >> single platform?  Later in the thread Jagan did enumerate the SoCs he
> > > > > >> tested the overall series on.  But there's a lot of Allwinner SoCs and
> > > > > >> boards.  I have a pine64 somewhere around here, but that's already been
> > > > > >> checked off.  My other allwinner platform I took out of my testing loop
> > > > > >> due to it not being a reliable piece of hardware.  So, does anyone have
> > > > > >> a feel for how many platforms may or may not be broken right now?
> > > > > >> Thanks!
> > > > > >
> > > > > >
> > > > > > You have 13 to 15 boards. Can you just report all of them?
> > > > >
> > > > > Issue, seems to be on SCSI side. MMC is able to probe and boot (you
> > > > > may see initial logs on the thread). Enabling DM_MMC breaking SCSI
> > > > > reads, debugging same with Pablo will get back.
> > > >
> > > > OK, thanks, please keep us up to date!
> > >
> > > As expected, SCSI reads failing. DM_MMC is forcing BLK so which
> > > expecting boards to enable DM_SCSI. So the migration plan for DM_SCSI
> > > and DM_MMC should be in same line, as of now it's not.
> >
> > Ah, is the scsi driver for Allwinner DM-ified yet?  And for the release
> > I'd really like to do today can we just switch off DM_MMC on the DM_SCSI
> > Allwinner platforms?
> 
> Since the migration plan still there for DM_SCSI (v2019.07) we haven't
> enabled DM_SCSI yet.

Except on sun4i.  But different driver then?  But I guess for today we
need to use:
        select DM_MMC if MMC && !(DM_SCSI && SCSI)
?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/a1de22b5/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Jagan Teki]

On Tue, Apr 9, 2019 at 12:43 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Apr 09, 2019 at 12:41:48AM +0530, Jagan Teki wrote:
> > On Tue, Apr 9, 2019 at 12:39 AM Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Tue, Apr 09, 2019 at 12:37:04AM +0530, Jagan Teki wrote:
> > > > Hi Tom,
> > > >
> > > > On Tue, Apr 9, 2019 at 12:19 AM Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Tue, Apr 09, 2019 at 12:17:15AM +0530, Jagan Teki wrote:
> > > > > > On Tue, Apr 9, 2019 at 12:02 AM Michael Nazzareno Trimarchi
> > > > > > <michael@amarulasolutions.com> wrote:
> > > > > > >
> > > > > > > Hi jagan
> > > > > > >
> > > > > > >
> > > > > > > On Mon., 8 Apr. 2019, 8:26 pm Tom Rini, <trini@konsulko.com> wrote:
> > > > > > >>
> > > > > > >> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > > > > >> > Hi Paul,
> > > > > > >> >
> > > > > > >> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > > >> > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > >> > >
> > > > > > >> > > Hi,
> > > > > > >> > >
> > > > > > >> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > >> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > >> > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > >> > > > Bisecting indicated that the problem was after
> > > > > > >> > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > > >> > >
> > > > > > >> > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > >> > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > >> > > anything about the test coverage for the change and what the status is
> > > > > > >> > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > >> > > before and there must have been a motivation for doing this on all
> > > > > > >> > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > >> > > those underlying reasons.
> > > > > > >> > >
> > > > > > >> > > I believe we should be more strict on patch review and not let any
> > > > > > >> > > change bringing such a major change get applied with a commit message
> > > > > > >> > > that provides no context about why the change is okay and how it was
> > > > > > >> > > tested.
> > > > > > >> >
> > > > > > >> > Appropriate your concern.
> > > > > > >> >
> > > > > > >> > If you please list what all boards are not working with this effect,
> > > > > > >> > please write back. we will defiantly look into it. All these changes
> > > > > > >> > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > > >> > like this is not the professional way.
> > > > > > >>
> > > > > > >> Being release day, here's my big concern.  How bad is this?  Is it a
> > > > > > >> single platform?  Later in the thread Jagan did enumerate the SoCs he
> > > > > > >> tested the overall series on.  But there's a lot of Allwinner SoCs and
> > > > > > >> boards.  I have a pine64 somewhere around here, but that's already been
> > > > > > >> checked off.  My other allwinner platform I took out of my testing loop
> > > > > > >> due to it not being a reliable piece of hardware.  So, does anyone have
> > > > > > >> a feel for how many platforms may or may not be broken right now?
> > > > > > >> Thanks!
> > > > > > >
> > > > > > >
> > > > > > > You have 13 to 15 boards. Can you just report all of them?
> > > > > >
> > > > > > Issue, seems to be on SCSI side. MMC is able to probe and boot (you
> > > > > > may see initial logs on the thread). Enabling DM_MMC breaking SCSI
> > > > > > reads, debugging same with Pablo will get back.
> > > > >
> > > > > OK, thanks, please keep us up to date!
> > > >
> > > > As expected, SCSI reads failing. DM_MMC is forcing BLK so which
> > > > expecting boards to enable DM_SCSI. So the migration plan for DM_SCSI
> > > > and DM_MMC should be in same line, as of now it's not.
> > >
> > > Ah, is the scsi driver for Allwinner DM-ified yet?  And for the release
> > > I'd really like to do today can we just switch off DM_MMC on the DM_SCSI
> > > Allwinner platforms?
> >
> > Since the migration plan still there for DM_SCSI (v2019.07) we haven't
> > enabled DM_SCSI yet.
>
> Except on sun4i.  But different driver then?  But I guess for today we
> need to use:
>         select DM_MMC if MMC && !(DM_SCSI && SCSI)
> ?

Yes, A10 has DM_SCSI


Above solutions seems to be 'recursive dependency' issue.

drivers/scsi/Kconfig:11:error: recursive dependency detected!
drivers/scsi/Kconfig:11:    symbol DM_SCSI depends on BLK
drivers/block/Kconfig:4:    symbol BLK depends on DM_MMC
drivers/mmc/Kconfig:26:    symbol DM_MMC is selected by DM_SCSI
For a resolution refer to Documentation/kbuild/kconfig-language.txt
subsection "Kconfig recursive dependency limitations"

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Tom Rini]

On Tue, Apr 09, 2019 at 12:56:48AM +0530, Jagan Teki wrote:
> On Tue, Apr 9, 2019 at 12:43 AM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Apr 09, 2019 at 12:41:48AM +0530, Jagan Teki wrote:
> > > On Tue, Apr 9, 2019 at 12:39 AM Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Tue, Apr 09, 2019 at 12:37:04AM +0530, Jagan Teki wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Tue, Apr 9, 2019 at 12:19 AM Tom Rini <trini@konsulko.com> wrote:
> > > > > >
> > > > > > On Tue, Apr 09, 2019 at 12:17:15AM +0530, Jagan Teki wrote:
> > > > > > > On Tue, Apr 9, 2019 at 12:02 AM Michael Nazzareno Trimarchi
> > > > > > > <michael@amarulasolutions.com> wrote:
> > > > > > > >
> > > > > > > > Hi jagan
> > > > > > > >
> > > > > > > >
> > > > > > > > On Mon., 8 Apr. 2019, 8:26 pm Tom Rini, <trini@konsulko.com> wrote:
> > > > > > > >>
> > > > > > > >> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> > > > > > > >> > Hi Paul,
> > > > > > > >> >
> > > > > > > >> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> > > > > > > >> > <paul.kocialkowski@bootlin.com> wrote:
> > > > > > > >> > >
> > > > > > > >> > > Hi,
> > > > > > > >> > >
> > > > > > > >> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> > > > > > > >> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with 2019.04rc, I
> > > > > > > >> > > > found that it wasn't booting, 2019.01 was working ok.
> > > > > > > >> > > > Bisecting indicated that the problem was after
> > > > > > > >> > > > http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> > > > > > > >> > >
> > > > > > > >> > > I think the patch should be reverted ASAP since it obviously breaks
> > > > > > > >> > > some supported configs. Sadly, the offending commit doesn't say
> > > > > > > >> > > anything about the test coverage for the change and what the status is
> > > > > > > >> > > after it. There is probably a reason why it was enabled for sun4i only
> > > > > > > >> > > before and there must have been a motivation for doing this on all
> > > > > > > >> > > sunxi platforms, but then again, the commit message says nothing about
> > > > > > > >> > > those underlying reasons.
> > > > > > > >> > >
> > > > > > > >> > > I believe we should be more strict on patch review and not let any
> > > > > > > >> > > change bringing such a major change get applied with a commit message
> > > > > > > >> > > that provides no context about why the change is okay and how it was
> > > > > > > >> > > tested.
> > > > > > > >> >
> > > > > > > >> > Appropriate your concern.
> > > > > > > >> >
> > > > > > > >> > If you please list what all boards are not working with this effect,
> > > > > > > >> > please write back. we will defiantly look into it. All these changes
> > > > > > > >> > were merged in MW which is 2.5 months back, commenting in final stage
> > > > > > > >> > like this is not the professional way.
> > > > > > > >>
> > > > > > > >> Being release day, here's my big concern.  How bad is this?  Is it a
> > > > > > > >> single platform?  Later in the thread Jagan did enumerate the SoCs he
> > > > > > > >> tested the overall series on.  But there's a lot of Allwinner SoCs and
> > > > > > > >> boards.  I have a pine64 somewhere around here, but that's already been
> > > > > > > >> checked off.  My other allwinner platform I took out of my testing loop
> > > > > > > >> due to it not being a reliable piece of hardware.  So, does anyone have
> > > > > > > >> a feel for how many platforms may or may not be broken right now?
> > > > > > > >> Thanks!
> > > > > > > >
> > > > > > > >
> > > > > > > > You have 13 to 15 boards. Can you just report all of them?
> > > > > > >
> > > > > > > Issue, seems to be on SCSI side. MMC is able to probe and boot (you
> > > > > > > may see initial logs on the thread). Enabling DM_MMC breaking SCSI
> > > > > > > reads, debugging same with Pablo will get back.
> > > > > >
> > > > > > OK, thanks, please keep us up to date!
> > > > >
> > > > > As expected, SCSI reads failing. DM_MMC is forcing BLK so which
> > > > > expecting boards to enable DM_SCSI. So the migration plan for DM_SCSI
> > > > > and DM_MMC should be in same line, as of now it's not.
> > > >
> > > > Ah, is the scsi driver for Allwinner DM-ified yet?  And for the release
> > > > I'd really like to do today can we just switch off DM_MMC on the DM_SCSI
> > > > Allwinner platforms?
> > >
> > > Since the migration plan still there for DM_SCSI (v2019.07) we haven't
> > > enabled DM_SCSI yet.
> >
> > Except on sun4i.  But different driver then?  But I guess for today we
> > need to use:
> >         select DM_MMC if MMC && !(DM_SCSI && SCSI)
> > ?
> 
> Yes, A10 has DM_SCSI
> 
> 
> Above solutions seems to be 'recursive dependency' issue.
> 
> drivers/scsi/Kconfig:11:error: recursive dependency detected!
> drivers/scsi/Kconfig:11:    symbol DM_SCSI depends on BLK
> drivers/block/Kconfig:4:    symbol BLK depends on DM_MMC
> drivers/mmc/Kconfig:26:    symbol DM_MMC is selected by DM_SCSI
> For a resolution refer to Documentation/kbuild/kconfig-language.txt
> subsection "Kconfig recursive dependency limitations"

Yeah, due to where we set it for SUN4I.  Can you rework things a little
so there's not a recursive dep?  Perhaps move the DM_MMC if MMC line to
each of the SoCs for today and for v2019.07 we'll have both DM_MMC and
DM_SCSI top-line.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190408/a7f6f399/attachment.sig>

[U-Boot] arm: sunxi: Bananapi_M2_Ultra not working with DM_MMC

[Peter Robinson]

On Tue, 9 Apr 2019, 01:48 Jagan Teki, <jagan@amarulasolutions.com> wrote:

> On Tue, Apr 9, 2019 at 12:02 AM Michael Nazzareno Trimarchi
> <michael@amarulasolutions.com> wrote:
> >
> > Hi jagan
> >
> >
> > On Mon., 8 Apr. 2019, 8:26 pm Tom Rini, <trini@konsulko.com> wrote:
> >>
> >> On Mon, Apr 08, 2019 at 06:23:29PM +0530, Jagan Teki wrote:
> >> > Hi Paul,
> >> >
> >> > On Mon, Apr 8, 2019 at 6:00 PM Paul Kocialkowski
> >> > <paul.kocialkowski@bootlin.com> wrote:
> >> > >
> >> > > Hi,
> >> > >
> >> > > On Thu, 2019-04-04 at 05:51 -0300, Pablo Sebastián Greco wrote:
> >> > > > A few days ago I tried to boot my Bananapi_M2_Ultra with
> 2019.04rc, I
> >> > > > found that it wasn't booting, 2019.01 was working ok.
> >> > > > Bisecting indicated that the problem was after
> >> > > >
> http://git.denx.de/?p=u-boot.git;a=commitdiff;h=a7cca5793774ee139b75a704d6efaa4d29f09f93
> >> > >
> >> > > I think the patch should be reverted ASAP since it obviously breaks
> >> > > some supported configs. Sadly, the offending commit doesn't say
> >> > > anything about the test coverage for the change and what the status
> is
> >> > > after it. There is probably a reason why it was enabled for sun4i
> only
> >> > > before and there must have been a motivation for doing this on all
> >> > > sunxi platforms, but then again, the commit message says nothing
> about
> >> > > those underlying reasons.
> >> > >
> >> > > I believe we should be more strict on patch review and not let any
> >> > > change bringing such a major change get applied with a commit
> message
> >> > > that provides no context about why the change is okay and how it was
> >> > > tested.
> >> >
> >> > Appropriate your concern.
> >> >
> >> > If you please list what all boards are not working with this effect,
> >> > please write back. we will defiantly look into it. All these changes
> >> > were merged in MW which is 2.5 months back, commenting in final stage
> >> > like this is not the professional way.
> >>
> >> Being release day, here's my big concern.  How bad is this?  Is it a
> >> single platform?  Later in the thread Jagan did enumerate the SoCs he
> >> tested the overall series on.  But there's a lot of Allwinner SoCs and
> >> boards.  I have a pine64 somewhere around here, but that's already been
> >> checked off.  My other allwinner platform I took out of my testing loop
> >> due to it not being a reliable piece of hardware.  So, does anyone have
> >> a feel for how many platforms may or may not be broken right now?
> >> Thanks!
> >
> >
> > You have 13 to 15 boards. Can you just report all of them?
>
> Issue, seems to be on SCSI side. MMC is able to probe and boot (you
> may see initial logs on the thread). Enabling DM_MMC breaking SCSI
> reads, debugging same with Pablo will get back.
>

This looks like the same issue I reported a few weeks back, I never got
back to it as I don't have an A20 and getting novices to git bisects is a
level of time consuming I don't have, add in a bunch of travel etc.

Peter

_______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot
>