All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sam Edwards <cfsworks@gmail.com>
To: Andre Przywara <andre.przywara@arm.com>
Cc: u-boot@lists.denx.de
Subject: Re: [RFC PATCH 00/17] sunxi: rework pinctrl and add T113s support
Date: Sun, 18 Jun 2023 13:01:33 -0600	[thread overview]
Message-ID: <71369065-8727-c011-0f29-fa33e0eb785c@gmail.com> (raw)
In-Reply-To: <20230615010754.503f4912@slackpad.lan>

Hi Andre,

On 6/14/23 18:07, Andre Przywara wrote:
> So I finally found some time to address some issues in the series,
> especially in the first patches (pinctrl rework and preparation).
> I pushed a branch to https://github.com/apritzel/u-boot/commits/r528-rc
> I need to do more testing, most importantly regression testing on other
> SoCs, and will only be able to post something next week, I guess.
> 
> If you could briefly list the things that are still missing, I could
> try to pick some low hanging fruits.

Rebasing onto this branch ended up eliminating a good chunk of my local 
hack commits. I've verified that everything is still working (but have 
not yet retested NAND SPL).

The remaining local changes I have are two additions to cpu_sunxi_ncat2.h:
+#define SUNXI_R_CPUCFG_BASE		0x07000400 /* for PSCI */
+#define SUNXI_RTC_BASE			0x07090000 /* for FEL */

The former can probably be brought into my PSCI series somehow (unless 
we expect more chips with CPUX blocks which might move those soft entry 
registers around, then it should be defined in cpu_sunxi_*.h). The 
latter is to support a reimplementation of Allwinner's `efex` command 
that I'm using for development (it pokes the magic number 0x5AA5A55A 
into RTC's GP_DATA_REG[2] and then resets; SPL clears that magic number 
and then does an early branch to BROM+0x0020 -- exactly what Allwinner's 
fork does).

I've also noticed exactly(!) one formatting difference in our clk_d1.c:
-	.num_gates = ARRAY_SIZE(d1_gates),
+	.num_gates  = ARRAY_SIZE(d1_gates),

Up to you if you prefer to align the = or not, but it does look 
inconsistent when .gates and .resets are aligned and .num_* aren't - 
might be a nitpick that comes up in patch review.

> Interesting, indeed this is left at 0, which I think will result in 288
> MHz.

Correct, at least that's what I was seeing.

> What is that frequency in your case? Do you know what the BSP
> programs?

1008 MHz, both.

> Traditionally we used something conservative that works
> without cooling and with the default voltage, but I don't know that
> value for the T113s.

For what it's worth, this board has a bare T113-s3 and the current OS 
does not reclock from 1008 MHz at all, and I don't know of any users of 
the board having stability issues.

In my own case, it idles at that clock at around ~35°C.

> I think CLK_SUN20I_D1 should be set by default now, so can you check
> that this is fixed?

It is now gone from my defconfig and still working, so indeed this is fixed.

> Why would we need H6 PSCI support? On the ARMv8 parts we use Trusted
> Firmware-A (TF-A) to provide PSCI services, which has a much more mature
> implementation.

It's not about the H6 and more about me being unsure whether R528/T113 
is the first ARMv7-based SoC to use the new CPU management registers. If 
it's not, and there's another such chip supported in U-Boot that just 
lacks PSCI, it would make more sense for me to land my PSCI series 
independently of our work here, and then you can add the R528 case 
later. It sounds like R528/T113 may be the first such chip needing this 
new code, though, so this may have to wait until the R528 series lands.

> How would this conflict, exactly? I don't see any other I2C2
> definition?

Well, no, the other definitions haven't landed in U-Boot yet. But they 
do exist in the kernel, datasheets, and physical chips themselves:

PB0/PB1/PB8/PB9/PE4/PE5: i2c2 defined as muxval 4
PC0/PC1/PD20/PD21/PG6/PG7/PG14/PG15: i2c2 defined as muxval 3
PE12/PE13: i2c2 defined as muxval 2

Defining i2c2=2 universally would mean that the pins for i2c2 cannot be 
changed, since it would conflict with every other definition.

> And what do you need I2C2 for, exactly?

Pins PE12/PE13 host an I²C bus with the board ID EEPROM and an Ethernet 
switch that should be reset (and have a few registers set to configure 
proper port isolation) very shortly after power-on.

> Well, there are shortcuts. I sketched some simpler idea in the comment
> at the top of pinctrl-sunxi.c.

My shortcut for the time being will probably be, "downstream patch."

>> At this time I have no interest in upstreaming the DT.
> 
> Why not?
> 
>> That might change
>> in the future, but for now it's very much meant to be out-of-tree.
> 
> Why is this? This only increases your update burden, and we might break
> something and not realise that, if your DT is not in the tree.
> The question to ask should be rather: why *not* to upstream the DT?
> Please keep in mind that this would block U-Boot support, since we need
> the DT approved in the kernel before we could merge it into U-Boot.

Currently, downstream is still fairly dependent on the Tina Linux 
kernel, not mainline. This is a situation I'd like to change, but it's a 
push for another day -- my focus right now is only on improving the 
bootloader situation.

