The 05/24/2021 10:46, Joel Stanley wrote: > On Mon, 24 May 2021 at 02:35, Steven Lee <steven_lee@xxxxxxxxxxxxxx> wrote: > > > > The 05/21/2021 09:25, Joel Stanley wrote: > > > Hi Steven, > > > > > > On Thu, 20 May 2021 at 10:16, Steven Lee <steven_lee@xxxxxxxxxxxxxx> wrote: > > > > > > > > AST2600 A2(or newer) EVB has gpio regulators for toggling signal voltage > > > > between 3.3v and 1.8v, the patch adds sdhci node and gpio regulator in the > > > > new dts file and adds commment for describing the reference design. > > > > > > spelling: comment > > > > > > > Thanks, will correct the typo. > > > > > I need you to justify the separate dts for the A2 EVB. > > > > > > What would happen if this device tree was loaded on to an A1 or A0? > > > > > > > Since the clock default value(SCU210) of A1 and A0 are different to A2, > > the following error would happen if A2 device tree was loaded on A1/A0. > > > > ``` > > [ 133.179825] mmc1: Reset 0x4 never completed. > > [ 133.184599] mmc1: sdhci: ============ SDHCI REGISTER DUMP =========== > > [ 133.191786] mmc1: sdhci: Sys addr: 0x00000000 | Version: 0x00000002 > > [ 133.198972] mmc1: sdhci: Blk size: 0x00007008 | Blk cnt: 0x00000001 > > [ 133.206158] mmc1: sdhci: Argument: 0x00000c00 | Trn mode: 0x00000013 > > [ 133.213343] mmc1: sdhci: Present: 0x01f70001 | Host ctl: 0x00000011 > > [ 133.220528] mmc1: sdhci: Power: 0x0000000f | Blk gap: 0x00000000 > > [ 133.227713] mmc1: sdhci: Wake-up: 0x00000000 | Clock: 0x00008007 > > [ 133.234898] mmc1: sdhci: Timeout: 0x0000000b | Int stat: 0x00000000 > > [ 133.242083] mmc1: sdhci: Int enab: 0x00ff0083 | Sig enab: 0x00ff0083 > > [ 133.249268] mmc1: sdhci: ACmd stat: 0x00000000 | Slot int: 0x00000000 > > [ 133.256453] mmc1: sdhci: Caps: 0x07f80080 | Caps_1: 0x00000007 > > [ 133.263638] mmc1: sdhci: Cmd: 0x0000341a | Max curr: 0x001f0f08 > > [ 133.270824] mmc1: sdhci: Resp[0]: 0x00000000 | Resp[1]: 0x01dd7f7f > > [ 133.278009] mmc1: sdhci: Resp[2]: 0x325b5900 | Resp[3]: 0x00400e00 > > [ 133.285193] mmc1: sdhci: Host ctl2: 0x00000000 > > [ 133.290148] mmc1: sdhci: ADMA Err: 0x00000000 | ADMA Ptr: 0xbe041200 > > [ 133.297332] mmc1: sdhci: ============================================ > > > > ``` > > > > Besides, A1/A0 EVBs don't have regulator, vmmc and vqmmc should be > > removed from sdhci node of A1/A0 dts. > > > > > Would this device tree be used for the A3 (and any future revision?) > > > > > > > Yes, A3 can use the A2 dts. > > > > > An alternative proposal: we modify the ast2600-evb.dts to support the > > > A2 (which I assume would also support the A3). > > > > > > If we need a separate board file for the A0 and A1 EVB, we add a new > > > one that supports these earlier revisions. Or we decide to only > > > support the latest revision in mainline. > > > > > > > In this patch, I add a new dts to support A2 sdhci, and include the > > original dts since the other settings can be loaded on A2. > > Do you mean creating a new file(e.g. aspeed-ast2600-evb-a1.dts) for A1, > > and modifying the original aspeed-ast2600-evb.dts for supporting A2? > > Yes, that would be my suggestion. The aspeed-ast2600-evb-a1.dts could > include the aspeed-ast2600-evb.dts. > Thanks for the prompt reply, I will add aspeed-ast2600-evb-a1.dts for A1 and A0. > > If we decide to only support the latest version in mainline, users > > should mark vmmc and vqmmc as comment and modify clk-phase manually > > for supporting A1. > > If you believe there will be users of the A1 for some time, then I > think it makes sense to support both A1 and future boards in mainline. > > Cheers, > > Joel