This means that there are actually two DTs: one for the kernel, using 
the Tina Linux binding values, and one for U-Boot's control FDT, which 
can only support U-Boot right now (and cannot yet be tested on a real 
kernel). So neither DT is acceptable upstream: the former uses 
incompatible values/includes, and the latter isn't meant for Linux.

Even after(/if) this situation is resolved, the unified DT will probably 
remain in a state of flux for a while, until some drivers can be updated 
upstream (there's a slight mess with the I²C driver that needs to be 
cleaned up and we have to use GPIO-bitbanged I²C until then, for 
example) so it'll take more work before we have a "final" DT. At *that* 
time, upstreaming would be a good idea...

...but for now it's very much meant to be out-of-tree. :)

(I also do not work for the company that produced this board -- I'm just 
a contributor to the firmware project. Whether the project would even 
use the mainline version of its DT in the first place is, though likely, 
ultimately not my call.)

> Cheers,
> Andre

Likewise,
Sam

  reply	other threads:[~2023-06-18 19:01 UTC|newest]

Thread overview: 62+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-06  0:45 [RFC PATCH 00/17] sunxi: rework pinctrl and add T113s support Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 01/17] sunxi: remove CONFIG_SATAPWR Andre Przywara
2022-12-14  8:37   ` Samuel Holland
2022-12-14 14:25     ` Andre Przywara
2022-12-14 23:40       ` Samuel Holland
2022-12-06  0:45 ` [RFC PATCH 02/17] sunxi: remove CONFIG_MACPWR Andre Przywara
2022-12-14  9:09   ` Samuel Holland
2022-12-14 14:23     ` Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 03/17] pinctrl: sunxi: remove struct sunxi_gpio Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 04/17] pinctrl: sunxi: add GPIO in/out wrappers Andre Przywara
2022-12-15  5:59   ` Samuel Holland
2022-12-06  0:45 ` [RFC PATCH 05/17] pinctrl: sunxi: move pinctrl code and remove GPIO_EXTRA_HEADER Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 06/17] pinctrl: sunxi: move PIO_BASE into sunxi_gpio.h Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 07/17] pinctrl: sunxi: add new D1 pinctrl support Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 08/17] sunxi: introduce NCAT2 generation model Andre Przywara
2022-12-06  5:38   ` Icenowy Zheng
2023-05-16  2:32   ` Sam Edwards
2023-05-16 21:08     ` Andre Przywara
2023-05-16 23:53       ` Sam Edwards
2023-05-17  0:43         ` Andre Przywara
2023-05-17  8:56           ` Andre Przywara
2023-05-17 14:04             ` Maxim Kiselev
2023-05-25 18:25               ` Maksim Kiselev
2023-05-26 11:05                 ` Andre Przywara
2023-06-03 18:03   ` Sam Edwards
2022-12-06  0:45 ` [RFC PATCH 09/17] pinctrl: sunxi: add Allwinner D1 pinctrl description Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 10/17] clk: sunxi: Add support for the D1 CCU Andre Przywara
2023-05-22  3:57   ` Sam Edwards
2023-05-24  0:58     ` Andre Przywara
2023-05-26  0:34   ` Sam Edwards
2023-05-26 10:50     ` Andre Przywara
2023-05-26 19:27       ` Maksim Kiselev
2023-05-26 20:22         ` Sam Edwards
2023-05-26 22:07           ` Andre Przywara
2023-05-27  2:15             ` Sam Edwards
2023-05-30  0:58               ` Sam Edwards
2023-05-31 15:19                 ` Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 11/17] sunxi: clock: D1/R528: Enable PLL LDO during PLL1 setup Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 12/17] sunxi: clock: support D1/R528 PLL6 clock Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 13/17] sunxi: add early Allwinner R528/T113 SoC support Andre Przywara
2023-05-16  2:52   ` Sam Edwards
2023-05-16 22:01     ` Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 14/17] sunxi: refactor serial base addresses to avoid asm/arch/cpu.h Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 15/17] riscv: dts: allwinner: Add the D1/D1s SoC devicetree Andre Przywara
2022-12-06  0:45 ` [RFC PATCH 16/17] arm: sunxi: add Allwinner T113s devicetree stub Andre Przywara
2022-12-06  5:55   ` Icenowy Zheng
2023-01-03 17:38     ` Andre Przywara
2023-01-04  5:49       ` Icenowy Zheng
2022-12-06  0:45 ` [RFC PATCH 17/17] sunxi: add preliminary MangoPi MQ-R board support Andre Przywara
2023-06-09 22:16 ` [RFC PATCH 00/17] sunxi: rework pinctrl and add T113s support Sam Edwards
2023-06-12  0:20   ` Andre Przywara
2023-06-12 21:18     ` Sam Edwards
2023-06-15  0:07       ` Andre Przywara
2023-06-18 19:01         ` Sam Edwards [this message]
2023-06-20 12:42           ` Andre Przywara
2023-06-20 22:11             ` Sam Edwards
2023-06-21 10:55               ` Andre Przywara
2023-06-21 20:22                 ` Sam Edwards
2023-06-16 15:59       ` Andre Przywara
2023-06-16 16:27         ` Maxim Kiselev
2023-06-16 16:36           ` Andre Przywara
2023-06-17  8:26             ` Maxim Kiselev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=71369065-8727-c011-0f29-fa33e0eb785c@gmail.com \
    --to=cfsworks@gmail.com \
    --cc=andre.przywara@arm.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.